diff --git a/src/protect/rules/source.js b/src/protect/rules/source.js index 0215c08..077a4e8 100644 --- a/src/protect/rules/source.js +++ b/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') { diff --git a/src/protect/rules/validate.js b/src/protect/rules/validate.js index 8015a66..5fa505f 100644 --- a/src/protect/rules/validate.js +++ b/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); diff --git a/src/protect/runtime.js b/src/protect/runtime.js index c928645..33c88b9 100644 --- a/src/protect/runtime.js +++ b/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) => { diff --git a/tests/protect/atomic-bundle.test.ts b/tests/protect/atomic-bundle.test.ts new file mode 100644 index 0000000..bfc7da4 --- /dev/null +++ b/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 + }); +});