fix: require trusted approval for sandbox override (#778)
This commit is contained in:
@@ -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<typeof inputSchema>;
|
||||
|
||||
@@ -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<string | string[]> =
|
||||
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<string | string[]> = [
|
||||
...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.',
|
||||
]
|
||||
|
||||
|
||||
74
src/tools/BashTool/shouldUseSandbox.test.ts
Normal file
74
src/tools/BashTool/shouldUseSandbox.test.ts
Normal file
@@ -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)
|
||||
})
|
||||
@@ -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<SandboxInput>): 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
|
||||
|
||||
@@ -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<typeof inputSchema>;
|
||||
|
||||
// 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
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user