diff --git a/src/core/auto-approval/__tests__/commands.spec.ts b/src/core/auto-approval/__tests__/commands.spec.ts index ade53c2b20..669b4f596b 100644 --- a/src/core/auto-approval/__tests__/commands.spec.ts +++ b/src/core/auto-approval/__tests__/commands.spec.ts @@ -38,9 +38,9 @@ describe("getCommandDecision", () => { }) }) -describe("containsDangerousSubstitution — node -e one-liner false positive regression", () => { - const nodeOneLiner = `node -e "const fs=require('fs');const p=JSON.parse(fs.readFileSync('prd.json','utf8'));const allowed=new Set(['pending','in-progress','complete','blocked']);const bad=(p.items||[]).filter(i=>!allowed.has(i.status));console.log('meta.status',p.meta?.status);console.log('workstreams', (p.workstreams||[]).length);console.log('items', (p.items||[]).length);console.log('statusCounts', (p.items||[]).reduce((a,i)=>(a[i.status]=(a[i.status]||0)+1,a),{}));console.log('invalidStatuses', bad.length);if(bad.length){console.log(bad.map(i=>i.id+':'+i.status).join('\\\\n'));process.exit(2);} "` +const nodeOneLiner = `node -e "const fs=require('fs');const p=JSON.parse(fs.readFileSync('prd.json','utf8'));const allowed=new Set(['pending','in-progress','complete','blocked']);const bad=(p.items||[]).filter(i=>!allowed.has(i.status));console.log('meta.status',p.meta?.status);console.log('workstreams', (p.workstreams||[]).length);console.log('items', (p.items||[]).length);console.log('statusCounts', (p.items||[]).reduce((a,i)=>(a[i.status]=(a[i.status]||0)+1,a),{}));console.log('invalidStatuses', bad.length);if(bad.length){console.log(bad.map(i=>i.id+':'+i.status).join('\\\\n'));process.exit(2);} "` +describe("containsDangerousSubstitution — node -e one-liner false positive regression", () => { it("should NOT flag the complex node -e one-liner as dangerous substitution", () => { expect(containsDangerousSubstitution(nodeOneLiner)).toBe(false) }) @@ -86,8 +86,6 @@ describe("getCommandDecision — integration with dangerous substitution checks" const allowedCommands = ["node", "echo"] it("should auto-approve the complex node -e one-liner when node is allowed", () => { - const nodeOneLiner = `node -e "const fs=require('fs');const p=JSON.parse(fs.readFileSync('prd.json','utf8'));const allowed=new Set(['pending','in-progress','complete','blocked']);const bad=(p.items||[]).filter(i=>!allowed.has(i.status));console.log('meta.status',p.meta?.status);console.log('workstreams', (p.workstreams||[]).length);console.log('items', (p.items||[]).length);console.log('statusCounts', (p.items||[]).reduce((a,i)=>(a[i.status]=(a[i.status]||0)+1,a),{}));console.log('invalidStatuses', bad.length);if(bad.length){console.log(bad.map(i=>i.id+':'+i.status).join('\\\\n'));process.exit(2);} "` - expect(getCommandDecision(nodeOneLiner, allowedCommands)).toBe("auto_approve") }) @@ -98,6 +96,689 @@ describe("getCommandDecision — integration with dangerous substitution checks" it("should ask user for dangerous parameter expansion even when command is allowed", () => { expect(getCommandDecision('echo "${var@P}"', allowedCommands)).toBe("ask_user") }) + + it("does not auto-approve a substitution embedded in a parameter expansion", () => { + // The shell runs $(rm -rf ~/work) when x is unset. The inner command must + // therefore match the lists on its own. Allowing `echo` must not approve + // the payload. + expect(getCommandDecision("echo ${x:-$(rm -rf ~/work)}", ["echo"])).toBe("ask_user") + }) + + it("does not auto-approve a process substitution embedded in a parameter expansion", () => { + // With x unset, bash expands the alternate word and runs `rm a`. bash + // substitutes a device path for it. The inner command must match the + // lists on its own. Allowing `echo` must not approve the payload. + expect(getCommandDecision("echo ${x:-<(rm a)}", ["echo"])).toBe("ask_user") + }) + + it("does not auto-approve a nested substitution riding outer allowlist entries", () => { + // `$(foo $(rm -rf /) bar)` must decompose so that the innermost + // `rm -rf /` is evaluated on its own. A cut at the first ")" would let + // the payload share the allowlist match of the `foo` and `bar` entries. + expect(getCommandDecision("ls $(foo $(rm -rf /) bar)", ["ls", "foo", "bar"])).toBe("ask_user") + }) + + it("asks user when a heredoc opened inside a parameter expansion carries a substitution", () => { + // With x unset, bash expands the alternate word. That runs the heredoc + // body's $(evil), even though the expansion itself stays one segment. + expect(getCommandDecision("cat ${x:-y < { + // bash drops the NUL and runs the glued substitution. The placeholder + // degradation must not leave `git status$(whoami)` looking like one + // allowed word. The parser rejects the input as malformed instead. + expect(getCommandDecision("git status\x00$(whoami)", ["git"])).toBe("malformed_command") + }) + + it("asks user for a substitution on the heredoc opener line after the delimiter", () => { + // The shell expands the opener line's remainder even when the delimiter + // is quoted. The $(whoami) there therefore matches the lists on its own. + expect(getCommandDecision("cat <