diff --git a/src/protect/detections.js b/src/protect/detections.js new file mode 100644 index 0000000..32444b7 --- /dev/null +++ b/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>} */ + let queue = []; + /** @type {ReturnType | 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, + }; +} diff --git a/src/protect/protect.d.ts b/src/protect/protect.d.ts index ef2f597..d91d85a 100644 --- a/src/protect/protect.d.ts +++ b/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). */ diff --git a/src/protect/runtime.js b/src/protect/runtime.js index 74b5ca6..b1e062c 100644 --- a/src/protect/runtime.js +++ b/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; diff --git a/tests/protect/detections.test.ts b/tests/protect/detections.test.ts new file mode 100644 index 0000000..53cfb61 --- /dev/null +++ b/tests/protect/detections.test.ts @@ -0,0 +1,214 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { createDetectionReporter, routeOf, ruleParameters } from '../../src/protect/detections.js'; +import { createProtection } from '../../src/protect/runtime.js'; + +/** + * Reporting what a rule WOULD have stopped. + * + * The block log answers "what did we stop", in the WordPress-compatible shape. This channel answers the + * question nothing could answer before: a rule carrying `enforcement: dry-run` saw traffic it would have + * blocked. Without it, a rule that is quietly wrong looks exactly like one that is protecting. + * + * Most of this file is about what the payload must NOT contain. A counting channel that carries matched + * values is a store of other people's data, and the difference between the two is one careless field. + */ + +/** `flush()` posts after awaiting the auth header, so a caller sees the request one tick later. */ +const drain = () => new Promise((resolve) => setTimeout(resolve, 0)); + +/** Everything the payload is allowed to carry, and nothing else. */ +const ALLOWED_KEYS = ['rule_id', 'route', 'parameters', 'phase', 'enforced', 'rules_etag', 'detected_at']; + +const pinnedRule = { + id: 'pulse-1', + rule_v2: [ + { parameter: 'server.REQUEST_URI', inclusive: true, match: { type: 'contains', value: '/api/preview' } }, + { parameter: 'get.url', inclusive: true, match: { type: 'internal_host' } }, + ], +}; + +function reporterWith(overrides: Record = {}) { + const posts: Array<{ url: string; body: any }> = []; + const fetchImpl = vi.fn(async (url: string, init?: RequestInit) => { + posts.push({ url, body: JSON.parse(String(init?.body ?? '{}')) }); + + return new Response('{}', { status: 202 }); + }); + const reporter = createDetectionReporter({ + siteUuid: 'site-1', + baseUrl: 'https://x.test/monitor/pulse', + rulesEtag: '"v7"', + fetchImpl: fetchImpl as unknown as typeof fetch, + ...overrides, + }); + + return { reporter, posts, fetchImpl }; +} + +afterEach(() => vi.restoreAllMocks()); + +describe('the detection payload', () => { + it('carries what accounting needs and nothing else', async () => { + const { reporter, posts } = reporterWith(); + + reporter.record({ rule: pinnedRule, phase: 'request', mode: 'dry-run', path: '/api/preview?url=x' }); + reporter.flush(); + await drain(); + + const [event] = posts[0].body.detections; + expect(Object.keys(event).sort()).toEqual([...ALLOWED_KEYS].sort()); + expect(event).toMatchObject({ + rule_id: 'pulse-1', + route: '/api/preview', + parameters: ['server.REQUEST_URI', 'get.url'], + phase: 'request', + // The point of the channel: this rule did not block, and that is the interesting case. + enforced: false, + rules_etag: '"v7"', + }); + expect(typeof event.detected_at).toBe('string'); + }); + + it('never puts a matched value, a query string, a body or a header on the wire', async () => { + // The load-bearing test, and deliberately a scan of the serialized payload rather than of the object + // we built: a field added later — `message`, `value`, `headers` — would pass every assertion above + // and fail here, which is the direction this needs to fail in. + const { reporter, posts } = reporterWith(); + + reporter.record({ + rule: pinnedRule, + phase: 'request', + mode: 'block', + // Everything a real detection has hanging off it. None of it may travel. + path: '/api/preview?url=http://169.254.169.254/latest/meta-data/&token=SUPER_SECRET', + method: 'POST', + ip: '203.0.113.9', + userAgent: 'curl/8.0', + message: 'Blocked by Patchstack WAF rule: internal host in get.url', + value: 'http://169.254.169.254/latest/meta-data/', + } as never); + reporter.flush(); + await drain(); + + const wire = JSON.stringify(posts[0].body); + for (const forbidden of ['SUPER_SECRET', '169.254.169.254', 'meta-data', '203.0.113.9', 'curl/8.0', 'Blocked by']) { + expect(wire, `${forbidden} must not reach the reporting endpoint`).not.toContain(forbidden); + } + // And the route survived, so the scan above is not passing because nothing was sent. + expect(wire).toContain('/api/preview'); + }); + + it('reports the enforcement state, not the site mode', async () => { + const { reporter, posts } = reporterWith(); + + reporter.record({ rule: pinnedRule, mode: 'block', path: '/a' }); + reporter.record({ rule: pinnedRule, mode: 'dry-run', path: '/a' }); + reporter.flush(); + await drain(); + + expect(posts[0].body.detections.map((d: any) => d.enforced)).toEqual([true, false]); + }); + + it('drops a detection without a rule id rather than sending an anonymous row', () => { + const { reporter, fetchImpl } = reporterWith(); + + reporter.record({ rule: {}, path: '/a' } as never); + reporter.record({ path: '/a' } as never); + reporter.flush(); + + expect(fetchImpl).not.toHaveBeenCalled(); + }); +}); + +describe('bounds and failure', () => { + it('bounds the queue and says how much it dropped', async () => { + // A detection storm must cost memory it cannot grow out of. The drop count travels WITH the batch: + // a consumer computing a rate from these needs to know its denominator is short, and inferring that + // from a gap is not something anyone does. + const { reporter, posts } = reporterWith({ maxQueue: 3 }); + + for (let i = 0; i < 10; i++) reporter.record({ rule: pinnedRule, mode: 'dry-run', path: `/a/${i}` }); + reporter.flush(); + await drain(); + + expect(posts[0].body.detections.length).toBe(3); + expect(posts[0].body.dropped).toBe(7); + // The survivors are the newest — a storm's tail is what a reviewer wants, not its head. + expect(posts[0].body.detections.map((d: any) => d.route)).toEqual(['/a/7', '/a/8', '/a/9']); + }); + + it('is silent and harmless when the endpoint rejects or throws', async () => { + const rejecting = vi.fn(async () => { + throw new Error('network down'); + }); + const reporter = createDetectionReporter({ + siteUuid: 'site-1', + baseUrl: 'https://x.test/monitor/pulse', + fetchImpl: rejecting as unknown as typeof fetch, + }); + + reporter.record({ rule: pinnedRule, mode: 'block', path: '/a' }); + expect(() => reporter.flush()).not.toThrow(); + await Promise.resolve(); + }); + + it('is a no-op without a site to report against', () => { + const previous = process.env.PATCHSTACK_SITE_UUID; + delete process.env.PATCHSTACK_SITE_UUID; + try { + const reporter = createDetectionReporter({ baseUrl: 'https://x.test/monitor/pulse' }); + expect(() => reporter.record({ rule: pinnedRule, path: '/a' })).not.toThrow(); + expect(() => reporter.flush()).not.toThrow(); + } finally { + if (previous !== undefined) process.env.PATCHSTACK_SITE_UUID = previous; + } + }); +}); + +describe('the helpers', () => { + it('keeps the path and drops the query', () => { + expect(routeOf('/api/preview?url=secret')).toBe('/api/preview'); + expect(routeOf('/api/preview#frag')).toBe('/api/preview'); + expect(routeOf('/api/preview')).toBe('/api/preview'); + expect(routeOf('')).toBeNull(); + expect(routeOf(undefined)).toBeNull(); + }); + + it('collects the parameters a rule reads, including nested ones', () => { + // `rules` is a grouping wrapper, not a parameter source — reporting it would name a thing the engine + // does not read. + expect(ruleParameters({ + rule_v2: [ + { parameter: 'raw', match: { type: 'contains', value: '__proto__' } }, + { parameter: 'rules', rules: [{ parameter: 'get.q', inclusive: true, match: { type: 'contains', value: 'x' } }] }, + ], + })).toEqual(['raw', 'get.q']); + + expect(ruleParameters(undefined)).toEqual([]); + expect(ruleParameters({ rule_v2: 'nonsense' })).toEqual([]); + }); +}); + +describe('wiring', () => { + it('reports nothing unless it is switched on', async () => { + // Opt-in on purpose: enabling it adds an outbound POST to every guard with a site UUID, which is a + // change in what an installed app does on the network. + const fetchMock = vi.fn(async () => new Response(JSON.stringify({ firewall: [], whitelists: [], enforcement: 'dry-run' }), { + status: 200, headers: { 'Content-Type': 'application/json' }, + })); + vi.stubGlobal('fetch', fetchMock); + + const p: any = await createProtection({ + siteUuid: 'site-1', + pulseRulesUrl: 'https://x.test/monitor/pulse', + rules: { firewall: [{ ...pinnedRule, rule_v2: [{ parameter: 'get.q', match: { type: 'contains', value: 'boom' } }] }] }, + mode: 'dry-run', + }); + await p.fetchGuard()(new Request('https://app.test/api/x?q=boom')); + + const posted = fetchMock.mock.calls.filter(([url]) => String(url).includes('/detections/')); + expect(posted.length, 'no detection report without reportDetections: true').toBe(0); + + p.stopRefresh?.(); + }); +});