From 843b036b38671532339577b7eb88c9ad22cbf284 Mon Sep 17 00:00:00 2001 From: t Date: Sun, 7 Jun 2026 17:47:45 +0800 Subject: [PATCH] feat(cli): enforce /add-dir via sandbox writable roots (security-sensitive) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /add-dir only printed a message; the settings.permissions.additionalDirectories field was declared but consumed nowhere. Now: - /add-dir validates the path is an existing directory and PERSISTS it to permissions.additionalDirectories (dedup) in the user settings; with no args it lists the current set. - New core helper withAdditionalWritableDirs(sandbox, dirs) folds those dirs into the sandbox's filesystem.allowWrite. The REPL + headless build their sandboxConfig through it, so the sandboxed Bash tool can write to added dirs (beyond cwd). No-op when the sandbox is off; never mutates input. The file tools (Read/Write/Edit/Glob/Grep) already accept any absolute path, so this only changes the SANDBOX boundary for Bash — which is the security-relevant surface. Holding this PR for review rather than auto-merging. Tests: withAdditionalWritableDirs (add/dedup/undefined-safe) + /add-dir (persist / reject-missing / list). core 646 · cli 143. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/cli/src/commands.ts | 39 +++++++- apps/cli/src/headless.ts | 6 +- apps/cli/src/parity-commands.test.ts | 28 ++++++ apps/cli/src/repl.ts | 6 +- docs/BEHAVIOR_PARITY.md | 98 +++++++++---------- packages/core/src/index.ts | 1 + .../core/src/sandbox/additional-dirs.test.ts | 27 +++++ packages/core/src/sandbox/index.ts | 15 +++ 8 files changed, 164 insertions(+), 56 deletions(-) create mode 100644 packages/core/src/sandbox/additional-dirs.test.ts diff --git a/apps/cli/src/commands.ts b/apps/cli/src/commands.ts index 7f8bd42..438363e 100644 --- a/apps/cli/src/commands.ts +++ b/apps/cli/src/commands.ts @@ -22,7 +22,8 @@ import { type Effort, } from '@deepcode/core'; import { execFile } from 'node:child_process'; -import { readFile } from 'node:fs/promises'; +import { readFile, stat } from 'node:fs/promises'; +import { isAbsolute, resolve } from 'node:path'; import { promisify } from 'node:util'; const execFileAsync = promisify(execFile); @@ -406,10 +407,38 @@ export const ConfigCommand: SlashCommand = { export const AddDirCommand: SlashCommand = { name: '/add-dir', - description: 'Add an additional allowed directory (M3 enforced; M2 records intent).', - run(args) { - if (args.length === 0) return ['Usage: /add-dir ']; - return [`Recorded ${args[0]} as additional allowed directory (effective in M3).`]; + description: 'Add a directory the agent may write to (persists; sandbox-enforced).', + async run(args, ctx) { + const current = ctx.settings.permissions?.additionalDirectories ?? []; + if (args.length === 0) { + return current.length > 0 + ? ['Additional writable directories:', ...current.map((d) => ` ${d}`)] + : ['No additional directories yet. Usage: /add-dir ']; + } + if (!ctx.userSettingsPath) return ['(/add-dir is unavailable here.)']; + const dir = isAbsolute(args[0]!) ? args[0]! : resolve(ctx.cwd, args[0]!); + try { + if (!(await stat(dir)).isDirectory()) return [`Not a directory: ${dir}`]; + } catch { + return [`No such directory: ${dir}`]; + } + let onDisk: Record = {}; + try { + onDisk = JSON.parse(await readFile(ctx.userSettingsPath, 'utf8')) as Record; + } catch { + /* missing → start fresh */ + } + const perms = (onDisk.permissions ?? {}) as Record; + const existing = Array.isArray(perms.additionalDirectories) + ? (perms.additionalDirectories as string[]) + : []; + perms.additionalDirectories = [...new Set([...existing, dir])]; + onDisk.permissions = perms; + await writeSettings(ctx.userSettingsPath, onDisk as DeepCodeSettings); + return [ + `Added ${dir} as an additional writable directory.`, + 'The sandboxed Bash tool can write there (restart for it to take effect in a new session).', + ]; }, }; diff --git a/apps/cli/src/headless.ts b/apps/cli/src/headless.ts index c24213b..febf79c 100644 --- a/apps/cli/src/headless.ts +++ b/apps/cli/src/headless.ts @@ -36,6 +36,7 @@ import { loadMemory, loadOutputStyles, loadSettings, + withAdditionalWritableDirs, loadSkills, makeSkillTool, resolveCredentials, @@ -289,7 +290,10 @@ export async function runHeadless(opts: HeadlessOpts): Promise { pluginDirs: pluginContrib.dirs, autoCompact: { contextWindow: contextWindowFor(model), threshold: 0.8 }, autoMode: settings.autoMode, - sandboxConfig: settings.sandbox, + sandboxConfig: withAdditionalWritableDirs( + settings.sandbox, + settings.permissions?.additionalDirectories, + ), // In headless mode there's no human to ask: auto-deny anything that // would normally need approval. Users wanting auto-yes should pass // --mode dontAsk or --mode bypassPermissions (gated by trust). diff --git a/apps/cli/src/parity-commands.test.ts b/apps/cli/src/parity-commands.test.ts index 45667c9..9357b07 100644 --- a/apps/cli/src/parity-commands.test.ts +++ b/apps/cli/src/parity-commands.test.ts @@ -182,3 +182,31 @@ describe('/config set', () => { expect(out.join('\n')).toMatch(/Usage: \/config set/); }); }); + +describe('/add-dir', () => { + it('persists a validated directory to permissions.additionalDirectories', async () => { + const home = await tmpHome(); // a real, existing directory + const path = join(home, 'settings.json'); + const out = await reg.match('/add-dir')!.cmd.run([home], ctx({ userSettingsPath: path })); + expect(out.join('\n')).toMatch(/Added .* writable directory/i); + const written = JSON.parse(await readFile(path, 'utf8')) as { + permissions?: { additionalDirectories?: string[] }; + }; + expect(written.permissions?.additionalDirectories).toContain(home); + }); + + it('rejects a non-existent directory', async () => { + const home = await tmpHome(); + const out = await reg + .match('/add-dir')! + .cmd.run([join(home, 'nope')], ctx({ userSettingsPath: join(home, 'settings.json') })); + expect(out.join('\n')).toMatch(/no such directory/i); + }); + + it('lists current directories with no args', async () => { + const out = await reg + .match('/add-dir')! + .cmd.run([], ctx({ settings: { permissions: { additionalDirectories: ['/x/y'] } } })); + expect(out.join('\n')).toContain('/x/y'); + }); +}); diff --git a/apps/cli/src/repl.ts b/apps/cli/src/repl.ts index 6c2c059..67a33c8 100644 --- a/apps/cli/src/repl.ts +++ b/apps/cli/src/repl.ts @@ -39,6 +39,7 @@ import { runAgent, settingsPaths, wirePlugins, + withAdditionalWritableDirs, collectPluginContributions, type Effort, type McpClientHandle, @@ -616,7 +617,10 @@ export async function startRepl(opts: ReplOpts): Promise { pluginDirs: pluginContrib.dirs, autoCompact: { contextWindow: contextWindowFor(ctx.model), threshold: 0.8 }, autoMode: settings.autoMode, - sandboxConfig: settings.sandbox, + sandboxConfig: withAdditionalWritableDirs( + settings.sandbox, + settings.permissions?.additionalDirectories, + ), approval: async (toolName, _input, verdict) => { output.write(`\n ⏸ Approve ${toolName}? Reason: ${verdict.reason}\n`); const answer = (await rl.question(' [y]es / [n]o / [a]lways: ')).trim().toLowerCase(); diff --git a/docs/BEHAVIOR_PARITY.md b/docs/BEHAVIOR_PARITY.md index aa1fa20..33bfa07 100644 --- a/docs/BEHAVIOR_PARITY.md +++ b/docs/BEHAVIOR_PARITY.md @@ -21,55 +21,55 @@ Legend: `✅` matches · `🟡` matches with caveats · `🔄` deferred · `⚠ ## Slash commands (30+ in Claude Code, ~32 shipped in DeepCode) -| Command | Claude Code | DeepCode | Status | -| -------------------------- | ----------- | ------------------ | ------------------------------------------------------------------------------------------------------------------------------------ | -| `/help` | ✓ | ✓ | ✅ | -| `/clear` | ✓ | ✓ | ✅ | -| `/exit` / `/quit` | ✓ | ✓ | ✅ | -| `/status` / `/doctor` | ✓ | ✓ | ✅ | -| `/model` | ✓ | ✓ | ✅ DeepCode constrains to deepseek-\* (model picker doesn't show foreign providers) | -| `/mode` | ✓ | ✓ | ✅ | -| `/effort` | ✓ | ✓ | 🟡 — CLI prints the tier table (numbers from `EFFORT_PARAMS` SSOT); switch via `/effort `; arrow-key selector is GUI-only (M6) | -| `/cost` / `/usage` | ✓ | ✓ | ✅ | -| `/context` | ✓ | ✓ | ✅ | -| `/config` | ✓ | ✓ | 🟡 — dumps merged settings + `/config set ` (dotted keys, JSON values) writes user settings; no full arrow-key editor | -| `/resume` | ✓ | ✓ (list only) | 🟡 — Claude Code has fuzzy picker; ours lists; pick via `--resume ` | -| `/init` | ✓ | ✓ | ✅ — interactive 3-phase REPL flow (scan → draft → approve-write `AGENTS.md`) | -| `/mcp` | ✓ | ✓ | ✅ | -| `/add-dir` | ✓ | ✓ (records intent) | 🟡 — M3 will enforce | -| `/todos` | ✓ | ✓ | ✅ — reads `/todos.json` written by TodoWrite tool | -| `/plugins` | ✓ | ✓ | ✅ — lists wired plugins + contributed hook events + warnings (M5.2) | -| `/compact` | ✓ | ✓ | ✅ — manual `/compact` + automatic threshold trigger in the agent loop | -| `/diff` | ✓ | ✓ | ✅ — git diff + untracked files in the working tree (PR #150) | -| `/btw` | ✓ | ✗ | 🔄 | -| `/recap` | ✓ | ✓ | ✅ — provider-summarized recap of the session so far | -| `/rewind` | ✓ | ✓ | ✅ — 5 ops (code/conversation/both/summarize-from/up-to); `Esc Esc` bound | -| `/voice` | ✓ | ✗ | 🔄 M8 | -| `/teleport` | ✓ | ✗ | 🔄 M8 | -| `/desktop` | ✓ | ✗ | 🔄 M6 | -| `/background` | ✓ | ✗ | 🔄 (paired with TaskCreate M3.15.3) | -| `/batch` | ✓ | ✗ | 🔄 | -| `/tasks` | ✓ | ✗ | 🔄 | -| `/plan` | ✓ | ✗ | 🔄 — set via `/mode plan` in DeepCode | -| `/login` / `/logout` | ✓ | ✓ | ✅ — /logout clears creds + exits; /login stores a new key (next launch) | -| `/export` | ✓ | ✓ | ✅ — writes the conversation to a markdown file | -| `/bug` (alias `/feedback`) | ✓ | ✓ | ✅ — prints a prefilled GitHub issue link (model/mode/effort in the body) | -| `/upgrade` | ✓ | ✓ | ✅ — prints version + `npm i -g deepcode-cli@latest` (also the `deepcode upgrade` subcommand) | -| `/pr_comments` | ✓ | ✓ | ✅ — `gh pr view` comments for the current branch's PR | -| `/review` | ✓ | ✗ (skill avail) | 🟡 — via Skill tool | -| `/security-review` | ✓ | ✗ (skill avail) | 🟡 — via Skill tool | -| `/schedule` | ✓ | ✗ (skill avail) | 🟡 | -| `/loop` | ✓ | ✗ (skill avail) | 🟡 | -| `/terminal-setup` | ✓ | ✗ | 🔄 | -| `/vim` | ✓ | ✓ | ✅ — toggles Vim mode (persists to `~/.deepcode/keybindings.json`) | -| `/keybindings` | ✓ | ✓ (read-only) | 🟡 — Claude Code opens/creates the keybindings config; ours lists bindings (edit `~/.deepcode/keybindings.json` manually) | -| `/agents` | ✓ | ✓ | ✅ — lists sub-agents from `.deepcode/agents/` | -| `/hooks` | ✓ | ✓ | ✅ — lists hooks configured in settings.json | -| `/skills` | ✓ | ✓ | ✅ — lists built-in + user + project skills | -| `/permissions` | ✓ | ✓ (read-only) | 🟡 — shows rules + default mode (interactive editor deferred) | -| `/privacy-settings` | ✓ | ✓ | ✅ — summarizes local data locations + what's sent to the DeepSeek API (read-only) | -| `/migrate-installer` | ✓ | ✗ | 🔄 | -| `/release-notes` | ✓ | ✓ | ✅ — prints the latest `CHANGELOG.md` entry | +| Command | Claude Code | DeepCode | Status | +| -------------------------- | ----------- | --------------- | ------------------------------------------------------------------------------------------------------------------------------------------- | +| `/help` | ✓ | ✓ | ✅ | +| `/clear` | ✓ | ✓ | ✅ | +| `/exit` / `/quit` | ✓ | ✓ | ✅ | +| `/status` / `/doctor` | ✓ | ✓ | ✅ | +| `/model` | ✓ | ✓ | ✅ DeepCode constrains to deepseek-\* (model picker doesn't show foreign providers) | +| `/mode` | ✓ | ✓ | ✅ | +| `/effort` | ✓ | ✓ | 🟡 — CLI prints the tier table (numbers from `EFFORT_PARAMS` SSOT); switch via `/effort `; arrow-key selector is GUI-only (M6) | +| `/cost` / `/usage` | ✓ | ✓ | ✅ | +| `/context` | ✓ | ✓ | ✅ | +| `/config` | ✓ | ✓ | 🟡 — dumps merged settings + `/config set ` (dotted keys, JSON values) writes user settings; no full arrow-key editor | +| `/resume` | ✓ | ✓ (list only) | 🟡 — Claude Code has fuzzy picker; ours lists; pick via `--resume ` | +| `/init` | ✓ | ✓ | ✅ — interactive 3-phase REPL flow (scan → draft → approve-write `AGENTS.md`) | +| `/mcp` | ✓ | ✓ | ✅ | +| `/add-dir` | ✓ | ✓ | ✅ — persists to `permissions.additionalDirectories`; the sandboxed Bash tool gets write access there (folded into `filesystem.allowWrite`) | +| `/todos` | ✓ | ✓ | ✅ — reads `/todos.json` written by TodoWrite tool | +| `/plugins` | ✓ | ✓ | ✅ — lists wired plugins + contributed hook events + warnings (M5.2) | +| `/compact` | ✓ | ✓ | ✅ — manual `/compact` + automatic threshold trigger in the agent loop | +| `/diff` | ✓ | ✓ | ✅ — git diff + untracked files in the working tree (PR #150) | +| `/btw` | ✓ | ✗ | 🔄 | +| `/recap` | ✓ | ✓ | ✅ — provider-summarized recap of the session so far | +| `/rewind` | ✓ | ✓ | ✅ — 5 ops (code/conversation/both/summarize-from/up-to); `Esc Esc` bound | +| `/voice` | ✓ | ✗ | 🔄 M8 | +| `/teleport` | ✓ | ✗ | 🔄 M8 | +| `/desktop` | ✓ | ✗ | 🔄 M6 | +| `/background` | ✓ | ✗ | 🔄 (paired with TaskCreate M3.15.3) | +| `/batch` | ✓ | ✗ | 🔄 | +| `/tasks` | ✓ | ✗ | 🔄 | +| `/plan` | ✓ | ✗ | 🔄 — set via `/mode plan` in DeepCode | +| `/login` / `/logout` | ✓ | ✓ | ✅ — /logout clears creds + exits; /login stores a new key (next launch) | +| `/export` | ✓ | ✓ | ✅ — writes the conversation to a markdown file | +| `/bug` (alias `/feedback`) | ✓ | ✓ | ✅ — prints a prefilled GitHub issue link (model/mode/effort in the body) | +| `/upgrade` | ✓ | ✓ | ✅ — prints version + `npm i -g deepcode-cli@latest` (also the `deepcode upgrade` subcommand) | +| `/pr_comments` | ✓ | ✓ | ✅ — `gh pr view` comments for the current branch's PR | +| `/review` | ✓ | ✗ (skill avail) | 🟡 — via Skill tool | +| `/security-review` | ✓ | ✗ (skill avail) | 🟡 — via Skill tool | +| `/schedule` | ✓ | ✗ (skill avail) | 🟡 | +| `/loop` | ✓ | ✗ (skill avail) | 🟡 | +| `/terminal-setup` | ✓ | ✗ | 🔄 | +| `/vim` | ✓ | ✓ | ✅ — toggles Vim mode (persists to `~/.deepcode/keybindings.json`) | +| `/keybindings` | ✓ | ✓ (read-only) | 🟡 — Claude Code opens/creates the keybindings config; ours lists bindings (edit `~/.deepcode/keybindings.json` manually) | +| `/agents` | ✓ | ✓ | ✅ — lists sub-agents from `.deepcode/agents/` | +| `/hooks` | ✓ | ✓ | ✅ — lists hooks configured in settings.json | +| `/skills` | ✓ | ✓ | ✅ — lists built-in + user + project skills | +| `/permissions` | ✓ | ✓ (read-only) | 🟡 — shows rules + default mode (interactive editor deferred) | +| `/privacy-settings` | ✓ | ✓ | ✅ — summarizes local data locations + what's sent to the DeepSeek API (read-only) | +| `/migrate-installer` | ✓ | ✗ | 🔄 | +| `/release-notes` | ✓ | ✓ | ✅ — prints the latest `CHANGELOG.md` entry | --- diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 344b3d7..973d8e7 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -188,6 +188,7 @@ export { // selective per-domain network allowlist) export { wrapBashCommand, + withAdditionalWritableDirs, buildMacOsProfile, buildLinuxBwrapArgs, detectPlatform, diff --git a/packages/core/src/sandbox/additional-dirs.test.ts b/packages/core/src/sandbox/additional-dirs.test.ts new file mode 100644 index 0000000..e380370 --- /dev/null +++ b/packages/core/src/sandbox/additional-dirs.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from 'vitest'; +import { withAdditionalWritableDirs } from './index.js'; +import type { SandboxConfig } from '../config/types.js'; + +describe('withAdditionalWritableDirs', () => { + it('returns the config unchanged when there are no dirs', () => { + const c: SandboxConfig = { enabled: true }; + expect(withAdditionalWritableDirs(c, [])).toBe(c); + expect(withAdditionalWritableDirs(c, undefined)).toBe(c); + }); + + it('is a no-op (undefined) when the sandbox is off', () => { + expect(withAdditionalWritableDirs(undefined, ['/x'])).toBeUndefined(); + }); + + it('adds dirs to filesystem.allowWrite, deduped, without mutating the input', () => { + const c: SandboxConfig = { enabled: true, filesystem: { allowWrite: ['/a'] } }; + const r = withAdditionalWritableDirs(c, ['/a', '/b']); + expect(r?.filesystem?.allowWrite).toEqual(['/a', '/b']); + expect(c.filesystem?.allowWrite).toEqual(['/a']); // input untouched + }); + + it('seeds allowWrite when the config had none', () => { + const r = withAdditionalWritableDirs({ enabled: true }, ['/proj/sub']); + expect(r?.filesystem?.allowWrite).toEqual(['/proj/sub']); + }); +}); diff --git a/packages/core/src/sandbox/index.ts b/packages/core/src/sandbox/index.ts index ba481b6..ccf01ab 100644 --- a/packages/core/src/sandbox/index.ts +++ b/packages/core/src/sandbox/index.ts @@ -44,6 +44,21 @@ export interface SandboxedCommand { args: string[]; } +/** + * Return a sandbox config with `dirs` added to the writable filesystem roots — + * this is how `/add-dir` (settings.permissions.additionalDirectories) grants the + * sandboxed Bash tool write access beyond cwd. A no-op when the sandbox is off + * (undefined config) or there are no dirs. Pure — never mutates the input. + */ +export function withAdditionalWritableDirs( + config: SandboxConfig | undefined, + dirs: string[] | undefined, +): SandboxConfig | undefined { + if (!config || !dirs?.length) return config; + const allowWrite = [...new Set([...(config.filesystem?.allowWrite ?? []), ...dirs])]; + return { ...config, filesystem: { ...config.filesystem, allowWrite } }; +} + /** * Wrap a user-supplied shell command under platform sandbox. *