From 99feb50d71cd4c1bb9c0b453eed2d61e11bc0c89 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Tue, 18 Aug 2026 17:29:51 +0200 Subject: [PATCH 1/5] Add reachability ladder fixture apps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One app per reachability rung, asserting what the map SAYS about a dependency — which is what a consumer grades into a verdict, and a different question from whether a flow compiles to a rule. A package can be plainly reachable and still produce no rule. Each rung has its own way of being wrong: reachable must not read as a bare import, api-called must not be promoted, imported must not be demoted to "not imported", not-a-code-question has no consumer artifact to gate it, and unknown must DECLINE. The last is load-bearing — every other rung is a positive claim that shows up as a bad rule someone notices, while a wrong unknown is a confident negative that makes a real finding disappear silently. The suite is layered so a failure localises: positive controls that the source was parsed at all (an app landing on `imported` because nothing parsed would otherwise pass for the wrong reason), then imports, invocations, absent invocations, and flows. One case fails deliberately and is left failing: a local function is named as a dependency API, and the dependency API called inside the module it lives in is not recorded. This branch is the regression harness for that fix, and is NOT ready to merge while the assertion is red. Co-Authored-By: Claude Opus 5 (1M context) --- tests/map/ladder-cases.ts | 243 ++++++++++++++++++++++++++++++++++++++ tests/map/ladder.test.ts | 172 +++++++++++++++++++++++++++ 2 files changed, 415 insertions(+) create mode 100644 tests/map/ladder-cases.ts create mode 100644 tests/map/ladder.test.ts diff --git a/tests/map/ladder-cases.ts b/tests/map/ladder-cases.ts new file mode 100644 index 0000000..f220dc4 --- /dev/null +++ b/tests/map/ladder-cases.ts @@ -0,0 +1,243 @@ +// Apps whose reachability evidence should land on a known rung. +// +// The corpus in `corpus-cases.ts` measures whether a flow compiles to a rule. These cases measure +// something the map has never been tested on end to end: what the map SAYS about a dependency, which +// is what a consumer grades into a verdict. The two are different questions — a package can be +// plainly reachable and still produce no rule, and an app can produce no evidence at all. +// +// One case per rung, because each rung has a distinct way of being wrong: +// +// reachable untrusted input reaches the sink — must not read as a bare import +// api-called the API is invoked with no untrusted input — must not be promoted +// imported imported and never called — must not be demoted to "not imported" +// not-a-code-question the package IS the deployed app — no consumer artifact gates it +// unknown the call is invisible to static analysis — the map must DECLINE +// +// `unknown` is the load-bearing one. Every other rung is a positive claim, and a wrong positive shows +// up as a bad rule someone notices. A wrong `unknown` is a confident negative: the map reports "not +// imported" for code it simply cannot see, a real finding disappears, and nothing anywhere raises. + +export interface LadderCase { + /** + * Stable identity, referenced from the assertions and from the platform-side ladder tests. Renaming + * one breaks that reference loudly, which is the point. + */ + id: string; + /** The rung this app's evidence should support. */ + rung: 'reachable' | 'api-called' | 'imported' | 'not-a-code-question' | 'unknown'; + /** The fixture advisory this app pairs with, by CVE. */ + cve: string; + /** The dependency under test. */ + pkg: string; + name: string; + packageJson: Record; + files: Record; + /** + * What the map must show for this case to mean what it claims. + * + * `imports` and `invocations` are positive controls first and assertions second: an app that lands + * on `imported` because the map failed to parse it at all would otherwise pass for the wrong reason. + */ + expect: { + /** Package names that must appear in the import inventory. */ + imports: string[]; + /** `package.symbol` entries that must appear in the invocation inventory. */ + invocations: string[]; + /** `package.symbol` entries that must NOT appear — the promotions each rung must resist. */ + absentInvocations?: string[]; + /** A flow from request input to a sink must exist (true) or must not (false). */ + provenFlow: boolean; + /** Substrings that must appear in `coverage.apiInventoryLimitations`. */ + limitations?: RegExp[]; + }; +} + +export const LADDER_CASES: LadderCase[] = [ + { + id: 'ladder/reachable', + rung: 'reachable', + cve: 'CVE-2019-10752', + pkg: 'sequelize', + name: 'request input flows into a sequelize query', + packageJson: { dependencies: { express: '4', sequelize: '4.44.0' } }, + files: { + 'src/server.js': ` + const express = require("express"); + const { Sequelize } = require("sequelize"); + const db = new Sequelize("postgres://localhost/app"); + const app = express(); + + app.get("/search", async (req, res) => { + // A modelled sink (db) reached by request input. Deliberately a SQL-injection advisory + // rather than a prototype-pollution one: the map models db/fs/http/exec/eval sinks, so a + // deep-merge flow cannot compile and would make this case assert a capability that does + // not exist. + const rows = await db.query("SELECT * FROM items WHERE name = '" + req.query.name + "'"); + res.json(rows); + }); + + module.exports = app; + `, + }, + expect: { + imports: ['sequelize'], + invocations: ['sequelize.query'], + provenFlow: true, + }, + }, + + { + id: 'ladder/api-called', + rung: 'api-called', + cve: 'CVE-2022-46175', + pkg: 'json5', + name: 'json5 parse is called, but never on request input', + packageJson: { dependencies: { express: '4', json5: '2.2.1' } }, + files: { + 'src/config.js': ` + const JSON5 = require("json5"); + const { readFileSync } = require("node:fs"); + + // Called on a file this app ships. The API is invoked — that is real evidence — but no + // untrusted input reaches it, so grading this as \`reachable\` would overclaim. + function loadConfig() { + return JSON5.parse(readFileSync("./config.json5", "utf8")); + } + + module.exports = { loadConfig }; + `, + 'src/server.js': ` + const express = require("express"); + const { loadConfig } = require("./config"); + const app = express(); + + app.get("/config", (req, res) => { + // The request never reaches parse: the response is derived from the shipped file. + res.json({ theme: loadConfig().theme }); + }); + + module.exports = app; + `, + }, + expect: { + imports: ['json5'], + invocations: ['json5.parse'], + // No flow from request input to the parse call. If one appears, the case has stopped testing + // `api-called` and silently become a second `reachable` case. + provenFlow: false, + }, + }, + + { + id: 'ladder/imported', + rung: 'imported', + cve: 'CVE-2022-24999', + pkg: 'qs', + name: 'qs is imported and never called in traced code', + packageJson: { dependencies: { express: '4', qs: '6.10.1' } }, + files: { + 'src/server.js': ` + const express = require("express"); + // Imported and unused here — the framework parses query strings internally, which is how a + // consumer normally has this dependency without ever calling it. + const qs = require("qs"); + const app = express(); + + app.get("/items", (req, res) => { + res.json({ status: req.query.status ?? "open" }); + }); + + module.exports = { app, qs }; + `, + }, + expect: { + imports: ['qs'], + invocations: [], + // The demotion this rung must resist: an import with no call is still an import. Reporting + // nothing here would read as "not installed", which is a different and false claim. + absentInvocations: ['qs.parse'], + provenFlow: false, + }, + }, + + { + id: 'ladder/not-a-code-question', + rung: 'not-a-code-question', + cve: 'CVE-2021-39138', + pkg: 'parse-server', + name: 'parse-server is the deployed app, not a library this code calls', + packageJson: { dependencies: { 'parse-server': '4.5.0', express: '4' } }, + files: { + 'index.js': ` + const express = require("express"); + const { ParseServer } = require("parse-server"); + + // The flaw is inside the deployed service's own session handling. Nothing in this file gates + // it: the app is configuration around a package that IS the application. Source analysis has + // no artifact to look at, which is a property of the advisory, not a gap in the scan. + const app = express(); + app.use("/parse", new ParseServer({ + databaseURI: process.env.DATABASE_URI, + appId: process.env.APP_ID, + masterKey: process.env.MASTER_KEY, + })); + + app.listen(1337); + `, + }, + expect: { + imports: ['parse-server'], + // Constructing the server is the only call there is. It is evidence the package is deployed, + // not evidence a vulnerable API was invoked — the distinction the rung exists to make. + invocations: ['parse-server.ParseServer'], + provenFlow: false, + }, + }, + + { + id: 'ladder/unknown', + rung: 'unknown', + cve: 'CVE-2017-5941', + pkg: 'node-serialize', + name: 'node-serialize is reached through a computed require the map cannot see', + packageJson: { dependencies: { express: '4', 'node-serialize': '0.0.4' } }, + files: { + 'src/plugins.js': ` + // The specifier is computed at runtime. No static analysis can resolve which package this is, + // so the map must report that it could not tell — not that the package is unused. + const REGISTRY = { serializer: "node-" + "serialize" }; + + function loadPlugin(kind) { + return require(REGISTRY[kind]); + } + + module.exports = { loadPlugin }; + `, + 'src/server.js': ` + const express = require("express"); + const { loadPlugin } = require("./plugins"); + const app = express(); + + app.post("/restore", (req, res) => { + // Untrusted input reaches a deserializer, and the map still cannot say which one. Both + // halves matter: there IS a real risk here, and the evidence for it is invisible. + const plugin = loadPlugin("serializer"); + res.json(plugin.unserialize(req.body.state)); + }); + + module.exports = app; + `, + }, + expect: { + // Nothing to find: the require is computed, so no import is attributable. + imports: [], + invocations: [], + // And critically, the invisible call must not be attributed to the package by name match. + absentInvocations: ['node-serialize.unserialize'], + provenFlow: false, + // The map has to SAY it cannot see this shape. Absent this, a consumer reading an empty + // inventory has no way to tell "nothing is called" from "nothing is visible". + limitations: [/dynamic import/, /computed callee/], + }, + }, +]; diff --git a/tests/map/ladder.test.ts b/tests/map/ladder.test.ts new file mode 100644 index 0000000..2805979 --- /dev/null +++ b/tests/map/ladder.test.ts @@ -0,0 +1,172 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { buildInputMap } from '../../src/map/index.js'; +import { LADDER_CASES, type LadderCase } from './ladder-cases.js'; +import type { InputMap } from '../../src/map/types.js'; + +// What the map reports about a dependency, per reachability rung. +// +// Every other map test asks whether a flow compiles to a rule. This asks what the map SAYS, because +// that is what a consumer grades into a verdict — and the two answers come apart: a package can be +// plainly reachable and compile no rule, and an app can produce no evidence at all. +// +// The assertions are deliberately split in two. Here we check the map's own output; the platform +// grades that output into a verdict, and it asserts the grade separately. Collapsed into one layer, a +// failure cannot tell you whether the map saw the wrong thing or the ladder mis-graded it. + +const maps = new Map(); +const dirs: string[] = []; + +beforeAll(async () => { + for (const c of LADDER_CASES) { + const dir = mkdtempSync(join(tmpdir(), 'ps-ladder-')); + dirs.push(dir); + + for (const [rel, body] of Object.entries(c.files)) { + const path = join(dir, rel); + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, body); + } + writeFileSync(join(dir, 'package.json'), JSON.stringify(c.packageJson)); + + const { map, error } = await buildInputMap(dir); + expect(error, `${c.id} must produce a map`).toBeUndefined(); + maps.set(c.id, map!); + } +}); +afterAll(() => dirs.forEach((d) => rmSync(d, { recursive: true, force: true }))); + +const mapFor = (c: LadderCase): InputMap => { + const map = maps.get(c.id); + if (map === undefined) throw new Error(`no map for ${c.id}`); + return map; +}; +const symbols = (map: InputMap): string[] => + (map.apiInvocations ?? []).map((i) => `${i.package}.${i.symbol}`); +const flows = (map: InputMap) => map.endpoints.flatMap((e) => e.flows ?? []); + +describe('every ladder case was actually analysed', () => { + // The positive control, and the reason it comes first: an app that lands on its rung because the + // map failed to read it would pass every assertion below for entirely the wrong reason. `imported` + // in particular is indistinguishable from "parsed nothing" without this. + it.each(LADDER_CASES.map((c) => [c.id, c] as const))('%s parsed its source', (_id, c) => { + const map = mapFor(c); + + expect(map.coverage.filesParsed, 'no files parsed means nothing below is evidence').toBeGreaterThan(0); + expect(map.coverage.sourceBytes).toBeGreaterThan(0); + }); + + it.each(LADDER_CASES.map((c) => [c.id, c] as const))('%s declares its dependency', (_id, c) => { + const declared = (map: InputMap) => ((map.imports ?? []) as Array<{ package: string }>).map((i) => i.package); + // Declared in package.json regardless of whether the map can attribute a usage. For the + // `unknown` case this is the whole point: the dependency is present and the usage is invisible. + expect(c.packageJson.dependencies).toHaveProperty(c.pkg); + expect(declared(mapFor(c)).length + (mapFor(c).apiInvocations ?? []).length).toBeGreaterThanOrEqual(0); + }); +}); + +describe('the import inventory reports what it can attribute', () => { + it.each(LADDER_CASES.map((c) => [c.id, c] as const))('%s', (_id, c) => { + const imported = ((mapFor(c).imports ?? []) as Array<{ package: string }>).map((i) => i.package); + + for (const expected of c.expect.imports) { + expect(imported, `${c.id} must attribute an import of ${expected}`).toContain(expected); + } + }); +}); + +describe('the invocation inventory reports calls, and only real ones', () => { + it.each(LADDER_CASES.map((c) => [c.id, c] as const))('%s records its expected calls', (_id, c) => { + const found = symbols(mapFor(c)); + + for (const expected of c.expect.invocations) { + expect(found, `${c.id} must record ${expected}`).toContain(expected); + } + }); + + it.each( + LADDER_CASES.filter((c) => (c.expect.absentInvocations ?? []).length > 0).map((c) => [c.id, c] as const), + )('%s does not invent a call it cannot see', (_id, c) => { + const found = symbols(mapFor(c)); + + for (const absent of c.expect.absentInvocations ?? []) { + // The promotions each rung must resist. `qs.parse` appearing for an imported-but-uncalled + // package, or `node-serialize.unserialize` appearing for a computed require, would both be the + // map claiming evidence it does not have. + expect(found, `${c.id} must not claim ${absent}`).not.toContain(absent); + } + }); +}); + +describe('a proven flow is reported only where one exists', () => { + it.each(LADDER_CASES.map((c) => [c.id, c] as const))('%s', (_id, c) => { + const count = flows(mapFor(c)).length; + + if (c.expect.provenFlow) { + expect(count, `${c.id} must show untrusted input reaching the sink`).toBeGreaterThan(0); + } else { + // Not merely "no rule compiled": no flow at all. Without this, the `api-called` case could + // quietly become a second `reachable` case and still pass everything else. + expect(count, `${c.id} must not claim a flow it has no evidence for`).toBe(0); + } + }); +}); + +describe('the map declines to answer where it cannot see', () => { + const unknown = LADDER_CASES.find((c) => c.rung === 'unknown')!; + + it('attributes no import for a computed require', () => { + const imported = ((mapFor(unknown).imports ?? []) as Array<{ package: string }>).map((i) => i.package); + + expect(imported).not.toContain(unknown.pkg); + }); + + it('says why absence is not evidence here', () => { + const limits = (mapFor(unknown).coverage.apiInventoryLimitations ?? []).join(' '); + + // The difference between "nothing is called" and "nothing is visible". A consumer reading an + // empty inventory has no way to tell them apart unless the map states the shapes it cannot see — + // and a confident "not imported" for code the map never resolved is the one negative claim this + // design forbids. + for (const pattern of unknown.expect.limitations ?? []) { + expect(limits, `the limitations must mention ${pattern}`).toMatch(pattern); + } + }); + + it('never offers a completeness flag a consumer could read as licence', () => { + const coverage = mapFor(unknown).coverage as Record; + + expect(coverage.apiInventoryComplete).toBeUndefined(); + expect(coverage.importsComplete, 'a computed require means the inventory is not complete') + .not.toBe(true); + }); + + it('still reports the request input, so the risk is visible even when the sink is not', () => { + // Both halves matter: there IS untrusted input in this app, and the call it reaches is invisible. + // Reporting neither would hide the case entirely; reporting the flow would overclaim. + const inputs = mapFor(unknown).endpoints.flatMap((e) => e.inputs ?? []); + + expect(inputs.length, 'the endpoint and its input are visible even though the sink is not') + .toBeGreaterThan(0); + }); +}); + +describe('the ladder cases stay distinguishable', () => { + it('covers all five rungs exactly once', () => { + const rungs = LADDER_CASES.map((c) => c.rung).sort(); + + expect(rungs).toEqual( + ['api-called', 'imported', 'not-a-code-question', 'reachable', 'unknown'].sort(), + ); + }); + + it('pairs each case with a distinct fixture advisory', () => { + const cves = LADDER_CASES.map((c) => c.cve); + + // The platform-side ladder assertions look these up in the shared CVE fixture. A duplicate would + // make two rungs indistinguishable there. + expect(new Set(cves).size).toBe(cves.length); + }); +}); From 9a163958a0c894614e467da9d3f3b53a2187c29c Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Tue, 18 Aug 2026 17:57:02 +0200 Subject: [PATCH 2/5] Root a receiver chain through a `new` expression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `new Pool().query()` — and, more importantly, `function getDb() { return new Pool() }` called as `getDb().query()` — rooted at nothing, because the root walk followed call expressions but not constructions. The generated-app shape is a module-level getter returning a client, so this was not an exotic gap: the receiver came back untraceable, and untraceable is reported the same way as app code — no sink, no invocation record, nothing saying a link was skipped. A construction is a chain link like any other, so the walk now follows it. --- src/map/ast.ts | 5 ++++- tests/map/api-invocations.test.ts | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/map/ast.ts b/src/map/ast.ts index 22766cc..75cba19 100644 --- a/src/map/ast.ts +++ b/src/map/ast.ts @@ -3,11 +3,14 @@ import type { TsModule } from './types.js'; // Dependency-free AST helpers shared by every recognizer in this directory. // Leftmost identifier of a member/call chain (`supabase.from(x).insert` → "supabase", `fs.writeFile` → "fs"). +// `new` is part of a chain like any other link: `new Pool().query` roots at `Pool`, and so does +// `function getDb() { return new Pool() }` — the shape a generated app uses for a database client. Omitting +// it dropped the receiver entirely, which reads as "not a dependency" rather than "not followed". export function rootIdentifier(node: any, ts: TsModule): string | undefined { let cur = node; while (cur) { if (ts.isIdentifier(cur)) return cur.text; - if (ts.isPropertyAccessExpression(cur) || ts.isElementAccessExpression(cur) || ts.isCallExpression(cur) || ts.isNonNullExpression(cur) || ts.isParenthesizedExpression(cur) || ts.isAwaitExpression(cur)) { + if (ts.isPropertyAccessExpression(cur) || ts.isElementAccessExpression(cur) || ts.isCallExpression(cur) || ts.isNewExpression(cur) || ts.isNonNullExpression(cur) || ts.isParenthesizedExpression(cur) || ts.isAwaitExpression(cur)) { cur = cur.expression; } else return undefined; } diff --git a/tests/map/api-invocations.test.ts b/tests/map/api-invocations.test.ts index 3e43272..bfce657 100644 --- a/tests/map/api-invocations.test.ts +++ b/tests/map/api-invocations.test.ts @@ -107,6 +107,33 @@ describe('the invocation inventory answers what sinks cannot', () => { expect(find(list, 'sequelize', 'define')!.resolution).toBe('factory'); }); + it('records a method on a client a local getter constructs', async () => { + // `function getDb() { return new Pool() }` in a sibling module, called as `getDb().query(...)`. The + // receiver chain runs through a `new` expression, and a chain link the root walk does not know about + // is indistinguishable from app code: the sink and the call both vanish, silently. + const d = mkdtempSync(join(tmpdir(), 'ps-inv-new-')); + mkdirSync(join(d, 'src'), { recursive: true }); + writeFileSync(join(d, 'package.json'), JSON.stringify({ dependencies: { express: '4', pg: '8' } })); + writeFileSync(join(d, 'src', 'db.js'), ` + const { Pool } = require("pg"); + function getDb() { return new Pool(); } + module.exports = { getDb }; + `); + writeFileSync(join(d, 'src', 'server.js'), ` + const express = require("express"); + const { getDb } = require("./db"); + const app = express(); + app.get("/rows", async (req, res) => { res.json(await getDb().query("select 1")); }); + module.exports = app; + `); + const { map } = await buildInputMap(d); + const query = (map!.apiInvocations ?? []).find((i) => i.package === 'pg' && i.symbol === 'query'); + + expect(query, 'a method on a constructed client belongs to the constructor’s package').toBeDefined(); + expect(query!.kind).toBe('member'); + rmSync(d, { recursive: true, force: true }); + }); + it('marks a dependency re-exported from a local module as a reexport', async () => { const list = await inventory(); const query = find(list, 'pg', 'query'); From 5c1a5541b8e73b661ded7c475d321187b4851ecc Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Tue, 18 Aug 2026 17:57:39 +0200 Subject: [PATCH 3/5] Report an API name only where the name came from the package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An invocation record makes two claims — a package and an API name — and tracing the value only established the first. A local helper re-exported from another module (`loadConfig()`, whose body returns `JSON5.parse(...)`) was recorded as an API of json5, which exports no such thing. So did a factory result called under the app's own name (`const log = createLogger(); log(...)`), and an aliased destructure (`const { pick: choose } = require("lodash")`) was recorded as `choose`. This is worse than recording nothing. A consumer comparing the inventory against an advisory's affected functions reads a name that appears in no advisory, and reads the absence of the real name as evidence about it. A trace now carries whether the NAME is the package's own — true for a binding that came straight out of the package here or through a pass-through re-export, false for anything the app derived — and only a named trace produces a record. For a pass-through, the name comes from the module that imported it, since an intermediate module may have renamed it on the way in. Calls we can trace but cannot name are counted as ambiguous, not as recorded dependency calls and not as correct local exclusions: it is a real miss, and the bucket that measures resolver quality is the honest place for it. Unchanged, and tested so it stays that way: a METHOD on a dependency-derived value (`getDb().query()`) is still that dependency's API — the method name comes from the package's surface even when the receiver's name does not. --- src/map/bindings.ts | 8 +- src/map/invocations.ts | 65 +++++++++++--- src/map/module-graph.ts | 12 ++- src/map/sinks.ts | 24 +++-- tests/map/invocation-naming.test.ts | 134 ++++++++++++++++++++++++++++ tests/map/ladder-cases.ts | 3 + 6 files changed, 227 insertions(+), 19 deletions(-) create mode 100644 tests/map/invocation-naming.test.ts diff --git a/src/map/bindings.ts b/src/map/bindings.ts index abd2da1..5706832 100644 --- a/src/map/bindings.ts +++ b/src/map/bindings.ts @@ -73,7 +73,13 @@ export function buildModuleBindings(sf: any, ts: TsModule): Bindings { const reqMod = requireSpecifier(init, ts); if (reqMod) { if (ts.isIdentifier(decl.name)) record(decl.name.text, reqMod); - else if (ts.isObjectBindingPattern(decl.name)) for (const el of decl.name.elements) if (ts.isIdentifier(el.name)) record(el.name.text, reqMod); + else if (ts.isObjectBindingPattern(decl.name)) for (const el of decl.name.elements) if (ts.isIdentifier(el.name)) { + record(el.name.text, reqMod); + // `const { merge: deepMerge } = require("lodash")` — the CommonJS twin of an import alias. + // Without this the local alias looks like the package's own export name, and reporting it as + // one invents an API the package does not have. + if (el.propertyName && ts.isIdentifier(el.propertyName)) exportNames.set(el.name.text, el.propertyName.text); + } } // const x = tracedFactory(...) / const x = new TracedClass(...) → x carries that package. // Looking up nameToModule (not just direct imports) makes this transitive: pool → conn → …. diff --git a/src/map/invocations.ts b/src/map/invocations.ts index db770c7..81fd881 100644 --- a/src/map/invocations.ts +++ b/src/map/invocations.ts @@ -16,6 +16,14 @@ import type { ModuleGraph } from './sinks.js'; // 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. // +// A record has two halves — the package and the API NAME — and tracing the value only establishes the first. +// A name the APP chose is not part of the package's surface however faithfully the value traces: `loadConfig` +// (a local function re-exported from a helper module) and `log` (`const log = createLogger()`) both trace to +// a real dependency, and neither is an API that dependency has. Reporting one is worse than reporting +// nothing: a consumer asking "is the vulnerable function called" gets a name that appears in no advisory, +// while the call that IS in the advisory's surface goes unmentioned. So a name is reported only where it came +// from the package itself, and calls we cannot name are counted, not invented. +// // 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 @@ -40,9 +48,10 @@ export interface InvocationContext { * local calls — precisely backwards for a metric meant to justify widening the parse. * * total every call/new expression seen — workload scale - * dependency traced to a package: what the inventory records + * dependency traced to a package AND nameable: exactly 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 + * ambiguous a receiver we could not classify either way, a computed/dynamic callee, or a value that + * traces to a package under a name the app chose (nothing to record — see `named` below) * * Resolver quality is `dependency / (dependency + ambiguous)`. `local` belongs in neither term: excluding a * local helper is a correct answer, not a miss. @@ -54,6 +63,17 @@ export interface CallCounts { ambiguous: number; } +/** A value traced to a package, and how strong that trace is. */ +interface Traced { + pkg: string; + specifier: string; + resolution: InvocationResolution; + /** Whether the name this value is known by here is the PACKAGE's own name for it. */ + named: boolean; + /** The package's own name, when an intermediate module renamed the binding on import. */ + apiName?: string; +} + /** Collect resolved dependency calls from one parsed file, with the call classification alongside. */ export function collectInvocations( sf: any, @@ -76,8 +96,17 @@ export function collectInvocations( } }; - /** 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 => { + /** + * The package a local name traces to, and how — including one hop for a re-exported dependency value. + * + * `named` answers the second question a record needs: is this name the PACKAGE's, or the app's? It is + * true only for a binding that came straight out of the package — an import/require here, or a + * pass-through re-export from a local module. A value the app derived (`const log = createLogger()`, or a + * local function whose return value happens to root in the package) carries a name the app invented, so + * `named` is false and there is no API to report even though the package is certain. `apiName` carries the + * package's own name for a pass-through, since an intermediate module may have renamed it on import. + */ + const traceRoot = (root: string): Traced | null => { const specifier = bindings.resolve(root); if (specifier === undefined) return null; @@ -85,22 +114,29 @@ export function collectInvocations( 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' }; + const resolution = bindings.originOf(root) ?? 'direct'; + return { pkg: direct, specifier, resolution, named: resolution === '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); + const viaModule = ctx.graph.importedBinding(ctx.file, specifier, exportName); if (viaModule !== undefined) { - return { pkg: viaModule, specifier, resolution: 'reexport' }; + return { + pkg: viaModule.package, + specifier, + resolution: 'reexport', + named: viaModule.origin === 'direct', + apiName: viaModule.name, + }; } return null; }; const push = ( - traced: { pkg: string; specifier: string; resolution: InvocationResolution }, + traced: Traced, api: string, receiver: string | undefined, kind: ApiInvocation['kind'], @@ -131,10 +167,19 @@ export function collectInvocations( 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; + if (traced?.named) { + // The package's own name for the binding: from the target module for a pass-through re-export, + // otherwise this file's (`import { saveOrder as write }` → `saveOrder`). + const api = traced.apiName ?? bindings.exportNameOf(callee.text) ?? callee.text; push(traced, api, undefined, ts.isNewExpression(node) ? 'construct' : 'call', node); counts.dependency++; + } else if (traced) { + // Traced to a package, under a name the app chose — `loadConfig()` from a helper module, or a + // factory result called directly. The only name available is one the package does not export, so + // there is nothing to record. Counted AMBIGUOUS rather than local or dependency: we failed to + // name an API here, and filing that as either a correct exclusion or a recorded call would hide + // the miss. (The real call is recorded where it is written — in the module that makes it.) + counts.ambiguous++; } else if (isLocalName(callee.text, node)) { counts.local++; } else { diff --git a/src/map/module-graph.ts b/src/map/module-graph.ts index f336c49..da11867 100644 --- a/src/map/module-graph.ts +++ b/src/map/module-graph.ts @@ -71,14 +71,22 @@ export function createModuleGraph(ts: TsModule, opts: { cwd: string; boundary: s // `db.from('orders').insert(...)` resolves to a relative specifier and nothing else. Refusing it (as // an unattributable receiver) is right for app code but wrong here: one hop away it is a real // dependency, and that chain is import-to-import, fully static — evidence, not inference. - importedPackage(fromFile, specifier, exportName) { + importedBinding(fromFile, specifier, exportName) { const target = resolveInProject(fromFile, specifier); if (!target) return undefined; const mod = load(target); if (!mod) return undefined; // The target module's own bindings answer it: `db` there resolves through // `const db = createClient(...)` back to the package `createClient` was imported from. - return npmPackageOf(mod.bindings.resolve(exportName)); + const pkg = npmPackageOf(mod.bindings.resolve(exportName)); + if (pkg === undefined) return undefined; + // `origin` and `name` come from the SAME module as the package, which is the point: a consumer + // one hop away can see neither how the value was produced there nor what the package calls it. + return { + package: pkg, + origin: mod.bindings.originOf(exportName) ?? 'direct', + name: mod.bindings.exportNameOf(exportName) ?? exportName, + }; }, }; } diff --git a/src/map/sinks.ts b/src/map/sinks.ts index dbedb0a..e6a0619 100644 --- a/src/map/sinks.ts +++ b/src/map/sinks.ts @@ -65,11 +65,23 @@ export interface ModuleGraph { /** Sinks of `exportName` in the module `specifier` resolves to, relative to `fromFile`. */ importedSinks(fromFile: string, specifier: string, exportName: string): Sink[]; /** - * The npm package `exportName` traces to inside the module `specifier` resolves to — for a client - * instance re-exported from a local module (`export const db = createClient(...)`). ONE hop: a - * re-export chain (`export { db } from './client'`) is not followed. + * What `exportName` traces to inside the module `specifier` resolves to — for a client instance + * re-exported from a local module (`export const db = createClient(...)`). ONE hop: a re-export + * chain (`export { db } from './client'`) is not followed. + * + * Three parts, because the package alone cannot answer whether the NAME belongs to that package: + * package the npm package the value came from + * origin `direct` — the target module imported this very binding from the package; + * `factory` — the target module DERIVED it (a call result, or its own function) + * name what the target module knows the binding as inside the package, i.e. its import + * alias resolved back (`import { merge as deepMerge }` → `merge`). The consumer's + * local name is the app's choice and is not part of the package's surface. */ - importedPackage(fromFile: string, specifier: string, exportName: string): string | undefined; + importedBinding( + fromFile: string, + specifier: string, + exportName: string, + ): { package: string; origin: 'direct' | 'factory'; name: string } | undefined; } export interface SinkContext { @@ -167,8 +179,8 @@ function directSinks(node: any, ts: TsModule, bindings: Bindings, ctx?: SinkCont // chain, so `attribution: 'import'`), and NO package means it stays app code — which is what keeps // `import * as helper from './util'; helper.exec(x)` correctly sink-free. if (relative && ctx && spec) { - const viaModule = ctx.graph.importedPackage(ctx.file, spec, bindings.exportNameOf(root) ?? root); - if (viaModule) return { pkg: viaModule, root, spec }; + const viaModule = ctx.graph.importedBinding(ctx.file, spec, bindings.exportNameOf(root) ?? root); + if (viaModule) return { pkg: viaModule.package, root, spec }; } return { local: bindings.locals.has(root), root, spec, relative }; }; diff --git a/tests/map/invocation-naming.test.ts b/tests/map/invocation-naming.test.ts new file mode 100644 index 0000000..a7074fa --- /dev/null +++ b/tests/map/invocation-naming.test.ts @@ -0,0 +1,134 @@ +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'; + +// An invocation record makes TWO claims — a package and an API name — and tracing the value only +// establishes the first. This file is about the second, because getting it wrong is worse than +// recording nothing: a consumer comparing the inventory against an advisory's affected functions +// reads a name that appears in no advisory, and takes the absence of the real name as evidence. +// +// The shapes here are the four ways a name can reach a package, and only two of them yield a name +// the package actually exports. +let dir: string; +beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'ps-naming-')); + mkdirSync(join(dir, 'src'), { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ + dependencies: { express: '4', json5: '2.2.1', lodash: '4', pg: '8', winston: '3' }, + })); + + // A local function that RETURNS a dependency value. `loadConfig` is this app's own function; json5 + // has no such export, and its return expression rooting in JSON5 does not make it one. + writeFileSync(join(dir, 'src', 'config.js'), ` + const JSON5 = require("json5"); + const { readFileSync } = require("node:fs"); + function loadConfig() { return JSON5.parse(readFileSync("./config.json5", "utf8")); } + module.exports = { loadConfig }; + `); + + // A local factory returning a real dependency object. Calling a METHOD on what it returns is a + // call into pg's surface — the positive control that keeps the fix from becoming a blanket refusal. + writeFileSync(join(dir, 'src', 'db.js'), ` + const { Pool } = require("pg"); + function getDb() { return new Pool(); } + module.exports = { getDb }; + `); + + // A pass-through re-export: the value never stops being lodash's, and the intermediate module + // renamed it on the way in — so the package's name for it is `merge`, not `deepMerge`. + writeFileSync(join(dir, 'src', 'util.js'), ` + const { merge: deepMerge } = require("lodash"); + module.exports = { deepMerge }; + `); + + writeFileSync(join(dir, 'src', 'server.js'), ` + const express = require("express"); + const { createLogger } = require("winston"); + const { pick: choose } = require("lodash"); + const { loadConfig } = require("./config"); + const { getDb } = require("./db"); + const { deepMerge } = require("./util"); + const log = createLogger({}); + const app = express(); + + app.get("/config", (req, res) => { res.json({ theme: loadConfig().theme }); }); + app.get("/rows", async (req, res) => { res.json(await getDb().query("select 1")); }); + app.post("/merge", (req, res) => { res.json(deepMerge({}, { ok: true })); }); + app.get("/log", (req, res) => { log("served"); res.end(); }); + app.get("/pick", (req, res) => { res.json(choose({ a: 1 }, ["a"])); }); + + module.exports = app; + `); +}); +afterAll(() => rmSync(dir, { recursive: true, force: true })); + +const inventory = async (): Promise => { + const { map } = await buildInputMap(dir); + return map!.apiInvocations ?? []; +}; +const symbols = (list: ApiInvocation[]): string[] => list.map((i) => `${i.package}.${i.symbol}`); + +describe('a name the app chose is not a package API', () => { + it('does not record a local function as an API of the package its return value came from', async () => { + const found = symbols(await inventory()); + + // The defect this fixes: `json5.loadConfig`. json5's surface is `parse`/`stringify` — a consumer + // checking whether the vulnerable function is called finds neither, and a name that is not json5's. + expect(found).not.toContain('json5.loadConfig'); + expect(found.filter((s) => s.startsWith('json5.') && s !== 'json5.parse')).toEqual([]); + }); + + it('does not record a factory result called directly under the app’s name for it', async () => { + const found = symbols(await inventory()); + + // `const log = createLogger({}); log("served")` — the value is winston's, the name is not. + expect(found).not.toContain('winston.log'); + // And the call that IS nameable is still there, so this is a naming rule and not a lost trace. + expect(found).toContain('winston.createLogger'); + }); + + it('still records a method called on a value a local factory returned', async () => { + const list = await inventory(); + const query = list.find((i) => i.package === 'pg' && i.symbol === 'query'); + + // `getDb().query(...)`. The receiver's name is the app's, which is why no receiver is reported — + // but `query` is pg's own method, and dropping it would trade a wrong record for a missing one. + expect(query, 'a method on a dependency value belongs to that dependency').toBeDefined(); + expect(query!.receiver).toBeUndefined(); + expect(query!.resolution).toBe('reexport'); + }); + + it('records a pass-through re-export under the package’s name, not the app’s alias', async () => { + const found = symbols(await inventory()); + + // `require("lodash").merge` re-exported as `deepMerge`. The value is lodash's own binding, so the + // call is real evidence — under `merge`, the name an advisory would use. + expect(found).toContain('lodash.merge'); + expect(found).not.toContain('lodash.deepMerge'); + }); + + it('resolves a renamed CommonJS destructure back to the exported name', async () => { + const found = symbols(await inventory()); + + // `const { pick: choose } = require("lodash")` in the calling file itself — the same rename, one + // hop shorter. An advisory names `pick`; `choose` is this app's word for it. + expect(found).toContain('lodash.pick'); + expect(found).not.toContain('lodash.choose'); + }); +}); + +describe('a call it cannot name is counted, not invented', () => { + it('counts an unnameable dependency call as ambiguous rather than as a recorded call', async () => { + const { map } = await buildInputMap(dir); + const c = map!.coverage as Record; + + // `loadConfig()` and `log()` are dependency-derived and unnameable. Counting them as `dependency` + // would make that bucket claim records the inventory does not hold; counting them as `local` would + // file a real miss as a correct exclusion. + expect(c.callsAmbiguous).toBeGreaterThanOrEqual(2); + expect(c.callsDependency + c.callsLocal + c.callsAmbiguous).toBe(c.callsTotal); + }); +}); diff --git a/tests/map/ladder-cases.ts b/tests/map/ladder-cases.ts index f220dc4..4f3bac7 100644 --- a/tests/map/ladder-cases.ts +++ b/tests/map/ladder-cases.ts @@ -122,6 +122,9 @@ export const LADDER_CASES: LadderCase[] = [ expect: { imports: ['json5'], invocations: ['json5.parse'], + // `loadConfig` is this app's own function. json5 has no such export, so recording it would hand a + // consumer a name that is in no advisory while `parse` — the name that IS — went unreported. + absentInvocations: ['json5.loadConfig'], // No flow from request input to the parse call. If one appears, the case has stopped testing // `api-called` and silently become a second `reachable` case. provenFlow: false, From c27f79259c4659fe4277506472e79a8a6bbd1454 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Tue, 18 Aug 2026 18:02:15 +0200 Subject: [PATCH 4/5] Report a receiver only where the package supplies its name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The receiver rule admitted any binding that came straight from the package, which includes the module object itself: `const JSON5 = require("json5"); JSON5.parse(x)` was recorded with symbol `JSON5.parse`. That name is convention, not API — the same require is written `J5` or `json5` in the next codebase — so a consumer matching the inventory against an advisory that names `parse` matches nothing, and reads the miss as evidence about the call. A receiver is now reported only when it is a NAMED member of the package, under the package's own name for it: `const { promises: fsp } = require("node:fs"); fsp.readFile(x)` stays `promises.readFile`, because `promises` is genuinely part of node:fs's surface, while the app's alias `fsp` never appears. A module object, a factory result, and a re-exported value all yield the method alone. The bindings now distinguish the two, since both are `direct` and only one carries a name the package chose. The ladder's `api-called` rung stays red here: `json5.parse` is written in a file the invocation pass does not reach yet, which the next commit addresses. --- src/map/bindings.ts | 14 +++++++++++ src/map/invocations.ts | 14 +++++++---- tests/map/invocation-naming.test.ts | 39 ++++++++++++++++++++++++++--- 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/src/map/bindings.ts b/src/map/bindings.ts index 5706832..5ea10b2 100644 --- a/src/map/bindings.ts +++ b/src/map/bindings.ts @@ -24,6 +24,16 @@ export interface Bindings { * and a consumer deciding what to act on needs to tell them apart. */ originOf(name: string): 'direct' | 'factory' | undefined; + /** + * Whether the name is a NAMED MEMBER of its package (`import { Pool } from 'pg'`, + * `const { parse } = require('json5')`) rather than the module object itself (`import * as fs`, + * `import JSON5 from 'json5'`, `const JSON5 = require('json5')`). + * + * Both are `direct`, but only the member's name belongs to the package. A module object's local name is + * the app's invention — the same require is written `JSON5`, `J5`, or `json5` in three codebases — so it + * is not a name any advisory can be matched against. + */ + isPackageMember(name: string): boolean; imports: Set; locals: Set; } @@ -31,6 +41,7 @@ export function buildModuleBindings(sf: any, ts: TsModule): Bindings { const nameToModule = new Map(); // local name → module specifier const declared = new Set(); // every name declared in this file const exportNames = new Map(); // local alias → exported name + const members = new Set(); // names bound to a NAMED export, not to the module object const imports = new Set(); const origins = new Map(); @@ -58,6 +69,7 @@ export function buildModuleBindings(sf: any, ts: TsModule): Bindings { if (ts.isNamespaceImport(nb)) record(nb.name.text, mod); else if (ts.isNamedImports(nb)) for (const el of nb.elements) { record(el.name.text, mod); + members.add(el.name.text); // `import { saveOrder as write }` — looking up `write` in the target module would miss. if (el.propertyName && ts.isIdentifier(el.propertyName)) exportNames.set(el.name.text, el.propertyName.text); } @@ -75,6 +87,7 @@ export function buildModuleBindings(sf: any, ts: TsModule): Bindings { if (ts.isIdentifier(decl.name)) record(decl.name.text, reqMod); else if (ts.isObjectBindingPattern(decl.name)) for (const el of decl.name.elements) if (ts.isIdentifier(el.name)) { record(el.name.text, reqMod); + members.add(el.name.text); // `const { merge: deepMerge } = require("lodash")` — the CommonJS twin of an import alias. // Without this the local alias looks like the package's own export name, and reporting it as // one invents an API the package does not have. @@ -125,6 +138,7 @@ export function buildModuleBindings(sf: any, ts: TsModule): Bindings { resolve: (name: string) => nameToModule.get(name), exportNameOf: (name: string) => exportNames.get(name), originOf: (name: string) => origins.get(name), + isPackageMember: (name: string) => members.has(name), imports, locals, }; diff --git a/src/map/invocations.ts b/src/map/invocations.ts index 81fd881..c8467bc 100644 --- a/src/map/invocations.ts +++ b/src/map/invocations.ts @@ -192,11 +192,15 @@ export function collectInvocations( 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; + // A receiver name is only reported when the package itself supplies that name — a NAMED member + // binding (`import { promises as fsp } from 'node:fs'` → `promises`). Everything else is the + // app's own word for a value: `pool` re-exported from `./lib`, `Student` returned by + // `sequelize.define()`, and — the quiet one — `JSON5` in `const JSON5 = require('json5')`, where + // the module object's local name is pure convention. Reporting any of them makes `symbol` a + // string no advisory contains: `JSON5.parse` where the advisory says `parse`. + const receiver = traced.resolution === 'direct' && bindings.isPackageMember(root) + ? 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()`, diff --git a/tests/map/invocation-naming.test.ts b/tests/map/invocation-naming.test.ts index a7074fa..d1af3f1 100644 --- a/tests/map/invocation-naming.test.ts +++ b/tests/map/invocation-naming.test.ts @@ -46,6 +46,8 @@ beforeAll(() => { writeFileSync(join(dir, 'src', 'server.js'), ` const express = require("express"); + const JSON5 = require("json5"); + const { promises: fsp } = require("node:fs"); const { createLogger } = require("winston"); const { pick: choose } = require("lodash"); const { loadConfig } = require("./config"); @@ -59,6 +61,10 @@ beforeAll(() => { app.post("/merge", (req, res) => { res.json(deepMerge({}, { ok: true })); }); app.get("/log", (req, res) => { log("served"); res.end(); }); app.get("/pick", (req, res) => { res.json(choose({ a: 1 }, ["a"])); }); + app.get("/dump", async (req, res) => { + await fsp.readFile("./config.json5", "utf8"); + res.type("text/plain").send(JSON5.stringify({ ok: true })); + }); module.exports = app; `); @@ -73,12 +79,14 @@ const symbols = (list: ApiInvocation[]): string[] => list.map((i) => `${i.packag describe('a name the app chose is not a package API', () => { it('does not record a local function as an API of the package its return value came from', async () => { - const found = symbols(await inventory()); + const list = await inventory(); // The defect this fixes: `json5.loadConfig`. json5's surface is `parse`/`stringify` — a consumer // checking whether the vulnerable function is called finds neither, and a name that is not json5's. - expect(found).not.toContain('json5.loadConfig'); - expect(found.filter((s) => s.startsWith('json5.') && s !== 'json5.parse')).toEqual([]); + expect(symbols(list)).not.toContain('json5.loadConfig'); + // Not just under json5, and not just as the whole symbol: the app's function name must not appear + // anywhere in the inventory, under any package or as any receiver. + expect(list.filter((i) => i.symbol.includes('loadConfig') || i.api === 'loadConfig')).toEqual([]); }); it('does not record a factory result called directly under the app’s name for it', async () => { @@ -120,6 +128,31 @@ describe('a name the app chose is not a package API', () => { }); }); +describe('a receiver is reported only where the package supplies its name', () => { + it('does not put the app’s name for a module object in the symbol', async () => { + const list = await inventory(); + const stringify = list.filter((i) => i.package === 'json5' && i.api === 'stringify'); + + // `const JSON5 = require("json5"); JSON5.stringify(...)`. `JSON5` is convention, not API: the same + // require is written `J5` or `json5` elsewhere, and an advisory naming `stringify` would match none + // of them. So the symbol is the method, and the receiver is omitted rather than invented. + expect(stringify).toHaveLength(1); + expect(stringify[0].symbol).toBe('stringify'); + expect(stringify[0].receiver).toBeUndefined(); + }); + + it('reports a receiver that IS one of the package’s exports, under the exported name', async () => { + const list = await inventory(); + const readFile = list.find((i) => i.api === 'readFile'); + + // `const { promises: fsp } = require("node:fs")` — here the receiver is a real export of node:fs, so + // `promises.readFile` is the API's own spelling and dropping it would lose information. The alias the + // app chose (`fsp`) is still not what gets reported. + expect(readFile, 'a named member receiver is part of the surface').toBeDefined(); + expect(readFile!.symbol).toBe('promises.readFile'); + }); +}); + describe('a call it cannot name is counted, not invented', () => { it('counts an unnameable dependency call as ambiguous rather than as a recorded call', async () => { const { map } = await buildInputMap(dir); From b1a5bea11febb9c605d4a09b4e24b21e5e0f72a1 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Tue, 18 Aug 2026 18:03:35 +0200 Subject: [PATCH 5/5] Collect invocations from the local modules an entry file imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An entry file rarely holds the dependency call. `src/server.js` imports `loadConfig` from `./config`, and the `JSON5.parse(...)` call lives in config.js — a file with no entry-point signal, so the walk only scanned it for imports. The map therefore reported json5 as imported and reported no call to it, which is precisely the evidence an advisory gated on *calling* the function needs. Files reachable from an entry file by a relative import are now parsed for their invocations. ONE hop, entry files only, invocations only — no endpoints, no sinks, no recursion into what the hop file itself imports. The bound is structural rather than a file budget, so coverage follows the app's shape instead of its size, and it is asserted in both directions: a call one hop away is recorded, a call two hops away is not. Sites carry the hop file, so a coordinate names the line that makes the call rather than the entry that reaches it. Its calls feed the same four buckets, so resolver quality is still measured over every call analysed. The count is reported separately from `filesParsed` — these files were already counted as pre-filtered, and the two answer different questions — and the limitations now state the new depth, in the terms the tests check. An unparseable hop file is fail-open and counted, but deliberately does NOT join the skipped-file list: that list gates the import-completeness flag, and this is a different question about a file whose imports were already read. Closes the `api-called` rung of the reachability ladder: 33/33. --- src/map/extract.ts | 52 ++++++++++++++- src/map/module-graph.ts | 4 +- src/map/sinks.ts | 7 +++ src/map/types.ts | 7 +++ tests/map/invocation-hop.test.ts | 105 +++++++++++++++++++++++++++++++ 5 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 tests/map/invocation-hop.test.ts diff --git a/src/map/extract.ts b/src/map/extract.ts index c19a5cb..f902e47 100644 --- a/src/map/extract.ts +++ b/src/map/extract.ts @@ -49,6 +49,9 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac let sourceBytes = 0; const calls = { total: 0, dependency: 0, local: 0, ambiguous: 0 }; const startedAt = Date.now(); + // Local modules an entry file imports directly — parsed after the walk for their invocations (below). + const entryFiles = new Set(); + const hopTargets = new Set(); for (const file of files) { try { @@ -67,10 +70,16 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac continue; } parsed++; + entryFiles.add(file); // Coordinates are only valid for the exact file content they were derived from. const fingerprint = createHash('sha256').update(text).digest('hex').slice(0, 16); const sf = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true, guessScriptKind(ts, file)); const bindings = buildModuleBindings(sf, ts); + for (const specifier of bindings.imports) { + if (!specifier.startsWith('.')) continue; // node_modules is not this app's surface + const target = graph.resolveLocal(file, specifier); + if (target !== undefined) hopTargets.add(target); + } imports.add(relFile, collectFileImports(sf, ts), true); // Counted on this path too. A parsed file is not a covered file: `collectFileImports` requires a // string-literal specifier, so a computed require in an entry file is just as unattributable as @@ -112,6 +121,41 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac } } + // --- one hop: the local modules an entry file imports ---------------------- + // An entry file's dependency calls are usually not written IN the entry file. `src/server.js` imports + // `loadConfig` from `./config`, and the `JSON5.parse(...)` call lives in config.js — a file with no entry + // signal, so the walk above only scanned its imports. Reading the call from the file it is written in is + // the difference between reporting the API an advisory names and reporting nothing about it. + // + // ONE hop, from entry files only, invocations only: no sinks, no endpoints, no recursion into what the + // hop file itself imports. The bound is structural — reachable from an entry by a relative import — + // rather than a file budget, so coverage is predictable from the app's shape instead of from its size. + let hopParsed = 0; + let hopFailures = 0; + for (const target of hopTargets) { + if (entryFiles.has(target)) continue; // already collected above, along with its endpoints + try { + const text = readFileSync(target, 'utf8'); + const sf = ts.createSourceFile(target, text, ts.ScriptTarget.Latest, true, guessScriptKind(ts, target)); + const bindings = buildModuleBindings(sf, ts); + // `owner` is the hop file, so a site points at the line that makes the call rather than at the + // entry file that reaches it. A coordinate naming the wrong file cannot be checked by hand. + const called = collectInvocations(sf, ts, bindings, { file: target, owner: relative(cwd, target), graph }); + invocations.add(called.invocations); + calls.total += called.counts.total; + calls.dependency += called.counts.dependency; + calls.local += called.counts.local; + calls.ambiguous += called.counts.ambiguous; + hopParsed++; + } catch (e) { + if (typeof process !== 'undefined' && process.env?.PS_MAP_DEBUG) console.error('[patchstack] map hop error', target, e); + // Fail-open, and counted: these files were already read once for their imports, so they must NOT + // join `failed` (that would double-count the skip and flip the import-completeness flag over a + // gap that belongs to a different question). + hopFailures++; + } + } + notes.push('Static analysis is best-effort — this is the DETECTED surface, not a proof of completeness.'); notes.push('`inputs` and `sinks` are INVENTORIES (both present in the handler). Only `flows` asserts that an input reaches a sink: require confidence "exact-local" or "transformed-local" before pinning a rule, and identify the input by `inputId` — a field NAME can occur in more than one request namespace.'); notes.push('Sinks are followed into same-file helpers and ONE hop into an imported relative module (a dependency\u2019s internals are not followed); deeper or dynamic indirection is not traced. Sinks inside declared-but-uncalled local functions are excluded.'); @@ -161,7 +205,10 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac 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.`); + notes.push(`\`apiInvocations\` records ${invocationList.length} dependency API call(s) resolved from ${parsed} entry file(s) plus ${hopParsed} local module(s) they import directly. POSITIVE EVIDENCE ONLY: a package missing from it may still have its API called — see coverage.apiInventoryLimitations.`); + } + if (hopFailures > 0) { + notes.push(`${hopFailures} local module(s) imported by an entry file could not be parsed, so any dependency call written in them is missing from \`apiInvocations\`.`); } // Node-only and best-effort: the map runs in a build, but the runtime this file belongs to must stay @@ -196,6 +243,7 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac filesDiscovered: stats.discovered, filesParsed: parsed, filesPreFiltered: preFiltered, + filesHopParsed: hopParsed, filesSkipped: failed.length, pathsUnwalked: stats.unwalked, importsComplete, @@ -213,7 +261,7 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac // 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', + 'only files carrying an entry-point signal, and the local modules they import directly, are parsed — a call two hops away 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', diff --git a/src/map/module-graph.ts b/src/map/module-graph.ts index da11867..38540b2 100644 --- a/src/map/module-graph.ts +++ b/src/map/module-graph.ts @@ -25,7 +25,7 @@ export function createModuleGraph(ts: TsModule, opts: { cwd: string; boundary: s const text = readFileSync(file, 'utf8'); const sf = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true, guessScriptKind(ts, file)); const bindings = buildModuleBindings(sf, ts); - // `bindings` is kept for `importedPackage`: the module's own view of what its exports came from. + // `bindings` is kept for `importedBinding`: the module's own view of what its exports came from. entry = { fnSinks: collectLocalSinks(sf, ts, bindings), calleesOf: collectCallees(sf, ts), bindings }; } catch { entry = null; // fail-open: an unreadable dependency must not break the map @@ -49,6 +49,8 @@ export function createModuleGraph(ts: TsModule, opts: { cwd: string; boundary: s }; return { + resolveLocal: resolveInProject, + importedSinks(fromFile, specifier, exportName) { const target = resolveInProject(fromFile, specifier); if (!target) return []; diff --git a/src/map/sinks.ts b/src/map/sinks.ts index e6a0619..6ae595d 100644 --- a/src/map/sinks.ts +++ b/src/map/sinks.ts @@ -82,6 +82,13 @@ export interface ModuleGraph { specifier: string, exportName: string, ): { package: string; origin: 'direct' | 'factory'; name: string } | undefined; + /** + * The in-project file a RELATIVE specifier resolves to, or undefined — a bare package specifier, a path + * that resolves to nothing, or a target outside the project boundary. Exposed because a caller that + * wants to analyse the target itself (rather than ask this graph about it) still needs the same + * resolution and the same boundary refusal; re-deriving either would let them drift apart. + */ + resolveLocal(fromFile: string, specifier: string): string | undefined; } export interface SinkContext { diff --git a/src/map/types.ts b/src/map/types.ts index 5c153cb..1f0b055 100644 --- a/src/map/types.ts +++ b/src/map/types.ts @@ -270,6 +270,13 @@ export interface Coverage { * it by subtracting, which reads as "91% unanalysed". */ filesPreFiltered: number; + /** + * Local modules parsed ONE hop from an entry file (a relative import), for their invocations only — no + * endpoints and no sinks are taken from them. Counted apart from `filesParsed` because these files were + * already reported under `filesPreFiltered`, and because the two answer different questions: how much of + * the app was analysed for entry points, versus how far the invocation inventory reached. + */ + filesHopParsed?: number; /** Files skipped because they could not be read/parsed (fail-open). */ filesSkipped: number; /** diff --git a/tests/map/invocation-hop.test.ts b/tests/map/invocation-hop.test.ts new file mode 100644 index 0000000..5c4a516 --- /dev/null +++ b/tests/map/invocation-hop.test.ts @@ -0,0 +1,105 @@ +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 { InputMap } from '../../src/map/types.js'; + +// A dependency call is rarely written in the file that holds the route. The handler calls a helper, and +// the helper — in a file with no entry-point signal, which the walk therefore only scanned for imports — +// is where the API call lives. Before this, such an app reported the package as imported and no call at +// all: exactly the evidence an advisory whose precondition is *calling* the function needs. +// +// The pass follows ONE hop, from entry files only. That bound is the other half of what these tests pin: +// a bound nobody asserts is a bound that quietly becomes "every file". +let map: InputMap; +let dir: string; +beforeAll(async () => { + dir = mkdtempSync(join(tmpdir(), 'ps-hop-')); + mkdirSync(join(dir, 'src'), { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ + dependencies: { express: '4', json5: '2.2.1', lodash: '4' }, + })); + + // ONE hop from the entry file: the call this pass exists to find. + writeFileSync(join(dir, 'src', 'config.js'), ` + const JSON5 = require("json5"); + const { readFileSync } = require("node:fs"); + const { summarise } = require("./deep"); + function loadConfig() { return JSON5.parse(readFileSync("./config.json5", "utf8")); } + module.exports = { loadConfig, summarise }; + `); + + // TWO hops: reached only through config.js, which is itself a hop. Out of bounds by design. + writeFileSync(join(dir, 'src', 'deep.js'), ` + const { pick } = require("lodash"); + function summarise(o) { return pick(o, ["theme"]); } + module.exports = { summarise }; + `); + + writeFileSync(join(dir, 'src', 'server.js'), ` + const express = require("express"); + const { loadConfig, summarise } = require("./config"); + const app = express(); + app.get("/config", (req, res) => { res.json(summarise(loadConfig())); }); + module.exports = app; + `); + + const built = await buildInputMap(dir); + expect(built.error).toBeUndefined(); + map = built.map!; +}); +afterAll(() => rmSync(dir, { recursive: true, force: true })); + +const symbols = (): string[] => (map.apiInvocations ?? []).map((i) => `${i.package}.${i.symbol}`); + +describe('a call written one hop from an entry file', () => { + it('is recorded', async () => { + expect(symbols()).toContain('json5.parse'); + }); + + it('is attributed to the file that makes it, not to the file that reached it', async () => { + const parse = (map.apiInvocations ?? []).find((i) => i.package === 'json5' && i.api === 'parse')!; + + // The site is the auditable half of the record. Pointing at server.js — where the call is not + // written — would make it uncheckable by hand and wrong in the one way nobody would think to check. + expect(parse.sites.every((s) => s.file === 'src/config.js')).toBe(true); + expect(parse.sites[0].line).toBeGreaterThan(0); + }); + + it('reports how many local modules it parsed, apart from the entry files', async () => { + const c = map.coverage as Record; + + // config.js is a hop, not an entry: it is already counted under filesPreFiltered, so adding it to + // filesParsed would double-count it and overstate how much of the app was analysed for endpoints. + expect(c.filesHopParsed).toBeGreaterThanOrEqual(1); + expect(c.filesParsed).toBe(1); + }); +}); + +describe('the hop stops where it says it stops', () => { + it('does not follow a second hop', async () => { + // `lodash.pick` is called in deep.js, which only config.js imports. Recording it would mean the + // bound had silently become "every file the app can reach", which is a different cost profile and a + // different decision than the one this shipped under. + expect(symbols()).not.toContain('lodash.pick'); + }); + + it('says so in the limitations, in the same terms', async () => { + const limits = (map.coverage.apiInventoryLimitations ?? []).join(' '); + + // The list is the map's only statement of what its silence means. If the depth changes and this + // sentence does not, a consumer reads the old bound and draws a conclusion the map cannot support. + expect(limits).toMatch(/local modules they import directly/); + expect(limits).toMatch(/two hops away is unseen/); + }); + + it('counts the hop file’s calls in the same buckets, so the totals still add up', async () => { + const c = map.coverage as Record; + + expect(c.callsDependency + c.callsLocal + c.callsAmbiguous).toBe(c.callsTotal); + // `JSON5.parse` and `readFileSync` are both in config.js; if the hop's calls were collected into the + // inventory but not into the counts, resolver quality would be measured over the wrong population. + expect(c.callsDependency).toBeGreaterThanOrEqual(3); + }); +});