Skip to content

Rewrite same-origin Origin to the upstream with --rewrite-host - #1251

Open
prk-Jr wants to merge 6 commits into
mainfrom
fix/dev-proxy-rewrite-origin
Open

prk-Jr wants to merge 6 commits into
mainfrom
fix/dev-proxy-rewrite-origin

Conversation

@prk-Jr

@prk-Jr prk-Jr commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • With --rewrite-host, ts dev proxy sent Host: <TO> but forwarded the browser's Origin: https://<FROM> unchanged. Trusted Server's mobile trace Enable/End actions (POST /_ts/trace/enable and /end) check that Origin matches their own origin, so through the proxy they always returned 403.
  • For the trace Enable/End actions (POST /_ts/trace/enable and POST /_ts/trace/end, matched exactly, with any query string ignored), the proxy now replaces a single same-origin Origin with the TO origin, so Origin and Host name the same server. It uses http:// with --upstream-plaintext and https:// otherwise, and keeps a non-default port. This fixes the dev tool rather than loosening the server's checks, which must keep ignoring spoofable forwarded headers.
  • The browser's origin is taken from the incoming Host header, so sessions on a port other than 443 are rewritten too. If there is no Host header, or more than one, nothing is rewritten.
  • Only those two actions are rewritten. Trusted Server forwards the browser's headers to the publisher origin and to integration vendors (Lockr, Didomi, Sourcepoint, DataDome). That includes integrations an operator mounts under /_ts, such as a Didomi proxy_path of _ts/consent. Every other request therefore keeps the browser's real Origin, as in production.
  • Cross-site, null, plain-http:// and duplicated Origin values pass through unchanged. Without --rewrite-host, Origin is never touched.

Depends on #1107, which adds the trace Enable/End endpoints this targets.

Changes

File Change
crates/trusted-server-cli/src/commands/dev/proxy/rewrite.rs RewriteOutcome gains upstream_origin, built from the TO scheme and port and set only with --rewrite-host. Tests for plaintext and TLS, and for the case without --rewrite-host.
crates/trusted-server-cli/src/commands/dev/proxy/server.rs New requires_upstream_origin, matching POST /_ts/trace/enable and /_ts/trace/end exactly. rewrite_first_party_origin compares Origin with https:// + the incoming Host, runs in proxy_to_upstream before Host is overwritten, and logs the rewrite at debug level. Unit tests: rewrite, non-443 port, foreign/null/http/duplicate pass-through, missing or duplicated Host, exact-route matching (GET, near-miss paths, /_ts/consent/...), no rewrite without --rewrite-host.
crates/trusted-server-cli/tests/proxy_e2e.rs End-to-end tests through the proxy against a server that echoes back what it received: same-origin trace action rewritten to the TO origin; same-origin publisher path and an integration route under /_ts (/_ts/consent/api/events) unchanged; cross-site Origin unchanged; no rewrite without --rewrite-host.
crates/trusted-server-cli/tests/support/mod.rs The echo server reports the Origin it received; new drive_request_with_origin helper takes a path and an Origin.
docs/guide/ts-dev-proxy.md Documents the Origin rewrite, that it covers only the trace Enable/End actions, and why: publisher and vendor requests, including integrations mounted under /_ts, keep the browser's real Origin.

Closes

Closes #1250

Test plan

  • cargo test-fastly && cargo test-axum (also cargo test-cloudflare)
  • cargo clippy-fastly && cargo clippy-axum (also cargo clippy-cloudflare and cargo clippy-cli)
  • cargo fmt --all -- --check
  • JS tests: cd crates/trusted-server-js/lib && npx vitest run
  • JS format: cd crates/trusted-server-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, with a proxy built from 4ac34aaa0: through the proxy against fastly compute serve, a same-origin Enable returns 200; a missing Sec-Fetch-Site and a foreign Origin return 403. In Chrome through the proxy, Enable shows "Tracing is on" and End shows "Tracing is off" with the cookie removed. The narrower route matching in 6323a0ec4 is covered by the end-to-end tests.
  • Other: cargo test --package trusted-server-cli --target aarch64-apple-darwin (606 unit tests plus integration suites, including 34 end-to-end proxy tests). With the rewrite call removed, the main end-to-end test fails; with the old /_ts rule, the integration-route test fails.

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

