A refused upload keeps its answer, no clear-text 502 in a tunnel, and OPTIONS * keeps its host

src/sandbox/mux-proxy.ts

@@ -120,7 +120,18 @@ export function createMuxProxyServer(opts: MuxProxyOptions): MuxProxyServer {
     const upstream = backendSocketPath
       ? connect(backendSocketPath)
       : connect(backendTcpPort!, '127.0.0.1')
+    let answered = false
+    upstream.once('data', () => {
+      answered = true
+    })
     upstream.on('error', err => {
+      // The 502 is an answer, for a client that has had none. One that has
+      // may be in the middle of anything, a TLS tunnel included, where clear
+      // text is noise. The 'close' that follows ends it.
+      if (answered) {
+        logForDebugging(`mux: HTTP backend error: ${(err as Error).message}`)
+        return
+      }
       // Surface a 502 with the errno code so callers (and CI logs) can tell
       // *why* the backend was unreachable instead of an opaque empty-reply.
       // Full error (including path) goes to debug logging only — the client

src/sandbox/request-filter.ts

@@ -723,9 +723,10 @@ export function refuseUnparsedRequest(
     'HTTP/1.1 400 Bad Request\r\nContent-Type: text/plain\r\n' +
       `${name}: ${value}\r\nContent-Length: ${refused.reason.length + 1}\r\n` +
       `Connection: close\r\n\r\n${refused.reason}\n`,
-    () => socket.destroy(),
   )
-  // A flush that never ends must not keep the socket.
+  // Ended, and destroyed a second later: a body still on its way is read and
+  // dropped meanwhile. Unread, it would turn the close into a reset, and the
+  // reset would discard the answer.
   setTimeout(() => socket.destroy(), 1000).unref?.()
   return refused
 }

src/sandbox/tls-terminate-proxy.ts

@@ -1066,16 +1066,15 @@ async function forwardUpstream(
     )
     return
   }
+  // RFC 9112 3.3: the target URI of the asterisk form (`OPTIONS *`) has an
+  // empty path. Put behind the authority, the `*` would be read into the host.
+  const url = `https://${authority}${path === '*' ? '' : path}`
   let body: Readable = req
   let decision: RequestDecision | undefined
   const sni = target.clientServerName ?? serverNameOf(req)
   if (target.requireHostMatch && target.clientServerNameUnreadable) {
     const why = 'TLS server name could not be read'
-    target.onFilterRequestDeny?.(
-      req.method ?? 'GET',
-      `https://${authority}${path}`,
-      why,
-    )
+    target.onFilterRequestDeny?.(req.method ?? 'GET', url, why)
     respondDenied(res, why, markFor('misdirected'), 421)
     return
   }
@@ -1087,11 +1086,7 @@ async function forwardUpstream(
       absoluteFormAuthority(req.url),
     )
     if (why !== undefined) {
-      target.onFilterRequestDeny?.(
-        req.method ?? 'GET',
-        `https://${authority}${path}`,
-        why,
-      )
+      target.onFilterRequestDeny?.(req.method ?? 'GET', url, why)
       respondDenied(res, why, markFor('misdirected'), 421)
       return
     }
@@ -1118,7 +1113,7 @@ async function forwardUpstream(
       filterRequest,
       req,
       res,
-      `https://${authority}${path}`,
+      url,
       ac.signal,
       target.onFilterRequestDeny,
       {

test/sandbox/mux-proxy.test.ts

@@ -120,6 +120,63 @@ describe('mux-proxy first-byte dispatch', () => {
     expect([errors, hadError]).toEqual([[], false])
   })
 
+  it.skipIf(process.platform === 'win32')(
+    'answers 502, with the reason, where the backend cannot be dialled',
+    async () => {
+      mux.removeBackendSocketSync()
+      const reply = await send(
+        Buffer.from('CONNECT x:443 HTTP/1.1\r\nHost: x:443\r\n\r\n'),
+      )
+      expect(reply.toString()).toBe(
+        'HTTP/1.1 502 Bad Gateway\r\nConnection: close\r\n\r\n' +
+          'mux backend dial failed (ENOENT)\n',
+      )
+    },
+  )
+
+  it.each([
+    // Whether a backend that goes away shows here as an error at all is the
+    // platform's. On Linux a write fails first, and it does. On macOS and
+    // Windows this client is just closed, so the case would tell nothing.
+    ...(process.platform === 'linux'
+      ? [['', 'HTTP/1.1 502 Bad Gateway\r\n']]
+      : []),
+    ['HTTP/1.1 200 Connection Established\r\n\r\n', ''],
+  ])(
+    'a backend that fails having answered %p: the mux adds %p',
+    async (answer, added) => {
+      httpStub.removeAllListeners('connect')
+      httpStub.on('connect', (_req, sock) => {
+        sock.on('error', () => {})
+        sock.resume()
+        if (answer !== '') sock.write(answer)
+        setTimeout(() => sock.destroy(), 50)
+      })
+      for (let n = 0; n < 5; n++) {
+        const reply = await new Promise<string>(resolve => {
+          let text = ''
+          // Still sending when the backend goes: the write that fails then
+          // is the error.
+          const chunk = Buffer.alloc(64 << 10)
+          const pump = (): void => {
+            while (!c.destroyed && c.writable && c.write(chunk));
+          }
+          const c = connect(port, '127.0.0.1', () => {
+            c.write('CONNECT x:443 HTTP/1.1\r\nHost: x:443\r\n\r\n')
+            pump()
+          })
+          c.on('drain', pump)
+          c.on('error', () => {})
+          c.on('data', d => (text += d.toString('latin1')))
+          c.on('close', () => resolve(text))
+        })
+        expect(reply.slice(0, answer.length)).toBe(answer)
+        expect(reply.slice(answer.length).slice(0, added.length)).toBe(added)
+        if (added === '') expect(reply).toBe(answer)
+      }
+    },
+  )
+
   it('routes a TLS ClientHello first byte (0x16) to HTTP, not SOCKS', async () => {
     // Not a valid HTTP request — the backend will 400 it — but the point
     // is it must NOT be treated as SOCKS.

test/sandbox/request-target-forwarding.test.ts

@@ -8,7 +8,8 @@ import {
   connect as tlsConnect,
   createServer as createTlsServer,
 } from 'node:tls'
-import { readFileSync } from 'node:fs'
+import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { PassThrough } from 'node:stream'
 import {
@@ -29,6 +30,7 @@ import {
 } from '../../src/sandbox/credential-aws-pairs.js'
 import { signSigv4, UNSIGNED_PAYLOAD } from '../../src/sandbox/aws-sigv4.js'
 import { SandboxManager } from '../../src/sandbox/sandbox-manager.js'
+import { spawnAsync } from '../helpers/spawn.js'
 
 const FIXTURE_DIR = join(import.meta.dir, '..', 'fixtures', 'tls-terminate')
 const CA_CERT = join(FIXTURE_DIR, 'ca.crt')
@@ -557,6 +559,23 @@ describe('request target forwarding', () => {
     },
   )
 
+  test.each(['default', 'asSpelled', 'normalize'] as const)(
+    'with a hook, %s: OPTIONS * is judged under its own host, and sent as it is',
+    async mode => {
+      const authority = `${UPSTREAM_NAME}:${tlsPort}`
+      const [asked, arrived] = [hookSeen.length, tlsSeen.length]
+      const reply = await exchange(
+        await tunnel(hookPort[mode]),
+        `OPTIONS * HTTP/1.1\r\nHost: ${authority}\r\n\r\n`,
+      )
+      expect(reply.status).toBe(200)
+      expect(tlsSeen.slice(arrived)).toEqual(['*'])
+      const [seen] = hookSeen.slice(asked)
+      expect(seen!.url).toBe(`https://${authority}/`)
+      expect(seen!.info!.requestTarget).toBe('*')
+    },
+  )
+
   test.each(['default', 'asSpelled', 'normalize'] as const)(
     'with a hook, %s: a fragment right after the authority leaves the root',
     async mode => {
@@ -1245,11 +1264,14 @@ describe('refuseUnparsedRequest', () => {
     expect(socket.readableLength).toBe(answer)
   })
 
-  test('the socket goes as soon as the answer is out', async () => {
+  test('the socket is ended at once, and goes a second later', async () => {
     const socket = new PassThrough({ autoDestroy: false })
     socket.resume()
     refuseUnparsedRequest(parseError('x', 0), socket, true)
-    await new Promise(r => setTimeout(r, 50))
+    expect(socket.writableEnded).toBe(true)
+    await new Promise(r => setTimeout(r, 500))
+    expect(socket.destroyed).toBe(false)
+    await new Promise(r => setTimeout(r, 700))
     expect(socket.destroyed).toBe(true)
   })
 
@@ -1429,4 +1451,52 @@ describe('network.ambiguousRequestTarget', () => {
       ])
     },
   )
+
+  // By a client in a process of its own, busy with its upload: one that shares
+  // this event loop reads the answer the moment it is there. And through the
+  // manager's proxy, mux and all.
+  test.skipIf(Bun.which('curl') === null)(
+    'in a tunnel, one with a body on its way still gets its answer',
+    async () => {
+      await SandboxManager.reset()
+      await SandboxManager.initialize({
+        network: {
+          allowedDomains: ['127.0.0.1'],
+          deniedDomains: [],
+          tlsTerminate: { caCertPath: CA_CERT, caKeyPath: CA_KEY },
+        },
+        filesystem: { denyRead: [], allowWrite: [], denyWrite: [] },
+      })
+      const store = SandboxManager.getSandboxViolationStore()
+      store.clear()
+      const dir = mkdtempSync(join(tmpdir(), 'refused-upload-'))
+      try {
+        writeFileSync(join(dir, 'body'), Buffer.alloc(256 << 10))
+        for (let n = 0; n < 8; n++) {
+          const curl = await spawnAsync(
+            'curl',
+            [
+              ...['-sk', '-m', '10', '--noproxy', ''],
+              '-x',
+              `http://srt:${SandboxManager.getProxyAuthToken()}@127.0.0.1:${SandboxManager.getProxyPort()}`,
+              ...['--data-binary', `@${join(dir, 'body')}`],
+              // As given: what curl makes of such a byte in a URL differs by
+              // version.
+              ...['--request-target', '/caf\u00e9'],
+              `https://127.0.0.1:${port}/`,
+            ],
+            { encoding: 'utf8', timeout: 15_000 },
+          )
+          expect([curl.status, curl.stdout]).toEqual([
+            0,
+            'malformed request-target: percent-encode its bytes outside printable ASCII\n',
+          ])
+        }
+        expect(store.getViolations()).toHaveLength(8)
+      } finally {
+        rmSync(dir, { recursive: true, force: true })
+      }
+    },
+    60_000,
+  )
 })