Add dual-path EdgeZero entry point with feature flag (PR 14)

[prk-Jr]

Summary

• Adds a feature-flagged dual-path entry point: requests dispatch through the new EdgeZero TrustedServerApp or the preserved legacy_main based on edgezero_enabled in the trusted_server_config Fastly ConfigStore. Missing, unreadable, or non-true flag values fall back to legacy.
• Keeps GET /health as a cheap entry-point shortcut that bypasses logging, settings, and app construction.
• Implements TrustedServerApp via edgezero_core::app::Hooks with FinalizeResponseMiddleware and AuthMiddleware; auction and publisher fallback routes fail closed when a configured consent store cannot be opened.
• Registers named routes from a single table that contains each path, its primary methods, and its handler. The same table also registers publisher fallback methods for non-primary verbs, so adding a named route no longer requires a second primary-method list.
• Keeps legacy streaming publisher responses from the PR13 base branch while buffering publisher fallback responses inside the EdgeZero route handler.
• Pins all four EdgeZero workspace dependencies to rev = "38198f9839b70aef03ab971ae5876982773fc2a1" and bumps toml to "1.1".
• Clarifies 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

File	Change
Cargo.toml	Pin edgezero-adapter-axum, edgezero-adapter-cloudflare, edgezero-adapter-fastly, and edgezero-core to rev = "38198f9839b70aef03ab971ae5876982773fc2a1"; bump toml to "1.1"
Cargo.lock	Updated for the pinned EdgeZero revision and toml version change
fastly.toml	Add local trusted_server_config with edgezero_enabled = "false" as the default
crates/trusted-server-adapter-fastly/src/main.rs	Add feature-flag dispatch, /health shortcut, EdgeZero dispatch_with_config, entry-point match get_settings() finalize block for router-level 405/404 responses, and PR13 legacy streaming handling
crates/trusted-server-adapter-fastly/src/app.rs	Add TrustedServerApp, middleware wiring, consent-aware runtime service selection, table-driven named route registration, and routes_for_state test seam
crates/trusted-server-adapter-fastly/src/middleware.rs	Add finalization and auth middleware, including the 401 geo-skip behavior and shared apply_finalize_headers helper
crates/trusted-server-adapter-fastly/src/route_tests.rs	Update legacy route tests for HandlerOutcome and the consent-store fail-closed behavior
crates/trusted-server-core/src/proxy.rs	Document and test ProxyRequestConfig.allowed_domains open-mode versus restricted-mode semantics

Closes

Closes #495

Test plan

• cargo test -p trusted-server-adapter-fastly dispatch_auction_with_missing_consent_store_returns_503
• cargo test -p trusted-server-adapter-fastly
• cargo test --workspace
• cargo clippy --workspace --all-targets --all-features -- -D warnings
• cargo fmt --all -- --check
• JS tests: cd crates/js/lib && npx vitest run (not rerun in this cleanup)
• JS format: cd crates/js/lib && npm run format (not rerun in this cleanup)
• Docs format: cd docs && npm run format (not rerun in this cleanup)

Checklist

• Changes follow CLAUDE.md conventions
• No unwrap() in production code - use expect("should ...")
• Uses log macros, not println!
• New EdgeZero consent-store regression coverage added
• No secrets or credentials committed

[aram356]

Summary

Reviewed the 13 files that PR 628 actually changes vs its base (feature/edgezero-pr13-...). The dual-path entry point is a sensible migration shape, and the explicit GET/POST "/" routes + the header-precedence middleware test are the kind of load-bearing details that prevent future outages. However, the EdgeZero path currently diverges from the legacy path in ways that are security- and reliability-relevant, and the branch = "main" pin on upstream deps makes builds non-deterministic. Blocking on those.

Blocking

🔧 wrench

