From df2b7ba15c256d9dd01ca009e3a69263968a0f2e Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Thu, 20 Aug 2026 16:54:09 +0200 Subject: [PATCH 1/5] Disclose detection reporting, and stop claiming it does not exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit detections.js ships in dist/, and the word "detection" appeared nowhere in AGENT-INSTALL.md. Agents npm pack the tarball and audit it before installing, so a capability in dist/ that the docs do not mention reads as misrepresentation — the install gets refused, and the refusal is right. Worse than an omission, one line was a claim that contradicted it: map --upload was described as the one opt-in that sends anything derived from your source, and a guard with reportDetections: true already sends route paths and the parameter names a rule reads. Turning that default on without fixing this would have shipped a false statement rather than a loose one. The new section states both reporting paths and the payload field by field, and says plainly what is excluded — matched values, bodies, headers, cookies, query-string values. Every claim was checked against detections.js rather than written from memory, including the conditions: a site UUID is required and PATCHSTACK_TELEMETRY=off disables it. Nothing caught this because nothing looks. The CI job named "Capability contract" checks that the committed manifest matches the map vocabulary in its source; it never reads documentation. So the disclosure is now a test: every outbound endpoint is read out of src/ and must be described in AGENT-INSTALL.md, including the ones built from a resolved base URL rather than a literal path — which is how the detection reporter is written, and therefore exactly the shape a check matching literal paths would have missed. Field test, --persona hostile --rounds 3: 2/3 rounds fully green. Round 1 refused, citing only the install prompt, and ran no commands at all, so it never read the shipped docs. Worth stating that the fixture installs the published package, so this run cannot exercise the new section either way. Co-Authored-By: Claude Opus 5 (1M context) --- AGENT-INSTALL.md | 28 +++++++- tests/endpoint-disclosure.test.ts | 102 ++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 tests/endpoint-disclosure.test.ts diff --git a/AGENT-INSTALL.md b/AGENT-INSTALL.md index 2606300..f99f967 100644 --- a/AGENT-INSTALL.md +++ b/AGENT-INSTALL.md @@ -12,7 +12,7 @@ This versioned reference ships inside `@patchstack/connect` and documents each s - The package also exposes **`protect`** directly (runtime exploit guard; its templates live under `dist/protect/`). `setup` invokes it automatically; `scan`, `guide`, `status`, and `mark-build` do not. It writes only local files and auto-wires known stacks — **TanStack Start + Supabase** (patches the Supabase client + `src/start.ts`), **Next.js** (scaffolds `middleware.ts`), **SvelteKit** (`src/hooks.server.ts`), **Astro** (`src/middleware.ts`), **Nuxt** (`server/middleware/`), **NestJS** (`app.use(patchstackMiddleware)` in the bootstrap), **Fastify** (`app.register(patchstackFastify)`), and **Express** (`app.use(patchstackMiddleware)`). On **any other stack** it scaffolds a framework-agnostic guard under `src/patchstack/` and prints a wiring plan — then you finish the install by importing that guard into your server entry (`protectFetch(handler)` for a Web-Fetch server, or `app.use(patchstackMiddleware)` for Node/Express) and running `patchstack-connect protect --check` to confirm it is wired (exit 1 until it is). Passing `--demo` seeds a broad sample rule set (for demonstrations, not production). - **`demo node-serialize` is an explicit production-backed walkthrough.** It requires `node-serialize@0.0.4` to already be present in the lockfile; it does not install the vulnerable dependency. It runs the same production `scan`, polls the configured site's public Pulse rules endpoint until rule `18843` is served, runs `protect`, verifies the generated guard, and prints exploit/benign test requests. It writes the same manifest/widget and guard files as those underlying commands. It does not start/restart the app and does not send the printed requests. - **`map` is a local, read-only analysis command.** It walks the project's server source (skipping `node_modules`, build output and dot-directories; it does not follow symlinks out of the project unless you pass `--follow-symlinks`), parses it with the project's **own** `typescript`, and prints JSON describing the attack surface: entry points, the inputs each reads, the sinks they can reach (database / file system / process / outbound HTTP) with the npm package behind each, and evidence-backed input→sink flows, each labelled with how the link was established — from an exact read at the sink's own call site, through a transformed or cross-module link, down to the two being present together with no proven link. Static analysis is best-effort, so the output reports the *detected* surface with coverage counters — not a completeness guarantee. It writes nothing (except the file you name with `--out`) and is never invoked by `scan`, `setup`, `guide`, `protect`, or `mark-build`. -- **`map --upload` is the one opt-in that sends anything derived from your source.** It POSTs the same JSON document to `monitor/pulse/input-map/` so Patchstack can pin protection rules to your app's own parameter names instead of guessing them. What is sent is exactly what `map` prints — a structural description: route paths, parameter/field names, the dependency behind each sink, and file paths with line numbers. **No source code, no file contents, no environment variable values.** It never runs without the flag, it is skipped when no entry points are detected, and a failure to reach Patchstack is reported and ignored rather than failing your build. Omit the flag and the command stays entirely local. +- **`map --upload` is the only command that sends a description of your source.** (The runtime guard can also report rule detections, which carry route paths and parameter names — see "Runtime guard reporting" below.) It POSTs the same JSON document to `monitor/pulse/input-map/` so Patchstack can pin protection rules to your app's own parameter names instead of guessing them. What is sent is exactly what `map` prints — a structural description: route paths, parameter/field names, the dependency behind each sink, and file paths with line numbers. **No source code, no file contents, no environment variable values.** It never runs without the flag, it is skipped when no entry points are detected, and a failure to reach Patchstack is reported and ignored rather than failing your build. Omit the flag and the command stays entirely local. - **`demo-guide node-serialize` is the read-only companion.** It checks the Host-created site configuration and vulnerable lockfile entry, explains the complete local prepare/run/restart/prove/cleanup sequence, and prints the next exact command. It does not require a deployment and does not change files or contact Patchstack. - Patchstack is not WordPress-only. This connector monitors any JS/Node project — Vite, Next.js, plain vanilla JS, anything with a lockfile. @@ -78,7 +78,7 @@ This versioned reference ships inside `@patchstack/connect` and documents each s Framework-specific placement patterns: https://cdn.patchstack.com/llm.html. The site UUID is public by design — it ships in client-side HTML and is not a secret. The credential is the opposite, and `scan` writes it for you — **there is no manual step, and you should never invent or ask the user for this value**: -- `apiKey` (also `PATCHSTACK_API_KEY`, WP format `{secret}-{oauth.id}`) — one credential for both paths. It authenticates **Pulse ingest** (manifest, attack-surface map, package removal), where it is exchanged for a short-lived token rather than sent directly, and **block-log reporting** through the connector `POST /api/logs/log`, so "Threats blocked" fills in the dashboard. +- `apiKey` (also `PATCHSTACK_API_KEY`, WP format `{secret}-{oauth.id}`) — one credential for both paths. It authenticates **Pulse ingest** (manifest, attack-surface map, package removal, rule detections), where it is exchanged for a short-lived token rather than sent directly, and **block-log reporting** through the connector `POST /api/logs/log`, so "Threats blocked" fills in the dashboard. It is server-only. Never put it in the widget tag, client bundles, or public env vars (`NEXT_PUBLIC_*`, etc.). Prefer `PATCHSTACK_API_KEY` in production; `.patchstackrc.json` is fine for local DX. If it is lost, `npx @patchstack/connect login` recovers it via dashboard approval — do not delete the file and re-provision, which would create a second site. Opt out of reporting with `PATCHSTACK_TELEMETRY=off`. If the project must not carry the widget, persist `"widget": false` in `.patchstackrc.json`; otherwise the next scan re-adds it. @@ -105,6 +105,30 @@ It is server-only. Never put it in the widget tag, client bundles, or public env - If a step fails, stop and report it. Don't proceed with placeholders. - In CI where `.patchstackrc.json` can't be committed, set `PATCHSTACK_SITE_UUID` and `PATCHSTACK_API_KEY` as env vars instead. Precedence: CLI flag → env var → `.patchstackrc.json`. `login` is interactive and refuses to run in CI, so CI always takes its credential from the environment. +## Runtime guard reporting + +The runtime guard (`protect`) can report the rules that matched, so the dashboard can show what a rule +would have stopped while it is still in dry-run. Two separate paths, with different triggers: + +- **Blocked requests** go to the connector `POST /api/logs/log`, the same path the WordPress plugin uses, + and fill in "Threats blocked". This runs when the guard is holding an `apiKey` and a rule blocked a + request. Disable with `PATCHSTACK_TELEMETRY=off`, or `reportFirewallLog: false` in `createProtection`. +- **Detections that did not block** go to `monitor/pulse/detections/`. This is **off + unless you pass `reportDetections: true`** to `createProtection`; the scaffolded guard does not pass it. + It also requires a provisioned site UUID and is disabled by `PATCHSTACK_TELEMETRY=off`. It exists + because a rule carrying `dry-run` blocks nothing, so nothing else distinguishes a rule that is + protecting from one that is quietly wrong. + +What a detection report contains, per matched rule: the rule id, the request path **with any query string +removed**, the parameter names that rule reads (from the rule's own definition), which phase matched, +whether it was enforced, the identifier of the rule bundle in use, and a timestamp. Each batch also +carries a count of reports dropped when traffic outran the flush, so a partial sample is not read as a +complete one. + +What it does not contain: **the matched value, the request body, headers, cookies, or query-string +values.** Reports are batched, capped in memory, and dropped rather than retried if Patchstack cannot be +reached — a reporting failure never delays or fails a request. + ## Verifying the install - `npx @patchstack/connect status` re-prints the site UUID and dashboard URL, and checks whether the site still exists on Patchstack (`Site status: active / removed / could not be verified`). diff --git a/tests/endpoint-disclosure.test.ts b/tests/endpoint-disclosure.test.ts new file mode 100644 index 0000000..e6612b4 --- /dev/null +++ b/tests/endpoint-disclosure.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, it } from 'vitest'; +import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { join, dirname, extname } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +/** + * Every Patchstack endpoint this package can call has to be named in the shipped docs. + * + * Not a style rule. Agents `npm pack` the tarball and audit it before installing, and a capability in + * `dist/` that the docs do not mention reads as misrepresentation — it gets installs refused, and the + * refusal is correct. The detection reporter shipped without a single mention in AGENT-INSTALL.md, and + * nothing noticed, because the only thing resembling this check ("Capability contract" in CI) is about + * the map vocabulary manifest and never looks at documentation. + * + * The guard is deliberately on the ENDPOINT rather than the wording: adding an outbound path fails this + * test until someone writes down how it is described, which is the moment to decide whether it should be + * disclosed at all. + */ +const root = join(dirname(fileURLToPath(import.meta.url)), '..'); + +/** + * How each endpoint is allowed to be described. A path segment is not always the phrase a reader needs — + * `package-removed` is documented as "package removal" — so this maps the wire name to acceptable prose + * rather than demanding the literal string. + */ +const DISCLOSED_AS: Record = { + manifest: /manifest/i, + rules: /rules/i, + 'input-map': /input-map/i, + detections: /detections/i, + 'package-removed': /package-removed|package removal/i, + token: /token/i, + 'logs\/log': /logs\/log/i, +}; + +function sourceFiles(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir)) { + const full = join(dir, entry); + if (statSync(full).isDirectory()) { + // Templates are scaffolded into the target app and are not this package's outbound surface. + if (entry !== 'templates' && entry !== 'node_modules') out.push(...sourceFiles(full)); + } else if (['.ts', '.js'].includes(extname(entry))) { + out.push(full); + } + } + + return out; +} + +/** Endpoint names the package can reach, read out of the source rather than listed by hand. */ +function endpointsInSource(): Set { + const found = new Set(); + for (const file of sourceFiles(join(root, 'src'))) { + const text = readFileSync(file, 'utf8'); + for (const m of text.matchAll(/monitor\/pulse\/([a-z][a-z-]*)/g)) found.add(m[1]); + // Built from a resolved base URL instead of a literal path — how the detection reporter is written, + // and therefore the shape this check would have missed if it only read literal paths. + for (const m of text.matchAll(/\$\{baseUrl\}\/([a-z][a-z-]*)\//g)) found.add(m[1]); + if (/api\/logs\/log/.test(text)) found.add('logs\/log'); + } + + return found; +} + +describe('shipped docs disclose every endpoint the package calls', () => { + const agentInstall = readFileSync(join(root, 'AGENT-INSTALL.md'), 'utf8'); + + it('finds the endpoints in the source', () => { + // The vacuity control. If the patterns above ever stop matching — a refactor, a new way of building a + // URL — the set goes empty, every assertion below passes over nothing, and the check would report + // that all endpoints are disclosed while reading none of them. + const found = endpointsInSource(); + + expect(found.size).toBeGreaterThanOrEqual(6); + for (const known of ['manifest', 'rules', 'input-map', 'detections']) { + expect(found, `${known} should be discoverable in src/`).toContain(known); + } + }); + + it('names each of them in AGENT-INSTALL.md', () => { + for (const endpoint of endpointsInSource()) { + const pattern = DISCLOSED_AS[endpoint]; + expect( + pattern, + `"${endpoint}" is an outbound endpoint with no entry in DISCLOSED_AS. Add it to AGENT-INSTALL.md ` + + `and record here how it is described — or establish that it should not ship.`, + ).toBeDefined(); + expect(agentInstall, `AGENT-INSTALL.md must describe the ${endpoint} endpoint`).toMatch(pattern); + } + }); + + it('says what a detection report carries, and what it does not', () => { + // The specific claim that was missing, asserted rather than assumed: an agent auditing this decides + // on the payload, so "we send detections" without saying what is in them is the disclosure failing at + // the point it matters. + expect(agentInstall).toMatch(/reportDetections/); + for (const claim of [/query string/i, /matched value/i, /request body/i]) { + expect(agentInstall, `the payload description must address ${claim}`).toMatch(claim); + } + }); +}); From 5f440edf243b597376cb2a7c8e18b5daf7dca8c2 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Thu, 20 Aug 2026 17:12:40 +0200 Subject: [PATCH 2/5] Disclose four more endpoints, and stop guessing which strings are URLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the OAuth token exchange outside the guard: firewall-log.js posts to ${apiBase}/oauth/token and no extraction pattern matched it, so the file claimed every outbound endpoint was disclosed while a credential-bearing request sat outside the check. The defect was the method, not the pattern list. Enumerating URL-building idioms produces a scan that can confirm what it recognises and say nothing about the rest — an incomplete inventory answering a completeness question, which is the mistake this repo keeps finding in other layers. Widening the patterns to cover oauth/token would have left the next idiom to be missed the same way. So the polarity is inverted. Candidates are extracted broadly and every one must be classified: an endpoint with documented prose, or explicitly not an endpoint with a reason. Anything unrecognised fails the test. Applied, it immediately surfaced three more real endpoints nobody had written down — the OAuth exchange, the widget-settings lookup behind `status`, and the older get-rules path used when the guard is configured with a token — plus the device-code flow, which was already documented by showing the approval URL it prints. Three scaffolder file paths match the same shape as a URL segment. They are classified rather than filtered by a heuristic, because a rule that guessed which template literals were URLs is precisely what let the real ones through. The second finding: the runtime records every match before separately posting enforced ones to the block log, so with reportDetections on, blocked matches go to the detections endpoint too. The text said only non-blocking detections were sent, which understated what leaves the app — the direction that matters, since a reader deciding whether to enable this is owed the larger number. Co-Authored-By: Claude Opus 5 (1M context) --- AGENT-INSTALL.md | 23 +++-- tests/endpoint-disclosure.test.ts | 153 ++++++++++++++++++++---------- 2 files changed, 121 insertions(+), 55 deletions(-) diff --git a/AGENT-INSTALL.md b/AGENT-INSTALL.md index f99f967..e798dcf 100644 --- a/AGENT-INSTALL.md +++ b/AGENT-INSTALL.md @@ -112,12 +112,14 @@ would have stopped while it is still in dry-run. Two separate paths, with differ - **Blocked requests** go to the connector `POST /api/logs/log`, the same path the WordPress plugin uses, and fill in "Threats blocked". This runs when the guard is holding an `apiKey` and a rule blocked a - request. Disable with `PATCHSTACK_TELEMETRY=off`, or `reportFirewallLog: false` in `createProtection`. -- **Detections that did not block** go to `monitor/pulse/detections/`. This is **off - unless you pass `reportDetections: true`** to `createProtection`; the scaffolded guard does not pass it. - It also requires a provisioned site UUID and is disabled by `PATCHSTACK_TELEMETRY=off`. It exists - because a rule carrying `dry-run` blocks nothing, so nothing else distinguishes a rule that is - protecting from one that is quietly wrong. + request. The credential is first exchanged at `POST /oauth/token` (client credentials) for a bearer + token; the `apiKey` itself is not sent to the log endpoint. Disable with `PATCHSTACK_TELEMETRY=off`, or + `reportFirewallLog: false` in `createProtection`. +- **Every rule that matched** goes to `monitor/pulse/detections/` — including matches that + blocked, which are reported on both paths. This is **off unless you pass `reportDetections: true`** to + `createProtection`; the scaffolded guard does not pass it. It also requires a provisioned site UUID and + is disabled by `PATCHSTACK_TELEMETRY=off`. It exists because a rule carrying `dry-run` blocks nothing, + so without it nothing distinguishes a rule that is protecting from one that is quietly wrong. What a detection report contains, per matched rule: the rule id, the request path **with any query string removed**, the parameter names that rule reads (from the rule's own definition), which phase matched, @@ -129,6 +131,15 @@ What it does not contain: **the matched value, the request body, headers, cookie values.** Reports are batched, capped in memory, and dropped rather than retried if Patchstack cannot be reached — a reporting failure never delays or fails a request. +Two more endpoints the package can call, for completeness: + +- `GET monitor/widget/settings/` — how `status` tells "this site was deleted on + Patchstack" apart from "still active". It sends no credential and nothing about your project; the site + UUID in the path is the whole request. +- `GET api/get-rules/3` — the older rules path, used only when the guard is configured with a `token` + instead of a site UUID. The zero-configuration flow provisions a site UUID and uses + `monitor/pulse/rules/` instead, so this is unreachable unless you pass `token` yourself. + ## Verifying the install - `npx @patchstack/connect status` re-prints the site UUID and dashboard URL, and checks whether the site still exists on Patchstack (`Site status: active / removed / could not be verified`). diff --git a/tests/endpoint-disclosure.test.ts b/tests/endpoint-disclosure.test.ts index e6612b4..5d7e4b3 100644 --- a/tests/endpoint-disclosure.test.ts +++ b/tests/endpoint-disclosure.test.ts @@ -7,30 +7,56 @@ import { fileURLToPath } from 'node:url'; * Every Patchstack endpoint this package can call has to be named in the shipped docs. * * Not a style rule. Agents `npm pack` the tarball and audit it before installing, and a capability in - * `dist/` that the docs do not mention reads as misrepresentation — it gets installs refused, and the - * refusal is correct. The detection reporter shipped without a single mention in AGENT-INSTALL.md, and - * nothing noticed, because the only thing resembling this check ("Capability contract" in CI) is about - * the map vocabulary manifest and never looks at documentation. + * `dist/` the docs do not mention reads as misrepresentation — it gets installs refused, and the refusal + * is correct. The detection reporter shipped with no mention anywhere and nothing noticed, because the + * only comparable check ("Capability contract" in CI) is about the map vocabulary manifest and never + * reads documentation. * - * The guard is deliberately on the ENDPOINT rather than the wording: adding an outbound path fails this - * test until someone writes down how it is described, which is the moment to decide whether it should be - * disclosed at all. + * ## Why this file is shaped the way it is + * + * The first version enumerated URL-building IDIOMS — a literal `monitor/pulse/x`, a `${baseUrl}/x/` + * template, the log path — and asserted that everything it recognised was disclosed. It passed while + * three real endpoints sat outside it: the OAuth token exchange, the widget-settings lookup, and the + * older `get-rules` path. A scan that recognises some shapes cannot support the sentence "every endpoint + * is disclosed"; it can only say the ones it happened to match were. + * + * So the polarity is inverted here. Candidates are extracted broadly, and every one must be CLASSIFIED — + * either an endpoint with a documented description, or explicitly not an endpoint with a reason. Anything + * unrecognised fails. New URL, new comment, new fixture path: all of them stop this test until someone + * decides which it is, and that decision is the point. */ const root = join(dirname(fileURLToPath(import.meta.url)), '..'); -/** - * How each endpoint is allowed to be described. A path segment is not always the phrase a reader needs — - * `package-removed` is documented as "package removal" — so this maps the wire name to acceptable prose - * rather than demanding the literal string. - */ +/** Endpoints the package can reach, and the wording that counts as describing each one. */ const DISCLOSED_AS: Record = { - manifest: /manifest/i, - rules: /rules/i, - 'input-map': /input-map/i, - detections: /detections/i, - 'package-removed': /package-removed|package removal/i, - token: /token/i, - 'logs\/log': /logs\/log/i, + 'monitor/pulse/manifest': /manifest/i, + 'monitor/pulse/rules': /monitor\/pulse\/rules|pulse rules/i, + 'monitor/pulse/input-map': /input-map/i, + // Built from the resolved Pulse base rather than a literal path, which is why it needs the second + // extraction pattern below — and why the first version of this file could not see it. + detections: /monitor\/pulse\/detections/i, + 'monitor/pulse/package-removed': /package-removed|package removal/i, + 'monitor/pulse/token': /short-lived token|pulse\/token/i, + 'monitor/widget/settings': /monitor\/widget\/settings/i, + 'monitor/claim': /claim/i, + 'oauth/token': /oauth\/token/i, + // The RFC 8628 login flow. Documented by showing the approval URL the command prints, which is the + // form a reader actually needs: it is where they are sent. + device: /monitor\/pulse\/device/i, + 'api/logs/log': /logs\/log/i, + 'api/get-rules/3': /get-rules/i, +}; + +/** Path-shaped strings that are not endpoints of ours. Each needs a reason, not just an entry. */ +const NOT_AN_ENDPOINT: Record = { + 'monitor/pulse': 'the base path the per-site endpoints are built on, not an endpoint itself', + 'api/tasks': "a route in the demo's own throwaway app on localhost, used as the default exploit target", + // Paths INSIDE the target project, written by the scaffolder. They match the same shape as a URL + // segment appended to a base, and are classified rather than filtered out by a heuristic: a rule that + // guessed which template literals were URLs is what let three real endpoints through last time. + patchstack: 'a directory in the target app that the scaffolder writes the guard into', + rules: 'a file the scaffolder writes beside the guard in the target app', + guard: 'the guard file the scaffolder writes into the target app', }; function sourceFiles(dir: string): string[] { @@ -38,7 +64,7 @@ function sourceFiles(dir: string): string[] { for (const entry of readdirSync(dir)) { const full = join(dir, entry); if (statSync(full).isDirectory()) { - // Templates are scaffolded into the target app and are not this package's outbound surface. + // Templates are scaffolded into the target app; they are that app's surface, not this package's. if (entry !== 'templates' && entry !== 'node_modules') out.push(...sourceFiles(full)); } else if (['.ts', '.js'].includes(extname(entry))) { out.push(full); @@ -48,16 +74,34 @@ function sourceFiles(dir: string): string[] { return out; } -/** Endpoint names the package can reach, read out of the source rather than listed by hand. */ -function endpointsInSource(): Set { - const found = new Set(); +/** Comments carry example URLs that are not endpoints; the code is what makes a request. */ +function stripComments(text: string): string { + return text.replace(/\/\*[\s\S]*?\*\//g, '').replace(/(^|[\s;,)])\/\/.*$/gm, '$1'); +} + +const API_ROOTS = ['monitor', 'api', 'oauth']; + +/** Every path-shaped candidate in the source, by whichever way its URL is assembled. */ +function candidates(): Map> { + const found = new Map>(); + const add = (path: string, file: string) => { + const set = found.get(path) ?? new Set(); + set.add(file); + found.set(path, set); + }; + for (const file of sourceFiles(join(root, 'src'))) { - const text = readFileSync(file, 'utf8'); - for (const m of text.matchAll(/monitor\/pulse\/([a-z][a-z-]*)/g)) found.add(m[1]); - // Built from a resolved base URL instead of a literal path — how the detection reporter is written, - // and therefore the shape this check would have missed if it only read literal paths. - for (const m of text.matchAll(/\$\{baseUrl\}\/([a-z][a-z-]*)\//g)) found.add(m[1]); - if (/api\/logs\/log/.test(text)) found.add('logs\/log'); + const text = stripComments(readFileSync(file, 'utf8')); + const name = file.slice(root.length + 1); + + // A path written under one of the API roots, however the rest of the URL is built. + for (const m of text.matchAll(/\b(monitor|api|oauth)\/([a-z][a-z0-9/-]*)/g)) { + add(`${m[1]}/${m[2]}`.replace(/\/$/, ''), name); + } + // A segment appended to an already-resolved base URL, where the root is not in the literal at all. + for (const m of text.matchAll(/\$\{[A-Za-z_$][\w$]*\}\/([a-z][a-z0-9-]*)/g)) { + if (!API_ROOTS.includes(m[1])) add(m[1], name); + } } return found; @@ -66,37 +110,48 @@ function endpointsInSource(): Set { describe('shipped docs disclose every endpoint the package calls', () => { const agentInstall = readFileSync(join(root, 'AGENT-INSTALL.md'), 'utf8'); - it('finds the endpoints in the source', () => { - // The vacuity control. If the patterns above ever stop matching — a refactor, a new way of building a - // URL — the set goes empty, every assertion below passes over nothing, and the check would report - // that all endpoints are disclosed while reading none of them. - const found = endpointsInSource(); + it('classifies every path-shaped candidate it finds', () => { + // The completeness assertion. An unclassified candidate is not skipped — it fails, because the only + // honest way to claim every endpoint is disclosed is to have accounted for everything found. + const unclassified = [...candidates().entries()] + .filter(([path]) => !(path in DISCLOSED_AS) && !(path in NOT_AN_ENDPOINT)) + .map(([path, files]) => `${path} (in ${[...files].sort().join(', ')})`); - expect(found.size).toBeGreaterThanOrEqual(6); - for (const known of ['manifest', 'rules', 'input-map', 'detections']) { - expect(found, `${known} should be discoverable in src/`).toContain(known); + expect( + unclassified, + 'Unclassified path(s). If the package can call it, describe it in AGENT-INSTALL.md and add it to ' + + 'DISCLOSED_AS; if it is not an endpoint of ours, add it to NOT_AN_ENDPOINT with the reason.', + ).toEqual([]); + }); + + it('finds the endpoints at all', () => { + // The vacuity control. If both patterns stop matching, every assertion here passes over an empty set + // and the file reports total disclosure while reading nothing. + const found = candidates(); + + expect(found.size).toBeGreaterThanOrEqual(10); + for (const known of ['monitor/pulse/manifest', 'oauth/token', 'api/logs/log', 'detections']) { + expect([...found.keys()], `${known} should be discoverable in src/`).toContain(known); } }); - it('names each of them in AGENT-INSTALL.md', () => { - for (const endpoint of endpointsInSource()) { - const pattern = DISCLOSED_AS[endpoint]; - expect( - pattern, - `"${endpoint}" is an outbound endpoint with no entry in DISCLOSED_AS. Add it to AGENT-INSTALL.md ` + - `and record here how it is described — or establish that it should not ship.`, - ).toBeDefined(); - expect(agentInstall, `AGENT-INSTALL.md must describe the ${endpoint} endpoint`).toMatch(pattern); + it('names each endpoint in AGENT-INSTALL.md', () => { + for (const [path, pattern] of Object.entries(DISCLOSED_AS)) { + expect(agentInstall, `AGENT-INSTALL.md must describe ${path}`).toMatch(pattern); } }); it('says what a detection report carries, and what it does not', () => { - // The specific claim that was missing, asserted rather than assumed: an agent auditing this decides - // on the payload, so "we send detections" without saying what is in them is the disclosure failing at - // the point it matters. expect(agentInstall).toMatch(/reportDetections/); for (const claim of [/query string/i, /matched value/i, /request body/i]) { expect(agentInstall, `the payload description must address ${claim}`).toMatch(claim); } }); + + it('does not describe detection reporting as limited to non-blocking matches', () => { + // The runtime records EVERY match, then additionally posts the enforced ones to the block log. Saying + // only non-blocking detections are sent understates what leaves the app, which is the direction that + // matters: a reader deciding whether to enable this is owed the larger number, not the smaller one. + expect(agentInstall).toMatch(/every rule that matched/i); + }); }); From 512256d3ab49f50aab758e18f8b30a57558dfdc9 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Thu, 20 Aug 2026 17:20:03 +0200 Subject: [PATCH 3/5] Say that a parameter identifier names its request region, and is sent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The exclusion list read "the matched value, the request body, headers, cookies, or query-string values", which scans as "headers and cookies are not sent". That is true of their values and false of their names. ruleParameters returns each condition's `parameter` verbatim, and those carry the request region they refer to: post.title, get.redirect_to, cookie.session, server.HTTP_AUTHORIZATION. So a rule inspecting a cookie or an Authorization header does send that cookie's or header's name. The old sentence claimed a privacy boundary wider than the code's, in the direction that flatters us, which is the direction an auditing agent is right to punish. Now stated as two separate facts. The identifiers are sent and name their region, read from the rule's definition rather than from traffic, so they describe what is being screened rather than what any request held. And the exclusion is about values only: no value that matched, no request body, and no value of any header, cookie or query-string parameter, including the ones named. Guarded as an absence as well as a presence: a test fails on any exclusion clause that names headers or cookies without scoping to their values, because that is a sentence someone would write again while tightening the prose. One earlier assertion was matching a literal phrase rather than the claim, and this rewrite broke it — it now matches either phrasing while still requiring the substance. Co-Authored-By: Claude Opus 5 (1M context) --- AGENT-INSTALL.md | 13 ++++++++++--- tests/endpoint-disclosure.test.ts | 21 ++++++++++++++++++++- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/AGENT-INSTALL.md b/AGENT-INSTALL.md index e798dcf..436b82c 100644 --- a/AGENT-INSTALL.md +++ b/AGENT-INSTALL.md @@ -127,9 +127,16 @@ whether it was enforced, the identifier of the rule bundle in use, and a timesta carries a count of reports dropped when traffic outran the flush, so a partial sample is not read as a complete one. -What it does not contain: **the matched value, the request body, headers, cookies, or query-string -values.** Reports are batched, capped in memory, and dropped rather than retried if Patchstack cannot be -reached — a reporting failure never delays or fails a request. +The parameter names are **identifiers, and they name the request region they refer to** — `post.title`, +`get.redirect_to`, `cookie.session`, `server.HTTP_AUTHORIZATION`. So a rule that inspects a cookie or an +`Authorization` header sends that cookie's or header's **name**. They are read from the rule's own +definition, not from your traffic, so they describe what is being screened rather than what any request +contained. + +What it does not contain: **no values of any kind.** Not the value that matched, not the request body, +and not the value of any header, cookie or query-string parameter — including those of the parameters +named above. Reports are batched, capped in memory, and dropped rather than retried if Patchstack cannot +be reached — a reporting failure never delays or fails a request. Two more endpoints the package can call, for completeness: diff --git a/tests/endpoint-disclosure.test.ts b/tests/endpoint-disclosure.test.ts index 5d7e4b3..237e8a1 100644 --- a/tests/endpoint-disclosure.test.ts +++ b/tests/endpoint-disclosure.test.ts @@ -143,11 +143,30 @@ describe('shipped docs disclose every endpoint the package calls', () => { it('says what a detection report carries, and what it does not', () => { expect(agentInstall).toMatch(/reportDetections/); - for (const claim of [/query string/i, /matched value/i, /request body/i]) { + // Phrasing-tolerant, substance-strict: the claim has to be there, not any particular sentence. + for (const claim of [/query string|query-string/i, /matched value|value that matched/i, /request body/i]) { expect(agentInstall, `the payload description must address ${claim}`).toMatch(claim); } }); + it('separates parameter identifiers from values, and does not exclude what it sends', () => { + // `ruleParameters` returns each condition's `parameter` verbatim, and those name a request region: + // `cookie.session`, `server.HTTP_AUTHORIZATION`. So a rule inspecting a cookie or an Authorization + // header sends that name. The exclusion list previously read "the matched value, the request body, + // headers, cookies, or query-string values", which scans as "headers and cookies are not sent" — + // true of their values, false of their names, and wrong in the direction that flatters us. + expect(agentInstall, 'must say the identifiers carry their request region').toMatch(/request region/i); + expect(agentInstall, 'must show a region-qualified example').toMatch(/cookie\.session/); + expect(agentInstall, 'must show a header example, since that is the sensitive case').toMatch(/server\.HTTP_/); + expect(agentInstall, 'the exclusion must be about values').toMatch(/no values of any kind/i); + + // The regression itself: an exclusion clause that names headers or cookies without scoping to their + // values. Asserted as an absence because the overclaim is a sentence someone would write again while + // tightening the prose. + const exclusion = /does not contain[^.]*?\b(headers|cookies)\b(?![^.]*\bvalue)/i; + expect(agentInstall, 'headers/cookies may only be excluded as VALUES').not.toMatch(exclusion); + }); + it('does not describe detection reporting as limited to non-blocking matches', () => { // The runtime records EVERY match, then additionally posts the enforced ones to the block log. Saying // only non-blocking detections are sent understates what leaves the app, which is the direction that From e50305c108d8fa60c014dd768a90f23105c09813 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Thu, 20 Aug 2026 17:25:43 +0200 Subject: [PATCH 4/5] Check the disclosure against a real payload, not against itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three rounds of review on this section found the same thing each time: the prose was wrong and the code was right. An endpoint nobody had written down, a trigger described as narrower than it is, and a privacy boundary claimed wider than the code's. The guards that existed assert that words appear. None of them reads the payload, so none could tell whether the words were true of it. This posts a detection through the real reporter, captures the serialized bytes, and asks two things of them. Every field emitted must be described. The check iterates the payload's own keys, so a field added to the report fails until someone writes it down — the failure names the key and says what to do. A vacuity control pairs with it, because iterating the payload's keys is satisfied perfectly by a payload with no keys: the documented set and the emitted set must be equal, not merely compatible. Every excluded value must actually be excluded. Sentinels are planted where the reporter could pick them up — a matched value, a request body, a header value, a cookie value, a query string — and asserted absent from the bytes. That proves the boundary rather than restating it. The query-string sentinel gets its own control: a reporter that sent no route at all would pass the exclusion while losing the field the disclosure describes. The rule used reads a cookie and an Authorization header on purpose. Those parameter names ARE sent, which is the distinction the wording got wrong, and a test that only looked for absent values would have agreed with the wrong version. Documentation stays hand-written. What is mechanised here is the inventory: generated prose would read as machine output to the agents who audit this file, and the wording is doing adversarial-UX work a serializer cannot do. Co-Authored-By: Claude Opus 5 (1M context) --- .../detection-payload-contract.test.ts | 170 ++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 tests/protect/detection-payload-contract.test.ts diff --git a/tests/protect/detection-payload-contract.test.ts b/tests/protect/detection-payload-contract.test.ts new file mode 100644 index 0000000..304f916 --- /dev/null +++ b/tests/protect/detection-payload-contract.test.ts @@ -0,0 +1,170 @@ +import { describe, expect, it, vi } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { join, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createDetectionReporter } from '../../src/protect/detections.js'; + +/** + * The disclosure, checked against a REAL serialized payload rather than against itself. + * + * Three rounds of review on this section all found the same thing: the prose was wrong and the code was + * right. An endpoint nobody had written down, a trigger described as narrower than it is, and a privacy + * boundary claimed wider than the code's. The guards that existed asserted that words APPEAR — none of + * them read the payload, so none could tell whether the words were TRUE OF IT. + * + * So this test posts a detection through the real reporter, captures the bytes, and asks two questions + * of them: is every field we emit described, and does every exclusion we claim actually hold. + * + * The prose stays hand-written. What is mechanised is the inventory — generated disclosure text would + * read as machine output to the agents auditing this file, and the wording is doing adversarial-UX work a + * serializer cannot do. + */ +const root = join(dirname(fileURLToPath(import.meta.url)), '..', '..'); +const disclosure = readFileSync(join(root, 'AGENT-INSTALL.md'), 'utf8'); + +/** + * Every key the payload may carry, and the wording that describes it. + * + * A key with no entry fails. That is the point: adding a field to the report stops the build until + * someone writes it down or decides it should not ship. + */ +const FIELD_DISCLOSURE: Record = { + rule_id: /rule id/i, + route: /request path/i, + parameters: /parameter names/i, + phase: /which phase matched/i, + enforced: /whether it was enforced/i, + rules_etag: /identifier of the rule bundle/i, + detected_at: /timestamp/i, +}; + +/** Envelope keys, described separately because they are per-batch rather than per-detection. */ +const ENVELOPE_DISCLOSURE: Record = { + detections: /per matched rule/i, + dropped: /count of reports dropped/i, +}; + +/** + * Values planted where the reporter could pick them up, each with the sentence that promises it will not. + * Proving the boundary beats restating it: if a future change starts forwarding any of these, the bytes + * change and this fails. + */ +const VALUE_EXCLUSIONS = [ + { what: 'the value that matched', sentinel: 'SENTINEL-MATCHED-VALUE', disclosed: /value that matched|matched value/i }, + { what: 'the request body', sentinel: 'SENTINEL-REQUEST-BODY', disclosed: /request body/i }, + { what: 'a header value', sentinel: 'SENTINEL-HEADER-VALUE', disclosed: /header/i }, + { what: 'a cookie value', sentinel: 'SENTINEL-COOKIE-VALUE', disclosed: /cookie/i }, + { what: 'a query-string value', sentinel: 'SENTINEL-QUERY-VALUE', disclosed: /query.string|query string/i }, +]; + +/** + * A rule that reads the sensitive regions on purpose — a cookie and an Authorization header — because + * those are the parameters whose NAMES are sent, which is the distinction the disclosure has to make. + */ +const RULE = { + id: 'PS-CVE-2026-0001', + rule_v2: [ + { parameter: 'post.title', match: { type: 'contains', value: 'x' } }, + { parameter: 'cookie.session', match: { type: 'contains', value: 'x' } }, + { parameter: 'server.HTTP_AUTHORIZATION', match: { type: 'contains', value: 'x' } }, + ], +}; + +/** One real detection, serialized by the real reporter. */ +async function capturePayload(): Promise<{ raw: string; body: Record }> { + let raw = ''; + const fetchImpl = vi.fn(async (_url: string, init: RequestInit) => { + raw = String(init.body); + + return new Response('{}', { status: 202 }); + }); + + const reporter = createDetectionReporter({ + siteUuid: 'site-contract', + baseUrl: 'https://api.test/monitor/pulse', + rulesEtag: '"bundle-7"', + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + reporter.record({ + rule: RULE, + phase: 'request', + mode: 'block', + // The query string carries a sentinel: the route is supposed to arrive with it stripped. + path: '/checkout/confirm?token=SENTINEL-QUERY-VALUE', + // Fields a detection could plausibly grow, planted so that forwarding them is a test failure rather + // than a silent change in what leaves the app. + matchedValue: 'SENTINEL-MATCHED-VALUE', + body: 'SENTINEL-REQUEST-BODY', + headers: { authorization: 'SENTINEL-HEADER-VALUE' }, + cookies: { session: 'SENTINEL-COOKIE-VALUE' }, + } as never); + + reporter.flush(); + await vi.waitFor(() => expect(raw).not.toBe('')); + + return { raw, body: JSON.parse(raw) as Record }; +} + +describe('the detection payload matches what AGENT-INSTALL.md says about it', () => { + it('describes every field it emits', async () => { + const { body } = await capturePayload(); + const detection = (body.detections as Array>)[0]; + + for (const key of Object.keys(detection)) { + const pattern = FIELD_DISCLOSURE[key]; + expect( + pattern, + `The payload emits "${key}" and nothing in FIELD_DISCLOSURE covers it. Describe it in ` + + `AGENT-INSTALL.md and map it here — or establish that it should not be sent.`, + ).toBeDefined(); + expect(disclosure, `AGENT-INSTALL.md must describe the "${key}" field`).toMatch(pattern); + } + + for (const key of Object.keys(body)) { + const pattern = ENVELOPE_DISCLOSURE[key]; + expect(pattern, `The batch envelope carries "${key}" with no entry in ENVELOPE_DISCLOSURE.`).toBeDefined(); + expect(disclosure, `AGENT-INSTALL.md must describe the "${key}" envelope field`).toMatch(pattern); + } + }); + + it('emits the fields it claims to, not a subset', async () => { + // The vacuity control for the check above: it iterates over the payload's keys, so a reporter that + // emitted nothing would satisfy it perfectly while the disclosure described a payload that no longer + // exists. Every documented field has to actually be there. + const { body } = await capturePayload(); + const detection = (body.detections as Array>)[0]; + + expect(Object.keys(detection).sort()).toEqual(Object.keys(FIELD_DISCLOSURE).sort()); + expect(Object.keys(body).sort()).toEqual(Object.keys(ENVELOPE_DISCLOSURE).sort()); + }); + + it('sends parameter identifiers, including their request region', async () => { + // The half the disclosure got wrong: these names ARE sent, and they say which region they read. A + // test that only checked for absent values would have agreed with the incorrect wording. + const { body } = await capturePayload(); + const detection = (body.detections as Array>)[0]; + + expect(detection.parameters).toContain('cookie.session'); + expect(detection.parameters).toContain('server.HTTP_AUTHORIZATION'); + expect(disclosure, 'the disclosure must say identifiers name their request region').toMatch(/request region/i); + }); + + it('excludes every value it promises to exclude', async () => { + const { raw } = await capturePayload(); + + for (const { what, sentinel, disclosed } of VALUE_EXCLUSIONS) { + expect(raw, `${what} must not reach the wire`).not.toContain(sentinel); + expect(disclosure, `AGENT-INSTALL.md must promise that ${what} is excluded`).toMatch(disclosed); + } + }); + + it('keeps the route while dropping the query string, rather than dropping both', async () => { + // The control for the query-string sentinel: a reporter that sent no route at all would pass the + // exclusion check while losing the field the disclosure describes. + const { body } = await capturePayload(); + const detection = (body.detections as Array>)[0]; + + expect(detection.route).toBe('/checkout/confirm'); + }); +}); From 3e958305bf8149c1c4ae71281ca1cd9c269b75ac Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Thu, 20 Aug 2026 17:32:27 +0200 Subject: [PATCH 5/5] Tie each classification to the call site that earned it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exemptions were keyed on the path string alone, so `rules` — a file the scaffolder writes — was exempt everywhere. A future request built as `${pulseBase}/rules/${uuid}` produces the same candidate from a different file and would have passed as "not an endpoint": this file's own original mistake repeated one level up, an exemption answering beyond the evidence that earned it. Classification is now per occurrence. A candidate is exempt only in the files that establish the exemption, disclosed only in the files that establish the endpoint, and unclassified anywhere else. A meta-assertion keeps it honest by refusing any bare-segment entry that does not name its files, since a rooted path cannot collide with a filename but a bare segment can. Making that change surfaced a second bug in the extraction. The interpolation pattern required `${identifier}`, and the rules client builds its URL from `${this.#baseUrl}` — a member expression the pattern could not see. So a real endpoint was invisible here for the same reason the earlier version missed the OAuth exchange: recognising one spelling of "a base URL" is not recognising a base URL. Widened to any expression, which immediately turned up four endpoints under their interpolated aliases, `rules` among them. `rules` is now in both tables, and both readings are correct because each is tied to its files: an endpoint in the client, a scaffolded file in the installer. That collision is the concrete argument for the scoping. The granularity is the file, not the call site, so a request added to a file that already exempts the same word is still missed. Narrowing further needs parsing rather than patterns; the residue is small because these exemptions live in scaffolder modules that write files and make no requests. Recorded next to the table rather than left for someone to discover. Co-Authored-By: Claude Opus 5 (1M context) --- tests/endpoint-disclosure.test.ts | 151 ++++++++++++++++++++++-------- 1 file changed, 111 insertions(+), 40 deletions(-) diff --git a/tests/endpoint-disclosure.test.ts b/tests/endpoint-disclosure.test.ts index 237e8a1..286f88c 100644 --- a/tests/endpoint-disclosure.test.ts +++ b/tests/endpoint-disclosure.test.ts @@ -27,36 +27,83 @@ import { fileURLToPath } from 'node:url'; */ const root = join(dirname(fileURLToPath(import.meta.url)), '..'); -/** Endpoints the package can reach, and the wording that counts as describing each one. */ -const DISCLOSED_AS: Record = { - 'monitor/pulse/manifest': /manifest/i, - 'monitor/pulse/rules': /monitor\/pulse\/rules|pulse rules/i, - 'monitor/pulse/input-map': /input-map/i, - // Built from the resolved Pulse base rather than a literal path, which is why it needs the second - // extraction pattern below — and why the first version of this file could not see it. - detections: /monitor\/pulse\/detections/i, - 'monitor/pulse/package-removed': /package-removed|package removal/i, - 'monitor/pulse/token': /short-lived token|pulse\/token/i, - 'monitor/widget/settings': /monitor\/widget\/settings/i, - 'monitor/claim': /claim/i, - 'oauth/token': /oauth\/token/i, - // The RFC 8628 login flow. Documented by showing the approval URL the command prints, which is the - // form a reader actually needs: it is where they are sent. - device: /monitor\/pulse\/device/i, - 'api/logs/log': /logs\/log/i, - 'api/get-rules/3': /get-rules/i, +/** + * Endpoints the package can reach, and the wording that counts as describing each one. + * + * `files` is required for any key that is a BARE SEGMENT rather than a rooted path, and there is a + * meta-assertion below enforcing that. A bare segment is ambiguous by construction — `detections` is an + * endpoint here and could be a filename somewhere else — so it may only be recognised where it was + * actually established, never globally. + */ +const DISCLOSED_AS: Record = { + 'monitor/pulse/manifest': { doc: /manifest/i }, + 'monitor/pulse/rules': { doc: /monitor\/pulse\/rules|pulse rules/i }, + 'monitor/pulse/input-map': { doc: /input-map/i }, + // Built from the resolved Pulse base rather than a literal path, which is why the second extraction + // pattern exists — and why the first version of this file could not see it. + detections: { doc: /monitor\/pulse\/detections/i, files: ['src/protect/detections.js'] }, + 'monitor/pulse/package-removed': { doc: /package-removed|package removal/i }, + 'monitor/pulse/token': { doc: /short-lived token|pulse\/token/i }, + 'monitor/widget/settings': { doc: /monitor\/widget\/settings/i }, + 'monitor/claim': { doc: /claim/i }, + 'oauth/token': { doc: /oauth\/token/i }, + 'api/logs/log': { doc: /logs\/log/i }, + 'api/get-rules/3': { doc: /get-rules/i }, + // The RFC 8628 login flow, documented by showing the approval URL the command prints. + device: { doc: /monitor\/pulse\/device/i, files: ['src/login.ts'] }, + + // The same endpoints again, reached through an interpolated base rather than a literal path. Listing + // them as bare segments is not duplication: it is the second identity each one has in the source, and + // the classification has to hold for both or the check is only as good as the spelling it happened to + // meet first. + 'input-map': { doc: /input-map/i, files: ['src/client.ts'] }, + 'package-removed': { doc: /package-removed|package removal/i, files: ['src/client.ts'] }, + // `rules` is BOTH: an endpoint here, and a file the scaffolder writes (see NOT_AN_ENDPOINT). Exactly + // the collision that makes global classification unsafe — and both readings are correct, because each + // is tied to the files that establish it. + rules: { + doc: /monitor\/pulse\/rules|pulse rules/i, + files: ['src/client.ts', 'src/protect/engine/pulse-client.js'], + }, + token: { doc: /short-lived token|pulse\/token/i, files: ['src/pulse-token.ts'] }, }; -/** Path-shaped strings that are not endpoints of ours. Each needs a reason, not just an entry. */ -const NOT_AN_ENDPOINT: Record = { - 'monitor/pulse': 'the base path the per-site endpoints are built on, not an endpoint itself', - 'api/tasks': "a route in the demo's own throwaway app on localhost, used as the default exploit target", - // Paths INSIDE the target project, written by the scaffolder. They match the same shape as a URL - // segment appended to a base, and are classified rather than filtered out by a heuristic: a rule that - // guessed which template literals were URLs is what let three real endpoints through last time. - patchstack: 'a directory in the target app that the scaffolder writes the guard into', - rules: 'a file the scaffolder writes beside the guard in the target app', - guard: 'the guard file the scaffolder writes into the target app', +/** + * Path-shaped strings that are not endpoints of ours, scoped to the files that establish them. + * + * Scoped, because the justification is about a call site and not about a word. `rules` is a file the + * scaffolder writes — in the scaffolder. A future `${pulseBase}/rules/${uuid}` request would produce the + * same candidate from a different file, and a globally-keyed exemption would wave a real endpoint through + * as "not an endpoint". That is this file's own original mistake repeated one level up: an exemption + * answering beyond the evidence that earned it. + * + * The granularity is the FILE, not the call site, and that is the remaining limit: an outbound request + * added to a file that already exempts the same word would still be missed. Narrowing further needs real + * parsing rather than patterns. What makes the residue small is that these exemptions live in scaffolder + * modules, which write files and make no requests — an outbound call appearing in one is odd enough to + * notice in review, which is the check this backstops rather than replaces. + */ +const NOT_AN_ENDPOINT: Record = { + 'monitor/pulse': { + why: 'the base path the per-site endpoints are built on, not an endpoint itself', + files: ['src/login.ts', 'src/protect/detections.js', 'src/protect/engine/pulse-client.js'], + }, + 'api/tasks': { + why: "a route in the demo's own throwaway app on localhost, used as the default exploit target", + files: ['src/cli.ts', 'src/demo.ts'], + }, + patchstack: { + why: 'a directory in the target app that the scaffolder writes the guard into', + files: ['src/protect/install/adapters/next.ts', 'src/protect/install/seam.ts'], + }, + rules: { + why: 'a file the scaffolder writes beside the guard in the target app', + files: ['src/protect/install/generic.ts'], + }, + guard: { + why: 'the guard file the scaffolder writes into the target app', + files: ['src/protect/install/generic.ts'], + }, }; function sourceFiles(dir: string): string[] { @@ -99,7 +146,11 @@ function candidates(): Map> { add(`${m[1]}/${m[2]}`.replace(/\/$/, ''), name); } // A segment appended to an already-resolved base URL, where the root is not in the literal at all. - for (const m of text.matchAll(/\$\{[A-Za-z_$][\w$]*\}\/([a-z][a-z0-9-]*)/g)) { + // The interpolation is matched as ANY expression, not an identifier: the rules client builds its URL + // from `${this.#baseUrl}`, and an identifier-only pattern silently skipped it — so a real endpoint + // was invisible here for the same reason the earlier version missed the OAuth exchange. Recognising + // one spelling of "a base URL" is not the same as recognising a base URL. + for (const m of text.matchAll(/\$\{[^}]*\}\/([a-z][a-z0-9-]*)/g)) { if (!API_ROOTS.includes(m[1])) add(m[1], name); } } @@ -110,20 +161,40 @@ function candidates(): Map> { describe('shipped docs disclose every endpoint the package calls', () => { const agentInstall = readFileSync(join(root, 'AGENT-INSTALL.md'), 'utf8'); - it('classifies every path-shaped candidate it finds', () => { - // The completeness assertion. An unclassified candidate is not skipped — it fails, because the only - // honest way to claim every endpoint is disclosed is to have accounted for everything found. - const unclassified = [...candidates().entries()] - .filter(([path]) => !(path in DISCLOSED_AS) && !(path in NOT_AN_ENDPOINT)) - .map(([path, files]) => `${path} (in ${[...files].sort().join(', ')})`); + it('classifies every path-shaped candidate, at every call site it appears in', () => { + // Per OCCURRENCE, not per path. A classification earned in one file says nothing about the same + // string appearing in another, and treating it as though it did is how a real endpoint would inherit + // an unrelated template-path exemption. + const unclassified: string[] = []; + for (const [path, files] of candidates()) { + for (const file of files) { + const exempt = NOT_AN_ENDPOINT[path]; + if (exempt && exempt.files.includes(file)) continue; + const disclosed = DISCLOSED_AS[path]; + if (disclosed && (disclosed.files === undefined || disclosed.files.includes(file))) continue; + unclassified.push(`${path} (in ${file})`); + } + } expect( - unclassified, - 'Unclassified path(s). If the package can call it, describe it in AGENT-INSTALL.md and add it to ' + - 'DISCLOSED_AS; if it is not an endpoint of ours, add it to NOT_AN_ENDPOINT with the reason.', + unclassified.sort(), + 'Unclassified occurrence(s). If the package can call it, describe it in AGENT-INSTALL.md and add ' + + 'the file to DISCLOSED_AS; if it is not an endpoint of ours, add the file to NOT_AN_ENDPOINT ' + + 'with the reason. An entry for the same string in another file does not cover this one.', ).toEqual([]); }); + it('requires a bare segment to name the files that establish it', () => { + // The meta-assertion that keeps the scoping honest: a rooted path like `monitor/pulse/rules` cannot + // collide with a scaffolder filename, but a bare `detections` or `rules` can. Any bare-segment entry + // recognised globally would reopen exactly the hole this scoping closes. + const unscoped = Object.entries(DISCLOSED_AS) + .filter(([path, entry]) => !path.includes('/') && entry.files === undefined) + .map(([path]) => path); + + expect(unscoped, 'bare-segment endpoints must declare `files`').toEqual([]); + }); + it('finds the endpoints at all', () => { // The vacuity control. If both patterns stop matching, every assertion here passes over an empty set // and the file reports total disclosure while reading nothing. @@ -136,8 +207,8 @@ describe('shipped docs disclose every endpoint the package calls', () => { }); it('names each endpoint in AGENT-INSTALL.md', () => { - for (const [path, pattern] of Object.entries(DISCLOSED_AS)) { - expect(agentInstall, `AGENT-INSTALL.md must describe ${path}`).toMatch(pattern); + for (const [path, { doc }] of Object.entries(DISCLOSED_AS)) { + expect(agentInstall, `AGENT-INSTALL.md must describe ${path}`).toMatch(doc); } });