With --rewrite-host the proxy sends Host: TO but forwarded the browser's
Origin: https://FROM unchanged, so upstream endpoints that verify a
same-origin Origin against their own origin rejected proxied requests.
Replace a single same-origin Origin with the TO origin (scheme from
--upstream-plaintext, non-default port kept) so Origin and Host name the
same authority. Cross-site, null, plain-http and duplicated Origin values
pass through unchanged.
@prk-Jr prk-Jr self-assigned this Oct 7, 2026
Derive the first-party origin from the inbound Host so non-443 sessions
are rewritten too, and apply the rewrite only to Trusted Server's /_ts
endpoints, so publisher and integration requests keep the browser's real
Origin. Cover the behavior end to end through the proxy.
@prk-Jr
prk-Jr marked this pull request as ready for review October 7, 2026 13:28
@prk-Jr
prk-Jr requested review from ChristianPavilonis, aram356 and dhruv8sh and removed request for aram356 October 7, 2026 13:28

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review summary

Reviewed 4ac34aaa05a36587b303c3bf9d4fc82ab5b6f4a4 against 7a0ecb4cbfa39af3d83f7e1ed2733d7eabdc4cb1. All five changed files and their affected callers and consumers were inspected. One P2 compatibility regression is posted inline.

Validation

  • cargo test --package trusted-server-cli --target x86_64-unknown-linux-gnu --lib commands::dev::proxy::: 113 passed.
  • cargo test --package trusted-server-cli --target x86_64-unknown-linux-gnu --test proxy_e2e: 33 passed.
  • cargo test --package trusted-server-cli --target x86_64-unknown-linux-gnu: passed, including 607 unit tests and the integration suites; 29 tests remained ignored.
  • cargo fmt --all -- --check, cargo clippy-cli, and cargo build-axum: passed.
  • Inline Python runtime fixtures: 24 proxy header exchanges passed; the custom Didomi route regression was reproduced through the real Axum adapter against a local Origin-validating vendor fixture.
  • All reported CI checks passed. Existing review feedback was empty.

Foreign, opaque, and duplicate Origins remained unchanged in the exercised cases. The trace handlers described in the PR are absent from this revision, so their actual Enable/End acceptance and cookie workflow were not independently verified. No repository files were modified.

Comment thread crates/trusted-server-cli/src/commands/dev/proxy/server.rs Outdated

@dhruv8sh dhruv8sh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Scoped, well-tested fix: with --rewrite-host, same-origin Origin on /_ts requests is mapped to the TO origin so it agrees with Host. The scheme (http under --upstream-plaintext) and default-port omission line up with how the trace action check canonicalizes its expected origin from the request ingress, and keeping the rewrite out of publisher/integration paths preserves real Origin for vendors that forward it.

1 of the inline comments below carries a one-click GitHub suggestion.

Non-blocking

  • 🌱 Log when Origin is rewritten — see inline at crates/trusted-server-cli/src/commands/dev/proxy/server.rs:792
  • ⛏ Byte-exact Origin/Host comparison — see inline at crates/trusted-server-cli/src/commands/dev/proxy/server.rs:789

Cross-cutting / body-level findings

  • 📝 Depends on unmerged trace endpoints — the /_ts/trace/enable and /end routes (and their Origin check in trace/actions.rs) currently live on spec/mobile-ad-render-trace-endpoint, not main. Worth noting for anyone verifying this against main.
  • 👍 Scoping and tests — restricting the rewrite to the /_ts namespace is the right call, and the coverage (non-443 port, missing/duplicated Host, null/http/foreign/duplicate Origin, /_tsx and /_ts-foo look-alikes, plus end-to-end through the echo upstream) is thorough.

