[dhruv8sh]

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.

• Commits under review: the four Add a host allowlist for creative asset rewriting #1231 commits 48d59b8d, ef8ad67f, 9d9fb273 and 2dfe7e7b, plus the review fixes in 2feb4897. Merges of main add nothing of their own beyond conflict resolution.
• The six commits below them (d8f04390…ee5715f1) are cherry-picks of Make creative click rewriting a separate switch #1252 with identical patch IDs, and are reviewed there.
• Base: main. **Spec**: creative asset host allowlist #1248 was closed without merging, so this PR carries the spec itself.
• Merge order: Make creative click rewriting a separate switch #1252 first. After that, this branch is rebased onto main and only the Add a host allowlist for creative asset rewriting #1231 commits remain.

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

[dhruv8sh]

@prk-Jr Your review comments on the spec PRs (#1248 and #1249) are addressed in the spec branches. Those changes are merged here, along with the matching code changes: the include_domains host round-trip check, the warning for exclude_domains entries that can never match, the <link> let-chain, keeping <base target>, the doc warning on the public rewrite functions, and the operator doc and CHANGELOG corrections. One thing differs from what we discussed: in clicks-only mode <base> still loses its href, because /auction click URLs are root-relative and would otherwise resolve against the bidder's base. The spec explains the tradeoff.

[aram356]

Summary

This PR adds [rewrite] include_domains and applies it to every asset handler on all three HTML paths, to CSS bodies, and to /first-party/sign. It also fixes the inline <link imagesrcset> double proxy. The core change holds up:

• A mutation pass over the new code (12 mutants, run on the PR merged with main) caught 10.
• The merged tree passes clippy-fastly and test-fastly.
• Every proxy URL builder goes through asset_target, so nothing bypasses the allowlist.

Three things block merging:

• the conflict with main;
• the #1252 commits this branch carries;
• an exclude_domains warning that tells operators to delete entries that work.

4 of the inline comments below carry a one-click GitHub suggestion. Use Commit suggestion, or Add suggestion to batch to apply several at once, to add them as commits on the PR branch. The other comments describe the fix in prose, because it spans several hunks or files.

Blocking

🔧 wrench

• The exclude_domains warning flags entries that do match, including the trailing-dot workaround. See inline at crates/trusted-server-core/src/settings.rs:708.

Non-blocking

🤔 thinking

• Sign declines log at error and count as 5xx on every adapter. See inline at crates/trusted-server-core/src/proxy.rs:1692.
• The include grammar rejects real hosts (_), and the 0x clause is redundant. See inline at crates/trusted-server-core/src/settings.rs:3494.

♻️ refactor

• The validation error doesn't name the bad entry. See inline at crates/trusted-server-core/src/settings.rs:3450.

⛏ nitpick

• Unset rewrite_clicks follows rewrite_creatives on the auction paths. See inline at docs/guide/creative-processing.md:762.
• The spec status is stale. See inline at docs/superpowers/specs/2026-10-07-1231-creative-asset-host-allowlist-design.md:7.

Cross-cutting / body-level findings

• 🔧 Conflicts with main. Merging origin/main (d2f0d8c) conflicts in two files:

  • CHANGELOG.md: both sides add Fixed entries. Keep both.
  • settings.rs normalize_deserialized: #1221 made image_optimizer.normalize() return Result. The resolution is:

          self.rewrite.normalize();
          self.image_optimizer.normalize()?;

  With that resolution, the merged tree passes cargo fmt --check, cargo clippy-fastly and cargo test-fastly (2964 core tests).

• 🔧 This branch carries #1252, including #1252's open blocking threads. The base is now main, so this diff includes the rewrite_clicks work in the same state as #1252's 7c5eca0da. Review 5507201594 on #1252 still has six open 🔧 threads:

  • auction-orchestration.md;
  • two CHANGELOG items;
  • the example TOML;
  • the creative-processing.md guard sentence;
  • the #1234 spec status.

  Merging this PR first would land all of them. As the description plans, land #1252 first, then rebase so only the #1231 commits remain. Two related problems:

  • The description is out of date. It names spec/1231-… as the base and lists four commits under review. The base is main, and the #1231 work now also includes f7885c199, e1332c9fe, 2af735d1d and the spec feedback in cce4e2a5e.
  • The shared Fixed entry in CHANGELOG.md no longer matches #1252's copy. This one adds "and other entries that can never match a host are logged with a warning". The plan says the shared text is identical in both PRs, so whichever lands second will conflict on that line.

• 📌 The trailing-dot bypass needs its follow-up issue. The spec's Follow-ups section says to track it in a follow-up issue, and I found none. https://blocked.example.com./x has host_str() blocked.example.com., so it escapes exclude_domains and still gets ts-ec. The spec suggests stripping one trailing . before matching. Until then, an exclude entry written with the trailing dot is the stopgap, which is why the wrench above matters.

CI Status

• CLAUDE.md symlink guard: PASS
• browser integration tests: PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo fmt: PASS (required)
• cargo test: PASS (required)
• cargo test (axum native): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native) (macos-latest): PASS
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• format-docs: PASS (required)
• format-typescript: PASS (required)
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• prepare integration artifacts: PASS
• vitest: PASS

