fix(egress): unify SSRF/egress behind one policy with named provenance profiles - #7305
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@greptile review |
There was a problem hiding this comment.
All reported issues were addressed across 177 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
Greptile SummaryThis PR replaces several inconsistent SSRF and outbound-networking controls with a unified, provenance-aware egress policy.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains within the scope of this follow-up review. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/security/src/egress.ts | Defines the pure egress-policy model and centralized decisions for schemes, ports, destination addresses, metadata endpoints, and allowlists. |
| apps/sim/lib/core/security/egress/profiles.ts | Maps URL provenance profiles to hosted and self-hosted egress policies. |
| apps/sim/lib/core/security/input-validation.ts | Integrates profile-aware validation into the application’s outbound request boundary. |
| apps/sim/lib/core/security/http-redirect-policy.ts | Centralizes redirect method, body, header, and cross-origin credential handling. |
| apps/sim/lib/mcp/pinned-fetch.ts | Applies pinned egress validation to MCP traffic while separating configured-server and cross-origin OAuth provenance. |
| scripts/check-egress-boundary.ts | Adds an audit that detects unguarded HTTP transports outside the approved egress boundary. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
URL[Outbound URL] --> Profile[Select provenance profile]
Profile --> Scheme[Check scheme and port]
Scheme --> Resolve[Resolve and pin destination]
Resolve --> Address[Check address and metadata policy]
Address --> Request[Send guarded request]
Request --> Redirect{Redirect?}
Redirect -- No --> Response[Return response]
Redirect -- Yes --> Headers[Apply credential and body policy]
Headers --> Scheme
Reviews (3): Last reviewed commit: "fix(egress): unify SSRF/egress behind on..." | Re-trigger Greptile
2d257d6 to
78b8868
Compare
78b8868 to
a539c1e
Compare
a539c1e to
7de005b
Compare
…e profiles Fixes #7200. Self-hosted Sim could not reach a private destination — a LAN vLLM, a Jupyter server, GitHub Enterprise, or `host.docker.internal` — because plain HTTP was permitted only for literal loopback and every private address was dropped with no operator opt-out, and the error named the wrong cause. This replaces a hardcoded default plus four incompatible escape hatches (`allowHttp`, an `isLocalhost && !isHosted` hardcode, `allowRedirectToIp`, `ALLOW_PRIVATE_DATABASE_HOSTS`) with one pure policy core, six provenance profiles that name where a URL came from, and two operator allowlists (`EGRESS_ALLOWED_HOSTS`, `EGRESS_ALLOWED_IP_RANGES`) that are ignored on Sim Cloud. Cloud-metadata endpoints are never liftable. `ALLOW_PRIVATE_DATABASE_HOSTS` stays as a deprecated alias so existing self-hosters keep working.
7de005b to
77d3516
Compare
|
Final cleanliness pass applied: moved a stranded TSDoc back onto createPinnedPrivateMcpFetch, de-duplicated the transport href helper, and tightened the redirects doc to state the drop-all-headers-by-default behavior. No logic change. |
Fixes #7200. Supersedes #7229 (same work, rebuilt clean on current
stagingafter a full-diff audit).The bug
The issue is filed as a Docker networking problem. The screenshot shows the real failure —
url must use https:// protocolonhttp://host.docker.internal:7274/.... Sim refused the URL before opening a socket, and the message named the wrong cause: switching tohttpswould have failed too, on the private-address check one step later.Plain http was permitted only for a literal
localhostor loopback IP. In a container loopback is the one address guaranteed useless — it is the container itself — so the carve-out existed exactly where it could not help. Behind it, the DNS filter dropped every private address with no operator opt-out, so self-hosted Sim could not reach a LAN vLLM, a Jupyter server, GitHub Enterprise, or a sibling container by service name either.Why it needed more than a patch
There was no policy object — just a hardcoded default plus four incompatible escape hatches grown one per use case:
allowHttpisLocalhost && !isHostedallowRedirectToIpALLOW_PRIVATE_DATABASE_HOSTSThey disagreed.
allowHttp: truerelaxed the protocol gate while the address gate refused the host anyway — so vLLM and Jupyter on a LAN were broken despite passing it, while 1Password and ClickHouse worked because they route through different validators with their own rules.The change
@sim/security/egress— a pure policy value and two decision functions. No DNS, no env, no deployment-posture global. That is what lets both postures be tested in one file with no module mocking; the hosted branch of the guard had no coverage at all before, becauseisHostedwas a module const the suite pinned tofalse.lib/core/security/egress— profiles keyed on where the URL came from, because provenance is what determines trust:configuredEndpointselfHostedServicerequestTargetfetchdatabaseHostcontentFetchproxycontentFetchearns the taxonomy: it is the class where SSRF is actually exploited, so it stays locked even on a deployment that allowlisted its whole internal range.databaseHostgets no loopback carve-out because loopback is where Sim's own database listens.The profile is required at all ~190 call sites and travels on
SecureFetchOptions, so every redirect hop is judged by the policy the request started under.allowHttpandallowRedirectToIpare deleted.Configuration
EGRESS_ALLOWED_HOSTS=host.docker.internal,*.svc.cluster.local EGRESS_ALLOWED_IP_RANGES=10.0.0.0/8Naming a destination permits plain http to it and lifts the blocked-port list for it — one decision about one host, not three switches. Cloud metadata endpoints stay blocked however broad the allowlist is. Both variables are ignored on the hosted platform. A host entry takes a hostname only (no port/CIDR), and a range shorter than
/8is refused as a near-catch-all.Allowlist only: no configurable blocklist. n8n exposes one and it is a footgun with no use case an allowlist does not serve better.
Compatibility
ALLOW_PRIVATE_DATABASE_HOSTSkeeps working as a deprecated alias that expands to the private space it always stood for (scoped todatabaseHost), so existing deployments are unaffected. It logs a deprecation warning at startup and is documented only in the upgrade note.Breaking changes
Self-hosted only, each with the same remedy — name the destination in
EGRESS_ALLOWED_HOSTS/EGRESS_ALLOWED_IP_RANGES:ALLOWED_MCP_DOMAINSsetThree with no allowlist remedy, because the provenance is content rather than configuration:
url_privatecontentFetchnever consults the allowlistproxyUrlon a private/loopback addressHosted (Sim Cloud): plain
http://endpoints for vLLM, Jupyter, 1Password and MCP servers, previously allowed viaallowHttp, are now refused — nothing is vouched on hosted, so a credential would cross the wire in the clear. Self-hosted keeps plain HTTP for these viaselfHostedService.And three that are the same for every deployment:
Authorization,Cookieandproxy-authorizationunlesssendCredentialsOnCrossOriginRedirectis set; previously any caller that passed no redirect policy forwarded them.allowCrossOriginBodyis set (a standard 301/302/303 drops the body to a GET first and continues).Error-message text changed throughout (each now names the cause and the remedy), so any automation matching the old strings should be updated.
One loosening, self-hosted only: naming a destination lifts the blocked-port list for it, so an allowlisted host exposes 22/3306/6379 on that host to every workflow author. The loopback carve-out deliberately does not —
http://localhost:5432stays refused untillocalhostis named.Parallel implementations retired
One policy governs every outbound request now.
allowRedirectToIp, the 1Password self-classifier, and the MCPdomain-checkSSRF half are gone — the last of which had two paths that ran with no guard at all, one wheneverALLOWED_MCP_DOMAINSwas set, leaving an allowlisted domain free to redirect anywhere including cloud metadata. Domain governance and the address check are separate questions and both apply now.Full-diff audit (this PR vs #7229)
The whole diff was re-audited line by line with a swarm of parallel subagents before this PR was opened. Confirmed findings, fixed and tested here:
fetchfor its internalauth()legs, whose URLs come from the server'sWWW-Authenticate/metadata. Those ran underselfHostedService. They are now judged ascontentFetchfor any cross-origin request, so a hostile server cannot steer an authenticated token POST at an allowlisted or loopback host, and the private-IP pin is never reused for a cross-origin auth hop.168.63.129.16is now blocked as a metadata endpoint.fetch.EGRESS_ALLOWED_IP_RANGESrejects a prefix shorter than/8;EGRESS_ALLOWED_HOSTSrejects an entry with a port, comma or colon.host.docker.internal:host-gateway; the two redirect caps are unified.Cleanup
Net production code shrinks. Removes nine dead exports, three copies of "validate then host-suffix allowlist", a duplicate of
validateJiraCloudId, and the secondvalidateUrlWithDNSin the API block handler — it discarded its pinned address and validated a pre-templating string that was not the URL dialled. Addscheck:egress-boundary, which fails CI if a raw HTTP transport appears outside the guard (auto-enrolled intocheck:audits).Verification
bunx turbo run type-check— 26/26 cleanbun run check:audits— 40/40 passbun run check:egress-boundary— 12,293 files, cleanbun run lint:helm— cleanapps/simsuite — 37,905 passed, 0 failed@sim/security— 268 passed, including the new egress cases (metadata across every IPv6 embedding form, near-catch-all ranges, Teredo/ISATAP, both postures)