From 11406caf4dbba72370d5ecea46f1e40b571cdb88 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Thu, 20 Aug 2026 15:30:34 +0200 Subject: [PATCH] Say so at boot when a site UUID has no credential behind it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The credential is read from .patchstackrc.json, which needs a filesystem and a working directory. Not every runtime this guard targets has either: on a Worker or an edge function the file is absent and only PATCHSTACK_PULSE_AUTH / PATCHSTACK_API_KEY can carry it. Nothing about that shows up in traffic. The rules fetch simply goes out unauthenticated, and if it is ever refused the guard fails open onto its cached or bundled rules and keeps screening every request. An app running the rule set it installed with looks exactly like an app running the current one — no error, no behavioural difference, nothing to notice. So report it once at boot, through onError and a warning, naming the site and the variable that fixes it. A warning rather than a throw: a missing credential costs rule freshness, and refusing to boot over it would cost protection entirely, which is strictly worse. Tested for the diagnostic AND its absence — no warning when a credential resolves, and none in bundled-rules mode where there is no per-site lookup to authenticate. Without those controls the assertion would also pass for a warning hard-wired to siteUuid, which would fire on every healthy install. Co-Authored-By: Claude Opus 5 (1M context) --- src/protect/runtime.js | 23 ++++ tests/protect/credential-visibility.test.ts | 132 ++++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 tests/protect/credential-visibility.test.ts diff --git a/src/protect/runtime.js b/src/protect/runtime.js index b1e062c..b3e981a 100644 --- a/src/protect/runtime.js +++ b/src/protect/runtime.js @@ -115,6 +115,29 @@ export async function createProtection(options = {}) { // Resolved once and threaded through ctx: reading it is a filesystem hit on // runtimes that have one, and refreshes should not repeat it. const pulseAuth = await resolvePulseAuth(options); + // A site UUID with no credential behind it, said out loud ONCE at boot. + // + // Resolution reads `.patchstackrc.json`, so it needs a filesystem and a working directory. The + // runtimes this guard is built for do not all have one: on a Worker or an edge function the file is + // absent and only `PATCHSTACK_PULSE_AUTH` / `PATCHSTACK_API_KEY` can carry the credential. + // + // Unauthenticated rule fetches are accepted today, so the failure is currently invisible — and it + // stays invisible once they are not, because a rejected fetch fails open onto the cached or bundled + // bundle. The guard then screens every request, reports healthy, and never receives another rule. + // That silence is the whole problem: an app protected by rules frozen at install time looks exactly + // like an app protected by current ones. + // + // A warning, not a throw. Booting is protection; refusing to boot over a missing credential would + // trade a stale rule set for no rule set at all. + if (options.siteUuid && !pulseAuth) { + const message = + 'Patchstack: no API credential resolved for site ' + + options.siteUuid + + '. Rule updates may be rejected and this guard would keep running on its cached rules. ' + + 'Set PATCHSTACK_API_KEY (or pass { pulseAuth }) — required on runtimes without a filesystem.'; + onError?.(new Error(message)); + console.warn(message); + } const bundle = await resolveRules(options, store, { timeoutMs: bootTimeoutMs, pulseAuth }); // OPT-IN, deliberately. Two reasons, and the first is not about privacy: switching it on adds an // outbound POST to every guard that has a site UUID, which is a change in what an installed app does diff --git a/tests/protect/credential-visibility.test.ts b/tests/protect/credential-visibility.test.ts new file mode 100644 index 0000000..8645139 --- /dev/null +++ b/tests/protect/credential-visibility.test.ts @@ -0,0 +1,132 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtempSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { createProtection } from '../../src/protect/runtime.js'; + +/** + * A site UUID with no credential behind it must be AUDIBLE at boot. + * + * The credential is read from `.patchstackrc.json`, which needs a filesystem and a working directory — + * neither of which exists on a Worker or an edge function, where only an environment variable can carry + * it. Nothing about that failure shows up in traffic: the rules fetch is rejected, the guard falls open + * onto its cached or bundled rules, and it goes on screening every request. An app frozen at the rule + * set it installed with is indistinguishable from a current one, from the outside. + * + * So the guarantee under test is a diagnostic, and the assertions are about what the operator can see: + * the warning fires when a credential is missing, does NOT fire when one resolves, and never costs the + * app its protection either way. + */ +const NO_CREDENTIAL_ENV = ['PATCHSTACK_API_KEY', 'PATCHSTACK_PULSE_AUTH'] as const; + +/** A directory with no `.patchstackrc.json` in it, standing in for a runtime with nothing to read. */ +const emptyCwd = () => mkdtempSync(join(tmpdir(), 'ps-no-credential-')); + +/** Rules passed inline so no fetch is needed: this file is about the credential, not the transport. */ +const RULES = { + firewall: [ + { + id: 'r1', + title: 'blocks a marker in the query', + rule_v2: [{ parameter: 'get.q', match: { type: 'contains', value: 'boom' } }], + }, + ], +}; + +const app = async () => new Response('ok', { status: 200 }); + +describe('a site UUID with no credential is reported at boot', () => { + const saved: Record = {}; + + beforeEach(() => { + for (const key of NO_CREDENTIAL_ENV) { + saved[key] = process.env[key]; + delete process.env[key]; + } + // Setting a site UUID makes the boot attempt a rule fetch. Stubbed so these tests neither touch the + // network nor depend on it: the fetch fails, the guard falls back to the inline rules, and what is + // left under test is the credential diagnostic. + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + throw new Error('offline'); + }), + ); + }); + + afterEach(() => { + for (const key of NO_CREDENTIAL_ENV) { + if (saved[key] === undefined) delete process.env[key]; + else process.env[key] = saved[key]; + } + vi.restoreAllMocks(); + }); + + it('warns, and names what to set', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + await createProtection({ siteUuid: 'site-nocred', rules: RULES, cwd: emptyCwd() }); + + const said = warn.mock.calls.flat().join(' '); + expect(said, 'the site it could not authenticate').toContain('site-nocred'); + // The remedy, not just the symptom: a warning that does not say what to set leaves an operator on a + // filesystem-less runtime with no next step, which is where this failure actually happens. + expect(said, 'the variable that fixes it').toContain('PATCHSTACK_API_KEY'); + }); + + it('reports through onError too, so a host that captures logs structurally sees it', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + const errors: Error[] = []; + + await createProtection({ + siteUuid: 'site-nocred', + rules: RULES, + cwd: emptyCwd(), + onError: (err: Error) => errors.push(err), + }); + + expect(errors.map((e) => e.message).join(' ')).toContain('site-nocred'); + }); + + it('does not warn when a credential resolves', async () => { + // The control. Without it the test above passes for a warning hard-wired to `siteUuid`, which would + // fire on every correctly-configured install and train operators to ignore it. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + await createProtection({ + siteUuid: 'site-nocred', + rules: RULES, + cwd: emptyCwd(), + pulseAuth: 'a-secret-40-chars-long-enough-for-this-1', + }); + + expect(warn.mock.calls.flat().join(' ')).not.toContain('site-nocred'); + }); + + it('does not warn when there is no site UUID to authenticate for', async () => { + // Bundled-rules mode is a supported configuration, not a misconfiguration: there is no per-site + // lookup to authenticate, so there is nothing missing. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + await createProtection({ rules: RULES, cwd: emptyCwd() }); + + expect(warn.mock.calls.flat().join(' ')).not.toMatch(/credential/i); + }); + + it('still protects — the warning never becomes a refusal to boot', async () => { + // The reason this is a warning and not a throw. A missing credential costs rule FRESHNESS; refusing + // to boot over it would cost protection entirely, which is strictly worse than running on stale rules. + vi.spyOn(console, 'warn').mockImplementation(() => {}); + process.env.PATCHSTACK_MODE = 'block'; + + try { + const p = await createProtection({ siteUuid: 'site-nocred', rules: RULES, cwd: emptyCwd() }); + + expect((await p.fetch(app)(new Request('https://x.test/?q=boom'))).status).toBe(403); + expect((await p.fetch(app)(new Request('https://x.test/?q=fine'))).status).toBe(200); + } finally { + delete process.env.PATCHSTACK_MODE; + } + }); + +});