diff --git a/AGENT-INSTALL.md b/AGENT-INSTALL.md index 0134a45..d7c0654 100644 --- a/AGENT-INSTALL.md +++ b/AGENT-INSTALL.md @@ -138,13 +138,15 @@ would have stopped while it is still in dry-run. Two separate paths, with differ `reportFirewallLog: false` in `createProtection`. - **Every rule that matched** goes to `monitor/pulse/detections/` — including matches that blocked, which are reported on both paths. This is **off unless you pass `reportDetections: true`** to - `createProtection`; the scaffolded guard does not pass it. It also requires a provisioned site UUID and - is disabled by `PATCHSTACK_TELEMETRY=off`. It exists because a rule carrying `dry-run` blocks nothing, - so without it nothing distinguishes a rule that is protecting from one that is quietly wrong. + `createProtection`; the scaffolded guard does not pass it. It also requires a provisioned site UUID, a + resolvable credential, and is disabled by `PATCHSTACK_TELEMETRY=off`. It exists because a rule carrying + `dry-run` blocks nothing, so without it nothing distinguishes a rule that is protecting from one that is + quietly wrong. What a detection report contains, per matched rule: the rule id, the request path **with any query string removed**, the parameter names that rule reads (from the rule's own definition), which phase matched, -whether it was enforced, the identifier of the rule bundle in use, and a timestamp. Each batch also +whether it was enforced, the identifier of the rule bundle in use, the revision of the rule itself when the +bundle carried one, and a timestamp. Each batch also carries a count of reports dropped when traffic outran the flush, so a partial sample is not read as a complete one. @@ -159,6 +161,16 @@ and not the value of any header, cookie or query-string parameter — including named above. Reports are batched, capped in memory, and dropped rather than retried if Patchstack cannot be reached — a reporting failure never delays or fails a request. +The endpoint needs a credential, so `reportDetections: true` with none resolved starts nothing: the guard +warns once at boot and `protection.detectionReporting` reads `unavailable-no-credential` instead of `on`. +When reporting is on, `protection.detectionHealth()` returns local counts — detections attempted, +acknowledged, refused or unreachable, dropped for queue pressure — and the time of the last +acknowledgement. Those counts stay in your process; nothing extra is sent to report them. + +`protection.stop()` stops everything the guard has running in the background — the rule-refresh loop, the +block-log reporter, the detection reporter — and flushes what is buffered. `protection.stopRefresh()` is +the same method under its older name. Call it on shutdown; it is safe to call twice. + Two more endpoints the package can call, for completeness: - `GET monitor/widget/settings/` — how `status` tells "this site was deleted on diff --git a/src/protect/detections.js b/src/protect/detections.js index 32444b7..cb6f73a 100644 --- a/src/protect/detections.js +++ b/src/protect/detections.js @@ -15,8 +15,8 @@ import { isSafeOrigin } from './safe-origin.js'; * ## 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. + * the bundle identity, and the rule's own revision where the bundle carried one. 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 @@ -61,6 +61,24 @@ export function ruleParameters(rule) { return [...out]; } +/** + * The rule's own revision, from the served rule. + * + * Read off the delivered rule rather than derived: whoever served it knows what document this is, and a + * value computed here would be this client's opinion of it. Accepts a string or a number, because the two + * kinds of rule that carry one number their revisions differently. + * + * @param {any} rule + * @returns {string | null} + */ +export function revisionOf(rule) { + const revision = rule?.source_revision; + if (typeof revision === 'string' && revision !== '') return revision; + if (typeof revision === 'number' && Number.isFinite(revision)) return String(revision); + + return null; +} + /** * The request path with the query string removed. * @@ -92,7 +110,11 @@ 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 }; + // It answers the whole interface, so a caller never has to know which kind it holds. + return { + record() {}, flush() {}, stop() {}, setRulesEtag() {}, dropped: () => 0, + health: () => ({ sent: 0, delivered: 0, failed: 0, dropped: 0, lastDeliveredAt: null }), + }; } const configured = opts.baseUrl ?? process.env?.PATCHSTACK_PULSE_RULES_URL; @@ -103,6 +125,22 @@ export function createDetectionReporter(opts) { 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; + // The bundle identity stamped onto each event. MUTABLE, because the guard's rules are: a refresh + // hot-swaps the ruleset in place, and a reporter holding the boot-time value would attribute a hit + // produced by the new bundle to the old one — sending a reviewer to a rule document that is not the + // one that fired. Updated by the runtime only after an accepted swap (see `setRulesEtag`). + let rulesEtag = opts.rulesEtag ?? null; + + // Delivery health. The capability declaration says a guard INTENDS to report; these say whether + // anything arrived. Counts and one timestamp only — a clean app and a broken delivery path are + // otherwise indistinguishable, and distinguishing them needs no request data at all. + let sent = 0; + let delivered = 0; + let failed = 0; + let droppedTotal = 0; + /** @type {string | null} */ + let lastDeliveredAt = null; + /** @type {Array>} */ let queue = []; /** @type {ReturnType | null} */ @@ -123,6 +161,8 @@ export function createDetectionReporter(opts) { // would make a truncated sample look like a complete one. const droppedWith = dropped; dropped = 0; + droppedTotal += droppedWith; + sent += batch.length; void (async () => { try { @@ -132,17 +172,26 @@ export function createDetectionReporter(opts) { '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. + // Same credential path as the rules fetch. The detections endpoint is site-addressed and + // requires a verified, site-bound token, so a batch sent without one is refused — which is + // why the runtime does not build a reporter when no credential resolves, rather than + // posting into a 401. ...(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(() => {}); + // Fail-open and no retry: a rejected or unreachable endpoint must not disturb the app, and a + // retry loop over a refusing endpoint is worse than the lost batch. The outcome is counted, so + // that a delivery path which refuses everything is distinguishable from an app where no rule + // fired — both are silence at the server otherwise. + if (res && res.ok) { + delivered += batch.length; + lastDeliveredAt = new Date().toISOString(); + } else { + failed += batch.length; + } } catch { - /* ignore */ + failed += batch.length; } })(); }; @@ -174,7 +223,12 @@ export function createDetectionReporter(opts) { // 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, + rules_etag: rulesEtag, + // The revision of THIS rule, as the bundle delivered it. The bundle identity above answers "which + // bundle", which changes whenever anything in it changes — so it cannot say whether the counts for + // one rule describe the document that rule has now. Passed through untouched, and null when the + // bundle carried none. + rule_revision: revisionOf(detection.rule), detected_at: new Date().toISOString(), }); @@ -190,6 +244,23 @@ export function createDetectionReporter(opts) { stopped = true; flush(); }, + /** + * Point later events at the bundle now running. Called after an ACCEPTED swap only — a rejected + * or failed refresh keeps the previous rules, so it must keep the previous identity too. + * + * Already-queued events are not rewritten: the stamp is taken when an event is recorded, which is + * the only moment the two are known to agree. + * + * @param {string | null | undefined} next + */ + setRulesEtag(next) { + rulesEtag = next ?? null; + }, dropped: () => dropped, + /** + * Delivery health, counted in events: attempted, acknowledged, refused or unreachable, and dropped + * for queue pressure — plus when a batch was last acknowledged. No request data of any kind. + */ + health: () => ({ sent, delivered, failed, dropped: droppedTotal + dropped, lastDeliveredAt }), }; } diff --git a/src/protect/engine/pulse-client.js b/src/protect/engine/pulse-client.js index dc676ec..6374916 100644 --- a/src/protect/engine/pulse-client.js +++ b/src/protect/engine/pulse-client.js @@ -7,8 +7,9 @@ const DEFAULT_CACHE_TTL = 300_000; // revalidate on the same tick (spreads load / avoids a thundering herd against the rules API). const JITTER_FRACTION = 0.1; -// Per-site rules client for Pulse (npm/JS) apps. Public endpoint — the site UUID is the only -// credential, passed in the path. Fail-open: any error returns success:false + empty rules so +// Per-site rules client for Pulse (npm/JS) apps. The site UUID addresses the site in the path; it is +// not a credential — the endpoint requires a verified token bound to that same site, so a fetch without +// one is refused. Fail-open regardless: any error returns success:false + empty rules so // createProtection falls back to the disk cache or the bundled rules. // // Conditional fetch: pass a prior `etag` (persisted with the last cached bundle) and the client @@ -61,24 +62,22 @@ export class PulseRuleClient { } const url = `${this.#baseUrl}/rules/${encodeURIComponent(this.#siteUuid)}`; try { - // Unauthenticated when no credential resolved, or when the exchange - // fails — the server still accepts the UUID, and protection must never - // hinge on getting a token. + // Sent without an `Authorization` header when no credential resolved or the exchange failed. + // The server refuses that, and the refusal is handled the same way as any other failure: fall + // back to cached or bundled rules. Attempting it anyway is deliberate — protection must never + // hinge on the token path, and the runtime warns at boot when no credential resolves. const auth = await pulseAuthHeader( { pulseAuth: this.#pulseAuth, endpoint: this.#baseUrl, timeoutMs: this.#timeoutMs }, fetch, ); const headers = { Accept: 'application/json', ...auth }; - // Claimed only on an authenticated request. The rules endpoint still accepts a bare UUID, so on that - // path this header would be an assertion anyone holding the UUID could make — and it asserts the - // reassuring thing: that reporting is on. A dashboard would then say a site is covered because a - // stranger said so. + // Claimed only on an authenticated request. Fetching rules must never hinge on getting a token + // (protection comes first), but CLAIMING a capability may: an unauthenticated request is one whose + // statements about this site carry no weight, and this header asserts the reassuring thing — that + // reporting is on. // - // Fetching rules must never hinge on getting a token (protection comes first), but CLAIMING a - // capability may: an unauthenticated request is one whose statements about this site carry no weight. - // This check only removes the ACCIDENTAL case. The forgeable one is not the client's to prevent, so - // anything acting on this header has to require a verified token itself before believing it — a - // client-side gate is a courtesy, never the guarantee. + // A courtesy, never the guarantee: a client-side gate only removes the accidental case. Anything + // acting on this header has to require a verified token itself before believing it. if (this.#reportsDetections && typeof auth.Authorization === 'string') { headers['X-Patchstack-Detections'] = 'enabled'; } diff --git a/src/protect/protect.d.ts b/src/protect/protect.d.ts index d91d85a..ed7b9eb 100644 --- a/src/protect/protect.d.ts +++ b/src/protect/protect.d.ts @@ -25,13 +25,31 @@ export interface Protection { node(options?: { maxBodyBytes?: number; screenResponses?: boolean }): (req: unknown, res: unknown, next: () => void) => void; /** Present when `egress: true` — restores the original global fetch. */ uninstallEgress?: () => void; - /** Present with a live source — re-fetch + hot-swap the rules once (used by the loop + push). */ - refresh?: () => Promise; + /** Present with a live source — re-fetch + hot-swap the rules once (used by the loop + push). + * Resolves with the outcome of the attempt: `ok: false` means the rules in force came from the + * cache or the bundled fallback, not from the source. It does not reject on a source failure. */ + refresh?: () => Promise<{ ok: boolean; reason?: string }>; /** Present with a live source — a fetch handler that runs `refresh()` when the request carries * the configured refresh secret (a push/zero-day trigger). No secret set → the handler 404s. */ refreshHandler?: () => (request: Request) => Promise; - /** Present when `refreshMs > 0` — stops the live rule-refresh loop. */ - stopRefresh?: () => void; + /** Stops everything with a timer or a buffer behind it: the refresh loop, the block log, the + * detection reporter (flushing what it holds). Always present, and safe to call twice. */ + stop: () => void; + /** Alias of `stop`, under the name callers already have. */ + stopRefresh: () => void; + /** Whether detection reporting is running, requested but undeliverable, or not requested. + * `unavailable-no-credential` means `reportDetections` was set but no credential resolved, so + * nothing is being sent. */ + detectionReporting: "on" | "off" | "unavailable-no-credential"; + /** Present when detection reporting is on — delivery counts (in events) and the last acknowledgement. + * Carries no request data. */ + detectionHealth?: () => { + sent: number; + delivered: number; + failed: number; + dropped: number; + lastDeliveredAt: string | null; + }; } export interface CreateProtectionOptions { @@ -81,6 +99,8 @@ export interface CreateProtectionOptions { * 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. + * Needs a resolvable API credential: the endpoint requires a verified, site-bound token, so with no + * credential no reporter is created and `detectionReporting` reads `unavailable-no-credential`. */ reportDetections?: boolean; /** How long to buffer detections before posting a batch. Default 5000ms. */ diff --git a/src/protect/rules/refresh.js b/src/protect/rules/refresh.js index d784a3a..7a7156a 100644 --- a/src/protect/rules/refresh.js +++ b/src/protect/rules/refresh.js @@ -3,7 +3,10 @@ // caller-supplied `tick` (which re-fetches + hot-swaps the engines): // - startRefresh: a self-scheduling poll LOOP (reschedules after each tick settles, so runs never // overlap), with ±jitter (avoid a thundering herd) and exponential backoff on consecutive -// failures. `unref`'d — it never keeps the process alive. +// failures. `unref`'d — it never keeps the process alive. A tick counts as failed when it throws +// OR when it reports `{ ok: false }`: the rule resolver absorbs an API or network failure into +// usable fallback rules, so a thrown error is not the only shape an outage takes, and a poller +// that waits for one keeps the whole fleet knocking at the normal interval while it lasts. // - makeRefreshHandler: a PUSH endpoint — an authenticated fetch handler the platform/SaaS hits // for an immediate refresh (zero-day fast lane) instead of waiting for the next poll. @@ -29,8 +32,9 @@ export function startRefresh(tick, { refreshMs, onError } = {}) { const run = async () => { if (stopped) return; try { - await tick(); - failures = 0; + const status = await tick(); + if (status && status.ok === false) failures++; + else failures = 0; } catch (err) { failures++; notify(onError, err, 'onError'); @@ -60,7 +64,10 @@ export function makeRefreshHandler(tick, secret) { if (provided !== secret) return new Response('forbidden', { status: 403 }); let refreshed = true; try { - await tick(); + // `{ ok: false }` means the tick ran but the rules did not come from the source, which is not a + // refresh — the caller pushed because it had something to deliver, and it did not arrive. + const status = await tick(); + if (status && status.ok === false) refreshed = false; } catch { refreshed = false; // fail-open: report the outcome, never throw } diff --git a/src/protect/rules/source.js b/src/protect/rules/source.js index 7d69980..3314dfa 100644 --- a/src/protect/rules/source.js +++ b/src/protect/rules/source.js @@ -2,6 +2,11 @@ // UUID (Pulse) or token — fetch it (conditional/If-None-Match via the persisted etag), and fall // back through last-known-good → bundled → empty. Fail-open: a fetch/parse error never throws. // The `store` (see ./store.js) is passed in so a refresh reuses the same tiered cache. +// +// Every returned bundle carries `source: { ok, reason? }` — whether the RULES came from the source or +// from a fallback. Absorbing a failure into usable rules is right for protection and insufficient for a +// caller that has its own decision to make: a poller reading only thrown errors treats an outage as a +// healthy poll. The rules answer "what do I enforce"; `source` answers "are these current". import { PatchstackRuleClient } from '../engine/index.js'; import { PulseRuleClient } from '../engine/pulse-client.js'; import { validateBundle } from './validate.js'; @@ -36,6 +41,11 @@ function reportRejections(rejected, options, label) { ), 'onError'); } +/** A bundle plus the outcome of the attempt that produced it. `source` is never written to the store. */ +function fromSource(bundle, reason) { + return reason === undefined ? { ...bundle, source: { ok: true } } : { ...bundle, source: { ok: false, reason } }; +} + 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 @@ -45,61 +55,66 @@ export async function resolveRules(options, store, ctx = {}) { const prior = await store.read(); // { bundle, etag } | null const client = new PulseRuleClient({ siteUuid: options.siteUuid, baseUrl: options.pulseRulesUrl, etag: prior?.etag, timeoutMs, pulseAuth: ctx.pulseAuth, reportsDetections: options.reportDetections === true }); const res = await client.getRules(); - if (res.success && res.notModified && prior?.bundle) return normalizeBundle(prior.bundle, options); + if (res.success && res.notModified && prior?.bundle) return fromSource(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(); + // Reached the source and refused what it sent. Not ok: the running rules are not the delivered + // ones, and asking again at the normal interval re-downloads the same rejected bundle. + if (prior?.bundle) return fromSource(normalizeBundle(prior.bundle, options), 'update rejected'); + if (options.rules) return fromSource(normalizeBundle(options.rules, options), 'update rejected'); + return fromSource(emptyBundle(), 'update rejected'); } const bundle = normalizeBundle(res, options); await store.write({ bundle, etag: res.etag ?? null }); - return bundle; + return fromSource(bundle); } if (prior?.bundle) { notify(options.onError, new Error(`pulse rule fetch failed (${res.error ?? 'no usable response'}); using cached bundle`), 'onError'); - return normalizeBundle(prior.bundle, options); + return fromSource(normalizeBundle(prior.bundle, options), res.error ?? 'no usable response'); } if (options.rules) { notify(options.onError, new Error(`pulse rule fetch failed (${res.error ?? 'no usable response'}); using bundled fallback`), 'onError'); - return normalizeBundle(options.rules, options); + return fromSource(normalizeBundle(options.rules, options), res.error ?? 'no usable response'); } notify(options.onError, new Error(`pulse rule fetch failed (${res.error ?? 'no usable response'}); no cache — running with no rules`), 'onError'); - return emptyBundle(); + return fromSource(emptyBundle(), res.error ?? 'no usable response'); } if (options.token) { const prior = await store.read(); const client = new PatchstackRuleClient({ token: options.token, baseUrl: options.baseUrl, etag: prior?.etag, timeoutMs }); const res = await client.getRules(); - if (res.success && res.notModified && prior?.bundle) return normalizeBundle(prior.bundle, options); + if (res.success && res.notModified && prior?.bundle) return fromSource(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(); + // Reached the source and refused what it sent. Not ok: the running rules are not the delivered + // ones, and asking again at the normal interval re-downloads the same rejected bundle. + if (prior?.bundle) return fromSource(normalizeBundle(prior.bundle, options), 'update rejected'); + if (options.rules) return fromSource(normalizeBundle(options.rules, options), 'update rejected'); + return fromSource(emptyBundle(), 'update rejected'); } const bundle = normalizeBundle(res, options); await store.write({ bundle, etag: res.etag ?? null }); - return bundle; + return fromSource(bundle); } if (prior?.bundle) { notify(options.onError, new Error(`rule fetch failed (${res.error ?? 'no usable response'}); using cached bundle`), 'onError'); - return normalizeBundle(prior.bundle, options); + return fromSource(normalizeBundle(prior.bundle, options), res.error ?? 'no usable response'); } notify(options.onError, new Error(`rule fetch failed (${res.error ?? 'no usable response'}); no cache — running with no rules`), 'onError'); - return emptyBundle(); + return fromSource(emptyBundle(), res.error ?? 'no usable response'); } + // No live source configured, so the bundle IS the source and cannot be behind one. if (options.rules) { - return normalizeBundle(options.rules, options); + return fromSource(normalizeBundle(options.rules, options)); } - return emptyBundle(); + return fromSource(emptyBundle()); } // Every rule path (live fetch, cache, bundled fallback) funnels through here, so this is where the diff --git a/src/protect/runtime.js b/src/protect/runtime.js index 3a4f236..c5a8d6d 100644 --- a/src/protect/runtime.js +++ b/src/protect/runtime.js @@ -123,11 +123,12 @@ export async function createProtection(options = {}) { // runtimes this guard is built for do not all have one: on a Worker or an edge function the file is // absent and only `PATCHSTACK_PULSE_AUTH` / `PATCHSTACK_API_KEY` can carry the credential. // - // Unauthenticated rule fetches are accepted today, so the failure is currently invisible — and it - // stays invisible once they are not, because a rejected fetch fails open onto the cached or bundled - // bundle. The guard then screens every request, reports healthy, and never receives another rule. - // That silence is the whole problem: an app protected by rules frozen at install time looks exactly - // like an app protected by current ones. + // Every site-addressed Pulse endpoint requires a verified, site-bound credential; only a first-time + // provisioning call is anonymous. So a missing credential is not a future problem — the rules fetch is + // refused now. And the refusal is invisible, because a failed fetch fails open onto the cached or + // bundled bundle: the guard then screens every request, reports healthy, and never receives another + // rule. That silence is the whole problem — an app protected by rules frozen at install time looks + // exactly like an app protected by current ones. // // A warning, not a throw. Booting is protection; refusing to boot over a missing credential would // trade a stale rule set for no rule set at all. @@ -135,7 +136,7 @@ export async function createProtection(options = {}) { const message = 'Patchstack: no API credential resolved for site ' + options.siteUuid + - '. Rule updates may be rejected and this guard would keep running on its cached rules. ' + + '. Rule updates will be rejected and this guard would keep running on its cached rules. ' + 'Set PATCHSTACK_API_KEY (or pass { pulseAuth }) — required on runtimes without a filesystem.'; notify(onError, new Error(message), 'onError'); console.warn(message); @@ -146,17 +147,35 @@ export async function createProtection(options = {}) { // 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. + // + // And it needs a credential. The detections endpoint is site-addressed and site-bound-token-only, so a + // reporter built without one queues events, posts them, and is refused — spending an outbound request + // per batch to accomplish nothing, while `reportDetections: true` in the config says reporting is on. + // Refusing to build it is the honest outcome; `protection.detectionReporting` says which it is. + let detectionReporting = 'off'; 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, - }); + if (!pulseAuth) { + detectionReporting = 'unavailable-no-credential'; + const message = + 'Patchstack: detection reporting is enabled for site ' + + options.siteUuid + + ' but no API credential resolved, so no report could be delivered. Reporting is off.'; + notify(onError, new Error(message), 'onError'); + console.warn(message); + } else { + detectionReporting = 'on'; + 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. Kept current across refreshes — + // see the refresh tick below. + 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. @@ -660,6 +679,17 @@ export async function createProtection(options = {}) { const next = await resolveRules(options, store, { timeoutMs: options.refreshTimeoutMs, pulseAuth }); mode = resolveMode(options, next); applyBundle(next); + // After the swap, and only after it: later detections belong to the bundle now running. A refresh + // that fell back to the cached or bundled ruleset kept the previous rules, and `store.read()` then + // still holds the previous identity — which is exactly the answer that stays true. + if (detections) detections.setRulesEtag((await store.read())?.etag ?? null); + + // The tick's own outcome, separate from the guard's. `resolveRules` deliberately absorbs an API or + // network failure and returns usable rules, which is right for protection and wrong for a poller: + // a scheduler that only counts THROWN errors reads a fleet-wide outage as a healthy poll and keeps + // knocking at the normal interval. Reported, not thrown — a caller's manual `refresh()` must not + // start failing because the platform is down and the cached rules held. + return next.source ?? { ok: true }; }; if (live) { @@ -670,19 +700,26 @@ export async function createProtection(options = {}) { protection.refreshHandler = () => makeRefreshHandler(runRefreshTick, refreshSecret); } - if (options.refreshMs > 0 && live) { - const loop = startRefresh(runRefreshTick, { refreshMs: options.refreshMs, onError }); - protection.stopRefresh = () => { - loop.stop(); - firewallLog?.stop(); - detections?.stop(); - }; - } else if (firewallLog) { - protection.stopRefresh = () => { - firewallLog.stop(); - detections?.stop(); - }; - } + const loop = options.refreshMs > 0 && live + ? startRefresh(runRefreshTick, { refreshMs: options.refreshMs, onError }) + : null; + + // One method, always present, that reaches everything holding a timer or a buffer: the refresh loop, + // the block log, the detection reporter. Always present because a lifecycle method that exists only + // for some configurations is one a caller cannot rely on — and each of these components can be the + // only one installed, so any of them can be the one left running. + protection.stop = () => { + loop?.stop(); + firewallLog?.stop(); + detections?.stop(); + }; + // The name callers already have, kept as an alias for it. + protection.stopRefresh = protection.stop; + // Which of the three states reporting is in: requested and running, requested but undeliverable, or + // not requested. A boolean would collapse the middle one into "off", which is the reassuring reading. + protection.detectionReporting = detectionReporting; + // Delivery health, when there is a reporter: what was attempted, acknowledged, refused, and dropped. + if (detections) protection.detectionHealth = () => detections.health(); return protection; } @@ -730,8 +767,9 @@ async function resolveApiKey(options) { * order and the same edge-runtime caution as resolveApiKey, and falls back to * it so guards installed before pulseAuth existed keep authenticating. * - * Returning undefined is fine: the rules fetch then goes out unauthenticated, - * which the server still accepts. + * Returning undefined does not fail the boot — protection still runs on the cached or bundled rules — + * but the fetch then goes out unauthenticated and the platform refuses it, so the guard stops receiving + * rules. That is why the caller warns about it at boot rather than treating it as a normal state. */ async function resolvePulseAuth(options) { if (typeof options?.pulseAuth === 'string' && options.pulseAuth.length > 0) return options.pulseAuth; diff --git a/src/pulse-token.ts b/src/pulse-token.ts index a03210b..d0e73dd 100644 --- a/src/pulse-token.ts +++ b/src/pulse-token.ts @@ -46,7 +46,9 @@ export function clearPulseToken(): void { * * Returns null whenever a token cannot be obtained — no credential, a rejected * exchange, a network failure. Callers then send the request unauthenticated, - * which the server still accepts while it runs dual-accept. + * and every site-addressed Pulse endpoint refuses it: only a first-time + * provisioning call is anonymous. Returning null rather than throwing keeps that + * refusal on the caller's own error path, where it can fall back or report. */ export async function getPulseToken( config: Config, diff --git a/tests/protect/detection-payload-contract.test.ts b/tests/protect/detection-payload-contract.test.ts index 304f916..3d1139e 100644 --- a/tests/protect/detection-payload-contract.test.ts +++ b/tests/protect/detection-payload-contract.test.ts @@ -35,6 +35,7 @@ const FIELD_DISCLOSURE: Record = { phase: /which phase matched/i, enforced: /whether it was enforced/i, rules_etag: /identifier of the rule bundle/i, + rule_revision: /revision of the rule/i, detected_at: /timestamp/i, }; diff --git a/tests/protect/detections.test.ts b/tests/protect/detections.test.ts index 91f5f41..e9dc6e4 100644 --- a/tests/protect/detections.test.ts +++ b/tests/protect/detections.test.ts @@ -17,7 +17,7 @@ import { createProtection } from '../../src/protect/runtime.js'; 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 ALLOWED_KEYS = ['rule_id', 'route', 'parameters', 'phase', 'enforced', 'rules_etag', 'rule_revision', 'detected_at']; const pinnedRule = { id: 'pulse-1', @@ -280,6 +280,11 @@ describe('the wiring actually runs', () => { const fetchMock = vi.fn(async (url: string) => { posted.push(String(url)); if (String(url).includes('/detections/')) return new Response('{}', { status: 202 }); + if (String(url).includes('/token')) { + return new Response(JSON.stringify({ access_token: 'jwt-abc', expires_in: 3600 }), { + status: 200, headers: { 'Content-Type': 'application/json' }, + }); + } return new Response( JSON.stringify({ @@ -294,6 +299,7 @@ describe('the wiring actually runs', () => { const p: any = await createProtection({ siteUuid: 'site-1', pulseRulesUrl: 'https://x.test/monitor/pulse', + pulseAuth: 'the-secret-40-chars-long-ish-value-here-987', reportDetections: true, detectionFlushMs: 1, }); @@ -308,10 +314,9 @@ describe('the wiring actually runs', () => { describe('the capability claim is only made when it carries weight', () => { it('stays silent on an unauthenticated rules fetch', async () => { - // The rules endpoint still accepts a bare UUID, so on that path this header is an assertion anyone - // holding the UUID could make — and it asserts the reassuring thing, that reporting is on. A dashboard - // would then report a site as covered because a stranger said so. Fetching rules must not hinge on a - // token; claiming a capability must. + // An unauthenticated request is one whose statements about this site carry no weight, and this header + // asserts the reassuring thing: that reporting is on. Fetching rules must not hinge on a token — + // protection comes first — but claiming a capability must. const seen: Array> = []; const fetchMock = vi.fn(async (_url: string, init?: RequestInit) => { seen.push((init?.headers ?? {}) as Record); @@ -340,3 +345,331 @@ describe('the capability claim is only made when it carries weight', () => { p.stopRefresh?.(); }); }); + +describe('the bundle identity travels with the detection', () => { + it('attributes a detection to the rules that were running when it fired', async () => { + // A detection is evidence about a rule document, so it has to name the one that fired. A guard + // refreshes in place, so the identity it stamps has to move with the swap: attributed to the boot-time + // bundle, a hit produced by revision B sends a reviewer to revision A's document — which may not even + // contain the rule. + const posts: any[] = []; + let etag = '"v1"'; + const fetchMock = vi.fn(async (url: string, init?: RequestInit) => { + const target = String(url); + if (target.includes('/token')) { + return new Response(JSON.stringify({ access_token: 'jwt-abc', expires_in: 3600 }), { + status: 200, headers: { 'Content-Type': 'application/json' }, + }); + } + if (target.includes('/detections/')) { + posts.push(JSON.parse(String(init?.body ?? '{}'))); + + return new Response('{}', { status: 202 }); + } + + return new Response( + JSON.stringify({ + firewall: [{ id: 'r1', title: 'boom', rule_v2: [{ parameter: 'get.q', match: { type: 'contains', value: 'boom' } }] }], + whitelists: [], enforcement: 'dry-run', + }), + { status: 200, headers: { 'Content-Type': 'application/json', ETag: etag } }, + ); + }); + vi.stubGlobal('fetch', fetchMock); + + const p: any = await createProtection({ + siteUuid: 'site-1', + pulseRulesUrl: 'https://x.test/monitor/pulse', + pulseAuth: 'the-secret-40-chars-long-ish-value-here-987', + reportDetections: true, + detectionFlushMs: 1, + }); + + await p.fetchGuard()(new Request('https://app.test/api/x?q=boom')); + await drain(); + await drain(); + + etag = '"v2"'; + await p.refresh(); + + await p.fetchGuard()(new Request('https://app.test/api/x?q=boom')); + p.stop(); + await drain(); + await drain(); + + const stamped = posts.flatMap((body) => body.detections.map((d: any) => d.rules_etag)); + expect(stamped).toEqual(['"v1"', '"v2"']); + }); + + it('keeps the previous identity when a refresh could not reach the source', async () => { + // The control. A failed refresh keeps the previous rules, so it has to keep the previous identity — + // moving it would attribute a hit to a bundle this guard never received. + let fail = false; + const posts: any[] = []; + const fetchMock = vi.fn(async (url: string, init?: RequestInit) => { + const target = String(url); + if (target.includes('/token')) { + return new Response(JSON.stringify({ access_token: 'jwt-abc', expires_in: 3600 }), { + status: 200, headers: { 'Content-Type': 'application/json' }, + }); + } + if (target.includes('/detections/')) { + posts.push(JSON.parse(String(init?.body ?? '{}'))); + + return new Response('{}', { status: 202 }); + } + if (fail) throw new Error('network down'); + + return new Response( + JSON.stringify({ + firewall: [{ id: 'r1', title: 'boom', rule_v2: [{ parameter: 'get.q', match: { type: 'contains', value: 'boom' } }] }], + whitelists: [], enforcement: 'dry-run', + }), + { status: 200, headers: { 'Content-Type': 'application/json', ETag: '"v1"' } }, + ); + }); + vi.stubGlobal('fetch', fetchMock); + + const p: any = await createProtection({ + siteUuid: 'site-1', + pulseRulesUrl: 'https://x.test/monitor/pulse', + pulseAuth: 'the-secret-40-chars-long-ish-value-here-987', + reportDetections: true, + detectionFlushMs: 1, + }); + + fail = true; + const status = await p.refresh(); + expect(status.ok, 'a refresh that fell back to cached rules is not a successful refresh').toBe(false); + + await p.fetchGuard()(new Request('https://app.test/api/x?q=boom')); + p.stop(); + await drain(); + await drain(); + + expect(posts.flatMap((body) => body.detections.map((d: any) => d.rules_etag))).toEqual(['"v1"']); + }); +}); + +describe('reporting that cannot be delivered', () => { + it('is not started, and says so', async () => { + // The detections endpoint requires a verified, site-bound token. A reporter built without a credential + // would queue every detection, post it, and be refused — an outbound request per batch, while the + // config says reporting is on. + const posted: string[] = []; + const fetchMock = vi.fn(async (url: string) => { + posted.push(String(url)); + + return new Response(JSON.stringify({ + firewall: [{ id: 'r1', title: 'boom', rule_v2: [{ parameter: 'get.q', match: { type: 'contains', value: 'boom' } }] }], + whitelists: [], enforcement: 'dry-run', + }), { status: 200, headers: { 'Content-Type': 'application/json' } }); + }); + vi.stubGlobal('fetch', fetchMock); + + const warnings: string[] = []; + const p: any = await createProtection({ + siteUuid: 'site-1', + pulseRulesUrl: 'https://x.test/monitor/pulse', + reportDetections: true, + detectionFlushMs: 1, + onError: (err: Error) => warnings.push(err.message), + }); + + await p.fetchGuard()(new Request('https://app.test/api/x?q=boom')); + await drain(); + await drain(); + + // Distinguished from "off": a boolean would report an undeliverable configuration as a deliberate one. + expect(p.detectionReporting).toBe('unavailable-no-credential'); + expect(p.detectionHealth, 'no reporter means no health to report').toBeUndefined(); + expect(posted.some((url) => url.includes('/detections/'))).toBe(false); + expect(warnings.some((m) => m.includes('detection reporting is enabled'))).toBe(true); + + p.stop(); + }); + + it('is started, and named as running, once a credential resolves', async () => { + // The control: same configuration plus a credential. Without it the assertion above would also pass + // for an implementation that never reports at all. + const fetchMock = vi.fn(async (url: string) => { + if (String(url).includes('/token')) { + return new Response(JSON.stringify({ access_token: 'jwt-abc', expires_in: 3600 }), { + status: 200, headers: { 'Content-Type': 'application/json' }, + }); + } + + return 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', + pulseAuth: 'the-secret-40-chars-long-ish-value-here-987', + reportDetections: true, + }); + + expect(p.detectionReporting).toBe('on'); + expect(typeof p.detectionHealth).toBe('function'); + p.stop(); + }); +}); + +describe('the reporter can always be reached', () => { + it('flushes on stop, with no refresh loop and no block log installed', async () => { + // Reporting alone is a valid configuration: no refresh interval, no API key. The final batch must not + // depend on a timer nobody can bring forward, or a clean shutdown loses it. + const posts: any[] = []; + const fetchMock = vi.fn(async (url: string, init?: RequestInit) => { + const target = String(url); + if (target.includes('/token')) { + return new Response(JSON.stringify({ access_token: 'jwt-abc', expires_in: 3600 }), { + status: 200, headers: { 'Content-Type': 'application/json' }, + }); + } + if (target.includes('/detections/')) { + posts.push(JSON.parse(String(init?.body ?? '{}'))); + + return new Response('{}', { status: 202 }); + } + + return new Response(JSON.stringify({ + firewall: [{ id: 'r1', title: 'boom', rule_v2: [{ parameter: 'get.q', match: { type: 'contains', value: 'boom' } }] }], + 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', + pulseAuth: 'the-secret-40-chars-long-ish-value-here-987', + reportDetections: true, + // The default buffer window, long enough that only an explicit flush can produce the post below. + }); + + expect(typeof p.stop, 'the lifecycle method exists for every configuration').toBe('function'); + + await p.fetchGuard()(new Request('https://app.test/api/x?q=boom')); + await drain(); + expect(posts.length, 'still buffered — nothing has asked it to flush').toBe(0); + + p.stop(); + await drain(); + await drain(); + + expect(posts.length).toBe(1); + expect(p.detectionHealth()).toMatchObject({ sent: 1, delivered: 1, failed: 0, dropped: 0 }); + expect(p.detectionHealth().lastDeliveredAt).not.toBeNull(); + }); +}); + +describe('delivery health', () => { + it('separates what was acknowledged from what was refused', async () => { + // The capability declaration says a guard intends to report. Only an acknowledgement says anything + // arrived, and without counting the refusals a delivery path that rejects everything reads the same + // as an app where no rule fired. + let status = 500; + const fetchImpl = vi.fn(async () => new Response('{}', { status })); + const reporter = createDetectionReporter({ + siteUuid: 'site-1', + baseUrl: 'https://x.test/monitor/pulse', + rulesEtag: '"v7"', + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + reporter.record({ rule: pinnedRule, phase: 'request', mode: 'dry-run', path: '/a' }); + reporter.flush(); + await drain(); + + expect(reporter.health()).toMatchObject({ sent: 1, delivered: 0, failed: 1 }); + expect(reporter.health().lastDeliveredAt).toBeNull(); + + status = 202; + reporter.record({ rule: pinnedRule, phase: 'request', mode: 'dry-run', path: '/a' }); + reporter.flush(); + await drain(); + + expect(reporter.health()).toMatchObject({ sent: 2, delivered: 1, failed: 1 }); + expect(reporter.health().lastDeliveredAt).not.toBeNull(); + }); + + it('counts events dropped for queue pressure, flushed or not', async () => { + const fetchImpl = vi.fn(async () => new Response('{}', { status: 202 })); + const reporter = createDetectionReporter({ + siteUuid: 'site-1', + baseUrl: 'https://x.test/monitor/pulse', + maxQueue: 2, + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + for (let i = 0; i < 5; i++) reporter.record({ rule: pinnedRule, phase: 'request', mode: 'dry-run', path: '/a' }); + + // Reported before the flush that would carry them, so a snapshot taken between batches is not short. + expect(reporter.health().dropped).toBe(3); + + reporter.flush(); + await drain(); + expect(reporter.health().dropped).toBe(3); + }); +}); + +describe('the rule revision travels with the detection', () => { + it('reports the revision the bundle served for that rule', async () => { + // The bundle identity says WHICH BUNDLE; it changes whenever anything in the bundle changes, so it + // cannot say whether one rule's counts describe the document that rule has now. The rule's own revision + // can, and the side that served it is the side that knows it. + const { reporter, posts } = reporterWith(); + + reporter.record({ + rule: { ...pinnedRule, source_revision: 'sha256:abcdef' }, + phase: 'request', + mode: 'dry-run', + path: '/api/preview', + }); + reporter.flush(); + await drain(); + + expect(posts[0].body.detections[0].rule_revision).toBe('sha256:abcdef'); + }); + + it('reports a numeric revision as the string it was served as', async () => { + // A generated rule numbers its revisions; a curated one hashes its document. Both are identifiers, and + // the reporter forwards rather than interprets. + const { reporter, posts } = reporterWith(); + + reporter.record({ rule: { ...pinnedRule, source_revision: 13 }, phase: 'request', mode: 'dry-run', path: '/a' }); + reporter.flush(); + await drain(); + + expect(posts[0].body.detections[0].rule_revision).toBe('13'); + }); + + it('reports null when the bundle carried no revision for the rule', async () => { + // A customer's own rule has none. Null is "cannot say", which the consumer has to be able to tell apart + // from a revision that no longer matches. + const { reporter, posts } = reporterWith(); + + reporter.record({ rule: pinnedRule, phase: 'request', mode: 'dry-run', path: '/a' }); + reporter.flush(); + await drain(); + + expect(posts[0].body.detections[0].rule_revision).toBeNull(); + }); + + it('reports no revision for a value that is not one', async () => { + // An object or a boolean in that field is a served rule this client cannot read, and forwarding it + // would put an uninterpretable value where a consumer expects an identifier. + const { reporter, posts } = reporterWith(); + + reporter.record({ rule: { ...pinnedRule, source_revision: { v: 1 } }, phase: 'request', mode: 'dry-run', path: '/a' }); + reporter.record({ rule: { ...pinnedRule, source_revision: '' }, phase: 'request', mode: 'dry-run', path: '/a' }); + reporter.flush(); + await drain(); + + for (const event of posts[0].body.detections) expect(event.rule_revision).toBeNull(); + }); +}); diff --git a/tests/protect/refresh-backoff.test.ts b/tests/protect/refresh-backoff.test.ts new file mode 100644 index 0000000..e43caf3 --- /dev/null +++ b/tests/protect/refresh-backoff.test.ts @@ -0,0 +1,169 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { createProtection } from '../../src/protect/runtime.js'; + +/** + * Backoff on a rule source that is down. + * + * Resolving rules is deliberately forgiving: an API or network failure becomes a cached or bundled + * ruleset rather than an exception, because protection has to keep running. The poller needs the + * opposite — it needs to know the fetch did not succeed, or every installed guard keeps knocking at its + * normal interval for as long as the outage lasts, and they all come back at once when it ends. + */ + +const RULES = { + firewall: [{ id: 'r1', title: 'boom', rule_v2: [{ parameter: 'get.q', match: { type: 'contains', value: 'boom' } }] }], + whitelists: [], + enforcement: 'dry-run', +}; + +function rulesEndpoint(state: { fail: boolean }) { + return vi.fn(async (url: string) => { + if (state.fail) throw new Error('rule source unreachable'); + + return new Response(JSON.stringify(RULES), { status: 200, headers: { 'Content-Type': 'application/json' } }); + }); +} + +/** Rule fetches only, so the manifest re-post on the same tick cannot be mistaken for one. */ +function ruleFetches(fetchMock: { mock: { calls: unknown[][] } }): number { + return fetchMock.mock.calls.filter(([url]) => String(url).includes('/rules/')).length; +} + +afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); +}); + +describe('the refresh loop', () => { + it('slows down while the source is unreachable, even though the guard keeps its rules', async () => { + vi.useFakeTimers(); + const state = { fail: false }; + const fetchMock = rulesEndpoint(state); + vi.stubGlobal('fetch', fetchMock); + + const p: any = await createProtection({ + siteUuid: 'site-1', + pulseRulesUrl: 'https://x.test/monitor/pulse', + reportManifest: false, + refreshMs: 10_000, + }); + expect(ruleFetches(fetchMock), 'the boot fetch').toBe(1); + + state.fail = true; + await vi.advanceTimersByTimeAsync(10_000); + expect(ruleFetches(fetchMock), 'one poll, which could not reach the source').toBe(2); + + // The rules are still in force — the failure is about currency, not protection. + expect(p.rules.request.length).toBe(1); + + await vi.advanceTimersByTimeAsync(10_000); + expect(ruleFetches(fetchMock), 'backed off past the next ordinary interval').toBe(2); + + await vi.advanceTimersByTimeAsync(10_000); + expect(ruleFetches(fetchMock)).toBe(3); + + p.stop(); + }); + + it('returns to the normal interval once the source answers again', async () => { + // The other half: a backoff that never resets would leave a fleet minutes behind a zero-day rule + // because of an outage that ended. + vi.useFakeTimers(); + const state = { fail: true }; + const fetchMock = rulesEndpoint(state); + vi.stubGlobal('fetch', fetchMock); + + const p: any = await createProtection({ + siteUuid: 'site-1', + pulseRulesUrl: 'https://x.test/monitor/pulse', + reportManifest: false, + rules: RULES, + refreshMs: 10_000, + }); + + await vi.advanceTimersByTimeAsync(10_000); + const failedPolls = ruleFetches(fetchMock); + + state.fail = false; + // Long enough to get through the backed-off delay and land the recovering poll. + await vi.advanceTimersByTimeAsync(80_000); + const recovered = ruleFetches(fetchMock); + expect(recovered).toBeGreaterThan(failedPolls); + + await vi.advanceTimersByTimeAsync(10_000); + expect(ruleFetches(fetchMock), 'polling at the configured interval again').toBeGreaterThan(recovered); + + p.stop(); + }); + + it('does not back off on a healthy poll', async () => { + // The control. Without it the first test would pass for a loop that backs off unconditionally. + vi.useFakeTimers(); + const state = { fail: false }; + const fetchMock = rulesEndpoint(state); + vi.stubGlobal('fetch', fetchMock); + + const p: any = await createProtection({ + siteUuid: 'site-1', + pulseRulesUrl: 'https://x.test/monitor/pulse', + reportManifest: false, + refreshMs: 10_000, + }); + + await vi.advanceTimersByTimeAsync(10_000); + await vi.advanceTimersByTimeAsync(10_000); + await vi.advanceTimersByTimeAsync(10_000); + expect(ruleFetches(fetchMock), 'boot plus one poll per interval').toBe(4); + + p.stop(); + }); + + it('reports a fallback as an unsuccessful refresh to a caller that asks for one', async () => { + const state = { fail: false }; + const fetchMock = rulesEndpoint(state); + vi.stubGlobal('fetch', fetchMock); + + const p: any = await createProtection({ + siteUuid: 'site-1', + pulseRulesUrl: 'https://x.test/monitor/pulse', + reportManifest: false, + }); + + expect(await p.refresh()).toEqual({ ok: true }); + + state.fail = true; + const failed = await p.refresh(); + expect(failed.ok).toBe(false); + expect(typeof failed.reason).toBe('string'); + + // Still resolved, not rejected: a manual refresh reports the outcome, and the rules it already had + // are still loaded. + expect(p.rules.request.length).toBe(1); + + p.stop(); + }); + + it('answers the push endpoint with what actually happened', async () => { + const state = { fail: true }; + const fetchMock = rulesEndpoint(state); + vi.stubGlobal('fetch', fetchMock); + + const p: any = await createProtection({ + siteUuid: 'site-1', + pulseRulesUrl: 'https://x.test/monitor/pulse', + reportManifest: false, + rules: RULES, + refreshSecret: 'push-secret', + }); + + const handler = p.refreshHandler(); + const refused = await handler(new Request('https://app.test/refresh', { headers: { 'x-patchstack-refresh': 'push-secret' } })); + expect(await refused.json()).toEqual({ refreshed: false }); + + state.fail = false; + const ok = await handler(new Request('https://app.test/refresh', { headers: { 'x-patchstack-refresh': 'push-secret' } })); + expect(await ok.json()).toEqual({ refreshed: true }); + + p.stop(); + }); +}); diff --git a/tests/protect/runtime-pulse.test.ts b/tests/protect/runtime-pulse.test.ts index cea78a4..dbc7797 100644 --- a/tests/protect/runtime-pulse.test.ts +++ b/tests/protect/runtime-pulse.test.ts @@ -207,7 +207,6 @@ describe('createProtection live rule refresh (refreshMs)', () => { vi.stubGlobal('fetch', fetchMock); const protection = await createProtection({ siteUuid: 'site-1', pulseRulesUrl: 'https://x.test/monitor/pulse', mode: 'block' }); - expect(protection.stopRefresh).toBeUndefined(); await vi.advanceTimersByTimeAsync(60_000); expect(fetchMock).toHaveBeenCalledTimes(1); // only the boot fetch — no interval re-fetches @@ -217,7 +216,10 @@ describe('createProtection live rule refresh (refreshMs)', () => { }); it('does not schedule a refresh without a live source, even with refreshMs set', async () => { + // A bundle is not a source: there is nothing to re-fetch, so neither trigger is offered. Asserted on + // `refresh`/`refreshHandler` rather than on the stop method, which exists for every configuration. const protection = await createProtection({ rules, mode: 'block', refreshMs: 1000 }); - expect(protection.stopRefresh).toBeUndefined(); + expect(protection.refresh).toBeUndefined(); + expect(protection.refreshHandler).toBeUndefined(); }); });