fix(proxy): stop leaking the /proxy prefix and the sandbox address to clients

[suixinio]

What

Two fixes on the sandbox proxy hop in src/api/proxy.rs, both about internal proxy details reaching the client:

1. /proxy/ — the prefix with nothing after it — matched neither prefix route, fell through to the header-routed fallback, and was forwarded to the sandbox unmodified, so the sandbox saw /proxy/ instead of /.
2. Location / Content-Location values pointing back at the sandbox's internal ip:port were passed through to the client verbatim.

Why

Bare /proxy/. The router matches /proxy exactly and /proxy/{*proxy_path} with a non-empty segment, so the empty-remainder shape reaches proxy_via_fallback, whose contract is to forward the path unchanged. The distributed gateway rewrites data-plane traffic to "/proxy" + path (upstreamTargetPath), so a client request for / always arrives at the node in exactly that shape. Consequences on a real deployment:

• applications without an SPA fallback answer their own home page with 404;
• dev servers normalize the trailing slash into Location: /proxy, which is relative, so it lands in the client's address bar and 404s on the next request;
• root-path websockets (Vite HMR) never reach the application.

Every non-root path was fine, which makes this look like an application bug rather than a proxy one. sandbox_proxy_classifier already treats path == "/proxy" || path.starts_with("/proxy/") as proxy traffic, so the classifier and the router disagreed about this one shape.

Sandbox address in redirects. Requests are forwarded with Host set to the upstream authority, so an application that builds absolute URLs from Host returns the sandbox's internal address in Location. Passing it through both exposes the sandbox network layout and points the client's next request at an address that is only reachable from that node.

Related issue

None. Scope is small and obvious, submitted directly per CONTRIBUTING.

Scope and non-goals

Included:

• one routing-table entry for /proxy/, reusing the existing proxy_via_prefix handler;
• rewriting self-referencing Location / Content-Location on the HTTP response path and on a rejected websocket handshake;
• three unit tests.

Not included:

• no change to request semantics, Host handling, or the fallback contract;
• response bodies are not rewritten — an application that writes absolute URLs into HTML still emits them, which is outside what a reverse proxy should do;
• host-based and header-based routing are otherwise untouched.

Design and behavior changes

Routing. .route("/proxy/", any(proxy_via_prefix::<I>)). The handler is unchanged: strip_proxy_prefix("/proxy/") already yields /. The set of paths the gateway can produce is closed — "/proxy" + <path starting with /> is either /proxy/ or /proxy/<non-empty> — and only the first was unrouted. Trailing slashes are not the criterion: /proxy/station/ (remainder station/) and /proxy// (remainder /) already matched the catch-all and keep their exact upstream paths.

Responses. rewrite_upstream_self_references() runs after hop-by-hop stripping in map_upstream_response and map_websocket_handshake_rejection_response. It rewrites a header only when the value parses as a URI whose authority equals the upstream authority, replacing it with path_and_query (/ when absent). Relative values and absolute URLs to any other host — an OAuth hop, a CDN — are left untouched.

Compatibility and operations

• Public API or generated protocol: N/A — no schema or generated code touched.
• Configuration or defaults: N/A.
• Snapshot manifest, artifact layout, or storage format: N/A.
• Upgrade and rollback: behavior-only, no persisted state; rolling back restores the previous behavior. A client that relied on receiving the sandbox's absolute address in Location now receives a path-relative value — that address was never reachable off-node.
• Host requirements, permissions, ports, or dependencies: N/A.
• Documentation: docs/src/concepts/proxy.md already documents ANY /proxy as forwarding to / inside the sandbox, so it describes the fixed behavior and needs no change.

Validation

• make fmt
• make clippy — scoped to the changed package, see below
• make test-unit — first command of the target only, see below
• Relevant Rust integration tests — not run: require root, /dev/kvm and network namespaces, unavailable in the container used for these checks
• make -C services test — N/A, no services/ change
• Generated clients/server regenerated with the documented make target — N/A, no generated code touched
• Documentation updated — N/A, see above
• Benchmarks or performance comparison completed — N/A, no performance-sensitive change