• Forwarded-header sanitization missing on EdgeZero path — legacy strips Forwarded / X-Forwarded-* / Fastly-SSL before routing; EdgeZero hands the raw req to dispatch_with_config. With edgezero_enabled = "true" as the local-dev default, this is the default path. (main.rs:95)
• build_state() panics on misconfig — expect("should …") on settings / orchestrator / registry. Legacy returns a structured error response; EdgeZero now 5xx's with no detail. (app.rs:75)
• Docstring "built once at startup" is misleading — every request spins up a fresh Wasm instance, so build_state() runs per-request. Invites future false caching. (app.rs:61)
• Stale #[allow(dead_code)] on now-live middleware — five suppressions with "until Task 4 wires app.rs" comments. Task 4 is this PR. (middleware.rs:50,57,96,103,146)
• AuthMiddleware flattens Report<TrustedServerError> into EdgeError::internal(io::Error::other(...)) — loses per-variant status code and user message; generic 500 instead of the specific error. (middleware.rs:122)
• edgezero-* deps pinned to branch = "main" — non-deterministic builds; supply-chain path into prod via a moving upstream branch. Pin to a specific rev or fork tag. (Cargo.toml:59-62)

Non-blocking

🤔 thinking

• TLS metadata dropped on EdgeZero path — tls_protocol / tls_cipher hardcoded to None; legacy populates both. Low impact today (debug logging only), but a silent regression if any future signing/audit path reads them. (app.rs:123-124)

♻️ refactor

• 11 near-identical handler closures in routes() — a pair of file-local make_sync_handler / make_async_handler helpers would cut ~120 lines without harming auditability. (app.rs:175-301)
• FinalizeResponseMiddleware hardcodes FastlyPlatformGeo — take Arc<dyn PlatformGeo> instead so Middleware::handle can be unit-tested end-to-end. (middleware.rs:68)
• build_per_request_services duplicates platform::build_runtime_services — extract a shared helper that takes ClientInfo. (app.rs:111-127)

🌱 seedling

• fastly.toml flips local dev default to EdgeZero — combined with the blockers above, every fastly compute serve now exposes them. Consider defaulting to "false" until the blockers land. (fastly.toml:52)

⛏ nitpick

• AppState fields can be private (not pub(crate)). (app.rs:62-66)
• Root-route pairs clone closures four times — upstream RouterService::get_many would help. (app.rs:374-377)

📝 note

• The dispatch_with_config comment explaining the set_logger panic is excellent "why, not what" documentation. (main.rs:91-94)

👍 praise

• operator_response_headers_override_earlier_headers codifies a brittle precedence contract. (middleware.rs:217-233)
• Explicit GET "/" / POST "/" routes with the in-code explanation of matchit's wildcard gap prevent a future 404 outage. (app.rs:374-377)

CI Status

• browser integration tests: PASS
• integration tests: PASS
• prepare integration artifacts: PASS
• fmt / clippy / unit tests: not surfaced by gh pr checks — please confirm these ran.

[ChristianPavilonis]

Summary

Supplementary review — see aram356's review for the primary findings. This review covers additional items not raised there.

Non-blocking

🔧 wrench (cross-cutting, from earlier PRs in this stack)

• set_header drops multi-valued headers: edge_request_to_fastly in platform.rs:187 uses set_header instead of append_header, silently dropping duplicate headers. Pre-existing pattern (also in compat::to_fastly_request), but the EdgeZero path creates a new copy of the same bug.

🌱 seedling

• parse_edgezero_flag is case-sensitive: "TRUE" and "True" silently fall through to legacy path. Consider eq_ignore_ascii_case or logging unrecognized values.

📝 note (cross-cutting, from earlier PRs)

• Stale doc comment in platform/mod.rs:31: References fastly::Body in publisher.rs, but PR 11 already migrated to EdgeBody.

♻️ refactor (cross-cutting, from earlier PRs)

• Duplicated body_as_reader helper: Identical function in proxy.rs:24 and publisher.rs:23. Extract to shared utility.

