Repository navigation
Add a pluggable Edge Cookie module seam with the built-in HMAC module - #1043
jwrosewell wants to merge 61 commits into
Conversation
312a4fc to
73b40b9
Compare
83e551d to
e278981
Compare
e278981 to
4529151
Compare
The five-PR series (IABTechLab#1043 to IABTechLab#1047) opens the identity, device and geo seams. The nine vendor integrations already in core sit behind the integration registry instead, which is a private table, so none of them can move out until that table is opened. This spec defines the one core change that opens it: public registration builders with a second input on IntegrationRegistry, browser JavaScript carried on the registration, startup validation as a hook, the same treatment for auction providers and the bid renderer contract, and neutral replacements for the two places where a vendor reaches into core. It then sets out the migration of all nine existing integrations, one PR each. The change is complete in itself: after it, no vendor move needs a core change. Written against the series' tree with the file and line references for every claim about the current code. Documentation only.
aram356
left a comment
There was a problem hiding this comment.
Summary
This PR lands the Edge Cookie provider seam with the built-in HMAC provider, per the pluggable-providers design spec carried in the same change. The lifecycle contract (mint, recognition, KV keying), the global identifier bounds, startup validation, the deprecated-passphrase migration, and the partner-path envelope fix are substantially implemented, with strong test coverage, and CI is fully green.
The major blocker is architectural: vendor extensibility should lean on the existing integration system rather than introduce a parallel "provider" mechanism. The codebase has one established home for vendor code (the integration registry), and this PR adds a second seam, a second config namespace, and a second nomenclature for what a vendor ships. We want that resolved at spec level before PRs 2-5 of the series build on the current shape - see the first cross-cutting finding below.
Beyond that, changes are requested on: a reproduced bypass of the advertised 32-byte passphrase minimum on the deprecated configuration form, two points where the implementation does not do what the spec states (unknown-key rejection in the hmac block; canonical-key routing on identity-graph reads and withdrawals), and an egress guarantee the proxy paths do not honor.
4 of the inline comments below carry a one-click GitHub
suggestion. Use Commit suggestion (or Add suggestion to batch) to apply them as commits on the PR branch. The remaining comments describe the fix in prose because the change spans multiple files or non-contiguous lines and cannot be auto-applied.
Blocking
🔧 wrench
- Vendor identity should lean on the integration system, not a second extension mechanism - the major blocker; cross-cutting, below
- Legacy
[ec] passphrasebypasses the new 32-byte minimum - see inline atcrates/trusted-server-core/src/settings.rs:658(suggestion) [ec.providers.hmac]silently accepts unknown keys - see inline atcrates/trusted-server-core/src/settings.rs:726(suggestion)- Identity-graph reads and withdrawal tombstones bypass the provider's canonical key - cross-cutting, below
❓ question
- Spec says an unrecognized cookie value is "never used or egressed", but the proxy forwarding paths egress it - cross-cutting, below
Non-blocking
♻️ refactor / 🤔 thinking / ⛏ nitpick / 📌 out of scope
- ♻️
build_providersilently returnsOk(None)forprovider = "hmac"with no block - see inline atcrates/trusted-server-core/src/ec/provider.rs:304(suggestion) - ⛏ 22-space run inside the mint-rejection error message - see inline at
crates/trusted-server-core/src/ec/mod.rs:444(suggestion) - ♻️
EdgeCookieProvider's doc comment is fused intoProviderCode's, leaving the trait undocumented - see inline atcrates/trusted-server-core/src/ec/provider.rs:177 - ⛏
ec::get_ec_idis dead code, yet was modified to accept any provider code - see inline atcrates/trusted-server-core/src/ec/mod.rs:137 - ⛏ Module docs describe constructor injection that is not how
RequestInfoflows - see inline atcrates/trusted-server-core/src/ec/provider.rs:4 - 🤔 Cluster prefix listing splits across the envelope migration - cross-cutting, below
- ♻️ Magic strings
"hmac"/"none"scattered across four call sites - cross-cutting, below - 🤔
RequestInfoaccessors have no production consumer in this PR - cross-cutting, below - 🤔 Spec revision followed the implementation - cross-cutting, below
- 📌 Operator guides still document
[ec] passphraseas the current form - cross-cutting, below
Cross-cutting / body-level findings
-
🔧 Vendor identity should lean on the integration system, not a second extension mechanism (the major blocker). The codebase already has one home for vendor code: the integration registry (
IntegrationRegistration::builder(ID).with_proxy().with_head_injector()...), capability-based and config-namespaced under[integrations.<id>]. This PR adds a second vendor seam -RuntimeServices::ec_provider, a single-slotOption<Arc<dyn EdgeCookieProvider>>matched byid(), configured under[ec.providers.<key>]- and a second nomenclature ("providers").RuntimeServicesis otherwise the platform composition surface (KV store, geo, HTTP client, client info: things the host supplies); a vendor identity module is not a host capability, and a vendor realistically ships a JS integration and an identity function together, which this split forces into two mechanisms. Please rework the vendor seam onto the integration system: identity provision as a registration capability (for example.with_ec_provider(...)), with[ec] provider = "<integration id>"still supplying the select-exactly-one semantics; the built-in HMAC provider can stay hard-wired in core as the default, and geo/device rightly remain platform services. If there is a reason this cannot work, the spec should defend the separate provider mechanism against this alternative explicitly - and we want that settled at spec level before PRs 2-5 of the series build on the current shape. -
🔧 Identity-graph reads and withdrawal tombstones bypass the provider's canonical key. The spec's lifecycle table (section 3) routes identity-graph row reads and writes through
normalize_id_for_kv. Mint honors that:EcContext::generate_with_providerkeys the row withprovider_kv_key(ec/mod.rs:476). Buthandle_identifyreads with the raw cookie value (kv.get(ec_id),ec/identify.rs:89), withdrawal tombstones are written under the raw value (ec/finalize.rs, thewrite_withdrawal_tombstoneloop), and EID ingestion keys by the raw value. For the built-in HMAC provider raw and canonical coincide, so nothing misbehaves today; for the first provider whose canonical form differs from the cookie value (exactly theCanonicalizingProvidercase this PR's own test proves at mint), identify misses the row written at mint, and a withdrawal tombstone lands on a key no live row uses, so the revocation never takes effect. Proposed fix: compute the canonical key once inEcContext(for example anec_kv_key()accessor derived from the selected provider) and use it in identify, the finalize tombstones, and EID ingestion - or amend the spec to state that reads and withdrawals become canonical-form-routed only when the first canonicalizing provider ships, and track that as a follow-up. -
❓ Spec says an unrecognized cookie value is "never used or egressed", but the proxy forwarding paths egress it. Section 3's Recognize row states that a value the selected provider does not recognize "is never used or egressed."
append_ec_id(proxy.rs:1263) andhandle_first_party_click(proxy.rs:1609) forward the rawts-eccookie /x-ts-echeader value to origin and click-target URLs throughedge_cookie::get_ec_id, which checks only the character/length allowlist - so a foreign-coded value (zz00~...), or any cookie in a stateless (no-provider) deployment, is egressed on those paths. The looseness predates this PR, but the PR introduces the spec claim. Which should change - the spec (scope the guarantee to the EC lifecycle paths and note the proxy forwarding exception) or the code (route those call sites through provider ownership)? -
🤔 Cluster prefix listing splits across the envelope migration. Section 3 says the pre-epic IP-cluster prefix listing "continues unchanged." Fresh mints are now keyed
hmac~<hash>.<suffix>, soevaluate_cluster's prefix (ec_hash,ec/kv.rs:715) becomeshmac~<hash>for coded rows while legacy rows still list under the bare<hash>. Two rows for the same client IP that straddle the envelope migration therefore never count each other, andcluster_size(a NAT/fraud signal in identify responses) undercounts while both populations coexist. Worth a sentence in the spec, and possibly a follow-up to bridge the count during the migration window. -
♻️ Magic strings
"hmac"/"none"are scattered across four call sites (Ec::validate_provider_selection,build_provider,provider_owns_id'sprovider.id() == "hmac", andHMAC_PROVIDER_CODEinec/generation.rs). A typed selector, for exampleenum EcProviderSelection { None, Hmac, Vendor(String) }with a custom deserializer (vendor keys are open-ended, so a catch-all variant is needed), would centralize the vocabulary before #1044 adds more built-ins. Non-blocking: the string form works and is startup-validated. -
🤔
RequestInfoaccessors have no production consumer in this PR.path(),query(),query_param(),header_names(),user_agent(), andheader()are supplied by production code but consumed only by tests in this PR (the HMAC provider reads onlyclient_ip()). The spec's own minimalism rule (section 4) requires a production caller in the same change that introduces a method; the consumers arrive later in the stack. For a stacked series this can be acceptable, but the spec should say which PR consumes each accessor, or the accessors should land with their consumers. -
🤔 Spec revision followed the implementation. The spec is commendably candid that it is the 2026-07-31 draft "revised against the implementation" with a revision-record table, and that table is genuinely useful. The process consequence is worth naming, though: when the normative spec is restated to match landed code, divergences become ratifications rather than decisions, and questions like the extension-model one above surface at review time instead of design time. For the remaining PRs in the series, it would serve the spec-first intent better to land spec changes ahead of the implementing PR and let review happen against the spec before the code exists.
-
📌 Operator guides still document
[ec] passphraseas the current form.docs/guide/configuration.md:1933,docs/guide/key-rotation.md:31,docs/guide/error-reference.md:72, plusec-setup-guide.md/edge-cookies.md/fastly.mdpredate the provider layout, the deprecation, and the new stateless default intrusted-server.example.toml. A docs pass is needed in this series; a follow-up PR is fine.
CI Status
- cargo fmt: PASS (required)
- cargo test: PASS (required)
- format-docs: PASS (required)
- format-typescript: PASS (required)
- cargo test (axum native): PASS
- cargo test (cloudflare native + wasm32-unknown-unknown check/build): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- cargo test (cross-adapter parity): PASS
- cargo test (ts CLI, native): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- vitest: PASS
- Analyze (rust): PASS
- Analyze (javascript-typescript): PASS
- Analyze (actions): PASS
- CodeQL: PASS
- prepare integration artifacts: PASS
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- browser integration tests: PASS
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Reviewed the pluggable Edge Cookie provider changes at 0f5c063214ba1d46478311851f08fe9b10c2ccf8. I am requesting changes based on the inline findings. This review includes one P1, three P2s, and one non-blocking migration clarification. cargo test-fastly and all 18 GitHub checks passed at the reviewed head; these findings concern runtime and provider-contract behavior rather than test failures.
0f5c063 to
11cc575
Compare
The five-PR series (IABTechLab#1043 to IABTechLab#1047) opens the identity, device and geo seams. The nine vendor integrations already in core sit behind the integration registry instead, which is a private table, so none of them can move out until that table is opened. This spec defines the one core change that opens it: public registration builders with a second input on IntegrationRegistry, browser JavaScript carried on the registration, startup validation as a hook, the same treatment for auction providers and the bid renderer contract, and neutral replacements for the two places where a vendor reaches into core. It then sets out the migration of all nine existing integrations, one PR each. The change is complete in itself: after it, no vendor move needs a core change. Written against the series' tree with the file and line references for every claim about the current code. Documentation only.
The review of IABTechLab#1043 asked that spec changes land before the code that implements them, so a divergence is a decision taken in review rather than a ratification of something already merged. PRs IABTechLab#1043 to IABTechLab#1047 each carried the design document for their own step, and IABTechLab#1043 carried a 607-line spec describing device providers, geo providers, the permission model and the browser resolve endpoint, none of which is in that PR. Move all six series documents here, so this PR carries the complete normative set and no code: - 2026-07-30-pluggable-providers-design.md (from IABTechLab#1043) - provider-code-registry.md (from IABTechLab#1043) - 2026-07-30-permission-model-design.md (from IABTechLab#1045) - 2026-07-30-client-cycle-ec-resolve-design.md (from IABTechLab#1046, later revised by IABTechLab#1047) - 2026-07-30-integration-response-header-hook-design.md (from IABTechLab#1047) - 2026-07-30-provider-migration-rollout-design.md (from IABTechLab#1047) Each file is taken verbatim at the tip of the stack, so the later revisions are preserved: the provider-switching continuity section, the geo requires-signal floor, and the code-envelope paragraph IABTechLab#1047 added to the client-cycle spec. The revision-record tables are unchanged. No document's substance was edited. The only edits are to this spec's own status line, which said the PR adds one document and that the series specs land with IABTechLab#1047, and a revision-record row recording the move.
|
This response was drafted with AI assistance and checked against the branches before posting. Thank you both. Twenty observations across the two reviews. Seventeen are answered in code on this branch and three are answered in the pull request of the chain where the answer belongs, named in the Addressed elsewhere table. Each fix is separately committed, so any one can be confirmed without reading a combined diff. Two further rows in the Addressed table are not yours, being things we found while answering and fixed in the same pass. A note on scope. Some of your observations reach past this PR into the ones before and after it, which is unavoidable because the work was split into a chain. Answering only within #1043 would be more confusing, not less, so this comment answers for the whole chain and says where each answer lives. #1043 is simply the PR the review happened on. Each commit's message ends with an The branch is rebased onto We also run the core library suite natively, at 2,463 tests, and #1047 adds that run to One CI note, and a small ask. CodeQL flags "Cleartext logging of sensitive information" on #1044 to #1047 and #1094. It is a false positive and we would ask you to dismiss it, since the alerts belong to this repository and we cannot. The passphrase it traces is held in a Where each piece is, and what changed between the PRsTwo things moved since Aram's review on 27 August that are not visible from this PR alone. The seam that the architectural finding asks this work to lean on now These are one block, and the order below is the order they should merge in. Splitting them is what creates the legacy this work exists to stop, because each one on its own leaves the core carrying a shape the next one removes. The last item is the point of the whole exercise, an unmerged vendor change landing without adding to the core, so no further legacy is added rather than removed later.
Rowena asked on 27 August whether #1044 must follow #1043, or whether #1045 could follow #1043 instead. The answer is that #1045 cannot move ahead of #1044, and here is the reason rather than the assertion. The permission model needs a jurisdiction baseline, which is the country whose rules apply when the geo lookup returns nothing. That baseline lives on the The rest of the order is the same kind of dependency rather than preference. #1046 is the browser-set path for an identity #1043 defines, and #1047 documents behavior the four before it introduce, so documenting it earlier would describe code that is not there. If a different order would help you, tell us what you need and we will say honestly whether it can be done, because we would rather rework the split than have the whole thing wait on the shape we happened to choose. Items 2 to 7 are one ordered chain, not six branches beside each other. #1043 is against The order we suggest is #1084 first, since it settles the design question and costs nothing, then #1043 to #1047 in sequence, then the implementation of #1084. That implementation is where identity, geo and device all become capabilities a registration declares, which is the architectural finding answered rather than deferred. It lands there and not here because a registration can only carry an Edge Cookie provider once that trait exists, and #1043 is what adds it, so the seam PR is the first point in the chain where both exist together. We would rather do it once, against a seam that exists, than rewrite five reviewed PRs onto a seam that did not exist when the review was written. Addressed
Addressed elsewhere in the chainEach is answered in the PR of the chain where the answer belongs rather than in this one.
Behavior changesFour, each called out deliberately rather than left to be found. Every one of them is necessary rather than incidental, and every one moves in the direction this project has already chosen, which is a core that is neutral between vendors and does nothing on a deployment's behalf that the deployment did not ask for. The last is the one an operator will feel most, so it is worth reading even if the rest are skimmed.
How providers see the requestApplying the minimalism rule to the evidence interface was the wrong call, and we are reversing it. Here is the design we are implementing instead, so the reasoning is on the record rather than arriving as a surprise in a later PR. A provider is given everything the request carries. The client IP, the User Restricting what a provider can see is the wrong lever. The right one is
That guards against a badly behaved provider twice over, without the interface deciding in advance what a vendor is allowed to look at. A permission describes what, not how, and that is the whole reason this boundary is the right one. A permission names a data use, being storage on the device, or personalized marketing. It never names a technology. There is no permission saying the User-Agent header may be read, or that a cookie may be used but local storage may not. Data protection works the same way round, because it governs the purpose data is put to rather than the mechanism used to achieve it. So restricting what a provider sees regulates the how, not the what. A provider blocked from one header can often reach the same purpose another way, and one allowed to see a header still may not use it for a purpose nobody granted. What stops the purpose is not running the provider at all, which is the first layer above. Drawing the boundary on the purpose rather than the mechanism also buys something we would like to build on. Every provider already declares the permissions its data use requires, so a build can be asked what it will do before it serves a single request. The core can emit a manifest for a given deployment listing every permission every module in it requires, derived from the modules themselves rather than from someone's notes. That is a machine-readable statement of what a deployment does with data, which is most of the work of writing a privacy notice, and it can be generated and kept current rather than maintained by hand and quietly going stale 🙂 And the claim can be checked, which is what makes it useful. A provider declaring the permissions its data use requires is, on its own, only a claim. What turns a claim into something a publisher can rely on is that the code is open, so anyone can read what a module actually does and hold it against what the module said it would do. That is a large part of what the word trusted in Trusted Server has to mean, because a trust nobody can verify is only a reputation. Checking used to be expensive enough that almost nobody did it. That has changed. An AI agent can read a module, read its declared permissions and report the difference in minutes, for very little, and can do it again on every release rather than once at onboarding. So a false declaration, or a module quietly doing more than it declared, moves from something findable in principle to something that will be found in practice. The consequence should follow the finding, and it should be plain. A vendor whose modules repeatedly do not do what they say should not have modules in this project, and should not remain a member of the organization that publishes it. Simple. That is the enforcement this model needs behind it, and it is available only because the code is open and the declarations are machine-readable. The caller is us, and it is the next step rather than part of this stack. We will use all of it, to the extent permissions allow, across the geo, device and Edge Cookie providers. We are deliberately not raising that pull request alongside these, because this stack is already a large change and a vendor module on top would make it harder to review. What that work needs is specific rather than speculative. The evidence interface #1043 carries already exposes the client IP, the User-Agent, headers read by name, header enumeration so a module sends a complete evidence set rather than working from an allowlist compiled into it, the path, and the query and its parameters, and it is whole on #1043 as of So the evidence interface is whole on #1043 as of We will prove the evidence actually arrives. A loopback provider that Two notes on sequencing. The advertise-and-gate half needs The gap this leaves, which we would like to fillThere is no conformance suite a provider can be run through. Core defends Found in
|
| The change | What it gives a publisher |
|---|---|
| A core that is neutral between vendors | No vendor's code sits inside the core everyone depends on, so no vendor's interests are built into it |
| Vendor modules owned and maintained by the vendor, with a maintainer recorded | You can see who stands behind the code carrying a vendor's name, and hold them to it |
| Permissions expressed as what data is used for, not which technology is allowed | The rule survives the next technology, because it never named one. It is also the way data protection law is written |
| A permissions manifest for a build | A deployment can state what it will do with data before it serves a single request, which is most of a privacy notice, generated from the modules rather than written by hand |
| The same evidence available to every vendor | Nobody gets a better view of the request than anybody else, so vendors compete on what they do rather than on access |
| Declarations that are machine-readable, in code that is open | A claim can be checked against actual behavior cheaply, by anyone, on every release rather than once at onboarding |
| A conformance suite any provider can be run through | A vendor can show their module behaves before shipping it, and the project can show it too |
| Startup that fails rather than falls back quietly | A misconfigured deployment stops instead of doing something nobody asked for and nobody notices |
Those are reasons for the wider ecosystem to engage with Trusted Server, not just reasons for us to like it. We would much rather arrive in New York with them shipped and running than describe them as a plan.
|
Correcting my earlier comment: I overstated how often this bitesIn my comment above I wrote that two identifiers for the same device produce The Edge Cookie is reused when the request already carries one. The generation So the consequence is that identity does not survive a reissue, rather than What I still think is worth doing here is unchanged, and it is the part
A sentence in the trait documentation saying the returned value is the identity Apologies for the noise of a correction. Better than leaving the first version |
Brings in the eight commits main gained since this branch was cut, of which three touch the same code as the provider seam. The parser-aware body hold gives every adapter a second entry point, `build_state_with_services`, so a caller can supply the `RuntimeServices` each request uses. The composition-root check the seam added now runs in that function rather than in `build_state_with_settings`, and it is given whatever provider those supplied services already carry instead of always `None`, so a caller that resolved one is not made to resolve it twice. On Cloudflare and Spin the state keeps both the provider this branch resolves once at start-up and the services main lets a caller supply. The free function this branch added is gone and its one job, handing the resolved provider to every request, moved into `services_for_request`, which is the method main introduced for the same purpose. The core README this branch corrected one line of has been rewritten wholesale by the documentation refresh, and the section that line was in no longer exists, so main's version is taken as it stands.
The per-request Edge Cookie test builds AppState by hand, and the merge left it without the field the supplied-services path added. It drives the per-request path, so it supplies none.
Takes the identity graph changes merged upstream on 28 September (conditional writes, tombstones from a snapshot, grouped batch sync, the pull sync marker and the EID sync source) and the reusable sandbox in the Fastly entry point, and keeps the provider seam on top of them. Conflicts, settled file by file: - crates/trusted-server-adapter-fastly/src/app.rs: upstream's sandbox lifecycle. The seam's settings_with_missing_consent_store test helper had no caller after the merge and is dropped. - crates/trusted-server-core/src/ec/batch_sync.rs: mappings are validated inside upstream's grouping loop, keyed by the owning provider's canonical form through accepted_providers.canonical_kv_key. The seam's three provider tests stay, followed by upstream's renamed fan-out test. - crates/trusted-server-core/src/ec/pull_sync.rs: build_pull_sync_context keeps the provider dispatch for the identifier and its key, then applies upstream's gate (pull-enabled partners, a consented row still missing a partner UID). PullSyncContext carries the key, and dispatch returns early with no pull partners. - crates/trusted-server-core/src/ec/finalize.rs: provider response headers are applied first, then the marker is validated and the gate read. The returning-user, generated and recovery paths key the snapshot, EID ingestion and sync by the provider's canonical key, and withdrawal tombstones each provider-derived key through tombstone_existing_from_snapshot, warning on a failed tombstone. Upstream's new code and tests assumed ec.passphrase and helpers that take no provider, which is not this branch's shape: - The pull sync marker key derives from the selected provider's HMAC passphrase, or from the proxy secret for a provider that has none, since ec.passphrase is only the deprecated location here. - Upstream's tests pass the accepted providers to process_mappings, the provider to handle_batch_sync_with_writer, the gate flag to the finalize context helpers, and a registry with a pull partner plus a seeded snapshot to build_pull_sync_context. Tests that set ec.passphrase select the hmac block instead, and the redaction canary reads ec.hmac.passphrase. The seam's EID ingestion test opens upstream's new EID sync source gate, as upstream's own returning-user tests do.
EcContext captured the request headers, path and query only when no usable identifier arrived, while orphan recovery runs only when one did. So a provider asked for a replacement identifier during recovery saw a request with no headers, although the docs said recovery reads the evidence captured at read time. A provider that derives identity from client hints then ran with no client hints on every recovery attempt. The snapshot is now also captured for a document navigation, the only request that can recover, so a returning visitor's subresource requests still clone nothing. The field doc says when it is captured. Test recovery_on_a_navigation_passes_request_parameters_and_cookies_to_the_provider fails without the change.
a_provider_reads_request_cookies_from_the_request_info built its own OwnedRequestInfo with a Cookie header and called a test double's generate with it, so it tested OwnedRequestInfo::header and nothing in core. The test and its CookieCapturingProvider double are gone, and the tests module no longer imports OwnedRequestInfo. The recovery test added in the previous commit covers a request's cookies reaching a provider through the request path.
remove_labeled_provider_secret_errors strips the error that judged a labeled block's secret key name as a passphrase. Nothing checked that it strips only that one, so it could drop the Edge Cookie section's other errors and no test would fail. push_validation_keeps_the_sections_other_errors_for_a_labeled_block selects [ec] provider = "primary" with a primary block for hmac holding the key name ec_key, adds a partner whose source domain carries a scheme, and expects the section's errors to hold the partner error and no passphrase error. They are read from the section rather than from the message, because deploy validation reports the same bad partner.
Comments and docs say what the code does. History and plans come out of them: the legacy reader retirement plan on provider_owns_id, the plan that the built-in provider becomes a module, the reader list seam on AcceptedProviders, the design-document references, the pull request narrative around the test doubles, and the "still built into core" wording. The header-appending rule on apply_provider_response_headers is stated in three sentences, the resolution docs say what resolves where, and edge_cookie.rs says it reads an inbound identifier and that its generation helpers are test-only. The ec module summary lists admin, finalize and provider. Tests that could not fail now can, and repeated tests are folded into tables. One OpaqueProvider double in ec::tests serves the admin lookup, batch sync and pull sync tests in place of four copies. The batch sync canonicalization test asserts the key the writer was given. The request path reuse test drives a built-in selection through an unthreaded services value, since the threaded helper already threads the provider. The uninjected provider test covers build_provider and the startup check together. The reserved response effect tests are one table over each header with and without an identifier, and the three discarded-candidate tests are one table. The withdrawal key cases and the HMAC grammar cases are tables. The consent gate test has an open-gate control and a backed context, so only the gate can withhold the cookie. New tests: a provider that creates nothing still has its headers applied; an orphan recovery that cannot rotate leaves a failed snapshot and no cookie, for a provider with no client IP, a store whose write fails and a replacement that always collides; an orphan rotation applies the replacement provider's response headers; both RequestInfo views list their header names; and the request path hashes an IPv6 client by its /64 prefix, which replaces the edge_cookie test that compared two test helpers. Tests say create where they said mint. noop_services_with_resolved_ec_provider duplicated noop_services_with_ec_provider and is gone.
What an operator selects to supply a capability is a module, and provider keeps the one meaning it has on main, an auction provider instance. This renames the Edge Cookie identity seam this series adds, and only the words it adds: every provider word was compared with upstream main, and a word that exists there, in the auction code or the EID extensions, is untouched. The retired [ec.providers] table keeps its literal in the refusal that names its replacement. - ec/provider.rs is ec/module.rs, so ec::provider is ec::module. - EdgeCookieProvider is EdgeCookieModule and HmacProvider is HmacModule. - EcProviderSelection, EcProviderBlock, EcProviderBlocks and EcProviderSettings are EcModuleSelection, EcModuleBlock, EcModuleBlocks and EcModuleSettings, HmacProviderConfig is HmacModuleConfig, and Ec's provider and provider_blocks fields are module and module_blocks, so the selector reads [ec] module and the environment override is TRUSTED_SERVER__EC__MODULE. - ProviderCode and the provider_code! macro are ModuleCode and module_code!, and HMAC_PROVIDER_KEY, BUILTIN_PROVIDER_KEYS, HMAC_PROVIDER_CODE, PROVIDER_CODE_SEPARATOR and PROVIDER_IMPLEMENTATION_KEY say MODULE. - build_provider, build_shared_provider, request_provider, provider_owns_id, apply_provider_code, provider_kv_key, apply_provider_response_headers, ensure_provider_available, split_provider_code and resolve_named_provider say module, as do AcceptedProviders and accepted_providers, selected_provider, validate_provider_selection and validate_provider_name, and remove_labeled_provider_secret_errors. - RuntimeServices' resolved_ec_provider and with_resolved_ec_provider, on the services and their builder, say module. - Test helpers, test doubles and test names in the Edge Cookie code say module. - The plain word, where it means an Edge Cookie module, in the Edge Cookie code, settings.rs, config.rs, config_payload.rs, test_support.rs, proxy.rs, testlight.rs, the adapters' entry points, the example settings, the integration fixture, the API reference and the edgecookie crate directory's README.
A module's name is the path of its crate below `crates/`, with `.` between the parts, taken from `CARGO_MANIFEST_DIR` when the crate is built, so the name cannot drift from the folder and no crate carries a hand-written id. `module_name!()` gives a crate its own name, `module_name::resolve` finds the name an operator wrote among the modules offered, as written or with the section's type folder in front, and `module_name::short_form` gives the name back without that folder. Core's own modules keep bare names. The Edge Cookie side is the first to use it. The type folder of every Edge Cookie crate is `edgecookie`, so `[ec] module` may name an injected module in full or with that folder left off, and an `implementation` line may do the same. The rule for a name written in `[ec]`, its blocks and their `implementation` lines is the module name rule, parts joined by `.`, each of lower case letters, digits, `_` or `-`, in place of snake_case.
Comments on the adapters and two test notes in core described what the code did before it changed. Each now says what the code does and why. - The resolved Edge Cookie module held on the Cloudflare, Fastly and Spin application state is resolved when the state is built, because resolving reads no request data. - The three tests that a selected module the adapter cannot build fails the request say what a default context would do. - The Spin state builder reads its settings from the config store, and its test says why the failure has to be the absence of a config store. - The test that keeps the internal header list in step with the Edge Cookie response headers, and the test for an unknown key in a module block, say what they guard. No code changes.
Core builds the identity-graph key from the module's code and whatever `normalize_id_for_kv` returns, so the method decides what two visits must have in common to be treated as the same visitor. Its documentation described normalization only and advised a module with a case-sensitive identifier to return the value unchanged. That is right for a stable identifier and wrong for one that carries a signature, a nonce or a timestamp, because each reissue then gets a row of its own and the identity does not survive it. The trait documentation now says that the returned value is the identity two visits must share, and that a module whose identifier has a part reissued each time must return the stable part. It also says that core asks `accepts_id` about the returned string as well as about the identifier as issued, so a module must accept its own canonical form, or no row is read or written for it. `two_issues_of_one_identity_share_one_identity_graph_key` drives a fixture module whose identifier is a stable part and a part that changes on each issue through `AcceptedModules::canonical_kv_key`. Two identifiers that differ only in the reissued part reach one key, and a different stable part reaches another. The test fails for a module that returns the value unchanged. No behavior changes.
…ords `KvNetwork` said a low cluster count "indicates an individual or household". A count of connections is a fact about a network, and reading a low one as an individual describes a device record as a record about a person. It now says a small network, such as a home connection. `KvEntry::ids` now says what it holds, which is each partner's identifier for the same browser on the same device, never an identifier for a person. Comments only, with no change in behavior.
# Conflicts: # crates/trusted-server-core/src/auction/endpoints.rs # crates/trusted-server-core/src/ec/finalize.rs # crates/trusted-server-core/src/test_support.rs
|
Moved into this repository as #1343, one branch of the stack, so the review continues there. This pull request is closed in its favor and its comments stay here for the record. |
This pull request is the first of six stacked pull requests and the only one
with no parent, so it depends on nothing beyond
mainand can merge on itsown. The stack is #1043, #1044, #1045, #1046, #1047 and #1094, each targeting
mainand each branch building on the one before. The first five decompose #838as requested in the #986 review, and #1094 opens the integration seam on top of
them. A later pull request's own change shows when its head branch is
compared with the previous pull request's head branch.
The design specs for the series are carried by the spec pull request (#1084).
The spec for this pull request is
2026-07-30-pluggable-providers-design.md,which covers both this pull request (the Edge Cookie seam) and the device and
location seam (#1044). The spec is revised to match this implementation, with a
revision record listing every divergence from the earlier draft and why.
What this pull request does
Edge Cookie identity generation becomes a selectable module behind the
EdgeCookieModuletrait incrates/trusted-server-core/src/ec/module.rs,with the existing HMAC implementation as the built-in and configuration
selecting it.
How a module is selected
[ec]follows the convention the rest of the stack follows:[ec] modulenames one module, and[ec.<name>]holds that module'ssettings. Leaving
[ec] moduleout means stateless operation with no EdgeCookie, and
module = "none"spells the same choice explicitly.carry
implementation = "<id>"and take a name of its own, which is whatlets a deployment label a module for what it does rather than for what it
is.
by
., each of lower case letters, digits,_or-, and anything elseis refused at startup with the rule in the message.
key a module does not know all fail at startup, so a misconfiguration is
loud rather than silent.
[ec]keeps its own settings besidemodule,being the store name, the partner registry and the cluster thresholds, and
those are not mistaken for module tables.
[ec] passphraseform still starts. It migrates tomodule = "hmac"plus[ec.hmac]with a deprecation warning, so a fleetcan move configuration and binaries independently. Both forms together are
rejected. In either form the passphrase names a secret-store key, as
[ec] passphrasedoes onmain, and the resolved value is held to the same32-byte minimum.
The identifier and the identity graph
and the alphabet
[A-Za-z0-9._~-], enforced at creation, cookie read-backand cookie write. A violating identifier is rejected outright and never
rewritten, so the cookie value and the identity-graph key can never silently
diverge. The previous sanitize-by-stripping path is removed.
accepts_id, and theidentity-graph key through its
normalize_id_for_kvcanonical form, so anopaque vendor identifier round-trips byte for byte. One test proves a
non-default module round-trips verbatim, another proves the graph is
keyed by the canonical form, and a third proves that two issues of one
identity share one key when the module returns the stable part.
[ec.<name>]tables are captured as raw values in core anddeserialized by the adapter that injects the vendor module, so core never
names a vendor.
in
provider-code-registry.md,the registry the spec set defines. Core creates
{code}~value, checks thecode at read-back, and keys the identity graph with it, so identifiers from
different modules can never collide and switching modules cannot silently
adopt another module's identities. The built-in module creates
hmac~<hash>.<suffix>and still reads its earlier bare form so deployedcookies keep working.
module's canonical form. Pull sync, batch sync and the admin lookup accept
an identifier through
AcceptedModules, which applies the global boundsand then asks the module that owns the identifier's code, so a code no
configured module owns is refused. Identify, Edge Cookie finalization, pull
sync, batch sync, the admin lookup,
/auction, the publisher navigationpreload and
/_ts/page-bidsall use the canonical key, so a module whosecanonical form differs from its cookie value still reaches the row it
created. The admin lookup reports the key it read as
kv_key.is_valid_ec_idstays the built-in HMAC grammar, accepting thehmac~envelope and the legacy bare form.
main's identity-graph flow, EID ingestion, orphan recovery and thewithdrawal tombstones are keyed the same way, so an ingested EID lands on
the live row and a withdrawal tombstones the row the identifier is stored
under.
What a module may do to the response
inside its own surface, being a
Set-Cookiefor ats-cookie, anyx-ts-header, and the framing, hop-by-hop and
cache-controlheaders. A refusedeffect makes generation return an error, and the page is served without an
Edge Cookie. Headers are kept only from a module response whose identifier
generation commits, or that produced no identifier, and finalization appends
them rather than replacing the origin's.
empty string when the host has none, and each module decides whether it
needs one. The built-in HMAC module refuses to create an identifier without
one.
the request's headers, path and query, the evidence it reads at creation.
How it was verified
The head this pull request shows now is
d33507b32, which mergesmainat 182fdf4 into6f527710a. The merge brings thets dev lint domainsandts dev install-hookscommands (#733) and a design document (#930), and changes none of this pull request's own code. No dependency of core or of an adapter changed version, and the one line of core the merge touches gains a comment. Ond33507b32these ran locally on Windows:cargo fmt --all --check, a locked dependency check (cargo tree --locked) and the core suite natively (3,003 tests).6f527710aadds three commits to6b2e303f1, one of comment wording, one that documentsnormalize_id_for_kvand adds a test, and one that words the identitygraph's comments as device records. On
6f527710athese ran locally onWindows:
cargo fmt --all --check, Clippy with warnings denied on all fouradapters, the core suite natively (3,003 tests), and the docs lint,
Prettier and VitePress build. On
6b2e303f1the wider set ran locally,being the core suite under Viceroy (3,239 across the crate's test binaries),
the Axum (45 tests), Cloudflare (55) and Spin (90) adapter suites and the
cross-adapter parity suite (17), and CI runs all of them on this head. The
CLI tests, the browser integration tests and the Fastly Edge Cookie
lifecycle test run only in CI.
CI on
d33507b32: every check passes, being Run Tests, Run Format, Integration Tests and CodeQL Advanced, with the Fastly Edge Cookie lifecycle test in the integration run.Framing
Privacy is a spectrum, and this change is neutral infrastructure. It does not
decide whether identity is created. It makes that decision configurable and
inspectable, and the deployer selects a module, or none, according to the
laws and policies that apply to them. Trust comes from that flexibility being
respected and visible in configuration rather than hard-coded.
References #777 and #778. Decomposes #838, which is kept as a draft reference until this
series merges. Spec baseline from #986.