Commands and results:

$ cargo fmt --all -- --check
FMT_STATUS=0

$ cargo clippy -p agentenv --all-targets -- -D warnings
CLIPPY_STATUS=0   # no warnings from this package

$ cargo test -p agentenv --lib api::proxy
test result: ok. 36 passed; 0 failed; 0 ignored; 0 measured; 692 filtered out

$ cargo test -p agentenv --lib          # with this branch
test result: FAILED. 718 passed; 6 failed; 4 ignored

$ cargo test -p agentenv --lib          # unpatched c0c6bbb, same container, for comparison
test result: FAILED. 715 passed; 6 failed; 4 ignored

The same six tests fail before and after the change (privileges::tests::scoped_spawn_clears_child_capabilities_without_mutating_caller, sandbox::ublk::overlaybd::tests::compact_layers_mixed_input_and_configured_output, two snapshot::p2p::tests::local_overlaybd_layers_*, snapshot::repository::backends::common::acr::source_image::tests::plans_descriptorless_sparse_overlaybd_delta_for_dense_export, snapshot::repository::backends::posixfs::artifacts::tests::import_built_artifacts_preserves_zfile_memory_lower_bytes). They need privileges and real devices that the unprivileged container does not provide; the delta of this branch is exactly the three new passing tests.

Skipped checks and reasons: integration tests and the capability-runner steps of make test-unit need root, /dev/kvm and netns; the environment available for these checks is an unprivileged container.

Beyond unit tests, both fixes were verified on a two-node k3s deployment (gateway + node DaemonSet, one microVM sandbox):

# inside the sandbox: python3 -m http.server on 5173, which has no SPA fallback
client path                 sandbox access log        # after the fix
/                           GET /            200
/proxy                      GET /            200
/proxy/                     GET /            200      # was: GET /proxy/ 404
/proxy//                    GET //           200
/proxy/http.log             GET /http.log    200
/nope                       GET /nope        404      # fallback still forwards verbatim

# a Vite app behind the same gateway, seen from the browser
GET /    before: 307 Location: /proxy   ->  after: 200, application home page
GET /    websocket upgrade              ->  101 Switching Protocols, sec-websocket-protocol: vite-hmr
GET //   before: 308 Location: http://10.11.0.1:5173/  ->  after: 308 Location: /

Risks and reviewer notes

• The new route only changes which handler serves /proxy/; a client that deliberately used header routing to reach a sandbox path literally named /proxy/ now reaches / instead. That path was already unreachable through the explicit prefix form.
• The redirect rewrite compares authorities byte-wise. An application that echoes the same host in different case, or adds a default port, keeps its absolute URL — deliberately conservative, since a false negative leaks nothing that is not leaked today while a false positive would break a third-party redirect.
• Both commits touch src/api/proxy.rs only, and they are independent: happy to split them into separate pull requests if you prefer to review or revert them separately.

Checklist

• The PR contains one coherent change and no unrelated formatting or refactoring.
• New behavior is covered by tests, or I explained why testing is impractical.
• Logs and examples contain no credentials, tokens, or private registry information.
• I did not manually edit generated code without updating its source and regenerating it.

[Copilot]

Pull request overview

This PR fixes two proxy-behavior leaks in the node-side sandbox reverse proxy (src/api/proxy.rs): (1) ensuring the gateway-generated root shape /proxy/ is routed through the prefix proxy handler (so the sandbox sees /), and (2) rewriting response Location / Content-Location headers that self-reference the sandbox’s internal ip:port so clients don’t see or follow unreachable internal addresses.

Changes:

• Add an explicit /proxy/ route that reuses the existing proxy_via_prefix handler so /proxy/ correctly forwards to upstream /.
• Rewrite Location and Content-Location headers when they point back at the upstream authority, replacing them with a path-relative target.
• Add unit tests covering /proxy/ forwarding, wildcard trailing-slash preservation, and redirect rewriting behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

