fix(proxy): make ampcode passthrough fully transparent#90
Conversation
The /api/* and /v0/management/* catch-all proxy was rewriting the amp CLI's own headers (stripping Authorization/X-Api-Key, injecting byokey's amp token) and buffering the entire upstream response before returning it. Both broke the amp `read` tool: the rewritten auth caused thread operations to run as the wrong principal, and buffering prevented the SSE turn-continuation from streaming back to the client. Forward the amp CLI's request verbatim (only hop-by-hop + host stripped per RFC 7230) and stream the response body back. /api/provider/* is unaffected — byokey still owns AI traffic. The forward_headers middleware is deleted since no other route needed it.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f63520c961
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mut fwd = rquest::header::HeaderMap::new(); | ||
| for (name, value) in &headers { | ||
| let name_str = name.as_str(); | ||
| if HOP_BY_HOP.contains(&name_str) || name_str == "host" { | ||
| continue; |
There was a problem hiding this comment.
This change makes /api/* and /v0/management/* fully pass through client headers, but it also removes the only runtime use of amp.upstream_key (documented in crates/config/src/schema/amp.rs as enabling shared-proxy mode). In deployments that rely on that setting, requests that previously worked with injected proxy credentials will now be sent with missing or client-scoped auth and start failing with upstream 401/403. If shared-proxy mode is being deprecated, this commit should fail fast when amp.upstream_key is set instead of silently ignoring it.
Useful? React with 👍 / 👎.
rquest is built with `features = ["full"]` so it auto-decompresses gzip/brotli/zstd response bodies before `bytes_stream()` yields them. The prior passthrough still forwarded upstream's `Content-Encoding` and `Content-Length` headers, which described the *compressed* payload — so the amp CLI saw `Content-Encoding: gzip` and tried to decode already-decoded bytes, got garbage, silently failed, and fell back to a degraded mode policy where `read`/`bash` aren't allowed in smart mode. Strip both headers on the catch-all response path so axum sends the decompressed body as chunked without a misleading encoding hint.
1 participant
Summary
/api/*and/v0/management/*now forward amp CLI traffic verbatim — only hop-by-hop headers +hostare stripped (RFC 7230). byokey no longer overwritesAuthorization/X-Api-Keyor injects its own amp token for these non-AI endpoints.Body::from_stream(resp.bytes_stream())instead of buffered withresp.bytes().await, so SSE turn-continuation (which fires after thereadtool returns a file) reaches the client in real time./api/provider/*is unchanged — byokey still owns AI traffic and its per-provider handlers build their own auth.forward_headers_middlewareand itsForwardedHeadersextension are deleted; no remaining handler reads them.Why
The amp CLI's
readtool reads a local file and POSTs the result back to a thread endpoint under/api/*. Before this change, byokey rewrote the request auth (causing thread ops to run as the wrong principal when byokey held an amp token) and held the streaming reply in memory until upstream closed. Net effect: the read tool appeared to hang or never continue the AI turn. See also the recent investigation captured indocs/dia-browser-interception.md/ the attached thread.Test plan
cargo check -p byokey-proxy— cleancargo test -p byokey-proxy --lib— 47 passed, 0 failed (includingtest_amp_login_redirect,test_amp_cli_login_redirect)cargo clippy -p byokey-proxy --all-targets— cleanreadtool and confirm the AI turn continues after the file is readFollow-up
config.amp.upstream_keyis now a dead config field — only the deleted middleware read it. Happy to rip it out in a separate PR if desired.