protect: validate delivered rule bundles; bound patterns; harden the telemetry origin

src/protect/engine/client.js

@@ -1,3 +1,5 @@
+import { safeBaseUrl } from '../safe-origin.js';
+
 const DEFAULT_BASE_URL = 'https://api.patchstack.com';
 const DEFAULT_CACHE_TTL = 300_000;
 // Randomly shorten the effective TTL by up to this fraction so many long-lived clients don't all
@@ -18,7 +20,7 @@ export class PatchstackRuleClient {
 
   constructor({ token, baseUrl, cacheTtl, etag } = {}) {
     this.#token = token ?? process.env.PATCHSTACK_WAF_TOKEN;
-    this.#baseUrl = baseUrl ?? process.env.PATCHSTACK_WAF_API_URL ?? DEFAULT_BASE_URL;
+    this.#baseUrl = safeBaseUrl(baseUrl ?? process.env.PATCHSTACK_WAF_API_URL, DEFAULT_BASE_URL, 'rule endpoint');
     this.#cacheTtl = Number.isFinite(cacheTtl) && cacheTtl > 0 ? cacheTtl : DEFAULT_CACHE_TTL;
     this.#etag = etag ?? null;
 

src/protect/engine/engine.js

@@ -29,10 +29,18 @@ function warnRejectedPatternOnce(pattern) {
   );
 }
 
+// An absurdly long pattern is either a mistake or an attack on our own matcher; compiling and running
+// it on every request is unbounded work. The rule-bundle validator rejects these upstream — this is the
+// backstop for a caller-supplied bundle that never went through it.
+const MAX_PATTERN_LENGTH = 1000;
+
 export function safeRegExp(pattern) {
   if (!pattern) {
     return null;
   }
+  if (typeof pattern !== 'string' || pattern.length > MAX_PATTERN_LENGTH) {
+    return null;
+  }
 
   for (const dangerous of REDOS_PATTERNS) {
     if (dangerous.test(pattern)) {

src/protect/engine/pulse-client.js

@@ -1,3 +1,5 @@
+import { safeBaseUrl } from '../safe-origin.js';
+
 const DEFAULT_BASE_URL = 'https://api.patchstack.com/monitor/pulse';
 const DEFAULT_CACHE_TTL = 300_000;
 // Randomly shorten the effective TTL by up to this fraction so many long-lived clients don't all
@@ -24,7 +26,8 @@ export class PulseRuleClient {
 
   constructor({ siteUuid, baseUrl, cacheTtl, etag } = {}) {
     this.#siteUuid = siteUuid ?? process.env.PATCHSTACK_SITE_UUID;
-    this.#baseUrl = baseUrl ?? process.env.PATCHSTACK_PULSE_RULES_URL ?? DEFAULT_BASE_URL;
+    // Rules are executed policy — refuse a plaintext remote override (see safe-origin.js).
+    this.#baseUrl = safeBaseUrl(baseUrl ?? process.env.PATCHSTACK_PULSE_RULES_URL, DEFAULT_BASE_URL, 'rule endpoint');
     this.#cacheTtl = Number.isFinite(cacheTtl) && cacheTtl > 0 ? cacheTtl : DEFAULT_CACHE_TTL;
     this.#etag = etag ?? null;
     if (!this.#siteUuid) {

src/protect/firewall-log.js

@@ -1,3 +1,4 @@
+import { isSafeOrigin } from './safe-origin.js';
 // Fire-and-forget reporter: Connect runtime → existing connector POST /api/logs/log
 // (same path WordPress uses). Auth: WP-style api_key (`{secret}-{oauth.id}`) →
 // POST /oauth/token (client_credentials) → Bearer JWT on /api/logs/log.
@@ -30,7 +31,16 @@ export function parseApiKey(apiKey) {
 export function resolveApiBase(pulseOrManifestUrl) {
   const fromEnv = typeof process !== 'undefined' ? process.env?.PATCHSTACK_API_BASE : undefined;
   if (typeof fromEnv === 'string' && fromEnv.length > 0) {
-    return fromEnv.replace(/\/$/, '');
+    // The site api_key is exchanged for a token against this origin, so a hostile/injected env value
+    // would be a credential-exfiltration path. Require HTTPS (localhost excepted for local testing);
+    // anything else falls back to the default origin rather than shipping the key off-platform.
+    const candidate = fromEnv.replace(/\/$/, '');
+    if (isSafeOrigin(candidate)) return candidate;
+    // eslint-disable-next-line no-console
+    console.warn(
+      '[patchstack] ignoring PATCHSTACK_API_BASE: block-log reporting requires an https origin ' +
+        '(or localhost). Falling back to the default API origin.',
+    );
   }
   if (typeof pulseOrManifestUrl === 'string' && pulseOrManifestUrl.length > 0) {
     try {

src/protect/rules/source.js

@@ -4,25 +4,26 @@
 // The `store` (see ./store.js) is passed in so a refresh reuses the same tiered cache.
 import { PatchstackRuleClient } from '../engine/index.js';
 import { PulseRuleClient } from '../engine/pulse-client.js';
+import { validateBundle } from './validate.js';
 
 export async function resolveRules(options, store) {
   if (options.siteUuid) {
     const prior = await store.read(); // { bundle, etag } | null
     const client = new PulseRuleClient({ siteUuid: options.siteUuid, baseUrl: options.pulseRulesUrl, etag: prior?.etag });
     const res = await client.getRules();
-    if (res.success && res.notModified && prior?.bundle) return normalizeBundle(prior.bundle);
+    if (res.success && res.notModified && prior?.bundle) return normalizeBundle(prior.bundle, options);
     if (res.success && !res.notModified) {
-      const bundle = normalizeBundle(res);
+      const bundle = normalizeBundle(res, options);
       await store.write({ bundle, etag: res.etag ?? null });
       return bundle;
     }
     if (prior?.bundle) {
       options.onError?.(new Error(`pulse rule fetch failed (${res.error ?? 'no usable response'}); using cached bundle`));
-      return normalizeBundle(prior.bundle);
+      return normalizeBundle(prior.bundle, options);
     }
     if (options.rules) {
       options.onError?.(new Error(`pulse rule fetch failed (${res.error ?? 'no usable response'}); using bundled fallback`));
-      return normalizeBundle(options.rules);
+      return normalizeBundle(options.rules, options);
     }
     options.onError?.(new Error(`pulse rule fetch failed (${res.error ?? 'no usable response'}); no cache — running with no rules`));
     return emptyBundle();
@@ -32,32 +33,55 @@ export async function resolveRules(options, store) {
     const prior = await store.read();
     const client = new PatchstackRuleClient({ token: options.token, baseUrl: options.baseUrl, etag: prior?.etag });
     const res = await client.getRules();
-    if (res.success && res.notModified && prior?.bundle) return normalizeBundle(prior.bundle);
+    if (res.success && res.notModified && prior?.bundle) return normalizeBundle(prior.bundle, options);
     if (res.success && !res.notModified) {
-      const bundle = normalizeBundle(res);
+      const bundle = normalizeBundle(res, options);
       await store.write({ bundle, etag: res.etag ?? null });
       return bundle;
     }
     if (prior?.bundle) {
       options.onError?.(new Error(`rule fetch failed (${res.error ?? 'no usable response'}); using cached bundle`));
-      return normalizeBundle(prior.bundle);
+      return normalizeBundle(prior.bundle, options);
     }
     options.onError?.(new Error(`rule fetch failed (${res.error ?? 'no usable response'}); no cache — running with no rules`));
     return emptyBundle();
   }
 
   if (options.rules) {
-    return normalizeBundle(options.rules);
+    return normalizeBundle(options.rules, options);
   }
 
   return emptyBundle();
 }
 
-export function normalizeBundle(b) {
+// Every rule path (live fetch, cache, bundled fallback) funnels through here, so this is where the
+// delivered policy is VALIDATED before the engine ever executes it: bounded rule count / conditions /
+// nesting / pattern length, known phases + actions. A rule that fails is dropped with a reported reason
+// (`onRuleRejected`) rather than silently kept — an unenforceable rule must never look enforced.
+export function normalizeBundle(b, options = {}) {
   const enforcement = b?.enforcement ?? b?.mode;
-  return {
+  const { bundle: checked, rejected } = validateBundle({
     firewall: Array.isArray(b.firewall) ? b.firewall : [],
     whitelists: Array.isArray(b.whitelists) ? b.whitelists : [],
+  });
+  if (rejected.length > 0) {
+    const report = options.onRuleRejected;
+    if (typeof report === 'function') {
+      for (const r of rejected) {
+        try { report(r); } catch { /* reporting must never break rule loading */ }
+      }
+    } else {
+      const sample = rejected.slice(0, 3).map((r) => `${r.id} (${r.reason})`).join('; ');
+      // eslint-disable-next-line no-console
+      console.warn(
+        `[patchstack] ${rejected.length} delivered rule(s) rejected as invalid/oversized and are NOT enforced: ${sample}` +
+          (rejected.length > 3 ? ', …' : ''),
+      );
+    }
+  }
+  return {
+    firewall: checked.firewall,
+    whitelists: checked.whitelists,
     whitelist_keys: b.whitelist_keys ?? {},
     ...(enforcement === 'block' || enforcement === 'dry-run' ? { enforcement } : {}),
   };

src/protect/rules/validate.js

@@ -0,0 +1,112 @@
+// Rule-bundle validation. Delivered rules are POLICY fetched over the network, and the engine executes
+// them on every request — so an upstream compromise, a schema drift, or a mistake in the corpus could
+// otherwise hand the app unbounded work (a 50k-rule bundle, a 500-deep condition tree, a pathological
+// regex) or silently unenforceable junk.
+//
+// Two principles:
+//   1. REJECT, don't silently skip. A rule that fails validation is dropped WITH a reported reason, so
+//      an unenforceable rule is visible instead of quietly protecting nothing.
+//   2. Bound everything the engine will walk: rule count, conditions per rule, nesting depth, and
+//      pattern length. Caps are deliberately far above any real corpus rule.
+//
+// Fail-open in spirit: validation never throws, and a bundle whose rules are all rejected simply means
+// "no rules" (the app keeps serving) — never a crash.
+
+export const LIMITS = {
+  maxRules: 5000,
+  maxWhitelists: 2000,
+  maxConditionsPerRule: 250,
+  maxNestingDepth: 12,
+  maxRegexLength: 1000,
+  maxValueLength: 8192,
+};
+
+const PHASES = new Set(['request', 'response', 'egress']);
+const ACTIONS = new Set(['block', 'redact', 'encode', 'set-header', 'remove-header', 'harden-cookie']);
+
+/**
+ * Validate a delivered bundle. Returns `{ bundle, rejected }` where `bundle` contains only rules that
+ * passed and `rejected` is `[{ id, reason }]` for everything dropped.
+ * @param {object} bundle
+ * @returns {{ bundle: object, rejected: Array<{id: string, reason: string}> }}
+ */
+export function validateBundle(bundle) {
+  const rejected = [];
+  const inFirewall = Array.isArray(bundle?.firewall) ? bundle.firewall : [];
+  const inWhitelists = Array.isArray(bundle?.whitelists) ? bundle.whitelists : [];
+
+  const firewall = [];
+  for (const rule of inFirewall) {
+    if (firewall.length >= LIMITS.maxRules) {
+      rejected.push({ id: idOf(rule), reason: `bundle exceeds maxRules (${LIMITS.maxRules})` });
+      continue;
+    }
+    const reason = ruleProblem(rule);
+    if (reason) rejected.push({ id: idOf(rule), reason });
+    else firewall.push(rule);
+  }
+
+  const whitelists = [];
+  for (const wl of inWhitelists) {
+    if (whitelists.length >= LIMITS.maxWhitelists) {
+      rejected.push({ id: idOf(wl), reason: `bundle exceeds maxWhitelists (${LIMITS.maxWhitelists})` });
+      continue;
+    }
+    // A whitelist SUPPRESSES rules, so a malformed one is a protection risk, not a detection risk.
+    const reason = conditionsProblem(wl?.rule_v2);
+    if (reason) rejected.push({ id: idOf(wl), reason: `whitelist: ${reason}` });
+    else whitelists.push(wl);
+  }
+
+  return {
+    bundle: { ...bundle, firewall, whitelists },
+    rejected,
+  };
+}
+
+function idOf(rule) {
+  const id = rule?.id ?? rule?.rule_id;
+  return id === undefined || id === null ? '(unidentified)' : String(id);
+}
+
+/** @returns {string|null} a reason the rule must be dropped, or null when it's acceptable. */
+function ruleProblem(rule) {
+  if (!rule || typeof rule !== 'object') return 'not an object';
+  if (rule.phase !== undefined && !PHASES.has(rule.phase)) return `unknown phase "${rule.phase}"`;
+  if (rule.action !== undefined && !ACTIONS.has(rule.action)) return `unknown action "${rule.action}"`;
+  const capOverride = rule.max_bytes;
+  if (capOverride !== undefined && !(Number(capOverride) > 0)) return 'max_bytes must be a positive number';
+  return conditionsProblem(rule.rule_v2);
+}
+
+function conditionsProblem(conditions, depth = 0) {
+  if (!Array.isArray(conditions)) return 'rule_v2 must be an array of conditions';
+  if (conditions.length === 0) return 'rule_v2 is empty (would never match)';
+  if (depth > LIMITS.maxNestingDepth) return `nesting deeper than ${LIMITS.maxNestingDepth}`;
+  if (conditions.length > LIMITS.maxConditionsPerRule) {
+    return `more than ${LIMITS.maxConditionsPerRule} conditions`;
+  }
+  for (const c of conditions) {
+    if (!c || typeof c !== 'object') return 'condition is not an object';
+    if (Array.isArray(c.rules)) {
+      const nested = conditionsProblem(c.rules, depth + 1);
+      if (nested) return nested;
+      continue; // a group carries no match of its own
+    }
+    const m = c.match;
+    if (!m || typeof m !== 'object') return 'condition has no match object';
+    if (typeof m.type !== 'string' || m.type === '') return 'match.type must be a non-empty string';
+    if (m.type === 'regex') {
+      if (typeof m.value !== 'string') return 'regex match.value must be a string';
+      if (m.value.length > LIMITS.maxRegexLength) return `regex longer than ${LIMITS.maxRegexLength} chars`;
+    } else if (typeof m.value === 'string' && m.value.length > LIMITS.maxValueLength) {
+      return `match.value longer than ${LIMITS.maxValueLength} chars`;
+    }
+    if (m.match) {
+      // array_key_value nests a sub-match; count it toward depth so a chain can't be unbounded.
+      const nested = conditionsProblem([{ match: m.match }], depth + 1);
+      if (nested) return nested;
+    }
+  }
+  return null;
+}

src/protect/safe-origin.js

@@ -0,0 +1,33 @@
+// Which origins this guard is willing to talk to. Both of its remote conversations are security
+// sensitive: the RULE endpoint delivers policy the engine then executes on every request (an
+// attacker-controlled endpoint could remove protection wholesale or serve a CPU-expensive ruleset), and
+// the telemetry endpoint receives the site api_key. Env/CI injection is the realistic threat, so a
+// non-default override must be https — with localhost permitted so local development and tests work.
+export function isSafeOrigin(value) {
+  try {
+    const u = new URL(value);
+    if (u.protocol === 'https:') return true;
+    return u.protocol === 'http:' && (u.hostname === 'localhost' || u.hostname === '127.0.0.1' || u.hostname === '[::1]' || u.hostname === '::1');
+  } catch {
+    return false;
+  }
+}
+
+/**
+ * Accept `candidate` only if it is a safe origin; otherwise warn once and fall back to `fallback`.
+ * @param {string|undefined} candidate @param {string} fallback @param {string} label
+ */
+export function safeBaseUrl(candidate, fallback, label) {
+  if (typeof candidate !== 'string' || candidate === '') return fallback;
+  if (isSafeOrigin(candidate)) return candidate;
+  warnOnce(label, `[patchstack] ignoring unsafe ${label} override (${candidate}): must be https (or localhost). Using the default.`);
+  return fallback;
+}
+
+const warned = new Set();
+function warnOnce(key, message) {
+  if (warned.has(key)) return;
+  warned.add(key);
+  // eslint-disable-next-line no-console
+  console.warn(message);
+}

tests/protect/egress-dns.test.ts

@@ -48,7 +48,10 @@ describe('egress DNS-rebinding screen (node:http)', () => {
   it('allows and pins a hostname that resolves to a permitted address', async () => {
     const http = await nodeHttp();
     const server = http.createServer((_req: any, res: any) => res.end('ok'));
-    await new Promise<void>((r) => server.listen(0, '127.0.0.1', r));
+    // Some sandboxed/CI environments refuse to bind a listener (EPERM). That's an environment limit,
+    // not a product failure — skip rather than fail or hang.
+    const bound = await listenOrSkip(server);
+    if (!bound) return;
     const { port } = server.address();
     try {
       // 127.* is permitted by this predicate, so the pinned resolution reaches the local server.
@@ -74,7 +77,10 @@ describe('egress DNS-rebinding screen (node:http)', () => {
   it('does not screen when dnsScreen is disabled (our resolver is never wired in)', async () => {
     const http = await nodeHttp();
     const server = http.createServer((_req: any, res: any) => res.end('ok'));
-    await new Promise<void>((r) => server.listen(0, '127.0.0.1', r));
+    // Some sandboxed/CI environments refuse to bind a listener (EPERM). That's an environment limit,
+    // not a product failure — skip rather than fail or hang.
+    const bound = await listenOrSkip(server);
+    if (!bound) return;
     const { port } = server.address();
     let called = false;
     try {
@@ -102,3 +108,19 @@ describe('egress DNS-rebinding screen (node:http)', () => {
     }
   });
 });
+
+// Bind a loopback listener, returning false when the environment forbids it (EPERM in some sandboxes).
+async function listenOrSkip(server: any): Promise<boolean> {
+  return new Promise((resolve) => {
+    const onError = () => resolve(false);
+    server.once('error', onError);
+    try {
+      server.listen(0, '127.0.0.1', () => {
+        server.removeListener('error', onError);
+        resolve(true);
+      });
+    } catch {
+      resolve(false);
+    }
+  });
+}