diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e3bf11..7377621 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.41 on GitHub](https://github.com/CodeTruss/codetruss-cli/releases/tag/v0.2.41), +The current public release is [v0.2.42 on GitHub](https://github.com/CodeTruss/codetruss-cli/releases/tag/v0.2.42), distributed from . The npm `latest` tag is still [`@codetruss/cli@0.2.24`](https://www.npmjs.com/package/@codetruss/cli/v/0.2.24): @@ -16,6 +16,58 @@ were superseded before distribution. No unreleased changes. +## 0.2.42 — 2026-08-07 + +- **A security scan that could have run for hours now finishes in seconds.** + Naming the callee of a chained call went the long way round, through a helper + that eagerly re-derived that same name twice more — so a left-deep method chain + cost 3^links to analyze. Eighteen chained `.replace()` calls in a single 19 KB + file extrapolated to roughly 2.1 hours, and a thirteen-link chain took + 30,839 ms; that chain now takes 2 ms. Only the number of times the name is + computed changed, never the answer. Two wall-clock ceilings back that up — + five seconds for one file, five minutes for a whole pass — so no future shape + can hang a scan instead of finishing it. A ceiling that fires is disclosed + rather than absorbed: the receipt names the files it cut and says plainly that + the rules which had not run there reported nothing, which is not the same as + finding nothing. +- **A finding in a file your change never touched is no longer reported as one + your change introduced.** Analyzers cap how many findings they report. Resolve + two and two cap slots free up, so findings that had merely been hidden in + untouched files entered the reported list for the first time — where the + baseline comparison called them introduced, and a signed receipt then asserted + that a change broke code its author never opened. The mirror image was just as + wrong: a finding pushed below the cap read as resolved when nothing had fixed + it. The comparison now runs over everything each pass found rather than only + what it reported, while the cap still decides what a receipt shows. The new + time ceilings above can hide a finding the same way, and that door is shut + too — but by the opposite means, because a capped finding was found and then + dropped whereas a file the clock cut was never analyzed at all. There is + nothing to recover in that case, so a file either the baseline or the final + could not finish is dropped from both sides, and the comparison makes no claim + about it in either direction. +- **A verification that passed is no longer reported as timed out because + something it started outlived it.** On Windows a descendant that escapes the + process tree keeps the inherited output pipes open, and CodeTruss waited on + those pipes — so a suite that passed in ten seconds and left a watcher behind + burned its entire deadline and produced exit code 124 on a signed receipt. A + local review provider lost finished reviews the same way. Capture now settles + two seconds after the command's own process exits, on the status the command + actually produced, and the escape is named in the output instead of being + absorbed silently. That grace is only ever paid when something really did + escape. What Windows still does not allow CodeTruss to reap, and what closing + it would cost, is stated in the code at the point the choice is made. +- **CodeTruss no longer reads its own receipts back in as your source code.** A + receipt `.patch` is the captured session diff — the full text of every changed + line — and it classified as source, so the tool analyzed its own audit trail. + Against this repository's real 156-receipt store that produced 52 spurious + findings, including duplication findings over receipts that repeat each other + by construction; those consumed the per-analyzer finding budgets and crowded + genuine findings out of the report entirely. It also ran the other way: an + identifier appearing anywhere in a receipt looked referenced, so "exported with + no consumer" findings silently vanished and returned depending on what the last + session happened to touch. A repository holding receipts now yields the same + findings as the same tree with no receipts in it at all. + ## 0.2.41 — 2026-08-07 - **You can dismiss a finding you have judged wrong, in the place the judgement diff --git a/packages/analyzer-engine/src/comment-slop.ts b/packages/analyzer-engine/src/comment-slop.ts index 6e997ea..ad28b2e 100644 --- a/packages/analyzer-engine/src/comment-slop.ts +++ b/packages/analyzer-engine/src/comment-slop.ts @@ -337,7 +337,6 @@ export const commentSlopAnalyzer: Analyzer = { const isDensityOutlier = (item: FileMeasurement) => item.ratio >= densityThreshold && item.commentLines >= DENSITY_MIN_COMMENTS - const findings: AnalyzerFinding[] = [] const restating = measurements .filter((item) => item.redundant.length >= REDUNDANT_FILE_THRESHOLD) .sort((left, right) => right.redundant.length - left.redundant.length) @@ -346,11 +345,15 @@ export const commentSlopAnalyzer: Analyzer = { .sort((left, right) => right.narration.length - left.narration.length) const findingLimit = 10 - for (const item of restating.slice(0, findingLimit)) { + // Built for every matching file, then split by the cap into reported and + // withheld. A finding that is never constructed cannot be compared against + // another run, and the delta then reads it as introduced the first time a + // cap slot frees up — see `analyzerWithheld` in types.ts. + const restatingFindings: AnalyzerFinding[] = restating.map((item) => { const outlier = isDensityOutlier(item) const low = item.redundant.length >= REDUNDANT_LOW_THRESHOLD && outlier const sample = item.redundant[0] - findings.push({ + return { category: 'DOCUMENTATION', severity: low ? 'LOW' : 'INFO', title: `${item.redundant.length} comments restate the code in ${item.path}`, @@ -368,15 +371,15 @@ export const commentSlopAnalyzer: Analyzer = { impactScore: low ? 25 : 15, effort: 'low', metadata: { count: item.redundant.length, sample: item.redundant.slice(0, 5) }, - }) - } + } + }) - for (const item of narrating.slice(0, findingLimit)) { + const narratingFindings: AnalyzerFinding[] = narrating.map((item) => { const low = item.narration.length >= NARRATION_LOW_THRESHOLD const sample = item.narration[0] const others = item.narration.length - 1 const allPlaceholder = item.narration.every((hit) => hit.tag === 'placeholder-deferral') - findings.push({ + return { category: 'DOCUMENTATION', severity: low ? 'LOW' : 'INFO', title: allPlaceholder @@ -399,8 +402,17 @@ export const commentSlopAnalyzer: Analyzer = { impactScore: low ? 25 : 15, effort: 'low', metadata: { count: item.narration.length, sample: item.narration.slice(0, 5) }, - }) - } + } + }) + + const findings = [ + ...restatingFindings.slice(0, findingLimit), + ...narratingFindings.slice(0, findingLimit), + ] + const withheld = [ + ...restatingFindings.slice(findingLimit), + ...narratingFindings.slice(findingLimit), + ] const metrics = { eligibleFiles: measurements.length, @@ -420,13 +432,13 @@ export const commentSlopAnalyzer: Analyzer = { truncated: true, detail: `Comment analysis hit a candidate bound (${candidates.length} candidate files).`, metrics: { ...metrics, candidates: candidates.length, candidateLimit }, - }) + }, withheld) } if (restating.length > findingLimit || narrating.length > findingLimit) { return annotatedAnalyzerOutput(findings, { detail: `Comment output capped at ${findingLimit} files per rule (${restating.length} restating, ${narrating.length} narrating).`, metrics: { ...metrics, candidates: candidates.length, candidateLimit }, - }) + }, withheld) } return annotatedAnalyzerOutput(findings, { metrics: { ...metrics, candidates: candidates.length, candidateLimit }, diff --git a/packages/analyzer-engine/src/complexity.ts b/packages/analyzer-engine/src/complexity.ts index b7ee67a..f919fe3 100644 --- a/packages/analyzer-engine/src/complexity.ts +++ b/packages/analyzer-engine/src/complexity.ts @@ -154,6 +154,10 @@ export const complexityAnalyzer: Analyzer = { const findingLimit = 20 const output = findings.slice(0, findingLimit) + // Handed to the runner, never reported: proof that this tree already + // contained them, so a comparison cannot read one entering a freed cap slot + // as a finding the change introduced. + const withheld = findings.slice(findingLimit) // Only the candidate-file cap loses coverage. The finding cap bounds the // persisted/displayed output after every candidate was analyzed, so it must // not make otherwise authoritative scores disappear. @@ -162,13 +166,13 @@ export const complexityAnalyzer: Analyzer = { truncated: true, detail: `Complexity analysis hit a candidate bound (${candidates.length} candidate files, ${findings.length} matches).`, metrics: { candidates: candidates.length, candidateLimit, matches: findings.length, findingLimit }, - }) + }, withheld) } if (findings.length > findingLimit) { return annotatedAnalyzerOutput(output, { detail: `Complexity output capped at ${findingLimit} of ${findings.length} matches.`, metrics: { candidates: candidates.length, candidateLimit, matches: findings.length, findingLimit }, - }) + }, withheld) } return output }, diff --git a/packages/analyzer-engine/src/dead-code.ts b/packages/analyzer-engine/src/dead-code.ts index 9c47f3a..fc3482d 100644 --- a/packages/analyzer-engine/src/dead-code.ts +++ b/packages/analyzer-engine/src/dead-code.ts @@ -102,18 +102,22 @@ export const deadCodeAnalyzer: Analyzer = { // bounds OUTPUT over an analysis that covered every candidate file. const truncated = jsFiles.length > candidateLimit const output = findings.slice(0, findingLimit) + // Unreported, but retained as evidence that this tree already contained + // them — a baseline/final comparison must not read a finding surfacing into + // a freed cap slot as one the change introduced. + const withheld = findings.slice(findingLimit) if (truncated) { return incompleteAnalyzerOutput(output, { truncated: true, detail: `Dead-code analysis hit a bound (${jsFiles.length} candidate files, ${findings.length} matches).`, metrics: { candidates: jsFiles.length, candidateLimit, matches: findings.length, findingLimit }, - }) + }, withheld) } if (findings.length > findingLimit) { return annotatedAnalyzerOutput(output, { detail: `Dead-code output capped at ${findingLimit} of ${findings.length} matches.`, metrics: { candidates: jsFiles.length, candidateLimit, matches: findings.length, findingLimit }, - }) + }, withheld) } return output }, diff --git a/packages/analyzer-engine/src/duplication.ts b/packages/analyzer-engine/src/duplication.ts index 664029a..6804e56 100644 --- a/packages/analyzer-engine/src/duplication.ts +++ b/packages/analyzer-engine/src/duplication.ts @@ -69,6 +69,9 @@ export const duplicationAnalyzer: Analyzer = { const findingLimit = 25 const output = findings.slice(0, findingLimit) + // Retained unreported so a comparison against another run can tell a pair + // that was merely over the cap here from one that did not exist here. + const withheld = findings.slice(findingLimit) // Scanning fewer candidates is real coverage loss. Capping the number of // persisted duplicate pairs after every candidate was compared is only an // output bound and keeps the pass authoritative. @@ -77,13 +80,13 @@ export const duplicationAnalyzer: Analyzer = { truncated: true, detail: `Duplication analysis hit a candidate bound (${candidates.length} candidate files, ${findings.length} matches).`, metrics: { candidates: candidates.length, candidateLimit, matches: findings.length, findingLimit }, - }) + }, withheld) } if (findings.length > findingLimit) { return annotatedAnalyzerOutput(output, { detail: `Duplication output capped at ${findingLimit} of ${findings.length} matches.`, metrics: { candidates: candidates.length, candidateLimit, matches: findings.length, findingLimit }, - }) + }, withheld) } return output }, diff --git a/packages/analyzer-engine/src/indexer.ts b/packages/analyzer-engine/src/indexer.ts index 88dee8c..236523f 100644 --- a/packages/analyzer-engine/src/indexer.ts +++ b/packages/analyzer-engine/src/indexer.ts @@ -19,6 +19,18 @@ const IGNORED_DIRS = new Set([ '.git', 'node_modules', '.next', 'dist', 'build', 'out', 'coverage', '.venv', 'venv', '__pycache__', '.pytest_cache', 'vendor', 'target', '.turbo', '.cache', '.idea', '.vscode', + // CodeTruss's own receipt/audit store. A receipt `.patch` is the captured + // session diff — the full text of every changed line — and classifies as + // `source`, so its identifiers counted as repo-wide usage and silently + // suppressed genuine "exported with no consumer" findings, flickering on and + // off with whatever the last session happened to touch. Ignored like `.git` + // rather than disclosed as an exclusion: the store is CodeTruss's own + // metadata, gitignored by design, not committed product code. The CLI pins it + // under `.codetruss/` (config `receiptDir` rejects any relocation outside it), + // so the directory name is the whole surface to exclude. This is an analysis + // exclusion only — scope classification reads Git, not this walk, and already + // filters `.codetruss/` in git.ts. + '.codetruss', ]) const DEFAULT_MAX_FILES = 20_000 diff --git a/packages/analyzer-engine/src/overengineering.ts b/packages/analyzer-engine/src/overengineering.ts index f906b59..42713da 100644 --- a/packages/analyzer-engine/src/overengineering.ts +++ b/packages/analyzer-engine/src/overengineering.ts @@ -249,12 +249,16 @@ export const overengineeringAnalyzer: Analyzer = { } } - const findings: AnalyzerFinding[] = [] const findingLimit = 10 - for (const file of speculative.slice(0, findingLimit)) { + // Every match is turned into a finding BEFORE the per-rule cap is applied. + // The cap then splits them into what this pass reports and what it withholds + // — a finding that only exists on one side of that split cannot be compared + // against another run, and an uncomparable finding is one a delta calls + // "introduced" the moment a cap slot frees up somewhere unrelated. + const speculativeFindings: AnalyzerFinding[] = speculative.map((file) => { const listed = file.names.slice(0, 5).map((name) => `\`${name}\``).join(', ') const rest = file.names.length - Math.min(file.names.length, 5) - findings.push({ + return { category: 'DEAD_CODE', severity: 'INFO', title: `${file.names.length} export(s) with no consumer in ${file.path}`, @@ -268,26 +272,33 @@ export const overengineeringAnalyzer: Analyzer = { impactScore: 15, effort: 'low', metadata: { exports: file.names.slice(0, 10) }, - }) - } + } + }) - for (const file of rethrows.slice(0, findingLimit)) { - findings.push({ - category: 'TECH_DEBT', - severity: 'INFO', - title: `${file.lines.length} catch block(s) log and rethrow in ${file.path}`, - description: - `${file.path} catches an error at line ${file.lines[0]}${file.lines.length > 1 ? ` and ${file.lines.length - 1} other place(s)` : ''}, ` - + 'logs it, and rethrows it unchanged. The handler adds a duplicate log line and no behaviour: the caller ' - + 'still receives the original error, and the stack is now reported twice.', - filePath: file.path, - line: file.lines[0], - suggestion: 'Remove the catch and let the error propagate, or handle it where the extra context exists.', - impactScore: 15, - effort: 'low', - metadata: { lines: file.lines.slice(0, 10) }, - }) - } + const rethrowFindings: AnalyzerFinding[] = rethrows.map((file) => ({ + category: 'TECH_DEBT', + severity: 'INFO', + title: `${file.lines.length} catch block(s) log and rethrow in ${file.path}`, + description: + `${file.path} catches an error at line ${file.lines[0]}${file.lines.length > 1 ? ` and ${file.lines.length - 1} other place(s)` : ''}, ` + + 'logs it, and rethrows it unchanged. The handler adds a duplicate log line and no behaviour: the caller ' + + 'still receives the original error, and the stack is now reported twice.', + filePath: file.path, + line: file.lines[0], + suggestion: 'Remove the catch and let the error propagate, or handle it where the extra context exists.', + impactScore: 15, + effort: 'low', + metadata: { lines: file.lines.slice(0, 10) }, + })) + + const findings = [ + ...speculativeFindings.slice(0, findingLimit), + ...rethrowFindings.slice(0, findingLimit), + ] + const withheld = [ + ...speculativeFindings.slice(findingLimit), + ...rethrowFindings.slice(findingLimit), + ] // Only the candidate-file cap loses coverage; the finding cap bounds output // over an analysis that still examined every candidate. @@ -304,13 +315,13 @@ export const overengineeringAnalyzer: Analyzer = { truncated: true, detail: `Speculative-structure analysis hit a candidate bound (${production.length} candidate files).`, metrics, - }) + }, withheld) } if (speculative.length > findingLimit || rethrows.length > findingLimit) { return annotatedAnalyzerOutput(findings, { detail: `Speculative-structure output capped at ${findingLimit} files per rule (${speculative.length} export, ${rethrows.length} rethrow).`, metrics, - }) + }, withheld) } return annotatedAnalyzerOutput(findings, { metrics }) }, diff --git a/packages/analyzer-engine/src/removed-routes.ts b/packages/analyzer-engine/src/removed-routes.ts index 34f0af7..3a525f4 100644 --- a/packages/analyzer-engine/src/removed-routes.ts +++ b/packages/analyzer-engine/src/removed-routes.ts @@ -68,10 +68,12 @@ export const removedRoutesAnalyzer: Analyzer = { } if (findings.length > FINDING_LIMIT) { + // The over-cap prefixes ride along unreported so a comparison against + // another run can tell "was already here, just not shown" from "is new". return annotatedAnalyzerOutput(findings.slice(0, FINDING_LIMIT), { detail: `Removed-route output capped at ${FINDING_LIMIT} of ${findings.length} prefixes.`, metrics: { prefixes: findings.length, findingLimit: FINDING_LIMIT }, - }) + }, findings.slice(FINDING_LIMIT)) } return findings }, diff --git a/packages/analyzer-engine/src/runner.ts b/packages/analyzer-engine/src/runner.ts index c169a18..b34f59b 100644 --- a/packages/analyzer-engine/src/runner.ts +++ b/packages/analyzer-engine/src/runner.ts @@ -1,6 +1,7 @@ import { getAnalyzers } from './registry' import { analyzerResult, + analyzerWithheld, REGISTRY_ONLY_ANALYSIS, type AnalyzerContext, type AnalyzerFinding, @@ -14,21 +15,32 @@ export interface AnalyzerPass { error?: string } -/** Run every deterministic analyzer without allowing one failed pass to abort the suite. */ +/** + * Run every deterministic analyzer without allowing one failed pass to abort the suite. + * + * `withheld` collects what each pass found and then dropped to respect its own + * finding cap. It is never reported or persisted — it exists so a baseline/final + * comparison can tell "hidden by a cap" apart from "not present", which is the + * difference between an honest delta and one that blames a change for findings + * it did not introduce. See `analyzerWithheld` in types.ts. + */ export async function runAnalyzers( index: RepoIndex, context: AnalyzerContext = REGISTRY_ONLY_ANALYSIS, -): Promise<{ findings: AnalyzerFinding[]; passes: AnalyzerPass[] }> { +): Promise<{ findings: AnalyzerFinding[]; withheld: AnalyzerFinding[]; passes: AnalyzerPass[] }> { const findings: AnalyzerFinding[] = [] + const withheld: AnalyzerFinding[] = [] const passes: AnalyzerPass[] = [] for (const analyzer of getAnalyzers()) { try { - const raw = analyzerResult(await analyzer.run(index, context)) + const output = await analyzer.run(index, context) + const raw = analyzerResult(output) const result = { ...raw, findings: raw.findings.map((finding) => ({ ...finding, analyzerId: analyzer.id })), } findings.push(...result.findings) + withheld.push(...analyzerWithheld(output).map((finding) => ({ ...finding, analyzerId: analyzer.id }))) passes.push({ id: analyzer.id, result }) } catch (error) { const detail = error instanceof Error ? error.message : String(error) @@ -39,5 +51,5 @@ export async function runAnalyzers( }) } } - return { findings, passes } + return { findings, withheld, passes } } diff --git a/packages/analyzer-engine/src/security/engine.ts b/packages/analyzer-engine/src/security/engine.ts index 8250047..3f7cac8 100644 --- a/packages/analyzer-engine/src/security/engine.ts +++ b/packages/analyzer-engine/src/security/engine.ts @@ -18,7 +18,7 @@ import { } from './taint' import { PATTERN_RULES, TAINT_ASSIGN_SINKS, TAINT_SINKS, asNamedValue, ruleAppliesTo, type TaintAssignSink, type TaintSink } from './rules' import { CWE } from './cwe' -import type { CodeLocation, Flow, SastFinding, SastResult } from './types' +import type { CodeLocation, Flow, SastDiagnostics, SastFinding, SastResult } from './types' /** * The SAST engine: parse → taint → match rules → findings. @@ -47,11 +47,74 @@ export interface ScanInput { export interface ScanOptions { /** Rule ids to keep. A rule outside the set never runs against a node. */ ruleIds?: ReadonlySet + /** Per-file wall-clock ceiling. Defaults to {@link FILE_TIME_BUDGET_MS}. */ + fileTimeBudgetMs?: number + /** Whole-pass wall-clock ceiling. Defaults to {@link PASS_TIME_BUDGET_MS}. */ + passTimeBudgetMs?: number } const MAX_FINDINGS_PER_FILE = 100 const MAX_FINDINGS_PER_SCAN = 100_000 +/** + * Wall-clock ceilings for the security pass. + * + * The node and taint budgets bound WORK, not TIME, and work is not a proxy for + * time: one visit can cost microseconds or, on a pathological shape, orders of + * magnitude more. Only a clock bounds the answer to "when does this finish". + * + * The numbers come from measurement, not taste. Over 8,976 files — CPython + * 3.14's stdlib (1,852), its site-packages (6,449, including sympy and + * pygments), and this repository's own `src/` (337) — the slowest single file + * took 609 ms, p99 was 101 ms, and p50 was 3 ms. 5 s is ~8x the slowest file any + * of those corpora produced, so nothing that completes today can trip it, while + * a file that would otherwise run for minutes is cut in seconds. The whole + * site-packages pass finishes in 63 s, so the 5-minute pass ceiling is ~5x the + * largest corpus measured: a repository would need roughly 30,000 analyzable + * files to approach it honestly. + * + * These are deliberately NOT determinism-preserving, and that is the trade: + * identical output run-to-run is worth more than a scan that never returns, and + * the moment a ceiling fires the diagnostics name the files it cut so the + * result is read as partial rather than clean. A silent skip would be worse + * than the hang it replaces. + * + * The hosted runner keeps its own, tighter job budget on top of these + * (`runIsolatedSecurityScan`); these are the backstop for every in-process + * caller — the CLI's local pass and the agent hooks — which had none. + */ +export const FILE_TIME_BUDGET_MS = 5_000 +export const PASS_TIME_BUDGET_MS = 300_000 + +/** How many capped/skipped paths the diagnostics carry. The counts stay + * authoritative for the totals; this bounds only what gets named. */ +const MAX_DISCLOSED_PATHS = 50 + +/** + * A wall-clock ceiling that is cheap enough to consult per AST node. + * + * `Date.now()` per node measurably costs on a 400k-node tree, so the clock is + * read once every {@link CLOCK_STRIDE} calls. Once expired it stays expired — + * no further clock reads, and no chance of a ceiling "un-firing". + */ +const CLOCK_STRIDE = 512 + +function deadlineGate(deadline: number): () => boolean { + // Starts at 1 so the FIRST consultation reads the clock: a budget already + // spent when the file began must fire even on a file too small to reach the + // stride. After that the stride amortizes the cost away. + let countdown = 1 + let expired = false + return () => { + if (expired) return true + if (--countdown > 0) return false + countdown = CLOCK_STRIDE + if (Date.now() < deadline) return false + expired = true + return true + } +} + /** Scan a set of source files and return findings + honest diagnostics. */ export async function scanFiles( files: ScanInput[], @@ -64,6 +127,14 @@ export async function scanFiles( let truncatedFiles = 0 let findingsTruncated = false const degraded = new Set() + let timeCappedFiles = 0 + let timeSkippedFiles = 0 + const timeCappedPaths: string[] = [] + const timeSkippedPaths: string[] = [] + + const fileBudgetMs = options.fileTimeBudgetMs ?? FILE_TIME_BUDGET_MS + const passDeadline = Date.now() + (options.passTimeBudgetMs ?? PASS_TIME_BUDGET_MS) + let passBudgetExceeded = false // web-tree-sitter grows its WASM heap while parsing and does not return that // high-water allocation to the host process between files. Continuing with @@ -90,14 +161,36 @@ export async function scanFiles( filesSkipped++ continue } + // The pass ceiling. A file past it is not analyzed at all, and is named as + // skipped rather than counted as scanned-and-clean. + if (passBudgetExceeded || Date.now() >= passDeadline) { + passBudgetExceeded = true + filesSkipped++ + timeSkippedFiles++ + if (timeSkippedPaths.length < MAX_DISCLOSED_PATHS) timeSkippedPaths.push(file.filePath) + continue + } try { - const fileFindings = await scanOne(file.filePath, file.content, lang, degraded, parser, options) + const fileFindings = await scanOne( + file.filePath, + file.content, + lang, + degraded, + parser, + options, + // Never let one file spend the whole pass budget. + Math.min(fileBudgetMs, passDeadline - Date.now()), + ) if (fileFindings === null) { filesSkipped++ continue } filesScanned++ if (fileFindings.truncated) truncatedFiles++ + if (fileFindings.timeCapped) { + timeCappedFiles++ + if (timeCappedPaths.length < MAX_DISCLOSED_PATHS) timeCappedPaths.push(file.filePath) + } for (const f of fileFindings.findings) { if (findings.length >= MAX_FINDINGS_PER_SCAN) { findingsTruncated = true @@ -125,6 +218,9 @@ export async function scanFiles( truncatedFiles, findingsTruncated, resourceLimitReached: memoryLimitReached, + ...(timeCappedFiles ? { timeCappedFiles, timeCappedPaths } : {}), + ...(timeSkippedFiles ? { timeSkippedFiles, timeSkippedPaths } : {}), + ...(passBudgetExceeded ? { budgetExceeded: true } : {}), }, } } @@ -146,12 +242,26 @@ export function mergeSastResults(results: SastResult[], inputFiles: number): Sas let filesScanned = 0 let filesSkipped = 0 let truncatedFiles = 0 + let timeCappedFiles = 0 + let timeSkippedFiles = 0 + const timeCappedPaths: string[] = [] + const timeSkippedPaths: string[] = [] for (const result of results) { filesScanned += result.diagnostics.filesScanned filesSkipped += result.diagnostics.filesSkipped truncatedFiles += result.diagnostics.truncatedFiles + timeCappedFiles += result.diagnostics.timeCappedFiles ?? 0 + timeSkippedFiles += result.diagnostics.timeSkippedFiles ?? 0 for (const language of result.diagnostics.degradedLanguages) degradedLanguages.add(language) + for (const path of result.diagnostics.timeCappedPaths ?? []) { + if (timeCappedPaths.length < MAX_DISCLOSED_PATHS) timeCappedPaths.push(path) + } + for (const path of result.diagnostics.timeSkippedPaths ?? []) { + if (timeSkippedPaths.length < MAX_DISCLOSED_PATHS) timeSkippedPaths.push(path) + } } + timeCappedPaths.sort() + timeSkippedPaths.sort() return { findings, @@ -164,6 +274,8 @@ export function mergeSastResults(results: SastResult[], inputFiles: number): Sas findingsTruncated, resourceLimitReached: results.some((result) => result.diagnostics.resourceLimitReached), budgetExceeded: results.some((result) => result.diagnostics.budgetExceeded), + ...(timeCappedFiles ? { timeCappedFiles, timeCappedPaths } : {}), + ...(timeSkippedFiles ? { timeSkippedFiles, timeSkippedPaths } : {}), failureReason: results.find((result) => result.diagnostics.failureReason)?.diagnostics.failureReason, }, } @@ -172,6 +284,48 @@ export function mergeSastResults(results: SastResult[], inputFiles: number): Sas interface FileScan { findings: SastFinding[] truncated: boolean + /** The per-file wall-clock ceiling fired: analysis of this file is partial. */ + timeCapped: boolean +} + +/** + * The sentence a receipt or a hosted report prints when a wall-clock ceiling + * fired — naming the files, not just counting them. + * + * One function so the CLI receipt and the hosted report say the SAME thing in + * the same voice. A timeout that quietly produced "no findings" would be worse + * than the hang it replaced, so the wording is explicit that a file the clock + * cut was not cleared: silence about it is missing evidence, not absence of a + * defect. Returns undefined when no ceiling fired, so callers can spread it. + */ +export function timeCeilingDisclosure(diagnostics: SastDiagnostics): string | undefined { + const parts: string[] = [] + const capped = diagnostics.timeCappedFiles ?? 0 + const skipped = diagnostics.timeSkippedFiles ?? 0 + if (capped > 0) { + parts.push( + // "a time ceiling", not "the per-file ceiling": a file that starts just + // before the pass deadline is cut by the pass budget through the same + // gate, and the sentence has to stay true in both cases. + `a time ceiling stopped security analysis partway through ${capped} file(s) — ` + + `${namePaths(diagnostics.timeCappedPaths ?? [], capped)}; the rules that had not run there reported nothing, ` + + `which is not the same as finding nothing`, + ) + } + if (skipped > 0) { + parts.push( + `the whole-pass time ceiling was reached and ${skipped} file(s) were not analyzed at all — ` + + `${namePaths(diagnostics.timeSkippedPaths ?? [], skipped)}`, + ) + } + return parts.length > 0 ? parts.join('; ') : undefined +} + +/** Name the paths we kept, and say plainly how many we did not keep. */ +function namePaths(paths: string[], total: number): string { + if (paths.length === 0) return 'their paths were not retained' + const rest = total - paths.length + return paths.join(', ') + (rest > 0 ? `, and ${rest} more` : '') } /** Which param indexes of a local function reach which sink. */ @@ -194,7 +348,11 @@ async function scanOne( degraded: Set, parser: SastParser, options: ScanOptions, + timeBudgetMs: number, ): Promise { + // The clock starts before the parse, so a slow parse eats into this file's + // own budget rather than escaping the ceiling. + const outOfTime = deadlineGate(Date.now() + timeBudgetMs) const parsed = await parser.parse(lang, content) if (!parsed) { degraded.add(lang) @@ -218,6 +376,15 @@ async function scanOne( let nodeCount = 0 let truncated = false + let timeCapped = false + // Consulted at every phase boundary and inside every walk. Once it fires the + // remaining phases short-circuit: partial findings are kept and the file is + // reported as capped, never as a fully-analyzed file with nothing in it. + const expired = () => { + if (!outOfTime()) return false + timeCapped = true + return true + } // ---- gather functions & solve taint per function ---- const fnRecords: FnRecord[] = [] @@ -226,6 +393,7 @@ async function scanOne( truncated = true return } + if (expired()) return const fn = asFunction(node, lang) if (fn && fn.body) { const ft = analyzeFunction(fn, lang) @@ -236,8 +404,10 @@ async function scanOne( // ---- direct sink findings + build interprocedural summaries ---- for (const rec of fnRecords) { if (!rec.fn.body) continue + if (expired()) break walk(rec.fn.body, (node) => { if (findings.length >= MAX_FINDINGS_PER_FILE) return + if (expired()) return if (isNestedFnBoundary(node, rec.fn, lang)) return // Assignment-shaped sinks (XSS): `__html:` is a JSX pair and // `el.innerHTML =` an assignment, so neither reaches a call-based rule. @@ -284,8 +454,10 @@ async function scanOne( if (summaries.size > 0) { for (const rec of fnRecords) { if (!rec.fn.body) continue + if (expired()) break walk(rec.fn.body, (node) => { if (findings.length >= MAX_FINDINGS_PER_FILE) return + if (expired()) return const call = asCall(node, lang) if (!call || call.isConstruct) return if (!bindsToLocalFn(call)) return // don't bind a member call to a same-name local fn @@ -306,7 +478,7 @@ async function scanOne( } // ---- pattern rules (single pass over the whole tree) ---- - runPatternRules(parsed.rootNode, applicablePatterns, lang, filePath, findings, lines) + runPatternRules(parsed.rootNode, applicablePatterns, lang, filePath, findings, lines, expired) parsed.release() // A drained taint budget means some solve or sink query degraded to @@ -314,7 +486,11 @@ async function scanOne( // nodes (parsed.hasError) is still fully walked best-effort, and the // per-file findings cap only bounds OUTPUT — neither degrades coverage. if (fnRecords.some((rec) => taintBudgetExhausted(rec.ft))) truncated = true - return { findings, truncated } + // A time-capped file is the same coverage loss as a node-budget one, so it + // rides the SAME `truncated` path every consumer already handles. The + // separate flag only lets the caller name it. + if (timeCapped) truncated = true + return { findings, truncated, timeCapped } } /** Pattern-rule pass over a tree. Cheap (no taint), so it always runs even when @@ -326,12 +502,14 @@ function runPatternRules( filePath: string, findings: SastFinding[], lines: string[], + expired: () => boolean, ): void { // Per-rule path exclusion (seed/migration/ops directories where the pattern // is expected and harmless) — resolved once per file, not per node. const activePatterns = patterns.filter((rule) => !rule.excludePath || !rule.excludePath.test(filePath)) walk(root, (node) => { if (findings.length >= MAX_FINDINGS_PER_FILE) return + if (expired()) return for (const rule of activePatterns) { for (const hit of rule.test(node, lang)) { const info = CWE[rule.cweKey] diff --git a/packages/analyzer-engine/src/security/normalize.ts b/packages/analyzer-engine/src/security/normalize.ts index 5572e11..0809340 100644 --- a/packages/analyzer-engine/src/security/normalize.ts +++ b/packages/analyzer-engine/src/security/normalize.ts @@ -184,22 +184,56 @@ const CALL_TYPES: Record> = { rust: new Set(['call_expression', 'macro_invocation']), } +/** + * The callee child of a call node — what {@link asCall} reports as + * {@link NCall.callee} — resolved without building an `NCall`. + * + * Split out because `dottedName` needs ONLY this. Reaching it through `asCall` + * made naming a chained call exponential: `asCall` eagerly resolves `fullName` + * and `receiverName` (two `dottedName` descents into the same receiver) and + * `dottedName` then descended into the callee a third time — 3x the work per + * link of a method chain, so `a.f().f()…` cost 3^links. An 18-link chain of + * `str.replace()` (pygments' `escape_tex`) took hours in a 19 KB file. + */ +function calleeOf(node: SyntaxNode, lang: SastLanguage): SyntaxNode | null { + if (lang === 'java') { + // method_invocation names the method; object_creation/explicit_constructor + // name the type. + return node.type === 'method_invocation' ? field(node, 'name') : field(node, 'type') + } + if (lang === 'python') return field(node, 'function') + if (lang === 'ruby') return field(node, 'method') + if (lang === 'php') { + if (node.type === 'object_creation_expression') { + return node.namedChildren.find((c) => c.type === 'name' || c.type === 'qualified_name') ?? null + } + if (node.type === 'function_call_expression') return field(node, 'function') + // member_call_expression / scoped_call_expression + return field(node, 'name') + } + if (lang === 'rust' && node.type === 'macro_invocation') return field(node, 'macro') + // JS family, C#, Go, Rust call_expression / new / invocation: + return node.type === 'new_expression' + ? field(node, 'constructor') + : field(node, 'function') ?? field(node, 'type') +} + export function asCall(node: SyntaxNode, lang: SastLanguage): NCall | null { if (!CALL_TYPES[lang].has(node.type)) return null const line = node.startPosition.row + 1 const isConstruct = node.type === 'new_expression' || node.type === 'object_creation_expression' + const callee = calleeOf(node, lang) if (lang === 'java') { if (node.type === 'method_invocation') { const obj = field(node, 'object') - const nameNode = field(node, 'name') - const name = nameNode?.text ?? '' + const name = callee?.text ?? '' const receiverName = obj ? dottedName(obj, lang) : null const fullName = receiverName ? `${receiverName}.${name}` : name return { node, line, - callee: nameNode, + callee, fullName, method: name, receiver: obj, @@ -209,48 +243,36 @@ export function asCall(node: SyntaxNode, lang: SastLanguage): NCall | null { } } // object_creation_expression: new Type(args) - const type = field(node, 'type') - const name = type?.text ?? '' - return mk(node, line, type, null, name, argList(field(node, 'arguments')), true, lang) + return mk(node, line, callee, null, callee?.text ?? '', argList(field(node, 'arguments')), true, lang) } if (lang === 'python') { - const fn = field(node, 'function') - return mk(node, line, fn, receiverOf(fn, lang), lastSegment(fn, lang), argList(field(node, 'arguments')), false, lang) + return mk(node, line, callee, receiverOf(callee, lang), lastSegment(callee, lang), argList(field(node, 'arguments')), false, lang) } if (lang === 'ruby') { const recv = field(node, 'receiver') - const method = field(node, 'method')?.text ?? '' - return mk(node, line, field(node, 'method'), recv, method, argList(field(node, 'arguments')), false, lang) + return mk(node, line, callee, recv, callee?.text ?? '', argList(field(node, 'arguments')), false, lang) } if (lang === 'php') { if (node.type === 'object_creation_expression') { - const type = node.namedChildren.find((c) => c.type === 'name' || c.type === 'qualified_name') - return mk(node, line, type ?? null, null, type?.text ?? '', argList(field(node, 'arguments')), true, lang) + return mk(node, line, callee, null, callee?.text ?? '', argList(field(node, 'arguments')), true, lang) } if (node.type === 'function_call_expression') { - const fn = field(node, 'function') - return mk(node, line, fn, null, fn?.text?.replace(/^\\/, '') ?? '', argList(field(node, 'arguments')), false, lang) + return mk(node, line, callee, null, callee?.text?.replace(/^\\/, '') ?? '', argList(field(node, 'arguments')), false, lang) } // member_call_expression / scoped_call_expression const obj = field(node, 'object') ?? field(node, 'scope') - const name = field(node, 'name')?.text ?? '' - return mk(node, line, field(node, 'name'), obj, name, argList(field(node, 'arguments')), false, lang) + return mk(node, line, callee, obj, callee?.text ?? '', argList(field(node, 'arguments')), false, lang) } if (lang === 'rust' && node.type === 'macro_invocation') { - const macro = field(node, 'macro') - return mk(node, line, macro, null, macro?.text ?? '', argList(node.namedChildren.find((c) => c.type === 'token_tree') ?? null), false, lang) + return mk(node, line, callee, null, callee?.text ?? '', argList(node.namedChildren.find((c) => c.type === 'token_tree') ?? null), false, lang) } // JS family, C#, Go, Rust call_expression / new / invocation: - const fnField = node.type === 'new_expression' - ? field(node, 'constructor') - : field(node, 'function') ?? field(node, 'type') - const fn = fnField - return mk(node, line, fn, receiverOf(fn, lang), lastSegment(fn, lang), argList(field(node, 'arguments')), isConstruct, lang) + return mk(node, line, callee, receiverOf(callee, lang), lastSegment(callee, lang), argList(field(node, 'arguments')), isConstruct, lang) } function mk( @@ -360,8 +382,12 @@ export function dottedName(node: SyntaxNode, lang: SastLanguage): string | null } const sub = subscriptBase(n) if (sub) return dottedName(sub, lang) - const call = asCall(n, lang) - if (call && call.callee) return dottedName(call.callee, lang) + // `calleeOf`, not `asCall`: building an NCall here re-derives this same name + // twice more and makes a chained call exponential in its length. + if (CALL_TYPES[lang].has(n.type)) { + const callee = calleeOf(n, lang) + if (callee) return dottedName(callee, lang) + } switch (n.type) { case 'identifier': case 'property_identifier': diff --git a/packages/analyzer-engine/src/security/types.ts b/packages/analyzer-engine/src/security/types.ts index 6f3c309..d57ef20 100644 --- a/packages/analyzer-engine/src/security/types.ts +++ b/packages/analyzer-engine/src/security/types.ts @@ -72,11 +72,28 @@ export interface SastDiagnostics { degradedLanguages: SastLanguage[] /** Files whose scan hit the per-file budget and returned partial results. */ truncatedFiles: number + /** + * Files the per-file WALL-CLOCK ceiling cut short. Also counted in + * {@link truncatedFiles} — it is the same coverage loss as the node budget — + * so every consumer that already handles truncation handles this too. + */ + timeCappedFiles?: number + /** Paths of those files, so a report can NAME them and not only count them. + * Bounded; {@link timeCappedFiles} stays authoritative for the total. */ + timeCappedPaths?: string[] + /** + * Files never analyzed at all because the whole-pass wall-clock ceiling was + * reached first. Also counted in {@link filesSkipped}. + */ + timeSkippedFiles?: number + /** Paths of those files, bounded the same way. */ + timeSkippedPaths?: string[] /** The global finding cap was reached and later findings were omitted. */ findingsTruncated: boolean /** A process resource bound stopped a batch before every file was scanned. */ resourceLimitReached?: boolean - /** The isolated scan stopped before the enclosing job's wall-clock limit. */ + /** A wall-clock ceiling stopped the scan before every file was analyzed — + * either the engine's own pass ceiling or the isolated runner's job limit. */ budgetExceeded?: boolean /** A systemic child-runtime failure stopped later batches from starting. */ failureReason?: string diff --git a/packages/analyzer-engine/src/types.ts b/packages/analyzer-engine/src/types.ts index 064ef0b..92eb022 100644 --- a/packages/analyzer-engine/src/types.ts +++ b/packages/analyzer-engine/src/types.ts @@ -139,28 +139,53 @@ export interface AnalyzerRunResult { } const COMPLETION = Symbol('codetruss.analyzer-completion') -type FindingList = AnalyzerFinding[] & { [COMPLETION]?: Omit } +const WITHHELD = Symbol('codetruss.analyzer-withheld') +type FindingList = AnalyzerFinding[] & { + [COMPLETION]?: Omit + [WITHHELD]?: AnalyzerFinding[] +} + +/** + * Record findings an OUTPUT CAP removed from `findings`. + * + * An output cap is not a coverage loss — every candidate was still examined — + * but the difference matters to anything that compares two runs of the same + * analyzer. When a change RESOLVES findings, cap slots free up and previously + * capped findings in untouched files enter the reported list for the first + * time. A comparison that only ever saw reported lists calls those introduced, + * and a signed receipt then asserts that a change broke code its author never + * opened. The mirror image is just as wrong: a finding pushed BELOW the cap by + * newer ones reads as resolved when nothing fixed it. + * + * So the withheld findings ride along, unreported, purely as evidence of what + * this tree contained. They stay out of `AnalyzerRunResult` on purpose: the cap + * still governs what a pass reports and persists, and `analyzerWithheld()` is + * the only way to read them. + */ +function attachOutput( + findings: AnalyzerFinding[], + completion: Omit, + withheld: AnalyzerFinding[], +): AnalyzerFinding[] { + Object.defineProperty(findings, COMPLETION, { value: completion, enumerable: false }) + if (withheld.length) Object.defineProperty(findings, WITHHELD, { value: withheld, enumerable: false }) + return findings +} export function incompleteAnalyzerOutput( findings: AnalyzerFinding[], status: Omit, + withheld: AnalyzerFinding[] = [], ): AnalyzerFinding[] { - Object.defineProperty(findings, COMPLETION, { - value: { ...status, complete: false }, - enumerable: false, - }) - return findings + return attachOutput(findings, { ...status, complete: false }, withheld) } export function annotatedAnalyzerOutput( findings: AnalyzerFinding[], status: Omit, + withheld: AnalyzerFinding[] = [], ): AnalyzerFinding[] { - Object.defineProperty(findings, COMPLETION, { - value: { ...status, complete: true }, - enumerable: false, - }) - return findings + return attachOutput(findings, { ...status, complete: true }, withheld) } export function analyzerResult(output: AnalyzerFinding[]): AnalyzerRunResult { @@ -168,6 +193,17 @@ export function analyzerResult(output: AnalyzerFinding[]): AnalyzerRunResult { return { findings: output, complete: status?.complete ?? true, ...status } } +/** + * Findings this pass produced and then removed from its own output to respect a + * finding cap. Empty for a pass that reported everything it found. + * + * Read by baseline/final comparisons only. Nothing renders or persists these — + * see `attachOutput` for why they exist at all. + */ +export function analyzerWithheld(output: AnalyzerFinding[]): AnalyzerFinding[] { + return (output as FindingList)[WITHHELD] ?? [] +} + /** * Which passes OUTSIDE the registry run alongside the analyzers in this * execution. The registry is shared, but the passes around it are not: the diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index 6df6954..5a10a9f 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -5,6 +5,58 @@ checksums are published at