Add Fermyon Spin adapter (wasm32-wasip1)#735
Merged
Merged
Conversation
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.
- Revert proxy.rs merge artifact: restore per-request allowed_domains at both redirect_is_permitted call sites; remove dead_code allow and stale comment — integration proxies defaulting to &[] get open mode again as documented - Drop unused trusted-server-js dep from adapter Cargo.toml - Fix check_response: gate body read behind error branch so 2xx paths do not buffer and discard the response body - Remove self-referential SECRET_UPSERT_METHOD test - Reorder write-cost doc so outbound HTTPS round-trip leads; handle-open caching noted as negligible - Refactor make_request to take fastly::http::Method; drop string match and unreachable arm; remove SECRET_UPSERT_METHOD const - Add SigningStoreIds named struct in endpoints.rs; update both call sites to destructure by name
…tion - Add VERIFY_MAX_BODY_BYTES (4096) cap to handle_verify_signature - Add AUCTION_MAX_BODY_BYTES (65536) cap to handle_auction - Add ADMIN_MAX_BODY_BYTES (4096) cap to handle_rotate_key and handle_deactivate_key - Delete platform_response_to_fastly from orchestrator; both call sites now use crate::compat::to_fastly_response directly - Add sign_rejects_oversized_body and rebuild_rejects_oversized_body regression tests asserting 413 on oversized POST bodies
… compat conflicts
…EADME - Run cargo fmt --all (7+ files had formatting failures) - Cap testlight POST body with collect_body_bounded + INTEGRATION_MAX_BODY_BYTES - Use dedicated AUCTION_MAX_BODY_BYTES (256 KiB) for /auction instead of INTEGRATION_MAX_BODY_BYTES - Update auction/README.md: parse_response is now async, parallel execution via select() is live
… collect_body_bounded
…thods FinalizeResponseMiddleware now absorbs errors from inner middleware/handlers by converting EdgeError to a Response via IntoResponse, so apply_finalize_headers always runs regardless of handler outcome. Geo lookup is moved after next.run and skipped for UNAUTHORIZED responses to avoid unnecessary backend calls. Register HEAD and OPTIONS catch-all routes so cache-validation and CORS preflight requests reach the publisher origin instead of returning 405. Update module docstring to note startup_error_router skips middleware.
…mpat 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
… bypass, refactors
Blocking:
- Remove unused serde_json and trusted-server-js from Cargo.toml
- Delete crate-local Cargo.lock (now silently ignored as workspace member)
- Fix CLAUDE.md workspace layout comment (drop "excluded from workspace")
- Fix log message naming NoopKvStore → UnavailableKvStore in build_runtime_services
- Wrap startup_error_router with FinalizeResponseMiddleware(Settings::default()) so
startup-error responses carry X-Geo-Info-Available and operator response_headers
Refactor:
- Move AxumPlatformHttpClient to AppState; share Arc across requests via
build_runtime_services(ctx, Arc::clone(&state.http_client)) to preserve
the reqwest connection pool
- Remove build_per_request_services no-op wrapper; inline at call sites
- Use temp_env::with_var in config/secret store tests for proper isolation
- Replace .unwrap() with .expect("should ...") throughout test code per CLAUDE.md
Nitpick:
- Delete redundant crate-local .gitignore (covered by workspace .gitignore)
…ce breadcrumbs Port fix: - main.rs reads PORT env var at startup; when set, uses AxumDevServer::with_config with a dynamic SocketAddr instead of the run_app default (hardcoded 8787) - Integration test spawner (axum.rs) now calls find_available_port() and passes PORT=<port> to the child process, matching the Fastly env's dynamic-port pattern - Fallback to AXUM_DEFAULT_PORT=8787 if find_available_port fails (offline runner) Divergence breadcrumbs: - send_async: debug log noting that execution is eager and errors surface immediately, not at select() time as they do on Fastly - select: debug log noting that index 0 is popped unconditionally (sequential, not first-to-complete) — any fan-out ordering tests should use the Fastly runtime
…ro-pr15-remove-fastly-core
…feature/edgezero-pr12-handler-layer-types
…into feature/edgezero-pr10-abstract-logging-initialization
…into feature/edgezero-pr11-utility-layer-migration-v2 Resolve conflicts by adopting PR10's ec_id naming throughout. cookies.rs set_ec_cookie/expire_ec_cookie retain Response<EdgeBody> types to satisfy migration_guards; Fastly-typed callers route through compat bridge. Remove synthetic.rs (deleted in PR10) and its migration_guards entry.
cookies.rs set_ec_cookie/expire_ec_cookie now take Response<EdgeBody> to satisfy the migration_guards invariant. registry.rs and publisher.rs call the Fastly-typed equivalents in compat instead. Remove synthetic.rs entry from migration_guards (file deleted in PR10).
…into feature/edgezero-pr11-utility-layer-migration-v2
…feature/edgezero-pr12-handler-layer-types
…/edgezero-pr13-integration-provider-type-migration
…on' into feature/edgezero-pr14-entry-point-dual-path
…erver' into feature/edgezero-pr18-phase5-verification
The legacy `/admin/keys/*` aliases were dropped from the route tables, which let them fall through to the publisher fallback. That fallback forwards the request — including the `Authorization` header and key-management body — to the publisher origin, leaking Trusted Server admin Basic credentials to origin logs. Fail closed locally instead: register the aliases to a 404 deny handler on every router (Fastly legacy `route_request`, Fastly EdgeZero, Axum, Cloudflare). The production basic-auth regex `^/_ts/admin` does not match these paths, so they are not auth-gated; denying them locally keeps admin credentials and key payloads off the wire. Canonical `/_ts/admin/keys/*` handling is unchanged. Adds regression tests on every router that POST a legacy alias with an `Authorization` header and assert a local 404 (not a publisher-proxy 5xx).
The Cloudflare local-build guard detects a non-0.7 `.worker-build/bin/worker-build` but reinstalled without `--force`, so `cargo install` refused to overwrite the existing binary and the script kept failing until the developer manually removed `.worker-build`. Add `--force` so the recovery path self-heals for wrangler dev.
- Satisfy clippy on the VICEROY_CONFIG_PATH override in the Viceroy environment helper: backtick `EdgeZero` in the doc comment and collapse the nested `if` into a let-chain. - Update the cross-adapter parity guard for the legacy `/admin/keys/*` aliases: they are now registered to a local 404 deny (not unrouted), so assert the aliases ARE registered and that a POST returns 404 with no auth challenge, rather than falling through to the publisher fallback.
…ification' into feature/edgezero-pr19-spin-adapter # Conflicts: # .github/workflows/format.yml # crates/trusted-server-adapter-cloudflare/build.sh # crates/trusted-server-adapter-fastly/src/platform.rs # crates/trusted-server-adapter-fastly/src/route_tests.rs
The Axum and Cloudflare integration-test builds set the removed TRUSTED_SERVER__EDGE_COOKIE__SECRET_KEY, which no settings key reads, so those bundles embedded the committed local-dev passphrase instead of the integration passphrase and could not catch cross-runtime EC regressions. Use the real TRUSTED_SERVER__EC__PASSPHRASE (32-byte value) the Fastly build already uses. Also give the Spin release-build step the same publisher/EC/proxy overrides so the artifact embeds usable settings and boots instead of falling back to the generic 503 router on placeholder-secret rejection.
normalize_spin_request re-derived a trusted client IP from the synthetic spin-client-addr but left any inbound x-forwarded-for in place. Shared proxy code forwards that header to origins and Prebid copies it into its outbound request, so a Spin client could spoof the IP seen by downstream services while auction device.ip used the trusted runtime address. Strip x-forwarded-for during normalization — Spin exposes no trusted forwarded-for chain. Adds a regression test with a spoofed X-Forwarded-For and a trusted spin-client-addr.
handle_first_party_proxy_rebuild rejected click targets whose host fell outside proxy.allowed_domains, but proxy.allowed_domains governs server-side proxy fetch redirect chains, not advertiser click 302s — and handle_first_party_click itself redirects any valid signed target without consulting it. Applying it only during rebuild was a cross-adapter regression for deployments that restrict proxy fetch destinations while allowing normal signed click redirects. The original signed click URL (including the immutable reserved tsurl parameter) is already validated in the rebuild, so the redirect host is fixed by the validated original.
ChristianPavilonis
requested changes
Jun 20, 2026
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Automated review: I reviewed PR #735 against feature/edgezero-pr18-phase5-verification at head b394581253790c6ab91d6cb7d103a29f3b6dce1b, focusing on the current Spin adapter, route/auth behavior, request normalization/header trust, platform services, proxy/signing changes, parity tests, and CI/tooling. CI is green and the prior blocking threads mostly appear addressed, but I found one remaining security regression that should block merge: Spin leaves the legacy /admin/keys/* aliases to fall through to the publisher fallback, unlike the Fastly/Axum/Cloudflare local-deny behavior added on this branch. That can forward admin Authorization headers and key-management payloads to the publisher origin. I left the actionable finding inline.
CI / Existing Reviews:
gh pr checks 735: all reported checks pass (fmt, Rust tests, cross-adapter parity, Spin native + wasm32-wasip1 check/build, Cloudflare checks, vitest, docs/TS formatting, integration/browser tests).- Existing review feedback already covered and appears to have fixed the earlier Spin variable encoder, outbound timeout docs, startup fallback, first-party URI normalization, fallback methods, proxy-rebuild signing controls, legacy KID removal, response-size limits, Host/full-url/client-IP normalization, health route, worker-build pinning, and previous legacy-admin alias key-handler exposure. This comment is for the remaining fallthrough-to-origin leak after those fixes.
aram356
approved these changes
Jun 22, 2026
aram356
left a comment
aram356
left a comment
Collaborator
There was a problem hiding this comment.
Re-review — all prior findings resolved
This pass re-reviews head b3945812 against the prior REQUEST_CHANGES review (head 3e6fbd90). Every blocking and non-blocking item from that review is resolved, and one additional P1 was self-identified and fixed:
- Normalization is now a structural chokepoint —
NormalizeMiddlewarerunsnormalize_spin_requeston every routed request (verified: edgezero applies middleware per-dispatch across named routes,/, and the/{*rest}publisher fallback); the per-handler opt-in calls are removed. - Client-IP spoofing (P1) fixed — the trusted IP is re-derived from the last
spin-client-addrand written intoSpinRequestContextbeforebuild_runtime_servicesreads it;x-forwarded-foris stripped (necessary — Prebid'scopy_request_headersforwards inbound XFF and core'sSPOOFABLE_FORWARDED_HEADERSexcludes it). - SSRF allowlist removed from click rebuilds (
b3945812) — sound:proxy.allowed_domainsgoverns server-side proxy fetch SSRF, not advertiser click 302s, and thetsurltarget is a reserved, signature-validated, immutable parameter. - Outbound wildcard documented, KV-TTL error semantics clarified, client-IP topology assumption documented, userinfo stripped, CI job renamed, redundant
--no-default-featuresremoved.
Auth-before-normalize is safe (auth matches on path only, which normalize never rewrites), the context overwrite propagates (no clone), the response-size ceiling still bounds every path, and there are no production-path panics. CI: all 13 checks green.
Approving. One optional, non-blocking hardening note inline.
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.
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Automated review: I re-reviewed PR #735 against feature/edgezero-pr18-phase5-verification, including the completed subagent passes for Spin routing/security, Spin platform services, and core/CI coverage.
CI is green and many prior review threads appear addressed. I’m submitting this as COMMENT rather than approval because I still found high-confidence issues that should be fixed before treating the Spin adapter as production-safe: legacy admin aliases fail open to publisher fallback, real Spin response conversion can collapse duplicate headers such as Set-Cookie, and the Spin HTTP client silently ignores core Image Optimizer / streaming-response adapter contracts. I also left one medium compatibility comment for encoded no-body responses.
CI / Existing Reviews
gh pr checks 735: all reported checks pass (fmt, Rust tests, cross-adapter parity, Spin native + wasm32-wasip1 check/build, Cloudflare checks, vitest, docs/TS formatting, integration/browser tests).- Existing review threads on request normalization, client-IP spoofing, response-size limits, KID validation, proxy-rebuild signing controls, startup fallback, and timeout docs appear addressed at current HEAD.
ChristianPavilonis
approved these changes
Jun 22, 2026
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).
Resolve PR19 conflicts against the latest PR18 base. Keep PR18's renamed crate layout/toolchain updates while preserving PR19's Spin workspace member, CI checks, route parity, and worker 0.8 compatibility. Combine Fastly TLS header hardening with PR18's client-info extraction helper, and update Spin/integration-test paths for the trusted-server-js rename.
…ezero-pr18-phase5-verification # Conflicts: # .github/workflows/integration-tests.yml
…ezero-pr18-phase5-verification # Conflicts: # crates/trusted-server-integration-tests/Cargo.lock
…/edgezero-pr19-spin-adapter # Conflicts: # crates/trusted-server-integration-tests/Cargo.lock
prk-Jr
changed the base branch from
feature/edgezero-pr18-phase5-verification
to
main
Keep the branch's evolved tree (fastly 0.12, worker 0.8, edgezero ce6bcf74, Spin adapter), which already supersedes main's PR16/PR18 content carried via the stack's merge history. Incorporate main's two new fixes that the branch lacked: - #819 root-relative first-party integration URLs (datadome, gpt, lockr, permutive, sourcepoint) - #822 Sourcepoint first-party consent iframe/same-origin guard Auto-merge picked main's older fastly-0.11 TLS code for the fastly adapter (main.rs, platform.rs, route_tests.rs); restore the branch versions so the get_tls_protocol/cipher calls match the locked fastly 0.12 API. Verified: cargo check-{fastly,cloudflare,axum,spin} --locked all pass.
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
3 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-spin— the Fermyon Spin entry point (wasm32-wasip1component) usingedgezero_adapter_spin::run_appwith Spin-backedRuntimeServices(config store, KV, secrets, buffered proxy client with gzip/br decompression)ce6bcf74(addsedgezero-adapter-spin), Fastly SDK to0.12, Cloudflareworkerto0.8, and Viceroy to0.16.5to resolve bot-analysis hostcall breakage introduced by the SDK bumpChanges
crates/trusted-server-adapter-spin/edgezero.toml,spin.toml, platform runtime services, route/auth smoke tests, EdgeZero manifest validation testCargo.toml/Cargo.locktrusted-server-adapter-spin; bump EdgeZero rev,fastly 0.12,log-fastly 0.12,worker 0.8crates/integration-tests/Cargo.toml/tests/parity.rs.cargo/config.tomltest-spin,check-spin,clippy-spin-native,clippy-spin-wasmaliases.github/workflows/test.ymlwasm32-wasip1CI jobs.github/workflows/format.yml.github/actions/setup-integration-test-env/action.yml0.16.5.tool-versions0.16.5CLAUDE.mdcrates/trusted-server-adapter-fastly/src/{main,platform,route_tests}.rsfastly 0.12API surfacecrates/trusted-server-adapter-cloudflare/Cargo.tomlworkerto0.8docs/superpowers/specs/2026-03-19-edgezero-migration-design.mdCloses
Closes #732
Known MVP limits
cargo build --workspaceintentionally fails (mixedwasm32-wasip1/wasm32-unknown-unknowntargets); target-matched aliases are the correct verification pathTest plan
cargo fmt --all -- --checkcargo check(native)cargo check-spin(wasm32-wasip1)cargo build --package trusted-server-adapter-spin --target wasm32-wasip1 --features spin --releasecargo test-spin(route/auth smoke tests, native host)cargo test-fastly(Viceroy 0.16.5)cargo test-axumcargo test-cloudflarecargo test --manifest-path crates/integration-tests/Cargo.toml --test parity(Fastly + Cloudflare + Spin)cargo clippy-fastly,clippy-axum,clippy-cloudflare,clippy-spin-native,clippy-spin-wasmcargo clippy --manifest-path crates/integration-tests/Cargo.toml -- -D warningscargo check -p trusted-server-adapter-fastly --target wasm32-wasip1cargo check -p trusted-server-adapter-cloudflare --target wasm32-unknown-unknown --features cloudflaregit diff --checkspin up --from crates/trusted-server-adapter-spin(requires local Spin CLI install)Checklist
unwrap()in production code — useexpect("should ...")logmacros (notprintln!)