feat(core): implement Windows sandbox dynamic expansion Phase 1 and 2.1 (#23691)

packages/core/src/services/sandboxManager.test.ts

@@ -3,13 +3,13 @@
  * Copyright 2025 Google LLC
  * SPDX-License-Identifier: Apache-2.0
  */
-
 import os from 'node:os';
 import path from 'node:path';
 import fs from 'node:fs/promises';
-import { describe, expect, it, vi, beforeEach } from 'vitest';
+import { afterEach, describe, expect, it, vi, beforeEach } from 'vitest';
 import {
   NoopSandboxManager,
+  LocalSandboxManager,
   sanitizePaths,
   tryRealpath,
 } from './sandboxManager.js';
@@ -18,225 +18,265 @@ import { LinuxSandboxManager } from '../sandbox/linux/LinuxSandboxManager.js';
 import { MacOsSandboxManager } from '../sandbox/macos/MacOsSandboxManager.js';
 import { WindowsSandboxManager } from '../sandbox/windows/WindowsSandboxManager.js';
 
-describe('sanitizePaths', () => {
-  it('should return undefined if no paths are provided', () => {
-    expect(sanitizePaths(undefined)).toBeUndefined();
-  });
+describe('SandboxManager', () => {
+  afterEach(() => vi.restoreAllMocks());
 
-  it('should deduplicate paths and return them', () => {
-    const paths = ['/workspace/foo', '/workspace/bar', '/workspace/foo'];
-    expect(sanitizePaths(paths)).toEqual(['/workspace/foo', '/workspace/bar']);
-  });
-
-  it('should throw an error if a path is not absolute', () => {
-    const paths = ['/workspace/foo', 'relative/path'];
-    expect(() => sanitizePaths(paths)).toThrow(
-      'Sandbox path must be absolute: relative/path',
-    );
-  });
-});
-
-describe('tryRealpath', () => {
-  beforeEach(() => {
-    vi.clearAllMocks();
-  });
-
-  it('should return the realpath if the file exists', async () => {
-    vi.spyOn(fs, 'realpath').mockResolvedValue('/real/path/to/file.txt');
-    const result = await tryRealpath('/some/symlink/to/file.txt');
-    expect(result).toBe('/real/path/to/file.txt');
-    expect(fs.realpath).toHaveBeenCalledWith('/some/symlink/to/file.txt');
-  });
-
-  it('should fallback to parent directory if file does not exist (ENOENT)', async () => {
-    vi.spyOn(fs, 'realpath').mockImplementation(async (p) => {
-      if (p === '/workspace/nonexistent.txt') {
-        throw Object.assign(new Error('ENOENT: no such file or directory'), {
-          code: 'ENOENT',
-        });
-      }
-      if (p === '/workspace') {
-        return '/real/workspace';
-      }
-      throw new Error(`Unexpected path: ${p}`);
+  describe('sanitizePaths', () => {
+    it('should return undefined if no paths are provided', () => {
+      expect(sanitizePaths(undefined)).toBeUndefined();
     });
 
-    const result = await tryRealpath('/workspace/nonexistent.txt');
-
-    // It should combine the real path of the parent with the original basename
-    expect(result).toBe(path.join('/real/workspace', 'nonexistent.txt'));
-  });
-
-  it('should recursively fallback up the directory tree on multiple ENOENT errors', async () => {
-    vi.spyOn(fs, 'realpath').mockImplementation(async (p) => {
-      if (p === '/workspace/missing_dir/missing_file.txt') {
-        throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' });
-      }
-      if (p === '/workspace/missing_dir') {
-        throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' });
-      }
-      if (p === '/workspace') {
-        return '/real/workspace';
-      }
-      throw new Error(`Unexpected path: ${p}`);
+    it('should deduplicate paths and return them', () => {
+      const paths = ['/workspace/foo', '/workspace/bar', '/workspace/foo'];
+      expect(sanitizePaths(paths)).toEqual([
+        '/workspace/foo',
+        '/workspace/bar',
+      ]);
     });
 
-    const result = await tryRealpath('/workspace/missing_dir/missing_file.txt');
-
-    // It should resolve '/workspace' to '/real/workspace' and append the missing parts
-    expect(result).toBe(
-      path.join('/real/workspace', 'missing_dir', 'missing_file.txt'),
-    );
+    it('should throw an error if a path is not absolute', () => {
+      const paths = ['/workspace/foo', 'relative/path'];
+      expect(() => sanitizePaths(paths)).toThrow(
+        'Sandbox path must be absolute: relative/path',
+      );
+    });
   });
 
-  it('should return the path unchanged if it reaches the root directory and it still does not exist', async () => {
-    const rootPath = path.resolve('/');
-    vi.spyOn(fs, 'realpath').mockImplementation(async () => {
-      throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' });
+  describe('tryRealpath', () => {
+    beforeEach(() => {
+      vi.clearAllMocks();
     });
 
-    const result = await tryRealpath(rootPath);
-    expect(result).toBe(rootPath);
-  });
+    it('should return the realpath if the file exists', async () => {
+      vi.spyOn(fs, 'realpath').mockResolvedValue('/real/path/to/file.txt');
+      const result = await tryRealpath('/some/symlink/to/file.txt');
+      expect(result).toBe('/real/path/to/file.txt');
+      expect(fs.realpath).toHaveBeenCalledWith('/some/symlink/to/file.txt');
+    });
 
-  it('should throw an error if realpath fails with a non-ENOENT error (e.g. EACCES)', async () => {
-    vi.spyOn(fs, 'realpath').mockImplementation(async () => {
-      throw Object.assign(new Error('EACCES: permission denied'), {
-        code: 'EACCES',
+    it('should fallback to parent directory if file does not exist (ENOENT)', async () => {
+      vi.spyOn(fs, 'realpath').mockImplementation(async (p) => {
+        if (p === '/workspace/nonexistent.txt') {
+          throw Object.assign(new Error('ENOENT: no such file or directory'), {
+            code: 'ENOENT',
+          });
+        }
+        if (p === '/workspace') {
+          return '/real/workspace';
+        }
+        throw new Error(`Unexpected path: ${p}`);
       });
+
+      const result = await tryRealpath('/workspace/nonexistent.txt');
+
+      // It should combine the real path of the parent with the original basename
+      expect(result).toBe(path.join('/real/workspace', 'nonexistent.txt'));
     });
 
-    await expect(tryRealpath('/secret/file.txt')).rejects.toThrow(
-      'EACCES: permission denied',
-    );
-  });
-});
+    it('should recursively fallback up the directory tree on multiple ENOENT errors', async () => {
+      vi.spyOn(fs, 'realpath').mockImplementation(async (p) => {
+        if (p === '/workspace/missing_dir/missing_file.txt') {
+          throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' });
+        }
+        if (p === '/workspace/missing_dir') {
+          throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' });
+        }
+        if (p === '/workspace') {
+          return '/real/workspace';
+        }
+        throw new Error(`Unexpected path: ${p}`);
+      });
 
-describe('NoopSandboxManager', () => {
-  const sandboxManager = new NoopSandboxManager();
+      const result = await tryRealpath(
+        '/workspace/missing_dir/missing_file.txt',
+      );
 
-  it('should pass through the command and arguments unchanged', async () => {
-    const req = {
-      command: 'ls',
-      args: ['-la'],
-      cwd: '/tmp',
-      env: { PATH: '/usr/bin' },
-    };
+      // It should resolve '/workspace' to '/real/workspace' and append the missing parts
+      expect(result).toBe(
+        path.join('/real/workspace', 'missing_dir', 'missing_file.txt'),
+      );
+    });
 
-    const result = await sandboxManager.prepareCommand(req);
+    it('should return the path unchanged if it reaches the root directory and it still does not exist', async () => {
+      const rootPath = path.resolve('/');
+      vi.spyOn(fs, 'realpath').mockImplementation(async () => {
+        throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' });
+      });
 
-    expect(result.program).toBe('ls');
-    expect(result.args).toEqual(['-la']);
+      const result = await tryRealpath(rootPath);
+      expect(result).toBe(rootPath);
+    });
+
+    it('should throw an error if realpath fails with a non-ENOENT error (e.g. EACCES)', async () => {
+      vi.spyOn(fs, 'realpath').mockImplementation(async () => {
+        throw Object.assign(new Error('EACCES: permission denied'), {
+          code: 'EACCES',
+        });
+      });
+
+      await expect(tryRealpath('/secret/file.txt')).rejects.toThrow(
+        'EACCES: permission denied',
+      );
+    });
   });
 
-  it('should sanitize the environment variables', async () => {
-    const req = {
-      command: 'echo',
-      args: ['hello'],
-      cwd: '/tmp',
-      env: {
-        PATH: '/usr/bin',
-        GITHUB_TOKEN: 'ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx',
-        MY_SECRET: 'super-secret',
-        SAFE_VAR: 'is-safe',
-      },
-    };
+  describe('NoopSandboxManager', () => {
+    const sandboxManager = new NoopSandboxManager();
 
-    const result = await sandboxManager.prepareCommand(req);
+    it('should pass through the command and arguments unchanged', async () => {
+      const req = {
+        command: 'ls',
+        args: ['-la'],
+        cwd: '/tmp',
+        env: { PATH: '/usr/bin' },
+      };
 
-    expect(result.env['PATH']).toBe('/usr/bin');
-    expect(result.env['SAFE_VAR']).toBe('is-safe');
-    expect(result.env['GITHUB_TOKEN']).toBeUndefined();
-    expect(result.env['MY_SECRET']).toBeUndefined();
-  });
+      const result = await sandboxManager.prepareCommand(req);
 
-  it('should NOT allow disabling environment variable redaction if requested in config (vulnerability fix)', async () => {
-    const req = {
-      command: 'echo',
-      args: ['hello'],
-      cwd: '/tmp',
-      env: {
-        API_KEY: 'sensitive-key',
-      },
-      policy: {
-        sanitizationConfig: {
-          enableEnvironmentVariableRedaction: false,
+      expect(result.program).toBe('ls');
+      expect(result.args).toEqual(['-la']);
+    });
+
+    it('should sanitize the environment variables', async () => {
+      const req = {
+        command: 'echo',
+        args: ['hello'],
+        cwd: '/tmp',
+        env: {
+          PATH: '/usr/bin',
+          GITHUB_TOKEN: 'ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx',
+          MY_SECRET: 'super-secret',
+          SAFE_VAR: 'is-safe',
         },
-      },
-    };
+      };
 
-    const result = await sandboxManager.prepareCommand(req);
+      const result = await sandboxManager.prepareCommand(req);
 
-    // API_KEY should be redacted because SandboxManager forces redaction and API_KEY matches NEVER_ALLOWED_NAME_PATTERNS
-    expect(result.env['API_KEY']).toBeUndefined();
-  });
+      expect(result.env['PATH']).toBe('/usr/bin');
+      expect(result.env['SAFE_VAR']).toBe('is-safe');
+      expect(result.env['GITHUB_TOKEN']).toBeUndefined();
+      expect(result.env['MY_SECRET']).toBeUndefined();
+    });
 
-  it('should respect allowedEnvironmentVariables in config but filter sensitive ones', async () => {
-    const req = {
-      command: 'echo',
-      args: ['hello'],
-      cwd: '/tmp',
-      env: {
-        MY_SAFE_VAR: 'safe-value',
-        MY_TOKEN: 'secret-token',
-      },
-      policy: {
-        sanitizationConfig: {
-          allowedEnvironmentVariables: ['MY_SAFE_VAR', 'MY_TOKEN'],
+    it('should NOT allow disabling environment variable redaction if requested in config (vulnerability fix)', async () => {
+      const req = {
+        command: 'echo',
+        args: ['hello'],
+        cwd: '/tmp',
+        env: {
+          API_KEY: 'sensitive-key',
         },
-      },
-    };
-
-    const result = await sandboxManager.prepareCommand(req);
-
-    expect(result.env['MY_SAFE_VAR']).toBe('safe-value');
-    // MY_TOKEN matches /TOKEN/i so it should be redacted despite being allowed in config
-    expect(result.env['MY_TOKEN']).toBeUndefined();
-  });
-
-  it('should respect blockedEnvironmentVariables in config', async () => {
-    const req = {
-      command: 'echo',
-      args: ['hello'],
-      cwd: '/tmp',
-      env: {
-        SAFE_VAR: 'safe-value',
-        BLOCKED_VAR: 'blocked-value',
-      },
-      policy: {
-        sanitizationConfig: {
-          blockedEnvironmentVariables: ['BLOCKED_VAR'],
+        policy: {
+          sanitizationConfig: {
+            enableEnvironmentVariableRedaction: false,
+          },
         },
-      },
-    };
+      };
 
-    const result = await sandboxManager.prepareCommand(req);
+      const result = await sandboxManager.prepareCommand(req);
 
-    expect(result.env['SAFE_VAR']).toBe('safe-value');
-    expect(result.env['BLOCKED_VAR']).toBeUndefined();
-  });
-});
+      // API_KEY should be redacted because SandboxManager forces redaction and API_KEY matches NEVER_ALLOWED_NAME_PATTERNS
+      expect(result.env['API_KEY']).toBeUndefined();
+    });
 
-describe('createSandboxManager', () => {
-  it('should return NoopSandboxManager if sandboxing is disabled', () => {
-    const manager = createSandboxManager({ enabled: false }, '/workspace');
-    expect(manager).toBeInstanceOf(NoopSandboxManager);
+    it('should respect allowedEnvironmentVariables in config but filter sensitive ones', async () => {
+      const req = {
+        command: 'echo',
+        args: ['hello'],
+        cwd: '/tmp',
+        env: {
+          MY_SAFE_VAR: 'safe-value',
+          MY_TOKEN: 'secret-token',
+        },
+        policy: {
+          sanitizationConfig: {
+            allowedEnvironmentVariables: ['MY_SAFE_VAR', 'MY_TOKEN'],
+          },
+        },
+      };
+
+      const result = await sandboxManager.prepareCommand(req);
+
+      expect(result.env['MY_SAFE_VAR']).toBe('safe-value');
+      // MY_TOKEN matches /TOKEN/i so it should be redacted despite being allowed in config
+      expect(result.env['MY_TOKEN']).toBeUndefined();
+    });
+
+    it('should respect blockedEnvironmentVariables in config', async () => {
+      const req = {
+        command: 'echo',
+        args: ['hello'],
+        cwd: '/tmp',
+        env: {
+          SAFE_VAR: 'safe-value',
+          BLOCKED_VAR: 'blocked-value',
+        },
+        policy: {
+          sanitizationConfig: {
+            blockedEnvironmentVariables: ['BLOCKED_VAR'],
+          },
+        },
+      };
+
+      const result = await sandboxManager.prepareCommand(req);
+
+      expect(result.env['SAFE_VAR']).toBe('safe-value');
+      expect(result.env['BLOCKED_VAR']).toBeUndefined();
+    });
+
+    it('should delegate isKnownSafeCommand to platform specific checkers', () => {
+      vi.spyOn(os, 'platform').mockReturnValue('darwin');
+      expect(sandboxManager.isKnownSafeCommand(['ls'])).toBe(true);
+      expect(sandboxManager.isKnownSafeCommand(['dir'])).toBe(false);
+
+      vi.spyOn(os, 'platform').mockReturnValue('win32');
+      expect(sandboxManager.isKnownSafeCommand(['dir'])).toBe(true);
+    });
+
+    it('should delegate isDangerousCommand to platform specific checkers', () => {
+      vi.spyOn(os, 'platform').mockReturnValue('darwin');
+      expect(sandboxManager.isDangerousCommand(['rm', '-rf', '.'])).toBe(true);
+      expect(sandboxManager.isDangerousCommand(['del'])).toBe(false);
+
+      vi.spyOn(os, 'platform').mockReturnValue('win32');
+      expect(sandboxManager.isDangerousCommand(['del'])).toBe(true);
+    });
   });
 
-  it.each([
-    { platform: 'linux', expected: LinuxSandboxManager },
-    { platform: 'darwin', expected: MacOsSandboxManager },
-    { platform: 'win32', expected: WindowsSandboxManager },
-  ] as const)(
-    'should return $expected.name if sandboxing is enabled and platform is $platform',
-    ({ platform, expected }) => {
-      const osSpy = vi.spyOn(os, 'platform').mockReturnValue(platform);
-      try {
+  describe('createSandboxManager', () => {
+    it('should return NoopSandboxManager if sandboxing is disabled', () => {
+      const manager = createSandboxManager({ enabled: false }, '/workspace');
+      expect(manager).toBeInstanceOf(NoopSandboxManager);
+    });
+
+    it.each([
+      { platform: 'linux', expected: LinuxSandboxManager },
+      { platform: 'darwin', expected: MacOsSandboxManager },
+    ] as const)(
+      'should return $expected.name if sandboxing is enabled and platform is $platform',
+      ({ platform, expected }) => {
+        vi.spyOn(os, 'platform').mockReturnValue(platform);
         const manager = createSandboxManager({ enabled: true }, '/workspace');
         expect(manager).toBeInstanceOf(expected);
-      } finally {
-        osSpy.mockRestore();
-      }
-    },
-  );
+      },
+    );
+
+    it("should return WindowsSandboxManager if sandboxing is enabled with 'windows-native' command on win32", () => {
+      vi.spyOn(os, 'platform').mockReturnValue('win32');
+      const manager = createSandboxManager(
+        { enabled: true, command: 'windows-native' },
+        '/workspace',
+      );
+      expect(manager).toBeInstanceOf(WindowsSandboxManager);
+    });
+
+    it('should return LocalSandboxManager on win32 if command is not windows-native', () => {
+      vi.spyOn(os, 'platform').mockReturnValue('win32');
+      const manager = createSandboxManager(
+        { enabled: true, command: 'docker' as unknown as 'windows-native' },
+        '/workspace',
+      );
+      expect(manager).toBeInstanceOf(LocalSandboxManager);
+    });
+  });
 });

packages/core/src/services/sandboxManager.ts

@@ -7,6 +7,14 @@
 import fs from 'node:fs/promises';
 import os from 'node:os';
 import path from 'node:path';
+import {
+  isKnownSafeCommand as isMacSafeCommand,
+  isDangerousCommand as isMacDangerousCommand,
+} from '../sandbox/macos/commandSafety.js';
+import {
+  isKnownSafeCommand as isWindowsSafeCommand,
+  isDangerousCommand as isWindowsDangerousCommand,
+} from '../sandbox/windows/commandSafety.js';
 import { isNodeError } from '../utils/errors.js';
 import {
   sanitizeEnvironment,
@@ -90,6 +98,16 @@ export interface SandboxManager {
    * Prepares a command to run in a sandbox, including environment sanitization.
    */
   prepareCommand(req: SandboxRequest): Promise<SandboxedCommand>;
+
+  /**
+   * Checks if a command with its arguments is known to be safe for this sandbox.
+   */
+  isKnownSafeCommand(args: string[]): boolean;
+
+  /**
+   * Checks if a command with its arguments is explicitly known to be dangerous for this sandbox.
+   */
+  isDangerousCommand(args: string[]): boolean;
 }
 
 /**
@@ -124,6 +142,18 @@ export class NoopSandboxManager implements SandboxManager {
       env: sanitizedEnv,
     };
   }
+
+  isKnownSafeCommand(args: string[]): boolean {
+    return os.platform() === 'win32'
+      ? isWindowsSafeCommand(args)
+      : isMacSafeCommand(args);
+  }
+
+  isDangerousCommand(args: string[]): boolean {
+    return os.platform() === 'win32'
+      ? isWindowsDangerousCommand(args)
+      : isMacDangerousCommand(args);
+  }
 }
 
 /**
@@ -133,6 +163,14 @@ export class LocalSandboxManager implements SandboxManager {
   async prepareCommand(_req: SandboxRequest): Promise<SandboxedCommand> {
     throw new Error('Tool sandboxing is not yet implemented.');
   }
+
+  isKnownSafeCommand(_args: string[]): boolean {
+    return false;
+  }
+
+  isDangerousCommand(_args: string[]): boolean {
+    return false;
+  }
 }
 
 /**

packages/core/src/services/sandboxManagerFactory.ts

@@ -29,24 +29,21 @@ export function createSandboxManager(
     return new NoopSandboxManager();
   }
 
-  const isWindows = os.platform() === 'win32';
-
-  if (
-    isWindows &&
-    (sandbox?.enabled || sandbox?.command === 'windows-native')
-  ) {
-    return new WindowsSandboxManager({ workspace });
-  }
+  const modeConfig =
+    policyManager && approvalMode
+      ? policyManager.getModeConfig(approvalMode)
+      : undefined;
 
   if (sandbox?.enabled) {
-    if (os.platform() === 'linux') {
+    if (os.platform() === 'win32' && sandbox?.command === 'windows-native') {
+      return new WindowsSandboxManager({
+        workspace,
+        modeConfig,
+        policyManager,
+      });
+    } else if (os.platform() === 'linux') {
       return new LinuxSandboxManager({ workspace });
-    }
-    if (os.platform() === 'darwin') {
-      const modeConfig =
-        policyManager && approvalMode
-          ? policyManager.getModeConfig(approvalMode)
-          : undefined;
+    } else if (os.platform() === 'darwin') {
       return new MacOsSandboxManager({
         workspace,
         modeConfig,

packages/core/src/services/sandboxedFileSystemService.test.ts

@@ -35,6 +35,14 @@ class MockSandboxManager implements SandboxManager {
       env: req.env || {},
     };
   }
+
+  isKnownSafeCommand(): boolean {
+    return false;
+  }
+
+  isDangerousCommand(): boolean {
+    return false;
+  }
 }
 
 describe('SandboxedFileSystemService', () => {

packages/core/src/services/shellExecutionService.test.ts

@@ -1918,6 +1918,8 @@ describe('ShellExecutionService environment variables', () => {
         args: ['-c', 'ls'],
         env: { SANDBOXED: 'true' },
       }),
+      isKnownSafeCommand: vi.fn().mockReturnValue(false),
+      isDangerousCommand: vi.fn().mockReturnValue(false),
     };
 
     const configWithSandbox: ShellExecutionConfig = {