CI Status

All 22 checks PASS (cargo fmt, cargo test, cargo test (axum native), cargo test (ts CLI, native) on ubuntu/macos, cross-adapter parity, cloudflare/spin checks, vitest, format-typescript, format-docs, integration + browser integration tests, CodeQL / Analyze).

Comment thread crates/trusted-server-cli/src/commands/dev/proxy/server.rs
Comment thread crates/trusted-server-cli/src/commands/dev/proxy/server.rs
prk-Jr and others added 3 commits October 8, 2026 09:51
Match POST /_ts/trace/enable and /_ts/trace/end exactly instead of the
whole /_ts namespace, so integration routes an operator mounts under /_ts
(such as a Didomi proxy_path of _ts/consent) keep the browser's real
Origin. Log the rewrite at debug level.

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review summary

Reviewed fbc7372bb79c8796b8cdbf956ca6408f889f4962 against 182fdf45c8a4e08fac68ea7b0b77f7f59a7284c1, from fix/dev-proxy-rewrite-origin into main. Inspected all five changed files and affected configuration, routing, forwarding, and test consumers.

No actionable new issues found.

Safety proof

  1. Only eligible trace actions receive the upstream Origin; publisher/vendor requests retain their browser Origin.

    • Highest evidence: Runtime.
    • Evidence: All 34 proxy end-to-end tests passed. An additional 36 CLI exchanges exercised plaintext upstreams, Enable/End, query handling, non-default ports, foreign/null/http/duplicate/missing Origins, missing Host, near-miss paths, publisher/vendor paths, and rewrite-host on/off.
    • Status: proven for the exercised inputs.
  2. Rewritten actions pass Trusted Server authorization without weakening its other controls.

    • Highest evidence: Path.
    • Evidence: Inspected the declared dependency #1107 at 285c75face3dc23853f053162b20fc7e18734586. Its trace actions compare Origin against trusted ingress metadata and independently require the action header, same-origin Fetch Metadata, and an empty body. Those handlers are absent from this reviewed revision.
    • Status: unproven.
    • Next check: After #1107 is integrated, exercise Enable/End and cookie observation through the proxy; verify foreign Origin and missing Fetch Metadata still return 403.

Validation and review context

  • cargo test --locked --package trusted-server-cli --target x86_64-unknown-linux-gnu --lib commands::dev::proxy::: 113 passed.
  • cargo test --locked --package trusted-server-cli --target x86_64-unknown-linux-gnu --test proxy_e2e: 34 passed.
  • cargo fmt --all -- --check and cargo clippy-cli: passed.
  • git diff 182fdf45c8a4e08fac68ea7b0b77f7f59a7284c1...fbc7372bb79c8796b8cdbf956ca6408f889f4962 --check: passed.
  • python3 - with an inline plaintext echo fixture: 36 exchanges passed. Initial certificate validation failed under Python's strict X.509 policy because the unchanged proxy certificate lacks an Authority Key Identifier. The rerun retained CA-chain and hostname verification with strict-policy enforcement disabled.
  • CI: all 22 checks successful; confirmed the test workflow ran against the locked head.
  • Existing feedback: inspected paginated reviews, inline comments, replies, issue comments, and thread resolution states. All three threads are resolved. The earlier custom Didomi-route regression is fixed and covered; no duplicate findings.
  • Residual risk: actual trace authorization and browser cookie lifecycle remain unverified pending #1107. Docs formatting was not rerun locally because Prettier is unavailable; its CI check passed.
  • No repository files were modified. Head/base revisions were rechecked before submission, and the worktree remained clean.

@prk-Jr prk-Jr added this to the 202610 milestone Oct 8, 2026

This branch has not been deployed

No deployments
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.

ts dev proxy --rewrite-host forwards a mismatched Origin, so same-origin POSTs are rejected

4 participants