Support Sourcepoint GPP consent for EC generation#642
Support Sourcepoint GPP consent for EC generation#642ChristianPavilonis wants to merge 28 commits intofeature/edge-cookies-finalfrom
Conversation
7009838 to
43df212
Compare
0e5c07c to
a8b5b1c
Compare
aram356
left a comment
There was a problem hiding this comment.
Summary
Sourcepoint GPP consent is a small, well-motivated slice — the GPP-US decoder, the allows_ec_creation gating branch, and their tests are solid. But as shipped, the feature is non-functional (the JS module is never served to browsers), the PR fails CI, and it bundles several large unrelated changes that should be separate PRs.
Blocking
🔧 wrench
-
Sourcepoint JS module is built but never served.
crates/trusted-server-core/src/integrations/registry.rs:790-792hardcodesJS_ALWAYS = &["creative"]. The design spec (docs/superpowers/specs/2026-04-15-sourcepoint-gpp-consent-design.md:38) says the integration follows the same "JS-only, no Rust registration" pattern ascreative, but that pattern requiresJS_ALWAYSto list the module.integrations/mod.rshas nosourcepoint::registerandJS_ALWAYShas no"sourcepoint"entry, soIntegrationRegistry::js_module_ids()will never include it —/static/tsjs=…will never concatenatetsjs-sourcepoint.jsinto a served bundle. End-to-end, no browser will ever executemirrorSourcepointConsent(). Fix: add"sourcepoint"toJS_ALWAYSand a test asserting it ships injs_module_ids_immediate(). -
Orphaned dead-code file
crates/trusted-server-core/src/ec/admin.rs. 380 lines implementingPOST /_ts/admin/partners/register, butec/mod.rscontains nopub mod admin;declaration (grep -rn "mod admin\|ec::admin::" crates/returns zero hits). The base branch (feature/edge-cookies-final) replaced the KV-backed partner registry with config-based partners in commit049a501e; that change droppedadmin.rsbut the rebase on this branch reinstated it unreferenced. Fix: deletecrates/trusted-server-core/src/ec/admin.rs. -
Clippy
-D dead_codefailures block CI. Three dead symbols introduced by the same rebase and carrying no callers:crates/trusted-server-core/src/platform/test_support.rs:174—recorded_request_headersmethodcrates/trusted-server-core/src/platform/test_support.rs:336—build_services_with_http_clientfunctioncrates/integration-tests/tests/common/runtime.rs:56—PartnerRegistrationFailedvariant (this is the actual failing CI log message)
Confirmed locally:
cargo clippy --workspace --all-targets --all-features -- -D warningsfails in the PR's worktree. Fix: delete them all — nothing references any of the three. -
Undisclosed scope: creative iframe sandbox lockdown.
crates/js/lib/src/core/render.ts:7-18removesallow-scripts,allow-same-origin, andallow-formsfrom the creative iframesandbox=attribute, andcrates/trusted-server-core/src/creative.rs:361-367strips<iframe>elements entirely during HTML sanitization. Together, any creative relying on JavaScript (viewability pixels, click tracking, rich media, VPAID) or on third-party ad-tag iframes will stop rendering. This is a major ad-tech behavioral change wholly unrelated to Sourcepoint GPP consent, and it is not mentioned in the PR body. It needs its own PR with a threat model, product sign-off, and a rollout plan. Fix: extract these two changes to a dedicated PR.
❓ question
- Why is Prebid User ID Module support shipping inside this PR? The title and body are "Support Sourcepoint GPP consent for EC generation," but the diff also contains ~132 lines of
build-all.mjsUser-ID-submodule generation, a new_user_ids.generated.ts, ~290 lines of new Prebid test coverage, a 212-line design spec (specs/2026-04-16-prebid-user-id-module-design.md), and a 599-line implementation plan. Two independent features in one review is hard to evaluate. Can Prebid User ID Module move to its own PR?
Non-blocking
📌 out of scope
- No re-mirror after mid-session CMP interaction.
mirrorSourcepointConsent()runs once when the module is parsed. If a user opens the Sourcepoint CMP and updates consent mid-session,__gpp/__gpp_sidwon't reflect the new state until the next page load — meaning the same session can continue to block EC creation even after consent is granted. Follow-up: subscribe to__gppapievents (or astoragelistener) and re-run on change.
|
Addressed the requested review feedback in 3bbdb1d. Summary:
Validation run locally:
I also replied to and resolved the inline threads above. |
…igration - Add ec/ module with EcContext lifecycle, generation, cookies, and consent - Compute cookie domain from publisher.domain, move EC cookie helpers - Fix auction consent gating, restore cookie_domain for non-EC cookies - Add integration proxy revocation, refactor EC parsing, clean up ec_hash - Remove fresh_id and ec_fresh per EC spec §12.1 - Migrate [edge_cookie] config to [ec] per spec §14
Implement Story 3 (#536): KV-backed identity graph with compare-and-swap concurrency, partner ID upserts, tombstone writes for consent withdrawal, and revive semantics. Includes schema types, metadata, 300s last-seen debounce, and comprehensive unit tests. Also incorporates earlier foundation work: EC module restructure, config migration from [edge_cookie] to [ec], cookie domain computation, consent gating fixes, and integration proxy revocation support.
Implement Story 4 (#537): partner KV store with API key hashing, POST /admin/partners/register with basic-auth protection, strict field validation (ID format, URL allowlists, domain normalization), and pull-sync config validation. Includes index-based API key lookup and comprehensive unit tests.
- Rename ssc_hash → ec_hash in batch sync wire format (§9.3) - Strip x-ts-* prefix headers in copy_custom_headers (§15) - Strip dynamic x-ts-<partner_id> headers in clear_ec_on_response (§5.2) - Add PartnerNotFound and PartnerAuthFailed error variants (§16) - Rename Ec error variant → EdgeCookie (§16) - Validate EC IDs at read time, discard malformed values (§4.2) - Add rotating hourly offset for pull sync partner dispatch (§10.3) - Add _pull_enabled secondary index for O(1+N) pull sync reads (§13.1)
Move admin route matching and basic-auth coverage to /_ts/admin for a hard cutover, and align tests and docs so operational guidance matches runtime behavior.
Prebid's liveIntentIdSystem.js uses a dynamic require() inside a build-flag-guarded branch that their gulp pipeline dead-codes via constant folding. esbuild leaves the require() in the output, causing ReferenceError: require is not defined at browser runtime. Remove from the bundle until we add an esbuild resolver plugin (or switch to Prebid's own build pipeline) — tracked as a follow-up in the design spec.
Introduces TSJS_PREBID_USER_IDS env var (mirroring TSJS_PREBID_ADAPTERS) to control which Prebid User ID submodules are bundled. The hardcoded imports in index.ts are replaced with a generated file written by build-all.mjs at build time, defaulting to the same 13-submodule set. - build-all.mjs: generatePrebidUserIds() validates names, denylists liveIntentIdSystem, and writes _user_ids.generated.ts. Existence check also probes dist/src/public/ to handle modules shipped as .ts in sources (sharedIdSystem). - index.ts: replaces 13 hardcoded submodule imports with import './_user_ids.generated' - _user_ids.generated.ts: committed default with all 13 submodules - Tests: updated mocks and regression guard; added 9 syncPrebidEidsCookie behavior tests - Docs: new "User ID Modules" section in prebid.md with TSJS_PREBID_USER_IDS usage; spec follow-up #1 marked complete
__gpp and __gpp_sid are read by the Rust server over HTTPS; they must be Secure. Also sets Max-Age=86400 (matching ts-eids) so stale consent state doesn't outlast the session, and replaces the legacy expires= deletion pattern with Max-Age=0.
3bbdb1d to
8a6df3a
Compare
Summary
_sp_user_consent_*from localStorage and mirrors GPP consent into__gpp/__gpp_sidcookiessale_opt_outfrom US GPP sections (IDs 7–23)allows_ec_creation()between existing TCF andus_privacychecksCloses #640
Test plan
cargo test --workspace— 992 tests including 8 new)npx vitest run— 288 tests including 6 new)🤖 Generated with Claude Code