Skip to content

Conversation

@ghostwriternr
Copy link
Member

Resolves port conflict issues by making the control plane port configurable.

Changes

This PR adds support for configuring the control plane port (default: 3000) via the SANDBOX_CONTROL_PLANE_PORT environment variable. This allows users to avoid port conflicts when port 3000 is already in use by their application.

Key Changes:

  1. Container Runtime (packages/sandbox-container/):

    • Added CONTROL_PLANE_PORT configuration to config.ts with validation (1-65535 range)
    • Updated index.ts to use configurable port with error handling for port conflicts
    • Enhanced error logging with actionable solutions when port conflicts occur
    • Updated security-service.ts to dynamically protect the configured control plane port
  2. SDK (packages/sandbox/):

    • Added controlPlanePort property to Sandbox class
    • Updated constructor to read SANDBOX_CONTROL_PLANE_PORT from environment
    • Updated SandboxClient initialization to use configurable port
    • Updated determinePort method to route control plane requests to configured port
  3. Documentation:

    • Updated CLAUDE.md with configuration instructions and examples

Testing:

  • ✅ All unit tests passing (409 tests)
  • ✅ E2E tests passing (smoke test + environment workflow)
  • ✅ Linter and typecheck passing

Usage:

In Dockerfile:

ENV SANDBOX_CONTROL_PLANE_PORT=3001

In Worker environment:

env.SANDBOX_CONTROL_PLANE_PORT = '3001'

Error Handling:

When a port conflict occurs, the container now provides clear error messages:

Port 3000 is already in use. The Sandbox SDK requires this port for its control plane. Either:
1. Stop the process using port 3000, or
2. Set SANDBOX_CONTROL_PLANE_PORT environment variable to a different port in your Dockerfile

Link to Devin run: https://app.devin.ai/sessions/001d37fe2d324d48bbd3a27a884fd98e
Requested by: @ghostwriternr

devin-ai-integration bot and others added 2 commits October 28, 2025 12:58
Co-Authored-By: Naresh Ramesh <[email protected]>
Add SANDBOX_CONTROL_PLANE_PORT environment variable to configure the
control plane port (default: 3000) used for internal communication
between the Sandbox DO and the container.

This resolves port conflicts when users need port 3000 for their
applications. The port can be configured in both the Worker environment
and the Dockerfile.

Additionally, improve error detection and logging for port conflicts.
When the control plane port is already in use, the container now
provides clear error messages with actionable solutions.

Co-Authored-By: Naresh Ramesh <[email protected]>
@changeset-bot
Copy link

changeset-bot bot commented Oct 28, 2025

🦋 Changeset detected

Latest commit: bee3e07

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@cloudflare/sandbox Patch
@repo/sandbox-container Patch

Not sure what this means? Click here to learn what changesets are.

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

@claude
Copy link

claude bot commented Oct 28, 2025

Claude Code Review

Summary: This PR makes the control plane port configurable to resolve port conflicts. The implementation is mostly solid with good test coverage.

Issues Found

1. Validation inconsistency between SDK and container (sandbox.ts:96-101 vs config.ts:96-97)

The SDK validates the port range as 1-65535, but the container uses the same range. However, both should enforce the 1024-65535 range consistently with the validatePort() function, which requires ports >= 1024 (see security.ts:28). Currently, the container would accept port 80 from the env var but then fail at runtime when validatePort() is called.

Fix: Both config validation points should use the same logic as validatePort() - minimum port 1024, not 1.

2. Port conflict detection is fragile (index.ts:60-61)

The error handling relies on string matching err.message.includes('EADDRINUSE'), but the actual Bun error might not contain this exact string. The try-catch catches the error, but if the string match fails, users get a less helpful error message.

Fix: Check err.code === 'EADDRINUSE' if available, or make the error message more generic about port conflicts.

3. Async getControlPlanePort in proxyToSandbox (request-handler.ts:43)

The method getControlPlanePort() is marked as async (await sandbox.getControlPlanePort()), but the actual implementation is synchronous (sandbox.ts:88-90). This adds unnecessary overhead in the hot path of every request.

Fix: Remove async/await - make it a synchronous getter since controlPlanePort is set in the constructor.

Minor Observations

  • Good: Comprehensive test coverage for the new functionality
  • Good: Clear error messages with actionable solutions
  • Good: Proper validation at multiple layers (env, runtime, request handler)
  • The changeset appropriately marks this as a patch version bump

Verdict

Approve with suggested fixes for the validation inconsistency and async overhead.

Remove all hard coded port 3000 references in request routing and
validation. Includes unit tests for configuration parsing and dynamic
validation.
@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 28, 2025

Open in StackBlitz

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

commit: bee3e07

@github-actions
Copy link
Contributor

🐳 Docker Image Published

FROM cloudflare/sandbox:0.0.0-pr-161-e815f30

Version: 0.0.0-pr-161-e815f30

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

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.

1 participant