feat(security): SSRF guard for outbound webhooks + partner postbacks - #64
Open
keithfawcett wants to merge 2 commits into
Open
feat(security): SSRF guard for outbound webhooks + partner postbacks#64keithfawcett wants to merge 2 commits into
keithfawcett wants to merge 2 commits into
Conversation
Both server-side fetch sinks took attacker-influenced URLs with only syntactic (z.string().url()) validation: webhook endpoint URLs (tenant admin) and partner postback URLs (partner — lower privilege), then fetched them with no host/IP restriction and default redirect-following. Neither is fully blind — the webhook test route returns status/error and postbacks store lastStatus/lastError — so both were usable as internal port-scan / metadata oracles (169.254.169.254, RFC1918, loopback, …). Adds a shared block-by-default guard (outbound-guard.ts) applied at both sinks: - http/https only, no embedded credentials, port on the policy allowlist. - Resolve EVERY address; reject unless each is globally-routable public unicast (ipaddr.js). Rejects IPv4-mapped IPv6, and octal/hex/decimal IPv4 encodings (WHATWG URL normalizes them before classification). Special-use hostnames (localhost/.local/.internal/.home.arpa/single-label) refused. - DNS-pin the validated address into a per-request undici dispatcher's connect.lookup so DNS can't rebind between check and connect; the original URL still supplies Host/SNI/cert identity. - redirect: 'manual', zero hops — a 3xx is a non-delivery. Blocked destinations throw OutboundBlockedError with a stable code (safe to store in partner-visible lastError), recorded as a failed delivery. Policy is deployment-scoped: hosted locks ports to 80/443; selfhost+single leaves ports open and honors OPENPARTNER_OUTBOUND_ALLOW_PRIVATE_CIDRS (an escape hatch that FAILS STARTUP if set in any other mode). Checked prod first: all 9 configured webhooks are public HTTPS/443 and there are 0 postbacks, so block-by-default breaks nothing live. Design reviewed adversarially with Codex. Deps: undici + ipaddr.js. Tests: outbound-guard.test.ts (schemes/ports/creds/IP classes/encodings/hostnames/ escape hatch + a live-server check that a blocked host never gets a socket); existing webhook + postback integration tests set the selfhost escape hatch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ail-fast boot Two findings from Codex review of the SSRF guard: 1. (HIGH-when-enabled) The private-CIDR escape hatch was shared with partner-controlled postbacks, so an operator opening an internal range for an admin webhook also handed every partner a scan/request oracle across it. safeFetch now takes a trust level: partner postbacks use a hardened policy (escape hatch stripped, ports never unrestricted — 80/443 fallback); admin webhooks keep the full deployment policy. Callers pass trust: 'admin' (webhook-dispatcher) / 'partner' (partner-postback). 2. (LOW) "Refuses to boot on bad config" was documented but not enforced — the policy built lazily on first delivery. server.ts now calls outboundPolicy() before listen(), so a misconfigured OPENPARTNER_OUTBOUND_* fails startup. Tests: new guard case asserting admin honors the escape hatch while partner does not (and never opens a socket); the postback integration test uses the new __setOutboundPolicyForTests override for its localhost receiver (the security split itself is covered in outbound-guard.test.ts). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Both server-side fetch sinks accepted attacker-influenced URLs with only syntactic (
z.string().url()) validation, then fetched them with no host/IP restriction and default redirect-following:webhook-dispatcher.ts→ tenant-admin-configured webhook URLs (/webhooks/:id/testfires on demand)partner-postback.ts→ partner-configured postback URLs (lower privilege), fetched after macro substitutionNeither is fully blind — the webhook test route returns status/error and postbacks store
lastStatus/lastError— so both were usable as internal port-scan / cloud-metadata oracles (169.254.169.254, RFC1918, loopback, …). Found via an unsteered Codex review; mitigation design pressure-tested with Codex.The guard (
outbound-guard.ts, block-by-default)http/httpsonly, no embedded credentials, port on the policy allowlist.ipaddr.js). Rejects IPv4-mapped IPv6 and octal/hex/decimal IPv4 encodings (WHATWG URL normalizes them before we classify). Special-use hostnames (localhost,.local,.internal,.home.arpa, single-label) refused without a DNS trip.undicidispatcher'sconnect.lookupso DNS can't rebind between check and connect; the original URL still supplies Host/SNI/cert identity (only socket resolution is substituted).redirect: 'manual', zero hops — a 3xx is a non-delivery (following would re-open the hole).Blocked destinations throw
OutboundBlockedErrorwith a stable code (safe for the partner-visiblelastError), recorded as a failed delivery via the existing catch paths.Policy is deployment-scoped
OPENPARTNER_OUTBOUND_ALLOW_PRIVATE_CIDRS(comma-separated CIDRs) for localhost testing. This env fails startup if set in any other mode.OPENPARTNER_OUTBOUND_ALLOWED_PORTSlets an operator widen ports (e.g.80,443,8443). Not tenant-configurable.Blast-radius check (done before choosing block-by-default)
Queried prod read-only: all 9 configured webhook endpoints are public HTTPS on 443 (Zapier ×7, xispark.com, a Supabase edge function) and 0 partner postbacks exist. So block-by-default breaks nothing currently live.
Tests
outbound-guard.test.ts: schemes, ports, credentials, every private/loopback/link-local/CGNAT/unspecified class, IPv4-mapped IPv6, octal/hex/decimal loopback encodings, special-use hostnames, the port allowlist, the private-CIDR escape hatch, the env fail-closed check, and a live-server test proving a blocked host never receives a socket (+ that the escape hatch reaches it). The existing webhook + postback integration suites set the selfhost escape hatch for their localhost receivers. Full API suite green; typecheck + lint (0 errors) clean.Notes / follow-ups
undici,ipaddr.js.undici@6, which runs on both 20 and 22, so the two PRs are independent).🤖 Generated with Claude Code