Skip to content

Add a host allowlist for creative asset rewriting - #1253

Draft
dhruv8sh wants to merge 10 commits into
spec/1231-creative-asset-host-allowlistfrom
feat/1231-creative-asset-host-allowlist
Draft

dhruv8sh wants to merge 10 commits into
spec/1231-creative-asset-host-allowlistfrom
feat/1231-creative-asset-host-allowlist

Conversation

@dhruv8sh

@dhruv8sh dhruv8sh commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds [rewrite] include_domains, a host allowlist for creative asset rewriting.
    • Why: today every absolute asset URL in a winning creative is proxied through /first-party/proxy, and each proxied fetch appends the EC ID, so every third-party asset host receives it.
    • Non-empty list: only matching asset hosts are proxied, and every other asset keeps its original URL. exclude_domains still wins over the list, and click-through links are never affected.
  • /first-party/sign applies the same policy for absolute and // input.
    • Off-list host: gets 502, the same as an excluded host, so the creative runtime loads the raw URL directly.
    • 403: still means only "blocked by proxy.allowed_domains".
  • Validation is strict. Entries that can never match a URL host are rejected at ts config push. That includes schemes, ports, paths, stray dots, unbracketed IPv6, and non-ASCII names, which must be written in punycode.
  • Fixes an existing bug: on the inline SSAT/page-bids path, <link imagesrcset> was proxied twice, wrapping the proxy URL inside another proxy URL.

Implements the approved design in #1248: docs/superpowers/specs/2026-10-07-1231-creative-asset-host-allowlist-design.md.

Stacked on #1252. This branch builds on the #1234 implementation, which introduces the shared normalizer and host-policy split.

Changes

These are the #1231 commits only.

File Change
crates/trusted-server-core/src/settings.rs include_domains field (left out of the blob when empty), load-time normalization, strict validate_include_domains, and the include clause in should_proxy_asset via is_host_permitted; tests
crates/trusted-server-core/src/creative.rs Allowlist matrix across every asset handler on all three HTML paths, plus CSS bodies. link[href] no longer rewrites imagesrcset; the [imagesrcset] handler rewrites it once. Tests and module docs
crates/trusted-server-core/src/proxy.rs is_host_permitted made pub(crate); sign decline log; an 8-case GET/POST sign matrix covering 200, 502 and 403
crates/trusted-server-core/src/config_payload.rs Default-omission and round-trip tests
docs/guide/configuration.md, creative-processing.md, first-party-proxy.md, api-reference.md Allowlist semantics, validation rules, the rollback warning, and the sign 502. Fixes the *.cdn.example.com apex example
trusted-server.example.toml Commented-out include_domains example
CHANGELOG.md Added include_domains; Fixed the inline imagesrcset double proxy

Closes

Closes #1231

Test plan

  • cargo test-fastly && cargo test-axum. Also cargo test-fastly-reuse, cargo test-cloudflare, cargo test-spin, ./scripts/test-cli.sh and the parity tests.
  • cargo clippy-fastly && cargo clippy-axum. All 8 clippy aliases pass, and every intermediate commit passes clippy-fastly.
  • cargo fmt --all -- --check
  • JS tests: cd crates/trusted-server-js/lib && npx vitest run (no JS changes)
  • JS format: cd crates/trusted-server-js/lib && npm run format (no JS changes)
  • Docs format: cd docs && npm run format, plus the markdown prettier check outside docs/
  • WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
  • Manual testing via fastly compute serve
  • Other: the imagesrcset regression test was seen failing on the unfixed code. Make creative click rewriting a separate switch #1252's switch-matrix tests pass together with this branch's allowlist tests.

Rollout: deploy the binary first, then add include_domains and run ts config push. Rollback: remove include_domains and push first, then roll back the binary; older binaries reject the key because of deny_unknown_fields.

Hardening note:

  • Invalid entries: an invalid include_domains entry fails settings validation with a config error rather than a panic. Covered by rewrite_include_domains_reject_malformed_entries.
  • Bad exclude_domains entries: empty and bare "*" entries are dropped with a warning instead of rejected, so existing configs keep loading after an upgrade.

Checklist

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

Replace to_abs with normalize_creative_url and move exclusion policy to Rewrite::should_proxy_asset and Rewrite::should_wrap_click, sharing the case-insensitive proxy host matcher. Normalize exclude_domains at load. This is the shared step for #1231 and #1234.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
…when unset

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
The rewrite pass registers asset handlers only when rewrite_creatives is on and the anchor handler only when click rewriting resolves on. Base removal and TSJS injection run whenever either is on.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
…rty proxy

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
…avior

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Add the [rewrite] include_domains field with load-time normalization, strict entry validation and blob round-trip tests. Enforcement arrives in the next commit, which applies the list to asset rewriting and first-party sign.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Apply the include list to every asset handler and to first-party sign. Click-through links and fetch-time proxy checks are unchanged.

Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
@dhruv8sh dhruv8sh self-assigned this Oct 7, 2026
@dhruv8sh dhruv8sh linked an issue Oct 7, 2026 that may be closed by this pull request
9 tasks
@aram356
aram356 marked this pull request as draft October 8, 2026 15:50
@aram356 aram356 mentioned this pull request Oct 9, 2026
5 of 14 tasks

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a host allowlist for creative asset rewriting

1 participant