Add trusted-server-adapter-axum native dev server (PR 16)#643
Merged
Conversation
…ls in formats and endpoints
Fix multi-line function call style in didomi.rs, line-break wrapping in publisher.rs test, and import ordering in synthetic.rs test module.
Adds noop_services_with_client_ip helper to test_support and a new test that verifies the client_ip path through generate_synthetic_id by asserting the HMAC differs when the IP changes.
…edgezero-pr7-geo-client-info
…zero-pr8-content-rewriting
…gezero-pr9-wire-signing-to-store-primitives
…te methods via management API Replace FastlyApiClient with FastlyManagementApiClient in the put/delete methods of FastlyPlatformConfigStore and the create/delete methods of FastlyPlatformSecretStore. Remove the now-unused FastlyApiClient import.
Thread RuntimeServices into AuctionContext so auction providers can access platform stores directly. Update PrebidAuctionProvider to use RequestSigner::from_services(context.services) instead of the now- removed from_config() shim. All construction sites and test helpers updated accordingly. This satisfies the final acceptance criterion of #490: no FastlyConfigStore/FastlySecretStore construction remains in the request_signing/ modules.
aram356
approved these changes
Jun 15, 2026
aram356
left a comment
aram356
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Round-6 re-review of the Axum native dev-server adapter. Every blocking and non-blocking finding from my previous review (f7e88462, "round-5") is fixed and independently verified — I read each diff and ran the new core CAS tests locally (4 passed). Approving.
Verified fixes:
- 🔧 Auction
failed_backend_nameis now attributed on the error path (backend_namecloned into the success.map,ready.as_ref().err().map(...)for the failure), with a newselect_attributes_failed_backend_nameregression test — the exact guard that was missing. - 🔧
/check-ci,/verify,/test-all, andbuild-validatormigrated to the*-fastly/*-axumaliases; newcheck-*/build-*aliases added;CLAUDE.mdcargo checkscoped. AxumPendingHandlenowDrops withhandle.abort(); both buffering pathslog::debug!the upstream size; KVdelete/countno longer double-wrap;apply_image_passthrough_metadatareturns();axum/towermoved to workspace deps; admin route tests assert== 501;test-axumtoolchain declareswasm32-wasip1.- New
ConflictInjectingEcKv+ three tests cover the CAS retry / concurrent-revive / exhaustion branches.
A fresh full-diff pass found no blocking issues. The Fastly KV → EcKvStore mapping is faithful and fastly is genuinely removed from core. A few minor, non-blocking items below.
Non-blocking
⛏ nitpick (inline)
- Byte-slice panic risk on non-ASCII prefix —
crates/trusted-server-adapter-fastly/src/ec_kv.rs:122. [workspace.dependencies]ordering —Cargo.toml:87.
📝 note / observation (adjacent to, or just outside, this PR's diff)
- Consent-store fail-closed comment is inaccurate —
crates/trusted-server-adapter-fastly/src/app.rs:171(runtime_services_for_consent_route). The 503-on-misconfigured-consent-store is a reasonable hardening, but the doc comment claims it "matches the legacyroute_requestbehavior" — legacy (main.rs:417) buildsruntime_serviceswithUnavailableKvStoreand never opens the named consent store, so it never fails closed. This is a new divergence on the EdgeZero path (gated off by default), not parity. Suggest rewording to "intentional hardening over legacy" and confirming the 503 is intended. (This sits on a context line just above the changed hunk, so it's body-level rather than inline.) get_metadatatransfers the full entry body —crates/trusted-server-core/src/ec/kv.rs:217.lookup()always callstake_body_bytes()(ec_kv.rs:67), so a metadata-only read downloads the body to discard it. The method has no production callers today, so this is a latent inefficiency rather than a regression — a dedicatedlookup_metadataprimitive would avoid it if it ever lands on a hot path.- Duplicated
DEFAULT_FIRST_BYTE_TIMEOUT = 15s— defined independently incrates/trusted-server-adapter-fastly/src/backend.rs:37andcrates/trusted-server-core/src/platform/mod.rs:57(the former is not touched by this PR). Consider exposing the core const and reusing it in the adapter to avoid drift.
CI Status
- integration tests / browser tests / prepare integration artifacts: PASS (on
f7e88462) - new core CAS-conflict tests: PASS locally (
cargo test -p trusted-server-core ec::kv, 4 tests) - fmt / clippy / test-fastly / test-axum: still gated to PRs targeting
main; clippy-split + new aliases verified correct locally
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.
…-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.
…t-dual-path' into feature/edgezero-pr15-remove-fastly-core # Conflicts: # crates/trusted-server-adapter-fastly/src/app.rs # crates/trusted-server-adapter-fastly/src/main.rs # crates/trusted-server-adapter-fastly/src/route_tests.rs
* Replace std::time::Instant with web_time::Instant in auction orchestrator
wasm32-unknown-unknown (Cloudflare Workers) does not support
std::time::Instant — it panics at runtime. web_time::Instant is a
zero-cost drop-in on native and JS-backed on WASM.
* Add trusted-server-adapter-cloudflare crate with host-target smoke tests
Implements the Cloudflare Workers adapter following the same pattern as
trusted-server-adapter-axum: TrustedServerApp implements the Hooks trait,
platform services use noop stubs on native (CI-compilable), and the
#[event(fetch)] entry point delegates to edgezero_adapter_cloudflare::run_app.
Also adds UnavailableHttpClient to trusted-server-core platform module,
parallel to the existing UnavailableKvStore.
* Add CI job for Cloudflare adapter and update CLAUDE.md
CI: test-cloudflare job checks native host compile, wasm32-unknown-unknown
compile (with cloudflare feature), and runs host-target unit tests.
CLAUDE.md: add cloudflare crate to workspace layout and build commands.
* Fix Cloudflare entry point: use worker 0.7 and pass manifest_src to run_app
The rev (38198f9) of edgezero used in this workspace requires worker 0.7
(not 0.6) and run_app() takes a manifest_src: &str as first argument.
Updated Cargo.toml and lib.rs accordingly.
* Add CloudflareHttpClient, build.sh, wrangler DX, and review fixes
- Implement CloudflareHttpClient (wasm32 only) using worker::Fetch for real
outbound proxy requests; strip content-encoding/transfer-encoding headers
since the Workers runtime auto-decompresses responses
- Add build.sh with cd-to-SCRIPT_DIR guard so worker-build always runs from
the correct crate root regardless of invocation directory
- Switch wrangler dev task to use --cwd from workspace root (same DX as Fastly)
- Add js-sys to workspace dependencies; reference it via { workspace = true }
- Fix #[ignore] messages on Cloudflare integration tests
- Replace std::time::{SystemTime,UNIX_EPOCH} with web_time in test code for
signing.rs and proxy.rs (consistency with production paths)
- Add NoopConfigStore/NoopSecretStore TODO comment tracking the gap
- Add extract_client_ip unit tests (parses cf-connecting-ip, absent, invalid)
- Remove empty crate_compiles_on_host_target test
- Add CloudflareHttpClient timeout doc noting Workers CPU-budget tradeoff
* Wire Cloudflare platform stores via edgezero handles
Replace direct worker::Env store construction with edgezero handles
already injected by run_app, reducing #[cfg(target_arch = "wasm32")]
blocks from 5 to 2.
- ConfigStoreHandleAdapter: bridges ctx.config_store() to
PlatformConfigStore — reuses the already-parsed TRUSTED_SERVER_CONFIG
JSON handle rather than re-parsing it on every request
- KvHandleAdapter: bridges ctx.kv_handle() to PlatformKvStore — reuses
the env.kv() handle opened by run_app rather than opening a new one
- CloudflareGeo: moved outside #[cfg]; reads cf-ipcountry and related
headers via ctx.request().headers() which needs no platform import
- CloudflareSecretStoreAdapter: kept under #[cfg] — env.secret() is
synchronous at the JS level but PlatformSecretStore::get_bytes is sync
while SecretHandle::get_bytes is async; direct env access is required
- Add .dev.vars to .gitignore (wrangler convention for local secrets)
- Add bytes workspace dep for KvStore impl
* Add Cloudflare Workers to CI integration tests
- Implement CloudflareWorkers::spawn() to start wrangler dev; in CI uses
wrangler.ci.toml (no build step, uses pre-built bundle); locally uses
wrangler.toml (triggers build.sh rebuild)
- Add wrangler.ci.toml: no [build] section so wrangler dev skips the
worker-build step when the bundle is pre-built as a CI artifact
- Add build-cloudflare input to setup-integration-test-env: adds
wasm32-unknown-unknown target and runs build.sh with integration test env vars
- In prepare-artifacts: enable build-cloudflare and upload build/ dir as artifact
- In integration-tests: restore CF build dir, install wrangler, set
CLOUDFLARE_WRANGLER_DIR, and remove --skip flags for cloudflare tests
* Resolve PR review findings in Cloudflare adapter
- Add GeoInfo happy-path test: build_geo_lookup_returns_some_with_populated_country
verifies country, city, continent, latitude, and longitude are correctly
populated when Cloudflare headers are present
- Simplify CloudflareSecretStoreAdapter::get_bytes: collapse brittle JsError
string-matching guards into a single error arm with contextual message
- Document sequential fan-out latency in CloudflareHttpClient: explain
sum(DSP_i) vs max(DSP_i) implication and why true parallelism is blocked
by the ?Send bound on PlatformHttpClient
- Fix stale wrangler.toml comment: update to reflect ConfigStoreHandleAdapter
wiring rather than the now-fallback NoopConfigStore
- Extend CI triggers to feature branches: format.yml and test.yml now run
fmt/clippy/tests on PRs targeting feature/** so stacking PRs are gated
- Fix fmt violations caught during pre-review verification: platform.rs
two-line Err wrapping and plan doc Prettier formatting
* Exclude Cloudflare adapter from wasm32-wasip1 test job
The adapter's tokio dev-dependency uses rt-multi-thread which is not
supported on wasm32. It is already tested in the dedicated test-cloudflare
job; exclude it from the workspace wasm test to avoid the compile error.
* Resolve PR review findings
* Resolve PR review findings
* Add alias for clouflare tests
* Fix lint error
* Resolve PR review findings
Blocking:
- Fix cargo fmt failure in integration-tests cloudflare.rs (long import, struct init)
- Replace `use worker::*` with explicit imports in adapter lib.rs
- Return generic 500 body from top-level dispatch error; log detail server-side
- Fix workerd process-group leak: spawn wrangler as group leader, killpg on drop
- Use find_available_port() for wrangler dev instead of hardcoded 8787
- Reject multi-provider fan-out in select() with PlatformError::HttpClient
instead of a noisy warn; per-provider timeout is now enforced by failing
loudly rather than silently degrading to sum(latencies)
- Fix clippy::type_complexity in axum platform.rs with ResponseTriplet alias
- Fix docs prettier formatting
Non-blocking:
- Return Err from execute() on Body::Stream outbound instead of silent empty body
- Assert unreachable! on Body::Stream in send_async (execute always returns Once)
- Extract shared dispatch() helper from get_fallback/post_fallback duplicates
- Rename auth test to auth_middleware_runs_in_chain_for_protected_routes
- Update stale #[ignore] reasons for Cloudflare integration tests
* Fix cargo clippy fastly
* Resolve PR 644 round-4 review findings
Blocking:
- Extract reject_multi_provider_fanout(len) as a target-agnostic free function
gated #[cfg(any(target_arch = \"wasm32\", test))]; add 4 unit tests covering
len=0 (pass), len=1 (pass), len=2 (reject), len=5 (reject with count in msg)
- Inline-confirm Secret::to_string() returns the raw JsValue string with no
wrapping — verified against worker-rs src/env.rs Display impl
Non-blocking:
- from_utf8_lossy -> std::str::from_utf8 with a hard error on non-UTF-8
outbound header values; lossy conversion silently mutated bytes
- to_ascii_lowercase() -> eq_ignore_ascii_case() for content-encoding /
transfer-encoding header checks — no allocation, same semantics
- Remove to_ascii_uppercase() on Method::from; worker 0.7 uppercases internally
- Strip Transfer-Encoding before setting Content-Length in buffered publisher
response (defense-in-depth; Workers likely strips it anyway)
- build.sh: gate cargo install with command -v to skip reinstall when warm
- wrangler.toml: add comment above placeholder KV ID explaining local vs remote
- wrangler.toml: add comment explaining compatibility_date and when to bump it
* Fix Method::from — worker 0.7 implements From<String> not From<&str>
The prior commit used Method::from(method.as_str()) based on reviewer note
that From<&str> is case-insensitive, but From<&str> is not implemented at all
in worker 0.7 — only From<String>. http::Method::to_string() already returns
uppercase so the to_ascii_uppercase() allocation is removed without changing
semantics.
* Add compile_error! for cloudflare feature on non-wasm32 targets
Enables the cloudflare feature on a native target pulls worker which
requires wasm-bindgen and produces cryptic linker errors. The guard
catches it immediately with a clear message pointing to the correct
--target wasm32-unknown-unknown flag.
* Collapse 13 handler closures into make_handler factory in app.rs
Extract make_handler<F, Fut>(state, f) -> impl Fn(RequestContext) -> BoxedHandlerFuture
that owns the build_per_request_services + ctx.into_request() boilerplate.
The routing table now reads as a flat list of (METHOD, PATH, handler-expr) triples
instead of 10 named handler variables each spanning 7-9 lines. ~120 lines removed.
* Address Cloudflare adapter review findings and fix axum CI wasm target
CI:
- Install the wasm32-wasip1 target in the test-axum job — the
'Verify Fastly WASM release build' step added by PR16 builds for
that target but the toolchain setup never installed it
Blocking findings:
- Strip stale Content-Length from decoded Workers fetch responses and
set it from the decoded body length. The origin value describes the
compressed payload while the Workers runtime auto-decompresses, so
pass-through responses could be sent with a truncating length
- Reject multi-provider auction fan-out before any request launches:
add PlatformHttpClient::supports_concurrent_fanout() (default true),
report false from CloudflareHttpClient whose send_async executes
eagerly, and validate in the orchestrator before the launch loop.
Previously the select()-time rejection fired only after every
provider request had already run sequentially and spent the auction
budget. The select()-time check remains as defense-in-depth.
Regression test asserts rejection happens with zero requests sent
Non-blocking findings:
- Outbound request headers use Headers::append instead of set so
duplicate header names forward every value, matching the response path
- Replace the unreachable! panic in send_async with a typed
PlatformError so an edgezero behavior change cannot panic a Worker
- Replace bare unwrap() with expect("should ...") in platform tests
per the testing conventions
* Extend per-target build/check aliases to the Cloudflare adapter
The build-fastly/check-fastly aliases merged from PR16 predate the
Cloudflare adapter and did not exclude it, so they compiled the
wasm32-unknown-unknown crate for wasm32-wasip1. Add the exclusion
(matching test-fastly/clippy-fastly), add a build-cloudflare alias
mirroring check-cloudflare, and update every dev-tooling doc that lists
the per-target commands: /check-ci, /verify, /test-all, the
build-validator agent, CLAUDE.md, AGENTS.md, and README.md now include
the cloudflare clippy/test/build/check steps alongside fastly and axum.
All aliases verified locally (check-fastly, check-axum,
check-cloudflare, build-cloudflare).
* Resolve PR17 round-5 review findings
- EC current_timestamp(): use web_time::SystemTime instead of std::time so
the EC path does not panic on wasm32-unknown-unknown (Cloudflare Workers),
matching the rest of core (proxy, orchestrator, signing).
- Cloudflare router: proxy the publisher fallback for all 7 methods
(GET/POST/HEAD/OPTIONS/PUT/PATCH/DELETE) via publisher_fallback_methods(),
mirroring the Axum/Fastly adapters; tsjs stays GET-only. Applies to both
the main and startup-error routers.
- build.sh: export .env.cloudflare.dev overrides with set -a so worker-build
sees them.
- rust-toolchain.toml: install wasm32-unknown-unknown so documented
cargo build-cloudflare works on a clean checkout.
- CI: run cargo clippy-cloudflare in the format workflow.
- ec_kv.rs: use prefix.get(..8).unwrap_or(prefix) for the error message so a non-ASCII prefix whose 8th byte falls mid-codepoint cannot panic the slice. - Cargo.toml: move simple_logger after serde_json to restore alphabetical ordering of [workspace.dependencies]. - app.rs: correct the consent-route doc comment — the fail-closed 503 is intentional hardening over legacy route_request (which uses UnavailableKvStore and never opens the consent store), not behavioral parity.
Brings PR15's main-merge progress (multi-backend asset proxy, S3 SigV4 signing, image optimizer primitive, DataDome server-side, settings/docs) up into the PR16 axum dev-server branch. Conflict resolutions: - fastly/app.rs: keep captured tls_protocol/tls_cipher (PR16) plus ..ClientInfo::default() for PR15's new device-signal fields; single publisher import incl. buffer_publisher_response + BoundedWriter. - fastly/main.rs: keep both the trusted x-ts-tls-* header injection (PR16) and derive_device_signals (PR15); drop the now-orphaned fastly-local resolve_publisher_response_buffered in favor of core buffer_publisher_response. - core/proxy.rs: keep PR16's origin_response_metadata/ apply_image_passthrough_metadata helper refactor (superset of PR15's inline image/pixel logic) and restore PR15 #754 IMAGE_FALLBACK_CONTENT_TYPE; union imports; web_time for EC clock, std SystemTime only at the chrono-bound s3 sign_headers call. - core/publisher.rs: dedupe the doubly-merged BoundedWriter, keeping the pub version the adapters import. - core/platform/test_support.rs: merge both StubHttpClient field sets (concurrent_fanout + image-optimizer/stream/method/uri stubs). - core/Cargo.toml: restore the wasm32-unknown-unknown uuid/getrandom js block the auto-merge dropped (required for the Cloudflare build). - axum/cloudflare platform.rs: ..ClientInfo::default() and PlatformBackendSpec.host_header_override for the new fields. - migration_guards.rs: ec/kv.rs and ec/rate_limiter.rs are now fully Fastly-free (required for the wasm32-unknown-unknown core build), so move them into checked_sources and drop the obsolete deferred-EC allowlist. - docs/getting-started.md: template version vars (PR15 #760) + Fastly-optional framing (PR16 axum dev server).
The PR15→PR16 merge restored the wasm32-unknown-unknown getrandom/uuid `js` feature block in trusted-server-core, which adds getrandom (with js-sys and wasm-bindgen) to core's dependency graph. The integration-tests crate is a separate workspace with its own lockfile that did not yet record this, so the --locked integration build failed. Add the missing entries only (no version churn); the shared getrandom 0.2.17 entry now matches the root lockfile.
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.
…t-dual-path' into feature/edgezero-pr15-remove-fastly-core # Conflicts: # crates/trusted-server-adapter-fastly/src/app.rs # crates/trusted-server-adapter-fastly/src/main.rs
…tly-core' into feature/edgezero-pr16-axum-dev-server # Conflicts: # crates/trusted-server-adapter-fastly/src/app.rs # crates/trusted-server-adapter-fastly/src/main.rs # crates/trusted-server-core/Cargo.toml
aram356
requested changes
Jun 22, 2026
aram356
left a comment
aram356
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Round-7 re-review. Since my approval at f7e88462 the PR's scope expanded materially: it now also bundles a new Cloudflare Workers adapter (PR17, ~1500 LoC, target wasm32-unknown-unknown) and a 92-line auction-orchestrator change, none of it previously reviewed. CI is now healthy — cargo fmt, cargo test, cargo test (axum native), cargo check (cloudflare native + wasm32-unknown-unknown), vitest, and EdgeZero integration tests all pass.
The round-6 nitpicks are all fixed (486e62de), and the Axum work I previously approved is unchanged. Requesting changes on the new Cloudflare surface: a redirect-handling SSRF bypass and an admin-route status divergence.
Blocking
🔧 wrench
- SSRF /
allowed_domainsallowlist bypass on the Cloudflare production path —crates/trusted-server-adapter-cloudflare/src/platform.rs:239.RequestInitdefaults toRequestRedirect::Follow, so Workers auto-follows 3xx to any host and core's manual per-hopallowed_domainsenforcement (proxy.rs:1180-1391) is never reached. Axum disables auto-redirect for this exact reason. See inline. - Cloudflare admin key routes return 500 instead of the parity 501 —
crates/trusted-server-adapter-cloudflare/src/app.rs:277. Wired to real handlers whose store writes are unsupported on this runtime; Axum returns a fixed 501. See inline.
Non-blocking
🤔 thinking
- No response-body size cap on the Cloudflare HTTP path —
platform.rs:272; Fastly caps at 10 MiB. See inline. - Cloudflare trusts client scheme headers (
X-Forwarded-Proto/Forwarded); no authoritative scheme injected —platform.rs:486. Bounded impact (EC cookies hardcodedSecure). See inline. execute/send_asyncsilently ignoreimage_optimizer/stream_response—platform.rs:305; the trait contract says reject. See inline.- Hop-by-hop response headers forwarded —
platform.rs:264; parity gap vs Axum. See inline.
🌱 seedling
- S3 SigV4
std::time::SystemTime::now()will panic onwasm32-unknown-unknown—proxy.rs:853. Not currently reachable on Cloudflare (asset proxy handler isn't routed there), but a latent landmine for when asset proxying lands. See inline.
⛏ nitpick
- Sequential-fan-out guard counts raw configured providers, not effective ones —
orchestrator.rs:324. See inline. - Weak Cloudflare route-test assertions + the Cloudflare HTTP client has no tests (wasm-only) —
tests/routes.rs:38. See inline.
Verified non-issues / context
- The Cloudflare-KV-vs-CAS concern does not apply: Cloudflare doesn't implement
EcKvStore, and the EC identity graph (the only CAS consumer) is Fastly-only — no lost-update hazard is introduced. WASM compat is otherwise clean (compile_error!off-target guard,web_time, nostd::net/thread/tokio), no committed secrets (placeholders only), middleware ordering + constant-time basic auth correct on Cloudflare, and the auctionfailed_backend_nameattribution is unchanged and still correct. - All three round-6 nitpicks resolved in
486e62de(ec_kvprefix.get(..8), Cargo.toml ordering, consent-comment reworded).
CI Status
- cargo fmt / cargo test / cargo test (axum native) / cargo check (cloudflare native + wasm32-unknown-unknown) / vitest / integration tests / integration tests (EdgeZero entry point): all PASS on
a50d2931 - Note: the Cloudflare
wasm32-only HTTP client is not exercised by any test target (see the routes.rs nitpick), which is how the redirect bug went unverified.
prk-Jr
changed the base branch from
feature/edgezero-pr15-remove-fastly-core
to
main
Resolve conflicts keeping this branch's full fastly-removal architecture while preserving main's independent improvements: - Cargo.toml / core Cargo.toml: keep tower (axum dep); drop the residual fastly dependency from core (this branch removes it entirely via the EcKvStore platform trait + adapter-local rate limiter, superseding main's allowlist approach). - migration_guards.rs: keep the no-fastly guards; drop main's deferred-EC fastly allowlist tests. - fastly app.rs / main.rs: keep this branch's fastly-in-adapter wiring (ec_kv, rate_limiter, use_finalize_kv, core buffer_publisher_response) and adopt main's full ClientInfo capture into request extensions, which fixes the EdgeZero bot-protection metadata loss for integration request filters (DataDome). - edge_cookie.rs / storage/kv_store.rs: take main's identifier-free consent and EC log messages. - proxy.rs: take main's SVG image-optimizer tests (superset of this branch's). - auction/orchestrator.rs: keep the sequential-fan-out rejection test.
Blocking: - Force manual redirect handling on the Cloudflare fetch path (RequestRedirect::Manual). The Workers runtime defaults to Follow and auto-chases 3xx to any host, bypassing core's per-hop allowed_domains enforcement in proxy_with_redirects (SSRF). Surfaces the 3xx back to core unfollowed, matching the Axum adapter. - Return a fixed 501 from /admin/keys/rotate and /admin/keys/deactivate. Cloudflare's config/secret stores reject writes, so the real handlers surfaced an opaque 500; 501 makes the unsupported-runtime contract explicit, matching Axum. Non-blocking parity/safety: - Cap the buffered upstream response body at 10 MiB (Content-Length pre-check + post-buffer check), mirroring the Fastly adapter, to avoid OOMing the isolate on a large/hostile origin. - Reject image_optimizer and stream_response in the fetch path rather than silently dropping the behavior, honoring the PlatformHttpRequest contract. - Strip hop-by-hop response headers (and Connection-listed tokens) before forwarding, matching the Axum adapter's is_hop_by_hop_response_header. - Derive the S3 SigV4 signing time from the wasm-safe web_time clock so it no longer calls std::time::SystemTime::now(), which panics on wasm32-unknown-unknown (latent once asset proxying lands on Cloudflare). Tests: - Add routes_with_settings/build_router seam to the Cloudflare adapter and assert admin routes return 501 and the tsjs catch-all returns < 500, matching the Axum route-test coverage that would have caught the admin status divergence.
The build script only installed worker-build when none was on PATH, so a
globally installed worker-build 0.8.x was used as-is and rejected the worker
0.7 crate pinned in Cargo.toml ("Unsupported version worker@0.7.x, expected at
least worker@0.8.4"), failing `wrangler dev`. Check the installed major.minor
and reinstall ^0.7 when it is absent or mismatched.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Resolve conflicts from main's crate rename (js→trusted-server-js, openrtb→trusted-server-openrtb, integration-tests→trusted-server-integration-tests), toolchain bump (1.95.0), and the edgezero #269 repin. Key resolutions: - Keep the branch's pinned edgezero rev 38198f9 and fastly 0.11.12. That rev's Body::into_bytes returns Bytes (not Option) and the Fastly 0.11 TLS getters return Option directly, so main's #269 glue (.into_bytes().unwrap_or_default(), .ok().flatten() on TLS getters, oneshot().expect()/?) is reverted to the branch's API shape. Forward repin to fastly 0.12 is deferred to its own PR. - Adopt toolchain 1.95.0 but keep both wasm targets (Cloudflare needs wasm32-unknown-unknown). - Take the platform-neutral EC KV rewrite (ec/kv.rs) over main's Fastly-coupled version; main's changes there were cosmetic. - Carry main's new features onto the branch: tester-cookie endpoints (/_ts/set-tester, /_ts/clear-tester) wired into the Fastly router, and the osano integration. - Fix the Cloudflare adapter's trusted-server-js path after the rename, drop the leftover renamed-away crate dirs, and restore repo-wide node_modules/dist ignores (the old crates/js/.gitignore was removed by the rename). All adapters build, lint, and test green (fastly+core, axum, cloudflare).
aram356
approved these changes
Jun 29, 2026
aram356
left a comment
aram356
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Round-8 re-review. The PR now targets main directly. Both blocking findings from my last review are fixed and independently verified, and the remaining non-blocking items are either fixed or deferred with sound, bounded-impact rationale. CI is green across every target (cargo fmt, cargo test, cargo test (axum native), cargo check (cloudflare native + wasm32-unknown-unknown), vitest, browser + integration + EdgeZero-entry-point integration tests, CodeQL). Approving.
Verified fixes (7f5c89c89)
- 🔧 SSRF /
allowed_domainsredirect bypass —executenow setsRequestRedirect::Manual, so the Workers runtime no longer auto-follows 3xx; the redirect surfaces back to core'sproxy_with_redirectsfor per-hop allowlist validation, matching Axum'sredirect::Policy::none(). - 🔧 Admin key routes —
/admin/keys/rotateand/admin/keys/deactivatenow return a fixed501viaadmin_not_supported_response(); route tests assert== 501. - 🤔 Response-body cap — 10 MiB two-stage cap (Content-Length pre-check + post-buffer check), mirroring Fastly.
- 🤔
image_optimizer/stream_response— now rejected up front with a typed error per thePlatformHttpRequestcontract. - 🤔 Hop-by-hop headers — stripped via
is_hop_by_hop_response_header+ Connection-token list. - 🌱 S3 SigV4 wasm panic — signing time derived from
web_timearithmetic (UNIX_EPOCH + now().duration_since(...)), never calling the panickingstd::time::SystemTime::now(). - ⛏ Cloudflare route tests —
routes_with_settingsseam +== 501/< 500assertions.
The fresh full-diff pass found no new blocking issues. The remaining net-diff surface beyond prior rounds is mechanical (the edgezero into_bytes() API migration across datadome/prebid/testlight/auction/formats/route_tests) or sound (the new check-integration-dependency-versions.sh). The Cloudflare crate builds cleanly for wasm32-unknown-unknown.
Tracked follow-ups (non-blocking)
- 📌 Cloudflare scheme hardening — derive scheme from the trusted
cf-visitorheader and strip spoofableX-Forwarded-Proto/Forwarded. Bounded today (EC cookies hardcodedSecure). See inline. - 📌 Native coverage for the wasm-only HTTP client — extract redirect/cap/header logic into native-testable functions. See inline.
CI Status
- cargo fmt / cargo test / cargo test (axum native) / cargo check (cloudflare native + wasm32-unknown-unknown) / vitest / integration tests / integration tests (EdgeZero entry point) / browser integration tests / CodeQL: all PASS on
2aa73301.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
trusted-server-adapter-axumas a native (non-WASM) dev server so the full trusted-server pipeline can be run and tested locally without Fastly Compute or Viceroytarget = "wasm32-wasip1"override from.cargo/config.toml; Fastly-specific commands now pass--target wasm32-wasip1explicitlyChanges
crates/trusted-server-adapter-axum/src/platform.rsPlatformConfigStore,PlatformSecretStore,PlatformBackend,PlatformGeo,PlatformHttpClient— env-var-backed implementationscrates/trusted-server-adapter-axum/src/middleware.rsFinalizeResponseMiddleware+AuthMiddleware— mirrors Fastly adapter, always emitsX-Geo-Info-Available: falsecrates/trusted-server-adapter-axum/src/app.rsTrustedServerAppimplementingHookswith all 11 routes wiredcrates/trusted-server-adapter-axum/src/main.rs+axum.tomlcrates/trusted-server-adapter-axum/tests/routes.rsEdgeZeroAxumService(no live TCP server)crates/integration-tests/tests/environments/axum.rsAxumDevServerruntime environment added to the matrixcrates/integration-tests/tests/environments/mod.rsAxumDevServeralongsideFastlyViceroycrates/integration-tests/tests/integration.rstest_wordpress_axum+test_nextjs_axumindividual test functionsscripts/integration-tests.sh.cargo/config.tomltarget = "wasm32-wasip1"; keep only the viceroy runnerCargo.tomltrusted-server-adapter-axumfrom[exclude]to[members]crates/trusted-server-adapter-axum/Cargo.toml.github/workflows/test.ymltest-axumCI job;test-rustnow passes--target wasm32-wasip1explicitlyCLAUDE.md.gitignore(root + adapter).edgezero/(local KV store created by dev server)Closes
Closes #497
Test plan
cargo test --workspace(Fastly/WASM crates via Viceroy)cargo test -p trusted-server-adapter-axum(8 route + middleware tests)cargo clippy --workspace --all-targets --all-features -- -D warningscargo fmt --all -- --checkcd crates/js/lib && npx vitest run(282 tests)test_wordpress_fastly,test_nextjs_fastly,test_wordpress_axum,test_nextjs_axumall passcargo run -p trusted-server-adapter-axumstarts on port 8787Checklist
unwrap()in production code — useexpect("should ...")logmacros (notprintln!)