fix(claude): bound unfinished picker uploads and refusal drains

src/claude/intercept/picker-listener.ts

@@ -34,6 +34,12 @@ export interface PickerListenerOptions {
   maxEncodedBytes?: number;
   /** Test seam: concurrent upstream requests; production uses PICKER_MAX_ACTIVE_UPSTREAMS. */
   maxActiveUpstreams?: number;
+  /** Test seam: maximum time for a client upload to finish. */
+  uploadDeadlineMs?: number;
+  /** Test seam: maximum time without upload progress. */
+  uploadIdleTimeoutMs?: number;
+  /** Test seam: observe or hold the relay's upstream writable before piping input. */
+  onUpstreamRequest?: (request: ReturnType<typeof httpsRequest>) => void;
   /**
    * Test seam: arms the drop of a connection whose backend ended, after PICKER_ABANDONED_INPUT_LINGER_MS;
    * returns a cancel function. Production uses a timer.
@@ -56,6 +62,9 @@ export const PICKER_MAX_ACTIVE_UPSTREAMS = 256;
 export const PICKER_MAX_CONCURRENT_STREAMS = 100;
 /** A connection that has not sent a complete ClientHello by then is dropped. */
 const CLIENT_HELLO_TIMEOUT_MS = 10_000;
+const PICKER_UPLOAD_DEADLINE_MS = 60_000;
+const PICKER_UPLOAD_IDLE_TIMEOUT_MS = 15_000;
+const MAX_TIMER_DELAY_MS = 2_147_483_647;
 /**
  * Once the backend has ended a connection and its reply has been flushed, input the client still
  * sends is discarded for at most this long before the connection is dropped. A cooperative client
@@ -163,6 +172,13 @@ export async function startPickerListener(options: PickerListenerOptions): Promi
   const upstream = options.upstream ?? { host: "claude.ai", port: 443, servername: "claude.ai" };
   const cap = options.maxEncodedBytes ?? BOOTSTRAP_MAX_ENCODED_BYTES;
   const maxActiveUpstreams = options.maxActiveUpstreams ?? PICKER_MAX_ACTIVE_UPSTREAMS;
+  const uploadDeadlineMs = options.uploadDeadlineMs ?? PICKER_UPLOAD_DEADLINE_MS;
+  const uploadIdleTimeoutMs = options.uploadIdleTimeoutMs ?? PICKER_UPLOAD_IDLE_TIMEOUT_MS;
+  for (const [name, value] of [["uploadDeadlineMs", uploadDeadlineMs], ["uploadIdleTimeoutMs", uploadIdleTimeoutMs]] as const) {
+    if (!Number.isFinite(value) || value <= 0 || value > MAX_TIMER_DELAY_MS) {
+      throw new RangeError(`${name} must be a positive finite timer duration`);
+    }
+  }
   const scheduleAbandonedInputDrop = options.scheduleAbandonedInputDrop ?? ((drop: () => void, ms: number) => {
     const timer = setTimeout(drop, ms);
     return () => clearTimeout(timer);
@@ -235,7 +251,64 @@ export async function startPickerListener(options: PickerListenerOptions): Promi
       // Do not leave a refused, still-uploading stream behind after delivering its empty reply.
       if (uploading && !req.complete) {
         req.once("error", () => res.destroy());
-        responseWritable.once("finish", () => closeInput(true));
+        if (h2) {
+          responseWritable.once("finish", () => closeInput(true));
+        } else {
+          let settled = false;
+          let replyFinished = false;
+          let inputFinished = false;
+          let deadlineTimer: NodeJS.Timeout | undefined;
+          let idleTimer: NodeJS.Timeout | undefined;
+          const cleanup = () => {
+            if (settled) return;
+            settled = true;
+            if (deadlineTimer) clearTimeout(deadlineTimer);
+            if (idleTimer) clearTimeout(idleTimer);
+            req.off("data", onProgress);
+            req.off("end", onBodyEnd);
+            req.off("close", onBodyGone);
+            req.off("error", onBodyGone);
+            responseWritable.off("finish", onReplyFinished);
+          };
+          // The overridden destroySoon already sent a graceful FIN and resumed input, so a
+          // timeout must destroy: another end() on that socket would be a no-op and a client
+          // that keeps uploading could hold the half-closed socket until shutdown.
+          const closeOnTimeout = () => {
+            cleanup();
+            req.destroy();
+          };
+          const onProgress = () => {
+            if (settled || !deadlineTimer) return;
+            if (idleTimer) clearTimeout(idleTimer);
+            idleTimer = setTimeout(closeOnTimeout, uploadIdleTimeoutMs);
+            idleTimer.unref?.();
+          };
+          // A body that finishes cleanly leaves a keep-alive socket the client may
+          // reuse; only a deadline or a premature close ends it.
+          const onBodyEnd = () => {
+            inputFinished = true;
+            if (replyFinished) cleanup();
+          };
+          const onBodyGone = () => {
+            if (inputFinished) return;
+            inputFinished = true;
+            if (replyFinished) { cleanup(); closeInput(true); }
+          };
+          const onReplyFinished = () => {
+            if (settled) return;
+            replyFinished = true;
+            if (inputFinished || req.complete) { cleanup(); return; }
+            deadlineTimer = setTimeout(closeOnTimeout, uploadDeadlineMs);
+            deadlineTimer.unref?.();
+            onProgress();
+          };
+          req.on("data", onProgress);
+          req.once("end", onBodyEnd);
+          req.once("close", onBodyGone);
+          req.once("error", onBodyGone);
+          responseWritable.once("finish", onReplyFinished);
+          req.resume();
+        }
       }
       res.writeHead(status, { "Content-Length": "0", ...(!h2 && uploading && !req.complete ? { Connection: "close" } : {}) });
       res.end();
@@ -271,6 +344,9 @@ export async function startPickerListener(options: PickerListenerOptions): Promi
     if (bootstrap) headers.push("Accept-Encoding", narrowBootstrapAcceptEncoding());
     let upReq: ReturnType<typeof httpsRequest>;
     let upstreamResponded = false;
+    // Upload clocks only bound the pre-response wait: once upstream answers, the
+    // reply must be allowed to finish even if the client's input never does.
+    let stopUploadClocks: () => void = () => {};
     try {
       // The HTTP/1.1 client validates the method, path and header values synchronously.
       upReq = httpsRequest({
@@ -279,6 +355,7 @@ export async function startPickerListener(options: PickerListenerOptions): Promi
         method, path: req.url, headers, maxHeaderSize: PICKER_MAX_HEADER_BYTES,
         }, upRes => {
         upstreamResponded = true;
+        stopUploadClocks();
         const status = upRes.statusCode ?? 502;
         const originalHeaders = filteredHeaders(upRes.rawHeaders);
         const sendHead = (raw: string[]) => {
@@ -358,9 +435,36 @@ export async function startPickerListener(options: PickerListenerOptions): Promi
     if (h2) (req as Http2ServerRequest).stream.once("close", onClientClose);
     if (uploading) {
       let uploadFinished = false;
+      let deadlineTimer: NodeJS.Timeout | undefined;
+      let idleTimer: NodeJS.Timeout | undefined;
+      let expireUpload: () => void = () => {};
+      const clearUploadTimers = () => {
+        if (deadlineTimer) clearTimeout(deadlineTimer);
+        if (idleTimer) clearTimeout(idleTimer);
+        deadlineTimer = undefined;
+        idleTimer = undefined;
+      };
+      stopUploadClocks = clearUploadTimers;
+      const onUploadIdle = () => {
+        idleTimer = undefined;
+        if (upstreamResponded) return;
+        // A full upstream buffer pauses the pipe: 'data' stops even while the client
+        // keeps sending, so backpressure is not client idleness. Recheck each interval
+        // until the upstream drains; the absolute deadline still bounds the upload.
+        if (upReq.writableNeedDrain) { onUploadProgress(); return; }
+        expireUpload();
+      };
+      const onUploadProgress = () => {
+        if (upstreamResponded) return;
+        if (idleTimer) clearTimeout(idleTimer);
+        idleTimer = setTimeout(onUploadIdle, uploadIdleTimeoutMs);
+        idleTimer.unref?.();
+      };
       const finishUpload = () => {
         if (uploadFinished) return;
         uploadFinished = true;
+        clearUploadTimers();
+        req.off("data", onUploadProgress);
         req.off("end", finishUpload);
         req.off("close", finishUpload);
         upReq.off("close", onUpstreamClose);
@@ -378,17 +482,28 @@ export async function startPickerListener(options: PickerListenerOptions): Promi
           upReq.destroy();
         }
       };
+      expireUpload = () => {
+        if (uploadFinished || upstreamResponded) return;
+        clientGone = true;
+        stopUnfinishedUpload();
+        release();
+      };
       const onResponseFinish = () => stopUnfinishedUpload(true);
       const onUpstreamClose = () => {
         if (!upstreamResponded && !res.writableEnded) stopUnfinishedUpload();
       };
       req.once("end", finishUpload);
       req.once("close", finishUpload);
+      req.on("data", onUploadProgress);
       upReq.once("close", onUpstreamClose);
       responseWritable.once("finish", onResponseFinish);
       res.once("close", stopUnfinishedUpload);
       if (h2) (req as Http2ServerRequest).stream.once("close", finishUpload);
+      deadlineTimer = setTimeout(expireUpload, uploadDeadlineMs);
+      deadlineTimer.unref?.();
+      onUploadProgress();
     }
+    options.onUpstreamRequest?.(upReq);
     req.pipe(upReq);
   };
   server.on("request", relay);

structure/clients/claude-desktop.md

@@ -220,9 +220,9 @@ natively, counting name + value + 32 bytes per field and rejecting an oversized
 The relay retains its 256-request aggregate ceiling across both HTTP versions and all sessions.
 Upload framing, not the method, determines whether input cleanup is needed: HTTP/2 headers
 without END_STREAM, or HTTP/1.1 transfer encoding or a positive Content-Length.
-Rejected unfinished uploads close after their empty reply's writable finishes; HTTP/2 uses the
+Rejected unfinished uploads preserve the empty reply before cleanup; HTTP/2 uses the
 underlying stream's finish event rather than the compatibility response's finish event, which
-waits for both stream halves to close. HTTP/1.1 refusals advertise `Connection: close` only while input is unfinished.
+waits for both stream halves to close.
 Early replies stop relaying unfinished input only after the downstream writable finishes or the
 response closes; upstream request closure alone does not prove a buffered reply was delivered.
 After a complete response, HTTP/2 closes only that stream with `RST_STREAM NO_ERROR` (RFC 9113
@@ -231,12 +231,16 @@ After a complete response, HTTP/2 closes only that stream with `RST_STREAM NO_ER
 HTTP/1.1 refusals, generated 502s and early replies advertise `Connection: close` only while input is unfinished; after the writable finishes,
 the listener unpipes the upstream upload, reads and discards remaining input, and ends the socket
 gracefully so queued response bytes flush. Errors and premature downstream closes still destroy
-the request. The request-side drain adds no upload deadline or byte cap and does not wait for request-body
+the request. The request-side drain adds no byte cap and does not wait for request-body
 completion before ending the response side. Bun 1.4.3's HTTP server ends a `Connection: close`
 response with `socket.destroySoon()` before the listener's own finish handler runs, which reset
 the splice before the reply reached the client; every HTTP/1.1 reply that closes unfinished input
 (refusal, generated 502, early upstream reply) therefore replaces that socket's `destroySoon` with
-the same resume-and-`end()`. The TCP front
+the same resume-and-`end()`. A refused HTTP/1.1 socket whose client keeps uploading past that
+graceful close is then destroyed outright at the upload deadline or after 15 s without progress.
+On the relay path the 60-second absolute and 15-second idle-progress deadlines close client input
+and upstream work and release the aggregate slot; upstream backpressure only
+suspends the idle clock, never the absolute deadline. The TCP front
 enables half-open reads once a ClientHello is recognized and, when the backend side ends, unpipes
 and keeps discarding client ciphertext, so a client still writing an abandoned upload reads the
 reply and FIN and can close (Bun 1.4.2 otherwise left it open). That front-side discard is bounded:
@@ -245,11 +249,12 @@ once the reply has flushed to the client socket (`finish`), the connection is dr
 hold it until shutdown (#6984). The timer is armed only after the reply has left the process, so
 it cannot cut a reply that is still being written; `finish` is not peer receipt, though, and a client
 that also stops reading for the whole budget can still lose unread reply bytes when the drop fires.
-A connection that closes first cancels the timer. Body completion removes the input-cleanup listeners,
+A connection that closes first cancels the timer. Body completion removes the input-cleanup listeners and both timers,
 leaving long-lived responses and SSE subscriptions independent of upload cleanup. Completed HTTP/1.1 uploads retain keep-alive on generated 502s and can reuse the same client socket.
 `tests/claude-integration/claude-picker-upload.test.ts` covers headers-only refusals on both
 protocols, byte-exact early replies under backpressure, completed uploads with SSE, cancellation,
 failure and shutdown cleanup, and healthy sibling streams.
+The upload regression fixture holds and observes the relay's actual upstream writable before asserting idle and absolute-deadline behavior.
 The picker CA (`picker-ca.ts`) carries critical
 name constraints permitting only `claude.ai` and excluding every IPv4 and IPv6 address. Its exportable
 signing identity is protected by the OS credential store and scoped to the canonical config directory;