-
Notifications
You must be signed in to change notification settings - Fork 54
Support undefined/null as unset in setEnvVars #342
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
Previously, passing undefined values to setEnvVars would either throw
runtime errors or silently do nothing. Now undefined/null triggers an
unset command, enabling patterns like { API_KEY: 'new', OLD_VAR: undefined }.
Also fixes a shell injection vulnerability by validating env var names
before interpolating into unset commands.
🦋 Changeset detectedLatest commit: 1a528ff 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 |
commit: |
🐳 Docker Images PublishedDefault: FROM cloudflare/sandbox:0.0.0-pr-342-0abb711With Python: FROM cloudflare/sandbox:0.0.0-pr-342-0abb711-pythonWith OpenCode: FROM cloudflare/sandbox:0.0.0-pr-342-0abb711-opencodeVersion: Use the 📦 Standalone BinaryFor arbitrary Dockerfiles: COPY --from=cloudflare/sandbox:0.0.0-pr-342-0abb711 /container-server/sandbox /sandbox
ENTRYPOINT ["/sandbox"]Download via GitHub CLI: gh run download 20971037432 -n sandbox-binaryExtract from Docker: docker run --rm cloudflare/sandbox:0.0.0-pr-342-0abb711 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
The implementation looks well-architected and follows established codebase patterns effectively. The change enables idiomatic JavaScript patterns where undefined means "unset", which aligns with how optional fields work throughout the SDK.
Key strengths:
- Leverages existing
shellEscape()andpartitionEnvVars()utilities consistently across both layers - Maintains atomic operations via session mutex locking
- POSIX environment variable name validation covers both set and unset operations
- Comprehensive test coverage including edge cases and E2E scenarios
- Type updates are consistent across the three-layer architecture
Documentation impact check: The changeset properly documents the behavior change for end users. No additional Cloudflare docs need updating since this doesn't change the core Workers/DO environment variable concepts.
Architecture consideration: The implementation correctly handles the flow through all three layers (SDK → DO → Container), with appropriate error handling and shell escaping at each boundary.
The approach is sound and ready to merge.
Summary
setEnvVars({ KEY: undefined })now runsunset KEYinstead of throwing or silently doing nothing, enabling idiomatic patterns: