diff --git a/crates/trusted-server-adapter-fastly/src/template_cache.rs b/crates/trusted-server-adapter-fastly/src/template_cache.rs index fe3148cb4..9f0d3d9b9 100644 --- a/crates/trusted-server-adapter-fastly/src/template_cache.rs +++ b/crates/trusted-server-adapter-fastly/src/template_cache.rs @@ -322,6 +322,7 @@ mod tests { name: "rsc".to_string(), values: Some(vec![b"1".to_vec()]), }], + cookie_values: Vec::new(), template_fingerprint: "fp".to_string(), schema_version: TEMPLATE_SCHEMA_VERSION, } diff --git a/crates/trusted-server-core/src/cookies.rs b/crates/trusted-server-core/src/cookies.rs index a716045f9..3fe6719ef 100644 --- a/crates/trusted-server-core/src/cookies.rs +++ b/crates/trusted-server-core/src/cookies.rs @@ -3,6 +3,8 @@ //! This module provides functionality for parsing, stripping, and forwarding cookies used in the //! trusted server system. +pub(crate) mod template_cache_policy; + use cookie::{Cookie, CookieJar}; use edgezero_core::body::Body as EdgeBody; use error_stack::{Report, ResultExt as _}; diff --git a/crates/trusted-server-core/src/cookies/template_cache_policy.rs b/crates/trusted-server-core/src/cookies/template_cache_policy.rs new file mode 100644 index 000000000..1f67ac62a --- /dev/null +++ b/crates/trusted-server-core/src/cookies/template_cache_policy.rs @@ -0,0 +1,525 @@ +//! Request-cookie classification for shared template caching. + +use std::collections::{HashMap, HashSet}; + +use crate::constants::{COOKIE_SHAREDID, COOKIE_TS_EC, COOKIE_TS_EIDS}; +use crate::platform::TemplateCookieValue; + +/// Whether the request can share a template and its explicit cookie dimensions. +#[derive(Debug, Eq, PartialEq)] +pub(crate) enum TemplateCookieDecision { + Bypass, + Eligible(Vec), +} + +/// Validates exact cookie names across both configured policies. +/// +/// # Errors +/// +/// Returns an error naming the field and cookie when a name is invalid or repeated. +pub(crate) fn validate_cookie_names( + key_names: &[String], + bypass_names: &[String], +) -> Result<(), String> { + let mut seen = HashSet::new(); + for (field, names) in [ + ("template_cache_key_cookies", key_names), + ("template_cache_bypass_cookies", bypass_names), + ] { + for name in names { + if !is_cookie_name(name.as_bytes()) { + return Err(format!("{field} contains invalid cookie name `{name}`")); + } + if field == "template_cache_key_cookies" + && [COOKIE_TS_EC, COOKIE_TS_EIDS, COOKIE_SHAREDID].contains(&name.as_str()) + { + return Err(format!( + "{field} must not key on Trusted Server identity cookie `{name}`" + )); + } + if !seen.insert(name.as_str()) { + return Err(format!( + "{field} repeats cookie name `{name}` within or across cookie policies" + )); + } + } + } + Ok(()) +} + +/// Classifies all request-cookie fields using validated configuration names. +/// +/// Named policies bypass malformed or ambiguous input. Empty policies retain the +/// legacy header-presence rule. Admitted dimensions are sorted by exact name and +/// own only the configured key-cookie values, preserving their raw bytes. +pub(crate) fn evaluate_cookie_policy( + headers: &http::HeaderMap, + key_names: &[String], + bypass_names: &[String], + independent: bool, +) -> TemplateCookieDecision { + if key_names.is_empty() && bypass_names.is_empty() { + return if headers.contains_key(http::header::COOKIE) && !independent { + TemplateCookieDecision::Bypass + } else { + TemplateCookieDecision::Eligible(Vec::new()) + }; + } + + let mut parsed = HashMap::<&[u8], &[u8]>::new(); + for field in headers.get_all(http::header::COOKIE) { + for raw_pair in field.as_bytes().split(|byte| *byte == b';') { + let pair = trim_pair(raw_pair); + let Some(separator) = pair.iter().position(|byte| *byte == b'=') else { + return TemplateCookieDecision::Bypass; + }; + let (name, rest) = pair.split_at(separator); + let value = &rest[1..]; + if !is_cookie_name(name) { + return TemplateCookieDecision::Bypass; + } + let keyed = key_names.iter().any(|item| item.as_bytes() == name); + if keyed && parsed.insert(name, value).is_some() { + return TemplateCookieDecision::Bypass; + } + if bypass_names.iter().any(|item| item.as_bytes() == name) { + return TemplateCookieDecision::Bypass; + } + if keyed { + if !is_cookie_value(value) { + return TemplateCookieDecision::Bypass; + } + } else if !independent || !is_ignored_cookie_value(value) { + return TemplateCookieDecision::Bypass; + } + } + } + + let mut ordered_names: Vec<&String> = key_names.iter().collect(); + ordered_names.sort_unstable(); + TemplateCookieDecision::Eligible( + ordered_names + .into_iter() + .map(|name| TemplateCookieValue { + name: name.clone(), + value: parsed.get(name.as_bytes()).map(|value| value.to_vec()), + }) + .collect(), + ) +} + +fn is_cookie_name(name: &[u8]) -> bool { + !name.is_empty() + && name + .iter() + .all(|byte| byte.is_ascii_alphanumeric() || b"!#$%&'*+-.^_`|~".contains(byte)) +} + +fn trim_pair(mut pair: &[u8]) -> &[u8] { + while pair + .first() + .is_some_and(|byte| matches!(byte, b' ' | b'\t')) + { + pair = &pair[1..]; + } + while pair.last().is_some_and(|byte| matches!(byte, b' ' | b'\t')) { + pair = &pair[..pair.len() - 1]; + } + pair +} + +// Unlisted cookies may use compact JSON or comma lists in real browsers. Keep +// framing checks even though their values do not enter the key. The independence +// assertion includes the origin parsing these ignored values as opaque: parsers +// that stop at nonstandard values cannot safely use this assertion. +fn is_ignored_cookie_value(value: &[u8]) -> bool { + let mut quoted = false; + for byte in value { + match byte { + b'"' => quoted = !quoted, + b',' => {} + 0x21 | 0x23..=0x2b | 0x2d..=0x3a | 0x3c..=0x5b | 0x5d..=0x7e => {} + _ => return false, + } + } + if quoted { + return false; + } + // Do not hide another cookie from origins that also split on commas. + !value.split(|byte| *byte == b',').skip(1).any(|part| { + part.iter() + .position(|byte| *byte == b'=') + .is_some_and(|separator| is_cookie_name(&part[..separator])) + }) +} + +fn is_cookie_value(value: &[u8]) -> bool { + let payload = if value.first() == Some(&b'"') { + if value.len() < 2 || value.last() != Some(&b'"') { + return false; + } + &value[1..value.len() - 1] + } else { + value + }; + payload + .iter() + .all(|byte| matches!(byte, 0x21 | 0x23..=0x2b | 0x2d..=0x3a | 0x3c..=0x5b | 0x5d..=0x7e)) +} + +#[cfg(test)] +mod tests { + use http::{HeaderMap, HeaderValue, header::COOKIE}; + + use super::*; + + fn names(values: &[&str]) -> Vec { + values.iter().map(|value| (*value).to_owned()).collect() + } + + fn headers(fields: &[&[u8]]) -> HeaderMap { + let mut headers = HeaderMap::new(); + for field in fields { + headers.append( + COOKIE, + HeaderValue::from_bytes(field).expect("should accept fixture bytes"), + ); + } + headers + } + + fn dimension(name: &str, value: Option<&[u8]>) -> TemplateCookieValue { + TemplateCookieValue { + name: name.to_owned(), + value: value.map(<[u8]>::to_vec), + } + } + + #[test] + fn template_cookie_policy_validates_names() { + for (key, bypass, valid) in [ + (vec![], vec![], true), + (vec!["AZaz09!#$%&'*+-.^_`|~"], vec!["session"], true), + (vec!["session"], vec!["Session"], true), + (vec!["ab_bucket", "ab_bucket"], vec![], false), + (vec![], vec!["session", "session"], false), + (vec!["session"], vec!["session"], false), + ] { + assert_eq!( + validate_cookie_names(&names(&key), &names(&bypass)).is_ok(), + valid, + "should validate exact names and list uniqueness" + ); + } + for invalid in [ + "", "a b", " a", "a ", "a=b", "a;b", "a,b", "a:b", "a/b", "a\\b", "a\"b", "a\tb", + "a\nb", "é", + ] { + for (key, bypass, field) in [ + (vec![invalid], vec![], "template_cache_key_cookies"), + (vec![], vec![invalid], "template_cache_bypass_cookies"), + ] { + let error = validate_cookie_names(&names(&key), &names(&bypass)) + .expect_err("should reject invalid cookie names"); + assert!( + error.contains(field) && error.contains(invalid), + "should identify the invalid configuration field and name" + ); + } + } + } + + #[test] + fn template_cookie_policy_rejects_identity_keys_but_allows_bypass() { + for name in [COOKIE_TS_EC, COOKIE_TS_EIDS, COOKIE_SHAREDID] { + let error = validate_cookie_names(&names(&[name]), &[]) + .expect_err("should reject identity cookie keys"); + assert!( + error.contains("template_cache_key_cookies") && error.contains(name), + "should identify the forbidden key cookie" + ); + assert!( + validate_cookie_names(&[], &names(&[name])).is_ok(), + "should allow identity cookie bypass policies" + ); + } + } + + #[test] + fn template_cookie_policy_allows_only_independent_unlisted_duplicates() { + for fields in [ + vec![b"a=1; a=1".as_slice()], + vec![b"a=1; a=2".as_slice()], + vec![b"a=1".as_slice(), b"a=2".as_slice()], + vec![br#"a={"value":1}; a={"value":2}"#.as_slice()], + ] { + for key in [vec![], names(&["ab_bucket"])] { + for independent in [false, true] { + let expected = if independent { + TemplateCookieDecision::Eligible( + key.iter().map(|name| dimension(name, None)).collect(), + ) + } else { + TemplateCookieDecision::Bypass + }; + assert_eq!( + evaluate_cookie_policy( + &headers(&fields), + &key, + &names(&["session"]), + independent + ), + expected, + "should ignore duplicates only with unlisted-cookie independence" + ); + } + } + for (key, bypass) in [(names(&["a"]), vec![]), (vec![], names(&["a"]))] { + assert_eq!( + evaluate_cookie_policy(&headers(&fields), &key, &bypass, true), + TemplateCookieDecision::Bypass, + "should bypass repeated key or bypass cookies" + ); + } + } + } + + #[test] + fn template_cookie_policy_legacy_boolean_matrix() { + for fields in [ + vec![], + vec![b"".as_slice()], + vec![b"broken".as_slice()], + vec![b"a=1; a=2".as_slice()], + vec![b"a=\xff".as_slice()], + vec![b"a=1".as_slice(), b"".as_slice()], + ] { + for independent in [false, true] { + let expected = if fields.is_empty() || independent { + TemplateCookieDecision::Eligible(vec![]) + } else { + TemplateCookieDecision::Bypass + }; + assert_eq!( + evaluate_cookie_policy(&headers(&fields), &[], &[], independent), + expected, + "should retain the legacy header-presence decision" + ); + } + } + } + + #[test] + fn template_cookie_policy_named_decision_matrix() { + for (key, bypass) in [ + (vec!["ab_bucket"], vec![]), + (vec![], vec!["session"]), + (vec!["ab_bucket"], vec!["session"]), + ] { + for independent in [false, true] { + for (field, expected_value, unknown, session) in [ + (None, None, false, false), + ( + Some("ab_bucket=A"), + Some(b"A".as_slice()), + key.is_empty(), + false, + ), + ( + Some("ab_bucket="), + Some(b"".as_slice()), + key.is_empty(), + false, + ), + (Some("ts-ec=reader"), None, true, false), + (Some("session="), None, true, true), + ( + Some("ab_bucket=A; session=login"), + Some(b"A".as_slice()), + true, + true, + ), + ( + Some("ab_bucket=A; ts-ec=reader"), + Some(b"A".as_slice()), + true, + false, + ), + (Some("Ab_bucket=B"), None, true, false), + (Some("Session=login"), None, true, false), + ] { + let fields = field + .map(|value| vec![value.as_bytes()]) + .unwrap_or_default(); + let expected = if (session && !bypass.is_empty()) || (unknown && !independent) { + TemplateCookieDecision::Bypass + } else { + TemplateCookieDecision::Eligible( + key.iter() + .map(|name| dimension(name, expected_value)) + .collect(), + ) + }; + assert_eq!( + evaluate_cookie_policy( + &headers(&fields), + &names(&key), + &names(&bypass), + independent + ), + expected, + "should apply list membership and scoped independence" + ); + } + } + } + } + + #[test] + fn template_cookie_policy_preserves_values_and_sorts_dimensions() { + let key = names(&["z", "ab_bucket", "A"]); + for fields in [ + vec![b"z=a=b%2F; ab_bucket=\"A\"; A=".as_slice()], + vec![ + b" \tA= \t; z=a=b%2F ".as_slice(), + b"ab_bucket=\"A\"".as_slice(), + ], + ] { + assert_eq!( + evaluate_cookie_policy(&headers(&fields), &key, &[], false), + TemplateCookieDecision::Eligible(vec![ + dimension("A", Some(b"")), + dimension("ab_bucket", Some(b"\"A\"")), + dimension("z", Some(b"a=b%2F")) + ]), + "should preserve representation and ignore pair order and splitting" + ); + } + let every_octet: &[u8] = + b"!#$%&'()*+-./0123456789:<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_`abcdefghijklmnopqrstuvwxyz{|}~"; + for value in [b"".as_slice(), b"\"\"", every_octet] { + let mut field = b"ab_bucket=".to_vec(); + field.extend_from_slice(value); + assert_eq!( + evaluate_cookie_policy(&headers(&[&field]), &names(&["ab_bucket"]), &[], false), + TemplateCookieDecision::Eligible(vec![dimension("ab_bucket", Some(value))]), + "should accept every cookie-octet and preserve quotes" + ); + } + } + + #[test] + fn template_cookie_policy_ignores_nonstandard_values_only_for_unlisted_cookies() { + for value in [ + r#"{"enabled":true,"nested":{"count":1}}"#, + "g=16/e:experiment,s:a,ex:123", + ] { + for field in [ + format!("ab_bucket=A; g_state={value}"), + format!("g_state={value}; ab_bucket=A"), + ] { + assert_eq!( + evaluate_cookie_policy( + &headers(&[field.as_bytes()]), + &names(&["ab_bucket"]), + &names(&["session"]), + true + ), + TemplateCookieDecision::Eligible(vec![dimension("ab_bucket", Some(b"A"))]), + "should ignore unrelated browser cookie values when independence is asserted" + ); + for (key, bypass, independent) in [ + (names(&["ab_bucket"]), names(&["session"]), false), + (names(&["ab_bucket", "g_state"]), names(&["session"]), true), + (names(&["ab_bucket"]), names(&["g_state"]), true), + ] { + assert_eq!( + evaluate_cookie_policy( + &headers(&[field.as_bytes()]), + &key, + &bypass, + independent + ), + TemplateCookieDecision::Bypass, + "should retain strict key values, bypass presence, and conservative independence" + ); + } + } + } + } + + #[test] + fn template_cookie_policy_rejects_ambiguous_ignored_values() { + for field in [ + r#"ignored=x,session=login; ab_bucket=A"#, + r#"ignored=x,ab_bucket=B; ab_bucket=A"#, + r#"ignored=x,other=value; ab_bucket=A"#, + r#"ignored=x session=login; ab_bucket=A"#, + r#"ignored="; ab_bucket=A"#, + r#"ignored={"value":"x;session=login"}; ab_bucket=A"#, + r#"ignored={"value":"x\y"}; ab_bucket=A"#, + ] { + assert_eq!( + evaluate_cookie_policy( + &headers(&[field.as_bytes()]), + &names(&["ab_bucket"]), + &names(&["session"]), + true + ), + TemplateCookieDecision::Bypass, + "should not tolerate values that obscure cookie boundaries" + ); + } + } + + #[test] + fn template_cookie_policy_rejects_malformed_and_duplicate_fields() { + for fields in [ + vec![b"".as_slice()], + vec![b" \t"], + vec![b"bare"], + vec![b"=value"], + vec![b"a=1;"], + vec![b";a=1"], + vec![b"a=1;;b=2"], + vec![b"ab_bucket =A"], + vec![b"ab_bucket= A"], + vec![b"a:b=1"], + vec![b"a=\""], + vec![b"a=\"x"], + vec![b"a=x\""], + vec![b"a=\"x\"y\""], + vec![b"a=x y"], + vec![b"a=x\ty"], + vec![b"ab_bucket=x,y"], + vec![b"a=x\\y"], + vec![b"a=\xff"], + vec![b"\xff=1"], + vec![b"ab_bucket=1; ab_bucket=1"], + vec![b"ab_bucket=1; ab_bucket=2"], + vec![b"ab_bucket=1", b"ab_bucket=1"], + vec![b"ab_bucket=1", b"ab_bucket=2"], + vec![b"ab_bucket=A", b""], + vec![b"ab_bucket=A", b"a=\xff"], + vec![b"ab_bucket=A", b"session="], + ] { + for independent in [false, true] { + assert_eq!( + evaluate_cookie_policy( + &headers(&fields), + &names(&["ab_bucket"]), + &names(&["session"]), + independent + ), + TemplateCookieDecision::Bypass, + "should reject ambiguous or malformed cookie input" + ); + } + assert_eq!( + evaluate_cookie_policy(&headers(&fields), &[], &[], true), + TemplateCookieDecision::Eligible(vec![]), + "should preserve legacy independent handling for the same malformed input" + ); + } + } +} diff --git a/crates/trusted-server-core/src/creative_opportunities.rs b/crates/trusted-server-core/src/creative_opportunities.rs index 2254c27d5..a4e62e9d7 100644 --- a/crates/trusted-server-core/src/creative_opportunities.rs +++ b/crates/trusted-server-core/src/creative_opportunities.rs @@ -331,21 +331,29 @@ pub struct CreativeOpportunitiesConfig { /// Spike-only. Same `Option` + `skip_serializing_if` reasoning as `assembly_mode`. #[serde(default, skip_serializing_if = "Option::is_none")] pub template_cache_max_age_seconds: Option, - /// Operator assertion that the origin's HTML does not depend on request cookies. + /// Named bounded cookie variants whose raw values enter the shared-template key. /// - /// Unset or `false` disqualifies **every cookie-bearing request** from the shared - /// template cache, in both directions. That is safe and it is also very nearly a - /// disable switch: Trusted Server sets its own identity cookie, so essentially every - /// repeat visitor carries one. Left at the default, the cache can only ever serve - /// first-ever page views and cookie-less clients. + /// Cookie names are case-sensitive. Use experiment arms or region buckets, never + /// session tokens or reader IDs. Missing cookies and present-empty values differ. + /// Unset or empty adds no cookie dimensions and preserves the legacy policy. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub template_cache_key_cookies: Option>, + /// Cookie names whose presence forces inline processing, with no lookup or store. /// - /// Setting `true` asserts the origin serves the same HTML with or without cookies. - /// It is not taken on trust alone — if the origin ever declares `Vary: Cookie`, the - /// response is refused regardless of this flag or the configured key. So a wrong - /// assertion is caught whenever the origin is honest about it, and this only widens - /// the window where the origin personalizes *silently*. + /// Empty values still count as present. Names must be unique and must not overlap + /// [`Self::template_cache_key_cookies`]. Unset or empty means no bypass cookies. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub template_cache_bypass_cookies: Option>, + /// Assertion that cookies outside the key and bypass lists do not affect origin HTML. /// - /// Spike-only. Same `Option` + `skip_serializing_if` reasoning as `assembly_mode`. + /// Defaults to false: any unlisted cookie disqualifies both lookup and storage. + /// With both lists empty, this retains the original all-cookie behavior. TS mints + /// its own identity cookie, so most repeat visitors bypass unless the operator can + /// safely assert independence. No identity or consent cookie is implicitly exempt. + /// + /// Named bypass cookies always disqualify, and origin `Vary: Cookie` always refuses + /// storage regardless of this assertion or the configured key. The origin must + /// still authorize positive shared freshness and pass every other response guard. #[serde(default, skip_serializing_if = "Option::is_none")] pub origin_is_cookie_independent: Option, /// Slot templates. An empty vec or `enabled = false` disables template delivery. @@ -360,16 +368,31 @@ impl CreativeOpportunitiesConfig { self.assembly_mode.unwrap_or_default() } - /// Whether a cookie-bearing request may participate in the shared cache. + /// Whether unlisted request cookies are asserted irrelevant to origin HTML. /// - /// Defaults to `false`, which is the conservative reading and also the one that - /// makes the cache almost inert on real traffic. See - /// [`Self::origin_is_cookie_independent`]. + /// Defaults to false. Named key cookies remain variant dimensions, and named + /// bypass cookies remain disqualifying regardless of this assertion. #[must_use] pub fn origin_is_cookie_independent(&self) -> bool { self.origin_is_cookie_independent.unwrap_or(false) } + /// Exact cookie names included as bounded shared-template key dimensions. + #[must_use] + pub fn template_cache_key_cookies(&self) -> &[String] { + self.template_cache_key_cookies + .as_deref() + .unwrap_or_default() + } + + /// Exact cookie names whose presence disqualifies shared-template caching. + #[must_use] + pub fn template_cache_bypass_cookies(&self) -> &[String] { + self.template_cache_bypass_cookies + .as_deref() + .unwrap_or_default() + } + /// Headers the cache key covers, per operator config. /// /// Unset yields an empty operator spec, so any origin `Vary` other than the @@ -455,11 +478,16 @@ impl CreativeOpportunitiesConfig { /// Returns an error string when [`gam_network_id`](Self::gam_network_id) is /// blank but consumed by a default path or `{network_id}` template; when a /// slot has an invalid identifier, page pattern set, format list, or - /// dimensions; when `template_cache_max_age_seconds` falls outside 1–86,400; + /// dimensions; when cookie policy names are invalid, duplicated, or overlapping; + /// when `template_cache_max_age_seconds` falls outside 1–86,400; /// when a `{section}` template lacks a valid /// [`section_root`](Self::section_root); or when configured values make a /// dynamic path exceed 100 UTF-8 bytes. pub fn validate_runtime(&self) -> Result<(), String> { + crate::cookies::template_cache_policy::validate_cookie_names( + self.template_cache_key_cookies(), + self.template_cache_bypass_cookies(), + )?; if self .template_cache_max_age_seconds .is_some_and(|seconds| !(1..=MAX_TEMPLATE_CACHE_MAX_AGE_SECONDS).contains(&seconds)) @@ -1354,6 +1382,8 @@ mod tests { assembly_mode: None, template_cache_vary: None, template_cache_max_age_seconds: None, + template_cache_key_cookies: None, + template_cache_bypass_cookies: None, origin_is_cookie_independent: None, section_segment: None, slot: vec![slot], @@ -1756,6 +1786,8 @@ mod tests { assembly_mode: None, template_cache_vary: None, template_cache_max_age_seconds: None, + template_cache_key_cookies: None, + template_cache_bypass_cookies: None, origin_is_cookie_independent: None, section_segment: None, slot: Vec::new(), @@ -2196,6 +2228,83 @@ mod tests { } } + #[test] + fn template_cookie_config_accepts_independent_lists_and_preserves_omission() { + for policy in [ + "", + "template_cache_key_cookies = []\ntemplate_cache_bypass_cookies = []", + "template_cache_key_cookies = [\"ab_bucket\"]", + "template_cache_bypass_cookies = [\"session\"]", + "template_cache_key_cookies = [\"ab_bucket\"]\ntemplate_cache_bypass_cookies = []", + "template_cache_key_cookies = []\ntemplate_cache_bypass_cookies = [\"session\"]", + "template_cache_key_cookies = [\"ab_bucket\", \"Session\"]\ntemplate_cache_bypass_cookies = [\"session\"]", + ] { + let config: CreativeOpportunitiesConfig = + toml::from_str(&format!("gam_network_id = \"99999\"\n{policy}")) + .expect("should deserialize optional cookie policies"); + config + .validate_runtime() + .expect("should accept valid cookie names"); + assert!( + !config.origin_is_cookie_independent(), + "should retain conservative default" + ); + let serialized = serde_json::to_value(&config).expect("should serialize configuration"); + for field in [ + "template_cache_key_cookies", + "template_cache_bypass_cookies", + ] { + assert_eq!( + serialized.get(field).is_some(), + policy.contains(field), + "should preserve omitted fields" + ); + } + } + } + + #[test] + fn template_cookie_config_rejects_invalid_duplicate_and_overlapping_names() { + for field in [ + "template_cache_key_cookies", + "template_cache_bypass_cookies", + ] { + for names in [ + vec![""], + vec!["bad name"], + vec!["a=b"], + vec!["a;b"], + vec!["é"], + vec!["a\t"], + vec!["a", "a"], + ] { + let value = serde_json::json!({"gam_network_id": "99999", (field): names}); + let config: CreativeOpportunitiesConfig = serde_json::from_value(value) + .expect("should deserialize names before validation"); + let error = config + .validate_runtime() + .expect_err("should reject invalid or repeated cookie names"); + assert!( + error.contains(field), + "should identify invalid policy field" + ); + } + } + let config: CreativeOpportunitiesConfig = serde_json::from_value(serde_json::json!({ + "gam_network_id": "99999", + "template_cache_key_cookies": ["session"], + "template_cache_bypass_cookies": ["session"] + })) + .expect("should deserialize overlapping lists before validation"); + assert!( + config + .validate_runtime() + .expect_err("should reject overlapping policies") + .contains("session"), + "should identify overlapping name" + ); + } + #[test] fn template_cache_max_age_accepts_a_positive_value_up_to_one_day() { for seconds in [1_u32, 1_200, 86_400] { diff --git a/crates/trusted-server-core/src/platform/mod.rs b/crates/trusted-server-core/src/platform/mod.rs index 2553229a4..1ea37f339 100644 --- a/crates/trusted-server-core/src/platform/mod.rs +++ b/crates/trusted-server-core/src/platform/mod.rs @@ -69,8 +69,8 @@ pub use template_cache::{ PlatformTemplateCache, PlatformTemplateCacheReservation, REPLAYABLE_POLICY_HEADERS, TEMPLATE_CACHE_PURGE_ALL_SURROGATE_KEY, TEMPLATE_SCHEMA_VERSION, TemplateCacheError, TemplateCacheKey, TemplateCacheLookup, TemplateCacheMiss, TemplateCacheReservation, - TemplateEntry, TemplateMetadata, TemplateMetadataEncodeError, UnavailableTemplateCache, - VaryHeaderValues, VarySpec, + TemplateCookieValue, TemplateEntry, TemplateMetadata, TemplateMetadataEncodeError, + UnavailableTemplateCache, VaryHeaderValues, VarySpec, }; pub use traits::{PlatformBackend, PlatformConfigStore, PlatformGeo, PlatformSecretStore}; pub use types::{ diff --git a/crates/trusted-server-core/src/platform/template_cache.rs b/crates/trusted-server-core/src/platform/template_cache.rs index e3bbc42da..574f8d4a2 100644 --- a/crates/trusted-server-core/src/platform/template_cache.rs +++ b/crates/trusted-server-core/src/platform/template_cache.rs @@ -67,6 +67,8 @@ pub struct TemplateCacheKey { /// order the origin listed them. Not a fixed list: the origin is authoritative, /// and hard-coding one here would silently drift when the origin's changes. pub vary_values: Vec, + /// Bounded cookie variants, sorted by exact case-sensitive name. Never reader IDs. + pub cookie_values: Vec, /// Digest of every setting that can shape the transformed template plus the tsjs /// bundle. Over-invalidating is safe; omitting a shaping input cross-serves bytes. pub template_fingerprint: String, @@ -121,6 +123,24 @@ impl TemplateCacheKey { } } + if !self.cookie_values.is_empty() { + push(&mut canonical, b"cookie-variants-v1"); + push( + &mut canonical, + &(self.cookie_values.len() as u64).to_be_bytes(), + ); + for cookie in &self.cookie_values { + push(&mut canonical, cookie.name.as_bytes()); + match &cookie.value { + None => push(&mut canonical, b"absent"), + Some(value) => { + push(&mut canonical, b"present"); + push(&mut canonical, value); + } + } + } + } + let digest = sha2::Sha256::digest(canonical); format!( "ts-template-cache-v{}-{}", @@ -156,6 +176,27 @@ fn digest_hex(bytes: &[u8]) -> String { hex::encode(sha2::Sha256::digest(bytes)) } +/// One bounded cookie variant selecting a reader-neutral template. +/// +/// Names are case-sensitive. Values preserve raw bytes and quotes; `None` is absent, +/// while `Some(Vec::new())` is present-empty. Debug output deliberately omits values. +#[derive(Clone, PartialEq, Eq)] +pub struct TemplateCookieValue { + /// Exact configured cookie name. + pub name: String, + /// Raw cookie value, or `None` when absent. + pub value: Option>, +} + +impl fmt::Debug for TemplateCookieValue { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("TemplateCookieValue") + .field("name", &self.name) + .field("present", &self.value.is_some()) + .finish_non_exhaustive() + } +} + /// One configured `Vary` input exactly as it appeared on the request. /// /// `None` means absent. `Some(vec![vec![]])` means present with one empty field @@ -743,6 +784,7 @@ mod tests { name: "rsc".to_string(), values: Some(vec![b"1".to_vec()]), }], + cookie_values: Vec::new(), template_fingerprint: "abc123".to_string(), schema_version: TEMPLATE_SCHEMA_VERSION, } @@ -893,6 +935,70 @@ mod tests { } } + #[test] + fn template_cookie_key_separates_names_presence_and_raw_values() { + let base = key(); + let variants = [ + ("ab_bucket", None), + ("ab_bucket", Some(b"".as_slice())), + ("ab_bucket", Some(b"A".as_slice())), + ("ab_bucket", Some(b"B".as_slice())), + ("ab_bucket", Some(b"\"A\"".as_slice())), + ("AB_bucket", Some(b"A".as_slice())), + ("ab", Some(b"bucketA".as_slice())), + ]; + let mut keys = HashSet::from([base.to_cache_key()]); + for (name, value) in variants { + let mut variant = base.clone(); + variant.cookie_values.push(TemplateCookieValue { + name: name.to_string(), + value: value.map(<[u8]>::to_vec), + }); + assert!( + keys.insert(variant.to_cache_key()), + "should distinguish cookie dimensions" + ); + assert_eq!( + variant.surrogate_keys(), + base.surrogate_keys(), + "should purge all URL variants together" + ); + } + let mut combined = base.clone(); + combined.cookie_values = vec![ + TemplateCookieValue { + name: "ab_bucket".to_string(), + value: Some(b"A".to_vec()), + }, + TemplateCookieValue { + name: "region".to_string(), + value: Some(b"west".to_vec()), + }, + ]; + assert!( + keys.insert(combined.to_cache_key()), + "should include every cookie dimension" + ); + combined.cookie_values[1].value = Some(b"east".to_vec()); + assert!( + keys.insert(combined.to_cache_key()), + "should distinguish secondary variants" + ); + } + + #[test] + fn template_cookie_key_debug_redacts_values() { + let cookie = TemplateCookieValue { + name: "ab_bucket".to_string(), + value: Some(b"private-value".to_vec()), + }; + assert_eq!( + format!("{cookie:?}"), + "TemplateCookieValue { name: \"ab_bucket\", present: true, .. }", + "should expose no cookie values in diagnostics" + ); + } + #[test] fn vary_header_names_are_matched_case_insensitively() { let mut upper = key(); diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index 05daf0b4e..693e4e011 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -57,6 +57,7 @@ use crate::cache_policy::{ use crate::consent::{consent_allows_server_side_auction, gate_eids_by_consent}; use crate::constants::{COOKIE_TS_EIDS, HEADER_X_COMPRESS_HINT}; use crate::cookies::handle_request_cookies; +use crate::cookies::template_cache_policy::{TemplateCookieDecision, evaluate_cookie_policy}; use crate::creative_opportunities::{AssemblyMode, CreativeOpportunitiesConfig}; use crate::ec::EcContext; use crate::ec::kv::KvIdentityGraph; @@ -4280,20 +4281,33 @@ pub async fn handle_publisher_request( .is_none_or(|marker| !marker.matches(req.headers())), _ => true, }; - let request_had_cookie = req.headers().contains_key(header::COOKIE); - // Whether carrying a cookie is itself disqualifying. Computed once and used for both - // the lookup and the store, so the two cannot drift apart. - // - // The conservative default disqualifies every cookie-bearing request, which is very - // nearly a disable switch — TS sets its own identity cookie, so essentially every - // repeat visitor carries one. An operator who knows their origin ignores cookies can - // say so; the `Vary: Cookie` drift guard still refuses the response if the origin - // ever contradicts them. - let cookie_disqualifies = request_had_cookie - && !settings - .creative_opportunities - .as_ref() - .is_some_and(CreativeOpportunitiesConfig::origin_is_cookie_independent); + // Classify cookies once before the origin consumes the request. The same decision + // governs lookup and storage, so a bypass cookie can never read a warm template. + let (key_cookie_names, bypass_cookie_names, cookie_independent) = settings + .creative_opportunities + .as_ref() + .map_or((&[][..], &[][..], false), |config| { + ( + config.template_cache_key_cookies(), + config.template_cache_bypass_cookies(), + config.origin_is_cookie_independent(), + ) + }); + let (cookie_disqualifies, cookie_values) = match evaluate_cookie_policy( + req.headers(), + key_cookie_names, + bypass_cookie_names, + cookie_independent, + ) { + TemplateCookieDecision::Bypass => (true, Vec::new()), + TemplateCookieDecision::Eligible(values) => (false, values), + }; + if cookie_disqualifies && matches!(assembly_mode, AssemblyMode::Esi) { + log::debug!( + "template_cache bypass: {}", + TemplateCacheBypassReason::CookiePolicy + ); + } let suppress_datadome_client_side_tag = req .extensions() .get::() @@ -4380,6 +4394,7 @@ pub async fn handle_publisher_request( .map(CreativeOpportunitiesConfig::template_cache_vary) .unwrap_or_else(|| VarySpec::new([])) .values_from(req.headers()), + cookie_values, template_fingerprint: template_fingerprint(settings), schema_version: crate::platform::TEMPLATE_SCHEMA_VERSION, }); @@ -5710,15 +5725,12 @@ pub(crate) enum TemplateCacheBypassReason { /// `Content-Encoding` while returning the untouched bytes on the fallback route. #[display("origin content encoding is not supported by the template transform")] UnsupportedContentEncoding, - /// The request carried a `Cookie`, which TS forwards to origin unchanged — there - /// is no `Cookie` strip on the publisher path. Cookie-personalized HTML is - /// therefore cross-servable unless the origin declares `Vary: Cookie` or marks - /// those responses private, and a response can be personalized without carrying - /// `Set-Cookie` itself when the session was established earlier. Named in §4 of - /// the design doc; disqualifying until the origin's `Vary` is verified to cover - /// it. - #[display("request carried Cookie and the origin's Vary does not cover it")] - CookieForwarded, + /// The prepared request's cookies are disqualified by the configured policy. + /// + /// Named bypass cookies, unlisted cookies without an independence assertion, + /// or ambiguous input under a named policy prohibit both lookup and storage. + #[display("request cookies disqualified by template cache policy")] + CookiePolicy, /// The origin varies on a header the cache key does not cover. /// /// The key is built *before* the fetch from a configured [`VarySpec`], because a @@ -6142,7 +6154,7 @@ fn template_cache_ttl( return Err(TemplateCacheBypassReason::AuthorizedRequest); } if cookie_disqualifies { - return Err(TemplateCacheBypassReason::CookieForwarded); + return Err(TemplateCacheBypassReason::CookiePolicy); } if response_headers.contains_key(header::SET_COOKIE) { return Err(TemplateCacheBypassReason::OriginSetCookie); @@ -8892,6 +8904,7 @@ mod tests { origin_identity: "https://origin.example.com\0origin.example.com".to_string(), assembly_mode: AssemblyMode::Esi, vary_values: vec![], + cookie_values: Vec::new(), template_fingerprint: "fp".to_string(), schema_version: crate::platform::TEMPLATE_SCHEMA_VERSION, } @@ -9346,7 +9359,7 @@ mod tests { /// Name of the bidding test double, matched by `[auction].providers`. const STUB_BIDDER: &str = "stub-bidder"; - /// The CPM the stub bids. Chosen so its price bucket (`"3.50"`) is a distinctive + /// The default CPM the stub bids. Its price bucket (`"3.50"`) is a distinctive /// string that cannot appear in the fixture page by accident. const STUB_BID_CPM: f64 = 3.5; @@ -9355,7 +9368,9 @@ mod tests { /// Every other fixture in this file leaves the orchestrator with no providers, so /// every auction resolves to an empty bid map. That is exactly why a defect that /// discarded *non-empty* maps survived: no test ever produced one. - struct WinningBidProvider; + struct WinningBidProvider { + price: f64, + } #[async_trait::async_trait(?Send)] impl crate::auction::provider::AuctionProvider for WinningBidProvider { @@ -9398,7 +9413,7 @@ mod tests { STUB_BIDDER, vec![Bid { slot_id: "test-slot".to_string(), - price: Some(STUB_BID_CPM), + price: Some(self.price), currency: "USD".to_string(), creative: None, adomain: None, @@ -9505,9 +9520,19 @@ mod tests { settings: &Arc, services: &RuntimeServices, request: Request, + ) -> Response { + run_bidding_at_price(settings, services, request, STUB_BID_CPM).await + } + + /// [`run_bidding`], with a distinct winning CPM for this request. + async fn run_bidding_at_price( + settings: &Arc, + services: &RuntimeServices, + request: Request, + price: f64, ) -> Response { let mut orchestrator = AuctionOrchestrator::new(settings.auction.clone()); - orchestrator.register_provider(Arc::new(WinningBidProvider)); + orchestrator.register_provider(Arc::new(WinningBidProvider { price })); run_with_orchestrator( settings, services, @@ -9549,12 +9574,31 @@ mod tests { .await .expect("should proxy publisher request"); + finalize_test_publisher_response( + publisher_response, + settings, + services, + ®istry, + orchestrator, + finalizer, + ) + .await + } + + async fn finalize_test_publisher_response( + publisher_response: PublisherResponse, + settings: &Arc, + services: &RuntimeServices, + registry: &IntegrationRegistry, + orchestrator: Arc, + finalizer: Finalizer, + ) -> Response { match finalizer { Finalizer::Streaming => publisher_response_into_streaming_response( publisher_response, &Method::GET, Arc::clone(settings), - ®istry, + registry, orchestrator, services.clone(), ) @@ -9564,7 +9608,7 @@ mod tests { publisher_response, &Method::GET, settings, - ®istry, + registry, &orchestrator, services, ) @@ -11478,6 +11522,990 @@ mod tests { .expect("should build cookie-bearing request") } + fn cookie_policy_settings( + key_names: Option<&[&str]>, + bypass_names: Option<&[&str]>, + independent: bool, + ) -> Arc { + let mut settings = settings_with_mode("esi"); + let config = settings + .creative_opportunities + .as_mut() + .expect("should configure opportunities"); + config.template_cache_key_cookies = + key_names.map(|names| names.iter().map(|name| (*name).to_string()).collect()); + config.template_cache_bypass_cookies = + bypass_names.map(|names| names.iter().map(|name| (*name).to_string()).collect()); + config.origin_is_cookie_independent = Some(independent); + config + .validate_runtime() + .expect("should validate cookie policy fixture"); + Arc::new(settings) + } + + fn cookie_policy_request(fields: &[&[u8]]) -> Request { + let mut request = navigation_request(); + for field in fields { + request.headers_mut().append( + header::COOKIE, + HeaderValue::from_bytes(field).expect("should build cookie field"), + ); + } + request + } + + // Exercise raw fields at the prepared-request boundary. Ordinary diagnostics + // preparation removes invalid fields/empty pairs before generic cookie handling. + fn prepared_cookie_policy_request(fields: &[&[u8]]) -> Request { + let mut request = cookie_policy_request(fields); + request.extensions_mut().insert( + crate::integrations::gpt_diagnostics::GptDiagnosticsRequestDecision::default(), + ); + request + } + + #[tokio::test] + async fn template_cookie_publisher_key_only_admits_listed_cookies() { + for unused in [None, Some([].as_slice())] { + let settings = cookie_policy_settings(Some(&["ab_bucket"]), unused, false); + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + queue_shareable_html(&stub); + queue_shareable_html(&stub); + for _ in 0..2 { + let _ = body_of( + run( + &settings, + &services, + cookie_policy_request(&[b"ab_bucket=A"]), + ) + .await, + ) + .await; + } + assert_eq!( + stub.recorded_request_uris().len(), + 1, + "should share a listed-only variant even with independence false" + ); + assert_eq!( + stored_cache_keys(&cache).len(), + 1, + "should store one variant" + ); + let lookups = looked_up_cache_keys(&cache).len(); + let _ = body_of( + run( + &settings, + &services, + cookie_policy_request(&[b"ab_bucket=A; unknown=1"]), + ) + .await, + ) + .await; + assert_eq!( + looked_up_cache_keys(&cache).len(), + lookups, + "should bypass for any unlisted cookie" + ); + assert_eq!( + stored_cache_keys(&cache).len(), + 1, + "should not store an unlisted-cookie response" + ); + assert_eq!( + stub.recorded_request_uris().len(), + 2, + "should fetch unlisted-cookie origin HTML" + ); + } + } + + #[tokio::test] + async fn template_cookie_publisher_session_bypasses_warm_and_cold_cache() { + for finalizer in [Finalizer::Streaming, Finalizer::Buffered] { + for unused in [None, Some([].as_slice()), Some(["ab_bucket"].as_slice())] { + for warm in [false, true] { + let settings = cookie_policy_settings(unused, Some(&["session"]), true); + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + if warm { + queue_shareable_html(&stub); + let _ = body_of( + run_via( + &settings, + &services, + cookie_policy_request(&[b"ab_bucket=A; ts-ec=reader"]), + finalizer, + ) + .await, + ) + .await; + assert_eq!( + stored_cache_keys(&cache).len(), + 1, + "should warm an anonymous template first" + ); + } + let lookups = looked_up_cache_keys(&cache).len(); + let stores = stored_cache_keys(&cache).len(); + for session in [b"session=".as_slice(), b"session=token".as_slice()] { + stub.push_response_with_headers( + 200, + b"personal-account".to_vec(), + vec![ + ("content-type", "text/html"), + ("cache-control", "public, max-age=300"), + ], + ); + let response = run_via( + &settings, + &services, + cookie_policy_request(&[b"ab_bucket=A; ts-ec=reader", session]), + finalizer, + ) + .await; + assert_eq!( + response.headers()[HEADER_X_TS_TEMPLATE_CACHE], + "bypass-request", + "should report request bypass" + ); + assert!( + !response.headers().contains_key(HEADER_X_TS_ASSEMBLY), + "should omit shared-assembly diagnostics on inline responses" + ); + assert!( + String::from_utf8(body_of(response).await) + .expect("should decode HTML") + .contains("personal-account"), + "should render this request's origin HTML" + ); + } + assert_eq!( + looked_up_cache_keys(&cache).len(), + lookups, + "should never look up or reserve for session requests" + ); + assert_eq!( + stored_cache_keys(&cache).len(), + stores, + "should never store session HTML" + ); + assert_eq!( + stub.recorded_request_uris().len(), + usize::from(warm) + 2, + "should fetch every session request" + ); + assert!( + cache + .entries + .lock() + .expect("should lock templates") + .values() + .all(|entry| !String::from_utf8_lossy(&entry.body) + .contains("personal-account")), + "should keep personal bytes out of templates" + ); + } + } + } + } + + #[tokio::test] + async fn template_cookie_publisher_downstream_variants_remain_separate() { + for finalizer in [Finalizer::Streaming, Finalizer::Buffered] { + let mut settings = + cookie_policy_settings(Some(&["ab_bucket"]), Some(&["session"]), true); + Arc::make_mut(&mut settings) + .creative_opportunities + .as_mut() + .expect("should configure opportunities") + .template_cache_vary = Some(vec!["x-exp-variant".to_string()]); + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + for arm in ["A", "B"] { + stub.push_response_with_headers( + 200, + format!("arm-{arm}").into_bytes(), + vec![ + ("content-type", "text/html"), + ("cache-control", "public, max-age=300"), + ("vary", "X-Exp-Variant"), + ], + ); + } + for (index, arm) in ["A", "B", "A", "B"].iter().enumerate() { + let cookies = format!( + "ab_bucket={arm}; ts-ec=reader{index}; ignored=scope{index}; ignored=other{index}" + ); + let request = cookie_policy_request(&[cookies.as_bytes()]); + assert!( + !request.headers().contains_key("x-exp-variant"), + "should reproduce downstream-only header topology" + ); + let response = run_via(&settings, &services, request, finalizer).await; + assert_eq!( + response.headers()[HEADER_X_TS_TEMPLATE_CACHE], + if index < 2 { "miss-stored" } else { "hit" }, + "should share within each arm" + ); + assert!( + response.headers()[header::CACHE_CONTROL] + .to_str() + .expect("should read cache control") + .contains("private"), + "should keep assembled output private" + ); + let html = + String::from_utf8(body_of(response).await).expect("should decode HTML"); + assert!( + html.contains(&format!("arm-{arm}")), + "should render the correct experiment arm" + ); + assert!( + !html.contains(if *arm == "A" { "arm-B" } else { "arm-A" }), + "should never cross-serve arms" + ); + } + assert_eq!( + stub.recorded_request_uris().len(), + 2, + "should skip origin on both warm arms" + ); + let stored = stored_cache_keys(&cache); + assert_eq!(stored.len(), 2, "should store two variants"); + assert_ne!(stored[0], stored[1], "should distinguish arms in the key"); + } + } + + #[tokio::test] + async fn template_cookie_publisher_absent_and_empty_are_separate() { + let settings = cookie_policy_settings(Some(&["ab_bucket"]), None, true); + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + for variant in ["absent", "empty"] { + stub.push_response_with_headers( + 200, + format!("variant-{variant}") + .into_bytes(), + vec![ + ("content-type", "text/html"), + ("cache-control", "public, max-age=300"), + ], + ); + } + for (field, expected) in [ + (b"ts-ec=reader1".as_slice(), "absent"), + (b"ab_bucket=".as_slice(), "empty"), + (b"ts-ec=reader2".as_slice(), "absent"), + (b"ab_bucket=; ts-ec=reader3".as_slice(), "empty"), + ] { + let html = String::from_utf8( + body_of(run(&settings, &services, cookie_policy_request(&[field])).await).await, + ) + .expect("should decode HTML"); + assert!( + html.contains(&format!("variant-{expected}")), + "should preserve absent versus empty variants" + ); + } + assert_eq!( + stored_cache_keys(&cache).len(), + 2, + "should store both presence variants" + ); + assert_eq!( + stub.recorded_request_uris().len(), + 2, + "should hit both variants without origin" + ); + } + + #[tokio::test] + async fn template_cookie_publisher_response_guards_remain_effective() { + for guard in [ + vec![("vary", "Cookie")], + vec![("vary", "X-Exp-Variant"), ("vary", "cOoKiE")], + vec![("vary", "X-Exp-Variant, Cookie")], + vec![("vary", "*")], + vec![("vary", "uncovered-header")], + vec![("set-cookie", "origin=value")], + ] { + let mut settings = + cookie_policy_settings(Some(&["ab_bucket"]), Some(&["session"]), true); + Arc::make_mut(&mut settings) + .creative_opportunities + .as_mut() + .expect("should configure opportunities") + .template_cache_vary = Some(vec!["x-exp-variant".to_string()]); + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + let mut response_headers = vec![ + ("content-type", "text/html"), + ("cache-control", "public, max-age=300"), + ]; + response_headers.extend(guard); + for _ in 0..2 { + stub.push_response_with_headers( + 200, + b"origin".to_vec(), + response_headers.clone(), + ); + let response = run( + &settings, + &services, + cookie_policy_request(&[b"ab_bucket=A"]), + ) + .await; + assert_eq!( + response.headers()[HEADER_X_TS_TEMPLATE_CACHE], + "bypass-response", + "should preserve origin response restrictions" + ); + let _ = body_of(response).await; + } + assert!( + stored_cache_keys(&cache).is_empty(), + "should never store a disqualified response" + ); + assert_eq!( + stub.recorded_request_uris().len(), + 2, + "should fetch each disqualified response" + ); + } + } + + #[tokio::test] + async fn template_cookie_publisher_malformed_later_fields_and_duplicates_bypass() { + for fields in [ + vec![b"ab_bucket=A".as_slice(), b"unknown=\xff".as_slice()], + vec![b"ab_bucket=A".as_slice(), b"ab_bucket=B".as_slice()], + vec![b"ab_bucket=A; ab_bucket=A".as_slice()], + vec![b"ab_bucket=A; broken".as_slice()], + vec![b"ab_bucket=A;".as_slice()], + vec![b"ab_bucket=A".as_slice(), b"".as_slice()], + ] { + for warm in [false, true] { + let settings = cookie_policy_settings(Some(&["ab_bucket"]), None, true); + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + if warm { + queue_shareable_html(&stub); + let _ = body_of( + run( + &settings, + &services, + cookie_policy_request(&[b"ab_bucket=A"]), + ) + .await, + ) + .await; + assert_eq!( + stored_cache_keys(&cache).len(), + 1, + "should warm cache before malformed input" + ); + } + let lookups = looked_up_cache_keys(&cache).len(); + let stores = stored_cache_keys(&cache).len(); + queue_shareable_html(&stub); + let response = run( + &settings, + &services, + prepared_cookie_policy_request(&fields), + ) + .await; + assert_eq!( + response.headers()[HEADER_X_TS_TEMPLATE_CACHE], + "bypass-request", + "should bypass malformed fields reaching the evaluator" + ); + let _ = body_of(response).await; + assert_eq!( + looked_up_cache_keys(&cache).len(), + lookups, + "should never look up ambiguous input" + ); + assert_eq!( + stored_cache_keys(&cache).len(), + stores, + "should never store ambiguous input" + ); + assert_eq!( + stub.recorded_request_uris().len(), + usize::from(warm) + 1, + "should forward requests accepted by earlier validation" + ); + } + } + } + + #[tokio::test] + async fn template_cookie_publisher_selected_invalid_header_keeps_existing_error() { + let settings = cookie_policy_settings(Some(&["ab_bucket"]), None, true); + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + let orchestrator = AuctionOrchestrator::new(settings.auction.clone()); + let mut ec_context = + EcContext::new_for_test(None, crate::consent::ConsentContext::default()); + let error = handle_publisher_request( + &settings, + &services, + None, + &mut ec_context, + AuctionDispatch { + orchestrator: &orchestrator, + slots: &[article_slot()], + registry: None, + }, + prepared_cookie_policy_request(&[b"ab_bucket=\xff"]), + EdgeCacheHeader::SMaxageFallback, + ) + .await + .err() + .expect("should retain the existing selected-header error"); + assert!( + matches!( + error.current_context(), + TrustedServerError::InvalidHeaderValue { .. } + ), + "should preserve the existing error type" + ); + assert!( + looked_up_cache_keys(&cache).is_empty(), + "should not reach shared lookup" + ); + assert!( + stored_cache_keys(&cache).is_empty(), + "should not store invalid requests" + ); + assert!( + stub.recorded_request_uris().is_empty(), + "should retain earlier rejection before origin" + ); + } + + #[tokio::test] + async fn template_cookie_publisher_ignores_json_without_changing_origin_cookies() { + for finalizer in [Finalizer::Streaming, Finalizer::Buffered] { + let settings = + cookie_policy_settings(Some(&["ab_bucket"]), Some(&["session"]), true); + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + let cold_cookie = r#"ab_bucket=A; g_state={"enabled":true,"count":1}"#; + queue_shareable_html(&stub); + for (cookie, state) in [ + (cold_cookie, "miss-stored"), + (r#"ab_bucket=A; g_state={"enabled":false,"count":2}"#, "hit"), + ] { + let response = run_via( + &settings, + &services, + cookie_policy_request(&[cookie.as_bytes()]), + finalizer, + ) + .await; + assert_eq!( + response.headers()[HEADER_X_TS_TEMPLATE_CACHE], + state, + "should share the template despite unrelated JSON cookie changes" + ); + let _ = body_of(response).await; + } + let forwarded = stub.recorded_request_headers(); + assert_eq!( + forwarded.len(), + 1, + "should fetch origin only for the cold request" + ); + assert_eq!( + forwarded[0] + .iter() + .filter(|(name, _)| name.eq_ignore_ascii_case(header::COOKIE.as_str())) + .map(|(_, value)| value.as_str()) + .collect::>(), + [cold_cookie], + "should preserve the ignored cookie when forwarding to origin" + ); + let lookups = looked_up_cache_keys(&cache).len(); + queue_shareable_html(&stub); + let response = run_via( + &settings, + &services, + cookie_policy_request(&[br#"ab_bucket=A; g_state={"enabled":true}; session="#]), + finalizer, + ) + .await; + assert_eq!( + response.headers()[HEADER_X_TS_TEMPLATE_CACHE], + "bypass-request", + "should honor bypass cookie presence alongside ignored JSON" + ); + let _ = body_of(response).await; + assert_eq!( + looked_up_cache_keys(&cache).len(), + lookups, + "should skip shared lookup for session requests" + ); + assert_eq!( + stored_cache_keys(&cache).len(), + 1, + "should not store session responses" + ); + } + } + + #[tokio::test] + async fn template_cookie_publisher_uses_cookies_after_existing_preparation() { + let settings = cookie_policy_settings(Some(&["ab_bucket"]), None, false); + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + queue_shareable_html(&stub); + // Normal preparation strips invalid fields and empty pairs before origin + // forwarding. Preserve it; the cache policy sees the prepared request. + let cold = run( + &settings, + &services, + cookie_policy_request(&[b"ab_bucket=A;", b"unknown=\xff"]), + ) + .await; + assert_eq!( + cold.headers()[HEADER_X_TS_TEMPLATE_CACHE], + "miss-stored", + "should classify the cookies actually forwarded" + ); + let _ = body_of(cold).await; + let warm = run( + &settings, + &services, + cookie_policy_request(&[b"ab_bucket=A"]), + ) + .await; + assert_eq!( + warm.headers()[HEADER_X_TS_TEMPLATE_CACHE], + "hit", + "should share identical prepared origin inputs" + ); + let _ = body_of(warm).await; + assert_eq!( + stub.recorded_request_uris().len(), + 1, + "should preserve existing request preparation" + ); + let forwarded = stub.recorded_request_headers(); + assert_eq!(forwarded.len(), 1, "should record one origin request"); + let forwarded_cookies = forwarded[0] + .iter() + .filter(|(name, _)| name.eq_ignore_ascii_case(header::COOKIE.as_str())) + .map(|(_, value)| value.as_str()) + .collect::>(); + assert_eq!( + forwarded_cookies, + ["ab_bucket=A"], + "should forward exactly the prepared cookie represented by the cache key" + ); + } + + #[tokio::test] + async fn template_cookie_publisher_empty_lists_keep_legacy_behavior() { + for independent in [false, true] { + let settings = cookie_policy_settings(Some(&[]), Some(&[]), independent); + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + for _ in 0..2 { + queue_shareable_html(&stub); + let _ = + body_of(run(&settings, &services, cookie_navigation_request()).await).await; + } + assert_eq!( + stub.recorded_request_uris().len(), + if independent { 1 } else { 2 }, + "should retain legacy boolean eligibility" + ); + assert_eq!( + stored_cache_keys(&cache).len(), + usize::from(independent), + "should preserve legacy storage" + ); + } + } + + #[tokio::test] + async fn template_cookie_publisher_policy_changes_invalidate_templates() { + let first = cookie_policy_settings(None, None, true); + let second = cookie_policy_settings(Some(&["ab_bucket"]), None, true); + let third = cookie_policy_settings(Some(&["ab_bucket"]), Some(&["session"]), true); + assert_ne!( + template_fingerprint(&first), + template_fingerprint(&second), + "should fingerprint the key policy" + ); + assert_ne!( + template_fingerprint(&second), + template_fingerprint(&third), + "should fingerprint the bypass policy" + ); + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + for settings in [&first, &second, &third] { + queue_shareable_html(&stub); + let _ = body_of( + run( + settings, + &services, + cookie_policy_request(&[b"ab_bucket=A"]), + ) + .await, + ) + .await; + } + let keys = stored_cache_keys(&cache); + assert_eq!( + keys.len(), + 3, + "should store a fresh entry under each policy" + ); + assert_ne!(keys[0], keys[1], "should not reuse pre-policy templates"); + assert_ne!( + keys[1], keys[2], + "should not reuse entries admitted under another bypass policy" + ); + assert_eq!( + stub.recorded_request_uris().len(), + 3, + "should fetch after each policy change" + ); + } + + #[tokio::test] + async fn template_cookie_publisher_warm_variant_runs_fresh_reader_assembly() { + let mut raw = settings_with_bidder("esi"); + let config = raw + .creative_opportunities + .as_mut() + .expect("should configure opportunities"); + config.template_cache_key_cookies = Some(vec!["ab_bucket".to_string()]); + config.origin_is_cookie_independent = Some(true); + let settings = Arc::new(raw); + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + for (index, field) in [ + b"ab_bucket=A; ts-ec=reader1".as_slice(), + b"ab_bucket=A; ts-ec=reader2".as_slice(), + ] + .iter() + .enumerate() + { + queue_bid_response(&stub); + if index == 0 { + queue_shareable_html(&stub); + } + let response = run_bidding_at_price( + &settings, + &services, + cookie_policy_request(&[field]), + if index == 0 { 3.5 } else { 7.5 }, + ) + .await; + assert_eq!( + response.headers()[HEADER_X_TS_TEMPLATE_CACHE], + if index == 0 { "miss-stored" } else { "hit" }, + "should serve the second reader from the shared template" + ); + assert!( + response.headers()[header::CACHE_CONTROL] + .to_str() + .expect("should read cache policy") + .contains("private"), + "should keep reader output private" + ); + let document = + String::from_utf8(body_of(response).await).expect("should decode document"); + assert_eq!( + seam_bids(&document) + .get("test-slot") + .and_then(|bid| bid.get("hb_pb")) + .and_then(serde_json::Value::as_str), + Some(if index == 0 { "3.50" } else { "7.50" }), + "should assemble this request's winning bid" + ); + } + let requests = stub.recorded_request_uris(); + assert_eq!( + requests + .iter() + .filter(|uri| uri.contains("/article")) + .count(), + 1, + "should fetch one shared origin template" + ); + assert_eq!( + requests.len(), + 3, + "should run a fresh auction on each reader request" + ); + let entries = cache.entries.lock().expect("should lock templates"); + assert_eq!(entries.len(), 1, "should share within a variant"); + let stored = String::from_utf8_lossy( + &entries + .values() + .next() + .expect("should store a template") + .body, + ); + assert!( + stored.contains(AD_ASSEMBLY_SEAM), + "should keep the unresolved reader assembly marker" + ); + for reader_bytes in ["reader1", "reader2", "window.tsjs", "hb_pb"] { + assert!( + !stored.contains(reader_bytes), + "should exclude per-reader state from stored bytes" + ); + } + } + + #[tokio::test] + async fn template_cookie_publisher_warm_variant_finalizes_ec_withdrawal() { + for finalizer in [Finalizer::Streaming, Finalizer::Buffered] { + let mut settings = cookie_policy_settings(Some(&["ab_bucket"]), None, true); + Arc::make_mut(&mut settings).auction.providers = + crate::auction_config_types::AuctionConfig::legacy_provider_map(&[ + SCHEDULING_PROVIDER, + ]); + let stub = Arc::new(StubHttpClient::new()); + let cache = Arc::new(MemoryTemplateCache::default()); + let services = services(Arc::clone(&stub), Arc::clone(&cache)); + let graph = KvIdentityGraph::in_memory("cookie-withdrawal-store"); + let identities = [ + format!("{}.Read01", "a".repeat(64)), + format!("{}.Read02", "b".repeat(64)), + ]; + for (index, identity) in identities.iter().enumerate() { + assert!( + crate::ec::generation::is_valid_ec_id(identity), + "should use valid EC identities in the fixture" + ); + graph + .create( + identity, + &crate::ec::kv_types::KvEntry::minimal( + "example.com", + &format!("partner-reader-{index}"), + crate::ec::current_timestamp(), + ), + ) + .expect("should seed a live reader identity"); + } + let registry = IntegrationRegistry::new(&settings) + .expect("should create integration registry"); + let partner = serde_json::from_value(serde_json::json!({ + "name": "Example partner", + "source_domain": "example.com", + "bidstream_enabled": true + })) + .expect("should deserialize fixture partner"); + let partners = PartnerRegistry::from_config(&[partner]) + .expect("should create partner registry"); + // Only the cold request has an origin response available. + queue_shareable_html(&stub); + + for (index, (identity, withdrawn)) in [ + (&identities[0], false), + (&identities[1], false), + (&identities[1], true), + ] + .into_iter() + .enumerate() + { + let consent = if withdrawn { + ConsentContext { + jurisdiction: crate::consent::jurisdiction::Jurisdiction::UsState( + "CA".to_owned(), + ), + gpc: true, + ..Default::default() + } + } else { + scheduling_consent() + }; + let mut ec_context = EcContext::new_for_test(Some(identity.clone()), consent); + assert_eq!( + ec_context.ec_allowed(), + !withdrawn, + "should apply reader consent" + ); + let captured = Arc::new(Mutex::new(None)); + let mut orchestrator = AuctionOrchestrator::new(settings.auction.clone()); + orchestrator.register_provider(Arc::new(SchedulingCaptureProvider { + captured: Arc::clone(&captured), + http: Arc::clone(&stub), + lookups: Arc::new(AtomicUsize::new(0)), + })); + let orchestrator = Arc::new(orchestrator); + let cookies = format!("ab_bucket=A; ts-ec={identity}"); + let response = handle_publisher_request( + &settings, + &services, + Some(&graph), + &mut ec_context, + AuctionDispatch { + orchestrator: &orchestrator, + slots: &[article_slot()], + registry: Some(&partners), + }, + cookie_policy_request(&[cookies.as_bytes()]), + EdgeCacheHeader::SMaxageFallback, + ) + .await + .expect("should serve a reader with an existing EC"); + assert!( + ec_context.kv_snapshot().entry_for(identity).is_some(), + "should preload this reader's identity even during withdrawal on a hit" + ); + let mut response = finalize_test_publisher_response( + response, + &settings, + &services, + ®istry, + orchestrator, + finalizer, + ) + .await; + crate::ec::finalize::ec_finalize_response( + &settings, + &mut ec_context, + Some(&graph), + &partners, + None, + None, + &mut response, + ); + assert_eq!( + response.headers()[HEADER_X_TS_TEMPLATE_CACHE], + if index == 0 { "miss-stored" } else { "hit" }, + "should share one variant across identities and withdrawal" + ); + let cache_control = response.headers()[header::CACHE_CONTROL] + .to_str() + .expect("should decode cache policy"); + for directive in ["private", "no-store"] { + assert!( + cache_control + .split(',') + .any(|value| value.trim() == directive), + "should keep finalized reader responses private and uncacheable" + ); + } + assert_eq!( + response + .headers() + .get_all(header::SET_COOKIE) + .iter() + .any(|value| { + let value = value.to_str().expect("should decode response cookie"); + value.starts_with("ts-ec=") && value.contains("Max-Age=0") + }), + withdrawn, + "should expire the EC cookie only for the withdrawing reader" + ); + let body = body_of(response).await; + assert!(!body.is_empty(), "should render a complete reader response"); + let captured = captured.lock().expect("should lock captured auction"); + let auction = captured.as_ref().expect("should dispatch a reader auction"); + assert_eq!( + auction.request.user.id.as_deref(), + if withdrawn { + None + } else { + Some(identity.as_str()) + }, + "should use this reader's identity and suppress it after withdrawal" + ); + if withdrawn { + assert!( + auction.request.user.eids.is_none(), + "should suppress withdrawn EIDs" + ); + } else { + let eids = auction + .request + .user + .eids + .as_ref() + .expect("should include consenting reader EIDs"); + assert_eq!(eids.len(), 1, "should expose only the configured partner"); + assert_eq!( + eids[0].source, "example.com", + "should use the registered source" + ); + assert_eq!( + eids[0].uids[0].id, + format!("partner-reader-{index}"), + "should use this reader's partner identity on cold and warm requests" + ); + } + } + assert_eq!( + stub.recorded_request_uris().len(), + 1, + "should fetch origin only once" + ); + assert_eq!( + looked_up_cache_keys(&cache).len(), + 3, + "should look up every reader" + ); + assert_eq!( + stored_cache_keys(&cache).len(), + 1, + "should store only the cold template" + ); + for (index, identity) in identities.iter().enumerate() { + let (entry, _) = graph + .get(identity) + .expect("should read reader identity") + .expect("should retain the live row or its tombstone"); + assert_eq!( + entry.consent.ok, + index == 0, + "should revoke only the second reader" + ); + assert_eq!( + entry.ids.is_empty(), + index == 1, + "should clear only revoked partner IDs" + ); + } + let entries = cache.entries.lock().expect("should lock cached templates"); + let stored = &entries + .values() + .next() + .expect("should retain shared template") + .body; + let stored = String::from_utf8_lossy(stored); + for identity in &identities { + assert!( + !stored.contains(identity), + "should keep reader identities out of cached bytes" + ); + } + } + } + #[tokio::test] async fn by_default_a_cookie_bearing_request_uses_no_shared_cache() { // The shipped default, and the reason the cache is nearly inert on real @@ -13058,7 +14086,7 @@ mod tests { } #[test] - fn a_forwarded_request_cookie_disqualifies_even_without_set_cookie() { + fn cookie_policy_disqualifies_even_without_set_cookie() { // The dangerous case: session established on an earlier request, so this // response carries no Set-Cookie, has no Cache-Control at all, is a 200, // and is HTML — yet is personalized because TS forwarded the Cookie to @@ -13074,7 +14102,7 @@ mod tests { &no_cache_control, ¬hing_covered(), ), - Some(TemplateCacheBypassReason::CookieForwarded), + Some(TemplateCacheBypassReason::CookiePolicy), "cookie-personalized HTML must not become a shared template" ); } @@ -13277,6 +14305,8 @@ mod tests { assembly_mode: None, template_cache_vary: None, template_cache_max_age_seconds: None, + template_cache_key_cookies: None, + template_cache_bypass_cookies: None, origin_is_cookie_independent: None, section_segment: None, slot: vec![slot()], @@ -18718,6 +19748,8 @@ mod tests { assembly_mode: None, template_cache_vary: None, template_cache_max_age_seconds: None, + template_cache_key_cookies: None, + template_cache_bypass_cookies: None, origin_is_cookie_independent: None, section_segment: None, slot: Vec::new(), diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 89dc43aea..c67e4da80 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -1882,9 +1882,9 @@ the mode but safely fall back to the inline transform on every request. This is not a top-level HTTP cache hit: Compute still runs and the final assembled response is always `Cache-Control: private, no-store`. -All four keys below belong directly under `[creative_opportunities]`. They are +All keys below belong directly under `[creative_opportunities]`. They are one feature contract: `assembly_mode` selects how creative-opportunity state is -delivered, while the other three constrain when and how long that mode may share +delivered, while the other keys constrain when and how long that mode may share its template. They are not a general top-level HTTP-cache configuration. @@ -1905,8 +1905,14 @@ template_cache_vary = [ # The origin's remaining edge freshness may make the actual lifetime shorter. template_cache_max_age_seconds = 1200 -# Default false. Enable only after proving publisher HTML ignores Cookie. -origin_is_cookie_independent = true +# Optional bounded variant and personal-session policies; omitted means empty. +template_cache_key_cookies = ["ab_bucket"] +template_cache_bypass_cookies = ["session"] + +# Default false. With the lists above, false still bypasses every request that +# carries any other cookie, including the TS identity cookie. Set true only +# after proving those unlisted cookies do not change origin HTML. +origin_is_cookie_independent = false ``` The cache fails closed. A template is stored only for a `GET` with a processable @@ -1951,13 +1957,90 @@ for each reader with `Vary: Accept-Encoding`. This assumes the origin's compression negotiation does. Do not enable ESI for an origin that changes the document's meaning based on `Accept-Encoding`. Never put `Cookie` or `Authorization` in `template_cache_vary`; startup rejects both because raw cookie -or credential values are not reader-neutral template dimensions. With -`origin_is_cookie_independent = false` (the safe default), all cookie-bearing -requests bypass. With it set to `true`, an origin `Vary: Cookie` still overrides -the assertion and refuses storage. Every other name the origin emits in `Vary` +or credential values are not reader-neutral template dimensions. An origin +`Vary: Cookie` always refuses storage, including when a named cookie policy is +configured or `origin_is_cookie_independent` is `true`. +Every other name the origin emits in `Vary` must appear in the configured list; an uncovered name safely refuses template storage. +The optional cookie lists control both template lookup and storage: + +- `template_cache_key_cookies` includes each named cookie's presence and value in + the template key. Use bounded, reader-neutral variants such as experiment arms + or region buckets, never account IDs, session tokens, or TS identity IDs. Missing + and empty values select different variants. Multiple names form a combined variant. +- `template_cache_bypass_cookies` forces inline processing whenever any named + cookie is present, including an empty value such as `session=`. This applies + regardless of the independence assertion or any key cookies in the request. +- When either list is nonempty, `origin_is_cookie_independent` applies only to + unlisted cookies. The safe default, `false`, bypasses requests containing any + unlisted cookie; `true` asserts that those cookies do not change origin HTML. + TS identity and consent cookies have no implicit exception. +- When both lists are omitted or empty, the existing all-cookie behavior remains: + `false` bypasses every request carrying a `Cookie` header; `true` asserts that + all cookies are irrelevant to origin HTML. + +Cookie names match exactly and case-sensitively: `session` and `Session` are +different names. Use nonempty ASCII HTTP token names; whitespace, `;`, `=`, and +non-ASCII characters are invalid. Configuration rejects invalid names, duplicates +within a list, overlap between lists, and TS identity cookies (`ts-ec`, `ts-eids`, +`sharedId`) in the key list. Identity cookies may be listed for bypass. Wildcards, +prefixes, and regular expressions are not supported. With either list nonempty, parsing of all +`Cookie` fields after existing request preparation bypasses duplicate key-cookie names, +invalid names, and malformed key-cookie values. When independence is `true`, +duplicate unlisted names are allowed if every value passes framing checks, and +unlisted values may also contain commas and balanced double quotes, supporting +compact JSON cookies such as `g_state` and comma-separated experiment metadata. +Unmatched quotes, whitespace within values, backslashes, controls, non-ASCII bytes, +and comma-separated fragments resembling another `name=value` cookie still bypass. +The origin must parse semicolon-separated cookies independently and treat these +unlisted values as opaque. If its parser stops at a nonstandard value or changes +how later key cookies are interpreted, the independence assertion is not valid. +Cookies are forwarded unchanged by this policy; their values are not exposed in +cache diagnostics. There is no value allowlist or cardinality limit, so operators +must ensure keyed values stay bounded and account for every origin HTML dependency. + +If a downstream CDN translates `ab_bucket=A` into `X-Exp-Variant: A` after the +request passes TS, configure both dimensions: + +```toml +[creative_opportunities] +assembly_mode = "esi" +# Retain any other header dimensions required by the origin. +template_cache_vary = ["x-exp-variant"] +template_cache_key_cookies = ["ab_bucket"] +template_cache_bypass_cookies = ["session"] +# Only after verifying that all remaining cookies leave origin HTML unchanged. +origin_is_cookie_independent = true +``` + +The cookie dimension separates experiment arms at TS even when the header is +absent there; the header dimension covers the origin's `Vary: X-Exp-Variant`. +Configuring the header alone cannot distinguish these readers. TS cannot verify +the downstream mapping or discover other inputs selecting origin HTML. During a +canary, check correct content for each arm, absent and empty experiment values, +and session-bearing requests, as well as cache diagnostics. + +The lists can also be used independently within `[creative_opportunities]`. For +experiment-only HTML, omit the bypass list: + +```toml +template_cache_vary = ["x-exp-variant"] +template_cache_key_cookies = ["ab_bucket"] +origin_is_cookie_independent = true +``` + +For anonymous sharing with a logged-in population, omit the key list: + +```toml +template_cache_bypass_cookies = ["session"] +origin_is_cookie_independent = true +``` + +Both examples require the same assertion that unlisted cookies do not change +origin HTML and retain all other ESI eligibility and header-coverage requirements. + For a canary, inspect `X-TS-Template-Cache`. Its bounded values are `hit`, `miss-stored`, `miss-store-error`, `miss-reserved`, `bypass-request`, `bypass-response`, `unsupported`, `invalid`, and `backend-error`. No URL, header @@ -1980,9 +2063,14 @@ Rollback must preserve configuration compatibility: 1. Change `assembly_mode` to `inline` and deploy/push that configuration. 2. Before rolling back to a binary that predates these fields, remove - `assembly_mode`, `template_cache_vary`, `template_cache_max_age_seconds`, and + `assembly_mode`, `template_cache_vary`, `template_cache_max_age_seconds`, + `template_cache_key_cookies`, `template_cache_bypass_cookies`, and `origin_is_cookie_independent`, then push the cleaned configuration. Older binaries - use `deny_unknown_fields` and intentionally reject unknown keys. + use `deny_unknown_fields` and intentionally reject unknown keys, even empty lists. + When rolling back only the named-cookie feature to a binary that supports ESI, + remove both cookie-list fields and keep `origin_is_cookie_independent = false` + or disable ESI if the origin depends on cookies. Keeping `true` after removing + the lists loses variant separation and session bypass. 3. Purge the Fastly surrogate key `ts-template` using the service's normal purge tooling, or wait for the bounded origin-derived lifetime to expire. @@ -1990,7 +2078,11 @@ Run `scripts/template-cache-local-test.sh esi` before a rollout and `scripts/template-cache-local-test.sh inline` as its control. The harness uses a temporary manifest, never edits the tracked `fastly.toml`, verifies cold/warm origin counts and response integrity, and executes the generated GPT module against -the served seam to require a real `defineSlot` call. +the served seam to require a real `defineSlot` call. Both modes also exercise a +cookie-selected origin: A/B isolation without a client variant header, absent +versus empty buckets, ignored compact JSON and comma-list cookies, and session +bypass on warm and cold URLs. Each request checks the selected HTML, cache +diagnostics, private response policy, winning-bid assembly, and origin fetch count. ### `gam_unit_path` templating diff --git a/docs/superpowers/plans/2026-09-08-1138-per-cookie-template-cache-policy.md b/docs/superpowers/plans/2026-09-08-1138-per-cookie-template-cache-policy.md new file mode 100644 index 000000000..4624b34d8 --- /dev/null +++ b/docs/superpowers/plans/2026-09-08-1138-per-cookie-template-cache-policy.md @@ -0,0 +1,641 @@ +# Per-cookie Template Cache Policy Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [x]`) syntax for tracking. + +**Goal:** Separate bounded cookie-selected HTML variants in the shared template cache and route session-bearing requests inline while preserving conservative defaults. + +**Architecture:** Extend the existing platform cache key with explicit cookie dimensions. A focused core module validates cookie names and evaluates all request cookie fields once; the publisher uses its decision for lookup and storage. Preserve the existing origin-response guards and early request-validation errors. + +**Tech Stack:** Rust 2024, existing `http`, `serde`, `sha2`, `hex`, and error-stack boundaries; existing core test harness, Viceroy, adapter-specific Cargo aliases, and Prettier. No new dependencies. + +--- + +## Inputs, scope, and execution rules + +- Spec: [Per-cookie shared-template cache policy](../specs/2026-09-08-1138-per-cookie-template-cache-policy-design.md). +- Branch: `feature/1138-per-cookie-template-cache-policy`. Continue on this branch + in the current clean checkout; do not create a replacement branch or require a + worktree migration for this already-isolated task. +- Read `CLAUDE.md` before implementation. Use @superpowers:test-driven-development + for behavior changes and @superpowers:verification-before-completion before + success claims or commits. Use @superpowers:systematic-debugging for failures. +- The task text records the implementation steps; the execution record below + reports actual validation. Completed steps are checked. Rust sketches retain + their planning form; source contains the reviewed implementation with imports, + documentation, and formatting. +- Only the completed feature is deployable. Intermediate commits may introduce + configuration or data types before the runtime consumer is connected. +- Do not change unrelated cookie helpers, error types, cache backends, origin + request headers, template schema version, or final-response privacy behavior. + +## Execution record + +Implemented on `feature/1138-per-cookie-template-cache-policy` and independently +reviewed for specification compliance and code quality. Runtime, tests, and docs +are committed together after full verification, consolidating the task-level +commit suggestions above. Fingerprint assertions live alongside the cookie-policy +publisher regressions so they also verify cache invalidation across settings. + +The existing diagnostics preparation was found to sanitize Cookie fields before +both generic parsing and the policy gate. Its behavior is preserved; raw parser +and evaluator regressions explicitly enter the already-prepared request boundary, +with a separate regression for normal preparation. The specification and Task 6 +record this boundary. Cookie values are also redacted from `Debug` output. + +Verification completed: + +| Check | Result | +| ----------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | +| Initial key isolation regression | Failed with cookie encoding absent, then passed with encoding | +| Initial configuration and evaluator regressions | Failed before implementation, then passed | +| Initial key-only publisher regression | Failed under the old blanket gate, then passed | +| A/B mutation removing cookie key dimensions | Failed when arm B incorrectly hit arm A; implementation restored | +| New cookie-policy tests | 20 passed | +| `cargo test-fastly` | Passed: 170 Fastly adapter, 2,476 core, 2 JS Rust, 21 OpenRTB tests; doc tests passed; existing ignored tests remain ignored | +| `cargo test-axum` | Passed: 38 tests | +| `cargo test-cloudflare` | Passed: 40 tests | +| `cargo test-spin` | Passed: 80 tests | +| Integration parity | 13 passed | +| Six target-specific Clippy aliases | All passed | +| Rust formatting | Passed | +| JS build, Vitest, JS formatting | Passed; 45 test files, 893 tests | +| Docs formatting and diff whitespace | Passed | + +Viceroy required access to the macOS certificate keychain, and two Axum tests +required loopback socket binding. Initial sandbox restrictions were resolved by +rerunning those commands with the required access; no product changes were made +for the environment. No merge, push, or deployment is part of this implementation. + +## File map + +| File | Responsibility | +| ----------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- | +| `crates/trusted-server-core/src/platform/template_cache.rs` | Public `TemplateCookieValue`, `TemplateCacheKey.cookie_values`, canonical encoding, key tests | +| `crates/trusted-server-core/src/platform/mod.rs` | Re-export the cookie dimension type | +| `crates/trusted-server-core/src/cookies/template_cache_policy.rs` (new) | Internal name validation and byte-preserving cookie-policy evaluator, with unit tests | +| `crates/trusted-server-core/src/cookies.rs` | Declare the internal child module; preserve existing helpers | +| `crates/trusted-server-core/src/creative_opportunities.rs` | Optional fields, borrowed list accessors, validation and config tests | +| `crates/trusted-server-core/src/publisher.rs` | Single request-policy evaluation, key construction, existing test fixtures and end-to-end regressions | +| `crates/trusted-server-adapter-fastly/src/template_cache.rs` | Update explicit key test fixture; backend behavior unchanged | +| `docs/guide/configuration.md` | Operator policy examples, guard limitations, safe rollback | +| `trusted-server.example.toml` | Commented example settings and bounded-value guidance | + +Keep the new parser out of the already large publisher module. Keep publisher +regressions beside the existing `template_cache_end_to_end_tests` harness so they +exercise real lookup, reservation, transformation, storage, and finalization. + +## Test commands and conventions + +Run commands from the repository root unless a working directory is stated. +`cargo test-fastly ` includes the shared core tests under the WASI/Viceroy +target. Other selected packages may report zero matching tests; confirm that the +core package actually executes the intended new tests. Do not use bare +`cargo test --workspace`. `cargo test-axum` alone does not run core unit tests. + +For each behavior step: add a test, run its filter and confirm the intended +failure, implement the minimum change, then rerun that filter. A missing new symbol +can be the initial compile failure, but verify behavioral regressions fail with a +compiling stub or old logic before relying on them. Do not count dependency, +toolchain, or simulator failures as a successful red test. + +After each completed runtime task, run the relevant target-matched suite; run +`cargo fmt --all -- --check` before its commit. Use descriptive assertions and +`expect("should ...")`; no new `unwrap()` or local imports. + +## Task 1: Add cookie dimensions to the platform key + +**Files:** `crates/trusted-server-core/src/platform/template_cache.rs`, +`crates/trusted-server-core/src/platform/mod.rs`, +`crates/trusted-server-core/src/publisher.rs`, and +`crates/trusted-server-adapter-fastly/src/template_cache.rs`. + +- [x] **1.1 Verify the pre-change key contract.** Reuse the existing + `rendered_key_is_fixed_size_and_contains_no_request_material` test beside + `platform::template_cache::tests::key`. Its pinned literal is + `ts-template-cache-v4-54431eb4ea82644d6378717a8c3f18302fafbf739e684598da79e392b16900a6`. + Run `cargo test-fastly rendered_key_is_fixed_size_and_contains_no_request_material` + and verify it passes on the old code. Keep the same expected value after adding + empty cookie dimensions; do not duplicate the fixture or derive the expected + key with the new implementation under test. +- [x] **1.2 Add failing key-isolation tests.** Use the common prefix + `template_cookie_key_` for tests comparing changed values/names, absent versus + present-empty, quoted versus unquoted values, and two cookie dimensions. Verify + adding a dimension changes the key, while URL/global surrogate keys stay equal. + Run `cargo test-fastly template_cookie_key_` and confirm failure before the + encoder is extended. +- [x] **1.3 Add the public domain type and field.** Follow existing platform API + visibility and documentation conventions: + + ```rust + #[derive(Debug, Clone, PartialEq, Eq)] + pub struct TemplateCookieValue { + pub name: String, + pub value: Option>, + } + ``` + + Add `pub cookie_values: Vec` to `TemplateCacheKey` and + re-export the type from `platform/mod.rs`. Document exact case-sensitive names, + absent/empty distinction, sorted producer order, and bounded variant use. The + publisher/evaluator owns sorting; the encoder consumes the supplied order as + the existing header encoder does. + +- [x] **1.4 Extend canonical encoding after the complete existing header section.** + Reuse the existing length-prefix `push` helper: + + ```rust + if !self.cookie_values.is_empty() { + push(&mut canonical, b"cookie-variants-v1"); + push( + &mut canonical, + &(self.cookie_values.len() as u64).to_be_bytes(), + ); + for cookie in &self.cookie_values { + push(&mut canonical, cookie.name.as_bytes()); + match &cookie.value { + None => push(&mut canonical, b"absent"), + Some(value) => { + push(&mut canonical, b"present"); + push(&mut canonical, value); + } + } + } + } + ``` + + Leave the hash, backend prefix, transform schema version, and surrogate methods + unchanged. Distinct framing of names and values must prevent concatenation + collisions; add a test with differently partitioned names/values. + +- [x] **1.5 Update every literal with `cookie_values: Vec::new()`.** Locate them + with `rg -n 'TemplateCacheKey \{' crates --glob '*.rs'`. This includes the + current publisher production constructor temporarily; Task 4 replaces its empty + value with evaluated dimensions. Do not change adapter algorithms. +- [x] **1.6 Verify and commit.** Run `cargo test-fastly template_cookie_key_`, + `cargo test-fastly rendered_key_is_fixed_size_and_contains_no_request_material`, + `cargo test-fastly`, and + `cargo fmt --all -- --check`. Expected: isolation tests and legacy fixture pass, + including the Fastly key consumer. Stage only these files and commit + `Add cookie variant dimensions to template cache keys`. + +## Task 2: Add and validate optional cookie policies + +**Files:** `crates/trusted-server-core/src/creative_opportunities.rs`, +`crates/trusted-server-core/src/cookies.rs`, new +`crates/trusted-server-core/src/cookies/template_cache_policy.rs`, and explicit +configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. + +- [x] **2.1 Add config tests with prefix `template_cookie_config_`.** Deserialize + TOML with omitted fields, explicit empty fields, each list independently, and + both lists. Verify missing fields serialize without either new JSON key and + that `origin_is_cookie_independent` still defaults false. Reject empty names, + whitespace, separators, control/non-ASCII names, within-list duplicates, and + exact-name overlap. Accept `session` and `Session` as distinct names. Retain the + existing header `Cookie`/`Authorization` rejection tests. Run + `cargo test-fastly template_cookie_config_` and confirm the pre-feature failure. +- [x] **2.2 Add fields and borrowed accessors.** Place them alongside the current + template cache fields and update boolean documentation to scoped semantics: + + ```rust + #[serde(default, skip_serializing_if = "Option::is_none")] + pub template_cache_key_cookies: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub template_cache_bypass_cookies: Option>, + ``` + + Accessors named `template_cache_key_cookies()` and + `template_cache_bypass_cookies()` return `&[String]` using + `self..as_deref().unwrap_or_default()`. Do not allocate on access or + materialize absent options. Add `None` fields to existing struct literals; + locate them with `rg -n 'CreativeOpportunitiesConfig \{' crates`. + +- [x] **2.3 Create shared name validation.** Add + `pub(crate) mod template_cache_policy;` to `cookies.rs`. In that child module, + define the reusable predicate below and a validation entry point: + + ```rust + fn is_cookie_name(name: &[u8]) -> bool { + !name.is_empty() + && name.iter().all(|byte| { + byte.is_ascii_alphanumeric() + || b"!#$%&'*+-.^_`|~".contains(byte) + }) + } + + pub(crate) fn validate_cookie_names( + key_names: &[String], + bypass_names: &[String], + ) -> Result<(), String> { + let mut seen = std::collections::HashSet::new(); + for (field, names) in [ + ("template_cache_key_cookies", key_names), + ("template_cache_bypass_cookies", bypass_names), + ] { + for name in names { + if !is_cookie_name(name.as_bytes()) { + return Err(format!("{field} contains invalid cookie name `{name}`")); + } + if !seen.insert(name.as_str()) { + return Err(format!( + "{field} repeats cookie name `{name}` within or across cookie policies" + )); + } + } + } + Ok(()) + } + ``` + + Call this from `CreativeOpportunitiesConfig::validate_runtime` using the two + accessors. Preserve its existing `Result<(), String>` interface and the + `Settings::validate` conversion into `Report`; do not invent + a new error stack for this helper. Expand the validation method's error docs. + +- [x] **2.4 Verify and commit.** Run + `cargo test-fastly template_cookie_config_`, + `cargo test-fastly template_cache_vary_rejects_invalid_header_names`, + `cargo test-fastly`, and `cargo fmt --all -- --check`. + Commit `Add validated per-cookie template cache configuration` with only the + files listed for this task. + +## Task 3: Implement the pure cookie-policy evaluator + +**File:** `crates/trusted-server-core/src/cookies/template_cache_policy.rs`. + +- [x] **3.1 Add decision-matrix tests before implementation.** Use prefix + `template_cookie_policy_` and raw `http::HeaderMap` fixtures. Cover both boolean + states; both lists empty; either list alone with the other omitted/resolved + empty; both lists active; key-only requests; unknown cookies; empty bypass + cookies; bypass mixed with a key cookie; and case-sensitive names. Test the + evaluator directly so early publisher validation cannot mask a parser test. + Run `cargo test-fastly template_cookie_policy_` to observe the missing behavior. +- [x] **3.2 Implement an admitted/bypassed result.** Keep it internal: + + ```rust + pub(crate) enum TemplateCookieDecision { + Bypass, + Eligible(Vec), + } + + pub(crate) fn evaluate_cookie_policy( + headers: &http::HeaderMap, + key_names: &[String], + bypass_names: &[String], + independent: bool, + ) -> TemplateCookieDecision { + if key_names.is_empty() && bypass_names.is_empty() { + return if headers.contains_key(http::header::COOKIE) && !independent { + TemplateCookieDecision::Bypass + } else { + TemplateCookieDecision::Eligible(Vec::new()) + }; + } + + let mut parsed = std::collections::HashMap::<&[u8], &[u8]>::new(); + for field in headers.get_all(http::header::COOKIE) { + for raw_pair in field.as_bytes().split(|byte| *byte == b';') { + let pair = trim_pair(raw_pair); + let Some(separator) = pair.iter().position(|byte| *byte == b'=') else { + return TemplateCookieDecision::Bypass; + }; + let (name, rest) = pair.split_at(separator); + let value = &rest[1..]; + if !is_cookie_name(name) || !is_cookie_value(value) { + return TemplateCookieDecision::Bypass; + } + if parsed.insert(name, value).is_some() { + return TemplateCookieDecision::Bypass; + } + if bypass_names.iter().any(|item| item.as_bytes() == name) { + return TemplateCookieDecision::Bypass; + } + if !independent && !key_names.iter().any(|item| item.as_bytes() == name) { + return TemplateCookieDecision::Bypass; + } + } + } + + let mut ordered_names: Vec<&String> = key_names.iter().collect(); + ordered_names.sort_unstable(); + TemplateCookieDecision::Eligible( + ordered_names + .into_iter() + .map(|name| TemplateCookieValue { + name: name.clone(), + value: parsed.get(name.as_bytes()).map(|value| value.to_vec()), + }) + .collect(), + ) + } + ``` + + Use these SP/HTAB-only trimming and value-validation helpers. Trimming a broader + whitespace set would erase malformed bytes that must cause bypass: + + ```rust + fn trim_pair(mut pair: &[u8]) -> &[u8] { + while pair.first().is_some_and(|byte| matches!(byte, b' ' | b'\t')) { + pair = &pair[1..]; + } + while pair.last().is_some_and(|byte| matches!(byte, b' ' | b'\t')) { + pair = &pair[..pair.len() - 1]; + } + pair + } + + fn is_cookie_value(value: &[u8]) -> bool { + let payload = if value.first() == Some(&b'"') { + if value.len() < 2 || value.last() != Some(&b'"') { + return false; + } + &value[1..value.len() - 1] + } else { + value + }; + payload.iter().all(|byte| { + matches!(byte, 0x21 | 0x23..=0x2b | 0x2d..=0x3a | 0x3c..=0x5b | 0x5d..=0x7e) + }) + } + ``` + + Import `TemplateCookieValue` from `crate::platform` at module scope. No request + mutation, cookie decoding, logging of values, or new request errors belong here. + Early return on any disqualifier is safe because the entire request bypasses; + every admitted request must have validated every field. Only key-cookie values + are owned in the result; parsed unlisted values remain borrowed and temporary. + +- [x] **3.3 Add malformed/ambiguous-input tests.** Cover repeated fields, duplicate + names across/within fields (equal and different values), empty fields/pairs, + trailing semicolons, bare names, bad quoting, forbidden bytes, additional `=`, + percent escapes, and quoted/unquoted representations. Assert `name= ` becomes + present-empty, while `name =A` and `name= A` bypass. Input ordering and header + splitting must not change eligible sorted dimensions. Under an empty policy, + all of these inputs retain the old header-presence/boolean decision. +- [x] **3.4 Run evaluator tests and the Fastly suite.** Run + `cargo test-fastly template_cookie_policy_` then `cargo test-fastly`. Expected: + policy matrix and raw-byte cases pass. Finish Task 4 before committing this + runtime helper so production consumes it without unused-code lint allowances. + +## Task 4: Connect the evaluator to publisher lookup and store + +**File:** `crates/trusted-server-core/src/publisher.rs`. + +- [x] **4.1 Add failing key-only and bypass-only integration tests.** Use prefix + `template_cookie_publisher_` inside `template_cache_end_to_end_tests`. Reuse + `settings_with_mode`, `navigation_request`, `MemoryTemplateCache`, and `run_via`. + A key-only request with independence false must be eligible, which fails under + the old gate. A bypass-only session request with independence true must make no + lookup or store, which fails under the old gate. Repeat with the unused list + explicitly empty. Run `cargo test-fastly template_cookie_publisher_` before + replacing the gate and confirm these behavioral failures. +- [x] **4.2 Replace the blanket cookie check at the existing pre-fetch point.** + Obtain list slices and the boolean from `settings.creative_opportunities`, using + `&[]`, `&[]`, and false when the table is absent. Call the evaluator exactly once: + + ```rust + let (cookie_disqualifies, cookie_values) = match evaluate_cookie_policy( + req.headers(), + key_cookie_names, + bypass_cookie_names, + origin_is_cookie_independent, + ) { + TemplateCookieDecision::Bypass => (true, Vec::new()), + TemplateCookieDecision::Eligible(values) => (false, values), + }; + ``` + + Import the function/enum at module scope. Remove the obsolete + `request_had_cookie` local and update only directly affected comments. Preserve + the existing `!cookie_disqualifies` lookup predicate and pass that same boolean + to the existing `template_cache_ttl` store check. Move `cookie_values` into the + key inside `request_can_use_shared_template.then(...)`. Leave all other gate + predicates and the earlier `handle_request_cookies(&req)?` call untouched. + +- [x] **4.3 Verify cache-call and inline behavior.** For both cold and warm cache, + compare `lookups`, `stored_keys`, and origin-request counts before/after the + session request. The warm test must first prove the anonymous response actually + stored a template. Bypass adds zero lookups/reservations/stores and one origin + request. Inspect `X-TS-Template-Cache`, `X-TS-Assembly`, and finalized body to + prove existing inline/private behavior. Run both `Finalizer::Streaming` and + `Finalizer::Buffered` for representative bypass cases. +- [x] **4.4 Verify and commit Tasks 3–4 together.** Run + `cargo test-fastly template_cookie_policy_`, + `cargo test-fastly template_cookie_publisher_`, `cargo test-fastly`, and + `cargo fmt --all -- --check`. Commit only the parser and publisher changes as + `Apply cookie policy to template lookup and storage`. + +## Task 5: Prove downstream variant isolation and response guards + +**File:** `crates/trusted-server-core/src/publisher.rs`. + +- [x] **5.1 Reproduce the original header-only failure in a regression.** Configure + key cookie `ab_bucket`, header dimension `x-exp-variant`, and independence true. + Keep that header absent on every incoming request. Queue distinguishable + shareable HTML bodies `arm-A` and `arm-B`, both declaring + `Vary: X-Exp-Variant`. Send A/reader1, B/reader2, A/reader3, and B/reader4. + Assert correct arm content for every response, exactly two origin fetches, + exactly two stored keys, and warm-hit diagnostics for the last two requests. + Briefly run the test with cookie dimensions omitted from key construction to + prove it detects cross-serving, then restore the implementation and rerun. + Do not commit the deliberate regression. +- [x] **5.2 Cover absence, empty values, and unknown-cookie policy.** Missing + `ab_bucket` and `ab_bucket=` store/hit separate bodies. With independence false, + a configured key cookie alone shares, while adding an unknown cookie bypasses. + With independence true, unknown cookie changes do not fragment a variant. + Repeat meaningful cases with both lists configured to prove session bypass wins. +- [x] **5.3 Cover response refusal under the new configuration.** With valid keyed + requests and otherwise public fresh HTML, verify no store for `Vary: Cookie` + (case variations and repeated fields), `Vary: *`, uncovered + `X-Exp-Variant`, and origin `Set-Cookie`. A cookie dimension alone must not count + as header coverage. Preserve the existing gate tests for authorization, + freshness, GET-only admission, and inline mode; do not duplicate their complete + matrices unnecessarily. +- [x] **5.4 Verify readers remain separate after a warm hit.** Adapt the existing + bidding/finalization tests to run two readers in the same keyed variant and + prove shared stored bytes remain reader-neutral while final output is private + and uses per-request assembly. At least the A/B test must run through both + `run_via` finalizers so storage and warm-hit rendering are exercised. +- [x] **5.5 Verify and commit.** Name the new tests with the + `template_cookie_publisher_` prefix, then run + `cargo test-fastly template_cookie_publisher_`, + `cargo test-fastly template_cache_gate_tests`, `cargo test-fastly`, and + `cargo fmt --all -- --check`. Commit `Verify cookie variant isolation and session bypass`. + +## Task 6: Pin early-error boundaries and policy compatibility + +**Files:** `crates/trusted-server-core/src/publisher.rs` and +`crates/trusted-server-core/src/creative_opportunities.rs` tests. + +- [x] **6.1 Preserve the selected-header error.** Build a Cookie `HeaderValue` from + raw bytes that `to_str()` rejects, place it in the selected field, and assert the + publisher retains `InvalidHeaderValue` at the already-prepared request boundary. + Set the existing default `GptDiagnosticsRequestDecision` extension in a narrow + test helper to exercise the idempotent preparation path: ordinary preparation + otherwise removes unsupported fields before generic cookie parsing. Use a narrow fallible test runner or + call the handler with the existing harness setup; the current `run` helper + expects success and must not be used to assert an error. Do not change its + behavior for existing tests. Assert no cache call occurs and no template stores. +- [x] **6.2 Distinguish later-field input.** Send a valid selected field followed + by a later field with the same unsupported bytes. Under a named policy, the + request reaching the evaluator must bypass and use existing inline processing. + Test a warm and cold cache, and prove the later field is not silently ignored. + Separately test ASCII malformed pairs and duplicate names that reach the gate. + Use the same prepared-request helper for raw-field cases, then add a normal + preparation test proving that existing removal of invalid fields/empty pairs + remains unchanged and identical forwarded cookie inputs share a template. +- [x] **6.3 Pin policy fingerprinting.** Extend `template_fingerprint_tests` to + show that adding/changing either cookie list changes the fingerprint. Compare + serialization of omitted-field fixtures against their pre-change shape, and + verify explicit-empty versus omitted fields have equal runtime decisions even + if fingerprints differ. Reuse one memory cache across two settings snapshots to + show a changed key/bypass policy cannot read an old template under the old key. +- [x] **6.4 Verify default compatibility.** Retain existing + `by_default_a_cookie_bearing_request_uses_no_shared_cache` and + `a_declared_cookie_independent_origin_lets_repeat_visitors_share` tests. Test + both lists explicitly empty under both boolean values and assert old behavior. + Raw unsupported bytes belong in evaluator tests for legacy policy decisions; + they do not imply the publisher bypasses its earlier validation. +- [x] **6.5 Verify and commit.** Run + `cargo test-fastly template_cookie_publisher_`, + `cargo test-fastly template_fingerprint_tests`, + `cargo test-fastly template_cookie_config_`, `cargo test-fastly`, and + `cargo fmt --all -- --check`. Commit `Preserve cookie validation and policy compatibility`. + +## Task 7: Document deployment and rollback + +**Files:** `docs/guide/configuration.md`, `trusted-server.example.toml`, and the +spec/plan status checkboxes when appropriate. + +- [x] **7.1 Update operator examples.** Add commented optional lists alongside + existing template cache settings. Describe bounded values, exact-case names, + bypass-on-presence including empty values, unlisted-cookie boolean behavior, + and conservative opt-in parsing. Include key-only A/B and bypass-only session + examples, using fictional values and domains only. +- [x] **7.2 Explain the downstream header contract.** Show why a header created + after TS cannot alone distinguish variants. The example must include both + `ab_bucket` and `x-exp-variant` in their respective lists. Explain that + `Vary: Cookie` always refuses storage and that cookie configuration does not + automatically cover a named header. Avoid implying the drift guard verifies + downstream transformations or checks cached hits against current origin policy. +- [x] **7.3 Document safe rollback.** Add both fields to the existing list that + must be removed before loading configuration into older binaries. Even empty + fields are unknown to those binaries. When removing policies for a + cookie-dependent origin, set independence false or disable ESI. Explain + fingerprint invalidation and existing purge tools without adding new tooling. +- [x] **7.4 Verify and commit.** Run `npm run format` from `docs`, and + `git diff --check` from root. If formatting fails, use the installed Prettier on + only changed Markdown files and rerun the check. Commit + `Document per-cookie template cache policy and rollback`. + +## Task 8: Full verification and implementation handoff + +**Files:** Only fixes directly required by the feature and validation evidence. + +- [x] **8.1 Inspect the final diff against the feature base.** Check that no raw + cookie values enter logging, metric labels, diagnostics, or cache Debug output. + Verify the full Cookie header is never a dimension, no origin headers are + mutated, and no user-shaped identity field is introduced as an implicit key. +- [x] **8.2 Run the full repository CI command set.** Execute each command and + record its actual result. Independent target commands may run concurrently if + Cargo locking/resource usage is acceptable; do not mistake a queued build for + a passing check. + + ```bash + cargo fmt --all -- --check + cargo clippy-fastly + cargo clippy-axum + cargo clippy-cloudflare + cargo clippy-cloudflare-wasm + cargo clippy-spin-native + cargo clippy-spin-wasm + cargo test-fastly + cargo test-axum + cargo test-cloudflare + cargo test-spin + cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity + ``` + + In `crates/trusted-server-js/lib`: + + ```bash + node build-all.mjs + npx vitest run + npm run format + ``` + + In `docs`: + + ```bash + npm run format + ``` + + If any command is blocked by dependencies, network, missing tools, or baseline + failures, record the exact command and error. Do not claim the full gate passed. + Use target-specific aliases; do not substitute all-feature workspace commands. + +- [x] **8.3 Request an independent implementation review.** Use + @superpowers:requesting-code-review with this plan, the spec, the base commit, + final diff, and actual validation results. Resolve concrete findings and rerun + affected checks; repeat broader checks only when the changes justify it. +- [x] **8.4 Mark completed tasks and report evidence.** Summarize variant isolation, + session bypass, unchanged defaults/response guards, and any validation limits. + Commit only intended changes. Confirm branch and working-tree state with + `git status --short --branch`. Publishing a PR or deploying follows the user's + subsequent instruction; neither is part of writing this plan. + +## Browser-cookie compatibility follow-up + +Live browser verification found that an unlisted compact JSON `g_state` cookie +prevented sharing despite the independence assertion. The original Task 3 parser +above is superseded for unlisted values by the updated spec section 6: + +- Keep strict key values, exact names, duplicate rejection, bypass presence, and + the legacy empty-policy behavior. +- With independence enabled, tolerate commas and balanced quotes in ignored + values. Reject whitespace, backslashes, unsafe bytes, unmatched quotes, and + comma-delimited fragments resembling additional cookie assignments. +- Document the origin-parser assumption: ignored values must not affect how + later configured cookies are interpreted. +- Add evaluator coverage and publisher cold/warm coverage through both finalizers, + including exact origin cookie forwarding and session lookup/store bypass. +- Verify focused regressions fail before the fix, then run the Fastly suite, + relevant formatting/lint checks, and repeat headless browser verification. + +## Review-readiness runtime follow-up (2026-09-10) + +Starting revision: `1f56ee08cb25a35d79cccc19a442a698334a398c`. The follow-up +extends `scripts/template-cache-local-test.sh` and its operator documentation; +production Rust and JavaScript are unchanged. + +The existing temporary origin now serves distinguishable cookie-selected HTML +for dedicated article paths and returns `Vary: X-Exp-Variant`. Requests omit that +header to model variant selection downstream of TS. The generated configuration +includes `ab_bucket` keying, `session` bypass, and independence for opaque unlisted +cookies. Both ESI and inline runs execute 17 requests covering A/B isolation, +absent/empty variants, ignored JSON/comma-list cookies, and warm/cold session +bypass. Every request checks HTML, origin fetch counts, cache state, private +response policy, and assembled winning bids. Existing CI already runs both modes. + +Verification evidence: + +- Release build: `cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1` passed. +- `BID_DELAY=3 ./scripts/template-cache-local-test.sh esi` passed: 22 harness + checks, including all 17 cookie requests. +- `BID_DELAY=3 ./scripts/template-cache-local-test.sh inline` passed: 9 harness + checks, including all 17 cookie requests with an origin fetch on every request. +- Negative control: a temporary copy with an empty key-cookie list failed on the + first B request because the returned HTML did not contain the B marker. No + production code or tracked configuration was changed for this control. +- Independent review of the harness found no issues; shell syntax and diff + whitespace checks passed. +- All six target-specific Clippy aliases, Rust formatting, Fastly tests (2,902 + tests/doc-tests passed, 10 ignored), Axum (41), Cloudflare (44), Spin (86), and + integration parity (13) passed. Fastly and Axum passed after retrying with the + required keychain and local socket access outside the sandbox. +- JS build, Vitest (45 files, 901 tests using the pinned Node 24.12.0 executable + from the library directory), JS formatting, and docs formatting passed. + +These checks run Viceroy directly through the repeatable harness. They are +distinct from the earlier headless-browser smoke test, whose exact tested commit +was not recorded in the PR description, and from `fastly compute serve`, which +was not used for this follow-up. diff --git a/docs/superpowers/specs/2026-09-08-1138-per-cookie-template-cache-policy-design.md b/docs/superpowers/specs/2026-09-08-1138-per-cookie-template-cache-policy-design.md new file mode 100644 index 000000000..190e5e99b --- /dev/null +++ b/docs/superpowers/specs/2026-09-08-1138-per-cookie-template-cache-policy-design.md @@ -0,0 +1,466 @@ +# Per-cookie shared-template cache policy + +Status: Implemented and independently reviewed; validation results are recorded in the implementation plan. + +Issue: [#1138](https://github.com/IABTechLab/trusted-server/issues/1138). + +## 1. Purpose + +Allow publishers to share reader-neutral HTML templates across anonymous readers +while separating bounded cookie-selected variants and excluding requests whose +cookies can cause personal HTML. Extend the existing cookie-independence assertion +without weakening the default or the response eligibility guards. + +This specification defines the issue's two configuration lists. The parsing, +validation, and compatibility rules below are proposed design decisions that make +the issue's behavior precise enough for implementation and testing. + +## 2. Current behavior and failure modes + +The relevant cache contains transformed origin HTML before per-reader assembly. +It is distinct from the platform's raw-origin cache and from the final response, +which must remain private and must never become a shared per-user response cache. + +In `crates/trusted-server-core/src/publisher.rs`, the publisher handler computes +`cookie_disqualifies` before sending the origin request. Currently any `Cookie` +header disqualifies the request unless `origin_is_cookie_independent` is true. +The same decision protects both lookup and storage. TS mints an identity cookie, +so the conservative default excludes most repeat visitors. + +The handler constructs `TemplateCacheKey` before the origin fetch. Configured +`template_cache_vary` values come from the request at TS's hop. On a response, +`template_cache_ttl` verifies that the configured header names cover the origin's +`Vary` declaration; it cannot prove what a downstream intermediary did to values. + +### 2.1 Experiment cookie translated downstream + +Request flow: + +```text +Browser: Cookie: ab_bucket=A + -> TS: selects a template key; X-Exp-Variant is absent + -> Publisher CDN: translates ab_bucket into X-Exp-Variant: A + -> Origin: renders arm A and responds with Vary: X-Exp-Variant +``` + +With cookie independence enabled and only `x-exp-variant` configured as a header +dimension, both arm A and arm B have the same absent header at TS. The drift guard +accepts the declared name, but the shared key does not distinguish the arms. A +template populated by one arm can therefore be served to the other. + +### 2.2 Small logged-in population + +An origin may render personal account state when `session` is present, while all +other visitors receive shareable HTML. The current boolean cannot exempt that +session-bearing population while admitting anonymous readers carrying TS cookies. + +## 3. Scope and invariants + +The change must: + +- Add named cookie values as explicit per-variant template key dimensions. +- Exclude requests carrying named bypass cookies from both lookup and storage, + falling back to the existing inline path. +- Apply `origin_is_cookie_independent` only to cookies outside those lists. +- Preserve existing behavior when both lists are absent or empty. +- Keep all existing request and response eligibility gates in force. +- Keep templates reader-neutral within each variant and assemble per-reader state + after lookup as today. + +The change does not introduce automatic cookie discovery, cookie-to-header mapping, +cookie mutation, session validation, a value allowlist, a cardinality limiter, a new +cache backend, or a new assembly mode. It does not alter the authorization carve-out, +raw-origin caching, or final-response privacy behavior. + +No TS identity or consent cookie is implicitly exempted. Operators must still +assert independence for unlisted cookies when that assertion is valid for their +origin. A cookie that affects consent-dependent origin HTML needs an appropriate +key or bypass policy just like any other cookie. + +## 4. Configuration contract + +The fields live in the existing `[creative_opportunities]` table: + +```toml +[creative_opportunities] +assembly_mode = "esi" + +# Include all applicable existing header dimensions for the deployment. +template_cache_vary = ["x-exp-variant"] + +# Bounded, reader-neutral variants only. +template_cache_key_cookies = ["ab_bucket"] + +# Presence, including an empty value, requires inline processing. +template_cache_bypass_cookies = ["session"] + +# Assertion applies only to cookies outside the two lists above. +origin_is_cookie_independent = true +``` + +Use `Option>` for each new field, with the existing +`#[serde(default, skip_serializing_if = "Option::is_none")]` convention. Missing +and empty lists have identical runtime meaning. Missing fields remain omitted +when serializing configuration. The boolean retains its default of false. + +### 4.1 Name validation + +Validate configuration at the existing creative-opportunities validation boundary: + +- Names are nonempty ASCII HTTP tokens: letters, digits, and + ``! # $ % & ' * + - . ^ _ ` | ~``. Whitespace, separators such as `;` or `=`, + control bytes, and non-ASCII bytes are invalid. +- Match names exactly and case-sensitively. Do not lowercase them as header names + are lowercased; `session` and `Session` are distinct cookie names. +- Reject TS identity cookie names (`ts-ec`, `ts-eids`, `sharedId`) in the key list; + allow them in the bypass list. +- Reject duplicates within either list and overlap between the two lists. Report + the offending field and name as a configuration error, using existing error + handling conventions. +- No wildcard, prefix, or regular-expression matching is supported. +- Preserve the existing prohibition on `Cookie` and `Authorization` in + `template_cache_vary`. + +Rejecting overlap makes contradictory operator intent visible at configuration +load. Runtime bypass still takes precedence whenever a request contains any +configured bypass cookie alongside configured key cookies. + +### 4.2 Operator responsibility + +Key cookies must represent bounded variants, such as experiment arms or region +buckets. Account IDs, session tokens, TS identity IDs, and other user identifiers +must not be keyed. Hashing such identifiers would still create a per-user cache +and would not make the template reader-neutral. + +The implementation cannot infer cardinality or validate the publisher's semantic +assertion from a cookie name. Choosing stable, bounded values and identifying all +origin HTML dependencies remain operator responsibilities. Arbitrary values can +fragment the cache; this change does not add admission or cardinality controls. + +## 5. Eligibility semantics + +Evaluate the policy at the existing pre-fetch request gate, against all `Cookie` +fields in the request that is about to be forwarded to the publisher origin. + +Preserve existing request preparation. In particular, GPT diagnostics preparation +removes its reserved cookie, drops fields that fail `to_str()`, removes empty +pairs, and combines retained pairs before generic cookie handling. The policy +classifies the resulting origin inputs; it does not recover or classify removed +browser bytes. Raw-header tests must also exercise the already-prepared request +boundary so earlier sanitization does not mask evaluator coverage. + +When both configured lists are empty, retain the exact existing decision: + +```text +cookie_disqualifies = Cookie header is present AND independence is false +cookie key dimensions = none +``` + +This legacy path deliberately preserves existing behavior even for empty or +malformed headers. The new parser does not silently change old deployments. + +When either list is nonempty: + +1. No `Cookie` header means no cookie disqualification. Each configured key cookie + contributes an explicit absent dimension. +2. Parse and validate all fields using section 6. Unclassifiable or ambiguous + input disqualifies the request. +3. Presence of any bypass cookie disqualifies the request, regardless of its value + or the independence assertion. +4. With independence false, any unlisted cookie disqualifies the request. +5. Otherwise the cookie gate admits the request. Extract all configured key-cookie + dimensions, including absence where applicable. + +Admission by this gate is necessary but insufficient for shared caching: GET, ESI +mode, authorization, request cache semantics, response shareability, and all other +existing gates still apply. + +For key list `["ab_bucket"]` and bypass list `["session"]`: + +| Request cookies | Independence false | Independence true | Key contribution if admitted | +| ---------------------------- | ------------------ | ----------------- | -------------------------------- | +| No header | Admit | Admit | `ab_bucket` absent | +| `ab_bucket=A` | Admit | Admit | `ab_bucket` present, value `A` | +| `ab_bucket=` | Admit | Admit | `ab_bucket` present, empty value | +| `ts-ec=reader1` | Bypass | Admit | `ab_bucket` absent | +| `ab_bucket=A; ts-ec=reader1` | Bypass | Admit | `ab_bucket` present, value `A` | +| `session=` | Bypass | Bypass | None | +| `ab_bucket=A; session=token` | Bypass | Bypass | None | +| `Session=token` | Bypass as unlisted | Admit | `ab_bucket` absent | +| Ambiguous or malformed input | Bypass | Bypass | None | + +Compute the cookie decision once, before the origin request is consumed. Reuse it +for both lookup eligibility and the response-side store gate. A bypassed request +must not perform cache lookup, acquire a cache reservation, or store a template, +even if a matching anonymous template already exists or the origin response is +otherwise shareable. It must take the existing origin/inline fallback. + +## 6. Cookie parsing contract + +Use a cache-policy-specific parser or evaluator with a narrow interface. Do not +change the semantics of unrelated cookie helpers. + +Existing `cookies.rs` helpers are insufficient as-is: `extract_cookie_value` reads +one selected header and the first matching pair, while the `CookieJar` helper +skips invalid pairs and collapses duplicate names. The origin can receive all +header fields, so those behaviors could discard a cache-relevant signal. + +For the named-policy path: + +- Inspect every `Cookie` field in wire order. Accept multiple fields when all + contain valid pairs with unique names across the complete request. +- Split each field on semicolons. Trim only surrounding space and horizontal tab + from each pair. Empty fields or empty pairs, including a trailing semicolon, + cause bypass rather than being silently discarded. +- Split the trimmed pair on its first `=`. Require a valid nonempty token name + immediately before it. A bare name or remaining whitespace in the name or value + is malformed and bypasses. Pair trimming happens first: `ab_bucket= ` becomes + an accepted empty value, while `ab_bucket =A` and `ab_bucket= A` bypass. +- For key cookies, accept an empty value. Otherwise accept unquoted cookie-octet bytes, or a value + enclosed by exactly one matching pair of double quotes containing cookie-octet + bytes. Cookie-octet bytes are hexadecimal `21`, `23–2B`, `2D–3A`, `3C–5B`, and + `5D–7E`. This excludes whitespace, controls, comma, semicolon, double quote, + backslash, and non-ASCII bytes from the value payload. +- For unlisted cookies only when independence is true, additionally accept commas + and balanced double quotes. This admits compact JSON and comma/colon lists seen + in browser cookies. Continue rejecting whitespace, backslashes, controls, + non-ASCII bytes, unmatched quotes, and comma-delimited fragments whose prefix + before `=` is a valid cookie name. Quotes cannot span semicolon-delimited pairs. + Bypass-cookie presence remains unconditional; unlisted cookies with independence + false still bypass regardless of value. This assumes origin cookie parsing + treats unlisted values as opaque and parses each semicolon-separated pair + independently. An origin parser that stops at nonstandard JSON could observe + different key cookies depending on order; that deployment cannot assert this + independence. The evaluator does not normalize or remove the ignored values. +- Preserve the original value bytes, including allowed `=` characters, case, + percent escapes, and any surrounding quotes. Do not URL-decode, unquote, or + otherwise normalize values. Quoted and unquoted representations may use + separate keys; over-separation is safer than merging distinct inputs. +- Repeated key-cookie names cause bypass even when their values match. Different + origins can interpret duplicates differently; this design does not choose first + or last wins. Repeated unlisted names are allowed only with independence asserted + and every value passing framing checks. Bypass names always bypass. +- For requests reaching this evaluator, malformed input or an unsupported byte + sequence causes cache bypass. The evaluator introduces no new request error. + Existing earlier validation errors remain unchanged for fields surviving + preparation: the publisher calls + `handle_request_cookies` before the cache gate, and failure of `to_str()` on its + selected header still returns the existing `InvalidHeaderValue` error. An + invalid later field that earlier parsing does not inspect must cause bypass + when the policy evaluator inspects all fields. + +Parsing may conservatively reduce cache hits for nonconforming clients. This is +intentional only when a named policy is active; the no-list compatibility path +continues to follow the previous boolean behavior. + +## 7. Cache key representation + +Extend `platform::TemplateCacheKey` with an explicit cookie-dimension collection, +using a small domain type with a cookie name and optional raw value bytes. +`None` means absent; a present zero-length value means `name=`. Do not reuse the +header namespace or inject synthetic headers. + +For admitted requests: + +- Include every configured key-cookie name, sorted by exact name bytes, with its + presence and value. Multiple configured cookies form a combined variant. +- Exclude bypass and unlisted cookie values entirely. +- Encode cookie dimensions in a separate domain-tagged, length-prefixed section + of the existing SHA-256 canonical key input. Include a count, each name, an + explicit presence marker, and the length-prefixed value when present. +- Append that section only when key-cookie dimensions are nonempty. An empty + collection must produce the same canonical bytes as the existing key format. +- Keep URL surrogate keys unchanged so a URL purge removes all cookie variants. + Keep the global template purge key unchanged. + +Thus cookie ordering and header splitting do not fragment valid equivalent +requests, but absent/empty values, different arms, and different cookie names do +not collide. Reordering configuration may still invalidate entries through the +existing complete-settings fingerprint; this harmless over-invalidation need +not be optimized away. + +The template fingerprint already hashes the complete typed settings and TSJS +content. The new fields must participate through ordinary serialization, so +policy changes cannot reuse an entry admitted under a different policy. + +No transform schema-version bump is required: template bytes and assembly markers +do not change. Preserve old keys when new fields are omitted and dimensions are +empty; nonempty dimensions and changed settings separate the new keys. + +Raw cookie values must not be added to logs, diagnostic response headers, metric +labels, or error messages. Do not log the expanded key through its `Debug` +representation. The opaque hashed backend key retains the existing diagnostics +boundary. + +## 8. Response guards and downstream variants + +Leave `template_cache_ttl` response requirements intact. In particular: + +- `Vary: Cookie` always refuses storage, including mixed-case names and repeated + `Vary` fields, even when all request cookies are keyed or asserted irrelevant. +- `Vary: *`, uncovered header names, origin `Set-Cookie`, lack of authorized + positive shared freshness, and other existing disqualifiers remain effective. +- The new cookie list does not automatically cover a header named in `Vary`. + +For the motivating topology, operators must configure both `ab_bucket` as a key +cookie and `x-exp-variant` as a header dimension. The former distinguishes readers +at TS; the latter satisfies the existing header coverage contract. An absent +keyed cookie gets its own dimension because the CDN may choose a distinct default +arm for it. + +This is still an operator assertion: TS cannot verify that the downstream header +is a deterministic function of the configured cookie. If the CDN also selects +HTML using another cookie or another unrepresented signal, that dependency needs +its own appropriate key or bypass policy. The old unsafe header-only +configuration is not detected or repaired automatically by this feature. + +Response drift checks run when an origin response is fetched, not on a cache hit. +They cannot retroactively validate an already cached template against an origin +policy change. Existing freshness and purge mechanisms remain necessary. + +## 9. Implementation boundaries + +| Location | Intended responsibility | +| ----------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------- | +| `crates/trusted-server-core/src/creative_opportunities.rs` | Optional configuration fields, validation, accessors as needed, and updated documentation of scoped independence | +| `crates/trusted-server-core/src/cookies.rs` or a focused adjacent core module | Pure named-policy parsing/evaluation with an explicit admitted/bypassed result and key-cookie dimensions | +| `crates/trusted-server-core/src/platform/template_cache.rs` | Cookie dimension type and canonical hashed key extension | +| `crates/trusted-server-core/src/platform/mod.rs` | Export the new dimension type if required by existing platform API conventions | +| `crates/trusted-server-core/src/publisher.rs` | Evaluate once, feed both gates, construct key dimensions, retain existing inline fallback and response guards | +| Existing core and Fastly adapter test fixtures | Update explicit configuration and key struct literals with empty/absent new fields | +| `docs/guide/configuration.md` and `trusted-server.example.toml` | Policy semantics, deployment examples, limitations, and rollback instructions | + +Keep cookie policy independent of runtime SDKs. No new OS-specific dependency, +async runtime, or platform-specific implementation is needed. The Fastly adapter +continues consuming the shared opaque key; other adapters retain their current +cache capabilities. + +Follow existing error-stack conventions at the configuration error boundary; +retain the existing `validate_runtime` string-error interface. Cookie policy +classification failure is a normal cache bypass, not a new page error. It does +not suppress errors raised by request handling before the evaluator runs. +Retain `X-TS-Template-Cache: bypass-request` and existing bounded diagnostics; +this change does not require a new public diagnostic value or metrics subsystem. + +## 10. Acceptance and regression tests + +### 10.1 Configuration + +- Omitted new fields deserialize successfully, resolve to empty lists, and remain + omitted on serialization. Existing fixture serialization/fingerprints remain + unchanged when both fields are omitted. +- Explicit empty lists have legacy runtime semantics. Their serialized presence + may safely change the settings fingerprint. +- Valid token names, including distinct case variants, are accepted. +- Empty/invalid names, duplicate entries, and cross-list overlap fail validation. +- Raw `Cookie` and `Authorization` remain prohibited header dimensions. +- Test key-only, bypass-only, and both-list configurations, with the unused list + either omitted or explicitly empty. Both lists empty select the legacy path; + either list nonempty activates the named policy. + +### 10.2 Parsing and key isolation + +- Cover missing header, missing keyed cookie, empty value, quoted values, values + containing `=`, and exact case-sensitive name matching. +- Equivalent cookie ordering and valid splitting across multiple header fields + yield equal dimensions and keys for the same settings. +- Changed key-cookie values, changed key-cookie names, and absent versus empty + values produce different backend keys; multiple dimensions compose correctly. +- With independence true, changing only an unlisted TS identity cookie leaves the + key unchanged. +- Empty bypass values disqualify. A bypass cookie in a later header also + disqualifies; it must not be hidden by first-header extraction. +- Duplicate key names with equal or different values, across pairs or fields, bypass. + Duplicate unlisted names remain eligible only with independence asserted and valid + framing for every value. +- At the evaluator level, bare names, invalid names, empty pairs/fields, invalid + quoting, forbidden value bytes, and non-ASCII input bypass under a named policy. +- At the evaluator level, preserve legacy boolean decisions for those same + malformed headers when both lists are empty. This does not imply that every + such request reaches the evaluator through the publisher handler. +- Pin the unchanged backend key for an existing fixture with empty cookie + dimensions, and verify surrogate keys remain common to all URL variants. + +### 10.3 Publisher behavior + +Use the existing in-memory template cache and origin stubs to verify observable +lookup/reservation/store counts, origin requests, and rendered response content: + +1. With `x-exp-variant` absent at TS and origin responses declaring + `Vary: X-Exp-Variant`, arm A and arm B populate separate templates. Later + requests hit the correct arm without another origin fetch. Readers in the + same arm with different `ts-ec` values share when independence is true. +2. Warm an anonymous template, then send a session-bearing request. It reaches + origin, makes no shared-cache call, and uses inline processing. A session + request against an empty cache likewise never stores a template, even with an + otherwise shareable origin response. Personal origin bytes never enter cache. +3. Independence false admits requests containing only configured key cookies, + but adding any unlisted cookie bypasses. Independence true admits that same + unlisted cookie while still honoring bypass cookies. +4. A missing experiment cookie and an explicitly empty one cannot reuse each + other's template. A request carrying only ignored cookies uses the missing-arm + template when independence is true. +5. Ambiguous/malformed cookie requests that reach the policy evaluator bypass an + already warm cache and cannot store on a cold cache under a named policy. + Separately preserve the existing error for a selected header that fails + `to_str()`, and prove that invalid bytes in a later field bypass rather than + being ignored by the evaluator. Neither path may access or store a template. + Exercise these raw-field cases at the already-prepared request boundary, and + separately verify that normal diagnostics preparation retains its existing + sanitization and keys the actual forwarded cookies. +6. `Vary: Cookie` refuses storage under the new policy, including combined header + lists. An uncovered downstream header still refuses storage even with a + configured cookie dimension. +7. Existing `Set-Cookie`, authorization, request-method, freshness, privacy, and + default inline-mode regressions continue passing. Per-reader assembly remains + fresh on warm hits and is not captured in the shared template. +8. Changing a configured cookie policy changes the fingerprint/key and prevents + reuse of entries created under the previous policy. +9. Exercise each list independently: a bypass-only configuration shares anonymous + traffic and excludes session traffic with independence true; a key-only + configuration separates experiment arms and admits only listed cookies with + independence false. Repeat with the unused list explicitly empty to verify + that it does not disable the named policy. + +For the eventual implementation, run target-matched tests after runtime changes +and the full CI gates documented in `CLAUDE.md` before PR handoff, including +adapter tests/clippy, integration parity, JS checks, and docs formatting. This +spec-only change requires document review, whitespace validation, and docs +formatting; it does not claim runtime tests have been executed. + +## 11. Deployment, compatibility, and rollback + +Existing configurations retain their behavior. An operator who configures only +the boolean still gets its existing all-cookie meaning because no cookies have +been explicitly classified. Enabling ESI remains a separate prerequisite. + +For an experiment rollout, identify every signal that selects origin HTML, add +bounded cookie dimensions and session bypass names, and retain required header +dimensions. Assert independence for remaining cookies only after confirming that +they do not change origin HTML. Check arm-correct content and cache diagnostics +with anonymous and session-bearing traffic during the canary. + +Changing the typed policy automatically changes the template fingerprint. Old +entries become unreachable under the new policy and can expire normally; use +existing URL/global purge mechanisms when necessary during an incident. Purging +alone cannot fix an unsafe unchanged configuration. + +Older binaries use `deny_unknown_fields` and reject either new field even when +its list is empty. Before rolling back a binary, remove both new fields from the +operator configuration and select a safe older policy. If the origin depends on +cookies, restore `origin_is_cookie_independent = false` or disable ESI; retaining +true after removing the lists loses both variant separation and session bypass. +Update the guide's existing rollback field-removal list accordingly. + +## 12. Alternatives and decision + +| Approach | Trade-off | Decision | +| ------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------- | +| Explicit key and bypass cookie lists | Direct operator intent; narrowly extends existing gate and key; requires precise parsing | Recommended and specified here | +| Derive synthetic headers before key construction | Equivalent variant expressiveness, but requires a mapping configuration and header namespace and still needs bypass handling | Not selected | +| Keep the boolean and bypass all cookie-bearing traffic | Retains conservative safety but cannot support the two publisher deployment shapes | Retained only as the default compatibility path | + +Keying the complete `Cookie` header is excluded by the existing reader-neutral +template design: identity values would create a per-user response cache in +practice. The chosen policy admits only explicitly named variant dimensions and +uses inline processing for personal HTML. diff --git a/scripts/template-cache-local-test.sh b/scripts/template-cache-local-test.sh index 58d714cf8..790cbd8b0 100755 --- a/scripts/template-cache-local-test.sh +++ b/scripts/template-cache-local-test.sh @@ -179,13 +179,29 @@ class H(BaseHTTPRequestHandler): ("Surrogate-Control", "max-age=1200, stale-while-revalidate=21600, stale-if-error=604800"), ("Vary", "Accept-Encoding"), ] + page = PAGE + if self.path.startswith("/article/cookie-policy"): + # Model a downstream CDN selecting HTML after TS has selected its key. + # Clients deliberately send no X-Exp-Variant header. + assert self.headers.get("X-Exp-Variant") is None + cookies = {} + for field in self.headers.get_all("Cookie", []): + for pair in field.split(";"): + name, separator, value = pair.strip().partition("=") + if separator: + cookies[name] = value + variant = cookies.get("ab_bucket", "absent") or "empty" + session = "session" in cookies + marker = f"

