feat(core): refactor SandboxManager to a stateless architecture and introduce explicit Deny interface (#23141)

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

@@ -6,12 +6,30 @@
 
 import os from 'node:os';
 import { describe, expect, it, vi } from 'vitest';
-import { NoopSandboxManager } from './sandboxManager.js';
+import { NoopSandboxManager, sanitizePaths } from './sandboxManager.js';
 import { createSandboxManager } from './sandboxManagerFactory.js';
 import { LinuxSandboxManager } from '../sandbox/linux/LinuxSandboxManager.js';
 import { MacOsSandboxManager } from '../sandbox/macos/MacOsSandboxManager.js';
 import { WindowsSandboxManager } from './windowsSandboxManager.js';
 
+describe('sanitizePaths', () => {
+  it('should return undefined if no paths are provided', () => {
+    expect(sanitizePaths(undefined)).toBeUndefined();
+  });
+
+  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('NoopSandboxManager', () => {
   const sandboxManager = new NoopSandboxManager();
 
@@ -58,7 +76,7 @@ describe('NoopSandboxManager', () => {
       env: {
         API_KEY: 'sensitive-key',
       },
-      config: {
+      policy: {
         sanitizationConfig: {
           enableEnvironmentVariableRedaction: false,
         },
@@ -80,7 +98,7 @@ describe('NoopSandboxManager', () => {
         MY_SAFE_VAR: 'safe-value',
         MY_TOKEN: 'secret-token',
       },
-      config: {
+      policy: {
         sanitizationConfig: {
           allowedEnvironmentVariables: ['MY_SAFE_VAR', 'MY_TOKEN'],
         },
@@ -103,7 +121,7 @@ describe('NoopSandboxManager', () => {
         SAFE_VAR: 'safe-value',
         BLOCKED_VAR: 'blocked-value',
       },
-      config: {
+      policy: {
         sanitizationConfig: {
           blockedEnvironmentVariables: ['BLOCKED_VAR'],
         },

packages/core/src/services/sandboxManager.ts

@@ -4,11 +4,37 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import os from 'node:os';
+import path from 'node:path';
 import {
   sanitizeEnvironment,
   getSecureSanitizationConfig,
   type EnvironmentSanitizationConfig,
 } from './environmentSanitization.js';
+/**
+ * Security boundaries and permissions applied to a specific sandboxed execution.
+ */
+export interface ExecutionPolicy {
+  /** Additional absolute paths to grant full read/write access to. */
+  allowedPaths?: string[];
+  /** Absolute paths to explicitly deny read/write access to (overrides allowlists). */
+  forbiddenPaths?: string[];
+  /** Whether network access is allowed. */
+  networkAccess?: boolean;
+  /** Rules for scrubbing sensitive environment variables. */
+  sanitizationConfig?: Partial<EnvironmentSanitizationConfig>;
+}
+
+/**
+ * Global configuration options used to initialize a SandboxManager.
+ */
+export interface GlobalSandboxOptions {
+  /**
+   * The primary workspace path the sandbox is anchored to.
+   * This directory is granted full read and write access.
+   */
+  workspace: string;
+}
 
 /**
  * Request for preparing a command to run in a sandbox.
@@ -22,12 +48,8 @@ export interface SandboxRequest {
   cwd: string;
   /** Environment variables to be passed to the program. */
   env: NodeJS.ProcessEnv;
-  /** Optional sandbox-specific configuration. */
-  config?: {
-    sanitizationConfig?: Partial<EnvironmentSanitizationConfig>;
-    allowedPaths?: string[];
-    networkAccess?: boolean;
-  };
+  /** Policy to use for this request. */
+  policy?: ExecutionPolicy;
 }
 
 /**
@@ -65,7 +87,7 @@ export class NoopSandboxManager implements SandboxManager {
    */
   async prepareCommand(req: SandboxRequest): Promise<SandboxedCommand> {
     const sanitizationConfig = getSecureSanitizationConfig(
-      req.config?.sanitizationConfig,
+      req.policy?.sanitizationConfig,
     );
 
     const sanitizedEnv = sanitizeEnvironment(req.env, sanitizationConfig);
@@ -87,4 +109,35 @@ export class LocalSandboxManager implements SandboxManager {
   }
 }
 
+/**
+ * Sanitizes an array of paths by deduplicating them and ensuring they are absolute.
+ */
+export function sanitizePaths(paths?: string[]): string[] | undefined {
+  if (!paths) return undefined;
+
+  // We use a Map to deduplicate paths based on their normalized,
+  // platform-specific identity e.g. handling case-insensitivity on Windows)
+  // while preserving the original string casing.
+  const uniquePathsMap = new Map<string, string>();
+  for (const p of paths) {
+    if (!path.isAbsolute(p)) {
+      throw new Error(`Sandbox path must be absolute: ${p}`);
+    }
+
+    // Normalize the path (resolves slashes and redundant components)
+    let key = path.normalize(p);
+
+    // Windows file systems are case-insensitive, so we lowercase the key for
+    // deduplication
+    if (os.platform() === 'win32') {
+      key = key.toLowerCase();
+    }
+
+    if (!uniquePathsMap.has(key)) {
+      uniquePathsMap.set(key, p);
+    }
+  }
+
+  return Array.from(uniquePathsMap.values());
+}
 export { createSandboxManager } from './sandboxManagerFactory.js';

packages/core/src/services/sandboxManagerFactory.ts

@@ -28,7 +28,7 @@ export function createSandboxManager(
     isWindows &&
     (sandbox?.enabled || sandbox?.command === 'windows-native')
   ) {
-    return new WindowsSandboxManager();
+    return new WindowsSandboxManager({ workspace });
   }
 
   if (sandbox?.enabled) {

packages/core/src/services/shellExecutionService.ts

@@ -437,7 +437,7 @@ export class ShellExecutionService {
       args: spawnArgs,
       env: baseEnv,
       cwd,
-      config: {
+      policy: {
         ...shellExecutionConfig,
         ...(shellExecutionConfig.sandboxConfig || {}),
         sanitizationConfig,

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

@@ -4,12 +4,28 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { describe, it, expect } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import os from 'node:os';
+import path from 'node:path';
 import { WindowsSandboxManager } from './windowsSandboxManager.js';
 import type { SandboxRequest } from './sandboxManager.js';
+import { spawnAsync } from '../utils/shell-utils.js';
+
+vi.mock('../utils/shell-utils.js', () => ({
+  spawnAsync: vi.fn(),
+}));
 
 describe('WindowsSandboxManager', () => {
-  const manager = new WindowsSandboxManager('win32');
+  let manager: WindowsSandboxManager;
+
+  beforeEach(() => {
+    vi.spyOn(os, 'platform').mockReturnValue('win32');
+    manager = new WindowsSandboxManager({ workspace: '/test/workspace' });
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
 
   it('should prepare a GeminiSandbox.exe command', async () => {
     const req: SandboxRequest = {
@@ -17,7 +33,7 @@ describe('WindowsSandboxManager', () => {
       args: ['/groups'],
       cwd: '/test/cwd',
       env: { TEST_VAR: 'test_value' },
-      config: {
+      policy: {
         networkAccess: false,
       },
     };
@@ -34,7 +50,7 @@ describe('WindowsSandboxManager', () => {
       args: [],
       cwd: '/test/cwd',
       env: {},
-      config: {
+      policy: {
         networkAccess: true,
       },
     };
@@ -52,7 +68,7 @@ describe('WindowsSandboxManager', () => {
         API_KEY: 'secret',
         PATH: '/usr/bin',
       },
-      config: {
+      policy: {
         sanitizationConfig: {
           allowedEnvironmentVariables: ['PATH'],
           blockedEnvironmentVariables: ['API_KEY'],
@@ -65,4 +81,30 @@ describe('WindowsSandboxManager', () => {
     expect(result.env['PATH']).toBe('/usr/bin');
     expect(result.env['API_KEY']).toBeUndefined();
   });
+
+  it('should grant Low Integrity access to the workspace and allowed paths', async () => {
+    const req: SandboxRequest = {
+      command: 'test',
+      args: [],
+      cwd: '/test/cwd',
+      env: {},
+      policy: {
+        allowedPaths: ['/test/allowed1'],
+      },
+    };
+
+    await manager.prepareCommand(req);
+
+    expect(spawnAsync).toHaveBeenCalledWith('icacls', [
+      path.resolve('/test/workspace'),
+      '/setintegritylevel',
+      'Low',
+    ]);
+
+    expect(spawnAsync).toHaveBeenCalledWith('icacls', [
+      path.resolve('/test/allowed1'),
+      '/setintegritylevel',
+      'Low',
+    ]);
+  });
 });

packages/core/src/services/windowsSandboxManager.ts

@@ -6,15 +6,18 @@
 
 import fs from 'node:fs';
 import path from 'node:path';
+import os from 'node:os';
 import { fileURLToPath } from 'node:url';
-import type {
-  SandboxManager,
-  SandboxRequest,
-  SandboxedCommand,
+import {
+  type SandboxManager,
+  type SandboxRequest,
+  type SandboxedCommand,
+  type GlobalSandboxOptions,
+  sanitizePaths,
 } from './sandboxManager.js';
 import {
   sanitizeEnvironment,
-  type EnvironmentSanitizationConfig,
+  getSecureSanitizationConfig,
 } from './environmentSanitization.js';
 import { debugLogger } from '../utils/debugLogger.js';
 import { spawnAsync } from '../utils/shell-utils.js';
@@ -29,18 +32,16 @@ const __dirname = path.dirname(__filename);
  */
 export class WindowsSandboxManager implements SandboxManager {
   private readonly helperPath: string;
-  private readonly platform: string;
   private initialized = false;
   private readonly lowIntegrityCache = new Set<string>();
 
-  constructor(platform: string = process.platform) {
-    this.platform = platform;
+  constructor(private readonly options: GlobalSandboxOptions) {
     this.helperPath = path.resolve(__dirname, 'scripts', 'GeminiSandbox.exe');
   }
 
   private async ensureInitialized(): Promise<void> {
     if (this.initialized) return;
-    if (this.platform !== 'win32') {
+    if (os.platform() !== 'win32') {
       this.initialized = true;
       return;
     }
@@ -145,36 +146,31 @@ export class WindowsSandboxManager implements SandboxManager {
   async prepareCommand(req: SandboxRequest): Promise<SandboxedCommand> {
     await this.ensureInitialized();
 
-    const sanitizationConfig: EnvironmentSanitizationConfig = {
-      allowedEnvironmentVariables:
-        req.config?.sanitizationConfig?.allowedEnvironmentVariables ?? [],
-      blockedEnvironmentVariables:
-        req.config?.sanitizationConfig?.blockedEnvironmentVariables ?? [],
-      enableEnvironmentVariableRedaction:
-        req.config?.sanitizationConfig?.enableEnvironmentVariableRedaction ??
-        true,
-    };
+    const sanitizationConfig = getSecureSanitizationConfig(
+      req.policy?.sanitizationConfig,
+    );
 
     const sanitizedEnv = sanitizeEnvironment(req.env, sanitizationConfig);
 
     // 1. Handle filesystem permissions for Low Integrity
-    // Grant "Low Mandatory Level" write access to the CWD.
-    await this.grantLowIntegrityAccess(req.cwd);
+    // Grant "Low Mandatory Level" write access to the workspace.
+    await this.grantLowIntegrityAccess(this.options.workspace);
 
     // Grant "Low Mandatory Level" read access to allowedPaths.
-    if (req.config?.allowedPaths) {
-      for (const allowedPath of req.config.allowedPaths) {
-        await this.grantLowIntegrityAccess(allowedPath);
-      }
+    const allowedPaths = sanitizePaths(req.policy?.allowedPaths) || [];
+    for (const allowedPath of allowedPaths) {
+      await this.grantLowIntegrityAccess(allowedPath);
     }
 
+    // TODO: handle forbidden paths
+
     // 2. Construct the helper command
     // GeminiSandbox.exe <network:0|1> <cwd> <command> [args...]
     const program = this.helperPath;
 
     // If the command starts with __, it's an internal command for the sandbox helper itself.
     const args = [
-      req.config?.networkAccess ? '1' : '0',
+      req.policy?.networkAccess ? '1' : '0',
       req.cwd,
       req.command,
       ...req.args,
@@ -191,7 +187,7 @@ export class WindowsSandboxManager implements SandboxManager {
    * Grants "Low Mandatory Level" access to a path using icacls.
    */
   private async grantLowIntegrityAccess(targetPath: string): Promise<void> {
-    if (this.platform !== 'win32') {
+    if (os.platform() !== 'win32') {
       return;
     }