⛏ nitpick (cross-cutting)

• Management API client re-created per write: Each put/delete in platform.rs constructs a new FastlyManagementApiClient. Fine for current usage, noted for future batch writes.

📌 out of scope

• compat.rs in core depends on fastly types: Already tracked as PR 15 removal target.

CI Status

• browser integration tests: PASS
• integration tests: PASS
• prepare integration artifacts: PASS

[ChristianPavilonis]

superseded

[ChristianPavilonis]

Superseding note: I re-posted the line-specific findings as inline comments after switching to the single-comment review-comments API with the PR head commit SHA.

One additional high-priority finding is still not inline because the relevant code is outside this PR diff:

P1 — ProxyRequestConfig.allowed_domains is ignored, so integration proxies inherit the first-party allowlist unintentionally (crates/trusted-server-core/src/proxy.rs:432-458,573-579,694-700)

ProxyRequestConfig documents two modes: integrations should pass &[] for open mode, while the first-party proxy should pass &settings.proxy.allowed_domains. But proxy_request() currently destructures allowed_domains into _, and proxy_with_redirects() always checks settings.proxy.allowed_domains instead. That means enabling proxy.allowed_domains for /first-party/proxy can unexpectedly break unrelated integration proxy traffic.

Suggested fix: thread config.allowed_domains through to proxy_with_redirects() and use that slice for both the initial target check and redirect-hop checks. I’d also add a regression test where settings.proxy.allowed_domains is non-empty but proxy_request(... allowed_domains: &[]) still succeeds.

[prk-Jr]

@ChristianPavilonis Fixed the non-inline ProxyRequestConfig.allowed_domains item in eb6a9e9f. ProxyRequestConfig::allowed_domains is now threaded into ProxyRedirectPolicy and used for both the initial target check and redirect-hop checks. Added regressions for open mode with a non-empty settings.proxy.allowed_domains and open-mode redirect hops; cargo test -p trusted-server-core proxy_request_ passes.

[aram356]

Summary

Round-3 review. The previously blocking findings from round 2 are all resolved: forwarded-header sanitization runs on the EdgeZero path, build_state returns a Result with a startup_error_router fallback, AuthMiddleware preserves per-variant status codes via http_error, FinalizeResponseMiddleware absorbs handler errors via IntoResponse, the AppState docstring is corrected, workspace edgezero deps are pinned to the full 40-char commit SHA 38198f9839b70aef03ab971ae5876982773fc2a1, parse_edgezero_flag is case-insensitive with full test coverage, HEAD/OPTIONS/PUT/PATCH/DELETE catch-all routes are registered, and FinalizeResponseMiddleware now takes Arc<dyn PlatformGeo> for testability.

This round surfaces two new blockers and four non-blocking findings. Both blockers are scope/behavior issues, not implementation bugs:

• Silent behavior change in proxy.rs — integration-proxy redirect chains are no longer bounded by settings.proxy.allowed_domains, and nothing in the PR body documents it.
• Non-standard HTTP methods bypass the middleware chain — legacy route_request falls through to publisher origin for every method; this path 405/404s them without running FinalizeResponseMiddleware, dropping all TS/geo/operator headers. Contradicts the FinalizeResponseMiddleware doc contract.

Both are addressable in this PR without a large rework. Inline comments describe concrete fix options.

Local CI is green: cargo fmt --check, cargo clippy --workspace --all-targets --all-features -D warnings, and cargo test --workspace (867 tests).

Blocking

🔧 wrench

• Scope creep: silent integration-proxy allowlist semantics change — proxy.rs now lets integration proxies opt out of settings.proxy.allowed_domains for initial target and redirect hops. Not in PR description. See inline on crates/trusted-server-core/src/proxy.rs:446.
• Non-standard HTTP methods bypass middleware — TRACE/CONNECT/WebDAV verbs skip FinalizeResponseMiddleware; TS/geo/operator headers dropped. Contradicts the doc contract. See inline on crates/trusted-server-adapter-fastly/src/app.rs:149.

