diff --git a/src/protect/egress.js b/src/protect/egress.js index 7986f27..9ba60a8 100644 --- a/src/protect/egress.js +++ b/src/protect/egress.js @@ -12,6 +12,8 @@ * lookup?: Function }} opts * @returns {Promise<() => void>} uninstall (restores every patched surface) */ +import { notify } from './notify.js'; + export async function installEgressGuard({ shouldBlock, onBlock, onSkip, dnsScreen = true, lookup, allowHosts } = {}) { const restores = []; if (typeof shouldBlock !== 'function') return () => {}; @@ -19,7 +21,10 @@ export async function installEgressGuard({ shouldBlock, onBlock, onSkip, dnsScre const block = (url, host, method) => { if (!shouldBlock(url, host, method)) return false; - onBlock?.({ url, host, method }); + // Reported AFTER the decision and contained, because this call sits between deciding to block and + // saying so. An escaping throw would replace a controlled block with the callback's exception, which + // hands the enforcement outcome to reporting code — the inverse of what a block is for. + notify(onBlock, { url, host, method }, 'onEgressBlock'); return true; }; @@ -44,7 +49,7 @@ export async function installEgressGuard({ shouldBlock, onBlock, onSkip, dnsScre // A resolver failure means the destination was NOT screened by IP — the hostname check alone let it // through. That's a real (if rare) coverage hole, so report it via onSkip instead of failing open // silently. Still fail-open: a broken resolver must not take the app's outbound traffic down. - const skip = (reason, detail) => { try { onSkip?.({ phase: 'egress', reason, detail }); } catch { /* never affect traffic */ } }; + const skip = (reason, detail) => notify(onSkip, { phase: 'egress', reason, detail }, 'onSkip'); // True when a hostname resolves to a disallowed address. Fail-open: any resolver error → false. const resolvesToDisallowed = (url, host, method) => diff --git a/src/protect/engine/engine.js b/src/protect/engine/engine.js index f8f1b86..fca619e 100644 --- a/src/protect/engine/engine.js +++ b/src/protect/engine/engine.js @@ -1,4 +1,5 @@ import { RequestResolver } from './request.js'; +import { notify } from '../notify.js'; import { normalizeRequest } from './normalizer.js'; // Catastrophic-backtracking shapes. Broad on purpose: a group whose inner content is quantified @@ -640,8 +641,9 @@ export class RuleEngine { // evaluating a rule is reported and the request is allowed through (fail open). A // malformed rule is skipped without aborting the rest of the ruleset. #reportError(err) { - if (this.#onError) { - this.#onError(err); + // A host handler that runs takes over reporting. One that THROWS does not: fall through to the + // built-in logging below, or a rule error would disappear into a broken reporter. + if (notify(this.#onError, err, 'onError')) { return; } // Default: log once per distinct message so a persistently-broken rule doesn't diff --git a/src/protect/engine/fetch.js b/src/protect/engine/fetch.js index 2f84054..1191888 100644 --- a/src/protect/engine/fetch.js +++ b/src/protect/engine/fetch.js @@ -7,6 +7,7 @@ // shape from a `Request`, so no engine changes are needed beyond keeping the hot path // free of Node-only APIs. import { RuleEngine } from './engine.js'; +import { notify } from '../notify.js'; // Cap how much request body we buffer for inspection. A larger body is left UNSCANNED // (fail-open) rather than buffered into memory — matches the node adapter's maxBodyBytes. @@ -243,20 +244,19 @@ export function createFetchMiddleware(rulesData, options = {}) { req = await fromFetchRequest(request); // shaping inside the try — a bad/relative request.url must fail open result = engine.evaluate(req); } catch (err) { - if (options.onError) { - options.onError(err); - } + notify(options.onError, err, 'onError'); return null; // fail open } if (result.blocked) { - if (options.onBlock) { - options.onBlock({ - rule: result.rule, - message: result.message, - request: { method: req.method, url: req.url, ip: req.ip } - }); - } + // Contained: this runs after the block decision and before the block response is built, so an + // escaping throw would replace the 403 with the callback's exception — reporting code deciding + // the enforcement outcome. + notify(options.onBlock, { + rule: result.rule, + message: result.message, + request: { method: req.method, url: req.url, ip: req.ip } + }, 'onBlock'); return (options.response || defaultBlockResponse)(result); } diff --git a/src/protect/engine/middleware.js b/src/protect/engine/middleware.js index 343a146..ab19a4a 100644 --- a/src/protect/engine/middleware.js +++ b/src/protect/engine/middleware.js @@ -1,3 +1,4 @@ +import { notify } from '../notify.js'; import { PatchstackRuleClient } from './client.js'; import { RuleEngine } from './engine.js'; @@ -8,17 +9,17 @@ export function createMiddleware(rulesData, options = {}) { const result = engine.evaluate(req); if (result.blocked) { - if (options.onBlock) { - options.onBlock({ - rule: result.rule, - message: result.message, - request: { - method: req.method, - url: req.url, - ip: req.ip ?? req.socket?.remoteAddress - } - }); - } + // Contained: a throw here would replace the 403 below with the callback's exception, which for + // Express means the error handler decides what a blocked request returns. + notify(options.onBlock, { + rule: result.rule, + message: result.message, + request: { + method: req.method, + url: req.url, + ip: req.ip ?? req.socket?.remoteAddress + } + }, 'onBlock'); return res.status(403).json({ error: 'Blocked by Patchstack WAF', @@ -110,9 +111,9 @@ export async function protect(options = {}) { return passThrough(); } - if (options.onScan) { - options.onScan(rulesData); - } + // Contained too: a throw here aborted lazy init, so the WAF never installed and every later + // request went unscreened — a reporting hook silently costing protection outright. + notify(options.onScan, rulesData, 'onScan'); const wafMiddleware = createMiddleware(rulesData, options); @@ -167,9 +168,7 @@ export function protectSync(options = {}) { initError = err; console.warn(`[patchstack] WAF lazy init failed: ${err.message}. Passing through.`); - if (options.onError) { - options.onError(err); - } + notify(options.onError, err, 'onError'); }); } diff --git a/src/protect/engine/node.js b/src/protect/engine/node.js index 790630b..20ae129 100644 --- a/src/protect/engine/node.js +++ b/src/protect/engine/node.js @@ -8,6 +8,7 @@ // any body-parser — it consumes the stream and exposes the parsed body as `req.body`. import { RuleEngine } from './engine.js'; import { parseBody } from './fetch.js'; +import { notify } from '../notify.js'; // Build the engine's request shape from a Node IncomingMessage + its raw body text. export function fromNodeRequest(req, rawBody = '') { @@ -133,20 +134,18 @@ export function createNodeMiddleware(rulesData, options = {}) { shaped = fromNodeRequest(req, rawBody); // shaping is inside the try too — never crash result = engine.evaluate(shaped); } catch (err) { - if (options.onError) { - options.onError(err); - } + notify(options.onError, err, 'onError'); return next(); // fail open } if (result.blocked) { - if (options.onBlock) { - options.onBlock({ - rule: result.rule, - message: result.message, - request: { method: shaped.method, url: shaped.url, ip: shaped.ip } - }); - } + // Contained, as on the fetch path: a throw here would replace the block response with the + // callback's exception. + notify(options.onBlock, { + rule: result.rule, + message: result.message, + request: { method: shaped.method, url: shaped.url, ip: shaped.ip } + }, 'onBlock'); return (options.response || defaultBlock)(res, result); } diff --git a/src/protect/notify.js b/src/protect/notify.js new file mode 100644 index 0000000..ec3b717 --- /dev/null +++ b/src/protect/notify.js @@ -0,0 +1,82 @@ +/** + * Deliver a value to a caller-supplied callback without letting it break anything. + * + * `onError`, `onDetect` and `onSkip` are hooks a host passes in, so their code is not ours and its + * failure is not ours to inherit. This package's one promise is that it never takes down the app it + * protects — an engine error fails open, a malformed rule is skipped, a slow API boots from cache. A + * reporting hook that throws has to fail open for the same reason, and for a sharper one: it fires + * exactly when something interesting happened, so an unguarded throw converts "we noticed something" + * into "the request died", and does it only on the requests that mattered. + * + * `onSkip` was already wrapped this way, with the reason written next to it. This is that same rule, + * applied to the hooks that were missed rather than restated for one of them. + * + * Not silent, though. A hook that throws is a bug in the host's code and swallowing it entirely would + * hide it forever, so the first failure per hook is reported — once, because these run per request and a + * persistently broken hook would otherwise print on every one. Same reasoning as the engine's + * report-once for a persistently broken rule. + * + * @param {unknown} fn the callback, or anything that is not a function (then this is a no-op) + * @param {unknown} arg the single argument to hand it + * @param {string} label which hook, for the one-time warning + * @returns {boolean} whether the callback ran to completion, so a caller can fall back to its own + * reporting rather than losing the report entirely. For an ASYNC callback this can only mean it + * started: a rejection arrives after we return, and is contained and warned about, but by then a + * caller has already decided not to fall back. Synchronous handlers get the stronger answer. + */ + +/** Hooks already reported as broken. Module-scoped: one warning per hook per process, not per guard. */ +const reported = new Set(); + +export function notify(fn, arg, label) { + if (typeof fn !== 'function') return false; + + try { + const result = fn(arg); + + // An ASYNC callback fails after this function has already returned. `async () => { throw ... }` does + // not throw — it hands back a rejected promise, and an unhandled rejection terminates the process by + // default on Node. So a try/catch alone would contain the synchronous hosts and leave the async ones + // able to kill the app, which is a worse outcome than the throw we set out to contain. + if (result !== null && typeof result === 'object' && typeof result.then === 'function') { + try { + result.then(undefined, (err) => warnOnce(label, err)); + } catch { + // A `then` that throws on access. Nothing more to attach to; the value is not a usable promise. + } + } + + return true; + } catch (err) { + warnOnce(label, err); + + return false; + } +} + +/** + * Report a broken callback once per process. + * + * Must not throw: it runs inside the containment, so its own failure would be the thing that breaks the + * guarantee it exists to report on. + */ +function warnOnce(label, err) { + if (reported.has(label)) return; + reported.add(label); + try { + // Named as the host's callback, not as a Patchstack failure: pointing at ourselves for someone + // else's throw sends them reading the wrong code. + console.warn( + `Patchstack: the ${label} callback passed to createProtection failed and was ignored. ` + + `Protection is unaffected; this is reported once per process. ` + + `Cause: ${err && err.message ? err.message : String(err)}`, + ); + } catch { + /* no console on this runtime */ + } +} + +/** Test seam: forget which hooks have been reported, so warn-once is assertable more than once. */ +export function resetNotifyWarnings() { + reported.clear(); +} diff --git a/src/protect/rules/refresh.js b/src/protect/rules/refresh.js index b7f56b9..d784a3a 100644 --- a/src/protect/rules/refresh.js +++ b/src/protect/rules/refresh.js @@ -7,6 +7,8 @@ // - makeRefreshHandler: a PUSH endpoint — an authenticated fetch handler the platform/SaaS hits // for an immediate refresh (zero-day fast lane) instead of waiting for the next poll. +import { notify } from '../notify.js'; + const JITTER_FRACTION = 0.1; const MAX_BACKOFF_MULTIPLIER = 8; // cap consecutive-failure backoff at 8× the base interval @@ -31,7 +33,7 @@ export function startRefresh(tick, { refreshMs, onError } = {}) { failures = 0; } catch (err) { failures++; - onError?.(err); + notify(onError, err, 'onError'); } schedule(); }; diff --git a/src/protect/rules/source.js b/src/protect/rules/source.js index ec33c67..7d69980 100644 --- a/src/protect/rules/source.js +++ b/src/protect/rules/source.js @@ -5,6 +5,7 @@ import { PatchstackRuleClient } from '../engine/index.js'; import { PulseRuleClient } from '../engine/pulse-client.js'; import { validateBundle } from './validate.js'; +import { notify } from '../notify.js'; // A LIVE update is accepted ATOMICALLY. Dropping individual invalid rules is fine for a bundle we // already trust (a cache entry, a bundled fallback), but for a fresh remote response it would let a @@ -25,14 +26,14 @@ function reportRejections(rejected, options, label) { const report = options.onRuleRejected; for (const r of rejected) { if (typeof report === 'function') { - try { report({ ...r, accepted: false }); } catch { /* reporting must never break rule loading */ } + notify(report, { ...r, accepted: false }, 'onRuleRejected'); } } const sample = rejected.slice(0, 3).map((r) => `${r.id} (${r.reason})`).join('; '); - options.onError?.(new Error( + notify(options.onError, new Error( `${label}: rejected the entire update because ${rejected.length} rule(s) failed validation — ` + `keeping the previous ruleset and NOT caching this response: ${sample}${rejected.length > 3 ? ', …' : ''}`, - )); + ), 'onError'); } export async function resolveRules(options, store, ctx = {}) { @@ -58,14 +59,14 @@ export async function resolveRules(options, store, ctx = {}) { return bundle; } if (prior?.bundle) { - options.onError?.(new Error(`pulse rule fetch failed (${res.error ?? 'no usable response'}); using cached bundle`)); + notify(options.onError, new Error(`pulse rule fetch failed (${res.error ?? 'no usable response'}); using cached bundle`), 'onError'); return normalizeBundle(prior.bundle, options); } if (options.rules) { - options.onError?.(new Error(`pulse rule fetch failed (${res.error ?? 'no usable response'}); using bundled fallback`)); + notify(options.onError, new Error(`pulse rule fetch failed (${res.error ?? 'no usable response'}); using bundled fallback`), 'onError'); return normalizeBundle(options.rules, options); } - options.onError?.(new Error(`pulse rule fetch failed (${res.error ?? 'no usable response'}); no cache — running with no rules`)); + notify(options.onError, new Error(`pulse rule fetch failed (${res.error ?? 'no usable response'}); no cache — running with no rules`), 'onError'); return emptyBundle(); } @@ -87,10 +88,10 @@ export async function resolveRules(options, store, ctx = {}) { return bundle; } if (prior?.bundle) { - options.onError?.(new Error(`rule fetch failed (${res.error ?? 'no usable response'}); using cached bundle`)); + notify(options.onError, new Error(`rule fetch failed (${res.error ?? 'no usable response'}); using cached bundle`), 'onError'); return normalizeBundle(prior.bundle, options); } - options.onError?.(new Error(`rule fetch failed (${res.error ?? 'no usable response'}); no cache — running with no rules`)); + notify(options.onError, new Error(`rule fetch failed (${res.error ?? 'no usable response'}); no cache — running with no rules`), 'onError'); return emptyBundle(); } @@ -118,7 +119,7 @@ export function normalizeBundle(b, options = {}) { const report = options.onRuleRejected; if (typeof report === 'function') { for (const r of rejected) { - try { report(r); } catch { /* reporting must never break rule loading */ } + notify(report, r, 'onRuleRejected'); } } else { const sample = rejected.slice(0, 3).map((r) => `${r.id} (${r.reason})`).join('; '); diff --git a/src/protect/runtime.js b/src/protect/runtime.js index a35143e..3a4f236 100644 --- a/src/protect/runtime.js +++ b/src/protect/runtime.js @@ -29,6 +29,7 @@ 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 { notify } from './notify.js'; import { createFirewallLogReporter, resolveApiBase, telemetryEnabled } from './firewall-log.js'; // Supabase-tunnel guard for AI-builder apps (Lovable / TanStack Start + Supabase). @@ -94,7 +95,7 @@ export async function createProtection(options = {}) { let detections = null; const onDetect = (detection) => { - userOnDetect(detection); + notify(userOnDetect, detection, 'onDetect'); if (detections) detections.record(detection); if (firewallLog && detection?.mode === 'block') { firewallLog.record({ @@ -136,7 +137,7 @@ export async function createProtection(options = {}) { 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)); + notify(onError, new Error(message), 'onError'); console.warn(message); } const bundle = await resolveRules(options, store, { timeoutMs: bootTimeoutMs, pulseAuth }); @@ -220,13 +221,9 @@ export async function createProtection(options = {}) { const recordSkip = (phase, reason, detail) => { const key = `${phase}:${reason}`; skipCounts[key] = (skipCounts[key] ?? 0) + 1; - if (onSkip) { - try { - onSkip({ phase, reason, detail, count: skipCounts[key] }); - } catch { - /* a reporting callback must never affect request handling */ - } - } + // A reporting callback must never affect request handling — including an async one, whose rejection + // lands after a try/catch here would have returned. + notify(onSkip, { phase, reason, detail, count: skipCounts[key] }, 'onSkip'); }; const maskFn = @@ -285,7 +282,7 @@ export async function createProtection(options = {}) { // the response phase used to build (which made `when` on a response rule inert). result = re.evaluate({ ...(reqCtx || {}), _response: { ...meta, body: text } }); } catch (err) { - onError?.(err); + notify(onError, err, 'onError'); continue; } if (!result.blocked) continue; @@ -426,7 +423,7 @@ export async function createProtection(options = {}) { try { r = screenText(text, { status: res.statusCode, headers: res.getHeaders ? res.getHeaders() : {} }, reqCtx); } catch (err) { - onError?.(err); + notify(onError, err, 'onError'); for (const c of chunks) origWrite(c); return origEnd(cb); } @@ -467,7 +464,7 @@ export async function createProtection(options = {}) { try { result = egressEngine.evaluate({ _egress: { url, host, method } }); } catch (err) { - onError?.(err); + notify(onError, err, 'onError'); return false; } if (!result.blocked) return false; @@ -507,7 +504,7 @@ export async function createProtection(options = {}) { try { result = engine.evaluate(await fromFetchRequest(request)); } catch (err) { - onError?.(err); + notify(onError, err, 'onError'); return null; // fail open } return decide('request', result, () => blockResponse(result, request), () => null, fetchRequestMeta(request)); @@ -533,7 +530,7 @@ export async function createProtection(options = {}) { try { result = engine.evaluate(req); } catch (err) { - onError?.(err); + notify(onError, err, 'onError'); if (exprOptions.screenResponses) wrapNodeResponse(res, reqContextFromNode(req)); return next(); } @@ -573,7 +570,7 @@ export async function createProtection(options = {}) { chunks.push(chunk); }); req.on('error', (err) => { - onError?.(err); + notify(onError, err, 'onError'); next(); }); req.on('end', () => { @@ -585,7 +582,7 @@ export async function createProtection(options = {}) { shaped = fromNodeRequest(req, rawBody); result = engine.evaluate(shaped); } catch (err) { - onError?.(err); + notify(onError, err, 'onError'); return next(); } decide( @@ -648,7 +645,7 @@ export async function createProtection(options = {}) { try { ({ reportManifest: reporter } = await import('./refresh-manifest.js')); } catch (err) { - onError?.(err); // scan pipeline unavailable (e.g. an edge runtime) — rules still refresh + notify(onError, err, 'onError'); // scan pipeline unavailable (e.g. an edge runtime) — rules still refresh } } @@ -657,7 +654,7 @@ export async function createProtection(options = {}) { try { await reporter(cwd); } catch (err) { - onError?.(err); // a failed report must not stop the rule refresh + notify(onError, err, 'onError'); // a failed report must not stop the rule refresh } } const next = await resolveRules(options, store, { timeoutMs: options.refreshTimeoutMs, pulseAuth }); diff --git a/tests/protect/callback-containment.test.ts b/tests/protect/callback-containment.test.ts new file mode 100644 index 0000000..f4d89d4 --- /dev/null +++ b/tests/protect/callback-containment.test.ts @@ -0,0 +1,294 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createProtection } from '../../src/protect/runtime.js'; +import { notify, resetNotifyWarnings } from '../../src/protect/notify.js'; +import { createFetchMiddleware } from '../../src/protect/engine/fetch.js'; + +/** + * A callback the host passed in must never break the guard that calls it. + * + * `onError`, `onDetect` and `onSkip` run host code, and this package's whole promise is that it does not + * take down the app it protects. `onSkip` was already wrapped for exactly that reason; the other two were + * not, and the sharper case is `onDetect`, which fires only when a rule matched — so an unguarded throw + * converted "we noticed something" into "the request died", on precisely the requests that mattered. + * + * Every containment test here is paired with a control, because "contained" has a cheap wrong + * implementation: never call the callback at all. + */ +const RULES = { + firewall: [ + { id: 'r1', title: 'marker in the query', rule_v2: [{ parameter: 'get.q', match: { type: 'contains', value: 'boom' } }] }, + ], +}; + +const app = async () => new Response('ok', { status: 200 }); +const hit = () => new Request('https://x.test/?q=boom'); +const miss = () => new Request('https://x.test/?q=fine'); +const boom = () => { + throw new Error('host callback is broken'); +}; +/** The shape a try/catch cannot contain: it returns a rejected promise instead of throwing. */ +const asyncBoom = async () => { + throw new Error('host callback rejected'); +}; + +/** Unhandled rejections during `fn`, which on Node terminate the process by default. */ +async function unhandledDuring(fn: () => Promise): Promise { + const seen: unknown[] = []; + const onUnhandled = (err: unknown) => seen.push(err); + process.on('unhandledRejection', onUnhandled); + try { + await fn(); + // Rejections surface a macrotask later than the code that caused them. + await new Promise((resolve) => setTimeout(resolve, 50)); + } finally { + process.off('unhandledRejection', onUnhandled); + } + + return seen; +} + +describe('a throwing host callback cannot break the guard', () => { + beforeEach(() => { + resetNotifyWarnings(); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + afterEach(() => { + delete process.env.PATCHSTACK_MODE; + vi.restoreAllMocks(); + }); + + it('serves the request when onDetect throws', async () => { + const p = await createProtection({ rules: RULES, onDetect: boom }); + + // Was: the throw propagated out of the guard and the request failed. In dry-run the request is + // allowed, so allowing it is the correct outcome — the detection is lost, the request is not. + expect((await p.fetch(app)(hit())).status).toBe(200); + // And it is not a one-shot failure that leaves the guard wedged for everything after it. + expect((await p.fetch(app)(hit())).status).toBe(200); + expect((await p.fetch(app)(miss())).status).toBe(200); + }); + + it('still BLOCKS when onDetect throws, rather than failing open past the rule', async () => { + // The containment must not become a bypass. A reporting callback is downstream of the decision, so a + // broken one costs the report — never the enforcement. Without this, "contained" could have meant + // swallowing the whole detection path and letting the request through. + process.env.PATCHSTACK_MODE = 'block'; + const p = await createProtection({ rules: RULES, onDetect: boom }); + + expect((await p.fetch(app)(hit())).status).toBe(403); + expect((await p.fetch(app)(miss())).status).toBe(200); + }); + + it('still calls a callback that works', async () => { + // The control for both tests above. + const seen: unknown[] = []; + const p = await createProtection({ rules: RULES, onDetect: (d: unknown) => seen.push(d) }); + + await p.fetch(app)(hit()); + + expect(seen).toHaveLength(1); + }); + + it('boots when onError throws', async () => { + // The rule fetch fails here (no network), which is a reported, recoverable condition — the guard falls + // back to the inline rules. Reporting that condition to a broken handler used to abort the boot, so an + // app lost its protection entirely over a bug in its own logging callback. + vi.stubGlobal('fetch', vi.fn(async () => { + throw new Error('offline'); + })); + + const p = await createProtection({ siteUuid: 'site-1', rules: RULES, onError: boom, cwd: '/nonexistent' }); + + expect(p.mode).toBeDefined(); + expect((await p.fetch(app)(hit())).status).toBe(200); + }); + + it('refreshes when onError throws', async () => { + vi.stubGlobal('fetch', vi.fn(async () => { + throw new Error('offline'); + })); + const p = await createProtection({ siteUuid: 'site-1', rules: RULES, onError: boom, cwd: '/nonexistent' }); + + // A refresh reports the same failure. It must settle rather than reject: an unhandled rejection in a + // poll loop is how a long-lived process dies hours after the mistake was made. + await expect(p.refresh()).resolves.not.toThrow(); + + p.stopRefresh?.(); + }); + + it('keeps serving when onSkip throws', async () => { + // Pre-existing behaviour, locked down: this one was already wrapped, and the fix must not have + // disturbed it while generalising the rule it demonstrated. + const p = await createProtection({ rules: RULES, onSkip: boom, maxBodyBytes: 8 }); + const big = new Request('https://x.test/', { method: 'POST', body: 'x'.repeat(64) }); + + expect((await p.fetch(app)(big)).status).toBe(200); + }); +}); + +describe('notify', () => { + beforeEach(() => { + resetNotifyWarnings(); + }); + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('reports whether the callback ran, so a caller can fall back', () => { + // The engine relies on this: a host handler that RUNS takes over reporting, one that THROWS must not, + // or a rule error would vanish between two broken reporters. + expect(notify(() => undefined, 'x', 'onError')).toBe(true); + expect(notify(boom, 'x', 'onError')).toBe(false); + expect(notify(undefined, 'x', 'onError')).toBe(false); + expect(notify('not a function', 'x', 'onError')).toBe(false); + }); + + it('warns once per callback, not once per call', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + for (let i = 0; i < 5; i++) notify(boom, 'x', 'onDetect'); + + // These run per request. A warning per failure would turn one bug into an unbounded log flood, and + // silence would hide it forever — so exactly one, naming the callback. + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls.flat().join(' ')).toContain('onDetect'); + }); + + it('survives a runtime with no console', () => { + // The warning is a diagnostic; it does not get to be the thing that breaks containment. + const original = globalThis.console; + try { + // @ts-expect-error deliberately removing console for this case + globalThis.console = undefined; + expect(() => notify(boom, 'x', 'onError')).not.toThrow(); + } finally { + globalThis.console = original; + } + }); +}); + + +describe('an async callback is contained too', () => { + beforeEach(() => { + resetNotifyWarnings(); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => { + delete process.env.PATCHSTACK_MODE; + vi.restoreAllMocks(); + }); + + it('produces no unhandled rejection when onDetect is async and rejects', async () => { + // `async () => { throw }` does not throw — it returns a rejected promise, so a try/catch around the + // call sees nothing and the rejection lands after containment has already returned. On Node an + // unhandled rejection terminates the process by default, which trades the throw we contained for + // something worse: the app dying, on the requests where a rule matched. + const p = await createProtection({ rules: RULES, onDetect: asyncBoom }); + + const unhandled = await unhandledDuring(async () => { + expect((await p.fetch(app)(hit())).status).toBe(200); + }); + + expect(unhandled).toEqual([]); + }); + + it('still enforces when an async onDetect rejects', async () => { + process.env.PATCHSTACK_MODE = 'block'; + const p = await createProtection({ rules: RULES, onDetect: asyncBoom }); + + const unhandled = await unhandledDuring(async () => { + expect((await p.fetch(app)(hit())).status).toBe(403); + }); + + expect(unhandled).toEqual([]); + }); + + it('boots with no unhandled rejection when onError is async and rejects', async () => { + vi.stubGlobal('fetch', vi.fn(async () => { + throw new Error('offline'); + })); + + const unhandled = await unhandledDuring(async () => { + const p = await createProtection({ siteUuid: 'site-1', rules: RULES, onError: asyncBoom, cwd: '/nonexistent' }); + expect(p.mode).toBeDefined(); + }); + + expect(unhandled).toEqual([]); + }); + + it('awaits nothing and reports the failure once', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const unhandled = await unhandledDuring(async () => { + for (let i = 0; i < 4; i++) notify(asyncBoom, 'x', 'onDetect'); + }); + + expect(unhandled).toEqual([]); + expect(warn).toHaveBeenCalledTimes(1); + }); + + it('leaves a resolving async callback alone', async () => { + // The control: containment must not have become "never call it", and a callback that works must not + // be reported as broken. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const seen: unknown[] = []; + + const unhandled = await unhandledDuring(async () => { + const p = await createProtection({ + rules: RULES, + onDetect: async (d: unknown) => { + seen.push(d); + }, + }); + await p.fetch(app)(hit()); + }); + + expect(seen).toHaveLength(1); + expect(unhandled).toEqual([]); + expect(warn).not.toHaveBeenCalled(); + }); +}); + +describe('a throwing onBlock cannot decide the enforcement outcome', () => { + beforeEach(() => { + resetNotifyWarnings(); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('still returns the block response on the fetch adapter', async () => { + // This callback runs after the decision to block and before the 403 is built. An escaping throw did + // not let the request through — it replaced the block with the callback's exception, so untrusted + // reporting code chose what a blocked request returned. Availability aside, that is enforcement + // integrity: the outcome of a block must come from the rule, never from the reporting hook. + const guard = createFetchMiddleware(RULES, { onBlock: boom }); + + const res = await guard(hit()); + + expect(res).not.toBeNull(); + expect(res?.status).toBe(403); + }); + + it('still returns the block response when onBlock rejects asynchronously', async () => { + const guard = createFetchMiddleware(RULES, { onBlock: asyncBoom }); + + const unhandled = await unhandledDuring(async () => { + expect((await guard(hit()))?.status).toBe(403); + }); + + expect(unhandled).toEqual([]); + }); + + it('still calls an onBlock that works, with the rule that fired', async () => { + const seen: Array<{ rule?: { id?: string } }> = []; + const guard = createFetchMiddleware(RULES, { onBlock: (info: { rule?: { id?: string } }) => seen.push(info) }); + + await guard(hit()); + + expect(seen).toHaveLength(1); + expect(seen[0]?.rule?.id).toBe('r1'); + }); +});