diff --git a/cli/src/cli-args.ts b/cli/src/cli-args.ts index 1ce73347cc..42c01de24c 100644 --- a/cli/src/cli-args.ts +++ b/cli/src/cli-args.ts @@ -18,6 +18,14 @@ export type ParsedArgs = { initialMode?: AgentMode /** Load repository `.agents` content without the trust prompt (CI opt-in). */ trustAgents: boolean + /** Disable shell command execution completely. */ + noShell: boolean + /** Ask for approval before every shell command. */ + askShell: boolean + /** Restrict agent to current project directory. */ + projectOnly: boolean + /** Read-only mode - deny write operations. */ + readOnly: boolean } export function loadPackageVersion(): string { @@ -67,6 +75,22 @@ export function parseArgs({ '--trust-agents', "Load this repository's .agents files and mcp.json without asking (for CI)", ) + .option( + '--no-shell', + 'Disable shell command execution (shell is enabled by default)', + ) + .option( + '--ask-shell', + 'Ask for approval before every shell command', + ) + .option( + '--project-only', + 'Restrict agent to current project directory', + ) + .option( + '--read-only', + 'Read-only mode - deny write operations', + ) .addArgument( new Argument('[command]', 'Command to run').choices(['login']), ) @@ -97,6 +121,22 @@ export function parseArgs({ '--trust-agents', "Load this repository's .agents files and mcp.json without asking (for CI)", ) + .option( + '--no-shell', + 'Disable shell command execution (shell is enabled by default)', + ) + .option( + '--ask-shell', + 'Ask for approval before every shell command', + ) + .option( + '--project-only', + 'Restrict agent to current project directory', + ) + .option( + '--read-only', + 'Read-only mode - deny write operations', + ) .option('--lite', 'Start in LITE mode') .option('--free', 'Start in LITE mode (deprecated alias)') .option('--max', 'Start in MAX mode') @@ -141,5 +181,10 @@ export function parseArgs({ cwd: options.cwd, initialMode, trustAgents: Boolean(options.trustAgents), + // commander --shell/--no-shell sets options.shell (default true when undefined) + noShell: options.shell === false, + askShell: Boolean(options.askShell), + projectOnly: Boolean(options.projectOnly), + readOnly: Boolean(options.readOnly), } } diff --git a/cli/src/hooks/use-ask-user-bridge.ts b/cli/src/hooks/use-ask-user-bridge.ts index 15ddac2eee..f3b191ffbb 100644 --- a/cli/src/hooks/use-ask-user-bridge.ts +++ b/cli/src/hooks/use-ask-user-bridge.ts @@ -1,4 +1,9 @@ import { AskUserBridge } from '@codebuff/common/utils/ask-user-bridge' +import { + subscribeShellApprovals, + submitShellApproval, + getActiveShellApproval, +} from '@codebuff/common/utils/shell-approval-bridge' import { useEffect } from 'react' import { useChatStore } from '../state/chat-store' @@ -49,11 +54,26 @@ function filterRedundantOptions(questions: AskUserQuestion[]): AskUserQuestion[] }) } +/** The single question the shell-approval prompt renders. */ +const SHELL_APPROVAL_QUESTIONS: AskUserQuestion[] = [ + { + question: 'Allow this shell command to run?', + header: 'Shell access', + options: [ + { label: 'Approve', description: 'Run the command once' }, + { label: 'Deny', description: 'Do not run the command' }, + ], + multiSelect: false, + }, +] + export function useAskUserBridge() { const setAskUserState = useChatStore((state) => state.setAskUserState) useEffect(() => { - const unsubscribe = AskUserBridge.subscribe((request) => { + const unsubAsk = AskUserBridge.subscribe((request) => { + // A shell approval owns the form while it is active; don't clobber it. + if (getActiveShellApproval()) return if (request) { // Filter out redundant "Custom"/"Other" options since UI provides its own const filteredQuestions = filterRedundantOptions(request.questions) @@ -68,7 +88,26 @@ export function useAskUserBridge() { setAskUserState(null) } }) - return unsubscribe + + // Surface shell-approval requests through the same form. The correlated + // bridge serializes them, so exactly one is active at a time. + const unsubShell = subscribeShellApprovals((active) => { + if (active) { + setAskUserState({ + toolCallId: active.id, + questions: SHELL_APPROVAL_QUESTIONS, + selectedAnswers: [-1], + otherTexts: [''], + }) + } else if (!AskUserBridge.getPendingRequest()) { + setAskUserState(null) + } + }) + + return () => { + unsubAsk() + unsubShell() + } }, [setAskUserState]) const submitAnswers = ( @@ -79,11 +118,24 @@ export function useAskUserBridge() { otherText?: string }> ) => { + // Route to the correlated shell-approval bridge when an approval is active. + const active = getActiveShellApproval() + if (active) { + const verdict = answers[0]?.selectedOption === 'Approve' ? 'approve' : 'deny' + submitShellApproval(active.id, verdict) + return + } // Don't clear input value - preserve user's input from before the questionnaire AskUserBridge.submit({ answers }) } const skip = () => { + const active = getActiveShellApproval() + if (active) { + // Skipping an approval is a denial, never an implicit approval. + submitShellApproval(active.id, 'deny') + return + } // Don't clear input value - preserve user's input from before the questionnaire AskUserBridge.submit({ skipped: true }) } diff --git a/cli/src/index.tsx b/cli/src/index.tsx index 65c7bcd52c..392c295fed 100644 --- a/cli/src/index.tsx +++ b/cli/src/index.tsx @@ -82,6 +82,7 @@ import { startClientEnvironmentProbe, } from './utils/client-environment' import { setOscDetectedTheme } from './utils/theme-system' +import { setParsedArgs } from './utils/parsed-args' import type { FileTreeNode } from '@codebuff/common/util/file' @@ -260,8 +261,29 @@ async function main(): Promise { cwd, initialMode, trustAgents, + noShell, + askShell, + projectOnly, + readOnly, } = parseArgs() + // Store parsed args for use by getCodebuffClient + setParsedArgs({ + initialPrompt, + command, + agent, + clearLogs, + continue: continueChat, + continueId, + cwd, + initialMode, + trustAgents, + noShell, + askShell, + projectOnly, + readOnly, + }) + const isLoginCommand = command === 'login' const isPublishCommand = command === 'publish' const hasAgentOverride = Boolean(agent?.trim()) diff --git a/cli/src/utils/codebuff-client.ts b/cli/src/utils/codebuff-client.ts index 236bfebbf3..d1afb5ae9a 100644 --- a/cli/src/utils/codebuff-client.ts +++ b/cli/src/utils/codebuff-client.ts @@ -1,6 +1,11 @@ import { API_KEY_ENV_VAR } from '@codebuff/common/old-constants' import { AskUserBridge } from '@codebuff/common/utils/ask-user-bridge' +import { + requestShellApproval as requestShellApprovalViaBridge, + cancelAllShellApprovals, +} from '@codebuff/common/utils/shell-approval-bridge' import { CodebuffClient } from '@codebuff/sdk' +import type { ShellApprovalCallback } from '@codebuff/sdk' import { getAuthTokenDetails } from './auth' import { getCliEnv, getSystemProcessEnv } from './env' @@ -11,6 +16,7 @@ import { createTraceWriter } from './trace-writer' import { terminalCommandBroker } from './terminal-command-broker' import { getRgPath } from '../native/ripgrep' import { getProjectRoot } from '../project-files' +import { getPermissionOptions } from './parsed-args' import type { ClientToolCall } from '@codebuff/common/tools/list' import type { ResolvedByokConnection } from '@codebuff/sdk' @@ -48,6 +54,8 @@ function removeUndefinedValues(obj: T): T { export function resetCodebuffClient(): void { clientInstance = null byokClientIdentity = null + // Deny any pending shell approvals so no caller hangs across a client reset. + cancelAllShellApprovals() } export async function getCodebuffClient( @@ -81,6 +89,24 @@ export async function getCodebuffClient( } } + // Get permission options from parsed CLI args + const permissionOptions = getPermissionOptions() + + // Shell approval callback bound to this run. Uses the correlated + // shell-approval bridge (serialized, timeout/abort-aware, id-matched) so a + // concurrent request can never be lost or misattributed. + const shellApproval: ShellApprovalCallback | undefined = + permissionOptions.askShell + ? (request) => + requestShellApprovalViaBridge({ + id: request.id, + command: request.command, + cwd: request.cwd, + processType: request.processType, + ...(request.signal ? { signal: request.signal } : {}), + }) + : undefined + try { const agentDefinitions = loadAgentDefinitions() clientInstance = new CodebuffClient({ @@ -105,6 +131,11 @@ export async function getCodebuffClient( logger, traceWriter: createTraceWriter(), terminalCommandBroker, + noShell: permissionOptions.noShell, + askShell: permissionOptions.askShell, + projectOnly: permissionOptions.projectOnly, + readOnly: permissionOptions.readOnly, + ...(shellApproval ? { shellApproval } : {}), overrideTools: { ask_user: async (input: ClientToolCall<'ask_user'>['input']) => { const askUserResponse = await AskUserBridge.request( diff --git a/cli/src/utils/parsed-args.ts b/cli/src/utils/parsed-args.ts new file mode 100644 index 0000000000..7a8d9464f8 --- /dev/null +++ b/cli/src/utils/parsed-args.ts @@ -0,0 +1,38 @@ +/** + * Module to store parsed CLI args for use across the application. + * This is populated once at startup in index.tsx before the app renders. + */ + +import type { ParsedArgs } from '../cli-args' + +let parsedArgs: ParsedArgs | null = null + +export function setParsedArgs(args: ParsedArgs): void { + parsedArgs = args +} + +export function getParsedArgs(): ParsedArgs | null { + return parsedArgs +} + +export function getPermissionOptions(): { + noShell: boolean + askShell: boolean + projectOnly: boolean + readOnly: boolean +} { + if (!parsedArgs) { + return { + noShell: false, + askShell: false, + projectOnly: false, + readOnly: false, + } + } + return { + noShell: parsedArgs.noShell, + askShell: parsedArgs.askShell, + projectOnly: parsedArgs.projectOnly, + readOnly: parsedArgs.readOnly, + } +} \ No newline at end of file diff --git a/common/src/utils/__tests__/shell-approval-bridge.test.ts b/common/src/utils/__tests__/shell-approval-bridge.test.ts new file mode 100644 index 0000000000..52643d4d3f --- /dev/null +++ b/common/src/utils/__tests__/shell-approval-bridge.test.ts @@ -0,0 +1,92 @@ +import { describe, test, expect, beforeEach } from 'bun:test' + +import { + requestShellApproval, + submitShellApproval, + cancelAllShellApprovals, + getActiveShellApproval, + __resetShellApprovalBridge, +} from '../shell-approval-bridge' + +function req(id: string, extra: Partial[0]> = {}) { + return { id, command: 'echo hi', cwd: '/tmp/p', processType: 'SYNC' as const, ...extra } +} + +describe('shell-approval-bridge', () => { + beforeEach(() => __resetShellApprovalBridge()) + + test('approve resolves only the matching id', async () => { + const p = requestShellApproval(req('a')) + expect(getActiveShellApproval()?.id).toBe('a') + expect(submitShellApproval('wrong', 'approve')).toBe(false) + expect(submitShellApproval('a', 'approve')).toBe(true) + expect(await p).toBe('approve') + }) + + test('deny resolves deny', async () => { + const p = requestShellApproval(req('a')) + submitShellApproval('a', 'deny') + expect(await p).toBe('deny') + }) + + test('timeout resolves deny without any submit', async () => { + const p = requestShellApproval(req('a'), { timeoutMs: 10 }) + expect(await p).toBe('deny') + }) + + test('abort resolves deny', async () => { + const controller = new AbortController() + const p = requestShellApproval(req('a', { signal: controller.signal })) + controller.abort() + expect(await p).toBe('deny') + }) + + test('already-aborted signal denies without queueing', async () => { + const controller = new AbortController() + controller.abort() + const p = requestShellApproval(req('a', { signal: controller.signal })) + expect(await p).toBe('deny') + expect(getActiveShellApproval()).toBeNull() + }) + + test('two concurrent requests: neither is lost; FIFO order', async () => { + const p1 = requestShellApproval(req('a')) + const p2 = requestShellApproval(req('b')) + // First is active, second queued. + expect(getActiveShellApproval()?.id).toBe('a') + submitShellApproval('a', 'approve') + expect(await p1).toBe('approve') + // Second becomes active after the first settles. + expect(getActiveShellApproval()?.id).toBe('b') + submitShellApproval('b', 'deny') + expect(await p2).toBe('deny') + }) + + test('a submit for a queued id is ignored (cannot approve a non-active request)', async () => { + const p1 = requestShellApproval(req('a')) + const p2 = requestShellApproval(req('b')) + expect(submitShellApproval('b', 'approve')).toBe(false) + submitShellApproval('a', 'approve') + await p1 + // b is now active and still unresolved until explicitly decided. + expect(submitShellApproval('b', 'approve')).toBe(true) + expect(await p2).toBe('approve') + }) + + test('cancelAll denies every pending request (UI shutdown)', async () => { + const p1 = requestShellApproval(req('a')) + const p2 = requestShellApproval(req('b')) + cancelAllShellApprovals() + expect(await p1).toBe('deny') + expect(await p2).toBe('deny') + expect(getActiveShellApproval()).toBeNull() + }) + + test('stale approval after settle does not re-resolve', async () => { + const p = requestShellApproval(req('a')) + submitShellApproval('a', 'deny') + expect(await p).toBe('deny') + // A late UI action for the same id is ignored. + expect(submitShellApproval('a', 'approve')).toBe(false) + }) +}) diff --git a/common/src/utils/shell-approval-bridge.ts b/common/src/utils/shell-approval-bridge.ts new file mode 100644 index 0000000000..a9db3a3756 --- /dev/null +++ b/common/src/utils/shell-approval-bridge.ts @@ -0,0 +1,160 @@ +/** + * Shell Approval Bridge + * + * A correlated, concurrency-safe bridge for shell-command approvals. + * + * The generic AskUserBridge holds a single `pendingRequest`, so a second + * concurrent request silently replaces the first and the first caller never + * resolves. That is unsafe for approvals: a lost request must never be treated + * as granted. This bridge: + * - correlates every request by a unique id, + * - serializes requests (the UI shows one at a time) in FIFO order, + * - resolves 'deny' on timeout, abort, or UI shutdown, + * - never resolves 'approve' unless an explicit submit says so. + */ + +export type ShellApprovalVerdict = 'approve' | 'deny' + +export interface ShellApprovalRequest { + id: string + command: string + cwd: string + processType: 'SYNC' | 'BACKGROUND' + signal?: AbortSignal +} + +export interface PendingShellApproval extends ShellApprovalRequest {} + +type Listener = (active: PendingShellApproval | null, queued: number) => void + +interface Entry { + request: ShellApprovalRequest + resolve: (verdict: ShellApprovalVerdict) => void + settled: boolean + timer: ReturnType | null + onAbort: (() => void) | null +} + +const DEFAULT_TIMEOUT_MS = 5 * 60 * 1000 + +let active: Entry | null = null +const queue: Entry[] = [] +const listeners: Listener[] = [] + +function notify() { + const snapshot = active ? { ...active.request } : null + for (const l of listeners) l(snapshot, queue.length) +} + +function settle(entry: Entry, verdict: ShellApprovalVerdict) { + if (entry.settled) return + entry.settled = true + if (entry.timer) clearTimeout(entry.timer) + if (entry.onAbort && entry.request.signal) { + entry.request.signal.removeEventListener('abort', entry.onAbort) + } + entry.resolve(verdict) +} + +function activateNext() { + while (queue.length > 0) { + const next = queue.shift()! + if (next.settled) continue + if (next.request.signal?.aborted) { + settle(next, 'deny') + continue + } + active = next + notify() + return + } + active = null + notify() +} + +/** + * Request approval for one command. Resolves 'approve' only when the matching + * id is submitted with that verdict; otherwise 'deny' (timeout/abort/shutdown). + */ +export function requestShellApproval( + request: ShellApprovalRequest, + options: { timeoutMs?: number } = {}, +): Promise { + const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS + return new Promise((resolve) => { + const entry: Entry = { + request, + resolve, + settled: false, + timer: null, + onAbort: null, + } + + // If the run is already aborted, deny without queueing. + if (request.signal?.aborted) { + entry.settled = true + resolve('deny') + return + } + + if (request.signal) { + entry.onAbort = () => settle(entry, 'deny') + request.signal.addEventListener('abort', entry.onAbort, { once: true }) + } + + entry.timer = setTimeout(() => settle(entry, 'deny'), timeoutMs) + entry.timer.unref?.() + + if (active) { + queue.push(entry) + notify() + } else { + active = entry + notify() + } + }) +} + +/** + * Submit a verdict for a specific approval id. Ids that do not match the active + * request are ignored, so a stale UI action cannot approve a different command. + */ +export function submitShellApproval( + id: string, + verdict: ShellApprovalVerdict, +): boolean { + if (!active || active.request.id !== id || active.settled) return false + const current = active + settle(current, verdict) + activateNext() + return true +} + +/** Deny every pending request. Called on UI shutdown so no caller hangs. */ +export function cancelAllShellApprovals(): void { + if (active) settle(active, 'deny') + for (const entry of queue) settle(entry, 'deny') + queue.length = 0 + active = null + notify() +} + +export function getActiveShellApproval(): PendingShellApproval | null { + return active ? { ...active.request } : null +} + +export function subscribeShellApprovals(listener: Listener): () => void { + listeners.push(listener) + listener(active ? { ...active.request } : null, queue.length) + return () => { + const idx = listeners.indexOf(listener) + if (idx !== -1) listeners.splice(idx, 1) + } +} + +/** Test-only: reset all state. */ +export function __resetShellApprovalBridge(): void { + active = null + queue.length = 0 + listeners.length = 0 +} diff --git a/sdk/src/index.ts b/sdk/src/index.ts index 5a64b4da7c..fbb39d72bd 100644 --- a/sdk/src/index.ts +++ b/sdk/src/index.ts @@ -147,6 +147,24 @@ export type { TerminalCommandProcess, TerminalCommandSpawnRequest, } from './tools/run-terminal-command' +export { + createExecutionPolicy, + authorizeToolCall, + classifyTool, + guardFileSystem, + canonicalizePath, + isPathWithinRoot, + isLexicallyWithinRoot, +} from './permissions/execution-policy' +export type { + ExecutionPolicy, + ShellApprovalCallback, + ShellApprovalRequest, + ShellApprovalVerdict, + ToolCapability, + ToolAuthorizationRequest, + PolicyDecision, +} from './permissions/execution-policy' // Containment for a sponsored run on the user's own machine (COD-336). Exported // from the SDK rather than kept in Desktop because the CLI needs the same // boundary, and a second copy of a seatbelt profile is a second thing to be diff --git a/sdk/src/permissions/__tests__/execution-policy.test.ts b/sdk/src/permissions/__tests__/execution-policy.test.ts new file mode 100644 index 0000000000..ffd1e927d8 --- /dev/null +++ b/sdk/src/permissions/__tests__/execution-policy.test.ts @@ -0,0 +1,312 @@ +/** + * Adversarial regression tests for the execution-permission policy (issue #1450). + * + * These encode the security properties the policy must hold. They test the + * policy module directly (no network, no real processes), including the + * cross-run isolation property that a process-global policy would violate. + */ + +import { describe, test, expect } from 'bun:test' +import { mkdtempSync, mkdirSync, symlinkSync, writeFileSync, rmSync } from 'fs' +import { tmpdir } from 'os' +import { join } from 'path' + +import { + createExecutionPolicy, + authorizeToolCall, + classifyTool, + guardFileSystem, + canonicalizePath, + isPathWithinRoot, + isContainedPath, +} from '../execution-policy' + +import type { CodebuffFileSystem } from '@codebuff/common/types/filesystem' + +function policy(overrides: Partial[0]> = {}) { + return createExecutionPolicy({ projectRoot: '/tmp/project', ...overrides }) +} + +describe('classifyTool', () => { + test('classifies shell, fs, network, and unknown tools', () => { + expect(classifyTool('run_terminal_command')).toBe('shell') + expect(classifyTool('write_file')).toBe('fs-write') + expect(classifyTool('apply_patch')).toBe('fs-write') + expect(classifyTool('read_files')).toBe('fs-read') + expect(classifyTool('read_url')).toBe('network') + // Unknown names (MCP, custom tools) are external, never assumed safe. + expect(classifyTool('mcp__github__create_issue')).toBe('external') + expect(classifyTool('my_custom_tool')).toBe('external') + }) +}) + +describe('per-run isolation (no process-global policy)', () => { + test('a permissive run does not leak permissions into a restricted run', () => { + const restricted = createExecutionPolicy({ + projectRoot: '/tmp/a', + readOnly: true, + noShell: true, + }) + const permissive = createExecutionPolicy({ projectRoot: '/tmp/b' }) + + // Building the permissive policy must not affect the restricted one. + expect( + authorizeToolCall(restricted, { + toolName: 'run_terminal_command', + capability: 'shell', + command: 'rm -rf /', + cwd: '/tmp/a', + }).allowed, + ).toBe(false) + expect( + authorizeToolCall(restricted, { + toolName: 'write_file', + capability: 'fs-write', + targetPath: '/tmp/a/x.ts', + }).allowed, + ).toBe(false) + // The permissive policy is independent. + expect( + authorizeToolCall(permissive, { + toolName: 'write_file', + capability: 'fs-write', + targetPath: '/tmp/b/x.ts', + }).allowed, + ).toBe(true) + }) +}) + +describe('shell policy', () => { + test('--no-shell denies unconditionally', () => { + const p = policy({ noShell: true }) + const d = authorizeToolCall(p, { toolName: 'run_terminal_command', capability: 'shell', command: 'echo hi', cwd: '/tmp/project' }) + expect(d.allowed).toBe(false) + }) + + test('--read-only denies shell (unrestricted shell can write)', () => { + const p = policy({ readOnly: true }) + const d = authorizeToolCall(p, { toolName: 'run_terminal_command', capability: 'shell', command: 'echo hi > /etc/passwd', cwd: '/tmp/project' }) + expect(d.allowed).toBe(false) + }) + + test('--project-only denies shell (unrestricted shell can escape)', () => { + const p = policy({ projectOnly: true }) + const d = authorizeToolCall(p, { toolName: 'run_terminal_command', capability: 'shell', command: 'cat ~/.ssh/id_rsa', cwd: '/tmp/project' }) + expect(d.allowed).toBe(false) + }) + + test('--ask-shell with no handler is denied', () => { + const p = policy({ askShell: true }) + const d = authorizeToolCall(p, { toolName: 'run_terminal_command', capability: 'shell', command: 'echo hi', cwd: '/tmp/project' }) + expect(d.allowed).toBe(false) + expect(d.approvalRequired).toBe(true) + }) + + test('--ask-shell with handler requires approval (not auto-allow)', () => { + const p = policy({ askShell: true, shellApproval: async () => 'deny' }) + const d = authorizeToolCall(p, { toolName: 'run_terminal_command', capability: 'shell', command: 'echo hi', cwd: '/tmp/project' }) + expect(d.allowed).toBe(false) + expect(d.approvalRequired).toBe(true) + }) + + test('background commands are gated identically to sync (approval before spawn)', () => { + const p = policy({ askShell: true, shellApproval: async () => 'deny' }) + const d = authorizeToolCall(p, { + toolName: 'run_terminal_command', + capability: 'shell', + command: 'npm run dev', + cwd: '/tmp/project', + processType: 'BACKGROUND', + }) + expect(d.allowed).toBe(false) + expect(d.approvalRequired).toBe(true) + }) + + test('background commands denied under read-only/no-shell', () => { + for (const flag of [{ readOnly: true }, { noShell: true }, { projectOnly: true }] as const) { + const p = policy(flag) + const d = authorizeToolCall(p, { + toolName: 'run_terminal_command', + capability: 'shell', + command: 'npm run dev', + cwd: '/tmp/project', + processType: 'BACKGROUND', + }) + expect(d.allowed).toBe(false) + } + }) +}) + +describe('filesystem write policy', () => { + test('--read-only denies writes regardless of target', () => { + const p = policy({ readOnly: true }) + expect(authorizeToolCall(p, { toolName: 'write_file', capability: 'fs-write', targetPath: '/tmp/project/a.ts' }).allowed).toBe(false) + expect(authorizeToolCall(p, { toolName: 'apply_patch', capability: 'fs-write', targetPath: '/tmp/project/b.ts' }).allowed).toBe(false) + }) + + test('--project-only denies writes outside the root', () => { + const p = policy({ projectOnly: true }) + expect(authorizeToolCall(p, { toolName: 'write_file', capability: 'fs-write', targetPath: '/tmp/project/a.ts' }).allowed).toBe(true) + expect(authorizeToolCall(p, { toolName: 'write_file', capability: 'fs-write', targetPath: '/tmp/elsewhere/a.ts' }).allowed).toBe(false) + // Prefix that merely starts with the root string is not containment. + expect(authorizeToolCall(p, { toolName: 'write_file', capability: 'fs-write', targetPath: '/tmp/project-evil/a.ts' }).allowed).toBe(false) + }) + + test('--project-only denies reads outside the root', () => { + const p = policy({ projectOnly: true }) + expect(authorizeToolCall(p, { toolName: 'read_files', capability: 'fs-read', targetPath: '/tmp/project/a.ts' }).allowed).toBe(true) + expect(authorizeToolCall(p, { toolName: 'read_files', capability: 'fs-read', targetPath: '/etc/passwd' }).allowed).toBe(false) + }) +}) + +describe('platform-correct containment (Windows branch)', () => { + const win = { caseInsensitive: true, sep: '\\' } + test('case-insensitive match on Windows accepts a same-path different-case child', () => { + expect(isContainedPath('C:\\Proj\\a.ts', 'C:\\proj', win)).toBe(true) + expect(isContainedPath('C:\\PROJ', 'C:\\proj', win)).toBe(true) + }) + test('a case-variant sibling outside the root is still rejected', () => { + expect(isContainedPath('C:\\proj-evil\\a.ts', 'C:\\proj', win)).toBe(false) + }) + test('Windows separator prevents the prefix-sibling false positive', () => { + // Without the trailing separator, C:\project would match root C:\proj. + expect(isContainedPath('C:\\project\\a.ts', 'C:\\proj', win)).toBe(false) + }) + test('POSIX is case-sensitive', () => { + const posix = { caseInsensitive: false, sep: '/' } + expect(isContainedPath('/tmp/Proj/a.ts', '/tmp/proj', posix)).toBe(false) + expect(isContainedPath('/tmp/proj/a.ts', '/tmp/proj', posix)).toBe(true) + }) +}) + +describe('external tools under strict policies', () => { + test('denied when any restriction is active and not trusted', () => { + for (const flag of [{ noShell: true }, { askShell: true }, { projectOnly: true }, { readOnly: true }] as const) { + const p = policy(flag) + expect(authorizeToolCall(p, { toolName: 'mcp__x__y', capability: 'external' }).allowed).toBe(false) + } + }) + + test('allowed when the host explicitly trusts external tools', () => { + const p = policy({ readOnly: true, allowExternalTools: true }) + expect(authorizeToolCall(p, { toolName: 'mcp__x__y', capability: 'external' }).allowed).toBe(true) + }) + + test('allowed when no restriction is active', () => { + const p = policy() + expect(authorizeToolCall(p, { toolName: 'mcp__x__y', capability: 'external' }).allowed).toBe(true) + }) +}) + +describe('symlink-aware containment', () => { + test('a symlink inside the root that points outside is not contained', async () => { + const base = mkdtempSync(join(tmpdir(), 'policy-test-')) + try { + const root = join(base, 'project') + const outside = join(base, 'outside') + mkdirSync(root) + mkdirSync(outside) + writeFileSync(join(outside, 'secret.txt'), 'top secret') + // root/link -> outside + symlinkSync(outside, join(root, 'link')) + + // Lexically it looks inside the root... + const lexicalTarget = join(root, 'link', 'secret.txt') + // ...but canonicalization resolves the symlink out of the root. + const contained = await isPathWithinRoot(lexicalTarget, root) + expect(contained).toBe(false) + } finally { + rmSync(base, { recursive: true, force: true }) + } + }) + + test('a real file inside the root is contained', async () => { + const base = mkdtempSync(join(tmpdir(), 'policy-test-')) + try { + const root = join(base, 'project') + mkdirSync(root) + writeFileSync(join(root, 'a.ts'), 'x') + expect(await isPathWithinRoot(join(root, 'a.ts'), root)).toBe(true) + expect(await isPathWithinRoot(root, root)).toBe(true) + } finally { + rmSync(base, { recursive: true, force: true }) + } + }) + + test('canonicalizePath resolves a symlinked ancestor', async () => { + const base = mkdtempSync(join(tmpdir(), 'policy-test-')) + try { + const real = join(base, 'real') + mkdirSync(real) + const link = join(base, 'link') + symlinkSync(real, link) + // A not-yet-existing file under the link canonicalizes under `real`. + const canonical = await canonicalizePath(join(link, 'new.txt')) + expect(canonical.startsWith(real)).toBe(true) + } finally { + rmSync(base, { recursive: true, force: true }) + } + }) +}) + +describe('guardFileSystem (defense in depth)', () => { + function mockFs(seen: { reads: string[]; writes: string[] }): CodebuffFileSystem { + return { + readFile: async (p: any) => { + seen.reads.push(String(p)) + return 'data' + }, + writeFile: async (p: any) => { + seen.writes.push(String(p)) + return undefined + }, + readdir: async (p: any) => { + seen.reads.push(String(p)) + return [] + }, + stat: async (p: any) => { + seen.reads.push(String(p)) + return {} as any + }, + mkdir: async (p: any) => { + seen.writes.push(String(p)) + return undefined + }, + unlink: async (p: any) => { + seen.writes.push(String(p)) + return undefined + }, + } as unknown as CodebuffFileSystem + } + + test('read-only blocks every write through the adapter', async () => { + const seen = { reads: [] as string[], writes: [] as string[] } + const guarded = guardFileSystem(mockFs(seen), policy({ readOnly: true })) + await expect(guarded.writeFile('/tmp/project/a.ts', 'x')).rejects.toThrow() + expect(seen.writes.length).toBe(0) + }) + + test('project-only blocks writes and reads outside the root', async () => { + const base = mkdtempSync(join(tmpdir(), 'policy-fs-')) + try { + const root = join(base, 'project') + mkdirSync(root) + const seen = { reads: [] as string[], writes: [] as string[] } + const guarded = guardFileSystem(mockFs(seen), policy({ projectOnly: true, projectRoot: root })) + await expect(guarded.writeFile(join(base, 'outside.ts'), 'x')).rejects.toThrow() + await expect(guarded.readFile('/etc/passwd')).rejects.toThrow() + // Inside the root is allowed. + await guarded.writeFile(join(root, 'a.ts'), 'x') + expect(seen.writes).toEqual([join(root, 'a.ts')]) + } finally { + rmSync(base, { recursive: true, force: true }) + } + }) + + test('unrestricted policy passes the adapter through untouched', () => { + const seen = { reads: [] as string[], writes: [] as string[] } + const raw = mockFs(seen) + const guarded = guardFileSystem(raw, policy()) + expect(guarded).toBe(raw) + }) +}) diff --git a/sdk/src/permissions/execution-policy.ts b/sdk/src/permissions/execution-policy.ts new file mode 100644 index 0000000000..655a8701ea --- /dev/null +++ b/sdk/src/permissions/execution-policy.ts @@ -0,0 +1,388 @@ +/** + * Execution Policy + * + * Per-run, immutable authorization for tool execution. + * + * There is deliberately NO module-global policy state. A global policy is a + * security defect in an embeddable SDK: two concurrent runs with different + * restrictions would overwrite each other, so a restricted run could inherit a + * permissive run's policy. Each run constructs its own policy and captures it + * in its tool-dispatch closure. + */ + +import { promises as nodeFs } from 'fs' +import { resolve, sep as sep_default } from 'path' + +import type { CodebuffFileSystem } from '@codebuff/common/types/filesystem' + +/** What a tool can do, for authorization purposes. */ +export type ToolCapability = + | 'shell' + | 'fs-read' + | 'fs-write' + | 'network' + | 'agent' + | 'external' + | 'inert' + +export type ShellApprovalVerdict = 'approve' | 'deny' + +export interface ShellApprovalRequest { + /** Unique id correlating this approval to exactly one command. */ + id: string + command: string + cwd: string + processType: 'SYNC' | 'BACKGROUND' + signal?: AbortSignal +} + +export type ShellApprovalCallback = ( + request: ShellApprovalRequest, +) => Promise + +export interface ExecutionPolicy { + readonly noShell: boolean + readonly askShell: boolean + readonly projectOnly: boolean + readonly readOnly: boolean + /** Absolute, canonicalized project root this run is confined to. */ + readonly projectRoot: string + /** + * Whether external tools (MCP servers, custom tool definitions, host + * overrides) may run. Default false: they are outside the SDK's enforcement + * boundary and must be explicitly trusted by the host. + */ + readonly allowExternalTools: boolean + readonly shellApproval?: ShellApprovalCallback +} + +export interface PolicyDecision { + allowed: boolean + reason?: string + approvalRequired?: boolean +} + +export interface ToolAuthorizationRequest { + toolName: string + capability: ToolCapability + /** Resolved absolute target path, for filesystem tools. */ + targetPath?: string + /** Shell command details, for the shell capability. */ + command?: string + cwd?: string + processType?: 'SYNC' | 'BACKGROUND' + signal?: AbortSignal +} + +/** + * Build an immutable policy for one run. + */ +export function createExecutionPolicy(input: { + projectRoot: string + noShell?: boolean + askShell?: boolean + projectOnly?: boolean + readOnly?: boolean + allowExternalTools?: boolean + shellApproval?: ShellApprovalCallback +}): ExecutionPolicy { + return Object.freeze({ + noShell: input.noShell ?? false, + askShell: input.askShell ?? false, + projectOnly: input.projectOnly ?? false, + readOnly: input.readOnly ?? false, + projectRoot: resolve(input.projectRoot), + allowExternalTools: input.allowExternalTools ?? false, + ...(input.shellApproval ? { shellApproval: input.shellApproval } : {}), + }) +} + +/** + * Classify a tool by capability. Unknown names are treated as `external` + * (denied under strict policies) rather than assumed safe. + */ +export function classifyTool(toolName: string): ToolCapability { + switch (toolName) { + case 'run_terminal_command': + return 'shell' + case 'write_file': + case 'str_replace': + case 'apply_patch': + return 'fs-write' + case 'read_files': + case 'read_subtree': + case 'list_directory': + case 'glob': + case 'code_search': + return 'fs-read' + case 'read_url': + case 'web_search': + case 'read_docs': + return 'network' + case 'spawn_agents': + case 'spawn_agent_inline': + return 'agent' + case 'think_deeply': + case 'set_output': + case 'end_turn': + case 'add_message': + case 'ask_user': + case 'create_plan': + case 'write_todos': + case 'suggest_followups': + case 'task_completed': + case 'update_subgoal': + case 'add_subgoal': + case 'render_ui': + case 'report_project_profile': + case 'lookup_agent_info': + case 'skill': + return 'inert' + default: + // MCP tools, custom tool definitions, and anything unrecognized. + return 'external' + } +} + +/** + * Canonicalize a path through symlinks by resolving the nearest existing + * ancestor with realpath, then re-appending the not-yet-existing tail. This + * catches a symlink inside the project that points outside it. + */ +export async function canonicalizePath( + targetPath: string, + realpath: (p: string) => Promise = nodeFs.realpath, +): Promise { + const absolute = resolve(targetPath) + const segments: string[] = [] + let current = absolute + for (;;) { + try { + const real = await realpath(current) + return segments.length === 0 ? real : resolve(real, ...segments.reverse()) + } catch { + const parent = resolve(current, '..') + if (parent === current) { + // Reached the filesystem root without finding an existing ancestor. + return absolute + } + segments.push(current.slice(parent.length + 1)) + current = parent + } + } +} + +/** + * Symlink-aware containment check: true only when the canonicalized target is + * the canonicalized root or a descendant of it. + */ +export async function isPathWithinRoot( + targetPath: string, + root: string, + realpath: (p: string) => Promise = nodeFs.realpath, +): Promise { + const [realTarget, realRoot] = await Promise.all([ + canonicalizePath(targetPath, realpath), + canonicalizePath(root, realpath), + ]) + return isContainedPath(realTarget, realRoot) +} + +/** + * Platform-correct containment over already-normalized paths. + * + * Windows paths are case-insensitive and use backslash separators, so a + * byte-prefix comparison would wrongly reject `C:\Proj\a.ts` against root + * `C:\proj` (or wrongly accept a sibling differing only in case). The flags are + * explicit so the Windows branch is unit-testable on any host. + */ +export function isContainedPath( + target: string, + root: string, + options: { caseInsensitive: boolean; sep: string } = { + caseInsensitive: process.platform === 'win32', + sep: process.platform === 'win32' ? '\\' : sep_default, + }, +): boolean { + const eq = (a: string, b: string) => + options.caseInsensitive ? a.toLowerCase() === b.toLowerCase() : a === b + const startsWith = (a: string, b: string) => + options.caseInsensitive + ? a.toLowerCase().startsWith(b.toLowerCase()) + : a.startsWith(b) + return eq(target, root) || startsWith(target, root + options.sep) +} + +/** Lexical containment check (no filesystem access). Used as a fast pre-filter. */ +export function isLexicallyWithinRoot(targetPath: string, root: string): boolean { + return isContainedPath(resolve(targetPath), resolve(root)) +} + +/** + * Authorize one tool call against one run's policy. Pure with respect to + * policy state: everything it needs is passed in. Filesystem containment is + * lexical here; callers that have a resolved target should additionally await + * {@link isPathWithinRoot} for symlink safety. + */ +export function authorizeToolCall( + policy: ExecutionPolicy, + request: ToolAuthorizationRequest, +): PolicyDecision { + const { capability } = request + + // External tools (MCP, custom definitions, overrides) are outside the SDK's + // enforcement boundary. Under any strict policy they are denied unless the + // host explicitly trusted them. + if (capability === 'external') { + const strict = + policy.noShell || policy.askShell || policy.projectOnly || policy.readOnly + if (strict && !policy.allowExternalTools) { + return { + allowed: false, + reason: `Tool '${request.toolName}' is an external capability and is not permitted while execution restrictions are active`, + } + } + return { allowed: true } + } + + if (capability === 'shell') { + if (policy.noShell) { + return { + allowed: false, + reason: 'Shell command execution is disabled (--no-shell)', + } + } + // read-only and project-only cannot be guaranteed while an unrestricted + // shell can write anywhere or read anywhere. Deny until real OS-level + // containment is active. + if (policy.readOnly) { + return { + allowed: false, + reason: + 'Shell execution is not permitted in read-only mode: an unrestricted shell can modify files outside the SDK’s control', + } + } + if (policy.projectOnly) { + return { + allowed: false, + reason: + 'Shell execution is not permitted in project-only mode: an unrestricted shell can access paths outside the project', + } + } + if (policy.askShell) { + if (!policy.shellApproval) { + return { + allowed: false, + reason: + 'Shell command requires approval but no approval handler is configured', + approvalRequired: true, + } + } + return { allowed: false, reason: 'Approval required', approvalRequired: true } + } + return { allowed: true } + } + + if (capability === 'fs-write') { + if (policy.readOnly) { + return { + allowed: false, + reason: `Write to '${request.targetPath ?? request.toolName}' denied: read-only mode (--read-only)`, + } + } + if (policy.projectOnly && request.targetPath) { + if (!isLexicallyWithinRoot(request.targetPath, policy.projectRoot)) { + return { + allowed: false, + reason: `Write to '${request.targetPath}' blocked: outside project root '${policy.projectRoot}' (--project-only)`, + } + } + } + return { allowed: true } + } + + if (capability === 'fs-read') { + if (policy.projectOnly && request.targetPath) { + if (!isLexicallyWithinRoot(request.targetPath, policy.projectRoot)) { + return { + allowed: false, + reason: `Read of '${request.targetPath}' blocked: outside project root '${policy.projectRoot}' (--project-only)`, + } + } + } + return { allowed: true } + } + + // network, agent, inert + return { allowed: true } +} + +/** + * Filesystem adapter wrapper enforcing read/write containment for a run. + * Defense in depth: even a tool path the dispatcher missed cannot read or + * write outside the policy's root, and symlink escapes are caught via realpath. + */ +export function guardFileSystem( + fs: CodebuffFileSystem, + policy: ExecutionPolicy, + realpath?: (p: string) => Promise, +): CodebuffFileSystem { + const restricted = policy.projectOnly || policy.readOnly + if (!restricted) return fs + + const assertReadable = async (target: string) => { + if (!policy.projectOnly) return + if (!(await isPathWithinRoot(target, policy.projectRoot, realpath))) { + throw new Error( + `Read of '${target}' blocked: outside project root '${policy.projectRoot}' (--project-only)`, + ) + } + } + const assertWritable = async (target: string) => { + if (policy.readOnly) { + throw new Error(`Write to '${target}' denied: read-only mode (--read-only)`) + } + if (!policy.projectOnly) return + if (!(await isPathWithinRoot(target, policy.projectRoot, realpath))) { + throw new Error( + `Write to '${target}' blocked: outside project root '${policy.projectRoot}' (--project-only)`, + ) + } + } + + const wrapRead = any>(fn: T): T => + (async (target: string, ...rest: unknown[]) => { + await assertReadable(target) + return fn(target, ...rest) + }) as unknown as T + const wrapWrite = any>(fn: T): T => + (async (target: string, ...rest: unknown[]) => { + await assertWritable(target) + return fn(target, ...rest) + }) as unknown as T + + return new Proxy(fs as object, { + get(obj, prop, receiver) { + const value = Reflect.get(obj, prop, receiver) + if (typeof value !== 'function') return value + switch (prop) { + case 'readFile': + case 'readdir': + case 'stat': + case 'lstat': + case 'access': + return wrapRead(value.bind(obj)) + case 'writeFile': + case 'unlink': + case 'rm': + case 'rmdir': + case 'mkdir': + case 'appendFile': + case 'rename': + return wrapWrite(value.bind(obj)) + default: + return value.bind(obj) + } + }, + }) as CodebuffFileSystem +} diff --git a/sdk/src/run.ts b/sdk/src/run.ts index ede227775a..47bfc8521d 100644 --- a/sdk/src/run.ts +++ b/sdk/src/run.ts @@ -59,6 +59,16 @@ import { getFiles, getImageFile } from './tools/read-files' import { readUrl } from './tools/read-url' import { runTerminalCommand } from './tools/run-terminal-command' import type { TerminalCommandBroker } from './tools/run-terminal-command' +import { + createExecutionPolicy, + authorizeToolCall, + classifyTool, + guardFileSystem, +} from './permissions/execution-policy' +import type { + ExecutionPolicy, + ShellApprovalCallback, +} from './permissions/execution-policy' import type { CustomToolDefinition } from './custom-tool' import type { CodebuffRequestHeadersProvider } from './impl/model-provider' @@ -231,6 +241,15 @@ export type CodebuffClientOptions = { /** Optional filter to classify files before reading (runs before gitignore check) */ fileFilter?: FileFilter + /** Disable shell command execution completely. */ + noShell?: boolean + /** Ask for approval before every shell command. */ + askShell?: boolean + /** Restrict agent to current project directory. */ + projectOnly?: boolean + /** Read-only mode - deny write operations. */ + readOnly?: boolean + overrideTools?: OverrideToolHandlers customToolDefinitions?: CustomToolDefinition[] @@ -318,6 +337,25 @@ export type RunOptions = { * know a new conversation's trace session before its first run returns. * Ignored when `previousRun` carries one. Unset mints a random UUID. */ traceSessionId?: string + /** Disable shell command execution completely. */ + noShell?: boolean + /** Ask for approval before every shell command. */ + askShell?: boolean + /** Restrict agent to current project directory. */ + projectOnly?: boolean + /** Read-only mode - deny write operations. */ + readOnly?: boolean + /** + * Host-provided approval handler for `askShell`. Bound to the run; receives + * a unique id per request and must resolve 'approve' or 'deny'. + */ + shellApproval?: ShellApprovalCallback + /** + * Permit external tools (MCP servers, custom tool definitions, host + * overrides) while execution restrictions are active. Off by default: those + * tools run outside the SDK's enforcement boundary. + */ + allowExternalTools?: boolean } /** @@ -582,9 +620,16 @@ async function runOnce({ terminalShell, projectKey, traceSessionId: traceSessionIdOption, + // Permission options + noShell = false, + askShell = false, + projectOnly = false, + readOnly = false, + shellApproval, + allowExternalTools = false, }: RunExecutionOptions): Promise { const fsSourceValue = typeof fsSource === 'function' ? fsSource() : fsSource - const fs = await fsSourceValue + const rawFs = await fsSourceValue let spawn: CodebuffSpawn if (spawnSource) { const spawnSourceValue = await spawnSource @@ -597,6 +642,21 @@ async function runOnce({ // only closed over by the model implementation below. const toolEnv = byok ? scrubByokCredentialEnv(env, byok.apiKey) : env + // One immutable policy for THIS run. Captured in the tool-dispatch closure + // below so concurrent runs cannot influence each other's permissions. + const policy = createExecutionPolicy({ + projectRoot: cwd ?? process.cwd(), + noShell, + askShell, + projectOnly, + readOnly, + allowExternalTools, + ...(shellApproval ? { shellApproval } : {}), + }) + // Defense in depth: every filesystem tool call goes through an adapter that + // enforces read/write containment even if a dispatcher branch missed a check. + const fs = guardFileSystem(rawFs, policy) + // Init session state let agentId if (typeof agent !== 'string') { @@ -823,6 +883,7 @@ async function runOnce({ apiKey, byok, signal, + policy, }) }, requestMcpToolData: async ({ mcpConfig, toolNames }) => { @@ -843,27 +904,51 @@ async function runOnce({ return filteredTools }, - requestFiles: ({ filePaths, fileWindows }) => - readFiles({ + requestFiles: ({ filePaths, fileWindows }) => { + // fs-adapter guard enforces containment on the actual reads below; the + // lexical pre-check here fails fast with a clear reason. + for (const filePath of filePaths) { + const decision = authorizeToolCall(policy, { + toolName: 'read_files', + capability: 'fs-read', + targetPath: path.resolve(requireCwd(cwd, 'read_files'), filePath), + }) + if (!decision.allowed) throw new Error(decision.reason) + } + return readFiles({ filePaths, fileWindows, override: overrideTools?.read_files, fileFilter, cwd, fs, - }), + }) + }, // Only a local read can return an image's bytes. An override reads a // remote workspace through the text-only contract, so it keeps images. requestImageFile: overrideTools?.read_files ? undefined - : ({ filePath }) => - getImageFile({ + : ({ filePath }) => { + const decision = authorizeToolCall(policy, { + toolName: 'read_files', + capability: 'fs-read', + targetPath: path.resolve(requireCwd(cwd, 'read_files'), filePath), + }) + if (!decision.allowed) throw new Error(decision.reason) + return getImageFile({ filePath, cwd: requireCwd(cwd, 'read_files'), fs, fileFilter, - }), + }) + }, requestOptionalFile: async ({ filePath }) => { + const decision = authorizeToolCall(policy, { + toolName: 'read_files', + capability: 'fs-read', + targetPath: path.resolve(requireCwd(cwd, 'read_files'), filePath), + }) + if (!decision.allowed) throw new Error(decision.reason) const files = await readFiles({ filePaths: [filePath], override: overrideTools?.read_files, @@ -1227,6 +1312,7 @@ async function handleToolCall({ apiKey, byok, signal, + policy, }: { action: ServerAction<'tool-call-request'> overrides: NonNullable @@ -1238,6 +1324,7 @@ async function handleToolCall({ apiKey: string byok?: ResolvedByokConnection signal?: AbortSignal + policy: ExecutionPolicy }): Promise<{ output: ToolResultOutput[] }> { const toolName = action.toolName const input = action.input @@ -1264,6 +1351,87 @@ async function handleToolCall({ } } + const denial = (message: string) => ({ + output: [{ type: 'json' as const, value: { errorMessage: message } }], + }) + + // ------------------------------------------------------------------------- + // Centralized authorization. EVERY dispatch path below — MCP, custom tools, + // host overrides, and built-in tools — is authorized here first, so none can + // bypass the run's policy. + // ------------------------------------------------------------------------- + const capability = classifyTool(toolName) + + if (capability === 'shell') { + const commandCwd = path.resolve( + requireCwd(cwd, 'run_terminal_command'), + (input as { cwd?: string }).cwd ?? '.', + ) + const command = (input as { command?: string }).command ?? '' + const processType = (input as { process_type?: 'SYNC' | 'BACKGROUND' }) + .process_type ?? 'SYNC' + const decision = authorizeToolCall(policy, { + toolName, + capability, + command, + cwd: commandCwd, + processType, + signal, + }) + if (!decision.allowed) { + if (decision.approvalRequired) { + if (!policy.shellApproval) { + return denial('Shell command requires approval but no approval handler is configured') + } + let verdict: 'approve' | 'deny' = 'deny' + try { + verdict = await policy.shellApproval({ + id: crypto.randomUUID(), + command, + cwd: commandCwd, + processType, + signal, + }) + } catch { + verdict = 'deny' + } + // Re-check abort AFTER approval: the user may have stopped the run + // while the prompt was open. + if (verdict !== 'approve' || signal?.aborted) { + return denial('Shell command denied') + } + } else { + return denial(decision.reason ?? 'Shell command blocked by policy') + } + } + } else if (capability === 'fs-write' || capability === 'fs-read') { + // Resolve every path the tool will touch and authorize each one. apply_patch + // carries its target under `operation.path`, matching applyPatchTool(). + const targets: string[] = [] + if (toolName === 'apply_patch') { + const op = (input as { operation?: { path?: string } }).operation + if (op?.path) { + targets.push(path.resolve(requireCwd(cwd, toolName), op.path)) + } + } else if (Array.isArray((input as { filePaths?: string[] }).filePaths)) { + for (const fp of (input as { filePaths: string[] }).filePaths) { + targets.push(path.resolve(requireCwd(cwd, toolName), fp)) + } + } else if (typeof (input as { path?: string }).path === 'string') { + targets.push(path.resolve(requireCwd(cwd, toolName), (input as { path: string }).path)) + } else { + // Directory-scoped tools (code_search, glob) act on the project root. + targets.push(requireCwd(cwd, toolName)) + } + for (const targetPath of targets) { + const decision = authorizeToolCall(policy, { toolName, capability, targetPath }) + if (!decision.allowed) return denial(decision.reason ?? 'Operation blocked by policy') + } + } else if (capability === 'external') { + const decision = authorizeToolCall(policy, { toolName, capability }) + if (!decision.allowed) return denial(decision.reason ?? 'Tool blocked by policy') + } + // Handle MCP tool calls when mcpConfig is present if (action.mcpConfig) { try { @@ -1340,9 +1508,10 @@ async function handleToolCall({ }) } else if (toolName === 'run_terminal_command') { const resolvedCwd = requireCwd(cwd, 'run_terminal_command') + const commandCwd = path.resolve(resolvedCwd, input.cwd ?? '.') result = await runTerminalCommand({ ...input, - cwd: path.resolve(resolvedCwd, input.cwd ?? '.'), + cwd: commandCwd, env, scrubEnvironmentKeys: byok ? [