From 513f5ce2ea5db99669f8d46a6fc9f9b6f7fc2442 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Thu, 27 Aug 2026 10:26:17 +0200 Subject: [PATCH] Derive addressable-source coverage from the vocabulary, not from a restated list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The round-trip suite's completeness assertion compared one hard-coded set against another hard-coded set, so it asserted that the file agreed with itself. A sixth addressable source would not have failed it — which is the gap it was written to close. The root cause was that the vocabulary could not be asked. `InputSource` was a bare type union, and a union cannot be enumerated at runtime, so a test wanting "every source the map can address" had no option but to restate the list. Every sibling vocabulary in `capabilities.ts` is a runtime array with its type derived from it; this one was the exception. `INPUT_SOURCES` is now that array and `InputSource` is derived from it, so the two cannot disagree. The assertion computes its own expectation: a source is addressable when `runtimeCoordinate` yields a parameter for it, and every addressable source must have a case driven through the real engine. The body-shaped sources are asserted to collapse to `post.` rather than assumed to, so a divergence cannot hide behind the grouping, and the unaddressable ones are asserted to stay unaddressable with a stated reason — a coordinate appearing for a route parameter would compile a rule the resolver cannot resolve. Verified by adding a sixth addressable source: the new assertion fails, the old one passed. ## A second instance of the same defect, found on the way Adding `INPUT_SOURCES` to `CAPABILITY_MANIFEST` did not put it in `capabilities.json`, because `emit-capabilities.mjs` assembles the JSON from its OWN restated list of vocabulary names. And `emit-capabilities --check` reported "up to date" throughout: both sides omitted the field, so they agreed. A vocabulary could be declared, typed, used, and never ship, with the check built to catch exactly that saying nothing. The emitter now emits it, and a test asserts over the SOURCE that every exported `as const` vocabulary reaches the manifest with the same members — the only form of this assertion that cannot be satisfied by two files agreeing to be wrong together. It has a floor on the number of vocabularies found, so an empty match cannot pass either. Verified the same way: with the emitter's line removed, `--check` still says "up to date" and the new guard fails. Capability version 1.1.0 → 1.2.0; the version check reads it as additive, as it should. Co-Authored-By: Claude Opus 5 (1M context) --- capabilities.json | 15 ++++++- scripts/emit-capabilities.mjs | 1 + src/map/capabilities.ts | 18 +++++++- src/map/types.ts | 21 ++++----- tests/map/capabilities.test.ts | 37 +++++++++++++++ tests/protect/coordinate-round-trip.test.ts | 50 ++++++++++++++++----- 6 files changed, 116 insertions(+), 26 deletions(-) diff --git a/capabilities.json b/capabilities.json index 5615e6e8..e2a6b026 100644 --- a/capabilities.json +++ b/capabilities.json @@ -1,6 +1,6 @@ { "$comment": "GENERATED from src/map/capabilities.ts by scripts/emit-capabilities.mjs — do not edit by hand. The single versioned definition of what the input-flow map can describe, vendored by the reachability recipe schema/validator and by the server that binds coordinates into rules.", - "version": "1.1.0", + "version": "1.2.0", "sinkKinds": [ "db", "fs", @@ -59,6 +59,19 @@ "route-param", "unknown" ], + "inputSources": [ + "json-body", + "form-body", + "multipart", + "body", + "query", + "route-param", + "header", + "cookie", + "file", + "server-fn-data", + "unknown" + ], "invocationKinds": [ "call", "member", diff --git a/scripts/emit-capabilities.mjs b/scripts/emit-capabilities.mjs index 31dee4ca..223709bb 100644 --- a/scripts/emit-capabilities.mjs +++ b/scripts/emit-capabilities.mjs @@ -47,6 +47,7 @@ const manifest = { autoPromotableConfidence: stringOf('AUTO_PROMOTABLE_CONFIDENCE'), attributions: arrayOf('ATTRIBUTIONS'), addressSpaces: arrayOf('ADDRESS_SPACES'), + inputSources: arrayOf('INPUT_SOURCES'), invocationKinds: arrayOf('INVOCATION_KINDS'), invocationResolutions: arrayOf('INVOCATION_RESOLUTIONS'), }; diff --git a/src/map/capabilities.ts b/src/map/capabilities.ts index 6b8ba230..65c012f8 100644 --- a/src/map/capabilities.ts +++ b/src/map/capabilities.ts @@ -20,7 +20,7 @@ * member is breaking and bumps the MAJOR, because a consumer pinned to the old list will keep emitting a * value that can no longer match. */ -export const CAPABILITY_VERSION = '1.1.0'; +export const CAPABILITY_VERSION = '1.2.0'; /** Sink families the extractor recognizes. A dangerous OPERATION, not a package. */ export const SINK_KINDS = ['db', 'fs', 'http', 'exec', 'eval'] as const; @@ -79,6 +79,21 @@ export const ATTRIBUTIONS = ['import', 'global', 'inferred'] as const; /** The request address spaces an input can live in — half of an input's identity. */ export const ADDRESS_SPACES = ['post', 'get', 'cookie', 'files', 'server', 'route-param', 'unknown'] as const; +/** + * Where an input is read from. The other half of an input's identity, and the axis that decides which + * address space — and therefore which engine parameter — can name it. + * + * A runtime array, like every other vocabulary here, because `InputSource` was a bare type union and a + * union cannot be enumerated at runtime. That is not a stylistic gap: a test asked to cover "every source + * the map can address" had no way to ask what those were, so it restated the list — and a restated list + * compared against another restated list passes forever, including after a sixth source is added. + */ +export const INPUT_SOURCES = [ + 'json-body', 'form-body', 'multipart', 'body', + 'query', 'route-param', 'header', 'cookie', 'file', + 'server-fn-data', 'unknown', +] as const; + /** The whole contract, as the other repos consume it. Key order is stable so the JSON is diffable. */ export const CAPABILITY_MANIFEST = { version: CAPABILITY_VERSION, @@ -90,6 +105,7 @@ export const CAPABILITY_MANIFEST = { autoPromotableConfidence: AUTO_PROMOTABLE_CONFIDENCE, attributions: ATTRIBUTIONS, addressSpaces: ADDRESS_SPACES, + inputSources: INPUT_SOURCES, invocationKinds: INVOCATION_KINDS, invocationResolutions: INVOCATION_RESOLUTIONS, } as const; diff --git a/src/map/types.ts b/src/map/types.ts index 3fc7f8b2..a7c4e4e0 100644 --- a/src/map/types.ts +++ b/src/map/types.ts @@ -1,14 +1,7 @@ // The closed vocabularies below are derived from `capabilities.ts` — the single versioned definition // shared with the rule-authoring and rule-binding layers. Declaring a union here too would be a copy, // and the failure mode of the two disagreeing is silent (a value that can never match). -import { - ADDRESS_SPACES, - ARGUMENT_ROLES, - CANDIDATE_FAMILIES, - INVOCATION_KINDS, - INVOCATION_RESOLUTIONS, - SINK_KINDS, -} from './capabilities.js'; +import { ADDRESS_SPACES, ARGUMENT_ROLES, CANDIDATE_FAMILIES, INPUT_SOURCES, INVOCATION_KINDS, INVOCATION_RESOLUTIONS, SINK_KINDS } from './capabilities.js'; // The build-time input-flow ("attack surface") map. connect's `map` command walks the app's source // and emits this per-site: entry points → the inputs each reads → the sinks/dependencies they reach. @@ -17,11 +10,13 @@ import { // Honesty is a first-class field: static analysis is best-effort, so `coverage` records what the // adapter could and couldn't see. Never present the map as "complete". -/** Where an input is read from — determines which runtime parameter namespace can address it. */ -export type InputSource = - | 'json-body' | 'form-body' | 'multipart' | 'body' - | 'query' | 'route-param' | 'header' | 'cookie' | 'file' - | 'server-fn-data' | 'unknown'; +/** + * Where an input is read from — determines which runtime parameter namespace can address it. + * + * Derived from the runtime vocabulary rather than written out again, so the two cannot disagree and so a + * test can ask what the members ARE instead of restating them. + */ +export type InputSource = (typeof INPUT_SOURCES)[number]; /** * Where an input lives in the request, as the rule engine addresses it. This — not the field name — is diff --git a/tests/map/capabilities.test.ts b/tests/map/capabilities.test.ts index 99e40e3a..e98b12ab 100644 --- a/tests/map/capabilities.test.ts +++ b/tests/map/capabilities.test.ts @@ -227,3 +227,40 @@ describe('every declared capability has a recognizer behind it', () => { }, 60_000); }); +/** + * Every vocabulary declared here must actually reach the manifest consumers vendor. + * + * This was found the hard way: `INPUT_SOURCES` was added to `CAPABILITY_MANIFEST` and did not appear in + * `capabilities.json`, because the emitter assembles the JSON from its OWN restated list of vocabulary + * names. `emit-capabilities --check` reported "up to date" the whole time — both sides omitted it, so they + * agreed. A member could be added, typed, used, and never ship, and the check designed to catch exactly + * that said nothing. + * + * Asserting over the source rather than over either list is the only version of this that cannot be + * satisfied by two files agreeing to be wrong together. + */ +describe('the emitted manifest covers every declared vocabulary', () => { + const source = readFileSync(join(root, 'src', 'map', 'capabilities.ts'), 'utf8'); + const manifest = JSON.parse(readFileSync(manifestPath, 'utf8')); + + // Exported `as const` arrays are the vocabularies. `CAPABILITY_MANIFEST` itself is the assembly, not a + // member, and the scalar pins are read separately by the emitter. + const declared = [...source.matchAll(/export const ([A-Z_]+) = \[/g)] + .map((m) => m[1]) + .filter((name) => name !== 'CAPABILITY_MANIFEST'); + + it('finds the vocabularies to check, so an empty list cannot pass', () => { + expect(declared.length).toBeGreaterThanOrEqual(8); + }); + + it.each(declared)('%s appears in capabilities.json with the same members', (name) => { + // camelCase key, as the manifest names them: SINK_KINDS → sinkKinds. + const key = name.toLowerCase().replace(/_(.)/g, (_, c) => c.toUpperCase()); + const members = [...(new RegExp(`export const ${name} = \\[([^\\]]*)\\] as const;`, 's').exec(source)?.[1] ?? '') + .matchAll(/'([^']+)'/g)].map((m) => m[1]); + + expect(manifest, `${name} is declared but ${key} is absent from capabilities.json`).toHaveProperty(key); + expect(manifest[key]).toEqual(members); + }); +}); + diff --git a/tests/protect/coordinate-round-trip.test.ts b/tests/protect/coordinate-round-trip.test.ts index a4f066b1..c35cf9ca 100644 --- a/tests/protect/coordinate-round-trip.test.ts +++ b/tests/protect/coordinate-round-trip.test.ts @@ -4,6 +4,8 @@ import { tmpdir } from 'node:os'; import path from 'node:path'; import { buildInputMap } from '../../src/map/index.js'; import { createProtection } from '../../src/protect/runtime.js'; +import { INPUT_SOURCES } from '../../src/map/capabilities.js'; +import { runtimeCoordinate } from '../../src/map/coordinates.js'; /** * The coordinate the MAP emits is the coordinate the ENGINE resolves. @@ -100,7 +102,8 @@ const multipart = (filename: string) => { }; interface Case { - space: string; + /** The `InputSource` this case drives, so coverage can be compared against the vocabulary. */ + source: (typeof INPUT_SOURCES)[number]; input: string; expected: string; exploit: () => Request; @@ -109,28 +112,28 @@ interface Case { const cases: Case[] = [ { - space: 'body', input: 'note', expected: 'post.note', + source: 'body', input: 'note', expected: 'post.note', exploit: () => new Request(URL_BASE, { method: 'POST', headers: { 'content-type': 'application/json' }, body: JSON.stringify({ note: EXPLOIT }) }), benign: () => new Request(URL_BASE, { method: 'POST', headers: { 'content-type': 'application/json' }, body: JSON.stringify({ note: 'hello' }) }), }, { - space: 'query', input: 'q', expected: 'get.q', + source: 'query', input: 'q', expected: 'get.q', exploit: () => new Request(`${URL_BASE}?q=${EXPLOIT}`), benign: () => new Request(`${URL_BASE}?q=hello`), }, { - space: 'cookie', input: 'session', expected: 'cookie.session', + source: 'cookie', input: 'session', expected: 'cookie.session', exploit: () => new Request(URL_BASE, { headers: { cookie: `session=${EXPLOIT}` } }), benign: () => new Request(URL_BASE, { headers: { cookie: 'session=abc123' } }), }, { - space: 'header', input: 'x-api-key', expected: 'server.HTTP_X_API_KEY', + source: 'header', input: 'x-api-key', expected: 'server.HTTP_X_API_KEY', exploit: () => new Request(URL_BASE, { headers: { 'x-api-key': EXPLOIT } }), benign: () => new Request(URL_BASE, { headers: { 'x-api-key': 'abc123' } }), }, { // A bare `files.` resolves to the FILENAME, so that is where the payload goes. - space: 'file', input: 'avatar', expected: 'files.avatar', + source: 'file', input: 'avatar', expected: 'files.avatar', exploit: () => multipart(`${EXPLOIT}.png`), benign: () => multipart('holiday.png'), }, @@ -138,7 +141,7 @@ const cases: Case[] = [ describe('a coordinate the map emitted', () => { for (const c of cases) { - describe(`${c.space} (${c.input})`, () => { + describe(`${c.source} (${c.input})`, () => { it('is the coordinate this suite expects the engine to need', () => { // Guards the test itself: if extraction stopped emitting this input, every assertion below would // be skipped on an undefined coordinate and the space would silently lose its coverage. @@ -155,9 +158,34 @@ describe('a coordinate the map emitted', () => { }); } - it('covers every space the map can address', () => { - // Without this, adding a new addressable space to `coordinates.ts` leaves it unverified against the - // resolver, which is the state this suite exists to end. - expect(new Set(cases.map((c) => c.space))).toEqual(new Set(['body', 'query', 'cookie', 'header', 'file'])); + it('covers every source the map can address, derived from the vocabulary', () => { + // The previous version compared one hard-coded set against another hard-coded set, so it asserted that + // this file agreed with itself. A sixth addressable source would not have failed it — which is exactly + // the gap it was written to close. + // + // The expectation is now computed from production: a source is addressable when `runtimeCoordinate` + // yields a parameter for it, and every addressable source must have a case here. Adding one to + // `INPUT_SOURCES` and giving it a coordinate now fails this until it is driven through the engine. + const addressable = INPUT_SOURCES.filter( + (source) => runtimeCoordinate(source, 'field').runtimeParameter !== null, + ); + const covered = new Set(cases.map((c) => c.source)); + + // The body-shaped sources all resolve to `post.`, so one case covers them; asserted explicitly + // rather than assumed, because a divergence would otherwise hide behind the grouping. + const bodyShaped = ['json-body', 'form-body', 'multipart', 'body', 'server-fn-data'] as const; + for (const source of bodyShaped) { + expect(runtimeCoordinate(source, 'field').runtimeParameter).toBe('post.field'); + } + + const needsItsOwnCase = addressable.filter((s) => !bodyShaped.includes(s as never)); + expect(new Set([...needsItsOwnCase, 'body'])).toEqual(covered); + + // And the unaddressable ones stay unaddressable: a coordinate appearing for a route parameter would + // compile a rule the resolver cannot resolve, which is the failure this whole file is about. + for (const source of INPUT_SOURCES.filter((s) => !addressable.includes(s))) { + expect(runtimeCoordinate(source, 'field').runtimeParameter).toBeNull(); + expect(runtimeCoordinate(source, 'field').runtimeParameterReason).toBeTruthy(); + } }); });