Repository navigation
Conversation
cf4983b to
1bc681f
Compare
|
@dhruv8sh, the same point as on #1487. Directionally, eight identity vendors as files in one shared crate is not compatible with V2 (the 55 pull requests tracked in #777, merged in order), where each vendor is a module in a folder of its own at The V2 spec is identifier-modules, with the folder layout in integrations-crate, both in #1342. The associated pull request is #1382 (a module hands core its identifiers, starting with the Prebid EID cookie). Could you evaluate the V2 spec and #1382 before progressing this pull request? |
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
Adds eight vendor identity providers and the coordinator hooks they need. Two opt-out handling defects and two UID2 refresh-lifecycle defects need correction before merging.
One inline comment carries a one-click GitHub
suggestion. Use Commit suggestion or Add suggestion to batch to apply it. The other fixes need manual changes across files or outside the diff.
Blocking
🔧 wrench
- UID2 SDK opt-outs leave cached tokens active — see inline at
crates/trusted-server-identity-providers/src/uid2.rs:347–349(suggestion). - LiveRamp's 204 opt-out never reaches its parser — see inline at
crates/trusted-server-identity-providers/src/liveramp.rs:265–267. - Overdue UID2 refresh credentials are permanently discarded — see the body-level finding below.
- A stale cookie rolls back a refreshed UID2 token — see inline at
crates/trusted-server-core/src/ec/coordinator/mod.rs:602–610.
Cross-cutting / body-level findings
🔧 wrench — Overdue UID2 refresh credentials are permanently discarded
References: crates/trusted-server-identity-providers/src/uid2.rs:173–176 and crates/trusted-server-core/src/ec/coordinator/validate.rs:151–155.
Two focused reproductions expose the same underlying problem:
- Importing a still-live identity after
refresh_fromdrops its refresh credentials, so subsequent navigations cannot refresh it. - If a refresh times out while the cookie repeats the stored token, validation drops the cookie's already-due refresh state. The cookie result survives the failed lookup and replaces the stored record without refresh credentials; future attempts stop.
Due refresh state must remain usable rather than becoming invalid merely because its scheduled time passed. The proposed validation change is:
Some(from) if usable => Ok(stored.with_refresh(KvRefresh {
state: refresh.state.into_owned(),
from,
})),Apply manually — the actual guard is outside a diff hunk; its documentation and existing test expectations also need updating. Retain the existing state-size checks and use the configured lookup rate limit to control retries. If the future-only policy is retained for lookup replies, distinguish cookie ingress instead of discarding its already-due credentials.
Verification
- Existing provider unit tests: 91 PASS locally.
- Five focused scratch reproductions confirmed the four findings on the reviewed head. The SDK opt-out regression passes with the suggestion applied in isolation.
- Exact suggestion bytes pass
cargo fmt --all -- --check, all eight clippy aliases,cargo check-fastly,cargo check-axum,cargo check-cloudflare, all four adapter test aliases,cargo test-fastly-reuse, and the cross-adapter parity suite. - The initial local Fastly attempt used Viceroy 0.17 and failed on an unsupported host import; rerunning with the repository-pinned Viceroy 0.21.0 passed.
- Byte-exact post-verification patch comparison: PASS. Scratch changes were discarded; the reviewer worktree is clean at the reviewed head.
Coverage: provider/coordinator implementations were read fully; large supporting files were reviewed through diffs and relevant context rather than exhaustively end-to-end. No live vendor service testing was performed. The prose fix sketches were not applied.
CI Status
- integration tests: PASS
- browser integration tests: PASS
- trace runtime and browser acceptance: PASS
- integration tests (Fastly EC lifecycle): PASS
- cargo test (axum native): PASS
- format-docs: PASS
- CI Node pin guard: PASS
- HTML EOF runtime (Fastly): PASS
- ts dev lint domains (changed lines): PASS
- cargo fmt: PASS
- prepare integration artifacts: PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- CLAUDE.md symlink guard: PASS
- worker-deadlines (spin): PASS
- docs-proposal scripts: PASS
- cargo test (cross-adapter parity): PASS
- format-typescript: PASS
- cargo test: PASS
- worker-deadlines (cloudflare): PASS
- vitest: PASS
No required checks were reported for this branch.
aram356
left a comment
There was a problem hiding this comment.
Summary
This adds eight vendor identity providers and the coordinator hooks they need. Four things block it. The coordinator's cookie path rewrites unchanged IDs while another source is due, which also loses refresh state. Criteo's ID differs from what Prebid sends. LiveRamp's server lookup cannot parse the v2 reply. And ID5's gpp_sid type needs an answer. This round does not repeat the four findings of the earlier review at this head; where a finding here extends one of them, the comment says so.
4 of the inline comments below carry a one-click GitHub
suggestion. Use Commit suggestion, or Add suggestion to batch for several at once. The other comments describe the fix in prose, because it touches several files or lines outside the diff.
Blocking
🔧 wrench
- Cookie IDs that repeat the held UIDs are rewritten while another source is due (a KV write on every navigation, and refresh state lost after a failed refresh): see inline at
crates/trusted-server-core/src/ec/coordinator/mod.rs:599-612 - The Criteo bid ID is stored percent-encoded, but Prebid sends it decoded: see inline at
crates/trusted-server-identity-providers/src/criteo.rs:92-102 - The LiveRamp Custom ID lookup cannot parse a v2 envelope reply (also
atypeandgpp_sid): see inline atcrates/trusted-server-identity-providers/src/liveramp.rs:272-273
❓ question
- Does
/gs/v2takegpp_sidas an array?: see inline atcrates/trusted-server-identity-providers/src/id5.rs:182-183
Non-blocking
♻️ refactor
- The Lockr capability reads nothing with its defaults: see inline at
crates/trusted-server-identity-providers/src/common.rs:284-287 - Say which client settings put the token in a cookie (UID2 and ID5; suggestion): see inline at
trusted-server.example.toml:243-247 - Send IPv6 addresses as
ipv6(ID5): see inline atcrates/trusted-server-identity-providers/src/id5.rs:360-364 - Match Prebid's ConnectID validity rules: see inline at
crates/trusted-server-identity-providers/src/connectid.rs:108-118 - Prebid sends the resolved Trade Desk ID as atype 1 (suggestion): see inline at
crates/trusted-server-identity-providers/src/liveintent.rs:355-372 - Compare more than the UID strings: see inline at
crates/trusted-server-core/src/ec/coordinator/mod.rs:832-854 - Providers now read the clock: see inline at
crates/trusted-server-identity-providers/src/common.rs:337-339 - Plans and docs that contradict the code: see the body-level finding below
🤔 thinking
- Cookie and consent-derived opt-outs now get the 30-day lockout: see inline at
crates/trusted-server-core/src/ec/coordinator/mod.rs:560-565 - A placeholder ID reads as "no ID", not as an opt-out: see inline at
crates/trusted-server-identity-providers/src/common.rs:153-159 - Structure of this PR: see the body-level finding below
⛏ nitpick
exclude_sourceskeeps the pre-exclusionkeptcount (suggestion): see inline atcrates/trusted-server-core/src/ec/coordinator/mod.rs:770-772- Where GDPR applies, UID2 is skipped for region, not consent (suggestion): see inline at
crates/trusted-server-identity-providers/src/uid2.rs:34-35
🌱 seedling
- A generated SharedID changes every 30 days: see inline at
crates/trusted-server-identity-providers/src/sharedid.rs:131-142 - Smaller gaps: see the body-level finding below
Cross-cutting / body-level findings
♻️ Plans and docs that contradict the code
The plans say "Status: Implemented", but several statements no longer match the code:
2026-11-02-identity-provider-uid2.md, "Stored ID": the token "is refreshed only when the cookies no longer yield a usable token". Since25f35720b, it is also refreshed while the cookie repeats it.2026-11-02-identity-provider-uid2.md, risk note: "a server refresh spends the SDK cookie's refresh token ... the browser SDK's own refresh of that identity may then fail". A refresh token is not spent. The operator'sUIDOperatorService.refreshIdentityhas no replay check, and the UID2 token docs say "Refreshing a UID2 token does not invalidate/expire the original or previous UID2 token". The server refresh and the SDK refresh both succeed and fork into two token chains. Combined with the stale-cookie rollback from the earlier review, that can repeat the refresh about every other navigation until the cookie'sidentity_expires, bounded only byrate_limit_per_hour(inferred).2026-11-02-identity-provider-liveramp.md: the goal and architecture still say "no server call" and "build_requestalways fails", and "Stored ID" documents200 {"envelope"}(see the LiveRamp comment).2026-11-02-identity-provider-id5.md: an opted-outuniversal_uidof0"is reported asLookupOutcome::NoMatch". The code reportsOptedOut.2026-11-02-identity-provider-connectid.md: "The provider reads no clock".is_expiredreads it.2026-11-02-identity-provider-liveintent.md: "TheExpiresheader becomesProviderExpiry::At". The code producesAfter(Expires - Date).2026-11-02-identity-provider-criteo.md,criteo.rs:4-6and the example config: "still percent-encoded" (see the Criteo comment).docs/guide/configuration.md:836:allowed_domainsis marked required, but cookie-only kinds without anendpointdo not need it, and the field has#[serde(default)].
🤔 Structure of this PR
jwrosewell's question on this PR about the V2 layout (one module per vendor under identity/<vendor>, each with its own maintainer; #1342 and #1382) is still unanswered. Independent of that answer, this PR combines framework changes with eight vendors in 7.2k lines. The description offers to move the framework hooks into #1487. Doing that, and landing the vendors as separate PRs, would let each vendor's fidelity be reviewed and verified against its own service on its own.
🌱 Smaller gaps
validate.rs:171clamps absolute expiries to 30 days but leavesProviderExpiry::Afterunbounded in core. Every provider here bounds it itself, so this is defense in depth.- The adapter's joining of multiple
Cookieheaders intoRequestCookies(crates/trusted-server-adapter-fastly/src/app.rs:433-439) has no test, and the adapter tests passRequestCookies::default(). id5.rsdefines the same"0"twice, asOPTED_OUT_UID(line 71) andOPT_OUT_UID(line 415).
Verification
- Every changed file was read at
25f35720b. Vendor behaviour was checked against Prebid.jsmastermodules, LiveRamp's ATS Envelope API docs, ID5's Prebid Server Java module,uid2-web-integrationsanduid2-operator. No live vendor calls were made. - The coordinator findings were reproduced with probe tests that run
dispatch,collectandplan_write. The fix proposed atmod.rs:599-612makes both blocking probes pass, and all existingec::tests and the 91 provider tests still pass (native run). The Criteo and ID5ipv6prose fixes were compiled and tested in a scratch tree; the other prose sketches were not applied. - Each suggestion was applied on its own at this head, and its patch was byte-identical before and after the checks:
liveintent.rs(TDID atype):cargo fmt --check,clippy-fastly,check-fastly,check-axum,check-cloudflare,test-fastly.coordinator/mod.rs(keptcount):cargo fmt --check,clippy-fastly,clippy-axum,clippy-cloudflare,clippy-cloudflare-wasm,clippy-spin-native,clippy-spin-wasm,clippy-cli, the threecheck-*aliases,test-fastly,test-fastly-reuse,test-axum,test-cloudflare,test-spin, and the parity suite.uid2.rs(module doc):cargo fmt --check,clippy-fastly.trusted-server.example.toml: every added line is a comment.- All four together:
cargo fmt --check,clippy-fastly, native core and provider tests.
CI Status
- CI Node pin guard: PASS
- CLAUDE.md symlink guard: PASS
- HTML EOF runtime (Fastly): PASS
- browser integration tests: PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- cargo fmt: PASS
- cargo test: PASS
- cargo test (axum native): PASS
- cargo test (cross-adapter parity): PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- docs-proposal scripts: PASS
- format-docs: PASS
- format-typescript: PASS
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- prepare integration artifacts: PASS
- trace runtime and browser acceptance: PASS
- ts dev lint domains (changed lines): PASS
- vitest: PASS
- worker-deadlines (cloudflare): PASS
- worker-deadlines (spin): PASS
No required checks are configured for this base branch.
| reader.lockr = names | ||
| .into_iter() | ||
| .filter(|name| !reader.own.contains(name)) | ||
| .collect(); |
There was a problem hiding this comment.
♻️ refactor: The Lockr capability reads nothing with its defaults
with_lockr drops Lockr names that are already in own, and every Lockr default equals that kind's own default: id5id for ID5, _lr_env for LiveRamp (own default ["_lr_env", "idl_env"]) and __uid2_advertising_token for UID2. So [settings.lockr] enabled = true with the defaults reads no extra cookie. Neither do the three Lockr tables in trusted-server.example.toml, which repeat the own names. An operator who turns Lockr on sees no change.
With custom names, [settings.lockr].cookie_names behaves exactly like appending those names to cookie_names: same parser, same order, read before any lookup. The only difference is a debug log line. That is a trait, a settings type, reads_lockr, gating tests, a plan and docs, with no behaviour beyond cookie_names.
Suggest removing the capability and documenting "add the Lockr key to cookie_names" until there is Lockr-specific behaviour, for example provenance through inserter, or the revoke-consent opt-out the Lockr plan lists as not ported. If it stays, the example tables should use a key that differs from the own cookies, and the docs should say the default Lockr keys are already read as own cookies.
There was a problem hiding this comment.
The capability stays, and f68083c changes it:
- The example tables now use Lockr keys that differ from the vendors' own cookies. The docs say the default Lockr keys are already read as own cookies, so enabling the capability with the defaults reads nothing extra.
- Lockr-specific behaviour: Lockr cookies are not read where GDPR applies. Prebid's
lockrAIMIdSystemstops whengdprAppliesis true, so a Lockr cookie on such a request is a leftover, not a current delivery. Own cookies are still read there.
inserter provenance and revoke-consent are not ported. Lockr's storage holds neither: per the Prebid module, it stores the raw advertising_token under the configured key_name, and its own keys are localStorage only. Both exist only in Lockr API responses, which the framework does not capture. The plan records this.
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
This round covers areas the two earlier reviews at this head did not reach: the auction read path, untrusted response handling in the providers, and the CLI's provider validation. It found two new blocking problems. UID2 IDs that the GDPR exclusion removes can still reach bidders. A large LiveIntent segments list drops LiveIntent's own ID. The earlier findings are not repeated here.
Blocking
🔧 wrench
- UIDs the GDPR exclusion removes are still sent to bidders: see inline at
crates/trusted-server-core/src/ec/coordinator/mod.rs:361 - A large LiveIntent
segmentslist dropsnonId: see inline atcrates/trusted-server-identity-providers/src/liveintent.rs:337-346
Non-blocking
🤔 thinking
- ID5 never sends back the previous
signature: see inline atcrates/trusted-server-identity-providers/src/id5.rs:21
⛏ nitpick
- ID5 percent-decodes the fetch reply before the opt-out check: see inline at
crates/trusted-server-identity-providers/src/id5.rs:390 RequestCookiesDebugprints the whole Cookie header: see inline atcrates/trusted-server-core/src/ec/provider.rs:168
Verification
- The findings were reproduced with probe tests in a scratch tree at
25f35720b: an auction-read probe for the GDPR finding, plus probes for the LiveIntent ext size and the ID5 reply decoding. The ID5 signature point is not checked against ID5's docs or a live call. cargo test -p trusted-server-core --lib ec::(native): 523 passed.- Checked and found sound:
CheckedAppConfig(no provider readssecretsat build, so validating before secrets resolve is safe); checking the rate limit only once a request is built; cookie and lookup merging; an unknown jurisdiction failing closed;endpoint: None(every request is still checked againstallowed_domains); UID2 refresh decryption and its panic paths; response size caps; and identifiers going only to the configured endpoint. - No live vendor calls were made. The fixes are prose sketches and were not applied.
CI Status
All 23 checks PASS at 25f35720b, including cargo test, cargo fmt, cross-adapter parity, axum/cloudflare/spin, ts CLI (macOS and Ubuntu), integration and browser integration tests, Fastly EC lifecycle, vitest and the format jobs. No required checks are configured for this base branch.
25f3572 to
9889b96
Compare
|
Re: review 5814450927 (ChristianPavilonis) Fixed in 5c0bca7. Validation now keeps refresh state whose Re: review 5821978901 (aram356) Plans and docs: fixed in 7b60dd6, with the UID2 risk note rewritten in 8f575ad. Token chains: a server refresh and an SDK refresh do fork, as you describe. Since 7667acf an older cookie no longer rolls the refreshed token back, and since 5c0bca7 a failed refresh keeps the state. Once the SDK refreshes, its newer cookie token replaces the stored one, so the row follows the SDK chain again instead of alternating on every navigation. What remains is one server refresh per Structure: No. We are keeping this as a stack instead: #1487 holds the framework and this PR the vendors. The coordinator hooks that only the vendors use (cookie reads, refresh freshness, Lockr) stay here, next to the code that needs them. Smaller gaps:
|
a0e4190 to
1d4cd4b
Compare
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Add an optional IdentityProvider::read_request_cookies step that runs for providers with due sources before any lookup. Cookie IDs pass the same consent gates, validation and write plan as lookup results and replace the lookup. The provider endpoint becomes optional so cookie-only kinds need none; lookup kinds reject a missing endpoint. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Prebid's Lockr AIM module does not run where GDPR applies, so a Lockr cookie on such a request is a leftover; cookie readers now ignore Lockr cookies there. The example tables use Lockr keys that differ from the vendors' own cookies, and the docs say the default Lockr keys are already read as own cookies. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
…pping file Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
LiveIntent now applies the shared usable-ID check, so a blank or 0 ID reads as no match. A LiveRamp envelope entry without a type no longer fails the whole reply. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Prebid's LiveIntent module sets the resolved Trade Desk ID's ext.provider to the page domain, falling back to liveintent.com, and copies the reply's m next to segments in the nonId EID ext. Carry the publisher domain to parse_response through the request context and build both ext values the same way. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
The coordinator drops sources a provider excludes where GDPR applies only when it writes the row, so a UID2 UID stored outside GDPR still reached bidders until the next committed write. Mark those sources in the partner registry for every configured provider and skip them in live_eids for a GDPR visitor. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Prebid's ID5 module sends the cached reply's signature as s so ID5 can return the same ID. Keep a non-empty reply signature as the UIDs' private refresh state, due a tenth of the TTL before they expire, and send it as s in the next fetch. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
The GDPR source exclusion applied only to stored rows, so a UID2 EID the browser sent in the /auction body or the ts-eids cookie still reached bidders for a GDPR visitor. drop_vendor_denied_client_eids now also drops client EIDs whose source a provider excludes where GDPR applies, which covers /auction and the server-side auction. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
… sensitive information' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…phic value' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
The Copilot Autofix change generated the key with aes_gcm::aead::OsRng, which this aes-gcm build does not export, so the crate stopped compiling. A fixed key in test-only code is intended. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
aa66842 to
4fd9bd8
Compare
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
Adds eight identity providers and their cookie, refresh and consent handling. Four correctness/privacy defects require correction: enabled providers cannot pass CLI validation, identify bypasses GDPR exclusions, an unchanged cookie reverses a server opt-out, and ID5 ignores partner-specific cookie payloads.
Blocking
- P1 — Valid enabled providers cannot pass CLI validation — see inline at
crates/trusted-server-cli/src/checked_app_config.rs:30. - P1 — Identify bypasses the GDPR source exclusion — see inline at
crates/trusted-server-core/src/ec/registry.rs:79. - P1 — An unchanged cookie reverses a server-reported opt-out — see inline at
crates/trusted-server-core/src/ec/coordinator/mod.rs:594. - P2 — ID5 reads the wrong partner’s cookie payload — see inline at
crates/trusted-server-identity-providers/src/id5.rs:334.
Corrections are prose proposals requiring changes beyond a single contiguous diff range; no one-click suggestions are included.
Verification
At reviewed head 4fd9bd8d970a590c455cc27d4bfb12e637efd4b7, four focused native scratch probes reproduced the four defects. Existing native EC tests (585), provider tests (104), the updated publisher auction-cookie test, and cargo fmt --all -- --check pass. Used Rust 1.98.1 and Node 24.20.0. Scratch changes were discarded. No live vendor calls or full adapter-gate rerun was performed.
The complete production source is identical to the previously investigated snapshot; the only file delta marks a publisher test cookie Secure and is inherited from the updated stacked base. Prior source coverage was reconciled against that complete tree comparison, and the new test was inspected and executed.
CI status
No failed checks observed at submission preflight. Checks still pending/running: cargo test, Analyze (rust), integration tests, integration tests (Fastly EC lifecycle), cargo test (axum native), cargo check (cloudflare native + wasm32-unknown-unknown), trace runtime and browser acceptance, browser integration tests, cargo test (cross-adapter parity). Passing hosted checks do not close the four reproduced defects.
|
|
||
| impl Validate for CheckedAppConfig { | ||
| fn validate(&self) -> Result<(), ValidationErrors> { | ||
| let mut errors = self.0.validate().err().unwrap_or_default(); |
There was a problem hiding this comment.
🔧 P1 — Valid enabled providers cannot pass CLI validation
The wrapper retains core validation's temporary rejection of every enabled provider. TrustedServerAppConfig::validate reaches validate_settings_for_deploy, which still calls reject_providers_without_a_kind regardless of registered kind. A valid enabled SharedID configuration builds successfully but fails overall validation with “this release registers no provider kinds.” This prevents operators enabling these providers through ts config validate, push and diff.
Replace the temporary blanket gate with provider-aware validation while preserving structural checks and adapter restrictions. Add a complete validation-success regression for an enabled registered provider. The existing wrapper tests inspect only the provider-specific field and ignore this core error. Apply manually: the guard and regression coverage require changes beyond this hunk.
| /// Whether this source's UIDs are withheld because a provider excludes it | ||
| /// and the visitor's jurisdiction is GDPR. | ||
| #[must_use] | ||
| pub fn excluded_for(&self, consent: &ConsentContext) -> bool { |
There was a problem hiding this comment.
🔧 P1 — Identify bypasses the GDPR source exclusion
This exclusion is applied by live_eids, but ec/identify.rs:104–113 calls source_views directly and filters only vendor consent. A live UID2 row created outside GDPR is therefore still returned as uid and eid on an authenticated GDPR identify request with purpose/vendor consent, including when the provider is disabled. A handler-level reproduction with a marked excluded source confirms both fields are returned.
Apply !partner.excluded_for(ec_context.consent()) alongside the vendor-consent check in identify, and add endpoint coverage. This is an incomplete sibling of the earlier auction-egress correction: that correction does not cover this independent reader. Apply manually: the affected consumer and its regression test are outside this diff.
| // An ID in the vendor's cookies means the visitor | ||
| // opted back in, so a remembered opt-out is | ||
| // forgotten. | ||
| if remembered_opt_out.is_some() { |
There was a problem hiding this comment.
🔧 P1 — An unchanged cookie reverses a server-reported opt-out
Any usable cookie is treated as proof of opting back in. A UID2 server refresh, however, never rewrites the browser cookie. A two-navigation reproduction first refreshes a due identity, receives a server opt-out, clears the source and persists opt-out memory. The next navigation sends the same unchanged cookie: this branch produces Found with OptOutMemory::Forget, allowing the cleared token to return without a new vendor lookup.
Keep server-reported suppression unless there is evidence of a renewed identity; the mere presence of the pre-opt-out cookie cannot establish that. Preserve legitimate opt-back-in with an explicit freshness/renewal criterion and add the two-request lifecycle regression. Apply manually: the correction requires lifecycle-state and regression changes beyond this range.
| } | ||
|
|
||
| /// Reads one ID5 cookie value: a fetch reply JSON or a bare ID. | ||
| fn cookie_eids<'cookie>(&self, value: &'cookie str) -> Option<Vec<ProviderEid<'cookie>>> { |
There was a problem hiding this comment.
🔧 P2 — ID5 reads the wrong partner’s cookie payload
Prebid's ID5 cookie stores per-partner responses in pbjs[partner_id], while the top-level response is overwritten by the most recent partner. This parser ignores the map and always consumes the top-level ID and signature. A cookie with distinct entries for partners 1234 and 5678 yields partner 5678's ID even when this provider is configured for 1234.
Select the configured partner's payload before parsing its ID, signature, expiry and opt-out state, while preserving legacy-cookie support. Add a distinct-partner regression. Prebid's getPartnerResponse and createResponse establish the format: https://github.com/prebid/Prebid.js/blob/master/modules/id5IdSystem.js#L576-L600. Apply manually: this needs parsing and opt-out handling changes plus regression coverage.
The encrypted_reply test helper built its AES-256-GCM key from a constant, which CodeQL flags as a hard-coded cryptographic value. Generate the key from the system RNG instead and return it alongside the body so the decrypting tests pass it as the reply context. The getrandom feature is enabled on aes-gcm for dev builds only. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Reverse the provider stack to #1487, #1488, #1489 and refresh the moved heads. Split the EdgeZero release so Axum serving does not wait for the record contract (R1b), add the ClientInfo contract (EZI-3) and JA4 (IABTechLab/edgezero#283) that ID-7 needs on Axum, and move the Axum side of providers, abuse protection and SharedID write-back into A10 so Fastly no longer waits on the Axum chain. Drop blockers that code at the PR heads shows are already met (#1189, #1556, #1141 before A1), move four-adapter and decision-gated issues out of the MVP, and align priorities and sizes. The MVP is now waves 0 to 9.
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
Adds eight vendor identity providers and the framework hooks for cookie reads, refreshes, opt-outs and GDPR source exclusion. Seven correctness/privacy issues need changes; the UID2 retry-lifetime observation is non-blocking.
One inline comment carries a one-click GitHub
suggestion. The other fixes are described manually because they require coordinated edits, tests, or changes outside the diff.
Blocking
🔧 wrench
- Normalize LiveIntent sources before checking GDPR exclusion — see inline at
crates/trusted-server-identity-providers/src/liveintent.rs:254. - Select the configured ID5 partner's cookie response — see inline at
crates/trusted-server-identity-providers/src/id5.rs:334-345. - Preserve per-cookie uniqueness in LiveRamp Custom IDs — see inline at
crates/trusted-server-identity-providers/src/liveramp.rs:237. - The four cross-cutting blockers below cover opt-out egress, deployment validation, identify egress and delayed-result revocation.
Non-blocking
🤔 thinking
- Clarify UID2 refresh lifetime and failed-retry policy — see inline at
crates/trusted-server-identity-providers/src/uid2.rs:311-319. This is not a request to reject refreshes solely because the advertising token expired.
Cross-cutting / body-level findings
🔧 Vendor cookie opt-outs must suppress the current request's identity egress
References: crates/trusted-server-core/src/ec/coordinator/mod.rs:572-579, crates/trusted-server-adapter-fastly/src/app.rs:1068-1111, and crates/trusted-server-core/src/auction/endpoints.rs:350-405.
The coordinator records a cookie opt-out for the later navigation write, but the publisher auction is dispatched before those results are collected and persisted. Named routes such as POST /auction do not run the cookie readers at all. A visitor with a stored SharedID and _pubcid_optout=1 can therefore still have that ID sent to bidders; repeated auction calls need not ever reach the navigation that would clear it.
A focused endpoint reproduction captured EIDs reaching the auction provider despite the opt-out cookie. Read vendor cookie opt-outs before identity egress and suppress the affected sources from both stored and client-supplied EIDs on that request, independently of the later KV write.
Proposed API shape:
// Read vendor cookie opt-outs before any identity egress.
ec_context.set_provider_denied_sources(denied_sources);
// Filter both browser-supplied and stored EIDs before dispatch.
eids.retain(|eid| !ec_context.provider_denied_sources().contains(&eid.source));Apply manually — the methods above illustrate the required request-local policy; implementing it touches provider-cookie handling, request context and egress paths together. Cover publisher auctions, page-bids, /auction and identify without introducing provider HTTP lookups on read-only routes.
🔧 Registered providers cannot pass deployment validation
References: crates/trusted-server-cli/src/checked_app_config.rs:30-31 and crates/trusted-server-core/src/config.rs:348,352-369.
CheckedAppConfig::validate first keeps the errors from self.0.validate(). That still calls core's reject_providers_without_a_kind, which rejects every enabled provider without checking its kind. The additional validate_providers call does not remove that error. Thus the supported validate/push/diff pipeline rejects even a buildable registered provider with “this release registers no provider kinds; set enabled = false”.
A focused reproduction successfully built a registered SharedID provider, then failed push-safe app-config validation with exactly that error.
Replace the blanket no-kinds rejection with the registered-kind validation now supplied by this PR. The generic core deploy-validation ending should be:
validate_identity_providers(&with_placeholder_partner_tokens(&settings.ec))?;
Ok(())Retain the provider crate's kind/capability validation in the CLI wrapper and unsupported-adapter checks. Apply manually — remove/update the obsolete core guard and its tests, and add an end-to-end passing validation test for a valid enabled provider rather than checking only the provider-specific error field.
🔧 Apply GDPR source exclusion to identify responses too
Reference: crates/trusted-server-core/src/ec/identify.rs:104-110; the policy is introduced at crates/trusted-server-core/src/ec/registry.rs:60-80.
Auction egress checks excluded_for, but identify calls source_views directly after checking only the owning vendor's TCF consent. An authenticated partner can therefore retrieve a stored GDPR-excluded ID while the visitor is under GDPR, even with the ordinary canonical source spelling. This is separate from the trailing-dot registry-marking issue.
A focused reproduction marked the source excluded and called identify with GDPR consent granted; the response still contained both uid and eid.
let vendor_denied = partner.owner_vendor_consent_denied(ec_context.consent())
|| partner.excluded_for(ec_context.consent());Apply manually — identify.rs is outside the PR's diff. Add coverage showing that an excluded source is omitted under GDPR but remains available elsewhere.
🔧 Do not let delayed results undo a newer cookie opt-out
References: crates/trusted-server-core/src/ec/write_plan.rs:133-159 and crates/trusted-server-core/src/ec/after_response.rs:74-103.
Cookie opt-outs now clear the sources without persisting revocation information. An older in-flight lookup can finish after that clear; the after-response writer re-reads the newer row and uses its generation, then writes the older result into it. The CAS protects only changes after this last read, not the opt-out that occurred while the lookup was in flight.
A deterministic reproduction committed a cookie opt-out, confirmed the source was absent, then committed the older pending result. The source was restored as p-before-optout.
Keep revocation/version information that survives removal of the source, capture the relevant provider revision at dispatch, and reject results superseded by a newer revocation. The required check has this shape:
if entry.provider_revision(&lookup.provider)
!= lookup.dispatched_provider_revision
{
continue;
}Apply manually — this is an API sketch, not existing methods. It needs coordinated dispatch/persistence changes and a regression for the older-result/newer-opt-out ordering; merely re-reading the current generation is insufficient.
Verification and scope
- All seven blocking findings were checked with focused scratch reproductions. The UID2 retry observation was also reproduced, but is not treated as a blocker.
- The exact one-click replacement was verified in isolation: formatting; a passing native trailing-dot/GDPR regression; six target-matched adapter clippy aliases; Fastly, Axum and Cloudflare compile checks; all four adapter test aliases; and cross-adapter parity. Byte-exact patch drift checks passed. Fastly tests were rerun successfully with the repository-pinned Viceroy 0.21.0 after the default local 0.17.0 runner could not launch the test binary.
- Review coverage focused on the new provider implementations and affected cookie/lookup, persistence, egress and CLI paths. Unchanged portions of the largest modified files were not exhaustively reviewed.
- Live-service UID2 refresh and LiveRamp Custom ID lookup were not exercised. Scratch changes were discarded; no code changes were committed or pushed.
CI Status
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- browser integration tests: PASS
- trace runtime and browser acceptance: 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 test (axum native): PASS
- HTML EOF runtime (Fastly): PASS
- worker-deadlines (spin): PASS
- cargo test: PASS (required)
- format-typescript: PASS (required)
- cargo fmt: PASS (required)
- worker-deadlines (cloudflare): PASS
- release scripts: PASS
- ts dev lint domains (changed lines): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- vitest: PASS
- docs-proposal scripts: PASS
- Analyze (rust): PASS
- Analyze (javascript-typescript): PASS
- format-docs: PASS (required)
- Analyze (python): PASS
- cargo test (cross-adapter parity): PASS
- CodeQL: PASS
- prepare integration artifacts: PASS
- Analyze (actions): PASS
- CI Node pin guard: PASS
- CLAUDE.md symlink guard: PASS
- Analyze (javascript-typescript): PASS
| /// Whether a `liveintent` provider excludes `source` where GDPR applies: the | ||
| /// UID2 tokens it resolves, matching the `uid2` provider. | ||
| pub(crate) fn excludes_where_gdpr_applies(source: &str) -> bool { | ||
| source.eq_ignore_ascii_case(UID2_SOURCE) |
There was a problem hiding this comment.
🔧 Normalize LiveIntent sources before checking GDPR exclusion
sources = ["UIDAPI.COM."] is valid and normalized when the provider is built. However, partner_registry calls this predicate with the raw configured string, so its trailing dot prevents the UID2 source from being marked excluded. Stored and browser-supplied UID2 EIDs can then pass the auction egress filter under GDPR.
A focused regression built that provider successfully and observed one UID2 EID from live_eids under GDPR instead of zero. Normalize the trailing dot consistently with SourceDomain:
| source.eq_ignore_ascii_case(UID2_SOURCE) | |
| source | |
| .trim_end_matches('.') | |
| .eq_ignore_ascii_case(UID2_SOURCE) |
| fn cookie_eids<'cookie>(&self, value: &'cookie str) -> Option<Vec<ProviderEid<'cookie>>> { | ||
| let eids = match percent_decoded(value) { | ||
| Cow::Borrowed(value) if value.trim_start().starts_with('{') => { | ||
| self.parse_reply(value.as_bytes()).ok()? | ||
| } | ||
| Cow::Owned(value) if value.trim_start().starts_with('{') => self | ||
| .parse_reply(value.as_bytes()) | ||
| .ok()? | ||
| .into_iter() | ||
| .map(owned) | ||
| .collect(), | ||
| value => { |
There was a problem hiding this comment.
🔧 Select the configured ID5 partner's cookie response
Prebid's shared id5id cookie can hold partner-specific responses under pbjs[partner_id]; its top-level response is copied from the last partner written. Parsing only the top level can therefore store another partner's UID/signature, and the separate top-level opt-out check can apply that partner's opt-out to this instance.
A focused regression configured partner 1234, supplied both 1234 and 5678 responses with 5678 at the top level, and received 5678's ID. Prebid's createResponse and getPartnerResponse show the shared-cookie shape and partner selection.
Proposed selection logic (illustrative, after decoding the JSON cookie):
let partner_id = self.partner_id.to_string();
let reply = match cookie.get("pbjs") {
Some(partners) => partners.get(partner_id.as_str())?,
None => &cookie,
};Use the same selected response for cookie IDs, refresh signature and opt-out detection. If a multiplexed cookie has no response for this partner, fall back to lookup rather than another partner's top-level result.
Apply manually — this requires coordinated edits to cookie parsing and opt-out detection plus regression coverage, rather than one small contiguous replacement.
| .ok_or_else(|| { | ||
| Report::new(ProviderError::Request).attach("liveramp needs a routable client IP") | ||
| })?; | ||
| let stable_id = lookup.ec_id.split('.').next().unwrap_or(lookup.ec_id); |
There was a problem hiding this comment.
🔧 Preserve per-cookie uniqueness in LiveRamp Custom IDs
The 64-hex EC prefix is an HMAC of the client IP; it is a network identifier, not the visitor's unique cookie identity. generate_ec_id explicitly adds the random suffix to distinguish users behind the same NAT/proxy (and IPv6 /64). Removing it gives those different visitors the same person-identity Custom ID.
With Custom ID lookup enabled, a mapping for one visitor can therefore resolve the same envelope for another visitor on that network. A focused request-building regression with two distinct EC suffixes produced identical iv values.
Use the full cookie identity, or derive the Custom ID from the full value:
let stable_id = lookup.ec_id;Apply manually — update the existing suffix-stripping test and the Custom ID mapping/documentation contract together. The publisher's mapping file must distinguish the full EC identities too.
| let state = lookup | ||
| .current | ||
| .iter() | ||
| .flat_map(|(_, eid)| eid.uids()) | ||
| .find_map(|uid| uid.refresh()) | ||
| .and_then(|refresh| serde_json::from_str::<RefreshState>(&refresh.state).ok()) | ||
| .ok_or_else(|| { | ||
| Report::new(ProviderError::Request).attach("no stored uid2 refresh token") | ||
| })?; |
There was a problem hiding this comment.
🤔 Clarify UID2 refresh lifetime and failed-retry policy — non-blocking
The plan says a failing refresh stops when the advertising token expires, but the expired source becomes due and OwnedEntries still exposes its refresh state here. A focused dispatch/collect/write-plan reproduction made refresh calls on two navigations after expiry; both failed with 400 and planned no write, leaving the same state available for the next navigation when no hourly limit is configured.
That does not by itself make the refresh incorrect: UID2 allows refreshing with an unexpired refresh token, whose lifetime differs from the advertising token. I do not recommend rejecting a refresh solely because its advertising token expired.
Please clarify the intended retry/lifetime contract in the docs, or consider a follow-up policy based on refresh-token expiry and failed responses. This observation is not a merge blocker and carries no code suggestion.
|
CREATOR: From the server-side identity plan (#1559, |
Stacked on #1487.
Summary
extPer vendor
/gs/v2fetch API with an RFC 3339tsandgpp_sidas an array; carrieslinkTypeand keepsext; auniversal_uidof0is an opt-outdid-distributor ID;segmentsonly when non-empty_lr_env, or the bare envelope Prebid stores inidl_env); with anendpoint,placement_idandcustom_id_namespace, a visitor without a cookie is looked up server side by Custom IDconnectIdfirst-party cookie; no server callendpoint, the token is refreshed server side before it expires_sharedid/_pubcidcookie as an atype 1 EID; honours_pubcid_optout; withgenerate = true(default off), generates a UUID stored only in the rowcto_bididcookie, stored verbatim as the atype 1 EID Prebid sendsTDIDfrom thepbjs-unifiedidcookie, withrtiPartnerext andadserver.orginserter/matcher, mm 4Consent and cookie handling
Known limitations
connectIdcookie is still read until it expires, and SharedID generation should be enabled only where the opt-out lives in the_pubcid_optoutcookie.The plans list the remaining unported reference behaviours.
Changes
crates/trusted-server-identity-providers/src/{id5,liveintent,liveramp,connectid,uid2,sharedid,criteo,unifiedid}.rscrates/trusted-server-identity-providers/src/common.rscrates/trusted-server-core/src/ec/provider.rs,ec/coordinator/crates/trusted-server-core/src/settings.rs,adapter-fastly/src/app.rstrusted-server.example.tomldocs/guide/configuration.md,docs/guide/api-reference.mddocs/superpowers/plans/2026-11-02-identity-provider-*.mdCloses
Part of #1246 and #1245. No issue is closed yet.
Spec
The spec is in #1487. This PR adds only plans, no new spec.
Test plan
cargo test-fastly && cargo test-fastly-reuse && cargo test-axum && cargo test-cloudflare && cargo test-spincargo fmt --all -- --checktemplate_build_digesttest and its clippy runscripts/lint-wasm-clock.sh./scripts/test-cli.sh, the node pin guard, docs-proposal scripts, andts dev lint domainsagainst the base branch/gs/v2fetch returned an ID that was stored in KV and attached to the auctionChecklist
unwrap()in production codelogmacrosCloses #1567