Implement consent forwarding pipeline#380
Conversation
b4dfdde to
3e8e3c5
Compare
|
This has a minimal TCF Decoding implementation. |
|
Also maybe #390 should be merged first, there will be conflicts. |
|
@ChristianPavilonis Please resolve conflicts |
aram356
left a comment
There was a problem hiding this comment.
Summary
Comprehensive consent forwarding pipeline spanning signal extraction, TCF/GPP/USP decoding, jurisdiction detection, KV persistence, and OpenRTB integration. The architecture is clean and well-decomposed. Four blocking issues around edge-case correctness and input validation need attention before merge.
Blocking
🔧 wrench
- Expired TCF leaks through GPP-embedded fallback: when both standalone TC and GPP-embedded TCF are present and expired, only one is cleared —
effective_tcf()still returns the GPP copy (crates/common/src/consent/mod.rs:171) is_empty()misclassifies__gpp_sid-only requests:gpp_section_idsis not checked, so requests with only__gpp_sid=2,6are treated as empty — skipping logging and KV writes (crates/common/src/consent/types.rs:155)- KV Store consent persistence disabled on
/auctionpath:synthetic_id: Nonecauses bothtry_kv_fallbackandtry_kv_writeto early-return (crates/common/src/auction/endpoints.rs:57) - No input length validation on consent strings before decoding: unbounded base64 decode from cookie input could allocate large heap buffers from malicious input (
crates/common/src/consent/tcf.rs:57)
Non-blocking
🤔 thinking
- KV fingerprint ignores
gpp_section_ids: SID-only changes are invisible to the fingerprint, skipping KV writes (crates/common/src/consent/kv.rs:178) regs.gdpr = Some(0)false negative in ambiguous cases: a GDPR-jurisdiction user without a TCF cookie getsregs.gdpr=0, signaling "GDPR does not apply" (crates/common/src/integrations/prebid.rs:644)
♻️ refactor
apply_tcf_conflict_resolutionclones eagerly: bothTcfConsentstructs are cloned before determining if a conflict exists (crates/common/src/consent/mod.rs:312)now_deciseconds()usesas u64truncation fromu128: safe in practice but avoidable withu64-native arithmetic (crates/common/src/consent/mod.rs:377)
🌱 seedling
extract_and_log_consentis declared but never called: public function with zero callers — either document the intent or remove (crates/common/src/consent/mod.rs:203)- No end-to-end test for consent → OpenRTB pipeline: individual consent modules are well-tested, but there's no integration test that starts from a
Requestwith consent cookies and asserts the finalOpenRtbRequestJSON contains correctregs.gdpr,regs.gpp,user.consent, etc. This cross-module path (endpoints.rs→formats.rs→prebid.rs) is where mismatches are most likely. Consider adding one in a follow-up. gate_eids_by_consentis defined but not wired in:gate_eids_by_consent(mod.rs:430) is implemented and unit-tested but never called in the actual bid request path. The EIDs field is alwaysNonein prebid.rs. Document whether this is deferred to a future phase or wire it in.
👍 praise
- Clean pipeline architecture: the extract → decode → normalize → gate pipeline is well-decomposed. Each stage is independently testable, proxy mode is a clean escape hatch, and the KV fingerprint-based write deduplication is smart. The conflict resolution strategies (Restrictive/Newest/Permissive) and config-driven jurisdiction lists are particularly well thought out.
- Dual-placement for Prebid compatibility: populating both OpenRTB 2.6 top-level fields and
regs.ext.*/user.ext.consentfor Prebid compatibility is the right call. TheRegsExtmirroring pattern with corresponding tests shows attention to real-world integration needs.
CI Status
- Analyze (javascript-typescript): PASS
Review feedback addressed in 798e4a2All 9 review comments addressed in a single commit. Here's how each was resolved: Bug fixes
Design concerns
Cleanup
|
Address PR #380 review findings: - Cap TCF vendor range expansion to MAX_VENDOR_ID (10,000) to prevent DoS - Add GPP string length guard (8,192 bytes) before parsing - Replace unwrap_or_default() with expect() in now_deciseconds - Document gate_eids_by_consent as deferred until EID wiring - Drop plotters feature from criterion to reduce compile-time deps - Add explanatory comment for dead_code allow in build.rs
Wire consent signals into OpenRTB bid requests, add per-partner forwarding modes, and persist consent to KV Store for returning users. Phase 2 - OpenRTB integration: populate regs/user consent fields with dual-placement (top-level 2.6 + ext), add EID consent gating, AC string forwarding, and new Eid/Uid/ConsentedProvidersSettings structs. Phase 3 - Configuration + observability: add [consent] config section with jurisdiction detection, expiration checking, GPC-to-US-Privacy construction, and structured logging. Phase 4 - Partner integrations: cookie stripping via ConsentForwardingMode, Prebid/Lockr consent cookie filtering, APS consent fields, adserver mock consent summary. Phase 5 - KV Store persistence: consent/kv.rs with KvConsentEntry and ConsentKvMetadata types, SHA-256 fingerprint change detection, read fallback when cookies absent, write-on-change via Fastly KV Store API.
Fix expired TCF leaking through GPP fallback by clearing both sources. Add gpp_section_ids to is_empty() check and KV fingerprint hash. Generate synthetic ID before consent pipeline in auction endpoint so KV fallback and write operations work correctly. Add max-length guard on TC strings before base64 decoding. Use jurisdiction to inform regs.gdpr instead of falsely emitting 0. Defer cloning in TCF conflict resolution and remove dead code.
Check TCF/GPP consent signals before setting the Synthetic Session Cookie. EU/UK users require explicit opt-in (TCF Purpose 1), US users are allowed unless they opt out via US Privacy or GPC, and non-regulated jurisdictions are unaffected. When consent is absent but an existing SSC cookie is present, the cookie is expired and the corresponding KV store entry is deleted. Revocation targets the cookie value directly (not the x-synthetic-id header) to avoid mismatched deletions. Closes #464, closes #465, closes #466.
Address PR #380 review findings: - Cap TCF vendor range expansion to MAX_VENDOR_ID (10,000) to prevent DoS - Add GPP string length guard (8,192 bytes) before parsing - Replace unwrap_or_default() with expect() in now_deciseconds - Document gate_eids_by_consent as deferred until EID wiring - Drop plotters feature from criterion to reduce compile-time deps - Add explanatory comment for dead_code allow in build.rs
6834a2e to
ba8eb3c
Compare
Summary
[consent]section with jurisdiction detection, per-partner forwarding modes, expiration checking, and GPC-to-US-Privacy construction.Changes
Consent-gated SSC creation
crates/common/src/consent/mod.rsallows_ssc_creation()— consent decision function: GDPR requires TCF P1 opt-in, US checks opt-out, non-regulated allowscrates/common/src/cookies.rsexpire_synthetic_cookie()— expires SSC cookie withMax-Age=0for revocationcrates/common/src/consent/kv.rsdelete_consent_from_kv()— deletes KV store entry on consent revocationcrates/common/src/publisher.rshandle_publisher_request(): set SSC (consent given), revoke (consent withdrawn + cookie present), or skip (no consent + no cookie)OpenRTB integration
crates/common/src/openrtb.rsregs/userconsent fields with dual-placement (top-level 2.6 +extfor older exchanges); addEid,Uid,ConsentedProvidersSettingsstructsConfiguration & observability
crates/common/src/consent_config.rs[consent]config section:ConsentConfig,ConsentMode,ConsentForwardingMode,GdprConfig(31 countries),UsStatesConfig(20 states), conflict resolution, expiration checkingcrates/common/src/consent/jurisdiction.rsJurisdictionenum (Gdpr, UsState, NonRegulated, Unknown) +detect_jurisdiction()from geo + configcrates/common/src/consent/mod.rsbuild_consent_context(),ConsentPipelineInput, KV fallback/write, expiration checking, GPC-to-US-Privacy, EID gatingcrates/common/src/consent/types.rsTcfConsenthelper methods (has_purpose_consent,has_storage_consent, etc.)crates/common/src/settings.rsconsent: ConsentConfigfieldcrates/common/src/lib.rsconsent_configcrates/common/build.rsconsent_config.rsin build inputsPartner integrations
crates/common/src/cookies.rsstrip_cookies,forward_cookie_header,CONSENT_COOKIE_NAMES)crates/common/src/integrations/prebid.rsConsentForwardingModesupport, consent cookie stripping inOpenrtbOnlymodecrates/common/src/integrations/lockr.rsforward_cookie_headercrates/common/src/integrations/aps.rsApsGdprConsentstruct, consent fields inApsBidRequestcrates/common/src/integrations/adserver_mock.rsextKV Store persistence
crates/common/src/consent/kv.rsKvConsentEntryandConsentKvMetadatatypes, SHA-256 fingerprint change detection, read fallback when cookies absent, write-on-change via Fastly KV Store APIWiring & config
crates/common/src/auction/endpoints.rs/auctionendpointcrates/common/src/publisher.rssynthetic_idinto publisher handlerfastly.tomlconsent_storeKV store for local devtrusted-server.toml[consent]config section with all optionsKey design decisions
extfor backward compatibility with older exchanges.ConsentForwardingModecontrols whether consent travels via OpenRTB body only (OpenrtbOnlystrips cookies) or both cookies and body (CookiesAndBody).allows_ssc_creation()centralizes the consent decision. GDPR requires TCF Purpose 1 (store/access device) opt-in; US state privacy uses opt-out model (block only if user explicitly opted out). Non-regulated and unknown jurisdictions default to allowing SSC.x-synthetic-idheader) to ensure the correct KV entry is deleted when both are present.How to enable
[consent]section intrusted-server.tomlconsent_storeinfastly.toml(already added for local dev)mode = "proxy"ormode = "interpreter"depending on desired consent processing depthTest plan
cargo fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warningscargo test --workspace— 493 tests passingnpx vitest run— 150 JS tests passingnpm run format(js + docs) — cleancargo build --bin trusted-server-fastly --release --target wasm32-wasip1— WASM build passesChecklist
Closes #312
Closes #464
Closes #465
Closes #466