Keep a detection's rule identity, and let a guard be shut down

AGENT-INSTALL.md

@@ -138,13 +138,15 @@ would have stopped while it is still in dry-run. Two separate paths, with differ
   `reportFirewallLog: false` in `createProtection`.
 - **Every rule that matched** goes to `monitor/pulse/detections/<your site uuid>` — including matches that
   blocked, which are reported on both paths. This is **off unless you pass `reportDetections: true`** to
-  `createProtection`; the scaffolded guard does not pass it. It also requires a provisioned site UUID and
-  is disabled by `PATCHSTACK_TELEMETRY=off`. It exists because a rule carrying `dry-run` blocks nothing,
-  so without it nothing distinguishes a rule that is protecting from one that is quietly wrong.
+  `createProtection`; the scaffolded guard does not pass it. It also requires a provisioned site UUID, a
+  resolvable credential, and is disabled by `PATCHSTACK_TELEMETRY=off`. It exists because a rule carrying
+  `dry-run` blocks nothing, so without it nothing distinguishes a rule that is protecting from one that is
+  quietly wrong.
 
 What a detection report contains, per matched rule: the rule id, the request path **with any query string
 removed**, the parameter names that rule reads (from the rule's own definition), which phase matched,
-whether it was enforced, the identifier of the rule bundle in use, and a timestamp. Each batch also
+whether it was enforced, the identifier of the rule bundle in use, the revision of the rule itself when the
+bundle carried one, and a timestamp. Each batch also
 carries a count of reports dropped when traffic outran the flush, so a partial sample is not read as a
 complete one.
 
@@ -159,6 +161,16 @@ and not the value of any header, cookie or query-string parameter — including
 named above. Reports are batched, capped in memory, and dropped rather than retried if Patchstack cannot
 be reached — a reporting failure never delays or fails a request.
 
+The endpoint needs a credential, so `reportDetections: true` with none resolved starts nothing: the guard
+warns once at boot and `protection.detectionReporting` reads `unavailable-no-credential` instead of `on`.
+When reporting is on, `protection.detectionHealth()` returns local counts — detections attempted,
+acknowledged, refused or unreachable, dropped for queue pressure — and the time of the last
+acknowledgement. Those counts stay in your process; nothing extra is sent to report them.
+
+`protection.stop()` stops everything the guard has running in the background — the rule-refresh loop, the
+block-log reporter, the detection reporter — and flushes what is buffered. `protection.stopRefresh()` is
+the same method under its older name. Call it on shutdown; it is safe to call twice.
+
 Two more endpoints the package can call, for completeness:
 
 - `GET monitor/widget/settings/<your site uuid>` — how `status` tells "this site was deleted on

src/protect/detections.js

@@ -15,8 +15,8 @@ import { isSafeOrigin } from './safe-origin.js';
  * ## The payload is deliberately small
  *
  * `rule_id`, route PATH, the parameters the rule reads, a timestamp, whether it was enforced, the phase,
- * and the bundle identity. That is enough to count hits per rule, compare them against traffic, and
- * decide whether a rule is wrong.
+ * the bundle identity, and the rule's own revision where the bundle carried one. That is enough to count
+ * hits per rule, compare them against traffic, and decide whether a rule is wrong.
  *
  * What it never carries: **the matched value, the request body, headers, or query-string values**. A
  * channel that counts detections is a different thing from a copy of an application's traffic, and once
@@ -61,6 +61,24 @@ export function ruleParameters(rule) {
   return [...out];
 }
 
+/**
+ * The rule's own revision, from the served rule.
+ *
+ * Read off the delivered rule rather than derived: whoever served it knows what document this is, and a
+ * value computed here would be this client's opinion of it. Accepts a string or a number, because the two
+ * kinds of rule that carry one number their revisions differently.
+ *
+ * @param {any} rule
+ * @returns {string | null}
+ */
+export function revisionOf(rule) {
+  const revision = rule?.source_revision;
+  if (typeof revision === 'string' && revision !== '') return revision;
+  if (typeof revision === 'number' && Number.isFinite(revision)) return String(revision);
+
+  return null;
+}
+
 /**
  * The request path with the query string removed.
  *
@@ -92,7 +110,11 @@ export function createDetectionReporter(opts) {
   const siteUuid = opts.siteUuid ?? process.env?.PATCHSTACK_SITE_UUID;
   if (!siteUuid) {
     // Nothing to report against. A no-op rather than a throw: reporting is never worth failing a boot.
-    return { record() {}, flush() {}, stop() {}, dropped: () => 0 };
+    // It answers the whole interface, so a caller never has to know which kind it holds.
+    return {
+      record() {}, flush() {}, stop() {}, setRulesEtag() {}, dropped: () => 0,
+      health: () => ({ sent: 0, delivered: 0, failed: 0, dropped: 0, lastDeliveredAt: null }),
+    };
   }
 
   const configured = opts.baseUrl ?? process.env?.PATCHSTACK_PULSE_RULES_URL;
@@ -103,6 +125,22 @@ export function createDetectionReporter(opts) {
   const flushMs = Number.isFinite(opts.flushMs) && opts.flushMs > 0 ? opts.flushMs : DEFAULT_FLUSH_MS;
   const maxQueue = Number.isFinite(opts.maxQueue) && opts.maxQueue > 0 ? opts.maxQueue : MAX_QUEUE;
 
+  // The bundle identity stamped onto each event. MUTABLE, because the guard's rules are: a refresh
+  // hot-swaps the ruleset in place, and a reporter holding the boot-time value would attribute a hit
+  // produced by the new bundle to the old one — sending a reviewer to a rule document that is not the
+  // one that fired. Updated by the runtime only after an accepted swap (see `setRulesEtag`).
+  let rulesEtag = opts.rulesEtag ?? null;
+
+  // Delivery health. The capability declaration says a guard INTENDS to report; these say whether
+  // anything arrived. Counts and one timestamp only — a clean app and a broken delivery path are
+  // otherwise indistinguishable, and distinguishing them needs no request data at all.
+  let sent = 0;
+  let delivered = 0;
+  let failed = 0;
+  let droppedTotal = 0;
+  /** @type {string | null} */
+  let lastDeliveredAt = null;
+
   /** @type {Array<Record<string, unknown>>} */
   let queue = [];
   /** @type {ReturnType<typeof setTimeout> | null} */
@@ -123,6 +161,8 @@ export function createDetectionReporter(opts) {
     // would make a truncated sample look like a complete one.
     const droppedWith = dropped;
     dropped = 0;
+    droppedTotal += droppedWith;
+    sent += batch.length;
 
     void (async () => {
       try {
@@ -132,17 +172,26 @@ export function createDetectionReporter(opts) {
             'Content-Type': 'application/json',
             Accept: 'application/json',
             'User-Agent': '@patchstack/connect',
-            // Same credential path as the rules fetch, and unauthenticated when none resolves: the
-            // server accepts the UUID, and reporting must never hinge on getting a token.
+            // Same credential path as the rules fetch. The detections endpoint is site-addressed and
+            // requires a verified, site-bound token, so a batch sent without one is refused — which is
+            // why the runtime does not build a reporter when no credential resolves, rather than
+            // posting into a 401.
             ...(await pulseAuthHeader({ pulseAuth: opts.pulseAuth, endpoint: baseUrl }, fetchImpl)),
           },
           body: JSON.stringify({ detections: batch, dropped: droppedWith }),
         });
-        // Fail-open and silent: a rejected or unreachable endpoint must not disturb the app, and must
-        // not retry into a loop either. The next flush carries whatever arrives next.
-        if (res && typeof res.then === 'function') res.catch(() => {});
+        // Fail-open and no retry: a rejected or unreachable endpoint must not disturb the app, and a
+        // retry loop over a refusing endpoint is worse than the lost batch. The outcome is counted, so
+        // that a delivery path which refuses everything is distinguishable from an app where no rule
+        // fired — both are silence at the server otherwise.
+        if (res && res.ok) {
+          delivered += batch.length;
+          lastDeliveredAt = new Date().toISOString();
+        } else {
+          failed += batch.length;
+        }
       } catch {
-        /* ignore */
+        failed += batch.length;
       }
     })();
   };
