diff --git a/CHANGELOG.md b/CHANGELOG.md index 20aacf256..18ef0efa6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Docs: https://docs.clawd.bot - Auto-reply: only report a model switch when session state is available. (#1465) Thanks @robbyczgw-cla. - Control UI: resolve local avatar URLs with basePath across injection + identity RPC. (#1457) Thanks @dlauer. - Agents: surface concrete API error details instead of generic AI service errors. +- Exec approvals: allow per-segment allowlists for chained shell commands on gateway + node hosts. (#1458) Thanks @czekaj. - Agents: avoid sanitizing tool call IDs for OpenAI responses to preserve Pi pairing. - Docs: fix gog auth services example to include docs scope. (#1454) Thanks @zerone0x. - macOS: prefer linked channels in gateway summary to avoid false “not linked” status. diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index 6857970f6..fc657a74f 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -113,6 +113,9 @@ that can run in allowlist mode **without** explicit allowlist entries. Safe bins positional file args and path-like tokens, so they can only operate on the incoming stream. Shell chaining and redirections are not auto-allowed in allowlist mode. +Shell chaining (`&&`, `||`, `;`) is allowed when every top-level segment satisfies the allowlist +(including safe bins or skill auto-allow). Redirections remain unsupported in allowlist mode. + Default safe bins: `jq`, `grep`, `cut`, `sort`, `uniq`, `head`, `tail`, `tr`, `wc`. ## Control UI editing diff --git a/src/agents/bash-tools.exec.approval-id.test.ts b/src/agents/bash-tools.exec.approval-id.test.ts index 651f1f4d9..16a198b5c 100644 --- a/src/agents/bash-tools.exec.approval-id.test.ts +++ b/src/agents/bash-tools.exec.approval-id.test.ts @@ -16,11 +16,15 @@ vi.mock("./tools/nodes-utils.js", () => ({ describe("exec approvals", () => { let previousHome: string | undefined; + let previousUserProfile: string | undefined; beforeEach(async () => { previousHome = process.env.HOME; + previousUserProfile = process.env.USERPROFILE; const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-test-")); process.env.HOME = tempDir; + // Windows uses USERPROFILE for os.homedir() + process.env.USERPROFILE = tempDir; }); afterEach(() => { @@ -30,6 +34,11 @@ describe("exec approvals", () => { } else { process.env.HOME = previousHome; } + if (previousUserProfile === undefined) { + delete process.env.USERPROFILE; + } else { + process.env.USERPROFILE = previousUserProfile; + } }); it("reuses approval id as the node runId", async () => { diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index 75f6b1927..1f102d8ab 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -10,8 +10,7 @@ import { type ExecSecurity, type ExecApprovalsFile, addAllowlistEntry, - analyzeShellCommand, - evaluateExecAllowlist, + evaluateShellAllowlist, maxAsk, minSecurity, requiresExecApproval, @@ -871,9 +870,16 @@ export function createExecTool( if (nodeEnv) { applyPathPrepend(nodeEnv, defaultPathPrepend, { requireExisting: true }); } - const analysis = analyzeShellCommand({ command: params.command, cwd: workdir, env }); + const baseAllowlistEval = evaluateShellAllowlist({ + command: params.command, + allowlist: [], + safeBins: new Set(), + cwd: workdir, + env, + }); + let analysisOk = baseAllowlistEval.analysisOk; let allowlistSatisfied = false; - if (hostAsk === "on-miss" && hostSecurity === "allowlist") { + if (hostAsk === "on-miss" && hostSecurity === "allowlist" && analysisOk) { try { const approvalsSnapshot = (await callGatewayTool( "exec.approvals.node.get", @@ -891,12 +897,15 @@ export function createExecTool( overrides: { security: "allowlist" }, }); // Allowlist-only precheck; safe bins are node-local and may diverge. - allowlistSatisfied = evaluateExecAllowlist({ - analysis, + const allowlistEval = evaluateShellAllowlist({ + command: params.command, allowlist: resolved.allowlist, safeBins: new Set(), cwd: workdir, - }).allowlistSatisfied; + env, + }); + allowlistSatisfied = allowlistEval.allowlistSatisfied; + analysisOk = allowlistEval.analysisOk; } } catch { // Fall back to requiring approval if node approvals cannot be fetched. @@ -905,7 +914,7 @@ export function createExecTool( const requiresAsk = requiresExecApproval({ ask: hostAsk, security: hostSecurity, - analysisOk: analysis.ok, + analysisOk, allowlistSatisfied, }); const commandText = params.command; @@ -1095,20 +1104,21 @@ export function createExecTool( if (hostSecurity === "deny") { throw new Error("exec denied: host=gateway security=deny"); } - const analysis = analyzeShellCommand({ command: params.command, cwd: workdir, env }); - const allowlistEval = evaluateExecAllowlist({ - analysis, + const allowlistEval = evaluateShellAllowlist({ + command: params.command, allowlist: approvals.allowlist, safeBins, cwd: workdir, + env, }); const allowlistMatches = allowlistEval.allowlistMatches; + const analysisOk = allowlistEval.analysisOk; const allowlistSatisfied = - hostSecurity === "allowlist" && analysis.ok ? allowlistEval.allowlistSatisfied : false; + hostSecurity === "allowlist" && analysisOk ? allowlistEval.allowlistSatisfied : false; const requiresAsk = requiresExecApproval({ ask: hostAsk, security: hostSecurity, - analysisOk: analysis.ok, + analysisOk, allowlistSatisfied, }); @@ -1117,7 +1127,7 @@ export function createExecTool( const approvalSlug = createApprovalSlug(approvalId); const expiresAtMs = Date.now() + DEFAULT_APPROVAL_TIMEOUT_MS; const contextKey = `exec:${approvalId}`; - const resolvedPath = analysis.segments[0]?.resolution?.resolvedPath; + const resolvedPath = allowlistEval.segments[0]?.resolution?.resolvedPath; const noticeSeconds = Math.max(1, Math.round(approvalRunningNoticeMs / 1000)); const commandText = params.command; const effectiveTimeout = @@ -1164,7 +1174,7 @@ export function createExecTool( if (askFallback === "full") { approvedByAsk = true; } else if (askFallback === "allowlist") { - if (!analysis.ok || !allowlistSatisfied) { + if (!analysisOk || !allowlistSatisfied) { deniedReason = "approval-timeout (allowlist-miss)"; } else { approvedByAsk = true; @@ -1177,7 +1187,7 @@ export function createExecTool( } else if (decision === "allow-always") { approvedByAsk = true; if (hostSecurity === "allowlist") { - for (const segment of analysis.segments) { + for (const segment of allowlistEval.segments) { const pattern = segment.resolution?.resolvedPath ?? ""; if (pattern) { addAllowlistEntry(approvals.file, agentId, pattern); @@ -1188,7 +1198,7 @@ export function createExecTool( if ( hostSecurity === "allowlist" && - (!analysis.ok || !allowlistSatisfied) && + (!analysisOk || !allowlistSatisfied) && !approvedByAsk ) { deniedReason = deniedReason ?? "allowlist-miss"; @@ -1288,7 +1298,7 @@ export function createExecTool( }; } - if (hostSecurity === "allowlist" && (!analysis.ok || !allowlistSatisfied)) { + if (hostSecurity === "allowlist" && (!analysisOk || !allowlistSatisfied)) { throw new Error("exec denied: allowlist miss"); } @@ -1302,7 +1312,7 @@ export function createExecTool( agentId, match, params.command, - analysis.segments[0]?.resolution?.resolvedPath, + allowlistEval.segments[0]?.resolution?.resolvedPath, ); } } diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index 191cc08bc..57abeb695 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -8,6 +8,7 @@ import { analyzeArgvCommand, analyzeShellCommand, evaluateExecAllowlist, + evaluateShellAllowlist, isSafeBinUsage, matchAllowlist, maxAsk, @@ -121,9 +122,10 @@ describe("exec approvals shell parsing", () => { expect(res.segments.map((seg) => seg.argv[0])).toEqual(["echo", "jq"]); }); - it("rejects chained commands", () => { + it("parses chained commands", () => { const res = analyzeShellCommand({ command: "ls && rm -rf /" }); - expect(res.ok).toBe(false); + expect(res.ok).toBe(true); + expect(res.chains?.map((chain) => chain[0]?.argv[0])).toEqual(["ls", "rm"]); }); it("parses argv commands", () => { @@ -133,6 +135,60 @@ describe("exec approvals shell parsing", () => { }); }); +describe("exec approvals shell allowlist (chained commands)", () => { + it("allows chained commands when all parts are allowlisted", () => { + const allowlist: ExecAllowlistEntry[] = [ + { pattern: "/usr/bin/obsidian-cli" }, + { pattern: "/usr/bin/head" }, + ]; + const result = evaluateShellAllowlist({ + command: + "/usr/bin/obsidian-cli print-default && /usr/bin/obsidian-cli search foo | /usr/bin/head", + allowlist, + safeBins: new Set(), + cwd: "/tmp", + }); + expect(result.analysisOk).toBe(true); + expect(result.allowlistSatisfied).toBe(true); + }); + + it("rejects chained commands when any part is not allowlisted", () => { + const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/obsidian-cli" }]; + const result = evaluateShellAllowlist({ + command: "/usr/bin/obsidian-cli print-default && /usr/bin/rm -rf /", + allowlist, + safeBins: new Set(), + cwd: "/tmp", + }); + expect(result.analysisOk).toBe(true); + expect(result.allowlistSatisfied).toBe(false); + }); + + it("returns analysisOk=false for malformed chains", () => { + const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/echo" }]; + const result = evaluateShellAllowlist({ + command: "/usr/bin/echo ok &&", + allowlist, + safeBins: new Set(), + cwd: "/tmp", + }); + expect(result.analysisOk).toBe(false); + expect(result.allowlistSatisfied).toBe(false); + }); + + it("respects quotes when splitting chains", () => { + const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/echo" }]; + const result = evaluateShellAllowlist({ + command: '/usr/bin/echo "foo && bar"', + allowlist, + safeBins: new Set(), + cwd: "/tmp", + }); + expect(result.analysisOk).toBe(true); + expect(result.allowlistSatisfied).toBe(true); + }); +}); + describe("exec approvals safe bins", () => { it("allows safe bins with non-path args", () => { const dir = makeTempDir(); diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index b2c53fc80..524a11d6b 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -506,29 +506,44 @@ export type ExecCommandAnalysis = { ok: boolean; reason?: string; segments: ExecCommandSegment[]; + chains?: ExecCommandSegment[][]; // Segments grouped by chain operator (&&, ||, ;) }; -const DISALLOWED_TOKENS = new Set([";", "&", ">", "<", "`", "\n", "\r", "(", ")"]); +const DISALLOWED_PIPELINE_TOKENS = new Set([">", "<", "`", "\n", "\r", "(", ")"]); -function splitShellPipeline(command: string): { ok: boolean; reason?: string; segments: string[] } { - const segments: string[] = []; +type IteratorAction = "split" | "skip" | "include" | { reject: string }; + +/** + * Iterates through a command string while respecting shell quoting rules. + * The callback receives each character and the next character, and returns an action: + * - "split": push current buffer as a segment and start a new one + * - "skip": skip this character (and optionally the next via skip count) + * - "include": add this character to the buffer + * - { reject: reason }: abort with an error + */ +function iterateQuoteAware( + command: string, + onChar: (ch: string, next: string | undefined, index: number) => IteratorAction, +): { ok: true; parts: string[]; hasSplit: boolean } | { ok: false; reason: string } { + const parts: string[] = []; let buf = ""; let inSingle = false; let inDouble = false; let escaped = false; + let hasSplit = false; - const pushSegment = () => { + const pushPart = () => { const trimmed = buf.trim(); - if (!trimmed) { - return false; + if (trimmed) { + parts.push(trimmed); } - segments.push(trimmed); buf = ""; - return true; }; for (let i = 0; i < command.length; i += 1) { const ch = command[i]; + const next = command[i + 1]; + if (escaped) { buf += ch; escaped = false; @@ -559,34 +574,66 @@ function splitShellPipeline(command: string): { ok: boolean; reason?: string; se buf += ch; continue; } - if (ch === "|" && command[i + 1] === "|") { - return { ok: false, reason: "unsupported shell token: ||", segments: [] }; + + const action = onChar(ch, next, i); + if (typeof action === "object" && "reject" in action) { + return { ok: false, reason: action.reject }; } - if (ch === "|" && command[i + 1] === "&") { - return { ok: false, reason: "unsupported shell token: |&", segments: [] }; - } - if (ch === "|") { - if (!pushSegment()) { - return { ok: false, reason: "empty pipeline segment", segments: [] }; - } + if (action === "split") { + pushPart(); + hasSplit = true; continue; } - if (DISALLOWED_TOKENS.has(ch)) { - return { ok: false, reason: `unsupported shell token: ${ch}`, segments: [] }; - } - if (ch === "$" && command[i + 1] === "(") { - return { ok: false, reason: "unsupported shell token: $()", segments: [] }; + if (action === "skip") { + continue; } buf += ch; } if (escaped || inSingle || inDouble) { - return { ok: false, reason: "unterminated shell quote/escape", segments: [] }; + return { ok: false, reason: "unterminated shell quote/escape" }; } - if (!pushSegment()) { - return { ok: false, reason: "empty command", segments: [] }; + pushPart(); + return { ok: true, parts, hasSplit }; +} + +function splitShellPipeline(command: string): { ok: boolean; reason?: string; segments: string[] } { + let emptySegment = false; + const result = iterateQuoteAware(command, (ch, next) => { + if (ch === "|" && next === "|") { + return { reject: "unsupported shell token: ||" }; + } + if (ch === "|" && next === "&") { + return { reject: "unsupported shell token: |&" }; + } + if (ch === "|") { + emptySegment = true; + return "split"; + } + if (ch === "&" || ch === ";") { + return { reject: `unsupported shell token: ${ch}` }; + } + if (DISALLOWED_PIPELINE_TOKENS.has(ch)) { + return { reject: `unsupported shell token: ${ch}` }; + } + if (ch === "$" && next === "(") { + return { reject: "unsupported shell token: $()" }; + } + emptySegment = false; + return "include"; + }); + + if (!result.ok) { + return { ok: false, reason: result.reason, segments: [] }; } - return { ok: true, segments }; + if (emptySegment || result.parts.length === 0) { + return { + ok: false, + reason: result.parts.length === 0 ? "empty command" : "empty pipeline segment", + segments: [], + }; + } + return { ok: true, segments: result.parts }; } function tokenizeShellSegment(segment: string): string[] | null { @@ -652,26 +699,61 @@ function tokenizeShellSegment(segment: string): string[] | null { return tokens; } +function parseSegmentsFromParts( + parts: string[], + cwd?: string, + env?: NodeJS.ProcessEnv, +): ExecCommandSegment[] | null { + const segments: ExecCommandSegment[] = []; + for (const raw of parts) { + const argv = tokenizeShellSegment(raw); + if (!argv || argv.length === 0) { + return null; + } + segments.push({ + raw, + argv, + resolution: resolveCommandResolutionFromArgv(argv, cwd, env), + }); + } + return segments; +} + export function analyzeShellCommand(params: { command: string; cwd?: string; env?: NodeJS.ProcessEnv; }): ExecCommandAnalysis { + // First try splitting by chain operators (&&, ||, ;) + const chainParts = splitCommandChain(params.command); + if (chainParts) { + const chains: ExecCommandSegment[][] = []; + const allSegments: ExecCommandSegment[] = []; + + for (const part of chainParts) { + const pipelineSplit = splitShellPipeline(part); + if (!pipelineSplit.ok) { + return { ok: false, reason: pipelineSplit.reason, segments: [] }; + } + const segments = parseSegmentsFromParts(pipelineSplit.segments, params.cwd, params.env); + if (!segments) { + return { ok: false, reason: "unable to parse shell segment", segments: [] }; + } + chains.push(segments); + allSegments.push(...segments); + } + + return { ok: true, segments: allSegments, chains }; + } + + // No chain operators, parse as simple pipeline const split = splitShellPipeline(params.command); if (!split.ok) { return { ok: false, reason: split.reason, segments: [] }; } - const segments: ExecCommandSegment[] = []; - for (const raw of split.segments) { - const argv = tokenizeShellSegment(raw); - if (!argv || argv.length === 0) { - return { ok: false, reason: "unable to parse shell segment", segments: [] }; - } - segments.push({ - raw, - argv, - resolution: resolveCommandResolutionFromArgv(argv, params.cwd, params.env), - }); + const segments = parseSegmentsFromParts(split.segments, params.cwd, params.env); + if (!segments) { + return { ok: false, reason: "unable to parse shell segment", segments: [] }; } return { ok: true, segments }; } @@ -771,27 +853,27 @@ export type ExecAllowlistEvaluation = { allowlistMatches: ExecAllowlistEntry[]; }; -export function evaluateExecAllowlist(params: { - analysis: ExecCommandAnalysis; - allowlist: ExecAllowlistEntry[]; - safeBins: Set; - cwd?: string; - skillBins?: Set; - autoAllowSkills?: boolean; -}): ExecAllowlistEvaluation { - const allowlistMatches: ExecAllowlistEntry[] = []; - if (!params.analysis.ok || params.analysis.segments.length === 0) { - return { allowlistSatisfied: false, allowlistMatches }; - } +function evaluateSegments( + segments: ExecCommandSegment[], + params: { + allowlist: ExecAllowlistEntry[]; + safeBins: Set; + cwd?: string; + skillBins?: Set; + autoAllowSkills?: boolean; + }, +): { satisfied: boolean; matches: ExecAllowlistEntry[] } { + const matches: ExecAllowlistEntry[] = []; const allowSkills = params.autoAllowSkills === true && (params.skillBins?.size ?? 0) > 0; - const allowlistSatisfied = params.analysis.segments.every((segment) => { + + const satisfied = segments.every((segment) => { const candidatePath = resolveAllowlistCandidatePath(segment.resolution, params.cwd); const candidateResolution = candidatePath && segment.resolution ? { ...segment.resolution, resolvedPath: candidatePath } : segment.resolution; const match = matchAllowlist(params.allowlist, candidateResolution); - if (match) allowlistMatches.push(match); + if (match) matches.push(match); const safe = isSafeBinUsage({ argv: segment.argv, resolution: segment.resolution, @@ -804,7 +886,230 @@ export function evaluateExecAllowlist(params: { : false; return Boolean(match || safe || skillAllow); }); - return { allowlistSatisfied, allowlistMatches }; + + return { satisfied, matches }; +} + +export function evaluateExecAllowlist(params: { + analysis: ExecCommandAnalysis; + allowlist: ExecAllowlistEntry[]; + safeBins: Set; + cwd?: string; + skillBins?: Set; + autoAllowSkills?: boolean; +}): ExecAllowlistEvaluation { + const allowlistMatches: ExecAllowlistEntry[] = []; + if (!params.analysis.ok || params.analysis.segments.length === 0) { + return { allowlistSatisfied: false, allowlistMatches }; + } + + // If the analysis contains chains, evaluate each chain part separately + if (params.analysis.chains) { + for (const chainSegments of params.analysis.chains) { + const result = evaluateSegments(chainSegments, { + allowlist: params.allowlist, + safeBins: params.safeBins, + cwd: params.cwd, + skillBins: params.skillBins, + autoAllowSkills: params.autoAllowSkills, + }); + if (!result.satisfied) { + return { allowlistSatisfied: false, allowlistMatches: [] }; + } + allowlistMatches.push(...result.matches); + } + return { allowlistSatisfied: true, allowlistMatches }; + } + + // No chains, evaluate all segments together + const result = evaluateSegments(params.analysis.segments, { + allowlist: params.allowlist, + safeBins: params.safeBins, + cwd: params.cwd, + skillBins: params.skillBins, + autoAllowSkills: params.autoAllowSkills, + }); + return { allowlistSatisfied: result.satisfied, allowlistMatches: result.matches }; +} + +/** + * Splits a command string by chain operators (&&, ||, ;) while respecting quotes. + * Returns null when no chain is present or when the chain is malformed. + */ +function splitCommandChain(command: string): string[] | null { + const parts: string[] = []; + let buf = ""; + let inSingle = false; + let inDouble = false; + let escaped = false; + let foundChain = false; + let invalidChain = false; + + const pushPart = () => { + const trimmed = buf.trim(); + if (trimmed) { + parts.push(trimmed); + buf = ""; + return true; + } + buf = ""; + return false; + }; + + for (let i = 0; i < command.length; i += 1) { + const ch = command[i]; + if (escaped) { + buf += ch; + escaped = false; + continue; + } + if (!inSingle && !inDouble && ch === "\\") { + escaped = true; + buf += ch; + continue; + } + if (inSingle) { + if (ch === "'") inSingle = false; + buf += ch; + continue; + } + if (inDouble) { + if (ch === '"') inDouble = false; + buf += ch; + continue; + } + if (ch === "'") { + inSingle = true; + buf += ch; + continue; + } + if (ch === '"') { + inDouble = true; + buf += ch; + continue; + } + + if (ch === "&" && command[i + 1] === "&") { + if (!pushPart()) invalidChain = true; + i += 1; + foundChain = true; + continue; + } + if (ch === "|" && command[i + 1] === "|") { + if (!pushPart()) invalidChain = true; + i += 1; + foundChain = true; + continue; + } + if (ch === ";") { + if (!pushPart()) invalidChain = true; + foundChain = true; + continue; + } + + buf += ch; + } + + const pushedFinal = pushPart(); + if (!foundChain) return null; + if (invalidChain || !pushedFinal) return null; + return parts.length > 0 ? parts : null; +} + +export type ExecAllowlistAnalysis = { + analysisOk: boolean; + allowlistSatisfied: boolean; + allowlistMatches: ExecAllowlistEntry[]; + segments: ExecCommandSegment[]; +}; + +/** + * Evaluates allowlist for shell commands (including &&, ||, ;) and returns analysis metadata. + */ +export function evaluateShellAllowlist(params: { + command: string; + allowlist: ExecAllowlistEntry[]; + safeBins: Set; + cwd?: string; + env?: NodeJS.ProcessEnv; + skillBins?: Set; + autoAllowSkills?: boolean; +}): ExecAllowlistAnalysis { + const chainParts = splitCommandChain(params.command); + if (!chainParts) { + const analysis = analyzeShellCommand({ + command: params.command, + cwd: params.cwd, + env: params.env, + }); + if (!analysis.ok) { + return { + analysisOk: false, + allowlistSatisfied: false, + allowlistMatches: [], + segments: [], + }; + } + const evaluation = evaluateExecAllowlist({ + analysis, + allowlist: params.allowlist, + safeBins: params.safeBins, + cwd: params.cwd, + skillBins: params.skillBins, + autoAllowSkills: params.autoAllowSkills, + }); + return { + analysisOk: true, + allowlistSatisfied: evaluation.allowlistSatisfied, + allowlistMatches: evaluation.allowlistMatches, + segments: analysis.segments, + }; + } + + const allowlistMatches: ExecAllowlistEntry[] = []; + const segments: ExecCommandSegment[] = []; + + for (const part of chainParts) { + const analysis = analyzeShellCommand({ + command: part, + cwd: params.cwd, + env: params.env, + }); + if (!analysis.ok) { + return { + analysisOk: false, + allowlistSatisfied: false, + allowlistMatches: [], + segments: [], + }; + } + + segments.push(...analysis.segments); + const evaluation = evaluateExecAllowlist({ + analysis, + allowlist: params.allowlist, + safeBins: params.safeBins, + cwd: params.cwd, + skillBins: params.skillBins, + autoAllowSkills: params.autoAllowSkills, + }); + allowlistMatches.push(...evaluation.allowlistMatches); + if (!evaluation.allowlistSatisfied) { + return { + analysisOk: true, + allowlistSatisfied: false, + allowlistMatches, + segments, + }; + } + } + + return { + analysisOk: true, + allowlistSatisfied: true, + allowlistMatches, + segments, + }; } export function requiresExecApproval(params: { diff --git a/src/node-host/runner.ts b/src/node-host/runner.ts index a66aa63f1..1274d83a5 100644 --- a/src/node-host/runner.ts +++ b/src/node-host/runner.ts @@ -6,8 +6,8 @@ import path from "node:path"; import { addAllowlistEntry, analyzeArgvCommand, - analyzeShellCommand, evaluateExecAllowlist, + evaluateShellAllowlist, requiresExecApproval, normalizeExecApprovals, recordAllowlistUse, @@ -18,6 +18,8 @@ import { resolveExecApprovalsSocketPath, saveExecApprovals, type ExecApprovalsFile, + type ExecAllowlistEntry, + type ExecCommandSegment, } from "../infra/exec-approvals.js"; import { requestExecHostViaSocket, @@ -585,24 +587,45 @@ async function handleInvoke( const sessionKey = params.sessionKey?.trim() || "node"; const runId = params.runId?.trim() || crypto.randomUUID(); const env = sanitizeEnv(params.env ?? undefined); - const analysis = rawCommand - ? analyzeShellCommand({ command: rawCommand, cwd: params.cwd ?? undefined, env }) - : analyzeArgvCommand({ argv, cwd: params.cwd ?? undefined, env }); const cfg = loadConfig(); const agentExec = agentId ? resolveAgentConfig(cfg, agentId)?.tools?.exec : undefined; const safeBins = resolveSafeBins(agentExec?.safeBins ?? cfg.tools?.exec?.safeBins); const bins = autoAllowSkills ? await skillBins.current() : new Set(); - const allowlistEval = evaluateExecAllowlist({ - analysis, - allowlist: approvals.allowlist, - safeBins, - cwd: params.cwd ?? undefined, - skillBins: bins, - autoAllowSkills, - }); - const allowlistMatches = allowlistEval.allowlistMatches; - const allowlistSatisfied = - security === "allowlist" && analysis.ok ? allowlistEval.allowlistSatisfied : false; + let analysisOk = false; + let allowlistMatches: ExecAllowlistEntry[] = []; + let allowlistSatisfied = false; + let segments: ExecCommandSegment[] = []; + if (rawCommand) { + const allowlistEval = evaluateShellAllowlist({ + command: rawCommand, + allowlist: approvals.allowlist, + safeBins, + cwd: params.cwd ?? undefined, + env, + skillBins: bins, + autoAllowSkills, + }); + analysisOk = allowlistEval.analysisOk; + allowlistMatches = allowlistEval.allowlistMatches; + allowlistSatisfied = + security === "allowlist" && analysisOk ? allowlistEval.allowlistSatisfied : false; + segments = allowlistEval.segments; + } else { + const analysis = analyzeArgvCommand({ argv, cwd: params.cwd ?? undefined, env }); + const allowlistEval = evaluateExecAllowlist({ + analysis, + allowlist: approvals.allowlist, + safeBins, + cwd: params.cwd ?? undefined, + skillBins: bins, + autoAllowSkills, + }); + analysisOk = analysis.ok; + allowlistMatches = allowlistEval.allowlistMatches; + allowlistSatisfied = + security === "allowlist" && analysisOk ? allowlistEval.allowlistSatisfied : false; + segments = analysis.segments; + } const useMacAppExec = process.platform === "darwin"; if (useMacAppExec) { @@ -709,7 +732,7 @@ async function handleInvoke( const requiresAsk = requiresExecApproval({ ask, security, - analysisOk: analysis.ok, + analysisOk, allowlistSatisfied, }); @@ -737,15 +760,15 @@ async function handleInvoke( return; } if (approvalDecision === "allow-always" && security === "allowlist") { - if (analysis.ok) { - for (const segment of analysis.segments) { + if (analysisOk) { + for (const segment of segments) { const pattern = segment.resolution?.resolvedPath ?? ""; if (pattern) addAllowlistEntry(approvals.file, agentId, pattern); } } } - if (security === "allowlist" && (!analysis.ok || !allowlistSatisfied) && !approvedByAsk) { + if (security === "allowlist" && (!analysisOk || !allowlistSatisfied) && !approvedByAsk) { await sendNodeEvent( client, "exec.denied", @@ -774,7 +797,7 @@ async function handleInvoke( agentId, match, cmdText, - analysis.segments[0]?.resolution?.resolvedPath, + segments[0]?.resolution?.resolvedPath, ); } }