Add dual-path EdgeZero entry point with feature flag (PR 14)#628
Add dual-path EdgeZero entry point with feature flag (PR 14)#628prk-Jr wants to merge 217 commits into
Conversation
Rename crates/common → crates/trusted-server-core and crates/fastly → crates/trusted-server-adapter-fastly following the EdgeZero naming convention. Add EdgeZero workspace dependencies pinned to rev 170b74b. Update all references across docs, CI workflows, scripts, agent files, and configuration.
Introduces trusted-server-core::platform with PlatformConfigStore, PlatformSecretStore, PlatformKvStore, PlatformBackend, PlatformHttpClient, and PlatformGeo traits alongside ClientInfo, PlatformError, and RuntimeServices. Wires the Fastly adapter implementations and threads RuntimeServices into route_request. Moves GeoInfo to platform/types as platform-neutral data and adds geo_from_fastly for field mapping.
…o-pr2-platform-traits
- Defer KV store opening: replace early error return with a local UnavailableKvStore fallback so routes that do not need synthetic ID access succeed when the KV store is missing or temporarily unavailable - Use ConfigStore::try_open + try_get and SecretStore::try_get throughout FastlyPlatformConfigStore and FastlyPlatformSecretStore to honour the Result contract instead of panicking on open/lookup failure - Encapsulate RuntimeServices service fields as pub(crate) with public getter methods (config_store, secret_store, backend, http_client, geo) and a pub new() constructor; adapter updated to use new() - Reference #487 in FastlyPlatformHttpClient stub (PR 6 implements it) - Remove unused KvPage re-export from platform/mod.rs - Use super::KvHandle shorthand in RuntimeServices::kv_handle()
- Split fastly_storage.rs into storage/{config_store,secret_store,api_client,mod}.rs
- Add PlatformConfigStore read path via FastlyPlatformConfigStore::get using ConfigStore::try_open/try_get
- Add PlatformError::NotImplemented variant; stub write methods on FastlyPlatformConfigStore and FastlyPlatformSecretStore
- Add StoreName/StoreId newtypes with From<String>, From<&str>, AsRef<str>
- Add UnavailableKvStore to core platform module
- Add RuntimeServicesBuilder replacing 7-arg constructor
- Migrate get_active_jwks and handle_trusted_server_discovery to use &RuntimeServices
- Update call sites in signing.rs, rotation.rs, main.rs
- Add success-path test for handle_trusted_server_discovery using StubJwksConfigStore
- Fix test_parse_cookies_to_jar_empty typo (was emtpy)
- Make StoreName and StoreId inner fields private; From/AsRef provide all needed construction and access - Add #[deprecated] to GeoInfo::from_request with #[allow(deprecated)] at the three legacy call sites to track migration progress - Enumerate the six platform traits in the platform module doc comment - Extract backend_config_from_spec helper to remove duplicate BackendConfig construction in predict_name and ensure - Replace .into_iter().collect() with .to_vec() on secret plaintext bytes - Remove unused bytes dependency from trusted-server-adapter-fastly - Add comment on SecretStore::open clarifying it already returns Result (unlike ConfigStore::open which panics)
Provider transport failures (select() errors) pushed a bare AuctionResponse::error with no metadata, while parse and launch failures attach error_type/message. Route transport failures through a shared helper with a new ERROR_TYPE_TRANSPORT classifier and a static, upstream-safe message, so the public /auction response shape no longer depends on how a provider failed. Add unit coverage for the new helper.
…n-provider-type-migration' into feature/edgezero-pr14-entry-point-dual-path # Conflicts: # crates/trusted-server-adapter-fastly/src/main.rs # crates/trusted-server-core/src/platform/test_support.rs # crates/trusted-server-core/src/proxy.rs
The publisher.max_buffered_body_bytes cap was only enforced by BoundedWriter on the bytes emitted from process_response_streaming. The HTML path with registered post-processors first accumulates every rewritten chunk in HtmlWithPostProcessing::accumulated_output and emits nothing until the final chunk, so a highly compressible document could grow that buffer far past the cap before the writer ever saw it — the exact Wasm-heap OOM the setting is meant to prevent. Thread the configured limit into the accumulator and reject before extending it, and re-check the post-processed output (post-processors may append content). Errors use the same message and propagate through the same process_chunk path that maps to a 5xx proxy error. Add a regression test proving the accumulator itself cannot grow past the cap mid-stream rather than only failing on the final write.
BoundedWriter errors map through TrustedServerError::Proxy, whose status is 502 Bad Gateway, not 500. Operators may build runbooks from this sample, so document the actual 502 proxy-error status.
prk-Jr
changed the base branch from
feature/edgezero-pr13-integration-provider-type-migration
to
main
…-entry-point-dual-path # Conflicts: # crates/trusted-server-adapter-fastly/src/main.rs # crates/trusted-server-core/src/ec/mod.rs # crates/trusted-server-core/src/proxy.rs # crates/trusted-server-core/src/publisher.rs
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Automated review: I reviewed the current PR #628 diff against origin/main, including the EdgeZero entry point, router fallback, EC finalization lifecycle, and current CI/review context. CI is green, and the earlier forwarded-header/router/middleware blockers appear addressed, but I found two high-confidence EdgeZero-path regressions that should be fixed before this feature flag can be safely enabled: configured asset routes are bypassed entirely, and invalid EC partner configuration fails open for fallback responses instead of failing the request as legacy does.
Inline comments contain the actionable findings. Because these are compatibility/correctness regressions on the new entry point, I am submitting REQUEST_CHANGES.
CI observed via gh pr checks 628: all listed checks pass (cargo fmt, cargo test, clippy/Analyze rust, vitest, format-typescript, format-docs, browser/integration tests, CodeQL). Existing reviews already covered forwarded header sanitization, router-level 405/finalize behavior, TLS metadata, parse_edgezero_flag, and buffering; I avoided duplicating those.
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
Re-review at head 34e16098. All three round-1 findings are resolved (the max_buffered_body_bytes default/null contradictions and the geo-401 parity note). New work — the EC identity lifecycle port, the app.router().oneshot() switch to preserve duplicate Set-Cookie headers, and fastly-ssl stripping before origin forwarding — is solid and well-tested. Two items need attention before merge: a shared-path buffering cap that changes legacy behavior while the docs scope it to EdgeZero, and confirmation that the client-IP request context still reaches the router after the dispatch bypass.
Blocking
🔧 wrench
- 16 MiB cap now also constrains the legacy path, but docs say EdgeZero-only: the buffered HTML-post-processed branch of the shared
handle_publisher_requestwent from unboundedVec::new()toBoundedWriter::new(max_buffered_body_bytes), so legacy/default-path post-processed responses over 16 MiB decoded now 500. Reconcile code with thesettings.rs/trusted-server.tomldocs (publisher.rs:699).
❓ question
- Confirm
client_ip(FastlyRequestContext) reaches the router on the EdgeZero path: theinto_core_request+oneshotbypass inserts onlyconfig_store; EC consent geo-gating and middleware geo read the IP viaFastlyRequestContext. Tests useNone-IP requests so they wouldn't catch a missing context (main.rs:243).
Non-blocking
🤔 thinking
- CORS preflight builds full EC state:
OPTIONS /_ts/api/v1/identifyruns the device/geo/KV prelude and attachesEcFinalizeStatebefore routing to the CORS handler; confirm this matches legacy or short-circuit OPTIONS before the prelude (app.rs:337).
CI Status
- browser integration tests: PASS
- integration tests: PASS
- prepare integration artifacts: PASS
- fmt / clippy / unit tests: not surfaced as separate PR checks
Address two EdgeZero-path regressions and one doc mismatch from PR 628 review: - Asset routes: dispatch_fallback now checks settings.asset_route_for_path for GET/HEAD before the publisher fallback and dispatches matches through handle_asset_proxy_request, mirroring legacy route_request. Asset responses skip EC finalization (no EcFinalizeState) and thread AssetProxyCachePolicy out so edgezero_main reapplies protected cache directives after finalization. - EC partner config: build PartnerRegistry in build_state_from_settings so an invalid ec.partners config fails closed at startup (routes() falls back to startup_error_router) instead of serving publisher fallback responses with the partner-registry error only logged at finalize. Per-request rebuilds in identify/auction/batch-sync now reuse the shared registry. - Docs: max_buffered_body_bytes cap applies to any buffered post-processed publisher response on both the legacy and EdgeZero paths; broaden the settings.rs and trusted-server.toml doc strings to match the shared code path. Add regression tests for the asset-route fallback and fail-closed partner config.
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Automated review: I reviewed the current PR #628 diff against origin/main with a focus on EdgeZero/legacy parity, EC lifecycle correctness, asset fallback behavior, and the new buffering setting. I found one blocking EdgeZero correctness regression: device signals are derived after the Fastly request has been converted, so the JA4/H2 signals used by the EC bot gate are lost and real browser requests are treated as non-browser traffic. I also left two non-blocking compatibility/maintainability findings around asset-route buffering and the Rust Default value for the new publisher buffer cap.
CI is currently green in GitHub checks (cargo test, vitest, format jobs, integration tests, and CodeQL). Existing review threads cover many earlier EdgeZero parity issues; these findings are for remaining/current issues I did not see resolved in prior comments.
…-entry-point-dual-path # Conflicts: # crates/trusted-server-adapter-fastly/src/main.rs
The EdgeZero path derived device signals from a synthetic fastly request reconstructed from the EdgeZero HTTP request. That request cannot expose Fastly-only TLS values (`get_tls_ja4`, `get_client_h2_fingerprint`), so the JA4/H2 class the EC bot gate relies on was lost and real browsers were classified as non-browser traffic — silently suppressing EC generation, finalize KV writes, and pull-sync. Derive `DeviceSignals` in `edgezero_main` from the original `FastlyRequest` before `into_core_request` consumes it, store them in the request extensions, and read them back in `build_ec_request_state` (falling back to the reconstructed request when absent, e.g. in tests). Add EdgeZero-path tests proving a browser-shaped request reaches the EC finalize path and that a request without captured signals is treated as non-browser.
The EdgeZero asset-route fallback unconditionally rewrote Content-Length to the buffered byte count whenever the asset origin returned a streaming body. For HEAD responses and bodiless statuses (204, 304) — which advertise the origin representation length while carrying no body — that rewrote a meaningful Content-Length to 0, corrupting the metadata. Only recompute Content-Length and attach the buffered body for body-bearing responses; HEAD/204/304 keep the origin Content-Length untouched. Add a unit test for the body-presence guard. Also document the interim asset buffering behavior: the EdgeZero path buffers asset bodies (legacy streams them uncapped) bounded by the publisher OOM guard. Restoring streaming and deciding whether assets warrant a dedicated cap are deferred to the streaming cutover (issue #495).
Publisher derived Default, so Publisher::default() / Settings::default() set max_buffered_body_bytes to usize's 0 while deserializing TOML without the key applied the intended 16 MiB default. Programmatic defaults are used in tests and helper code, where any buffered post-processing would fail immediately at a zero-byte cap. Replace the derived Default with a manual impl that sources default_max_buffered_body_bytes(), keeping the other fields at their derived defaults. Add a unit test asserting the manual default and the TOML default stay aligned.
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Automated review:
Review Summary
Reviewed PR #628 at b0a57ac against main, focusing on the dual-path EdgeZero entry point, routing/middleware parity, EC/publisher handling, config, and tests. CI is green, and several prior review findings appear addressed, but I found one blocking security/parity regression: the EdgeZero router never runs the integration request-filter pipeline, which bypasses DataDome-style protection/challenge behavior and drops request/response mutations when the flag is enabled.
Findings
P0 / Blockers
- EdgeZero path bypasses integration request filters — see inline comment on
crates/trusted-server-adapter-fastly/src/app.rs.
P1 / High
None.
P2 / Medium
publisher.max_buffered_body_bytes = 0is accepted and can break all buffered publisher responses — see inline comment oncrates/trusted-server-core/src/settings.rs.- Publisher
HEADresponses can be returned withContent-Length: 0after EdgeZero buffering — see inline comment oncrates/trusted-server-adapter-fastly/src/main.rs. - No enabled EdgeZero-path integration/Viceroy coverage — the local
trusted_server_config.edgezero_enableddefault isfalse, and the integration-test workflow does not override it, so CI still primarily exercises the legacy entry point while this PR's new production path can regress in Fastly request conversion, config-store dispatch, and end-to-end middleware/EC wiring. Please add at least one integration-test variant with the flag enabled covering representative publisher fallback, protected-route/auth, EC/API, and asset/proxy paths.
P3 / Low
- Configuration docs omit the new publisher buffer cap —
trusted-server.tomldocumentspublisher.max_buffered_body_bytes, butdocs/guide/configuration.mdstill omits the field and its environment override. Please add the default, operational impact, effective Fastly limits, andTRUSTED_SERVER__PUBLISHER__MAX_BUFFERED_BODY_BYTESexample.
CI / Existing Reviews
gh pr checks 628 reports all visible checks passing, including Rust analysis, cargo test, cargo fmt, vitest, TypeScript/docs format, CodeQL, and integration/browser tests. Existing reviews include earlier CHANGES_REQUESTED rounds; several prior findings have author replies/fixes, but the PR review decision remains CHANGES_REQUESTED. This review is submitted as REQUEST_CHANGES because the request-filter bypass is a blocking security/correctness regression when edgezero_enabled is enabled.
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
Reviewed the current PR #628 diff after the latest fixes. Requesting changes because the EdgeZero path still misses a security-critical legacy hook: integration request filters, including DataDome server-side protection, do not run when edgezero_enabled=true.
Cross-cutting finding
🔧 wrench — CI does not exercise the EdgeZero Fastly entry point
The runtime branch only enters edgezero_main when trusted_server_config.edgezero_enabled is readable and true, but the Rust Viceroy fixture defines only jwks_store, and both browser CI runs point at that same fixture. That means the Fastly request conversion/finalization/send-to-client behavior introduced by this PR can be untested while CI stays green. Please add an EdgeZero-enabled Viceroy fixture or CI matrix entry with trusted_server_config.contents.edgezero_enabled = "true" and run at least a focused parity/smoke subset against it before merging.
Existing GitHub checks are green, but they currently appear to exercise the legacy entry point unless a test explicitly bypasses main.
Address PR review findings on the dual-path EdgeZero entry point. - Run the integration request-filter pipeline (DataDome protection) on the EdgeZero dispatch path, mirroring legacy ordering: after auth, before route match. Respond short-circuits keep EcFinalizeState; response effects thread out via extensions and apply after EC finalization in edgezero_main. Add dispatch regression tests via a test-utils-gated registry constructor. - Reject publisher.max_buffered_body_bytes = 0 at config validation. - Make the buffered publisher resolver method-aware so HEAD and bodiless statuses keep origin Content-Length instead of a misleading 0. - Bound cumulative decoded HTML input so a placeholder-emitting rewriter cannot grow the Wasm heap behind the output cap. - Add an EdgeZero-enabled Viceroy fixture and integration-tests-edgezero CI job running the EC lifecycle suite against the EdgeZero entry point. - Document publisher.max_buffered_body_bytes and the edgezero_enabled runtime flag in the configuration guide.
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Automated review:
Review Summary
Reviewed PR #628 against main, focusing on the current dual-path EdgeZero entry point after prior review fixes. The major previously reported parity issues appear addressed, but I found one remaining security-relevant regression: the EdgeZero path drops Fastly request metadata that DataDome protection consumes for bot detection. I also left a non-blocking CI canary suggestion.
Findings
P0 / Blockers
None.
P1 / High
See inline comment on crates/trusted-server-adapter-fastly/src/app.rs.
P2 / Medium
See inline comment on .github/workflows/integration-tests.yml.
P3 / Low
None.
CI / Existing Reviews
GitHub checks are passing, including the EdgeZero integration job. Existing reviews already covered forwarded-header sanitization, build-state panic handling, dependency pinning, router-level 405/finalization, flag parsing, auth error conversion, and several HTTP/header parity issues; I avoided duplicating those.
| .backend(Arc::new(FastlyPlatformBackend)) | ||
| .http_client(Arc::new(FastlyPlatformHttpClient)) | ||
| .geo(Arc::new(FastlyPlatformGeo)) | ||
| .client_info(ClientInfo { |
There was a problem hiding this comment.
Automated review: P1 / High — EdgeZero drops bot-protection metadata before DataDome sees it.
build_per_request_services only carries client_ip into RuntimeServices::client_info and leaves tls_protocol, tls_cipher, tls_ja4, h2_fingerprint, server_hostname, and server_region empty. The legacy Fastly path populates the same fields from the original fastly::Request and env (platform.rs:570-577), and DataDome protection serializes them into its payload (ServerName, ServerRegion, TlsProtocol, TlsCipher, JA4, H2Fingerprint). On the EdgeZero path, DataDome therefore receives a materially weaker bot-protection payload even though edgezero_main still has the original Fastly request before into_core_request consumes it.
Suggested fix: capture the full ClientInfo in edgezero_main before conversion (or insert an extension alongside DeviceSignals) and have this builder read that extension instead of defaulting these fields to empty. Add a regression test with sample TLS/JA4/H2/server metadata and a DataDome request filter assertion that the EdgeZero protection payload includes those fields.
| # dispatch, publisher fallback proxying, and end-to-end EC/API wiring on | ||
| # the EdgeZero path. The legacy `integration-tests` job above still covers | ||
| # the full framework matrix. | ||
| - name: Run EdgeZero EC lifecycle tests |
There was a problem hiding this comment.
Automated review: P2 / Medium — EdgeZero CI can false-pass on the legacy path.
This job relies on VICEROY_CONFIG_PATH enabling edgezero_enabled, but main() intentionally falls back to legacy_main when the config store cannot be opened/read. The reused EC lifecycle scenarios are also valid on the legacy path, so a fixture/env/config-store regression could make this job green while exercising legacy instead of EdgeZero.
Suggested fix: add an EdgeZero-only canary to this job/suite, such as asserting TRACE / returns the documented EdgeZero router-level 405 rather than the legacy publisher fallback, or exposing a test-only response marker that proves the EdgeZero branch was taken.
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
Re-review at head ba07d4b0 (base is now main; PR12/PR13 merged). All findings from the two prior review rounds are resolved and verified, including reading the pinned stackpop/edgezero source to confirm into_core_request inserts the client-IP FastlyRequestContext into the router request. The new work this round — the integration request-filter pipeline, asset-route fallback, startup-built partner registry, pre-conversion device-signal capture, and the HTML post-processing accumulator bound (now capping both decoded input and accumulated output) — is exact-parity matched to legacy route_request and covered by tests, including a new dedicated "integration tests (EdgeZero entry point)" CI job that exercises the EC lifecycle against the flag-on path. No blocking issues found. Two non-blocking observations are left inline.
Non-blocking
🌱 seedling
- Request filters run on CORS preflight:
OPTIONS /_ts/api/v1/identifypasses through the request-filter pipeline beforecors_preflight_identify; confirm filters are inert forOPTIONSso a challenge cannot break CORS (app.rs:455).
📝 note
- Asset bodies buffered, not streamed on the EdgeZero path, bounded by
publisher.max_buffered_body_bytes; documented as interim until the streaming cutover (app.rs:713, issue #495).
CI Status
- cargo fmt: PASS
- clippy (Analyze rust): PASS
- cargo test: PASS
- vitest: PASS
- integration tests: PASS
- integration tests (EdgeZero entry point): PASS
- browser integration tests: PASS
- format-docs / format-typescript: PASS
| } | ||
|
|
||
| let effects = | ||
| match run_pre_route_filters(&state, &services, &mut req, ec.geo_info.as_ref()).await { |
There was a problem hiding this comment.
🌱 seedling — The pre-route integration request-filter pipeline runs for OPTIONS /_ts/api/v1/identify too, before the OPTIONS branch reaches cors_preflight_identify. This faithfully matches legacy route_request ordering (filters run after auth, before route match), so it is not a regression. The forward-looking concern: a bot-protection filter (e.g. DataDome) that elects to challenge a request would challenge the CORS preflight, and browsers do not follow a challenge response on a preflight — the cross-origin call would simply fail. Worth confirming the shipped request filters are inert for OPTIONS (or short-circuiting preflights before the filter step), so enabling such a filter can't silently break CORS for the identify API.
| let (mut response, stream_body) = asset_response.into_response_and_body(); | ||
|
|
||
| if let Some(body) = stream_body { | ||
| match buffer_asset_body(body, state.settings.publisher.max_buffered_body_bytes) |
There was a problem hiding this comment.
📝 note — On the EdgeZero path asset bodies are fully buffered (bounded by publisher.max_buffered_body_bytes) rather than streamed to the client as on the legacy path, because edgezero_main converts the whole response before sending. This is clearly documented as interim until the streaming cutover (#495) and is bounded against Wasm-heap OOM, so it's fine to ship — flagging only to keep the buffering-vs-streaming tradeoff and the reuse of the publisher cap for assets on the record for that follow-up.
…subsystem (PR 15) (#635) * Add PR15 implementation plan: remove fastly from core crate * Move compat conversion fns to adapter, delete core compat.rs * Move geo_from_fastly from core to adapter platform * Move BackendConfig from core to adapter backend module BackendConfig uses fastly::backend::Backend directly, making it incompatible with the platform-portability goal. This commit: - Copies backend.rs verbatim into trusted-server-adapter-fastly, updating the one crate-internal import path - Adds url dependency to the adapter Cargo.toml - Rewires platform.rs and management_api.rs to use crate::backend - Removes pub mod backend from trusted-server-core/lib.rs - Migrates aps.rs, adserver_mock.rs, and prebid.rs off the deleted BackendConfig, using context.services.backend().ensure() for registration and a new predict_backend_name_for_url free function in integrations/mod.rs for stateless name prediction cargo check --workspace passes with zero errors. * Delete dead backend_name_for_url from adapter backend All callers were migrated to predict_backend_name_for_url in core's integrations module. Remove the now-unused method and update the parse_origin doc comment that referenced it. * Delete legacy FastlyConfigStore and FastlySecretStore from core Remove crates/trusted-server-core/src/storage/ entirely (config_store.rs, secret_store.rs, mod.rs). All call sites migrated to PlatformConfigStore / PlatformSecretStore traits. Also drop the now-broken intra-doc links in trusted-server-adapter-fastly/src/platform.rs that referenced the deleted types. * Remove fastly::kv_store from core consent module Replace direct fastly::kv_store::KVStore usage in consent/kv.rs with a platform-neutral ConsentKvOps trait. The trait has four sync methods (load_entry, save_entry_with_ttl, fingerprint_unchanged, delete_entry) keeping the consent pipeline synchronous. - consent/kv.rs: add ConsentKvOps trait; replace load_consent_from_kv / save_consent_to_kv / delete_consent_from_kv with load_consent / save_consent (trait-based); remove open_store / fingerprint_unchanged private fns; drop ConsentKvMetadata / metadata_from_context (metadata API was Fastly-specific; fingerprint now lives in the body fp field); add StubKvOps + integration tests - consent/mod.rs: add kv_ops field to ConsentPipelineInput; update try_kv_fallback and try_kv_write to consume kv_ops instead of store_name; update all struct literal sites - publisher.rs: add kv_ops: Option<&dyn ConsentKvOps> parameter to handle_publisher_request; wire it into ConsentPipelineInput and use it for the SSC-revocation delete - adapter/platform.rs: add FastlyConsentKvStore wrapping fastly::kv_store::KVStore implementing ConsentKvOps via the sync API - adapter/app.rs + main.rs: open FastlyConsentKvStore from settings.consent.consent_store and pass as kv_ops to handle_publisher_request * Fix consent KV trait design and formatting Remove `fingerprint_unchanged` from `ConsentKvOps` trait so the common case (consent unchanged) costs one KV read instead of two. `save_consent` now loads the entry once and compares the fingerprint inline. Extract `open_consent_kv` helper in `app.rs` to deduplicate the two identical KV-open blocks. Replace bare `unwrap()` with descriptive `expect` messages in consent KV tests. Run `cargo fmt --all` to fix all whitespace issues. * Move tokio to dev-dependencies in core (test-only usage) * Remove fastly dependency from trusted-server-core * Remove stale fastly:: references from core doc comments * Apply cargo fmt formatting fixes * Wire consent KV into auction path and remove tokio from core tests * Address PR15 review findings: diagnostic regression, header dedup, compat tests - predict_backend_name_for_url now returns Result<String, Report<TrustedServerError>> instead of Option<String>; call sites use inspect_err(...).ok() to preserve the Option<String> trait interface while logging the actual failure reason - Promote SPOOFABLE_FORWARDED_HEADERS to pub in http_util; replace inline copy in compat.rs with a use import — single source of truth for the strip list - Add sanitize_fastly_forwarded_headers_strips_spoofable test (all 4 entries + Host preserved) and to_fastly_response_with_streaming_body_produces_empty_body test (pins silent-drop behaviour) to compat.rs - Change Consent KV non-ItemNotFound error log from debug "lookup miss" to warn "lookup failed" for operational visibility - Drop stale "will be removed in PR 15" forward references from app.rs and main.rs - Drop unnecessary u16 type suffixes on port literal defaults * Resolve PR15 consent and backend review findings * fix cargo fmt * Resolve PR review suggestions * Restore EdgeZero consent KV wiring * Resolve PR 635 round-1 review findings Blocking: - select(): return Err(PlatformError::HttpClient) when get_backend_name() returns None instead of silently propagating "" and losing bid correlation - Remove dead certificate_check: bool from predict_integration_backend_name; hardcode true inside to match ensure_integration_backend_with_timeout so predict/ensure cannot silently diverge and break orchestrator correlation Non-blocking: - Fix duplicate enable_ssl().sni_hostname() in BackendConfig::ensure; call once unconditionally, then add check_certificate() conditionally - Add debug_assert!(false) in to_fastly_response Stream arm to catch caller misuse in debug production builds; gated with #[cfg(not(test))] so the behavior-documentation test still passes - Add integration-route assertion to edgezero_missing_consent_store test: GET /integrations/datadome/tags.js must not return 503 when consent KV is misconfigured, locking in that integration proxy bypasses consent gating - Rename AppState.kv_store -> default_kv_store to signal it is only the fallback when no per-route consent KV override applies * Update integration-tests lock file for getrandom wasm dep and tokio dev-dep move * Finish tokio removal from trusted-server-core Convert all remaining #[tokio::test] tests to futures::executor::block_on across proxy.rs, publisher.rs, auction/endpoints.rs, auction/orchestrator.rs, and integrations/testlight.rs. Drop tokio from [dev-dependencies] in trusted-server-core/Cargo.toml — tokio is no longer referenced anywhere in the crate. Also fix two stale doc comments in proxy.rs that still referenced fastly::Response and platform_response_to_fastly. * Fix stale fastly::Body doc reference in platform/mod.rs * Resolve PR15 re-review findings Blocking: - Restore the non-panicking '/' fallback in from_fastly_request: a URL Fastly accepts but http::Uri rejects degrades with a warning instead of aborting the Wasm instance. Adds a regression test using a >65534-byte URL (http::Uri's length limit; url::Url has none) - Deduplicate the EC ID generator: edge_cookie.rs delegates to the canonical ec::generation normalize_ip + generate_ec_id, removing the IPv6 /64 normalization divergence that minted non-correlating identity prefixes. Adds a test asserting both paths hash to the same prefix - Wire consent KV read-fallback and write-on-change into build_consent_context: loads persisted consent when the request carries no signals (jurisdiction re-derived from current geo) and persists cookie-sourced consent after the context is built. Adds three pipeline-level tests with an in-memory KV store Non-blocking: - git rm the orphaned trusted-server-core/src/backend.rs (no longer compiled since lib.rs dropped the module), collapsing the duplicated DEFAULT_FIRST_BYTE_TIMEOUT to the platform definition - Restore the two lgtm[rust/cleartext-logging] suppressions in publisher.rs — CodeQL still runs in CI (.github/workflows/codeql.yml) - Wrap prebid backend resolution errors in TrustedServerError::Auction with the endpoint, matching aps and adserver_mock attribution - Log a warning when an EC Set-Cookie header value is rejected instead of silently no-opping (set_ec_cookie and expire_ec_cookie) - Replace the inline block_on(select(vec![...])) + .ready in pull sync with the equivalent http_client().wait() helper * Document why trusted-server-core still depends on fastly The EC KV identity graph requires compare-and-swap semantics (InsertMode::Add and generation preconditions) that the EdgeZero KvStore trait does not expose, and the rate limiter uses fastly::erl, which has no EdgeZero abstraction. Removing the dependency is deferred until EdgeZero grows equivalent APIs; note this on the manifest entry so the deferral is discoverable. * Sync integration-tests lockfile after dropping core wasm js deps
…-entry-point-dual-path # Conflicts: # crates/trusted-server-core/src/integrations/mod.rs # crates/trusted-server-core/src/platform/types.rs # crates/trusted-server-core/src/proxy.rs
The EdgeZero path built RuntimeServices with only client_ip, leaving tls_protocol, tls_cipher, tls_ja4, h2_fingerprint, server_hostname, and server_region empty. DataDome request filters serialize these into their bot-detection payload, so the EdgeZero path produced a materially weaker payload than the legacy path. edgezero_main now captures the full ClientInfo from the original FastlyRequest (the only place Fastly's TLS/JA4/H2 accessors return real values) and threads it through the request extensions, mirroring the existing DeviceSignals handoff. build_per_request_services reads it back, falling back to the dispatch-inserted client IP when absent. The field extraction is shared with the legacy path via client_info_from_request.
The EdgeZero EC lifecycle job points Viceroy at a config store with edgezero_enabled = "true", but main() silently falls back to legacy_main when the config store cannot be opened or read. The EC lifecycle scenarios pass on both paths, so a fixture/env/config-store regression could green this job while it actually exercises legacy instead of EdgeZero. assert_edgezero_entry_point sends a TRACE request and asserts a router-level 405: the EdgeZero router rejects methods outside its registered set, whereas the legacy path proxied every method to the publisher origin. Run before the scenarios so the job fails fast if the EdgeZero branch was not taken.
DataDome's is_request_protected already returns false for OPTIONS, so the filter never challenges a CORS preflight — a challenge would break the identify API's CORS since browsers do not follow challenge responses on preflights. Add a regression test asserting filter_protection_request returns Continue for an OPTIONS preflight, locking the contract so a future filter change cannot silently reintroduce the break.
4 participants
Summary
TrustedServerAppor the preservedlegacy_mainbased onedgezero_enabledin thetrusted_server_configFastly ConfigStore. Missing, unreadable, or non-true flag values fall back to legacy.GET /healthas a cheap entry-point shortcut that bypasses logging, settings, and app construction.TrustedServerAppviaedgezero_core::app::HookswithFinalizeResponseMiddlewareandAuthMiddleware; auction and publisher fallback routes fail closed when a configured consent store cannot be opened.rev = "38198f9839b70aef03ab971ae5876982773fc2a1"and bumpstomlto"1.1".ProxyRequestConfig.allowed_domains:&[]is open mode for operator-configured integration proxies; a non-empty slice restricts first-party proxy initial targets and redirect hops.Changes
Cargo.tomledgezero-adapter-axum,edgezero-adapter-cloudflare,edgezero-adapter-fastly, andedgezero-coretorev = "38198f9839b70aef03ab971ae5876982773fc2a1"; bumptomlto"1.1"Cargo.locktomlversion changefastly.tomltrusted_server_configwithedgezero_enabled = "false"as the defaultcrates/trusted-server-adapter-fastly/src/main.rs/healthshortcut, EdgeZerodispatch_with_config, entry-pointmatch get_settings()finalize block for router-level 405/404 responses, and PR13 legacy streaming handlingcrates/trusted-server-adapter-fastly/src/app.rsTrustedServerApp, middleware wiring, consent-aware runtime service selection, table-driven named route registration, androutes_for_statetest seamcrates/trusted-server-adapter-fastly/src/middleware.rsapply_finalize_headershelpercrates/trusted-server-adapter-fastly/src/route_tests.rsHandlerOutcomeand the consent-store fail-closed behaviorcrates/trusted-server-core/src/proxy.rsProxyRequestConfig.allowed_domainsopen-mode versus restricted-mode semanticsCloses
Closes #495
Test plan
cargo test -p trusted-server-adapter-fastly dispatch_auction_with_missing_consent_store_returns_503cargo test -p trusted-server-adapter-fastlycargo test --workspacecargo clippy --workspace --all-targets --all-features -- -D warningscargo fmt --all -- --checkcd crates/js/lib && npx vitest run(not rerun in this cleanup)cd crates/js/lib && npm run format(not rerun in this cleanup)cd docs && npm run format(not rerun in this cleanup)Checklist
unwrap()in production code - useexpect("should ...")logmacros, notprintln!