Non-blocking

🤔 thinking

• Per-request INFO-level routing log — noisy at production traffic; consider log::debug! or once-per-cold-start. Inline on main.rs:96.

📝 note

• No tests for FinalizeResponseMiddleware::handle / AuthMiddleware::handle — the code changed this round has no direct unit coverage. Inline on middleware.rs:130.
• No end-to-end test for the EdgeZero dispatch path — would catch the method-bypass regression directly. Inline on app.rs:392.

CI Status

• browser integration tests: PASS (GitHub)
• integration tests: PASS (GitHub)
• prepare integration artifacts: PASS (GitHub)
• fmt: PASS (local)
• clippy: PASS (local)
• rust tests: PASS (local, 867 tests)

[ChristianPavilonis]

Summary

Reviewed the remaining PR #628 findings after excluding the consent_store replacement item and the orchestrator issue per discussion. I found two high-impact EdgeZero/legacy parity concerns plus a few documentation/comment accuracy issues. GitHub checks currently shown for the PR are passing.

[aram356]

Summary

Re-review against main-merge intent. The dual-path entry point and middleware shape are right, and prior round-2 blockers (build_state Result, forwarded-header sanitization, parse_edgezero_flag casing) are resolved. Three new functional regressions remain that change request handling vs legacy on identical input, plus two correctness/process gaps.

PR base is feature/edgezero-pr13-…; the diff vs main is 58 files because PRs 9–13 are unmerged. Findings below scope to the 13 files in this PR's actual delta.

CI: PASS (browser/integration tests, prepare artifacts).

Blocking

🔧 wrench

• EdgeZero path silently degrades when consent_store is misconfigured (compliance regression) — AppState::kv_store is hardcoded to UnavailableKvStore and the EdgeZero path never calls runtime_services_for_consent_route. Legacy returns 503 fail-closed when open_kv_store(name) fails (see route_tests.rs::configured_missing_consent_store_only_breaks_consent_routes); EdgeZero fail-opens because try_kv_fallback swallows KvError::Unavailable via ?. Inline on app.rs:92.
• Entry-point geo lookup overrides middleware's 401 geo-skip in production — Middleware correctly skips geo on 401 and emits X-Geo-Info-Available: false; the entry-point apply_finalize_headers then runs an unconditional geo lookup and overwrites it with full geo headers when a real client IP is present. The unit test passes only because it bypasses main() and lookup(None) → None. Inline on main.rs:115.
• Non-GET/POST methods on registered named-route paths regress to 405 — Named routes register a single method; matchit prefers the exact path over /{*rest}, so HEAD /first-party/proxy, OPTIONS /auction, PUT /admin/keys/rotate, etc. return 405 (legacy proxies them to publisher origin). The existing 405 test only covers TRACE on /, which doesn't exercise this case. Inline on app.rs:376.
• Silent settings drop on entry-point if let Ok(settings) = get_settings() — On Err, response goes out without TS headers; legacy fails fast at build_state(). Inline on main.rs:110.
• PR description doesn't match the diff — undisclosed tokio-test = "0.4" workspace dep (unused, not in Cargo.lock); description says edgezero deps pinned to branch=main but actual is a 40-char SHA; proxy.rs refactor + new "Behavior change from pre-PR-14" doc paragraph (factually wrong vs origin/main) not mentioned. Inlines on Cargo.toml:86 and proxy.rs:89.

❓ question

• Where is consent KV opened on the EdgeZero path? — If finding 1 is intentional (wired in PR 15/16), please link the issue/spec. Inline on app.rs:92.

Non-blocking

🤔 thinking

