feat(core): add forbiddenPaths to GlobalSandboxOptions and refactor createSandboxManager (#23936)

This commit is contained in:
Emily Hedlund
2026-03-27 12:57:26 -04:00
committed by GitHub
parent 33cf2da1df
commit 535667baf6
11 changed files with 170 additions and 155 deletions
+11 -5
View File
@@ -944,7 +944,9 @@ export class Config implements McpContext, AgentLoopContext {
networkAccess: false,
};
this._sandboxManager = createSandboxManager(this.sandbox, params.targetDir);
this._sandboxManager = createSandboxManager(this.sandbox, {
workspace: params.targetDir,
});
if (
!(this._sandboxManager instanceof NoopSandboxManager) &&
@@ -965,8 +967,10 @@ export class Config implements McpContext, AgentLoopContext {
'default';
this._sandboxManager = createSandboxManager(
this.sandbox,
params.targetDir,
this._sandboxPolicyManager,
{
workspace: params.targetDir,
policyManager: this._sandboxPolicyManager,
},
initialApprovalMode,
);
@@ -1618,8 +1622,10 @@ export class Config implements McpContext, AgentLoopContext {
private refreshSandboxManager(): void {
this._sandboxManager = createSandboxManager(
this.sandbox,
this.targetDir,
this._sandboxPolicyManager,
{
workspace: this.targetDir,
policyManager: this._sandboxPolicyManager,
},
this.getApprovalMode(),
);
this.shellExecutionConfig.sandboxManager = this._sandboxManager;
@@ -362,16 +362,21 @@ describe('LinuxSandboxManager', () => {
});
vi.mocked(fs.realpathSync).mockImplementation((p) => p.toString());
const bwrapArgs = await getBwrapArgs({
command: 'ls',
args: ['-la'],
cwd: workspace,
env: {},
policy: {
forbiddenPaths: ['/tmp/cache', '/opt/secret.txt'],
},
const customManager = new LinuxSandboxManager({
workspace,
forbiddenPaths: ['/tmp/cache', '/opt/secret.txt'],
});
const bwrapArgs = await getBwrapArgs(
{
command: 'ls',
args: ['-la'],
cwd: workspace,
env: {},
},
customManager,
);
const cacheIndex = bwrapArgs.indexOf('/tmp/cache');
expect(bwrapArgs[cacheIndex - 1]).toBe('--tmpfs');
@@ -389,16 +394,21 @@ describe('LinuxSandboxManager', () => {
return p.toString();
});
const bwrapArgs = await getBwrapArgs({
command: 'ls',
args: ['-la'],
cwd: workspace,
env: {},
policy: {
forbiddenPaths: ['/tmp/forbidden-symlink'],
},
const customManager = new LinuxSandboxManager({
workspace,
forbiddenPaths: ['/tmp/forbidden-symlink'],
});
const bwrapArgs = await getBwrapArgs(
{
command: 'ls',
args: ['-la'],
cwd: workspace,
env: {},
},
customManager,
);
const secretIndex = bwrapArgs.indexOf('/opt/real-target.txt');
expect(bwrapArgs[secretIndex - 2]).toBe('--ro-bind');
expect(bwrapArgs[secretIndex - 1]).toBe('/dev/null');
@@ -412,16 +422,21 @@ describe('LinuxSandboxManager', () => {
});
vi.mocked(fs.realpathSync).mockImplementation((p) => p.toString());
const bwrapArgs = await getBwrapArgs({
command: 'ls',
args: [],
cwd: workspace,
env: {},
policy: {
forbiddenPaths: ['/tmp/not-here.txt'],
},
const customManager = new LinuxSandboxManager({
workspace,
forbiddenPaths: ['/tmp/not-here.txt'],
});
const bwrapArgs = await getBwrapArgs(
{
command: 'ls',
args: [],
cwd: workspace,
env: {},
},
customManager,
);
const idx = bwrapArgs.indexOf('/tmp/not-here.txt');
expect(bwrapArgs[idx - 2]).toBe('--symlink');
expect(bwrapArgs[idx - 1]).toBe('/dev/null');
@@ -436,16 +451,21 @@ describe('LinuxSandboxManager', () => {
return p.toString();
});
const bwrapArgs = await getBwrapArgs({
command: 'ls',
args: [],
cwd: workspace,
env: {},
policy: {
forbiddenPaths: ['/tmp/dir-link'],
},
const customManager = new LinuxSandboxManager({
workspace,
forbiddenPaths: ['/tmp/dir-link'],
});
const bwrapArgs = await getBwrapArgs(
{
command: 'ls',
args: [],
cwd: workspace,
env: {},
},
customManager,
);
const idx = bwrapArgs.indexOf('/opt/real-dir');
expect(bwrapArgs[idx - 1]).toBe('--tmpfs');
});
@@ -456,17 +476,24 @@ describe('LinuxSandboxManager', () => {
);
vi.mocked(fs.realpathSync).mockImplementation((p) => p.toString());
const bwrapArgs = await getBwrapArgs({
command: 'ls',
args: ['-la'],
cwd: workspace,
env: {},
policy: {
allowedPaths: ['/tmp/conflict'],
forbiddenPaths: ['/tmp/conflict'],
},
const customManager = new LinuxSandboxManager({
workspace,
forbiddenPaths: ['/tmp/conflict'],
});
const bwrapArgs = await getBwrapArgs(
{
command: 'ls',
args: ['-la'],
cwd: workspace,
env: {},
policy: {
allowedPaths: ['/tmp/conflict'],
},
},
customManager,
);
const bindTryIdx = bwrapArgs.indexOf('--bind-try');
const tmpfsIdx = bwrapArgs.lastIndexOf('--tmpfs');
@@ -25,7 +25,6 @@ import {
} from '../../services/environmentSanitization.js';
import { debugLogger } from '../../utils/debugLogger.js';
import { spawnAsync } from '../../utils/shell-utils.js';
import { type SandboxPolicyManager } from '../../policy/sandboxPolicyManager.js';
import {
isStrictlyApproved,
verifySandboxOverrides,
@@ -134,20 +133,10 @@ function touch(filePath: string, isDirectory: boolean) {
* 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 {
private static maskFilePath: string | undefined;
constructor(private readonly options: LinuxSandboxOptions) {}
constructor(private readonly options: GlobalSandboxOptions) {}
isKnownSafeCommand(args: string[]): boolean {
return isKnownSafeCommand(args);
@@ -333,7 +322,7 @@ export class LinuxSandboxManager implements SandboxManager {
}
}
const forbiddenPaths = sanitizePaths(req.policy?.forbiddenPaths) || [];
const forbiddenPaths = sanitizePaths(this.options.forbiddenPaths) || [];
for (const p of forbiddenPaths) {
let resolved: string;
try {
@@ -35,7 +35,10 @@ describe('MacOsSandboxManager', () => {
networkAccess: mockNetworkAccess,
};
manager = new MacOsSandboxManager({ workspace: mockWorkspace });
manager = new MacOsSandboxManager({
workspace: mockWorkspace,
forbiddenPaths: [],
});
// Mock the seatbelt args builder to isolate manager tests
vi.spyOn(seatbeltArgsBuilder, 'buildSeatbeltArgs').mockReturnValue([
@@ -68,7 +71,7 @@ describe('MacOsSandboxManager', () => {
workspace: mockWorkspace,
allowedPaths: mockAllowedPaths,
networkAccess: mockNetworkAccess,
forbiddenPaths: undefined,
forbiddenPaths: [],
workspaceWrite: true,
additionalPermissions: {
fileSystem: {
@@ -177,15 +180,16 @@ describe('MacOsSandboxManager', () => {
describe('forbiddenPaths', () => {
it('should parameterize forbidden paths and explicitly deny them', async () => {
await manager.prepareCommand({
const managerWithForbidden = new MacOsSandboxManager({
workspace: mockWorkspace,
forbiddenPaths: ['/tmp/forbidden1'],
});
await managerWithForbidden.prepareCommand({
command: 'echo',
args: [],
cwd: mockWorkspace,
env: {},
policy: {
...mockPolicy,
forbiddenPaths: ['/tmp/forbidden1'],
},
policy: mockPolicy,
});
expect(seatbeltArgsBuilder.buildSeatbeltArgs).toHaveBeenCalledWith(
@@ -196,15 +200,16 @@ describe('MacOsSandboxManager', () => {
});
it('explicitly denies non-existent forbidden paths to prevent creation', async () => {
await manager.prepareCommand({
const managerWithForbidden = new MacOsSandboxManager({
workspace: mockWorkspace,
forbiddenPaths: ['/tmp/does-not-exist'],
});
await managerWithForbidden.prepareCommand({
command: 'echo',
args: [],
cwd: mockWorkspace,
env: {},
policy: {
...mockPolicy,
forbiddenPaths: ['/tmp/does-not-exist'],
},
policy: mockPolicy,
});
expect(seatbeltArgsBuilder.buildSeatbeltArgs).toHaveBeenCalledWith(
@@ -215,7 +220,11 @@ describe('MacOsSandboxManager', () => {
});
it('should override allowed paths if a path is also in forbidden paths', async () => {
await manager.prepareCommand({
const managerWithForbidden = new MacOsSandboxManager({
workspace: mockWorkspace,
forbiddenPaths: ['/tmp/conflict'],
});
await managerWithForbidden.prepareCommand({
command: 'echo',
args: [],
cwd: mockWorkspace,
@@ -223,7 +232,6 @@ describe('MacOsSandboxManager', () => {
policy: {
...mockPolicy,
allowedPaths: ['/tmp/conflict'],
forbiddenPaths: ['/tmp/conflict'],
},
});
@@ -27,27 +27,14 @@ import {
isDangerousCommand,
isStrictlyApproved,
} from '../utils/commandSafety.js';
import { type SandboxPolicyManager } from '../../policy/sandboxPolicyManager.js';
import { verifySandboxOverrides } from '../utils/commandUtils.js';
import { parsePosixSandboxDenials } from '../utils/sandboxDenialUtils.js';
export interface MacOsSandboxOptions extends GlobalSandboxOptions {
/** The current sandbox mode behavior from config. */
modeConfig?: {
readonly?: boolean;
network?: boolean;
approvedTools?: string[];
allowOverrides?: boolean;
};
/** The policy manager for persistent approvals. */
policyManager?: SandboxPolicyManager;
}
/**
* A SandboxManager implementation for macOS that uses Seatbelt.
*/
export class MacOsSandboxManager implements SandboxManager {
constructor(private readonly options: MacOsSandboxOptions) {}
constructor(private readonly options: GlobalSandboxOptions) {}
isKnownSafeCommand(args: string[]): boolean {
const toolName = args[0];
@@ -121,7 +108,7 @@ export class MacOsSandboxManager implements SandboxManager {
const sandboxArgs = buildSeatbeltArgs({
workspace: this.options.workspace,
allowedPaths: [...(req.policy?.allowedPaths || [])],
forbiddenPaths: req.policy?.forbiddenPaths,
forbiddenPaths: this.options.forbiddenPaths,
networkAccess: mergedAdditional.network,
workspaceWrite,
additionalPermissions: mergedAdditional,
@@ -38,6 +38,7 @@ describe('WindowsSandboxManager', () => {
manager = new WindowsSandboxManager({
workspace: testCwd,
modeConfig: { readonly: false, allowOverrides: true },
forbiddenPaths: [],
});
});
@@ -106,6 +107,7 @@ describe('WindowsSandboxManager', () => {
const planManager = new WindowsSandboxManager({
workspace: testCwd,
modeConfig: { readonly: true, allowOverrides: false },
forbiddenPaths: [],
});
const req: SandboxRequest = {
command: 'curl',
@@ -135,6 +137,7 @@ describe('WindowsSandboxManager', () => {
workspace: testCwd,
modeConfig: { allowOverrides: true, network: false },
policyManager: mockPolicyManager,
forbiddenPaths: [],
});
const req: SandboxRequest = {
@@ -362,17 +365,19 @@ describe('WindowsSandboxManager', () => {
fs.rmSync(missingPath, { recursive: true, force: true });
}
const managerWithForbidden = new WindowsSandboxManager({
workspace: testCwd,
forbiddenPaths: [missingPath],
});
const req: SandboxRequest = {
command: 'test',
args: [],
cwd: testCwd,
env: {},
policy: {
forbiddenPaths: [missingPath],
},
};
await manager.prepareCommand(req);
await managerWithForbidden.prepareCommand(req);
// Should NOT have called icacls to deny the missing path
expect(spawnAsync).not.toHaveBeenCalledWith('icacls', [
@@ -388,17 +393,19 @@ describe('WindowsSandboxManager', () => {
fs.mkdirSync(forbiddenPath);
}
try {
const managerWithForbidden = new WindowsSandboxManager({
workspace: testCwd,
forbiddenPaths: [forbiddenPath],
});
const req: SandboxRequest = {
command: 'test',
args: [],
cwd: testCwd,
env: {},
policy: {
forbiddenPaths: [forbiddenPath],
},
};
await manager.prepareCommand(req);
await managerWithForbidden.prepareCommand(req);
expect(spawnAsync).toHaveBeenCalledWith('icacls', [
path.resolve(forbiddenPath),
@@ -416,6 +423,11 @@ describe('WindowsSandboxManager', () => {
fs.mkdirSync(conflictPath);
}
try {
const managerWithForbidden = new WindowsSandboxManager({
workspace: testCwd,
forbiddenPaths: [conflictPath],
});
const req: SandboxRequest = {
command: 'test',
args: [],
@@ -423,11 +435,10 @@ describe('WindowsSandboxManager', () => {
env: {},
policy: {
allowedPaths: [conflictPath],
forbiddenPaths: [conflictPath],
},
};
await manager.prepareCommand(req);
await managerWithForbidden.prepareCommand(req);
const spawnMock = vi.mocked(spawnAsync);
const allowCallIndex = spawnMock.mock.calls.findIndex(
@@ -33,24 +33,11 @@ import {
isDangerousCommand,
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);
export interface WindowsSandboxOptions extends GlobalSandboxOptions {
/** The current sandbox mode behavior from config. */
modeConfig?: {
readonly?: boolean;
network?: boolean;
approvedTools?: string[];
allowOverrides?: boolean;
};
/** The policy manager for persistent approvals. */
policyManager?: SandboxPolicyManager;
}
/**
* A SandboxManager implementation for Windows that uses Restricted Tokens,
* Job Objects, and Low Integrity levels for process isolation.
@@ -62,7 +49,7 @@ export class WindowsSandboxManager implements SandboxManager {
private readonly allowedCache = new Set<string>();
private readonly deniedCache = new Set<string>();
constructor(private readonly options: WindowsSandboxOptions) {
constructor(private readonly options: GlobalSandboxOptions) {
this.helperPath = path.resolve(__dirname, 'GeminiSandbox.exe');
}
@@ -309,7 +296,7 @@ export class WindowsSandboxManager implements SandboxManager {
// is restricted to avoid host corruption. External commands rely on
// Low Integrity read/write restrictions, while internal commands
// use the manifest for enforcement.
const forbiddenPaths = sanitizePaths(req.policy?.forbiddenPaths) || [];
const forbiddenPaths = sanitizePaths(this.options.forbiddenPaths) || [];
for (const forbiddenPath of forbiddenPaths) {
try {
await this.denyLowIntegrityAccess(forbiddenPath);
@@ -142,7 +142,7 @@ function ensureSandboxAvailable(): boolean {
describe('SandboxManager Integration', () => {
const workspace = process.cwd();
const manager = createSandboxManager({ enabled: true }, workspace);
const manager = createSandboxManager({ enabled: true }, { workspace });
// Skip if we are on an unsupported platform or if it's a NoopSandboxManager
const shouldSkip =
@@ -235,7 +235,7 @@ describe('SandboxManager Integration', () => {
try {
const osManager = createSandboxManager(
{ enabled: true },
tempWorkspace,
{ workspace: tempWorkspace, forbiddenPaths: [forbiddenDir] },
);
const { command, args } = Platform.touch(testFile);
@@ -244,7 +244,6 @@ describe('SandboxManager Integration', () => {
args,
cwd: tempWorkspace,
env: process.env,
policy: { forbiddenPaths: [forbiddenDir] },
});
const result = await runCommand(sandboxed);
@@ -268,7 +267,7 @@ describe('SandboxManager Integration', () => {
try {
const osManager = createSandboxManager(
{ enabled: true },
tempWorkspace,
{ workspace: tempWorkspace, forbiddenPaths: [forbiddenDir] },
);
const { command, args } = Platform.cat(nestedFile);
@@ -277,7 +276,6 @@ describe('SandboxManager Integration', () => {
args,
cwd: tempWorkspace,
env: process.env,
policy: { forbiddenPaths: [forbiddenDir] },
});
const result = await runCommand(sandboxed);
@@ -298,7 +296,7 @@ describe('SandboxManager Integration', () => {
try {
const osManager = createSandboxManager(
{ enabled: true },
tempWorkspace,
{ workspace: tempWorkspace, forbiddenPaths: [conflictDir] },
);
const { command, args } = Platform.touch(testFile);
@@ -309,7 +307,6 @@ describe('SandboxManager Integration', () => {
env: process.env,
policy: {
allowedPaths: [conflictDir],
forbiddenPaths: [conflictDir],
},
});
@@ -329,7 +326,7 @@ describe('SandboxManager Integration', () => {
try {
const osManager = createSandboxManager(
{ enabled: true },
tempWorkspace,
{ workspace: tempWorkspace, forbiddenPaths: [nonExistentPath] },
);
const { command, args } = Platform.echo('survived');
const sandboxed = await osManager.prepareCommand({
@@ -339,7 +336,6 @@ describe('SandboxManager Integration', () => {
env: process.env,
policy: {
allowedPaths: [nonExistentPath],
forbiddenPaths: [nonExistentPath],
},
});
const result = await runCommand(sandboxed);
@@ -362,7 +358,7 @@ describe('SandboxManager Integration', () => {
try {
const osManager = createSandboxManager(
{ enabled: true },
tempWorkspace,
{ workspace: tempWorkspace, forbiddenPaths: [nonExistentFile] },
);
// We use touch to attempt creation of the file
@@ -374,7 +370,6 @@ describe('SandboxManager Integration', () => {
args: argsTouch,
cwd: tempWorkspace,
env: process.env,
policy: { forbiddenPaths: [nonExistentFile] },
});
// Execute the command, we expect it to fail (permission denied or read-only file system)
@@ -402,7 +397,7 @@ describe('SandboxManager Integration', () => {
try {
const osManager = createSandboxManager(
{ enabled: true },
tempWorkspace,
{ workspace: tempWorkspace, forbiddenPaths: [symlinkFile] },
);
// Attempt to read the target file directly
@@ -413,7 +408,6 @@ describe('SandboxManager Integration', () => {
args: argsTarget,
cwd: tempWorkspace,
env: process.env,
policy: { forbiddenPaths: [symlinkFile] }, // Forbid the symlink
});
const resultTarget = await runCommand(commandTarget);
expect(resultTarget.status).not.toBe(0);
@@ -426,7 +420,6 @@ describe('SandboxManager Integration', () => {
args: argsLink,
cwd: tempWorkspace,
env: process.env,
policy: { forbiddenPaths: [symlinkFile] }, // Forbid the symlink
});
const resultLink = await runCommand(commandLink);
expect(resultLink.status).not.toBe(0);
@@ -364,7 +364,10 @@ describe('SandboxManager', () => {
describe('createSandboxManager', () => {
it('should return NoopSandboxManager if sandboxing is disabled', () => {
const manager = createSandboxManager({ enabled: false }, '/workspace');
const manager = createSandboxManager(
{ enabled: false },
{ workspace: '/workspace' },
);
expect(manager).toBeInstanceOf(NoopSandboxManager);
});
@@ -375,7 +378,10 @@ describe('SandboxManager', () => {
'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');
const manager = createSandboxManager(
{ enabled: true },
{ workspace: '/workspace' },
);
expect(manager).toBeInstanceOf(expected);
},
);
@@ -384,7 +390,7 @@ describe('SandboxManager', () => {
vi.spyOn(os, 'platform').mockReturnValue('win32');
const manager = createSandboxManager(
{ enabled: true, command: 'windows-native' },
'/workspace',
{ workspace: '/workspace' },
);
expect(manager).toBeInstanceOf(WindowsSandboxManager);
});
@@ -393,7 +399,7 @@ describe('SandboxManager', () => {
vi.spyOn(os, 'platform').mockReturnValue('win32');
const manager = createSandboxManager(
{ enabled: true, command: 'docker' as unknown as 'windows-native' },
'/workspace',
{ workspace: '/workspace' },
);
expect(manager).toBeInstanceOf(LocalSandboxManager);
});
+17 -2
View File
@@ -22,6 +22,7 @@ import {
type EnvironmentSanitizationConfig,
} from './environmentSanitization.js';
import type { ShellExecutionResult } from './shellExecutionService.js';
import type { SandboxPolicyManager } from '../policy/sandboxPolicyManager.js';
export interface SandboxPermissions {
/** Filesystem permissions. */
fileSystem?: {
@@ -40,8 +41,6 @@ export interface SandboxPermissions {
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. */
@@ -50,6 +49,16 @@ export interface ExecutionPolicy {
additionalPermissions?: SandboxPermissions;
}
/**
* Configuration for the sandbox mode behavior.
*/
export interface SandboxModeConfig {
readonly?: boolean;
network?: boolean;
approvedTools?: string[];
allowOverrides?: boolean;
}
/**
* Global configuration options used to initialize a SandboxManager.
*/
@@ -59,6 +68,12 @@ export interface GlobalSandboxOptions {
* This directory is granted full read and write access.
*/
workspace: string;
/** Absolute paths to explicitly deny read/write access to (overrides allowlists). */
forbiddenPaths?: string[];
/** The current sandbox mode behavior from config. */
modeConfig?: SandboxModeConfig;
/** The policy manager for persistent approvals. */
policyManager?: SandboxPolicyManager;
}
/**
@@ -9,50 +9,36 @@ import {
type SandboxManager,
NoopSandboxManager,
LocalSandboxManager,
type GlobalSandboxOptions,
} from './sandboxManager.js';
import { LinuxSandboxManager } from '../sandbox/linux/LinuxSandboxManager.js';
import { MacOsSandboxManager } from '../sandbox/macos/MacOsSandboxManager.js';
import { WindowsSandboxManager } from '../sandbox/windows/WindowsSandboxManager.js';
import type { SandboxConfig } from '../config/config.js';
import { type SandboxPolicyManager } from '../policy/sandboxPolicyManager.js';
/**
* Creates a sandbox manager based on the provided settings.
*/
export function createSandboxManager(
sandbox: SandboxConfig | undefined,
workspace: string,
policyManager?: SandboxPolicyManager,
options: GlobalSandboxOptions,
approvalMode?: string,
): SandboxManager {
if (approvalMode === 'yolo') {
return new NoopSandboxManager();
}
const modeConfig =
policyManager && approvalMode
? policyManager.getModeConfig(approvalMode)
: undefined;
if (!options.modeConfig && options.policyManager && approvalMode) {
options.modeConfig = options.policyManager.getModeConfig(approvalMode);
}
if (sandbox?.enabled) {
if (os.platform() === 'win32' && sandbox?.command === 'windows-native') {
return new WindowsSandboxManager({
workspace,
modeConfig,
policyManager,
});
return new WindowsSandboxManager(options);
} else if (os.platform() === 'linux') {
return new LinuxSandboxManager({
workspace,
modeConfig,
policyManager,
});
return new LinuxSandboxManager(options);
} else if (os.platform() === 'darwin') {
return new MacOsSandboxManager({
workspace,
modeConfig,
policyManager,
});
return new MacOsSandboxManager(options);
}
return new LocalSandboxManager();
}