[ChristianPavilonis]

Please address the host-classification issue: valid DNS names such as 0xproxy are rejected as IPv4 candidates, preventing otherwise valid allowlist configurations.

Also clarify the changelog and operator guidance: include_domains controls newly rewritten or signed asset targets, not fetch-time enforcement. Previously issued proxy URLs can still forward EC IDs to hosts subsequently removed from the list. This is a documentation correction, not a request to expand runtime policy.

[dhruv8sh]

Filed #1393 for the trailing-dot and IDN host bypass, and #1394 for the status and log level when signing is declined.

[ChristianPavilonis]

Approved. No findings to raise on the creative asset host allowlist and imagesrcset double-proxy fix.

[aram356]

Summary

Round 2 at e15eea5a1. The round-1 fixes in 2feb489 work. I checked each one with the PR-head ts binary and a mutation pass:

• exclude_domains has its own looser check, so blocked.example.com., ads_cdn.example.com and *.example.net are no longer flagged.
• include_domains accepts cdn_assets.example.com and 0xproxy, and still rejects 0xc0000201.
• A bad include_domains entry is now named, along with the reason.

The mutation pass made 8 deliberate breaks to the new code, and the tests caught 7. The one they miss is deleting the warning loop, which is log-only. The sign-decline status and log level are now tracked in #1394, and the trailing-dot bypass in #1393.

The PR merged with current main passes cargo fmt --check, clippy-fastly and test-fastly (2978 core tests).

Two things still block merging:

• the CodeQL autofix stripped the exclude_domains warning back down;
• this PR's copy of #1252 is now out of date.

1 of the inline comments below carries a one-click GitHub suggestion. Use Commit suggestion to apply it.

Blocking

🔧 wrench

• The CodeQL autofix removed the entry and the reason from the exclude_domains warning. See inline at crates/trusted-server-core/src/settings.rs:712.

Cross-cutting / body-level findings

• 🔧 This PR's copy of #1252 is out of date, and either merge order now conflicts heavily.

  • #1252 picked up 89d4897 ("Align creative click switch docs and tests with review feedback"), which this branch doesn't have.
  • The description's claim that the six commits below the #1231 work are "cherry-picks of #1252 with identical patch IDs, and are reviewed there" no longer holds.
  • Measured conflicts with git merge-tree: merging #1252's head into this branch, or this branch after #1252 lands, conflicts in 11 files with 29 hunks, about 840 conflicting lines in total. Most of it is in creative.rs, settings.rs and proxy.rs.
  • If this PR landed first, main would get #1252's code without 89d4897.

  Rebuild instead of merging. Once #1252 lands, rebuild this branch on main by cherry-picking only the #1231 commits:

  53cfb8149 48d59b8da ef8ad67f9 9d9fb2733 ea642da7e 2dfe7e7b5 cce4e2a5e f7885c199 e1332c9fe 2af735d1d 2feb48970 8aabb3920 e15eea5a1
  

  I ran that onto main + #1252 at efdd8a00e:

  • Five commits conflict, with 8 small hunks between them:
    • 48d59b8da: the exclude_domains doc comment in settings.rs;
    • ef8ad67f9: creative.rs and settings.rs;
    • 2dfe7e7b5: CHANGELOG and creative-processing.md;
    • 2af735d1d: CHANGELOG;
    • 2feb48970: CHANGELOG and the sign # Errors doc.
  • The result touches only the 12 #1231 files.
  • It passes cargo fmt --check, clippy-fastly and test-fastly (2978 core tests).

  I resolved the conflicts by taking the #1231 side, so read the CHANGELOG and doc hunks rather than trusting that resolution. In particular, the shared Fixed entry should keep #1252's wording plus the one #1231 clause about the new warning.

CI Status

• Analyze (actions): PASS
• Analyze (javascript-typescript): PASS (two runs)
• Analyze (python): PASS
• Analyze (rust): PASS
• browser integration tests: PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo fmt: PASS (required)
• cargo test: PASS (required)
• cargo test (axum native): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native) (macos-latest): PASS
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• CLAUDE.md symlink guard: PASS
• CodeQL: PASS
• format-docs: PASS (required)
• format-typescript: PASS (required)
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• prepare integration artifacts: PASS
• vitest: PASS

