From 450a33269e5cef95fcd4eb99088a8a91b32610e2 Mon Sep 17 00:00:00 2001 From: kunaldhongade <74715881+kunaldhongade@users.noreply.github.com> Date: Wed, 1 Jul 2026 01:05:45 +0530 Subject: [PATCH] fix(matchers): ignore auth prose for missing-auth guards --- packages/cli/src/benchmark/corpus.ts | 58 +++++++++++++++++ packages/cli/test/benchmark-corpus.test.ts | 8 ++- packages/cli/test/benchmark.test.ts | 34 ++++++---- packages/matchers/src/defaults.ts | 37 ++++++++++- packages/matchers/src/utils.ts | 76 ++++++++++++++++++++++ packages/matchers/test/matchers.test.ts | 69 ++++++++++++++++++++ 6 files changed, 263 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/benchmark/corpus.ts b/packages/cli/src/benchmark/corpus.ts index ad5c359..45d84fd 100644 --- a/packages/cli/src/benchmark/corpus.ts +++ b/packages/cli/src/benchmark/corpus.ts @@ -80,6 +80,12 @@ export function createDefaultBenchmarkCorpus(): BenchmarkCorpus { setup: createPlainExportedDestructiveMissingAuthRepo, expectedRuleIds: ["security-missing-auth-entrypoint"] }, + { + id: "auth-comment-destructive-missing-auth", + kind: "positive", + setup: createAuthCommentDestructiveMissingAuthRepo, + expectedRuleIds: ["security-missing-auth-entrypoint"] + }, { id: "one-hop-path-join-traversal", kind: "positive", @@ -103,6 +109,12 @@ export function createDefaultBenchmarkCorpus(): BenchmarkCorpus { kind: "decoy", setup: createRequestNameCollisionDecoyRepo, expectedRuleIds: [] + }, + { + id: "guarded-destructive-auth-decoy", + kind: "decoy", + setup: createGuardedDestructiveAuthDecoyRepo, + expectedRuleIds: [] } ], cleanup: cleanupBenchmarkCorpus @@ -336,6 +348,29 @@ export function createPlainExportedDestructiveMissingAuthRepo(): string { return repo; } +export function createAuthCommentDestructiveMissingAuthRepo(): string { + const repo = createRepo({ + "src/admin.ts": "export function listUsers() { return []; }\n" + }); + + writeFile( + repo, + "src/admin.ts", + [ + "import { Pool } from 'pg';", + "const pool = new Pool();", + "// destructive admin action with NO authentication/authorization check", + "export async function deleteAllUsers(req: { body: unknown }) {", + " await pool.query('DELETE FROM users');", + " return { ok: true };", + "}", + "" + ].join("\n") + ); + + return repo; +} + export function createOneHopPathJoinTraversalRepo(): string { const repo = createRepo({ "src/api/files.ts": "export function readUpload() { return 'ok'; }\n" @@ -427,6 +462,29 @@ export function createRequestNameCollisionDecoyRepo(): string { return repo; } +export function createGuardedDestructiveAuthDecoyRepo(): string { + const repo = createRepo({ + "src/admin.ts": "export function listUsers() { return []; }\n" + }); + + writeFile( + repo, + "src/admin.ts", + [ + "import { Pool } from 'pg';", + "const pool = new Pool();", + "export async function deleteAllUsers(req: { body: unknown }) {", + " requireAuth(req);", + " await pool.query('DELETE FROM users');", + " return { ok: true };", + "}", + "" + ].join("\n") + ); + + return repo; +} + function createBranchingFunction(name: string, ifCount: number): string { return [ `export function ${name}(input) {`, diff --git a/packages/cli/test/benchmark-corpus.test.ts b/packages/cli/test/benchmark-corpus.test.ts index 8427f49..9c5019f 100644 --- a/packages/cli/test/benchmark-corpus.test.ts +++ b/packages/cli/test/benchmark-corpus.test.ts @@ -247,14 +247,16 @@ describe("unified harness planted issue corpus", () => { const corpus = createDefaultBenchmarkCorpus(); expect(corpus.rules.map((rule) => rule.ruleId)).toEqual(DEFAULT_BENCHMARK_RULES.map((rule) => rule.ruleId)); - expect(corpus.scenarios.filter((scenario) => scenario.kind === "positive")).toHaveLength(5); - expect(corpus.scenarios.filter((scenario) => scenario.kind === "decoy")).toHaveLength(3); + expect(corpus.scenarios.filter((scenario) => scenario.kind === "positive")).toHaveLength(6); + expect(corpus.scenarios.filter((scenario) => scenario.kind === "decoy")).toHaveLength(4); expect(corpus.scenarios.map((scenario) => scenario.id)).toEqual( expect.arrayContaining([ "one-hop-sqli", "plain-exported-destructive-missing-auth", + "auth-comment-destructive-missing-auth", "one-hop-path-join-traversal", - "request-name-collision-decoy" + "request-name-collision-decoy", + "guarded-destructive-auth-decoy" ]) ); }); diff --git a/packages/cli/test/benchmark.test.ts b/packages/cli/test/benchmark.test.ts index 86f4dec..d9f8ac6 100644 --- a/packages/cli/test/benchmark.test.ts +++ b/packages/cli/test/benchmark.test.ts @@ -27,27 +27,27 @@ describe("codedecay benchmark CLI contract", () => { expect(result.exitCode).toBe(0); expect(result.stderr).toBe(""); expect(report.corpus).toBe("default"); - expect(report.summary).toMatchObject({ - totalExpected: 21, - totalMatched: 21, - overallRecall: 1, - falsePositives: 2, - falsePositiveRate: 0.037, - costUsd: 0, - llmCalled: false, - telemetrySent: false - }); + expect(report.summary).toMatchObject({ + totalExpected: 22, + totalMatched: 22, + overallRecall: 1, + falsePositives: 2, + falsePositiveRate: 0.0278, + costUsd: 0, + llmCalled: false, + telemetrySent: false + }); expect(report.summary.falsePositiveRate).toBeLessThan(0.1); expect(report.summary.durationMs).toBeGreaterThanOrEqual(0); expect(report.metrics.byArea).toEqual([ - expect.objectContaining({ area: "security", expected: 11, matched: 11, recall: 1, falsePositives: 0 }), + expect.objectContaining({ area: "security", expected: 12, matched: 12, recall: 1, falsePositives: 0 }), expect.objectContaining({ area: "regression", expected: 5, matched: 5, recall: 1, falsePositives: 2 }), expect.objectContaining({ area: "quality", expected: 5, matched: 5, recall: 1, falsePositives: 0 }) ]); expect(report.metrics.byRuleId).toEqual( expect.arrayContaining([ expect.objectContaining({ ruleId: "security-sql-injection", expected: 2, matched: 2 }), - expect.objectContaining({ ruleId: "security-missing-auth-entrypoint", expected: 2, matched: 2 }), + expect.objectContaining({ ruleId: "security-missing-auth-entrypoint", expected: 3, matched: 3 }), expect.objectContaining({ ruleId: "security-path-traversal", expected: 2, matched: 2 }), expect.objectContaining({ ruleId: "happy-path-only-test", expected: 1, matched: 1 }), expect.objectContaining({ ruleId: "missing-nearby-tests", expected: 1, matched: 1 }) @@ -63,6 +63,10 @@ describe("codedecay benchmark CLI contract", () => { id: "plain-exported-destructive-missing-auth", matchedRuleIds: ["security-missing-auth-entrypoint"] }), + expect.objectContaining({ + id: "auth-comment-destructive-missing-auth", + matchedRuleIds: ["security-missing-auth-entrypoint"] + }), expect.objectContaining({ id: "one-hop-path-join-traversal", matchedRuleIds: ["security-path-traversal"] @@ -70,6 +74,10 @@ describe("codedecay benchmark CLI contract", () => { expect.objectContaining({ id: "request-name-collision-decoy", falsePositiveRuleIds: [] + }), + expect.objectContaining({ + id: "guarded-destructive-auth-decoy", + falsePositiveRuleIds: [] }) ]) ); @@ -86,7 +94,7 @@ describe("codedecay benchmark CLI contract", () => { expect(result.stderr).toBe(""); expect(rendered).toContain("## CodeDecay Benchmark"); expect(rendered).toContain("| Overall recall | 100% |"); - expect(rendered).toContain("| False-positive rate | 3.7% |"); + expect(rendered).toContain("| False-positive rate | 2.78% |"); expect(rendered).toContain("- LLM/model called: no"); expect(rendered).toContain("- Telemetry sent: no"); }); diff --git a/packages/matchers/src/defaults.ts b/packages/matchers/src/defaults.ts index cb96423..e4c4448 100644 --- a/packages/matchers/src/defaults.ts +++ b/packages/matchers/src/defaults.ts @@ -8,7 +8,8 @@ import { hasTemplateUserInputExpression, hasUserInputMarker, lineMatches, - maskStringLiterals + maskStringLiterals, + stripComments } from "./utils"; const AUTH_MARKERS = ["auth", "session", "jwt", "token", "currentuser", "current_user", "requireuser", "requireauth", "isallowed"]; @@ -254,8 +255,7 @@ export const missingAuthEntryPointMatcher: SecurityMatcher = { } ], match(context) { - const lowerContent = context.content.toLowerCase(); - if (!hasRouteEntryPoint(context.filePath, context.content) || containsAny(lowerContent, AUTH_MARKERS)) { + if (!hasRouteEntryPoint(context.filePath, context.content) || hasAuthGuard(context.content)) { return []; } @@ -385,6 +385,37 @@ function hasCredentialAssignment(codeLine: string): boolean { }); } +function hasAuthGuard(content: string): boolean { + const codeOnly = stripComments(content) + .split(/\n/) + .map((line) => maskStringLiterals(line).toLowerCase()) + .join("\n"); + + return AUTH_MARKERS.some((marker) => hasAuthMarker(codeOnly, marker)); +} + +function hasAuthMarker(code: string, marker: string): boolean { + const escapedMarker = escapeRegExp(marker.toLowerCase()); + + if (new RegExp(`\\b${escapedMarker}\\b\\s*(?:\\(|\\.|\\[|\\?)`).test(code)) { + return true; + } + + if (new RegExp(`\\b(?:assert|check|ensure|get|require|validate|verify)${escapedMarker}\\b\\s*\\(`).test(code)) { + return true; + } + + if (["currentuser", "current_user", "isallowed"].includes(marker.toLowerCase())) { + return new RegExp(`\\b${escapedMarker}\\b`).test(code); + } + + return false; +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + function uniqueMatches(matches: Array<{ line: number; text: string }>): Array<{ line: number; text: string }> { const byKey = new Map(); for (const match of matches) { diff --git a/packages/matchers/src/utils.ts b/packages/matchers/src/utils.ts index 2bf6ec1..0fd6b44 100644 --- a/packages/matchers/src/utils.ts +++ b/packages/matchers/src/utils.ts @@ -216,6 +216,82 @@ export function maskStringLiterals(line: string): string { return output; } +export function stripComments(content: string): string { + let output = ""; + let quote: "'" | "\"" | "`" | undefined; + let escaped = false; + let inLineComment = false; + let inBlockComment = false; + + for (let index = 0; index < content.length; index += 1) { + const char = content[index] ?? ""; + const next = content[index + 1] ?? ""; + + if (inLineComment) { + if (char === "\n") { + inLineComment = false; + output += "\n"; + } else { + output += " "; + } + continue; + } + + if (inBlockComment) { + if (char === "*" && next === "/") { + inBlockComment = false; + output += " "; + index += 1; + } else { + output += char === "\n" ? "\n" : " "; + } + continue; + } + + if (quote !== undefined) { + output += char; + if (escaped) { + escaped = false; + continue; + } + + if (char === "\\") { + escaped = true; + continue; + } + + if (char === quote) { + quote = undefined; + } + continue; + } + + if (char === "'" || char === "\"" || char === "`") { + quote = char; + output += char; + continue; + } + + if (char === "/" && next === "/") { + inLineComment = true; + output += " "; + index += 1; + continue; + } + + if (char === "/" && next === "*") { + inBlockComment = true; + output += " "; + index += 1; + continue; + } + + output += char; + } + + return output; +} + export function hasRouteEntryPoint(filePath: string, content: string): boolean { const normalized = filePath.toLowerCase(); const lowerContent = content.toLowerCase(); diff --git a/packages/matchers/test/matchers.test.ts b/packages/matchers/test/matchers.test.ts index c7f7547..967414b 100644 --- a/packages/matchers/test/matchers.test.ts +++ b/packages/matchers/test/matchers.test.ts @@ -223,6 +223,75 @@ describe("scanSecurityCandidates", () => { expect(result.candidates.map((candidate) => candidate.ruleId)).toContain("security-missing-auth-entrypoint"); }); + it("flags destructive exported functions when auth is mentioned only in comments", () => { + const result = scanSecurityCandidates({ + files: [ + { + path: "src/admin.ts", + content: [ + "import { Pool } from \"pg\";", + "const pool = new Pool();", + "// destructive admin action with NO authentication/authorization check", + "export async function deleteAllUsers(req: { body: unknown }) {", + " await pool.query(\"DELETE FROM users\");", + " return { ok: true };", + "}", + "" + ].join("\n") + } + ] + }); + const missingAuth = result.candidates.filter((candidate) => candidate.ruleId === "security-missing-auth-entrypoint"); + + expect(missingAuth).toHaveLength(1); + expect(missingAuth[0]).toMatchObject({ + file: "src/admin.ts", + severity: "high" + }); + }); + + it("does not flag destructive exported functions with a real auth guard", () => { + const result = scanSecurityCandidates({ + files: [ + { + path: "src/admin.ts", + content: [ + "import { Pool } from \"pg\";", + "const pool = new Pool();", + "export async function deleteAllUsers(req: { body: unknown }) {", + " requireAuth(req);", + " await pool.query(\"DELETE FROM users\");", + " return { ok: true };", + "}", + "" + ].join("\n") + } + ] + }); + + expect(result.candidates.map((candidate) => candidate.ruleId)).not.toContain("security-missing-auth-entrypoint"); + }); + + it("does not treat author or authentication prose as missing-auth evidence in benign files", () => { + const result = scanSecurityCandidates({ + files: [ + { + path: "src/profile.ts", + content: [ + "// This authentication note documents the author profile page.", + "export function authorBio(author: { name: string }) {", + " const authorLabel = `Author: ${author.name}`;", + " return authorLabel;", + "}", + "" + ].join("\n") + } + ] + }); + + expect(result.candidates.map((candidate) => candidate.ruleId)).not.toContain("security-missing-auth-entrypoint"); + }); + it("detects JWT decode-without-verify and unsafe verification options while avoiding safe decoys", () => { const risky = scanSecurityCandidates({ files: [