Ws transport

[deathbyknowledge]

Adds a WS transport as an alternative to HTTP for all sandbox operations. When running inside workers/DOs this lets you multiplex everything over a single WS connection instead of making individual HTTP requests for each file op, command, etc. which counted towards sub-request limits.

The DO keeps a persistent WS connection to the container and routes all SDK calls through it. From the sdk side you just pass useWebSocket: true in options it handles everything transparently.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 611ca4b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/sandbox-sdk/@cloudflare/sandbox@253

commit: 611ca4b

[github-actions]

🐳 Docker Image Published

FROM cloudflare/sandbox:0.0.0-pr-253-be15028

Version: 0.0.0-pr-253-be15028

You can use this Docker image with the preview package from this PR.

[github-actions]

🐳 Docker Images Published

Default (no Python):

FROM cloudflare/sandbox:0.0.0-pr-253-ffde886

With Python:

FROM cloudflare/sandbox:0.0.0-pr-253-ffde886-python

Version: 0.0.0-pr-253-ffde886

Use the -python variant if you need Python code execution.

[claude]

Claude Code Review

The WebSocket transport feature is architecturally sound and addresses a real need. However, 5 critical issues from the previous review remain unresolved, plus several new issues were discovered.

Critical Issues (Must Fix)

Issue #1: SSE parsing data loss bug (ws-handler.ts:286)

• buffer.indexOf('', processedIndex) returns processedIndex, breaking buffer tracking
• Will cause stream corruption for multi-line SSE events
• See inline comment for fix

Issue #4: Send failures leave connections hung (ws-handler.ts:312)

• When ws.send() fails, error is logged but connection stays open
• Client hangs waiting for response that never arrives
• Need to close connection and abort processing

Issue #7: Buffer overflow DoS (ws-handler.ts:220)

• Unbounded buffer growth in SSE parsing
• If parsing fails or stream lacks delimiters, memory grows without limit
• Add MAX_BUFFER_SIZE check

Important Issues (Should Fix)

Issue #2: Module-level mutable state (index.ts:67)

• wsHandler violates DI pattern used throughout codebase
• Makes testing harder, creates fragile initialization order
• Move into createApplication() return value

Issue #3: Unsafe type casting (index.ts:78)

• as unknown as ServerWebSocket<WSData> bypasses type safety
• Violates CLAUDE.md "never use any" rule
• Define proper type interface for Bun's WebSocket

Issue #6: Connection race condition (ws-transport.ts:108)

• Two simultaneous connect() calls can create duplicate connections
• Set connectPromise before initiating connection

Issue #9: Incomplete cleanup (ws-transport.ts:488)

• handleClose rejects promises but doesn't close stream controllers
• Client-side streams left in error state without proper closure

Testing Gaps

• No E2E tests for WebSocket transport (tests cover exposing WS ports, not WS transport)
• No tests for send failures, mid-stream errors, buffer overflow
• Missing SSE parsing edge case tests (would have caught Issue #1)

Security Findings

• DoS via buffer overflow (Issue #7)
• No limit on concurrent pending requests (memory exhaustion)
• No size limit on incoming JSON messages

Verdict

Request changes. The bugs aren't edge cases - they'll hit production. The SSE parsing bug will corrupt streams, send failures will hang clients, and buffer overflow is a security issue. These are fixable but need attention before merge.

See inline comments for detailed fixes.

[claude]

Critical: SSE parsing bug causes data loss

When line === '', buffer.indexOf(line, processedIndex) returns processedIndex (empty string matches at current position). This makes processedIndex = processedIndex + 0 + 1, only advancing 1 char, which breaks buffer tracking.

This bug is repeated at lines 292, 295, 297.

Fix: Replace with split-based approach:

private parseSSEEvents(buffer: string): {
  events: Array<{ event?: string; data: string }>;
  remaining: string;
} {
  const events: Array<{ event?: string; data: string }> = [];
  const blocks = buffer.split('\n\n');
  const remaining = blocks.pop() || '';
  
  for (const block of blocks) {
    const lines = block.split('\n');
    let event: string | undefined;
    const dataLines: string[] = [];
    
    for (const line of lines) {
      if (line.startsWith('event:')) event = line.substring(6).trim();
      else if (line.startsWith('data:')) dataLines.push(line.substring(5).trim());
    }
    
    if (dataLines.length > 0) {
      events.push({ event, data: dataLines.join('\n') });
    }
  }
  
  return { events, remaining };
}

[claude]

Critical: Unbounded buffer growth (DoS vector)

If parseSSEEvents has bugs (Issue #1) or stream lacks proper delimiters, buffer grows unboundedly. Add limit:

const MAX_BUFFER_SIZE = 1024 * 1024; // 1MB

buffer += decoder.decode(value, { stream: true });

if (buffer.length > MAX_BUFFER_SIZE) {
  this.sendError(ws, requestId, 'STREAM_ERROR', 
    'Buffer overflow: stream too large without delimiters', 413);
  reader.releaseLock();
  return;
}

[claude]

Critical: Send failures leave client hung

When ws.send() fails, error is logged but connection stays open. Client waits for response that never arrives (until 2min timeout).

Fix: Return status and close on failure:

private send(ws: ServerWebSocket<WSData>, message: WSServerMessage): boolean {
  try {
    ws.send(JSON.stringify(message));
    return true;
  } catch (error) {
    this.logger.error('Send failed', error instanceof Error ? error : new Error(String(error)));
    try { ws.close(1011, 'Send failed'); } catch {}
    return false;
  }
}

// In handleStreamingResponse:
if (!this.send(ws, chunk)) {
  reader.releaseLock();
  return;
}

[claude]

Issue: Module-level mutable state violates DI pattern

wsHandler declared at module level but initialized here. This creates fragile initialization order and makes testing harder. Move into createApplication() return:

return {
  fetch: async (req, server) => { ... },
  router,
  wsHandler: new WebSocketHandler(router, logger)
};

Then use app.wsHandler in websocket handlers below.

[claude]

Issue: Unsafe type cast bypasses type safety

as unknown as ServerWebSocket<WSData> violates CLAUDE.md "never use any" rule. Define proper type:

interface BunWebSocket {
  data: WSData;
  send(message: string): void;
  close(code?: number, reason?: string): void;
  readyState: number;
}

// Then:
open(ws: BunWebSocket) {
  app.wsHandler.onOpen(ws);
}

Update WebSocketHandler signature to match.

[claude]

Issue: Race condition in connection state

If two requests call connect() simultaneously, both can pass the if (!this.connectPromise) check before either sets it, creating duplicate connections.

Fix: Set promise before starting:

if (this.connectPromise) {
  return this.connectPromise;
}

this.connectPromise = this.doConnect();
try {
  await this.connectPromise;
} finally {
  this.connectPromise = null;
}

Extract connection logic to private async doConnect().

[claude]

Issue: Incomplete cleanup for streaming requests

handleClose rejects promises but doesn't close stream controllers. Streams are left in error state without proper closure.

Fix:

for (const [id, pending] of this.pendingRequests) {
  if (pending.streamController) {
    try {
      pending.streamController.error(error);
    } catch {}
  }
  pending.reject(error);
}

[claude]

Suggestion: More unsafe casting

Define proper type for Workers WebSocket upgrade:

interface WorkersWebSocketUpgradeResponse extends Response {
  webSocket: WebSocket & { accept: () => void };
}

const wsResponse = response as WorkersWebSocketUpgradeResponse;
if (!wsResponse.webSocket) throw new Error('No WebSocket in upgrade response');
wsResponse.webSocket.accept();
this.ws = wsResponse.webSocket;