map: inventory every imported package, and mark the ones we cannot model - #136
Merged
Conversation
…odel 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.
|
Comprehensive import inventory feature with detailed reporting and strong tests. 🎯 Quality: 96% Elite · 📦 Size: Large — consider splitting if possible 🛡️ Standards: no pre-flight fit check ran for this change — wire 📈 This month: Your 57th PR — above team average · Averaging Excellent |
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.
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.
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.
Contributor
Author
|
/review |
daniloradovic
approved these changes
Aug 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A sink is only recorded for the few API families the extractor models, and only inside a recognized handler. That makes the sink list the wrong instrument for the question a vulnerability correlator actually asks — does this app use package P, and can input reach it? — 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 from the sink list yields "no flow", which reads as not reachable and closes a real vulnerability. That is the worst direction to be wrong in, and it is silent.
What
Two additions, both additive on schema v3 — a v3 reader that ignores them stays correct, and bumping would make every existing consumer (including the Pulse ingest gate) reject the document instead.
SiteInputMap.imports— every package the app imports, from all source files rather than only the ones holding an entry point, because an AI-built app keeps its data layer in a file with no handler in it. Per package: the specifiers as written (so a subpath-scoped advisory likelodash/mergestays matchable), the bound names where the file was parsed, capped import sites, and a total count.Files with no entry-point signal are not parsed — they are scanned with TypeScript's own pre-processor. Token-accurate (a specifier inside a comment or string is not an import) and cheap enough to run over every file. The mixed fidelity is reported as
namesCompleterather than smoothed over: a partial name set must never be read as proof a name is absent.ImportedPackage.recognizedSinkKinds— which sink families the extractor can model for that package. Empty means the map cannot answer a dataflow question about it at all. A vulnerability in such a package stays "needs review"; it can never be closed as unreachable on the strength of this map.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 number a reviewer acts on. Read fromtsconfig/jsconfigpaths— tolerantly, since those files ship with comments and trailing commas — with an npm-name check as the backstop when there is no config to consult (@app/*is indistinguishable from a real scoped package by name alone, which is why the config is read rather than guessed).The CLI summary now reports both counts, since a reader who only sees flows would never learn how much of the dependency surface the map cannot speak to:
Verification
recognizedSinkKindshas to exist.