From f1549a13ebc5bcff5dd2cbd0edde0f889800c4c8 Mon Sep 17 00:00:00 2001
From: Dave Jong <dave.jong@patchstack.com>
Date: Thu, 20 Aug 2026 16:28:32 +0200
Subject: [PATCH 1/2] Stop a host's callback from breaking the guard that calls
 it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

createProtection takes three callbacks from the host: onError, onDetect and
onSkip. Only onSkip was wrapped so a throw could not escape, with the reason
written next to it — a reporting callback must never affect request handling. The
other two were unguarded, across 23 call sites.

onDetect is the sharp one, because it runs on the request path. A handler that
throws propagates out of the fetch guard and the request fails, and it fires only
when a rule matched — so a bug in the host's logging turns "we noticed
something" into "the app broke", on precisely the requests that mattered. That
is the opposite of the one promise this package makes.

onError costs boots and refreshes. Reporting a recovered condition — the rule
fetch failed, falling back to cached or bundled rules — to a broken handler
aborted createProtection, so an app lost protection entirely over a bug in its
own logging. On the refresh path the same throw becomes an unhandled rejection in
a poll loop, which kills a long-lived process long after the mistake was made.

So this is not a new rule, it is the rule onSkip already demonstrated, applied
where it was missing. One helper at every site, returning whether the callback
ran so a caller can fall back: the engine uses that to keep its own report-once
logging when a host handler throws, rather than letting a rule error disappear
between two broken reporters. Not silent either — the first failure per callback
is warned about once per process, since these run per request and the choice is
between an unbounded log flood and hiding the host's bug forever.

Every containment test is paired with a control, because containment has a cheap
wrong implementation: never call the callback at all. That mutation is caught
only by the controls. One further test says containment is not a bypass — a
broken onDetect still blocks in block mode, because a reporting callback sits
downstream of the decision and costs the report, never the enforcement.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/protect/engine/engine.js               |   6 +-
 src/protect/engine/fetch.js                |   5 +-
 src/protect/engine/middleware.js           |   5 +-
 src/protect/engine/node.js                 |   5 +-
 src/protect/notify.js                      |  59 ++++++++
 src/protect/rules/refresh.js               |   4 +-
 src/protect/rules/source.js                |  15 ++-
 src/protect/runtime.js                     |  23 ++--
 tests/protect/callback-containment.test.ts | 148 +++++++++++++++++++++
 9 files changed, 240 insertions(+), 30 deletions(-)
 create mode 100644 src/protect/notify.js
 create mode 100644 tests/protect/callback-containment.test.ts

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..534cc91 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,9 +244,7 @@ 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
     }
 
diff --git a/src/protect/engine/middleware.js b/src/protect/engine/middleware.js
index 343a146..a5846d4 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';
 
@@ -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..c13b8cf 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,9 +134,7 @@ 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
       }
 
