From a077ace50310c55301a75a1972ec935fb76ce0ec Mon Sep 17 00:00:00 2001 From: Nathan Flurry Date: Thu, 25 Jun 2026 06:47:47 -0700 Subject: [PATCH] fix(runtime): strip module shebang, stream large http responses, reject oversized vm.fetch by content-length - v8-runtime: strip a leading #!shebang in build_module_source so the patched Claude agent SDK (which begins with #!/usr/bin/env node) parses in guest V8 instead of SyntaxError-crashing the adapter on session/new (host Node strips it; the guest loader did not). - v8-bridge: ServerResponseBridge streams a single res.end(body) to the connection socket in bounded slices (incremental byteLength) instead of buffering the whole body + its serialize/transmit copies, which tripped the isolate heap-limit OOM guard on multi-MB responses. Socket-backed path only; loopback http unchanged. - sidecar: vm.fetch rejects a response whose declared Content-Length exceeds VM_FETCH_BUFFER_LIMIT_BYTES before stalling on a body it cannot stream through the bounded kernel socket buffer; keeps the per-VM configured-limit payload check intact. - limits-inventory: classify DEFAULT_HEAP_LIMIT_MB (policy) and MAX_V8_USERLAND_CODE_BYTES (invariant). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/execution/assets/v8-bridge.source.js | 131 ++++++++++++++++-- crates/sidecar/src/execution.rs | 12 ++ .../tests/fixtures/limits-inventory.json | 13 ++ crates/v8-runtime/src/execution.rs | 33 +++++ 4 files changed, 175 insertions(+), 14 deletions(-) diff --git a/crates/execution/assets/v8-bridge.source.js b/crates/execution/assets/v8-bridge.source.js index 00d3dae09..07c6868b0 100644 --- a/crates/execution/assets/v8-bridge.source.js +++ b/crates/execution/assets/v8-bridge.source.js @@ -13623,6 +13623,10 @@ ${headerLines}\r _rawTrailerNames = /* @__PURE__ */ new Map(); _informational = []; _pendingRawInfoBuffer = ""; + _streamSocket = null; + _streamRequest = null; + _streamedDirectly = false; + _streamCloseConnection = false; constructor() { this._closedPromise = new Promise((resolve) => { this._resolveClosed = resolve; @@ -13841,6 +13845,28 @@ ${headerLines}\r chunk = chunkOrCallback; endCallback = typeof encodingOrCallback === "function" ? encodingOrCallback : callback; } + // Streaming fast path for socket-backed servers: a single `res.end(body)` + // with no prior `res.write()` flushes headers then streams the body to the + // connection socket in bounded slices. This avoids materializing the whole + // body (plus its serialize/transmit copies) at once — a multi-MB response + // otherwise trips the guest isolate heap-limit OOM guard before the host + // can apply its own response-size limit. Per-slice `socket.destroyed` + // checks also make the host's mid-stream rejection close graceful instead + // of crashing the guest. + if ( + this._streamSocket && + this._chunks.length === 0 && + !this.writableFinished && + !this._streamedDirectly + ) { + const encoding = + typeof encodingOrCallback === "string" ? encodingOrCallback : void 0; + this._streamEndBody(chunk, encoding); + if (typeof endCallback === "function") { + queueMicrotask(endCallback); + } + return this; + } if (chunk != null) { if (typeof chunk === "string" && typeof encodingOrCallback === "string") { this._appendChunk(chunk, encodingOrCallback, false); @@ -13854,6 +13880,57 @@ ${headerLines}\r } return this; } + _streamEndBody(body, encoding) { + const isString = typeof body === "string"; + const SLICE_BYTES = 256 * 1024; + // Compute the body byte length in bounded slices. `Buffer.byteLength` on a + // whole multi-MB string allocates enough that, with the isolate already + // near its heap cap, it trips the OOM guard before a single byte is sent. + let byteLength = 0; + if (body != null) { + if (isString) { + for (let offset = 0; offset < body.length; offset += SLICE_BYTES) { + byteLength += Buffer.byteLength( + body.slice(offset, offset + SLICE_BYTES), + encoding, + ); + } + } else { + byteLength = body.length; + } + } + if (!this._headers.has("content-length") && !this._headers.has("transfer-encoding")) { + this._headers.set("content-length", String(byteLength)); + this._rawHeaderNames.set("content-length", "Content-Length"); + } + this.headersSent = true; + // Serialize headers only (no buffered chunks => empty body in the payload), + // then stream the real body separately in bounded slices. + const headerResponse = this.serialize(); + const built = serializeLoopbackResponse(headerResponse, this._streamRequest, true); + this._streamCloseConnection = built.closeConnection; + this._streamedDirectly = true; + this.outputSize += byteLength; + if (!this._streamSocket.destroyed && built.payload.length > 0) { + this._streamSocket.write(built.payload); + } + if (body != null && byteLength > 0) { + if (isString) { + for (let offset = 0; offset < body.length; offset += SLICE_BYTES) { + if (this._streamSocket.destroyed) break; + this._streamSocket.write( + Buffer.from(body.slice(offset, offset + SLICE_BYTES), encoding), + ); + } + } else { + for (let offset = 0; offset < body.length; offset += SLICE_BYTES) { + if (this._streamSocket.destroyed) break; + this._streamSocket.write(body.subarray(offset, offset + SLICE_BYTES)); + } + } + } + this._finalize(); + } getHeaderNames() { return Array.from(this._headers.keys()); } @@ -14603,12 +14680,18 @@ ${headerLines}\r server._endRequestDispatch(); } } - async function dispatchSocketBackedServerRequest(server, requestInput) { + async function dispatchSocketBackedServerRequest(server, requestInput, streamSocket) { const request = typeof requestInput === "string" ? JSON.parse(requestInput) : requestInput; const incoming = new ServerIncomingMessage(request); const outgoing = new ServerResponseBridge(); incoming.socket = outgoing.socket; incoming.connection = outgoing.socket; + // Enable the streaming fast path so a single large `res.end(body)` is written + // to the connection socket in slices instead of buffered + serialized whole. + if (streamSocket) { + outgoing._streamSocket = streamSocket; + outgoing._streamRequest = request; + } server._beginRequestDispatch(); try { try { @@ -14631,15 +14714,24 @@ ${headerLines}\r } await outgoing.waitForClose(); let aborted = false; + const abortRequest = () => { + if (aborted) { + return; + } + aborted = true; + incoming._abort(); + }; + if (outgoing._streamedDirectly) { + // Response already written straight to the socket; nothing left to serialize. + return { + streamedDirectly: true, + closeConnection: outgoing._streamCloseConnection, + abortRequest + }; + } return { responseJson: JSON.stringify(outgoing.serialize()), - abortRequest: () => { - if (aborted) { - return; - } - aborted = true; - incoming._abort(); - } + abortRequest }; } finally { server._endRequestDispatch(); @@ -14732,20 +14824,31 @@ ${headerLines}\r server._emit("upgrade", incoming, socket, parsed.upgradeHead); return; } - const { responseJson } = await dispatchSocketBackedServerRequest(server, parsed.request); + const result = await dispatchSocketBackedServerRequest( + server, + parsed.request, + socket, + ); if (detached || socket.destroyed) { return; } - const response = JSON.parse(responseJson); // Keep-alive for socket-backed HTTP servers is intentionally deferred: // pipelined bytes already in `buffer` drain, then this connection closes. // Revisit when the bridge owns full Node-compatible request lifecycle // timers and per-socket request limits. - const serialized = serializeLoopbackResponse(response, parsed.request, true); - if (!closeAfterDrain && serialized.payload.length > 0) { - socket.write(serialized.payload); + let mustClose; + if (result.streamedDirectly) { + // Response was already streamed straight to the socket by res.end(). + mustClose = result.closeConnection; + } else { + const response = JSON.parse(result.responseJson); + const serialized = serializeLoopbackResponse(response, parsed.request, true); + if (!closeAfterDrain && serialized.payload.length > 0) { + socket.write(serialized.payload); + } + mustClose = serialized.closeConnection; } - if (serialized.closeConnection) { + if (mustClose) { closeAfterDrain = true; if (buffer.length === 0) { finishSocket(); diff --git a/crates/sidecar/src/execution.rs b/crates/sidecar/src/execution.rs index f4eea8a7a..e58057c33 100644 --- a/crates/sidecar/src/execution.rs +++ b/crates/sidecar/src/execution.rs @@ -16548,6 +16548,18 @@ fn parse_kernel_http_fetch_response( ))); } else if let Some(content_length) = content_length { let body_end = body_start.saturating_add(content_length); + // Reject a response whose declared length exceeds the hard raw-buffer cap + // before waiting for the whole body: such a body can never satisfy the + // limit, and the guest cannot stream it through the bounded kernel socket + // buffer before the request deadline, so the read loop would otherwise + // stall on an incomplete body until it times out. (A smaller-but-still- + // over-a-configured-limit response is delivered in full and rejected by + // the payload-size check, preserving that error's message.) + if body_end > VM_FETCH_BUFFER_LIMIT_BYTES { + return Err(SidecarError::Execution(format!( + "vm.fetch raw response buffer is {body_end} bytes, limit is {VM_FETCH_BUFFER_LIMIT_BYTES}" + ))); + } if buffer.len() < body_end { return Ok(None); } diff --git a/crates/sidecar/tests/fixtures/limits-inventory.json b/crates/sidecar/tests/fixtures/limits-inventory.json index cec279784..1a4da3051 100644 --- a/crates/sidecar/tests/fixtures/limits-inventory.json +++ b/crates/sidecar/tests/fixtures/limits-inventory.json @@ -930,6 +930,19 @@ "class": "invariant", "rationale": "Build-time artifact sanity guard on first-party assets, not guest input." }, + { + "name": "MAX_V8_USERLAND_CODE_BYTES", + "path": "crates/v8-runtime/src/snapshot.rs", + "class": "invariant", + "rationale": "Sanity bound on the client-configured agent-SDK userland snapshot bundle; trusted-config artifact guard, not guest input." + }, + { + "name": "DEFAULT_HEAP_LIMIT_MB", + "path": "crates/v8-runtime/src/isolate.rs", + "class": "policy", + "rationale": "Engine-side default for the wired V8 isolate heap limit (128 MiB, Cloudflare-matching); operator-raisable via the configured jsRuntime heap limit.", + "wired": "VmLimits.js_runtime.v8_heap_limit_mb" + }, { "name": "TRAILING_OUTPUT_DRAIN_MAX_MS", "path": "packages/core/src/kernel-proxy.ts", diff --git a/crates/v8-runtime/src/execution.rs b/crates/v8-runtime/src/execution.rs index 9ca1b497f..7a2b9dcd6 100644 --- a/crates/v8-runtime/src/execution.rs +++ b/crates/v8-runtime/src/execution.rs @@ -2046,12 +2046,27 @@ fn throw_module_error(scope: &mut v8::HandleScope, message: &str) { /// Detect if source code is likely CommonJS (not ESM). /// Checks for module.exports, exports.X, or require() patterns without ESM import/export. +/// Node strips a leading shebang (`#!`) line before parsing a module. The guest +/// loader must match, or modules shipped as executables (CLI/SDK bundles that +/// begin with `#!/usr/bin/env node`) fail with "Invalid or unexpected token" on +/// the `#`. The newline is preserved so line numbers in stack traces stay aligned. +fn strip_leading_shebang(source: &str) -> &str { + match source.strip_prefix("#!") { + Some(rest) => match rest.find('\n') { + Some(idx) => &rest[idx..], + None => "", + }, + None => source, + } +} + fn build_module_source( scope: &mut v8::HandleScope, raw_source: &str, resolved_path: &str, module_format: Option, ) -> String { + let raw_source = strip_leading_shebang(raw_source); let normalized_path = resolved_path.to_ascii_lowercase(); if normalized_path.ends_with(".json") || module_format == Some(ResolvedModuleFormat::Json) { return build_json_esm_shim(resolved_path); @@ -3230,6 +3245,24 @@ mod tests { use std::io::{Cursor, Write}; use std::sync::{Arc, Mutex}; + #[test] + fn strip_leading_shebang_matches_node() { + // Shebang stripped (newline preserved so line numbers hold). + assert_eq!( + strip_leading_shebang("#!/usr/bin/env node\nexport const x = 1;\n"), + "\nexport const x = 1;\n" + ); + // No shebang -> untouched. + assert_eq!( + strip_leading_shebang("export const x = 1;\n"), + "export const x = 1;\n" + ); + // `#` not at byte 0 -> untouched (only a leading shebang is special). + assert_eq!(strip_leading_shebang(" #!nope\n"), " #!nope\n"); + // Whole file is just a shebang. + assert_eq!(strip_leading_shebang("#!/usr/bin/env node"), ""); + } + /// Shared writer that captures output for test inspection struct SharedWriter(Arc>>);