[prk-Jr]

Summary

Reviewed the complete diff against main, including the stacked changes: creative HTML/CSS/srcset rewriting, host normalization and validation, signing and proxy policies, click controls, auction and SSAT delivery, browser runtime interactions, config serialization, CLI overlays, and rollout compatibility. No blocking correctness or security findings.

Validation

• All 2,984 core unit tests passed when running the compiled test binary directly with TRUSTED_SERVER__GIT_VERSION absent. Cargo's test launcher exposes an existing environment collision affecting 12 legacy config tests; the responsible build-script behavior is unchanged from main.
• Rust formatting, changed documentation formatting, and diff whitespace checks passed.
• Full adapter verification was not rerun locally. Remote checks still pending below remain subject to completion.

CI Status

• CodeQL: SKIPPED
• cargo test (ts CLI, native) (macos-latest): PASS
• format-typescript: PASS
• cargo test (cross-adapter parity): PASS
• cargo test: PENDING
• Analyze (python): PASS
• Analyze (javascript-typescript): PASS
• vitest: PASS
• cargo test (axum native): PENDING
• cargo fmt: PASS
• Analyze (javascript-typescript): PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• format-docs: PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• docs-proposal scripts: PASS
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• Analyze (actions): PASS
• prepare integration artifacts: PENDING
• Analyze (rust): PENDING
• CLAUDE.md symlink guard: PASS

[aram356]

Please address the malformed exclude-domain warning in this owning PR before it is refreshed into RC 202610. The RC's CodeQL check reports this new logging sink as alert #210.

This request concerns the warning added here. The other RC alerts involve code already on main or test fixtures and need separate triage; changing URL encryption algorithms is not warranted by those findings.

[aram356]

Summary

Round 4 at 83fe9f5d5. Since round 3, the author fixed the P2 from review 5606398526 and merged main:

• The P2 fix works. The warning now gives the entry's position, counted before empty and * entries are dropped, plus a fixed reason, and a test with a credential sentinel covers it. With the PR-head ts binary, an exclude_domains entry carrying example-user:example-secret prints rewrite.exclude_domains[2] never matches a URL host (not a valid URL host); the entry is kept, with no credential.
• CodeQL is clear. Alert 210 is marked fixed on this PR's merge ref.
• The main merge is correct. d342e47df kept both CHANGELOG.md entries.
• CI is green. All 23 checks pass.

Two things still block merging:

• validate_include_domains still echoes the raw entry, which is the same leak on the other list;
• this PR's copy of #1252, which is now three commits behind.

1 of the inline comments below carries a one-click GitHub suggestion. Use Commit suggestion to apply it.

Blocking

🔧 wrench

• include_domains validation errors echo the raw entry, credentials included. See inline at crates/trusted-server-core/src/settings.rs:3617.

Non-blocking

⛏ nitpick

• The normalize doc doesn't cover the new warnings, and no test pins their ordering. See inline at crates/trusted-server-core/src/settings.rs:692.

Cross-cutting / body-level findings

• 🔧 This PR's copy of #1252 is now three commits behind. It lacks 89d4897, 70ba721 and fa87a49. Merging #1252's head into this branch, or this branch after #1252, now conflicts in 11 files with 37 hunks.

  Once #1252 lands, rebuild this branch on main with the round-3 recipe, adding the P2 fix at the end:

  53cfb8149 48d59b8da ef8ad67f9 9d9fb2733 ea642da7e 2dfe7e7b5 cce4e2a5e f7885c199 e1332c9fe 2af735d1d 2feb48970 83fe9f5d5
  

  I re-ran it onto main + #1252 at fa87a4989. The same nine hunks conflict, and round 3's per-hunk resolutions still apply. 83fe9f5d5 applies cleanly on top. The result touches only the 12 #1231 files. It passes cargo fmt --check, docs prettier, clippy-fastly and test-fastly (3009 core tests).

  The same warning applies: don't resolve with -X theirs or git checkout --theirs. The first drops main's CHANGELOG.md entries and #1252's <base target> doc; the second takes the whole file and reverts unrelated code.

CI Status

• Analyze (actions): PASS
• Analyze (javascript-typescript): PASS (two runs)
• Analyze (python): PASS
• Analyze (rust): PASS
• browser integration tests: PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo fmt: PASS (required)
• cargo test: PASS (required)
• cargo test (axum native): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native) (macos-latest): PASS
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• CLAUDE.md symlink guard: PASS
• CodeQL: PASS
• docs-proposal scripts: PASS
• format-docs: PASS (required)
• format-typescript: PASS (required)
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• prepare integration artifacts: PASS
• vitest: PASS

