[BUG] [SECURITY] [CRITICAL] Auto-Approve: Denied command filter bypass

[PlanBernhard]

Problem (one or two sentences)

The agent ignores the denied command rm, he bypasses it with such code:

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

Context (who is affected and when)

Everyone

Reproduction steps

1. Open Settings -> Auto Approve
2. Write inside "Denied Commands" the word "rm" -> Add
3. Press "Save"
4. Now the agent is still be able to use rm without approval

Expected result

Asking for approval

Actual result

The agent could execute the code without approval

Variations tried (optional)

No response

App Version

Version: 3.53.0 (44f6bba)

API Provider (optional)

None

Model Used (optional)

No response

Roo Code Task Links (optional)

No response

Relevant logs or errors (optional)

I've analyzed this issue and here's my proposed implementation plan:

Root Cause:
The denied commands check in src/core/auto-approval/index.ts (lines 114-130) is nested inside the if (state.alwaysAllowExecute === true) guard. This means denied commands are only evaluated when the "Always approve execute_command" toggle is enabled. If alwaysAllowExecute is false, the denied commands list is completely ignored -- the function falls through to { decision: "ask" }, which still lets the user manually approve a command that should be blocked.

Even more critically, the entire auto-approval function is gated by if (!state || !state.autoApprovalEnabled) on line 62, so if auto-approval is disabled, denied commands have no effect at all.

The expected behavior (matching the issue report) is that denied commands should act as an absolute block -- commands matching the deny list should be auto-denied regardless of whether alwaysAllowExecute or autoApprovalEnabled is enabled.

Proposed Fix:

1. Move denied commands check before the alwaysAllowExecute guard in checkAutoApproval() in src/core/auto-approval/index.ts. Specifically:

   • Extract a new helper (e.g., isCommandDenied) from getCommandDecision that only checks the deny list
   • Add a denied-commands check for ask === "command" that runs before the autoApprovalEnabled early return (line 62), so denied commands are always enforced
   • If the command matches the deny list (and is not overridden by a more specific allowed-command prefix), return { decision: "deny" } immediately

2. Add tests in src/core/auto-approval/__tests__/commands.spec.ts to verify:

   • Denied commands are blocked when alwaysAllowExecute is false
   • Denied commands are blocked when autoApprovalEnabled is false
   • Denied commands inside && chains are caught (the heredoc scenario from the issue)
   • Longest-prefix-match still works (e.g., allowed rm -i overrides denied rm)

This approach ensures the deny list works as a safety net that cannot be bypassed, while preserving the existing longest-prefix-match conflict resolution logic.

Please mention @roomote in a comment saying that you approve this plan, and I'll proceed with the implementation.

[PlanBernhard]

Thanks @roomote
It happens all the time. It should be searched for words inside textblocks not if a command is the first command. the agent wrappes code inside previous commands to bypass security

Thanks @PlanBernhard for the feedback. I've seen your comment and will explore a fix that searches for denied commands anywhere within the command text (including chained commands with &&, ||, ;, heredocs, etc.), not just as the first command. I'll open a PR to share progress.

Opened PR #12293 with my attempt at addressing this. The fix moves the denied commands check to run before the auto-approval guard, so denied commands are always enforced regardless of settings. Chained commands (&&, ||, ;, |) and multi-line scripts are already parsed by the existing command parser, so denied commands within them are caught too. Feedback welcome!

[PlanBernhard]

Thank @roomote , I changed the title to CRITICAL

Thanks for flagging this as CRITICAL, @PlanBernhard. PR #12293 is already open and CI is passing. The fix ensures denied commands are always enforced regardless of auto-approval settings, and the existing command parser splits chains (&&, ||, ;, |) into individual sub-commands so denied commands wrapped inside chains or multi-line scripts are caught too.

[PlanBernhard]

@roomote , just an idea: I think it would help - if possible - when denied commads inside those textblocks have a red font color. If the check did not work the user can see it immediately

Thanks @PlanBernhard for the suggestion! I've seen your comment about adding red font color to denied commands in the settings UI. I'll explore implementing this and open a PR to share progress.

Opened PR #12294 with my attempt at adding red styling to denied commands in the Auto Approve settings. Denied command tags now display with red text and a subtle red background tint to make them visually distinct from allowed commands. Feedback welcome!