diff --git a/src/map/coordinates.ts b/src/map/coordinates.ts index 0404f9e..844c8e1 100644 --- a/src/map/coordinates.ts +++ b/src/map/coordinates.ts @@ -40,6 +40,19 @@ export function runtimeCoordinate(source: InputSource | undefined, path: string) } } +/** + * The rule-engine NAMESPACE an input lands in (`post`, `get`, `cookie`, `files`, `server`), or null when + * it has no address. Derived from `runtimeCoordinate` on purpose: comparing raw source labels would call + * `json-body` and an Express `req.body` read different places when both resolve to `post.*`, and would + * miss that `post.id` and `get.id` are genuinely different places. + */ +export function namespaceOf(source: InputSource | undefined, path: string): string | null { + const { runtimeParameter } = runtimeCoordinate(source, path); + if (!runtimeParameter) return null; + const dot = runtimeParameter.indexOf('.'); + return dot === -1 ? runtimeParameter : runtimeParameter.slice(0, dot); +} + /** Attach `source` + the runtime coordinate to every extracted input. */ export function withCoordinates(fields: InputField[], source: InputSource): InputField[] { return fields.map((f) => ({ ...f, source: f.source ?? source, ...runtimeCoordinate(f.source ?? source, f.name) })); diff --git a/src/map/entries.ts b/src/map/entries.ts index 376633d..22ef141 100644 --- a/src/map/entries.ts +++ b/src/map/entries.ts @@ -4,13 +4,13 @@ import type { Bindings } from './bindings.js'; import { functionNameFromPath, ROUTE_REGISTER, routeFromChain, routeObject } from './routes.js'; import { withCoordinates } from './coordinates.js'; import { inputsFromHandler, inputsFromValidator } from './inputs.js'; -import { sinksFrom, type ModuleGraph } from './sinks.js'; +import { sinksFrom, type SinkContext } from './sinks.js'; import { linkedFlows } from './flows.js'; const HTTP_METHODS = new Set(['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'HEAD', 'OPTIONS']); // --- entry-point recognizers ----------------------------------------------- -export function extractFromFile(sf: any, ts: TsModule, localSinks: Map, bindings: Bindings, ctx: { file: string; graph: ModuleGraph }): Omit[] { +export function extractFromFile(sf: any, ts: TsModule, localSinks: Map, bindings: Bindings, ctx: SinkContext): Omit[] { const out: Omit[] = []; const isServerActionsFile = fileHasUseServer(sf, ts); @@ -134,7 +134,7 @@ function handlerEntry( ts: TsModule, localSinks: Map, bindings: Bindings, - ctx: { file: string; graph: ModuleGraph }, + ctx: SinkContext, extra: { method?: string; route?: string; line?: number; start?: number; end?: number } = {}, ): Omit { const entryKind = kindLabel === 'route-registration' || kindLabel === 'server-action' || kindLabel === 'edge-function' diff --git a/src/map/extract.ts b/src/map/extract.ts index 0e4611c..a6731a1 100644 --- a/src/map/extract.ts +++ b/src/map/extract.ts @@ -50,8 +50,8 @@ export async function extractInputMap(cwd: string, ts: TsModule, options: Extrac const sf = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true, guessScriptKind(ts, file)); const bindings = buildModuleBindings(sf, ts); const localSinks = collectLocalSinks(sf, ts, bindings); - for (const ep of extractFromFile(sf, ts, localSinks, bindings, { file, graph })) { - const relFile = relative(cwd, file); + const relFile = relative(cwd, file); + for (const ep of extractFromFile(sf, ts, localSinks, bindings, { file, owner: relFile, graph })) { // 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 68d07da..3fbaaba 100644 --- a/src/map/flows.ts +++ b/src/map/flows.ts @@ -155,6 +155,16 @@ function linkFlows( if (!input.runtimeParameter) reasons.push(input.runtimeParameterReason ?? 'input has no runtime parameter'); if (sink.file !== undefined) reasons.push('sink is in an imported module: no local call-site evidence'); if (sink.start === undefined) reasons.push('sink call could not be located in the source'); + // Only a receiver traced to a dependency ('import') or a genuine runtime global earns a rule. + // 'inferred' is deliberately NOT enough: the package came from some OTHER import in the file, not + // 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.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` + : `sink receiver could not be traced to a dependency (${sink.kind}.${sink.op ?? '?'} on an unresolved receiver): a rule here would be a guess`); + } if (precise && argumentRole === 'unknown') reasons.push(`sink argument role is not modelled for ${sink.kind}.${sink.op ?? '?'}`); // A dynamic key or a spread in this sink's arguments means no coordinate can name the field that // actually reaches it — report the specific cause rather than a generic "heuristic". diff --git a/src/map/inputs.ts b/src/map/inputs.ts index b8e5ef8..5ada8d1 100644 --- a/src/map/inputs.ts +++ b/src/map/inputs.ts @@ -1,7 +1,7 @@ import type { InputField, InputSource, TsModule } from './types.js'; import { bindingKey, rootIdentifier } from './ast.js'; import { npmPackageOf, type Bindings } from './bindings.js'; -import { runtimeCoordinate, withCoordinates } from './coordinates.js'; +import { namespaceOf, runtimeCoordinate, withCoordinates } from './coordinates.js'; const ZOD_BASE = new Set(['string', 'number', 'boolean', 'array', 'object', 'enum', 'bigint', 'date', 'record']); // String-format refinements a validator can declare — kept on the field so a rule can pin the shape. @@ -26,13 +26,45 @@ export function inputsFromHandler( ): InputField[] { // A validated schema inside a handler describes the request body — except for a payload-style entry // (a server action), where the schema describes the action's own argument. - const fields = withCoordinates(zodObjectFields(body, ts, bindings), opts.validatorSource ?? 'json-body'); - const names = new Set(fields.map((f) => f.name)); - for (const { name, source } of requestMemberAccesses(params, body, ts, opts)) { - if (!names.has(name)) { - names.add(name); - fields.push({ name, source, ...runtimeCoordinate(source, name) }); + const schemaSource = opts.validatorSource ?? 'json-body'; + const schemaFields = zodObjectFields(body, ts, bindings); + const reads = requestMemberAccesses(params, body, ts, opts); + + // ONE collision pass over both origins. A schema field and a request read can share a name while + // addressing different places — `z.object({ id }).parse(req.body)` next to `fs.read(req.query.id)` + // used to yield the schema's `post.id` while the sink actually consumed `get.id`, so a rule inspected + // a parameter the payload never travels in. Grouping by name is what makes that invisible, so the + // grouping now carries every source, and the verdict compares effective NAMESPACES: `json-body` and + // an Express `req.body` read are both `post.*` (not a conflict), `post.id` vs `get.id` is. + const sourcesByName = new Map(); + const add = (name: string, source: InputSource) => { + const list = sourcesByName.get(name) ?? []; + if (!list.includes(source)) list.push(source); + sourcesByName.set(name, list); + }; + for (const f of schemaFields) add(f.name, f.source ?? schemaSource); + for (const r of reads) for (const s of r.sources) add(r.name, s); + + const coordFor = (name: string, primary: InputSource) => { + const sources = sourcesByName.get(name) ?? [primary]; + if (new Set(sources.map((s) => namespaceOf(s, name))).size > 1) { + return { + runtimeParameter: null, + runtimeParameterReason: `field is read from more than one request namespace (${sources.join(', ')}): no single parameter addresses it`, + }; } + return runtimeCoordinate(primary, name); + }; + + const fields: InputField[] = withCoordinates(schemaFields, schemaSource) + .map((f) => ({ ...f, ...coordFor(f.name, f.source ?? schemaSource) })); + const names = new Set(fields.map((f) => f.name)); + for (const { name, sources } of reads) { + if (names.has(name)) continue; + const primary = sources[0]; // first-seen, so the reported source is deterministic + if (!primary) continue; + names.add(name); + fields.push({ name, source: primary, ...coordFor(name, primary) }); } return fields; } @@ -117,9 +149,19 @@ function requestMemberAccesses( body: any, ts: TsModule, opts: { payloadParam?: boolean } = {}, -): Array<{ name: string; source: InputSource }> { +): Array<{ name: string; sources: InputSource[] }> { if (!body) return []; - const out = new Map(); + // Keyed by FIELD NAME, which two namespaces can share (`params.id` and `query.id` in one handler). + // Last-write-wins silently picked one, and since the pick decided the coordinate, a handler reading + // both compiled a rule pinned to `get.id` for data that arrives in the path segment — a wrong-input + // pin, the one failure this whole layer exists to prevent. Collisions are now recorded, and the + // first-seen source wins so the record is at least deterministic. + const out = new Map(); + const record = (name: string, source: InputSource) => { + const list = out.get(name) ?? []; + if (!list.includes(source)) list.push(source); + out.set(name, list); + }; const p0 = params?.[0]; const reqName = p0 && ts.isIdentifier(p0.name) ? p0.name.text : undefined; // Identifiers that ARE a request-input object (destructured `({ body })` param, `await req.json()`), @@ -157,25 +199,34 @@ function requestMemberAccesses( const visit = (n: any) => { // . if (ts.isPropertyAccessExpression(n) && isReqSourceExpr(n.expression)) { - out.set(n.name.text, sourceOfExpr(n.expression)); + record(n.name.text, sourceOfExpr(n.expression)); } if (ts.isVariableDeclaration(n) && n.initializer) { const init = unwrap(n.initializer); // const b = await request.json() → b is a request-input object from here on. if (ts.isIdentifier(n.name) && isBodyReadCall(n.initializer)) sourceNames.set(n.name.text, bodyReadSource(n.initializer)); + // const { query: q } = req → the SAME namespace capture as a destructured handler param, just one + // statement later. Without this the fields read off `q` are invisible: no coordinate is emitted + // (so nothing is mis-addressed) but the surface goes unreported, which reads as "nothing here". + if (ts.isObjectBindingPattern(n.name) && ts.isIdentifier(init) && reqName && init.text === reqName) { + for (const el of n.name.elements) { + const key = bindingKey(el, ts); + if (key && REQ_SOURCES.includes(key) && ts.isIdentifier(el.name)) sourceNames.set(el.name.text, namespaceSource(key)); + } + } // const { a, b } = | await request.json() if (ts.isObjectBindingPattern(n.name) && (isReqSourceExpr(init) || isBodyReadCall(n.initializer))) { const src = isBodyReadCall(n.initializer) ? bodyReadSource(n.initializer) : sourceOfExpr(init); for (const el of n.name.elements) { const key = bindingKey(el, ts); - if (key) out.set(key, src); + if (key) record(key, src); } } } ts.forEachChild(n, visit); }; visit(body); - return [...out].map(([name, source]) => ({ name, source })); + return [...out].map(([name, sources]) => ({ name, sources })); // `req.body.x` / `req.query.x` / `req.params.x` — the namespace decides the runtime coordinate, and // route params notably have NONE, so this distinction is load-bearing rather than cosmetic. diff --git a/src/map/sinks.ts b/src/map/sinks.ts index f5bf9ef..f09ec45 100644 --- a/src/map/sinks.ts +++ b/src/map/sinks.ts @@ -22,12 +22,26 @@ const HTTP_MEMBER_METHODS = new Set(['get', 'post', 'put', 'patch', 'delete', 'h const DB_PACKAGES = ['@supabase/supabase-js', '@prisma/client', 'drizzle-orm', 'knex', 'kysely', 'pg', 'mysql2', 'mysql', 'sequelize', 'typeorm', 'mongoose', 'better-sqlite3']; 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 +// requiring a match keeps recognition tied to a RESOLVED import rather than to a method name. +const FS_PACKAGES = ['fs-extra', 'graceful-fs', 'memfs']; +const isFsPackage = (pkg: string) => /^node:fs(\/promises)?$/.test(pkg) || FS_PACKAGES.includes(pkg); +const EXEC_PACKAGES = ['execa', 'cross-spawn', 'shelljs', 'zx']; +const isExecPackage = (pkg: string) => pkg === 'node:child_process' || EXEC_PACKAGES.includes(pkg); export interface ModuleGraph { /** Sinks of `exportName` in the module `specifier` resolves to, relative to `fromFile`. */ importedSinks(fromFile: string, specifier: string, exportName: string): Sink[]; } +export interface SinkContext { + /** Absolute path of the file being analyzed (for resolving relative imports). */ + file: string; + /** The same file, REPO-RELATIVE — part of a sink's identity, so it must not vary by machine. */ + owner: string; + graph: ModuleGraph; +} + // --- sinks (agnostic) ------------------------------------------------------- export function collectLocalSinks(sf: any, ts: TsModule, bindings: Bindings): Map { const map = new Map(); @@ -46,7 +60,7 @@ export function collectLocalSinks(sf: any, ts: TsModule, bindings: Bindings): Ma return map; } -export function sinksFrom(arrowOrNode: any, ts: TsModule, localSinks: Map, bindings: Bindings, ctx?: { file: string; graph: ModuleGraph }): Sink[] { +export function sinksFrom(arrowOrNode: any, ts: TsModule, localSinks: Map, bindings: Bindings, ctx?: SinkContext): Sink[] { if (!arrowOrNode) return []; const body = arrowOrNode.isSyntheticBody ? arrowOrNode.body : isFnLike(arrowOrNode, ts) ? arrowOrNode.body : arrowOrNode; @@ -63,7 +77,32 @@ export function sinksFrom(arrowOrNode: any, ts: TsModule, localSinks: Map { + const out: Array<[string, string]> = []; + const visit = (n: any) => { + if (ts.isCallExpression(n) && ts.isPropertyAccessExpression(n.expression) && ts.isIdentifier(n.expression.expression)) { + out.push([n.expression.expression.text, n.expression.name.text]); + } + ts.forEachChild(n, visit); + }; + visit(node); + return out; } // Provider-agnostic sink recognizers over a subtree. Each sink is tagged with the npm package behind @@ -72,13 +111,24 @@ export function sinksFrom(arrowOrNode: any, ts: TsModule, localSinks: Map { + // `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 + // absence of one (an untraceable receiver such as a handler param). The member-call recognizers below + // need that distinction — `import * as helper from './util'; helper.exec(x)` is not child_process. + const baseOf = (base: any): { pkg?: string; local?: boolean; root?: string; spec?: string; relative?: boolean } => { const root = base ? rootIdentifier(base, ts) : undefined; if (!root) return {}; - const pkg = npmPackageOf(bindings.resolve(root)); - if (pkg) return { pkg, root }; - return { local: bindings.locals.has(root), root }; + const spec = bindings.resolve(root); + const relative = spec !== undefined && (spec.startsWith('.') || spec.startsWith('/')); + const pkg = npmPackageOf(spec); + if (pkg) return { pkg, 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 + // package inferred from the file's OTHER imports is a guess that is usually right ('inferred'); an + // 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; 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; @@ -100,35 +150,48 @@ function directSinks(node: any, ts: TsModule, bindings: Bindings): Sink[] { const t = n.arguments[0]; const table = t && ts.isStringLiteralLike(t) ? t.text : undefined; const parent = n.parent; - if (!b.local && parent && ts.isPropertyAccessExpression(parent) && DB_OPS.has(parent.name.text)) { - push({ kind: 'db', provider: 'sql', package: b.pkg ?? infer('db'), table, op: parent.name.text, ...spanOf(opCallOf(parent, ts)) }); + 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)) }); } } if (ts.isPropertyAccessExpression(callee)) { const method = callee.name.text; const b = baseOf(callee.expression); - if (!b.local) { + // `!b.relative` is the member-call twin of the bare-call justification below: a receiver that + // resolves to a RELATIVE module is app code, whatever its methods are named. Without it, + // `import * as helper from './util'; helper.exec(req.body.cmd)` was read as child_process and + // produced a precise, auto-generatable command-injection candidate for harmless local code. + // Such a receiver is not dropped outright — `sinksFrom` follows it into its module instead. + if (!b.local && !b.relative) { // db: prisma-style `prisma..()` — the op names are generic (`delete`, `update`, …), // so require a real prisma signal: a resolved binding, the import, or a prisma-named receiver. 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, ...spanOf(n) }); + if (prismaLikely) push({ kind: 'db', provider: 'prisma', package: '@prisma/client', table: callee.expression.name.text, op: method, attribution: b.pkg ? 'import' : 'inferred', ...spanOf(n) }); } - // db: raw `.query(` / `.execute(` + // 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') { - push({ kind: 'db', provider: 'sql', package: b.pkg ?? infer('db'), op: method, ...spanOf(n) }); + const pkg = b.pkg ?? infer('db'); + push({ kind: 'db', provider: 'sql', package: pkg, op: method, attribution: attributionOf(b, pkg), ...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. + if (FS_CALLS.test(method) && b.pkg && isFsPackage(b.pkg)) { + push({ kind: 'fs', package: b.pkg, op: method, attribution: 'import', ...spanOf(n) }); + } + if (EXEC_CALLS.test(method) && b.pkg && isExecPackage(b.pkg)) { + push({ kind: 'exec', package: b.pkg, op: method, attribution: 'import', ...spanOf(n) }); } - // fs / exec via a namespace: `fs.writeFile(` / `child_process.exec(` - if (FS_CALLS.test(method)) push({ kind: 'fs', package: b.pkg, op: method, ...spanOf(n) }); - if (EXEC_CALLS.test(method)) push({ kind: 'exec', package: b.pkg, op: method, ...spanOf(n) }); // http: any client whose binding resolves to a known http package (`axios.get`, `ky.post`, // `http.request`), else the classic identifiers by name as a heuristic. if (HTTP_MEMBER_METHODS.has(method)) { if (b.pkg && isHttpPackage(b.pkg)) { - push({ kind: 'http', provider: b.root, package: b.pkg, op: method, ...spanOf(n) }); - } else if (!b.pkg && ts.isIdentifier(callee.expression) && /^(axios|http|https|got|ky)$/.test(callee.expression.text)) { - push({ kind: 'http', provider: callee.expression.text, package: infer('http'), op: method, ...spanOf(n) }); + push({ kind: 'http', provider: b.root, package: b.pkg, op: method, attribution: 'import', ...spanOf(n) }); + } else if (!b.spec && ts.isIdentifier(callee.expression) && /^(axios|http|https|got|ky)$/.test(callee.expression.text)) { + push({ kind: 'http', provider: callee.expression.text, package: infer('http'), op: method, attribution: 'inferred', ...spanOf(n) }); } } } @@ -148,22 +211,22 @@ function directSinks(node: any, ts: TsModule, bindings: Bindings): Sink[] { const trueGlobal = !fromModule && !shadowed; if (HTTP_CALLS.test(name)) { - if (pkg && isHttpPackage(pkg)) push({ kind: 'http', provider: name, package: pkg, op: 'request', ...spanOf(n) }); - else if (name === 'fetch' && trueGlobal) push({ kind: 'http', provider: 'fetch', op: 'request', ...spanOf(n) }); + if (pkg && isHttpPackage(pkg)) push({ kind: 'http', provider: name, package: pkg, op: 'request', attribution: 'import', ...spanOf(n) }); + else if (name === 'fetch' && trueGlobal) push({ kind: 'http', provider: 'fetch', op: 'request', attribution: 'global', ...spanOf(n) }); } // `readFile`/`exec` are never globals: without a matching module binding this is app code. - if (FS_CALLS.test(name) && pkg && /^node:fs(\/promises)?$/.test(pkg)) { - push({ kind: 'fs', package: pkg, op: name, ...spanOf(n) }); + if (FS_CALLS.test(name) && pkg && isFsPackage(pkg)) { + push({ kind: 'fs', package: pkg, op: name, attribution: 'import', ...spanOf(n) }); } - if (EXEC_CALLS.test(name) && pkg === 'node:child_process') { - push({ kind: 'exec', package: pkg, op: name, ...spanOf(n) }); + if (EXEC_CALLS.test(name) && pkg && isExecPackage(pkg)) { + push({ kind: 'exec', package: pkg, op: name, attribution: 'import', ...spanOf(n) }); } - if (name === 'eval' && trueGlobal) push({ kind: 'eval', op: 'eval', ...spanOf(n) }); + if (name === 'eval' && trueGlobal) push({ kind: 'eval', op: 'eval', attribution: 'global', ...spanOf(n) }); } } if (ts.isNewExpression(n) && ts.isIdentifier(n.expression) && n.expression.text === 'Function' && !bindings.locals.has('Function') && !isShadowedByEnclosingBinding(n, 'Function', ts)) { - push({ kind: 'eval', op: 'new Function', ...spanOf(n) }); + push({ kind: 'eval', op: 'new Function', attribution: 'global', ...spanOf(n) }); } ts.forEachChild(n, visit); }; @@ -228,20 +291,23 @@ export function argumentRoleOf(sinkKind: string, method: string | undefined, ind } // Deterministic identity for a sink, so `Flow.sink` (an embedded copy) can be correlated back to the -// inventory entry without deep-equality. -function sinkId(s: Sink): string { +// inventory entry without deep-equality. `owner` is the endpoint's own repo-relative file, which the +// span alone does NOT imply: duplicated route boilerplate across two files puts the same call at the +// same offsets, and hashing only the span made those sinks share an id while the schema promises +// map-wide identity. A repo-relative path (never absolute) keeps the id stable across machines. +function sinkId(s: Sink, owner: string): string { return createHash('sha256') - .update([s.kind, s.provider, s.package, s.table, s.op, s.file, s.start, s.end].join('|')) + .update([s.kind, s.provider, s.package, s.table, s.op, s.file ?? owner, s.start, s.end].join('|')) .digest('hex') .slice(0, 12); } -function dedupeSinks(sinks: Sink[]): Sink[] { +function dedupeSinks(sinks: Sink[], owner: string): Sink[] { const seen = new Set(); const out: Sink[] = []; for (const s of sinks) { const key = `${s.kind}:${s.provider}:${s.package}:${s.table}:${s.op}:${s.line}:${s.start}`; - if (!seen.has(key)) { seen.add(key); out.push({ ...s, id: sinkId(s) }); } + if (!seen.has(key)) { seen.add(key); out.push({ ...s, id: sinkId(s, owner) }); } } return out; } diff --git a/src/map/types.ts b/src/map/types.ts index 6d82764..c33e940 100644 --- a/src/map/types.ts +++ b/src/map/types.ts @@ -67,6 +67,15 @@ export interface Sink { * — i.e. it does not live in the endpoint's file. Without this, `line` would point at the wrong file. */ file?: string; + /** + * How `package` was established — the difference between evidence and a guess. `import`: the + * receiver resolves to that dependency through this file's imports. `global`: a genuine runtime + * global (`fetch`, `eval`, `Function`). `inferred`: the receiver could not be resolved, so the + * package was taken from another import in the file. **Absent**: the receiver could not be + * attributed at all (e.g. `ctx.db.query(x)`) — the sink is reported for human review but must + * never drive an auto-generated rule, since any object can own a method by that name. + */ + attribution?: 'import' | 'global' | 'inferred'; /** * 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/aliased-namespace.test.ts b/tests/map/aliased-namespace.test.ts index 3f89eef..d83a2df 100644 --- a/tests/map/aliased-namespace.test.ts +++ b/tests/map/aliased-namespace.test.ts @@ -24,6 +24,9 @@ beforeAll(() => { app.get("/param/:id", ({ params: p }, res) => { res.end(fs.readFileSync(p.id)); }); app.post("/bodyalias", ({ body: b }, res) => { res.end(fs.readFileSync(b.file)); }); app.post("/nested", ({ query: q }, res) => { const { doc } = q; res.end(fs.readFileSync(doc)); }); + // The same namespace capture, one statement later instead of in the parameter list. + app.get("/fromreq", (req, res) => { const { query: q } = req; res.end(fs.readFileSync(q.doc)); }); + app.get("/fromreq/:id", (req, res) => { const { params: p } = req; res.end(fs.readFileSync(p.id)); }); `); }); afterAll(() => rmSync(dir, { recursive: true, force: true })); @@ -63,11 +66,27 @@ describe('aliased request namespaces', () => { expect(field).toMatchObject({ source: 'query', runtimeParameter: 'get.doc' }); }); + it('captures the namespace when destructured from the request identifier itself', async () => { + const { field } = await input('/fromreq', 'doc'); + // Previously invisible: no coordinate was mis-addressed, but the surface went unreported, which + // reads as "nothing here" rather than "something we cannot address". + expect(field).toMatchObject({ source: 'query', runtimeParameter: 'get.doc' }); + }); + + it('still refuses a coordinate for a route param destructured that way', async () => { + const { field } = await input('/fromreq/:id', 'id'); + expect(field.source).toBe('route-param'); + expect(field.runtimeParameter).toBeNull(); + }); + it('compiles candidates for the addressable ones only', async () => { const { map } = await buildInputMap(dir); const got = map!.endpoints .flatMap((e) => e.flows.filter((f) => f.ruleGeneratable).map((f) => `${e.route}:${f.input}`)) .sort(); + // `/fromreq` is absent by design: the input is now VISIBLE, but its flow evidence is heuristic + // (the alias is not yet tracked in the taint paths), so it must not compile a rule. Visible and + // non-generatable is the safe half of this fix; making such flows precise is a separate change. expect(got).toEqual(['/bodyalias:file', '/nested:doc', '/plain:doc', '/renamed:doc']); }); }); diff --git a/tests/map/corpus.test.ts b/tests/map/corpus.test.ts index c3fdfa8..775840c 100644 --- a/tests/map/corpus.test.ts +++ b/tests/map/corpus.test.ts @@ -20,6 +20,14 @@ import type { SiteInputMap } from '../../src/map/types.js'; interface Case { name: string; + /** + * `stack`: a project shape a builder really generates — measures recall and correct pinning. + * `adversarial`: app code CONSTRUCTED to look dangerous. A permanent category, not a bag of + * regressions: every false-candidate class we have found came from code that merely resembled a + * dangerous API, so the corpus has to contain lookalikes on purpose. These cases must produce a + * visible surface (inputs, usually sinks) and still compile no rule. + */ + kind?: 'stack' | 'adversarial'; pkg: Record; files: Record; /** `family @ runtimeParameter` for every flow that SHOULD compile to a candidate. */ @@ -28,6 +36,147 @@ interface Case { expectRefused?: Array<[string, RegExp]>; } +const ADVERSARIAL: Case[] = [ + { + name: 'adversarial: app code whose exports collide with dangerous API names', + kind: 'adversarial', + pkg: { dependencies: { express: '4' } }, + files: { + 'src/util.ts': ` + export function exec(x) { return x.length } + export function query(x) { return x } + export function readFileSync(p) { return p } + export function fetch(u) { return { u } } + `, + 'src/server.ts': ` + import * as helper from "./util"; + import { fetch, exec } from "./util"; + import express from "express"; + const app = express(); + // Namespace member calls AND named imports, both from a relative module. + app.post("/ns", (req, res) => { + helper.exec(req.body.cmd); helper.query(req.body.sql); helper.readFileSync(req.body.path); + res.end(); + }); + app.post("/named", (req, res) => { fetch(req.body.url); exec(req.body.cmd2); res.end(); }); + `, + }, + expectCandidates: [], + }, + { + name: 'adversarial: untraceable receivers in a file that imports real db clients', + kind: 'adversarial', + pkg: { dependencies: { express: '4', pg: '8', '@supabase/supabase-js': '2' } }, + files: { + 'src/server.ts': ` + import { Pool } from "pg"; + import { createClient } from "@supabase/supabase-js"; + import express from "express"; + const app = express(); + // The package is only INFERRED from the file's imports; the receivers are app objects. + app.post("/raw", (req, res) => { res.locals.db.query(req.body.sql); res.end(); }); + app.post("/from", (req, res) => { res.locals.sb.from("t").insert({ v: req.body.v }); res.end(); }); + `, + }, + expectCandidates: [], + expectRefused: [['sql', /inferred from the file's other imports/]], + }, + { + name: 'adversarial: parameters shadowing dangerous globals', + kind: 'adversarial', + pkg: { dependencies: { express: '4' } }, + files: { + 'src/server.ts': ` + import express from "express"; + const app = express(); + app.post("/shadow", (req, res) => { + const send = (fetch) => fetch(req.body.url); + send((u) => ({ u })); + const run = (eval2) => eval2(req.body.code); + run((c) => c); + res.end(); + }); + `, + }, + expectCandidates: [], + }, + { + name: 'adversarial: one field name read from two request namespaces', + kind: 'adversarial', + pkg: { dependencies: { express: '4' } }, + files: { + // `params.id` and `query.id` share a NAME but not an address. Whichever read the walker saw last + // used to decide the coordinate, so this handler compiled a rule pinned to `get.id` for data + // arriving in the path segment. Both orders are covered because that is what made it a bug. + 'src/a.ts': ` + import express from "express"; + import fs from "node:fs"; + const app = express(); + app.get("/qp/:id", ({ params: p, query: q }, res) => { fs.readFileSync(q.id); fs.readFileSync(p.id); res.end(); }); + `, + 'src/b.ts': ` + import express from "express"; + import fs from "node:fs"; + const app = express(); + app.get("/pq/:id", ({ params: p, query: q }, res) => { fs.readFileSync(p.id); fs.readFileSync(q.id); res.end(); }); + `, + }, + expectCandidates: [], + expectRefused: [['id', /more than one request namespace/]], + }, + { + name: 'adversarial: a validator field and the sink read address different namespaces', + kind: 'adversarial', + pkg: { dependencies: { express: '4', zod: '3' } }, + files: { + // The schema describes the BODY; the sink consumes the QUERY. Both are called `id`, and grouping + // inputs by name made the schema's `post.id` the only surviving entry — so the candidate pinned a + // parameter the payload never travels in. Namespace comparison (not source labels) is what catches + // this: a schema field and an Express `req.body` read are both `post.*` and must NOT be a conflict. + 'src/server.ts': ` + import express from "express"; + import fs from "node:fs"; + import { z } from "zod"; + const app = express(); + app.post("/mismatch", (req, res) => { + z.object({ id: z.string() }).parse(req.body); + res.end(fs.readFileSync(req.query.id)); + }); + app.post("/agree", (req, res) => { + z.object({ doc: z.string() }).parse(req.body); + res.end(fs.readFileSync(req.body.doc)); + }); + `, + }, + // `/agree` must still compile: the point is to refuse conflicts, not to refuse validated bodies. + expectCandidates: ['path-traversal @ post.doc'], + expectRefused: [['id', /more than one request namespace/]], + }, + { + name: 'adversarial: sibling expressions must not contaminate each other', + kind: 'adversarial', + pkg: { dependencies: { express: '4' } }, + files: { + 'src/server.ts': ` + import express from "express"; + import fs from "node:fs"; + import { exec } from "node:child_process"; + const STATIC = "ls -la"; + const app = express(); + // Only ONE pairing is real: path -> readFileSync. \`label\` reaches no sink, and the exec call + // takes no request data at all — an inventory-level "both present" must not become a flow. + app.post("/two", (req, res) => { + const label = req.body.label; + fs.readFileSync(req.body.path); + exec(STATIC); + res.end(label); + }); + `, + }, + expectCandidates: ['path-traversal @ post.path'], + }, +]; + const CASES: Case[] = [ { name: 'lovable / tanstack start + supabase (server fns, validated payload)', @@ -141,11 +290,13 @@ const CASES: Case[] = [ }, ]; +const ALL: Case[] = [...CASES, ...ADVERSARIAL]; + const maps = new Map(); let dirs: string[] = []; beforeAll(async () => { - for (const c of CASES) { + for (const c of ALL) { const d = mkdtempSync(join(tmpdir(), 'ps-corpus-')); dirs.push(d); for (const [rel, body] of Object.entries(c.files)) { @@ -175,7 +326,7 @@ function candidatesOf(map: SiteInputMap): string[] { } describe('golden corpus', () => { - for (const c of CASES) { + for (const c of ALL) { describe(c.name, () => { it('compiles exactly the expected candidates — no wrong-input pins', () => { const got = candidatesOf(maps.get(c.name)!); @@ -208,9 +359,35 @@ describe('golden corpus', () => { }); } + // An adversarial case that found NOTHING would pass its zero-candidate assertion for the wrong + // reason — a parser bug or a bad fixture would read as a security property. Each one has to prove it + // actually saw the handler and the request fields, and only then that it compiled no rule. + it('adversarial cases detect a real surface and still refuse to compile a rule', () => { + for (const c of ADVERSARIAL) { + const map = maps.get(c.name)!; + const inputs = map.endpoints.flatMap((e) => e.inputs); + expect(map.endpoints.length, `${c.name}: no endpoint detected — the fixture proves nothing`).toBeGreaterThan(0); + expect(inputs.length, `${c.name}: no inputs detected — the fixture proves nothing`).toBeGreaterThan(0); + const generatable = map.endpoints.flatMap((e) => e.flows).filter((f) => f.ruleGeneratable); + const declared = new Set(c.expectCandidates); + const coord = new Map(map.endpoints.flatMap((e) => e.inputs).map((i) => [i.name, i.runtimeParameter])); + expect(generatable.filter((f) => !declared.has(`${f.candidateFamily} @ ${coord.get(f.input)}`))).toEqual([]); + } + }); + + 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); + 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']) { + expect(names, `missing adversarial class: ${cls}`).toContain(cls); + } + }); + it('reports corpus-wide metrics (the numbers that gate auto-promotion)', () => { let candidates = 0, precise = 0, heuristic = 0, refusedWithReason = 0, noCoordinate = 0; - for (const c of CASES) { + for (const c of ALL) { for (const ep of maps.get(c.name)!.endpoints) { for (const i of ep.inputs) if (!i.runtimeParameter) noCoordinate++; for (const f of ep.flows) { @@ -221,11 +398,11 @@ describe('golden corpus', () => { } } // eslint-disable-next-line no-console - console.log(`corpus: ${CASES.length} projects · ${candidates} candidates · ${precise} precise / ${heuristic} heuristic flows · ${refusedWithReason} refused-with-reason · ${noCoordinate} inputs without a coordinate`); + console.log(`corpus: ${CASES.length} stack + ${ADVERSARIAL.length} adversarial projects · ${candidates} candidates · ${precise} precise / ${heuristic} heuristic flows · ${refusedWithReason} refused-with-reason · ${noCoordinate} inputs without a coordinate`); expect(candidates).toBeGreaterThan(0); // the compiler does something expect(refusedWithReason).toBeGreaterThan(0); // and refuses a lot, explicitly // Every non-candidate must explain itself: silence is what makes a map untrustworthy. - for (const c of CASES) { + for (const c of ALL) { for (const ep of maps.get(c.name)!.endpoints) { for (const f of ep.flows.filter((x) => x.ruleGeneratable === false)) { expect(f.ruleGeneratableReasons?.length, `${c.name}/${f.input}: refused without a reason`).toBeGreaterThan(0); diff --git a/tests/map/sink-attribution-members.test.ts b/tests/map/sink-attribution-members.test.ts new file mode 100644 index 0000000..b6dd071 --- /dev/null +++ b/tests/map/sink-attribution-members.test.ts @@ -0,0 +1,213 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { buildInputMap } from '../../src/map/index.js'; + +// The MEMBER-call companion to sink-attribution.test.ts (which covers bare calls). +// A dangerous METHOD NAME is not a dangerous API. `import * as helper from './util'` gives a receiver +// that is neither a local binding nor a package, and admitting it produced precise, auto-generatable +// command-injection / SQLi / path-traversal candidates for ordinary app code. The bare-call path already +// required justification; the member-call path did not — the same bug, one syntax over. +let dir: string; +beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'ps-attr-')); + mkdirSync(join(dir, 'src'), { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ dependencies: { express: '4' } })); + // App code whose exported names collide with dangerous APIs. + writeFileSync(join(dir, 'src', 'util.ts'), ` + export function exec(x: string) { return x.length } + export function query(x: string) { return x } + export function readFileSync(p: string) { return p } + `); + // A relative namespace helper that DOES reach a real sink — recall must survive the fix. + writeFileSync(join(dir, 'src', 'store.ts'), ` + import fs from "node:fs"; + export function save(p: string) { return fs.writeFileSync(p, "x") } + `); + writeFileSync(join(dir, 'src', 'app.ts'), ` + import * as helper from "./util"; + import * as store from "./store"; + import express from "express"; + const app = express(); + app.post("/lookalike", (req, res) => { + helper.exec(req.body.cmd); + helper.query(req.body.sql); + helper.readFileSync(req.body.path); + res.end("ok"); + }); + app.post("/viaHelper", (req, res) => { store.save(req.body.p); res.end("ok"); }); + app.post("/untraceable", (req, res) => { res.locals.db.query(req.body.sql); res.end("ok"); }); + `); + writeFileSync(join(dir, 'src', 'real.ts'), ` + import fs from "node:fs"; + import { exec } from "node:child_process"; + import express from "express"; + const app = express(); + app.post("/real", (req, res) => { + fs.readFileSync(req.body.path); + exec(req.body.cmd); + res.end("ok"); + }); + `); +}); +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('member-call sinks require an attributable receiver', () => { + it('does not treat a relative namespace helper as fs/exec/db', async () => { + const e = await ep('/lookalike'); + expect(e.sinks).toEqual([]); + }); + + it('generates no candidate for it — the false-blocking-rule case', async () => { + const e = await ep('/lookalike'); + // The inputs are still reported: the surface is real, the SINK was not. + expect(e.inputs.map((i) => i.name).sort()).toEqual(['cmd', 'path', 'sql']); + expect(e.flows.filter((f) => f.ruleGeneratable)).toEqual([]); + }); + + it('still follows that receiver into its module, so a real sink is not lost', async () => { + const e = await ep('/viaHelper'); + const fsSink = e.sinks.find((s) => s.kind === 'fs'); + expect(fsSink).toBeDefined(); + expect(fsSink!.package).toBe('node:fs'); + expect(fsSink!.file).toBe('src/store.ts'); // attributed to where it actually lives + }); + + it('keeps an untraceable receiver in the inventory but refuses to auto-rule it', async () => { + const e = await ep('/untraceable'); + const db = e.sinks.find((s) => s.kind === 'db'); + expect(db).toBeDefined(); + expect(db!.attribution).toBeUndefined(); + const flow = e.flows.find((f) => f.sink.kind === 'db')!; + expect(flow.ruleGeneratable).toBe(false); + expect(flow.ruleGeneratableReasons!.join(' ')).toMatch(/could not be traced to a dependency/); + }); + + it('leaves genuinely imported sinks fully generatable', async () => { + const e = await ep('/real'); + expect(e.sinks.map((s) => `${s.kind}:${s.attribution}`).sort()).toEqual(['exec:import', 'fs:import']); + const gen = e.flows.filter((f) => f.ruleGeneratable).map((f) => f.candidateFamily).sort(); + expect(gen).toEqual(['command-injection', 'path-traversal']); + }); +}); + +describe('sink identity is map-wide', () => { + it('does not collide across two files with identical boilerplate at identical offsets', async () => { + const d = mkdtempSync(join(tmpdir(), 'ps-dup-')); + mkdirSync(join(d, 'src'), { recursive: true }); + writeFileSync(join(d, 'package.json'), JSON.stringify({ dependencies: { express: '4' } })); + // Byte-identical but for the route string, so every span matches. + const src = (route: string) => ` + import express from "express"; + import fs from "node:fs"; + const app = express(); + app.post("${route}", (req, res) => { res.end(fs.readFileSync(req.body.f)); }); + `; + writeFileSync(join(d, 'src', 'a.ts'), src('/aa')); + writeFileSync(join(d, 'src', 'b.ts'), src('/bb')); + const { map } = await buildInputMap(d); + const ids = map!.endpoints.flatMap((e) => e.sinks.map((s) => s.id)); + expect(ids).toHaveLength(2); + expect(new Set(ids).size).toBe(2); + rmSync(d, { recursive: true, force: true }); + }); + + it('stays deterministic across runs', async () => { + const a = await ep('/real'); + const b = await ep('/real'); + expect(a.sinks.map((s) => s.id)).toEqual(b.sinks.map((s) => s.id)); + }); +}); + +// An INFERRED package is not evidence about the receiver. `res.locals.db.query(x)` in a file that +// happens to import `pg` was getting package "pg" and a precise SQL-injection candidate — the exact +// guarantee this file's first half claims to enforce, defeated by the fallback that fills in a package +// from the file's OTHER imports. Inferred sinks stay in the inventory; they never compile a rule. +describe('an inferred package does not license a rule', () => { + let d: string; + beforeAll(() => { + d = mkdtempSync(join(tmpdir(), 'ps-infer-')); + mkdirSync(join(d, 'src'), { recursive: true }); + writeFileSync(join(d, 'package.json'), JSON.stringify({ + dependencies: { express: '4', pg: '8', '@supabase/supabase-js': '2', '@prisma/client': '5' }, + })); + writeFileSync(join(d, 'src', 'inferred.ts'), ` + import { Pool } from "pg"; + import { createClient } from "@supabase/supabase-js"; + import { PrismaClient } from "@prisma/client"; + import express from "express"; + const app = express(); + // Every receiver here is an app object the analyzer cannot trace. + app.post("/raw", (req, res) => { res.locals.db.query(req.body.sql); res.end("ok"); }); + app.post("/from", (req, res) => { res.locals.sb.from("t").insert({ v: req.body.v }); res.end("ok"); }); + app.post("/prisma", (req, res) => { res.locals.prisma.user.update({ where: { id: req.body.id } }); res.end("ok"); }); + `); + // Controls: receivers that really do resolve to the dependency. + writeFileSync(join(d, 'src', 'resolved.ts'), ` + import { Pool } from "pg"; + import { createClient } from "@supabase/supabase-js"; + import express from "express"; + const pool = new Pool(); + const sb = createClient("u", "k"); + const app = express(); + app.post("/pool", (req, res) => { pool.query(req.body.sql); res.end("ok"); }); + app.post("/sb", (req, res) => { sb.from("t").insert({ v: req.body.v }); res.end("ok"); }); + `); + }); + 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('refuses a rule for an untraceable receiver even when the file imports a db package', async () => { + const e = await route('/raw'); + const sink = e.sinks.find((s) => s.kind === 'db')!; + // Note WHICH package inference picks: the first known db package the file imports, in table order — + // here supabase, not the `pg` a reader would guess from `.query()`. A useful hint for a human + // reviewer, and a good illustration of why it must not address a rule. + expect(sink.package).toBe('@supabase/supabase-js'); + expect(sink.attribution).toBe('inferred'); + const flow = e.flows.find((f) => f.sink.kind === 'db')!; + expect(flow.confidence).toBe('precise'); // the data really does reach it + expect(flow.ruleGeneratable).toBe(false); // and it still must not be auto-ruled + expect(flow.ruleGeneratableReasons!.join(' ')).toMatch(/inferred from the file's other imports/); + }); + + it('refuses the same for an untraceable .from().insert() receiver', async () => { + const e = await route('/from'); + const sink = e.sinks.find((s) => s.kind === 'db')!; + expect(sink.attribution).toBe('inferred'); + const flow = e.flows.find((f) => f.sink.kind === 'db')!; + expect(flow.ruleGeneratable).toBe(false); + // Assert the ATTRIBUTION reason specifically: this shape is also refused for its argument role + // ("values"), so without this the fixture would pass for the wrong reason and regress silently. + expect(flow.ruleGeneratableReasons!.join(' ')).toMatch(/inferred from the file's other imports/); + }); + + 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'); + expect(e.flows.filter((f) => f.ruleGeneratable)).toEqual([]); + }); + + it('still generates for receivers that genuinely resolve to the dependency', async () => { + const pool = await route('/pool'); + const sink = pool.sinks.find((s) => s.kind === 'db')!; + expect(sink.attribution).toBe('import'); + const flow = pool.flows.find((f) => f.sink.kind === 'db')!; + expect(flow.ruleGeneratable).toBe(true); + expect(flow.candidateFamily).toBe('sql-injection'); + // And the resolved supabase receiver keeps its (separate, role-based) refusal — unchanged. + const sb = await route('/sb'); + expect(sb.sinks.find((s) => s.kind === 'db')!.attribution).toBe('import'); + }); +});