Add domain allowlist to block SSRF via first-party proxy redirects

[prk-Jr]

Summary

• The first-party proxy followed redirects with no restriction on destination hosts. A signed URL pointing to an attacker-controlled origin could redirect to an internal service, enabling SSRF.
• Adds proxy.allowed_domains — an opt-in list of permitted redirect-target hostnames supporting exact match ("example.com") and subdomain wildcard ("*.example.com") with dot-boundary enforcement to prevent evil-example.com bypass.
• When the list is empty (the default), all redirect destinations are permitted — preserving backward compatibility for existing deployments.

Changes

File	Change
crates/common/src/proxy.rs	Add is_host_allowed helper; enforce allowlist on each redirect hop in proxy_with_redirects; 13 unit tests
crates/common/src/settings.rs	Add allowed_domains: Vec<String> field to Proxy struct with serde default
trusted-server.toml	Uncomment [proxy] section; add allowed_domains key with commented vendor examples
.env.dev	Document TRUSTED_SERVER__PROXY__ALLOWED_DOMAINS env var override

Closes

Closes #414

Test plan

• cargo test --workspace
• cargo clippy --all-targets --all-features -- -D warnings
• cargo fmt --all -- --check
• JS tests: cd crates/js/lib && npx vitest run
• JS format: cd crates/js/lib && npm run format
• WASM build: cargo build --bin trusted-server-fastly --release --target wasm32-wasip1
• Manual testing via fastly compute serve

Note: JS tests fail with ERR_REQUIRE_ESM due to a pre-existing jsdom@28 / html-encoding-sniffer incompatibility on main — unrelated to this PR.

Checklist

• Changes follow CLAUDE.md conventions
• No unwrap() in production code — use expect("should ...")
• Uses log macros (not println!)
• New code has tests
• No secrets or credentials committed

[ChristianPavilonis]

Nice hardening update overall — the redirect allowlist behavior and wildcard boundary checks look solid.

I found two follow-ups worth addressing:

1. Medium: proxy.allowed_domains entries are currently used as-is, so empty/whitespace patterns can make the allowlist non-empty but non-matchable, unexpectedly blocking redirects.

   • Evidence: crates/common/src/proxy.rs:587, crates/common/src/proxy.rs:470, crates/common/src/settings.rs:292
   • Recommendation: normalize at config load time (trim/lowercase/drop empties), and optionally validate pattern shape (example.com or *.example.com).

2. Low: tests cover is_host_allowed logic well, but don’t yet exercise full redirect follow enforcement end-to-end in proxy_with_redirects.

   • Evidence: crates/common/src/proxy.rs:1907, crates/common/src/proxy.rs:484
   • Recommendation: add one allowed + one blocked redirect-chain test through proxy_request/redirect flow.

Everything else looked good, and CI is green.

[ChristianPavilonis]

LGTM — core SSRF hardening looks correct and CI is green.

One suggestion for follow-up: the new proxy.allowed_domains setting should be documented in the proxy and configuration guides (docs/guide/first-party-proxy.md and docs/guide/configuration.md) so operators are aware of this security control. Specifically, it would be helpful to add a [proxy] section documenting:

• allowed_domains semantics and wildcard matching rules
• The default-open behavior when the setting is omitted
• A production recommendation to explicitly set the allowlist

[aram356]

Summary

Solid, well-structured security PR. The core is_host_allowed logic is correct and handles the important edge cases (dot-boundary bypass, case sensitivity, deep subdomains, empty hosts). The main issues are documentation inaccuracies that would confuse operators.

Blocking

🔧 wrench

• Docs claim 403 but code returns 502: TrustedServerError::Proxy maps to StatusCode::BAD_GATEWAY (502). Multiple places in first-party-proxy.md and configuration.md state 403. Either fix the docs or add a dedicated error variant that returns 403 for allowlist violations. (docs/guide/configuration.md:764, docs/guide/first-party-proxy.md:441,565,578)
• Docs claim /first-party/click enforces the allowlist: handle_first_party_click returns a 302 directly to the browser — it never calls proxy_with_redirects where the allowlist check lives. The click endpoint relies on tstoken signature verification, not the domain allowlist. Remove /first-party/click from the allowlist docs or clarify the distinction. (docs/guide/configuration.md:764)
• first-party-proxy.md lines 565 and 578 (outside the diff): the example error responses show HTTP 403 Forbidden for signature mismatch and token expiry, but the actual code returns 502 for all TrustedServerError::Proxy variants.

❓ question

