fix(rules): silence false positives in agent/mcp-tool-capability-risk and server-sequential-independent-await (#838, #839)

[devin-ai-integration]

Summary

Fixes two independent false positives in oxlint-plugin-react-doctor rules. Each fix has a regression test, and the new util has its own unit test.

Note: this branch originally also fixed #837 (url-prefilled-privileged-action), but that was resolved independently on main by #843. After merging main, this branch carries main's #837 fix verbatim and no longer contributes its own — so this PR is now scoped to #838 and #839.

#838 — agent-tool-capability-risk / mcp-tool-capability-risk

The dangerous-capability gate matched its keywords against comment-stripped content, so prose inside a tool's description string (e.g. "ALWAYS fetch the underlying numbers first") satisfied the gate and the tool was flagged even though it exposes nothing.

Fix: capability keywords must now match in code only. Added scanByPattern({ requireAllInCode }), gated on a new content view that blanks string-literal prose in addition to comments:

getCodeOnlyContent(file) = stripStringLiteralsKeepingModuleSpecifiers(getScannableContent(file))

Both rules move AGENT_TOOL_DANGEROUS_CAPABILITY_PATTERN from requireAll → requireAllInCode. The MCP import gate stays in requireAll because it intentionally matches the from "@modelcontextprotocol/sdk…" import string.

stripStringLiteralsKeepingModuleSpecifiers is a stack-based scanner that blanks '/"/` contents with spaces while preserving delimiters, newlines, and every offset (so reported line/column stay correct). Two kinds of string content are deliberately kept, because they are code rather than prose:

• module-specifier strings — from "node:child_process", require("node:fs") — so a dangerous import still trips the gate (member access like Buffer.from("…") is excluded).
• template ${…} interpolations — a capability call written only inside an interpolation (`${exec(cmd)}`) must still count; the scanner descends into the expression, balancing brace depth, and blanks only the surrounding template text.

A handler that actually calls exec/fetch/readFile/etc. still fires.

#839 — server-sequential-independent-await

collectDeclaredNames only handled Identifier elements inside an ArrayPattern, so the names bound by const [{ slug }, { isEnabled }] = await Promise.all(...) were never recorded. The next await reading slug/isEnabled therefore looked independent and was wrongly flagged as a waterfall.

Fix: reuse the existing recursive collectPatternNames util instead of the rule's incomplete inline logic, so all nested destructuring patterns (object-in-array, deep { data: { token } }, defaults, rest) are captured.

for (const declarator of declaration.declarations ?? []) {
  collectPatternNames(declarator.id, names);
}

Testing

• New/updated regression tests for both rules + a unit test for the new util (incl. module-specifier and ${…} interpolation cases).
• oxlint-plugin-react-doctor security-scan + server suites: 222 passing.
• @react-doctor/core suite (incl. the check-security-scan integration test): 841 passing.
• Typecheck (401 rules), lint, and format check clean.

Link to Devin session: https://app.devin.ai/sessions/448036f10f8f4ac8bea23f600e6e74b3
Requested by: @aidenybai

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/eslint-plugin-react-doctor@841

npm i https://pkg.pr.new/oxlint-plugin-react-doctor@841

npm i https://pkg.pr.new/react-doctor@841

commit: 2cdd5a1

[github-actions]

No React Doctor issues found. 🎉

_{Reviewed by React Doctor for commit 2cdd5a1.}

[devin-ai-integration]

Devin Review found 1 potential issue.

[devin-ai-integration]

🟡 Template literal expressions (${...}) are incorrectly blanked as string content, causing false negatives

The stripStringLiteralsKeepingModuleSpecifiers function treats backtick-delimited template literals as simple strings, blanking all content between the opening and closing backtick. However, template literals can contain ${...} interpolation expressions that are executable code, not prose. For example, a tool handler like:

execute: async ({ cmd }) => `Result: ${exec(cmd)}`

would have exec(cmd) blanked because it's "inside" the template literal, even though it's a real function call. Since requireAllInCode gates (used by agent-tool-capability-risk and mcp-tool-capability-risk) test against this code-only view, dangerous capabilities inside template expressions will not be detected — the rules will produce false negatives for genuinely dangerous tools that happen to invoke exec/fetch/spawn etc. inside template expressions.

Prompt for agents

The stripStringLiteralsKeepingModuleSpecifiers function in packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.ts needs to handle template literal expressions (${...}). Currently, when a backtick is encountered, it blanks everything until the closing backtick, including code inside ${...} interpolation expressions.

The fix requires tracking a nesting depth for template expressions. When inside a backtick-delimited string and a `${` sequence is encountered, the parser should stop treating the content as string text and instead treat what follows as code (i.e., don't blank it). It needs to track brace depth so it knows when the `}` that closes the expression is found (vs nested braces in objects/blocks). After the closing `}`, it resumes blanking template literal text.

This requires changes to the main while loop: when `stringDelimiter` is backtick and the current character is `$` followed by `{`, push into a code context (stop blanking). Track brace depth. When depth returns to 0 on a `}`, resume the template literal blanking mode. Note that template expressions can themselves contain nested template literals, so a stack-based approach may be needed.

The impact is limited because most tool handler code uses regular function calls outside of template expressions, but this is a correctness gap that can cause false negatives in the security scanning rules.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Fixed in 90a3469. The stripper is now interpolation-aware: inside a backtick literal it blanks only the literal text and treats ${…} as code, balancing brace depth (so a } from a nested object/block doesn't end the expression early) and descending into nested strings/templates within the interpolation. So execute: async ({ cmd }) => `Result: ${exec(cmd)}` now keeps exec(cmd) in the code-only view and the capability rules fire. Covered by a new util test (interpolation kept, nested prose string blanked) and an agent-tool-capability-risk regression test for a handler that calls a capability inside ${…}.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 2cdd5a1. Configure here.}

[cursor]

Destructuring keys mask dependency

Low Severity

After nested destructuring names are collected correctly, declarationReadsAnyName still walks the entire next const declaration. A destructuring property key whose identifier matches an earlier binding (e.g. { slug: title } after [{ slug }]) is treated as using that binding even when the await initializer does not reference it, so independent sequential awaits can be missed.

 

^{Reviewed by Cursor Bugbot for commit 2cdd5a1. Configure here.}