From 554a5a36a32d52339de977bc33aceace5bc174d9 Mon Sep 17 00:00:00 2001 From: Gal Zahavi <38544478+galz10@users.noreply.github.com> Date: Tue, 31 Mar 2026 12:39:51 -0700 Subject: [PATCH] fix(core): implement __read and __write commands in sandbox managers (#24283) --- .../sandbox/linux/LinuxSandboxManager.test.ts | 4 +- .../src/sandbox/linux/LinuxSandboxManager.ts | 19 ++--- .../src/sandbox/macos/MacOsSandboxManager.ts | 10 ++- .../core/src/sandbox/utils/commandSafety.ts | 2 + .../sandbox/utils/sandboxReadWriteUtils.ts | 81 +++++++++++++++++++ .../sandbox/windows/WindowsSandboxManager.ts | 6 ++ .../core/src/sandbox/windows/commandSafety.ts | 2 + 7 files changed, 112 insertions(+), 12 deletions(-) create mode 100644 packages/core/src/sandbox/utils/sandboxReadWriteUtils.ts diff --git a/packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts b/packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts index c4551b1043..eb9d285467 100644 --- a/packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts +++ b/packages/core/src/sandbox/linux/LinuxSandboxManager.test.ts @@ -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 () => { diff --git a/packages/core/src/sandbox/linux/LinuxSandboxManager.ts b/packages/core/src/sandbox/linux/LinuxSandboxManager.ts index 2167e28740..ce4c1c589d 100644 --- a/packages/core/src/sandbox/linux/LinuxSandboxManager.ts +++ b/packages/core/src/sandbox/linux/LinuxSandboxManager.ts @@ -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', diff --git a/packages/core/src/sandbox/macos/MacOsSandboxManager.ts b/packages/core/src/sandbox/macos/MacOsSandboxManager.ts index e3a6a7216a..e6f8fc987d 100644 --- a/packages/core/src/sandbox/macos/MacOsSandboxManager.ts +++ b/packages/core/src/sandbox/macos/MacOsSandboxManager.ts @@ -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: () => { diff --git a/packages/core/src/sandbox/utils/commandSafety.ts b/packages/core/src/sandbox/utils/commandSafety.ts index c57f77512b..180d0748d2 100644 --- a/packages/core/src/sandbox/utils/commandSafety.ts +++ b/packages/core/src/sandbox/utils/commandSafety.ts @@ -130,6 +130,8 @@ function isSafeToCallWithExec(args: string[]): boolean { const cmd = args[0]; const safeCommands = new Set([ + '__read', + '__write', 'cat', 'cd', 'cut', diff --git a/packages/core/src/sandbox/utils/sandboxReadWriteUtils.ts b/packages/core/src/sandbox/utils/sandboxReadWriteUtils.ts new file mode 100644 index 0000000000..21f8c1f7c3 --- /dev/null +++ b/packages/core/src/sandbox/utils/sandboxReadWriteUtils.ts @@ -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 }; +} diff --git a/packages/core/src/sandbox/windows/WindowsSandboxManager.ts b/packages/core/src/sandbox/windows/WindowsSandboxManager.ts index 16d952ea1b..5c5080409c 100644 --- a/packages/core/src/sandbox/windows/WindowsSandboxManager.ts +++ b/packages/core/src/sandbox/windows/WindowsSandboxManager.ts @@ -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; diff --git a/packages/core/src/sandbox/windows/commandSafety.ts b/packages/core/src/sandbox/windows/commandSafety.ts index bff2976e62..b2cc842df9 100644 --- a/packages/core/src/sandbox/windows/commandSafety.ts +++ b/packages/core/src/sandbox/windows/commandSafety.ts @@ -72,6 +72,8 @@ export function isKnownSafeCommand(args: string[]): boolean { // Native Windows/PowerShell safe commands const safeCommands = new Set([ + '__read', + '__write', 'dir', 'type', 'echo',