Report detections the guard did not block, not only the ones it did (#156)

A rule that blocks nothing reports nothing. `firewall-log.js` posts enforced blocks in the
WordPress-compatible shape and answers "what did we stop"; there was no way to see that a rule
carrying `enforcement: dry-run` matched traffic it would otherwise have blocked. Without that, a
rule which is quietly wrong and a rule which is protecting look identical from the outside.

## The payload is small on purpose

Per detection: the rule id, the request PATH with the query string removed, the parameters the rule
reads, the phase, whether it was enforced, the rule-bundle ETag, and a timestamp.

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 the difference is
one careless field: once values are collected, every question about retention, access and
jurisdiction arrives with them. Anything value-level belongs behind its own explicit opt-in with its
own controls, not as a side effect of counting.

The route drops the query because `?token=…` is a value, and the guard against regression is a scan
of the SERIALIZED payload rather than of the object being built: a field added later (`message`,
`value`, `headers`) passes every structural assertion and fails that one.

`parameters` is the set the rule READS, from its own definition — not the condition that matched. The
engine reports a rule, not which of its conditions fired, and threading that out would mean changing
evaluation for the sake of a reporting field.

## Off by default

Enabling it adds an outbound POST to every guard configured with a site UUID, which is a change in
what an installed app does on the network — that belongs in the shipped docs before it becomes a
default rather than after. `reportDetections: true` switches it on.

Bounded (500 events, oldest dropped) and fail-open: an unreachable endpoint is silent and never
retries into a loop. The drop count is sent WITH the batch, because a consumer computing a rate from
these needs to know its denominator is short, and nobody infers that from a gap.

1249 tests, typecheck clean. Four guarantees are mutation-checked: keeping the query string, adding
the block message to the payload, removing the queue cap, and deriving `enforced` from the site mode
rather than the rule each fail the assertion that names them.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: patchstackdave

src/protect/detections.js

@@ -0,0 +1,195 @@
+import { pulseAuthHeader } from '../pulse-token.js';
+import { isSafeOrigin } from './safe-origin.js';
+
+/**
+ * Detection reporter: every rule that fired, whether or not it blocked.
+ *
+ * Separate from `firewall-log.js`, which posts ENFORCED blocks in the WordPress-compatible shape
+ * (`fid`, `request_uri`, `ip`, `user_agent`). That path answers "what did we stop". This one answers a
+ * question nothing else could: **what would this rule have stopped**, for a rule that carries
+ * `enforcement: dry-run` and therefore blocks nothing.
+ *
+ * Without it, a rule that is quietly wrong and a rule that is protecting look identical from the
+ * outside, because neither produces a block to report.
+ *
+ * ## 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.
+ *
+ * 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
+ * values are collected every question about retention, access and jurisdiction arrives with them.
+ * Anything value-level belongs behind its own explicit opt-in with its own controls, not as a side
+ * effect of counting.
+ *
+ * The route is the request PATH with any query string dropped, because `?token=…` is a value.
+ */
+
+const DEFAULT_BASE_URL = 'https://api.patchstack.com/monitor/pulse';
+const DEFAULT_FLUSH_MS = 5000;
+const MAX_BATCH = 50;
+/** Bounded so a detection storm costs memory it cannot grow out of. Oldest go first. */
+const MAX_QUEUE = 500;
+
+/**
+ * The parameters a rule reads, from its own definition.
+ *
+ * NOT "the condition that matched": the engine reports a rule, not which of its conditions fired, and
+ * threading that out would mean changing evaluation for the sake of a reporting field. A narrowly scoped
+ * rule reads exactly one parameter, so the two answers coincide there; for a broad rule this is the set
+ * it reads, which is what the field name says.
+ *
+ * @param {any} rule
+ * @returns {string[]}
+ */
+export function ruleParameters(rule) {
+  const out = new Set();
+  const walk = (conditions) => {
+    if (!Array.isArray(conditions)) return;
+    for (const condition of conditions) {
+      if (!condition || typeof condition !== 'object') continue;
+      if (typeof condition.parameter === 'string' && condition.parameter !== 'rules') {
+        out.add(condition.parameter);
+      }
+      if (Array.isArray(condition.rules)) walk(condition.rules);
+    }
+  };
+  walk(rule?.rule_v2);
+
+  return [...out];
+}
+
+/**
+ * The request path with the query string removed.
+ *
+ * A path is a route; a query string is data. `/api/preview?url=http://169.254.169.254/` names both the
+ * endpoint and the attack payload, and only the first belongs in a counting channel.
+ *
+ * @param {unknown} path
+ * @returns {string | null}
+ */
+export function routeOf(path) {
+  if (typeof path !== 'string' || path === '') return null;
+  const cut = path.search(/[?#]/);
+
+  return cut === -1 ? path : path.slice(0, cut);
+}
+
+/**
+ * @param {{
+ *   siteUuid?: string,
+ *   baseUrl?: string,
+ *   pulseAuth?: unknown,
+ *   rulesEtag?: string | null,
+ *   fetchImpl?: typeof fetch,
+ *   flushMs?: number,
+ *   maxQueue?: number,
+ * }} opts
+ */
+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 };
+  }
+
+  const configured = opts.baseUrl ?? process.env?.PATCHSTACK_PULSE_RULES_URL;
+  const baseUrl = typeof configured === 'string' && isSafeOrigin(configured)
+    ? configured.replace(/\/$/, '')
+    : DEFAULT_BASE_URL;
+  const fetchImpl = opts.fetchImpl ?? globalThis.fetch;
+  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;
+
+  /** @type {Array<Record<string, unknown>>} */
+  let queue = [];
+  /** @type {ReturnType<typeof setTimeout> | null} */
+  let timer = null;
+  let stopped = false;
+  let dropped = 0;
+
+  const flush = () => {
+    if (timer) {
+      clearTimeout(timer);
+      timer = null;
+    }
+    if (queue.length === 0 || typeof fetchImpl !== 'function') return;
+
+    const batch = queue.splice(0, MAX_BATCH);
+    // The count of what never made it, sent WITH the batch rather than inferred from a gap: a consumer
+    // computing a false-positive rate needs to know its denominator is short, and silence about that
+    // would make a truncated sample look like a complete one.
+    const droppedWith = dropped;
+    dropped = 0;
+
+    void (async () => {
+      try {
+        const res = await fetchImpl(`${baseUrl}/detections/${encodeURIComponent(siteUuid)}`, {
+          method: 'POST',
+          headers: {
+            '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.
+            ...(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(() => {});
+      } catch {
+        /* ignore */
+      }
+    })();
+  };
+
+  return {
+    /**
+     * @param {{
+     *   rule?: { id?: string, rule_v2?: unknown },
+     *   phase?: string,
+     *   mode?: string,
+     *   path?: string,
+     * }} detection
+     */
+    record(detection) {
+      if (stopped) return;
+      const ruleId = detection?.rule?.id;
+      if (ruleId === undefined || ruleId === null || ruleId === '') return;
+
+      if (queue.length >= maxQueue) {
+        queue.shift();
+        dropped++;
+      }
+
+      queue.push({
+        rule_id: ruleId,
+        route: routeOf(detection.path),
+        parameters: ruleParameters(detection.rule),
+        phase: detection.phase ?? null,
+        // 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,
+        detected_at: new Date().toISOString(),
+      });
+
+      if (queue.length >= MAX_BATCH) {
+        flush();
+
+        return;
+      }
+      if (!timer) timer = setTimeout(flush, flushMs);
+    },
+    flush,
+    stop() {
+      stopped = true;
+      flush();
+    },
+    dropped: () => dropped,
+  };
+}

src/protect/protect.d.ts

@@ -68,6 +68,23 @@ export interface CreateProtectionOptions {
    * Also disabled when `PATCHSTACK_TELEMETRY=off` or when no apiKey is available.
    */
   reportFirewallLog?: boolean;
+  /**
+   * Report EVERY rule that fired — including one in `dry-run` that did not block — to the Pulse
+   * detections endpoint. Off unless explicitly `true`.
+   *
+   * Why it exists: a rule that blocks nothing reports nothing, so a rule that is quietly wrong and a
+   * rule that is protecting look identical from the outside.
+   *
+   * What it sends, per detection: the rule id, the request PATH with the query string removed, the
+   * parameters the rule reads, the phase, whether it was enforced, the rule-bundle ETag, and a
+   * timestamp. It does NOT send the matched value, the request body, headers, or query-string values —
+   * 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.
+   */
+  reportDetections?: boolean;
+  /** How long to buffer detections before posting a batch. Default 5000ms. */
+  detectionFlushMs?: number;
   /** Optional Source-Host header for connector hostname checks. */
   sourceHost?: string;
   /** Optional fetch override (tests). */

src/protect/runtime.js

@@ -86,8 +86,15 @@ export async function createProtection(options = {}) {
         })
       : null;
 
+  // Every detection, enforced or not, to the Pulse detections endpoint. Distinct from the block log
+  // above: that records what was STOPPED, in the WordPress-compatible shape; this records what a rule
+  // WOULD have stopped, which is otherwise unobservable for a rule carrying `enforcement: dry-run`.
+  // Minimal payload by design; see `detections.js`.
+  let detections = null;
+
   const onDetect = (detection) => {
     userOnDetect(detection);
+    if (detections) detections.record(detection);
     if (firewallLog && detection?.mode === 'block') {
       firewallLog.record({
         rule: detection.rule,
@@ -109,6 +116,23 @@ export async function createProtection(options = {}) {
   // runtimes that have one, and refreshes should not repeat it.
   const pulseAuth = await resolvePulseAuth(options);
   const bundle = await resolveRules(options, store, { timeoutMs: bootTimeoutMs, pulseAuth });
+  // OPT-IN, deliberately. Two reasons, and the first is not about privacy: switching it on adds an
+  // outbound POST to every guard that has a site UUID, which is a change in what an installed app does
+  // on the network — the kind of thing that must be disclosed in the shipped docs before it is a default,
+  // not after. The second is that the default belongs to whoever owns that disclosure, so the capability
+  // lands here and the flip is a separate, deliberate change.
+  if (options.reportDetections === true && options.siteUuid && telemetryEnabled()) {
+    detections = createDetectionReporter({
+      siteUuid: options.siteUuid,
+      baseUrl: options.pulseRulesUrl,
+      pulseAuth,
+      // The bundle the guard is actually running, so a hit can be attributed to the rules that produced
+      // it rather than to whatever is current when the report is read.
+      rulesEtag: (await store.read())?.etag ?? null,
+      fetchImpl: options.fetchImpl,
+      flushMs: options.detectionFlushMs,
+    });
+  }
   // Mode is mutable so a Pulse refresh can flip dry-run ↔ block when SaaS enables production.
   // Precedence: PATCHSTACK_MODE env (local override) > API enforcement > options.mode > dry-run.
   let mode = resolveMode(options, bundle);
@@ -630,9 +654,13 @@ export async function createProtection(options = {}) {
     protection.stopRefresh = () => {
       loop.stop();
       firewallLog?.stop();
+      detections?.stop();
     };
   } else if (firewallLog) {
-    protection.stopRefresh = () => firewallLog.stop();
+    protection.stopRefresh = () => {
+      firewallLog.stop();
+      detections?.stop();
+    };
   }
 
   return protection;