diff --git a/src/core/auto-approval/__tests__/checkAutoApproval.spec.ts b/src/core/auto-approval/__tests__/checkAutoApproval.spec.ts new file mode 100644 index 00000000000..736899178e7 --- /dev/null +++ b/src/core/auto-approval/__tests__/checkAutoApproval.spec.ts @@ -0,0 +1,126 @@ +import { checkAutoApproval } from "../index" +import type { ExtensionState } from "@roo-code/types" + +function makeState(overrides: Record = {}) { + return { + autoApprovalEnabled: false, + alwaysAllowReadOnly: false, + alwaysAllowWrite: false, + alwaysAllowExecute: false, + alwaysAllowMcp: false, + alwaysAllowModeSwitch: false, + alwaysAllowSubtasks: false, + alwaysAllowFollowupQuestions: false, + alwaysAllowReadOnlyOutsideWorkspace: false, + alwaysAllowWriteOutsideWorkspace: false, + alwaysAllowWriteProtected: false, + followupAutoApproveTimeoutMs: 0, + mcpServers: {}, + deniedCommands: [] as string[], + allowedCommands: [] as string[], + ...overrides, + } as Pick +} + +describe("checkAutoApproval — denied commands enforcement", () => { + const deniedCommands = ["rm", "sudo"] + + it("should deny a command matching the deny list even when autoApprovalEnabled is false", async () => { + const result = await checkAutoApproval({ + state: makeState({ deniedCommands }), + ask: "command", + text: "rm -rf /tmp/test", + }) + + expect(result.decision).toBe("deny") + }) + + it("should deny a command matching the deny list even when alwaysAllowExecute is false", async () => { + const result = await checkAutoApproval({ + state: makeState({ autoApprovalEnabled: true, deniedCommands }), + ask: "command", + text: "sudo apt install something", + }) + + expect(result.decision).toBe("deny") + }) + + it("should deny a chained command where denied command appears after &&", async () => { + const result = await checkAutoApproval({ + state: makeState({ deniedCommands }), + ask: "command", + text: "cat file.txt && rm file.txt", + }) + + expect(result.decision).toBe("deny") + }) + + it("should deny the exact heredoc bypass scenario from the issue", async () => { + const command = [ + "cat > verify-hook-install.nu << 'HEREDOC'", + "use scripts/development/modules/nu/install_hooks.nu [install-git-hooks]", + "let project_root = ($env | get -o FILE_PWD | default (pwd))", + "install-git-hooks $project_root", + "HEREDOC", + "nu verify-hook-install.nu && rm verify-hook-install.nu", + ].join("\n") + + const result = await checkAutoApproval({ + state: makeState({ + autoApprovalEnabled: true, + alwaysAllowExecute: true, + deniedCommands: ["rm"], + allowedCommands: ["cat", "nu"], + }), + ask: "command", + text: command, + }) + + expect(result.decision).toBe("deny") + }) + + it("should return 'ask' for a non-denied command when autoApprovalEnabled is false", async () => { + const result = await checkAutoApproval({ + state: makeState({ deniedCommands }), + ask: "command", + text: "git status", + }) + + expect(result.decision).toBe("ask") + }) + + it("should not deny when deny list is empty", async () => { + const result = await checkAutoApproval({ + state: makeState(), + ask: "command", + text: "rm -rf /tmp/test", + }) + + expect(result.decision).toBe("ask") + }) + + it("should respect longest prefix match: allowed 'rm -i' overrides denied 'rm'", async () => { + const result = await checkAutoApproval({ + state: makeState({ + autoApprovalEnabled: true, + alwaysAllowExecute: true, + deniedCommands: ["rm"], + allowedCommands: ["rm -i"], + }), + ask: "command", + text: "rm -i file.txt", + }) + + expect(result.decision).toBe("approve") + }) + + it("should return 'ask' when state is undefined", async () => { + const result = await checkAutoApproval({ + state: undefined, + ask: "command", + text: "rm file", + }) + + expect(result.decision).toBe("ask") + }) +}) diff --git a/src/core/auto-approval/__tests__/commands.spec.ts b/src/core/auto-approval/__tests__/commands.spec.ts index 90d2808ba9c..09528b012b8 100644 --- a/src/core/auto-approval/__tests__/commands.spec.ts +++ b/src/core/auto-approval/__tests__/commands.spec.ts @@ -82,6 +82,46 @@ describe("containsDangerousSubstitution — true positives still caught", () => }) }) +describe("getCommandDecision — denied commands in chained/wrapped commands", () => { + it("should auto_deny when denied command appears after && in a chain", () => { + expect(getCommandDecision("cat file.txt && rm file.txt", [], ["rm"])).toBe("auto_deny") + }) + + it("should auto_deny when denied command appears after || in a chain", () => { + expect(getCommandDecision("test -f file || rm file", [], ["rm"])).toBe("auto_deny") + }) + + it("should auto_deny when denied command appears after ; in a chain", () => { + expect(getCommandDecision("echo done; rm -rf /tmp/test", [], ["rm"])).toBe("auto_deny") + }) + + it("should auto_deny when denied command appears after pipe", () => { + expect(getCommandDecision("ls | rm file", [], ["rm"])).toBe("auto_deny") + }) + + it("should auto_deny for heredoc-style bypass with rm at end (multi-line)", () => { + const command = `cat > script.sh << 'HEREDOC'\necho hello\nHEREDOC\nnu script.sh && rm script.sh` + expect(getCommandDecision(command, [], ["rm"])).toBe("auto_deny") + }) + + it("should auto_deny when denied command is the first in a chain", () => { + expect(getCommandDecision("rm file && echo done", [], ["rm"])).toBe("auto_deny") + }) + + it("should auto_deny for the exact issue scenario", () => { + const command = `cat > verify-hook-install.nu << 'HEREDOC'\nuse scripts/development/modules/nu/install_hooks.nu [install-git-hooks]\nlet project_root = ($env | get -o FILE_PWD | default (pwd))\ninstall-git-hooks $project_root\nHEREDOC\nnu verify-hook-install.nu && rm verify-hook-install.nu` + expect(getCommandDecision(command, [], ["rm"])).toBe("auto_deny") + }) + + it("should not deny when denied command is not present", () => { + expect(getCommandDecision("git status && echo done", [], ["rm"])).toBe("ask_user") + }) + + it("should respect longest prefix match: allowed 'rm -i' overrides denied 'rm'", () => { + expect(getCommandDecision("rm -i file.txt", ["rm -i"], ["rm"])).toBe("auto_approve") + }) +}) + describe("getCommandDecision — integration with dangerous substitution checks", () => { const allowedCommands = ["node", "echo"] diff --git a/src/core/auto-approval/index.ts b/src/core/auto-approval/index.ts index c8293c2a79f..b8ff8cb9f60 100644 --- a/src/core/auto-approval/index.ts +++ b/src/core/auto-approval/index.ts @@ -59,6 +59,17 @@ export async function checkAutoApproval({ return { decision: "approve" } } + // Always enforce denied commands regardless of auto-approval settings. + // This prevents agents from bypassing the deny list by wrapping denied + // commands inside chains (&&, ||, ;) or multi-line scripts. + if (ask === "command" && text && state?.deniedCommands?.length) { + const decision = getCommandDecision(text, state.allowedCommands || [], state.deniedCommands) + + if (decision === "auto_deny") { + return { decision: "deny" } + } + } + if (!state || !state.autoApprovalEnabled) { return { decision: "ask" } }