From 1c9946804ffb0a36d1da98d21122264c2dbbfd4b Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Mon, 17 Aug 2026 15:00:38 +0200 Subject: [PATCH 1/3] [ENG-3582] map: record which dependency APIs the app calls, separately from dataflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sink analysis answers "can request input reach a dangerous operation", which is only askable for the few API families it models — measured at ~3.6% of real advisories, identically across two independent 500-record TI samples. For the library tail the useful question is simpler and we could not answer it at all: is the package's API invoked? The motivating case is real and was verified before writing anything. sequelize CVE-2026-69240's own proof of concept is `Model.findOne({ where: { x: req.query.x } })`, and `findOne` is not a recognized sink operation — so the advisory's own demonstration shape produced NOTHING, and the vulnerability could only ever be reported as "the package is imported". It is now recorded, and a test asserts the same call still produces no sink, so the two layers cannot quietly become redundant. Discipline is the sink discipline: resolve the RECEIVER, never trust a method name. An untraceable receiver is not an invocation of a package, and `inferred` attribution does not qualify at all here — unlike a sink there is no argument role or dangerous operation to corroborate a guess. Every record carries its own evidence rather than leaving strength to be inferred from context: `attribution`, `resolution` (direct | factory | reexport) and the span. "Called on an imported binding" and "called on a value we followed through a factory" are different claims. PARTIAL BY CONSTRUCTION, and deliberately with no completeness flag. Parsing every file would raise recall but would not make absence safe: dynamic property access, computed import()/require(), reflection, unfollowed aliases, generated code and unparseable syntax all remain invisible. So `coverage.apiInventoryLimitations` enumerates the shapes instead — the list IS the statement, because a boolean is something a consumer could read as licence for "the vulnerable API is not called". Full parsing stays a performance question, not a completeness proof, and the measurements to decide it are now recorded per run: source bytes, wall time, peak RSS, invocations, and resolved vs unresolved call sites (the honest denominator — 32 invocations means nothing without the 37% trace rate beside it). Two corrections found while building it. `receiver` initially reported pg's method as `pool.query`, where `pool` is the local module's export name and not part of pg's API; it is now recorded only when the binding came straight from the package. And `Bindings` now records HOW a name reached its package, because the resolution had been inferred rather than known. Capability vocabulary goes through the versioned contract: invocationKinds and invocationResolutions added, 1.0.0 -> 1.1.0, which the version checker confirmed as an additive minor. --- capabilities.json | 12 +- scripts/emit-capabilities.mjs | 2 + src/cli.ts | 12 ++ src/map/bindings.ts | 28 +++- src/map/capabilities.ts | 20 ++- src/map/extract.ts | 44 ++++++ src/map/invocations.ts | 179 +++++++++++++++++++++++++ src/map/types.ts | 80 +++++++++++ tests/map/api-invocations.test.ts | 214 ++++++++++++++++++++++++++++++ 9 files changed, 585 insertions(+), 6 deletions(-) create mode 100644 src/map/invocations.ts create mode 100644 tests/map/api-invocations.test.ts diff --git a/capabilities.json b/capabilities.json index 5dbbed1..5615e6e 100644 --- a/capabilities.json +++ b/capabilities.json @@ -1,6 +1,6 @@ { "$comment": "GENERATED from src/map/capabilities.ts by scripts/emit-capabilities.mjs — do not edit by hand. The single versioned definition of what the input-flow map can describe, vendored by the reachability recipe schema/validator and by the server that binds coordinates into rules.", - "version": "1.0.0", + "version": "1.1.0", "sinkKinds": [ "db", "fs", @@ -58,5 +58,15 @@ "server", "route-param", "unknown" + ], + "invocationKinds": [ + "call", + "member", + "construct" + ], + "invocationResolutions": [ + "direct", + "factory", + "reexport" ] } diff --git a/scripts/emit-capabilities.mjs b/scripts/emit-capabilities.mjs index c75f441..31dee4c 100644 --- a/scripts/emit-capabilities.mjs +++ b/scripts/emit-capabilities.mjs @@ -47,6 +47,8 @@ const manifest = { autoPromotableConfidence: stringOf('AUTO_PROMOTABLE_CONFIDENCE'), attributions: arrayOf('ATTRIBUTIONS'), addressSpaces: arrayOf('ADDRESS_SPACES'), + invocationKinds: arrayOf('INVOCATION_KINDS'), + invocationResolutions: arrayOf('INVOCATION_RESOLUTIONS'), }; const out = join(root, 'capabilities.json'); diff --git a/src/cli.ts b/src/cli.ts index 5301a5a..ca9b620 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -235,6 +235,18 @@ async function runMap(args: ParsedArgs): Promise { `established at ("exact-local" and "transformed-local" are proven; "imported", "heuristic" and ` + `"unknown" are not).`, ); + const invoked = map.apiInvocations ?? []; + if (invoked.length > 0) { + const c = map.coverage as unknown as Record; + const total = (c.apiCallsResolved ?? 0) + (c.apiCallsUnresolved ?? 0); + const rate = total > 0 ? Math.round((100 * (c.apiCallsResolved ?? 0)) / total) : 0; + // The coverage rate is reported because the count alone has no scale: "31 dependency calls" means + // nothing without knowing how many calls we could not trace. + console.error( + `patchstack: ${invoked.length} dependency API call(s) resolved across ${new Set(invoked.map((i) => i.package)).size} package(s) ` + + `— ${rate}% of call sites traced. Positive evidence only: absence here never means an API is not called.`, + ); + } const imported = map.imports ?? []; if (imported.length > 0) { // The unmodelled count is the honest headline: it is how much of the dependency surface this map diff --git a/src/map/bindings.ts b/src/map/bindings.ts index b619580..abd2da1 100644 --- a/src/map/bindings.ts +++ b/src/map/bindings.ts @@ -15,6 +15,15 @@ export interface Bindings { resolve(name: string): string | undefined; /** For `import { saveOrder as write }`, maps the local name back to the EXPORTED name. */ exportNameOf(name: string): string | undefined; + /** + * HOW a name reached its package: `direct` from an import/require declaration, `factory` through a + * traced call chain (`const db = createClient(...)`, `const conn = pool.promise()`). + * + * Recorded rather than inferred because the API inventory reports it as evidence: "this call is on a + * value we followed through a factory" is a weaker claim than "this call is on an imported binding", + * and a consumer deciding what to act on needs to tell them apart. + */ + originOf(name: string): 'direct' | 'factory' | undefined; imports: Set; locals: Set; } @@ -23,8 +32,13 @@ export function buildModuleBindings(sf: any, ts: TsModule): Bindings { const declared = new Set(); // every name declared in this file const exportNames = new Map(); // local alias → exported name const imports = new Set(); + const origins = new Map(); - const record = (local: string, mod: string) => { nameToModule.set(local, mod); imports.add(mod); }; + const record = (local: string, mod: string, origin: 'direct' | 'factory' = 'direct') => { + nameToModule.set(local, mod); + imports.add(mod); + if (!origins.has(local)) origins.set(local, origin); + }; const declareBound = (nameNode: any) => { if (ts.isIdentifier(nameNode)) declared.add(nameNode.text); else if (ts.isObjectBindingPattern(nameNode) || ts.isArrayBindingPattern(nameNode)) { @@ -69,7 +83,7 @@ export function buildModuleBindings(sf: any, ts: TsModule): Bindings { const root = callee ? rootIdentifier(callee, ts) : undefined; if (root) { pending.push([decl.name.text, root]); - if (nameToModule.has(root)) record(decl.name.text, nameToModule.get(root)!); + if (nameToModule.has(root)) record(decl.name.text, nameToModule.get(root)!, 'factory'); } } } @@ -96,12 +110,18 @@ export function buildModuleBindings(sf: any, ts: TsModule): Bindings { for (let i = 0; i < 4; i++) { let changed = false; for (const [name, root] of pending) { - if (!nameToModule.has(name) && nameToModule.has(root)) { record(name, nameToModule.get(root)!); changed = true; } + if (!nameToModule.has(name) && nameToModule.has(root)) { record(name, nameToModule.get(root)!, 'factory'); changed = true; } } if (!changed) break; } const locals = new Set([...declared].filter((n) => !nameToModule.has(n))); - return { resolve: (name: string) => nameToModule.get(name), exportNameOf: (name: string) => exportNames.get(name), imports, locals }; + return { + resolve: (name: string) => nameToModule.get(name), + exportNameOf: (name: string) => exportNames.get(name), + originOf: (name: string) => origins.get(name), + imports, + locals, + }; } // Root identifier of what a function body returns (`return createClient(…)` → "createClient"), for diff --git a/src/map/capabilities.ts b/src/map/capabilities.ts index 275bfdb..6b8ba23 100644 --- a/src/map/capabilities.ts +++ b/src/map/capabilities.ts @@ -20,7 +20,7 @@ * member is breaking and bumps the MAJOR, because a consumer pinned to the old list will keep emitting a * value that can no longer match. */ -export const CAPABILITY_VERSION = '1.0.0'; +export const CAPABILITY_VERSION = '1.1.0'; /** Sink families the extractor recognizes. A dangerous OPERATION, not a package. */ export const SINK_KINDS = ['db', 'fs', 'http', 'exec', 'eval'] as const; @@ -57,6 +57,22 @@ export const PROVEN_CONFIDENCE_TIERS = ['exact-local', 'transformed-local'] as c /** The single tier eligible for automatic promotion to blocking (subject to the server's own gates). */ export const AUTO_PROMOTABLE_CONFIDENCE = 'exact-local'; +/** + * Shapes of a recognized call to a dependency's API, for the invocation inventory. Narrower than it looks: + * these are the three syntactic forms whose RECEIVER we can resolve, not a taxonomy of call syntax. + */ +export const INVOCATION_KINDS = ['call', 'member', 'construct'] as const; + +/** + * How the value being called was traced back to its package — evidence, not decoration. `direct` is an + * imported binding; `factory` followed a traced call chain (`const db = createClient(...)`); `reexport` + * crossed one hop into a local module that re-exports a dependency value. + * + * A consumer weighing whether to act on an invocation needs this: "called on an imported binding" and + * "called on something we followed two steps" are different strengths of the same claim. + */ +export const INVOCATION_RESOLUTIONS = ['direct', 'factory', 'reexport'] as const; + /** How a sink's package was established. Absent attribution is deliberately not a member: see `Sink`. */ export const ATTRIBUTIONS = ['import', 'global', 'inferred'] as const; @@ -74,6 +90,8 @@ export const CAPABILITY_MANIFEST = { autoPromotableConfidence: AUTO_PROMOTABLE_CONFIDENCE, attributions: ATTRIBUTIONS, addressSpaces: ADDRESS_SPACES, + invocationKinds: INVOCATION_KINDS, + invocationResolutions: INVOCATION_RESOLUTIONS, } as const; /** diff --git a/src/map/extract.ts b/src/map/extract.ts index e908965..9d07932 100644 --- a/src/map/extract.ts +++ b/src/map/extract.ts @@ -11,6 +11,7 @@ import { createModuleGraph } from './module-graph.js'; import { isProvenFlow } from './coordinates.js'; import { extractFromFile } from './entries.js'; import { collectFileImports, createImportInventory, readPathAliases, scanFileImports } from './imports.js'; +import { collectInvocations, createInvocationInventory } from './invocations.js'; // Framework-AGNOSTIC input-flow extractor. It doesn't gate on a specific stack — it walks any JS/TS // source and applies recognizer tables for (1) entry points, (2) inputs, (3) sinks, so it generalizes @@ -40,14 +41,20 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac const stats: WalkStats = { discovered: 0, unwalked: 0 }; const files = collectSources(cwd, boundary, { followOutside: options.followSymlinks }, [], new Set(), stats); const imports = createImportInventory(readPathAliases(cwd)); + const invocations = createInvocationInventory(); let parsed = 0; let preFiltered = 0; let importScanFailures = 0; + let sourceBytes = 0; + let callsResolved = 0; + let callsUnresolved = 0; + const startedAt = Date.now(); for (const file of files) { try { const text = readFileSync(file, 'utf8'); const relFile = relative(cwd, file); + sourceBytes += text.length; // Imports are collected from EVERY file, entry point or not: the data layer of an AI-built app // usually lives in a file with no handler in it, so a pre-filtered file is exactly where the // interesting dependency is imported. @@ -66,6 +73,12 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac imports.add(relFile, collectFileImports(sf, ts), true); const ctx = { file, owner: relFile, graph }; // The ctx reaches helper summaries too, so a same-file helper using an imported client resolves. + // The invocation inventory rides the parse we already did for sinks — no second pass, which is what + // makes it cheap enough to ship before deciding whether to parse more files. + const called = collectInvocations(sf, ts, bindings, ctx); + invocations.add(called.invocations); + callsResolved += called.invocations.length; + callsUnresolved += called.unresolved; const localSinks = collectLocalSinks(sf, ts, bindings, ctx); for (const ep of extractFromFile(sf, ts, localSinks, bindings, ctx)) { // A FILE-BASED route handler carries its URL path in its location, not in the code, so derive @@ -124,11 +137,26 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac notes.push(`The import inventory is INCOMPLETE: ${failed.length} file(s) could not be read, ${importScanFailures} could not be scanned, and ${stats.unwalked} path(s) could not be walked at all (unreadable directory, broken link, or a symlink leaving the project). A package may therefore be imported without appearing in \`imports\`. Do not read a package's absence as "not imported" while coverage.importsComplete is false.`); } + const invocationList = invocations.list(); + if (invocationList.length > 0 || parsed > 0) { + notes.push(`\`apiInvocations\` records ${invocationList.length} dependency API call(s) resolved from the ${parsed} parsed file(s). POSITIVE EVIDENCE ONLY: a package missing from it may still have its API called — see coverage.apiInventoryLimitations.`); + } + + let peakRssBytes; + try { + // Node-only and best-effort: the map runs in a build, but the runtime this file belongs to must stay + // edge-safe, so nothing here may assume `process`. + peakRssBytes = typeof process !== 'undefined' && typeof process.memoryUsage === 'function' + ? process.memoryUsage().rss + : undefined; + } catch { /* not available */ } + return { version: 3, framework: detectFramework(cwd), endpoints, imports: importList, + apiInvocations: invocationList, coverage: { adapter: 'agnostic-v1', filesDiscovered: stats.discovered, @@ -137,6 +165,22 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac filesSkipped: failed.length, pathsUnwalked: stats.unwalked, importsComplete, + apiInvocations: invocationList.length, + apiCallsResolved: callsResolved, + apiCallsUnresolved: callsUnresolved, + sourceBytes, + analysisMs: Date.now() - startedAt, + ...(peakRssBytes !== undefined ? { peakRssBytes } : {}), + // The list IS the partiality statement. There is deliberately no `apiInventoryComplete`: parsing + // every file would raise recall without removing any of these, so no parsing budget could make an + // absent invocation mean "not called". + apiInventoryLimitations: [ + 'only files carrying an entry-point signal are parsed, so a call in any other file is unseen', + 'a computed callee or property (`client[name]()`) cannot be resolved to an API', + 'a dynamic import()/require() with a non-literal specifier is not traced', + 'reflection, generated code, and syntax that fails to parse are invisible', + 'a receiver reached through more than one local hop, or through dependency injection, is not followed', + ], roots: ['.'], notes, }, diff --git a/src/map/invocations.ts b/src/map/invocations.ts new file mode 100644 index 0000000..e357947 --- /dev/null +++ b/src/map/invocations.ts @@ -0,0 +1,179 @@ +import type { ApiInvocation, InvocationResolution, TsModule } from './types.js'; +import { rootIdentifier, spanOf } from './ast.js'; +import { npmPackageOf, type Bindings } from './bindings.js'; +import type { ModuleGraph } from './sinks.js'; + +// Which dependency APIs the app actually CALLS — a different question from the sink analysis, and a much +// cheaper one. +// +// A sink answers "can request input reach a dangerous operation". That is only askable for the handful of +// API families the extractor models, which measured out at ~3.6% of real advisories. This answers "is this +// package's API invoked at all", which is askable for every package and is the whole answer for an advisory +// whose precondition is *calling* the vulnerable function rather than feeding it untrusted input. +// +// The discipline is the sink discipline, deliberately: a receiver we cannot trace to a dependency is not an +// invocation of that dependency, no matter how suggestive the method name. `inferred` attribution — the +// package guessed from another import in the same file — does not qualify at all here, because unlike a +// sink there is no second signal (an argument role, a dangerous operation) to corroborate it. +// +// PARTIAL BY CONSTRUCTION, and that is not a parsing budget problem. Parsing every file would raise recall, +// but dynamic property access (`client[method]()`), dynamic `import()`/`require()` with a computed +// specifier, reflection, aliases we do not follow, generated code and syntax we cannot parse all remain +// invisible. So absence here NEVER licenses "the vulnerable API is not called"; the document says so in +// `coverage.apiInventoryLimitations` rather than carrying a boolean a consumer could misread. + +/** Sites kept per distinct invocation, so a hot API cannot dominate the document. */ +const MAX_SITES = 3; + +export interface InvocationContext { + /** Absolute path of the file being analysed, for resolving relative re-exports. */ + file: string; + /** The same file, repo-relative — part of the record's identity, so it must not vary by machine. */ + owner: string; + graph: ModuleGraph; +} + +/** + * Collect resolved dependency calls from one parsed file. + * + * Returns the invocations plus a count of calls whose callee could NOT be traced to a package. That count + * is the honest denominator: it is what turns "we found 40 invocations" into "we resolved 40 of 300 calls", + * which is the number that decides whether more parsing is worth paying for. + */ +export function collectInvocations( + sf: any, + ts: TsModule, + bindings: Bindings, + ctx: InvocationContext, +): { invocations: ApiInvocation[]; unresolved: number } { + const found: ApiInvocation[] = []; + let unresolved = 0; + + const lineOf = (node: any): number | undefined => { + try { + return sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1; + } catch { + return undefined; + } + }; + + /** The package a local name traces to, and how — including one hop for a re-exported dependency value. */ + const traceRoot = (root: string): { pkg: string; specifier: string; resolution: InvocationResolution } | null => { + const specifier = bindings.resolve(root); + if (specifier === undefined) return null; + + const direct = npmPackageOf(specifier); + if (direct !== undefined) { + // `direct` vs `factory` is already recorded by the bindings; default to direct when unknown so a + // missing origin cannot silently upgrade a weaker claim. + return { pkg: direct, specifier, resolution: bindings.originOf(root) ?? 'direct' }; + } + + // A relative specifier: the value may still be a dependency re-exported from a local module + // (`export const db = createClient(...)` in lib/db.ts — the layout generated apps actually use). + const exportName = bindings.exportNameOf(root) ?? root; + const viaModule = ctx.graph.importedPackage(ctx.file, specifier, exportName); + if (viaModule !== undefined) { + return { pkg: viaModule, specifier, resolution: 'reexport' }; + } + + return null; + }; + + const push = ( + traced: { pkg: string; specifier: string; resolution: InvocationResolution }, + api: string, + receiver: string | undefined, + kind: ApiInvocation['kind'], + node: any, + ) => { + const span = spanOf(node); + found.push({ + package: traced.pkg, + specifiers: [traced.specifier], + api, + ...(receiver !== undefined ? { receiver } : {}), + symbol: receiver !== undefined ? `${receiver}.${api}` : api, + kind, + // Only ever `import`: a global has no package to correlate against, and an inferred package is not + // evidence about the value being called. + attribution: 'import', + resolution: traced.resolution, + callCount: 1, + sites: [{ file: ctx.owner, line: lineOf(node), start: span.start, end: span.end }], + }); + }; + + const visit = (node: any) => { + if (ts.isCallExpression(node) || ts.isNewExpression(node)) { + const callee = node.expression; + + if (ts.isIdentifier(callee)) { + // `merge(a, b)` / `new Pool()` — the callee itself is the imported binding. + const traced = traceRoot(callee.text); + if (traced) { + const api = bindings.exportNameOf(callee.text) ?? callee.text; + push(traced, api, undefined, ts.isNewExpression(node) ? 'construct' : 'call', node); + } else { + unresolved++; + } + } else if (ts.isPropertyAccessExpression(callee) && ts.isIdentifier(callee.name)) { + // `pool.query(sql)` / `helper.exec(cmd)` — resolve the RECEIVER, never the method name. A + // dangerous-looking method on an untraceable object is exactly the lookalike the corpus exists for. + const root = rootIdentifier(callee.expression, ts); + const traced = root !== undefined ? traceRoot(root) : null; + if (traced && root !== undefined) { + // A receiver name is only reported when the binding came STRAIGHT from the package. Anything + // else is the app's own name for a value: `pool` re-exported from `./lib`, or `Student` returned + // by `sequelize.define()`. Reporting those as part of the package's API would be inventing an + // API name — and the first version of this did exactly that, calling pg's method `pool.query`. + const receiver = traced.resolution === 'direct' ? bindings.exportNameOf(root) ?? root : undefined; + push(traced, callee.name.text, receiver, 'member', node); + } else { + unresolved++; + } + } else { + unresolved++; // computed callee, IIFE, dynamic import — see the limitations note + } + } + ts.forEachChild(node, visit); + }; + visit(sf); + + return { invocations: found, unresolved }; +} + +/** + * Aggregate invocations across files. + * + * Keyed by package + symbol + kind + resolution, so two call sites with DIFFERENT evidence stay separate + * entries rather than being merged under whichever was seen first — the strength of the claim is part of + * the record, not a detail to round off. + */ +export function createInvocationInventory() { + const byKey = new Map(); + + return { + add(invocations: ApiInvocation[]): void { + for (const invocation of invocations) { + const key = `${invocation.package}|${invocation.symbol}|${invocation.kind}|${invocation.resolution}`; + const existing = byKey.get(key); + if (existing === undefined) { + byKey.set(key, { ...invocation }); + continue; + } + existing.callCount += invocation.callCount; + for (const specifier of invocation.specifiers) { + if (!existing.specifiers.includes(specifier)) existing.specifiers.push(specifier); + } + if (existing.sites.length < MAX_SITES) existing.sites.push(...invocation.sites.slice(0, MAX_SITES - existing.sites.length)); + } + }, + + list(): ApiInvocation[] { + return [...byKey.entries()] + .sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)) + .map(([, invocation]) => ({ ...invocation, specifiers: [...invocation.specifiers].sort() })); + }, + }; +} diff --git a/src/map/types.ts b/src/map/types.ts index c19d151..386ecdc 100644 --- a/src/map/types.ts +++ b/src/map/types.ts @@ -5,6 +5,8 @@ import { ADDRESS_SPACES, ARGUMENT_ROLES, CANDIDATE_FAMILIES, + INVOCATION_KINDS, + INVOCATION_RESOLUTIONS, SINK_KINDS, } from './capabilities.js'; @@ -276,6 +278,24 @@ export interface Coverage { * subtree makes the project look *smaller*, so no other counter moves. */ pathsUnwalked?: number; + /** + * Metrics for the API-invocation pass, so the cost/benefit of parsing more files is decided with numbers + * rather than intuition. `apiCallsResolved / (apiCallsResolved + apiCallsUnresolved)` is the coverage + * rate — the share of call sites whose callee we could trace to a dependency. + */ + apiInvocations?: number; + apiCallsResolved?: number; + apiCallsUnresolved?: number; + sourceBytes?: number; + analysisMs?: number; + peakRssBytes?: number; + /** + * Invocation shapes this pass cannot see. Present whenever the inventory is — the list IS the statement + * that the inventory is partial, which is why there is no `apiInventoryComplete` boolean: parsing more + * files raises recall but none of these go away, so no amount of parsing could make absence here mean + * "the vulnerable API is not called". + */ + apiInventoryLimitations?: string[]; /** * Whether `SiteInputMap.imports` covers every discovered file — false when at least one file could not * be read or scanned, so a package may be imported without appearing there. @@ -321,6 +341,66 @@ export interface SiteInputMap { * Absent on maps produced before this shipped — treat missing as "unknown", never as "imports nothing". */ imports?: ImportedPackage[]; + /** + * Dependency APIs the app calls — see `ApiInvocation`. **Positive evidence only**: its absence for a + * package never means the package's API is not called (see `coverage.apiInventoryLimitations`). + * Collected from the files the extractor parses, which is not every file. + */ + apiInvocations?: ApiInvocation[]; +} + +export type InvocationKind = (typeof INVOCATION_KINDS)[number]; +export type InvocationResolution = (typeof INVOCATION_RESOLUTIONS)[number]; + +/** + * A dependency API the app CALLS, independent of whether request input reaches it. + * + * Why this is separate from `Sink`: a sink asserts "a dangerous operation that input can reach", which the + * extractor can only claim for the few API families it models. An invocation asserts the much simpler + * "this package's function is called here" — askable for any package, and the entire answer for an advisory + * whose precondition is calling the vulnerable function rather than feeding it untrusted input. Folding the + * two together would weaken what a sink means. + * + * Every record carries its own evidence (`attribution`, `resolution`, the span) rather than leaving a + * consumer to infer strength from context: "called on an imported binding" and "called on a value we + * followed through a factory" are different claims, and a server deciding what to act on needs both told + * apart and told plainly. + */ +export interface ApiInvocation { + /** npm package root, scope kept, or a `node:` builtin. */ + package: string; + /** Module specifiers as written that reached this API. */ + specifiers: string[]; + /** The called function or method, as the package exports/documents it — never a local alias. */ + api: string; + /** + * The receiver's name, and ONLY when the binding came straight from the package (`resolution: 'direct'`). + * Absent for a factory-derived value or a re-exported one, because those names are the app's own — a + * `pool` re-exported from `./lib`, a `Student` returned by `sequelize.define()` — and reporting them as + * part of the package's API would invent an API name. + * + * For a named import the local name IS the exported name; for a default or namespace import it is the + * app's alias, so treat this as a hint for a reader and match on `api` when correlating. + */ + receiver?: string; + /** `receiver.api` when a receiver is known, else `api` — the form advisories tend to name. */ + symbol: string; + kind: InvocationKind; + /** Always `import`: a global has no package, and an inferred package is not evidence about the value. */ + attribution: 'import'; + resolution: InvocationResolution; + /** How many call sites were seen; `sites` is capped, this is not. */ + callCount: number; + /** Where it is called — capped, with the span so the exact call can be pointed at. */ + sites: InvocationSite[]; +} + +/** A call site: the file and line for a human, the span for machine correlation. */ +export interface InvocationSite { + file: string; + line?: number; + start?: number; + end?: number; } /** One place a package is imported. */ diff --git a/tests/map/api-invocations.test.ts b/tests/map/api-invocations.test.ts new file mode 100644 index 0000000..ef35c8b --- /dev/null +++ b/tests/map/api-invocations.test.ts @@ -0,0 +1,214 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { buildInputMap } from '../../src/map/index.js'; +import type { ApiInvocation } from '../../src/map/types.js'; + +// The invocation inventory exists for the advisories the sink analysis cannot see. The motivating case is +// real: sequelize CVE-2026-69240's own proof of concept is `Model.findOne({ where: { x: req.query.x } })`, +// and `findOne` is not a recognized sink operation — so that advisory's demonstration shape produced +// nothing at all, and the vulnerability could only ever be reported as "the package is imported". +// +// The discipline is the sink discipline: resolve the RECEIVER, never trust a method name. So most of this +// file is about what must NOT be recorded. +let dir: string; +beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'ps-inv-')); + mkdirSync(join(dir, 'src'), { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ + dependencies: { express: '4', sequelize: '6', lodash: '4', pg: '8' }, + })); + // A client re-exported from a local module — the layout generated apps actually use. + writeFileSync(join(dir, 'src', 'lib.ts'), ` + import { Pool } from "pg"; + export const pool = new Pool(); + `); + writeFileSync(join(dir, 'src', 'server.ts'), ` + import express from "express"; + import { Sequelize } from "sequelize"; + import merge from "lodash/merge"; + import { pool } from "./lib"; + const sequelize = new Sequelize("oracle://x", { dialect: "oracle" }); + const Student = sequelize.define("Student", {}); + const app = express(); + app.get("/find", async (req, res) => { + const found = await Student.findOne({ where: { firstName: req.query.firstName } }); + res.json(merge({}, found)); + }); + app.post("/raw", async (req, res) => { + await pool.query(req.body.q); + await Student.findOne({ where: { id: req.body.id } }); + res.end(); + }); + app.post("/lookalike", (req, res) => { + // A dangerous-looking method on a receiver nothing can trace. + res.locals.db.query(req.body.sql); + res.end(); + }); + `); + // A local function whose name collides with a dependency export. + writeFileSync(join(dir, 'src', 'shadow.ts'), ` + import express from "express"; + function merge(a, b) { return { ...a, ...b } } + const app = express(); + app.post("/shadow", (req, res) => { res.json(merge(req.body, {})); }); + `); +}); +afterAll(() => rmSync(dir, { recursive: true, force: true })); + +const inventory = async (): Promise => { + const { map } = await buildInputMap(dir); + return map!.apiInvocations ?? []; +}; +const find = (list: ApiInvocation[], pkg: string, symbol: string) => + list.find((i) => i.package === pkg && i.symbol === symbol); + +describe('the invocation inventory answers what sinks cannot', () => { + it('records a model method the sink analysis produces no sink for', async () => { + const list = await inventory(); + const findOne = find(list, 'sequelize', 'findOne'); + + expect(findOne, 'the advisory PoC shape must be visible here even though it is not a sink').toBeDefined(); + expect(findOne!.kind).toBe('member'); + expect(findOne!.callCount, 'called from two handlers').toBe(2); + }); + + it('confirms that same call really produces no sink, so the two layers are not redundant', async () => { + const { map } = await buildInputMap(dir); + const sinks = map!.endpoints.flatMap((e) => e.sinks).filter((s) => s.package === 'sequelize' && s.op === 'findOne'); + + expect(sinks, 'if this ever becomes a sink, the motivating case for this layer changed').toEqual([]); + }); + + it('records a bare call from a named import under its exported name', async () => { + const list = await inventory(); + const merge = find(list, 'lodash', 'merge'); + + expect(merge).toBeDefined(); + expect(merge!.kind).toBe('call'); + expect(merge!.resolution).toBe('direct'); + expect(merge!.specifiers).toContain('lodash/merge'); + }); + + it('records a construction', async () => { + const list = await inventory(); + const sequelize = find(list, 'sequelize', 'Sequelize'); + + expect(sequelize).toBeDefined(); + expect(sequelize!.kind).toBe('construct'); + expect(sequelize!.resolution).toBe('direct'); + }); + + it('marks a value reached through a factory as such, rather than as a direct import', async () => { + const list = await inventory(); + + // `sequelize.define(...)` — the receiver came from `new Sequelize(...)`, not from the import itself. + expect(find(list, 'sequelize', 'define')!.resolution).toBe('factory'); + }); + + it('marks a dependency re-exported from a local module as a reexport', async () => { + const list = await inventory(); + const query = find(list, 'pg', 'query'); + + expect(query, 'a client imported from ./lib still belongs to its package').toBeDefined(); + expect(query!.resolution).toBe('reexport'); + }); +}); + +describe('a method name is not an API', () => { + it('does not record a call on a receiver it cannot trace', async () => { + const list = await inventory(); + // `res.locals.db.query(...)` — `pg` is imported elsewhere in the project, and this is still not it. + const fromUntraceable = list.filter((i) => i.symbol === 'query' && i.resolution !== 'reexport'); + + expect(fromUntraceable, 'an untraceable receiver must not be attributed to a package').toEqual([]); + }); + + it('does not record a local function that shares a dependency export name', async () => { + const list = await inventory(); + const merge = find(list, 'lodash', 'merge')!; + + // One call site, in server.ts. The `merge` in shadow.ts is app code that happens to share the name. + expect(merge.callCount).toBe(1); + expect(merge.sites.every((s) => s.file === 'src/server.ts')).toBe(true); + }); + + it('never claims an attribution other than a resolved import', async () => { + const list = await inventory(); + + expect(list.every((i) => i.attribution === 'import')).toBe(true); + }); + + it('omits the receiver when the receiver is the app’s own name for a value', async () => { + const list = await inventory(); + // `Student` is what this app called the model; it is not part of sequelize's API, so reporting it as + // one would be inventing an API name. + expect(find(list, 'sequelize', 'findOne')!.receiver).toBeUndefined(); + }); +}); + +describe('the inventory says plainly that it is partial', () => { + it('carries the limitations, and no completeness flag of any kind', async () => { + const { map } = await buildInputMap(dir); + const coverage = map!.coverage as Record; + + expect(Array.isArray(coverage.apiInventoryLimitations)).toBe(true); + expect((coverage.apiInventoryLimitations as string[]).length).toBeGreaterThan(3); + // No boolean a consumer could read as licence for "the vulnerable API is not called". Parsing more + // files would raise recall without removing a single limitation, so completeness is not on offer. + expect(coverage.apiInventoryComplete).toBeUndefined(); + }); + + it('names the shapes that make absence meaningless', async () => { + const { map } = await buildInputMap(dir); + const limits = ((map!.coverage as Record).apiInventoryLimitations as string[]).join(' '); + + expect(limits).toMatch(/computed callee/); + expect(limits).toMatch(/dynamic import/); + expect(limits).toMatch(/entry-point signal/); + }); + + it('warns in the notes that this is positive evidence only', async () => { + const { map } = await buildInputMap(dir); + + expect(map!.coverage.notes.some((n) => n.includes('POSITIVE EVIDENCE ONLY'))).toBe(true); + }); +}); + +describe('the measurements needed to decide whether to parse more', () => { + it('reports the cost and the coverage rate', async () => { + const { map } = await buildInputMap(dir); + const c = map!.coverage as Record; + + expect(c.apiInvocations).toBeGreaterThan(0); + expect(c.apiCallsResolved).toBeGreaterThan(0); + // Unresolved calls are the honest denominator: without them "we found N invocations" has no scale. + expect(c.apiCallsUnresolved).toBeGreaterThan(0); + expect(c.sourceBytes).toBeGreaterThan(0); + expect(c.analysisMs).toBeGreaterThanOrEqual(0); + expect(c.filesParsed).toBeGreaterThan(0); + }); + + it('counts every call site even though the sites list is capped', async () => { + const d = mkdtempSync(join(tmpdir(), 'ps-inv-cap-')); + mkdirSync(join(d, 'src'), { recursive: true }); + writeFileSync(join(d, 'package.json'), JSON.stringify({ dependencies: { express: '4', lodash: '4' } })); + writeFileSync(join(d, 'src', 'many.ts'), ` + import express from "express"; + import merge from "lodash/merge"; + const app = express(); + app.post("/a", (req, res) => { res.json(merge({}, req.body)); }); + app.post("/b", (req, res) => { res.json(merge({}, req.body)); }); + app.post("/c", (req, res) => { res.json(merge({}, req.body)); }); + app.post("/d", (req, res) => { res.json(merge({}, req.body)); }); + app.post("/e", (req, res) => { res.json(merge({}, req.body)); }); + `); + const { map } = await buildInputMap(d); + const merge = (map!.apiInvocations ?? []).find((i) => i.symbol === 'merge')!; + + expect(merge.callCount).toBe(5); + expect(merge.sites.length).toBeLessThanOrEqual(3); + rmSync(d, { recursive: true, force: true }); + }); +}); From 735707f7d8c0e4c88a3c3519e6449e4fcfc76e42 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Mon, 17 Aug 2026 15:09:34 +0200 Subject: [PATCH 2/3] map: measure resolver quality, not what fraction of an app's calls are dependency calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The metric was wrong in a way that would have misinformed the decision it exists for. `apiCallsUnresolved` counted every untraced call expression, so ordinary local helpers and application methods landed in it. `resolved / (resolved + unresolved)` was therefore a property of the APP — how much of its code calls dependencies — not of the resolver. Worse, it would have gone DOWN as parsing widened, because more parsing finds more local calls: exactly backwards for a number meant to justify parsing more. Four buckets now, and they sum to the total (asserted, so none can silently absorb calls): callsTotal every call/new expression — workload scale callsDependency traced to a package callsLocal a known local binding or an enclosing parameter — correctly excluded callsAmbiguous a receiver we could not classify, or a computed/dynamic callee Resolver quality is `callsDependency / (callsDependency + callsAmbiguous)`. `callsLocal` is in neither term: declining to attribute `res.json()` to a package is a correct answer, not a miss. The line between local and ambiguous took a correction of its own. A handler parameter is local, so `res.json()` is local — but `res.locals.db.query()` is NOT, even though its root is that same parameter: the value being called is whatever was stashed on `res.locals`, and a database client is precisely what apps put there. Only a method called DIRECTLY on a local binding counts as local; anything deeper is ambiguous, which matches how the sink analysis already treats those receivers. On the real app the corrected figure is 52% of dependency-candidate receivers resolved, against the 37% the old rate reported — and the old number would have looked worse the more we parsed. --- src/cli.ts | 14 +++++--- src/map/extract.ts | 15 +++++---- src/map/invocations.ts | 53 ++++++++++++++++++++++++------- src/map/types.ts | 22 ++++++++++--- tests/map/api-invocations.test.ts | 53 ++++++++++++++++++++++++++++--- 5 files changed, 127 insertions(+), 30 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index ca9b620..9ff722f 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -238,13 +238,17 @@ async function runMap(args: ParsedArgs): Promise { const invoked = map.apiInvocations ?? []; if (invoked.length > 0) { const c = map.coverage as unknown as Record; - const total = (c.apiCallsResolved ?? 0) + (c.apiCallsUnresolved ?? 0); - const rate = total > 0 ? Math.round((100 * (c.apiCallsResolved ?? 0)) / total) : 0; - // The coverage rate is reported because the count alone has no scale: "31 dependency calls" means - // nothing without knowing how many calls we could not trace. + const dependency = c.callsDependency ?? 0; + const ambiguous = c.callsAmbiguous ?? 0; + // Resolver quality, NOT "share of all calls": local helpers are excluded from both terms, because + // declining to attribute `res.json()` to a package is a correct answer rather than a miss. + const denominator = dependency + ambiguous; + const quality = denominator > 0 ? Math.round((100 * dependency) / denominator) : 100; console.error( `patchstack: ${invoked.length} dependency API call(s) resolved across ${new Set(invoked.map((i) => i.package)).size} package(s) ` + - `— ${rate}% of call sites traced. Positive evidence only: absence here never means an API is not called.`, + `from ${c.callsTotal ?? 0} call site(s) — ${quality}% of dependency-candidate receivers resolved ` + + `(${c.callsLocal ?? 0} local, ${ambiguous} ambiguous). Positive evidence only: absence here never ` + + `means an API is not called.`, ); } const imported = map.imports ?? []; diff --git a/src/map/extract.ts b/src/map/extract.ts index 9d07932..6c160f6 100644 --- a/src/map/extract.ts +++ b/src/map/extract.ts @@ -46,8 +46,7 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac let preFiltered = 0; let importScanFailures = 0; let sourceBytes = 0; - let callsResolved = 0; - let callsUnresolved = 0; + const calls = { total: 0, dependency: 0, local: 0, ambiguous: 0 }; const startedAt = Date.now(); for (const file of files) { @@ -77,8 +76,10 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac // makes it cheap enough to ship before deciding whether to parse more files. const called = collectInvocations(sf, ts, bindings, ctx); invocations.add(called.invocations); - callsResolved += called.invocations.length; - callsUnresolved += called.unresolved; + calls.total += called.counts.total; + calls.dependency += called.counts.dependency; + calls.local += called.counts.local; + calls.ambiguous += called.counts.ambiguous; const localSinks = collectLocalSinks(sf, ts, bindings, ctx); for (const ep of extractFromFile(sf, ts, localSinks, bindings, ctx)) { // A FILE-BASED route handler carries its URL path in its location, not in the code, so derive @@ -166,8 +167,10 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac pathsUnwalked: stats.unwalked, importsComplete, apiInvocations: invocationList.length, - apiCallsResolved: callsResolved, - apiCallsUnresolved: callsUnresolved, + callsTotal: calls.total, + callsDependency: calls.dependency, + callsLocal: calls.local, + callsAmbiguous: calls.ambiguous, sourceBytes, analysisMs: Date.now() - startedAt, ...(peakRssBytes !== undefined ? { peakRssBytes } : {}), diff --git a/src/map/invocations.ts b/src/map/invocations.ts index e357947..db770c7 100644 --- a/src/map/invocations.ts +++ b/src/map/invocations.ts @@ -1,5 +1,5 @@ import type { ApiInvocation, InvocationResolution, TsModule } from './types.js'; -import { rootIdentifier, spanOf } from './ast.js'; +import { isShadowedByEnclosingBinding, rootIdentifier, spanOf } from './ast.js'; import { npmPackageOf, type Bindings } from './bindings.js'; import type { ModuleGraph } from './sinks.js'; @@ -34,20 +34,39 @@ export interface InvocationContext { } /** - * Collect resolved dependency calls from one parsed file. + * How the call expressions in a file classified. Four buckets rather than resolved-vs-not, because + * "resolved / everything else" measures the APP, not the resolver: a codebase full of local helpers would + * score badly through no fault of ours, and widening the parse would *lower* the number by finding more + * local calls — precisely backwards for a metric meant to justify widening the parse. * - * Returns the invocations plus a count of calls whose callee could NOT be traced to a package. That count - * is the honest denominator: it is what turns "we found 40 invocations" into "we resolved 40 of 300 calls", - * which is the number that decides whether more parsing is worth paying for. + * total every call/new expression seen — workload scale + * dependency traced to a package: what the inventory records + * local the callee is a known local binding or an enclosing parameter — correctly not a dependency + * ambiguous a receiver we could not classify either way, or a computed/dynamic callee + * + * Resolver quality is `dependency / (dependency + ambiguous)`. `local` belongs in neither term: excluding a + * local helper is a correct answer, not a miss. */ +export interface CallCounts { + total: number; + dependency: number; + local: number; + ambiguous: number; +} + +/** Collect resolved dependency calls from one parsed file, with the call classification alongside. */ export function collectInvocations( sf: any, ts: TsModule, bindings: Bindings, ctx: InvocationContext, -): { invocations: ApiInvocation[]; unresolved: number } { +): { invocations: ApiInvocation[]; counts: CallCounts } { const found: ApiInvocation[] = []; - let unresolved = 0; + const counts: CallCounts = { total: 0, dependency: 0, local: 0, ambiguous: 0 }; + + /** A name we can positively account for as app-local: declared in-file, or an enclosing parameter. */ + const isLocalName = (name: string, node: any): boolean => + bindings.locals.has(name) || isShadowedByEnclosingBinding(node, name, ts); const lineOf = (node: any): number | undefined => { try { @@ -107,6 +126,7 @@ export function collectInvocations( const visit = (node: any) => { if (ts.isCallExpression(node) || ts.isNewExpression(node)) { const callee = node.expression; + counts.total++; if (ts.isIdentifier(callee)) { // `merge(a, b)` / `new Pool()` — the callee itself is the imported binding. @@ -114,8 +134,11 @@ export function collectInvocations( if (traced) { const api = bindings.exportNameOf(callee.text) ?? callee.text; push(traced, api, undefined, ts.isNewExpression(node) ? 'construct' : 'call', node); + counts.dependency++; + } else if (isLocalName(callee.text, node)) { + counts.local++; } else { - unresolved++; + counts.ambiguous++; } } else if (ts.isPropertyAccessExpression(callee) && ts.isIdentifier(callee.name)) { // `pool.query(sql)` / `helper.exec(cmd)` — resolve the RECEIVER, never the method name. A @@ -123,24 +146,32 @@ export function collectInvocations( const root = rootIdentifier(callee.expression, ts); const traced = root !== undefined ? traceRoot(root) : null; if (traced && root !== undefined) { + counts.dependency++; // A receiver name is only reported when the binding came STRAIGHT from the package. Anything // else is the app's own name for a value: `pool` re-exported from `./lib`, or `Student` returned // by `sequelize.define()`. Reporting those as part of the package's API would be inventing an // API name — and the first version of this did exactly that, calling pg's method `pool.query`. const receiver = traced.resolution === 'direct' ? bindings.exportNameOf(root) ?? root : undefined; push(traced, callee.name.text, receiver, 'member', node); + } else if (root !== undefined && ts.isIdentifier(callee.expression) && isLocalName(root, node)) { + // A method called DIRECTLY on a local binding or a handler parameter — `res.json()`, + // `shape.build()`. Declining to attribute that to a package is a correct answer. + counts.local++; } else { - unresolved++; + // Anything deeper is ambiguous even when the ROOT is local: in `res.locals.db.query()` the value + // being called is whatever was stashed on `res.locals`, not the parameter itself, and a database + // client is exactly what apps put there. Filing it as "local" would hide a real miss. + counts.ambiguous++; } } else { - unresolved++; // computed callee, IIFE, dynamic import — see the limitations note + counts.ambiguous++; // computed callee, IIFE, dynamic import — see the limitations note } } ts.forEachChild(node, visit); }; visit(sf); - return { invocations: found, unresolved }; + return { invocations: found, counts }; } /** diff --git a/src/map/types.ts b/src/map/types.ts index 386ecdc..42d0625 100644 --- a/src/map/types.ts +++ b/src/map/types.ts @@ -280,12 +280,26 @@ export interface Coverage { pathsUnwalked?: number; /** * Metrics for the API-invocation pass, so the cost/benefit of parsing more files is decided with numbers - * rather than intuition. `apiCallsResolved / (apiCallsResolved + apiCallsUnresolved)` is the coverage - * rate — the share of call sites whose callee we could trace to a dependency. + * rather than intuition. + * + * Four buckets, not resolved-vs-not, because a two-way split measures the APP rather than the resolver: + * a codebase full of local helpers would score badly through no fault of the analysis, and widening the + * parse would LOWER such a rate by finding more local calls — backwards for a number meant to justify + * widening the parse. + * + * callsTotal every call/new expression seen — workload scale + * callsDependency traced to a package: what `apiInvocations` records + * callsLocal a known local binding or an enclosing parameter (`res.json()`) — correctly excluded + * callsAmbiguous a receiver that could not be classified either way, or a computed/dynamic callee + * + * **Resolver quality is `callsDependency / (callsDependency + callsAmbiguous)`.** `callsLocal` belongs in + * neither term: excluding a local helper is a correct answer, not a miss. */ apiInvocations?: number; - apiCallsResolved?: number; - apiCallsUnresolved?: number; + callsTotal?: number; + callsDependency?: number; + callsLocal?: number; + callsAmbiguous?: number; sourceBytes?: number; analysisMs?: number; peakRssBytes?: number; diff --git a/tests/map/api-invocations.test.ts b/tests/map/api-invocations.test.ts index ef35c8b..165a826 100644 --- a/tests/map/api-invocations.test.ts +++ b/tests/map/api-invocations.test.ts @@ -177,19 +177,64 @@ describe('the inventory says plainly that it is partial', () => { }); describe('the measurements needed to decide whether to parse more', () => { - it('reports the cost and the coverage rate', async () => { + it('reports the cost and the four call buckets', async () => { const { map } = await buildInputMap(dir); const c = map!.coverage as Record; expect(c.apiInvocations).toBeGreaterThan(0); - expect(c.apiCallsResolved).toBeGreaterThan(0); - // Unresolved calls are the honest denominator: without them "we found N invocations" has no scale. - expect(c.apiCallsUnresolved).toBeGreaterThan(0); + expect(c.callsDependency).toBeGreaterThan(0); expect(c.sourceBytes).toBeGreaterThan(0); expect(c.analysisMs).toBeGreaterThanOrEqual(0); expect(c.filesParsed).toBeGreaterThan(0); }); + it('accounts for every call expression exactly once', async () => { + const { map } = await buildInputMap(dir); + const c = map!.coverage as Record; + + // If the buckets do not sum, one of them is silently absorbing calls and every ratio built on them is + // wrong in an unknown direction. + expect(c.callsDependency + c.callsLocal + c.callsAmbiguous).toBe(c.callsTotal); + }); + + it('counts a local helper as local, not as a failure to resolve', async () => { + // The measurement error this replaced: counting local calls as "unresolved" made the rate a property + // of the app rather than of the resolver — and widening the parse would have LOWERED it by finding more + // local calls, which is backwards for a number meant to justify widening the parse. + const d = mkdtempSync(join(tmpdir(), 'ps-inv-local-')); + mkdirSync(join(d, 'src'), { recursive: true }); + writeFileSync(join(d, 'package.json'), JSON.stringify({ dependencies: { express: '4' } })); + writeFileSync(join(d, 'src', 'local.ts'), ` + import express from "express"; + function helper(x) { return x + 1 } + const shape = { build: (x) => x }; + const app = express(); + app.post("/x", (req, res) => { + helper(req.body.a); + shape.build(req.body.b); + res.json({ ok: true }); // a handler PARAMETER, not a dependency + }); + `); + const { map } = await buildInputMap(d); + const c = map!.coverage as Record; + + // helper(), shape.build(), res.json() — three local calls, none of them ambiguous. + expect(c.callsLocal).toBeGreaterThanOrEqual(3); + expect(c.callsAmbiguous).toBe(0); + // And resolver quality is unaffected by how many local helpers the app happens to have. + expect(c.callsDependency / (c.callsDependency + c.callsAmbiguous)).toBe(1); + rmSync(d, { recursive: true, force: true }); + }); + + it('counts an untraceable receiver as ambiguous, because it is a real miss', async () => { + const { map } = await buildInputMap(dir); + const c = map!.coverage as Record; + + // `res.locals.db.query(...)` in the fixture: we cannot say whether that is a dependency, so it counts + // against resolver quality rather than being quietly filed as local. + expect(c.callsAmbiguous).toBeGreaterThan(0); + }); + it('counts every call site even though the sites list is capped', async () => { const d = mkdtempSync(join(tmpdir(), 'ps-inv-cap-')); mkdirSync(join(d, 'src'), { recursive: true }); From b12b5b231b4c1048c3b2980ea7b03b1a7697d019 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Mon, 17 Aug 2026 15:25:46 +0200 Subject: [PATCH 3/3] map: name the memory readings for what they measure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `peakRssBytes` held `memoryUsage().rss` — the resident size at the moment extraction finished, after the walk's garbage may already have been collected. That is a point-in-time reading, and calling it a peak would have overstated it in exactly the place it would be used: deciding whether parsing more files is affordable. Split into two honestly-labelled numbers rather than just renaming, because the peak is the one the decision needs: rssBytes resident size when extraction finished — point-in-time, not a high-water mark peakRssBytes a real high-water mark from the OS (`resourceUsage().maxRSS`), but PROCESS-wide: it includes loading the TypeScript compiler, so it bounds the cost of running `map` from above rather than attributing a peak to extraction alone Both are documented with those caveats, and a test pins the ordering invariant. Absent where the platform does not report them — nothing here may assume `process`, since this file's runtime has to stay edge-safe. --- src/map/extract.ts | 23 ++++++++++++++++++----- src/map/types.ts | 11 +++++++++++ tests/map/api-invocations.test.ts | 13 +++++++++++++ 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/map/extract.ts b/src/map/extract.ts index 6c160f6..9ac3b7d 100644 --- a/src/map/extract.ts +++ b/src/map/extract.ts @@ -143,13 +143,25 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac notes.push(`\`apiInvocations\` records ${invocationList.length} dependency API call(s) resolved from the ${parsed} parsed file(s). POSITIVE EVIDENCE ONLY: a package missing from it may still have its API called — see coverage.apiInventoryLimitations.`); } + // Node-only and best-effort: the map runs in a build, but the runtime this file belongs to must stay + // edge-safe, so nothing here may assume `process`. + // + // Two numbers, because the obvious one is not the one a performance decision needs. `memoryUsage().rss` + // is the resident size AT THIS MOMENT — after extraction, with the walk's garbage possibly already + // collected — so calling it a peak would overstate what it measures. `resourceUsage().maxRSS` is a real + // high-water mark, but for the whole process (it includes loading TypeScript), so it is an upper bound on + // the cost of running `map` rather than extraction's own peak. Both are reported and both are labelled + // for what they are. + let rssBytes; let peakRssBytes; try { - // Node-only and best-effort: the map runs in a build, but the runtime this file belongs to must stay - // edge-safe, so nothing here may assume `process`. - peakRssBytes = typeof process !== 'undefined' && typeof process.memoryUsage === 'function' - ? process.memoryUsage().rss - : undefined; + if (typeof process !== 'undefined') { + if (typeof process.memoryUsage === 'function') rssBytes = process.memoryUsage().rss; + if (typeof process.resourceUsage === 'function') { + const maxRssKb = process.resourceUsage().maxRSS; // kilobytes, per Node's docs + if (typeof maxRssKb === 'number' && maxRssKb > 0) peakRssBytes = maxRssKb * 1024; + } + } } catch { /* not available */ } return { @@ -173,6 +185,7 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac callsAmbiguous: calls.ambiguous, sourceBytes, analysisMs: Date.now() - startedAt, + ...(rssBytes !== undefined ? { rssBytes } : {}), ...(peakRssBytes !== undefined ? { peakRssBytes } : {}), // The list IS the partiality statement. There is deliberately no `apiInventoryComplete`: parsing // every file would raise recall without removing any of these, so no parsing budget could make an diff --git a/src/map/types.ts b/src/map/types.ts index 42d0625..fe3b5c6 100644 --- a/src/map/types.ts +++ b/src/map/types.ts @@ -302,6 +302,17 @@ export interface Coverage { callsAmbiguous?: number; sourceBytes?: number; analysisMs?: number; + /** + * Resident set size when extraction finished — a point-in-time reading, NOT a peak. Named for what it is: + * the walk's garbage may already have been collected by the time it is taken, so treating it as a + * high-water mark would overstate it. + */ + rssBytes?: number; + /** + * A real high-water mark, from the OS (`resourceUsage().maxRSS`), but for the whole PROCESS — it includes + * loading the TypeScript compiler. So it bounds the cost of running `map` from above rather than + * attributing a peak to extraction alone. Absent where the platform does not report it. + */ peakRssBytes?: number; /** * Invocation shapes this pass cannot see. Present whenever the inventory is — the list IS the statement diff --git a/tests/map/api-invocations.test.ts b/tests/map/api-invocations.test.ts index 165a826..3e43272 100644 --- a/tests/map/api-invocations.test.ts +++ b/tests/map/api-invocations.test.ts @@ -188,6 +188,19 @@ describe('the measurements needed to decide whether to parse more', () => { expect(c.filesParsed).toBeGreaterThan(0); }); + it('labels the two memory readings for what they actually are', async () => { + const { map } = await buildInputMap(dir); + const c = map!.coverage as Record; + + // `rssBytes` is a point-in-time reading taken after extraction; calling it a peak would overstate it, + // since the walk's garbage may already be collected. `peakRssBytes` is a real high-water mark from the + // OS, but process-wide — it includes loading the TypeScript compiler — so it bounds the cost from above. + if (c.rssBytes !== undefined && c.peakRssBytes !== undefined) { + expect(c.peakRssBytes).toBeGreaterThanOrEqual(c.rssBytes); + } + expect(c.rssBytes ?? 1).toBeGreaterThan(0); + }); + it('accounts for every call expression exactly once', async () => { const { map } = await buildInputMap(dir); const c = map!.coverage as Record;