variant={variant};session={str(session).lower()}

" + page = PAGE.replace(b"

Body copy.

", marker.encode()) + base.append(("Vary", "X-Exp-Variant")) if "gzip" in (self.headers.get("Accept-Encoding") or ""): print("origin: served COMPRESSED", flush=True) - self._send(gzip.compress(PAGE), "text/html; charset=utf-8", + self._send(gzip.compress(page), "text/html; charset=utf-8", base + [("Content-Encoding", "gzip")]) else: print("origin: served PLAINTEXT", flush=True) - self._send(PAGE, "text/html; charset=utf-8", base) + self._send(page, "text/html; charset=utf-8", base) def do_POST(self): print(f"origin: received POST {self.path}", flush=True) @@ -295,7 +311,9 @@ s = replace_once( # and must go at the end: inserted here it would swallow every scalar key that # follows into `[[creative_opportunities.slot]]`. scalars = f'''assembly_mode = "{mode}" -template_cache_vary = [] +template_cache_vary = ["x-exp-variant"] +template_cache_key_cookies = ["ab_bucket"] +template_cache_bypass_cookies = ["session"] origin_is_cookie_independent = true''' lines = s.split("\n") lines.insert(lines.index("[creative_opportunities]") + 1, scalars) @@ -303,7 +321,7 @@ lines.append(''' [[creative_opportunities.slot]] id = "ts-slot-header" div_id = "ts-slot-header" -page_patterns = ["/article"] +page_patterns = ["/article", "/article/cookie-policy*"] formats = [{ width = 728, height = 90 }] ''') open(out, "w").write("\n".join(lines)) @@ -762,6 +780,90 @@ if [ "$MODE" != "inline" ]; then "$(grep -c 'served PLAINTEXT' "$WORK/origin.log" || true)" "0" fi +info "Cookie variant isolation and session bypass (mode: $MODE)" +if python3 - "$TS_PORT" "$MODE" "$WORK/origin.log" "$REQUEST_TIMEOUT_SECONDS" <<'PYEOF' +import gzip +import sys +import urllib.request +from pathlib import Path + +port, mode, origin_log, timeout = sys.argv[1:] +opener = urllib.request.build_opener(urllib.request.ProxyHandler({})) + + +def origin_gets(path): + return Path(origin_log).read_text().splitlines().count(f"origin: received GET {path}") + + +def request(path, cookie, variant, state, fetches, session=False): + before = origin_gets(path) + headers = { + "Host": "ts.example.com", + "Accept-Encoding": "gzip", + "Sec-Fetch-Dest": "document", + "Sec-Fetch-Mode": "navigate", + } + if cookie is not None: + headers["Cookie"] = cookie + req = urllib.request.Request(f"http://127.0.0.1:{port}{path}", headers=headers) + with opener.open(req, timeout=float(timeout)) as response: + body = response.read() + if response.headers.get("Content-Encoding") == "gzip": + body = gzip.decompress(body) + html = body.decode() + assert response.status == 200, response.status + marker = f"

