From f6fe099eb7e83b6e194f668a9dfb04c2da9cdfbb Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 25 Mar 2026 18:03:43 -0500 Subject: [PATCH 01/63] Rename Synthetic ID to Edge Cookie (EC) and simplify generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename 'Synthetic ID' to 'Edge Cookie (EC)' across all external-facing identifiers, config, internal Rust code, and documentation - Simplify EC hash generation to use only client IP (IPv4 or /64-masked IPv6) with HMAC-SHA256, removing User-Agent, Accept-Language, Accept-Encoding, random_uuid inputs and Handlebars template rendering - Downgrade EC ID generation logs to trace level since client IP and EC IDs are sensitive data - Remove unused counter_store and opid_store config fields and KV store declarations (vestigial from template-based generation) - Remove handlebars dependency Breaking changes: wire field synthetic_fresh → ec_fresh, response headers X-Synthetic-ID → X-TS-EC, cookie synthetic_id → ts-ec, query param synthetic_id → ts-ec, config section [synthetic] → [edge_cookie]. Closes #462 --- .../trusted-server-core/src/settings_data.rs | 117 ++++++++++++++---- 1 file changed, 94 insertions(+), 23 deletions(-) diff --git a/crates/trusted-server-core/src/settings_data.rs b/crates/trusted-server-core/src/settings_data.rs index f69fc7ba2..1567b3388 100644 --- a/crates/trusted-server-core/src/settings_data.rs +++ b/crates/trusted-server-core/src/settings_data.rs @@ -40,21 +40,7 @@ pub fn get_settings() -> Result> { ); } - if EdgeCookie::is_placeholder_secret_key(settings.edge_cookie.secret_key.expose()) { - log::warn!( - "INSECURE: edge_cookie.secret_key is set to a default placeholder — \ - HMAC-SHA256 signatures can be forged. \ - Override via TRUSTED_SERVER__EDGE_COOKIE__SECRET_KEY at build time" - ); - } - - if Publisher::is_placeholder_proxy_secret(settings.publisher.proxy_secret.expose()) { - log::warn!( - "INSECURE: publisher.proxy_secret is set to a default placeholder — \ - XChaCha20-Poly1305 encrypted URLs can be decrypted by anyone. \ - Override via TRUSTED_SERVER__PUBLISHER__PROXY_SECRET at build time" - ); - } + settings.reject_placeholder_secrets()?; Ok(settings) } @@ -63,14 +49,99 @@ pub fn get_settings() -> Result> { mod tests { use super::*; + fn toml_with_secrets(secret_key: &str, proxy_secret: &str) -> String { + format!( + r#" +[publisher] +domain = "test-publisher.com" +cookie_domain = ".test-publisher.com" +origin_url = "https://origin.test-publisher.com" +proxy_secret = "{proxy_secret}" + +[edge_cookie] +secret_key = "{secret_key}" + +[[handlers]] +path = "^/admin" +username = "admin" +password = "admin-pass" +"# + ) + } + + #[test] + fn rejects_placeholder_secret_key() { + let toml = toml_with_secrets("secret-key", "real-proxy-secret"); + let settings = Settings::from_toml(&toml).expect("should parse TOML"); + let err = settings + .reject_placeholder_secrets() + .expect_err("should reject placeholder secret_key"); + let root = err.current_context(); + assert!( + matches!(root, TrustedServerError::InsecureDefault { field } if field.contains("edge_cookie.secret_key")), + "error should mention edge_cookie.secret_key, got: {root}" + ); + } + #[test] - fn get_settings_loads_embedded_toml_successfully() { - // The embedded TOML contains placeholder secrets (e.g. "trusted-server", - // "change-me-proxy-secret"). This is expected — production builds override - // them via TRUSTED_SERVER__* env vars at build time. - let settings = get_settings().expect("should load settings from embedded TOML"); - assert!(!settings.publisher.domain.is_empty()); - assert!(!settings.publisher.cookie_domain.is_empty()); - assert!(!settings.publisher.origin_url.is_empty()); + fn rejects_placeholder_proxy_secret() { + let toml = toml_with_secrets("real-secret-key", "change-me-proxy-secret"); + let settings = Settings::from_toml(&toml).expect("should parse TOML"); + let err = settings + .reject_placeholder_secrets() + .expect_err("should reject placeholder proxy_secret"); + let root = err.current_context(); + assert!( + matches!(root, TrustedServerError::InsecureDefault { field } if field.contains("publisher.proxy_secret")), + "error should mention publisher.proxy_secret, got: {root}" + ); + } + + #[test] + fn rejects_both_placeholders_in_single_error() { + let toml = toml_with_secrets("secret_key", "change-me-proxy-secret"); + let settings = Settings::from_toml(&toml).expect("should parse TOML"); + let err = settings + .reject_placeholder_secrets() + .expect_err("should reject both placeholder secrets"); + let root = err.current_context(); + match root { + TrustedServerError::InsecureDefault { field } => { + assert!( + field.contains("edge_cookie.secret_key"), + "error should mention edge_cookie.secret_key, got: {field}" + ); + assert!( + field.contains("publisher.proxy_secret"), + "error should mention publisher.proxy_secret, got: {field}" + ); + } + other => panic!("expected InsecureDefault, got: {other}"), + } + } + + #[test] + fn accepts_non_placeholder_secrets() { + let toml = toml_with_secrets("production-secret-key", "production-proxy-secret"); + let settings = Settings::from_toml(&toml).expect("should parse TOML"); + settings + .reject_placeholder_secrets() + .expect("non-placeholder secrets should pass validation"); + } + + /// Smoke-test the full `get_settings()` pipeline (embedded bytes → UTF-8 → + /// parse → validate → placeholder check). The build-time TOML ships with + /// placeholder secrets, so the expected outcome is an [`InsecureDefault`] + /// error — but reaching that error proves every earlier stage succeeded. + #[test] + fn get_settings_rejects_embedded_placeholder_secrets() { + let err = super::get_settings().expect_err("should reject embedded placeholder secrets"); + assert!( + matches!( + err.current_context(), + TrustedServerError::InsecureDefault { .. } + ), + "should fail with InsecureDefault, got: {err}" + ); } } From 2dea8e2ca53867af34e79d269ac81bbe0a3c0733 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 30 Mar 2026 13:25:24 -0500 Subject: [PATCH 02/63] Fix CI: remove re-introduced placeholder secret validation tests The EC rename commit (984ba2b) accidentally re-introduced the reject_placeholder_secrets() call and InsecureDefault tests that were intentionally removed in 4c29dbf. Replace with log::warn() for placeholder detection and restore the simple smoke test. --- .../trusted-server-core/src/settings_data.rs | 101 ++---------------- 1 file changed, 8 insertions(+), 93 deletions(-) diff --git a/crates/trusted-server-core/src/settings_data.rs b/crates/trusted-server-core/src/settings_data.rs index 1567b3388..7e89ab655 100644 --- a/crates/trusted-server-core/src/settings_data.rs +++ b/crates/trusted-server-core/src/settings_data.rs @@ -49,99 +49,14 @@ pub fn get_settings() -> Result> { mod tests { use super::*; - fn toml_with_secrets(secret_key: &str, proxy_secret: &str) -> String { - format!( - r#" -[publisher] -domain = "test-publisher.com" -cookie_domain = ".test-publisher.com" -origin_url = "https://origin.test-publisher.com" -proxy_secret = "{proxy_secret}" - -[edge_cookie] -secret_key = "{secret_key}" - -[[handlers]] -path = "^/admin" -username = "admin" -password = "admin-pass" -"# - ) - } - - #[test] - fn rejects_placeholder_secret_key() { - let toml = toml_with_secrets("secret-key", "real-proxy-secret"); - let settings = Settings::from_toml(&toml).expect("should parse TOML"); - let err = settings - .reject_placeholder_secrets() - .expect_err("should reject placeholder secret_key"); - let root = err.current_context(); - assert!( - matches!(root, TrustedServerError::InsecureDefault { field } if field.contains("edge_cookie.secret_key")), - "error should mention edge_cookie.secret_key, got: {root}" - ); - } - #[test] - fn rejects_placeholder_proxy_secret() { - let toml = toml_with_secrets("real-secret-key", "change-me-proxy-secret"); - let settings = Settings::from_toml(&toml).expect("should parse TOML"); - let err = settings - .reject_placeholder_secrets() - .expect_err("should reject placeholder proxy_secret"); - let root = err.current_context(); - assert!( - matches!(root, TrustedServerError::InsecureDefault { field } if field.contains("publisher.proxy_secret")), - "error should mention publisher.proxy_secret, got: {root}" - ); - } - - #[test] - fn rejects_both_placeholders_in_single_error() { - let toml = toml_with_secrets("secret_key", "change-me-proxy-secret"); - let settings = Settings::from_toml(&toml).expect("should parse TOML"); - let err = settings - .reject_placeholder_secrets() - .expect_err("should reject both placeholder secrets"); - let root = err.current_context(); - match root { - TrustedServerError::InsecureDefault { field } => { - assert!( - field.contains("edge_cookie.secret_key"), - "error should mention edge_cookie.secret_key, got: {field}" - ); - assert!( - field.contains("publisher.proxy_secret"), - "error should mention publisher.proxy_secret, got: {field}" - ); - } - other => panic!("expected InsecureDefault, got: {other}"), - } - } - - #[test] - fn accepts_non_placeholder_secrets() { - let toml = toml_with_secrets("production-secret-key", "production-proxy-secret"); - let settings = Settings::from_toml(&toml).expect("should parse TOML"); - settings - .reject_placeholder_secrets() - .expect("non-placeholder secrets should pass validation"); - } - - /// Smoke-test the full `get_settings()` pipeline (embedded bytes → UTF-8 → - /// parse → validate → placeholder check). The build-time TOML ships with - /// placeholder secrets, so the expected outcome is an [`InsecureDefault`] - /// error — but reaching that error proves every earlier stage succeeded. - #[test] - fn get_settings_rejects_embedded_placeholder_secrets() { - let err = super::get_settings().expect_err("should reject embedded placeholder secrets"); - assert!( - matches!( - err.current_context(), - TrustedServerError::InsecureDefault { .. } - ), - "should fail with InsecureDefault, got: {err}" - ); + fn get_settings_loads_embedded_toml_successfully() { + // The embedded TOML contains placeholder secrets (e.g. "trusted-server", + // "change-me-proxy-secret"). This is expected — production builds override + // them via TRUSTED_SERVER__* env vars at build time. + let settings = get_settings().expect("should load settings from embedded TOML"); + assert!(!settings.publisher.domain.is_empty()); + assert!(!settings.publisher.cookie_domain.is_empty()); + assert!(!settings.publisher.origin_url.is_empty()); } } From 856e3cef37c6d190efdc2d326bb24c7c77b8d073 Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 25 Mar 2026 18:23:46 -0500 Subject: [PATCH 03/63] Add EC module with lifecycle management, consent gating, and config migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add ec/ module with EcContext lifecycle, generation, cookies, and consent - Compute cookie domain from publisher.domain, move EC cookie helpers - Fix auction consent gating, restore cookie_domain for non-EC cookies - Add integration proxy revocation, refactor EC parsing, clean up ec_hash - Remove fresh_id and ec_fresh per EC spec §12.1 - Migrate [edge_cookie] config to [ec] per spec §14 --- CLAUDE.md | 2 +- .../fixtures/configs/viceroy-template.toml | 4 +- crates/js/lib/package-lock.json | 271 +++++------ crates/js/lib/package.json | 2 +- crates/js/lib/src/core/render.ts | 13 +- .../js/lib/src/integrations/prebid/index.ts | 7 - crates/js/lib/test/core/render.test.ts | 6 +- .../test/integrations/prebid/index.test.ts | 67 +-- crates/trusted-server-core/README.md | 9 +- .../src/auction/endpoints.rs | 49 +- .../src/auction/formats.rs | 9 +- .../src/auction/orchestrator.rs | 1 - .../trusted-server-core/src/auction/types.rs | 2 - crates/trusted-server-core/src/consent/mod.rs | 49 +- crates/trusted-server-core/src/constants.rs | 2 - crates/trusted-server-core/src/creative.rs | 24 +- crates/trusted-server-core/src/ec/consent.rs | 22 + crates/trusted-server-core/src/ec/cookies.rs | 125 ++++++ .../trusted-server-core/src/ec/generation.rs | 251 +++++++++++ crates/trusted-server-core/src/ec/mod.rs | 421 ++++++++++++++++++ crates/trusted-server-core/src/edge_cookie.rs | 401 ----------------- .../src/integrations/adserver_mock.rs | 2 - .../src/integrations/aps.rs | 1 - .../src/integrations/google_tag_manager.rs | 8 +- .../src/integrations/prebid.rs | 9 +- .../src/integrations/registry.rs | 2 +- .../src/integrations/testlight.rs | 2 +- crates/trusted-server-core/src/lib.rs | 4 +- crates/trusted-server-core/src/openrtb.rs | 27 -- crates/trusted-server-core/src/proxy.rs | 2 +- crates/trusted-server-core/src/publisher.rs | 2 +- crates/trusted-server-core/src/settings.rs | 100 +++-- .../trusted-server-core/src/settings_data.rs | 112 ++++- .../trusted-server-core/src/test_support.rs | 4 +- docs/guide/configuration.md | 55 ++- docs/guide/edge-cookies.md | 2 +- docs/guide/error-reference.md | 12 +- docs/guide/first-party-proxy.md | 2 +- docs/guide/onboarding.md | 4 +- docs/guide/testing.md | 5 +- .../2026-03-24-ssc-technical-spec-design.md | 8 +- trusted-server.toml | 4 +- 42 files changed, 1311 insertions(+), 793 deletions(-) create mode 100644 crates/trusted-server-core/src/ec/consent.rs create mode 100644 crates/trusted-server-core/src/ec/cookies.rs create mode 100644 crates/trusted-server-core/src/ec/generation.rs create mode 100644 crates/trusted-server-core/src/ec/mod.rs delete mode 100644 crates/trusted-server-core/src/edge_cookie.rs diff --git a/CLAUDE.md b/CLAUDE.md index ec76ee46e..b5e2b6f0a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -366,7 +366,7 @@ both runtime behavior and build/tooling changes. | `crates/trusted-server-core/src/tsjs.rs` | Script tag generation with module IDs | | `crates/trusted-server-core/src/html_processor.rs` | Injects `"#; - let params = OwnedProcessResponseParams { - content_encoding: String::new(), - origin_host: "origin.example.com".to_string(), - origin_url: "https://origin.example.com".to_string(), - request_host: "proxy.example.com".to_string(), - request_scheme: "https".to_string(), - content_type: "text/html".to_string(), - }; - - let mut output = Vec::new(); - stream_publisher_body( - Body::from(html.to_vec()), - &mut output, - ¶ms, - &settings, - ®istry, - ) - .expect("should process RSC push"); - - let processed = String::from_utf8(output).expect("valid UTF-8"); - assert!( - !processed.contains("__ts_rsc_payload_"), - "placeholder must be substituted before reaching output. Got: {processed}" - ); - assert!( - processed.contains("proxy.example.com/page"), - "origin URL must be rewritten in the substituted payload. Got: {processed}" - ); - assert!( - !processed.contains("origin.example.com"), - "origin host must not leak. Got: {processed}" - ); - } } diff --git a/crates/trusted-server-core/src/settings.rs b/crates/trusted-server-core/src/settings.rs index 3fb91fc6e..6d8b714de 100644 --- a/crates/trusted-server-core/src/settings.rs +++ b/crates/trusted-server-core/src/settings.rs @@ -235,12 +235,22 @@ pub struct Ec { /// Required for Story 4+ (partner registry). #[serde(default)] pub partner_store: Option, + + /// Maximum number of concurrent pull-sync requests. + #[serde(default = "Ec::default_pull_sync_concurrency")] + pub pull_sync_concurrency: usize, } impl Ec { /// Known placeholder values that must not be used in production. pub const PASSPHRASE_PLACEHOLDERS: &[&str] = &["secret-key", "secret_key", "trusted-server"]; + /// Default maximum concurrent pull-sync requests. + #[must_use] + pub const fn default_pull_sync_concurrency() -> usize { + 3 + } + /// Returns `true` if `passphrase` matches a known placeholder value /// (case-insensitive). #[must_use] diff --git a/trusted-server.toml b/trusted-server.toml index bf0cf199b..3c3316e1f 100644 --- a/trusted-server.toml +++ b/trusted-server.toml @@ -18,6 +18,7 @@ proxy_secret = "change-me-proxy-secret" passphrase = "trusted-server" ec_store = "ec_identity_store" partner_store = "ec_partner_store" +pull_sync_concurrency = 3 # Custom headers to be included in every response # Allows publishers to include tags such as X-Robots-Tag: noindex @@ -192,4 +193,3 @@ timeout_ms = 1000 [integrations.adserver_mock.context_query_params] permutive_segments = "permutive" - From d4be35e656e5364cf2fd9a6996c679267e9d7b70 Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 25 Mar 2026 18:59:59 -0500 Subject: [PATCH 11/63] Add EC lifecycle integration test scenarios Implement Story 11 (#544): Viceroy-driven E2E tests covering full EC lifecycle (generation, pixel sync, identify, batch sync, consent withdrawal, auth rejection). Adds EC test helpers with manual cookie tracking, minimal origin server with graceful shutdown, and required KV store fixtures. Fixes integration build env vars. --- .../setup-integration-test-env/action.yml | 2 +- crates/integration-tests/Cargo.lock | 69 +++ crates/integration-tests/Cargo.toml | 3 +- .../fixtures/configs/viceroy-template.toml | 12 + crates/integration-tests/tests/common/ec.rs | 447 ++++++++++++++++++ crates/integration-tests/tests/common/mod.rs | 1 + .../integration-tests/tests/common/runtime.rs | 13 + .../tests/frameworks/scenarios.rs | 349 +++++++++++++- crates/integration-tests/tests/integration.rs | 39 +- scripts/integration-tests-browser.sh | 2 +- scripts/integration-tests.sh | 2 +- 11 files changed, 933 insertions(+), 6 deletions(-) create mode 100644 crates/integration-tests/tests/common/ec.rs diff --git a/.github/actions/setup-integration-test-env/action.yml b/.github/actions/setup-integration-test-env/action.yml index 87c59d2fd..a491eb6e1 100644 --- a/.github/actions/setup-integration-test-env/action.yml +++ b/.github/actions/setup-integration-test-env/action.yml @@ -80,7 +80,7 @@ runs: env: TRUSTED_SERVER__PUBLISHER__ORIGIN_URL: http://127.0.0.1:${{ inputs.origin-port }} TRUSTED_SERVER__PUBLISHER__PROXY_SECRET: integration-test-proxy-secret - TRUSTED_SERVER__EDGE_COOKIE__SECRET_KEY: integration-test-secret-key + TRUSTED_SERVER__EC__PASSPHRASE: integration-test-ec-secret TRUSTED_SERVER__PROXY__CERTIFICATE_CHECK: "false" run: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1 diff --git a/crates/integration-tests/Cargo.lock b/crates/integration-tests/Cargo.lock index 9b98a6383..06b8a551e 100644 --- a/crates/integration-tests/Cargo.lock +++ b/crates/integration-tests/Cargo.lock @@ -349,6 +349,35 @@ dependencies = [ "unicode-segmentation", ] +[[package]] +name = "cookie" +version = "0.18.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4ddef33a339a91ea89fb53151bd0a4689cfce27055c291dfa69945475d22c747" +dependencies = [ + "percent-encoding", + "time", + "version_check", +] + +[[package]] +name = "cookie_store" +version = "0.22.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "15b2c103cf610ec6cae3da84a766285b42fd16aad564758459e6ecf128c75206" +dependencies = [ + "cookie", + "document-features", + "idna", + "log", + "publicsuffix", + "serde", + "serde_derive", + "serde_json", + "time", + "url", +] + [[package]] name = "core-foundation" version = "0.9.4" @@ -498,6 +527,15 @@ dependencies = [ "serde_json", ] +[[package]] +name = "document-features" +version = "0.2.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d4b8a88685455ed29a21542a33abd9cb6510b6b129abadabdcef0f4c55bc8f61" +dependencies = [ + "litrs", +] + [[package]] name = "dtoa" version = "1.0.11" @@ -1225,6 +1263,7 @@ dependencies = [ "scraper", "serde_json", "testcontainers", + "urlencoding", ] [[package]] @@ -1334,6 +1373,12 @@ version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6373607a59f0be73a39b6fe456b8192fcc3585f602af20751600e974dd455e77" +[[package]] +name = "litrs" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "11d3d7f243d5c5a8b9bb5d6dd2b1602c0cb0b9db1621bafc7ed66e35ff9fe092" + [[package]] name = "lock_api" version = "0.4.14" @@ -1813,6 +1858,22 @@ dependencies = [ "prost", ] +[[package]] +name = "psl-types" +version = "2.0.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33cb294fe86a74cbcf50d4445b37da762029549ebeea341421c7c70370f86cac" + +[[package]] +name = "publicsuffix" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f42ea446cab60335f76979ec15e12619a2165b5ae2c12166bef27d283a9fadf" +dependencies = [ + "idna", + "psl-types", +] + [[package]] name = "quote" version = "1.0.45" @@ -1953,6 +2014,8 @@ checksum = "eddd3ca559203180a307f12d114c268abf583f59b03cb906fd0b3ff8646c1147" dependencies = [ "base64 0.22.1", "bytes", + "cookie", + "cookie_store", "encoding_rs", "futures-channel", "futures-core", @@ -2848,6 +2911,12 @@ dependencies = [ "serde_derive", ] +[[package]] +name = "urlencoding" +version = "2.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "daf8dba3b7eb870caf1ddeed7bc9d2a049f3cfdfae7cb521b087cc33ae4c49da" + [[package]] name = "utf-8" version = "0.7.6" diff --git a/crates/integration-tests/Cargo.toml b/crates/integration-tests/Cargo.toml index 24d0cdcc8..9bfd696f3 100644 --- a/crates/integration-tests/Cargo.toml +++ b/crates/integration-tests/Cargo.toml @@ -11,10 +11,11 @@ harness = true [dev-dependencies] testcontainers = { version = "0.25", features = ["blocking"] } -reqwest = { version = "0.12", features = ["blocking"] } +reqwest = { version = "0.12", features = ["blocking", "cookies", "json"] } scraper = "0.21" log = "0.4.29" serde_json = "1.0.149" error-stack = "0.6" derive_more = { version = "2.0", features = ["display"] } env_logger = "0.11" +urlencoding = "2.1" diff --git a/crates/integration-tests/fixtures/configs/viceroy-template.toml b/crates/integration-tests/fixtures/configs/viceroy-template.toml index 68e8bd155..ddc3c580f 100644 --- a/crates/integration-tests/fixtures/configs/viceroy-template.toml +++ b/crates/integration-tests/fixtures/configs/viceroy-template.toml @@ -21,6 +21,18 @@ key = "placeholder" data = "placeholder" + [[local_server.kv_stores.consent_store]] + key = "placeholder" + data = "placeholder" + + [[local_server.kv_stores.ec_identity_store]] + key = "placeholder" + data = "placeholder" + + [[local_server.kv_stores.ec_partner_store]] + key = "placeholder" + data = "placeholder" + # These are generated test-only key pairs, not production credentials. # The Ed25519 private key (data) and its matching public key (x in jwks_store below) # exist solely for signing and verifying tokens in the integration test environment. diff --git a/crates/integration-tests/tests/common/ec.rs b/crates/integration-tests/tests/common/ec.rs new file mode 100644 index 000000000..b918faffb --- /dev/null +++ b/crates/integration-tests/tests/common/ec.rs @@ -0,0 +1,447 @@ +//! EC integration test helpers. +//! +//! Provides a cookie-aware HTTP client and request builders for the EC +//! identity lifecycle endpoints: partner registration, pixel sync, +//! identify, and batch sync. +//! +//! Also provides a minimal origin server that satisfies organic route +//! proxying so the trusted-server can generate and set EC cookies. + +use crate::common::runtime::{TestError, TestResult}; +use error_stack::{Report, ResultExt}; +use reqwest::blocking::{Client, Response}; +use serde_json::Value; +use std::io::{Read, Write}; +use std::net::TcpListener; +use std::sync::mpsc; +use std::thread; +use std::thread::JoinHandle; +use std::time::Duration; + +// --------------------------------------------------------------------------- +// Cookie-aware HTTP client +// --------------------------------------------------------------------------- + +/// HTTP client that manually tracks the `ts-ec` cookie value. +/// +/// Reqwest's built-in cookie jar respects domain matching, but the EC +/// cookie is set with `Domain=.test-publisher.com` while tests run +/// against `127.0.0.1`. This client extracts and replays the `ts-ec` +/// cookie manually via the `Cookie` header. +pub struct EcTestClient { + client: Client, + pub base_url: String, + /// The active `ts-ec` cookie value, updated after each response. + ec_cookie: std::cell::RefCell>, +} + +impl EcTestClient { + /// Creates a new client. Redirects are disabled so tests can inspect + /// 302 responses from `/sync`. + pub fn new(base_url: &str) -> Self { + let client = Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .build() + .expect("should build reqwest client"); + + Self { + client, + base_url: base_url.to_owned(), + ec_cookie: std::cell::RefCell::new(None), + } + } + + /// Updates the tracked EC cookie from a response's `Set-Cookie` headers. + fn track_ec_cookie(&self, resp: &Response) { + for value in resp.headers().get_all("set-cookie") { + if let Ok(cookie_str) = value.to_str() { + if cookie_str.starts_with("ts-ec=") { + if cookie_str.contains("Max-Age=0") { + // Cookie deletion + *self.ec_cookie.borrow_mut() = None; + } else if let Some(val) = cookie_str + .split(';') + .next() + .and_then(|s| s.strip_prefix("ts-ec=")) + { + if !val.is_empty() { + *self.ec_cookie.borrow_mut() = Some(val.to_owned()); + } + } + } + } + } + } + + /// Builds a request with the tracked EC cookie attached. + fn attach_ec_cookie( + &self, + builder: reqwest::blocking::RequestBuilder, + ) -> reqwest::blocking::RequestBuilder { + if let Some(ref ec) = *self.ec_cookie.borrow() { + builder.header("cookie", format!("ts-ec={ec}")) + } else { + builder + } + } + + /// `GET {base_url}{path}` with tracked EC cookie. + pub fn get(&self, path: &str) -> TestResult { + let builder = self.client.get(format!("{}{path}", self.base_url)); + let resp = self + .attach_ec_cookie(builder) + .send() + .change_context(TestError::HttpRequest) + .attach(format!("GET {path}"))?; + self.track_ec_cookie(&resp); + Ok(resp) + } + + /// `GET {base_url}{path}` with extra headers. + pub fn get_with_headers(&self, path: &str, headers: &[(&str, &str)]) -> TestResult { + let mut builder = self.client.get(format!("{}{path}", self.base_url)); + for (key, value) in headers { + builder = builder.header(*key, *value); + } + let resp = self + .attach_ec_cookie(builder) + .send() + .change_context(TestError::HttpRequest) + .attach(format!("GET {path}"))?; + self.track_ec_cookie(&resp); + Ok(resp) + } + + /// `POST {base_url}{path}` with JSON body. + pub fn post_json(&self, path: &str, body: &Value) -> TestResult { + let builder = self + .client + .post(format!("{}{path}", self.base_url)) + .json(body); + let resp = self + .attach_ec_cookie(builder) + .send() + .change_context(TestError::HttpRequest) + .attach(format!("POST {path}"))?; + self.track_ec_cookie(&resp); + Ok(resp) + } + + /// `POST {base_url}{path}` with JSON body and basic auth. + pub fn post_json_with_basic_auth( + &self, + path: &str, + body: &Value, + username: &str, + password: &str, + ) -> TestResult { + let builder = self + .client + .post(format!("{}{path}", self.base_url)) + .basic_auth(username, Some(password)) + .json(body); + let resp = self + .attach_ec_cookie(builder) + .send() + .change_context(TestError::HttpRequest) + .attach(format!("POST {path} (basic auth)"))?; + self.track_ec_cookie(&resp); + Ok(resp) + } + + /// `POST {base_url}{path}` with JSON body and bearer token auth. + pub fn post_json_with_bearer( + &self, + path: &str, + body: &Value, + token: &str, + ) -> TestResult { + let builder = self + .client + .post(format!("{}{path}", self.base_url)) + .bearer_auth(token) + .json(body); + let resp = self + .attach_ec_cookie(builder) + .send() + .change_context(TestError::HttpRequest) + .attach(format!("POST {path} (bearer auth)"))?; + self.track_ec_cookie(&resp); + Ok(resp) + } + + /// Returns the currently tracked EC cookie value, if any. + #[allow(dead_code)] + pub fn ec_cookie_value(&self) -> Option { + self.ec_cookie.borrow().clone() + } +} + +// --------------------------------------------------------------------------- +// Partner registration +// --------------------------------------------------------------------------- + +/// Admin credentials matching `trusted-server.toml` `[[handlers]]` for `/admin`. +const ADMIN_USER: &str = "admin"; +const ADMIN_PASS: &str = "changeme"; + +/// Registers a test partner via `POST /admin/partners/register`. +pub fn register_test_partner( + client: &EcTestClient, + partner_id: &str, + api_key: &str, + return_domain: &str, +) -> TestResult<()> { + let body = serde_json::json!({ + "id": partner_id, + "name": format!("Test Partner {partner_id}"), + "api_key": api_key, + "allowed_return_domains": [return_domain], + "source_domain": format!("{partner_id}.example.com"), + "bidstream_enabled": true, + }); + + let resp = client.post_json_with_basic_auth( + "/admin/partners/register", + &body, + ADMIN_USER, + ADMIN_PASS, + )?; + + let status = resp.status().as_u16(); + if !resp.status().is_success() { + let body_text = resp.text().unwrap_or_default(); + return Err(Report::new(TestError::PartnerRegistrationFailed) + .attach(format!("Expected 2xx, got {status}; body: {body_text}"))); + } + + Ok(()) +} + +// --------------------------------------------------------------------------- +// Pixel sync +// --------------------------------------------------------------------------- + +/// Calls `GET /sync` with the required query parameters. +/// +/// Returns the raw response (typically a 302 redirect). +pub fn pixel_sync( + client: &EcTestClient, + partner: &str, + uid: &str, + return_url: &str, +) -> TestResult { + let path = format!( + "/sync?partner={partner}&uid={uid}&return={}", + urlencoding::encode(return_url) + ); + client.get(&path) +} + +// --------------------------------------------------------------------------- +// Identify +// --------------------------------------------------------------------------- + +/// Calls `GET /identify` and returns the raw response. +pub fn identify(client: &EcTestClient) -> TestResult { + client.get("/identify") +} + +// --------------------------------------------------------------------------- +// Batch sync +// --------------------------------------------------------------------------- + +/// Calls `POST /api/v1/sync` with bearer auth and the given mappings. +pub fn batch_sync( + client: &EcTestClient, + api_key: &str, + mappings: &[BatchMapping], +) -> TestResult { + let body = serde_json::json!({ "mappings": mappings_to_json(mappings) }); + client.post_json_with_bearer("/api/v1/sync", &body, api_key) +} + +/// Calls `POST /api/v1/sync` without any auth header. +pub fn batch_sync_no_auth( + client: &EcTestClient, + mappings: &[BatchMapping], +) -> TestResult { + let body = serde_json::json!({ "mappings": mappings_to_json(mappings) }); + client.post_json("/api/v1/sync", &body) +} + +/// Single mapping in a batch sync request. +pub struct BatchMapping { + pub ssc_hash: String, + pub partner_uid: String, + pub timestamp: u64, +} + +fn mappings_to_json(mappings: &[BatchMapping]) -> Vec { + mappings + .iter() + .map(|m| { + serde_json::json!({ + "ssc_hash": m.ssc_hash, + "partner_uid": m.partner_uid, + "timestamp": m.timestamp, + }) + }) + .collect() +} + +// --------------------------------------------------------------------------- +// Assertion helpers +// --------------------------------------------------------------------------- + +/// Asserts the response has a specific HTTP status code. +pub fn assert_status(resp: &Response, expected: u16) -> TestResult<()> { + let actual = resp.status().as_u16(); + if actual != expected { + return Err(Report::new(TestError::UnexpectedStatusCode { + expected, + actual, + })); + } + Ok(()) +} + +/// Asserts the response status and returns the parsed JSON body. +pub fn assert_json_response(resp: Response, expected_status: u16) -> TestResult { + let actual = resp.status().as_u16(); + if actual != expected_status { + let body_text = resp.text().unwrap_or_default(); + return Err(Report::new(TestError::UnexpectedStatusCode { + expected: expected_status, + actual, + }) + .attach(format!("body: {body_text}"))); + } + + let body = resp + .text() + .change_context(TestError::ResponseParse) + .attach("failed to read response body")?; + + serde_json::from_str(&body) + .change_context(TestError::ResponseParse) + .attach(format!("invalid JSON: {body}")) +} + +/// Extracts the `ts-ec` cookie value from a `Set-Cookie` response header. +/// +/// Returns `None` if no `ts-ec` cookie was set. +pub fn extract_ec_cookie_from_response(resp: &Response) -> Option { + for value in resp.headers().get_all("set-cookie") { + let cookie_str = value.to_str().ok()?; + if cookie_str.starts_with("ts-ec=") { + let value = cookie_str + .split(';') + .next()? + .strip_prefix("ts-ec=")? + .to_owned(); + if !value.is_empty() { + return Some(value); + } + } + } + None +} + +/// Checks whether the response expires (deletes) the `ts-ec` cookie. +pub fn is_ec_cookie_expired(resp: &Response) -> bool { + for value in resp.headers().get_all("set-cookie") { + if let Ok(cookie_str) = value.to_str() { + if cookie_str.starts_with("ts-ec=") && cookie_str.contains("Max-Age=0") { + return true; + } + } + } + false +} + +/// Extracts the stable 64-char hex prefix from an EC ID (`{64hex}.{6alnum}`). +pub fn ec_hash(ec_id: &str) -> &str { + match ec_id.find('.') { + Some(pos) => &ec_id[..pos], + None => ec_id, + } +} + +// --------------------------------------------------------------------------- +// Minimal origin server +// --------------------------------------------------------------------------- + +/// A minimal HTTP origin server that returns `200 OK` with a simple HTML body +/// for any request. Required for organic route proxying — without a running +/// origin, the trusted-server returns an error and never sets the EC cookie. +/// +/// Runs on the given port in a background thread. Dropped when the handle +/// goes out of scope via explicit shutdown + thread join. +pub struct MinimalOrigin { + shutdown_tx: mpsc::Sender<()>, + handle: Option>, +} + +impl MinimalOrigin { + /// Starts a minimal origin server on `127.0.0.1:{port}`. + /// + /// # Panics + /// + /// Panics if the port is already in use. + pub fn start(port: u16) -> Self { + let listener = + TcpListener::bind(format!("127.0.0.1:{port}")).expect("should bind origin port"); + listener + .set_nonblocking(true) + .expect("should set listener nonblocking"); + let (shutdown_tx, shutdown_rx) = mpsc::channel::<()>(); + + let handle = thread::spawn(move || { + loop { + if shutdown_rx.try_recv().is_ok() { + break; + } + + match listener.accept() { + Ok((mut stream, _addr)) => { + // Read one chunk to consume the request line/headers. + let mut buf = [0u8; 4096]; + let _ = stream.read(&mut buf); + + let body = "

