mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
Sas Swart
bf3db74b83
AI Gateway passthrough forwards the client's escaped path upstream unchanged. `http.ServeMux` only cleans literal dot segments, so a percent-encoded dot segment (for example `/openai/v1/models/%2e%2e/files`) matched an allowlisted passthrough route here, while the upstream decoded it and served a different endpoint with the centralized key attached. This adds `xurl.ContainsEncodedPath` in `coderd/util/xurl` so other forwarding code can reuse it. It is applied at the gateway entrypoint (`NewRequestBridge`, via `routing.RejectInvalidForwardPath`) and in the experimental `x/proxy.Router`, before routing. Requests get a 400 `invalid request path` if any path segment, after decoding once: - contains a `.` or `..` element, including behind an escaped `/` or `\`, - still contains `%`, which only arises from multiple encoding, - contains control characters. Escaped separators in ordinary segments such as `/models/org%2Fmodel` keep forwarding unchanged. Fixes AIGOV-678 <details> <summary>Decisions</summary> - **Validate once at the entrypoint instead of per handler.** One check covers every provider and route, including the reverse-proxy handler that will mount behind `x/proxy.Router`. - **Shared helper in `coderd/util/xurl`.** Per review, the check is a generic `func ContainsEncodedPath(*url.URL) bool` so other proxies can reuse a single implementation. - **Reject rather than normalize.** Legitimate clients have no reason to send dot segments to these APIs, and rejecting avoids authorizing a path the upstream interprets differently. - **Decode exactly once, and reject leftover `%`.** One decode matches RFC 3986 section 2.4. Rejecting a remaining `%` means further decoding by any downstream component cannot produce a dot segment. - **Split decoded segments on `/` and `\`.** Guards against components that decode reserved characters. `org%2Fmodel` still passes. - **Literal `..` now returns 400** instead of the ServeMux 307 redirect, because the check runs before the mux. - **Out of scope:** tightening passthrough route patterns and key-pool hardening. Both are separate follow-ups. </details> --- _Generated by Coder Agents on behalf of @SasSwart._