Stop a host's callback from breaking the guard that calls it

src/protect/egress.js

@@ -12,14 +12,19 @@
  *           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 () => {};
   const exempt = new Set((allowHosts ?? []).map((h) => String(h).toLowerCase()));
 
   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) =>

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

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);
     }
 

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');
         });
     }
 

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);
       }
 

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();
+}

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();
   };

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('; ');