From 9eca626e285a225641bec18af076f35d9fff1918 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 16 Jun 2026 10:01:54 +0000 Subject: [PATCH 1/4] fix(rules): silence false positives in 3 security/server rules (#838, #837, #839) Co-Authored-By: Aiden Bai --- ...ix-security-server-rule-false-positives.md | 23 ++++++++ ...t-tool-capability-risk.regressions.test.ts | 57 +++++++++++++++++++ .../agent-tool-capability-risk.ts | 4 +- .../security-scan/mcp-tool-capability-risk.ts | 5 +- ...lled-privileged-action.regressions.test.ts | 24 ++++++++ .../url-prefilled-privileged-action.ts | 6 +- .../security-scan/utils/scan-by-pattern.ts | 32 ++++++++++- ...ring-literals-preserving-positions.test.ts | 28 +++++++++ ...ip-string-literals-preserving-positions.ts | 46 +++++++++++++++ ...erver-sequential-independent-await.test.ts | 52 +++++++++++++++++ .../server-sequential-independent-await.ts | 20 +------ 11 files changed, 275 insertions(+), 22 deletions(-) create mode 100644 .changeset/fix-security-server-rule-false-positives.md create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.regressions.test.ts create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.test.ts create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.ts create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.test.ts diff --git a/.changeset/fix-security-server-rule-false-positives.md b/.changeset/fix-security-server-rule-false-positives.md new file mode 100644 index 000000000..553c613fd --- /dev/null +++ b/.changeset/fix-security-server-rule-false-positives.md @@ -0,0 +1,23 @@ +--- +"oxlint-plugin-react-doctor": patch +--- + +Fix false positives in three rules (#838, #837, #839). + +- `agent-tool-capability-risk` / `mcp-tool-capability-risk` (#838): capability + keywords are now matched in code only. Words inside a tool's `description` + string (e.g. "ALWAYS fetch the underlying numbers first") and the AI-SDK + `execute:` handler key no longer satisfy the dangerous-capability gate. A + handler that actually calls `exec`/`fetch`/`readFile`/etc. still fires. + +- `url-prefilled-privileged-action` (#837): the validating-helper lookbehind now + allows a member-access object between the helper call and the read, so + `sanitizeAuthCallbackURL(url.searchParams.get("callbackURL"))` and + `resolveSafeAuthCallbackURL(url.searchParams.get(...))` are recognized as + validated and suppressed. + +- `server-sequential-independent-await` (#839): declared-name collection now + recurses into nested destructuring patterns, so + `const [{ slug }, { isEnabled }] = await Promise.all(...)` followed by an + `await` that reads `slug`/`isEnabled` is correctly treated as dependent and + not flagged as a waterfall. diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.regressions.test.ts new file mode 100644 index 000000000..d01fc2f37 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.regressions.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from "vite-plus/test"; +import { runScanRule } from "../../../test-utils/run-scan-rule.js"; +import { agentToolCapabilityRisk } from "./agent-tool-capability-risk.js"; + +describe("security-scan/agent-tool-capability-risk — regressions", () => { + it("stays silent when capability words appear only in the description and execute handler key", () => { + const content = [ + 'import { tool } from "ai";', + "export const listItems = tool({", + ' description: "List items. ALWAYS fetch the underlying numbers first.",', + " inputSchema: z.object({ organizationId: z.string() }),", + " execute: async ({ organizationId }) => {", + ' if (organizationId !== allowedOrgId) return { error: "Access denied" };', + " return prisma.item.findMany({ where: { organizationId } });", + " },", + "});", + ].join("\n"); + const findings = runScanRule(agentToolCapabilityRisk, { + relativePath: "src/lib/tools/campaign-stats.ts", + content, + }); + expect(findings).toHaveLength(0); + }); + + it("flags a tool handler that actually shells out", () => { + const content = [ + 'import { tool } from "ai";', + "export const runShell = tool({", + ' description: "Run a command",', + " inputSchema: z.object({ cmd: z.string() }),", + " execute: async ({ cmd }) => {", + " return exec(cmd);", + " },", + "});", + ].join("\n"); + const findings = runScanRule(agentToolCapabilityRisk, { + relativePath: "src/lib/tools/run-shell.ts", + content, + }); + expect(findings).toHaveLength(1); + }); + + it("flags a tool handler that actually calls fetch in code", () => { + const content = [ + 'import { tool } from "ai";', + "export const getData = tool({", + ' description: "Get data",', + " execute: async ({ url }) => fetch(url),", + "});", + ].join("\n"); + const findings = runScanRule(agentToolCapabilityRisk, { + relativePath: "src/lib/tools/get-data.ts", + content, + }); + expect(findings).toHaveLength(1); + }); +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.ts index 0fbb18d09..c11c6abcc 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.ts @@ -20,7 +20,9 @@ export const agentToolCapabilityRisk = defineRule({ isProductionSourcePath(file.relativePath) && AGENT_TOOL_CONTEXT_PATH_PATTERN.test(file.relativePath), pattern: AGENT_TOOL_DEFINITION_PATTERN, - requireAll: [AGENT_TOOL_DANGEROUS_CAPABILITY_PATTERN], + // In code only: the AI-SDK `execute:` handler key and capability words in + // `description` prose ("...fetch the numbers...") must not satisfy the gate. + requireAllInCode: [AGENT_TOOL_DANGEROUS_CAPABILITY_PATTERN], message: "An agent-callable tool appears to expose network, filesystem, shell, or code-execution capability.", }), diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/mcp-tool-capability-risk.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/mcp-tool-capability-risk.ts index 675ce8d3b..ca92ce835 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/mcp-tool-capability-risk.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/mcp-tool-capability-risk.ts @@ -24,7 +24,10 @@ export const mcpToolCapabilityRisk = defineRule({ scan: scanByPattern({ shouldScan: (file) => isProductionSourcePath(file.relativePath), pattern: MCP_TOOL_SURFACE_PATTERN, - requireAll: [MCP_IMPORT_PATTERN, AGENT_TOOL_DANGEROUS_CAPABILITY_PATTERN], + requireAll: [MCP_IMPORT_PATTERN], + // In code only: a capability word inside a tool `description` string is + // documentation, not an exposed primitive, so it must not satisfy the gate. + requireAllInCode: [AGENT_TOOL_DANGEROUS_CAPABILITY_PATTERN], message: "An MCP tool/resource/prompt handler appears to expose file, shell, network, or code-execution capability.", }), diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/url-prefilled-privileged-action.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/url-prefilled-privileged-action.regressions.test.ts index 2a537a154..f7f3c9e6f 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/url-prefilled-privileged-action.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/url-prefilled-privileged-action.regressions.test.ts @@ -34,4 +34,28 @@ describe("security-scan/url-prefilled-privileged-action — regressions", () => }); expect(findings).toHaveLength(0); }); + + it("stays silent when a member-access read is wrapped in an infix-named validator (resolveSafe…)", () => { + const findings = runScanRule(urlPrefilledPrivilegedAction, { + relativePath: "src/app/login/page.tsx", + content: `url.searchParams.set("callbackURL",\n resolveSafeAuthCallbackURL(url.searchParams.get("callbackURL")));\n`, + }); + expect(findings).toHaveLength(0); + }); + + it("stays silent when an aliased sanitiz*-named validator wraps a member-access read", () => { + const findings = runScanRule(urlPrefilledPrivilegedAction, { + relativePath: "src/app/login/page.tsx", + content: `import { resolveSafeAuthCallbackURL as sanitizeAuthCallbackURL } from "~/lib/auth-callback";\nconst safe = sanitizeAuthCallbackURL(url.searchParams.get("callbackURL"));\n`, + }); + expect(findings).toHaveLength(0); + }); + + it("still flags an unvalidated callbackUrl read passed through a non-validating call", () => { + const findings = runScanRule(urlPrefilledPrivilegedAction, { + relativePath: "src/app/login/page.tsx", + content: `const callback = decodeURIComponent(url.searchParams.get("callbackUrl"));\n`, + }); + expect(findings).toHaveLength(1); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/url-prefilled-privileged-action.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/url-prefilled-privileged-action.ts index c15bee39e..fc642acfa 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/url-prefilled-privileged-action.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/url-prefilled-privileged-action.ts @@ -7,8 +7,12 @@ import { scanByPattern } from "./utils/scan-by-pattern.js"; // No `email`/`user`: prefilled emails and username route params are benign // booking/contact UX, not privileged actions. The lookbehind skips reads // already wrapped in a validating helper (`getRelativeNextPath(...get("next"))`). +// The validator name is matched as an infix (lookbehind scans every suffix, so +// `resolveSafeAuthCallbackURL(...)` is recognized via its `Safe` segment), and +// a member-access object between the helper and the read is allowed so +// `sanitizeAuthCallbackURL(url.searchParams.get("callbackURL"))` is suppressed. const PRIVILEGED_QUERY_PARAM_PATTERN = - /(? boolean; @@ -11,6 +12,10 @@ export interface ScanByPatternInput { // Conjunction gates: every pattern must also match somewhere in the file // (e.g. an MCP import that proves the matched tool surface is MCP). readonly requireAll?: ReadonlyArray; + // Conjunction gates that must match in CODE only — string-literal contents + // (and comments) are blanked before testing, so a capability keyword that + // appears solely inside a `description` string or prose doesn't count. + readonly requireAllInCode?: ReadonlyArray; // Veto: a match anywhere in the file suppresses the finding (e.g. a // signature-verification call that answers the rule's concern). readonly suppressWhen?: RegExp; @@ -18,6 +23,7 @@ export interface ScanByPatternInput { } const strippedContentCache = new WeakMap(); +const codeOnlyContentCache = new WeakMap(); // Comments are a recurring false-positive source ("Ajv compiles schemas via // `new Function(...)`"); blank them for JS/TS files before pattern matching. @@ -31,14 +37,38 @@ export const getScannableContent = (file: ScannedFile): string => { return strippedContent; }; +// Comments AND string-literal contents blanked — the "is this keyword in real +// code?" view used by `requireAllInCode` gates so prose inside `description` +// strings can't satisfy a capability gate. +const getCodeOnlyContent = (file: ScannedFile): string => { + const cachedContent = codeOnlyContentCache.get(file); + if (cachedContent !== undefined) return cachedContent; + const codeOnlyContent = stripStringLiteralsPreservingPositions(getScannableContent(file)); + codeOnlyContentCache.set(file, codeOnlyContent); + return codeOnlyContent; +}; + export const scanByPattern = - ({ shouldScan, pattern, requireAll, suppressWhen, message }: ScanByPatternInput): FileScan => + ({ + shouldScan, + pattern, + requireAll, + requireAllInCode, + suppressWhen, + message, + }: ScanByPatternInput): FileScan => (file) => { if (!shouldScan(file)) return []; const content = getScannableContent(file); if (requireAll !== undefined && !requireAll.every((gate) => gate.test(content))) { return []; } + if ( + requireAllInCode !== undefined && + !requireAllInCode.every((gate) => gate.test(getCodeOnlyContent(file))) + ) { + return []; + } const patterns = pattern instanceof RegExp ? [pattern] : pattern; const matchedPattern = patterns.find((candidate) => candidate.test(content)); if (matchedPattern === undefined) return []; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.test.ts new file mode 100644 index 000000000..7ab4c56ca --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from "vite-plus/test"; +import { stripStringLiteralsPreservingPositions } from "./strip-string-literals-preserving-positions.js"; + +describe("security-scan/utils/strip-string-literals-preserving-positions", () => { + it("blanks string contents while preserving offsets, delimiters, and newlines", () => { + const source = `const description = "ALWAYS fetch the numbers first";\nconst run = exec(cmd);`; + const stripped = stripStringLiteralsPreservingPositions(source); + expect(stripped).toHaveLength(source.length); + expect(stripped).not.toContain("fetch"); + expect(stripped.split("\n")[0]).toMatch(/^const description = " +";$/); + expect(stripped.split("\n")[1]).toBe("const run = exec(cmd);"); + }); + + it("blanks template-literal text but keeps newlines for multi-line strings", () => { + const source = "const help = `line one\nfetch line two`;\nconst keep = true;"; + const stripped = stripStringLiteralsPreservingPositions(source); + expect(stripped).not.toContain("fetch"); + expect(stripped.split("\n")).toHaveLength(3); + expect(stripped.split("\n")[2]).toBe("const keep = true;"); + }); + + it("does not let an escaped quote close the string early", () => { + const source = `const note = "say \\"exec\\" out loud"; const real = spawn(cmd);`; + const stripped = stripStringLiteralsPreservingPositions(source); + expect(stripped).not.toContain("exec"); + expect(stripped).toContain("spawn(cmd)"); + }); +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.ts new file mode 100644 index 000000000..7be9bdb26 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.ts @@ -0,0 +1,46 @@ +// Pattern scans repeatedly match capability keywords that live only inside +// string literals — a tool's human-readable `description`, prose like +// "ALWAYS fetch the underlying numbers first". This blanks string-literal +// contents with spaces so a keyword gate can require a real code occurrence; +// the delimiters, newlines, and every offset are preserved so match indices, +// lines, and columns still map 1:1 onto the original file. Expects +// comment-stripped input so a quote inside a comment is never treated as a +// string delimiter. +export const stripStringLiteralsPreservingPositions = (content: string): string => { + const characters = content.split(""); + let stringDelimiter: string | null = null; + let index = 0; + + while (index < content.length) { + const character = content[index]; + + if (stringDelimiter !== null) { + if (character === "\\") { + characters[index] = " "; + if (content[index + 1] !== undefined && content[index + 1] !== "\n") { + characters[index + 1] = " "; + } + index += 2; + continue; + } + if (character === stringDelimiter) { + stringDelimiter = null; + index += 1; + continue; + } + if (character !== "\n") characters[index] = " "; + index += 1; + continue; + } + + if (character === '"' || character === "'" || character === "`") { + stringDelimiter = character; + index += 1; + continue; + } + + index += 1; + } + + return characters.join(""); +}; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.test.ts new file mode 100644 index 000000000..e8b8e321d --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from "vite-plus/test"; +import { runRule } from "../../../test-utils/run-rule.js"; +import { serverSequentialIndependentAwait } from "./server-sequential-independent-await.js"; + +describe("server-sequential-independent-await", () => { + it("flags two genuinely independent sequential awaits", () => { + const result = runRule( + serverSequentialIndependentAwait, + `export const load = async () => { + const user = await fetchUser(); + const posts = await fetchPosts(); + return { user, posts }; +};`, + { filename: "/repo/app/page.tsx" }, + ); + + expect(result.diagnostics).toHaveLength(1); + }); + + it("stays silent when the second await reads names destructured from the first (array of object patterns)", () => { + const result = runRule( + serverSequentialIndependentAwait, + `export default async function Page({ params }: PageProps<"/preview/blog/[slug]">) { + const [{ slug }, { isEnabled }] = await Promise.all([params, draftMode()]); + const data = await client.fetch( + BlogPostQuery, + { slug }, + isEnabled ? { perspective: "drafts", stega: true } : { next: { revalidate: 3600 } }, + ); + if (!data) notFound(); + return ; +}`, + { filename: "/repo/app/(site)/preview/blog/[slug]/page.tsx" }, + ); + + expect(result.diagnostics).toHaveLength(0); + }); + + it("stays silent when the second await reads a deeply nested destructured binding", () => { + const result = runRule( + serverSequentialIndependentAwait, + `export const load = async () => { + const { data: { token } } = await getSession(); + const profile = await fetchProfile(token); + return profile; +};`, + { filename: "/repo/app/page.tsx" }, + ); + + expect(result.diagnostics).toHaveLength(0); + }); +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.ts index 14ef8f3f8..8733eaaf8 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/server/server-sequential-independent-await.ts @@ -1,3 +1,4 @@ +import { collectPatternNames } from "../../utils/collect-pattern-names.js"; import { defineRule } from "../../utils/define-rule.js"; import { walkAst } from "../../utils/walk-ast.js"; import type { EsTreeNode } from "../../utils/es-tree-node.js"; @@ -19,24 +20,7 @@ const collectDeclaredNames = (declaration: EsTreeNode): Set => { const names = new Set(); if (!isNodeOfType(declaration, "VariableDeclaration")) return names; for (const declarator of declaration.declarations ?? []) { - if (isNodeOfType(declarator.id, "Identifier")) { - names.add(declarator.id.name); - } else if (isNodeOfType(declarator.id, "ObjectPattern")) { - for (const property of declarator.id.properties ?? []) { - if (isNodeOfType(property, "Property") && isNodeOfType(property.value, "Identifier")) { - names.add(property.value.name); - } else if ( - isNodeOfType(property, "RestElement") && - isNodeOfType(property.argument, "Identifier") - ) { - names.add(property.argument.name); - } - } - } else if (isNodeOfType(declarator.id, "ArrayPattern")) { - for (const element of declarator.id.elements ?? []) { - if (isNodeOfType(element, "Identifier")) names.add(element.name); - } - } + collectPatternNames(declarator.id, names); } return names; }; From 88a55b45c172ff37f1393daf2247fc6bc05ca54c Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 16 Jun 2026 10:11:55 +0000 Subject: [PATCH 2/4] fix(security-scan): keep import specifiers in capability code-only view A dangerous capability named only in an import path (e.g. `from "node:child_process"`) is a real signal, but the code-only view used by requireAllInCode was blanking all string literals, so agent-tool-capability-risk stopped firing on such tools. Preserve module-specifier strings while still blanking prose strings like a tool `description`. Co-Authored-By: Aiden Bai --- ...t-tool-capability-risk.regressions.test.ts | 19 ++++++ .../security-scan/utils/scan-by-pattern.ts | 18 ++--- ...literals-keeping-module-specifiers.test.ts | 49 +++++++++++++ ...ring-literals-keeping-module-specifiers.ts | 68 +++++++++++++++++++ ...ring-literals-preserving-positions.test.ts | 28 -------- ...ip-string-literals-preserving-positions.ts | 46 ------------- 6 files changed, 146 insertions(+), 82 deletions(-) create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.test.ts create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.ts delete mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.test.ts delete mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.ts diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.regressions.test.ts index d01fc2f37..c7efb93a9 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.regressions.test.ts @@ -54,4 +54,23 @@ describe("security-scan/agent-tool-capability-risk — regressions", () => { }); expect(findings).toHaveLength(1); }); + + it("flags a tool that names a dangerous module in its import specifier", () => { + const content = [ + 'import { tool } from "ai";', + 'import { execFile } from "node:child_process";', + "export const runCommandTool = tool({", + ' description: "Run a repository maintenance command",', + " execute: async ({ command }) => {", + " execFile(command, []);", + " return { ok: true };", + " },", + "});", + ].join("\n"); + const findings = runScanRule(agentToolCapabilityRisk, { + relativePath: "src/agents/tools/run-command-tool.ts", + content, + }); + expect(findings).toHaveLength(1); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/scan-by-pattern.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/scan-by-pattern.ts index 997e2d077..4815acbf3 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/scan-by-pattern.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/scan-by-pattern.ts @@ -2,7 +2,7 @@ import { SOURCE_FILE_PATTERN } from "../../../constants/security-scan.js"; import type { FileScan, ScannedFile } from "../../../utils/file-scan.js"; import { getMatchLocation } from "./get-match-location.js"; import { stripCommentsPreservingPositions } from "./strip-comments-preserving-positions.js"; -import { stripStringLiteralsPreservingPositions } from "./strip-string-literals-preserving-positions.js"; +import { stripStringLiteralsKeepingModuleSpecifiers } from "./strip-string-literals-keeping-module-specifiers.js"; export interface ScanByPatternInput { readonly shouldScan: (file: ScannedFile) => boolean; @@ -12,9 +12,10 @@ export interface ScanByPatternInput { // Conjunction gates: every pattern must also match somewhere in the file // (e.g. an MCP import that proves the matched tool surface is MCP). readonly requireAll?: ReadonlyArray; - // Conjunction gates that must match in CODE only — string-literal contents - // (and comments) are blanked before testing, so a capability keyword that - // appears solely inside a `description` string or prose doesn't count. + // Conjunction gates that must match in CODE only — comments and prose + // string-literal contents are blanked before testing (module-specifier + // strings are kept, since an import path is code), so a capability keyword + // that appears solely inside a `description` string doesn't count. readonly requireAllInCode?: ReadonlyArray; // Veto: a match anywhere in the file suppresses the finding (e.g. a // signature-verification call that answers the rule's concern). @@ -37,13 +38,14 @@ export const getScannableContent = (file: ScannedFile): string => { return strippedContent; }; -// Comments AND string-literal contents blanked — the "is this keyword in real -// code?" view used by `requireAllInCode` gates so prose inside `description` -// strings can't satisfy a capability gate. +// Comments AND prose string-literal contents blanked (import paths kept) — the +// "is this keyword in real code?" view used by `requireAllInCode` gates so prose +// inside a `description` string can't satisfy a capability gate, while a +// dangerous import like `from "node:child_process"` still counts. const getCodeOnlyContent = (file: ScannedFile): string => { const cachedContent = codeOnlyContentCache.get(file); if (cachedContent !== undefined) return cachedContent; - const codeOnlyContent = stripStringLiteralsPreservingPositions(getScannableContent(file)); + const codeOnlyContent = stripStringLiteralsKeepingModuleSpecifiers(getScannableContent(file)); codeOnlyContentCache.set(file, codeOnlyContent); return codeOnlyContent; }; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.test.ts new file mode 100644 index 000000000..502c9f91d --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from "vite-plus/test"; +import { stripStringLiteralsKeepingModuleSpecifiers } from "./strip-string-literals-keeping-module-specifiers.js"; + +describe("security-scan/utils/strip-string-literals-keeping-module-specifiers", () => { + it("blanks string contents while preserving offsets, delimiters, and newlines", () => { + const source = `const description = "ALWAYS fetch the numbers first";\nconst run = exec(cmd);`; + const stripped = stripStringLiteralsKeepingModuleSpecifiers(source); + expect(stripped).toHaveLength(source.length); + expect(stripped).not.toContain("fetch"); + expect(stripped.split("\n")[0]).toMatch(/^const description = " +";$/); + expect(stripped.split("\n")[1]).toBe("const run = exec(cmd);"); + }); + + it("blanks template-literal text but keeps newlines for multi-line strings", () => { + const source = "const help = `line one\nfetch line two`;\nconst keep = true;"; + const stripped = stripStringLiteralsKeepingModuleSpecifiers(source); + expect(stripped).not.toContain("fetch"); + expect(stripped.split("\n")).toHaveLength(3); + expect(stripped.split("\n")[2]).toBe("const keep = true;"); + }); + + it("does not let an escaped quote close the string early", () => { + const source = `const note = "say \\"exec\\" out loud"; const real = spawn(cmd);`; + const stripped = stripStringLiteralsKeepingModuleSpecifiers(source); + expect(stripped).not.toContain("exec"); + expect(stripped).toContain("spawn(cmd)"); + }); + + it("keeps module-specifier strings: import, export-from, and require paths", () => { + const source = [ + `import { execFile } from "node:child_process";`, + `export { readFile } from "node:fs/promises";`, + `const vm = require("node:vm");`, + `const dynamic = import("node:child_process");`, + ].join("\n"); + const stripped = stripStringLiteralsKeepingModuleSpecifiers(source); + expect(stripped).toContain("node:child_process"); + expect(stripped).toContain("node:fs/promises"); + expect(stripped).toContain("node:vm"); + expect(stripped).toBe(source); + }); + + it("blanks member-access strings that merely look like an import", () => { + const source = `const copy = Buffer.from("fetch the bytes");`; + const stripped = stripStringLiteralsKeepingModuleSpecifiers(source); + expect(stripped).not.toContain("fetch"); + expect(stripped).toMatch(/^const copy = Buffer\.from\(" +"\);$/); + }); +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.ts new file mode 100644 index 000000000..7e98849d8 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.ts @@ -0,0 +1,68 @@ +// Builds the "is this keyword in real code?" view used by capability/keyword +// gates: string-literal *contents* are blanked with spaces so a keyword that +// appears only in prose — a tool's human-readable `description`, "ALWAYS fetch +// the numbers first" — can't satisfy a gate. Module-specifier strings +// (`from "node:child_process"`, `require("node:fs")`) are kept, because an +// import path is code, not prose, and naming a dangerous module is a real +// capability signal. Delimiters, newlines, and every offset are preserved so a +// blanked region still maps 1:1 onto the original file. Expects comment-stripped +// input so a quote inside a comment is never treated as a string delimiter. +const MODULE_SPECIFIER_KEYWORDS = new Set(["from", "import", "require"]); + +const isModuleSpecifierQuote = (content: string, quoteIndex: number): boolean => { + let cursor = quoteIndex - 1; + while (cursor >= 0 && /\s/.test(content[cursor])) cursor -= 1; + if (content[cursor] === "(") { + cursor -= 1; + while (cursor >= 0 && /\s/.test(content[cursor])) cursor -= 1; + } + const wordEnd = cursor; + while (cursor >= 0 && /[\w$]/.test(content[cursor])) cursor -= 1; + const precedingWord = content.slice(cursor + 1, wordEnd + 1); + if (!MODULE_SPECIFIER_KEYWORDS.has(precedingWord)) return false; + // A member access (`Buffer.from("…")`, `db.import("…")`) is not an import. + return content[cursor] !== "."; +}; + +export const stripStringLiteralsKeepingModuleSpecifiers = (content: string): string => { + const characters = content.split(""); + let stringDelimiter: string | null = null; + let isModuleSpecifier = false; + let index = 0; + + while (index < content.length) { + const character = content[index]; + + if (stringDelimiter !== null) { + if (character === "\\") { + if (!isModuleSpecifier) { + characters[index] = " "; + if (content[index + 1] !== undefined && content[index + 1] !== "\n") { + characters[index + 1] = " "; + } + } + index += 2; + continue; + } + if (character === stringDelimiter) { + stringDelimiter = null; + index += 1; + continue; + } + if (!isModuleSpecifier && character !== "\n") characters[index] = " "; + index += 1; + continue; + } + + if (character === '"' || character === "'" || character === "`") { + stringDelimiter = character; + isModuleSpecifier = isModuleSpecifierQuote(content, index); + index += 1; + continue; + } + + index += 1; + } + + return characters.join(""); +}; diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.test.ts deleted file mode 100644 index 7ab4c56ca..000000000 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.test.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { describe, expect, it } from "vite-plus/test"; -import { stripStringLiteralsPreservingPositions } from "./strip-string-literals-preserving-positions.js"; - -describe("security-scan/utils/strip-string-literals-preserving-positions", () => { - it("blanks string contents while preserving offsets, delimiters, and newlines", () => { - const source = `const description = "ALWAYS fetch the numbers first";\nconst run = exec(cmd);`; - const stripped = stripStringLiteralsPreservingPositions(source); - expect(stripped).toHaveLength(source.length); - expect(stripped).not.toContain("fetch"); - expect(stripped.split("\n")[0]).toMatch(/^const description = " +";$/); - expect(stripped.split("\n")[1]).toBe("const run = exec(cmd);"); - }); - - it("blanks template-literal text but keeps newlines for multi-line strings", () => { - const source = "const help = `line one\nfetch line two`;\nconst keep = true;"; - const stripped = stripStringLiteralsPreservingPositions(source); - expect(stripped).not.toContain("fetch"); - expect(stripped.split("\n")).toHaveLength(3); - expect(stripped.split("\n")[2]).toBe("const keep = true;"); - }); - - it("does not let an escaped quote close the string early", () => { - const source = `const note = "say \\"exec\\" out loud"; const real = spawn(cmd);`; - const stripped = stripStringLiteralsPreservingPositions(source); - expect(stripped).not.toContain("exec"); - expect(stripped).toContain("spawn(cmd)"); - }); -}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.ts deleted file mode 100644 index 7be9bdb26..000000000 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-preserving-positions.ts +++ /dev/null @@ -1,46 +0,0 @@ -// Pattern scans repeatedly match capability keywords that live only inside -// string literals — a tool's human-readable `description`, prose like -// "ALWAYS fetch the underlying numbers first". This blanks string-literal -// contents with spaces so a keyword gate can require a real code occurrence; -// the delimiters, newlines, and every offset are preserved so match indices, -// lines, and columns still map 1:1 onto the original file. Expects -// comment-stripped input so a quote inside a comment is never treated as a -// string delimiter. -export const stripStringLiteralsPreservingPositions = (content: string): string => { - const characters = content.split(""); - let stringDelimiter: string | null = null; - let index = 0; - - while (index < content.length) { - const character = content[index]; - - if (stringDelimiter !== null) { - if (character === "\\") { - characters[index] = " "; - if (content[index + 1] !== undefined && content[index + 1] !== "\n") { - characters[index + 1] = " "; - } - index += 2; - continue; - } - if (character === stringDelimiter) { - stringDelimiter = null; - index += 1; - continue; - } - if (character !== "\n") characters[index] = " "; - index += 1; - continue; - } - - if (character === '"' || character === "'" || character === "`") { - stringDelimiter = character; - index += 1; - continue; - } - - index += 1; - } - - return characters.join(""); -}; From 90a346960ba5cca2b64fb37d5795bc0412070fd4 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 16 Jun 2026 10:23:32 +0000 Subject: [PATCH 3/4] fix(security-scan): keep template ${...} interpolations in capability code-only view A capability call written only inside a template-literal interpolation (e.g. `${exec(cmd)}`) was blanked along with the surrounding prose, so agent-tool-capability-risk / mcp-tool-capability-risk could miss it. Make the stripper interpolation-aware: blank template text but treat ${...} as code, balancing braces and descending into nested strings/templates. Co-Authored-By: Aiden Bai --- ...t-tool-capability-risk.regressions.test.ts | 15 +++ ...literals-keeping-module-specifiers.test.ts | 15 +++ ...ring-literals-keeping-module-specifiers.ts | 107 ++++++++++++++---- 3 files changed, 117 insertions(+), 20 deletions(-) diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.regressions.test.ts index c7efb93a9..5f1e75629 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/agent-tool-capability-risk.regressions.test.ts @@ -55,6 +55,21 @@ describe("security-scan/agent-tool-capability-risk — regressions", () => { expect(findings).toHaveLength(1); }); + it("flags a tool whose handler calls a capability inside a template interpolation", () => { + const content = [ + 'import { tool } from "ai";', + "export const proxy = tool({", + ' description: "Proxy a request",', + " execute: async ({ url }) => `result: ${await fetch(url)}`,", + "});", + ].join("\n"); + const findings = runScanRule(agentToolCapabilityRisk, { + relativePath: "src/lib/tools/proxy.ts", + content, + }); + expect(findings).toHaveLength(1); + }); + it("flags a tool that names a dangerous module in its import specifier", () => { const content = [ 'import { tool } from "ai";', diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.test.ts index 502c9f91d..faec6e39a 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.test.ts @@ -46,4 +46,19 @@ describe("security-scan/utils/strip-string-literals-keeping-module-specifiers", expect(stripped).not.toContain("fetch"); expect(stripped).toMatch(/^const copy = Buffer\.from\(" +"\);$/); }); + + it("keeps template `${…}` interpolation code while blanking the surrounding text", () => { + const source = "const out = `please fetch ${exec(cmd)} right now`;"; + const stripped = stripStringLiteralsKeepingModuleSpecifiers(source); + expect(stripped).toContain("exec(cmd)"); + expect(stripped).not.toContain("please fetch"); + expect(stripped).not.toContain("right now"); + }); + + it("blanks prose strings nested inside an interpolation but keeps the call", () => { + const source = "const out = `${runCommand({ shell: \"fetch the data\" })}`;"; + const stripped = stripStringLiteralsKeepingModuleSpecifiers(source); + expect(stripped).toContain("runCommand({ shell:"); + expect(stripped).not.toContain("fetch the data"); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.ts index 7e98849d8..d39071ec2 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.ts @@ -1,14 +1,31 @@ // Builds the "is this keyword in real code?" view used by capability/keyword // gates: string-literal *contents* are blanked with spaces so a keyword that // appears only in prose — a tool's human-readable `description`, "ALWAYS fetch -// the numbers first" — can't satisfy a gate. Module-specifier strings -// (`from "node:child_process"`, `require("node:fs")`) are kept, because an -// import path is code, not prose, and naming a dangerous module is a real -// capability signal. Delimiters, newlines, and every offset are preserved so a -// blanked region still maps 1:1 onto the original file. Expects comment-stripped -// input so a quote inside a comment is never treated as a string delimiter. +// the numbers first" — can't satisfy a gate. Three things stay, because they are +// code rather than prose: module-specifier strings (`from "node:child_process"`, +// `require("node:fs")`), template-literal `${…}` interpolations (e.g. a real +// `${exec(cmd)}` call), and every delimiter/newline/offset (so a blanked region +// still maps 1:1 onto the original file). Expects comment-stripped input so a +// quote inside a comment is never treated as a string delimiter. const MODULE_SPECIFIER_KEYWORDS = new Set(["from", "import", "require"]); +interface StringFrame { + readonly kind: "string"; + readonly delimiter: string; + readonly shouldBlank: boolean; +} + +interface TemplateFrame { + readonly kind: "template"; +} + +interface InterpolationFrame { + readonly kind: "interpolation"; + braceDepth: number; +} + +type ScanFrame = StringFrame | TemplateFrame | InterpolationFrame; + const isModuleSpecifierQuote = (content: string, quoteIndex: number): boolean => { let cursor = quoteIndex - 1; while (cursor >= 0 && /\s/.test(content[cursor])) cursor -= 1; @@ -26,37 +43,87 @@ const isModuleSpecifierQuote = (content: string, quoteIndex: number): boolean => export const stripStringLiteralsKeepingModuleSpecifiers = (content: string): string => { const characters = content.split(""); - let stringDelimiter: string | null = null; - let isModuleSpecifier = false; + const frames: ScanFrame[] = []; let index = 0; + const blankCharacter = (characterIndex: number): void => { + if (content[characterIndex] !== "\n") characters[characterIndex] = " "; + }; + while (index < content.length) { const character = content[index]; + const currentFrame = frames.at(-1); - if (stringDelimiter !== null) { + if (currentFrame?.kind === "string") { if (character === "\\") { - if (!isModuleSpecifier) { - characters[index] = " "; - if (content[index + 1] !== undefined && content[index + 1] !== "\n") { - characters[index + 1] = " "; - } + if (currentFrame.shouldBlank) { + blankCharacter(index); + if (content[index + 1] !== undefined) blankCharacter(index + 1); } index += 2; continue; } - if (character === stringDelimiter) { - stringDelimiter = null; + if (character === currentFrame.delimiter) { + frames.pop(); + index += 1; + continue; + } + if (currentFrame.shouldBlank) blankCharacter(index); + index += 1; + continue; + } + + if (currentFrame?.kind === "template") { + if (character === "\\") { + blankCharacter(index); + if (content[index + 1] !== undefined) blankCharacter(index + 1); + index += 2; + continue; + } + if (character === "`") { + frames.pop(); index += 1; continue; } - if (!isModuleSpecifier && character !== "\n") characters[index] = " "; + // `${…}` is executable code, not prose, so descend into it instead of + // blanking — a capability call written only there must still count. + if (character === "$" && content[index + 1] === "{") { + frames.push({ kind: "interpolation", braceDepth: 1 }); + index += 2; + continue; + } + blankCharacter(index); index += 1; continue; } - if (character === '"' || character === "'" || character === "`") { - stringDelimiter = character; - isModuleSpecifier = isModuleSpecifierQuote(content, index); + // Top-level code or an interpolation expression: keep everything, but + // balance interpolation braces so the matching `}` returns to the template. + if (currentFrame?.kind === "interpolation") { + if (character === "{") { + currentFrame.braceDepth += 1; + index += 1; + continue; + } + if (character === "}") { + currentFrame.braceDepth -= 1; + if (currentFrame.braceDepth === 0) frames.pop(); + index += 1; + continue; + } + } + + if (character === '"' || character === "'") { + frames.push({ + kind: "string", + delimiter: character, + shouldBlank: !isModuleSpecifierQuote(content, index), + }); + index += 1; + continue; + } + if (character === "`") { + frames.push({ kind: "template" }); index += 1; continue; } From afcf1df4c11d95c2f02b9185d19fab4b82aa7c78 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 16 Jun 2026 10:29:57 +0000 Subject: [PATCH 4/4] style: format interpolation test string with prettier quote style Co-Authored-By: Aiden Bai --- .../strip-string-literals-keeping-module-specifiers.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.test.ts index faec6e39a..451f85e5a 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/strip-string-literals-keeping-module-specifiers.test.ts @@ -56,7 +56,7 @@ describe("security-scan/utils/strip-string-literals-keeping-module-specifiers", }); it("blanks prose strings nested inside an interpolation but keeps the call", () => { - const source = "const out = `${runCommand({ shell: \"fetch the data\" })}`;"; + const source = 'const out = `${runCommand({ shell: "fetch the data" })}`;'; const stripped = stripStringLiteralsKeepingModuleSpecifiers(source); expect(stripped).toContain("runCommand({ shell:"); expect(stripped).not.toContain("fetch the data");