• TLS metadata still hardcoded to None — tls_protocol/tls_cipher not populated from FastlyRequestContext; silent regression for any future signing/audit path. Inline on app.rs:124. Previously flagged.
• Two geo lookups per request on the happy path — middleware + entry-point. Guard the second behind a "response missing TS headers" check. Inline on main.rs:111.
• Per-request double open of trusted_server_config config store — is_edgezero_enabled + dispatch_with_config both open it; SDK caches the handle, but the duplication is avoidable. Inline on main.rs:66. Previously flagged.

♻️ refactor

• Eleven near-identical handler closures — app.rs:273-359. Helper or route! macro collapses them. Inline on app.rs:351. Previously flagged.
• build_per_request_services duplicates platform::build_runtime_services — bodies identical except for client_info source. Inline on app.rs:127. Previously flagged.
• Module docstring overstates middleware coverage — top summary contradicts the "Startup error handling" section. Inline on app.rs:9.

🌱 seedling

• fastly.toml defaults local dev to EdgeZero — every fastly compute serve reproduces the regressions above. Recommend defaulting to "false" until parity is reached. Inline on fastly.toml:48.

📝 note

• No EdgeZero-side test for the consent-store regression — mirror configured_missing_consent_store_only_breaks_consent_routes driving TrustedServerApp::routes() after fix.
• No EdgeZero-side test for HEAD/OPTIONS on a named-route path — add dispatch_head_on_named_get_route_proxies_to_publisher.
• legacy_main cleanup TODO references issue #495 — confirm the issue covers compat.rs, route_request, legacy_main, the app::http_error duplication, and the entry-point finalize wrap.

CI Status

• browser integration tests: PASS
• integration tests: PASS
• prepare integration artifacts: PASS

[aram356]

Summary

