diff --git a/src/map/extract.ts b/src/map/extract.ts index 43e483c..b5180ce 100644 --- a/src/map/extract.ts +++ b/src/map/extract.ts @@ -50,9 +50,11 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac 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 localSinks = collectLocalSinks(sf, ts, bindings); const relFile = relative(cwd, file); - for (const ep of extractFromFile(sf, ts, localSinks, bindings, { file, owner: relFile, graph })) { + 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); + for (const ep of extractFromFile(sf, ts, localSinks, bindings, ctx)) { // A FILE-BASED route handler carries its URL path in its location, not in the code, so derive // it here — without this a rule can only be param-pinned, never route-scoped (`when.path`). if (ep.route === undefined && ep.entryKind === 'edge-function') { diff --git a/src/map/flows.ts b/src/map/flows.ts index f64ce42..cba4422 100644 --- a/src/map/flows.ts +++ b/src/map/flows.ts @@ -193,7 +193,12 @@ function linkFlows( : 'heuristic'; const roles = new Set(matched.flatMap(({ roles: rs }) => [...rs])); // Prefer a role that maps to a mitigation class over a generic one (a value can reach two args). - const family = [...roles].map((r) => CANDIDATE_FAMILIES[sink.kind]?.[r]).find(Boolean); + // A sink whose package does not establish this API cannot support the mitigation class either: + // labelling a GraphQL `.query()` as the sql-injection family would mis-classify it for any consumer + // that reads `candidateFamily` without also checking `ruleGeneratable`. + const family = sink.apiUnconfirmed + ? undefined + : [...roles].map((r) => CANDIDATE_FAMILIES[sink.kind]?.[r]).find(Boolean); const argumentRole = family ? [...roles].find((r) => CANDIDATE_FAMILIES[sink.kind]?.[r]) : [...roles].find((r) => r !== 'unknown') ?? (proven ? 'unknown' : undefined); @@ -211,6 +216,9 @@ function linkFlows( // from the receiver, so `res.locals.db.query(x)` in a file that happens to import `pg` looks // identical to a real pool — and `res.locals.db` may be any app object. Such sinks stay in the // inventory for review; they just cannot compile a rule that blocks live traffic on a guess. + if (sink.apiUnconfirmed) { + 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)`); + } if (sink.attribution !== 'import' && sink.attribution !== 'global') { reasons.push(sink.attribution === 'inferred' ? `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( ? `dynamic computed key reaches this sink (${l.detail}): the field cannot be named by a parameter` : `spread reaches this sink (${l.detail}): the specific field is not identifiable`); } - if (proven && argumentRole && argumentRole !== 'unknown' && !family) { + // `!sink.apiUnconfirmed`: when the family was withheld because the PACKAGE does not establish this + // API, the role is not what's wrong — role "sql" is normally blockable, and saying otherwise sends a + // reviewer to look at the wrong thing. The package reason above already explains the refusal. + if (proven && argumentRole && argumentRole !== 'unknown' && !family && !sink.apiUnconfirmed) { // e.g. a request value in a parameterized db `values` object: real reachability, but not a // pattern a generic blocking rule can express. reasons.push(`argument role "${argumentRole}" on a ${sink.kind} sink is not a blockable pattern on its own`); diff --git a/src/map/module-graph.ts b/src/map/module-graph.ts index 7ee50dc..f336c49 100644 --- a/src/map/module-graph.ts +++ b/src/map/module-graph.ts @@ -2,7 +2,7 @@ import { readFileSync, realpathSync, statSync } from 'node:fs'; import { dirname, join, relative, resolve as resolvePath } from 'node:path'; import type { Sink, TsModule } from './types.js'; import { guessScriptKind, isFnLike, localCalls } from './ast.js'; -import { buildModuleBindings } from './bindings.js'; +import { buildModuleBindings, npmPackageOf, type Bindings } from './bindings.js'; import { isInside } from './sources.js'; import { collectLocalSinks, type ModuleGraph } from './sinks.js'; @@ -14,17 +14,19 @@ import { collectLocalSinks, type ModuleGraph } from './sinks.js'; const RESOLVE_EXTS = ['.ts', '.tsx', '.mts', '.cts', '.js', '.jsx', '.mjs', '.cjs']; export function createModuleGraph(ts: TsModule, opts: { cwd: string; boundary: string; followOutside?: boolean }): ModuleGraph { - // file → { fnSinks, calleesOf } | null (unreadable/unparseable) - const cache = new Map; calleesOf: Map } | null>(); + // file → { fnSinks, calleesOf, bindings } | null (unreadable/unparseable) + type Entry = { fnSinks: Map; calleesOf: Map; bindings: Bindings }; + const cache = new Map(); const load = (file: string) => { if (cache.has(file)) return cache.get(file) ?? null; - let entry: { fnSinks: Map; calleesOf: Map } | null = null; + let entry: Entry | null = null; try { const text = readFileSync(file, 'utf8'); const sf = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true, guessScriptKind(ts, file)); const bindings = buildModuleBindings(sf, ts); - entry = { fnSinks: collectLocalSinks(sf, ts, bindings), calleesOf: collectCallees(sf, ts) }; + // `bindings` is kept for `importedPackage`: the module's own view of what its exports came from. + entry = { fnSinks: collectLocalSinks(sf, ts, bindings), calleesOf: collectCallees(sf, ts), bindings }; } catch { entry = null; // fail-open: an unreadable dependency must not break the map } @@ -32,17 +34,24 @@ export function createModuleGraph(ts: TsModule, opts: { cwd: string; boundary: s return entry; }; + /** Shared guard: resolve a relative specifier, and refuse to leave the project. */ + const resolveInProject = (fromFile: string, specifier: string): string | undefined => { + const target = resolveRelativeModule(fromFile, specifier); + if (!target) return undefined; + // Stay inside the project: `../../other-repo/db` (or a symlink) would otherwise pull an unrelated + // codebase into this app's attack surface. The primary walker enforces this; so must the resolver. + if (!opts.followOutside) { + let real = target; + try { real = realpathSync(target); } catch { /* use as-is */ } + if (!isInside(real, opts.boundary)) return undefined; + } + return target; + }; + return { importedSinks(fromFile, specifier, exportName) { - const target = resolveRelativeModule(fromFile, specifier); + const target = resolveInProject(fromFile, specifier); if (!target) return []; - // Stay inside the project: `../../other-repo/db` (or a symlink) would otherwise pull an unrelated - // codebase into this app's attack surface. The primary walker enforces this; so must the resolver. - if (!opts.followOutside) { - let real = target; - try { real = realpathSync(target); } catch { /* use as-is */ } - if (!isInside(real, opts.boundary)) return []; - } const mod = load(target); if (!mod) return []; const collected = [...(mod.fnSinks.get(exportName) ?? [])]; @@ -55,6 +64,22 @@ export function createModuleGraph(ts: TsModule, opts: { cwd: string; boundary: s const rel = relative(opts.cwd, target); return collected.map((s) => ({ ...s, file: rel })); }, + + // A different question about the same module: not "what sinks are in there" but "what does this + // export TRACE TO". The common AI-built layout puts the client in a lib file — + // `export const db = createClient(...)` in `lib/db.ts`, imported everywhere — so the receiver of + // `db.from('orders').insert(...)` resolves to a relative specifier and nothing else. Refusing it (as + // an unattributable receiver) is right for app code but wrong here: one hop away it is a real + // dependency, and that chain is import-to-import, fully static — evidence, not inference. + importedPackage(fromFile, specifier, exportName) { + const target = resolveInProject(fromFile, specifier); + if (!target) return undefined; + const mod = load(target); + if (!mod) return undefined; + // The target module's own bindings answer it: `db` there resolves through + // `const db = createClient(...)` back to the package `createClient` was imported from. + return npmPackageOf(mod.bindings.resolve(exportName)); + }, }; } diff --git a/src/map/sinks.ts b/src/map/sinks.ts index 050ca5f..73a885f 100644 --- a/src/map/sinks.ts +++ b/src/map/sinks.ts @@ -20,6 +20,21 @@ const HTTP_MEMBER_METHODS = new Set(['get', 'post', 'put', 'patch', 'delete', 'h // When a sink's base can't be traced precisely, infer its package from the file's imports of a known // provider for that sink kind (a file almost always uses one db/http client). const DB_PACKAGES = ['@supabase/supabase-js', '@prisma/client', 'drizzle-orm', 'knex', 'kysely', 'pg', 'mysql2', 'mysql', 'sequelize', 'typeorm', 'mongoose', 'better-sqlite3']; +// Drivers that are not in the inference list above but do establish a database API when a receiver +// resolves to them. Extend deliberately: this list is what separates "a package we can trace" from +// "a package that proves a DB API", and admitting the wrong one produces a false SQL-injection rule. +const MORE_DB_PACKAGES = ['postgres', 'mssql', 'tedious', 'oracledb', 'sqlite3', 'mongodb', 'ioredis', + '@planetscale/database', '@neondatabase/serverless', '@libsql/client', '@vercel/postgres', 'slonik', 'sql.js']; +/** + * Does `pkg` establish a DATABASE api? Package provenance is not API provenance: `.query()` is a generic + * method name, and an `@apollo/client` (or any HTTP-ish client) instance resolves to a real package while + * having nothing to do with SQL. Without this gate, `client.query(req.body.sql)` compiled a precise + * SQL-injection candidate for a GraphQL call — a rule that blocks legitimate traffic and mitigates nothing. + * Subpath imports count (`drizzle-orm/node-postgres`). + */ +const isDbPackage = (pkg: string) => + DB_PACKAGES.includes(pkg) || MORE_DB_PACKAGES.includes(pkg) || + [...DB_PACKAGES, ...MORE_DB_PACKAGES].some((p) => pkg.startsWith(p + '/')); const HTTP_PACKAGES = ['axios', 'got', 'node-fetch', 'undici', 'superagent', 'ky']; const isHttpPackage = (pkg: string) => HTTP_PACKAGES.includes(pkg) || pkg === 'node:http' || pkg === 'node:https'; // A filesystem/process API is not only a node: builtin — these wrappers expose the same sinks, and @@ -32,6 +47,12 @@ const isExecPackage = (pkg: string) => pkg === 'node:child_process' || EXEC_PACK export interface ModuleGraph { /** Sinks of `exportName` in the module `specifier` resolves to, relative to `fromFile`. */ importedSinks(fromFile: string, specifier: string, exportName: string): Sink[]; + /** + * The npm package `exportName` traces to inside the module `specifier` resolves to — for a client + * instance re-exported from a local module (`export const db = createClient(...)`). ONE hop: a + * re-export chain (`export { db } from './client'`) is not followed. + */ + importedPackage(fromFile: string, specifier: string, exportName: string): string | undefined; } export interface SinkContext { @@ -43,14 +64,14 @@ export interface SinkContext { } // --- sinks (agnostic) ------------------------------------------------------- -export function collectLocalSinks(sf: any, ts: TsModule, bindings: Bindings): Map { +export function collectLocalSinks(sf: any, ts: TsModule, bindings: Bindings, ctx?: SinkContext): Map { const map = new Map(); const visit = (node: any) => { - if (ts.isFunctionDeclaration(node) && node.name && node.body) map.set(node.name.text, directSinks(node.body, ts, bindings)); + if (ts.isFunctionDeclaration(node) && node.name && node.body) map.set(node.name.text, directSinks(node.body, ts, bindings, ctx)); else if (ts.isVariableStatement(node)) { for (const decl of node.declarationList.declarations) { if (ts.isIdentifier(decl.name) && decl.initializer && isFnLike(decl.initializer, ts)) { - map.set(decl.name.text, directSinks(decl.initializer.body, ts, bindings)); + map.set(decl.name.text, directSinks(decl.initializer.body, ts, bindings, ctx)); } } } @@ -65,7 +86,7 @@ export function sinksFrom(arrowOrNode: any, ts: TsModule, localSinks: Map // it: resolved precisely from the call's base identifier via the file's imports, else inferred from // the file's imports of a known provider for that sink kind. A receiver that traces to a plain local // object/class/function is NOT a dependency sink and is dropped. -function directSinks(node: any, ts: TsModule, bindings: Bindings): Sink[] { +function directSinks(node: any, ts: TsModule, bindings: Bindings, ctx?: SinkContext): Sink[] { const sinks: Sink[] = []; // `spec` is the raw module specifier the receiver came from, which `pkg` cannot express: a RELATIVE // specifier yields no package, and that is a positive fact (the receiver is app code) rather than the @@ -122,6 +143,16 @@ function directSinks(node: any, ts: TsModule, bindings: Bindings): Sink[] { const relative = spec !== undefined && (spec.startsWith('.') || spec.startsWith('/')); const pkg = npmPackageOf(spec); if (pkg) return { pkg, root, spec }; + // A RELATIVE receiver is app code — unless one hop away it is a dependency. `import { db } from + // './lib/db'` where that module does `export const db = createClient(...)` is the most common layout + // in generated apps, and treating it as app code made the sink vanish entirely. Ask the target module + // what the export traces to: a package means the receiver IS that dependency (an import-to-import + // chain, so `attribution: 'import'`), and NO package means it stays app code — which is what keeps + // `import * as helper from './util'; helper.exec(x)` correctly sink-free. + if (relative && ctx && spec) { + const viaModule = ctx.graph.importedPackage(ctx.file, spec, bindings.exportNameOf(root) ?? root); + if (viaModule) return { pkg: viaModule, root, spec }; + } return { local: bindings.locals.has(root), root, spec, relative }; }; // Whether `package` is evidence or a guess. A resolved import binding is evidence ('import'); a @@ -129,6 +160,21 @@ function directSinks(node: any, ts: TsModule, bindings: Bindings): Sink[] { // untraceable receiver is neither (undefined) and must not drive an auto-generated rule. const attributionOf = (b: { pkg?: string }, pkg: string | undefined): Sink['attribution'] => b.pkg ? 'import' : pkg ? 'inferred' : undefined; + /** + * Claim `provider: 'sql'` only when the package actually establishes a database API. A traced package + * that is not a DB provider stays in the INVENTORY (a `.query()` on it is worth a human's attention) + * but is marked so no rule can be compiled from it — and it does not get to call itself SQL. + */ + const dbApi = (pkg: string | undefined, attribution: Sink['attribution']): { provider?: string; apiUnconfirmed?: true } => { + if (pkg && !isDbPackage(pkg)) return { apiUnconfirmed: true }; + // `provider` is a claim about the API at THIS call site, so it needs the receiver — not just the file. + // An INFERRED package means "this file talks to pg", never "this receiver is a pg client", and + // `res.locals.db.query(x)` in a file that imports pg is exactly that. The flow is already refused; + // asserting `provider: 'sql'` anyway would overstate it in the inventory, where a human reads it. + // `package` still carries the hint, and `attribution` already says how strong it is — deriving the + // claim from it here keeps one source of truth rather than a second confidence field to drift. + return attribution === 'import' || attribution === 'global' ? { provider: 'sql' } : {}; + }; const infer = (kind: 'db' | 'http'): string | undefined => { const table = kind === 'db' ? DB_PACKAGES : HTTP_PACKAGES; for (const p of table) if (bindings.imports.has(p)) return p; @@ -152,7 +198,8 @@ function directSinks(node: any, ts: TsModule, bindings: Bindings): Sink[] { const parent = n.parent; if (!b.local && !b.relative && parent && ts.isPropertyAccessExpression(parent) && DB_OPS.has(parent.name.text)) { const pkg = b.pkg ?? infer('db'); - push({ kind: 'db', provider: 'sql', package: pkg, table, op: parent.name.text, attribution: attributionOf(b, pkg), ...spanOf(opCallOf(parent, ts)) }); + const attribution = attributionOf(b, pkg); + push({ kind: 'db', ...dbApi(pkg, attribution), package: pkg, table, op: parent.name.text, attribution, ...spanOf(opCallOf(parent, ts)) }); } } if (ts.isPropertyAccessExpression(callee)) { @@ -169,13 +216,16 @@ function directSinks(node: any, ts: TsModule, bindings: Bindings): Sink[] { if (PRISMA_OPS.has(method) && ts.isPropertyAccessExpression(callee.expression)) { const prismaLikely = b.pkg === '@prisma/client' || (!b.pkg && (bindings.imports.has('@prisma/client') || /prisma/i.test(b.root ?? ''))); - if (prismaLikely) push({ kind: 'db', provider: 'prisma', package: '@prisma/client', table: callee.expression.name.text, op: method, attribution: b.pkg ? 'import' : 'inferred', ...spanOf(n) }); + // Same rule for the provider claim: only a resolved receiver earns `provider: 'prisma'`; a + // prisma-NAMED receiver is a hint, and `package` + `attribution` already say so. + if (prismaLikely) push({ kind: 'db', ...(b.pkg ? { provider: 'prisma' } : {}), package: '@prisma/client', table: callee.expression.name.text, op: method, attribution: b.pkg ? 'import' : 'inferred', ...spanOf(n) }); } // db: raw `.query(` / `.execute(`. Any object can have a `.query` method, so an untraceable // receiver stays in the inventory with NO attribution — visible to a human, never auto-ruled. if (method === 'query' || method === 'execute') { const pkg = b.pkg ?? infer('db'); - push({ kind: 'db', provider: 'sql', package: pkg, op: method, attribution: attributionOf(b, pkg), ...spanOf(n) }); + const attribution = attributionOf(b, pkg); + push({ kind: 'db', ...dbApi(pkg, attribution), package: pkg, op: method, attribution, ...spanOf(n) }); } // fs / exec via a namespace: `fs.writeFile(` / `child_process.exec(`. The receiver must // actually resolve to a filesystem/process package — the method name alone proves nothing. diff --git a/src/map/types.ts b/src/map/types.ts index cead235..5ca5912 100644 --- a/src/map/types.ts +++ b/src/map/types.ts @@ -96,6 +96,20 @@ export interface Sink { * never drive an auto-generated rule, since any object can own a method by that name. */ attribution?: 'import' | 'global' | 'inferred'; + /** + * Note on `provider` + `attribution` together: `provider` is a claim about the API at this call site, so + * it is only set when the RECEIVER was traced (`attribution: 'import'`/`'global'`). An `inferred` package + * says "this file talks to pg", not "this receiver is a pg client", so no provider is claimed — read + * `package` as a hint in that case. There is deliberately no separate provider-confidence field: it + * would be derived from these two and could drift out of step with them. + */ + /** + * Set when the receiver resolved to a real package that does **not** establish this kind of API — + * package provenance is not API provenance. `client.query(x)` on an `@apollo/client` instance traces to + * a genuine dependency while having nothing to do with SQL. Such a sink is reported for review and can + * never compile a rule: a candidate here would block legitimate traffic and mitigate nothing. + */ + apiUnconfirmed?: true; /** * Character span of the sink's operation call in `file` (or the endpoint's file). This is the sink's * IDENTITY: flow analysis binds evidence to this exact call, never to a line or an enclosing diff --git a/tests/map/corpus.test.ts b/tests/map/corpus.test.ts index 8e5eb7b..5814cf0 100644 --- a/tests/map/corpus.test.ts +++ b/tests/map/corpus.test.ts @@ -156,6 +156,28 @@ const ADVERSARIAL: Case[] = [ // The schema's `post:id` is declared but never read by the sink — proven-nothing, not blockable. expectRefused: [['id', /no proven local read/]], }, + { + name: 'adversarial: a traced package that does not establish the API', + kind: 'adversarial', + pkg: { dependencies: { express: '4', '@apollo/client': '3' } }, + files: { + // `.query()` is a generic method name. An ApolloClient instance resolves to a REAL dependency, so + // attribution alone admits it — and a GraphQL call became a precise SQL-injection candidate. Package + // provenance is not API provenance. + 'src/lib/gql.ts': ` + import { ApolloClient } from "@apollo/client"; + export const client = new ApolloClient({ uri: "https://api.example.com" }); + `, + 'src/server.ts': ` + import express from "express"; + import { client } from "./lib/gql"; + const app = express(); + app.post("/graphql", async (req, res) => { await client.query(req.body.sql); res.end(); }); + `, + }, + expectCandidates: [], + expectRefused: [['sql', /does not establish a db API/]], + }, { name: 'adversarial: sibling expressions must not contaminate each other', kind: 'adversarial', @@ -292,6 +314,34 @@ const CASES: Case[] = [ }, expectCandidates: [], }, + { + name: 'express + a client in lib/ (the layout generated apps actually use)', + pkg: { dependencies: { express: '4', '@supabase/supabase-js': '2', pg: '8' } }, + files: { + // The handler's file imports the CLIENT, not the driver. The receiver therefore resolves to a + // relative specifier, and treating that as app code made these sinks vanish — which also broke the + // package join a server needs to connect a CVE in `pg` to the endpoint that reaches it. + 'src/lib/db.ts': ` + import { createClient } from "@supabase/supabase-js"; + export const db = createClient(process.env.URL, process.env.KEY); + `, + 'src/lib/pool.ts': ` + import { Pool } from "pg"; + export const pool = new Pool({ connectionString: process.env.DATABASE_URL }); + `, + 'src/server.ts': ` + import express from "express"; + import { db } from "./lib/db"; + import { pool } from "./lib/pool"; + const app = express(); + app.post("/tasks", async (req, res) => { await db.from("tasks").insert({ title: req.body.title }); res.end(); }); + app.post("/report", async (req, res) => { await pool.query(req.body.sql); res.end(); }); + `, + }, + // The raw query is a blockable pattern; the inserted row value is context, not a rule. + expectCandidates: ['sql-injection @ post.sql'], + expectRefused: [['title', /not a blockable pattern/]], + }, ]; const ALL: Case[] = [...CASES, ...ADVERSARIAL]; @@ -395,9 +445,9 @@ describe('golden corpus', () => { it('keeps a standing adversarial category (lookalikes are how every false candidate got in)', () => { // Guards against the category quietly emptying out; the classes listed are the ones that have // actually produced false candidates, so losing one should fail loudly. - expect(ADVERSARIAL.length).toBeGreaterThanOrEqual(6); + expect(ADVERSARIAL.length).toBeGreaterThanOrEqual(7); const names = ADVERSARIAL.map((c) => c.name).join(' | '); - for (const cls of ['collide with dangerous API names', 'untraceable receivers', 'shadowing dangerous globals', 'two request namespaces', 'sibling expressions', 'different namespaces']) { + 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']) { expect(names, `missing adversarial class: ${cls}`).toContain(cls); } }); diff --git a/tests/map/imported-client.test.ts b/tests/map/imported-client.test.ts new file mode 100644 index 0000000..f6cb11c --- /dev/null +++ b/tests/map/imported-client.test.ts @@ -0,0 +1,225 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, symlinkSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { buildInputMap } from '../../src/map/index.js'; + +// The client almost never lives in the handler's file. Generated apps put it in `lib/db.ts` and import +// it everywhere, so the receiver of `db.from('orders').insert(...)` resolves to a RELATIVE specifier — +// which the attributable-receiver rule correctly treats as app code, and which therefore made the sink +// disappear entirely. One hop away it is a real dependency, and that chain is import-to-import, so it is +// evidence rather than inference. The negative cases below are the reason this is narrow: a relative +// receiver only becomes a dependency when the export actually TRACES to one. +let dir: string; +beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'ps-impclient-')); + mkdirSync(join(dir, 'src', 'lib'), { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ + dependencies: { express: '4', '@supabase/supabase-js': '2', pg: '8' }, + })); + writeFileSync(join(dir, 'src', 'lib', 'db.ts'), ` + import { createClient } from "@supabase/supabase-js"; + export const db = createClient("u", "k"); + `); + writeFileSync(join(dir, 'src', 'lib', 'pool.ts'), ` + import { Pool } from "pg"; + export const pool = new Pool(); + `); + // App code whose export names collide with dangerous APIs and trace to NOTHING. + writeFileSync(join(dir, 'src', 'lib', 'util.ts'), ` + export function exec(x: string) { return x.length } + export function query(x: string) { return x } + export function from(x: string) { return x } + `); + // A re-export chain: deliberately NOT followed (one hop only). + writeFileSync(join(dir, 'src', 'lib', 'reexport.ts'), `export { db } from "./db";`); + writeFileSync(join(dir, 'src', 'server.ts'), ` + import express from "express"; + import { db } from "./lib/db"; + import { pool } from "./lib/pool"; + import { db as renamed } from "./lib/db"; + import * as helper from "./lib/util"; + import { db as chained } from "./lib/reexport"; + const app = express(); + app.post("/orders", async (req, res) => { await db.from("orders").insert({ title: req.body.title }); res.end(); }); + app.post("/sql", async (req, res) => { await pool.query(req.body.sql); res.end(); }); + app.post("/renamed", async (req, res) => { await renamed.from("t").insert({ v: req.body.v }); res.end(); }); + app.post("/lookalike", (req, res) => { helper.exec(req.body.cmd); helper.query(req.body.sql); helper.from("t").insert({ v: req.body.v }); res.end(); }); + app.post("/chained", async (req, res) => { await chained.from("t").insert({ v: req.body.v }); res.end(); }); + `); +}); +afterAll(() => rmSync(dir, { recursive: true, force: true })); + +const ep = async (route: string) => { + const { map } = await buildInputMap(dir); + return map!.endpoints.find((e) => e.route === route)!; +}; + +describe('a client imported from a local module', () => { + it('resolves to the dependency it was created from', async () => { + const e = await ep('/orders'); + const sink = e.sinks.find((s) => s.kind === 'db'); + expect(sink).toBeDefined(); + expect(sink!.package).toBe('@supabase/supabase-js'); + // Not `inferred`: the receiver itself traces there, through this file's import and that module's. + expect(sink!.attribution).toBe('import'); + // The sink call is in THIS file, so it keeps local call-site evidence (no `file` override). + expect(sink!.file).toBeUndefined(); + }); + + it('lets a real candidate compile that was previously invisible', async () => { + const e = await ep('/sql'); + const sink = e.sinks.find((s) => s.kind === 'db')!; + expect(sink.package).toBe('pg'); + const flow = e.flows.find((f) => f.inputId === 'post:sql')!; + expect(flow.confidence).toBe('exact-local'); + expect(flow.ruleGeneratable).toBe(true); + expect(flow.candidateFamily).toBe('sql-injection'); + }); + + it('follows a renamed import via its exported name', async () => { + const e = await ep('/renamed'); + expect(e.sinks.map((s) => `${s.kind}/${s.package}`)).toEqual(['db/@supabase/supabase-js']); + }); + + it('does NOT resurrect lookalikes: an export that traces to nothing stays app code', async () => { + const e = await ep('/lookalike'); + // `exec`, `query` and even `from(...).insert(...)` here are ordinary local functions. + expect(e.sinks).toEqual([]); + expect(e.flows.filter((f) => f.ruleGeneratable)).toEqual([]); + // The inputs are still reported — the surface is real, the sink was not. + expect(e.inputs.map((i) => i.name).sort()).toEqual(['cmd', 'sql', 'v']); + }); + + it('stops at one hop: a re-export chain is not followed (documented limitation)', async () => { + const e = await ep('/chained'); + expect(e.sinks).toEqual([]); + }); +}); + +describe('the project boundary still holds', () => { + it('does not resolve a client through a symlink that leaves the project', async () => { + const outside = mkdtempSync(join(tmpdir(), 'ps-outside-')); + writeFileSync(join(outside, 'secretdb.ts'), ` + import { createClient } from "@supabase/supabase-js"; + export const db = createClient("u", "k"); + `); + const inside = mkdtempSync(join(tmpdir(), 'ps-inside-')); + mkdirSync(join(inside, 'src'), { recursive: true }); + writeFileSync(join(inside, 'package.json'), JSON.stringify({ dependencies: { express: '4', '@supabase/supabase-js': '2' } })); + symlinkSync(join(outside, 'secretdb.ts'), join(inside, 'src', 'linked.ts')); + writeFileSync(join(inside, 'src', 'server.ts'), ` + import express from "express"; + import { db } from "./linked"; + const app = express(); + app.post("/x", async (req, res) => { await db.from("t").insert({ v: req.body.v }); res.end(); }); + `); + const { map } = await buildInputMap(inside); + const e = map!.endpoints.find((x) => x.route === '/x')!; + expect(e.sinks).toEqual([]); // the resolver refuses to leave the project + rmSync(inside, { recursive: true, force: true }); + rmSync(outside, { recursive: true, force: true }); + }); +}); + +// Package provenance is NOT API provenance. `.query()` is a generic method name: an `@apollo/client` +// instance resolves to a genuine dependency while having nothing to do with SQL, and admitting that as a +// database sink compiled a precise SQL-injection candidate for a GraphQL call — a rule that would block +// legitimate traffic and mitigate nothing. This predates the imported-client hop (a same-file +// `new ApolloClient()` hit it too); the hop only made it easier to reach. +describe('a traced package must also establish the API', () => { + let d: string; + beforeAll(() => { + d = mkdtempSync(join(tmpdir(), 'ps-api-')); + mkdirSync(join(d, 'src', 'lib'), { recursive: true }); + writeFileSync(join(d, 'package.json'), JSON.stringify({ + dependencies: { express: '4', '@apollo/client': '3', pg: '8', 'drizzle-orm': '0.30' }, + })); + writeFileSync(join(d, 'src', 'lib', 'gql.ts'), ` + import { ApolloClient } from "@apollo/client"; + export const client = new ApolloClient({ uri: "https://api.example.com" }); + `); + writeFileSync(join(d, 'src', 'lib', 'pool.ts'), ` + import { Pool } from "pg"; + export const pool = new Pool(); + `); + writeFileSync(join(d, 'src', 'lib', 'orm.ts'), ` + import { drizzle } from "drizzle-orm/node-postgres"; + export const orm = drizzle({}); + `); + writeFileSync(join(d, 'src', 'server.ts'), ` + import express from "express"; + import { ApolloClient } from "@apollo/client"; + import { Pool } from "pg"; + import { client } from "./lib/gql"; + import { pool } from "./lib/pool"; + import { orm } from "./lib/orm"; + const app = express(); + const inline = new ApolloClient({ uri: "https://api.example.com" }); + app.post("/gql-imported", async (req, res) => { await client.query(req.body.sql); res.end(); }); + app.post("/gql-inline", async (req, res) => { await inline.query(req.body.sql); res.end(); }); + app.post("/pg", async (req, res) => { await pool.query(req.body.sql); res.end(); }); + app.post("/orm", async (req, res) => { await orm.execute(req.body.sql); res.end(); }); + // The direct \`pg\` import above is what makes this an INFERRED package rather than an untraceable + // one: the file demonstrably talks to pg, but nothing traces \`res.locals.db\` to it. + app.post("/untraced", async (req, res) => { await res.locals.db.query(req.body.sql); res.end(); }); + `); + }); + afterAll(() => rmSync(d, { recursive: true, force: true })); + + const route = async (r: string) => { + const { map } = await buildInputMap(d); + return map!.endpoints.find((e) => e.route === r)!; + }; + + it.each(['/gql-imported', '/gql-inline'])('refuses a rule for a non-DB client at %s', async (r) => { + const e = await route(r); + const sink = e.sinks.find((s) => s.kind === 'db')!; + expect(sink.package).toBe('@apollo/client'); + expect(sink.attribution).toBe('import'); // the package IS established… + expect(sink.apiUnconfirmed).toBe(true); // …but it does not establish a DB API + expect(sink.provider).toBeUndefined(); // so it does not get to call itself SQL either + const flow = e.flows.find((f) => f.sink.kind === 'db')!; + expect(flow.confidence).toBe('exact-local'); // the data really does reach it + expect(flow.ruleGeneratable).toBe(false); + expect(flow.candidateFamily).toBeUndefined(); // and it must not advertise a class it cannot support + expect(flow.ruleGeneratableReasons!.join(' ')).toMatch(/does not establish a db API/); + // …and ONLY that. Withholding the family must not make the role look like the problem: role "sql" is + // normally blockable, so that reason would send a reviewer to look at the wrong thing. + expect(flow.ruleGeneratableReasons).toHaveLength(1); + expect(flow.ruleGeneratableReasons!.join(' ')).not.toMatch(/not a blockable pattern/); + }); + + it('claims a provider only when the RECEIVER was traced, not just the package', async () => { + // `res.locals.db.query(x)` in a file that imports pg: the package is inferred from the file, so the + // sink must not assert `provider: 'sql'` about a receiver nobody traced. The flow was already refused + // for the inferred attribution; this is about not overstating it in the inventory, where a human reads + // it. No separate confidence field — `attribution` already carries the strength. + const e = await route('/untraced'); + const sink = e.sinks.find((s) => s.kind === 'db')!; + expect(sink.package).toBe('pg'); // the hint survives + expect(sink.attribution).toBe('inferred'); + expect(sink.provider).toBeUndefined(); // …but the API claim does not + expect(sink.apiUnconfirmed).toBeUndefined(); // and this is NOT the "wrong package" case + expect(e.flows.every((f) => f.ruleGeneratable === false)).toBe(true); + }); + + it('keeps the sink in the inventory — a .query() on an unknown client is worth a human look', async () => { + const e = await route('/gql-imported'); + expect(e.sinks).toHaveLength(1); + }); + + it.each([ + ['/pg', 'pg'], + ['/orm', 'drizzle-orm'], + ])('still generates for a real driver at %s', async (r, pkg) => { + const e = await route(r); + const sink = e.sinks.find((s) => s.kind === 'db')!; + expect(sink.package).toBe(pkg); // subpath imports resolve to the package + expect(sink.apiUnconfirmed).toBeUndefined(); + expect(sink.provider).toBe('sql'); + const flow = e.flows.find((f) => f.sink.kind === 'db')!; + expect(flow.ruleGeneratable).toBe(true); + expect(flow.candidateFamily).toBe('sql-injection'); + }); +}); diff --git a/tests/map/sink-attribution-members.test.ts b/tests/map/sink-attribution-members.test.ts index 0fd6dfc..2732939 100644 --- a/tests/map/sink-attribution-members.test.ts +++ b/tests/map/sink-attribution-members.test.ts @@ -194,8 +194,13 @@ describe('an inferred package does not license a rule', () => { it('refuses the prisma-shaped path on an untraceable receiver', async () => { const e = await route('/prisma'); - const sink = e.sinks.find((s) => s.provider === 'prisma'); - expect(sink?.attribution).toBe('inferred'); + // Found by PACKAGE, not by provider: a prisma-shaped call on an untraceable receiver deliberately + // makes no provider claim — `provider` describes the API at this call site, and that needs the + // receiver. The package remains as the hint that made us look. + const sink = e.sinks.find((s) => s.package === '@prisma/client'); + expect(sink).toBeDefined(); + expect(sink!.attribution).toBe('inferred'); + expect(sink!.provider).toBeUndefined(); expect(e.flows.filter((f) => f.ruleGeneratable)).toEqual([]); });