From 81238f893cbc9e71fc22acc03a25101869de2149 Mon Sep 17 00:00:00 2001 From: Emily Hedlund Date: Tue, 7 Apr 2026 12:05:22 -0700 Subject: [PATCH] merge main and revert files --- .../core/src/sandbox/windows/GeminiSandbox.cs | 247 +++++++-- .../windows/WindowsSandboxManager.test.ts | 295 ++++++----- .../sandbox/windows/WindowsSandboxManager.ts | 483 +++++++----------- .../sandboxManager.integration.test.ts | 9 +- 4 files changed, 569 insertions(+), 465 deletions(-) diff --git a/packages/core/src/sandbox/windows/GeminiSandbox.cs b/packages/core/src/sandbox/windows/GeminiSandbox.cs index eef08b250b..9216a90508 100644 --- a/packages/core/src/sandbox/windows/GeminiSandbox.cs +++ b/packages/core/src/sandbox/windows/GeminiSandbox.cs @@ -142,6 +142,12 @@ public class GeminiSandbox { [DllImport("advapi32.dll", SetLastError = true, CharSet = CharSet.Auto)] static extern bool ConvertStringSidToSid(string StringSid, out IntPtr ptrSid); + [DllImport("advapi32.dll", SetLastError = true)] + static extern bool InitializeAcl(IntPtr pAcl, uint nAclLength, uint dwAclRevision); + + [DllImport("advapi32.dll", SetLastError = true)] + static extern bool AddMandatoryAce(IntPtr pAcl, uint dwAceRevision, uint AceFlags, uint MandatoryPolicy, IntPtr pLabelSid); + [DllImport("advapi32.dll", SetLastError = true)] static extern bool SetTokenInformation(IntPtr TokenHandle, int TokenInformationClass, IntPtr TokenInformation, uint TokenInformationLength); @@ -156,33 +162,99 @@ public class GeminiSandbox { public SID_AND_ATTRIBUTES Label; } + [DllImport("advapi32.dll", CharSet = CharSet.Auto, SetLastError = true)] + static extern bool ConvertStringSecurityDescriptorToSecurityDescriptor(string StringSecurityDescriptor, uint StringSDRevision, out IntPtr SecurityDescriptor, out uint SecurityDescriptorSize); + + [DllImport("advapi32.dll", SetLastError = true)] + static extern bool GetSecurityDescriptorSacl(IntPtr pSecurityDescriptor, out bool lpbSaclPresent, out IntPtr pSacl, out bool lpbSaclDefaulted); + + [DllImport("advapi32.dll", CharSet = CharSet.Auto, SetLastError = true)] + static extern uint SetNamedSecurityInfo(string pObjectName, int ObjectType, uint SecurityInfo, IntPtr psidOwner, IntPtr psidGroup, IntPtr pDacl, IntPtr pSacl); + + [DllImport("kernel32.dll")] + static extern IntPtr LocalFree(IntPtr hMem); + + [StructLayout(LayoutKind.Sequential, Pack = 1)] + struct TOKEN_PRIVILEGES { + public uint PrivilegeCount; + public LUID_AND_ATTRIBUTES Privileges; + } + + [StructLayout(LayoutKind.Sequential)] + struct LUID_AND_ATTRIBUTES { + public LUID Luid; + public uint Attributes; + } + + [StructLayout(LayoutKind.Sequential)] + struct LUID { + public uint LowPart; + public int HighPart; + } + + [DllImport("advapi32.dll", SetLastError = true)] + static extern bool AdjustTokenPrivileges(IntPtr TokenHandle, bool DisableAllPrivileges, ref TOKEN_PRIVILEGES NewState, uint BufferLength, IntPtr PreviousState, IntPtr ReturnLength); + + [DllImport("advapi32.dll", SetLastError = true, CharSet = CharSet.Auto)] + static extern bool LookupPrivilegeValue(string lpSystemName, string lpName, out LUID lpLuid); + + private const string SE_SECURITY_NAME = "SeSecurityPrivilege"; + private const uint SE_PRIVILEGE_ENABLED = 0x00000002; + private const uint TOKEN_ADJUST_PRIVILEGES = 0x0020; + private const uint TOKEN_QUERY = 0x0008; + + private static void EnablePrivilege(string privilege) { + IntPtr hToken; + if (OpenProcessToken(GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, out hToken)) { + try { + LUID luid; + if (LookupPrivilegeValue(null, privilege, out luid)) { + TOKEN_PRIVILEGES tp = new TOKEN_PRIVILEGES(); + tp.PrivilegeCount = 1; + tp.Privileges.Luid = luid; + tp.Privileges.Attributes = SE_PRIVILEGE_ENABLED; + AdjustTokenPrivileges(hToken, false, ref tp, 0, IntPtr.Zero, IntPtr.Zero); + } + } finally { + CloseHandle(hToken); + } + } + } + private const int TokenIntegrityLevel = 25; private const uint SE_GROUP_INTEGRITY = 0x00000020; private const uint TOKEN_ALL_ACCESS = 0xF01FF; private const uint DISABLE_MAX_PRIVILEGE = 0x1; + private const int SE_FILE_OBJECT = 1; + private const uint LABEL_SECURITY_INFORMATION = 0x00000010; + private const uint SECURITY_MANDATORY_LOW_RID = 0x00001000; + private const uint SYSTEM_MANDATORY_LABEL_NO_WRITE_UP = 0x1; + private const uint CONTAINER_INHERIT_ACE = 0x2; + private const uint OBJECT_INHERIT_ACE = 0x1; + + private static HashSet forbiddenPaths = new HashSet(StringComparer.OrdinalIgnoreCase); static int Main(string[] args) { if (args.Length < 3) { - Console.Error.WriteLine("Usage: GeminiSandbox.exe [--forbidden-manifest ] [args...]"); - Console.Error.WriteLine("Internal commands: __read , __write "); + Console.Error.WriteLine("Usage: GeminiSandbox.exe [--setup-manifest ] [args...]"); + Console.Error.WriteLine("Internal commands: __read , __write , __apply_batch_acls "); return 1; } - bool networkAccess = args[0] == "1"; - string cwd = args[1]; - HashSet forbiddenPaths = new HashSet(StringComparer.OrdinalIgnoreCase); - int argIndex = 2; + int argIndex = 0; + string networkArg = args[argIndex++]; + bool networkAccess = networkArg == "1"; + string cwd = args[argIndex++]; - if (argIndex < args.Length && args[argIndex] == "--forbidden-manifest") { + // Batch ACL command: __apply_batch_acls (maintained for backward compatibility/standalone use) + if (networkArg == "__apply_batch_acls") { + ApplyManifest(cwd); // In this case, cwd is actually the manifest path + return 0; + } + + if (argIndex < args.Length && args[argIndex] == "--setup-manifest") { if (argIndex + 1 < args.Length) { - string manifestPath = args[argIndex + 1]; - if (File.Exists(manifestPath)) { - foreach (string line in File.ReadAllLines(manifestPath)) { - if (!string.IsNullOrWhiteSpace(line)) { - forbiddenPaths.Add(GetNormalizedPath(line.Trim())); - } - } - } + ApplyManifest(args[argIndex + 1]); argIndex += 2; } } @@ -278,7 +350,7 @@ public class GeminiSandbox { return 1; } string path = args[argIndex + 1]; - CheckForbidden(path, forbiddenPaths); + CheckForbidden(path, cwd); return RunInImpersonation(hRestrictedToken, () => { try { using (FileStream fs = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.Read)) @@ -297,7 +369,7 @@ public class GeminiSandbox { return 1; } string path = args[argIndex + 1]; - CheckForbidden(path, forbiddenPaths); + CheckForbidden(path, cwd); try { using (MemoryStream ms = new MemoryStream()) { @@ -321,6 +393,22 @@ public class GeminiSandbox { } // External Process + string commandLine = ""; + for (int i = argIndex; i < args.Length; i++) { + if (i > argIndex) commandLine += " "; + commandLine += QuoteArgument(args[i]); + } + + // Third layer of defense: block the external process if it's trying to touch a forbidden path directly + foreach (string arg in args) { + try { + CheckForbidden(arg, cwd); + } catch (Exception e) { + Console.Error.WriteLine(e.Message); + return 1; + } + } + STARTUPINFO si = new STARTUPINFO(); si.cb = (uint)Marshal.SizeOf(si); si.dwFlags = 0x00000100; // STARTF_USESTDHANDLES @@ -328,12 +416,6 @@ public class GeminiSandbox { si.hStdOutput = GetStdHandle(-11); si.hStdError = GetStdHandle(-12); - string commandLine = ""; - for (int i = argIndex; i < args.Length; i++) { - if (i > argIndex) commandLine += " "; - commandLine += QuoteArgument(args[i]); - } - // Creation Flags: 0x01000000 (CREATE_BREAKAWAY_FROM_JOB) to allow job assignment if parent is in job // 0x00000004 (CREATE_SUSPENDED) to prevent the process from executing before being placed in the job uint creationFlags = 0x01000000 | 0x00000004; @@ -356,7 +438,7 @@ public class GeminiSandbox { int err = Marshal.GetLastWin32Error(); Console.Error.WriteLine("Error: WaitForSingleObject failed (" + err + ")"); } - + uint exitCode = 0; if (!GetExitCodeProcess(pi.hProcess, out exitCode)) { int err = Marshal.GetLastWin32Error(); @@ -395,21 +477,97 @@ public class GeminiSandbox { } } - private static string GetNormalizedPath(string path) { - string fullPath = Path.GetFullPath(path); - StringBuilder longPath = new StringBuilder(1024); - uint result = GetLongPathName(fullPath, longPath, (uint)longPath.Capacity); - if (result > 0 && result < longPath.Capacity) { - return longPath.ToString(); + private static void SetLowIntegritySacl(string path) { + IntPtr pSid = IntPtr.Zero; + IntPtr pSacl = IntPtr.Zero; + try { + if (ConvertStringSidToSid("S-1-16-4096", out pSid)) { + uint cbAcl = 100; + pSacl = Marshal.AllocHGlobal((int)cbAcl); + if (InitializeAcl(pSacl, cbAcl, 2)) { + if (AddMandatoryAce(pSacl, 2, CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE, SYSTEM_MANDATORY_LABEL_NO_WRITE_UP, pSid)) { + SetNamedSecurityInfo(path, 1, 16, IntPtr.Zero, IntPtr.Zero, IntPtr.Zero, pSacl); + } + } + } + } catch { + // Ignore errors for individual paths + } finally { + if (pSid != IntPtr.Zero) LocalFree(pSid); + if (pSacl != IntPtr.Zero) Marshal.FreeHGlobal(pSacl); } - return fullPath; } - private static void CheckForbidden(string path, HashSet forbiddenPaths) { - string fullPath = GetNormalizedPath(path); + private static void ApplyManifest(string manifestPath) { + EnablePrivilege(SE_SECURITY_NAME); + if (!File.Exists(manifestPath)) return; + foreach (string rawLine in File.ReadAllLines(manifestPath)) { + string line = rawLine.Trim(); + if (string.IsNullOrEmpty(line) || line.Length < 3) continue; + + // Handle UTF-8 BOM if present on the first line + if ((int)line[0] == 65279) line = line.Substring(1).Trim(); + if (line.Length < 3) continue; + + char op = line[0]; + string path = line.Substring(1).Trim(); + + if (op == 'L') { + SetLowIntegritySacl(path); + + try { + if (Directory.Exists(path)) { + DirectoryInfo dInfo = new DirectoryInfo(path); + DirectorySecurity ds = dInfo.GetAccessControl(); + ds.AddAccessRule(new FileSystemAccessRule(new SecurityIdentifier("S-1-16-4096"), FileSystemRights.Modify, InheritanceFlags.ContainerInherit | InheritanceFlags.ObjectInherit, PropagationFlags.None, AccessControlType.Allow)); + dInfo.SetAccessControl(ds); + } else if (File.Exists(path)) { + FileInfo fInfo = new FileInfo(path); + FileSecurity fs = fInfo.GetAccessControl(); + fs.AddAccessRule(new FileSystemAccessRule(new SecurityIdentifier("S-1-16-4096"), FileSystemRights.Modify, AccessControlType.Allow)); + fInfo.SetAccessControl(fs); + } + } catch { + // Ignore access errors + } + } else if (op == 'D') { + DenyLowIntegrityDacl(path); + forbiddenPaths.Add(GetNormalizedPath(path, null)); + } + } + } + + private static string GetNormalizedPath(string path, string basePath) { + try { + string absolutePath = path; + if (!Path.IsPathRooted(path) && !string.IsNullOrEmpty(basePath)) { + absolutePath = Path.Combine(basePath, path); + } + string fullPath = Path.GetFullPath(absolutePath); + StringBuilder longPath = new StringBuilder(1024); + uint result = GetLongPathName(fullPath, longPath, (uint)longPath.Capacity); + if (result > 0 && result < longPath.Capacity) { + return longPath.ToString(); + } + return fullPath; + } catch { + return path; + } + } + + private static void CheckForbidden(string arg, string basePath) { foreach (string forbidden in forbiddenPaths) { - if (fullPath.Equals(forbidden, StringComparison.OrdinalIgnoreCase) || fullPath.StartsWith(forbidden + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase)) { - throw new UnauthorizedAccessException("Access to forbidden path is denied: " + path); + if (arg.IndexOf(forbidden, StringComparison.OrdinalIgnoreCase) >= 0) { + throw new UnauthorizedAccessException("Access to forbidden path is denied (matched " + forbidden + "): " + arg); + } + } + + // Also check normalized path for direct hits + string fullPath = GetNormalizedPath(arg, basePath); + foreach (string forbidden in forbiddenPaths) { + if (fullPath.Equals(forbidden, StringComparison.OrdinalIgnoreCase) || + fullPath.StartsWith(forbidden + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase)) { + throw new UnauthorizedAccessException("Access to forbidden path is denied: " + arg); } } } @@ -456,4 +614,23 @@ public class GeminiSandbox { sb.Append('\"'); return sb.ToString(); } + + + private static void DenyLowIntegrityDacl(string path) { + try { + if (Directory.Exists(path)) { + DirectorySecurity ds = Directory.GetAccessControl(path); + ds.SetAccessRuleProtection(true, true); + ds.AddAccessRule(new FileSystemAccessRule(new SecurityIdentifier("S-1-16-4096"), FileSystemRights.FullControl, InheritanceFlags.ContainerInherit | InheritanceFlags.ObjectInherit, PropagationFlags.None, AccessControlType.Deny)); + Directory.SetAccessControl(path, ds); + } else if (File.Exists(path)) { + FileSecurity fs = File.GetAccessControl(path); + fs.SetAccessRuleProtection(true, true); + fs.AddAccessRule(new FileSystemAccessRule(new SecurityIdentifier("S-1-16-4096"), FileSystemRights.FullControl, AccessControlType.Deny)); + File.SetAccessControl(path, fs); + } + } catch (Exception e) { + Console.Error.WriteLine("Error in DenyLowIntegrityDacl for " + path + ": " + e.Message); + } + } } diff --git a/packages/core/src/sandbox/windows/WindowsSandboxManager.test.ts b/packages/core/src/sandbox/windows/WindowsSandboxManager.test.ts index c814f740f7..32da3ca208 100644 --- a/packages/core/src/sandbox/windows/WindowsSandboxManager.test.ts +++ b/packages/core/src/sandbox/windows/WindowsSandboxManager.test.ts @@ -11,22 +11,24 @@ import path from 'node:path'; import { WindowsSandboxManager } from './WindowsSandboxManager.js'; import * as sandboxManager from '../../services/sandboxManager.js'; import type { SandboxRequest } from '../../services/sandboxManager.js'; -import { spawnAsync } from '../../utils/shell-utils.js'; import type { SandboxPolicyManager } from '../../policy/sandboxPolicyManager.js'; +import { spawnAsync } from '../../utils/shell-utils.js'; vi.mock('../../utils/shell-utils.js', async (importOriginal) => { const actual = await importOriginal(); return { ...actual, - spawnAsync: vi.fn(), initializeShellParsers: vi.fn(), isStrictlyApproved: vi.fn().mockResolvedValue(true), + spawnAsync: vi + .fn() + .mockResolvedValue({ stdout: '', stderr: '', exitCode: 0 }), }; }); describe('WindowsSandboxManager', () => { - let manager: WindowsSandboxManager; + let manager: WindowsSandboxManager | undefined; let testCwd: string; /** @@ -58,6 +60,7 @@ describe('WindowsSandboxManager', () => { }); testCwd = createTempDir('cwd'); + vi.spyOn(fs, 'writeFileSync'); manager = new WindowsSandboxManager({ workspace: testCwd, @@ -74,6 +77,10 @@ describe('WindowsSandboxManager', () => { }); it('should prepare a GeminiSandbox.exe command', async () => { + manager = new WindowsSandboxManager({ + workspace: testCwd, + forbiddenPaths: async () => [], + }); const req: SandboxRequest = { command: 'whoami', args: ['/groups'], @@ -90,14 +97,18 @@ describe('WindowsSandboxManager', () => { expect(result.args).toEqual([ '0', testCwd, - '--forbidden-manifest', - expect.stringMatching(/manifest\.txt$/), + '--setup-manifest', + expect.stringMatching(/gemini-cli-sandbox-.*\.txt$/), 'whoami', '/groups', ]); }); it('should handle networkAccess from config', async () => { + manager = new WindowsSandboxManager({ + workspace: testCwd, + forbiddenPaths: async () => [], + }); const req: SandboxRequest = { command: 'whoami', args: [], @@ -128,20 +139,18 @@ describe('WindowsSandboxManager', () => { await manager.prepareCommand(req); - // Verify spawnAsync was called for icacls + // Verify spawnAsync was NOT called for icacls (batching manifest is used instead) const icaclsCalls = vi .mocked(spawnAsync) .mock.calls.filter((call) => call[0] === 'icacls'); - - // Should NOT have called icacls for C:\, D:\, etc. - const driveRootCalls = icaclsCalls.filter( - (call) => - typeof call[1]?.[0] === 'string' && /^[A-Z]:\\$/.test(call[1][0]), - ); - expect(driveRootCalls).toHaveLength(0); + expect(icaclsCalls).toHaveLength(0); }); it('should handle network access from additionalPermissions', async () => { + manager = new WindowsSandboxManager({ + workspace: testCwd, + forbiddenPaths: async () => [], + }); const req: SandboxRequest = { command: 'whoami', args: [], @@ -159,11 +168,12 @@ describe('WindowsSandboxManager', () => { }); it('should reject network access in Plan mode', async () => { - const planManager = new WindowsSandboxManager({ + manager = new WindowsSandboxManager({ workspace: testCwd, modeConfig: { readonly: true, allowOverrides: false }, forbiddenPaths: async () => [], }); + const req: SandboxRequest = { command: 'curl', args: ['google.com'], @@ -174,7 +184,7 @@ describe('WindowsSandboxManager', () => { }, }; - await expect(planManager.prepareCommand(req)).rejects.toThrow( + await expect(manager.prepareCommand(req)).rejects.toThrow( 'Sandbox request rejected: Cannot override readonly/network/filesystem restrictions in Plan mode.', ); }); @@ -189,7 +199,7 @@ describe('WindowsSandboxManager', () => { }), } as unknown as SandboxPolicyManager; - const managerWithPolicy = new WindowsSandboxManager({ + manager = new WindowsSandboxManager({ workspace: testCwd, modeConfig: { allowOverrides: true, network: false }, policyManager: mockPolicyManager, @@ -203,24 +213,22 @@ describe('WindowsSandboxManager', () => { env: {}, }; - const result = await managerWithPolicy.prepareCommand(req); + const result = await manager.prepareCommand(req); expect(result.args[0]).toBe('1'); // Network allowed by persistent policy - const icaclsArgs = vi - .mocked(spawnAsync) - .mock.calls.filter((c) => c[0] === 'icacls') - .map((c) => c[1]); - - expect(icaclsArgs).toContainEqual([ - persistentPath, - '/grant', - '*S-1-16-4096:(OI)(CI)(M)', - '/setintegritylevel', - '(OI)(CI)Low', - ]); + const writeFileSyncCalls = vi.mocked(fs.writeFileSync).mock.calls; + const aclCall = writeFileSyncCalls.find((call) => + String(call[0]).match(/gemini-cli-sandbox-.*\.txt$/), + ); + expect(aclCall).toBeDefined(); + expect(aclCall![1]).toContain(`L ${persistentPath}`); }); it('should sanitize environment variables', async () => { + manager = new WindowsSandboxManager({ + workspace: testCwd, + forbiddenPaths: async () => [], + }); const req: SandboxRequest = { command: 'test', args: [], @@ -244,6 +252,10 @@ describe('WindowsSandboxManager', () => { }); it('should ensure governance files exist', async () => { + manager = new WindowsSandboxManager({ + workspace: testCwd, + forbiddenPaths: async () => [], + }); const req: SandboxRequest = { command: 'test', args: [], @@ -272,28 +284,15 @@ describe('WindowsSandboxManager', () => { }, }; - await manager.prepareCommand(req); + await manager!.prepareCommand(req); - const icaclsArgs = vi - .mocked(spawnAsync) - .mock.calls.filter((c) => c[0] === 'icacls') - .map((c) => c[1]); - - expect(icaclsArgs).toContainEqual([ - testCwd, - '/grant', - '*S-1-16-4096:(OI)(CI)(M)', - '/setintegritylevel', - '(OI)(CI)Low', - ]); - - expect(icaclsArgs).toContainEqual([ - allowedPath, - '/grant', - '*S-1-16-4096:(OI)(CI)(M)', - '/setintegritylevel', - '(OI)(CI)Low', - ]); + const writeFileSyncCalls = vi.mocked(fs.writeFileSync).mock.calls; + const aclCall = writeFileSyncCalls.find((call) => + String(call[0]).match(/gemini-cli-sandbox-.*\.txt$/), + ); + expect(aclCall).toBeDefined(); + expect(aclCall![1]).toContain(`L ${path.resolve(testCwd)}`); + expect(aclCall![1]).toContain(`L ${path.resolve(allowedPath)}`); } finally { fs.rmSync(allowedPath, { recursive: true, force: true }); } @@ -316,28 +315,26 @@ describe('WindowsSandboxManager', () => { }, }; - await manager.prepareCommand(req); + await manager!.prepareCommand(req); - const icaclsArgs = vi - .mocked(spawnAsync) - .mock.calls.filter((c) => c[0] === 'icacls') - .map((c) => c[1]); - - expect(icaclsArgs).toContainEqual([ - extraWritePath, - '/grant', - '*S-1-16-4096:(OI)(CI)(M)', - '/setintegritylevel', - '(OI)(CI)Low', - ]); + const writeFileSyncCalls = vi.mocked(fs.writeFileSync).mock.calls; + const aclCall = writeFileSyncCalls.find((call) => + String(call[0]).match(/gemini-cli-sandbox-.*\.txt$/), + ); + expect(aclCall).toBeDefined(); + expect(aclCall![1]).toContain(`L ${path.resolve(extraWritePath)}`); } finally { fs.rmSync(extraWritePath, { recursive: true, force: true }); } }); it.runIf(process.platform === 'win32')( - 'should reject UNC paths in grantLowIntegrityAccess', + 'should reject UNC paths in grantLowIntegrityOp', async () => { + manager = new WindowsSandboxManager({ + workspace: testCwd, + forbiddenPaths: async () => [], + }); const uncPath = '\\\\attacker\\share\\malicious.txt'; const req: SandboxRequest = { command: 'test', @@ -356,12 +353,13 @@ describe('WindowsSandboxManager', () => { // Rejected because it's an unreachable/invalid UNC path or it doesn't exist await expect(manager.prepareCommand(req)).rejects.toThrow(); - const icaclsArgs = vi - .mocked(spawnAsync) - .mock.calls.filter((c) => c[0] === 'icacls') - .map((c) => c[1]); - - expect(icaclsArgs).not.toContainEqual(expect.arrayContaining([uncPath])); + const writeFileSyncCalls = vi.mocked(fs.writeFileSync).mock.calls; + const aclCall = writeFileSyncCalls.find((call) => + String(call[0]).match(/gemini-cli-sandbox-.*\.txt$/), + ); + if (aclCall) { + expect(aclCall[1]).not.toContain(`L ${uncPath}`); + } }, ); @@ -388,31 +386,19 @@ describe('WindowsSandboxManager', () => { }, }; - await manager.prepareCommand(req); + await manager!.prepareCommand(req); - const icaclsArgs = vi - .mocked(spawnAsync) - .mock.calls.filter((c) => c[0] === 'icacls') - .map((c) => c[1]); - - expect(icaclsArgs).toContainEqual([ - path.resolve(longPath), - '/grant', - '*S-1-16-4096:(OI)(CI)(M)', - '/setintegritylevel', - '(OI)(CI)Low', - ]); - expect(icaclsArgs).toContainEqual([ - path.resolve(devicePath), - '/grant', - '*S-1-16-4096:(OI)(CI)(M)', - '/setintegritylevel', - '(OI)(CI)Low', - ]); + const writeFileSyncCalls = vi.mocked(fs.writeFileSync).mock.calls; + const aclCall = writeFileSyncCalls.find((call) => + String(call[0]).match(/gemini-cli-sandbox-.*\.txt$/), + ); + expect(aclCall).toBeDefined(); + expect(aclCall![1]).toContain(`L ${path.resolve(longPath)}`); + expect(aclCall![1]).toContain(`L ${path.resolve(devicePath)}`); }, ); - it('skips denying access to non-existent forbidden paths to prevent icacls failure', async () => { + it('skips denying access to non-existent forbidden paths to prevent failure', async () => { const missingPath = path.join( os.tmpdir(), 'gemini-cli-test-missing', @@ -424,7 +410,7 @@ describe('WindowsSandboxManager', () => { fs.rmSync(missingPath, { recursive: true, force: true }); } - const managerWithForbidden = new WindowsSandboxManager({ + manager = new WindowsSandboxManager({ workspace: testCwd, forbiddenPaths: async () => [missingPath], }); @@ -436,20 +422,70 @@ describe('WindowsSandboxManager', () => { env: {}, }; - await managerWithForbidden.prepareCommand(req); + await manager.prepareCommand(req); - // Should NOT have called icacls to deny the missing path - expect(spawnAsync).not.toHaveBeenCalledWith('icacls', [ - path.resolve(missingPath), - '/deny', - '*S-1-16-4096:(OI)(CI)(F)', - ]); + // Should NOT have included the missing path in the ACL manifest + const writeFileSyncCalls = vi.mocked(fs.writeFileSync).mock.calls; + const aclCall = writeFileSyncCalls.find((call) => + String(call[0]).match(/gemini-cli-sandbox-.*\.txt$/), + ); + if (aclCall) { + expect(aclCall[1]).not.toContain(`D ${path.resolve(missingPath)}`); + } + }); + + it('should deny access to discovered secret files (e.g., .env)', async () => { + manager = new WindowsSandboxManager({ + workspace: testCwd, + forbiddenPaths: async () => [], + }); + const envFile = path.join(testCwd, '.env'); + fs.writeFileSync(envFile, 'API_KEY=secret'); + + const req: SandboxRequest = { + command: 'test', + args: [], + cwd: testCwd, + env: {}, + }; + + await manager.prepareCommand(req); + + const writeFileSyncCalls = vi.mocked(fs.writeFileSync).mock.calls; + const aclCall = writeFileSyncCalls.find((call) => + String(call[0]).match(/gemini-cli-sandbox-.*\.txt$/), + ); + expect(aclCall).toBeDefined(); + expect(aclCall![1]).toContain(`D ${path.resolve(envFile)}`); + }); + + describe('isKnownSafeCommand', () => { + it('should return true for approved tools in modeConfig', () => { + manager = new WindowsSandboxManager({ + workspace: testCwd, + modeConfig: { approvedTools: ['my-safe-tool'] }, + forbiddenPaths: async () => [], + }); + + expect(manager.isKnownSafeCommand(['my-safe-tool', 'arg'])).toBe(true); + expect(manager.isKnownSafeCommand(['MY-SAFE-TOOL', 'arg'])).toBe(true); + }); + + it('should fall back to default isKnownSafeCommand logic', () => { + manager = new WindowsSandboxManager({ + workspace: testCwd, + forbiddenPaths: async () => [], + }); + // 'git' is typically a known safe command in commandSafety.ts + expect(manager.isKnownSafeCommand(['git', 'status'])).toBe(true); + expect(manager.isKnownSafeCommand(['unknown-tool'])).toBe(false); + }); }); it('should deny Low Integrity access to forbidden paths', async () => { const forbiddenPath = createTempDir('forbidden'); try { - const managerWithForbidden = new WindowsSandboxManager({ + manager = new WindowsSandboxManager({ workspace: testCwd, forbiddenPaths: async () => [forbiddenPath], }); @@ -461,13 +497,14 @@ describe('WindowsSandboxManager', () => { env: {}, }; - await managerWithForbidden.prepareCommand(req); + await manager.prepareCommand(req); - expect(spawnAsync).toHaveBeenCalledWith('icacls', [ - forbiddenPath, - '/deny', - '*S-1-16-4096:(OI)(CI)(F)', - ]); + const writeFileSyncCalls = vi.mocked(fs.writeFileSync).mock.calls; + const aclCall = writeFileSyncCalls.find((call) => + String(call[0]).match(/gemini-cli-sandbox-.*\.txt$/), + ); + expect(aclCall).toBeDefined(); + expect(aclCall![1]).toContain(`D ${path.resolve(forbiddenPath)}`); } finally { fs.rmSync(forbiddenPath, { recursive: true, force: true }); } @@ -476,7 +513,7 @@ describe('WindowsSandboxManager', () => { it('should override allowed paths if a path is also in forbidden paths', async () => { const conflictPath = createTempDir('conflict'); try { - const managerWithForbidden = new WindowsSandboxManager({ + manager = new WindowsSandboxManager({ workspace: testCwd, forbiddenPaths: async () => [conflictPath], }); @@ -491,27 +528,21 @@ describe('WindowsSandboxManager', () => { }, }; - await managerWithForbidden.prepareCommand(req); + await manager.prepareCommand(req); - const spawnMock = vi.mocked(spawnAsync); - const allowCallIndex = spawnMock.mock.calls.findIndex( - (call) => - call[1] && - call[1].includes('/setintegritylevel') && - call[0] === 'icacls' && - call[1][0] === conflictPath, - ); - const denyCallIndex = spawnMock.mock.calls.findIndex( - (call) => - call[1] && - call[1].includes('/deny') && - call[0] === 'icacls' && - call[1][0] === conflictPath, + const writeFileSyncCalls = vi.mocked(fs.writeFileSync).mock.calls; + const aclCall = writeFileSyncCalls.find((call) => + String(call[0]).match(/gemini-cli-sandbox-.*\.txt$/), ); + expect(aclCall).toBeDefined(); - // Conflict should have been filtered out of allow calls - expect(allowCallIndex).toBe(-1); - expect(denyCallIndex).toBeGreaterThan(-1); + const content = String(aclCall![1]); + const allowIndex = content.indexOf(`L ${path.resolve(conflictPath)}`); + const denyIndex = content.indexOf(`D ${path.resolve(conflictPath)}`); + + // Forbidden path should have been filtered out of grant list + expect(allowIndex).toBe(-1); + expect(denyIndex).toBeGreaterThan(-1); } finally { fs.rmSync(conflictPath, { recursive: true, force: true }); } @@ -527,14 +558,14 @@ describe('WindowsSandboxManager', () => { env: {}, }; - const result = await manager.prepareCommand(req); + const result = await manager!.prepareCommand(req); - // [network, cwd, --forbidden-manifest, manifestPath, command, ...args] + // [network, cwd, --setup-manifest, manifestPath, command, ...args] expect(result.args[4]).toBe('__write'); expect(result.args[5]).toBe(filePath); }); - it('should safely handle special characters in __write path using environment variables', async () => { + it('should safely handle special characters in __write path', async () => { const maliciousPath = path.join(testCwd, 'foo & echo bar; ! .txt'); fs.writeFileSync(maliciousPath, ''); const req: SandboxRequest = { @@ -544,7 +575,7 @@ describe('WindowsSandboxManager', () => { env: {}, }; - const result = await manager.prepareCommand(req); + const result = await manager!.prepareCommand(req); // Native commands pass arguments directly; the binary handles quoting via QuoteArgument expect(result.args[4]).toBe('__write'); @@ -561,7 +592,7 @@ describe('WindowsSandboxManager', () => { env: {}, }; - const result = await manager.prepareCommand(req); + const result = await manager!.prepareCommand(req); expect(result.args[4]).toBe('__read'); expect(result.args[5]).toBe(filePath); @@ -575,14 +606,12 @@ describe('WindowsSandboxManager', () => { env: {}, }; - const result = await manager.prepareCommand(req); + const result = await manager!.prepareCommand(req); const manifestPath = result.args[3]; expect(fs.existsSync(manifestPath)).toBe(true); - expect(result.cleanup).toBeDefined(); result.cleanup?.(); expect(fs.existsSync(manifestPath)).toBe(false); - expect(fs.existsSync(path.dirname(manifestPath))).toBe(false); }); }); diff --git a/packages/core/src/sandbox/windows/WindowsSandboxManager.ts b/packages/core/src/sandbox/windows/WindowsSandboxManager.ts index 943a339960..b4e0b05a07 100644 --- a/packages/core/src/sandbox/windows/WindowsSandboxManager.ts +++ b/packages/core/src/sandbox/windows/WindowsSandboxManager.ts @@ -41,7 +41,6 @@ const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); // S-1-16-4096 is the SID for "Low Mandatory Level" (Low Integrity) -const LOW_INTEGRITY_SID = '*S-1-16-4096'; /** * A SandboxManager implementation for Windows that uses Restricted Tokens, @@ -50,8 +49,15 @@ const LOW_INTEGRITY_SID = '*S-1-16-4096'; */ export class WindowsSandboxManager implements SandboxManager { static readonly HELPER_EXE = 'GeminiSandbox.exe'; - private readonly helperPath: string; + static readonly HELPER_SOURCE = 'GeminiSandbox.cs'; + + private helperPath: string; private initialized = false; + + /** + * Optimistically caches modified ACLs to prevent redundant Win32 API calls. + * Skips even if a previous application failed. + */ private readonly allowedCache = new Set(); private readonly deniedCache = new Set(); @@ -72,119 +78,51 @@ export class WindowsSandboxManager implements SandboxManager { return isDangerousCommand(args); } - parseDenials(result: ShellExecutionResult): ParsedSandboxDenial | undefined { - return parseWindowsSandboxDenials(result); - } - - getWorkspace(): string { - return this.options.workspace; - } - - getOptions(): GlobalSandboxOptions { - return this.options; - } - - /** - * Ensures a file or directory exists. - */ - private touch(filePath: string, isDirectory: boolean): void { - try { - // If it exists (even as a broken symlink), do nothing - if (fs.lstatSync(filePath)) return; - } catch { - // Ignore ENOENT - } - - if (isDirectory) { - fs.mkdirSync(filePath, { recursive: true }); - } else { - const dir = path.dirname(filePath); - if (!fs.existsSync(dir)) { - fs.mkdirSync(dir, { recursive: true }); - } - fs.closeSync(fs.openSync(filePath, 'a')); - } - } - private async ensureInitialized(): Promise { if (this.initialized) return; - if (os.platform() !== 'win32') { - this.initialized = true; - return; - } try { if (!fs.existsSync(this.helperPath)) { debugLogger.log( - `WindowsSandboxManager: Helper not found at ${this.helperPath}. Attempting to compile...`, + 'WindowsSandboxManager: Helper not found at', + this.helperPath, + ); + const sourcePath = path.resolve( + __dirname, + WindowsSandboxManager.HELPER_SOURCE, ); - // If the exe doesn't exist, we try to compile it from the .cs file - const sourcePath = this.helperPath.replace(/\.exe$/, '.cs'); if (fs.existsSync(sourcePath)) { - const systemRoot = process.env['SystemRoot'] || 'C:\\Windows'; - const cscPaths = [ - 'csc.exe', // Try in PATH first - path.join( + debugLogger.log( + 'WindowsSandboxManager: Compiling helper from source...', + ); + try { + // Try to compile using csc.exe (C# compiler, usually in Windows\Microsoft.NET\Framework64\v4.0.30319) + const systemRoot = process.env['SystemRoot'] || 'C:\\Windows'; + const cscPath = path.join( systemRoot, 'Microsoft.NET', 'Framework64', 'v4.0.30319', 'csc.exe', - ), - path.join( - systemRoot, - 'Microsoft.NET', - 'Framework', - 'v4.0.30319', - 'csc.exe', - ), - // Added newer framework paths - path.join( - systemRoot, - 'Microsoft.NET', - 'Framework64', - 'v4.8', - 'csc.exe', - ), - path.join( - systemRoot, - 'Microsoft.NET', - 'Framework', - 'v4.8', - 'csc.exe', - ), - path.join( - systemRoot, - 'Microsoft.NET', - 'Framework64', - 'v3.5', - 'csc.exe', - ), - ]; - - let compiled = false; - for (const csc of cscPaths) { - try { + ); + if (fs.existsSync(cscPath)) { + await spawnAsync(cscPath, [ + '/target:exe', + `/out:${this.helperPath}`, + sourcePath, + ]); debugLogger.log( - `WindowsSandboxManager: Trying to compile using ${csc}...`, + `WindowsSandboxManager: Compiled helper to ${this.helperPath}`, ); - // We use spawnAsync but we don't need to capture output - await spawnAsync(csc, ['/out:' + this.helperPath, sourcePath]); + } else { debugLogger.log( - `WindowsSandboxManager: Successfully compiled sandbox helper at ${this.helperPath}`, - ); - compiled = true; - break; - } catch (e) { - debugLogger.log( - `WindowsSandboxManager: Failed to compile using ${csc}: ${e instanceof Error ? e.message : String(e)}`, + 'WindowsSandboxManager: csc.exe not found. Cannot compile helper.', ); } - } - - if (!compiled) { + } catch (e) { debugLogger.log( - 'WindowsSandboxManager: Failed to compile sandbox helper from any known CSC path.', + 'WindowsSandboxManager: Failed to compile helper:', + e, ); } } else { @@ -274,7 +212,9 @@ export class WindowsSandboxManager implements SandboxManager { // New files created within these roots will inherit the Low label. const writableRoots: string[] = []; - // 1. Workspace access + // 1. Determine filesystem permissions to grant + const pathsToGrant = new Set(); + const isApproved = allowOverrides ? await isStrictlyApproved( command, @@ -284,43 +224,24 @@ export class WindowsSandboxManager implements SandboxManager { : false; if (!isReadonlyMode || isApproved) { - await this.grantLowIntegrityAccess(this.options.workspace); + pathsToGrant.add(this.options.workspace); writableRoots.push(this.options.workspace); } - // 2. Globally included directories - const includeDirs = sanitizePaths(this.options.includeDirectories); - for (const includeDir of includeDirs) { - await this.grantLowIntegrityAccess(includeDir); - writableRoots.push(includeDir); - } - - // 3. Explicitly allowed paths from the request policy - for (const allowedPath of allowedPaths) { - const resolved = resolveToRealPath(allowedPath); - try { - await fs.promises.access(resolved, fs.constants.F_OK); - } catch { - throw new Error( - `Sandbox request rejected: Allowed path does not exist: ${resolved}. ` + - 'On Windows, granular sandbox access can only be granted to existing paths to avoid broad parent directory permissions.', - ); - } - await this.grantLowIntegrityAccess(resolved); + allowedPaths.forEach((p) => { + const resolved = resolveToRealPath(p); + pathsToGrant.add(resolved); writableRoots.push(resolved); - } + }); - // 4. Additional write paths (e.g. from internal __write command) - const additionalWritePaths = sanitizePaths( - mergedAdditional.fileSystem?.write, - ); - for (const writePath of additionalWritePaths) { - const resolved = resolveToRealPath(writePath); - try { - await fs.promises.access(resolved, fs.constants.F_OK); - await this.grantLowIntegrityAccess(resolved); - continue; - } catch { + const extraWritePaths = + sanitizePaths(mergedAdditional.fileSystem?.write) || []; + extraWritePaths.forEach((p) => { + const resolved = resolveToRealPath(p); + if (fs.existsSync(resolved)) { + pathsToGrant.add(resolved); + writableRoots.push(resolved); + } else { // If the file doesn't exist, it's only allowed if it resides within a granted root. const isInherited = writableRoots.some((root) => isSubpath(root, resolved), @@ -333,207 +254,150 @@ export class WindowsSandboxManager implements SandboxManager { ); } } - } + }); - // 2. Collect secret files and apply protective ACLs - // On Windows, we explicitly deny access to secret files for Low Integrity - // processes to ensure they cannot be read or written. - const secretsToBlock: string[] = []; + const includeDirs = sanitizePaths(this.options.includeDirectories); + includeDirs.forEach((p) => { + const resolved = resolveToRealPath(p); + pathsToGrant.add(resolved); + writableRoots.push(resolved); + }); + + // 2. Identify forbidden paths and secrets to deny + const pathsToDeny = new Set(); + forbiddenPaths.forEach((p) => pathsToDeny.add(resolveToRealPath(p))); + + // Scoped scan for secrets to explicitly block for Low Integrity processes const searchDirs = new Set([ this.options.workspace, ...allowedPaths, ...includeDirs, ]); - for (const dir of searchDirs) { - try { - // We use maxDepth 3 to catch common nested secrets while keeping performance high. - const secretFiles = await findSecretFiles(dir, 3); - for (const secretFile of secretFiles) { - try { - secretsToBlock.push(secretFile); - await this.denyLowIntegrityAccess(secretFile); - } catch (e) { - debugLogger.log( - `WindowsSandboxManager: Failed to secure secret file ${secretFile}`, - e, - ); - } + await Promise.all( + Array.from(searchDirs).map(async (dir) => { + try { + // We use maxDepth 3 to catch common nested secrets while keeping performance high. + const secrets = await findSecretFiles(dir, 3); + secrets.forEach((s) => pathsToDeny.add(resolveToRealPath(s))); + } catch (e) { + debugLogger.log( + `WindowsSandboxManager: Secret scan failed for ${dir}`, + e, + ); } - } catch (e) { - debugLogger.log( - `WindowsSandboxManager: Failed to find secret files in ${dir}`, - e, - ); - } - } + }), + ); + + // 3. Reconcile Grant and Deny paths + for (const p of pathsToGrant) { + if (pathsToDeny.has(p)) { + pathsToGrant.delete(p); + continue; + } - // Denies access to forbiddenPaths for Low Integrity processes. - // Note: Denying access to arbitrary paths (like system files) via icacls - // is restricted to avoid host corruption. External commands rely on - // Low Integrity read/write restrictions, while internal commands - // use the manifest for enforcement. - for (const forbiddenPath of forbiddenPaths) { try { - await this.denyLowIntegrityAccess(forbiddenPath); - } catch (e) { - debugLogger.log( - `WindowsSandboxManager: Failed to secure forbidden path ${forbiddenPath}`, - e, - ); + await fs.promises.access(p, fs.constants.F_OK); + } catch { + // If it doesn't exist, we can't grant access on Windows. + pathsToGrant.delete(p); } } - // 3. Protected governance files - // These must exist on the host before running the sandbox to prevent - // the sandboxed process from creating them with Low integrity. - // By being created as Medium integrity, they are write-protected from Low processes. + // 4. Generate setup manifest operations (L = Grant, D = Deny) + const opResults = await Promise.all([ + ...Array.from(pathsToGrant).map((p) => this.getLowIntegrityOp(p, 'L')), + ...Array.from(pathsToDeny).map((p) => this.getLowIntegrityOp(p, 'D')), + ]); + + const pendingAcls = opResults.filter( + (op): op is string => op !== undefined, + ); + + // 5. Ensure governance files are write-protected for (const file of GOVERNANCE_FILES) { - const filePath = path.join(this.options.workspace, file.path); - this.touch(filePath, file.isDirectory); + this.touch( + path.join(this.options.workspace, file.path), + file.isDirectory, + ); } - // 4. Forbidden paths manifest - // We use a manifest file to avoid command-line length limits. - const allForbidden = Array.from( - new Set([...secretsToBlock, ...forbiddenPaths]), - ); - const tempDir = fs.mkdtempSync( - path.join(os.tmpdir(), 'gemini-cli-forbidden-'), - ); - const manifestPath = path.join(tempDir, 'manifest.txt'); - fs.writeFileSync(manifestPath, allForbidden.join('\n')); - - // 5. Construct the helper command - // GeminiSandbox.exe --forbidden-manifest [args...] - const program = this.helperPath; - - const finalArgs = [ - networkAccess ? '1' : '0', - req.cwd, - '--forbidden-manifest', - manifestPath, - command, - ...args, - ]; + // 6. Create setup manifest if needed + let manifestPath: string | undefined; + if (pendingAcls.length > 0) { + manifestPath = path.join( + os.tmpdir(), + `gemini-cli-sandbox-${Date.now()}-${Math.random().toString(36).slice(2)}.txt`, + ); + fs.writeFileSync(manifestPath, pendingAcls.join('\n'), { mode: 0o600 }); + } const finalEnv = { ...sanitizedEnv }; return { - program, - args: finalArgs, + program: this.helperPath, + args: [ + networkAccess ? '1' : '0', + req.cwd, + ...(manifestPath ? ['--setup-manifest', manifestPath] : []), + command, + ...args, + ], env: finalEnv, cwd: req.cwd, cleanup: () => { - try { - fs.rmSync(tempDir, { recursive: true, force: true }); - } catch { - // Ignore errors + if (manifestPath) { + try { + fs.unlinkSync(manifestPath); + } catch { + // Ignore cleanup errors + } } }, }; } /** - * Grants "Low Mandatory Level" access to a path using icacls. + * Resolves a path and generates a Grant (L) or Deny (D) operation for the setup manifest. + * Checks for platform, caches, system directories, and existence (for Deny). */ - private async grantLowIntegrityAccess(targetPath: string): Promise { - if (os.platform() !== 'win32') { - return; - } + private async getLowIntegrityOp( + targetPath: string, + mode: 'L' | 'D', + ): Promise { + if (os.platform() !== 'win32') return undefined; - const resolvedPath = resolveToRealPath(targetPath); - if (this.allowedCache.has(resolvedPath)) { - return; - } + const resolved = resolveToRealPath(targetPath); + const cache = mode === 'L' ? this.allowedCache : this.deniedCache; + if (cache.has(resolved)) return undefined; - // Explicitly reject UNC paths to prevent credential theft/SSRF, - // but allow local extended-length and device paths. + // Security: Block UNC paths for Grants (prevents NTLM exfiltration/SSRF) if ( - resolvedPath.startsWith('\\\\') && - !resolvedPath.startsWith('\\\\?\\') && - !resolvedPath.startsWith('\\\\.\\') + mode === 'L' && + resolved.startsWith('\\\\') && + !resolved.startsWith('\\\\?\\') && + !resolved.startsWith('\\\\.\\') ) { debugLogger.log( - 'WindowsSandboxManager: Rejecting UNC path for Low Integrity grant:', - resolvedPath, + 'WindowsSandboxManager: Rejecting UNC path for grant:', + resolved, ); - return; + return undefined; } - if (this.isSystemDirectory(resolvedPath)) { - return; - } + if (this.isSystemDirectory(resolved)) return undefined; - try { - // 1. Grant explicit Modify access to the Low Integrity SID - // 2. Set the Mandatory Label to Low to allow "Write Up" from Low processes - await spawnAsync('icacls', [ - resolvedPath, - '/grant', - `${LOW_INTEGRITY_SID}:(OI)(CI)(M)`, - '/setintegritylevel', - '(OI)(CI)Low', - ]); - this.allowedCache.add(resolvedPath); - } catch (e) { - debugLogger.log( - 'WindowsSandboxManager: icacls failed for', - resolvedPath, - e, - ); - } - } - - /** - * Explicitly denies access to a path for Low Integrity processes using icacls. - */ - private async denyLowIntegrityAccess(targetPath: string): Promise { - if (os.platform() !== 'win32') { - return; - } - - const resolvedPath = resolveToRealPath(targetPath); - if (this.deniedCache.has(resolvedPath)) { - return; - } - - // Never modify ACEs for system directories - if (this.isSystemDirectory(resolvedPath)) { - return; - } - - // icacls flags: (OI) Object Inherit, (CI) Container Inherit, (F) Full Access Deny. - // Omit /T (recursive) for performance; (OI)(CI) ensures inheritance for new items. - // Windows dynamically evaluates existing items, though deep explicit Allow ACEs - // could potentially bypass this inherited Deny rule. - const DENY_ALL_INHERIT = '(OI)(CI)(F)'; - - // icacls fails on non-existent paths, so we cannot explicitly deny - // paths that do not yet exist (unlike macOS/Linux). - // Skip to prevent sandbox initialization failure. - try { - await fs.promises.stat(resolvedPath); - } catch (e: unknown) { - if (isNodeError(e) && e.code === 'ENOENT') { - return; + // Deny ops fail if the path doesn't exist + if (mode === 'D') { + try { + await fs.promises.stat(resolved); + } catch (e: unknown) { + if (isNodeError(e) && e.code === 'ENOENT') return undefined; + throw e; } - throw e; } - try { - await spawnAsync('icacls', [ - resolvedPath, - '/deny', - `${LOW_INTEGRITY_SID}:${DENY_ALL_INHERIT}`, - ]); - this.deniedCache.add(resolvedPath); - } catch (e) { - throw new Error( - `Failed to deny access to forbidden path: ${resolvedPath}. ${ - e instanceof Error ? e.message : String(e) - }`, - ); - } + cache.add(resolved); + return `${mode} ${resolved}`; } private isSystemDirectory(resolvedPath: string): boolean { @@ -543,9 +407,44 @@ export class WindowsSandboxManager implements SandboxManager { process.env['ProgramFiles(x86)'] || 'C:\\Program Files (x86)'; return ( - resolvedPath.toLowerCase().startsWith(systemRoot.toLowerCase()) || - resolvedPath.toLowerCase().startsWith(programFiles.toLowerCase()) || - resolvedPath.toLowerCase().startsWith(programFilesX86.toLowerCase()) + isSubpath(systemRoot, resolvedPath) || + isSubpath(programFiles, resolvedPath) || + isSubpath(programFilesX86, resolvedPath) ); } + + /** + * Touches a file or directory to ensure it exists. + */ + private touch(filePath: string, isDirectory: boolean): void { + try { + if (isDirectory) { + if (!fs.existsSync(filePath)) { + fs.mkdirSync(filePath, { recursive: true }); + } + } else { + const dir = path.dirname(filePath); + if (!fs.existsSync(dir)) { + fs.mkdirSync(dir, { recursive: true }); + } + if (!fs.existsSync(filePath)) { + fs.writeFileSync(filePath, ''); + } + } + } catch (e) { + debugLogger.log(`WindowsSandboxManager: Failed to touch ${filePath}:`, e); + } + } + + parseDenials(result: ShellExecutionResult): ParsedSandboxDenial | undefined { + return parseWindowsSandboxDenials(result); + } + + getWorkspace(): string { + return this.options.workspace; + } + + getOptions(): GlobalSandboxOptions | undefined { + return this.options; + } } diff --git a/packages/core/src/services/sandboxManager.integration.test.ts b/packages/core/src/services/sandboxManager.integration.test.ts index 4923de97bf..d395f58f72 100644 --- a/packages/core/src/services/sandboxManager.integration.test.ts +++ b/packages/core/src/services/sandboxManager.integration.test.ts @@ -68,7 +68,7 @@ const Platform = { /** Returns a path that is strictly outside the workspace and likely blocked. */ getExternalBlockedPath() { return this.isWindows - ? 'C:\\Windows\\System32\\drivers\\etc\\hosts' + ? 'C:\\gemini_blocked_test.txt' : '/Users/Shared/.gemini_test_blocked'; }, }; @@ -103,10 +103,9 @@ function ensureSandboxAvailable(): boolean { const platform = os.platform(); if (platform === 'win32') { - // Windows sandboxing relies on icacls, which is a core system utility and - // always available. - // TODO: reenable once flakiness is addressed - return false; + // Windows sandboxing relies on the GeminiSandbox.exe helper, which is compiled + // using the built-in .NET Framework compiler (csc.exe) and is always available. + return true; } if (platform === 'darwin') {