Repository navigation
Add a pluggable Edge Cookie module seam with the built-in HMAC module - #1343
Open
jwrosewell wants to merge 81 commits into
Open
jwrosewell wants to merge 81 commits into
jwrosewell wants to merge 81 commits into
Conversation
…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)
`ec::get_ec_id` had no callers anywhere in the workspace, and this
branch loosened its filter to accept any well-formed `{code}~` value
with no ownership check against the selected provider. A future caller
picking it up would adopt another provider's identifiers, which
`EcContext` deliberately treats as absent.
The no-callers claim was checked across every crate in the workspace
(the four adapters, the CLI, core, the integration tests, openrtb) plus
benches, tests and docs. The only matches are for a different,
crate-private `edge_cookie::get_ec_id`, which reads the `x-ts-ec` header
as well as the cookie and is what `proxy.rs` and the testlight
integration call.
Deleted rather than realigned, for two reasons. The workspace sets
`publish = false`, so `trusted-server-core` is not distributed and
nothing outside this repository depends on the symbol. And aligning the
filter would mean calling `provider_owns_id`, which needs a
`&dyn EdgeCookieProvider` that a function taking only `&Request` cannot
obtain, so it would have meant changing the signature of a function with
no callers. `EcContext::read_from_request` already performs the
provider-aware read that production uses.
`parse_ec_from_request`, `is_valid_ec_id` and `log_id` all keep other
callers in the module, so nothing else becomes dead. The core README
line that advertised the helper is removed in the same commit.
Addresses: Aram Grigoryan review of PR 1043, crates/trusted-server-core/src/ec/mod.rs:137 (nitpick)
The `ec/provider.rs` module doc said a provider's constructor takes the services it needs, naming `RequestInfo` as the example, and its opening sentence was garbled where two half-sentences had been spliced together. `RequestInfo` is not a constructor argument. It is borrowed per call as the `request_info` parameter of `EdgeCookieProvider::generate`, so the first thing a vendor implementer read contradicted the trait they were about to implement. `evidence.rs` carried the same claim in its own words, that a constructor takes services as `Arc<dyn Trait>` supplied per request. Nothing in the workspace passes `RequestInfo` that way. Every use site is a `&dyn RequestInfo` argument. Both module docs now describe the real shape, which is construction once at startup from configuration or adapter injection, then borrowed request evidence on every call with nothing retained. The `evidence.rs` title changes to match, and its pointer to the borrowed view `BorrowedRequestInfo` is named alongside `OwnedRequestInfo`. Documentation only, no behavior change. `cargo doc --no-deps` reports no warning against either module. Addresses: Aram Grigoryan review of PR 1043, crates/trusted-server-core/src/ec/provider.rs:4 (nitpick)
The keys `"hmac"` and `"none"` were spelled as bare string literals at
four places: `Ec::validate_provider_selection`, `build_provider`,
`provider_owns_id`'s `provider.id() == "hmac"` check, and a private
`HMAC_PROVIDER_CODE` in `ec/generation.rs`. Nothing tied them together,
so a fifth built-in provider would add a fifth spelling and a typo in
any one of them would compile.
`EcProviderSelection { None, Hmac, Vendor(String) }` now holds the
vocabulary in `ec/provider.rs`, with `NONE_KEY` and `HMAC_KEY` as the
only places those two words are written. Vendor keys are open-ended, so
the catch-all `Vendor` variant takes any other key and
`#[serde(from = "String", into = "String")]` gives the enum an
infallible conversion in each direction rather than a hand-written
visitor. `HMAC_PROVIDER_CODE` moves next to it as a `ProviderCode`
const, built from `HMAC_KEY`, and `generation.rs` uses that instead of
its own copy. `HmacProvider::id` and `HmacProvider::code` return the
same two constants.
`Ec::provider` becomes `Option<EcProviderSelection>`, so the two
validation paths and `build_provider` match on variants rather than
comparing strings, and `Option` still distinguishes an absent selector
from an explicit `"none"` exactly as before.
The configuration surface is unchanged. The selector reads and writes
the same string, so an existing `trusted-server.toml` parses to the same
choice and a config push writes the same key back.
Tests: `the_selector_round_trips_through_serialization` parses `none`,
`hmac` and an arbitrary vendor key from TOML, checks each maps to its
variant, and checks each serializes back to the same string.
`each_selection_builds_what_its_string_key_built_before` proves the
three selections still build what they built before, which is nothing
for `none`, the built-in provider with the built-in code for `hmac`, and
the adapter-injected provider of that id for a vendor key.
Addresses: Aram Grigoryan review of PR 1043, crates/trusted-server-core/src/ec/provider.rs (refactor)
A provider's response headers were inserted into the outbound response without any check on what they set. A provider could return `Set-Cookie: ts-ec=...`, including on a request where it minted no identifier at all, and so write the managed identity cookie without going through core's identifier validation or its requirement that a minted identifier have an identity-graph row. It could also overwrite an `x-ts-*` header or a framing header. Core now defends by reserving its own namespace rather than banning `Set-Cookie`, because providers legitimately need cookies of their own. `reserved_response_effect` in `ec/provider.rs` classifies one header and rejects three things: a `Set-Cookie` naming a cookie in the `ts-` prefix core manages (`ts-ec`, `ts-eids`, `ts-tester`), a header in the `x-ts-` namespace core emits and strips, and a message framing or hop-by-hop header (RFC 7230 6.1 plus `content-length`, the same set each adapter's `is_hop_by_hop_response_header` uses). Everything else, a provider's own cookie included, passes through unchanged. The cookie name is read from the raw header bytes so a value that is not valid UTF-8 cannot smuggle a managed name past the check. A rejected effect fails the request rather than being dropped with a log. The check sits in `EcContext::generate_with_provider`, the only place provider headers are captured, next to the identifier-bounds check that already fails the request when a provider mints outside the cookie-safe alphabet. Both are the same kind of fault, a provider breaking its contract, and this branch has already decided that identity problems stop the request rather than serving without identity. Finalization cannot fail a request in any case, since it returns no result. Tests cover the classifier directly (managed cookie, reserved header, framing header, a non-UTF-8 `Set-Cookie`, and the allowed cases), and cover both halves through the organic generate path: a provider setting `ts-ec` with no identifier fails the request, and a provider setting its own `acme-evidence` cookie mints normally and has that cookie reach the response alongside core's own `ts-ec`. Addresses: Christian Pavilonis review of PR 1043, crates/trusted-server-core/src/ec/finalize.rs:57 (P2)
`is_valid_ec_id` is the built-in HMAC grammar and rejects every other provider code, yet pull sync, batch sync, and the admin lookup all called it directly. A deployment running a non-HMAC provider therefore minted and read identifiers on the organic path that these three paths skipped or rejected. PR #1044's `hs00~` host-signal provider makes that concrete. The check is now split in two, in `AcceptedProviders` in `ec/provider.rs`. The global cookie bounds, the length cap and the cookie-safe alphabet in `ec_id_has_only_allowed_chars`, apply to every identifier whoever minted it. The rest is dispatched by the `{code}~` prefix to the provider that owns that code, which canonicalizes its own value part and decides whether the canonical form is one of its own. Dispatch is on the code alone, before any provider inspects a value, so an identifier a partner echoed back in a different case still reaches its own provider to be canonicalized rather than being rejected first. KV normalization goes the same way through `canonical_kv_key`, so a row is always keyed by the owning provider's canonical form. A code no configured provider reads is rejected. The set of accepted providers is the deployment's active provider. `legacy_providers`, the design's list of readers that never mint, is not implemented on this branch (the key is rejected as unknown, see section 6.1 of the pluggable-providers design), so `AcceptedProviders::active` fills the reader list with the one active provider. The list is the seam: configured legacy readers are pushed alongside it and neither `accepts` nor `canonical_kv_key` changes. With no provider selected at all the deployment is stateless, and the built-in grammar stays the fallback, matching what `EcContext::accepts_id` has always done. Wiring: `EcContext::accepts_id` now goes through `AcceptedProviders`, so pull sync validates through it; `handle_batch_sync` and `handle_admin_ec_lookup` take the selected provider, which the Fastly adapter builds at both call sites. Tests cover a non-HMAC identifier accepted in pull sync, batch sync, and the admin lookup; a code neither active nor configured rejected in batch sync and the admin lookup, including one in the built-in HMAC shape; KV normalization dispatched to the owning provider (the built-in lowercases its hash segment, an opaque provider keys verbatim); and the global bounds rejecting before any provider is consulted. Addresses: Christian Pavilonis review of PR 1043, crates/trusted-server-core/src/ec/generation.rs:207 (P2)
`EcContext::generate_if_needed` failed the request whenever the host could not determine a client IP, before the selected provider was asked anything. `RequestInfo::client_ip` already defines the empty string as the unavailable state and providers are meant to read only the evidence they need, so the generic check blocked every header-, cookie-, query- and client-derived provider that can work without an IP. The requirement moves into `HmacProvider`, whose only input is the client IP. With none it fails rather than hashing the empty string into an identifier every visitor on that host would share. That failure propagates out of `generate_if_needed` exactly as the old check did, so a provider that genuinely needs the IP and cannot get it still fails the request rather than quietly minting nothing, matching this branch's decision to stop rather than serve without identity. Providers that read other evidence now receive the documented empty value and run. `HmacProvider` is the only provider on this branch that reads the client IP; the injected vendor seam leaves the decision to each vendor crate. Tests cover both answers: a provider deriving identity from the request query and cookies mints on a host with no client IP, and the built-in HMAC provider refuses on the same host with no identifier committed. A `noop_services_with_ec_provider_without_client_ip` test helper models that host. Addresses: Christian Pavilonis review of PR 1043, crates/trusted-server-core/src/ec/mod.rs:376 (P2)
The comment on `provider_owns_id` promised bare-HMAC compatibility for one release cycle, which the code cannot honor. A returning visitor's bare cookie is never rewritten into the coded form, so the promise was shorter than the cookie's own life. The comment now states the condition the quantities actually support, read off the code rather than estimated. Neither the cookie nor its identity-graph row is refreshed on an ordinary page view (see `ec_finalize_response`), so each has one fixed lifetime from the moment it was written: `COOKIE_MAX_AGE` in `ec/cookies.rs` and `ENTRY_TTL` in `ec/kv.rs`, both one year and neither operator-configurable. The earliest safe retirement is one year after the last release that could still mint a bare identifier has stopped running anywhere, plus the deployment's own rollout skew. The comment also says plainly that the second half of the condition cannot be checked: nothing counts or logs a bare-form read-back, so there is no observed legacy-reader traffic to look at and elapsed time alone proves nothing. No metric is named that is not emitted. The reader stays, at the cost of one string comparison per read-back, and the one-release wording is gone from the comment and from the provider code registry. A new test in `ec/cookies.rs` pins `COOKIE_MAX_AGE` to one year, next to the existing `ENTRY_TTL` assertion in `ec/kv.rs`, so the two figures the retirement condition is written in terms of cannot drift unnoticed. Addresses: Christian Pavilonis review of PR 1043, crates/trusted-server-core/src/ec/provider.rs:145 (non-blocking)
The lifecycle contract routes every identity-graph row through the owning
provider's canonical form, and generation already did: it keys the row it
creates with `provider_kv_key`. Three read and write-back paths did not.
`handle_identify` read with the raw cookie value, the withdrawal
tombstones in `ec_finalize_response` were written under the raw value,
and EID ingestion keyed its upsert by the raw value too.
Raw and canonical are the same string for the built-in HMAC provider, so
nothing misbehaved. For the first provider whose canonical form differs
from the cookie value, which is exactly the case the mint test on this
branch already pins, identify missed the row generation had written, an
ingested EID was dropped because the upsert found no row under the raw
value, and a withdrawal tombstone landed on a key no live row used, so
the revocation never took effect.
The key is now derived in one place, `EcContext::kv_key_for`, reached by
`ec_kv_key` for the active identifier and `cookie_ec_kv_key` for the
`ts-ec` cookie the request carried. Both go through `AcceptedProviders`,
so the owning provider is picked by the identifier's `{code}~` prefix and
supplies the canonical form of its own value part. Identify, the
tombstones, and both EID ingestion call sites use them.
`AcceptedProviders` came from the partner-path dispatch commit and fixed
none of these three; it changed pull sync, batch sync, and the admin
lookup only. What it did give this fix is `canonical_kv_key`, the
code-dispatched derivation these paths now share, and it also subsumes
the shape filter `withdrawal_ec_ids` applied by hand: a key exists
exactly when some provider this deployment reads owns the identifier, so
`withdrawal_kv_keys` filters by deriving.
The mint test now asserts `ec_kv_key` returns the key generation actually
wrote, so the read side and the write side cannot drift apart.
Tests: identify finds the row generation keyed by the canonical form and
still echoes the cookie value to the partner; a withdrawal tombstones the
canonical row and writes nothing under the raw cookie value; an ingested
EID joins the canonical row. Each was run against the unfixed code first
and each failed there.
Addresses: Aram Grigoryan review of PR 1043, crates/trusted-server-core/src/ec/identify.rs:89 (wrench)
Section 3's Recognize row says a value the selected provider does not recognize "is never used or egressed". Three paths egressed one anyway. `append_ec_id` put the raw `ts-ec` cookie or `x-ts-ec` header on the outbound origin URL, `handle_first_party_click` put it on the click target's redirect URL, and the testlight integration put it in the proxied body as `user.id`. All three read through `edge_cookie::get_ec_id`, which checks the cookie-safe alphabet and the length cap and nothing else, so a value carrying another deployment's provider code (`zz00~...`), and any cookie at all in a deployment with no provider selected, was handed on. The code changes rather than the claim. `edge_cookie::recognized_ec_id` reads the value and then asks the selected provider whether it owns it, through `provider_owns_id`, which is the same test `EcContext` applies when it reads the cookie back, so the egress paths and the EC lifecycle agree on what this deployment issued. All three call sites use it. Behavior change: a deployment with no Edge Cookie provider selected now forwards no `ts-ec` value at all, on any of the three paths. It previously forwarded whatever the browser sent. An operator running stateless and relying on the raw cookie reaching the origin, the click target, or the testlight upstream will see that value stop arriving, and testlight, which requires an identifier, will fail the request rather than proxy it. The fix for such a deployment is to select a provider, which is what makes the value this deployment's to hand on. The testlight call site is in scope on the evidence rather than by assumption: its value is written into the request body as `user.id` by `rewrite_request_body` and that body is POSTed to the operator-configured endpoint, so the identifier leaves the edge even though the integration sets `forward_ec_id = false` (which suppresses only the query-parameter copy on the same request). The spec's Recognize row now names the three egress paths in the column that says where core applies recognition, and records that a stateless deployment recognizes nothing and so egresses nothing. The claim is left as strong as it was. Tests: each of the three paths, with a foreign-coded value and with a stateless deployment, plus a positive control on each that the deployment's own identifier still gets through. The testlight cases assert no upstream call is made at all. `click_appends_ec_id_when_present` used `ec-123`, which no provider issues, and now uses an identifier the built-in HMAC provider owns. Every new test was run against the unfixed code and failed there. Addresses: Aram Grigoryan review of PR 1043, crates/trusted-server-core/src/proxy.rs:1263 (question)
Section 3 said the pre-epic IP-cluster prefix listing "continues unchanged". The listing does, but the key space it lists over does not. A fresh mint is keyed `hmac~<hash>.<suffix>`, so the prefix `evaluate_cluster` derives is `hmac~<hash>` for a coded row while a legacy bare row still lists under `<hash>` alone. Prefix matching is anchored at the start of the key, so two rows for the same client IP that straddle the envelope never count each other and `cluster_size` under-reports while both populations coexist. The decision is to accept the undercount rather than bridge it, and the spec now says so along with the bound and the reasoning, and the prefix derivation in `evaluate_cluster` carries the same note so the next reader of that line is not surprised by it. The "gates nothing" half of the reasoning was checked rather than assumed. Every read of `cluster_size` in the workspace is a store, a log line, or the optional field in the identify response. The single read that reaches a branch is the cache short circuit in `evaluate_cluster` itself, which tests whether a value is stored, not what it is, so `Some(1)` and `Some(100000)` take the same path. There are no matches at all in the TypeScript or the integration tests. Two settings look like they gate on it and do not: `cluster_trust_threshold` (whose doc comment says entries at or below it "are treated as individual users for identity resolution") and `cluster_recheck_secs` are parsed and defaulted but have no readers anywhere in the code. They are noted here because they are what would make a reader believe the count is a control. They are outside this change; the unimplemented threshold wants an issue of its own. Addresses: Aram Grigoryan review of PR 1043, crates/trusted-server-core/src/ec/kv.rs:715 (thinking)
The two egress tests added in the previous commit dropped the `Report<TrustedServerError>` that `expect_err` returns on the floor. `Report` is `#[must_use]`, so building the library's test target warned, and clippy runs with `--all-targets -- -D warnings`, which would have failed the CI gate rather than only warning.
The spec's minimalism rule wants a production caller in the same change that introduces a method. `RequestInfo` arrived with seven accessors and only one of them, `client_ip`, is read by production code, in `HmacProvider::generate` at crates/trusted-server-core/src/ec/provider.rs. The other six had no non-test caller anywhere in the workspace on this branch as it stands: - `user_agent()` and `header_names()` had no caller at all, test or otherwise, beyond their two implementations. - `header()` and `query_param()` were called only by two test doubles in `ec/mod.rs`, both inside `#[cfg(test)]`, plus evidence.rs's own tests. - `path()` and `query()` were called only by evidence.rs's own tests, and `query()` by the default body of `query_param()`, which nothing called. Note the file is crates/trusted-server-core/src/evidence.rs; there is no `ec/evidence.rs`. The many `.path()`, `.query()` and `.header()` hits elsewhere in the workspace are `http::Uri`, `http::request::Builder` and the unrelated `http_util::RequestInfo` struct, which has `host` and `scheme` fields and none of these methods. All six are removed, along with everything that existed only to feed them: the `headers`, `path` and `query` fields and the `with_request_target` builder on both `OwnedRequestInfo` and `BorrowedRequestInfo`, the header snapshot argument of `OwnedRequestInfo::new`, `BorrowedRequestInfo::new` and the test-only `edge_cookie::generate_ec_id`, and the `request_headers` / `request_path` / `request_query` snapshot `EcContext` took at read time to fill them. Leaving state a provider can no longer read would be worse than the accessors themselves. Two test doubles went with them, `CookieCapturingProvider` and `EvidenceCapturingProvider`, along with the two tests that existed to prove the removed accessors carried cookies and query parameters. The third test that used `EvidenceCapturingProvider`, `a_provider_that_reads_no_client_ip_mints_when_the_host_has_none`, tests something else (a provider that needs no client IP still mints on a host that has none), so it stays, now with a `NoClientIpProvider` double that also asserts such a host passes the documented empty string rather than failing. The trait keeps its role as the seam. Its docs, the provider module docs and section 4 of the design now say that further evidence arrives as a defaulted accessor in the change that first reads it, rather than claiming a provider can already read headers, cookies, client hints and the URL. Probe: removing `client_ip` from the trait fails the library build at ec/provider.rs, where `HmacProvider::generate` reads it. Removing the other six failed nothing outside the tests deleted with them, which is the asymmetry this commit is about. Addresses: Aram Grigoryan review of PR 1043, crates/trusted-server-core/src/evidence.rs:27 (thinking)
Every Edge Cookie provider goes through one mechanism and none of them is special, so the selector no longer carries a variant for the built-in HMAC provider. `EcProviderSelection` is now `None` for explicit statelessness and `Named(String)` for a provider chosen by name, and `hmac` is an ordinary name in the same open-ended namespace a vendor crate names its own provider from. A variant per provider wrote the special case into the type, so every match on it had to know that one provider was different, and the built-in provider is due to become a vendor-supplied module rather than living in core. The one place that still knows `hmac` is built into core is the resolution in `build_provider`, lifted into `resolve_named_provider` and commented to say that it goes when the built-in provider becomes a module, after which `hmac` resolves through the injected path like any other name. Nothing else branches on whether a name is built in. `Ec::validate_provider_selection` is now a name lookup through the new `EcProviders::has_block`, and the unreferenced-block check reads the new `EcProviders::configured_keys` rather than pushing `hmac` in by hand, which also drops `has_vendor` and `vendor_keys`, both of which only answered for names that are not built in. `EcProviderSelection::HMAC_KEY` becomes the module-level `HMAC_PROVIDER_KEY` beside `HMAC_PROVIDER_CODE`, because the selection type should not name any one provider. The configuration surface does not change. `[ec] provider = "none"`, `"hmac"` and any vendor key parse to the same behavior and are written back as exactly the same string, which the round-trip test now proves on the serialized scalar itself rather than only on the surrounding document text. This is not the reviewer's finding about scattered string literals, which the typed selector already fixed. It is the project's own rule that no provider is special.
The response EC finalization edits is the finished one, so it already carries whatever the publisher's origin returned. The provider-header loop used `HeaderMap::insert`, which drops every existing value for that name, so a provider setting one evidence cookie deleted every `Set-Cookie` the origin had written, a publisher's session and sign-in cookies included, and a provider setting `Vary` deleted the origin's. `response_headers` is a list of pairs precisely so a provider can set more than one cookie, and `insert` collapsed those too. The rule, written out on the new `apply_provider_response_headers`, is that this seam is additive. A provider only ever adds evidence about the request, it never corrects the origin's output, so core has no grounds to discard a value it did not write. `Set-Cookie` can never be folded into one field line, the list-valued headers a provider realistically sets (`Vary` above all) mean the union of their field lines, and the single-valued headers where replacing would be right are exactly the ones `reserved_response_effect` already fails the request for. So nothing a provider may set here needs to replace, and appending is the direction that cannot silently destroy someone else's header. No test anywhere covered a provider header reaching a response. The new one drives a provider that sets its own cookie and its own `Vary` through the real mint path onto a response the origin has already written to, and asserts the origin's cookie, the provider's cookie, core's own `ts-ec` and both `Vary` entries all survive.
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 (#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.
`create_url_replacer` lists four replacements for a CSS, JavaScript, JSON or other text response that core rewrites without parsing it: the origin's `https://` URL, its `http://` URL, its protocol-relative address and its bare host. `StreamingReplacer` made them one after another, each reading the previous one's output. When the request host contains the origin host, a later replacement matched inside an earlier one's output and wrote the rest of the request host again. With `publisher.example` served as `publisher.example.localhost` over `http`, `https://publisher.example/` came out as `http://publisher.example.localhost.localhost.localhost.localhost/`, and with `example.com` served as `www.example.com`, `https://example.com/` came out as `https://www.www.example.com/`. `replace_each_once` scans the text once from the start. It takes the earliest match of any replacement, the longer of two that start at the same place, writes that replacement and carries on after the matched text, so no replacement is read again. It keeps each replacement's next match until the pass moves beyond it, so the text is still searched once for each replacement. The comments of `test_pattern_priority` and `test_overlapping_patterns`, which said the replacements were applied in order, now say why their results hold. The tests check that a request host which begins with the origin host is written once in every form over both schemes, whole and in chunks of seven bytes, that a hyphenated host is written as before and a `www.` host over an apex origin no longer gains a second `www.`, and `replace_each_once` on its own, with its replacements listed in either order.
This was referenced Oct 17, 2026
* Require agent-reviewed specs for feature PRs Trusted Server spans four runtime adapters, consent, identity, auctions, HTML rewriting, and browser bundles, so design problems are cheaper to catch in a spec than in code review. Feature pull requests now need a spec written with the superpowers brainstorming skill and reviewed with an AI coding agent over at least two rounds. Add the requirement to CONTRIBUTING.md, a banner to README.md, and a Spec field and checklist item to the pull request template. * Match spec review round count across the guide * Move spec banner to the Development section * Place spec banner after the development commands
12 of 14 tasks
Edge Cookie identity is off in the example settings that ts config init writes, so no Edge Cookie secret was looked up, removing ec_passphrase changed nothing, and each smoke's missing-ec case could not get the failure it expects. smoke_initialize_config now selects the built-in hmac module and uncomments its [ec.hmac] block, and stops when the example no longer holds those lines. The four smokes expect the path the secret is resolved at, ec.hmac.passphrase. The Cloudflare and Fastly guides say the smoke selects the module.
The built-in hmac module writes hmac~ in front of every identifier it creates, and the guide still gave the bare format. It now gives the format the code writes, and says, as the setup guide does, that a bare value with no prefix still resolves.
* Add documentation proposal automation design and plan Record the design and implementation plan for issue #1105. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Propose documentation updates after code merges to main Add a Documentation proposal workflow that runs GitHub Copilot CLI on each code merge to main, proposes edits to docs/guide or docs/index.md, and opens one pull request per merge on docs/auto/<sha12>. The publish job runs the docs gates first, skips empty diffs, and never reopens a closed proposal. It also posts a review comment on every hunk that cites the merged source and offers a suggestion reverting the hunk. Closes #1105. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Isolate documentation proposal builds from publication credentials Split the docs gates into a validate job with a read-only token and no persisted credentials, since VitePress executes Vue in generated Markdown. The publish job now re-checks the path allowlist on a fresh checkout without building. Every job runs the scripts from the workflow's revision against a separate checkout of the merge commit, so historical merges can be retried. Queue every pending run instead of cancelling all but one, and have Copilot inspect the full pushed range so merge commits and multi-commit pushes are not missed. A retry with an unchanged tree now reuses the pushed commit and posts the review if an earlier run failed before posting it. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Keep maintainer commits on proposal branches across retries A retry for the same merge regenerated the proposal and force-pushed it, discarding commits maintainers added on top, including applied revert suggestions. An empty rerun also closed the pull request and deleted that branch. publish.sh now leaves a branch alone when its head is not the generated proposal commit, and pushes with a lease instead of a bare force. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Stop allowing Copilot to run git grep git grep --open-files-in-pager (or -O) runs an arbitrary command, so allowing shell(git grep:*) let content in the merged change steer Copilot into executing commands in the propose job. Copilot's built-in file reads cover what git grep was used for. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Drop the npm cache from jobs that run untrusted content Copilot can write files in the propose job that propose.sh then executes, and validate builds generated Markdown that VitePress runs as Vue. Either job could therefore save a poisoned main-scoped setup-node cache entry that later runs restore. Neither job restores or saves a dependency cache now, and the design spells out that the job, not the tool allowlist, is the trust boundary. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Bound documentation proposal jobs with timeouts Copilot CLI has no limit of its own, so a hung run inherited the 360-minute default and, with the concurrency queue, delayed every later merge's proposal. The propose, validate, and publish jobs now time out after 30, 15, and 10 minutes. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Neutralize mentions in the proposal rationale Evidence already had its @ mentions broken because it is model output, but up to 20 KB of Copilot's rationale went into the pull request body verbatim, so a rationale naming a team would ping it on every proposal. hunks.mjs gains a sanitize subcommand that inserts a zero width space after each @, which the PR body helper now pipes the rationale through. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Refuse proposal retries that inspect a different range A manual dispatch had no base, so retrying a multi-commit push such as a rebase merge narrowed the inspected range to the final commit. The retry could then miss documented behavior, or produce an empty patch that closed and deleted a valid proposal. The dispatch gains an optional base input. propose.sh records the resolved base in the artifact and warns when it discards a supplied base. publish.sh records the base in the pull request body and fails, naming the original base, when an open proposal was generated from a different range. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Document the proposal scripts CI gate and first-run prerequisites AGENTS.md now lists the docs-proposal scripts job among the CI gates. scripts/README.md names all three scripts the workflow runs, and warns that the run triggered by merging the workflow fails unless the Copilot policy and pull request setting are already enabled. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Fill the proposal work directory into Copilot prompts propose.sh accepts any work directory, but both prompts told Copilot to write its outputs under .docs-proposal/. The prompts now use a <work-dir> placeholder that propose.sh replaces with the work directory relative to the repository root. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Guard empty-rerun branch deletion with a lease An empty rerun deleted the proposal branch via gh pr close --delete-branch, so a maintainer push after the head inspection was lost, and a failed fetch hid existing maintainer commits. Look up the branch with git ls-remote --exit-code and fail unless it is verifiably absent, delete it with --force-with-lease on the inspected head, and close the pull request only after the deletion succeeds. Add regressions for a concurrent maintainer push, fetch failures on the empty and push paths, and the lease-guarded deletion. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Harden documentation proposals against forks, symlinks, and live rationale Ignore fork pull requests that reuse the proposal branch name, reject non-regular files such as symlinks, and render the rationale as an inert text block. Comment before closing an emptied proposal, keep a proposal when the evidence run fails, resolve a manual base to a full SHA, and fill prompts portably across bash versions. Ask for deletion instead of an empty suggestion on new pages, drop the npm cache from the Pages deploy, mark the original plan superseded, and cover the evidence half, symlinks, and fork pull requests in the script tests. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Harden docs proposal evidence rendering and test prerequisites Render evidence reasons as inline code so links, images, HTML, and issue references stay inert, tell Copilot new pages need a sidebar entry, fail test.sh early when jq is missing, and document that the propose job allows arbitrary code execution and that privileged workflows must not restore caches. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Update docs/superpowers/specs/2026-10-01-docs-proposal-automation-design.md Co-authored-by: AG <132480+aram356@users.noreply.github.com> * Update scripts/docs-proposal/lib.sh Co-authored-by: AG <132480+aram356@users.noreply.github.com> --------- Signed-off-by: dhruv8sh <dhruv8sh@proton.me> Co-authored-by: prk-Jr <49094961+prk-Jr@users.noreply.github.com> Co-authored-by: AG <132480+aram356@users.noreply.github.com>
* Rewrite publisher origin URLs in proxied response headers Publisher pages had origin URLs rewritten in the body but not in headers, so browsers followed Location and Refresh to the origin, preloaded Link targets from it, and blocked rewritten resources under the forwarded CSP. Map origin URLs in Location, Content-Location, Refresh, and Link (including imagesrcset) to the serving host on every publisher response, and add serving-host sources beside origin sources in Content-Security-Policy and its report-only variant. Rewriting runs before the template-cache gate so cached templates replay the rewritten policy headers; bump the template schema to 6 so entries stored with origin-only policy metadata are not replayed. Leave a Location or Refresh target unchanged when it names the current request URL under a different scheme than the origin, so an origin scheme upgrade cannot loop the browser. Closes #1128 Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Normalize navigation targets before the redirect-loop check A scheme-change redirect such as https://origin.example.com?x=1 or one with dot segments was compared to the request path as raw text, so it did not match /?x=1 and was rewritten back to the URL the browser had just requested, looping. Parse the target as a URL so the comparison sees the same path and query the browser will follow. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Compare CSP source paths case-sensitively when deduplicating Deduplication compared whole source expressions case-insensitively, so a policy listing origin paths /A.js and /a.js gained a serving-host sibling only for /A.js and the rewritten /a.js stayed blocked. Compare scheme and host case-insensitively and the path exactly, as CSP path matching does. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Log the scheme-change redirect escape at warn level When the origin forces a different scheme than publisher.origin_url, every navigation leaves the serving host and bypasses Trusted Server. That almost always means origin_url has the wrong scheme, so surface it as a warning naming the setting to check. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Note that CSP report-to is also left unchanged Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Document the Set-Cookie Domain redirect limitation Same-scheme redirects to the current URL are now kept on the serving host, but Set-Cookie Domain attributes are not rewritten. A cookie scoped to the origin host is rejected on the serving host, so a cookie-gated self-redirect repeats. State this in the navigation rewrite docs and the configuration guide. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Keep cookieless same-URL GET redirects off the serving host A GET or HEAD redirect to the current URL that sets no cookie loops through Trusted Server when the origin redirects on something sent unchanged, such as an origin Host override. Leave its Location unchanged and log a warning. Also allocate lazily when rewriting Link values, reuse the captured request path and query, document the meta tag gap and the proxy reach of serving-host CSP sources, and add a changelog entry. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Keep host-override redirects to the current URL even when they set a cookie When publisher.origin_host_header_override sends a Host that differs from the origin host, a Location naming the origin host and the current path is a host change the retry cannot satisfy, so cookies such as load-balancer stickiness no longer re-enable the rewrite. Also cover HEAD in the cookieless redirect test. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Distinguish Host-override and cookieless self-redirect warnings Log a separate warning for each reason an origin redirect to the current URL is kept, so an operator can tell a Host-override mismatch from a cookieless self-redirect. Rewrap the loop-exception paragraph in the configuration guide. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> --------- Signed-off-by: dhruv8sh <dhruv8sh@proton.me> Co-authored-by: AG <132480+aram356@users.noreply.github.com>
…1259) * Give each Rust CI job its own cache and save caches only from main All Rust jobs shared one rust-cache key, so the fastest job saved it and every slower job restored a cache built for a different target, logged "Cache up-to-date", and rebuilt its dependencies on every run. Use a per-job shared key, restrict saves to main so PR-scoped caches stop evicting main's, and uninstall the runner image's preinstalled stable toolchain so image rollouts no longer rotate the cache key. Integration tests restore the axum job's cache, which builds the same artifacts. Closes #1258 Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Move pinned Rust setup into a shared composite action Make the integration cache restore-only so a workflow_dispatch run on main cannot claim the axum key, and unset the rustup default after removing stable so rustup cannot silently reinstall it. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Require Rust jobs to use the shared setup-rust action Add a step to the clippy job that fails when any workflow or action calls setup-rust-toolchain or rust-cache directly, so new Rust jobs cannot skip the stable removal and main-only cache save gate. Signed-off-by: dhruv8sh <dhruv8sh@proton.me> * Catch quoted uses values in the shared Rust setup guard Signed-off-by: dhruv8sh <dhruv8sh@proton.me> --------- Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
crates/trusted-server-core/srcdocs/superpowersscripts/docs-proposaldocs/guide.github/workflowscrates/trusted-server-adapter-spin/srccrates/trusted-server-adapter-fastly/srccrates/trusted-server-adapter-axum/srccrates/trusted-server-adapter-cloudflare/srcCONTRIBUTING.mdcrates/trusted-server-adapter-cloudflare/testscrates/trusted-server-adapter-axum/testscrates/trusted-server-adapter-spin/tests.github/actionsscripts/README.mdtrusted-server.example.tomlscripts/smoke-fastly.shcrates/edgecookie/README.mdcrates/trusted-server-integration-tests/testscrates/trusted-server-cli/tests.claude/skillsCargo.lockscripts/smoke-common.sh.github/pull_request_template.mdcrates/trusted-server-integration-tests/fixturesfastly.tomlCargo.tomledgezero.tomlAGENTS.mdcrates/trusted-server-adapter-cloudflare/build.shcrates/trusted-server-js/build.rs.gitignoreREADME.mdscripts/template-cache-local-test.shCHANGELOG.mdcrates/trusted-server-cli/srcscripts/smoke-axum.shscripts/smoke-cloudflare.shscripts/smoke-spin.shCloses
Closes #778
Spec
Spec: pluggable-providers
Test plan
CI on this pull request at ec6bdab passed all 29 checks.
cargo test-fastly && cargo test-axumcargo clippy-fastly && cargo clippy-axumcargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest runcd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute serveChecklist
unwrap()in production code (clippy deniesunwrap_usedin CI)logmacros as AGENTS.md asks (notprintln!)Stacks on the design documents (#1342), whose pluggable providers spec it
implements. With the four pull requests after it (#1344 to #1347) it
decomposes the combined module change (#838), as the review of the earlier
design specs (#986) asked, and the integration seam (#1348) opens on top of
them.
The design specs for the series are carried by the spec pull request (#1342).
The spec for this pull request is
2026-07-30-pluggable-providers-design.md,which covers both this pull request (the Edge Cookie seam) and the device and
location seam (#1344). The spec is revised to match this implementation, with a
revision record listing every divergence from the earlier draft and why.
What this pull request does
Edge Cookie identity generation becomes a selectable module behind the
EdgeCookieModuletrait incrates/trusted-server-core/src/ec/module.rs,with the existing HMAC implementation as the built-in and configuration
selecting it.
How a module is selected
[ec]follows the convention the rest of the stack follows:[ec] modulenames one module, and[ec.<name>]holds that module'ssettings. Leaving
[ec] moduleout means stateless operation with no EdgeCookie, and
module = "none"spells the same choice explicitly.carry
implementation = "<id>"and take a name of its own, which is whatlets a deployment label a module for what it does rather than for what it
is.
by
., each of lower case letters, digits,_or-, and anything elseis refused at startup with the rule in the message.
key a module does not know all fail at startup, so a misconfiguration is
loud rather than silent.
[ec]keeps its own settings besidemodule,being the store name, the partner registry and the cluster thresholds, and
those are not mistaken for module tables.
[ec] passphraseform still starts. It migrates tomodule = "hmac"plus[ec.hmac]with a deprecation warning, so a fleetcan move configuration and binaries independently. Both forms together are
rejected. In either form the passphrase names a secret-store key, as
[ec] passphrasedoes onmain, and the resolved value is held to the same32-byte minimum.
The identifier and the identity graph
and the alphabet
[A-Za-z0-9._~-], enforced at creation, cookie read-backand cookie write. A violating identifier is rejected outright and never
rewritten, so the cookie value and the identity-graph key can never silently
diverge. The previous sanitize-by-stripping path is removed.
accepts_id, and theidentity-graph key through its
normalize_id_for_kvcanonical form, so anopaque vendor identifier round-trips byte for byte. One test proves a
non-default module round-trips verbatim, another proves the graph is
keyed by the canonical form, and a third proves that two issues of one
identity share one key when the module returns the stable part.
[ec.<name>]tables are captured as raw values in core anddeserialized by the adapter that injects the vendor module, so core never
names a vendor.
in
provider-code-registry.md,the registry the spec set defines. Core creates
{code}~value, checks thecode at read-back, and keys the identity graph with it, so identifiers from
different modules can never collide and switching modules cannot silently
adopt another module's identities. The built-in module creates
hmac~<hash>.<suffix>and still reads its earlier bare form so deployedcookies keep working.
module's canonical form. Pull sync, batch sync and the admin lookup accept
an identifier through
AcceptedModules, which applies the global boundsand then asks the module that owns the identifier's code, so a code no
configured module owns is refused. Identify, Edge Cookie finalization, pull
sync, batch sync, the admin lookup,
/auction, the publisher navigationpreload and
/_ts/page-bidsall use the canonical key, so a module whosecanonical form differs from its cookie value still reaches the row it
created. The admin lookup reports the key it read as
kv_key.is_valid_ec_idstays the built-in HMAC grammar, accepting thehmac~envelope and the legacy bare form.
main's identity-graph flow, EID ingestion, orphan recovery and thewithdrawal tombstones are keyed the same way, so an ingested EID lands on
the live row and a withdrawal tombstones the row the identifier is stored
under.
What a module may do to the response
inside its own surface, being a
Set-Cookiefor ats-cookie, anyx-ts-header, and the framing, hop-by-hop and
cache-controlheaders. A refusedeffect makes generation return an error, and the page is served without an
Edge Cookie. Headers are kept only from a module response whose identifier
generation commits, or that produced no identifier, and finalization appends
them rather than replacing the origin's.
empty string when the host has none, and each module decides whether it
needs one. The built-in HMAC module refuses to create an identifier without
one.
the request's headers, path and query, the evidence it reads at creation.
Framing
Privacy is a spectrum, and this change is neutral infrastructure. It does not
decide whether identity is created. It makes that decision configurable and
inspectable, and the deployer selects a module, or none, according to the
laws and policies that apply to them. Trust comes from that flexibility being
respected and visible in configuration rather than hard-coded.
References #777 and #778. Decomposes #838, which is kept as a draft reference until this
series merges. Spec baseline from #986.