Skip to content

Thread EC KV read through the request and recover orphaned cookies#885

Open
prk-Jr wants to merge 10 commits into
mainfrom
fix/kv-eid-request-snapshot-ec-ttl
Open

Thread EC KV read through the request and recover orphaned cookies#885
prk-Jr wants to merge 10 commits into
mainfrom
fix/kv-eid-request-snapshot-ec-ttl

Conversation

@prk-Jr

@prk-Jr prk-Jr commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Remove the redundant per-request EC identity-graph reads that gated the publisher-navigation hot path, and overlap the Fastly origin fetch with the KV lookup + auction dispatch. One request-scoped snapshot now feeds auction EID resolution, finalize, and pull sync (was up to three reads).
  • Recover an orphaned ts-ec cookie (valid cookie, missing KV row) by rotating to a freshly generated, KV-backed EC ID on consent-granted real-browser navigations — fixing the TTL-decay identity loss where quiet long-lived users silently lost their identity graph.
  • Collapse pull sync's per-partner read-modify-write into one request-wide bulk CAS, and make withdrawal tombstones existing-key-only so a forged cookie can never create a KV root.

Changes

File Change
ec/mod.rs EcKvSnapshot request-scoped state (NotRead/Missing/Present/Failed), EC-ID binding, recovery-eligibility flag; generate_if_needed uses bounded Add-only creation
ec/kv.rs load_snapshot, create_if_absent, snapshot-aware bulk upsert, existing-key-only conditional tombstone; CAS/refresh semantics + tests
ec/finalize.rs Consume/return snapshot, orphan recovery (browser-nav gated, atomic add-then-cookie), two-ID existing-only withdrawal
ec/prebid_eids.rs Split validated update collection (collect_eid_cookie_updates) from KV persistence
ec/pull_sync.rs Reuse finalized snapshot for eligibility; aggregate partner results across batches into one bulk write
auction/endpoints.rs Resolve auction EIDs from a snapshot instead of performing KV I/O
publisher.rs Origin/auction scheduling (concurrent: origin-first then KV+dispatch then await; eager: preload then dispatch then origin); rewrite_origin_request helper
http_util.rs Comment documenting the fastly-ssl scheme-signal layering wart (no behavior change)
adapter-fastly/{app,main}.rs Carry snapshot + navigation eligibility through EcRequestState/EcFinalizeState; mutable finalize outcome into post-send pull sync

Closes

Closes #851

Test plan

  • cargo test-fastly && cargo test-axum
  • cargo clippy-fastly && cargo clippy-axum
  • cargo fmt --all -- --check
  • JS tests: cd crates/trusted-server-js/lib && npx vitest run — N/A, no JS changes
  • JS format: cd crates/trusted-server-js/lib && npm run format — N/A, no JS changes
  • Docs format: cd docs && npm run format — N/A, no docs changes in code commits
  • WASM build: release artifact — not run locally (CI covers)
  • Manual testing via fastly compute serve
  • Other: also ran test-cloudflare, test-spin, cross-adapter parity, and all six clippy targets (fastly/axum/cloudflare/cloudflare-wasm/spin-native/spin-wasm); new tests verified under viceroy/wasm

Hardening note

N/A — no config-derived regex or pattern compilation touched. Fail-closed invariants covered by tests: KV errors never become authoritative misses (no rotation), tombstones are never revived, cookies are emitted only after a backing row exists, and store/CAS failures return a failed snapshot rather than claiming request-local IDs were persisted.

Checklist

  • Changes follow CLAUDE.md conventions
  • No unwrap() in production code — use expect("should ...")
  • Uses log macros (not println!)
  • New code has tests
  • No secrets or credentials committed

