diff --git a/CHANGELOG.md b/CHANGELOG.md index 21e2e1e..3dfb867 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ CodeTruss CLI follows semantic versioning. Release artifacts and their SHA-256 checksums are published at . -The current public release is [v0.2.46 on GitHub](https://github.com/CodeTruss/codetruss-cli/releases/tag/v0.2.46), +The current public release is [v0.2.50 on GitHub](https://github.com/CodeTruss/codetruss-cli/releases/tag/v0.2.50), distributed from . The npm `latest` tag is still [`@codetruss/cli@0.2.41`](https://www.npmjs.com/package/@codetruss/cli/v/0.2.41): @@ -16,6 +16,210 @@ were superseded before distribution. No unreleased changes. +## 0.2.50 — 2026-08-08 + +- **`dead-code` spent 26 of this analysis's 27 seconds and bought nothing with + them.** The pass concatenated every indexed JS/TS file into a single string — + 18.2 MB on `calcom/cal.com` — and then ran one regular expression per + candidate module against the whole of it. That is O(candidates × corpus + bytes): 1,500 sweeps of an 18 MB string, roughly 64 GB of scanning, to decide + 1,500 yes/no questions. And it bought nothing, because the pass has been + saturated at its 20-finding output cap the entire time — more scanning changed + only *which* candidates were examined, never how many findings came out. It + now reads the corpus twice, building one index of the filename stems the + repository actually references, and answers each candidate with a set lookup. + On `calcom/cal.com@b2c28a23` (7,691 files, 517,420 LOC) `dead-code` drops from + 25.8s to 0.03s and the whole deterministic analyzer phase from 27.3s to 1.5s. + + **The findings are unchanged, and that was checked rather than assumed.** The + old expression matched a filename stem wherever it appeared — inside strings, + inside comments, inside unrelated tokens — and a tidier index that quietly + stopped doing so would start reporting live modules as dead. So the + replacement reproduces that looseness exactly. On four pinned repositories + (cal.com, astro, TanStack/query, hono) the reported findings, the findings + withheld behind the cap, the completeness flags and the pass metrics are + byte-identical before and after, and a differential run against the original + expression agrees on all 60,600 generated cases. A stem containing whitespace + or a quote character can still straddle the delimiters the index keys on, so + those candidates keep the original whole-corpus test; no real filename needs + it, and the fallback exists so the rewrite cannot narrow the rule by accident. + +## 0.2.49 — 2026-08-08 + +- **A redirect to a path this codebase wrote itself was reported as an open + redirect, and on one real repository seventeen of nineteen such reports were + false.** 0.2.45 widened the open-redirect sink to JSX `href`/`action`, + `location.assign|replace|href` and `router.push|replace`, and that widening + was narrowed against this repository alone. An open redirect requires the + attacker to control the ORIGIN — the host, or the leading `//` or scheme that + decides it — and two shapes cannot reach it. `new URL(reference, base)` + resolves by WHATWG rules, so any reference that is not itself absolute takes + the base's origin and the origin question collapses onto the first argument; + the head-position analysis that already answers exactly that question for a + template or a concatenation now re-asks itself there instead of treating the + construction as opaque. `new URLSearchParams({ … })` is a query-string + builder whose serialization is `application/x-www-form-urlencoded`, so `/`, + `:`, `<`, `&`, `=`, `?` and `#` all come back percent-encoded and a value put + in this way can only ever emerge as an encoded query value. On `calcom/cal.com` + the open-redirect count falls from 19 to 12, on `vercel/commerce` from 1 to 0, + on `shadcn-ui/taxonomy` from 1 to 0, and six further SaaS repositories produce + byte-identical findings before and after. + + What still fires, deliberately: `new URL(req.query.next, req.url)`, because a + bare reference may be absolute and override the base; a protocol-relative + reference, which discards the base origin; `new URL(x)` with one argument, + where `x` is the whole URL; and `new URLSearchParams(location.search)`, which + PARSES rather than builds, so reading a value back out of it keeps its taint. + Writing a tainted value into a query string is safe; reading one out and using + it as the target is the vulnerability, and that direction is unchanged. + + Recall cost, stated plainly: a redirect whose reference begins with a literal + `/` is no longer reported however tainted the rest of it is, which takes + `new URL('/fixed', attackerControlledBase)` host injection with it — the same + trade-off `evalOrigins` already documented for the constant case. Neither + suppression can be reached through a local variable, so + ``const u = `/x/${t}`; res.redirect(u)`` is still reported. + +## 0.2.48 — 2026-08-07 + +- **Ten bounded passes said "truncated" without ever learning what they were + truncated out of, so the largest repositories got no score at all.** A + required pass that stopped early is not authoritative, and the only way back + is to show it still covered enough of its input. An output cap could never + show that: it stops at N and never counts past N, so no denominator exists and + the gate has to assume the worst. On a 7,690-file, 517k-LOC repository ten + required passes tripped a cap — acquisition, the symbol graph, the knowledge + graph, and the duplication, secrets, dead-code, complexity, vulnerabilities, + comment-slop and speculative-structure analyzers — and all five score axes + were withheld. Nobody decided that; it fell out of the arithmetic, and it got + more certain the bigger the repository was. + + Every one of those caps now reports what it cost, in the unit it bounds. + Candidate-file bounds (duplication, dead-code, complexity, comment-slop, + speculative structure) divide the files they examined by the files the filter + produced — both numbers already existed, one slice apart. The secret scan and + the TODO scan keep counting after they stop collecting, so they report matches + shown over matches found. The vulnerability pass reports manifests read over + manifests present, and package versions checked over package versions + declared, whichever is worse. Acquisition divides the archive entries it wrote + by the entries it saw. The symbol graph reports files parsed over candidate + files and call sites kept over call sites found; the knowledge graph counts + the distinct nodes and edges its caps refuse, so those have denominators too. + + What this is not: a way to publish a score that should be withheld. The + threshold is unchanged at 95%, no pass gained a path around it, and a cap that + still cannot name its denominator still claims nothing and still voids the + score. The change is that a pass which CAN name it now does, so the existing + gate can tell one oversized archive entry in 7,690 apart from an analyzer that + lost half its findings. A repository whose secret scan reported 500 of 5,000 + matches is withheld exactly as it was before — and now says so in those words. + +- **`vulnerabilities` withheld every score with a reason that said nothing had + been lost.** The pass truncated on the dependency-manifest bound and then + reported package-version coverage, which was complete: "covered 181 of 181 + package versions", printed as the justification for publishing no score at + all. Two independent bounds share that pass, and only one of them was ever + described. Each bound that bites is now named in its own unit — "read 5 of 92 + dependency manifests", "checked 200 of 400 declared package versions" — and + the coverage claimed is the worst of them. + +- The secret scan's reported coverage is the fraction of MATCHES shown rather + than the fraction of files read, superseding the file-fraction measure added + in 0.2.47. Both answer "how much did the cap cost", but only the match count + answers it in findings: the file measure reads 100% whenever the cap is + reached on the last file scanned, and 2% when it is reached on the first, + neither of which is the number of credentials a reader cannot see. The sweep + no longer stops at the cap, which costs one more pass over a tree the scanner + already sweeps in full whenever the cap is not reached. + +- The TODO pass counted markers only up to its 500-marker retention bound, so + its headline finding — a count — reported the bound instead of the codebase on + any repository above it. The count is now exact; the bound governs only which + markers get individual findings. + +## 0.2.47 — 2026-08-07 + +- **A loop variable named `event` was treated as an HTTP request.** The taint + engine decided an expression was untrusted input by looking at the NAME of the + root identifier: `event`, `args`, `context`, `params` and `ctx` were on the + request-root list alongside `req` and the PHP superglobals, and nothing + checked that a parameter had actually bound the name. In a 517k-LOC codebase + we scanned, `for (const event of salesforceEvents)` made every + `event.` a source, and the three CRITICAL SQL-injection reports that + came out of that one loop were 39% of the repository's entire security + deduction. The rows were already inside the database they were said to be + injecting into. Those five ambiguous roots now count as a request only when + the enclosing function's signature bound them, positionally or by + destructuring (`({ ctx, input })` is how most handlers are written, so the + gate sees through patterns). The unambiguous roots are untouched: nobody names + a loop variable `req` or `$_GET`. + + The narrowing is real and we are not hiding it. A handler that lands its + request in a LOCAL named `event` — `const event = JSON.parse(body)` — or a + serverless adapter that binds it through a module-level variable now loses the + source, and every finding that depended on it. The one shape that mattered in + practice, `const params = useSearchParams()`, is covered again by treating + `useSearchParams()` and `useParams()` as sources in their own right: the call + is evidence, the variable's name never was. + +- **Placeholder detection could bless a real key, and did it silently.** Two + compounding defects. The words that mark a value as fake were matched anywhere + inside it, so `xxx` occurring by chance inside a random credential silenced + the line — measured against 400 real `generateKeyPairSync` RSA-2048 keys, 18 + of them (4.5%) contain a matching run. And the check ran against the whole + regex match rather than the value, so `examplePassword = ""` was + dismissed because of the identifier next to it. Every alternative is now + anchored to a token boundary, the pre-existing ones included, and the subject + is the extracted quoted value. Private key blocks are excluded from + value-level matching entirely — a base64 PEM body is a high-entropy blob, not + a string that can announce itself as fake — so the 4.5% collision cannot reach + the "No action needed." path by another route. The same 400 keys now produce + zero matches. `mock`, `stub`, `test-key`/`test-secret` and `not-a-real` join + the list as genuine placeholder forms. + + Consequence worth knowing before you upgrade: a credential-shaped string that + merely CONTAINS a word like `fake` inside its random body is no longer + downgraded to INFO. `sk_live_51QxR8fake2eKjL9…` is indistinguishable from a + live key by any rule that does not simply trust a substring, so it is reported + rather than blessed. Announce a placeholder with delimiters + (`sk_live_test-key-…`, `not-a-real-password`) and it is recognised as before. + +- **A placeholder declared one line up was invisible.** The placeholder check + read a single line, so a Swagger `@ApiProperty({ example: { … } })` two lines + above a documentation sample did not cover it and the sample was reported as a + committed leak. A placeholder marker that opens a block now covers that block, + which ends at the first non-blank line indented no further than the opener. + +- **Findings were double-charged, and repeats were charged in full.** Scoring + summed severity weights flat. Eight firings of one rule against one mock + string in one spec file cost eight findings — 14% of one repository's security + deduction for a single review decision — and a hard-coded credential found by + both the secrets analyzer and the SAST rule was charged twice for one line. + Repeat hits of the same rule in the same file now decay to `1 + ln(n)`, with + the worst finding in the group still charged in full, and `(filePath, line)` + is deduped across analyzers. The finding LIST is unchanged — both entries + still appear, with their own evidence and their own fix; only the arithmetic + collapses them. + + The cost lands on concentration: a file with twelve DISTINCT injection sinks + now prices close to a file with one, and two genuinely different defects on + one line price as one. Concentration is legitimate signal and most of it is + lost from the score. It remains visible in the findings. + +- **The secret scan stopped at 50 matches and took every score down with it.** + 50 was reached exactly on a 517k-LOC repository, which made every count anyone + quoted a truncated prefix — and because a truncated required pass is not + authoritative, a benign cap withheld all five score axes. The cap is now 500, + and the pass reports the fraction of its eligible files it actually read, so + an immaterial cap no longer voids the scores while a real coverage loss still + does. + +- Known, unchanged: the `Private key block` pattern matches only the PEM header, + so anything that inspects the matched text sees `-----BEGIN PRIVATE KEY-----` + and never the body. That is why the exclusion above is written by credential + type rather than by trusting the value check to hold over 1,700 random + characters. + ## 0.2.46 — 2026-08-07 - **A repository's own scope globs could crash the review that reads them.** diff --git a/packages/analyzer-engine/src/comment-slop.ts b/packages/analyzer-engine/src/comment-slop.ts index ad28b2e..2089f2b 100644 --- a/packages/analyzer-engine/src/comment-slop.ts +++ b/packages/analyzer-engine/src/comment-slop.ts @@ -1,6 +1,7 @@ import { annotatedAnalyzerOutput, incompleteAnalyzerOutput, + measuredCoverage, type Analyzer, type AnalyzerFinding, } from './types' @@ -430,7 +431,10 @@ export const commentSlopAnalyzer: Analyzer = { if (candidates.length > candidateLimit) { return incompleteAnalyzerOutput(findings, { truncated: true, - detail: `Comment analysis hit a candidate bound (${candidates.length} candidate files).`, + // The filter ran over the whole tree before the slice, so the files the + // bound cut are counted, not merely unknown. + coverageRatio: measuredCoverage(candidateLimit, candidates.length), + detail: `Comment analysis measured ${candidateLimit} of ${candidates.length} candidate files.`, metrics: { ...metrics, candidates: candidates.length, candidateLimit }, }, withheld) } diff --git a/packages/analyzer-engine/src/complexity.ts b/packages/analyzer-engine/src/complexity.ts index f919fe3..9a33f5a 100644 --- a/packages/analyzer-engine/src/complexity.ts +++ b/packages/analyzer-engine/src/complexity.ts @@ -1,6 +1,7 @@ import { annotatedAnalyzerOutput, incompleteAnalyzerOutput, + measuredCoverage, type Analyzer, type AnalyzerFinding, } from './types' @@ -164,7 +165,10 @@ export const complexityAnalyzer: Analyzer = { if (candidates.length > candidateLimit) { return incompleteAnalyzerOutput(output, { truncated: true, - detail: `Complexity analysis hit a candidate bound (${candidates.length} candidate files, ${findings.length} matches).`, + // The filter ran over the whole tree before the slice, so the files the + // bound cut are counted, not merely unknown. + coverageRatio: measuredCoverage(candidateLimit, candidates.length), + detail: `Complexity analysis measured ${candidateLimit} of ${candidates.length} candidate files (${findings.length} matches).`, metrics: { candidates: candidates.length, candidateLimit, matches: findings.length, findingLimit }, }, withheld) } diff --git a/packages/analyzer-engine/src/dead-code.ts b/packages/analyzer-engine/src/dead-code.ts index fc3482d..0ac314b 100644 --- a/packages/analyzer-engine/src/dead-code.ts +++ b/packages/analyzer-engine/src/dead-code.ts @@ -1,6 +1,7 @@ import { annotatedAnalyzerOutput, incompleteAnalyzerOutput, + measuredCoverage, type Analyzer, type AnalyzerFinding, } from './types' @@ -17,6 +18,101 @@ import { */ export const CONVENTION_FILENAME = /(page|layout|route|loading|error|not-found|template|default|middleware|proxy|instrumentation|opengraph-image|twitter-image|icon|apple-icon|sitemap|robots|manifest|index|main|app|server|config|next-env|globals)\.[jt]sx?$|^instrumentation[-.][a-z]+\.[jt]sx?$|\.(stories|story)\.[jt]sx?$|\.(d|config|test|spec)\.[cm]?[jt]s$/ +/** + * The characters that end a filename stem in every reference shape the needle + * below recognises: the three quote styles it looks between, plus whitespace + * and the path separator. + */ +const STEM_BOUNDARY = /[\s/'"`]/ + +/** Module extensions the needle accepts after a stem. Mirrors `\.[cm]?[jt]sx?`. */ +const REFERENCE_EXTENSION = /\.[cm]?[jt]sx?(?=[\s'"`])/g + +/** `"…/stem"` — an extensionless import specifier closing on its quote. */ +const QUOTED_PATH_TAIL = /\/([^\s/'"`]+)['"`]/g + +function isQuote(char: string | undefined): boolean { + return char === '"' || char === "'" || char === '`' +} + +/** + * Imported anywhere? look for `/stem'`, `/stem"`, or `from './stem`-style + * refs — or a bare quoted `stem.ext` (a spawn-by-string worker path built + * from segments, e.g. join(cwd, 'src', 'lib', 'batch-process.ts')). + * The final alternative catches a path embedded in a longer command + * string — `"sync-stripe": "node --env-file .env sync-stripe.js"` — where + * the quote does not directly abut the path. + */ +function referenceNeedle(stem: string): RegExp { + return new RegExp( + `['"\`](?:[^'"\`]*/${escapeRegExp(stem)}(\\.[cm]?[jt]sx?)?|${escapeRegExp(stem)}\\.[cm]?[jt]sx?)['"\`]` + + `|[\\s/]${escapeRegExp(stem)}\\.[cm]?[jt]sx?(?=[\\s'"\`])`, + ) +} + +/** + * Which of `stems` the concatenated corpus refers to, decided in TWO passes over + * the corpus instead of one whole-corpus regex per candidate. + * + * Running `referenceNeedle` per candidate is O(candidates x corpus bytes): on a + * 500k-LOC monorepo that is 1,500 sweeps of an 18 MB string, roughly 64 GB of + * scanning, and it dominated the entire analysis. The membership question + * answered here is deliberately the SAME one — matches inside strings, comments + * and unrelated tokens included. This is a cost fix, not a precision change; a + * stricter index would silently change which modules are called dead. + * + * The needle's three branches, rewritten as (character before stem, stem, + * character after the stem's extension): + * + * A. `['"`][^'"`]*\/stem['"`]` — `/` before, quote after, NO extension + * B. `['"`]stem.ext['"`]` — quote before, quote after + * C. `[\s/]stem.ext(?=[\s'"`])` — whitespace or `/` before, whitespace + * or quote after + * + * B and C are found together by anchoring on the extension (rare in source text) + * and reading backwards to the delimiter that opens the stem; the extension form + * of A — `['"`][^'"`]*\/stem.ext['"`]` — is a strict subset of C, because C + * already admits a `/` before and a quote after and asks for no opening quote. + * A's extensionless form has no extension to anchor on, so it gets its own scan. + * + * Correct only for stems that contain no boundary character, which is why the + * caller keeps the original needle for the ones that do. + */ +function referencedStems(corpus: string, stems: ReadonlySet): Set { + const referenced = new Set() + if (stems.size === 0) return referenced + const note = (stem: string) => { + if (stems.has(stem)) referenced.add(stem) + } + + // B and C. A stem with no boundary character is exactly the run of characters + // between the extension and the delimiter that precedes it. + for (const match of corpus.matchAll(REFERENCE_EXTENSION)) { + const dot = match.index + let start = dot + while (start > 0 && !STEM_BOUNDARY.test(corpus[start - 1])) start -= 1 + // No opening delimiter at all, or nothing between it and the extension: + // every branch needs one character of each. + if (start === 0 || start === dot) continue + // A quoted `"stem.ext"` (B) must close on its quote. The unquoted form (C) + // may close on whitespace but can only OPEN on whitespace or `/`, so a + // stem that opens on a quote and closes on whitespace matches neither. + if (isQuote(corpus[start - 1]) && !isQuote(corpus[dot + match[0].length])) continue + note(corpus.slice(start, dot)) + } + + // A, extensionless. `[^'"`]*` spans anything quote-free, so the opening quote + // is satisfied by the nearest quote before the slash — that is, by any quote + // occurring earlier in the corpus at all. + const firstQuote = corpus.search(/['"`]/) + if (firstQuote !== -1) { + for (const match of corpus.matchAll(QUOTED_PATH_TAIL)) { + if (match.index > firstQuote) note(match[1]) + } + } + return referenced +} + /** * Dead-code candidates: JS/TS modules that are never imported anywhere. * Heuristic (static string matching), so results are labeled candidates. @@ -61,6 +157,10 @@ export const deadCodeAnalyzer: Analyzer = { generatedDirs.some((dir) => (dir === '' ? !path.includes('/') : path.startsWith(dir))) const candidateLimit = 1500 + // Select every candidate before testing any of them, so the reference index + // is built once for exactly the stems that will be asked about. The order is + // the order of `jsFiles`, which is the order the findings keep below. + const candidates: Array<{ path: string; stem: string }> = [] for (const file of jsFiles.slice(0, candidateLimit)) { if (nearGeneratedOutput(file.path)) continue // CLI/tooling entry points are invoked by runners, not imports @@ -74,28 +174,32 @@ export const deadCodeAnalyzer: Analyzer = { if (base.startsWith('.')) continue const stem = base.replace(/\.[cm]?[jt]sx?$/, '') if (stem.length < 3) continue // too ambiguous to match safely - // Imported anywhere? look for `/stem'`, `/stem"`, or `from './stem`-style - // refs — or a bare quoted `stem.ext` (a spawn-by-string worker path built - // from segments, e.g. join(cwd, 'src', 'lib', 'batch-process.ts')). - // The final alternative catches a path embedded in a longer command - // string — `"sync-stripe": "node --env-file .env sync-stripe.js"` — where - // the quote does not directly abut the path. - const needle = new RegExp( - `['"\`](?:[^'"\`]*/${escapeRegExp(stem)}(\\.[cm]?[jt]sx?)?|${escapeRegExp(stem)}\\.[cm]?[jt]sx?)['"\`]` - + `|[\\s/]${escapeRegExp(stem)}\\.[cm]?[jt]sx?(?=[\\s'"\`])`, - ) - if (!needle.test(allContent)) { - findings.push({ - category: 'DEAD_CODE', - severity: 'LOW', - title: `Possibly unused module: ${file.path}`, - description: `No other file appears to import "${stem}". If it is not loaded by convention or tooling, it is dead code.`, - filePath: file.path, - suggestion: 'Verify with your bundler or `knip`/`ts-prune`, then delete if truly unused.', - impactScore: 35, - effort: 'low', - }) - } + candidates.push({ path: file.path, stem }) + } + + // A stem holding whitespace or a quote can straddle the delimiters the index + // keys on, so those candidates keep the original whole-corpus needle. No + // real filename does this; the fallback exists so the rewrite cannot narrow + // the rule by accident on one that does. + const indexable = new Set( + candidates.map((c) => c.stem).filter((stem) => !STEM_BOUNDARY.test(stem)), + ) + const referenced = referencedStems(allContent, indexable) + const isReferenced = (stem: string) => + indexable.has(stem) ? referenced.has(stem) : referenceNeedle(stem).test(allContent) + + for (const { path, stem } of candidates) { + if (isReferenced(stem)) continue + findings.push({ + category: 'DEAD_CODE', + severity: 'LOW', + title: `Possibly unused module: ${path}`, + description: `No other file appears to import "${stem}". If it is not loaded by convention or tooling, it is dead code.`, + filePath: path, + suggestion: 'Verify with your bundler or `knip`/`ts-prune`, then delete if truly unused.', + impactScore: 35, + effort: 'low', + }) } const findingLimit = 20 // Only the candidate-file cap is real coverage loss; the finding cap just @@ -109,7 +213,10 @@ export const deadCodeAnalyzer: Analyzer = { if (truncated) { return incompleteAnalyzerOutput(output, { truncated: true, - detail: `Dead-code analysis hit a bound (${jsFiles.length} candidate files, ${findings.length} matches).`, + // The candidate list was built from the whole tree before the slice, so + // the files the bound cut are counted, not merely unknown. + coverageRatio: measuredCoverage(candidateLimit, jsFiles.length), + detail: `Dead-code analysis examined ${candidateLimit} of ${jsFiles.length} candidate files (${findings.length} matches).`, metrics: { candidates: jsFiles.length, candidateLimit, matches: findings.length, findingLimit }, }, withheld) } diff --git a/packages/analyzer-engine/src/duplication.ts b/packages/analyzer-engine/src/duplication.ts index 6804e56..c9ce2ed 100644 --- a/packages/analyzer-engine/src/duplication.ts +++ b/packages/analyzer-engine/src/duplication.ts @@ -2,6 +2,7 @@ import { createHash } from 'crypto' import { annotatedAnalyzerOutput, incompleteAnalyzerOutput, + measuredCoverage, type Analyzer, type AnalyzerFinding, } from './types' @@ -78,7 +79,10 @@ export const duplicationAnalyzer: Analyzer = { if (candidates.length > candidateLimit) { return incompleteAnalyzerOutput(output, { truncated: true, - detail: `Duplication analysis hit a candidate bound (${candidates.length} candidate files, ${findings.length} matches).`, + // The filter ran over the whole tree before the slice, so the files the + // bound cut are counted, not merely unknown. + coverageRatio: measuredCoverage(candidateLimit, candidates.length), + detail: `Duplication analysis compared ${candidateLimit} of ${candidates.length} candidate files (${findings.length} matches).`, metrics: { candidates: candidates.length, candidateLimit, matches: findings.length, findingLimit }, }, withheld) } diff --git a/packages/analyzer-engine/src/overengineering.ts b/packages/analyzer-engine/src/overengineering.ts index 42713da..c6c8371 100644 --- a/packages/analyzer-engine/src/overengineering.ts +++ b/packages/analyzer-engine/src/overengineering.ts @@ -1,6 +1,7 @@ import { annotatedAnalyzerOutput, incompleteAnalyzerOutput, + measuredCoverage, type Analyzer, type AnalyzerFinding, type RepoIndex, @@ -313,7 +314,10 @@ export const overengineeringAnalyzer: Analyzer = { if (production.length > candidateLimit) { return incompleteAnalyzerOutput(findings, { truncated: true, - detail: `Speculative-structure analysis hit a candidate bound (${production.length} candidate files).`, + // The production-file list was built from the whole tree before the + // slice, so the files the bound cut are counted, not merely unknown. + coverageRatio: measuredCoverage(candidateLimit, production.length), + detail: `Speculative-structure analysis examined ${candidateLimit} of ${production.length} candidate files.`, metrics, }, withheld) } diff --git a/packages/analyzer-engine/src/scoring.ts b/packages/analyzer-engine/src/scoring.ts index eb4f186..57d2c15 100644 Binary files a/packages/analyzer-engine/src/scoring.ts and b/packages/analyzer-engine/src/scoring.ts differ diff --git a/packages/analyzer-engine/src/secrets.ts b/packages/analyzer-engine/src/secrets.ts index d70ae0f..b1986ca 100644 --- a/packages/analyzer-engine/src/secrets.ts +++ b/packages/analyzer-engine/src/secrets.ts @@ -1,5 +1,5 @@ import { moveSecretToEnvFix, untrackEnvFileFix } from './fixes' -import { incompleteAnalyzerOutput, type Analyzer, type AnalyzerFinding } from './types' +import { incompleteAnalyzerOutput, measuredCoverage, type Analyzer, type AnalyzerFinding } from './types' /** * Defensive secret-exposure detection: flags credentials that appear to be @@ -46,7 +46,36 @@ export function redactSecrets(text: string): string { } const SKIP_FILES = /(\.env\.example|\.md|\.lock|package-lock\.json|pnpm-lock\.yaml)$/i -const PLACEHOLDER = /(example|placeholder|your[-_]|xxx|changeme|dummy|<[^>]+>|\$\{)/i +/** + * A line that announces itself as documentation or a template. Matching here + * skips the line in silence. + * + * The word markers are TOKEN-ANCHORED. Unanchored they matched inside any + * alphanumeric run, and `xxx` is three characters: a random 2048-bit credential + * contains one about 4% of the time, so one line in twenty-five carrying a real + * key was dropped without a finding of any kind. The structural markers + * (``, `${VAR}`) are punctuation-delimited already and stay as + * they are, and `your-`/`your_` anchors on its left only because the token + * continues past it (`your_api_key`). + */ +const PLACEHOLDER = new RegExp( + [ + '<[^>]+>', + '\\$\\{', + '(?"` — so judging it let an identifier + * bless the credential sitting beside it. Judge the quoted literal when the + * pattern captured one; patterns that match a bare credential token (an AWS key + * in YAML) carry no quotes and are judged whole. */ -const FAKE_LITERAL_VALUE = - /(example|sample|dummy|fake|placeholder|changeme|your[-_]?(key|token|secret)|abc123|xxx+)/i +function placeholderSubject(credentialType: string, matched: string): string | null { + // A private key's "value" is a base64 PEM body: a high-entropy blob, not a + // string that can announce itself as fake. Excluded by construction rather + // than by trusting the anchoring above to hold for 1,700 random characters. + if (credentialType === 'Private key block') return null + return /['"`]([^'"`]*)['"`]/.exec(matched)?.[1] ?? matched +} /** SCREAMING_SNAKE values are constant identifiers, not credentials — * `UPDATE_PASSWORD = 'UPDATE_PASSWORD'` is an enum member. */ @@ -96,7 +160,34 @@ export const secretsAnalyzer: Analyzer = { description: 'Detects credentials committed to the repository (defensive; values are never reported).', async run(index) { const findings: AnalyzerFinding[] = [] - const findingLimit = 50 + // 50 was hit exactly on a 300k-LOC repository, which made every count + // anyone quoted a truncated prefix AND — because a truncated required pass + // is not authoritative — voided all five score axes over a benign cap. + // 500 is above any real repository's genuine count while still bounding a + // pathological tree. + const findingLimit = 500 + /** + * Every credential-shaped line this pass matched, including the ones the cap + * left no room to report. Counting past the cap is the whole point: 500 + * reported findings could mean 500 secrets or 5,000, and a consumer that + * cannot tell them apart has to treat both as a total loss. + * + * This is why the sweep no longer stops at the cap. Reading on costs one + * more regex pass over a tree the analyzer already sweeps whenever the cap + * is NOT hit, and it buys the difference between "we showed you 500 of 512" + * and "we showed you 500 of 9,000" — which is the difference between an + * immaterial cap and a security score nobody should publish. + */ + let matches = 0 + /** + * Building the finding is deferred because a reported secret carries a + * generated fix diff, and generating one for a finding the cap will discard + * is pure waste. + */ + const report = (build: () => AnalyzerFinding): void => { + matches++ + if (findings.length < findingLimit) findings.push(build()) + } // Where a move-to-env fix appends its variable. Undefined means the file // does not exist, which the diff renders as a new-file hunk. const envExample = index.files.find((file) => file.path === '.env.example')?.content @@ -117,15 +208,30 @@ export const secretsAnalyzer: Analyzer = { const isTestContext = TEST_PATH_RE.test(file.path) const isSeedScript = SEED_PATH_RE.test(file.path) const lines = content.split('\n') - for (let i = 0; i < lines.length && findings.length < findingLimit; i++) { + /** Indent column of the open placeholder block, or -1 when none is open. */ + let placeholderBlock = -1 + for (let i = 0; i < lines.length; i++) { const line = lines[i] - if (PLACEHOLDER.test(line)) continue + const indent = line.length - line.trimStart().length + const blank = line.trim() === '' + // A blank line ends nothing; the first non-blank line at or left of the + // opener's indent does. + if (placeholderBlock >= 0 && !blank && indent <= placeholderBlock) placeholderBlock = -1 + const declaresPlaceholder = PLACEHOLDER.test(line) + const inPlaceholderBlock = placeholderBlock >= 0 + // Only the OUTERMOST block is tracked; anything nested inside it is + // more deeply indented and therefore already covered. + if (placeholderBlock < 0 && declaresPlaceholder && PLACEHOLDER_BLOCK_OPEN.test(line)) { + placeholderBlock = indent + } + if (declaresPlaceholder || inPlaceholderBlock) continue for (const { name, re } of SECRET_PATTERNS) { const match = line.match(re) if (!match) continue if (RUNTIME_CREDENTIAL_REFERENCE.test(match[0])) continue - if (FAKE_LITERAL_VALUE.test(match[0])) { - findings.push({ + const subject = placeholderSubject(name, match[0]) + if (subject !== null && FAKE_LITERAL_VALUE.test(subject)) { + report(() => ({ category: 'SECURITY_HYGIENE', severity: 'INFO', title: `Credential-shaped placeholder ignored in ${file.path.split('/').pop()}`, @@ -136,7 +242,7 @@ export const secretsAnalyzer: Analyzer = { impactScore: 5, effort: 'low', metadata: { credentialType: name, placeholder: true }, - }) + })) break } // A credential committed to source is a single token. Internal @@ -161,7 +267,7 @@ export const secretsAnalyzer: Analyzer = { const isEnvFile = /(^|\/)\.env/.test(file.path) const baseName = file.path.split('/').pop() if (!isEnvFile && !messageString && isSeedScript && TEST_DOWNGRADEABLE.has(name)) { - findings.push({ + report(() => ({ category: 'SECURITY_HYGIENE', severity: 'MEDIUM', title: `Seed-script credential in ${baseName}`, @@ -172,11 +278,11 @@ export const secretsAnalyzer: Analyzer = { impactScore: 45, effort: 'low', metadata: { credentialType: name, seedScript: true }, - }) + })) break } if (!isEnvFile && (messageString || (isTestContext && TEST_DOWNGRADEABLE.has(name)))) { - findings.push({ + report(() => ({ category: 'SECURITY_HYGIENE', severity: 'LOW', title: messageString @@ -193,7 +299,7 @@ export const secretsAnalyzer: Analyzer = { impactScore: messageString ? 10 : 25, effort: 'low', metadata: { credentialType: name, testContext: isTestContext, messageString }, - }) + })) } else { // A concrete fix only where the evidence determines one: a tracked // .env is untracked wholesale, and a source assignment becomes an @@ -205,40 +311,46 @@ export const secretsAnalyzer: Analyzer = { // but its LINE is not the place to fix them. The next generation // overwrites any edit; the credential has to leave the generator's // input. So the leak is reported and the diff is withheld. - const fix = isEnvFile - ? untrackEnvFileFix(file.path) - : isGeneratedFile - ? undefined - : moveSecretToEnvFix({ - filePath: file.path, - line: i + 1, - lineText: line, - credentialType: name, - envExampleLines, - }) - findings.push({ - category: 'SECURITY_HYGIENE', - severity: isEnvFile ? 'CRITICAL' : 'HIGH', - title: `Possible ${name} committed in ${file.path.split('/').pop()}`, - description: `Line ${i + 1} of ${file.path} appears to contain a ${name}. Committed credentials should be treated as compromised.`, - filePath: file.path, - line: i + 1, - suggestion: 'Rotate this credential immediately, move it to environment configuration, and add the file to .gitignore. Consider a pre-commit secret scanner.', - ...(fix ? { fix } : {}), - impactScore: 95, - effort: 'low', - metadata: { credentialType: name }, + report(() => { + const fix = isEnvFile + ? untrackEnvFileFix(file.path) + : isGeneratedFile + ? undefined + : moveSecretToEnvFix({ + filePath: file.path, + line: i + 1, + lineText: line, + credentialType: name, + envExampleLines, + }) + return { + category: 'SECURITY_HYGIENE', + severity: isEnvFile ? 'CRITICAL' : 'HIGH', + title: `Possible ${name} committed in ${file.path.split('/').pop()}`, + description: `Line ${i + 1} of ${file.path} appears to contain a ${name}. Committed credentials should be treated as compromised.`, + filePath: file.path, + line: i + 1, + suggestion: 'Rotate this credential immediately, move it to environment configuration, and add the file to .gitignore. Consider a pre-commit secret scanner.', + ...(fix ? { fix } : {}), + impactScore: 95, + effort: 'low', + metadata: { credentialType: name }, + } }) } break // one finding per line } } } - return findings.length >= findingLimit + // Reaching the cap exactly is not a loss — every match was reported — so + // only an overflow truncates. What it reports is measurable: the share of + // this repository's real matches the reader can actually see. + return matches > findingLimit ? incompleteAnalyzerOutput(findings, { truncated: true, - detail: `Secret scanning stopped after ${findingLimit} matches.`, - metrics: { matches: findings.length, findingLimit }, + coverageRatio: measuredCoverage(findings.length, matches), + detail: `Secret scanning reported ${findings.length} of ${matches} credential matches; the ${findingLimit}-finding cap dropped the rest.`, + metrics: { matches, reported: findings.length, findingLimit }, }) : findings }, diff --git a/packages/analyzer-engine/src/security/engine.ts b/packages/analyzer-engine/src/security/engine.ts index 4cef718..4d3f453 100644 --- a/packages/analyzer-engine/src/security/engine.ts +++ b/packages/analyzer-engine/src/security/engine.ts @@ -560,16 +560,43 @@ const PATH_ANCHOR = /^(\/(?!\/)|[?#])/ const AUTHORITY_OPENER = /^([a-z][a-z0-9+.-]*:)?\/\//i /** - * Position-aware suppression for head-position sinks (SSRF). A URL built as a - * template/concat is only attacker-steerable when taint can reach its scheme/ - * authority: suppress when a constant head pins the authority (or is a - * single-slash relative path), or when the head expression is untainted (e.g. - * a `${BASE}` config var) and every tainted part lands after a path-anchoring - * constant fragment. Deliberately conservative — an empty, protocol-relative - * ('//') or incomplete-host ('http://', 'http://a') prefix, or a tainted head, - * still flags, and a non-template/concat argument is never suppressed. + * The relative reference of `new URL(reference, base)`, or null when the shape + * does not apply. + * + * WHATWG resolution makes the base supply the origin for every reference that + * is not itself absolute, so the origin-position question collapses onto the + * FIRST argument and the base drops out: `new URL('/booking/' + uid, WEBAPP_URL)` + * lands on WEBAPP_URL's origin no matter what `uid` holds. A single argument is + * not this shape — there is no base, so that argument is the whole URL. + * + * The base is not re-examined, matching the constant-first-argument carve-out + * in `evalOrigins`: `new URL('/login', req.url)` is the Next.js middleware + * idiom, and the same trade-off (a host injection through an attacker-chosen + * base is suppressed too) is accepted here for the same reason. + */ +function urlBaseRelativeReference(arg: SyntaxNode, lang: SastLanguage): SyntaxNode | null { + const call = asCall(arg, lang) + if (!call || !call.isConstruct || call.fullName.toLowerCase() !== 'url') return null + return call.args.length >= 2 ? call.args[0] ?? null : null +} + +/** + * Position-aware suppression for head-position sinks (SSRF, open redirect). A + * URL built as a template/concat is only attacker-steerable when taint can + * reach its scheme/authority: suppress when a constant head pins the authority + * (or is a single-slash relative path), or when the head expression is + * untainted (e.g. a `${BASE}` config var) and every tainted part lands after a + * path-anchoring constant fragment. Deliberately conservative — an empty, + * protocol-relative ('//') or incomplete-host ('http://', 'http://a') prefix, + * or a tainted head, still flags, and a non-template/concat argument is never + * suppressed. */ -function headTaintSuppressed(arg: SyntaxNode, ft: FunctionTaint, lang: SastLanguage): boolean { +function headTaintSuppressed(arg: SyntaxNode, ft: FunctionTaint, lang: SastLanguage, depth = 0): boolean { + // `new URL(ref, base)` — ask the same question of `ref`, which is what decides + // whether the base's origin survives. A reference that opens its own authority + // ('//host', 'https://host') fails the checks below exactly as it should. + const reference = depth < 4 ? urlBaseRelativeReference(arg, lang) : null + if (reference) return headTaintSuppressed(reference, ft, lang, depth + 1) const { constantPrefix, headExpr, parts } = urlHeadOf(arg, lang) if (!parts) return false // opaque expression — the whole argument is the URL if (constantPrefix !== null && (AUTHORITY_PINNED_PREFIX.test(constantPrefix) || SINGLE_SLASH_PATH.test(constantPrefix))) { diff --git a/packages/analyzer-engine/src/security/normalize.ts b/packages/analyzer-engine/src/security/normalize.ts index 0809340..191ea75 100644 --- a/packages/analyzer-engine/src/security/normalize.ts +++ b/packages/analyzer-engine/src/security/normalize.ts @@ -61,22 +61,60 @@ export function asFunction(node: SyntaxNode, lang: SastLanguage): NFunc | null { return { name, params: paramNames(node), body, node, line: node.startPosition.row + 1 } } -function paramNames(node: SyntaxNode): string[] { - // The parameter container differs per grammar; find it, then pull leaf names. +/** The parameter container for a function node; grammars name it differently. */ +function paramContainer(node: SyntaxNode): SyntaxNode | null { const containerTypes = new Set([ 'formal_parameters', 'parameters', 'parameter_list', 'method_parameters', 'formal_parameter_list', 'block_parameters', 'closure_parameters', ]) - let container: SyntaxNode | null = field(node, 'parameters') - if (!container) { - for (let i = 0; i < node.childCount; i++) { - const c = node.child(i) - if (c && containerTypes.has(c.type)) { - container = c - break - } + const container = field(node, 'parameters') + if (container) return container + for (let i = 0; i < node.childCount; i++) { + const c = node.child(i) + if (c && containerTypes.has(c.type)) return c + } + return null +} + +/** + * EVERY name a function's signature binds, including the leaves of a + * destructuring pattern (`({ ctx, input })`, `({ params }: Props)`). + * + * Distinct from {@link paramNames}, which returns POSITIONAL parameters and + * feeds the interprocedural summary — a destructured leaf has no argument + * position, so it cannot appear there. This set answers a different question: + * "did this function's signature introduce this identifier?" Destructuring is + * how request-handling code normally binds its request, so a gate that only + * saw positional names would miss the majority of real handlers. + * + * Over-approximates slightly: an identifier inside a default-value expression + * (`function f(a = fallback)`) is collected too. That only ever makes the gate + * more permissive, which is the safe direction for a precision narrowing. + */ +export function paramBoundNames(node: SyntaxNode): Set { + const names = new Set() + const add = (n: SyntaxNode | null): void => { + if (!n) return + if (n.type === 'identifier' || n.type === 'shorthand_property_identifier_pattern') { + names.add(n.text) + return + } + if (n.type === 'variable_name') { + names.add(n.text.replace(/^\$/, '')) + return } + // Type annotations contribute `type_identifier`, not `identifier`, so a + // blanket descent does not pull type names in. + for (const c of n.namedChildren) add(c) } + const container = paramContainer(node) + if (container) for (const c of container.namedChildren) add(c) + else add(field(node, 'parameter')) + return names +} + +function paramNames(node: SyntaxNode): string[] { + const container = paramContainer(node) // JS arrow with a single bare identifier param: `x => ...` if (!container) { const p = field(node, 'parameter') diff --git a/packages/analyzer-engine/src/security/taint.ts b/packages/analyzer-engine/src/security/taint.ts index 53e6d1c..fb7fcea 100644 --- a/packages/analyzer-engine/src/security/taint.ts +++ b/packages/analyzer-engine/src/security/taint.ts @@ -7,6 +7,7 @@ import { dottedName, identifierName, interpolationExprs, + paramBoundNames, stringLiteralValue, subscriptBase, unwrap, @@ -69,6 +70,47 @@ const REQUEST_ROOTS = new Set([ 'req', 'request', 'ctx', 'context', 'httprequest', 'httpcontext', 'event', 'args', '$_get', '$_post', '$_request', '$_cookie', '$_server', 'params', 'searchparams', ]) +/** + * Request roots whose NAME alone establishes nothing. + * + * `event`, `args`, `context`, `params` and `ctx` are ordinary local and loop + * names in ordinary code. `for (const event of salesforceEvents)` made every + * `event.` an untrusted source, so rows already read back from a CRM + * were treated as attacker input — three CRITICAL injection reports out of one + * loop variable. So these count as a request object only when the enclosing + * function BOUND them, positionally or by destructuring. + * + * The unambiguous roots are deliberately left ungated: nobody names a loop + * variable `req`, `httpRequest` or `$_GET`, and a framework that hands a + * request to a non-parameter binding still reaches those. + * + * Recall cost, stated rather than hidden: a handler that destructures its + * request into a LOCAL named `event` — `const event = JSON.parse(body)` — or a + * serverless adapter that binds the request through a module-level variable + * loses the source, and every finding that depended on it. That is a real + * narrowing, accepted because the name alone was never evidence. + */ +const PARAM_BOUND_REQUEST_ROOTS = new Set(['event', 'args', 'context', 'params', 'ctx']) + +/** Names the enclosing function's signature bound, for the ambiguous-root gate. */ +export interface TaintScope { + paramNames: ReadonlySet +} + +/** + * Whether `name` denotes an untrusted request object in the given scope. + * + * `scope` is undefined for callers that recognize request SHAPES without a + * function context (the navigation-target predicate in rules.ts). Those keep + * the pre-gate answer: they consult this only to decide whether an already- + * tainted value IS the request rather than something built from it, so + * narrowing them would change a measured predicate for no stated defect. + */ +function isRequestRoot(name: string, scope?: TaintScope): boolean { + if (!REQUEST_ROOTS.has(name)) return false + if (!PARAM_BOUND_REQUEST_ROOTS.has(name)) return true + return scope ? scope.paramNames.has(name) : true +} /** Member properties on a request object that expose untrusted data. */ const REQUEST_PROPS = new Set([ 'query', 'body', 'params', 'cookies', 'headers', 'form', 'get', 'post', 'data', @@ -139,7 +181,7 @@ const READER_SOURCE_METHODS = new Set([ const READER_RECEIVER = /reader|stream|buffer|^(br|rdr)$/ /** If the expression is an untrusted source, return its kind label. */ -export function sourceKindOf(node: SyntaxNode, lang: SastLanguage): string | null { +export function sourceKindOf(node: SyntaxNode, lang: SastLanguage, scope?: TaintScope): string | null { // `await params` / `await searchParams` — Next 15+ async route/page props. // Checked on the RAW node because unwrap() strips the await. Awaiting a bare // identifier with exactly these names is a strong framework signal; ordinary @@ -166,23 +208,28 @@ export function sourceKindOf(node: SyntaxNode, lang: SastLanguage): string | nul if ( (method === 'json' || method === 'text' || method === 'formdata') && call.receiverName && - REQUEST_ROOTS.has(call.receiverName.toLowerCase()) + isRequestRoot(call.receiverName.toLowerCase(), scope) ) { return `${call.receiverName}.${method}()` } // next/headers cookies() — every cookie value is attacker-controlled. + // useSearchParams()/useParams() — the client-side equivalents: both read + // straight off the URL. These are named here rather than inferred from the + // variable they land in, which is what makes the parameter-binding gate on + // `params` affordable: `const params = useSearchParams()` is a local, so + // the name proves nothing, but the CALL proves everything. if ( - method === 'cookies' && + (method === 'cookies' || method === 'usesearchparams' || method === 'useparams') && !call.receiverName && !call.isConstruct && (lang === 'javascript' || lang === 'typescript' || lang === 'tsx') ) { - return 'cookies()' + return method === 'cookies' ? 'cookies()' : `${call.method}()` } // Getter calls on a request-shaped root: searchParams.get('q'), // params.get(…). The member branch below already treats `.anything` // as a source, so this adds no exposure the member form doesn't have. - if (method === 'get' && call.receiverName && REQUEST_ROOTS.has(call.receiverName.toLowerCase())) { + if (method === 'get' && call.receiverName && isRequestRoot(call.receiverName.toLowerCase(), scope)) { return `${call.receiverName}.get` } // bare input() in python @@ -207,8 +254,8 @@ export function sourceKindOf(node: SyntaxNode, lang: SastLanguage): string | nul if (m) { const rootName = rootIdentifier(n, lang)?.toLowerCase() const prop = m.property.toLowerCase() - if (rootName && REQUEST_ROOTS.has(rootName) && REQUEST_PROPS.has(prop)) return `${rootName}.${prop}` - if (rootName && REQUEST_ROOTS.has(rootName)) return rootName // req..x + if (rootName && isRequestRoot(rootName, scope) && REQUEST_PROPS.has(prop)) return `${rootName}.${prop}` + if (rootName && isRequestRoot(rootName, scope)) return rootName // req..x // Go: r.URL / r.Header / r.Body etc. — request fields, gated on a // request-shaped root so a *sql.Rows named `r` is not caught. if (lang === 'go' && rootName && GO_REQUEST_ROOTS.has(rootName) && GO_REQUEST_MEMBERS.has(prop)) { @@ -274,6 +321,8 @@ function isSanitizer(call: NCall): boolean { interface Env { lang: SastLanguage + /** Names this function's signature bound — gates the ambiguous request roots. */ + scope: TaintScope taint: Map /** Remaining node-visit budget for the current phase (fixpoint solve, then * sink queries). Each evalOrigins visit accesses web-tree-sitter node @@ -304,7 +353,7 @@ function evalOrigins(node: SyntaxNode, env: Env, depth = 0): Origins { // 1. direct source? (raw node — sourceKindOf needs to see `await params` // before unwrap strips the await; it unwraps internally for everything else) - const src = sourceKindOf(node, lang) + const src = sourceKindOf(node, lang, env.scope) if (src) return [{ kind: 'source', sourceKind: src, node: n }] // 2. calls @@ -318,6 +367,24 @@ function evalOrigins(node: SyntaxNode, env: Env, depth = 0): Origins { // req.url))` idiom, not an open redirect or SSRF. Trade-off: the rare // `new URL('/fixed', attackerControlledBase)` host injection is suppressed too. if (call.fullName.toLowerCase() === 'url' && stringLiteralValue(call.args[0], lang) !== null) return [] + // `new URLSearchParams({ … })` is a query-string BUILDER, and its + // serialization is application/x-www-form-urlencoded: every character that + // could carry structure — '/', ':', '<', '&', '=', '?', '#' — comes back + // percent-encoded. A value put in this way can only ever emerge as an + // encoded query VALUE, so it can reach neither an origin nor a markup + // context. Gated on the object-literal argument form, which is the builder: + // `new URLSearchParams(location.search)` PARSES instead, and reading a + // value back out of it is the DOM-XSS shape, so that form keeps its taint. + // Residual unsoundness: `.get()` on a builder returns the value decoded + // again. That round trip is a no-op nobody writes. + if ( + call.isConstruct && + call.fullName.toLowerCase() === 'urlsearchparams' && + call.args[0] && + unwrap(call.args[0], lang).type === 'object' + ) { + return [] + } let acc: Origins = [] if (call.receiver) acc = mergeOrigins(acc, evalOrigins(call.receiver, env, depth + 1)) for (const a of call.args) acc = mergeOrigins(acc, evalOrigins(a, env, depth + 1)) @@ -393,7 +460,12 @@ export interface FunctionTaint { */ export function analyzeFunction(fn: NFunc, lang: SastLanguage): FunctionTaint { const taint = new Map() - const env: Env = { lang, taint, budget: TAINT_VISIT_BUDGET, exhausted: false } + // Lower-cased because every root lookup in sourceKindOf is lower-cased — + // `searchParams` must still match the `searchparams` root. + const paramNames = new Set( + [...paramBoundNames(fn.node)].map((p) => p.toLowerCase()), + ) + const env: Env = { lang, scope: { paramNames }, taint, budget: TAINT_VISIT_BUDGET, exhausted: false } // Seed parameters so we can learn which ones reach sinks. fn.params.forEach((p, i) => { diff --git a/packages/analyzer-engine/src/todos.ts b/packages/analyzer-engine/src/todos.ts index 19277ca..553f20c 100644 --- a/packages/analyzer-engine/src/todos.ts +++ b/packages/analyzer-engine/src/todos.ts @@ -1,4 +1,4 @@ -import { incompleteAnalyzerOutput, type Analyzer, type AnalyzerFinding } from './types' +import { incompleteAnalyzerOutput, measuredCoverage, type Analyzer, type AnalyzerFinding } from './types' const MARKER = /(?:\/\/|#|\/\*|