Repository navigation
Conversation
Add EID value types, an additive KV schema for provider-written EIDs with owner and expiry, EID-level fields on the OpenRTB Eid, and a resolver that serves only live UIDs to auctions, x-ts-eids, identify and the admin auction preview. Auction EIDs merge per (source, inserter, matcher, mm, ext) with first-occurrence UID dedupe. KV writes over 16 KiB are refused. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Top-level document navigations on the publisher route receive a single-use write permit; finalize plans one row change (browser ingest, provider results, prune, size cap) and commits it once without retry. Non-navigation requests and redirects never write, integration proxies no longer mint ECs, and new ECs are created by the page-load write instead of at generation. Withdrawal tombstones only the active EC and keeps the EC cookie when the tombstone does not land; the withdrawal completion marker is the one deliberate extra write. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Add the IdentityProvider/ProviderKind interface and [[ec.identity_providers]] config, response validation, a deadline-bounded LookupTransport over select_until, the IdentityCoordinator with consent, allowlist and budget checks, the trusted-server-identity-providers crate (no kinds yet), and Fastly wiring that dispatches before the origin fetch and collects before the single page-load write. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Orphan recovery minted a new EC ID and then discarded the navigation's provider results because they were looked up under the orphaned ID. Lookups depend on the browser, not the EC ID, and the orphan has no row, so retarget them to the replacement as the spec requires. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Collection that starts after its deadline takes one nonblocking snapshot. A request still in the headers state moved to the body state without reading bytes, which counted as no progress, so a response that had fully arrived was abandoned unread and reported as a timeout. Count entering the body state as progress. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
A response body the host already holds can report not ready until the host services async I/O once, so an expired snapshot abandoned identity responses that arrived well before the deadline and reported them as timeouts. Give each pending body one host select capped at 1 ms before abandoning it. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
…not fill Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Page-load writes only run on real navigations, which browsers mark with both sec-fetch-dest: document and sec-fetch-mode: navigate. Without the mode header the consent withdrawal scenario never got a write permit, so the EC cookie was kept. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
A lookup sends the EC ID to the provider before finalize can decline the EC, so GPC, sale opt-out and unknown-jurisdiction requests must not dispatch. Matches the gate pull sync already applies. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
… read LiveRamp envelopes are about 700 bytes, so the 512-byte limit dropped every one from KV and the auction. Rows written under a higher limit now load with the oversized IDs removed instead of failing as a whole. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
304f395 to
2de6d0d
Compare
|
@dhruv8sh, directionally the layout here is not compatible with V2 (the 55 pull requests tracked in #777, merged in order). This pull request creates one shared The V2 spec is identifier-modules and pluggable-providers, with the folder layout in integrations-crate, all in #1342. The associated pull requests are #1382 (a module hands core its identifiers, starting with the Prebid EID cookie) and #1343 (the Edge Cookie as a module, with the HMAC module built in). Could you evaluate the V2 spec and those pull requests before progressing this one? |
…readable rows Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
…or gate Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Providers set to collect = "after_response" look up after the page is sent and store their results with one compare-and-swap follow-up write (Fastly only). Providers may keep private refresh state with a UID, generate IDs, pass request context to their reply parser and report an opt-out. Refresh is due only while the UID is live. Unusable refresh state is dropped without dropping the UID. The admin EC view redacts refresh state. Operators can bound lookups per provider with rate_limit_per_hour. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
…0 days Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
An EC minted on the navigation, or one whose row is missing or withdrawn when read after the response, is skipped before any provider is called, so no vendor is sent an EC ID whose row cannot take the result. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
The follow-up write was guarded by the generation read before the lookups were dispatched, up to the full lookup budget earlier, so any write from another request in that window dropped the results. The row is now read just before the write, so only the gap between that read and the write can conflict. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
A refresh time at or before now made the source due on every navigation until the UID expired. Such state is now dropped like other unusable refresh state; the UID itself is kept. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
|
If a user withdraws vendor consent for a provider but keeps Purposes 1 and 4, the lookup is skipped, but the UIDs that provider already wrote stay in the row and keep going out in |
…nularity Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
aram356
left a comment
There was a problem hiding this comment.
Summary
This PR adds the identity-provider framework, full-fidelity EID storage and a single page-load write. CI is green and the pieces fit together, but four problems block it. The single-write change drops consent withdrawals outside permitted navigations. Withdrawn vendor consent does not stop stored provider UIDs from going to bidders. Both the config blob and the 1024-byte UID limit break rollback. And until #1488 lands, every configured provider is skipped with no startup error.
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 this branch. The other comments describe the fix in prose because it touches several files or hunks and can't be auto-applied.
Blocking
🔧 wrench
- A withdrawal now takes effect only on a permitted, non-redirect publisher navigation (inline at
crates/trusted-server-core/src/ec/finalize.rs:503) - Withdrawn TCF vendor consent does not stop a provider's stored UIDs from going to bidders (inline at
crates/trusted-server-core/src/ec/eids.rs:118) - A config blob pushed by this build breaks a runtime from before it, even with no providers (inline at
crates/trusted-server-core/src/settings.rs:723) - An enabled provider with an unregistered kind, or one that fails to build, is skipped without a startup error (inline at
crates/trusted-server-identity-providers/src/lib.rs:136) - Two providers can share an
id, and then clear and read each other's IDs (inline atcrates/trusted-server-core/src/ec/provider.rs:495, suggestion) - A deeply nested
extpasses this check, then panics on every serve of the row (inline atcrates/trusted-server-core/src/ec/eid_types.rs:383, suggestion) - The new row size cap makes batch sync give up on the rest of the batch (inline at
crates/trusted-server-core/src/ec/kv.rs:187) - Raising the UID limit to 1024 bytes lets a rollback lose consent withdrawals (inline at
crates/trusted-server-core/src/ec/kv_types.rs:42) - Operator docs and the CHANGELOG still describe main (body, below)
❓ question
- A freshly minted EC reaches bidders and providers before its row exists (inline at
crates/trusted-server-core/src/ec/mod.rs:481) - Spec link and the V2 layout question (body, below)
Non-blocking
🤔 thinking
- A withdrawal tombstone gets one CAS attempt, and a listed-but-missed row is no longer tombstoned (inline at
crates/trusted-server-core/src/ec/finalize.rs:587) - Browser EIDs from
/auctionand page-bids are no longer persisted (inline atcrates/trusted-server-core/src/ec/finalize.rs:227) - The after-response permit is issued even when the orphan-confirm write fails (inline at
crates/trusted-server-core/src/ec/finalize.rs:411) - An expired record blocks browser fill once its source is no longer provider-owned (inline at
crates/trusted-server-core/src/ec/write_plan.rs:88) rate_limit_per_houris per IP-derived EC hash, not per EC (inline atcrates/trusted-server-core/src/ec/coordinator/mod.rs:336)- Generated IDs are compared with the due sources before normalizing (inline at
crates/trusted-server-core/src/ec/coordinator/mod.rs:539, suggestion) - No negative caching: an unmatched visitor triggers a vendor call on every navigation (inline at
crates/trusted-server-core/src/ec/coordinator/mod.rs:294) - A provider UID without a stated expiry is never looked up again (inline at
crates/trusted-server-core/src/ec/coordinator/validate.rs:135) - Neither identity budget is range-validated (inline at
crates/trusted-server-core/src/settings.rs:710) - Identity lookups inherit
proxy.certificate_check(inline atcrates/trusted-server-adapter-fastly/src/main.rs:807) - The after-response re-read may often lose its CAS to this navigation's own writes (inline at
crates/trusted-server-core/src/ec/after_response.rs:95) - This expired-snapshot change also changes the auction contract (inline at
crates/trusted-server-adapter-fastly/src/platform/deadline.rs:283) - Read tolerance covers only UID length, and a partial skip is silent (inline at
crates/trusted-server-core/src/ec/kv_types.rs:302) - A browser record in a provider-owned source is served indefinitely (inline at
crates/trusted-server-core/src/ec/eids.rs:93) - Client EIDs for provider-owned sources still reach bidders after a provider opt-out (inline at
crates/trusted-server-core/src/auction/endpoints.rs:649)
♻️ refactor
RegisteredProvider::newignores the configuredrate_limit_per_hour(inline atcrates/trusted-server-core/src/ec/coordinator/mod.rs:163, suggestion)- Move the after-response sequence into core so it can be tested in this PR (inline at
crates/trusted-server-adapter-fastly/src/main.rs:771)
⛏ nitpick
- This example fails validation as written (inline at
docs/guide/configuration.md:834, suggestion) - Uncommented in order, these budget lines land in the partner table (inline at
trusted-server.example.toml:112, suggestion) - Smaller items (body, below)
Cross-cutting / body-level findings
- 🔧 Operator docs and the CHANGELOG still describe main.
docs/guide/integrations/prebid.md:868-873says the same/auctionrequest ingests the body entry into KV and that page-bids ingests the cookie;:961-963still lists/auctionrequest-body EIDs as an ingestion source. Neither is true at this head.docs/guide/edge-cookies.md:45says EC generation happens on the publisher proxy, integration proxy and auction. A new EC is now minted only on a publisher-proxy navigation withGET,Sec-Fetch-Dest: document,Sec-Fetch-Mode: navigateand noprefetchinSec-Purpose, and that requirement is not documented anywhere. Chrome prerender (Sec-Purpose: prefetch;prerender) never mints or writes.edge-cookies.md:136: the diagram still says "/auction body + ts-eids + sharedId".edge-cookies.md:151calls the navigation "the only request that writes KV"; batch sync, pull sync, the after-response write and tombstones also write.:187("at most one conditional write", "reads the row once") is not true with after-response providers, and no re-read happens after a conflict.CHANGELOG.md[Unreleased]has no entries for this PR. Candidates for Breaking: clients without Fetch Metadata get no EC; integration proxies no longer mint;/auctionand page-bids EIDs are not persisted; withdrawals apply only on navigations (if kept); the rollback limits in thesettings.rsandMAX_UID_LENGTHcomments.configuration.md:819andtrusted-server.example.toml:119say the kind must be "built into this release", but no kind is registered in this release.configuration.md:823markssourcesas required, but an empty list is accepted (and the provider is then alwaysNotDue).:848says after-response lookups run "only for an EC whose row already existed"; a row recovered from an orphan on the same navigation also qualifies, and the phase also needsec_store, a successful page-load write and a non-redirect status. Providers run only on Fastly: Axum, Cloudflare and Spin ignoreec.identity_providerswith no log, which the docs don't say.docs/guide/api-reference.md:365-366lists the batch-sync item reasons without the newprovider_owned, and no handler-level test checks the 207 response for it.
- ❓ Spec link and the V2 layout question.
- CONTRIBUTING's "Features start with a spec" asks feature PRs to link an agent-reviewed spec that ends in a review log. The description says the spec was "reviewed separately; not included", and the spec in #1428 was closed with changes requested. Please link the spec this PR implements. Code comments cite its sections ("spec §3.1", "§8.4", "§12.1", "D1") in
adapter-fastly/src/app.rs:1049,ec/finalize.rs:501,ec/write_plan.rs:7,ec/provider.rs:479andconfig.rs:347, but no such spec is in the repo. - jwrosewell asked on 2026-11-03 for an evaluation of the V2 identifier-modules layout (#1342, #1382, #1343) before this PR progresses. That question has no reply here yet.
- CONTRIBUTING's "Features start with a spec" asks feature PRs to link an agent-reviewed spec that ends in a review log. The description says the spec was "reviewed separately; not included", and the spec in #1428 was closed with changes requested. Please link the spec this PR implements. Code comments cite its sections ("spec §3.1", "§8.4", "§12.1", "D1") in
- ⛏ Smaller items.
crates/trusted-server-identity-providers/Cargo.toml:logis unused, and the AGENTS.md workspace layout does not list the new crate.- The description cites
ec/coordinator/after_response.rs; the file isec/after_response.rs. ec/kv.rs:463links to the removedSelf::create_if_absent(rustdoc reports it as unresolved), and "Production EC generation usescreate_if_absent" is no longer true.- Dead plumbing left by the single write:
set_client_eidsand its comment atauction/endpoints.rs:333-339(only/auctionsetsclient_eids, and/auctionnever writes), the stale comment at:349-352,EidSyncSource::NewEc, the unreachable "no row change was planned" branch (ec/finalize.rs:194-196), thekeep_presentguard (publisher.rs:5366-5376),SkipReason::Disabled, and the pass-throughIdentityCoordinator::plan_write. - A
build_requestfailure is reported asSkipReason::Input(coordinator/mod.rs:552-562), and a 404 or 204 becomes NoMatch before the provider parses the response (:393-398), so a provider bug or a wrong endpoint path looks healthy in metrics. publisher.rs:5315-5316: the snapshot preload reads KV on navigations without a permit (no Fetch Metadata, prefetch), which can never write.ec/kv_types.rs:199:prune_expiredclones the whole record on every call just to compare it.
- 📌 Provider EIDs now go back to the browser.
auction/formats.rs:570-576(outside the diff): the/auctionx-ts-eidsresponse header now carries server-side provider EIDs, withext,inserterandmm, back to the page. The mechanism is the same as for synced partner IDs on main, but the data is new; worth confirming vendor terms allow it, especially for RampID envelopes. - 🌱 One secret per provider.
ProviderSecretsholds onlytoken. A vendor that needs a client ID plus a secret, or a signing key, would have to put the second credential insettingsor the endpoint's query string, and both are pushed unredacted in the config blob. Worth confirming every #1488 vendor fits one token before the API settles.
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
- CI Node pin guard: PASS
- CLAUDE.md symlink guard: PASS
- CodeQL: PASS
- docs-proposal scripts: PASS
- format-docs: PASS (required)
- format-typescript: PASS (required)
- HTML EOF runtime (Fastly): 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
| let Some(permit) = permit else { | ||
| // Not a page-load navigation: serve nothing, keep the EC cookie so the | ||
| // next navigation can write the tombstone (spec §3.1). | ||
| return; | ||
| }; |
There was a problem hiding this comment.
🔧 A withdrawal now takes effect only on a permitted, non-redirect publisher navigation
On main, finalize_unusable_consent tombstoned the row and expired ts-ec on any finalized request that carried an explicit withdrawal and a cookie. Here, without a PageLoadWrite it returns at this line, so the cookie stays and the row keeps consent.ok = true.
A probe with a live row, a ts-ec cookie and a CA + GPC withdrawal, run at this head and at the merge base:
head navigation 200: row_consent_ok=false ec_cookie_expired=true
head navigation 302: row_consent_ok=true ec_cookie_expired=false
head POST /auction: row_consent_ok=true ec_cookie_expired=false
base any route, 200 or 302: row_consent_ok=false ec_cookie_expired=true
The withdrawal is now never written for:
/auction,/_ts/page-bids(SPA soft navigations), integration proxies and/identify;- navigations answered with 301/302/303/307/308, POST navigations, and
Sec-Purpose: prefetchor prerender; - every client that sends no Fetch Metadata (Safari before 16.4, some privacy proxies). None of their requests gets a permit, so their row stays consented until it expires.
While the row says consent.ok = true, batch sync keeps writing into it, and the guarantee in pull_sync.rs:100-110 (a concurrent withdrawal tombstones the row before dispatch) no longer holds. The description's "Behaviour changes" section does not list this, and docs/guide/edge-cookies.md:79-83 and :273 still describe main.
Proposed fix (apply manually, it changes the permit contract): treat the withdrawal tombstone as a privacy write outside the one-write-per-page-load rule. Withdrawals are rare and the completion marker already stops repeats, so for any finalized request with consent_withdrawn && cookie_was_present, write the tombstone (as main's tombstone_existing_from_snapshot did) and expire the cookie once it lands or the row is proven absent. Please add tests for a withdrawal on /auction and on a redirect; withdrawal_without_permit_keeps_row_and_cookie currently pins the regression.
| pub fn live_eids<'row>( | ||
| &'row self, | ||
| registry: &'row PartnerRegistry, | ||
| now: UnixSeconds, | ||
| ) -> impl Iterator<Item = EidView<'row>> + 'row { | ||
| self.ids.iter().flat_map(move |(source, record)| { | ||
| let partner = registry | ||
| .get(source) | ||
| .filter(|partner| partner.bidstream_enabled); | ||
| partner | ||
| .into_iter() | ||
| .flat_map(move |partner| source_views(source, record, partner.openrtb_atype, now)) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🔧 Withdrawn TCF vendor consent does not stop a provider's stored UIDs from going to bidders
This is aleksUIX's open comment, and it still holds at this head. When tcf_vendor_id is denied, the coordinator skips the lookup with SkipReason::Consent (coordinator/mod.rs:501-507), but cleared is filled only for OptedOut (mod.rs:356-360), so the stored UIDs stay. live_eids filters only on bidstream_enabled, and gate_eids_by_consent (called at auction/endpoints.rs:383 and publisher.rs:6170) checks only Purposes 1 and 4.
A probe with a stored provider UID, tcf_vendor_id = 9999, and a TC string granting Purposes 1 and 4 with vendor list [1]:
status=Skipped(Consent) cleared=[] sent=0
plan_write is_none=true
egress after gate_eids_by_consent = ["ids.example.com=p-1"]
No test covers egress with a denied vendor. Prebid's enrichEids rule in tcfControl drops a module's EIDs without its vendor consent, and the server path should match.
Proposed fix (apply manually, spans registry.rs, eids.rs and both auction call sites): record the owning provider's tcf_vendor_id on the partner entry in PartnerRegistry::from_ec_with_kinds, next to provider_owned, and skip that source here when tcf_vendor_consent_denied(consent, vendor) holds. That needs the consent context passed into live_eids / resolve_auction_eids. Then add the test asked for in the thread: a stored UID, Purposes 1 and 4 granted, vendor denied, and user.eids without it.
Related: tcf_vendor_id is optional, so a provider configured without it sends EU lookups (EC ID, IP and UA) on purposes alone. Should validation require it, or should configuration.md at least say so?
|
|
||
| /// Shared time budget, in milliseconds, for all provider lookups on one | ||
| /// navigation. | ||
| #[serde( | ||
| default = "Ec::default_identity_budget_ms", | ||
| deserialize_with = "from_value_or_str" | ||
| )] | ||
| pub identity_budget_ms: u64, | ||
|
|
||
| /// Time budget, in milliseconds, for lookups of providers with | ||
| /// `collect = "after_response"`, measured from their dispatch after the | ||
| /// response has been sent. | ||
| #[serde( | ||
| default = "Ec::default_identity_after_response_budget_ms", | ||
| deserialize_with = "from_value_or_str" | ||
| )] | ||
| pub identity_after_response_budget_ms: u64, | ||
|
|
||
| /// Server-side identity providers. | ||
| #[serde(default, deserialize_with = "vec_from_seq_or_map")] | ||
| pub identity_providers: Vec<IdentityProviderConfig>, |
There was a problem hiding this comment.
🔧 A config blob pushed by this build breaks a runtime from before it, even with no providers
ts config push writes the serialized Settings (config.rs:101-107). Ec is deny_unknown_fields, and these three fields have no skip_serializing_if, so every blob this CLI pushes carries identity_budget_ms, identity_after_response_budget_ms and identity_providers, whether or not any provider is configured.
A probe serialized this head's minimal settings and read them back with the merge base's Settings:
head blob ec keys = [..., "identity_after_response_budget_ms", "identity_budget_ms", "identity_providers", ...]
base reads head blob -> unknown field `identity_budget_ms`, expected one of `passphrase`, `ec_store`, ...
base reads head blob without those three keys -> ok
head reads base blob -> ok
So pushing config before the new runtime is live, or rolling the runtime back after a push, leaves the deployed runtime unable to load any config. The repo already handles this for allow_async_html_finish, html_finalization_timeout_ms, include_domains and sandbox_metrics_enabled, with tests such as default_debug_config_omits_sandbox_metrics_for_rollback.
Proposed fix (apply manually: the predicates go in impl Ec, a separate hunk). With this change the merge base reads this head's default blob:
#[serde(
default = "Ec::default_identity_budget_ms",
skip_serializing_if = "Ec::is_default_identity_budget_ms",
deserialize_with = "from_value_or_str"
)]
pub identity_budget_ms: u64,
// Same shape for identity_after_response_budget_ms.
#[serde(
default,
skip_serializing_if = "Vec::is_empty",
deserialize_with = "vec_from_seq_or_map"
)]
pub identity_providers: Vec<IdentityProviderConfig>, #[allow(clippy::trivially_copy_pass_by_ref)]
fn is_default_identity_budget_ms(value: &u64) -> bool {
*value == Self::default_identity_budget_ms()
}Add a rollback test that a default Ec serializes without the three keys. A blob with providers configured still cannot roll back, so configuration.md should say that, the way docs/guide/cli.md:663-675 does for ad-template keys.
| .filter(|config| { | ||
| config.enabled && config.collect == timing && is_registered_kind(config.kind()) | ||
| }) |
There was a problem hiding this comment.
🔧 An enabled provider with an unregistered kind, or one that fails to build, is skipped without a startup error
This filter drops unregistered kinds before build, so ProviderConfigError::UnknownKind is unreachable in production. registry.rs:240-248 logs a warning and leaves the provider's sources unowned. The coordinators are built per navigation (adapter-fastly/src/app.rs:484-497, main.rs:785-792), so a capability mismatch or a bad settings table becomes log::error!("... disabled for this request") on every page view, and one bad provider disables every provider in its phase (the collect::<Result<_>>()? below).
Scenario: an operator configures kind = "examplekind" (a typo), or any kind before #1488 lands. ts config push, validate_settings_for_runtime and startup all succeed (the PR's own runtime_validation_accepts_valid_identity_provider uses the unregistered "example-kind"), the provider never runs, and its sources quietly go back to ts-eids browser ingest, batch sync and pull sync. The guarantee that browser IDs never fill provider-owned sources is then off with no startup signal. The doc comments at config.rs:349-350 and provider.rs:483-484 say unknown kinds "are enforced when the adapter builds the coordinator"; they are not.
Proposed fix (apply manually, crosses crates):
- Build both phase coordinators once in
build_state_from_settings, next to the cachedpartner_registry, keep them onAppState, and fail startup on any error. That also stops cloning every provider config and re-parsing URLs on each navigation. - Drop
is_registered_kindfrom this filter so an unknown kind reachesbuildand fails. - Have Axum, Cloudflare and Spin reject an enabled
ec.identity_providersentry at startup instead of ignoring it with no log. - Make the warn branch in
from_ec_with_kindsan error, or remove it.
| let mut claimed: HashSet<SourceDomain> = HashSet::new(); | ||
| for config in configs.into_iter().filter(|config| config.enabled) { |
There was a problem hiding this comment.
🔧 Two providers can share an id, and then clear and read each other's IDs
validate_enabled_configs checks endpoints, partners and source claims, but not the id. That id is the owner on every UID, the key in opt_outs, and part of the rate-limit key (identity:{id}:{hash}). With two enabled entries id = "shared-id" on different sources, provider A's opt-out is remembered under the shared id; on the next navigation both are skipped as OptedOut, and coordinator/mod.rs:356-360 clears both sets of sources:
provider=shared-id status=Skipped(OptedOut) cleared=[SourceDomain("ids.example.com")]
provider=shared-id status=Skipped(OptedOut) cleared=[SourceDomain("other.example.com")]
requests sent=0
They would also share OwnedEntries and refresh_due_for, so B's refresh state (possibly a credential) is handed to A. An empty id is accepted too.
With this suggestion the same two configs are rejected (probe: duplicate ids accepted=true at this head, false with the change). A unit test next to validate_enabled_configs_rejects_endpoint_outside_allowlist would pin it. (Verified alone on this head: rustfmt, all six clippy aliases, test-fastly, test-fastly-reuse, test-axum, test-cloudflare, test-spin and the parity suite.)
| let mut claimed: HashSet<SourceDomain> = HashSet::new(); | |
| for config in configs.into_iter().filter(|config| config.enabled) { | |
| let mut claimed: HashSet<SourceDomain> = HashSet::new(); | |
| let mut ids: HashSet<&str> = HashSet::new(); | |
| for config in configs.into_iter().filter(|config| config.enabled) { | |
| // The id keys UID ownership, opt-outs and the rate-limit bucket in | |
| // the row, so two providers sharing one would read, refresh and clear | |
| // each other's IDs. | |
| if config.id.trim().is_empty() || !ids.insert(config.id.as_str()) { | |
| return Err(Report::new(ProviderConfigError::Invalid).attach(format!( | |
| "provider id `{}` is empty or used by another enabled provider", | |
| config.id | |
| ))); | |
| } |
| fn deserialize_readable_uids<'de, D: Deserializer<'de>>( | ||
| deserializer: D, | ||
| ) -> Result<Vec<KvUid>, D::Error> { | ||
| let raw = Vec::<Box<RawValue>>::deserialize(deserializer)?; | ||
| let mut uids = Vec::with_capacity(raw.len()); | ||
| for value in &raw { | ||
| let oversized = serde_json::from_str::<StoredId<'_>>(value.get()) | ||
| .is_ok_and(|stored| stored.id.len() > MAX_UID_LENGTH); | ||
| if oversized { | ||
| continue; | ||
| } | ||
| uids.push(serde_json::from_str(value.get()).map_err(de::Error::custom)?); | ||
| } | ||
| Ok(uids) | ||
| } |
There was a problem hiding this comment.
🤔 Read tolerance covers only UID length, and a partial skip is silent
deserialize_readable_uids skips an oversized id, but every other new bound still fails the whole row:
mm=6 (a future OpenRTB value): whole row fails
eid ext 1100 bytes: whole row fails
uid ext 1100 bytes: whole row fails
inserter 300 bytes: whole row fails (validate)
uid id 1100 bytes: row loads
The reason for tolerating long UIDs (a later build raises a limit, then a rollback) applies to each of these. Separately, drop_unreadable_ids counts only whole records, so a UID skipped inside a record that keeps another UID logs nothing, and the next unrelated write (a batch upsert for another partner, say) persists the row without it (probe: stored body still has 1100-byte uid = false). Consider lenient deserializers in KvEidRepr and KvUid (an unknown mm, or an oversized ext, inserter or matcher, read as None) and counting skipped UIDs in the log.
| let single = | ||
| (!record.is_provider_written() && !record.uid.is_empty()).then_some(EidView::Single { | ||
| source, | ||
| uid: record.uid.as_str(), | ||
| atype: default_atype, | ||
| }); |
There was a problem hiding this comment.
🤔 A browser record in a provider-owned source is served indefinitely
A Single record written by browser ingest before the provider was enabled, or left by a rollback (see the MAX_UID_LENGTH comment), keeps being served here. A NoMatch lookup does not remove it (the write plan only inserts on Found, and cleared is filled only on OptedOut); browser ingest, batch sync and pull sync can no longer refresh it; and it has no expiry. The source also stays due on every navigation, because due_sources ignores records the provider did not write. Consider removing Single records in a provider's sources on a completed NoMatch, or not serving them for provider-owned sources.
| if eid.source.is_empty() { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🤔 Client EIDs for provider-owned sources still reach bidders after a provider opt-out
When a provider reports an opt-out, the write plan clears its sources and remembers the opt-out for 30 days, but the browser's ts-eids cookie or the /auction body can still carry an ID for that source from Prebid. merge_auction_eids has no registry, so that ID goes into user.ext.eids for every bidder. "An opted-out ID cannot come back from the browser" (description, configuration.md:809-810) holds only for KV. Should client EIDs for a provider-owned source be dropped, at least while opted_out() holds for its owner?
| [ec] | ||
| identity_budget_ms = 200 |
There was a problem hiding this comment.
⛏ This example fails validation as written
It enables a provider (enabled defaults to true) whose source has no [[ec.partners]] entry, so validation rejects it: source 'ids.example.com' of provider 'example-provider' has no [[ec.partners]] entry. With the partner entry below, the same block passes validate_identity_providers (checked with a parse-and-validate probe, and prettier is clean).
| [ec] | |
| identity_budget_ms = 200 | |
| [ec] | |
| identity_budget_ms = 200 | |
| [[ec.partners]] | |
| name = "Example IDs" | |
| source_domain = "ids.example.com" |
| # Optional server-side identity providers, looked up on publisher navigations | ||
| # before the origin fetch. Every enabled provider shares one time budget: | ||
| # identity_budget_ms = 200 # default 200 |
There was a problem hiding this comment.
⛏ Uncommented in order, these budget lines land in the partner table
They sit after the commented [[ec.partners]] block, so uncommenting both gives unknown field 'identity_after_response_budget_ms', expected one of 'name', 'source_domain', …. Moving the two budget lines next to pull_sync_concurrency is the cleaner fix, but those lines are outside this hunk, so this suggestion adds a placement note instead. The provider's sources = ["ids.example.com"] also has no matching partner in the example (partner.example.com).
| # Optional server-side identity providers, looked up on publisher navigations | |
| # before the origin fetch. Every enabled provider shares one time budget: | |
| # identity_budget_ms = 200 # default 200 | |
| # Optional server-side identity providers, looked up on publisher navigations | |
| # before the origin fetch. Set both budgets in [ec] above, next to | |
| # pull_sync_concurrency: placed after an [[ec.partners]] entry they would land | |
| # in that partner's table. Every enabled provider shares one time budget: | |
| # identity_budget_ms = 200 # default 200 |
Summary
atype,ext) and serves them unchanged into OpenRTB and Prebid, so bidders see the same shape the provider returned.This PR registers no provider kinds yet, so none of the provider paths run in production until the vendor PR lands. Follow-up PRs stack on this branch: mirror/pull sync (#1489), then the per-vendor provider implementations (#1488).
Changes
ec/eid_types.rs,ec/kv_types.rs,ec/kv.rs,openrtb.rs,ec/prebid_eids.rsec/write_plan.rs,ec/page_load.rs,ec/finalize.rs,publisher.rsec/provider.rs,ec/coordinator/*,ec/registry.rs,settings.rs,config.rs, newtrusted-server-identity-providerscrate, adapterapp.rs/main.rsec/coordinator/after_response.rs, adaptermain.rsdocs/guide/configuration.md,docs/guide/edge-cookies.md,docs/guide/integrations/prebid.md,trusted-server.example.tomlec.identity_providersconfiguration docs and exampleCollection
collect = "page"(default) runs before the response withinidentity_budget_ms(default 200).collect = "after_response"runs after the response is sent withinidentity_after_response_budget_ms(default 2000).tcf_vendor_id, when set, is checked only where GDPR applies or a TCF string is present. A KV read failure skips every provider.opt_outsfield on the row, and the provider is not called again until it lapses. Older builds ignore the field.rate_limit_per_houris per EC. On Fastly it is enforced per minute with a minimum of one, so values below 60 behave like 60.Storage and writes
ts-eidscookie, so an opted-out ID cannot come back from the browser.Behaviour changes from
mainSec-Fetch-Dest: documentandSec-Fetch-Mode: navigate, the same check that permits its row write. Clients that send no Fetch Metadata no longer get a new EC./auctionrequest body are never persisted.Cleanup
ProviderResponsedebug output hides the body and any request context.Closes
Part of #1246 and #1245; the provider implementations that close them come in follow-up PRs.
Spec
Spec: reviewed separately; not included in this PR.
Test plan
cargo test-fastly && cargo test-fastly-reuse && cargo test-axum && cargo test-cloudflare && cargo test-spinclippy-fastly,-axum,-cloudflare,-cloudflare-wasm,-spin-native,-spin-wasm,-cli,-codegen)cargo fmt --all -- --checkscripts/lint-wasm-clock.shcd crates/trusted-server-js/lib && npx vitest run(plusnpm run formatandnpm run lint)./scripts/test-cli.sh, domain lint againstorigin/main, CI Node pin guard, docs-proposal scriptsfastly compute serveChecklist
unwrap()in production code — useexpect("should ...")logmacros (notprintln!)