prk-Jr added 4 commits July 10, 2026 15:24
@prk-Jr
docs: design request-scoped EC KV reuse
1c78479
@prk-Jr
docs: plan EC KV snapshot implementation
1139646
@prk-Jr
fix: reuse EC KV state across publisher requests
a2fc5e2
@prk-Jr
Add EC KV snapshot test coverage and extract origin-rewrite helper
e53bef0 
Harden the request-scoped EC KV snapshot work (#851) with the test
coverage the spec's test strategy called for, plus small cleanups the
plan requested.

Tests:
- ec::kv: snapshot upsert (write-without-read, unchanged-preserves-gen,
  refresh-once, CAS re-merge, tombstone-rejects, store-fail) and
  conditional tombstone (CAS conflict, store failure, disappear-on-retry)
- ec::mod: generate_if_needed collision retry + exhaustion; default and
  read-path recovery-ineligibility (non-Fastly adapter contract)
- ec::prebid_eids: collect_eid_cookie_updates merge and empty-registry
- ec::finalize: NotRead/Failed/tombstone/subresource no-rotate paths and
  two-ID existing-only withdrawal
- ec::pull_sync: request-wide aggregation into one bulk write across
  concurrency batches, plus no-dispatch for non-present snapshots
- publisher: concurrent-vs-eager origin scheduling order and origin-start
  failure, using recording HTTP/KV collaborators

Cleanups:
- Extract rewrite_origin_request to remove the duplicated origin-rewrite
  logic across the concurrent and eager paths
- Bind the orphaned EC ID once in recover_orphaned_ec

Docs:
- Note the fastly-ssl vendor-header layering wart in core scheme detection
  and the origin-forwarding strip (comments only; behavior unchanged)
@prk-Jr prk-Jr self-assigned this Jul 10, 2026
prk-Jr and others added 3 commits July 13, 2026 17:36
@prk-Jr
Merge branch 'main' into fix/kv-eid-request-snapshot-ec-ttl
2c5fa4b
@prk-Jr
Re-read identity graph before skipping a withdrawal tombstone
ddcba88 
A Failed request-scoped snapshot no longer short-circuits
tombstone_existing_from_snapshot. A transient read error earlier in the
request must not silently drop a consent withdrawal, so a non-authoritative
snapshot is re-read (bounded by MAX_CAS_RETRIES) and the row is tombstoned
when present. An authoritative Missing snapshot stays a no-op.
@prk-Jr
Fix markdown formatting in EC KV snapshot plan doc
49a8977
@prk-Jr
prk-Jr marked this pull request as ready for review July 13, 2026 15:01
@prk-Jr
prk-Jr requested review from ChristianPavilonis and aram356 and removed request for ChristianPavilonis July 13, 2026 15:01
ChristianPavilonis
ChristianPavilonis requested changes Jul 13, 2026
View reviewed changes

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Changes requested. The snapshot/recovery design is promising, but I found two high-severity identity/privacy issues and three medium correctness, latency, and resource-usage issues. Details are attached inline.

Comment thread crates/trusted-server-adapter-fastly/src/app.rs Outdated
Comment thread crates/trusted-server-core/src/ec/kv.rs
Comment thread crates/trusted-server-core/src/publisher.rs Outdated
Comment thread crates/trusted-server-core/src/ec/kv.rs
Comment thread crates/trusted-server-core/src/auction/endpoints.rs Outdated
@ChristianPavilonis ChristianPavilonis mentioned this pull request Jul 13, 2026
Draft
14 tasks
@prk-Jr
Harden EC KV snapshot recovery against transient misses and misuse
81896a9 
Address PR review findings:

- Gate orphan-recovery eligibility to the publisher fallback after a
  successful origin start. Named routes, integration proxies, and
  request-filter short circuits no longer reach EC finalization with
  recovery authorized, so a blocked or non-publisher response cannot
  rotate an identity.
- Never downgrade an in-request Add-confirmed Present snapshot on a
  preload refresh miss, and confirm an authoritative miss with a second
  read (after the origin round trip) before rotating. A single
  eventually-consistent edge miss can no longer rotate a valid identity;
  a now-visible row is adopted instead.
- Preload the origin-overlapped snapshot with the unfiltered active EC
  ID while keeping the consent-filtered ID for auction identity, so
  consent-withdrawn navigations keep the withdrawal CAS off the
  post-origin latency path.
- Resolve the initial usable snapshot outside the CAS retry counter in
  both partner upsert and conditional tombstoning, so a
  generation-unavailable or refreshed snapshot keeps all five write
  attempts.
- Defer /auction and page-bids identity-graph reads until a live auction
  actually runs with a partner registry, avoiding billable KV reads that
  cannot be consumed.

Add tests for four-conflicts-then-fifth-write CAS, transient
Add->Missing->Present confirmation, and recovery-eligibility lifecycle
across named routes, filter short circuits, and origin-start failures.
prk-Jr added 2 commits July 16, 2026 14:07
@prk-Jr
Merge remote-tracking branch 'origin/main' into fix/kv-eid-request-sn…
8fb80ae 
…apshot-ec-ttl

# Conflicts:
#	crates/trusted-server-adapter-fastly/src/app.rs
#	crates/trusted-server-core/src/auction/endpoints.rs
#	crates/trusted-server-core/src/ec/finalize.rs
#	crates/trusted-server-core/src/ec/prebid_eids.rs
#	crates/trusted-server-core/src/ec/pull_sync.rs
#	crates/trusted-server-core/src/http_util.rs
@prk-Jr
Fix cargo fmt lint failure
a058822
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aram356 aram356 Awaiting requested review from aram356
@ChristianPavilonis ChristianPavilonis Awaiting requested review from ChristianPavilonis

Requested changes must be addressed to merge this pull request.

Assignees

@prk-Jr prk-Jr

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Thread the KV EID read through the request and fix the EC TTL decay bug

2 participants

@prk-Jr @ChristianPavilonis