variant={variant};session={str(session).lower()}

" + assert marker in html, f"wrong cookie-selected HTML: expected {marker}" + assert html.count("

variant=") == 1, "should contain only this reader's variant" + assert "" not in html, "unresolved assembly seam" + assert '\\"hb_pb\\":\\"4.25\\"' in html, "missing assembled winning bid" + policy = response.headers.get("Cache-Control", "").lower() + assert "private" in policy and "no-store" in policy, policy + actual = response.headers.get("X-TS-Template-Cache") + if mode == "esi": + assert actual == state, f"{cookie!r}: expected {state}, got {actual}" + if state == "hit": + assert response.headers.get("X-TS-Assembly") == "byte-seam" + else: + assert actual not in ("hit", "miss-stored", "miss-reserved"), actual + expected_fetches = fetches if mode == "esi" else 1 + actual_fetches = origin_gets(path) - before + assert actual_fetches == expected_fetches, ( + f"{cookie!r}: expected {expected_fetches} origin fetches, got {actual_fetches}" + ) + print(f" PASS {path} {cookie!r}: correct HTML, cache state, assembly and origin count") + + +path = "/article/cookie-policy" +for arm, state in [("A", "miss-stored"), ("B", "miss-stored"), ("A", "hit"), ("B", "hit")]: + request(path, f"ab_bucket={arm}", arm, state, int(state != "hit")) + +# Presence is a key dimension: neither missing nor empty may reuse A, B, or each other. +for state in ["miss-stored", "hit"]: + request(path, None, "absent", state, int(state != "hit")) + request(path, "ab_bucket=", "empty", state, int(state != "hit")) + +# Unlisted opaque values must not fragment a warmed experiment arm. +request(path, 'g_state={"i_l":0}; ab_bucket=A', "A", "hit", 0) +request(path, "ab_bucket=A; metadata=one,two", "A", "hit", 0) + +# Bypass applies even when the anonymous arm is already warm, including empty sessions. +for cookie in ["ab_bucket=A; session=test", "ab_bucket=A; session=test", "ab_bucket=A; session="]: + request(path, cookie, "A", "bypass-request", 1, session=True) +request(path, "ab_bucket=A", "A", "hit", 0) + +# A cold session request must neither populate nor reserve an anonymous template. +cold_path = "/article/cookie-policy-cold-session" +request(cold_path, "ab_bucket=B; session=test", "B", "bypass-request", 1, session=True) +request(cold_path, "ab_bucket=B", "B", "miss-stored", 1) +request(cold_path, "ab_bucket=B", "B", "hit", 0) +PYEOF +then + ok "cookie runtime matrix" +else + bad "cookie runtime matrix" +fi + info "Result" printf ' %d passed, %d failed\n\n' "$PASS" "$FAIL" [ "$FAIL" -eq 0 ] diff --git a/trusted-server.example.toml b/trusted-server.example.toml index c484519ad..6d809f648 100644 --- a/trusted-server.example.toml +++ b/trusted-server.example.toml @@ -340,7 +340,7 @@ auction_timeout_ms = 500 # Initial-page delivery mode (spike/experimental). `inline` is the default and # current production behaviour; `esi` is an opt-in Fastly Core Cache experiment # storing an inert comment in a shared, reader-neutral template cache. See -# docs/guide/configuration.md before enabling. This and the three cache-safety +# docs/guide/configuration.md before enabling. This and the cache-safety # keys below belong in this [creative_opportunities] table. # assembly_mode = "inline" # Request headers (besides Accept-Encoding) the origin may name in Vary. Every @@ -355,10 +355,23 @@ auction_timeout_ms = 500 # Safety ceiling (seconds) for one shared template; TS uses the smaller of this # and the origin-authorized remaining edge freshness. Default 60; range 1-86400. # template_cache_max_age_seconds = 1200 -# Unsafe unless independently verified: excludes cookie-bearing requests from the -# template cache by default. Set true only when origin HTML is byte-independent -# of Cookie; an origin `Vary: Cookie` is still refused. +# Optional exact, case-sensitive cookie names; omitted and empty lists are equal. +# Key bounded variants only, never account, session, or TS identity IDs. +# Absent and empty values select different variants. No cardinality limit exists. +# template_cache_key_cookies = ["ab_bucket"] +# Presence, including session=, bypasses template lookup and storage. +# template_cache_bypass_cookies = ["session"] +# With either list nonempty, false bypasses unlisted cookies; true asserts those +# cookies do not change origin HTML. No identity or consent cookie is exempt. +# With both lists empty, false bypasses all Cookie headers, as before. +# An origin `Vary: Cookie` is always refused, regardless of these settings. # origin_is_cookie_independent = false +# Invalid names, duplicates, and cross-list overlap reject configuration. +# Named policies bypass ambiguous/nonconforming Cookie fields, including duplicates. +# If a downstream CDN maps ab_bucket to X-Exp-Variant, also add x-exp-variant to +# template_cache_vary; that header alone cannot separate arms at TS. +# Rollback: older binaries reject both new fields even as empty lists. Remove +# both fields AND restore independence=false or disable ESI when HTML uses cookies. # # `gam_unit_path` may be a template. Supported placeholders: # {network_id} -> gam_network_id