Skip to content

fix(interceptors): answer a malformed egress request instead of dying on it - #133

Open
Cedric921 wants to merge 1 commit into
Memnox:mainfrom
Cedric921:fix/survive-a-malformed-egress-request
Open

Cedric921 wants to merge 1 commit into
Memnox:mainfrom
Cedric921:fix/survive-a-malformed-egress-request

Conversation

@Cedric921

Copy link
Copy Markdown
Contributor

Closes #13.

What this changes

Both handlers ran as void answerRequest(...) and void answerConnect(...) with no catch, and nothing registers an unhandledRejection. So a rejection ended the process — and cli/src/daemon/egress.ts:127 starts this server inside the daemon.

Running the new tests against main shows it rather than argues it:

⎯⎯⎯ Unhandled Rejection ⎯⎯⎯
URIError: URI malformed
⎯⎯⎯ Unhandled Rejection ⎯⎯⎯
TypeError: Protocol "https:" not supported. Expected "http:"

Both are reachable by any local process: the first is a Proxy-Authorization carrying %zz, the second a GET https://host/ in absolute form.

All five rows from the issue:

  • A catch on each handler. A request that throws gets 400 and an ended response; a CONNECT that throws gets its socket destroyed, since nothing has been written to a tunnel yet.
  • The CONNECT authority goes through URL. authority.split(':') gave host [2001 and port NaN for [2001:db8::1]:443, which net.connect threw on, and [::1]:443 gave host [ on port 0.
  • decodeURIComponent is guarded. A mangled percent sequence is a credential this seam does not recognise, not a reason to stop.
  • An absolute-form request whose scheme this proxy cannot speak is refused with the reason, before httpRequest throws on it. https travels by CONNECT.
  • The upstream error handler checks headersSent, because a partial answer may already be on the wire and writing the head twice throws.

An IPv6 hostname also has its brackets taken off before forwarding — target.hostname keeps them, so the lookup failed.

How it was verified

Five tests in packages/interceptors/test/egress-server.test.ts, each against a real local server. Four fail on main:

  • CONNECT [::1]:<port> tunnels and the body comes back
  • a mangled Proxy-Authorization is answered, and the proxy serves the next request normally — which is the half that proves the process is still alive
  • an https:// absolute-form request is refused with 403 and a reason, and the next request still works
  • http://[::1]:<port>/ forwards rather than failing to resolve

The fifth pins that a CONNECT authority too broken to read still destroys its socket and leaves the server serving, which was already true and must stay.

Test Files  286 passed (286)
     Tests  8457 passed (8457)

prettier, tsc, vitest and knip all clean, on Node 24.

Checklist

  • pnpm format && pnpm typecheck && pnpm test && pnpm deadcode all pass
  • Behaviour change ships with a test
  • No any, no magic values, no console.* outside cli-output.ts
  • If this touches the decision path: n/a — the seam still rules on every host it forwards; this is what happens when the request never parses
  • If this changes a verb table: n/a
  • If this changes a command, flag or file it writes: n/a; a refusal gains a reason line, which the changeset names

@Cedric921
Cedric921 requested a review from moise10r as a code owner October 6, 2026 15:04

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.

egress: one malformed request crashes the whole proxy (IPv6 CONNECT, bad Proxy-Authorization, https absolute form)

1 participant