From 3ae4cdb45315be7e68f20455f3c45288d9182483 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Tue, 8 Sep 2026 14:59:09 +0530 Subject: [PATCH 1/8] Specify per-cookie template cache policy --- ...per-cookie-template-cache-policy-design.md | 420 ++++++++++++++++++ 1 file changed, 420 insertions(+) create mode 100644 docs/superpowers/specs/2026-09-08-1138-per-cookie-template-cache-policy-design.md 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..2d37332ec --- /dev/null +++ b/docs/superpowers/specs/2026-09-08-1138-per-cookie-template-cache-policy-design.md @@ -0,0 +1,420 @@ +# Per-cookie shared-template cache policy + +Status: Proposed specification for review; implementation will follow on the same feature branch. + +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 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. + +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. +- 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. +- 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. +- Any repeated cookie name anywhere in the request causes bypass, even when the + values match or the cookie is unlisted. Different origins can interpret + duplicates differently; this design does not choose first or last wins. +- Malformed input or an unsupported byte sequence causes cache bypass, not a new + request rejection. Preserve the existing request and origin handling. + +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 for configuration errors. Runtime +classification failure is a normal cache bypass, not an exceptional page error. +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. + +### 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 names with equal or different values, across pairs or fields, bypass. +- Bare names, invalid names, empty pairs/fields, invalid quoting, forbidden value + bytes, and non-ASCII input bypass under a named policy. +- Preserve legacy boolean decisions for those same malformed headers when both + lists are empty. +- 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 bypass an already warm cache and cannot + store on a cold cache under a named policy. +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. + +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. From e7ef989711411a34994876cee49e6186d62fbac7 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Tue, 8 Sep 2026 15:13:25 +0530 Subject: [PATCH 2/8] Resolve cookie policy review and plan implementation --- ...8-1138-per-cookie-template-cache-policy.md | 539 ++++++++++++++++++ ...per-cookie-template-cache-policy-design.md | 39 +- 2 files changed, 568 insertions(+), 10 deletions(-) create mode 100644 docs/superpowers/plans/2026-09-08-1138-per-cookie-template-cache-policy.md 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..203411abd --- /dev/null +++ b/docs/superpowers/plans/2026-09-08-1138-per-cookie-template-cache-policy.md @@ -0,0 +1,539 @@ +# 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 (`- [ ]`) 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. +- This document is a plan, not evidence that code or tests have run. All checkboxes + start unchecked. Proposed Rust below must receive normal imports, documentation, + formatting, and verification when installed in source. +- 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. + +## 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`. + +- [ ] **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. +- [ ] **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. +- [ ] **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. + +- [ ] **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. + +- [ ] **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. +- [ ] **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`. + +- [ ] **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. +- [ ] **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`. + +- [ ] **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. + +- [ ] **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`. + +- [ ] **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. +- [ ] **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. + +- [ ] **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. +- [ ] **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`. + +- [ ] **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. +- [ ] **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. + +- [ ] **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. +- [ ] **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`. + +- [ ] **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. +- [ ] **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. +- [ ] **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. +- [ ] **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. +- [ ] **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. + +- [ ] **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`. 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. +- [ ] **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. +- [ ] **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. +- [ ] **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. +- [ ] **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. + +- [ ] **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. +- [ ] **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. +- [ ] **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. +- [ ] **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. + +- [ ] **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. +- [ ] **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. + +- [ ] **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. +- [ ] **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. 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 index 2d37332ec..fc623b6e7 100644 --- 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 @@ -219,8 +219,13 @@ For the named-policy path: - Any repeated cookie name anywhere in the request causes bypass, even when the values match or the cookie is unlisted. Different origins can interpret duplicates differently; this design does not choose first or last wins. -- Malformed input or an unsupported byte sequence causes cache bypass, not a new - request rejection. Preserve the existing request and origin handling. +- 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: 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 @@ -308,8 +313,10 @@ 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 for configuration errors. Runtime -classification failure is a normal cache bypass, not an exceptional page error. +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. @@ -325,6 +332,9 @@ this change does not require a new public diagnostic value or metrics subsystem. - 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 @@ -339,10 +349,11 @@ this change does not require a new public diagnostic value or metrics subsystem. - Empty bypass values disqualify. A bypass cookie in a later header also disqualifies; it must not be hidden by first-header extraction. - Duplicate names with equal or different values, across pairs or fields, bypass. -- Bare names, invalid names, empty pairs/fields, invalid quoting, forbidden value - bytes, and non-ASCII input bypass under a named policy. -- Preserve legacy boolean decisions for those same malformed headers when both - lists are empty. +- 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. @@ -365,8 +376,11 @@ lookup/reservation/store counts, origin requests, and rendered response content: 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 bypass an already warm cache and cannot - store on a cold cache under a named policy. +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. 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. @@ -375,6 +389,11 @@ lookup/reservation/store counts, origin requests, and rendered response content: 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 From 80d462dfbb780cdbb997d8b6db50a2395d35bd7c Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Tue, 8 Sep 2026 15:44:51 +0530 Subject: [PATCH 3/8] Add per-cookie template cache policy --- .../src/template_cache.rs | 1 + crates/trusted-server-core/src/cookies.rs | 2 + .../src/cookies/template_cache_policy.rs | 362 +++++++++ .../src/creative_opportunities.rs | 143 +++- .../trusted-server-core/src/platform/mod.rs | 4 +- .../src/platform/template_cache.rs | 106 +++ crates/trusted-server-core/src/publisher.rs | 716 +++++++++++++++++- docs/guide/configuration.md | 96 ++- ...8-1138-per-cookie-template-cache-policy.md | 127 +++- ...per-cookie-template-cache-policy-design.md | 15 +- trusted-server.example.toml | 21 +- 11 files changed, 1495 insertions(+), 98 deletions(-) create mode 100644 crates/trusted-server-core/src/cookies/template_cache_policy.rs 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..d49e1073f --- /dev/null +++ b/crates/trusted-server-core/src/cookies/template_cache_policy.rs @@ -0,0 +1,362 @@ +//! Strict request-cookie classification for shared template caching. + +use std::collections::{HashMap, HashSet}; + +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 !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) || !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(), + ) +} + +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 +} + +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_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" + ); + } + for value in [b"".as_slice(), b"\"\"", b"!#$%&'()*+-./0123456789:<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_`abcdefghijklmnopqrstuvwxyz{|}~"] { + 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_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"a=x,y"], + vec![b"a=x\\y"], + vec![b"a=\xff"], + vec![b"\xff=1"], + vec![b"a=1; a=1"], + vec![b"a=1; a=2"], + vec![b"a=1", b"a=1"], + vec![b"a=1", b"a=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 ab272e4da..b7e1a8db5 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 names 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)) @@ -1353,6 +1381,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], @@ -1755,6 +1785,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(), @@ -2132,6 +2164,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 4ac8e17df..77b855b4b 100644 --- a/crates/trusted-server-core/src/platform/mod.rs +++ b/crates/trusted-server-core/src/platform/mod.rs @@ -64,8 +64,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 3d7e7e74d..ca9151baf 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; @@ -4279,20 +4280,27 @@ 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), + }; let suppress_datadome_client_side_tag = req .extensions() .get::() @@ -4379,6 +4387,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, }); @@ -5696,14 +5705,11 @@ 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")] + /// 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")] CookieForwarded, /// The origin varies on a header the cache key does not cover. /// @@ -8850,6 +8856,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, } @@ -11435,6 +11442,667 @@ 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}"); + 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; unknown=1; unknown=1".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_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" + ); + } + + #[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(&settings, &services, cookie_policy_request(&[field])).await; + 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("3.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 by_default_a_cookie_bearing_request_uses_no_shared_cache() { // The shipped default, and the reason the cache is nearly inert on real @@ -13234,6 +13902,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()], @@ -18451,6 +19121,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 a2b90b240..e03884b5c 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -1679,9 +1679,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. @@ -1702,8 +1702,12 @@ 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. Enable only after proving unlisted cookies do not change HTML. +origin_is_cookie_independent = false ``` The cache fails closed. A template is stored only for a `GET` with a processable @@ -1748,13 +1752,80 @@ 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, and overlap between lists. Wildcards, prefixes, and regular +expressions are not supported. With either list nonempty, strict parsing of all +`Cookie` fields after existing request preparation also bypasses ambiguous or nonconforming inputs, including +duplicate cookie names or malformed values. Cookie 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 @@ -1777,9 +1848,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. 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 index 203411abd..ea33ac0c8 100644 --- 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 @@ -1,6 +1,6 @@ # 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 (`- [ ]`) syntax for tracking. +> **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. @@ -19,14 +19,53 @@ - 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. -- This document is a plan, not evidence that code or tests have run. All checkboxes - start unchecked. Proposed Rust below must receive normal imports, documentation, - formatting, and verification when installed in source. +- 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 | @@ -70,7 +109,7 @@ After each completed runtime task, run the relevant target-matched suite; run `crates/trusted-server-core/src/publisher.rs`, and `crates/trusted-server-adapter-fastly/src/template_cache.rs`. -- [ ] **1.1 Verify the pre-change key contract.** Reuse the existing +- [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`. @@ -78,13 +117,13 @@ After each completed runtime task, run the relevant target-matched suite; run 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. -- [ ] **1.2 Add failing key-isolation tests.** Use the common prefix +- [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. -- [ ] **1.3 Add the public domain type and field.** Follow existing platform API +- [x] **1.3 Add the public domain type and field.** Follow existing platform API visibility and documentation conventions: ```rust @@ -101,7 +140,7 @@ After each completed runtime task, run the relevant target-matched suite; run publisher/evaluator owns sorting; the encoder consumes the supplied order as the existing header encoder does. -- [ ] **1.4 Extend canonical encoding after the complete existing header section.** +- [x] **1.4 Extend canonical encoding after the complete existing header section.** Reuse the existing length-prefix `push` helper: ```rust @@ -128,11 +167,11 @@ After each completed runtime task, run the relevant target-matched suite; run unchanged. Distinct framing of names and values must prevent concatenation collisions; add a test with differently partitioned names/values. -- [ ] **1.5 Update every literal with `cookie_values: Vec::new()`.** Locate them +- [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. -- [ ] **1.6 Verify and commit.** Run `cargo test-fastly template_cookie_key_`, +- [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, @@ -146,7 +185,7 @@ After each completed runtime task, run the relevant target-matched suite; run `crates/trusted-server-core/src/cookies/template_cache_policy.rs`, and explicit configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. -- [ ] **2.1 Add config tests with prefix `template_cookie_config_`.** Deserialize +- [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, @@ -154,7 +193,7 @@ configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. 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. -- [ ] **2.2 Add fields and borrowed accessors.** Place them alongside the current +- [x] **2.2 Add fields and borrowed accessors.** Place them alongside the current template cache fields and update boolean documentation to scoped semantics: ```rust @@ -170,7 +209,7 @@ configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. materialize absent options. Add `None` fields to existing struct literals; locate them with `rg -n 'CreativeOpportunitiesConfig \{' crates`. -- [ ] **2.3 Create shared name validation.** Add +- [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: @@ -212,7 +251,7 @@ configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. `Settings::validate` conversion into `Report`; do not invent a new error stack for this helper. Expand the validation method's error docs. -- [ ] **2.4 Verify and commit.** Run +- [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`. @@ -223,14 +262,14 @@ configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. **File:** `crates/trusted-server-core/src/cookies/template_cache_policy.rs`. -- [ ] **3.1 Add decision-matrix tests before implementation.** Use prefix +- [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. -- [ ] **3.2 Implement an admitted/bypassed result.** Keep it internal: +- [x] **3.2 Implement an admitted/bypassed result.** Keep it internal: ```rust pub(crate) enum TemplateCookieDecision { @@ -325,14 +364,14 @@ configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. every admitted request must have validated every field. Only key-cookie values are owned in the result; parsed unlisted values remain borrowed and temporary. -- [ ] **3.3 Add malformed/ambiguous-input tests.** Cover repeated fields, duplicate +- [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. -- [ ] **3.4 Run evaluator tests and the Fastly suite.** Run +- [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. @@ -341,7 +380,7 @@ configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. **File:** `crates/trusted-server-core/src/publisher.rs`. -- [ ] **4.1 Add failing key-only and bypass-only integration tests.** Use prefix +- [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 @@ -349,7 +388,7 @@ configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. 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. -- [ ] **4.2 Replace the blanket cookie check at the existing pre-fetch point.** +- [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: @@ -372,14 +411,14 @@ configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. key inside `request_can_use_shared_template.then(...)`. Leave all other gate predicates and the earlier `handle_request_cookies(&req)?` call untouched. -- [ ] **4.3 Verify cache-call and inline behavior.** For both cold and warm cache, +- [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. -- [ ] **4.4 Verify and commit Tasks 3–4 together.** Run +- [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 @@ -389,7 +428,7 @@ configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. **File:** `crates/trusted-server-core/src/publisher.rs`. -- [ ] **5.1 Reproduce the original header-only failure in a regression.** Configure +- [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 @@ -399,24 +438,24 @@ configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. 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. -- [ ] **5.2 Cover absence, empty values, and unknown-cookie policy.** Missing +- [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. -- [ ] **5.3 Cover response refusal under the new configuration.** With valid keyed +- [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. -- [ ] **5.4 Verify readers remain separate after a warm hit.** Adapt the existing +- [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. -- [ ] **5.5 Verify and commit.** Name the new tests with the +- [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 @@ -427,30 +466,36 @@ configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. **Files:** `crates/trusted-server-core/src/publisher.rs` and `crates/trusted-server-core/src/creative_opportunities.rs` tests. -- [ ] **6.1 Preserve the selected-header error.** Build a Cookie `HeaderValue` from +- [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`. Use a narrow fallible test runner or + 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. -- [ ] **6.2 Distinguish later-field input.** Send a valid selected field followed +- [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. -- [ ] **6.3 Pin policy fingerprinting.** Extend `template_fingerprint_tests` to + 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. -- [ ] **6.4 Verify default compatibility.** Retain existing +- [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. -- [ ] **6.5 Verify and commit.** Run +- [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 @@ -461,23 +506,23 @@ configuration fixtures in `crates/trusted-server-core/src/publisher.rs`. **Files:** `docs/guide/configuration.md`, `trusted-server.example.toml`, and the spec/plan status checkboxes when appropriate. -- [ ] **7.1 Update operator examples.** Add commented optional lists alongside +- [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. -- [ ] **7.2 Explain the downstream header contract.** Show why a header created +- [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. -- [ ] **7.3 Document safe rollback.** Add both fields to the existing list that +- [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. -- [ ] **7.4 Verify and commit.** Run `npm run format` from `docs`, and +- [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`. @@ -486,11 +531,11 @@ spec/plan status checkboxes when appropriate. **Files:** Only fixes directly required by the feature and validation evidence. -- [ ] **8.1 Inspect the final diff against the feature base.** Check that no raw +- [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. -- [ ] **8.2 Run the full repository CI command set.** Execute each command and +- [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. @@ -528,11 +573,11 @@ spec/plan status checkboxes when appropriate. 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. -- [ ] **8.3 Request an independent implementation review.** Use +- [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. -- [ ] **8.4 Mark completed tasks and report evidence.** Summarize variant isolation, +- [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 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 index fc623b6e7..d26a5d861 100644 --- 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 @@ -1,6 +1,6 @@ # Per-cookie shared-template cache policy -Status: Proposed specification for review; implementation will follow on the same feature branch. +Status: Implemented and independently reviewed; validation results are recorded in the implementation plan. Issue: [#1138](https://github.com/IABTechLab/trusted-server/issues/1138). @@ -140,6 +140,13 @@ fragment the cache; this change does not add admission or cardinality controls. 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 @@ -221,7 +228,8 @@ For the named-policy path: duplicates differently; this design does not choose first or last wins. - 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: the publisher calls + 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 @@ -381,6 +389,9 @@ lookup/reservation/store counts, origin requests, and rendered response content: 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. diff --git a/trusted-server.example.toml b/trusted-server.example.toml index 83bb0d196..fe1a1043a 100644 --- a/trusted-server.example.toml +++ b/trusted-server.example.toml @@ -301,7 +301,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 @@ -316,10 +316,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 From ba5b88d526cd29d96cffb38edaa9187dd6223974 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Tue, 8 Sep 2026 16:05:43 +0530 Subject: [PATCH 4/8] Strengthen cookie cache forwarding and reader assembly tests --- crates/trusted-server-core/src/publisher.rs | 48 ++++++++++++++++++--- 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index ca9151baf..653d45d2f 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -9311,7 +9311,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; @@ -9320,7 +9320,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 { @@ -9363,7 +9365,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, @@ -9468,9 +9470,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, @@ -11949,6 +11961,18 @@ mod tests { 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] @@ -12048,8 +12072,18 @@ mod tests { if index == 0 { queue_shareable_html(&stub); } - let response = - run_bidding(&settings, &services, cookie_policy_request(&[field])).await; + 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() @@ -12064,7 +12098,7 @@ mod tests { .get("test-slot") .and_then(|bid| bid.get("hb_pb")) .and_then(serde_json::Value::as_str), - Some("3.50"), + Some(if index == 0 { "3.50" } else { "7.50" }), "should assemble this request's winning bid" ); } From d38c0d3afd730dccd3bc333f63900cbf8e479474 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Tue, 8 Sep 2026 18:16:48 +0530 Subject: [PATCH 5/8] Allow compact JSON values in ignored template cookies --- .../src/cookies/template_cache_policy.rs | 103 +++++++++++++++++- crates/trusted-server-core/src/publisher.rs | 71 ++++++++++++ docs/guide/configuration.md | 14 ++- ...8-1138-per-cookie-template-cache-policy.md | 18 +++ ...per-cookie-template-cache-policy-design.md | 13 ++- 5 files changed, 211 insertions(+), 8 deletions(-) diff --git a/crates/trusted-server-core/src/cookies/template_cache_policy.rs b/crates/trusted-server-core/src/cookies/template_cache_policy.rs index d49e1073f..c552dc4b4 100644 --- a/crates/trusted-server-core/src/cookies/template_cache_policy.rs +++ b/crates/trusted-server-core/src/cookies/template_cache_policy.rs @@ -1,4 +1,4 @@ -//! Strict request-cookie classification for shared template caching. +//! Request-cookie classification for shared template caching. use std::collections::{HashMap, HashSet}; @@ -67,7 +67,7 @@ pub(crate) fn evaluate_cookie_policy( }; let (name, rest) = pair.split_at(separator); let value = &rest[1..]; - if !is_cookie_name(name) || !is_cookie_value(value) { + if !is_cookie_name(name) { return TemplateCookieDecision::Bypass; } if parsed.insert(name, value).is_some() { @@ -76,7 +76,12 @@ pub(crate) fn evaluate_cookie_policy( if bypass_names.iter().any(|item| item.as_bytes() == name) { return TemplateCookieDecision::Bypass; } - if !independent && !key_names.iter().any(|item| item.as_bytes() == name) { + let keyed = key_names.iter().any(|item| item.as_bytes() == name); + if keyed { + if !is_cookie_value(value) { + return TemplateCookieDecision::Bypass; + } + } else if !independent || !is_ignored_cookie_value(value) { return TemplateCookieDecision::Bypass; } } @@ -115,6 +120,31 @@ fn trim_pair(mut pair: &[u8]) -> &[u8] { 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'"') { @@ -309,6 +339,71 @@ mod tests { } } + #[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"#, + r#"ignored={"value":1}; ignored={"value":2}; 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 [ @@ -328,7 +423,7 @@ mod tests { vec![b"a=\"x\"y\""], vec![b"a=x y"], vec![b"a=x\ty"], - vec![b"a=x,y"], + vec![b"ab_bucket=x,y"], vec![b"a=x\\y"], vec![b"a=\xff"], vec![b"\xff=1"], diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index 653d45d2f..95614f7d2 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -11923,6 +11923,77 @@ mod tests { ); } + #[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); diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index e03884b5c..137a46ee9 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -1780,9 +1780,17 @@ 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, and overlap between lists. Wildcards, prefixes, and regular -expressions are not supported. With either list nonempty, strict parsing of all -`Cookie` fields after existing request preparation also bypasses ambiguous or nonconforming inputs, including -duplicate cookie names or malformed values. Cookie values are not exposed in +expressions are not supported. With either list nonempty, parsing of all +`Cookie` fields after existing request preparation bypasses duplicate names, +invalid names, and malformed key-cookie values. When independence is `true`, +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. 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 index ea33ac0c8..73827762d 100644 --- 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 @@ -582,3 +582,21 @@ spec/plan status checkboxes when appropriate. 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. 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 index d26a5d861..fcaaa9e64 100644 --- 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 @@ -214,11 +214,22 @@ For the named-policy path: 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. -- Accept an empty value. Otherwise accept unquoted cookie-octet bytes, or a value +- 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 From afd71a74d2adc3af495e5920cbb93ed247c2c9a9 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Wed, 9 Sep 2026 14:18:53 +0530 Subject: [PATCH 6/8] Test EC withdrawal on warm template hits --- crates/trusted-server-core/src/publisher.rs | 241 ++++++++++++++++++++ 1 file changed, 241 insertions(+) diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index 25324f7e6..a0af756a3 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -12251,6 +12251,247 @@ mod tests { } } + #[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 = match finalizer { + Finalizer::Streaming => publisher_response_into_streaming_response( + response, + &Method::GET, + Arc::clone(&settings), + ®istry, + orchestrator, + services.clone(), + ) + .await + .expect("should finalize streaming reader response"), + Finalizer::Buffered => buffer_publisher_response_async( + response, + &Method::GET, + &settings, + ®istry, + &orchestrator, + &services, + ) + .await + .expect("should finalize buffered reader response"), + }; + 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 From 191380f02c52c77c825c63beedcb4dbd7bdcf0e9 Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Thu, 10 Sep 2026 17:50:31 +0530 Subject: [PATCH 7/8] Verify cookie template policies in the local runtime harness --- docs/guide/configuration.md | 6 +- ...8-1138-per-cookie-template-cache-policy.md | 39 +++++++ scripts/template-cache-local-test.sh | 110 +++++++++++++++++- 3 files changed, 150 insertions(+), 5 deletions(-) diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index f13c8be74..b16890b55 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -2074,7 +2074,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 index 73827762d..4624b34d8 100644 --- 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 @@ -600,3 +600,42 @@ above is superseded for unlisted values by the updated spec section 6: 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/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 ] From cd9ccd4ef2dd4b645b2beade530d0489b9f6356c Mon Sep 17 00:00:00 2001 From: prk-Jr Date: Fri, 11 Sep 2026 16:29:32 +0530 Subject: [PATCH 8/8] Refine template cookie policy safeguards --- .../src/cookies/template_cache_policy.rs | 86 +++++++++++++++++-- .../src/creative_opportunities.rs | 2 +- crates/trusted-server-core/src/publisher.rs | 74 +++++++++------- docs/guide/configuration.md | 12 ++- ...per-cookie-template-cache-policy-design.md | 13 ++- 5 files changed, 139 insertions(+), 48 deletions(-) diff --git a/crates/trusted-server-core/src/cookies/template_cache_policy.rs b/crates/trusted-server-core/src/cookies/template_cache_policy.rs index c552dc4b4..1f67ac62a 100644 --- a/crates/trusted-server-core/src/cookies/template_cache_policy.rs +++ b/crates/trusted-server-core/src/cookies/template_cache_policy.rs @@ -2,6 +2,7 @@ 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. @@ -29,6 +30,13 @@ pub(crate) fn validate_cookie_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" @@ -70,13 +78,13 @@ pub(crate) fn evaluate_cookie_policy( if !is_cookie_name(name) { return TemplateCookieDecision::Bypass; } - if parsed.insert(name, value).is_some() { + 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; } - let keyed = key_names.iter().any(|item| item.as_bytes() == name); if keyed { if !is_cookie_value(value) { return TemplateCookieDecision::Bypass; @@ -221,6 +229,61 @@ mod tests { } } + #[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 [ @@ -332,10 +395,16 @@ mod tests { "should preserve representation and ignore pair order and splitting" ); } - for value in [b"".as_slice(), b"\"\"", b"!#$%&'()*+-./0123456789:<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_`abcdefghijklmnopqrstuvwxyz{|}~"] { + 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"); + 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" + ); } } @@ -389,7 +458,6 @@ mod tests { r#"ignored="; ab_bucket=A"#, r#"ignored={"value":"x;session=login"}; ab_bucket=A"#, r#"ignored={"value":"x\y"}; ab_bucket=A"#, - r#"ignored={"value":1}; ignored={"value":2}; ab_bucket=A"#, ] { assert_eq!( evaluate_cookie_policy( @@ -427,10 +495,10 @@ mod tests { vec![b"a=x\\y"], vec![b"a=\xff"], vec![b"\xff=1"], - vec![b"a=1; a=1"], - vec![b"a=1; a=2"], - vec![b"a=1", b"a=1"], - vec![b"a=1", b"a=2"], + 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="], diff --git a/crates/trusted-server-core/src/creative_opportunities.rs b/crates/trusted-server-core/src/creative_opportunities.rs index 198818e56..a4e62e9d7 100644 --- a/crates/trusted-server-core/src/creative_opportunities.rs +++ b/crates/trusted-server-core/src/creative_opportunities.rs @@ -341,7 +341,7 @@ pub struct CreativeOpportunitiesConfig { /// Cookie names whose presence forces inline processing, with no lookup or store. /// /// Empty values still count as present. Names must be unique and must not overlap - /// [`Self::template_cache_key_cookies`]. Unset or empty names no bypass cookies. + /// [`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. diff --git a/crates/trusted-server-core/src/publisher.rs b/crates/trusted-server-core/src/publisher.rs index a0af756a3..693e4e011 100644 --- a/crates/trusted-server-core/src/publisher.rs +++ b/crates/trusted-server-core/src/publisher.rs @@ -4302,6 +4302,12 @@ pub async fn handle_publisher_request( 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::() @@ -5724,7 +5730,7 @@ pub(crate) enum TemplateCacheBypassReason { /// 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")] - CookieForwarded, + 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 @@ -6148,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); @@ -9568,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(), ) @@ -9583,7 +9608,7 @@ mod tests { publisher_response, &Method::GET, settings, - ®istry, + registry, &orchestrator, services, ) @@ -11713,7 +11738,9 @@ mod tests { ); } for (index, arm) in ["A", "B", "A", "B"].iter().enumerate() { - let cookies = format!("ab_bucket={arm}; ts-ec=reader{index}"); + 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"), @@ -11859,7 +11886,7 @@ mod tests { 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; unknown=1; unknown=1".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()], @@ -12349,28 +12376,15 @@ mod tests { ec_context.kv_snapshot().entry_for(identity).is_some(), "should preload this reader's identity even during withdrawal on a hit" ); - let mut response = match finalizer { - Finalizer::Streaming => publisher_response_into_streaming_response( - response, - &Method::GET, - Arc::clone(&settings), - ®istry, - orchestrator, - services.clone(), - ) - .await - .expect("should finalize streaming reader response"), - Finalizer::Buffered => buffer_publisher_response_async( - response, - &Method::GET, - &settings, - ®istry, - &orchestrator, - &services, - ) - .await - .expect("should finalize buffered reader response"), - }; + let mut response = finalize_test_publisher_response( + response, + &settings, + &services, + ®istry, + orchestrator, + finalizer, + ) + .await; crate::ec::finalize::ec_finalize_response( &settings, &mut ec_context, @@ -14072,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 @@ -14088,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" ); } diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index b16890b55..c67e4da80 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -1909,7 +1909,9 @@ template_cache_max_age_seconds = 1200 template_cache_key_cookies = ["ab_bucket"] template_cache_bypass_cookies = ["session"] -# Default false. Enable only after proving unlisted cookies do not change HTML. +# 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 ``` @@ -1982,10 +1984,12 @@ The optional cookie lists control both template lookup and storage: 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, and overlap between lists. Wildcards, prefixes, and regular -expressions are not supported. With either list nonempty, parsing of all -`Cookie` fields after existing request preparation bypasses duplicate names, +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, 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 index fcaaa9e64..190e5e99b 100644 --- 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 @@ -112,6 +112,8 @@ Validate configuration at the existing creative-opportunities validation boundar 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. @@ -234,9 +236,10 @@ For the named-policy path: 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. -- Any repeated cookie name anywhere in the request causes bypass, even when the - values match or the cookie is unlisted. Different origins can interpret - duplicates differently; this design does not choose first or last wins. +- 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 @@ -367,7 +370,9 @@ this change does not require a new public diagnostic value or metrics subsystem. 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 names with equal or different values, across pairs or fields, bypass. +- 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