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

.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.

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);
+  });
+});

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.",
   }),

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.",
   }),

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);
+  });
 });

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 =
-  /(?<!(?:safe|valid|sanitiz|relativ|allowlist|whitelist)[\w$]*\(\s*(?:new\s+)?)\b(?:searchParams|useSearchParams\s*\(\s*\)|URLSearchParams\s*\([^)]{0,120}\))(?:[?!])?\.get(?:All)?\s*\(\s*["'](?:userstoinvite|role|permission|sharingaction|invite|admin|next|continue|returnTo|redirect_uri|callbackUrl)["']|\bsearchParams\.(?:userstoinvite|role|permission|sharingaction|invite|admin|returnTo|redirect_uri|callbackUrl)\b/i;
+  /(?<!(?:safe|valid|sanitiz|relativ|allowlist|whitelist)[\w$]*\(\s*(?:new\s+)?(?:[\w$]+\s*\.\s*)*)\b(?:searchParams|useSearchParams\s*\(\s*\)|URLSearchParams\s*\([^)]{0,120}\))(?:[?!])?\.get(?:All)?\s*\(\s*["'](?:userstoinvite|role|permission|sharingaction|invite|admin|next|continue|returnTo|redirect_uri|callbackUrl)["']|\bsearchParams\.(?:userstoinvite|role|permission|sharingaction|invite|admin|returnTo|redirect_uri|callbackUrl)\b/i;
 
 export const urlPrefilledPrivilegedAction = defineRule({
   id: "url-prefilled-privileged-action",

packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/scan-by-pattern.ts

@@ -2,6 +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";
 
 export interface ScanByPatternInput {
   readonly shouldScan: (file: ScannedFile) => boolean;
@@ -11,13 +12,18 @@ 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<RegExp>;
+  // 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<RegExp>;
   // 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;
   readonly message: string;
 }
 
 const strippedContentCache = new WeakMap<ScannedFile, string>();
+const codeOnlyContentCache = new WeakMap<ScannedFile, string>();
 
 // 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 [];

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)");
+  });
+});

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("");
+};

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 <BlogPostPageUI blogPost={data} />;
+}`,
+      { 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);
+  });
+});

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<string> => {
   const names = new Set<string>();
   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;
 };