From 30e0ab102a22dd8a93c06fda320be147a120b00d Mon Sep 17 00:00:00 2001 From: Gal Zahavi <38544478+galz10@users.noreply.github.com> Date: Wed, 25 Mar 2026 18:58:45 -0700 Subject: [PATCH] feat(sandbox): dynamic Linux sandbox expansion and worktree support (#23692) Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- .../sandbox/linux/LinuxSandboxManager.test.ts | 274 +++++++------- .../src/sandbox/linux/LinuxSandboxManager.ts | 342 ++++++++++-------- .../sandbox/macos/MacOsSandboxManager.test.ts | 2 +- .../src/sandbox/macos/MacOsSandboxManager.ts | 17 +- .../sandbox/macos/seatbeltArgsBuilder.test.ts | 123 +++---- .../src/sandbox/macos/seatbeltArgsBuilder.ts | 64 ++-- .../sandbox/{macos => utils}/commandSafety.ts | 0 .../core/src/sandbox/utils/commandUtils.ts | 82 +++++ packages/core/src/sandbox/utils/fsUtils.ts | 92 +++++ .../windows/WindowsSandboxManager.test.ts | 2 +- .../sandbox/windows/WindowsSandboxManager.ts | 13 +- packages/core/src/services/sandboxManager.ts | 2 +- .../src/services/sandboxManagerFactory.ts | 6 +- 13 files changed, 604 insertions(+), 415 deletions(-) rename packages/core/src/sandbox/{macos => utils}/commandSafety.ts (100%) create mode 100644 packages/core/src/sandbox/utils/commandUtils.ts create mode 100644 packages/core/src/sandbox/utils/fsUtils.ts diff --git a/packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts b/packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts index 5bde6a44da..b58fe271f6 100644 --- a/packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts +++ b/packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts @@ -6,7 +6,6 @@ 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'; @@ -18,14 +17,16 @@ vi.mock('node:fs', async () => { // @ts-expect-error - Property 'default' does not exist on type 'typeof import("node:fs")' ...actual.default, existsSync: vi.fn(() => true), - realpathSync: vi.fn((p: string | Buffer) => p.toString()), + realpathSync: vi.fn((p) => p.toString()), + statSync: vi.fn(() => ({ isDirectory: () => true }) as fs.Stats), mkdirSync: vi.fn(), openSync: vi.fn(), closeSync: vi.fn(), writeFileSync: vi.fn(), }, existsSync: vi.fn(() => true), - realpathSync: vi.fn((p: string | Buffer) => p.toString()), + realpathSync: vi.fn((p) => p.toString()), + statSync: vi.fn(() => ({ isDirectory: () => true }) as fs.Stats), mkdirSync: vi.fn(), openSync: vi.fn(), closeSync: vi.fn(), @@ -48,8 +49,12 @@ describe('LinuxSandboxManager', () => { vi.restoreAllMocks(); }); - const getBwrapArgs = async (req: SandboxRequest) => { - const result = await manager.prepareCommand(req); + const getBwrapArgs = async ( + req: SandboxRequest, + customManager?: LinuxSandboxManager, + ) => { + const mgr = customManager || manager; + const result = await mgr.prepareCommand(req); expect(result.program).toBe('sh'); expect(result.args[0]).toBe('-c'); expect(result.args[1]).toBe( @@ -60,41 +65,6 @@ 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); - }; - describe('prepareCommand', () => { it('should correctly format the base command and args', async () => { const bwrapArgs = await getBwrapArgs({ @@ -117,7 +87,7 @@ describe('LinuxSandboxManager', () => { '/proc', '--tmpfs', '/tmp', - '--bind', + '--ro-bind-try', workspace, workspace, '--ro-bind', @@ -137,6 +107,73 @@ describe('LinuxSandboxManager', () => { ]); }); + it('binds workspace read-write when readonly is false', async () => { + const customManager = new LinuxSandboxManager({ + workspace, + modeConfig: { readonly: false }, + }); + const bwrapArgs = await getBwrapArgs( + { + command: 'ls', + args: [], + cwd: workspace, + env: {}, + }, + customManager, + ); + + expect(bwrapArgs).toContain('--bind-try'); + expect(bwrapArgs).toContain(workspace); + }); + + it('maps network permissions to --share-net', async () => { + const bwrapArgs = await getBwrapArgs({ + command: 'curl', + args: [], + cwd: workspace, + env: {}, + policy: { additionalPermissions: { network: true } }, + }); + + expect(bwrapArgs).toContain('--share-net'); + }); + + it('maps explicit write permissions to --bind-try', async () => { + const bwrapArgs = await getBwrapArgs({ + command: 'touch', + args: [], + cwd: workspace, + env: {}, + policy: { + additionalPermissions: { + fileSystem: { write: ['/home/user/workspace/out/dir'] }, + }, + }, + }); + + const index = bwrapArgs.indexOf('--bind-try'); + expect(index).not.toBe(-1); + expect(bwrapArgs[index + 1]).toBe('/home/user/workspace/out/dir'); + }); + + it('rejects overrides in plan mode', async () => { + const customManager = new LinuxSandboxManager({ + workspace, + modeConfig: { allowOverrides: false }, + }); + await expect( + customManager.prepareCommand({ + command: 'ls', + args: [], + cwd: workspace, + env: {}, + policy: { additionalPermissions: { network: true } }, + }), + ).rejects.toThrow( + /Cannot override readonly\/network\/filesystem restrictions in Plan mode/, + ); + }); + it('should correctly pass through the cwd to the resulting command', async () => { const req: SandboxRequest = { command: 'ls', @@ -184,12 +221,7 @@ describe('LinuxSandboxManager', () => { }, }); - expect(bwrapArgs).toContain('--unshare-user'); - expect(bwrapArgs).toContain('--unshare-ipc'); - expect(bwrapArgs).toContain('--unshare-pid'); - expect(bwrapArgs).toContain('--unshare-uts'); - expect(bwrapArgs).toContain('--unshare-cgroup'); - expect(bwrapArgs).not.toContain('--unshare-all'); + expect(bwrapArgs).toContain('--share-net'); }); describe('governance files', () => { @@ -252,15 +284,32 @@ describe('LinuxSandboxManager', () => { }, }); - // Verify the specific bindings were added correctly - expectDynamicBinds(bwrapArgs, [ + expect(bwrapArgs).toContain('--bind-try'); + expect(bwrapArgs[bwrapArgs.indexOf('/tmp/cache') - 1]).toBe( '--bind-try', - '/tmp/cache', - '/tmp/cache', + ); + expect(bwrapArgs[bwrapArgs.indexOf('/opt/tools') - 1]).toBe( '--bind-try', - '/opt/tools', - '/opt/tools', - ]); + ); + }); + + it('should not grant read-write access to allowedPaths inside the workspace when readonly mode is active', async () => { + const manager = new LinuxSandboxManager({ + workspace, + modeConfig: { readonly: true }, + }); + const result = await manager.prepareCommand({ + command: 'ls', + args: [], + cwd: workspace, + env: {}, + policy: { + allowedPaths: [workspace + '/subdirectory'], + }, + }); + const bwrapArgs = result.args; + const bindIndex = bwrapArgs.indexOf(workspace + '/subdirectory'); + expect(bwrapArgs[bindIndex - 1]).toBe('--ro-bind-try'); }); it('should not bind the workspace twice even if it has a trailing slash in allowedPaths', async () => { @@ -274,23 +323,20 @@ describe('LinuxSandboxManager', () => { }, }); - // Should only contain the primary workspace bind and governance files, not the second workspace bind with a trailing slash - expectDynamicBinds(bwrapArgs, []); + const binds = bwrapArgs.filter((a) => a === workspace); + expect(binds.length).toBe(2); }); }); describe('forbiddenPaths', () => { it('should parameterize forbidden paths and explicitly deny them', async () => { - vi.spyOn(fs.promises, 'stat').mockImplementation(async (p) => { - // Mock /tmp/cache as a directory, and /opt/secret.txt as a file + vi.mocked(fs.statSync).mockImplementation((p) => { 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(), - ); + vi.mocked(fs.realpathSync).mockImplementation((p) => p.toString()); const bwrapArgs = await getBwrapArgs({ command: 'ls', @@ -302,27 +348,22 @@ describe('LinuxSandboxManager', () => { }, }); - expectDynamicBinds(bwrapArgs, [ - '--tmpfs', - '/tmp/cache', - '--remount-ro', - '/tmp/cache', - '--ro-bind-try', - '/dev/null', - '/opt/secret.txt', - ]); + const cacheIndex = bwrapArgs.indexOf('/tmp/cache'); + expect(bwrapArgs[cacheIndex - 1]).toBe('--tmpfs'); + + const secretIndex = bwrapArgs.indexOf('/opt/secret.txt'); + expect(bwrapArgs[secretIndex - 2]).toBe('--ro-bind'); + expect(bwrapArgs[secretIndex - 1]).toBe('/dev/null'); }); it('resolves forbidden symlink paths to their real paths', 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(); - }, + vi.mocked(fs.statSync).mockImplementation( + () => ({ isDirectory: () => false }) as fs.Stats, ); + vi.mocked(fs.realpathSync).mockImplementation((p) => { + if (p === '/tmp/forbidden-symlink') return '/opt/real-target.txt'; + return p.toString(); + }); const bwrapArgs = await getBwrapArgs({ command: 'ls', @@ -334,24 +375,18 @@ describe('LinuxSandboxManager', () => { }, }); - // 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', - ]); + const secretIndex = bwrapArgs.indexOf('/opt/real-target.txt'); + expect(bwrapArgs[secretIndex - 2]).toBe('--ro-bind'); + expect(bwrapArgs[secretIndex - 1]).toBe('/dev/null'); }); it('explicitly denies non-existent forbidden paths to prevent creation', 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(), - ); + vi.mocked(fs.statSync).mockImplementation(() => { + throw error; + }); + vi.mocked(fs.realpathSync).mockImplementation((p) => p.toString()); const bwrapArgs = await getBwrapArgs({ command: 'ls', @@ -363,23 +398,19 @@ describe('LinuxSandboxManager', () => { }, }); - expectDynamicBinds(bwrapArgs, [ - '--symlink', - '/.forbidden', - '/tmp/not-here.txt', - ]); + const idx = bwrapArgs.indexOf('/tmp/not-here.txt'); + expect(bwrapArgs[idx - 2]).toBe('--symlink'); + expect(bwrapArgs[idx - 1]).toBe('/dev/null'); }); 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(); - }, + vi.mocked(fs.statSync).mockImplementation( + () => ({ isDirectory: () => true }) as fs.Stats, ); + vi.mocked(fs.realpathSync).mockImplementation((p) => { + if (p === '/tmp/dir-link') return '/opt/real-dir'; + return p.toString(); + }); const bwrapArgs = await getBwrapArgs({ command: 'ls', @@ -391,25 +422,15 @@ describe('LinuxSandboxManager', () => { }, }); - expectDynamicBinds(bwrapArgs, [ - '--tmpfs', - '/opt/real-dir', - '--remount-ro', - '/opt/real-dir', - '--tmpfs', - '/tmp/dir-link', - '--remount-ro', - '/tmp/dir-link', - ]); + const idx = bwrapArgs.indexOf('/opt/real-dir'); + expect(bwrapArgs[idx - 1]).toBe('--tmpfs'); }); it('should override allowed paths if a path is also in forbidden paths', async () => { - vi.spyOn(fs.promises, 'stat').mockImplementation( - async () => ({ isDirectory: () => true }) as fs.Stats, - ); - vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) => - p.toString(), + vi.mocked(fs.statSync).mockImplementation( + () => ({ isDirectory: () => true }) as fs.Stats, ); + vi.mocked(fs.realpathSync).mockImplementation((p) => p.toString()); const bwrapArgs = await getBwrapArgs({ command: 'ls', @@ -422,15 +443,12 @@ describe('LinuxSandboxManager', () => { }, }); - expectDynamicBinds(bwrapArgs, [ - '--bind-try', - '/tmp/conflict', - '/tmp/conflict', - '--tmpfs', - '/tmp/conflict', - '--remount-ro', - '/tmp/conflict', - ]); + const bindTryIdx = bwrapArgs.indexOf('--bind-try'); + const tmpfsIdx = bwrapArgs.lastIndexOf('--tmpfs'); + + expect(bwrapArgs[bindTryIdx + 1]).toBe('/tmp/conflict'); + expect(bwrapArgs[tmpfsIdx + 1]).toBe('/tmp/conflict'); + expect(tmpfsIdx).toBeGreaterThan(bindTryIdx); }); }); }); diff --git a/packages/core/src/sandbox/linux/LinuxSandboxManager.ts b/packages/core/src/sandbox/linux/LinuxSandboxManager.ts index 2b3e8cc7c9..33f12beafa 100644 --- a/packages/core/src/sandbox/linux/LinuxSandboxManager.ts +++ b/packages/core/src/sandbox/linux/LinuxSandboxManager.ts @@ -5,6 +5,7 @@ */ import fs from 'node:fs'; +import { debugLogger } from '../../utils/debugLogger.js'; import { join, dirname, normalize } from 'node:path'; import os from 'node:os'; import { @@ -12,15 +13,25 @@ import { type GlobalSandboxOptions, type SandboxRequest, type SandboxedCommand, + type SandboxPermissions, GOVERNANCE_FILES, sanitizePaths, - tryRealpath, } from '../../services/sandboxManager.js'; import { sanitizeEnvironment, getSecureSanitizationConfig, } from '../../services/environmentSanitization.js'; -import { isNodeError } from '../../utils/errors.js'; +import { type SandboxPolicyManager } from '../../policy/sandboxPolicyManager.js'; +import { + isStrictlyApproved, + verifySandboxOverrides, + getCommandName, +} from '../utils/commandUtils.js'; +import { + tryRealpath, + resolveGitWorktreePaths, + isErrnoException, +} from '../utils/fsUtils.js'; let cachedBpfPath: string | undefined; @@ -102,13 +113,24 @@ function touch(filePath: string, isDirectory: boolean) { import { isKnownSafeCommand, isDangerousCommand, -} from '../macos/commandSafety.js'; +} from '../utils/commandSafety.js'; /** * A SandboxManager implementation for Linux that uses Bubblewrap (bwrap). */ + +export interface LinuxSandboxOptions extends GlobalSandboxOptions { + modeConfig?: { + readonly?: boolean; + network?: boolean; + approvedTools?: string[]; + allowOverrides?: boolean; + }; + policyManager?: SandboxPolicyManager; +} + export class LinuxSandboxManager implements SandboxManager { - constructor(private readonly options: GlobalSandboxOptions) {} + constructor(private readonly options: LinuxSandboxOptions) {} isKnownSafeCommand(args: string[]): boolean { return isKnownSafeCommand(args); @@ -119,6 +141,41 @@ export class LinuxSandboxManager implements SandboxManager { } async prepareCommand(req: SandboxRequest): Promise { + const isReadonlyMode = this.options.modeConfig?.readonly ?? true; + const allowOverrides = this.options.modeConfig?.allowOverrides ?? true; + + verifySandboxOverrides(allowOverrides, req.policy); + + const commandName = await getCommandName(req); + const isApproved = allowOverrides + ? await isStrictlyApproved(req, this.options.modeConfig?.approvedTools) + : false; + const workspaceWrite = !isReadonlyMode || isApproved; + const networkAccess = + this.options.modeConfig?.network ?? req.policy?.networkAccess ?? false; + + const persistentPermissions = allowOverrides + ? this.options.policyManager?.getCommandPermissions(commandName) + : undefined; + + const mergedAdditional: SandboxPermissions = { + fileSystem: { + read: [ + ...(persistentPermissions?.fileSystem?.read ?? []), + ...(req.policy?.additionalPermissions?.fileSystem?.read ?? []), + ], + write: [ + ...(persistentPermissions?.fileSystem?.write ?? []), + ...(req.policy?.additionalPermissions?.fileSystem?.write ?? []), + ], + }, + network: + networkAccess || + persistentPermissions?.network || + req.policy?.additionalPermissions?.network || + false, + }; + const sanitizationConfig = getSecureSanitizationConfig( req.policy?.sanitizationConfig, ); @@ -126,13 +183,142 @@ export class LinuxSandboxManager implements SandboxManager { const sanitizedEnv = sanitizeEnvironment(req.env, sanitizationConfig); const bwrapArgs: string[] = [ - ...this.getNetworkArgs(req), - ...this.getBaseArgs(), - ...this.getGovernanceArgs(), - ...this.getAllowedPathsArgs(req.policy?.allowedPaths), - ...(await this.getForbiddenPathsArgs(req.policy?.forbiddenPaths)), + '--unshare-all', + '--new-session', // Isolate session + '--die-with-parent', // Prevent orphaned runaway processes ]; + if (mergedAdditional.network) { + bwrapArgs.push('--share-net'); + } + + bwrapArgs.push( + '--ro-bind', + '/', + '/', + '--dev', // Creates a safe, minimal /dev (replaces --dev-bind) + '/dev', + '--proc', // Creates a fresh procfs for the unshared PID namespace + '/proc', + '--tmpfs', // Provides an isolated, writable /tmp directory + '/tmp', + ); + + const workspacePath = tryRealpath(this.options.workspace); + + const bindFlag = workspaceWrite ? '--bind-try' : '--ro-bind-try'; + + if (workspaceWrite) { + bwrapArgs.push( + '--bind-try', + this.options.workspace, + this.options.workspace, + ); + if (workspacePath !== this.options.workspace) { + bwrapArgs.push('--bind-try', workspacePath, workspacePath); + } + } else { + bwrapArgs.push( + '--ro-bind-try', + this.options.workspace, + this.options.workspace, + ); + if (workspacePath !== this.options.workspace) { + bwrapArgs.push('--ro-bind-try', workspacePath, workspacePath); + } + } + + const { worktreeGitDir, mainGitDir } = + resolveGitWorktreePaths(workspacePath); + if (worktreeGitDir) { + bwrapArgs.push(bindFlag, worktreeGitDir, worktreeGitDir); + } + if (mainGitDir) { + bwrapArgs.push(bindFlag, mainGitDir, mainGitDir); + } + + const allowedPaths = sanitizePaths(req.policy?.allowedPaths) || []; + const normalizedWorkspace = normalize(workspacePath).replace(/\/$/, ''); + for (const allowedPath of allowedPaths) { + const resolved = tryRealpath(allowedPath); + if (!fs.existsSync(resolved)) continue; + const normalizedAllowedPath = normalize(resolved).replace(/\/$/, ''); + if (normalizedAllowedPath !== normalizedWorkspace) { + if ( + !workspaceWrite && + normalizedAllowedPath.startsWith(normalizedWorkspace + '/') + ) { + bwrapArgs.push('--ro-bind-try', resolved, resolved); + } else { + bwrapArgs.push('--bind-try', resolved, resolved); + } + } + } + + const additionalReads = + sanitizePaths(mergedAdditional.fileSystem?.read) || []; + for (const p of additionalReads) { + try { + const safeResolvedPath = tryRealpath(p); + bwrapArgs.push('--ro-bind-try', safeResolvedPath, safeResolvedPath); + } catch (e: unknown) { + debugLogger.warn(e instanceof Error ? e.message : String(e)); + } + } + + const additionalWrites = + sanitizePaths(mergedAdditional.fileSystem?.write) || []; + for (const p of additionalWrites) { + try { + const safeResolvedPath = tryRealpath(p); + bwrapArgs.push('--bind-try', safeResolvedPath, safeResolvedPath); + } catch (e: unknown) { + debugLogger.warn(e instanceof Error ? e.message : String(e)); + } + } + + for (const file of GOVERNANCE_FILES) { + const filePath = join(this.options.workspace, file.path); + touch(filePath, file.isDirectory); + const realPath = tryRealpath(filePath); + bwrapArgs.push('--ro-bind', filePath, filePath); + if (realPath !== filePath) { + bwrapArgs.push('--ro-bind', realPath, realPath); + } + } + + const forbiddenPaths = sanitizePaths(req.policy?.forbiddenPaths) || []; + for (const p of forbiddenPaths) { + let resolved: string; + try { + resolved = tryRealpath(p); // Forbidden paths should still resolve to block the real path + if (!fs.existsSync(resolved)) continue; + } catch (e: unknown) { + debugLogger.warn( + `Failed to resolve forbidden path ${p}: ${e instanceof Error ? e.message : String(e)}`, + ); + bwrapArgs.push('--ro-bind', '/dev/null', p); + continue; + } + try { + const stat = fs.statSync(resolved); + if (stat.isDirectory()) { + bwrapArgs.push('--tmpfs', resolved, '--remount-ro', resolved); + } else { + bwrapArgs.push('--ro-bind', '/dev/null', resolved); + } + } catch (e: unknown) { + if (isErrnoException(e) && e.code === 'ENOENT') { + bwrapArgs.push('--symlink', '/dev/null', resolved); + } else { + debugLogger.warn( + `Failed to stat forbidden path ${resolved}: ${e instanceof Error ? e.message : String(e)}`, + ); + bwrapArgs.push('--ro-bind', '/dev/null', resolved); + } + } + } + const bpfPath = getSeccompBpfPath(); bwrapArgs.push('--seccomp', '9'); @@ -153,142 +339,4 @@ export class LinuxSandboxManager implements SandboxManager { cwd: req.cwd, }; } - - /** - * Generates arguments for network isolation. - */ - private getNetworkArgs(req: SandboxRequest): string[] { - return req.policy?.networkAccess - ? [ - '--unshare-user', - '--unshare-ipc', - '--unshare-pid', - '--unshare-uts', - '--unshare-cgroup', - ] - : ['--unshare-all']; - } - - /** - * Generates the base bubblewrap arguments for isolation. - */ - private getBaseArgs(): string[] { - return [ - '--new-session', // Isolate session - '--die-with-parent', // Prevent orphaned runaway processes - '--ro-bind', - '/', - '/', - '--dev', // Creates a safe, minimal /dev (replaces --dev-bind) - '/dev', - '--proc', // Creates a fresh procfs for the unshared PID namespace - '/proc', - '--tmpfs', // Provides an isolated, writable /tmp directory - '/tmp', - // Note: --dev /dev sets up /dev/pts automatically - '--bind', - this.options.workspace, - this.options.workspace, - ]; - } - - /** - * Generates arguments for protected governance files. - */ - private getGovernanceArgs(): string[] { - const args: string[] = []; - // Protected governance files are bind-mounted as read-only, even if the workspace is RW. - // We ensure they exist on the host and resolve real paths to prevent symlink bypasses. - // In bwrap, later binds override earlier ones for the same path. - for (const file of GOVERNANCE_FILES) { - const filePath = join(this.options.workspace, file.path); - touch(filePath, file.isDirectory); - - const realPath = fs.realpathSync(filePath); - - args.push('--ro-bind', filePath, filePath); - if (realPath !== filePath) { - args.push('--ro-bind', realPath, realPath); - } - } - return args; - } - - /** - * Generates arguments for allowed paths. - */ - private getAllowedPathsArgs(allowedPaths?: string[]): string[] { - const args: string[] = []; - const paths = sanitizePaths(allowedPaths) || []; - const normalizedWorkspace = this.normalizePath(this.options.workspace); - - for (const p of paths) { - if (this.normalizePath(p) !== normalizedWorkspace) { - args.push('--bind-try', p, p); - } - } - return args; - } - - /** - * Generates arguments for forbidden paths. - */ - private async getForbiddenPathsArgs( - forbiddenPaths?: string[], - ): Promise { - const args: string[] = []; - const paths = sanitizePaths(forbiddenPaths) || []; - - for (const p of paths) { - try { - const originalPath = this.normalizePath(p); - const resolvedPath = await tryRealpath(originalPath); - - // Mask the resolved path to prevent access to the underlying file. - const resolvedMask = await this.getMaskArgs(resolvedPath); - args.push(...resolvedMask); - - // If the original path was a symlink, mask it as well to prevent access - // through the link itself. - if (resolvedPath !== originalPath) { - const originalMask = await this.getMaskArgs(originalPath); - args.push(...originalMask); - } - } catch (e) { - throw new Error( - `Failed to deny access to forbidden path: ${p}. ${ - e instanceof Error ? e.message : String(e) - }`, - ); - } - } - return args; - } - - /** - * Generates bubblewrap arguments to mask a forbidden path. - */ - private async getMaskArgs(path: string): Promise { - try { - const stats = await fs.promises.stat(path); - - if (stats.isDirectory()) { - // Directories are masked by mounting an empty, read-only tmpfs. - return ['--tmpfs', path, '--remount-ro', path]; - } - // Existing files are masked by binding them to /dev/null. - return ['--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. - return ['--symlink', '/.forbidden', path]; - } - throw e; - } - } - - private normalizePath(p: string): string { - return normalize(p).replace(/\/$/, ''); - } } diff --git a/packages/core/src/sandbox/macos/MacOsSandboxManager.test.ts b/packages/core/src/sandbox/macos/MacOsSandboxManager.test.ts index 0c7e83ecfe..3f23a22553 100644 --- a/packages/core/src/sandbox/macos/MacOsSandboxManager.test.ts +++ b/packages/core/src/sandbox/macos/MacOsSandboxManager.test.ts @@ -38,7 +38,7 @@ describe('MacOsSandboxManager', () => { manager = new MacOsSandboxManager({ workspace: mockWorkspace }); // Mock the seatbelt args builder to isolate manager tests - vi.spyOn(seatbeltArgsBuilder, 'buildSeatbeltArgs').mockResolvedValue([ + vi.spyOn(seatbeltArgsBuilder, 'buildSeatbeltArgs').mockReturnValue([ '-p', '(mock profile)', '-D', diff --git a/packages/core/src/sandbox/macos/MacOsSandboxManager.ts b/packages/core/src/sandbox/macos/MacOsSandboxManager.ts index c767c18b82..db2768d7c6 100644 --- a/packages/core/src/sandbox/macos/MacOsSandboxManager.ts +++ b/packages/core/src/sandbox/macos/MacOsSandboxManager.ts @@ -24,8 +24,9 @@ import { isKnownSafeCommand, isDangerousCommand, isStrictlyApproved, -} from './commandSafety.js'; +} from '../utils/commandSafety.js'; import { type SandboxPolicyManager } from '../../policy/sandboxPolicyManager.js'; +import { verifySandboxOverrides } from '../utils/commandUtils.js'; export interface MacOsSandboxOptions extends GlobalSandboxOptions { /** The current sandbox mode behavior from config. */ @@ -70,17 +71,7 @@ export class MacOsSandboxManager implements SandboxManager { const allowOverrides = this.options.modeConfig?.allowOverrides ?? true; // Reject override attempts in plan mode - if (!allowOverrides && req.policy?.additionalPermissions) { - const perms = req.policy.additionalPermissions; - if ( - perms.network || - (perms.fileSystem?.write && perms.fileSystem.write.length > 0) - ) { - throw new Error( - 'Sandbox request rejected: Cannot override readonly/network restrictions in Plan mode.', - ); - } - } + verifySandboxOverrides(allowOverrides, req.policy); // If not in readonly mode OR it's a strictly approved pipeline, allow workspace writes const isApproved = allowOverrides @@ -120,7 +111,7 @@ export class MacOsSandboxManager implements SandboxManager { false, }; - const sandboxArgs = await buildSeatbeltArgs({ + const sandboxArgs = buildSeatbeltArgs({ workspace: this.options.workspace, allowedPaths: [...(req.policy?.allowedPaths || [])], forbiddenPaths: req.policy?.forbiddenPaths, diff --git a/packages/core/src/sandbox/macos/seatbeltArgsBuilder.test.ts b/packages/core/src/sandbox/macos/seatbeltArgsBuilder.test.ts index dd2c95235e..fcab494059 100644 --- a/packages/core/src/sandbox/macos/seatbeltArgsBuilder.test.ts +++ b/packages/core/src/sandbox/macos/seatbeltArgsBuilder.test.ts @@ -3,25 +3,31 @@ * Copyright 2026 Google LLC * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; import { buildSeatbeltArgs } from './seatbeltArgsBuilder.js'; -import * as sandboxManager from '../../services/sandboxManager.js'; +import * as fsUtils from '../utils/fsUtils.js'; import fs from 'node:fs'; import os from 'node:os'; +vi.mock('../utils/fsUtils.js', async () => { + const actual = await vi.importActual('../utils/fsUtils.js'); + return { + ...actual, + tryRealpath: vi.fn((p) => p), + resolveGitWorktreePaths: vi.fn(() => ({})), + }; +}); + describe('seatbeltArgsBuilder', () => { - beforeEach(() => { + afterEach(() => { vi.restoreAllMocks(); }); describe('buildSeatbeltArgs', () => { - it('should build a strict allowlist profile allowing the workspace via param', async () => { - // Mock tryRealpath to just return the path for testing - vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation( - async (p) => p, - ); + it('should build a strict allowlist profile allowing the workspace via param', () => { + vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p); - const args = await buildSeatbeltArgs({ + const args = buildSeatbeltArgs({ workspace: '/Users/test/workspace', }); @@ -38,11 +44,9 @@ describe('seatbeltArgsBuilder', () => { expect(args).toContain(`TMPDIR=${os.tmpdir()}`); }); - it('should allow network when networkAccess is true', async () => { - vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation( - async (p) => p, - ); - const args = await buildSeatbeltArgs({ + it('should allow network when networkAccess is true', () => { + vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p); + const args = buildSeatbeltArgs({ workspace: '/test', networkAccess: true, }); @@ -51,10 +55,8 @@ describe('seatbeltArgsBuilder', () => { }); describe('governance files', () => { - it('should inject explicit deny rules for governance files', async () => { - vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) => - p.toString(), - ); + it('should inject explicit deny rules for governance files', () => { + vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p.toString()); vi.spyOn(fs, 'existsSync').mockReturnValue(true); vi.spyOn(fs, 'lstatSync').mockImplementation( (p) => @@ -64,35 +66,29 @@ describe('seatbeltArgsBuilder', () => { }) as unknown as fs.Stats, ); - const args = await buildSeatbeltArgs({ - workspace: '/Users/test/workspace', + const args = buildSeatbeltArgs({ + workspace: '/test/workspace', }); const profile = args[1]; - // .gitignore should be a literal deny expect(args).toContain('-D'); - expect(args).toContain( - 'GOVERNANCE_FILE_0=/Users/test/workspace/.gitignore', - ); + expect(args).toContain('GOVERNANCE_FILE_0=/test/workspace/.gitignore'); expect(profile).toContain( '(deny file-write* (literal (param "GOVERNANCE_FILE_0")))', ); - // .git should be a subpath deny - expect(args).toContain('GOVERNANCE_FILE_2=/Users/test/workspace/.git'); + expect(args).toContain('GOVERNANCE_FILE_2=/test/workspace/.git'); expect(profile).toContain( '(deny file-write* (subpath (param "GOVERNANCE_FILE_2")))', ); }); - it('should protect both the symlink and the real path if they differ', async () => { - vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation( - async (p) => { - if (p === '/test/workspace/.gitignore') - return '/test/real/.gitignore'; - return p.toString(); - }, - ); + it('should protect both the symlink and the real path if they differ', () => { + vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => { + if (p === '/test/workspace/.gitignore') + return '/test/real/.gitignore'; + return p.toString(); + }); vi.spyOn(fs, 'existsSync').mockReturnValue(true); vi.spyOn(fs, 'lstatSync').mockImplementation( () => @@ -102,7 +98,7 @@ describe('seatbeltArgsBuilder', () => { }) as unknown as fs.Stats, ); - const args = await buildSeatbeltArgs({ workspace: '/test/workspace' }); + const args = buildSeatbeltArgs({ workspace: '/test/workspace' }); const profile = args[1]; expect(args).toContain('GOVERNANCE_FILE_0=/test/workspace/.gitignore'); @@ -117,15 +113,13 @@ describe('seatbeltArgsBuilder', () => { }); describe('allowedPaths', () => { - it('should parameterize allowed paths and normalize them', async () => { - vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation( - async (p) => { - if (p === '/test/symlink') return '/test/real_path'; - return p; - }, - ); + it('should parameterize allowed paths and normalize them', () => { + vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => { + if (p === '/test/symlink') return '/test/real_path'; + return p; + }); - const args = await buildSeatbeltArgs({ + const args = buildSeatbeltArgs({ workspace: '/test', allowedPaths: ['/custom/path1', '/test/symlink'], }); @@ -141,12 +135,10 @@ describe('seatbeltArgsBuilder', () => { }); describe('forbiddenPaths', () => { - it('should parameterize forbidden paths and explicitly deny them', async () => { - vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation( - async (p) => p, - ); + it('should parameterize forbidden paths and explicitly deny them', () => { + vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p); - const args = await buildSeatbeltArgs({ + const args = buildSeatbeltArgs({ workspace: '/test', forbiddenPaths: ['/secret/path'], }); @@ -161,22 +153,21 @@ describe('seatbeltArgsBuilder', () => { ); }); - it('resolves forbidden symlink paths to their real paths', async () => { - vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation( - async (p) => { - if (p === '/test/symlink') return '/test/real_path'; - return p; - }, - ); + it('resolves forbidden symlink paths to their real paths', () => { + vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => { + if (p === '/test/symlink' || p === '/test/missing-dir') { + return '/test/real_path'; + } + return p; + }); - const args = await buildSeatbeltArgs({ + const args = buildSeatbeltArgs({ workspace: '/test', forbiddenPaths: ['/test/symlink'], }); const profile = args[1]; - // The builder should resolve the symlink and explicitly deny the real target path expect(args).toContain('-D'); expect(args).toContain('FORBIDDEN_PATH_0=/test/real_path'); expect(profile).toContain( @@ -184,12 +175,10 @@ describe('seatbeltArgsBuilder', () => { ); }); - it('explicitly denies non-existent forbidden paths to prevent creation', async () => { - vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation( - async (p) => p, - ); + it('explicitly denies non-existent forbidden paths to prevent creation', () => { + vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p); - const args = await buildSeatbeltArgs({ + const args = buildSeatbeltArgs({ workspace: '/test', forbiddenPaths: ['/test/missing-dir/missing-file.txt'], }); @@ -205,12 +194,10 @@ describe('seatbeltArgsBuilder', () => { ); }); - it('should override allowed paths if a path is also in forbidden paths', async () => { - vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation( - async (p) => p, - ); + it('should override allowed paths if a path is also in forbidden paths', () => { + vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p); - const args = await buildSeatbeltArgs({ + const args = buildSeatbeltArgs({ workspace: '/test', allowedPaths: ['/custom/path1'], forbiddenPaths: ['/custom/path1'], @@ -226,8 +213,6 @@ describe('seatbeltArgsBuilder', () => { expect(profile).toContain(allowString); expect(profile).toContain(denyString); - // Verify ordering: The explicit deny must appear AFTER the explicit allow in the profile string - // Seatbelt rules are evaluated in order where the latest rule matching a path wins const allowIndex = profile.indexOf(allowString); const denyIndex = profile.indexOf(denyString); expect(denyIndex).toBeGreaterThan(allowIndex); diff --git a/packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts b/packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts index f72229b5cc..cfdcee1687 100644 --- a/packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts +++ b/packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts @@ -15,8 +15,8 @@ import { type SandboxPermissions, sanitizePaths, GOVERNANCE_FILES, - tryRealpath, } from '../../services/sandboxManager.js'; +import { tryRealpath, resolveGitWorktreePaths } from '../utils/fsUtils.js'; /** * Options for building macOS Seatbelt arguments. @@ -44,13 +44,11 @@ export interface SeatbeltArgsOptions { * Returns arguments up to the end of sandbox-exec configuration (e.g. ['-p', '', '-D', ...]) * Does not include the final '--' separator or the command to run. */ -export async function buildSeatbeltArgs( - options: SeatbeltArgsOptions, -): Promise { +export function buildSeatbeltArgs(options: SeatbeltArgsOptions): string[] { let profile = BASE_SEATBELT_PROFILE + '\n'; const args: string[] = []; - const workspacePath = await tryRealpath(options.workspace); + const workspacePath = tryRealpath(options.workspace); args.push('-D', `WORKSPACE=${workspacePath}`); args.push('-D', `WORKSPACE_RAW=${options.workspace}`); profile += `(allow file-read* (subpath (param "WORKSPACE_RAW")))\n`; @@ -67,7 +65,7 @@ export async function buildSeatbeltArgs( // (Seatbelt evaluates rules in order, later rules win for same path). for (let i = 0; i < GOVERNANCE_FILES.length; i++) { const governanceFile = path.join(workspacePath, GOVERNANCE_FILES[i].path); - const realGovernanceFile = await tryRealpath(governanceFile); + const realGovernanceFile = tryRealpath(governanceFile); // Determine if it should be treated as a directory (subpath) or a file (literal). // .git is generally a directory, while ignore files are literals. @@ -92,42 +90,20 @@ export async function buildSeatbeltArgs( } // Auto-detect and support git worktrees by granting read and write access to the underlying git directory - try { - const gitPath = path.join(workspacePath, '.git'); - const gitStat = fs.lstatSync(gitPath); - if (gitStat.isFile()) { - const gitContent = fs.readFileSync(gitPath, 'utf8'); - const match = gitContent.match(/^gitdir:\s*(.+)$/m); - if (match && match[1]) { - let worktreeGitDir = match[1].trim(); - if (!path.isAbsolute(worktreeGitDir)) { - worktreeGitDir = path.resolve(workspacePath, worktreeGitDir); - } - const resolvedWorktreeGitDir = await tryRealpath(worktreeGitDir); - - // Grant write access to the worktree's specific .git directory - args.push('-D', `WORKTREE_GIT_DIR=${resolvedWorktreeGitDir}`); - profile += `(allow file-read* file-write* (subpath (param "WORKTREE_GIT_DIR")))\n`; - - // Grant write access to the main repository's .git directory (objects, refs, etc. are shared) - // resolvedWorktreeGitDir is usually like: /path/to/main-repo/.git/worktrees/worktree-name - const mainGitDir = await tryRealpath( - path.dirname(path.dirname(resolvedWorktreeGitDir)), - ); - if (mainGitDir && mainGitDir.endsWith('.git')) { - args.push('-D', `MAIN_GIT_DIR=${mainGitDir}`); - profile += `(allow file-read* file-write* (subpath (param "MAIN_GIT_DIR")))\n`; - } - } - } - } catch (_e) { - // Ignore if .git doesn't exist, isn't readable, etc. + const { worktreeGitDir, mainGitDir } = resolveGitWorktreePaths(workspacePath); + if (worktreeGitDir) { + args.push('-D', `WORKTREE_GIT_DIR=${worktreeGitDir}`); + profile += `(allow file-read* file-write* (subpath (param "WORKTREE_GIT_DIR")))\n`; + } + if (mainGitDir) { + args.push('-D', `MAIN_GIT_DIR=${mainGitDir}`); + profile += `(allow file-read* file-write* (subpath (param "MAIN_GIT_DIR")))\n`; } - const tmpPath = await tryRealpath(os.tmpdir()); + const tmpPath = tryRealpath(os.tmpdir()); args.push('-D', `TMPDIR=${tmpPath}`); - const nodeRootPath = await tryRealpath( + const nodeRootPath = tryRealpath( path.dirname(path.dirname(process.execPath)), ); args.push('-D', `NODE_ROOT=${nodeRootPath}`); @@ -142,7 +118,7 @@ export async function buildSeatbeltArgs( for (const p of paths) { if (!p.trim()) continue; try { - let resolved = await tryRealpath(p); + let resolved = tryRealpath(p); // If this is a 'bin' directory (like /usr/local/bin or homebrew/bin), // also grant read access to its parent directory so that symlinked @@ -165,8 +141,10 @@ export async function buildSeatbeltArgs( // Handle allowedPaths const allowedPaths = sanitizePaths(options.allowedPaths) || []; + const resolvedAllowedPaths: string[] = []; for (let i = 0; i < allowedPaths.length; i++) { - const allowedPath = await tryRealpath(allowedPaths[i]); + const allowedPath = tryRealpath(allowedPaths[i]); + resolvedAllowedPaths.push(allowedPath); args.push('-D', `ALLOWED_PATH_${i}=${allowedPath}`); profile += `(allow file-read* file-write* (subpath (param "ALLOWED_PATH_${i}")))\n`; } @@ -176,7 +154,7 @@ export async function buildSeatbeltArgs( const { read, write } = options.additionalPermissions.fileSystem; if (read) { for (let i = 0; i < read.length; i++) { - const resolved = await tryRealpath(read[i]); + const resolved = tryRealpath(read[i]); const paramName = `ADDITIONAL_READ_${i}`; args.push('-D', `${paramName}=${resolved}`); let isFile = false; @@ -194,7 +172,7 @@ export async function buildSeatbeltArgs( } if (write) { for (let i = 0; i < write.length; i++) { - const resolved = await tryRealpath(write[i]); + const resolved = tryRealpath(write[i]); const paramName = `ADDITIONAL_WRITE_${i}`; args.push('-D', `${paramName}=${resolved}`); let isFile = false; @@ -215,7 +193,7 @@ export async function buildSeatbeltArgs( // Handle forbiddenPaths const forbiddenPaths = sanitizePaths(options.forbiddenPaths) || []; for (let i = 0; i < forbiddenPaths.length; i++) { - const forbiddenPath = await tryRealpath(forbiddenPaths[i]); + const forbiddenPath = tryRealpath(forbiddenPaths[i]); args.push('-D', `FORBIDDEN_PATH_${i}=${forbiddenPath}`); profile += `(deny file-read* file-write* (subpath (param "FORBIDDEN_PATH_${i}")))\n`; } diff --git a/packages/core/src/sandbox/macos/commandSafety.ts b/packages/core/src/sandbox/utils/commandSafety.ts similarity index 100% rename from packages/core/src/sandbox/macos/commandSafety.ts rename to packages/core/src/sandbox/utils/commandSafety.ts diff --git a/packages/core/src/sandbox/utils/commandUtils.ts b/packages/core/src/sandbox/utils/commandUtils.ts new file mode 100644 index 0000000000..772df65afa --- /dev/null +++ b/packages/core/src/sandbox/utils/commandUtils.ts @@ -0,0 +1,82 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { type SandboxRequest } from '../../services/sandboxManager.js'; +import { + getCommandRoots, + initializeShellParsers, + splitCommands, + stripShellWrapper, +} from '../../utils/shell-utils.js'; +import { isKnownSafeCommand } from './commandSafety.js'; +import { parse as shellParse } from 'shell-quote'; +import path from 'node:path'; + +export async function isStrictlyApproved( + req: SandboxRequest, + approvedTools?: string[], +): Promise { + if (!approvedTools || approvedTools.length === 0) { + return false; + } + + await initializeShellParsers(); + + const fullCmd = [req.command, ...req.args].join(' '); + const stripped = stripShellWrapper(fullCmd); + + const roots = getCommandRoots(stripped); + if (roots.length === 0) return false; + + const allRootsApproved = roots.every((root) => approvedTools.includes(root)); + if (allRootsApproved) { + return true; + } + + const pipelineCommands = splitCommands(stripped); + if (pipelineCommands.length === 0) return false; + + for (const cmdString of pipelineCommands) { + const parsedArgs = shellParse(cmdString).map(String); + if (!isKnownSafeCommand(parsedArgs)) { + return false; + } + } + + return true; +} + +export async function getCommandName(req: SandboxRequest): Promise { + await initializeShellParsers(); + const fullCmd = [req.command, ...req.args].join(' '); + const stripped = stripShellWrapper(fullCmd); + const roots = getCommandRoots(stripped).filter( + (r) => r !== 'shopt' && r !== 'set', + ); + if (roots.length > 0) { + return roots[0]; + } + return path.basename(req.command); +} + +export function verifySandboxOverrides( + allowOverrides: boolean, + policy: SandboxRequest['policy'], +) { + if (!allowOverrides) { + if ( + policy?.networkAccess || + policy?.allowedPaths?.length || + policy?.additionalPermissions?.network || + policy?.additionalPermissions?.fileSystem?.read?.length || + policy?.additionalPermissions?.fileSystem?.write?.length + ) { + throw new Error( + 'Sandbox request rejected: Cannot override readonly/network/filesystem restrictions in Plan mode.', + ); + } + } +} diff --git a/packages/core/src/sandbox/utils/fsUtils.ts b/packages/core/src/sandbox/utils/fsUtils.ts new file mode 100644 index 0000000000..f7fafd4c59 --- /dev/null +++ b/packages/core/src/sandbox/utils/fsUtils.ts @@ -0,0 +1,92 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import fs from 'node:fs'; +import path from 'node:path'; + +export function isErrnoException(e: unknown): e is NodeJS.ErrnoException { + return e instanceof Error && 'code' in e; +} + +export function tryRealpath(p: string): string { + try { + return fs.realpathSync(p); + } catch (_e) { + if (isErrnoException(_e) && _e.code === 'ENOENT') { + const parentDir = path.dirname(p); + if (parentDir === p) { + return p; + } + return path.join(tryRealpath(parentDir), path.basename(p)); + } + throw _e; + } +} + +export function resolveGitWorktreePaths(workspacePath: string): { + worktreeGitDir?: string; + mainGitDir?: string; +} { + try { + const gitPath = path.join(workspacePath, '.git'); + const gitStat = fs.lstatSync(gitPath); + if (gitStat.isFile()) { + const gitContent = fs.readFileSync(gitPath, 'utf8'); + const match = gitContent.match(/^gitdir:\s+(.+)$/m); + if (match && match[1]) { + let worktreeGitDir = match[1].trim(); + if (!path.isAbsolute(worktreeGitDir)) { + worktreeGitDir = path.resolve(workspacePath, worktreeGitDir); + } + const resolvedWorktreeGitDir = tryRealpath(worktreeGitDir); + + // Security check: Verify the bidirectional link to prevent sandbox escape + let isValid = false; + try { + const backlinkPath = path.join(resolvedWorktreeGitDir, 'gitdir'); + const backlink = fs.readFileSync(backlinkPath, 'utf8').trim(); + // The backlink must resolve to the workspace's .git file + if (tryRealpath(backlink) === tryRealpath(gitPath)) { + isValid = true; + } + } catch (_e) { + // Fallback for submodules: check core.worktree in config + try { + const configPath = path.join(resolvedWorktreeGitDir, 'config'); + const config = fs.readFileSync(configPath, 'utf8'); + const match = config.match(/^\s*worktree\s*=\s*(.+)$/m); + if (match && match[1]) { + const worktreePath = path.resolve( + resolvedWorktreeGitDir, + match[1].trim(), + ); + if (tryRealpath(worktreePath) === tryRealpath(workspacePath)) { + isValid = true; + } + } + } catch (_e2) { + // Ignore + } + } + + if (!isValid) { + return {}; // Reject: valid worktrees/submodules must have a readable backlink + } + + const mainGitDir = tryRealpath( + path.dirname(path.dirname(resolvedWorktreeGitDir)), + ); + return { + worktreeGitDir: resolvedWorktreeGitDir, + mainGitDir: mainGitDir.endsWith('.git') ? mainGitDir : undefined, + }; + } + } + } catch (_e) { + // Ignore if .git doesn't exist, isn't readable, etc. + } + return {}; +} diff --git a/packages/core/src/sandbox/windows/WindowsSandboxManager.test.ts b/packages/core/src/sandbox/windows/WindowsSandboxManager.test.ts index 8f9b9d617c..2c7e08a730 100644 --- a/packages/core/src/sandbox/windows/WindowsSandboxManager.test.ts +++ b/packages/core/src/sandbox/windows/WindowsSandboxManager.test.ts @@ -111,7 +111,7 @@ describe('WindowsSandboxManager', () => { }; await expect(planManager.prepareCommand(req)).rejects.toThrow( - 'Sandbox request rejected: Cannot override readonly/network restrictions in Plan mode.', + 'Sandbox request rejected: Cannot override readonly/network/filesystem restrictions in Plan mode.', ); }); diff --git a/packages/core/src/sandbox/windows/WindowsSandboxManager.ts b/packages/core/src/sandbox/windows/WindowsSandboxManager.ts index 0a5d08637c..a213d7b619 100644 --- a/packages/core/src/sandbox/windows/WindowsSandboxManager.ts +++ b/packages/core/src/sandbox/windows/WindowsSandboxManager.ts @@ -31,6 +31,7 @@ import { isStrictlyApproved, } from './commandSafety.js'; import { type SandboxPolicyManager } from '../../policy/sandboxPolicyManager.js'; +import { verifySandboxOverrides } from '../utils/commandUtils.js'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); @@ -214,17 +215,7 @@ export class WindowsSandboxManager implements SandboxManager { const allowOverrides = this.options.modeConfig?.allowOverrides ?? true; // Reject override attempts in plan mode - if (!allowOverrides && req.policy?.additionalPermissions) { - const perms = req.policy.additionalPermissions; - if ( - perms.network || - (perms.fileSystem?.write && perms.fileSystem.write.length > 0) - ) { - throw new Error( - 'Sandbox request rejected: Cannot override readonly/network restrictions in Plan mode.', - ); - } - } + verifySandboxOverrides(allowOverrides, req.policy); // Fetch persistent approvals for this command const commandName = await getCommandName(req.command, req.args); diff --git a/packages/core/src/services/sandboxManager.ts b/packages/core/src/services/sandboxManager.ts index 0e282b0748..ea18e5857d 100644 --- a/packages/core/src/services/sandboxManager.ts +++ b/packages/core/src/services/sandboxManager.ts @@ -10,7 +10,7 @@ import path from 'node:path'; import { isKnownSafeCommand as isMacSafeCommand, isDangerousCommand as isMacDangerousCommand, -} from '../sandbox/macos/commandSafety.js'; +} from '../sandbox/utils/commandSafety.js'; import { isKnownSafeCommand as isWindowsSafeCommand, isDangerousCommand as isWindowsDangerousCommand, diff --git a/packages/core/src/services/sandboxManagerFactory.ts b/packages/core/src/services/sandboxManagerFactory.ts index bb8cea4752..6e09ab135f 100644 --- a/packages/core/src/services/sandboxManagerFactory.ts +++ b/packages/core/src/services/sandboxManagerFactory.ts @@ -42,7 +42,11 @@ export function createSandboxManager( policyManager, }); } else if (os.platform() === 'linux') { - return new LinuxSandboxManager({ workspace }); + return new LinuxSandboxManager({ + workspace, + modeConfig, + policyManager, + }); } else if (os.platform() === 'darwin') { return new MacOsSandboxManager({ workspace,