Open the integration seam so a vendor module can live outside core - #1094
Open
jwrosewell wants to merge 98 commits into
Open
Open the integration seam so a vendor module can live outside core#1094jwrosewell wants to merge 98 commits into
jwrosewell wants to merge 98 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 IABTechLab#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 (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.
`config.rs` named every vendor by hand: a per-vendor import block, a `#[cfg(test)] DEPLOY_VALIDATED_INTEGRATION_IDS` list a test compared the registry against, and a `validate_enabled_integrations` body that called `validate_integration::<T>` once per integration. A vendor integration living in its own crate could not be reached by any of it, and the list was a second place to keep in step by hand. Deploy validation now walks the builders. `validate_settings_for_deploy` delegates to a new public `validate_settings_for_deploy_with(settings, extra_integrations, extra_auction_providers)`, which runs `crate::integrations::all_builders(...)` for the integrations and `crate::auction::all_provider_builders(...)` for the auction providers, collecting the enabled provider names the auction builders report. Every builder validates whether or not it is enabled, so a typo in a disabled block is still caught, as before. `validate_auction_provider_names` and `report_to_validation_errors` are unchanged; the per-vendor imports, `DEPLOY_VALIDATED_INTEGRATION_IDS`, `validate_enabled_integrations`, `validate_prebid` and `validate_integration` are gone. `IntegrationBuilder::validate` and `AuctionProviderBuilder::validate` now have a caller, so both lose their temporary `#[allow(dead_code)]`. The enabled auction provider set is unchanged. Each vendor `validate` is the same expression the deleted code used (prebid via `validate_config_for_startup(...).is_some()`, aps and adserver_mock via `integration_config::<T>(id).is_some()`, with the id consts holding the same literals), and a temporary parity test compared the deleted shape against the builder shape over all 27 enabled/disabled/absent permutations of prebid, aps and adserver_mock plus three invalid blocks, matching on both the resulting set and the rejection text. `adserver_mock` is the one id that is not an integration builder; it is validated through its auction provider builder. `deploy_validation_covers_registered_integration_builders` compared the hand list against the registry, which no longer exists. `deploy_validation_runs_every_registered_builder` replaces it, asserting the set deploy validation walks equals the set the registry registers. Three new tests cover the seam: an external integration builder's rejection and an external auction provider builder's rejection both reach the caller with the message intact, and an external auction provider satisfies a configured `[auction] providers` name that fails without it.
…in tests The filter now inserts `PersonalizedResponse` alongside `DataDomeClientTagSuppressed` at two sites, but every assertion in `protection.rs` went through `has_client_tag_suppression_marker`, which reads only the DataDome type, and the publisher tests insert both markers by hand. Deleting either `PersonalizedResponse` insert left the whole suite green while HTML responses that must stay private became shareable through a cache or a template. What used to be structural, one type driving both concerns, had become a convention at two sites with nothing holding it. Add `has_personalized_response_marker` next to the DataDome helper and assert the two together at all fifteen sites, positive and negative, so both markers are checked everywhere the filter is exercised. Verified not vacuous by removing each insert in turn: dropping the test-bypass insert fails three tests, dropping the scope-skip insert fails two. Both restored. Each site also takes one `extensions_mut()` binding instead of two calls, which puts the paired inserts next to each other. `deploy_validation_runs_every_registered_builder` could not fail. Both sides derived from `BUILT_IN_BUILDERS` through different paths and it never called `validate_settings_for_deploy_with`, so its claim to guard against a builder being deployed unvalidated was not one it could make. Replace it with two tests that can fail. `deploy_validation_runs_every_builder_it_is_given` passes recording builders through `validate_settings_for_deploy_with` and counts the calls in a static, since a builder holds fn pointers and cannot capture. `deploy_validation_reaches_every_built_in_builder` plants a block each config type cannot deserialize, a string in the boolean `enabled` field, and asserts deploy validation rejects for every built-in integration and auction provider id. Verified not vacuous: gutting the integration validate loop fails five config tests, skipping the auction validate fails three. Its doc records what it cannot catch, a builder deleted from `BUILT_IN_BUILDERS`, because no independent list of the built-ins exists in the crate. The `PersonalizedResponse` doc overstated the mechanism. Buffering is gated on `is_html_document_request` and private caching on a body-carrying HTML content type, while only the origin-path choice is unconditional. Reword to say so.
BidRenderer was a closed enum with a single Aps variant, and the APS
descriptor types sat in the neutral auction types, so an APS integration
could not move into its own crate without taking core's renderer enum
with it.
BidRenderer is now a type tag plus a payload the auction provider that
produced the bid defines, serialized flat as {"type": "<tag>", ...payload}.
Providers build one with new() from JSON or from_typed() from their own
struct, and read one back with payload_as::<T>(tag), which returns None
under a different tag. A payload must be a JSON object and must not carry
its own "type" key, since that would collide with the tag.
The serialized bytes are unchanged. The page receives the renderer through
serde_json::to_value, in the OpenRTB response extension via BidExt::to_ext
and in window.tsjs.bids via build_bid_map, and both forms produce the same
JSON object with the same keys and values. The exact strings were captured
from the enum form before the change and are pinned by a test, and the
existing wire-shape assertions in auction/types.rs and auction/formats.rs
pass unchanged.
ApsRendererV1 and ApsTagType moved from auction/types.rs into
integrations/aps.rs, with a shared APS_RENDERER_TYPE tag constant, so they
travel with APS when it moves out. adserver_mock builds the neutral
descriptor from the wire keys instead, since the mock is not APS.
Every adapter used to call one named integration, GPT diagnostics, directly on the request path before routing, which kept a vendor-shaped integration wired into all four entry points. The registry now collects the request preparers the integration builders carry and runs them in registration order, so the adapters call `state.registry.prepare_request(...)` and no longer name an integration. Preparers are collected before the enabled check, so they run whether or not their integration is enabled. That is the behavior the adapters had, because an integration must be able to strip its own reserved query or cookie in a deployment that has it switched off, which is exactly what GPT diagnostics does with `ts_console` and `__Host-ts-console`. GPT diagnostics is now attached as a builder hook, `with_request_preparer(gpt_diagnostics::prepare_request_hook)`, in the built-in table. The hook discards the decision, which stays in the request extensions for the publisher path to read, and core still calls `gpt_diagnostics::prepare_request` directly in `publisher.rs` as an idempotent safety net for direct handler tests.
Geolocation is supplied by vendors as often as by hosts, so it becomes the first capability an integration module can declare on its own registration and a deployment can select by configuration. Identity and device providers follow the same shape once their traits land. An `IntegrationRegistration` now carries an optional `geo_provider`, set with the new `with_geo_provider` builder method. The registry collects every declared provider from the enabled registrations and resolves the new `[geo] provider` setting against them once at construction: - unset leaves the adapter's own host lookup in place, - `none` resolves the new `DisabledGeo`, which returns no location at all, - a module id resolves that module's provider. Anything else is a startup configuration error. Because only an enabled registration reaches the collection loop, the error tells the three cases apart by name, so a selector pointing at a module that is registered but switched off says so rather than reading as a module that never declared the capability. A module that declares a geo provider the selector does not choose is logged as a warning, so an operator can see a capability shipped but unused. Each adapter applies the resolved provider where it builds per-request services, through `RuntimeServices::with_geo`, so the request path is unchanged from the caller's point of view when no module is selected.
A deployment that ships a vendor crate needs to add that crate's integration and auction provider builders without the adapter naming the vendor, so each adapter now takes them as arguments. State construction is split. Every adapter gains `build_state_with_registrations(settings, integrations, auction_providers)`, which calls `build_orchestrator_with_providers` and `IntegrationRegistry::with_registrations`, and the existing entry points (`build_state_with_settings` on Axum, Cloudflare and Spin, `build_state_from_settings` on Fastly) keep their names and delegate to it with no extra builders, so today's callers are unchanged. The three adapters that expose a routing seam gain `TrustedServerApp::routes_with_registrations`, which builds the state from the supplied builders and then calls the same `build_router` the plain `routes_with_settings` uses, so a composed deployment routes exactly as the plain one does and there is one route table, not two. Both builder lists are taken as slices rather than owned vectors because neither is consumed, which is also what the clippy gate requires. The Axum route tests now build their settings through a `test_settings` helper and assert that a duplicate integration id supplied through `routes_with_registrations` is rejected with an error naming both the built-in source and the supplied one.
Every earlier commit on this branch opened one part of `trusted-server-core` so a vendor integration can live in its own crate, but nothing proved the whole thing works from outside core. A seam only its own author has used is not a seam, so this adds a module that lives outside core and drives every part of the seam through a real adapter. `crates/integrations/seam-probe` (package `trusted-server-integration-seam-probe`) is a test fixture and says so in its crate description and its top-level doc comment. It must never ship in a deployment: it reports internal request state over an unauthenticated route and resolves location from static configuration. It is a dev-dependency of the Axum adapter only, so no adapter's production build reaches it, and `default-members` is unchanged. One registration exercises, together: - a browser module the crate carries, `js/probe.js` embedded with `include_str!` and its SHA-256 stated as a literal beside it. The registry rejects a mismatch at construction, so a crate unit test keeps the literal honest, and a `.gitattributes` entry pins the file to LF so the literal holds on a Windows checkout under `core.autocrlf`; - a proxy route, `/integrations/seam_probe/report`, that returns JSON naming the country the geo provider resolved and how many times the request preparer ran; - a request preparer that inserts a marker type into the request extensions; - a geo provider returning the country its own configuration names; - its own configuration block, `[integrations.seam_probe]`, whose `validate` rejects a country that is not two letters; - an auction provider builder declaring the name `seam_probe`. The round trips live in `crates/trusted-server-adapter-axum/tests/seam_probe.rs` and each asserts an observable outcome rather than that a function was called: 1. the carried module is served in the unified bundle, the body starts with core and contains the probe's source, and the response is immutable when the request's `?v=` matches the composed hash; 2. the proxy route reports `ZZ`, the country the module's own geo provider resolved under `[geo] provider = "seam_probe"`, and the marker the preparer left, which is the first end-to-end proof that an adapter runs registry preparers on the request path; 3. `validate_settings_for_deploy_with` rejects a three-letter country with the probe's own message, and the same settings pass without the probe's builder, so the rejection is the module's and not core's; 4. `[geo] provider` naming an enabled module that declares no geo provider fails in `routes_with_registrations`, naming the module and the missing capability; 5. two builders claiming one integration id are rejected, naming the id and both sources; 6. the auction provider name the outside builder declares satisfies `[auction] providers`, and the same settings are refused without the builder. Each was checked with a negative control: the corresponding production path was broken, the test was confirmed to fail, and the path was restored. Round trip 2 records one thing worth fixing separately. The preparer run count is 2, not 1, because the Axum adapter runs the registry preparers twice on the fallback path: once in `execute_handler` before the handler is called, and again at the top of `dispatch_fallback`. The test pins today's number so running them once becomes a deliberate change to the test rather than a silent change to what a module observes.
The Axum adapter prepared twice for every request the fallback served. `execute_handler` called `registry.prepare_request` before invoking the handler, and `dispatch_fallback` called it again at the top. GPT diagnostics, the only preparer in the tree today, survived it because it is idempotent and returns its stored decision, but a vendor preparer that appends a header, emits telemetry or counts anything was wrong. The call graph was established before changing anything, in all four adapters: - Axum: every route except `/health` is registered to `named_route_handler` or `fallback_handler`, and both wrap `execute_handler`. `dispatch_fallback` has exactly one caller, the closure `fallback_handler` passes to `execute_handler`, so it can never be entered without `execute_handler` having run. Fallback routes therefore prepared twice and named routes once. - Cloudflare: named routes go through `make_handler`, and the fallback `dispatch` is registered directly rather than wrapped in `make_handler`, so the two call sites lie on disjoint paths and each request prepares once. - Spin: the three call sites are the `/auction` handler, the `/_ts/page-bids` handler and the fallback `dispatch`, each registered directly and none nesting inside another, so each request prepares at most once. - Fastly: `named_route_handler` reaches `execute_named` and `fallback_route_handler` reaches `execute_fallback` then `dispatch_fallback`; `dispatch_fallback` is never called from `execute_named`, so the two call sites are disjoint and each request prepares at most once. Only Axum had the double run, so only Axum changes here. The call removed is the one in `dispatch_fallback`, and the one kept is in `execute_handler`, which is strictly earlier on the same path and covers every route the removed call covered, plus the named routes it never reached. Preparers still run before routing, and before the tsjs, integration-proxy and publisher branches that `dispatch_fallback` chooses between. The probe's assertion moves from the pinned 2 to 1 and now states the invariant: a request prepares exactly once. A second round trip, `every_route_prepares_the_request_exactly_once`, pins the invariant on both paths by driving a named route (`/admin/keys/rotate`, the legacy alias denied locally, which is not covered by the `^/_ts/admin` auth handler so the request reaches the preparer) and the probe's own proxy route on the fallback. It counts through a new per-token counter in the probe rather than the request extensions, because a named route returns a fixed response and reports nothing about the request it was given; each caller passes its own token so tests running in parallel count separately. Both negative controls were re-run against the fix. Removing the remaining call makes both assertions fail at 0, and putting the second call back makes them fail at 2, with the named-route assertion still passing, which localises the defect to the fallback path.
Documents the `[geo]` section in the configuration guide, covering its three states (unset keeps the host lookup, `none` resolves no location, a module id selects that module's provider) and what happens when the selector names a module that is unregistered, disabled, or supplies no geo provider. Adds a section to the integration guide on a module that ships in its own crate, covering what the vendor crate provides (a builder with its id and source, a build function, a validate function, optionally a request preparer), what a registration can declare (proxy routes, head injection, its own browser module with its hash, immediate, deferred or standalone delivery, a geo provider), how an adapter composes it in through `routes_with_registrations` or `build_state_with_registrations`, and the two traps this branch found. The first is that a carried module's hash literal must match the file's bytes, which line-ending rewriting on checkout will break. The second is that a vendor's own deploy rules do not run through `ts config validate` or `ts config push`, because those go through `TrustedServerAppConfig`, which validates with the built-in builders only. `CLAUDE.md` gains a table of the pluggable capabilities and the `crates/integrations` directory in its workspace layout. Fixes the auction orchestration guide, which still described provider registration as a list of bare function pointers behind a `ProviderBuilder` type. Providers now register through `AuctionProviderBuilder`, which also names the crate a provider came from, and `build_orchestrator_with_providers` takes the ones an adapter supplies. The source-neutrality guard asserts that no core source file reaches for the Fastly SDK, and it read the files through a hand-maintained list of `include_str!` calls. That list had drifted: 18 of core's 115 source files were missing from it, so the guarantee was not being applied to files such as `platform/geo.rs`, `cache_policy.rs` and `integrations/osano.rs`. The list is now generated by `build.rs` from every `.rs` file under `src`, which both closes the drift and means a vendor integration moving out of core leaves the guard on its own instead of breaking the test build on a missing `include_str!` path. `migration_guards.rs` is the one file left out, because the banned pattern appears there as the regex literal the guard matches with. A second test pins the generated list to the tree so a generation bug cannot leave the guard passing while checking nothing. Verified by adding a file containing `use fastly::Request;` to `src` (the guard failed, naming it) and deleting it again (the guard passed, with no build break).
`compose` started from an empty `String` and pushed each part, so serving the browser bundle grew the buffer about a dozen times and copied roughly twice the bundle's size for nothing. Every piece the walk yields has a known length before the walk runs, so walk once to total them, allocate that exact size, then walk again to write. The bytes are unchanged: the parity tests that compare `compose` against `trusted_server_js::concatenate_modules` for every compile-time module set still pass. A new test asserts the returned `String`'s capacity equals its length, which holds only if the buffer was allocated once at the right size and never grew.
`compose_hash` claimed a per-request caller "hashes a given module set once per process or isolate". Reading the pinned EdgeZero adapters shows that is true on one host of four. `edgezero_adapter_cloudflare::run_app` and `edgezero_adapter_spin::run_app` both call `A::build_app()` inside the per-request entry point, Fastly starts a fresh Wasm instance per request, and only the Axum dev server builds once, because its `main` calls `TrustedServerApp::routes()` before serving. So on three hosts of four the memo never hits. Correcting the documentation rather than removing the memo. The memo is what makes Axum hash the bundle once instead of on every page view, and a miss costs a key vector, two mutex locks and a stored copy of the hash, all next to a SHA-256 over several hundred kilobytes of script, so what it wastes where it cannot hit is a rounding error against what it saves where it can. The suggested alternative, hashing the immediate set once when the registry is built, was not taken. It would touch `IntegrationRegistry`, `tsjs.rs` and three publisher call sites, and it would not help: the three hosts that cannot hit the memo rebuild the registry per request too, so the hash would still be computed per request, only eagerly. It would move the hash onto every request rather than only the requests that serve HTML or the bundle, which makes asset and API requests pay for a bundle hash nothing reads.
The `hb_adid` fallback in `build_bid_map_with_auction_id` wants one field, the bid identifier, and reached it through `BidRenderer::payload_as::<ApsRendererV1>`, which clones the entire payload map and deserializes it into an owned struct. One of those fields is `aaxResponse`, a base64 creative envelope capped at 256 KB, so every bid on every page view that runs the ad stack copied that envelope twice, once into the cloned `Value` and once into the owned `String`, only to drop both. `BidRenderer::payload_field` borrows one value out of the payload with the same type-tag check `payload_as` applies, and the call site uses it with a new `APS_RENDERER_BID_ID_KEY` naming the wire key. Nothing is serialized differently: `payload_field` only reads, so the wire form is untouched. `payload_as` stays for the callers that genuinely want the whole descriptor, with a doc note pointing one-field readers at the new accessor. The one behavior difference is that `payload_field` does not check that the rest of the payload deserializes, so a malformed payload that still carries `bidId` now yields it. Every APS payload is built by `from_typed` from an `ApsRendererV1`, so no production path reaches that case, and the accessor documents it. Evidence the output is unchanged: - `renderer_bid_id_key_matches_the_serialized_form` (aps.rs) asserts the new accessor returns exactly what `payload_as` deserializes, and pins `APS_RENDERER_BID_ID_KEY` to the camelCase name `ApsRendererV1` actually serializes. - `bid_map_prefers_the_renderer_bid_id_over_ad_id_and_the_openrtb_bid_id` (publisher.rs) gives every `hb_adid` source a different value and a 200 KB envelope, so it passes only if the renderer field is the one read. - `bid_map_ignores_a_renderer_bid_id_carried_under_another_type_tag` covers the tag check at the call site. - The pre-existing `bid_map_exposes_aps_renderer_and_selected_bid_id` still passes untouched.
The comment on the carried-module hash check said the SHA-256 costs one per request on Fastly and "once per process on the other adapters". That is wrong on two hosts of three. Reading the pinned EdgeZero adapters (v0.0.4, git checkout 9e661ae): `edgezero_adapter_cloudflare::run_app` calls `A::build_app()` inside the per-request entry point, and its own comment says "every Worker request re-enters this function"; `edgezero_adapter_spin::run_app` does the same, with the matching comment about `#[http_service]`. Fastly's `run_app` also calls `A::build_app()` per request, and this repo's Fastly `main` calls `TrustedServerApp::build_app_with_state()` per request in any case. Only `edgezero_adapter_axum::run_app` calls `A::build_app()` once before serving, and this repo's Axum `main` likewise calls `TrustedServerApp::routes()` before handing the router to the dev server. So the application is built once on Axum and per request everywhere else. Corrected here: - `integrations/registry.rs`, the carried-module hash comment, which is the comment the finding names. - `docs/guide/configuration.md`, which said a provider-selector failure "shows there on every request rather than once" only because "the Fastly adapter builds the application per request". It shows on every request on Cloudflare Workers and Spin too. `tsjs_bundle.rs`'s `compose_hash` doc carried the same claim and was corrected in the previous commit. Also corrected, though pre-existing rather than new on this branch: the `AppState` doc comments in the Cloudflare and Spin adapters both said "built once at startup and shared across all requests", which is false for the same reason, and the Axum one now says why it is the exception. Fastly's already said "once per Wasm instance" and is accurate.
`the_open_renderer_serializes_to_the_same_bytes_as_the_aps_variant_did` compared `serde_json::to_string` against two literal strings whose keys happen to be in alphabetical order. That order holds only while `serde_json::Map` is a `BTreeMap`, which is only while the crate's `preserve_order` feature is off. The feature is reachable in this workspace. `trusted-server-cli` depends on `edgezero-cli`, which depends on `handlebars`, which declares `serde_json` with `features = ["preserve_order"]` unconditionally. Cargo unifies features across everything built for one target, so any command that builds the CLI and core together for the host target turns the map into an `IndexMap` inside core. `cargo tree -p trusted-server-core --target x86_64-pc-windows-msvc -e features -i serde_json` shows no `preserve_order`; adding `-p trusted-server-cli` to the same command shows three. A maintainer running `cargo test --workspace --target <host>` gets the second build, and the test would fail there for a reason that has nothing to do with the wire form. Both sides now go through a `with_sorted_keys` helper before comparison, so the key order is fixed by the test rather than by which map `serde_json` was compiled with. The literals are unchanged, byte for byte, and nothing is weakened: two JSON objects serialize to the same sorted bytes only when they carry exactly the same keys with exactly the same values, which is the promise the literals were there to make.
Fallout from rebasing this branch onto the five-PR stack rather than onto main, where both sides had changed the same functions. The Axum adapter's test helper still called `build_orchestrator`, which this branch's import list replaced with `build_orchestrator_with_providers`. It now calls the latter with no external providers, which is the same thing. The Cloudflare adapter had a test calling `build_per_request_services` with the old `(ctx, settings)` argument pair. That function now takes `(state, ctx)` so it can apply a module's geo provider, and the test has no application state, so it calls `build_runtime_services` directly, which is what it actually wanted. Four doc comments still said an unset `[geo] provider` leaves the host's own lookup in place. Reconciling the two geo designs settled that unset resolves nothing and makes no host geo call, matching the permission model's default, and that `"platform"` is the explicit opt-in to the host lookup. The comments now say that on all four adapters. Addresses: rebase of this branch onto split/5, conflicts resolved in the composition roots and the geo selector
Two checks were written when the host lookup was the only geo provider there could be, and both treat any other value as no geo at all. validate_jurisdiction_acknowledgment asked whether the selector was `"platform"`, so a deployment selecting a module's geo provider was told it had none and made to set assume_single_jurisdiction to acknowledge something untrue. It now asks the question the other way round: only an unset selector and the explicit `none` resolve nothing, and everything else resolves a location. build_geo_provider's doc said an unknown provider is rejected by GeoConfig::validate_provider_selection, which is no longer where that happens. The function returns DisabledGeo for a module id because it cannot see the registry, and the adapter then substitutes the module's provider. The doc now says that the value is the base the adapter starts from rather than what serves the request, and that the registry is the layer that rejects a selector naming a module with no geo provider. The seam probe's test settings also predate the permission model, so they now carry the required [geo] default_country. The two tests that select a geo provider write their own [geo] table, so the fixture adds one only when the caller has not. Addresses: reconciling the geo selector across the permission model and the seam
Answers the architectural finding on IABTechLab#1043 rather than deferring it. Vendor identity reached core through RuntimeServices injection while everything else a module supplies was declared on its integration registration, so there were two extension mechanisms and identity was on the second one. A registration can now declare an Edge Cookie provider and a device provider the same way it declares a geo provider. The registry resolves each against its selector, warns when a module declares a capability the selector does not choose, and the adapters apply all three to RuntimeServices in one place. RuntimeServices gains a device slot, which it had no way to carry before, and with_ec_provider and with_device_provider to match with_geo. Identity and device differ from geo in one way that matters. Both have providers built into core, so a selector naming a built-in is not an error at the registry, and resolution returns None for those and lets core resolve them as before. Two closed allowlists had to open, being the same fault in two more places. The device selector accepted only `builtin` and `fastly`, so any module id was rejected before the registry saw it. The jurisdiction check asked whether the geo selector was `platform`, so a deployment selecting a module's geo provider was told it had none. Neither could stand once a module can supply these. Proven by the probe, which now declares all three capabilities from one registration. Two tests select the module for identity and for device and assert the resolved provider is the module's own. The probe's identity provider mints a value the built-in HMAC provider cannot produce, so a passing assertion means the module's provider ran rather than core's. Addresses: Aram, `ec/provider.rs`, vendor identity should lean on the integration system rather than a second extension mechanism
The test-only constructors of IntegrationRegistryInner still set only the geo fields, so the Fastly target failed to compile while the Axum one did not, because the two targets gate those constructors differently. Addresses: build failure from the registration capability change
Three tests still asserted the behaviour these branches deliberately replaced, and two more asserted a serialized shape that stopped being right once a default country became required. Settings validation no longer refuses a provider key it does not recognize, because a module id is a legitimate value and a closed list would shut every module out of geo and device. The two selector tests now assert that settings accepts such a key and that building the registry is what refuses one no module supplies, which is where the knowledge lives. Removing that refusal makes both fail. An unset geo selector no longer leaves the adapter's host lookup in place. It resolves the disabled provider, so no client IP reaches a host geo service, which is the permission model's privacy default, and `platform` is the explicit opt-in that leaves the host lookup standing. The registry test now asserts both halves, because `platform` is the case that resolves nothing. The device selector was the odd one out. Two doc comments already said a key no module supplies is rejected, but `resolve_device_provider` returned an `Option` and quietly fell back to the built-in provider, so a mistyped selector was silently ignored. It now mirrors geo through `module_device_provider` and names the module in the error, which is what makes the device test able to fail at all. The two serialization tests asserted that the whole `[geo]` table is absent when no selector is set. The table is always written now that `default_country` is required, so they assert the selector key is absent instead, which is what blob compatibility actually turns on. The second also appended a second `[geo]` table to settings that already had one, which TOML rejects as a duplicate key, so it sets the selector inside the existing table. Addresses: crates/trusted-server-core/src/settings.rs and crates/trusted-server-core/src/integrations/registry.rs, where five core tests encoded the pre-seam contract and the device selector performed no rejection at all.
…r needs Every provider trait method was synchronous while every platform I/O service is asynchronous, and a provider was handed no services at all. So a provider could not make a backend call, read a key-value store, or fetch a secret. That made every real vendor provider impossible to write, which defeats the point of having a pluggable seam. `EdgeCookieProvider::generate`, `DeviceProvider::detect` and `PlatformGeo::lookup` are now async and take `&RuntimeServices`. `EdgeCookieProvider::resolve_from_client` gets the same treatment, because it is the client-side counterpart of `generate` and its own documentation requires an implementation to verify what the browser posted, which generally means reading a secret or calling the vendor's backend. Leaving that one synchronous would have recreated the defect on the single method that most needs the services. The three traits use `#[async_trait(?Send)]` while keeping their `Send + Sync` bound, which is what `PlatformHttpClient` already does and for the same reason: the provider object stays safe to share and run multi-threaded, and only the future it returns is pinned to one thread, because the host SDKs produce `!Send` futures on wasm32. No provider keeps a synchronous method, the built-in HMAC, client-fixed and host-signal providers included. One that does no I/O declares an async method that returns immediately rather than taking an escape hatch, so the runtime decides how the work is driven and the implementer only has to make the type safe to share. Geo turned out not to be the problem it looked like. It is called just ahead of permission resolution rather than from inside it, and its result is passed in as a `GeoStatus`, so making the lookup async stops at `EcContext::read_from_request_resolving_geo` and the permission assembly stays synchronous. Two places had no services to hand a provider. The Fastly finalize middleware and the entry-point finalize path both run after the handler, where the per-request services are out of scope, so they now build the graph once through `build_finalize_services` and apply the request's client metadata with the new `RuntimeServices::with_client_info`. The entry point itself is synchronous host code, so it drives the two async calls with `futures::executor::block_on`, at the same boundary it already drives the router. `resolve_geo_for_response` took a synchronous closure, which an async lookup cannot satisfy, so it is now the predicate `geo_allowed_for_response` and each caller awaits the lookup itself. The 401 skip rule it exists for is unchanged. Two tests prove the services arriving at a provider are the caller's real ones, rather than the parameter being inert. Each drives a provider that resolves a value out of the config store it is handed, through the production path, and asserts the value reaches the output. Substituting a different services graph anywhere in either chain fails them. Addresses: crates/trusted-server-core/src/ec/provider.rs, crates/trusted-server-core/src/ec/device.rs and crates/trusted-server-core/src/platform/traits.rs, where the three provider seams were synchronous and blind to the platform services.
…mplementation Fix the seam-probe fixture crate's own unit tests so they compile and pass (A14): add tokio to dev-dependencies, build a valid RuntimeServices from self-contained stub stores plus core's unavailable implementations, make the geo test async, and give the fixture settings the now-required `[geo] default_country`. Add a host-target seam-probe test step to the CI job so the fault cannot recur unseen. Add a round-trip test (A13) that drives the module's Edge Cookie provider through `generate` and asserts the `seam-probe-` prefix the built-in HMAC provider can never produce, and narrow the selection test's doc to point at it. Document the identity and device capabilities (A7): add `.with_ec_provider(...)` and `.with_device_provider(...)` rows to the integration guide, identity and device rows to the CLAUDE.md capabilities table, and correct the Validate row and the geo default wording. Correct doc comments that contradict the code (B4): the geo resolver run-on and its "unset means host lookup" claims, the Fastly service-graph doc attachment, the "module-supplied geo provider" docs that omit identity and device, the router-level `# Errors` that omit the Edge Cookie provider failure, the AppContext and prepare-request docs, the registered-builder-ids and 256 KB base64 claims, and the seam-probe module and validation docs. House-style over lines this branch added: fingerprint to signals or hash by sense, mint to create or derive, several to a plain quantity, initialise to initialize.
This was referenced Aug 31, 2026
Collaborator
This was referenced Aug 31, 2026
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
2 participants
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.
What this does and why
trusted-server-corecarries nine vendor integrations as core code becauseevery place an integration plugs in is closed. The builder table is private,
browser JavaScript is fixed at compile time, deploy validation names each
vendor's config type, the auction provider table is a second private list, and
two vendors reach into core through named types. Every new vendor is therefore
another core change, most recently in #1054.
This branch opens those places. An integration can now ship in its own crate
with its Rust, its browser script, its configuration type, its deploy rules and
its tests, and an adapter composes that crate into a deployment at startup
without core naming the vendor. No existing integration changes behavior, the
served script keeps its exact bytes and its
?v=hash, and the built-in setstill registers through the same path.
This branch sits on top of the five-PR stack rather than beside it, so the six
pull requests form one ordered chain with #1043 against
mainand nothing ableto merge out of order. Measured against #1047, the last of the five, it is 34
commits and 81 files.
What is now composable:
IntegrationBuilder::new(id, source, build, validate), passed toIntegrationRegistry::with_registrationsAuctionProviderBuilder::new(name, source, build, validate), passed tobuild_orchestrator_with_providers.with_js_module(CarriedJsModule { source, sha256 })on the registrationvalidate_settings_for_deploy_with.with_request_preparer(...)on the builder, run byIntegrationRegistry::prepare_request.with_geo_provider(...)on the registration, selected by[geo] providerTwo supporting changes come with those. The bid renderer is no longer an enum
with a single APS variant, it is a type tag plus the payload the auction
provider supplies, serialized flat so the response a page receives is byte for
byte what it was, and
ApsRendererV1moves into the APS integration. DataDome'scache and origin-path marker becomes a neutral
PersonalizedResponserequestextension that any integration may set, so core acts on the marker without
knowing which integration asked for it.
Relationship to #1084
#1084 is the design and carries no code. It defines the seam in
docs/superpowers/specs/2026-08-27-integration-provider-seam-design.md,sections 3.1 to 3.6, together with the five provider-series specs. This branch
is the implementation of that design. Read #1084 first, then read
this branch as the answer to it. The section below headed "What is not in this
change" lists every part of the specified design that this branch does not yet
deliver, so the two can be compared without reading both in full.
What a reviewer should look at first
crates/trusted-server-adapter-axum/tests/seam_probe.rs(7 tests). Everyseam is driven end to end from a crate core does not know about, through the
Axum adapter's real router. The tests turn on observable outcomes, being the
bytes served, the JSON a route returns and the error a startup or deploy
check produces, rather than on a function having been called. Start here,
because a seam is only proven by an implementation that is not the built-in
one.
crates/integrations/seam-probe/src/lib.rs, the fixture those testsdrive. It is a workspace member and a dev-dependency of the Axum adapter
only, so no production build reaches it. One registration carries a browser
module, a proxy route, a request preparer, a geo provider and its own
configuration block, and the crate also supplies an auction provider builder.
crates/trusted-server-core/src/integrations/registry.rs,with_registrations. Three checks live in the construction loop, being theduplicate-id refusal that names both sources, the SHA-256 check on a carried
browser module, and the resolution of
[geo] provideragainst the modulesthat declared a geo provider.
crates/trusted-server-core/src/tsjs_bundle.rs, new. Composition of theserved script moves out of
trusted-server-jsand into core, keyed oncontent rather than on ids, because a module a vendor crate carries is not in
the compile-time map. The byte rule is unchanged, being core first then each
part joined by
;\n. The tests in that file compare the composed bytes andthe composed hash against
trusted_server_js::concatenate_modulesandtrusted_server_js::concatenated_hashfor every compile-time module set,which is the evidence that no current
?v=value moves.crates/trusted-server-core/src/config.rs. The hand-written list offourteen vendor config types is gone, replaced by a loop over the builders.
Note the doc comment on
deploy_validation_reaches_every_built_in_builder,which states plainly what that test cannot catch.
crates/trusted-server-core/src/auction/types.rs,BidRenderer. This isthe riskiest single change, because the wire shape
{"type":"aps", ...}hasto survive byte for byte.
aps_renderer_serializes_to_versioned_camel_case_contractpins theserialized form and
renderer_bid_id_key_matches_the_serialized_formpinsthe one field the publisher reads by key.
One more file worth a look is
crates/trusted-server-core/build.rs, new. Themigration guard used to list every core source file by hand with
include_str!,which meant a vendor moving out of core would break the build rather than a
test. The list is now generated from the source tree, so a file that leaves core
leaves the guard with nobody editing anything.
The provider interfaces are asynchronous, and providers get the services
Until this change every provider trait method was synchronous while every
platform service was asynchronous, and a provider was handed no services at all.
So a provider that needed to call a backend, read a key-value store or fetch a
secret could not be written. That makes a pluggable seam pluggable in name only,
because the vendors most likely to want one are the ones with a backend behind
them.
All three provider interfaces now take
&RuntimeServicesand are asynchronous,along with
resolve_from_client, whose own documentation says an implementationmust verify what the browser posted, which generally means reading a secret or
calling the vendor. Forty-one implementations across four traits were converted
and none keeps a synchronous method, including the built-in ones. A provider
doing no work simply returns immediately.
The traits keep their
Send + Syncbound and use#[async_trait(?Send)], whichis what
PlatformHttpClientin this repository already does and for the samereason, so the provider stays safe to share while the future stays on one
thread. The implementer's job is to make the type safe to share, and the runtime
decides how the work is driven.
Two tests drive a provider that resolves a value out of the config store it is
handed, through the production path, and assert the value reaches the output.
They exist because after everything compiled and passed, nothing anywhere read
the services parameter, and a parameter no code exercises is not a working seam.
One design decision made while stacking this on #1045
Two designs for
[geo]met when this branch moved onto the stack, and theydisagreed about what an unset selector means. The permission model in #1045
treats unset as resolving nothing and making no host geo call, so a default
deployment is not tied to any host geo service. This branch treated unset as
leaving the host's own lookup in place. Both cannot be true.
The permission model's meaning is kept, because a deployment that has not asked
for a host geo lookup should not be making one. So unset and
"none"bothresolve nothing,
"platform"is the explicit opt-in to the adapter's ownlookup, and any other value names a module that declared a geo provider.
That change also had to open the
[geo] providervalidation, which was a closedlist of unset,
"platform"and"none". A closed list cannot stand once amodule can supply geo, because it rejects every module id before the registry
ever sees it. Validation now accepts any value and the registry raises the error
when no module supplies the named provider, so an operator still gets a startup
failure for a typo, just from the layer that can tell.
How it was verified
These are the gate commands from
CLAUDE.md, run in the branch's worktree onWindows.
cargo fmt --all -- --checkcargo clippy-fastlycargo clippy-axumcargo clippy-cloudflarecargo clippy-cloudflare-wasmcargo clippy-spin-nativecargo clippy-spin-wasmcargo test-fastlycargo test-axumseam_probe.rstests, 0 failedcargo test-cloudflarecargo test-spincargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test paritycd crates/trusted-server-js/lib && node build-all.mjscd crates/trusted-server-js/lib && npx vitest runmain(see below)cd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatThe two JavaScript failures are
installTsAdInit > preserves legacy state in the edge bootstrap when getConfig does not report itand
installTsAdInit > tracks setConfig state and re-enabling in the edge bootstrap,both in
crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.ts.They are not this branch's.
git diffovercrates/trusted-server-js/betweenthe merge base and the branch tip is empty, so no file under that directory
changed here, and running that one test file in a worktree checked out at
upstream/main(d516a9e94) with the samenode_modulesreproduces the sametwo failures, 123 passed and 2 failed. They need fixing on
mainrather thanhere.
The two Prettier checks cannot be trusted on this machine and were not treated
as passing. The working tree has CRLF line terminators
(
docs/guide/integrations-overview.mdreports "CRLF line terminators"), soprettier --checkreports style issues in 110 files undercrates/trusted-server-js/liband 162 underdocs, most of which the branchnever touches. Checking only the four documentation files this branch edits
gives three warnings on the branch and four on the merge base, so the branch
removes one and adds none. Both gates still need a run on Linux CI before anyone
reads them as green.
Two things that CI does not cover and a reviewer should know about. Neither the
test aliases nor the clippy aliases name
trusted-server-integration-seam-probe, so the fixture crate's own 8 unit testsand 2 doc-tests, and clippy over its source, run in no gate. They were run by
hand here with
cargo test -p trusted-server-integration-seam-probe --target x86_64-pc-windows-msvc,which passed, 8 unit tests and 2 doc-tests, 0 failed. If the crate stays, the
aliases or the workflow should pick it up.
What is not in this change
The design in #1084 is wider than this branch. Being straight about the gap is
more useful than appearing complete, so here is every part of it the branch does
not deliver.
Identity and device capabilities are here (section 3.6). They were not, when
this branch sat beside the five-PR stack rather than on top of it, because
neither
EdgeCookieProvidernorDeviceProviderexists onmainand aregistration cannot carry a trait that does not exist. Stacking the branch on
#1043 and #1044 put both traits underneath it, so the registration now declares
all three capabilities.
with_ec_providerandwith_device_providersitalongside
with_geo_provider, the registry resolves each against its ownselector and warns when a module declares a capability the selector does not
choose, and the adapters apply all three to
RuntimeServicesin one place.RuntimeServicesgained a device slot, which it had no way to carry before.Two tests in
seam_probe.rsselect the probe module for identity and for deviceand assert the resolved provider is the module's own. The probe's identity
provider creates a value beginning
seam-probe-, which the built-in HMAC providercannot produce, so a passing assertion means the module's provider ran rather
than core's. Acceptance item 2 in section 6, one module declaring all three
capabilities, is met.
Two closed allowlists had to open to make this work, and both were the same
fault. The device selector accepted only
builtinandfastly, so any module idwas rejected before the registry saw it. The jurisdiction check asked whether the
geo selector was
platform, so a deployment selecting a module's geo providerwas told it had none and made to acknowledge single-jurisdiction operation it was
not in. Neither can stand once a module can supply these capabilities.
No provider moves out of core (section 3.6). The spec has the HMAC identity
provider and the User-Agent-only device provider becoming Tech Lab-owned module
crates, with core keeping only the
nonestate for each capability. That workbelongs with the two PRs that introduce those providers. What this branch adds
on the location side is
DisabledGeo, which is what an unset[geo] providerresolves to, and what
"none"resolves to when spelled explicitly. Neithersends a client IP to a host geo service.
"platform"is the opt-in to theadapter's own lookup.
Nothing is rejected for a missing host signal (section 3.6). A registration
declares no host-signal requirements, so a provider that needs a signal the
running adapter does not expose is not turned back at startup. That check has
nowhere to hang until the identity and device capabilities land.
There is no finalize hook (section 3.5). The prepare half is done, and
IntegrationRegistry::prepare_requestreplaced the directgpt_diagnostics::prepare_requestcalls in all four adapters. The finalize halfis not. The registration builder has no finalize method, and the GPT diagnostics
decision is still a named type inside
html_processor.rsandpublisher.rs. AGPT move needs that hook first.
A module's own deploy rules still run nowhere an operator can reach (section
8, items 1 and 6).
ts config validateandts config pushgo throughTrustedServerAppConfig, which callsvalidate_settings_for_deploywith noextra builders, so only core's rules run on the path an operator uses. The
registry calls a builder's build function and not its validate function, so a
vendor whose checks live in
validatehas them enforced on no path at allunless the deployment's own code calls
validate_settings_for_deploy_withandhands over the builder. The seam probe works around this by repeating its check
inside its build function, which is exactly the copying every vendor would have
to do. The gap is written down in
CLAUDE.mdand indocs/guide/integration-guide.mdrather than papered over, and it needs adecision, either the CLI is built per deployment with its vendor crates, or the
registry runs
validatewhen it builds.A Fastly deployment still cannot take a vendor crate (section 8, item 7).
The Axum, Cloudflare and Spin adapters expose
build_state_with_registrationsand
routes_with_registrationspublicly. The Fastly adapter'sbuild_state_with_registrationsispub(crate)and the crate has onlysrc/main.rs, so it has no library target to call into. Composing a module intoa Fastly deployment means editing that adapter. Fastly is the primary deployment
target, so this one decides whether the seam is usable in production or only in
the dev server, and it should land before the first vendor is asked to use the
seam.
A provider can still be resolved more than once per request (section 8, item
3). The branch adds no per-request provider context. The seam probe's proxy
route calls the geo provider itself while the request path has already called
it, which is the case the spec records. A module sharing one backend across
identity, geo and device therefore has nowhere to hang a single call per request
yet.
Request preparers still cover different routes on each host (section 8, item
5). Replacing the vendor-named call with the registry call did not make the
covered route set uniform. Axum, Cloudflare and Fastly each prepare at two
points, Spin at three. Axum's two points are the only ones documented as
covering every route in the table except
/health. A module that strips its ownreserved query or cookie is therefore protected on a different set of routes
depending on which host it is deployed to.
One core reader still reaches into an APS payload (section 8, item 4). The
hb_adidfallback inpublisher.rsreads the APS renderer's bid id. It nowgoes through the neutral accessor
BidRenderer::payload_field, which reads onefield instead of cloning the whole payload, but
publisher.rsstill importsAPS_RENDERER_TYPEandAPS_RENDERER_BID_ID_KEY, so the APS migration needs aneutral answer for that fallback and not only the auction seam.
DataDome is still named once in core.
publisher.rsreadsDataDomeClientTagSuppressedto decide whether DataDome's head injection leavesits client-side tag out. That marker decides nothing about caching or the origin
path any more, which is what the neutral
PersonalizedResponsemarker now does,and it moves out with the DataDome integration. The code comment at the read
site says so.
The nine vendor moves are not here (section 4), and neither is the
ts auditvendor table. Each vendor moves in its own PR after this one, which is the
point of doing the seam once. The
ts auditcommand keeps its own vendordetection patterns in
crates/trusted-server-cli/src/commands/audit/, outsidethe registry, and how the CLI learns a vendor's detection pattern from a crate
is still an open question.
One deliberate deviation from the spec. Section 4 says the source-file guard
should drop the nine vendors' entries. The branch generates the whole guard list
from the source tree instead, through a new
crates/trusted-server-core/build.rs, so a file joins or leaves the guard onits own and no vendor move has to touch it. The goal is the same and the
mechanism is different, which is worth a reviewer's eye.
What the round trip does not exercise. The seam probe declares a proxy, a
carried browser module, a request preparer and a geo provider. It declares no
head injector, attribute rewriter, script rewriter, HTML post processor or
request filter, and it uses neither the deferred nor the standalone script
delivery flag. Those declarations are covered by tests inside core, not from
outside it. Acceptance item 1 in section 6 also asks that the module's hooks run
"in the right order", and the tests prove that the preparer runs before routing
and exactly once, but no test asserts the ordering of several hooks on one
registration.
A note on how this was written
An AI assistant wrote the code on this branch and this description of it. Every
gate result quoted above was produced by running the command named, and every
test named was read rather than assumed, but the change is wide and it needs a
human review before it is merged. The places to press hardest are the served
script path, where a mistake shows up as a wrong hash or a missing module, and
the renderer contract, where the wire shape has to be identical to what pages
receive today.