[ENG-3582] map: record which dependency APIs the app calls, separately from dataflow - #139
Merged
Merged
Conversation
…y from dataflow The sink analysis answers "can request input reach a dangerous operation", which is only askable for the few API families it models — measured at ~3.6% of real advisories, identically across two independent 500-record TI samples. For the library tail the useful question is simpler and we could not answer it at all: is the package's API invoked? The motivating case is real and was verified before writing anything. sequelize CVE-2026-69240's own proof of concept is `Model.findOne({ where: { x: req.query.x } })`, and `findOne` is not a recognized sink operation — so the advisory's own demonstration shape produced NOTHING, and the vulnerability could only ever be reported as "the package is imported". It is now recorded, and a test asserts the same call still produces no sink, so the two layers cannot quietly become redundant. Discipline is the sink discipline: resolve the RECEIVER, never trust a method name. An untraceable receiver is not an invocation of a package, and `inferred` attribution does not qualify at all here — unlike a sink there is no argument role or dangerous operation to corroborate a guess. Every record carries its own evidence rather than leaving strength to be inferred from context: `attribution`, `resolution` (direct | factory | reexport) and the span. "Called on an imported binding" and "called on a value we followed through a factory" are different claims. PARTIAL BY CONSTRUCTION, and deliberately with no completeness flag. Parsing every file would raise recall but would not make absence safe: dynamic property access, computed import()/require(), reflection, unfollowed aliases, generated code and unparseable syntax all remain invisible. So `coverage.apiInventoryLimitations` enumerates the shapes instead — the list IS the statement, because a boolean is something a consumer could read as licence for "the vulnerable API is not called". Full parsing stays a performance question, not a completeness proof, and the measurements to decide it are now recorded per run: source bytes, wall time, peak RSS, invocations, and resolved vs unresolved call sites (the honest denominator — 32 invocations means nothing without the 37% trace rate beside it). Two corrections found while building it. `receiver` initially reported pg's method as `pool.query`, where `pool` is the local module's export name and not part of pg's API; it is now recorded only when the binding came straight from the package. And `Bindings` now records HOW a name reached its package, because the resolution had been inferred rather than known. Capability vocabulary goes through the versioned contract: invocationKinds and invocationResolutions added, 1.0.0 -> 1.1.0, which the version checker confirmed as an additive minor.
|
Comprehensive invocation inventory feature with thorough tests and clear structure. 🎯 Quality: 93% Elite · 📦 Size: Large — consider splitting if possible 🛡️ Standards: no pre-flight fit check ran for this change — wire 📈 This month: Your 60th PR — above team average · Averaging Good |
…e dependency calls The metric was wrong in a way that would have misinformed the decision it exists for. `apiCallsUnresolved` counted every untraced call expression, so ordinary local helpers and application methods landed in it. `resolved / (resolved + unresolved)` was therefore a property of the APP — how much of its code calls dependencies — not of the resolver. Worse, it would have gone DOWN as parsing widened, because more parsing finds more local calls: exactly backwards for a number meant to justify parsing more. Four buckets now, and they sum to the total (asserted, so none can silently absorb calls): callsTotal every call/new expression — workload scale callsDependency traced to a package callsLocal a known local binding or an enclosing parameter — correctly excluded callsAmbiguous a receiver we could not classify, or a computed/dynamic callee Resolver quality is `callsDependency / (callsDependency + callsAmbiguous)`. `callsLocal` is in neither term: declining to attribute `res.json()` to a package is a correct answer, not a miss. The line between local and ambiguous took a correction of its own. A handler parameter is local, so `res.json()` is local — but `res.locals.db.query()` is NOT, even though its root is that same parameter: the value being called is whatever was stashed on `res.locals`, and a database client is precisely what apps put there. Only a method called DIRECTLY on a local binding counts as local; anything deeper is ambiguous, which matches how the sink analysis already treats those receivers. On the real app the corrected figure is 52% of dependency-candidate receivers resolved, against the 37% the old rate reported — and the old number would have looked worse the more we parsed.
`peakRssBytes` held `memoryUsage().rss` — the resident size at the moment extraction finished, after
the walk's garbage may already have been collected. That is a point-in-time reading, and calling it a
peak would have overstated it in exactly the place it would be used: deciding whether parsing more
files is affordable.
Split into two honestly-labelled numbers rather than just renaming, because the peak is the one the
decision needs:
rssBytes resident size when extraction finished — point-in-time, not a high-water mark
peakRssBytes a real high-water mark from the OS (`resourceUsage().maxRSS`), but PROCESS-wide: it
includes loading the TypeScript compiler, so it bounds the cost of running `map` from
above rather than attributing a peak to extraction alone
Both are documented with those caveats, and a test pins the ordering invariant. Absent where the
platform does not report them — nothing here may assume `process`, since this file's runtime has to
stay edge-safe.
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
2 participants
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.
Closes ENG-3582. Builds on the import inventory and the capability contract (#136, #137, both merged).
Why
Sinks answer "can request input reach a dangerous operation". That is only askable for the few API families the extractor models — ~3.6% of real advisories, the same rate across two independent 500-record TI samples. For the library tail the useful question is simpler, and we could not answer it at all: is the package's API invoked?
The motivating case is real, and I verified it before writing anything:
sequelizeCVE-2026-69240's own PoC isModel.findOne({ where: { x: req.query.x } }).findOneis not a recognized sink operation, so the advisory's own demonstration shape produced nothing — the vulnerability could only ever be reported as "the package is imported".It is recorded now, and a test asserts that same call still produces no sink, so the two layers cannot quietly become redundant.
Discipline
Resolve the receiver, never trust a method name — the sink rule, for the sink reason. An untraceable receiver is not an invocation of a package, and
inferredattribution does not qualify at all here: unlike a sink there is no argument role or dangerous operation to corroborate a guess.Each record carries its own evidence rather than leaving strength to be inferred from context:
{ "package": "sequelize", "api": "findOne", "symbol": "findOne", "kind": "member", "attribution": "import", "resolution": "factory", "callCount": 2, "sites": [{ "file": "src/search.ts", "line": 42, "start": 1201, "end": 1224 }] }resolutionseparates called on an imported binding from called on a value followed through a factory or through a local re-export — different strengths of the same claim, which a consumer deciding what to act on needs told apart.Partial by construction, and no completeness flag
Parsing every file would raise recall but would not make absence safe: dynamic property access, computed
import()/require(), reflection, unfollowed aliases, generated code and unparseable syntax all stay invisible.So
coverage.apiInventoryLimitationsenumerates those shapes and there is deliberately noapiInventoryComplete— a boolean is something a consumer could read as licence for "the vulnerable API is not called", and no parsing budget could earn that. Tests assert the limitations exist and that no completeness flag does.The numbers to decide full parsing with
Recorded per run, because "32 invocations" means nothing without scale. On the real Lovable app: 66 files discovered, 6 parsed, 190KB source, 29ms, 32 invocations across 4 packages, 58 resolved / 99 unresolved = 37% of call sites traced, document 19KB → 30KB.
That is the basis for deciding later whether parsing more files moves enough real TI pairs to justify the cost — a performance question, explicitly not a completeness one.
Two corrections found while building it
receiverfirst reported pg's method aspool.query, wherepoolis the local module's export name and no part of pg's API. It is now recorded only when the binding came straight from the package — otherwise we would be inventing API names.Bindingsnow records how a name reached its package instead of the resolution being inferred at the call site.Verification
15 new tests, 1009 passing, typecheck clean. Most of that file is about what must not be recorded: an untraceable receiver, a local function shadowing a dependency export, and any attribution other than a resolved import.
Capability vocabulary went through the versioned contract —
invocationKinds,invocationResolutions, 1.0.0 → 1.1.0 — which the version checker confirmed as an additive minor.Next
The platform side consumes this as a verdict tier between
importedandreachable, namedapi-calledrather than anything implying the vulnerable API was called: the inventory knows the package's API was invoked, not which one the advisory implicates. That needs TI to name the affected API (ENG-3583).