Round-5 re-review against feature/edgezero-pr13-… (PR's actual base). The round-4 blockers are resolved: EdgeZero now fails-closed on misconfigured consent_store (auction + fallback handlers call runtime_services_for_consent_route), the entry-point geo lookup skips on 401, every named path registers publisher_fallback_methods() so HEAD/OPTIONS/PUT/PATCH/DELETE no longer regress to 405, fastly.toml defaults edgezero_enabled = "false", and the previously undisclosed tokio-test dep is gone.

The feature is gated off in production (any flag-read failure falls back to legacy) and reversible by flipping one config-store value, so blast radius is bounded. Approving in spirit — but blocking on three specific items because the alternative is merging a PR with a stale description, no direct test for a security-relevant fix, and a regression-prone footgun in the route table.

Local CI: cargo fmt --check PASS, cargo clippy --workspace --all-targets --all-features -- -D warnings PASS, cargo test --workspace PASS (967 tests). GitHub CI on c17fa27e: browser/integration tests + prepare artifacts all PASS.

Blocking

🔧 wrench

• PR description does not match the diff. Says Pin all four edgezero deps to branch=main — Cargo.toml actually pins to rev = "38198f9839b70aef03ab971ae5876982773fc2a1" (Cargo.lock confirms). Missing from the description: the proxy.rs ProxyRequestConfig.allowed_domains semantics change, the /health short-circuit in main.rs, the match get_settings() finalize block in main.rs, and the named_paths_primary_methods bookkeeping in app.rs. Reviewers shouldn't have to reverse-engineer the diff. Round-3 and round-4 both flagged the description being inaccurate.
• Missing EdgeZero-side test for the consent-store regression. Inline on route_tests.rs:184 — see comment. The only existing test exercises the legacy route_request path; the actual fix lives in app.rs.
• named_paths_primary_methods has no compile-time guard. Inline on app.rs:444 — see comment. Adding a new named route requires editing two places to stay consistent; mismatch silently regresses HEAD/OPTIONS to 405. Either fix here or file a tracked follow-up issue with explicit ownership.

Non-blocking

🤔 thinking

• Two geo lookups per request on the happy path (main.rs:116)
• get_settings() runs 2× per EdgeZero request and is uncached (main.rs:110) — this also drives the INSECURE: … warning spam below
• INSECURE: … warning spam on EdgeZero path. Direct consequence of the previous item: each get_settings() call emits placeholder-secret warnings, so misconfigured deployments log them 2–3× per request. Should be emitted once at startup (cache Settings in a OnceLock).
• Per-request double open of trusted_server_config (main.rs:66)
• TLS metadata still hardcoded to None on EdgeZero path (app.rs:158)
• assert_ne!(status, METHOD_NOT_ALLOWED) is a brittle regression guard (app.rs:590)

♻️ refactor

• 11 near-identical handler closures in routes() (app.rs:308)
• build_per_request_services duplicates platform::build_runtime_services (app.rs:146)
• fallback_handler.clone() runs ~59 times per routes() call (app.rs:449)

📝 note

• /health shortcut has no test (main.rs:78)

🌱 seedling

• AppState.kv_store is dead state on most routes (app.rs:83)
• Entry-point apply_finalize_headers could be conditional — paired with the "two geo lookups" finding; sentinel-header approach cleans both up post-#495.

👍 praise

• finalize_handle_skips_geo_lookup_for_401 (middleware.rs:331) — PanicGeo is the right pattern for skip-condition tests.
• finalize_handle_absorbs_handler_error_and_injects_headers (middleware.rs:308) — locks down a real round-2 regression.
• dispatch_unregistered_method_returns_405_at_router_level (app.rs:598) — known limitation documented in test code, not a rotting comment.

CI Status

• browser integration tests: PASS (GitHub)
• integration tests: PASS (GitHub)
• prepare integration artifacts: PASS (GitHub)
• fmt: PASS (local)
• clippy -D warnings: PASS (local)
• rust tests: PASS (local, 967 tests)

[ChristianPavilonis]

Summary

I found 1 high-priority issue and 4 follow-up documentation/maintainability issues in the current PR diff. The main runtime concern is that the EdgeZero publisher fallback buffers streaming publisher responses into memory, regressing the legacy streaming path.

CI currently shown by gh pr checks: browser integration tests, integration tests, and prepare integration artifacts are passing.

Verdict: COMMENT because there are no P0 blockers in the included findings.

[ChristianPavilonis]

Summary

I reviewed the EdgeZero entry-point dual-path changes. Most previously raised blockers appear resolved, but I found two remaining EdgeZero-path parity issues that should be addressed before relying on the feature flag.

[ChristianPavilonis]

EdgeZero path drops TLS scheme detection, causing HTTPS requests to be treated as HTTP

build_per_request_services() sets both tls_protocol and tls_cipher to None. After the forwarded-header sanitization added in this PR, RequestInfo::detect_request_scheme() has no trusted TLS signal and falls back to "http" (crates/trusted-server-core/src/http_util.rs:222). That means publisher fallback URL rewriting and any scheme-sensitive logic on the EdgeZero path can generate http URLs for HTTPS traffic, unlike the legacy path which populates TLS metadata from Fastly.

Suggestion: Preserve a trusted scheme/TLS signal before dispatch, either by extending the EdgeZero Fastly context to include TLS metadata or by deriving a trusted HTTPS indicator from the Fastly request at the adapter boundary and threading it into ClientInfo.

[ChristianPavilonis]

JA4 debug endpoint is bypassed by EdgeZero routing

The legacy path short-circuits GET /_ts/debug/ja4 before normal routing (main.rs:255), returning either the Fastly TLS fingerprint debug response or a 404 based on settings.debug.ja4_endpoint_enabled. The EdgeZero route table does not register this path, so when the feature flag is enabled the request falls through to the publisher origin instead. This contradicts the settings documentation that the endpoint returns 404 when disabled and breaks the debug feature when enabled.

Suggestion: Add an EdgeZero-path equivalent short-circuit before dispatch_with_config_handle, while the original FastlyRequest and TLS/JA4 accessors are still available, or register a dedicated route that can produce parity behavior.