feat(sandbox): implement forbiddenPaths for OS-specific sandbox managers (#23282)

Co-authored-by: Gal Zahavi <38544478+galz10@users.noreply.github.com>

packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts

@@ -4,8 +4,9 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { LinuxSandboxManager } from './LinuxSandboxManager.js';
+import * as sandboxManager from '../../services/sandboxManager.js';
 import type { SandboxRequest } from '../../services/sandboxManager.js';
 import fs from 'node:fs';
 
@@ -43,6 +44,10 @@ describe('LinuxSandboxManager', () => {
     manager = new LinuxSandboxManager({ workspace });
   });
 
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
   const getBwrapArgs = async (req: SandboxRequest) => {
     const result = await manager.prepareCommand(req);
     expect(result.program).toBe('sh');
@@ -55,6 +60,41 @@ describe('LinuxSandboxManager', () => {
     return result.args.slice(4);
   };
 
+  /**
+   * Helper to verify only the dynamic, policy-based binds (e.g. allowedPaths, forbiddenPaths).
+   * It asserts that the base workspace and governance files are present exactly once,
+   * then strips them away, leaving only the dynamic binds for a focused, non-brittle assertion.
+   */
+  const expectDynamicBinds = (
+    bwrapArgs: string[],
+    expectedDynamicBinds: string[],
+  ) => {
+    const bindsIndex = bwrapArgs.indexOf('--seccomp');
+    const allBinds = bwrapArgs.slice(bwrapArgs.indexOf('--bind'), bindsIndex);
+
+    const baseBinds = [
+      '--bind',
+      workspace,
+      workspace,
+      '--ro-bind',
+      `${workspace}/.gitignore`,
+      `${workspace}/.gitignore`,
+      '--ro-bind',
+      `${workspace}/.geminiignore`,
+      `${workspace}/.geminiignore`,
+      '--ro-bind',
+      `${workspace}/.git`,
+      `${workspace}/.git`,
+    ];
+
+    // Verify the base binds are present exactly at the beginning
+    expect(allBinds.slice(0, baseBinds.length)).toEqual(baseBinds);
+
+    // Extract the remaining dynamic binds
+    const dynamicBinds = allBinds.slice(baseBinds.length);
+    expect(dynamicBinds).toEqual(expectedDynamicBinds);
+  };
+
   it('correctly outputs bwrap as the program with appropriate isolation flags', async () => {
     const bwrapArgs = await getBwrapArgs({
       command: 'ls',
@@ -108,22 +148,7 @@ describe('LinuxSandboxManager', () => {
     });
 
     // Verify the specific bindings were added correctly
-    const bindsIndex = bwrapArgs.indexOf('--seccomp');
-    const binds = bwrapArgs.slice(bwrapArgs.indexOf('--bind'), bindsIndex);
-
-    expect(binds).toEqual([
-      '--bind',
-      workspace,
-      workspace,
-      '--ro-bind',
-      `${workspace}/.gitignore`,
-      `${workspace}/.gitignore`,
-      '--ro-bind',
-      `${workspace}/.geminiignore`,
-      `${workspace}/.geminiignore`,
-      '--ro-bind',
-      `${workspace}/.git`,
-      `${workspace}/.git`,
+    expectDynamicBinds(bwrapArgs, [
       '--bind-try',
       '/tmp/cache',
       '/tmp/cache',
@@ -186,23 +211,156 @@ describe('LinuxSandboxManager', () => {
       },
     });
 
-    const bindsIndex = bwrapArgs.indexOf('--seccomp');
-    const binds = bwrapArgs.slice(bwrapArgs.indexOf('--bind'), bindsIndex);
-
     // Should only contain the primary workspace bind and governance files, not the second workspace bind with a trailing slash
-    expect(binds).toEqual([
-      '--bind',
-      workspace,
-      workspace,
-      '--ro-bind',
-      `${workspace}/.gitignore`,
-      `${workspace}/.gitignore`,
-      '--ro-bind',
-      `${workspace}/.geminiignore`,
-      `${workspace}/.geminiignore`,
-      '--ro-bind',
-      `${workspace}/.git`,
-      `${workspace}/.git`,
+    expectDynamicBinds(bwrapArgs, []);
+  });
+
+  it('maps forbiddenPaths to empty mounts', async () => {
+    vi.spyOn(fs.promises, 'stat').mockImplementation(async (p) => {
+      // Mock /tmp/cache as a directory, and /opt/secret.txt as a file
+      if (p.toString().includes('cache')) {
+        return { isDirectory: () => true } as fs.Stats;
+      }
+      return { isDirectory: () => false } as fs.Stats;
+    });
+    vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) =>
+      p.toString(),
+    );
+
+    const bwrapArgs = await getBwrapArgs({
+      command: 'ls',
+      args: ['-la'],
+      cwd: workspace,
+      env: {},
+      policy: {
+        forbiddenPaths: ['/tmp/cache', '/opt/secret.txt'],
+      },
+    });
+
+    expectDynamicBinds(bwrapArgs, [
+      '--tmpfs',
+      '/tmp/cache',
+      '--remount-ro',
+      '/tmp/cache',
+      '--ro-bind-try',
+      '/dev/null',
+      '/opt/secret.txt',
+    ]);
+  });
+
+  it('overrides allowedPaths if a path is also in forbiddenPaths', async () => {
+    vi.spyOn(fs.promises, 'stat').mockImplementation(
+      async () => ({ isDirectory: () => true }) as fs.Stats,
+    );
+    vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) =>
+      p.toString(),
+    );
+
+    const bwrapArgs = await getBwrapArgs({
+      command: 'ls',
+      args: ['-la'],
+      cwd: workspace,
+      env: {},
+      policy: {
+        allowedPaths: ['/tmp/conflict'],
+        forbiddenPaths: ['/tmp/conflict'],
+      },
+    });
+
+    expectDynamicBinds(bwrapArgs, [
+      '--bind-try',
+      '/tmp/conflict',
+      '/tmp/conflict',
+      '--tmpfs',
+      '/tmp/conflict',
+      '--remount-ro',
+      '/tmp/conflict',
+    ]);
+  });
+
+  it('protects both the resolved path and the original path for forbidden symlinks', async () => {
+    vi.spyOn(fs.promises, 'stat').mockImplementation(
+      async () => ({ isDirectory: () => false }) as fs.Stats,
+    );
+    vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) => {
+      if (p === '/tmp/forbidden-symlink') return '/opt/real-target.txt';
+      return p.toString();
+    });
+
+    const bwrapArgs = await getBwrapArgs({
+      command: 'ls',
+      args: ['-la'],
+      cwd: workspace,
+      env: {},
+      policy: {
+        forbiddenPaths: ['/tmp/forbidden-symlink'],
+      },
+    });
+
+    // Should explicitly mask both the resolved path and the original symlink path
+    expectDynamicBinds(bwrapArgs, [
+      '--ro-bind-try',
+      '/dev/null',
+      '/opt/real-target.txt',
+      '--ro-bind-try',
+      '/dev/null',
+      '/tmp/forbidden-symlink',
+    ]);
+  });
+
+  it('masks non-existent forbidden paths with a broken symlink', async () => {
+    const error = new Error('File not found') as NodeJS.ErrnoException;
+    error.code = 'ENOENT';
+    vi.spyOn(fs.promises, 'stat').mockRejectedValue(error);
+    vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) =>
+      p.toString(),
+    );
+
+    const bwrapArgs = await getBwrapArgs({
+      command: 'ls',
+      args: [],
+      cwd: workspace,
+      env: {},
+      policy: {
+        forbiddenPaths: ['/tmp/not-here.txt'],
+      },
+    });
+
+    expectDynamicBinds(bwrapArgs, [
+      '--symlink',
+      '/.forbidden',
+      '/tmp/not-here.txt',
+    ]);
+  });
+
+  it('masks directory symlinks with tmpfs for both paths', async () => {
+    vi.spyOn(fs.promises, 'stat').mockImplementation(
+      async () => ({ isDirectory: () => true }) as fs.Stats,
+    );
+    vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) => {
+      if (p === '/tmp/dir-link') return '/opt/real-dir';
+      return p.toString();
+    });
+
+    const bwrapArgs = await getBwrapArgs({
+      command: 'ls',
+      args: [],
+      cwd: workspace,
+      env: {},
+      policy: {
+        forbiddenPaths: ['/tmp/dir-link'],
+      },
+    });
+
+    expectDynamicBinds(bwrapArgs, [
+      '--tmpfs',
+      '/opt/real-dir',
+      '--remount-ro',
+      '/opt/real-dir',
+      '--tmpfs',
+      '/tmp/dir-link',
+      '--remount-ro',
+      '/tmp/dir-link',
     ]);
   });
 });

