From 962923fc46bbb2c32e73c2e81c9ee6cf5c2c4205 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Mon, 28 Sep 2026 16:13:39 +0200 Subject: [PATCH 1/2] Share one outbound guard between protections and release it on stop fetch and node:http(s) are process-wide, so every protection with egress enabled now registers its own screen with one shared guard. A call is refused when any registered screen refuses it, and each protection reports its own refusals. Uninstalling removes only that protection's screen; the last one out restores the original functions. protection.stop() now uninstalls the protection's screen too. A fetch call whose arguments the runtime's Request rejects is still screened when its destination can be read, and is counted as an unscreened call when it cannot. Co-Authored-By: Claude Opus 5.5 --- AGENT-INSTALL.md | 6 +- src/protect/egress.js | 138 ++++++++++--- src/protect/protect.d.ts | 7 +- src/protect/runtime.js | 11 +- tests/protect/egress-guard-lifecycle.test.ts | 187 ++++++++++++++++++ .../egress-unrecognised-fetch-input.test.ts | 55 ++++++ 6 files changed, 366 insertions(+), 38 deletions(-) create mode 100644 tests/protect/egress-guard-lifecycle.test.ts create mode 100644 tests/protect/egress-unrecognised-fetch-input.test.ts diff --git a/AGENT-INSTALL.md b/AGENT-INSTALL.md index dc40cdbc..b78bfc40 100644 --- a/AGENT-INSTALL.md +++ b/AGENT-INSTALL.md @@ -574,8 +574,10 @@ carries no detections and would otherwise read as one. Those counts stay in your 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. +block-log reporter, the detection reporter — and flushes what is buffered. With `egress: true` it also +removes this guard's outbound-request screening; once no guard in the process is screening, `fetch` and +`node:http`/`node:https` are restored. `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: diff --git a/src/protect/egress.js b/src/protect/egress.js index 02ee6779..12a2edc3 100644 --- a/src/protect/egress.js +++ b/src/protect/egress.js @@ -10,12 +10,50 @@ * onBlock?: (info:{url:string,host:string|null,method:string})=>void, * dnsScreen?: boolean, * lookup?: Function }} opts - * @returns {Promise<() => void>} uninstall (restores every patched surface) + * @returns {Promise<() => void>} uninstall (removes this screen; the last one out restores the patched surfaces) */ import { notify } from './notify.js'; +// fetch and node:http(s) are process-wide, so the guard on them is too: one wrapper per surface, shared +// by every protection in the process (including another copy of this package), each registering its own +// screen. A call is refused when any registered screen refuses it. Keyed on a global symbol so that two +// copies of this module share one registry rather than each deciding the other's wrapper is enough. +const REGISTRY = Symbol.for('patchstack.connect.egress-guard'); + +function egressRegistry() { + const existing = globalThis[REGISTRY]; + if (existing && existing.screens instanceof Set && existing.surfaces instanceof Map) return existing; + const created = { screens: new Set(), surfaces: new Map() }; + Object.defineProperty(globalThis, REGISTRY, { value: created, configurable: true, writable: true }); + return created; +} + +const refusal = (host) => new Error(`Patchstack blocked an outbound request to a disallowed address: ${host}`); + +// The destination of a fetch call whose arguments `Request` would not accept, when one can be read: a +// URL string, a URL object, or an object carrying `url`/`href`. Null when there is no parseable URL. +function readableDestination(input, init) { + try { + const raw = typeof input === 'string' || input instanceof URL ? String(input) : input?.url ?? input?.href; + if (typeof raw !== 'string' && !(raw instanceof URL)) return null; + const url = new URL(String(raw)).href; + const method = String(init?.method ?? input?.method ?? 'GET').toUpperCase(); + return { url, method }; + } catch { + return null; + } +} + +// Asks every screen, so each one reports its own refusal, then answers whether any refused. +function anyRefuses(screens, url, host, method) { + let refused = false; + for (const screen of screens) { + if (screen.block(url, host, method)) refused = true; + } + return refused; +} + export async function installEgressGuard({ shouldBlock, onBlock, onSkip, dnsScreen = true, lookup, allowHosts } = {}) { - const restores = []; if (typeof shouldBlock !== 'function') return () => {}; const exempt = new Set((allowHosts ?? []).map((h) => String(h).toLowerCase())); @@ -89,6 +127,10 @@ export async function installEgressGuard({ shouldBlock, onBlock, onSkip, dnsScre } }); + const registry = egressRegistry(); + const own = { block, prescreen: resolvesToDisallowed, dns: screen, skip }; + registry.screens.add(own); + // 1. global fetch — synchronous install, so it's active the instant this returns (no startup race). const originalFetch = globalThis.fetch; if (typeof originalFetch === 'function' && !originalFetch.__patchstackGuarded) { @@ -158,8 +200,8 @@ export async function installEgressGuard({ shouldBlock, onBlock, onSkip, dnsScre } }; - // Screen one outbound URL: hostname/allowlist/literal-IP check, then a DNS-resolution check for - // real hostnames. Throws if the destination is disallowed. + // Screen one outbound URL against every registered screen: hostname/allowlist/literal-IP check, + // then a DNS-resolution check for real hostnames. Throws if any screen disallows the destination. const screenUrl = async (u, method) => { let host = null; try { @@ -167,9 +209,10 @@ export async function installEgressGuard({ shouldBlock, onBlock, onSkip, dnsScre } catch { host = null; } - if (block(u, host, method) || (await resolvesToDisallowed(u, host, method))) { - throw new Error(`Patchstack blocked an outbound request to a disallowed address: ${host ?? u}`); - } + const screens = [...registry.screens]; + if (anyRefuses(screens, u, host, method)) throw refusal(host ?? u); + const resolved = await Promise.all(screens.map((each) => each.prescreen(u, host, method))); + if (resolved.includes(true)) throw refusal(host ?? u); }; const guarded = async (input, init) => { @@ -177,7 +220,13 @@ export async function installEgressGuard({ shouldBlock, onBlock, onSkip, dnsScre try { cur = new Request(input, { ...(init || {}), redirect: 'manual' }); } catch { - return originalFetch(input, init); // odd input we can't normalize — fail open, don't break the caller + // An input this runtime's Request refuses is handed to the underlying fetch as it came, which + // decides whether it is a request at all. Its destination is still screened when it can be read; + // when it cannot, the call goes out unscreened and is counted as such. + const destination = readableDestination(input, init); + if (destination) await screenUrl(destination.url, destination.method); + else for (const each of registry.screens) each.skip('unrecognised-request', {}); + return originalFetch(input, init); } const callerRedirect = (init && init.redirect) || (input && input.redirect) || 'follow'; @@ -247,8 +296,15 @@ export async function installEgressGuard({ shouldBlock, onBlock, onSkip, dnsScre }; guarded.__patchstackGuarded = true; globalThis.fetch = guarded; - restores.push(() => { - if (globalThis.fetch === guarded) globalThis.fetch = originalFetch; + // Released only while it is still the global: a wrapper layered on top later (an APM agent, …) + // keeps calling this one, which then stays registered and screens with whichever screens are + // registered at the time — none, until a protection registers again. + registry.surfaces.set(guarded, { + release() { + if (globalThis.fetch !== guarded) return false; + globalThis.fetch = originalFetch; + return true; + }, }); } @@ -270,12 +326,16 @@ export async function installEgressGuard({ shouldBlock, onBlock, onSkip, dnsScre for (const moduleName of ['node:http', 'node:https']) { try { const mod = await import(moduleName); - const restore = patchHttpModule(mod.default ?? mod, block, screen, skip); - if (restore) { + if (registry.surfaces.has(moduleName)) continue; + const release = patchHttpModule(mod.default ?? mod, registry); + if (release) { patchedAny = true; - restores.push(() => { - restore(); - syncBuiltins(); + registry.surfaces.set(moduleName, { + release() { + if (!release()) return false; + syncBuiltins(); + return true; + }, }); } } catch { @@ -292,10 +352,14 @@ export async function installEgressGuard({ shouldBlock, onBlock, onSkip, dnsScre // and a hostname-only check would over-promise the control. Outbound screening covers fetch and // node:http/https. + // Removes this screen only. The last screen to leave releases the surfaces it can; one that is no + // longer the outermost wrapper stays registered, screening nothing until a screen registers again. return () => { - for (const restore of restores) { + registry.screens.delete(own); + if (registry.screens.size > 0) return; + for (const [name, surface] of registry.surfaces) { try { - restore(); + if (surface.release()) registry.surfaces.delete(name); } catch { /* ignore */ } @@ -305,7 +369,7 @@ export async function installEgressGuard({ shouldBlock, onBlock, onSkip, dnsScre // Wrap http(s).request/get — and, on node:http, the ClientRequest constructor they build — so a // blocked destination throws before the socket opens. -function patchHttpModule(http, block, screen, skip) { +function patchHttpModule(http, registry) { if (!http || typeof http.request !== 'function' || http.__patchstackGuarded) return null; const originalRequest = http.request; const originalGet = http.get; @@ -314,14 +378,23 @@ function patchHttpModule(http, block, screen, skip) { // The arguments to hand on, after screening them. Throws when the destination is refused. const guardArgs = (args) => { const target = extractHttpTarget(args); - if (target && block(target.url, target.host, target.method)) { - throw new Error(`Patchstack blocked an outbound request to a disallowed address: ${target.host ?? target.url}`); - } - // DNS screen: only for real hostnames (a literal IP was already covered by the check above), - // and skip an explicitly allowlisted host (the operator trusts it — don't second-guess its DNS). - if (target && screen && target.host && screen.isIP(target.host) === 0 && !screen.isExempt(target.host)) { + if (!target) return args; + const screens = [...registry.screens]; + if (anyRefuses(screens, target.url, target.host, target.method)) throw refusal(target.host ?? target.url); + // DNS screen: only for real hostnames (a literal IP was already covered by the check above), and + // not for a screen that allowlists this host (the operator trusts it — don't second-guess its DNS). + // One resolution serves every screen that wants one, so the connection is pinned to addresses that + // all of them checked; it goes through the resolver of the earliest of those screens. + const resolving = target.host + ? screens.filter((each) => each.dns && each.dns.isIP(target.host) === 0 && !each.dns.isExempt(target.host)) + : []; + if (resolving.length > 0) { + const block = (url, host, method) => anyRefuses(resolving, url, host, method); + const skip = (reason, detail) => { + for (const each of resolving) each.skip(reason, detail); + }; try { - return withScreeningLookup(args, target, block, screen.lookup, skip); + return withScreeningLookup(args, target, block, resolving[0].dns.lookup, skip); } catch { // The call goes on with the arguments it came with. Nothing is counted as a fail-open bypass: // the only thing here that can throw is reading the caller's options, and Node copies that @@ -365,13 +438,18 @@ function patchHttpModule(http, block, screen, skip) { } http.__patchstackGuarded = true; + // Released only when every wrapper is still the module's own export — don't clobber a wrapper another + // library (an APM agent, etc.) layered on top of us after install. Otherwise nothing is restored and + // the module keeps calling through ours, so it is never left half-guarded. return () => { - // Only restore if our wrapper is still installed — don't clobber a wrapper another library - // (an APM agent, etc.) layered on top of us after install. - if (http.request === guardedRequest) http.request = originalRequest; - if (guardedGet && http.get === guardedGet) http.get = originalGet; - if (GuardedClientRequest && http.ClientRequest === GuardedClientRequest) http.ClientRequest = OriginalClientRequest; + if (http.request !== guardedRequest) return false; + if (guardedGet && http.get !== guardedGet) return false; + if (GuardedClientRequest && http.ClientRequest !== GuardedClientRequest) return false; + http.request = originalRequest; + if (guardedGet) http.get = originalGet; + if (GuardedClientRequest) http.ClientRequest = OriginalClientRequest; delete http.__patchstackGuarded; + return true; }; } diff --git a/src/protect/protect.d.ts b/src/protect/protect.d.ts index 1047bb30..7f67dd4c 100644 --- a/src/protect/protect.d.ts +++ b/src/protect/protect.d.ts @@ -29,7 +29,9 @@ export interface Protection { screenResponse(response: Response, request?: Request): Promise; express(options?: { screenResponses?: boolean }): (req: unknown, res: unknown, next: () => void) => void; node(options?: { maxBodyBytes?: number; screenResponses?: boolean }): (req: unknown, res: unknown, next: () => void) => void; - /** Present when `egress: true` — restores the original global fetch. */ + /** Present when `egress: true` — removes this protection's outbound screen. Outbound calls are + * screened by every protection that has one registered; when the last one leaves, the original + * `fetch` and `node:http`/`node:https` functions are restored. `stop()` calls this too. */ uninstallEgress?: () => void; /** 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 @@ -46,7 +48,8 @@ export interface Protection { * the configured refresh secret (a push/zero-day trigger). No secret set → the handler 404s. */ refreshHandler?: () => (request: Request) => Promise; /** 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. */ + * detection reporter (flushing what it holds), and this protection's outbound screen. Always + * present, and safe to call twice. */ /** * Stop everything holding a timer or a buffer. * diff --git a/src/protect/runtime.js b/src/protect/runtime.js index 51575f02..233f3673 100644 --- a/src/protect/runtime.js +++ b/src/protect/runtime.js @@ -1685,10 +1685,11 @@ export async function createProtection(options = {}) { const canAsk = Boolean(options.token || pulseAuth); recovery = live && canAsk && !loop && ruleSource.ok === false ? startRecovery(refreshTick, { 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. + // One method, always present, that reaches everything holding a timer, a buffer or a process-wide + // hook: the refresh loop, the block log, the detection reporter, the outbound screen. 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. // // Returns a promise that settles when the reporter has finished draining, so a host shutting down can // await it rather than racing the last batch against process exit. Bounded and best-effort — a runtime @@ -1696,6 +1697,8 @@ export async function createProtection(options = {}) { protection.stop = () => { loop?.stop(); recovery?.stop(); + // This protection's outbound screen leaves the shared guard; other protections keep theirs. + protection.uninstallEgress?.(); // Both reporters, because the promise says every buffer this reaches is finished with. Waiting only // for one would resolve while the other still had records outstanding — and resolve immediately in a // configuration where the one being waited for was never built. diff --git a/tests/protect/egress-guard-lifecycle.test.ts b/tests/protect/egress-guard-lifecycle.test.ts new file mode 100644 index 00000000..16420acf --- /dev/null +++ b/tests/protect/egress-guard-lifecycle.test.ts @@ -0,0 +1,187 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { createProtection } from '../../src/protect/runtime.js'; +import { installEgressGuard } from '../../src/protect/egress.js'; + +// The egress guard is process-wide: fetch and node:http are shared by every protection in the process. +// Each protection registers its own screen, a call is refused when any registered screen refuses it, a +// protection leaving takes only its own screen with it, and stop() is one of the ways it leaves. + +const originalFetch = globalThis.fetch; +const active: any[] = []; + +afterEach(async () => { + for (const protection of active.splice(0)) await protection.stop(); + globalThis.fetch = originalFetch; +}); + +async function nodeHttp() { + const ns: any = await import('node:http'); + return ns.default ?? ns; +} + +const refuse = (id: string, host: string) => ({ + id, + category: 'ssrf', + rule_v2: [{ parameter: 'egress.host', match: { type: 'equals', value: host } }], +}); + +async function guard(host: string, blocks: string[] = []) { + const protection: any = await createProtection({ + egress: true, + mode: 'block', + screenDns: false, + egressRules: [refuse(`refuse-${host}`, host)], + onEgressBlock: ({ host: blocked }: { host: string }) => blocks.push(blocked), + }); + active.push(protection); + return protection; +} + +function stubFetch() { + const seen: string[] = []; + globalThis.fetch = (async (input: any) => { + seen.push(typeof input === 'string' ? input : input.url); + return new Response('stub'); + }) as any; + return seen; +} + +const refused = (call: () => unknown) => { + try { + call(); + return false; + } catch (error) { + return /Patchstack blocked/.test(String(error)); + } +}; + +describe('egress guard lifecycle', () => { + it('restores fetch and node:http when the protection stops', async () => { + stubFetch(); + const before = globalThis.fetch; + const http = await nodeHttp(); + const request = http.request; + const ClientRequest = http.ClientRequest; + + const protection = await guard('first.test'); + expect(globalThis.fetch).not.toBe(before); + expect(http.request).not.toBe(request); + + await protection.stop(); + active.splice(active.indexOf(protection), 1); + expect(globalThis.fetch).toBe(before); + expect(http.request).toBe(request); + expect(http.ClientRequest).toBe(ClientRequest); + }); + + it('screens a call against every registered protection', async () => { + const seen = stubFetch(); + const firstBlocks: string[] = []; + const secondBlocks: string[] = []; + await guard('first.test', firstBlocks); + await guard('second.test', secondBlocks); + const http = await nodeHttp(); + + await expect(fetch('http://first.test/')).rejects.toThrow(/Patchstack blocked/); + await expect(fetch('http://second.test/')).rejects.toThrow(/Patchstack blocked/); + expect(await (await fetch('http://third.test/')).text()).toBe('stub'); + expect(refused(() => http.request('http://first.test/'))).toBe(true); + expect(refused(() => http.request('http://second.test/'))).toBe(true); + + expect(firstBlocks).toEqual(['first.test', 'first.test']); + expect(secondBlocks).toEqual(['second.test', 'second.test']); + expect(seen).toEqual(['http://third.test/']); + }); + + it('keeps the remaining screens when one protection leaves', async () => { + stubFetch(); + const before = globalThis.fetch; + const first = await guard('first.test'); + const second = await guard('second.test'); + const http = await nodeHttp(); + + first.uninstallEgress(); + expect(await (await fetch('http://first.test/')).text()).toBe('stub'); + await expect(fetch('http://second.test/')).rejects.toThrow(/Patchstack blocked/); + expect(refused(() => http.request('http://second.test/'))).toBe(true); + + // Leaving twice changes nothing for the protection still registered. + first.uninstallEgress(); + await expect(fetch('http://second.test/')).rejects.toThrow(/Patchstack blocked/); + + await second.stop(); + active.length = 0; + expect(globalThis.fetch).toBe(before); + }); + + it('pins a node:http call to one resolution that every resolving screen checked', async () => { + const http = await nodeHttp(); + const internal = [{ address: '10.0.0.5', family: 4 }]; + const asked: string[] = []; + const blocks: string[] = []; + const install = (name: string, allowHosts: string[] = []) => + installEgressGuard({ + shouldBlock: (_url: string, host: string | null) => host === '10.0.0.5', + onBlock: () => blocks.push(name), + allowHosts, + lookup: (_host: string, _options: unknown, callback: any) => { + asked.push(name); + callback(null, internal); + }, + }); + const connectError = () => + new Promise((resolve) => { + const request = http.request({ host: 'service.test', port: 80, path: '/' }); + request.on('error', (error: Error) => resolve(error.message)); + request.end(); + }); + + const trusting = await install('trusting', ['service.test']); + const strict = await install('strict'); + const later = await install('later'); + try { + expect(await connectError()).toContain('resolved to 10.0.0.5'); + // Resolved once, through the earliest screen that screens this host, and checked by each of them. + expect(asked).toEqual(['strict']); + expect(blocks).toEqual(['strict', 'later']); + } finally { + trusting(); + strict(); + later(); + } + }); + + it("checks a fetch call's resolution against every screen that resolves it", async () => { + const seen = stubFetch(); + const blocks: string[] = []; + const install = (name: string, allowHosts: string[] = []) => + installEgressGuard({ + shouldBlock: (_url: string, host: string | null) => host === '10.0.0.5', + onBlock: () => blocks.push(name), + allowHosts, + lookup: (_host: string, _options: unknown, callback: any) => callback(null, [{ address: '10.0.0.5', family: 4 }]), + }); + + const trusting = await install('trusting', ['service.test']); + const strict = await install('strict'); + try { + await expect(fetch('http://service.test/')).rejects.toThrow(/Patchstack blocked/); + expect(blocks).toEqual(['strict']); + expect(seen).toEqual([]); + } finally { + trusting(); + strict(); + } + }); + + it('screens a later protection after an earlier one stopped', async () => { + stubFetch(); + const first = await guard('first.test'); + await first.stop(); + active.length = 0; + + await guard('second.test'); + await expect(fetch('http://second.test/')).rejects.toThrow(/Patchstack blocked/); + expect(await (await fetch('http://first.test/')).text()).toBe('stub'); + }); +}); diff --git a/tests/protect/egress-unrecognised-fetch-input.test.ts b/tests/protect/egress-unrecognised-fetch-input.test.ts new file mode 100644 index 00000000..779dca27 --- /dev/null +++ b/tests/protect/egress-unrecognised-fetch-input.test.ts @@ -0,0 +1,55 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { createProtection } from '../../src/protect/runtime.js'; + +// A fetch call whose arguments the runtime's `Request` refuses is still handed to the underlying fetch, +// which may accept it. Its destination is screened when it can be read, and counted as unscreened when +// it cannot. + +const originalFetch = globalThis.fetch; +let protection: any; + +afterEach(async () => { + await protection?.stop(); + protection = undefined; + globalThis.fetch = originalFetch; +}); + +async function setup() { + const forwarded: unknown[] = []; + globalThis.fetch = (async (input: unknown) => { + forwarded.push(input); + return new Response('stub'); + }) as any; + const skips: any[] = []; + protection = await createProtection({ egress: true, mode: 'block', onSkip: (event: any) => skips.push(event) }); + return { forwarded, skips }; +} + +describe('fetch input the runtime Request refuses', () => { + it('screens a readable destination', async () => { + const { forwarded, skips } = await setup(); + const input = { url: 'http://127.0.0.1/admin' }; + + await expect(fetch(input as any)).rejects.toThrow(/Patchstack blocked/); + expect(forwarded).toEqual([]); + expect(skips).toEqual([]); + }); + + it('passes a readable public destination on as it came', async () => { + const { forwarded, skips } = await setup(); + const input = { href: 'http://93.184.216.34/api', method: 'post' }; + + expect(await (await fetch(input as any)).text()).toBe('stub'); + expect(forwarded).toEqual([input]); + expect(skips).toEqual([]); + }); + + it('counts an unreadable destination as unscreened', async () => { + const { forwarded, skips } = await setup(); + + expect(await (await fetch(12345 as any)).text()).toBe('stub'); + expect(forwarded).toEqual([12345]); + expect(skips).toMatchObject([{ phase: 'egress', reason: 'unrecognised-request' }]); + expect(protection.coverage().skipped).toMatchObject({ 'egress:unrecognised-request': 1 }); + }); +}); From 7af0d7c9fe8fd5a75f5f41d0171460d7a4206c71 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Mon, 28 Sep 2026 16:40:10 +0200 Subject: [PATCH 2/2] Keep stopRefresh() to the rule refresh stopRefresh() now stops only the rule refresh: the poll loop and its recovery retries. The reporters and the protection's outbound screening keep running; stop() still ends everything, including removing the outbound screen. Test cleanups that relied on the full shutdown now call stop(). Docs and declarations now say that when several protections screen outbound calls, any one of them can refuse a call, so one protection's allowHosts does not override another's refusal. Co-Authored-By: Claude Opus 5.5 --- AGENT-INSTALL.md | 9 ++- src/protect/protect.d.ts | 9 ++- src/protect/runtime.js | 10 ++- tests/protect/callback-containment.test.ts | 2 +- tests/protect/detections.test.ts | 10 +-- tests/protect/egress-guard-lifecycle.test.ts | 18 +++++ tests/protect/firewall-log.test.ts | 4 +- tests/protect/generated-rule-chain.test.ts | 4 +- tests/protect/pulse-chain.test.ts | 2 +- tests/protect/refresh-manifest.test.ts | 2 +- tests/protect/runtime-pulse.test.ts | 4 +- tests/protect/stop-refresh-scope.test.ts | 85 ++++++++++++++++++++ 12 files changed, 138 insertions(+), 21 deletions(-) create mode 100644 tests/protect/stop-refresh-scope.test.ts diff --git a/AGENT-INSTALL.md b/AGENT-INSTALL.md index b78bfc40..394ed6cc 100644 --- a/AGENT-INSTALL.md +++ b/AGENT-INSTALL.md @@ -576,8 +576,13 @@ 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. With `egress: true` it also removes this guard's outbound-request screening; once no guard in the process is screening, `fetch` and -`node:http`/`node:https` are restored. `protection.stopRefresh()` is the same method under its older name. -Call it on shutdown; it is safe to call twice. +`node:http`/`node:https` are restored. Call it on shutdown; it is safe to call twice. +`protection.stopRefresh()` stops only the rule refresh: the reporters and outbound-request screening keep +running. + +When more than one guard in a process has `egress: true`, an outbound call is checked by each of them and +refused if any one refuses it. A host listed in one guard's `allowHosts` is still refused when another guard +refuses it. Two more endpoints the package can call, for completeness: diff --git a/src/protect/protect.d.ts b/src/protect/protect.d.ts index 7f67dd4c..ec073d00 100644 --- a/src/protect/protect.d.ts +++ b/src/protect/protect.d.ts @@ -30,8 +30,10 @@ export interface Protection { express(options?: { screenResponses?: boolean }): (req: unknown, res: unknown, next: () => void) => void; node(options?: { maxBodyBytes?: number; screenResponses?: boolean }): (req: unknown, res: unknown, next: () => void) => void; /** Present when `egress: true` — removes this protection's outbound screen. Outbound calls are - * screened by every protection that has one registered; when the last one leaves, the original - * `fetch` and `node:http`/`node:https` functions are restored. `stop()` calls this too. */ + * screened by every protection that has one registered, and any one of them can refuse a call: + * a host in one protection's `allowHosts` is still refused when another protection refuses it. + * When the last one leaves, the original `fetch` and `node:http`/`node:https` functions are + * restored. `stop()` calls this too; `stopRefresh()` does not. */ uninstallEgress?: () => void; /** 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 @@ -70,7 +72,8 @@ export interface Protection { * to a failed token exchange, a failed post, or a shutdown that ran out of time is reported nowhere. */ stop: () => Promise; - /** Alias of `stop`, under the name callers already have. */ + /** Stops the rule refresh only — the poll loop and its recovery retries. The reporters and this + * protection's outbound screening keep running; use `stop()` to end those too. */ stopRefresh: () => Promise; /** * Where the rules in force came from, and whether the most recent resolution was clean — the same diff --git a/src/protect/runtime.js b/src/protect/runtime.js index 233f3673..bf3b5ca9 100644 --- a/src/protect/runtime.js +++ b/src/protect/runtime.js @@ -1708,8 +1708,14 @@ export async function createProtection(options = {}) { return Promise.all(outstanding).then(() => undefined); }; - // The name callers already have, kept as an alias for it. - protection.stopRefresh = protection.stop; + // The rule refresh only: the poll loop and the recovery retries. The reporters and this protection's + // outbound screen keep running; `stop()` ends those as well. + protection.stopRefresh = () => { + loop?.stop(); + recovery?.stop(); + + return Promise.resolve(); + }; // 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. // A getter, because the state follows refreshes: a property assigned once would report the boot value diff --git a/tests/protect/callback-containment.test.ts b/tests/protect/callback-containment.test.ts index f4d89d42..58d23877 100644 --- a/tests/protect/callback-containment.test.ts +++ b/tests/protect/callback-containment.test.ts @@ -114,7 +114,7 @@ describe('a throwing host callback cannot break the guard', () => { // poll loop is how a long-lived process dies hours after the mistake was made. await expect(p.refresh()).resolves.not.toThrow(); - p.stopRefresh?.(); + p.stop?.(); }); it('keeps serving when onSkip throws', async () => { diff --git a/tests/protect/detections.test.ts b/tests/protect/detections.test.ts index 97c89079..1844d5ea 100644 --- a/tests/protect/detections.test.ts +++ b/tests/protect/detections.test.ts @@ -322,7 +322,7 @@ describe('wiring', () => { const posted = fetchMock.mock.calls.filter(([url]) => String(url).includes('/detections/')); expect(posted.length, 'no detection report without reportDetections: true').toBe(0); - p.stopRefresh?.(); + p.stop?.(); }); }); @@ -369,7 +369,7 @@ describe('declaring the capability', () => { // The first fetch of a site with no cached bundle honestly reports that it holds no managed rules // yet; the state that follows the resolution is asserted separately below. expect(p.detectionReporting).toBe('on'); - p.stopRefresh?.(); + p.stop?.(); }); it('says nothing when reporting is off', async () => { @@ -388,7 +388,7 @@ describe('declaring the capability', () => { const p: any = await createProtection({ siteUuid: 'site-1', pulseRulesUrl: 'https://x.test/monitor/pulse' }); expect(seen.every((h) => h['X-Patchstack-Detections'] === undefined)).toBe(true); - p.stopRefresh?.(); + p.stop?.(); }); }); @@ -427,7 +427,7 @@ describe('the wiring actually runs', () => { }); await p.fetchGuard()(new Request('https://app.test/api/x?q=boom')); - p.stopRefresh?.(); + p.stop?.(); await new Promise((resolve) => setTimeout(resolve, 5)); expect(posted.some((url) => url.includes('/detections/site-1'))).toBe(true); @@ -464,7 +464,7 @@ describe('the capability claim is only made when it carries weight', () => { .toBeUndefined(); } - p.stopRefresh?.(); + p.stop?.(); }); }); diff --git a/tests/protect/egress-guard-lifecycle.test.ts b/tests/protect/egress-guard-lifecycle.test.ts index 16420acf..d150332f 100644 --- a/tests/protect/egress-guard-lifecycle.test.ts +++ b/tests/protect/egress-guard-lifecycle.test.ts @@ -93,6 +93,24 @@ describe('egress guard lifecycle', () => { expect(seen).toEqual(['http://third.test/']); }); + it("refuses a host one protection allows when another protection refuses it", async () => { + const seen = stubFetch(); + const trusting: any = await createProtection({ egress: true, mode: 'block', allowHosts: ['127.0.0.1'] }); + active.push(trusting); + expect(await (await fetch('http://127.0.0.1/admin')).text()).toBe('stub'); + + const strict: any = await createProtection({ egress: true, mode: 'block' }); + active.push(strict); + await expect(fetch('http://127.0.0.1/admin')).rejects.toThrow(/Patchstack blocked/); + const http = await nodeHttp(); + expect(refused(() => http.request('http://127.0.0.1/admin'))).toBe(true); + + await strict.stop(); + active.splice(active.indexOf(strict), 1); + expect(await (await fetch('http://127.0.0.1/admin')).text()).toBe('stub'); + expect(seen).toEqual(['http://127.0.0.1/admin', 'http://127.0.0.1/admin']); + }); + it('keeps the remaining screens when one protection leaves', async () => { stubFetch(); const before = globalThis.fetch; diff --git a/tests/protect/firewall-log.test.ts b/tests/protect/firewall-log.test.ts index 74c99e3f..17e7bdfc 100644 --- a/tests/protect/firewall-log.test.ts +++ b/tests/protect/firewall-log.test.ts @@ -153,7 +153,7 @@ describe('createProtection connector log reporting', () => { expect(fetchImpl.mock.calls.some(([u]) => String(u).includes('/api/logs/log'))).toBe(true); }); - protection.stopRefresh?.(); + protection.stop?.(); }); it('does not report without an api key', async () => { @@ -194,7 +194,7 @@ describe('createProtection connector log reporting', () => { expect(fetchImpl.mock.calls.filter(([u]) => String(u).includes('/api/logs/log'))).toHaveLength(0); expect(fetchImpl.mock.calls.filter(([u]) => String(u).includes('/oauth/token'))).toHaveLength(0); - protection.stopRefresh?.(); + protection.stop?.(); }); }); diff --git a/tests/protect/generated-rule-chain.test.ts b/tests/protect/generated-rule-chain.test.ts index 54e5c171..c59bd61a 100644 --- a/tests/protect/generated-rule-chain.test.ts +++ b/tests/protect/generated-rule-chain.test.ts @@ -143,7 +143,7 @@ describe('generated coordinate-pinned rule, through Pulse and the HTTP guard', ( expect((await p.fetch(appHandler)(req(THIRD_PARTY))).status).toBe(200); expect((await p.fetch(appHandler)(req(OTHER_ROUTE))).status).toBe(200); - p.stopRefresh?.(); + p.stop?.(); }); it('is refused at the gate when its scope names no key the engine knows', async () => { @@ -225,7 +225,7 @@ describe('generated coordinate-pinned rule, through Pulse and the HTTP guard', ( expect((await p.fetch(appHandler)(req(SSRF))).status).toBe(200); expect(detections.length, 'an unbound template cannot match anything').toBe(0); - p.stopRefresh?.(); + p.stop?.(); }); it('binds the fixture from a real serve, with no placeholder left in it', () => { diff --git a/tests/protect/pulse-chain.test.ts b/tests/protect/pulse-chain.test.ts index e616081b..e0c93ca3 100644 --- a/tests/protect/pulse-chain.test.ts +++ b/tests/protect/pulse-chain.test.ts @@ -98,6 +98,6 @@ describe('static-rule-through-Pulse chain (HTTP guard + manual refresh promotion expect(calls[calls.length - 1]).toMatchObject({ status: 304, ifNoneMatch: '"v2"' }); expect(p.mode).toBe('block'); // 304 keeps the last-known-good enforcement - p.stopRefresh?.(); + p.stop?.(); }); }); diff --git a/tests/protect/refresh-manifest.test.ts b/tests/protect/refresh-manifest.test.ts index 5f1040a3..87ee982f 100644 --- a/tests/protect/refresh-manifest.test.ts +++ b/tests/protect/refresh-manifest.test.ts @@ -86,7 +86,7 @@ describe('createProtection refresh re-posts the manifest (sandbox path)', () => expect(calls.some((c) => c.startsWith('POST') && c.includes('/manifest/'))).toBe(true); expect(calls.some((c) => c.includes('/rules/'))).toBe(true); } finally { - protection.stopRefresh?.(); + protection.stop?.(); await rm(dir, { recursive: true, force: true }); vi.restoreAllMocks(); } diff --git a/tests/protect/runtime-pulse.test.ts b/tests/protect/runtime-pulse.test.ts index dbc77974..ebdd5505 100644 --- a/tests/protect/runtime-pulse.test.ts +++ b/tests/protect/runtime-pulse.test.ts @@ -161,7 +161,7 @@ describe('createProtection Pulse enforcement field', () => { expect(protection.mode).toBe('block'); expect((await createServerFnGuard({ protection })({ title: '' }))?.rule).toBe('rm-npm-0001'); - protection.stopRefresh?.(); + protection.stop?.(); vi.useRealTimers(); }); }); @@ -196,7 +196,7 @@ describe('createProtection live rule refresh (refreshMs)', () => { expect(protection.rules.request.length).toBeGreaterThan(0); expect((await guard({ title: '' }))?.rule).toBe('rm-npm-0001'); - protection.stopRefresh?.(); + protection.stop?.(); vi.useRealTimers(); vi.restoreAllMocks(); }); diff --git a/tests/protect/stop-refresh-scope.test.ts b/tests/protect/stop-refresh-scope.test.ts new file mode 100644 index 00000000..61b0c2fe --- /dev/null +++ b/tests/protect/stop-refresh-scope.test.ts @@ -0,0 +1,85 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { createProtection } from '../../src/protect/runtime.js'; + +// `stopRefresh()` ends the rule refresh and nothing else; `stop()` ends everything, including this +// protection's outbound screening. + +const URL_OPT = 'https://x.test/monitor/pulse'; +const CREDENTIAL = 'a-credential-long-enough-to-be-accepted-1234'; +const SITE = '44444444-4444-4444-8444-444444444444'; +const RULES = { + firewall: [{ id: 'live-1', rule_v2: [{ parameter: 'get.q', match: { type: 'contains', value: 'x' } }] }], + whitelists: [], + whitelist_keys: {}, +}; + +function rulesService(available: boolean) { + let fetches = 0; + vi.stubGlobal( + 'fetch', + vi.fn(async (url: string) => { + if (!String(url).includes('/rules/')) return new Response('{}', { status: 200 }); + fetches += 1; + if (!available) return new Response('unavailable', { status: 503 }); + + return new Response(JSON.stringify(RULES), { status: 200, headers: { etag: `"v${fetches}"` } }); + }), + ); + + return { fetches: () => fetches }; +} + +const live = (extra: Record = {}) => + createProtection({ siteUuid: SITE, pulseAuth: CREDENTIAL, pulseRulesUrl: URL_OPT, mode: 'block', reportManifest: false, ...extra }); + +const originalFetch = globalThis.fetch; + +afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + globalThis.fetch = originalFetch; +}); + +describe('stopRefresh()', () => { + it('stops the poll loop', async () => { + vi.useFakeTimers(); + const service = rulesService(true); + const p: any = await live({ refreshMs: 1000 }); + + await vi.advanceTimersByTimeAsync(3500); + const polled = service.fetches(); + expect(polled).toBeGreaterThan(1); + + await p.stopRefresh(); + await vi.advanceTimersByTimeAsync(60_000); + expect(service.fetches()).toBe(polled); + await p.stop(); + }); + + it('stops the retries after an unclean start', async () => { + vi.useFakeTimers(); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + const service = rulesService(false); + const p: any = await live(); + expect(service.fetches()).toBe(1); + + await p.stopRefresh(); + await vi.advanceTimersByTimeAsync(60 * 60 * 1000); + expect(service.fetches()).toBe(1); + await p.stop(); + }); + + it('leaves outbound screening in place, which stop() then removes', async () => { + const stub = (async () => new Response('stub')) as typeof fetch; + globalThis.fetch = stub; + const p: any = await createProtection({ egress: true, mode: 'block' }); + + await p.stopRefresh(); + await expect(fetch('http://127.0.0.1/admin')).rejects.toThrow(/Patchstack blocked/); + + await p.stop(); + expect(globalThis.fetch).toBe(stub); + expect(await (await fetch('http://127.0.0.1/admin')).text()).toBe('stub'); + }); +});