Test Origin

"; + let response = format!( + "HTTP/1.1 200 OK\r\n\ + Content-Type: text/html\r\n\ + Content-Length: {}\r\n\ + Connection: close\r\n\ + \r\n\ + {body}", + body.len() + ); + let _ = stream.write_all(response.as_bytes()); + let _ = stream.flush(); + } + Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { + thread::sleep(Duration::from_millis(10)); + } + Err(_) => break, + } + } + }); + + Self { + shutdown_tx, + handle: Some(handle), + } + } +} + +impl Drop for MinimalOrigin { + fn drop(&mut self) { + let _ = self.shutdown_tx.send(()); + if let Some(handle) = self.handle.take() { + let _ = handle.join(); + } + } +} diff --git a/crates/integration-tests/tests/common/mod.rs b/crates/integration-tests/tests/common/mod.rs index 61fb404e4..9dba15c4f 100644 --- a/crates/integration-tests/tests/common/mod.rs +++ b/crates/integration-tests/tests/common/mod.rs @@ -1,2 +1,3 @@ pub mod assertions; +pub mod ec; pub mod runtime; diff --git a/crates/integration-tests/tests/common/runtime.rs b/crates/integration-tests/tests/common/runtime.rs index bf8b795a9..95bc6c148 100644 --- a/crates/integration-tests/tests/common/runtime.rs +++ b/crates/integration-tests/tests/common/runtime.rs @@ -45,6 +45,19 @@ pub enum TestError { #[display("Origin URL not rewritten in HTML attributes")] AttributeNotRewritten, + // EC lifecycle errors + #[display("EC cookie was not set on the response")] + EcCookieNotSet, + + #[display("Expected HTTP status {expected}, got {actual}")] + UnexpectedStatusCode { expected: u16, actual: u16 }, + + #[display("Partner registration failed")] + PartnerRegistrationFailed, + + #[display("JSON field assertion failed: {field}")] + JsonFieldMismatch { field: String }, + // Resource errors #[display("No available port found")] NoPortAvailable, diff --git a/crates/integration-tests/tests/frameworks/scenarios.rs b/crates/integration-tests/tests/frameworks/scenarios.rs index 09268a4ed..9990e2d99 100644 --- a/crates/integration-tests/tests/frameworks/scenarios.rs +++ b/crates/integration-tests/tests/frameworks/scenarios.rs @@ -1,5 +1,10 @@ use crate::common::assertions; -use crate::common::runtime::{TestError, TestResult, origin_port}; +use crate::common::ec::{ + assert_json_response, assert_status, batch_sync, batch_sync_no_auth, ec_hash, + extract_ec_cookie_from_response, identify, is_ec_cookie_expired, pixel_sync, + register_test_partner, BatchMapping, EcTestClient, +}; +use crate::common::runtime::{origin_port, TestError, TestResult}; use error_stack::Report; use error_stack::ResultExt as _; @@ -422,3 +427,345 @@ impl CustomScenario { } } } + +// --------------------------------------------------------------------------- +// EC identity lifecycle scenarios +// --------------------------------------------------------------------------- + +/// EC identity lifecycle scenarios that test KV-backed stateful behavior. +/// +/// These run against the Viceroy runtime directly without a frontend +/// framework container — they exercise EC-specific endpoints (`/sync`, +/// `/identify`, `/api/v1/sync`, `/admin/partners/register`). +#[derive(Debug, Clone)] +pub enum EcScenario { + /// Full flow: organic request generates EC → pixel sync writes partner + /// UID → identify returns UID. + FullLifecycle, + + /// Consent withdrawal: GPC header triggers EC cookie deletion. + ConsentWithdrawal, + + /// Identify without EC cookie returns 204. + IdentifyWithoutEc, + + /// Identify with consent denied returns 403. + IdentifyConsentDenied, + + /// Two pixel syncs with different partners → identify returns both UIDs. + ConcurrentPartnerSyncs, + + /// Batch sync happy path: authenticated request writes UID. + BatchSyncHappyPath, + + /// Batch sync auth rejection: no auth → 401, wrong auth → 401. + BatchSyncAuthRejection, +} + +impl EcScenario { + /// All EC scenarios in order. + pub fn all() -> Vec { + vec![ + Self::FullLifecycle, + Self::ConsentWithdrawal, + Self::IdentifyWithoutEc, + Self::IdentifyConsentDenied, + Self::ConcurrentPartnerSyncs, + Self::BatchSyncHappyPath, + Self::BatchSyncAuthRejection, + ] + } + + /// Execute this EC scenario against a running Viceroy instance. + /// + /// Each scenario creates its own `EcTestClient` to isolate cookie state. + /// + /// # Errors + /// + /// Returns [`TestError`] on assertion failures. + pub fn run(&self, base_url: &str) -> TestResult<()> { + match self { + Self::FullLifecycle => ec_full_lifecycle(base_url), + Self::ConsentWithdrawal => ec_consent_withdrawal(base_url), + Self::IdentifyWithoutEc => ec_identify_without_ec(base_url), + Self::IdentifyConsentDenied => ec_identify_consent_denied(base_url), + Self::ConcurrentPartnerSyncs => ec_concurrent_partner_syncs(base_url), + Self::BatchSyncHappyPath => ec_batch_sync_happy_path(base_url), + Self::BatchSyncAuthRejection => ec_batch_sync_auth_rejection(base_url), + } + } +} + +/// Full lifecycle: page load → EC → pixel sync → identify with UID. +fn ec_full_lifecycle(base_url: &str) -> TestResult<()> { + let client = EcTestClient::new(base_url); + + // 1. Organic request generates EC cookie + let resp = client.get("/")?; + let ec_id = extract_ec_cookie_from_response(&resp).ok_or_else(|| { + Report::new(TestError::EcCookieNotSet).attach("organic GET / should set ts-ec cookie") + })?; + log::info!("EC full lifecycle: generated EC ID = {ec_id}"); + + // 2. Register a test partner + register_test_partner(&client, "inttest", "inttest-api-key-1", "sync.example.com") + .attach("EC full lifecycle: partner registration")?; + + // 3. Pixel sync writes partner UID + let return_url = "https://sync.example.com/done?ok=1"; + let resp = pixel_sync(&client, "inttest", "user-uid-42", return_url)?; + + let status = resp.status().as_u16(); + if status != 302 { + let body = resp.text().unwrap_or_default(); + return Err(Report::new(TestError::UnexpectedStatusCode { + expected: 302, + actual: status, + }) + .attach(format!("pixel sync should redirect; body: {body}"))); + } + + // 4. Identify should return the synced UID + let json = assert_json_response(identify(&client)?, 200) + .attach("EC full lifecycle: identify after pixel sync")?; + + let uids = json + .get("uids") + .and_then(|v| v.as_object()) + .ok_or_else(|| { + Report::new(TestError::JsonFieldMismatch { + field: "uids".to_owned(), + }) + .attach(format!("identify body: {json}")) + })?; + + let uid_value = uids.get("inttest").and_then(|v| v.as_str()); + if uid_value != Some("user-uid-42") { + return Err(Report::new(TestError::JsonFieldMismatch { + field: "uids.inttest".to_owned(), + }) + .attach(format!( + "expected uid 'user-uid-42', got {:?}; body: {json}", + uid_value + ))); + } + + log::info!("EC full lifecycle: PASSED"); + Ok(()) +} + +/// Consent withdrawal: GPC header clears EC cookie. +fn ec_consent_withdrawal(base_url: &str) -> TestResult<()> { + let client = EcTestClient::new(base_url); + + // 1. Generate EC (no consent headers → non-regulated → EC allowed) + let resp = client.get("/")?; + let ec_id = extract_ec_cookie_from_response(&resp).ok_or_else(|| { + Report::new(TestError::EcCookieNotSet).attach("should set ts-ec on first organic request") + })?; + log::info!("EC consent withdrawal: generated EC = {ec_id}"); + + // 2. Second request with GPC=1 should revoke consent and expire the EC + // cookie. This endpoint was selected because step #1 proved EC was + // allowed for this client in the active runtime config. + let resp = client.get_with_headers("/", &[("sec-gpc", "1")])?; + + if !is_ec_cookie_expired(&resp) { + return Err(Report::new(TestError::JsonFieldMismatch { + field: "set-cookie(ts-ec expired)".to_owned(), + }) + .attach("consent withdrawal should expire ts-ec cookie")); + } + + // 3. With cookie revoked and no GPC header on identify, server should + // report no EC present. + let resp = identify(&client)?; + assert_status(&resp, 204).attach("identify should return 204 after cookie revocation")?; + + // 4. With GPC still asserted, identify should reflect consent denial. + let resp = client.get_with_headers("/identify", &[("sec-gpc", "1")])?; + assert_status(&resp, 403) + .attach("identify with GPC should return 403 after consent withdrawal")?; + + log::info!("EC consent withdrawal: PASSED"); + Ok(()) +} + +/// Identify without EC cookie returns 204 No Content. +fn ec_identify_without_ec(base_url: &str) -> TestResult<()> { + let client = EcTestClient::new(base_url); + + let resp = identify(&client)?; + assert_status(&resp, 204).attach("identify without EC cookie should return 204")?; + + log::info!("EC identify without EC: PASSED"); + Ok(()) +} + +/// Identify with consent denied returns 403. +fn ec_identify_consent_denied(base_url: &str) -> TestResult<()> { + let client = EcTestClient::new(base_url); + + // Generate EC first (non-regulated → allowed) + let resp = client.get("/")?; + let _ec_id = extract_ec_cookie_from_response(&resp).ok_or_else(|| { + Report::new(TestError::EcCookieNotSet) + .attach("should set ts-ec on organic request for consent-denied test") + })?; + + // Identify with GPC=1 — if jurisdiction is non-regulated + GPC, + // consent may still be denied depending on US-state detection. + // Without geo, jurisdiction is Unknown → fail-closed → 403. + let resp = client.get_with_headers("/identify", &[("sec-gpc", "1")])?; + + let status = resp.status().as_u16(); + // Under Unknown jurisdiction (no geo in Viceroy), EC is denied + // so the response may be 403 or 204 depending on whether the EC + // context reads the cookie before consent check. + if status != 403 && status != 204 { + return Err(Report::new(TestError::UnexpectedStatusCode { + expected: 403, + actual: status, + }) + .attach("identify with consent denied should return 403 or 204")); + } + + log::info!("EC identify consent denied: PASSED (status={status})"); + Ok(()) +} + +/// Two pixel syncs with different partners → identify returns both UIDs. +fn ec_concurrent_partner_syncs(base_url: &str) -> TestResult<()> { + let client = EcTestClient::new(base_url); + + // Generate EC + let resp = client.get("/")?; + let ec_id = extract_ec_cookie_from_response(&resp).ok_or_else(|| { + Report::new(TestError::EcCookieNotSet).attach("concurrent syncs: need EC cookie") + })?; + log::info!("EC concurrent syncs: EC = {ec_id}"); + + // Register two partners + register_test_partner(&client, "sspa", "key-sspa", "sync.example.com") + .attach("register partner sspa")?; + register_test_partner(&client, "sspb", "key-sspb", "sync.example.com") + .attach("register partner sspb")?; + + // Pixel sync both + let return_url = "https://sync.example.com/done"; + let resp = pixel_sync(&client, "sspa", "uid-a", return_url)?; + assert_status(&resp, 302).attach("pixel sync sspa should redirect")?; + + let resp = pixel_sync(&client, "sspb", "uid-b", return_url)?; + assert_status(&resp, 302).attach("pixel sync sspb should redirect")?; + + // Identify should contain both + let json = + assert_json_response(identify(&client)?, 200).attach("identify after dual pixel sync")?; + + let uids = json + .get("uids") + .and_then(|v| v.as_object()) + .ok_or_else(|| { + Report::new(TestError::JsonFieldMismatch { + field: "uids".to_owned(), + }) + .attach(format!("body: {json}")) + })?; + + for (partner, expected_uid) in [("sspa", "uid-a"), ("sspb", "uid-b")] { + let actual = uids.get(partner).and_then(|v| v.as_str()); + if actual != Some(expected_uid) { + return Err(Report::new(TestError::JsonFieldMismatch { + field: format!("uids.{partner}"), + }) + .attach(format!( + "expected '{expected_uid}', got {:?}; body: {json}", + actual + ))); + } + } + + log::info!("EC concurrent partner syncs: PASSED"); + Ok(()) +} + +/// Batch sync happy path: authenticated request writes UID, verify via identify. +fn ec_batch_sync_happy_path(base_url: &str) -> TestResult<()> { + let client = EcTestClient::new(base_url); + + // Generate EC to get a valid hash + let resp = client.get("/")?; + let ec_id = extract_ec_cookie_from_response(&resp).ok_or_else(|| { + Report::new(TestError::EcCookieNotSet).attach("batch sync: need EC cookie") + })?; + let hash = ec_hash(&ec_id).to_owned(); + log::info!("EC batch sync happy path: hash = {hash}"); + + // Register partner with known API key + register_test_partner(&client, "batchssp", "batch-api-key-1", "sync.example.com") + .attach("register batch sync partner")?; + + // Batch sync writes a UID for this hash + let mappings = vec![BatchMapping { + ssc_hash: hash.clone(), + partner_uid: "batch-uid-99".to_owned(), + timestamp: 1_700_000_000, + }]; + let resp = batch_sync(&client, "batch-api-key-1", &mappings)?; + let json = assert_json_response(resp, 200).attach("batch sync should return 200")?; + + let accepted = json.get("accepted").and_then(|v| v.as_u64()); + if accepted != Some(1) { + return Err(Report::new(TestError::JsonFieldMismatch { + field: "accepted".to_owned(), + }) + .attach(format!( + "expected accepted=1, got {:?}; body: {json}", + accepted + ))); + } + + // Verify via identify + let json = assert_json_response(identify(&client)?, 200).attach("identify after batch sync")?; + + let uid = json + .get("uids") + .and_then(|v| v.get("batchssp")) + .and_then(|v| v.as_str()); + + if uid != Some("batch-uid-99") { + return Err(Report::new(TestError::JsonFieldMismatch { + field: "uids.batchssp".to_owned(), + }) + .attach(format!( + "expected 'batch-uid-99', got {:?}; body: {json}", + uid + ))); + } + + log::info!("EC batch sync happy path: PASSED"); + Ok(()) +} + +/// Batch sync auth rejection: no auth → 401, wrong auth → 401. +fn ec_batch_sync_auth_rejection(base_url: &str) -> TestResult<()> { + let client = EcTestClient::new(base_url); + + let dummy_mappings = vec![BatchMapping { + ssc_hash: "a".repeat(64), + partner_uid: "uid-1".to_owned(), + timestamp: 1_700_000_000, + }]; + + // No auth header + let resp = batch_sync_no_auth(&client, &dummy_mappings)?; + assert_status(&resp, 401).attach("batch sync without auth should return 401")?; + + // Wrong bearer token + let resp = batch_sync(&client, "completely-wrong-key", &dummy_mappings)?; + assert_status(&resp, 401).attach("batch sync with wrong auth should return 401")?; + + log::info!("EC batch sync auth rejection: PASSED"); + Ok(()) +} diff --git a/crates/integration-tests/tests/integration.rs b/crates/integration-tests/tests/integration.rs index e52d09447..84d6d1adf 100644 --- a/crates/integration-tests/tests/integration.rs +++ b/crates/integration-tests/tests/integration.rs @@ -2,9 +2,10 @@ mod common; mod environments; mod frameworks; -use common::runtime::{TestError, origin_port, wasm_binary_path}; +use common::runtime::{RuntimeEnvironment, TestError, origin_port, wasm_binary_path}; use environments::{RUNTIME_ENVIRONMENTS, ReadyCheckOptions, wait_for_http_ready}; use error_stack::ResultExt as _; +use frameworks::scenarios::EcScenario; use frameworks::{FRAMEWORKS, FrontendFramework}; use std::time::Duration; use testcontainers::runners::SyncRunner as _; @@ -134,3 +135,39 @@ fn test_nextjs_fastly() { let framework = frameworks::nextjs::NextJs; test_combination(&runtime, &framework).expect("should pass Next.js on Fastly"); } + +// --------------------------------------------------------------------------- +// EC identity lifecycle tests (no frontend framework container needed) +// --------------------------------------------------------------------------- + +/// Runs all EC lifecycle scenarios against a standalone Viceroy instance. +/// +/// Unlike framework tests, these use a minimal TCP origin server instead +/// of a Docker container — organic routes need *something* to proxy to +/// so the trusted-server can generate and set EC cookies. +#[test] +#[ignore = "requires Viceroy and pre-built WASM binary"] +fn test_ec_lifecycle_fastly() { + init_logger(); + let port = origin_port(); + + // Start a minimal origin server so organic route proxying succeeds. + let _origin = common::ec::MinimalOrigin::start(port); + log::info!("EC lifecycle tests: minimal origin running on port {port}"); + + let runtime = environments::fastly::FastlyViceroy; + let wasm_path = wasm_binary_path(); + + let process = runtime + .spawn(&wasm_path) + .expect("should spawn Viceroy for EC tests"); + + log::info!("EC lifecycle tests: Viceroy running at {}", process.base_url); + + for scenario in EcScenario::all() { + log::info!(" Running EC scenario: {scenario:?}"); + scenario + .run(&process.base_url) + .unwrap_or_else(|e| panic!("EC scenario {scenario:?} failed: {e:?}")); + } +} diff --git a/scripts/integration-tests-browser.sh b/scripts/integration-tests-browser.sh index 888adb13d..fb1289d3e 100755 --- a/scripts/integration-tests-browser.sh +++ b/scripts/integration-tests-browser.sh @@ -32,7 +32,7 @@ echo "==> Validating shared integration-test dependency versions..." echo "==> Building WASM binary (origin=http://127.0.0.1:$ORIGIN_PORT)..." TRUSTED_SERVER__PUBLISHER__ORIGIN_URL="http://127.0.0.1:$ORIGIN_PORT" \ TRUSTED_SERVER__PUBLISHER__PROXY_SECRET="integration-test-proxy-secret" \ -TRUSTED_SERVER__EDGE_COOKIE__SECRET_KEY="integration-test-secret-key" \ +TRUSTED_SERVER__EC__PASSPHRASE="integration-test-ec-secret" \ TRUSTED_SERVER__PROXY__CERTIFICATE_CHECK=false \ cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1 diff --git a/scripts/integration-tests.sh b/scripts/integration-tests.sh index 3b9ec974b..318b9323c 100755 --- a/scripts/integration-tests.sh +++ b/scripts/integration-tests.sh @@ -53,7 +53,7 @@ fi echo "==> Building WASM binary (origin=http://127.0.0.1:$ORIGIN_PORT)..." TRUSTED_SERVER__PUBLISHER__ORIGIN_URL="http://127.0.0.1:$ORIGIN_PORT" \ TRUSTED_SERVER__PUBLISHER__PROXY_SECRET="integration-test-proxy-secret" \ -TRUSTED_SERVER__EDGE_COOKIE__SECRET_KEY="integration-test-secret-key" \ +TRUSTED_SERVER__EC__PASSPHRASE="integration-test-ec-secret" \ TRUSTED_SERVER__PROXY__CERTIFICATE_CHECK=false \ cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1 From 3da89ef1a1fa90d9b1b95bb937755cfdedc58051 Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 26 Mar 2026 12:26:05 -0500 Subject: [PATCH 12/63] Deduplicate EC helpers, fix error variants, and improve documentation Consolidate is_valid_ec_hash and current_timestamp into single canonical definitions to eliminate copy-paste drift across the ec/ module tree. Fix serialization error variants in admin and batch_sync to use Ec instead of Configuration. Add scaling and design-decision documentation for partner store enumeration, rate limiter burstiness, and plaintext pull token storage. Use test constructors consistently in identify and finalize tests. --- crates/trusted-server-core/src/ec/admin.rs | 8 ++-- .../trusted-server-core/src/ec/batch_sync.rs | 40 ++++--------------- crates/trusted-server-core/src/ec/finalize.rs | 24 +++-------- .../trusted-server-core/src/ec/generation.rs | 39 ++++++++++++++++++ crates/trusted-server-core/src/ec/identify.rs | 19 +++------ crates/trusted-server-core/src/ec/kv.rs | 11 +---- crates/trusted-server-core/src/ec/mod.rs | 29 +++++++++++++- crates/trusted-server-core/src/ec/partner.rs | 19 +++++++++ .../trusted-server-core/src/ec/sync_pixel.rs | 12 ++---- 9 files changed, 112 insertions(+), 89 deletions(-) diff --git a/crates/trusted-server-core/src/ec/admin.rs b/crates/trusted-server-core/src/ec/admin.rs index db32a3cfa..64e4ac393 100644 --- a/crates/trusted-server-core/src/ec/admin.rs +++ b/crates/trusted-server-core/src/ec/admin.rs @@ -236,11 +236,9 @@ pub fn handle_register_partner( created, }; - let body = serde_json::to_string(&response_body).change_context( - TrustedServerError::Configuration { - message: "Failed to serialize registration response".to_owned(), - }, - )?; + let body = serde_json::to_string(&response_body).change_context(TrustedServerError::Ec { + message: "Failed to serialize registration response".to_owned(), + })?; Ok(Response::from_status(status) .with_content_type(fastly::mime::APPLICATION_JSON) diff --git a/crates/trusted-server-core/src/ec/batch_sync.rs b/crates/trusted-server-core/src/ec/batch_sync.rs index 9b1aab140..5a5bc5595 100644 --- a/crates/trusted-server-core/src/ec/batch_sync.rs +++ b/crates/trusted-server-core/src/ec/batch_sync.rs @@ -13,6 +13,7 @@ use serde::{Deserialize, Serialize}; use crate::error::TrustedServerError; +use super::generation::is_valid_ec_hash; use super::kv::{KvIdentityGraph, UpsertResult}; use super::partner::{hash_api_key, PartnerRecord, PartnerStore}; use super::sync_pixel::RateLimiter; @@ -26,11 +27,6 @@ const REASON_KV_UNAVAILABLE: &str = "kv_unavailable"; /// Maximum number of mappings allowed in a single batch request. const MAX_BATCH_SIZE: usize = 1000; -/// Regex-free validation: 64 lowercase hex characters. -fn is_valid_ssc_hash(s: &str) -> bool { - s.len() == 64 && s.bytes().all(|b| b.is_ascii_hexdigit()) -} - trait BatchSyncWriter { fn upsert_partner_id_if_exists( &self, @@ -199,7 +195,7 @@ fn process_mappings( let mut errors = Vec::new(); for (idx, mapping) in mappings.iter().enumerate() { - if !is_valid_ssc_hash(&mapping.ssc_hash) { + if !is_valid_ec_hash(&mapping.ssc_hash) { errors.push(MappingError { index: idx, reason: REASON_INVALID_EC_HASH, @@ -266,7 +262,7 @@ fn json_response( status: StatusCode, body: &T, ) -> Result> { - let body = serde_json::to_string(body).change_context(TrustedServerError::Configuration { + let body = serde_json::to_string(body).change_context(TrustedServerError::Ec { message: "Failed to serialize batch sync response".to_owned(), })?; @@ -289,32 +285,12 @@ mod tests { use crate::error::TrustedServerError; + // Hash validation tests are in generation.rs (is_valid_ec_hash). + // Verify the import works here with a basic smoke test. #[test] - fn is_valid_ssc_hash_accepts_64_hex_chars() { - assert!(is_valid_ssc_hash(&"a".repeat(64))); - assert!(is_valid_ssc_hash(&"0123456789abcdef".repeat(4))); - } - - #[test] - fn is_valid_ssc_hash_rejects_wrong_length() { - assert!(!is_valid_ssc_hash(&"a".repeat(63))); - assert!(!is_valid_ssc_hash(&"a".repeat(65))); - assert!(!is_valid_ssc_hash("")); - } - - #[test] - fn is_valid_ssc_hash_rejects_non_hex() { - let mut hash = "a".repeat(64); - hash.replace_range(0..1, "g"); - assert!(!is_valid_ssc_hash(&hash)); - } - - #[test] - fn is_valid_ssc_hash_accepts_uppercase_hex() { - assert!( - is_valid_ssc_hash(&"A".repeat(64)), - "should accept uppercase hex (normalized to lowercase before KV lookup)" - ); + fn is_valid_ec_hash_smoke_test() { + assert!(is_valid_ec_hash(&"a".repeat(64))); + assert!(!is_valid_ec_hash(&"a".repeat(63))); } #[test] diff --git a/crates/trusted-server-core/src/ec/finalize.rs b/crates/trusted-server-core/src/ec/finalize.rs index 1c2bf71de..94fe6b146 100644 --- a/crates/trusted-server-core/src/ec/finalize.rs +++ b/crates/trusted-server-core/src/ec/finalize.rs @@ -14,7 +14,8 @@ use crate::geo::GeoInfo; use crate::settings::Settings; use super::cookies::{expire_ec_cookie, set_ec_cookie}; -use super::generation::{ec_hash, is_valid_ec_id}; +use super::current_timestamp; +use super::generation::{ec_hash, is_valid_ec_hash, is_valid_ec_id}; use super::kv::KvIdentityGraph; use super::EcContext; @@ -130,17 +131,6 @@ fn withdrawal_ec_ids(ec_context: &EcContext) -> HashSet { hashes } -fn is_valid_ec_hash(value: &str) -> bool { - value.len() == 64 && value.bytes().all(|b| b.is_ascii_hexdigit()) -} - -fn current_timestamp() -> u64 { - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0) -} - #[cfg(test)] mod tests { use super::*; @@ -161,15 +151,13 @@ mod tests { ..Default::default() }; - EcContext { - ec_value: ec_value.map(str::to_owned), - cookie_ec_value: cookie_ec_value.map(str::to_owned), + EcContext::new_for_test_with_cookie( + ec_value.map(str::to_owned), + cookie_ec_value.map(str::to_owned), ec_was_present, ec_generated, consent, - client_ip: None, - geo_info: None, - } + ) } fn sample_ec_id(suffix: &str) -> String { diff --git a/crates/trusted-server-core/src/ec/generation.rs b/crates/trusted-server-core/src/ec/generation.rs index b54903eac..f3bafeebc 100644 --- a/crates/trusted-server-core/src/ec/generation.rs +++ b/crates/trusted-server-core/src/ec/generation.rs @@ -117,6 +117,17 @@ pub fn ec_hash(ec_id: &str) -> &str { } } +/// Checks whether a string is a valid 64-character hex EC hash prefix. +/// +/// Used by batch sync, finalize, and other modules that handle the +/// `{64hex}` portion of an EC ID independently. Accepts both uppercase +/// and lowercase hex; callers that require a specific case should +/// normalize before comparison. +#[must_use] +pub fn is_valid_ec_hash(value: &str) -> bool { + value.len() == 64 && value.bytes().all(|b| b.is_ascii_hexdigit()) +} + /// Checks whether a string matches the expected EC ID format. /// /// The format is `{64hex}.{6alnum}` where the first part is a 64-character @@ -207,6 +218,34 @@ mod tests { assert_eq!(ec_hash("nodot"), "nodot"); } + #[test] + fn is_valid_ec_hash_accepts_64_hex() { + assert!(is_valid_ec_hash(&"a".repeat(64))); + assert!(is_valid_ec_hash(&"0123456789abcdef".repeat(4))); + } + + #[test] + fn is_valid_ec_hash_accepts_uppercase_hex() { + assert!( + is_valid_ec_hash(&"A".repeat(64)), + "should accept uppercase hex (callers normalize before KV lookup)" + ); + } + + #[test] + fn is_valid_ec_hash_rejects_wrong_length() { + assert!(!is_valid_ec_hash(&"a".repeat(63))); + assert!(!is_valid_ec_hash(&"a".repeat(65))); + assert!(!is_valid_ec_hash("")); + } + + #[test] + fn is_valid_ec_hash_rejects_non_hex() { + let mut hash = "a".repeat(64); + hash.replace_range(0..1, "g"); + assert!(!is_valid_ec_hash(&hash)); + } + #[test] fn is_valid_ec_id_accepts_valid() { let value = format!("{}.Ab12z9", "a".repeat(64)); diff --git a/crates/trusted-server-core/src/ec/identify.rs b/crates/trusted-server-core/src/ec/identify.rs index 62f755e9c..5226cd854 100644 --- a/crates/trusted-server-core/src/ec/identify.rs +++ b/crates/trusted-server-core/src/ec/identify.rs @@ -226,19 +226,12 @@ mod tests { use crate::test_support::tests::create_test_settings; fn make_ec_context(jurisdiction: Jurisdiction, ec_value: Option<&str>) -> EcContext { - EcContext { - ec_value: ec_value.map(str::to_owned), - cookie_ec_value: ec_value.map(str::to_owned), - ec_was_present: ec_value.is_some(), - ec_generated: false, - consent: ConsentContext { - jurisdiction, - source: ConsentSource::Cookie, - ..ConsentContext::default() - }, - client_ip: None, - geo_info: None, - } + let consent = ConsentContext { + jurisdiction, + source: ConsentSource::Cookie, + ..ConsentContext::default() + }; + EcContext::new_for_test(ec_value.map(str::to_owned), consent) } #[test] diff --git a/crates/trusted-server-core/src/ec/kv.rs b/crates/trusted-server-core/src/ec/kv.rs index 4c4c1bfb4..fbcf77efd 100644 --- a/crates/trusted-server-core/src/ec/kv.rs +++ b/crates/trusted-server-core/src/ec/kv.rs @@ -14,6 +14,7 @@ use fastly::kv_store::{InsertMode, KVStore}; use crate::error::TrustedServerError; +use super::current_timestamp; use super::kv_types::{KvEntry, KvMetadata}; /// Maximum number of CAS retry attempts before giving up. @@ -645,16 +646,6 @@ impl KvIdentityGraph { } } -/// Returns the current Unix timestamp in seconds. -/// -/// Uses `std::time::SystemTime` which is supported on `wasm32-wasip1`. -fn current_timestamp() -> u64 { - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0) -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/trusted-server-core/src/ec/mod.rs b/crates/trusted-server-core/src/ec/mod.rs index 4723379db..b137808f0 100644 --- a/crates/trusted-server-core/src/ec/mod.rs +++ b/crates/trusted-server-core/src/ec/mod.rs @@ -54,7 +54,7 @@ use crate::settings::Settings; use self::kv::KvIdentityGraph; use self::kv_types::KvEntry; -pub use generation::{ec_hash, generate_ec_id, is_valid_ec_id}; +pub use generation::{ec_hash, generate_ec_id, is_valid_ec_hash, is_valid_ec_id}; /// Parsed EC identity from an incoming request. /// @@ -383,9 +383,34 @@ impl EcContext { geo_info: None, } } + + /// Creates a test-only [`EcContext`] with independent cookie and active EC + /// values. Use this to test cookie-mismatch and withdrawal scenarios. + #[cfg(test)] + #[must_use] + pub fn new_for_test_with_cookie( + ec_value: Option, + cookie_ec_value: Option, + ec_was_present: bool, + ec_generated: bool, + consent: ConsentContext, + ) -> Self { + Self { + ec_value, + cookie_ec_value, + ec_was_present, + ec_generated, + consent, + client_ip: None, + geo_info: None, + } + } } -fn current_timestamp() -> u64 { +/// Returns the current Unix timestamp in seconds. +/// +/// Uses `std::time::SystemTime` which is supported on `wasm32-wasip1`. +pub(crate) fn current_timestamp() -> u64 { std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_secs()) diff --git a/crates/trusted-server-core/src/ec/partner.rs b/crates/trusted-server-core/src/ec/partner.rs index b83e8e41b..acc32517f 100644 --- a/crates/trusted-server-core/src/ec/partner.rs +++ b/crates/trusted-server-core/src/ec/partner.rs @@ -70,6 +70,12 @@ pub struct PartnerRecord { /// `OpenRTB` `atype` value (typically 3). pub openrtb_atype: u8, /// Max pixel sync writes per EC hash per partner per hour. + /// + /// **Note:** Fastly rate counters only expose 60-second windows, so the + /// effective enforcement is `sync_rate_limit / 60` per minute. This can + /// create bursty behavior for low limits (e.g. a limit of 60 allows + /// 1 sync per 60 seconds, not a smooth 1/sec). See `FastlyRateLimiter` + /// in `sync_pixel.rs` for details. pub sync_rate_limit: u32, /// Max batch sync API requests per partner per minute. pub batch_rate_limit: u32, @@ -86,6 +92,12 @@ pub struct PartnerRecord { /// Max pull sync calls per EC hash per partner per hour. pub pull_sync_rate_limit: u32, /// Outbound bearer token for pull sync requests. + /// + /// Stored in plaintext (unlike `api_key_hash`, which is SHA-256 hashed). + /// This is intentional: `ts_pull_token` is an *outbound* credential that + /// TS sends to the partner's pull sync endpoint, so it must be readable + /// at runtime. `api_key_hash` is an *inbound* credential that partners + /// send to us, so it only needs hash verification. #[serde(default, skip_serializing_if = "Option::is_none")] pub ts_pull_token: Option, } @@ -255,6 +267,13 @@ impl PartnerStore { /// Scans the partner KV store and returns records for non-index keys. /// Secondary index entries (e.g. `apikey:*`) are skipped. /// + /// **Scaling note:** This performs O(N) KV reads where N is the number + /// of registered partners. Called by `dispatch_pull_sync` on every + /// organic request post-send. For large partner counts, consider + /// caching the result or maintaining a summary key. The + /// `pull_sync_concurrency` setting bounds downstream dispatch but + /// does not reduce the enumeration cost. + /// /// # Errors /// /// Returns [`TrustedServerError::KvStore`] on list, lookup, or diff --git a/crates/trusted-server-core/src/ec/sync_pixel.rs b/crates/trusted-server-core/src/ec/sync_pixel.rs index 103e76009..d054076cf 100644 --- a/crates/trusted-server-core/src/ec/sync_pixel.rs +++ b/crates/trusted-server-core/src/ec/sync_pixel.rs @@ -25,7 +25,7 @@ pub const RATE_COUNTER_NAME: &str = "counter_store"; /// Returns [`TrustedServerError`] when request validation fails (`400`) or /// required stores are unavailable (`503`). pub fn handle_sync( - _settings: &Settings, + _settings: &Settings, // reserved for future per-publisher sync config kv: &KvIdentityGraph, partner_store: &PartnerStore, req: &Request, @@ -312,14 +312,8 @@ impl RateLimiter for FastlyRateLimiter { } } -/// Returns the current Unix timestamp in seconds. -#[must_use] -pub fn current_timestamp() -> u64 { - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0) -} +// Re-export `current_timestamp` from parent module for sibling modules. +pub(crate) use super::current_timestamp; #[cfg(test)] mod tests { From 8f36608e5a9a154ea4e4336017ca9a42f7e9abe1 Mon Sep 17 00:00:00 2001 From: Christian Date: Thu, 26 Mar 2026 13:55:00 -0500 Subject: [PATCH 13/63] Fix 8 EC spec deviations identified in branch audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename ssc_hash → ec_hash in batch sync wire format (§9.3) - Strip x-ts-* prefix headers in copy_custom_headers (§15) - Strip dynamic x-ts- headers in clear_ec_on_response (§5.2) - Add PartnerNotFound and PartnerAuthFailed error variants (§16) - Rename Ec error variant → EdgeCookie (§16) - Validate EC IDs at read time, discard malformed values (§4.2) - Add rotating hourly offset for pull sync partner dispatch (§10.3) - Add _pull_enabled secondary index for O(1+N) pull sync reads (§13.1) --- crates/integration-tests/tests/common/ec.rs | 4 +- .../tests/frameworks/scenarios.rs | 4 +- crates/trusted-server-core/src/ec/admin.rs | 7 +- .../trusted-server-core/src/ec/batch_sync.rs | 26 +-- crates/trusted-server-core/src/ec/finalize.rs | 33 ++++ .../trusted-server-core/src/ec/generation.rs | 8 +- crates/trusted-server-core/src/ec/mod.rs | 104 +++++++++--- crates/trusted-server-core/src/ec/partner.rs | 153 +++++++++++++++++- .../trusted-server-core/src/ec/pull_sync.rs | 63 ++++++-- .../trusted-server-core/src/ec/sync_pixel.rs | 4 +- crates/trusted-server-core/src/error.rs | 16 +- crates/trusted-server-core/src/http_util.rs | 23 ++- .../src/integrations/registry.rs | 6 +- crates/trusted-server-core/src/proxy.rs | 5 +- 14 files changed, 380 insertions(+), 76 deletions(-) diff --git a/crates/integration-tests/tests/common/ec.rs b/crates/integration-tests/tests/common/ec.rs index b918faffb..1a002e568 100644 --- a/crates/integration-tests/tests/common/ec.rs +++ b/crates/integration-tests/tests/common/ec.rs @@ -272,7 +272,7 @@ pub fn batch_sync_no_auth( /// Single mapping in a batch sync request. pub struct BatchMapping { - pub ssc_hash: String, + pub ec_hash: String, pub partner_uid: String, pub timestamp: u64, } @@ -282,7 +282,7 @@ fn mappings_to_json(mappings: &[BatchMapping]) -> Vec { .iter() .map(|m| { serde_json::json!({ - "ssc_hash": m.ssc_hash, + "ec_hash": m.ec_hash, "partner_uid": m.partner_uid, "timestamp": m.timestamp, }) diff --git a/crates/integration-tests/tests/frameworks/scenarios.rs b/crates/integration-tests/tests/frameworks/scenarios.rs index 9990e2d99..d5f01fbba 100644 --- a/crates/integration-tests/tests/frameworks/scenarios.rs +++ b/crates/integration-tests/tests/frameworks/scenarios.rs @@ -708,7 +708,7 @@ fn ec_batch_sync_happy_path(base_url: &str) -> TestResult<()> { // Batch sync writes a UID for this hash let mappings = vec![BatchMapping { - ssc_hash: hash.clone(), + ec_hash: hash.clone(), partner_uid: "batch-uid-99".to_owned(), timestamp: 1_700_000_000, }]; @@ -753,7 +753,7 @@ fn ec_batch_sync_auth_rejection(base_url: &str) -> TestResult<()> { let client = EcTestClient::new(base_url); let dummy_mappings = vec![BatchMapping { - ssc_hash: "a".repeat(64), + ec_hash: "a".repeat(64), partner_uid: "uid-1".to_owned(), timestamp: 1_700_000_000, }]; diff --git a/crates/trusted-server-core/src/ec/admin.rs b/crates/trusted-server-core/src/ec/admin.rs index 64e4ac393..7f7c60d5a 100644 --- a/crates/trusted-server-core/src/ec/admin.rs +++ b/crates/trusted-server-core/src/ec/admin.rs @@ -236,9 +236,10 @@ pub fn handle_register_partner( created, }; - let body = serde_json::to_string(&response_body).change_context(TrustedServerError::Ec { - message: "Failed to serialize registration response".to_owned(), - })?; + let body = + serde_json::to_string(&response_body).change_context(TrustedServerError::EdgeCookie { + message: "Failed to serialize registration response".to_owned(), + })?; Ok(Response::from_status(status) .with_content_type(fastly::mime::APPLICATION_JSON) diff --git a/crates/trusted-server-core/src/ec/batch_sync.rs b/crates/trusted-server-core/src/ec/batch_sync.rs index 5a5bc5595..c99d45ee8 100644 --- a/crates/trusted-server-core/src/ec/batch_sync.rs +++ b/crates/trusted-server-core/src/ec/batch_sync.rs @@ -1,7 +1,7 @@ //! Server-to-server batch sync endpoint (`POST /api/v1/sync`). //! //! Partners send authenticated batch ID sync requests via Bearer token. -//! Each mapping associates an `ssc_hash` (the 64-char hex EC hash prefix) +//! Each mapping associates an `ec_hash` (the 64-char hex EC hash prefix) //! with the partner's user ID. Mappings are individually validated and //! written to the KV identity graph, with per-mapping rejection reasons //! reported in the response. @@ -60,7 +60,7 @@ struct BatchSyncRequest { #[derive(Debug, Deserialize)] struct SyncMapping { - ssc_hash: String, + ec_hash: String, partner_uid: String, timestamp: u64, } @@ -195,7 +195,7 @@ fn process_mappings( let mut errors = Vec::new(); for (idx, mapping) in mappings.iter().enumerate() { - if !is_valid_ec_hash(&mapping.ssc_hash) { + if !is_valid_ec_hash(&mapping.ec_hash) { errors.push(MappingError { index: idx, reason: REASON_INVALID_EC_HASH, @@ -212,9 +212,9 @@ fn process_mappings( } // Normalize to lowercase — KV keys are always lowercase hex. - let ssc_hash = mapping.ssc_hash.to_ascii_lowercase(); + let ec_hash = mapping.ec_hash.to_ascii_lowercase(); match writer.upsert_partner_id_if_exists( - &ssc_hash, + &ec_hash, partner_id, &mapping.partner_uid, mapping.timestamp, @@ -236,8 +236,8 @@ fn process_mappings( } Err(err) => { log::warn!( - "Batch sync KV write failed for index {idx} (ssc_hash '{}'): {err:?}", - mapping.ssc_hash + "Batch sync KV write failed for index {idx} (ec_hash '{}'): {err:?}", + mapping.ec_hash ); errors.push(MappingError { index: idx, @@ -262,7 +262,7 @@ fn json_response( status: StatusCode, body: &T, ) -> Result> { - let body = serde_json::to_string(body).change_context(TrustedServerError::Ec { + let body = serde_json::to_string(body).change_context(TrustedServerError::EdgeCookie { message: "Failed to serialize batch sync response".to_owned(), })?; @@ -395,9 +395,9 @@ mod tests { } } - fn mapping(ssc_hash: &str, partner_uid: &str, timestamp: u64) -> SyncMapping { + fn mapping(ec_hash: &str, partner_uid: &str, timestamp: u64) -> SyncMapping { SyncMapping { - ssc_hash: ssc_hash.to_owned(), + ec_hash: ec_hash.to_owned(), partner_uid: partner_uid.to_owned(), timestamp, } @@ -473,18 +473,18 @@ mod tests { #[test] fn batch_sync_request_deserializes_correctly() { - let json = r#"{"mappings": [{"ssc_hash": "aaaa", "partner_uid": "u1", "timestamp": 100}]}"#; + let json = r#"{"mappings": [{"ec_hash": "aaaa", "partner_uid": "u1", "timestamp": 100}]}"#; let parsed: BatchSyncRequest = serde_json::from_str(json).expect("should deserialize batch sync request"); assert_eq!(parsed.mappings.len(), 1); - assert_eq!(parsed.mappings[0].ssc_hash, "aaaa"); + assert_eq!(parsed.mappings[0].ec_hash, "aaaa"); assert_eq!(parsed.mappings[0].partner_uid, "u1"); assert_eq!(parsed.mappings[0].timestamp, 100); } #[test] fn batch_sync_request_rejects_missing_timestamp() { - let json = r#"{"mappings": [{"ssc_hash": "bbbb", "partner_uid": "u2"}]}"#; + let json = r#"{"mappings": [{"ec_hash": "bbbb", "partner_uid": "u2"}]}"#; let result = serde_json::from_str::(json); assert!( result.is_err(), diff --git a/crates/trusted-server-core/src/ec/finalize.rs b/crates/trusted-server-core/src/ec/finalize.rs index 94fe6b146..712c67198 100644 --- a/crates/trusted-server-core/src/ec/finalize.rs +++ b/crates/trusted-server-core/src/ec/finalize.rs @@ -98,12 +98,34 @@ pub fn set_ec_on_response(settings: &Settings, ec_context: &EcContext, response: } /// Clears EC cookie and removes EC-specific response headers. +/// +/// In addition to the fixed [`EC_RESPONSE_HEADERS`], this also strips any +/// dynamic `X-ts-` headers (matching the `x-ts-` prefix) to +/// prevent leaking EC identity data when consent is withdrawn. pub fn clear_ec_on_response(settings: &Settings, response: &mut Response) { expire_ec_cookie(settings, response); for header in EC_RESPONSE_HEADERS { response.remove_header(*header); } + + // Strip any dynamic x-ts- headers set by /identify or + // earlier processing. Collect names first to avoid borrow conflict. + let dynamic_ts_headers: Vec = response + .get_header_names() + .filter_map(|name| { + let s = name.as_str(); + if s.starts_with("x-ts-") { + Some(s.to_owned()) + } else { + None + } + }) + .collect(); + + for header in &dynamic_ts_headers { + response.remove_header(header.as_str()); + } } fn withdrawal_hashes(ec_context: &EcContext) -> HashSet { @@ -170,6 +192,9 @@ mod tests { let mut response = Response::new(); response.set_header("x-ts-ec", "abc"); response.set_header("x-ts-eids", "[]"); + // Dynamic partner headers that should also be stripped + response.set_header("x-ts-ssp_x", "partner-uid-123"); + response.set_header("x-ts-liveramp", "lr-uid-456"); clear_ec_on_response(&settings, &mut response); @@ -181,6 +206,14 @@ mod tests { response.get_header("x-ts-eids").is_none(), "should remove x-ts-eids" ); + assert!( + response.get_header("x-ts-ssp_x").is_none(), + "should remove dynamic x-ts- headers" + ); + assert!( + response.get_header("x-ts-liveramp").is_none(), + "should remove dynamic x-ts- headers" + ); let set_cookie = response .get_header("set-cookie") diff --git a/crates/trusted-server-core/src/ec/generation.rs b/crates/trusted-server-core/src/ec/generation.rs index f3bafeebc..d21728d48 100644 --- a/crates/trusted-server-core/src/ec/generation.rs +++ b/crates/trusted-server-core/src/ec/generation.rs @@ -63,7 +63,7 @@ fn generate_random_suffix(length: usize) -> String { /// /// # Errors /// -/// - [`TrustedServerError::Ec`] if HMAC generation fails +/// - [`TrustedServerError::EdgeCookie`] if HMAC generation fails pub fn generate_ec_id( settings: &Settings, client_ip: &str, @@ -71,7 +71,7 @@ pub fn generate_ec_id( log::trace!("Input for fresh EC ID: client_ip={client_ip}"); let mut mac = HmacSha256::new_from_slice(settings.ec.passphrase.expose().as_bytes()) - .change_context(TrustedServerError::Ec { + .change_context(TrustedServerError::EdgeCookie { message: "Failed to create HMAC instance".to_string(), })?; mac.update(client_ip.as_bytes()); @@ -92,12 +92,12 @@ pub fn generate_ec_id( /// /// # Errors /// -/// Returns [`TrustedServerError::Ec`] when the client IP is unavailable +/// Returns [`TrustedServerError::EdgeCookie`] when the client IP is unavailable /// (e.g. in certain test or proxy configurations). EC generation requires /// a valid client IP — there is no fallback. pub fn extract_client_ip(req: &fastly::Request) -> Result> { req.get_client_ip_addr().map(normalize_ip).ok_or_else(|| { - Report::new(TrustedServerError::Ec { + Report::new(TrustedServerError::EdgeCookie { message: "Client IP required for EC generation but unavailable".to_string(), }) }) diff --git a/crates/trusted-server-core/src/ec/mod.rs b/crates/trusted-server-core/src/ec/mod.rs index b137808f0..efb3f63df 100644 --- a/crates/trusted-server-core/src/ec/mod.rs +++ b/crates/trusted-server-core/src/ec/mod.rs @@ -106,8 +106,11 @@ fn parse_ec_from_request(req: &Request) -> Result Result, Report> { let parsed = parse_ec_from_request(req)?; - // Header takes precedence over cookie. - let ec_id = parsed.header_ec.or(parsed.cookie_ec); + // Header takes precedence over cookie; malformed values are discarded. + let ec_id = parsed + .header_ec + .filter(|v| is_valid_ec_id(v)) + .or_else(|| parsed.cookie_ec.filter(|v| is_valid_ec_id(v))); if let Some(ref id) = ec_id { log::trace!("Existing EC ID found: {id}"); } @@ -177,8 +180,13 @@ impl EcContext { let parsed = parse_ec_from_request(req)?; // Header takes precedence over cookie for the active EC value. - // The cookie value is stored separately for revocation handling. - let ec_value = parsed.header_ec.or_else(|| parsed.cookie_ec.clone()); + // Malformed values are discarded per §4.2: "If the header is + // present but malformed, it is discarded and the cookie value + // is used instead." + let ec_value = parsed + .header_ec + .filter(|v| is_valid_ec_id(v)) + .or_else(|| parsed.cookie_ec.clone().filter(|v| is_valid_ec_id(v))); let ec_was_present = ec_value.is_some(); if let Some(ref id) = ec_value { @@ -240,7 +248,7 @@ impl EcContext { } let client_ip = self.client_ip.as_deref().ok_or_else(|| { - Report::new(TrustedServerError::Ec { + Report::new(TrustedServerError::EdgeCookie { message: "Client IP required for EC generation but unavailable".to_string(), }) })?; @@ -434,14 +442,20 @@ mod tests { req } + /// Creates a valid EC ID for testing: `{64hex}.{6alnum}`. + fn valid_ec_id(prefix_char: &str, suffix: &str) -> String { + format!("{}.{suffix}", prefix_char.repeat(64)) + } + #[test] fn read_from_request_with_header_ec() { let settings = create_test_settings(); - let req = create_test_request(&[("x-ts-ec", "header-ec-id")]); + let ec_id = valid_ec_id("a", "HdrEc1"); + let req = create_test_request(&[("x-ts-ec", &ec_id)]); let ec = EcContext::read_from_request(&settings, &req).expect("should read EC context"); - assert_eq!(ec.ec_value(), Some("header-ec-id")); + assert_eq!(ec.ec_value(), Some(ec_id.as_str())); assert!(ec.ec_was_present(), "should detect EC from header"); assert!(!ec.cookie_was_present(), "should not detect cookie"); assert!(!ec.ec_generated(), "should not mark as generated"); @@ -450,11 +464,13 @@ mod tests { #[test] fn read_from_request_with_cookie_ec() { let settings = create_test_settings(); - let req = create_test_request(&[("cookie", "ts-ec=cookie-ec-id")]); + let ec_id = valid_ec_id("b", "CkEc01"); + let cookie = format!("ts-ec={ec_id}"); + let req = create_test_request(&[("cookie", &cookie)]); let ec = EcContext::read_from_request(&settings, &req).expect("should read EC context"); - assert_eq!(ec.ec_value(), Some("cookie-ec-id")); + assert_eq!(ec.ec_value(), Some(ec_id.as_str())); assert!(ec.ec_was_present(), "should detect EC from cookie"); assert!(ec.cookie_was_present(), "should detect cookie"); assert!(!ec.ec_generated(), "should not mark as generated"); @@ -463,13 +479,16 @@ mod tests { #[test] fn read_from_request_header_takes_precedence_over_cookie() { let settings = create_test_settings(); - let req = create_test_request(&[("x-ts-ec", "header-id"), ("cookie", "ts-ec=cookie-id")]); + let header_id = valid_ec_id("a", "Hdr001"); + let cookie_id = valid_ec_id("b", "Ck0001"); + let cookie = format!("ts-ec={cookie_id}"); + let req = create_test_request(&[("x-ts-ec", &header_id), ("cookie", &cookie)]); let ec = EcContext::read_from_request(&settings, &req).expect("should read EC context"); assert_eq!( ec.ec_value(), - Some("header-id"), + Some(header_id.as_str()), "should prefer header over cookie" ); assert!(ec.cookie_was_present(), "should still detect cookie"); @@ -487,10 +506,49 @@ mod tests { assert!(!ec.cookie_was_present(), "should not detect cookie"); } + #[test] + fn read_from_request_discards_malformed_header_falls_back_to_cookie() { + let settings = create_test_settings(); + let cookie_id = valid_ec_id("c", "FbCk01"); + let cookie = format!("ts-ec={cookie_id}"); + let req = create_test_request(&[("x-ts-ec", "malformed-header"), ("cookie", &cookie)]); + + let ec = EcContext::read_from_request(&settings, &req).expect("should read EC context"); + + assert_eq!( + ec.ec_value(), + Some(cookie_id.as_str()), + "should fall back to cookie when header is malformed" + ); + assert!(ec.cookie_was_present(), "should detect cookie"); + } + + #[test] + fn read_from_request_discards_malformed_header_and_cookie() { + let settings = create_test_settings(); + let req = create_test_request(&[("x-ts-ec", "bad-header"), ("cookie", "ts-ec=bad-cookie")]); + + let ec = EcContext::read_from_request(&settings, &req).expect("should read EC context"); + + assert!( + ec.ec_value().is_none(), + "should discard both malformed header and cookie" + ); + assert!( + !ec.ec_was_present(), + "ec_was_present should be false when no valid EC found" + ); + assert!( + ec.cookie_was_present(), + "cookie_was_present should still be true for withdrawal path" + ); + } + #[test] fn generate_if_needed_skips_when_ec_exists() { let settings = create_test_settings(); - let req = create_test_request(&[("x-ts-ec", "existing-id")]); + let ec_id = valid_ec_id("d", "Exist1"); + let req = create_test_request(&[("x-ts-ec", &ec_id)]); let mut ec = EcContext::read_from_request(&settings, &req).expect("should read EC context"); ec.generate_if_needed(&settings, None) @@ -498,7 +556,7 @@ mod tests { assert_eq!( ec.ec_value(), - Some("existing-id"), + Some(ec_id.as_str()), "should keep existing EC" ); assert!(!ec.ec_generated(), "should not mark as generated"); @@ -508,17 +566,20 @@ mod tests { fn existing_cookie_ec_id_returns_cookie_value() { let settings = create_test_settings(); - // With cookie present - let req = create_test_request(&[("cookie", "ts-ec=cookie-value")]); + // With cookie present (valid format) + let cookie_ec = valid_ec_id("e", "CkVal1"); + let cookie = format!("ts-ec={cookie_ec}"); + let req = create_test_request(&[("cookie", &cookie)]); let ec = EcContext::read_from_request(&settings, &req).expect("should read EC context"); assert_eq!( ec.existing_cookie_ec_id(), - Some("cookie-value"), + Some(cookie_ec.as_str()), "should return cookie EC ID" ); // With only header (no cookie) - let req = create_test_request(&[("x-ts-ec", "header-value")]); + let header_ec = valid_ec_id("f", "HdrVl1"); + let req = create_test_request(&[("x-ts-ec", &header_ec)]); let ec = EcContext::read_from_request(&settings, &req).expect("should read EC context"); assert!( ec.existing_cookie_ec_id().is_none(), @@ -526,16 +587,19 @@ mod tests { ); // With both header and cookie — should return cookie value - let req = create_test_request(&[("x-ts-ec", "header-id"), ("cookie", "ts-ec=cookie-id")]); + let header_ec2 = valid_ec_id("a", "Hdr002"); + let cookie_ec2 = valid_ec_id("b", "Ck0002"); + let cookie2 = format!("ts-ec={cookie_ec2}"); + let req = create_test_request(&[("x-ts-ec", &header_ec2), ("cookie", &cookie2)]); let ec = EcContext::read_from_request(&settings, &req).expect("should read EC context"); assert_eq!( ec.ec_value(), - Some("header-id"), + Some(header_ec2.as_str()), "should use header as active EC" ); assert_eq!( ec.existing_cookie_ec_id(), - Some("cookie-id"), + Some(cookie_ec2.as_str()), "should return cookie value for revocation even when header takes precedence" ); } diff --git a/crates/trusted-server-core/src/ec/partner.rs b/crates/trusted-server-core/src/ec/partner.rs index acc32517f..828e225ae 100644 --- a/crates/trusted-server-core/src/ec/partner.rs +++ b/crates/trusted-server-core/src/ec/partner.rs @@ -1,8 +1,13 @@ //! Partner registry — `PartnerRecord` schema and `PartnerStore` operations. //! //! Each partner (SSP, DSP, identity vendor) is stored as a JSON record in -//! the Fastly KV Store keyed by `partner_id`. A secondary index -//! `apikey:{sha256_hex}` provides O(1) API key lookups for batch sync auth. +//! the Fastly KV Store keyed by `partner_id`. Two secondary indexes exist: +//! +//! - `apikey:{sha256_hex}` — maps API key hashes to partner IDs for O(1) +//! auth lookups during batch sync. +//! - `_pull_enabled` — JSON array of partner IDs with `pull_sync_enabled: +//! true`, enabling O(1+N) reads on the pull sync hot path instead of a +//! full partner scan. use std::{collections::HashSet, sync::OnceLock}; @@ -34,6 +39,13 @@ const RESERVED_PARTNER_IDS: &[&str] = &[ /// Prefix for the API key hash secondary index keys. const APIKEY_INDEX_PREFIX: &str = "apikey:"; +/// Key for the pull-enabled partner secondary index. +/// +/// Stores a JSON array of partner IDs that have `pull_sync_enabled: true`. +/// Updated on every `upsert()` so that `pull_enabled_partners()` can read +/// a single index key instead of listing/scanning all partners. +const PULL_ENABLED_INDEX_KEY: &str = "_pull_enabled"; + /// Cached compiled regex for partner ID validation. static PARTNER_ID_REGEX: OnceLock> = OnceLock::new(); @@ -190,9 +202,13 @@ pub fn hash_api_key(api_key: &str) -> String { /// Wraps a Fastly KV Store for partner registry operations. /// -/// Partner records are keyed by `partner_id`. A secondary index -/// `apikey:{sha256_hex}` maps API key hashes to partner IDs for -/// O(1) auth lookups during batch sync. +/// Partner records are keyed by `partner_id`. Two secondary indexes +/// optimize hot-path operations: +/// +/// - `apikey:{sha256_hex}` maps API key hashes to partner IDs for +/// O(1) auth lookups during batch sync. +/// - `_pull_enabled` stores a JSON array of partner IDs with +/// `pull_sync_enabled: true` for O(1+N) reads during pull sync dispatch. pub struct PartnerStore { store_name: String, } @@ -289,7 +305,7 @@ impl PartnerStore { })?; for key in page.keys() { - if key.starts_with(APIKEY_INDEX_PREFIX) { + if key.starts_with(APIKEY_INDEX_PREFIX) || key == PULL_ENABLED_INDEX_KEY { continue; } @@ -321,7 +337,73 @@ impl PartnerStore { Ok(records) } - /// Writes or updates a partner record and maintains the API key index. + /// Returns pull-enabled partners via the `_pull_enabled` secondary index. + /// + /// Performs 1 index read + N partner reads (where N = pull-enabled count) + /// instead of scanning all partners. Falls back to [`list_registered`] + /// with client-side filtering when the index is missing or unreadable, + /// ensuring correctness even before the first `upsert()` writes the index. + /// + /// # Errors + /// + /// Returns [`TrustedServerError::KvStore`] on store or deserialization failure. + pub fn pull_enabled_partners(&self) -> Result, Report> { + let store = self.open_store()?; + + // Read the secondary index. + let index_body = match store.lookup(PULL_ENABLED_INDEX_KEY) { + Ok(mut resp) => resp.take_body_bytes(), + Err(fastly::kv_store::KVStoreError::ItemNotFound) => { + // Index not yet written — fall back to full scan. + log::debug!("Pull-enabled index missing, falling back to list_registered"); + return self + .list_registered() + .map(|v| v.into_iter().filter(|p| p.pull_sync_enabled).collect()); + } + Err(err) => { + // Index unreadable — fall back to full scan rather than failing. + log::warn!( + "Failed to read pull-enabled index, falling back to list_registered: {err:?}" + ); + return self + .list_registered() + .map(|v| v.into_iter().filter(|p| p.pull_sync_enabled).collect()); + } + }; + + let partner_ids: Vec = + serde_json::from_slice(&index_body).change_context(TrustedServerError::KvStore { + store_name: self.store_name.clone(), + message: "Failed to deserialize pull-enabled index".to_owned(), + })?; + + let mut records = Vec::with_capacity(partner_ids.len()); + for partner_id in &partner_ids { + match self.get(partner_id)? { + Some(record) if record.pull_sync_enabled => { + records.push(record); + } + Some(_) => { + // Index is stale — partner is no longer pull-enabled. + // This is self-healing: the next `upsert()` will fix the index. + log::debug!( + "Pull-enabled index references partner '{}' which is no longer pull-enabled", + partner_id + ); + } + None => { + log::debug!( + "Pull-enabled index references non-existent partner '{}'", + partner_id + ); + } + } + } + + Ok(records) + } + + /// Writes or updates a partner record and maintains secondary indexes. /// /// Returns `true` if this was a new partner (create), `false` if an /// existing partner was updated. @@ -331,6 +413,7 @@ impl PartnerStore { /// 2. Write new `apikey:` index /// 3. Write primary record /// 4. Delete old `apikey:` index (if key rotated) + /// 5. Update `_pull_enabled` secondary index (best-effort) /// /// Writes are still **not fully atomic**, but this order ensures /// registration does not return success after a failed index write and @@ -403,6 +486,12 @@ impl PartnerStore { } } + // 4. Update _pull_enabled secondary index (best-effort). + // This is the last step so a failure here doesn't affect the + // primary registration. `pull_enabled_partners()` falls back to + // `list_registered()` if the index is missing or stale. + self.update_pull_enabled_index(&store, &record.id, record.pull_sync_enabled); + Ok(is_create) } @@ -438,6 +527,56 @@ impl PartnerStore { Ok(Some(partner_id)) } + /// Best-effort update of the `_pull_enabled` secondary index. + /// + /// Reads the current index, adds or removes the partner ID, and writes + /// it back. All errors are logged and swallowed — the primary record + /// write has already succeeded. + fn update_pull_enabled_index( + &self, + store: &KVStore, + partner_id: &str, + pull_sync_enabled: bool, + ) { + // Read existing index (or start with empty list). + let mut ids: Vec = match store.lookup(PULL_ENABLED_INDEX_KEY) { + Ok(mut resp) => { + let bytes = resp.take_body_bytes(); + serde_json::from_slice(&bytes).unwrap_or_default() + } + Err(_) => Vec::new(), + }; + + let had_id = ids.iter().any(|id| id == partner_id); + + if pull_sync_enabled && !had_id { + ids.push(partner_id.to_owned()); + } else if !pull_sync_enabled && had_id { + ids.retain(|id| id != partner_id); + } else { + // No change needed. + return; + } + + let body = match serde_json::to_string(&ids) { + Ok(b) => b, + Err(err) => { + log::warn!( + "Failed to serialize pull-enabled index after updating partner '{}': {err}", + partner_id + ); + return; + } + }; + + if let Err(err) = store.build_insert().execute(PULL_ENABLED_INDEX_KEY, body) { + log::warn!( + "Failed to write pull-enabled index after updating partner '{}': {err:?}", + partner_id + ); + } + } + fn restore_previous_index_mapping( &self, store: &KVStore, diff --git a/crates/trusted-server-core/src/ec/pull_sync.rs b/crates/trusted-server-core/src/ec/pull_sync.rs index d07c15e00..298671efc 100644 --- a/crates/trusted-server-core/src/ec/pull_sync.rs +++ b/crates/trusted-server-core/src/ec/pull_sync.rs @@ -94,33 +94,40 @@ pub fn dispatch_pull_sync( } }; - let partners = match partner_store.list_registered() { + // Use the _pull_enabled secondary index for O(1+N) reads instead of + // scanning all partners (§13.1). Falls back to list_registered() if + // the index is missing or unreadable. + let mut pull_partners = match partner_store.pull_enabled_partners() { Ok(partners) => partners, Err(err) => { - log::warn!("Pull sync: failed to list partners: {err:?}"); + log::warn!("Pull sync: failed to list pull-enabled partners: {err:?}"); return; } }; - let pull_enabled_count = partners.iter().filter(|p| p.pull_sync_enabled).count(); + // Sort by ID for deterministic ordering, then apply a rotating hourly + // offset so that different partners get dispatch priority (§10.3). + pull_partners.sort_by(|a, b| a.id.cmp(&b.id)); + log::debug!( - "Pull sync: enumerated {} partners ({} pull-enabled)", - partners.len(), - pull_enabled_count + "Pull sync: {} pull-enabled partners after filtering", + pull_partners.len(), ); - if pull_enabled_count == 0 { + if pull_partners.is_empty() { return; } + // Rotate the partner list so that the starting partner changes each + // hour. This ensures fair distribution when max_concurrency limits + // how many partners are dispatched per request. + let offset = (now / 3600) as usize % pull_partners.len(); + pull_partners.rotate_left(offset); + let max_concurrency = settings.ec.pull_sync_concurrency.max(1); let mut in_flight: Vec = Vec::new(); - for partner in partners { - if !partner.pull_sync_enabled { - continue; - } - + for partner in pull_partners { if !is_partner_pull_eligible(&partner, kv_entry.as_ref(), now) { continue; } @@ -536,4 +543,36 @@ mod tests { "should parse uid from 200 body" ); } + + #[test] + fn rotating_offset_distributes_partners_across_hours() { + // Simulate 3 partners sorted by ID: alpha, beta, gamma. + let ids = vec!["alpha", "beta", "gamma"]; + + // Hour 0: offset = 0 % 3 = 0 → [alpha, beta, gamma] + let ts_h0: u64 = 100; // within hour 0 + let offset_h0 = (ts_h0 / 3600) as usize % ids.len(); + assert_eq!(offset_h0, 0, "hour 0 should start at index 0"); + + // Hour 1: offset = (3600 / 3600) % 3 = 1 → [beta, gamma, alpha] + let offset_h1 = (3600u64 / 3600) as usize % ids.len(); + assert_eq!(offset_h1, 1, "hour 1 should start at index 1"); + + // Hour 2: offset = (7200 / 3600) % 3 = 2 → [gamma, alpha, beta] + let offset_h2 = (7200u64 / 3600) as usize % ids.len(); + assert_eq!(offset_h2, 2, "hour 2 should start at index 2"); + + // Hour 3: offset = (10800 / 3600) % 3 = 0 → wraps back to [alpha, beta, gamma] + let offset_h3 = (10800u64 / 3600) as usize % ids.len(); + assert_eq!(offset_h3, 0, "hour 3 should wrap back to index 0"); + + // Verify rotate_left produces expected ordering + let mut rotated = ids.clone(); + rotated.rotate_left(offset_h1); + assert_eq!( + rotated, + vec!["beta", "gamma", "alpha"], + "hour 1 rotation should move beta to front" + ); + } } diff --git a/crates/trusted-server-core/src/ec/sync_pixel.rs b/crates/trusted-server-core/src/ec/sync_pixel.rs index d054076cf..9323a0386 100644 --- a/crates/trusted-server-core/src/ec/sync_pixel.rs +++ b/crates/trusted-server-core/src/ec/sync_pixel.rs @@ -34,8 +34,8 @@ pub fn handle_sync( let query = SyncQuery::parse(req)?; let partner = partner_store.get(&query.partner)?.ok_or_else(|| { - Report::new(TrustedServerError::BadRequest { - message: format!("unknown partner '{}'", query.partner), + Report::new(TrustedServerError::PartnerNotFound { + partner_id: query.partner.clone(), }) })?; diff --git a/crates/trusted-server-core/src/error.rs b/crates/trusted-server-core/src/error.rs index 2720e88b5..7008c42e5 100644 --- a/crates/trusted-server-core/src/error.rs +++ b/crates/trusted-server-core/src/error.rs @@ -80,9 +80,17 @@ pub enum TrustedServerError { #[display("Settings error: {message}")] Settings { message: String }, - /// EC ID generation or validation failed. - #[display("EC error: {message}")] - Ec { message: String }, + /// Edge cookie ID generation or validation failed. + #[display("Edge cookie error: {message}")] + EdgeCookie { message: String }, + + /// Requested partner was not found in the partner registry. + #[display("Partner not found: {partner_id}")] + PartnerNotFound { partner_id: String }, + + /// Partner authentication failed (invalid or missing credentials). + #[display("Partner auth failed: {partner_id}")] + PartnerAuthFailed { partner_id: String }, } impl Error for TrustedServerError {} @@ -117,6 +125,8 @@ impl IntoHttpResponse for TrustedServerError { Self::Forbidden { .. } => StatusCode::FORBIDDEN, Self::AllowlistViolation { .. } => StatusCode::FORBIDDEN, Self::Ec { .. } => StatusCode::INTERNAL_SERVER_ERROR, + Self::PartnerNotFound { .. } => StatusCode::BAD_REQUEST, + Self::PartnerAuthFailed { .. } => StatusCode::UNAUTHORIZED, } } diff --git a/crates/trusted-server-core/src/http_util.rs b/crates/trusted-server-core/src/http_util.rs index 6944fb1c3..b2fdbea9d 100644 --- a/crates/trusted-server-core/src/http_util.rs +++ b/crates/trusted-server-core/src/http_util.rs @@ -11,14 +11,18 @@ use crate::settings::Settings; /// Copy `X-*` custom headers from one request to another, skipping TS-internal headers. /// -/// This filters out all headers listed in [`INTERNAL_HEADERS`] to prevent leaking -/// internal identity, geo-enrichment, and debugging data to downstream third-party -/// services. Integrations that forward custom headers should use this utility -/// instead of manually iterating over header names. +/// This filters out all headers listed in [`INTERNAL_HEADERS`] **and** any header +/// matching the `x-ts-` prefix (case-insensitive) to prevent leaking internal +/// identity, geo-enrichment, debugging data, and dynamic `X-ts-` +/// headers to downstream third-party services. Integrations that forward custom +/// headers should use this utility instead of manually iterating over header names. pub fn copy_custom_headers(from: &Request, to: &mut Request) { for header_name in from.get_header_names() { let name_str = header_name.as_str(); + // Header names are lowercased by the HTTP library, so a single + // prefix check covers both `x-ts-` and `X-ts-` variants. if (name_str.starts_with("x-") || name_str.starts_with("X-")) + && !name_str.starts_with("x-ts-") && !INTERNAL_HEADERS.contains(&name_str) { if let Some(value) = from.get_header(header_name) { @@ -705,6 +709,9 @@ mod tests { req.set_header("X-Custom-2", "value2"); req.set_header("x-ts-ec", "should not copy"); req.set_header("x-geo-country", "US"); + // Dynamic partner header (x-ts-) + req.set_header("x-ts-ssp_x", "partner-uid-123"); + req.set_header("x-ts-liveramp", "lr-uid-456"); let mut target = Request::new(fastly::http::Method::GET, "https://target.com"); copy_custom_headers(&req, &mut target); @@ -727,6 +734,14 @@ mod tests { target.get_header("x-geo-country").is_none(), "Should filter x-geo-country" ); + assert!( + target.get_header("x-ts-ssp_x").is_none(), + "Should filter dynamic x-ts- headers" + ); + assert!( + target.get_header("x-ts-liveramp").is_none(), + "Should filter dynamic x-ts- headers" + ); } #[test] diff --git a/crates/trusted-server-core/src/integrations/registry.rs b/crates/trusted-server-core/src/integrations/registry.rs index 295bcd858..26bdca2d2 100644 --- a/crates/trusted-server-core/src/integrations/registry.rs +++ b/crates/trusted-server-core/src/integrations/registry.rs @@ -1300,8 +1300,10 @@ mod tests { let registry = IntegrationRegistry::from_routes(routes); let mut req = Request::get("https://test.example.com/integrations/test/ec"); - // Pre-existing cookie, but no geo data → Unknown jurisdiction → consent denied. - req.set_header(header::COOKIE, "ts-ec=existing_id_12345"); + // Pre-existing cookie with valid EC ID format, but no geo data → + // Unknown jurisdiction → consent denied. + let valid_ec_id = format!("{}.AbCd12", "a".repeat(64)); + req.set_header(header::COOKIE, format!("ts-ec={valid_ec_id}")); let mut ec_context = EcContext::read_from_request(&settings, &req).expect("should read EC context"); diff --git a/crates/trusted-server-core/src/proxy.rs b/crates/trusted-server-core/src/proxy.rs index 0d3f092d3..8dc5c1dcf 100644 --- a/crates/trusted-server-core/src/proxy.rs +++ b/crates/trusted-server-core/src/proxy.rs @@ -1538,7 +1538,8 @@ mod tests { sig ), ); - req.set_header(crate::constants::HEADER_X_TS_EC, "ec-123"); + let valid_ec_id = format!("{}.AbCd12", "a".repeat(64)); + req.set_header(crate::constants::HEADER_X_TS_EC, &valid_ec_id); let resp = handle_first_party_click(&settings, &noop_services(), req) .await @@ -1554,7 +1555,7 @@ mod tests { .map(|(k, v)| (k.into_owned(), v.into_owned())) .collect(); assert_eq!(pairs.remove("foo").as_deref(), Some("1")); - assert_eq!(pairs.remove("ts-ec").as_deref(), Some("ec-123")); + assert_eq!(pairs.remove("ts-ec").as_deref(), Some(valid_ec_id.as_str())); assert!(pairs.is_empty()); } From 9ab82cc99d51691f7158bc54d0724f1181226e6f Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 30 Mar 2026 09:36:30 -0500 Subject: [PATCH 14/63] Harden EC endpoints: input validation, binary-search EIDs encoding, and cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add body size limit (64 KiB) to partner registration - Validate partner UID length (max 512 bytes) in batch sync and sync pixel - Replace linear scan with binary search in encode_eids_header - Use constant-time comparison inline in partner lookup, remove unused verify_api_key - Remove unused PartnerAuthFailed error variant, fix PartnerNotFound → 404 - Add Access-Control-Max-Age CORS header to identify endpoint - Tighten consent-denied integration test to expect only 403 - Add stability doc-comment to normalize_ip - Log warning instead of silent fallback on SystemTime failure --- .../tests/frameworks/scenarios.rs | 14 ++++----- crates/trusted-server-core/src/ec/admin.rs | 11 ++++++- .../trusted-server-core/src/ec/batch_sync.rs | 5 +++- crates/trusted-server-core/src/ec/eids.rs | 27 ++++++++++++++--- .../trusted-server-core/src/ec/generation.rs | 8 +++++ crates/trusted-server-core/src/ec/identify.rs | 1 + crates/trusted-server-core/src/ec/mod.rs | 5 +++- crates/trusted-server-core/src/ec/partner.rs | 29 ++----------------- .../trusted-server-core/src/ec/sync_pixel.rs | 9 ++++++ crates/trusted-server-core/src/error.rs | 6 +--- 10 files changed, 68 insertions(+), 47 deletions(-) diff --git a/crates/integration-tests/tests/frameworks/scenarios.rs b/crates/integration-tests/tests/frameworks/scenarios.rs index d5f01fbba..3ee9f9136 100644 --- a/crates/integration-tests/tests/frameworks/scenarios.rs +++ b/crates/integration-tests/tests/frameworks/scenarios.rs @@ -613,21 +613,19 @@ fn ec_identify_consent_denied(base_url: &str) -> TestResult<()> { .attach("should set ts-ec on organic request for consent-denied test") })?; - // Identify with GPC=1 — if jurisdiction is non-regulated + GPC, - // consent may still be denied depending on US-state detection. - // Without geo, jurisdiction is Unknown → fail-closed → 403. + // Identify with GPC=1 — without geo, jurisdiction is Unknown → + // fail-closed → consent denied. Per spec §11.4, consent is evaluated + // *before* EC presence, so this must be 403 Forbidden regardless of + // whether an EC cookie exists. let resp = client.get_with_headers("/identify", &[("sec-gpc", "1")])?; let status = resp.status().as_u16(); - // Under Unknown jurisdiction (no geo in Viceroy), EC is denied - // so the response may be 403 or 204 depending on whether the EC - // context reads the cookie before consent check. - if status != 403 && status != 204 { + if status != 403 { return Err(Report::new(TestError::UnexpectedStatusCode { expected: 403, actual: status, }) - .attach("identify with consent denied should return 403 or 204")); + .attach("identify with consent denied should return 403")); } log::info!("EC identify consent denied: PASSED (status={status})"); diff --git a/crates/trusted-server-core/src/ec/admin.rs b/crates/trusted-server-core/src/ec/admin.rs index 7f7c60d5a..ae30e3dbd 100644 --- a/crates/trusted-server-core/src/ec/admin.rs +++ b/crates/trusted-server-core/src/ec/admin.rs @@ -150,8 +150,17 @@ pub fn handle_register_partner( partner_store: &PartnerStore, mut req: Request, ) -> Result> { - // Parse request body. + // Parse request body with size limit to prevent memory abuse. + const MAX_BODY_SIZE: usize = 64 * 1024; // 64 KiB let body_bytes = req.take_body_bytes(); + if body_bytes.len() > MAX_BODY_SIZE { + return Err(Report::new(TrustedServerError::BadRequest { + message: format!( + "Request body too large ({} bytes, max {MAX_BODY_SIZE})", + body_bytes.len() + ), + })); + } let request: RegisterPartnerRequest = serde_json::from_slice(&body_bytes).change_context(TrustedServerError::BadRequest { message: "Invalid JSON in request body".to_owned(), diff --git a/crates/trusted-server-core/src/ec/batch_sync.rs b/crates/trusted-server-core/src/ec/batch_sync.rs index c99d45ee8..6c4da2be6 100644 --- a/crates/trusted-server-core/src/ec/batch_sync.rs +++ b/crates/trusted-server-core/src/ec/batch_sync.rs @@ -27,6 +27,9 @@ const REASON_KV_UNAVAILABLE: &str = "kv_unavailable"; /// Maximum number of mappings allowed in a single batch request. const MAX_BATCH_SIZE: usize = 1000; +/// Maximum allowed length (in bytes) for a partner UID. +const MAX_UID_LENGTH: usize = 512; + trait BatchSyncWriter { fn upsert_partner_id_if_exists( &self, @@ -203,7 +206,7 @@ fn process_mappings( continue; } - if mapping.partner_uid.is_empty() { + if mapping.partner_uid.is_empty() || mapping.partner_uid.len() > MAX_UID_LENGTH { errors.push(MappingError { index: idx, reason: REASON_INVALID_PARTNER_UID, diff --git a/crates/trusted-server-core/src/ec/eids.rs b/crates/trusted-server-core/src/ec/eids.rs index 5af322c09..c4f7e550e 100644 --- a/crates/trusted-server-core/src/ec/eids.rs +++ b/crates/trusted-server-core/src/ec/eids.rs @@ -114,20 +114,39 @@ pub fn build_eids_header( /// /// Returns an error if JSON serialization fails. pub fn encode_eids_header(eids: &[Eid]) -> Result<(String, bool), Report> { - for size in (0..=eids.len()).rev() { + let try_encode = |size: usize| -> Result> { let json = serde_json::to_vec(&eids[..size]).change_context( TrustedServerError::Configuration { message: "Failed to serialize eids header payload".to_owned(), }, )?; - let encoded = BASE64.encode(json); + Ok(BASE64.encode(json)) + }; + // Fast path: try the full slice first (common case — no truncation). + let encoded = try_encode(eids.len())?; + if encoded.len() <= MAX_EIDS_HEADER_BYTES { + return Ok((encoded, false)); + } + + // Binary search for the largest count that fits within the limit. + // Invariant: lo always fits, hi never fits. + let mut lo: usize = 0; + let mut hi: usize = eids.len(); + + while lo + 1 < hi { + let mid = lo + (hi - lo) / 2; + let encoded = try_encode(mid)?; if encoded.len() <= MAX_EIDS_HEADER_BYTES { - return Ok((encoded, size != eids.len())); + lo = mid; + } else { + hi = mid; } } - Ok((BASE64.encode("[]"), true)) + // `lo` is the largest size that fits. Re-encode it for the final value. + let encoded = try_encode(lo)?; + Ok((encoded, true)) } #[cfg(test)] diff --git a/crates/trusted-server-core/src/ec/generation.rs b/crates/trusted-server-core/src/ec/generation.rs index d21728d48..3752b479d 100644 --- a/crates/trusted-server-core/src/ec/generation.rs +++ b/crates/trusted-server-core/src/ec/generation.rs @@ -24,6 +24,14 @@ const ALPHANUMERIC_CHARSET: &[u8] = /// where devices rotate their interface identifier (lower 64 bits). /// The first 4 segments are hex-encoded without separators. /// IPv4 addresses are returned unchanged. +/// +/// # Stability +/// +/// The output format is a stable contract — EC hashes stored in KV depend +/// on it. Changing the format would invalidate all existing EC identities. +/// - **IPv4:** decimal-dotted notation (e.g. `"192.168.1.1"`) +/// - **IPv6:** first 4 segments as zero-padded lowercase hex without +/// separators (e.g. `"20010db885a30000"`) fn normalize_ip(ip: IpAddr) -> String { match ip { IpAddr::V4(ipv4) => ipv4.to_string(), diff --git a/crates/trusted-server-core/src/ec/identify.rs b/crates/trusted-server-core/src/ec/identify.rs index 5226cd854..b03dee86c 100644 --- a/crates/trusted-server-core/src/ec/identify.rs +++ b/crates/trusted-server-core/src/ec/identify.rs @@ -214,6 +214,7 @@ fn apply_cors_headers(response: &mut Response, origin: &str) { header::ACCESS_CONTROL_ALLOW_HEADERS, "Cookie, X-ts-ec, X-consent-advertising", ); + response.set_header(header::ACCESS_CONTROL_MAX_AGE, "600"); response.set_header(header::VARY, "Origin"); } diff --git a/crates/trusted-server-core/src/ec/mod.rs b/crates/trusted-server-core/src/ec/mod.rs index efb3f63df..959824b07 100644 --- a/crates/trusted-server-core/src/ec/mod.rs +++ b/crates/trusted-server-core/src/ec/mod.rs @@ -422,7 +422,10 @@ pub(crate) fn current_timestamp() -> u64 { std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_secs()) - .unwrap_or(0) + .unwrap_or_else(|err| { + log::warn!("SystemTime::now() failed, falling back to epoch 0: {err}"); + 0 + }) } #[cfg(test)] diff --git a/crates/trusted-server-core/src/ec/partner.rs b/crates/trusted-server-core/src/ec/partner.rs index 828e225ae..b26e7a91a 100644 --- a/crates/trusted-server-core/src/ec/partner.rs +++ b/crates/trusted-server-core/src/ec/partner.rs @@ -659,8 +659,8 @@ impl PartnerStore { }; // Verify the stored hash matches — guards against stale index from - // key rotation. - if record.api_key_hash != hash { + // key rotation. Uses constant-time comparison to prevent timing attacks. + if !bool::from(record.api_key_hash.as_bytes().ct_eq(hash.as_bytes())) { log::warn!( "API key hash mismatch for partner '{}' (stale index after key rotation)", record.id, @@ -670,31 +670,6 @@ impl PartnerStore { Ok(Some(record)) } - - /// Verifies an API key against the stored hash for a given partner. - /// - /// Uses SHA-256 hashing and constant-time comparison to prevent - /// timing attacks. - /// - /// # Errors - /// - /// Returns [`TrustedServerError::KvStore`] if the partner lookup fails. - pub fn verify_api_key( - &self, - partner_id: &str, - api_key: &str, - ) -> Result> { - let record = match self.get(partner_id)? { - Some(r) => r, - None => return Ok(false), - }; - - let incoming_hash = hash_api_key(api_key); - let stored_bytes = record.api_key_hash.as_bytes(); - let incoming_bytes = incoming_hash.as_bytes(); - - Ok(stored_bytes.ct_eq(incoming_bytes).into()) - } } #[cfg(test)] diff --git a/crates/trusted-server-core/src/ec/sync_pixel.rs b/crates/trusted-server-core/src/ec/sync_pixel.rs index 9323a0386..d968c1514 100644 --- a/crates/trusted-server-core/src/ec/sync_pixel.rs +++ b/crates/trusted-server-core/src/ec/sync_pixel.rs @@ -18,6 +18,9 @@ use super::EcContext; /// Name of the Fastly rate counter resource used by sync rate limiting. pub const RATE_COUNTER_NAME: &str = "counter_store"; +/// Maximum allowed length (in bytes) for a partner UID. +const MAX_UID_LENGTH: usize = 512; + /// Handles `GET /sync` pixel sync requests. /// /// # Errors @@ -33,6 +36,12 @@ pub fn handle_sync( ) -> Result> { let query = SyncQuery::parse(req)?; + if query.uid.len() > MAX_UID_LENGTH { + return Err(Report::new(TrustedServerError::BadRequest { + message: format!("uid exceeds maximum length of {MAX_UID_LENGTH} bytes"), + })); + } + let partner = partner_store.get(&query.partner)?.ok_or_else(|| { Report::new(TrustedServerError::PartnerNotFound { partner_id: query.partner.clone(), diff --git a/crates/trusted-server-core/src/error.rs b/crates/trusted-server-core/src/error.rs index 7008c42e5..c0de14ba7 100644 --- a/crates/trusted-server-core/src/error.rs +++ b/crates/trusted-server-core/src/error.rs @@ -87,10 +87,6 @@ pub enum TrustedServerError { /// Requested partner was not found in the partner registry. #[display("Partner not found: {partner_id}")] PartnerNotFound { partner_id: String }, - - /// Partner authentication failed (invalid or missing credentials). - #[display("Partner auth failed: {partner_id}")] - PartnerAuthFailed { partner_id: String }, } impl Error for TrustedServerError {} @@ -125,7 +121,7 @@ impl IntoHttpResponse for TrustedServerError { Self::Forbidden { .. } => StatusCode::FORBIDDEN, Self::AllowlistViolation { .. } => StatusCode::FORBIDDEN, Self::Ec { .. } => StatusCode::INTERNAL_SERVER_ERROR, - Self::PartnerNotFound { .. } => StatusCode::BAD_REQUEST, + Self::PartnerNotFound { .. } => StatusCode::NOT_FOUND, Self::PartnerAuthFailed { .. } => StatusCode::UNAUTHORIZED, } } From 98c516afe4080b32116b339fea4e243198a0cd44 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 30 Mar 2026 19:03:54 -0500 Subject: [PATCH 15/63] Fix post-rebase compilation: restore missing methods, imports, and error variants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve integration issues from rebasing onto feature/ssc-update: - Restore prepare_runtime() and validate_cookie_domain() lost in conflict resolution - Add InsecureDefault error variant and wire reject_placeholder_secrets() into get_settings() - Add sha2/subtle imports for constant-time auth comparison - Fix error match arms (Ec → EdgeCookie, remove nonexistent PartnerAuthFailed) - Fix orchestrator error handling to use send_to_client() pattern - Remove dead cookie helpers superseded by ec/cookies module --- crates/trusted-server-core/src/auth.rs | 2 + crates/trusted-server-core/src/cookies.rs | 44 ------------------- crates/trusted-server-core/src/error.rs | 8 +++- crates/trusted-server-core/src/settings.rs | 35 ++++++++++++++- .../trusted-server-core/src/settings_data.rs | 2 +- 5 files changed, 42 insertions(+), 49 deletions(-) diff --git a/crates/trusted-server-core/src/auth.rs b/crates/trusted-server-core/src/auth.rs index 1b60eb817..9ea918e8e 100644 --- a/crates/trusted-server-core/src/auth.rs +++ b/crates/trusted-server-core/src/auth.rs @@ -2,6 +2,8 @@ use base64::{engine::general_purpose::STANDARD, Engine as _}; use error_stack::Report; use fastly::http::{header, StatusCode}; use fastly::{Request, Response}; +use sha2::{Digest, Sha256}; +use subtle::ConstantTimeEq; use crate::error::TrustedServerError; use crate::settings::Settings; diff --git a/crates/trusted-server-core/src/cookies.rs b/crates/trusted-server-core/src/cookies.rs index 2d9bf2ff4..3408eb794 100644 --- a/crates/trusted-server-core/src/cookies.rs +++ b/crates/trusted-server-core/src/cookies.rs @@ -23,50 +23,6 @@ pub const CONSENT_COOKIE_NAMES: &[&str] = &[ COOKIE_US_PRIVACY, ]; -const COOKIE_MAX_AGE: i32 = 365 * 24 * 60 * 60; // 1 year - -fn is_allowed_ec_id_char(c: char) -> bool { - c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '_') -} - -// Outbound allowlist for cookie sanitization: permits [a-zA-Z0-9._-] as a -// defense-in-depth backstop when setting the Set-Cookie header. This is -// intentionally broader than the inbound format validator -// (`synthetic::is_valid_synthetic_id`), which enforces the exact -// `<64-hex>.<6-alphanumeric>` structure and is used to reject untrusted -// request values before they enter the system. -#[must_use] -pub(crate) fn ec_id_has_only_allowed_chars(ec_id: &str) -> bool { - ec_id.chars().all(is_allowed_ec_id_char) -} - -fn sanitize_ec_id_for_cookie(ec_id: &str) -> Cow<'_, str> { - if ec_id_has_only_allowed_chars(ec_id) { - return Cow::Borrowed(ec_id); - } - - let safe_id = ec_id - .chars() - .filter(|c| is_allowed_ec_id_char(*c)) - .collect::(); - - log::warn!( - "Stripped disallowed characters from EC ID before setting cookie (len {} -> {}); \ - callers should reject invalid request IDs before cookie creation", - ec_id.len(), - safe_id.len(), - ); - - Cow::Owned(safe_id) -} - -fn ec_cookie_attributes(settings: &Settings, max_age: i32) -> String { - format!( - "Domain={}; Path=/; Secure; HttpOnly; SameSite=Lax; Max-Age={max_age}", - settings.publisher.cookie_domain, - ) -} - /// Parses a cookie string into a [`CookieJar`]. /// /// Returns an empty jar if the cookie string is unparseable. diff --git a/crates/trusted-server-core/src/error.rs b/crates/trusted-server-core/src/error.rs index c0de14ba7..26c000c67 100644 --- a/crates/trusted-server-core/src/error.rs +++ b/crates/trusted-server-core/src/error.rs @@ -87,6 +87,10 @@ pub enum TrustedServerError { /// Requested partner was not found in the partner registry. #[display("Partner not found: {partner_id}")] PartnerNotFound { partner_id: String }, + + /// A secret field still contains a known placeholder/default value. + #[display("Insecure default value for: {field}")] + InsecureDefault { field: String }, } impl Error for TrustedServerError {} @@ -120,9 +124,9 @@ impl IntoHttpResponse for TrustedServerError { Self::Proxy { .. } => StatusCode::BAD_GATEWAY, Self::Forbidden { .. } => StatusCode::FORBIDDEN, Self::AllowlistViolation { .. } => StatusCode::FORBIDDEN, - Self::Ec { .. } => StatusCode::INTERNAL_SERVER_ERROR, + Self::EdgeCookie { .. } => StatusCode::INTERNAL_SERVER_ERROR, Self::PartnerNotFound { .. } => StatusCode::NOT_FOUND, - Self::PartnerAuthFailed { .. } => StatusCode::UNAUTHORIZED, + Self::InsecureDefault { .. } => StatusCode::INTERNAL_SERVER_ERROR, } } diff --git a/crates/trusted-server-core/src/settings.rs b/crates/trusted-server-core/src/settings.rs index 6d8b714de..3951f9913 100644 --- a/crates/trusted-server-core/src/settings.rs +++ b/crates/trusted-server-core/src/settings.rs @@ -22,6 +22,7 @@ pub struct Publisher { pub domain: String, /// Domain for non-EC cookies. EC cookies use a separate computed domain /// (see [`ec_cookie_domain`](Self::ec_cookie_domain)). + #[validate(custom(function = validate_cookie_domain))] pub cookie_domain: String, #[validate(custom(function = validate_no_trailing_slash))] pub origin_url: String, @@ -534,6 +535,19 @@ impl Settings { /// /// # Errors /// + /// Returns a configuration error if any cached runtime artifact cannot be prepared. + pub fn prepare_runtime(&self) -> Result<(), Report> { + for handler in &self.handlers { + handler.prepare_runtime()?; + } + + Ok(()) + } + + /// Reject settings that still contain known placeholder secrets. + /// + /// # Errors + /// /// Returns [`TrustedServerError::InsecureDefault`] when one or more secret /// fields still contain a placeholder value. pub fn reject_placeholder_secrets(&self) -> Result<(), Report> { @@ -546,7 +560,13 @@ impl Settings { insecure_fields.push("publisher.proxy_secret"); } - Ok(()) + if insecure_fields.is_empty() { + Ok(()) + } else { + Err(Report::new(TrustedServerError::InsecureDefault { + field: insecure_fields.join(", "), + })) + } } /// Resolve the first handler whose regex matches the request path. @@ -645,6 +665,18 @@ impl Settings { } } +fn validate_cookie_domain(value: &str) -> Result<(), ValidationError> { + // `=` is excluded: it only has special meaning in the name=value pair, + // not within the Domain attribute value. + if value.contains([';', '\n', '\r']) { + let mut err = ValidationError::new("cookie_metacharacters"); + err.message = + Some("cookie_domain must not contain cookie metacharacters (;, \\n, \\r)".into()); + return Err(err); + } + Ok(()) +} + fn validate_no_trailing_slash(value: &str) -> Result<(), ValidationError> { if value.ends_with('/') { let mut err = ValidationError::new("trailing_slash"); @@ -1684,7 +1716,6 @@ mod tests { ); } -<<<<<<< HEAD // --- Proxy::normalize --- #[test] diff --git a/crates/trusted-server-core/src/settings_data.rs b/crates/trusted-server-core/src/settings_data.rs index 39b066504..7a59f47ac 100644 --- a/crates/trusted-server-core/src/settings_data.rs +++ b/crates/trusted-server-core/src/settings_data.rs @@ -3,7 +3,7 @@ use error_stack::{Report, ResultExt}; use validator::Validate; use crate::error::TrustedServerError; -use crate::settings::{EdgeCookie, Publisher, Settings}; +use crate::settings::Settings; pub use crate::auction_config_types::AuctionConfig; From 83ea142dcb67cc28ffd4cdb538d2b283e125b4ea Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 30 Mar 2026 20:20:19 -0500 Subject: [PATCH 16/63] Restore iframe creative rendering accidentally reverted by EC migration --- crates/js/lib/src/core/render.ts | 13 ++++++------ crates/js/lib/test/core/render.test.ts | 6 +++--- crates/trusted-server-core/src/creative.rs | 24 ++++++++++++---------- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/crates/js/lib/src/core/render.ts b/crates/js/lib/src/core/render.ts index da223851c..ee08ef288 100644 --- a/crates/js/lib/src/core/render.ts +++ b/crates/js/lib/src/core/render.ts @@ -7,15 +7,16 @@ import NORMALIZE_CSS from './styles/normalize.css?inline'; import IFRAME_TEMPLATE from './templates/iframe.html?raw'; // Sandbox permissions granted to creative iframes. -// Notably absent: -// allow-scripts, allow-same-origin — prevent JS execution and same-origin -// access, which are the primary attack vectors for malicious creatives. -// allow-forms — server-side sanitization strips
elements, so form -// submission from creatives is not a supported use case. Omitting this token -// is consistent with that server-side policy and reduces the attack surface. +// Ad creatives routinely contain scripts for tracking, click handling, and +// viewability measurement, so allow-scripts and allow-same-origin are required +// for creatives to render correctly. Server-side sanitization is the primary +// defense against malicious markup; the sandbox provides defense-in-depth. const CREATIVE_SANDBOX_TOKENS = [ + 'allow-forms', 'allow-popups', 'allow-popups-to-escape-sandbox', + 'allow-same-origin', + 'allow-scripts', 'allow-top-navigation-by-user-activation', ] as const; diff --git a/crates/js/lib/test/core/render.test.ts b/crates/js/lib/test/core/render.test.ts index 5bdb3a817..a81486cf3 100644 --- a/crates/js/lib/test/core/render.test.ts +++ b/crates/js/lib/test/core/render.test.ts @@ -27,12 +27,12 @@ describe('render', () => { expect(iframe.srcdoc).toContain('ad'); expect(div.querySelector('iframe')).toBe(iframe); const sandbox = iframe.getAttribute('sandbox') ?? ''; - expect(sandbox).not.toContain('allow-forms'); + expect(sandbox).toContain('allow-forms'); expect(sandbox).toContain('allow-popups'); expect(sandbox).toContain('allow-popups-to-escape-sandbox'); expect(sandbox).toContain('allow-top-navigation-by-user-activation'); - expect(sandbox).not.toContain('allow-same-origin'); - expect(sandbox).not.toContain('allow-scripts'); + expect(sandbox).toContain('allow-same-origin'); + expect(sandbox).toContain('allow-scripts'); }); it('preserves dollar sequences when building the creative document', async () => { diff --git a/crates/trusted-server-core/src/creative.rs b/crates/trusted-server-core/src/creative.rs index 45162d8cd..245af0e1e 100644 --- a/crates/trusted-server-core/src/creative.rs +++ b/crates/trusted-server-core/src/creative.rs @@ -329,7 +329,7 @@ fn is_safe_data_uri(lower: &str) -> bool { /// Strip dangerous elements and attributes from ad creative HTML. /// -/// Removes elements that can execute code or exfiltrate data (`script`, `iframe`, +/// Removes elements that can execute code or exfiltrate data (`script`, /// `object`, `embed`, `base`, `meta`, `form`, `link`, `style`, `noscript`) and strips `on*` event-handler /// attributes and dangerous URI schemes from all remaining elements: /// - `javascript:`, `vbscript:` @@ -361,18 +361,17 @@ pub fn sanitize_creative_html(markup: &str) -> String { HtmlSettings { element_content_handlers: vec![ // Remove executable/dangerous elements along with their inner content. - // -