diff --git a/src/infra/exec-approvals-allowlist.ts b/src/infra/exec-approvals-allowlist.ts index c039c6fc0..9a81328ec 100644 --- a/src/infra/exec-approvals-allowlist.ts +++ b/src/infra/exec-approvals-allowlist.ts @@ -16,6 +16,11 @@ import { validateSafeBinArgv, } from "./exec-safe-bin-policy.js"; import { isTrustedSafeBinPath } from "./exec-safe-bin-trust.js"; + +function hasShellLineContinuation(command: string): boolean { + return /\\(?:\r\n|\n|\r)/.test(command); +} + export function normalizeSafeBins(entries?: string[]): Set { if (!Array.isArray(entries)) { return new Set(); @@ -375,6 +380,12 @@ export function evaluateShellAllowlist(params: { segmentSatisfiedBy: [], }); + // Keep allowlist analysis conservative: line-continuation semantics are shell-dependent + // and can rewrite token boundaries at runtime. + if (hasShellLineContinuation(params.command)) { + return analysisFailure(); + } + const chainParts = isWindowsPlatform(params.platform) ? null : splitCommandChain(params.command); if (!chainParts) { const analysis = analyzeShellCommand({ diff --git a/src/infra/exec-approvals-analysis.ts b/src/infra/exec-approvals-analysis.ts index c851c7070..eab607997 100644 --- a/src/infra/exec-approvals-analysis.ts +++ b/src/infra/exec-approvals-analysis.ts @@ -317,7 +317,7 @@ export type ShellChainPart = { }; const DISALLOWED_PIPELINE_TOKENS = new Set([">", "<", "`", "\n", "\r", "(", ")"]); -const DOUBLE_QUOTE_ESCAPES = new Set(["\\", '"', "$", "`", "\n", "\r"]); +const DOUBLE_QUOTE_ESCAPES = new Set(["\\", '"', "$", "`"]); const WINDOWS_UNSUPPORTED_TOKENS = new Set([ "&", "|", @@ -336,6 +336,10 @@ function isDoubleQuoteEscape(next: string | undefined): next is string { return Boolean(next && DOUBLE_QUOTE_ESCAPES.has(next)); } +function isEscapedLineContinuation(next: string | undefined): next is string { + return next === "\n" || next === "\r"; +} + function splitShellPipeline(command: string): { ok: boolean; reason?: string; segments: string[] } { type HeredocSpec = { delimiter: string; @@ -485,6 +489,9 @@ function splitShellPipeline(command: string): { ok: boolean; reason?: string; se continue; } if (inDouble) { + if (ch === "\\" && isEscapedLineContinuation(next)) { + return { ok: false, reason: "unsupported shell token: newline", segments: [] }; + } if (ch === "\\" && isDoubleQuoteEscape(next)) { buf += ch; buf += next; @@ -749,6 +756,10 @@ export function splitCommandChainWithOperators(command: string): ShellChainPart[ continue; } if (inDouble) { + if (ch === "\\" && isEscapedLineContinuation(next)) { + invalidChain = true; + break; + } if (ch === "\\" && isDoubleQuoteEscape(next)) { buf += ch; buf += next; diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index 5afb0e7be..3b52da2a0 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -343,6 +343,14 @@ describe("exec approvals shell parsing", () => { command: "/usr/bin/echo first line\n/usr/bin/echo second line", reason: "unsupported shell token: \n", }, + { + command: 'echo "ok $\\\n(id -u)"', + reason: "unsupported shell token: newline", + }, + { + command: 'echo "ok $\\\r\n(id -u)"', + reason: "unsupported shell token: newline", + }, { command: "ping 127.0.0.1 -n 1 & whoami", reason: "unsupported windows shell token: &", @@ -548,6 +556,17 @@ describe("exec approvals shell allowlist (chained commands)", () => { expect(result.allowlistSatisfied).toBe(true); } }); + + it("fails allowlist analysis for shell line continuations", () => { + const result = evaluateShellAllowlist({ + command: 'echo "ok $\\\n(id -u)"', + allowlist: [{ pattern: "/usr/bin/echo" }], + safeBins: new Set(), + cwd: "/tmp", + }); + expect(result.analysisOk).toBe(false); + expect(result.allowlistSatisfied).toBe(false); + }); }); describe("exec approvals safe bins", () => { diff --git a/src/node-host/exec-policy.test.ts b/src/node-host/exec-policy.test.ts index 67a14cc8a..c74ce5ad2 100644 --- a/src/node-host/exec-policy.test.ts +++ b/src/node-host/exec-policy.test.ts @@ -22,9 +22,18 @@ describe("formatSystemRunAllowlistMissMessage", () => { expect(formatSystemRunAllowlistMissMessage()).toBe("SYSTEM_RUN_DENIED: allowlist miss"); }); + it("adds shell-wrapper guidance when wrappers are blocked", () => { + expect( + formatSystemRunAllowlistMissMessage({ + shellWrapperBlocked: true, + }), + ).toContain("shell wrappers like sh/bash/zsh -c require approval"); + }); + it("adds Windows shell-wrapper guidance when blocked by cmd.exe policy", () => { expect( formatSystemRunAllowlistMissMessage({ + shellWrapperBlocked: true, windowsShellWrapperBlocked: true, }), ).toContain("Windows shell wrappers like cmd.exe /c require approval"); @@ -42,6 +51,7 @@ describe("evaluateSystemRunPolicy", () => { approved: false, isWindows: false, cmdInvocation: false, + shellWrapperInvocation: false, }); expect(decision.allowed).toBe(false); if (decision.allowed) { @@ -61,6 +71,7 @@ describe("evaluateSystemRunPolicy", () => { approved: false, isWindows: false, cmdInvocation: false, + shellWrapperInvocation: false, }); expect(decision.allowed).toBe(false); if (decision.allowed) { @@ -80,6 +91,7 @@ describe("evaluateSystemRunPolicy", () => { approved: false, isWindows: false, cmdInvocation: false, + shellWrapperInvocation: false, }); expect(decision.allowed).toBe(true); if (!decision.allowed) { @@ -98,6 +110,7 @@ describe("evaluateSystemRunPolicy", () => { approved: false, isWindows: false, cmdInvocation: false, + shellWrapperInvocation: false, }); expect(decision.allowed).toBe(false); if (decision.allowed) { @@ -107,7 +120,27 @@ describe("evaluateSystemRunPolicy", () => { expect(decision.errorMessage).toBe("SYSTEM_RUN_DENIED: allowlist miss"); }); - it("treats Windows cmd.exe wrappers as allowlist misses", () => { + it("treats shell wrappers as allowlist misses", () => { + const decision = evaluateSystemRunPolicy({ + security: "allowlist", + ask: "off", + analysisOk: true, + allowlistSatisfied: true, + approvalDecision: null, + approved: false, + isWindows: false, + cmdInvocation: false, + shellWrapperInvocation: true, + }); + expect(decision.allowed).toBe(false); + if (decision.allowed) { + throw new Error("expected denied decision"); + } + expect(decision.shellWrapperBlocked).toBe(true); + expect(decision.errorMessage).toContain("shell wrappers like sh/bash/zsh -c"); + }); + + it("keeps Windows-specific guidance for cmd.exe wrappers", () => { const decision = evaluateSystemRunPolicy({ security: "allowlist", ask: "off", @@ -117,11 +150,13 @@ describe("evaluateSystemRunPolicy", () => { approved: false, isWindows: true, cmdInvocation: true, + shellWrapperInvocation: true, }); expect(decision.allowed).toBe(false); if (decision.allowed) { throw new Error("expected denied decision"); } + expect(decision.shellWrapperBlocked).toBe(true); expect(decision.windowsShellWrapperBlocked).toBe(true); expect(decision.errorMessage).toContain("Windows shell wrappers like cmd.exe /c"); }); @@ -136,6 +171,7 @@ describe("evaluateSystemRunPolicy", () => { approved: false, isWindows: false, cmdInvocation: false, + shellWrapperInvocation: false, }); expect(decision.allowed).toBe(true); if (!decision.allowed) { diff --git a/src/node-host/exec-policy.ts b/src/node-host/exec-policy.ts index cbc266ed3..ce4c2d57b 100644 --- a/src/node-host/exec-policy.ts +++ b/src/node-host/exec-policy.ts @@ -5,6 +5,7 @@ export type ExecApprovalDecision = "allow-once" | "allow-always" | null; export type SystemRunPolicyDecision = { analysisOk: boolean; allowlistSatisfied: boolean; + shellWrapperBlocked: boolean; windowsShellWrapperBlocked: boolean; requiresAsk: boolean; approvalDecision: ExecApprovalDecision; @@ -28,6 +29,7 @@ export function resolveExecApprovalDecision(value: unknown): ExecApprovalDecisio } export function formatSystemRunAllowlistMissMessage(params?: { + shellWrapperBlocked?: boolean; windowsShellWrapperBlocked?: boolean; }): string { if (params?.windowsShellWrapperBlocked) { @@ -37,6 +39,13 @@ export function formatSystemRunAllowlistMissMessage(params?: { "approve once/always or run with --ask on-miss|always)" ); } + if (params?.shellWrapperBlocked) { + return ( + "SYSTEM_RUN_DENIED: allowlist miss " + + "(shell wrappers like sh/bash/zsh -c require approval; " + + "approve once/always or run with --ask on-miss|always)" + ); + } return "SYSTEM_RUN_DENIED: allowlist miss"; } @@ -49,11 +58,13 @@ export function evaluateSystemRunPolicy(params: { approved?: boolean; isWindows: boolean; cmdInvocation: boolean; + shellWrapperInvocation: boolean; }): SystemRunPolicyDecision { + const shellWrapperBlocked = params.security === "allowlist" && params.shellWrapperInvocation; const windowsShellWrapperBlocked = - params.security === "allowlist" && params.isWindows && params.cmdInvocation; - const analysisOk = windowsShellWrapperBlocked ? false : params.analysisOk; - const allowlistSatisfied = windowsShellWrapperBlocked ? false : params.allowlistSatisfied; + shellWrapperBlocked && params.isWindows && params.cmdInvocation; + const analysisOk = shellWrapperBlocked ? false : params.analysisOk; + const allowlistSatisfied = shellWrapperBlocked ? false : params.allowlistSatisfied; const approvedByAsk = params.approvalDecision !== null || params.approved === true; if (params.security === "deny") { @@ -63,6 +74,7 @@ export function evaluateSystemRunPolicy(params: { errorMessage: "SYSTEM_RUN_DISABLED: security=deny", analysisOk, allowlistSatisfied, + shellWrapperBlocked, windowsShellWrapperBlocked, requiresAsk: false, approvalDecision: params.approvalDecision, @@ -83,6 +95,7 @@ export function evaluateSystemRunPolicy(params: { errorMessage: "SYSTEM_RUN_DENIED: approval required", analysisOk, allowlistSatisfied, + shellWrapperBlocked, windowsShellWrapperBlocked, requiresAsk, approvalDecision: params.approvalDecision, @@ -94,9 +107,13 @@ export function evaluateSystemRunPolicy(params: { return { allowed: false, eventReason: "allowlist-miss", - errorMessage: formatSystemRunAllowlistMissMessage({ windowsShellWrapperBlocked }), + errorMessage: formatSystemRunAllowlistMissMessage({ + shellWrapperBlocked, + windowsShellWrapperBlocked, + }), analysisOk, allowlistSatisfied, + shellWrapperBlocked, windowsShellWrapperBlocked, requiresAsk, approvalDecision: params.approvalDecision, @@ -108,6 +125,7 @@ export function evaluateSystemRunPolicy(params: { allowed: true, analysisOk, allowlistSatisfied, + shellWrapperBlocked, windowsShellWrapperBlocked, requiresAsk, approvalDecision: params.approvalDecision, diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index 916d8c169..eca2af4ec 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -166,6 +166,37 @@ export async function handleSystemRunInvoke(opts: { const cmdInvocation = shellCommand ? opts.isCmdExeInvocation(segments[0]?.argv ?? []) : opts.isCmdExeInvocation(argv); + const policy = evaluateSystemRunPolicy({ + security, + ask, + analysisOk, + allowlistSatisfied, + approvalDecision, + approved: opts.params.approved === true, + isWindows, + cmdInvocation, + shellWrapperInvocation: shellCommand !== null, + }); + analysisOk = policy.analysisOk; + allowlistSatisfied = policy.allowlistSatisfied; + if (!policy.allowed) { + await opts.sendNodeEvent( + opts.client, + "exec.denied", + opts.buildExecEventPayload({ + sessionKey, + runId, + host: "node", + command: cmdText, + reason: policy.eventReason, + }), + ); + await opts.sendInvokeResult({ + ok: false, + error: { code: "UNAVAILABLE", message: policy.errorMessage }, + }); + return; + } const useMacAppExec = opts.preferMacAppExecHost; if (useMacAppExec) { @@ -232,37 +263,6 @@ export async function handleSystemRunInvoke(opts: { } } - const policy = evaluateSystemRunPolicy({ - security, - ask, - analysisOk, - allowlistSatisfied, - approvalDecision, - approved: opts.params.approved === true, - isWindows, - cmdInvocation, - }); - analysisOk = policy.analysisOk; - allowlistSatisfied = policy.allowlistSatisfied; - if (!policy.allowed) { - await opts.sendNodeEvent( - opts.client, - "exec.denied", - opts.buildExecEventPayload({ - sessionKey, - runId, - host: "node", - command: cmdText, - reason: policy.eventReason, - }), - ); - await opts.sendInvokeResult({ - ok: false, - error: { code: "UNAVAILABLE", message: policy.errorMessage }, - }); - return; - } - if (policy.approvalDecision === "allow-always" && security === "allowlist") { if (policy.analysisOk) { const patterns = resolveAllowAlwaysPatterns({