• Initial proxy target URL not checked against allowlist: The allowlist only validates redirect hops (proxy.rs:617), not the original tsurl. A signed URL pointing directly to http://169.254.169.254/latest/meta-data/ would be proxied without allowlist checking. The tstoken signature prevents arbitrary URL crafting, but if an attacker controls a legitimate signed URL target, the allowlist won't help. Is this intentional, or should the initial target also be validated?

Non-blocking

🤔 thinking

• No startup warning for open mode: When allowed_domains is empty, all redirects are permitted. A log::warn! in normalize() would help operators notice this. (crates/trusted-server-core/src/settings.rs:349)

🌱 seedling

• Newtype for AllowedDomains: Per project conventions, Vec<String> could become an AllowedDomains newtype encapsulating normalization and matching logic.
• Additional edge-case tests: IP literal targets, empty Location headers — all correctly handled by current logic but worth explicit tests to prevent regressions.

👍 praise

• Dot-boundary enforcement: The strip_suffix + ends_with('.') approach correctly prevents the classic evil-example.com bypass of *.example.com. Well-tested at line 1924.
• Scheme check before allowlist: Non-http/https redirect schemes are silently finalized (proxy.rs:613-614), preventing data:, javascript:, ftp: protocol abuse.

CI Status

• fmt: PASS
• clippy: PASS
• rust tests: PASS
• JS tests: Pre-existing failure (unrelated jsdom@28 issue on main)

[prk-Jr]

All blocking items and the open question have been addressed:

Blocking fixes

• Added TrustedServerError::AllowlistViolation { host } → StatusCode::FORBIDDEN (403). The docs at configuration.md:764 and first-party-proxy.md:441 now correctly say 403 for allowlist violations.
• Removed /first-party/click from the allowlist enforcement description in configuration.md. The click endpoint is protected by tstoken signature verification only and never calls proxy_with_redirects.
• Corrected first-party-proxy.md lines 565, 578, and 643 to show HTTP 502 Bad Gateway for signature mismatch and token expiry (those still go through TrustedServerError::Proxy → 502).

Open question — initial target now checked
Decided to extend the allowlist check to the initial tsurl target as well. The check now runs at the top of the proxy_with_redirects loop for every URL (iteration 0 = initial target, subsequent iterations = redirect hops). Added proxy_initial_target_blocked_by_allowlist test to cover this.

Non-blocking

• Added log::warn! in settings.rs normalize() when allowed_domains is empty (open mode warning).
• AllowedDomains newtype deferred to a follow-up to keep this PR focused.

CI: 744 tests passing.

[aram356]

Summary

Solid SSRF mitigation adding domain allowlist enforcement to proxy redirect chains. The core matching logic (dot-boundary, case-insensitive, wildcard subdomains) is correct and well-tested. Three issues need attention before merge.

Blocking

🔧 wrench

• Allowlist scope broader than documented: The check lives in proxy_with_redirects which gates all proxy calls (including integration proxies like testlight/GTM/GPT), but docs say it only governs first-party proxy. Users following the docs will break integration proxies. (crates/trusted-server-core/src/proxy.rs:553)
• Missing IP address bypass tests: No tests verify that IP-literal redirect targets (169.254.169.254, [::1]) are blocked — the most important SSRF vectors. The logic handles it correctly, but a security feature needs explicit proof. (crates/trusted-server-core/src/proxy.rs:496)

❓ question

• Wildcard "*" silently blocks everything: allowed_domains = ["*"] falls through to exact match (host == "*"), blocking all traffic — opposite of user intent. Should normalize() reject this with a warning? (crates/trusted-server-core/src/proxy.rs:496)

Non-blocking

🤔 thinking

• Default "allow all" language: Consider stronger wording than "intentional for zero-config development" — this is a security feature where the default is permissive. (docs/guide/first-party-proxy.md:451)
• Unicode/IDN domains: to_ascii_lowercase() doesn't normalize Punycode. Low risk in ad-tech but worth a doc note. (crates/trusted-server-core/src/proxy.rs:496)

🌱 seedling

• No integration test for redirect-hop blocking: Only the initial target is tested end-to-end; no test for Nth-hop blocking in a redirect chain. (crates/trusted-server-core/src/proxy.rs)

⛏ nitpick

• &[String] → &[impl AsRef<str>]: Minor flexibility improvement for redirect_is_permitted. (crates/trusted-server-core/src/proxy.rs:484)

📝 note

• Empty host on redirect: Returns AllowlistViolation instead of the more descriptive "missing host" error. Not a bug — less informative for debugging. (crates/trusted-server-core/src/proxy.rs:639)

CI Status

• fmt: PASS
• clippy: PASS
• rust tests: PASS
• js tests: PASS