Skip to content

Commit ba6e2bf

Browse files
ChristianPavilonisChristianPavilonis
committed
Merge PR #956 (creative sanitization optional) into rc/july
2 parents eb51d69 + 3d9e2b6 commit ba6e2bf

14 files changed

Lines changed: 683 additions & 83 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313
- **Breaking** — `bid_param_zone_overrides` inner values must now be JSON objects; previously non-object or empty values (`"header" = "x"`, `"header" = {}`) were accepted and silently produced a dead rule at runtime. They now fail at startup with a configuration error. Operators upgrading should audit their `bid_param_zone_overrides` config for non-object zone entries.
1414
- **Breaking** — Integration configuration strings are no longer globally reinterpreted as JSON scalars. Operators upgrading should audit `[integrations.*]` settings and use native TOML/typed-config booleans and numbers (for example, `enabled = true`, not `enabled = "true"`); quoted numeric and boolean scalars now fail validation instead of silently converting.
1515
- **Breaking** — Sourcepoint browser module inclusion now requires explicit `[integrations.sourcepoint].enabled = true`; operators relying on the previous unconditional Sourcepoint module should enable the integration before upgrading.
16-
- **Breaking** — Auction creative sanitization is now opt-in: the new `[auction].sanitize_creatives` defaults to `false` because unconditional sanitization blanked script-based creatives (the majority of programmatic display) while recording normal impressions. `[auction].rewrite_creatives` keeps its `true` default. The 1 MiB per-creative cap is now enforced in every processing mode, and a supplied creative that processing rejects no longer falls back to PBS Cache coordinates (which would deliver the raw cached `adm`). The creative iframe sandbox no longer grants `allow-same-origin`, restoring origin isolation; rewritten-click recovery from the resulting opaque-origin iframe uses the GET `/first-party/proxy-rebuild` navigation fallback, now registered in every adapter, and dynamic resource signing inside those iframes is disabled pending [#982](https://github.com/IABTechLab/trusted-server/issues/982). Upgrading: binaries that predate `sanitize_creatives` reject a blob carrying it, so upgrade the binary first, then push the config. Rollback: non-default values (`sanitize_creatives = true`, `rewrite_creatives = false`) are serialized into the config blob and older binaries reject unknown fields — before rolling back to a binary that predates a field, restore its default, push the default-compatible blob, then roll back.
16+
- **Breaking** — Auction creative sanitization is now opt-in: the new `[auction].sanitize_creatives` defaults to `false` because unconditional sanitization blanked script-based creatives (the majority of programmatic display) while recording normal impressions. `[auction].rewrite_creatives` keeps its `true` default. The 1 MiB cap is now enforced on rewritten output as well as raw input and in every processing mode, rewriting fails closed on parser errors instead of emitting partial output, and `hb_cache_host`/`hb_cache_path` are emitted only for bids that supplied no creative — any bid carrying its own `adm` ships without them, so a processed or rejected creative can never be re-fetched raw from PBS Cache. Creative markup with no `<body>` token now receives the click-guard runtime, and bidder `<base>` elements are stripped whenever rewriting is enabled. The creative iframe sandbox no longer grants `allow-same-origin`, restoring origin isolation; rewritten-click recovery from the resulting opaque-origin iframe uses the GET `/first-party/proxy-rebuild` navigation fallback, now registered in every adapter, and dynamic resource signing inside those iframes is disabled pending [#982](https://github.com/IABTechLab/trusted-server/issues/982). Upgrading: binaries that predate `sanitize_creatives` reject a blob carrying it, so upgrade the binary first, then push the config. Rollback: non-default values (`sanitize_creatives = true`, `rewrite_creatives = false`) are serialized into the config blob and older binaries reject unknown fields — before rolling back to a binary that predates a field, restore its default, push the default-compatible blob, then roll back.
1717
- Added optional APS `inventory_domain` and `inventory_page_origin` overrides for deployments whose edge hostname differs from the APS-authorized inventory identity.
1818
- Preserved APS renderer capabilities through the client-side `trustedServer` Prebid adapter, allowing its generated `hb_adid` to render through GAM and Prebid Universal Creative instead of producing an empty creative.
1919
- The SPA re-auction endpoint moved from `/__ts/page-bids` to `/_ts/page-bids`, joining every other internal route in the `/_ts/` namespace. The old path stays registered as a deprecated alias so already-loaded bundles keep serving ads, and responses on it carry a `Link: …; rel="deprecation"` header so remaining traffic is measurable from edge logs; removal is tracked in [#970](https://github.com/IABTechLab/trusted-server/issues/970). Two deployment notes: audit `[[handlers]]` for patterns broad enough to cover `/_ts` (for example `^/_ts`), which would put this browser-facing endpoint behind Basic Auth and return `401` to every visitor — scope them to `^/_ts/admin`; and prefer rolling forward over rolling back, since a server reverted past this release does not register the canonical path. In both cases the shipped client falls back to the deprecated alias, so the exposure is bounded until that alias is removed.
@@ -33,8 +33,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3333
- Added opt-in APS HTTP debug metadata for controlled test sites, exposing the direct request and response under `/auction` provider metadata using the Prebid Server `debug.httpcalls` shape.
3434
- Added typed APS renderer transport for direct auctions and GAM/Prebid Universal Creative, using a minimized one-bid envelope, a fragment-bound nonce, and an opaque sandboxed renderer endpoint.
3535
- Added the `[auction].rewrite_creatives` (default `true`) and `[auction].sanitize_creatives` (default `false`) options. `rewrite_creatives` rewrites winning-bid adm to first-party endpoints across `POST /auction` and publisher SSAT/page-bids delivery (proxy/click URL conversion, bidder `<base>` removal; creative TSJS injection on `POST /auction` only). Enabling `sanitize_creatives` strips executable markup from winning-bid adm before delivery.
36-
- Added opt-in APS HTTP debug metadata for controlled test sites, exposing the direct request and response under `/auction` provider metadata using the Prebid Server `debug.httpcalls` shape.
37-
- Added typed APS renderer transport for direct auctions and GAM/Prebid Universal Creative, using a minimized one-bid envelope, a fragment-bound nonce, and an opaque sandboxed renderer endpoint.
3836
- `creative_opportunities.slot.gam_unit_path` is now a template supporting `{network_id}`, `{slot_id}`, and `{section}`, so a publisher whose ad unit varies by site section expresses it in one slot rule instead of one per (slot × section). `{section}` derives from the request path: `[creative_opportunities].section_segment` selects which path segment names the section (0-based, default `0`; set `1` for locale-prefixed URLs), and `section_root` supplies the value for paths with no such segment. `section_root` is required when a template uses `{section}`. Existing static and absent `gam_unit_path` configs are unchanged. Startup now also rejects a blank `gam_network_id` when slots are configured. Note that a config setting `section_root` or `section_segment` requires a binary that knows those keys — rolling the binary back below this release while the keys are present fails config load; configs that omit them roll back cleanly.
3937
- Added Osano consent mirror integration docs and public enablement guidance.
4038
- Implemented basic authentication for configurable endpoint paths (#73)

‎crates/trusted-server-adapter-spin/tests/routes.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -520,8 +520,10 @@ async fn first_party_proxy_rebuild_is_routed() {
520520
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
521521
async fn first_party_proxy_rebuild_get_is_routed() {
522522
// The opaque-origin creative click guard recovers via GET navigation, so the
523-
// route must be registered for GET (the handler 302s) and must not fall
524-
// through to the publisher origin.
523+
// route must be registered for GET and must not fall through to the
524+
// publisher origin. This asserts routing only; the 302 and its rebuilt
525+
// Location are covered by `proxy_rebuild_get_with_origin_form_uri_redirects`
526+
// in the core crate, which can sign a real `tsclick`.
525527
let router = test_router();
526528
let req = request_builder()
527529
.method("GET")

‎crates/trusted-server-core/src/creative.rs‎

Lines changed: 130 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,16 @@ fn rewrite_creative_html_impl(
652652
) -> String {
653653
// No size parsing needed now; all absolute/protocol-relative URLs are proxied uniformly.
654654
let mut out = Vec::with_capacity(markup.len() + 64);
655-
let injected_ts_creative = std::cell::Cell::new(false);
655+
// Shared with the `body` handler through an `Rc` so the outcome is readable
656+
// here after rewriting: cloning a bare `Cell` would hand the handler an
657+
// independent copy and always report "not injected".
658+
let injected_ts_creative = std::rc::Rc::new(std::cell::Cell::new(false));
659+
// Rewriting amplifies: every short URL becomes a signed proxy/click URL and
660+
// anchors gain a `data-tsclick` copy, so an input comfortably under the cap
661+
// can expand well past it. Bound the OUTPUT too, and stop accumulating once
662+
// the limit trips, so a bidder cannot drive unbounded allocation in the
663+
// WASM runtime by packing a creative with URL-bearing elements.
664+
let overflowed = std::cell::Cell::new(false);
656665
let mut rewriter = HtmlRewriter::new(
657666
HtmlSettings {
658667
element_content_handlers: vec![
@@ -668,7 +677,7 @@ fn rewrite_creative_html_impl(
668677
}),
669678
// Inject unified tsjs bundle at the top of body once
670679
element!("body", {
671-
let injected = injected_ts_creative.clone();
680+
let injected = std::rc::Rc::clone(&injected_ts_creative);
672681
move |el| {
673682
if inject_tsjs && !injected.get() {
674683
let script_tag = tsjs::tsjs_unified_script_tag();
@@ -842,12 +851,62 @@ fn rewrite_creative_html_impl(
842851
],
843852
..HtmlSettings::default()
844853
},
845-
|c: &[u8]| out.extend_from_slice(c),
854+
|c: &[u8]| {
855+
if overflowed.get() {
856+
return;
857+
}
858+
if out.len() + c.len() > MAX_CREATIVE_SIZE {
859+
overflowed.set(true);
860+
out.clear();
861+
out.shrink_to_fit();
862+
return;
863+
}
864+
out.extend_from_slice(c);
865+
},
846866
);
847867

848-
let _ = rewriter.write(markup.as_bytes());
849-
let _ = rewriter.end();
850-
String::from_utf8(out).unwrap_or_else(|_| markup.to_owned())
868+
// Fail closed on parser or output-limit failures, matching the sanitizer:
869+
// a partially rewritten document has an unknown mix of mediated and direct
870+
// URLs, and truncated markup can reopen tags the rewriter had closed.
871+
// Do not call end() after a failed write — lol_html's rewriter is in an
872+
// error state and may emit garbage.
873+
if rewriter.write(markup.as_bytes()).is_err() || rewriter.end().is_err() {
874+
log::warn!("rewrite_creative_html: html rewrite failed; rejecting creative");
875+
return String::new();
876+
}
877+
if overflowed.get() {
878+
log::warn!(
879+
"rewrite_creative_html: rewritten creative exceeds {} byte cap; rejecting",
880+
MAX_CREATIVE_SIZE
881+
);
882+
return String::new();
883+
}
884+
885+
let mut rewritten = match String::from_utf8(out) {
886+
Ok(rewritten) => rewritten,
887+
Err(_) => {
888+
log::warn!("rewrite_creative_html: rewriter emitted non-UTF-8 output; rejecting");
889+
return String::new();
890+
}
891+
};
892+
893+
// Creative `adm` is frequently a bare fragment (`<a>…</a><script>…</script>`)
894+
// with no `<body>` token for the handler above to match, and lol_html does
895+
// not synthesize one. Without this fallback such fragments would ship
896+
// without the click guard, leaving rewritten links unmediated once bidder
897+
// script mutates them.
898+
if inject_tsjs && !injected_ts_creative.get() {
899+
rewritten.insert_str(0, &tsjs::tsjs_unified_script_tag());
900+
if rewritten.len() > MAX_CREATIVE_SIZE {
901+
log::warn!(
902+
"rewrite_creative_html: creative exceeds {} byte cap after runtime injection; rejecting",
903+
MAX_CREATIVE_SIZE
904+
);
905+
return String::new();
906+
}
907+
}
908+
909+
rewritten
851910
}
852911

853912
/// Stream processor for creative HTML that rewrites URLs to first-party proxy.
@@ -1688,6 +1747,71 @@ mod tests {
16881747
);
16891748
}
16901749

1750+
#[test]
1751+
fn rewrite_injects_runtime_into_body_less_fragment() {
1752+
// Bidder `adm` is commonly a bare fragment with no <body> token, and
1753+
// lol_html does not synthesize one. Without the runtime the click guard
1754+
// never installs, so rewritten links lose first-party mediation as soon
1755+
// as surviving bidder script mutates them.
1756+
let settings = crate::test_support::tests::create_test_settings();
1757+
let fragment = r#"<a href="https://click.example/landing">x</a><script>marker</script>"#;
1758+
1759+
let out = rewrite_creative_html(&settings, fragment);
1760+
1761+
assert!(
1762+
out.contains("/static/tsjs=tsjs-unified.min.js"),
1763+
"should inject the creative runtime without a body token: {out}"
1764+
);
1765+
assert_eq!(
1766+
out.matches("/static/tsjs=tsjs-unified.min.js").count(),
1767+
1,
1768+
"should inject exactly once: {out}"
1769+
);
1770+
assert!(
1771+
out.contains("/first-party/click?tsurl="),
1772+
"should still rewrite click URLs: {out}"
1773+
);
1774+
}
1775+
1776+
#[test]
1777+
fn inline_rewrite_does_not_inject_runtime_into_fragment() {
1778+
// The foreign-origin inline path deliberately omits the bundle; the
1779+
// body-less fallback must not reintroduce it there.
1780+
let settings = crate::test_support::tests::create_test_settings();
1781+
let fragment = r#"<a href="https://click.example/landing">x</a>"#;
1782+
1783+
let out =
1784+
rewrite_inline_creative_html(&settings, "https://news.publisher.example", fragment);
1785+
1786+
assert!(
1787+
!out.contains("/static/tsjs="),
1788+
"inline rewriting must not inject the bundle: {out}"
1789+
);
1790+
}
1791+
1792+
#[test]
1793+
fn rewrite_rejects_output_exceeding_the_cap() {
1794+
// Rewriting amplifies: each short URL becomes a signed proxy/click URL
1795+
// and anchors gain a data-tsclick copy. An input under the cap can
1796+
// therefore expand past it, so the OUTPUT is bounded too.
1797+
let settings = crate::test_support::tests::create_test_settings();
1798+
let anchor = r#"<a href="https://click.example/landing?q=0123456789">x</a>"#;
1799+
let repeats = (super::MAX_CREATIVE_SIZE / anchor.len()) / 2;
1800+
let input = anchor.repeat(repeats);
1801+
assert!(
1802+
input.len() < super::MAX_CREATIVE_SIZE,
1803+
"test input must start under the cap"
1804+
);
1805+
1806+
let out = rewrite_creative_html(&settings, &input);
1807+
1808+
assert!(
1809+
out.is_empty(),
1810+
"should reject a creative whose rewritten output exceeds the cap (got {} bytes)",
1811+
out.len()
1812+
);
1813+
}
1814+
16911815
#[test]
16921816
fn inline_rewrite_strips_base_elements() {
16931817
let mut settings = crate::test_support::tests::create_test_settings();

0 commit comments

Comments
 (0)