-
Notifications
You must be signed in to change notification settings - Fork 54
Fix preview URL parsing for sandbox IDs with hyphens #352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Tokens now use only [a-z0-9_] characters, allowing unambiguous URL parsing by splitting at the last hyphen. This fixes preview URL routing when sandbox IDs contain hyphens (like UUIDs).
|
| Name | Type |
|---|---|
| @cloudflare/sandbox | Patch |
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
- Add custom token documentation to ports API - Update token format restrictions (no hyphens, only a-z, 0-9, _) - Add examples showing custom token usage - Update preview URL format to include token - Add best practices for using custom tokens Ref: cloudflare/sandbox-sdk#352
commit: |
🐳 Docker Images PublishedDefault: FROM cloudflare/sandbox:0.0.0-pr-352-df17cbbWith Python: FROM cloudflare/sandbox:0.0.0-pr-352-df17cbb-pythonWith OpenCode: FROM cloudflare/sandbox:0.0.0-pr-352-df17cbb-opencodeVersion: Use the 📦 Standalone BinaryFor arbitrary Dockerfiles: COPY --from=cloudflare/sandbox:0.0.0-pr-352-df17cbb /container-server/sandbox /sandbox
ENTRYPOINT ["/sandbox"]Download via GitHub CLI: gh run download 20994437049 -n sandbox-binaryExtract from Docker: docker run --rm cloudflare/sandbox:0.0.0-pr-352-df17cbb cat /container-server/sandbox > sandbox && chmod +x sandbox |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OpenCode Review
This is a solid fix for the UUID parsing issue. The solution is elegant - restricting token characters to make URL parsing unambiguous.
A few observations:
Good:
- The parsing logic in
extractSandboxRoute()is now robust and handles UUID sandbox IDs correctly - Token validation is properly updated in both generation and validation functions
- Test coverage includes the new hyphen rejection case
- Changeset documentation is updated to reflect the character restriction
Minor suggestions:
-
Test coverage gap: The current tests don't specifically verify the UUID parsing scenario that triggered this bug. Consider adding a test case with a UUID sandbox ID to ensure it parses correctly.
-
Edge case validation: The new parsing logic assumes at least one hyphen exists after the port. While the current validation catches this, it might be worth adding a comment explaining why
lastIndexOf('-')is safe here.
The fix is architecturally sound and maintains backward compatibility for existing tokens that don't use hyphens. The restriction to [a-z0-9_] is reasonable for URL tokens and eliminates the parsing ambiguity.
Looks good to merge.
Summary
Fix preview URL parsing for sandbox IDs containing hyphens
PR #329 changed the token regex from
{16}to+to support variable-length custom tokens. This made parsing ambiguous when sandbox IDs contain hyphens (UUIDs), causing the wrong sandbox to receive requests.Breaking change
Tokens can no longer contain hyphens. Existing preview URLs with hyphenated tokens will fail until the port is re-exposed.
Why this wasn't caught
PR CI deploys to
workers.devwhere preview URL tests are skipped. Main CI runs locally where they execute.