Skip to content

fix(proxy): missing proxyRes error handler + SSE corruption on error after headers#18

Draft
aattaran wants to merge 1 commit into
mainfrom
claude/fix-audit-shared-bugs-0vfeG
Draft

fix(proxy): missing proxyRes error handler + SSE corruption on error after headers#18
aattaran wants to merge 1 commit into
mainfrom
claude/fix-audit-shared-bugs-0vfeG

Conversation

@aattaran
Copy link
Copy Markdown
Owner

@aattaran aattaran commented May 7, 2026

Summary

Two production bugs in proxy/model-proxy.js surfaced by an audit run against aattaran/Jarvis2 (shared proxy ancestry — both fixes apply verbatim).

Bug 1 — missing proxyRes error handler (process crash)

The three response paths inside the httpsRequest callback —

  • proxyRes.pipe(norm).pipe(clientRes) (SSE)
  • the data/end JSON collector
  • the passthrough proxyRes.pipe(clientRes)

— never attach an 'error' handler to proxyRes. If the upstream connection drops mid-response (TCP RST, TLS abort, server crash), Node emits an unhandled 'error' on the response stream and the entire proxy process exits, killing every concurrent in-flight Claude Code session.

Fix: attach a single proxyRes.on('error', ...) immediately after the response is opened, before any of the branch-specific piping. Mirrors the existing proxyReq.on('error') shape: 502+JSON if headers haven't gone out, otherwise destroy the response.

Bug 2 — SSE corruption when proxyReq errors after headers sent

Current proxyReq.on('error') unconditionally calls:

clientRes.end(JSON.stringify({ error: { message: 'Upstream connection error' } }));

If headers already went out as text/event-stream (the common SSE case for /v1/messages), that JSON gets injected mid-stream. Claude Code's SSE parser then sees a non-event payload and either silently truncates the assistant turn or throws — depending on where in the stream the upstream died.

Fix: clientRes.destroy(err) once clientRes.headersSent is true, so the client sees a clean transport-level abort instead of a corrupted event stream.

Files

  • proxy/model-proxy.js — added proxyRes error handler; gated proxyReq error handler's end() on !headersSent.

Note on the third audit finding

The same audit also flagged the /_proxy/mode Origin-prefix CSRF/DNS-rebinding issue. That's already covered by PR #16 (open, from aaronjmars), so it is intentionally not in this PR.

Test plan

  • Local repro: kill upstream mid-SSE (iptables -A OUTPUT -p tcp --dport 443 -j REJECT after first chunk) → before patch: process crashes / SSE garbage; after patch: clean client error, proxy survives.
  • Existing happy path unchanged: streaming + JSON /v1/messages still flow through UsageNormalizer and produce correct /_proxy/cost.
  • /_proxy/status, /_proxy/mode, /_proxy/cost still respond correctly.

Generated by Claude Code

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.

1 participant