Repository navigation
Conversation
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Eid gains inserter, matcher, mm and ext, each skipped when absent, so serialization without them is byte-identical. Eid::new replaces the struct literals that never carry these fields. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
ec::client_eids parses each array element on its own, so one malformed entry or field costs only itself. It validates sources, UIDs, atype, EID-level provenance fields and ext sizes, drops duplicates before the 64-entry cap, and logs field names and sources but never values. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
AdRequest captures eids as raw JSON so a lone surrogate in one entry no longer rejects the whole /auction body. The ts-eids structured branch uses the same parser, so a mistyped field or bad entry costs only itself and the body's caps apply to the cookie. Admin diagnostics still report raw sources and UIDs. The end-to-end fixture tests land with the merge rewrite, which they also depend on. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Browser entries reach providers unchanged, in wire order, with repeated sources kept apart. KV adds entries only for registered sources the browser did not send, so its stale UID and operator atype no longer override the browser's. Includes the end-to-end TSJS fixture tests deferred from the shared-parser change, since they depend on this merge keeping EID-level fields. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Finalization chooses its browser input by sync source. /auction reads the client set the auction used, so a cookie from an earlier page is not ingested next to a scoped body. Other routes keep reading ts-eids, and the sharedId cookie is still applied last. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
collectBidderEids copies ortb2.user.ext.eids and user.eids as plain data within a 64 KiB UTF-8 budget. fitEidCookie builds the ts-eids value without EID-level ext, trimmed to 3,072 bytes and encoded as UTF-8 base64. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
…buildRequests buildRequests reads bidderRequest.ortb2 instead of getUserIdsAsEids(), so IDs restricted to other bidders or redacted for trustedServer are no longer forwarded, and ortb2 EIDs such as Lockr's ID5 now arrive with their EID-level fields. The cookie copies each request's set before it is sent and is cleared when Prebid's TCF rules block trustedServer. bidsBackHandler no longer touches it. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
The served shim measured 42,144 characters after it gained the bidder-request EID collector, ts-eids cookie fitting and the tcf2Enforcement listener, so the bound moves from 41,000 to 42,500. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
installUserIdModules forced auctionDelay 0, syncDelay 3000, syncsPerBidder 5 and iframe syncs for every bidder on window.load. Prebid accumulates userSync settings, so every later auction ran without waiting for pending IDs. TS now writes only managed userIds and, on a late seed, autoRefresh. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
A JSDOM suite evaluates the generated bundle and the served shim and captures /auction bodies. It covers a bidder-restricted ID, same-source entries with their own fields, the first auction after window.load, and TCF with and without the full trustedServer exception. Every arm except the timing control fails against main. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
D1, D3 and both TCF arms now assert that document.readyState is still loading after install and after the auction, so each proves it ran before window.load as spec 7.5 requires. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Only tests read resolvePrebidUserIdModulesFromEids and the registry's eidSources. The data could not be made correct, since several modules share sources. The CLI and bundle builder read only module and config names. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Guides now describe forwarding the trustedServer bidder request's EIDs, the browser-first merge, per-entry validation, KV ingestion inputs by route, TS no longer setting userSync, and the full trustedServer TCF rule with its warning. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Guard the cookie clear so a blocked document.cookie costs only the cookie, and cover the nothing-fits clear path and both failure paths with adapter tests. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Admin diagnostics lost the parse error for cookies that decode to a non-array JSON value. Routing is unchanged. Record the final review fixes in the red log. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
aram356
changed the title
…BTechLab/trusted-server into spec/1246-prebid-eid-fidelity
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Requesting changes on two correctness concerns in the EID fidelity work:
- Gate browser-side EID cookie persistence on storage consent, clearing the cookie on denial while preserving forwarding.
- Validate legacy-cookie UIDs before applying browser precedence so invalid IDs cannot reach providers or suppress valid KV identities.
Please add regression coverage for both cases. The inline comments also include two non-blocking cleanups: import grouping and borrowing temporary raw EID elements instead of allocating copies.
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
prk-Jr
left a comment
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
The EID fidelity changes preserve bidder-scoped entries and provenance through the browser and server paths, but two blocking correctness concerns remain at commit 5ab7d4994fca2e78beab1cba1efa426c31588076.
Blocking
- 🔧 [P1] Gate browser EID cookie persistence on storage consent. The shim writes the one-day
ts-eidscookie whenever bidder EIDs exist, without checking storage permission. Transmission permission does not establish storage permission, and the TCF listener checks only blocked bidders/EIDs. Gate persistence on usable storage consent, clear on denial, and add paired regression coverage while preserving permitted forwarding. See the existing inline finding. - 🔧 [P2] Validate legacy cookie UIDs before applying browser precedence. The legacy decoder accepts whitespace-only and oversized IDs, while the revised merge trusts nonempty UID vectors. Such entries can reach providers and suppress a valid KV identity for the same source. Validate legacy UIDs before constructing the browser-source set and cover whitespace and 513-byte IDs with a valid KV fallback. See the existing inline finding.
These findings reference the existing inline threads to avoid duplicate comments. No replacement suggestions are included because the fixes also require regression coverage.
CI Status
All 23 reported checks pass. Local tests were not rerun; this review uses the reported remote CI results.
- integration tests: PASS
- browser integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- CodeQL: PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- format-docs: PASS (required)
- cargo test (cross-adapter parity): PASS
- Analyze (actions): PASS
- vitest: PASS
- cargo fmt: PASS (required)
- cargo test (axum native): PASS
- cargo test: PASS (required)
- format-typescript: PASS (required)
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- Analyze (javascript-typescript): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- docs-proposal scripts: PASS
- CLAUDE.md symlink guard: PASS
- prepare integration artifacts: PASS
- Analyze (rust): PASS
- Analyze (javascript-typescript): PASS
- Analyze (python): PASS
Prebid lets trustedServer see ortb2 EIDs on any purpose from 2 to 10, so forwarding does not establish storage consent. Where GDPR applies, buildRequests now writes the cookie only with Purpose 1 in the bidder request's gdprConsent and otherwise clears it, still forwarding the EIDs in the /auction body. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
The legacy flattened cookie branch accepted whitespace-only and oversized IDs. Because the merge lets any browser entry displace KV's entry for its source, such an ID reached providers and hid a valid KV identity. Apply the shared UID rule and drop blank sources. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
The input RawValue already owns the JSON text, and the element slices are used only inside each function. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
The server uses or stores browser EIDs only with Purpose 1 and Purpose 4 (allows_eid_persistence, gate_eids_by_consent), so a cookie written on Purpose 1 alone held IDs no server path reads. The write now needs both where GDPR applies, and is still unrestricted when gdprApplies is not true, so CMPs that answer outside the EU with an empty purpose record keep the cookie. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
dhruv8sh
changed the title
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
The per-entry parser, the browser-first merge and the /auction ingestion change hold up. The new guards are tightly tested: 19 of 23 Rust mutants and 33 of 44 JS mutants are killed, and the remaining Rust survivors are equivalent or diagnostics-only. Two problems remain in the browser cookie path: the new storage-consent check still lets ts-eids be written in two GDPR cases (a CMP timeout and a GPP-only CMP), and the guide's eidsRequireP4Consent instruction, followed literally, removes the publisher's Purpose 4 EID policy for every bidder. Both were reproduced against real Prebid 10.26.0. The rest is docs and spec drift since the last three commits, test gaps, and smaller cleanups.
6 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 comments describe the fix in prose because the change touches multiple files or lines outside the diff and can't be auto-applied.
Blocking
🔧 wrench
- The cookie consent check fails open on Prebid's null consent (CMP timeout) and on GPP-only EU CMPs: see inline at
crates/trusted-server-js/lib/src/integrations/prebid/index.ts:2852 - The
eidsRequireP4Consentinstruction weakens the publisher's EID policy for every bidder: see inline atdocs/guide/integrations/prebid.md:1074 - The guides overstate when the cookie consent check applies: see inline at
docs/guide/edge-cookies.md:164anddocs/guide/integrations/prebid.md:882 - The spec, review log and red log predate the last three design commits: see inline at
docs/superpowers/specs/2026-10-18-prebid-eid-fidelity-design.md:3 /auctioningestion is misdescribed: see inline atdocs/guide/ec-setup-guide.md:196- The legacy cookie's blank-source rule is untested, and the legacy branch skips the body's caps: see inline at
crates/trusted-server-core/src/auction/endpoints.rs:1913 - Untested guards in
fitEidCookieandhasEidPersistenceConsent: see inline atcrates/trusted-server-js/lib/src/integrations/prebid/eids.ts:188 - Quadratic cookie trim loop on the auction path: see inline at
crates/trusted-server-js/lib/src/integrations/prebid/eids.ts:198 - The
autoRefreshclaim is too broad: see inline atdocs/guide/integrations/prebid.md:343 - Unneeded
onEventcast: see inline atcrates/trusted-server-js/lib/src/integrations/prebid/index.ts:2333 - Shim size bound has about 1,200 characters of slack: see inline at
crates/trusted-server-js/lib/test/prebid-artifact-integration.test.mjs:316 openrtb_atypescope is overstated: see inline atdocs/guide/configuration.md:656- "Only a malformed body outside
eidsrejects the request" is overstated: see inline atdocs/guide/api-reference.md:237 AuctionEidis an alias forunknown: see inline atcrates/trusted-server-js/lib/src/core/auction.ts:30- Stale test title: see inline at
crates/trusted-server-js/lib/test/integrations/prebid/user_id_modules.test.ts:10 isRecordis duplicated: see inline atcrates/trusted-server-js/lib/src/integrations/prebid/eids.ts:36
Cross-cutting / body-level findings
-
🔧 No
CHANGELOG.mdentry for operator- and publisher-visible changes. Spec 8.3 (:1426-1427) says "The release notes say so", and[Unreleased]already carries Breaking entries for comparable changes. This PR changes behavior that operators and publishers will notice:- Trusted Server no longer overwrites the publisher's
userSync. Iframe user syncs are no longer enabled for every bidder, and auctions now wait up to Prebid's 500 msauctionDelaywhile IDs are pending. - Browser EIDs go out first and KV-only sources after. KV's value and
atypeare no longer sent for a source the browser sends, and a source can repeat. ts-eidsis cleared for GDPR traffic without atrustedServerexception, or without Purposes 1 and 4.- Sites with GDPR traffic need the full
trustedServerTCF exception rule. - Bundles with a baked-in shim must be regenerated with
ts prebid client. - Each provider needs the staging check for repeated sources and the four new fields.
Please add a Breaking entry.
- Trusted Server no longer overwrites the publisher's
-
🔧 The credential-based IdentityLink validation still checks the API the adapter stopped reading.
docs/guide/integrations/prebid.md:948(outside the diff hunks) says "Confirmpbjs.getUserIdsAsEids()reports sourceliveramp.com". The adapter no longer reads that API (index.ts:2930; the guide says so at:667-668). Under TCF without atrustedServerexception, that step passes while no LiveRamp EID reaches Trusted Server. Point it at thetrustedServerbidRequestedevent'sortb2.user.ext.eidsinpbjs.getEvents(), without recording the value, aslockr.md:237-238already does.
CI Status
- Analyze (actions): PASS
- Analyze (javascript-typescript): PASS
- Analyze (python): PASS
- Analyze (rust): 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 (required)
- cargo test: PASS (required)
- 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
- CLAUDE.md symlink guard: PASS
- CodeQL: PASS
- docs-proposal scripts: PASS
- format-docs: PASS (required)
- format-typescript: PASS (required)
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- prepare integration artifacts: PASS
- vitest: PASS
| function hasEidPersistenceConsent(bidderRequest: object): boolean { | ||
| const { gdprConsent } = bidderRequest as { gdprConsent?: unknown }; | ||
| if (!isRecord(gdprConsent) || gdprConsent.gdprApplies !== true) { | ||
| return true; | ||
| } | ||
| const { vendorData } = gdprConsent; | ||
| if (!isRecord(vendorData) || !isRecord(vendorData.purpose)) { | ||
| return false; | ||
| } | ||
| const { consents } = vendorData.purpose; | ||
| return isRecord(consents) && consents[1] === true && consents[4] === true; | ||
| } |
There was a problem hiding this comment.
🔧 wrench: The Purpose 1 and 4 check still lets ts-eids be written in two GDPR cases
hasEidPersistenceConsent treats a gdprConsent without gdprApplies: true, and a missing gdprConsent, as "GDPR does not apply". Two real Prebid 10.26 states reach that branch for EU traffic. I reproduced both in a JSDOM harness with the generated bundle and the served shim, reusing prebid-eid-fidelity.test.mjs:
- The CMP stub is present but never answers. On
consentManagement.gdpr.timeout, Prebid stores its null consent (libraries/consentManagement/cmUtils.ts:79-87,getNullConsent→toConsentData(null)inmodules/consentManagementTcf.ts:154-160,190):{ gdprApplies: <defaultGdprScope, false unless set>, apiVersion: 2 }, with novendorData. With__tcfapi = () => {}andtimeout: 200, thetrustedServerbidder request carried{"gdprApplies":false,"apiVersion":2}, nothing was blocked, andts-eidswas overwritten. The same page withdefaultGdprScope: trueblockstrustedServerand the listener clears the cookie, so the gap is only the shim reading Prebid's default as an answer. - A GPP-only CMP reports the EU TCF section.
consentManagementGppfills onlygppConsent;gdprConsentis attached only fromgdprDataHandler(src/adapterManager.ts:700-709). WithapplicableSections: [2]and everytcfeuv2purpose denied,gdprConsentwas absent and the cookie was written.
The server never uses these EIDs (no EC without Purpose 1 in a GDPR jurisdiction, and effective_tcf reads the GPP EU section). What remains is a one-day, script-readable device write without consent: the same class as the earlier Purpose 1 thread, and it makes "written only with Purpose 1 and 4 consent" untrue. A CMP that is absent altogether (for example one loaded only for EU visitors) leaves gdprConsent unset (cmUtils.ts:97-100), so failing closed on these two shapes costs no non-EU traffic.
Proposed fix (apply manually: it changes this function and test/integrations/prebid/index.test.ts, so it can't be a single suggestion):
function hasEidPersistenceConsent(bidderRequest: object): boolean {
const { gdprConsent, gppConsent } = bidderRequest as {
gdprConsent?: unknown;
gppConsent?: unknown;
};
if (!isRecord(gdprConsent)) {
// A GPP-only CMP never fills gdprConsent, but its EU TCF section (2)
// still governs storage, so fail closed when it applies.
return !(
isRecord(gppConsent) &&
Array.isArray(gppConsent.applicableSections) &&
gppConsent.applicableSections.includes(2)
);
}
const { vendorData } = gdprConsent;
// Prebid's null consent after a CMP timeout has no vendorData, and its
// gdprApplies is only `defaultGdprScope`, so it says nothing about the user.
if (!isRecord(vendorData)) {
return false;
}
if (gdprConsent.gdprApplies !== true) {
return true;
}
if (!isRecord(vendorData.purpose)) {
return false;
}
const { consents } = vendorData.purpose;
return isRecord(consents) && consents[1] === true && consents[4] === true;
}Test changes in the storage consent block of index.test.ts:
- Add
['the CMP timed out (Prebid null consent)', { gdprApplies: false, apiVersion: 2 }]to the "forwards EIDs but clears ts-eids when %s" rows. - Change the "GDPR does not apply" fixture to
{ gdprApplies: false, vendorData: { gdprApplies: false } }, which is what a CMP returns outside the EU. Today's{ gdprApplies: false }is exactly the null-consent shape, so the current test pins the defect. - Add a GPP pair:
it.each([
['clears ts-eids when only GPP reports the EU TCF section', [2], undefined],
['writes ts-eids when GPP reports only non-EU sections', [7], [storedEid]],
])('%s', (_case, applicableSections, expected) => {
const spec = getAdapterSpec();
presetTsEidsCookie('preset-value');
spec.buildRequests(
trustedServerBidRequests(),
deepFreeze({
bidderCode: 'trustedServer',
auctionId: null,
ortb2: { user: { ext: { eids: [storedEid] } } },
gppConsent: { applicableSections },
})
);
expect(
expected === undefined ? readTsEidsCookie() : decodedTsEidsCookie(),
'should follow the GPP EU TCF section'
).toEqual(expected);
});With this patch, index.test.ts and the real-Prebid suite pass (312 tests), eslint and prettier are clean, and both new "clears" cases fail against the current function. A Purpose-level read of gppConsent.parsedSections.tcfeuv2 would be a later refinement; failing closed is the safe default.
| With the full rule, `trustedServer` still needs Purpose 2 consent and skips | ||
| only the vendor check. A publisher that sets `eidsRequireP4Consent` must also | ||
| add the same full rule for `personalizedAds`; without it `trustedServer` gets | ||
| bid requests with no EIDs and `ts-eids` is cleared after every auction. |
There was a problem hiding this comment.
🔧 wrench: Followed literally, this paragraph weakens the publisher's EID policy for every bidder
Prebid keys consentManagement.gdpr.rules by purpose with Object.fromEntries, so the last rule listed for a purpose wins (modules/tcfControl.ts:392-398). It also reads eidsRequireP4Consent from the active personalizedAds rule itself (:311). "Add the same full rule for personalizedAds" therefore produces a second rule.
I probed it with Purposes 1 to 3 granted, Purpose 4 denied, and vendor consent for a second bidder. The publisher's own Purpose 4 rule gives eidsBlocked: ["appnexus","trustedServer"]. Appending the full rule as this paragraph says gives eidsBlocked: []: every bidder receives EIDs with Purpose 4 denied. A publisher who already has a basicAds rule (for example with deferS2Sbidders) and appends the snippet above loses that rule the same way. Purpose 2 also accepts legitimate interest (LI_PURPOSES = [2], :126, :208-219).
| With the full rule, `trustedServer` still needs Purpose 2 consent and skips | |
| only the vendor check. A publisher that sets `eidsRequireP4Consent` must also | |
| add the same full rule for `personalizedAds`; without it `trustedServer` gets | |
| bid requests with no EIDs and `ts-eids` is cleared after every auction. | |
| With the full rule, `trustedServer` still needs Purpose 2 consent or legitimate | |
| interest and skips only the vendor check. Prebid keeps one rule per purpose, | |
| the last one listed, so add `softVendorExceptions: ['trustedServer']` to an | |
| existing `basicAds` rule instead of listing a second one. The same holds for | |
| `eidsRequireP4Consent`, which is a field of the `personalizedAds` rule. A | |
| publisher that sets it must extend that rule to | |
| `{ purpose: 'personalizedAds', enforcePurpose: true, enforceVendor: true, eidsRequireP4Consent: true, softVendorExceptions: ['trustedServer'] }` | |
| rather than add a second `personalizedAds` rule. A second rule listed after it | |
| drops `eidsRequireP4Consent` for every bidder; one listed before it is | |
| ignored, and `trustedServer` then gets bid requests with no EIDs and `ts-eids` | |
| is cleared after every auction. |
One more sentence belongs near the snippet. A publisher on the legacy top-level consentManagement shape (CMP settings with no gdpr key) who adds gdpr: { rules } makes Prebid read only config.gdpr (modules/consentManagementTcf.ts:200), which silently drops their top-level CMP settings. The guide should tell them to move those settings under gdpr.
| set is empty or when Prebid's TCF rules block `trustedServer`. Where GDPR | ||
| applies, it writes the cookie only with TCF Purpose 1 (store/access device) and | ||
| Purpose 4 (personalized ads) consent, the rule below; without it, the adapter | ||
| clears the cookie and still sends the EIDs in the `/auction` body. Auctions that | ||
| do not include `trustedServer` leave it unchanged. |
There was a problem hiding this comment.
🔧 wrench: The cookie consent check follows Prebid's consent data, not the visitor's jurisdiction
hasEidPersistenceConsent (index.ts:2852-2856) checks Purposes 1 and 4 only when the bidder request's gdprConsent.gdprApplies is true. Without Prebid GDPR data, which means no consentManagement.gdpr (Trusted Server enables it only for managed User IDs, index.ts:846), the adapter writes the cookie for every visitor, EU included. "Where GDPR applies" reads as a jurisdiction guarantee an operator or DPO would rely on. Spec 4.3 already states the condition precisely. The same wording sits in prebid.md:882-884, which has its own suggestion.
| set is empty or when Prebid's TCF rules block `trustedServer`. Where GDPR | |
| applies, it writes the cookie only with TCF Purpose 1 (store/access device) and | |
| Purpose 4 (personalized ads) consent, the rule below; without it, the adapter | |
| clears the cookie and still sends the EIDs in the `/auction` body. Auctions that | |
| do not include `trustedServer` leave it unchanged. | |
| set is empty or when Prebid's TCF rules block `trustedServer`. When Prebid's | |
| consent data for the request reports `gdprApplies: true`, it writes the cookie | |
| only with TCF Purpose 1 (store/access device) and Purpose 4 (personalized ads) | |
| consent, the rule below; without it, the adapter clears the cookie and still | |
| sends the EIDs in the `/auction` body. When Prebid has no GDPR data for the | |
| request (no `consentManagement.gdpr`, or no CMP on the page), the adapter | |
| writes the cookie and the server's gate below decides whether it is used. | |
| Auctions that do not include `trustedServer` leave it unchanged. |
| With such an exception but without TCF Purpose 1 (store/access device) and | ||
| Purpose 4 (personalized ads) consent, the adapter still forwards the EIDs in the | ||
| `/auction` body but clears `ts-eids` instead of writing it. |
There was a problem hiding this comment.
🔧 wrench: Same consent-condition wording as edge-cookies.md:164. The check applies to Prebid's GDPR consent data for the request, and without that data the adapter writes ts-eids.
| With such an exception but without TCF Purpose 1 (store/access device) and | |
| Purpose 4 (personalized ads) consent, the adapter still forwards the EIDs in the | |
| `/auction` body but clears `ts-eids` instead of writing it. | |
| With such an exception but without TCF Purpose 1 (store/access device) and | |
| Purpose 4 (personalized ads) consent in Prebid's GDPR consent data, the adapter | |
| still forwards the EIDs in the `/auction` body but clears `ts-eids` instead of | |
| writing it. Without Prebid GDPR data for the request, the adapter writes | |
| `ts-eids`, and Trusted Server applies its own Purpose 1 and 4 gate before using | |
| it. |
| @@ -0,0 +1,1617 @@ | |||
| # Prebid EID fidelity | |||
|
|
|||
| Status: Reviewed. Round 5 found nothing blocking (see the review log). Backwards | |||
There was a problem hiding this comment.
🔧 wrench: The spec, its review log and the red log predate the last three design commits
d51bbeced and 9b901f237 added the cookie consent gate, and 88725f4d6 changed legacy cookie validation, but only 4.3, 4.8, 8.2 and 8.5 were updated. forwardBidderEids (index.ts:2947-2951) now clears the cookie without consent even when the set is non-empty, and these sections contradict that:
- G2 (
:244-247) and A7 (:1501-1504) say the cookie is cleared only "when the set is empty or Prebid's TCF rules blocktrustedServer". Add "or, where Prebid reportsgdprApplies: true, TCF Purpose 1 or 4 is not consented". - 4.2 step 3 (
:612-614) says "Ifeidsisundefined, it clearsts-eids. Otherwise it writests-eids". - 4.6 (
:829-830) says Lockr's ID5 entry "reaches the body and the cookie" unconditionally. - The 4.11 table has no row for "GDPR applies without Purposes 1 and 4 in
gdprConsent| EIDs sent in the body; cookie cleared before the request". - 4.12 (
:1006-1008) omits the'cleared: no consent'decision the code logs. - 7.3 and 7.5 don't list the storage-consent unit cases or the real-Prebid "Purpose 1 denied" arm.
Two smaller stale points:
- 4.4 step 1 (
:680) saysVec<Box<RawValue>>, but328dd2c17borrowsVec<&RawValue>. - 4.6 (
:807) says "Any other source, or none: thets-eidscookie", but a returning EC with no sync source ingests nothing (ec/finalize.rs:83-88).
Process gaps:
- Line 3 says "Round 5 found nothing blocking", yet the design changed after Round 5, and Round 4 (
:1589-1590) still calls the missing Purpose 1 check a follow-up. CONTRIBUTING (:85-101) asks for one log entry per round, and for rounds until one finds nothing blocking, so this needs a Round 6 entry covering the three commits. - The red log (
implementation-notes/2026-10-18-prebid-eid-fidelity.md:188-235) has no entries for thestorage consentit.eachcases, the real-Prebid "Purpose 1 denied" arm, orinvalid_legacy_cookie_uids_neither_reach_providers_nor_displace_kv, which A12 (:1518) requires. - The plan's hand-off text (
plans/2026-10-18-prebid-eid-capture-fidelity.md:5444,:5447) still says the cookie is written "without a Purpose 1 check of its own" and lists "gating the cookie write on Purpose 1" as a follow-up.
| `openrtb_atype` sets the `atype` of EIDs that Trusted Server builds from KV. | ||
| It applies only where KV supplies a source the browser did not send; for a | ||
| source in the browser's EIDs, the browser's own entry and `atype` are | ||
| forwarded instead. |
There was a problem hiding this comment.
🔧 wrench: openrtb_atype still sets the atype that /identify returns (ec/identify.rs:132), so "applies only where KV supplies a source the browser did not send" is too broad. Start the second sentence with "In auction requests, it applies only where…".
| Each entry is validated on its own. A malformed entry or field is dropped and | ||
| the request proceeds; only a malformed body outside `eids` rejects the request. |
There was a problem hiding this comment.
🔧 wrench: Raw capture isolates semantic faults, not syntax. A body whose eids contains a raw control character, an invalid \u hex escape or unbalanced brackets is still rejected as a whole (client_eids.rs:61-63 captures each element without decoding escapes, but serde_json still checks syntax). Suggested wording: "Each entry is validated on its own. A malformed entry or field, including one that fails to parse on its own such as a lone surrogate escape, is dropped and the request proceeds; a body that is not well-formed JSON, or that is malformed outside eids, is rejected."
| * request as plain data and the server validates each one, so the server | ||
| * defines the contract. | ||
| */ | ||
| export type AuctionEid = unknown; |
There was a problem hiding this comment.
🔧 wrench: AuctionEid is now an alias for unknown, used only in this file (:36, :72) and not re-exported from src/index.ts or core/index.ts. A named alias for unknown suggests a contract that doesn't exist. Inline it as eids?: unknown[] and keep this doc comment on the field.
| }); | ||
| }); | ||
|
|
||
| it('exposes config names for modules that do not map EID sources', () => { |
There was a problem hiding this comment.
🔧 wrench: With eidSources gone, no module maps EID sources, so this title describes a distinction that no longer exists. Suggested title: 'exposes config names for every registered module'.
|
|
||
| const utf8Encoder = new TextEncoder(); | ||
|
|
||
| function isRecord(value: unknown): value is Record<string, unknown> { |
There was a problem hiding this comment.
🔧 wrench: isRecord now exists twice, here and at index.ts:250, and tcfBlocksTrustedServer (index.ts:2963-2974) re-implements the same object check inline. Export it from one place (this module, or a small shared helper) and reuse it in index.ts.
prk-Jr
left a comment
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
The PR's own changes are unchanged since the previous review at 9b901f2; the two new commits merge main. One new problem comes from that merge: the required cargo test check now fails because two tests each install a process-wide test logger. The Rust core changes I re-read (the shared parser, the browser-first merge, /auction ingestion, the new Eid fields) raised no new concerns. The earlier review's findings are all still open.
Blocking
🔧 wrench
- Two tests install a global test logger, so the required
cargo testcheck fails: see inline atcrates/trusted-server-core/src/ec/client_eids.rs:272
Cross-cutting / body-level findings
- 🔧 Required check
cargo testfails on88cc505.publisher::tests::html_finalization_client_cancellation_drops_work_and_warns_for_auction_once(added onmainby #1290, merged in here) fails withleft: Some(0), right: Some(1). The cause and fix are in the inline comment. The panic aborts the whole Viceroy test binary, so CI ran none of the core tests after it. Locally, skipping that one test, the rest of the core suite passes (3,087 passed, 0 failed), so nothing else is hiding behind the abort. - 📝 The previous review's findings are still open. All 17 inline threads from the
9b901f2review are unresolved with no reply, and the code they point at is unchanged. Two remain the most consequential:hasEidPersistenceConsentstill treats a missinggdprConsent, orgdprApplies !== true, as "GDPR does not apply", sots-eidsis written after a CMP timeout or with a GPP-only CMP. And the guide'seidsRequireP4Consentinstruction, followed literally, drops the publisher's Purpose 4 EID rule for every bidder. The two body-level items, aCHANGELOG.mdBreaking entry and the IdentityLink validation step, are also still open.
CI Status
- cargo test: FAIL (required)
- cargo fmt: PASS (required)
- format-docs: PASS (required)
- format-typescript: PASS (required)
- Analyze (actions): PASS
- Analyze (javascript-typescript): PASS
- Analyze (python): PASS
- Analyze (rust): PASS
- browser integration tests: PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): 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
- CLAUDE.md symlink guard: PASS
- CodeQL: PASS
- docs-proposal scripts: PASS
- HTML EOF runtime (Fastly): PASS
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- prepare integration artifacts: PASS
- vitest: PASS
|
|
||
| fn install_capturing_logger() { | ||
| INSTALL_LOGGER.call_once(|| { | ||
| log::set_logger(&CapturingLogger).expect("should install the capturing test logger"); |
There was a problem hiding this comment.
🔧 wrench: Two tests install a global test logger, so the required cargo test check fails
log::set_logger succeeds once per process. This test module installs CapturingLogger. publisher::tests::html_finalization_client_cancellation_drops_work_and_warns_for_auction_once, added on main by #1290 and merged into this branch in 88cc505, installs its own CancellationLogger with let _ = log::set_logger(&LOGGER) and then expects exactly one cancellation warning. Under Viceroy the core tests run one at a time in name order, and ec::client_eids sorts before publisher, so this logger always wins. The publisher test then counts zero warnings:
assertion `left == right` failed: should preserve exactly one existing auction cancellation warning
left: Some(0)
right: Some(1)
Reproduced locally at 88cc505 with cargo test -p trusted-server-core --lib --target wasm32-wasip1:
- The publisher test alone passes.
ec::client_eidsalone passes (15 tests).- Both together abort as in CI.
On native multithreaded runs the order is random, and this .expect can panic instead.
Proposed fix (apply manually: it touches test_support.rs, this module and the publisher.rs test, so it can't be a single suggestion). Install one logger for the whole crate's tests and send records to a per-thread buffer that a test switches on:
// crates/trusted-server-core/src/test_support.rs, inside `#[cfg(test)] mod tests`
static INSTALL: std::sync::Once = std::sync::Once::new();
thread_local! {
static CAPTURED: std::cell::RefCell<Option<Vec<String>>> =
const { std::cell::RefCell::new(None) };
}
struct ThreadCaptureLogger;
impl log::Log for ThreadCaptureLogger {
fn enabled(&self, _: &log::Metadata<'_>) -> bool {
true
}
fn log(&self, record: &log::Record<'_>) {
CAPTURED.with(|captured| {
if let Some(lines) = captured.borrow_mut().as_mut() {
lines.push(record.args().to_string());
}
});
}
fn flush(&self) {}
}
/// Runs `f` and returns the log lines it emitted on this thread.
pub(crate) fn capture_logs<R>(f: impl FnOnce() -> R) -> (R, Vec<String>) {
INSTALL.call_once(|| {
log::set_logger(&ThreadCaptureLogger).expect("should install the test logger");
log::set_max_level(log::LevelFilter::Debug);
});
CAPTURED.with(|captured| *captured.borrow_mut() = Some(Vec::new()));
let result = f();
let lines = CAPTURED.with(|captured| captured.borrow_mut().take().unwrap_or_default());
(result, lines)
}Then this test wraps parsed_json(...) in capture_logs, and the publisher test counts the lines that start with Dispatched server-side auction dropped without collection. Both stop depending on test order, on Viceroy and natively.
Summary
main.trustedServerPrebid adapter forwards exactly the EIDs Prebid placed in its own bidder request (ortb2.user.ext.eidsanduser.eids), with their OpenRTB 2.6inserter,matcher,mmandext. It no longer readspbjs.getUserIdsAsEids(), which leaked IDs restricted to other bidders or redacted fortrustedServer(D1) and missed everyortb2EID, such as Lockr's ID5 (D2, Write Lockr IDs to KV store #1246).ts-eidswriter moved intobuildRequests, so the cookie copies each request's set (without EID-levelext) before the request is sent. Where GDPR applies, it is written only with TCF Purpose 1 and Purpose 4 consent, the rule the server applies before it uses or stores these EIDs, and atcf2Enforcementlistener clears it when Prebid's TCF rules blocktrustedServer. The server validates body and cookie with one parser that drops only the bad entry or field, forwards browser entries as sent with KV filling only absent registered sources, and ingests into KV on/auctiononly the client set the auction used. TS stops overwriting the publisher's PrebiduserSyncsettings (D4).ts-eidsandx-ts-eidsboth stay.Changes
docs/superpowers/specs/2026-10-18-prebid-eid-fidelity-design.mddocs/superpowers/plans/2026-10-18-prebid-eid-capture-fidelity.mddocs/superpowers/implementation-notes/2026-10-18-prebid-eid-fidelity.mdcrates/trusted-server-core/src/openrtb.rsEidgainsinserter,matcher,mm,ext(skipped when absent, byte-identical otherwise) andEid::newcrates/trusted-server-core/src/ec/client_eids.rs(new)crates/trusted-server-core/src/auction/formats.rs,auction/endpoints.rsAdRequest.eidscaptured as raw JSON; body and cookie go through the shared parser;merge_auction_eidsforwards browser entries as sent and adds KV only for absent registered sourcescrates/trusted-server-core/src/ec/prebid_eids.rs,ec/admin.rsts-eidscookie decoded through the shared parser; legacy cookie UIDs validated before browser precedence; admin diagnostics keep raw sources and UIDscrates/trusted-server-core/src/ec/finalize.rs/auctionreads its own client set, other routes the cookiecrates/trusted-server-core/src/{publisher.rs, ec/eids.rs, ec/identify.rs, integrations/*, auction/*}Eid::newcall sites; outbound serialization, merge and ingestion testscrates/trusted-server-adapter-fastly/src/app.rsts-eidshanded to finalizationcrates/trusted-server-js/lib/src/integrations/prebid/eids.ts(new)ts-eidsvalue builder (no EID-levelext, 3,072 bytes, UTF-8 base64)crates/trusted-server-js/lib/src/integrations/prebid/index.ts,src/core/auction.tsbuildRequestsforwards the bidder request's EIDs and writes or clears the cookie behind the Purpose 1 and 4 check; TCF listener; old collector, cookie sync andinstallUserIdModulesremovedcrates/trusted-server-js/lib/src/integrations/prebid/user_id_modules.{ts,json}eidSourcesremovedcrates/trusted-server-js/lib/test/**eids.test.ts; adapter tests with agetUserIdsAsEidscanary and paired consent cases; real-Prebid JSDOM suiteprebid-eid-fidelity.test.mjs; cross-language fixturesdocs/guide/**trustedServerTCF exception ruleCloses
Closes #1246
Spec
Spec:
docs/superpowers/specs/2026-10-18-prebid-eid-fidelity-design.md(this PR; five agent review rounds in its review log)What breaks (spec 8.2)
atypeare no longer sent for a source the browser sends, and repeated sources are no longer folded.syncEnabled,filterSettings,syncsPerBidder,syncDelayandauctionDelay, and no longer force-enables iframe syncs. Auctions wait up toauctionDelay(500 ms by default) while IDs are pending.trustedServer: IDs restricted to them are no longer forwarded.trustedServerexception the cookie is cleared instead of written fromgetUserIdsAsEids(); with one, it is written only with Purpose 1 and 4 consent. Body-less routes and KV ingestion lose browser EIDs for the rest of that traffic.ortb2EIDs and provenance fields, never EID-levelext, written at request time./auctionno longer reads the cookie next to the body.user.ext.eidsandx-ts-eids./_ts/admin/eidsconsumers: new fields in the cookie entries; a mistyped field is dropped instead of failing the whole cookie.Operator actions (spec 8.3)
ts prebid client).userSyncchange.trustedServerthe full TCF exception rule.Follow-ups (spec 8.5)
Prebid.js identityLink fix for a malformed
_lr_env(D7);sharedIdcookie ingestion ignoring Prebid restrictions; TS's own TCF identity, with a vendor-level check on thets-eidswrite; per-bidder EID forwarding through Prebid Servereidpermissions; refresh rules for browser-sourced KV IDs; restoringautoRefreshafter a late managed seed; a server-written cookie; a lenient parse of the rest of the/auctionbody.Test plan
cargo test-fastly && cargo test-axum(on Preserve Prebid EID fidelity from browser capture to bidder request #1415: core 2985 passed, plustest-fastly-reuse,test-cloudflare,test-spin; after the review fixes:test-fastly, core 3015 passed)cargo clippy-fastly && cargo clippy-axum(all eight aliases on Preserve Prebid EID fidelity from browser capture to bidder request #1415;clippy-fastlyafter the review fixes)cargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest run(1258 passed, Node 24.12.0), including the real-Prebid suitenpm run format && npm run lintcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute servetemplate_build_digest,scripts/lint-wasm-clock.sh,scripts/test-cli.shon Preserve Prebid EID fidelity from browser capture to bidder request #1415ts dev proxyon a page with Lockr and the default User ID preset (spec 7.6)Checklist
unwrap()in production code — useexpect("should ...")logmacros per AGENTS.md (notprintln!)