diff --git a/AGENT-INSTALL.md b/AGENT-INSTALL.md index dc40cdbc..2604c236 100644 --- a/AGENT-INSTALL.md +++ b/AGENT-INSTALL.md @@ -466,7 +466,8 @@ whatever supplied it. `client_ip_source` is one of `runtime` (the address the tr `unavailable`. When it is `unavailable` the `client_ip` field is **omitted entirely** rather than sent empty, so a missing address cannot read as a failed lookup of a real one. A forwarded header is never trusted implicitly: with no `trustedProxy` policy the address is whatever the transport observed, and in a -runtime that exposes no transport peer there is no address to report at all. +runtime that exposes no transport peer there is no address to report at all unless your code supplies one +with `peerAddress` (below). ### Behaviour change: how the client address is determined @@ -481,11 +482,17 @@ Two consequences if you are upgrading: peer. **If your app runs behind a proxy or load balancer, addresses will now show as the proxy's** until you declare your proxies with `trustedProxy` (below) — which affects attribution in reports and any rule matching on `server.ip` or `REMOTE_ADDR`. -- **Fetch runtimes report no address at all.** A WHATWG `Request` exposes no transport peer, so a Fetch - guard (Workers, Deno, Bun, edge) has nothing to observe, and no forwarded header is accepted in its - place under any `trustedProxy` policy: `client_ip_source` is `unavailable` and no address is sent. +- **Fetch runtimes report no address unless you supply the peer.** A WHATWG `Request` exposes no + transport peer, so a Fetch guard (Workers, Deno, Bun, edge) has nothing to observe on its own, and no + forwarded header is accepted in its place: `client_ip_source` is `unavailable` and no address is sent. Earlier versions reported the forwarded header here, so an address-scoped rule that appeared to work on - such a runtime was matching a client-supplied value. + such a runtime was matching a client-supplied value. Where your runtime does know the peer, pass + `peerAddress: (request, ...handlerArgs) => string` — for example `(req, info) => info.remoteAddr.hostname` + on Deno, or `(req, server) => server.requestIP(req)?.address` on Bun. It receives the request and the + arguments your handler was called with (`fetchGuard()(request, ...args)` and + `screenResponse(response, request, ...args)` pass them on), and that address then counts as the + transport peer, including for `trustedProxy`. With `trustedProxy` set and no peer supplied, the guard + warns once, because the policy can never apply. `trustedProxy` is the only way to make a forwarded header count. It takes the proxies you actually run — `{ peers: ['10.0.0.0/8'] }`, or `{ hops: 1 }` to trust that many hops in from the peer, plus optional diff --git a/src/protect/protect.d.ts b/src/protect/protect.d.ts index 1047bb30..4ed86e06 100644 --- a/src/protect/protect.d.ts +++ b/src/protect/protect.d.ts @@ -15,8 +15,9 @@ export interface Protection { mode: "block" | "dry-run"; /** Active rules split by phase. */ rules: { request: unknown[]; response: unknown[]; egress: unknown[] }; - /** (request) => Response (403 when blocked) | null (allow / dry-run). Request phase only. */ - fetchGuard(): (request: Request) => Promise; + /** (request, ...hostArgs) => Response (403 when blocked) | null (allow / dry-run). Request phase only. + * Any further arguments are the host's handler arguments, passed on to `peerAddress`. */ + fetchGuard(): (request: Request, ...hostArgs: unknown[]) => Promise; /** Screens the request, then the response (secret-leak redaction / withhold). */ fetch(handler: (request: Request, ...rest: unknown[]) => unknown): (request: Request, ...rest: unknown[]) => Promise; /** @@ -25,8 +26,11 @@ export interface Protection { * Pass the originating `request` wherever it is available. A response rule can be scoped to a route or a * method (`when`), and that scope can only be applied if the engine is given the request the response * belongs to — without it, a scoped response rule is delivered, counted as protection, and never matches. + * + * The client address is the one resolved when this guard screened that request, if it did; otherwise it + * is resolved here, and any further arguments are passed to `peerAddress` as the host's handler arguments. */ - screenResponse(response: Response, request?: Request): Promise; + screenResponse(response: Response, request?: Request, ...hostArgs: unknown[]): 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. */ @@ -273,11 +277,20 @@ export interface CreateProtectionOptions { read(): unknown | Promise; write(envelope: unknown): unknown | Promise; }; + /** + * The transport peer of a Fetch request, for runtimes where the host knows it and the `Request` does + * not — e.g. `(req, info) => info.remoteAddr.hostname` on Deno, `(req, server) => server.requestIP(req)?.address` + * on Bun. Called with the request the host served and the arguments its handler received (passed through + * `fetch(handler)`, `fetchGuard()(request, ...args)` and `screenResponse(response, request, ...args)`). + * The result counts as the peer for client address resolution, including `trustedProxy`. A throw, or a result that is not an address, supplies no peer. + */ + peerAddress?: (request: Request, ...hostArgs: unknown[]) => string | null | undefined; /** * Declare which peers are this deployment's own reverse proxies, so a forwarded header can be believed. * - * With no policy, the client address is whatever the transport observed — the socket peer on Node, and - * nothing at all in a runtime that exposes no peer, where the provenance reads `unavailable`. A + * With no policy, the client address is whatever the transport observed — the socket peer on Node, the + * `peerAddress` result on a Fetch runtime, and nothing at all where neither is available, in which case + * the provenance reads `unavailable` (and, with a policy set, the guard warns once). A * forwarded header is never trusted implicitly: it is ordinary request input that any caller can send. * * A policy must say WHO is trusted, not just which header to read. Declare at least one of: @@ -385,7 +398,7 @@ export function createSupabaseGuard(opts: { maxBodyBytes?: number; /** Maximum upstream request duration. Default 30 seconds. */ timeoutMs?: number; -}): (request: Request) => Promise; +}): (request: Request, ...hostArgs: unknown[]) => Promise; /** * Server-function guard: inspect a TanStack server function's decoded args against the same diff --git a/src/protect/runtime.js b/src/protect/runtime.js index 51575f02..3acfa2a5 100644 --- a/src/protect/runtime.js +++ b/src/protect/runtime.js @@ -38,6 +38,7 @@ import { createDetectionReporter } from './detections.js'; import { reportingState } from './reporting-state.js'; import { hardensWithoutBody } from './response-hardening.js'; import { notify } from './notify.js'; +import { SOURCE_REQUEST } from './supabase-guard.js'; import { createFirewallLogReporter, resolveApiBase, telemetryEnabled } from './firewall-log.js'; // Supabase-tunnel guard for AI-builder apps (Lovable / TanStack Start + Supabase). @@ -687,20 +688,74 @@ export async function createProtection(options = {}) { }; }; + /** + * The transport peer for a Fetch request, from the host's `peerAddress` callback. + * + * A WHATWG Request carries no peer, so on a Fetch runtime the address is only known to the host (Deno's + * handler info, Bun's server, a Node adapter's socket). The callback gets the request the host served — + * for a request rebuilt from another, the original — and the host's own handler arguments. What it + * returns is validated as an address by the resolver; a callback that throws supplies no peer. + */ + const peerOf = (request, hostArgs) => { + if (typeof options.peerAddress !== 'function') return undefined; + try { + return options.peerAddress(request?.[SOURCE_REQUEST] ?? request, ...hostArgs); + } catch (err) { + notify(onError, err, 'onError'); + + return undefined; + } + }; + // A trust policy names the peers a forwarded header may be believed from, so without a usable peer it + // can never apply and every address reads `unavailable`. Said once per guard, because it holds for + // every request. + let warnedNoPeer = false; + const warnNoPeer = () => { + if (warnedNoPeer) return; + warnedNoPeer = true; + const message = + '[patchstack] trustedProxy is set, but this Fetch runtime supplied no peer address, so forwarded ' + + 'headers are not used and client addresses read as unavailable. Pass { peerAddress } to supply one.'; + if (typeof onError === 'function') notify(onError, new Error(message), 'onError'); + else console.warn(message); + }; + // The address resolved for each screened Fetch request, so a later response screen for the same request + // names the same client rather than resolving again without the host's arguments. + const clientByRequest = new WeakMap(); + // The client for a response screened on its own: the request phase's answer when there was one, + // otherwise resolved here the same way the request phase would have. + const clientForResponse = (request, hostArgs) => { + const known = clientByRequest.get(request); + if (known) return known; + const client = resolveClientIp({ + peer: peerOf(request, hostArgs), + headers: headerObject(request.headers), + trustedProxy: options.trustedProxy, + }); + if (options.trustedProxy !== undefined && client.source === 'unavailable') warnNoPeer(); + + return client; + }; + /** * Screen a fetch request once, and hand back both the decision and the address it resolved. * * Shared by `fetchGuard()` and `fetch(handler)` so the response phase can reuse the request phase's * resolution instead of making its own. */ - const screenFetchRequest = async (request) => { + const screenFetchRequest = async (request, hostArgs = []) => { let result; let shaped; try { shaped = await fromFetchRequest(request, { + peer: peerOf(request, hostArgs), trustedProxy: options.trustedProxy, maxBodyBytes: options.maxBodyBytes, }); + if (shaped?._clientIp) { + clientByRequest.set(request, shaped._clientIp); + if (options.trustedProxy !== undefined && shaped._clientIp.source === 'unavailable') warnNoPeer(); + } if (shaped?._bodyInspectionSkip) { recordSkip('request', shaped._bodyInspectionSkip, { limit: options.maxBodyBytes ?? 1024 * 1024 }); } @@ -1442,20 +1497,12 @@ export async function createProtection(options = {}) { // .fetch(), and by the Supabase guard on its forwarded upstream response. // A standalone response screen with no request phase of its own — the Supabase guard's forwarded // upstream response. It resolves once here, which is the only resolution for this call. - screenResponse: (response, request) => - screenResp( - response, - request - ? reqContextFromFetch( - request, - resolveClientIp({ headers: headerObject(request.headers), trustedProxy: options.trustedProxy }), - ) - : undefined, - ), + screenResponse: (response, request, ...hostArgs) => + screenResp(response, request ? reqContextFromFetch(request, clientForResponse(request, hostArgs)) : undefined), // (request) => Response | null (null = allow, caller proceeds). Request phase only. fetchGuard() { - return async (request) => (await screenFetchRequest(request)).blocked; + return async (request, ...hostArgs) => (await screenFetchRequest(request, hostArgs)).blocked; }, // Wrap a fetch handler: screens the request, then the response (redact/block). @@ -1465,7 +1512,7 @@ export async function createProtection(options = {}) { // screening making a second one. Two resolutions for one request can disagree, and a response // detection naming a different address than the request detection describes two clients that do // not exist. - const { blocked, client } = await screenFetchRequest(request); + const { blocked, client } = await screenFetchRequest(request, rest); if (blocked) return blocked; const response = await handler(request, ...rest); diff --git a/src/protect/supabase-guard.js b/src/protect/supabase-guard.js index b22d4a1b..462a1213 100644 --- a/src/protect/supabase-guard.js +++ b/src/protect/supabase-guard.js @@ -88,6 +88,9 @@ function responseHeaders(input) { return output; } +/** Marks a request rebuilt from another with the request the host served. */ +export const SOURCE_REQUEST = Symbol.for('@patchstack/connect.sourceRequest'); + /** * @param {object} opts * @param {{ fetchGuard: () => (req: Request) => Promise }} opts.protection a createProtection() result @@ -95,7 +98,7 @@ function responseHeaders(input) { * @param {typeof fetch} [opts.fetchImpl] injectable fetch (tests) * @param {number} [opts.maxBodyBytes] maximum tunneled request body size * @param {number} [opts.timeoutMs] maximum upstream request duration - * @returns {(request: Request) => Promise} + * @returns {(request: Request, ...hostArgs: unknown[]) => Promise} */ export function createSupabaseGuard({ protection, @@ -109,7 +112,7 @@ export function createSupabaseGuard({ const bodyLimit = Number.isFinite(maxBodyBytes) && maxBodyBytes > 0 ? maxBodyBytes : DEFAULT_MAX_BODY_BYTES; const upstreamTimeout = Number.isFinite(timeoutMs) && timeoutMs > 0 ? timeoutMs : DEFAULT_TIMEOUT_MS; - return async function handleGuardRequest(request) { + return async function handleGuardRequest(request, ...hostArgs) { const target = request.headers.get('x-ps-target'); if (!target) return new Response('patchstack: missing x-ps-target', { status: 400 }); @@ -148,7 +151,9 @@ export function createSupabaseGuard({ headers: request.headers, body, }); - const blocked = await guard(evalReq); + // The rebuilt request stands for the one the host served, so the peer is read from that one. + Object.defineProperty(evalReq, SOURCE_REQUEST, { value: request }); + const blocked = await guard(evalReq, ...hostArgs); if (blocked) return blocked; // Allowed → forward to Supabase, server-side. diff --git a/tests/protect/fetch-peer-address.test.ts b/tests/protect/fetch-peer-address.test.ts new file mode 100644 index 00000000..611f7f15 --- /dev/null +++ b/tests/protect/fetch-peer-address.test.ts @@ -0,0 +1,219 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { createProtection, createSupabaseGuard, GUARD_PATH } from '../../src/protect/runtime.js'; + +// A Fetch runtime's Request carries no transport peer. `peerAddress` lets the host supply the one it +// knows, and that address then counts as the peer, including for a declared proxy policy. + +const addressRule = () => ({ + firewall: [{ + id: 'address-under-test', title: 'address under test', + rule_v2: [{ parameter: 'server.ip', match: { type: 'contains', value: '198.51.100.' } }], + }], + whitelists: [], + whitelist_keys: {}, +}); + +async function guardWith(options: Record) { + const detections: any[] = []; + const errors: unknown[] = []; + const protection: any = await createProtection({ + rules: addressRule(), + mode: 'dry-run', + onDetect: (event: any) => detections.push(event), + onError: (err: unknown) => errors.push(err), + ...options, + }); + return { protection, detections, errors }; +} + +const request = (headers: Record = {}) => new Request('https://app.example.test/', { headers }); + +afterEach(() => vi.restoreAllMocks()); + +describe('Fetch peer address', () => { + it('uses the address the host supplies', async () => { + const { protection, detections } = await guardWith({ peerAddress: () => '198.51.100.7' }); + await protection.fetchGuard()(request()); + expect(detections).toHaveLength(1); + expect(detections[0]).toMatchObject({ ip: '198.51.100.7', clientIpSource: 'runtime' }); + await protection.stop(); + }); + + it('passes the host handler arguments to the callback', async () => { + const info = { remoteAddr: { hostname: '198.51.100.9' } }; + const peerAddress = vi.fn((_req: Request, hostInfo: any) => hostInfo.remoteAddr.hostname); + const { protection, detections } = await guardWith({ peerAddress }); + const served = request(); + await protection.fetch(async () => new Response('sample'))(served, info); + expect(peerAddress).toHaveBeenCalledWith(served, info); + expect(detections[0]).toMatchObject({ ip: '198.51.100.9', clientIpSource: 'runtime' }); + await protection.stop(); + }); + + it('applies a declared proxy policy to the supplied peer', async () => { + const { protection, detections, errors } = await guardWith({ + peerAddress: () => '10.0.0.5', + trustedProxy: { peers: ['10.0.0.0/8'] }, + }); + await protection.fetchGuard()(request({ 'x-forwarded-for': '198.51.100.20' })); + expect(detections[0]).toMatchObject({ ip: '198.51.100.20', clientIpSource: 'trusted-proxy' }); + expect(errors).toHaveLength(0); + await protection.stop(); + }); + + it('does not believe a forwarded header from a peer outside the policy', async () => { + const { protection, detections } = await guardWith({ + peerAddress: () => '203.0.113.5', + trustedProxy: { peers: ['10.0.0.0/8'] }, + }); + await protection.fetchGuard()(request({ 'x-forwarded-for': '198.51.100.20' })); + expect(detections).toHaveLength(0); + await protection.stop(); + }); + + it('keeps the unavailable result without a callback, and warns once when a policy is set', async () => { + const { protection, detections, errors } = await guardWith({ trustedProxy: { peers: ['10.0.0.0/8'] } }); + await protection.fetchGuard()(request({ 'x-forwarded-for': '198.51.100.20' })); + await protection.fetchGuard()(request({ 'x-forwarded-for': '198.51.100.21' })); + expect(detections).toHaveLength(0); + expect(errors).toHaveLength(1); + expect(String((errors[0] as Error).message)).toContain('peerAddress'); + await protection.stop(); + }); + + it('warns on the console when no onError is given', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const protection: any = await createProtection({ rules: addressRule(), trustedProxy: { hops: 1 } }); + await protection.fetchGuard()(request()); + await protection.fetchGuard()(request()); + expect(warn.mock.calls.filter(([m]) => String(m).includes('peerAddress'))).toHaveLength(1); + await protection.stop(); + }); + + it.each([['a non-address', () => 'not-an-address'], ['a non-string', () => 42]])( + 'warns once with a policy when the callback returns %s', async (_label, peerAddress) => { + const { protection, errors } = await guardWith({ peerAddress, trustedProxy: { peers: ['10.0.0.0/8'] } }); + await protection.fetchGuard()(request()); + await protection.fetchGuard()(request()); + expect(errors).toHaveLength(1); + await protection.stop(); + }, + ); + + it('does not warn without a policy', async () => { + const { protection, errors } = await guardWith({}); + await protection.fetchGuard()(request()); + expect(errors).toHaveLength(0); + await protection.stop(); + }); + + it.each([ + ['throws', () => { throw new Error('sample'); }, 1], + ['returns a non-string', () => 42, 0], + ['returns an empty string', () => '', 0], + ])('supplies no peer when the callback %s', async (_label, peerAddress, errorCount) => { + const { protection, detections, errors } = await guardWith({ peerAddress }); + const blocked = await protection.fetchGuard()(request()); + expect(blocked).toBeNull(); + expect(detections).toHaveLength(0); + expect(errors).toHaveLength(errorCount); + await protection.stop(); + }); + + it('gives the response phase the address the request phase resolved', async () => { + const detections: any[] = []; + const protection: any = await createProtection({ + rules: { firewall: [], whitelists: [], whitelist_keys: {} }, + mode: 'dry-run', + peerAddress: () => '198.51.100.30', + onDetect: (event: any) => detections.push(event), + responseRules: [{ + id: 'response-under-test', phase: 'response', action: 'redact', + rule_v2: [{ parameter: 'response.body', match: { type: 'contains', value: 'SAMPLE_VALUE' } }], + }], + }); + const served = request(); + await protection.fetchGuard()(served); + await protection.screenResponse(new Response('SAMPLE_VALUE'), served); + expect(detections).toHaveLength(1); + expect(detections[0]).toMatchObject({ phase: 'response', ip: '198.51.100.30', clientIpSource: 'runtime' }); + await protection.stop(); + }); + + describe('response screening on its own', () => { + const responseGuard = async (options: Record) => { + const detections: any[] = []; + const protection: any = await createProtection({ + rules: { firewall: [], whitelists: [], whitelist_keys: {} }, + mode: 'dry-run', + onDetect: (event: any) => detections.push(event), + responseRules: [{ + id: 'response-under-test', phase: 'response', action: 'redact', + rule_v2: [{ parameter: 'response.body', match: { type: 'contains', value: 'SAMPLE_VALUE' } }], + }], + ...options, + }); + return { protection, detections }; + }; + + it('asks the callback when the request was not screened first', async () => { + const { protection, detections } = await responseGuard({ peerAddress: () => '198.51.100.50' }); + await protection.screenResponse(new Response('SAMPLE_VALUE'), request()); + expect(detections).toHaveLength(1); + expect(detections[0]).toMatchObject({ ip: '198.51.100.50', clientIpSource: 'runtime' }); + await protection.stop(); + }); + + it('passes the host arguments on and applies a declared proxy policy', async () => { + const peerAddress = vi.fn((_req: Request, server: any) => server.peer); + const { protection, detections } = await responseGuard({ peerAddress, trustedProxy: { peers: ['10.0.0.0/8'] } }); + const served = request({ 'x-forwarded-for': '198.51.100.51' }); + await protection.screenResponse(new Response('SAMPLE_VALUE'), served, { peer: '10.0.0.9' }); + expect(peerAddress).toHaveBeenCalledWith(served, { peer: '10.0.0.9' }); + expect(detections[0]).toMatchObject({ ip: '198.51.100.51', clientIpSource: 'trusted-proxy' }); + await protection.stop(); + }); + + it('warns once when a policy is set and no peer is supplied', async () => { + const errors: unknown[] = []; + const { protection } = await responseGuard({ trustedProxy: { peers: ['10.0.0.0/8'] }, onError: (e: unknown) => errors.push(e) }); + await protection.screenResponse(new Response('SAMPLE_VALUE'), request()); + await protection.screenResponse(new Response('SAMPLE_VALUE'), request()); + expect(errors).toHaveLength(1); + await protection.stop(); + }); + + it('reuses the request-phase address rather than asking again', async () => { + const peers = ['198.51.100.60', '198.51.100.61']; + const peerAddress = vi.fn(() => peers.shift()); + const { protection, detections } = await responseGuard({ peerAddress }); + const served = request(); + await protection.fetchGuard()(served); + await protection.screenResponse(new Response('SAMPLE_VALUE'), served); + expect(peerAddress).toHaveBeenCalledTimes(1); + expect(detections[0]).toMatchObject({ ip: '198.51.100.60' }); + await protection.stop(); + }); + }); + + it('reads the peer from the served request through the Supabase tunnel', async () => { + const supabase = 'https://project.supabase.example'; + const served = new Request('https://app.example.test' + GUARD_PATH, { + method: 'POST', + headers: { 'content-type': 'application/json', 'x-ps-target': supabase + '/rest/v1/items' }, + body: '{}', + }); + const peerAddress = vi.fn((req: Request, server: any) => server.requestIP(req)?.address); + const server = { requestIP: (req: Request) => (req === served ? { address: '198.51.100.40' } : null) }; + const { protection, detections } = await guardWith({ peerAddress }); + const handle = createSupabaseGuard({ + protection, + supabaseUrl: supabase, + fetchImpl: (async () => new Response('[]', { headers: { 'content-type': 'application/json' } })) as any, + }); + const response = await handle(served, server); + expect(response.status).toBe(200); + expect(detections[0]).toMatchObject({ ip: '198.51.100.40', clientIpSource: 'runtime' }); + await protection.stop(); + }); +});