From 5bd30347a223d43724df56bcdd8f62daa2a72955 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Mon, 17 Aug 2026 09:48:20 +0200 Subject: [PATCH 1/4] map: inventory every imported package, and say which ones we cannot model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A sink is only recorded for the few API families the extractor models, and only inside a recognized handler — so "no sink for package P" was being used to answer a question it cannot answer: does this app use P, and can input reach it? For a package whose API we have no recognizer for, the map returned nothing, which reads as "not reachable" and closes a real vulnerability. That is the worst direction to be wrong in. Two additions, both on schema v3 (additive: a v3 reader that ignores them stays correct, and a bump would make existing consumers reject the document): - `imports` — every package the app imports, collected from ALL source files rather than only the ones holding an entry point, since an AI-built app keeps its data layer in a file with no handler in it. Specifiers are kept as written so a subpath-scoped advisory (`lodash/merge`) is still matchable; bound names come from files that were parsed anyway, and `namesComplete` admits when the set is partial rather than implying completeness. Files without an entry-point signal are scanned with TypeScript's own pre-processor instead of being parsed, which is token-accurate (a specifier in a comment is not an import) and cheap enough for every file. - `recognizedSinkKinds` — which sink families exist for that package. Empty means the map cannot speak to it at all, so a vulnerability there stays "needs review" and can never be closed as unreachable on this map's say-so. Path aliases are excluded: every AI-built app maps `@/*` to `./src/*`, and those entries would otherwise land in the unmodelled bucket and inflate exactly the count a reviewer acts on. Read from tsconfig/jsconfig `paths` (tolerantly — these files carry comments), with an npm-name check as the backstop for the unaliased case. --- src/cli.ts | 10 ++ src/map/extract.ts | 20 ++- src/map/imports.ts | 242 +++++++++++++++++++++++++++++ src/map/sinks.ts | 19 ++- src/map/types.ts | 59 +++++++ tests/map/import-inventory.test.ts | 216 +++++++++++++++++++++++++ 6 files changed, 563 insertions(+), 3 deletions(-) create mode 100644 src/map/imports.ts create mode 100644 tests/map/import-inventory.test.ts diff --git a/src/cli.ts b/src/cli.ts index e7160fb..de366c5 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -233,6 +233,16 @@ async function runMap(args: ParsedArgs): Promise { (c.filesSkipped ? `, ${c.filesSkipped} could not be analysed` : '') + `. DETECTED surface only — static analysis is best-effort; unproven pairs are marked "heuristic".`, ); + 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 + // cannot speak to at all, and a reader who only sees flows would never learn it. + const unmodelled = imported.filter((d) => d.recognizedSinkKinds.length === 0).length; + console.error( + `patchstack: ${imported.length} package(s) imported — ${unmodelled} with no recognized sink family, ` + + `so a vulnerability in those cannot be judged reachable or unreachable from this map.`, + ); + } const json = JSON.stringify(map, null, 2); const out = getStringFlag(args.flags, 'out'); if (out) { diff --git a/src/map/extract.ts b/src/map/extract.ts index b5180ce..95ca89d 100644 --- a/src/map/extract.ts +++ b/src/map/extract.ts @@ -10,6 +10,7 @@ import { collectLocalSinks } from './sinks.js'; import { createModuleGraph } from './module-graph.js'; import { isProvenFlow } from './coordinates.js'; import { extractFromFile } from './entries.js'; +import { collectFileImports, createImportInventory, readPathAliases, scanFileImports } from './imports.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 @@ -38,19 +39,28 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac const graph = createModuleGraph(ts, { cwd, boundary, followOutside: options.followSymlinks }); // shared cache const stats: WalkStats = { discovered: 0 }; const files = collectSources(cwd, boundary, { followOutside: options.followSymlinks }, [], new Set(), stats); + const imports = createImportInventory(readPathAliases(cwd)); let parsed = 0; let preFiltered = 0; for (const file of files) { try { const text = readFileSync(file, 'utf8'); - if (!hasEntrySignal(text)) { preFiltered++; continue; } + const relFile = relative(cwd, file); + // 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. + if (!hasEntrySignal(text)) { + preFiltered++; + imports.add(relFile, scanFileImports(text, ts), false); + continue; + } parsed++; // 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); - const relFile = relative(cwd, file); + 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. const localSinks = collectLocalSinks(sf, ts, bindings, ctx); @@ -98,10 +108,16 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac } if (endpoints.length === 0) notes.push('No recognized server-side entry points found under the analyzed roots.'); + const importList = imports.list(); + const unmodelled = importList.filter((d) => d.recognizedSinkKinds.length === 0).length; + notes.push('`imports` lists every package the app imports, from ALL source files — not only files holding an entry point. Absence of a package there is meaningful; absence of a SINK for a package is not.'); + notes.push(`${unmodelled} of ${importList.length} imported package(s) have no recognized sink family (recognizedSinkKinds: []). The extractor models a small set of API families, so for those packages it cannot tell whether input reaches them: a vulnerability in one must stay "needs review" and can never be closed as unreachable using this map.`); + return { version: 3, framework: detectFramework(cwd), endpoints, + imports: importList, coverage: { adapter: 'agnostic-v1', filesDiscovered: stats.discovered, diff --git a/src/map/imports.ts b/src/map/imports.ts new file mode 100644 index 0000000..93be475 --- /dev/null +++ b/src/map/imports.ts @@ -0,0 +1,242 @@ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import type { ImportedPackage, ImportSite, TsModule } from './types.js'; +import { npmPackageOf } from './bindings.js'; +import { recognizedSinkKinds } from './sinks.js'; + +// The app's IMPORT inventory — "which packages does this code pull in", answered for every source file, +// not just the ones that hold an entry point. +// +// Why it is separate from sink collection: a sink is only recorded for the few API families the extractor +// models, and only inside a recognized handler. That makes sinks the wrong instrument for the question a +// vulnerability correlator actually asks — "does this app use package P at all?" — because P may be used +// heavily through an API we have no recognizer for, or from a file with no entry point in it. Answering +// that question from the sink list yields a confident "no", which is the worst possible wrong answer: +// it closes a real vulnerability as unreachable. So imports are collected on their own terms. +// +// Two fidelities, deliberately. Files with an entry-point signal are fully parsed anyway, so their imports +// come from the AST (specifiers AND bound names). The rest — most of a project — are only scanned for +// module specifiers, which is far cheaper than building a syntax tree for every file and is the half that +// matters for correlation. The mixed fidelity is reported per package as `namesComplete` rather than +// smoothed over. + +/** One import edge as found in a single file. */ +export interface RawImport { + /** Module specifier exactly as written (`node:fs`, `lodash/merge`, `./db`). */ + specifier: string; + /** Bound names: real named bindings, plus the markers `default`, `*`, `require`, `import()`. */ + names: string[]; + /** 1-based line of the import. */ + line?: number; +} + +/** Cap per package: enough to point a reviewer at the usage, bounded so a big app can't bloat the map. */ +const MAX_SITES = 5; + +/** + * A bare specifier that is a legal npm package name. Bare does NOT mean "package": every AI-built app + * configures a path alias (`@/components` → `./src/components`), and those are the app's OWN code. Letting + * them into the inventory is not cosmetic — each one lands in the "no recognized sink family" bucket and + * inflates the count a reviewer reads as unanalysable dependencies. + */ +const NPM_NAME = /^(?:@[^/@\s~][^/@\s]*\/)?[^/@\s.~][^/@\s]*$/; +function isNpmPackageName(pkg: string): boolean { + return pkg.startsWith('node:') || NPM_NAME.test(pkg); +} + +/** + * Path-alias prefixes declared in `tsconfig.json` / `jsconfig.json` (`compilerOptions.paths`). Best-effort + * and deliberately shallow: `extends` chains and bundler-config aliases (a `vite.config.ts` `resolve.alias` + * is code, not data) are not followed, so the name check above remains the backstop. Reads tolerantly — + * these files routinely carry comments and trailing commas, which `JSON.parse` rejects. + */ +export function readPathAliases(cwd: string): string[] { + const prefixes: string[] = []; + for (const name of ['tsconfig.json', 'jsconfig.json']) { + let paths: Record | undefined; + try { + const raw = readFileSync(join(cwd, name), 'utf8'); + const parsed = JSON.parse(stripJsonComments(raw)) as { compilerOptions?: { paths?: Record } }; + paths = parsed.compilerOptions?.paths; + } catch { + continue; // absent or unparseable: fall back to the name check + } + for (const key of Object.keys(paths ?? {})) { + if (key.startsWith('.')) continue; // a relative alias is already excluded as a non-package + // `@/*` matches anything under `@/`; a key without a wildcard matches only itself. + prefixes.push(key.endsWith('*') ? key.slice(0, -1) : key); + } + } + return prefixes; +} + +function stripJsonComments(text: string): string { + let out = ''; + let inString = false; + let quote = ''; + for (let i = 0; i < text.length; i++) { + const c = text[i]; + const next = text[i + 1]; + if (inString) { + out += c; + if (c === '\\') { out += next ?? ''; i++; continue; } + if (c === quote) inString = false; + continue; + } + if (c === '"' || c === "'") { inString = true; quote = c; out += c; continue; } + if (c === '/' && next === '/') { while (i < text.length && text[i] !== '\n') i++; out += '\n'; continue; } + if (c === '/' && next === '*') { i += 2; while (i < text.length && !(text[i] === '*' && text[i + 1] === '/')) i++; i++; continue; } + out += c; + } + return out.replace(/,(\s*[}\]])/g, '$1'); // trailing commas +} + +/** + * Import edges from a parsed source file: `import`/`export … from`, `require(…)` and dynamic `import(…)`. + * Relative specifiers are included here and filtered later — the caller decides what counts as a package, + * and keeping the raw edge makes this function reusable and easy to test. + */ +export function collectFileImports(sf: any, ts: TsModule): RawImport[] { + const found: RawImport[] = []; + const lineOf = (node: any): number | undefined => { + try { + return sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1; + } catch { + return undefined; + } + }; + + const visit = (node: any) => { + // import x, { a as b }, * as ns from 'mod' / import 'mod' + if (ts.isImportDeclaration(node) && ts.isStringLiteralLike(node.moduleSpecifier)) { + const names: string[] = []; + const clause = node.importClause; + if (clause?.name) names.push('default'); + const nb = clause?.namedBindings; + if (nb) { + if (ts.isNamespaceImport(nb)) names.push('*'); + else if (ts.isNamedImports(nb)) { + // The EXPORTED name is what an advisory names; `import { merge as m }` is still `merge`. + for (const el of nb.elements) names.push((el.propertyName ?? el.name).text); + } + } + found.push({ specifier: node.moduleSpecifier.text, names, line: lineOf(node) }); + } + // export { a } from 'mod' — an import edge that re-exports; the package is still pulled in. + if (ts.isExportDeclaration(node) && node.moduleSpecifier && ts.isStringLiteralLike(node.moduleSpecifier)) { + const names: string[] = []; + const ec = node.exportClause; + if (ec && ts.isNamedExports(ec)) for (const el of ec.elements) names.push((el.propertyName ?? el.name).text); + else if (ec && ts.isNamespaceExport(ec)) names.push('*'); + else names.push('*'); // export * from 'mod' + found.push({ specifier: node.moduleSpecifier.text, names, line: lineOf(node) }); + } + // require('mod') and import('mod') — the call form carries no binding info at the call site, so the + // marker name records HOW it was imported instead of inventing a binding. + if (ts.isCallExpression(node)) { + const arg = node.arguments[0]; + const isRequire = ts.isIdentifier(node.expression) && node.expression.text === 'require'; + const isDynamic = node.expression.kind === ts.SyntaxKind.ImportKeyword; + if ((isRequire || isDynamic) && arg && ts.isStringLiteralLike(arg)) { + found.push({ specifier: arg.text, names: [isRequire ? 'require' : 'import()'], line: lineOf(node) }); + } + } + ts.forEachChild(node, visit); + }; + visit(sf); + return found; +} + +/** + * Module specifiers from raw text, without building a syntax tree — TypeScript's own pre-processor, the + * same scan the compiler uses to discover a file's dependencies. Token-accurate (a specifier inside a + * comment or string is not reported), and cheap enough to run over every file in a project. + * Names are not recoverable this way; callers mark the result as name-incomplete. + */ +export function scanFileImports(text: string, ts: TsModule): RawImport[] { + let refs: Array<{ fileName: string; pos: number }>; + try { + const pre = ts.preProcessFile(text, /* readImportFiles */ true, /* detectJavaScriptImports */ true); + refs = pre.importedFiles ?? []; + } catch { + return []; // fail-open: the inventory is best-effort, never a reason to lose a file + } + if (refs.length === 0) return []; + const lineStarts = lineStartOffsets(text); + return refs.map((r) => ({ specifier: r.fileName, names: [], line: lineAt(lineStarts, r.pos) })); +} + +/** + * Aggregates per-file import edges into the per-package inventory. Order-independent: the output is + * sorted, so two runs over the same tree produce byte-identical documents. + */ +export function createImportInventory(aliasPrefixes: string[] = []) { + interface Acc { + specifiers: Set; + names: Set; + namesComplete: boolean; + sites: ImportSite[]; + siteCount: number; + } + const byPackage = new Map(); + + return { + /** @param parsed whether these edges came from a full parse (names are trustworthy) or the scan. */ + add(relFile: string, edges: RawImport[], parsed: boolean): void { + for (const edge of edges) { + if (aliasPrefixes.some((p) => edge.specifier === p || edge.specifier.startsWith(p))) continue; + const pkg = npmPackageOf(edge.specifier); + if (!pkg) continue; // relative/absolute path: app code, not a dependency + if (!isNpmPackageName(pkg)) continue; // an unaliased-but-bare path (`@/x`, `~/lib`) + let acc = byPackage.get(pkg); + if (!acc) { + acc = { specifiers: new Set(), names: new Set(), namesComplete: true, sites: [], siteCount: 0 }; + byPackage.set(pkg, acc); + } + acc.specifiers.add(edge.specifier); + for (const n of edge.names) acc.names.add(n); + // One unparsed site makes the whole package's name set a subset — say so rather than imply + // completeness from the names that happen to be present. + if (!parsed) acc.namesComplete = false; + acc.siteCount++; + if (acc.sites.length < MAX_SITES) acc.sites.push({ file: relFile, line: edge.line }); + } + }, + + list(): ImportedPackage[] { + return [...byPackage.entries()] + .sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)) + .map(([pkg, acc]) => { + const names = [...acc.names].sort(); + const entry: ImportedPackage = { + package: pkg, + specifiers: [...acc.specifiers].sort(), + namesComplete: acc.namesComplete, + sites: acc.sites, + siteCount: acc.siteCount, + recognizedSinkKinds: recognizedSinkKinds(pkg), + }; + if (names.length > 0) entry.names = names; + return entry; + }); + }, + }; +} + +function lineStartOffsets(text: string): number[] { + const starts = [0]; + for (let i = 0; i < text.length; i++) if (text.charCodeAt(i) === 10) starts.push(i + 1); + return starts; +} + +/** 1-based line containing `pos`, by binary search over the line starts. */ +function lineAt(starts: number[], pos: number): number { + let lo = 0; + let hi = starts.length - 1; + while (lo < hi) { + const mid = (lo + hi + 1) >> 1; + if ((starts[mid] ?? 0) <= pos) lo = mid; + else hi = mid - 1; + } + return lo + 1; +} diff --git a/src/map/sinks.ts b/src/map/sinks.ts index 73a885f..dbedb0a 100644 --- a/src/map/sinks.ts +++ b/src/map/sinks.ts @@ -1,5 +1,5 @@ import { createHash } from 'node:crypto'; -import type { ArgumentRole, CandidateFamily, Sink, TsModule } from './types.js'; +import type { ArgumentRole, CandidateFamily, Sink, SinkKind, TsModule } from './types.js'; import { isFnLike, isShadowedByEnclosingBinding, @@ -44,6 +44,23 @@ const isFsPackage = (pkg: string) => /^node:fs(\/promises)?$/.test(pkg) || FS_PA const EXEC_PACKAGES = ['execa', 'cross-spawn', 'shelljs', 'zx']; const isExecPackage = (pkg: string) => pkg === 'node:child_process' || EXEC_PACKAGES.includes(pkg); +/** + * Which sink families this package can produce, per the recognizer tables above — i.e. what the map is + * even *able* to see about it. Empty for the vast majority of npm: a package with no recognizer can still + * be imported and called, we just have no model of its API, so no flow into it will ever be reported. + * Exposed so the import inventory can say that out loud rather than let a consumer read "no sink" as + * "not reachable". Takes a package ROOT (subpaths already normalized away by `npmPackageOf`), except for + * `node:fs/promises`, whose subpath is part of the builtin's identity. + */ +export function recognizedSinkKinds(pkg: string): SinkKind[] { + const kinds: SinkKind[] = []; + if (isDbPackage(pkg)) kinds.push('db'); + if (isFsPackage(pkg)) kinds.push('fs'); + if (isExecPackage(pkg)) kinds.push('exec'); + if (isHttpPackage(pkg)) kinds.push('http'); + return kinds; +} + export interface ModuleGraph { /** Sinks of `exportName` in the module `specifier` resolves to, relative to `fromFile`. */ importedSinks(fromFile: string, specifier: string, exportName: string): Sink[]; diff --git a/src/map/types.ts b/src/map/types.ts index 5ca5912..15a44d0 100644 --- a/src/map/types.ts +++ b/src/map/types.ts @@ -291,6 +291,65 @@ export interface SiteInputMap { framework: string; endpoints: Endpoint[]; coverage: Coverage; + /** + * Every package the app imports — see `ImportedPackage`. **Still version 3 on purpose:** this field is + * purely additive, so a v3 reader that ignores it keeps behaving correctly. The version exists to catch + * *silent-failure* changes (a field whose meaning shifted under an unchanged name); a new optional field + * is not one, and bumping for it would make every existing consumer reject the document instead. + * Absent on maps produced before this shipped — treat missing as "unknown", never as "imports nothing". + */ + imports?: ImportedPackage[]; +} + +/** One place a package is imported. */ +export interface ImportSite { + /** Repo-relative file. */ + file: string; + /** 1-based line of the import statement. */ + line?: number; +} + +/** + * A package the app imports, INDEPENDENT of whether anything flows into it. + * + * Why this exists separately from `Endpoint.sinks`: a sink is only recorded for the handful of API + * families the extractor models (`SinkKind`), so "this app has no sink for package P" is not evidence + * that P is unused — it usually means P's API is not one we recognize. A consumer correlating a + * vulnerable dependency against the map needs to tell those two apart, because reading the second as the + * first turns "we cannot see it" into "it is not reachable" — a confident false negative on a real + * vulnerability. `recognizedSinkKinds` is what separates them. + */ +export interface ImportedPackage { + /** npm package root, scope kept (`@supabase/supabase-js`), or a `node:` builtin. */ + package: string; + /** + * The module specifiers as WRITTEN, deduped (`lodash`, `lodash/merge`). Advisories are frequently + * scoped to a subpath ("only `lodash/merge` is affected"), which the package root alone cannot express. + */ + specifiers: string[]; + /** + * Imported binding names — `default`, `*` for a namespace import, `require` for a CJS whole-module + * require, `import()` for a dynamic import, otherwise the named bindings. Only collected from files the + * extractor fully parsed; see `namesComplete`. + */ + names?: string[]; + /** + * false when at least one import of this package came from the cheap specifier scan rather than a full + * parse, so `names` is a SUBSET of what is actually imported. A consumer must not treat a name's absence + * as proof it is not imported when this is false. + */ + namesComplete: boolean; + /** Where it is imported — capped, so `siteCount` carries the real total. */ + sites: ImportSite[]; + /** Total number of import sites found, including any beyond the `sites` cap. */ + siteCount: number; + /** + * Which sink families the extractor can recognize for this package. **Empty means the map cannot answer + * a dataflow question about this package at all** — not that nothing reaches it. A vulnerability in a + * package with no recognized kind must stay "needs review"; it can never be closed as unreachable on the + * strength of this map. + */ + recognizedSinkKinds: SinkKind[]; } /** The TypeScript module surface we use (a subset of `typescript`), resolved from the target app. */ diff --git a/tests/map/import-inventory.test.ts b/tests/map/import-inventory.test.ts new file mode 100644 index 0000000..45a9008 --- /dev/null +++ b/tests/map/import-inventory.test.ts @@ -0,0 +1,216 @@ +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 { ImportedPackage } from '../../src/map/types.js'; + +// The import inventory answers a question the sink list cannot: "does this app use package P at all?" +// A vulnerability correlator that asks the sink list instead gets "no sink for P" and reads it as "P is +// not reachable" — closing a real vulnerability. Two properties keep that from happening, and they are +// what this file guards: the inventory must cover files with NO entry point (where the data layer of an +// AI-built app usually lives), and it must say out loud when a package's API is one the extractor has no +// model for. +let dir: string; +beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'ps-imports-')); + mkdirSync(join(dir, 'src'), { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ + dependencies: { express: '4', pg: '8', decompress: '4', lodash: '4' }, + })); + // No entry-point signal anywhere in this file: the pre-filter skips it before parsing, and it is + // exactly where the interesting dependency is imported. + writeFileSync(join(dir, 'src', 'lib.ts'), ` + import { Pool } from "pg"; + import merge from "lodash/merge"; + const decompress = require("decompress"); + export const pool = new Pool(); + export async function unpack(f: string) { return decompress(f, "/tmp/out") } + export const cfg = (a: object, b: object) => merge(a, b); + `); + writeFileSync(join(dir, 'src', 'app.ts'), ` + import express from "express"; + import * as store from "./lib"; + import { readFileSync } from "node:fs"; + const app = express(); + app.post("/unpack", async (req, res) => { await store.unpack(req.body.archive); res.end("ok"); }); + app.post("/read", (req, res) => { res.end(readFileSync(req.body.path)); }); + `); +}); +afterAll(() => rmSync(dir, { recursive: true, force: true })); + +const inventory = async () => { + const { map } = await buildInputMap(dir); + const byName = new Map(); + for (const d of map!.imports ?? []) byName.set(d.package, d); + return byName; +}; + +describe('the import inventory covers the whole project', () => { + it('records packages imported from a file with no entry point', async () => { + const inv = await inventory(); + // `src/lib.ts` is pre-filtered — never parsed — yet its dependencies must still be known. + expect(inv.get('pg')).toBeDefined(); + expect(inv.get('pg')!.sites[0]!.file).toBe('src/lib.ts'); + expect(inv.get('decompress')).toBeDefined(); + }); + + it('keeps the specifier as written, so a subpath-scoped advisory can be matched', async () => { + const inv = await inventory(); + // "only lodash/merge is affected" is a real advisory shape; the package root cannot express it. + expect(inv.get('lodash')!.specifiers).toEqual(['lodash/merge']); + }); + + it('collects bound names from parsed files and admits when they are partial', async () => { + const inv = await inventory(); + expect(inv.get('node:fs')!.names).toEqual(['readFileSync']); + expect(inv.get('node:fs')!.namesComplete).toBe(true); + // pg was only ever seen by the cheap scan, so its name set is a subset — never read absence as proof. + expect(inv.get('pg')!.namesComplete).toBe(false); + }); + + it('excludes relative imports — app code is not a dependency', async () => { + const inv = await inventory(); + expect([...inv.keys()].filter((k) => k.startsWith('.'))).toEqual([]); + expect(inv.has('./lib')).toBe(false); + }); + + it('is deterministic and deduped across runs', async () => { + const a = await inventory(); + const b = await inventory(); + expect([...a.keys()]).toEqual([...b.keys()]); + expect([...a.keys()]).toEqual([...a.keys()].sort()); + expect(a.get('decompress')!.siteCount).toBe(1); + }); +}); + +describe('unmodelled is not unreachable', () => { + it('marks a package the extractor has no sink recognizer for', async () => { + const inv = await inventory(); + // `decompress` (zip-slip) is a real, exploitable dataflow shape — and not one of the API families + // the extractor models. The map must not imply otherwise. + expect(inv.get('decompress')!.recognizedSinkKinds).toEqual([]); + }); + + it('produces no sink for it, which is exactly why the marker has to exist', async () => { + const { map } = await buildInputMap(dir); + const ep = map!.endpoints.find((e) => e.route === '/unpack')!; + // The endpoint reads an input and calls into the vulnerable package — and the map sees no sink. + expect(ep.inputs.map((i) => i.name)).toContain('archive'); + expect(ep.sinks.filter((s) => s.package === 'decompress')).toEqual([]); + // Without recognizedSinkKinds, a consumer correlating a `decompress` CVE against this endpoint would + // find no flow and conclude "not reachable". The inventory is what turns that into "cannot tell". + const dep = (map!.imports ?? []).find((d) => d.package === 'decompress')!; + expect(dep.recognizedSinkKinds).toHaveLength(0); + expect(dep.siteCount).toBeGreaterThan(0); + }); + + it('still reports the recognized families for packages it does model', async () => { + const inv = await inventory(); + expect(inv.get('pg')!.recognizedSinkKinds).toEqual(['db']); + expect(inv.get('node:fs')!.recognizedSinkKinds).toEqual(['fs']); + }); + + it('counts the unmodelled packages in the coverage notes', async () => { + const { map } = await buildInputMap(dir); + const note = map!.coverage.notes.find((n) => n.includes('recognizedSinkKinds: []'))!; + expect(note).toBeDefined(); + expect(note).toMatch(/needs review/); + }); +}); + +describe('the document stays a v3 wire contract', () => { + it('adds the inventory without bumping the version', async () => { + const { map } = await buildInputMap(dir); + // Additive-only: a v3 consumer that ignores `imports` is still correct, so bumping would break every + // existing reader (including the saas ingest gate) for no safety gain. + expect(map!.version).toBe(3); + expect(Array.isArray(map!.imports)).toBe(true); + }); +}); + +describe('a path alias is app code, not a dependency', () => { + it('excludes tsconfig path aliases and bare non-package specifiers', async () => { + const d = mkdtempSync(join(tmpdir(), 'ps-imp-alias-')); + mkdirSync(join(d, 'src'), { recursive: true }); + writeFileSync(join(d, 'package.json'), JSON.stringify({ dependencies: { express: '4' } })); + // Comments and a trailing comma: the shape real tsconfigs ship in, which JSON.parse rejects. + writeFileSync(join(d, 'tsconfig.json'), `{ + /* Bundler mode */ + "compilerOptions": { + "strict": true, // on purpose + "paths": { "@/*": ["./src/*"], "@app/*": ["./src/app/*"], }, + } + }`); + writeFileSync(join(d, 'src', 'aliased.ts'), ` + import express from "express"; + import { Button } from "@/components/button"; + import { thing } from "@app/thing"; + import { home } from "~/pages/home"; + const app = express(); + app.get("/a", (req, res) => res.end(Button + thing + home)); + `); + const { map } = await buildInputMap(d); + const pkgs = (map!.imports ?? []).map((x) => x.package); + // `@app/*` is indistinguishable from a real scoped package by name alone — only the tsconfig says + // otherwise, which is why the aliases are read rather than guessed. + expect(pkgs).toEqual(['express']); + rmSync(d, { recursive: true, force: true }); + }); + + it('falls back to the name check when there is no tsconfig', async () => { + const d = mkdtempSync(join(tmpdir(), 'ps-imp-noalias-')); + mkdirSync(join(d, 'src'), { recursive: true }); + writeFileSync(join(d, 'package.json'), JSON.stringify({ dependencies: { express: '4' } })); + writeFileSync(join(d, 'src', 'bare.ts'), ` + import express from "express"; + import { Card } from "@/components/card"; + const app = express(); + app.get("/b", (req, res) => res.end(Card)); + `); + const { map } = await buildInputMap(d); + // `@/components` is not a legal npm name, so it is excluded even with no tsconfig to consult. + expect((map!.imports ?? []).map((x) => x.package)).toEqual(['express']); + rmSync(d, { recursive: true, force: true }); + }); +}); + +describe('import forms', () => { + it('records require, dynamic import and re-export edges', async () => { + const d = mkdtempSync(join(tmpdir(), 'ps-imp-forms-')); + mkdirSync(join(d, 'src'), { recursive: true }); + writeFileSync(join(d, 'package.json'), JSON.stringify({ dependencies: { express: '4' } })); + writeFileSync(join(d, 'src', 'forms.ts'), ` + import express from "express"; + export { compile } from "handlebars"; + const yaml = require("js-yaml"); + const app = express(); + app.post("/x", async (req, res) => { + const { render } = await import("ejs"); + res.end(yaml.load(req.body.doc) + render("")); + }); + `); + const { map } = await buildInputMap(d); + const names = (map!.imports ?? []).map((x) => x.package); + expect(names).toContain('handlebars'); + expect(names).toContain('js-yaml'); + expect(names).toContain('ejs'); + const ejs = (map!.imports ?? []).find((x) => x.package === 'ejs')!; + expect(ejs.names).toEqual(['import()']); + rmSync(d, { recursive: true, force: true }); + }); + + it('does not report a specifier that only appears in a comment or string', async () => { + const d = mkdtempSync(join(tmpdir(), 'ps-imp-cmt-')); + mkdirSync(join(d, 'src'), { recursive: true }); + writeFileSync(join(d, 'package.json'), JSON.stringify({ dependencies: {} })); + // The cheap scan is token-accurate, not a grep — this file imports nothing. + writeFileSync(join(d, 'src', 'notes.ts'), ` + // import lodash from "lodash"; + export const doc = 'require("pg")'; + `); + const { map } = await buildInputMap(d); + expect((map!.imports ?? []).map((x) => x.package)).toEqual([]); + rmSync(d, { recursive: true, force: true }); + }); +}); From c8cc6785224818efc03afce19220e2a030f444e4 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Mon, 17 Aug 2026 10:02:03 +0200 Subject: [PATCH 2/4] map: an absent package is only evidence when the inventory is complete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the import inventory. Three were real defects. `coverage.importsComplete` — a package missing from the inventory could mean "not imported", "this file could not be read", or "the scan of it failed", and only the first licenses a negative conclusion. The scan swallowed its own failures and returned an empty array, which states the strongest of those three. It now returns null, the extractor counts it alongside unreadable files, and the flag says whether absence means anything at all. Absent (rather than false) on maps produced before the inventory existed — also "we do not know". A non-wildcard tsconfig alias was matched as a prefix, so an alias `"foo"` excluded the real dependency `foobar` — a package silently vanishing from the inventory, which is the exact failure this inventory was added to prevent. Wildcard information is kept and only `"foo/*"` prefix-matches. The walk took directory entries in filesystem order, so the claim that two runs produce the same bytes held on APFS and not in general. Entries are sorted: the same tree now yields the same document on any machine, and a rebuild in CI no longer looks like a changed app and cuts a needless revision. Also: the summary line still said unproven pairs are marked "heuristic", which stopped being true when v3 split the unproven tiers into imported, heuristic and unknown. --- src/cli.ts | 4 +- src/map/extract.ts | 13 +++++- src/map/imports.ts | 32 +++++++++++---- src/map/sources.ts | 4 ++ src/map/types.ts | 11 +++++ tests/map/import-inventory.test.ts | 66 +++++++++++++++++++++++++++++- 6 files changed, 117 insertions(+), 13 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index de366c5..5301a5a 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -231,7 +231,9 @@ async function runMap(args: ParsedArgs): Promise { `patchstack: ${c.filesDiscovered} file(s) found — ${c.filesParsed} analysed, ` + `${c.filesPreFiltered} skipped (no server entry point)` + (c.filesSkipped ? `, ${c.filesSkipped} could not be analysed` : '') + - `. DETECTED surface only — static analysis is best-effort; unproven pairs are marked "heuristic".`, + `. DETECTED surface only — static analysis is best-effort; every flow carries the tier it was ` + + `established at ("exact-local" and "transformed-local" are proven; "imported", "heuristic" and ` + + `"unknown" are not).`, ); const imported = map.imports ?? []; if (imported.length > 0) { diff --git a/src/map/extract.ts b/src/map/extract.ts index 95ca89d..5db998e 100644 --- a/src/map/extract.ts +++ b/src/map/extract.ts @@ -42,6 +42,7 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac const imports = createImportInventory(readPathAliases(cwd)); let parsed = 0; let preFiltered = 0; + let importScanFailures = 0; for (const file of files) { try { @@ -52,7 +53,9 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac // interesting dependency is imported. if (!hasEntrySignal(text)) { preFiltered++; - imports.add(relFile, scanFileImports(text, ts), false); + const scanned = scanFileImports(text, ts); + if (scanned === null) importScanFailures++; // this file's imports are unknown, not empty + else imports.add(relFile, scanned, false); continue; } parsed++; @@ -110,8 +113,13 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac const importList = imports.list(); const unmodelled = importList.filter((d) => d.recognizedSinkKinds.length === 0).length; - notes.push('`imports` lists every package the app imports, from ALL source files — not only files holding an entry point. Absence of a package there is meaningful; absence of a SINK for a package is not.'); + // A file we could not read is a file whose imports we do not know — the same unknown as a failed scan. + const importsComplete = importScanFailures === 0 && failed.length === 0; + notes.push('`imports` lists every package the app imports, from ALL source files — not only files holding an entry point. Absence of a SINK for a package is never evidence; absence of the PACKAGE is evidence only when coverage.importsComplete is true.'); notes.push(`${unmodelled} of ${importList.length} imported package(s) have no recognized sink family (recognizedSinkKinds: []). The extractor models a small set of API families, so for those packages it cannot tell whether input reaches them: a vulnerability in one must stay "needs review" and can never be closed as unreachable using this map.`); + if (!importsComplete) { + notes.push(`The import inventory is INCOMPLETE: ${failed.length} file(s) could not be read and ${importScanFailures} could not be scanned, so a package may be imported without appearing in \`imports\`. Do not read a package's absence as "not imported" while coverage.importsComplete is false.`); + } return { version: 3, @@ -124,6 +132,7 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac filesParsed: parsed, filesPreFiltered: preFiltered, filesSkipped: failed.length, + importsComplete, roots: ['.'], notes, }, diff --git a/src/map/imports.ts b/src/map/imports.ts index 93be475..7edb26e 100644 --- a/src/map/imports.ts +++ b/src/map/imports.ts @@ -20,6 +20,12 @@ import { recognizedSinkKinds } from './sinks.js'; // matters for correlation. The mixed fidelity is reported per package as `namesComplete` rather than // smoothed over. +/** A `compilerOptions.paths` entry: `"@/*"` is a prefix, `"foo"` matches only the specifier `foo`. */ +export interface PathAlias { + prefix: string; + wildcard: boolean; +} + /** One import edge as found in a single file. */ export interface RawImport { /** Module specifier exactly as written (`node:fs`, `lodash/merge`, `./db`). */ @@ -50,8 +56,8 @@ function isNpmPackageName(pkg: string): boolean { * is code, not data) are not followed, so the name check above remains the backstop. Reads tolerantly — * these files routinely carry comments and trailing commas, which `JSON.parse` rejects. */ -export function readPathAliases(cwd: string): string[] { - const prefixes: string[] = []; +export function readPathAliases(cwd: string): PathAlias[] { + const aliases: PathAlias[] = []; for (const name of ['tsconfig.json', 'jsconfig.json']) { let paths: Record | undefined; try { @@ -63,11 +69,14 @@ export function readPathAliases(cwd: string): string[] { } for (const key of Object.keys(paths ?? {})) { if (key.startsWith('.')) continue; // a relative alias is already excluded as a non-package - // `@/*` matches anything under `@/`; a key without a wildcard matches only itself. - prefixes.push(key.endsWith('*') ? key.slice(0, -1) : key); + // The wildcard has to be carried, not flattened into a prefix: an alias `"foo"` covers the + // specifier `foo` and nothing else, so prefix-matching it would also swallow the real npm + // package `foobar`. Only `"foo/*"` is a prefix. + if (key.endsWith('*')) aliases.push({ prefix: key.slice(0, -1), wildcard: true }); + else aliases.push({ prefix: key, wildcard: false }); } } - return prefixes; + return aliases; } function stripJsonComments(text: string): string { @@ -152,14 +161,18 @@ export function collectFileImports(sf: any, ts: TsModule): RawImport[] { * same scan the compiler uses to discover a file's dependencies. Token-accurate (a specifier inside a * comment or string is not reported), and cheap enough to run over every file in a project. * Names are not recoverable this way; callers mark the result as name-incomplete. + * + * Returns **null** when the scan itself failed. That is deliberately distinct from an empty array: a file + * we could not scan is not a file that imports nothing, and collapsing the two is what would let a server + * conclude "package absent" from a gap in our own analysis. */ -export function scanFileImports(text: string, ts: TsModule): RawImport[] { +export function scanFileImports(text: string, ts: TsModule): RawImport[] | null { let refs: Array<{ fileName: string; pos: number }>; try { const pre = ts.preProcessFile(text, /* readImportFiles */ true, /* detectJavaScriptImports */ true); refs = pre.importedFiles ?? []; } catch { - return []; // fail-open: the inventory is best-effort, never a reason to lose a file + return null; // fail-open for the map, but the caller must record that the inventory is now partial } if (refs.length === 0) return []; const lineStarts = lineStartOffsets(text); @@ -170,7 +183,7 @@ export function scanFileImports(text: string, ts: TsModule): RawImport[] { * Aggregates per-file import edges into the per-package inventory. Order-independent: the output is * sorted, so two runs over the same tree produce byte-identical documents. */ -export function createImportInventory(aliasPrefixes: string[] = []) { +export function createImportInventory(aliases: PathAlias[] = []) { interface Acc { specifiers: Set; names: Set; @@ -184,7 +197,8 @@ export function createImportInventory(aliasPrefixes: string[] = []) { /** @param parsed whether these edges came from a full parse (names are trustworthy) or the scan. */ add(relFile: string, edges: RawImport[], parsed: boolean): void { for (const edge of edges) { - if (aliasPrefixes.some((p) => edge.specifier === p || edge.specifier.startsWith(p))) continue; + const aliased = aliases.some((a) => (a.wildcard ? edge.specifier.startsWith(a.prefix) : edge.specifier === a.prefix)); + if (aliased) continue; const pkg = npmPackageOf(edge.specifier); if (!pkg) continue; // relative/absolute path: app code, not a dependency if (!isNpmPackageName(pkg)) continue; // an unaliased-but-bare path (`@/x`, `~/lib`) diff --git a/src/map/sources.ts b/src/map/sources.ts index a1091a1..32af2cb 100644 --- a/src/map/sources.ts +++ b/src/map/sources.ts @@ -70,6 +70,10 @@ export function collectSources( seen.add(key); let entries; try { entries = readdirSync(dir, { withFileTypes: true }); } catch { return out; } + // Directory order is filesystem-dependent (APFS and ext4 disagree), and it decides the order of + // endpoints and import sites in the document. Sorting makes the same tree produce the same bytes on + // any machine — otherwise a rebuild in CI looks like a changed app and cuts a pointless new revision. + entries.sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)); for (const e of entries) { if (SKIP_DIRS.has(e.name) || (e.name.startsWith('.') && e.name !== '.')) continue; const full = join(dir, e.name); diff --git a/src/map/types.ts b/src/map/types.ts index 15a44d0..cac5ef9 100644 --- a/src/map/types.ts +++ b/src/map/types.ts @@ -265,6 +265,17 @@ export interface Coverage { filesPreFiltered: number; /** Files skipped because they could not be read/parsed (fail-open). */ filesSkipped: number; + /** + * 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. + * + * This is the field that licenses a NEGATIVE conclusion. A package's absence from the inventory means + * "not imported" only when this is true; otherwise it means "we do not know", and the difference is a + * vulnerability wrongly closed. **Absent (rather than false) on maps produced before the inventory + * existed** — that case is also "we do not know", so a consumer must check the field is present and + * true, not merely that it isn't false. + */ + importsComplete?: boolean; /** Source roots analyzed, repo-relative. */ roots: string[]; /** Honest notes on what static analysis could not resolve (dynamic dispatch, indirection, …). */ diff --git a/tests/map/import-inventory.test.ts b/tests/map/import-inventory.test.ts index 45a9008..f2efcf9 100644 --- a/tests/map/import-inventory.test.ts +++ b/tests/map/import-inventory.test.ts @@ -1,8 +1,9 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, chmodSync, readFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { buildInputMap } from '../../src/map/index.js'; +import { scanFileImports } from '../../src/map/imports.js'; import type { ImportedPackage } from '../../src/map/types.js'; // The import inventory answers a question the sink list cannot: "does this app use package P at all?" @@ -119,6 +120,48 @@ describe('unmodelled is not unreachable', () => { }); }); +describe('absence is only evidence when the inventory is complete', () => { + it('reports the inventory as complete for a clean tree', async () => { + const { map } = await buildInputMap(dir); + expect(map!.coverage.importsComplete).toBe(true); + }); + + it('marks it incomplete when a file could not be analysed, and says so in the notes', async (ctx) => { + const d = mkdtempSync(join(tmpdir(), 'ps-imp-partial-')); + mkdirSync(join(d, 'src'), { recursive: true }); + writeFileSync(join(d, 'package.json'), JSON.stringify({ dependencies: { express: '4' } })); + writeFileSync(join(d, 'src', 'ok.ts'), ` + import express from "express"; + const app = express(); + app.get("/ok", (req, res) => res.end("ok")); + `); + // The walker finds this file, reading it throws, and its imports are therefore UNKNOWN. A server + // must not conclude "package P is not imported" from an inventory with a hole in it. + const bad = join(d, 'src', 'secret.ts'); + writeFileSync(bad, 'import hidden from "hidden-package";'); + chmodSync(bad, 0o000); + let unreadable = false; + try { readFileSync(bad, 'utf8'); } catch { unreadable = true; } + if (!unreadable) { rmSync(d, { recursive: true, force: true }); ctx.skip(); return; } // running as root + + const { map } = await buildInputMap(d); + expect(map!.coverage.importsComplete).toBe(false); + expect(map!.coverage.notes.some((n) => n.includes('import inventory is INCOMPLETE'))).toBe(true); + // The point of the flag: the package really is missing from the inventory, so the flag is the only + // thing standing between a hole in our analysis and a "not imported" conclusion. + expect((map!.imports ?? []).map((p) => p.package)).not.toContain('hidden-package'); + chmodSync(bad, 0o644); + rmSync(d, { recursive: true, force: true }); + }); + + it('reports a failed scan as unknown rather than as no imports', () => { + // null, not [] — an empty array would say "this file imports nothing", which is a claim we cannot + // make about a file we failed to read. + const exploding = { preProcessFile() { throw new Error('boom'); } } as unknown as Parameters[1]; + expect(scanFileImports('import x from "pg";', exploding)).toBeNull(); + }); +}); + describe('the document stays a v3 wire contract', () => { it('adds the inventory without bumping the version', async () => { const { map } = await buildInputMap(dir); @@ -158,6 +201,27 @@ describe('a path alias is app code, not a dependency', () => { rmSync(d, { recursive: true, force: true }); }); + it('does not let a non-wildcard alias swallow a real package that shares its prefix', async () => { + const d = mkdtempSync(join(tmpdir(), 'ps-imp-wild-')); + mkdirSync(join(d, 'src'), { recursive: true }); + writeFileSync(join(d, 'package.json'), JSON.stringify({ dependencies: { express: '4', foobar: '1' } })); + // `"foo"` aliases exactly one specifier. Treating it as a prefix would also exclude `foobar` — + // a real dependency silently vanishing from the inventory, which is the failure mode this whole + // file exists to prevent. + writeFileSync(join(d, 'tsconfig.json'), '{"compilerOptions":{"paths":{"foo":["./src/foo.ts"],"@/*":["./src/*"]}}}'); + writeFileSync(join(d, 'src', 'w.ts'), ` + import express from "express"; + import x from "foo"; + import y from "foobar"; + import z from "@/util"; + const app = express(); + app.get("/w", (req, res) => res.end(String(x) + y + z)); + `); + const { map } = await buildInputMap(d); + expect((map!.imports ?? []).map((p) => p.package)).toEqual(['express', 'foobar']); + rmSync(d, { recursive: true, force: true }); + }); + it('falls back to the name check when there is no tsconfig', async () => { const d = mkdtempSync(join(tmpdir(), 'ps-imp-noalias-')); mkdirSync(join(d, 'src'), { recursive: true }); From e4ca159011a6549b3a27a28dcf2b2291424e4d30 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Mon, 17 Aug 2026 10:08:37 +0200 Subject: [PATCH 3/4] map: an unwalked subtree forfeits the completeness claim too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The walk returned silently on an unreadable directory, a broken link, or a symlink leaving the project. Those paths never become files, so nothing downstream could notice them: the project simply looked smaller, filesDiscovered and filesSkipped both stayed clean, and importsComplete certified an inventory that was missing an entire subtree. A server acting on that would close a package as absent while an unread directory imported it. The walk now counts every path it did not enter, coverage reports it as pathsUnwalked, and any non-zero value clears importsComplete. An escaping symlink counts only when it could have held source — a symlinked README outside the project should not forfeit the claim. The test for it asserts the giveaway explicitly: with a locked directory, filesSkipped is still zero while the inventory is missing a package. --- src/map/extract.ts | 10 ++++++--- src/map/sources.ts | 34 +++++++++++++++++++++++------- src/map/types.ts | 6 ++++++ tests/map/import-inventory.test.ts | 30 +++++++++++++++++++++++++- 4 files changed, 68 insertions(+), 12 deletions(-) diff --git a/src/map/extract.ts b/src/map/extract.ts index 5db998e..e908965 100644 --- a/src/map/extract.ts +++ b/src/map/extract.ts @@ -37,7 +37,7 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac try { boundary = realpathSync(cwd); } catch { /* use cwd as-is */ } const graph = createModuleGraph(ts, { cwd, boundary, followOutside: options.followSymlinks }); // shared cache - const stats: WalkStats = { discovered: 0 }; + const stats: WalkStats = { discovered: 0, unwalked: 0 }; const files = collectSources(cwd, boundary, { followOutside: options.followSymlinks }, [], new Set(), stats); const imports = createImportInventory(readPathAliases(cwd)); let parsed = 0; @@ -114,11 +114,14 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac const importList = imports.list(); const unmodelled = importList.filter((d) => d.recognizedSinkKinds.length === 0).length; // A file we could not read is a file whose imports we do not know — the same unknown as a failed scan. - const importsComplete = importScanFailures === 0 && failed.length === 0; + // An unwalked subtree is the quietest gap of the three: it produces no file, so no counter moves and + // the tree just looks smaller. It has to be part of this or the flag certifies an inventory with a + // hole in it. + const importsComplete = importScanFailures === 0 && failed.length === 0 && stats.unwalked === 0; notes.push('`imports` lists every package the app imports, from ALL source files — not only files holding an entry point. Absence of a SINK for a package is never evidence; absence of the PACKAGE is evidence only when coverage.importsComplete is true.'); notes.push(`${unmodelled} of ${importList.length} imported package(s) have no recognized sink family (recognizedSinkKinds: []). The extractor models a small set of API families, so for those packages it cannot tell whether input reaches them: a vulnerability in one must stay "needs review" and can never be closed as unreachable using this map.`); if (!importsComplete) { - notes.push(`The import inventory is INCOMPLETE: ${failed.length} file(s) could not be read and ${importScanFailures} could not be scanned, so a package may be imported without appearing in \`imports\`. Do not read a package's absence as "not imported" while coverage.importsComplete is false.`); + 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.`); } return { @@ -132,6 +135,7 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac filesParsed: parsed, filesPreFiltered: preFiltered, filesSkipped: failed.length, + pathsUnwalked: stats.unwalked, importsComplete, roots: ['.'], notes, diff --git a/src/map/sources.ts b/src/map/sources.ts index 32af2cb..9cf60c6 100644 --- a/src/map/sources.ts +++ b/src/map/sources.ts @@ -47,7 +47,19 @@ const SKIP_DIRS = new Set([ 'vendor', 'tmp', 'temp', '__pycache__', ]); -export interface WalkStats { discovered: number } +export interface WalkStats { + discovered: number; + /** + * Directories or links the walk could not traverse (permissions, a broken link, a vanished path) plus + * subtrees deliberately left unvisited because a symlink escapes the project. + * + * These never become "files", so nothing downstream can notice them by counting: an unreadable + * directory simply makes the tree look smaller. That is fine for the surface view and NOT fine for the + * import inventory, whose whole value is that a package's absence means something — an unwalked subtree + * can import anything. Any non-zero value here forfeits that claim. + */ + unwalked: number; +} /** * Walk the project for source files. Symlinks are followed ONLY while they stay inside the project @@ -61,15 +73,15 @@ export function collectSources( opts: { followOutside?: boolean }, out: string[] = [], seen = new Set(), - stats: WalkStats = { discovered: 0 }, + stats: WalkStats = { discovered: 0, unwalked: 0 }, ): string[] { let key: string; - try { key = realpathSync(dir); } catch { return out; } - if (seen.has(key)) return out; - if (!opts.followOutside && !isInside(key, boundary)) return out; + try { key = realpathSync(dir); } catch { stats.unwalked++; return out; } + if (seen.has(key)) return out; // already walked via another path — not a gap + if (!opts.followOutside && !isInside(key, boundary)) { stats.unwalked++; return out; } seen.add(key); let entries; - try { entries = readdirSync(dir, { withFileTypes: true }); } catch { return out; } + try { entries = readdirSync(dir, { withFileTypes: true }); } catch { stats.unwalked++; return out; } // Directory order is filesystem-dependent (APFS and ext4 disagree), and it decides the order of // endpoints and import sites in the document. Sorting makes the same tree produce the same bytes on // any machine — otherwise a rebuild in CI looks like a changed app and cuts a pointless new revision. @@ -80,8 +92,14 @@ export function collectSources( if (e.isDirectory()) collectSources(full, boundary, opts, out, seen, stats); else if (e.isSymbolicLink()) { let st, real; - try { st = statSync(full); real = realpathSync(full); } catch { continue; } - if (!opts.followOutside && !isInside(real, boundary)) continue; // link escapes the project + try { st = statSync(full); real = realpathSync(full); } catch { stats.unwalked++; continue; } + // The link escapes the project: deliberate, but still a part of the tree we did not read, so it + // counts against the inventory's completeness exactly like a failure would — but only when it + // could have held source. A symlinked README outside the project must not forfeit the claim. + if (!opts.followOutside && !isInside(real, boundary)) { + if (st.isDirectory() || isSourceFile(e.name)) stats.unwalked++; + continue; + } if (st.isDirectory()) collectSources(full, boundary, opts, out, seen, stats); else if (st.isFile() && isSourceFile(e.name)) { out.push(full); stats.discovered++; } } else if (isSourceFile(e.name)) { out.push(full); stats.discovered++; } diff --git a/src/map/types.ts b/src/map/types.ts index cac5ef9..25f4ffc 100644 --- a/src/map/types.ts +++ b/src/map/types.ts @@ -265,6 +265,12 @@ export interface Coverage { filesPreFiltered: number; /** Files skipped because they could not be read/parsed (fail-open). */ filesSkipped: number; + /** + * Paths the walk never entered: an unreadable directory, a broken link, or a symlinked subtree that + * leaves the project. Distinct from `filesSkipped` because these never became files — an unwalked + * subtree makes the project look *smaller*, so no other counter moves. + */ + pathsUnwalked?: number; /** * 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. diff --git a/tests/map/import-inventory.test.ts b/tests/map/import-inventory.test.ts index f2efcf9..5ed50d8 100644 --- a/tests/map/import-inventory.test.ts +++ b/tests/map/import-inventory.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { mkdtempSync, mkdirSync, writeFileSync, rmSync, chmodSync, readFileSync } from 'node:fs'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, chmodSync, readFileSync, readdirSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { buildInputMap } from '../../src/map/index.js'; @@ -154,6 +154,34 @@ describe('absence is only evidence when the inventory is complete', () => { rmSync(d, { recursive: true, force: true }); }); + it('marks it incomplete when a directory could not be walked', async (ctx) => { + const d = mkdtempSync(join(tmpdir(), 'ps-imp-dir-')); + mkdirSync(join(d, 'src', 'locked'), { recursive: true }); + writeFileSync(join(d, 'package.json'), JSON.stringify({ dependencies: { express: '4' } })); + writeFileSync(join(d, 'src', 'ok.ts'), ` + import express from "express"; + const app = express(); + app.get("/ok", (req, res) => res.end("ok")); + `); + writeFileSync(join(d, 'src', 'locked', 'hidden.ts'), 'import x from "hidden-package";'); + // An unreadable DIRECTORY is the quiet case: its files are never discovered, so filesDiscovered and + // filesSkipped both stay silent and the tree merely looks smaller. Nothing but an explicit traversal + // counter can notice — and without it the flag would certify an inventory missing a whole subtree. + chmodSync(join(d, 'src', 'locked'), 0o000); + let unwalkable = false; + try { readdirSync(join(d, 'src', 'locked')); } catch { unwalkable = true; } + if (!unwalkable) { chmodSync(join(d, 'src', 'locked'), 0o755); rmSync(d, { recursive: true, force: true }); ctx.skip(); return; } + + const { map } = await buildInputMap(d); + expect(map!.coverage.pathsUnwalked).toBeGreaterThan(0); + expect(map!.coverage.importsComplete).toBe(false); + expect((map!.imports ?? []).map((p) => p.package)).not.toContain('hidden-package'); + // The give-away: the file counters look perfectly healthy while a subtree is missing. + expect(map!.coverage.filesSkipped).toBe(0); + chmodSync(join(d, 'src', 'locked'), 0o755); + rmSync(d, { recursive: true, force: true }); + }); + it('reports a failed scan as unknown rather than as no imports', () => { // null, not [] — an empty array would say "this file imports nothing", which is a claim we cannot // make about a file we failed to read. From f9510cc1f24f724f3c205bf26b35b621e2189338 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Mon, 17 Aug 2026 11:11:29 +0200 Subject: [PATCH 4/4] test: drop an internal service name from a comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit connect is a public package. The point the comment makes — that a v3 reader ignoring the new field stays correct, so bumping would break existing readers for no gain — holds for any consumer and needs no internal reference to land. --- tests/map/import-inventory.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/map/import-inventory.test.ts b/tests/map/import-inventory.test.ts index 5ed50d8..ff28117 100644 --- a/tests/map/import-inventory.test.ts +++ b/tests/map/import-inventory.test.ts @@ -194,7 +194,7 @@ describe('the document stays a v3 wire contract', () => { it('adds the inventory without bumping the version', async () => { const { map } = await buildInputMap(dir); // Additive-only: a v3 consumer that ignores `imports` is still correct, so bumping would break every - // existing reader (including the saas ingest gate) for no safety gain. + // existing reader for no safety gain. expect(map!.version).toBe(3); expect(Array.isArray(map!.imports)).toBe(true); });