Add custom token support to exposePort for stable preview URLs #329
Conversation
Allows users to pass a custom token to exposePort() to maintain consistent preview URLs across deployments and container restarts. This solves the issue of URLs breaking when sandboxes are restarted during deployments, enabling stable URL sharing in production. Token validation ensures 16-character length and URL-safe characters (lowercase letters, numbers, hyphens, underscores) matching the existing routing pattern. Backward compatible - tokens are optional and auto-generated if not provided. Includes comprehensive unit tests for validation logic and custom token usage.
Update token validation to accept variable-length tokens (1-63 chars) instead of fixed 16-character requirement. This provides more flexibility for users to choose meaningful token names while maintaining DNS subdomain compatibility. Changes: - Updated validateCustomToken() to check 1-63 char range - Updated request-handler regex to accept variable-length tokens - Updated all documentation and JSDoc comments - Updated unit tests to cover variable-length tokens - Updated changeset with new token length requirements
🦋 Changeset detectedLatest commit: 6ca6c73 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Add documentation for the new custom token parameter in exposePort() that enables stable preview URLs across container restarts and deployments. Updated files: - API reference (ports.mdx): Added token parameter documentation with examples - Concepts (preview-urls.mdx): Updated URL format and added URL stability section - Guide (expose-services.mdx): Added practical examples and use case guidance Related PR: cloudflare/sandbox-sdk#329
commit: |
🐳 Docker Images PublishedDefault: FROM cloudflare/sandbox:0.0.0-pr-329-f158959With Python: FROM cloudflare/sandbox:0.0.0-pr-329-f158959-pythonWith OpenCode: FROM cloudflare/sandbox:0.0.0-pr-329-f158959-opencodeVersion: Use the 📦 Standalone BinaryFor arbitrary Dockerfiles: COPY --from=cloudflare/sandbox:0.0.0-pr-329-f158959 /container-server/sandbox /sandbox
ENTRYPOINT ["/sandbox"]Download via GitHub CLI: gh run download 20868388697 -n sandbox-binaryExtract from Docker: docker run --rm cloudflare/sandbox:0.0.0-pr-329-f158959 cat /container-server/sandbox > sandbox && chmod +x sandbox |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
Critical Issue: Validation Ordering Bug
Location: packages/sandbox/src/sandbox.ts:2133-2151
Token validation happens after the container API call. Invalid tokens are sent to the container before validation runs:
// Line 2133: Container called FIRST
await this.client.ports.exposePort(
port,
sessionId,
options?.name,
options?.token // Could be invalid
);
// Line 2151: Validation SECOND (too late)
if (options.token) {
this.validateCustomToken(options.token);
token = options.token;
}If a user passes token: "INVALID@#$", it reaches the container unchecked. While validation eventually throws, the container has already processed the request.
Move validation before the container call:
// Validate token FIRST
let token: string;
if (options.token) {
this.validateCustomToken(options.token);
token = options.token;
} else {
token = this.generatePortToken();
}
const sessionId = await this.ensureDefaultSession();
await this.client.ports.exposePort(
port,
sessionId,
options?.name,
options.token
);Missing Test Coverage
Three critical integration scenarios aren't tested:
- 63-character boundary: Tests reject 64 chars but don't verify 63 chars works (packages/sandbox/tests/sandbox.test.ts:293)
- Token storage: No test verifies custom tokens are stored correctly, unlike auto-generated tokens (line 340-346)
- Request routing: All request-handler tests use 16-char tokens; none test the regex change from
{16}to+with variable lengths (packages/sandbox/tests/request-handler.test.ts)
Add tests that verify custom tokens flow through correctly, not just that bad tokens are rejected.
Verdict
Fix the validation ordering before merge. The test gaps should be addressed but aren't blocking.
| let token: string; | ||
| if (options.token) { | ||
| // Validate custom token | ||
| this.validateCustomToken(options.token); |
There was a problem hiding this comment.
Validation happens after the container API call at line 2133. Move the entire token generation/validation block (lines 2147-2155) before the client.ports.exposePort() call to validate before side effects.
Update custom token validation to enforce a maximum of 16 characters to match the auto-generated token length and ensure consistency. Changes: - Updated validateCustomToken() to check 1-16 char range (was 1-63) - Updated all JSDoc comments to reflect 1-16 char requirement - Updated interface documentation in ExposePortRequest - Updated unit tests to verify 16-char maximum - Updated changeset with corrected token length
There was a problem hiding this comment.
Claude Code Review
The implementation is well-structured with good validation logic and test coverage. However, there's one critical security issue that must be addressed before merging.
Critical Issue: Token Collision
Security Risk (10/10): The code doesn't prevent multiple ports from using the same custom token. This creates a security vulnerability where users could potentially access wrong ports or hijack preview URLs.
// Currently allowed - creates security issue:
await sandbox.exposePort(8080, { hostname: 'example.com', token: 'mytoken' });
await sandbox.exposePort(8081, { hostname: 'example.com', token: 'mytoken' });
// Both URLs use same token - which port gets accessed?Required fix: Before storing a custom token (sandbox.ts:2157), check if it's already in use by another port:
if (options.token) {
const existingTokens = tokens || {};
const existingPort = Object.entries(existingTokens).find(
([p, t]) => t === options.token && p !== port.toString()
);
if (existingPort) {
throw new SecurityError(
`Token '${options.token}' is already in use by port ${existingPort[0]}.`
);
}
token = options.token;
}Required test: Add to sandbox.test.ts:
it('should reject exposing different ports with same custom token', async () => {
await sandbox.exposePort(8080, { hostname: 'example.com', token: 'shared' });
await expect(
sandbox.exposePort(8081, { hostname: 'example.com', token: 'shared' })
).rejects.toThrow(/already in use/);
});Other Issues
Token persistence testing: Missing E2E test for token survival across DO hibernation. This is critical for production stability - if tokens don't persist, preview URLs break after sleep/wake cycles.
Regex edge cases: The URL pattern regex needs tests for consecutive separators (abc--def, test__123) to ensure they parse correctly.
What Looks Good
- Comprehensive input validation with clear error messages
- Proper SecurityError usage
- Type consistency across SDK and shared packages
- Good unit test coverage for validation rules
- Well-documented with JSDoc and examples
- Changeset follows project standards
Verdict: Fix the token collision issue, then this is ready to merge.
packages/sandbox/src/sandbox.ts
Outdated
| token = this.generatePortToken(); | ||
| } | ||
|
|
||
| // Store token for this port (storage is protected by input gates) |
There was a problem hiding this comment.
Security issue: Need to check if the custom token is already in use by another port before storing. Currently allows token collision which could let users access wrong ports.
Add before line 2157:
if (options.token) {
const existingPort = Object.entries(tokens).find(
([p, t]) => t === options.token && p !== port.toString()
);
if (existingPort) {
throw new SecurityError(
`Token '${options.token}' is already in use by port ${existingPort[0]}.`
);
}
}| function extractSandboxRoute(url: URL): RouteInfo | null { | ||
| // Parse subdomain pattern: port-sandboxId-token.domain (tokens mandatory) | ||
| // Token is always exactly 16 chars (generated by generatePortToken) | ||
| // Token can be any length (1-63 chars) containing only [a-z0-9_-] |
There was a problem hiding this comment.
Comment says "1-63 chars" but validateCustomToken limits to 16 chars. The regex accepts up to 63 to be forward-compatible with DNS limits, which is fine, but the comment could be clearer: "Token can be 1-16 chars (SDK validates) or up to 63 chars (DNS limit)"
| }) | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing critical test case: token collision prevention. Add:
it('should reject exposing different ports with same custom token', async () => {
await sandbox.exposePort(8080, { hostname: 'example.com', token: 'shared' });
await expect(
sandbox.exposePort(8081, { hostname: 'example.com', token: 'shared' })
).rejects.toThrow(/already in use/);
});…prevention Critical fixes: - Move token validation before container API call to prevent invalid tokens from reaching the container - Add token collision check to prevent multiple ports using same token - This fixes security vulnerability where wrong ports could be accessed Additional improvements: - Add test for token collision prevention - Add test for custom token storage verification - Add test for re-exposing same port with same token (valid scenario) - Update comment in request-handler.ts for clarity on token length All feedback from Claude Code Review has been addressed.
2 participants
Adds support for custom tokens in exposePort. - Allows for consistent URLs across sessions