diff --git a/CHANGELOG.md b/CHANGELOG.md index c7a0eb1ec..d1bec9248 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Breaking** — `bid_param_zone_overrides` inner values must now be JSON objects; previously non-object or empty values (`"header" = "x"`, `"header" = {}`) were accepted and silently produced a dead rule at runtime. They now fail at startup with a configuration error. Operators upgrading should audit their `bid_param_zone_overrides` config for non-object zone entries. - **Breaking** — Integration configuration strings are no longer globally reinterpreted as JSON scalars. Operators upgrading should audit `[integrations.*]` settings and use native TOML/typed-config booleans and numbers (for example, `enabled = true`, not `enabled = "true"`); quoted numeric and boolean scalars now fail validation instead of silently converting. - **Breaking** — Sourcepoint browser module inclusion now requires explicit `[integrations.sourcepoint].enabled = true`; operators relying on the previous unconditional Sourcepoint module should enable the integration before upgrading. -- **Breaking** — Auction creative sanitization is now opt-in: the new `[auction].sanitize_creatives` defaults to `false` because unconditional sanitization blanked script-based creatives (the majority of programmatic display) while recording normal impressions. `[auction].rewrite_creatives` keeps its `true` default. The per-creative cap is now enforced on rewritten output as well as raw input and in every processing mode (1 MiB for auction `adm`; proxied HTML documents keep the proxy's own 10 MiB bound), rewriting fails closed on parser errors instead of emitting partial output and never turns a rejected creative into a runtime-only `adm`, and `hb_cache_host`/`hb_cache_path` are emitted only for bids that supplied no creative — any bid carrying its own `adm` ships without them, so a processed or rejected creative can never be re-fetched raw from PBS Cache. Creative markup with no `` token now receives the click-guard runtime, and bidder `` elements are stripped whenever rewriting is enabled. The creative iframe sandbox no longer grants `allow-same-origin`, restoring origin isolation; rewritten-click recovery from the resulting opaque-origin iframe uses the GET `/first-party/proxy-rebuild` navigation fallback, now registered in every adapter and documented alongside the POST JSON form. Inside those iframes, dynamic resource signing and CORS-mode subresources (ES modules, `crossorigin` fonts) are unavailable pending the constrained asset capability in [#982](https://github.com/IABTechLab/trusted-server/issues/982); ordinary image, script, and stylesheet loads are unaffected. Upgrading: binaries that predate `sanitize_creatives` reject a blob carrying it, so upgrade the binary first, then push the config. Rollback: non-default values (`sanitize_creatives = true`, `rewrite_creatives = false`) are serialized into the config blob and older binaries reject unknown fields — before rolling back to a binary that predates a field, restore its default, push the default-compatible blob, then roll back. +- **Breaking** — Auction creative sanitization is now opt-in: the new `[auction].sanitize_creatives` defaults to `false` because unconditional sanitization blanked script-based creatives (the majority of programmatic display) while recording normal impressions. `[auction].rewrite_creatives` keeps its `true` default. The per-creative cap is now enforced on rewritten output as well as raw input and in every processing mode (1 MiB for auction `adm`; proxied HTML documents keep the proxy's own 10 MiB bound), rewriting fails closed on parser errors instead of emitting partial output and never turns a rejected creative into a runtime-only `adm`, and `hb_cache_host`/`hb_cache_path` are emitted only for bids that supplied no creative — any bid carrying its own `adm` ships without them, so a processed or rejected creative can never be re-fetched raw from PBS Cache. Creative markup with no `` token now receives the click-guard runtime, and a bidder `` loses its `href` whenever asset or click rewriting is on (one without `target` is removed). The creative iframe sandbox no longer grants `allow-same-origin`, restoring origin isolation; rewritten-click recovery from the resulting opaque-origin iframe uses the GET `/first-party/proxy-rebuild` navigation fallback, now registered in every adapter and documented alongside the POST JSON form. Inside those iframes, dynamic resource signing and CORS-mode subresources (ES modules, `crossorigin` fonts) are unavailable pending the constrained asset capability in [#982](https://github.com/IABTechLab/trusted-server/issues/982); ordinary image, script, and stylesheet loads are unaffected. Upgrading: binaries that predate `sanitize_creatives` reject a blob carrying it, so upgrade the binary first, then push the config. Rollback: non-default values (`sanitize_creatives = true`, `rewrite_creatives = false`) are serialized into the config blob and older binaries reject unknown fields — before rolling back to a binary that predates a field, restore its default, push the default-compatible blob, then roll back. - The SPA re-auction endpoint moved from `/__ts/page-bids` to `/_ts/page-bids`, joining every other internal route in the `/_ts/` namespace. The old path stays registered as a deprecated alias so already-loaded bundles keep serving ads, and responses on it carry a `Link: …; rel="deprecation"` header so remaining traffic is measurable from edge logs; removal is tracked in [#970](https://github.com/IABTechLab/trusted-server/issues/970). Two deployment notes: audit `[[handlers]]` for patterns broad enough to cover `/_ts` (for example `^/_ts`), which would put this browser-facing endpoint behind Basic Auth and return `401` to every visitor — scope them to `^/_ts/admin`; and prefer rolling forward over rolling back, since a server reverted past this release does not register the canonical path. In both cases the shipped client falls back to the deprecated alias, so the exposure is bounded until that alias is removed. - Added optional APS `inventory_domain` and `inventory_page_origin` overrides for deployments whose edge hostname differs from the APS-authorized inventory identity. - Preserved APS renderer capabilities through the client-side `trustedServer` Prebid adapter, allowing its generated `hb_adid` to render through GAM and Prebid Universal Creative instead of producing an empty creative. @@ -34,6 +34,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 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_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#""#); @@ -1697,43 +1863,216 @@ mod tests { } #[test] - fn to_abs_conversions() { - let settings = crate::test_support::tests::create_test_settings(); - assert_eq!( - to_abs(&settings, "//cdn.example/x"), - Some("https://cdn.example/x".to_owned()) - ); - assert_eq!( - to_abs(&settings, "HTTPS://cdn.example/x"), - Some("HTTPS://cdn.example/x".to_owned()) - ); - assert_eq!( - to_abs(&settings, "http://cdn.example/x"), - Some("http://cdn.example/x".to_owned()) - ); - assert_eq!(to_abs(&settings, "/local/x"), None); - assert_eq!( - to_abs(&settings, " //cdn.example/y "), - Some("https://cdn.example/y".to_owned()) - ); - assert_eq!(to_abs(&settings, "data:image/png;base64,abcd"), None); - assert_eq!(to_abs(&settings, "javascript:alert(1)"), None); - assert_eq!(to_abs(&settings, "mailto:test@example.com"), None); + 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(" without target 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 to_abs_preserves_port_in_protocol_relative() { - let settings = crate::test_support::tests::create_test_settings(); - assert_eq!( - to_abs(&settings, "//cdn.example.com:8080/asset.js"), - Some("https://cdn.example.com:8080/asset.js".to_owned()), - "should preserve port 8080 in protocol-relative URL" - ); - assert_eq!( - to_abs(&settings, "//cdn.example.com:9443/img.png"), - Some("https://cdn.example.com:9443/img.png".to_owned()), - "should preserve port 9443 in protocol-relative URL" - ); + 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 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); + + 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()) + } + + #[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), + ("ftp://cdn.example/x", None), + ("ws://cdn.example/x", None), + ("file://cdn.example/x", None), + ("https://exa mple.example/x", None), + ] { + assert_eq!( + normalized(raw).as_deref(), + expected, + "should normalize `{raw}` to {expected:?}" + ); + } } #[test] @@ -3416,18 +3755,6 @@ b{background:url(\"https://cdn.example/c.png\")}"; } } - #[test] - fn to_abs_additional_cases() { - let settings = crate::test_support::tests::create_test_settings(); - assert_eq!( - to_abs(&settings, " https://cdn.example/a "), - Some("https://cdn.example/a".to_owned()) - ); - assert_eq!(to_abs(&settings, "blob:xyz"), None); - assert_eq!(to_abs(&settings, "tel:+123"), None); - assert_eq!(to_abs(&settings, "about:blank"), None); - } - #[test] fn rewrites_lazy_img_data_src_and_data_srcset() { let settings = crate::test_support::tests::create_test_settings(); @@ -3443,62 +3770,86 @@ b{background:url(\"https://cdn.example/c.png\")}"; } #[test] - fn to_abs_respects_exclude_domains() { + 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()]; + settings.rewrite.exclude_domains = vec![ + "trusted-cdn.example.com".to_owned(), + "*.example.org".to_owned(), + ]; - // Excluded domain should return None (not proxied) - assert_eq!( - to_abs(&settings, "https://trusted-cdn.example.com/lib.js"), - None - ); + 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}`" + ); + } + } - assert_eq!( - to_abs(&settings, "//trusted-cdn.example.com/lib.js"), - None, - "should exclude a protocol-relative URL by exact domain" + #[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}" ); + } - // Non-excluded domain should return Some - assert_eq!( - to_abs(&settings, "https://other-cdn.example.com/lib.js"), - Some("https://other-cdn.example.com/lib.js".to_owned()) + #[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_eq!( - to_abs(&settings, "//other-cdn.example.com/lib.js"), - Some("https://other-cdn.example.com/lib.js".to_owned()), - "should normalize a non-excluded protocol-relative URL" + assert!( + !out.contains("data-tsclick"), + "should not mark an unparseable link for the click guard: {out}" ); } #[test] - fn to_abs_respects_wildcard_domains() { + fn exclude_domains_match_case_insensitively_in_the_rewrite_pass() { let mut settings = crate::test_support::tests::create_test_settings(); - settings.rewrite.exclude_domains = vec!["*.cloudflare.com".to_owned()]; + settings + .rewrite + .exclude_domains + .extend(["Landing.Example.com", "CDN.example.com"].map(str::to_owned)); + let html = r#"x"#; - // Should exclude base domain - assert_eq!(to_abs(&settings, "https://cloudflare.com/cdn.js"), None); + let out = rewrite_creative_html(&settings, html); - // Should exclude subdomain - assert_eq!( - to_abs(&settings, "https://cdnjs.cloudflare.com/lib.js"), - None - ); - assert_eq!( - to_abs(&settings, "//cloudflare.com/cdn.js"), - None, - "should exclude a protocol-relative wildcard base domain" - ); - assert_eq!( - to_abs(&settings, "//cdnjs.cloudflare.com/lib.js"), - None, - "should exclude a protocol-relative wildcard subdomain" + assert!( + out.contains(r#""#), + "should leave a link raw when its host matches a mixed-case entry: {out}" ); - - // Should not exclude different domain - assert_eq!( - to_abs(&settings, "https://notcloudflare.com/lib.js"), - Some("https://notcloudflare.com/lib.js".to_owned()) + assert!( + out.contains(r#""#), + "should leave an asset raw when its host matches a mixed-case entry: {out}" ); } diff --git a/crates/trusted-server-core/src/proxy.rs b/crates/trusted-server-core/src/proxy.rs index 6a30645ab..34d5f4774 100644 --- a/crates/trusted-server-core/src/proxy.rs +++ b/crates/trusted-server-core/src/proxy.rs @@ -1692,7 +1692,8 @@ pub async fn handle_first_party_click( /// # Errors /// /// Returns an error if JSON parsing fails, the URL cannot be parsed, the URL uses an -/// unsupported scheme, the URL lacks a host, or the host violates `proxy.allowed_domains`. +/// unsupported scheme, the URL lacks a host, the host matches +/// `rewrite.exclude_domains`, or the host violates `proxy.allowed_domains`. pub async fn handle_first_party_proxy_sign( settings: &Settings, _services: &RuntimeServices, @@ -1741,37 +1742,31 @@ pub async fn handle_first_party_proxy_sign( }; let trimmed = payload.url.trim(); - let abs = if trimmed.starts_with("//") { - format!("{}:{}", request_scheme, trimmed) + 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 { - crate::creative::to_abs(settings, trimmed).ok_or_else(|| { - Report::new(TrustedServerError::Proxy { - message: "unsupported url".to_string(), - }) - })? + trimmed }; - - if settings.rewrite.is_excluded(&abs) { - return Err(Report::new(TrustedServerError::Proxy { + let target = crate::creative::normalize_creative_url(candidate).ok_or_else(|| { + Report::new(TrustedServerError::Proxy { message: "unsupported url".to_string(), - })); - } - - let parsed = url::Url::parse(&abs).change_context(TrustedServerError::Proxy { - message: "invalid url".to_string(), + }) })?; - let scheme = parsed.scheme(); - if scheme != "http" && scheme != "https" { - return Err(Report::new(TrustedServerError::Proxy { - message: "unsupported scheme".to_string(), - })); - } - - let host = parsed.host_str().ok_or_else(|| { + 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(), + })); + } if !is_host_permitted(&settings.proxy.allowed_domains, host) { log::warn!( "sign request for `{}` blocked: host not in proxy.allowed_domains", @@ -1791,10 +1786,10 @@ pub async fn handle_first_party_proxy_sign( .to_string(); let extras = vec![(String::from("tsexp"), tsexp)]; - let mut base = parsed.clone(); + let mut base = target.clone(); base.set_query(None); base.set_fragment(None); - let proxied = crate::creative::build_proxy_url_with_extras(settings, &abs, &extras); + let proxied = crate::creative::build_proxy_url_with_extras(settings, target.as_str(), &extras); let resp = ProxySignResp { href: proxied, @@ -2766,20 +2761,58 @@ mod tests { let mut settings = create_test_settings(); settings.rewrite.exclude_domains = vec!["cdn.example".to_owned()]; - for url in ["https://cdn.example/asset.js", "//cdn.example/asset.js"] { - let body = serde_json::json!({ "url": url }); - let req = - build_http_post_json_request("https://edge.example/first-party/sign", &body); - let err: Report = - handle_first_party_proxy_sign(&settings, &noop_services(), req) - .await - .expect_err("should reject excluded URL"); + 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() + ); + } + } + }); + } - assert_eq!( - err.current_context().status_code(), - StatusCode::BAD_GATEWAY, - "should reject excluded URL `{url}` as unsupported" - ); + #[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() + ); + } } }); } @@ -3694,6 +3727,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. diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index 682ecc0f1..682232903 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -22717,6 +22717,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 diff --git a/crates/trusted-server-core/src/settings.rs b/crates/trusted-server-core/src/settings.rs index 3efc15770..d4ba0a82d 100644 --- a/crates/trusted-server-core/src/settings.rs +++ b/crates/trusted-server-core/src/settings.rs @@ -23,6 +23,7 @@ use crate::creative_opportunities::CreativeOpportunitiesConfig; use crate::error::TrustedServerError; use crate::host_header::validate_host_header_override_value; use crate::platform::PlatformImageOptimizerRegion; +use crate::proxy::is_host_allowed; use crate::redacted::Redacted; #[cfg(test)] @@ -637,36 +638,55 @@ impl Ec { #[serde(deny_unknown_fields)] pub struct Rewrite { /// List of domains to exclude from rewriting. Supports wildcards (e.g., "*.example.com"). - /// URLs from these domains will not be proxied through first-party endpoints. + /// URLs from these domains are neither proxied through first-party + /// endpoints nor click-wrapped. #[serde(default)] pub exclude_domains: Vec, } impl Rewrite { - /// Checks if a URL should be excluded from rewriting based on domain matching - #[allow(dead_code)] + /// 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 `crate::proxy::is_host_allowed` for the pattern rules. #[must_use] - pub fn is_excluded(&self, url: &str) -> bool { - // Parse URL to extract host - let Ok(parsed) = url::Url::parse(url) else { - return false; - }; + pub fn should_proxy_asset(&self, host: &str) -> bool { + !self.is_excluded_host(host) + } - let host = parsed.host_str().unwrap_or(""); + /// 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) + } - // Check exact domain matches (with wildcard support) - for domain in &self.exclude_domains { - if let Some(suffix) = domain.strip_prefix("*.") { - // Wildcard: *.example.com matches both example.com and sub.example.com - if host == suffix || host.ends_with(&format!(".{}", suffix)) { - return true; - } - } else if host == domain { - return true; - } - } + fn is_excluded_host(&self, host: &str) -> bool { + self.exclude_domains + .iter() + .any(|pattern| is_host_allowed(host, pattern)) + } - false + /// 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" + ); + } } } @@ -3081,6 +3101,7 @@ impl Settings { pub(crate) fn normalize_deserialized(&mut self) -> Result<(), Report> { self.cache.normalize(); self.proxy.normalize(); + self.rewrite.normalize(); self.image_optimizer.normalize()?; self.debug.auction_html_comment_options.normalize(); self.tinybird.normalize(); @@ -6673,29 +6694,82 @@ source_domain = "partner.example.com" } #[test] - fn test_rewrite_is_excluded() { - let rewrite = Rewrite { - exclude_domains: vec!["cdn.example.com".to_string(), "*.example2.com".to_string()], - }; + 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)); - // Exact domain match - assert!(rewrite.is_excluded("http://cdn.example.com/image.png")); + for (host, expected) in [ + ("cdn.example.com", false), + ("CDN.EXAMPLE.COM", false), + ("example.org", false), + ("a.b.example.org", false), + ("evil-example.org", true), + ("example.org.evil.example.net", 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}" + ); + } + } - // Wildcard match - base domain - assert!(rewrite.is_excluded("https://example2.com/cdn.js")); - // Wildcard match - subdomains - assert!(rewrite.is_excluded("https://cdnjs.example2.com/lib.js")); - assert!(rewrite.is_excluded("https://sub.domain.example2.com/asset.js")); + #[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)); - // Should NOT match - assert!(!rewrite.is_excluded("https://other.example.com/asset.js")); - assert!(!rewrite.is_excluded("https://sub.cdn.example.com/asset.js")); - assert!(!rewrite.is_excluded("https://example2.com.fake.com/asset.js")); - assert!(!rewrite.is_excluded("https://notexample.com/asset.js")); + rewrite.normalize(); - // Invalid URLs should not crash and should return false - assert!(!rewrite.is_excluded("not a url")); - assert!(!rewrite.is_excluded("")); + 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" + ); } #[test] @@ -6716,6 +6790,32 @@ source_domain = "partner.example.com" !settings.auction.sanitize_creatives, "creative sanitization is opt-in when the setting is omitted" ); + assert_eq!( + settings.auction.rewrite_clicks, None, + "click rewriting stays unset when the setting is omitted" + ); + } + + #[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}" + ); + } } #[test] diff --git a/crates/trusted-server-js/lib/src/shared/origin.ts b/crates/trusted-server-js/lib/src/shared/origin.ts index e08431ae4..6f88be2ba 100644 --- a/crates/trusted-server-js/lib/src/shared/origin.ts +++ b/crates/trusted-server-js/lib/src/shared/origin.ts @@ -53,8 +53,8 @@ export function hasOpaqueOrigin(): boolean { // Inherited from the embedder and therefore honours a publisher ``. // // Pinning at load time means bidder script cannot redirect resolution later by -// injecting ``; the server-side rewriter also strips `` from -// creative markup whenever rewriting is enabled. +// injecting ``; the server-side rewriter also drops the `` `href` +// (keeping `target`) when asset or click rewriting is on. export const TRUSTED_BASE_URL: string = (() => { try { const stamped = normalizeTrustedOrigin( diff --git a/docs/guide/api-reference.md b/docs/guide/api-reference.md index f6c8f72d3..7efc90865 100644 --- a/docs/guide/api-reference.md +++ b/docs/guide/api-reference.md @@ -169,8 +169,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..b93813c59 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 ``` @@ -633,13 +633,16 @@ EC identity is maintained with the `ts-ec` cookie; auction responses do not emit Winning creatives returned by `POST /auction` pass through two independent transforms. `sanitize_creatives` (opt-in, default `false`) strips executable -markup with its inner content. `rewrite_creatives` (default `true`) runs an -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. +markup with its inner content. `rewrite_creatives` (default `true`) converts +eligible external asset and inline CSS `url(...)` URLs to signed +`/first-party/proxy` paths. `rewrite_clicks` (unset by default, following +`rewrite_creatives`) wraps ``/`` in `/first-party/click` and +adds `data-tsclick`; see +[Creative Processing](/guide/creative-processing#assets-and-clicks). When +either is on, the pass drops a bidder `` URL (keeping its `target`) 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. ```toml [auction] @@ -647,6 +650,12 @@ sanitize_creatives = false rewrite_creatives = true ``` +The table below assumes `rewrite_clicks` is unset, so links follow +`rewrite_creatives`. An explicit `rewrite_clicks` decides link wrapping and +click-guard attributes on its own, and turns on creative TSJS injection on +`POST /auction` even when `rewrite_creatives = false`; see +[Assets and clicks](/guide/creative-processing#assets-and-clicks). + | `sanitize_creatives` | `rewrite_creatives` | Winning-bid `adm` behavior | | -------------------- | ------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | | `false` (default) | `false` | Deliver the creative exactly as the bidder returned it (subject to the size cap). | @@ -657,17 +666,18 @@ rewrite_creatives = true When sanitization is enabled, scripts, stylesheets, style blocks, forms, event handlers, dangerous URL schemes, and other rejected content are removed together with their inner content — which blanks script-based creatives. Disabling -rewriting removes the injected creative runtime and first-party proxy/click +both `rewrite_creatives` and `rewrite_clicks` removes the injected creative runtime and first-party proxy/click mediation from the resulting `adm`, so the browser may contact third-party hosts without mediation. Sanitizer-accepted hosts are not allowlisted or trusted merely because their URLs remain in the output. -Both settings apply to winning-bid `adm` in both the shared `POST /auction` +All three settings apply to winning-bid `adm` in both the shared `POST /auction` response converter and the production publisher SSAT/page-bids path. The former emits root-relative first-party URLs and injects creative TSJS; the latter emits absolute first-party URLs for its foreign-origin renderer and does not inject that bundle. HTML/CSS returned by `/first-party/proxy` continues to be -rewritten independently. `[debug].inject_adm_for_testing` adds the diagnostic +rewritten independently of `rewrite_creatives`; only an explicit +`rewrite_clicks` changes its link wrapping. `[debug].inject_adm_for_testing` adds the diagnostic `debug_bid` blob and enables a testing-only direct GAM replacement; it does not control whether processed `adm` is delivered. @@ -702,6 +712,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" @@ -835,7 +846,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: @@ -850,9 +863,12 @@ 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 +changes link wrapping when `rewrite_clicks` differed from `rewrite_creatives`, +and changes proxied HTML when `rewrite_clicks` was `false`. 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 f39ad638c..fbe148c85 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -1936,15 +1936,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` @@ -1952,23 +1953,32 @@ 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 asset and link behavior. One change applies to existing +configs too: whenever either setting is on, a bidder-supplied `` element +loses its `href`; it keeps a `target` and is removed when it has none (before +this release the whole element was removed). With `sanitize_creatives = true`, +the sanitizer removes `` entirely before the rewrite pass. 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 @@ -1982,14 +1992,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 @@ -2177,8 +2193,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 are wrapped, and creative TSJS injected on `POST /auction`, +only 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 5edf64c9e..4b7ad4cdc 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, 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 Sanitization removes `script`/`object`/`embed`/`form` and similar elements @@ -90,6 +92,54 @@ 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` | `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. +The `` column assumes `sanitize_creatives = false`; the sanitizer removes +`` entirely before the rewrite pass. +`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 and `rewrite_creatives` on, the injected +guard 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, 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. +With `sanitize_creatives = true` the sanitizer removes `` entirely first. + +On `POST /auction`, the injected click guard navigates a `data-tsclick` link +using only the link's own `target`, not a kept ``. A creative that +relies on `` can therefore open wrapped links in the frame +and unwrapped links in a new tab; give links an explicit `target` to avoid this. + ::: 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 @@ -130,9 +180,10 @@ ships without them, so a processed or refused creative can never be re-fetched raw. ::: -Neither setting affects HTML/CSS response rewriting under -`/first-party/proxy`. The publisher SSAT/page-bids path is a production creative -path and follows both settings. `[debug].inject_adm_for_testing` controls only the +`rewrite_creatives` and `sanitize_creatives` do not affect HTML/CSS rewriting +under `/first-party/proxy`; an explicit `rewrite_clicks` does apply to links +there. The publisher SSAT/page-bids path is a production creative path and +follows all three settings. `[debug].inject_adm_for_testing` controls only the additional `debug_bid` diagnostic blob and testing-only direct GAM replacement; it does not control whether the processed `adm` is present. @@ -307,15 +358,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 ``` @@ -658,10 +712,12 @@ Pattern: *.cdn.example.com Matches: ✅ assets.cdn.example.com ✅ images.cdn.example.com - ❌ cdn.example.com (no subdomain) + ✅ cdn.example.com (apex also matches) ❌ cdn.example.com.evil.com (different domain) ``` +Matching is case-insensitive. + **Exact Patterns**: No `*` requires exact host match ``` @@ -923,8 +979,8 @@ fn test_image_src_rewrite() { **Relative URLs Not Working**: -- Ensure origin response includes proper `` tag -- Or convert to absolute URLs before rewriting +- The rewriter drops a `` `href`, so relative URLs do not resolve against it +- Convert relative URLs to absolute URLs before rewriting **Data URIs Being Rewritten**: 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..d1c1625b9 --- /dev/null +++ b/docs/superpowers/plans/2026-10-07-1234-creative-click-rewrite-switch.md @@ -0,0 +1,1856 @@ +# 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 asset or link output for any existing config (a bidder `` is now kept). + +**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. +- `` 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. + +**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, except for the shared step's CHANGELOG Fixed items and the kept ``. +- `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 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.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 + 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 `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) + } + + /// 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 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 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_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 + } +``` + +- [ ] **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() { + 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 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); + + 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** + +`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. +- `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`** + +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![ + // (`` handler, changed as shown below) + // (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()`. + +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 +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." + + 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." +- `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: `554 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#"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}" + ); + } + } +``` + +- [ ] **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`." + +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` + +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 +``` + +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 +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 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`** + +- 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; + - 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** + +- `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`; + - 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. + +- [ ] **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 `` 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: + +```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** + +```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 | `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** + +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..a331fa445 --- /dev/null +++ b/docs/superpowers/specs/2026-10-07-1234-creative-click-rewrite-switch-design.md @@ -0,0 +1,530 @@ +# 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:** Implemented in #1252 (spec PR #1249 was closed in favor of it) + +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, 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. + +## 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:1898-1906`). + +## 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; 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, +``` + +`AuctionConfig::default()` sets it to `None`. Two methods resolve it, so no call site reads the raw field: + +```rust +impl AuctionConfig { + /// 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 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, except for the `` fix below. + +`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 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`: + +```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-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. + 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 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. `/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, as one entry, in whichever PR lands the shared step. The exact text is in the plan's "Shared step" section. + +### 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]`, `