protect: accept live rule updates atomically; reject rule_id-less whitelists

src/protect/rules/source.js

@@ -6,6 +6,35 @@ import { PatchstackRuleClient } from '../engine/index.js';
 import { PulseRuleClient } from '../engine/pulse-client.js';
 import { validateBundle } from './validate.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
+// broken update REPLACE known-good policy with partial or empty policy — turning "we validated it" into
+// a loss of protection, and caching that loss. So: if any rule/whitelist fails validation, reject the
+// whole update, keep last-known-good, report it, and do NOT write the cache. Opt in to the old
+// behaviour with `acceptPartialBundle: true` (metrics still report every drop).
+function liveUpdateRejections(res, options) {
+  if (options.acceptPartialBundle) return [];
+  const { rejected } = validateBundle(
+    { firewall: Array.isArray(res.firewall) ? res.firewall : [], whitelists: Array.isArray(res.whitelists) ? res.whitelists : [] },
+    { allowGlobalWhitelists: options.allowGlobalWhitelists },
+  );
+  return rejected;
+}
+
+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 */ }
+    }
+  }
+  const sample = rejected.slice(0, 3).map((r) => `${r.id} (${r.reason})`).join('; ');
+  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 ? ', …' : ''}`,
+  ));
+}
+
 export async function resolveRules(options, store, ctx = {}) {
   // The INITIAL load is on the app's startup path, so the runtime gives it a short budget (see
   // bootTimeoutMs) and falls back to cache/bundled rather than delaying boot; refreshes get the full
@@ -17,6 +46,13 @@ export async function resolveRules(options, store, ctx = {}) {
     const res = await client.getRules();
     if (res.success && res.notModified && prior?.bundle) return normalizeBundle(prior.bundle, options);
     if (res.success && !res.notModified) {
+      const rejected = liveUpdateRejections(res, options);
+      if (rejected.length > 0) {
+        reportRejections(rejected, options, 'rule update rejected');
+        if (prior?.bundle) return normalizeBundle(prior.bundle, options);
+        if (options.rules) return normalizeBundle(options.rules, options);
+        return emptyBundle();
+      }
       const bundle = normalizeBundle(res, options);
       await store.write({ bundle, etag: res.etag ?? null });
       return bundle;
@@ -39,6 +75,13 @@ export async function resolveRules(options, store, ctx = {}) {
     const res = await client.getRules();
     if (res.success && res.notModified && prior?.bundle) return normalizeBundle(prior.bundle, options);
     if (res.success && !res.notModified) {
+      const rejected = liveUpdateRejections(res, options);
+      if (rejected.length > 0) {
+        reportRejections(rejected, options, 'rule update rejected');
+        if (prior?.bundle) return normalizeBundle(prior.bundle, options);
+        if (options.rules) return normalizeBundle(options.rules, options);
+        return emptyBundle();
+      }
       const bundle = normalizeBundle(res, options);
       await store.write({ bundle, etag: res.etag ?? null });
       return bundle;
@@ -64,10 +107,13 @@ export async function resolveRules(options, store, ctx = {}) {
 // (`onRuleRejected`) rather than silently kept — an unenforceable rule must never look enforced.
 export function normalizeBundle(b, options = {}) {
   const enforcement = b?.enforcement ?? b?.mode;
-  const { bundle: checked, rejected } = validateBundle({
-    firewall: Array.isArray(b.firewall) ? b.firewall : [],
-    whitelists: Array.isArray(b.whitelists) ? b.whitelists : [],
-  });
+  const { bundle: checked, rejected } = validateBundle(
+    {
+      firewall: Array.isArray(b.firewall) ? b.firewall : [],
+      whitelists: Array.isArray(b.whitelists) ? b.whitelists : [],
+    },
+    { allowGlobalWhitelists: options.allowGlobalWhitelists },
+  );
   if (rejected.length > 0) {
     const report = options.onRuleRejected;
     if (typeof report === 'function') {

src/protect/rules/validate.js

@@ -30,7 +30,7 @@ const ACTIONS = new Set(['block', 'redact', 'encode', 'set-header', 'remove-head
  * @param {object} bundle
  * @returns {{ bundle: object, rejected: Array<{id: string, reason: string}> }}
  */
-export function validateBundle(bundle) {
+export function validateBundle(bundle, opts = {}) {
   const rejected = [];
   const inFirewall = Array.isArray(bundle?.firewall) ? bundle.firewall : [];
   const inWhitelists = Array.isArray(bundle?.whitelists) ? bundle.whitelists : [];
@@ -53,6 +53,12 @@ export function validateBundle(bundle) {
       continue;
     }
     // A whitelist SUPPRESSES rules, so a malformed one is a protection risk, not a detection risk.
+    // One with no `rule_id` applies to EVERY rule — a single tripped condition disables the whole
+    // firewall for that request — so it must be opted into explicitly.
+    if (!opts.allowGlobalWhitelists && wl && Array.isArray(wl.rule_v2) && !wl.rule_id) {
+      rejected.push({ id: idOf(wl), reason: 'whitelist has no rule_id (would suppress every rule); set allowGlobalWhitelists to permit' });
+      continue;
+    }
     const reason = conditionsProblem(wl?.rule_v2);
     if (reason) rejected.push({ id: idOf(wl), reason: `whitelist: ${reason}` });
     else whitelists.push(wl);

src/protect/runtime.js

@@ -162,6 +162,8 @@ export async function createProtection(options = {}) {
   // parse failure, a DNS resolver failure. Each of those is a real hole in enforcement, and until now
   // it was SILENT — "always-on" read as "always inspected". Every such bypass is now counted and
   // reported to `onSkip`, so a host can alert on it and `protection.coverage()` can be surfaced.
+  // `onSkip` is a TRUSTED SERVER callback: `detail` carries operational context (sizes, statuses,
+  // outbound hostnames) for logging/alerting. Do not forward it to a client response.
   const skipCounts = Object.create(null);
   const onSkip = typeof options.onSkip === 'function' ? options.onSkip : null;
   const recordSkip = (phase, reason, detail) => {

tests/protect/atomic-bundle.test.ts

@@ -0,0 +1,94 @@
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { mkdtempSync, rmSync, existsSync, readFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { createProtection } from '../../src/protect/runtime.js';
+
+// Validation must never make a bad update WORSE than no update. Dropping individual bad rules is fine
+// for a bundle we already trust, but for a fresh remote response it would let a broken/oversized/
+// truncated update replace known-good policy with partial or empty policy — and cache that loss.
+// A live update is therefore accepted atomically: all-or-nothing, keep last-known-good, don't cache.
+
+const GOOD = { id: 'good-1', rule_v2: [{ parameter: 'raw', match: { type: 'contains', value: '__proto__' } }] };
+const BAD = { id: 'bad-1', phase: 'sideways', rule_v2: [{ parameter: 'raw', match: { type: 'contains', value: 'x' } }] };
+const bundle = (firewall: any[]) => JSON.stringify({ firewall, whitelists: [], whitelist_keys: {} });
+
+afterEach(() => vi.restoreAllMocks());
+
+describe('atomic live-bundle acceptance', () => {
+  it('keeps the cached ruleset and does NOT overwrite the cache when an update fails validation', async () => {
+    const cacheDir = mkdtempSync(join(tmpdir(), 'ps-atomic-'));
+    try {
+      // 1. A good update is accepted and cached.
+      vi.stubGlobal('fetch', vi.fn(async () => new Response(bundle([GOOD]), { status: 200, headers: { ETag: '"v1"' } })));
+      const p1: any = await createProtection({ siteUuid: 's1', pulseRulesUrl: 'https://x.test/p', cacheDir, mode: 'block' });
+      expect(p1.rules.request.map((r: any) => r.id)).toEqual(['good-1']);
+      const cachedAfterGood = readFileSync(join(cacheDir, 'patchstack-rules.json'), 'utf8');
+      expect(cachedAfterGood).toContain('good-1');
+
+      // 2. A later update contains an invalid rule → reject the WHOLE update.
+      const errors: Error[] = [];
+      vi.stubGlobal('fetch', vi.fn(async () => new Response(bundle([GOOD, BAD]), { status: 200, headers: { ETag: '"v2"' } })));
+      const p2: any = await createProtection({
+        siteUuid: 's1', pulseRulesUrl: 'https://x.test/p', cacheDir, mode: 'block',
+        onError: (e: Error) => errors.push(e),
+      });
+      // Still protected by last-known-good, and the cache was NOT replaced.
+      expect(p2.rules.request.map((r: any) => r.id)).toEqual(['good-1']);
+      expect(readFileSync(join(cacheDir, 'patchstack-rules.json'), 'utf8')).toBe(cachedAfterGood);
+      expect(errors.map((e) => e.message).join(' ')).toMatch(/rejected the entire update/i);
+    } finally {
+      rmSync(cacheDir, { recursive: true, force: true });
+    }
+  });
+
+  it('reports each rejected rule and never caches the bad response', async () => {
+    const cacheDir = mkdtempSync(join(tmpdir(), 'ps-atomic2-'));
+    try {
+      const rejected: any[] = [];
+      vi.stubGlobal('fetch', vi.fn(async () => new Response(bundle([BAD]), { status: 200 })));
+      // No prior cache and no bundled fallback → running with no rules is correct, but nothing is cached.
+      const p: any = await createProtection({
+        siteUuid: 's1', pulseRulesUrl: 'https://x.test/p', cacheDir, mode: 'block',
+        onRuleRejected: (r: any) => rejected.push(r),
+      });
+      expect(p.rules.request).toEqual([]);
+      expect(rejected[0]).toMatchObject({ id: 'bad-1', accepted: false });
+      expect(existsSync(join(cacheDir, 'patchstack-rules.json'))).toBe(false);
+    } finally {
+      rmSync(cacheDir, { recursive: true, force: true });
+    }
+  });
+
+  it('falls back to the bundled ruleset rather than an empty policy', async () => {
+    vi.stubGlobal('fetch', vi.fn(async () => new Response(bundle([BAD]), { status: 200 })));
+    const p: any = await createProtection({
+      siteUuid: 's1', pulseRulesUrl: 'https://x.test/p', mode: 'block',
+      rules: { firewall: [GOOD], whitelists: [], whitelist_keys: {} } as any,
+    });
+    expect(p.rules.request.map((r: any) => r.id)).toEqual(['good-1']);
+  });
+
+  it('accepts a partial bundle only when explicitly opted in', async () => {
+    vi.stubGlobal('fetch', vi.fn(async () => new Response(bundle([GOOD, BAD]), { status: 200 })));
+    const p: any = await createProtection({
+      siteUuid: 's1', pulseRulesUrl: 'https://x.test/p', mode: 'block', acceptPartialBundle: true,
+    });
+    expect(p.rules.request.map((r: any) => r.id)).toEqual(['good-1']); // bad one dropped, good kept
+  });
+
+  it('rejects a whitelist with no rule_id, which would suppress every rule', async () => {
+    const rejected: any[] = [];
+    const global = { rule_v2: [{ parameter: 'get.debug', match: { type: 'equals', value: '1' } }] };
+    vi.stubGlobal('fetch', vi.fn(async () => new Response(
+      JSON.stringify({ firewall: [GOOD], whitelists: [global], whitelist_keys: {} }), { status: 200 },
+    )));
+    const p: any = await createProtection({
+      siteUuid: 's1', pulseRulesUrl: 'https://x.test/p', mode: 'block',
+      rules: { firewall: [GOOD], whitelists: [], whitelist_keys: {} } as any,
+      onRuleRejected: (r: any) => rejected.push(r),
+    });
+    expect(rejected.some((r) => /no rule_id/.test(r.reason))).toBe(true);
+    expect(p.rules.request.map((r: any) => r.id)).toEqual(['good-1']); // fell back, still protected
+  });
+});