Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 126 additions & 0 deletions src/core/auto-approval/__tests__/checkAutoApproval.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import { checkAutoApproval } from "../index"
import type { ExtensionState } from "@roo-code/types"

function makeState(overrides: Record<string, unknown> = {}) {
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<ExtensionState, any>
}

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")
})
})
40 changes: 40 additions & 0 deletions src/core/auto-approval/__tests__/commands.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand Down
11 changes: 11 additions & 0 deletions src/core/auto-approval/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
}
Expand Down
Loading