packages/core/src/sandbox/linux/LinuxSandboxManager.ts

@@ -14,11 +14,13 @@ import {
   type SandboxedCommand,
   GOVERNANCE_FILES,
   sanitizePaths,
+  tryRealpath,
 } from '../../services/sandboxManager.js';
 import {
   sanitizeEnvironment,
   getSecureSanitizationConfig,
 } from '../../services/environmentSanitization.js';
+import { isNodeError } from '../../utils/errors.js';
 
 let cachedBpfPath: string | undefined;
 
@@ -111,7 +113,15 @@ export class LinuxSandboxManager implements SandboxManager {
     const sanitizedEnv = sanitizeEnvironment(req.env, sanitizationConfig);
 
     const bwrapArgs: string[] = [
-      '--unshare-all',
+      ...(req.policy?.networkAccess
+        ? [
+            '--unshare-user',
+            '--unshare-ipc',
+            '--unshare-pid',
+            '--unshare-uts',
+            '--unshare-cgroup',
+          ]
+        : ['--unshare-all']),
       '--new-session', // Isolate session
       '--die-with-parent', // Prevent orphaned runaway processes
       '--ro-bind',
@@ -145,18 +155,35 @@ export class LinuxSandboxManager implements SandboxManager {
     }
 
     const allowedPaths = sanitizePaths(req.policy?.allowedPaths) || [];
-    const normalizedWorkspace = normalize(this.options.workspace).replace(
-      /\/$/,
-      '',
-    );
-    for (const allowedPath of allowedPaths) {
-      const normalizedAllowedPath = normalize(allowedPath).replace(/\/$/, '');
-      if (normalizedAllowedPath !== normalizedWorkspace) {
-        bwrapArgs.push('--bind-try', allowedPath, allowedPath);
+    const normalizedWorkspace = this.normalizePath(this.options.workspace);
+    for (const p of allowedPaths) {
+      if (this.normalizePath(p) !== normalizedWorkspace) {
+        bwrapArgs.push('--bind-try', p, p);
       }
     }
 
-    // TODO: handle forbidden paths
+    const forbiddenPaths = sanitizePaths(req.policy?.forbiddenPaths) || [];
+    for (const p of forbiddenPaths) {
+      try {
+        const originalPath = this.normalizePath(p);
+        const resolvedPath = await tryRealpath(originalPath);
+
+        // Mask the resolved path to prevent access to the underlying file.
+        await this.applyMasking(bwrapArgs, resolvedPath);
+
+        // If the original path was a symlink, mask it as well to prevent access
+        // through the link itself.
+        if (resolvedPath !== originalPath) {
+          await this.applyMasking(bwrapArgs, originalPath);
+        }
+      } catch (e) {
+        throw new Error(
+          `Failed to deny access to forbidden path: ${p}. ${
+            e instanceof Error ? e.message : String(e)
+          }`,
+        );
+      }
+    }
 
     const bpfPath = getSeccompBpfPath();
 
@@ -177,4 +204,33 @@ export class LinuxSandboxManager implements SandboxManager {
       env: sanitizedEnv,
     };
   }
+
+  /**
+   * Applies bubblewrap arguments to mask a forbidden path.
+   */
+  private async applyMasking(args: string[], path: string) {
+    try {
+      const stats = await fs.promises.stat(path);
+
+      if (stats.isDirectory()) {
+        // Directories are masked by mounting an empty, read-only tmpfs.
+        args.push('--tmpfs', path, '--remount-ro', path);
+      } else {
+        // Existing files are masked by binding them to /dev/null.
+        args.push('--ro-bind-try', '/dev/null', path);
+      }
+    } catch (e) {
+      if (isNodeError(e) && e.code === 'ENOENT') {
+        // Non-existent paths are masked by a broken symlink. This prevents
+        // creation within the sandbox while avoiding host remnants.
+        args.push('--symlink', '/.forbidden', path);
+        return;
+      }
+      throw e;
+    }
+  }
+
+  private normalizePath(p: string): string {
+    return normalize(p).replace(/\/$/, '');
+  }
 }