Skip to content

Commit a0fa826

Browse files
ChristianPavilonisChristianPavilonis
committed
Fix DataDome backend integration review issues
1 parent c18ba1b commit a0fa826

8 files changed

Lines changed: 184 additions & 29 deletions

File tree

‎crates/integration-tests/Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/trusted-server-adapter-fastly/src/main.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,6 @@ fn main() {
239239
match outcome {
240240
HandlerOutcome::Buffered(mut response) | HandlerOutcome::AuthChallenge(mut response) => {
241241
finalize_response(&settings, geo_info.as_ref(), &mut response);
242-
request_filter_effects.apply_to_response(&mut response);
243242
asset_cache_policy.apply_after_route_finalization(&mut response);
244243
let mut fastly_resp = compat::to_fastly_response(response);
245244
if should_finalize_ec {
@@ -253,6 +252,7 @@ fn main() {
253252
&mut fastly_resp,
254253
);
255254
}
255+
request_filter_effects.apply_to_fastly_response(&mut fastly_resp);
256256
fastly_resp.send_to_client();
257257

258258
if is_real_browser {
@@ -267,7 +267,6 @@ fn main() {
267267
params,
268268
} => {
269269
finalize_response(&settings, geo_info.as_ref(), &mut response);
270-
request_filter_effects.apply_to_response(&mut response);
271270
asset_cache_policy.apply_after_route_finalization(&mut response);
272271
let mut fastly_resp = compat::to_fastly_response_skeleton(response);
273272
if should_finalize_ec {
@@ -281,6 +280,7 @@ fn main() {
281280
&mut fastly_resp,
282281
);
283282
}
283+
request_filter_effects.apply_to_fastly_response(&mut fastly_resp);
284284
let mut streaming_body = fastly_resp.stream_to_client();
285285
let mut stream_succeeded = false;
286286
match stream_publisher_body(
@@ -313,9 +313,9 @@ fn main() {
313313
}
314314
HandlerOutcome::AssetStreaming { mut response, body } => {
315315
finalize_response(&settings, geo_info.as_ref(), &mut response);
316-
request_filter_effects.apply_to_response(&mut response);
317316
asset_cache_policy.apply_after_route_finalization(&mut response);
318-
let fastly_resp = compat::to_fastly_response_skeleton(response);
317+
let mut fastly_resp = compat::to_fastly_response_skeleton(response);
318+
request_filter_effects.apply_to_fastly_response(&mut fastly_resp);
319319
let mut streaming_body = fastly_resp.stream_to_client();
320320
if let Err(e) =
321321
futures::executor::block_on(stream_asset_body(body, &mut streaming_body))

‎crates/trusted-server-adapter-fastly/src/route_tests.rs‎

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -457,8 +457,8 @@ fn datadome_protection_toml() -> &'static str {
457457
[integrations.datadome]
458458
enabled = true
459459
enable_protection = true
460-
server_side_key_secret_store = "datadome"
461-
server_side_key_secret_name = "server_side_key"
460+
server_side_key_secret_store = "ts_secrets"
461+
server_side_key_secret_name = "datadome_server_side_key"
462462
"#
463463
}
464464

@@ -482,7 +482,7 @@ fn create_datadome_auction_test_settings(providers: &str) -> Settings {
482482

483483
fn datadome_secret_store() -> Arc<dyn PlatformSecretStore> {
484484
Arc::new(HashMapSecretStore::new(HashMap::from([(
485-
"server_side_key".to_string(),
485+
"datadome_server_side_key".to_string(),
486486
b"datadome-server-side-key".to_vec(),
487487
)])))
488488
}
@@ -597,7 +597,6 @@ fn route_result_to_fastly_response(
597597
.unwrap_or(None)
598598
};
599599
super::finalize_response(settings, geo_info.as_ref(), &mut response);
600-
request_filter_effects.apply_to_response(&mut response);
601600
asset_cache_policy.apply_after_route_finalization(&mut response);
602601

603602
let mut fastly_response = compat::to_fastly_response(response);
@@ -612,6 +611,7 @@ fn route_result_to_fastly_response(
612611
&mut fastly_response,
613612
);
614613
}
614+
request_filter_effects.apply_to_fastly_response(&mut fastly_response);
615615
fastly_response
616616
}
617617

@@ -908,6 +908,64 @@ fn datadome_skips_internal_and_static_asset_routes_by_default() {
908908
);
909909
}
910910

911+
#[test]
912+
fn datadome_skips_registered_integration_routes_with_custom_prefix() {
913+
let base = base_route_settings_toml();
914+
let datadome = datadome_protection_toml();
915+
let config = format!(
916+
r#"{base}
917+
918+
{datadome}
919+
920+
[integrations.didomi]
921+
enabled = true
922+
proxy_path = "my-consent"
923+
sdk_origin = "https://sdk.privacy-center.org"
924+
api_origin = "https://api.privacy-center.org"
925+
926+
[auction]
927+
enabled = true
928+
providers = []
929+
timeout_ms = 2000
930+
"#,
931+
);
932+
let settings = Settings::from_toml(&config)
933+
.expect("should parse DataDome and custom Didomi route test settings");
934+
let (orchestrator, integration_registry) = build_route_stack(&settings);
935+
let req = Request::get("https://test.com/my-consent/notice");
936+
let http_client = Arc::new(RecordingHttpClient::new(StatusCode::OK));
937+
let services = test_runtime_services_with_secret_and_http_client(
938+
&req,
939+
Arc::new(FixedBackend),
940+
datadome_secret_store(),
941+
Arc::clone(&http_client) as Arc<dyn PlatformHttpClient>,
942+
);
943+
944+
let response = route_buffered_response(
945+
&settings,
946+
&orchestrator,
947+
&integration_registry,
948+
&services,
949+
req,
950+
"should route custom Didomi proxy request without DataDome",
951+
);
952+
953+
assert_eq!(
954+
response.get_status(),
955+
StatusCode::OK,
956+
"custom integration proxy route should still be handled"
957+
);
958+
let calls = http_client
959+
.calls
960+
.lock()
961+
.expect("should lock recorded calls");
962+
assert_eq!(calls.len(), 1, "should call only the Didomi upstream");
963+
assert_eq!(
964+
calls[0].uri, "https://sdk.privacy-center.org/notice",
965+
"should not call the DataDome Protection API for registered integration routes"
966+
);
967+
}
968+
911969
#[test]
912970
fn routes_use_request_local_consent() {
913971
let settings = create_test_settings();

‎crates/trusted-server-core/src/integrations/datadome.rs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,11 +240,11 @@ fn default_protection_api_origin() -> String {
240240
}
241241

242242
fn default_server_side_key_secret_store() -> String {
243-
"datadome".to_string()
243+
"ts_secrets".to_string()
244244
}
245245

246246
fn default_server_side_key_secret_name() -> String {
247-
"server_side_key".to_string()
247+
"datadome_server_side_key".to_string()
248248
}
249249

250250
fn default_timeout_ms() -> u32 {
@@ -1021,6 +1021,17 @@ mod tests {
10211021
}
10221022
}
10231023

1024+
#[test]
1025+
fn protection_secret_defaults_match_sample_config() {
1026+
let config = DataDomeConfig::default();
1027+
1028+
assert_eq!(config.server_side_key_secret_store, "ts_secrets");
1029+
assert_eq!(
1030+
config.server_side_key_secret_name,
1031+
"datadome_server_side_key"
1032+
);
1033+
}
1034+
10241035
#[test]
10251036
fn protection_enabled_requires_server_side_key_secret_store() {
10261037
let mut config = test_config();

‎crates/trusted-server-core/src/integrations/datadome/protection.rs‎

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ impl DataDomeIntegration {
9595
.await
9696
.change_context(Self::error("Failed to call DataDome Protection API"))?;
9797

98-
Ok(self.classify_protection_response(platform_response.response))
98+
Ok(self.classify_protection_response(platform_response.response, input.request.method()))
9999
}
100100

101101
fn is_request_protected(&self, input: &RequestFilterInput<'_>) -> bool {
@@ -104,6 +104,10 @@ impl DataDomeIntegration {
104104
return false;
105105
}
106106

107+
if input.is_integration_route {
108+
return false;
109+
}
110+
107111
let path = req.uri().path();
108112
if is_internal_path(path) {
109113
return false;
@@ -356,6 +360,7 @@ impl DataDomeIntegration {
356360
fn classify_protection_response(
357361
&self,
358362
response: edgezero_core::http::Response,
363+
request_method: &Method,
359364
) -> RequestFilterDecision {
360365
let (parts, body) = response.into_parts();
361366
let status = parts.status;
@@ -386,14 +391,21 @@ impl DataDomeIntegration {
386391
}
387392

388393
if matches!(status.as_u16(), 301 | 302 | 401 | 403 | 429) {
389-
if body.is_stream() {
390-
log::warn!("[datadome] Protection API challenge body was streaming; failing open");
391-
return RequestFilterDecision::Continue(RequestFilterEffects::default());
392-
}
393-
let body_bytes = body.into_bytes();
394+
let response_body = if request_method == Method::HEAD {
395+
EdgeBody::empty()
396+
} else {
397+
if body.is_stream() {
398+
log::warn!(
399+
"[datadome] Protection API challenge body was streaming; failing open"
400+
);
401+
return RequestFilterDecision::Continue(RequestFilterEffects::default());
402+
}
403+
let body_bytes = body.into_bytes();
404+
EdgeBody::from(body_bytes.as_ref().to_vec())
405+
};
394406
let challenge = Response::builder()
395407
.status(status)
396-
.body(EdgeBody::from(body_bytes.as_ref().to_vec()))
408+
.body(response_body)
397409
.expect("should build DataDome challenge response");
398410
return RequestFilterDecision::Respond {
399411
response: Box::new(challenge),
@@ -468,7 +480,7 @@ fn header_value(req: &Request<EdgeBody>, name: &str) -> String {
468480
fn headers_list(req: &Request<EdgeBody>) -> String {
469481
req.headers()
470482
.keys()
471-
.map(|name| name.as_str())
483+
.map(HeaderName::as_str)
472484
.collect::<Vec<_>>()
473485
.join(",")
474486
}
@@ -677,7 +689,10 @@ mod tests {
677689
fn load_server_side_key_reads_secret_store() {
678690
clear_datadome_server_side_key_cache_for_tests();
679691
let mut secrets = HashMap::new();
680-
secrets.insert("server_side_key".to_string(), b"secret-from-store".to_vec());
692+
secrets.insert(
693+
"datadome_server_side_key".to_string(),
694+
b"secret-from-store".to_vec(),
695+
);
681696
let services = build_services_with_config_and_secret(
682697
NoopConfigStore,
683698
HashMapSecretStore::new(secrets),
@@ -746,6 +761,32 @@ mod tests {
746761
assert_eq!(truncate_utf8("ééé", -4), "éé");
747762
}
748763

764+
#[test]
765+
fn classify_head_challenge_omits_response_body() {
766+
let integration = protection_integration();
767+
let response = edgezero_core::http::response_builder()
768+
.status(StatusCode::FORBIDDEN)
769+
.header(HEADER_DATADOME_RESPONSE, "403")
770+
.body(EdgeBody::from("blocked"))
771+
.expect("should build DataDome response");
772+
773+
let decision = integration.classify_protection_response(response, &Method::HEAD);
774+
775+
let RequestFilterDecision::Respond { response, .. } = decision else {
776+
panic!("should return a challenge response for DataDome 403");
777+
};
778+
assert_eq!(
779+
response.status(),
780+
StatusCode::FORBIDDEN,
781+
"should preserve challenge status"
782+
);
783+
assert_eq!(
784+
response.into_body().into_bytes().as_ref(),
785+
b"",
786+
"HEAD challenges should not include a response body"
787+
);
788+
}
789+
749790
#[test]
750791
fn form_encode_url_encodes_values() {
751792
let encoded = form_encode(&[("Key".to_string(), "a b+c".to_string())]);

‎crates/trusted-server-core/src/integrations/registry.rs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,8 @@ pub struct RequestFilterInput<'a> {
330330
pub services: &'a RuntimeServices,
331331
pub request: &'a Request<EdgeBody>,
332332
pub geo_info: Option<&'a GeoInfo>,
333+
/// Whether the request matches a registered integration proxy route.
334+
pub is_integration_route: bool,
333335
}
334336

335337
/// How a header mutation should be applied.
@@ -391,6 +393,12 @@ impl RequestFilterEffects {
391393
apply_header_mutation_to_response(response, mutation);
392394
}
393395
}
396+
397+
pub fn apply_to_fastly_response(&self, response: &mut fastly::Response) {
398+
for mutation in &self.response_headers {
399+
apply_header_mutation_to_fastly_response(response, mutation);
400+
}
401+
}
394402
}
395403

396404
/// Decision returned by an integration request filter.
@@ -511,6 +519,40 @@ fn apply_header_mutation_to_response(response: &mut Response<EdgeBody>, mutation
511519
}
512520
}
513521

522+
fn apply_header_mutation_to_fastly_response(
523+
response: &mut fastly::Response,
524+
mutation: &HeaderMutation,
525+
) {
526+
if is_forbidden_filter_header(&mutation.name) {
527+
log::warn!(
528+
"Skipping forbidden response-filter header: {}",
529+
mutation.name
530+
);
531+
return;
532+
}
533+
534+
let Ok(name) = fastly::http::HeaderName::from_bytes(mutation.name.as_bytes()) else {
535+
log::warn!("Skipping invalid response-filter header: {}", mutation.name);
536+
return;
537+
};
538+
let Ok(value) = fastly::http::HeaderValue::from_str(&mutation.value) else {
539+
log::warn!(
540+
"Skipping invalid response-filter header value: {}",
541+
mutation.name
542+
);
543+
return;
544+
};
545+
546+
match mutation.mode {
547+
HeaderMutationMode::Set => {
548+
response.set_header(name, value);
549+
}
550+
HeaderMutationMode::Append => {
551+
response.append_header(name, value);
552+
}
553+
}
554+
}
555+
514556
/// Trait for integration-provided HTML attribute rewrite hooks.
515557
pub trait IntegrationAttributeRewriter: Send + Sync {
516558
/// Identifier for logging/diagnostics.
@@ -889,6 +931,7 @@ impl IntegrationRegistry {
889931
geo_info,
890932
} = input;
891933
let mut accumulated = RequestFilterEffects::default();
934+
let is_integration_route = self.has_route(req.method(), req.uri().path());
892935

893936
for filter in &self.inner.request_filters {
894937
let decision = filter
@@ -897,6 +940,7 @@ impl IntegrationRegistry {
897940
services,
898941
request: req,
899942
geo_info,
943+
is_integration_route,
900944
})
901945
.await?;
902946

0 commit comments

Comments
 (0)