@@ -174,7 +223,12 @@ export function createDetectionReporter(opts) {
         // The state this detection was handled under, which is the whole point: `false` is a rule that
         // saw traffic it would have stopped.
         enforced: detection.mode === 'block',
-        rules_etag: opts.rulesEtag ?? null,
+        rules_etag: rulesEtag,
+        // The revision of THIS rule, as the bundle delivered it. The bundle identity above answers "which
+        // bundle", which changes whenever anything in it changes — so it cannot say whether the counts for
+        // one rule describe the document that rule has now. Passed through untouched, and null when the
+        // bundle carried none.
+        rule_revision: revisionOf(detection.rule),
         detected_at: new Date().toISOString(),
       });
 
@@ -190,6 +244,23 @@ export function createDetectionReporter(opts) {
       stopped = true;
       flush();
     },
+    /**
+     * Point later events at the bundle now running. Called after an ACCEPTED swap only — a rejected
+     * or failed refresh keeps the previous rules, so it must keep the previous identity too.
+     *
+     * Already-queued events are not rewritten: the stamp is taken when an event is recorded, which is
+     * the only moment the two are known to agree.
+     *
+     * @param {string | null | undefined} next
+     */
+    setRulesEtag(next) {
+      rulesEtag = next ?? null;
+    },
     dropped: () => dropped,
