Skip to content

fix: harden command auto-approval against inline JS false positives#11382

Merged
hannesrudolph merged 1 commit intomainfrom
fix/harden-command-auto-approval-inline-js
Feb 10, 2026
Merged

fix: harden command auto-approval against inline JS false positives#11382
hannesrudolph merged 1 commit intomainfrom
fix/harden-command-auto-approval-inline-js

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Feb 10, 2026

Summary

Follow-up to #11365. The merged fix narrowed the zsh process-substitution regex with a negative lookbehind (?<![a-zA-Z0-9_]), but this still false-positives on inline JS expressions in node -e commands where =(...) is preceded by characters like ], ), or }.

Example command that was still blocked:

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('statusCounts', (p.items||[]).reduce((a,i)=>(a[i.status]=(a[i.status]||0)+1,a),{}));if(bad.length){process.exit(2);}"

The fragment `=(a[i.status]||0)` inside the JS reducer was matching because `]` is not in `[a-zA-Z0-9_]`.

Changes

1. `src/core/auto-approval/commands.ts` — Regex refinement

Changed zsh process-substitution detector from:
```
/(?<![a-zA-Z0-9_])=\([^)]+\/

to:

/(?:(?<=^)|(?<=[\s;|&(<]))=([^)]+)/

This only matches `=(...)` when preceded by start-of-string or whitespace/shell operators — not arbitrary non-alnum characters that commonly appear in JS expressions.

**True positives preserved:** standalone `=(whoami)`, space-prefixed ` =(ls)`, `echo =(cat /etc/passwd)`, pipe-chained `cmd | =(cmd)`.

### 2. `src/core/tools/ExecuteCommandTool.ts` — Command canonicalization
Moved `unescapeHtmlEntities()` call early and unified all downstream uses (rooignore validation, approval check, timeout allowlist, terminal execution) to use the same `canonicalCommand` value. Previously, the rooignore check used the raw HTML-escaped command while approval used the unescaped form.

### 3. `src/core/auto-approval/__tests__/commands.spec.ts` — Regression tests (10→19)
Added 9 new tests:
- The exact `node -e` one-liner that triggered this investigation
- Arrow function patterns (`(b)=>b`, `(x) => x * 2`, `i=>!set.has(i)`)
- True-positive verification for all 5 dangerous substitution categories
- `getCommandDecision()` integration tests confirming auto-approve with allowlist

## Test Results
- 19/19 commands.spec.ts ✅
- 32/32 full auto-approval suite ✅
- 11/11 ExecuteCommandTool.spec.ts ✅
EOF
)
----
<!-- ELLIPSIS_HIDDEN -->


> [!IMPORTANT]
> Refines regex and command handling to prevent false positives in command auto-approval, with added tests for verification.
> 
>   - **Regex Refinement**:
>     - Updated zsh process-substitution regex in `commands.ts` to only match `=(...)` when preceded by start-of-string or whitespace/shell operators.
>   - **Command Handling**:
>     - Moved `unescapeHtmlEntities()` call earlier in `ExecuteCommandTool.ts` and unified downstream uses to use `canonicalCommand`.
>   - **Testing**:
>     - Added 9 new tests in `commands.spec.ts` for regression and true-positive verification.
>     - Tests include `node -e` one-liner, arrow functions, and dangerous substitution patterns.
> 
> <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=RooCodeInc%2FRoo-Code&utm_source=github&utm_medium=referral)<sup> for e68f89749344662e009ac3ff7d0c171dcd801161. You can [customize](https://app.ellipsis.dev/RooCodeInc/settings/summaries) this summary. It will automatically update as commits are pushed.</sup>


<!-- ELLIPSIS_HIDDEN -->

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Feb 10, 2026
@roomote
Copy link
Contributor

roomote bot commented Feb 10, 2026

Rooviewer Clock   Follow task

Reviewing your PR now. Feedback coming soon!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 10, 2026
@hannesrudolph hannesrudolph merged commit 08a96af into main Feb 10, 2026
20 of 21 checks passed
@hannesrudolph hannesrudolph deleted the fix/harden-command-auto-approval-inline-js branch February 10, 2026 21:44
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Feb 10, 2026
daniel-lxs added a commit that referenced this pull request Feb 13, 2026
* fix: cancel backend auto-approval timeout when auto-approve is toggled off mid-countdown (#11439)

Co-authored-by: Sannidhya <sann@Sannidhyas-MacBook-Pro.local>

* fix: prevent chat history loss during cloud/settings navigation (#11371) (#11372)

Co-authored-by: Sannidhya <sann@Sannidhyas-MacBook-Pro.local>

* fix: preserve pasted images in chatbox during chat activity (#11375)

Co-authored-by: Roo Code <roomote@roocode.com>

* fix: resolve chat scroll anchoring and task-switch scroll race condit… (#11385)

* fix: avoid zsh process-substitution false positives in assignments (#11365)

* fix(editor): make tab close best-effort in DiffViewProvider.open (#11363)

* fix(checkpoints): canonicalize core.worktree comparison to prevent Windows path mismatch failures (#11346)

* fix: prevent double notification sound playback (#11283)

* fix: prevent false unsaved changes prompt with OpenAI Compatible headers (#8230) (#11334)

fix: prevent false unsaved changes prompt with OpenAI Compatible headers

Mark automatic header syncs in ApiOptions and OpenAICompatible as
non-user actions (isUserAction: false) and enhance SettingsView change
detection to skip automatic syncs with semantically equal values.

Root cause: two components (ApiOptions and OpenAICompatible) manage
openAiHeaders state and automatically sync it back on mount/remount.
These syncs were treated as user changes, triggering a false dirty state.

Co-authored-by: Robert McIntyre <robertjmcintyre@users.noreply.github.com>

* fix: remove noisy console.warn logs from NativeToolCallParser (#11264)

Remove two console.warn messages that fire excessively when loading tasks
from history:
- 'Attempting to finalize unknown tool call' in finalizeStreamingToolCall()
- 'Received chunk for unknown tool call' in processStreamingChunk()

The defensive null-return behavior is preserved; only the log output is removed.

* refactor: remove footgun prompting (file-based system prompt override) (#11387)

* refactor: delete orphaned per-provider caching transform files (#11388)

* feat: add disabledTools setting to globally disable native tools (#11277)

* feat: add disabledTools setting to globally disable native tools

Add a disabledTools field to GlobalSettings that allows disabling specific
native tools by name. This enables cloud agents to be configured with
restricted tool access.

Schema:
- Add disabledTools: z.array(toolNamesSchema).optional() to globalSettingsSchema
- Add disabledTools to organizationDefaultSettingsSchema.pick()
- Add disabledTools to ExtensionState Pick type

Prompt generation (tool filtering):
- Add disabledTools to BuildToolsOptions interface
- Pass disabledTools through filterSettings to filterNativeToolsForMode()
- Remove disabled tools from allowedToolNames set in filterNativeToolsForMode()

Execution-time validation (safety net):
- Extract disabledTools from state in presentAssistantMessage
- Convert disabledTools to toolRequirements format for validateToolUse()

Wiring:
- Add disabledTools to ClineProvider getState() and getStateToPostToWebview()
- Pass disabledTools to all buildNativeToolsArrayWithRestrictions() call sites

EXT-778

* fix: check toolRequirements before ALWAYS_AVAILABLE_TOOLS

Moves the toolRequirements check before the ALWAYS_AVAILABLE_TOOLS
early-return in isToolAllowedForMode(). This ensures disabledTools
can block always-available tools (switch_mode, new_task, etc.) at
execution time, making the validation layer consistent with the
filtering layer.

* feat: add support for .agents/skills directory (#11181)

* feat: add support for .agents/skills directory

This change adds support for discovering skills from the .agents/skills
directory, following the Agent Skills convention for sharing skills
across different AI coding tools.

Priority order (later entries override earlier ones):
1. Global ~/.agents/skills (shared across AI coding tools, lowest priority)
2. Project .agents/skills
3. Global ~/.roo/skills (Roo-specific)
4. Project .roo/skills (highest priority)

Changes:
- Add getGlobalAgentsDirectory() and getProjectAgentsDirectoryForCwd()
  functions to roo-config
- Update SkillsManager.getSkillsDirectories() to include .agents/skills
- Update SkillsManager.setupFileWatchers() to watch .agents/skills
- Add tests for new functionality

* fix: clarify skill priority comment to match actual behavior

* fix: clarify skill priority comment to explain Map.set replacement mechanism

---------

Co-authored-by: Roo Code <roomote@roocode.com>

* feat(history): render nested subtasks as recursive tree (#11299)

* feat(history): render nested subtasks as recursive tree

* fix(lockfile): resolve missing ai-sdk provider entry

* fix: address review feedback — dedupe countAll, increase SubtaskRow max-h

- HistoryView: replace local countAll with imported countAllSubtasks from types.ts
- SubtaskRow: increase nested children max-h from 500px to 2000px to match TaskGroupItem

* perf(refactor): consolidate getState calls in resolveWebviewView (#11320)

* perf(refactor): consolidate getState calls in resolveWebviewView

Replace three separate this.getState().then() calls with a single
await this.getState() and destructuring. This avoids running the
full getState() method (CloudService calls, ContextProxy reads, etc.)
three times during webview view resolution.

* fix: keep getState consolidation non-blocking to avoid delaying webview render

---------

Co-authored-by: daniel-lxs <ricciodaniel98@gmail.com>

* fix: harden command auto-approval against inline JS false positives (#11382)

* feat: rename search_and_replace tool to edit and unify edit-family UI (#11296)

* Revert "refactor: delete orphaned per-provider caching transform files (#11388)"

This reverts commit 13a45b0.

* chore: regenerate built-in-skills.ts with updated formatting

* fix: add missing maxReadFileLine property to test baseState

The ExtensionState type now requires maxReadFileLine property (added in commit 63e3f76).
Update the test to include this property with the default value of -1 (unlimited reading).

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* feat: add pnpm serve command for code-server development (#10964)

Co-authored-by: Roo Code <roomote@roocode.com>

* chore: remove Feature Request from issue template options (#11141)

Co-authored-by: Roo Code <roomote@roocode.com>

* refactor(docs-extractor): simplify mode to focus on raw fact extraction (#11129)

* Add cli support for linux (#11167)

* fix: replace heredocs with echo statements in cli-release workflow (#11168)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* Drop MacOS-13 cli support (#11169)

* fix(cli): correct example in install script (#11170)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* feat: add Kimi K2.5 model to Fireworks provider (#11177)

* feat(cli): improve dev experience and roo provider API key support (#11203)

- Allow --api-key and ROO_API_KEY env var for the roo provider instead of
  requiring cloud auth token
- Switch dev/start scripts to use tsx for running directly from source
  without building first
- Fix path resolution (version.ts, extension.ts, extension-host.ts) to
  work from both source and bundled locations
- Disable debug log file (~/.roo/cli-debug.log) unless --debug is passed
- Update README with complete env var table and dev workflow docs

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* Roo Code CLI v0.0.50 (#11204)

* Roo Code CLI v0.0.50

* docs(cli): add --exit-on-error to changelog

---------

Co-authored-by: Roo Code <roomote@roocode.com>

* feat(cli): update default model from Opus 4.5 to Opus 4.6 (#11273)

Co-authored-by: Roo Code <roomote@roocode.com>

* feat(web): replace Roomote Control with Linear Integration in cloud features grid (#11280)

Co-authored-by: Roo Code <roomote@roocode.com>

* Add linux-arm64 for the roo cli (#11314)

* chore: clean up repo-facing mode rules (#11410)

* Make CLI auto-approve by default with require-approval opt-in (#11424)

Co-authored-by: Roo Code <roomote@roocode.com>

* Add new code owners to CODEOWNERS file

* Update next.js (#11108)

* feat(web): Replace bespoke navigation menu with shadcn navigation menu (#11117)

Co-authored-by: Roo Code <roomote@roocode.com>

---------

Co-authored-by: SannidhyaSah <sah_sannidhya@outlook.com>
Co-authored-by: Sannidhya <sann@Sannidhyas-MacBook-Pro.local>
Co-authored-by: roomote[bot] <219738659+roomote[bot]@users.noreply.github.com>
Co-authored-by: Roo Code <roomote@roocode.com>
Co-authored-by: Hannes Rudolph <hrudolph@gmail.com>
Co-authored-by: 0xMink <dennis@dennismink.com>
Co-authored-by: Robert McIntyre <robertjmcintyre@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Matt Rubens <mrubens@users.noreply.github.com>
Co-authored-by: Chris Estreich <cestreich@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants