diff --git a/CHANGELOG.md b/CHANGELOG.md index c00487769..aade766a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Breaking** — Admin Basic-auth coverage now includes `GET /_ts/admin/ec`, `GET /_ts/admin/ec/{id}`, and `GET /_ts/admin/eids`. Existing configurations whose `[[handlers]]` patterns protect only the key-management endpoints now fail startup; broaden coverage before deploying, preferably with a namespace-boundary pattern such as `^/_ts/admin(?:/|$)`. Coverage of the dynamic `/_ts/admin/ec/{id}` route is no longer inferred from ID-shaped samples: the router accepts any segment after `/_ts/admin/ec/` and Basic Auth runs on the raw path before routing, so patterns anchored to the EC ID grammar (for example `^/_ts/admin/ec/[a-f0-9]{64}[.][A-Za-z0-9]{6}$`) are rejected in favor of a prefix-level matcher. Placeholder and well-known weak handler passwords (`changeme`, `password`, `admin`, `replace-with-…`) now fail startup on every handler rather than only on handlers inferred to cover an admin endpoint, because first-match-wins handler selection lets a narrow handler shadow the admin namespace. - **Breaking** — Replaced the legacy APS contextual integration with APS OpenRTB at `/e/pb/bid`. APS configuration now uses canonical `account_id` (`pub_id` remains a compatibility alias), no longer requires APS-specific slot IDs, and defaults script creative eligibility off. Operators must update the endpoint, disable native APS demand for Trusted Server cohorts, and prepare GAM/Universal Creative targeting for `hb_bidder=aps` before rollout. `aps` entries in Prebid bidder lists are logged and stripped. APS renderer winners now preserve the upstream bid `id`, omit `crid` when APS omits it, and carry `ext.trusted_server.renderer` instead of `adm`; external `/auction` consumers must support this response shape. - **Breaking** — All auction paths now forward only a validated publisher-owned page URL as `site.page`, removing query and fragment data. APS OpenRTB omits `site.ref`; the existing Prebid Server path continues to forward the browser `Referer` as `site.ref`. Query-driven sites may lose contextual targeting and per-page reporting signals that previously came from query parameters. - **Breaking** — `bid_param_zone_overrides` inner values must now be JSON objects; previously non-object or empty values (`"header" = "x"`, `"header" = {}`) were accepted and silently produced a dead rule at runtime. They now fail at startup with a configuration error. Operators upgrading should audit their `bid_param_zone_overrides` config for non-object zone entries. @@ -21,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security +- Reserved the complete admin namespace at the publisher-fallback boundary. Percent-encoded separators (`/_ts/admin%2Fec`, `%2f`, and double-encoded forms) matched the `^/_ts/admin` Basic-auth handler but escaped the literal-slash namespace check, so an authenticated request fell through to publisher fallback and forwarded its `Authorization` header and body to the publisher origin. The reservation now spans the whole `/_ts/admin` prefix plus the retired `/admin/keys` aliases — including trailing, descendant, and encoded-separator forms — evaluated on both the raw and percent-decoded path, and applies to every adapter. - Validate synthetic ID format on inbound values from the `x-synthetic-id` header and `synthetic_id` cookie; values that do not match the expected format (`64-hex-hmac.6-alphanumeric-suffix`) are discarded and a fresh ID is generated rather than forwarded to response headers, cookies, or third-party APIs ### Fixed diff --git a/crates/trusted-server-adapter-axum/src/app.rs b/crates/trusted-server-adapter-axum/src/app.rs index 1bed830ac..a96ba9e3c 100644 --- a/crates/trusted-server-adapter-axum/src/app.rs +++ b/crates/trusted-server-adapter-axum/src/app.rs @@ -12,6 +12,10 @@ use error_stack::Report; use trusted_server_core::auction::endpoints::handle_auction; use trusted_server_core::auction::{AuctionOrchestrator, build_orchestrator}; use trusted_server_core::ec::EcContext; +use trusted_server_core::ec::admin::{ + admin_ec_lookup_not_supported, deny_admin_diagnostic_fallback, handle_admin_eids_lookup, +}; +use trusted_server_core::ec::registry::PartnerRegistry; use trusted_server_core::error::{IntoHttpResponse as _, TrustedServerError}; use trusted_server_core::integrations::{IntegrationRegistry, ProxyDispatchInput}; use trusted_server_core::proxy::{ @@ -178,6 +182,10 @@ async fn dispatch_fallback( services: &RuntimeServices, mut req: Request, ) -> Result> { + if let Some(response) = deny_admin_diagnostic_fallback(&req) { + return Ok(response); + } + trusted_server_core::integrations::gpt_diagnostics::prepare_request(&state.settings, &mut req)?; let path = req.uri().path().to_string(); let method = req.method().clone(); @@ -259,6 +267,8 @@ enum NamedRouteHandler { TrustedServerDiscovery, VerifySignature, AdminNotSupported, + AdminEcNotSupported, + AdminEidsLookup, /// Legacy `/admin/keys/*` aliases — denied locally with 404 so they never /// reach the publisher fallback (which would leak admin credentials). LegacyAdminDenied, @@ -286,7 +296,7 @@ const LEGACY_ADMIN_DENY_METHODS: &[Method] = &[ Method::DELETE, ]; -fn named_routes() -> [NamedRoute; 13] { +fn named_routes() -> [NamedRoute; 16] { [ NamedRoute { path: "/.well-known/trusted-server.json", @@ -311,6 +321,26 @@ fn named_routes() -> [NamedRoute; 13] { primary_methods: &[Method::POST], handler: NamedRouteHandler::AdminNotSupported, }, + // Admin EC lookup routes. Registered explicitly (like the key routes + // above) so they never fall through to the publisher fallback, and + // they match `Settings::ADMIN_ENDPOINTS` for auth coverage. + NamedRoute { + path: "/_ts/admin/ec", + primary_methods: &[Method::GET], + handler: NamedRouteHandler::AdminEcNotSupported, + }, + NamedRoute { + path: "/_ts/admin/ec/{id}", + primary_methods: &[Method::GET], + handler: NamedRouteHandler::AdminEcNotSupported, + }, + // Admin EIDs echo: pure request inspection (no KV), so the dev + // server serves the real handler. + NamedRoute { + path: "/_ts/admin/eids", + primary_methods: &[Method::GET], + handler: NamedRouteHandler::AdminEidsLookup, + }, // The legacy non-`/_ts` aliases (`/admin/keys/*`) are denied locally with // a 404, matching the Fastly and Cloudflare adapters: the production // basic-auth handler regex `^/_ts/admin` does not match them, and letting @@ -407,6 +437,16 @@ fn named_route_handler( ); Ok(resp) } + NamedRouteHandler::AdminEcNotSupported => { + // The EC identity graph is Fastly KV backed; the Axum + // dev server has no store to read. + Ok(admin_ec_lookup_not_supported()) + } + NamedRouteHandler::AdminEidsLookup => { + let partner_registry = + PartnerRegistry::from_config(&state.settings.ec.partners)?; + handle_admin_eids_lookup(&partner_registry, &req) + } NamedRouteHandler::LegacyAdminDenied => Ok(legacy_admin_alias_denied()), NamedRouteHandler::Auction => { // Build the geo-aware EC context so the auction consent diff --git a/crates/trusted-server-adapter-axum/tests/routes.rs b/crates/trusted-server-adapter-axum/tests/routes.rs index 03caa3d11..d9f0f6b5a 100644 --- a/crates/trusted-server-adapter-axum/tests/routes.rs +++ b/crates/trusted-server-adapter-axum/tests/routes.rs @@ -74,6 +74,9 @@ fn all_explicit_routes_are_registered() { ("POST", "/verify-signature"), ("POST", "/_ts/admin/keys/rotate"), ("POST", "/_ts/admin/keys/deactivate"), + ("GET", "/_ts/admin/ec"), + ("GET", "/_ts/admin/ec/{id}"), + ("GET", "/_ts/admin/eids"), ("POST", "/admin/keys/rotate"), ("POST", "/admin/keys/deactivate"), ("POST", "/auction"), @@ -267,6 +270,189 @@ async fn admin_route_without_credentials_returns_401() { ); } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn authenticated_admin_ec_routes_return_501() { + // The EC identity graph is Fastly KV backed, so the Axum dev server + // answers the admin EC lookup routes locally with 501 instead of letting + // them fall through to the publisher fallback. + let sample_ec_id = format!("{}.abc123", "a".repeat(64)); + for path in [ + "/_ts/admin/ec".to_owned(), + format!("/_ts/admin/ec/{sample_ec_id}"), + ] { + let mut svc = make_service(); + let req = Request::builder() + .method("GET") + .uri(&path) + .header("authorization", "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(AxumBody::empty()) + .expect("should build request"); + let resp = svc + .ready() + .await + .expect("should be ready") + .call(req) + .await + .expect("should respond"); + assert_eq!( + resp.status().as_u16(), + 501, + "{path} should report that Axum EC lookup is unsupported" + ); + assert_eq!( + resp.headers() + .get("content-type") + .and_then(|v| v.to_str().ok()), + Some("application/json") + ); + assert_eq!( + resp.headers() + .get("cache-control") + .and_then(|v| v.to_str().ok()), + Some("no-store") + ); + } +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn admin_ec_route_without_credentials_returns_401() { + let mut svc = make_service(); + let req = Request::builder() + .method("GET") + .uri("/_ts/admin/ec") + .body(AxumBody::empty()) + .expect("should build unauthenticated admin EC request"); + let resp = svc + .ready() + .await + .expect("should be ready") + .call(req) + .await + .expect("should respond"); + + assert_eq!(resp.status().as_u16(), 401); + assert!( + resp.headers().contains_key("www-authenticate"), + "admin EC 401 should include the Basic authentication challenge" + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn authenticated_admin_eids_route_returns_200() { + // The EIDs echo is pure request inspection (no KV), so the dev server + // serves the real handler. + let mut svc = make_service(); + let req = Request::builder() + .method("GET") + .uri("/_ts/admin/eids") + .header("authorization", "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(AxumBody::empty()) + .expect("should build request"); + let resp = svc + .ready() + .await + .expect("should be ready") + .call(req) + .await + .expect("should respond"); + assert_eq!( + resp.status().as_u16(), + 200, + "/_ts/admin/eids should serve the real EIDs echo handler" + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn authenticated_admin_diagnostic_fallback_is_denied_locally() { + let ec_id = format!("{}.abc123", "a".repeat(64)); + let valid_paths = [ + "/_ts/admin/ec".to_owned(), + format!("/_ts/admin/ec/{ec_id}"), + "/_ts/admin/eids".to_owned(), + ]; + + for path in valid_paths { + for method in ["POST", "HEAD", "OPTIONS", "PUT", "PATCH", "DELETE"] { + let request = Request::builder() + .method(method) + .uri(&path) + .header("authorization", "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(AxumBody::from("sensitive-admin-body")) + .expect("should build authenticated admin request"); + let response = make_service() + .ready() + .await + .expect("should be ready") + .call(request) + .await + .expect("should respond"); + + assert_eq!(response.status().as_u16(), 405); + assert_eq!( + response + .headers() + .get("allow") + .and_then(|v| v.to_str().ok()), + Some("GET") + ); + assert_eq!( + response + .headers() + .get("cache-control") + .and_then(|v| v.to_str().ok()), + Some("no-store") + ); + } + } + + for path in [ + "/_ts/admin/ec/".to_owned(), + format!("/_ts/admin/ec/{ec_id}/extra"), + "/_ts/admin/eids/".to_owned(), + "/_ts/admin/eids/extra".to_owned(), + "/_ts/admin/eids.json".to_owned(), + "/_ts/admin/ec;foo".to_owned(), + format!("/_ts/admin/ec%2F{ec_id}"), + // Percent-encoded separators match the `^/_ts/admin` basic-auth + // handler but not a literal-slash namespace check, so they must be + // reserved before publisher fallback forwards credentials upstream. + "/_ts/admin%2Fec".to_owned(), + "/_ts/admin%2fec".to_owned(), + // Retired non-`/_ts` alias namespace: only the two exact paths are + // routed to a local deny, so descendants and encoded separators must + // be reserved at the shared fallback boundary. + "/admin/keys".to_owned(), + "/admin/keys/rotate/extra".to_owned(), + "/admin/keys%2Frotate".to_owned(), + "/admin%2fkeys/rotate".to_owned(), + ] { + for method in ["GET", "POST"] { + let request = Request::builder() + .method(method) + .uri(&path) + .header("authorization", "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(AxumBody::from("sensitive-admin-body")) + .expect("should build malformed admin request"); + let response = make_service() + .ready() + .await + .expect("should be ready") + .call(request) + .await + .expect("should respond"); + + assert_eq!(response.status().as_u16(), 404); + assert_eq!( + response + .headers() + .get("cache-control") + .and_then(|v| v.to_str().ok()), + Some("no-store") + ); + } + } +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn legacy_admin_aliases_denied_locally_not_proxied_to_publisher() { // Regression for the credential-leak finding: the production basic-auth regex diff --git a/crates/trusted-server-adapter-cloudflare/src/app.rs b/crates/trusted-server-adapter-cloudflare/src/app.rs index 644676fc5..cade09a4e 100644 --- a/crates/trusted-server-adapter-cloudflare/src/app.rs +++ b/crates/trusted-server-adapter-cloudflare/src/app.rs @@ -13,6 +13,11 @@ use trusted_server_core::auction::{AuctionOrchestrator, build_orchestrator}; #[cfg(target_arch = "wasm32")] use trusted_server_core::config_payload::settings_from_config_blob; use trusted_server_core::ec::EcContext; +use trusted_server_core::ec::admin::{ + admin_ec_lookup_not_supported as core_admin_ec_lookup_not_supported, + deny_admin_diagnostic_fallback, handle_admin_eids_lookup, +}; +use trusted_server_core::ec::registry::PartnerRegistry; use trusted_server_core::error::{IntoHttpResponse as _, TrustedServerError}; use trusted_server_core::integrations::{IntegrationRegistry, ProxyDispatchInput}; use trusted_server_core::platform::RuntimeServices; @@ -249,6 +254,10 @@ fn admin_key_management_not_supported() -> Response { response } +fn admin_ec_lookup_not_supported() -> Response { + core_admin_ec_lookup_not_supported() +} + /// Builds the local `404 Not Found` returned for legacy `/admin/keys/*` /// aliases on the Cloudflare adapter. /// @@ -368,6 +377,9 @@ fn build_router(state: &Arc) -> RouterService { ) -> Result { let services = build_per_request_services(&ctx); let mut req = ctx.into_request(); + if let Some(response) = deny_admin_diagnostic_fallback(&req) { + return Ok(response); + } if let Err(error) = trusted_server_core::integrations::gpt_diagnostics::prepare_request( &state.settings, &mut req, @@ -474,6 +486,26 @@ fn build_router(state: &Arc) -> RouterService { .post("/_ts/admin/keys/deactivate", |_ctx: RequestContext| async { Ok::(admin_key_management_not_supported()) }) + // Admin EC lookup routes. Registered explicitly (like the key + // routes above) so they never fall through to the publisher + // fallback, and they match `Settings::ADMIN_ENDPOINTS` for auth + // coverage. The EC identity graph is Fastly KV backed, so this + // adapter has no store to read. + .get("/_ts/admin/ec", |_ctx: RequestContext| async { + Ok::(admin_ec_lookup_not_supported()) + }) + .get("/_ts/admin/ec/{id}", |_ctx: RequestContext| async { + Ok::(admin_ec_lookup_not_supported()) + }) + // Admin EIDs echo: pure request inspection (no KV), so this + // adapter serves the real handler. + .get( + "/_ts/admin/eids", + make_handler(Arc::clone(&state), |s, _services, req| async move { + let partner_registry = PartnerRegistry::from_config(&s.settings.ec.partners)?; + handle_admin_eids_lookup(&partner_registry, &req) + }), + ) .post( "/auction", make_handler(Arc::clone(&state), |s, services, req| async move { diff --git a/crates/trusted-server-adapter-cloudflare/tests/routes.rs b/crates/trusted-server-adapter-cloudflare/tests/routes.rs index 09e3ed324..1ad07bcdc 100644 --- a/crates/trusted-server-adapter-cloudflare/tests/routes.rs +++ b/crates/trusted-server-adapter-cloudflare/tests/routes.rs @@ -215,6 +215,9 @@ fn all_explicit_routes_are_registered() { ("POST", "/verify-signature"), ("POST", "/_ts/admin/keys/rotate"), ("POST", "/_ts/admin/keys/deactivate"), + ("GET", "/_ts/admin/ec"), + ("GET", "/_ts/admin/ec/{id}"), + ("GET", "/_ts/admin/eids"), ("POST", "/auction"), // SPA re-auction endpoint, plus its deprecated `/__ts/` alias. Both // paths are spelled out as literals rather than referencing @@ -275,6 +278,158 @@ async fn authenticated_admin_routes_return_501() { } } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn authenticated_admin_ec_routes_return_501() { + // The EC identity graph is Fastly KV backed, so Cloudflare answers the + // admin EC lookup routes locally with 501 instead of letting them fall + // through to the publisher fallback. + let sample_ec_id = format!("{}.abc123", "a".repeat(64)); + for path in [ + "/_ts/admin/ec".to_owned(), + format!("/_ts/admin/ec/{sample_ec_id}"), + ] { + let req = request_builder() + .method("GET") + .uri(&path) + .header("authorization", "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(edgezero_core::body::Body::empty()) + .expect("should build request"); + let resp = route(test_router(), req).await; + + assert_eq!( + resp.status().as_u16(), + 501, + "{path} should report that Cloudflare EC lookup is unsupported" + ); + assert_eq!( + resp.headers() + .get("content-type") + .and_then(|v| v.to_str().ok()), + Some("application/json") + ); + assert_eq!( + resp.headers() + .get("cache-control") + .and_then(|v| v.to_str().ok()), + Some("no-store") + ); + } +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn admin_ec_route_without_credentials_returns_401() { + let req = request_builder() + .method("GET") + .uri("/_ts/admin/ec") + .body(edgezero_core::body::Body::empty()) + .expect("should build unauthenticated admin EC request"); + let resp = route(test_router(), req).await; + + assert_eq!(resp.status().as_u16(), 401); + assert!( + resp.headers().contains_key("www-authenticate"), + "admin EC 401 should include the Basic authentication challenge" + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn authenticated_admin_eids_route_returns_200() { + // The EIDs echo is pure request inspection (no KV), so this adapter + // serves the real handler. + let req = request_builder() + .method("GET") + .uri("/_ts/admin/eids") + .header("authorization", "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(edgezero_core::body::Body::empty()) + .expect("should build request"); + let resp = route(test_router(), req).await; + + assert_eq!( + resp.status().as_u16(), + 200, + "/_ts/admin/eids should serve the real EIDs echo handler" + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn authenticated_admin_diagnostic_fallback_is_denied_locally() { + let ec_id = format!("{}.abc123", "a".repeat(64)); + let valid_paths = [ + "/_ts/admin/ec".to_owned(), + format!("/_ts/admin/ec/{ec_id}"), + "/_ts/admin/eids".to_owned(), + ]; + + for path in valid_paths { + for method in ["POST", "HEAD", "OPTIONS", "PUT", "PATCH", "DELETE"] { + let request = request_builder() + .method(method) + .uri(&path) + .header("authorization", "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(edgezero_core::body::Body::from("sensitive-admin-body")) + .expect("should build authenticated admin request"); + let response = route(test_router(), request).await; + + assert_eq!(response.status().as_u16(), 405); + assert_eq!( + response + .headers() + .get("allow") + .and_then(|v| v.to_str().ok()), + Some("GET") + ); + assert_eq!( + response + .headers() + .get("cache-control") + .and_then(|v| v.to_str().ok()), + Some("no-store") + ); + } + } + + for path in [ + "/_ts/admin/ec/".to_owned(), + format!("/_ts/admin/ec/{ec_id}/extra"), + "/_ts/admin/eids/".to_owned(), + "/_ts/admin/eids/extra".to_owned(), + "/_ts/admin/eids.json".to_owned(), + "/_ts/admin/ec;foo".to_owned(), + format!("/_ts/admin/ec%2F{ec_id}"), + // Percent-encoded separators match the `^/_ts/admin` basic-auth + // handler but not a literal-slash namespace check, so they must be + // reserved before publisher fallback forwards credentials upstream. + "/_ts/admin%2Fec".to_owned(), + "/_ts/admin%2fec".to_owned(), + // Retired non-`/_ts` alias namespace: only the two exact paths are + // routed to a local deny, so descendants and encoded separators must + // be reserved at the shared fallback boundary. + "/admin/keys".to_owned(), + "/admin/keys/rotate/extra".to_owned(), + "/admin/keys%2Frotate".to_owned(), + "/admin%2fkeys/rotate".to_owned(), + ] { + for method in ["GET", "POST"] { + let request = request_builder() + .method(method) + .uri(&path) + .header("authorization", "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(edgezero_core::body::Body::from("sensitive-admin-body")) + .expect("should build malformed admin request"); + let response = route(test_router(), request).await; + + assert_eq!(response.status().as_u16(), 404); + assert_eq!( + response + .headers() + .get("cache-control") + .and_then(|v| v.to_str().ok()), + Some("no-store") + ); + } + } +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn admin_route_without_credentials_returns_401() { let router = test_router(); diff --git a/crates/trusted-server-adapter-fastly/src/app.rs b/crates/trusted-server-adapter-fastly/src/app.rs index d6090c983..000768666 100644 --- a/crates/trusted-server-adapter-fastly/src/app.rs +++ b/crates/trusted-server-adapter-fastly/src/app.rs @@ -22,6 +22,9 @@ //! | POST | `/verify-signature` | [`handle_verify_signature`] | //! | POST | `/_ts/admin/keys/rotate` | [`handle_rotate_key`] | //! | POST | `/_ts/admin/keys/deactivate` | [`handle_deactivate_key`] | +//! | GET | `/_ts/admin/ec` | [`handle_admin_ec_lookup`] | +//! | GET | `/_ts/admin/ec/{id}` | [`handle_admin_ec_lookup`] | +//! | GET | `/_ts/admin/eids` | [`handle_admin_eids_lookup`] | //! | POST | `/_ts/api/v1/batch-sync` | [`handle_batch_sync`] | //! | GET | `/_ts/api/v1/identify` | [`handle_identify`] | //! | GET | `/_ts/set-tester` | [`handle_set_tester`] | @@ -49,7 +52,8 @@ //! `route_request` (tracked in issue #495): //! //! - [`build_ec_request_state`] runs before every dispatched route (except -//! batch-sync, which uses Bearer auth) and reproduces the legacy +//! batch-sync, which uses Bearer auth, and the read-only admin diagnostics) +//! and reproduces the legacy //! pre-routing prelude: device signals, bot gate, `ts-eids`/`sharedid` //! cookie capture, geo lookup, [`EcContext`] creation, and KV-graph gating. //! - `handle_auction` and integration proxy dispatch receive the same @@ -99,6 +103,9 @@ use trusted_server_core::auction::endpoints::handle_auction; use trusted_server_core::auction::{AuctionOrchestrator, build_orchestrator}; use trusted_server_core::constants::{COOKIE_SHAREDID, COOKIE_TS_EIDS}; use trusted_server_core::ec::EcContext; +use trusted_server_core::ec::admin::{ + deny_admin_diagnostic_fallback, handle_admin_ec_lookup, handle_admin_eids_lookup, +}; use trusted_server_core::ec::batch_sync::handle_batch_sync; use trusted_server_core::ec::consent::ec_consent_withdrawn; use trusted_server_core::ec::device::DeviceSignals; @@ -525,6 +532,29 @@ async fn execute_named( return Ok(run_batch_sync(&state, &services, req)); } + // These diagnostics are read-only. Running the normal EC lifecycle would + // attach finalization state and could ingest request cookies into KV after + // the handler returns, violating that contract. + if matches!( + handler, + NamedRouteHandler::AdminEcLookup | NamedRouteHandler::AdminEidsLookup + ) { + let response = PartnerRegistry::from_config(&state.settings.ec.partners) + .and_then(|registry| match handler { + NamedRouteHandler::AdminEcLookup => { + // Deliberately do not use an EC request-state graph: that + // copy is bot-gated, while operators use curl for this + // authenticated diagnostic. + let kv = crate::maybe_identity_graph(&state.settings); + handle_admin_ec_lookup(kv.as_ref(), ®istry, &req) + } + NamedRouteHandler::AdminEidsLookup => handle_admin_eids_lookup(®istry, &req), + _ => unreachable!("admin diagnostics should use early dispatch"), + }) + .unwrap_or_else(|error| http_error(&error)); + return Ok(response); + } + if let Err(report) = trusted_server_core::integrations::gpt_diagnostics::prepare_request( &state.settings, &mut req, @@ -574,6 +604,9 @@ async fn run_named_route( } NamedRouteHandler::RotateKey => handle_rotate_key(&state.settings, services, req), NamedRouteHandler::DeactivateKey => handle_deactivate_key(&state.settings, services, req), + NamedRouteHandler::AdminEcLookup | NamedRouteHandler::AdminEidsLookup => { + unreachable!("admin diagnostics should be handled before EC setup") + } NamedRouteHandler::LegacyAdminDenied => Ok(legacy_admin_alias_denied()), NamedRouteHandler::BatchSync => { // Dispatched by execute_named before EC state is built. @@ -708,6 +741,10 @@ async fn dispatch_fallback( services: &RuntimeServices, mut req: Request, ) -> Response { + if let Some(response) = deny_admin_diagnostic_fallback(&req) { + return response; + } + let path = req.uri().path().to_string(); let method = req.method().clone(); @@ -1001,6 +1038,8 @@ enum NamedRouteHandler { VerifySignature, RotateKey, DeactivateKey, + AdminEcLookup, + AdminEidsLookup, /// Legacy `/admin/keys/*` aliases — denied locally with 404 so they never /// reach the publisher fallback (which would leak admin credentials). LegacyAdminDenied, @@ -1053,6 +1092,25 @@ const NAMED_ROUTES: &[NamedRoute] = &[ primary_methods: &[Method::POST], handler: NamedRouteHandler::DeactivateKey, }, + // Admin EC lookup: the bare route reads the EC ID from the caller's + // `ts-ec` cookie; the parameterized route takes an explicit EC ID. + NamedRoute { + path: "/_ts/admin/ec", + primary_methods: &[Method::GET], + handler: NamedRouteHandler::AdminEcLookup, + }, + NamedRoute { + path: "/_ts/admin/ec/{id}", + primary_methods: &[Method::GET], + handler: NamedRouteHandler::AdminEcLookup, + }, + // Admin EIDs echo: decodes the request's ts-eids/sharedId cookies with + // an ingestion preview. Pure request inspection — no KV access. + NamedRoute { + path: "/_ts/admin/eids", + primary_methods: &[Method::GET], + handler: NamedRouteHandler::AdminEidsLookup, + }, // The legacy non-`/_ts` aliases (`/admin/keys/*`) are denied locally with a // 404 instead of executing key operations: the production basic-auth handler // regex `^/_ts/admin` does not match them, and letting them fall through to @@ -1241,6 +1299,7 @@ mod tests { AppState, NAMED_ROUTES, NamedRouteHandler, PAGE_BIDS_LEGACY_PATH, PAGE_BIDS_PATH, TrustedServerApp, build_state_from_settings, startup_error_router, }; + use base64::Engine as _; use bytes::Bytes; use edgezero_core::body::Body; use edgezero_core::http::{Method, Response, StatusCode, header, request_builder}; @@ -1651,6 +1710,44 @@ mod tests { } } + #[test] + fn admin_ec_lookup_routes_are_registered() { + // Both lookup shapes must be explicitly routed to the admin EC + // handler: the bare cookie-based route and the parameterized route. + // Leaving either unrouted would fall through to the publisher + // fallback, forwarding the caller's `Authorization` header to the + // origin. + for path in ["/_ts/admin/ec", "/_ts/admin/ec/{id}"] { + let route = NAMED_ROUTES + .iter() + .find(|route| route.path == path) + .unwrap_or_else(|| panic!("{path} must be a named route")); + assert!( + matches!(route.handler, NamedRouteHandler::AdminEcLookup), + "{path} must map to the admin EC lookup handler" + ); + assert_eq!( + route.primary_methods, + &[Method::GET], + "{path} must have GET as its only primary method" + ); + } + + let eids_route = NAMED_ROUTES + .iter() + .find(|route| route.path == "/_ts/admin/eids") + .expect("should register /_ts/admin/eids as a named route"); + assert!( + matches!(eids_route.handler, NamedRouteHandler::AdminEidsLookup), + "/_ts/admin/eids must map to the admin EIDs lookup handler" + ); + assert_eq!( + eids_route.primary_methods, + &[Method::GET], + "/_ts/admin/eids must have GET as its only primary method" + ); + } + #[test] fn page_bids_serves_canonical_path_and_deprecated_alias() { // The SPA re-auction endpoint lives at the canonical single-underscore @@ -1744,6 +1841,93 @@ mod tests { } } + #[test] + fn authenticated_admin_diagnostic_fallback_is_denied_locally() { + let router = test_router(); + let ec_id = format!("{}.abc123", "a".repeat(64)); + let valid_paths = [ + "/_ts/admin/ec".to_owned(), + format!("/_ts/admin/ec/{ec_id}"), + "/_ts/admin/eids".to_owned(), + ]; + + for path in valid_paths { + for method in [ + Method::POST, + Method::HEAD, + Method::OPTIONS, + Method::PUT, + Method::PATCH, + Method::DELETE, + ] { + let request = request_builder() + .method(method.clone()) + .uri(format!("https://test-publisher.com{path}")) + .header(header::AUTHORIZATION, "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(Body::from("sensitive-admin-body")) + .expect("should build authenticated admin request"); + let response = route(&router, request); + + assert_eq!(response.status(), StatusCode::METHOD_NOT_ALLOWED); + assert_eq!( + response + .headers() + .get(header::ALLOW) + .and_then(|v| v.to_str().ok()), + Some("GET") + ); + assert_eq!( + response + .headers() + .get(header::CACHE_CONTROL) + .and_then(|v| v.to_str().ok()), + Some("no-store") + ); + } + } + + for path in [ + "/_ts/admin/ec/".to_owned(), + format!("/_ts/admin/ec/{ec_id}/extra"), + "/_ts/admin/eids/".to_owned(), + "/_ts/admin/eids/extra".to_owned(), + "/_ts/admin/eids.json".to_owned(), + "/_ts/admin/ec;foo".to_owned(), + format!("/_ts/admin/ec%2F{ec_id}"), + // Percent-encoded separators match the `^/_ts/admin` basic-auth + // handler but not a literal-slash namespace check, so they must be + // reserved before publisher fallback forwards credentials upstream. + "/_ts/admin%2Fec".to_owned(), + "/_ts/admin%2fec".to_owned(), + // Retired non-`/_ts` alias namespace: only the two exact paths are + // routed to a local deny, so descendants and encoded separators must + // be reserved at the shared fallback boundary. + "/admin/keys".to_owned(), + "/admin/keys/rotate/extra".to_owned(), + "/admin/keys%2Frotate".to_owned(), + "/admin%2fkeys/rotate".to_owned(), + ] { + for method in [Method::GET, Method::POST] { + let request = request_builder() + .method(method) + .uri(format!("https://test-publisher.com{path}")) + .header(header::AUTHORIZATION, "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(Body::from("sensitive-admin-body")) + .expect("should build malformed admin request"); + let response = route(&router, request); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + assert_eq!( + response + .headers() + .get(header::CACHE_CONTROL) + .and_then(|v| v.to_str().ok()), + Some("no-store") + ); + } + } + } + #[test] fn dispatch_identify_options_routes_to_cors_preflight() { // Parity guard: OPTIONS /_ts/api/v1/identify must reach @@ -2029,6 +2213,103 @@ mod tests { ); } + #[test] + fn admin_eids_diagnostic_skips_ec_finalization() { + let router = test_router(); + let ec_id = format!("{}.abc123", "a".repeat(64)); + let eids = serde_json::json!([{ + "source": "example.com", + "uids": [{ "id": "example-uid", "atype": 1 }] + }]); + let eids_cookie = base64::engine::general_purpose::STANDARD.encode(eids.to_string()); + let mut request = request_builder() + .method(Method::GET) + .uri("https://test-publisher.com/_ts/admin/eids") + .header(header::AUTHORIZATION, "Basic YWRtaW46YWRtaW4tcGFzcw==") + .header( + header::COOKIE, + format!("ts-ec={ec_id}; ts-eids={eids_cookie}; sharedId=example-shared-id"), + ) + .body(Body::empty()) + .expect("should build authenticated EIDs diagnostic request"); + request.extensions_mut().insert(DeviceSignals::derive( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 \ + (KHTML, like Gecko) Chrome/146.0.0.0 Safari/537.36", + Some("t13d1516h2_8daaf6152771_b186095e22b6"), + Some("1:65536;2:0;4:6291456;6:262144"), + )); + + let response = route(&router, request); + + assert_eq!(response.status(), StatusCode::OK); + assert!( + response + .extensions() + .get::() + .is_none(), + "admin EIDs diagnostics should not attach EC finalization state" + ); + } + + #[test] + fn admin_ec_diagnostic_skips_ec_finalization() { + let router = test_router(); + let ec_id = format!("{}.abc123", "a".repeat(64)); + let eids = serde_json::json!([{ + "source": "example.com", + "uids": [{ "id": "example-uid", "atype": 1 }] + }]); + let eids_cookie = base64::engine::general_purpose::STANDARD.encode(eids.to_string()); + let mut request = request_builder() + .method(Method::GET) + .uri(format!("https://test-publisher.com/_ts/admin/ec/{ec_id}")) + .header(header::AUTHORIZATION, "Basic YWRtaW46YWRtaW4tcGFzcw==") + .header( + header::COOKIE, + format!("ts-ec={ec_id}; ts-eids={eids_cookie}; sharedId=example-shared-id"), + ) + .body(Body::empty()) + .expect("should build authenticated EC diagnostic request"); + request.extensions_mut().insert(DeviceSignals::derive( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 \ + (KHTML, like Gecko) Chrome/146.0.0.0 Safari/537.36", + Some("t13d1516h2_8daaf6152771_b186095e22b6"), + Some("1:65536;2:0;4:6291456;6:262144"), + )); + + let response = route(&router, request); + + assert_eq!( + response.status(), + StatusCode::NOT_IMPLEMENTED, + "configured admin EC handler should run and report the unavailable test KV graph" + ); + assert!( + response + .extensions() + .get::() + .is_none(), + "admin EC diagnostics should not attach EC finalization state" + ); + assert!( + response.headers().get(header::SET_COOKIE).is_none(), + "admin EC diagnostics should not mutate the EC cookie" + ); + } + + #[test] + fn admin_ec_route_without_credentials_returns_401() { + let router = test_router(); + + let response = route(&router, empty_request(Method::GET, "/_ts/admin/ec")); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert!( + response.headers().contains_key(header::WWW_AUTHENTICATE), + "admin EC 401 should include the Basic authentication challenge" + ); + } + #[test] fn dispatch_head_on_named_get_route_falls_through_to_publisher_fallback() { // Regression guard: HEAD /first-party/proxy must reach the publisher diff --git a/crates/trusted-server-adapter-spin/src/app.rs b/crates/trusted-server-adapter-spin/src/app.rs index 960bafc41..77aaa0567 100644 --- a/crates/trusted-server-adapter-spin/src/app.rs +++ b/crates/trusted-server-adapter-spin/src/app.rs @@ -11,6 +11,11 @@ use error_stack::Report; use trusted_server_core::auction::endpoints::handle_auction; use trusted_server_core::auction::{AuctionOrchestrator, build_orchestrator}; use trusted_server_core::ec::EcContext; +use trusted_server_core::ec::admin::{ + admin_ec_lookup_not_supported as core_admin_ec_lookup_not_supported, + deny_admin_diagnostic_fallback, handle_admin_eids_lookup, +}; +use trusted_server_core::ec::registry::PartnerRegistry; use trusted_server_core::error::{IntoHttpResponse as _, TrustedServerError}; use trusted_server_core::http_util::sanitize_forwarded_headers; use trusted_server_core::integrations::{IntegrationRegistry, ProxyDispatchInput}; @@ -142,12 +147,15 @@ const LEGACY_ADMIN_DENY_METHODS: &[Method] = &[ Method::DELETE, ]; -fn named_fallback_paths() -> [(&'static str, &'static [Method]); 13] { +fn named_fallback_paths() -> [(&'static str, &'static [Method]); 16] { [ ("/.well-known/trusted-server.json", &[Method::GET]), ("/verify-signature", &[Method::POST]), ("/_ts/admin/keys/rotate", &[Method::POST]), ("/_ts/admin/keys/deactivate", &[Method::POST]), + ("/_ts/admin/ec", &[Method::GET]), + ("/_ts/admin/ec/{id}", &[Method::GET]), + ("/_ts/admin/eids", &[Method::GET]), ("/admin/keys/rotate", LEGACY_ADMIN_DENY_METHODS), ("/admin/keys/deactivate", LEGACY_ADMIN_DENY_METHODS), ("/auction", &[Method::POST]), @@ -361,6 +369,10 @@ fn admin_key_management_not_supported() -> Response { response } +fn admin_ec_lookup_not_supported() -> Response { + core_admin_ec_lookup_not_supported() +} + // --------------------------------------------------------------------------- // Error helper // --------------------------------------------------------------------------- @@ -513,6 +525,23 @@ fn build_router(state: &Arc) -> RouterService { Ok::(admin_key_management_not_supported()) }; + let admin_ec_not_supported_handler = |_ctx: RequestContext| async { + Ok::(admin_ec_lookup_not_supported()) + }; + + // Admin EIDs echo: pure request inspection (no KV), so this adapter + // serves the real handler. + let s = Arc::clone(&state); + let admin_eids_handler = move |ctx: RequestContext| { + let s = Arc::clone(&s); + async move { + let req = ctx.into_request(); + let result = PartnerRegistry::from_config(&s.settings.ec.partners) + .and_then(|registry| handle_admin_eids_lookup(®istry, &req)); + Ok::(result.unwrap_or_else(|e| http_error(&e))) + } + }; + // /auction let s = Arc::clone(&state); let auction_handler = move |ctx: RequestContext| { @@ -652,6 +681,9 @@ fn build_router(state: &Arc) -> RouterService { ) -> Result { let services = build_runtime_services(&ctx); let mut req = ctx.into_request(); + if let Some(response) = deny_admin_diagnostic_fallback(&req) { + return Ok(response); + } if let Err(error) = trusted_server_core::integrations::gpt_diagnostics::prepare_request( &state.settings, &mut req, @@ -758,6 +790,14 @@ fn build_router(state: &Arc) -> RouterService { // credentials and key-management payloads to the origin. .post("/_ts/admin/keys/rotate", admin_not_supported_handler) .post("/_ts/admin/keys/deactivate", admin_not_supported_handler) + // Admin EC lookup routes. Registered explicitly (like the key + // routes above) so they never fall through to the publisher + // fallback, and they match `Settings::ADMIN_ENDPOINTS` for auth + // coverage. The EC identity graph is Fastly KV backed, so this + // adapter has no store to read. + .get("/_ts/admin/ec", admin_ec_not_supported_handler) + .get("/_ts/admin/ec/{id}", admin_ec_not_supported_handler) + .get("/_ts/admin/eids", admin_eids_handler) .post("/auction", auction_handler) .get(PAGE_BIDS_PATH, page_bids_handler.clone()) .route(PAGE_BIDS_PATH, Method::OPTIONS, page_bids_options_handler) diff --git a/crates/trusted-server-adapter-spin/tests/routes.rs b/crates/trusted-server-adapter-spin/tests/routes.rs index 2f7b1037e..b130b9a28 100644 --- a/crates/trusted-server-adapter-spin/tests/routes.rs +++ b/crates/trusted-server-adapter-spin/tests/routes.rs @@ -113,6 +113,158 @@ async fn authenticated_admin_routes_return_501() { } } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn authenticated_admin_ec_routes_return_501() { + // The EC identity graph is Fastly KV backed, so Spin answers the admin + // EC lookup routes locally with 501 instead of letting them fall through + // to the publisher fallback. + let sample_ec_id = format!("{}.abc123", "a".repeat(64)); + for path in [ + "/_ts/admin/ec".to_owned(), + format!("/_ts/admin/ec/{sample_ec_id}"), + ] { + let req = request_builder() + .method("GET") + .uri(&path) + .header("authorization", "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(edgezero_core::body::Body::empty()) + .expect("should build request"); + let resp = route(test_router(), req).await; + + assert_eq!( + resp.status().as_u16(), + 501, + "{path} should report that Spin EC lookup is unsupported" + ); + assert_eq!( + resp.headers() + .get("content-type") + .and_then(|v| v.to_str().ok()), + Some("application/json") + ); + assert_eq!( + resp.headers() + .get("cache-control") + .and_then(|v| v.to_str().ok()), + Some("no-store") + ); + } +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn admin_ec_route_without_credentials_returns_401() { + let req = request_builder() + .method("GET") + .uri("/_ts/admin/ec") + .body(edgezero_core::body::Body::empty()) + .expect("should build unauthenticated admin EC request"); + let resp = route(test_router(), req).await; + + assert_eq!(resp.status().as_u16(), 401); + assert!( + resp.headers().contains_key("www-authenticate"), + "admin EC 401 should include the Basic authentication challenge" + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn authenticated_admin_eids_route_returns_200() { + // The EIDs echo is pure request inspection (no KV), so this adapter + // serves the real handler. + let req = request_builder() + .method("GET") + .uri("/_ts/admin/eids") + .header("authorization", "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(edgezero_core::body::Body::empty()) + .expect("should build request"); + let resp = route(test_router(), req).await; + + assert_eq!( + resp.status().as_u16(), + 200, + "/_ts/admin/eids should serve the real EIDs echo handler" + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn authenticated_admin_diagnostic_fallback_is_denied_locally() { + let ec_id = format!("{}.abc123", "a".repeat(64)); + let valid_paths = [ + "/_ts/admin/ec".to_owned(), + format!("/_ts/admin/ec/{ec_id}"), + "/_ts/admin/eids".to_owned(), + ]; + + for path in valid_paths { + for method in ["POST", "HEAD", "OPTIONS", "PUT", "PATCH", "DELETE"] { + let request = request_builder() + .method(method) + .uri(&path) + .header("authorization", "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(edgezero_core::body::Body::from("sensitive-admin-body")) + .expect("should build authenticated admin request"); + let response = route(test_router(), request).await; + + assert_eq!(response.status().as_u16(), 405); + assert_eq!( + response + .headers() + .get("allow") + .and_then(|v| v.to_str().ok()), + Some("GET") + ); + assert_eq!( + response + .headers() + .get("cache-control") + .and_then(|v| v.to_str().ok()), + Some("no-store") + ); + } + } + + for path in [ + "/_ts/admin/ec/".to_owned(), + format!("/_ts/admin/ec/{ec_id}/extra"), + "/_ts/admin/eids/".to_owned(), + "/_ts/admin/eids/extra".to_owned(), + "/_ts/admin/eids.json".to_owned(), + "/_ts/admin/ec;foo".to_owned(), + format!("/_ts/admin/ec%2F{ec_id}"), + // Percent-encoded separators match the `^/_ts/admin` basic-auth + // handler but not a literal-slash namespace check, so they must be + // reserved before publisher fallback forwards credentials upstream. + "/_ts/admin%2Fec".to_owned(), + "/_ts/admin%2fec".to_owned(), + // Retired non-`/_ts` alias namespace: only the two exact paths are + // routed to a local deny, so descendants and encoded separators must + // be reserved at the shared fallback boundary. + "/admin/keys".to_owned(), + "/admin/keys/rotate/extra".to_owned(), + "/admin/keys%2Frotate".to_owned(), + "/admin%2fkeys/rotate".to_owned(), + ] { + for method in ["GET", "POST"] { + let request = request_builder() + .method(method) + .uri(&path) + .header("authorization", "Basic YWRtaW46YWRtaW4tcGFzcw==") + .body(edgezero_core::body::Body::from("sensitive-admin-body")) + .expect("should build malformed admin request"); + let response = route(test_router(), request).await; + + assert_eq!(response.status().as_u16(), 404); + assert_eq!( + response + .headers() + .get("cache-control") + .and_then(|v| v.to_str().ok()), + Some("no-store") + ); + } + } +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn health_route_returns_ok() { // Parity with the Fastly/Axum adapters: GET /health is a cheap liveness probe diff --git a/crates/trusted-server-core/src/auction/endpoints.rs b/crates/trusted-server-core/src/auction/endpoints.rs index c4af6fd3d..326453157 100644 --- a/crates/trusted-server-core/src/auction/endpoints.rs +++ b/crates/trusted-server-core/src/auction/endpoints.rs @@ -11,6 +11,7 @@ use crate::auction::formats::AdRequest; use crate::auction::orchestrator::OrchestrationResult; use crate::consent::{consent_allows_server_side_auction, gate_eids_by_consent}; use crate::constants::COOKIE_TS_EIDS; +use crate::cookies::extract_cookie_value; use crate::ec::EcContext; use crate::ec::eids::{resolve_partner_ids, to_eids}; use crate::ec::kv::KvIdentityGraph; @@ -408,22 +409,6 @@ pub(crate) fn resolve_auction_eids( Some(to_eids(&resolved)) } -fn extract_cookie_value(req: &Request, name: &str) -> Option { - let cookie_header = req - .headers() - .get(header::COOKIE) - .and_then(|v| v.to_str().ok())?; - for pair in cookie_header.split(';') { - let pair = pair.trim(); - if let Some((key, value)) = pair.split_once('=') - && key.trim() == name - { - return Some(value.trim().to_owned()); - } - } - None -} - pub(crate) fn resolve_client_auction_eids( raw: Option<&JsonValue>, cookie_value: Option<&str>, diff --git a/crates/trusted-server-core/src/auth.rs b/crates/trusted-server-core/src/auth.rs index 8e70aa020..6c92d042d 100644 --- a/crates/trusted-server-core/src/auth.rs +++ b/crates/trusted-server-core/src/auth.rs @@ -20,7 +20,9 @@ const BASIC_AUTH_REALM: &str = r#"Basic realm="Trusted Server""#; /// Admin endpoints are protected by requiring a handler during settings /// finalization; see [`Settings::from_toml`]. Credential checks use constant-time /// comparison for both username and password, and evaluate both regardless of -/// individual match results to avoid timing oracles. +/// individual match results to avoid timing oracles. Runtime requests within +/// the reserved admin namespace fail closed if no handler matches, providing +/// defense in depth for malformed and parameterized paths. /// /// # Errors /// @@ -30,7 +32,13 @@ pub fn enforce_basic_auth( settings: &Settings, req: &Request, ) -> Result>, Report> { - let Some(handler) = settings.handler_for_path(req.uri().path())? else { + let path = req.uri().path(); + let Some(handler) = settings.handler_for_path(path)? else { + if Settings::is_admin_path(path) { + return Err(Report::new(TrustedServerError::Configuration { + message: format!("Admin path `{path}` has no configured handler"), + })); + } return Ok(None); }; @@ -117,6 +125,29 @@ mod tests { ); } + #[test] + fn encoded_admin_separator_path_is_auth_gated() { + // `^/_ts/admin` matches the raw path, so a percent-encoded separator + // still consumes admin credentials. The publisher-fallback boundary + // reserves the same paths so those credentials are never forwarded + // upstream (see `ec::admin::deny_admin_diagnostic_fallback`). + let settings = create_test_settings(); + + for path in ["/_ts/admin%2Fec", "/_ts/admin%2fec"] { + let req = build_request(Method::GET, &format!("https://example.com{path}")); + + let response = enforce_basic_auth(&settings, &req) + .expect("should evaluate auth") + .unwrap_or_else(|| panic!("should challenge {path}")); + + assert_eq!( + response.status(), + StatusCode::UNAUTHORIZED, + "should require credentials for {path}" + ); + } + } + #[test] fn no_challenge_for_non_protected_path() { let settings = create_test_settings(); @@ -304,4 +335,53 @@ mod tests { .expect("should challenge admin path with missing credentials"); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); } + + #[test] + fn concrete_admin_path_without_matching_handler_fails_closed() { + let config = crate_test_settings_str().replace( + r#"path = "^/_ts/admin" + username = "admin" + password = "admin-pass""#, + r#"path = "^/_ts/admin/(keys/rotate|keys/deactivate|ec|eids)$" + username = "admin" + password = "strong-test-password" + + [[handlers]] + path = "^/_ts/admin/ec/[{]id[}]$" + username = "admin" + password = "strong-test-password""#, + ); + let settings: Settings = + toml::from_str(&config).expect("should deserialize settings without finalization"); + let ec_id = format!("{}.abc123", "a".repeat(64)); + let req = build_request( + Method::GET, + &format!("https://example.com/_ts/admin/ec/{ec_id}"), + ); + + let error = enforce_basic_auth(&settings, &req) + .expect_err("should fail closed without a matching admin handler"); + assert!( + error.to_string().contains("no configured handler"), + "should describe the missing admin handler" + ); + } + + #[test] + fn similar_non_admin_prefix_without_handler_remains_public() { + let config = crate_test_settings_str().replace( + r#"path = "^/_ts/admin""#, + r#"path = "^/_ts/admin/keys/rotate$""#, + ); + let settings: Settings = + toml::from_str(&config).expect("should deserialize settings without finalization"); + let req = build_request(Method::GET, "https://example.com/_ts/administrator"); + + assert!( + enforce_basic_auth(&settings, &req) + .expect("should evaluate auth") + .is_none(), + "should not classify a similar prefix as the admin namespace" + ); + } } diff --git a/crates/trusted-server-core/src/cookies.rs b/crates/trusted-server-core/src/cookies.rs index 2ddc7a8b4..a716045f9 100644 --- a/crates/trusted-server-core/src/cookies.rs +++ b/crates/trusted-server-core/src/cookies.rs @@ -66,6 +66,19 @@ pub fn handle_request_cookies( } } +/// Returns the named value from the request's selected `Cookie` header. +/// +/// Values are trimmed and may contain additional `=` characters. A missing or +/// non-UTF-8 selected header returns `None`. +#[must_use] +pub fn extract_cookie_value(req: &Request, name: &str) -> Option { + let cookie_header = req.headers().get(header::COOKIE)?.to_str().ok()?; + cookie_header.split(';').find_map(|pair| { + let (key, value) = pair.trim().split_once('=')?; + (key.trim() == name).then(|| value.trim().to_owned()) + }) +} + /// Strips named cookies from a `Cookie` header value string. /// /// Parses the semicolon-separated cookie pairs, filters out any whose name @@ -242,6 +255,37 @@ mod tests { ); } + #[test] + fn extract_cookie_value_returns_none_without_cookie_header() { + let req = build_request(None); + + assert_eq!(extract_cookie_value(&req, "session"), None); + } + + #[test] + fn extract_cookie_value_trims_pairs_and_preserves_embedded_equals() { + let req = build_request(Some("first=one; token = abc== ; last=three")); + + assert_eq!( + extract_cookie_value(&req, "token").as_deref(), + Some("abc==") + ); + assert_eq!(extract_cookie_value(&req, "last").as_deref(), Some("three")); + } + + #[test] + fn extract_cookie_value_returns_none_for_selected_non_utf8_header() { + let invalid = HeaderValue::from_bytes(b"\xff=value").expect("should build header value"); + let mut req = build_request(None); + req.headers_mut().append(header::COOKIE, invalid); + req.headers_mut().append( + header::COOKIE, + HeaderValue::from_static("session=from-later-header"), + ); + + assert_eq!(extract_cookie_value(&req, "session"), None); + } + // --------------------------------------------------------------- // forward_cookie_header tests // --------------------------------------------------------------- diff --git a/crates/trusted-server-core/src/ec/admin.rs b/crates/trusted-server-core/src/ec/admin.rs new file mode 100644 index 000000000..a94679781 --- /dev/null +++ b/crates/trusted-server-core/src/ec/admin.rs @@ -0,0 +1,1400 @@ +//! Admin endpoints for inspecting EC identity state. +//! +//! Serves `GET /_ts/admin/ec` (EC ID taken from the request's `ts-ec` +//! cookie) and `GET /_ts/admin/ec/{id}` (explicit EC ID). Returns the raw +//! stored [`KvEntry`] plus a derived view of the EIDs the auction would +//! attach, so operators can debug KV-to-auction propagation without KV +//! console access. +//! +//! Also serves `GET /_ts/admin/eids`, which echoes the request's `ts-eids` +//! and `sharedId` cookies with an ingestion preview — the client-side half +//! of EID propagation that is never stored server-side. +//! +//! Authentication is enforced by the `^/_ts/admin` basic-auth handler +//! configuration; startup validation rejects configs that leave these paths +//! uncovered (see `Settings::ADMIN_ENDPOINTS`). Because the endpoints are +//! auth-gated and operator-facing, responses intentionally include full +//! internal detail (raw consent strings, partner UIDs, parse errors). + +use std::borrow::Cow; +use std::collections::BTreeMap; + +use http::{HeaderValue, Method, Request, Response, StatusCode, header}; +use serde::Serialize; +use serde_json::Value as JsonValue; + +use edgezero_core::body::Body as EdgeBody; +use error_stack::{Report, ResultExt as _}; + +use crate::constants::{COOKIE_SHAREDID, COOKIE_TS_EC, COOKIE_TS_EIDS}; +use crate::cookies::extract_cookie_value; +use crate::error::TrustedServerError; +use crate::openrtb::Eid; + +use super::eids::{resolve_partner_ids, to_eids}; +use super::generation::is_valid_ec_id; +use super::kv::KvIdentityGraph; +use super::kv_backend::EcKvLookup; +use super::kv_types::{KvEntry, KvMetadata}; +use super::log_id; +use super::prebid_eids::{ + analyze_prebid_eids_cookie, collect_sharedid_update, dedupe_partner_updates, is_valid_eid_uid, +}; +use super::registry::PartnerRegistry; + +/// Route prefix shared by the cookie-based and explicit-ID lookup routes. +const ADMIN_EC_PATH: &str = "/_ts/admin/ec"; + +/// Route used by the request-only EID cookie diagnostic. +const ADMIN_EIDS_PATH: &str = "/_ts/admin/eids"; + +/// Reserved Trusted Server admin prefix. +/// +/// Mirrors the documented `^/_ts/admin` basic-auth handler regex, so every +/// path that handler authenticates is also reserved at the fallback boundary. +/// Matching on the bare prefix — rather than on `/_ts/admin` plus a literal +/// `/` — also covers percent-encoded separators such as `/_ts/admin%2Fec`, +/// which the auth handler matches but a literal-slash check does not. +const ADMIN_NAMESPACE_PREFIX: &str = "/_ts/admin"; + +/// Retired non-`/_ts` admin key alias prefix. +/// +/// The exact `/admin/keys/rotate` and `/admin/keys/deactivate` aliases are +/// routed to a local deny by each adapter; the rest of the retired namespace +/// (trailing, descendant, and encoded-separator forms) is reserved here. +const RETIRED_ADMIN_KEYS_PREFIX: &str = "/admin/keys"; + +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +enum AdminDiagnosticShape { + ValidResource, + Malformed, +} + +fn admin_diagnostic_shape(path: &str) -> Option { + if path == ADMIN_EC_PATH || path == ADMIN_EIDS_PATH { + return Some(AdminDiagnosticShape::ValidResource); + } + + if let Some(remainder) = path.strip_prefix("/_ts/admin/ec/") { + return Some(if !remainder.is_empty() && !remainder.contains('/') { + AdminDiagnosticShape::ValidResource + } else { + AdminDiagnosticShape::Malformed + }); + } + + // Reserve the complete admin namespace at the publisher-fallback boundary. + // A successfully authenticated malformed or future admin path must never + // forward its Authorization header or body to the publisher origin. + let reserved = is_reserved_admin_path(path) + || percent_decoded_path(path).is_some_and(|decoded| is_reserved_admin_path(&decoded)); + + reserved.then_some(AdminDiagnosticShape::Malformed) +} + +/// Returns whether `path` sits in a namespace that must never reach publisher +/// fallback, because doing so would forward Trusted Server admin credentials +/// and request bodies to the publisher origin. +fn is_reserved_admin_path(path: &str) -> bool { + path.starts_with(ADMIN_NAMESPACE_PREFIX) || path.starts_with(RETIRED_ADMIN_KEYS_PREFIX) +} + +/// Percent-decodes `path` once, returning `None` when the path contains no +/// escape sequence or decodes to invalid UTF-8. +/// +/// Routers and the basic-auth matcher both operate on the raw path, so an +/// encoded separator can shift a request out of the literal admin namespace +/// while still matching the admin auth handler. Checking the decoded form as +/// well keeps the reservation closed for `%2F`, `%2f`, and their +/// double-encoded variants. +fn percent_decoded_path(path: &str) -> Option { + if !path.contains('%') { + return None; + } + + urlencoding::decode(path).ok().map(Cow::into_owned) +} + +/// Returns a local denial response when an admin diagnostic request reaches +/// an adapter's publisher fallback. +/// +/// Valid diagnostic resources reject non-GET methods with `405 Method Not +/// Allowed`. Malformed, trailing, unknown, and any valid GET admin route that +/// unexpectedly reaches fallback return `404 Not Found`. The reservation +/// spans the whole `/_ts/admin` prefix — including percent-encoded separators +/// such as `/_ts/admin%2Fec` — plus the retired `/admin/keys` alias namespace, +/// evaluated on both the raw and the percent-decoded path. Paths outside those +/// namespaces return `None`, preserving normal fallback. +#[must_use] +pub fn deny_admin_diagnostic_fallback(req: &Request) -> Option> { + let shape = admin_diagnostic_shape(req.uri().path())?; + let mut response = + if shape == AdminDiagnosticShape::ValidResource && req.method() != Method::GET { + json_error(StatusCode::METHOD_NOT_ALLOWED, "method not allowed") + } else { + json_error(StatusCode::NOT_FOUND, "admin diagnostic route not found") + }; + + if response.status() == StatusCode::METHOD_NOT_ALLOWED { + response + .headers_mut() + .insert(header::ALLOW, HeaderValue::from_static("GET")); + } + + Some(response) +} + +/// Successful admin EC lookup payload. +#[derive(Debug, Serialize)] +struct AdminEcLookupResponse { + /// The EC ID that was looked up. + ec_id: String, + /// Platform KV store name the entry was read from. + store: String, + /// Store generation marker for the entry. + generation: u64, + /// `true` when the entry is a consent-withdrawal tombstone + /// (`consent.ok = false`). Absent when the body failed to parse as JSON or + /// deserialize as a [`KvEntry`]. + #[serde(skip_serializing_if = "Option::is_none")] + tombstone: Option, + /// The stored entry, preserved as raw JSON except for derived + /// `created_iso` / `updated_iso` companions added next to the stored + /// unix-seconds timestamps for readability. Absent when the body + /// was not valid JSON (see `entry_error` / `raw_body`). + #[serde(skip_serializing_if = "Option::is_none")] + entry: Option, + /// JSON parsing, schema deserialization, or validation failure detail. + #[serde(skip_serializing_if = "Option::is_none")] + entry_error: Option, + /// Raw entry body (lossy UTF-8) when it was not valid JSON. + #[serde(skip_serializing_if = "Option::is_none")] + raw_body: Option, + /// The stored KV metadata JSON, when present and parseable. + #[serde(skip_serializing_if = "Option::is_none")] + metadata: Option, + /// JSON parsing or schema deserialization failure detail for metadata. + #[serde(skip_serializing_if = "Option::is_none")] + metadata_error: Option, + /// Derived auction view. Present only when the entry deserializes and + /// validates — the same precondition the auction read path applies, so + /// its absence means the auction would attach no KV-derived EIDs. + /// Live requests additionally gate on per-request consent, which is not + /// reproducible here. + #[serde(skip_serializing_if = "Option::is_none")] + auction: Option, +} + +/// What the auction EID decoration would produce for this entry. +#[derive(Debug, Serialize)] +struct AuctionEidsView { + /// EIDs the auction would attach to `user.eids`, exactly as produced by + /// the auction resolution path. + eids: Vec, + /// Stored partner IDs that the auction resolution filters out, with the + /// reason each was skipped. + skipped: Vec, +} + +/// A stored partner ID excluded from auction EIDs. +#[derive(Debug, Serialize)] +struct SkippedPartnerId { + /// Partner namespace key in the entry's `ids` map. + source_domain: String, + /// Why the auction resolution skips it: `empty_uid`, `not_in_registry`, + /// or `bidstream_disabled`. + reason: &'static str, +} + +/// Handles `GET /_ts/admin/ec` and `GET /_ts/admin/ec/{id}`. +/// +/// Resolves the EC ID from the path when present, falling back to the +/// request's `ts-ec` cookie for the bare route. Responds: +/// +/// - `200 OK` with an [`AdminEcLookupResponse`] JSON body when the key +/// exists (including corrupt entries, which are reported with +/// `entry_error` and `raw_body` instead of failing closed); +/// - `400 Bad Request` when the resolved ID is not a valid EC ID; +/// - `404 Not Found` when the key does not exist, or the bare route was +/// called without a `ts-ec` cookie; +/// - `501 Not Implemented` when no EC identity graph is configured. +/// +/// # Errors +/// +/// Returns [`TrustedServerError::KvStore`] when the store open or read +/// fails. +pub fn handle_admin_ec_lookup( + kv: Option<&KvIdentityGraph>, + registry: &PartnerRegistry, + req: &Request, +) -> Result, Report> { + let Some(kv) = kv else { + return Ok(admin_ec_lookup_not_supported()); + }; + + let ec_id = match requested_ec_id(req) { + Ok(ec_id) => ec_id, + Err(response) => return Ok(*response), + }; + + let Some(lookup) = kv.lookup_raw(&ec_id)? else { + log::info!("Admin EC lookup: no entry for '{}'", log_id(&ec_id)); + return Ok(json_error( + StatusCode::NOT_FOUND, + "EC entry not found (KV reads are eventually consistent; a very \ + recent entry may not be visible yet)", + )); + }; + + log::info!("Admin EC lookup: returning entry for '{}'", log_id(&ec_id)); + let payload = build_lookup_response(registry, kv.store_name(), ec_id, &lookup); + let body = + serde_json::to_string(&payload).change_context(TrustedServerError::Configuration { + message: "failed to serialize admin EC lookup response".to_owned(), + })?; + Ok(json_response(StatusCode::OK, body)) +} + +/// Returns the portable response used when an adapter has no EC KV backend. +#[must_use] +pub fn admin_ec_lookup_not_supported() -> Response { + json_error( + StatusCode::NOT_IMPLEMENTED, + "EC identity graph is not configured on this deployment", + ) +} + +/// Resolves the EC ID to look up from the path or the `ts-ec` cookie. +/// +/// Returns the (boxed) error response to send directly when no valid ID is +/// available. +fn requested_ec_id(req: &Request) -> Result>> { + let remainder = req + .uri() + .path() + .strip_prefix(ADMIN_EC_PATH) + .unwrap_or("") + .trim_matches('/'); + + let ec_id = if remainder.is_empty() { + match extract_cookie_value(req, COOKIE_TS_EC) { + Some(cookie_ec_id) => cookie_ec_id, + None => { + return Err(Box::new(json_error( + StatusCode::NOT_FOUND, + "no EC ID in path and no ts-ec cookie on the request — pass \ + an explicit id: /_ts/admin/ec/{id}", + ))); + } + } + } else { + remainder.to_owned() + }; + + if !is_valid_ec_id(&ec_id) { + return Err(Box::new(json_error( + StatusCode::BAD_REQUEST, + "invalid EC ID format (expected {64hex}.{6alnum})", + ))); + } + + Ok(ec_id) +} + +/// Builds the success payload from a raw KV lookup. +/// +/// Parse failures are reported in the payload rather than propagated, so +/// corrupt entries remain inspectable. +fn build_lookup_response( + registry: &PartnerRegistry, + store_name: &str, + ec_id: String, + lookup: &EcKvLookup, +) -> AdminEcLookupResponse { + let mut payload = AdminEcLookupResponse { + ec_id, + store: store_name.to_owned(), + generation: lookup.generation, + tombstone: None, + entry: None, + entry_error: None, + raw_body: None, + metadata: None, + metadata_error: None, + auction: None, + }; + + match serde_json::from_slice::(&lookup.body) { + Ok(mut entry_json) => { + add_iso_timestamp_companions(&mut entry_json); + payload.entry = Some(entry_json); + + match serde_json::from_slice::(&lookup.body) { + Ok(entry) => { + payload.tombstone = Some(!entry.consent.ok); + match entry.validate() { + Ok(()) => payload.auction = Some(build_auction_view(registry, &entry)), + Err(message) => { + payload.entry_error = Some(format!( + "entry failed validation (auction reads fail closed \ + and attach no EIDs): {message}" + )); + } + } + } + Err(error) => { + payload.entry_error = + Some(format!("failed to deserialize entry schema: {error}")); + } + } + } + Err(error) => { + payload.entry_error = Some(format!("failed to parse entry JSON: {error}")); + payload.raw_body = Some(String::from_utf8_lossy(&lookup.body).into_owned()); + } + } + + match &lookup.metadata { + None => {} + Some(bytes) => match serde_json::from_slice::(bytes) { + Ok(metadata_json) => { + payload.metadata = Some(metadata_json); + if let Err(error) = serde_json::from_slice::(bytes) { + payload.metadata_error = + Some(format!("failed to deserialize metadata schema: {error}")); + } + } + Err(error) => { + payload.metadata_error = Some(format!( + "failed to parse metadata JSON: {error} (raw: {})", + String::from_utf8_lossy(bytes) + )); + } + }, + } + + payload +} + +/// Adds derived ISO 8601 companions next to the +/// stored unix-seconds timestamps (`created_iso`, `consent.updated_iso`). +/// +/// Every stored value, including pre-existing ISO companions, stays untouched. +/// The derived fields exist purely for operator readability when absent. +fn add_iso_timestamp_companions(entry_json: &mut JsonValue) { + let created = entry_json.get("created").and_then(JsonValue::as_u64); + let updated = entry_json + .get("consent") + .and_then(|consent| consent.get("updated")) + .and_then(JsonValue::as_u64); + if let Some(object) = entry_json.as_object_mut() { + if let Some(iso) = created.and_then(iso_timestamp) { + object + .entry("created_iso".to_owned()) + .or_insert(JsonValue::String(iso)); + } + if let Some(consent) = object.get_mut("consent").and_then(JsonValue::as_object_mut) + && let Some(iso) = updated.and_then(iso_timestamp) + { + consent + .entry("updated_iso".to_owned()) + .or_insert(JsonValue::String(iso)); + } + } +} + +/// Formats a unix-seconds timestamp as ISO 8601 (`yyyy-MM-ddTHH:mm:ss.SSSZ`). +/// +/// Returns `None` for values outside the representable date range. +fn iso_timestamp(unix_seconds: u64) -> Option { + let unix_seconds = i64::try_from(unix_seconds).ok()?; + chrono::DateTime::from_timestamp(unix_seconds, 0) + .map(|datetime| datetime.format("%Y-%m-%dT%H:%M:%S%.3fZ").to_string()) +} + +/// Derives the auction EID view for a valid entry, mirroring the filters in +/// [`resolve_partner_ids`] and reporting why each stored ID was skipped. +fn build_auction_view(registry: &PartnerRegistry, entry: &KvEntry) -> AuctionEidsView { + let resolved = resolve_partner_ids(registry, entry); + let eids = to_eids(&resolved); + + let mut skipped = Vec::new(); + for (source_domain, partner_uid) in &entry.ids { + let reason = if partner_uid.uid.is_empty() { + "empty_uid" + } else { + match registry.get(source_domain) { + None => "not_in_registry", + Some(partner) if !partner.bidstream_enabled => "bidstream_disabled", + Some(_) => continue, + } + }; + skipped.push(SkippedPartnerId { + source_domain: source_domain.clone(), + reason, + }); + } + + AuctionEidsView { eids, skipped } +} + +/// Admin EIDs echo payload. +#[derive(Debug, Serialize)] +struct AdminEidsResponse { + /// Whether a `ts-eids` cookie was present on the request. + cookie_present: bool, + /// EIDs parsed from the `ts-eids` cookie. Absent when the cookie is + /// missing or failed to parse. + #[serde(skip_serializing_if = "Option::is_none")] + eids: Option>, + /// Parse failure detail when the `ts-eids` cookie could not be decoded. + #[serde(skip_serializing_if = "Option::is_none")] + parse_error: Option, + /// Whether a `sharedId` cookie was present on the request. + sharedid_present: bool, + /// Number of partners configured in the registry. + partners_configured: usize, + /// Preview of what cookie ingestion would write into the EC entry's + /// `ids` map on a navigation carrying these cookies. + ingest: IngestPreview, +} + +/// What cookie ingestion would store, and what it would drop. +#[derive(Debug, Serialize)] +struct IngestPreview { + /// Cookie sources matched to a configured partner, with the UID that + /// would be stored (deduplicated exactly like the ingestion path). + matched: Vec, + /// `ts-eids` sources dropped on ingestion, with the reason. + unmatched: Vec, +} + +/// A cookie-derived partner UID that ingestion would store. +#[derive(Debug, Serialize)] +struct MatchedPartnerId { + /// Partner namespace key in the EC entry's `ids` map. + source_domain: String, + /// The UID that would be stored. + uid: String, +} + +#[derive(Debug, Serialize)] +struct DroppedEidSource { + /// EID source from the cookie. + source: String, + /// Why ingestion would drop the source. + reason: DroppedEidReason, +} + +#[derive(Debug, Serialize)] +#[serde(rename_all = "snake_case")] +enum DroppedEidReason { + NoPartner, + NoValidUid, +} + +/// Handles `GET /_ts/admin/eids`. +/// +/// Echoes the request's `ts-eids` and `sharedId` cookies: the parsed EID +/// list plus a preview of what cookie ingestion would write into the EC +/// entry's `ids` map given the configured partner registry. Pure request +/// inspection — no KV access — so it works on every adapter. +/// +/// Always responds `200 OK`; missing or malformed cookies are reported in +/// the payload instead of as errors. +/// +/// # Errors +/// +/// Returns [`TrustedServerError::Configuration`] only when the response +/// payload fails JSON serialization. +pub fn handle_admin_eids_lookup( + registry: &PartnerRegistry, + req: &Request, +) -> Result, Report> { + let eids_cookie = extract_cookie_value(req, COOKIE_TS_EIDS); + let sharedid_cookie = extract_cookie_value(req, COOKIE_SHAREDID); + + let (eids, parse_error, diagnostic_sources, mut updates) = match &eids_cookie { + None => (None, None, Vec::new(), Vec::new()), + Some(value) => match analyze_prebid_eids_cookie(value, registry) { + Ok(analysis) => ( + Some(analysis.eids), + None, + analysis.diagnostic_sources, + analysis.updates, + ), + Err(error) => ( + None, + Some(format!("failed to parse ts-eids cookie: {error}")), + Vec::new(), + Vec::new(), + ), + }, + }; + + // Mirror the ingestion path (`ingest_eid_cookies`): collect matches from + // both cookies, then dedupe the same way so the preview reports exactly + // what a navigation would store. + if let Some(value) = &sharedid_cookie + && let Some(update) = collect_sharedid_update(value, registry) + { + updates.push(update); + } + let matched = dedupe_partner_updates(updates) + .into_iter() + .map(|update| MatchedPartnerId { + source_domain: update.partner_id, + uid: update.uid, + }) + .collect(); + + let mut source_has_valid_uid = BTreeMap::new(); + for diagnostic_source in diagnostic_sources { + let has_valid_uid = diagnostic_source + .uids + .iter() + .any(|uid| is_valid_eid_uid(uid)); + source_has_valid_uid + .entry(diagnostic_source.source) + .and_modify(|source_has_valid_uid| *source_has_valid_uid |= has_valid_uid) + .or_insert(has_valid_uid); + } + let unmatched = source_has_valid_uid + .into_iter() + .filter_map(|(source, has_valid_uid)| { + let reason = if registry.find_by_source_domain(&source).is_none() { + DroppedEidReason::NoPartner + } else if !has_valid_uid { + DroppedEidReason::NoValidUid + } else { + return None; + }; + Some(DroppedEidSource { source, reason }) + }) + .collect(); + + let payload = AdminEidsResponse { + cookie_present: eids_cookie.is_some(), + eids, + parse_error, + sharedid_present: sharedid_cookie.is_some(), + partners_configured: registry.len(), + ingest: IngestPreview { matched, unmatched }, + }; + + let body = + serde_json::to_string(&payload).change_context(TrustedServerError::Configuration { + message: "failed to serialize admin EIDs response".to_owned(), + })?; + Ok(json_response(StatusCode::OK, body)) +} + +fn json_error(status: StatusCode, message: &str) -> Response { + let body = serde_json::json!({ "error": message }); + json_response(status, body.to_string()) +} + +fn json_response(status: StatusCode, body: String) -> Response { + Response::builder() + .status(status) + .header(header::CONTENT_TYPE, mime::APPLICATION_JSON.as_ref()) + .header(header::CACHE_CONTROL, "no-store") + .header(header::X_CONTENT_TYPE_OPTIONS, "nosniff") + .body(EdgeBody::from(body.into_bytes())) + .expect("should build admin EC lookup response") +} + +#[cfg(test)] +mod tests { + use std::time::Duration; + + use base64::{Engine as _, engine::general_purpose::STANDARD as BASE64}; + + use super::*; + use crate::ec::kv_backend::test_support::InMemoryEcKv; + use crate::ec::kv_backend::{EcKvStore as _, EcKvWrite, EcKvWriteMode}; + use crate::ec::kv_types::KvPartnerId; + use crate::redacted::Redacted; + use crate::settings::EcPartner; + + fn test_ec_id() -> String { + format!("{}.abc123", "a".repeat(64)) + } + + fn make_test_partner(source_domain: &str, bidstream_enabled: bool) -> EcPartner { + EcPartner { + name: format!("Partner {source_domain}"), + source_domain: source_domain.to_owned(), + openrtb_atype: EcPartner::default_openrtb_atype(), + bidstream_enabled, + api_token: Redacted::new(format!("test-token-{source_domain:-<32}")), + batch_rate_limit: EcPartner::default_batch_rate_limit(), + pull_sync_enabled: false, + pull_sync_url: None, + pull_sync_allowed_domains: vec![], + pull_sync_ttl_sec: EcPartner::default_pull_sync_ttl_sec(), + pull_sync_rate_limit: EcPartner::default_pull_sync_rate_limit(), + ts_pull_token: None, + } + } + + fn test_registry() -> PartnerRegistry { + PartnerRegistry::from_config(&[ + make_test_partner("bidstream.example", true), + make_test_partner("disabled.example", false), + ]) + .expect("should build test partner registry") + } + + fn get_request(path: &str) -> Request { + Request::builder() + .method("GET") + .uri(format!("https://edge.example.com{path}")) + .body(EdgeBody::empty()) + .expect("should build test request") + } + + fn get_request_with_cookie(path: &str, cookie: &str) -> Request { + Request::builder() + .method("GET") + .uri(format!("https://edge.example.com{path}")) + .header(header::COOKIE, cookie) + .body(EdgeBody::empty()) + .expect("should build test request") + } + + fn request_with_method(method: http::Method, path: &str) -> Request { + Request::builder() + .method(method) + .uri(format!("https://edge.example.com{path}")) + .body(EdgeBody::empty()) + .expect("should build test request") + } + + fn kv_with_entry(ec_id: &str, entry: &KvEntry) -> KvIdentityGraph { + let kv = KvIdentityGraph::in_memory("test-store"); + kv.create(ec_id, entry).expect("should seed KV entry"); + kv + } + + fn kv_with_raw_body(ec_id: &str, body: &str) -> KvIdentityGraph { + let metadata = serde_json::json!({ "ok": true, "country": "US", "v": 1 }).to_string(); + kv_with_raw_body_and_metadata(ec_id, body, &metadata) + } + + fn kv_with_raw_body_and_metadata(ec_id: &str, body: &str, metadata: &str) -> KvIdentityGraph { + let store = InMemoryEcKv::new("test-store"); + store + .insert( + ec_id, + EcKvWrite { + body, + metadata, + ttl: Duration::from_secs(60), + mode: EcKvWriteMode::Add, + }, + ) + .expect("should seed raw KV body"); + KvIdentityGraph::new(store) + } + + fn response_json(response: Response) -> JsonValue { + serde_json::from_slice(&response.into_body().into_bytes().unwrap_or_default()) + .expect("should parse response body as JSON") + } + + fn sample_entry() -> KvEntry { + let mut entry = KvEntry::minimal("bidstream.example", "uid-live", 1_741_824_000); + entry.ids.insert( + "disabled.example".to_owned(), + KvPartnerId { + uid: "uid-disabled".to_owned(), + }, + ); + entry.ids.insert( + "unknown.example".to_owned(), + KvPartnerId { + uid: "uid-unknown".to_owned(), + }, + ); + entry + } + + #[test] + fn admin_diagnostic_fallback_rejects_wrong_methods_locally() { + let ec_id = test_ec_id(); + let paths = [ + "/_ts/admin/ec".to_owned(), + format!("/_ts/admin/ec/{ec_id}"), + "/_ts/admin/eids".to_owned(), + ]; + let methods = [ + http::Method::POST, + http::Method::HEAD, + http::Method::OPTIONS, + http::Method::PUT, + http::Method::PATCH, + http::Method::DELETE, + ]; + + for path in paths { + for method in &methods { + let request = request_with_method(method.clone(), &path); + let response = deny_admin_diagnostic_fallback(&request) + .unwrap_or_else(|| panic!("should deny {method} {path} locally")); + + assert_eq!(response.status(), StatusCode::METHOD_NOT_ALLOWED); + assert_eq!( + response.headers().get(header::ALLOW), + Some(&http::HeaderValue::from_static("GET")), + "should advertise GET for {path}" + ); + assert_eq!( + response.headers().get(header::CACHE_CONTROL), + Some(&http::HeaderValue::from_static("no-store")), + "should prevent caching for {path}" + ); + } + } + } + + #[test] + fn admin_diagnostic_fallback_rejects_malformed_paths_locally() { + let ec_id = test_ec_id(); + let paths = [ + "/_ts/admin/ec/".to_owned(), + format!("/_ts/admin/ec/{ec_id}/extra"), + "/_ts/admin/eids/".to_owned(), + "/_ts/admin/eids/extra".to_owned(), + "/_ts/admin/eids.json".to_owned(), + "/_ts/admin/ec;foo".to_owned(), + format!("/_ts/admin/ec%2F{ec_id}"), + "/_ts/admin/unknown".to_owned(), + ]; + + for path in paths { + for method in [http::Method::GET, http::Method::POST] { + let request = request_with_method(method.clone(), &path); + let response = deny_admin_diagnostic_fallback(&request) + .unwrap_or_else(|| panic!("should deny {method} {path} locally")); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + assert_eq!( + response.headers().get(header::CACHE_CONTROL), + Some(&http::HeaderValue::from_static("no-store")), + "should prevent caching for {path}" + ); + } + } + } + + #[test] + fn admin_diagnostic_fallback_reserves_encoded_admin_separators() { + // `/_ts/admin%2Fec` matches the documented `^/_ts/admin` basic-auth + // handler, so it is authenticated, but a literal-slash namespace check + // misses it. Reaching publisher fallback would forward the caller's + // `Authorization` header and body to the origin. + let paths = [ + "/_ts/admin%2Fec", + "/_ts/admin%2fec", + "/_ts/admin%2Fkeys/rotate", + "/_ts/admin%252Fec", + "/_ts/admin%5Cec", + "/_ts/adminec", + "/%5Fts/admin/ec", + ]; + + for path in paths { + for method in [http::Method::GET, http::Method::POST] { + let request = request_with_method(method.clone(), path); + let response = deny_admin_diagnostic_fallback(&request) + .unwrap_or_else(|| panic!("should deny {method} {path} locally")); + + assert_eq!( + response.status(), + StatusCode::NOT_FOUND, + "should deny {path} before publisher fallback" + ); + } + } + } + + #[test] + fn admin_diagnostic_fallback_reserves_retired_admin_keys_namespace() { + // The retired non-`/_ts` aliases are not covered by the `^/_ts/admin` + // basic-auth handler. Only the two exact paths are routed to a local + // deny, so trailing, descendant, and encoded-separator forms must be + // denied at the shared fallback boundary instead. + let paths = [ + "/admin/keys", + "/admin/keys/", + "/admin/keys/rotate/", + "/admin/keys/rotate/extra", + "/admin/keys%2Frotate", + "/admin/keys%2frotate", + "/admin%2Fkeys/rotate", + "/admin%2fkeys%2Frotate", + ]; + + for path in paths { + for method in [http::Method::GET, http::Method::POST] { + let request = request_with_method(method.clone(), path); + let response = deny_admin_diagnostic_fallback(&request) + .unwrap_or_else(|| panic!("should deny {method} {path} locally")); + + assert_eq!( + response.status(), + StatusCode::NOT_FOUND, + "should deny {path} before publisher fallback" + ); + } + } + } + + #[test] + fn admin_diagnostic_fallback_ignores_unrelated_publisher_paths() { + for path in [ + "/articles/example", + "/admin", + "/admin/login", + "/admin/keyboards", + "/_ts/api/v1/batch-sync", + ] { + let request = request_with_method(http::Method::POST, path); + + assert!( + deny_admin_diagnostic_fallback(&request).is_none(), + "should leave unrelated publisher fallback unchanged for {path}" + ); + } + } + + #[test] + fn returns_entry_with_auction_view() { + let ec_id = test_ec_id(); + let kv = kv_with_entry(&ec_id, &sample_entry()); + let req = get_request(&format!("/_ts/admin/ec/{ec_id}")); + + let response = handle_admin_ec_lookup(Some(&kv), &test_registry(), &req) + .expect("should handle lookup"); + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response + .headers() + .get(header::CACHE_CONTROL) + .and_then(|v| v.to_str().ok()), + Some("no-store"), + "should send no-store on admin responses" + ); + + let json = response_json(response); + assert_eq!(json["ec_id"], ec_id.as_str()); + assert_eq!(json["store"], "test-store"); + assert_eq!(json["tombstone"], false); + assert_eq!( + json["entry"]["ids"]["bidstream.example"]["uid"], "uid-live", + "should echo the stored entry verbatim" + ); + assert_eq!( + json["entry"]["created"], 1_741_824_000_u64, + "should keep the stored unix-seconds timestamp" + ); + assert_eq!( + json["entry"]["created_iso"], "2025-03-13T00:00:00.000Z", + "should add an ISO 8601 companion for created" + ); + assert_eq!( + json["entry"]["consent"]["updated_iso"], "2025-03-13T00:00:00.000Z", + "should add an ISO 8601 companion for consent.updated" + ); + + let eids = json["auction"]["eids"] + .as_array() + .expect("should have auction eids"); + assert_eq!(eids.len(), 1, "should resolve only the bidstream partner"); + assert_eq!(eids[0]["source"], "bidstream.example"); + assert_eq!(eids[0]["uids"][0]["id"], "uid-live"); + + let skipped = json["auction"]["skipped"] + .as_array() + .expect("should have skipped list"); + assert_eq!(skipped.len(), 2, "should report both filtered partners"); + assert!( + skipped + .iter() + .any(|s| s["source_domain"] == "disabled.example" + && s["reason"] == "bidstream_disabled"), + "should report the bidstream-disabled partner" + ); + assert!( + skipped.iter().any( + |s| s["source_domain"] == "unknown.example" && s["reason"] == "not_in_registry" + ), + "should report the unregistered partner" + ); + } + + #[test] + fn preserves_raw_entry_and_metadata_shapes() { + let ec_id = test_ec_id(); + let body = serde_json::json!({ + "v": 1, + "created": 1_741_824_000_u64, + "created_iso": "stored-created-iso", + "future_top_level": { "enabled": true }, + "consent": { + "ok": true, + "updated": 1_741_824_000_u64, + "updated_iso": "stored-updated-iso", + "future_consent": "preserve-me" + }, + "geo": { "country": "US" }, + "pub_properties": { + "origin_domain": "example.com", + "seen_domains": { + "example.com": { "first": 1000, "last": 1200, "visits": 3 } + } + }, + "ids": { + "bidstream.example": { "uid": "uid-live", "synced": 1100 } + } + }) + .to_string(); + let metadata = serde_json::json!({ + "ok": true, + "country": "US", + "v": 1, + "future_metadata": { "source": "edge" } + }) + .to_string(); + let kv = kv_with_raw_body_and_metadata(&ec_id, &body, &metadata); + let req = get_request(&format!("/_ts/admin/ec/{ec_id}")); + + let response = handle_admin_ec_lookup(Some(&kv), &test_registry(), &req) + .expect("should handle lookup"); + let json = response_json(response); + + assert_eq!(json["entry"]["future_top_level"]["enabled"], true); + assert_eq!(json["entry"]["consent"]["future_consent"], "preserve-me"); + assert_eq!(json["entry"]["ids"]["bidstream.example"]["synced"], 1100); + assert!( + json["entry"]["pub_properties"]["seen_domains"].is_object(), + "legacy map-shaped seen_domains should remain unchanged" + ); + assert_eq!(json["entry"]["created_iso"], "stored-created-iso"); + assert_eq!( + json["entry"]["consent"]["updated_iso"], + "stored-updated-iso" + ); + assert_eq!(json["metadata"]["future_metadata"]["source"], "edge"); + assert_eq!(json["auction"]["eids"][0]["source"], "bidstream.example"); + } + + #[test] + fn valid_json_with_invalid_entry_schema_remains_visible() { + let ec_id = test_ec_id(); + let body = serde_json::json!({ "future": "value" }).to_string(); + let kv = kv_with_raw_body(&ec_id, &body); + let req = get_request(&format!("/_ts/admin/ec/{ec_id}")); + + let response = handle_admin_ec_lookup(Some(&kv), &test_registry(), &req) + .expect("should handle lookup"); + let json = response_json(response); + + assert_eq!(json["entry"]["future"], "value"); + assert!( + json["entry_error"] + .as_str() + .expect("should have entry_error") + .contains("failed to deserialize entry schema") + ); + assert!(json.get("raw_body").is_none()); + assert!(json.get("auction").is_none()); + } + + #[test] + fn reports_tombstone_entries() { + let ec_id = test_ec_id(); + let kv = KvIdentityGraph::in_memory("test-store"); + kv.write_withdrawal_tombstone(&ec_id) + .expect("should write tombstone"); + let req = get_request(&format!("/_ts/admin/ec/{ec_id}")); + + let response = handle_admin_ec_lookup(Some(&kv), &test_registry(), &req) + .expect("should handle lookup"); + + assert_eq!(response.status(), StatusCode::OK); + let json = response_json(response); + assert_eq!(json["tombstone"], true, "should flag tombstone entries"); + assert!( + json["auction"]["eids"] + .as_array() + .expect("should have auction eids") + .is_empty(), + "tombstone should resolve no EIDs" + ); + } + + #[test] + fn missing_entry_returns_404() { + let kv = KvIdentityGraph::in_memory("test-store"); + let req = get_request(&format!("/_ts/admin/ec/{}", test_ec_id())); + + let response = handle_admin_ec_lookup(Some(&kv), &test_registry(), &req) + .expect("should handle lookup"); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } + + #[test] + fn invalid_id_returns_400() { + let kv = KvIdentityGraph::in_memory("test-store"); + let req = get_request("/_ts/admin/ec/not-a-valid-id"); + + let response = handle_admin_ec_lookup(Some(&kv), &test_registry(), &req) + .expect("should handle lookup"); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + } + + #[test] + fn corrupt_entry_returns_parse_error_and_raw_body() { + let ec_id = test_ec_id(); + let kv = kv_with_raw_body(&ec_id, "not json at all"); + let req = get_request(&format!("/_ts/admin/ec/{ec_id}")); + + let response = handle_admin_ec_lookup(Some(&kv), &test_registry(), &req) + .expect("should handle lookup"); + + assert_eq!( + response.status(), + StatusCode::OK, + "corrupt entries should be inspectable, not opaque errors" + ); + let json = response_json(response); + assert!( + json["entry_error"] + .as_str() + .expect("should have entry_error") + .contains("failed to parse entry JSON"), + "should describe the parse failure" + ); + assert_eq!(json["raw_body"], "not json at all"); + assert!(json.get("entry").is_none(), "should omit unparsed entry"); + assert!( + json.get("auction").is_none(), + "should omit auction view for unparseable entries" + ); + assert_eq!( + json["metadata"]["country"], "US", + "should still parse the stored metadata" + ); + } + + #[test] + fn invalid_schema_version_reports_validation_error() { + let ec_id = test_ec_id(); + let body = serde_json::json!({ + "v": 99, + "created": 1000, + "consent": { "ok": true, "updated": 1000 }, + "geo": { "country": "US" } + }) + .to_string(); + let kv = kv_with_raw_body(&ec_id, &body); + let req = get_request(&format!("/_ts/admin/ec/{ec_id}")); + + let response = handle_admin_ec_lookup(Some(&kv), &test_registry(), &req) + .expect("should handle lookup"); + + assert_eq!(response.status(), StatusCode::OK); + let json = response_json(response); + assert!( + json["entry_error"] + .as_str() + .expect("should have entry_error") + .contains("failed validation"), + "should describe the validation failure" + ); + assert_eq!(json["entry"]["v"], 99, "should still show the parsed entry"); + assert!( + json.get("auction").is_none(), + "should omit auction view when the auction read would fail closed" + ); + } + + #[test] + fn bare_route_uses_ts_ec_cookie() { + let ec_id = test_ec_id(); + let kv = kv_with_entry(&ec_id, &sample_entry()); + let req = get_request_with_cookie("/_ts/admin/ec", &format!("other=1; ts-ec={ec_id}; x=2")); + + let response = handle_admin_ec_lookup(Some(&kv), &test_registry(), &req) + .expect("should handle lookup"); + + assert_eq!(response.status(), StatusCode::OK); + let json = response_json(response); + assert_eq!( + json["ec_id"], + ec_id.as_str(), + "should resolve the EC ID from the ts-ec cookie" + ); + } + + #[test] + fn bare_route_without_cookie_returns_404() { + let kv = KvIdentityGraph::in_memory("test-store"); + let req = get_request("/_ts/admin/ec"); + + let response = handle_admin_ec_lookup(Some(&kv), &test_registry(), &req) + .expect("should handle lookup"); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + let json = response_json(response); + assert!( + json["error"] + .as_str() + .expect("should have error message") + .contains("ts-ec cookie"), + "should explain the missing cookie" + ); + } + + #[test] + fn missing_identity_graph_returns_501() { + let req = get_request(&format!("/_ts/admin/ec/{}", test_ec_id())); + + let response = + handle_admin_ec_lookup(None, &test_registry(), &req).expect("should handle lookup"); + + assert_eq!(response.status(), StatusCode::NOT_IMPLEMENTED); + } + + #[test] + fn unsupported_ec_lookup_response_is_json_and_no_store() { + let response = admin_ec_lookup_not_supported(); + + assert_eq!(response.status(), StatusCode::NOT_IMPLEMENTED); + assert_eq!( + response.headers().get(header::CONTENT_TYPE), + Some(&HeaderValue::from_static("application/json")) + ); + assert_eq!( + response.headers().get(header::CACHE_CONTROL), + Some(&HeaderValue::from_static("no-store")) + ); + assert_eq!( + response.headers().get(header::X_CONTENT_TYPE_OPTIONS), + Some(&HeaderValue::from_static("nosniff")) + ); + assert!( + response_json(response)["error"] + .as_str() + .is_some_and(|message| message.contains("not configured")) + ); + } + + #[test] + fn kv_read_failure_propagates() { + let kv = KvIdentityGraph::failing("broken-store"); + let req = get_request(&format!("/_ts/admin/ec/{}", test_ec_id())); + + let result = handle_admin_ec_lookup(Some(&kv), &test_registry(), &req); + + assert!(result.is_err(), "should propagate KV read failures"); + } + + fn eids_cookie_for(entries: &serde_json::Value) -> String { + BASE64.encode(entries.to_string()) + } + + #[test] + fn eids_lookup_without_cookies_returns_empty_payload() { + let req = get_request("/_ts/admin/eids"); + + let response = + handle_admin_eids_lookup(&test_registry(), &req).expect("should handle eids lookup"); + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response.headers().get(header::X_CONTENT_TYPE_OPTIONS), + Some(&HeaderValue::from_static("nosniff")) + ); + let json = response_json(response); + assert_eq!(json["cookie_present"], false); + assert_eq!(json["sharedid_present"], false); + assert_eq!(json["partners_configured"], 2); + assert!( + json["ingest"]["matched"] + .as_array() + .expect("should have matched list") + .is_empty(), + "should preview no matches without cookies" + ); + } + + #[test] + fn eids_lookup_parses_cookie_and_previews_ingestion() { + let cookie = eids_cookie_for(&serde_json::json!([ + { + "source": "bidstream.example", + "uids": [{ "id": "uid-configured", "atype": 1 }] + }, + { + "source": "unknown.example", + "uids": [{ "id": "uid-unknown", "atype": 1 }] + } + ])); + let req = get_request_with_cookie("/_ts/admin/eids", &format!("ts-eids={cookie}")); + + let response = + handle_admin_eids_lookup(&test_registry(), &req).expect("should handle eids lookup"); + + assert_eq!(response.status(), StatusCode::OK); + let json = response_json(response); + assert_eq!(json["cookie_present"], true); + assert_eq!( + json["eids"] + .as_array() + .expect("should have parsed eids") + .len(), + 2, + "should echo both parsed EID sources" + ); + + let matched = json["ingest"]["matched"] + .as_array() + .expect("should have matched list"); + assert_eq!(matched.len(), 1, "should match only the configured partner"); + assert_eq!(matched[0]["source_domain"], "bidstream.example"); + assert_eq!(matched[0]["uid"], "uid-configured"); + + let unmatched = json["ingest"]["unmatched"] + .as_array() + .expect("should have unmatched list"); + assert_eq!(unmatched.len(), 1, "should report the unregistered source"); + assert_eq!(unmatched[0]["source"], "unknown.example"); + assert_eq!(unmatched[0]["reason"], "no_partner"); + } + + #[test] + fn eids_lookup_reports_configured_source_without_valid_uid() { + let oversized_uid = "x".repeat(513); + let cookie = eids_cookie_for(&serde_json::json!([{ + "source": "bidstream.example", + "uids": [ + { "id": "", "atype": 1 }, + { "id": " ", "atype": 1 }, + { "id": oversized_uid, "atype": 1 } + ] + }])); + let req = get_request_with_cookie("/_ts/admin/eids", &format!("ts-eids={cookie}")); + + let response = + handle_admin_eids_lookup(&test_registry(), &req).expect("should handle eids lookup"); + let json = response_json(response); + + assert!( + json["ingest"]["matched"] + .as_array() + .expect("should have matched list") + .is_empty(), + "invalid UIDs should not be matched" + ); + let unmatched = json["ingest"]["unmatched"] + .as_array() + .expect("should have unmatched list"); + assert_eq!(unmatched.len(), 1, "should report one dropped source"); + assert_eq!(unmatched[0]["source"], "bidstream.example"); + assert_eq!(unmatched[0]["reason"], "no_valid_uid"); + } + + #[test] + fn eids_lookup_does_not_drop_duplicate_source_with_valid_uid() { + let cookie = eids_cookie_for(&serde_json::json!([ + { + "source": "bidstream.example", + "uids": [{ "id": " ", "atype": 1 }] + }, + { + "source": "bidstream.example", + "uids": [{ "id": "uid-valid", "atype": 1 }] + } + ])); + let req = get_request_with_cookie("/_ts/admin/eids", &format!("ts-eids={cookie}")); + + let response = + handle_admin_eids_lookup(&test_registry(), &req).expect("should handle eids lookup"); + let json = response_json(response); + + let matched = json["ingest"]["matched"] + .as_array() + .expect("should have matched list"); + assert_eq!(matched.len(), 1, "should match the valid duplicate source"); + assert_eq!(matched[0]["uid"], "uid-valid"); + assert!( + json["ingest"]["unmatched"] + .as_array() + .expect("should have unmatched list") + .is_empty(), + "a valid duplicate should suppress no_valid_uid" + ); + } + + #[test] + fn eids_lookup_reports_parse_error() { + let req = get_request_with_cookie("/_ts/admin/eids", "ts-eids=!!!not-base64!!!"); + + let response = + handle_admin_eids_lookup(&test_registry(), &req).expect("should handle eids lookup"); + + assert_eq!( + response.status(), + StatusCode::OK, + "malformed cookies should be reported, not errored" + ); + let json = response_json(response); + assert_eq!(json["cookie_present"], true); + assert!( + json["parse_error"] + .as_str() + .expect("should have parse_error") + .contains("ts-eids"), + "should describe the parse failure" + ); + assert!(json.get("eids").is_none(), "should omit unparsed eids"); + assert!( + json["ingest"]["matched"] + .as_array() + .expect("should have matched list") + .is_empty(), + "unparseable cookie should preview no matches" + ); + } + + #[test] + fn eids_lookup_includes_sharedid_match() { + let registry = PartnerRegistry::from_config(&[ + make_test_partner("bidstream.example", true), + make_test_partner("sharedid.org", true), + ]) + .expect("should build sharedid test registry"); + let req = get_request_with_cookie("/_ts/admin/eids", "sharedId=shared-uid-123"); + + let response = + handle_admin_eids_lookup(®istry, &req).expect("should handle eids lookup"); + + assert_eq!(response.status(), StatusCode::OK); + let json = response_json(response); + assert_eq!(json["cookie_present"], false); + assert_eq!(json["sharedid_present"], true); + + let matched = json["ingest"]["matched"] + .as_array() + .expect("should have matched list"); + assert_eq!(matched.len(), 1, "should match the sharedid partner"); + assert_eq!(matched[0]["source_domain"], "sharedid.org"); + assert_eq!(matched[0]["uid"], "shared-uid-123"); + } +} diff --git a/crates/trusted-server-core/src/ec/kv.rs b/crates/trusted-server-core/src/ec/kv.rs index 7be767557..3572581ce 100644 --- a/crates/trusted-server-core/src/ec/kv.rs +++ b/crates/trusted-server-core/src/ec/kv.rs @@ -21,7 +21,7 @@ use crate::error::TrustedServerError; use super::current_timestamp; use super::generation::ec_hash; -use super::kv_backend::{EcKvStore, EcKvWrite, EcKvWriteMode, EcKvWriteOutcome}; +use super::kv_backend::{EcKvLookup, EcKvStore, EcKvWrite, EcKvWriteMode, EcKvWriteOutcome}; use super::kv_types::{KvEntry, KvMetadata, KvNetwork}; use super::log_id; @@ -170,6 +170,25 @@ impl KvIdentityGraph { Ok((body, meta_str)) } + /// Reads the raw stored body, metadata, and generation for an EC ID key. + /// + /// Unlike [`Self::get`], the entry body is returned without + /// deserialization or validation, so corrupt or legacy-schema records can + /// still be inspected instead of failing closed. Used by the admin EC + /// lookup endpoint. + /// + /// Returns `Ok(None)` when the key does not exist. + /// + /// # Errors + /// + /// Returns [`TrustedServerError::KvStore`] on store open or read failure. + pub fn lookup_raw( + &self, + ec_id: &str, + ) -> Result, Report> { + self.store.lookup(ec_id) + } + /// Reads the full entry and its generation marker for CAS writes. /// /// Returns `Ok(None)` when the key does not exist. diff --git a/crates/trusted-server-core/src/ec/mod.rs b/crates/trusted-server-core/src/ec/mod.rs index d17fcd519..840ce90d3 100644 --- a/crates/trusted-server-core/src/ec/mod.rs +++ b/crates/trusted-server-core/src/ec/mod.rs @@ -31,6 +31,7 @@ mod auth; +pub mod admin; pub mod batch_sync; pub mod consent; pub mod cookies; diff --git a/crates/trusted-server-core/src/ec/prebid_eids.rs b/crates/trusted-server-core/src/ec/prebid_eids.rs index 2f99c8182..4a1a8d156 100644 --- a/crates/trusted-server-core/src/ec/prebid_eids.rs +++ b/crates/trusted-server-core/src/ec/prebid_eids.rs @@ -49,6 +49,22 @@ struct StructuredCookieUid { ext: Option, } +enum DecodedCookieEids { + Legacy(Vec), + Structured(Vec), +} + +pub(crate) struct DiagnosticEidSource { + pub(crate) source: String, + pub(crate) uids: Vec, +} + +pub(crate) struct PrebidEidAnalysis { + pub(crate) eids: Vec, + pub(crate) diagnostic_sources: Vec, + pub(crate) updates: Vec, +} + trait PartnerIdBulkWriter { fn upsert_partner_ids( &self, @@ -77,6 +93,10 @@ impl PartnerIdBulkWriter for KvIdentityGraph { /// Returns an error when the cookie exceeds the raw size limit, is not valid /// base64, or does not contain either supported JSON payload shape. pub fn parse_prebid_eids_cookie(cookie_value: &str) -> Result, String> { + decode_prebid_eids_cookie(cookie_value).map(DecodedCookieEids::into_openrtb) +} + +fn decode_prebid_eids_cookie(cookie_value: &str) -> Result { if eids_cookie_exceeds_size_limit(cookie_value) { return Err(format!( "ts-eids cookie too large ({} bytes)", @@ -89,12 +109,42 @@ pub fn parse_prebid_eids_cookie(cookie_value: &str) -> Result, String> .map_err(|e| format!("base64 decode failed: {e}"))?; if let Ok(eids) = serde_json::from_slice::>(&bytes) { - return Ok(legacy_cookie_eids_to_openrtb(eids)); + return Ok(DecodedCookieEids::Legacy(eids)); } let structured = serde_json::from_slice::>(&bytes) .map_err(|e| format!("JSON parse failed: {e}"))?; - Ok(structured_cookie_eids_to_openrtb(structured)) + Ok(DecodedCookieEids::Structured(structured)) +} + +impl DecodedCookieEids { + fn diagnostic_sources(&self) -> Vec { + match self { + Self::Legacy(entries) => entries + .iter() + .filter(|entry| !entry.source.is_empty()) + .map(|entry| DiagnosticEidSource { + source: entry.source.clone(), + uids: vec![entry.id.clone()], + }) + .collect(), + Self::Structured(entries) => entries + .iter() + .filter(|entry| !entry.source.is_empty()) + .map(|entry| DiagnosticEidSource { + source: entry.source.clone(), + uids: entry.uids.iter().map(|uid| uid.id.clone()).collect(), + }) + .collect(), + } + } + + fn into_openrtb(self) -> Vec { + match self { + Self::Legacy(entries) => legacy_cookie_eids_to_openrtb(entries), + Self::Structured(entries) => structured_cookie_eids_to_openrtb(entries), + } + } } /// Parses request-local EID cookies and writes matched partner UIDs to KV. @@ -175,17 +225,40 @@ fn ingest_eid_cookies_with_writer( } } -fn collect_prebid_eid_updates( +pub(crate) fn collect_prebid_eid_updates( cookie_value: &str, registry: &PartnerRegistry, ) -> Vec { - let Ok(eids) = parse_prebid_eids_cookie(cookie_value) else { + let Ok(analysis) = analyze_prebid_eids_cookie(cookie_value, registry) else { log::trace!("Prebid EIDs: failed to decode ts-eids cookie; dropping"); return Vec::new(); }; + analysis.updates +} + +pub(crate) fn analyze_prebid_eids_cookie( + cookie_value: &str, + registry: &PartnerRegistry, +) -> Result { + let decoded = decode_prebid_eids_cookie(cookie_value)?; + let diagnostic_sources = decoded.diagnostic_sources(); + let eids = decoded.into_openrtb(); + let updates = collect_prebid_eid_updates_from_eids(&eids, registry); + + Ok(PrebidEidAnalysis { + eids, + diagnostic_sources, + updates, + }) +} + +fn collect_prebid_eid_updates_from_eids( + eids: &[Eid], + registry: &PartnerRegistry, +) -> Vec { let mut updates = Vec::new(); - for eid in &eids { + for eid in eids { let Some(partner) = registry.find_by_source_domain(&eid.source) else { log::debug!("Prebid EIDs: no partner for source '{}'", eid.source); continue; @@ -209,7 +282,7 @@ fn collect_prebid_eid_updates( updates } -fn dedupe_partner_updates(updates: Vec) -> Vec { +pub(crate) fn dedupe_partner_updates(updates: Vec) -> Vec { let mut latest = std::collections::BTreeMap::new(); for update in updates { latest.insert(update.partner_id, update.uid); @@ -222,9 +295,11 @@ fn dedupe_partner_updates(updates: Vec) -> Vec } fn first_valid_uid(uids: &[Uid]) -> Option<&Uid> { - uids.iter() - .filter(|uid| !uid.id.trim().is_empty()) - .find(|uid| !eid_id_exceeds_size_limit(&uid.id)) + uids.iter().find(|uid| is_valid_eid_uid(&uid.id)) +} + +pub(crate) fn is_valid_eid_uid(uid: &str) -> bool { + !uid.trim().is_empty() && !eid_id_exceeds_size_limit(uid) } /// `SharedID` EID source domain used for partner registry lookup. @@ -246,7 +321,7 @@ pub fn ingest_sharedid_cookie( ingest_eid_cookies(None, Some(cookie_value), ec_id, kv, registry); } -fn collect_sharedid_update( +pub(crate) fn collect_sharedid_update( cookie_value: &str, registry: &PartnerRegistry, ) -> Option { diff --git a/crates/trusted-server-core/src/settings.rs b/crates/trusted-server-core/src/settings.rs index 598c00056..bacb7aa36 100644 --- a/crates/trusted-server-core/src/settings.rs +++ b/crates/trusted-server-core/src/settings.rs @@ -2191,15 +2191,62 @@ impl Settings { Ok(None) } + /// Returns whether `path` is within the reserved Trusted Server admin + /// namespace. + #[must_use] + pub(crate) fn is_admin_path(path: &str) -> bool { + path == "/_ts/admin" || path.starts_with("/_ts/admin/") + } + /// Known admin endpoint paths that must be covered by a handler. /// /// [`from_toml`](Self::from_toml) rejects configurations /// where any of these paths lack a matching handler, ensuring admin /// endpoints are always protected by authentication. /// Update [`ADMIN_ENDPOINTS`](Self::ADMIN_ENDPOINTS) when adding new - /// admin routes to `crates/trusted-server-adapter-fastly/src/main.rs`. - pub(crate) const ADMIN_ENDPOINTS: &[&str] = - &["/_ts/admin/keys/rotate", "/_ts/admin/keys/deactivate"]; + /// admin routes to `crates/trusted-server-adapter-fastly/src/app.rs`. + /// + /// The `/_ts/admin/ec/{id}` entry is the canonical router pattern. Its + /// coverage is checked via [`admin_auth_probes`](Self::admin_auth_probes), + /// while validation errors continue to report this operator-facing route + /// template. + pub(crate) const ADMIN_ENDPOINTS: &[&str] = &[ + "/_ts/admin/keys/rotate", + "/_ts/admin/keys/deactivate", + "/_ts/admin/ec", + "/_ts/admin/ec/{id}", + "/_ts/admin/eids", + ]; + + /// Probes that establish handler coverage for the dynamic + /// `/_ts/admin/ec/{id}` route. + /// + /// Coverage cannot be sampled: the router accepts any single segment after + /// `/_ts/admin/ec/` and basic auth runs on the raw path before routing, so + /// a handler that matches only some ID shapes leaves the rest of the route + /// surface — including malformed IDs, which still reach the admin handler — + /// unauthenticated at configuration time and fail-closed at runtime. + /// + /// Both probes must match the same configuration for the route to count as + /// covered. The bare prefix rejects handlers anchored to specific ID + /// shapes; the concrete ID rejects handlers anchored to the prefix itself + /// (`^/_ts/admin/ec/$`). Together they admit only prefix-level matchers + /// such as `^/_ts/admin` or `^/_ts/admin/ec/`. + const ADMIN_EC_ID_AUTH_PROBES: [&str; 2] = [ + "/_ts/admin/ec/", + concat!( + "/_ts/admin/ec/", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + ".Ab12Z9", + ), + ]; + + fn admin_auth_probes(path: &'static str) -> [&'static str; 2] { + match path { + "/_ts/admin/ec/{id}" => Self::ADMIN_EC_ID_AUTH_PROBES, + path => [path, path], + } + } /// Returns admin endpoint paths that no configured handler covers. /// @@ -2215,12 +2262,16 @@ impl Settings { ) -> Result, Report> { let mut uncovered = Vec::new(); for &path in Self::ADMIN_ENDPOINTS { - let mut covered = false; - for h in &self.handlers { - if h.matches_path(path)? { - covered = true; - break; + let mut covered = true; + for probe in Self::admin_auth_probes(path) { + let mut probe_covered = false; + for handler in &self.handlers { + if handler.matches_path(probe)? { + probe_covered = true; + break; + } } + covered &= probe_covered; } if !covered { uncovered.push(path); @@ -2250,18 +2301,19 @@ impl Settings { })) } + /// Rejects placeholder and well-known weak handler passwords. + /// + /// Applies to every handler rather than to handlers inferred to cover an + /// admin endpoint: handler selection is first-match-wins over operator + /// regexes, so a narrow handler can shadow the admin namespace for paths no + /// probe enumerates. Handlers are Trusted Server's own basic-auth gates, so + /// a placeholder password is never valid on any of them. fn validate_admin_handler_passwords(&self) -> Result<(), Report> { for handler in &self.handlers { - let covers_admin = Self::ADMIN_ENDPOINTS - .iter() - .try_fold(false, |covered, path| { - handler.matches_path(path).map(|matches| covered || matches) - })?; - - if covers_admin && is_admin_placeholder_password(handler.password.expose()) { + if is_admin_placeholder_password(handler.password.expose()) { return Err(Report::new(TrustedServerError::Configuration { message: format!( - "Admin handler `{}` uses a placeholder password; configure a strong secret", + "Handler `{}` uses a placeholder password; configure a strong secret", handler.path ), })); @@ -4874,7 +4926,13 @@ origin_host_header_overide = "www.example.com""#, .expect("should check admin coverage"); assert_eq!( uncovered, - vec!["/_ts/admin/keys/rotate", "/_ts/admin/keys/deactivate"], + vec![ + "/_ts/admin/keys/rotate", + "/_ts/admin/keys/deactivate", + "/_ts/admin/ec", + "/_ts/admin/ec/{id}", + "/_ts/admin/eids", + ], "should report every admin endpoint as uncovered" ); } @@ -4908,11 +4966,195 @@ origin_host_header_overide = "www.example.com""#, .expect("should check admin coverage"); assert_eq!( uncovered, - vec!["/_ts/admin/keys/deactivate"], + vec![ + "/_ts/admin/keys/deactivate", + "/_ts/admin/ec", + "/_ts/admin/ec/{id}", + "/_ts/admin/eids", + ], "should detect the admin endpoints not covered by the narrow handler" ); } + #[test] + fn from_toml_rejects_literal_parameter_template_auth_coverage() { + let toml_str = crate_test_settings_str().replace( + r#"path = "^/_ts/admin" + username = "admin" + password = "admin-pass""#, + r#"path = "^/_ts/admin/(keys/rotate|keys/deactivate|ec|eids)$" + username = "admin" + password = "strong-test-password" + + [[handlers]] + path = "^/_ts/admin/ec/[{]id[}]$" + username = "admin" + password = "strong-test-password""#, + ); + + let error = Settings::from_toml(&toml_str) + .expect_err("should reject literal parameter-template auth coverage"); + let message = format!("{error:?}"); + assert!( + message.contains("/_ts/admin/ec/{id}"), + "should identify the concrete EC route as uncovered, got: {message}" + ); + } + + #[test] + fn from_toml_rejects_lowercase_only_dynamic_admin_ec_auth_coverage() { + let toml_str = crate_test_settings_str().replace( + r#"path = "^/_ts/admin" + username = "admin" + password = "admin-pass""#, + r#"path = "^/_ts/admin/(keys/rotate|keys/deactivate|ec|eids)$" + username = "admin" + password = "strong-test-password" + + [[handlers]] + path = "^/_ts/admin/ec/[a-f0-9]{64}[.][a-z0-9]{6}$" + username = "admin" + password = "strong-test-password""#, + ); + + let error = Settings::from_toml(&toml_str) + .expect_err("should reject lowercase-only dynamic EC auth coverage"); + let message = format!("{error:?}"); + assert!( + message.contains("/_ts/admin/ec/{id}"), + "should identify the mixed-case EC route as uncovered, got: {message}" + ); + } + + #[test] + fn from_toml_rejects_placeholder_password_on_shadowing_admin_handler() { + // Handler selection is first-match-wins, so a narrow handler placed + // ahead of the admin matcher governs the EC IDs it matches. No probe + // enumerates those IDs, so the placeholder check cannot be limited to + // handlers inferred to cover an admin endpoint. + let toml_str = crate_test_settings_str().replace( + r#"path = "^/_ts/admin" + username = "admin" + password = "admin-pass""#, + r#"path = "^/_ts/admin/ec/[a-f0-9]{64}[.]zzzzzz$" + username = "admin" + password = "change-me-admin-password" + + [[handlers]] + path = "^/_ts/admin" + username = "admin" + password = "strong-test-password""#, + ); + + let error = Settings::from_toml(&toml_str) + .expect_err("should reject placeholder password on shadowing admin handler"); + let message = format!("{error:?}"); + assert!( + message.contains("placeholder password"), + "should identify the placeholder handler password, got: {message}" + ); + } + + #[test] + fn from_toml_rejects_weak_password_on_non_admin_handler() { + let toml_str = crate_test_settings_str().replace( + r#"path = "^/_ts/admin" + username = "admin" + password = "admin-pass""#, + r#"path = "^/_ts/admin" + username = "admin" + password = "strong-test-password" + + [[handlers]] + path = "^/private" + username = "admin" + password = "changeme""#, + ); + + let error = Settings::from_toml(&toml_str) + .expect_err("should reject a weak password on any handler"); + let message = format!("{error:?}"); + assert!( + message.contains("placeholder password"), + "should identify the weak handler password, got: {message}" + ); + } + + #[test] + fn from_toml_rejects_sampled_id_only_dynamic_admin_ec_auth_coverage() { + // A handler anchored to the full EC ID grammar still leaves the rest of + // the route surface (malformed IDs, which the router accepts and the + // admin handler rejects with 400) unauthenticated, so coverage must not + // be inferred from ID-shaped samples. + let toml_str = crate_test_settings_str().replace( + r#"path = "^/_ts/admin" + username = "admin" + password = "admin-pass""#, + r#"path = "^/_ts/admin/(keys/rotate|keys/deactivate|ec|eids)$" + username = "admin" + password = "strong-test-password" + + [[handlers]] + path = "^/_ts/admin/ec/[a-f0-9]{64}[.][A-Za-z0-9]{6}$" + username = "admin" + password = "strong-test-password""#, + ); + + let error = Settings::from_toml(&toml_str) + .expect_err("should reject ID-sampled dynamic EC auth coverage"); + let message = format!("{error:?}"); + assert!( + message.contains("/_ts/admin/ec/{id}"), + "should identify the dynamic EC route as uncovered, got: {message}" + ); + } + + #[test] + fn from_toml_rejects_prefix_anchored_admin_ec_auth_coverage() { + // `^/_ts/admin/ec/$` matches the prefix probe but no actual lookup. + let toml_str = crate_test_settings_str().replace( + r#"path = "^/_ts/admin" + username = "admin" + password = "admin-pass""#, + r#"path = "^/_ts/admin/(keys/rotate|keys/deactivate|ec|eids)$" + username = "admin" + password = "strong-test-password" + + [[handlers]] + path = "^/_ts/admin/ec/$" + username = "admin" + password = "strong-test-password""#, + ); + + let error = Settings::from_toml(&toml_str) + .expect_err("should reject prefix-anchored dynamic EC auth coverage"); + let message = format!("{error:?}"); + assert!( + message.contains("/_ts/admin/ec/{id}"), + "should identify the dynamic EC route as uncovered, got: {message}" + ); + } + + #[test] + fn from_toml_accepts_prefix_matcher_admin_ec_auth_coverage() { + let toml_str = crate_test_settings_str().replace( + r#"path = "^/_ts/admin" + username = "admin" + password = "admin-pass""#, + r#"path = "^/_ts/admin/(keys/rotate|keys/deactivate|ec|eids)$" + username = "admin" + password = "strong-test-password" + + [[handlers]] + path = "^/_ts/admin/ec/" + username = "admin" + password = "strong-test-password""#, + ); + + Settings::from_toml(&toml_str) + .expect("should accept a prefix-level matcher for the dynamic EC route"); + } + #[test] fn from_toml_and_env_rejects_config_without_admin_handler() { let origin_key = format!( diff --git a/docs/guide/api-reference.md b/docs/guide/api-reference.md index 340d5d425..b4cb64481 100644 --- a/docs/guide/api-reference.md +++ b/docs/guide/api-reference.md @@ -7,6 +7,7 @@ Quick reference for all Trusted Server HTTP endpoints. - [First-Party Endpoints](#first-party-endpoints) - Core ad serving and proxying - [Edge Cookie Endpoints](#edge-cookie-endpoints) - Identity sync and enrichment - [Request Signing](#request-signing-endpoints) - Cryptographic signing and key management +- [Admin Diagnostics](#admin-diagnostic-endpoints) - Protected EC troubleshooting - [TSJS Library](#tsjs-library-endpoint) - JavaScript library serving - [Utility Endpoints](#utility-endpoints) - Optional operational helpers - [Integration Endpoints](#integration-endpoints) - Third-party service proxying @@ -580,6 +581,87 @@ curl -X POST https://edge.example.com/_ts/admin/keys/deactivate \ --- +## Admin Diagnostic Endpoints + +These endpoints expose sensitive identity and cookie data and require HTTP Basic Authentication. Configure a handler that covers the entire `/_ts/admin` namespace; startup rejects configurations that do not protect every admin route, including handlers that match only some `/_ts/admin/ec/{id}` values — the dynamic route needs a prefix-level matcher such as `^/_ts/admin` or `^/_ts/admin/ec/`. The whole `/_ts/admin` prefix is reserved: any admin path that reaches publisher fallback — unknown, malformed, or percent-encoded (`/_ts/admin%2Fec`) — is answered locally with `404` and is never proxied, so an admin `Authorization` header and request body never reach the publisher origin. The retired non-`/_ts` `/admin/keys` aliases are reserved the same way. Normal diagnostic-handler responses after successful authentication are JSON with `Cache-Control: no-store`. Missing or invalid credentials receive the shared plaintext `401 Unauthorized` Basic-auth challenge. Unexpected configuration or KV failures use the adapter's shared plaintext `5xx` error response. Those authentication and internal-error responses are outside the diagnostic JSON and cache-header contract. + +The examples below use fictional IDs and values only. + +### GET /\_ts/admin/ec + +### GET /\_ts/admin/ec/`{id}` + +Reads an EC identity-graph record for troubleshooting. The explicit route accepts an EC ID in `{64 lowercase hex}.{6 alphanumeric}` format. The bare route uses the request's `ts-ec` cookie. + +This lookup is implemented only by the Fastly adapter because the identity graph is stored in Fastly KV. Other adapters return `501 Not Implemented`. + +**Response fields:** + +- `ec_id`, `store`, and `generation` identify the raw KV lookup. +- `entry` preserves the stored JSON shape, including unknown and legacy fields. Derived `created_iso` and `consent.updated_iso` fields are added only when absent. +- `metadata` preserves the stored metadata JSON shape. +- `tombstone` reports whether consent has been withdrawn. It is absent when the entry body cannot be parsed as JSON or deserialized as the typed EC schema. +- `auction.eids` previews the partner EIDs the stored record can contribute; `auction.skipped` explains filtered IDs. +- `entry_error`, `metadata_error`, and `raw_body` keep malformed or schema-incompatible records inspectable. + +The auction preview validates the stored record and partner configuration, but cannot reproduce live per-request consent checks. It must not be treated as proof that a specific auction request will receive those EIDs. + +**Status codes:** + +| Status | Meaning | +| ------ | ----------------------------------------------------------- | +| `200` | Record found, including inspectable corrupt records | +| `400` | Invalid explicit EC ID | +| `401` | Missing or invalid Basic credentials | +| `404` | Record not found, or the bare route has no `ts-ec` cookie | +| `405` | Method other than `GET` (`Allow: GET`) | +| `501` | EC identity graph unavailable on this adapter or deployment | +| `5xx` | Unexpected configuration or KV failure (plaintext) | + +```bash +curl -u admin:secure-password \ + "https://edge.example.com/_ts/admin/ec/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.abc123" + +curl -u admin:secure-password \ + --cookie "ts-ec=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.abc123" \ + "https://edge.example.com/_ts/admin/ec" +``` + +### GET /\_ts/admin/eids + +Parses the request's `ts-eids` and `sharedId` cookies and previews which configured partner IDs cookie ingestion would match or drop. It performs request inspection only: it does not read or write KV and is available on every adapter. + +After successful authentication this endpoint always returns `200 OK`; missing or malformed cookies are represented by `cookie_present`, `sharedid_present`, and `parse_error`. The `ingest.matched` and `ingest.unmatched` arrays show the ingestion preview. Each unmatched entry contains its `source` and either a `no_partner` reason when no configured partner recognizes it or `no_valid_uid` when the partner exists but every supplied UID is empty or exceeds the storage limit. + +```json +{ + "ingest": { + "matched": [ + { + "source_domain": "configured.example", + "uid": "fictional-uid" + } + ], + "unmatched": [ + { + "source": "unknown.example", + "reason": "no_partner" + } + ] + } +} +``` + +```bash +curl -u admin:secure-password \ + --cookie "sharedId=fictional-shared-id" \ + "https://edge.example.com/_ts/admin/eids" +``` + +Malformed diagnostic paths return a local `404`, and unsupported methods return a local `405`; they are never forwarded to the publisher origin. + +--- + ## TSJS Library Endpoint ### GET /static/tsjs=`` @@ -768,6 +850,9 @@ curl -u admin:secure-password https://edge.example.com/_ts/admin/keys/rotate - `/_ts/admin/keys/rotate` - `/_ts/admin/keys/deactivate` +- `/_ts/admin/ec` +- `/_ts/admin/ec/{id}` +- `/_ts/admin/eids` - Any paths matching configured `handlers` patterns --- diff --git a/docs/guide/architecture.md b/docs/guide/architecture.md index 3b20000ec..da1a58bcd 100644 --- a/docs/guide/architecture.md +++ b/docs/guide/architecture.md @@ -53,13 +53,13 @@ Native Axum dev/test adapter (native binary): **Current limitations compared to the Fastly adapter:** -| Feature | Axum dev server | -| ------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------- | -| KV store | Unavailable — synthetic-ID and consent routes degrade gracefully | -| Geo lookup | Always returns `None` | -| Config/secret-store writes | Return an error (read-only via env vars) | -| Admin key management (`/_ts/admin/keys/*`) | Returns 501 Not Implemented. Legacy `/admin/keys/*` aliases are denied locally with 404 and are not proxied to the publisher fallback | -| Auction fan-out ordering | Requests run concurrently via `tokio::spawn`; `select` returns first-to-complete but does not replicate Fastly's priority-queue tie-breaking | +| Feature | Axum dev server | +| ------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| KV store | Unavailable — synthetic-ID and consent routes degrade gracefully | +| Geo lookup | Always returns `None` | +| Config/secret-store writes | Return an error (read-only via env vars) | +| Admin key management (`/_ts/admin/keys/*`) | Returns 501 Not Implemented. Retired `/admin/keys` aliases, including trailing, descendant, and percent-encoded forms, are denied locally with 404 and are not proxied to the publisher fallback | +| Auction fan-out ordering | Requests run concurrently via `tokio::spawn`; `select` returns first-to-complete but does not replicate Fastly's priority-queue tie-breaking | ### trusted-server-adapter-spin diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index ddb6544ce..905df9e60 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -645,6 +645,23 @@ path = "^/api/v[0-9]+/private" # /api/v1/private, /api/v2/private **Validation**: Application startup fails if regex is invalid. +::: warning Admin coverage and passwords are validated at startup + +Startup fails when no handler covers an admin route. The dynamic +`/_ts/admin/ec/{id}` route accepts any segment after `/_ts/admin/ec/`, and +Basic Auth runs on the raw path before routing, so coverage cannot be inferred +from ID-shaped samples: a pattern such as +`^/_ts/admin/ec/[a-f0-9]{64}[.][A-Za-z0-9]{6}$` is rejected. Use a prefix-level +matcher (`^/_ts/admin`, or `^/_ts/admin/ec/` alongside the other admin +patterns). + +Startup also fails when any handler — admin or not — uses a placeholder or +well-known weak password (`changeme`, `password`, `admin`, or a +`replace-with-…` template value). Handler selection is first-match-wins, so a +narrow handler ahead of the admin pattern governs the paths it matches. + +::: + ::: warning Scope patterns to the paths you mean Handler patterns are matched against the full request path, so a broad pattern diff --git a/docs/superpowers/plans/2026-08-18-admin-diagnostics-review-fixes.md b/docs/superpowers/plans/2026-08-18-admin-diagnostics-review-fixes.md new file mode 100644 index 000000000..b13430a45 --- /dev/null +++ b/docs/superpowers/plans/2026-08-18-admin-diagnostics-review-fixes.md @@ -0,0 +1,576 @@ +# Admin Diagnostics Review Fixes Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Close every PR #928 review finding by making admin diagnostics fail closed, preventing publisher/KV side effects, preserving raw KV JSON, and documenting the API. + +**Architecture:** Core settings owns the canonical admin-template-to-auth-probe mapping and runtime admin namespace classification. Core EC admin code owns one shared fallback-denial response so each adapter only adds a small guard at its publisher fallback boundary. Fastly dispatches the read-only EIDs diagnostic before EC setup, while raw JSON display and typed interpretation remain separate inside the core handler. + +**Tech Stack:** Rust 2024, `http`, `serde_json`, `error-stack`, EdgeZero adapter routers, Fastly/Viceroy, Markdown documentation. + +**Design spec:** `docs/superpowers/specs/2026-08-18-admin-diagnostics-review-fixes-design.md` + +--- + +## File map + +- Modify `crates/trusted-server-core/src/settings.rs`: canonical admin route/auth + probes, admin namespace classification, startup validation, and tests. +- Modify `crates/trusted-server-core/src/auth.rs`: runtime fail-closed behavior + and regression tests. +- Modify `crates/trusted-server-core/src/ec/admin.rs`: shared diagnostic + fallback denial, lossless raw JSON display, and unit tests. +- Modify `crates/trusted-server-adapter-fastly/src/app.rs`: fallback guard, + early read-only EIDs dispatch, and adapter tests. +- Modify each portability adapter's `src/app.rs` and `tests/routes.rs`: fallback + guard and cross-adapter route regressions. +- Modify `docs/guide/api-reference.md`: operator-facing contract. + +No new crate, dependency, schema type, or test-only production seam is needed. + +### Task 1: Make admin authentication coverage parameter-aware and fail closed + +**Files:** + +- Modify: `crates/trusted-server-core/src/settings.rs:2194-2279` +- Modify: `crates/trusted-server-core/src/settings.rs:4876-4970` +- Modify: `crates/trusted-server-core/src/auth.rs:29-55` +- Test: `crates/trusted-server-core/src/auth.rs:79-315` + +- [ ] **Step 1: Add a failing literal-template startup regression** + +Build settings TOML whose first handler covers the four non-parameterized admin +routes and whose second handler covers only literal braces: + +```rust +[[handlers]] +path = "^/_ts/admin/(keys/rotate|keys/deactivate|ec|eids)$" +username = "admin" +password = "strong-test-password" + +[[handlers]] +path = "^/_ts/admin/ec/[{]id[}]$" +username = "admin" +password = "strong-test-password" +``` + +Assert `Settings::from_toml` fails and identifies `/_ts/admin/ec/{id}` as +uncovered. + +- [ ] **Step 2: Run the regression and verify RED** + +```bash +cargo test-fastly from_toml_rejects_literal_parameter_template_auth_coverage +``` + +Expected: FAIL because current validation accepts the literal template match. + +- [ ] **Step 3: Add and run a failing concrete-handler password regression** + +Add settings with a concrete-ID handler regex +`^/_ts/admin/ec/[a-f0-9]{64}[.][a-z0-9]{6}$` and placeholder password +`change-me-admin-password`. Assert finalization rejects it as an admin handler. + +```bash +cargo test-fastly from_toml_rejects_placeholder_password_for_concrete_admin_ec_handler +``` + +Expected: FAIL because password validation also uses the literal template. + +- [ ] **Step 4: Implement one canonical template-to-auth-probe mapping** + +Keep `Settings::ADMIN_ENDPOINTS` as canonical templates. Add a fixed fictional +valid EC probe and a helper used by both coverage and password validation: + +```rust +const ADMIN_EC_ID_AUTH_PROBE: &str = concat!( + "/_ts/admin/ec/", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + ".abc123", +); + +fn admin_auth_probe(path: &'static str) -> &'static str { + match path { + "/_ts/admin/ec/{id}" => ADMIN_EC_ID_AUTH_PROBE, + path => path, + } +} +``` + +Make `uncovered_admin_endpoints` report the template but match its probe. Make +`validate_admin_handler_passwords` use the same helper. Update stale comments. + +- [ ] **Step 5: Run both settings regressions and verify GREEN** + +```bash +cargo test-fastly literal_parameter_template_auth_coverage +cargo test-fastly placeholder_password_for_concrete_admin_ec_handler +``` + +Expected: PASS. + +- [ ] **Step 6: Add failing runtime fail-closed auth tests** + +In `auth.rs`, deserialize settings directly with `toml::from_str` to bypass +startup finalization. With the literal-template-only configuration, send a +concrete valid EC request and assert `enforce_basic_auth` returns a configuration +error rather than `Ok(None)`. Add `/_ts/administrator` as a boundary case that +must remain public when no handler matches. + +- [ ] **Step 7: Run runtime tests and verify RED** + +```bash +cargo test-fastly concrete_admin_path_without_matching_handler_fails_closed +``` + +Expected: FAIL because current auth returns `Ok(None)`. + +- [ ] **Step 8: Implement the runtime namespace invariant** + +Add: + +```rust +#[must_use] +pub fn is_admin_path(path: &str) -> bool { + path == "/_ts/admin" || path.starts_with("/_ts/admin/") +} +``` + +When `handler_for_path` returns `None`, make `enforce_basic_auth` return +`TrustedServerError::Configuration` for an admin path and retain `Ok(None)` for +all other paths. + +- [ ] **Step 9: Run core tests and target-matched suite** + +```bash +cargo test-fastly admin_path +cargo test-fastly uncovered_admin_endpoints +cargo test-fastly +``` + +Expected: PASS without warnings. + +- [ ] **Step 10: Commit Task 1** + +```bash +git add crates/trusted-server-core/src/settings.rs crates/trusted-server-core/src/auth.rs +git commit -m "Fail closed for concrete admin routes" +``` + +### Task 2: Add a shared local denial for diagnostic fallback requests + +**Files:** + +- Modify: `crates/trusted-server-core/src/ec/admin.rs:20-45` +- Modify: `crates/trusted-server-core/src/ec/admin.rs:432-464` +- Test: `crates/trusted-server-core/src/ec/admin.rs:464-925` + +- [ ] **Step 1: Add failing table-driven fallback tests** + +Test a new +`deny_admin_diagnostic_fallback(&Request) -> Option>` +helper. For bare EC, single-ID EC, and EIDs, every non-GET publisher fallback +method must return `405`, `Allow: GET`, and `Cache-Control: no-store`. GET and +non-GET requests to trailing/extra-segment EC/EIDs forms must return `404` and +`no-store`. An unrelated publisher path must return `None`. + +- [ ] **Step 2: Run focused tests and verify RED** + +```bash +cargo test-fastly admin_diagnostic_fallback +``` + +Expected: compilation FAIL because the helper does not exist. + +- [ ] **Step 3: Implement classification and response construction** + +Add a private path-shape classifier and documented public helper. Its core +response logic is: + +```rust +let mut response = if shape.is_valid_resource() && req.method() != Method::GET { + json_error(StatusCode::METHOD_NOT_ALLOWED, "method not allowed") +} else { + json_error(StatusCode::NOT_FOUND, "admin diagnostic route not found") +}; +if response.status() == StatusCode::METHOD_NOT_ALLOWED { + response + .headers_mut() + .insert(header::ALLOW, HeaderValue::from_static("GET")); +} +``` + +Reuse `json_error`/`json_response`, which already set JSON and `no-store`. +Classify only the EC/EIDs families. EC bare and exactly one non-empty ID segment +are valid resource shapes; EIDs exact is valid; suffix/trailing forms are +malformed. A valid GET that somehow reaches fallback returns local `404`. + +- [ ] **Step 4: Run focused and target-matched tests** + +```bash +cargo test-fastly admin_diagnostic_fallback +cargo test-fastly +``` + +Expected: PASS. + +- [ ] **Step 5: Commit Task 2** + +```bash +git add crates/trusted-server-core/src/ec/admin.rs +git commit -m "Deny admin diagnostics in publisher fallback" +``` + +### Task 3: Wire the denial guard into every adapter + +**Files:** + +- Modify/Test: `crates/trusted-server-adapter-fastly/src/app.rs` +- Modify: `crates/trusted-server-adapter-axum/src/app.rs` +- Test: `crates/trusted-server-adapter-axum/tests/routes.rs` +- Modify: `crates/trusted-server-adapter-cloudflare/src/app.rs` +- Test: `crates/trusted-server-adapter-cloudflare/tests/routes.rs` +- Modify: `crates/trusted-server-adapter-spin/src/app.rs` +- Test: `crates/trusted-server-adapter-spin/tests/routes.rs` + +- [ ] **Step 1: Add Fastly adapter regressions and verify RED** + +Using authenticated router requests, test the valid diagnostic shapes against +`POST`, `HEAD`, `OPTIONS`, `PUT`, `PATCH`, and `DELETE`, asserting `405`, +`Allow: GET`, and `no-store`. Test authenticated GET and POST requests for +trailing/extra-segment forms, asserting `404` and `no-store`. + +```bash +cargo test-fastly authenticated_admin_diagnostic_fallback +``` + +Expected: FAIL because requests enter publisher fallback. + +- [ ] **Step 2: Wire Fastly and verify GREEN** + +Import the helper and call it at the start of `dispatch_fallback`, before GPT +preparation, filters, integration routing, or publisher handling: + +```rust +if let Some(response) = deny_admin_diagnostic_fallback(&req) { + return response; +} +``` + +```bash +cargo test-fastly authenticated_admin_diagnostic_fallback +cargo test-fastly +``` + +Expected: PASS. + +- [ ] **Step 3: Add Axum regressions and verify RED** + +Add the same matrices in `tests/routes.rs` using `make_service()`. + +```bash +cargo test-axum authenticated_admin_diagnostic_fallback +``` + +Expected: FAIL. + +- [ ] **Step 4: Wire Axum and verify GREEN** + +Call the helper at the start of Axum's fallback `dispatch` before publisher +handling. + +```bash +cargo test-axum authenticated_admin_diagnostic_fallback +cargo test-axum +``` + +Expected: PASS. + +- [ ] **Step 5: Add Cloudflare regressions and verify RED** + +Use `request_builder()` plus `route(test_router(), req)`. + +```bash +cargo test-cloudflare authenticated_admin_diagnostic_fallback +``` + +Expected: FAIL. + +- [ ] **Step 6: Wire Cloudflare and verify GREEN** + +Call the helper before Cloudflare integration/publisher dispatch. + +```bash +cargo test-cloudflare authenticated_admin_diagnostic_fallback +cargo test-cloudflare +``` + +Expected: PASS. + +- [ ] **Step 7: Add Spin regressions and verify RED** + +Use the existing Spin router helpers with the same matrices. + +```bash +cargo test-spin authenticated_admin_diagnostic_fallback +``` + +Expected: FAIL. + +- [ ] **Step 8: Wire Spin and verify GREEN** + +Call the helper before Spin integration/publisher dispatch. + +```bash +cargo test-spin authenticated_admin_diagnostic_fallback +cargo test-spin +``` + +Expected: PASS. + +- [ ] **Step 9: Commit Task 3** + +```bash +git add crates/trusted-server-adapter-fastly/src/app.rs \ + crates/trusted-server-adapter-axum/src/app.rs \ + crates/trusted-server-adapter-axum/tests/routes.rs \ + crates/trusted-server-adapter-cloudflare/src/app.rs \ + crates/trusted-server-adapter-cloudflare/tests/routes.rs \ + crates/trusted-server-adapter-spin/src/app.rs \ + crates/trusted-server-adapter-spin/tests/routes.rs +git commit -m "Keep admin diagnostics out of publisher fallback" +``` + +### Task 4: Make Fastly EIDs diagnostics structurally read-only + +**Files:** + +- Modify/Test: `crates/trusted-server-adapter-fastly/src/app.rs:501-595` +- Reference: `crates/trusted-server-adapter-fastly/src/main.rs:184-247` +- Reference: `crates/trusted-server-core/src/ec/finalize.rs:77-106` + +- [ ] **Step 1: Add and run a failing finalization-state regression** + +Create an authenticated browser-shaped `GET /_ts/admin/eids` request with valid +EC, EID, and shared-ID cookies. Assert `200` and: + +```rust +assert!( + response.extensions().get::().is_none(), + "admin EIDs diagnostics should not attach EC finalization state" +); +``` + +```bash +cargo test-fastly admin_eids_diagnostic_skips_ec_finalization +``` + +Expected: FAIL because `execute_named` attaches `EcFinalizeState`. + +- [ ] **Step 2: Add the early EIDs dispatch** + +Before GPT preparation or EC setup, build the registry, call the handler, map +errors through `http_error`, and return without `attach_dispatch_extensions`: + +```rust +if matches!(handler, NamedRouteHandler::AdminEidsLookup) { + let result = PartnerRegistry::from_config(&state.settings.ec.partners) + .and_then(|registry| handle_admin_eids_lookup(®istry, &req)); + return Ok(result.unwrap_or_else(|error| http_error(&error))); +} +``` + +Make the normal route arm explicitly unreachable or remove it cleanly. Update +module lifecycle comments. + +- [ ] **Step 3: Run focused and Fastly suites** + +```bash +cargo test-fastly admin_eids_diagnostic_skips_ec_finalization +cargo test-fastly +``` + +Expected: PASS. + +- [ ] **Step 4: Commit Task 4** + +```bash +git add crates/trusted-server-adapter-fastly/src/app.rs +git commit -m "Keep admin EID diagnostics read only" +``` + +### Task 5: Preserve raw parseable KV entries and metadata + +**Files:** + +- Modify/Test: `crates/trusted-server-core/src/ec/admin.rs:47-278` +- Modify/Test: `crates/trusted-server-core/src/ec/admin.rs:464-925` + +- [ ] **Step 1: Add and run a failing lossless-display regression** + +Seed a valid raw entry with unknown top-level, consent, and partner fields; +legacy map-shaped `seen_domains`; valid auction partner data; and metadata with +an unknown field. Assert the final response preserves all raw values and shape, +keeps numeric timestamps, adds ISO companions, preserves metadata, and still +derives auction EIDs. + +```bash +cargo test-fastly parseable_legacy_entry_and_metadata_preserve_raw_json +``` + +Expected: FAIL because typed reserialization drops and normalizes data. + +- [ ] **Step 2: Add and run failing schema/collision tests** + +For valid JSON that cannot deserialize as `KvEntry`, assert raw `entry` remains +present with `entry_error` and no `raw_body`. For stored `created_iso` and +`consent.updated_iso`, assert neither is overwritten. + +```bash +cargo test-fastly valid_json_with_invalid_kv_schema_remains_visible +cargo test-fastly stored_iso_fields_are_not_overwritten +``` + +Expected: FAIL. + +- [ ] **Step 3: Separate raw display from typed interpretation** + +Parse `lookup.body` as `JsonValue` for `payload.entry`, then independently as +`KvEntry` for tombstone, validation, and auction. Invalid JSON sets +`entry_error` plus lossy `raw_body`; valid JSON with an invalid schema remains +visible and sets only `entry_error`. + +Replace `entry_json_with_iso_timestamps(&KvEntry)` with a helper accepting +`&mut JsonValue`. Read raw `created` and `consent.updated` as `u64`, and use +`entry(...).or_insert(...)` for ISO companions so stored collisions win. + +Parse metadata independently as `JsonValue` for display and `KvMetadata` only +for diagnostics. Typed metadata failure must not erase parseable raw metadata; +invalid JSON remains in `metadata_error`. + +- [ ] **Step 4: Run admin and Fastly suites** + +```bash +cargo test-fastly ec::admin::tests +cargo test-fastly +``` + +Expected: PASS, including existing corrupt-entry, validation, timestamp, and +auction tests. + +- [ ] **Step 5: Commit Task 5** + +```bash +git add crates/trusted-server-core/src/ec/admin.rs +git commit -m "Preserve raw admin EC diagnostic records" +``` + +### Task 6: Document the operator API contract + +**Files:** + +- Modify: `docs/guide/api-reference.md:1-20` +- Modify: `docs/guide/api-reference.md:497-590` +- Modify: `docs/guide/api-reference.md:746-772` + +- [ ] **Step 1: Add the Admin Diagnostic Endpoints section** + +Document Basic Auth and sensitive data; both EC lookup forms; response fields +and the raw/typed error matrix; `401`, `400`, `404`, `405`, and `501`; Fastly +support and portability `501`; EIDs cookie inputs, payload, always-`200` +post-auth semantics, and all-adapter support; JSON/no-store behavior; `Allow: +GET`; and the live-consent limitation. Use only fictional/example data. + +- [ ] **Step 2: Update navigation and protected endpoints** + +Add the section to the API category list and the three routes to Protected +Endpoints. + +- [ ] **Step 3: Format and inspect documentation** + +```bash +cd docs && npm run format +git diff --check +git diff -- docs/guide/api-reference.md +``` + +Expected: formatting passes, no whitespace errors, and the contract matches +implemented statuses and headers. + +- [ ] **Step 4: Commit Task 6** + +```bash +git add docs/guide/api-reference.md +git commit -m "Document admin EC and EID diagnostics" +``` + +### Task 7: Verify the complete review resolution + +**Files:** Verify all files changed by Tasks 1-6. + +- [ ] **Step 1: Run formatting** + +```bash +cargo fmt --all -- --check +cd docs && npm run format +``` + +Expected: PASS. + +- [ ] **Step 2: Run all adapter tests** + +```bash +cargo test-fastly +cargo test-axum +cargo test-cloudflare +cargo test-spin +``` + +Expected: PASS. + +- [ ] **Step 3: Run parity tests** + +```bash +cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity +``` + +Expected: PASS. + +- [ ] **Step 4: Run all target-matched lint gates** + +```bash +cargo clippy-fastly +cargo clippy-axum +cargo clippy-cloudflare +cargo clippy-cloudflare-wasm +cargo clippy-spin-native +cargo clippy-spin-wasm +``` + +Expected: PASS with `-D warnings`. + +- [ ] **Step 5: Inspect final branch state** + +```bash +git diff main...HEAD --check +git status --short +git log --oneline --decorate -10 +``` + +Expected: no uncommitted implementation changes and a focused commit sequence. + +- [ ] **Step 6: Request code review** + +Invoke `superpowers:requesting-code-review` with the approved spec and plan. +Address only verified findings and rerun affected tests after corrections. + +- [ ] **Step 7: Verify before completion** + +Invoke `superpowers:verification-before-completion`, confirm fresh output for +every claimed gate, and report environmental limitations rather than claiming +success. + +- [ ] **Step 8: Prepare review-thread resolution notes** + +Map each of the five findings to its implementing commit and test evidence. Do +not post or resolve GitHub threads without separate user authorization. diff --git a/docs/superpowers/plans/2026-08-19-pr928-comprehensive-review-resolution.md b/docs/superpowers/plans/2026-08-19-pr928-comprehensive-review-resolution.md new file mode 100644 index 000000000..6305cfd35 --- /dev/null +++ b/docs/superpowers/plans/2026-08-19-pr928-comprehensive-review-resolution.md @@ -0,0 +1,487 @@ +# PR #928 Comprehensive Review Resolution Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Resolve all actionable PR #928 review findings while preserving existing authentication, cookie-ingestion, and adapter behavior outside the new admin diagnostics. + +**Architecture:** Fastly handles both diagnostic routes before the EC lifecycle; core settings checks the parameterized admin route against a representative valid-ID corpus; core EID parsing exposes a source-preserving diagnostic view alongside unchanged production output. Shared core cookie parsing and admin JSON headers remove duplication and response drift, while adapter regressions and documentation pin the external contract. + +**Tech Stack:** Rust 2024, `http`, `serde`, `serde_json`, `error-stack`, EdgeZero adapter routers, Fastly/Viceroy, Markdown/VitePress. + +**Design spec:** `docs/superpowers/specs/2026-08-19-pr928-comprehensive-review-resolution-design.md` + +--- + +## File Map + +- Modify `crates/trusted-server-adapter-fastly/src/app.rs`: early EC diagnostic dispatch and Fastly authentication/finalization regressions. +- Modify `crates/trusted-server-core/src/settings.rs`: dynamic-route authentication probe corpus and settings regression. +- Modify `crates/trusted-server-core/src/ec/prebid_eids.rs`: source-preserving diagnostic parse view and shared UID-validity rule. +- Modify `crates/trusted-server-core/src/ec/admin.rs`: reason-tagged EID drops, shared cookie helper use, `nosniff`, and unit tests. +- Modify `crates/trusted-server-core/src/cookies.rs`: generic request-cookie extraction helper and focused tests. +- Modify `crates/trusted-server-core/src/auction/endpoints.rs`: use the shared cookie helper. +- Modify `crates/trusted-server-adapter-axum/tests/routes.rs`: unauthenticated diagnostic regression. +- Modify `crates/trusted-server-adapter-cloudflare/tests/routes.rs`: unauthenticated diagnostic regression. +- Modify `crates/trusted-server-adapter-spin/tests/routes.rs`: unauthenticated diagnostic regression. +- Modify `docs/guide/api-reference.md`: accurate authentication response contract and reason-tagged preview schema. +- Modify `CHANGELOG.md`: Unreleased configuration compatibility warning. + +No dependency, public configuration schema, or test-only production seam is added. + +### Task 1: Keep Fastly EC diagnostics outside the mutating lifecycle + +**Files:** + +- Modify: `crates/trusted-server-adapter-fastly/src/app.rs:527-610` +- Test: `crates/trusted-server-adapter-fastly/src/app.rs:2200-2250` + +- [ ] **Step 1: Add the failing cookie-bearing EC diagnostic regression** + +Add `admin_ec_diagnostic_skips_ec_finalization` next to the EIDs equivalent. +Build an authenticated `GET /_ts/admin/ec/{valid_id}` request carrying +`ts-ec`, base64-encoded `ts-eids`, and `sharedId`, plus browser-shaped +`DeviceSignals`. Assert the response has no `EcFinalizeState` and no +`Set-Cookie` header. + +- [ ] **Step 2: Run the focused test and verify RED** + +```bash +cargo test-fastly admin_ec_diagnostic_skips_ec_finalization +``` + +Expected: FAIL because the current response carries `EcFinalizeState`. + +- [ ] **Step 3: Early-dispatch both diagnostic handlers** + +Change the current `AdminEidsLookup` early branch to match both variants: + +```rust +if matches!( + handler, + NamedRouteHandler::AdminEcLookup | NamedRouteHandler::AdminEidsLookup +) { + let response = PartnerRegistry::from_config(&state.settings.ec.partners) + .and_then(|registry| match handler { + NamedRouteHandler::AdminEcLookup => { + let kv = crate::maybe_identity_graph(&state.settings); + handle_admin_ec_lookup(kv.as_ref(), ®istry, &req) + } + NamedRouteHandler::AdminEidsLookup => handle_admin_eids_lookup(®istry, &req), + _ => unreachable!("admin diagnostics should match an early-dispatch handler"), + }) + .unwrap_or_else(|error| http_error(&error)); + return Ok(response); +} +``` + +Replace the later `AdminEcLookup` implementation arm with `unreachable!`, like +the EIDs arm. Update comments to describe both diagnostics. + +- [ ] **Step 4: Run both finalization regressions and verify GREEN** + +```bash +cargo test-fastly diagnostic_skips_ec_finalization +``` + +Expected: both EC and EIDs tests PASS. + +- [ ] **Step 5: Commit Task 1** + +```bash +git add crates/trusted-server-adapter-fastly/src/app.rs +git commit -m "Keep Fastly admin EC lookups read only" +``` + +### Task 2: Validate dynamic admin authentication with a suffix corpus + +**Files:** + +- Modify: `crates/trusted-server-core/src/settings.rs:2200-2300` +- Test: `crates/trusted-server-core/src/settings.rs:4960-5035` + +- [ ] **Step 1: Add a failing mixed-case coverage regression** + +Add `from_toml_rejects_lowercase_only_dynamic_admin_ec_auth_coverage`. Configure +the static admin routes with strong credentials and the parameterized route +with: + +```toml +[[handlers]] +path = "^/_ts/admin/ec/[a-f0-9]{64}[.][a-z0-9]{6}$" +username = "admin" +password = "strong-test-password" +``` + +Assert `Settings::from_toml` returns a configuration error naming +`/_ts/admin/ec/{id}`. + +- [ ] **Step 2: Run the regression and verify RED** + +```bash +cargo test-fastly from_toml_rejects_lowercase_only_dynamic_admin_ec_auth_coverage +``` + +Expected: FAIL because `.abc123` is the only probe and the settings load. + +- [ ] **Step 3: Replace the single probe with a fixed corpus** + +Define lowercase and mixed-case concrete paths, and make the mapping return a +slice: + +```rust +const ADMIN_EC_ID_AUTH_PROBES: &[&str] = &[ + concat!("/_ts/admin/ec/", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", ".abc123"), + concat!("/_ts/admin/ec/", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", ".Ab12Z9"), +]; + +fn admin_auth_probes(path: &'static str) -> &'static [&'static str] { + match path { + "/_ts/admin/ec/{id}" => Self::ADMIN_EC_ID_AUTH_PROBES, + path => core::slice::from_ref(&path), + } +} +``` + +If `core::slice::from_ref` cannot produce the required static lifetime for the +match binding, use static one-element probe arrays for the non-parameterized +routes rather than allocating. + +In `uncovered_admin_endpoints`, require every probe to match at least one +handler. In `validate_admin_handler_passwords`, classify a handler as admin +when it matches any probe for any canonical endpoint. Preserve canonical +template reporting. + +- [ ] **Step 4: Run the focused and existing coverage tests** + +```bash +cargo test-fastly dynamic_admin_ec_auth_coverage +cargo test-fastly literal_parameter_template_auth_coverage +cargo test-fastly placeholder_password_for_concrete_admin_ec_handler +cargo test-fastly uncovered_admin_endpoints +``` + +Expected: PASS. + +- [ ] **Step 5: Commit Task 2** + +```bash +git add crates/trusted-server-core/src/settings.rs +git commit -m "Validate mixed-case admin EC auth coverage" +``` + +### Task 3: Report reason-tagged EID ingestion drops + +**Files:** + +- Modify: `crates/trusted-server-core/src/ec/prebid_eids.rs:25-110,178-230,275-340` +- Modify: `crates/trusted-server-core/src/ec/admin.rs:410-515` +- Test: `crates/trusted-server-core/src/ec/prebid_eids.rs:400-620` +- Test: `crates/trusted-server-core/src/ec/admin.rs:1075-1185` + +- [ ] **Step 1: Add failing admin preview regressions** + +Update the existing unmatched assertion to expect: + +```json +{ "source": "unknown.example", "reason": "no_partner" } +``` + +Add a test whose configured source has only whitespace/empty and over-limit +UID candidates; expect one `no_valid_uid` drop. Add a duplicate-source test +where one entry has no valid UID and a later entry has a valid UID; expect the +source in `matched` and no drop. + +- [ ] **Step 2: Run the focused tests and verify RED** + +```bash +cargo test-fastly eids_lookup_ +``` + +Expected: existing string-shaped unmatched output fails and invalid-only +sources are absent. + +- [ ] **Step 3: Refactor cookie decoding without changing production output** + +In `prebid_eids.rs`, introduce a private decoded wire enum holding +`Vec` or `Vec`. Move size/base64/JSON +selection into one decoder. Keep `parse_prebid_eids_cookie` public and map the +decoded wire representation through the existing conversion functions so all +current parser tests remain unchanged. + +Add a crate-visible `PrebidEidAnalysis` and analysis function that decode once +and derive all data the admin handler needs: the filtered `Vec`, retained +diagnostic sources with raw UID strings, and `Vec` selected by +the live partner/UID rules. Make `collect_prebid_eid_updates` call the same +analysis function and extract only `updates`. Add or expose a crate-visible +predicate implementing the existing live rule: + +```rust +pub(crate) fn is_valid_eid_uid(uid: &str) -> bool { + !uid.trim().is_empty() && !eid_id_exceeds_size_limit(uid) +} +``` + +Make `first_valid_uid` call this predicate. + +- [ ] **Step 4: Implement deterministic drop classification** + +In `admin.rs`, replace `Vec` with: + +```rust +#[derive(Debug, Serialize)] +struct DroppedEidSource { + source: String, + reason: DroppedEidReason, +} + +#[derive(Debug, Serialize)] +#[serde(rename_all = "snake_case")] +enum DroppedEidReason { + NoPartner, + NoValidUid, +} +``` + +Group diagnostic sources in a `BTreeMap` for stable output. Emit one +`NoPartner` for an unconfigured source. For a configured source, emit one +`NoValidUid` only when no entry contains a UID satisfying +`is_valid_eid_uid`. Do not let `sharedId` suppress a `ts-eids` drop. Preserve +the existing matched-update collection and deduplication. + +Replace the admin handler's separate `parse_prebid_eids_cookie` and +`collect_prebid_eid_updates` calls with one `analyze_prebid_eids_cookie` call. +On success, move its filtered EIDs into the response, classify its diagnostic +sources, and extend the matched-update list from its updates. On failure, set +the existing `parse_error` and produce no EIDs, drops, or Prebid updates. + +- [ ] **Step 5: Run parser, preview, and ingestion tests** + +```bash +cargo test-fastly prebid_eids +cargo test-fastly eids_lookup_ +``` + +Expected: PASS, including unchanged live-ingestion cases. + +- [ ] **Step 6: Commit Task 3** + +```bash +git add crates/trusted-server-core/src/ec/prebid_eids.rs crates/trusted-server-core/src/ec/admin.rs +git commit -m "Explain dropped admin EID preview sources" +``` + +### Task 4: Consolidate core request-cookie extraction + +**Files:** + +- Modify: `crates/trusted-server-core/src/cookies.rs:1-120` +- Modify: `crates/trusted-server-core/src/ec/admin.rs:20-45,225-245,445-525` +- Modify: `crates/trusted-server-core/src/auction/endpoints.rs:1-35,240-255,400-430` +- Test: `crates/trusted-server-core/src/cookies.rs` + +- [ ] **Step 1: Add focused helper tests** + +Add tests for missing header, whitespace trimming, a value containing `=`, and +multiple cookie pairs. Add a request with two Cookie header values where the +selected `headers().get` value is invalid UTF-8 and assert `None`, pinning the +old helper semantics rather than scanning later values. + +- [ ] **Step 2: Run the focused test and verify RED** + +```bash +cargo test-fastly extract_cookie_value +``` + +Expected: compilation FAIL because the shared helper does not exist. + +- [ ] **Step 3: Add the generic shared helper** + +Add a documented crate-public function in `cookies.rs`: + +```rust +#[must_use] +pub fn extract_cookie_value(req: &Request, name: &str) -> Option { + let cookie_header = req.headers().get(header::COOKIE)?.to_str().ok()?; + cookie_header.split(';').find_map(|pair| { + let (key, value) = pair.trim().split_once('=')?; + (key.trim() == name).then(|| value.trim().to_owned()) + }) +} +``` + +- [ ] **Step 4: Migrate both core callers and delete local copies** + +Import `crate::cookies::extract_cookie_value` in `ec/admin.rs` and +`auction/endpoints.rs`. Remove their byte-identical local helpers and any +imports made unused. + +- [ ] **Step 5: Run focused caller tests** + +```bash +cargo test-fastly extract_cookie_value +cargo test-fastly eids_lookup_ +cargo test-fastly auction +``` + +Expected: PASS. + +- [ ] **Step 6: Commit Task 4** + +```bash +git add crates/trusted-server-core/src/cookies.rs crates/trusted-server-core/src/ec/admin.rs crates/trusted-server-core/src/auction/endpoints.rs +git commit -m "Share core request cookie extraction" +``` + +### Task 5: Harden diagnostic JSON and pin adapter authentication + +**Files:** + +- Modify: `crates/trusted-server-core/src/ec/admin.rs:90-130,530-550` +- Modify: `crates/trusted-server-adapter-fastly/src/app.rs:1800-1875` +- Modify: `crates/trusted-server-adapter-axum/tests/routes.rs:245-325` +- Modify: `crates/trusted-server-adapter-cloudflare/tests/routes.rs:275-330` +- Modify: `crates/trusted-server-adapter-spin/tests/routes.rs:105-165` + +- [ ] **Step 1: Add failing `nosniff` response assertions** + +In core admin tests, assert representative success and JSON error responses +contain `X-Content-Type-Options: nosniff`. + +- [ ] **Step 2: Run the focused test and verify RED** + +```bash +cargo test-fastly admin_ec_lookup +``` + +Expected: FAIL because the header is absent. + +- [ ] **Step 3: Add the shared response header and correct field docs** + +Add `.header(header::X_CONTENT_TYPE_OPTIONS, "nosniff")` to +`json_response`. Update `tombstone` documentation to say it is absent when the +body fails JSON parsing or typed `KvEntry` deserialization. + +- [ ] **Step 4: Add one new-route unauthenticated test per adapter** + +For Fastly, Axum, Cloudflare, and Spin, send `GET /_ts/admin/ec` without an +Authorization header. Assert `401 Unauthorized` and the existing Basic +`WWW-Authenticate` realm. Place each test next to the adapter's authenticated +EC diagnostic test and reuse its established router/service helper. + +- [ ] **Step 5: Run each adapter's focused authentication test** + +```bash +cargo test-fastly admin_ec_route_without_credentials +cargo test-axum admin_ec_route_without_credentials +cargo test-cloudflare admin_ec_route_without_credentials +cargo test-spin admin_ec_route_without_credentials +``` + +Expected: PASS. + +- [ ] **Step 6: Commit Task 5** + +```bash +git add crates/trusted-server-core/src/ec/admin.rs crates/trusted-server-adapter-fastly/src/app.rs crates/trusted-server-adapter-axum/tests/routes.rs crates/trusted-server-adapter-cloudflare/tests/routes.rs crates/trusted-server-adapter-spin/tests/routes.rs +git commit -m "Harden admin diagnostic responses" +``` + +### Task 6: Correct operator-facing documentation + +**Files:** + +- Modify: `docs/guide/api-reference.md:580-640` +- Modify: `CHANGELOG.md:8-25` + +- [ ] **Step 1: Qualify authentication response behavior** + +State that successfully authenticated diagnostic-handler responses are JSON +with `Cache-Control: no-store`, while missing or invalid credentials receive +the shared plaintext `401 Unauthorized` Basic challenge. + +- [ ] **Step 2: Document reason-tagged EID drops** + +Describe `ingest.unmatched` entries as `{source, reason}` objects and define +`no_partner` and `no_valid_uid`. Include a compact example covering one match +and one drop. + +- [ ] **Step 3: Add the Unreleased compatibility warning** + +Under `CHANGELOG.md`'s Unreleased Changed section, state that startup now +requires authenticated handler coverage for the EC/EID diagnostics in addition +to key management. Tell operators with narrow key-only patterns to broaden +coverage before deploying, preferably to `^/_ts/admin(?:/|$)`. + +- [ ] **Step 4: Format documentation** + +```bash +cd docs && npm run format +``` + +Expected: formatter exits 0 with only intended Markdown changes. + +- [ ] **Step 5: Commit Task 6** + +```bash +git add docs/guide/api-reference.md CHANGELOG.md +git commit -m "Clarify admin diagnostics contracts" +``` + +### Task 7: Full verification + +**Files:** + +- Verify all modified files. + +- [ ] **Step 1: Check formatting** + +```bash +cargo fmt --all -- --check +``` + +Expected: PASS. + +- [ ] **Step 2: Run adapter and core test suites** + +```bash +cargo test-fastly +cargo test-axum +cargo test-cloudflare +cargo test-spin +``` + +Expected: all PASS. + +- [ ] **Step 3: Run target-matched clippy checks** + +```bash +cargo clippy-fastly +cargo clippy-axum +cargo clippy-cloudflare +cargo clippy-cloudflare-wasm +cargo clippy-spin-native +cargo clippy-spin-wasm +``` + +Expected: all PASS with warnings denied by repository aliases. + +- [ ] **Step 4: Verify documentation and diff hygiene** + +```bash +cd docs && npm run format +git diff --check +git status --short +``` + +Expected: formatter and diff check PASS; status shows only intentional plan or +implementation state. + +- [ ] **Step 5: Review the branch diff against the PR base** + +```bash +git diff --stat origin/main...HEAD +git diff origin/main...HEAD -- crates/trusted-server-adapter-fastly/src/app.rs crates/trusted-server-core/src/settings.rs crates/trusted-server-core/src/ec/admin.rs crates/trusted-server-core/src/ec/prebid_eids.rs crates/trusted-server-core/src/cookies.rs crates/trusted-server-core/src/auction/endpoints.rs docs/guide/api-reference.md CHANGELOG.md +``` + +Expected: every change maps to the approved spec; no unrelated refactor or +behavior change is present. diff --git a/docs/superpowers/specs/2026-08-18-admin-diagnostics-review-fixes-design.md b/docs/superpowers/specs/2026-08-18-admin-diagnostics-review-fixes-design.md new file mode 100644 index 000000000..b77b5732b --- /dev/null +++ b/docs/superpowers/specs/2026-08-18-admin-diagnostics-review-fixes-design.md @@ -0,0 +1,275 @@ +# Admin diagnostics review fixes + +**PR:** #928 +**Date:** 2026-08-18 +**Status:** Approved design + +## Problem + +PR #928 adds authenticated operator diagnostics at `/_ts/admin/ec`, +`/_ts/admin/ec/{id}`, and `/_ts/admin/eids`. Review identified five issues: + +1. Startup authentication coverage checks the literal router template + `/_ts/admin/ec/{id}`, while runtime authentication checks concrete request + paths. A handler that matches only the literal braces can therefore pass + startup validation while leaving real EC IDs unauthenticated. +2. Non-GET requests and malformed or trailing diagnostic paths enter the + publisher fallback after successful authentication. That can forward the + admin `Authorization` header and request body to the publisher origin. +3. Fastly dispatches the EIDs diagnostic through normal EC setup and attaches + `EcFinalizeState`. Entry-point finalization can then ingest EID cookies and + write to KV even though the diagnostic is documented as read-only. +4. Parseable KV bodies and metadata are displayed by serializing typed schema + values. Unknown fields are dropped, and legacy representations such as + map-shaped `seen_domains` are normalized instead of being shown as stored. +5. The operator-facing API is missing from the API reference. + +## Goals + +- Require valid Basic authentication for every recognized admin request at + runtime, including concrete EC IDs. +- Reject invalid admin handler coverage during startup using a concrete EC ID + probe while preserving router-template diagnostics in error messages. +- Ensure diagnostic requests never enter publisher fallback, regardless of + supported method or malformed/trailing path shape. +- Keep `GET /_ts/admin/eids` read-only on Fastly by preventing all EC + finalization state from being attached. +- Display all parseable KV entry and metadata JSON without dropping or + normalizing stored fields. +- Preserve typed validation and auction derivation independently of the raw + display representation. +- Document authentication, requests, responses, status codes, cache policy, + and adapter limitations. +- Keep changes narrowly scoped to the new admin diagnostics. + +## Non-goals + +- Do not change authentication behavior for non-admin handler patterns. +- Do not change publisher fallback behavior outside the reserved admin + namespace. +- Do not add EC lookup support to Axum, Cloudflare, or Spin. +- Do not change live auction EID resolution or cookie-ingestion semantics. +- Do not add a new KV abstraction solely to spy on Fastly writes in tests. +- Do not redesign the admin API payload beyond preserving stored JSON and + documenting its existing derived fields. +- Do not push commits, reply to GitHub review threads, or resolve review + conversations as part of implementation. + +## Design + +### 1. Parameter-aware startup authentication coverage + +Keep the canonical admin route templates as the source used for coverage +errors and route-consistency tests. Define one canonical mapping from each +template to its concrete authentication probe. When testing whether a +configured handler covers `/_ts/admin/ec/{id}`, match the handler against a +fixed representative valid EC path instead of the literal template. The +representative ID will use fictional test data and satisfy the production +`{64hex}.{6alnum}` format. + +All non-parameterized admin routes continue to use their canonical paths as +their coverage probes. A prefix handler such as `^/_ts/admin` therefore remains +valid, while a regex matching only literal braces is rejected at startup and +reported as failing to cover `/_ts/admin/ec/{id}`. + +Use the same template-to-probe mapping everywhere settings validation decides +whether a handler protects an admin endpoint. This includes both uncovered +endpoint detection and placeholder-password rejection. A handler that protects +concrete EC IDs must therefore be recognized as an admin handler for credential +strength validation even when it does not match the literal router template. + +### 2. Runtime authentication fails closed for admin paths + +`enforce_basic_auth` currently treats a missing matching handler as meaning the +request is public. Add a shared admin-namespace classifier with a segment +boundary: it recognizes `/_ts/admin` and paths beginning `/_ts/admin/`, but not +similar publisher paths such as `/_ts/administrator`. + +If no handler matches a recognized admin path, return a configuration error +instead of `Ok(None)`. Existing adapter middleware converts that error into a +local server response, so the request cannot reach a route handler or publisher +origin. If a handler matches, credential extraction and constant-time +comparison remain unchanged. + +This runtime invariant is defense in depth. Startup validation catches known +route misconfiguration, while runtime classification also protects malformed, +trailing, and future admin paths. + +### 3. Local denial before publisher fallback + +Add a shared core classifier for the EC/EIDs diagnostic path family and use it +at the top of each adapter's fallback dispatcher, before integration or +publisher handling. This avoids a repetitive route matrix and covers path +shapes that a fixed route table can miss. + +For the seven methods supported by publisher fallback (`GET`, `POST`, `HEAD`, +`OPTIONS`, `PUT`, `PATCH`, and `DELETE`), apply this contract after successful +authentication: + +- A non-GET request to `/_ts/admin/ec`, a single-segment + `/_ts/admin/ec/{id}`, or `/_ts/admin/eids` returns local `405 Method Not +Allowed` with `Allow: GET`. +- A path with a trailing slash, an extra segment, a missing segment structure, + or an EIDs suffix returns local `404 Not Found` and never reaches the + publisher. +- Existing GET handling remains unchanged: Fastly validates an explicit EC ID + and can return `400`, while portability adapters return their existing local + `501` for the two EC lookup forms. + +All denial responses use `Cache-Control: no-store`. Unsupported methods such as +`TRACE` continue to receive the router's local `405`; they are not registered +for publisher fallback and therefore cannot leak credentials upstream. + +The guard is duplicated only at the four adapter fallback entry points. Path +classification and response construction remain shared so status, headers, +and behavior cannot drift. + +### 4. Fastly EIDs dispatch skips EC setup and finalization + +Handle `NamedRouteHandler::AdminEidsLookup` in `execute_named` before request +filters and `build_ec_request_state`, next to the existing early batch-sync +branch. Build only the partner registry, call `handle_admin_eids_lookup`, map +errors through the existing HTTP error conversion, and return the response +without calling `attach_dispatch_extensions`. + +Basic authentication and standard response-header middleware remain outside +this dispatch function and continue to run. Because the returned response has +no `EcFinalizeState`, the Fastly entry point cannot call EC finalization, +`ingest_eid_cookies`, pull sync, or any EC KV write for this endpoint. + +The normal `run_named_route` EIDs arm will become unreachable or be removed in +the smallest form that keeps the enum dispatch exhaustive and clear. + +### 5. Separate raw display parsing from typed interpretation + +For entry bodies, perform two independent parses: + +1. Parse `lookup.body` as `serde_json::Value` for `payload.entry`. +2. Parse the same bytes as `KvEntry` only for tombstone calculation, schema + validation, and auction derivation. + +Add derived `created_iso` and `consent.updated_iso` fields directly to the raw +JSON object using its stored numeric timestamps. Never overwrite a stored field +with the same derived-field name. Unknown fields and legacy nested shapes stay +unchanged. + +The entry outcomes are: + +- Invalid JSON: omit `entry`; include `entry_error` and lossy UTF-8 `raw_body`; + omit tombstone and auction. +- Valid JSON but invalid `KvEntry`: include the raw `entry`; include + `entry_error`; omit tombstone and auction. +- Valid typed entry that fails validation: include raw `entry` and tombstone; + include the validation error; omit auction. +- Valid and validated typed entry: include raw `entry`, tombstone, and the + existing derived auction view. + +For metadata, parse bytes as `serde_json::Value` for display and independently +as `KvMetadata` for existing schema diagnostics. Parseable raw metadata remains +visible even if typed metadata parsing reports an error. Invalid JSON remains +omitted and its raw/error detail stays in `metadata_error`. + +Auction derivation continues to use `KvEntry`, `resolve_partner_ids`, and +`to_eids`, preserving production semantics. Live request consent remains a +documented limitation of the diagnostic view. + +### 6. Operator API documentation + +Add an Admin Diagnostic Endpoints section to +`docs/guide/api-reference.md` covering: + +- Basic authentication and the sensitivity of returned data. +- `GET /_ts/admin/ec` with ID resolution from the `ts-ec` cookie. +- `GET /_ts/admin/ec/{id}` and its explicit ID format. +- EC success fields, raw/typed error outcomes, auction derivation, `400`, + `401`, `404`, `405`, and `501` responses. +- Fastly-only EC lookup support and authenticated `501` responses from Axum, + Cloudflare, and Spin. +- `GET /_ts/admin/eids`, its cookie inputs, always-`200` diagnostic payload, + and support on every adapter. +- `Content-Type` and `Cache-Control: no-store` behavior. +- The three diagnostic paths in the protected-endpoint list. + +Examples use only reserved domains and fictional IDs. + +## Testing strategy + +Implementation follows red-green-refactor, one behavior at a time. + +### Core authentication and settings + +- A template-only handler configuration fails startup coverage for the + parameterized EC route. +- A concrete-ID handler with a placeholder password is still rejected as an + admin handler through the shared template-to-probe mapping. +- A concrete valid EC request under that configuration cannot be treated as + public by runtime auth. +- Existing broad and exact non-parameterized handler coverage remains valid. +- Similar non-admin prefixes remain public unless configured otherwise. + +### Cross-adapter routing + +For Fastly, Axum, Cloudflare, and Spin, authenticated requests verify: + +- Wrong supported methods on the bare EC, single-ID EC, and EIDs routes return + local `405` with `Allow: GET` and `no-store`. +- Trailing and extra-segment EC/EIDs paths return local `404` with `no-store`. +- Marker bodies and credentials do not reach publisher handling; deterministic + local status provides the existing adapter test seam for this invariant. +- Valid GET behavior remains `200`/domain error on Fastly, `501` for EC lookup + on portability adapters, and `200` for EIDs on every adapter. + +### Fastly read-only behavior + +An authenticated browser-shaped EIDs request carrying valid EC, EID, and +shared-ID cookies returns `200` without an `EcFinalizeState` response extension. +The Fastly entry point only performs EC writes when that extension is present, +so its absence proves the diagnostic cannot invoke the KV write path without +introducing test-only production seams. + +### Raw KV display + +- A parseable entry with unknown top-level and nested fields preserves those + exact values. +- Legacy map-shaped `seen_domains` remains a map with its nested history data. +- Parseable metadata preserves unknown fields. +- Stored timestamps remain unchanged and ISO companions are added without + overwriting stored collisions. +- Typed parsing still produces the expected auction view from the same entry. +- Valid JSON with an invalid typed schema remains visible with `entry_error`. + +### Verification + +After targeted tests pass, run the repository-required checks relevant to all +touched crates and documentation: + +- `cargo fmt --all -- --check` +- `cargo test-fastly` +- `cargo test-axum` +- `cargo test-cloudflare` +- `cargo test-spin` +- `cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity` +- `cargo clippy-fastly` +- `cargo clippy-axum` +- `cargo clippy-cloudflare` +- `cargo clippy-cloudflare-wasm` +- `cargo clippy-spin-native` +- `cargo clippy-spin-wasm` +- `cd docs && npm run format` + +JS sources are untouched; JS build, test, and format gates are not required for +the implementation-specific verification unless another change introduces a +JS dependency. + +## Risks and mitigations + +- **Overbroad runtime auth classification:** use an exact namespace segment + boundary and add a similar-prefix regression. +- **Adapter behavior drift:** share path classification and denial response + construction; keep only the fallback entry-point call adapter-local. +- **Route precedence regressions:** preserve named GET handlers and guard only + requests that reached fallback. +- **Accidental raw-data normalization:** assert unknown fields and legacy + structures on the final serialized handler response, not only helper values. +- **Fastly write regression:** assert the structural finalization gate is absent + from the response. diff --git a/docs/superpowers/specs/2026-08-19-pr928-comprehensive-review-resolution-design.md b/docs/superpowers/specs/2026-08-19-pr928-comprehensive-review-resolution-design.md new file mode 100644 index 000000000..7a1363e99 --- /dev/null +++ b/docs/superpowers/specs/2026-08-19-pr928-comprehensive-review-resolution-design.md @@ -0,0 +1,237 @@ +# PR #928 Comprehensive Review Resolution + +**PR:** #928 +**Date:** 2026-08-19 +**Status:** Approved design + +## Problem + +PR #928 adds authenticated EC and EID diagnostic endpoints. Follow-up review +found three blocking correctness issues and six related quality gaps: + +1. Fastly's EC lookup still enters the mutating EC request/finalization + lifecycle, so a diagnostic GET can ingest browser cookies, mint identity + state, write a withdrawal tombstone, or trigger pull sync. +2. Startup authentication coverage checks only one lowercase-suffix EC ID. A + narrower handler regex can pass startup while valid mixed-case IDs fail + closed at runtime. +3. The API guide describes every response as JSON with `no-store`, although + the shared Basic-auth rejection is plaintext and has no cache header. +4. The EID preview omits configured sources whose UID list contains no value + accepted by the real ingestion path. +5. Core contains duplicate request-cookie extraction helpers. +6. Admin JSON responses lack `X-Content-Type-Options: nosniff`. +7. The tombstone field documentation omits typed deserialization failures. +8. New diagnostic routes lack explicit unauthenticated adapter regressions. +9. Operators are not warned that narrow pre-existing admin handler patterns + must expand to cover the new routes. + +## Goals + +- Make both Fastly diagnostic GET handlers read-only by construction. +- Detect common under-coverage of the full valid EC ID suffix alphabet during + settings finalization while retaining runtime fail-closed protection. +- Make the documented response contract accurately distinguish authentication + failures from successfully authenticated diagnostic-handler responses. +- Report every parsed EID source that ingestion drops, with an operator-useful + reason. +- Consolidate identical core cookie-header parsing without changing semantics. +- Apply browser-safe response headers consistently to admin diagnostic JSON. +- Pin the new routes' authentication behavior on every adapter. +- Document the intentional configuration compatibility impact. + +## Non-goals + +- Do not change Basic-auth middleware response bodies or headers. That shared + behavior predates the diagnostic endpoints and is outside this PR's scope. +- Do not attempt formal regex-language inclusion. Arbitrary configured regexes + make exhaustive proof impractical; runtime authentication remains the final + fail-closed invariant. +- Do not change live EID ingestion, partner matching, UID validation, or + deduplication behavior. +- Do not add EC lookup support to Axum, Cloudflare, or Spin. +- Do not introduce a test-only Fastly KV abstraction solely to spy on writes. +- Do not alter unrelated response builders or cookie parsing behavior. +- Do not push the branch, reply to GitHub threads, or resolve review + conversations as part of implementation. + +## Design + +### 1. Read-only Fastly diagnostic dispatch + +Move `AdminEcLookup` into the same `execute_named` early-dispatch branch as +`AdminEidsLookup`, before GPT diagnostic preparation, EC request-state +construction, and request filters. Construct the partner registry once, match +the requested diagnostic handler, and for EC lookup construct its KV identity +graph directly from settings, as the existing handler already does. Return the +handler response immediately without `attach_dispatch_extensions`. + +Keep exhaustive `run_named_route` arms for both diagnostic variants as +`unreachable!`, documenting that they must be handled before EC setup. Basic +authentication and normal outer response middleware continue to run because +they wrap `execute_named`. + +The regression uses an authenticated, browser-shaped EC request containing +`ts-ec`, `ts-eids`, and `sharedId` cookies plus device signals. It asserts a +successful diagnostic response has neither `EcFinalizeState` nor EC cookie +mutation headers. The Fastly entry point invokes EC finalization and all of its +KV writes only when `EcFinalizeState` is present, so absence tests the +production write gate without adding a test-only storage seam. Existing core +lookup tests continue to establish that the handler performs reads only. + +### 2. Dynamic authentication probe corpus + +Retain `Settings::ADMIN_ENDPOINTS` as the canonical operator-facing route list, +but map `/_ts/admin/ec/{id}` to a small fixed corpus of concrete valid IDs. The +corpus contains at least: + +- a lowercase-and-digit suffix such as `.abc123`; and +- a mixed-case-and-digit suffix such as `.Ab12Z9`. + +All probes use a valid 64-character lowercase hexadecimal hash. An endpoint is +covered only when every probe has a matching configured handler. Different +handlers may collectively cover the corpus because every matched handler still +requires Basic authentication. Placeholder-password validation classifies a +handler as protecting the dynamic admin route when it matches any probe, so a +narrow handler cannot escape credential-strength checks. + +Add a startup regression in which a lowercase-only suffix regex covers the +first probe but not the mixed-case probe. The configuration must be rejected +and continue to report the canonical `/_ts/admin/ec/{id}` template. Existing +runtime fail-closed authentication remains unchanged and protects valid IDs +outside the representative corpus. + +### 3. Accurate API and upgrade documentation + +Change the Admin Diagnostic Endpoints introduction to say that responses +produced after successful authentication are JSON with +`Cache-Control: no-store`. Explicitly note that missing or invalid credentials +use the shared plaintext `401 Unauthorized` challenge contract. + +Document the reason-tagged EID drop objects described below. Add an Unreleased +changelog entry stating that configurations which protected only the older key +management routes now fail startup and must broaden their authenticated handler +coverage to all `/_ts/admin` diagnostic routes. Recommend a namespace-wide +pattern such as `^/_ts/admin(?:/|$)` while preserving the existing warning +against accidentally protecting non-admin browser endpoints. + +### 4. Reason-tagged EID ingestion drops + +Replace `ingest.unmatched: string[]` with a list of objects containing: + +- `source`: the EID source string; and +- `reason`: `no_partner` or `no_valid_uid`. + +The production OpenRTB conversion intentionally removes structured entries +whose UID list becomes empty, so it cannot be the only diagnostic parse. Decode +the cookie once into the existing legacy-or-structured wire representation, +then return one shared analysis result with three derived views, without +changing live behavior: + +- the existing filtered `Vec` used by ingestion and returned in `eids`; +- a private diagnostic source view that retains non-empty source names even + when all supplied UIDs are empty or otherwise unusable; and +- the partner updates selected by the same lookup and first-valid-UID rules as + live ingestion. + +Classify retained `ts-eids` sources using the same partner lookup and valid-UID +predicate as live ingestion: + +- no configured partner for the source becomes `no_partner`; +- a configured partner with no non-empty UID within the ingestion size limit + becomes `no_valid_uid`; +- a configured partner with a valid UID is represented by the existing + deduplicated `matched` output and is not dropped. + +Group duplicate cookie entries by source for drop reporting. If any entry for +a configured source has a valid UID, emit no `no_valid_uid` drop for that +source; otherwise emit exactly one. An unconfigured source emits exactly one +`no_partner` drop. `sharedId` does not suppress a `ts-eids` drop because the +preview is explaining that source's own cookie input. + +Malformed `ts-eids` remains represented by `parse_error`, because there is no +parsed source to classify. `sharedId` behavior remains unchanged. This response +schema is safe to establish now because the endpoint is new in this unmerged +PR; the API guide and tests change in the same commit. + +Refactor decoding behind private helpers in the existing Prebid EID ingestion +module so the public production parser retains identical output. Add one +crate-visible analysis function used by the admin handler; make the production +update collector reuse the same analysis and extract only its updates. Expose +the UID-validity predicate within the crate as needed. This keeps one decode per +caller and prevents preview classification and live ingestion from drifting. + +### 5. Shared core cookie extraction + +Add a generic request-cookie value helper to the existing core `cookies` +module. It accepts `&http::Request` so both current core call sites can use +it regardless of body type, and preserves the existing behavior exactly: +inspect only the value selected by `headers().get(COOKIE)`; return `None` when +that selected value is absent or invalid UTF-8; otherwise split +semicolon-delimited pairs, trim whitespace, split only on the first `=`, and +return an owned value for the requested name. It does not scan later repeated +Cookie header values. + +Use it from `ec/admin.rs` and `auction/endpoints.rs`, deleting both local +copies. The Fastly adapter's separate helper operates on Fastly's platform +request type and remains local; forcing it through an incompatible abstraction +would expand scope without removing meaningful duplication. + +### 6. Diagnostic response hardening and field documentation + +Add `X-Content-Type-Options: nosniff` to the shared admin diagnostic +`json_response` builder. This covers EC lookup, EID preview, unsupported-adapter +responses, and local diagnostic fallback denials. Tests assert the header on +representative success and error responses. + +Update the `tombstone` field comment to state that it is absent when the body +cannot be parsed as JSON or deserialized as the typed `KvEntry` schema. No +runtime behavior changes. + +### 7. Cross-adapter authentication regressions + +Add one unauthenticated `GET /_ts/admin/ec` test to each adapter's established +route-test layer: Fastly, Axum, Cloudflare, and Spin. Each test asserts `401` +and the Basic `WWW-Authenticate` challenge. Keep existing authenticated route +tests unchanged so the pair establishes authentication-before-handler ordering +for both the Fastly implementation and portability adapters' `501` response. + +## Error Handling + +No new public failure mode is introduced. Partner-registry and KV-graph +construction errors continue through the existing `http_error` conversion. +Probe validation continues to return `TrustedServerError::Configuration` and +reports canonical route templates. EID preview classification is infallible +after cookie parsing. Shared cookie extraction intentionally ignores malformed +header encoding exactly as the removed helpers do. + +## Testing Strategy + +Implementation follows red-green-refactor, one review concern at a time: + +1. Add the cookie-bearing Fastly EC lookup regression, observe + `EcFinalizeState`, then early-dispatch the handler and verify its absence. +2. Add the lowercase-only auth-handler regression, observe startup success, + then require the mixed-case probe and verify rejection. +3. Add EID preview cases for `no_partner`, empty UID, and over-limit UID before + implementing the reason-tagged drop type and shared validity helper. +4. Add shared cookie-helper unit coverage, migrate the two core callers, and + run their focused tests. +5. Add `nosniff` assertions and the four adapter unauthenticated regressions. +6. Update API and changelog text and run documentation formatting. +7. Run the affected target suites, all repository-required format checks, and + target-matched clippy commands before claiming completion. + +## Expected Files + +- `crates/trusted-server-adapter-fastly/src/app.rs` +- `crates/trusted-server-adapter-axum/tests/routes.rs` +- `crates/trusted-server-adapter-cloudflare/tests/routes.rs` +- `crates/trusted-server-adapter-spin/tests/routes.rs` +- `crates/trusted-server-core/src/settings.rs` +- `crates/trusted-server-core/src/cookies.rs` +- `crates/trusted-server-core/src/auction/endpoints.rs` +- `crates/trusted-server-core/src/ec/admin.rs` +- `crates/trusted-server-core/src/ec/prebid_eids.rs` +- `docs/guide/api-reference.md` +- `CHANGELOG.md`