From 3631a82932744c49e0e2e001da691171aec9a28f Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 14:48:36 +0530 Subject: [PATCH 01/14] Add design spec and plan for separate creative click rewrite switch Spec and implementation plan for #1234: an [auction] rewrite_clicks switch that follows rewrite_creatives when unset, so click wrapping can be controlled independently of asset rewriting. Signed-off-by: dhruv8sh --- ...0-07-1234-creative-click-rewrite-switch.md | 1710 +++++++++++++++++ ...34-creative-click-rewrite-switch-design.md | 511 +++++ 2 files changed, 2221 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-07-1234-creative-click-rewrite-switch.md create mode 100644 docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md diff --git a/docs/superpowers/plans/2026-10-07-1234-creative-click-rewrite-switch.md b/docs/superpowers/plans/2026-10-07-1234-creative-click-rewrite-switch.md new file mode 100644 index 000000000..03f83b802 --- /dev/null +++ b/docs/superpowers/plans/2026-10-07-1234-creative-click-rewrite-switch.md @@ -0,0 +1,1710 @@ +# Creative Click Rewrite Switch 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 `[auction] rewrite_clicks` so creative click wrapping (`` / `` → signed `/first-party/click`) can be turned on or off independently of asset rewriting, without changing output for any existing config. + +**Architecture:** + +- Replace `to_abs` with a pure URL normalizer plus host policy on `Rewrite`. This "shared step" is written identically in #1231's PR. +- Add an `Option` field on `AuctionConfig` with two resolution methods. +- Give the lol_html rewrite pass a `CreativeFeatures { assets, clicks }` parameter, and register the asset and anchor handlers only when their feature is on. +- `` removal and TSJS injection run whenever the pass runs. + +**Tech Stack:** Rust 2024 (`trusted-server-core`, `trusted-server-cli`), `lol_html`, `url`, `serde`, EdgeZero typed app-config loader, Viceroy for `wasm32-wasip1` tests. + +**Spec:** `docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md` (issue [#1234](https://github.com/IABTechLab/trusted-server/issues/1234)). Read it first. + +**Status:** Validated by an exploratory implementation on 2026-10-07. Every code block below is the code that compiled, passed clippy and passed its tests. Each step's "Expected" line is the observed result. + +## Global Constraints + +- With `rewrite_clicks` unset, rewritten output must be byte-identical to today for both values of `rewrite_creatives` on `/auction` and inline SSAT/page-bids, and unchanged for `/first-party/proxy` HTML. +- `rewrite_clicks` is `Option` with `#[serde(default, skip_serializing_if = "Option::is_none")]`. `AuctionConfig` keeps `deny_unknown_fields`. +- Unset resolution: auction paths use `rewrite_clicks.unwrap_or(rewrite_creatives)`, and proxied HTML uses `rewrite_clicks.unwrap_or(true)`. +- `exclude_domains` applies to anchors. The #1231 include list never does. +- The shared matcher is `crate::proxy::is_host_allowed` (`proxy.rs:1242`), which is case-insensitive. +- No JavaScript, adapter, routing or dependency changes. `/first-party/click` and `/first-party/proxy-rebuild` stay routed. +- AGENTS.md rules apply: + - example.com-style domains only; + - `expect("should ...")` messages; + - comments on their own line; + - no local `use` inside functions; + - every public item documented; + - at most 7 function arguments. +- Commits use sentence case, imperative mood, and `git commit --signoff -S` with multiple `-m` flags. No prefixes, no attribution trailers, never a heredoc. + +## Environment notes (learned during validation) + +- `cargo test-fastly` runs the wasm test binary through Viceroy. If `viceroy` is installed under `~/.cargo/bin` but not on `PATH`, prefix commands with `PATH=$HOME/.cargo/bin:$PATH`. Validation used Viceroy 0.21.1; `.tool-versions` pins 0.17.0. +- Under Viceroy, one panicking test aborts the whole wasm test binary (exit 134) and hides the assertion message. To watch a test fail, run it alone: + + ```bash + cargo test-fastly -p trusted-server-core --lib -- --exact --nocapture + ``` + + Read the `panicked at` line. + +- A test that only adds a new symbol fails at compile time (`E0425`/`E0599`/`E0609`). That is the red step for API-shaped tests. Tests that only use existing APIs must be run against the old code to see a runtime red. +- The worktree may have no `docs/node_modules`. Use a checkout that has one: + + ```bash + /path/to/checkout/docs/node_modules/.bin/prettier --config docs/.prettierrc ... + ``` + + `cd docs && npm run format` (check mode) still works. + +## File Map + +| File | Responsibility in this change | +| ------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------- | +| `crates/trusted-server-core/src/creative.rs` | `normalize_creative_url`, `asset_target`/`click_target`, `CreativeFeatures`, conditional handler registration, caller wiring, matrix tests. | +| `crates/trusted-server-core/src/settings.rs` | `Rewrite::should_proxy_asset` / `should_wrap_click` / `normalize`, removal of `is_excluded`, TOML tests. | +| `crates/trusted-server-core/src/proxy.rs` | `/first-party/sign` call-site migration, sign and proxied-HTML tests. | +| `crates/trusted-server-core/src/auction_config_types.rs` | `rewrite_clicks`, `rewrites_auction_clicks` / `rewrites_proxied_clicks`, serialization tests. | +| `crates/trusted-server-core/src/config_payload.rs` | Blob round-trip tests. | +| `crates/trusted-server-core/src/auction/orchestrator.rs` | `rewrite_clicks: None` in the one exhaustive `AuctionConfig` literal. | +| `crates/trusted-server-core/src/auction/formats.rs` | Doc comment, debug log, `/auction` end-to-end test. | +| `crates/trusted-server-core/src/publisher.rs` | Inline end-to-end test. | +| `crates/trusted-server-core/src/auction/endpoints.rs` | Response doc comment. | +| `crates/trusted-server-cli/tests/config_env_overlay.rs` | Overlay tests for `TRUSTED_SERVER__AUCTION__REWRITE_CLICKS`. | +| `trusted-server.example.toml` | Reworded `rewrite_creatives` comment and commented-out `rewrite_clicks` leaf. | +| `docs/guide/*.md`, `crates/trusted-server-core/src/auction/README.md`, `CHANGELOG.md` | Operator documentation. | + +## 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`. + +--- + +### Task 0: Pre-flight, check whether #1231 already landed the shared step + +**Files:** none modified. + +- [ ] **Step 1: Check main** + +```bash +git fetch origin +git grep -n 'fn normalize_creative_url' origin/main -- crates/trusted-server-core/src/creative.rs +git grep -n 'fn should_wrap_click' origin/main -- crates/trusted-server-core/src/settings.rs +``` + +- [ ] **Step 2: Decide** + +- Both greps print nothing: run **Task 1**. +- Both print matches: `git rebase origin/main`, **skip Task 1**, and confirm that `click_target` exists with `git grep -n 'fn click_target' -- crates/trusted-server-core/src/creative.rs`. It is part of the shared step, so it should. Note in the PR description that the shared step came from #1231. + +--- + +### Task 1: Shared step (skip if Task 0 found it on main) + +This task writes the [Shared step](#shared-step) code and tests verbatim. They are identical to #1231's Tasks 1–2. + +**Files:** + +- Modify: `crates/trusted-server-core/src/settings.rs` (import, `impl Rewrite`, `normalize_deserialized`, tests) +- Modify: `crates/trusted-server-core/src/creative.rs` (module doc, `to_abs`, `CssUrlRewriter::rewrite`, `proxy_if_abs`, `rewrite_srcset`, anchor handler, test imports, tests) +- Modify: `crates/trusted-server-core/src/proxy.rs` (`handle_first_party_proxy_sign`, tests) +- Modify: `CHANGELOG.md` + +**Interfaces:** + +- Produces: + - `pub(super) fn normalize_creative_url(url: &str) -> Option`; + - private `asset_target(&Settings, &str) -> Option` and `click_target(&Settings, &str) -> Option`; + - `pub fn Rewrite::should_proxy_asset(&self, host: &str) -> bool`; + - `pub fn Rewrite::should_wrap_click(&self, host: &str) -> bool`; + - private `Rewrite::is_excluded_host` and `Rewrite::normalize`. +- Removes: `to_abs`, `Rewrite::is_excluded`. `proxy::is_host_permitted` stays private. + +- [ ] **Step 1: Settings tests** + +Replace `test_rewrite_is_excluded` in `settings.rs` with: + +```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}" + ); + } + } + + #[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" + ); + } + + #[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" + ); + } + + #[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 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: Creative tests** + +Change the `creative.rs` test module's import list 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` 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`; its cases are in the table above. Replace `to_abs_respects_exclude_domains` and `to_abs_respects_wildcard_domains` 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}`" + ); + } + } + + #[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}" + ); + } + + #[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}" + ); + } +``` + +The unparseable tests assert `contains`, not equality: `rewrite_creative_html` injects the TSJS script tag even into markup with no ``. + +- [ ] **Step 3: Sign tests** + +In `proxy.rs`, replace `proxy_sign_rejects_excluded_urls`, and add the case test directly after it: + +```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() + ); + } + } + }); + } + + #[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 4: Verify red** + +Run: `cargo test-fastly --no-run` + +Expected: compile errors, including `error[E0432]: unresolved import super::normalize_creative_url` and `no method named should_proxy_asset found for struct settings::Rewrite` (also `should_wrap_click` and `normalize`). + +Five tests use only existing APIs. Run each alone against the unmodified code, as described in the environment notes. Each panics: + +- `unparseable_absolute_url_is_left_byte_identical`: the output re-quotes the value as ``. +- `unparseable_absolute_click_url_is_left_byte_identical`: the output is ``. +- `exclude_domains_match_case_insensitively_in_the_rewrite_pass`: the link is wrapped in `/first-party/click`. +- `proxy_sign_rejects_excluded_urls_case_insensitively`: sign returns `Response { status: 200 ... }`. +- `settings_load_normalizes_rewrite_exclude_domains`: the loaded list is not normalized. + +- [ ] **Step 5: Implement** + +Apply the [Shared step](#shared-step) code to `settings.rs`, `creative.rs` and `proxy.rs`. Delete `Rewrite::is_excluded` with its `#[allow(dead_code)]`. + +Run: `git grep -n "to_abs\|is_excluded(" crates/` + +Expected: no matches. + +- [ ] **Step 6: Verify green** + +Run: `cargo fmt --all && cargo test-fastly -- normalize_creative_url proxy_if_abs_respects unparseable_absolute exclude_domains_match_case proxy_sign_ rewrite_policy_matches rewrite_normalize_trims rewrite_exclude_domains_match settings_load_normalizes` + +Expected: `16 passed` in `trusted-server-core`: the eleven shared tests plus the five existing `proxy_sign_*` tests. + +- [ ] **Step 7: Lint and full suite** + +Run: `cargo clippy-fastly && cargo test-fastly` + +Expected: clippy exits 0 with no `#[allow]` attributes. Every crate passes `test-fastly`, and no existing rewrite-output assertion needs editing. + +- [ ] **Step 8: CHANGELOG** + +Under `## [Unreleased]` › `### Fixed` (create the heading if absent), as the first entry: + +```markdown +- `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. +``` + +- [ ] **Step 9: Commit** + +```bash +git add crates/trusted-server-core/src/creative.rs crates/trusted-server-core/src/settings.rs crates/trusted-server-core/src/proxy.rs CHANGELOG.md +git commit --signoff -S -m "Split creative URL normalization from rewrite host policy" -m "Replace to_abs with normalize_creative_url and move exclusion policy to Rewrite::should_proxy_asset and Rewrite::should_wrap_click, sharing the case-insensitive proxy host matcher. Normalize exclude_domains at load. This is the shared step for #1231 and #1234." +``` + +--- + +### Task 2: `rewrite_clicks` setting and resolution + +**Files:** + +- Modify: `crates/trusted-server-core/src/auction_config_types.rs` (field docs and field, `Default`, `impl AuctionConfig`, tests) +- Modify: `crates/trusted-server-core/src/auction/orchestrator.rs` (`test_no_providers_configured` literal) +- Modify: `crates/trusted-server-core/src/config_payload.rs` (tests) +- Modify: `crates/trusted-server-core/src/settings.rs` (tests) + +**Interfaces:** + +- Produces: + - `pub rewrite_clicks: Option` on `AuctionConfig`; + - `pub fn AuctionConfig::rewrites_auction_clicks(&self) -> bool`; + - `pub fn AuctionConfig::rewrites_proxied_clicks(&self) -> bool`. + +- [ ] **Step 1: Write the tests** + +`auction_config_types.rs`, before `default_sanitize_creatives_is_not_serialized`: + +```rust + #[test] + fn default_rewrite_clicks_is_unset_and_not_serialized() { + let config = AuctionConfig::default(); + + assert_eq!( + config.rewrite_clicks, None, + "should leave click rewriting unset by default" + ); + let serialized = serde_json::to_value(&config).expect("should serialize defaults"); + assert!( + serialized.get("rewrite_clicks").is_none(), + "should omit the unset click setting from serialized config" + ); + } + + #[test] + fn explicit_rewrite_clicks_is_serialized() { + for value in [true, false] { + let config = AuctionConfig { + rewrite_clicks: Some(value), + ..AuctionConfig::default() + }; + + let serialized = + serde_json::to_value(config).expect("should serialize explicit click setting"); + + assert_eq!( + serialized.get("rewrite_clicks"), + Some(&serde_json::Value::Bool(value)), + "should serialize explicit rewrite_clicks = {value}" + ); + } + } + + #[test] + fn click_rewriting_resolves_per_entry_point() { + // (rewrite_creatives, rewrite_clicks, auction clicks, proxied clicks) + let cases = [ + (true, None, true, true), + (false, None, false, true), + (true, Some(false), false, false), + (false, Some(true), true, true), + (true, Some(true), true, true), + (false, Some(false), false, false), + ]; + + for (rewrite_creatives, rewrite_clicks, auction, proxied) in cases { + let config = AuctionConfig { + rewrite_creatives, + rewrite_clicks, + ..AuctionConfig::default() + }; + + assert_eq!( + config.rewrites_auction_clicks(), + auction, + "auction clicks for rewrite_creatives={rewrite_creatives} rewrite_clicks={rewrite_clicks:?}" + ); + assert_eq!( + config.rewrites_proxied_clicks(), + proxied, + "proxied clicks for rewrite_creatives={rewrite_creatives} rewrite_clicks={rewrite_clicks:?}" + ); + } + } +``` + +`config_payload.rs`, before `strings_that_look_like_json_scalars_round_trip_as_strings`: + +```rust + #[test] + fn legacy_blob_without_rewrite_clicks_follows_rewrite_creatives() { + for rewrite_creatives in [true, false] { + let mut original = test_settings(); + original.auction.rewrite_creatives = rewrite_creatives; + let data = + serde_json::to_value(&original).expect("should serialize settings to JSON"); + assert!( + data["auction"].get("rewrite_clicks").is_none(), + "should omit unset rewrite_clicks from the payload" + ); + + let reconstructed = load_settings(&envelope_json(&original)) + .expect("should reconstruct settings without rewrite_clicks"); + + assert_eq!( + reconstructed.auction.rewrite_clicks, None, + "should load a blob without rewrite_clicks as unset" + ); + assert_eq!( + reconstructed.auction.rewrites_auction_clicks(), + rewrite_creatives, + "should follow rewrite_creatives = {rewrite_creatives} when unset" + ); + } + } + + #[test] + fn explicit_rewrite_clicks_survives_blob_round_trip() { + for (rewrite_creatives, rewrite_clicks) in [(true, false), (false, true)] { + let mut original = test_settings(); + original.auction.rewrite_creatives = rewrite_creatives; + original.auction.rewrite_clicks = Some(rewrite_clicks); + + let reconstructed = load_settings(&envelope_json(&original)) + .expect("should reconstruct explicit rewrite_clicks"); + + assert_eq!( + reconstructed.auction.rewrite_clicks, + Some(rewrite_clicks), + "should preserve rewrite_clicks = {rewrite_clicks} with rewrite_creatives = {rewrite_creatives}" + ); + } + } +``` + +`settings.rs`, before `test_auction_rewrite_creatives_accepts_explicit_false`: + +```rust + #[test] + fn test_auction_rewrite_clicks_accepts_explicit_values() { + for value in [true, false] { + let toml_str = crate_test_settings_str() + + &format!( + r#" + [auction] + enabled = true + rewrite_clicks = {value} + "# + ); + + let settings = Settings::from_toml(&toml_str).expect("should parse valid TOML"); + + assert_eq!( + settings.auction.rewrite_clicks, + Some(value), + "should parse explicit rewrite_clicks = {value}" + ); + } + } +``` + +Append this assertion to `test_auction_creative_processing_defaults_when_omitted`: + +```rust + assert_eq!( + settings.auction.rewrite_clicks, None, + "click rewriting stays unset when the setting is omitted" + ); +``` + +- [ ] **Step 2: Verify red** + +Run: `cargo test-fastly -p trusted-server-core --lib --no-run` + +Expected: 11 errors: + +- `E0560` (`AuctionConfig` has no field named `rewrite_clicks`), twice; +- `E0599` (`no method named rewrites_auction_clicks`), twice; +- `E0599` (`rewrites_proxied_clicks`), once; +- `E0609` (`no field rewrite_clicks`), six times. + +- [ ] **Step 3: Implement** + +`auction_config_types.rs`: replace the first paragraph of the `rewrite_creatives` doc comment, and add the field after `pub rewrite_creatives: bool,`: + +```rust + /// Rewrite winning-bid creative asset URLs (images, scripts, styles, + /// media, iframes, CSS `url()`) to first-party `/first-party/proxy` + /// endpoints, applied after sanitization when + /// [`Self::sanitize_creatives`] is enabled. + /// + /// Bidder `` removal and creative TSJS injection run whenever this + /// or click rewriting ([`Self::rewrites_auction_clicks`]) is on. + /// + /// The default stays omitted from serialized config blobs to avoid adding + /// this field when it has no effect. Any rollback across schema versions + /// still requires restoring the matching old-schema blob with the old + /// binary. + #[serde( + default = "default_rewrite_creatives", + skip_serializing_if = "is_default_rewrite_creatives" + )] + pub rewrite_creatives: bool, + + /// Wrap creative click-through links (``, ``) in signed + /// `/first-party/click` redirects. + /// + /// Unset keeps each path's existing behavior: auction creatives follow + /// [`Self::rewrite_creatives`], and HTML fetched through + /// `/first-party/proxy` keeps wrapping. An explicit value applies to every + /// path. Unset is omitted from serialized config blobs, so older binaries + /// keep loading them; any explicit value is serialized and rejected by + /// binaries that predate this field. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub rewrite_clicks: Option, +``` + +In `impl Default for AuctionConfig`, after `rewrite_creatives: default_rewrite_creatives(),`, add `rewrite_clicks: None,`. At the top of `impl AuctionConfig`: + +```rust + /// Whether auction creatives (`POST /auction` and inline SSAT/page-bids) + /// wrap click-through links. + /// + /// Unset [`Self::rewrite_clicks`] follows [`Self::rewrite_creatives`]. + #[must_use] + pub fn rewrites_auction_clicks(&self) -> bool { + self.rewrite_clicks.unwrap_or(self.rewrite_creatives) + } + + /// Whether HTML fetched through `/first-party/proxy` wraps click-through + /// links. + /// + /// Unset [`Self::rewrite_clicks`] keeps wrapping, as before the setting + /// existed. + #[must_use] + pub fn rewrites_proxied_clicks(&self) -> bool { + self.rewrite_clicks.unwrap_or(true) + } +``` + +In `auction/orchestrator.rs`, `test_no_providers_configured`, add `rewrite_clicks: None,` after `rewrite_creatives: true,`. It is the only `AuctionConfig` literal without `..Default::default()`. + +- [ ] **Step 4: Verify green** + +Run: `cargo test-fastly -p trusted-server-core --lib -- auction_config_types:: rewrite_clicks test_auction_creative_processing_defaults` + +Expected: `15 passed; 0 failed`. + +- [ ] **Step 5: Commit** + +```bash +git add crates/trusted-server-core/src/auction_config_types.rs crates/trusted-server-core/src/auction/orchestrator.rs crates/trusted-server-core/src/config_payload.rs crates/trusted-server-core/src/settings.rs +git commit --signoff -S -m "Add an auction rewrite_clicks setting that follows rewrite_creatives when unset" +``` + +--- + +### Task 3: Gate asset and click handlers in the rewrite pass + +**Files:** + +- Modify: `crates/trusted-server-core/src/creative.rs` (`process_*`, wrappers and their docs, `CreativeFeatures`, `rewrite_creative_html_impl`, tests) +- Modify: `crates/trusted-server-core/src/auction/formats.rs` (doc comment, debug log, test) +- Modify: `crates/trusted-server-core/src/publisher.rs` (test) +- Modify: `crates/trusted-server-core/src/auction/endpoints.rs` (doc comment) + +**Interfaces:** + +- Consumes: `AuctionConfig::rewrites_auction_clicks()` (Task 2), `asset_target` and `click_target` (Task 1). +- Produces, private to `creative.rs`: + +```rust +struct CreativeFeatures { assets: bool, clicks: bool } +impl CreativeFeatures { const ALL: Self; fn any(self) -> bool; } +fn rewrite_creative_html_impl( + settings: &Settings, + markup: &str, + base_origin: &str, + inject_tsjs: bool, + max_output_size: usize, + features: CreativeFeatures, +) -> String; +``` + +- [ ] **Step 1: Matrix fixtures and helpers** + +At the top of `creative.rs` `mod tests`, after the `use super::{...}` block: + +```rust + use crate::settings::Settings; + + const MATRIX_FIXTURE_WITH_BODY: &str = r#"
AdExcludedMail"#; + const MATRIX_FIXTURE_FRAGMENT: &str = r#"AdExcluded"#; + 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 + } +``` + +- [ ] **Step 2: Matrix and unset-pin tests** + +```rust + #[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) => 1, + }; + 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 anchor carries the click URL in href and data-tsclick. + let expected_click_urls = match (clicks, has_body) { + (false, _) => 0, + (true, true) => 4, + (true, false) => 2, + }; + 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}" + ); + if has_body { + 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" + ); + } + } +``` + +- [ ] **Step 3: End-to-end tests** + +`auction/formats.rs`, before `sanitize_creatives_defaults_to_disabled`: + +```rust + #[test] + fn convert_to_openrtb_response_wraps_clicks_without_rewriting_assets() { + let mut settings = make_settings(); + settings.auction.sanitize_creatives = false; + settings.auction.rewrite_creatives = false; + settings.auction.rewrite_clicks = Some(true); + let auction_request = make_auction_request(); + let result = make_result(make_complete_creative_bid()); + + let response = convert_to_openrtb_response(&result, &settings, &auction_request, false) + .expect("should convert creative with click rewriting only"); + let adm = response_adm(response); + + assert!( + adm.contains("/first-party/click?tsurl=") && adm.contains("data-tsclick"), + "should wrap the landing link: {adm}" + ); + assert!( + !adm.contains("/first-party/proxy?tsurl="), + "should not proxy any asset: {adm}" + ); + assert!( + adm.contains(r#"src="https://cdn.example.com/ad.png""#), + "should keep the image URL direct: {adm}" + ); + assert!( + adm.contains("tsjs-unified.min.js"), + "should inject the creative runtime for the click guard: {adm}" + ); + } +``` + +`publisher.rs`, before `build_bid_map_uses_request_origin_for_inline_urls`: + +```rust + #[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}" + ); + } +``` + +- [ ] **Step 4: Verify red** + +Run each test alone, as described in the environment notes. Expected: + +- `asset_and_click_switches_combine_on_auction_and_inline_paths` panics with "Auction assets=true clicks=false body=true: click URL count". +- `convert_to_openrtb_response_wraps_clicks_without_rewriting_assets` panics with "should wrap the landing link". Nothing was rewritten. +- `build_bid_map_keeps_inline_anchors_raw_when_clicks_are_off` panics with "should leave the landing link raw". The anchor was wrapped. +- `unset_rewrite_clicks_preserves_existing_output` **passes**. It pins today's behavior. + +- [ ] **Step 5: Add `CreativeFeatures`** + +Above the `rewrite_creative_html_impl` doc comment: + +```rust +/// Which rewrite features one creative pass applies. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct CreativeFeatures { + /// Proxy eligible asset URLs through `/first-party/proxy`. + assets: bool, + /// Wrap eligible click-through links in `/first-party/click`. + clicks: bool, +} + +impl CreativeFeatures { + /// Both features on: the behavior of the public rewrite wrappers. + const ALL: Self = Self { + assets: true, + clicks: true, + }; + + /// Whether the pass needs to run at all. + fn any(self) -> bool { + self.assets || self.clicks + } +} +``` + +Extend the `rewrite_creative_html_impl` doc comment's sentence to read "...`max_output_size` bounds the rewritten result; `features` selects which handlers run. `` removal and TSJS injection always run, since callers only invoke the pass when at least one feature is on." Add `features: CreativeFeatures,` as the last parameter. + +- [ ] **Step 6: Build the handler vector conditionally** + +Inside `rewrite_creative_html_impl`, after `let overflowed = ...;`, build the vector, then hand it to `HtmlRewriter::new`. Each block below is the existing handler moved verbatim with its comment. Only the indentation changes, so review with `git diff -w`. + +```rust + let mut element_content_handlers = vec![ + // (existing `` removal handler with its comment) + // (existing `` TSJS handler) + ]; + if features.assets { + element_content_handlers.extend([ + // (existing handlers, unchanged and in their current order: img, + // script[src], link[href], video/audio/source, object[data], + // embed[src], input[src], SVG image/use, [style], text!("style"), + // iframe, [srcset], [imagesrcset]) + ]); + } + if features.clicks { + element_content_handlers.push( + // Click-through links + 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(()) + }), + ); + } + let mut rewriter = HtmlRewriter::new( + HtmlSettings { + element_content_handlers, + ..HtmlSettings::default() + }, + // (existing output sink closure, unchanged) + ); +``` + +`extend([...])` with an array of `element!`/`text!` invocations type-checks with no annotations, because every invocation yields the same tuple type. The body-less TSJS fallback keeps `if inject_tsjs && !injected_ts_creative.get() && !rewritten.is_empty()`. + +- [ ] **Step 7: Wire the callers** + +```rust +pub(crate) fn process_auction_creative(settings: &Settings, raw: &str) -> String { + process_auction_creative_with_rewriter(settings, raw, |sanitized, features| { + rewrite_creative_html_impl(settings, sanitized, "", true, MAX_CREATIVE_SIZE, features) + }) +} + +pub(crate) fn process_inline_auction_creative( + settings: &Settings, + base_origin: &str, + raw: &str, +) -> String { + process_auction_creative_with_rewriter(settings, raw, |sanitized, features| { + rewrite_creative_html_impl( + settings, + sanitized, + base_origin, + false, + MAX_CREATIVE_SIZE, + features, + ) + }) +} +``` + +In `process_auction_creative_with_rewriter`, change the closure type to `rewrite: impl FnOnce(&str, CreativeFeatures) -> String`, and replace the `if settings.auction.rewrite_creatives { ... }` tail with: + +```rust + let features = CreativeFeatures { + assets: settings.auction.rewrite_creatives, + clicks: settings.auction.rewrites_auction_clicks(), + }; + if features.any() { + rewrite(&sanitized, features) + } else { + sanitized + } +``` + +Public wrappers: pass `CreativeFeatures::ALL` as the sixth argument in `rewrite_creative_html` (`"", true, MAX_CREATIVE_SIZE`), `rewrite_inline_creative_html` (`base_origin, false, MAX_CREATIVE_SIZE`) and, for now, `rewrite_proxied_html` (`"", true, MAX_REWRITABLE_BODY_SIZE`). Task 4 changes the last one. + +- [ ] **Step 8: Doc comments** + +- `process_auction_creative`: "Sanitization is controlled by [`...sanitize_creatives`], asset rewriting by [`...rewrite_creatives`], and click wrapping by [`...rewrites_auction_clicks`]. With all three disabled the creative is returned exactly as the bidder sent it." +- `process_inline_auction_creative`: "Applies the same opt-in sanitization and the same asset and click switches as [`process_auction_creative`]. Proxy and click URLs are emitted as absolute URLs against `base_origin` without injecting the creative TSJS bundle." +- `rewrite_creative_html`: replace the stale `tsjs-creative` bullet with: + - "`` / `` → `/first-party/click?tsurl=…`, copied into `data-tsclick`"; + - "Injects the unified TSJS bundle (`/static/tsjs=tsjs-unified.min.js`), whose creative runtime installs the click guard, once at the top of ``." + + Add: "Rewrites assets and clicks unconditionally; the auction settings are applied by the auction processing entry points, not here." + +- `rewrite_inline_creative_html`: "its click guard is unnecessary for click URLs that are already absolute here". +- `auction/endpoints.rs` response doc: "Creative HTML is inlined in each bid's `adm` field after optional sanitization ([`auction.sanitize_creatives`][...]). First-party asset rewriting ([`auction.rewrite_creatives`][...]) and click wrapping ([`auction.rewrite_clicks`][...]) are enabled by default. Bidder `` removal and creative TSJS injection run when either is on." This replaces the stale "mandatory server-side sanitization". +- `auction/formats.rs`, on `convert_to_openrtb_response`: name `rewrite_clicks` next to `rewrite_creatives` and add the intra-doc link target `/// [`AuctionConfig::rewrite_clicks`]: crate::auction_config_types::AuctionConfig::rewrite_clicks`. Add `clicks {}` to the "Processed creative" debug log, fed by `settings.auction.rewrites_auction_clicks()`. + +- [ ] **Step 9: Verify green** + +Run: `cargo fmt --all && cargo test-fastly -p trusted-server-core --lib -- creative:: auction::formats publisher::` + +Expected: `551 passed; 0 failed`, including every pre-existing rewrite test. + +Run: `cargo clippy-fastly` + +Expected: exit 0. + +- [ ] **Step 10: Commit** + +```bash +git add crates/trusted-server-core/src/creative.rs crates/trusted-server-core/src/auction/formats.rs crates/trusted-server-core/src/publisher.rs crates/trusted-server-core/src/auction/endpoints.rs +git commit --signoff -S -m "Gate creative asset and click rewriting on separate switches" -m "The rewrite pass registers asset handlers only when rewrite_creatives is on and the anchor handler only when click rewriting resolves on. Base removal and TSJS injection run whenever either is on." +``` + +--- + +### Task 4: Apply an explicit `rewrite_clicks` to HTML fetched through `/first-party/proxy` + +**Files:** + +- Modify: `crates/trusted-server-core/src/creative.rs` (`rewrite_proxied_html`) +- Test: `crates/trusted-server-core/src/proxy.rs` + +- [ ] **Step 1: Write the test** + +Before `html_response_rewrite_preserves_non_standard_port`: + +```rust + #[test] + fn proxied_html_click_wrapping_follows_explicit_rewrite_clicks() { + let html = r#""#; + // (rewrite_creatives, rewrite_clicks, expect wrapped clicks) + let cases = [ + (true, None, true), + (false, None, 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; + 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}" + ); + assert_eq!( + body.contains("/first-party/click?tsurl="), + expect_clicks, + "{label}: click wrapping in proxied HTML: {body}" + ); + assert_eq!( + body.contains("data-tsclick"), + expect_clicks, + "{label}: data-tsclick in proxied HTML: {body}" + ); + assert!( + body.contains("/static/tsjs="), + "{label}: proxied HTML always receives the runtime: {body}" + ); + } + } +``` + +- [ ] **Step 2: Verify red** + +Run the test alone. Expected: a panic with "rewrite_creatives=true rewrite_clicks=Some(false): click wrapping in proxied HTML". + +- [ ] **Step 3: Implement** + +In `rewrite_proxied_html`, replace `CreativeFeatures::ALL` with: + +```rust + CreativeFeatures { + assets: true, + clicks: settings.auction.rewrites_proxied_clicks(), + }, +``` + +Append this paragraph to its doc comment: "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`." + +- [ ] **Step 4: Verify green** + +Run: `cargo test-fastly -p trusted-server-core --lib -- proxy::tests::proxied_html proxy::tests::auction_rewrite_setting_does_not_change` + +Expected: `2 passed; 0 failed`. + +- [ ] **Step 5: Commit** + +```bash +git add crates/trusted-server-core/src/creative.rs crates/trusted-server-core/src/proxy.rs +git commit --signoff -S -m "Apply an explicit rewrite_clicks to HTML fetched through the first-party proxy" +``` + +--- + +### Task 5: Example config and CLI overlay coverage + +**Files:** + +- Modify: `trusted-server.example.toml` +- Test: `crates/trusted-server-cli/tests/config_env_overlay.rs` + +- [ ] **Step 1: Write the overlay tests** + +Add a constant after `SANITIZE_ENV`: + +```rust +const CLICKS_ENV: &str = "TRUSTED_SERVER__AUCTION__REWRITE_CLICKS"; +``` + +Before `config_validate_explains_legacy_provider_list_migration`: + +```rust +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" + ); +} +``` + +- [ ] **Step 2: Run** + +Run: `cargo test --package trusted-server-cli --target "$(rustc -vV | awk '/host:/ { print $2 }')" --test config_env_overlay rewrite_clicks` + +Expected: `2 passed; 0 failed`. + +These pin existing EdgeZero behavior, so there is no red step. The leaf-present test proves the override is read. The leaf-absent test proves an override for a missing leaf is ignored, so the spec's overlay caveat holds. If the leaf-absent test ever fails, update the spec and the configuration guide before continuing. + +- [ ] **Step 3: Example TOML** + +Replace the `rewrite_creatives` comment and line in `[auction]`: + +```toml +# 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 +``` + +- [ ] **Step 4: Commit** + +```bash +git add trusted-server.example.toml crates/trusted-server-cli/tests/config_env_overlay.rs +git commit --signoff -S -m "Document rewrite_clicks in the example config and pin its overlay behavior" +``` + +--- + +### Task 6: Operator documentation and CHANGELOG + +**Files:** + +- Modify: `docs/guide/configuration.md` +- Modify: `docs/guide/creative-processing.md` +- Modify: `docs/guide/auction-orchestration.md` +- Modify: `docs/guide/api-reference.md` +- Modify: `crates/trusted-server-core/src/auction/README.md` +- Modify: `CHANGELOG.md` + +- [ ] **Step 1: `configuration.md` `[auction]`** + +- Table: change the `rewrite_creatives` description to "Rewrite winning-bid asset URLs through first-party endpoints". Add a row: + + ```markdown + | `rewrite_clicks` | Boolean | unset | Wrap creative links in `/first-party/click`; unset follows `rewrite_creatives` | + ``` + + Prettier re-pads the whole table. + +- Processing paragraph: + - assets go to `/first-party/proxy`; + - `rewrite_clicks` controls ``/`` and follows `rewrite_creatives` when unset; + - `` is removed, and TSJS injected on `POST /auction`, whenever either setting is on; + - "With all three disabled" replaces "With both disabled"; + - an explicit `rewrite_clicks` applies to links in proxied HTML, and unset keeps wrapping them. +- Warning block: + - add "any explicit `rewrite_clicks`" to the serialized values; + - rollback note: "Older binaries cannot express clicks off with assets on; rolling back from that configuration re-enables click wrapping."; + - overlay note: "`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`." + +- [ ] **Step 2: `creative-processing.md`** + +- Processing Triggers, item 1, append: "Click-through links are wrapped according to `[auction].rewrite_clicks`, which follows `rewrite_creatives` when unset." +- Item 2, append: "An explicitly set `[auction].rewrite_clicks` also applies to links in proxied HTML." +- After the sanitization warning in "Auction Rewrite Control", add an `### Assets and clicks` subsection containing: + - the TOML snippet (`rewrite_creatives = true`, `# rewrite_clicks = false`); + - the four-row table (assets × clicks → asset URLs, links, ``, TSJS); + - "SSAT/page-bids follows the same table with absolute URLs and never injects TSJS. `exclude_domains` applies to both assets and links."; + - the `clickGuard` versus `rewrite_clicks` paragraph. +- "Anchors (Click Tracking)": add `**Controlled by**: [auction].rewrite_clicks (follows rewrite_creatives when unset)`, switch the example to `advertiser.example.com`, and show the `data-tsclick` attribute. + +- [ ] **Step 3: Remaining guides** + +- `auction-orchestration.md`: + - mermaid note: `rewrite_creatives=false: asset URLs direct; clicks follow rewrite_clicks`; + - flow: `├─[rewrite_creatives or rewrite_clicks] Rewrite assets and/or links, inject creative TSJS`; + - Creative Processing paragraph: link to `/guide/creative-processing#assets-and-clicks`. +- `api-reference.md`: list `[auction].rewrite_clicks` with the other two settings. +- `auction/README.md`: add the `rewrite_clicks` bullet. + +- [ ] **Step 4: CHANGELOG** + +Under `### Added`, as the first entry: + +```markdown +- 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 cannot express clicks off with assets on. +``` + +- [ ] **Step 5: Format** + +```bash +PRETTIER=/path/to/checkout/docs/node_modules/.bin/prettier +$PRETTIER --config docs/.prettierrc --write docs/guide/configuration.md docs/guide/creative-processing.md docs/guide/auction-orchestration.md docs/guide/api-reference.md +(cd docs && npm run format) +$PRETTIER --config docs/.prettierrc --check "*.md" ".claude/**/*.md" ".github/**/*.md" "crates/**/*.md" "scripts/**/*.md" "tinybird/**/*.md" +``` + +Expected: the first `npm run format` before `--write` flags `guide/configuration.md`, because of the new table row width. After `--write`, both checks print "All matched files use Prettier code style!". + +- [ ] **Step 6: Commit** + +```bash +git add docs/guide CHANGELOG.md crates/trusted-server-core/src/auction/README.md +git commit --signoff -S -m "Document the rewrite_clicks creative setting" +``` + +--- + +### Task 7: Full verification + +- [ ] **Step 1: Run every CI gate** + +```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 +cargo test-fastly && cargo test-fastly-reuse && cargo test-axum && cargo test-cloudflare && cargo test-spin +./scripts/test-cli.sh +cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity +(cd crates/trusted-server-js/lib && npx vitest run && npm run format) +(cd docs && npm run format) +``` + +Expected: every command exits 0. The validation run's results are in the spec handoff. + +- [ ] **Step 2: Map completion criteria to tests** + +| Criterion | Test | +| --------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Unset output unchanged | `unset_rewrite_clicks_preserves_existing_output`, existing rewrite tests, unset rows of `proxied_html_click_wrapping_follows_explicit_rewrite_clicks` | +| Four-combination matrix on both paths | `asset_and_click_switches_combine_on_auction_and_inline_paths` | +| Clicks off: raw href, no `data-tsclick` | matrix, `build_bid_map_keeps_inline_anchors_raw_when_clicks_are_off` | +| Clicks on, assets off | matrix, `convert_to_openrtb_response_wraps_clicks_without_rewriting_assets` | +| `exclude_domains` applies to anchors | matrix (excluded anchor), `rewrite_click_urls_excludes_blacklisted_domains`, `exclude_domains_match_case_insensitively_in_the_rewrite_pass` | +| Proxied HTML rule | `proxied_html_click_wrapping_follows_explicit_rewrite_clicks` | +| Default omitted from the blob | `default_rewrite_clicks_is_unset_and_not_serialized`, `legacy_blob_without_rewrite_clicks_follows_rewrite_creatives`, `rewrite_clicks_environment_override_is_ignored_without_leaf` | +| Docs and CHANGELOG | Task 6 | + +- [ ] **Step 3: Hand off** + +Open the PR through the `pr-creator` agent. Link the spec, state whether this PR introduced the shared step or rebased onto #1231's, and paste the rollout and rollback order from the spec. diff --git a/docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md b/docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md new file mode 100644 index 000000000..97bed4dfc --- /dev/null +++ b/docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md @@ -0,0 +1,511 @@ +# Creative click rewriting switch + +**Issue:** [IABTechLab/trusted-server#1234](https://github.com/IABTechLab/trusted-server/issues/1234) + +**Related:** [IABTechLab/trusted-server#1231](https://github.com/IABTechLab/trusted-server/issues/1231) (asset host allowlist) + +**Date:** 2026-10-07 + +**Status:** Draft + +Line numbers below are at `main` 7a0ecb4c. + +## Problem + +Click wrapping is not a feature of its own. The creative rewrite pass turns every eligible `` and `` into a signed `/first-party/click` redirect. It does this in the same lol_html pass, behind the same switch (`[auction] rewrite_creatives`) and through the same per-URL check (`to_abs`) that it uses to proxy assets. + +As a result an operator cannot: + +- keep first-party click tracking while leaving creative assets direct; or +- proxy creative assets while leaving advertiser landing links untouched. + +## Goals + +- Add a switch that turns click wrapping on or off without affecting asset rewriting. +- Narrow `rewrite_creatives` to asset rewriting. Keep its name so existing configs still parse. +- Leave rewritten output unchanged for every existing config. In particular, upgrading must not turn click wrapping on for operators who have `rewrite_creatives = false`. +- Keep `[rewrite] exclude_domains` applying to anchors. +- Define the `to_abs` split that this issue and #1231 share, so either implementation can land first. +- Keep the config blob compatible with older binaries while the new setting is unset. + +## Non-goals + +- The asset host allowlist (#1231). It never applies to anchors. +- Unregistering `/first-party/click` or `/first-party/proxy-rebuild` when clicks are off. Creatives that were already served can still hold signed click URLs. +- Adding a click guard to the inline SSAT/page-bids path. +- Mapping any server setting onto the client `TsCreativeConfig` flags. +- Changing asset rewriting inside HTML or CSS fetched through `/first-party/proxy`. That path keeps proxying assets regardless of `rewrite_creatives`. +- Domain checks at `/first-party/click` and `/first-party/proxy-rebuild`. Those endpoints redirect any validly signed target by design (`proxy.rs:1896-1904`). + +## Current behavior + +### One pass, one switch + +`process_auction_creative_with_rewriter` (`crates/trusted-server-core/src/creative.rs:1008-1035`) enforces the size cap, optionally sanitizes, and then runs the rewrite pass only when `settings.auction.rewrite_creatives` is true (`creative.rs:1031`). With rewriting off, the markup ships exactly as sanitization left it. + +The pass itself, `rewrite_creative_html_impl` (`creative.rs:1125`), registers every handler unconditionally: + +| Handler | Location | Effect | +| -------------------------------- | ------------------------------------ | -------------------------------------------------------------------------- | +| `` removal | `creative.rs:1163` | Removes bidder `` so root-relative first-party URLs cannot rebase. | +| `` TSJS injection | `creative.rs:1168` | Prepends the unified bundle once when `inject_tsjs` is set. | +| Asset handlers | `creative.rs:1179-1281`, `1293-1339` | img, script, link, media, object/embed, input, SVG, iframe, srcset, CSS. | +| Anchor handler | `creative.rs:1283` | `a[href], area[href]` → `build_click_url`, sets `href` and `data-tsclick`. | +| Body-less fragment TSJS fallback | `creative.rs:1391` | Prepends the bundle when no `` token was seen. | + +Every per-URL decision for assets and anchors goes through `to_abs` (`creative.rs:54-75`). It trims, maps `//host` to `https://host`, rejects anything that is not an `http://` or `https://` prefix, and returns `None` when `settings.rewrite.is_excluded` matches (`creative.rs:70`). `None` leaves the attribute untouched, so an excluded anchor keeps its raw `href` and gets no `data-tsclick`. + +`to_abs` returns the trimmed input string, not a parsed URL. `build_signed_url_for` (`creative.rs:554`) parses it again with `url::Url` and, if parsing fails, returns the clear URL unchanged (`creative.rs:560-562`). + +`Rewrite::is_excluded` (`crates/trusted-server-core/src/settings.rs:645-670`) parses the URL and compares the lowercased host against raw config entries, so an entry written in uppercase never matches. `proxy::is_host_allowed` (`crates/trusted-server-core/src/proxy.rs:1242`) already lowercases both sides and uses the same exact or `*.example.com` rule. + +### Three entry points + +| Entry point | Caller | Gated by `rewrite_creatives` | URL form | TSJS | +| ------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------- | ------------------------- | ------------------------------------------------------ | +| `POST /auction` `adm` | `auction/formats.rs:398` → `process_auction_creative` (`creative.rs:985`) | Yes | Root-relative | Injected (`rewrite_creative_html`, `creative.rs:1051`) | +| SSAT and page-bids inline `adm` | `publisher.rs:5835` → `process_inline_auction_creative` (`creative.rs:998`) | Yes | Absolute on `base_origin` | Not injected (`creative.rs:1085`) | +| HTML fetched via `/first-party/proxy` | `finalize_proxied_response` (`proxy.rs:643-678`) → `CreativeHtmlProcessor` (`creative.rs:1409`) → `rewrite_proxied_html` (`creative.rs:1062`) | No | Root-relative | Injected | + +The proxy path ignoring `rewrite_creatives` is pinned by `auction_rewrite_setting_does_not_change_proxied_html_or_css_rewriting` (`proxy.rs:3582`). + +### Runtime pieces + +- `/first-party/click` (`handle_first_party_click`, `proxy.rs:1545`) validates the signature, appends `ts-ec`, and returns a `302`. +- `/first-party/proxy-rebuild` (`handle_first_party_proxy_rebuild`, `proxy.rs:1812`) re-signs a click after creative script added or removed query parameters. +- Both are routed in every adapter and gated by no setting. +- The injected script is the unified bundle (`tsjs::tsjs_unified_script_tag`, `tsjs.rs:66`), which is core plus immediate modules (`publisher.rs:544-549`). `creative` is always an immediate module (`integrations/registry.rs:1216`). +- The creative runtime (`crates/trusted-server-js/lib/src/integrations/creative/index.ts:13-16`) installs the click guard when `TsCreativeConfig.clickGuard` is true (the default) and the render guard when `renderGuard` is true (default false). `TsCreativeConfig` is defined in `crates/trusted-server-js/lib/src/shared/globals.ts:10-15`. No server setting maps to either flag. +- The click guard (`crates/trusted-server-js/lib/src/integrations/creative/click.ts`) acts only on anchors that carry a server-set `data-tsclick`. `handleGuardedClick` returns early without one (`click.ts:433-434`), and the mutation monitor scans only `a[data-tsclick], area[data-tsclick]` (`click.ts:467`). + +### Config plumbing + +`AuctionConfig` uses `deny_unknown_fields` (`crates/trusted-server-core/src/auction_config_types.rs:16-17`). `rewrite_creatives` is a `bool` with default `true` and `skip_serializing_if = "is_default_rewrite_creatives"` (`auction_config_types.rs:46-50`, `141-148`), so the default never reaches the blob. The tests are `default_rewrite_creatives_is_not_serialized` (`auction_config_types.rs:233`), `legacy_blob_without_rewrite_creatives_preserves_rewriting` (`config_payload.rs:715`) and `disabled_rewrite_creatives_survives_blob_round_trip` (`config_payload.rs:739`). + +The EdgeZero typed loader applies `TRUSTED_SERVER__...` overlays at `ts config` time. It only replaces scalar leaves already present in the TOML, and the existing TOML type drives coercion (see `docs/guide/cli.md:118-124` and the CLI tests in `crates/trusted-server-cli/tests/config_env_overlay.rs:123-342`). `trusted-server.example.toml:257-268` keeps `rewrite_creatives = true` uncommented for that reason. + +## Design + +### Overview + +```mermaid +flowchart TD + A[Creative markup] --> B{assets or clicks on?} + B -->|Neither| P[Pass through unchanged] + B -->|Either| R[Rewrite pass] + R --> BASE[Remove base] + R --> TSJS[Inject TSJS when the path injects] + R -->|assets on| AS[Asset handlers: normalize, should_proxy_asset, /first-party/proxy] + R -->|clicks on| CL[Anchor handler: normalize, should_wrap_click, /first-party/click] +``` + +### Setting + +Add one field to `AuctionConfig`, next to `rewrite_creatives`: + +```rust +/// Wrap creative click-through links (``, ``) in signed +/// `/first-party/click` redirects. +/// +/// Unset keeps each path's existing behavior: auction creatives follow +/// [`Self::rewrite_creatives`], and HTML fetched through `/first-party/proxy` +/// keeps wrapping. An explicit value applies to every path. Unset is omitted +/// from serialized config blobs, so older binaries keep loading them. +#[serde(default, skip_serializing_if = "Option::is_none")] +pub rewrite_clicks: Option, +``` + +`AuctionConfig::default()` sets it to `None`. Two methods resolve it, so no call site reads the raw field: + +```rust +impl AuctionConfig { + /// Whether auction creatives (`/auction` and inline SSAT) wrap clicks. + #[must_use] + pub fn rewrites_auction_clicks(&self) -> bool { + self.rewrite_clicks.unwrap_or(self.rewrite_creatives) + } + + /// Whether HTML fetched through `/first-party/proxy` wraps clicks. + #[must_use] + pub fn rewrites_proxied_clicks(&self) -> bool { + self.rewrite_clicks.unwrap_or(true) + } +} +``` + +Resolution: + +| `rewrite_creatives` | `rewrite_clicks` | Auction assets | Auction clicks | Proxied HTML assets | Proxied HTML clicks | +| ------------------- | ---------------- | -------------- | -------------- | ------------------- | ------------------- | +| `true` (default) | unset (default) | on | on | on | on | +| `false` | unset | off | off | on | on | +| `true` | `false` | on | off | on | off | +| `false` | `true` | off | on | on | on | +| `true` | `true` | on | on | on | on | +| `false` | `false` | off | off | on | off | + +The first two rows are today's behavior, byte for byte. + +`rewrite_creatives` keeps its name and type. Its doc comment, the example TOML and the guides change to say it governs asset rewriting only, plus the shared `` removal and TSJS injection described below. + +### URL normalization and policy (shared with #1231) + +This section is the coordination contract. The #1231 spec describes the same split, and the shared step's code is identical in both implementation PRs. The canonical code is in the implementation plan's "Shared step" section. + +`to_abs` is replaced by a pure normalizer in `creative.rs` with the same crate visibility `to_abs` has today: + +```rust +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 + } +} +``` + +It trims the input, maps a protocol-relative `//host/...` to `https://host/...`, and accepts only a case-insensitive `http://` or `https://` prefix. It parses absolute input in place without allocating, and returns `None` for empty, relative, non-http(s) or unparseable input. It needs no separate hostless check, because `url` rejects an `http(s)` URL without a host. It makes no policy decision and reads no settings. + +Policy lives on `Rewrite` in `settings.rs`: + +```rust +impl Rewrite { + #[must_use] + pub fn should_proxy_asset(&self, host: &str) -> bool { + !self.is_excluded_host(host) + } + + #[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)) + } + + fn normalize(&mut self) { /* trim, lowercase, drop "" and "*" with a warning */ } +} +``` + +- `should_proxy_asset`: not excluded. #1231 adds its include clause ("include list empty or `host` included") through `proxy::is_host_permitted`. Only #1231 makes `is_host_permitted` `pub(crate)` and imports it, because this issue doesn't use it and an unused import fails clippy. +- `should_wrap_click`: not excluded. The #1231 include list never applies here. +- The shared matcher is the existing case-insensitive `proxy::is_host_allowed` (`proxy.rs:1242`): an exact host, or `*.example.com` matching `example.com` and any subdomain on a dot boundary. `host` is `Url::host_str()` from the normalized URL. +- `Rewrite::normalize` trims and lowercases `exclude_domains`, and drops empty and bare `"*"` entries with a warning. It runs from `Settings::normalize_deserialized`, right after `self.proxy.normalize()`, so it covers TOML and config-blob loads alike. This mirrors `Proxy::normalize` for `allowed_domains`. +- `Rewrite::is_excluded` and its stale `#[allow(dead_code)]` are removed. + +Call sites in `creative.rs` use two private helpers: + +```rust +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)) + }) +} + +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)) + }) +} +``` + +- `proxy_if_abs`, `rewrite_srcset` and `CssUrlRewriter::rewrite` call `asset_target`. `proxied_attr_value` follows through `proxy_if_abs`. +- The anchor handler calls `click_target`. +- `build_proxy_url` and `build_click_url` keep their `&str` parameter and receive `url.as_str()`. `build_signed_url_for` already re-parses its input (`creative.rs:560`), and re-parsing a serialized `Url` is the identity, so the signed output is unchanged. +- `/first-party/sign` (`proxy.rs:1672-1702`) handles the target in this order: + 1. It keeps its own request-scheme handling for `//` input, building the string into a `let protocol_relative;` binding, and borrows absolute input as-is. + 2. It calls `normalize_creative_url`, binding the result as `target`. `None` becomes `unsupported url`. + 3. It keeps the `missing host` guard. + 4. It calls `should_proxy_asset`. A decline logs `log::debug!("sign request for `{host}` declined by rewrite policy")` and returns `unsupported url`. + 5. It runs the existing `proxy.allowed_domains` check unchanged. + + Malformed input that used to return `invalid url`, and a non-http scheme that used to return `unsupported scheme`, now both return `unsupported url`. All three are `TrustedServerError::Proxy`, which maps to `502`, so client-visible status is unchanged. + +These edge cases change, all in the safe direction: + +- Uppercase `exclude_domains` entries start matching, because the shared matcher is case-insensitive and entries are lowercased at load. +- Empty and bare `"*"` exclude entries, which never matched a host, are dropped with a warning. +- An `http(s)`-prefixed value that `url::Url` cannot parse (for example `https://exa mple.example/x`) is left byte-for-byte untouched. Before, `to_abs` accepted it and `build_signed_url_for` wrote the raw string back re-quoted, so `` became ``, and an anchor also gained `data-tsclick="https://exa mple.example/x"`. Red-phase runs against `main` reproduced both. + +These go in `CHANGELOG.md` under Unreleased › Fixed, in whichever PR lands the shared step. + +### Rewrite pass + +`rewrite_creative_html_impl` takes one more parameter, which stays within the seven-argument limit: + +```rust +/// Which rewrite features one creative pass applies. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct CreativeFeatures { + assets: bool, + clicks: bool, +} + +impl CreativeFeatures { + const ALL: Self = Self { assets: true, clicks: true }; + fn any(self) -> bool { self.assets || self.clicks } +} + +fn rewrite_creative_html_impl( + settings: &Settings, + markup: &str, + base_origin: &str, + inject_tsjs: bool, + max_output_size: usize, + features: CreativeFeatures, +) -> String +``` + +`CreativeFeatures` is its own type because `process_auction_creative_with_rewriter` resolves it once from settings and hands it to the per-path rewriter closure. Two positional `bool`s would be easy to swap. A full options struct for all five per-call arguments was considered and dropped, because it grows the diff without simplifying any caller. + +The handler vector is built before `HtmlRewriter::new`: + +```rust +let mut element_content_handlers = vec![/* removal, TSJS */]; +if features.assets { + element_content_handlers.extend([/* asset handlers, unchanged */]); +} +if features.clicks { + element_content_handlers.push(/* anchor handler */); +} +``` + +- `` removal and the `` TSJS handler are always registered. The pass only runs when at least one switch is on. +- Asset handlers (img, script, link, media, object, embed, input, SVG, iframe, `[style]`, `
AdExcludedMail"#; - const MATRIX_FIXTURE_FRAGMENT: &str = r#"AdExcluded"#; + const MATRIX_FIXTURE_FRAGMENT: &str = r#"
AdExcludedMail"#; const INLINE_ORIGIN: &str = "https://www.example.com"; #[derive(Debug, Clone, Copy)] @@ -1001,6 +1001,8 @@ At the top of `creative.rs` `mod tests`, after the `use super::{...}` block: - [ ] **Step 2: Matrix and unset-pin tests** +`MATRIX_DEFAULT_AUCTION_OUTPUT` and `MATRIX_DEFAULT_INLINE_OUTPUT` are the exact output of `MATRIX_FIXTURE_WITH_BODY` on `main`, before this change. To capture them, add a throwaway test to a scratch worktree of `main` that builds the same settings without `rewrite_clicks` (`create_test_settings()`, `sanitize_creatives = false`, `rewrite_creatives = true`, `exclude_domains = ["excluded.example.com"]`). Have it print `process_auction_creative` and `process_inline_auction_creative(.., "https://www.example.com", ..)` twice each, run it with `--nocapture`, and confirm the two runs match. Then remove the scratch worktree. Signatures are deterministic for the fixed test settings, and the injected script tag carries no hash. + ```rust #[test] fn asset_and_click_switches_combine_on_auction_and_inline_paths() { @@ -1012,8 +1014,7 @@ At the top of `creative.rs` `mod tests`, after the `use super::{...}` block: 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 label = format!("{path:?} assets={assets} clicks={clicks} body={has_body}"); let prefix = match path { CreativePath::Auction => "", CreativePath::Inline => INLINE_ORIGIN, @@ -1024,7 +1025,7 @@ At the top of `creative.rs` `mod tests`, after the `use super::{...}` block: let expected_proxied = match (assets, has_body) { (false, _) => 0, (true, true) => 4, - (true, false) => 1, + (true, false) => 3, }; assert_eq!( out.matches("/first-party/proxy?tsurl=").count(), @@ -1042,12 +1043,9 @@ At the top of `creative.rs` `mod tests`, after the `use super::{...}` block: "{label}: image raw only when assets are off: {out}" ); - // Each wrapped anchor carries the click URL in href and data-tsclick. - let expected_click_urls = match (clicks, has_body) { - (false, _) => 0, - (true, true) => 4, - (true, false) => 2, - }; + // 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, @@ -1072,12 +1070,10 @@ At the top of `creative.rs` `mod tests`, after the `use super::{...}` block: out.contains(r#"href="https://excluded.example.com/page""#), "{label}: excluded anchor always stays raw: {out}" ); - if has_body { - assert!( - out.contains(r#"href="mailto:ads@example.com""#), - "{label}: mailto 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("
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" + ); + } + } ``` - [ ] **Step 3: End-to-end tests** @@ -1215,7 +1232,7 @@ Run each test alone, as described in the environment notes. Expected: - `asset_and_click_switches_combine_on_auction_and_inline_paths` panics with "Auction assets=true clicks=false body=true: click URL count". - `convert_to_openrtb_response_wraps_clicks_without_rewriting_assets` panics with "should wrap the landing link". Nothing was rewritten. - `build_bid_map_keeps_inline_anchors_raw_when_clicks_are_off` panics with "should leave the landing link raw". The anchor was wrapped. -- `unset_rewrite_clicks_preserves_existing_output` **passes**. It pins today's behavior. +- `unset_rewrite_clicks_preserves_existing_output` and `default_settings_rewrite_matrix_fixture_byte_for_byte_as_before` **pass**. They pin today's behavior. - [ ] **Step 5: Add `CreativeFeatures`** @@ -1346,12 +1363,18 @@ Public wrappers: pass `CreativeFeatures::ALL` as the sixth argument in `rewrite_ - `rewrite_inline_creative_html`: "its click guard is unnecessary for click URLs that are already absolute here". - `auction/endpoints.rs` response doc: "Creative HTML is inlined in each bid's `adm` field after optional sanitization ([`auction.sanitize_creatives`][...]). First-party asset rewriting ([`auction.rewrite_creatives`][...]) and click wrapping ([`auction.rewrite_clicks`][...]) are enabled by default. Bidder `` removal and creative TSJS injection run when either is on." This replaces the stale "mandatory server-side sanitization". - `auction/formats.rs`, on `convert_to_openrtb_response`: name `rewrite_clicks` next to `rewrite_creatives` and add the intra-doc link target `/// [`AuctionConfig::rewrite_clicks`]: crate::auction_config_types::AuctionConfig::rewrite_clicks`. Add `clicks {}` to the "Processed creative" debug log, fed by `settings.auction.rewrites_auction_clicks()`. +- `auction/formats.rs`, the comment above `serialize_renderer` in `convert_to_openrtb_response`: "sanitization is opt-in, asset rewriting and click wrapping are on by default, and with all three disabled the creative ships exactly as the bidder returned it." +- `creative.rs` module docs: + - add the goal "Route click-through links through a signed first-party click redirect.", and call the proxied list "asset URLs"; + - add key behaviors for ``/`` wrapping with `data-tsclick`, for `exclude_domains` applying to assets and links, and for `` removal; + - add a "Switches" list. Auction creatives rewrite assets on `rewrite_creatives` and wrap links on `rewrites_auction_clicks`, and an unset `rewrite_clicks` follows `rewrite_creatives`. `` removal and TSJS run when either is on, and with both off the pass is skipped. Proxied HTML always rewrites assets and wraps links unless `rewrite_clicks` is explicitly `false`. The public wrappers rewrite both. + - Link the `AuctionConfig` items with reference definitions at the end of the module docs, because `creative.rs` does not import `AuctionConfig`. - [ ] **Step 9: Verify green** Run: `cargo fmt --all && cargo test-fastly -p trusted-server-core --lib -- creative:: auction::formats publisher::` -Expected: `551 passed; 0 failed`, including every pre-existing rewrite test. +Expected: `554 passed; 0 failed`, including every pre-existing rewrite test. Run: `cargo clippy-fastly` @@ -1380,11 +1403,12 @@ Before `html_response_rewrite_preserves_non_standard_port`: ```rust #[test] fn proxied_html_click_wrapping_follows_explicit_rewrite_clicks() { - let html = r#""#; + 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), @@ -1394,9 +1418,9 @@ Before `html_response_rewrite_preserves_non_standard_port`: let mut settings = create_test_settings(); settings.auction.rewrite_creatives = rewrite_creatives; settings.auction.rewrite_clicks = rewrite_clicks; - let label = format!( - "rewrite_creatives={rewrite_creatives} 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( @@ -1418,16 +1442,36 @@ Before `html_response_rewrite_preserves_non_standard_port`: 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.contains("/first-party/click?tsurl="), - expect_clicks, + body.matches("/first-party/click?tsurl=").count(), + if expect_clicks { 2 } else { 0 }, "{label}: click wrapping in proxied HTML: {body}" ); assert_eq!( - body.contains("data-tsclick"), - expect_clicks, + 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}" @@ -1453,6 +1497,18 @@ In `rewrite_proxied_html`, replace `CreativeFeatures::ALL` with: Append this paragraph to its doc comment: "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`." +Fix the `CreativeHtmlProcessor` doc comment, which names `rewrite_creative_html` although the processor calls `rewrite_proxied_html`: + +```rust +/// 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_proxied_html`] internally. Asset URLs are +/// always proxied; links are wrapped unless `[auction] rewrite_clicks` is +/// explicitly `false`. +``` + - [ ] **Step 4: Verify green** Run: `cargo test-fastly -p trusted-server-core --lib -- proxy::tests::proxied_html proxy::tests::auction_rewrite_setting_does_not_change` @@ -1587,6 +1643,17 @@ rewrite_creatives = true # rewrite_clicks = true ``` +In the `sanitize_creatives` comment just below, replace the sentence that starts "Note that with `rewrite_creatives = true`" so it names both switches: + +```toml +# (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 +``` + - [ ] **Step 4: Commit** ```bash @@ -1625,18 +1692,24 @@ git commit --signoff -S -m "Document rewrite_clicks in the example config and pi - an explicit `rewrite_clicks` applies to links in proxied HTML, and unset keeps wrapping them. - Warning block: - add "any explicit `rewrite_clicks`" to the serialized values; - - rollback note: "Older binaries cannot express clicks off with assets on; rolling back from that configuration re-enables click wrapping."; + - rollback note: "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."; - overlay note: "`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`." - [ ] **Step 2: `creative-processing.md`** - Processing Triggers, item 1, append: "Click-through links are wrapped according to `[auction].rewrite_clicks`, which follows `rewrite_creatives` when unset." - Item 2, append: "An explicitly set `[auction].rewrite_clicks` also applies to links in proxied HTML." +- "Auction Rewrite Control" intro: "Three auction settings control the processing applied to winning-bid `adm` ... 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)." Add `# rewrite_clicks unset: follows rewrite_creatives` to its TOML snippet. +- Its `sanitize_creatives` × `rewrite_creatives` table: asset URLs follow `rewrite_creatives` and links follow `rewrite_clicks`. + - `false`/`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`: "...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`: "Rewrite eligible resource/CSS URLs ... removing any bidder `` element. Links are wrapped unless `rewrite_clicks = false`. Executable markup is preserved." - After the sanitization warning in "Auction Rewrite Control", add an `### Assets and clicks` subsection containing: - the TOML snippet (`rewrite_creatives = true`, `# rewrite_clicks = false`); - the four-row table (assets × clicks → asset URLs, links, ``, TSJS); - "SSAT/page-bids follows the same table with absolute URLs and never injects TSJS. `exclude_domains` applies to both assets and links."; - - the `clickGuard` versus `rewrite_clicks` paragraph. + - the `clickGuard` versus `rewrite_clicks` paragraph; + - a `renderGuard` note: "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." - "Anchors (Click Tracking)": add `**Controlled by**: [auction].rewrite_clicks (follows rewrite_creatives when unset)`, switch the example to `advertiser.example.com`, and show the `data-tsclick` attribute. - [ ] **Step 3: Remaining guides** @@ -1653,7 +1726,13 @@ git commit --signoff -S -m "Document rewrite_clicks in the example config and pi Under `### Added`, as the first entry: ```markdown -- 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 cannot express clicks off with assets on. +- 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`. +``` + +In the existing Unreleased `rewrite_creatives`/`sanitize_creatives` entry, replace "(proxy/click URL conversion, bidder `` removal; creative TSJS injection on `POST /auction` only)." with: + +```markdown +(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. ``` - [ ] **Step 5: Format** @@ -1694,16 +1773,16 @@ Expected: every command exits 0. The validation run's results are in the spec ha - [ ] **Step 2: Map completion criteria to tests** -| Criterion | Test | -| --------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Unset output unchanged | `unset_rewrite_clicks_preserves_existing_output`, existing rewrite tests, unset rows of `proxied_html_click_wrapping_follows_explicit_rewrite_clicks` | -| Four-combination matrix on both paths | `asset_and_click_switches_combine_on_auction_and_inline_paths` | -| Clicks off: raw href, no `data-tsclick` | matrix, `build_bid_map_keeps_inline_anchors_raw_when_clicks_are_off` | -| Clicks on, assets off | matrix, `convert_to_openrtb_response_wraps_clicks_without_rewriting_assets` | -| `exclude_domains` applies to anchors | matrix (excluded anchor), `rewrite_click_urls_excludes_blacklisted_domains`, `exclude_domains_match_case_insensitively_in_the_rewrite_pass` | -| Proxied HTML rule | `proxied_html_click_wrapping_follows_explicit_rewrite_clicks` | -| Default omitted from the blob | `default_rewrite_clicks_is_unset_and_not_serialized`, `legacy_blob_without_rewrite_clicks_follows_rewrite_creatives`, `rewrite_clicks_environment_override_is_ignored_without_leaf` | -| Docs and CHANGELOG | Task 6 | +| Criterion | Test | +| --------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Unset output unchanged | `default_settings_rewrite_matrix_fixture_byte_for_byte_as_before`, `unset_rewrite_clicks_preserves_existing_output`, existing rewrite tests, unset rows of `proxied_html_click_wrapping_follows_explicit_rewrite_clicks` | +| Four-combination matrix on both paths | `asset_and_click_switches_combine_on_auction_and_inline_paths` | +| Clicks off: raw href, no `data-tsclick` | matrix, `build_bid_map_keeps_inline_anchors_raw_when_clicks_are_off` | +| Clicks on, assets off | matrix, `convert_to_openrtb_response_wraps_clicks_without_rewriting_assets` | +| `exclude_domains` applies to anchors | matrix (excluded anchor), `rewrite_click_urls_excludes_blacklisted_domains`, `exclude_domains_match_case_insensitively_in_the_rewrite_pass` | +| Proxied HTML rule | `proxied_html_click_wrapping_follows_explicit_rewrite_clicks` | +| Default omitted from the blob | `default_rewrite_clicks_is_unset_and_not_serialized`, `legacy_blob_without_rewrite_clicks_follows_rewrite_creatives`, `rewrite_clicks_environment_override_is_ignored_without_leaf` | +| Docs and CHANGELOG | Task 6 | - [ ] **Step 3: Hand off** diff --git a/docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md b/docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md index 97bed4dfc..99a1b18ea 100644 --- a/docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md +++ b/docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md @@ -233,15 +233,17 @@ fn click_target(settings: &Settings, raw: &str) -> Option { 4. It calls `should_proxy_asset`. A decline logs `log::debug!("sign request for `{host}` declined by rewrite policy")` and returns `unsupported url`. 5. It runs the existing `proxy.allowed_domains` check unchanged. - Malformed input that used to return `invalid url`, and a non-http scheme that used to return `unsupported scheme`, now both return `unsupported url`. All three are `TrustedServerError::Proxy`, which maps to `502`, so client-visible status is unchanged. + Malformed input that used to return `invalid url`, and a non-http scheme that used to return `unsupported scheme`, now both return `unsupported url`. All three are `TrustedServerError::Proxy`, which maps to `502`, so those statuses are unchanged. + + One client-visible status does change. A host matched only by a mixed-case `exclude_domains` entry used to pass the exclusion check, so sign returned `200`, or `403` when the host was also outside `proxy.allowed_domains`. It now returns `502`, like every other excluded host. These edge cases change, all in the safe direction: -- Uppercase `exclude_domains` entries start matching, because the shared matcher is case-insensitive and entries are lowercased at load. +- Uppercase `exclude_domains` entries start matching, because the shared matcher is case-insensitive and entries are lowercased at load. `/first-party/sign` returns `502` for those hosts, where it used to return `200`, or `403` for hosts outside `proxy.allowed_domains`. - Empty and bare `"*"` exclude entries, which never matched a host, are dropped with a warning. - An `http(s)`-prefixed value that `url::Url` cannot parse (for example `https://exa mple.example/x`) is left byte-for-byte untouched. Before, `to_abs` accepted it and `build_signed_url_for` wrote the raw string back re-quoted, so `` became ``, and an anchor also gained `data-tsclick="https://exa mple.example/x"`. Red-phase runs against `main` reproduced both. -These go in `CHANGELOG.md` under Unreleased › Fixed, in whichever PR lands the shared step. +These go in `CHANGELOG.md` under Unreleased › Fixed, as one entry, in whichever PR lands the shared step. The exact text is in the plan's "Shared step" section. ### Rewrite pass @@ -339,6 +341,7 @@ Both run whenever the pass runs, which is when either switch is on. - `` removal protects root-relative `/first-party/proxy` URLs, root-relative `/first-party/click` URLs and the root-relative `/static/tsjs=` script alike, so it is needed whichever feature is on. On the inline path URLs are absolute, but removal is kept for parity, as `inline_rewrite_strips_base_elements` (`creative.rs`) pins today. - TSJS injection on `/auction` with clicks on delivers the click guard, which is required. +- TSJS injection on `/auction` with only clicks on also gives a creative that sets `tsCreativeConfig.renderGuard` a way to send assets inserted by its own script through `/first-party/sign` and `/first-party/proxy`, although the server left the markup's asset URLs direct. `creative-processing.md` says so. - TSJS injection on `/auction` with only assets on is usually inert. The click guard has no `data-tsclick` anchor to act on, and the render guard defaults to off (`renderGuard: false`). The reason to keep it is that toggling `rewrite_clicks` then changes only anchors and nothing else in the output. Assets-only output is today's default output with the anchors left raw, which keeps the four-combination matrix easy to reason about and test. - The inline path never injects TSJS, as today. - With both switches off, nothing runs: no `` removal, no TSJS, byte-for-byte pass-through. @@ -410,7 +413,7 @@ The shared test set is listed in the plan's "Shared step" section and is identic One table-driven test (`asset_and_click_switches_combine_on_auction_and_inline_paths`) covers both entry points, `process_auction_creative` (`/auction`) and `process_inline_auction_creative` (inline), over `(rewrite_creatives, rewrite_clicks)` in `{(true, Some(true)), (true, Some(false)), (false, Some(true)), (false, Some(false))}`. Each case names its combination and path in every assertion message. -Fixture: a ``, ``, `srcset`, an inline `style` with `url()`, a `
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 7c7e202d1ad06c96bd2b728df2e28870bcf49345 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 15:04:25 +0530 Subject: [PATCH 06/14] 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 6b82d6de78fdb9fc986b5304f4073e311b3207c2 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 15:04:44 +0530 Subject: [PATCH 07/14] 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 3c1012dad635891059bb765b8bd1b945f09b5fc4 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Wed, 7 Oct 2026 15:04:44 +0530 Subject: [PATCH 08/14] 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 f1c0c14b505e1579cd78eab825526db4fdd640a3 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Mon, 12 Oct 2026 17:53:13 +0530 Subject: [PATCH 09/14] Address review feedback on the rewrite_clicks spec and plan Correct the renderGuard caveat for the sandboxed /auction renderer, list the operator docs that become false, keep a bidder while dropping its href, and warn on the public rewrite wrappers. Also fix citations, the shared CHANGELOG Fixed wording, red-step ordering, the Step 5 grep, rollout and staged rollback notes, and other precision items. Signed-off-by: dhruv8sh --- ...0-07-1234-creative-click-rewrite-switch.md | 93 ++++++++++++++++--- ...34-creative-click-rewrite-switch-design.md | 60 +++++++----- 2 files changed, 117 insertions(+), 36 deletions(-) diff --git a/docs/superpowers/plans/2026-10-07-1234-creative-click-rewrite-switch.md b/docs/superpowers/plans/2026-10-07-1234-creative-click-rewrite-switch.md index ebbb2ff11..674abf296 100644 --- a/docs/superpowers/plans/2026-10-07-1234-creative-click-rewrite-switch.md +++ b/docs/superpowers/plans/2026-10-07-1234-creative-click-rewrite-switch.md @@ -9,7 +9,7 @@ - Replace `to_abs` with a pure URL normalizer plus host policy on `Rewrite`. This "shared step" is written identically in #1231's PR. - Add an `Option` field on `AuctionConfig` with two resolution methods. - Give the lol_html rewrite pass a `CreativeFeatures { assets, clicks }` parameter, and register the asset and anchor handlers only when their feature is on. -- `` removal and TSJS injection run whenever the pass runs. +- `` handling and TSJS injection run whenever the pass runs. The pass drops the bidder base URL (`href`) and keeps `target`; a `` with no `target` is removed as today. **Tech Stack:** Rust 2024 (`trusted-server-core`, `trusted-server-cli`), `lol_html`, `url`, `serde`, EdgeZero typed app-config loader, Viceroy for `wasm32-wasip1` tests. @@ -19,7 +19,7 @@ ## Global Constraints -- With `rewrite_clicks` unset, rewritten output must be byte-identical to today for both values of `rewrite_creatives` on `/auction` and inline SSAT/page-bids, and unchanged for `/first-party/proxy` HTML. +- With `rewrite_clicks` unset, rewritten output must be byte-identical to today for both values of `rewrite_creatives` on `/auction` and inline SSAT/page-bids, and unchanged for `/first-party/proxy` HTML, except for the shared step's CHANGELOG Fixed items. - `rewrite_clicks` is `Option` with `#[serde(default, skip_serializing_if = "Option::is_none")]`. `AuctionConfig` keeps `deny_unknown_fields`. - Unset resolution: auction paths use `rewrite_clicks.unwrap_or(rewrite_creatives)`, and proxied HTML uses `rewrite_clicks.unwrap_or(true)`. - `exclude_domains` applies to anchors. The #1231 include list never does. @@ -32,11 +32,11 @@ - no local `use` inside functions; - every public item documented; - at most 7 function arguments. -- Commits use sentence case, imperative mood, and `git commit --signoff -S` with multiple `-m` flags. No prefixes, no attribution trailers, never a heredoc. +- Commits follow `CONTRIBUTING.md`: sentence case, imperative mood, subject near 50 characters, wrapped body. No prefixes, no attribution trailers. ## Environment notes (learned during validation) -- `cargo test-fastly` runs the wasm test binary through Viceroy. If `viceroy` is installed under `~/.cargo/bin` but not on `PATH`, prefix commands with `PATH=$HOME/.cargo/bin:$PATH`. Validation used Viceroy 0.21.1; `.tool-versions` pins 0.17.0. +- `cargo test-fastly` runs the wasm test binary through Viceroy. If `viceroy` is installed under `~/.cargo/bin` but not on `PATH`, prefix commands with `PATH=$HOME/.cargo/bin:$PATH`. Validation used Viceroy 0.17.0, as pinned in `.tool-versions`. - Under Viceroy, one panicking test aborts the whole wasm test binary (exit 134) and hides the assertion message. To watch a test fail, run it alone: ```bash @@ -89,7 +89,7 @@ impl Rewrite { /// `/first-party/proxy`. /// /// The host must not match [`Self::exclude_domains`]. Matching is - /// case-insensitive; see [`is_host_allowed`] for the pattern rules. + /// case-insensitive; see `crate::proxy::is_host_allowed` for the pattern rules. #[must_use] pub fn should_proxy_asset(&self, host: &str) -> bool { !self.is_excluded_host(host) @@ -287,7 +287,7 @@ The `creative.rs` test module's `use super::{...}` list gains `normalize_creativ CHANGELOG: the shared step adds this entry as the first item under `## [Unreleased]` › `### Fixed` (create the heading if absent), in the same commit as the code: ```markdown -- `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, and `/first-party/sign` now rejects them with `502`; it used to sign them, or return `403` when the host was also outside `proxy.allowed_domains`. Empty and bare `"*"` `exclude_domains` entries, which never matched a host, are dropped at load with a warning. Absolute `http(s)` creative URLs that cannot be parsed (for example, a host containing a space) are now left untouched; previously the attribute was re-quoted and a link also gained `data-tsclick`. Audit `exclude_domains` for mixed-case entries before upgrading. +- `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, and `/first-party/sign` now rejects them with `502`; it used to sign them, or return `403` when the host was also outside `proxy.allowed_domains`. Empty and bare `"*"` `exclude_domains` entries, which never matched a host, are dropped at load with a warning. Absolute `http(s)` creative URLs that cannot be parsed (for example, a host containing a space) are now left untouched; previously the trimmed value was written back re-quoted (with `https:` prepended to `//` input, so `src=" //exa mple.example/x "` became `https://exa mple.example/x`), an unparseable quoted URL in `
AdExcludedMail"#; const MATRIX_DEFAULT_INLINE_OUTPUT: &str = r#"
AdExcludedMail"#; + #[test] + fn base_target_is_kept_when_the_base_url_is_dropped() { + let fixture = r#"Ad"#; + for path in [CreativePath::Auction, CreativePath::Inline] { + 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}"); + + let out = process_for_path(&settings, path, fixture); + + if assets || clicks { + assert!( + out.contains(r#""#), + "{label}: should keep base target without href: {out}" + ); + assert!( + !out.contains("base.example.com"), + "{label}: should drop the bidder base URL: {out}" + ); + } else { + assert_eq!( + out, fixture, + "{label}: should pass through byte for byte with both switches off" + ); + } + assert!( + out.contains(r#""#), + "{label}: relative image should stay raw: {out}" + ); + assert!( + out.contains(r#""#), + "{label}: relative anchor should stay raw: {out}" + ); + } + } + } + #[test] fn default_settings_rewrite_matrix_fixture_byte_for_byte_as_before() { let settings = matrix_settings(true, None); @@ -1232,6 +1271,7 @@ Run each test alone, as described in the environment notes. Expected: - `asset_and_click_switches_combine_on_auction_and_inline_paths` panics with "Auction assets=true clicks=false body=true: click URL count". - `convert_to_openrtb_response_wraps_clicks_without_rewriting_assets` panics with "should wrap the landing link". Nothing was rewritten. - `build_bid_map_keeps_inline_anchors_raw_when_clicks_are_off` panics with "should leave the landing link raw". The anchor was wrapped. +- `base_target_is_kept_when_the_base_url_is_dropped` panics with "Auction assets=true clicks=true: should keep base target without href". The whole element was removed. - `unset_rewrite_clicks_preserves_existing_output` and `default_settings_rewrite_matrix_fixture_byte_for_byte_as_before` **pass**. They pin today's behavior. - [ ] **Step 5: Add `CreativeFeatures`** @@ -1270,7 +1310,7 @@ Inside `rewrite_creative_html_impl`, after `let overflowed = ...;`, build the ve ```rust let mut element_content_handlers = vec![ - // (existing `` removal handler with its comment) + // (`` handler, changed as shown below) // (existing `` TSJS handler) ]; if features.assets { @@ -1307,6 +1347,25 @@ Inside `rewrite_creative_html_impl`, after `let overflowed = ...;`, build the ve `extend([...])` with an array of `element!`/`text!` invocations type-checks with no annotations, because every invocation yields the same tuple type. The body-less TSJS fallback keeps `if inject_tsjs && !injected_ts_creative.get() && !rewritten.is_empty()`. +Replace the `` handler so it keeps `target`: + +```rust + // Drop a bidder-supplied base URL: it would rebase the root-relative + // `/first-party/…` and `/static/tsjs=…` URLs this pass emits onto an + // attacker-chosen origin, hijacking proxy/click mediation and leaking + // signed URL data. Keep `target` so raw links still open where the + // bidder intended, and remove a `` left without one. The + // sanitizer also strips , but rewriting must not depend on + // sanitization, which is independently optional. + element!("base", |el| { + el.remove_attribute("href"); + if !el.has_attribute("target") { + el.remove(); + } + Ok(()) + }), +``` + - [ ] **Step 7: Wire the callers** ```rust @@ -1360,7 +1419,9 @@ Public wrappers: pass `CreativeFeatures::ALL` as the sixth argument in `rewrite_ Add: "Rewrites assets and clicks unconditionally; the auction settings are applied by the auction processing entry points, not here." -- `rewrite_inline_creative_html`: "its click guard is unnecessary for click URLs that are already absolute here". + Add to both `rewrite_creative_html` and `rewrite_inline_creative_html` a `# Warning` section: "This wrapper applies neither `rewrite_creatives` nor `rewrite_clicks`. Production code must go through the auction processing entry points, which honor both switches." They stay `pub` because `pub(crate)` would trip `dead_code` outside tests. + +- `rewrite_inline_creative_html`: replace "its only job is to safeguard click URLs, which are already absolute here" with "its click guard is unnecessary for click URLs that are already absolute here". - `auction/endpoints.rs` response doc: "Creative HTML is inlined in each bid's `adm` field after optional sanitization ([`auction.sanitize_creatives`][...]). First-party asset rewriting ([`auction.rewrite_creatives`][...]) and click wrapping ([`auction.rewrite_clicks`][...]) are enabled by default. Bidder `` removal and creative TSJS injection run when either is on." This replaces the stale "mandatory server-side sanitization". - `auction/formats.rs`, on `convert_to_openrtb_response`: name `rewrite_clicks` next to `rewrite_creatives` and add the intra-doc link target `/// [`AuctionConfig::rewrite_clicks`]: crate::auction_config_types::AuctionConfig::rewrite_clicks`. Add `clicks {}` to the "Processed creative" debug log, fed by `settings.auction.rewrites_auction_clicks()`. - `auction/formats.rs`, the comment above `serialize_renderer` in `convert_to_openrtb_response`: "sanitization is opt-in, asset rewriting and click wrapping are on by default, and with all three disabled the creative ships exactly as the bidder returned it." @@ -1693,6 +1754,7 @@ git commit --signoff -S -m "Document rewrite_clicks in the example config and pi - Warning block: - add "any explicit `rewrite_clicks`" to the serialized values; - rollback note: "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."; + - processing paragraph's last sentence (`configuration.md:1956`): replace "Neither setting affects HTML or CSS fetched through `/first-party/proxy`" with "`rewrite_creatives` and `sanitize_creatives` do not affect HTML or CSS fetched through `/first-party/proxy`; an explicit `rewrite_clicks` does apply to links in proxied HTML." - overlay note: "`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`." - [ ] **Step 2: `creative-processing.md`** @@ -1709,7 +1771,8 @@ git commit --signoff -S -m "Document rewrite_clicks in the example config and pi - the four-row table (assets × clicks → asset URLs, links, ``, TSJS); - "SSAT/page-bids follows the same table with absolute URLs and never injects TSJS. `exclude_domains` applies to both assets and links."; - the `clickGuard` versus `rewrite_clicks` paragraph; - - a `renderGuard` note: "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." + - a `renderGuard` note: "With `rewrite_creatives = false` and `rewrite_clicks = true`, `POST /auction` still injects TSJS, but this does not reopen asset proxying in practice: `/auction` creatives render in a sandbox without `allow-same-origin`, so the render guard skips `/first-party/sign` and script-inserted assets load direct. `/first-party/sign` itself is not gated by `rewrite_creatives`." + - a `` note: "Whenever either switch is on, a bidder `` loses its `href` and keeps `target`; a `` without `target` is removed. With `rewrite_creatives = false` and `rewrite_clicks = true`, relative asset URLs therefore no longer resolve against the bidder base, so bidders should use absolute asset URLs." - "Anchors (Click Tracking)": add `**Controlled by**: [auction].rewrite_clicks (follows rewrite_creatives when unset)`, switch the example to `advertiser.example.com`, and show the `data-tsclick` attribute. - [ ] **Step 3: Remaining guides** @@ -1717,7 +1780,11 @@ git commit --signoff -S -m "Document rewrite_clicks in the example config and pi - `auction-orchestration.md`: - mermaid note: `rewrite_creatives=false: asset URLs direct; clicks follow rewrite_clicks`; - flow: `├─[rewrite_creatives or rewrite_clicks] Rewrite assets and/or links, inject creative TSJS`; - - Creative Processing paragraph: link to `/guide/creative-processing#assets-and-clicks`. + - Creative Processing paragraph: link to `/guide/creative-processing#assets-and-clicks`; + - the full example TOML (line 704): add `# rewrite_clicks = true` under `rewrite_creatives`; + - the environment-override section (lines 837-844): list `TRUSTED_SERVER__AUCTION__REWRITE_CLICKS` next to `REWRITE_CREATIVES` and repeat that the leaf must exist in the TOML first; + - the rollback note (line 853): name both switches and point to the configuration guide's rollback order. +- `configuration.md:2171`: replace "`rewrite_creatives = false` skips first-party URL rewriting and creative TSJS injection" with "`rewrite_creatives = false` leaves asset URLs direct; links still follow `rewrite_clicks`, and creative TSJS is still injected when `rewrite_clicks = true`." - `api-reference.md`: list `[auction].rewrite_clicks` with the other two settings. - `auction/README.md`: add the `rewrite_clicks` bullet. @@ -1726,7 +1793,7 @@ git commit --signoff -S -m "Document rewrite_clicks in the example config and pi Under `### Added`, as the first entry: ```markdown -- 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 `[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 `` handling and creative TSJS injection run when either setting is on. A bidder `` is now kept with its `href` dropped, instead of removing the whole element, so raw links keep opening where the bidder intended. 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`. ``` In the existing Unreleased `rewrite_creatives`/`sanitize_creatives` entry, replace "(proxy/click URL conversion, bidder `` removal; creative TSJS injection on `POST /auction` only)." with: diff --git a/docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md b/docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md index 99a1b18ea..2b43694c5 100644 --- a/docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md +++ b/docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md @@ -23,7 +23,7 @@ As a result an operator cannot: - Add a switch that turns click wrapping on or off without affecting asset rewriting. - Narrow `rewrite_creatives` to asset rewriting. Keep its name so existing configs still parse. -- Leave rewritten output unchanged for every existing config. In particular, upgrading must not turn click wrapping on for operators who have `rewrite_creatives = false`. +- Leave rewritten output unchanged for every existing config, except for the shared step's listed Fixed items (case-insensitive `exclude_domains`, unparseable URLs left untouched) and the `` fix below. In particular, upgrading must not turn click wrapping on for operators who have `rewrite_creatives = false`. - Keep `[rewrite] exclude_domains` applying to anchors. - Define the `to_abs` split that this issue and #1231 share, so either implementation can land first. - Keep the config blob compatible with older binaries while the new setting is unset. @@ -35,7 +35,7 @@ As a result an operator cannot: - Adding a click guard to the inline SSAT/page-bids path. - Mapping any server setting onto the client `TsCreativeConfig` flags. - Changing asset rewriting inside HTML or CSS fetched through `/first-party/proxy`. That path keeps proxying assets regardless of `rewrite_creatives`. -- Domain checks at `/first-party/click` and `/first-party/proxy-rebuild`. Those endpoints redirect any validly signed target by design (`proxy.rs:1896-1904`). +- Domain checks at `/first-party/click` and `/first-party/proxy-rebuild`. Those endpoints redirect any validly signed target by design (`proxy.rs:1898-1906`). ## Current behavior @@ -110,7 +110,9 @@ Add one field to `AuctionConfig`, next to `rewrite_creatives`: /// Unset keeps each path's existing behavior: auction creatives follow /// [`Self::rewrite_creatives`], and HTML fetched through `/first-party/proxy` /// keeps wrapping. An explicit value applies to every path. Unset is omitted -/// from serialized config blobs, so older binaries keep loading them. +/// from serialized config blobs, so older binaries keep loading them; any +/// explicit value is serialized and rejected by binaries that predate this +/// field. #[serde(default, skip_serializing_if = "Option::is_none")] pub rewrite_clicks: Option, ``` @@ -119,7 +121,10 @@ pub rewrite_clicks: Option, ```rust impl AuctionConfig { - /// Whether auction creatives (`/auction` and inline SSAT) wrap clicks. + /// Whether auction creatives (`POST /auction` and inline SSAT/page-bids) + /// wrap click-through links. + /// + /// Unset [`Self::rewrite_clicks`] follows [`Self::rewrite_creatives`]. #[must_use] pub fn rewrites_auction_clicks(&self) -> bool { self.rewrite_clicks.unwrap_or(self.rewrite_creatives) @@ -173,7 +178,7 @@ pub(super) fn normalize_creative_url(url: &str) -> Option { } ``` -It trims the input, maps a protocol-relative `//host/...` to `https://host/...`, and accepts only a case-insensitive `http://` or `https://` prefix. It parses absolute input in place without allocating, and returns `None` for empty, relative, non-http(s) or unparseable input. It needs no separate hostless check, because `url` rejects an `http(s)` URL without a host. It makes no policy decision and reads no settings. +It trims the input, maps a protocol-relative `//host/...` to `https://host/...`, and accepts only a case-insensitive `http://` or `https://` prefix. It parses absolute input without an intermediate `format!`, and returns `None` for empty, relative, non-http(s) or unparseable input. It needs no separate hostless check, because `url` rejects an `http(s)` URL without a host. It makes no policy decision and reads no settings. Policy lives on `Rewrite` in `settings.rs`: @@ -226,7 +231,7 @@ fn click_target(settings: &Settings, raw: &str) -> Option { - `proxy_if_abs`, `rewrite_srcset` and `CssUrlRewriter::rewrite` call `asset_target`. `proxied_attr_value` follows through `proxy_if_abs`. - The anchor handler calls `click_target`. - `build_proxy_url` and `build_click_url` keep their `&str` parameter and receive `url.as_str()`. `build_signed_url_for` already re-parses its input (`creative.rs:560`), and re-parsing a serialized `Url` is the identity, so the signed output is unchanged. -- `/first-party/sign` (`proxy.rs:1672-1702`) handles the target in this order: +- `/first-party/sign` (`proxy.rs:1672-1703`) handles the target in this order: 1. It keeps its own request-scheme handling for `//` input, building the string into a `let protocol_relative;` binding, and borrows absolute input as-is. 2. It calls `normalize_creative_url`, binding the result as `target`. `None` becomes `unsupported url`. 3. It keeps the `missing host` guard. @@ -302,14 +307,14 @@ Callers: | `rewrite_proxied_html` (`CreativeHtmlProcessor`) | `true` | `auction.rewrites_proxied_clicks()` | `true` | | `rewrite_creative_html`, `rewrite_inline_creative_html` (public wrappers) | `true` | `true` | as today | -`process_auction_creative_with_rewriter` replaces the `rewrite_creatives` check with `features.any()`. When both features are off it returns the sanitized markup unchanged, exactly as today. The `process_*` functions now call `rewrite_creative_html_impl` directly, so the public wrappers keep full-rewrite semantics and existing callers, such as `proxy.rs:3543`, are unaffected. +`process_auction_creative_with_rewriter` replaces the `rewrite_creatives` check with `features.any()`. When both features are off it returns the sanitized markup unchanged, exactly as today. The `process_*` functions now call `rewrite_creative_html_impl` directly, so the public wrappers keep full-rewrite semantics and existing test callers, such as `proxy.rs:3543` (inside `#[cfg(test)]`), are unaffected. The wrappers have no production callers. Making them `pub(crate)` would trip `dead_code` outside tests, so each wrapper's doc comment instead warns that it applies neither `rewrite_creatives` nor `rewrite_clicks` and that production callers must go through the `process_*` entry points. `CreativeCssProcessor` contains no anchors and is unchanged. ### Endpoints and client - `/first-party/click` and `/first-party/proxy-rebuild` stay routed and ungated. -- No JavaScript changes. With clicks off no anchor carries `data-tsclick`, so the click guard installs but does nothing. +- No JavaScript changes. With clicks off no anchor carries `data-tsclick`, so the click guard still installs its listeners and attribute observer (`click.ts:484-512`) but never acts. - No change to `TsCreativeConfig`. ## Open decisions resolved @@ -339,9 +344,11 @@ The rule for operators is short: unset means "as before" on every path, and a se Both run whenever the pass runs, which is when either switch is on. -- `` removal protects root-relative `/first-party/proxy` URLs, root-relative `/first-party/click` URLs and the root-relative `/static/tsjs=` script alike, so it is needed whichever feature is on. On the inline path URLs are absolute, but removal is kept for parity, as `inline_rewrite_strips_base_elements` (`creative.rs`) pins today. +- Dropping the bidder base URL protects root-relative `/first-party/proxy` URLs, root-relative `/first-party/click` URLs and the root-relative `/static/tsjs=` script alike, so it is needed whichever feature is on. Leaving `` alone in clicks-only mode is not safe: `/auction` click URLs are root-relative, and a bidder `href` would rebase them onto the bidder origin. On the inline path URLs are absolute, but the handling is kept for parity, as `inline_rewrite_strips_base_elements` (`creative.rs`) pins today. +- `` handling keeps `target` and drops only the base URL: the pass removes the `href` attribute and removes the element only when no `target` remains. Today the whole element is removed, which also drops ``, common in display creatives, so raw (unwrapped) links would open inside the iframe. `` without `target` is still removed outright, so default output is unchanged; only a `` that carries `target` changes, to ``. The sanitizer still removes `` entirely when `sanitize_creatives = true`. +- Side effect in clicks-only mode (`rewrite_creatives = false`, `rewrite_clicks = true`): relative asset URLs that resolved against the bidder base now resolve against the document's own base. This matches today's default mode, where the base is removed too. Bidders that need assets to load in that mode must use absolute URLs. `creative-processing.md` says so. - TSJS injection on `/auction` with clicks on delivers the click guard, which is required. -- TSJS injection on `/auction` with only clicks on also gives a creative that sets `tsCreativeConfig.renderGuard` a way to send assets inserted by its own script through `/first-party/sign` and `/first-party/proxy`, although the server left the markup's asset URLs direct. `creative-processing.md` says so. +- TSJS injection on `/auction` with only clicks on does not reopen asset proxying in practice: `/auction` creatives render in a sandbox without `allow-same-origin` (`render.ts:14-29`), so the render guard's `signProxyUrl` returns early on an opaque origin (`proxy_sign.ts:40`) and script-inserted assets load direct. Independently, `/first-party/sign` checks no auction setting, so `rewrite_creatives = false` has never gated it; `creative-processing.md` says so. - TSJS injection on `/auction` with only assets on is usually inert. The click guard has no `data-tsclick` anchor to act on, and the render guard defaults to off (`renderGuard: false`). The reason to keep it is that toggling `rewrite_clicks` then changes only anchors and nothing else in the output. Assets-only output is today's default output with the anchors left raw, which keeps the four-combination matrix easy to reason about and test. - The inline path never injects TSJS, as today. - With both switches off, nothing runs: no `` removal, no TSJS, byte-for-byte pass-through. @@ -354,7 +361,7 @@ They stay independent, and no server setting maps to `clickGuard`. - `rewrite_clicks` decides whether the server emits signed click URLs and `data-tsclick`. - `clickGuard` decides whether the client repairs those URLs when creative script mutates them. It is a client-side opt-out that a creative or publisher can set through `tsCreativeConfig`. -- With `rewrite_clicks` off there is nothing to guard. The guard installs and stays inert (`click.ts:433-434`, `click.ts:467`), so plumbing the server switch into the client would add config surface with no behavior change. +- With `rewrite_clicks` off there is nothing to guard. The guard still installs its click listeners and attribute observer (`click.ts:484-512`) but never acts, because no anchor carries `data-tsclick` (`click.ts:433-434`, `click.ts:467`), so plumbing the server switch into the client would add config surface with no behavior change. - With `rewrite_clicks` on and `clickGuard = false`, signed click URLs still redirect, but mutated `href` values are not re-signed. That stays a client choice. - `creative-processing.md` documents both, and how they interact. @@ -369,7 +376,7 @@ Both issues replace `to_abs` with the normalizer and policy split above. - the shared tests; - the CHANGELOG Fixed entry. - Shared tests build `Rewrite::default()` and `.extend`/`.push` its lists, never a `Rewrite { .. }` literal, so they keep compiling when #1231 adds a field. Extending a list rather than assigning the whole field also avoids `clippy::field_reassign_with_default`, so no test needs an `#[allow]`. -- The canonical shared code and tests are #1231's text plus four tests from this issue (`unparseable_absolute_click_url_is_left_byte_identical`, `exclude_domains_match_case_insensitively_in_the_rewrite_pass`, `proxy_sign_rejects_excluded_urls_case_insensitively`, `settings_load_normalizes_rewrite_exclude_domains`). Both plans' "Shared step" sections are identical. +- The canonical shared code and tests are the text in both plans' identical "Shared step" sections, including the four tests this issue contributed (`unparseable_absolute_click_url_is_left_byte_identical`, `exclude_domains_match_case_insensitively_in_the_rewrite_pass`, `proxy_sign_rejects_excluded_urls_case_insensitively`, `settings_load_normalizes_rewrite_exclude_domains`), which #1231's plan already carries. - The second PR rebases onto it and adds only its own piece. #1231 adds the include list to `should_proxy_asset`. This issue adds `rewrite_clicks` and gates the anchor handler. - Neither PR changes the other's semantics. `should_wrap_click` never consults the include list, and `should_proxy_asset` never consults `rewrite_clicks`. - Both features add a field to a `deny_unknown_fields` struct with its default skipped, so blob compatibility does not depend on landing order. @@ -415,12 +422,12 @@ One table-driven test (`asset_and_click_switches_combine_on_auction_and_inline_p Fixture: a ``, ``, `srcset`, an inline `style` with `url()`, a `
AdExcludedMail"#; const MATRIX_DEFAULT_INLINE_OUTPUT: &str = r#"
AdExcludedMail"#; + #[test] + fn base_target_is_kept_when_the_base_url_is_dropped() { + let fixture = r#"Ad"#; + for path in [CreativePath::Auction, CreativePath::Inline] { + 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}"); + + let out = process_for_path(&settings, path, fixture); + + if assets || clicks { + assert!( + out.contains(r#""#), + "{label}: should keep base target without href: {out}" + ); + assert!( + !out.contains("base.example.com"), + "{label}: should drop the bidder base URL: {out}" + ); + } else { + assert_eq!( + out, fixture, + "{label}: should pass through byte for byte with both switches off" + ); + } + assert!( + out.contains(r#""#), + "{label}: relative image should stay raw: {out}" + ); + assert!( + out.contains(r#""#), + "{label}: relative anchor should stay raw: {out}" + ); + } + } + } + #[test] fn default_settings_rewrite_matrix_fixture_byte_for_byte_as_before() { let settings = matrix_settings(true, None); From be1ae725104c7bb2919c15649f8a9ffd740ae5ea Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Mon, 12 Oct 2026 17:56:13 +0530 Subject: [PATCH 13/14] Correct operator docs for rewrite_clicks Name rewrite_clicks in the orchestration example, environment overrides and rollback note, fix the rewrite_creatives = false summary, correct the renderGuard caveat for the sandboxed renderer, and document base target handling and the inert click guard. Signed-off-by: dhruv8sh --- docs/guide/auction-orchestration.md | 13 +++++++++---- docs/guide/configuration.md | 9 +++++---- docs/guide/creative-processing.md | 30 +++++++++++++++++------------ 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/docs/guide/auction-orchestration.md b/docs/guide/auction-orchestration.md index 46347c80e..74376dd6e 100644 --- a/docs/guide/auction-orchestration.md +++ b/docs/guide/auction-orchestration.md @@ -705,6 +705,7 @@ Each proxied URL includes a `tstoken` HMAC signature for tamper protection. See enabled = true sanitize_creatives = false # Opt-in; blanks script-based creatives when enabled rewrite_creatives = true +# rewrite_clicks = true # Unset follows rewrite_creatives timeout_ms = 2000 mediator = "adserver_mock" @@ -838,7 +839,9 @@ The typed `ts config validate`, `ts config diff`, and `ts config push` flows can override existing scalar leaves. The pinned EdgeZero loader does not create missing leaves or replace arrays, tables, maps, or rules. Existing configs must add `rewrite_creatives = true` and `sanitize_creatives = false` before relying on -those scalar overrides. Edit and re-push TOML for other values. Provider map +those scalar overrides. `rewrite_clicks` is unset by default; add +`rewrite_clicks = true` or `false` under `[auction]` before relying on +`TRUSTED_SERVER__AUCTION__REWRITE_CLICKS`. Edit and re-push TOML for other values. Provider map keys preserve hyphens, so `pbs-main` uses the `PBS-MAIN` segment and needs `env` shell syntax: @@ -853,9 +856,11 @@ env 'TRUSTED_SERVER__AUCTION__ENABLED=true' \ ``` Before rolling back to a binary that does not know a creative-processing field, -remove that field's non-default value (`rewrite_creatives = false` or -`sanitize_creatives = true`), push the default-compatible blob, and then roll -back. See [Configuration](/guide/configuration#auction-configuration) for the +remove that field's non-default value (`rewrite_creatives = false`, +`sanitize_creatives = true`, or any explicit `rewrite_clicks`), push the default-compatible blob, and then roll +back. Older binaries tie click wrapping to `rewrite_creatives`, so rolling back +also changes link wrapping when `rewrite_clicks` was set. See +[Configuration](/guide/configuration#auction-configuration) for the complete migration, upgrade-sequencing, and rollback guidance. ## Floor Prices diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 908239d2b..bb153df68 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -1948,8 +1948,8 @@ 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 +configs keep their current behavior. Whenever either setting is on, a bidder-supplied `` element +loses its `href`; it keeps a `target` and is removed when it has none. 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 @@ -2181,8 +2181,9 @@ finishes local decision and delivery. An auction can therefore exceed its configured wall-clock timeout. Creative sanitization is opt-in. `sanitize_creatives = true` strips executable -markup before delivery. `rewrite_creatives = false` skips first-party URL -rewriting and creative TSJS injection. See +markup before delivery. `rewrite_creatives = false` leaves asset URLs +direct; links still follow `rewrite_clicks`, and creative TSJS is still +injected when `rewrite_clicks = true`. See [Creative Processing](/guide/creative-processing#auction-rewrite-control). **Environment overrides** replace map leaves that already exist in TOML: diff --git a/docs/guide/creative-processing.md b/docs/guide/creative-processing.md index 823ae1260..0c9193ee9 100644 --- a/docs/guide/creative-processing.md +++ b/docs/guide/creative-processing.md @@ -77,7 +77,7 @@ rejected and its `adm` is dropped. | -------------------- | ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `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. | +| `false` | `true` (default) | Rewrite eligible resource/CSS URLs in the raw bidder markup to signed first-party endpoints, dropping any bidder `` URL. 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 @@ -104,12 +104,12 @@ rewrite_creatives = true # 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 | +| 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` | `href` dropped | injected | +| `true` | `false` | `/first-party/proxy` | direct | `href` dropped | injected | +| `false` | `true` | direct | `/first-party/click` + `data-tsclick` | `href` dropped | 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. @@ -117,13 +117,19 @@ SSAT/page-bids follows the same table with absolute URLs and never injects TSJS. 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. +parameters. With `rewrite_clicks` off the guard still installs its listeners but never +acts, because no link carries `data-tsclick`. 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. +still injects TSJS, but this does not reopen asset proxying in practice: +`/auction` creatives render in a sandbox without `allow-same-origin`, so the +render guard skips `/first-party/sign` and script-inserted assets load direct. +`/first-party/sign` itself is not gated by `rewrite_creatives`. + +Whenever either switch is on, a bidder `` loses its `href` and keeps +`target`; a `` without `target` is removed. With `rewrite_creatives = +false` and `rewrite_clicks = true`, relative asset URLs therefore no longer +resolve against the bidder base, so bidders should use absolute asset URLs. ::: info Runtime protections inside the sandboxed creative iframe Creatives rendered by Trusted Server's own path run in a sandboxed iframe From 7c5eca0dab7c6c87d0a0dfcd49e5d7d8239f4477 Mon Sep 17 00:00:00 2001 From: dhruv8sh Date: Mon, 12 Oct 2026 17:56:13 +0530 Subject: [PATCH 14/14] Expand the CHANGELOG notes for the shared step List the CSS re-quoting, https: prefixing and sign error text changes in the Fixed entry, and note that a bidder base target is now kept. Signed-off-by: dhruv8sh --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce3904fea..6c6ab874a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,7 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- `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, and `/first-party/sign` now rejects them with `502`; it used to sign them, or return `403` when the host was also outside `proxy.allowed_domains`. Empty and bare `"*"` `exclude_domains` entries, which never matched a host, are dropped at load with a warning. Absolute `http(s)` creative URLs that cannot be parsed (for example, a host containing a space) are now left untouched; previously the attribute was re-quoted and a link also gained `data-tsclick`. Audit `exclude_domains` for mixed-case entries before upgrading. +- `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, and `/first-party/sign` now rejects them with `502`; it used to sign them, or return `403` when the host was also outside `proxy.allowed_domains`. Empty and bare `"*"` `exclude_domains` entries, which never matched a host, are dropped at load with a warning. Absolute `http(s)` creative URLs that cannot be parsed (for example, a host containing a space) are now left untouched; previously the trimmed value was written back re-quoted (with `https:` prepended to `//` input, so `src=" //exa mple.example/x "` became `https://exa mple.example/x`), an unparseable quoted URL in `