Repository navigation
Extract existing Sourcepoint consent synchronization - #1535
ChristianPavilonis wants to merge 19 commits into
Conversation
prk-Jr
left a comment
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
Server-side reconciliation looks sound: the Origin check plus the JSON content type block cross-site posts, only __gpp/__gpp_sid are replaced (identity cookie, us_privacy and GPC survive), the body bound is applied before parsing, and the endpoint never enters generation, EID ingestion or orphan recovery (those stay on publisher navigation). A null snapshot is not an explicit withdrawal, so a CMP dialog's not-ready phase cannot tombstone anything. One blocking issue on the browser side: consent sync has no enable switch separate from the integration, so deployments without an identity store get a guaranteed-503 POST on every page view.
Blocking
🔧 wrench
- Consent sync posts on every page view where the server can only answer 503 — see inline at
crates/trusted-server-js/lib/src/integrations/sourcepoint/index.ts:315
Non-blocking
🤔 thinking
- The 204 is optimistic; a skipped finalization acknowledges an unprocessed withdrawal — see inline at
crates/trusted-server-core/src/ec/consent_update.rs:191 - Fastly and Axum disagree for bot-gated clients; Fastly/Cloudflare/Spin lack route tests — see inline at
crates/trusted-server-adapter-fastly/src/app.rs:718
CI Status
- CLAUDE.md symlink guard: PASS
- cargo fmt: PASS
- cargo test: PASS
- cargo test (axum native): PASS
- cargo test (cross-adapter parity): PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip2): PASS
- vitest: PASS
- format-typescript: PASS
- format-docs: PASS
- docs-proposal scripts: PASS
- prepare integration artifacts: PASS
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- browser integration tests: PASS
| } | ||
|
|
||
| initialized = true; | ||
| consentSync = |
There was a problem hiding this comment.
🔧 Consent sync posts on every page view where the server can only answer 503
ConsentSync is constructed whenever the Sourcepoint module loads, so every page view POSTs the current snapshot to /_ts/consent. The endpoint intentionally answers 503 when there is no identity graph, and that is the permanent answer on several supported configurations:
- Cloudflare and Spin always call
handle_consent_update(&req, None). - Fastly without
ec.ec_store(maybe_identity_graphreturnsNone). - Axum without native EC persistence.
After a non-204, failed() clears lastSnapshot (intended, and covered by permits a later event to retry an unconfirmed update), so each subsequent focus/visibilitychange re-POSTs and fails again with a log.warn. A publisher already using this integration only for cookie mirroring gets one guaranteed-failing request per page view plus more per focus after upgrading: extra edge cost and 503 noise in monitoring. The guide says "Deploy the server endpoint and persistence support before enabling capture", but there is no separate thing to enable.
Suggested fix (spans Rust + JS, so not a one-click suggestion): extend the existing window.__tsjs_sourcepoint config script in crates/trusted-server-core/src/integrations/sourcepoint.rs with a flag (e.g. "syncConsent": true) emitted only when the adapter has an identity graph, and construct ConsentSync only when it is set; then point the guide's "before enabling capture" sentence at that flag. Cookie mirroring is unaffected. Treating 503 as terminal for the page would also stop the noise, but would drop the retry after a transient store outage, so the flag seems preferable.
| if req.extensions().get::<PreparedConsentUpdate>().is_none() { | ||
| return response(StatusCode::BAD_REQUEST); | ||
| } | ||
| let mut response = response(if graph.is_some() { |
There was a problem hiding this comment.
🤔 The 204 is optimistic; a skipped finalization acknowledges an unprocessed withdrawal
The handler returns 204 whenever a graph exists, and only ec_finalize_response can downgrade it via mark_reconciliation_failed. On Fastly the finalize step is skipped when settings cannot be loaded or PartnerRegistry::from_config fails (trusted-server-adapter-fastly/src/main.rs, the apply_edgezero_ec_finalize error arm only logs). In that case an opt-out POST receives 204 with no tombstone written.
This is unlikely since settings are validated at load, and on the normal path the response extension does survive to the Fastly finalize hook. Inverting the default would make the endpoint fail closed: start at 503 and have finalization promote it to 204 once withdrawal (or the no-op grant path) has actually been evaluated.
| .await | ||
| } | ||
| } | ||
| NamedRouteHandler::ConsentUpdate => Ok( |
There was a problem hiding this comment.
🤔 Fastly and Axum disagree for bot-gated clients
Fastly passes finalize_kv_graph, which is None for a client that fails looks_like_browser() unless the request is a withdrawal, so a grant from such a client gets 503. Axum passes the ungated ec.graph, so the same request gets 204. On Fastly, a privacy-hardened real browser misclassified by the bot gate will re-POST on every focus. Minor, but worth picking one rule and documenting it.
Related coverage note: /_ts/consent has route-level tests only in trusted-server-adapter-axum/tests/routes.rs; the Fastly wiring (prepare before build_ec_request_state, 503 marking through the entry-point finalize) and the Cloudflare/Spin reservations are exercised only indirectly through the core unit tests.
Retain the public Sourcepoint history above rebuilt native EC. Observe the existing GPP API and carry each real CMP decision in its own bounded first-party snapshot without minting identity or restoring partner IDs. Prepare consent with the shared body reader before EC extraction, preserve runtime origin evidence and typed read-failure status, and deny unclaimed methods and foreign origins locally. Keep current direct clients and response ownership rather than the former ingress/body-preload bridge.
There was a problem hiding this comment.
Superseded by a follow-up review with inline comments: #1535 (review)
dhruv8sh
left a comment
dhruv8sh
left a comment
There was a problem hiding this comment.
Summary
Adds POST /_ts/consent so the Sourcepoint module can push each GPP snapshot to the server, which reconciles it through the existing EC finalize path (tombstoning on withdrawal, never minting). The request preparation is tight (same-origin, bounded body, strict schema, GPP-only cookie replacement) and the failure semantics (503 on missing store or failed tombstone) are well tested. Only non-blocking items below.
3 of the inline comments below carry a one-click GitHub
suggestion— use Commit suggestion (or Add suggestion to batch for several at once) to apply them as commits on the PR branch. The remaining comment describes the fix in prose because it spans many lines across the test module.
Non-blocking
♻️ refactor
- Axum acknowledges with an unfiltered graph, diverging from Fastly — see inline at
crates/trusted-server-adapter-axum/src/app.rs:369
⛏ nitpick
- Explain why every method is claimed — see inline at
crates/trusted-server-adapter-axum/src/app.rs:524 - Public consts lack
///docs — see inline atcrates/trusted-server-core/src/ec/consent_update.rs:20 - Test
expect/assert messages — see inline atcrates/trusted-server-core/src/ec/consent_update.rs:288
CI Status
- worker-deadlines (spin): PASS
- worker-deadlines (cloudflare): PASS
- vitest: PASS
- ts dev lint domains (changed lines): PASS
- trace runtime and browser acceptance: PASS
- release scripts: PASS
- prepare integration artifacts: PASS
- integration tests (Fastly EC lifecycle): PASS
- integration tests: PASS
- HTML EOF runtime (Fastly): PASS
- format-typescript: PASS
- format-docs: PASS
- Fastly response diff script: PASS
- docs-proposal scripts: PASS
- CLAUDE.md symlink guard: PASS
- CI Node pin guard: PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- cargo test (cross-adapter parity): PASS
- cargo test (axum native): PASS
- cargo test: PASS
- cargo fmt: PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- browser integration tests: PASS
| EcRoute::ConsentUpdate => Ok( | ||
| trusted_server_core::ec::consent_update::handle_consent_update(&req, ec.graph.as_ref()), | ||
| ), |
There was a problem hiding this comment.
♻️ refactor — ConsentUpdate passes the unfiltered ec.graph, but NativeEcRequest::finalize (line 280-283) only writes through the graph when self.browser || ec_consent_withdrawn(..). A non-browser grant therefore gets 204 on Axum while Fastly, which passes finalize_kv_graph (same filter), returns 503. Apply the finalize filter so the acknowledgement matches what finalization can actually persist and the adapters agree.
| EcRoute::ConsentUpdate => Ok( | |
| trusted_server_core::ec::consent_update::handle_consent_update(&req, ec.graph.as_ref()), | |
| ), | |
| EcRoute::ConsentUpdate => Ok( | |
| trusted_server_core::ec::consent_update::handle_consent_update( | |
| &req, | |
| ec.graph | |
| .as_ref() | |
| .filter(|_| ec.browser || ec_consent_withdrawn(ec.context.consent())), | |
| ), | |
| ), |
| [ | ||
| NamedRoute { | ||
| path: trusted_server_core::ec::consent_update::CONSENT_UPDATE_PATH, | ||
| primary_methods: &[ |
There was a problem hiding this comment.
⛏ nitpick — Listing every method (including CONNECT/TRACE) for a POST-only endpoint looks surprising without context; the reason only appears in the Axum route test. A short comment here (and on the matching Fastly NAMED_ROUTES entry) would save the next reader a trip.
| primary_methods: &[ | |
| // Claim every method so non-POST requests reach | |
| // `prepare_consent_update` and get a local 405 with `Allow: POST` | |
| // instead of falling through to the publisher proxy. | |
| primary_methods: &[ |
| pub const CONSENT_UPDATE_PATH: &str = "/_ts/consent"; | ||
| // Two browser-cookie-sized signals plus their JSON representation. This is a | ||
| // consent-only transport, not another general auction or identity payload. | ||
| pub const MAX_CONSENT_UPDATE_BODY_BYTES: usize = 16 * 1024; |
There was a problem hiding this comment.
⛏ nitpick — Both constants are pub but carry no /// doc comment (AGENTS.md: each public item must have one); the body-size rationale is a plain // comment. Promote it to docs:
| pub const CONSENT_UPDATE_PATH: &str = "/_ts/consent"; | |
| // Two browser-cookie-sized signals plus their JSON representation. This is a | |
| // consent-only transport, not another general auction or identity payload. | |
| pub const MAX_CONSENT_UPDATE_BODY_BYTES: usize = 16 * 1024; | |
| /// Same-origin path the Sourcepoint module posts consent snapshots to. | |
| pub const CONSENT_UPDATE_PATH: &str = "/_ts/consent"; | |
| /// Upper bound, in bytes, on a consent snapshot request body. | |
| /// | |
| /// Two browser-cookie-sized signals plus their JSON representation. This is a | |
| /// consent-only transport, not another general auction or identity payload. | |
| pub const MAX_CONSENT_UPDATE_BODY_BYTES: usize = 16 * 1024; |
| ); | ||
| req.headers_mut() | ||
| .insert("sec-gpc", HeaderValue::from_static("1")); | ||
| let req = prepare_consent_update(&noop_services(), req) |
There was a problem hiding this comment.
⛏ nitpick — The new tests use expect() messages that do not follow the "should ..." convention ("fixture cookie", "valid snapshot", "cookies", "jar", "must reject origin", "read", "tombstone", ...) and many assert!/assert_eq! calls have no message (e.g. lines 294-298, 320-322, 338, 351-352, 372, 405-407, 533, 545, 557, 589). The same applies to the bare assert!(!response.headers().contains_key("set-cookie")) calls in crates/trusted-server-adapter-axum/tests/routes.rs. Apply manually — the edits span most of the test module and fall outside a single contiguous range, so they can't be expressed as one suggestion.
Per the user, every feature is built once and works on every adapter in platform order, with platform gaps sent to EdgeZero (DEC-16, SH-5). T21 moves the identity routes, EC lifecycle, providers and write-back into one router in core that each adapter mounts; A2, A10, T8, T9, T17 and T18 shrink to turning it on per adapter and passing the conformance suite, and T7, T14 and A11 fold into A2 and A10. EZI-2a adds EdgeZero's local store for Axum development and EZI-4 binds Axum's geo provider from the manifest. Restack the Axum RC units on #1506: #1531 carries A1 in place of #1528, #1533 and #1534 close as copies of #1525 and #1526, and the image and Terraform units sit directly on #1532 so A6 and A7 stop waiting on A2. Record #1566, refresh the provider stack's fixes at their current heads, and flag #1535 as unplanned.
Per the user, celld stays in the MVP (DEC-17), ID5 and LiveRamp are the mirrored identity providers the MVP's checks use (DEC-10), which makes the LiveRamp Custom ID (DEC-9) a launch need, and #1535's Sourcepoint consent sync is required. ID-9's EC route now also takes later consent changes, so a withdrawal clears the identity cookies and tombstones the record at once; #1535's separate per-adapter /_ts/consent route folds into T19 and its GPP capture into T20.
Per the user, the RC line is forward-ported to main (DEC-6). Rechecking the proposed stack against DEC-3 and DEC-16: #1536 and #1537 are mostly Axum-only stores, store selection and wiring that EZI-2b, T6 and T21 replace, so A2 becomes a new pull request that takes their tests, and both close with #1535 once their code moves. The image and Terraform units restack on #1543, which publishes the image and shares .dockerignore with #1539. #1531 goes on #1506 for A1 and #1532 on #1531 for #1529.
Summary
Draft baseline review unit. Not a current-main merge candidate, deployment approval or completed current-head CI claim.
Baseline and sequence
Head:
e2672968a52e7efbad2509204cadab4b2113e3c1. Base:baseline/native-ec-20261110.Historical main anchor:
2dd968aeebeb2cc0d5110115f64e0ad58e2e1d13. Original demo: TS2423a2ed plus separately preserved dirty inputs. Matching EdgeZero: draft431, exact revision83d7a6296d285e135c4ea70021b19e6c8dca7e9f.The user selected compatible-baseline cleanup. Current-main integration with EdgeZero462 and the authenticated-forwarder incompatibility are deferred. Preserve current-main PR1525/1526 ready and1527/1528 draft, with their existing bases. No excluded1499 or independent1506 implementation, relay/privacy experiment, new transport fallback, group/codec/phase design or operational deployment is included.
Changes
Closes
Refs #1530. This unit completes one part of the baseline extraction and does not close the overall tracker on its own.
Spec
Not a new feature. This is extraction of existing demo behavior and bounded build/caller corrections. Original native and EC storage design/review records remain in the baseline where applicable; they are not presented as approval of later combined source. No new forwarding, consent, timer or provider policy is designed here.
Test plan
Sourcepoint68 JS cases pass, full JS lint/format pass, Core3016 and native suites/strict lint pass. This is the existing e7c7f57 delta, not new consent policy or parked Prebid relay fixes.
The b67ffc6 baseline checkpoint has passing documented local gates: Fastly/default/reuse/native/Cloudflare/Spin suites, all adapter strict lint lanes, 17 cross-adapter parity cases, 1252 JS cases, CLI suites, formatting, clock/digest/proposal checks and changed-lines domain lint. Passing checks at121ebcc36 are reused only for unchanged relevant inputs; the final follow-up covers the CLI test-import and documentation-only corrections. These results are not a substitute for this PR's current-head hosted checks.
Checklist
Operational Terraform remains in trusted-server-deployer. Local checks and prototype tests do not qualify cloud access, fleet durability or a live publisher.
Hosted docs-build correction
The first hosted format-docs job caught an additional site-build gate that the original local formatter checkpoint had not run: a relative link from the native EC guide to a Terraform example outside the site. Native EC correction8b11dbad3 replaces it with the plain repository path; no dead-link check is disabled. That docs-only correction is carried through the existing upper PR identities. Native EC and final integration8985b111 both pass docs ESLint and VitePress build; the final docs formatter also passes. Runtime, dependency, image-build-context and test inputs are unchanged from b67ffc6, so their prior exact-input proofs are reused. Hosted current-head checks still control readiness.
Startup negative-probe correction
A hosted native EC case failed the no-listener probe while its child logged the expected startup validation failure. The old log has no socket endpoints, so the failure's exact subtype is not claimed confirmed. A separate controlled Linux reproduction demonstrates a successful TCP self-connection with no listening server. Native correction8490c691d distinguishes equal local/peer endpoints from a real listener; actual socket controls prove the self-connection is excluded and a real listener is still detected. Nonzero exit, expected diagnostic, secret-redaction and the original startup deadline remain required. This changes only the acceptance fixture, not production admission, transport or trust.
The earliest native unit and final9356c557 default/all-feature acceptance checks and strict lint pass, including both controls and the original failure case. Ignores remain explicit; earlier separately executed celld proof is not relabeled as a new ignored-case run. Correction is merged upward through existing PR identities without force-pushing or retargeting. Current-head hosted checks must pass; the old failure is retained rather than rerun-only waived. Runtime/API/dependency inputs remain unchanged. A fresh exact9356 producer build and real runtime smoke now pass: image
sha256:65a735d38e9dbc0573d0f6068a8b261bd202c43aea05085c2b2d6bccc91ebdbd, linux/amd64 UID/GID10001, including startup rejection and in-flight SIGTERM drain. Prior b67 image receipts remain separate; no image was pushed. All local parent runners are complete.Browser CI host compatibility
Preserved older browser failures show Playwright1.58.2 rejecting ubuntu-latest's new Ubuntu26.04 image before browser installation; later missing-executable failures are consequences, not passing browser tests. Nativeb1216763a pins only the browser job to ubuntu-24.04, leaving dependency versions, tests and assertions unchanged. Direct Actionlint1.7.12 passes at the owning native and final integration points; the first shim-only attempt could not run because no global version was configured, and is not counted as lint proof. No global tooling was changed. The only9356 ->ee7 delta is this workflow host/comment; runtime, Cargo, docs, tests and Docker-context inputs are unchanged. Actual browser execution on the supported hosted runner remains pending and must pass before readiness. No waiver or browser skip.