Skip to content

Commit 711dc38

Browse files
committed
Apply review follow-ups for server-side auction hardening
- Stop forwarding client-supplied X-Forwarded-For to Prebid Server; synthesize it from the platform-attested client IP instead - Make the APS slot ID remapping request-scoped by threading the AuctionRequest through parse_response_with_context, removing the shared provider-instance mutex that concurrent auctions could race - Guard dispatch_auction against multi-provider fan-out on platforms whose HTTP client executes requests sequentially, mirroring run_providers_parallel - Reject negative and non-finite creative-opportunity floor prices at config validation - Re-run the Set-Cookie cache-privacy downgrade after operator response headers are applied so configured Set-Cookie plus public cache headers cannot produce a shared-cacheable response - Install Prebid user ID modules immediately when the bundle loads after window.load (slim-Prebid lazy path), with a once-only listener - Drop allow-same-origin from debug ADM fallback iframes and validate extracted iframe src schemes to http(s) only
1 parent 224fcaf commit 711dc38

11 files changed

Lines changed: 651 additions & 137 deletions

File tree

crates/trusted-server-core/src/auction/orchestrator.rs

Lines changed: 96 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,12 @@ impl AuctionOrchestrator {
276276
// /__ts/page-bids must match or mediated cache bids lose the metadata
277277
// needed for creative rendering and win/billing beacons.
278278
let mediator_resp = mediator
279-
.parse_response_with_context(platform_resp, response_time_ms, &mediator_context)
279+
.parse_response_with_context(
280+
platform_resp,
281+
response_time_ms,
282+
request,
283+
&mediator_context,
284+
)
280285
.await
281286
.change_context(TrustedServerError::Auction {
282287
message: format!("Mediator {} parse failed", mediator.provider_name()),
@@ -546,7 +551,12 @@ impl AuctionOrchestrator {
546551
// parallel (`/auction`, page-bids) and collect (publisher)
547552
// paths. The default impl delegates to `parse_response`.
548553
match provider
549-
.parse_response_with_context(response, response_time_ms, context)
554+
.parse_response_with_context(
555+
response,
556+
response_time_ms,
557+
request,
558+
context,
559+
)
550560
.await
551561
{
552562
Ok(auction_response) => {
@@ -764,6 +774,23 @@ impl AuctionOrchestrator {
764774
return None;
765775
}
766776

777+
// Mirror run_providers_parallel: reject multi-provider fan-out before
778+
// any request launches when the platform executes `send_async` eagerly
779+
// (e.g. Cloudflare Workers, Spin). Sequential execution would accrue
780+
// the sum of provider latencies before the origin fetch and then fail
781+
// collection with empty bids.
782+
if provider_names.len() > 1 && !context.services.http_client().supports_concurrent_fanout()
783+
{
784+
log::warn!(
785+
"{} auction providers configured, but this platform's HTTP client \
786+
executes requests sequentially — skipping initial-page auction \
787+
dispatch; configure a single provider, or use an adapter with \
788+
concurrent fan-out support",
789+
provider_names.len(),
790+
);
791+
return None;
792+
}
793+
767794
let auction_start = Instant::now();
768795
let mut backend_to_provider: HashMap<String, (String, Instant, Arc<dyn AuctionProvider>)> =
769796
HashMap::new();
@@ -939,6 +966,7 @@ impl AuctionOrchestrator {
939966
.parse_response_with_context(
940967
platform_response,
941968
response_time_ms,
969+
&request,
942970
context,
943971
)
944972
.await
@@ -1082,6 +1110,7 @@ impl AuctionOrchestrator {
10821110
.parse_response_with_context(
10831111
platform_resp,
10841112
response_time_ms,
1113+
&request,
10851114
&mediator_context,
10861115
)
10871116
.await
@@ -1355,6 +1384,7 @@ mod tests {
13551384
&self,
13561385
_response: PlatformResponse,
13571386
response_time_ms: u64,
1387+
_request: &AuctionRequest,
13581388
_context: &AuctionContext<'_>,
13591389
) -> Result<AuctionResponse, Report<TrustedServerError>> {
13601390
// Context-aware path: restores nurl/ad_id from the collected SSP bids.
@@ -1959,6 +1989,70 @@ mod tests {
19591989
});
19601990
}
19611991

1992+
#[test]
1993+
fn dispatch_auction_skips_multi_provider_fanout_on_sequential_platform() {
1994+
futures::executor::block_on(async {
1995+
// Arrange: two configured providers on a platform whose HTTP
1996+
// client executes send_async eagerly (no concurrent fan-out).
1997+
// The initial-page dispatch path must apply the same guard as
1998+
// run_providers_parallel or the summed provider latency lands
1999+
// before the origin fetch.
2000+
let stub = Arc::new(StubHttpClient::new());
2001+
stub.set_concurrent_fanout(false);
2002+
let stub_for_assertion = Arc::clone(&stub);
2003+
2004+
let services = build_services_with_http_client(stub);
2005+
// SAFETY: `Box::leak` creates a `'static` reference for test use only.
2006+
// The leaked allocation is bounded to the test process lifetime.
2007+
let services: &'static RuntimeServices = Box::leak(Box::new(services));
2008+
2009+
let config = AuctionConfig {
2010+
enabled: true,
2011+
providers: vec!["provider-a".to_string(), "provider-b".to_string()],
2012+
timeout_ms: 2000,
2013+
mediator: None,
2014+
..Default::default()
2015+
};
2016+
let mut orchestrator = AuctionOrchestrator::new(config);
2017+
orchestrator.register_provider(Arc::new(StubAuctionProvider {
2018+
name: "provider-a",
2019+
backend: "backend-a",
2020+
}));
2021+
orchestrator.register_provider(Arc::new(StubAuctionProvider {
2022+
name: "provider-b",
2023+
backend: "backend-b",
2024+
}));
2025+
2026+
let request = create_test_auction_request();
2027+
let settings = create_test_settings();
2028+
let req = http::Request::builder()
2029+
.method(http::Method::GET)
2030+
.uri("https://example.com/test")
2031+
.body(edgezero_core::body::Body::empty())
2032+
.expect("should build request");
2033+
let context = AuctionContext {
2034+
settings: &settings,
2035+
request: &req,
2036+
timeout_ms: 2000,
2037+
provider_responses: None,
2038+
services,
2039+
};
2040+
2041+
// Act
2042+
let dispatched = orchestrator.dispatch_auction(&request, &context).await;
2043+
2044+
// Assert: no dispatch and no provider request launched.
2045+
assert!(
2046+
dispatched.is_none(),
2047+
"should skip initial-page dispatch on sequential platforms"
2048+
);
2049+
assert!(
2050+
stub_for_assertion.recorded_backend_names().is_empty(),
2051+
"should not launch any provider request on a sequential platform"
2052+
);
2053+
});
2054+
}
2055+
19622056
#[test]
19632057
fn test_apply_floor_prices_drops_bids_with_undecoded_price() {
19642058
// Bids that reach apply_floor_prices with `price=None` cannot have a

crates/trusted-server-core/src/auction/provider.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,13 @@ pub trait AuctionProvider: Send + Sync {
4949
response_time_ms: u64,
5050
) -> Result<AuctionResponse, Report<TrustedServerError>>;
5151

52-
/// Parse the response with access to the original auction context.
52+
/// Parse the response with access to the original auction request and context.
5353
///
5454
/// Providers that need request-local metadata while transforming responses
55-
/// can override this method. The default preserves the existing
56-
/// response-only provider contract.
55+
/// can override this method. `request` is the [`AuctionRequest`] the
56+
/// orchestrator dispatched, so request-scoped data (e.g. slot ID mappings)
57+
/// can be derived here instead of stored on the shared provider instance.
58+
/// The default preserves the existing response-only provider contract.
5759
///
5860
/// # Errors
5961
///
@@ -62,9 +64,10 @@ pub trait AuctionProvider: Send + Sync {
6264
&self,
6365
response: PlatformResponse,
6466
response_time_ms: u64,
67+
request: &AuctionRequest,
6568
context: &AuctionContext<'_>,
6669
) -> Result<AuctionResponse, Report<TrustedServerError>> {
67-
let _ = context;
70+
let _ = (request, context);
6871
self.parse_response(response, response_time_ms).await
6972
}
7073

crates/trusted-server-core/src/creative_opportunities.rs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,18 @@ impl CreativeOpportunitySlot {
143143
format.validate_runtime(&self.id)?;
144144
}
145145

146+
// A negative floor silently disables minimum-price enforcement, and a
147+
// non-finite floor (NaN/infinity) produces surprising all-pass/all-drop
148+
// comparisons and an invalid OpenRTB `bidfloor`.
149+
if let Some(floor_price) = self.floor_price {
150+
if !floor_price.is_finite() || floor_price < 0.0 {
151+
return Err(format!(
152+
"slot `{}` floor_price must be a finite value >= 0.0, got {floor_price}",
153+
self.id
154+
));
155+
}
156+
}
157+
146158
// An explicit empty/whitespace `div_id` override is rejected: the
147159
// injected JS resolves slots with `candidate.id.startsWith(slot.div_id)`,
148160
// and every element id starts with the empty string, so an empty override
@@ -568,6 +580,42 @@ mod tests {
568580
);
569581
}
570582

583+
#[test]
584+
fn validate_runtime_rejects_invalid_floor_prices() {
585+
let mut slot = make_slot("atf", vec!["/"]);
586+
slot.compile_patterns();
587+
588+
slot.floor_price = Some(-0.01);
589+
assert!(
590+
slot.validate_runtime("1234").is_err(),
591+
"negative floor_price should fail validation"
592+
);
593+
594+
slot.floor_price = Some(f64::NAN);
595+
assert!(
596+
slot.validate_runtime("1234").is_err(),
597+
"NaN floor_price should fail validation"
598+
);
599+
600+
slot.floor_price = Some(f64::INFINITY);
601+
assert!(
602+
slot.validate_runtime("1234").is_err(),
603+
"infinite floor_price should fail validation"
604+
);
605+
606+
slot.floor_price = Some(0.0);
607+
assert!(
608+
slot.validate_runtime("1234").is_ok(),
609+
"zero floor_price should pass validation"
610+
);
611+
612+
slot.floor_price = None;
613+
assert!(
614+
slot.validate_runtime("1234").is_ok(),
615+
"absent floor_price should pass validation"
616+
);
617+
}
618+
571619
#[test]
572620
fn to_ad_slot_wires_aps_params_into_bidders() {
573621
let mut slot = make_slot("atf", vec!["/"]);

crates/trusted-server-core/src/integrations/adserver_mock.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,7 @@ impl AuctionProvider for AdServerMockProvider {
497497
&self,
498498
response: PlatformResponse,
499499
response_time_ms: u64,
500+
_request: &AuctionRequest,
500501
context: &AuctionContext<'_>,
501502
) -> Result<AuctionResponse, Report<TrustedServerError>> {
502503
// Rebuild the SSP-bid lookup from the orchestrator-provided bidder

0 commit comments

Comments
 (0)