-
Notifications
You must be signed in to change notification settings - Fork 36
Add S3-compatible bucket mounting #190
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
base: main
Are you sure you want to change the base?
Conversation
Enable sandboxes to mount S3-compatible buckets as local filesystem paths using s3fs-fuse. This allows code executing in sandboxes to read and write files directly to cloud storage using standard file operations. The implementation provides automatic credential detection from environment variables (AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY) and intelligent provider detection from endpoint URLs. Supported providers include AWS S3, Cloudflare R2, Google Cloud Storage, MinIO, Backblaze B2, Wasabi, and DigitalOcean Spaces. Each provider has optimized s3fs flags (e.g., R2 requires nomixupload and endpoint=auto) to ensure reliable operation. Users can override these defaults by providing custom s3fsOptions.
Remove examples and verbose logging to keep the codebase clean. Inline single-use injectCredentials method. Update CI workflow to pass R2 credentials from GitHub secrets instead of relying on local .env setup.
Apply stricter criteria for v1 by reducing provider list from 8 to 4. Remove backblaze, wasabi, and digitalocean support. Updated type definitions, detection logic, and test cases accordingly.
Enable bucket mounting/unmounting from session objects returned by createSession(). Sessions share the filesystem, so mount operations affect all sessions in the sandbox.
🦋 Changeset detectedLatest commit: 624c099 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 Image PublishedFROM cloudflare/sandbox:0.0.0-pr-190-f0ac765Version: You can use this Docker image with the preview package from this PR. |
Add shell escaping for user-provided input in mount paths, bucket names, git URLs, and branch names. Use shellEscape() utility in shared package for consistent POSIX single-quote escaping. Fix race condition in mountBucket() by reserving mount path before executing mount operations. Fix provider detection to use endsWith() instead of includes() to prevent malicious subdomain matching.
Switches from environment variables to password files for s3fs authentication, eliminating credential race conditions and improving isolation. Each mount now gets a unique password file that's cleaned up on unmount or destroy. Also fixes s3fs options injection vulnerability by escaping the entire options string before passing to shell.
R2 mounts passed both endpoint=auto and explicit url= causing conflicting s3fs configuration. Removed endpoint=auto since explicit URL is always provided. Failed unmounts deleted tracking entry while mount stayed active, orphaning the mount. Move delete into try block to only execute on successful unmount.
Port 9000 detection was unreliable and could match non-MinIO services. MinIO buckets still work via safe fallback defaults (use_path_request_style).
Merge s3fs/fuse installation with runtime packages to reduce image layer count.
Claude Code ReviewThis is a well-implemented feature with strong security considerations. The implementation follows SDK patterns and includes comprehensive error handling. A few issues need attention: Critical: Credential Storage in MemoryIssue: The
Fix: Remove Security: Mount Command LoggingIssue: At Consider: Either remove this debug log or sanitize the endpoint to hide account identifiers (e.g., replace account ID with Type Export MissingIssue: Fix: Remove Verification NeededThe E2E test at What Works WellSecurity measures:
Architecture:
Testing:
Code quality:
|
| mountPath: string; | ||
| endpoint: string; | ||
| provider: BucketProvider | null; | ||
| credentials: BucketCredentials; |
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.
Security concern: Storing credentials in the MountInfo object keeps sensitive data in memory for the lifetime of the Durable Object. Since the password file already provides credential access to s3fs and is properly cleaned up, there's no need to retain credentials here.
Consider removing the credentials field entirely to minimize credential exposure.
| const optionsStr = shellEscape(s3fsArgs.join(',')); | ||
| const mountCmd = `s3fs ${shellEscape(bucket)} ${shellEscape(mountPath)} -o ${optionsStr}`; | ||
|
|
||
| this.logger.debug(`Executing mount command: ${mountCmd}`, { |
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.
Logging the full mount command exposes the endpoint URL which may contain sensitive account IDs (e.g., Cloudflare account ID in R2 URLs). Consider sanitizing the endpoint or removing this debug log.
| mountPath, | ||
| endpoint: options.endpoint, | ||
| provider, | ||
| credentials, |
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.
Credentials stored here remain in memory. Consider removing credentials from MountInfo since the password file is sufficient for s3fs operation.
|
Hey @ghostwriternr are there any timelines for when this is pushed? Or maybe if there is a way for us to run this branch on our account? This is the last we need to switch to CF :) |
Add ability to mount S3-compatible buckets as local directories using s3fs-fuse.
Key features:
Supported providers:
Example:
Includes E2E tests and comprehensive error handling.
Fixes #130