Skip to content

Add a pluggable Edge Cookie provider seam with the built-in HMAC provider - #1043

Open
jwrosewell wants to merge 36 commits into
IABTechLab:mainfrom
jwrosewell:split/1-ec-provider
Open

Add a pluggable Edge Cookie provider seam with the built-in HMAC provider#1043
jwrosewell wants to merge 36 commits into
IABTechLab:mainfrom
jwrosewell:split/1-ec-provider

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

First of five stacked PRs decomposing #838 as requested in the #986 review, where each PR carries one feature and its design spec. This PR is the Edge Cookie provider seam. The stack order is #1043, #1044, #1045, #1046, #1047. Each PR's own change is visible by comparing its head branch to the previous PR's head branch, and this first PR is independently mergeable to main.

Spec: docs/superpowers/specs/2026-07-30-pluggable-providers-design.md, which is the Tech Lab 2026-07-31 draft revised to match this implementation, with a revision-record table listing every divergence and why. The spec covers both this PR (the EC seam) and #1044 (device and geo selection).

What this PR does

Edge Cookie identity generation becomes a selectable provider behind the EdgeCookieProvider trait in crates/trusted-server-core/src/ec/provider.rs, with the existing HMAC implementation as the built-in and configuration selecting it.

  • [ec] provider names a block under [ec.providers.<key>]. Omitted means stateless with no Edge Cookie, and provider = "none" spells the same choice explicitly (rejected if provider blocks are left configured). A selected provider with no block, an unreferenced stray block, and an unknown key in a block all fail at startup, so misconfiguration is loud.
  • The deprecated [ec] passphrase form still starts. It migrates to provider = "hmac" plus [ec.providers.hmac] with a deprecation warning, so a fleet can move configuration and binaries independently. Both forms together are rejected.
  • Identifier bounds are global and provider-independent, with at most 256 bytes and the alphabet [A-Za-z0-9._~-], enforced at mint, cookie read-back, and 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).
  • Read-back goes through the selected provider's accepts_id, and the identity-graph key through its normalize_id_for_kv canonical form, so an opaque vendor identifier round-trips byte-for-byte. One test proves a non-default provider round-trips verbatim and another proves the graph is keyed by the canonical form.
  • Vendor [ec.providers.<key>] blocks are captured as raw values in core and deserialized by the adapter that injects the vendor provider, so core never names a vendor.
  • Every provider carries a mandatory registered four-character code (provider-code-registry.md, the registry your spec set defines). Core mints {code}~value, checks the code at read-back, and keys the identity graph with it, so identifiers from different providers can never collide and switching providers cannot silently adopt another provider's identities. The built-in provider mints hmac~<hash>.<suffix> and dual-reads its pre-envelope bare form for one release cycle, so deployed cookies keep working.
  • The partner-facing paths accept the enveloped form. A cold read on 27 August found that pull sync, batch sync and the admin lookup still validated the bare shape, so a freshly minted hmac~ identifier was skipped by pull sync, refused by batch sync and answered 400 by the admin lookup while CI stayed green on a bare seeded cookie. is_valid_ec_id now accepts the hmac~ envelope as well as the legacy bare form and rejects any other provider's code, normalize_ec_id_for_kv keeps the envelope so the key matches the one written at mint, and each of the three call sites has a test with a coded identifier (commit 6f15e50c0).
  • Generation failures log at error level, not warn.

Breaking change

A minimum HMAC passphrase length of 32 bytes is now enforced wherever the passphrase is configured. A shorter passphrase that previously started will fail startup validation with a direct message.

How it was verified

Full local gate on this branch, all clean. cargo test-fastly (core plus adapter suites), cargo test-axum, cargo test-cloudflare, cargo test-spin, the integration parity suite, cargo fmt --check, and all six per-target clippy aliases.

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 provider (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. Decomposes #838 (kept as a draft reference until this series merges). Spec baseline from #986.

Produced with AI assistance under James Rosewell's direction, and flagged here so reviewers know to apply the usual scrutiny.

@jwrosewell
jwrosewell force-pushed the split/1-ec-provider branch from 312a4fc to 73b40b9 Compare August 20, 2026 01:47
@jwrosewell
jwrosewell force-pushed the split/1-ec-provider branch 2 times, most recently from 83e551d to e278981 Compare August 25, 2026 13:37
@jwrosewell
jwrosewell force-pushed the split/1-ec-provider branch from e278981 to 4529151 Compare August 25, 2026 16:46
jwrosewell added a commit to jwrosewell/trusted-server that referenced this pull request Aug 27, 2026
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
aram356 requested review from ChristianPavilonis, aram356 and prk-Jr and removed request for prk-Jr August 27, 2026 15:57

@aram356 aram356 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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] passphrase bypasses the new 32-byte minimum - see inline at crates/trusted-server-core/src/settings.rs:658 (suggestion)
  • [ec.providers.hmac] silently accepts unknown keys - see inline at crates/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_provider silently returns Ok(None) for provider = "hmac" with no block - see inline at crates/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 into ProviderCode's, leaving the trait undocumented - see inline at crates/trusted-server-core/src/ec/provider.rs:177
  • ec::get_ec_id is dead code, yet was modified to accept any provider code - see inline at crates/trusted-server-core/src/ec/mod.rs:137
  • Module docs describe constructor injection that is not how RequestInfo flows - see inline at crates/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
  • 🤔 RequestInfo accessors have no production consumer in this PR - cross-cutting, below
  • 🤔 Spec revision followed the implementation - cross-cutting, below
  • 📌 Operator guides still document [ec] passphrase as 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-slot Option<Arc<dyn EdgeCookieProvider>> matched by id(), configured under [ec.providers.<key>] - and a second nomenclature ("providers"). RuntimeServices is 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_provider keys the row with provider_kv_key (ec/mod.rs:476). But handle_identify reads with the raw cookie value (kv.get(ec_id), ec/identify.rs:89), withdrawal tombstones are written under the raw value (ec/finalize.rs, the write_withdrawal_tombstone loop), 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 the CanonicalizingProvider case 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 in EcContext (for example an ec_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) and handle_first_party_click (proxy.rs:1609) forward the raw ts-ec cookie / x-ts-ec header value to origin and click-target URLs through edge_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>, so evaluate_cluster's prefix (ec_hash, ec/kv.rs:715) becomes hmac~<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, and cluster_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's provider.id() == "hmac", and HMAC_PROVIDER_CODE in ec/generation.rs). A typed selector, for example enum 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.

  • 🤔 RequestInfo accessors have no production consumer in this PR. path(), query(), query_param(), header_names(), user_agent(), and header() are supplied by production code but consumed only by tests in this PR (the HMAC provider reads only client_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] passphrase as the current form. docs/guide/configuration.md:1933, docs/guide/key-rotation.md:31, docs/guide/error-reference.md:72, plus ec-setup-guide.md / edge-cookies.md / fastly.md predate the provider layout, the deprecation, and the new stateless default in trusted-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

Comment on lines +652 to +658
log::warn!(
"[ec] passphrase is deprecated; move it to [ec.providers.hmac] passphrase and \
set [ec] provider = \"hmac\""
);
self.provider = Some("hmac".to_owned());
self.providers.hmac = Some(HmacProviderConfig { passphrase });
Ok(())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 wrench - The advertised 32-byte passphrase minimum is bypassed on the deprecated form. finalize_deserialized runs derive validation before migrate_legacy_ec_layout(), and this deprecated field no longer carries a #[validate] attribute, so [ec] passphrase = "short" (or an empty value) migrates and starts successfully. Reproduced with a scratch test: Settings::from_toml returns Ok for a legacy 5-byte passphrase. That contradicts the PR description ("enforced wherever the passphrase is configured") and the commit message. Validating inside the migration keeps the enforcement self-contained for every construction path:

Suggested change
log::warn!(
"[ec] passphrase is deprecated; move it to [ec.providers.hmac] passphrase and \
set [ec] provider = \"hmac\""
);
self.provider = Some("hmac".to_owned());
self.providers.hmac = Some(HmacProviderConfig { passphrase });
Ok(())
Self::validate_passphrase(&passphrase).map_err(|err| {
Report::new(TrustedServerError::Configuration {
message: format!(
"[ec] passphrase (deprecated) is invalid ({err}): use a random secret \
of at least {} bytes, placed in [ec.providers.hmac]",
Self::MIN_PASSPHRASE_LENGTH,
),
})
})?;
log::warn!(
"[ec] passphrase is deprecated; move it to [ec.providers.hmac] passphrase and \
set [ec] provider = \"hmac\""
);
self.provider = Some("hmac".to_owned());
self.providers.hmac = Some(HmacProviderConfig { passphrase });
Ok(())