[aram356]

🔧 wrench: The P2 fix keeps raw values out of the exclude_domains warning, but validate_include_domains still puts the raw entry into both of its error messages. I asked for the entry to be named back in round 1, before the P2 made the case for positions instead, so this is the same leak on the other list.

With the PR-head ts binary, include_domains = ["assets.example.com", "https://example-user:example-secret@cdn.example.com/path"] fails ts config validate --no-env with:

[ts] typed app-config failed validation: rewrite.include_domains: rewrite.include_domains entry `https://example-user:example-secret@cdn.example.com/path` must be a host name with an optional leading `*.`, a dotted-quad IPv4 address or a bracketed IPv6 address (character outside a-z, 0-9, `-`, `_` and `.`)

The exposure is narrower than the warning's was. The edge prints only the error code, through validation_error_summary. The message still reaches the operator's terminal and any CI log that runs ts config validate or push. The non-ASCII branch has the same problem, and so does the value param.

An index-based message still lets the operator find the entry. normalize doesn't drop include_domains entries, so the position matches the TOML on both the CLI and the edge:

fn validate_include_domains(patterns: &[String]) -> Result<(), ValidationError> {
    for (index, pattern) in patterns.iter().enumerate() {
        if !pattern.is_ascii() {
            let mut err = ValidationError::new("non_ascii_rewrite_include_domain");
            err.add_param("index".into(), &index);
            err.message = Some(
                format!(
                    "rewrite.include_domains[{index}] is not ASCII; write internationalized domain names in punycode (xn--) form"
                )
                .into(),
            );
            return Err(err);
        }
        if let Err(reason) = check_include_domain(pattern) {
            let mut err = ValidationError::new("invalid_rewrite_include_domain");
            err.add_param("index".into(), &index);
            err.add_param("reason".into(), &reason);
            err.message = Some(
                format!(
                    "rewrite.include_domains[{index}] must be a host name with an optional leading `*.`, a dotted-quad IPv4 address or a bracketed IPv6 address ({reason})"
                )
                .into(),
            );
            return Err(err);
        }
    }
    Ok(())
}

rewrite_include_domains_report_the_bad_entry_and_reason then needs to look for the position. This version also covers the credential case:

    #[test]
    fn rewrite_include_domains_report_the_bad_entry_and_reason() {
        let err = validate_include_domains(&[
            "assets.example.com".to_owned(),
            "https://example-user:example-secret@cdn.example.com/path".to_owned(),
        ])
        .expect_err("should reject an entry with a scheme and credentials");

        let params = format!("{:?}", err.params);
        let message = err.message.expect("should set a message").into_owned();
        assert!(
            message.contains("rewrite.include_domains[1]") && message.contains("character outside"),
            "should name the bad entry by index and give the reason: {message}"
        );
        assert!(
            !message.contains("example-secret") && !params.contains("example-secret"),
            "should keep the raw value out of the message and params: {message} {params}"
        );
    }

With both changes applied, clippy-fastly and test-fastly pass, and the same config prints:

[ts] typed app-config failed validation: rewrite.include_domains: rewrite.include_domains[1] must be a host name with an optional leading `*.`, a dotted-quad IPv4 address or a bracketed IPv6 address (character outside a-z, 0-9, `-`, `_` and `.`)

This has to be applied by hand: the validator and the test sit in different hunks, and the validator change alone fails the current test.

[aram356]

⛏ nitpick: The normalize doc still describes only the empty and * drop. Two facts about the new warnings belong here:

• Positions are counted before that drop, which is what makes them match the operator's TOML.
• ts config push stores the normalized list, so an edge log can show a lower position than ts config validate does when empty or * entries came first.

The first point also has no test. Moving the inert_exclude_domain_warnings call below the filter leaves every test passing (mutation run), because rewrite_exclude_domains_warnings_omit_raw_values calls the helper directly instead of going through normalize. To pin the ordering, normalize could get its warnings from a helper that also does the filtering, so one test can check both.

Suggested change

	/// Trims and lowercases host patterns in place.
	///
	/// Empty and bare `*` entries in `exclude_domains` are dropped with a
	/// warning: neither can match a host, so dropping them changes no behavior.
	/// Trims and lowercases host patterns in place.
	///
	/// Empty and bare `*` entries in `exclude_domains` are dropped with a
	/// warning: neither can match a host, so dropping them changes no behavior.
	/// Other entries that no URL host can match are kept and reported by
	/// position, counted before that drop; see
	/// [`Self::inert_exclude_domain_warnings`]. `ts config push` stores the
	/// normalized list, so positions logged at the edge can sit lower than the
	/// ones `ts config validate` prints when empty or `*` entries came first.