Use configured publisher domain for navigation-path auction requests#937
Use configured publisher domain for navigation-path auction requests#937aram356 wants to merge 2 commits into
Conversation
build_auction_request derived publisher.domain, site.domain, and the page URL host from the incoming request Host header. On the SSAT proxy path that header is the trusted-server edge host (e.g. the staging domain), which then leaked into the outbound OpenRTB bid request and, through it, into injected creatives and the IAS brand-safety pixel. Source these fields from settings.publisher.domain instead, matching what convert_tsjs_to_auction_request already does on the /auction endpoint path. Closes #936
…n requests) into rc/july # Conflicts: # crates/trusted-server-core/src/publisher.rs
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
Fixes the navigation-path (SSAT/SPA) AuctionRequest builder so publisher.domain, site.domain, and the page URL host come from the configured settings.publisher.domain instead of the incoming edge Host header, matching the pattern already used by convert_tsjs_to_auction_request on the /auction path. Minimal, well-targeted diff (publisher.rs only) with a solid test-first regression test.
Non-blocking
🌱 seedling
- Scheme source differs between the two
AuctionRequestbuilders:build_auction_request(publisher.rs:2021) still derivespage_url's scheme fromrequest_info.scheme(edge-detected), whileconvert_tsjs_to_auction_request(auction/formats.rs:202) hardcodes"https"for the same field. Not a bug today (the edge always terminates TLS ashttpsin production), but worth aligning in a follow-up so both builders source scheme the same way.
👍 praise
- Test-first regression coverage:
auction_request_uses_configured_publisher_domain_not_edge_hostuses a deliberately divergent edge host (ts.example.com) vs. configured domain (www.example.com) and asserts all four leak points (publisher.domain,site.domain,page_url,site.page). Confirmed to fail on the pre-fix code per the PR description.
CI Status
- fmt: PASS
- clippy: not reported as a distinct check (cargo check jobs pass)
- rust tests: PASS (fastly/axum/cloudflare/spin, cross-adapter parity)
- js tests (vitest): PASS
- integration tests: PASS
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
The core OpenRTB/APS domain correction is narrow and correct. Approving with two follow-up findings noted inline: navigation telemetry still uses the edge host as publisher_domain, and the new regression test covers the helper rather than the production handlers.
| ec_id, | ||
| &consent_context, | ||
| &request_info, | ||
| &settings.publisher.domain, |
There was a problem hiding this comment.
🔧 P1 / High — Navigation telemetry still records the edge host as the publisher domain
The auction request now correctly receives settings.publisher.domain here, but the AuctionObservationContext::from_parts call immediately above still receives request_host. The SPA path has the same mismatch at lines 2438–2440, where it passes request_info.host. On the affected routing model those values are the edge/staging host this PR is removing from the auction request.
AuctionObservationContext copies the value into publisher_domain, which Tinybird uses for sorting, rollups, dashboard filters, and ingestion-freshness reporting. Navigation metrics will therefore remain attributed to the edge host while /auction metrics and outbound bids use the configured publisher domain.
Suggested fix: Pass &settings.publisher.domain to both observation constructors and add coverage with divergent request/configured hosts that asserts the emitted event rows use the configured publisher domain.
| } | ||
|
|
||
| #[test] | ||
| fn auction_request_uses_configured_publisher_domain_not_edge_host() { |
There was a problem hiding this comment.
🌱 P2 / Medium — The regression test does not exercise either production call path
This test invokes build_auction_request directly and supplies "www.example.com" itself. It would continue passing if either newly changed handler reverted to passing request_info.host, because both sources have the same &str type.
The original defect was a source-selection error between builders and request paths, so helper-only coverage proves field assignment but not that initial and SPA navigation choose the correct source.
Suggested fix: Add handler-level tests for initial navigation and /__ts/page-bids with divergent edge/configured hosts, capturing the dispatched auction or OpenRTB request. Those tests can also cover the telemetry inconsistency noted above.
Summary
On the navigation / SSAT proxy path, the server-side auction advertised the incoming request
Hostheader — which on that path is the trusted-server edge host (e.g. the staging domain) — as the publisher domain. That value flowed into the outbound OpenRTB bid request and, through it, into injected creatives and the IAS brand-safety pixel, so both carried the edge/staging host instead of the real publisher domain.build_auction_requestsetpublisher.domain,site.domain, and thepage_urlhost fromrequest_info.host. This change sources those fields fromsettings.publisher.domaininstead — matching whatconvert_tsjs_to_auction_requestalready does on the/auctionendpoint path.Closes #936.
Root cause
Two
AuctionRequestbuilders derived the publisher host from opposite sources:convert_tsjs_to_auction_request(auction/formats.rs) — used config (settings.publisher.domain). ✅build_auction_request(publisher.rs) — used the raw incomingHostheader. ❌ (navigation path:AuctionSource::InitialNavigationandSpaNavigation)PR #934 only corrected
ext.trusted_server.request_hostinside the Prebid provider; it did not touchsite.domain/site.publisher.domain/site.page, and on the navigation pathrequest.publisher.domainwas itself the poisoned edge host. This fixes the builder at the source.Changes
publisher_domain: &strparameter tobuild_auction_requestand use it forpublisher.domain,site.domain, and thepage_urlhost (scheme and path are unchanged).&settings.publisher.domain.auction_request_uses_configured_publisher_domain_not_edge_host: with a divergent edge host (ts.example.com) and configured domain (www.example.com), the bid request advertises the configured domain forpublisher.domain,site.domain,page_url, andsite.page. Written test-first (verified it fails on the old behavior before the fix).Test plan
cargo fmt --all -- --checkcargo test-fastly(wasm — core + fastly, incl. new regression test)cargo test-axum(native — shared core)cargo clippy-*— could not run locally:clippy-fastlyfails in thetrusted-server-jsbuild script (Unrecognized option: 'p'), which reproduces on unmodifiedmain, so it is a pre-existing local-environment issue unrelated to this change. Deferred to CI.Notes
Targets
main(the buggy code exists onmain, not just the release branch). Can be cherry-picked / merged intorc/julyas needed.