You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
map: resolve a client imported from a local module to its dependency (#132)
* map: resolve a client imported from a local module to its dependency
A regression I introduced, found while reviewing the integration plan against the code. The
attributable-receiver rule treats a RELATIVE receiver as app code — correct for a lookalike helper, wrong
for the layout generated apps actually use:
// src/lib/db.ts
export const db = createClient(url, key); // @supabase/supabase-js
// src/server.ts
import { db } from './lib/db';
app.post('/orders', (req, res) => db.from('orders').insert({ title: req.body.title }));
`db` resolved to the specifier `./lib/db` and nothing followed it, so the endpoint reported ZERO sinks.
Before the attribution work it at least surfaced as an unattributed sink (inventory only, never a
candidate); after, it was invisible — worse, because correlating a CVE to an endpoint joins on the sink's
package, so no vulnerability in `pg` or `@supabase/supabase-js` could ever be pinned to a route in the
common case.
The module graph already resolves relative specifiers and parses the target module (that is how imported
helper functions are followed). This adds a different question about the same data: not "what sinks are in
there" but "what does this export TRACE TO" — `importedPackage(fromFile, specifier, exportName)`, answered
by the target module's own bindings. `baseOf` asks it when the receiver is relative; a package means the
receiver IS that dependency, through an import-to-import chain that is fully static, so it earns
`attribution: 'import'` rather than the weaker `inferred`.
The narrowness is the point, and it is what keeps the earlier fix intact: the hop only produces a sink
when the export actually terminates in a dependency. `import * as helper from './util'` where that module
exports ordinary functions — including one named `from` — still yields nothing at all.
Restored, with a candidate that was previously invisible:
db.from('orders').insert({title}) -> db sink, @supabase/supabase-js, attribution import
pool.query(req.body.sql) -> db sink, pg, exact-local, sql-injection candidate
import { db as renamed } -> followed via its exported name
helper.exec(req.body.cmd) -> still no sink (the guard)
export { db } from './client' -> still no sink (one hop only, asserted as a limitation)
The project boundary applies to the new resolver too — a symlink leading out of the project is refused,
asserted rather than assumed. Corpus gains the lib/-client layout as a STACK case (not adversarial: this
is what normal generated code looks like), and every new assertion was checked against a build with the
hop disabled — 5 of them fail without it. 918 tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* map: a traced package must also establish the API before it can be a DB sink
Review of this branch caught a false-candidate path, and checking it against main showed the class is
older than this branch: package provenance was being treated as API provenance.
`.query()` / `.execute()` / `.from().insert()` are generic method names. Any receiver that resolved to a
real package was admitted as a SQL sink, so an `@apollo/client` instance — a genuine dependency with
nothing to do with SQL — produced `attribution: 'import'` and a precise SQL-injection candidate for a
GraphQL call. That rule would block legitimate traffic and mitigate nothing, which is the worst shape a
generated rule can have.
main: const local = new ApolloClient(); local.query(req.body.sql) -> candidate (already live)
main: import { client } from './lib/gql'; client.query(...) -> no sink
here: both shapes -> candidate
So the imported-client hop widened an existing hole rather than opening one. The fix is at the root: a DB
recognizer now requires the resolved package to establish a database API (`isDbPackage`, covering the
inference list plus real drivers not in it, and subpath imports such as `drizzle-orm/node-postgres`).
A traced package that is NOT a DB provider keeps its inventory entry — a `.query()` on an unknown client
is worth a human's attention — but it is marked `apiUnconfirmed`, which means: no rule, no `provider:
'sql'` claim, and no `candidateFamily` either. Advertising the sql-injection family on a GraphQL call
would mis-classify it for any consumer that reads the family without checking `ruleGeneratable`.
The refusal says which of the two things is missing, since they ask a reviewer to check different things:
"not a known db provider: it does not establish a db API (method name alone is not evidence)" vs the
existing untraceable-receiver and inferred-package reasons.
Coverage: both Apollo shapes (imported and same-file) plus controls for `pg` and a `drizzle-orm` subpath,
and a new adversarial corpus case — a non-DB client with a `.query()` method is precisely a lookalike, so
it belongs in that category. Verified 5 assertions fail with the gate disabled. 927 tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* map: claim a provider only when the receiver was traced
Follow-up from review, taken now rather than deferred because it is the same mistake one field over.
`res.locals.db.query(x)` in a file that imports `pg` was reported as `provider: 'sql'`. The flow was
already refused (the package is only INFERRED from the file, not traced to the receiver), so no rule could
come of it — but the inventory still asserted a SQL API about a receiver nobody traced, and the inventory
is what a human reads. An inferred package means "this file talks to pg", never "this receiver is a pg
client".
`provider` is now set only when the receiver resolved (`attribution: 'import'`/`'global'`), for the SQL and
prisma paths alike. `package` still carries the hint that made us look, and `attribution` already states how
strong it is.
The review suggested a `providerConfidence` field for this. I went the other way deliberately: that value
would be derived from `attribution` and `provider`, and a second confidence field is exactly what drifts —
this codebase has already had `confidence: 'precise'` survive in prose after it stopped existing in code.
Deriving the claim at the point of construction keeps one source of truth. If a consumer later needs
"possible DB API" as a distinct display state, it can compute it from the two fields it already has.
Note this is NOT the same state as `apiUnconfirmed`: there the package is wrong for the API (`@apollo/client`
for a `.query()`), here the package is right and the receiver is unknown. Two different things for a
reviewer to check, so they stay distinguishable.
One existing test looked its sink up BY `provider === 'prisma'` — the very claim being removed — so it now
finds the sink by package and asserts the absent provider. Verified the new assertion fails with the gate
reverted. 928 tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* map: an unconfirmed API must not make the argument role look wrong
Found by refreshing the demo viewer against this branch, which is a useful reminder that a consumer
reading the output notices things a test asserting one field does not.
Withholding `candidateFamily` for a sink whose package does not establish the API had a side effect: the
"argument role X is not a blockable pattern on its own" check keys off a missing family, so it fired too,
and the GraphQL `.query()` refusal read:
sink package "@apollo/client" is not a known db provider …
argument role "sql" on a db sink is not a blockable pattern on its own <- misleading
Role `sql` IS normally blockable. That second line sends a reviewer to look at the argument when the
problem is the package, and the queue of reasons is meant to be a work list, not a pile. The role check now
skips a sink whose API is unconfirmed; the package reason already explains the refusal, and the test pins
the refusal to exactly one reason.
928 tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
// from the receiver, so `res.locals.db.query(x)` in a file that happens to import `pg` looks
212
217
// identical to a real pool — and `res.locals.db` may be any app object. Such sinks stay in the
213
218
// inventory for review; they just cannot compile a rule that blocks live traffic on a guess.
219
+
if(sink.apiUnconfirmed){
220
+
reasons.push(`sink package "${sink.package}" is not a known ${sink.kind} provider: it does not establish a ${sink.kind} API (method name alone is not evidence)`);
? `sink package "${sink.package}" was inferred from the file's other imports, not from the receiver (${sink.kind}.${sink.op??'?'}): the receiver may be any app object`
@@ -224,7 +232,10 @@ function linkFlows(
224
232
? `dynamic computed key reaches this sink (${l.detail}): the field cannot be named by a parameter`
225
233
: `spread reaches this sink (${l.detail}): the specific field is not identifiable`);
0 commit comments