Skip to content

Commit c23a69c

Browse files
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>
1 parent 7a5ecb1 commit c23a69c

5 files changed

Lines changed: 144 additions & 5 deletions

File tree

src/map/flows.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,12 @@ function linkFlows(
193193
: 'heuristic';
194194
const roles = new Set<ArgumentRole>(matched.flatMap(({ roles: rs }) => [...rs]));
195195
// Prefer a role that maps to a mitigation class over a generic one (a value can reach two args).
196-
const family = [...roles].map((r) => CANDIDATE_FAMILIES[sink.kind]?.[r]).find(Boolean);
196+
// A sink whose package does not establish this API cannot support the mitigation class either:
197+
// labelling a GraphQL `.query()` as the sql-injection family would mis-classify it for any consumer
198+
// that reads `candidateFamily` without also checking `ruleGeneratable`.
199+
const family = sink.apiUnconfirmed
200+
? undefined
201+
: [...roles].map((r) => CANDIDATE_FAMILIES[sink.kind]?.[r]).find(Boolean);
197202
const argumentRole = family
198203
? [...roles].find((r) => CANDIDATE_FAMILIES[sink.kind]?.[r])
199204
: [...roles].find((r) => r !== 'unknown') ?? (proven ? 'unknown' : undefined);
@@ -211,6 +216,9 @@ function linkFlows(
211216
// from the receiver, so `res.locals.db.query(x)` in a file that happens to import `pg` looks
212217
// identical to a real pool — and `res.locals.db` may be any app object. Such sinks stay in the
213218
// 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)`);
221+
}
214222
if (sink.attribution !== 'import' && sink.attribution !== 'global') {
215223
reasons.push(sink.attribution === 'inferred'
216224
? `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`

src/map/sinks.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,21 @@ const HTTP_MEMBER_METHODS = new Set(['get', 'post', 'put', 'patch', 'delete', 'h
2020
// When a sink's base can't be traced precisely, infer its package from the file's imports of a known
2121
// provider for that sink kind (a file almost always uses one db/http client).
2222
const DB_PACKAGES = ['@supabase/supabase-js', '@prisma/client', 'drizzle-orm', 'knex', 'kysely', 'pg', 'mysql2', 'mysql', 'sequelize', 'typeorm', 'mongoose', 'better-sqlite3'];
23+
// Drivers that are not in the inference list above but do establish a database API when a receiver
24+
// resolves to them. Extend deliberately: this list is what separates "a package we can trace" from
25+
// "a package that proves a DB API", and admitting the wrong one produces a false SQL-injection rule.
26+
const MORE_DB_PACKAGES = ['postgres', 'mssql', 'tedious', 'oracledb', 'sqlite3', 'mongodb', 'ioredis',
27+
'@planetscale/database', '@neondatabase/serverless', '@libsql/client', '@vercel/postgres', 'slonik', 'sql.js'];
28+
/**
29+
* Does `pkg` establish a DATABASE api? Package provenance is not API provenance: `.query()` is a generic
30+
* method name, and an `@apollo/client` (or any HTTP-ish client) instance resolves to a real package while
31+
* having nothing to do with SQL. Without this gate, `client.query(req.body.sql)` compiled a precise
32+
* SQL-injection candidate for a GraphQL call — a rule that blocks legitimate traffic and mitigates nothing.
33+
* Subpath imports count (`drizzle-orm/node-postgres`).
34+
*/
35+
const isDbPackage = (pkg: string) =>
36+
DB_PACKAGES.includes(pkg) || MORE_DB_PACKAGES.includes(pkg) ||
37+
[...DB_PACKAGES, ...MORE_DB_PACKAGES].some((p) => pkg.startsWith(p + '/'));
2338
const HTTP_PACKAGES = ['axios', 'got', 'node-fetch', 'undici', 'superagent', 'ky'];
2439
const isHttpPackage = (pkg: string) => HTTP_PACKAGES.includes(pkg) || pkg === 'node:http' || pkg === 'node:https';
2540
// A filesystem/process API is not only a node: builtin — these wrappers expose the same sinks, and
@@ -145,6 +160,13 @@ function directSinks(node: any, ts: TsModule, bindings: Bindings, ctx?: SinkCont
145160
// untraceable receiver is neither (undefined) and must not drive an auto-generated rule.
146161
const attributionOf = (b: { pkg?: string }, pkg: string | undefined): Sink['attribution'] =>
147162
b.pkg ? 'import' : pkg ? 'inferred' : undefined;
163+
/**
164+
* Claim `provider: 'sql'` only when the package actually establishes a database API. A traced package
165+
* that is not a DB provider stays in the INVENTORY (a `.query()` on it is worth a human's attention)
166+
* but is marked so no rule can be compiled from it — and it does not get to call itself SQL.
167+
*/
168+
const dbApi = (pkg: string | undefined): { provider?: string; apiUnconfirmed?: true } =>
169+
pkg && !isDbPackage(pkg) ? { apiUnconfirmed: true } : { provider: 'sql' };
148170
const infer = (kind: 'db' | 'http'): string | undefined => {
149171
const table = kind === 'db' ? DB_PACKAGES : HTTP_PACKAGES;
150172
for (const p of table) if (bindings.imports.has(p)) return p;
@@ -168,7 +190,7 @@ function directSinks(node: any, ts: TsModule, bindings: Bindings, ctx?: SinkCont
168190
const parent = n.parent;
169191
if (!b.local && !b.relative && parent && ts.isPropertyAccessExpression(parent) && DB_OPS.has(parent.name.text)) {
170192
const pkg = b.pkg ?? infer('db');
171-
push({ kind: 'db', provider: 'sql', package: pkg, table, op: parent.name.text, attribution: attributionOf(b, pkg), ...spanOf(opCallOf(parent, ts)) });
193+
push({ kind: 'db', ...dbApi(pkg), package: pkg, table, op: parent.name.text, attribution: attributionOf(b, pkg), ...spanOf(opCallOf(parent, ts)) });
172194
}
173195
}
174196
if (ts.isPropertyAccessExpression(callee)) {
@@ -191,7 +213,7 @@ function directSinks(node: any, ts: TsModule, bindings: Bindings, ctx?: SinkCont
191213
// receiver stays in the inventory with NO attribution — visible to a human, never auto-ruled.
192214
if (method === 'query' || method === 'execute') {
193215
const pkg = b.pkg ?? infer('db');
194-
push({ kind: 'db', provider: 'sql', package: pkg, op: method, attribution: attributionOf(b, pkg), ...spanOf(n) });
216+
push({ kind: 'db', ...dbApi(pkg), package: pkg, op: method, attribution: attributionOf(b, pkg), ...spanOf(n) });
195217
}
196218
// fs / exec via a namespace: `fs.writeFile(` / `child_process.exec(`. The receiver must
197219
// actually resolve to a filesystem/process package — the method name alone proves nothing.

src/map/types.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,13 @@ export interface Sink {
9696
* never drive an auto-generated rule, since any object can own a method by that name.
9797
*/
9898
attribution?: 'import' | 'global' | 'inferred';
99+
/**
100+
* Set when the receiver resolved to a real package that does **not** establish this kind of API —
101+
* package provenance is not API provenance. `client.query(x)` on an `@apollo/client` instance traces to
102+
* a genuine dependency while having nothing to do with SQL. Such a sink is reported for review and can
103+
* never compile a rule: a candidate here would block legitimate traffic and mitigate nothing.
104+
*/
105+
apiUnconfirmed?: true;
99106
/**
100107
* Character span of the sink's operation call in `file` (or the endpoint's file). This is the sink's
101108
* IDENTITY: flow analysis binds evidence to this exact call, never to a line or an enclosing

tests/map/corpus.test.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,28 @@ const ADVERSARIAL: Case[] = [
156156
// The schema's `post:id` is declared but never read by the sink — proven-nothing, not blockable.
157157
expectRefused: [['id', /no proven local read/]],
158158
},
159+
{
160+
name: 'adversarial: a traced package that does not establish the API',
161+
kind: 'adversarial',
162+
pkg: { dependencies: { express: '4', '@apollo/client': '3' } },
163+
files: {
164+
// `.query()` is a generic method name. An ApolloClient instance resolves to a REAL dependency, so
165+
// attribution alone admits it — and a GraphQL call became a precise SQL-injection candidate. Package
166+
// provenance is not API provenance.
167+
'src/lib/gql.ts': `
168+
import { ApolloClient } from "@apollo/client";
169+
export const client = new ApolloClient({ uri: "https://api.example.com" });
170+
`,
171+
'src/server.ts': `
172+
import express from "express";
173+
import { client } from "./lib/gql";
174+
const app = express();
175+
app.post("/graphql", async (req, res) => { await client.query(req.body.sql); res.end(); });
176+
`,
177+
},
178+
expectCandidates: [],
179+
expectRefused: [['sql', /does not establish a db API/]],
180+
},
159181
{
160182
name: 'adversarial: sibling expressions must not contaminate each other',
161183
kind: 'adversarial',
@@ -423,9 +445,9 @@ describe('golden corpus', () => {
423445
it('keeps a standing adversarial category (lookalikes are how every false candidate got in)', () => {
424446
// Guards against the category quietly emptying out; the classes listed are the ones that have
425447
// actually produced false candidates, so losing one should fail loudly.
426-
expect(ADVERSARIAL.length).toBeGreaterThanOrEqual(6);
448+
expect(ADVERSARIAL.length).toBeGreaterThanOrEqual(7);
427449
const names = ADVERSARIAL.map((c) => c.name).join(' | ');
428-
for (const cls of ['collide with dangerous API names', 'untraceable receivers', 'shadowing dangerous globals', 'two request namespaces', 'sibling expressions', 'different namespaces']) {
450+
for (const cls of ['collide with dangerous API names', 'untraceable receivers', 'shadowing dangerous globals', 'two request namespaces', 'sibling expressions', 'different namespaces', 'does not establish the API']) {
429451
expect(names, `missing adversarial class: ${cls}`).toContain(cls);
430452
}
431453
});

tests/map/imported-client.test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,3 +121,83 @@ describe('the project boundary still holds', () => {
121121
rmSync(outside, { recursive: true, force: true });
122122
});
123123
});
124+
125+
// Package provenance is NOT API provenance. `.query()` is a generic method name: an `@apollo/client`
126+
// instance resolves to a genuine dependency while having nothing to do with SQL, and admitting that as a
127+
// database sink compiled a precise SQL-injection candidate for a GraphQL call — a rule that would block
128+
// legitimate traffic and mitigate nothing. This predates the imported-client hop (a same-file
129+
// `new ApolloClient()` hit it too); the hop only made it easier to reach.
130+
describe('a traced package must also establish the API', () => {
131+
let d: string;
132+
beforeAll(() => {
133+
d = mkdtempSync(join(tmpdir(), 'ps-api-'));
134+
mkdirSync(join(d, 'src', 'lib'), { recursive: true });
135+
writeFileSync(join(d, 'package.json'), JSON.stringify({
136+
dependencies: { express: '4', '@apollo/client': '3', pg: '8', 'drizzle-orm': '0.30' },
137+
}));
138+
writeFileSync(join(d, 'src', 'lib', 'gql.ts'), `
139+
import { ApolloClient } from "@apollo/client";
140+
export const client = new ApolloClient({ uri: "https://api.example.com" });
141+
`);
142+
writeFileSync(join(d, 'src', 'lib', 'pool.ts'), `
143+
import { Pool } from "pg";
144+
export const pool = new Pool();
145+
`);
146+
writeFileSync(join(d, 'src', 'lib', 'orm.ts'), `
147+
import { drizzle } from "drizzle-orm/node-postgres";
148+
export const orm = drizzle({});
149+
`);
150+
writeFileSync(join(d, 'src', 'server.ts'), `
151+
import express from "express";
152+
import { ApolloClient } from "@apollo/client";
153+
import { client } from "./lib/gql";
154+
import { pool } from "./lib/pool";
155+
import { orm } from "./lib/orm";
156+
const app = express();
157+
const inline = new ApolloClient({ uri: "https://api.example.com" });
158+
app.post("/gql-imported", async (req, res) => { await client.query(req.body.sql); res.end(); });
159+
app.post("/gql-inline", async (req, res) => { await inline.query(req.body.sql); res.end(); });
160+
app.post("/pg", async (req, res) => { await pool.query(req.body.sql); res.end(); });
161+
app.post("/orm", async (req, res) => { await orm.execute(req.body.sql); res.end(); });
162+
`);
163+
});
164+
afterAll(() => rmSync(d, { recursive: true, force: true }));
165+
166+
const route = async (r: string) => {
167+
const { map } = await buildInputMap(d);
168+
return map!.endpoints.find((e) => e.route === r)!;
169+
};
170+
171+
it.each(['/gql-imported', '/gql-inline'])('refuses a rule for a non-DB client at %s', async (r) => {
172+
const e = await route(r);
173+
const sink = e.sinks.find((s) => s.kind === 'db')!;
174+
expect(sink.package).toBe('@apollo/client');
175+
expect(sink.attribution).toBe('import'); // the package IS established…
176+
expect(sink.apiUnconfirmed).toBe(true); // …but it does not establish a DB API
177+
expect(sink.provider).toBeUndefined(); // so it does not get to call itself SQL either
178+
const flow = e.flows.find((f) => f.sink.kind === 'db')!;
179+
expect(flow.confidence).toBe('exact-local'); // the data really does reach it
180+
expect(flow.ruleGeneratable).toBe(false);
181+
expect(flow.candidateFamily).toBeUndefined(); // and it must not advertise a class it cannot support
182+
expect(flow.ruleGeneratableReasons!.join(' ')).toMatch(/does not establish a db API/);
183+
});
184+
185+
it('keeps the sink in the inventory — a .query() on an unknown client is worth a human look', async () => {
186+
const e = await route('/gql-imported');
187+
expect(e.sinks).toHaveLength(1);
188+
});
189+
190+
it.each([
191+
['/pg', 'pg'],
192+
['/orm', 'drizzle-orm'],
193+
])('still generates for a real driver at %s', async (r, pkg) => {
194+
const e = await route(r);
195+
const sink = e.sinks.find((s) => s.kind === 'db')!;
196+
expect(sink.package).toBe(pkg); // subpath imports resolve to the package
197+
expect(sink.apiUnconfirmed).toBeUndefined();
198+
expect(sink.provider).toBe('sql');
199+
const flow = e.flows.find((f) => f.sink.kind === 'db')!;
200+
expect(flow.ruleGeneratable).toBe(true);
201+
expect(flow.candidateFamily).toBe('sql-injection');
202+
});
203+
});

0 commit comments

Comments
 (0)