Verified: with this applied, the legacy short passphrase fails startup, and the full local gate passes.

Comment on lines +725 to +726
#[derive(Debug, Default, Clone, Deserialize, Serialize, Validate)]
pub struct HmacProviderConfig {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 wrench - The spec (section 6) states deny_unknown_fields is on "both built-in provider config structs", but this struct has no such attribute, so [ec.providers.hmac] passphrase = "..." typo_key = "x" is silently accepted. (A typo'd block name is caught by the stray-block rule; a typo'd key inside the block is not.)

Suggested change
#[derive(Debug, Default, Clone, Deserialize, Serialize, Validate)]
pub struct HmacProviderConfig {
#[derive(Debug, Default, Clone, Deserialize, Serialize, Validate)]
#[serde(deny_unknown_fields)]
pub struct HmacProviderConfig {

Verified: with this applied, an unknown key in the hmac block fails startup, and the full local gate passes.

Comment on lines +300 to +304
"hmac" => ec
.providers
.hmac
.as_ref()
.map(|config| Box::new(HmacProvider::new(config.passphrase.clone())) as _),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ refactor - When provider = "hmac" is selected but the block is absent, this arm returns Ok(None) and the deployment silently runs stateless. Settings validation rejects that configuration at startup, but if this seam is ever reached with such a config (programmatic Settings, a future construction path), the result is the exact "silent identity outage" the spec's failure-mode table exists to prevent. The vendor arm fails loudly; this arm should too:

Suggested change
"hmac" => ec
.providers
.hmac
.as_ref()
.map(|config| Box::new(HmacProvider::new(config.passphrase.clone())) as _),
"hmac" => match ec.providers.hmac.as_ref() {
Some(config) => Some(Box::new(HmacProvider::new(config.passphrase.clone())) as _),
// Settings validation rejects a selected provider with no block;
// if that is bypassed, fail loudly rather than silently running
// stateless.
None => {
return Err(Report::new(TrustedServerError::EdgeCookie {
message: "Edge Cookie provider `hmac` is selected but [ec.providers.hmac] \
is not configured"
.to_owned(),
}));
}
},

if !ec_id_has_only_allowed_chars(&ec_id) {
return Err(Report::new(TrustedServerError::EdgeCookie {
message: format!(
"Provider `{}` produced an identifier that is empty, over {} bytes, or outside the cookie-safe alphabet",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick - This string carries a 22-space run (a missing \ line continuation), so the logged error reads "...bytes, or outside the cookie-safe alphabet".

Suggested change
"Provider `{}` produced an identifier that is empty, over {} bytes, or outside the cookie-safe alphabet",
"Provider `{}` produced an identifier that is empty, over {} bytes, or \
outside the cookie-safe alphabet",

}
}

pub trait EdgeCookieProvider: Send + Sync + core::fmt::Debug {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ refactor - This trait has no doc comment: the paragraph written for it ("A strategy for deriving an Edge Cookie identifier...") is fused into the doc block of ProviderCode above (lines 58-77), where it reads as part of that struct's documentation. The stray text is also stale: it says a provider returns Ok(None) from generate, but generate returns GeneratedEdgeCookie { id: None }. Apply manually (two non-contiguous edit sites, so this cannot be a single suggestion): move the strategy paragraph here, reword the Ok(None) sentence to the id: None semantics, and leave ProviderCode with only its own registry-code doc.

// Accept the coded form (any provider's `{code}~value` within the global
// identifier bounds) and the legacy bare HMAC form. Provider-aware
// ownership lives in `EcContext`; this helper only reads the string.
let ec_id = parsed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick - This pub fn get_ec_id has no callers anywhere in the workspace (proxy.rs and testlight.rs use edge_cookie::get_ec_id), yet this PR loosened its filter to accept any {code}~ value without an ownership check against the selected provider. A future caller picking it up would adopt foreign-coded identifiers that EcContext deliberately treats as absent. Either delete the function or align its filter with provider_owns_id.

//! Edge Cookie identity providers.
//!
//! An [`EdgeCookieProvider`] derives an Edge Cookie identifier. Providers are
//! wired by dependency injection: a provider's constructor takes the services it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick - The module doc says a provider's constructor takes the services it needs, with RequestInfo as the example, but RequestInfo is passed at generate call time, not at construction; the opening sentence is also garbled ("...for the client IP) (the adapter, through [build_provider]) supplies instances per request."). Same constructor-injection claim in evidence.rs lines 3-7. Worth a small rewrite so the first thing a vendor implementer reads matches the trait signature.

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

.filter(|provider| provider.id() == other)
.map(|provider| Box::new(SharedProvider(provider)) as _);
if provider.is_none() {
return Err(Report::new(TrustedServerError::EdgeCookie {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Portability adapters swallow unavailable-provider errors

build_provider correctly returns an error here, but Axum, Cloudflare, and Spin catch it from read_from_request_with_geo and replace the EC context with EcContext::default(), so the request continues without identity. That contradicts the provider contract, which says an unavailable required service or injected provider stops the request. Fastly already propagates the error. Please return an error response in those adapters or reject the selection while building adapter state, and add a regression test for an uninjected provider on each adapter.

// generation (for example to request more client evidence). This is empty
// unless a provider produced headers, so it is safe on every path.
for (name, value) in ec_context.response_headers() {
response.headers_mut().insert(name, value.clone());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Provider response effects can overwrite core-managed state

These headers are inserted without checking names or cookie ownership. A provider can return Set-Cookie: ts-ec=..., including when it returns no identifier, and bypass core's identifier validation, graph-write requirement, and managed EC cookie code. It can also overwrite reserved x-ts-* or response-framing headers. Providers may legitimately need their own evidence cookies, so banning every Set-Cookie would be too broad. Please validate these effects or expose a typed response API that reserves the managed ts-* cookie names, the x-ts-* namespace, and framing or hop-by-hop headers while allowing provider-owned cookies.

let mut parts = value.split('.');
let bare = match split_provider_code(value) {
(Some(code), bare) if code == HMAC_PROVIDER_CODE => bare,
(Some(_), _) => return false,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Partner paths reject identifiers from the next provider

is_valid_ec_id explicitly rejects every provider code except hmac, and pull sync, batch sync, and the admin lookup all call it. This is already concrete in the stacked work: PR #1044 adds the hs00~ host-signal provider without changing these consumers, so its valid identifiers work in organic read and write paths but are skipped or rejected by all three partner and diagnostic paths. Please separate global cookie bounds from provider-specific validation, dispatch validation and KV normalization by provider code, and cover a non-HMAC identifier in pull sync, batch sync, and admin lookup tests.

// guards so a stateless deployment on a host with no client IP does not
// log spurious errors. The provider reads it borrowed at generate time
// (see [`generate_with_provider`]), so nothing is cloned here.
if self.client_ip.is_none() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Generic generation requires client IP before calling the provider

This check rejects the request before the selected provider can decide whether it needs client IP. RequestInfo::client_ip already defines an empty string as the unavailable state, and providers are meant to read only the request evidence they need. The generic check therefore blocks header-, cookie-, query-, and client-derived providers that can operate without IP. Please move the requirement into HmacProvider and any other provider that uses IP, then pass the documented empty value to providers that do not.

/// carries, with the value part accepted by that provider's
/// [`accepts_id`](EdgeCookieProvider::accepts_id). A legacy bare identifier
/// (no code prefix) belongs only to the built-in HMAC provider, which
/// dual-reads its pre-envelope form for one release cycle so deployed cookies

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: Define when the bare HMAC reader can be removed

This comment promises one release cycle of bare-HMAC compatibility, but returning users do not have their bare cookie rewritten and the cookie lifetime is one year. The current code is safe while this reader remains. Before scheduling its removal, please define a retirement condition based on the maximum cookie and graph-row lifetime plus rollout skew and observed legacy-reader traffic. Otherwise, remove the one-release wording and keep the reader.

…ider

First of five PRs decomposing the provider and permission epic. The
EdgeCookieProvider trait routes Edge Cookie minting, cookie read-back,
and KV keying through the selected provider, so a vendor identifier
round-trips verbatim instead of being dropped by the built-in shape
check.

- [ec] provider selector with per-provider [ec.providers.<key>] blocks.
  The deprecated [ec] passphrase form still starts for one release
  cycle: it maps to provider = "hmac" with a deprecation warning, and a
  configuration carrying both forms is rejected. provider = "none"
  spells explicit statelessness. A configured block that is not the
  selected provider is rejected at startup, as is a block with no
  selector.
- Global identifier bounds enforced by core at mint, read-back, and
  cookie write: the cookie-safe alphabet [A-Za-z0-9._~-] and a 256-byte
  cap. An identifier outside the bounds is rejected loudly, never
  rewritten, so the cookie value and the identity-graph key can never
  silently diverge.
- The identity graph is keyed by the provider's canonical form of the
  identifier (normalize_id_for_kv), so equivalent representations of
  one identity share one row.
- Request evidence abstraction (crate::evidence) giving providers read
  access to the client IP, headers (including cookies), URL path, and
  query parameters.
- Adapter injection seam: RuntimeServices carries an optional vendor
  provider, so a vendor provider lives in its own crate and core never
  names it. A selected provider the adapter does not inject fails the
  request loudly rather than silently running stateless.
- Provider generate failures log at error level with the request
  proceeding stateless.

Edge Cookie creation and use stay gated by the existing consent context
exactly as on main, including with no provider selected; the permission
model replaces that input in the third PR of this series.

Config migration: move [ec] passphrase to [ec.providers.hmac] and set
[ec] provider = "hmac". The old form keeps working for one release with
a warning. Passphrases shorter than 32 characters are now rejected at
startup; previously they were accepted.

The design spec for this slice and the next lives at
docs/superpowers/specs/2026-07-30-pluggable-providers-design.md, the
2026-07-31 draft revised to match the implementation with a
revision-record table of every divergence.

Every provider carries a mandatory registered four-character code
(provider-code-registry.md): core mints {code}~value, checks the code
at read-back, and keys the identity graph with it, so identifiers from
different providers can never collide and a switch of provider cannot
silently adopt another provider's identities. The built-in hmac
provider mints hmac~<hash>.<suffix> and dual-reads its pre-envelope
bare form for one release cycle.
Since the provider-code envelope, the mint path issues identifiers as
hmac~{64hex}.{6alnum}, and that is the value identify hands to partners.
Pull sync, batch sync and the admin lookup still validated the bare
shape through is_valid_ec_id, so pull sync skipped every freshly minted
identifier, batch sync answered invalid_ec_id for the value partners were
given, and the admin lookup answered 400. CI stayed green because the
lifecycle scenario seeds a bare cookie.

is_valid_ec_id now accepts the hmac envelope as well as the legacy bare
form and rejects any other provider's code, and normalize_ec_id_for_kv
keeps the envelope so the key matches the one written at mint. Tests
cover the validator, the normalizer and each of the three call sites
with a coded identifier.
CodeQL's cleartext-logging query treats a call whose name contains
"passphrase" as a sensitive source, and because the method mutates the
Settings it belongs to, every later log line that prints anything from
Settings (store names, timeouts, header names) is reported as writing a
secret to a log. The passphrase itself is a Redacted<String> and none of
the flagged lines prints it. The method now describes what it does,
migrate_legacy_ec_layout, and its behavior is unchanged.
A reviewer raised a P1 against the pluggable Edge Cookie provider work:
three of the four adapters broke the provider contract that an
unavailable required service or an uninjected provider stops the
request.

The Axum, Cloudflare and Spin adapters each read the Edge Cookie context
with `EcContext::read_from_request_with_geo(...).unwrap_or_else(...)`,
logged a warning and continued with `EcContext::default()`. A deployment
whose selected provider could not be built therefore came up and served
every request with no identity, silently. The Fastly adapter already
kept the report and answered with an error response.

`build_ec_context` on the three adapters now returns
`Result<EcContext, Report<TrustedServerError>>` and every call site
propagates it to that adapter's own `http_error`, the same helper Fastly
uses, so all four answer with the same status and shape.

The design this implements has the composition root check a selected
provider's needs once at startup rather than per request, so
`ensure_provider_available` was added to `ec/provider.rs` and is called
from `build_state_with_settings` on all four adapters (Fastly included,
so the rule is uniform). Building a provider reads no request data, so
a selection an adapter can never supply now fails when application state
is built, and the three adapters answer every route from their existing
`startup_error_router` instead of coming up. Statelessness, meaning no
`[ec] provider` selector or the explicit `"none"`, still passes and
still serves.

The widening question was checked rather than assumed.
`read_from_request_with_geo` can only fail from two places: the provider
build, and a `Cookie` header that is not valid UTF-8. A malformed cookie
value is dropped with a warning by `request_ec_id_if_allowed`, consent
parsing returns a value rather than a `Result`, and the geo lookup is
already swallowed by the adapter before the call, so no ordinary parse
problem reaches the error path and none is turned into a failed request.

Tests: each of the three adapters gains a route test proving an
uninjected provider fails at startup, and an in-crate test proving
`build_ec_context` returns the error rather than a default context.
Core gains a test that the startup check rejects an uninjected provider
and still allows statelessness both ways.

Addresses: Christian Pavilonis review of PR 1043, crates/trusted-server-core/src/ec/provider.rs:317 (P1)
`Settings::finalize_deserialized` runs derive validation before
`Ec::migrate_legacy_ec_layout`, and the deprecated `[ec] passphrase`
field carries no `#[validate]` attribute of its own, so the advertised
32-byte minimum was only enforced on the new `[ec.providers.hmac]`
location. A configuration still on the old form could start with
`passphrase = "short"`, or with an empty value, and mint identifiers
from keying material the new location rejects.

The migration now calls `Ec::validate_passphrase` on the value it is
about to move, before it logs the deprecation warning and writes the
`[ec.providers.hmac]` block, and reports a configuration error naming
the minimum length and the new location.

Tests: `a_legacy_passphrase_is_held_to_the_passphrase_rules` drives
`Settings::from_toml` with the `[ec]` section rewritten to the
deprecated form and proves a short value and an empty value are both
rejected, and that a passphrase of adequate length still migrates to
`provider = "hmac"` with the passphrase in the hmac block. Removing the
new check makes that test fail, so it tests the fix rather than the
surrounding code.

Addresses: Aram Grigoryan review of PR 1043, crates/trusted-server-core/src/settings.rs:658 (wrench)
The provider spec (section 6) says `deny_unknown_fields` is set on both
built-in provider config structs, but `HmacProviderConfig` carried no
such attribute, so `[ec.providers.hmac] typo_key = "x"` was accepted
silently. An operator who mistypes a key gets a deployment that starts
and quietly uses the default for the setting they meant to change.

`HmacProviderConfig` now sets `#[serde(deny_unknown_fields)]`, matching
`Ec` itself and the rest of the settings tree. The struct is a plain
field of `EcProviders` rather than a flattened one, so the attribute
does not collide with the `#[serde(flatten)]` vendor map alongside it.

Tests: `an_unknown_key_in_the_hmac_provider_block_is_rejected` adds an
unknown key to the block in the crate test configuration and proves
`Settings::from_toml` fails and names the key. Removing the attribute
makes that test fail.

Addresses: Aram Grigoryan review of PR 1043, crates/trusted-server-core/src/settings.rs:726 (wrench)
`build_provider`'s `"hmac"` arm mapped over `ec.providers.hmac`, so a
deployment that selected `provider = "hmac"` with no
`[ec.providers.hmac]` block got `Ok(None)` and ran stateless under a
selector that says it has an identity provider. Every other unbuildable
selection in the same match already errors.

The arm now returns `TrustedServerError::EdgeCookie` naming the missing
block, which the startup check `ensure_provider_available` turns into a
failed application state on every adapter.

`Ec::validate_provider_selection` rejects that pair before settings
reach the composition root, so nothing routes through the new arm today.
It is the drift guard for the case where the two checks stop agreeing,
which is exactly the shape of the defect being fixed, so it is worth
keeping rather than leaving the silent branch in place.

Tests: `selecting_hmac_without_its_block_fails_loudly` builds the `Ec`
programmatically, bypassing settings validation to reach the seam, and
proves the error names the missing block. The doc comment's `# Errors`
section is corrected in the same commit, since it still claimed no
built-in construction can fail.

Addresses: Aram Grigoryan review of PR 1043, crates/trusted-server-core/src/ec/provider.rs:304 (refactor)
The error raised when a provider mints an identifier outside the
identifier bounds was written across two source lines without the
trailing backslash that joins them, so the 22 spaces of source
indentation became part of the literal and the logged message read
"...bytes, or                      outside the cookie-safe alphabet".

The continuation is restored, so the message reads as one sentence.

The whole of ec/mod.rs was scanned for the same fault, matching every
string literal and stripping real continuations before looking for runs
of more than one space or a newline inside a literal. This message was
the only one.

Addresses: Aram Grigoryan review of PR 1043, crates/trusted-server-core/src/ec/mod.rs:444 (nitpick)
The paragraph written for the `EdgeCookieProvider` trait sat at the top
of `ProviderCode`'s doc block, so rustdoc rendered it as part of that
struct's documentation and the trait itself had no doc comment at all.
A vendor implementer opening the trait saw nothing, and a reader of
`ProviderCode` saw two subjects run together.

The paragraph moves onto the trait and `ProviderCode` keeps only the
registry text that belongs to it.

The moved sentence was also stale: it said a provider returns `Ok(None)`
from `generate`, but `generate` returns a `GeneratedEdgeCookie` and
signals "no identifier this request" through its `id` field. The
sentence now describes the actual return, with an intra-doc link to the
field.

`cargo doc --no-deps` reports no warning against either item.

Addresses: Aram Grigoryan review of PR 1043, crates/trusted-server-core/src/ec/provider.rs:177 (nitpick)
The pluggable-providers spec required, in its provider-switching section,
that switching must not strand the identities the previous provider
minted and "above all must not make a later opt-out unable to revoke
them". It then claimed old cookies stay recognized after a switch
whenever the newly selected provider accepts their shape.

That claim is false and cannot be made true here. Ownership is decided on
the `{code}~` prefix before any provider is asked about shape, and the
check is enforced twice: `AcceptedProviders::owner` dispatches on the
code, and `canonical_kv_key` re-checks the derived key through
`provider_owns_id`. So a newly selected provider rejects every identifier
the previous one minted, whatever its shape. The new test drives this and
shows the result: after a switch the retired identifier is never adopted,
withdrawal still expires the browser cookie, but the retired provider's
identity-graph row keeps `consent.ok = true` and is never tombstoned. It
then sits for the one-year entry TTL.

I changed the spec rather than the code. The mechanism the spec itself
names for carrying identities across a switch is the `legacy_providers`
reader list, which the same section marks as deferred to the migration
spec, and `AcceptedProviders` is already built as the seam for it. Even
once it lands the requirement would not hold on its own, because it would
depend on the operator listing the retired provider, so an unconditional
guarantee was never something the code could provide. The old wording
also contradicted section 5 of the same document, which already states
the true rule that a cookie carrying another provider's code is treated
as absent.

The replacement says plainly what a switch does to read-back, to the
browser cookie and to the graph rows, and what an operator must do about
revocation: deal with the retired provider's rows at the switch, since
they are identifiable by that provider's `{code}~` key prefix, or accept
that later withdrawals are recorded only in the browser until the TTL
expires. The `cookie_ec_kv_key` doc comment claimed the same reach the
spec did and is corrected to match.
The provider series design specs move to the spec-only PR (IABTechLab#1084) so
they can be reviewed before the code that implements them. Three doc
comments cited those files by repository path, which no longer resolves
from this branch. Refer to each document by name instead, so the
comment stays true whichever PR is read first.
The mint-rejection fix restored one collapsed continuation in ec/mod.rs and
said the rest of that file was clean, which it was. The same fault exists in
four more places on this branch, so fixing only the reported one leaves the
pattern half addressed.

Each was written across two source lines without the trailing backslash, so the
source indentation became a run of spaces inside the message:

  ec/admin.rs:373                the invalid-EC-ID response an operator sees
  ec/finalize.rs:125             the skipped-response-write log line
  ec/provider.rs:635             the missing-client-IP error from the HMAC provider
  ec/pull_sync.rs:72             the skipped-dispatch log line
  integrations/testlight.rs:196  the no-recognized-EC-ID error

The continuation is restored in each, so every message reads as one sentence.
The whole of trusted-server-core was scanned for the same shape, matching runs
of five or more spaces inside a string literal. The only remaining matches are
TOML fixtures in settings.rs tests, where the embedded newlines are deliberate.

Addresses: ec/mod.rs:444 follow-up, the same fault outside the file first reported
The commit that rewrote the module docs to match the trait signature left two
item-level doc comments in the same file still describing constructor
injection, so the claim that nothing passes evidence by constructor was
contradicted three declarations further down.

IdentityInput's doc said request data reaches a provider "through the services
injected into its constructor". EdgeCookieProvider::generate's doc said the
identifier is derived "from the provider's injected services". Neither matches
the signature, which takes request_info: &dyn RequestInfo as a parameter and
reads evidence from it. The built-in HMAC provider does exactly that at
ec/provider.rs:632.

Both now describe the parameter the evidence actually arrives on. The crate was
searched for the same wording; the only other mention is in ec/mod.rs on a
test-only helper, where it correctly describes how the provider itself is
constructed rather than how request evidence reaches it.

Addresses: ec/provider.rs:4 follow-up, item docs still describing constructor injection
`ProviderCode::new` is public and validated its argument with `assert!`,
so any caller outside this workspace could take down a live request by
passing a code that was not exactly four characters of [a-z0-9]. The doc
comment claimed the panic "never" fires on a request path, which held
only for as long as every caller happened to pass a literal, and nothing
enforced that. A vendor Edge Cookie provider is exactly the caller the
claim could not cover.

`new` now returns `Option<ProviderCode>`, so it cannot panic whatever it
is given, and a caller outside core has to handle a malformed code. The
compile-time guarantee the codes in this workspace relied on moves into
a new `provider_code!` macro, which runs the same check inside a `const`
block, so a bad literal fails the build and the value it yields needs no
unwrapping. Every code in the workspace, the built-in HMAC code
included, now goes through the macro.

Addresses: crates/trusted-server-core/src/ec/provider.rs, where
`ProviderCode::new` could panic at run time while its documentation said
it could not.
`resolve_named_provider` looked for a built-in provider before the one
the adapter injects, so a vendor provider whose id is `hmac` was dropped
in favour of core's own and nothing said so. Nothing reserved the name
and nothing warned, which left an operator with a configured vendor
provider that never ran and no way to see why.

This is not only a missing warning. Once this work merges, IAB Tech Lab
is itself a vendor shipping an HMAC provider while core still ships one,
so two suppliers really can arrive under one name in a single
deployment, and there is no correct way to pick between them.

`build_provider` now refuses that pair through `ensure_no_name_collision`
and the error names both claimants, core and the deployment's adapter,
along with the contested name. The check runs before the selector is
read, so selecting a different provider does not hide the clash, and
because the adapters call it through `ensure_provider_available` while
they build application state, an operator is told at startup rather than
on the first request that happens to select the name.

Addresses: crates/trusted-server-core/src/ec/provider.rs, where
`resolve_named_provider` silently preferred the built-in `hmac` provider
over an injected one of the same name.
`EC_RESPONSE_HEADERS` in the EC finalization module and the first four
entries of `INTERNAL_HEADERS` in the constants module were the same four
header names written out twice, in two files, with nothing keeping them
in step. The two lists do different jobs, one is stripped from a
response the request may not carry an identity on and the other is never
forwarded to a third party, but every Edge Cookie output header has to
be in both, so adding a fifth to one and forgetting the other would send
Edge Cookie output to an origin that should never see it.

`EC_RESPONSE_HEADERS` now lives once, in the constants module, and
`INTERNAL_HEADERS` is assembled from it and the remaining internal names
while the crate is compiled, so the Edge Cookie half cannot be edited in
one place and missed in the other. EC finalization reads the same
constant instead of keeping a copy. The new test in the constants module
asserts the containment, the total, and that no name appears twice, so
going back to two hand-written lists fails the build.

Addresses: crates/trusted-server-core/src/ec/finalize.rs and
crates/trusted-server-core/src/constants.rs, where one list of Edge
Cookie response headers was maintained by hand in two places.
The composition root resolved `[ec] provider` and threw the provider
away, keeping only the knowledge that the selection could be satisfied,
and then the request path resolved the same settings again to get a
provider it could use. On the Fastly, Cloudflare and Spin adapters that
is twice for every request, because those three run a fresh instance per
request and rebuild application state each time, which was confirmed by
reading `run_app` in the matching edgezero adapters.

The composition root now keeps what it resolved, in `AppState`, and
hands the same instance to every request through the new
`RuntimeServices::resolved_ec_provider`. Core reads it through
`request_provider`, which returns the threaded instance when there is
one and otherwise resolves exactly as before, so an adapter that threads
nothing, the core tests and any embedder driving core directly included,
keeps today's behaviour, the loud failure on a selected but uninjected
provider included. Nothing about which provider is chosen changes, only
how many times the choosing happens.

The Axum adapter is deliberately left checking rather than keeping,
because it is a long-lived process whose application state is built once
at start-up, so it has no second resolution to save.

Addresses: crates/trusted-server-core/src/ec/provider.rs and the Fastly,
Cloudflare and Spin adapters, where `ensure_provider_available` and
`EcContext::read_from_request` each built the provider once per request.
The Spin adapter cannot start on upstream/main today, and this fixes it
here. `build_state` compiled `trusted-server.example.toml` into the
binary and parsed it, but that template ships placeholder secrets by
design and its placeholder admin password is the first entry in
`PASSWORD_PLACEHOLDERS`, so `validate_admin_handler_passwords` refused it
every time. `build_state` therefore never returned `Ok`, the router fell
back to the start-up error handler, and the component answered 503 to
every request. The failure is "Handler `^/_ts/admin` uses a placeholder
password; configure a strong secret".

Nothing caught it because nothing called `build_state`. Every Spin test
enters through the `routes_with_settings` parity seam and supplies its
own settings, so the one path a deployed component actually takes was
the one path never exercised.

Settings now come from the platform config store at run time, which is
what the Fastly, Axum and Cloudflare adapters already do, so an operator
publishes one with `ts config push` and the component reads it. The new
`SpinPlatformConfigStore` reads Spin component variables directly rather
than through the per-request handle, because application state is built
before any request context exists. Component variables are ambient, which
is how the secret store already reads them, and both paths map keys
through `spin_variable_name` so start-up and the request path read the
same variable for the same key.

The new test calls `build_state` and requires any failure to be the
absence of a config store. Outside the Spin runtime there are no
component variables, so it cannot return `Ok` under `cargo test`, but a
configuration compiled into the binary would fail for a different reason
and the test says so. Restoring the old body fails it with the
placeholder-password message.

Addresses: crates/trusted-server-adapter-spin/src/app.rs, where
`build_state` parsed a baked example template whose placeholder admin
password made every request fail.
`get_ec_id` is public on upstream/main today and this fixes it here. It
reads the `x-ts-ec` request header and then the `ts-ec` cookie, and
checks the result only with `ec_id_has_only_allowed_chars`. That function
is the global cookie backstop, the length cap and the cookie-safe
alphabet, and its own documentation in `ec/cookies.rs` says the strict
check is the one used to reject untrusted request values. On its own it
accepts any run of `[A-Za-z0-9._~-]` up to the cap, so it cannot tell an
identifier this deployment minted from one an attacker typed. `x-ts-ec`
is stripped from responses but not from inbound requests, so the header
really is the client's to set, and the raw reader prefers it over the
cookie. This is the inbound twin of the egress fault this branch already
fixes, which is why it belongs here.

The right check is not the built-in strict format validator. A vendor
provider's identifier is not required to match the HMAC
`<64 hex>.<6 alphanumeric>` shape, so holding every deployment to it
would drop exactly the opaque identifiers the provider model exists to
carry. The right check is provider ownership, where the `{code}~` prefix
is dispatched to the provider that owns it and that provider's
`accepts_id` decides, which is what `recognized_ec_id` already does and
what the EC lifecycle applies on read-back.

The raw reader cannot make that check, because it has neither settings
nor the selected provider, so it stops being a public entry point. It is
now `pub(crate)` and named `unvalidated_ec_id_from_request`, so no caller
can read it as returning a validated identifier, and `recognized_ec_id`
is the only way in from outside the module. Nothing outside the crate
called the old name.

The new test drives three identifiers this deployment could never have
issued through both readers, shows the bounds alone accept all three,
and requires the public path to recognize none of them, while an
identifier the selected provider does own is still returned. Replacing
the ownership check with the bounds fails it on the first one.

Addresses: crates/trusted-server-core/src/edge_cookie.rs, where
`get_ec_id` was public and validated client-supplied identifiers with the
outbound backstop list.
Reverts the removal of the request-evidence accessors, so RequestInfo carries
the client IP, the User-Agent, headers by name, header names, the path, the
query and its parameters again.

They were removed to satisfy a rule in our own specification, which says every
trait method needs a production caller in the change that introduces it. That
rule is right for a behavioural trait, where a method nothing calls is dead
weight. It is wrong for an evidence interface, and applying it here was our
mistake rather than anyone else's.

An evidence interface describes what a request carries, not what today's code
happens to read. Held to the caller rule it grows a method every time a vendor
arrives, so no vendor can write against it and it cannot stay stable across a
release. It also puts the boundary in the wrong place, because what a provider
may see was never the control. What a provider may do with what it sees is the
control, and that is the permission model.

The specification is amended in the same series rather than quietly ignored.

Two test provider codes restored with the revert predate ProviderCode::new
returning an Option, so they now build through the macro that cannot fail.

Addresses: crates/trusted-server-core/src/evidence.rs, an evidence interface
narrowed to today's callers
Fixes doc comments, error strings and TOML comments on split/1 so they
match the code they describe, and applies house-style wording rules to
every added line touched. Continues and completes work a prior agent
started (which stopped partway through the B3 item list), reviewed
against the run books at .claude/pr1/runbook-track-1-code-chain.md and
.claude/pr1/comments-docs-runbook.md in the trusted-server repo.

F2 (verified, already done by the prior agent): added "cache-control"
to FRAMING_OR_HOP_BY_HOP_HEADERS in ec/provider.rs with a regression
test, and removed the false reference to a per-adapter
is_hop_by_hop_response_header function.

B3.9 (verified, already done by the prior agent): recorded, rather
than fixed, the gap where pull sync (ec/pull_sync.rs) and the admin
lookup (ec/admin.rs) key identity-graph rows by the raw identifier
instead of the canonical form the three organic paths use. A doc
comment on EcContext::kv_key_for and AcceptedProviders now names the
gap and points at commit 343ac3e, which fixed the three organic paths.
Recorded as a known issue for a later change rather than changed now,
because routing these two paths through the canonical key this late
changes behavior.

Wrong-claim and stale-reference fixes (runbook Part B3, items 1-25):
verified each item against the current tree. Most were already
corrected by the prior agent (HmacProvider failure handling, the
environment-variable override claim, EcProviders selection docs,
provider construction timing, BorrowedRequestInfo allocation, the
generate_if_needed and validate_provider_selection # Errors lists, the
retirement-arithmetic doc, kv_key_for, ec_allowed, request_headers,
edge_cookie.rs recognized_ec_id, admin.rs and cookies.rs identifier
grammar, IdentityInput gating, and the plural "built-in providers"
wording in platform/types.rs). This pass added the one remaining fix:
crates/trusted-server-adapter-spin/src/app.rs no longer claims
Cloudflare reads settings the same way as Fastly and Axum (Cloudflare
also reads a JSON binding and compiles in the example TOML natively).

House-style sweeps (runbook Part C), restricted to lines split/1 added
over upstream/main d516a9e, verified per line via git diff before
editing:
- "mint"/"minted"/"mints" -> "create"/"created"/"creates" (or "issue"/
  "derive" by sense) in doc comments, inline comments and expect()/
  assert messages across ec/provider.rs, ec/mod.rs, ec/admin.rs,
  ec/batch_sync.rs, ec/cookies.rs, ec/finalize.rs, ec/generation.rs,
  ec/identify.rs, ec/pull_sync.rs, edge_cookie.rs,
  integrations/testlight.rs, platform/test_support.rs, proxy.rs, and
  crates/trusted-server-adapter-fastly/src/app.rs. Left the `mint: bool`
  test-builder struct field name alone (an identifier, not prose), and
  left every "mint" occurrence that predates split/1 alone (confirmed
  against upstream per file before editing; two lines were edited by
  mistake and then reverted once the upstream check showed they were
  pre-existing text, not split/1 additions).
- "initialise" -> "initialize" in the four adapters' build_state /
  build_state_with_settings doc comments.
- "several" -> "multiple" in ec/generation.rs.
- "HTTP/2 fingerprint" -> "TLS and HTTP/2 signal" in ec/generation.rs.
- "built-in HMAC default" -> "built-in HMAC provider" (there is no
  default provider) in crates/edgecookie/README.md.
- Environment variable casing: TRUSTED_SERVER__ec__provider ->
  TRUSTED_SERVER__EC__PROVIDER in trusted-server.example.toml, to match
  settings.rs and the documented upper-case form.
- Bytes vs characters: trusted-server.example.toml's new
  [ec.providers.hmac] block comment now says ">= 32 bytes" to match the
  startup error message, which counts bytes.
- A8 cargo-feature overclaim: trusted-server.example.toml no longer
  says a vendor provider "needs its own cargo feature" (none exists);
  it now says a vendor provider ships in its own crate the adapter
  composes in.
- A15 environment-loading overclaim: trusted-server.example.toml's [ec]
  block comment now matches settings.rs, saying deployment tooling can
  merge an environment value into the published configuration before
  load, and that the running server itself reads settings from the
  platform config store, not the environment.

Also fixed two doc-comment line-wrap glitches left by the prior
agent's edits (a stray single-word line in ec/provider.rs's
build_provider doc and in settings.rs's Ec::provider doc), where a
mid-sentence line break had been left in place after wording changed.

Left for a documented human decision rather than changed, per the
runbook's own "James decides" note: two em dashes in
trusted-server.example.toml:76 and crates/trusted-server-core/README.md
that follow an existing dash-separated heading/bullet convention used
throughout each file; and the one new "test-publisher.com" test URI in
ec/identify.rs, which matches roughly twenty pre-existing (not
split/1-introduced) occurrences of the same fixture domain already in
that file, so changing only the new one would be inconsistent and
changing the rest is outside split/1's introduced lines.

Out of scope for this branch, so not touched: the "no host-specific
call" overclaim (B1 item 1, on split/2/3), the geo-default docs (A1, on
split/6/7), and the drafted GitHub text fixes (A2-A4, A10-A16), all of
which live on later branches or in .claude/pr1/ review artifacts.

Not built or tested per instructions; verification is deferred to the
full-stack gate run after every branch in the chain is rebased.

AI assistance note: this commit was produced by an AI coding session
that continued a prior AI session's partially completed edits, reading
both against the run books named above. A human should review the
"James decides" items before the stack is pushed.
@jwrosewell
jwrosewell force-pushed the split/1-ec-provider branch from 0f5c063 to 11cc575 Compare August 31, 2026 12:50
jwrosewell added a commit to jwrosewell/trusted-server that referenced this pull request Aug 31, 2026
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.
jwrosewell added a commit to jwrosewell/trusted-server that referenced this pull request Aug 31, 2026
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.
@jwrosewell

Copy link
Copy Markdown
Contributor Author

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 Addresses: line naming the file, line and label it answers, so the mapping below is verifiable from the branch itself rather than only from this table.

The branch is rebased onto d516a9e94 and merges cleanly. Every commit was tested before the next was written, and the gate set passes on the final head of the chain, being cargo fmt, all six clippy targets, test-fastly, test-axum, test-cloudflare, test-spin, and the 62 host-target CLI tests.

We also run the core library suite natively, at 2,463 tests, and #1047 adds that run to test.yml. This matters for reading any red build, not only ours. The WebAssembly targets build with panic=abort, so their harness stops at the first failing test and reports every later one as never run, hiding them until the first is fixed. The native run reports them all at once, at the cost of one extra compilation of a crate the job already builds.

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 Redacted type whose Debug and Display both print [REDACTED], and the value only ever feeds the HMAC, never a log. The flagged lines log the jurisdiction and redacted identifiers, nothing sensitive. CodeQL taints the whole Edge Cookie context because it now holds the provider that carries the redacted passphrase, so it marks every log of a context field. Nothing in cleartext reaches a log.

Where each piece is, and what changed between the PRs

Two 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
exists, as a sixth body of work.
#1084 is still design and no code, and it has grown since we raised it. It now carries all seven design documents, 3,925 lines across seven files, rather than the single seam spec it started as. We moved the other six out of the code PRs and into it deliberately, because a spec sitting in the same PR as a later piece of code describes behavior that does not arrive until two or three PRs further on. Merging #1084 first puts every design document in place before any code that implements one lands. Implementing the seam is a separate PR we will raise, at 34 commits and 81 files, proven end to end by a test integration that lives outside trusted-server-core and is registered through a real adapter. Keeping it out of #1043 is deliberate, so this PR stays reviewable against the finding it answers.

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.

# PR What it is Own change
1 #1084 the design set, no code. It now carries all seven specs, not the one it was raised with, because we moved them out of the code PRs. Mergeable today 7 files
2 #1043, this one pluggable Edge Cookie identity, plus 20 commits responding to both reviews 44 files
3 #1044 device and geo provider selection, and the host-signal provider 32 files
4 #1045 the permission model, whose vocabulary is the IAB Privacy Taxonomy Data Uses, mapped from the IAB TCF Europe purposes where no Data Use exists yet 46 files
5 #1046 the browser-set Edge Cookie path 13 files
6 #1047 the documentation set 16 files
7 #1094 the implementation of #1084, which makes the seam real rather than specified 81 files
8 #1054 reworked, to follow #1054, the managed LiveRamp RampID integration, reworked onto the seam. As it stands it adds 511 lines to integrations/prebid.rs inside core. On the seam a vendor module registers from its own crate, so the same feature can land with those 511 lines outside core instead. Same feature, same author, no larger core. We will do that rework and raise it to follow

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 [geo] configuration, and [geo] does not exist until #1044 creates it. #1045 adds default_country and assume_single_jurisdiction to that structure, and references the device provider trait as well. Applied to a tree without #1044 it has nothing to attach to.

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 main at d516a9e94 and each of the rest sits on the one above it, so none of them can merge out of order and none needs a merge commit to get in. #1084 is the exception and deliberately so, because it is seven specification files and the chain touches none of them, so it conflicts with nothing and can go in first on its own. Each of the seven passes the full gate set on its own, so none is green only because the branch above it fixes something.

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

Feedback How it is addressed Commit
Christian, P1, ec/provider.rs:317: portability adapters swallow unavailable-provider errors, so the request continues with no identity Axum, Cloudflare and Spin now return an error response instead of an empty context, matching Fastly. All four also now check at startup that the adapter can actually supply the provider the operator selected, so a deployment that names a provider its host cannot build fails when the application starts rather than on every request. A deployment that selects no provider at all is unaffected and still serves normally. f0ca12a (8 files)
Aram 🔧, settings.rs:658: legacy [ec] passphrase bypasses the 32-byte minimum Validation moved inside the migration, so every construction path is covered. Your suggested wording used verbatim. b2bb944 (settings.rs)
Aram 🔧, settings.rs:726: [ec.providers.hmac] accepts unknown keys deny_unknown_fields added, with a test that a typo inside the block fails startup. 472218b (settings.rs)
Aram 🔧, ec/identify.rs, ec/finalize.rs: identity-graph reads and withdrawal tombstones bypass the provider's canonical key One derivation on EcContext, used by identify, the withdrawal tombstones and EID ingestion. Three tests, one per path, using a provider whose canonical form differs from the cookie value. 343ac3e (ec/identify.rs, ec/finalize.rs, ec/mod.rs)
Aram ❓, proxy.rs:1263, :1609: the spec says an unrecognized value is never egressed, but the proxy paths egress it The code changed, not the spec. Those paths now forward only a value the selected provider recognizes. Changes behavior, see Behavior changes below. 8684c69 (proxy.rs, edge_cookie.rs, testlight.rs)
Aram ♻️, ec/provider.rs:304: build_provider returns Ok(None) for hmac with no block Selecting hmac with no [ec.providers.hmac] block now returns an error naming the missing block, rather than quietly building no provider and running with no identity. Configuration validation already rejects that pair, so the test reaches the seam by constructing the settings directly. c4d2f1d (ec/provider.rs)
Aram ⛏, ec/mod.rs:444: 22-space run in the message rejecting an out-of-bounds identifier Line continuation restored. Every other string literal in the file checked for the same fault, and this was the only one. 93cd1e8 (ec/mod.rs)
Aram ♻️, ec/provider.rs:177: the trait's doc is fused into ProviderCode's and is stale Paragraph moved onto the trait, the Ok(None) sentence corrected to the id: None semantics, ProviderCode left with its own text. a265c96 (ec/provider.rs)
Aram ⛏, ec/mod.rs:137: ec::get_ec_id is dead yet was loosened to accept any provider code Deleted. No caller anywhere in the workspace, and publish = false means nothing outside can depend on it. 004581c (ec/mod.rs)
Aram ⛏, ec/provider.rs:4: module docs describe constructor injection that is not how evidence flows Both module docs rewritten to match the trait signature. Verified nothing passes evidence by constructor. d6041f0 (ec/provider.rs, evidence.rs)
Aram ♻️, magic strings "hmac" and "none" across four call sites Typed EcProviderSelection { None, Named(String) }. Every provider now resolves by name through one path, including the built-in HMAC one, so a provider that happens to live in core gets no special case and no shortcut the vendor crates do not have. The configuration surface is unchanged and each form has a round-trip test. 885e3ce then b146aeb (4 files)
Aram 🤔, ec/kv.rs:715: cluster prefix listing splits across the envelope migration Accepted rather than bridged, and now documented with the bound and the reason. Every consumer of cluster_size was checked, and it gates nothing, being reported in identify responses only. 20bb082 (ec/kv.rs, spec)
Aram 🤔, evidence.rs: accessors with no production consumer Not done, and we think the observation is right about the rule and wrong about this interface. All the evidence is retained. The short reason is that an evidence interface describes what a request carries, not what today's code reads, and what a provider may see was never the control. What it may do with what it sees is, and that is the permission model. Full reasoning under How providers see the request. We have amended our own specification rather than leave it contradicting the code. 6cc3c91
Christian, P2, ec/finalize.rs:57: provider response effects can overwrite core-managed state Core reserves its own surface, being the ts- cookie prefix, the x-ts- header prefix, and framing and hop-by-hop headers. A violation fails the request. A provider's own cookies still pass. Cookie names read as bytes, so a non-UTF-8 value cannot smuggle a reserved name through. 69649aa (ec/finalize.rs, ec/provider.rs, ec/mod.rs)
Christian, P2, ec/generation.rs:207: partner paths reject the next provider's identifiers Global cookie bounds split from provider-specific validation, and both validation and KV normalization now dispatch by provider code. Non-HMAC identifiers covered in pull sync, batch sync and admin lookup tests. 53d632e (7 files)
Christian, P2, ec/mod.rs:376: generic generation requires a client IP before calling the provider Requirement moved into HmacProvider, the only provider that reads it. A provider deriving identity from other evidence now creates an identifier on a host with no client IP. 941297f (ec/mod.rs, ec/provider.rs)
Christian, non-blocking, ec/provider.rs:145: define when the bare HMAC reader can be removed Condition written from the real constants rather than estimates, being one year, being the longer of the cookie lifetime and the graph row TTL, measured from the last write that refreshes a bare-form row, which for a returning visitor carrying a ts-eids or sharedId cookie is later than the last release that could create one, plus rollout skew. The comment also says plainly that the observable half cannot be checked, because nothing counts a bare-form read. The one-release wording is gone. e317190 (ec/provider.rs, ec/cookies.rs, registry doc)
Found while answering the above, the collapsed line continuation was not unique to ec/mod.rs The same fault is in four more messages on this branch, at ec/admin.rs:373, ec/finalize.rs:125, ec/provider.rs:635 and ec/pull_sync.rs:72, plus integrations/testlight.rs:196. Two of the five are operator-visible. All are restored, and the crate was scanned for the same fault, so the claim now covers this crate rather than one file. ada4d79 (5 files)
Found while answering the above, two item docs still described constructor injection The earlier pass rewrote the module docs but left IdentityInput and EdgeCookieProvider::generate saying evidence arrives through injected services, which contradicted the row above it. Both now name the request_info parameter the evidence actually arrives on. 4cf202f (ec/provider.rs)

Addressed elsewhere in the chain

Each is answered in the PR of the chain where the answer belongs rather than in this one.

Feedback How it is addressed Where, with the commits
Aram 🔧, the major blocker: vendor identity should lean on the integration system rather than a second extension mechanism Done, in the seam PR. A registration now declares an Edge Cookie provider and a device provider exactly as it declares a geo provider, the registry resolves each against its selector, and the adapters apply all three in one place. There is no second mechanism for identity to sit on any more. That PR also makes the three provider interfaces asynchronous and hands a provider the platform services, because until now every provider method was synchronous while every platform service was asynchronous and a provider was handed none of them, so a provider that needed to call a backend, read a store or fetch a secret could not be written at all. The traits keep their Send + Sync bound and use #[async_trait(?Send)], which is the pattern PlatformHttpClient already uses in this codebase, so the provider stays safe to share while the future stays on one thread. No provider keeps a synchronous method, including the built-in ones, and two tests drive a provider that reads a value out of the config store it is handed, so the services parameter is exercised rather than merely present. Two tests select the probe module for identity and for device and assert the resolved provider is the module's own, so its id() is seam_probe and not core's HMAC. A full generate round trip that drives the probe's provider and asserts the cookie value it produces is added in the seam PR. It could not be done on #1043 itself, because a registration can only carry an Edge Cookie provider once EdgeCookieProvider exists, and that is what #1043 adds. The seam PR is the first point in the chain where the trait and the registry exist together, which is why it lands there and why the chain has to merge in order. The seam PR, #1094. 50fffbd carries identity and device on the registration and adds the two tests, and bedd495 completes it.
Aram 🤔: the spec revision followed the implementation, so divergences become ratifications Taken, and the practice is changed rather than defended. #838 was opened on 2 July, before much of what is now in core, so that sequence was always going to be awkward and we are not going to pretend otherwise. What we did next is the answer, because the #1084 seam spec was written on 27 August and its implementation began on 28 August, so the design was fixed before the code existed. What implementing it then taught us is published in that spec as section 8, "What implementing this found", as its own section rather than folded quietly into the body. A reader can see what changed and why, which is the thing a ratification hides. Avoiding a repeat of it is also the practical reason this stack needs to merge now. The longer the code sits unmerged while main moves, the more the specifications describe something the tree no longer matches, and the only ways out of that are to rewrite the specs to fit what happened, which is the ratification Aram objected to, or to rewrite the code. Merging the chain in order ends that pressure rather than managing it. #1084. 8b0f9c0 wrote the seam spec on 27 August, before any of its code existed, and 9b5b328 added section 8 recording what implementing it then found.
Aram 📌: operator guides still document [ec] passphrase as the current form Done. The provider documentation set is #1047, the last PR of the chain, which is where the operator guides are rewritten for the new form. Putting it on #1043 would mean documenting four PRs' worth of behavior in the first of them, so a reader of #1043's guides would be reading about code that is not there yet. #1047. e82366c adds the provider documentation set, and 5df37ef corrects it against the code.

Behavior changes

Four, 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.

Change Before After Why we think it is right
A stateless deployment no longer egresses the Edge Cookie value [ec] provider unset. A browser sends Cookie: ts-ec=abc123. Trusted Server forwards that value to the origin, to click targets, and testlight posts it as user.id. Nothing is forwarded on any of those three paths. Testlight, which requires an identifier, fails rather than proxying. Aram's ❓ asked which should change, the spec or the code. The pluggable-providers spec, Recognize row, says a value the selected provider does not recognize "is never used or egressed". We changed the code so that sentence is true, rather than weakening the sentence.
A short deprecated passphrase now fails startup [ec] passphrase = "short", five bytes. It migrates into the new provider block, the application starts, and identifiers are created from a five-byte secret. Startup fails. The operator sees [ec] passphrase (deprecated) is invalid: use a random secret of at least 32 bytes, placed in [ec.providers.hmac], so the message says both what is wrong and where the secret now belongs. Aram's 🔧 at settings.rs:658, using his suggested wording verbatim. This PR already advertised a 32-byte minimum. The check simply never ran on the deprecated form, so the advertisement was false.
A provider cannot write into core's own response surface A provider returns Set-Cookie: ts-ec=…, or an x-ts-… header. It is written to the response, bypassing core's identifier validation and its identity-graph write. The request fails with Provider \acme` returned a response header `set-cookie` that sets a cookie in the `ts-` namespace Trusted Server manages, so the log names the provider, the header and the reason. A provider setting its own cookie, for example acme-evidence`, still passes through untouched. Christian's P2 at finalize.rs:57 offered two remedies, validating the effects or a typed response API. We took the first, which is the smaller change and the one that keeps a provider able to set its own cookies, as he asked.
Host geolocation becomes opt-in, where it was always on On main there is no [geo] section at all, and the Fastly adapter builds FastlyPlatformGeo unconditionally, so every deployment resolves location. With the chain merged, [geo] default_country is required, so a config that lacks a [geo] section fails at startup with a message naming the missing key. Once the operator adds it, location resolves only if they also set provider = "platform" for the host lookup, or name a module that supplies one. Unset and "none" both resolve nothing and make no host geo call. A deployment should not be sending client IPs to a host geo service because nobody turned it off. Making it opt-in means a default deployment is tied to no geo vendor, which is the same neutrality argument as the rest of this work. It fails loudly rather than quietly, because default_country is required, so an operator cannot upgrade without meeting the [geo] section and deciding. We would rather state this here than have a deployment discover its targeting changed.

How providers see the request

Applying 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
Agent, every header, the path, the query and its parameters, the form parameters and their values, and the host signals a host can supply. Not a subset chosen by what today's callers happen to read. An interface that grows one method each time a vendor arrives is not something a vendor can write against, and it cannot be stable across a release, which is the thing a vendor needs most. 51Degrees will use all of them, to the extent a request's permissions allow.

Restricting what a provider can see is the wrong lever. The right one is
permissions, and it is two layers deep:

  1. A provider advertises the permissions it requires. Core does not run it at
    all when those permissions are not available for the request. A provider that needs an identifier it may not store never executes.
  2. The provider is given the resolved permissions and decides what it may use of
    what it can see.

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 6cc3c91. The one addition the later work brings is a form_param accessor, because evidence conventions of this kind populate their query keys from POST bodies as well as from the query string.

So the evidence interface is whole on #1043 as of 6cc3c91, and form_param follows with the vendor work that reads a form value, alongside the code that calls it.

We will prove the evidence actually arrives. A loopback provider that
consumes every piece of evidence and returns it, used in tests, so a change that quietly stops delivering some part of the request fails rather than passing silently. That is also the beginning of the conformance suite below.

Two notes on sequencing. The advertise-and-gate half needs required_permissions on the provider trait, which the pluggable-providers spec places with the permission model, so it lands in #1045 rather than here. And form values are not reachable by a provider today, because neither the request info nor the identity input carries the body, so we are adding that lazily, meaning the body is parsed only if a provider actually asks for a form value, so a deployment whose provider never reads one pays nothing.

The gap this leaves, which we would like to fill

There is no conformance suite a provider can be run through. Core defends
itself against a provider in multiple places, being identifier bounds and alphabet, the reserved response surface, canonical key handling, behavior when evidence it needs is absent. Each is tested where it happens. None of it is expressed as a suite any implementation can be run through to show it behaves, so a vendor writing a provider cannot check their own work, and core cannot show a new provider is well behaved except by someone remembering to look. The loopback provider described above is the first piece of it, and it does not exist yet either. We think the rest belongs with the module seam rather than with this PR, and we are willing to write it. It is not filed as an issue, because the code it would test is the code these pull requests propose rather than anything on main.

Found in main while doing this, raised as issues

Eight observations about code that exists on main today, which neither review raised. Each is filed as its own issue.

We found more than these eight, but the rest are in the code these pull requests propose rather than in the code the core team has. Those are not issues, because an issue is a statement about the accepted codebase and this code is not accepted yet. They are either fixed in the pull request that introduces them or written up in that pull request as a known design question, which is where they belong.

Fixed in this stack.

  • Edge Cookie identifiers leave the edge without an ownership check on the proxy and testlight paths #1096: the live edge_cookie::get_ec_id, used on the proxy replay and click paths and by testlight, accepted any well-formed value with no ownership check, so an identifier this deployment never issued was read back and egressed. This is the same trap Aram flagged on the dead ec::get_ec_id, but on a production path. Fixed in the egress commit above, and the issue is filed so the fix is traceable and closes when this merges.

  • The Edge Cookie response header list is a second hand-maintained copy of the internal header list #1099: EC_RESPONSE_HEADERS in ec/finalize.rs is a second hand-maintained copy of the first entries of INTERNAL_HEADERS, with nothing keeping the two in step, so a header added to one and not the other is silently forwarded or silently stripped. The list now lives once and the internal list is assembled from it at compile time, with a test that fails if they drift.

  • The Spin adapter cannot build application state, so it answers every request except the health probe with 503 #1101: the Spin adapter compiles the shipped example configuration into the binary, whose admin password is a placeholder that configuration validation rejects unconditionally, so build_state never succeeds and every request is answered with 503. No test caught it because every Spin test supplies its own settings through routes_with_settings, so the one path a deployed component takes is the one path never exercised. Settings now load from the platform config store at run time, as they do on the other three adapters.

  • Inbound Edge Cookie identifiers are checked with the outbound backstop, so another deployment's identifier is accepted #1095: inbound Edge Cookie identifiers are checked only against the outbound character backstop, so a correctly shaped identifier issued by a different deployment is accepted and read back. The backstop's own doc comment says the strict path is the one for untrusted request values. x-ts-ec is also absent from the spoofable-header list, so the header is the client's to set and is preferred over the cookie. Now dispatched to the provider that owns the identifier, which is the only layer that can judge a vendor identifier it did not create.

  • On Cloudflare no region is resolved, so every US privacy signal fails open #1102: the Cloudflare adapter resolves no region, so jurisdiction detection never reaches its US branch and falls through to non-regulated, where an Edge Cookie is created outright. Global Privacy Control, the GPP US sale opt-out and the US Privacy string are all consulted only on the branch that is never reached, so all three signals failed open rather than only the first. The region now comes from cf-region-code, which carries the subdivision code the privacy-state list is written in, rather than cf-region, which carries the name and would have matched nothing.

Not fixed here, reported for the core team.

What we are asking for

These eight pull requests, merged in this order, before other changes land on main. Six exist today and two are ours to raise:

  1. Add the integration provider seam design spec #1084, the design set. No code, conflicts with nothing, mergeable today.
  2. Add a pluggable Edge Cookie provider seam with the built-in HMAC provider #1043, this one.
  3. Add device and geo provider selection with the host-signal Edge Cookie provider #1044, device and geo provider selection.
  4. Add the permission model with the Privacy Taxonomy vocabulary #1045, the permission model. Its vocabulary is not ours. It is the IAB Privacy Taxonomy Data Uses, with the IAB TCF Europe purposes mapped onto them where no Data Use exists yet, so the permissions a provider declares are stated in the industry's own terms rather than in a vocabulary we made up. Aligning the permission model this way followed a suggestion from Rowena.
  5. Add the client-set Edge Cookie value path #1046, the client-set Edge Cookie path.
  6. Add the provider documentation set and finish the decomposition #1047, the documentation set.
  7. The implementation of Add the integration provider seam design spec #1084, which we will raise. This is where identity, geo and device all become registration capabilities, so the second extension mechanism goes away rather than being documented.
  8. Add managed LiveRamp RampID integration #1054 reworked onto the seam, which we will raise. Someone else's vendor work, landing without enlarging the core, which is the whole point of the exercise.

Items 2 to 7 are one chain and cannot merge out of order. Item 1 is independent and can go first on its own. Item 8 needs the chain merged before it means anything.

This is the direction the Task Force agreed on 27 August, which is a neutral core with vendor-specific work in modules the vendors themselves own and maintain. These PRs are that direction in code. #1084 and its implementation are what make it possible for any vendor, not only us, and we have written that part at our own cost and contributed it.

The request to merge these first is practical rather than procedural, and the reason is Arena.

There is one deployment, and Arena is running on proof-of-concept code. It needs to be on an MVP and then a Release 1, and this stack is most of what moves it there. Every change that lands before this one is written against the proof-of-concept shape and has to be moved afterwards, and moving is far cheaper while there is one deployment than after there are more.

There is also a smaller running cost while the stack waits. Main took three commits on 28 August alone, and each one means rebasing seven branches. One of those rebases produced a merge that git resolved cleanly but that failed to compile, so the work is not mechanical.

Where this puts the project. The Task Force agreed the direction on 27 August, being a neutral core with vendor work in modules the vendors themselves own and maintain. Merged in the next few days, this stack delivers that at the start of September, as something done rather than something still being debated. That matters with the New York session at the end of the month, because it is the difference between presenting a direction and presenting a working answer, and it means the obvious awkward question, which is whether any of this is real yet, has already been answered.

It also gives Trusted Server capabilities nothing else in this space has, and they are worth saying out loud rather than leaving buried in a diff.

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.

@aram356

aram356 commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

The sequencing discussion for this series is on #1084. This PR is superseded rather than rejected. The design in §3.6 is accepted and most of the provider work carries over onto the reordered base. See #1084.

@aram356

aram356 commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Feedback left on spec #1084 Implementation should match spec after spec feedback is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants