fix(core): implement __read and __write commands in sandbox managers (#24283)

This commit is contained in:
Gal Zahavi
2026-03-31 12:39:51 -07:00
committed by GitHub
parent 7e117ac0ac
commit 554a5a36a3
7 changed files with 112 additions and 12 deletions

View File

@@ -317,7 +317,7 @@ describe('LinuxSandboxManager', () => {
);
});
it('should not grant read-write access to allowedPaths inside the workspace when readonly mode is active', async () => {
it('should grant read-write access to allowedPaths inside the workspace even when readonly mode is active', async () => {
const manager = new LinuxSandboxManager({
workspace,
modeConfig: { readonly: true },
@@ -333,7 +333,7 @@ describe('LinuxSandboxManager', () => {
});
const bwrapArgs = result.args;
const bindIndex = bwrapArgs.indexOf(workspace + '/subdirectory');
expect(bwrapArgs[bindIndex - 1]).toBe('--ro-bind-try');
expect(bwrapArgs[bindIndex - 1]).toBe('--bind-try');
});
it('should not bind the workspace twice even if it has a trailing slash in allowedPaths', async () => {

View File

@@ -40,6 +40,7 @@ import {
isDangerousCommand,
} from '../utils/commandSafety.js';
import { parsePosixSandboxDenials } from '../utils/sandboxDenialUtils.js';
import { handleReadWriteCommands } from '../utils/sandboxReadWriteUtils.js';
let cachedBpfPath: string | undefined;
@@ -211,6 +212,13 @@ export class LinuxSandboxManager implements SandboxManager {
false,
};
const { command: finalCommand, args: finalArgs } = handleReadWriteCommands(
req,
mergedAdditional,
this.options.workspace,
req.policy?.allowedPaths,
);
const sanitizationConfig = getSecureSanitizationConfig(
req.policy?.sanitizationConfig,
);
@@ -279,14 +287,7 @@ export class LinuxSandboxManager implements SandboxManager {
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);
}
bwrapArgs.push('--bind-try', resolved, resolved);
}
}
@@ -362,7 +363,7 @@ export class LinuxSandboxManager implements SandboxManager {
const bpfPath = getSeccompBpfPath();
bwrapArgs.push('--seccomp', '9');
bwrapArgs.push('--', req.command, ...req.args);
bwrapArgs.push('--', finalCommand, ...finalArgs);
const shArgs = [
'-c',

View File

@@ -32,6 +32,7 @@ import {
} from '../utils/commandSafety.js';
import { verifySandboxOverrides } from '../utils/commandUtils.js';
import { parsePosixSandboxDenials } from '../utils/sandboxDenialUtils.js';
import { handleReadWriteCommands } from '../utils/sandboxReadWriteUtils.js';
export class MacOsSandboxManager implements SandboxManager {
constructor(private readonly options: GlobalSandboxOptions) {}
@@ -105,6 +106,13 @@ export class MacOsSandboxManager implements SandboxManager {
false,
};
const { command: finalCommand, args: finalArgs } = handleReadWriteCommands(
req,
mergedAdditional,
this.options.workspace,
req.policy?.allowedPaths,
);
const sandboxArgs = buildSeatbeltProfile({
workspace: this.options.workspace,
allowedPaths: [...(req.policy?.allowedPaths || [])],
@@ -118,7 +126,7 @@ export class MacOsSandboxManager implements SandboxManager {
return {
program: '/usr/bin/sandbox-exec',
args: ['-f', tempFile, '--', req.command, ...req.args],
args: ['-f', tempFile, '--', finalCommand, ...finalArgs],
env: sanitizedEnv,
cwd: req.cwd,
cleanup: () => {

View File

@@ -130,6 +130,8 @@ function isSafeToCallWithExec(args: string[]): boolean {
const cmd = args[0];
const safeCommands = new Set([
'__read',
'__write',
'cat',
'cd',
'cut',

View File

@@ -0,0 +1,81 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import * as path from 'node:path';
import {
type SandboxPermissions,
type SandboxRequest,
} from '../../services/sandboxManager.js';
/**
* Validates if the requested paths are within the allowed workspace or allowed paths.
*/
function validatePaths(
paths: string[],
workspace: string,
allowedPaths: string[],
): boolean {
for (const p of paths) {
const resolvedPath = path.resolve(p);
const resolvedWorkspace = path.resolve(workspace);
const isInsideWorkspace =
resolvedPath.startsWith(resolvedWorkspace + path.sep) ||
resolvedPath === resolvedWorkspace;
let isInsideAllowed = false;
for (const allowed of allowedPaths) {
const resolvedAllowed = path.resolve(allowed);
if (
resolvedPath.startsWith(resolvedAllowed + path.sep) ||
resolvedPath === resolvedAllowed
) {
isInsideAllowed = true;
break;
}
}
if (!isInsideWorkspace && !isInsideAllowed) {
return false; // Path traversal or unauthorized access attempt
}
}
return true;
}
export function handleReadWriteCommands(
req: SandboxRequest,
mergedAdditional: SandboxPermissions,
workspace: string,
allowedPaths: string[] = [],
): { command: string; args: string[] } {
let finalCommand = req.command;
let finalArgs = req.args;
if (req.command === '__read') {
finalCommand = '/bin/cat';
if (req.args.length > 0) {
if (validatePaths(req.args, workspace, allowedPaths)) {
mergedAdditional.fileSystem!.read!.push(...req.args);
} else {
throw new Error(
`Sandbox Error: Path traversal or unauthorized access attempt detected in __read: ${req.args.join(', ')}`,
);
}
}
} else if (req.command === '__write') {
finalCommand = '/bin/sh';
finalArgs = ['-c', 'tee -- "$@" > /dev/null', '_', ...req.args];
if (req.args.length > 0) {
if (validatePaths(req.args, workspace, allowedPaths)) {
mergedAdditional.fileSystem!.write!.push(...req.args);
} else {
throw new Error(
`Sandbox Error: Path traversal or unauthorized access attempt detected in __write: ${req.args.join(', ')}`,
);
}
}
}
return { command: finalCommand, args: finalArgs };
}

View File

@@ -236,6 +236,12 @@ export class WindowsSandboxManager implements SandboxManager {
false,
};
if (req.command === '__read' && req.args[0]) {
mergedAdditional.fileSystem!.read!.push(req.args[0]);
} else if (req.command === '__write' && req.args[0]) {
mergedAdditional.fileSystem!.write!.push(req.args[0]);
}
const defaultNetwork =
this.options.modeConfig?.network || req.policy?.networkAccess || false;
const networkAccess = defaultNetwork || mergedAdditional.network;

View File

@@ -72,6 +72,8 @@ export function isKnownSafeCommand(args: string[]): boolean {
// Native Windows/PowerShell safe commands
const safeCommands = new Set([
'__read',
'__write',
'dir',
'type',
'echo',