From aab489055c53dd64369414116fe93226d2656273 Mon Sep 17 00:00:00 2001 From: Kevin Codex Date: Mon, 20 Apr 2026 12:01:44 +0800 Subject: [PATCH] fix: require trusted approval for sandbox override (#778) --- src/entrypoints/sandboxTypes.ts | 4 +- src/tools/BashTool/BashTool.tsx | 15 +++- src/tools/BashTool/prompt.ts | 36 +-------- src/tools/BashTool/shouldUseSandbox.test.ts | 74 +++++++++++++++++++ src/tools/BashTool/shouldUseSandbox.ts | 7 +- src/tools/PowerShellTool/PowerShellTool.tsx | 21 ++++-- src/utils/api.ts | 4 - .../processUserInput/processBashCommand.tsx | 15 ++-- 8 files changed, 119 insertions(+), 57 deletions(-) create mode 100644 src/tools/BashTool/shouldUseSandbox.test.ts diff --git a/src/entrypoints/sandboxTypes.ts b/src/entrypoints/sandboxTypes.ts index b38a156e..9f117cf0 100644 --- a/src/entrypoints/sandboxTypes.ts +++ b/src/entrypoints/sandboxTypes.ts @@ -114,8 +114,8 @@ export const SandboxSettingsSchema = lazySchema(() => .boolean() .optional() .describe( - 'Allow commands to run outside the sandbox via the dangerouslyDisableSandbox parameter. ' + - 'When false, the dangerouslyDisableSandbox parameter is completely ignored and all commands must run sandboxed. ' + + 'Allow trusted, user-initiated commands to run outside the sandbox. ' + + 'When false, sandbox override requests are ignored and all commands must run sandboxed. ' + 'Default: true.', ), network: SandboxNetworkConfigSchema(), diff --git a/src/tools/BashTool/BashTool.tsx b/src/tools/BashTool/BashTool.tsx index 4ad8bcf3..8b0a997e 100644 --- a/src/tools/BashTool/BashTool.tsx +++ b/src/tools/BashTool/BashTool.tsx @@ -240,21 +240,28 @@ For commands that are harder to parse at a glance (piped commands, obscure flags - curl -s url | jq '.data[]' → "Fetch JSON from URL and extract data array elements"`), run_in_background: semanticBoolean(z.boolean().optional()).describe(`Set to true to run this command in the background. Use Read to read the output later.`), dangerouslyDisableSandbox: semanticBoolean(z.boolean().optional()).describe('Set this to true to dangerously override sandbox mode and run commands without sandboxing.'), + _dangerouslyDisableSandboxApproved: z.boolean().optional().describe('Internal: user-approved sandbox override'), _simulatedSedEdit: z.object({ filePath: z.string(), newContent: z.string() }).optional().describe('Internal: pre-computed sed edit result from preview') })); -// Always omit _simulatedSedEdit from the model-facing schema. It is an internal-only -// field set by SedEditPermissionRequest after the user approves a sed edit preview. -// Exposing it in the schema would let the model bypass permission checks and the -// sandbox by pairing an innocuous command with an arbitrary file write. +// Always omit internal-only fields from the model-facing schema. +// _simulatedSedEdit is set by SedEditPermissionRequest after the user approves a +// sed edit preview; exposing it would let the model bypass permission checks and +// the sandbox by pairing an innocuous command with an arbitrary file write. +// dangerouslyDisableSandbox is also omitted because sandbox escape must be tied +// to trusted user/internal provenance, not model-controlled tool input. // Also conditionally remove run_in_background when background tasks are disabled. const inputSchema = lazySchema(() => isBackgroundTasksDisabled ? fullInputSchema().omit({ run_in_background: true, + dangerouslyDisableSandbox: true, + _dangerouslyDisableSandboxApproved: true, _simulatedSedEdit: true }) : fullInputSchema().omit({ + dangerouslyDisableSandbox: true, + _dangerouslyDisableSandboxApproved: true, _simulatedSedEdit: true })); type InputSchema = ReturnType; diff --git a/src/tools/BashTool/prompt.ts b/src/tools/BashTool/prompt.ts index fb865908..6f73310f 100644 --- a/src/tools/BashTool/prompt.ts +++ b/src/tools/BashTool/prompt.ts @@ -179,9 +179,6 @@ function getSimpleSandboxSection(): string { const networkRestrictionConfig = SandboxManager.getNetworkRestrictionConfig() const allowUnixSockets = SandboxManager.getAllowUnixSockets() const ignoreViolations = SandboxManager.getIgnoreViolations() - const allowUnsandboxedCommands = - SandboxManager.areUnsandboxedCommandsAllowed() - // Replace the per-UID temp dir literal (e.g. /private/tmp/claude-1001/) with // "$TMPDIR" so the prompt is identical across users — avoids busting the // cross-user global prompt cache. The sandbox already sets $TMPDIR at runtime. @@ -225,38 +222,9 @@ function getSimpleSandboxSection(): string { ) } - const sandboxOverrideItems: Array = - allowUnsandboxedCommands - ? [ - 'You should always default to running commands within the sandbox. Do NOT attempt to set `dangerouslyDisableSandbox: true` unless:', - [ - 'The user *explicitly* asks you to bypass sandbox', - 'A specific command just failed and you see evidence of sandbox restrictions causing the failure. Note that commands can fail for many reasons unrelated to the sandbox (missing files, wrong arguments, network issues, etc.).', - ], - 'Evidence of sandbox-caused failures includes:', - [ - '"Operation not permitted" errors for file/network operations', - 'Access denied to specific paths outside allowed directories', - 'Network connection failures to non-whitelisted hosts', - 'Unix socket connection errors', - ], - 'When you see evidence of sandbox-caused failure:', - [ - "Immediately retry with `dangerouslyDisableSandbox: true` (don't ask, just do it)", - 'Briefly explain what sandbox restriction likely caused the failure. Be sure to mention that the user can use the `/sandbox` command to manage restrictions.', - 'This will prompt the user for permission', - ], - 'Treat each command you execute with `dangerouslyDisableSandbox: true` individually. Even if you have recently run a command with this setting, you should default to running future commands within the sandbox.', - 'Do not suggest adding sensitive paths like ~/.bashrc, ~/.zshrc, ~/.ssh/*, or credential files to the sandbox allowlist.', - ] - : [ - 'All commands MUST run in sandbox mode - the `dangerouslyDisableSandbox` parameter is disabled by policy.', - 'Commands cannot run outside the sandbox under any circumstances.', - 'If a command fails due to sandbox restrictions, work with the user to adjust sandbox settings instead.', - ] - const items: Array = [ - ...sandboxOverrideItems, + 'Commands MUST run in sandbox mode. If a command fails due to sandbox restrictions, explain the likely restriction and work with the user to adjust sandbox settings or run an explicit user-initiated shell command.', + 'Do not suggest adding sensitive paths like ~/.bashrc, ~/.zshrc, ~/.ssh/*, or credential files to the sandbox allowlist.', 'For temporary files, always use the `$TMPDIR` environment variable. TMPDIR is automatically set to the correct sandbox-writable directory in sandbox mode. Do NOT use `/tmp` directly - use `$TMPDIR` instead.', ] diff --git a/src/tools/BashTool/shouldUseSandbox.test.ts b/src/tools/BashTool/shouldUseSandbox.test.ts new file mode 100644 index 00000000..f2b9f0dc --- /dev/null +++ b/src/tools/BashTool/shouldUseSandbox.test.ts @@ -0,0 +1,74 @@ +import { afterEach, expect, test } from 'bun:test' + +import { SandboxManager } from '../../utils/sandbox/sandbox-adapter.js' +import { BashTool } from './BashTool.js' +import { PowerShellTool } from '../PowerShellTool/PowerShellTool.js' +import { shouldUseSandbox } from './shouldUseSandbox.js' + +const originalSandboxMethods = { + isSandboxingEnabled: SandboxManager.isSandboxingEnabled, + areUnsandboxedCommandsAllowed: SandboxManager.areUnsandboxedCommandsAllowed, +} + +afterEach(() => { + SandboxManager.isSandboxingEnabled = + originalSandboxMethods.isSandboxingEnabled + SandboxManager.areUnsandboxedCommandsAllowed = + originalSandboxMethods.areUnsandboxedCommandsAllowed +}) + +test('model-facing Bash schema rejects dangerouslyDisableSandbox', () => { + const result = BashTool.inputSchema.safeParse({ + command: 'cat /etc/passwd', + dangerouslyDisableSandbox: true, + }) + + expect(result.success).toBe(false) +}) + +test('model-facing PowerShell schema rejects dangerouslyDisableSandbox', () => { + const result = PowerShellTool.inputSchema.safeParse({ + command: 'Get-Content C:\\Windows\\System32\\drivers\\etc\\hosts', + dangerouslyDisableSandbox: true, + }) + + expect(result.success).toBe(false) +}) + +test('model-controlled dangerouslyDisableSandbox does not bypass sandbox', () => { + SandboxManager.isSandboxingEnabled = () => true + SandboxManager.areUnsandboxedCommandsAllowed = () => true + + expect( + shouldUseSandbox({ + command: 'cat /etc/passwd', + dangerouslyDisableSandbox: true, + }), + ).toBe(true) +}) + +test('trusted internal approval can disable sandbox when policy allows it', () => { + SandboxManager.isSandboxingEnabled = () => true + SandboxManager.areUnsandboxedCommandsAllowed = () => true + + expect( + shouldUseSandbox({ + command: 'cat /etc/passwd', + dangerouslyDisableSandbox: true, + _dangerouslyDisableSandboxApproved: true, + }), + ).toBe(false) +}) + +test('trusted internal approval cannot disable sandbox when policy forbids it', () => { + SandboxManager.isSandboxingEnabled = () => true + SandboxManager.areUnsandboxedCommandsAllowed = () => false + + expect( + shouldUseSandbox({ + command: 'cat /etc/passwd', + dangerouslyDisableSandbox: true, + _dangerouslyDisableSandboxApproved: true, + }), + ).toBe(true) +}) diff --git a/src/tools/BashTool/shouldUseSandbox.ts b/src/tools/BashTool/shouldUseSandbox.ts index 486e3678..eedb6071 100644 --- a/src/tools/BashTool/shouldUseSandbox.ts +++ b/src/tools/BashTool/shouldUseSandbox.ts @@ -13,6 +13,7 @@ import { type SandboxInput = { command?: string dangerouslyDisableSandbox?: boolean + _dangerouslyDisableSandboxApproved?: boolean } // NOTE: excludedCommands is a user-facing convenience feature, not a security boundary. @@ -141,9 +142,13 @@ export function shouldUseSandbox(input: Partial): boolean { return false } - // Don't sandbox if explicitly overridden AND unsandboxed commands are allowed by policy + // Only trusted internal callers may request an unsandboxed command. The + // model-facing Bash schema omits _dangerouslyDisableSandboxApproved, so a + // tool_use payload cannot disable the sandbox by setting + // dangerouslyDisableSandbox directly. if ( input.dangerouslyDisableSandbox && + input._dangerouslyDisableSandboxApproved && SandboxManager.areUnsandboxedCommandsAllowed() ) { return false diff --git a/src/tools/PowerShellTool/PowerShellTool.tsx b/src/tools/PowerShellTool/PowerShellTool.tsx index 09bccfea..52f40c48 100644 --- a/src/tools/PowerShellTool/PowerShellTool.tsx +++ b/src/tools/PowerShellTool/PowerShellTool.tsx @@ -230,13 +230,20 @@ const fullInputSchema = lazySchema(() => z.strictObject({ timeout: semanticNumber(z.number().optional()).describe(`Optional timeout in milliseconds (max ${getMaxTimeoutMs()})`), description: z.string().optional().describe('Clear, concise description of what this command does in active voice.'), run_in_background: semanticBoolean(z.boolean().optional()).describe(`Set to true to run this command in the background. Use Read to read the output later.`), - dangerouslyDisableSandbox: semanticBoolean(z.boolean().optional()).describe('Set this to true to dangerously override sandbox mode and run commands without sandboxing.') + dangerouslyDisableSandbox: semanticBoolean(z.boolean().optional()).describe('Set this to true to dangerously override sandbox mode and run commands without sandboxing.'), + _dangerouslyDisableSandboxApproved: z.boolean().optional().describe('Internal: user-approved sandbox override') })); -// Conditionally remove run_in_background from schema when background tasks are disabled +// Omit internal-only sandbox override fields from the model-facing schema. +// Conditionally remove run_in_background from schema when background tasks are disabled. const inputSchema = lazySchema(() => isBackgroundTasksDisabled ? fullInputSchema().omit({ - run_in_background: true -}) : fullInputSchema()); + run_in_background: true, + dangerouslyDisableSandbox: true, + _dangerouslyDisableSandboxApproved: true +}) : fullInputSchema().omit({ + dangerouslyDisableSandbox: true, + _dangerouslyDisableSandboxApproved: true +})); type InputSchema = ReturnType; // Use fullInputSchema for the type to always include run_in_background @@ -697,7 +704,8 @@ async function* runPowerShellCommand({ description, timeout, run_in_background, - dangerouslyDisableSandbox + dangerouslyDisableSandbox, + _dangerouslyDisableSandboxApproved } = input; const timeoutMs = Math.min(timeout || getDefaultTimeoutMs(), getMaxTimeoutMs()); let fullOutput = ''; @@ -749,7 +757,8 @@ async function* runPowerShellCommand({ // The explicit platform check is redundant-but-obvious. shouldUseSandbox: getPlatform() === 'windows' ? false : shouldUseSandbox({ command, - dangerouslyDisableSandbox + dangerouslyDisableSandbox, + _dangerouslyDisableSandboxApproved }), shouldAutoBackground }); diff --git a/src/utils/api.ts b/src/utils/api.ts index 616b1544..324e48e5 100644 --- a/src/utils/api.ts +++ b/src/utils/api.ts @@ -662,10 +662,6 @@ export function normalizeToolInput( ...(timeout !== undefined && { timeout }), ...(description !== undefined && { description }), ...(run_in_background !== undefined && { run_in_background }), - ...('dangerouslyDisableSandbox' in parsed && - parsed.dangerouslyDisableSandbox !== undefined && { - dangerouslyDisableSandbox: parsed.dangerouslyDisableSandbox, - }), } as z.infer } case FileEditTool.name: { diff --git a/src/utils/processUserInput/processBashCommand.tsx b/src/utils/processUserInput/processBashCommand.tsx index 0ea5b891..14a3e840 100644 --- a/src/utils/processUserInput/processBashCommand.tsx +++ b/src/utils/processUserInput/processBashCommand.tsx @@ -65,10 +65,11 @@ export async function processBashCommand(inputString: string, precedingInputBloc }); }; - // User-initiated `!` commands run outside sandbox. Both shell tools honor - // dangerouslyDisableSandbox (checked against areUnsandboxedCommandsAllowed() - // in shouldUseSandbox.ts). PS sandbox is Linux/macOS/WSL2 only — on Windows - // native, shouldUseSandbox() returns false regardless (unsupported platform). + // User-initiated `!` commands run outside sandbox when policy allows it. + // Bash requires an internal approval marker so model-controlled tool input + // cannot disable sandboxing by setting dangerouslyDisableSandbox directly. + // PS sandbox is Linux/macOS/WSL2 only — on Windows native, shouldUseSandbox() + // returns false regardless (unsupported platform). // Lazy-require PowerShellTool so its ~300KB chunk only loads when the // user has actually selected the powershell default shell. type PSMod = typeof import('src/tools/PowerShellTool/PowerShellTool.js'); @@ -81,10 +82,12 @@ export async function processBashCommand(inputString: string, precedingInputBloc const shellTool = PowerShellTool ?? BashTool; const response = PowerShellTool ? await PowerShellTool.call({ command: inputString, - dangerouslyDisableSandbox: true + dangerouslyDisableSandbox: true, + _dangerouslyDisableSandboxApproved: true }, bashModeContext, undefined, undefined, onProgress) : await BashTool.call({ command: inputString, - dangerouslyDisableSandbox: true + dangerouslyDisableSandbox: true, + _dangerouslyDisableSandboxApproved: true }, bashModeContext, undefined, undefined, onProgress); const data = response.data; if (!data) {