From 53cfb8149f5a75f8e8202a318bb35217ce395f74 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 14:48:36 +0530 Subject: [PATCH 01/20] Add design spec and plan for creative asset host allowlist Spec and implementation plan for #1231: a [rewrite] include_domains allowlist for creative asset rewriting, case-insensitive exclude_domains matching, and a single rewrite of link imagesrcset on the inline path. Signed-off-by: dhruv8sh --- ...0-07-1231-creative-asset-host-allowlist.md | 1654 +++++++++++++++++ ...31-creative-asset-host-allowlist-design.md | 409 ++++ 2 files changed, 2063 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-07-1231-creative-asset-host-allowlist.md create mode 100644 docs/superpowers/specs/2026-10-07-1231-creative-asset-host-allowlist-design.md diff --git a/docs/superpowers/plans/2026-10-07-1231-creative-asset-host-allowlist.md b/docs/superpowers/plans/2026-10-07-1231-creative-asset-host-allowlist.md new file mode 100644 index 000000000..f0f76d928 --- /dev/null +++ b/docs/superpowers/plans/2026-10-07-1231-creative-asset-host-allowlist.md @@ -0,0 +1,1654 @@ +# Creative asset host allowlist implementation plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add `[rewrite] include_domains`, an asset host allowlist for the creative rewrite pass and `/first-party/sign`, and make `exclude_domains` matching case-insensitive. + +**Architecture:** Replace `creative::to_abs` with a pure normalizer, `normalize_creative_url`, and move host policy onto `Rewrite` as `should_proxy_asset` and `should_wrap_click`, built on the existing `proxy::is_host_allowed` and `proxy::is_host_permitted`. Asset call sites go through `asset_target`, the anchor handler through `click_target`, and `/first-party/sign` pairs the normalizer with `should_proxy_asset`. The include list is then one clause in `should_proxy_asset`. + +**Tech Stack:** Rust 2024 (`trusted-server-core`), `url`, `lol_html`, `cssparser`, `serde`, `validator` 0.20. + +**Spec:** `docs/superpowers/specs/2026-10-07-1231-creative-asset-host-allowlist-design.md` + +**Status:** Validated. Every code block below was implemented on `7a0ecb4c`, driven red then green, and passed the gates in Task 7. Line numbers are against `7a0ecb4c`. + +## Global Constraints + +- Issue: [IABTechLab/trusted-server#1231](https://github.com/IABTechLab/trusted-server/issues/1231). Sibling: #1234. +- Setting: `[rewrite] include_domains`, `Vec`, `#[serde(default, skip_serializing_if = "Vec::is_empty")]`. Never named `allowed_domains`. +- Empty `include_domains` keeps today's rewritten output byte-for-byte for every existing fixture. +- `exclude_domains` wins over `include_domains`. `include_domains` never applies to `` or ``. +- Pattern syntax for both lists: exact host, or `*.example.com` matching the apex and any subdomain. Case-insensitive, dot boundary, matched against `Url::host_str()` only. +- Load-time normalization: trim and ASCII-lowercase both lists. `exclude_domains` drops `""` and `*` with a `log::warn!`. `include_domains` rejects `""`, `*`, `*.` and any `*` other than a leading `*.` with a validation error. +- `/first-party/sign`: an excluded or off-list host returns `TrustedServerError::Proxy { message: "unsupported url" }` (`502`) for absolute and `//` input, through GET and POST. That check runs before `proxy.allowed_domains`; `403` stays reserved for `proxy.allowed_domains`. +- No JS, adapter, routing or dependency changes. +- Fictional data only: `example.com`, `example.net`, `example.org` hosts. +- Rust conventions from `AGENTS.md`: `expect("should ...")`, no `unwrap()` outside tests, no local `use` inside functions, comments above code, descriptive assertion messages, doc comments on public items. +- Tests build `Rewrite` with `Rewrite::default()` and push or extend its lists. Never write a `Rewrite { .. }` literal, and never assign a whole field on a `Default::default()` value: `clippy::field_reassign_with_default` rejects it under `-D warnings`. +- Commits: sentence case, imperative, no `fix:`-style prefixes, `--signoff -S`, no AI attribution lines. + +## Coordination with #1234 (read before Task 1) + +Both #1231 and #1234 rely on one shared refactor, reproduced verbatim in [Shared step](#shared-step). Whichever implementation PR lands first ships it. Tasks 1 and 2 are that shared step. + +Before starting, check `main`: + +```bash +git fetch origin main +git grep -n "fn normalize_creative_url" origin/main -- crates/trusted-server-core/src/creative.rs +git grep -n "fn should_proxy_asset" origin/main -- crates/trusted-server-core/src/settings.rs +``` + +- **Both found** (#1234 landed first): rebase onto `origin/main`, **skip Tasks 1 and 2**, and start at Task 3. Diff the merged code against [Shared step](#shared-step); if names differ, use the merged names and note it in the PR description. +- **Neither found:** do Tasks 1 and 2. In the PR description, say that this PR introduces the shared step and that #1234 should rebase and skip its equivalent. +- **Only one found:** stop and ask the coordinator; the split landed partially. + +## Shared step + +This section is identical in the #1231 and #1234 plans. Both PRs write this production code and this test set verbatim; whichever PR lands first ships them, and the other skips its shared-step task. `proxy::is_host_permitted` stays private in the shared step: #1231 makes it `pub(crate)` in the task that adds the include clause, and #1234 never needs it. + +`crates/trusted-server-core/src/settings.rs`, imports (`:26`). The shared step uses only `is_host_allowed`, so it imports only that; importing `is_host_permitted` here would fail `-D warnings` as an unused import. #1231's include-clause task widens the import. + +```rust +use crate::proxy::is_host_allowed; +``` + +`settings.rs`, replaces `impl Rewrite` (`:645-670`), removing `is_excluded` and its `#[allow(dead_code)]`: + +```rust +impl Rewrite { + /// Returns `true` when an asset URL on `host` should be rewritten to + /// `/first-party/proxy`. + /// + /// The host must not match [`Self::exclude_domains`]. Matching is + /// case-insensitive; see [`is_host_allowed`] for the pattern rules. + #[must_use] + pub fn should_proxy_asset(&self, host: &str) -> bool { + !self.is_excluded_host(host) + } + + /// Returns `true` when a click-through URL on `host` should be wrapped in + /// `/first-party/click`. + /// + /// Only [`Self::exclude_domains`] applies to click-through links. + #[must_use] + pub fn should_wrap_click(&self, host: &str) -> bool { + !self.is_excluded_host(host) + } + + fn is_excluded_host(&self, host: &str) -> bool { + self.exclude_domains + .iter() + .any(|pattern| is_host_allowed(host, pattern)) + } + + /// 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. + fn normalize(&mut self) { + let before = self.exclude_domains.len(); + self.exclude_domains = self + .exclude_domains + .iter() + .map(|pattern| pattern.trim().to_ascii_lowercase()) + .filter(|pattern| !pattern.is_empty() && pattern != "*") + .collect(); + if self.exclude_domains.len() < before { + log::warn!( + "rewrite.exclude_domains: removed empty or bare \"*\" entries, which never match a host" + ); + } + } +} +``` + +`settings.rs`, in `Settings::normalize_deserialized` (`:2988`), right after `self.proxy.normalize();`: + +```rust + self.rewrite.normalize(); +``` + +`crates/trusted-server-core/src/creative.rs`, replaces `to_abs` and its comment (`:52-75`): + +```rust +/// Normalizes a creative URL to an absolute HTTP(S) [`url::Url`]. +/// +/// Trims surrounding whitespace, resolves a protocol-relative `//host/...` +/// against `https:`, and accepts only `http://` and `https://` input (ASCII +/// case-insensitive). Returns `None` for empty, relative, non-network-scheme or +/// unparseable input. Applies no host policy; see +/// [`crate::settings::Rewrite::should_proxy_asset`] and +/// [`crate::settings::Rewrite::should_wrap_click`]. +pub(super) fn normalize_creative_url(url: &str) -> Option { + let trimmed = url.trim(); + let is_http = trimmed + .get(..7) + .is_some_and(|prefix| prefix.eq_ignore_ascii_case("http://")); + let is_https = trimmed + .get(..8) + .is_some_and(|prefix| prefix.eq_ignore_ascii_case("https://")); + if trimmed.starts_with("//") { + url::Url::parse(&format!("https:{trimmed}")).ok() + } else if is_http || is_https { + url::Url::parse(trimmed).ok() + } else { + None + } +} + +/// Normalizes `raw` and returns it when asset policy allows proxying its host. +fn asset_target(settings: &Settings, raw: &str) -> Option { + normalize_creative_url(raw).filter(|url| { + url.host_str() + .is_some_and(|host| settings.rewrite.should_proxy_asset(host)) + }) +} + +/// Normalizes `raw` and returns it when click policy allows wrapping its host. +fn click_target(settings: &Settings, raw: &str) -> Option { + normalize_creative_url(raw).filter(|url| { + url.host_str() + .is_some_and(|host| settings.rewrite.should_wrap_click(host)) + }) +} +``` + +`creative.rs` call sites (`:402`, `:409`, `:639-641`, `:726-727`, `:1285-1287`): + +```rust + // CssUrlRewriter::rewrite + let Some(target) = asset_target(self.settings, value) else { + return; + }; + // ... later in the same function: + let proxied = build_proxy_url(self.settings, target.as_str(), self.base_origin); + +// proxy_if_abs +pub(super) fn proxy_if_abs(settings: &Settings, val: &str, base_origin: &str) -> Option { + asset_target(settings, val).map(|url| build_proxy_url(settings, url.as_str(), base_origin)) +} + + // rewrite_srcset + let rewritten = if let Some(target) = asset_target(settings, url) { + build_proxy_url(settings, target.as_str(), base_origin) + } else { + url.to_owned() + }; + + // Anchor handler + element!("a[href], area[href]", |el| { + if let Some(href) = el.get_attribute("href") + && let Some(target) = click_target(settings, &href) + { + let click = build_click_url(settings, target.as_str(), base_origin); + let _ = el.set_attribute("href", &click); + let _ = el.set_attribute("data-tsclick", &click); + } + Ok(()) + }), +``` + +`creative.rs` module doc (`:25-27`): + +```rust +//! - `normalize_creative_url(&str) -> Option`: Normalizes an absolute or +//! protocol-relative http(s) URL; returns `None` for relative input, +//! non-network schemes and unparseable values. Host policy lives on +//! [`crate::settings::Rewrite`]. +``` + +`proxy.rs`, `handle_first_party_proxy_sign` (`:1625`): replace `:1672-1703` (from `let trimmed` up to the `is_host_permitted` check) with the block below, then at `:1723` and `:1726` use `target` in place of `parsed` and `target.as_str()` in place of `&abs`. The old parse, scheme and duplicate exclusion checks go away because the normalizer already guarantees an `http(s)` URL. + +```rust + let trimmed = payload.url.trim(); + let protocol_relative; + // A protocol-relative target inherits the signing request's scheme before + // normalization, so `//` and absolute input reach the same policy check. + let candidate = if trimmed.starts_with("//") { + protocol_relative = format!("{request_scheme}:{trimmed}"); + protocol_relative.as_str() + } else { + trimmed + }; + let target = crate::creative::normalize_creative_url(candidate).ok_or_else(|| { + Report::new(TrustedServerError::Proxy { + message: "unsupported url".to_string(), + }) + })?; + let host = target.host_str().ok_or_else(|| { + Report::new(TrustedServerError::Proxy { + message: "missing host".to_string(), + }) + })?; + if !settings.rewrite.should_proxy_asset(host) { + log::debug!("sign request for `{host}` declined by rewrite policy"); + return Err(Report::new(TrustedServerError::Proxy { + message: "unsupported url".to_string(), + })); + } +``` + +```rust + let mut base = target.clone(); + base.set_query(None); + base.set_fragment(None); + let proxied = crate::creative::build_proxy_url_with_extras(settings, target.as_str(), &extras); +``` + +### Shared tests + +Every shared test builds `Rewrite` with `Rewrite::default()` and `.extend`/`.push` on its lists, or sets `settings.rewrite.exclude_domains` on a full `Settings`. None uses a `Rewrite { .. }` literal or `#[allow(clippy::field_reassign_with_default)]`. The code for each test is in the task that adds it in each plan. + +| File | Test | Replaces or placement | +| ------------- | -------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------- | +| `settings.rs` | `rewrite_policy_matches_exclude_patterns_case_insensitively` | replaces `test_rewrite_is_excluded` | +| `settings.rs` | `rewrite_normalize_trims_lowercases_and_drops_inert_exclude_entries` | after the previous test | +| `settings.rs` | `rewrite_exclude_domains_match_mixed_case_entries_from_toml` | after the previous test | +| `settings.rs` | `settings_load_normalizes_rewrite_exclude_domains` | after the previous test | +| `creative.rs` | `normalized` helper and `normalize_creative_url_conversions` | replace `to_abs_conversions`, `to_abs_preserves_port_in_protocol_relative` and `to_abs_additional_cases` | +| `creative.rs` | `proxy_if_abs_respects_exclude_domains` | replaces `to_abs_respects_exclude_domains` and `to_abs_respects_wildcard_domains` | +| `creative.rs` | `unparseable_absolute_url_is_left_byte_identical` | after the previous test | +| `creative.rs` | `unparseable_absolute_click_url_is_left_byte_identical` | after the previous test | +| `creative.rs` | `exclude_domains_match_case_insensitively_in_the_rewrite_pass` | after the previous test | +| `proxy.rs` | `proxy_sign_rejects_excluded_urls` | rewritten in place: GET and POST, plus an uppercase target host | +| `proxy.rs` | `proxy_sign_rejects_excluded_urls_case_insensitively` | after the previous test: mixed-case config entry, absolute and `//` input, GET and POST | + +The `creative.rs` test module's `use super::{...}` list gains `normalize_creative_url` and `proxy_if_abs` and loses `to_abs`. + +CHANGELOG: the shared step's entry goes under `## [Unreleased]` › `### Fixed`. + +## File map + +| File | Responsibility in this change | +| ---------------------------------------------------------------- | ----------------------------------------------------------------------------------------- | +| `crates/trusted-server-core/src/settings.rs` | `Rewrite` fields, normalization, validation, policy methods, their unit tests. | +| `crates/trusted-server-core/src/creative.rs` | `normalize_creative_url`, `asset_target`, `click_target`, call sites, rewrite-pass tests. | +| `crates/trusted-server-core/src/proxy.rs` | `is_host_permitted` visibility, `/first-party/sign` normalization and policy, sign tests. | +| `crates/trusted-server-core/src/config_payload.rs` | Blob default-omission and round-trip tests. | +| `docs/guide/*.md`, `trusted-server.example.toml`, `CHANGELOG.md` | Operator documentation. | + +Run Rust test steps with `cargo test-fastly -- `. Core tests run on `wasm32-wasip1` under Viceroy, which must be on `PATH` (`export PATH="$HOME/.cargo/bin:$PATH"` if `cargo install` put it there). A wasm test panic aborts the whole test binary, so when one test fails, the remaining tests in that binary do not run; filter to the tests you are working on, and rerun with `--nocapture` to see the panic message above the wasm backtrace. + +--- + +### Task 1: Host policy methods and case-insensitive `exclude_domains` (shared step, part 1) + +Skip if `fn should_proxy_asset` is already on `main` (see Coordination). + +**Files:** + +- Modify: `crates/trusted-server-core/src/settings.rs:26` (import), `:645-670` (`impl Rewrite`), `:2988-2990` (`normalize_deserialized`) +- Test: `settings.rs` tests, after `test_rewrite_is_excluded` (`:6523`) + +**Interfaces:** + +- Consumes: `crate::proxy::is_host_allowed(host: &str, pattern: &str) -> bool` (`proxy.rs:1242`, `pub(crate)`). +- Produces: `pub fn Rewrite::should_proxy_asset(&self, host: &str) -> bool`, `pub fn Rewrite::should_wrap_click(&self, host: &str) -> bool`, private `fn Rewrite::is_excluded_host(&self, host: &str) -> bool`, private `fn Rewrite::normalize(&mut self)`. + +- [ ] **Step 1: Write the failing tests** + +```rust + #[test] + fn rewrite_policy_matches_exclude_patterns_case_insensitively() { + let mut rewrite = Rewrite::default(); + rewrite + .exclude_domains + .extend(["cdn.example.com", "*.example.org"].map(str::to_owned)); + + for (host, expected) in [ + ("cdn.example.com", false), + ("CDN.EXAMPLE.COM", false), + ("example.org", false), + ("a.b.example.org", false), + ("evil-example.org", true), + ("sub.cdn.example.com", true), + ("other.example.com", true), + ] { + assert_eq!( + rewrite.should_proxy_asset(host), + expected, + "should_proxy_asset(`{host}`) should be {expected}" + ); + assert_eq!( + rewrite.should_wrap_click(host), + expected, + "should_wrap_click(`{host}`) should be {expected}" + ); + } + } +``` + +```rust + #[test] + fn rewrite_normalize_trims_lowercases_and_drops_inert_exclude_entries() { + let mut rewrite = Rewrite::default(); + rewrite + .exclude_domains + .extend([" CDN.Example.com ", "", "*", "*.Example.ORG"].map(str::to_owned)); + + rewrite.normalize(); + + assert_eq!( + rewrite.exclude_domains, + vec!["cdn.example.com".to_owned(), "*.example.org".to_owned()], + "should trim, lowercase, and drop empty and bare `*` entries" + ); + } +``` + +```rust + #[test] + fn rewrite_exclude_domains_match_mixed_case_entries_from_toml() { + let toml_str = crate_test_settings_str() + + r#" + [rewrite] + exclude_domains = ["CDN.Example.com"] + "#; + + let settings = Settings::from_toml(&toml_str).expect("should parse valid TOML"); + + assert!( + !settings.rewrite.should_proxy_asset("cdn.example.com"), + "should exclude a host whose config entry was written in mixed case" + ); + } +``` + +```rust + #[test] + fn settings_load_normalizes_rewrite_exclude_domains() { + let toml_str = crate_test_settings_str() + + r#" + [rewrite] + exclude_domains = [" CDN.Example.com ", "*", ""] + "#; + + let settings = Settings::from_toml(&toml_str).expect("should parse valid TOML"); + + assert_eq!( + settings.rewrite.exclude_domains, + vec!["cdn.example.com".to_owned()], + "should trim, lowercase, and drop inert entries when settings load" + ); + } +``` + +`settings_load_normalizes_rewrite_exclude_domains` is the only test here that proves `normalize` is wired into settings loading: the matcher is case-insensitive on its own, so the mixed-case TOML test would pass even without the `normalize_deserialized` call. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cargo test-fastly -- rewrite_policy_matches rewrite_normalize_trims rewrite_exclude_domains_match settings_load_normalizes` + +Expected: compile errors, `no method named should_proxy_asset found for struct settings::Rewrite` (and `should_wrap_click`, `normalize`). + +- [ ] **Step 3: Implement** + +Add the import and the methods from [Shared step](#shared-step) (`use crate::proxy::is_host_allowed;`, `should_proxy_asset`, `should_wrap_click`, `is_excluded_host`, `normalize`), and the `self.rewrite.normalize();` call. For this task only, keep `is_excluded` in place above the new methods; its two callers still exist until Task 2. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cargo test-fastly -- rewrite_policy_matches rewrite_normalize_trims rewrite_exclude_domains_match settings_load_normalizes test_rewrite_is_excluded` + +Expected: `5 passed`. + +- [ ] **Step 5: Lint** + +Run: `cargo fmt --all && cargo clippy-fastly` + +Expected: no warnings. + +- [ ] **Step 6: Commit** + +```bash +git add crates/trusted-server-core/src/settings.rs +git commit --signoff -S -m "Add rewrite host policy methods and case-insensitive exclude matching" +``` + +--- + +### Task 2: Replace `to_abs` with `normalize_creative_url` (shared step, part 2) + +Skip if `fn normalize_creative_url` is already on `main` (see Coordination). + +**Files:** + +- Modify: `crates/trusted-server-core/src/creative.rs:25-27`, `:52-75`, `:402-409`, `:639-641`, `:726-727`, `:1285-1287`, `:1497-1501` (test imports) +- Modify: `crates/trusted-server-core/src/proxy.rs:1672-1726` +- Modify: `crates/trusted-server-core/src/settings.rs` (remove `is_excluded` and `test_rewrite_is_excluded`) +- Test: `creative.rs` tests (`:1699-1738`, `:3418-3504`), `proxy.rs` `proxy_sign_rejects_excluded_urls` (`:2692-2715`) + +**Interfaces:** + +- Consumes: `Rewrite::should_proxy_asset`, `Rewrite::should_wrap_click` from Task 1. +- Produces: `pub(super) fn normalize_creative_url(url: &str) -> Option` (visible crate-wide, as `to_abs` was); private `fn asset_target(settings: &Settings, raw: &str) -> Option` and `fn click_target(settings: &Settings, raw: &str) -> Option`. Signatures of `proxy_if_abs`, `proxied_attr_value`, `rewrite_srcset`, `CssUrlRewriter::rewrite` and the `build_*_url` helpers do not change. + +- [ ] **Step 1: Write the failing tests** + +In `creative.rs` tests, change the `use super::{...}` list (`:1497-1501`) to: + +```rust + use super::{ + CreativeCssProcessor, StreamProcessor as _, normalize_creative_url, + process_auction_creative, proxy_if_abs, rewrite_creative_html, + rewrite_inline_creative_html, rewrite_srcset, rewrite_style_urls, sanitize_creative_html, + }; +``` + +Replace `to_abs_conversions` and `to_abs_preserves_port_in_protocol_relative` (`:1699-1738`) with: + +```rust + fn normalized(raw: &str) -> Option { + normalize_creative_url(raw).map(|url| url.as_str().to_owned()) + } + + #[test] + fn normalize_creative_url_conversions() { + for (raw, expected) in [ + ("//cdn.example/x", Some("https://cdn.example/x")), + ("HTTPS://cdn.example/x", Some("https://cdn.example/x")), + ("http://cdn.example/x", Some("http://cdn.example/x")), + (" //cdn.example/y ", Some("https://cdn.example/y")), + (" https://cdn.example/a ", Some("https://cdn.example/a")), + ( + "//cdn.example.com:8080/asset.js", + Some("https://cdn.example.com:8080/asset.js"), + ), + ( + "//cdn.example.com:9443/img.png", + Some("https://cdn.example.com:9443/img.png"), + ), + ("/local/x", None), + ("", None), + ("data:image/png;base64,abcd", None), + ("javascript:alert(1)", None), + ("mailto:test@example.com", None), + ("blob:xyz", None), + ("tel:+123", None), + ("about:blank", None), + ("https://exa mple.example/x", None), + ] { + assert_eq!( + normalized(raw).as_deref(), + expected, + "should normalize `{raw}` to {expected:?}" + ); + } + } +``` + +Delete `to_abs_additional_cases` (`:3418-3428`); its cases are in the table above. + +Replace `to_abs_respects_exclude_domains` and `to_abs_respects_wildcard_domains` (`:3445-3504`) with: + +```rust + #[test] + fn proxy_if_abs_respects_exclude_domains() { + let mut settings = crate::test_support::tests::create_test_settings(); + settings.rewrite.exclude_domains = vec![ + "trusted-cdn.example.com".to_owned(), + "*.example.org".to_owned(), + ]; + + for excluded in [ + "https://trusted-cdn.example.com/lib.js", + "//trusted-cdn.example.com/lib.js", + "https://example.org/cdn.js", + "//cdnjs.example.org/lib.js", + ] { + assert_eq!( + proxy_if_abs(&settings, excluded, ""), + None, + "should leave excluded URL `{excluded}` unproxied" + ); + } + for proxied in [ + "https://other-cdn.example.com/lib.js", + "//other-cdn.example.com/lib.js", + "https://notexample.org/lib.js", + ] { + assert!( + proxy_if_abs(&settings, proxied, "") + .is_some_and(|url| url.starts_with("/first-party/proxy?tsurl=")), + "should proxy non-excluded URL `{proxied}`" + ); + } + } +``` + +```rust + #[test] + fn unparseable_absolute_url_is_left_byte_identical() { + let settings = crate::test_support::tests::create_test_settings(); + let html = ""; + + let out = rewrite_creative_html(&settings, html); + + assert!( + out.contains(html), + "should leave an unparseable absolute URL untouched, including its quoting: {out}" + ); + } +``` + +The unparseable test asserts `contains`, not equality: `rewrite_creative_html` injects the TSJS script tag even when the markup has no ``, so the output is never equal to the input. + +```rust + #[test] + fn unparseable_absolute_click_url_is_left_byte_identical() { + let settings = crate::test_support::tests::create_test_settings(); + let html = "x"; + + let out = rewrite_creative_html(&settings, html); + + assert!( + out.contains(html), + "should leave an unparseable click URL untouched, including its quoting: {out}" + ); + assert!( + !out.contains("data-tsclick"), + "should not mark an unparseable link for the click guard: {out}" + ); + } + + #[test] + fn exclude_domains_match_case_insensitively_in_the_rewrite_pass() { + let mut settings = crate::test_support::tests::create_test_settings(); + settings + .rewrite + .exclude_domains + .extend(["Landing.Example.com", "CDN.example.com"].map(str::to_owned)); + let html = r#"x"#; + + let out = rewrite_creative_html(&settings, html); + + assert!( + out.contains(r#""#), + "should leave a link raw when its host matches a mixed-case entry: {out}" + ); + assert!( + out.contains(r#""#), + "should leave an asset raw when its host matches a mixed-case entry: {out}" + ); + } +``` + +These two cover the click path, which `proxy_if_abs_respects_exclude_domains` and the asset-only unparseable test do not, and mixed-case entries that bypass load-time normalization because the test sets them directly. + +In `proxy.rs`, replace `proxy_sign_rejects_excluded_urls` (`:2692-2715`): + +```rust + #[test] + fn proxy_sign_rejects_excluded_urls() { + futures::executor::block_on(async { + let mut settings = create_test_settings(); + settings.rewrite.exclude_domains = vec!["cdn.example".to_owned()]; + + for method in [&Method::GET, &Method::POST] { + for url in [ + "https://cdn.example/asset.js", + "//cdn.example/asset.js", + "https://CDN.Example/asset.js", + ] { + let req = build_proxy_sign_request( + method, + "https://edge.example/first-party/sign", + url, + ); + let err: Report = + handle_first_party_proxy_sign(&settings, &noop_services(), req) + .await + .expect_err("should reject excluded URL"); + + assert_eq!( + err.current_context().status_code(), + StatusCode::BAD_GATEWAY, + "{} should reject excluded URL `{url}` as unsupported", + method.as_str() + ); + } + } + }); + } +``` + +Directly after it, add: + +```rust + #[test] + fn proxy_sign_rejects_excluded_urls_case_insensitively() { + futures::executor::block_on(async { + let mut settings = create_test_settings(); + settings.rewrite.exclude_domains = vec!["CDN.Example".to_owned()]; + + for method in [&Method::GET, &Method::POST] { + for url in ["https://cdn.example/asset.js", "//cdn.example/asset.js"] { + let req = build_proxy_sign_request( + method, + "https://edge.example/first-party/sign", + url, + ); + let err: Report = + handle_first_party_proxy_sign(&settings, &noop_services(), req) + .await + .expect_err("should reject a host excluded by a mixed-case entry"); + + assert_eq!( + err.current_context().status_code(), + StatusCode::BAD_GATEWAY, + "{} should reject `{url}` excluded by a mixed-case entry as unsupported", + method.as_str() + ); + } + } + }); + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cargo test-fastly -- normalize_creative_url proxy_if_abs_respects unparseable_absolute exclude_domains_match_case proxy_sign_rejects_excluded` + +Expected: `error[E0432]: unresolved import super::normalize_creative_url`. `unparseable_absolute_click_url_is_left_byte_identical`, `exclude_domains_match_case_insensitively_in_the_rewrite_pass` and `proxy_sign_rejects_excluded_urls_case_insensitively` use only existing APIs. Against the pre-change code they fail at runtime: the anchor gains `data-tsclick`, the mixed-case-excluded link is wrapped, and sign returns `200`. Run each alone with `--exact --nocapture` to see the panic. + +- [ ] **Step 3: Implement the normalizer, helpers and call sites** + +Apply the `creative.rs` parts of [Shared step](#shared-step): `normalize_creative_url`, `asset_target`, `click_target`, the four call sites and the module doc. + +- [ ] **Step 4: Migrate `/first-party/sign`** + +Apply the `proxy.rs` parts of [Shared step](#shared-step): the new normalization block, and `target` in the `base`/`proxied` lines. `is_host_permitted` stays private until Task 4. + +- [ ] **Step 5: Remove `is_excluded`** + +Delete `Rewrite::is_excluded` (with its `#[allow(dead_code)]`) and the `test_rewrite_is_excluded` test (`settings.rs:6522-6546`); Task 1's tests cover its cases. + +Run: `git grep -n "to_abs\|is_excluded(" crates/` + +Expected: no matches. + +- [ ] **Step 6: Run the tests to verify they pass** + +Run: `cargo test-fastly -- normalize_creative_url proxy_if_abs_respects unparseable_absolute exclude_domains_match_case proxy_sign_` + +Expected: `12 passed` (the seven new or changed tests plus the five existing `proxy_sign_*` tests). + +Then prove empty-list parity with the full suite: + +Run: `cargo test-fastly` + +Expected: all pass with no edits to any existing rewrite-output assertion. If one fails, the refactor changed output; fix the code, not the assertion. + +- [ ] **Step 7: Lint** + +Run: `cargo fmt --all && cargo clippy-fastly` + +Expected: no warnings. + +- [ ] **Step 8: Commit** + +```bash +git add crates/trusted-server-core/src/creative.rs crates/trusted-server-core/src/proxy.rs crates/trusted-server-core/src/settings.rs +git commit --signoff -S -m "Split creative URL normalization from rewrite host policy" -m "Replace to_abs with normalize_creative_url and route asset, click and first-party sign decisions through Rewrite::should_proxy_asset and Rewrite::should_wrap_click." +``` + +--- + +### Task 3: `include_domains` setting, validation and blob compatibility + +**Files:** + +- Modify: `crates/trusted-server-core/src/settings.rs:636-643` (`struct Rewrite`), `Rewrite::normalize`, new validator before `validate_publisher_domain` (`:3384`) +- Test: `settings.rs` tests (before `test_auction_creative_processing_defaults_when_omitted`, `:6549`), `crates/trusted-server-core/src/config_payload.rs` tests (before `disabled_rewrite_creatives_survives_blob_round_trip`, `:739`) + +**Interfaces:** + +- Consumes: `Rewrite::normalize` from Task 1. +- Produces: `pub include_domains: Vec` on `Rewrite`; private `fn validate_include_domains(patterns: &[String]) -> Result<(), ValidationError>`. + +- [ ] **Step 1: Write the failing tests** + +In `settings.rs` tests: + +```rust + #[test] + fn rewrite_include_domains_are_normalized_from_toml() { + let toml_str = crate_test_settings_str() + + r#" + [rewrite] + include_domains = ["*.CDN.Example.com", " img.example.net "] + "#; + + let settings = Settings::from_toml(&toml_str).expect("should parse valid TOML"); + + assert_eq!( + settings.rewrite.include_domains, + vec!["*.cdn.example.com".to_owned(), "img.example.net".to_owned()], + "should trim and lowercase include_domains entries" + ); + } +``` + +```rust + #[test] + fn rewrite_include_domains_reject_malformed_entries() { + for entry in ["", " ", "*", "*.", "cdn.*.example.com", "**.example.com"] { + let toml_str = crate_test_settings_str() + + &format!("\n[rewrite]\ninclude_domains = [\"{entry}\"]\n"); + + let result = Settings::from_toml(&toml_str); + + assert!( + result.is_err(), + "should reject include_domains entry `{entry}`" + ); + } + } +``` + +```rust + #[test] + fn rewrite_include_domains_default_to_empty() { + let settings = create_test_settings(); + + assert!( + settings.rewrite.include_domains.is_empty(), + "should default include_domains to an empty list" + ); + } +``` + +In `config_payload.rs` tests: + +```rust + #[test] + fn legacy_blob_without_rewrite_include_domains_loads_with_empty_list() { + let data = + serde_json::to_value(test_settings()).expect("should serialize settings to JSON"); + let rewrite = data + .get("rewrite") + .and_then(serde_json::Value::as_object) + .expect("should serialize rewrite settings as an object"); + assert!( + !rewrite.contains_key("include_domains"), + "should omit the default include list from the payload" + ); + + let reconstructed = load_settings(&envelope_json(&test_settings())) + .expect("should reconstruct settings without include_domains"); + + assert!( + reconstructed.rewrite.include_domains.is_empty(), + "should load an empty include list from a blob without the key" + ); + } +``` + +```rust + #[test] + fn rewrite_include_domains_survive_blob_round_trip() { + let mut original = test_settings(); + original.rewrite.include_domains = vec!["*.cdn.example.com".to_owned()]; + + let reconstructed = load_settings(&envelope_json(&original)) + .expect("should reconstruct settings with include_domains"); + + assert_eq!( + reconstructed.rewrite.include_domains, + vec!["*.cdn.example.com".to_owned()], + "should preserve the configured include list" + ); + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cargo test-fastly -- rewrite_include_domains legacy_blob_without_rewrite` + +Expected: five `error[E0609]: no field include_domains on type settings::Rewrite`. + +- [ ] **Step 3: Implement** + +Replace `struct Rewrite` (`settings.rs:636-643`): + +```rust +#[derive(Debug, Default, Clone, Deserialize, Serialize, Validate)] +#[serde(deny_unknown_fields)] +pub struct Rewrite { + /// Hosts never rewritten, for assets and click-through links. + /// + /// Each entry is an exact host or `*.example.com`, which matches the apex + /// and any subdomain. Matching is case-insensitive. Wins over + /// [`Self::include_domains`]. + #[serde(default)] + pub exclude_domains: Vec, + /// When non-empty, the only hosts whose asset URLs are rewritten to + /// `/first-party/proxy`. + /// + /// Same pattern syntax as [`Self::exclude_domains`]. Empty (the default) + /// proxies every eligible asset URL. Never applies to click-through links. + /// The default is omitted from serialized config so older binaries, which + /// reject unknown fields, can still read it. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[validate(custom(function = validate_include_domains))] + pub include_domains: Vec, +} +``` + +At the end of `Rewrite::normalize`, after the `exclude_domains` warning block: + +```rust + self.include_domains = self + .include_domains + .iter() + .map(|pattern| pattern.trim().to_ascii_lowercase()) + .collect(); +``` + +Before `validate_publisher_domain`: + +```rust +/// Rejects `rewrite.include_domains` entries that cannot name a host. +/// +/// Runs after [`Rewrite::normalize`], so entries are already trimmed and +/// lowercased. An entry must be an exact host or `*.` followed by a non-empty +/// suffix with no further `*`. +fn validate_include_domains(patterns: &[String]) -> Result<(), ValidationError> { + for pattern in patterns { + let suffix = pattern.strip_prefix("*.").unwrap_or(pattern); + if suffix.is_empty() || suffix.contains('*') { + return Err(ValidationError::new("invalid_rewrite_include_domain")); + } + } + Ok(()) +} +``` + +The `validator` derive passes `&Vec`, which coerces to `&[String]`, so no `clippy::ptr_arg` allowance is needed. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cargo test-fastly -- rewrite_include_domains legacy_blob_without_rewrite` + +Expected: `6 passed` (the five new tests plus `legacy_blob_without_rewrite_creatives_preserves_rewriting`). + +- [ ] **Step 5: Lint** + +Run: `cargo fmt --all && cargo clippy-fastly` + +Expected: no warnings. + +- [ ] **Step 6: Commit** + +```bash +git add crates/trusted-server-core/src/settings.rs crates/trusted-server-core/src/config_payload.rs +git commit --signoff -S -m "Add rewrite include_domains setting with validation" +``` + +--- + +### Task 4: Apply `include_domains` to asset rewriting and `/first-party/sign` + +**Files:** + +- Modify: `crates/trusted-server-core/src/settings.rs` (import, `Rewrite::should_proxy_asset`) +- Modify: `crates/trusted-server-core/src/proxy.rs:1228` (`is_host_permitted` visibility), decline log line +- Test: `settings.rs` tests, `creative.rs` tests (before `rewrite_html_excludes_blacklisted_domains`), `proxy.rs` tests (before `proxy_sign_inherits_the_request_scheme_for_protocol_relative_urls`) + +**Interfaces:** + +- Consumes: `Rewrite::include_domains` (Task 3); `proxy::is_host_permitted` (made `pub(crate)` in Step 3); `rewrite_creative_html`, `rewrite_inline_creative_html(settings, base_origin, markup)`, `rewrite_proxied_html`, `rewrite_css_body(settings, css) -> Result`; `build_proxy_sign_request(method, uri, target)` and `noop_services()` (existing `proxy.rs` test helpers). +- Produces: `should_proxy_asset` with the include clause. No new public items. + +- [ ] **Step 1: Write the failing tests** + +In `settings.rs` tests: + +```rust + #[test] + fn rewrite_should_proxy_asset_honors_include_list() { + let mut rewrite = Rewrite::default(); + rewrite + .include_domains + .extend(["*.example.com", "img.example.net"].map(str::to_owned)); + rewrite + .exclude_domains + .push("blocked.example.com".to_owned()); + + for (host, expected) in [ + ("example.com", true), + ("a.b.example.com", true), + ("img.example.net", true), + ("IMG.EXAMPLE.NET", true), + ("evil-example.com", false), + ("cdn.example.org", false), + ("blocked.example.com", false), + ] { + assert_eq!( + rewrite.should_proxy_asset(host), + expected, + "should_proxy_asset(`{host}`) should be {expected}" + ); + } + for host in ["cdn.example.org", "evil-example.com", "example.com"] { + assert!( + rewrite.should_wrap_click(host), + "should wrap click for `{host}` regardless of include_domains" + ); + } + assert!( + !rewrite.should_wrap_click("blocked.example.com"), + "should not wrap a click to an excluded host" + ); + } +``` + +In `creative.rs`, add `rewrite_css_body` and `rewrite_proxied_html` to the test `use super::{...}` list, giving: + +```rust + use super::{ + CreativeCssProcessor, StreamProcessor as _, normalize_creative_url, + process_auction_creative, proxy_if_abs, rewrite_creative_html, rewrite_css_body, + rewrite_inline_creative_html, rewrite_proxied_html, rewrite_srcset, rewrite_style_urls, + sanitize_creative_html, + }; +``` + +Then add, before `rewrite_html_excludes_blacklisted_domains`: + +```rust + const ALLOWLIST_LISTED: &[&str] = &[ + "https://assets.example.com/a.css", + "https://assets.example.com/p1.png", + "https://assets.example.com/a.js", + "https://assets.example.com/i.css", + "https://assets.example.com/bg.png", + "https://assets.example.com/set.png", + "https://assets.example.com/img.png", + "https://assets.example.com/lazy.png", + "https://assets.example.com/s1.png", + "https://assets.example.com/v.mp4", + "https://assets.example.com/o.swf", + "https://assets.example.com/e.swf", + "https://assets.example.com/btn.png", + "https://assets.example.com/icon.svg", + "https://assets.example.com/sprite.svg", + "https://assets.example.com/frame.html", + "https://assets.example.com/inline.png", + ]; + + const ALLOWLIST_OFF_LIST: &[&str] = &[ + "https://cdn.example.net/a.css", + "https://cdn.example.net/p2.png", + "https://cdn.example.net/a.js", + "https://cdn.example.net/i.css", + "https://cdn.example.net/bg.png", + "https://cdn.example.net/set.png", + "https://cdn.example.net/img.png", + "https://cdn.example.net/lazy.png", + "https://cdn.example.net/s2.png", + "https://cdn.example.net/v.mp4", + "https://cdn.example.net/o.swf", + "https://cdn.example.net/e.swf", + "https://cdn.example.net/btn.png", + "https://cdn.example.net/icon.svg", + "https://cdn.example.net/sprite.svg", + "https://cdn.example.net/frame.html", + "https://cdn.example.net/inline.png", + ]; + + const ALLOWLIST_CREATIVE: &str = r#" + + + + + + + + + + + + + + + + + + + + + +
+
+Offer +Area +"#; + + fn encoded_tsurl(url: &str) -> String { + format!( + "tsurl={}", + url::form_urlencoded::byte_serialize(url.as_bytes()).collect::() + ) + } + + fn allowlist_settings() -> crate::settings::Settings { + let mut settings = crate::test_support::tests::create_test_settings(); + settings.rewrite.include_domains = vec!["*.example.com".to_owned()]; + settings + } + + fn assert_allowlist_applied(label: &str, out: &str) { + for listed in ALLOWLIST_LISTED { + assert!( + out.contains(&encoded_tsurl(listed)), + "{label}: should proxy listed URL `{listed}`: {out}" + ); + } + for off_list in ALLOWLIST_OFF_LIST { + assert!( + !out.contains(&encoded_tsurl(off_list)), + "{label}: should not proxy off-list URL `{off_list}`: {out}" + ); + assert!( + out.contains(off_list), + "{label}: should keep off-list URL `{off_list}` raw: {out}" + ); + } + for landing in [ + "https://landing.example.net/offer", + "https://landing.example.org/area", + ] { + assert!( + out.contains(&format!("/first-party/click?{}", encoded_tsurl(landing))), + "{label}: should wrap click `{landing}` regardless of include_domains: {out}" + ); + } + assert_eq!( + out.matches("data-tsclick=").count(), + 2, + "{label}: should mark both click-through links" + ); + } + + #[test] + fn include_domains_limit_asset_rewriting_on_every_html_path() { + let settings = allowlist_settings(); + + assert_allowlist_applied( + "auction", + &rewrite_creative_html(&settings, ALLOWLIST_CREATIVE), + ); + assert_allowlist_applied( + "inline", + &rewrite_inline_creative_html(&settings, "https://www.example.com", ALLOWLIST_CREATIVE), + ); + assert_allowlist_applied( + "proxied", + &rewrite_proxied_html(&settings, ALLOWLIST_CREATIVE), + ); + } + + #[test] + fn include_domains_limit_css_body_rewriting() { + let settings = allowlist_settings(); + let css = r#"@import "https://assets.example.com/i.css"; +@import "https://cdn.example.net/i.css"; +.a { background: url(https://assets.example.com/bg.png); } +.b { background: url(https://cdn.example.net/bg.png); }"#; + + let out = rewrite_css_body(&settings, css).expect("should rewrite CSS body"); + + for listed in [ + "https://assets.example.com/i.css", + "https://assets.example.com/bg.png", + ] { + assert!( + out.contains(&encoded_tsurl(listed)), + "should proxy listed CSS URL `{listed}`: {out}" + ); + } + for off_list in [ + "https://cdn.example.net/i.css", + "https://cdn.example.net/bg.png", + ] { + assert!( + out.contains(off_list) && !out.contains(&encoded_tsurl(off_list)), + "should keep off-list CSS URL `{off_list}` raw: {out}" + ); + } + } +``` + +```rust + #[test] + fn host_in_both_lists_is_left_alone() { + let mut settings = allowlist_settings(); + settings.rewrite.exclude_domains = vec!["assets.example.com".to_owned()]; + let html = r#"x"#; + + let out = rewrite_creative_html(&settings, html); + + assert!( + out.contains(r#""#), + "should leave an asset on a host in both lists raw: {out}" + ); + assert!( + out.contains(r#""#) + && !out.contains("data-tsclick"), + "should leave an excluded click-through link raw: {out}" + ); + } +``` + +In `proxy.rs` tests: + +```rust + #[test] + fn proxy_sign_applies_rewrite_include_domains_for_get_and_post() { + struct Case { + name: &'static str, + include_domains: &'static [&'static str], + exclude_domains: &'static [&'static str], + allowed_domains: &'static [&'static str], + target: &'static str, + expected: StatusCode, + } + + let cases = [ + Case { + name: "listed absolute", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "https://img.example.com/a.png", + expected: StatusCode::OK, + }, + Case { + name: "listed protocol-relative", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "//img.example.com/a.png", + expected: StatusCode::OK, + }, + Case { + name: "off-list absolute", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "https://img.example.net/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "off-list protocol-relative", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "//img.example.net/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "host in both lists", + include_domains: &["*.example.com"], + exclude_domains: &["img.example.com"], + allowed_domains: &[], + target: "https://img.example.com/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "off include list and off proxy allowlist", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &["*.example.org"], + target: "https://img.example.net/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "listed but off proxy allowlist", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &["*.example.org"], + target: "https://img.example.com/a.png", + expected: StatusCode::FORBIDDEN, + }, + Case { + name: "mixed-case host", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "https://IMG.Example.COM/a.png", + expected: StatusCode::OK, + }, + ]; + + let to_owned = |values: &[&str]| -> Vec { + values.iter().map(|value| (*value).to_owned()).collect() + }; + + futures::executor::block_on(async { + for case in &cases { + for method in [&Method::GET, &Method::POST] { + let label = format!("{} {}", method.as_str(), case.name); + let mut settings = create_test_settings(); + settings.rewrite.include_domains = to_owned(case.include_domains); + settings.rewrite.exclude_domains = to_owned(case.exclude_domains); + settings.proxy.allowed_domains = to_owned(case.allowed_domains); + let req = build_proxy_sign_request( + method, + "https://edge.example.com/first-party/sign", + case.target, + ); + + let status = + match handle_first_party_proxy_sign(&settings, &noop_services(), req).await + { + Ok(response) => response.status(), + Err(error) => error.current_context().status_code(), + }; + + assert_eq!( + status, case.expected, + "{label} should return {}", + case.expected + ); + } + } + }); + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run each separately, because a wasm panic stops the binary: + +```bash +cargo test-fastly -- rewrite_should_proxy_asset_honors --nocapture +cargo test-fastly -- include_domains_limit_asset --nocapture +cargo test-fastly -- include_domains_limit_css --nocapture +cargo test-fastly -- proxy_sign_applies_rewrite_include_domains --nocapture +``` + +Expected failures: + +- `should_proxy_asset(`evil-example.com`) should be false` +- `auction: should not proxy off-list URL `https://cdn.example.net/a.css`` +- `should keep off-list CSS URL `https://cdn.example.net/i.css` raw` +- `GET off-list absolute should return 502 Bad Gateway` + +`host_in_both_lists_is_left_alone` already passes here: `exclude_domains` wins without the include clause. It guards precedence after Step 3. + +- [ ] **Step 3: Implement the include clause** + +In `proxy.rs:1228`, widen `is_host_permitted`'s visibility only. This belongs to the include clause, not the shared step: + +```rust +pub(crate) fn is_host_permitted>(allowed_domains: &[S], host: &str) -> bool { +``` + +In `settings.rs`, widen the import: + +```rust +use crate::proxy::{is_host_allowed, is_host_permitted}; +``` + +Replace `should_proxy_asset` and its doc comment: + +```rust + /// Returns `true` when an asset URL on `host` should be rewritten to + /// `/first-party/proxy`. + /// + /// The host must not match [`Self::exclude_domains`], and must match + /// [`Self::include_domains`] when that list is non-empty. Matching is + /// case-insensitive; see [`is_host_allowed`] for the pattern rules. + #[must_use] + pub fn should_proxy_asset(&self, host: &str) -> bool { + !self.is_excluded_host(host) && is_host_permitted(&self.include_domains, host) + } +``` + +`is_host_permitted` returns `true` for an empty list, which is the "include list empty" half of the rule. + +In `proxy.rs`, update the decline log in `handle_first_party_proxy_sign`: + +```rust + log::debug!( + "sign request for `{host}` declined: host excluded or not in rewrite.include_domains" + ); +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cargo test-fastly -- rewrite_should_proxy_asset_honors include_domains_limit host_in_both_lists proxy_sign_` + +Expected: `12 passed`. + +- [ ] **Step 5: Run the full core suite and lint** + +Run: `cargo fmt --all && cargo test-fastly && cargo clippy-fastly` + +Expected: PASS, no warnings. + +- [ ] **Step 6: Commit** + +```bash +git add crates/trusted-server-core/src/settings.rs crates/trusted-server-core/src/creative.rs crates/trusted-server-core/src/proxy.rs +git commit --signoff -S -m "Limit creative asset rewriting to rewrite include_domains" -m "Apply the include list to every asset handler and to first-party sign. Click-through links and fetch-time proxy checks are unchanged." +``` + +--- + +### Task 5: Rewrite `` once on the inline path + +On ``, both the `link[href]` handler and the standalone `[imagesrcset]` handler rewrite `imagesrcset`. On the root-relative paths (`/auction`, proxied HTML) the second pass sees `/first-party/proxy?...`, a relative URL, and leaves it alone. On the inline path the first pass emits absolute `https:///first-party/proxy?...` URLs, so the second pass proxies them again, producing duplicated `tsurl` and `tstoken` parameters. The fix removes the `imagesrcset` handling from `link[href]`. `[imagesrcset]` already rewrites it once on every element. + +**Files:** + +- Modify: `crates/trusted-server-core/src/creative.rs` (`link[href]` handler in `rewrite_creative_html_impl`, `:1220-1242`) +- Test: `creative.rs` tests, before `host_in_both_lists_is_left_alone` + +**Interfaces:** none change. + +- [ ] **Step 1: Write the failing test and the root-relative guard** + +```rust + // `imagesrcset` is rewritten only by the `[imagesrcset]` handler. When the + // `link[href]` handler also rewrote it, the inline path's absolute proxy + // URLs were proxied a second time. + #[test] + fn inline_link_imagesrcset_is_proxied_once() { + let settings = crate::test_support::tests::create_test_settings(); + let html = r#""#; + + let out = rewrite_inline_creative_html(&settings, "https://publisher.example.com", html); + + let imagesrcset = out + .split("imagesrcset=\"") + .nth(1) + .and_then(|rest| rest.split('"').next()) + .expect("should keep an imagesrcset attribute"); + assert_eq!( + imagesrcset.matches("tsurl=").count(), + 1, + "should proxy the imagesrcset candidate exactly once: {imagesrcset}" + ); + } + + #[test] + fn root_relative_link_imagesrcset_is_proxied_once() { + let settings = crate::test_support::tests::create_test_settings(); + let html = r#""#; + + for (label, out) in [ + ("auction", rewrite_creative_html(&settings, html)), + ("proxied", rewrite_proxied_html(&settings, html)), + ] { + let imagesrcset = out + .split("imagesrcset=\"") + .nth(1) + .and_then(|rest| rest.split('"').next()) + .expect("should keep an imagesrcset attribute"); + assert_eq!( + imagesrcset.matches("tsurl=").count(), + 1, + "{label}: should proxy the imagesrcset candidate exactly once: {imagesrcset}" + ); + } + } +``` + +- [ ] **Step 2: Run the tests to verify the inline one fails** + +Run each alone (a wasm panic aborts the whole test binary): + +```bash +cargo test-fastly -p trusted-server-core --lib -- --exact creative::tests::inline_link_imagesrcset_is_proxied_once --nocapture +cargo test-fastly -p trusted-server-core --lib -- --exact creative::tests::root_relative_link_imagesrcset_is_proxied_once +``` + +Expected: + +- The inline test panics with "should proxy the imagesrcset candidate exactly once: https://publisher.example.com/first-party/proxy?tsurl=https%3A%2F%2Fpublisher.example.com%2Ffirst-party%2Fproxy&tsurl=...". +- The root-relative guard passes. Those paths were never affected. + +- [ ] **Step 3: Implement** + +In the `link[href]` handler, delete the `imagesrcset` block and keep the `href` handling: + +```rust + // Stylesheets and preloads + element!("link[href]", |el| { + let rel = el + .get_attribute("rel") + .unwrap_or_default() + .to_ascii_lowercase(); + if rel.contains("stylesheet") + || rel.contains("preload") + || rel.contains("prefetch") + { + // `imagesrcset` is left to the `[imagesrcset]` handler, which + // also runs on this element; rewriting it here too would + // proxy the inline path's absolute proxy URLs a second time. + if let Some(p) = + proxied_attr_value(settings, el.get_attribute("href"), base_origin) + { + let _ = el.set_attribute("href", &p); + } + } + Ok(()) + }), +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cargo test-fastly -p trusted-server-core --lib -- imagesrcset include_domains_limit` + +Expected: `9 passed`. This includes both new tests, `include_domains_limit_asset_rewriting_on_every_html_path` (whose inline case uses the realistic `https://www.example.com` origin), and the existing `imagesrcset` rewrite and sanitizer tests. + +- [ ] **Step 5: Run the full core suite and lint** + +Run: `cargo fmt --all && cargo test-fastly && cargo clippy-fastly` + +Expected: PASS, no warnings. + +- [ ] **Step 6: Commit** + +```bash +git add crates/trusted-server-core/src/creative.rs +git commit --signoff -S -m "Rewrite link imagesrcset once on the inline creative path" +``` + +--- + +### Task 6: Operator documentation, example config and changelog + +**Files:** + +- Modify: `trusted-server.example.toml:127-129` +- Modify: `docs/guide/configuration.md:985-1068` +- Modify: `docs/guide/creative-processing.md:637-700` +- Modify: `docs/guide/first-party-proxy.md:173`, `:509-521` +- Modify: `docs/guide/api-reference.md:565-570` +- Modify: `CHANGELOG.md` (`## [Unreleased]`: `### Fixed` at `:34`, `### Added` at `:41`; both already exist) + +- [ ] **Step 1: `trusted-server.example.toml`** + +Replace lines 127-129 with: + +```toml +# [rewrite] +# Hosts never rewritten: assets stay direct and click-through links are not +# wrapped. Supports "*.example.com" wildcards (matches the apex too). +# exclude_domains = ["*.cdn.example.com"] +# When non-empty, only asset URLs on these hosts are proxied; other assets load +# directly. Click-through links are not affected. Set this in TOML; environment +# overrides cannot replace arrays. +# include_domains = ["assets.example.com", "*.img.example.com"] +``` + +- [ ] **Step 2: `docs/guide/configuration.md`** + +- Section intro (`:989`): "Control which hosts first-party rewriting covers." +- `[rewrite]` table (`:993-995`): + +```markdown +| Field | Type | Required | Description | +| ----------------- | ------------- | ---------------- | --------------------------------------------------------------------------------------------------- | +| `exclude_domains` | Array[String] | No (default: []) | Hosts never rewritten, for assets and click-through links | +| `include_domains` | Array[String] | No (default: []) | When non-empty, the only hosts whose asset URLs are proxied. Does not apply to click-through links. | +``` + +- Add `include_domains = ["assets.example.com", "*.img.example.com"]` to the example and change the Environment Override text to "EdgeZero v0.0.4 cannot replace these arrays or address their elements by index. Edit `exclude_domains` and `include_domains` in TOML, then validate and push the file again." +- After the exact-pattern list, add: + +```markdown +**Normalization and precedence**: + +- Both lists are trimmed and lowercased at load, so matching is case-insensitive. +- `exclude_domains` wins when a host matches both lists. +- `include_domains` rejects empty entries, a bare `*`, and any `*` other than a + leading `*.`. Empty and bare `*` entries in `exclude_domains` are dropped with + a warning, because they never match a host. + +::: tip +When both `include_domains` and `proxy.allowed_domains` are set, keep every +`include_domains` host inside `proxy.allowed_domains`. Otherwise a listed asset +is rewritten to a proxy URL that then fails with `403`. +::: +``` + +- [ ] **Step 3: `docs/guide/creative-processing.md`** + +- Wildcard example (`:656-663`): replace `❌ cdn.example.com (no subdomain)` with `✅ cdn.example.com (base domain)`. +- Under `### Pattern Matching`, add "Matching is case-insensitive. Entries are trimmed and lowercased at load." and change the wildcard lead-in to "`*.` matches the base domain and any subdomain". +- After the Exclude Domains use cases, add: + +```markdown +## Include Domains + +Limit asset rewriting to hosts you list: + + [rewrite] + include_domains = ["assets.example.com", "*.img.example.com"] + +- When `include_domains` is empty (the default), every eligible asset URL is + proxied. +- When it is non-empty, only asset URLs whose host matches an entry are + rewritten to `/first-party/proxy`. Other assets keep their original URL, load + directly, and do not receive the EC ID that proxied fetches append. +- `exclude_domains` wins when a host matches both lists. +- Click-through links (``, ``) are not affected; they are + still wrapped in `/first-party/click` unless excluded. +- HTML and CSS fetched through `/first-party/proxy` go through the same rewrite + pass and follow the same list. +- `/first-party/sign` declines off-list hosts with a non-`403` error, so the + creative runtime loads them directly. + +Patterns use the same syntax as `exclude_domains`. +``` + +(In the guide the TOML is a fenced `toml` block.) + +- [ ] **Step 4: `docs/guide/first-party-proxy.md`** + +- Rename `### URL Rewrite Exclusions` (`:509`) to `### URL Rewrite Host Lists`, and after "URLs matching these patterns will NOT be rewritten to `/first-party/proxy`." add a fenced `toml` example with `include_domains = ["assets.example.com", "*.img.example.com"]` followed by: "When `include_domains` is non-empty, only asset URLs on matching hosts are rewritten. `exclude_domains` wins when a host matches both lists, and click-through links ignore `include_domains`." +- Before "A valid host outside a non-empty allowlist returns `403 Forbidden`." (`:173`), add the paragraph: "A host excluded by `rewrite.exclude_domains`, or outside a non-empty `rewrite.include_domains`, returns `502`. The creative runtime treats it as a non-policy failure and loads the URL directly. These checks run before `proxy.allowed_domains`." + +- [ ] **Step 5: `docs/guide/api-reference.md`** + +In `/first-party/sign` Error Responses, after the `413` bullet: + +```markdown +- `502 Bad Gateway`: The target host matches `rewrite.exclude_domains` or is outside a non-empty `rewrite.include_domains` +``` + +- [ ] **Step 6: `CHANGELOG.md`** + +Append to the existing `### Fixed` list under `## [Unreleased]`: + +```markdown +- Inline SSAT/page-bids creatives no longer proxy `` candidates twice. The `imagesrcset` attribute is now rewritten once, by the same handler on every creative path; previously the inline path's absolute proxy URLs were wrapped in a second `/first-party/proxy` URL. +- `rewrite.exclude_domains` matching is now case-insensitive and ignores surrounding whitespace. Entries written with uppercase letters previously never matched and now take effect, so those hosts stop being proxied and click-wrapped. Audit `exclude_domains` for mixed-case entries before upgrading. +``` + +Insert at the top of the existing `### Added` list under `## [Unreleased]`: + +```markdown +- `[rewrite] include_domains` limits creative asset rewriting to listed hosts. When non-empty, only asset URLs on matching hosts are proxied through `/first-party/proxy`, so only those hosts receive the EC ID on proxied fetches. Other assets keep their original URL, and `/first-party/sign` returns `502` for them so the creative runtime loads them directly. Click-through links are not affected, and `exclude_domains` still wins. +``` + +- [ ] **Step 7: Format and check** + +Run: `cd docs && npm ci && cd .. && docs/node_modules/.bin/prettier --config docs/.prettierrc --write docs/guide/configuration.md docs/guide/creative-processing.md docs/guide/first-party-proxy.md docs/guide/api-reference.md CHANGELOG.md` + +Expected: only `configuration.md` changes (table column alignment); the rest report `(unchanged)`. + +- [ ] **Step 8: Commit** + +```bash +git add docs/guide/configuration.md docs/guide/creative-processing.md docs/guide/first-party-proxy.md docs/guide/api-reference.md trusted-server.example.toml CHANGELOG.md +git commit --signoff -S -m "Document rewrite include_domains and case-insensitive exclusions" +``` + +--- + +### Task 7: Full CI gate + +**Files:** none. + +- [ ] **Step 1: Format and lint** + +```bash +cargo fmt --all -- --check +cargo clippy-fastly && cargo clippy-axum && cargo clippy-cloudflare && cargo clippy-cloudflare-wasm \ + && cargo clippy-spin-native && cargo clippy-spin-wasm && cargo clippy-cli && cargo clippy-codegen +``` + +Expected: all pass. + +- [ ] **Step 2: Tests** + +```bash +cargo test-fastly && cargo test-fastly-reuse && cargo test-axum && cargo test-cloudflare && cargo test-spin +cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity +./scripts/test-cli.sh +``` + +Expected on `7a0ecb4c` plus this change: `test-fastly` 3155 passed, 10 ignored (core 2908 passed, 6 ignored); `test-fastly-reuse` 219 passed; `test-axum` 43 passed; `test-cloudflare` 53 passed; `test-spin` 87 passed; parity 17 passed; CLI 733 passed. + +- [ ] **Step 3: JS and docs** + +```bash +cd crates/trusted-server-js/lib && npx vitest run && npm run format && cd - +cd docs && npm run format && cd - +docs/node_modules/.bin/prettier --config docs/.prettierrc --check "*.md" ".claude/**/*.md" ".github/**/*.md" "crates/**/*.md" "scripts/**/*.md" "tinybird/**/*.md" +``` + +Expected: all pass. JS is unchanged. + +- [ ] **Step 4: Spec conformance** + +| Criterion | Evidence | +| --------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------ | +| Empty list leaves existing rewrite assertions unchanged | Task 2 Step 6 and Task 4 Step 5: full suite, no assertion edits | +| Listed proxied, off-list raw, every handler, three HTML paths | `include_domains_limit_asset_rewriting_on_every_html_path`, `include_domains_limit_css_body_rewriting` | +| Host in both lists left alone | `host_in_both_lists_is_left_alone`, sign case "host in both lists" | +| Case-insensitive; `*.example.com` matches apex | Task 1 and Task 4 policy tests, `rewrite_exclude_domains_match_mixed_case_entries_from_toml` | +| Sign returns `502` off-list for absolute and `//`, GET and POST | `proxy_sign_applies_rewrite_include_domains_for_get_and_post` | +| Anchors wrapped regardless of allowlist | `assert_allowlist_applied` click assertions | +| Default omitted from serialized blob | `legacy_blob_without_rewrite_include_domains_loads_with_empty_list` | +| Docs, example TOML, CHANGELOG | Task 6 | + +- [ ] **Step 5: Open the PR** + +Use the `pr-creator` agent. Link the spec and #1231. State whether this PR introduced the shared step (Tasks 1-2) or rebased onto #1234's. + +## Notes + +- **Unparseable absolute URLs.** Before this change an absolute-looking value that `url::Url::parse` rejects had its attribute rewritten to its own value, re-quoted with double quotes. After, it is left untouched. `unparseable_absolute_url_is_left_byte_identical` and `unparseable_absolute_click_url_is_left_byte_identical` pin the new behavior. Against the pre-change code both were observed failing: `` came back re-quoted, and the anchor also gained `data-tsclick`. diff --git a/docs/superpowers/specs/2026-10-07-1231-creative-asset-host-allowlist-design.md b/docs/superpowers/specs/2026-10-07-1231-creative-asset-host-allowlist-design.md new file mode 100644 index 000000000..65aa552bc --- /dev/null +++ b/docs/superpowers/specs/2026-10-07-1231-creative-asset-host-allowlist-design.md @@ -0,0 +1,409 @@ +# Creative asset host allowlist + +**Issue:** [IABTechLab/trusted-server#1231](https://github.com/IABTechLab/trusted-server/issues/1231) + +**Date:** 2026-10-07 + +**Status:** Draft (design validated by an exploratory implementation; see the plan, `docs/superpowers/plans/2026-10-07-1231-creative-asset-host-allowlist.md`) + +**Related:** [#1234](https://github.com/IABTechLab/trusted-server/issues/1234) (separate click-rewriting switch). Both issues replace `to_abs`; see [Coordination with #1234](#coordination-with-1234). + +Line numbers below are against `main` at `7a0ecb4c`. + +## Problem + +The creative rewrite pass proxies every absolute HTTP(S) asset URL in a creative through `/first-party/proxy` unless its host matches `[rewrite] exclude_domains`. An operator cannot say "proxy only these hosts". + +That matters for more than egress control. A proxied fetch appends the visitor's EC ID to the upstream URL (`append_ec_id`, `crates/trusted-server-core/src/proxy.rs:1204`, called at `proxy.rs:793`), so today every third-party asset host that appears in a winning creative receives `ts-ec`. A deny list cannot bound that set, because creatives name new hosts all the time. + +`proxy.allowed_domains` does not solve it. It is the fetch-time SSRF control, shared with the Prebid external bundle fetch. With it set, the rewriter still turns off-list URLs into `/first-party/proxy` links, and the fetch then fails with `403`. The asset breaks instead of loading directly. + +A second, smaller defect sits in the same code: `Rewrite::is_excluded` compares raw config entries against a host the `url` crate has already lowercased, so an entry written with an uppercase letter never matches. + +## Goals + +- Add an asset host allowlist, `[rewrite] include_domains`. Empty (the default) keeps today's behavior. +- When non-empty, rewrite only asset URLs whose host matches the list. Every other asset URL keeps its original bytes. +- `exclude_domains` wins when a host matches both lists. +- Apply the allowlist to every asset handler in the pass, on all three entry points, including `srcset` candidates and CSS `url()`, `image-set()` strings and `@import`. +- Never apply the allowlist to anchors (``, ``). +- Apply the same policy at `/first-party/sign` for absolute and protocol-relative input, returning the same non-`403` error excluded hosts already get. +- Use one case-insensitive host matcher for both rewrite lists, which also fixes `exclude_domains` case sensitivity. +- Keep older binaries able to read config blobs written with the default. +- Fix the inline path's double rewrite of ``, so each candidate is proxied exactly once on every path. + +## Non-goals + +- Click rewriting policy or a click switch. That is #1234. +- Domain checks at `/first-party/proxy`, `/first-party/click` or `/first-party/proxy-rebuild`. Fetch-time SSRF stays with `proxy.allowed_domains`, and the click endpoints redirect any validly signed target by design (`proxy.rs:1898-1907`). +- Narrowing EC forwarding on click redirects (`handle_first_party_click`, `proxy.rs:1545`) or on proxy redirect hops. The allowlist only decides which asset URLs become proxy URLs. +- Cross-validating `include_domains` against `proxy.allowed_domains`. The docs recommend keeping the allowlist inside the proxy allowlist; a load-time warning can be a follow-up. +- Client-side changes. The render guard asks `/first-party/sign` and follows its answer. +- Environment-overlay support for the list. EdgeZero overlays only replace scalar leaves that already exist in the TOML, so the list is set in `trusted-server.toml` and published with `ts config push`, like `exclude_domains`. + +## Current behavior + +### The rewrite pass + +`rewrite_creative_html_impl` (`crates/trusted-server-core/src/creative.rs:1125`) is one lol_html pass. Its asset handlers live at `creative.rs:1180-1339`: + +| Handler | Lines | Per-URL helper | +| ------------------------------------------------------------------ | --------- | -------------------------------------- | +| `img` `src`, `data-src` | 1180-1192 | `proxy_if_abs` | +| `script[src]` | 1194-1201 | `proxied_attr_value` → `proxy_if_abs` | +| `link[href]` (stylesheet, preload, prefetch) and its `imagesrcset` | 1203-1225 | `proxied_attr_value`, `rewrite_srcset` | +| `video`, `audio`, `source` `src` | 1227-1234 | `proxied_attr_value` | +| `object[data]`, `embed[src]` | 1236-1251 | `proxied_attr_value` | +| `input[type=image]` | 1253-1267 | `proxied_attr_value` | +| SVG `image`/`use` `href`, `xlink:href` | 1269-1281 | `proxied_attr_value` | +| `[style]` attribute | 1294-1302 | `CssUrlRewriter::rewrite` | +| `
AdExcludedMail"#; + const MATRIX_FIXTURE_FRAGMENT: &str = r#"
AdExcludedMail"#; + const INLINE_ORIGIN: &str = "https://www.example.com"; + + #[derive(Debug, Clone, Copy)] + enum CreativePath { + Auction, + Inline, + } + + fn process_for_path(settings: &Settings, path: CreativePath, html: &str) -> String { + match path { + CreativePath::Auction => process_auction_creative(settings, html), + CreativePath::Inline => { + super::process_inline_auction_creative(settings, INLINE_ORIGIN, html) + } + } + } + + fn matrix_settings(assets: bool, clicks: Option) -> Settings { + let mut settings = crate::test_support::tests::create_test_settings(); + settings.auction.sanitize_creatives = false; + settings.auction.rewrite_creatives = assets; + settings.auction.rewrite_clicks = clicks; + settings.rewrite.exclude_domains = vec!["excluded.example.com".to_owned()]; + settings + } + fn rewrite_srcset_attr(attr_name: &str, attr_value: &str) -> String { let settings = crate::test_support::tests::create_test_settings(); let html = format!(r#""#); @@ -1713,6 +1834,141 @@ mod tests { ); } + #[test] + fn asset_and_click_switches_combine_on_auction_and_inline_paths() { + for path in [CreativePath::Auction, CreativePath::Inline] { + for (fixture, has_body) in [ + (MATRIX_FIXTURE_WITH_BODY, true), + (MATRIX_FIXTURE_FRAGMENT, false), + ] { + for (assets, clicks) in [(true, true), (true, false), (false, true), (false, false)] + { + let settings = matrix_settings(assets, Some(clicks)); + let label = format!("{path:?} assets={assets} clicks={clicks} body={has_body}"); + let prefix = match path { + CreativePath::Auction => "", + CreativePath::Inline => INLINE_ORIGIN, + }; + + let out = process_for_path(&settings, path, fixture); + + let expected_proxied = match (assets, has_body) { + (false, _) => 0, + (true, true) => 4, + (true, false) => 3, + }; + assert_eq!( + out.matches("/first-party/proxy?tsurl=").count(), + expected_proxied, + "{label}: proxied asset URL count: {out}" + ); + assert_eq!( + out.contains(&format!("src=\"{prefix}/first-party/proxy?tsurl=")), + assets, + "{label}: image proxied only when assets are on: {out}" + ); + assert_eq!( + out.contains(r#"src="https://cdn.example.com/ad.png""#), + !assets, + "{label}: image raw only when assets are off: {out}" + ); + + // Each wrapped `` and `` carries the click URL in + // href and data-tsclick. + let expected_click_urls = if clicks { 4 } else { 0 }; + assert_eq!( + out.matches("/first-party/click?tsurl=").count(), + expected_click_urls, + "{label}: click URL count: {out}" + ); + assert_eq!( + out.contains(&format!("href=\"{prefix}/first-party/click?tsurl=")), + clicks, + "{label}: anchor wrapped only when clicks are on: {out}" + ); + assert_eq!( + out.contains(r#"href="https://landing.example.com/page""#), + !clicks, + "{label}: anchor raw only when clicks are off: {out}" + ); + assert_eq!( + out.contains("data-tsclick"), + clicks, + "{label}: data-tsclick only when clicks are on: {out}" + ); + assert!( + out.contains(r#"href="https://excluded.example.com/page""#), + "{label}: excluded anchor always stays raw: {out}" + ); + assert!( + out.contains(r#"href="mailto:ads@example.com""#), + "{label}: mailto anchor always stays raw: {out}" + ); + + assert_eq!( + out.contains(" removed whenever either switch is on: {out}" + ); + let expected_tsjs = + usize::from(matches!(path, CreativePath::Auction) && (assets || clicks)); + assert_eq!( + out.matches("/static/tsjs=").count(), + expected_tsjs, + "{label}: TSJS injected once on /auction when either switch is on: {out}" + ); + if !assets && !clicks { + assert_eq!( + out, fixture, + "{label}: should pass through byte for byte with both switches off" + ); + } + } + } + } + } + + #[test] + fn unset_rewrite_clicks_preserves_existing_output() { + for path in [CreativePath::Auction, CreativePath::Inline] { + let unset = matrix_settings(true, None); + let explicit = matrix_settings(true, Some(true)); + assert_eq!( + process_for_path(&unset, path, MATRIX_FIXTURE_WITH_BODY), + process_for_path(&explicit, path, MATRIX_FIXTURE_WITH_BODY), + "{path:?}: unset rewrite_clicks with rewrite_creatives = true should match full rewriting" + ); + + let disabled = matrix_settings(false, None); + assert_eq!( + process_for_path(&disabled, path, MATRIX_FIXTURE_WITH_BODY), + MATRIX_FIXTURE_WITH_BODY, + "{path:?}: unset rewrite_clicks with rewrite_creatives = false should pass through" + ); + } + } + + // Captured from `main` before `rewrite_clicks` existed: default settings must + // keep producing these bytes on both auction paths. + const MATRIX_DEFAULT_AUCTION_OUTPUT: &str = r#"
AdExcludedMail"#; + const MATRIX_DEFAULT_INLINE_OUTPUT: &str = r#"
AdExcludedMail"#; + + #[test] + fn default_settings_rewrite_matrix_fixture_byte_for_byte_as_before() { + let settings = matrix_settings(true, None); + + for (path, expected) in [ + (CreativePath::Auction, MATRIX_DEFAULT_AUCTION_OUTPUT), + (CreativePath::Inline, MATRIX_DEFAULT_INLINE_OUTPUT), + ] { + assert_eq!( + process_for_path(&settings, path, MATRIX_FIXTURE_WITH_BODY), + expected, + "{path:?}: default settings should match the pre-rewrite_clicks output exactly" + ); + } + } + fn normalized(raw: &str) -> Option { normalize_creative_url(raw).map(|url| url.as_str().to_owned()) } diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index 61bc7efe5..254118f6f 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -22668,6 +22668,50 @@ mod tests { ); } + #[test] + fn build_bid_map_keeps_inline_anchors_raw_when_clicks_are_off() { + let mut settings = test_settings(); + settings.auction.rewrite_creatives = true; + settings.auction.rewrite_clicks = Some(false); + settings.publisher.domain = "example.com".to_string(); + + let mut winning_bids = HashMap::new(); + let mut bid = make_bid( + "atf_sidebar_ad", + 1.50, + "examplessp", + "abc123", + "https://ssp.example.com/win", + "https://ssp.example.com/bill", + ); + bid.creative = Some( + "" + .to_string(), + ); + winning_bids.insert("atf_sidebar_ad".to_string(), bid); + + let map = build_bid_map(&winning_bids, PriceGranularity::Dense, &settings, "", false); + let adm = map + .get("atf_sidebar_ad") + .and_then(|v| v.as_object()) + .and_then(|o| o.get("adm")) + .and_then(|v| v.as_str()) + .expect("should include a rewritten adm"); + + assert!( + adm.contains("https://example.com/first-party/proxy?tsurl="), + "should still proxy the image with an absolute URL, got: {adm}" + ); + assert!( + adm.contains("href=\"https://landing.example.com/page\""), + "should leave the landing link raw, got: {adm}" + ); + assert!( + !adm.contains("data-tsclick") && !adm.contains("/first-party/click"), + "should not wrap the landing link, got: {adm}" + ); + } + #[test] fn build_bid_map_uses_request_origin_for_inline_urls() { // The inline adm's absolute first-party URLs must resolve against the From 35dd1d197ff6a1d9dad99d709ca996b77d0277b0 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 15:04:25 +0530 Subject: [PATCH 05/20] Apply an explicit rewrite_clicks to HTML fetched through the first-party proxy Signed-off-by: dhruv8sh --- crates/trusted-server-core/src/creative.rs | 16 ++++- crates/trusted-server-core/src/proxy.rs | 78 ++++++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/crates/trusted-server-core/src/creative.rs b/crates/trusted-server-core/src/creative.rs index 1fcb9b84c..074e5f35d 100644 --- a/crates/trusted-server-core/src/creative.rs +++ b/crates/trusted-server-core/src/creative.rs @@ -1125,6 +1125,10 @@ pub fn rewrite_creative_html(settings: &Settings, markup: &str) -> String { /// own [`MAX_REWRITABLE_BODY_SIZE`] rather than the per-creative auction cap: /// a proxied document is a whole page, not an `adm`, and legitimately exceeds /// 1 MiB. The creative runtime is still injected so click mediation survives. +/// +/// Asset rewriting here is unconditional. Click wrapping follows an explicit +/// `[auction] rewrite_clicks`; when that is unset, links keep being wrapped +/// regardless of `rewrite_creatives`. #[must_use] pub fn rewrite_proxied_html(settings: &Settings, markup: &str) -> String { rewrite_creative_html_impl( @@ -1133,7 +1137,10 @@ pub fn rewrite_proxied_html(settings: &Settings, markup: &str) -> String { "", true, MAX_REWRITABLE_BODY_SIZE, - CreativeFeatures::ALL, + CreativeFeatures { + assets: true, + clicks: settings.auction.rewrites_proxied_clicks(), + }, ) } @@ -1510,10 +1517,13 @@ fn rewrite_creative_html_impl( rewritten } -/// Stream processor for creative HTML that rewrites URLs to first-party proxy. +/// Stream processor for HTML fetched through `/first-party/proxy` that rewrites +/// asset and click-through URLs to first-party endpoints. /// /// This processor buffers input chunks and processes the complete HTML document -/// when the stream ends, using `rewrite_creative_html` internally. +/// when the stream ends, using [`rewrite_proxied_html`] internally. Asset URLs are +/// always proxied; links are wrapped unless `[auction] rewrite_clicks` is +/// explicitly `false`. pub struct CreativeHtmlProcessor<'a> { settings: &'a Settings, buffer: Vec, diff --git a/crates/trusted-server-core/src/proxy.rs b/crates/trusted-server-core/src/proxy.rs index 6d1805c25..1b9139753 100644 --- a/crates/trusted-server-core/src/proxy.rs +++ b/crates/trusted-server-core/src/proxy.rs @@ -3655,6 +3655,84 @@ mod tests { ); } + #[test] + fn proxied_html_click_wrapping_follows_explicit_rewrite_clicks() { + let html = r#"Excluded"#; + // (rewrite_creatives, rewrite_clicks, expect wrapped clicks) + let cases = [ + (true, None, true), + (false, None, true), + (true, Some(true), true), + (true, Some(false), false), + (false, Some(false), false), + (false, Some(true), true), + ]; + + for (rewrite_creatives, rewrite_clicks, expect_clicks) in cases { + let mut settings = create_test_settings(); + settings.auction.rewrite_creatives = rewrite_creatives; + settings.auction.rewrite_clicks = rewrite_clicks; + settings.rewrite.exclude_domains = vec!["excluded.example.com".to_owned()]; + let label = + format!("rewrite_creatives={rewrite_creatives} rewrite_clicks={rewrite_clicks:?}"); + let req = build_http_request(Method::GET, "https://edge.example.com/first-party/proxy"); + let mut response = build_http_response(StatusCode::OK, EdgeBody::from(html)); + response.headers_mut().insert( + header::CONTENT_TYPE, + HeaderValue::from_static("text/html; charset=utf-8"), + ); + + let body = response_body_string( + finalize( + &settings, + &req, + "https://cdn.example.com/creative.html", + response, + ) + .expect("should finalize proxied HTML"), + ); + + assert!( + body.contains("/first-party/proxy?tsurl="), + "{label}: proxied HTML always proxies assets: {body}" + ); + // A wrapped landing link carries the click URL in href and data-tsclick. + assert_eq!( + body.matches("/first-party/click?tsurl=").count(), + if expect_clicks { 2 } else { 0 }, + "{label}: click wrapping in proxied HTML: {body}" + ); + assert_eq!( + body.matches("data-tsclick").count(), + usize::from(expect_clicks), + "{label}: data-tsclick in proxied HTML: {body}" + ); + if !expect_clicks { + let landing_href = body + .split("Excluded"#), + "{label}: excluded link always stays raw: {body}" + ); + assert!( + !body.contains(": {body}" + ); + assert!( + body.contains("/static/tsjs="), + "{label}: proxied HTML always receives the runtime: {body}" + ); + } + } + #[test] fn html_response_rewrite_preserves_non_standard_port() { // Verify that HTML rewriting preserves non-standard ports in sub-resource URLs. From 0396aba1f48471f61a56ffca6626556fc5055b87 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 15:04:44 +0530 Subject: [PATCH 06/20] Document rewrite_clicks in the example config and pin its overlay behavior Signed-off-by: dhruv8sh --- .../tests/config_env_overlay.rs | 71 +++++++++++++++++++ trusted-server.example.toml | 29 +++++--- 2 files changed, 91 insertions(+), 9 deletions(-) diff --git a/crates/trusted-server-cli/tests/config_env_overlay.rs b/crates/trusted-server-cli/tests/config_env_overlay.rs index d81b0366a..06cc3c058 100644 --- a/crates/trusted-server-cli/tests/config_env_overlay.rs +++ b/crates/trusted-server-cli/tests/config_env_overlay.rs @@ -29,6 +29,7 @@ ids = ["trusted_server_secrets"] "#; const REWRITE_ENV: &str = "TRUSTED_SERVER__AUCTION__REWRITE_CREATIVES"; const SANITIZE_ENV: &str = "TRUSTED_SERVER__AUCTION__SANITIZE_CREATIVES"; +const CLICKS_ENV: &str = "TRUSTED_SERVER__AUCTION__REWRITE_CLICKS"; const GAM_ATTRIBUTION_ENV: &str = "TRUSTED_SERVER__INTEGRATIONS__GPT__GAM_ATTRIBUTION_ENABLED"; const AD_TEMPLATES_ENABLED_ENV: &str = "TRUSTED_SERVER__CREATIVE_OPPORTUNITIES__ENABLED"; const PROVIDER_ENDPOINT_ENV: &str = "TRUSTED_SERVER__AUCTION__PROVIDERS__PBS-MAIN__ENDPOINT"; @@ -82,6 +83,76 @@ fn validate_with_overlay(project: &MigratedProject, raw_value: &str) -> Output { .expect("should run ts config validate") } +fn pushed_auction_with_env( + project: &MigratedProject, + key: &str, + raw_value: &str, +) -> serde_json::Value { + let output = Command::new(env!("CARGO_BIN_EXE_ts")) + .args(["config", "push", "--adapter", "axum", "--manifest"]) + .arg(&project.manifest_path) + .arg("--app-config") + .arg(&project.config_path) + .args(["--yes", "--no-diff"]) + .current_dir(project.directory.path()) + .env(key, raw_value) + .output() + .expect("should run ts config push"); + assert!( + output.status.success(), + "config push should succeed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let local_store_path = project + .directory + .path() + .join(".edgezero/local-config-trusted_server_config.json"); + let local_store: serde_json::Value = serde_json::from_str( + &fs::read_to_string(local_store_path).expect("should read pushed local config"), + ) + .expect("should parse local config store"); + let envelope_json = local_store + .as_object() + .and_then(|entries| entries.values().next()) + .and_then(serde_json::Value::as_str) + .expect("should contain a blob envelope"); + let envelope: serde_json::Value = + serde_json::from_str(envelope_json).expect("should parse blob envelope"); + envelope["data"]["auction"].clone() +} + +#[test] +fn rewrite_clicks_environment_override_applies_when_leaf_present() { + let project = migrated_project(); + let mut document = fs::read_to_string(&project.config_path) + .expect("should read migrated config") + .parse::() + .expect("should parse migrated config"); + document["auction"]["rewrite_clicks"] = value(true); + fs::write(&project.config_path, document.to_string()).expect("should write config"); + + let auction = pushed_auction_with_env(&project, CLICKS_ENV, "false"); + + assert_eq!( + auction["rewrite_clicks"], + serde_json::Value::Bool(false), + "pushed config should contain the rewrite_clicks environment override" + ); +} + +#[test] +fn rewrite_clicks_environment_override_is_ignored_without_leaf() { + let project = migrated_project(); + + let auction = pushed_auction_with_env(&project, CLICKS_ENV, "false"); + + assert!( + auction.get("rewrite_clicks").is_none(), + "an override for a missing leaf should be ignored and the unset default omitted" + ); +} + #[test] fn config_validate_explains_legacy_provider_list_migration() { let project = migrated_project(); diff --git a/trusted-server.example.toml b/trusted-server.example.toml index e841515ae..d4789570c 100644 --- a/trusted-server.example.toml +++ b/trusted-server.example.toml @@ -261,18 +261,29 @@ proxy = "enabled" # Keep disabled until provider endpoints, routes, and profile values below are # replaced with deployment-specific settings. enabled = false -# Rewrite winning-bid creative HTML to first-party endpoints (default true). Set -# false to skip proxy/click-URL conversion and creative TSJS injection. -# Sanitization is controlled separately by `sanitize_creatives` below. Restore -# true before rolling back to an older binary that rejects unknown fields. +# Rewrite winning-bid creative asset URLs (images, scripts, styles, media, +# iframes, CSS url()) to first-party /first-party/proxy endpoints (default +# true). Set false to leave asset URLs direct. Bidder removal and +# creative TSJS injection run when this or rewrite_clicks is on. Sanitization +# is controlled separately by `sanitize_creatives` below. Restore true before +# rolling back to an older binary that rejects unknown fields. rewrite_creatives = true +# Wrap creative click-through links (, ) in signed +# /first-party/click redirects. Unset (the default) follows rewrite_creatives +# for auction creatives and keeps wrapping links in HTML fetched through +# /first-party/proxy; a set value applies to both. Leave it commented out to +# keep that behavior: any explicit value is stored in the pushed config, and +# binaries older than this setting reject it. Uncomment it before relying on +# TRUSTED_SERVER__AUCTION__REWRITE_CLICKS, which cannot create a missing leaf. +# rewrite_clicks = true # Strip executable markup (script/object/embed/form/...) from winning-bid adm, # removing those elements together with their inner content. Defaults to false -# (executable markup preserved). Note that with `rewrite_creatives = true` the -# adm is still not untouched: eligible URLs are rewritten, bidder `` -# elements are removed, and the creative TSJS runtime is injected. Enable it -# whenever creatives can render in a context that shares the publisher origin -# (its primary defence there); leave it off when creatives render in a +# (executable markup preserved). Note that with `rewrite_creatives` or +# `rewrite_clicks` on, the adm is still not untouched: eligible asset URLs +# and/or links are rewritten, bidder `` elements are removed, and the +# creative TSJS runtime is injected. Enable it whenever creatives can render +# in a context that shares the publisher origin (its primary defence there); +# leave it off when creatives render in a # foreign-origin frame (e.g. the Prebid Universal Creative inside the ad # server's iframe), since it removes script-based creatives entirely and would # blank slots on a script-heavy demand stack. From ee5715f11049ce26257f10e51ab4c88b1fba07c3 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 15:04:44 +0530 Subject: [PATCH 07/20] Document the rewrite_clicks creative setting Signed-off-by: dhruv8sh --- CHANGELOG.md | 3 +- .../trusted-server-core/src/auction/README.md | 6 +- docs/guide/api-reference.md | 5 +- docs/guide/auction-orchestration.md | 11 ++-- docs/guide/configuration.md | 57 +++++++++------- docs/guide/creative-processing.md | 66 +++++++++++++++---- 6 files changed, 103 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c370373ff..ce3904fea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,7 +41,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Added the `[auction].rewrite_creatives` (default `true`) and `[auction].sanitize_creatives` (default `false`) options. `rewrite_creatives` rewrites winning-bid adm to first-party endpoints across `POST /auction` and publisher SSAT/page-bids delivery (proxy/click URL conversion, bidder `` removal; creative TSJS injection on `POST /auction` only). Enabling `sanitize_creatives` strips executable markup from winning-bid adm before delivery. +- Added `[auction].rewrite_clicks` to control creative click-through wrapping (``/`` → signed `/first-party/click`) independently of asset rewriting. Unset (the default) follows `rewrite_creatives` for `POST /auction` and SSAT/page-bids and keeps wrapping links in HTML fetched through `/first-party/proxy`, so existing configs behave as before; an explicit value applies to every path. `rewrite_creatives` now governs asset URLs only; bidder `` removal and creative TSJS injection run when either setting is on. Upgrading: deploy the binary first, then push a config that sets `rewrite_clicks`. Rolling back: remove any explicit `rewrite_clicks` (and its environment override), push the resulting config, then roll back the binary. Older binaries tie click wrapping to `rewrite_creatives` in both directions, so rolling back turns clicks back on for `rewrite_creatives = true` with `rewrite_clicks = false`, and off for `rewrite_creatives = false` with `rewrite_clicks = true`. +- Added the `[auction].rewrite_creatives` (default `true`) and `[auction].sanitize_creatives` (default `false`) options. `rewrite_creatives` rewrites winning-bid adm to first-party endpoints across `POST /auction` and publisher SSAT/page-bids delivery (asset URL conversion to `/first-party/proxy`, bidder `` removal; creative TSJS injection on `POST /auction` only). Click-through wrapping is controlled by `[auction].rewrite_clicks`, which follows `rewrite_creatives` when unset. Enabling `sanitize_creatives` strips executable markup from winning-bid adm before delivery. - `creative_opportunities.slot.gam_unit_path` is now a template supporting `{network_id}`, `{slot_id}`, and `{section}`, so a publisher whose ad unit varies by site section expresses it in one slot rule instead of one per (slot × section). `{section}` derives from the request path: `[creative_opportunities].section_segment` selects which path segment names the section (0-based, default `0`; set `1` for locale-prefixed URLs), and `section_root` supplies the value for paths with no such segment. `section_root` is required when a template uses `{section}`. Existing static and absent `gam_unit_path` configs are unchanged. Startup rejects a blank `gam_network_id` only when an absent/default path or `{network_id}` template consumes it. Trusted Server conservatively caps whole rendered dynamic paths at 100 UTF-8 bytes, informed by Google's 100-character per-ad-unit-code limit; an over-limit request-specific path omits that slot without failing the response. During typed/startup finalization, every placeholder-bearing template that omits `section_segment` materializes `section_segment = 0`, so an older binary rejects the blob loudly. Static and absent paths remain legacy-schema compatible only when both `section_root` and `section_segment` are omitted. Before rolling back below this feature, replace or remove dynamic paths, remove both keys, re-push and finalize the config, then roll back the binary. - Added opt-in APS HTTP debug metadata for controlled test sites, exposing the direct request and response under `/auction` provider metadata using the Prebid Server `debug.httpcalls` shape. - Added typed APS renderer transport for direct auctions and GAM/Prebid Universal Creative, using a minimized one-bid envelope, a fragment-bound nonce, and an opaque sandboxed renderer endpoint. diff --git a/crates/trusted-server-core/src/auction/README.md b/crates/trusted-server-core/src/auction/README.md index 8a745eabc..7b0ae585b 100644 --- a/crates/trusted-server-core/src/auction/README.md +++ b/crates/trusted-server-core/src/auction/README.md @@ -108,8 +108,10 @@ as `pbs-main`. response. - `sanitize_creatives = true` strips executable markup. It is opt-in. -- `rewrite_creatives = true` rewrites eligible URLs through first-party routes - and removes bidder `` elements. It is enabled by default. +- `rewrite_creatives = true` rewrites eligible asset URLs through first-party + routes and removes bidder `` elements. It is enabled by default. +- `rewrite_clicks` wraps creative links in `/first-party/click`. Unset, it + follows `rewrite_creatives`. - The publisher inline delivery path uses absolute first-party URLs without injecting the direct endpoint's creative runtime. - Creatives over the configured hard cap are rejected. diff --git a/docs/guide/api-reference.md b/docs/guide/api-reference.md index e444ec147..bc95ff9ac 100644 --- a/docs/guide/api-reference.md +++ b/docs/guide/api-reference.md @@ -168,8 +168,9 @@ curl -i "https://edge.example.com/_ts/clear-tester" Browser and programmatic auction endpoint. It accepts the Trusted Server ad-unit request shape and returns an OpenRTB response. Creative markup follows the -independent `[auction].sanitize_creatives` and `[auction].rewrite_creatives` -settings; sanitization is opt-in and rewriting is enabled by default. +independent `[auction].sanitize_creatives`, `[auction].rewrite_creatives` and +`[auction].rewrite_clicks` settings; sanitization is opt-in, asset rewriting is +enabled by default, and click wrapping follows `rewrite_creatives` unless set. Configured provider IDs appear in response metadata and provider responses. Consumers that previously matched the literal provider name `prebid` must use diff --git a/docs/guide/auction-orchestration.md b/docs/guide/auction-orchestration.md index edbc38471..46347c80e 100644 --- a/docs/guide/auction-orchestration.md +++ b/docs/guide/auction-orchestration.md @@ -161,7 +161,7 @@ sequenceDiagram Note right of Client: Fragment-bound nonce and one-time acknowledgement
No allow-same-origin on the outer frame else Ordinary creative Client->>Client: Inject winning creative
Render iframe
Load creative resources - Note right of Client: Default: first-party proxy/click URLs
rewrite_creatives=false: accepted external URLs remain direct + Note right of Client: Default: first-party proxy/click URLs
rewrite_creatives=false: asset URLs direct; clicks follow rewrite_clicks end deactivate Client end @@ -195,7 +195,7 @@ AuctionOrchestrator.run_auction() Convert OrchestrationResult → OpenRTB 2.x Response │ ├─[sanitize_creatives=true] Strip executable markup - ├─[rewrite_creatives=true] Rewrite URLs and inject creative TSJS + ├─[rewrite_creatives or rewrite_clicks] Rewrite assets and/or links, inject creative TSJS ├─ Add ext.orchestrator metadata └─ Set consent and optional EID response headers ``` @@ -638,8 +638,11 @@ HTML rewriter (`lol_html`) that converts eligible external resource and click URLs to signed first-party paths, adds `data-tsclick`, rewrites inline CSS `url(...)` values, removes bidder-supplied `` elements, and injects the unified creative TSJS runtime exactly once, whether or not the bidder supplied a -`` element. In every mode, a creative -larger than the 1 MiB per-creative cap is rejected and its `adm` is dropped. +`` element. `rewrite_clicks` (unset by default, following +`rewrite_creatives`) controls click-through link wrapping separately; see +[Creative Processing](/guide/creative-processing#assets-and-clicks). In every +mode, a creative larger than the 1 MiB per-creative cap is rejected and its +`adm` is dropped. ```toml [auction] diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index f7220d58c..908239d2b 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -1927,15 +1927,16 @@ provider or bidder route. ### `[auction]` -| Field | Type | Default | Description | -| ---------------------- | ------- | ------------------ | -------------------------------------------------------------- | -| `enabled` | Boolean | `false` | Enable the auction orchestrator | -| `sanitize_creatives` | Boolean | `false` | Strip executable markup from winning-bid `adm` before delivery | -| `rewrite_creatives` | Boolean | `true` | Rewrite winning-bid `adm` through first-party endpoints | -| `timeout_ms` | Integer | `2000` | Logical auction budget in milliseconds | -| `mediator` | String | `None` | Optional separate `adserver_mock` mediator | -| `creative_store` | String | `"creative_store"` | Deprecated; creatives are delivered inline | -| `allowed_context_keys` | Array | `[]` | Request context keys admitted into the auction | +| Field | Type | Default | Description | +| ---------------------- | ------- | ------------------ | ------------------------------------------------------------------------------ | +| `enabled` | Boolean | `false` | Enable the auction orchestrator | +| `sanitize_creatives` | Boolean | `false` | Strip executable markup from winning-bid `adm` before delivery | +| `rewrite_creatives` | Boolean | `true` | Rewrite winning-bid asset URLs through first-party endpoints | +| `rewrite_clicks` | Boolean | unset | Wrap creative links in `/first-party/click`; unset follows `rewrite_creatives` | +| `timeout_ms` | Integer | `2000` | Logical auction budget in milliseconds | +| `mediator` | String | `None` | Optional separate `adserver_mock` mediator | +| `creative_store` | String | `"creative_store"` | Deprecated; creatives are delivered inline | +| `allowed_context_keys` | Array | `[]` | Request context keys admitted into the auction | Creative markup delivered by `POST /auction` and the publisher SSAT/page-bids path is processed by two independent passes. With `sanitize_creatives = true` @@ -1943,23 +1944,29 @@ path is processed by two independent passes. With `sanitize_creatives = true` and event handlers) is stripped together with its inner content. This blanks script-based creatives, so enable it only when creatives render in a context that shares the publisher's origin. With `rewrite_creatives = true` (the -default), eligible absolute or protocol-relative resource and click URLs not -excluded by rewrite configuration are converted to signed first-party -endpoints, and any bidder-supplied `` element is removed. The -`POST /auction` path emits root-relative endpoints and injects the creative TSJS -runtime exactly once, whether or not the bidder supplied a ``, since bare +default), eligible absolute or protocol-relative asset URLs not excluded by +rewrite configuration are converted to signed `/first-party/proxy` endpoints. +`rewrite_clicks` controls click-through links (``, ``) +separately: when it is unset, it follows `rewrite_creatives`, so existing +configs keep their current behavior. Any bidder-supplied `` element is +removed whenever either setting is on. The `POST /auction` path emits +root-relative endpoints and, when either setting is on, injects the creative +TSJS runtime exactly once, whether or not the bidder supplied a ``, since bare fragments are the common `adm` shape. The foreign-origin SSAT renderer emits -absolute endpoints and does not inject that bundle. With both disabled, `adm` -ships exactly as the bidder returned it, except that a creative larger than the +absolute endpoints and does not inject that bundle. With all three disabled, +`adm` ships exactly as the bidder returned it, except that a creative larger than the 1 MiB per-creative cap is rejected in every mode and its `adm` is dropped. -Accepted external URLs are not host allowlisted by the sanitizer. Neither -setting affects HTML or CSS fetched through `/first-party/proxy`. See +Accepted external URLs are not host allowlisted by the sanitizer. +`rewrite_creatives` and `sanitize_creatives` do not affect HTML or CSS fetched +through `/first-party/proxy`. An explicitly set `rewrite_clicks` does apply to +links in proxied HTML; unset, proxied HTML keeps wrapping links. See [Creative Processing](/guide/creative-processing#auction-rewrite-control). ::: warning Existing configs, upgrade sequencing, and rollback Default values are omitted from stored JSON; non-default values -(`sanitize_creatives = true`, `rewrite_creatives = false`) are serialized, and -older `AuctionConfig` schemas reject unknown fields. +(`sanitize_creatives = true`, `rewrite_creatives = false`, and any explicit +`rewrite_clicks`) are serialized, and older `AuctionConfig` schemas reject +unknown fields. **Upgrading:** binaries that predate `sanitize_creatives` reject a blob that carries it, so in a rolling deployment upgrade the binary **first**, then push @@ -1973,14 +1980,20 @@ on new code, while an explicit `true` fails startup on old code. **Rolling back:** before reverting to a binary that does not know a field, remove that field's non-default value (and any environment override), run `ts config validate`, push the resulting default-compatible blob, and only then -roll back the binary. +roll back the binary. Older binaries tie click wrapping to `rewrite_creatives` +in both directions: rolling back from `rewrite_creatives = true` with +`rewrite_clicks = false` turns click wrapping back on, and rolling back from +`rewrite_creatives = false` with `rewrite_clicks = true` turns it off. **Environment overlays:** The pinned EdgeZero loader cannot create missing TOML leaves. Existing configs must add **both** leaves under `[auction]` (`rewrite_creatives` and `sanitize_creatives`) before `TRUSTED_SERVER__AUCTION__REWRITE_CREATIVES` / `TRUSTED_SERVER__AUCTION__SANITIZE_CREATIVES` can take effect. An override for a -missing leaf is silently ignored. +missing leaf is silently ignored. `rewrite_clicks` is unset by default and so +cannot appear as a TOML leaf until you set it; add `rewrite_clicks = true` or +`false` under `[auction]` before relying on +`TRUSTED_SERVER__AUCTION__REWRITE_CLICKS`. ::: ### Provider map diff --git a/docs/guide/creative-processing.md b/docs/guide/creative-processing.md index 10dcb62d5..823ae1260 100644 --- a/docs/guide/creative-processing.md +++ b/docs/guide/creative-processing.md @@ -47,8 +47,8 @@ publisher's first-party domain. This provides: The creative rewriters are invoked by independent delivery paths: -1. **Auction `adm`**: Winning-bid HTML returned by `POST /auction` is optionally sanitized (`[auction].sanitize_creatives`, opt-in, default `false`) and rewritten (`[auction].rewrite_creatives`, default `true`). -2. **First-party proxy**: Non-streaming `text/html` and `text/css` responses fetched through `/first-party/proxy` are rewritten independently of the auction setting. +1. **Auction `adm`**: Winning-bid HTML returned by `POST /auction` is optionally sanitized (`[auction].sanitize_creatives`, opt-in, default `false`) and rewritten (`[auction].rewrite_creatives`, default `true`). Click-through links are wrapped according to `[auction].rewrite_clicks`, which follows `rewrite_creatives` when unset. +2. **First-party proxy**: Non-streaming `text/html` and `text/css` responses fetched through `/first-party/proxy` are rewritten independently of the auction setting. An explicitly set `[auction].rewrite_clicks` also applies to links in proxied HTML. 3. **Integration processing**: Publisher HTML integrations use their own registration and configuration controls. ::: info Streaming Mode @@ -57,26 +57,28 @@ When `with_streaming()` is enabled in `ProxyRequestConfig`, proxied HTML/CSS pro ## Auction Rewrite Control -Two independent auction settings control the processing applied to winning-bid -`adm` returned by `POST /auction` and delivered through the publisher -SSAT/page-bids path. Sanitization is opt-in (default `false`); rewriting is -enabled by default: +Three auction settings control the processing applied to winning-bid `adm` +returned by `POST /auction` and delivered through the publisher SSAT/page-bids +path. Sanitization is opt-in (default `false`) and asset rewriting is enabled +by default. Click wrapping is controlled by `rewrite_clicks`, which follows +`rewrite_creatives` when unset; see [Assets and clicks](#assets-and-clicks). ```toml [auction] sanitize_creatives = false rewrite_creatives = true +# rewrite_clicks unset: follows rewrite_creatives ``` Regardless of mode, a creative larger than the 1 MiB per-creative cap is rejected and its `adm` is dropped. -| `sanitize_creatives` | `rewrite_creatives` | Auction winning-bid `adm` behavior | -| -------------------- | ------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `false` (default) | `false` | Deliver the creative exactly as the bidder returned it (subject to the size cap). | -| `true` | `false` | Strip executable markup (`script`/`object`/`embed`/`form`, event handlers) with its inner content, then deliver without rewriting. Sanitizer-accepted external resource, click, and inline CSS URLs remain direct. | -| `false` | `true` (default) | Rewrite eligible resource/CSS and click URLs in the raw bidder markup to signed first-party endpoints, removing any bidder `` element. Executable markup is preserved. | -| `true` | `true` | Sanitize first, then rewrite. `POST /auction` emits root-relative endpoints and injects creative TSJS exactly once, whether or not the bidder supplied a ``; SSAT/page-bids emits absolute endpoints for its foreign-origin renderer and does not inject the bundle. | +| `sanitize_creatives` | `rewrite_creatives` | Auction winning-bid `adm` behavior | +| -------------------- | ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `false` (default) | `false` | Asset URLs stay direct. With `rewrite_clicks` unset or `false`, deliver the creative exactly as the bidder returned it (subject to the size cap); with `rewrite_clicks = true`, wrap links as described in [Assets and clicks](#assets-and-clicks). | +| `true` | `false` | Strip executable markup (`script`/`object`/`embed`/`form`, event handlers) with its inner content, then deliver without asset rewriting. Sanitizer-accepted external resource and inline CSS URLs remain direct; links follow `rewrite_clicks` and stay direct when it is unset or `false`. | +| `false` | `true` (default) | Rewrite eligible resource/CSS URLs in the raw bidder markup to signed first-party endpoints, removing any bidder `` element. Links are wrapped unless `rewrite_clicks = false`. Executable markup is preserved. | +| `true` | `true` | Sanitize first, then rewrite. `POST /auction` emits root-relative endpoints and injects creative TSJS exactly once, whether or not the bidder supplied a ``; SSAT/page-bids emits absolute endpoints for its foreign-origin renderer and does not inject the bundle. | ::: warning Sanitization blanks script-based creatives Sanitization removes `script`/`object`/`embed`/`form` and similar elements @@ -90,6 +92,39 @@ accepted HTTP(S) URLs may cause the browser to contact external creative hosts directly. ::: +### Assets and clicks + +`rewrite_creatives` governs asset URLs. `rewrite_clicks` governs click-through +links and follows `rewrite_creatives` when unset: + +```toml +[auction] +rewrite_creatives = true +# Unset: follows rewrite_creatives. Set false to keep landing links direct. +# rewrite_clicks = false +``` + +| Assets (`rewrite_creatives`) | Clicks (`rewrite_clicks`) | Asset URLs | Links | `` | TSJS on `POST /auction` | +| ---------------------------- | ------------------------- | -------------------- | ------------------------------------- | -------- | ----------------------- | +| `true` | `true` or unset | `/first-party/proxy` | `/first-party/click` + `data-tsclick` | removed | injected | +| `true` | `false` | `/first-party/proxy` | direct | removed | injected | +| `false` | `true` | direct | `/first-party/click` + `data-tsclick` | removed | injected | +| `false` | `false` or unset | direct | direct | kept | not injected | + +SSAT/page-bids follows the same table with absolute URLs and never injects TSJS. +`exclude_domains` applies to both assets and links. + +The client-side `tsCreativeConfig.clickGuard` flag is separate. `rewrite_clicks` +decides whether the server emits signed click URLs; `clickGuard` decides whether +the creative runtime re-signs them after creative script changes their query +parameters. With `rewrite_clicks` off there is nothing for the guard to act on. + +With `rewrite_creatives = false` and `rewrite_clicks = true`, `POST /auction` +still injects TSJS. A creative that turns on `tsCreativeConfig.renderGuard` can +therefore still send assets inserted by its own script through +`/first-party/sign` and `/first-party/proxy`, even though the server left the +markup's asset URLs direct. + ::: info Runtime protections inside the sandboxed creative iframe Creatives rendered by Trusted Server's own path run in a sandboxed iframe **without** `allow-same-origin`, i.e. an opaque origin. The injected creative @@ -307,15 +342,18 @@ If the iframe content itself contains HTML, it will be processed recursively. Ea **Rewrite Mode**: Uses `/first-party/click` for direct redirects +**Controlled by**: `[auction].rewrite_clicks` (follows `rewrite_creatives` when unset) + **Example**: ```html -Buy Now +Buy Now Buy Now ``` From 48d59b8daa6ea703420c15246dc6553936c59ab1 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 15:08:47 +0530 Subject: [PATCH 08/20] Add rewrite include_domains setting with validation 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 --- .../trusted-server-core/src/config_payload.rs | 37 ++++ crates/trusted-server-core/src/settings.rs | 206 +++++++++++++++++- 2 files changed, 241 insertions(+), 2 deletions(-) diff --git a/crates/trusted-server-core/src/config_payload.rs b/crates/trusted-server-core/src/config_payload.rs index eee5d7ee8..f818571c6 100644 --- a/crates/trusted-server-core/src/config_payload.rs +++ b/crates/trusted-server-core/src/config_payload.rs @@ -735,6 +735,43 @@ mod tests { ); } + #[test] + fn legacy_blob_without_rewrite_include_domains_loads_with_empty_list() { + let data = + serde_json::to_value(test_settings()).expect("should serialize settings to JSON"); + let rewrite = data + .get("rewrite") + .and_then(serde_json::Value::as_object) + .expect("should serialize rewrite settings as an object"); + assert!( + !rewrite.contains_key("include_domains"), + "should omit the default include list from the payload" + ); + + let reconstructed = load_settings(&envelope_json(&test_settings())) + .expect("should reconstruct settings without include_domains"); + + assert!( + reconstructed.rewrite.include_domains.is_empty(), + "should load an empty include list from a blob without the key" + ); + } + + #[test] + fn rewrite_include_domains_survive_blob_round_trip() { + let mut original = test_settings(); + original.rewrite.include_domains = vec!["*.cdn.example.com".to_owned()]; + + let reconstructed = load_settings(&envelope_json(&original)) + .expect("should reconstruct settings with include_domains"); + + assert_eq!( + reconstructed.rewrite.include_domains, + vec!["*.cdn.example.com".to_owned()], + "should preserve the configured include list" + ); + } + #[test] fn disabled_rewrite_creatives_survives_blob_round_trip() { let mut original = test_settings(); diff --git a/crates/trusted-server-core/src/settings.rs b/crates/trusted-server-core/src/settings.rs index 0d3cad1c9..c4d0321d2 100644 --- a/crates/trusted-server-core/src/settings.rs +++ b/crates/trusted-server-core/src/settings.rs @@ -637,10 +637,23 @@ impl Ec { #[derive(Debug, Default, Clone, Deserialize, Serialize, Validate)] #[serde(deny_unknown_fields)] pub struct Rewrite { - /// List of domains to exclude from rewriting. Supports wildcards (e.g., "*.example.com"). - /// URLs from these domains will not be proxied through first-party endpoints. + /// Hosts never rewritten, for assets and click-through links. + /// + /// Each entry is an exact host or `*.example.com`, which matches the apex + /// and any subdomain. Matching is case-insensitive. Wins over + /// [`Self::include_domains`]. #[serde(default)] pub exclude_domains: Vec, + /// When non-empty, the only hosts whose asset URLs are rewritten to + /// `/first-party/proxy`. + /// + /// Same pattern syntax as [`Self::exclude_domains`]. Empty (the default) + /// proxies every eligible asset URL. Never applies to click-through links. + /// The default is omitted from serialized config so older binaries, which + /// reject unknown fields, can still read it. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[validate(custom(function = validate_include_domains))] + pub include_domains: Vec, } impl Rewrite { @@ -686,6 +699,11 @@ impl Rewrite { "rewrite.exclude_domains: removed empty or bare \"*\" entries, which never match a host" ); } + self.include_domains = self + .include_domains + .iter() + .map(|pattern| pattern.trim().to_ascii_lowercase()) + .collect(); } } @@ -3401,6 +3419,89 @@ impl Settings { } } +/// Rejects `rewrite.include_domains` entries that can never equal a URL host. +/// +/// Runs after [`Rewrite::normalize`], so entries are already trimmed and +/// lowercased. Entries are matched against [`Url::host_str`], so each one must +/// be: +/// +/// - a DNS name, optionally prefixed with `*.`: one or more `.`-separated, +/// non-empty labels of ASCII `a-z`, `0-9` and `-`; +/// - an IPv4 address in dotted-quad form, such as `192.0.2.1`; or +/// - an IPv6 address in brackets and compressed form, such as `[2001:db8::1]`. +/// +/// A scheme, port, path, space, leading or trailing `.`, empty label, wildcard +/// IP address or unbracketed IPv6 address is rejected with +/// `invalid_rewrite_include_domain`. A non-ASCII (IDN) name is rejected with +/// `non_ascii_rewrite_include_domain`; write it in punycode (`xn--`) form. +fn validate_include_domains(patterns: &[String]) -> Result<(), ValidationError> { + for pattern in patterns { + if !pattern.is_ascii() { + let mut err = ValidationError::new("non_ascii_rewrite_include_domain"); + err.add_param("value".into(), pattern); + err.message = Some( + "rewrite.include_domains entries must be 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("value".into(), pattern); + err.add_param("reason".into(), &reason); + err.message = Some( + "rewrite.include_domains entries must be a host name, an optional leading `*.`, a dotted-quad IPv4 address or a bracketed IPv6 address" + .into(), + ); + return Err(err); + } + } + Ok(()) +} + +/// Checks one normalized ASCII `include_domains` entry, returning why it can +/// never equal a URL host. +fn check_include_domain(pattern: &str) -> Result<(), &'static str> { + if let Some(bracketed) = pattern.strip_prefix('[') { + let address = bracketed + .strip_suffix(']') + .and_then(|inner| inner.parse::().ok()) + .ok_or("not a bracketed IPv6 address")?; + if format!("[{address}]") != pattern { + return Err("IPv6 address not in compressed form"); + } + return Ok(()); + } + if pattern.parse::().is_ok() { + return Err("IPv6 address without brackets"); + } + + let name = pattern.strip_prefix("*.").unwrap_or(pattern); + if name.is_empty() { + return Err("no host"); + } + if !name.bytes().all(|byte| { + byte.is_ascii_lowercase() || byte.is_ascii_digit() || matches!(byte, b'-' | b'.') + }) { + return Err("character outside a-z, 0-9, `-` and `.`"); + } + if name.split('.').any(str::is_empty) { + return Err("leading or trailing `.` or empty label"); + } + + // A URL host whose last label is numeric is parsed as an IPv4 address, so + // such an entry only matches as an exact dotted-quad address. + let last_label = name.rsplit('.').next().unwrap_or(name); + let numeric_last_label = + last_label.bytes().all(|byte| byte.is_ascii_digit()) || last_label.starts_with("0x"); + if numeric_last_label + && (name.len() != pattern.len() || name.parse::().is_err()) + { + return Err("numeric host that is not a dotted-quad IPv4 address"); + } + Ok(()) +} + fn validate_publisher_domain(value: &str) -> Result<(), ValidationError> { if value.trim() != value || value.is_empty() || value.len() > 253 { return Err(ValidationError::new("invalid_publisher_domain")); @@ -6617,6 +6718,107 @@ source_domain = "partner.example.com" ); } + #[test] + fn rewrite_include_domains_are_normalized_from_toml() { + let toml_str = crate_test_settings_str() + + r#" + [rewrite] + include_domains = ["*.CDN.Example.com", " img.example.net "] + "#; + + let settings = Settings::from_toml(&toml_str).expect("should parse valid TOML"); + + assert_eq!( + settings.rewrite.include_domains, + vec!["*.cdn.example.com".to_owned(), "img.example.net".to_owned()], + "should trim and lowercase include_domains entries" + ); + } + + fn include_domains_toml(entry: &str) -> String { + crate_test_settings_str() + &format!("\n[rewrite]\ninclude_domains = [\"{entry}\"]\n") + } + + #[test] + fn rewrite_include_domains_reject_malformed_entries() { + for entry in [ + "", + " ", + "*", + "*.", + "cdn.*.example.com", + "**.example.com", + "https://cdn.example.com", + "cdn.example.com:443", + "cdn.example.com/x", + ".example.com", + "example.com.", + "cdn..example.com", + "cdn example.com", + "cdn_assets.example.com", + "::1", + "[::1", + "[2001:0db8::1]", + "*.192.0.2.1", + "192.0.2", + "256.0.2.1", + ] { + let result = Settings::from_toml(&include_domains_toml(entry)); + + let err = result.expect_err(&format!("should reject include_domains entry `{entry}`")); + assert!( + format!("{err:?}").contains("invalid_rewrite_include_domain"), + "should report `{entry}` as an invalid include_domains entry: {err:?}" + ); + } + } + + #[test] + fn rewrite_include_domains_reject_non_ascii_entries_with_a_punycode_hint() { + for entry in ["bücher.example", "*.bücher.example"] { + let result = Settings::from_toml(&include_domains_toml(entry)); + + let err = result.expect_err(&format!("should reject non-ASCII entry `{entry}`")); + assert!( + format!("{err:?}").contains("non_ascii_rewrite_include_domain"), + "should report `{entry}` as non-ASCII: {err:?}" + ); + } + } + + #[test] + fn rewrite_include_domains_accept_hosts_and_ip_literals() { + for entry in [ + "cdn.example.com", + "*.example.com", + "xn--bcher-kva.example", + "192.0.2.1", + "[2001:db8::1]", + "[::1]", + ] { + let settings = + Settings::from_toml(&include_domains_toml(entry)).unwrap_or_else(|err| { + panic!("should accept include_domains entry `{entry}`: {err:?}") + }); + + assert_eq!( + settings.rewrite.include_domains, + vec![entry.to_owned()], + "should keep accepted entry `{entry}` as written" + ); + } + } + + #[test] + fn rewrite_include_domains_default_to_empty() { + let settings = create_test_settings(); + + assert!( + settings.rewrite.include_domains.is_empty(), + "should default include_domains to an empty list" + ); + } + #[test] fn test_auction_creative_processing_defaults_when_omitted() { let toml_str = crate_test_settings_str() From ef8ad67f9a4e6161c9fcfd950d0a44e4e166bb1d Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 15:09:34 +0530 Subject: [PATCH 09/20] Limit creative asset rewriting to rewrite include_domains 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 --- crates/trusted-server-core/src/creative.rs | 215 ++++++++++++++++++++- crates/trusted-server-core/src/proxy.rs | 119 +++++++++++- crates/trusted-server-core/src/settings.rs | 44 ++++- 3 files changed, 371 insertions(+), 7 deletions(-) diff --git a/crates/trusted-server-core/src/creative.rs b/crates/trusted-server-core/src/creative.rs index 074e5f35d..14f3bbd5a 100644 --- a/crates/trusted-server-core/src/creative.rs +++ b/crates/trusted-server-core/src/creative.rs @@ -24,6 +24,8 @@ //! click guard. //! - Hosts matching `[rewrite] exclude_domains` are left untouched, for assets //! and links alike. +//! - When `[rewrite] include_domains` is non-empty, only asset URLs on listed +//! hosts are proxied. Links ignore it. //! - Bidder-supplied `` elements are removed so root-relative first-party //! URLs cannot be rebased. //! - Relative URLs (e.g., `/path`, `../path`, `local/file`) remain unchanged. @@ -1614,8 +1616,9 @@ impl StreamProcessor for CreativeCssProcessor<'_> { mod tests { use super::{ CreativeCssProcessor, StreamProcessor as _, normalize_creative_url, - process_auction_creative, proxy_if_abs, rewrite_creative_html, - rewrite_inline_creative_html, rewrite_srcset, rewrite_style_urls, sanitize_creative_html, + process_auction_creative, proxy_if_abs, rewrite_creative_html, rewrite_css_body, + rewrite_inline_creative_html, rewrite_proxied_html, rewrite_srcset, rewrite_style_urls, + sanitize_creative_html, }; use crate::settings::Settings; @@ -3795,6 +3798,214 @@ b{background:url(\"https://cdn.example/c.png\")}"; ); } + const ALLOWLIST_LISTED: &[&str] = &[ + "https://assets.example.com/a.css", + "https://assets.example.com/p1.png", + "https://assets.example.com/p1-2x.png", + "https://assets.example.com/a.js", + "https://assets.example.com/i.css", + "https://assets.example.com/bg.png", + "https://assets.example.com/set.png", + "https://assets.example.com/img.png", + "https://assets.example.com/lazy.png", + "https://assets.example.com/s1.png", + "https://assets.example.com/v.mp4", + "https://assets.example.com/o.swf", + "https://assets.example.com/e.swf", + "https://assets.example.com/btn.png", + "https://assets.example.com/icon.svg", + "https://assets.example.com/sprite.svg", + "https://assets.example.com/frame.html", + "https://assets.example.com/inline.png", + ]; + + const ALLOWLIST_OFF_LIST: &[&str] = &[ + "https://cdn.example.net/a.css", + "https://cdn.example.net/p2.png", + "https://cdn.example.net/a.js", + "https://cdn.example.net/i.css", + "https://cdn.example.net/bg.png", + "https://cdn.example.net/set.png", + "https://cdn.example.net/img.png", + "https://cdn.example.net/lazy.png", + "https://cdn.example.net/s2.png", + "https://cdn.example.net/v.mp4", + "https://cdn.example.net/o.swf", + "https://cdn.example.net/e.swf", + "https://cdn.example.net/btn.png", + "https://cdn.example.net/icon.svg", + "https://cdn.example.net/sprite.svg", + "https://cdn.example.net/frame.html", + "https://cdn.example.net/inline.png", + ]; + + const ALLOWLIST_CREATIVE: &str = r#" + + + + + + + + + + + + + + + + + + + + + +
+
+Offer +Area +"#; + + fn encoded_tsurl(url: &str) -> String { + format!( + "tsurl={}", + url::form_urlencoded::byte_serialize(url.as_bytes()).collect::() + ) + } + + fn allowlist_settings() -> crate::settings::Settings { + let mut settings = crate::test_support::tests::create_test_settings(); + settings.rewrite.include_domains = vec!["*.example.com".to_owned()]; + settings + } + + fn assert_allowlist_applied(label: &str, out: &str) { + for listed in ALLOWLIST_LISTED { + assert!( + out.contains(&encoded_tsurl(listed)), + "{label}: should proxy listed URL `{listed}`: {out}" + ); + } + for off_list in ALLOWLIST_OFF_LIST { + assert!( + !out.contains(&encoded_tsurl(off_list)), + "{label}: should not proxy off-list URL `{off_list}`: {out}" + ); + assert!( + out.contains(off_list), + "{label}: should keep off-list URL `{off_list}` raw: {out}" + ); + } + for landing in [ + "https://landing.example.net/offer", + "https://landing.example.org/area", + ] { + assert!( + out.contains(&format!("/first-party/click?{}", encoded_tsurl(landing))), + "{label}: should wrap click `{landing}` regardless of include_domains: {out}" + ); + } + assert_eq!( + out.matches("data-tsclick=").count(), + 2, + "{label}: should mark both click-through links" + ); + } + + #[test] + fn include_domains_limit_asset_rewriting_on_every_html_path() { + let settings = allowlist_settings(); + + assert_allowlist_applied( + "auction", + &rewrite_creative_html(&settings, ALLOWLIST_CREATIVE), + ); + assert_allowlist_applied( + "inline", + &rewrite_inline_creative_html(&settings, "https://www.example.com", ALLOWLIST_CREATIVE), + ); + assert_allowlist_applied( + "proxied", + &rewrite_proxied_html(&settings, ALLOWLIST_CREATIVE), + ); + } + + #[test] + fn include_domains_limit_css_body_rewriting() { + let settings = allowlist_settings(); + let css = r#"@import "https://assets.example.com/i.css"; +@import "https://cdn.example.net/i.css"; +.a { background: url(https://assets.example.com/bg.png); } +.b { background: url(https://cdn.example.net/bg.png); }"#; + + let out = rewrite_css_body(&settings, css).expect("should rewrite CSS body"); + + for listed in [ + "https://assets.example.com/i.css", + "https://assets.example.com/bg.png", + ] { + assert!( + out.contains(&encoded_tsurl(listed)), + "should proxy listed CSS URL `{listed}`: {out}" + ); + } + for off_list in [ + "https://cdn.example.net/i.css", + "https://cdn.example.net/bg.png", + ] { + assert!( + out.contains(off_list) && !out.contains(&encoded_tsurl(off_list)), + "should keep off-list CSS URL `{off_list}` raw: {out}" + ); + } + } + + #[test] + fn host_in_both_lists_is_left_alone() { + let mut settings = allowlist_settings(); + settings.rewrite.exclude_domains = vec!["assets.example.com".to_owned()]; + let html = ALLOWLIST_CREATIVE.replace( + "", + r#"Excluded"#, + ); + + for (label, out) in [ + ("auction", rewrite_creative_html(&settings, &html)), + ( + "inline", + rewrite_inline_creative_html(&settings, "https://www.example.com", &html), + ), + ("proxied", rewrite_proxied_html(&settings, &html)), + ] { + assert!( + !out.contains("tsurl=https%3A%2F%2Fassets.example.com"), + "{label}: should not rewrite any URL on a host in both lists: {out}" + ); + for listed in ALLOWLIST_LISTED { + assert!( + out.contains(listed), + "{label}: should keep `{listed}` raw when its host is also excluded: {out}" + ); + } + assert!( + out.contains(r#"Excluded"#), + "{label}: should leave an excluded click-through link raw: {out}" + ); + assert_eq!( + out.matches("data-tsclick=").count(), + 2, + "{label}: should wrap only the two links on hosts that are not excluded: {out}" + ); + } + } + #[test] fn rewrite_html_excludes_blacklisted_domains() { let mut settings = crate::test_support::tests::create_test_settings(); diff --git a/crates/trusted-server-core/src/proxy.rs b/crates/trusted-server-core/src/proxy.rs index 1b9139753..7f6f1364e 100644 --- a/crates/trusted-server-core/src/proxy.rs +++ b/crates/trusted-server-core/src/proxy.rs @@ -1225,7 +1225,7 @@ fn append_ec_id(req: &Request, target_url_parsed: &mut url::Url) { /// /// When `allowed_domains` is empty every host is permitted (open mode). /// When non-empty the host must match at least one pattern via [`is_host_allowed`]. -fn is_host_permitted>(allowed_domains: &[S], host: &str) -> bool { +pub(crate) fn is_host_permitted>(allowed_domains: &[S], host: &str) -> bool { allowed_domains.is_empty() || allowed_domains .iter() @@ -1690,7 +1690,9 @@ pub async fn handle_first_party_proxy_sign( }) })?; if !settings.rewrite.should_proxy_asset(host) { - log::debug!("sign request for `{host}` declined by rewrite policy"); + log::debug!( + "sign request for `{host}` declined: host excluded or not in rewrite.include_domains" + ); return Err(Report::new(TrustedServerError::Proxy { message: "unsupported url".to_string(), })); @@ -2745,6 +2747,119 @@ mod tests { }); } + #[test] + fn proxy_sign_applies_rewrite_include_domains_for_get_and_post() { + struct Case { + name: &'static str, + include_domains: &'static [&'static str], + exclude_domains: &'static [&'static str], + allowed_domains: &'static [&'static str], + target: &'static str, + expected: StatusCode, + } + + let cases = [ + Case { + name: "listed absolute", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "https://img.example.com/a.png", + expected: StatusCode::OK, + }, + Case { + name: "listed protocol-relative", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "//img.example.com/a.png", + expected: StatusCode::OK, + }, + Case { + name: "off-list absolute", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "https://img.example.net/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "off-list protocol-relative", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "//img.example.net/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "host in both lists", + include_domains: &["*.example.com"], + exclude_domains: &["img.example.com"], + allowed_domains: &[], + target: "https://img.example.com/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "off include list and off proxy allowlist", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &["*.example.org"], + target: "https://img.example.net/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "listed but off proxy allowlist", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &["*.example.org"], + target: "https://img.example.com/a.png", + expected: StatusCode::FORBIDDEN, + }, + Case { + name: "mixed-case host", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "https://IMG.Example.COM/a.png", + expected: StatusCode::OK, + }, + ]; + + let to_owned = |values: &[&str]| -> Vec { + values.iter().map(|value| (*value).to_owned()).collect() + }; + + futures::executor::block_on(async { + for case in &cases { + for method in [&Method::GET, &Method::POST] { + let label = format!("{} {}", method.as_str(), case.name); + let mut settings = create_test_settings(); + settings.rewrite.include_domains = to_owned(case.include_domains); + settings.rewrite.exclude_domains = to_owned(case.exclude_domains); + settings.proxy.allowed_domains = to_owned(case.allowed_domains); + let req = build_proxy_sign_request( + method, + "https://edge.example.com/first-party/sign", + case.target, + ); + + let status = + match handle_first_party_proxy_sign(&settings, &noop_services(), req).await + { + Ok(response) => response.status(), + Err(error) => error.current_context().status_code(), + }; + + assert_eq!( + status, case.expected, + "{label} should return {}", + case.expected + ); + } + } + }); + } + #[test] fn proxy_sign_inherits_the_request_scheme_for_protocol_relative_urls() { // A protocol-relative target adopts the scheme the visitor is on. It diff --git a/crates/trusted-server-core/src/settings.rs b/crates/trusted-server-core/src/settings.rs index c4d0321d2..4f478e575 100644 --- a/crates/trusted-server-core/src/settings.rs +++ b/crates/trusted-server-core/src/settings.rs @@ -23,7 +23,7 @@ use crate::creative_opportunities::CreativeOpportunitiesConfig; use crate::error::TrustedServerError; use crate::host_header::validate_host_header_override_value; use crate::platform::PlatformImageOptimizerRegion; -use crate::proxy::is_host_allowed; +use crate::proxy::{is_host_allowed, is_host_permitted}; use crate::redacted::Redacted; #[cfg(test)] @@ -660,11 +660,12 @@ impl Rewrite { /// Returns `true` when an asset URL on `host` should be rewritten to /// `/first-party/proxy`. /// - /// The host must not match [`Self::exclude_domains`]. Matching is + /// The host must not match [`Self::exclude_domains`], and must match + /// [`Self::include_domains`] when that list is non-empty. Matching is /// case-insensitive; see [`is_host_allowed`] for the pattern rules. #[must_use] pub fn should_proxy_asset(&self, host: &str) -> bool { - !self.is_excluded_host(host) + !self.is_excluded_host(host) && is_host_permitted(&self.include_domains, host) } /// Returns `true` when a click-through URL on `host` should be wrapped in @@ -6718,6 +6719,43 @@ source_domain = "partner.example.com" ); } + #[test] + fn rewrite_should_proxy_asset_honors_include_list() { + let mut rewrite = Rewrite::default(); + rewrite + .include_domains + .extend(["*.example.com", "img.example.net"].map(str::to_owned)); + rewrite + .exclude_domains + .push("blocked.example.com".to_owned()); + + for (host, expected) in [ + ("example.com", true), + ("a.b.example.com", true), + ("img.example.net", true), + ("IMG.EXAMPLE.NET", true), + ("evil-example.com", false), + ("cdn.example.org", false), + ("blocked.example.com", false), + ] { + assert_eq!( + rewrite.should_proxy_asset(host), + expected, + "should_proxy_asset(`{host}`) should be {expected}" + ); + } + for host in ["cdn.example.org", "evil-example.com", "example.com"] { + assert!( + rewrite.should_wrap_click(host), + "should wrap click for `{host}` regardless of include_domains" + ); + } + assert!( + !rewrite.should_wrap_click("blocked.example.com"), + "should not wrap a click to an excluded host" + ); + } + #[test] fn rewrite_include_domains_are_normalized_from_toml() { let toml_str = crate_test_settings_str() From 9d9fb273320e094e127bba0d4e0f0fd21a10537d Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 15:09:54 +0530 Subject: [PATCH 10/20] Rewrite link imagesrcset once on the inline creative path Signed-off-by: dhruv8sh --- crates/trusted-server-core/src/creative.rs | 55 +++++++++++++++++++--- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/crates/trusted-server-core/src/creative.rs b/crates/trusted-server-core/src/creative.rs index 14f3bbd5a..e468fbe28 100644 --- a/crates/trusted-server-core/src/creative.rs +++ b/crates/trusted-server-core/src/creative.rs @@ -16,7 +16,7 @@ //! - `` //! - SVG: ``, `` //! - `