From bc13f2773ba97e8de07f7ba6ef6f3d0a3e37427f Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Mon, 20 Jul 2026 16:38:03 -0700 Subject: [PATCH 01/13] Upgrade EdgeZero to the deploy-actions branch Point the edgezero-* dependencies at the feature/edgezero-deploy-actions branch (PR #316) and adapt Trusted Server to its API changes: - Wire the new ts CLI subcommands surfaced by edgezero-cli: active-version, healthcheck, and rollback, plus deploy --stage and a --version flag, with argument-parsing coverage. - Migrate TrustedServerAppConfig to the AppConfigMeta::secret_fields() method that replaces the removed SECRET_FIELDS associated constant. --- Cargo.lock | 37 +++-- Cargo.toml | 12 +- crates/trusted-server-cli/src/run.rs | 174 ++++++++++++++++++++++- crates/trusted-server-core/src/config.rs | 4 +- 4 files changed, 205 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 68f14e753..bd92ec5ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1398,7 +1398,7 @@ dependencies = [ [[package]] name = "edgezero-adapter" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.4#9e661ae520a8130660f18fd10f42703d7f3e050b" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" dependencies = [ "toml", ] @@ -1406,7 +1406,7 @@ dependencies = [ [[package]] name = "edgezero-adapter-axum" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.4#9e661ae520a8130660f18fd10f42703d7f3e050b" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" dependencies = [ "anyhow", "async-trait", @@ -1434,7 +1434,7 @@ dependencies = [ [[package]] name = "edgezero-adapter-cloudflare" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.4#9e661ae520a8130660f18fd10f42703d7f3e050b" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" dependencies = [ "anyhow", "async-trait", @@ -1449,7 +1449,7 @@ dependencies = [ "log", "serde_json", "tempfile", - "toml_edit", + "toml_edit 0.25.12+spec-1.1.0", "walkdir", "worker", ] @@ -1457,7 +1457,7 @@ dependencies = [ [[package]] name = "edgezero-adapter-fastly" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.4#9e661ae520a8130660f18fd10f42703d7f3e050b" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" dependencies = [ "anyhow", "async-stream", @@ -1479,14 +1479,14 @@ dependencies = [ "serde_json", "sha2 0.10.9", "thiserror 2.0.18", - "toml_edit", + "toml_edit 0.25.12+spec-1.1.0", "walkdir", ] [[package]] name = "edgezero-adapter-spin" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.4#9e661ae520a8130660f18fd10f42703d7f3e050b" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" dependencies = [ "anyhow", "async-trait", @@ -1506,14 +1506,14 @@ dependencies = [ "subtle", "thiserror 2.0.18", "toml", - "toml_edit", + "toml_edit 0.25.12+spec-1.1.0", "walkdir", ] [[package]] name = "edgezero-cli" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.4#9e661ae520a8130660f18fd10f42703d7f3e050b" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" dependencies = [ "chrono", "clap", @@ -1538,7 +1538,7 @@ dependencies = [ [[package]] name = "edgezero-core" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.4#9e661ae520a8130660f18fd10f42703d7f3e050b" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" dependencies = [ "anyhow", "async-compression", @@ -1569,7 +1569,7 @@ dependencies = [ [[package]] name = "edgezero-macros" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?tag=v0.0.4#9e661ae520a8130660f18fd10f42703d7f3e050b" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" dependencies = [ "log", "proc-macro2", @@ -5074,6 +5074,19 @@ dependencies = [ "winnow 0.7.15", ] +[[package]] +name = "toml_edit" +version = "0.25.12+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2153edc6955a6c354fad8f5efd38b6a8769bdccf9fe50f8e1329f81b0baa5d7" +dependencies = [ + "indexmap 2.14.0", + "toml_datetime 1.1.1+spec-1.1.0", + "toml_parser", + "toml_writer", + "winnow 1.0.3", +] + [[package]] name = "toml_parser" version = "1.1.2+spec-1.1.0" @@ -5339,7 +5352,7 @@ dependencies = [ "tokio", "tokio-rustls", "toml", - "toml_edit", + "toml_edit 0.23.10+spec-1.0.0", "trusted-server-core", "url", "webpki-roots", diff --git a/Cargo.toml b/Cargo.toml index 695099d49..54569599b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,12 +53,12 @@ criterion = { version = "0.5", default-features = false, features = ["cargo_benc derive_more = { version = "2.0", features = ["display", "error"] } directories = "5" ed25519-dalek = { version = "2.2", features = ["rand_core"] } -edgezero-adapter-axum = { git = "https://github.com/stackpop/edgezero", tag = "v0.0.4", default-features = false } -edgezero-adapter-cloudflare = { git = "https://github.com/stackpop/edgezero", tag = "v0.0.4", default-features = false } -edgezero-adapter-fastly = { git = "https://github.com/stackpop/edgezero", tag = "v0.0.4", default-features = false } -edgezero-adapter-spin = { git = "https://github.com/stackpop/edgezero", tag = "v0.0.4", default-features = false } -edgezero-cli = { git = "https://github.com/stackpop/edgezero", tag = "v0.0.4" } -edgezero-core = { git = "https://github.com/stackpop/edgezero", tag = "v0.0.4", default-features = false } +edgezero-adapter-axum = { git = "https://github.com/stackpop/edgezero", branch = "feature/edgezero-deploy-actions", default-features = false } +edgezero-adapter-cloudflare = { git = "https://github.com/stackpop/edgezero", branch = "feature/edgezero-deploy-actions", default-features = false } +edgezero-adapter-fastly = { git = "https://github.com/stackpop/edgezero", branch = "feature/edgezero-deploy-actions", default-features = false } +edgezero-adapter-spin = { git = "https://github.com/stackpop/edgezero", branch = "feature/edgezero-deploy-actions", default-features = false } +edgezero-cli = { git = "https://github.com/stackpop/edgezero", branch = "feature/edgezero-deploy-actions" } +edgezero-core = { git = "https://github.com/stackpop/edgezero", branch = "feature/edgezero-deploy-actions", default-features = false } env_logger = "0.11" error-stack = "0.6" fastly = "0.12" diff --git a/crates/trusted-server-cli/src/run.rs b/crates/trusted-server-cli/src/run.rs index 1b0bdfa29..b3be949a5 100644 --- a/crates/trusted-server-cli/src/run.rs +++ b/crates/trusted-server-cli/src/run.rs @@ -2,8 +2,8 @@ use std::process; use clap::{Parser, Subcommand}; use edgezero_cli::args::{ - AuthArgs, BuildArgs, ConfigDiffArgs, ConfigPushArgs, ConfigValidateArgs, DeployArgs, - ProvisionArgs, ServeArgs, + ActiveVersionArgs, AuthArgs, BuildArgs, ConfigDiffArgs, ConfigPushArgs, ConfigValidateArgs, + DeployArgs, HealthcheckArgs, ProvisionArgs, RollbackArgs, ServeArgs, }; use trusted_server_core::config::TrustedServerAppConfig; @@ -13,7 +13,7 @@ use crate::commands::config::init::{ConfigInitArgs, run_config_init}; use crate::prebid_bundle::{NpmPrebidBundleGenerator, PrebidBundleArgs, run_bundle}; #[derive(Debug, Parser)] -#[command(name = "ts", about = "Trusted Server CLI")] +#[command(name = "ts", version, about = "Trusted Server CLI")] struct Args { #[command(subcommand)] command: Command, @@ -21,6 +21,8 @@ struct Args { #[derive(Debug, Subcommand)] enum Command { + /// Print the currently active deployment version for a target adapter. + ActiveVersion(ActiveVersionArgs), /// Audit a public page and write draft Trusted Server artifacts. Audit(AuditArgs), /// Sign in / out / status against an `EdgeZero` adapter. @@ -32,10 +34,14 @@ enum Command { Config(ConfigCommand), /// Deploy the project through a target adapter. Deploy(DeployArgs), + /// Probe a deployed version until it reports healthy. + Healthcheck(HealthcheckArgs), /// Trusted Server Prebid commands. Prebid(PrebidArgs), /// Provision platform resources through a target adapter. Provision(ProvisionArgs), + /// Roll a service back to a previously active deployment version. + Rollback(RollbackArgs), /// Serve the project locally through a target adapter. Serve(ServeArgs), /// Local developer tools (e.g. the macOS-only production-hostname proxy). @@ -79,6 +85,7 @@ pub fn run_from_env() -> Result<(), String> { fn dispatch(args: Args) -> Result<(), String> { match args.command { + Command::ActiveVersion(args) => edgezero_cli::run_active_version(&args), Command::Audit(args) => { let stdout = std::io::stdout(); let mut out = stdout.lock(); @@ -102,6 +109,7 @@ fn dispatch(args: Args) -> Result<(), String> { edgezero_cli::run_config_validate_typed::(&args) } Command::Deploy(args) => edgezero_cli::run_deploy(&args), + Command::Healthcheck(args) => edgezero_cli::run_healthcheck(&args), Command::Prebid(prebid) => { let mut generator = NpmPrebidBundleGenerator; let mut stdout = std::io::stdout(); @@ -113,6 +121,7 @@ fn dispatch(args: Args) -> Result<(), String> { } } Command::Provision(args) => edgezero_cli::run_provision(&args), + Command::Rollback(args) => edgezero_cli::run_rollback(&args), Command::Serve(args) => edgezero_cli::run_serve(&args), Command::Dev(command) => crate::commands::dev::run(command), } @@ -131,6 +140,165 @@ mod tests { Args::try_parse_from(args).expect("should parse args") } + #[test] + fn parses_active_version() { + let args = parse(&[ + "ts", + "active-version", + "--adapter", + "fastly", + "--service-id", + "service-123", + ]); + let Command::ActiveVersion(active_version) = args.command else { + panic!("expected active-version command"); + }; + assert_eq!(active_version.adapter, "fastly"); + assert_eq!(active_version.service_id, "service-123"); + } + + #[test] + fn parses_healthcheck_with_retry_defaults() { + let args = parse(&[ + "ts", + "healthcheck", + "--adapter", + "fastly", + "--service-id", + "service-123", + "--version", + "7", + "--domain", + "edge.example", + ]); + let Command::Healthcheck(healthcheck) = args.command else { + panic!("expected healthcheck command"); + }; + assert_eq!(healthcheck.domain, "edge.example"); + assert_eq!(healthcheck.version, "7"); + assert_eq!(healthcheck.retry, 3, "should default to 3 retries"); + assert_eq!( + healthcheck.retry_delay, 5, + "should default to a 5s retry delay" + ); + assert_eq!(healthcheck.timeout, 10, "should default to a 10s timeout"); + assert!(!healthcheck.staging, "should probe production by default"); + } + + #[test] + fn parses_healthcheck_with_staging_overrides() { + let args = parse(&[ + "ts", + "healthcheck", + "--adapter", + "fastly", + "--service-id", + "service-123", + "--version", + "7", + "--domain", + "edge.example", + "--staging", + "--retry", + "9", + "--retry-delay", + "2", + "--timeout", + "30", + ]); + let Command::Healthcheck(healthcheck) = args.command else { + panic!("expected healthcheck command"); + }; + assert!(healthcheck.staging); + assert_eq!(healthcheck.retry, 9); + assert_eq!(healthcheck.retry_delay, 2); + assert_eq!(healthcheck.timeout, 30); + } + + #[test] + fn healthcheck_requires_domain() { + Args::try_parse_from([ + "ts", + "healthcheck", + "--adapter", + "fastly", + "--service-id", + "service-123", + "--version", + "7", + ]) + .expect_err("should reject healthcheck without a domain"); + } + + #[test] + fn parses_rollback_with_explicit_target() { + let args = parse(&[ + "ts", + "rollback", + "--adapter", + "fastly", + "--service-id", + "service-123", + "--version", + "8", + "--rollback-to", + "7", + ]); + let Command::Rollback(rollback) = args.command else { + panic!("expected rollback command"); + }; + assert_eq!(rollback.version, "8"); + assert_eq!(rollback.rollback_to, Some("7".to_owned())); + assert!(!rollback.staging); + } + + #[test] + fn parses_staging_rollback_without_target() { + let args = parse(&[ + "ts", + "rollback", + "--adapter", + "fastly", + "--service-id", + "service-123", + "--version", + "8", + "--staging", + ]); + let Command::Rollback(rollback) = args.command else { + panic!("expected rollback command"); + }; + assert!(rollback.staging); + assert_eq!( + rollback.rollback_to, None, + "staging rollback should not need an explicit target" + ); + } + + #[test] + fn rollback_requires_service_id() { + Args::try_parse_from(["ts", "rollback", "--adapter", "fastly", "--version", "8"]) + .expect_err("should reject rollback without a service id"); + } + + #[test] + fn parses_deploy_with_staging_flags() { + let args = parse(&[ + "ts", + "deploy", + "--adapter", + "fastly", + "--service-id", + "service-123", + "--stage", + ]); + let Command::Deploy(deploy) = args.command else { + panic!("expected deploy command"); + }; + assert_eq!(deploy.service_id, Some("service-123".to_owned())); + assert!(deploy.stage); + } + #[test] fn parses_audit_with_default_outputs() { let args = parse(&["ts", "audit", "https://publisher.example"]); diff --git a/crates/trusted-server-core/src/config.rs b/crates/trusted-server-core/src/config.rs index 7bbecd747..991ed7a2f 100644 --- a/crates/trusted-server-core/src/config.rs +++ b/crates/trusted-server-core/src/config.rs @@ -110,7 +110,9 @@ impl edgezero_core::app_config::AppConfigMeta for TrustedServerAppConfig { // app-config blob. Migrating app-level secrets to `EdgeZero` secret-store // references needs nested/array extraction support and operator migration // work tracked separately. - const SECRET_FIELDS: &'static [edgezero_core::app_config::SecretField] = &[]; + fn secret_fields() -> Vec { + Vec::new() + } } /// Runs Trusted Server deploy-time validation for pushed app config. From b743345fd7d43eaf36e0fce86ba448e27e9e5b6e Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Mon, 3 Aug 2026 17:51:39 -0700 Subject: [PATCH 02/13] Update EdgeZero to latest deploy-actions branch tip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-resolve the six edgezero-* deps from 145f1699 to bb441162 (current tip of feature/edgezero-deploy-actions, PR #316). The deploy staging flag was renamed there from --stage to --staging, standardizing on the same verb healthcheck/rollback/config-push already use; update the deploy CLI parse test to match. No production dispatch change is needed โ€” ts passes the edgezero-cli arg structs through, so the renamed flag is picked up automatically. --- Cargo.lock | 26 +++++++++++++------------- crates/trusted-server-cli/src/run.rs | 4 ++-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 72fa293ce..e5caa1a7a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -767,7 +767,7 @@ version = "3.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.48.0", ] [[package]] @@ -1398,7 +1398,7 @@ dependencies = [ [[package]] name = "edgezero-adapter" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#bb4411625856472b1279a3db49aeeac5e8b1507e" dependencies = [ "toml", ] @@ -1406,7 +1406,7 @@ dependencies = [ [[package]] name = "edgezero-adapter-axum" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#bb4411625856472b1279a3db49aeeac5e8b1507e" dependencies = [ "anyhow", "async-trait", @@ -1434,7 +1434,7 @@ dependencies = [ [[package]] name = "edgezero-adapter-cloudflare" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#bb4411625856472b1279a3db49aeeac5e8b1507e" dependencies = [ "anyhow", "async-trait", @@ -1457,7 +1457,7 @@ dependencies = [ [[package]] name = "edgezero-adapter-fastly" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#bb4411625856472b1279a3db49aeeac5e8b1507e" dependencies = [ "anyhow", "async-stream", @@ -1486,7 +1486,7 @@ dependencies = [ [[package]] name = "edgezero-adapter-spin" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#bb4411625856472b1279a3db49aeeac5e8b1507e" dependencies = [ "anyhow", "async-trait", @@ -1513,7 +1513,7 @@ dependencies = [ [[package]] name = "edgezero-cli" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#bb4411625856472b1279a3db49aeeac5e8b1507e" dependencies = [ "chrono", "clap", @@ -1538,7 +1538,7 @@ dependencies = [ [[package]] name = "edgezero-core" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#bb4411625856472b1279a3db49aeeac5e8b1507e" dependencies = [ "anyhow", "async-compression", @@ -1569,7 +1569,7 @@ dependencies = [ [[package]] name = "edgezero-macros" version = "0.1.0" -source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#145f1699d8e8f51027804f21faddbf985cff9a1e" +source = "git+https://github.com/stackpop/edgezero?branch=feature%2Fedgezero-deploy-actions#bb4411625856472b1279a3db49aeeac5e8b1507e" dependencies = [ "log", "proc-macro2", @@ -3604,7 +3604,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "be769465445e8c1474e9c5dac2018218498557af32d9ed057325ec9a41ae81bf" dependencies = [ "heck", - "itertools 0.13.0", + "itertools 0.10.5", "log", "multimap", "once_cell", @@ -3624,7 +3624,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8a56d757972c98b346a9b766e3f02746cde6dd1cd1d1d563472929fdd74bec4d" dependencies = [ "anyhow", - "itertools 0.13.0", + "itertools 0.10.5", "proc-macro2", "quote", "syn 2.0.118", @@ -3637,7 +3637,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b570b25f7617e43d59005d0990ccb79e950a423952cea19671b7a876da390adf" dependencies = [ "anyhow", - "itertools 0.13.0", + "itertools 0.10.5", "proc-macro2", "quote", "syn 2.0.118", @@ -5908,7 +5908,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.48.0", ] [[package]] diff --git a/crates/trusted-server-cli/src/run.rs b/crates/trusted-server-cli/src/run.rs index b3be949a5..7374c56a7 100644 --- a/crates/trusted-server-cli/src/run.rs +++ b/crates/trusted-server-cli/src/run.rs @@ -290,13 +290,13 @@ mod tests { "fastly", "--service-id", "service-123", - "--stage", + "--staging", ]); let Command::Deploy(deploy) = args.command else { panic!("expected deploy command"); }; assert_eq!(deploy.service_id, Some("service-123".to_owned())); - assert!(deploy.stage); + assert!(deploy.staging); } #[test] From 44f765878c783887a37e35cea2fb756b3949c0a9 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 3 Aug 2026 12:30:16 -0500 Subject: [PATCH 03/13] Add DataDome protection decision logs --- .../src/integrations/datadome.rs | 5 +- .../src/integrations/datadome/protection.rs | 81 ++++++++++++++++++- 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/crates/trusted-server-core/src/integrations/datadome.rs b/crates/trusted-server-core/src/integrations/datadome.rs index a46f626a6..fa5c79992 100644 --- a/crates/trusted-server-core/src/integrations/datadome.rs +++ b/crates/trusted-server-core/src/integrations/datadome.rs @@ -841,9 +841,10 @@ fn build( }; log::info!( - "[datadome] Registering integration (sdk_origin: {}, rewrite_sdk: {})", + "[datadome] Registering integration (sdk_origin: {}, rewrite_sdk: {}, enable_protection: {})", config.sdk_origin, - config.rewrite_sdk + config.rewrite_sdk, + config.enable_protection ); Ok(Some(DataDomeIntegration::try_new(config)?)) diff --git a/crates/trusted-server-core/src/integrations/datadome/protection.rs b/crates/trusted-server-core/src/integrations/datadome/protection.rs index c2759a864..a7c12bbc9 100644 --- a/crates/trusted-server-core/src/integrations/datadome/protection.rs +++ b/crates/trusted-server-core/src/integrations/datadome/protection.rs @@ -98,7 +98,13 @@ impl DataDomeIntegration { .change_context(Self::error("Failed to call DataDome Protection API")) .map_err(ProtectionRequestError::Runtime)?; - Ok(self.classify_protection_response(platform_response.response, input.request.method())) + let status = platform_response.response.status(); + let datadome_status = datadome_response_status(platform_response.response.headers()); + let decision = + self.classify_protection_response(platform_response.response, input.request.method()); + log_protection_result(&input, status, datadome_status, &decision); + + Ok(decision) } fn is_request_protected(&self, input: &RequestFilterInput<'_>) -> bool { @@ -126,7 +132,7 @@ impl DataDomeIntegration { match self.protection_scope.evaluate(&facts, input.services) { ProtectionScopeDecision::Protect => {} ProtectionScopeDecision::Skip { rule_id, reason } => { - log::debug!("[datadome] Skipping Protection API for rule {rule_id} ({reason})"); + log_protection_skip(input, &rule_id, reason); return false; } } @@ -413,6 +419,77 @@ impl DataDomeIntegration { } } +fn log_protection_skip(input: &RequestFilterInput<'_>, rule_id: &str, reason: &str) { + if matches!( + reason, + "client_ip" | "client_ip_source" | "ip_cidr" | "ip_cidr_source" + ) { + log::info!( + "[datadome] protection decision=skipped rule={} reason={} method={} host={} path={} client_ip={}", + rule_id, + reason, + input.request.method(), + request_host(input.request), + input.request.uri().path(), + client_ip_for_log(input), + ); + } else { + log::debug!( + "[datadome] protection decision=skipped rule={} reason={} method={} host={} path={} client_ip={}", + rule_id, + reason, + input.request.method(), + request_host(input.request), + input.request.uri().path(), + client_ip_for_log(input), + ); + } +} + +fn log_protection_result( + input: &RequestFilterInput<'_>, + status: StatusCode, + datadome_status: Option, + decision: &RequestFilterDecision, +) { + let method = input.request.method(); + let host = request_host(input.request); + let path = input.request.uri().path(); + let client_ip = client_ip_for_log(input); + + match decision { + RequestFilterDecision::Respond { .. } => log::info!( + "[datadome] protection decision=blocked status={} method={} host={} path={} client_ip={} route=short_circuit", + status.as_u16(), + method, + host, + path, + client_ip, + ), + RequestFilterDecision::Continue(_) + if status == StatusCode::OK && datadome_status == Some(status.as_u16()) => + { + log::info!( + "[datadome] protection decision=allowed status={} method={} host={} path={} client_ip={} route=continue", + status.as_u16(), + method, + host, + path, + client_ip, + ); + } + RequestFilterDecision::Continue(_) => {} + } +} + +fn client_ip_for_log(input: &RequestFilterInput<'_>) -> String { + input + .services + .client_info() + .client_ip + .map_or_else(|| "unknown".to_string(), |ip| ip.to_string()) +} + struct ProtectionPayload { fields: Vec<(String, String)>, uses_header_client_id: bool, From 302d161e32b3d8e29665d2211f36fb8f85765ebb Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 3 Aug 2026 14:15:41 -0500 Subject: [PATCH 04/13] Log incoming DataDome client IP --- .../src/integrations/datadome/protection.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/trusted-server-core/src/integrations/datadome/protection.rs b/crates/trusted-server-core/src/integrations/datadome/protection.rs index a7c12bbc9..a86791110 100644 --- a/crates/trusted-server-core/src/integrations/datadome/protection.rs +++ b/crates/trusted-server-core/src/integrations/datadome/protection.rs @@ -36,6 +36,16 @@ impl DataDomeIntegration { &self, input: RequestFilterInput<'_>, ) -> RequestFilterDecision { + if self.config.enable_protection { + log::info!( + "[datadome] protection incoming client_ip={} method={} host={} path={}", + client_ip_for_log(&input), + input.request.method(), + request_host(input.request), + input.request.uri().path(), + ); + } + if !self.config.enable_protection || !self.is_request_protected(&input) { return RequestFilterDecision::Continue(RequestFilterEffects::default()); } From aeb3f4c7e3257c6388c9036607763f528230618f Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 3 Aug 2026 17:52:25 -0500 Subject: [PATCH 05/13] feat(datadome): suppress client tag for excluded IPs --- .../benches/html_processor_bench.rs | 1 + .../trusted-server-core/src/html_processor.rs | 61 +++ .../src/integrations/datadome.rs | 31 +- .../src/integrations/datadome/protection.rs | 269 ++++++++-- .../src/integrations/registry.rs | 11 +- .../src/platform/test_support.rs | 19 + crates/trusted-server-core/src/publisher.rs | 181 ++++++- docs/guide/integrations/datadome.md | 27 + ...6-08-03-datadome-ip-excluded-client-tag.md | 475 ++++++++++++++++++ ...-datadome-ip-excluded-client-tag-design.md | 339 +++++++++++++ 10 files changed, 1378 insertions(+), 36 deletions(-) create mode 100644 docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md create mode 100644 docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md diff --git a/crates/trusted-server-core/benches/html_processor_bench.rs b/crates/trusted-server-core/benches/html_processor_bench.rs index 96eec2f1f..7c1303dd4 100644 --- a/crates/trusted-server-core/benches/html_processor_bench.rs +++ b/crates/trusted-server-core/benches/html_processor_bench.rs @@ -13,6 +13,7 @@ fn make_config() -> HtmlProcessorConfig { ad_bids_state: std::sync::Arc::new(std::sync::Mutex::new(None)), max_buffered_body_bytes: 16 * 1024 * 1024, gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, } } diff --git a/crates/trusted-server-core/src/html_processor.rs b/crates/trusted-server-core/src/html_processor.rs index 889234b56..4c827ace0 100644 --- a/crates/trusted-server-core/src/html_processor.rs +++ b/crates/trusted-server-core/src/html_processor.rs @@ -13,6 +13,7 @@ use lol_html::{ text, }; +use crate::integrations::datadome::{DATADOME_INTEGRATION_ID, DataDomeClientTagSuppressed}; use crate::integrations::gpt_diagnostics::GptDiagnosticsRequestDecision; use crate::integrations::{ AttributeRewriteOutcome, IntegrationAttributeContext, IntegrationDocumentState, @@ -175,6 +176,8 @@ pub struct HtmlProcessorConfig { pub max_buffered_body_bytes: usize, /// Request-scoped conditional diagnostics delivery decision. pub gpt_diagnostics: Option, + /// Whether to omit Trusted Server's automatic `DataDome` client-side tag. + pub suppress_datadome_client_side_tag: bool, } impl HtmlProcessorConfig { @@ -196,6 +199,7 @@ impl HtmlProcessorConfig { ad_bids_state: std::sync::Arc::new(std::sync::Mutex::new(None)), max_buffered_body_bytes: settings.publisher.max_buffered_body_bytes, gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, } } @@ -223,6 +227,13 @@ impl HtmlProcessorConfig { self.gpt_diagnostics = decision; self } + + /// Attach the request-scoped `DataDome` client-tag suppression decision. + #[must_use] + pub fn with_datadome_client_tag_suppression(mut self, suppress: bool) -> Self { + self.suppress_datadome_client_side_tag = suppress; + self + } } /// Create an HTML processor with URL replacement and integration hooks. @@ -235,6 +246,9 @@ impl HtmlProcessorConfig { pub fn create_html_processor(config: HtmlProcessorConfig) -> impl StreamProcessor { let post_processors = config.integrations.html_post_processors(); let document_state = IntegrationDocumentState::default(); + if config.suppress_datadome_client_side_tag { + document_state.get_or_insert_with(DATADOME_INTEGRATION_ID, || DataDomeClientTagSuppressed); + } // Simplified URL patterns structure - stores only core data and generates variants on-demand struct UrlPatterns { @@ -692,6 +706,7 @@ mod tests { ad_bids_state: std::sync::Arc::new(std::sync::Mutex::new(None)), max_buffered_body_bytes: 16 * 1024 * 1024, gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, } } @@ -950,6 +965,46 @@ mod tests { assert_eq!(config.request_scheme, "https"); } + #[test] + fn suppressed_datadome_tag_is_not_injected_into_processed_html() { + let mut settings = create_test_settings(); + settings + .integrations + .insert_config( + "datadome", + &json!({ + "enabled": true, + "client_side_key": "test-client-key", + }), + ) + .expect("should configure DataDome integration"); + let registry = IntegrationRegistry::new(&settings) + .expect("should create integration registry with DataDome"); + let config = HtmlProcessorConfig::from_settings( + &settings, + ®istry, + "origin.example.com", + "test.example.com", + "https", + ) + .with_datadome_client_tag_suppression(true); + let mut processor = create_html_processor(config); + + let output = processor + .process_chunk(b"content", true) + .expect("should process HTML"); + let html = String::from_utf8(output).expect("should produce UTF-8 HTML"); + + assert!( + !html.contains("window.ddjskey"), + "should omit the DataDome client configuration" + ); + assert!( + !html.contains("/integrations/datadome/tags.js"), + "should omit the DataDome client tag URL" + ); + } + #[test] fn test_real_publisher_html() { // Test with publisher HTML from test_publisher.html @@ -1539,6 +1594,7 @@ mod tests { ad_bids_state: std::sync::Arc::new(std::sync::Mutex::new(None)), max_buffered_body_bytes: 16 * 1024 * 1024, gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let mut processor = create_html_processor(config); let output = processor @@ -1613,6 +1669,7 @@ mod tests { ad_bids_state: state, max_buffered_body_bytes: 16 * 1024 * 1024, gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let mut processor = create_html_processor(config); let output = processor @@ -1649,6 +1706,7 @@ mod tests { ad_bids_state: state, max_buffered_body_bytes: 16 * 1024 * 1024, gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let mut processor = create_html_processor(config); // Malformed HTML with two elements (common in CMS template pages) @@ -1684,6 +1742,7 @@ mod tests { ad_bids_state: std::sync::Arc::new(std::sync::Mutex::new(None)), max_buffered_body_bytes: 16 * 1024 * 1024, gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let mut processor = create_html_processor(config); let output = processor @@ -1737,6 +1796,7 @@ mod tests { ad_bids_state: state, max_buffered_body_bytes: 16 * 1024 * 1024, gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let mut processor = create_html_processor(config); let output = processor @@ -1764,6 +1824,7 @@ mod tests { ad_bids_state: state, max_buffered_body_bytes: 16 * 1024 * 1024, gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let mut processor = create_html_processor(config); let output = processor diff --git a/crates/trusted-server-core/src/integrations/datadome.rs b/crates/trusted-server-core/src/integrations/datadome.rs index fa5c79992..db5932fb2 100644 --- a/crates/trusted-server-core/src/integrations/datadome.rs +++ b/crates/trusted-server-core/src/integrations/datadome.rs @@ -88,7 +88,12 @@ pub use protection_scope::{ use protection_scope::ProtectionScope; -pub(super) const DATADOME_INTEGRATION_ID: &str = "datadome"; +pub(crate) const DATADOME_INTEGRATION_ID: &str = "datadome"; + +/// Request marker indicating that Trusted Server should omit its automatic +/// `DataDome` client-side tag for the current response. +#[derive(Debug, Clone, Copy)] +pub(crate) struct DataDomeClientTagSuppressed; /// Regex pattern for matching and rewriting `DataDome` URLs in script content. /// @@ -765,7 +770,15 @@ impl IntegrationHeadInjector for DataDomeIntegration { DATADOME_INTEGRATION_ID } - fn head_inserts(&self, _ctx: &IntegrationHtmlContext<'_>) -> Vec { + fn head_inserts(&self, ctx: &IntegrationHtmlContext<'_>) -> Vec { + if ctx + .document_state + .get::(DATADOME_INTEGRATION_ID) + .is_some() + { + return Vec::new(); + } + if !self.config.inject_client_side_tag || self.config.client_side_key.trim().is_empty() { return Vec::new(); } @@ -1249,6 +1262,20 @@ mod tests { #[test] fn head_injector_omits_client_side_tag_when_disabled_or_blank() { + let mut suppressed = test_config(); + suppressed.client_side_key = "test-client-key".to_string(); + let suppressed_integration = DataDomeIntegration::new(suppressed); + let suppressed_state = crate::integrations::IntegrationDocumentState::default(); + suppressed_state + .get_or_insert_with(DATADOME_INTEGRATION_ID, || DataDomeClientTagSuppressed); + let suppressed_ctx = html_context_for_tests(&suppressed_state); + assert!( + suppressed_integration + .head_inserts(&suppressed_ctx) + .is_empty(), + "should omit the tag when the request is IP-excluded" + ); + let mut blank_key = test_config(); blank_key.client_side_key = " ".to_string(); let integration = DataDomeIntegration::new(blank_key); diff --git a/crates/trusted-server-core/src/integrations/datadome/protection.rs b/crates/trusted-server-core/src/integrations/datadome/protection.rs index a86791110..4c74c71e8 100644 --- a/crates/trusted-server-core/src/integrations/datadome/protection.rs +++ b/crates/trusted-server-core/src/integrations/datadome/protection.rs @@ -34,19 +34,18 @@ enum ProtectionRequestError { impl DataDomeIntegration { pub(super) async fn filter_protection_request( &self, - input: RequestFilterInput<'_>, + mut input: RequestFilterInput<'_>, ) -> RequestFilterDecision { if self.config.enable_protection { log::info!( - "[datadome] protection incoming client_ip={} method={} host={} path={}", - client_ip_for_log(&input), + "[datadome] protection incoming method={} host={} path={}", input.request.method(), request_host(input.request), input.request.uri().path(), ); } - if !self.config.enable_protection || !self.is_request_protected(&input) { + if !self.config.enable_protection || !self.is_request_protected(&mut input) { return RequestFilterDecision::Continue(RequestFilterEffects::default()); } @@ -117,8 +116,8 @@ impl DataDomeIntegration { Ok(decision) } - fn is_request_protected(&self, input: &RequestFilterInput<'_>) -> bool { - let req = input.request; + fn is_request_protected(&self, input: &mut RequestFilterInput<'_>) -> bool { + let req = &*input.request; if req.method() == Method::OPTIONS { return false; } @@ -142,6 +141,13 @@ impl DataDomeIntegration { match self.protection_scope.evaluate(&facts, input.services) { ProtectionScopeDecision::Protect => {} ProtectionScopeDecision::Skip { rule_id, reason } => { + let client_tag_omitted = is_ip_exclusion_reason(reason); + if client_tag_omitted { + input + .request + .extensions_mut() + .insert(super::DataDomeClientTagSuppressed); + } log_protection_skip(input, &rule_id, reason); return false; } @@ -210,7 +216,7 @@ impl DataDomeIntegration { input: &RequestFilterInput<'_>, server_side_key: &Redacted, ) -> ProtectionPayload { - let req = input.request; + let req = &*input.request; let client_info = input.services.client_info(); let mut fields = Vec::new(); let header_client_id = header_value(req, HEADER_DATADOME_CLIENT_ID); @@ -429,29 +435,31 @@ impl DataDomeIntegration { } } -fn log_protection_skip(input: &RequestFilterInput<'_>, rule_id: &str, reason: &str) { - if matches!( +fn is_ip_exclusion_reason(reason: &str) -> bool { + matches!( reason, "client_ip" | "client_ip_source" | "ip_cidr" | "ip_cidr_source" - ) { + ) +} + +fn log_protection_skip(input: &RequestFilterInput<'_>, rule_id: &str, reason: &str) { + if is_ip_exclusion_reason(reason) { log::info!( - "[datadome] protection decision=skipped rule={} reason={} method={} host={} path={} client_ip={}", + "[datadome] protection decision=skipped rule={} reason={} client_tag=omitted method={} host={} path={}", rule_id, reason, input.request.method(), request_host(input.request), input.request.uri().path(), - client_ip_for_log(input), ); } else { log::debug!( - "[datadome] protection decision=skipped rule={} reason={} method={} host={} path={} client_ip={}", + "[datadome] protection decision=skipped rule={} reason={} method={} host={} path={}", rule_id, reason, input.request.method(), request_host(input.request), input.request.uri().path(), - client_ip_for_log(input), ); } } @@ -465,41 +473,30 @@ fn log_protection_result( let method = input.request.method(); let host = request_host(input.request); let path = input.request.uri().path(); - let client_ip = client_ip_for_log(input); match decision { RequestFilterDecision::Respond { .. } => log::info!( - "[datadome] protection decision=blocked status={} method={} host={} path={} client_ip={} route=short_circuit", + "[datadome] protection decision=blocked status={} method={} host={} path={} route=short_circuit", status.as_u16(), method, host, path, - client_ip, ), RequestFilterDecision::Continue(_) if status == StatusCode::OK && datadome_status == Some(status.as_u16()) => { log::info!( - "[datadome] protection decision=allowed status={} method={} host={} path={} client_ip={} route=continue", + "[datadome] protection decision=allowed status={} method={} host={} path={} route=continue", status.as_u16(), method, host, path, - client_ip, ); } RequestFilterDecision::Continue(_) => {} } } -fn client_ip_for_log(input: &RequestFilterInput<'_>) -> String { - input - .services - .client_info() - .client_ip - .map_or_else(|| "unknown".to_string(), |ip| ip.to_string()) -} - struct ProtectionPayload { fields: Vec<(String, String)>, uses_header_client_id: bool, @@ -732,11 +729,17 @@ fn truncate_utf8(value: &str, limit: i32) -> String { #[cfg(test)] mod tests { use std::collections::HashMap; + use std::net::{IpAddr, Ipv4Addr}; use std::sync::Arc; - use crate::integrations::datadome::DataDomeConfig; + use crate::integrations::datadome::{ + DataDomeConfig, ProtectionExclusionRuleConfig, ProtectionMatcherConfig, + }; + use crate::platform::GeoInfo; use crate::platform::test_support::{ - HashMapSecretStore, NoopConfigStore, NoopSecretStore, build_services_with_config_and_secret, + HashMapConfigStore, HashMapSecretStore, NoopConfigStore, NoopSecretStore, + build_services_with_config_and_secret, build_services_with_config_and_secret_and_client_ip, + noop_services_with_client_ip, }; use crate::settings::Settings; @@ -751,6 +754,210 @@ mod tests { DataDomeIntegration::try_new(config).expect("should create integration") } + fn request_for_filter() -> Request { + request_builder() + .method(Method::GET.as_str()) + .uri("https://publisher.example/page") + .body(EdgeBody::empty()) + .expect("should build filter request") + } + + fn filter_marks_request( + config: DataDomeConfig, + services: &RuntimeServices, + ) -> Request { + filter_marks_request_with_geo(config, services, None) + } + + fn filter_marks_request_with_geo( + config: DataDomeConfig, + services: &RuntimeServices, + geo_info: Option<&GeoInfo>, + ) -> Request { + let integration = + DataDomeIntegration::try_new(config).expect("should create DataDome integration"); + let settings = Settings::default(); + let mut request = request_for_filter(); + let decision = futures::executor::block_on(integration.filter_protection_request( + RequestFilterInput { + settings: &settings, + services, + request: &mut request, + geo_info, + is_integration_route: false, + }, + )); + assert!( + matches!(decision, RequestFilterDecision::Continue(_)), + "an excluded request should continue without a Protection API response" + ); + request + } + + fn has_client_tag_suppression_marker(request: &Request) -> bool { + request + .extensions() + .get::() + .is_some() + } + + #[test] + fn ip_exclusions_mark_requests_for_client_tag_suppression() { + let ip = IpAddr::V4(Ipv4Addr::new(192, 0, 2, 10)); + let mut inline = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_excluded_ip_cidrs: vec!["192.0.2.0/24".to_string()], + ..DataDomeConfig::default() + }; + let inline_request = + filter_marks_request(inline.clone(), &noop_services_with_client_ip(ip)); + assert!( + has_client_tag_suppression_marker(&inline_request), + "inline IP exclusions should mark the request" + ); + + inline.protection_excluded_ip_cidrs.clear(); + inline.protection_excluded_ip_cidr_sources = + vec![super::super::ProtectionIpCidrSourceConfig { + config_store: "datadome-test-source".to_string(), + key: "inline-source".to_string(), + }]; + let mut source_values = HashMap::new(); + source_values.insert("inline-source".to_string(), "192.0.2.0/24".to_string()); + let source_services = build_services_with_config_and_secret_and_client_ip( + HashMapConfigStore::new(source_values), + NoopSecretStore, + ip, + ); + let source_request = filter_marks_request(inline, &source_services); + assert!( + has_client_tag_suppression_marker(&source_request), + "Config Store IP exclusions should mark the request" + ); + + let structured_ip = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_exclusion_rules: vec![ProtectionExclusionRuleConfig { + id: "structured-ip".to_string(), + enabled: true, + methods: Vec::new(), + matcher: ProtectionMatcherConfig::IpCidr { + cidrs: vec!["192.0.2.0/24".to_string()], + }, + }], + ..DataDomeConfig::default() + }; + let structured_request = + filter_marks_request(structured_ip, &noop_services_with_client_ip(ip)); + assert!( + has_client_tag_suppression_marker(&structured_request), + "structured IP exclusions should mark the request" + ); + + let structured_source = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_exclusion_rules: vec![ProtectionExclusionRuleConfig { + id: "structured-ip-source".to_string(), + enabled: true, + methods: Vec::new(), + matcher: ProtectionMatcherConfig::IpCidrSource { + config_store: "datadome-test-source".to_string(), + key: "structured-source".to_string(), + }, + }], + ..DataDomeConfig::default() + }; + let mut structured_values = HashMap::new(); + structured_values.insert("structured-source".to_string(), "192.0.2.0/24".to_string()); + let structured_services = build_services_with_config_and_secret_and_client_ip( + HashMapConfigStore::new(structured_values), + NoopSecretStore, + ip, + ); + let structured_source_request = + filter_marks_request(structured_source, &structured_services); + assert!( + has_client_tag_suppression_marker(&structured_source_request), + "structured Config Store IP exclusions should mark the request" + ); + } + + #[test] + fn non_ip_exclusions_do_not_mark_requests_for_client_tag_suppression() { + let ip = IpAddr::V4(Ipv4Addr::new(192, 0, 2, 10)); + let cases = [DataDomeConfig { + enabled: true, + enable_protection: true, + protection_exclusion_rules: vec![ProtectionExclusionRuleConfig { + id: "path".to_string(), + enabled: true, + methods: Vec::new(), + matcher: ProtectionMatcherConfig::PathExact { + paths: vec!["/page".to_string()], + }, + }], + ..DataDomeConfig::default() + }]; + + for config in cases { + let request = filter_marks_request(config, &noop_services_with_client_ip(ip)); + assert!( + !has_client_tag_suppression_marker(&request), + "non-IP exclusions should not mark the request" + ); + } + } + + #[test] + fn asn_exclusions_do_not_mark_requests_for_client_tag_suppression() { + let config = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_excluded_asns: vec![64500], + ..DataDomeConfig::default() + }; + let geo_info = GeoInfo { + city: String::new(), + country: String::new(), + continent: String::new(), + latitude: 0.0, + longitude: 0.0, + metro_code: 0, + region: None, + asn: Some(64500), + }; + let request = filter_marks_request_with_geo( + config, + &noop_services_with_client_ip(IpAddr::V4(Ipv4Addr::new(192, 0, 2, 10))), + Some(&geo_info), + ); + assert!( + !has_client_tag_suppression_marker(&request), + "ASN exclusions should not mark the request" + ); + } + + #[test] + fn non_matching_ip_does_not_mark_request_for_client_tag_suppression() { + let config = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_excluded_ip_cidrs: vec!["192.0.2.0/24".to_string()], + ..DataDomeConfig::default() + }; + let request = filter_marks_request( + config, + &noop_services_with_client_ip(IpAddr::V4(Ipv4Addr::new(198, 51, 100, 10))), + ); + assert!( + !has_client_tag_suppression_marker(&request), + "a non-matching IP should not mark the request" + ); + } + #[test] fn load_server_side_key_reads_secret_store() { let mut secrets = HashMap::new(); @@ -835,7 +1042,7 @@ mod tests { // the Protection API. let services = build_services_with_config_and_secret(NoopConfigStore, NoopSecretStore); let settings = Settings::default(); - let request = request_builder() + let mut request = request_builder() .method(Method::OPTIONS.as_str()) .uri("https://publisher.example/_ts/api/v1/identify") .body(EdgeBody::empty()) @@ -846,7 +1053,7 @@ mod tests { RequestFilterInput { settings: &settings, services: &services, - request: &request, + request: &mut request, geo_info: None, is_integration_route: false, }, diff --git a/crates/trusted-server-core/src/integrations/registry.rs b/crates/trusted-server-core/src/integrations/registry.rs index 0644fb522..291a54242 100644 --- a/crates/trusted-server-core/src/integrations/registry.rs +++ b/crates/trusted-server-core/src/integrations/registry.rs @@ -327,7 +327,7 @@ pub trait IntegrationProxy: Send + Sync { pub struct RequestFilterInput<'a> { pub settings: &'a Settings, pub services: &'a RuntimeServices, - pub request: &'a Request, + pub request: &'a mut Request, pub geo_info: Option<&'a GeoInfo>, /// Whether the request matches a registered integration proxy route. pub is_integration_route: bool, @@ -1345,6 +1345,8 @@ mod tests { } struct EnrichingRequestFilter; + #[derive(Clone, Copy)] + struct RequestAnnotation; #[async_trait(?Send)] impl IntegrationRequestFilter for EnrichingRequestFilter { @@ -1354,8 +1356,9 @@ mod tests { async fn filter_request( &self, - _input: RequestFilterInput<'_>, + input: RequestFilterInput<'_>, ) -> Result> { + input.request.extensions_mut().insert(RequestAnnotation); Ok(RequestFilterDecision::Continue(RequestFilterEffects { request_headers: vec![HeaderMutation::set("x-datadome-isbot", "1")], response_headers: vec![HeaderMutation::set("x-dd-b", "allowed")], @@ -1487,6 +1490,10 @@ mod tests { Some("1"), "should apply DataDome-style request enrichment before routing" ); + assert!( + req.extensions().get::().is_some(), + "should preserve private request annotations for downstream routing" + ); match outcome { RequestFilterRegistryOutcome::Continue(effects) => { assert_eq!( diff --git a/crates/trusted-server-core/src/platform/test_support.rs b/crates/trusted-server-core/src/platform/test_support.rs index 94b42161c..13b6d70cd 100644 --- a/crates/trusted-server-core/src/platform/test_support.rs +++ b/crates/trusted-server-core/src/platform/test_support.rs @@ -631,6 +631,25 @@ pub(crate) fn build_services_with_config_and_secret( .build() } +pub(crate) fn build_services_with_config_and_secret_and_client_ip( + config_store: impl PlatformConfigStore + 'static, + secret_store: impl PlatformSecretStore + 'static, + client_ip: IpAddr, +) -> RuntimeServices { + RuntimeServices::builder() + .config_store(Arc::new(config_store)) + .secret_store(Arc::new(secret_store)) + .kv_store(Arc::new(edgezero_core::key_value_store::NoopKvStore)) + .backend(Arc::new(NoopBackend)) + .http_client(Arc::new(NoopHttpClient)) + .geo(Arc::new(NoopGeo)) + .client_info(ClientInfo { + client_ip: Some(client_ip), + ..ClientInfo::default() + }) + .build() +} + pub(crate) fn build_request_signing_services() -> RuntimeServices { let signing_key = SigningKey::generate(&mut OsRng); let key_b64 = general_purpose::STANDARD.encode(signing_key.as_bytes()); diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index d3410b4ed..48e0eaacf 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -357,6 +357,7 @@ struct ProcessResponseParams<'a> { integration_registry: &'a IntegrationRegistry, ad_slots_script: Option<&'a str>, ad_bids_state: &'a Arc>>, + suppress_datadome_client_side_tag: bool, gpt_diagnostics: Option<&'a crate::integrations::gpt_diagnostics::GptDiagnosticsRequestDecision>, } @@ -383,6 +384,7 @@ impl PublisherBodyProcessor { integration_registry, ad_slots_script: params.ad_slots_script.as_deref().map(str::to_string), ad_bids_state: Arc::clone(¶ms.ad_bids_state), + suppress_datadome_client_side_tag: params.suppress_datadome_client_side_tag, gpt_diagnostics: params.gpt_diagnostics.clone(), })?) } else if is_rsc_flight { @@ -460,6 +462,7 @@ fn process_response_streaming( integration_registry: params.integration_registry, ad_slots_script: params.ad_slots_script.map(str::to_string), ad_bids_state: params.ad_bids_state.clone(), + suppress_datadome_client_side_tag: params.suppress_datadome_client_side_tag, gpt_diagnostics: params.gpt_diagnostics.cloned(), })?; StreamingPipeline::new(config, processor) @@ -944,6 +947,7 @@ struct HtmlStreamProcessorParams<'a> { integration_registry: &'a IntegrationRegistry, ad_slots_script: Option, ad_bids_state: Arc>>, + suppress_datadome_client_side_tag: bool, gpt_diagnostics: Option, } @@ -960,7 +964,8 @@ fn create_html_stream_processor( params.request_scheme, ) .with_ad_state(params.ad_slots_script, params.ad_bids_state) - .with_gpt_diagnostics(params.gpt_diagnostics); + .with_gpt_diagnostics(params.gpt_diagnostics) + .with_datadome_client_tag_suppression(params.suppress_datadome_client_side_tag); Ok(create_html_processor(config)) } @@ -1081,6 +1086,8 @@ pub struct OwnedProcessResponseParams { pub(crate) dispatched_auction: Option, /// Price granularity used to bucket bids when building `tsjs.bids`. pub(crate) price_granularity: PriceGranularity, + /// Whether to omit Trusted Server's automatic `DataDome` client-side tag. + pub(crate) suppress_datadome_client_side_tag: bool, /// Request-scoped conditional diagnostics delivery decision. pub(crate) gpt_diagnostics: Option, @@ -1431,6 +1438,28 @@ fn response_carries_body(method: &Method, status: StatusCode) -> bool { && status != StatusCode::NOT_MODIFIED } +/// Prevent shared caches from replaying tag-suppressed HTML to other clients. +fn apply_datadome_client_tag_cache_privacy( + response: &mut Response, + method: &Method, + suppress_datadome_client_side_tag: bool, + content_type: &str, +) { + if !suppress_datadome_client_side_tag + || !response_carries_body(method, response.status()) + || !is_html_content_type(content_type) + { + return; + } + + response.headers_mut().insert( + header::CACHE_CONTROL, + HeaderValue::from_static("private, max-age=0"), + ); + response.headers_mut().remove("surrogate-control"); + response.headers_mut().remove("fastly-surrogate-control"); +} + /// Drop a bodiless response's body and correct its framing headers. /// /// The response keeps no body, and its `Content-Length` is corrected where the @@ -1547,6 +1576,7 @@ pub fn stream_publisher_body( integration_registry, ad_slots_script: params.ad_slots_script.as_deref(), ad_bids_state: ¶ms.ad_bids_state, + suppress_datadome_client_side_tag: params.suppress_datadome_client_side_tag, gpt_diagnostics: params.gpt_diagnostics.as_ref(), }; process_response_streaming(body, output, &borrowed) @@ -1640,6 +1670,7 @@ pub async fn stream_publisher_body_async( integration_registry, ad_slots_script: params.ad_slots_script.as_deref().map(str::to_string), ad_bids_state: params.ad_bids_state.clone(), + suppress_datadome_client_side_tag: params.suppress_datadome_client_side_tag, gpt_diagnostics: params.gpt_diagnostics.clone(), }) { Ok(processor) => processor, @@ -2859,6 +2890,11 @@ pub async fn handle_publisher_request( // sets the flag unconditionally and tolerates buffered fallback): adapters // without streaming support may reject the flag outright rather than // silently buffering, which would fail every publisher fetch. + let request_method = req.method().clone(); + let suppress_datadome_client_side_tag = req + .extensions() + .get::() + .is_some(); let mut platform_request = PlatformHttpRequest::new(req, backend_name); if services.http_client().supports_streaming_responses() { platform_request = platform_request.with_stream_response(); @@ -3051,6 +3087,12 @@ pub async fn handle_publisher_request( content_encoding ); + apply_datadome_client_tag_cache_privacy( + &mut response, + &request_method, + suppress_datadome_client_side_tag, + &content_type, + ); let body = std::mem::replace(response.body_mut(), EdgeBody::empty()); response.headers_mut().remove(header::CONTENT_LENGTH); @@ -3066,6 +3108,7 @@ pub async fn handle_publisher_request( content_type, ad_slots_script: ad_slots_script.clone(), ad_bids_state: ad_bids_state.clone(), + suppress_datadome_client_side_tag, auction_observation, auction_request: auction_request_for_telemetry, dispatched_auction, @@ -4282,6 +4325,7 @@ mod tests { dispatched_auction: None, price_granularity: Default::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, } } @@ -5274,6 +5318,121 @@ mod tests { ); } + #[test] + fn suppressed_datadome_tag_reaches_publisher_html_pipeline() { + let mut settings = create_test_settings(); + settings + .integrations + .insert_config( + "datadome", + &serde_json::json!({ + "enabled": true, + "client_side_key": "test-client-key", + }), + ) + .expect("should configure DataDome integration"); + let registry = IntegrationRegistry::new(&settings) + .expect("should create integration registry with DataDome"); + let mut params = make_stream_params(&settings, "identity"); + params.content_type = "text/html; charset=utf-8".to_string(); + params.suppress_datadome_client_side_tag = true; + let mut output = Vec::new(); + + stream_publisher_body( + EdgeBody::from(b"content".to_vec()), + &mut output, + ¶ms, + &settings, + ®istry, + ) + .expect("should process suppressed HTML"); + + let html = String::from_utf8(output).expect("should produce UTF-8 HTML"); + assert!( + !html.contains("window.ddjskey"), + "publisher processing should omit the DataDome client configuration" + ); + assert!( + !html.contains("/integrations/datadome/tags.js"), + "publisher processing should omit the DataDome client tag URL" + ); + } + + #[test] + fn suppressed_datadome_html_is_private_and_not_shared_cached() { + let mut response = Response::builder() + .status(StatusCode::OK) + .header(header::CACHE_CONTROL, "public, max-age=600") + .header("surrogate-control", "max-age=600") + .header("fastly-surrogate-control", "max-age=600") + .body(EdgeBody::empty()) + .expect("should build cacheable HTML response"); + + super::apply_datadome_client_tag_cache_privacy( + &mut response, + &Method::GET, + true, + "text/html; charset=utf-8", + ); + + assert_eq!( + response + .headers() + .get(header::CACHE_CONTROL) + .and_then(|value| value.to_str().ok()), + Some("private, max-age=0"), + "suppressed HTML should be private" + ); + assert!( + response.headers().get("surrogate-control").is_none(), + "suppressed HTML should not retain Surrogate-Control" + ); + assert!( + response.headers().get("fastly-surrogate-control").is_none(), + "suppressed HTML should not retain Fastly-Surrogate-Control" + ); + } + + #[test] + fn datadome_cache_privacy_does_not_change_non_html_or_unsuppressed_responses() { + let mut response = Response::builder() + .status(StatusCode::OK) + .header(header::CACHE_CONTROL, "public, max-age=600") + .header("surrogate-control", "max-age=600") + .body(EdgeBody::empty()) + .expect("should build cacheable response"); + + super::apply_datadome_client_tag_cache_privacy( + &mut response, + &Method::GET, + false, + "text/html; charset=utf-8", + ); + assert_eq!( + response + .headers() + .get(header::CACHE_CONTROL) + .and_then(|value| value.to_str().ok()), + Some("public, max-age=600"), + "unsuppressed HTML should retain its existing cache policy" + ); + + super::apply_datadome_client_tag_cache_privacy( + &mut response, + &Method::GET, + true, + "text/css", + ); + assert_eq!( + response + .headers() + .get(header::CACHE_CONTROL) + .and_then(|value| value.to_str().ok()), + Some("public, max-age=600"), + "non-HTML should retain its existing cache policy" + ); + } + #[test] fn response_carries_body_preserves_bodiless_metadata() { // A processable GET 200 buffers a body and recomputes Content-Length. @@ -6229,6 +6388,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let mut output = Vec::new(); @@ -6277,6 +6437,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let mut output = Vec::new(); @@ -6314,6 +6475,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let body = EdgeBody::from_stream(futures::stream::iter(vec![Ok::<_, io::Error>( bytes::Bytes::from_static(b"live"), @@ -6429,6 +6591,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let body = EdgeBody::stream(futures::stream::iter(vec![ bytes::Bytes::from_static(b"body{background:url('https://origin.example.com/"), @@ -6482,6 +6645,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let compressed = gzip_encode(b"body{background:url('https://origin.example.com/asset.png')}"); @@ -6538,6 +6702,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let compressed = deflate_encode(b"body{background:url('https://origin.example.com/asset.png')}"); @@ -6594,6 +6759,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let compressed = brotli_encode(b"body{background:url('https://origin.example.com/asset.png')}"); @@ -6650,6 +6816,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let compressed = brotli_encode(b"body{background:url('https://origin.example.com/asset.png')}"); @@ -6694,6 +6861,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, } } @@ -6888,6 +7056,7 @@ mod tests { )), price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let body = EdgeBody::stream(futures::stream::iter(vec![ bytes::Bytes::from_static(b"hello"), @@ -6952,6 +7121,7 @@ mod tests { )), price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; // The `` that triggers bid injection lives in the SECOND gzip // member. `flate2::read::GzDecoder` decodes only the first member, so @@ -7015,6 +7185,7 @@ mod tests { )), price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let body = EdgeBody::stream(futures::stream::iter(vec![bytes::Bytes::from_static( b"body{background:url('https://origin.example.com/asset.png')}", @@ -7071,6 +7242,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let publisher_response = PublisherResponse::Stream { response, @@ -7207,6 +7379,7 @@ mod tests { dispatched_auction, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, } } @@ -7558,6 +7731,7 @@ mod tests { )), price_granularity: PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, } }; let make_stream_response = || PublisherResponse::Stream { @@ -7737,6 +7911,7 @@ mod tests { )), price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let publisher_response = PublisherResponse::Stream { response, @@ -7804,6 +7979,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let mut output = Vec::new(); @@ -7854,6 +8030,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let bogus_body = EdgeBody::from(b"not gzip".to_vec()); @@ -7962,6 +8139,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let mut output = Vec::new(); stream_publisher_body(body, &mut output, ¶ms, &settings, ®istry) @@ -8019,6 +8197,7 @@ mod tests { dispatched_auction: None, price_granularity: crate::price_bucket::PriceGranularity::default(), gpt_diagnostics: None, + suppress_datadome_client_side_tag: false, }; let mut output = Vec::new(); diff --git a/docs/guide/integrations/datadome.md b/docs/guide/integrations/datadome.md index 9c342679b..1743a1b75 100644 --- a/docs/guide/integrations/datadome.md +++ b/docs/guide/integrations/datadome.md @@ -173,6 +173,33 @@ Static assets are excluded by default using a case-insensitive file-extension re Auction traffic at `/auction` is protected by default. +### IP-excluded client-side tag behavior + +On the Fastly adapter, a request that matches an IP-based DataDome exclusion +also omits Trusted Server's automatically injected client-side DataDome tag +from processed HTML. This keeps the client-side layer consistent with the +server-side Protection API skip. + +This behavior applies to: + +- `protection_excluded_ip_cidrs`; +- `protection_excluded_ip_cidr_sources`; +- structured `ip_cidr` rules; and +- structured `ip_cidr_source` rules. + +ASN, method, path, query-parameter, static-asset, and internal-route +exclusions do not automatically suppress the client-side tag. DataDome tags +already present in publisher HTML are not removed or changed by this behavior, +and `/integrations/datadome/tags.js` remains available when requested directly. + +Because the processed HTML differs by client IP, tag-suppressed HTML is marked +`private, max-age=0` and removed from shared surrogate caches. The decision is +reported in the existing protection log, for example: + +```text +[datadome] protection decision=skipped rule=excluded-ip-cidrs reason=client_ip client_tag=omitted method=GET host=example.com path=/page +``` + ### Structured exclusion rules Use structured rules for all DataDome protection exclusions. Each rule has an `id`, optional `methods`, and a typed matcher. The default configuration includes a `path_regex` rule for common static assets. diff --git a/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md b/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md new file mode 100644 index 000000000..5ea5f6ff3 --- /dev/null +++ b/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md @@ -0,0 +1,475 @@ +# DataDome IP-excluded client tag suppression โ€” Implementation Plan + +> **Status:** Approved for implementation +> +> **For implementers:** Work task by task and keep the workspace buildable. +> Follow `CLAUDE.md`: use target-matched Cargo aliases, do not use bare +> workspace tests, and do not add an internal HTTP header for request state. + +**Goal:** When Fastly's authoritative client IP matches a DataDome IP exclusion, +skip the Protection API call and omit only Trusted Server's automatically +injected DataDome client tag from every processed HTML response. + +**Issue:** #994 +**Design:** +`docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md` + +## Approved behavior + +| Request condition | Protection API | Trusted Server auto-injected tag | Publisher-originated tag | +| ------------------------------------------------------------- | --------------- | -------------------------------- | ------------------------ | +| Inline IP CIDR match | Skipped | Omitted | Unchanged | +| Config Store IP CIDR-source match | Skipped | Omitted | Unchanged | +| Structured `ip_cidr` match | Skipped | Omitted | Unchanged | +| Structured `ip_cidr_source` match | Skipped | Omitted | Unchanged | +| ASN, method, path, query, static, or internal-route exclusion | Skipped | Preserved | Unchanged | +| No exclusion match | Called normally | Preserved | Unchanged | +| Protection API fail-open | Continued | Preserved | Unchanged | + +The Fastly-only scope means that other adapters receive the default +non-suppressed value. Do not add a configuration option and do not modify their +request-filter wiring. + +## Runtime contracts + +1. **Trusted identity source:** determine exclusion from + `RuntimeServices::client_info().client_ip`, never a caller-provided header. +2. **Single evaluation:** use the existing `ProtectionScope` decision. Do not + evaluate CIDRs a second time while injecting HTML; this avoids diverging + Config Store/cache behavior. +3. **Private marker:** communicate the decision with a typed request extension, + never a request/response header. The marker cannot leak to the origin or + client. +4. **Precise scope:** tag suppression is keyed only on decision reasons + `client_ip`, `client_ip_source`, `ip_cidr`, and `ip_cidr_source`. +5. **Cache safety:** an HTML response with the tag omitted differs by client IP. + A suppressed processed HTML response must be `private, max-age=0` and have + `Surrogate-Control` and `Fastly-Surrogate-Control` removed. Do not alter + cache headers when the response is not processed HTML, because this feature + does not alter that body. +6. **No behavior drift:** DataDome proxy endpoints, response-header effects, + `rewrite_sdk`, and DataDome tags that were already in origin HTML retain + their current behavior. + +## File map + +| File | Change | +| ----------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `crates/trusted-server-core/src/integrations/registry.rs` | Permit filters to attach private typed request extensions while retaining header-effect semantics. Extend the HTML context with the propagated boolean. | +| `crates/trusted-server-core/src/integrations/datadome.rs` | Define the crate-private marker and have the head injector honor the HTML-context flag. | +| `crates/trusted-server-core/src/integrations/datadome/protection.rs` | Recognize IP scope skips, attach the marker, and add `client_tag=omitted` to the existing info log. | +| `crates/trusted-server-core/src/html_processor.rs` | Carry the per-response suppression boolean from config to all integration HTML contexts. | +| `crates/trusted-server-core/src/publisher.rs` | Snapshot the marker before origin dispatch, propagate it through every HTML streaming path, and apply cache privacy to suppressed processed HTML. | +| `docs/guide/integrations/datadome.md` | Document the Fastly IP-exclusion behavior and its limits. | +| `docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md` | Already updated with the cache-variance safeguard. | + +No changes are expected in `trusted-server.example.toml`, JavaScript bundles, +or non-Fastly adapters. + +--- + +## Task 1: Make the request-filter input capable of private annotations + +**Files:** + +- Modify: `crates/trusted-server-core/src/integrations/registry.rs` +- Test: its existing `#[cfg(test)]` module + +The current `RequestFilterInput` holds `&Request`. Change it to hold +`&mut Request` so a request filter can add a typed extension. This is +the narrowest safe transport because the registry already has exclusive mutable +access to the request while it invokes each filter. + +- [ ] **Step 1: Add a regression test for an extension-producing filter.** Create + a test-only zero-sized marker and filter that writes it to + `input.request.extensions_mut()`. Run `IntegrationRegistry::filter_request` + and assert the original mutable request has the marker afterward. In the + same test, verify normal `RequestFilterEffects` still apply their request + header mutation and return their response header mutation. +- [ ] **Step 2: Change `RequestFilterInput::request` to a mutable borrow.** Keep + the `IntegrationRequestFilter` method signature and `RequestFilterEffects` + unchanged. +- [ ] **Step 3: Update `IntegrationRegistry::filter_request`.** Pass its existing + `&mut Request` directly to each `RequestFilterInput`. Keep the ordering: + filter mutation first, then registry-applied request-header effects, then + the next filter. +- [ ] **Step 4: Update all direct filter tests and test filters.** Calls that build + `RequestFilterInput` must construct a mutable request and pass + `request: &mut request`. Read-only filters should continue to compile by + simply not mutating the request. +- [ ] **Step 5: Run focused tests.** + +```bash +cargo test-fastly integrations::registry +``` + +**Acceptance:** a filter can retain a typed marker for downstream route handling +without emitting a synthetic `x-*` header, and existing header effects retain +their behavior. + +--- + +## Task 2: Mark IP-based DataDome exclusions and log the outcome + +**Files:** + +- Modify: `crates/trusted-server-core/src/integrations/datadome.rs` +- Modify: `crates/trusted-server-core/src/integrations/datadome/protection.rs` +- Test: `crates/trusted-server-core/src/integrations/datadome/protection.rs` +- Reuse: `crates/trusted-server-core/src/integrations/datadome/protection_scope.rs` + +- [ ] **Step 1: Add a crate-private marker in `datadome.rs`.** Define a + zero-sized type with a behavior-oriented name, such as + `DataDomeClientTagSuppressed`. It must be visible to `publisher.rs` and + `protection.rs` through `pub(crate)`, but must not be exported as public + integration configuration or API. +- [ ] **Step 2: Add an IP-reason predicate beside protection logging.** Centralize + the exact four eligible scope reasons in one helper: + +```rust +matches!(reason, "client_ip" | "client_ip_source" | "ip_cidr" | "ip_cidr_source") +``` + + Do not infer eligibility from rule ID: Config Store source rule IDs are + operator-configured strings. + +- [ ] **Step 3: Make `filter_protection_request` own a mutable input and pass it + mutably to `is_request_protected`.** In the existing + `ProtectionScopeDecision::Skip` arm: + + 1. determine whether the reason is IP-based; + 2. if so, insert the typed marker into `input.request.extensions_mut()`; + 3. call the updated skip logger with `client_tag_omitted = true`; and + 4. return `false` exactly as today so the Protection API is not called. + + Do not set the marker for the early method/integration/internal-route + returns. Do not set it when the API call returns a fail-open error. + +- [ ] **Step 4: Update `log_protection_skip`.** Keep IP exclusions at `info` and + non-IP exclusions at `debug`. For the IP branch, extend the existing + structured text after the reason with `client_tag=omitted`; retain rule, + reason, method, host, and path, but do not include the client IP. The + desired shape is: + +```text +[datadome] protection decision=skipped rule=excluded-ip-cidrs reason=client_ip client_tag=omitted method=GET host=example.com path=/page +``` + +- [ ] **Step 5: Add filter-level marker tests.** Add small helpers in the + protection test module to build `RuntimeServices` with a fixed client IP, + optional Config Store data, and a mutable request. For each case, call + `filter_protection_request`, assert it returns `Continue`, and inspect the + request extension: + + - inline `protection_excluded_ip_cidrs` match โ†’ marker present; + - `protection_excluded_ip_cidr_sources` match โ†’ marker present; + - structured `ProtectionMatcherConfig::IpCidr` match โ†’ marker present; + - structured `ProtectionMatcherConfig::IpCidrSource` match โ†’ marker present. + + Clear the process-global CIDR-source test cache before and after source + tests so cached values cannot affect another case. + +- [ ] **Step 6: Add negative filter-level tests.** Assert the marker is absent + for a non-matching IP, a configured ASN match, a structured path match, + a structured query match, an excluded method, and an internal/integration + route. Reuse the existing `ProtectionScope` unit tests for matching + semantics; these new tests verify only the new side effect. +- [ ] **Step 7: Preserve API-call behavior.** For an IP marker test, use an HTTP + client double that records calls or errors if called. Assert no Protection + API request is sent. This protects against accidentally marking a request + while still invoking DataDome. +- [ ] **Step 8: Run focused tests.** + +```bash +cargo test-fastly datadome::protection +cargo test-fastly datadome::protection_scope +``` + +**Acceptance:** only the four IP decision reasons add the private marker and +produce the augmented informational skip log; all other exclusion and fail-open +paths keep their current tag behavior. + +--- + +## Task 3: Thread suppression through publisher response processing + +**Files:** + +- Modify: `crates/trusted-server-core/src/publisher.rs` +- Modify: `crates/trusted-server-core/src/html_processor.rs` +- Modify: `crates/trusted-server-core/src/integrations/registry.rs` +- Test: `publisher.rs` and `html_processor.rs` test modules + +### Data flow to implement + +```text +DataDomeClientTagSuppressed request extension + -> bool captured by handle_publisher_request before origin dispatch + -> OwnedProcessResponseParams + -> ProcessResponseParams / HtmlStreamProcessorParams + -> HtmlProcessorConfig + -> IntegrationHtmlContext + -> DataDomeIntegration::head_inserts +``` + +- [ ] **Step 1: Capture the marker once in `handle_publisher_request`.** Read + `req.extensions().get::().is_some()` before + `req` is rewritten and moved into `PlatformHttpRequest`. Store the boolean + only in the `PublisherResponse::Stream` parameters, because that is the + only response route that passes through HTML injection. +- [ ] **Step 2: Add a boolean to the owned and borrowed publisher-processing + parameter structs.** Add a clearly named field such as + `suppress_datadome_client_side_tag` to: + + - `OwnedProcessResponseParams`; + - `ProcessResponseParams`; and + - `HtmlStreamProcessorParams`. + + Pass it through all three existing HTML construction sites: + + - `PublisherBodyProcessor::new` for async buffered processing; + - `process_response_streaming` for synchronous processing; and + - `stream_publisher_body_async` for the Fastly streaming auction-hold path. + + Every test fixture that constructs `OwnedProcessResponseParams` directly + must set `false` unless it explicitly exercises suppression. + +- [ ] **Step 3: Extend `HtmlProcessorConfig`.** Add the same boolean, default it + to `false` in `from_settings`, and add a narrow builder method used by + `create_html_stream_processor`. Update direct `HtmlProcessorConfig` + fixtures and the benchmark fixture to set `false` explicitly. +- [ ] **Step 4: Extend `IntegrationHtmlContext`.** Add the boolean as immutable + request-scoped context. Populate it at both construction sites in + `html_processor.rs`: + + - the streaming `` element handler; and + - `HtmlWithPostProcessing::process_chunk` for full-document post-processors. + + Update every test helper that constructs `IntegrationHtmlContext` to set + `false` by default. + +- [ ] **Step 5: Add plumbing tests.** + + - `HtmlProcessorConfig::from_settings` defaults to non-suppressed. + - A test head injector records the context flag and sees `true` when a config + is built with suppression. + - A `publisher.rs` route test inserts the DataDome marker into a request, + receives a processable HTML `PublisherResponse::Stream`, and verifies the + owned parameters carry `true`. + - A buffered and a streaming-body path both preserve `true` to head injection. + +- [ ] **Step 6: Run focused tests.** + +```bash +cargo test-fastly html_processor +cargo test-fastly publisher +``` + +**Acceptance:** the decision is read once from a private request extension and +is available to every head injector for every processed HTML response, including +Fastly's streaming path. + +--- + +## Task 4: Omit only Trusted Server's injected DataDome tag + +**Files:** + +- Modify: `crates/trusted-server-core/src/integrations/datadome.rs` +- Test: `crates/trusted-server-core/src/integrations/datadome.rs` +- Test: `crates/trusted-server-core/src/html_processor.rs` or `publisher.rs` + +- [ ] **Step 1: Add a direct head-injector regression test.** With a client-side + key configured and `ctx.suppress_datadome_client_side_tag = true`, assert + `head_inserts()` returns an empty vector. The same config with `false` + must still return exactly one snippet containing both `window.ddjskey` and + the configured tag URL. +- [ ] **Step 2: Implement the guard as the first condition in + `DataDomeIntegration::head_inserts`.** Return an empty vector when the + context flag is true; otherwise retain all current serialization, + escaping, blank-key, and `inject_client_side_tag` behavior unchanged. +- [ ] **Step 3: Add an end-to-end HTML pipeline test.** Configure the DataDome + integration with a client-side key, process representative HTML with + suppression enabled, and assert the result contains neither: + +```text +window.ddjskey= +/integrations/datadome/tags.js +``` + + Repeat with suppression disabled and assert both appear. + +- [ ] **Step 4: Pin publisher-originated-tag behavior.** Feed origin HTML that + contains a DataDome `tags.js` element. With suppression enabled, assert + that element remains in output and is rewritten by `rewrite_sdk` exactly + as before. This distinguishes automatic injection from origin markup. +- [ ] **Step 5: Pin direct route behavior.** Retain or add a DataDome proxy test + showing that `GET /integrations/datadome/tags.js` remains registered and + fetches/proxies the SDK normally; suppression affects only HTML injection. +- [ ] **Step 6: Run focused tests.** + +```bash +cargo test-fastly datadome +``` + +**Acceptance:** suppression removes only the generated configuration/script +pair; nothing removes publisher markup or disables DataDome endpoints. + +--- + +## Task 5: Make tag-suppressed processed HTML private + +**Files:** + +- Modify: `crates/trusted-server-core/src/publisher.rs` +- Test: `crates/trusted-server-core/src/publisher.rs` + +The automatic tag makes the processed HTML vary by client IP. Cache privacy is +therefore a correctness and protection requirement, not an optional +optimization. + +- [ ] **Step 1: Add a failing cache-privacy test.** Build a `PublisherResponse` + with a processable HTML content type, suppression `true`, and cacheable + origin headers (`Cache-Control`, `Surrogate-Control`, and + `Fastly-Surrogate-Control`). Assert the stream response is: + + - `Cache-Control: private, max-age=0`; and + - missing both surrogate cache headers. + +- [ ] **Step 2: Apply privacy only in the `ResponseRoute::Stream` HTML arm.** + After response classification confirms a processable HTML stream, use the + existing per-user ad-stack policy as the model. Do not alter cache headers + for CSS, RSC, non-processable pass-through, unsupported encodings, HEAD, + 204/205/304, or responses without suppression: none has a body variation + created by this feature. +- [ ] **Step 3: Add non-regression cache tests.** Verify that: + + - non-suppressed processed HTML keeps its existing cache headers unless + another existing policy changes them; + - a suppressed CSS/non-HTML stream is not made private by this feature; and + - existing ad-stack privacy behavior remains unchanged when both features are + active. + +- [ ] **Step 4: Run focused tests.** + +```bash +cargo test-fastly publisher +``` + +**Acceptance:** a shared cache cannot replay an IP-excluded client's tagless +HTML to a non-excluded visitor, while unchanged responses retain their existing +cacheability. + +--- + +## Task 6: Add Fastly-path regression coverage + +**Files:** + +- Modify: `crates/trusted-server-adapter-fastly/src/app.rs` tests only if the + existing dispatch helpers can exercise the DataDome registry with a stubbed + publisher response. +- Otherwise, document the existing core filter + publisher pipeline tests as + the executable behavioral coverage; do not refactor Fastly production code + merely to enable a duplicate test. + +- [ ] **Step 1: Extend the existing Fastly request-filter dispatch regression + test or add a focused equivalent.** Configure a DataDome request filter, + insert trusted `ClientInfo` into the request extensions with a matching + IP, and confirm the filter runs before publisher routing. +- [ ] **Step 2: Assert that the routed request retains the private DataDome + marker.** The assertion must inspect request extensions or the processed + HTML result, not an HTTP header. +- [ ] **Step 3: Ensure no actual DataDome API call occurs for the matching IP.** + Use a recording/failing HTTP client or the existing Fastly test seam. +- [ ] **Step 4: Add the non-matching counterpart.** It must not receive the + marker and must continue to inject the configured tag when HTML is + processed. +- [ ] **Step 5: Run Fastly adapter tests.** + +```bash +cargo test-fastly +``` + +**Acceptance:** the production adapter's actual filter ordering preserves the +marker from authoritative Fastly client metadata through publisher HTML +processing. If the current test seam cannot stub a full origin response, retain +this as focused request-filter-order coverage and rely on Task 3's core +pipeline tests for body output rather than expanding adapter production code. + +--- + +## Task 7: Document operator-visible behavior + +**Files:** + +- Modify: `docs/guide/integrations/datadome.md` +- Do not modify: `trusted-server.example.toml` + +- [ ] **Step 1: Add a subsection adjacent to โ€œProtected trafficโ€ or โ€œClient-side + setup.โ€** State that, on Fastly, an IP exclusion skips the Protection API + and suppresses only Trusted Server's automatic DataDome tag injection on + processed HTML. +- [ ] **Step 2: List the four covered IP sources.** Use the exact configuration + names and structured rule types. +- [ ] **Step 3: State the exclusions that do not suppress the client tag.** ASN, + method, path, query, static-asset, and internal-route exclusions retain + normal auto-injection. +- [ ] **Step 4: State the limits.** Publisher-originated/manual tags are not + removed; `/integrations/datadome/tags.js` remains available; no new + configuration is required; and tag-suppressed processed HTML is private + to prevent shared-cache replay. +- [ ] **Step 5: Add the diagnostic example.** Use an example-only host/IP and + include `client_tag=omitted` with rule and reason. +- [ ] **Step 6: Format-check the changed documentation.** + +```bash +cd docs +npx prettier --check guide/integrations/datadome.md \ + superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md \ + superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md +``` + +**Acceptance:** operators can predict exactly when the tag will be omitted and +understand that this is an IP-based Fastly behavior, not a general exclusion +side effect. + +--- + +## Final verification + +- [ ] Confirm the working tree contains only the intended core, Fastly-test, + guide, spec, and plan changes. +- [ ] Run formatting. + +```bash +cargo fmt --all -- --check +``` + +- [ ] Run the relevant target-matched test suites. + +```bash +cargo test-fastly +cargo test-axum +cargo test-cloudflare +``` + +- [ ] Run required lint suites. + +```bash +cargo clippy-fastly +cargo clippy-axum +cargo clippy-cloudflare +``` + +- [ ] Run the docs check from Task 7. +- [ ] Review the diff for accidental exposure of the marker as a request or + response header, duplicate CIDR evaluation, unintended publisher-tag + removal, or shared-cacheable tag-suppressed HTML. + +## Deferred acceptance + +Do **not** perform live production/browser verification in this change. After +deployment, the separate testing workflow should verify that a matching +whitelisted IP receives processed HTML without Trusted Server's +`/integrations/datadome/tags.js` injection, while an unlisted IP retains it. diff --git a/docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md b/docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md new file mode 100644 index 000000000..c48ad2fda --- /dev/null +++ b/docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md @@ -0,0 +1,339 @@ +# DataDome IP-excluded client tag suppression + +**Issue:** #994 +**Date:** 2026-08-03 +**Status:** Proposed + +## Problem + +Trusted Server has two DataDome protection layers: + +1. Server-side Protection API validation, which can be skipped for configured + client IP CIDRs. +2. Client-side tag auto-injection, which adds `window.ddjskey`, + `window.ddoptions`, and the configured `tags.js` script to processed HTML. + +When a request matches an IP-based server-side exclusion, the Protection API is +skipped, but the client-side tag is currently still injected. The browser can +therefore continue running client-side DataDome protection for a request that +was explicitly whitelisted at Trusted Server. + +The desired behavior is that Fastly requests skipped by an IP-based DataDome +exclusion also omit Trusted Server's automatically injected client-side tag. + +## Goals + +- Suppress Trusted Server's automatically injected DataDome client-side tag for + Fastly requests skipped by an IP-based protection exclusion. +- Reuse the existing authoritative protection-scope decision. +- Cover all supported IP-based exclusion mechanisms: + - `protection_excluded_ip_cidrs` + - `protection_excluded_ip_cidr_sources` + - structured `ip_cidr` rules + - structured `ip_cidr_source` rules +- Preserve current behavior for non-IP exclusions. +- Leave publisher-originated or manually configured DataDome tags untouched. +- Add an informational diagnostic indicating that the client tag is omitted. +- Keep the implementation independent of caller-supplied IP headers. +- Prevent a shared cache from replaying IP-specific, tag-suppressed HTML to + non-excluded visitors. + +## Non-goals + +- Do not add a configuration flag or make this behavior opt-in. +- Do not change Axum, Cloudflare, or Spin request-filter wiring. This behavior + is intentionally scoped to the Fastly adapter, where the DataDome server-side + request filter is currently run. +- Do not suppress DataDome tags that originate in publisher HTML. +- Do not remove or disable the `/integrations/datadome/tags.js` route. +- Do not change DataDome signal-collection proxy behavior. +- Do not change ASN, path, query-parameter, method, static-asset, or internal + route exclusions. +- Do not perform live production verification as part of implementation. + +## Confirmed decisions + +1. **Adapter scope:** Fastly only. +2. **IP scope:** all four IP-based exclusion mechanisms listed above. +3. **Tag scope:** Trusted Server's auto-injected tag only. +4. **HTML scope:** every HTML response that enters the existing HTML processing + pipeline. +5. **Logging:** enrich the existing IP-exclusion skip log with + `client_tag=omitted`, including the matched rule and reason. +6. **Live testing:** deferred until after implementation and deployment/testing + workflow review. + +## Current architecture + +### Server-side protection + +`DataDomeIntegration::is_request_protected()` in +`crates/trusted-server-core/src/integrations/datadome/protection.rs` evaluates +method, internal-route, ASN, IP, and structured exclusion conditions. It uses +the client IP from `RuntimeServices::client_info()`, which is populated from +trusted Fastly request metadata. It does not use a caller-supplied IP header. + +The current function reduces the protection-scope result to a boolean. For an +IP exclusion it logs the skip and returns `false`, causing the request filter to +continue without calling the Protection API. + +The Fastly EdgeZero fallback path runs this request filter before route +selection and publisher proxying. The request continues into +`handle_publisher_request()` after the filter returns a continue decision. + +### Client-side injection + +`DataDomeIntegration::head_inserts()` in +`crates/trusted-server-core/src/integrations/datadome.rs` emits the client-side +snippet when: + +- `inject_client_side_tag` is true; and +- `client_side_key` is non-empty. + +The injector currently receives `IntegrationHtmlContext`, which contains HTML +host/scheme and document state but no request IP or protection decision. + +The publisher response path carries request-specific values through: + +```text +Request + -> OwnedProcessResponseParams + -> HtmlStreamProcessorParams + -> HtmlProcessorConfig + -> IntegrationHtmlContext + -> IntegrationHeadInjector +``` + +The existing DataDome attribute rewriter separately rewrites DataDome URLs +found in publisher HTML. That behavior must remain unchanged. + +## Design + +### 1. Capture an IP-exclusion marker at the request filter + +The request filter must attach a typed, internal request-scoped marker when the +existing protection-scope evaluation returns a skip for one of these reasons: + +- `client_ip` +- `client_ip_source` +- `ip_cidr` +- `ip_cidr_source` + +The marker must be attached only after the existing scope decision confirms the +IP exclusion. It must not be inferred from request headers or recomputed later +in the HTML pipeline. + +The request-filter API currently exposes an immutable request view. Add the +smallest internal mechanism needed for a filter to attach a typed request +extension without introducing a caller-visible header. Header mutations should +continue to use `RequestFilterEffects` as they do today. + +The marker should be a zero-sized or otherwise minimal internal type. It only +needs to answer whether Trusted Server's DataDome client tag should be +suppressed; the existing skip log supplies the rule ID and reason. + +The marker must not be attached for: + +- `OPTIONS` or other excluded methods before scope evaluation; +- internal or integration routes; +- ASN exclusions; +- path, query, or other non-IP structured exclusions; +- unmatched IP rules; +- Protection API fail-open behavior; or +- requests where `enable_protection` is false and the request filter does not + run. + +### 2. Enrich the existing skip log + +For IP-based skips, extend the existing informational log with +`client_tag=omitted`: + +```text +[datadome] protection decision=skipped rule=excluded-ip-cidrs reason=client_ip client_tag=omitted method=GET host=example.com path=/page +``` + +The existing rule ID, reason, and request metadata remain part of the log. +Client IP values are not included. Non-IP skip logs retain their current +behavior and level. + +This log represents the request policy decision. It may also apply to a +non-HTML response, for which no HTML tag would have been injected anyway. + +### 3. Propagate the marker into HTML processing + +Before the publisher request is moved into the platform HTTP client, snapshot +whether the request carries the marker. Carry that request-scoped boolean +through `OwnedProcessResponseParams`, `HtmlStreamProcessorParams`, and +`HtmlProcessorConfig`. + +The value should default to `false` in all existing constructors and direct +unit-test fixtures. Non-Fastly adapters will naturally retain the default +because they do not currently produce the Fastly request-filter marker. + +Expose the value to head injectors through the existing HTML processing context +or equivalent request-scoped integration context. The propagation must work for +both: + +- the normal buffered HTML path; and +- the streaming HTML path, including the auction-hold path. + +The value is irrelevant for non-HTML, RSC, pass-through, and unmodified +responses, which should retain their current processing. + +### 4. Keep IP-specific HTML out of shared cache + +A processed HTML response differs by client IP when the generated tag is +suppressed. In the `PublisherResponse::Stream` path, when suppression is active +and the response is HTML, set `Cache-Control: private, max-age=0` and remove +`Surrogate-Control` and `Fastly-Surrogate-Control` before the body is streamed. + +This matches the existing per-user ad-stack cache policy. It prevents Fastly or +another shared cache from replaying a tag-suppressed response to a visitor whose +IP does not match an exclusion. Do not change cache headers for non-HTML, +pass-through, or unmodified responses because their output does not vary by this +feature. + +### 5. Suppress only the generated DataDome snippet + +At the start of `DataDomeIntegration::head_inserts()`: + +1. Check the request-scoped suppression marker. +2. If present, return no DataDome head inserts. +3. Otherwise preserve the current `inject_client_side_tag` and + `client_side_key` checks and emit the existing snippet unchanged. + +When suppression is active, omit both: + +```html + + +``` + +Do not alter: + +- publisher-originated DataDome script tags; +- `rewrite_sdk` behavior; +- the DataDome SDK proxy route; +- the signal collection API proxy; +- DataDome configuration serialization for non-suppressed requests; or +- injection behavior for requests without the marker. + +## Testing plan + +### Protection-filter tests + +Add or extend tests in +`crates/trusted-server-core/src/integrations/datadome/protection.rs` to verify +that the marker is attached for: + +- a matching inline IPv4 CIDR; +- a matching Config Store-backed CIDR source; +- a matching structured `ip_cidr` rule; and +- a matching structured `ip_cidr_source` rule. + +Verify that the marker is absent for: + +- a non-matching IP; +- an ASN exclusion; +- a path exclusion; +- a query-parameter exclusion; +- an excluded method; and +- an internal or integration route. + +Verify the existing protection behavior remains unchanged: IP-matched requests +continue without a Protection API call. + +### Head-injector tests + +Add tests in +`crates/trusted-server-core/src/integrations/datadome.rs` verifying that: + +- a configured client tag is omitted when suppression is active; +- a configured client tag is emitted when suppression is inactive; +- a blank client-side key remains a no-op; and +- `inject_client_side_tag = false` remains a no-op. + +### HTML pipeline tests + +Add coverage for the request-scoped value flowing through the HTML processor, +including the streaming path. Confirm that a suppressed processed HTML response +contains neither the injected `window.ddjskey` configuration nor the configured +DataDome `tags.js` script. For a suppressed HTML stream, assert the response is +private and has no surrogate cache headers. Confirm a non-suppressed HTML stream +retains its origin cache behavior. + +Confirm that publisher-originated DataDome tags remain in the output and are +still rewritten according to the existing `rewrite_sdk` behavior. + +### Fastly dispatch tests + +Add a Fastly adapter dispatch test with: + +- DataDome protection enabled; +- a client IP matching an inline exclusion; +- a configured client-side key; and +- an HTML publisher response. + +The test should verify that the request continues without a Protection API +call, the response includes the `client_tag=omitted` decision log through the +existing test logging seam where available, and the generated tag is absent. + +Also cover a non-excluded request to confirm the generated tag remains present. + +## Documentation changes + +Update `docs/guide/integrations/datadome.md` to state that IP-excluded Fastly +requests skip both: + +- server-side Protection API validation; and +- Trusted Server's automatic client-side tag injection. + +Document that this does not remove or disable publisher-originated DataDome +tags, and that non-IP exclusions do not automatically suppress the client-side +tag. + +No configuration template changes are required because this behavior has no +new setting. + +## Files expected to change + +- `crates/trusted-server-core/src/integrations/registry.rs` + - Support the internal request-scoped annotation mechanism. +- `crates/trusted-server-core/src/integrations/datadome.rs` + - Define the marker and conditionally suppress head injection. +- `crates/trusted-server-core/src/integrations/datadome/protection.rs` + - Attach the marker for IP-based scope skips and enrich the skip log. +- `crates/trusted-server-core/src/integrations/registry.rs` or the relevant + HTML context definition + - Carry the suppression decision into head injection. +- `crates/trusted-server-core/src/html_processor.rs` + - Carry the request-scoped value into HTML integration context. +- `crates/trusted-server-core/src/publisher.rs` + - Snapshot and propagate the request marker through response processing. +- `docs/guide/integrations/datadome.md` + - Document the behavior. +- Relevant unit and Fastly adapter test modules. + +The exact split between registry request annotations and HTML context plumbing +should remain minimal and should not introduce a new public configuration API. + +## Verification + +Implementation verification should use the repository's target-matched +commands: + +```bash +cargo fmt --all -- --check +cargo test-fastly +cargo test-axum +cargo test-cloudflare +cargo clippy-fastly +cargo clippy-axum +cargo clippy-cloudflare +``` + +No live production validation is required for this implementation task. Live +browser verification will be performed later through the deployment/testing +workflow. From fc963341a085da137212e5e2ee83ec07c462a082 Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 5 Aug 2026 19:49:24 -0500 Subject: [PATCH 06/13] Add DataDome staging test bypass --- .../src/integrations/datadome.rs | 128 +++++++++++++- .../src/integrations/datadome/protection.rs | 161 +++++++++++++++++- docs/guide/integrations/datadome.md | 52 ++++-- 3 files changed, 328 insertions(+), 13 deletions(-) diff --git a/crates/trusted-server-core/src/integrations/datadome.rs b/crates/trusted-server-core/src/integrations/datadome.rs index db5932fb2..4dd55b28f 100644 --- a/crates/trusted-server-core/src/integrations/datadome.rs +++ b/crates/trusted-server-core/src/integrations/datadome.rs @@ -61,7 +61,7 @@ use async_trait::async_trait; use edgezero_core::body::Body as EdgeBody; use error_stack::{Report, ResultExt}; use http::header; -use http::{Method, StatusCode}; +use http::{HeaderName, Method, StatusCode}; use regex::Regex; use serde::Deserialize; use serde_json::Value as JsonValue; @@ -77,6 +77,7 @@ use crate::integrations::{ collect_body_bounded, collect_response_bounded, ensure_integration_backend, }; use crate::platform::{PlatformHttpRequest, RuntimeServices}; +use crate::redacted::Redacted; use crate::settings::{IntegrationConfig, Settings}; mod protection; @@ -117,6 +118,27 @@ static DATADOME_URL_PATTERN: LazyLock = LazyLock::new(|| { .expect("DataDome URL rewrite regex should compile") }); +/// Temporary static-header bypass for server-side `DataDome` protection. +/// +/// This is intended only for an access-controlled staging environment. A +/// matching header bypasses the server-side Protection API and is removed +/// before the publisher origin receives the request. +#[derive(Debug, Default, Clone, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ProtectionTestBypassConfig { + /// Enables the bypass. Defaults to disabled when the section is present. + #[serde(default)] + pub enabled: bool, + + /// Header name carrying the temporary bypass credential. + #[serde(default)] + pub header_name: String, + + /// Static credential expected in [`Self::header_name`]. + #[serde(default)] + pub credential: Redacted, +} + /// Configuration for `DataDome` integration. #[derive(Debug, Clone, Deserialize, Validate)] #[serde(deny_unknown_fields)] @@ -199,6 +221,10 @@ pub struct DataDomeConfig { )] pub protection_exclusion_rules: Vec, + /// Temporary static-header bypass for access-controlled staging tests. + #[serde(default)] + pub protection_test_bypass: Option, + /// Reserved flag for future GraphQL payload extraction. #[serde(default)] pub enable_graphql_support: bool, @@ -329,6 +355,7 @@ impl Default for DataDomeConfig { protection_excluded_ip_cidr_sources: Vec::new(), protection_ip_list_cache_ttl_seconds: default_protection_ip_list_cache_ttl_seconds(), protection_exclusion_rules: default_protection_exclusion_rules(), + protection_test_bypass: None, enable_graphql_support: false, client_side_key: String::new(), inject_client_side_tag: default_inject_client_side_tag(), @@ -362,6 +389,9 @@ impl DataDomeIntegration { config.server_side_key_secret_name = config.server_side_key_secret_name.trim().to_string(); config.protection_api_origin = config.protection_api_origin.trim().to_string(); config.client_side_tag_url = config.client_side_tag_url.trim().to_string(); + if let Some(bypass) = &mut config.protection_test_bypass { + bypass.header_name = bypass.header_name.trim().to_string(); + } if config.enable_protection { if config.server_side_key_secret_store.is_empty() @@ -373,6 +403,7 @@ impl DataDomeIntegration { } Self::validate_protection_api_origin(&config.protection_api_origin)?; } + Self::validate_protection_test_bypass(&config)?; if config.inject_client_side_tag { Self::validate_client_side_tag_url(&config.client_side_tag_url)?; @@ -422,6 +453,36 @@ impl DataDomeIntegration { Ok(()) } + fn validate_protection_test_bypass( + config: &DataDomeConfig, + ) -> Result<(), Report> { + let Some(bypass) = config + .protection_test_bypass + .as_ref() + .filter(|bypass| bypass.enabled) + else { + return Ok(()); + }; + + if !config.enable_protection { + return Err(Report::new(Self::error( + "protection_test_bypass requires enable_protection to be true", + ))); + } + if HeaderName::from_bytes(bypass.header_name.as_bytes()).is_err() { + return Err(Report::new(Self::error( + "protection_test_bypass.header_name must be a valid HTTP header name", + ))); + } + if bypass.credential.expose().is_empty() { + return Err(Report::new(Self::error( + "protection_test_bypass.credential must not be empty when enabled", + ))); + } + + Ok(()) + } + fn validate_client_side_tag_url(tag_url: &str) -> Result<(), Report> { if tag_url.starts_with('/') && !tag_url.starts_with("//") { if tag_url.chars().any(is_unsafe_client_side_tag_path_char) { @@ -1098,6 +1159,71 @@ mod tests { config.server_side_key_secret_name, "datadome_server_side_key" ); + assert!( + config.protection_test_bypass.is_none(), + "the temporary test bypass should be disabled by default" + ); + } + + #[test] + fn protection_test_bypass_deserializes_nested_configuration() { + let config: DataDomeConfig = toml::from_str( + r#" + enabled = true + enable_protection = true + + [protection_test_bypass] + enabled = true + header_name = "x-ts-datadome-test-bypass" + credential = "temporary-test-credential" + "#, + ) + .expect("should deserialize DataDome test bypass configuration"); + let bypass = config + .protection_test_bypass + .expect("should deserialize the nested test bypass configuration"); + + assert!(bypass.enabled, "should retain the enabled flag"); + assert_eq!( + bypass.header_name, "x-ts-datadome-test-bypass", + "should retain the configured header name" + ); + assert_eq!( + bypass.credential.expose(), + "temporary-test-credential", + "should retain the configured credential" + ); + } + + #[test] + fn protection_test_bypass_requires_protection_header_and_credential() { + for (enable_protection, header_name, credential, expected_message) in [ + ( + false, + "x-ts-datadome-test-bypass", + "temporary-test-credential", + "requires enable_protection", + ), + (true, "", "temporary-test-credential", "header_name"), + (true, "x-ts-datadome-test-bypass", "", "credential"), + ] { + let mut config = test_config(); + config.enable_protection = enable_protection; + config.protection_test_bypass = Some(ProtectionTestBypassConfig { + enabled: true, + header_name: header_name.to_string(), + credential: Redacted::new(credential.to_string()), + }); + + let err = match DataDomeIntegration::try_new(config) { + Ok(_) => panic!("should reject invalid protection test bypass configuration"), + Err(err) => err, + }; + assert!( + format!("{err:?}").contains(expected_message), + "should explain the invalid protection test bypass configuration" + ); + } } #[test] diff --git a/crates/trusted-server-core/src/integrations/datadome/protection.rs b/crates/trusted-server-core/src/integrations/datadome/protection.rs index 4c74c71e8..e67a83e36 100644 --- a/crates/trusted-server-core/src/integrations/datadome/protection.rs +++ b/crates/trusted-server-core/src/integrations/datadome/protection.rs @@ -4,6 +4,8 @@ use edgezero_core::body::Body as EdgeBody; use edgezero_core::http::{HeaderMap, HeaderName, request_builder}; use error_stack::{Report, ResultExt}; use http::{Method, Request, Response, StatusCode, header}; +use sha2::{Digest as _, Sha256}; +use subtle::ConstantTimeEq as _; use url::Url; use crate::error::TrustedServerError; @@ -45,10 +47,20 @@ impl DataDomeIntegration { ); } + let test_bypass_matched = self.take_protection_test_bypass_header(input.request); if !self.config.enable_protection || !self.is_request_protected(&mut input) { return RequestFilterDecision::Continue(RequestFilterEffects::default()); } + if test_bypass_matched { + input + .request + .extensions_mut() + .insert(super::DataDomeClientTagSuppressed); + log_protection_test_bypass(&input); + return RequestFilterDecision::Continue(RequestFilterEffects::default()); + } + match self.filter_protection_request_inner(input).await { Ok(decision) => decision, Err(ProtectionRequestError::Setup(err)) => { @@ -156,6 +168,26 @@ impl DataDomeIntegration { true } + fn take_protection_test_bypass_header(&self, req: &mut Request) -> bool { + let Some(bypass) = self + .config + .protection_test_bypass + .as_ref() + .filter(|bypass| bypass.enabled) + else { + return false; + }; + let header_name = HeaderName::from_bytes(bypass.header_name.as_bytes()) + .expect("should validate protection test bypass header name during setup"); + let Some(value) = req.headers_mut().remove(&header_name) else { + return false; + }; + + let actual = Sha256::digest(value.as_bytes()); + let expected = Sha256::digest(bypass.credential.expose().as_bytes()); + bool::from(actual.ct_eq(&expected)) + } + fn protection_validate_url(&self) -> String { format!( "{}{}", @@ -442,6 +474,15 @@ fn is_ip_exclusion_reason(reason: &str) -> bool { ) } +fn log_protection_test_bypass(input: &RequestFilterInput<'_>) { + log::info!( + "[datadome] protection decision=skipped rule=protection-test-bypass reason=test_bypass client_tag=omitted method={} host={} path={}", + input.request.method(), + request_host(input.request), + input.request.uri().path(), + ); +} + fn log_protection_skip(input: &RequestFilterInput<'_>, rule_id: &str, reason: &str) { if is_ip_exclusion_reason(reason) { log::info!( @@ -734,12 +775,13 @@ mod tests { use crate::integrations::datadome::{ DataDomeConfig, ProtectionExclusionRuleConfig, ProtectionMatcherConfig, + ProtectionTestBypassConfig, }; use crate::platform::GeoInfo; use crate::platform::test_support::{ - HashMapConfigStore, HashMapSecretStore, NoopConfigStore, NoopSecretStore, + HashMapConfigStore, HashMapSecretStore, NoopConfigStore, NoopSecretStore, StubHttpClient, build_services_with_config_and_secret, build_services_with_config_and_secret_and_client_ip, - noop_services_with_client_ip, + build_services_with_secret_and_http_client, noop_services_with_client_ip, }; use crate::settings::Settings; @@ -801,6 +843,121 @@ mod tests { .is_some() } + #[test] + fn protection_test_bypass_skips_api_suppresses_tag_and_strips_header() { + let config = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_test_bypass: Some(ProtectionTestBypassConfig { + enabled: true, + header_name: "x-ts-datadome-test-bypass".to_string(), + credential: Redacted::new("temporary-test-credential".to_string()), + }), + ..DataDomeConfig::default() + }; + let integration = DataDomeIntegration::try_new(config).expect("should create integration"); + let http_client = Arc::new(StubHttpClient::new()); + let services = + build_services_with_secret_and_http_client(NoopSecretStore, http_client.clone()); + let settings = Settings::default(); + let mut request = request_for_filter(); + request.headers_mut().insert( + "x-ts-datadome-test-bypass", + edgezero_core::http::HeaderValue::from_static("temporary-test-credential"), + ); + + let decision = futures::executor::block_on(integration.filter_protection_request( + RequestFilterInput { + settings: &settings, + services: &services, + request: &mut request, + geo_info: None, + is_integration_route: false, + }, + )); + + assert!( + matches!(decision, RequestFilterDecision::Continue(_)), + "a matching test credential should continue without a challenge" + ); + assert!( + has_client_tag_suppression_marker(&request), + "the bypass should suppress the automatic DataDome client tag" + ); + assert!( + request.headers().get("x-ts-datadome-test-bypass").is_none(), + "the bypass credential must not reach the publisher origin" + ); + assert!( + http_client.recorded_backend_names().is_empty(), + "a matching test credential must not call the Protection API" + ); + } + + #[test] + fn protection_test_bypass_strips_invalid_credential_without_bypassing() { + let config = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_test_bypass: Some(ProtectionTestBypassConfig { + enabled: true, + header_name: "x-ts-datadome-test-bypass".to_string(), + credential: Redacted::new("temporary-test-credential".to_string()), + }), + ..DataDomeConfig::default() + }; + let integration = DataDomeIntegration::try_new(config).expect("should create integration"); + let mut secrets = HashMap::new(); + secrets.insert( + "datadome_server_side_key".to_string(), + b"server-side-key".to_vec(), + ); + let http_client = Arc::new(StubHttpClient::new()); + http_client.push_response_with_headers( + 200, + Vec::new(), + vec![(HEADER_DATADOME_RESPONSE, "200")], + ); + let services = build_services_with_secret_and_http_client( + HashMapSecretStore::new(secrets), + http_client.clone(), + ); + let settings = Settings::default(); + let mut request = request_for_filter(); + request.headers_mut().insert( + "x-ts-datadome-test-bypass", + edgezero_core::http::HeaderValue::from_static("wrong-credential"), + ); + + let decision = futures::executor::block_on(integration.filter_protection_request( + RequestFilterInput { + settings: &settings, + services: &services, + request: &mut request, + geo_info: None, + is_integration_route: false, + }, + )); + + assert!( + matches!(decision, RequestFilterDecision::Continue(_)), + "an allowed Protection API response should continue" + ); + assert!( + !has_client_tag_suppression_marker(&request), + "a non-matching credential must not suppress the DataDome client tag" + ); + assert!( + request.headers().get("x-ts-datadome-test-bypass").is_none(), + "an invalid bypass credential must not reach the publisher origin" + ); + assert_eq!( + http_client.recorded_backend_names().len(), + 1, + "a non-matching credential must still call the Protection API" + ); + } + #[test] fn ip_exclusions_mark_requests_for_client_tag_suppression() { let ip = IpAddr::V4(Ipv4Addr::new(192, 0, 2, 10)); diff --git a/docs/guide/integrations/datadome.md b/docs/guide/integrations/datadome.md index 1743a1b75..ba3081f2a 100644 --- a/docs/guide/integrations/datadome.md +++ b/docs/guide/integrations/datadome.md @@ -86,6 +86,7 @@ patterns = ["(?i)\\.(avi|flv|mka|mkv|mov|mp4|mpeg|mpg|mp3|flac|ogg|ogm|opus|wav| | `protection_excluded_ip_cidr_sources` | array | `[]` | Config Store sources containing dynamic client IP CIDR bypass lists | | `protection_ip_list_cache_ttl_seconds` | integer | `300` | Process-local cache TTL for Config Store-backed IP CIDR bypass lists | | `protection_exclusion_rules` | array | Static asset path regex | Structured method/path/query/IP/ASN exclusion rules | +| `protection_test_bypass` | object | omitted | Temporary static-header bypass for access-controlled staging tests | | `enable_graphql_support` | boolean | `false` | Reserved for future GraphQL body inspection; ignored in v1 | | `client_side_key` | string | `""` | DataDome client-side JavaScript key used for tag injection | | `inject_client_side_tag` | boolean | `true` | Auto-inject the browser tag when `client_side_key` is non-empty | @@ -168,36 +169,67 @@ A request is protected when all of the following are true: 5. The client IP does not match `protection_excluded_ip_cidrs` or any Config Store-backed CIDR source. 6. The client ASN is not listed in `protection_excluded_asns`. 7. No `protection_exclusion_rules` match. +8. The request does not contain a matching enabled `protection_test_bypass` credential. Static assets are excluded by default using a case-insensitive file-extension regex. Trusted Server internal routes such as `/static/tsjs=`, `/integrations/`, `/first-party/`, admin routes, discovery routes, and signature-verification routes are also excluded by default. Auction traffic at `/auction` is protected by default. -### IP-excluded client-side tag behavior +### Staging test bypass + +For short-lived browser automation on an access-controlled staging site, you +can configure a static header credential that skips only the server-side +Protection API: + +```toml +[integrations.datadome.protection_test_bypass] +enabled = true +header_name = "x-ts-datadome-test-bypass" +credential = "temporary-test-credential" +``` + +`protection_test_bypass` requires `enable_protection = true`; it is disabled +when omitted. Treat the credential as a temporary secret: configure it only +while needed, protect the site with an outer access control such as Basic Auth, +and remove the section when testing finishes. Do not enable it in production. + +A matching header is compared in constant time, removed before the request can +reach DataDome or the publisher origin, and never logged. With Playwright, +apply it to the browser context: + +```ts +await context.setExtraHTTPHeaders({ + "X-TS-DataDome-Test-Bypass": process.env.DATADOME_TEST_BYPASS!, +}); +``` + +### Client-side tag suppression behavior On the Fastly adapter, a request that matches an IP-based DataDome exclusion -also omits Trusted Server's automatically injected client-side DataDome tag -from processed HTML. This keeps the client-side layer consistent with the -server-side Protection API skip. +or the configured test-bypass credential also omits Trusted Server's +automatically injected client-side DataDome tag from processed HTML. This keeps +the client-side layer consistent with the server-side Protection API skip. This behavior applies to: - `protection_excluded_ip_cidrs`; - `protection_excluded_ip_cidr_sources`; -- structured `ip_cidr` rules; and -- structured `ip_cidr_source` rules. +- structured `ip_cidr` rules; +- structured `ip_cidr_source` rules; and +- a matching enabled `protection_test_bypass` credential. ASN, method, path, query-parameter, static-asset, and internal-route exclusions do not automatically suppress the client-side tag. DataDome tags already present in publisher HTML are not removed or changed by this behavior, and `/integrations/datadome/tags.js` remains available when requested directly. -Because the processed HTML differs by client IP, tag-suppressed HTML is marked -`private, max-age=0` and removed from shared surrogate caches. The decision is -reported in the existing protection log, for example: +Because the processed HTML differs by client IP or test credential, +tag-suppressed HTML is marked `private, max-age=0` and removed from shared +surrogate caches. The decision is reported in the existing protection log, for +example: ```text -[datadome] protection decision=skipped rule=excluded-ip-cidrs reason=client_ip client_tag=omitted method=GET host=example.com path=/page +[datadome] protection decision=skipped rule=protection-test-bypass reason=test_bypass client_tag=omitted method=GET host=example.com path=/page ``` ### Structured exclusion rules From 1ce7892b145d3643a0e916311291035759d3e7ea Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 6 Aug 2026 10:49:17 -0500 Subject: [PATCH 07/13] Log DataDome test-bypass registration state Log whether protection_test_bypass is enabled when registering the DataDome integration and include configured header name when enabled. Keep credential secret out of logs., --- .../src/integrations/datadome.rs | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/crates/trusted-server-core/src/integrations/datadome.rs b/crates/trusted-server-core/src/integrations/datadome.rs index 4dd55b28f..112d41da1 100644 --- a/crates/trusted-server-core/src/integrations/datadome.rs +++ b/crates/trusted-server-core/src/integrations/datadome.rs @@ -914,12 +914,26 @@ fn build( return Ok(None); }; - log::info!( - "[datadome] Registering integration (sdk_origin: {}, rewrite_sdk: {}, enable_protection: {})", - config.sdk_origin, - config.rewrite_sdk, - config.enable_protection - ); + if let Some(bypass) = config + .protection_test_bypass + .as_ref() + .filter(|bypass| bypass.enabled) + { + log::info!( + "[datadome] Registering integration (sdk_origin: {}, rewrite_sdk: {}, enable_protection: {}, protection_test_bypass: enabled, protection_test_bypass_header: {})", + config.sdk_origin, + config.rewrite_sdk, + config.enable_protection, + bypass.header_name, + ); + } else { + log::info!( + "[datadome] Registering integration (sdk_origin: {}, rewrite_sdk: {}, enable_protection: {}, protection_test_bypass: disabled)", + config.sdk_origin, + config.rewrite_sdk, + config.enable_protection, + ); + } Ok(Some(DataDomeIntegration::try_new(config)?)) } From 7e6f365ad04b329dfa138cac0736c12220ebb8be Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 6 Aug 2026 12:49:49 -0500 Subject: [PATCH 08/13] fix datadome staging bypass privacy --- crates/trusted-server-core/src/config.rs | 42 ++++- .../src/integrations/datadome.rs | 111 +++++------ .../src/integrations/datadome/protection.rs | 173 +++++++++++++----- crates/trusted-server-core/src/publisher.rs | 102 ++++++++++- .../src/response_privacy.rs | 3 +- docs/guide/integrations/datadome.md | 31 ++-- ...6-08-03-datadome-ip-excluded-client-tag.md | 2 +- ...-datadome-ip-excluded-client-tag-design.md | 4 +- 8 files changed, 347 insertions(+), 121 deletions(-) diff --git a/crates/trusted-server-core/src/config.rs b/crates/trusted-server-core/src/config.rs index cdee3b222..e74ef4150 100644 --- a/crates/trusted-server-core/src/config.rs +++ b/crates/trusted-server-core/src/config.rs @@ -155,7 +155,9 @@ fn validate_enabled_integrations( validate_integration::(settings, "sourcepoint")?; validate_integration::(settings, "osano")?; validate_integration::(settings, "google_tag_manager")?; - validate_integration::(settings, "datadome")?; + if let Some(config) = settings.integration_config::("datadome")? { + crate::integrations::datadome::DataDomeIntegration::validate_config_for_startup(config)?; + } validate_integration::(settings, "gpt")?; validate_integration::(settings, "gpt_diagnostics")?; @@ -404,6 +406,44 @@ password = "production-admin-password-32-bytes" ); } + #[test] + fn deploy_validation_rejects_invalid_datadome_test_bypass() { + for (enable_protection, store, name, expected_message) in [ + ( + false, + "ts_secrets", + "datadome_test_bypass", + "requires enable_protection", + ), + (true, "", "datadome_test_bypass", "credential_secret_store"), + (true, "ts_secrets", "", "credential_secret_name"), + ] { + let mut settings = valid_settings(); + settings + .integrations + .insert_config( + "datadome", + &serde_json::json!({ + "enabled": true, + "enable_protection": enable_protection, + "protection_test_bypass": { + "enabled": true, + "credential_secret_store": store, + "credential_secret_name": name, + }, + }), + ) + .expect("should insert DataDome config"); + + let err = validate_settings_for_deploy(&settings) + .expect_err("should reject invalid DataDome test bypass"); + assert!( + format!("{err:?}").contains(expected_message), + "error should mention the invalid bypass setting: {err:?}" + ); + } + } + #[test] fn validate_trait_reports_deploy_errors() { let mut settings = valid_settings(); diff --git a/crates/trusted-server-core/src/integrations/datadome.rs b/crates/trusted-server-core/src/integrations/datadome.rs index 112d41da1..2486c16be 100644 --- a/crates/trusted-server-core/src/integrations/datadome.rs +++ b/crates/trusted-server-core/src/integrations/datadome.rs @@ -61,7 +61,7 @@ use async_trait::async_trait; use edgezero_core::body::Body as EdgeBody; use error_stack::{Report, ResultExt}; use http::header; -use http::{HeaderName, Method, StatusCode}; +use http::{Method, StatusCode}; use regex::Regex; use serde::Deserialize; use serde_json::Value as JsonValue; @@ -77,7 +77,6 @@ use crate::integrations::{ collect_body_bounded, collect_response_bounded, ensure_integration_backend, }; use crate::platform::{PlatformHttpRequest, RuntimeServices}; -use crate::redacted::Redacted; use crate::settings::{IntegrationConfig, Settings}; mod protection; @@ -90,6 +89,7 @@ pub use protection_scope::{ use protection_scope::ProtectionScope; pub(crate) const DATADOME_INTEGRATION_ID: &str = "datadome"; +pub(crate) const HEADER_DATADOME_TEST_BYPASS: &str = "x-ts-datadome-bypass"; /// Request marker indicating that Trusted Server should omit its automatic /// `DataDome` client-side tag for the current response. @@ -121,8 +121,9 @@ static DATADOME_URL_PATTERN: LazyLock = LazyLock::new(|| { /// Temporary static-header bypass for server-side `DataDome` protection. /// /// This is intended only for an access-controlled staging environment. A -/// matching header bypasses the server-side Protection API and is removed -/// before the publisher origin receives the request. +/// matching `x-ts-datadome-bypass` header bypasses the server-side Protection +/// API and is removed before the publisher origin receives the request. The +/// credential itself is loaded from the Secret Store at runtime. #[derive(Debug, Default, Clone, Deserialize)] #[serde(deny_unknown_fields)] pub struct ProtectionTestBypassConfig { @@ -130,13 +131,13 @@ pub struct ProtectionTestBypassConfig { #[serde(default)] pub enabled: bool, - /// Header name carrying the temporary bypass credential. - #[serde(default)] - pub header_name: String, + /// Secret Store containing the temporary bypass credential. + #[serde(default = "default_protection_test_bypass_secret_store")] + pub credential_secret_store: String, - /// Static credential expected in [`Self::header_name`]. - #[serde(default)] - pub credential: Redacted, + /// Secret name containing the temporary bypass credential. + #[serde(default = "default_protection_test_bypass_secret_name")] + pub credential_secret_name: String, } /// Configuration for `DataDome` integration. @@ -278,6 +279,14 @@ fn default_server_side_key_secret_name() -> String { "datadome_server_side_key".to_string() } +fn default_protection_test_bypass_secret_store() -> String { + "ts_secrets".to_string() +} + +fn default_protection_test_bypass_secret_name() -> String { + "datadome_test_bypass".to_string() +} + fn default_timeout_ms() -> u32 { 1500 } @@ -390,7 +399,8 @@ impl DataDomeIntegration { config.protection_api_origin = config.protection_api_origin.trim().to_string(); config.client_side_tag_url = config.client_side_tag_url.trim().to_string(); if let Some(bypass) = &mut config.protection_test_bypass { - bypass.header_name = bypass.header_name.trim().to_string(); + bypass.credential_secret_store = bypass.credential_secret_store.trim().to_string(); + bypass.credential_secret_name = bypass.credential_secret_name.trim().to_string(); } if config.enable_protection { @@ -453,6 +463,12 @@ impl DataDomeIntegration { Ok(()) } + pub(crate) fn validate_config_for_startup( + config: DataDomeConfig, + ) -> Result<(), Report> { + Self::try_new(config).map(|_| ()) + } + fn validate_protection_test_bypass( config: &DataDomeConfig, ) -> Result<(), Report> { @@ -469,14 +485,9 @@ impl DataDomeIntegration { "protection_test_bypass requires enable_protection to be true", ))); } - if HeaderName::from_bytes(bypass.header_name.as_bytes()).is_err() { + if bypass.credential_secret_store.is_empty() || bypass.credential_secret_name.is_empty() { return Err(Report::new(Self::error( - "protection_test_bypass.header_name must be a valid HTTP header name", - ))); - } - if bypass.credential.expose().is_empty() { - return Err(Report::new(Self::error( - "protection_test_bypass.credential must not be empty when enabled", + "protection_test_bypass credential_secret_store and credential_secret_name must not be empty when enabled", ))); } @@ -914,28 +925,25 @@ fn build( return Ok(None); }; - if let Some(bypass) = config + let integration = DataDomeIntegration::try_new(config)?; + let protection_test_bypass = integration + .config .protection_test_bypass .as_ref() - .filter(|bypass| bypass.enabled) - { - log::info!( - "[datadome] Registering integration (sdk_origin: {}, rewrite_sdk: {}, enable_protection: {}, protection_test_bypass: enabled, protection_test_bypass_header: {})", - config.sdk_origin, - config.rewrite_sdk, - config.enable_protection, - bypass.header_name, - ); - } else { - log::info!( - "[datadome] Registering integration (sdk_origin: {}, rewrite_sdk: {}, enable_protection: {}, protection_test_bypass: disabled)", - config.sdk_origin, - config.rewrite_sdk, - config.enable_protection, - ); - } + .is_some_and(|bypass| bypass.enabled); + log::info!( + "[datadome] Registering integration (sdk_origin: {}, rewrite_sdk: {}, enable_protection: {}, protection_test_bypass: {})", + integration.config.sdk_origin, + integration.config.rewrite_sdk, + integration.config.enable_protection, + if protection_test_bypass { + "enabled" + } else { + "disabled" + }, + ); - Ok(Some(DataDomeIntegration::try_new(config)?)) + Ok(Some(integration)) } /// Register the `DataDome` integration with Trusted Server. @@ -1188,8 +1196,8 @@ mod tests { [protection_test_bypass] enabled = true - header_name = "x-ts-datadome-test-bypass" - credential = "temporary-test-credential" + credential_secret_store = "ts_secrets" + credential_secret_name = "datadome_test_bypass" "#, ) .expect("should deserialize DataDome test bypass configuration"); @@ -1199,34 +1207,33 @@ mod tests { assert!(bypass.enabled, "should retain the enabled flag"); assert_eq!( - bypass.header_name, "x-ts-datadome-test-bypass", - "should retain the configured header name" + bypass.credential_secret_store, "ts_secrets", + "should retain the configured credential Secret Store" ); assert_eq!( - bypass.credential.expose(), - "temporary-test-credential", - "should retain the configured credential" + bypass.credential_secret_name, "datadome_test_bypass", + "should retain the configured credential secret name" ); } #[test] - fn protection_test_bypass_requires_protection_header_and_credential() { - for (enable_protection, header_name, credential, expected_message) in [ + fn protection_test_bypass_requires_protection_and_secret_references() { + for (enable_protection, store, name, expected_message) in [ ( false, - "x-ts-datadome-test-bypass", - "temporary-test-credential", + "ts_secrets", + "datadome_test_bypass", "requires enable_protection", ), - (true, "", "temporary-test-credential", "header_name"), - (true, "x-ts-datadome-test-bypass", "", "credential"), + (true, "", "datadome_test_bypass", "credential_secret_store"), + (true, "ts_secrets", "", "credential_secret_name"), ] { let mut config = test_config(); config.enable_protection = enable_protection; config.protection_test_bypass = Some(ProtectionTestBypassConfig { enabled: true, - header_name: header_name.to_string(), - credential: Redacted::new(credential.to_string()), + credential_secret_store: store.to_string(), + credential_secret_name: name.to_string(), }); let err = match DataDomeIntegration::try_new(config) { diff --git a/crates/trusted-server-core/src/integrations/datadome/protection.rs b/crates/trusted-server-core/src/integrations/datadome/protection.rs index e67a83e36..bc06214ff 100644 --- a/crates/trusted-server-core/src/integrations/datadome/protection.rs +++ b/crates/trusted-server-core/src/integrations/datadome/protection.rs @@ -38,20 +38,8 @@ impl DataDomeIntegration { &self, mut input: RequestFilterInput<'_>, ) -> RequestFilterDecision { - if self.config.enable_protection { - log::info!( - "[datadome] protection incoming method={} host={} path={}", - input.request.method(), - request_host(input.request), - input.request.uri().path(), - ); - } - - let test_bypass_matched = self.take_protection_test_bypass_header(input.request); - if !self.config.enable_protection || !self.is_request_protected(&mut input) { - return RequestFilterDecision::Continue(RequestFilterEffects::default()); - } - + let test_bypass_matched = + self.take_protection_test_bypass_header(input.request, input.services); if test_bypass_matched { input .request @@ -61,6 +49,10 @@ impl DataDomeIntegration { return RequestFilterDecision::Continue(RequestFilterEffects::default()); } + if !self.config.enable_protection || !self.is_request_protected(&mut input) { + return RequestFilterDecision::Continue(RequestFilterEffects::default()); + } + match self.filter_protection_request_inner(input).await { Ok(decision) => decision, Err(ProtectionRequestError::Setup(err)) => { @@ -168,7 +160,11 @@ impl DataDomeIntegration { true } - fn take_protection_test_bypass_header(&self, req: &mut Request) -> bool { + fn take_protection_test_bypass_header( + &self, + req: &mut Request, + services: &RuntimeServices, + ) -> bool { let Some(bypass) = self .config .protection_test_bypass @@ -177,14 +173,32 @@ impl DataDomeIntegration { else { return false; }; - let header_name = HeaderName::from_bytes(bypass.header_name.as_bytes()) - .expect("should validate protection test bypass header name during setup"); - let Some(value) = req.headers_mut().remove(&header_name) else { + let Some(value) = req.headers_mut().remove(super::HEADER_DATADOME_TEST_BYPASS) else { return false; }; + let store_name = StoreName::from(bypass.credential_secret_store.as_str()); + let credential = match services + .secret_store() + .get_string(&store_name, &bypass.credential_secret_name) + { + Ok(credential) if !credential.is_empty() => credential, + Ok(_) => { + log::warn!( + "[datadome] DataDome test bypass credential is empty; ignoring bypass header" + ); + return false; + } + Err(err) => { + log::warn!( + "[datadome] Failed to load DataDome test bypass credential; ignoring bypass header: {err:?}" + ); + return false; + } + }; + let actual = Sha256::digest(value.as_bytes()); - let expected = Sha256::digest(bypass.credential.expose().as_bytes()); + let expected = Sha256::digest(credential.as_bytes()); bool::from(actual.ct_eq(&expected)) } @@ -476,31 +490,25 @@ fn is_ip_exclusion_reason(reason: &str) -> bool { fn log_protection_test_bypass(input: &RequestFilterInput<'_>) { log::info!( - "[datadome] protection decision=skipped rule=protection-test-bypass reason=test_bypass client_tag=omitted method={} host={} path={}", + "[datadome] protection decision=skipped rule=protection-test-bypass reason=test_bypass client_tag=omitted method={}", input.request.method(), - request_host(input.request), - input.request.uri().path(), ); } fn log_protection_skip(input: &RequestFilterInput<'_>, rule_id: &str, reason: &str) { if is_ip_exclusion_reason(reason) { log::info!( - "[datadome] protection decision=skipped rule={} reason={} client_tag=omitted method={} host={} path={}", + "[datadome] protection decision=skipped rule={} reason={} client_tag=omitted method={}", rule_id, reason, input.request.method(), - request_host(input.request), - input.request.uri().path(), ); } else { log::debug!( - "[datadome] protection decision=skipped rule={} reason={} method={} host={} path={}", + "[datadome] protection decision=skipped rule={} reason={} method={}", rule_id, reason, input.request.method(), - request_host(input.request), - input.request.uri().path(), ); } } @@ -512,26 +520,20 @@ fn log_protection_result( decision: &RequestFilterDecision, ) { let method = input.request.method(); - let host = request_host(input.request); - let path = input.request.uri().path(); match decision { RequestFilterDecision::Respond { .. } => log::info!( - "[datadome] protection decision=blocked status={} method={} host={} path={} route=short_circuit", + "[datadome] protection decision=blocked status={} method={} route=short_circuit", status.as_u16(), method, - host, - path, ), RequestFilterDecision::Continue(_) if status == StatusCode::OK && datadome_status == Some(status.as_u16()) => { log::info!( - "[datadome] protection decision=allowed status={} method={} host={} path={} route=continue", + "[datadome] protection decision=allowed status={} method={} route=continue", status.as_u16(), method, - host, - path, ); } RequestFilterDecision::Continue(_) => {} @@ -850,19 +852,26 @@ mod tests { enable_protection: true, protection_test_bypass: Some(ProtectionTestBypassConfig { enabled: true, - header_name: "x-ts-datadome-test-bypass".to_string(), - credential: Redacted::new("temporary-test-credential".to_string()), + credential_secret_store: "ts_secrets".to_string(), + credential_secret_name: "datadome_test_bypass".to_string(), }), ..DataDomeConfig::default() }; let integration = DataDomeIntegration::try_new(config).expect("should create integration"); + let mut secrets = HashMap::new(); + secrets.insert( + "datadome_test_bypass".to_string(), + b"temporary-test-credential".to_vec(), + ); let http_client = Arc::new(StubHttpClient::new()); - let services = - build_services_with_secret_and_http_client(NoopSecretStore, http_client.clone()); + let services = build_services_with_secret_and_http_client( + HashMapSecretStore::new(secrets), + http_client.clone(), + ); let settings = Settings::default(); let mut request = request_for_filter(); request.headers_mut().insert( - "x-ts-datadome-test-bypass", + super::super::HEADER_DATADOME_TEST_BYPASS, edgezero_core::http::HeaderValue::from_static("temporary-test-credential"), ); @@ -885,7 +894,10 @@ mod tests { "the bypass should suppress the automatic DataDome client tag" ); assert!( - request.headers().get("x-ts-datadome-test-bypass").is_none(), + request + .headers() + .get(super::super::HEADER_DATADOME_TEST_BYPASS) + .is_none(), "the bypass credential must not reach the publisher origin" ); assert!( @@ -894,6 +906,68 @@ mod tests { ); } + #[test] + fn protection_test_bypass_wins_over_other_exclusions() { + let config = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_exclusion_rules: vec![ProtectionExclusionRuleConfig { + id: "staging-page-exclusion".to_string(), + enabled: true, + methods: Vec::new(), + matcher: ProtectionMatcherConfig::PathExact { + paths: vec!["/page".to_string()], + }, + }], + protection_test_bypass: Some(ProtectionTestBypassConfig { + enabled: true, + credential_secret_store: "ts_secrets".to_string(), + credential_secret_name: "datadome_test_bypass".to_string(), + }), + ..DataDomeConfig::default() + }; + let integration = DataDomeIntegration::try_new(config).expect("should create integration"); + let mut secrets = HashMap::new(); + secrets.insert( + "datadome_test_bypass".to_string(), + b"temporary-test-credential".to_vec(), + ); + let http_client = Arc::new(StubHttpClient::new()); + let services = build_services_with_secret_and_http_client( + HashMapSecretStore::new(secrets), + http_client.clone(), + ); + let settings = Settings::default(); + let mut request = request_for_filter(); + request.headers_mut().insert( + super::super::HEADER_DATADOME_TEST_BYPASS, + edgezero_core::http::HeaderValue::from_static("temporary-test-credential"), + ); + + let decision = futures::executor::block_on(integration.filter_protection_request( + RequestFilterInput { + settings: &settings, + services: &services, + request: &mut request, + geo_info: None, + is_integration_route: false, + }, + )); + + assert!( + matches!(decision, RequestFilterDecision::Continue(_)), + "a matching test credential should continue" + ); + assert!( + has_client_tag_suppression_marker(&request), + "a matching test credential should suppress the tag even on an excluded path" + ); + assert!( + http_client.recorded_backend_names().is_empty(), + "a matching test credential must not call the Protection API" + ); + } + #[test] fn protection_test_bypass_strips_invalid_credential_without_bypassing() { let config = DataDomeConfig { @@ -901,8 +975,8 @@ mod tests { enable_protection: true, protection_test_bypass: Some(ProtectionTestBypassConfig { enabled: true, - header_name: "x-ts-datadome-test-bypass".to_string(), - credential: Redacted::new("temporary-test-credential".to_string()), + credential_secret_store: "ts_secrets".to_string(), + credential_secret_name: "datadome_test_bypass".to_string(), }), ..DataDomeConfig::default() }; @@ -912,6 +986,10 @@ mod tests { "datadome_server_side_key".to_string(), b"server-side-key".to_vec(), ); + secrets.insert( + "datadome_test_bypass".to_string(), + b"temporary-test-credential".to_vec(), + ); let http_client = Arc::new(StubHttpClient::new()); http_client.push_response_with_headers( 200, @@ -925,7 +1003,7 @@ mod tests { let settings = Settings::default(); let mut request = request_for_filter(); request.headers_mut().insert( - "x-ts-datadome-test-bypass", + super::super::HEADER_DATADOME_TEST_BYPASS, edgezero_core::http::HeaderValue::from_static("wrong-credential"), ); @@ -948,7 +1026,10 @@ mod tests { "a non-matching credential must not suppress the DataDome client tag" ); assert!( - request.headers().get("x-ts-datadome-test-bypass").is_none(), + request + .headers() + .get(super::super::HEADER_DATADOME_TEST_BYPASS) + .is_none(), "an invalid bypass credential must not reach the publisher origin" ); assert_eq!( diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index 48e0eaacf..5fc8dac0e 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -1452,12 +1452,21 @@ fn apply_datadome_client_tag_cache_privacy( return; } - response.headers_mut().insert( - header::CACHE_CONTROL, - HeaderValue::from_static("private, max-age=0"), - ); - response.headers_mut().remove("surrogate-control"); - response.headers_mut().remove("fastly-surrogate-control"); + let already_uncacheable = response + .headers() + .get(header::CACHE_CONTROL) + .and_then(|value| value.to_str().ok()) + .map(str::to_ascii_lowercase) + .is_some_and(|value| value.contains("private") || value.contains("no-store")); + if !already_uncacheable { + response.headers_mut().insert( + header::CACHE_CONTROL, + HeaderValue::from_static("private, max-age=0"), + ); + } + for header_name in CDN_CACHE_HEADERS { + response.headers_mut().remove(*header_name); + } } /// Drop a bodiless response's body and correct its framing headers. @@ -2895,6 +2904,10 @@ pub async fn handle_publisher_request( .extensions() .get::() .is_some(); + if suppress_datadome_client_side_tag { + req.headers_mut().remove(header::IF_NONE_MATCH); + req.headers_mut().remove(header::IF_MODIFIED_SINCE); + } let mut platform_request = PlatformHttpRequest::new(req, backend_name); if services.http_client().supports_streaming_responses() { platform_request = platform_request.with_stream_response(); @@ -5203,6 +5216,50 @@ mod tests { ); } + #[tokio::test] + async fn suppressed_publisher_request_removes_conditional_validators() { + let settings = create_test_settings(); + let stub = Arc::new(StubHttpClient::new()); + stub.push_response_with_headers( + 200, + b"origin".to_vec(), + vec![("content-type", "text/html; charset=utf-8")], + ); + let services = build_services_with_http_client( + Arc::clone(&stub) as Arc + ); + let mut req = HttpRequest::builder() + .method(Method::GET) + .uri("https://publisher.example/page") + .header(header::HOST, "publisher.example") + .header(header::IF_NONE_MATCH, "\"cached-page\"") + .header(header::IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT") + .body(EdgeBody::empty()) + .expect("should build conditional request"); + req.extensions_mut() + .insert(crate::integrations::datadome::DataDomeClientTagSuppressed); + + let _response = run_publisher_proxy(&settings, &services, req).await; + + let headers = stub + .recorded_request_headers() + .into_iter() + .next() + .expect("should record one outbound request"); + assert!( + headers + .iter() + .all(|(name, _)| !name.eq_ignore_ascii_case(header::IF_NONE_MATCH.as_str())), + "suppressed requests must not forward If-None-Match" + ); + assert!( + headers + .iter() + .all(|(name, _)| !name.eq_ignore_ascii_case(header::IF_MODIFIED_SINCE.as_str())), + "suppressed requests must not forward If-Modified-Since" + ); + } + #[tokio::test] async fn publisher_origin_fetch_leaves_stream_response_disabled_when_unsupported() { let settings = create_test_settings(); @@ -5365,6 +5422,8 @@ mod tests { .header(header::CACHE_CONTROL, "public, max-age=600") .header("surrogate-control", "max-age=600") .header("fastly-surrogate-control", "max-age=600") + .header("cloudflare-cdn-cache-control", "max-age=600") + .header("cdn-cache-control", "max-age=600") .body(EdgeBody::empty()) .expect("should build cacheable HTML response"); @@ -5391,6 +5450,37 @@ mod tests { response.headers().get("fastly-surrogate-control").is_none(), "suppressed HTML should not retain Fastly-Surrogate-Control" ); + assert!( + response + .headers() + .get("cloudflare-cdn-cache-control") + .is_none(), + "suppressed HTML should not retain Cloudflare-CDN-Cache-Control" + ); + assert!( + response.headers().get("cdn-cache-control").is_none(), + "suppressed HTML should not retain CDN-Cache-Control" + ); + + let mut no_store_response = Response::builder() + .status(StatusCode::OK) + .header(header::CACHE_CONTROL, "no-store") + .body(EdgeBody::empty()) + .expect("should build no-store HTML response"); + super::apply_datadome_client_tag_cache_privacy( + &mut no_store_response, + &Method::GET, + true, + "text/html; charset=utf-8", + ); + assert_eq!( + no_store_response + .headers() + .get(header::CACHE_CONTROL) + .and_then(|value| value.to_str().ok()), + Some("no-store"), + "suppressed HTML should preserve an existing no-store policy" + ); } #[test] diff --git a/crates/trusted-server-core/src/response_privacy.rs b/crates/trusted-server-core/src/response_privacy.rs index e23348211..2205ae892 100644 --- a/crates/trusted-server-core/src/response_privacy.rs +++ b/crates/trusted-server-core/src/response_privacy.rs @@ -22,6 +22,7 @@ pub const CDN_CACHE_HEADERS: &[&str] = &[ "fastly-surrogate-control", "cdn-cache-control", "cloudflare-cdn-cache-control", + "cdn-cache-control", ]; /// Forces cookie-bearing responses to stay private to shared caches. @@ -37,7 +38,7 @@ pub fn enforce_set_cookie_cache_privacy(response: &mut Response) { if !response.headers().contains_key(header::SET_COOKIE) { return; } - // Surrogate cache headers must come off every cookie-bearing response, even + // Shared-cache control headers must come off every cookie-bearing response, even // one already carrying a stricter `no-store`/`private` directive โ€” they are // independent of Cache-Control and would otherwise let a shared cache store // and replay one visitor's Set-Cookie. diff --git a/docs/guide/integrations/datadome.md b/docs/guide/integrations/datadome.md index ba3081f2a..35eda5d3e 100644 --- a/docs/guide/integrations/datadome.md +++ b/docs/guide/integrations/datadome.md @@ -184,23 +184,30 @@ Protection API: ```toml [integrations.datadome.protection_test_bypass] enabled = true -header_name = "x-ts-datadome-test-bypass" -credential = "temporary-test-credential" +credential_secret_store = "ts_secrets" +credential_secret_name = "datadome_test_bypass" ``` `protection_test_bypass` requires `enable_protection = true`; it is disabled -when omitted. Treat the credential as a temporary secret: configure it only -while needed, protect the site with an outer access control such as Basic Auth, -and remove the section when testing finishes. Do not enable it in production. +when omitted. Store the temporary credential in the configured Secret Store, +configure this section only while needed, protect the site with an outer access +control such as Basic Auth, and remove the section when testing finishes. Do not +enable it in production. -A matching header is compared in constant time, removed before the request can -reach DataDome or the publisher origin, and never logged. With Playwright, -apply it to the browser context: +The fixed `x-ts-datadome-bypass` header is compared in constant time, removed +before the request can reach DataDome or the publisher origin, and never +logged. Scope the header to the staging origin; do not attach it to every +request in a browser context because that can disclose the credential to +third-party origins. With Playwright: ```ts -await context.setExtraHTTPHeaders({ - "X-TS-DataDome-Test-Bypass": process.env.DATADOME_TEST_BYPASS!, -}); +await context.route('https://staging.example.com/**', async (route) => { + const headers = { + ...route.request().headers(), + 'x-ts-datadome-bypass': process.env.DATADOME_TEST_BYPASS!, + } + await route.continue({ headers }) +}) ``` ### Client-side tag suppression behavior @@ -229,7 +236,7 @@ surrogate caches. The decision is reported in the existing protection log, for example: ```text -[datadome] protection decision=skipped rule=protection-test-bypass reason=test_bypass client_tag=omitted method=GET host=example.com path=/page +[datadome] protection decision=skipped rule=protection-test-bypass reason=test_bypass client_tag=omitted method=GET ``` ### Structured exclusion rules diff --git a/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md b/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md index 5ea5f6ff3..0dfc923cf 100644 --- a/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md +++ b/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md @@ -152,7 +152,7 @@ matches!(reason, "client_ip" | "client_ip_source" | "ip_cidr" | "ip_cidr_source" desired shape is: ```text -[datadome] protection decision=skipped rule=excluded-ip-cidrs reason=client_ip client_tag=omitted method=GET host=example.com path=/page +[datadome] protection decision=skipped rule=excluded-ip-cidrs reason=client_ip client_tag=omitted method=GET ``` - [ ] **Step 5: Add filter-level marker tests.** Add small helpers in the diff --git a/docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md b/docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md index c48ad2fda..d6d811780 100644 --- a/docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md +++ b/docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md @@ -149,10 +149,10 @@ For IP-based skips, extend the existing informational log with `client_tag=omitted`: ```text -[datadome] protection decision=skipped rule=excluded-ip-cidrs reason=client_ip client_tag=omitted method=GET host=example.com path=/page +[datadome] protection decision=skipped rule=excluded-ip-cidrs reason=client_ip client_tag=omitted method=GET ``` -The existing rule ID, reason, and request metadata remain part of the log. +The existing rule ID, reason, and method remain part of the log. Host and path are omitted to avoid placing dynamic request data in the protection logs. Client IP values are not included. Non-IP skip logs retain their current behavior and level. From 126c17fc73437adc7869d1e512901678eadec223 Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 6 Aug 2026 14:56:44 -0500 Subject: [PATCH 09/13] Improve publisher HTML cache policy when SSAT is inactive --- crates/trusted-server-core/src/publisher.rs | 105 ++++++++++++++------ 1 file changed, 77 insertions(+), 28 deletions(-) diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index d3410b4ed..8919f09b3 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -2932,33 +2932,49 @@ pub async fn handle_publisher_request( // // Gate on `should_run_ad_stack` rather than content-type alone: when no slot // matched, the feature is disabled, or this is not an ad-eligible navigation, - // no per-user `tsjs.adSlots`/`tsjs.bids` are injected, so forcing private - // here would needlessly strip shared cacheability from ordinary publisher - // HTML. Applies regardless of the auction *outcome* (empty bids still inject - // per-user slot state). The separate EC-cookie cache net in the adapter's - // `finalize_response` keeps first-visit identity responses private. + // no per-user `tsjs.adSlots`/`tsjs.bids` are injected. Applies regardless of + // the auction *outcome* (empty bids still inject per-user slot state). The + // separate EC-cookie cache net in the adapter's `finalize_response` keeps + // first-visit identity responses private. let origin_content_type = response .headers() .get(header::CONTENT_TYPE) .and_then(|h| h.to_str().ok()) .unwrap_or_default(); - if should_run_ad_stack && is_html_content_type(origin_content_type) { - response.headers_mut().insert( - header::CACHE_CONTROL, - HeaderValue::from_static("private, no-store"), - ); - response.headers_mut().remove(header::ETAG); - response.headers_mut().remove(header::LAST_MODIFIED); - // Every CDN-targeted cache directive, not just the browser-facing - // `Cache-Control` above: an origin emitting any of these would otherwise - // instruct an intermediary to store a synthesized per-navigation - // document. `Surrogate-Control` and `Fastly-Surrogate-Control` cover - // Fastly; `CDN-Cache-Control` is the standard targeted field (RFC 9213) - // and `Cloudflare-CDN-Cache-Control` is the Cloudflare-specific field - // that overrides it there, so both are needed to close the gap on the - // Cloudflare adapter. - for directive in CDN_CACHE_HEADERS { - response.headers_mut().remove(*directive); + if is_html_content_type(origin_content_type) { + if should_run_ad_stack { + response.headers_mut().insert( + header::CACHE_CONTROL, + HeaderValue::from_static("private, no-store"), + ); + response.headers_mut().remove(header::ETAG); + response.headers_mut().remove(header::LAST_MODIFIED); + // Every CDN-targeted cache directive, not just the browser-facing + // `Cache-Control` above: an origin emitting any of these would otherwise + // instruct an intermediary to store a synthesized per-navigation + // document. `Surrogate-Control` and `Fastly-Surrogate-Control` cover + // Fastly; `CDN-Cache-Control` is the standard targeted field (RFC 9213) + // and `Cloudflare-CDN-Cache-Control` is the Cloudflare-specific field + // that overrides it there, so both are needed to close the gap on the + // Cloudflare adapter. + for directive in CDN_CACHE_HEADERS { + response.headers_mut().remove(*directive); + } + } else { + let origin_cache_control = response + .headers() + .get(header::CACHE_CONTROL) + .and_then(|value| value.to_str().ok()) + .map(str::to_ascii_lowercase); + if !origin_cache_control + .as_deref() + .is_some_and(|value| value.contains("private") || value.contains("no-store")) + { + response.headers_mut().insert( + header::CACHE_CONTROL, + HeaderValue::from_static("max-age=60"), + ); + } } } @@ -4697,13 +4713,16 @@ mod tests { .expect("should build conditional navigation request") } - fn queue_cacheable_html_response(stub: &StubHttpClient) { + fn queue_html_response_with_cache_control( + stub: &StubHttpClient, + cache_control: &'static str, + ) { stub.push_response_with_headers( 200, b"origin".to_vec(), vec![ ("content-type", "text/html; charset=utf-8"), - ("cache-control", "public, max-age=300"), + ("cache-control", cache_control), ("etag", ORIGIN_ETAG), ("last-modified", ORIGIN_LAST_MODIFIED), ("surrogate-control", "max-age=300"), @@ -4773,7 +4792,7 @@ mod tests { // Arrange let settings = settings_with_enabled_auction_and_creative_opportunities(); let stub = Arc::new(StubHttpClient::new()); - queue_cacheable_html_response(&stub); + queue_html_response_with_cache_control(&stub, "public, max-age=300"); let services = build_services_with_http_client( Arc::clone(&stub) as Arc ); @@ -4866,11 +4885,11 @@ mod tests { } #[tokio::test] - async fn navigation_without_matched_slots_preserves_origin_cache_policy() { + async fn navigation_without_matched_slots_uses_short_browser_cache_policy() { // Arrange let settings = settings_with_enabled_auction_and_creative_opportunities(); let stub = Arc::new(StubHttpClient::new()); - queue_cacheable_html_response(&stub); + queue_html_response_with_cache_control(&stub, "public, max-age=300"); let services = build_services_with_http_client( Arc::clone(&stub) as Arc ); @@ -4916,7 +4935,7 @@ mod tests { ); for (header_name, expected) in [ - (header::CACHE_CONTROL, "public, max-age=300"), + (header::CACHE_CONTROL, "max-age=60"), (header::ETAG, ORIGIN_ETAG), (header::LAST_MODIFIED, ORIGIN_LAST_MODIFIED), ( @@ -4947,6 +4966,36 @@ mod tests { } } + #[tokio::test] + async fn navigation_without_matched_slots_preserves_private_origin_cache_policy() { + let settings = settings_with_enabled_auction_and_creative_opportunities(); + + for cache_control in ["private, max-age=0", "No-Store"] { + // Arrange + let stub = Arc::new(StubHttpClient::new()); + queue_html_response_with_cache_control(&stub, cache_control); + let services = build_services_with_http_client( + Arc::clone(&stub) as Arc + ); + + // Act + let response = + run_with_slots(&settings, &services, &[], conditional_navigation_request()) + .await; + let response_head = response_head(response); + + // Assert + assert_eq!( + response_head + .headers + .get(header::CACHE_CONTROL) + .and_then(|value| value.to_str().ok()), + Some(cache_control), + "origin {cache_control} policy should not be weakened" + ); + } + } + #[tokio::test] async fn eligible_navigation_rejects_unexpected_origin_304() { for content_type in [None, Some("text/html; charset=utf-8")] { From 92e2a78062dc1b57194cbd8873a73f8ef7e5bdec Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 6 Aug 2026 17:17:40 -0500 Subject: [PATCH 10/13] Document dedicated server-side ad template switch --- ...-server-side-ad-templates-cache-control.md | 238 ++++++++++++++++++ 1 file changed, 238 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-06-server-side-ad-templates-cache-control.md diff --git a/docs/superpowers/plans/2026-08-06-server-side-ad-templates-cache-control.md b/docs/superpowers/plans/2026-08-06-server-side-ad-templates-cache-control.md new file mode 100644 index 000000000..f96bdcb28 --- /dev/null +++ b/docs/superpowers/plans/2026-08-06-server-side-ad-templates-cache-control.md @@ -0,0 +1,238 @@ +# Dedicated Server-Side Ad Templates Switch and Cache Policy Plan + +> **For agentic workers:** Implement this plan task-by-task, keeping the dedicated +> template switch separate from the global auction configuration. + +**Goal:** Add an explicit on/off switch for server-side ad templates, while +retaining the browser-facing cache policy from issue #1007: + +- Server-side ad templates active: `Cache-Control: private, no-store`. +- Server-side ad templates inactive: `Cache-Control: max-age=60`, unless the + origin already sends `private` or `no-store`. +- CDN-specific cache headers must not change when templates are inactive. + +**Issue context:** The current cache-policy change uses the runtime +`should_run_ad_stack` gate. That gate is also affected by `[auction].enabled`, +which is not the right configuration boundary for publisher templates. A +browser can call `POST /auction`, and that endpoint is a separate server-run +auction API. The new switch must disable publisher HTML/page-bids template +delivery without disabling that API. + +## Configuration decision + +Add this field to the existing `[creative_opportunities]` section: + +```toml +[creative_opportunities] +enabled = true +``` + +Use `enabled = false` to turn off server-side ad templates while retaining the +slot definitions and keeping direct `POST /auction` behavior available. + +### Compatibility rules + +- The field defaults to `true` when omitted, preserving existing behavior for + deployments that already have `[creative_opportunities]` configured. +- The section remains optional. An absent section continues to mean that the + feature is unavailable. +- Serialize the default `true` value as omitted, matching the existing + rollback-compatibility pattern for newer creative-opportunity fields. An + explicit `false` must remain serialized so the setting is not silently lost. +- `auction.enabled` remains a separate auction/orchestrator setting. Do not use + it as the dedicated template switch and do not thread the new template flag + into `POST /auction`. + +## Current cache behavior to retain + +The existing HTML policy block in `publisher.rs` must remain structurally +consistent with the current issue #952 behavior: + +1. For an eligible request that runs the server-side ad stack and receives HTML: + - Set `Cache-Control: private, no-store`. + - Remove `ETag` and `Last-Modified`. + - Remove `Surrogate-Control`, `Fastly-Surrogate-Control`, `CDN-Cache-Control`, + and `Cloudflare-CDN-Cache-Control`. +2. For HTML where the server-side ad stack does not run, including an explicit + template disable: + - Read the browser-facing `Cache-Control` header. + - If its value contains `private` or `no-store`, case-insensitively, preserve + the origin value exactly. + - Otherwise set exactly `Cache-Control: max-age=60`. + - Leave validators and all CDN-specific cache headers untouched. +3. Preserve the later adapter response-privacy finalization for cookie-bearing + responses; this plan does not refactor that behavior. + +## File map + +### Configuration and compatibility + +- `crates/trusted-server-core/src/creative_opportunities.rs` + - Add `CreativeOpportunitiesConfig::enabled` with a default-true serde + implementation and documentation. + - Add a small accessor if it improves readability, but keep the source of + truth in this config type. + - Update config constructors and serialization tests. +- `crates/trusted-server-core/src/settings.rs` + - Keep `creative_opportunities` parsing and runtime preparation compatible with + the new field. + - Make `creative_opportunity_slots()` return an empty slice when the section + is absent or explicitly disabled, so all adapters receive one consistent + runtime view. + - Add TOML and environment-override coverage for `enabled = false`. +- `crates/trusted-server-core/src/config.rs` + - Extend legacy-schema tests to prove default `enabled = true` is omitted from + serialized blobs and remains readable by older binaries. + - Prove an explicit `enabled = false` is serialized, making rollback failure + loud rather than silently re-enabling templates. +- `trusted-server.example.toml` + - Document `creative_opportunities.enabled` and show how to turn templates off + without deleting slot definitions. +- `docs/guide/configuration.md` + - Add the field to the creative-opportunities reference and document the + environment override: + `TRUSTED_SERVER__CREATIVE_OPPORTUNITIES__ENABLED=false`. + - Clarify that this switch controls publisher HTML/page-bids template + delivery, not direct `POST /auction` callers. +- `CHANGELOG.md` + - Add an entry describing the dedicated template switch and cache behavior. + +### Publisher execution and cache policy + +- `crates/trusted-server-core/src/publisher.rs` + - Include the dedicated flag in the initial publisher eligibility decision. + - Do not match, dispatch, or inject server-side ad templates when the flag is + false, even if slots are configured and `[auction].enabled` is true. + - Apply the issue #1007 inactive-HTML cache policy in this state. + - Update skip-reason diagnostics/telemetry so `ad_templates_disabled` is + distinguishable from `auction_disabled`, consent denial, bots, prefetch, and + no matching slots. + - Update `handle_page_bids` so an explicit template disable returns the normal + empty JSON shape (`slots: []`, `bids: {}`) rather than slot definitions. Keep + the current `404` behavior for an absent `[creative_opportunities]` section. + - Extend the existing SSAT cache-policy and eligibility tests. +- `crates/trusted-server-core/src/auction/endpoints.rs` + - Do not gate `POST /auction` on the new template flag. + - Add a regression test or test fixture proving that disabling + `creative_opportunities.enabled` does not suppress a direct auction request + when providers are configured. + - Separately document/verify the existing behavior of `[auction].enabled` for + this endpoint; do not conflate that global setting with the new template + switch. + +### Adapter propagation and browser behavior + +The adapters already pass `Settings::creative_opportunity_slots()` into the +publisher/page-bids handlers. Update and verify these call sites so the central +empty-slice behavior is honored; avoid adding four divergent config checks: + +- `crates/trusted-server-adapter-fastly/src/app.rs` +- `crates/trusted-server-adapter-axum/src/app.rs` +- `crates/trusted-server-adapter-cloudflare/src/app.rs` +- `crates/trusted-server-adapter-spin/src/app.rs` + +No route-level flag is needed if the core `Settings` accessor and handlers are +correct. Add adapter route assertions only where existing fixtures make them +useful. + +The browser runtime already defaults `window.tsjs.adSlots` and +`window.tsjs.bids` to empty values when the edge does not inject templates. If +terminology is updated, adjust these comments/tests without changing runtime +semantics: + +- `crates/trusted-server-js/lib/src/core/index.ts` +- `crates/trusted-server-js/lib/src/integrations/gpt/index.ts` +- Relevant page-bids tests under `crates/trusted-server-js/lib/test/integrations/gpt/` + +## Implementation tasks + +### Task 1: Add and serialize the dedicated setting + +- [ ] Add `enabled: bool` to `CreativeOpportunitiesConfig` with default `true`. +- [ ] Use `skip_serializing_if` so the default value does not appear in stored + config blobs; explicit `false` must serialize. +- [ ] Update all Rust struct literals in `creative_opportunities.rs` and + `publisher.rs` tests. +- [ ] Add parsing, default, false-value, and environment-override tests. +- [ ] Update the legacy compatibility tests in `config.rs`. + +### Task 2: Thread the setting through publisher eligibility + +- [ ] Update `should_run_server_side_ad_stack` to accept the dedicated template + flag as an explicit gate, with a descriptive parameter/doc comment. +- [ ] Ensure initial publisher slot matching and `Settings::creative_opportunity_slots` + do not expose slots when templates are disabled. +- [ ] Preserve the existing `[auction].enabled` and consent gates as separate + conditions. +- [ ] Add an `ad_templates_disabled` diagnostic/telemetry skip reason where the + current branch records a skipped auction. + +### Task 3: Apply the cache policy to the dedicated-off state + +- [ ] Keep the active-SSAT `private, no-store` behavior and validator/CDN header + removal unchanged. +- [ ] Keep the inactive-HTML `max-age=60` behavior from issue #1007. +- [ ] Verify that explicit template disable changes only browser-facing + `Cache-Control` for cacheable HTML; preserve `ETag`, `Last-Modified`, and + every CDN-specific header. +- [ ] Verify that origin `private`, `PRIVATE`, `no-store`, and `No-Store` values + remain unchanged. + +### Task 4: Gate SPA page-bids/template delivery + +- [ ] Include `co_config.enabled` in the `ad_stack_enabled` decision in + `handle_page_bids`. +- [ ] Return empty slots and bids for an explicit disable while retaining the + endpoint and its existing response privacy headers. +- [ ] Keep the absent-section `404` behavior unchanged. +- [ ] Add tests for enabled, disabled, absent, consent-denied, bot, and prefetch + cases as appropriate; preserve existing tests for `[auction].enabled=false`. + +### Task 5: Protect direct `POST /auction` from accidental coupling + +- [ ] Add a focused endpoint test with `creative_opportunities.enabled=false` + and a recording provider. +- [ ] Assert that the provider still sees the direct auction request and that + the response remains a normal OpenRTB response. +- [ ] If the test reveals that `[auction].enabled=false` also needs a separate + product decision for `/auction`, record that as a follow-up rather than + changing it as part of the template-switch work. + +### Task 6: Update docs, examples, comments, and adapter coverage + +- [ ] Update the example config, configuration guide, and changelog. +- [ ] Update stale comments that call `[auction].enabled` the universal template + kill switch. +- [ ] Verify all four adapter call sites use the centralized disabled-slot view. +- [ ] Run JS tests if comments or tests are touched; no JS behavior change is + expected. + +## Test plan + +Use target-matched commands; do not run bare workspace tests because the +workspace contains multiple runtime targets. + +- [ ] `cargo test-axum -p trusted-server-core publisher` +- [ ] `cargo test-fastly` +- [ ] `cargo test-axum` +- [ ] `cargo test-cloudflare` +- [ ] `cargo test-spin` +- [ ] `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` +- [ ] `cd crates/trusted-server-js/lib && npx vitest run` if JS tests/comments change +- [ ] `cd docs && npm run format` if documentation formatting is required + +## Non-goals + +- Do not change CDN-specific cache policy for inactive templates. +- Do not change adapter response privacy or cookie handling. +- Do not use `auction.rewrite_creatives` as the template switch; it controls + creative URL rewriting, not whether the server-side template stack runs. +- Do not gate or disable direct `POST /auction` as part of this feature. +- Do not remove slot definitions when the switch is off; the point of the switch + is to provide a reversible runtime control. From 58054463a16ce801198f877b687b579a33a9d9a3 Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 6 Aug 2026 18:03:25 -0500 Subject: [PATCH 11/13] Add dedicated server-side ad template switch --- CHANGELOG.md | 1 + .../src/auction/endpoints.rs | 127 +++++++++- crates/trusted-server-core/src/config.rs | 27 ++ .../src/creative_opportunities.rs | 47 +++- crates/trusted-server-core/src/publisher.rs | 237 +++++++++++++++--- crates/trusted-server-core/src/settings.rs | 46 +++- .../trusted-server-js/lib/src/core/index.ts | 8 +- .../lib/src/integrations/gpt/index.ts | 12 +- .../test/integrations/gpt/spa_hook.test.ts | 6 +- docs/guide/configuration.md | 20 +- trusted-server.example.toml | 3 + 11 files changed, 480 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36655763c..7c43a6fa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Publisher HTML now uses `Cache-Control: max-age=60` when server-side ad templates are inactive, while preserving origin `private`/`no-store` policies and CDN-specific cache headers. Set `[creative_opportunities].enabled = false` to disable publisher HTML and SPA template delivery without disabling direct `POST /auction` callers. - **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. - **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. - **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. diff --git a/crates/trusted-server-core/src/auction/endpoints.rs b/crates/trusted-server-core/src/auction/endpoints.rs index 36e083948..b80b9339c 100644 --- a/crates/trusted-server-core/src/auction/endpoints.rs +++ b/crates/trusted-server-core/src/auction/endpoints.rs @@ -554,10 +554,14 @@ mod tests { use crate::consent::types::ConsentContext; use crate::openrtb::Uid; use crate::platform::test_support::{ - NoopBackend, NoopConfigStore, NoopGeo, NoopHttpClient, NoopSecretStore, noop_services, + NoopBackend, NoopConfigStore, NoopGeo, NoopHttpClient, NoopSecretStore, StubHttpClient, + noop_services, }; - use crate::platform::{ClientInfo, PlatformPendingRequest, PlatformResponse}; - use crate::test_support::tests::create_test_settings; + use crate::platform::{ + ClientInfo, PlatformHttpClient, PlatformHttpRequest, PlatformPendingRequest, + PlatformResponse, + }; + use crate::test_support::tests::{crate_test_settings_str, create_test_settings}; use base64::Engine as _; use base64::engine::general_purpose::STANDARD as BASE64; use serde_json::json; @@ -642,6 +646,123 @@ mod tests { } } + /// Provider used to prove that direct `/auction` remains available when + /// publisher server-side ad templates are disabled. + struct TemplateSwitchProbeProvider { + calls: Arc>, + } + + #[async_trait::async_trait(?Send)] + impl AuctionProvider for TemplateSwitchProbeProvider { + fn provider_name(&self) -> &'static str { + "template_switch_probe" + } + + async fn request_bids( + &self, + _request: &AuctionRequest, + context: &AuctionContext<'_>, + ) -> Result> { + *self.calls.lock().expect("should lock provider call count") += 1; + let request = Request::builder() + .method("POST") + .uri("https://bidder.example/auction") + .body(EdgeBody::empty()) + .expect("should build probe provider request"); + context + .services + .http_client() + .send_async(PlatformHttpRequest::new( + request, + "template-switch-probe-backend", + )) + .await + .change_context(TrustedServerError::Auction { + message: "probe provider launch failed".to_string(), + }) + } + + async fn parse_response( + &self, + _response: PlatformResponse, + _response_time_ms: u64, + ) -> Result> { + Ok(AuctionResponse::success( + self.provider_name(), + Vec::new(), + 0, + )) + } + + fn timeout_ms(&self) -> u32 { + 100 + } + + fn backend_name(&self, _services: &RuntimeServices, _timeout_ms: u32) -> Option { + Some("template-switch-probe-backend".to_string()) + } + } + + #[tokio::test] + async fn direct_auction_remains_available_when_templates_are_disabled() { + let settings_toml = format!( + "{}\n[auction]\nenabled = true\nproviders = [\"template_switch_probe\"]\n\n[creative_opportunities]\nenabled = false\ngam_network_id = \"12345\"\n", + crate_test_settings_str() + ); + let settings = Settings::from_toml(&settings_toml) + .expect("should parse settings with disabled templates"); + let calls = Arc::new(Mutex::new(0)); + let mut orchestrator = AuctionOrchestrator::new(settings.auction.clone()); + orchestrator.register_provider(Arc::new(TemplateSwitchProbeProvider { + calls: Arc::clone(&calls), + })); + + let stub = Arc::new(StubHttpClient::new()); + stub.push_response(200, b"probe response".to_vec()); + let services = RuntimeServices::builder() + .config_store(Arc::new(NoopConfigStore)) + .secret_store(Arc::new(NoopSecretStore)) + .kv_store(Arc::new(edgezero_core::key_value_store::NoopKvStore)) + .backend(Arc::new(NoopBackend)) + .http_client(Arc::clone(&stub) as Arc) + .geo(Arc::new(NoopGeo)) + .client_info(ClientInfo::default()) + .build(); + let ec_context = make_ec_context(Jurisdiction::NonRegulated, None); + let body = json!({ + "adUnits": [{ + "code": "div-gpt-ad-1", + "mediaTypes": { "banner": { "sizes": [[300, 250]] } } + }] + }); + let req = Request::builder() + .method("POST") + .uri("https://test-publisher.com/auction") + .body(EdgeBody::from( + serde_json::to_vec(&body).expect("should serialize body"), + )) + .expect("should build auction request"); + + let response = handle_auction( + &settings, + &orchestrator, + None, + None, + &ec_context, + &services, + req, + ) + .await + .expect("direct auction should remain available"); + + assert_eq!( + *calls.lock().expect("should lock provider call count"), + 1, + "disabling publisher templates must not disable direct /auction" + ); + assert_eq!(response.status(), StatusCode::OK); + } + #[tokio::test] async fn auction_endpoint_consent_gate_returns_no_bid_without_contacting_providers() { // GDPR/unknown jurisdiction lacking effective TCF Purpose 1 must not run diff --git a/crates/trusted-server-core/src/config.rs b/crates/trusted-server-core/src/config.rs index cdee3b222..033e6c908 100644 --- a/crates/trusted-server-core/src/config.rs +++ b/crates/trusted-server-core/src/config.rs @@ -321,10 +321,37 @@ formats = [{ width = 300, height = 250 }] fn absent_gam_unit_template_is_accepted_by_legacy_schema() { let creative_opportunities = serialized_creative_opportunities(None); + assert!( + creative_opportunities.get("enabled").is_none(), + "default template switch should be omitted for legacy binaries" + ); serde_json::from_value::(creative_opportunities) .expect("should accept absent GAM unit template"); } + #[test] + fn disabled_creative_opportunities_flag_is_visible_to_legacy_schema() { + let mut toml = crate_test_settings_str(); + toml.push_str( + r#" + +[creative_opportunities] +enabled = false +gam_network_id = "99999" +"#, + ); + let app_config: TrustedServerAppConfig = + toml::from_str(&toml).expect("should deserialize app config wrapper"); + let creative_opportunities = serde_json::to_value(app_config) + .expect("should serialize app config wrapper") + .get("creative_opportunities") + .cloned() + .expect("should contain creative opportunities"); + + serde_json::from_value::(creative_opportunities) + .expect_err("legacy binaries should reject an explicit disabled switch"); + } + #[test] fn deploy_validation_rejects_placeholders() { let settings = Settings::from_toml( diff --git a/crates/trusted-server-core/src/creative_opportunities.rs b/crates/trusted-server-core/src/creative_opportunities.rs index 10a85b3e8..a6b70e1d6 100644 --- a/crates/trusted-server-core/src/creative_opportunities.rs +++ b/crates/trusted-server-core/src/creative_opportunities.rs @@ -183,10 +183,27 @@ fn derive_section(path: &str, section_root: &str, section_segment: usize) -> Str } } +const fn default_enabled() -> bool { + true +} + +const fn is_default_enabled(value: &bool) -> bool { + *value == default_enabled() +} + /// Top-level configuration for the creative opportunities system. #[derive(Debug, Clone, Deserialize, Serialize)] #[serde(deny_unknown_fields)] pub struct CreativeOpportunitiesConfig { + /// Enables server-side ad template delivery on publisher HTML and page-bids requests. + /// + /// This does not disable the direct `POST /auction` endpoint. The default is + /// `true` so existing creative-opportunity configurations retain their behavior. + #[serde( + default = "default_enabled", + skip_serializing_if = "is_default_enabled" + )] + pub enabled: bool, /// GAM network ID used to build default unit paths. pub gam_network_id: String, /// Maximum time in milliseconds to wait for the server-side auction before @@ -244,7 +261,7 @@ pub struct CreativeOpportunitiesConfig { /// [`section_root`](Self::section_root) are omitted. #[serde(default, skip_serializing_if = "Option::is_none")] pub section_segment: Option, - /// Slot templates. Empty vec = feature disabled (no auction fired, no globals injected). + /// Slot templates. An empty vec or `enabled = false` disables template delivery. #[serde(default, deserialize_with = "vec_from_seq_or_map")] pub slot: Vec, } @@ -1139,12 +1156,39 @@ mod tests { assert_eq!(derive_section("/%%%/x", "home", 0), "_"); } + #[test] + fn enabled_defaults_true_and_is_omitted_from_serialized_config() { + let config = make_config_with_section_template(None); + assert!( + config.enabled, + "template delivery should default to enabled" + ); + let value = serde_json::to_value(&config).expect("should serialize config"); + assert!( + value.get("enabled").is_none(), + "default enabled value should be omitted for rollback compatibility" + ); + } + + #[test] + fn disabled_template_switch_is_serialized() { + let mut config = make_config_with_section_template(None); + config.enabled = false; + let value = serde_json::to_value(&config).expect("should serialize config"); + assert_eq!( + value.get("enabled"), + Some(&serde_json::Value::Bool(false)), + "explicitly disabled template delivery must remain in config blobs" + ); + } + fn make_config_with_section_template( section_root: Option<&str>, ) -> CreativeOpportunitiesConfig { let mut slot = make_slot("ad-header-0", vec!["/news/*"]); slot.gam_unit_path = Some("/{network_id}/example/{section}".to_string()); CreativeOpportunitiesConfig { + enabled: true, gam_network_id: "99999".to_string(), auction_timeout_ms: None, price_granularity: PriceGranularity::default(), @@ -1542,6 +1586,7 @@ mod tests { // Older binaries deserialize this struct with `deny_unknown_fields`, so // a pushed config blob must not carry `"section_root": null`. let config = CreativeOpportunitiesConfig { + enabled: true, gam_network_id: "99999".to_string(), auction_timeout_ms: None, price_granularity: PriceGranularity::default(), diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index 8919f09b3..87ab8317b 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -1751,27 +1751,34 @@ pub(crate) fn is_prefetch_request(req: &Request) -> bool { header("sec-purpose") || header("purpose") } -/// Returns true only when the publisher request should run the full -/// server-side ad stack: auction dispatch plus initial ad-slot injection. +#[derive(Debug, Clone, Copy)] +struct ServerSideAdStackConfig { + /// Dedicated `[creative_opportunities].enabled` switch. + ad_templates_enabled: bool, + /// Global `[auction].enabled` gate used by publisher/page-bids flows. + auction_enabled: bool, +} + +/// Returns true only when the publisher should inject and run server-side ad templates. /// -/// `auction_enabled` is the global `[auction].enabled` kill switch โ€” when -/// false, no automatic server-side auction or ad-slot injection runs. -pub(crate) fn should_run_server_side_ad_stack( +/// This includes auction dispatch plus initial ad-slot injection. +fn should_run_server_side_ad_stack( is_get: bool, is_navigation: bool, is_prefetch: bool, is_bot: bool, has_matched_slots: bool, consent_allows_auction: bool, - auction_enabled: bool, + config: ServerSideAdStackConfig, ) -> bool { is_get && is_navigation && !is_prefetch && !is_bot + && config.ad_templates_enabled && has_matched_slots && consent_allows_auction - && auction_enabled + && config.auction_enabled } /// Write winning bids from an auction result into the shared `ad_bids_state` lock. @@ -2632,7 +2639,10 @@ pub async fn handle_publisher_request( let is_prefetch = is_prefetch_request(&req); let is_bot = is_bot_user_agent(&req); - let matched_slots = if is_get { + let creative_opportunities = settings.creative_opportunities.as_ref(); + let ad_templates_enabled = creative_opportunities.is_some_and(|co_config| co_config.enabled); + let ad_templates_disabled = creative_opportunities.is_some_and(|co_config| !co_config.enabled); + let matched_slots = if is_get && ad_templates_enabled { settings .creative_opportunities .as_ref() @@ -2655,7 +2665,10 @@ pub async fn handle_publisher_request( is_bot, !matched_slots.is_empty(), consent_allows_auction, - auction.orchestrator.is_enabled(), + ServerSideAdStackConfig { + ad_templates_enabled, + auction_enabled: auction.orchestrator.is_enabled(), + }, ); let should_run_auction = should_run_ad_stack; // Diagnostic: shows which gate suppresses the server-side auction. Pair with @@ -2663,14 +2676,16 @@ pub async fn handle_publisher_request( // when `consent_allows_auction=false`. log::debug!( "server-side ad-stack gate: is_get={is_get} is_navigation={is_navigation} \ - is_prefetch={is_prefetch} is_bot={is_bot} matched_slots={} \ - consent_allows_auction={consent_allows_auction} orchestrator_enabled={} \ - -> should_run_auction={should_run_auction}", + is_prefetch={is_prefetch} is_bot={is_bot} ad_templates_enabled={ad_templates_enabled} \ + matched_slots={} consent_allows_auction={consent_allows_auction} \ + orchestrator_enabled={} -> should_run_auction={should_run_auction}", matched_slots.len(), auction.orchestrator.is_enabled(), ); - if matched_slots.is_empty() && settings.creative_opportunities.is_some() { + if ad_templates_disabled { + log::debug!("Server-side ad templates are disabled by configuration"); + } else if matched_slots.is_empty() && settings.creative_opportunities.is_some() { log::debug!( "No creative opportunity slots matched path '{}' โ€” skipping auction and injection", request_path @@ -2795,7 +2810,9 @@ pub async fn handle_publisher_request( } } } else { - let skip_reason = if !auction.orchestrator.is_enabled() { + let skip_reason = if ad_templates_disabled { + "ad_templates_disabled" + } else if !auction.orchestrator.is_enabled() { "auction_disabled" } else if !consent_allows_auction { "consent_denied" @@ -3771,7 +3788,11 @@ pub async fn handle_page_bids( }) .unwrap_or_else(|| "/".to_string()); - let matched_slots = match_renderable_slots(auction.slots, co_config, &path_param); + let matched_slots = if co_config.enabled { + match_renderable_slots(auction.slots, co_config, &path_param) + } else { + Vec::new() + }; let request_info = crate::http_util::RequestInfo::from_request(&req, services.client_info()); let ec_id = ec_context.ec_value().filter(|_| ec_context.ec_allowed()); @@ -3790,7 +3811,10 @@ pub async fn handle_page_bids( let is_bot = is_bot_user_agent(&req); let auction_enabled = auction.orchestrator.is_enabled(); - if !auction_enabled { + let ad_templates_enabled = co_config.enabled; + if !ad_templates_enabled { + log::debug!("page-bids: [creative_opportunities].enabled is false โ€” skipping templates"); + } else if !auction_enabled { log::debug!("page-bids: [auction].enabled is false โ€” skipping auction"); } else if matched_slots.is_empty() { log::debug!( @@ -3806,14 +3830,14 @@ pub async fn handle_page_bids( ); } - // The [auction].enabled kill switch and a consent denial disable the entire - // server-side ad stack. In those states the endpoint must return no slots, - // so the SPA hook does not assign `ts.adSlots` and call `adInit()` โ€” - // otherwise the kill switch/consent gate would stop SSP calls but still let - // the client create/refresh GPT slots. Bot/prefetch requests, by contrast, - // keep their slot definitions (the placement structure is unchanged) but - // skip the live auction, matching the existing bot/prefetch behaviour. - let ad_stack_enabled = auction_enabled && consent_allows_auction; + // The dedicated template switch, [auction].enabled, and a consent denial + // disable the entire server-side ad stack. In those states the endpoint must + // return no slots, so the SPA hook does not assign `ts.adSlots` and call + // `adInit()` โ€” otherwise the gate would stop SSP calls but still let the + // client create/refresh GPT slots client-side. Bot/prefetch requests, by + // contrast, keep their slot definitions (the placement structure is + // unchanged) but skip the live auction, matching the existing behavior. + let ad_stack_enabled = ad_templates_enabled && auction_enabled && consent_allows_auction; let winning_bids = if matched_slots.is_empty() { std::collections::HashMap::new() @@ -3903,7 +3927,9 @@ pub async fn handle_page_bids( } } } else { - let skip_reason = if !auction_enabled { + let skip_reason = if !ad_templates_enabled { + "ad_templates_disabled" + } else if !auction_enabled { "auction_disabled" } else if !consent_allows_auction { "consent_denied" @@ -4655,6 +4681,15 @@ mod tests { .expect("should parse settings with auction and creative opportunities enabled") } + fn settings_with_disabled_ad_templates() -> Settings { + let toml = format!( + "{}\n[auction]\nenabled = true\n\n\ + [creative_opportunities]\nenabled = false\ngam_network_id = \"12345\"\n", + crate_test_settings_str() + ); + Settings::from_toml(&toml).expect("should parse settings with disabled ad templates") + } + fn settings_with_dispatching_provider() -> Settings { let toml = format!( "{}\n[auction]\nenabled = true\nproviders = [\"{UNEXPECTED_304_PROVIDER}\"]\n\n\ @@ -4966,6 +5001,72 @@ mod tests { } } + #[tokio::test] + async fn disabled_ad_templates_use_short_browser_cache_policy() { + // Arrange + let settings = settings_with_disabled_ad_templates(); + let stub = Arc::new(StubHttpClient::new()); + queue_html_response_with_cache_control(&stub, "public, max-age=300"); + let services = build_services_with_http_client( + Arc::clone(&stub) as Arc + ); + let slots = [article_slot()]; + + // Act + let response = run_with_slots( + &settings, + &services, + &slots, + conditional_navigation_request(), + ) + .await; + let response_head = response_head(response); + + // Assert + assert_eq!( + stub.recorded_cache_bypass_flags(), + vec![false], + "disabled server-side ad templates should not bypass the origin cache" + ); + assert_eq!( + response_head + .headers + .get(header::CACHE_CONTROL) + .and_then(|value| value.to_str().ok()), + Some("max-age=60"), + "disabled server-side ad templates should use the short browser cache policy" + ); + for (header_name, expected) in [ + (header::ETAG, ORIGIN_ETAG), + (header::LAST_MODIFIED, ORIGIN_LAST_MODIFIED), + ( + header::HeaderName::from_static("surrogate-control"), + "max-age=300", + ), + ( + header::HeaderName::from_static("fastly-surrogate-control"), + "max-age=300", + ), + ( + header::HeaderName::from_static("cdn-cache-control"), + "max-age=300", + ), + ( + header::HeaderName::from_static("cloudflare-cdn-cache-control"), + "max-age=300", + ), + ] { + assert_eq!( + response_head + .headers + .get(&header_name) + .and_then(|value| value.to_str().ok()), + Some(expected), + "disabled server-side ad templates should preserve {header_name}" + ); + } + } + #[tokio::test] async fn navigation_without_matched_slots_preserves_private_origin_cache_policy() { let settings = settings_with_enabled_auction_and_creative_opportunities(); @@ -5450,39 +5551,69 @@ mod tests { #[test] fn server_side_ad_stack_runs_only_when_all_auction_gates_pass() { + let enabled_config = ServerSideAdStackConfig { + ad_templates_enabled: true, + auction_enabled: true, + }; assert!( - should_run_server_side_ad_stack(true, true, false, false, true, true, true), - "GET, real navigation, matched slots, and consent should run TS ad stack" + should_run_server_side_ad_stack(true, true, false, false, true, true, enabled_config,), + "GET, real navigation, enabled templates, matched slots, and consent should run TS ad stack" ); assert!( - !should_run_server_side_ad_stack(false, true, false, false, true, true, true), + !should_run_server_side_ad_stack(false, true, false, false, true, true, enabled_config,), "non-GET requests should skip TS ad stack" ); assert!( - !should_run_server_side_ad_stack(true, false, false, false, true, true, true), + !should_run_server_side_ad_stack(true, false, false, false, true, true, enabled_config,), "non-document requests should skip TS ad stack" ); assert!( - !should_run_server_side_ad_stack(true, true, true, false, true, true, true), + !should_run_server_side_ad_stack(true, true, true, false, true, true, enabled_config,), "prefetch requests should skip TS ad stack and injection" ); assert!( - !should_run_server_side_ad_stack(true, true, false, true, true, true, true), + !should_run_server_side_ad_stack(true, true, false, true, true, true, enabled_config,), "bot requests should skip TS ad stack and injection" ); assert!( - !should_run_server_side_ad_stack(true, true, false, false, false, true, true), + !should_run_server_side_ad_stack(true, true, false, false, false, true, enabled_config,), "requests with no matching slots should skip TS ad stack" ); assert!( - !should_run_server_side_ad_stack(true, true, false, false, true, false, true), + !should_run_server_side_ad_stack(true, true, false, false, true, false, enabled_config,), "requests without required consent should skip TS ad stack and injection" ); assert!( - !should_run_server_side_ad_stack(true, true, false, false, true, true, false), + !should_run_server_side_ad_stack( + true, + true, + false, + false, + true, + true, + ServerSideAdStackConfig { + ad_templates_enabled: true, + auction_enabled: false, + }, + ), "disabled [auction].enabled kill switch should skip TS ad stack and injection" ); + assert!( + !should_run_server_side_ad_stack( + true, + true, + false, + false, + true, + true, + ServerSideAdStackConfig { + ad_templates_enabled: false, + auction_enabled: true, + }, + ), + "disabled [creative_opportunities].enabled switch should skip TS ad stack and injection" + ); } #[tokio::test] @@ -8120,6 +8251,7 @@ mod tests { fn make_config() -> CreativeOpportunitiesConfig { CreativeOpportunitiesConfig { + enabled: true, gam_network_id: "21765378893".to_string(), auction_timeout_ms: Some(500), price_granularity: PriceGranularity::Dense, @@ -9337,6 +9469,14 @@ mod tests { Settings::from_toml(&toml).expect("should parse settings with creative_opportunities") } + fn settings_with_co_templates_disabled() -> Settings { + let toml = format!( + "{}\n[auction]\nenabled = true\n\n[creative_opportunities]\nenabled = false\ngam_network_id = \"12345\"\n", + crate_test_settings_str() + ); + Settings::from_toml(&toml).expect("should parse settings with disabled templates") + } + async fn run_page_bids( settings: &Settings, orchestrator: &AuctionOrchestrator, @@ -9886,6 +10026,35 @@ mod tests { ); } + #[tokio::test] + async fn disabled_server_side_ad_templates_return_no_slots_or_bids() { + // The dedicated template switch must suppress publisher/page-bids + // delivery without using the global auction switch. + let settings = settings_with_co_templates_disabled(); + let orchestrator = AuctionOrchestrator::new(settings.auction.clone()); + let slots = article_slot(); + let req = make_page_bids_request("/2024/01/my-article/"); + + let body = run_page_bids_consent_allowed(&settings, &orchestrator, &slots, req).await; + + assert_eq!( + body["slots"] + .as_array() + .expect("slots should be array") + .len(), + 0, + "disabled server-side ad templates must not return slot definitions" + ); + assert_eq!( + body["bids"] + .as_object() + .expect("bids should be object") + .len(), + 0, + "disabled server-side ad templates must not produce bids" + ); + } + #[tokio::test] async fn consent_denied_returns_no_slots_or_bids() { // When consent denies the server-side auction (here: Jurisdiction diff --git a/crates/trusted-server-core/src/settings.rs b/crates/trusted-server-core/src/settings.rs index 1021ee9ec..06152659c 100644 --- a/crates/trusted-server-core/src/settings.rs +++ b/crates/trusted-server-core/src/settings.rs @@ -2095,13 +2095,14 @@ impl Settings { Ok(()) } - /// Returns compiled creative opportunity slots, or empty slice if feature is disabled. + /// Returns compiled creative opportunity slots when template delivery is enabled. #[must_use] pub fn creative_opportunity_slots( &self, ) -> &[crate::creative_opportunities::CreativeOpportunitySlot] { self.creative_opportunities .as_ref() + .filter(|co| co.enabled) .map(|co| co.slot.as_slice()) .unwrap_or(&[]) } @@ -5010,6 +5011,10 @@ formats = [{ width = 300, height = 250 }] let co = settings .creative_opportunities .expect("should have creative_opportunities"); + assert!( + co.enabled, + "creative-opportunity templates should default to enabled" + ); assert_eq!(co.gam_network_id, "21765378893"); assert_eq!(co.auction_timeout_ms, Some(500)); assert_eq!( @@ -5019,6 +5024,45 @@ formats = [{ width = 300, height = 250 }] ); } + #[test] + fn settings_disables_creative_opportunity_slots_when_configured_off() { + let toml = format!( + "{}\n[creative_opportunities]\nenabled = false\ngam_network_id = \"21765378893\"\n\n[[creative_opportunities.slot]]\nid = \"atf\"\npage_patterns = [\"/\"]\nformats = [{{ width = 300, height = 250 }}]\n", + crate_test_settings_str() + ); + let settings = Settings::from_toml(&toml).expect("should parse disabled templates"); + assert!( + settings.creative_opportunity_slots().is_empty(), + "disabled template delivery should expose no runtime slots" + ); + } + + #[test] + fn settings_creative_opportunity_enabled_flag_supports_environment_override() { + let toml = format!( + "{}\n[creative_opportunities]\nenabled = true\ngam_network_id = \"21765378893\"\n", + crate_test_settings_str() + ); + let env_key = format!( + "{}{}CREATIVE_OPPORTUNITIES{}ENABLED", + ENVIRONMENT_VARIABLE_PREFIX, + ENVIRONMENT_VARIABLE_SEPARATOR, + ENVIRONMENT_VARIABLE_SEPARATOR + ); + + temp_env::with_var(env_key, Some("false"), || { + let settings = Settings::from_toml_and_env(&toml) + .expect("should parse template enabled environment override"); + assert!( + !settings + .creative_opportunities + .expect("should have creative opportunities") + .enabled, + "environment override should disable template delivery" + ); + }); + } + #[test] fn settings_rejects_invalid_creative_opportunity_slot_id() { let toml = r#" diff --git a/crates/trusted-server-js/lib/src/core/index.ts b/crates/trusted-server-js/lib/src/core/index.ts index 5d8e41971..b2c4e41e1 100644 --- a/crates/trusted-server-js/lib/src/core/index.ts +++ b/crates/trusted-server-js/lib/src/core/index.ts @@ -36,10 +36,10 @@ api.getConfig = getConfig; // Provide core requestAds API api.requestAds = requestAds; // Defensive defaults: the edge injects adSlots (head-open) and bids (before -// ) only when the server-side ad stack runs for the request. When it -// is gated off (kill switch, consent fail-closed, bots, prefetch), page code -// reading window.tsjs.bids / window.tsjs.adSlots must still see defined -// values instead of throwing. Injected scripts overwrite these wholesale. +// ) only when server-side ad templates run for the request. When template +// delivery is disabled or gated off (auction/consent, bots, prefetch), page code +// reading window.tsjs.bids / window.tsjs.adSlots must still see defined values +// instead of throwing. Injected scripts overwrite these wholesale. api.adSlots ??= []; api.bids ??= {}; // Point global tsjs diff --git a/crates/trusted-server-js/lib/src/integrations/gpt/index.ts b/crates/trusted-server-js/lib/src/integrations/gpt/index.ts index 30cfbbbdd..acea49fa6 100644 --- a/crates/trusted-server-js/lib/src/integrations/gpt/index.ts +++ b/crates/trusted-server-js/lib/src/integrations/gpt/index.ts @@ -689,8 +689,8 @@ export function installTsAdInit(): void { ts.prevSlotTargetingKeys = nextSlotTargetingKeys; // Whether this call produced any TS slot to render. A gated page-bids - // response (auction kill switch or consent denial) returns no slots, so - // the loops above leave these empty. + // response (template switch, auction gate, or consent denial) returns no + // slots, so the loops above leave these empty. const hasRenderableWork = slotsToDisplay.length > 0 || slotsToRefresh.length > 0; // enableSingleRequest and enableServices must only be called once per page @@ -971,10 +971,10 @@ export function installSpaAuctionHook(): void { // This route is now the committed, loaded state โ€” a later failed // navigation rolls back here, and a return trip no-ops correctly. lastAppliedPath = path; - // An empty page-bids response (auction kill switch or consent gate) carries - // no TS slots. Only run adInit() when there are slots to apply or prior TS - // state to sweep โ€” otherwise a consent-denied or kill-switched navigation - // must not enter the GPT command queue and risk activating services. + // An empty page-bids response (template switch, auction, or consent gate) + // carries no TS slots. Only run adInit() when there are slots to apply or + // prior TS state to sweep โ€” otherwise a gated navigation must not enter + // the GPT command queue and risk activating services. const hasPriorTsState = (ts.prevGptSlots?.length ?? 0) > 0 || Object.keys(ts.prevSlotTargetingKeys ?? {}).length > 0 || diff --git a/crates/trusted-server-js/lib/test/integrations/gpt/spa_hook.test.ts b/crates/trusted-server-js/lib/test/integrations/gpt/spa_hook.test.ts index 19739e3dd..36c41aa71 100644 --- a/crates/trusted-server-js/lib/test/integrations/gpt/spa_hook.test.ts +++ b/crates/trusted-server-js/lib/test/integrations/gpt/spa_hook.test.ts @@ -115,9 +115,9 @@ describe('installSpaAuctionHook', () => { }); it('skips adInit on an empty page-bids response with no prior TS state', async () => { - // A gated page-bids response (auction kill switch or consent denial) returns - // no slots. With no prior TS state to sweep, the hook must not call adInit() - // so a consent-denied navigation cannot activate the publisher's GPT setup. + // A gated page-bids response (template switch, auction gate, or consent + // denial) returns no slots. With no prior TS state to sweep, the hook must + // not call adInit() so a gated navigation cannot activate publisher GPT. fetchStub.mockResolvedValue({ ok: true, json: async () => ({ slots: [], bids: {} }), diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index aa2b0264b..ef991eea0 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -1312,8 +1312,16 @@ Defines the ad slots the trusted server offers on a page: which pages each slot appears on (`page_patterns`), its supported sizes (`formats`), and the GAM ad unit it maps to (`gam_unit_path`). +`enabled` is the dedicated server-side ad-template switch. It defaults to `true` +for compatibility with existing configurations. Set it to `false` to stop +publisher HTML and SPA page-bids template delivery while retaining the slot +configuration and direct `POST /auction` endpoint. The browser-facing cache +policy for a disabled template stack is `Cache-Control: max-age=60`, unless the +origin already sends `private` or `no-store`. + ```toml [creative_opportunities] +enabled = true # set to false to disable server-side ad templates gam_network_id = "123456789" price_granularity = "dense" @@ -1332,6 +1340,13 @@ page_patterns = ["/", "/news", "/news/*", "/reviews", "/reviews/*"] formats = [{ width = 728, height = 90 }] ``` +The same switch can be overridden through the legacy environment-variable +loader: + +```bash +TRUSTED_SERVER__CREATIVE_OPPORTUNITIES__ENABLED=false +``` + ### `gam_unit_path` templating `gam_unit_path` is a template. A publisher whose ad unit varies by site section @@ -1385,8 +1400,9 @@ publisher-specific. Startup fails if `{section}` is used without a valid `section_root`. Startup rejects a blank `gam_network_id` only when an absent path/default or a `{network_id}` template consumes it; static paths and templates without `{network_id}` do not consume it. A -`[creative_opportunities]` block with no slots is disabled, so its -`gam_network_id` is not checked. +`[creative_opportunities]` block with `enabled = false` or no slots is +inactive, so no publisher templates are delivered and its `gam_network_id` is +not checked when no slot uses it. Both knobs are config-driven, so the URLโ†’section convention stays with the publisher: `section_segment` selects which segment names the section, and diff --git a/trusted-server.example.toml b/trusted-server.example.toml index e78d2b255..ef5bf5487 100644 --- a/trusted-server.example.toml +++ b/trusted-server.example.toml @@ -155,6 +155,9 @@ ja4_endpoint_enabled = false auction_html_comment = false [creative_opportunities] +# Set to false to disable server-side ad templates while retaining slot definitions +# and direct POST /auction callers. +enabled = true gam_network_id = "123456789" # FCP is not affected by this value โ€” body content above has already # streamed and painted before the hold begins. What this caps is the slip on From d0fffb9d4e886681b92a50e826cfa877f89d1a51 Mon Sep 17 00:00:00 2001 From: Christian Date: Tue, 11 Aug 2026 08:04:10 -0500 Subject: [PATCH 12/13] Address DataDome review feedback --- .../trusted-server-core/src/html_processor.rs | 24 ++- .../src/integrations/datadome.rs | 26 ++- .../src/integrations/datadome/protection.rs | 199 +++++++++++++++--- .../src/integrations/registry.rs | 49 ++++- crates/trusted-server-core/src/publisher.rs | 67 +++--- .../src/response_privacy.rs | 1 - docs/guide/integrations/datadome.md | 24 ++- ...6-08-03-datadome-ip-excluded-client-tag.md | 7 - 8 files changed, 304 insertions(+), 93 deletions(-) diff --git a/crates/trusted-server-core/src/html_processor.rs b/crates/trusted-server-core/src/html_processor.rs index 4c827ace0..3bff588fe 100644 --- a/crates/trusted-server-core/src/html_processor.rs +++ b/crates/trusted-server-core/src/html_processor.rs @@ -966,7 +966,7 @@ mod tests { } #[test] - fn suppressed_datadome_tag_is_not_injected_into_processed_html() { + fn suppressed_datadome_tag_preserves_and_rewrites_publisher_tag() { let mut settings = create_test_settings(); settings .integrations @@ -991,7 +991,10 @@ mod tests { let mut processor = create_html_processor(config); let output = processor - .process_chunk(b"content", true) + .process_chunk( + br#"content"#, + true, + ) .expect("should process HTML"); let html = String::from_utf8(output).expect("should produce UTF-8 HTML"); @@ -1000,8 +1003,21 @@ mod tests { "should omit the DataDome client configuration" ); assert!( - !html.contains("/integrations/datadome/tags.js"), - "should omit the DataDome client tag URL" + html.contains("id=\"publisher-datadome\""), + "should preserve the publisher-originated DataDome tag" + ); + assert!( + html.contains("src=\"/integrations/datadome/tags.js\""), + "should rewrite the publisher-originated DataDome tag" + ); + assert!( + !html.contains("https://js.datadome.co/tags.js"), + "should remove the original third-party DataDome URL" + ); + assert_eq!( + html.matches("/integrations/datadome/tags.js").count(), + 1, + "should leave exactly one publisher-originated DataDome tag" ); } diff --git a/crates/trusted-server-core/src/integrations/datadome.rs b/crates/trusted-server-core/src/integrations/datadome.rs index 2486c16be..75e6c3fdc 100644 --- a/crates/trusted-server-core/src/integrations/datadome.rs +++ b/crates/trusted-server-core/src/integrations/datadome.rs @@ -68,6 +68,7 @@ use serde_json::Value as JsonValue; use url::Url; use validator::Validate; +use crate::constants::ENV_FASTLY_IS_STAGING; use crate::error::TrustedServerError; use crate::integrations::{ AttributeRewriteAction, INTEGRATION_MAX_BODY_BYTES, IntegrationAttributeContext, @@ -469,6 +470,17 @@ impl DataDomeIntegration { Self::try_new(config).map(|_| ()) } + fn active_protection_test_bypass(&self) -> Option<&ProtectionTestBypassConfig> { + if std::env::var(ENV_FASTLY_IS_STAGING).as_deref() != Ok("1") { + return None; + } + + self.config + .protection_test_bypass + .as_ref() + .filter(|bypass| bypass.enabled) + } + fn validate_protection_test_bypass( config: &DataDomeConfig, ) -> Result<(), Report> { @@ -926,18 +938,26 @@ fn build( }; let integration = DataDomeIntegration::try_new(config)?; - let protection_test_bypass = integration + let protection_test_bypass_configured = integration .config .protection_test_bypass .as_ref() .is_some_and(|bypass| bypass.enabled); + let protection_test_bypass_active = integration.active_protection_test_bypass().is_some(); + if protection_test_bypass_configured && !protection_test_bypass_active { + log::warn!( + "[datadome] DataDome test bypass is configured but inactive because FASTLY_IS_STAGING is not 1" + ); + } log::info!( "[datadome] Registering integration (sdk_origin: {}, rewrite_sdk: {}, enable_protection: {}, protection_test_bypass: {})", integration.config.sdk_origin, integration.config.rewrite_sdk, integration.config.enable_protection, - if protection_test_bypass { - "enabled" + if protection_test_bypass_active { + "active" + } else if protection_test_bypass_configured { + "configured-inactive" } else { "disabled" }, diff --git a/crates/trusted-server-core/src/integrations/datadome/protection.rs b/crates/trusted-server-core/src/integrations/datadome/protection.rs index bc06214ff..f2803b38f 100644 --- a/crates/trusted-server-core/src/integrations/datadome/protection.rs +++ b/crates/trusted-server-core/src/integrations/datadome/protection.rs @@ -165,15 +165,11 @@ impl DataDomeIntegration { req: &mut Request, services: &RuntimeServices, ) -> bool { - let Some(bypass) = self - .config - .protection_test_bypass - .as_ref() - .filter(|bypass| bypass.enabled) - else { + let value = req.headers_mut().remove(super::HEADER_DATADOME_TEST_BYPASS); + let Some(bypass) = self.active_protection_test_bypass() else { return false; }; - let Some(value) = req.headers_mut().remove(super::HEADER_DATADOME_TEST_BYPASS) else { + let Some(value) = value else { return false; }; @@ -806,6 +802,23 @@ mod tests { .expect("should build filter request") } + fn filter_with_staging( + integration: &DataDomeIntegration, + settings: &Settings, + services: &RuntimeServices, + request: &mut Request, + ) -> RequestFilterDecision { + temp_env::with_var(crate::constants::ENV_FASTLY_IS_STAGING, Some("1"), || { + futures::executor::block_on(integration.filter_protection_request(RequestFilterInput { + settings, + services, + request, + geo_info: None, + is_integration_route: false, + })) + }) + } + fn filter_marks_request( config: DataDomeConfig, services: &RuntimeServices, @@ -875,15 +888,7 @@ mod tests { edgezero_core::http::HeaderValue::from_static("temporary-test-credential"), ); - let decision = futures::executor::block_on(integration.filter_protection_request( - RequestFilterInput { - settings: &settings, - services: &services, - request: &mut request, - geo_info: None, - is_integration_route: false, - }, - )); + let decision = filter_with_staging(&integration, &settings, &services, &mut request); assert!( matches!(decision, RequestFilterDecision::Continue(_)), @@ -906,6 +911,148 @@ mod tests { ); } + #[test] + fn protection_test_bypass_header_is_stripped_when_unconfigured_or_disabled() { + for protection_test_bypass in [ + None, + Some(ProtectionTestBypassConfig { + enabled: false, + credential_secret_store: "ts_secrets".to_string(), + credential_secret_name: "datadome_test_bypass".to_string(), + }), + ] { + let config = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_test_bypass, + ..DataDomeConfig::default() + }; + let integration = + DataDomeIntegration::try_new(config).expect("should create integration"); + let mut secrets = HashMap::new(); + secrets.insert( + "datadome_server_side_key".to_string(), + b"server-side-key".to_vec(), + ); + let http_client = Arc::new(StubHttpClient::new()); + http_client.push_response_with_headers( + 200, + Vec::new(), + vec![(HEADER_DATADOME_RESPONSE, "200")], + ); + let services = build_services_with_secret_and_http_client( + HashMapSecretStore::new(secrets), + http_client.clone(), + ); + let settings = Settings::default(); + let mut request = request_for_filter(); + request.headers_mut().insert( + super::super::HEADER_DATADOME_TEST_BYPASS, + edgezero_core::http::HeaderValue::from_static("stale-test-credential"), + ); + + let decision = filter_with_staging(&integration, &settings, &services, &mut request); + + assert!( + matches!(decision, RequestFilterDecision::Continue(_)), + "an allowed Protection API response should continue" + ); + assert!( + request + .headers() + .get(super::super::HEADER_DATADOME_TEST_BYPASS) + .is_none(), + "the bypass header must be stripped when the bypass is unconfigured or disabled" + ); + assert!( + !has_client_tag_suppression_marker(&request), + "an inactive bypass must not suppress the DataDome client tag" + ); + assert_eq!( + http_client.recorded_backend_names().len(), + 1, + "an inactive bypass must still call the Protection API" + ); + } + } + + #[test] + fn protection_test_bypass_is_inactive_outside_staging() { + let config = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_test_bypass: Some(ProtectionTestBypassConfig { + enabled: true, + credential_secret_store: "ts_secrets".to_string(), + credential_secret_name: "datadome_test_bypass".to_string(), + }), + ..DataDomeConfig::default() + }; + let integration = DataDomeIntegration::try_new(config).expect("should create integration"); + let mut secrets = HashMap::new(); + secrets.insert( + "datadome_server_side_key".to_string(), + b"server-side-key".to_vec(), + ); + secrets.insert( + "datadome_test_bypass".to_string(), + b"temporary-test-credential".to_vec(), + ); + let http_client = Arc::new(StubHttpClient::new()); + http_client.push_response_with_headers( + 200, + Vec::new(), + vec![(HEADER_DATADOME_RESPONSE, "200")], + ); + let services = build_services_with_secret_and_http_client( + HashMapSecretStore::new(secrets), + http_client.clone(), + ); + let settings = Settings::default(); + let mut request = request_for_filter(); + request.headers_mut().insert( + super::super::HEADER_DATADOME_TEST_BYPASS, + edgezero_core::http::HeaderValue::from_static("temporary-test-credential"), + ); + + let decision = temp_env::with_var( + crate::constants::ENV_FASTLY_IS_STAGING, + None::<&str>, + || { + futures::executor::block_on(integration.filter_protection_request( + RequestFilterInput { + settings: &settings, + services: &services, + request: &mut request, + geo_info: None, + is_integration_route: false, + }, + )) + }, + ); + + assert!( + matches!(decision, RequestFilterDecision::Continue(_)), + "an allowed Protection API response should continue" + ); + assert!( + request + .headers() + .get(super::super::HEADER_DATADOME_TEST_BYPASS) + .is_none(), + "the bypass credential must be stripped outside staging" + ); + assert!( + !has_client_tag_suppression_marker(&request), + "the bypass must not suppress the DataDome client tag outside staging" + ); + assert_eq!( + http_client.recorded_backend_names().len(), + 1, + "the bypass must still call the Protection API outside staging" + ); + } + #[test] fn protection_test_bypass_wins_over_other_exclusions() { let config = DataDomeConfig { @@ -944,15 +1091,7 @@ mod tests { edgezero_core::http::HeaderValue::from_static("temporary-test-credential"), ); - let decision = futures::executor::block_on(integration.filter_protection_request( - RequestFilterInput { - settings: &settings, - services: &services, - request: &mut request, - geo_info: None, - is_integration_route: false, - }, - )); + let decision = filter_with_staging(&integration, &settings, &services, &mut request); assert!( matches!(decision, RequestFilterDecision::Continue(_)), @@ -1007,15 +1146,7 @@ mod tests { edgezero_core::http::HeaderValue::from_static("wrong-credential"), ); - let decision = futures::executor::block_on(integration.filter_protection_request( - RequestFilterInput { - settings: &settings, - services: &services, - request: &mut request, - geo_info: None, - is_integration_route: false, - }, - )); + let decision = filter_with_staging(&integration, &settings, &services, &mut request); assert!( matches!(decision, RequestFilterDecision::Continue(_)), diff --git a/crates/trusted-server-core/src/integrations/registry.rs b/crates/trusted-server-core/src/integrations/registry.rs index 291a54242..16cbac868 100644 --- a/crates/trusted-server-core/src/integrations/registry.rs +++ b/crates/trusted-server-core/src/integrations/registry.rs @@ -1,4 +1,4 @@ -use std::any::Any; +use std::any::{Any, TypeId}; use std::collections::BTreeMap; use std::sync::{Arc, Mutex}; @@ -98,17 +98,19 @@ pub struct IntegrationScriptContext<'a> { pub document_state: &'a IntegrationDocumentState, } +type IntegrationDocumentStateMap = BTreeMap<(&'static str, TypeId), Arc>; + /// Per-document state shared between HTML/script rewriters and post-processors. /// /// This exists to support multi-phase HTML processing without requiring a second HTML parse. #[derive(Clone, Default)] pub struct IntegrationDocumentState { - inner: Arc>>>, + inner: Arc>, } impl std::fmt::Debug for IntegrationDocumentState { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let keys: Vec<&'static str> = { + let keys: Vec<(&'static str, TypeId)> = { let guard = self .inner .lock() @@ -136,7 +138,7 @@ impl IntegrationDocumentState { .inner .lock() .expect("should lock integration document state"); - let value = guard.get(integration_id)?; + let value = guard.get(&(integration_id, TypeId::of::()))?; let cloned: Arc = Arc::clone(value); cloned.downcast::().ok() } @@ -159,17 +161,15 @@ impl IntegrationDocumentState { .lock() .expect("should lock integration document state"); - if let Some(existing) = guard.get(integration_id) + let key = (integration_id, TypeId::of::()); + if let Some(existing) = guard.get(&key) && let Ok(downcast) = Arc::clone(existing).downcast::() { return downcast; } let value: Arc = Arc::new(init()); - guard.insert( - integration_id, - Arc::clone(&value) as Arc, - ); + guard.insert(key, Arc::clone(&value) as Arc); value } @@ -1405,6 +1405,37 @@ mod tests { } } + #[test] + fn document_state_keeps_multiple_types_for_one_integration() { + let state = IntegrationDocumentState::default(); + let number = state.get_or_insert_with("test", || 7_u32); + let label = state.get_or_insert_with("test", || "first".to_string()); + let repeated_number = state.get_or_insert_with("test", || 99_u32); + + assert!( + Arc::ptr_eq(&number, &repeated_number), + "repeated insertion should preserve the original typed state" + ); + assert_eq!( + *state.get::("test").expect("should retrieve number"), + 7, + "should retain numeric state" + ); + assert_eq!( + state + .get::("test") + .expect("should retrieve label") + .as_str(), + "first", + "should retain string state under the same integration ID" + ); + assert_eq!( + label.as_str(), + "first", + "should return inserted string state" + ); + } + #[test] fn default_html_post_processor_should_process_is_false() { let processor = NoopHtmlPostProcessor; diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index 57c4be3db..79ebf4436 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -1423,6 +1423,14 @@ pub async fn publisher_response_into_streaming_response( } } +/// Removes request headers that can produce a bodyless or partial origin response. +fn strip_conditional_and_range_headers(req: &mut Request) { + req.headers_mut().remove(header::IF_NONE_MATCH); + req.headers_mut().remove(header::IF_MODIFIED_SINCE); + req.headers_mut().remove(header::RANGE); + req.headers_mut().remove(header::IF_RANGE); +} + /// Returns `true` when a buffered publisher response should carry a body and a /// recomputed `Content-Length`. /// @@ -1464,6 +1472,8 @@ fn apply_datadome_client_tag_cache_privacy( HeaderValue::from_static("private, max-age=0"), ); } + response.headers_mut().remove(header::ETAG); + response.headers_mut().remove(header::LAST_MODIFIED); for header_name in CDN_CACHE_HEADERS { response.headers_mut().remove(*header_name); } @@ -2881,11 +2891,15 @@ pub async fn handle_publisher_request( } ); - if should_run_ad_stack { - req.headers_mut().remove(header::IF_NONE_MATCH); - req.headers_mut().remove(header::IF_MODIFIED_SINCE); - req.headers_mut().remove(header::RANGE); - req.headers_mut().remove(header::IF_RANGE); + let suppress_datadome_client_side_tag = req + .extensions() + .get::() + .is_some(); + if should_run_ad_stack || suppress_datadome_client_side_tag { + // The origin content type is not known yet, so request hints cannot safely + // narrow this to HTML without allowing 304 or 206 responses to bypass a + // response mutation that becomes necessary after the fetch. + strip_conditional_and_range_headers(&mut req); } // Only advertise encodings the rewrite pipeline can decode and re-encode. @@ -2912,14 +2926,6 @@ pub async fn handle_publisher_request( // without streaming support may reject the flag outright rather than // silently buffering, which would fail every publisher fetch. let request_method = req.method().clone(); - let suppress_datadome_client_side_tag = req - .extensions() - .get::() - .is_some(); - if suppress_datadome_client_side_tag { - req.headers_mut().remove(header::IF_NONE_MATCH); - req.headers_mut().remove(header::IF_MODIFIED_SINCE); - } let mut platform_request = PlatformHttpRequest::new(req, backend_name); if services.http_client().supports_streaming_responses() { platform_request = platform_request.with_stream_response(); @@ -5339,6 +5345,8 @@ mod tests { .header(header::HOST, "publisher.example") .header(header::IF_NONE_MATCH, "\"cached-page\"") .header(header::IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT") + .header(header::RANGE, "bytes=0-18") + .header(header::IF_RANGE, "\"cached-page\"") .body(EdgeBody::empty()) .expect("should build conditional request"); req.extensions_mut() @@ -5351,18 +5359,19 @@ mod tests { .into_iter() .next() .expect("should record one outbound request"); - assert!( - headers - .iter() - .all(|(name, _)| !name.eq_ignore_ascii_case(header::IF_NONE_MATCH.as_str())), - "suppressed requests must not forward If-None-Match" - ); - assert!( - headers - .iter() - .all(|(name, _)| !name.eq_ignore_ascii_case(header::IF_MODIFIED_SINCE.as_str())), - "suppressed requests must not forward If-Modified-Since" - ); + for header_name in [ + header::IF_NONE_MATCH, + header::IF_MODIFIED_SINCE, + header::RANGE, + header::IF_RANGE, + ] { + assert!( + headers + .iter() + .all(|(name, _)| !name.eq_ignore_ascii_case(header_name.as_str())), + "suppressed requests must not forward {header_name}" + ); + } } #[tokio::test] @@ -5529,6 +5538,8 @@ mod tests { .header("fastly-surrogate-control", "max-age=600") .header("cloudflare-cdn-cache-control", "max-age=600") .header("cdn-cache-control", "max-age=600") + .header(header::ETAG, "\"origin-tag\"") + .header(header::LAST_MODIFIED, "Wed, 21 Oct 2015 07:28:00 GMT") .body(EdgeBody::empty()) .expect("should build cacheable HTML response"); @@ -5566,6 +5577,12 @@ mod tests { response.headers().get("cdn-cache-control").is_none(), "suppressed HTML should not retain CDN-Cache-Control" ); + for header_name in [header::ETAG, header::LAST_MODIFIED] { + assert!( + !response.headers().contains_key(&header_name), + "suppressed HTML should not retain {header_name}" + ); + } let mut no_store_response = Response::builder() .status(StatusCode::OK) diff --git a/crates/trusted-server-core/src/response_privacy.rs b/crates/trusted-server-core/src/response_privacy.rs index 2205ae892..c2b1447e3 100644 --- a/crates/trusted-server-core/src/response_privacy.rs +++ b/crates/trusted-server-core/src/response_privacy.rs @@ -22,7 +22,6 @@ pub const CDN_CACHE_HEADERS: &[&str] = &[ "fastly-surrogate-control", "cdn-cache-control", "cloudflare-cdn-cache-control", - "cdn-cache-control", ]; /// Forces cookie-bearing responses to stay private to shared caches. diff --git a/docs/guide/integrations/datadome.md b/docs/guide/integrations/datadome.md index 35eda5d3e..ff2eb6aaa 100644 --- a/docs/guide/integrations/datadome.md +++ b/docs/guide/integrations/datadome.md @@ -169,7 +169,7 @@ A request is protected when all of the following are true: 5. The client IP does not match `protection_excluded_ip_cidrs` or any Config Store-backed CIDR source. 6. The client ASN is not listed in `protection_excluded_asns`. 7. No `protection_exclusion_rules` match. -8. The request does not contain a matching enabled `protection_test_bypass` credential. +8. The request does not contain a matching enabled `protection_test_bypass` credential while `FASTLY_IS_STAGING=1`. Static assets are excluded by default using a case-insensitive file-extension regex. Trusted Server internal routes such as `/static/tsjs=`, `/integrations/`, `/first-party/`, admin routes, discovery routes, and signature-verification routes are also excluded by default. @@ -182,6 +182,7 @@ can configure a static header credential that skips only the server-side Protection API: ```toml +# Runtime activation also requires FASTLY_IS_STAGING=1. [integrations.datadome.protection_test_bypass] enabled = true credential_secret_store = "ts_secrets" @@ -189,14 +190,17 @@ credential_secret_name = "datadome_test_bypass" ``` `protection_test_bypass` requires `enable_protection = true`; it is disabled -when omitted. Store the temporary credential in the configured Secret Store, -configure this section only while needed, protect the site with an outer access -control such as Basic Auth, and remove the section when testing finishes. Do not -enable it in production. - -The fixed `x-ts-datadome-bypass` header is compared in constant time, removed -before the request can reach DataDome or the publisher origin, and never -logged. Scope the header to the staging origin; do not attach it to every +when omitted and is runtime-active only when `FASTLY_IS_STAGING=1`. A retained +section cannot bypass protection in a production or other non-staging runtime. +Store the temporary credential in the configured Secret Store, configure this +section only while needed, protect the site with an outer access control such +as Basic Auth, and remove the section when testing finishes. + +Whenever the enabled DataDome request filter runs, the fixed +`x-ts-datadome-bypass` header is removed before configuration or credential +checks. It therefore cannot reach DataDome or the publisher origin when the +bypass is absent, disabled, inactive, or invalid. Active credentials are +compared in constant time and never logged. Scope the header to the staging origin; do not attach it to every request in a browser context because that can disclose the credential to third-party origins. With Playwright: @@ -223,7 +227,7 @@ This behavior applies to: - `protection_excluded_ip_cidr_sources`; - structured `ip_cidr` rules; - structured `ip_cidr_source` rules; and -- a matching enabled `protection_test_bypass` credential. +- a matching enabled `protection_test_bypass` credential in a staging runtime. ASN, method, path, query-parameter, static-asset, and internal-route exclusions do not automatically suppress the client-side tag. DataDome tags diff --git a/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md b/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md index 0dfc923cf..2d9b0eae2 100644 --- a/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md +++ b/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md @@ -136,7 +136,6 @@ matches!(reason, "client_ip" | "client_ip_source" | "ip_cidr" | "ip_cidr_source" - [ ] **Step 3: Make `filter_protection_request` own a mutable input and pass it mutably to `is_request_protected`.** In the existing `ProtectionScopeDecision::Skip` arm: - 1. determine whether the reason is IP-based; 2. if so, insert the typed marker into `input.request.extensions_mut()`; 3. call the updated skip logger with `client_tag_omitted = true`; and @@ -160,7 +159,6 @@ matches!(reason, "client_ip" | "client_ip_source" | "ip_cidr" | "ip_cidr_source" optional Config Store data, and a mutable request. For each case, call `filter_protection_request`, assert it returns `Continue`, and inspect the request extension: - - inline `protection_excluded_ip_cidrs` match โ†’ marker present; - `protection_excluded_ip_cidr_sources` match โ†’ marker present; - structured `ProtectionMatcherConfig::IpCidr` match โ†’ marker present; @@ -220,7 +218,6 @@ DataDomeClientTagSuppressed request extension - [ ] **Step 2: Add a boolean to the owned and borrowed publisher-processing parameter structs.** Add a clearly named field such as `suppress_datadome_client_side_tag` to: - - `OwnedProcessResponseParams`; - `ProcessResponseParams`; and - `HtmlStreamProcessorParams`. @@ -241,7 +238,6 @@ DataDomeClientTagSuppressed request extension - [ ] **Step 4: Extend `IntegrationHtmlContext`.** Add the boolean as immutable request-scoped context. Populate it at both construction sites in `html_processor.rs`: - - the streaming `` element handler; and - `HtmlWithPostProcessing::process_chunk` for full-document post-processors. @@ -249,7 +245,6 @@ DataDomeClientTagSuppressed request extension `false` by default. - [ ] **Step 5: Add plumbing tests.** - - `HtmlProcessorConfig::from_settings` defaults to non-suppressed. - A test head injector records the context flag and sees `true` when a config is built with suppression. @@ -332,7 +327,6 @@ optimization. with a processable HTML content type, suppression `true`, and cacheable origin headers (`Cache-Control`, `Surrogate-Control`, and `Fastly-Surrogate-Control`). Assert the stream response is: - - `Cache-Control: private, max-age=0`; and - missing both surrogate cache headers. @@ -343,7 +337,6 @@ optimization. 204/205/304, or responses without suppression: none has a body variation created by this feature. - [ ] **Step 3: Add non-regression cache tests.** Verify that: - - non-suppressed processed HTML keeps its existing cache headers unless another existing policy changes them; - a suppressed CSS/non-HTML stream is not made private by this feature; and From 73b700bf50afae07bfe89dbe0cd539550c18c4fb Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 12 Aug 2026 11:07:03 -0500 Subject: [PATCH 13/13] Harden DataDome exclusion and bypass handling --- .../src/integrations/datadome.rs | 9 +- .../src/integrations/datadome/protection.rs | 419 +++++++++++++++--- .../integrations/datadome/protection_scope.rs | 260 +++++++++-- .../src/platform/test_support.rs | 10 +- crates/trusted-server-core/src/publisher.rs | 256 ++++++++--- .../src/response_privacy.rs | 56 ++- docs/guide/integrations/datadome.md | 56 ++- ...6-08-03-datadome-ip-excluded-client-tag.md | 22 +- ...-datadome-ip-excluded-client-tag-design.md | 32 +- 9 files changed, 917 insertions(+), 203 deletions(-) diff --git a/crates/trusted-server-core/src/integrations/datadome.rs b/crates/trusted-server-core/src/integrations/datadome.rs index 75e6c3fdc..d95ee35ee 100644 --- a/crates/trusted-server-core/src/integrations/datadome.rs +++ b/crates/trusted-server-core/src/integrations/datadome.rs @@ -90,6 +90,7 @@ pub use protection_scope::{ use protection_scope::ProtectionScope; pub(crate) const DATADOME_INTEGRATION_ID: &str = "datadome"; +/// Fixed request header used by the staging-only protection test bypass. pub(crate) const HEADER_DATADOME_TEST_BYPASS: &str = "x-ts-datadome-bypass"; /// Request marker indicating that Trusted Server should omit its automatic @@ -136,7 +137,7 @@ pub struct ProtectionTestBypassConfig { #[serde(default = "default_protection_test_bypass_secret_store")] pub credential_secret_store: String, - /// Secret name containing the temporary bypass credential. + /// Secret name containing at least 32 bytes of high-entropy bypass material. #[serde(default = "default_protection_test_bypass_secret_name")] pub credential_secret_name: String, } @@ -464,6 +465,12 @@ impl DataDomeIntegration { Ok(()) } + /// Validates `DataDome` configuration before runtime registration. + /// + /// # Errors + /// + /// Returns an error when protection, bypass, or client-tag configuration is + /// invalid. pub(crate) fn validate_config_for_startup( config: DataDomeConfig, ) -> Result<(), Report> { diff --git a/crates/trusted-server-core/src/integrations/datadome/protection.rs b/crates/trusted-server-core/src/integrations/datadome/protection.rs index f2803b38f..665060014 100644 --- a/crates/trusted-server-core/src/integrations/datadome/protection.rs +++ b/crates/trusted-server-core/src/integrations/datadome/protection.rs @@ -9,6 +9,7 @@ use subtle::ConstantTimeEq as _; use url::Url; use crate::error::TrustedServerError; +use crate::http_util::is_navigation_request; use crate::integrations::{ HeaderMutation, RequestFilterDecision, RequestFilterEffects, RequestFilterInput, }; @@ -16,7 +17,11 @@ use crate::platform::{PlatformBackendSpec, PlatformHttpRequest, RuntimeServices, use crate::redacted::Redacted; use super::DataDomeIntegration; -use super::protection_scope::{ProtectionRequestFacts, ProtectionScopeDecision}; +use super::protection_scope::{ + ProtectionRequestFacts, ProtectionScopeDecision, ProtectionSkipReason, +}; + +const MIN_TEST_BYPASS_CREDENTIAL_BYTES: usize = 32; const VALIDATE_REQUEST_PATH: &str = "/validate-request"; const REQUEST_MODULE_NAME: &str = "Trusted-Server-Rust"; @@ -144,15 +149,18 @@ impl DataDomeIntegration { }; match self.protection_scope.evaluate(&facts, input.services) { ProtectionScopeDecision::Protect => {} - ProtectionScopeDecision::Skip { rule_id, reason } => { - let client_tag_omitted = is_ip_exclusion_reason(reason); - if client_tag_omitted { + ProtectionScopeDecision::Skip { + rule_id, + reason, + suppress_client_tag, + } => { + if suppress_client_tag { input .request .extensions_mut() .insert(super::DataDomeClientTagSuppressed); } - log_protection_skip(input, &rule_id, reason); + log_protection_skip(input, &rule_id, reason, suppress_client_tag); return false; } } @@ -165,11 +173,23 @@ impl DataDomeIntegration { req: &mut Request, services: &RuntimeServices, ) -> bool { - let value = req.headers_mut().remove(super::HEADER_DATADOME_TEST_BYPASS); - let Some(bypass) = self.active_protection_test_bypass() else { + let supplied_values = req + .headers() + .get_all(super::HEADER_DATADOME_TEST_BYPASS) + .iter() + .cloned() + .collect::>(); + req.headers_mut().remove(super::HEADER_DATADOME_TEST_BYPASS); + if supplied_values.is_empty() { return false; - }; - let Some(value) = value else { + } + if supplied_values.len() != 1 { + log::warn!( + "[datadome] Multiple DataDome test bypass headers supplied; ignoring bypass" + ); + return false; + } + let Some(bypass) = self.active_protection_test_bypass() else { return false; }; @@ -178,10 +198,10 @@ impl DataDomeIntegration { .secret_store() .get_string(&store_name, &bypass.credential_secret_name) { - Ok(credential) if !credential.is_empty() => credential, + Ok(credential) if credential.len() >= MIN_TEST_BYPASS_CREDENTIAL_BYTES => credential, Ok(_) => { log::warn!( - "[datadome] DataDome test bypass credential is empty; ignoring bypass header" + "[datadome] DataDome test bypass credential does not meet security requirements; ignoring bypass header" ); return false; } @@ -193,7 +213,7 @@ impl DataDomeIntegration { } }; - let actual = Sha256::digest(value.as_bytes()); + let actual = Sha256::digest(supplied_values[0].as_bytes()); let expected = Sha256::digest(credential.as_bytes()); bool::from(actual.ct_eq(&expected)) } @@ -421,13 +441,16 @@ impl DataDomeIntegration { let (parts, body) = response.into_parts(); let status = parts.status; let Some(datadome_status) = datadome_response_status(&parts.headers) else { - log::warn!("[datadome] Protection API response missing X-DataDomeResponse"); + log::warn!( + "[datadome] Protection API response has missing or non-numeric verdict: api_status={} datadome_status=missing_or_invalid", + status.as_u16() + ); return RequestFilterDecision::Continue(RequestFilterEffects::default()); }; if datadome_status != status.as_u16() { log::warn!( - "[datadome] Protection API status/header mismatch: status={} header={}", + "[datadome] Protection API status/verdict mismatch: api_status={} datadome_status={}", status.as_u16(), datadome_status ); @@ -470,20 +493,14 @@ impl DataDomeIntegration { } log::warn!( - "[datadome] Protection API returned fail-open status {}", - status.as_u16() + "[datadome] Protection API returned unexpected fail-open status: api_status={} datadome_status={}", + status.as_u16(), + datadome_status ); RequestFilterDecision::Continue(RequestFilterEffects::default()) } } -fn is_ip_exclusion_reason(reason: &str) -> bool { - matches!( - reason, - "client_ip" | "client_ip_source" | "ip_cidr" | "ip_cidr_source" - ) -} - fn log_protection_test_bypass(input: &RequestFilterInput<'_>) { log::info!( "[datadome] protection decision=skipped rule=protection-test-bypass reason=test_bypass client_tag=omitted method={}", @@ -491,14 +508,37 @@ fn log_protection_test_bypass(input: &RequestFilterInput<'_>) { ); } -fn log_protection_skip(input: &RequestFilterInput<'_>, rule_id: &str, reason: &str) { - if is_ip_exclusion_reason(reason) { +fn suppression_skip_log_level(suppress_client_tag: bool, is_navigation: bool) -> log::Level { + if suppress_client_tag && is_navigation { + log::Level::Info + } else { + log::Level::Debug + } +} + +fn log_protection_skip( + input: &RequestFilterInput<'_>, + rule_id: &str, + reason: ProtectionSkipReason, + suppress_client_tag: bool, +) { + let reason = reason.as_str(); + if suppression_skip_log_level(suppress_client_tag, is_navigation_request(input.request)) + == log::Level::Info + { log::info!( "[datadome] protection decision=skipped rule={} reason={} client_tag=omitted method={}", rule_id, reason, input.request.method(), ); + } else if suppress_client_tag { + log::debug!( + "[datadome] protection decision=skipped rule={} reason={} client_tag=omitted method={}", + rule_id, + reason, + input.request.method(), + ); } else { log::debug!( "[datadome] protection decision=skipped rule={} reason={} method={}", @@ -509,6 +549,29 @@ fn log_protection_skip(input: &RequestFilterInput<'_>, rule_id: &str, reason: &s } } +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +enum ProtectionResultKind { + Allowed, + Blocked, + FailedOpen, +} + +fn classify_logged_protection_result( + status: StatusCode, + datadome_status: Option, + decision: &RequestFilterDecision, +) -> ProtectionResultKind { + match decision { + RequestFilterDecision::Respond { .. } => ProtectionResultKind::Blocked, + RequestFilterDecision::Continue(_) + if status == StatusCode::OK && datadome_status == Some(status.as_u16()) => + { + ProtectionResultKind::Allowed + } + RequestFilterDecision::Continue(_) => ProtectionResultKind::FailedOpen, + } +} + fn log_protection_result( input: &RequestFilterInput<'_>, status: StatusCode, @@ -516,23 +579,30 @@ fn log_protection_result( decision: &RequestFilterDecision, ) { let method = input.request.method(); - - match decision { - RequestFilterDecision::Respond { .. } => log::info!( - "[datadome] protection decision=blocked status={} method={} route=short_circuit", + let result_kind = classify_logged_protection_result(status, datadome_status, decision); + let datadome_status = datadome_status + .map(|value| value.to_string()) + .unwrap_or_else(|| "missing_or_invalid".to_string()); + + match result_kind { + ProtectionResultKind::Blocked => log::info!( + "[datadome] protection decision=blocked api_status={} datadome_status={} method={} route=short_circuit", status.as_u16(), + datadome_status, + method, + ), + ProtectionResultKind::Allowed => log::info!( + "[datadome] protection decision=allowed api_status={} datadome_status={} method={} route=continue", + status.as_u16(), + datadome_status, + method, + ), + ProtectionResultKind::FailedOpen => log::warn!( + "[datadome] protection decision=failed_open api_status={} datadome_status={} method={} route=continue", + status.as_u16(), + datadome_status, method, ), - RequestFilterDecision::Continue(_) - if status == StatusCode::OK && datadome_status == Some(status.as_u16()) => - { - log::info!( - "[datadome] protection decision=allowed status={} method={} route=continue", - status.as_u16(), - method, - ); - } - RequestFilterDecision::Continue(_) => {} } } @@ -830,11 +900,24 @@ mod tests { config: DataDomeConfig, services: &RuntimeServices, geo_info: Option<&GeoInfo>, + ) -> Request { + filter_marks_request_for_uri(config, services, geo_info, "https://publisher.example/page") + } + + fn filter_marks_request_for_uri( + config: DataDomeConfig, + services: &RuntimeServices, + geo_info: Option<&GeoInfo>, + uri: &str, ) -> Request { let integration = DataDomeIntegration::try_new(config).expect("should create DataDome integration"); let settings = Settings::default(); - let mut request = request_for_filter(); + let mut request = request_builder() + .method(Method::GET.as_str()) + .uri(uri) + .body(EdgeBody::empty()) + .expect("should build filter request"); let decision = futures::executor::block_on(integration.filter_protection_request( RequestFilterInput { settings: &settings, @@ -874,7 +957,7 @@ mod tests { let mut secrets = HashMap::new(); secrets.insert( "datadome_test_bypass".to_string(), - b"temporary-test-credential".to_vec(), + b"temporary-test-credential-32-bytes!".to_vec(), ); let http_client = Arc::new(StubHttpClient::new()); let services = build_services_with_secret_and_http_client( @@ -885,7 +968,7 @@ mod tests { let mut request = request_for_filter(); request.headers_mut().insert( super::super::HEADER_DATADOME_TEST_BYPASS, - edgezero_core::http::HeaderValue::from_static("temporary-test-credential"), + edgezero_core::http::HeaderValue::from_static("temporary-test-credential-32-bytes!"), ); let decision = filter_with_staging(&integration, &settings, &services, &mut request); @@ -996,7 +1079,7 @@ mod tests { ); secrets.insert( "datadome_test_bypass".to_string(), - b"temporary-test-credential".to_vec(), + b"temporary-test-credential-32-bytes!".to_vec(), ); let http_client = Arc::new(StubHttpClient::new()); http_client.push_response_with_headers( @@ -1012,7 +1095,7 @@ mod tests { let mut request = request_for_filter(); request.headers_mut().insert( super::super::HEADER_DATADOME_TEST_BYPASS, - edgezero_core::http::HeaderValue::from_static("temporary-test-credential"), + edgezero_core::http::HeaderValue::from_static("temporary-test-credential-32-bytes!"), ); let decision = temp_env::with_var( @@ -1077,7 +1160,7 @@ mod tests { let mut secrets = HashMap::new(); secrets.insert( "datadome_test_bypass".to_string(), - b"temporary-test-credential".to_vec(), + b"temporary-test-credential-32-bytes!".to_vec(), ); let http_client = Arc::new(StubHttpClient::new()); let services = build_services_with_secret_and_http_client( @@ -1088,7 +1171,7 @@ mod tests { let mut request = request_for_filter(); request.headers_mut().insert( super::super::HEADER_DATADOME_TEST_BYPASS, - edgezero_core::http::HeaderValue::from_static("temporary-test-credential"), + edgezero_core::http::HeaderValue::from_static("temporary-test-credential-32-bytes!"), ); let decision = filter_with_staging(&integration, &settings, &services, &mut request); @@ -1127,7 +1210,7 @@ mod tests { ); secrets.insert( "datadome_test_bypass".to_string(), - b"temporary-test-credential".to_vec(), + b"temporary-test-credential-32-bytes!".to_vec(), ); let http_client = Arc::new(StubHttpClient::new()); http_client.push_response_with_headers( @@ -1170,6 +1253,158 @@ mod tests { ); } + #[test] + fn duplicate_test_bypass_headers_fail_closed_and_are_all_stripped() { + let config = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_test_bypass: Some(ProtectionTestBypassConfig { + enabled: true, + credential_secret_store: "ts_secrets".to_string(), + credential_secret_name: "datadome_test_bypass".to_string(), + }), + ..DataDomeConfig::default() + }; + let integration = DataDomeIntegration::try_new(config).expect("should create integration"); + let mut secrets = HashMap::new(); + secrets.insert( + "datadome_server_side_key".to_string(), + b"server-side-key".to_vec(), + ); + secrets.insert( + "datadome_test_bypass".to_string(), + b"temporary-test-credential-32-bytes!".to_vec(), + ); + let http_client = Arc::new(StubHttpClient::new()); + http_client.push_response_with_headers( + 200, + Vec::new(), + vec![(HEADER_DATADOME_RESPONSE, "200")], + ); + let services = build_services_with_secret_and_http_client( + HashMapSecretStore::new(secrets), + http_client.clone(), + ); + let settings = Settings::default(); + let mut request = request_for_filter(); + for value in [ + "temporary-test-credential-32-bytes!", + "temporary-test-credential-32-bytes!", + ] { + request.headers_mut().append( + super::super::HEADER_DATADOME_TEST_BYPASS, + edgezero_core::http::HeaderValue::from_static(value), + ); + } + + let decision = filter_with_staging(&integration, &settings, &services, &mut request); + + assert!(matches!(decision, RequestFilterDecision::Continue(_))); + assert!( + request + .headers() + .get(super::super::HEADER_DATADOME_TEST_BYPASS) + .is_none(), + "all duplicate bypass values should be stripped" + ); + assert!(!has_client_tag_suppression_marker(&request)); + assert_eq!(http_client.recorded_backend_names().len(), 1); + } + + #[test] + fn test_bypass_credential_requires_at_least_32_bytes() { + for (credential, should_match) in [ + (Some("1234567890123456789012345678901"), false), + (Some("12345678901234567890123456789012"), true), + (Some(""), false), + (None, false), + ] { + let config = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_test_bypass: Some(ProtectionTestBypassConfig { + enabled: true, + credential_secret_store: "ts_secrets".to_string(), + credential_secret_name: "datadome_test_bypass".to_string(), + }), + ..DataDomeConfig::default() + }; + let integration = + DataDomeIntegration::try_new(config).expect("should create integration"); + let mut secrets = HashMap::new(); + secrets.insert( + "datadome_server_side_key".to_string(), + b"server-side-key".to_vec(), + ); + if let Some(credential) = credential { + secrets.insert( + "datadome_test_bypass".to_string(), + credential.as_bytes().to_vec(), + ); + } + let http_client = Arc::new(StubHttpClient::new()); + if !should_match { + http_client.push_response_with_headers( + 200, + Vec::new(), + vec![(HEADER_DATADOME_RESPONSE, "200")], + ); + } + let services = build_services_with_secret_and_http_client( + HashMapSecretStore::new(secrets), + http_client.clone(), + ); + let settings = Settings::default(); + let mut request = request_for_filter(); + let supplied = credential.unwrap_or("12345678901234567890123456789012"); + request.headers_mut().insert( + super::super::HEADER_DATADOME_TEST_BYPASS, + edgezero_core::http::HeaderValue::from_str(supplied) + .expect("should build bypass header"), + ); + + let decision = filter_with_staging(&integration, &settings, &services, &mut request); + + assert!(matches!(decision, RequestFilterDecision::Continue(_))); + assert_eq!(has_client_tag_suppression_marker(&request), should_match); + assert_eq!( + http_client.recorded_backend_names().is_empty(), + should_match, + "only a credential meeting the minimum should skip the API" + ); + } + } + + #[test] + fn protection_result_classifier_and_suppression_log_level_cover_outcomes() { + let continue_decision = RequestFilterDecision::Continue(RequestFilterEffects::default()); + let blocked_decision = RequestFilterDecision::Respond { + response: Box::new(Response::new(EdgeBody::empty())), + effects: RequestFilterEffects::default(), + }; + + assert_eq!( + classify_logged_protection_result(StatusCode::OK, Some(200), &continue_decision), + ProtectionResultKind::Allowed + ); + assert_eq!( + classify_logged_protection_result(StatusCode::FORBIDDEN, Some(403), &blocked_decision), + ProtectionResultKind::Blocked + ); + for (status, datadome_status) in [ + (StatusCode::OK, None), + (StatusCode::OK, Some(403)), + (StatusCode::CREATED, Some(201)), + ] { + assert_eq!( + classify_logged_protection_result(status, datadome_status, &continue_decision), + ProtectionResultKind::FailedOpen + ); + } + assert_eq!(suppression_skip_log_level(true, true), log::Level::Info); + assert_eq!(suppression_skip_log_level(true, false), log::Level::Debug); + } + #[test] fn ip_exclusions_mark_requests_for_client_tag_suppression() { let ip = IpAddr::V4(Ipv4Addr::new(192, 0, 2, 10)); @@ -1257,29 +1492,89 @@ mod tests { #[test] fn non_ip_exclusions_do_not_mark_requests_for_client_tag_suppression() { let ip = IpAddr::V4(Ipv4Addr::new(192, 0, 2, 10)); - let cases = [DataDomeConfig { - enabled: true, - enable_protection: true, - protection_exclusion_rules: vec![ProtectionExclusionRuleConfig { - id: "path".to_string(), - enabled: true, - methods: Vec::new(), - matcher: ProtectionMatcherConfig::PathExact { - paths: vec!["/page".to_string()], + let cases = [ + ( + ProtectionMatcherConfig::PathExact { + paths: vec!["/exact".to_string()], }, - }], - ..DataDomeConfig::default() - }]; + "https://publisher.example/exact", + ), + ( + ProtectionMatcherConfig::PathPrefix { + prefixes: vec!["/prefix/".to_string()], + }, + "https://publisher.example/prefix/page", + ), + ( + ProtectionMatcherConfig::PathRegex { + patterns: vec![r"^/regex/[0-9]+$".to_string()], + }, + "https://publisher.example/regex/42", + ), + ( + ProtectionMatcherConfig::QueryParamNonEmpty { + names: vec!["skip".to_string()], + }, + "https://publisher.example/page?skip=yes", + ), + ]; - for config in cases { - let request = filter_marks_request(config, &noop_services_with_client_ip(ip)); + for (matcher, uri) in cases { + let config = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_exclusion_rules: vec![ProtectionExclusionRuleConfig { + id: "non-ip".to_string(), + enabled: true, + methods: Vec::new(), + matcher, + }], + ..DataDomeConfig::default() + }; + let request = + filter_marks_request_for_uri(config, &noop_services_with_client_ip(ip), None, uri); assert!( !has_client_tag_suppression_marker(&request), - "non-IP exclusions should not mark the request" + "matching non-IP exclusion should not mark {uri}" ); } } + #[test] + fn overlapping_path_and_ip_exclusions_still_mark_request() { + let ip = IpAddr::V4(Ipv4Addr::new(192, 0, 2, 10)); + let config = DataDomeConfig { + enabled: true, + enable_protection: true, + protection_exclusion_rules: vec![ + ProtectionExclusionRuleConfig { + id: "path-first".to_string(), + enabled: true, + methods: Vec::new(), + matcher: ProtectionMatcherConfig::PathExact { + paths: vec!["/page".to_string()], + }, + }, + ProtectionExclusionRuleConfig { + id: "ip-second".to_string(), + enabled: true, + methods: Vec::new(), + matcher: ProtectionMatcherConfig::IpCidr { + cidrs: vec!["192.0.2.0/24".to_string()], + }, + }, + ], + ..DataDomeConfig::default() + }; + + let request = filter_marks_request(config, &noop_services_with_client_ip(ip)); + + assert!( + has_client_tag_suppression_marker(&request), + "overlapping IP exclusion should suppress even when path remains the primary reason" + ); + } + #[test] fn asn_exclusions_do_not_mark_requests_for_client_tag_suppression() { let config = DataDomeConfig { diff --git a/crates/trusted-server-core/src/integrations/datadome/protection_scope.rs b/crates/trusted-server-core/src/integrations/datadome/protection_scope.rs index cce56f2fa..a83f73217 100644 --- a/crates/trusted-server-core/src/integrations/datadome/protection_scope.rs +++ b/crates/trusted-server-core/src/integrations/datadome/protection_scope.rs @@ -91,12 +91,52 @@ pub(super) struct ProtectionRequestFacts<'a> { pub(super) asn: Option, } +/// Typed reason why `DataDome` protection was skipped. +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub(super) enum ProtectionSkipReason { + Method, + ClientIp, + ClientIpSource, + Asn, + PathExact, + PathPrefix, + PathRegex, + QueryParamNonEmpty, + IpCidr, + IpCidrSource, +} + +impl ProtectionSkipReason { + pub(super) const fn as_str(self) -> &'static str { + match self { + Self::Method => "method", + Self::ClientIp => "client_ip", + Self::ClientIpSource => "client_ip_source", + Self::Asn => "asn", + Self::PathExact => "path_exact", + Self::PathPrefix => "path_prefix", + Self::PathRegex => "path_regex", + Self::QueryParamNonEmpty => "query_param_non_empty", + Self::IpCidr => "ip_cidr", + Self::IpCidrSource => "ip_cidr_source", + } + } + + pub(super) const fn is_ip_based(self) -> bool { + matches!( + self, + Self::ClientIp | Self::ClientIpSource | Self::IpCidr | Self::IpCidrSource + ) + } +} + /// Result of evaluating whether `DataDome` protection should run. pub(super) enum ProtectionScopeDecision { Protect, Skip { rule_id: String, - reason: &'static str, + reason: ProtectionSkipReason, + suppress_client_tag: bool, }, } @@ -210,51 +250,75 @@ impl ProtectionScope { facts: &ProtectionRequestFacts<'_>, services: &RuntimeServices, ) -> ProtectionScopeDecision { - if self.excluded_methods.matches(facts.method) { - return ProtectionScopeDecision::Skip { - rule_id: "excluded-methods".to_string(), - reason: "method", - }; - } + let mut primary = self + .excluded_methods + .matches(facts.method) + .then(|| ("excluded-methods".to_string(), ProtectionSkipReason::Method)); if let Some(client_ip) = facts.client_ip { if cidrs_match(&self.excluded_ip_cidrs, client_ip) { + let (rule_id, reason) = primary.unwrap_or_else(|| { + ( + "excluded-ip-cidrs".to_string(), + ProtectionSkipReason::ClientIp, + ) + }); return ProtectionScopeDecision::Skip { - rule_id: "excluded-ip-cidrs".to_string(), - reason: "client_ip", + rule_id, + reason, + suppress_client_tag: true, }; } for source in &self.excluded_ip_cidr_sources { if source.matches(client_ip, services, self.ip_list_cache_ttl) { + let (rule_id, reason) = primary.unwrap_or_else(|| { + (source.rule_id(), ProtectionSkipReason::ClientIpSource) + }); return ProtectionScopeDecision::Skip { - rule_id: source.rule_id(), - reason: "client_ip_source", + rule_id, + reason, + suppress_client_tag: true, }; } } } - if facts - .asn - .is_some_and(|asn| self.excluded_asns.contains(&asn)) + if primary.is_none() + && facts + .asn + .is_some_and(|asn| self.excluded_asns.contains(&asn)) { - return ProtectionScopeDecision::Skip { - rule_id: "excluded-asns".to_string(), - reason: "asn", - }; + primary = Some(("excluded-asns".to_string(), ProtectionSkipReason::Asn)); } for rule in &self.exclusion_rules { + let reason = rule.matcher.reason(); + if primary.is_some() && !reason.is_ip_based() { + continue; + } if rule.matches(facts, services, self.ip_list_cache_ttl) { - return ProtectionScopeDecision::Skip { - rule_id: rule.id.clone(), - reason: rule.matcher.reason(), - }; + let suppress_client_tag = reason.is_ip_based(); + let (rule_id, reason) = primary.unwrap_or_else(|| (rule.id.clone(), reason)); + if suppress_client_tag { + return ProtectionScopeDecision::Skip { + rule_id, + reason, + suppress_client_tag, + }; + } + primary = Some((rule_id, reason)); } } - ProtectionScopeDecision::Protect + match primary { + Some((rule_id, reason)) => ProtectionScopeDecision::Skip { + rule_id, + reason, + suppress_client_tag: false, + }, + None => ProtectionScopeDecision::Protect, + } } } @@ -489,15 +553,15 @@ impl ProtectionMatcher { } } - fn reason(&self) -> &'static str { + fn reason(&self) -> ProtectionSkipReason { match self { - ProtectionMatcher::PathExact(_) => "path_exact", - ProtectionMatcher::PathPrefix(_) => "path_prefix", - ProtectionMatcher::PathRegex(_) => "path_regex", - ProtectionMatcher::QueryParamNonEmpty(_) => "query_param_non_empty", - ProtectionMatcher::Asn(_) => "asn", - ProtectionMatcher::IpCidr(_) => "ip_cidr", - ProtectionMatcher::IpCidrSource(_) => "ip_cidr_source", + ProtectionMatcher::PathExact(_) => ProtectionSkipReason::PathExact, + ProtectionMatcher::PathPrefix(_) => ProtectionSkipReason::PathPrefix, + ProtectionMatcher::PathRegex(_) => ProtectionSkipReason::PathRegex, + ProtectionMatcher::QueryParamNonEmpty(_) => ProtectionSkipReason::QueryParamNonEmpty, + ProtectionMatcher::Asn(_) => ProtectionSkipReason::Asn, + ProtectionMatcher::IpCidr(_) => ProtectionSkipReason::IpCidr, + ProtectionMatcher::IpCidrSource(_) => ProtectionSkipReason::IpCidrSource, } } } @@ -741,7 +805,8 @@ mod tests { assert!(matches!( decision, ProtectionScopeDecision::Skip { - reason: "method", + reason: ProtectionSkipReason::Method, + suppress_client_tag: false, .. } )); @@ -758,7 +823,11 @@ mod tests { assert!(matches!( decision, - ProtectionScopeDecision::Skip { reason: "asn", .. } + ProtectionScopeDecision::Skip { + reason: ProtectionSkipReason::Asn, + suppress_client_tag: false, + .. + } )); } @@ -783,7 +852,8 @@ mod tests { assert!(matches!( decision, ProtectionScopeDecision::Skip { - reason: "client_ip", + reason: ProtectionSkipReason::ClientIp, + suppress_client_tag: true, .. } )); @@ -817,7 +887,8 @@ mod tests { assert!(matches!( decision, ProtectionScopeDecision::Skip { - reason: "client_ip_source", + reason: ProtectionSkipReason::ClientIpSource, + suppress_client_tag: true, .. } )); @@ -840,7 +911,8 @@ mod tests { assert!(matches!( scope.evaluate(&facts("GET", "/app.JSON", None, None, None), &services), ProtectionScopeDecision::Skip { - reason: "path_regex", + reason: ProtectionSkipReason::PathRegex, + suppress_client_tag: false, .. } )); @@ -850,6 +922,119 @@ mod tests { )); } + #[test] + fn method_skip_detects_overlapping_inline_ip_exclusion() { + let mut config = config_with_protection(); + config.protection_excluded_methods = vec!["GET".to_string()]; + config.protection_excluded_ip_cidrs = vec!["192.0.2.0/24".to_string()]; + let scope = ProtectionScope::compile(&config).expect("should compile scope"); + let services = crate::platform::test_support::noop_services(); + + let decision = scope.evaluate( + &facts( + "GET", + "/page", + None, + Some(IpAddr::V4(Ipv4Addr::new(192, 0, 2, 10))), + None, + ), + &services, + ); + + assert!(matches!( + decision, + ProtectionScopeDecision::Skip { + reason: ProtectionSkipReason::Method, + suppress_client_tag: true, + .. + } + )); + } + + #[test] + fn earlier_non_ip_skip_detects_later_structured_ip_exclusion() { + for primary_asn in [None, Some(64500)] { + let mut config = config_with_protection(); + config.protection_excluded_asns = primary_asn.into_iter().collect(); + config.protection_exclusion_rules = vec![ + ProtectionExclusionRuleConfig { + id: "path".to_string(), + enabled: primary_asn.is_none(), + methods: Vec::new(), + matcher: ProtectionMatcherConfig::PathExact { + paths: vec!["/page".to_string()], + }, + }, + ProtectionExclusionRuleConfig { + id: "ip".to_string(), + enabled: true, + methods: Vec::new(), + matcher: ProtectionMatcherConfig::IpCidr { + cidrs: vec!["192.0.2.0/24".to_string()], + }, + }, + ]; + let scope = ProtectionScope::compile(&config).expect("should compile scope"); + let services = crate::platform::test_support::noop_services(); + + let decision = scope.evaluate( + &facts( + "GET", + "/page", + None, + Some(IpAddr::V4(Ipv4Addr::new(192, 0, 2, 10))), + primary_asn, + ), + &services, + ); + + assert!(matches!( + decision, + ProtectionScopeDecision::Skip { + reason: ProtectionSkipReason::Asn | ProtectionSkipReason::PathExact, + suppress_client_tag: true, + .. + } + )); + } + } + + #[test] + fn method_scoped_structured_ip_does_not_suppress_when_scope_does_not_apply() { + let mut config = config_with_protection(); + config.protection_excluded_methods = vec!["POST".to_string()]; + config.protection_exclusion_rules = vec![ProtectionExclusionRuleConfig { + id: "get-ip".to_string(), + enabled: true, + methods: vec!["GET".to_string()], + matcher: ProtectionMatcherConfig::IpCidr { + cidrs: vec!["192.0.2.0/24".to_string()], + }, + }]; + let scope = ProtectionScope::compile(&config).expect("should compile scope"); + let services = crate::platform::test_support::noop_services(); + + let decision = scope.evaluate( + &facts( + "POST", + "/page", + None, + Some(IpAddr::V4(Ipv4Addr::new(192, 0, 2, 10))), + None, + ), + &services, + ); + + assert!(matches!( + decision, + ProtectionScopeDecision::Skip { + reason: ProtectionSkipReason::Method, + suppress_client_tag: false, + .. + } + )); + } + #[test] fn rule_query_param_non_empty_matches_rsc() { let mut config = config_with_protection(); @@ -870,7 +1055,8 @@ mod tests { &services ), ProtectionScopeDecision::Skip { - reason: "query_param_non_empty", + reason: ProtectionSkipReason::QueryParamNonEmpty, + suppress_client_tag: false, .. } )); diff --git a/crates/trusted-server-core/src/platform/test_support.rs b/crates/trusted-server-core/src/platform/test_support.rs index 10389c787..917f1bf50 100644 --- a/crates/trusted-server-core/src/platform/test_support.rs +++ b/crates/trusted-server-core/src/platform/test_support.rs @@ -769,6 +769,14 @@ pub(crate) fn build_services_with_backend_and_http_client( pub(crate) fn build_services_with_secret_and_http_client( secret_store: impl PlatformSecretStore + 'static, http_client: Arc, +) -> RuntimeServices { + build_services_with_secret_http_client_and_client_ip(secret_store, http_client, None) +} + +pub(crate) fn build_services_with_secret_http_client_and_client_ip( + secret_store: impl PlatformSecretStore + 'static, + http_client: Arc, + client_ip: Option, ) -> RuntimeServices { RuntimeServices::builder() .config_store(Arc::new(NoopConfigStore)) @@ -778,7 +786,7 @@ pub(crate) fn build_services_with_secret_and_http_client( .http_client(http_client) .geo(Arc::new(NoopGeo)) .client_info(ClientInfo { - client_ip: None, + client_ip, tls_protocol: None, tls_cipher: None, ..ClientInfo::default() diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index 66b8a9553..4bed98327 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -59,7 +59,7 @@ use crate::http_util::{RequestInfo, is_navigation_request, serve_static_with_eta use crate::integrations::IntegrationRegistry; use crate::platform::{GeoInfo, PlatformBackendSpec, PlatformHttpRequest, RuntimeServices}; use crate::price_bucket::{PriceGranularity, price_bucket}; -use crate::response_privacy::CDN_CACHE_HEADERS; +use crate::response_privacy::enforce_synthesized_html_cache_privacy; use crate::rsc_flight::RscFlightUrlRewriter; use crate::settings::Settings; use crate::streaming_processor::{ @@ -1424,6 +1424,22 @@ pub async fn publisher_response_into_streaming_response( } } +/// Returns whether a request can render an HTML document context. +fn is_html_document_request(req: &Request) -> bool { + if let Some(destination) = req + .headers() + .get("sec-fetch-dest") + .and_then(|value| value.to_str().ok()) + { + return matches!( + destination.trim().to_ascii_lowercase().as_str(), + "document" | "embed" | "fencedframe" | "frame" | "iframe" | "object" + ); + } + + is_navigation_request(req) +} + /// Removes request headers that can produce a bodyless or partial origin response. fn strip_conditional_and_range_headers(req: &mut Request) { req.headers_mut().remove(header::IF_NONE_MATCH); @@ -1454,29 +1470,11 @@ fn apply_datadome_client_tag_cache_privacy( suppress_datadome_client_side_tag: bool, content_type: &str, ) { - if !suppress_datadome_client_side_tag - || !response_carries_body(method, response.status()) - || !is_html_content_type(content_type) + if suppress_datadome_client_side_tag + && response_carries_body(method, response.status()) + && is_html_content_type(content_type) { - return; - } - - let already_uncacheable = response - .headers() - .get(header::CACHE_CONTROL) - .and_then(|value| value.to_str().ok()) - .map(str::to_ascii_lowercase) - .is_some_and(|value| value.contains("private") || value.contains("no-store")); - if !already_uncacheable { - response.headers_mut().insert( - header::CACHE_CONTROL, - HeaderValue::from_static("private, max-age=0"), - ); - } - response.headers_mut().remove(header::ETAG); - response.headers_mut().remove(header::LAST_MODIFIED); - for header_name in CDN_CACHE_HEADERS { - response.headers_mut().remove(*header_name); + enforce_synthesized_html_cache_privacy(response); } } @@ -2900,10 +2898,11 @@ pub async fn handle_publisher_request( .extensions() .get::() .is_some(); - if should_run_ad_stack || suppress_datadome_client_side_tag { - // The origin content type is not known yet, so request hints cannot safely - // narrow this to HTML without allowing 304 or 206 responses to bypass a - // response mutation that becomes necessary after the fetch. + if should_run_ad_stack || (suppress_datadome_client_side_tag && is_html_document_request(&req)) + { + // HTML document contexts whose output may be synthesized must not + // receive a cached 304 or partial 206. Non-document subresources contain + // no executable injected tag, so retain their validators and ranges. strip_conditional_and_range_headers(&mut req); } @@ -3015,23 +3014,7 @@ pub async fn handle_publisher_request( .and_then(|h| h.to_str().ok()) .unwrap_or_default(); if should_run_ad_stack && is_html_content_type(origin_content_type) { - response.headers_mut().insert( - header::CACHE_CONTROL, - HeaderValue::from_static("private, no-store"), - ); - response.headers_mut().remove(header::ETAG); - response.headers_mut().remove(header::LAST_MODIFIED); - // Every CDN-targeted cache directive, not just the browser-facing - // `Cache-Control` above: an origin emitting any of these would otherwise - // instruct an intermediary to store a synthesized per-navigation - // document. `Surrogate-Control` and `Fastly-Surrogate-Control` cover - // Fastly; `CDN-Cache-Control` is the standard targeted field (RFC 9213) - // and `Cloudflare-CDN-Cache-Control` is the Cloudflare-specific field - // that overrides it there, so both are needed to close the gap on the - // Cloudflare adapter. - for directive in CDN_CACHE_HEADERS { - response.headers_mut().remove(*directive); - } + enforce_synthesized_html_cache_privacy(&mut response); } let content_type = response @@ -4218,7 +4201,8 @@ mod tests { use crate::auction::types::{AdFormat, AdSlot, MediaType}; use crate::integrations::IntegrationRegistry; use crate::platform::test_support::{ - StubHttpClient, build_services_with_http_client, noop_services, + NoopSecretStore, StubHttpClient, build_services_with_http_client, + build_services_with_secret_http_client_and_client_ip, noop_services, noop_services_with_telemetry_sink, }; use crate::test_support::tests::create_test_settings; @@ -5386,7 +5370,7 @@ mod tests { } #[tokio::test] - async fn suppressed_publisher_request_removes_conditional_validators() { + async fn suppressed_navigation_removes_conditional_and_range_headers() { let settings = create_test_settings(); let stub = Arc::new(StubHttpClient::new()); stub.push_response_with_headers( @@ -5401,6 +5385,7 @@ mod tests { .method(Method::GET) .uri("https://publisher.example/page") .header(header::HOST, "publisher.example") + .header("sec-fetch-dest", "document") .header(header::IF_NONE_MATCH, "\"cached-page\"") .header(header::IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT") .header(header::RANGE, "bytes=0-18") @@ -5427,7 +5412,105 @@ mod tests { headers .iter() .all(|(name, _)| !name.eq_ignore_ascii_case(header_name.as_str())), - "suppressed requests must not forward {header_name}" + "suppressed navigations must not forward {header_name}" + ); + } + } + + #[tokio::test] + async fn suppressed_iframe_removes_conditional_and_range_headers() { + let settings = create_test_settings(); + let stub = Arc::new(StubHttpClient::new()); + stub.push_response_with_headers( + 200, + b"frame".to_vec(), + vec![("content-type", "text/html; charset=utf-8")], + ); + let services = build_services_with_http_client( + Arc::clone(&stub) as Arc + ); + let mut req = HttpRequest::builder() + .method(Method::GET) + .uri("https://publisher.example/frame") + .header(header::HOST, "publisher.example") + .header("sec-fetch-dest", "iframe") + .header(header::IF_NONE_MATCH, "\"cached-frame\"") + .header(header::IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT") + .header(header::RANGE, "bytes=0-18") + .header(header::IF_RANGE, "\"cached-frame\"") + .body(EdgeBody::empty()) + .expect("should build conditional iframe request"); + req.extensions_mut() + .insert(crate::integrations::datadome::DataDomeClientTagSuppressed); + + let _response = run_publisher_proxy(&settings, &services, req).await; + + let headers = stub + .recorded_request_headers() + .into_iter() + .next() + .expect("should record one outbound request"); + for header_name in [ + header::IF_NONE_MATCH, + header::IF_MODIFIED_SINCE, + header::RANGE, + header::IF_RANGE, + ] { + assert!( + headers + .iter() + .all(|(name, _)| !name.eq_ignore_ascii_case(header_name.as_str())), + "suppressed iframe documents must not forward {header_name}" + ); + } + } + + #[tokio::test] + async fn suppressed_subresource_preserves_conditional_and_range_headers() { + let settings = create_test_settings(); + let stub = Arc::new(StubHttpClient::new()); + stub.push_response_with_headers( + 200, + b"video".to_vec(), + vec![("content-type", "video/mp4")], + ); + let services = build_services_with_http_client( + Arc::clone(&stub) as Arc + ); + let mut req = HttpRequest::builder() + .method(Method::GET) + .uri("https://publisher.example/video.mp4") + .header(header::HOST, "publisher.example") + .header("sec-fetch-dest", "video") + .header(header::IF_NONE_MATCH, "\"cached-video\"") + .header(header::IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT") + .header(header::RANGE, "bytes=0-18") + .header(header::IF_RANGE, "\"cached-video\"") + .body(EdgeBody::empty()) + .expect("should build conditional subresource request"); + req.extensions_mut() + .insert(crate::integrations::datadome::DataDomeClientTagSuppressed); + + let _response = run_publisher_proxy(&settings, &services, req).await; + + let headers = stub + .recorded_request_headers() + .into_iter() + .next() + .expect("should record one outbound request"); + for (header_name, expected) in [ + (header::IF_NONE_MATCH, "\"cached-video\""), + (header::IF_MODIFIED_SINCE, "Wed, 21 Oct 2015 07:28:00 GMT"), + (header::RANGE, "bytes=0-18"), + (header::IF_RANGE, "\"cached-video\""), + ] { + assert_eq!( + headers + .iter() + .find(|(name, _)| name.eq_ignore_ascii_case(header_name.as_str())) + .map(|(_, value)| value.as_str()), + Some(expected), + "suppressed subresources should preserve {header_name}" ); } } @@ -5547,6 +5630,77 @@ mod tests { ); } + #[tokio::test] + async fn datadome_filter_marker_survives_into_publisher_html_pipeline() { + let mut settings = create_test_settings(); + settings + .integrations + .insert_config( + "datadome", + &serde_json::json!({ + "enabled": true, + "enable_protection": true, + "protection_excluded_ip_cidrs": ["192.0.2.0/24"], + "client_side_key": "test-client-key", + }), + ) + .expect("should configure DataDome integration"); + let registry = IntegrationRegistry::new(&settings) + .expect("should create integration registry with DataDome"); + let stub = Arc::new(StubHttpClient::new()); + stub.push_response_with_headers( + 200, + b"content".to_vec(), + vec![("content-type", "text/html; charset=utf-8")], + ); + let services = build_services_with_secret_http_client_and_client_ip( + NoopSecretStore, + Arc::clone(&stub) as Arc, + Some("192.0.2.10".parse().expect("should parse client IP")), + ); + let mut req = HttpRequest::builder() + .method(Method::GET) + .uri("https://publisher.example/page") + .header(header::HOST, "publisher.example") + .header("sec-fetch-dest", "document") + .body(EdgeBody::empty()) + .expect("should build request"); + + let filter_outcome = registry + .filter_request(crate::integrations::RequestFilterRegistryInput { + settings: &settings, + services: &services, + req: &mut req, + geo_info: None, + }) + .await + .expect("should run DataDome filter"); + assert!(matches!( + filter_outcome, + crate::integrations::RequestFilterRegistryOutcome::Continue(_) + )); + let publisher_response = run_publisher_proxy(&settings, &services, req).await; + let response = buffer_publisher_response_async( + publisher_response, + &Method::GET, + &settings, + ®istry, + &AuctionOrchestrator::new(settings.auction.clone()), + &services, + ) + .await + .expect("should buffer publisher response"); + let html = response_body_string(response); + + assert!(!html.contains("window.ddjskey")); + assert!(!html.contains("/integrations/datadome/tags.js")); + assert_eq!( + stub.recorded_backend_names().len(), + 1, + "only the publisher origin should be called" + ); + } + #[test] fn suppressed_datadome_tag_reaches_publisher_html_pipeline() { let mut settings = create_test_settings(); @@ -5613,8 +5767,8 @@ mod tests { .headers() .get(header::CACHE_CONTROL) .and_then(|value| value.to_str().ok()), - Some("private, max-age=0"), - "suppressed HTML should be private" + Some("private, no-store"), + "suppressed HTML should be private and non-storable" ); assert!( response.headers().get("surrogate-control").is_none(), @@ -5658,8 +5812,8 @@ mod tests { .headers() .get(header::CACHE_CONTROL) .and_then(|value| value.to_str().ok()), - Some("no-store"), - "suppressed HTML should preserve an existing no-store policy" + Some("private, no-store"), + "suppressed HTML should use the exact synthesized-HTML policy" ); } diff --git a/crates/trusted-server-core/src/response_privacy.rs b/crates/trusted-server-core/src/response_privacy.rs index c2b1447e3..21ba9f20b 100644 --- a/crates/trusted-server-core/src/response_privacy.rs +++ b/crates/trusted-server-core/src/response_privacy.rs @@ -24,6 +24,27 @@ pub const CDN_CACHE_HEADERS: &[&str] = &[ "cloudflare-cdn-cache-control", ]; +fn strip_cdn_cache_headers(response: &mut Response) { + for name in CDN_CACHE_HEADERS { + response.headers_mut().remove(*name); + } +} + +/// Forces synthesized HTML to be private and non-storable. +/// +/// Use this exact policy whenever Trusted Server changes an origin HTML +/// representation with request-specific content: force `private, no-store`, +/// remove origin validators, and remove all CDN-targeted cache directives. +pub(crate) fn enforce_synthesized_html_cache_privacy(response: &mut Response) { + response.headers_mut().insert( + header::CACHE_CONTROL, + HeaderValue::from_static("private, no-store"), + ); + response.headers_mut().remove(header::ETAG); + response.headers_mut().remove(header::LAST_MODIFIED); + strip_cdn_cache_headers(response); +} + /// Forces cookie-bearing responses to stay private to shared caches. /// /// Any response that sets a per-user cookie (notably the EC identity cookie) @@ -41,9 +62,7 @@ pub fn enforce_set_cookie_cache_privacy(response: &mut Response) { // one already carrying a stricter `no-store`/`private` directive โ€” they are // independent of Cache-Control and would otherwise let a shared cache store // and replay one visitor's Set-Cookie. - for name in CDN_CACHE_HEADERS { - response.headers_mut().remove(*name); - } + strip_cdn_cache_headers(response); // Cache-Control directives are case-insensitive (RFC 9111 ยง5.2), so match // against a lowercased copy โ€” `No-Store` / `Private` must count. let already_uncacheable = response @@ -149,6 +168,37 @@ mod tests { s } + #[test] + fn synthesized_html_is_forced_no_store_without_validators_or_cdn_headers() { + let mut response = response_builder() + .header(header::CACHE_CONTROL, "private, max-age=600") + .header(header::ETAG, "\"origin\"") + .header(header::LAST_MODIFIED, "Wed, 21 Oct 2015 07:28:00 GMT") + .header("surrogate-control", "max-age=600") + .header("fastly-surrogate-control", "max-age=600") + .header("cdn-cache-control", "max-age=600") + .header("cloudflare-cdn-cache-control", "max-age=600") + .body(edgezero_core::body::Body::empty()) + .expect("should build response"); + + enforce_synthesized_html_cache_privacy(&mut response); + + assert_eq!( + response.headers()[header::CACHE_CONTROL], + "private, no-store", + "synthesized HTML should always be non-storable" + ); + for header_name in [header::ETAG.as_str(), header::LAST_MODIFIED.as_str()] + .into_iter() + .chain(CDN_CACHE_HEADERS.iter().copied()) + { + assert!( + !response.headers().contains_key(header_name), + "synthesized HTML should remove {header_name}" + ); + } + } + #[test] fn downgrades_public_cache_control_on_cookie_response() { let settings = settings_with_response_headers(&[("cache-control", "public, max-age=600")]); diff --git a/docs/guide/integrations/datadome.md b/docs/guide/integrations/datadome.md index ff2eb6aaa..0c2d8ae6c 100644 --- a/docs/guide/integrations/datadome.md +++ b/docs/guide/integrations/datadome.md @@ -86,7 +86,7 @@ patterns = ["(?i)\\.(avi|flv|mka|mkv|mov|mp4|mpeg|mpg|mp3|flac|ogg|ogm|opus|wav| | `protection_excluded_ip_cidr_sources` | array | `[]` | Config Store sources containing dynamic client IP CIDR bypass lists | | `protection_ip_list_cache_ttl_seconds` | integer | `300` | Process-local cache TTL for Config Store-backed IP CIDR bypass lists | | `protection_exclusion_rules` | array | Static asset path regex | Structured method/path/query/IP/ASN exclusion rules | -| `protection_test_bypass` | object | omitted | Temporary static-header bypass for access-controlled staging tests | +| `protection_test_bypass` | object | omitted | Staging-only fixed-header bypass; secret must contain at least 32 bytes | | `enable_graphql_support` | boolean | `false` | Reserved for future GraphQL body inspection; ignored in v1 | | `client_side_key` | string | `""` | DataDome client-side JavaScript key used for tag injection | | `inject_client_side_tag` | boolean | `true` | Auto-inject the browser tag when `client_side_key` is non-empty | @@ -190,19 +190,25 @@ credential_secret_name = "datadome_test_bypass" ``` `protection_test_bypass` requires `enable_protection = true`; it is disabled -when omitted and is runtime-active only when `FASTLY_IS_STAGING=1`. A retained +when omitted and is runtime-active only when `FASTLY_IS_STAGING=1`. +`FASTLY_IS_STAGING` is supplied at runtime by Fastly (`1` in staging and `0` in +production); it is not compiled into or promoted with the Wasm artifact. Verify +staging through the `X-TS-ENV: staging` response signal and the integration +activation log, and verify production omits that response signal. A retained section cannot bypass protection in a production or other non-staging runtime. -Store the temporary credential in the configured Secret Store, configure this -section only while needed, protect the site with an outer access control such -as Basic Auth, and remove the section when testing finishes. - -Whenever the enabled DataDome request filter runs, the fixed -`x-ts-datadome-bypass` header is removed before configuration or credential -checks. It therefore cannot reach DataDome or the publisher origin when the -bypass is absent, disabled, inactive, or invalid. Active credentials are -compared in constant time and never logged. Scope the header to the staging origin; do not attach it to every -request in a browser context because that can disclose the credential to -third-party origins. With Playwright: +Store a randomly generated credential containing at least 32 bytes of +high-entropy material in the configured Secret Store, configure this section +only while needed, protect the site with an outer access control such as Basic +Auth, and remove the section when testing finishes. + +Whenever the enabled DataDome request filter runs on the Fastly adapter, the +fixed `x-ts-datadome-bypass` header is removed before configuration or +credential checks. It therefore cannot reach DataDome or the publisher origin +through that path when the bypass is absent, disabled, inactive, or invalid. +Active credentials are compared in constant time and never logged. Duplicate +header values fail closed. Scope the header to the staging origin; do not attach +it to every request in a browser context because that can disclose the +credential to third-party origins. With Playwright: ```ts await context.route('https://staging.example.com/**', async (route) => { @@ -229,15 +235,25 @@ This behavior applies to: - structured `ip_cidr_source` rules; and - a matching enabled `protection_test_bypass` credential in a staging runtime. -ASN, method, path, query-parameter, static-asset, and internal-route -exclusions do not automatically suppress the client-side tag. DataDome tags -already present in publisher HTML are not removed or changed by this behavior, -and `/integrations/datadome/tags.js` remains available when requested directly. +Method, ASN, path, query-parameter, static-asset, and internal-route exclusions +alone do not suppress the client-side tag. However, a simultaneous matching IP +exclusion suppresses it regardless of which first-match rule and reason are +logged. DataDome tags already present in publisher HTML are not removed or +changed by this behavior, and `/integrations/datadome/tags.js` remains available +when requested directly. Because the processed HTML differs by client IP or test credential, -tag-suppressed HTML is marked `private, max-age=0` and removed from shared -surrogate caches. The decision is reported in the existing protection log, for -example: +tag-suppressed HTML is marked `private, no-store`, has origin validators +removed, and has shared-surrogate cache directives removed. This response-time +policy cannot invalidate tag-bearing HTML already held by a shared cache in +front of Trusted Server. Guaranteed suppression requires bypassing or purging +that cache, or avoiding shared caching ahead of Trusted Server. + +IP-exclusion suppression skips are logged at `info` for navigations and `debug` +for subresources; matching test-bypass events remain at `info` as security audit +events. Protection API result logs classify `allowed`, `blocked`, and +`failed_open` outcomes and use distinct `api_status` and `datadome_status` +fields. For example: ```text [datadome] protection decision=skipped rule=protection-test-bypass reason=test_bypass client_tag=omitted method=GET diff --git a/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md b/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md index 2d9b0eae2..9f2c1674f 100644 --- a/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md +++ b/docs/superpowers/plans/2026-08-03-datadome-ip-excluded-client-tag.md @@ -40,10 +40,9 @@ request-filter wiring. 3. **Private marker:** communicate the decision with a typed request extension, never a request/response header. The marker cannot leak to the origin or client. -4. **Precise scope:** tag suppression is keyed only on decision reasons - `client_ip`, `client_ip_source`, `ip_cidr`, and `ip_cidr_source`. +4. **Precise scope:** tag suppression uses typed scope metadata and applies whenever an IP exclusion overlaps the primary first-match skip reason. 5. **Cache safety:** an HTML response with the tag omitted differs by client IP. - A suppressed processed HTML response must be `private, max-age=0` and have + A suppressed processed HTML response must be `private, no-store` and have `Surrogate-Control` and `Fastly-Surrogate-Control` removed. Do not alter cache headers when the response is not processed HTML, because this feature does not alter that body. @@ -183,9 +182,11 @@ cargo test-fastly datadome::protection cargo test-fastly datadome::protection_scope ``` -**Acceptance:** only the four IP decision reasons add the private marker and -produce the augmented informational skip log; all other exclusion and fail-open -paths keep their current tag behavior. +**Acceptance:** typed scope metadata adds the private marker whenever an +applicable IP exclusion matches, including overlap with an earlier primary skip +reason. Navigation suppression uses the augmented informational log, +subresources use debug, and non-overlapping/fail-open paths keep their current +tag behavior. --- @@ -327,7 +328,7 @@ optimization. with a processable HTML content type, suppression `true`, and cacheable origin headers (`Cache-Control`, `Surrogate-Control`, and `Fastly-Surrogate-Control`). Assert the stream response is: - - `Cache-Control: private, max-age=0`; and + - `Cache-Control: private, no-store`; and - missing both surrogate cache headers. - [ ] **Step 2: Apply privacy only in the `ResponseRoute::Stream` HTML arm.** @@ -349,9 +350,10 @@ optimization. cargo test-fastly publisher ``` -**Acceptance:** a shared cache cannot replay an IP-excluded client's tagless -HTML to a non-excluded visitor, while unchanged responses retain their existing -cacheability. +**Acceptance:** newly synthesized tagless HTML is private and non-storable, +while unchanged responses retain their existing cacheability. A shared cache in +front of Trusted Server that already holds tag-bearing HTML must be bypassed or +purged for guaranteed suppression. --- diff --git a/docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md b/docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md index d6d811780..0bc7a856d 100644 --- a/docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md +++ b/docs/superpowers/specs/2026-08-03-datadome-ip-excluded-client-tag-design.md @@ -112,16 +112,11 @@ found in publisher HTML. That behavior must remain unchanged. ### 1. Capture an IP-exclusion marker at the request filter The request filter must attach a typed, internal request-scoped marker when the -existing protection-scope evaluation returns a skip for one of these reasons: - -- `client_ip` -- `client_ip_source` -- `ip_cidr` -- `ip_cidr_source` - -The marker must be attached only after the existing scope decision confirms the -IP exclusion. It must not be inferred from request headers or recomputed later -in the HTML pipeline. +existing protection-scope evaluation reports `suppress_client_tag` metadata. +The scope preserves the current first-match rule ID and typed reason for +logging, while independently checking applicable IP exclusions when an earlier +method, ASN, path, or query rule already matched. The marker must not be +inferred from request headers or recomputed later in the HTML pipeline. The request-filter API currently exposes an immutable request view. Add the smallest internal mechanism needed for a filter to attach a typed request @@ -134,19 +129,17 @@ suppressed; the existing skip log supplies the rule ID and reason. The marker must not be attached for: -- `OPTIONS` or other excluded methods before scope evaluation; - internal or integration routes; -- ASN exclusions; -- path, query, or other non-IP structured exclusions; -- unmatched IP rules; +- method, ASN, path, or query exclusions without an overlapping IP match; +- unmatched or method-inapplicable IP rules; - Protection API fail-open behavior; or - requests where `enable_protection` is false and the request filter does not run. ### 2. Enrich the existing skip log -For IP-based skips, extend the existing informational log with -`client_tag=omitted`: +For navigation skips that suppress the tag, extend the existing informational +log with `client_tag=omitted` (subresource suppression skips use `debug`): ```text [datadome] protection decision=skipped rule=excluded-ip-cidrs reason=client_ip client_tag=omitted method=GET @@ -184,8 +177,11 @@ responses, which should retain their current processing. A processed HTML response differs by client IP when the generated tag is suppressed. In the `PublisherResponse::Stream` path, when suppression is active -and the response is HTML, set `Cache-Control: private, max-age=0` and remove -`Surrogate-Control` and `Fastly-Surrogate-Control` before the body is streamed. +and the response is HTML, use the shared synthesized-HTML policy: +`Cache-Control: private, no-store`, no origin validators, and no CDN-targeted +cache headers. This response-time policy cannot invalidate tag-bearing HTML +already held by a fronting shared cache; guaranteed suppression requires +bypassing or purging that cache. This matches the existing per-user ad-stack cache policy. It prevents Fastly or another shared cache from replaying a tag-suppressed response to a visitor whose