diff --git a/CHANGELOG.md b/CHANGELOG.md index 101087b8d..c69e28abc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,14 +33,18 @@ 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, and other entries that can never match a host are logged 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 +1862,213 @@ 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), + ("https://exa mple.example/x", None), + ] { + assert_eq!( + normalized(raw).as_deref(), + expected, + "should normalize `{raw}` to {expected:?}" + ); + } } #[test] @@ -3416,18 +3751,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,65 +3766,341 @@ 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!( + out.contains(r#""#), + "should leave a link raw when its host matches a mixed-case entry: {out}" ); + assert!( + out.contains(r#""#), + "should leave an asset raw when its host matches a mixed-case entry: {out}" + ); + } + + const ALLOWLIST_LISTED: &[&str] = &[ + "https://assets.example.com/a.css", + "https://assets.example.com/p1.png", + "https://assets.example.com/p1-2x.png", + "https://assets.example.com/a.js", + "https://assets.example.com/i.css", + "https://assets.example.com/bg.png", + "https://assets.example.com/set.png", + "https://assets.example.com/img.png", + "https://assets.example.com/lazy.png", + "https://assets.example.com/s1.png", + "https://assets.example.com/v.mp4", + "https://assets.example.com/o.swf", + "https://assets.example.com/e.swf", + "https://assets.example.com/btn.png", + "https://assets.example.com/icon.svg", + "https://assets.example.com/sprite.svg", + "https://assets.example.com/frame.html", + "https://assets.example.com/inline.png", + ]; + + const ALLOWLIST_OFF_LIST: &[&str] = &[ + "https://cdn.example.net/a.css", + "https://cdn.example.net/p2.png", + "https://cdn.example.net/a.js", + "https://cdn.example.net/i.css", + "https://cdn.example.net/bg.png", + "https://cdn.example.net/set.png", + "https://cdn.example.net/img.png", + "https://cdn.example.net/lazy.png", + "https://cdn.example.net/s2.png", + "https://cdn.example.net/v.mp4", + "https://cdn.example.net/o.swf", + "https://cdn.example.net/e.swf", + "https://cdn.example.net/btn.png", + "https://cdn.example.net/icon.svg", + "https://cdn.example.net/sprite.svg", + "https://cdn.example.net/frame.html", + "https://cdn.example.net/inline.png", + ]; + + const ALLOWLIST_CREATIVE: &str = r#" + + + + + + + + + + + + + + + + + + + + + +
+
+Offer +Area +"#; + + fn encoded_tsurl(url: &str) -> String { + format!( + "tsurl={}", + url::form_urlencoded::byte_serialize(url.as_bytes()).collect::() + ) + } + + fn allowlist_settings() -> crate::settings::Settings { + let mut settings = crate::test_support::tests::create_test_settings(); + settings.rewrite.include_domains = vec!["*.example.com".to_owned()]; + settings + } + + fn assert_allowlist_applied(label: &str, out: &str) { + for listed in ALLOWLIST_LISTED { + assert!( + out.contains(&encoded_tsurl(listed)), + "{label}: should proxy listed URL `{listed}`: {out}" + ); + } + for off_list in ALLOWLIST_OFF_LIST { + assert!( + !out.contains(&encoded_tsurl(off_list)), + "{label}: should not proxy off-list URL `{off_list}`: {out}" + ); + assert!( + out.contains(off_list), + "{label}: should keep off-list URL `{off_list}` raw: {out}" + ); + } + for landing in [ + "https://landing.example.net/offer", + "https://landing.example.org/area", + ] { + assert!( + out.contains(&format!("/first-party/click?{}", encoded_tsurl(landing))), + "{label}: should wrap click `{landing}` regardless of include_domains: {out}" + ); + } assert_eq!( - to_abs(&settings, "//cloudflare.com/cdn.js"), - None, - "should exclude a protocol-relative wildcard base domain" + out.matches("data-tsclick=").count(), + 2, + "{label}: should mark both click-through links" ); - assert_eq!( - to_abs(&settings, "//cdnjs.cloudflare.com/lib.js"), - None, - "should exclude a protocol-relative wildcard subdomain" + } + + #[test] + fn include_domains_limit_asset_rewriting_on_every_html_path() { + let settings = allowlist_settings(); + + assert_allowlist_applied( + "auction", + &rewrite_creative_html(&settings, ALLOWLIST_CREATIVE), + ); + assert_allowlist_applied( + "inline", + &rewrite_inline_creative_html(&settings, "https://www.example.com", ALLOWLIST_CREATIVE), + ); + assert_allowlist_applied( + "proxied", + &rewrite_proxied_html(&settings, ALLOWLIST_CREATIVE), ); + } - // Should not exclude different domain + #[test] + fn include_domains_limit_css_body_rewriting() { + let settings = allowlist_settings(); + let css = r#"@import "https://assets.example.com/i.css"; +@import "https://cdn.example.net/i.css"; +.a { background: url(https://assets.example.com/bg.png); } +.b { background: url(https://cdn.example.net/bg.png); }"#; + + let out = rewrite_css_body(&settings, css).expect("should rewrite CSS body"); + + for listed in [ + "https://assets.example.com/i.css", + "https://assets.example.com/bg.png", + ] { + assert!( + out.contains(&encoded_tsurl(listed)), + "should proxy listed CSS URL `{listed}`: {out}" + ); + } + for off_list in [ + "https://cdn.example.net/i.css", + "https://cdn.example.net/bg.png", + ] { + assert!( + out.contains(off_list) && !out.contains(&encoded_tsurl(off_list)), + "should keep off-list CSS URL `{off_list}` raw: {out}" + ); + } + } + + // `imagesrcset` is rewritten only by the `[imagesrcset]` handler. When the + // `link[href]` handler also rewrote it, the inline path's absolute proxy + // URLs were proxied a second time. + #[test] + fn inline_link_imagesrcset_is_proxied_once() { + let settings = crate::test_support::tests::create_test_settings(); + let html = r#""#; + + let out = rewrite_inline_creative_html(&settings, "https://publisher.example.com", html); + + let imagesrcset = out + .split("imagesrcset=\"") + .nth(1) + .and_then(|rest| rest.split('"').next()) + .expect("should keep an imagesrcset attribute"); assert_eq!( - to_abs(&settings, "https://notcloudflare.com/lib.js"), - Some("https://notcloudflare.com/lib.js".to_owned()) + imagesrcset.matches("tsurl=").count(), + 1, + "should proxy the imagesrcset candidate exactly once: {imagesrcset}" ); } + #[test] + fn root_relative_link_imagesrcset_is_proxied_once() { + let settings = crate::test_support::tests::create_test_settings(); + let html = r#""#; + + for (label, out) in [ + ("auction", rewrite_creative_html(&settings, html)), + ("proxied", rewrite_proxied_html(&settings, html)), + ] { + let imagesrcset = out + .split("imagesrcset=\"") + .nth(1) + .and_then(|rest| rest.split('"').next()) + .expect("should keep an imagesrcset attribute"); + assert_eq!( + imagesrcset.matches("tsurl=").count(), + 1, + "{label}: should proxy the imagesrcset candidate exactly once: {imagesrcset}" + ); + } + } + + #[test] + fn host_in_both_lists_is_left_alone() { + let mut settings = allowlist_settings(); + settings.rewrite.exclude_domains = vec!["assets.example.com".to_owned()]; + let html = ALLOWLIST_CREATIVE.replace( + "", + r#"Excluded"#, + ); + + for (label, out) in [ + ("auction", rewrite_creative_html(&settings, &html)), + ( + "inline", + rewrite_inline_creative_html(&settings, "https://www.example.com", &html), + ), + ("proxied", rewrite_proxied_html(&settings, &html)), + ] { + assert!( + !out.contains("tsurl=https%3A%2F%2Fassets.example.com"), + "{label}: should not rewrite any URL on a host in both lists: {out}" + ); + for listed in ALLOWLIST_LISTED { + assert!( + out.contains(listed), + "{label}: should keep `{listed}` raw when its host is also excluded: {out}" + ); + } + assert!( + out.contains(r#"Excluded"#), + "{label}: should leave an excluded click-through link raw: {out}" + ); + assert_eq!( + out.matches("data-tsclick=").count(), + 2, + "{label}: should wrap only the two links on hosts that are not excluded: {out}" + ); + } + } + #[test] fn rewrite_html_excludes_blacklisted_domains() { let mut settings = crate::test_support::tests::create_test_settings(); diff --git a/crates/trusted-server-core/src/proxy.rs b/crates/trusted-server-core/src/proxy.rs index 5d659cdbf..7f6f1364e 100644 --- a/crates/trusted-server-core/src/proxy.rs +++ b/crates/trusted-server-core/src/proxy.rs @@ -1225,7 +1225,7 @@ fn append_ec_id(req: &Request, target_url_parsed: &mut url::Url) { /// /// When `allowed_domains` is empty every host is permitted (open mode). /// When non-empty the host must match at least one pattern via [`is_host_allowed`]. -fn is_host_permitted>(allowed_domains: &[S], host: &str) -> bool { +pub(crate) fn is_host_permitted>(allowed_domains: &[S], host: &str) -> bool { allowed_domains.is_empty() || allowed_domains .iter() @@ -1670,37 +1670,33 @@ 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: host excluded or not in rewrite.include_domains" + ); + 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", @@ -1720,10 +1716,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, @@ -2695,20 +2691,171 @@ 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() + ); + } + } + }); + } + + #[test] + fn proxy_sign_applies_rewrite_include_domains_for_get_and_post() { + struct Case { + name: &'static str, + include_domains: &'static [&'static str], + exclude_domains: &'static [&'static str], + allowed_domains: &'static [&'static str], + target: &'static str, + expected: StatusCode, + } + + let cases = [ + Case { + name: "listed absolute", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "https://img.example.com/a.png", + expected: StatusCode::OK, + }, + Case { + name: "listed protocol-relative", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "//img.example.com/a.png", + expected: StatusCode::OK, + }, + Case { + name: "off-list absolute", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "https://img.example.net/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "off-list protocol-relative", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "//img.example.net/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "host in both lists", + include_domains: &["*.example.com"], + exclude_domains: &["img.example.com"], + allowed_domains: &[], + target: "https://img.example.com/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "off include list and off proxy allowlist", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &["*.example.org"], + target: "https://img.example.net/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "listed but off proxy allowlist", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &["*.example.org"], + target: "https://img.example.com/a.png", + expected: StatusCode::FORBIDDEN, + }, + Case { + name: "mixed-case host", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "https://IMG.Example.COM/a.png", + expected: StatusCode::OK, + }, + ]; + + let to_owned = |values: &[&str]| -> Vec { + values.iter().map(|value| (*value).to_owned()).collect() + }; + + futures::executor::block_on(async { + for case in &cases { + for method in [&Method::GET, &Method::POST] { + let label = format!("{} {}", method.as_str(), case.name); + let mut settings = create_test_settings(); + settings.rewrite.include_domains = to_owned(case.include_domains); + settings.rewrite.exclude_domains = to_owned(case.exclude_domains); + settings.proxy.allowed_domains = to_owned(case.allowed_domains); + let req = build_proxy_sign_request( + method, + "https://edge.example.com/first-party/sign", + case.target, + ); + + let status = + match handle_first_party_proxy_sign(&settings, &noop_services(), req).await + { + Ok(response) => response.status(), + Err(error) => error.current_context().status_code(), + }; + + assert_eq!( + status, case.expected, + "{label} should return {}", + case.expected + ); + } } }); } @@ -3623,6 +3770,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 d8c217076..2759655f7 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -22714,6 +22714,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 bf4ff7999..06ce12a3c 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, is_host_permitted}; use crate::redacted::Redacted; #[cfg(test)] @@ -636,37 +637,84 @@ impl Ec { #[derive(Debug, Default, Clone, Deserialize, Serialize, Validate)] #[serde(deny_unknown_fields)] pub struct Rewrite { - /// List of domains to exclude from rewriting. Supports wildcards (e.g., "*.example.com"). - /// URLs from these domains will not be proxied through first-party endpoints. + /// Hosts never rewritten, for assets and click-through links. + /// + /// Each entry is an exact host or `*.example.com`, which matches the apex + /// and any subdomain. Matching is case-insensitive. Wins over + /// [`Self::include_domains`]. #[serde(default)] pub exclude_domains: Vec, + /// When non-empty, the only hosts whose asset URLs are rewritten to + /// `/first-party/proxy`. + /// + /// Same pattern syntax as [`Self::exclude_domains`]. Empty (the default) + /// proxies every eligible asset URL. Never applies to click-through links. + /// The default is omitted from serialized config so older binaries, which + /// reject unknown fields, can still read it. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[validate(custom(function = validate_include_domains))] + pub include_domains: Vec, } impl Rewrite { - /// 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`], and must match + /// [`Self::include_domains`] when that list is non-empty. 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) && is_host_permitted(&self.include_domains, 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)) + } + + /// 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" + ); + } + self.include_domains = self + .include_domains + .iter() + .map(|pattern| pattern.trim().to_ascii_lowercase()) + .collect(); + // An `exclude_domains` entry that can never equal a URL host keeps its + // host proxied and receiving `ts-ec`. Rejecting it would stop existing + // blobs from loading, so warn once per entry and keep it. + for pattern in &self.exclude_domains { + if let Err(reason) = check_include_domain(pattern) { + log::warn!( + "rewrite.exclude_domains: entry `{pattern}` never matches a URL host ({reason})" + ); } } - - false } } @@ -2988,6 +3036,7 @@ impl Settings { pub(crate) fn normalize_deserialized(&mut self) { self.cache.normalize(); self.proxy.normalize(); + self.rewrite.normalize(); self.image_optimizer.normalize(); self.debug.auction_html_comment_options.normalize(); self.tinybird.normalize(); @@ -3381,6 +3430,98 @@ impl Settings { } } +/// Rejects `rewrite.include_domains` entries that can never equal a URL host. +/// +/// Runs after [`Rewrite::normalize`], so entries are already trimmed and +/// lowercased. Entries are matched against [`Url::host_str`], so each one must +/// be: +/// +/// - a DNS name, optionally prefixed with `*.`: one or more `.`-separated, +/// non-empty labels of ASCII `a-z`, `0-9` and `-`; +/// - an IPv4 address in dotted-quad form, such as `192.0.2.1`; or +/// - an IPv6 address in brackets and compressed form, such as `[2001:db8::1]`. +/// +/// A scheme, port, path, space, leading or trailing `.`, empty label, wildcard +/// IP address or unbracketed IPv6 address is rejected with +/// `invalid_rewrite_include_domain`. A non-ASCII (IDN) name is rejected with +/// `non_ascii_rewrite_include_domain`; write it in punycode (`xn--`) form. +fn validate_include_domains(patterns: &[String]) -> Result<(), ValidationError> { + for pattern in patterns { + if !pattern.is_ascii() { + let mut err = ValidationError::new("non_ascii_rewrite_include_domain"); + err.add_param("value".into(), pattern); + err.message = Some( + "rewrite.include_domains entries must be ASCII; write internationalized domain names in punycode (xn--) form" + .into(), + ); + return Err(err); + } + if let Err(reason) = check_include_domain(pattern) { + let mut err = ValidationError::new("invalid_rewrite_include_domain"); + err.add_param("value".into(), pattern); + err.add_param("reason".into(), &reason); + err.message = Some( + "rewrite.include_domains entries must be a host name, an optional leading `*.`, a dotted-quad IPv4 address or a bracketed IPv6 address" + .into(), + ); + return Err(err); + } + } + Ok(()) +} + +/// Checks one normalized ASCII `include_domains` entry, returning why it can +/// never equal a URL host. +fn check_include_domain(pattern: &str) -> Result<(), &'static str> { + if let Some(bracketed) = pattern.strip_prefix('[') { + bracketed + .strip_suffix(']') + .and_then(|inner| inner.parse::().ok()) + .ok_or("not a bracketed IPv6 address")?; + return check_host_round_trip(pattern); + } + if pattern.parse::().is_ok() { + return Err("IPv6 address without brackets"); + } + + let name = pattern.strip_prefix("*.").unwrap_or(pattern); + if name.is_empty() { + return Err("no host"); + } + if !name.bytes().all(|byte| { + byte.is_ascii_lowercase() || byte.is_ascii_digit() || matches!(byte, b'-' | b'.') + }) { + return Err("character outside a-z, 0-9, `-` and `.`"); + } + if name.split('.').any(str::is_empty) { + return Err("leading or trailing `.` or empty label"); + } + + // A URL host whose last label is numeric is parsed as an IPv4 address, so + // such an entry only matches as an exact dotted-quad address. + let last_label = name.rsplit('.').next().unwrap_or(name); + let numeric_last_label = + last_label.bytes().all(|byte| byte.is_ascii_digit()) || last_label.starts_with("0x"); + if numeric_last_label + && (name.len() != pattern.len() || name.parse::().is_err()) + { + return Err("numeric host that is not a dotted-quad IPv4 address"); + } + check_host_round_trip(name) +} + +/// Parses `host` with the parser behind [`Url::host_str`] and requires the +/// rendered form to equal the input, so an entry that `url` rewrites (an +/// IPv4-mapped IPv6 address in dotted form, an invalid `xn--` label) or rejects +/// can never load and then silently fail to match. +fn check_host_round_trip(host: &str) -> Result<(), &'static str> { + let parsed = url::Host::parse(host).map_err(|_| "not a valid URL host")?; + if parsed.to_string() != host { + return Err("not in the form Url::host_str() renders"); + } + Ok(()) +} + fn validate_publisher_domain(value: &str) -> Result<(), ValidationError> { if value.trim() != value || value.is_empty() || value.len() > 253 { return Err(ValidationError::new("invalid_publisher_domain")); @@ -6520,29 +6661,246 @@ 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}" + ); + } + } + + #[test] + fn rewrite_normalize_trims_lowercases_and_drops_inert_exclude_entries() { + let mut rewrite = Rewrite::default(); + rewrite + .exclude_domains + .extend([" CDN.Example.com ", "", "*", "*.Example.ORG"].map(str::to_owned)); + + rewrite.normalize(); + + assert_eq!( + rewrite.exclude_domains, + vec!["cdn.example.com".to_owned(), "*.example.org".to_owned()], + "should trim, lowercase, and drop empty and bare `*` entries" + ); + } - // 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_exclude_domains_match_mixed_case_entries_from_toml() { + let toml_str = crate_test_settings_str() + + r#" + [rewrite] + exclude_domains = ["CDN.Example.com"] + "#; - // 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")); + let settings = Settings::from_toml(&toml_str).expect("should parse valid TOML"); - // Invalid URLs should not crash and should return false - assert!(!rewrite.is_excluded("not a url")); - assert!(!rewrite.is_excluded("")); + 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] + fn rewrite_should_proxy_asset_honors_include_list() { + let mut rewrite = Rewrite::default(); + rewrite + .include_domains + .extend(["*.example.com", "img.example.net", "*.UPPER.Example.org"].map(str::to_owned)); + rewrite + .exclude_domains + .push("blocked.example.com".to_owned()); + + for (host, expected) in [ + ("example.com", true), + ("a.b.example.com", true), + ("img.example.net", true), + ("IMG.EXAMPLE.NET", true), + ("cdn.upper.example.org", true), + ("evil-example.com", false), + ("cdn.example.org", false), + ("blocked.example.com", false), + ] { + assert_eq!( + rewrite.should_proxy_asset(host), + expected, + "should_proxy_asset(`{host}`) should be {expected}" + ); + } + for host in ["cdn.example.org", "evil-example.com", "example.com"] { + assert!( + rewrite.should_wrap_click(host), + "should wrap click for `{host}` regardless of include_domains" + ); + } + assert!( + !rewrite.should_wrap_click("blocked.example.com"), + "should not wrap a click to an excluded host" + ); + } + + #[test] + fn rewrite_exclude_domains_keep_entries_that_never_match() { + let toml_str = crate_test_settings_str() + + r#" + [rewrite] + exclude_domains = ["https://cdn.example.com", "cdn.example.com:443", "[2001:0db8::1]"] + "#; + + let settings = Settings::from_toml(&toml_str) + .expect("should load inert exclude_domains entries with a warning"); + + assert_eq!( + settings.rewrite.exclude_domains, + vec![ + "https://cdn.example.com".to_owned(), + "cdn.example.com:443".to_owned(), + "[2001:0db8::1]".to_owned(), + ], + "should keep inert exclude_domains entries unchanged" + ); + } + + #[test] + fn rewrite_include_domains_are_normalized_from_toml() { + let toml_str = crate_test_settings_str() + + r#" + [rewrite] + include_domains = ["*.CDN.Example.com", " img.example.net "] + "#; + + let settings = Settings::from_toml(&toml_str).expect("should parse valid TOML"); + + assert_eq!( + settings.rewrite.include_domains, + vec!["*.cdn.example.com".to_owned(), "img.example.net".to_owned()], + "should trim and lowercase include_domains entries" + ); + } + + fn include_domains_toml(entry: &str) -> String { + crate_test_settings_str() + &format!("\n[rewrite]\ninclude_domains = [\"{entry}\"]\n") + } + + #[test] + fn rewrite_include_domains_reject_malformed_entries() { + for entry in [ + "", + " ", + "*", + "*.", + "cdn.*.example.com", + "**.example.com", + "https://cdn.example.com", + "cdn.example.com:443", + "cdn.example.com/x", + ".example.com", + "example.com.", + "cdn..example.com", + "cdn example.com", + "cdn_assets.example.com", + "::1", + "[::1", + "[2001:0db8::1]", + "[::ffff:192.0.2.1]", + "xn--zz.example", + "*.192.0.2.1", + "192.0.2", + "256.0.2.1", + ] { + let result = Settings::from_toml(&include_domains_toml(entry)); + + let err = result.expect_err(&format!("should reject include_domains entry `{entry}`")); + assert!( + format!("{err:?}").contains("invalid_rewrite_include_domain"), + "should report `{entry}` as an invalid include_domains entry: {err:?}" + ); + } + } + + #[test] + fn rewrite_include_domains_reject_non_ascii_entries_with_a_punycode_hint() { + for entry in ["bücher.example", "*.bücher.example"] { + let result = Settings::from_toml(&include_domains_toml(entry)); + + let err = result.expect_err(&format!("should reject non-ASCII entry `{entry}`")); + assert!( + format!("{err:?}").contains("non_ascii_rewrite_include_domain"), + "should report `{entry}` as non-ASCII: {err:?}" + ); + } + } + + #[test] + fn rewrite_include_domains_accept_hosts_and_ip_literals() { + for entry in [ + "cdn.example.com", + "*.example.com", + "xn--bcher-kva.example", + "192.0.2.1", + "[2001:db8::1]", + "[::1]", + "[::ffff:c000:201]", + ] { + let settings = + Settings::from_toml(&include_domains_toml(entry)).unwrap_or_else(|err| { + panic!("should accept include_domains entry `{entry}`: {err:?}") + }); + + assert_eq!( + settings.rewrite.include_domains, + vec![entry.to_owned()], + "should keep accepted entry `{entry}` as written" + ); + } + } + + #[test] + fn rewrite_include_domains_default_to_empty() { + let settings = create_test_settings(); + + assert!( + settings.rewrite.include_domains.is_empty(), + "should default include_domains to an empty list" + ); } #[test] @@ -6563,6 +6921,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/docs/guide/api-reference.md b/docs/guide/api-reference.md index e444ec147..91b9c61fc 100644 --- a/docs/guide/api-reference.md +++ b/docs/guide/api-reference.md @@ -168,8 +168,9 @@ curl -i "https://edge.example.com/_ts/clear-tester" Browser and programmatic auction endpoint. It accepts the Trusted Server ad-unit request shape and returns an OpenRTB response. Creative markup follows the -independent `[auction].sanitize_creatives` and `[auction].rewrite_creatives` -settings; sanitization is opt-in and rewriting is enabled by default. +independent `[auction].sanitize_creatives`, `[auction].rewrite_creatives` and +`[auction].rewrite_clicks` settings; sanitization is opt-in, asset rewriting is +enabled by default, and click wrapping follows `rewrite_creatives` unless set. Configured provider IDs appear in response metadata and provider responses. Consumers that previously matched the literal provider name `prebid` must use @@ -566,6 +567,7 @@ curl -X POST https://edge.example.com/first-party/sign \ - `403 Forbidden`: The target has a valid host that does not match a non-empty `proxy.allowed_domains` list - `413 Payload Too Large`: The POST body exceeds 64 KiB +- `502 Bad Gateway`: The target host matches `rewrite.exclude_domains` or is outside a non-empty `rewrite.include_domains` - Shared `5xx`: Malformed JSON or an invalid/unsupported target under the current proxy-error mapping diff --git a/docs/guide/auction-orchestration.md b/docs/guide/auction-orchestration.md index edbc38471..74376dd6e 100644 --- a/docs/guide/auction-orchestration.md +++ b/docs/guide/auction-orchestration.md @@ -161,7 +161,7 @@ sequenceDiagram Note right of Client: Fragment-bound nonce and one-time acknowledgement
No allow-same-origin on the outer frame else Ordinary creative Client->>Client: Inject winning creative
Render iframe
Load creative resources - Note right of Client: Default: first-party proxy/click URLs
rewrite_creatives=false: accepted external URLs remain direct + Note right of Client: Default: first-party proxy/click URLs
rewrite_creatives=false: asset URLs direct; clicks follow rewrite_clicks end deactivate Client end @@ -195,7 +195,7 @@ AuctionOrchestrator.run_auction() Convert OrchestrationResult → OpenRTB 2.x Response │ ├─[sanitize_creatives=true] Strip executable markup - ├─[rewrite_creatives=true] Rewrite URLs and inject creative TSJS + ├─[rewrite_creatives or rewrite_clicks] Rewrite assets and/or links, inject creative TSJS ├─ Add ext.orchestrator metadata └─ Set consent and optional EID response headers ``` @@ -638,8 +638,11 @@ HTML rewriter (`lol_html`) that converts eligible external resource and click URLs to signed first-party paths, adds `data-tsclick`, rewrites inline CSS `url(...)` values, removes bidder-supplied `` elements, and injects the unified creative TSJS runtime exactly once, whether or not the bidder supplied a -`` element. In every mode, a creative -larger than the 1 MiB per-creative cap is rejected and its `adm` is dropped. +`` element. `rewrite_clicks` (unset by default, following +`rewrite_creatives`) controls click-through link wrapping separately; see +[Creative Processing](/guide/creative-processing#assets-and-clicks). In every +mode, a creative larger than the 1 MiB per-creative cap is rejected and its +`adm` is dropped. ```toml [auction] @@ -702,6 +705,7 @@ Each proxied URL includes a `tstoken` HMAC signature for tamper protection. See enabled = true sanitize_creatives = false # Opt-in; blanks script-based creatives when enabled rewrite_creatives = true +# rewrite_clicks = true # Unset follows rewrite_creatives timeout_ms = 2000 mediator = "adserver_mock" @@ -835,7 +839,9 @@ The typed `ts config validate`, `ts config diff`, and `ts config push` flows can override existing scalar leaves. The pinned EdgeZero loader does not create missing leaves or replace arrays, tables, maps, or rules. Existing configs must add `rewrite_creatives = true` and `sanitize_creatives = false` before relying on -those scalar overrides. Edit and re-push TOML for other values. Provider map +those scalar overrides. `rewrite_clicks` is unset by default; add +`rewrite_clicks = true` or `false` under `[auction]` before relying on +`TRUSTED_SERVER__AUCTION__REWRITE_CLICKS`. Edit and re-push TOML for other values. Provider map keys preserve hyphens, so `pbs-main` uses the `PBS-MAIN` segment and needs `env` shell syntax: @@ -850,9 +856,11 @@ env 'TRUSTED_SERVER__AUCTION__ENABLED=true' \ ``` Before rolling back to a binary that does not know a creative-processing field, -remove that field's non-default value (`rewrite_creatives = false` or -`sanitize_creatives = true`), push the default-compatible blob, and then roll -back. See [Configuration](/guide/configuration#auction-configuration) for the +remove that field's non-default value (`rewrite_creatives = false`, +`sanitize_creatives = true`, or any explicit `rewrite_clicks`), push the default-compatible blob, and then roll +back. Older binaries tie click wrapping to `rewrite_creatives`, so rolling back +also changes link wrapping when `rewrite_clicks` was set. See +[Configuration](/guide/configuration#auction-configuration) for the complete migration, upgrade-sequencing, and rollback guidance. ## Floor Prices diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 98e375627..38035c228 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -986,13 +986,14 @@ before pushing the config. ## URL Rewrite Configuration -Control which domains are excluded from first-party rewriting. +Control which hosts first-party rewriting covers. ### `[rewrite]` -| Field | Type | Required | Description | -| ----------------- | ------------- | ---------------- | ------------------------- | -| `exclude_domains` | Array[String] | No (default: []) | Domains to skip rewriting | +| Field | Type | Required | Description | +| ----------------- | ------------- | ---------------- | --------------------------------------------------------------------------------------------------- | +| `exclude_domains` | Array[String] | No (default: []) | Hosts never rewritten, for assets and click-through links | +| `include_domains` | Array[String] | No (default: []) | When non-empty, the only hosts whose asset URLs are proxied. Does not apply to click-through links. | **Example**: @@ -1003,12 +1004,14 @@ exclude_domains = [ "first-party.publisher.com", # Exact match "localhost", # Development ] +include_domains = ["assets.example.com", "*.img.example.com"] ``` **Environment Override**: -EdgeZero v0.0.4 cannot replace this array or address its elements by index. Edit -`exclude_domains` in TOML, then validate and push the file again. +EdgeZero v0.0.4 cannot replace these arrays or address their elements by index. +Edit `exclude_domains` and `include_domains` in TOML, then validate and push the +file again. ### Pattern Matching @@ -1037,6 +1040,49 @@ Matches: - ❌ `www.api.example.com` - ❌ `api.example.com.evil.com` +**Normalization and precedence**: + +- Both lists are trimmed and lowercased at load, so matching is case-insensitive. +- `exclude_domains` wins when a host matches both lists. +- Each `include_domains` entry must be able to equal a URL host. After + lowercasing, an entry must be one of: + - a host name of ASCII letters, digits, `-` and `.`, optionally prefixed with + `*.`, with no leading or trailing `.` and no empty label; + - an IPv4 address in dotted-quad form, such as `192.0.2.1`; + - an IPv6 address in brackets, written the way URL hosts render it, such as + `[2001:db8::1]` or `[::ffff:c000:201]` (not `[::ffff:192.0.2.1]`). +- Settings load and `ts config validate` reject any other `include_domains` + entry: an empty entry, a bare `*`, a `*` other than a leading `*.`, a scheme + (`https://`), a port, a path, spaces, an unbracketed IPv6 address, or any + entry a URL parser would rewrite or reject, such as an invalid `xn--` label. + Non-ASCII (internationalized) names are rejected too; write them in punycode, + such as `xn--bcher-kva.example`. +- Empty and bare `*` entries in `exclude_domains` are dropped with a warning, + because they never match a host. Other `exclude_domains` entries that can + never match a host (for example `https://cdn.example.com` or + `cdn.example.com:443`) are kept but logged with a warning at load; fix them, + or their hosts stay proxied. + +::: tip +When both `include_domains` and `proxy.allowed_domains` are set, keep every +`include_domains` host inside `proxy.allowed_domains`. Otherwise a listed asset +is rewritten to a proxy URL that then fails with `403`. +::: + +::: warning Upgrade sequencing and rollback for `include_domains` +An empty `include_domains` is omitted from stored JSON. A non-empty list is +serialized, and older binaries reject a blob that carries it because `[rewrite]` +uses `deny_unknown_fields`. + +**Upgrading:** deploy the binary **first**, then add `include_domains` to +`trusted-server.toml`, run `ts config validate`, and push. + +**Rolling back:** remove `include_domains` from the TOML, run +`ts config validate`, push the resulting blob, and only then roll back the +binary. Rolling back the binary also restores case-sensitive `exclude_domains` +matching. +::: + ### Use Cases **Trusted Partners**: @@ -1927,15 +1973,16 @@ provider or bidder route. ### `[auction]` -| Field | Type | Default | Description | -| ---------------------- | ------- | ------------------ | -------------------------------------------------------------- | -| `enabled` | Boolean | `false` | Enable the auction orchestrator | -| `sanitize_creatives` | Boolean | `false` | Strip executable markup from winning-bid `adm` before delivery | -| `rewrite_creatives` | Boolean | `true` | Rewrite winning-bid `adm` through first-party endpoints | -| `timeout_ms` | Integer | `2000` | Logical auction budget in milliseconds | -| `mediator` | String | `None` | Optional separate `adserver_mock` mediator | -| `creative_store` | String | `"creative_store"` | Deprecated; creatives are delivered inline | -| `allowed_context_keys` | Array | `[]` | Request context keys admitted into the auction | +| Field | Type | Default | Description | +| ---------------------- | ------- | ------------------ | ------------------------------------------------------------------------------ | +| `enabled` | Boolean | `false` | Enable the auction orchestrator | +| `sanitize_creatives` | Boolean | `false` | Strip executable markup from winning-bid `adm` before delivery | +| `rewrite_creatives` | Boolean | `true` | Rewrite winning-bid asset URLs through first-party endpoints | +| `rewrite_clicks` | Boolean | unset | Wrap creative links in `/first-party/click`; unset follows `rewrite_creatives` | +| `timeout_ms` | Integer | `2000` | Logical auction budget in milliseconds | +| `mediator` | String | `None` | Optional separate `adserver_mock` mediator | +| `creative_store` | String | `"creative_store"` | Deprecated; creatives are delivered inline | +| `allowed_context_keys` | Array | `[]` | Request context keys admitted into the auction | Creative markup delivered by `POST /auction` and the publisher SSAT/page-bids path is processed by two independent passes. With `sanitize_creatives = true` @@ -1943,23 +1990,29 @@ path is processed by two independent passes. With `sanitize_creatives = true` and event handlers) is stripped together with its inner content. This blanks script-based creatives, so enable it only when creatives render in a context that shares the publisher's origin. With `rewrite_creatives = true` (the -default), eligible absolute or protocol-relative resource and click URLs not -excluded by rewrite configuration are converted to signed first-party -endpoints, and any bidder-supplied `` element is removed. The -`POST /auction` path emits root-relative endpoints and injects the creative TSJS -runtime exactly once, whether or not the bidder supplied a ``, since bare +default), eligible absolute or protocol-relative asset URLs not excluded by +rewrite configuration are converted to signed `/first-party/proxy` endpoints. +`rewrite_clicks` controls click-through links (``, ``) +separately: when it is unset, it follows `rewrite_creatives`, so existing +configs keep their current behavior. Whenever either setting is on, a bidder-supplied `` element +loses its `href`; it keeps a `target` and is removed when it has none. The `POST /auction` path emits +root-relative endpoints and, when either setting is on, injects the creative +TSJS runtime exactly once, whether or not the bidder supplied a ``, since bare fragments are the common `adm` shape. The foreign-origin SSAT renderer emits -absolute endpoints and does not inject that bundle. With both disabled, `adm` -ships exactly as the bidder returned it, except that a creative larger than the +absolute endpoints and does not inject that bundle. With all three disabled, +`adm` ships exactly as the bidder returned it, except that a creative larger than the 1 MiB per-creative cap is rejected in every mode and its `adm` is dropped. -Accepted external URLs are not host allowlisted by the sanitizer. Neither -setting affects HTML or CSS fetched through `/first-party/proxy`. See +Accepted external URLs are not host allowlisted by the sanitizer. +`rewrite_creatives` and `sanitize_creatives` do not affect HTML or CSS fetched +through `/first-party/proxy`. An explicitly set `rewrite_clicks` does apply to +links in proxied HTML; unset, proxied HTML keeps wrapping links. See [Creative Processing](/guide/creative-processing#auction-rewrite-control). ::: warning Existing configs, upgrade sequencing, and rollback Default values are omitted from stored JSON; non-default values -(`sanitize_creatives = true`, `rewrite_creatives = false`) are serialized, and -older `AuctionConfig` schemas reject unknown fields. +(`sanitize_creatives = true`, `rewrite_creatives = false`, and any explicit +`rewrite_clicks`) are serialized, and older `AuctionConfig` schemas reject +unknown fields. **Upgrading:** binaries that predate `sanitize_creatives` reject a blob that carries it, so in a rolling deployment upgrade the binary **first**, then push @@ -1973,14 +2026,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 @@ -2168,8 +2227,9 @@ finishes local decision and delivery. An auction can therefore exceed its configured wall-clock timeout. Creative sanitization is opt-in. `sanitize_creatives = true` strips executable -markup before delivery. `rewrite_creatives = false` skips first-party URL -rewriting and creative TSJS injection. See +markup before delivery. `rewrite_creatives = false` leaves asset URLs +direct; links still follow `rewrite_clicks`, and creative TSJS is still +injected when `rewrite_clicks = true`. See [Creative Processing](/guide/creative-processing#auction-rewrite-control). **Environment overrides** replace map leaves that already exist in TOML: diff --git a/docs/guide/creative-processing.md b/docs/guide/creative-processing.md index 10dcb62d5..cb5a58856 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,45 @@ 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. +`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 the guard still installs its listeners but never +acts, because no link carries `data-tsclick`. + +With `rewrite_creatives = false` and `rewrite_clicks = true`, `POST /auction` +still injects TSJS, but this does not reopen asset proxying in practice: +`/auction` creatives render in a sandbox without `allow-same-origin`, so the +render guard skips `/first-party/sign` and script-inserted assets load direct. +`/first-party/sign` itself is not gated by `rewrite_creatives`. + +Whenever either switch is on, a bidder `` loses its `href` and keeps +`target`; a `` without `target` is removed. With `rewrite_creatives = +false` and `rewrite_clicks = true`, relative asset URLs therefore no longer +resolve against the bidder base, so bidders should use absolute asset URLs. + ::: info Runtime protections inside the sandboxed creative iframe Creatives rendered by Trusted Server's own path run in a sandboxed iframe **without** `allow-same-origin`, i.e. an opaque origin. The injected creative @@ -307,15 +348,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 ``` @@ -651,14 +695,16 @@ exclude_domains = [ ### Pattern Matching -**Wildcard Patterns**: `*` matches any subdomain +Matching is case-insensitive. Entries are trimmed and lowercased at load. + +**Wildcard Patterns**: `*.` matches the base domain and any subdomain ``` Pattern: *.cdn.example.com Matches: ✅ assets.cdn.example.com ✅ images.cdn.example.com - ❌ cdn.example.com (no subdomain) + ✅ cdn.example.com (base domain) ❌ cdn.example.com.evil.com (different domain) ``` @@ -698,6 +744,33 @@ exclude_domains = ["assets.publisher.com"] Skip resources already on your domain. +## Include Domains + +Limit asset rewriting to hosts you list: + +```toml +[rewrite] +include_domains = ["assets.example.com", "*.img.example.com"] +``` + +- When `include_domains` is empty (the default), every eligible asset URL is + proxied. +- When it is non-empty, only asset URLs whose host matches an entry are + rewritten to `/first-party/proxy`. Other assets keep their original URL, load + directly, and do not receive the EC ID that proxied fetches append. +- `exclude_domains` wins when a host matches both lists. +- Click-through links (``, ``) are not affected. Whether + they are wrapped in `/first-party/click` depends only on `rewrite_clicks` and + `exclude_domains`. +- HTML and CSS fetched through `/first-party/proxy` go through the same rewrite + pass and follow the same list. +- `/first-party/sign` declines off-list hosts with a non-`403` error, so the + creative runtime loads them directly. + +Patterns use the same syntax as `exclude_domains`. Entries that can never +equal a URL host, such as ones with a scheme, port, path or non-ASCII name, fail +validation; see [Pattern Matching](/guide/configuration#pattern-matching). + ## Integration Hooks ### Attribute Rewriters diff --git a/docs/guide/first-party-proxy.md b/docs/guide/first-party-proxy.md index 8b9056f0e..3e7f76f67 100644 --- a/docs/guide/first-party-proxy.md +++ b/docs/guide/first-party-proxy.md @@ -170,6 +170,8 @@ POST /first-party/sign | `href` | Complete signed proxy URL ready to use | | `base` | Original base URL (without query parameters) | +A host excluded by `rewrite.exclude_domains`, or outside a non-empty `rewrite.include_domains`, returns `502`. The creative runtime treats it as a non-policy failure and loads the URL directly. These checks run before `proxy.allowed_domains`. + A valid host outside a non-empty allowlist returns `403 Forbidden`. The creative runtime treats this response as a policy rejection and does not assign the attempted raw image or iframe URL. Network failures, malformed success responses, and non-403 errors keep the existing direct-load fallback. An empty allowlist permits every valid host. This blocked-assignment behavior applies when the runtime intercepts a `src` setter or `setAttribute` call before native assignment. Markup inserted through `innerHTML` or `document.write` may begin a native load before the `MutationObserver` requests a signature. A later `403` does not retract that existing attribute. @@ -506,7 +508,7 @@ allowed_domains = [ ::: -### URL Rewrite Exclusions +### URL Rewrite Host Lists Exclude specific domains from rewriting: @@ -520,6 +522,17 @@ exclude_domains = [ URLs matching these patterns will NOT be rewritten to `/first-party/proxy`. +Limit asset rewriting to listed hosts: + +```toml +[rewrite] +include_domains = ["assets.example.com", "*.img.example.com"] +``` + +When `include_domains` is non-empty, only asset URLs on matching hosts are +rewritten. `exclude_domains` wins when a host matches both lists, and +click-through links ignore `include_domains`. + ### Streaming vs Buffered Control whether responses are streamed or buffered: diff --git a/docs/superpowers/plans/2026-10-07-1231-creative-asset-host-allowlist.md b/docs/superpowers/plans/2026-10-07-1231-creative-asset-host-allowlist.md new file mode 100644 index 000000000..79ae8b0cc --- /dev/null +++ b/docs/superpowers/plans/2026-10-07-1231-creative-asset-host-allowlist.md @@ -0,0 +1,1898 @@ +# Creative asset host allowlist implementation plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add `[rewrite] include_domains`, an asset host allowlist for the creative rewrite pass and `/first-party/sign`, and make `exclude_domains` matching case-insensitive. + +**Architecture:** Replace `creative::to_abs` with a pure normalizer, `normalize_creative_url`, and move host policy onto `Rewrite` as `should_proxy_asset` and `should_wrap_click`, built on the existing `proxy::is_host_allowed` and `proxy::is_host_permitted`. Asset call sites go through `asset_target`, the anchor handler through `click_target`, and `/first-party/sign` pairs the normalizer with `should_proxy_asset`. The include list is then one clause in `should_proxy_asset`. + +**Tech Stack:** Rust 2024 (`trusted-server-core`), `url`, `lol_html`, `cssparser`, `serde`, `validator` 0.20. + +**Spec:** `docs/superpowers/specs/2026-10-07-1231-creative-asset-host-allowlist-design.md` + +**Status:** Validated. Every code block below was implemented on `7a0ecb4c`, driven red then green, and passed the gates in Task 7. Line numbers are against `7a0ecb4c`. + +## Global Constraints + +- Issue: [IABTechLab/trusted-server#1231](https://github.com/IABTechLab/trusted-server/issues/1231). Sibling: #1234. +- Setting: `[rewrite] include_domains`, `Vec`, `#[serde(default, skip_serializing_if = "Vec::is_empty")]`. Never named `allowed_domains`. +- Empty `include_domains` keeps today's rewritten output byte-for-byte for every existing fixture. +- `exclude_domains` wins over `include_domains`. `include_domains` never applies to `` or ``. +- Pattern syntax for both lists: exact host, or `*.example.com` matching the apex and any subdomain. Case-insensitive, dot boundary, matched against `Url::host_str()` only. +- Load-time normalization: trim and ASCII-lowercase both lists. `exclude_domains` drops `""` and `*` with a `log::warn!`. +- `include_domains` validation rejects every entry that can never equal `Url::host_str()`. After normalization an entry must be a DNS name of non-empty `[a-z0-9-]` labels with an optional leading `*.`, a dotted-quad IPv4 address, or a bracketed IPv6 address in the form `Url::host_str()` renders, such as `[::1]` or `[::ffff:c000:201]`. Every entry finishes with a round trip through `url::Host::parse`, so an entry that `url` would rewrite or reject fails. Rejected entries include `""`, `*`, `*.`, a `*` other than a leading `*.`, a scheme, a port, a path, spaces or other characters outside `[a-z0-9.-]`, a leading or trailing `.`, an empty label, an unbracketed IPv6 address, and a wildcard or malformed IP address. These fail with `invalid_rewrite_include_domain`. Non-ASCII (IDN) entries fail with `non_ascii_rewrite_include_domain` and are not converted to punycode. +- `/first-party/sign`: an excluded or off-list host returns `TrustedServerError::Proxy { message: "unsupported url" }` (`502`) for absolute and `//` input, through GET and POST. That check runs before `proxy.allowed_domains`; `403` stays reserved for `proxy.allowed_domains`. +- No JS, adapter, routing or dependency changes. +- Fictional data only: `example.com`, `example.net`, `example.org` hosts. +- Rust conventions from `AGENTS.md`: `expect("should ...")`, no `unwrap()` outside tests, no local `use` inside functions, comments above code, descriptive assertion messages, doc comments on public items. +- Tests build `Rewrite` with `Rewrite::default()` and push or extend its lists. Never write a `Rewrite { .. }` literal, and never assign a whole field on a `Default::default()` value: `clippy::field_reassign_with_default` rejects it under `-D warnings`. +- Commits: sentence case, imperative, no `fix:`-style prefixes, `--signoff -S`, no AI attribution lines. + +## Coordination with #1234 (read before Task 1) + +Both #1231 and #1234 rely on one shared refactor, reproduced verbatim in [Shared step](#shared-step). Whichever implementation PR lands first ships it. Tasks 1 and 2 are that shared step. + +Before starting, check `main`: + +```bash +git fetch origin main +git grep -n "fn normalize_creative_url" origin/main -- crates/trusted-server-core/src/creative.rs +git grep -n "fn should_proxy_asset" origin/main -- crates/trusted-server-core/src/settings.rs +``` + +- **Both found** (#1234 landed first): rebase onto `origin/main`, **skip Tasks 1 and 2**, and start at Task 3. Diff the merged code against [Shared step](#shared-step); if names differ, use the merged names and note it in the PR description. +- **Neither found:** do Tasks 1 and 2. In the PR description, say that this PR introduces the shared step and that #1234 should rebase and skip its equivalent. +- **Only one found:** stop and ask the coordinator; the split landed partially. + +## Shared step + +This section is identical in the #1231 and #1234 plans. Both PRs write this production code and this test set verbatim; whichever PR lands first ships them, and the other skips its shared-step task. `proxy::is_host_permitted` stays private in the shared step: #1231 makes it `pub(crate)` in the task that adds the include clause, and #1234 never needs it. + +`crates/trusted-server-core/src/settings.rs`, imports (`:26`). The shared step uses only `is_host_allowed`, so it imports only that; importing `is_host_permitted` here would fail `-D warnings` as an unused import. #1231's include-clause task widens the import. + +```rust +use crate::proxy::is_host_allowed; +``` + +`settings.rs`, replaces `impl Rewrite` (`:645-670`), removing `is_excluded` and its `#[allow(dead_code)]`: + +```rust +impl Rewrite { + /// Returns `true` when an asset URL on `host` should be rewritten to + /// `/first-party/proxy`. + /// + /// The host must not match [`Self::exclude_domains`]. Matching is + /// case-insensitive; see `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 attribute was re-quoted and a link also gained `data-tsclick`. Audit `exclude_domains` for mixed-case entries before upgrading. +``` + +## File map + +| File | Responsibility in this change | +| ---------------------------------------------------------------- | ----------------------------------------------------------------------------------------- | +| `crates/trusted-server-core/src/settings.rs` | `Rewrite` fields, normalization, validation, policy methods, their unit tests. | +| `crates/trusted-server-core/src/creative.rs` | `normalize_creative_url`, `asset_target`, `click_target`, call sites, rewrite-pass tests. | +| `crates/trusted-server-core/src/proxy.rs` | `is_host_permitted` visibility, `/first-party/sign` normalization and policy, sign tests. | +| `crates/trusted-server-core/src/config_payload.rs` | Blob default-omission and round-trip tests. | +| `docs/guide/*.md`, `trusted-server.example.toml`, `CHANGELOG.md` | Operator documentation. | + +Run Rust test steps with `cargo test-fastly -- `. Core tests run on `wasm32-wasip1` under Viceroy, which must be on `PATH` (`export PATH="$HOME/.cargo/bin:$PATH"` if `cargo install` put it there). A wasm test panic aborts the whole test binary, so when one test fails, the remaining tests in that binary do not run; filter to the tests you are working on, and rerun with `--nocapture` to see the panic message above the wasm backtrace. + +--- + +### Task 1: Host policy methods and case-insensitive `exclude_domains` (shared step, part 1) + +Skip if `fn should_proxy_asset` is already on `main` (see Coordination). + +**Files:** + +- Modify: `crates/trusted-server-core/src/settings.rs:26` (import), `:645-670` (`impl Rewrite`), `:2988-2990` (`normalize_deserialized`) +- Test: `settings.rs` tests, after `test_rewrite_is_excluded` (`:6523`) + +**Interfaces:** + +- Consumes: `crate::proxy::is_host_allowed(host: &str, pattern: &str) -> bool` (`proxy.rs:1242`, `pub(crate)`). +- Produces: `pub fn Rewrite::should_proxy_asset(&self, host: &str) -> bool`, `pub fn Rewrite::should_wrap_click(&self, host: &str) -> bool`, private `fn Rewrite::is_excluded_host(&self, host: &str) -> bool`, private `fn Rewrite::normalize(&mut self)`. + +- [ ] **Step 1: Write the failing tests** + +```rust + #[test] + fn rewrite_policy_matches_exclude_patterns_case_insensitively() { + let mut rewrite = Rewrite::default(); + rewrite + .exclude_domains + .extend(["cdn.example.com", "*.example.org"].map(str::to_owned)); + + for (host, expected) in [ + ("cdn.example.com", false), + ("CDN.EXAMPLE.COM", false), + ("example.org", false), + ("a.b.example.org", false), + ("evil-example.org", true), + ("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}" + ); + } + } +``` + +```rust + #[test] + fn rewrite_normalize_trims_lowercases_and_drops_inert_exclude_entries() { + let mut rewrite = Rewrite::default(); + rewrite + .exclude_domains + .extend([" CDN.Example.com ", "", "*", "*.Example.ORG"].map(str::to_owned)); + + rewrite.normalize(); + + assert_eq!( + rewrite.exclude_domains, + vec!["cdn.example.com".to_owned(), "*.example.org".to_owned()], + "should trim, lowercase, and drop empty and bare `*` entries" + ); + } +``` + +```rust + #[test] + fn rewrite_exclude_domains_match_mixed_case_entries_from_toml() { + let toml_str = crate_test_settings_str() + + r#" + [rewrite] + exclude_domains = ["CDN.Example.com"] + "#; + + let settings = Settings::from_toml(&toml_str).expect("should parse valid TOML"); + + assert!( + !settings.rewrite.should_proxy_asset("cdn.example.com"), + "should exclude a host whose config entry was written in mixed case" + ); + } +``` + +```rust + #[test] + fn settings_load_normalizes_rewrite_exclude_domains() { + let toml_str = crate_test_settings_str() + + r#" + [rewrite] + exclude_domains = [" CDN.Example.com ", "*", ""] + "#; + + let settings = Settings::from_toml(&toml_str).expect("should parse valid TOML"); + + assert_eq!( + settings.rewrite.exclude_domains, + vec!["cdn.example.com".to_owned()], + "should trim, lowercase, and drop inert entries when settings load" + ); + } +``` + +`settings_load_normalizes_rewrite_exclude_domains` is the only test here that proves `normalize` is wired into settings loading: the matcher is case-insensitive on its own, so the mixed-case TOML test would pass even without the `normalize_deserialized` call. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cargo test-fastly -- rewrite_policy_matches rewrite_normalize_trims rewrite_exclude_domains_match settings_load_normalizes` + +Expected: compile errors, `no method named should_proxy_asset found for struct settings::Rewrite` (and `should_wrap_click`, `normalize`). + +- [ ] **Step 3: Implement** + +Add the import and the methods from [Shared step](#shared-step) (`use crate::proxy::is_host_allowed;`, `should_proxy_asset`, `should_wrap_click`, `is_excluded_host`, `normalize`), and the `self.rewrite.normalize();` call. For this task only, keep `is_excluded` in place above the new methods; its two callers still exist until Task 2. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cargo test-fastly -- rewrite_policy_matches rewrite_normalize_trims rewrite_exclude_domains_match settings_load_normalizes test_rewrite_is_excluded` + +Expected: `5 passed`. + +- [ ] **Step 5: Lint** + +Run: `cargo fmt --all && cargo clippy-fastly` + +Expected: no warnings. + +- [ ] **Step 6: Commit** + +```bash +git add crates/trusted-server-core/src/settings.rs +git commit --signoff -S -m "Add rewrite host policy methods and case-insensitive exclude matching" +``` + +--- + +### Task 2: Replace `to_abs` with `normalize_creative_url` (shared step, part 2) + +Skip if `fn normalize_creative_url` is already on `main` (see Coordination). + +**Files:** + +- Modify: `crates/trusted-server-core/src/creative.rs:25-27`, `:52-75`, `:402-409`, `:639-641`, `:726-727`, `:1285-1287`, `:1497-1501` (test imports) +- Modify: `crates/trusted-server-core/src/proxy.rs:1672-1726` +- Modify: `crates/trusted-server-core/src/settings.rs` (remove `is_excluded` and `test_rewrite_is_excluded`) +- Test: `creative.rs` tests (`:1699-1738`, `:3418-3504`), `proxy.rs` `proxy_sign_rejects_excluded_urls` (`:2692-2715`) + +**Interfaces:** + +- Consumes: `Rewrite::should_proxy_asset`, `Rewrite::should_wrap_click` from Task 1. +- Produces: `pub(super) fn normalize_creative_url(url: &str) -> Option` (visible crate-wide, as `to_abs` was); private `fn asset_target(settings: &Settings, raw: &str) -> Option` and `fn click_target(settings: &Settings, raw: &str) -> Option`. Signatures of `proxy_if_abs`, `proxied_attr_value`, `rewrite_srcset`, `CssUrlRewriter::rewrite` and the `build_*_url` helpers do not change. + +- [ ] **Step 1: Write the failing tests** + +In `creative.rs` tests, change the `use super::{...}` list (`:1497-1501`) to: + +```rust + use super::{ + CreativeCssProcessor, StreamProcessor as _, normalize_creative_url, + process_auction_creative, proxy_if_abs, rewrite_creative_html, + rewrite_inline_creative_html, rewrite_srcset, rewrite_style_urls, sanitize_creative_html, + }; +``` + +Replace `to_abs_conversions` and `to_abs_preserves_port_in_protocol_relative` (`:1699-1738`) with: + +```rust + fn normalized(raw: &str) -> Option { + normalize_creative_url(raw).map(|url| url.as_str().to_owned()) + } + + #[test] + fn normalize_creative_url_conversions() { + for (raw, expected) in [ + ("//cdn.example/x", Some("https://cdn.example/x")), + ("HTTPS://cdn.example/x", Some("https://cdn.example/x")), + ("http://cdn.example/x", Some("http://cdn.example/x")), + (" //cdn.example/y ", Some("https://cdn.example/y")), + (" https://cdn.example/a ", Some("https://cdn.example/a")), + ( + "//cdn.example.com:8080/asset.js", + Some("https://cdn.example.com:8080/asset.js"), + ), + ( + "//cdn.example.com:9443/img.png", + Some("https://cdn.example.com:9443/img.png"), + ), + ("/local/x", None), + ("", None), + ("data:image/png;base64,abcd", None), + ("javascript:alert(1)", None), + ("mailto:test@example.com", None), + ("blob:xyz", None), + ("tel:+123", None), + ("about:blank", None), + ("https://exa mple.example/x", None), + ] { + assert_eq!( + normalized(raw).as_deref(), + expected, + "should normalize `{raw}` to {expected:?}" + ); + } + } +``` + +Delete `to_abs_additional_cases` (`:3418-3428`); its cases are in the table above. + +Replace `to_abs_respects_exclude_domains` and `to_abs_respects_wildcard_domains` (`:3445-3504`) with: + +```rust + #[test] + fn proxy_if_abs_respects_exclude_domains() { + let mut settings = crate::test_support::tests::create_test_settings(); + settings.rewrite.exclude_domains = vec![ + "trusted-cdn.example.com".to_owned(), + "*.example.org".to_owned(), + ]; + + for excluded in [ + "https://trusted-cdn.example.com/lib.js", + "//trusted-cdn.example.com/lib.js", + "https://example.org/cdn.js", + "//cdnjs.example.org/lib.js", + ] { + assert_eq!( + proxy_if_abs(&settings, excluded, ""), + None, + "should leave excluded URL `{excluded}` unproxied" + ); + } + for proxied in [ + "https://other-cdn.example.com/lib.js", + "//other-cdn.example.com/lib.js", + "https://notexample.org/lib.js", + ] { + assert!( + proxy_if_abs(&settings, proxied, "") + .is_some_and(|url| url.starts_with("/first-party/proxy?tsurl=")), + "should proxy non-excluded URL `{proxied}`" + ); + } + } +``` + +```rust + #[test] + fn unparseable_absolute_url_is_left_byte_identical() { + let settings = crate::test_support::tests::create_test_settings(); + let html = ""; + + let out = rewrite_creative_html(&settings, html); + + assert!( + out.contains(html), + "should leave an unparseable absolute URL untouched, including its quoting: {out}" + ); + } +``` + +The unparseable test asserts `contains`, not equality: `rewrite_creative_html` injects the TSJS script tag even when the markup has no ``, so the output is never equal to the input. + +```rust + #[test] + fn unparseable_absolute_click_url_is_left_byte_identical() { + let settings = crate::test_support::tests::create_test_settings(); + let html = "x"; + + let out = rewrite_creative_html(&settings, html); + + assert!( + out.contains(html), + "should leave an unparseable click URL untouched, including its quoting: {out}" + ); + assert!( + !out.contains("data-tsclick"), + "should not mark an unparseable link for the click guard: {out}" + ); + } + + #[test] + fn exclude_domains_match_case_insensitively_in_the_rewrite_pass() { + let mut settings = crate::test_support::tests::create_test_settings(); + settings + .rewrite + .exclude_domains + .extend(["Landing.Example.com", "CDN.example.com"].map(str::to_owned)); + let html = r#"x"#; + + let out = rewrite_creative_html(&settings, html); + + assert!( + out.contains(r#""#), + "should leave a link raw when its host matches a mixed-case entry: {out}" + ); + assert!( + out.contains(r#""#), + "should leave an asset raw when its host matches a mixed-case entry: {out}" + ); + } +``` + +These two cover the click path, which `proxy_if_abs_respects_exclude_domains` and the asset-only unparseable test do not, and mixed-case entries that bypass load-time normalization because the test sets them directly. + +In `proxy.rs`, replace `proxy_sign_rejects_excluded_urls` (`:2692-2715`): + +```rust + #[test] + fn proxy_sign_rejects_excluded_urls() { + futures::executor::block_on(async { + let mut settings = create_test_settings(); + settings.rewrite.exclude_domains = vec!["cdn.example".to_owned()]; + + for method in [&Method::GET, &Method::POST] { + for url in [ + "https://cdn.example/asset.js", + "//cdn.example/asset.js", + "https://CDN.Example/asset.js", + ] { + let req = build_proxy_sign_request( + method, + "https://edge.example/first-party/sign", + url, + ); + let err: Report = + handle_first_party_proxy_sign(&settings, &noop_services(), req) + .await + .expect_err("should reject excluded URL"); + + assert_eq!( + err.current_context().status_code(), + StatusCode::BAD_GATEWAY, + "{} should reject excluded URL `{url}` as unsupported", + method.as_str() + ); + } + } + }); + } +``` + +Directly after it, add: + +```rust + #[test] + fn proxy_sign_rejects_excluded_urls_case_insensitively() { + futures::executor::block_on(async { + let mut settings = create_test_settings(); + settings.rewrite.exclude_domains = vec!["CDN.Example".to_owned()]; + + for method in [&Method::GET, &Method::POST] { + for url in ["https://cdn.example/asset.js", "//cdn.example/asset.js"] { + let req = build_proxy_sign_request( + method, + "https://edge.example/first-party/sign", + url, + ); + let err: Report = + handle_first_party_proxy_sign(&settings, &noop_services(), req) + .await + .expect_err("should reject a host excluded by a mixed-case entry"); + + assert_eq!( + err.current_context().status_code(), + StatusCode::BAD_GATEWAY, + "{} should reject `{url}` excluded by a mixed-case entry as unsupported", + method.as_str() + ); + } + } + }); + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cargo test-fastly -- normalize_creative_url proxy_if_abs_respects unparseable_absolute exclude_domains_match_case proxy_sign_rejects_excluded` + +Expected: `error[E0432]: unresolved import super::normalize_creative_url`. To observe the runtime reds, do this in two passes. First add only `unparseable_absolute_click_url_is_left_byte_identical`, `exclude_domains_match_case_insensitively_in_the_rewrite_pass` and `proxy_sign_rejects_excluded_urls_case_insensitively`, keeping the old `use super::{...}` list; they use only existing APIs. Run `cargo test-fastly -- unparseable_absolute_click exclude_domains_match_case proxy_sign_rejects_excluded_urls_case` and watch all three fail: the anchor gains `data-tsclick`, the mixed-case-excluded link is wrapped, and sign returns `200`. Then switch the import and add the rest of Step 1, which brings the compile error above. + +- [ ] **Step 3: Implement the normalizer, helpers and call sites** + +Apply the `creative.rs` parts of [Shared step](#shared-step): `normalize_creative_url`, `asset_target`, `click_target`, the four call sites and the module doc. + +- [ ] **Step 4: Migrate `/first-party/sign`** + +Apply the `proxy.rs` parts of [Shared step](#shared-step): the new normalization block, and `target` in the `base`/`proxied` lines. `is_host_permitted` stays private until Task 4. + +- [ ] **Step 5: Remove `is_excluded`** + +Delete `Rewrite::is_excluded` (with its `#[allow(dead_code)]`) and the `test_rewrite_is_excluded` test (`settings.rs:6522-6546`); Task 1's tests cover its cases. + +Run: `git grep -nw -e to_abs -e is_excluded crates/` + +Expected: no matches. + +- [ ] **Step 6: Run the tests to verify they pass** + +Run: `cargo test-fastly -- normalize_creative_url proxy_if_abs_respects unparseable_absolute exclude_domains_match_case proxy_sign_` + +Expected: `12 passed` (the seven new or changed tests plus the five existing `proxy_sign_*` tests). + +Then prove empty-list parity with the full suite: + +Run: `cargo test-fastly` + +Expected: all pass with no edits to any existing rewrite-output assertion. If one fails, the refactor changed output; fix the code, not the assertion. + +- [ ] **Step 7: Lint** + +Run: `cargo fmt --all && cargo clippy-fastly` + +Expected: no warnings. + +- [ ] **Step 8: CHANGELOG** + +Add the [Shared step](#shared-step) CHANGELOG entry verbatim, as the first item under `## [Unreleased]` › `### Fixed`. It ships with the shared step, so Task 6 does not repeat it. + +- [ ] **Step 9: Commit** + +```bash +git add crates/trusted-server-core/src/creative.rs crates/trusted-server-core/src/proxy.rs crates/trusted-server-core/src/settings.rs CHANGELOG.md +git commit --signoff -S -m "Split creative URL normalization from rewrite host policy" -m "Replace to_abs with normalize_creative_url and route asset, click and first-party sign decisions through Rewrite::should_proxy_asset and Rewrite::should_wrap_click." +``` + +--- + +### Task 3: `include_domains` setting, validation and blob compatibility + +**Files:** + +- Modify: `crates/trusted-server-core/src/settings.rs:636-643` (`struct Rewrite`), `Rewrite::normalize`, new validator before `validate_publisher_domain` (`:3384`) +- Test: `settings.rs` tests (before `test_auction_creative_processing_defaults_when_omitted`, `:6549`), `crates/trusted-server-core/src/config_payload.rs` tests (before `disabled_rewrite_creatives_survives_blob_round_trip`, `:739`) + +**Interfaces:** + +- Consumes: `Rewrite::normalize` from Task 1. +- Produces: `pub include_domains: Vec` on `Rewrite`; private `fn validate_include_domains(patterns: &[String]) -> Result<(), ValidationError>`. + +- [ ] **Step 1: Write the failing tests** + +In `settings.rs` tests: + +```rust + #[test] + fn rewrite_exclude_domains_keep_entries_that_never_match() { + let toml_str = crate_test_settings_str() + + r#" + [rewrite] + exclude_domains = ["https://cdn.example.com", "cdn.example.com:443", "[2001:0db8::1]"] + "#; + + let settings = Settings::from_toml(&toml_str) + .expect("should load inert exclude_domains entries with a warning"); + + assert_eq!( + settings.rewrite.exclude_domains, + vec![ + "https://cdn.example.com".to_owned(), + "cdn.example.com:443".to_owned(), + "[2001:0db8::1]".to_owned(), + ], + "should keep inert exclude_domains entries unchanged" + ); + } + + #[test] + fn rewrite_include_domains_are_normalized_from_toml() { + let toml_str = crate_test_settings_str() + + r#" + [rewrite] + include_domains = ["*.CDN.Example.com", " img.example.net "] + "#; + + let settings = Settings::from_toml(&toml_str).expect("should parse valid TOML"); + + assert_eq!( + settings.rewrite.include_domains, + vec!["*.cdn.example.com".to_owned(), "img.example.net".to_owned()], + "should trim and lowercase include_domains entries" + ); + } +``` + +```rust + fn include_domains_toml(entry: &str) -> String { + crate_test_settings_str() + &format!("\n[rewrite]\ninclude_domains = [\"{entry}\"]\n") + } + + #[test] + fn rewrite_include_domains_reject_malformed_entries() { + for entry in [ + "", + " ", + "*", + "*.", + "cdn.*.example.com", + "**.example.com", + "https://cdn.example.com", + "cdn.example.com:443", + "cdn.example.com/x", + ".example.com", + "example.com.", + "cdn..example.com", + "cdn example.com", + "cdn_assets.example.com", + "::1", + "[::1", + "[2001:0db8::1]", + "[::ffff:192.0.2.1]", + "xn--zz.example", + "*.192.0.2.1", + "192.0.2", + "256.0.2.1", + ] { + let result = Settings::from_toml(&include_domains_toml(entry)); + + let err = result.expect_err(&format!("should reject include_domains entry `{entry}`")); + assert!( + format!("{err:?}").contains("invalid_rewrite_include_domain"), + "should report `{entry}` as an invalid include_domains entry: {err:?}" + ); + } + } + + #[test] + fn rewrite_include_domains_reject_non_ascii_entries_with_a_punycode_hint() { + for entry in ["bücher.example", "*.bücher.example"] { + let result = Settings::from_toml(&include_domains_toml(entry)); + + let err = result.expect_err(&format!("should reject non-ASCII entry `{entry}`")); + assert!( + format!("{err:?}").contains("non_ascii_rewrite_include_domain"), + "should report `{entry}` as non-ASCII: {err:?}" + ); + } + } + + #[test] + fn rewrite_include_domains_accept_hosts_and_ip_literals() { + for entry in [ + "cdn.example.com", + "*.example.com", + "xn--bcher-kva.example", + "192.0.2.1", + "[2001:db8::1]", + "[::1]", + "[::ffff:c000:201]", + ] { + let settings = + Settings::from_toml(&include_domains_toml(entry)).unwrap_or_else(|err| { + panic!("should accept include_domains entry `{entry}`: {err:?}") + }); + + assert_eq!( + settings.rewrite.include_domains, + vec![entry.to_owned()], + "should keep accepted entry `{entry}` as written" + ); + } + } +``` + +```rust + #[test] + fn rewrite_include_domains_default_to_empty() { + let settings = create_test_settings(); + + assert!( + settings.rewrite.include_domains.is_empty(), + "should default include_domains to an empty list" + ); + } +``` + +In `config_payload.rs` tests: + +```rust + #[test] + fn legacy_blob_without_rewrite_include_domains_loads_with_empty_list() { + let data = + serde_json::to_value(test_settings()).expect("should serialize settings to JSON"); + let rewrite = data + .get("rewrite") + .and_then(serde_json::Value::as_object) + .expect("should serialize rewrite settings as an object"); + assert!( + !rewrite.contains_key("include_domains"), + "should omit the default include list from the payload" + ); + + let reconstructed = load_settings(&envelope_json(&test_settings())) + .expect("should reconstruct settings without include_domains"); + + assert!( + reconstructed.rewrite.include_domains.is_empty(), + "should load an empty include list from a blob without the key" + ); + } +``` + +```rust + #[test] + fn rewrite_include_domains_survive_blob_round_trip() { + let mut original = test_settings(); + original.rewrite.include_domains = vec!["*.cdn.example.com".to_owned()]; + + let reconstructed = load_settings(&envelope_json(&original)) + .expect("should reconstruct settings with include_domains"); + + assert_eq!( + reconstructed.rewrite.include_domains, + vec!["*.cdn.example.com".to_owned()], + "should preserve the configured include list" + ); + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cargo test-fastly -- rewrite_include_domains rewrite_exclude_domains_keep legacy_blob_without_rewrite` + +Expected: compile errors, `error[E0609]: no field include_domains on type settings::Rewrite`. + +- [ ] **Step 3: Implement** + +Replace `struct Rewrite` (`settings.rs:636-643`): + +```rust +#[derive(Debug, Default, Clone, Deserialize, Serialize, Validate)] +#[serde(deny_unknown_fields)] +pub struct Rewrite { + /// Hosts never rewritten, for assets and click-through links. + /// + /// Each entry is an exact host or `*.example.com`, which matches the apex + /// and any subdomain. Matching is case-insensitive. Wins over + /// [`Self::include_domains`]. + #[serde(default)] + pub exclude_domains: Vec, + /// When non-empty, the only hosts whose asset URLs are rewritten to + /// `/first-party/proxy`. + /// + /// Same pattern syntax as [`Self::exclude_domains`]. Empty (the default) + /// proxies every eligible asset URL. Never applies to click-through links. + /// The default is omitted from serialized config so older binaries, which + /// reject unknown fields, can still read it. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[validate(custom(function = validate_include_domains))] + pub include_domains: Vec, +} +``` + +At the end of `Rewrite::normalize`, after the `exclude_domains` warning block: + +```rust + self.include_domains = self + .include_domains + .iter() + .map(|pattern| pattern.trim().to_ascii_lowercase()) + .collect(); + // An `exclude_domains` entry that can never equal a URL host keeps its + // host proxied and receiving `ts-ec`. Rejecting it would stop existing + // blobs from loading, so warn once per entry and keep it. + for pattern in &self.exclude_domains { + if let Err(reason) = check_include_domain(pattern) { + log::warn!( + "rewrite.exclude_domains: entry `{pattern}` never matches a URL host ({reason})" + ); + } + } +``` + +Before `validate_publisher_domain`: + +```rust +/// Rejects `rewrite.include_domains` entries that can never equal a URL host. +/// +/// Runs after [`Rewrite::normalize`], so entries are already trimmed and +/// lowercased. Entries are matched against [`Url::host_str`], so each one must +/// be: +/// +/// - a DNS name, optionally prefixed with `*.`: one or more `.`-separated, +/// non-empty labels of ASCII `a-z`, `0-9` and `-`; +/// - an IPv4 address in dotted-quad form, such as `192.0.2.1`; or +/// - an IPv6 address in brackets and compressed form, such as `[2001:db8::1]`. +/// +/// A scheme, port, path, space, leading or trailing `.`, empty label, wildcard +/// IP address or unbracketed IPv6 address is rejected with +/// `invalid_rewrite_include_domain`. A non-ASCII (IDN) name is rejected with +/// `non_ascii_rewrite_include_domain`; write it in punycode (`xn--`) form. +fn validate_include_domains(patterns: &[String]) -> Result<(), ValidationError> { + for pattern in patterns { + if !pattern.is_ascii() { + let mut err = ValidationError::new("non_ascii_rewrite_include_domain"); + err.add_param("value".into(), pattern); + err.message = Some( + "rewrite.include_domains entries must be ASCII; write internationalized domain names in punycode (xn--) form" + .into(), + ); + return Err(err); + } + if let Err(reason) = check_include_domain(pattern) { + let mut err = ValidationError::new("invalid_rewrite_include_domain"); + err.add_param("value".into(), pattern); + err.add_param("reason".into(), &reason); + err.message = Some( + "rewrite.include_domains entries must be a host name, an optional leading `*.`, a dotted-quad IPv4 address or a bracketed IPv6 address" + .into(), + ); + return Err(err); + } + } + Ok(()) +} + +/// Checks one normalized ASCII `include_domains` entry, returning why it can +/// never equal a URL host. +fn check_include_domain(pattern: &str) -> Result<(), &'static str> { + if let Some(bracketed) = pattern.strip_prefix('[') { + bracketed + .strip_suffix(']') + .and_then(|inner| inner.parse::().ok()) + .ok_or("not a bracketed IPv6 address")?; + return check_host_round_trip(pattern); + } + if pattern.parse::().is_ok() { + return Err("IPv6 address without brackets"); + } + + let name = pattern.strip_prefix("*.").unwrap_or(pattern); + if name.is_empty() { + return Err("no host"); + } + if !name.bytes().all(|byte| { + byte.is_ascii_lowercase() || byte.is_ascii_digit() || matches!(byte, b'-' | b'.') + }) { + return Err("character outside a-z, 0-9, `-` and `.`"); + } + if name.split('.').any(str::is_empty) { + return Err("leading or trailing `.` or empty label"); + } + + // A URL host whose last label is numeric is parsed as an IPv4 address, so + // such an entry only matches as an exact dotted-quad address. + let last_label = name.rsplit('.').next().unwrap_or(name); + let numeric_last_label = + last_label.bytes().all(|byte| byte.is_ascii_digit()) || last_label.starts_with("0x"); + if numeric_last_label + && (name.len() != pattern.len() || name.parse::().is_err()) + { + return Err("numeric host that is not a dotted-quad IPv4 address"); + } + check_host_round_trip(name) +} + +/// Parses `host` with the parser behind [`Url::host_str`] and requires the +/// rendered form to equal the input, so an entry that `url` rewrites (an +/// IPv4-mapped IPv6 address in dotted form, an invalid `xn--` label) or rejects +/// can never load and then silently fail to match. +fn check_host_round_trip(host: &str) -> Result<(), &'static str> { + let parsed = url::Host::parse(host).map_err(|_| "not a valid URL host")?; + if parsed.to_string() != host { + return Err("not in the form Url::host_str() renders"); + } + Ok(()) +} +``` + +The `validator` derive passes `&Vec`, which coerces to `&[String]`, so no `clippy::ptr_arg` allowance is needed. + +`validation_error_summary` reports only each error's code, so the code carries the distinction an operator needs. `non_ascii_rewrite_include_domain` says to use punycode, and `invalid_rewrite_include_domain` covers everything else. The `message` and `value`/`reason` params give the detail to anyone reading `ValidationErrors` directly. The checks are exact on purpose. Patterns are compared with `Url::host_str()`, so an entry with a scheme, a port, a path, a non-canonical IPv6 form (including dotted IPv4-mapped forms such as `[::ffff:192.0.2.1]`), an invalid `xn--` label or a non-ASCII name would load fine and then never match. The operator would see a silently empty allowlist instead of an error at `ts config push`. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cargo test-fastly -- rewrite_include_domains rewrite_exclude_domains_keep legacy_blob_without_rewrite` + +Expected: the eight new tests pass (`rewrite_exclude_domains_keep_entries_that_never_match`, `rewrite_include_domains_are_normalized_from_toml`, `rewrite_include_domains_reject_malformed_entries`, `rewrite_include_domains_reject_non_ascii_entries_with_a_punycode_hint`, `rewrite_include_domains_accept_hosts_and_ip_literals`, `rewrite_include_domains_default_to_empty`, `legacy_blob_without_rewrite_include_domains_loads_with_empty_list` and `rewrite_include_domains_survive_blob_round_trip`), together with the existing `legacy_blob_without_rewrite_*` tests. + +- [ ] **Step 5: Lint** + +Run: `cargo fmt --all && cargo clippy-fastly` + +Expected: no warnings. + +- [ ] **Step 6: Commit** + +```bash +git add crates/trusted-server-core/src/settings.rs crates/trusted-server-core/src/config_payload.rs +git commit --signoff -S -m "Add rewrite include_domains setting with validation" -m "Add the [rewrite] include_domains field with load-time normalization, strict entry validation and blob round-trip tests. Enforcement arrives in the next commit, which applies the list to asset rewriting and first-party sign." +``` + +--- + +### Task 4: Apply `include_domains` to asset rewriting and `/first-party/sign` + +**Files:** + +- Modify: `crates/trusted-server-core/src/settings.rs` (import, `Rewrite::should_proxy_asset`) +- Modify: `crates/trusted-server-core/src/proxy.rs:1228` (`is_host_permitted` visibility), decline log line +- Test: `settings.rs` tests, `creative.rs` tests (before `rewrite_html_excludes_blacklisted_domains`), `proxy.rs` tests (before `proxy_sign_inherits_the_request_scheme_for_protocol_relative_urls`) + +**Interfaces:** + +- Consumes: `Rewrite::include_domains` (Task 3); `proxy::is_host_permitted` (made `pub(crate)` in Step 3); `rewrite_creative_html`, `rewrite_inline_creative_html(settings, base_origin, markup)`, `rewrite_proxied_html`, `rewrite_css_body(settings, css) -> Result`; `build_proxy_sign_request(method, uri, target)` and `noop_services()` (existing `proxy.rs` test helpers). +- Produces: `should_proxy_asset` with the include clause. No new public items. + +- [ ] **Step 1: Write the failing tests** + +In `settings.rs` tests: + +```rust + #[test] + fn rewrite_should_proxy_asset_honors_include_list() { + let mut rewrite = Rewrite::default(); + rewrite + .include_domains + .extend(["*.example.com", "img.example.net", "*.UPPER.Example.org"].map(str::to_owned)); + rewrite + .exclude_domains + .push("blocked.example.com".to_owned()); + + for (host, expected) in [ + ("example.com", true), + ("a.b.example.com", true), + ("img.example.net", true), + ("IMG.EXAMPLE.NET", true), + ("cdn.upper.example.org", true), + ("evil-example.com", false), + ("cdn.example.org", false), + ("blocked.example.com", false), + ] { + assert_eq!( + rewrite.should_proxy_asset(host), + expected, + "should_proxy_asset(`{host}`) should be {expected}" + ); + } + for host in ["cdn.example.org", "evil-example.com", "example.com"] { + assert!( + rewrite.should_wrap_click(host), + "should wrap click for `{host}` regardless of include_domains" + ); + } + assert!( + !rewrite.should_wrap_click("blocked.example.com"), + "should not wrap a click to an excluded host" + ); + } +``` + +In `creative.rs`, add `rewrite_css_body` and `rewrite_proxied_html` to the test `use super::{...}` list, giving: + +```rust + use super::{ + CreativeCssProcessor, StreamProcessor as _, normalize_creative_url, + process_auction_creative, proxy_if_abs, rewrite_creative_html, rewrite_css_body, + rewrite_inline_creative_html, rewrite_proxied_html, rewrite_srcset, rewrite_style_urls, + sanitize_creative_html, + }; +``` + +Then add, before `rewrite_html_excludes_blacklisted_domains`: + +```rust + const ALLOWLIST_LISTED: &[&str] = &[ + "https://assets.example.com/a.css", + "https://assets.example.com/p1.png", + "https://assets.example.com/p1-2x.png", + "https://assets.example.com/a.js", + "https://assets.example.com/i.css", + "https://assets.example.com/bg.png", + "https://assets.example.com/set.png", + "https://assets.example.com/img.png", + "https://assets.example.com/lazy.png", + "https://assets.example.com/s1.png", + "https://assets.example.com/v.mp4", + "https://assets.example.com/o.swf", + "https://assets.example.com/e.swf", + "https://assets.example.com/btn.png", + "https://assets.example.com/icon.svg", + "https://assets.example.com/sprite.svg", + "https://assets.example.com/frame.html", + "https://assets.example.com/inline.png", + ]; + + const ALLOWLIST_OFF_LIST: &[&str] = &[ + "https://cdn.example.net/a.css", + "https://cdn.example.net/p2.png", + "https://cdn.example.net/a.js", + "https://cdn.example.net/i.css", + "https://cdn.example.net/bg.png", + "https://cdn.example.net/set.png", + "https://cdn.example.net/img.png", + "https://cdn.example.net/lazy.png", + "https://cdn.example.net/s2.png", + "https://cdn.example.net/v.mp4", + "https://cdn.example.net/o.swf", + "https://cdn.example.net/e.swf", + "https://cdn.example.net/btn.png", + "https://cdn.example.net/icon.svg", + "https://cdn.example.net/sprite.svg", + "https://cdn.example.net/frame.html", + "https://cdn.example.net/inline.png", + ]; + + const ALLOWLIST_CREATIVE: &str = r#" + + + + + + + + + + + + + + + + + + + + + +
+
+Offer +Area +"#; + + fn encoded_tsurl(url: &str) -> String { + format!( + "tsurl={}", + url::form_urlencoded::byte_serialize(url.as_bytes()).collect::() + ) + } + + fn allowlist_settings() -> crate::settings::Settings { + let mut settings = crate::test_support::tests::create_test_settings(); + settings.rewrite.include_domains = vec!["*.example.com".to_owned()]; + settings + } + + fn assert_allowlist_applied(label: &str, out: &str) { + for listed in ALLOWLIST_LISTED { + assert!( + out.contains(&encoded_tsurl(listed)), + "{label}: should proxy listed URL `{listed}`: {out}" + ); + } + for off_list in ALLOWLIST_OFF_LIST { + assert!( + !out.contains(&encoded_tsurl(off_list)), + "{label}: should not proxy off-list URL `{off_list}`: {out}" + ); + assert!( + out.contains(off_list), + "{label}: should keep off-list URL `{off_list}` raw: {out}" + ); + } + for landing in [ + "https://landing.example.net/offer", + "https://landing.example.org/area", + ] { + assert!( + out.contains(&format!("/first-party/click?{}", encoded_tsurl(landing))), + "{label}: should wrap click `{landing}` regardless of include_domains: {out}" + ); + } + assert_eq!( + out.matches("data-tsclick=").count(), + 2, + "{label}: should mark both click-through links" + ); + } + + #[test] + fn include_domains_limit_asset_rewriting_on_every_html_path() { + let settings = allowlist_settings(); + + assert_allowlist_applied( + "auction", + &rewrite_creative_html(&settings, ALLOWLIST_CREATIVE), + ); + assert_allowlist_applied( + "inline", + &rewrite_inline_creative_html(&settings, "https://www.example.com", ALLOWLIST_CREATIVE), + ); + assert_allowlist_applied( + "proxied", + &rewrite_proxied_html(&settings, ALLOWLIST_CREATIVE), + ); + } + + #[test] + fn include_domains_limit_css_body_rewriting() { + let settings = allowlist_settings(); + let css = r#"@import "https://assets.example.com/i.css"; +@import "https://cdn.example.net/i.css"; +.a { background: url(https://assets.example.com/bg.png); } +.b { background: url(https://cdn.example.net/bg.png); }"#; + + let out = rewrite_css_body(&settings, css).expect("should rewrite CSS body"); + + for listed in [ + "https://assets.example.com/i.css", + "https://assets.example.com/bg.png", + ] { + assert!( + out.contains(&encoded_tsurl(listed)), + "should proxy listed CSS URL `{listed}`: {out}" + ); + } + for off_list in [ + "https://cdn.example.net/i.css", + "https://cdn.example.net/bg.png", + ] { + assert!( + out.contains(off_list) && !out.contains(&encoded_tsurl(off_list)), + "should keep off-list CSS URL `{off_list}` raw: {out}" + ); + } + } +``` + +```rust + #[test] + fn host_in_both_lists_is_left_alone() { + let mut settings = allowlist_settings(); + settings.rewrite.exclude_domains = vec!["assets.example.com".to_owned()]; + let html = ALLOWLIST_CREATIVE.replace( + "", + r#"Excluded"#, + ); + + for (label, out) in [ + ("auction", rewrite_creative_html(&settings, &html)), + ( + "inline", + rewrite_inline_creative_html(&settings, "https://www.example.com", &html), + ), + ("proxied", rewrite_proxied_html(&settings, &html)), + ] { + assert!( + !out.contains("tsurl=https%3A%2F%2Fassets.example.com"), + "{label}: should not rewrite any URL on a host in both lists: {out}" + ); + for listed in ALLOWLIST_LISTED { + assert!( + out.contains(listed), + "{label}: should keep `{listed}` raw when its host is also excluded: {out}" + ); + } + assert!( + out.contains(r#"Excluded"#), + "{label}: should leave an excluded click-through link raw: {out}" + ); + assert_eq!( + out.matches("data-tsclick=").count(), + 2, + "{label}: should wrap only the two links on hosts that are not excluded: {out}" + ); + } + } +``` + +In `proxy.rs` tests: + +```rust + #[test] + fn proxy_sign_applies_rewrite_include_domains_for_get_and_post() { + struct Case { + name: &'static str, + include_domains: &'static [&'static str], + exclude_domains: &'static [&'static str], + allowed_domains: &'static [&'static str], + target: &'static str, + expected: StatusCode, + } + + let cases = [ + Case { + name: "listed absolute", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "https://img.example.com/a.png", + expected: StatusCode::OK, + }, + Case { + name: "listed protocol-relative", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "//img.example.com/a.png", + expected: StatusCode::OK, + }, + Case { + name: "off-list absolute", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "https://img.example.net/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "off-list protocol-relative", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "//img.example.net/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "host in both lists", + include_domains: &["*.example.com"], + exclude_domains: &["img.example.com"], + allowed_domains: &[], + target: "https://img.example.com/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "off include list and off proxy allowlist", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &["*.example.org"], + target: "https://img.example.net/a.png", + expected: StatusCode::BAD_GATEWAY, + }, + Case { + name: "listed but off proxy allowlist", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &["*.example.org"], + target: "https://img.example.com/a.png", + expected: StatusCode::FORBIDDEN, + }, + Case { + name: "mixed-case host", + include_domains: &["*.example.com"], + exclude_domains: &[], + allowed_domains: &[], + target: "https://IMG.Example.COM/a.png", + expected: StatusCode::OK, + }, + ]; + + let to_owned = |values: &[&str]| -> Vec { + values.iter().map(|value| (*value).to_owned()).collect() + }; + + futures::executor::block_on(async { + for case in &cases { + for method in [&Method::GET, &Method::POST] { + let label = format!("{} {}", method.as_str(), case.name); + let mut settings = create_test_settings(); + settings.rewrite.include_domains = to_owned(case.include_domains); + settings.rewrite.exclude_domains = to_owned(case.exclude_domains); + settings.proxy.allowed_domains = to_owned(case.allowed_domains); + let req = build_proxy_sign_request( + method, + "https://edge.example.com/first-party/sign", + case.target, + ); + + let status = + match handle_first_party_proxy_sign(&settings, &noop_services(), req).await + { + Ok(response) => response.status(), + Err(error) => error.current_context().status_code(), + }; + + assert_eq!( + status, case.expected, + "{label} should return {}", + case.expected + ); + } + } + }); + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run each separately, because a wasm panic stops the binary: + +```bash +cargo test-fastly -- rewrite_should_proxy_asset_honors --nocapture +cargo test-fastly -- include_domains_limit_asset --nocapture +cargo test-fastly -- include_domains_limit_css --nocapture +cargo test-fastly -- proxy_sign_applies_rewrite_include_domains --nocapture +``` + +Expected failures: + +- `should_proxy_asset(`evil-example.com`) should be false` +- `auction: should not proxy off-list URL `https://cdn.example.net/a.css`` +- `should keep off-list CSS URL `https://cdn.example.net/i.css` raw` +- `GET off-list absolute should return 502 Bad Gateway` + +`host_in_both_lists_is_left_alone` already passes here: `exclude_domains` wins without the include clause. It guards precedence after Step 3. It runs the whole `ALLOWLIST_CREATIVE`, plus an anchor on the excluded host, through all three HTML paths with `exclude_domains = ["assets.example.com"]`, so every handler is covered. It asserts that no `assets.example.com` URL is rewritten anywhere, that every listed URL stays raw, and that only the two non-excluded links are wrapped. + +The `link imagesrcset` listed candidate (`p1-2x.png`) is a different URL from the link's `href` (`p1.png`). That way the matrix proves `imagesrcset` itself is proxied while the list is set, and an `href` match cannot satisfy the assertion. + +- [ ] **Step 3: Implement the include clause** + +In `proxy.rs:1228`, widen `is_host_permitted`'s visibility only. This belongs to the include clause, not the shared step: + +```rust +pub(crate) fn is_host_permitted>(allowed_domains: &[S], host: &str) -> bool { +``` + +In `settings.rs`, widen the import: + +```rust +use crate::proxy::{is_host_allowed, is_host_permitted}; +``` + +Replace `should_proxy_asset` and its doc comment: + +```rust + /// Returns `true` when an asset URL on `host` should be rewritten to + /// `/first-party/proxy`. + /// + /// The host must not match [`Self::exclude_domains`], and must match + /// [`Self::include_domains`] when that list is non-empty. Matching is + /// case-insensitive; see `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) && is_host_permitted(&self.include_domains, host) + } +``` + +`is_host_permitted` returns `true` for an empty list, which is the "include list empty" half of the rule. + +In the `creative.rs` module docs, append to the key behaviors list after the "Non-network schemes are ignored" bullet (`creative.rs:21-22`): + +```rust +//! - When `[rewrite] include_domains` is non-empty, only asset URLs on listed +//! hosts are proxied. Links ignore it. +``` + +In `proxy.rs`, update the decline log in `handle_first_party_proxy_sign`: + +```rust + log::debug!( + "sign request for `{host}` declined: host excluded or not in rewrite.include_domains" + ); +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cargo test-fastly -- rewrite_should_proxy_asset_honors include_domains_limit host_in_both_lists proxy_sign_` + +Expected: `12 passed`. + +- [ ] **Step 5: Run the full core suite and lint** + +Run: `cargo fmt --all && cargo test-fastly && cargo clippy-fastly` + +Expected: PASS, no warnings. + +- [ ] **Step 6: Commit** + +```bash +git add crates/trusted-server-core/src/settings.rs crates/trusted-server-core/src/creative.rs crates/trusted-server-core/src/proxy.rs +git commit --signoff -S -m "Limit creative asset rewriting to rewrite include_domains" -m "Apply the include list to every asset handler and to first-party sign. Click-through links and fetch-time proxy checks are unchanged." +``` + +--- + +### Task 5: Rewrite `` once on the inline path + +On ``, both the `link[href]` handler and the standalone `[imagesrcset]` handler rewrite `imagesrcset`. On the root-relative paths (`/auction`, proxied HTML) the second pass sees `/first-party/proxy?...`, a relative URL, and leaves it alone. On the inline path the first pass emits absolute `https:///first-party/proxy?...` URLs, so the second pass proxies them again, producing duplicated `tsurl` and `tstoken` parameters. The fix removes the `imagesrcset` handling from `link[href]`. `[imagesrcset]` already rewrites it once on every element. + +**Files:** + +- Modify: `crates/trusted-server-core/src/creative.rs` (`link[href]` handler in `rewrite_creative_html_impl`, `:1220-1242`) +- Test: `creative.rs` tests, before `host_in_both_lists_is_left_alone` + +**Interfaces:** none change. + +- [ ] **Step 1: Write the failing test and the root-relative guard** + +```rust + // `imagesrcset` is rewritten only by the `[imagesrcset]` handler. When the + // `link[href]` handler also rewrote it, the inline path's absolute proxy + // URLs were proxied a second time. + #[test] + fn inline_link_imagesrcset_is_proxied_once() { + let settings = crate::test_support::tests::create_test_settings(); + let html = r#""#; + + let out = rewrite_inline_creative_html(&settings, "https://publisher.example.com", html); + + let imagesrcset = out + .split("imagesrcset=\"") + .nth(1) + .and_then(|rest| rest.split('"').next()) + .expect("should keep an imagesrcset attribute"); + assert_eq!( + imagesrcset.matches("tsurl=").count(), + 1, + "should proxy the imagesrcset candidate exactly once: {imagesrcset}" + ); + } + + #[test] + fn root_relative_link_imagesrcset_is_proxied_once() { + let settings = crate::test_support::tests::create_test_settings(); + let html = r#""#; + + for (label, out) in [ + ("auction", rewrite_creative_html(&settings, html)), + ("proxied", rewrite_proxied_html(&settings, html)), + ] { + let imagesrcset = out + .split("imagesrcset=\"") + .nth(1) + .and_then(|rest| rest.split('"').next()) + .expect("should keep an imagesrcset attribute"); + assert_eq!( + imagesrcset.matches("tsurl=").count(), + 1, + "{label}: should proxy the imagesrcset candidate exactly once: {imagesrcset}" + ); + } + } +``` + +- [ ] **Step 2: Run the tests to verify the inline one fails** + +Run each alone (a wasm panic aborts the whole test binary): + +```bash +cargo test-fastly -p trusted-server-core --lib -- --exact creative::tests::inline_link_imagesrcset_is_proxied_once --nocapture +cargo test-fastly -p trusted-server-core --lib -- --exact creative::tests::root_relative_link_imagesrcset_is_proxied_once +``` + +Expected: + +- The inline test panics with "should proxy the imagesrcset candidate exactly once: https://publisher.example.com/first-party/proxy?tsurl=https%3A%2F%2Fpublisher.example.com%2Ffirst-party%2Fproxy&tsurl=...". +- The root-relative guard passes. Those paths were never affected. + +- [ ] **Step 3: Implement** + +In the `link[href]` handler, delete the `imagesrcset` block and keep the `href` handling. A let-chain keeps `clippy::collapsible_if` quiet without depending on where a comment sits: + +```rust + // Stylesheets and preloads. `imagesrcset` is left to the + // `[imagesrcset]` handler, which also runs on this element; + // rewriting it here too would proxy the inline path's absolute + // proxy URLs a second time. + element!("link[href]", |el| { + let rel = el + .get_attribute("rel") + .unwrap_or_default() + .to_ascii_lowercase(); + if (rel.contains("stylesheet") + || rel.contains("preload") + || rel.contains("prefetch")) + && let Some(p) = + proxied_attr_value(settings, el.get_attribute("href"), base_origin) + { + let _ = el.set_attribute("href", &p); + } + Ok(()) + }), +``` + +In the `creative.rs` module docs, drop "and `imagesrcset`" from the `` bullet, which becomes ``//! - `` ``. The `[imagesrcset]` bullet above it already covers the attribute. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cargo test-fastly -p trusted-server-core --lib -- imagesrcset include_domains_limit` + +Expected: `9 passed`. This includes both new tests, `include_domains_limit_asset_rewriting_on_every_html_path` (whose inline case uses the realistic `https://www.example.com` origin), and the existing `imagesrcset` rewrite and sanitizer tests. + +- [ ] **Step 5: Run the full core suite and lint** + +Run: `cargo fmt --all && cargo test-fastly && cargo clippy-fastly` + +Expected: PASS, no warnings. + +- [ ] **Step 6: Commit** + +```bash +git add crates/trusted-server-core/src/creative.rs +git commit --signoff -S -m "Rewrite link imagesrcset once on the inline creative path" +``` + +--- + +### Task 6: Operator documentation, example config and changelog + +**Files:** + +- Modify: `trusted-server.example.toml:127-129` +- Modify: `docs/guide/configuration.md:985-1068` +- Modify: `docs/guide/creative-processing.md:637-700` +- Modify: `docs/guide/first-party-proxy.md:173`, `:509-521` +- Modify: `docs/guide/api-reference.md:565-570` +- Modify: `CHANGELOG.md` (`## [Unreleased]`: `### Fixed` at `:34`, `### Added` at `:41`; both already exist) + +- [ ] **Step 1: `trusted-server.example.toml`** + +Replace lines 127-129 with: + +```toml +# [rewrite] +# Hosts never rewritten: assets stay direct and click-through links are not +# wrapped. Supports "*.example.com" wildcards (matches the apex too). +# exclude_domains = ["*.cdn.example.com"] +# When non-empty, only asset URLs on these hosts are proxied; other assets load +# directly. Click-through links are not affected. Set this in TOML; environment +# overrides cannot replace arrays. +# include_domains = ["assets.example.com", "*.img.example.com"] +``` + +- [ ] **Step 2: `docs/guide/configuration.md`** + +- Section intro (`:989`): "Control which hosts first-party rewriting covers." +- `[rewrite]` table (`:993-995`): + +```markdown +| Field | Type | Required | Description | +| ----------------- | ------------- | ---------------- | --------------------------------------------------------------------------------------------------- | +| `exclude_domains` | Array[String] | No (default: []) | Hosts never rewritten, for assets and click-through links | +| `include_domains` | Array[String] | No (default: []) | When non-empty, the only hosts whose asset URLs are proxied. Does not apply to click-through links. | +``` + +- Add `include_domains = ["assets.example.com", "*.img.example.com"]` to the example and change the Environment Override text to "EdgeZero v0.0.4 cannot replace these arrays or address their elements by index. Edit `exclude_domains` and `include_domains` in TOML, then validate and push the file again." +- After the exact-pattern list, add: + +```markdown +**Normalization and precedence**: + +- Both lists are trimmed and lowercased at load, so matching is case-insensitive. +- `exclude_domains` wins when a host matches both lists. +- Each `include_domains` entry must be able to equal a URL host. After + lowercasing, an entry must be one of: + - a host name of ASCII letters, digits, `-` and `.`, optionally prefixed with + `*.`, with no leading or trailing `.` and no empty label; + - an IPv4 address in dotted-quad form, such as `192.0.2.1`; + - an IPv6 address in brackets and compressed form, such as `[2001:db8::1]`. +- Settings load and `ts config validate` reject any other `include_domains` + entry: an empty entry, a bare `*`, a `*` other than a leading `*.`, a scheme + (`https://`), a port, a path, spaces, or an unbracketed IPv6 address. + Non-ASCII (internationalized) names are rejected too; write them in punycode, + such as `xn--bcher-kva.example`. +- Empty and bare `*` entries in `exclude_domains` are dropped with a warning, + because they never match a host. + +::: tip +When both `include_domains` and `proxy.allowed_domains` are set, keep every +`include_domains` host inside `proxy.allowed_domains`. Otherwise a listed asset +is rewritten to a proxy URL that then fails with `403`. +::: + +::: warning Upgrade sequencing and rollback for `include_domains` +An empty `include_domains` is omitted from stored JSON. A non-empty list is +serialized, and older binaries reject a blob that carries it because `[rewrite]` +uses `deny_unknown_fields`. + +**Upgrading:** deploy the binary **first**, then add `include_domains` to +`trusted-server.toml`, run `ts config validate`, and push. + +**Rolling back:** remove `include_domains` from the TOML, run +`ts config validate`, push the resulting blob, and only then roll back the +binary. Rolling back the binary also restores case-sensitive `exclude_domains` +matching. +::: +``` + +The warning box mirrors the existing `[auction] rewrite_creatives` upgrade and rollback box (and #1234's `[auction] rewrite_clicks` box if that has landed). + +- [ ] **Step 3: `docs/guide/creative-processing.md`** + +- Wildcard example (`:656-663`): replace `❌ cdn.example.com (no subdomain)` with `✅ cdn.example.com (base domain)`. +- Under `### Pattern Matching`, add "Matching is case-insensitive. Entries are trimmed and lowercased at load." and change the wildcard lead-in to "`*.` matches the base domain and any subdomain". +- After the Exclude Domains use cases, add: + +```markdown +## Include Domains + +Limit asset rewriting to hosts you list: + + [rewrite] + include_domains = ["assets.example.com", "*.img.example.com"] + +- When `include_domains` is empty (the default), every eligible asset URL is + proxied. +- When it is non-empty, only asset URLs whose host matches an entry are + rewritten to `/first-party/proxy`. Other assets keep their original URL, load + directly, and do not receive the EC ID that proxied fetches append. +- `exclude_domains` wins when a host matches both lists. +- Click-through links (``, ``) are not affected. Whether + they are wrapped in `/first-party/click` depends only on `rewrite_clicks` and + `exclude_domains`. +- HTML and CSS fetched through `/first-party/proxy` go through the same rewrite + pass and follow the same list. +- `/first-party/sign` declines off-list hosts with a non-`403` error, so the + creative runtime loads them directly. + +Patterns use the same syntax as `exclude_domains`. Entries that can never +equal a URL host, such as ones with a scheme, port, path or non-ASCII name, fail +validation; see [Pattern Matching](/guide/configuration#pattern-matching). +``` + +If #1234 has not landed, drop the `rewrite_clicks` mention from the click-through bullet. + +(In the guide the TOML is a fenced `toml` block.) + +- [ ] **Step 4: `docs/guide/first-party-proxy.md`** + +- Rename `### URL Rewrite Exclusions` (`:509`) to `### URL Rewrite Host Lists`, and after "URLs matching these patterns will NOT be rewritten to `/first-party/proxy`." add a fenced `toml` example with `include_domains = ["assets.example.com", "*.img.example.com"]` followed by: "When `include_domains` is non-empty, only asset URLs on matching hosts are rewritten. `exclude_domains` wins when a host matches both lists, and click-through links ignore `include_domains`." +- Before "A valid host outside a non-empty allowlist returns `403 Forbidden`." (`:173`), add the paragraph: "A host excluded by `rewrite.exclude_domains`, or outside a non-empty `rewrite.include_domains`, returns `502`. The creative runtime treats it as a non-policy failure and loads the URL directly. These checks run before `proxy.allowed_domains`." + +- [ ] **Step 5: `docs/guide/api-reference.md`** + +In `/first-party/sign` Error Responses, after the `413` bullet: + +```markdown +- `502 Bad Gateway`: The target host matches `rewrite.exclude_domains` or is outside a non-empty `rewrite.include_domains` +``` + +- [ ] **Step 6: `CHANGELOG.md`** + +Append to the existing `### Fixed` list under `## [Unreleased]`. The `exclude_domains` entry already came with the shared step. + +```markdown +- Inline SSAT/page-bids creatives no longer proxy `` candidates twice. The `imagesrcset` attribute is now rewritten once, by the same handler on every creative path; previously the inline path's absolute proxy URLs were wrapped in a second `/first-party/proxy` URL. +``` + +Insert at the top of the existing `### Added` list under `## [Unreleased]`: + +```markdown +- `[rewrite] include_domains` limits creative asset rewriting to listed hosts. When non-empty, only asset URLs on matching hosts are proxied through `/first-party/proxy`, so only those hosts receive the EC ID on proxied fetches. Other assets keep their original URL, and `/first-party/sign` returns `502` for them so the creative runtime loads them directly. Click-through links are not affected, and `exclude_domains` still wins. Entries must be host names, optionally prefixed with `*.`, dotted-quad IPv4 addresses, or bracketed IPv6 addresses; entries with a scheme, port, path, spaces or non-ASCII characters are rejected at load. Upgrading: deploy the binary first, then push a config that sets `include_domains`. Rolling back: remove `include_domains`, push the resulting config, then roll back the binary; older binaries reject the key because `[rewrite]` denies unknown fields. +``` + +- [ ] **Step 7: Format and check** + +Run: `cd docs && npm ci && cd .. && docs/node_modules/.bin/prettier --config docs/.prettierrc --write docs/guide/configuration.md docs/guide/creative-processing.md docs/guide/first-party-proxy.md docs/guide/api-reference.md CHANGELOG.md` + +Expected: only `configuration.md` changes (table column alignment); the rest report `(unchanged)`. + +- [ ] **Step 8: Commit** + +```bash +git add docs/guide/configuration.md docs/guide/creative-processing.md docs/guide/first-party-proxy.md docs/guide/api-reference.md trusted-server.example.toml CHANGELOG.md +git commit --signoff -S -m "Document rewrite include_domains and case-insensitive exclusions" +``` + +--- + +### Task 7: Full CI gate + +**Files:** none. + +- [ ] **Step 1: Format and lint** + +```bash +cargo fmt --all -- --check +cargo clippy-fastly && cargo clippy-axum && cargo clippy-cloudflare && cargo clippy-cloudflare-wasm \ + && cargo clippy-spin-native && cargo clippy-spin-wasm && cargo clippy-cli && cargo clippy-codegen +``` + +Expected: all pass. + +- [ ] **Step 2: Tests** + +```bash +cargo test-fastly && cargo test-fastly-reuse && cargo test-axum && cargo test-cloudflare && cargo test-spin +cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity +./scripts/test-cli.sh +``` + +Expected on `7a0ecb4c` plus #1234's code and this change (the review-fix run): `test-fastly` 3169 passed, 10 ignored (core 2922 passed, 6 ignored); `test-fastly-reuse` 219 passed; `test-axum` 43 passed; `test-cloudflare` 53 passed; `test-spin` 87 passed; parity 17 passed; CLI 735 passed across `./scripts/test-cli.sh`. Without #1234 in front, the counts are lower by #1234's own tests. + +- [ ] **Step 3: JS and docs** + +```bash +cd crates/trusted-server-js/lib && npx vitest run && npm run format && cd - +cd docs && npm run format && cd - +docs/node_modules/.bin/prettier --config docs/.prettierrc --check "*.md" ".claude/**/*.md" ".github/**/*.md" "crates/**/*.md" "scripts/**/*.md" "tinybird/**/*.md" +``` + +Expected: all pass. JS is unchanged. + +- [ ] **Step 4: Spec conformance** + +| Criterion | Evidence | +| --------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------ | +| Empty list leaves existing rewrite assertions unchanged | Task 2 Step 6 and Task 4 Step 5: full suite, no assertion edits | +| Listed proxied, off-list raw, every handler, three HTML paths | `include_domains_limit_asset_rewriting_on_every_html_path`, `include_domains_limit_css_body_rewriting` | +| Host in both lists left alone | `host_in_both_lists_is_left_alone`, sign case "host in both lists" | +| Case-insensitive; `*.example.com` matches apex | Task 1 and Task 4 policy tests, `rewrite_exclude_domains_match_mixed_case_entries_from_toml` | +| Sign returns `502` off-list for absolute and `//`, GET and POST | `proxy_sign_applies_rewrite_include_domains_for_get_and_post` | +| Anchors wrapped regardless of allowlist | `assert_allowlist_applied` click assertions | +| Default omitted from serialized blob | `legacy_blob_without_rewrite_include_domains_loads_with_empty_list` | +| Docs, example TOML, CHANGELOG | Task 6 | + +- [ ] **Step 5: Open the PR** + +Use the `pr-creator` agent. Link the spec and #1231. State whether this PR introduced the shared step (Tasks 1-2) or rebased onto #1234's. + +## Notes + +- **Unparseable absolute URLs.** Before this change an absolute-looking value that `url::Url::parse` rejects had its attribute rewritten to its own value, re-quoted with double quotes. After, it is left untouched. `unparseable_absolute_url_is_left_byte_identical` and `unparseable_absolute_click_url_is_left_byte_identical` pin the new behavior. Against the pre-change code both were observed failing: `` came back re-quoted, and the anchor also gained `data-tsclick`. diff --git a/docs/superpowers/specs/2026-10-07-1231-creative-asset-host-allowlist-design.md b/docs/superpowers/specs/2026-10-07-1231-creative-asset-host-allowlist-design.md new file mode 100644 index 000000000..3efea3bdd --- /dev/null +++ b/docs/superpowers/specs/2026-10-07-1231-creative-asset-host-allowlist-design.md @@ -0,0 +1,430 @@ +# Creative asset host allowlist + +**Issue:** [IABTechLab/trusted-server#1231](https://github.com/IABTechLab/trusted-server/issues/1231) + +**Date:** 2026-10-07 + +**Status:** Draft (design validated by an exploratory implementation; see the plan, `docs/superpowers/plans/2026-10-07-1231-creative-asset-host-allowlist.md`) + +**Related:** [#1234](https://github.com/IABTechLab/trusted-server/issues/1234) (separate click-rewriting switch). Both issues replace `to_abs`; see [Coordination with #1234](#coordination-with-1234). + +Line numbers below are against `main` at `7a0ecb4c`. + +## Problem + +The creative rewrite pass proxies every absolute HTTP(S) asset URL in a creative through `/first-party/proxy` unless its host matches `[rewrite] exclude_domains`. An operator cannot say "proxy only these hosts". + +That matters for more than egress control. A proxied fetch appends the visitor's EC ID to the upstream URL (`append_ec_id`, `crates/trusted-server-core/src/proxy.rs:1204`, called at `proxy.rs:793`), so today every third-party asset host that appears in a winning creative receives `ts-ec`. A deny list cannot bound that set, because creatives name new hosts all the time. + +`proxy.allowed_domains` does not solve it. It is the fetch-time SSRF control, shared with the Prebid external bundle fetch. With it set, the rewriter still turns off-list URLs into `/first-party/proxy` links, and the fetch then fails with `403`. The asset breaks instead of loading directly. + +A second, smaller defect sits in the same code: `Rewrite::is_excluded` compares raw config entries against a host the `url` crate has already lowercased, so an entry written with an uppercase letter never matches. + +## Goals + +- Add an asset host allowlist, `[rewrite] include_domains`. Empty (the default) keeps today's behavior. +- When non-empty, rewrite only asset URLs whose host matches the list. Every other asset URL keeps its original bytes. +- `exclude_domains` wins when a host matches both lists. +- Apply the allowlist to every asset handler in the pass, on all three entry points, including `srcset` candidates and CSS `url()`, `image-set()` strings and `@import`. +- Never apply the allowlist to anchors (``, ``). +- Apply the same policy at `/first-party/sign` for absolute and protocol-relative input, returning the same non-`403` error excluded hosts already get. +- Use one case-insensitive host matcher for both rewrite lists, which also fixes `exclude_domains` case sensitivity. +- Keep older binaries able to read config blobs written with the default. +- Fix the inline path's double rewrite of ``, so each candidate is proxied exactly once on every path. + +## Non-goals + +- Click rewriting policy or a click switch. That is #1234. +- Domain checks at `/first-party/proxy`, `/first-party/click` or `/first-party/proxy-rebuild`. Fetch-time SSRF stays with `proxy.allowed_domains`, and the click endpoints redirect any validly signed target by design (`proxy.rs:1898-1907`). +- Narrowing EC forwarding on click redirects (`handle_first_party_click`, `proxy.rs:1545`) or on proxy redirect hops. The allowlist only decides which asset URLs become proxy URLs. +- Cross-validating `include_domains` against `proxy.allowed_domains`. The docs recommend keeping the allowlist inside the proxy allowlist; a load-time warning can be a follow-up. +- Client-side changes. The render guard asks `/first-party/sign` and follows its answer. +- Environment-overlay support for the list. EdgeZero overlays only replace scalar leaves that already exist in the TOML, so the list is set in `trusted-server.toml` and published with `ts config push`, like `exclude_domains`. + +## Current behavior + +### The rewrite pass + +`rewrite_creative_html_impl` (`crates/trusted-server-core/src/creative.rs:1125`) is one lol_html pass. Its asset handlers live at `creative.rs:1180-1339`: + +| Handler | Lines | Per-URL helper | +| ------------------------------------------------------------------ | --------- | -------------------------------------- | +| `img` `src`, `data-src` | 1180-1192 | `proxy_if_abs` | +| `script[src]` | 1194-1201 | `proxied_attr_value` → `proxy_if_abs` | +| `link[href]` (stylesheet, preload, prefetch) and its `imagesrcset` | 1203-1225 | `proxied_attr_value`, `rewrite_srcset` | +| `video`, `audio`, `source` `src` | 1227-1234 | `proxied_attr_value` | +| `object[data]`, `embed[src]` | 1236-1251 | `proxied_attr_value` | +| `input[type=image]` | 1253-1267 | `proxied_attr_value` | +| SVG `image`/`use` `href`, `xlink:href` | 1269-1281 | `proxied_attr_value` | +| `[style]` attribute | 1294-1302 | `CssUrlRewriter::rewrite` | +| `