🔍 OpenCodeReview found 1 issue(s) in this PR.

• ✅ Successfully posted inline: 1 comment(s)

[github-actions]

http::Uri does not support URI fragments, but Location is allowed to contain a URI-reference such as http://10.x.y.z:5173/next#section. Parsing that value fails, so this code silently leaves the internal sandbox authority exposed and the redirect unusable. Split and preserve the optional fragment before parsing/comparing the URI, then append it to the rewritten path/query; cover this with a test.

[suixinio]

Fixed, with one correction to the mechanism. With http 1.x (this crate's dependency; 1.4.0 in the lockfile) "http://10.0.0.1:5173/next#section".parse::<Uri>() succeeds — it does not fail — and path_and_query() returns /next. So the fragment was silently dropped rather than the sandbox authority being left exposed. I checked 0.2.12 as well, same behavior:

http://10.0.0.1:5173/next#section      OK  auth=Some("10.0.0.1:5173") path="/next" pq=Some("/next")

The rewrite now splits the fragment off before parsing and appends it verbatim, so Location keeps #section. Covered by a unit case and by the HTTP-level test (/proxy/fragment).

[github-actions]

Raw authority equality misses semantically identical self-references. In particular, when the sandbox target port is 80, URL libraries commonly normalize http://10.x.y.z:80/... to http://10.x.y.z/...; that redirect will bypass this rewrite and expose the internal address. Compare parsed hosts case-insensitively and compare effective ports (including the scheme's default port) rather than authority text, and test a target using port 80.

[suixinio]

Fixed. Self-references are now matched by host (case-insensitive) and effective port — the scheme's default is filled in (80 for http/ws, 443 for https/wss) — instead of by authority text. The upstream URL always carries an explicit port, so a target whose port cannot be determined never compares equal, which keeps the matcher conservative in the other direction.

Covered by a unit case with an upstream on port 80 and a target that omits it.

While there, I closed the same class for scheme-relative references: //10.0.0.1:5173/next parses with host() == None, so the old authority comparison let it through as well. It is now parsed against the upstream scheme before the comparison.

[github-actions]

An absolute self-reference with an empty path and a query (for example, http://10.x.y.z:5173?x=1) has a path_and_query of ?x=1, so this emits a query-relative Location. The client then retains its current path rather than redirecting to the upstream root represented by the absolute URI. Normalize a query-only value to /?x=1 (or construct the result from target.path() plus target.query()) and add a regression test.

Suggestion:

Suggested change

	target
	.path_and_query()
	.map(|path_and_query| path_and_query.as_str().to_owned())
	.unwrap_or_else(|| "/".to_owned())
	let path = target.path();
	match target.query() {
	Some(query) => format!("{path}?{query}"),
	None => path.to_owned(),
	}

[suixinio]

Fixed, thanks — confirmed that "http://10.0.0.1:5173?x=1" has path_and_query() == "?x=1" while path() == "/". The result is now built from path() + query() (+ fragment) as suggested, so it emits /?x=1. Regression covered by a unit case and by the HTTP-level test (/proxy/query-only).

[github-actions]

Include the scheme in the endpoint comparison. As written, an HTTP upstream at host:5173 also matches https://host:5173/path (and ws similarly matches wss) because only host and effective port are checked. Rewriting that value to /path silently removes an application-requested protocol transition and makes the client retain the gateway's current scheme. Scheme-relative references already inherit the upstream scheme before this check, so requiring equal schemes preserves those while leaving explicit cross-scheme redirects untouched.

Suggestion:

Suggested change

	target_host.eq_ignore_ascii_case(upstream_host)
	&& effective_port(target) == effective_port(upstream_uri)
	target_host.eq_ignore_ascii_case(upstream_host)
	&& target.scheme_str() == upstream_uri.scheme_str()
	&& effective_port(target) == effective_port(upstream_uri)