Rewrite publisher origin URLs in proxied response headers

[dhruv8sh]

Summary

• Publisher pages had origin URLs rewritten in the body but not in response headers. Browsers then followed Location/Refresh off the appliance, preloaded Link targets straight from the origin, and blocked rewritten resources under a CSP that only allowed the origin.
• Response headers now get the same origin-to-serving-host mapping as the body. Location, Content-Location, Refresh and Link (including imagesrcset) are rewritten. For CSP, a serving-host source is added beside each origin source, and no origin source is removed. Each response's own header values are rewritten, so per-page policies survive; this is not one static replacement.
• An origin redirect to the current URL under a different scheme (for example, an http:// origin forcing HTTPS) is left unchanged so it can't loop. Same-scheme redirects to the current URL, such as after a form POST, are still rewritten.

Changes

File	Change
crates/trusted-server-core/src/response_header_rewrite.rs	New module that rewrites URLs in Location, Content-Location, Refresh, Link (<target> and quoted imagesrcset) and CSP/CSP-Report-Only. CSP is additive with de-duplication, and report-uri/report-to are skipped. Includes the scheme-change loop guard and unit tests.
crates/trusted-server-core/src/publisher.rs	Calls the rewrite on every publisher route, right after the origin response and before the template-cache gate, so cached templates replay the rewritten policy headers. Adds handler tests: a 302 with an HTML body, a bodiless 301, scheme-upgrade and same-scheme redirects to the current URL, and a template-cache hit replaying the rewritten CSP/Link. Updates two schema-version pins.
crates/trusted-server-core/src/platform/template_cache.rs	Bumps TEMPLATE_SCHEMA_VERSION from 5 to 6, so templates stored with origin-only policy metadata are not replayed after deploy. Updates the golden-key test.
crates/trusted-server-core/src/lib.rs	Registers the module (pub(crate)).
docs/guide/configuration.md	Documents the header behavior, the loop exception, that [response_headers] still overrides the rewrite, and that host rewriting cannot authorize inline inserts that lack a nonce.

Closes

Closes #1128

Test plan

• cargo test-fastly && cargo test-axum (also cargo test-cloudflare, cargo test-spin, and the parity integration test)
• cargo clippy-fastly && cargo clippy-axum (also clippy-cloudflare, clippy-cloudflare-wasm, clippy-spin-native, clippy-spin-wasm)
• cargo fmt --all -- --check
• JS tests: cd crates/trusted-server-js/lib && npx vitest run (Node 24.12.0, per .tool-versions)
• JS format: cd crates/trusted-server-js/lib && npm run format
• Docs format: cd docs && npm run format (plus the markdown check outside docs/)
• WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
• Manual testing via fastly compute serve
• Other: mutation check. Moving the rewrite after the template-cache policy capture makes the cache-hit replay test fail.

Not yet done: re-running the issue's browser checks (Chrome against the Axum dev server) for Location, Refresh, Link preload and the CSP img-src case.

Checklist

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

[prk-Jr]

Summary

Solid, well-tested change. Rewrites origin URLs in Location, Content-Location, Refresh, Link, and both CSP headers; Set-Cookie/Vary untouched. Host matching is strict and case-insensitive, multi-value headers preserve order, and the rewrite runs before template-cache capture (with the schema bump to 6) in the shared handle_publisher_request path used by all adapters. No blocking findings.

Non-blocking

• 🤔 Loop-guard escape logs only at debug (inline, suggestion)
• 🤔 Same-URL cookie-setting redirect can loop when the cookie has Domain=<origin host> (inline)
• 🌱 CSP :* / explicit default-port sources not matched (inline)
• ⛏ Docs row omits report-to (inline, suggestion)

📌 Out of scope

Set-Cookie Domain, Reporting-Endpoints/Report-To, and Access-Control-Allow-Origin still pass through unchanged. Leaving report endpoints alone matches the report-uri choice; the Set-Cookie Domain gap (see inline comment) is worth a follow-up issue. Asset-proxy routes (handle_asset_proxy_request) also bypass this rewrite, consistent with the docs saying "publisher response".

📝 Note

The PR checklist says "Uses tracing macros", but the code correctly uses log — only the checklist wording is off.

Verification

• cargo fmt --all -- --check ✅
• cargo clippy-fastly ✅ (no warnings)
• cargo test -p trusted-server-core (host) ✅ 2916 passed, 0 failed (includes 18 new response_header_rewrite tests and 5 new publisher tests)
• Both suggestions scratch-verified (fmt/clippy/tests; docs prettier check)
• CI: all 20 checks passing

[ChristianPavilonis]

Review summary

Reviewed 7871acdd56324d20c182e0c3093e5bd535dd96e9 against 80483011e30a446ac741b4423e36f8d142c5d2ac, branch fix/rewrite-origin-urls-in-response-headers into main. Approving with two P2 edge-case findings posted inline.

Inspected all five changed files and traced the rewrite through publisher response modes, cache capture/replay, all four adapters, and final header overrides. Cache-hit, repeated-policy-header, and previous-schema rejection tests confirm rewritten policy replay for the tested core cache implementation.

Validation and review context

• cargo test -p trusted-server-core response_header_rewrite --locked: 18 passed. Initial compilation timed out; rerun completed.
• cargo test -p trusted-server-core publisher::tests --locked: 372 passed.
• cargo test -p trusted-server-core platform::template_cache --locked: 32 passed.
• cargo test -p trusted-server-core --locked --quiet: 2,916 unit tests passed; 4 doctests passed, 5 ignored.
• cargo fmt --all -- --check: passed.
• An exact-module scratch executable and two node --input-type=module Chromium fixtures reproduced the inline findings.
• CI: All 20 reported checks passed; none reported failed or skipped.
• Existing feedback: Inspected the review, inline comments, unresolved threads, replies, and issue comments. Neither finding duplicates existing feedback.
• Residual risk: Browser fixtures exercised the exact rewrite module, not a deployed adapter. Adapter suites, production Fastly cache, and lint checks were not rerun locally.
• Revision remained unchanged; working tree is clean. No repository files were edited and no review work was delegated.

[aram356]

Summary

Rewrites publisher-origin URLs in Location, Content-Location, Refresh, Link, and CSP headers so the browser stays on the serving host (closes #1128). The rewrite holds up in a real browser, but the branch now conflicts with main after #1210, and #1210 changes the right resolution for the schema bump.

2 of the inline comments below carry a one-click GitHub suggestion. Use Commit suggestion (or Add suggestion to batch for both) to apply them as commits on the PR branch. The remaining comments describe the fix in prose because the change touches multiple files or lines outside the diff and can't be auto-applied.

Blocking

🔧 wrench

• Conflicts with main; drop the v6 schema bump: see inline at crates/trusted-server-core/src/platform/template_cache.rs:41

Non-blocking

🤔 thinking / 📌 out of scope / ⛏ nitpick

• 🤔 A Host override turns a canonical-host redirect into a loop: see inline at crates/trusted-server-core/src/response_header_rewrite.rs:92
• 🤔 A serving-host CSP source also allows /first-party/proxy (suggestion): see inline at docs/guide/configuration.md:828
• 📌 <meta http-equiv> refresh and CSP in the page body are not rewritten: see inline at docs/guide/configuration.md:783
• ⛏ Allocate lazily in rewrite_link (suggestion): see inline at crates/trusted-server-core/src/response_header_rewrite.rs:296
• ⛏ Reuse request_path_and_query: see inline at crates/trusted-server-core/src/publisher.rs:4523

Cross-cutting / body-level findings

• 🏕 CHANGELOG entry: operators upgrading will see changed Location values and wider CSP headers on proxied pages. A suggested line under ### Fixed:

  • Publisher responses now map origin URLs in Location, Content-Location, Refresh, and Link headers to the serving host, and each CSP source naming the origin gains a serving-host source beside it. A redirect to the current URL under a different scheme is left unchanged to avoid a loop. See "Origin URLs in proxied response headers" in the configuration guide.

Verification

• Browser checks from the test plan's unchecked item, run with the Axum dev server and Chrome at 0d0689aca. Location, Refresh, and the Link preload stay on the serving host, and the origin log shows no direct hits. In the CSP img-src case, the rewritten image loads with no violation. The scheme-upgrade redirect is left unchanged, with one hop and no loop. On main, the same headers pass through unrewritten.
• A scratch merge with origin/main (da31a215e), resolved as described in the 🔧 comment, passes fmt, clippy-fastly, test-fastly (2,919 core tests), test-axum, test-cloudflare, test-spin, and parity.
• Both suggestions are scratch-verified. The docs one passes the docs prettier check. The Rust one passes fmt, all six adapter clippy aliases, the four adapter test suites, and parity.

CI Status

All checks ran on head 0d0689aca, which predates #1210. Nothing has run on a merge with current main.

• Analyze (actions): PASS
• Analyze (javascript-typescript): PASS
• Analyze (javascript-typescript): PASS
• Analyze (python): PASS
• Analyze (rust): PASS
• browser integration tests: PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo fmt: PASS (required)
• cargo test: PASS (required)
• cargo test (axum native): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native) (macos-latest): PASS
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• CLAUDE.md symlink guard: PASS
• CodeQL: PASS
• format-docs: PASS (required)
• format-typescript: PASS (required)
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• prepare integration artifacts: PASS
• vitest: PASS

[aram356]

🔧 wrench — This branch conflicts with main after #1210 (mergeable: CONFLICTING), and #1210 also makes this bump unnecessary. The template fingerprint now includes a build digest of all of crates/trusted-server-core/src/. That covers response_header_rewrite.rs and the new publisher.rs call site, so templates stored with origin-only CSP and Link metadata already miss after deploy. Main's new doc comment on this constant says to bump only for compatibility changes outside that digest. An auto-merge keeps 6 and this row under that doc, which contradicts it.

Resolution, verified in a scratch merge of 0d0689aca with origin/main (da31a215e):

git merge origin/main
# publisher.rs: take main's side of the one conflict hunk
# (the shared_template_ad_seam_is_readable_and_versioned pin)
git checkout origin/main -- crates/trusted-server-core/src/platform/template_cache.rs
# publisher.rs, parser_validation_does_not_change_the_cached_schema: 6 -> 5

After that, the diff against main is just the four feature files. The merged tree passes fmt, clippy-fastly, test-fastly (2,919 core tests), test-axum, test-cloudflare, test-spin, and parity. That includes a_cache_hit_replays_policy_headers_rewritten_to_the_serving_host, which doesn't depend on the version number. The PR description's template_cache.rs row and the "two schema-version pins" note would go too.

Apply manually: the fix is a merge resolution across template_cache.rs and publisher.rs, so it can't be a one-click suggestion.

[aram356]

🤔 thinking — "Only a scheme change can loop" doesn't hold when publisher.origin_host_header_override is set. I reproduced a loop with the Axum dev server at this head:

• origin http://127.0.0.1:8301, origin_host_header_override = "www.example.com", TS on 127.0.0.1:3031;
• an origin route that answers any request whose Host isn't 127.0.0.1:8301 with 301 Location: http://127.0.0.1:8301/canon (a canonical-host redirect).

TS rewrites that Location to http://127.0.0.1:3031/canon, which is the URL just requested, and refetches with the same override Host. Chrome made 19 hops and stopped at ERR_TOO_MANY_REDIRECTS. The origin log shows 20 hits, all through TS. On main (59595fb60) with the same config, the browser follows the redirect to the origin and the page loads: it leaves TS, but it doesn't loop.

More generally, a same-scheme redirect to the current URL loops whenever the origin redirects because of something TS sends unchanged on the retry. The override Host is one case. The Set-Cookie Domain case documented below is another. Options:

• Document it next to the Set-Cookie note, here and in configuration.md.
• Leave a same-scheme Location to the current URL unchanged when the request was GET/HEAD and the response sets no cookie. Without TS, that retry reaches the origin with a different Host, which is what ends the cycle. POST-redirect-GET, cookie-setting redirects, and Refresh reloads would still be rewritten.

[aram356]

🤔 thinking — A serving-host source without a path allows every path on that host, including Trusted Server's own routes. /first-party/sign is a public route, and with proxy.allowed_domains unset (the default) it signs any target. So script-src https://<serving host> allows any third-party script loaded through /first-party/proxy. 'self' already has this property, but this PR extends it to policies that named only the origin, which never allowed it. Worth saying in the warning:

Suggested change

	::: warning CSP and injected scripts
	Host rewriting only adds serving-host sources. It does not authorize inline
	content Trusted Server inserts without a nonce, or the `/static/tsjs=` bundle
	when the policy allows neither `'self'` nor the serving host. Check
	`script-src` (or `default-src`) on pages with a restrictive policy.
	:::
	::: warning CSP and injected scripts
	Host rewriting only adds serving-host sources. It does not authorize inline
	content Trusted Server inserts without a nonce, or the `/static/tsjs=` bundle
	when the policy allows neither `'self'` nor the serving host. Check
	`script-src` (or `default-src`) on pages with a restrictive policy.
	A serving-host source without a path also allows Trusted Server's own routes
	on that host, including `/first-party/proxy`, which serves any target that
	`/first-party/sign` signs. A policy that limited scripts to the origin now
	allows those too. Set `proxy.allowed_domains` to bound what the proxy serves.
	:::

[aram356]

📌 out of scope — The page-body counterparts of Refresh and the CSP header aren't rewritten. The HTML rewriter only touches href, src, action, srcset, and imagesrcset, so <meta http-equiv="refresh"> and <meta http-equiv="Content-Security-Policy"> keep origin URLs. Confirmed in Chrome against the Axum dev server at this head:

• <meta http-equiv="Content-Security-Policy" content="img-src http://127.0.0.1:8301"> plus an origin <img>: the img src is rewritten to the serving host, the meta policy isn't, and Chrome blocks the image with an img-src violation.
• <meta http-equiv="refresh" content="0; url=http://127.0.0.1:8301/refreshed.html">: the browser navigates straight to the origin.

These are the #1128 symptoms through a different carrier, and they need the same rules as the headers: the loop guard for refresh and additive sources for CSP. A follow-up issue seems right. Until then, a sentence here saying meta tags aren't covered would set operator expectations.

[aram356]

⛏ nitpick — This allocates for every Link value on every proxied response, including the common case where nothing names the origin. String::new() doesn't allocate until the first push_str, which only runs once a target is rewritten.

Suggested change

	let mut out = String::with_capacity(value.len());
	let mut out = String::new();

[aram356]

⛏ nitpick — The same path and query is read from req twice more below, with the same "/" fallback: readthrough_reader_url (around line 4771) and the template key's request_path (around line 4784). The URI doesn't change between here and rewrite_origin_request, so both could use this value:

format!("{request_scheme}://{request_host}{request_path_and_query}")
// ...
request_path: request_path_and_query.clone(),

That leaves one definition of "the path and query the reader asked for".

Apply manually: the other reads are outside the diff.