From 34af6f558d6698ef652bff4108e5cd0c80c5fe0f Mon Sep 17 00:00:00 2001 From: t Date: Mon, 3 Aug 2026 12:31:22 +0800 Subject: [PATCH] feat(sandbox): make it a real axis, and turn it on by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Findings F5/F6 in docs/THREE_WAY_REVIEW.md: the sandbox was off unless configured, and a single `mode` expressed both what a command may touch and how it gets approved. Codex separates those; the alignment plan §5.5 says to, and hadn't. The axis. `sandbox.mode` in settings and `--sandbox` on the CLI take read-only / workspace-write / danger-full-access, orthogonal to `--mode`. The legacy `enabled` boolean still works (true → workspace-write, false → danger-full-access) and `mode` wins when both are set. Every host now defaults to workspace-write via RuntimeHost; library callers of wrapBashCommand keep the old "off unless configured" behaviour unless they pass defaultMode, so embedders can't be silently sandboxed by an upgrade. Why it wasn't usable. On macOS the profile is `(deny default)` with no rule for cwd, so an enabled sandbox denied reads of the project directory itself — `cat src/a.ts` failed inside it, while Linux bound cwd read-write. Nobody hit this because nobody could turn it on. Reads become allow-by-default. The read allowlist did not survive real commands: git couldn't resolve Xcode's developer dir under /Applications, nothing could open /dev/null, temp writes failed because SBPL `subpath` doesn't match the directory node itself, and ~/.gitconfig was denied. Each failure surfaces inside an agent as a confusing permission error. Writes and network stay deny-by-default; well-known credential stores (~/.ssh, ~/.aws, ~/.gnupg, ~/.netrc, ~/.config/gh, the DeepCode credentials file, Keychains) are denied for reading, and filesystem.denyRead still applies last. Package-manager caches are writable — denying ~/.npm turns `npm install` into a permission error while protecting a content-addressed cache. A linked worktree's git dirs live outside the workspace and are added too, or every git command fails in the worktrees EnterWorktree creates. Verified on macOS rather than asserted: under workspace-write, workspace read/write, temp writes, git, node, npm install, tsc and vitest all succeed while writes outside the workspace and reads of ~/.ssh are denied; read-only additionally denies workspace writes. The end-to-end attack test moved its target out of $TMPDIR, which the profile intentionally allows — it had been passing for the wrong reason. Co-Authored-By: Claude Opus 5 --- apps/cli/src/cli.ts | 2 + apps/cli/src/commands.ts | 3 + apps/cli/src/headless.ts | 8 +- apps/cli/src/parse-args.test.ts | 24 ++++ apps/cli/src/parse-args.ts | 18 ++- apps/cli/src/repl.ts | 8 +- docs/security-model.md | 47 ++++++++ packages/core/src/agent.ts | 3 + packages/core/src/config/index.ts | 1 + packages/core/src/config/types.ts | 15 +++ packages/core/src/index.ts | 7 ++ packages/core/src/runtime/host.ts | 15 ++- packages/core/src/sandbox/attacks.test.ts | 13 ++- packages/core/src/sandbox/index.ts | 52 ++++++++- packages/core/src/sandbox/policy.test.ts | 134 ++++++++++++++++++++++ packages/core/src/sandbox/policy.ts | 97 ++++++++++++++++ packages/core/src/sandbox/profile.test.ts | 20 +++- packages/core/src/sandbox/profile.ts | 94 +++++++++++---- packages/core/src/tools/bash.ts | 6 +- packages/core/src/types.ts | 2 + 20 files changed, 529 insertions(+), 40 deletions(-) create mode 100644 packages/core/src/sandbox/policy.test.ts create mode 100644 packages/core/src/sandbox/policy.ts diff --git a/apps/cli/src/cli.ts b/apps/cli/src/cli.ts index b2fd976..47bd63d 100644 --- a/apps/cli/src/cli.ts +++ b/apps/cli/src/cli.ts @@ -157,6 +157,7 @@ async function main(): Promise { cwd: process.cwd(), prompt: args.prompt, outputFormat: args.outputFormat, + sandbox: args.sandbox, mode: args.mode, model: args.model, effort: args.effort, @@ -208,6 +209,7 @@ async function main(): Promise { bare: args.bare, noColor: args.noColor, hideThinking: args.noThinking, + sandbox: args.sandbox, noPlugins: args.noPlugins, settingsPath: args.settingsFile, }); diff --git a/apps/cli/src/commands.ts b/apps/cli/src/commands.ts index e9b00b0..a4c962a 100644 --- a/apps/cli/src/commands.ts +++ b/apps/cli/src/commands.ts @@ -14,6 +14,8 @@ import type { VoiceStatus, } from '@deepcode/core'; import { + describeSandboxMode, + resolveSandboxMode, contextWindowFor, estimateCost, redact, @@ -242,6 +244,7 @@ export const StatusCommand: SlashCommand = { `CWD : ${ctx.cwd}`, `Model : ${ctx.model}`, `Mode : ${ctx.mode}`, + `Sandbox : ${describeSandboxMode(resolveSandboxMode(ctx.settings.sandbox))}`, `Effort : ${ctx.effort}`, `API key : ${redact(ctx.creds.apiKey ?? ctx.creds.authToken)}`, `Base URL : ${ctx.creds.baseURL ?? 'https://api.deepseek.com/v1'}`, diff --git a/apps/cli/src/headless.ts b/apps/cli/src/headless.ts index f011175..9ce53a8 100644 --- a/apps/cli/src/headless.ts +++ b/apps/cli/src/headless.ts @@ -39,6 +39,8 @@ import { loadOutputStyles, loadSettings, withAdditionalWritableDirs, + withSandboxMode, + type SandboxMode, loadSkills, makeSkillTool, resolveCredentials, @@ -63,6 +65,8 @@ export interface HeadlessOpts { prompt: string; /** text | json | stream-json (cli default 'text'). */ outputFormat: 'text' | 'json' | 'stream-json'; + /** `--sandbox ` → overrides settings.sandbox.mode for this run. */ + sandbox?: SandboxMode; mode?: string; model?: string; effort?: Effort; @@ -236,7 +240,7 @@ export async function runHeadless(opts: HeadlessOpts): Promise { hooks, capabilities: buildPluginCapabilitiesHeadless(cwd), sandbox: withAdditionalWritableDirs( - settings.sandbox, + withSandboxMode(settings.sandbox, opts.sandbox), settings.permissions?.additionalDirectories, cwd, ), @@ -301,7 +305,7 @@ export async function runHeadless(opts: HeadlessOpts): Promise { pluginDirs: pluginContrib.dirs, autoMode: settings.autoMode, sandboxConfig: withAdditionalWritableDirs( - settings.sandbox, + withSandboxMode(settings.sandbox, opts.sandbox), settings.permissions?.additionalDirectories, cwd, ), diff --git a/apps/cli/src/parse-args.test.ts b/apps/cli/src/parse-args.test.ts index 52abcfb..253ae87 100644 --- a/apps/cli/src/parse-args.test.ts +++ b/apps/cli/src/parse-args.test.ts @@ -237,3 +237,27 @@ describe('helpText', () => { expect(help).toMatch(/--bare\s+Suppress the REPL startup banner/); }); }); + +describe('--sandbox', () => { + it('accepts each mode', () => { + expect(parseArgs(['--sandbox', 'read-only']).sandbox).toBe('read-only'); + expect(parseArgs(['--sandbox', 'workspace-write']).sandbox).toBe('workspace-write'); + expect(parseArgs(['--sandbox', 'danger-full-access']).sandbox).toBe('danger-full-access'); + }); + + it('rejects anything else instead of silently ignoring it', () => { + const p = parseArgs(['--sandbox', 'yolo']); + expect(p.sandbox).toBeUndefined(); + expect(p.unknownFlags).toEqual(['--sandbox yolo']); + }); + + it('is independent of --mode', () => { + const p = parseArgs(['--mode', 'bypassPermissions', '--sandbox', 'read-only']); + expect(p.mode).toBe('bypassPermissions'); + expect(p.sandbox).toBe('read-only'); + }); + + it('is documented in --help', () => { + expect(helpText('1.0.0')).toContain('--sandbox'); + }); +}); diff --git a/apps/cli/src/parse-args.ts b/apps/cli/src/parse-args.ts index 4978183..c7bb226 100644 --- a/apps/cli/src/parse-args.ts +++ b/apps/cli/src/parse-args.ts @@ -3,7 +3,8 @@ // Returns a strongly-typed shape. Unknown flags are collected into `unknown` for // graceful "did you mean..." errors. -import type { Effort, Mode } from '@deepcode/core'; +import type { Effort, Mode, SandboxMode } from '@deepcode/core'; +import { SANDBOX_MODES } from '@deepcode/core'; export interface ParsedArgs { // Action triggers (mutually exclusive — first match wins) @@ -25,6 +26,11 @@ export interface ParsedArgs { effort?: Effort; maxTurns?: number; bare: boolean; + /** + * `--sandbox ` — what a command may touch. Orthogonal to `--mode`, + * which is about how a tool call gets approved. + */ + sandbox?: SandboxMode; /** `-C` / `--cd `: chdir to this directory before running (Codex parity). */ cwd?: string; @@ -200,6 +206,12 @@ export function parseArgs(argv: string[]): ParsedArgs { case a === '--bare': out.bare = true; break; + case a === '--sandbox': { + const v = next(); + if (v && (SANDBOX_MODES as string[]).includes(v)) out.sandbox = v as SandboxMode; + else out.unknownFlags.push(`--sandbox ${v ?? ''}`); + break; + } case a === '-C' || a === '--cd': out.cwd = next(); break; @@ -331,6 +343,10 @@ MODE WORKING DIRECTORY -C, --cd Change to before running (default: current dir) +SANDBOX (what commands may touch — independent of --mode, which is how they're approved) + --sandbox read-only | workspace-write | danger-full-access + Default: workspace-write + MODEL & EFFORT --model deepseek-chat | deepseek-reasoner --effort low | medium | high | xhigh | max diff --git a/apps/cli/src/repl.ts b/apps/cli/src/repl.ts index 2083434..1cb8579 100644 --- a/apps/cli/src/repl.ts +++ b/apps/cli/src/repl.ts @@ -42,6 +42,8 @@ import { settingsPaths, wirePlugins, withAdditionalWritableDirs, + withSandboxMode, + type SandboxMode, collectPluginContributions, type Effort, type McpClientHandle, @@ -112,6 +114,8 @@ export interface ReplOpts { noColor?: boolean; /** `--no-thinking` → don't stream the model's reasoning. */ hideThinking?: boolean; + /** `--sandbox ` → overrides settings.sandbox.mode for this run. */ + sandbox?: SandboxMode; } const DEFAULT_SYSTEM_PROMPT = `You are DeepCode, an AI coding assistant powered by DeepSeek. Help the user with their codebase using the available tools (Read, Write, Edit, Bash, Grep, Glob). Be concise and accurate. When you modify files, briefly explain what you changed and why.`; @@ -450,7 +454,7 @@ export async function startRepl(opts: ReplOpts): Promise { hooks, capabilities: buildPluginCapabilities(cwd), sandbox: withAdditionalWritableDirs( - settings.sandbox, + withSandboxMode(settings.sandbox, opts.sandbox), settings.permissions?.additionalDirectories, cwd, ), @@ -472,7 +476,7 @@ export async function startRepl(opts: ReplOpts): Promise { pluginDirs: pluginContrib.dirs, autoMode: settings.autoMode, sandboxConfig: withAdditionalWritableDirs( - settings.sandbox, + withSandboxMode(settings.sandbox, opts.sandbox), settings.permissions?.additionalDirectories, cwd, ), diff --git a/docs/security-model.md b/docs/security-model.md index 597f345..57cee2d 100644 --- a/docs/security-model.md +++ b/docs/security-model.md @@ -224,3 +224,50 @@ etc.) as **untrusted**. We: 1. Do NOT open a public GitHub issue. 2. Email security@.dev with reproduction steps + commit SHA. 3. We aim to triage within 72 hours. + +## Sandbox modes (0.2.1) + +The Bash tool runs under a platform sandbox whose posture is chosen by +`sandbox.mode` (settings) or `--sandbox` (CLI), independently of the permission +`Mode` that decides how a tool call is approved: + +| Mode | Workspace | Temp + package caches | Elsewhere | +| -------------------- | ------------------------------------ | --------------------- | --------- | +| `read-only` | read | write | read | +| `workspace-write` | read+write | write | read | +| `danger-full-access` | unrestricted — no sandbox is applied | + +**`workspace-write` is the default** for every host (CLI, headless, app-server). +Library callers of `wrapBashCommand` keep the previous "off unless configured" +behaviour unless they pass `defaultMode`, so embedding DeepCode cannot become +silently sandboxed by an upgrade. + +### What is and is not protected + +Writes and network are deny-by-default. **Reads are allowed** except for a +denied set of credential stores (`~/.ssh`, `~/.aws`, `~/.gnupg`, `~/.netrc`, +`~/.docker/config.json`, `~/.config/gh`, `~/.deepcode/credentials.json`, +`~/Library/Keychains`) plus anything in `filesystem.denyRead`. + +This is a deliberate change from the previous read allowlist, which did not +survive contact with real commands: git could not resolve Xcode's developer +directory, nothing could open `/dev/null`, temp writes failed because SBPL +`subpath` does not match the directory node itself, and `~/.gitconfig` was +denied. A sandbox that breaks `ls` is a sandbox nobody turns on. + +Package-manager caches (`~/.npm`, `~/.cache`, `~/.cargo`, `~/.pnpm-store`, +`~/.yarn`, `~/.bun`, `~/Library/Caches`) are writable: they are +content-addressed caches, and denying them turns `npm install` into a confusing +permission error while protecting nothing. + +A linked git worktree's git directories live outside the workspace, so they are +added to the writable set — otherwise every git command fails inside the +worktrees DeepCode's own `EnterWorktree` tool creates. + +### Verified on macOS + +Under `workspace-write`: workspace read/write, temp writes, `git`, `node`, +`npm install`, `tsc` and `vitest` all succeed; writes outside the workspace and +reads of `~/.ssh` are denied. Under `read-only` the same holds except workspace +writes are denied. Linux (bwrap) already bound cwd read-write and is unchanged +apart from the shared mode resolution. diff --git a/packages/core/src/agent.ts b/packages/core/src/agent.ts index 86351b1..8567a09 100644 --- a/packages/core/src/agent.ts +++ b/packages/core/src/agent.ts @@ -74,6 +74,8 @@ export interface RunAgentOptions { autoMode?: import('./config/types.js').AutoModeConfig; /** M3.5: passed through to Bash tool ctx for sandbox wrapping. */ sandboxConfig?: import('./config/types.js').SandboxConfig; + /** Sandbox mode applied when settings name none. Hosts pass workspace-write. */ + sandboxDefaultMode?: import('./config/types.js').SandboxMode; /** M3c: auto-compact when cumulative tokens approach contextWindow * threshold. * When triggered, runs the summarizer call and replaces history mid-loop. */ autoCompact?: { @@ -263,6 +265,7 @@ export async function runAgent(opts: RunAgentOptions): Promise { cwd: opts.cwd, signal: opts.signal, sandboxConfig: opts.sandboxConfig, + sandboxDefaultMode: opts.sandboxDefaultMode, sessionDir: opts.session ? `${opts.session.manager.root}/${opts.session.id}` : undefined, turnId: opts.session?.turnId, askUser: opts.askUser, diff --git a/packages/core/src/config/index.ts b/packages/core/src/config/index.ts index de76c46..020cc35 100644 --- a/packages/core/src/config/index.ts +++ b/packages/core/src/config/index.ts @@ -12,6 +12,7 @@ export type { McpServerConfig, StatusLineConfig, SandboxConfig, + SandboxMode, UpdateConfig, WorktreeConfig, AutoModeConfig, diff --git a/packages/core/src/config/types.ts b/packages/core/src/config/types.ts index 81554aa..75caf7a 100644 --- a/packages/core/src/config/types.ts +++ b/packages/core/src/config/types.ts @@ -69,7 +69,22 @@ export interface StatusLineConfig { command: string; } +/** + * How much the Bash tool may touch, independent of how tool calls get approved. + * Mirrors the axis Codex exposes as `--sandbox`; `mode` is the modern spelling + * and `enabled` is kept for existing settings files. + */ +export type SandboxMode = 'read-only' | 'workspace-write' | 'danger-full-access'; + export interface SandboxConfig { + /** + * Preferred over `enabled`. When both are set, `mode` wins. + * read-only — the workspace is readable, nothing is writable + * workspace-write — the workspace + temp dirs are writable + * danger-full-access — no sandbox at all + */ + mode?: SandboxMode; + /** Legacy switch: true → workspace-write, false → danger-full-access. */ enabled?: boolean; filesystem?: { allowWrite?: string[]; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 1519ed6..fb42c03 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -117,6 +117,7 @@ export { type McpServerConfig, type StatusLineConfig, type SandboxConfig, + type SandboxMode, type UpdateConfig, type WorktreeConfig, type AutoModeConfig, @@ -226,6 +227,12 @@ export { NetworkSandboxUnavailable, startDnsProxy, withAdditionalWritableDirs, + SANDBOX_MODES, + isSandboxMode, + resolveSandboxMode, + sandboxConfigForMode, + describeSandboxMode, + withSandboxMode, type SandboxPlatform, type SandboxedCommand, type SpawnNetworkSandboxOpts, diff --git a/packages/core/src/runtime/host.ts b/packages/core/src/runtime/host.ts index 9b8d866..4b5f5aa 100644 --- a/packages/core/src/runtime/host.ts +++ b/packages/core/src/runtime/host.ts @@ -4,7 +4,12 @@ import { type RunAgentOptions, type RunAgentResult, } from '../agent.js'; -import type { AutoModeConfig, PermissionRules, SandboxConfig } from '../config/types.js'; +import type { + AutoModeConfig, + PermissionRules, + SandboxConfig, + SandboxMode, +} from '../config/types.js'; import type { HookDispatcher } from '../hooks/index.js'; import type { Provider } from '../providers/types.js'; import type { ToolRegistry } from '../tools/registry.js'; @@ -23,6 +28,12 @@ export interface RuntimeHostOptions { approval?: ApprovalCallback; autoMode?: AutoModeConfig; sandboxConfig?: SandboxConfig; + /** + * Sandbox mode when settings name none. Every host gets `workspace-write`: + * commands may write inside the workspace and temp/cache dirs, and nowhere + * else. Pass `'danger-full-access'` to opt a host out. + */ + sandboxDefaultMode?: SandboxMode; pluginDirs?: string[]; } @@ -35,6 +46,7 @@ type HostBoundOption = | 'approval' | 'autoMode' | 'sandboxConfig' + | 'sandboxDefaultMode' | 'pluginDirs'; export type RuntimeTurnOptions = Omit & { @@ -74,6 +86,7 @@ export class RuntimeHost { approval: approval ?? this.options.approval, autoMode: this.options.autoMode, sandboxConfig: this.options.sandboxConfig, + sandboxDefaultMode: this.options.sandboxDefaultMode ?? 'workspace-write', pluginDirs: this.options.pluginDirs, }); } diff --git a/packages/core/src/sandbox/attacks.test.ts b/packages/core/src/sandbox/attacks.test.ts index 265dff6..b5be444 100644 --- a/packages/core/src/sandbox/attacks.test.ts +++ b/packages/core/src/sandbox/attacks.test.ts @@ -14,7 +14,7 @@ import { spawnSync } from 'node:child_process'; import { promises as fs } from 'node:fs'; -import { tmpdir } from 'node:os'; +import { homedir, tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { wrapBashCommand } from './index.js'; @@ -214,9 +214,13 @@ describe.runIf(hasSandboxExec)('sandbox-exec end-to-end (macOS)', () => { it('blocks writing outside allowed paths', async () => { // Try to write to ~/Documents/foo — NOT in allowWrite, must fail. - const target = join(workDir, 'untrusted-write-target'); - // We pick a path under workDir so we can be sure it doesn't exist; the - // sandbox should be configured to allow only a SIBLING dir for writes. + // Deliberately NOT under the OS temp dir: the profile allows temp writes + // (compilers, package managers and mktemp all need them), so a target + // inside $TMPDIR would test the temp allowance rather than the workspace + // boundary this case is about. + const outsideRoot = await fs.mkdtemp(join(homedir(), '.deepcode-sb-e2e-')); + const target = join(outsideRoot, 'untrusted-write-target'); + // The sandbox should be configured to allow only a SIBLING dir for writes. const allowedDir = join(workDir, 'allowed'); await fs.mkdir(allowedDir); const wrapped = await wrapBashCommand({ @@ -238,6 +242,7 @@ describe.runIf(hasSandboxExec)('sandbox-exec end-to-end (macOS)', () => { } catch { exists = false; } + await fs.rm(outsideRoot, { recursive: true, force: true }); expect(exists).toBe(false); // The shell may exit non-zero or stderr should mention permission const combined = (res.stderr ?? '') + ' ' + (res.stdout ?? ''); diff --git a/packages/core/src/sandbox/index.ts b/packages/core/src/sandbox/index.ts index 5ddd91e..4cf7955 100644 --- a/packages/core/src/sandbox/index.ts +++ b/packages/core/src/sandbox/index.ts @@ -5,8 +5,9 @@ import { promises as fs } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import type { SandboxConfig } from '../config/types.js'; +import { join, resolve } from 'node:path'; +import type { SandboxConfig, SandboxMode } from '../config/types.js'; +import { resolveSandboxMode, sandboxConfigForMode } from './policy.js'; import { allClausesExcluded } from './pipeline.js'; import { buildLinuxBwrapArgs, buildMacOsProfile, detectPlatform } from './profile.js'; @@ -58,8 +59,15 @@ export async function wrapBashCommand(args: { userCommand: string; cwd: string; config: SandboxConfig | undefined; + /** + * Mode to apply when the config names none. Hosts pass the resolved policy; + * library callers that omit it keep the historical "off unless configured" + * behaviour so an embedder can't be silently sandboxed by an upgrade. + */ + defaultMode?: SandboxMode; }): Promise { - const config = args.config; + const mode = resolveSandboxMode(args.config, args.defaultMode ?? 'danger-full-access'); + const config = sandboxConfigForMode(args.config, mode, args.cwd, await linkedGitDirs(args.cwd)); if (!config?.enabled) { return { command: '/bin/sh', args: ['-c', args.userCommand] }; } @@ -93,4 +101,42 @@ export async function wrapBashCommand(args: { return { command: '/bin/sh', args: ['-c', args.userCommand] }; } +/** + * The git directories a workspace needs but doesn't contain. + * + * In a linked worktree `.git` is a file pointing at + * `
/.git/worktrees/`, and the shared object store lives one level + * up again — both outside cwd. Sandboxing the workspace without them breaks + * every git command in exactly the worktrees DeepCode's own EnterWorktree tool + * creates. Best-effort: any read failure just yields no extra paths. + */ +async function linkedGitDirs(cwd: string): Promise { + try { + const pointer = await fs.readFile(join(cwd, '.git'), 'utf8'); + const match = /^gitdir:\s*(.+)$/m.exec(pointer); + if (!match) return []; + const gitDir = resolve(cwd, match[1]!.trim()); + const dirs = [gitDir]; + try { + const commonDir = (await fs.readFile(join(gitDir, 'commondir'), 'utf8')).trim(); + if (commonDir) dirs.push(resolve(gitDir, commonDir)); + } catch { + /* no commondir — a plain gitdir pointer */ + } + return dirs; + } catch { + // `.git` is a directory (ordinary repo) or absent — nothing extra needed. + return []; + } +} + export { withAdditionalWritableDirs } from './additional-dirs.js'; + +export { + SANDBOX_MODES, + isSandboxMode, + resolveSandboxMode, + sandboxConfigForMode, + describeSandboxMode, + withSandboxMode, +} from './policy.js'; diff --git a/packages/core/src/sandbox/policy.test.ts b/packages/core/src/sandbox/policy.test.ts new file mode 100644 index 0000000..c956636 --- /dev/null +++ b/packages/core/src/sandbox/policy.test.ts @@ -0,0 +1,134 @@ +import { describe, expect, it } from 'vitest'; +import { + describeSandboxMode, + isSandboxMode, + resolveSandboxMode, + sandboxConfigForMode, + withSandboxMode, +} from './policy.js'; + +describe('resolveSandboxMode', () => { + it('takes mode when set', () => { + expect(resolveSandboxMode({ mode: 'read-only' })).toBe('read-only'); + }); + + it('lets mode win over the legacy enabled flag', () => { + expect(resolveSandboxMode({ mode: 'read-only', enabled: false })).toBe('read-only'); + expect(resolveSandboxMode({ mode: 'danger-full-access', enabled: true })).toBe( + 'danger-full-access', + ); + }); + + it('maps the legacy boolean on', () => { + expect(resolveSandboxMode({ enabled: true })).toBe('workspace-write'); + expect(resolveSandboxMode({ enabled: false })).toBe('danger-full-access'); + }); + + it('falls back to workspace-write when nothing is configured', () => { + expect(resolveSandboxMode(undefined)).toBe('workspace-write'); + expect(resolveSandboxMode({})).toBe('workspace-write'); + }); + + it('honours a caller-supplied fallback, so libraries keep the old default', () => { + expect(resolveSandboxMode(undefined, 'danger-full-access')).toBe('danger-full-access'); + }); + + it('ignores a mode value that is not a mode', () => { + expect(resolveSandboxMode({ mode: 'yolo' as never })).toBe('workspace-write'); + }); +}); + +describe('sandboxConfigForMode', () => { + it('always makes the workspace readable — the bug that made the sandbox unusable', () => { + const cfg = sandboxConfigForMode({}, 'workspace-write', '/repo'); + expect(cfg?.filesystem?.allowRead).toContain('/repo'); + }); + + it('makes the workspace writable in workspace-write only', () => { + expect(sandboxConfigForMode({}, 'workspace-write', '/repo')?.filesystem?.allowWrite).toContain( + '/repo', + ); + expect(sandboxConfigForMode({}, 'read-only', '/repo')?.filesystem?.allowWrite).toEqual([]); + }); + + it('keeps read access to the workspace in read-only', () => { + expect(sandboxConfigForMode({}, 'read-only', '/repo')?.filesystem?.allowRead).toContain( + '/repo', + ); + }); + + it('disables the sandbox entirely for danger-full-access', () => { + expect(sandboxConfigForMode({}, 'danger-full-access', '/repo')?.enabled).toBe(false); + }); + + it('adds a linked worktree’s git dirs, which live outside the workspace', () => { + const cfg = sandboxConfigForMode({}, 'workspace-write', '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/repo/wt', [ + '/main/.git/worktrees/wt', + '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/main/.git', + ]); + expect(cfg?.filesystem?.allowWrite).toEqual( + expect.arrayContaining(['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/repo/wt', '/main/.git/worktrees/wt', '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/main/.git']), + ); + }); + + it('preserves configured allow lists rather than replacing them', () => { + const cfg = sandboxConfigForMode( + { filesystem: { allowRead: ['/data'], allowWrite: ['/out'] } }, + 'workspace-write', + '/repo', + ); + expect(cfg?.filesystem?.allowRead).toEqual(['/data', '/repo']); + expect(cfg?.filesystem?.allowWrite).toEqual(['/out', '/repo']); + }); + + it('does not duplicate a workspace already listed', () => { + const cfg = sandboxConfigForMode( + { filesystem: { allowWrite: ['/repo'] } }, + 'workspace-write', + '/repo', + ); + expect(cfg?.filesystem?.allowWrite).toEqual(['/repo']); + }); + + it('keeps network settings untouched', () => { + const cfg = sandboxConfigForMode( + { network: { allowedDomains: ['example.com'] } }, + 'workspace-write', + '/repo', + ); + expect(cfg?.network?.allowedDomains).toEqual(['example.com']); + }); +}); + +describe('withSandboxMode', () => { + it('returns the config unchanged when no override is given', () => { + const config = { mode: 'read-only' as const }; + expect(withSandboxMode(config, undefined)).toBe(config); + }); + + it('applies the override', () => { + expect(withSandboxMode({}, 'read-only')?.mode).toBe('read-only'); + }); + + it('drops a stale enabled:false so an explicit flag is not silently defeated', () => { + const result = withSandboxMode({ enabled: false }, 'workspace-write'); + expect(result?.mode).toBe('workspace-write'); + expect(result?.enabled).toBeUndefined(); + expect(resolveSandboxMode(result)).toBe('workspace-write'); + }); +}); + +describe('isSandboxMode / describeSandboxMode', () => { + it('recognises exactly the three modes', () => { + expect(isSandboxMode('read-only')).toBe(true); + expect(isSandboxMode('workspace-write')).toBe(true); + expect(isSandboxMode('danger-full-access')).toBe(true); + expect(isSandboxMode('off')).toBe(false); + }); + + it('describes each mode in terms of what a command may do', () => { + expect(describeSandboxMode('read-only')).toMatch(/cannot write/); + expect(describeSandboxMode('workspace-write')).toMatch(/can write/); + expect(describeSandboxMode('danger-full-access')).toMatch(/unsandboxed/); + }); +}); diff --git a/packages/core/src/sandbox/policy.ts b/packages/core/src/sandbox/policy.ts new file mode 100644 index 0000000..3abccc4 --- /dev/null +++ b/packages/core/src/sandbox/policy.ts @@ -0,0 +1,97 @@ +// Resolving what the sandbox should actually do. +// +// Two axes, deliberately separate (docs/CODEX_ALIGNMENT_PLAN.md §5.5): `mode` +// (this file) decides what a command may touch; the permission `Mode` decides +// how a tool call gets approved. A single knob could not express "never ask me, +// but still keep writes inside the workspace". +// +// Pure — no fs, no platform checks — so every host resolves the same way. + +import type { SandboxConfig, SandboxMode } from '../config/types.js'; + +export const SANDBOX_MODES: SandboxMode[] = ['read-only', 'workspace-write', 'danger-full-access']; + +export function isSandboxMode(value: string): value is SandboxMode { + return (SANDBOX_MODES as string[]).includes(value); +} + +/** + * The mode a config asks for. + * + * `mode` wins when set. Otherwise the legacy `enabled` boolean maps on: + * `true` → workspace-write, `false` → danger-full-access. An unconfigured + * sandbox resolves to `fallback`, which the caller picks — hosts default to + * workspace-write, and `resolveSandboxMode(cfg, 'danger-full-access')` recovers + * the pre-mode behaviour for callers that need it. + */ +export function resolveSandboxMode( + config: SandboxConfig | undefined, + fallback: SandboxMode = 'workspace-write', +): SandboxMode { + if (config?.mode && isSandboxMode(config.mode)) return config.mode; + if (config?.enabled === true) return 'workspace-write'; + if (config?.enabled === false) return 'danger-full-access'; + return fallback; +} + +/** + * The config a platform profile builder should see, with the resolved mode + * folded in: the workspace is always readable, and writable in workspace-write. + * + * Before this, an enabled macOS sandbox denied reads of the project directory + * itself — `(deny default)` with no rule for cwd — so `cat src/a.ts` failed + * inside it while the Linux path bound cwd read-write. Nobody hit it because + * the sandbox was off by default. + */ +export function sandboxConfigForMode( + config: SandboxConfig | undefined, + mode: SandboxMode, + cwd: string, + /** Extra paths the workspace depends on — a linked worktree's git dirs. */ + extraWorkspacePaths: string[] = [], +): SandboxConfig | undefined { + if (mode === 'danger-full-access') return { ...config, enabled: false }; + + const filesystem = { ...(config?.filesystem ?? {}) }; + const allowRead = [...(filesystem.allowRead ?? [])]; + const allowWrite = [...(filesystem.allowWrite ?? [])]; + + // Git writes to its own directory even for read-ish commands (index.lock, + // refs, reflog), so the linked dirs go in both lists whenever anything is + // writable at all. + for (const path of [cwd, ...extraWorkspacePaths]) { + if (!allowRead.includes(path)) allowRead.push(path); + if (mode === 'workspace-write' && !allowWrite.includes(path)) allowWrite.push(path); + } + + return { + ...config, + mode, + enabled: true, + filesystem: { ...filesystem, allowRead, allowWrite: mode === 'read-only' ? [] : allowWrite }, + }; +} + +/** One line for `doctor` / `/status`: what the sandbox will do to a command. */ +export function describeSandboxMode(mode: SandboxMode): string { + switch (mode) { + case 'read-only': + return 'read-only — commands can read the workspace but cannot write to it'; + case 'workspace-write': + return 'workspace-write — commands can write inside the workspace and temp dirs'; + case 'danger-full-access': + return 'danger-full-access — commands run unsandboxed'; + } +} + +/** Apply a `--sandbox ` override on top of whatever settings say. */ +export function withSandboxMode( + config: SandboxConfig | undefined, + mode: SandboxMode | undefined, +): SandboxConfig | undefined { + if (!mode) return config; + // `enabled` is dropped: a stale `enabled: false` in settings must not + // silently defeat an explicit `--sandbox workspace-write`. + const { enabled: _enabled, ...rest } = config ?? {}; + return { ...rest, mode }; +} diff --git a/packages/core/src/sandbox/profile.test.ts b/packages/core/src/sandbox/profile.test.ts index 79f4396..854ac60 100644 --- a/packages/core/src/sandbox/profile.test.ts +++ b/packages/core/src/sandbox/profile.test.ts @@ -1,3 +1,4 @@ +import { homedir } from 'node:os'; import { describe, expect, it } from 'vitest'; import { buildLinuxBwrapArgs, buildMacOsProfile, detectPlatform } from './profile.js'; @@ -13,11 +14,26 @@ describe('buildMacOsProfile', () => { expect(buildMacOsProfile({ enabled: false }, '/x')).toBe(''); }); - it('starts with deny-default + allows system reads', () => { + it('denies by default, allows reads, and keeps writes narrow', () => { const profile = buildMacOsProfile({ enabled: true }, '/proj'); expect(profile).toMatch(/\(deny default\)/); - expect(profile).toMatch(/file-read\* \(subpath "\/usr"\)/); + // Reads are broadly allowed — a read allowlist could not survive real + // commands (git, /dev/null, temp dirs). Writes stay deny-by-default. + expect(profile).toMatch(/\(allow file-read\*\)/); expect(profile).toMatch(/file-write\* \(subpath "\/private\/tmp"\)/); + expect(profile).not.toMatch(/\(allow file-write\*\)\n/); + }); + + it('denies the credential stores an agent has no business reading', () => { + const profile = buildMacOsProfile({ enabled: true }, '/proj'); + for (const secret of ['.ssh', '.aws', '.gnupg', '.netrc', 'Library/Keychains']) { + expect(profile).toContain(`(deny file-read* (subpath "${homedir()}/${secret}"))`); + } + }); + + it('allows package-manager caches so npm install still works', () => { + const profile = buildMacOsProfile({ enabled: true }, '/proj'); + expect(profile).toContain(`(allow file-write* (subpath "${homedir()}/.npm"))`); }); it('includes allowRead + allowWrite paths', () => { diff --git a/packages/core/src/sandbox/profile.ts b/packages/core/src/sandbox/profile.ts index df97aca..e0c665c 100644 --- a/packages/core/src/sandbox/profile.ts +++ b/packages/core/src/sandbox/profile.ts @@ -12,6 +12,7 @@ // Windows: disabled per §0.2. import { homedir, platform } from 'node:os'; +import { dirname } from 'node:path'; import type { SandboxConfig } from '../config/types.js'; export type SandboxPlatform = 'macos' | 'linux' | 'unsupported'; @@ -33,7 +34,7 @@ export function detectPlatform(): SandboxPlatform { * has 200+ predicates) is out of scope for M3.5. We cover the dimensions plan * §3.9a calls out: fs read/write, net allow/deny, excluded commands. */ -export function buildMacOsProfile(config: SandboxConfig, _cwd: string): string { +export function buildMacOsProfile(config: SandboxConfig, cwd: string): string { if (!config.enabled) return ''; const fs = config.filesystem ?? {}; const net = config.network ?? {}; @@ -51,32 +52,46 @@ export function buildMacOsProfile(config: SandboxConfig, _cwd: string): string { '(allow mach-lookup)', '(allow iokit-open)', '(allow ipc-posix-shm)', - '; allow read of system libraries + caches', - // Literal entries for root + /private so path traversal (getcwd, stat of - // ancestor dirs) doesn't get denied. `subpath` matches contents under but - // NOT the directory entry itself. - '(allow file-read* (literal "/"))', - '(allow file-read* (literal "/private"))', - '(allow file-read* (literal "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/private/var"))', - '(allow file-read* (literal "/Users"))', - '(allow file-read* (subpath "/usr"))', - '(allow file-read* (subpath "/System"))', - '(allow file-read* (subpath "/Library"))', - '(allow file-read* (subpath "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/private/etc"))', - '(allow file-read* (subpath "/private/var/db"))', - '(allow file-read* (subpath "/private/var/folders"))', // dyld closure cache - '(allow file-read* (subpath "/dev"))', - '(allow file-read* (subpath "/bin"))', - '(allow file-read* (subpath "/sbin"))', - '(allow file-read* (subpath "/opt"))', - `(allow file-read* (subpath "${home}/.config"))`, - `(allow file-read* (subpath "${home}/.npm"))`, - `(allow file-read* (subpath "${home}/.cache"))`, - `(allow file-read* (subpath "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/private/tmp"))`, + // Reads are allowed, writes are not. + // + // This used to be a read allowlist, and it did not survive contact with + // real commands: git could not resolve Xcode's developer dir, nothing + // could open /dev/null, temp writes failed because `subpath` does not + // match the directory node itself, and reading ~/.gitconfig was denied. + // Every one of those is a silent, confusing failure inside an agent. + // + // What the sandbox actually protects is *writes* and *network*, which stay + // deny-by-default. Well-known credential stores are denied below, and any + // filesystem.denyRead entry is applied last, so a stricter posture is still + // expressible — it is just no longer the thing that has to be right for + // `ls` to work. + '; reads: broadly allowed, with credential stores denied below', + '(allow file-read*)', + '; writes: denied unless granted for the workspace / temp dirs', `(allow file-write* (subpath "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/private/tmp"))`, - `(allow file-write* (subpath "/private/var/folders"))`, // macOS tmp + `(allow file-write* (subpath "/private/var/folders"))`, // macOS per-user temp + // /dev/null and /dev/tty are opened read-write by ~everything. Character + // devices are not the filesystem the sandbox is protecting. + `(allow file-write* (subpath "/dev"))`, + // Package-manager caches. Denying these turns `npm install` — one of the + // most common things an agent runs — into a confusing permission error, + // while protecting nothing: they are content-addressed caches, not source + // and not secrets. Everything else under $HOME stays read-only. + ...['.npm', '.cache', '.cargo', '.pnpm-store', '.yarn', '.bun', 'Library/Caches'].map( + (dir) => `(allow file-write* (subpath "${escapeSbpl(`${home}/${dir}`)}"))`, + ), ]; + // The workspace itself. `(deny default)` means an enabled sandbox denied + // reads of the project directory unless someone remembered to list it under + // filesystem.allowRead — so `cat src/a.ts` failed inside the sandbox, while + // the Linux path bound cwd read-write. sandboxConfigForMode() now folds cwd + // into allowRead/allowWrite, and these entries cover the ancestor directories + // a resolved path has to traverse. + for (const ancestor of ancestorsOf(cwd)) { + lines.push(`(allow file-read* (literal "${escapeSbpl(ancestor)}"))`); + } + for (const p of fs.allowRead ?? []) { lines.push(`(allow file-read* (subpath "${escapeSbpl(expandTilde(p, home))}"))`); } @@ -85,6 +100,23 @@ export function buildMacOsProfile(config: SandboxConfig, _cwd: string): string { lines.push(`(allow file-read* (subpath "${expanded}"))`); lines.push(`(allow file-write* (subpath "${expanded}"))`); } + // Credential stores an agent has no business reading. Denies beat the blanket + // read allow above (SBPL is last-match-wins), and a user's own denyRead + // entries come after these. + for (const secret of [ + `${home}/.ssh`, + `${home}/.aws`, + `${home}/.gnupg`, + `${home}/.netrc`, + `${home}/.docker/config.json`, + `${home}/.config/gh`, + `${home}/.deepcode/credentials.json`, + `${home}/Library/Keychains`, + ]) { + lines.push(`(deny file-read* (subpath "${escapeSbpl(secret)}"))`); + lines.push(`(deny file-read* (literal "${escapeSbpl(secret)}"))`); + } + // Explicit deny rules go LAST so they override the allows above for (const p of fs.denyRead ?? []) { lines.push(`(deny file-read* (subpath "${escapeSbpl(expandTilde(p, home))}"))`); @@ -111,6 +143,20 @@ export function buildMacOsProfile(config: SandboxConfig, _cwd: string): string { return lines.join('\n') + '\n'; } +/** Every directory between "/" and `p`, inclusive — needed for path traversal. */ +function ancestorsOf(p: string): string[] { + const out: string[] = []; + let current = p; + while (current && current !== '/' && current !== '.') { + out.push(current); + const next = dirname(current); + if (next === current) break; + current = next; + } + out.push('/'); + return out; +} + function escapeSbpl(s: string): string { // Escape backslash and double-quote return s.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); diff --git a/packages/core/src/tools/bash.ts b/packages/core/src/tools/bash.ts index 3b8c5dd..9b58ec7 100644 --- a/packages/core/src/tools/bash.ts +++ b/packages/core/src/tools/bash.ts @@ -18,7 +18,7 @@ import { wrapBashCommand, } from '../sandbox/index.js'; import type { NetworkSandboxHandle, SpawnNetworkSandboxOpts } from '../sandbox/index.js'; -import type { SandboxConfig } from '../config/types.js'; +import type { SandboxConfig, SandboxMode } from '../config/types.js'; import type { ToolContext, ToolHandler, ToolResult } from '../types.js'; interface BashInput { @@ -31,6 +31,8 @@ interface BashInput { // ToolContext carries sandbox config (+ optional test seams) from the loop owner. type SandboxCtx = ToolContext & { sandboxConfig?: SandboxConfig; + /** Mode to apply when settings name none — hosts set workspace-write. */ + sandboxDefaultMode?: SandboxMode; /** Test seam: override the platform used for the net-sandbox decision. */ sandboxPlatform?: NodeJS.Platform; /** Test seam: override the network-sandbox spawner. */ @@ -213,6 +215,7 @@ export const BashTool: ToolHandler = { userCommand: input.command, cwd: ctx.cwd, config: bgCfg, + defaultMode: sctx.sandboxDefaultMode, }); const dir = join(ctx.sessionDir ?? tmpdir(), 'bg'); const id = `bg-${Date.now().toString(36)}-${process.pid}-${bgSeq++}`; @@ -266,6 +269,7 @@ export const BashTool: ToolHandler = { userCommand: input.command, cwd: ctx.cwd, config: effectiveCfg, + defaultMode: sctx.sandboxDefaultMode, }); return new Promise((resolvePromise) => { diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 4abd4a8..0b04184 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -118,6 +118,8 @@ export interface ToolContext { signal?: AbortSignal; /** Optional platform sandbox config — passed through to Bash tool (M3.5). */ sandboxConfig?: import('./config/types.js').SandboxConfig; + /** Sandbox mode used when `sandboxConfig` names none (hosts: workspace-write). */ + sandboxDefaultMode?: import('./config/types.js').SandboxMode; /** * Host callback for interactive prompts (AskUserQuestion). Returns undefined * in headless mode. Called by the AskUserQuestion tool with the question +