+    /**
+     * Delivery health, counted in events: attempted, acknowledged, refused or unreachable, and dropped
+     * for queue pressure — plus when a batch was last acknowledged. No request data of any kind.
+     */
+    health: () => ({ sent, delivered, failed, dropped: droppedTotal + dropped, lastDeliveredAt }),
   };
 }

src/protect/engine/pulse-client.js

@@ -7,8 +7,9 @@ const DEFAULT_CACHE_TTL = 300_000;
 // revalidate on the same tick (spreads load / avoids a thundering herd against the rules API).
 const JITTER_FRACTION = 0.1;
 
-// Per-site rules client for Pulse (npm/JS) apps. Public endpoint — the site UUID is the only
-// credential, passed in the path. Fail-open: any error returns success:false + empty rules so
+// Per-site rules client for Pulse (npm/JS) apps. The site UUID addresses the site in the path; it is
+// not a credential — the endpoint requires a verified token bound to that same site, so a fetch without
+// one is refused. Fail-open regardless: any error returns success:false + empty rules so
 // createProtection falls back to the disk cache or the bundled rules.
 //
 // Conditional fetch: pass a prior `etag` (persisted with the last cached bundle) and the client
@@ -61,24 +62,22 @@ export class PulseRuleClient {
     }
     const url = `${this.#baseUrl}/rules/${encodeURIComponent(this.#siteUuid)}`;
     try {
-      // Unauthenticated when no credential resolved, or when the exchange
-      // fails — the server still accepts the UUID, and protection must never
-      // hinge on getting a token.
+      // Sent without an `Authorization` header when no credential resolved or the exchange failed.
+      // The server refuses that, and the refusal is handled the same way as any other failure: fall
+      // back to cached or bundled rules. Attempting it anyway is deliberate — protection must never
+      // hinge on the token path, and the runtime warns at boot when no credential resolves.
       const auth = await pulseAuthHeader(
         { pulseAuth: this.#pulseAuth, endpoint: this.#baseUrl, timeoutMs: this.#timeoutMs },
         fetch,
       );
       const headers = { Accept: 'application/json', ...auth };
-      // Claimed only on an authenticated request. The rules endpoint still accepts a bare UUID, so on that
-      // path this header would be an assertion anyone holding the UUID could make — and it asserts the
-      // reassuring thing: that reporting is on. A dashboard would then say a site is covered because a
-      // stranger said so.
+      // Claimed only on an authenticated request. Fetching rules must never hinge on getting a token
+      // (protection comes first), but CLAIMING a capability may: an unauthenticated request is one whose
+      // statements about this site carry no weight, and this header asserts the reassuring thing — that
+      // reporting is on.
       //
-      // Fetching rules must never hinge on getting a token (protection comes first), but CLAIMING a
-      // capability may: an unauthenticated request is one whose statements about this site carry no weight.
-      // This check only removes the ACCIDENTAL case. The forgeable one is not the client's to prevent, so
-      // anything acting on this header has to require a verified token itself before believing it — a
-      // client-side gate is a courtesy, never the guarantee.
+      // A courtesy, never the guarantee: a client-side gate only removes the accidental case. Anything
+      // acting on this header has to require a verified token itself before believing it.
       if (this.#reportsDetections && typeof auth.Authorization === 'string') {
         headers['X-Patchstack-Detections'] = 'enabled';
       }

src/protect/protect.d.ts

@@ -25,13 +25,31 @@ export interface Protection {
   node(options?: { maxBodyBytes?: number; screenResponses?: boolean }): (req: unknown, res: unknown, next: () => void) => void;
   /** Present when `egress: true` — restores the original global fetch. */
   uninstallEgress?: () => void;
-  /** Present with a live source — re-fetch + hot-swap the rules once (used by the loop + push). */
-  refresh?: () => Promise<void>;
+  /** Present with a live source — re-fetch + hot-swap the rules once (used by the loop + push).
+   *  Resolves with the outcome of the attempt: `ok: false` means the rules in force came from the
+   *  cache or the bundled fallback, not from the source. It does not reject on a source failure. */
+  refresh?: () => Promise<{ ok: boolean; reason?: string }>;
   /** Present with a live source — a fetch handler that runs `refresh()` when the request carries
    *  the configured refresh secret (a push/zero-day trigger). No secret set → the handler 404s. */
   refreshHandler?: () => (request: Request) => Promise<Response>;
-  /** Present when `refreshMs > 0` — stops the live rule-refresh loop. */
-  stopRefresh?: () => void;
+  /** Stops everything with a timer or a buffer behind it: the refresh loop, the block log, the
+   *  detection reporter (flushing what it holds). Always present, and safe to call twice. */
+  stop: () => void;
+  /** Alias of `stop`, under the name callers already have. */
+  stopRefresh: () => void;
+  /** Whether detection reporting is running, requested but undeliverable, or not requested.
+   *  `unavailable-no-credential` means `reportDetections` was set but no credential resolved, so
+   *  nothing is being sent. */
+  detectionReporting: "on" | "off" | "unavailable-no-credential";
+  /** Present when detection reporting is on — delivery counts (in events) and the last acknowledgement.
+   *  Carries no request data. */
+  detectionHealth?: () => {
+    sent: number;
+    delivered: number;
+    failed: number;
+    dropped: number;
+    lastDeliveredAt: string | null;
+  };
 }
 
 export interface CreateProtectionOptions {
@@ -81,6 +99,8 @@ export interface CreateProtectionOptions {
    * this is a counting channel, not a copy of your traffic.
    *
    * Off by default because switching it on adds an outbound request to every guard with a site UUID.
+   * Needs a resolvable API credential: the endpoint requires a verified, site-bound token, so with no
+   * credential no reporter is created and `detectionReporting` reads `unavailable-no-credential`.
    */
   reportDetections?: boolean;
   /** How long to buffer detections before posting a batch. Default 5000ms. */

src/protect/rules/refresh.js

@@ -3,7 +3,10 @@
 // caller-supplied `tick` (which re-fetches + hot-swaps the engines):
 //   - startRefresh: a self-scheduling poll LOOP (reschedules after each tick settles, so runs never
 //     overlap), with ±jitter (avoid a thundering herd) and exponential backoff on consecutive
-//     failures. `unref`'d — it never keeps the process alive.
+//     failures. `unref`'d — it never keeps the process alive. A tick counts as failed when it throws
+//     OR when it reports `{ ok: false }`: the rule resolver absorbs an API or network failure into
+//     usable fallback rules, so a thrown error is not the only shape an outage takes, and a poller
+//     that waits for one keeps the whole fleet knocking at the normal interval while it lasts.
 //   - 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.
 
@@ -29,8 +32,9 @@ export function startRefresh(tick, { refreshMs, onError } = {}) {
   const run = async () => {
     if (stopped) return;
     try {
-      await tick();
-      failures = 0;
+      const status = await tick();
+      if (status && status.ok === false) failures++;
+      else failures = 0;
     } catch (err) {
       failures++;
       notify(onError, err, 'onError');
@@ -60,7 +64,10 @@ export function makeRefreshHandler(tick, secret) {
     if (provided !== secret) return new Response('forbidden', { status: 403 });
     let refreshed = true;
     try {
-      await tick();
+      // `{ ok: false }` means the tick ran but the rules did not come from the source, which is not a
+      // refresh — the caller pushed because it had something to deliver, and it did not arrive.
+      const status = await tick();
+      if (status && status.ok === false) refreshed = false;
     } catch {
       refreshed = false; // fail-open: report the outcome, never throw
     }