From 97dd837c7196b7494f48b203fc382ca9f40b420a Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Thu, 20 Aug 2026 14:39:57 +0200 Subject: [PATCH 1/3] Fix an unresolved import, and declare the reporting capability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **The fix first, because it is on main.** `runtime.js` calls `createDetectionReporter` and never imported it, so `reportDetections: true` threw `ReferenceError` at boot. Being opt-in, nobody reached it — including the tests: they exercised the reporter directly, or asserted that nothing is posted when the feature is OFF, and neither path enters the branch that builds it. A feature that only fails for whoever turns it on, guarded by tests that never turn it on. Closed with the test that was missing: switch reporting on, fire a matching request, and assert a detection reaches the endpoint. Removing the import fails it, which is what the previous suite could not do. **Then the capability.** Detections are sent only when a rule fires, so silence at the server means one of three things — nothing matched, reporting is off, or reports are not arriving — and nothing distinguished them. The rules fetch now carries `X-Patchstack-Detections: enabled` when reporting is on: a request that is already periodic, already authenticated, and already identifies the site. No new outbound path and no request data. A capability, not a heartbeat. The header says "this guard reports"; the server records when IT saw that. A client-supplied "alive as of" would be a value from outside dressed as an observation, and a wrong or stale clock is exactly what it would fake. Asserted in both directions: a guard with reporting on says so, and a guard without it stays silent. The second matters as much — a declaration that is sent regardless tells the server nothing. 1252 tests, typecheck clean. Co-Authored-By: Claude Opus 5 (1M context) --- src/protect/engine/pulse-client.js | 15 +++++- src/protect/rules/source.js | 2 +- src/protect/runtime.js | 1 + tests/protect/detections.test.ts | 82 ++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 2 deletions(-) diff --git a/src/protect/engine/pulse-client.js b/src/protect/engine/pulse-client.js index 7459f52..e967825 100644 --- a/src/protect/engine/pulse-client.js +++ b/src/protect/engine/pulse-client.js @@ -27,7 +27,9 @@ export class PulseRuleClient { #etag; #pulseAuth; - constructor({ siteUuid, baseUrl, cacheTtl, etag, timeoutMs, pulseAuth } = {}) { + #reportsDetections; + + constructor({ siteUuid, baseUrl, cacheTtl, etag, timeoutMs, pulseAuth, reportsDetections } = {}) { // Bounded so app STARTUP can't hang on a slow API: hosted platforms fail a deploy whose health // check is slow, and we always have a cache/bundled fallback to boot from. this.#timeoutMs = Number(timeoutMs) > 0 ? Number(timeoutMs) : 30_000; @@ -37,6 +39,16 @@ export class PulseRuleClient { this.#cacheTtl = Number.isFinite(cacheTtl) && cacheTtl > 0 ? cacheTtl : DEFAULT_CACHE_TTL; this.#etag = etag ?? null; this.#pulseAuth = pulseAuth ?? null; + // Whether this guard reports detections, declared on a request it already makes. + // + // Detections are only sent when a rule fires, so silence at the server means one of three things — + // nothing matched, reporting is off, or reports are not arriving — and nothing distinguishes them. + // Saying "reporting is on" on the rules fetch does, without a new outbound path or any request data: + // the fetch is already periodic, already authenticated, and already carries this site's identity. + // + // A capability, not a timestamp: the server records when IT saw this, because a client clock is a + // value from outside and "alive as of" is exactly the claim a stale or wrong clock would fake. + this.#reportsDetections = reportsDetections === true; if (!this.#siteUuid) { throw new Error('Patchstack site UUID is required. Pass { siteUuid } or set PATCHSTACK_SITE_UUID.'); } @@ -59,6 +71,7 @@ export class PulseRuleClient { fetch, )), }; + if (this.#reportsDetections) headers['X-Patchstack-Detections'] = 'enabled'; if (this.#etag) headers['If-None-Match'] = this.#etag; const response = await fetch(url, { method: 'GET', headers, signal: AbortSignal.timeout(this.#timeoutMs) }); diff --git a/src/protect/rules/source.js b/src/protect/rules/source.js index f352207..ec33c67 100644 --- a/src/protect/rules/source.js +++ b/src/protect/rules/source.js @@ -42,7 +42,7 @@ export async function resolveRules(options, store, ctx = {}) { const timeoutMs = ctx.timeoutMs; if (options.siteUuid) { const prior = await store.read(); // { bundle, etag } | null - const client = new PulseRuleClient({ siteUuid: options.siteUuid, baseUrl: options.pulseRulesUrl, etag: prior?.etag, timeoutMs, pulseAuth: ctx.pulseAuth }); + const client = new PulseRuleClient({ siteUuid: options.siteUuid, baseUrl: options.pulseRulesUrl, etag: prior?.etag, timeoutMs, pulseAuth: ctx.pulseAuth, reportsDetections: options.reportDetections === true }); const res = await client.getRules(); if (res.success && res.notModified && prior?.bundle) return normalizeBundle(prior.bundle, options); if (res.success && !res.notModified) { diff --git a/src/protect/runtime.js b/src/protect/runtime.js index b1e062c..fdd72cf 100644 --- a/src/protect/runtime.js +++ b/src/protect/runtime.js @@ -28,6 +28,7 @@ import { renderBlockPage } from './block-page.js'; import { makeStore } from './rules/store.js'; import { resolveRules } from './rules/source.js'; import { startRefresh, makeRefreshHandler } from './rules/refresh.js'; +import { createDetectionReporter } from './detections.js'; import { createFirewallLogReporter, resolveApiBase, telemetryEnabled } from './firewall-log.js'; // Supabase-tunnel guard for AI-builder apps (Lovable / TanStack Start + Supabase). diff --git a/tests/protect/detections.test.ts b/tests/protect/detections.test.ts index 53cfb61..7b2faf6 100644 --- a/tests/protect/detections.test.ts +++ b/tests/protect/detections.test.ts @@ -212,3 +212,85 @@ describe('wiring', () => { p.stopRefresh?.(); }); }); + +describe('declaring the capability', () => { + it('tells the server reporting is on, on a request it already makes', async () => { + // Detections are sent only when a rule fires, so silence at the server means nothing matched, or + // reporting is off, or reports are not arriving — and nothing tells those apart. The rules fetch does, + // with a header: no new outbound path, no request data, and no client timestamp (the server records + // when IT saw this, because "alive as of" is the claim a wrong clock would fake). + const seen: Array> = []; + const fetchMock = vi.fn(async (_url: string, init?: RequestInit) => { + seen.push((init?.headers ?? {}) as Record); + + return new Response(JSON.stringify({ firewall: [], whitelists: [], enforcement: 'dry-run' }), { + status: 200, headers: { 'Content-Type': 'application/json' }, + }); + }); + vi.stubGlobal('fetch', fetchMock); + + const p: any = await createProtection({ + siteUuid: 'site-1', + pulseRulesUrl: 'https://x.test/monitor/pulse', + reportDetections: true, + }); + + expect(seen.some((h) => h['X-Patchstack-Detections'] === 'enabled')).toBe(true); + p.stopRefresh?.(); + }); + + it('says nothing when reporting is off', async () => { + // The declaration has to mean something: a guard that is not reporting must not claim it is, or the + // server cannot tell a configured site from an unconfigured one — which is the whole point. + const seen: Array> = []; + const fetchMock = vi.fn(async (_url: string, init?: RequestInit) => { + seen.push((init?.headers ?? {}) as Record); + + return new Response(JSON.stringify({ firewall: [], whitelists: [], enforcement: 'dry-run' }), { + status: 200, headers: { 'Content-Type': 'application/json' }, + }); + }); + vi.stubGlobal('fetch', fetchMock); + + const p: any = await createProtection({ siteUuid: 'site-1', pulseRulesUrl: 'https://x.test/monitor/pulse' }); + + expect(seen.every((h) => h['X-Patchstack-Detections'] === undefined)).toBe(true); + p.stopRefresh?.(); + }); +}); + +describe('the wiring actually runs', () => { + it('posts a detection when reporting is switched on', async () => { + // The gap that let a broken build merge: every other test here either exercised the reporter directly + // or asserted that NOTHING is posted when the feature is off. Neither enters the branch that builds the + // reporter, so an unresolved import in it threw only for someone who turned the feature on — which, + // being opt-in, was nobody. This test is the one that fails if the wiring is broken. + const posted: string[] = []; + const fetchMock = vi.fn(async (url: string) => { + posted.push(String(url)); + if (String(url).includes('/detections/')) return new Response('{}', { status: 202 }); + + return new Response( + JSON.stringify({ + firewall: [{ id: 'r1', title: 'boom', rule_v2: [{ parameter: 'get.q', match: { type: 'contains', value: 'boom' } }] }], + whitelists: [], enforcement: 'dry-run', + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ); + }); + vi.stubGlobal('fetch', fetchMock); + + const p: any = await createProtection({ + siteUuid: 'site-1', + pulseRulesUrl: 'https://x.test/monitor/pulse', + reportDetections: true, + detectionFlushMs: 1, + }); + + await p.fetchGuard()(new Request('https://app.test/api/x?q=boom')); + p.stopRefresh?.(); + await new Promise((resolve) => setTimeout(resolve, 5)); + + expect(posted.some((url) => url.includes('/detections/site-1'))).toBe(true); + }); +}); From 2c6a014e22527ac75edd18a3f49e281ce9fd1263 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Thu, 20 Aug 2026 14:44:54 +0200 Subject: [PATCH 2/3] Claim the capability only on an authenticated request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header went out whether or not a bearer token resolved. The rules endpoint still accepts a bare UUID while token auth is optional, so on that path this was an assertion anyone holding the UUID could make — and it asserts the reassuring thing: that reporting is on. A dashboard would then show a site as covered because a stranger said so, which is the worst direction for a wrong answer to point. Fetching rules must never hinge on getting a token — protection comes first, and that is unchanged. Claiming a capability may: an unauthenticated request is one whose statements about a site carry no weight, and there is nothing to lose by withholding a claim. The server will gate on a verified token as well. A client-side check only removes the ACCIDENTAL case — a guard on the legacy path telling the truth about itself — because the forgeable case is not the client's to prevent. Both halves are needed and neither is sufficient. Also fixed a test that was passing for the wrong reason: the positive case supplied no credential, so it had been asserting the header on an unauthenticated request. It now exchanges a real one and checks that the claim travels with `Bearer`, which is what the gate is about. Mutation-checked: claiming it regardless of the credential fails the unauthenticated test and nothing else. 1253 tests, typecheck clean. Co-Authored-By: Claude Opus 5 (1M context) --- src/protect/engine/pulse-client.js | 25 ++++++++++----- tests/protect/detections.test.ts | 50 ++++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 10 deletions(-) diff --git a/src/protect/engine/pulse-client.js b/src/protect/engine/pulse-client.js index e967825..137a592 100644 --- a/src/protect/engine/pulse-client.js +++ b/src/protect/engine/pulse-client.js @@ -64,14 +64,23 @@ export class PulseRuleClient { // Unauthenticated when no credential resolved, or when the exchange // fails — the server still accepts the UUID, and protection must never // hinge on getting a token. - const headers = { - Accept: 'application/json', - ...(await pulseAuthHeader( - { pulseAuth: this.#pulseAuth, endpoint: this.#baseUrl, timeoutMs: this.#timeoutMs }, - fetch, - )), - }; - if (this.#reportsDetections) headers['X-Patchstack-Detections'] = 'enabled'; + const auth = await pulseAuthHeader( + { pulseAuth: this.#pulseAuth, endpoint: this.#baseUrl, timeoutMs: this.#timeoutMs }, + fetch, + ); + const headers = { Accept: 'application/json', ...auth }; + // Claimed only on an authenticated request. The rules endpoint still accepts a bare UUID, so on that + // path this header would be an assertion anyone holding the UUID could make — and it asserts the + // reassuring thing: that reporting is on. A dashboard would then say a site is covered because a + // stranger said so. + // + // Fetching rules must never hinge on getting a token (protection comes first), but CLAIMING a + // capability may: an unauthenticated request is one whose statements about this site carry no weight. + // The server gates on a verified token as well — a client-side check only removes the accidental + // case, since the forgeable one is not the client's to prevent. + if (this.#reportsDetections && typeof auth.Authorization === 'string') { + headers['X-Patchstack-Detections'] = 'enabled'; + } if (this.#etag) headers['If-None-Match'] = this.#etag; const response = await fetch(url, { method: 'GET', headers, signal: AbortSignal.timeout(this.#timeoutMs) }); diff --git a/tests/protect/detections.test.ts b/tests/protect/detections.test.ts index 7b2faf6..91f5f41 100644 --- a/tests/protect/detections.test.ts +++ b/tests/protect/detections.test.ts @@ -220,7 +220,12 @@ describe('declaring the capability', () => { // with a header: no new outbound path, no request data, and no client timestamp (the server records // when IT saw this, because "alive as of" is the claim a wrong clock would fake). const seen: Array> = []; - const fetchMock = vi.fn(async (_url: string, init?: RequestInit) => { + const fetchMock = vi.fn(async (url: string, init?: RequestInit) => { + if (String(url).includes('token')) { + return new Response(JSON.stringify({ access_token: 'jwt-abc', expires_in: 3600 }), { + status: 200, headers: { 'Content-Type': 'application/json' }, + }); + } seen.push((init?.headers ?? {}) as Record); return new Response(JSON.stringify({ firewall: [], whitelists: [], enforcement: 'dry-run' }), { @@ -232,10 +237,16 @@ describe('declaring the capability', () => { const p: any = await createProtection({ siteUuid: 'site-1', pulseRulesUrl: 'https://x.test/monitor/pulse', + pulseAuth: 'the-secret-40-chars-long-ish-value-here-987', reportDetections: true, }); - expect(seen.some((h) => h['X-Patchstack-Detections'] === 'enabled')).toBe(true); + // Authenticated, so the claim carries weight and is made. + const claimed = seen.filter((h) => h['X-Patchstack-Detections'] === 'enabled'); + expect(claimed.length).toBeGreaterThan(0); + for (const headers of claimed) { + expect(headers.Authorization, 'the claim only travels on an authenticated request').toContain('Bearer'); + } p.stopRefresh?.(); }); @@ -294,3 +305,38 @@ describe('the wiring actually runs', () => { expect(posted.some((url) => url.includes('/detections/site-1'))).toBe(true); }); }); + +describe('the capability claim is only made when it carries weight', () => { + it('stays silent on an unauthenticated rules fetch', async () => { + // The rules endpoint still accepts a bare UUID, so on that path this header is an assertion anyone + // holding the UUID could make — and it asserts the reassuring thing, that reporting is on. A dashboard + // would then report a site as covered because a stranger said so. Fetching rules must not hinge on a + // token; claiming a capability must. + const seen: Array> = []; + const fetchMock = vi.fn(async (_url: string, init?: RequestInit) => { + seen.push((init?.headers ?? {}) as Record); + + return new Response(JSON.stringify({ firewall: [], whitelists: [], enforcement: 'dry-run' }), { + status: 200, headers: { 'Content-Type': 'application/json' }, + }); + }); + vi.stubGlobal('fetch', fetchMock); + + // No credential anywhere: no `pulseAuth`, and nothing for the token exchange to find. + const p: any = await createProtection({ + siteUuid: 'site-1', + pulseRulesUrl: 'https://x.test/monitor/pulse', + reportDetections: true, + }); + + const rulesRequests = seen.filter((h) => h.Accept === 'application/json'); + expect(rulesRequests.length).toBeGreaterThan(0); + for (const headers of rulesRequests) { + expect(headers.Authorization).toBeUndefined(); + expect(headers['X-Patchstack-Detections'], 'an unauthenticated request may not claim the capability') + .toBeUndefined(); + } + + p.stopRefresh?.(); + }); +}); From fdf8fcf13e8fc0bcd5f2d9ac3e4548ed79a76a6a Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Thu, 20 Aug 2026 15:13:13 +0200 Subject: [PATCH 3/3] Describe the capability header's trust boundary as a requirement, not a guarantee The comment asserted that the server verifies a token before accepting this header. Server-side verification of the rules fetch is configurable, and nothing consumes the header yet, so the sentence claimed a present guarantee where there is a requirement on whatever eventually reads it. State the requirement instead: a client-side gate removes the accidental case only, and a consumer has to verify the token itself before believing the claim. Co-Authored-By: Claude Opus 5 (1M context) --- src/protect/engine/pulse-client.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/protect/engine/pulse-client.js b/src/protect/engine/pulse-client.js index 137a592..dc676ec 100644 --- a/src/protect/engine/pulse-client.js +++ b/src/protect/engine/pulse-client.js @@ -76,8 +76,9 @@ export class PulseRuleClient { // // Fetching rules must never hinge on getting a token (protection comes first), but CLAIMING a // capability may: an unauthenticated request is one whose statements about this site carry no weight. - // The server gates on a verified token as well — a client-side check only removes the accidental - // case, since the forgeable one is not the client's to prevent. + // This check only removes the ACCIDENTAL case. The forgeable one is not the client's to prevent, so + // anything acting on this header has to require a verified token itself before believing it — a + // client-side gate is a courtesy, never the guarantee. if (this.#reportsDetections && typeof auth.Authorization === 'string') { headers['X-Patchstack-Detections'] = 'enabled'; }