Skip to content

Add path-based asset backend proxy routing#668

Open
ChristianPavilonis wants to merge 7 commits intomainfrom
feature/multi-backend-asset-proxy
Open

Add path-based asset backend proxy routing#668
ChristianPavilonis wants to merge 7 commits intomainfrom
feature/multi-backend-asset-proxy

Conversation

@ChristianPavilonis
Copy link
Copy Markdown
Collaborator

@ChristianPavilonis ChristianPavilonis commented Apr 28, 2026

Summary

  • Add configurable [[proxy.asset_routes]] path-prefix routing so selected first-party asset paths can proxy to a different backend origin than publisher.origin_url.
  • Route matching is transparent for normal inbound requests, limited to GET/HEAD, uses longest-prefix-wins, and preserves the full incoming path/query while swapping only the origin.
  • Keep built-in and integration routes ahead of asset routes, and bypass the publisher consent/cookie/rewrite pipeline for matched asset requests by serving them as lean raw pass-through proxies.

Changes

File Change
docs/superpowers/specs/2026-04-28-multi-backend-asset-proxy-design.md Add the detailed design/spec for path-based multi-backend asset proxy routing.
crates/trusted-server-core/src/settings.rs Add ProxyAssetRoute, proxy.asset_routes config support, asset-route normalization/validation, duplicate-prefix warnings, and longest-prefix matching helpers.
crates/trusted-server-core/src/proxy.rs Add the raw asset proxy handler and target URL / Host header helpers for forwarding matched asset requests to alternate origins.
crates/trusted-server-adapter-fastly/src/main.rs Wire asset-route matching into top-level request routing after explicit routes and before publisher fallback.
crates/trusted-server-adapter-fastly/src/route_tests.rs Add route-precedence and consent-bypass tests for asset-route behavior.
trusted-server.toml Document the new [[proxy.asset_routes]] configuration shape with a commented example.

Closes

Closes #663

Test plan

  • 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
  • JS format: cd crates/js/lib && npm run format
  • Docs format: cd docs && npm run format
  • WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
  • Manual testing via fastly compute serve
  • Other:

Checklist

  • Changes follow CLAUDE.md conventions
  • No unwrap() in production code — use expect("should ...")
  • Uses tracing macros (not println!)
  • New code has tests
  • No secrets or credentials committed

@ChristianPavilonis ChristianPavilonis marked this pull request as ready for review April 29, 2026 20:12
@aram356 aram356 assigned aram356 and ChristianPavilonis and unassigned aram356 Apr 29, 2026
Copy link
Copy Markdown
Collaborator

@aram356 aram356 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Path-based asset proxy routing closely follows the spec, with a clean separation from the publisher pipeline and a conservative forwarded-header set. Concerns are concentrated around (1) origin_url validation that lets path/query through silently, (2) missing test coverage for spec-stated invariants — most importantly that asset failures must not silently fall back to publisher.origin_url, and (3) one CLAUDE.md doc-comment compliance gap.

Blocking

🔧 wrench

  • Missing doc comments on ProxyAssetRoute (crates/trusted-server-core/src/settings.rs:336-339)
  • origin_url validation accepts URLs with path/query that are silently dropped at runtime (crates/trusted-server-core/src/settings.rs:740-766)
  • No test that asset-origin failures stop at 502 without falling back to publisher.origin_url (crates/trusted-server-adapter-fastly/src/route_tests.rs, around the asset_routes_* tests) — spec §9 forbids the fallback; the impl is correct but unpinned by tests.

Non-blocking

🤔 thinking

  • Strict-Transport-Security from asset origin forwarded unchanged (crates/trusted-server-core/src/proxy.rs:657-663) — same class of risk as upstream Set-Cookie, which is correctly stripped. Either strip HSTS too or document explicitly.
  • Spec-listed test gaps: HEAD passthrough, non-GET/HEAD bypass, redirect pass-through, query-string-ignored-for-matching. All small additions given existing helpers.
  • String-prefix matching can match non-segment boundaries (prefix = "/static" matches /staticfile.js). Spec is explicit, but every example uses a trailing /. Worth a sentence in the toml comment and field doc.

♻️ refactor

  • Idiomatic method-set check in route_request (crates/trusted-server-adapter-fastly/src/main.rs:153-156) — replace the match &method { &Method::GET | &Method::HEAD => ... } with matches!(...).then(...).flatten().
  • Test-stub duplication: StaticResponseHttpClient could move into platform::test_support by extending StubHttpClient with push_response_with_headers.
  • Validation split between Proxy::normalize and Proxy::prepare_runtime is inconsistent with the rest of Settings, which uses #[validate] attributes. Either consolidate or comment the rationale.

🌱 seedling

  • WASM heap pressure for large asset bodies (crates/trusted-server-adapter-fastly/src/platform.rs:223-237fastly_response_to_platform calls take_body_bytes(); crates/trusted-server-core/src/proxy.rs:657 re-buffers via set_body(Vec<u8>)). Same as PublisherResponse::PassThrough, so not a regression — but this PR is specifically targeting asset traffic, where bodies are routinely larger than HTML. Track as a streaming pass-through follow-up mirroring PublisherResponse::Stream. (Anchored in the body since platform.rs is unchanged in this PR.)

⛏ nitpick

  • Validation error wording (crates/trusted-server-core/src/settings.rs:716, validate_no_trailing_slash): the spec says "must not include a trailing slash"; the error message says "origin_url must not end with '/'". Aligning the wording reads slightly better in operator-facing errors. The function itself is unchanged in this PR but is reached via the new validate_proxy_origin_url. (Anchored in the body since the line is outside the diff.)
  • Redundant to_ascii_lowercase() on target_url.scheme() — already lowercase from URL parsing. See inline comment.
  • Set-Cookie strip test only verifies a single header is removed; should test multiple. See inline comment.

CI Status

  • fmt: PASS
  • clippy: PASS
  • rust tests: PASS (858 tests, 0 failures locally; CI green)
  • vitest: PASS
  • format-typescript: PASS
  • format-docs: PASS
  • browser & integration tests: PASS
  • CodeQL / Analyze: PASS

Comment thread crates/trusted-server-core/src/settings.rs
Comment thread crates/trusted-server-core/src/settings.rs
Comment thread crates/trusted-server-adapter-fastly/src/route_tests.rs
Comment thread crates/trusted-server-core/src/proxy.rs
Comment thread crates/trusted-server-adapter-fastly/src/route_tests.rs
Comment thread crates/trusted-server-adapter-fastly/src/main.rs Outdated
Comment thread crates/trusted-server-core/src/proxy.rs Outdated
Comment thread crates/trusted-server-core/src/settings.rs
Comment thread crates/trusted-server-core/src/proxy.rs Outdated
Comment thread crates/trusted-server-core/src/proxy.rs
@ChristianPavilonis ChristianPavilonis requested a review from aram356 May 5, 2026 12:43
@ChristianPavilonis
Copy link
Copy Markdown
Collaborator Author

Tested proxying, it does work, however proxying to a s3 bucket that requires authentication isn't implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

As publisher i want to support proxying assets from different backend from content backend

2 participants