diff --git a/src/protect/notify.js b/src/protect/notify.js
new file mode 100644
index 0000000..a15086a
--- /dev/null
+++ b/src/protect/notify.js
@@ -0,0 +1,59 @@
+/**
+ * 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 — lets a caller fall back to its own
+ *   reporting when a host's handler is broken, rather than losing the report entirely
+ */
+
+/** 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 {
+    fn(arg);
+
+    return true;
+  } catch (err) {
+    if (!reported.has(label)) {
+      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 threw 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 */
+      }
+    }
+
+    return false;
+  }
+}
+
+/** 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..3ef2c68 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
@@ -29,10 +30,10 @@ function reportRejections(rejected, options, label) {
     }
   }
   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();
   }
 
diff --git a/src/protect/runtime.js b/src/protect/runtime.js
index a35143e..25f7ec8 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 });
@@ -285,7 +286,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 +427,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 +468,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 +508,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 +534,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 +574,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 +586,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 +649,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 +658,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..ec956ce
--- /dev/null
+++ b/tests/protect/callback-containment.test.ts
@@ -0,0 +1,148 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { createProtection } from '../../src/protect/runtime.js';
+import { notify, resetNotifyWarnings } from '../../src/protect/notify.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');
+};
+
+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;
+    }
+  });
+});

From 751691b5199f81ab6170cad279e296738c54d2b3 Mon Sep 17 00:00:00 2001
From: Dave Jong <dave.jong@patchstack.com>
Date: Thu, 20 Aug 2026 16:37:42 +0200
Subject: [PATCH 2/2] Contain async rejections, and every remaining callback
 site
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two holes in the previous commit, both from scoping the problem by callback name
instead of by what the code actually calls.

An async callback escapes a try/catch entirely. `async () => { throw ... }` does
not throw — it returns a rejected promise, which settles after containment has
returned, and on Node an unhandled rejection terminates the process by default.
So the fix as written contained synchronous hosts and left async ones able to
kill the app: worse than the throw it set out to catch, because the app dies
rather than losing a report. notify now attaches a rejection handler when the
callback hands back a thenable, and routes it to the same one-time warning.

That hole was not limited to the new code. The two pre-existing try/catch
wrappers around onSkip had it as well, which is the argument for the containment
living in one place instead of being written out at each site.

onBlock, onEgressBlock and onScan were still called directly, because the first
pass enumerated three callbacks by name rather than everything the code invokes.
onBlock runs after the decision to block and before the block response is
produced, so an escaping throw did not let the request through — it replaced the
403 with the callback's exception. That is not availability, it is enforcement
integrity: what a blocked request returns has to come from the rule and never
from reporting code. onScan is worse in a quieter way, since a throw there
aborted lazy init and left every later request unscreened.

Reachability, stated because severity depends on it: onEgressBlock is wired from
createProtection today, while onBlock and onScan are reachable only through the
adapters, which the public entry point does not forward to. They are guarded
anyway — the invariant belongs at the call site, not in an assumption that
nobody passes the option.

Return contract now documents its limit: for an async callback, a true result can
only mean the call started, so the engine's fall-through to its own logging
applies to synchronous handlers only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/protect/egress.js                      |   9 +-
 src/protect/engine/fetch.js                |  15 ++-
 src/protect/engine/middleware.js           |  28 ++--
 src/protect/engine/node.js                 |  14 +-
 src/protect/notify.js                      |  53 +++++---
 src/protect/rules/source.js                |   4 +-
 src/protect/runtime.js                     |  10 +-
 tests/protect/callback-containment.test.ts | 146 +++++++++++++++++++++
 8 files changed, 225 insertions(+), 54 deletions(-)

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/fetch.js b/src/protect/engine/fetch.js
index 534cc91..1191888 100644
--- a/src/protect/engine/fetch.js
+++ b/src/protect/engine/fetch.js
@@ -249,13 +249,14 @@ export function createFetchMiddleware(rulesData, options = {}) {
     }
 
     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 a5846d4..ab19a4a 100644
--- a/src/protect/engine/middleware.js
+++ b/src/protect/engine/middleware.js
@@ -9,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',
@@ -111,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);
 
diff --git a/src/protect/engine/node.js b/src/protect/engine/node.js
index c13b8cf..20ae129 100644
--- a/src/protect/engine/node.js
+++ b/src/protect/engine/node.js
@@ -139,13 +139,13 @@ export function createNodeMiddleware(rulesData, options = {}) {
       }
 
       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
index a15086a..ec3b717 100644
--- a/src/protect/notify.js
+++ b/src/protect/notify.js
@@ -19,8 +19,10 @@
  * @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 — lets a caller fall back to its own
- *   reporting when a host's handler is broken, rather than losing the report entirely
+ * @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. */
@@ -30,29 +32,50 @@ export function notify(fn, arg, label) {
   if (typeof fn !== 'function') return false;
 
   try {
-    fn(arg);
+    const result = fn(arg);
 
-    return true;
-  } catch (err) {
-    if (!reported.has(label)) {
-      reported.add(label);
+    // 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 {
-        // 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 threw and was ignored. ` +
-            `Protection is unaffected; this is reported once per process. ` +
-            `Cause: ${err && err.message ? err.message : String(err)}`,
-        );
+        result.then(undefined, (err) => warnOnce(label, err));
       } catch {
-        /* no console on this runtime */
+        // 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/source.js b/src/protect/rules/source.js
index 3ef2c68..7d69980 100644
--- a/src/protect/rules/source.js
+++ b/src/protect/rules/source.js
@@ -26,7 +26,7 @@ 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('; ');
@@ -119,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 25f7ec8..3a4f236 100644
--- a/src/protect/runtime.js
+++ b/src/protect/runtime.js
@@ -221,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 =
diff --git a/tests/protect/callback-containment.test.ts b/tests/protect/callback-containment.test.ts
index ec956ce..f4d89d4 100644
--- a/tests/protect/callback-containment.test.ts
+++ b/tests/protect/callback-containment.test.ts
@@ -1,6 +1,7 @@
 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.
@@ -25,6 +26,26 @@ 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<unknown>): Promise<unknown[]> {
+  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(() => {
@@ -146,3 +167,128 @@ describe('notify', () => {
     }
   });
 });
+
+
+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');
+  });
+});