refactor(core): standardize OS-specific sandbox tests and extract linux helper methods (#23715)

This commit is contained in:
Emily Hedlund
2026-03-24 22:37:32 -04:00
committed by GitHub
parent a6c7affedb
commit 5b7f7b30a7
6 changed files with 967 additions and 687 deletions
@@ -95,272 +95,343 @@ describe('LinuxSandboxManager', () => {
expect(dynamicBinds).toEqual(expectedDynamicBinds);
};
it('correctly outputs bwrap as the program with appropriate isolation flags', async () => {
const bwrapArgs = await getBwrapArgs({
command: 'ls',
args: ['-la'],
cwd: workspace,
env: {},
describe('prepareCommand', () => {
it('should correctly format the base command and args', async () => {
const bwrapArgs = await getBwrapArgs({
command: 'ls',
args: ['-la'],
cwd: workspace,
env: {},
});
expect(bwrapArgs).toEqual([
'--unshare-all',
'--new-session',
'--die-with-parent',
'--ro-bind',
'/',
'/',
'--dev',
'/dev',
'--proc',
'/proc',
'--tmpfs',
'/tmp',
'--bind',
workspace,
workspace,
'--ro-bind',
`${workspace}/.gitignore`,
`${workspace}/.gitignore`,
'--ro-bind',
`${workspace}/.geminiignore`,
`${workspace}/.geminiignore`,
'--ro-bind',
`${workspace}/.git`,
`${workspace}/.git`,
'--seccomp',
'9',
'--',
'ls',
'-la',
]);
});
expect(bwrapArgs).toEqual([
'--unshare-all',
'--new-session',
'--die-with-parent',
'--ro-bind',
'/',
'/',
'--dev',
'/dev',
'--proc',
'/proc',
'--tmpfs',
'/tmp',
'--bind',
workspace,
workspace,
'--ro-bind',
`${workspace}/.gitignore`,
`${workspace}/.gitignore`,
'--ro-bind',
`${workspace}/.geminiignore`,
`${workspace}/.geminiignore`,
'--ro-bind',
`${workspace}/.git`,
`${workspace}/.git`,
'--seccomp',
'9',
'--',
'ls',
'-la',
]);
});
it('should correctly pass through the cwd to the resulting command', async () => {
const req: SandboxRequest = {
command: 'ls',
args: [],
cwd: '/different/cwd',
env: {},
};
it('maps allowedPaths to bwrap binds', async () => {
const bwrapArgs = await getBwrapArgs({
command: 'node',
args: ['script.js'],
cwd: workspace,
env: {},
policy: {
allowedPaths: ['/tmp/cache', '/opt/tools', workspace],
},
const result = await manager.prepareCommand(req);
expect(result.cwd).toBe('/different/cwd');
});
// Verify the specific bindings were added correctly
expectDynamicBinds(bwrapArgs, [
'--bind-try',
'/tmp/cache',
'/tmp/cache',
'--bind-try',
'/opt/tools',
'/opt/tools',
]);
});
it('should apply environment sanitization via the default mechanisms', async () => {
const req: SandboxRequest = {
command: 'test',
args: [],
cwd: workspace,
env: {
API_KEY: 'secret',
PATH: '/usr/bin',
},
policy: {
sanitizationConfig: {
allowedEnvironmentVariables: ['PATH'],
blockedEnvironmentVariables: ['API_KEY'],
enableEnvironmentVariableRedaction: true,
},
},
};
it('protects real paths of governance files if they are symlinks', async () => {
vi.mocked(fs.realpathSync).mockImplementation((p) => {
if (p.toString() === `${workspace}/.gitignore`)
return '/shared/global.gitignore';
return p.toString();
const result = await manager.prepareCommand(req);
expect(result.env['PATH']).toBe('/usr/bin');
expect(result.env['API_KEY']).toBeUndefined();
});
const bwrapArgs = await getBwrapArgs({
command: 'ls',
args: [],
cwd: workspace,
env: {},
it('should allow network when networkAccess is true', async () => {
const bwrapArgs = await getBwrapArgs({
command: 'ls',
args: ['-la'],
cwd: workspace,
env: {},
policy: {
networkAccess: true,
},
});
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('--ro-bind');
expect(bwrapArgs).toContain(`${workspace}/.gitignore`);
expect(bwrapArgs).toContain('/shared/global.gitignore');
describe('governance files', () => {
it('should ensure governance files exist', async () => {
vi.mocked(fs.existsSync).mockReturnValue(false);
// Check that both are bound
const gitignoreIndex = bwrapArgs.indexOf(`${workspace}/.gitignore`);
expect(bwrapArgs[gitignoreIndex - 1]).toBe('--ro-bind');
expect(bwrapArgs[gitignoreIndex + 1]).toBe(`${workspace}/.gitignore`);
await getBwrapArgs({
command: 'ls',
args: [],
cwd: workspace,
env: {},
});
const realGitignoreIndex = bwrapArgs.indexOf('/shared/global.gitignore');
expect(bwrapArgs[realGitignoreIndex - 1]).toBe('--ro-bind');
expect(bwrapArgs[realGitignoreIndex + 1]).toBe('/shared/global.gitignore');
});
expect(fs.mkdirSync).toHaveBeenCalled();
expect(fs.openSync).toHaveBeenCalled();
});
it('touches governance files if they do not exist', async () => {
vi.mocked(fs.existsSync).mockReturnValue(false);
it('should protect both the symlink and the real path if they differ', async () => {
vi.mocked(fs.realpathSync).mockImplementation((p) => {
if (p.toString() === `${workspace}/.gitignore`)
return '/shared/global.gitignore';
return p.toString();
});
await getBwrapArgs({
command: 'ls',
args: [],
cwd: workspace,
env: {},
const bwrapArgs = await getBwrapArgs({
command: 'ls',
args: [],
cwd: workspace,
env: {},
});
expect(bwrapArgs).toContain('--ro-bind');
expect(bwrapArgs).toContain(`${workspace}/.gitignore`);
expect(bwrapArgs).toContain('/shared/global.gitignore');
// Check that both are bound
const gitignoreIndex = bwrapArgs.indexOf(`${workspace}/.gitignore`);
expect(bwrapArgs[gitignoreIndex - 1]).toBe('--ro-bind');
expect(bwrapArgs[gitignoreIndex + 1]).toBe(`${workspace}/.gitignore`);
const realGitignoreIndex = bwrapArgs.indexOf(
'/shared/global.gitignore',
);
expect(bwrapArgs[realGitignoreIndex - 1]).toBe('--ro-bind');
expect(bwrapArgs[realGitignoreIndex + 1]).toBe(
'/shared/global.gitignore',
);
});
});
expect(fs.mkdirSync).toHaveBeenCalled();
expect(fs.openSync).toHaveBeenCalled();
});
describe('allowedPaths', () => {
it('should parameterize allowed paths and normalize them', async () => {
const bwrapArgs = await getBwrapArgs({
command: 'node',
args: ['script.js'],
cwd: workspace,
env: {},
policy: {
allowedPaths: ['/tmp/cache', '/opt/tools', workspace],
},
});
it('should not bind the workspace twice even if it has a trailing slash in allowedPaths', async () => {
const bwrapArgs = await getBwrapArgs({
command: 'ls',
args: ['-la'],
cwd: workspace,
env: {},
policy: {
allowedPaths: [workspace + '/'],
},
// Verify the specific bindings were added correctly
expectDynamicBinds(bwrapArgs, [
'--bind-try',
'/tmp/cache',
'/tmp/cache',
'--bind-try',
'/opt/tools',
'/opt/tools',
]);
});
it('should not bind the workspace twice even if it has a trailing slash in allowedPaths', async () => {
const bwrapArgs = await getBwrapArgs({
command: 'ls',
args: ['-la'],
cwd: workspace,
env: {},
policy: {
allowedPaths: [workspace + '/'],
},
});
// Should only contain the primary workspace bind and governance files, not the second workspace bind with a trailing slash
expectDynamicBinds(bwrapArgs, []);
});
});
// Should only contain the primary workspace bind and governance files, not the second workspace bind with a trailing slash
expectDynamicBinds(bwrapArgs, []);
});
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
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(),
);
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;
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('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();
},
);
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('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(),
);
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',
]);
});
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(),
);
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',
]);
});
});
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',
]);
});
});
@@ -113,78 +113,13 @@ export class LinuxSandboxManager implements SandboxManager {
const sanitizedEnv = sanitizeEnvironment(req.env, sanitizationConfig);
const bwrapArgs: string[] = [
...(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',
'/',
'/',
'--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,
...this.getNetworkArgs(req),
...this.getBaseArgs(),
...this.getGovernanceArgs(),
...this.getAllowedPathsArgs(req.policy?.allowedPaths),
...(await this.getForbiddenPathsArgs(req.policy?.forbiddenPaths)),
];
// 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);
bwrapArgs.push('--ro-bind', filePath, filePath);
if (realPath !== filePath) {
bwrapArgs.push('--ro-bind', realPath, realPath);
}
}
const allowedPaths = sanitizePaths(req.policy?.allowedPaths) || [];
const normalizedWorkspace = this.normalizePath(this.options.workspace);
for (const p of allowedPaths) {
if (this.normalizePath(p) !== normalizedWorkspace) {
bwrapArgs.push('--bind-try', p, p);
}
}
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();
bwrapArgs.push('--seccomp', '9');
@@ -202,29 +137,139 @@ export class LinuxSandboxManager implements SandboxManager {
program: 'sh',
args: shArgs,
env: sanitizedEnv,
cwd: req.cwd,
};
}
/**
* Applies bubblewrap arguments to mask a forbidden path.
* Generates arguments for network isolation.
*/
private async applyMasking(args: string[], path: string) {
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<string[]> {
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<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);
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.
args.push('--symlink', '/.forbidden', path);
return;
return ['--symlink', '/.forbidden', path];
}
throw e;
}