Skip to content

Conversation

@ghostwriternr
Copy link
Member

Manual test script for bucket mounting with FUSE support.

Required because wrangler dev doesn't support --device and --cap-add flags for local testing. Uses environment variables for credentials and validates complete data round-trip through R2.

Not intended to be merged - reference for manual testing until local dev tooling supports FUSE device passthrough.

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.
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.
Provides test script and documentation for validating bucket mounting
with proper FUSE support. Required because wrangler dev doesn't support
passing Docker device flags for local testing.

Script uses environment variables for credentials and validates complete
data round-trip through R2 using independent verification via wrangler
CLI.
@changeset-bot
Copy link

changeset-bot bot commented Nov 5, 2025

⚠️ No Changeset found

Latest commit: 74e6fa9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

@claude
Copy link
Contributor

claude bot commented Nov 5, 2025

Claude Code Review

PR Summary: Manual test script for bucket mounting with FUSE support. Not intended for merge - reference for local testing workaround.

Issues

1. Shell script uses hardcoded version
test-bucket-mount-manual.sh:18

CONTAINER_IMAGE="cloudflare/sandbox-test:0.4.14"

This will become outdated. Consider using latest tag or document that users should update this value.

2. Incomplete cleanup on early failures
Multiple early exits (exit 1) don't clean up the container or temp files. Lines 13, 120 fail before cleanup runs.

Add trap for guaranteed cleanup:

cleanup() {
  docker stop "$CONTAINER_NAME" 2>/dev/null || true
  docker rm "$CONTAINER_NAME" 2>/dev/null || true
  rm -f "$R2_TEMP_FILE" "$WRANGLER_CONFIG"
}
trap cleanup EXIT

3. Wrangler config contains credentials in plaintext
WRANGLER_CONFIG is created with account ID but cleaned up. However, if script crashes before line 172, it persists. Not a critical issue since it only contains account ID (not secrets), but worth noting.

4. Verification retry logic could be clearer
Lines 94-106: The download retry loop silences all output (>/dev/null 2>&1), making debugging difficult. Consider showing error on final failure.

Minor

  • Line 159: Comment says "eventual consistency delay" but doesn't retry deletion verification like it does for creation (lines 94-106). Inconsistent.
  • Documentation is thorough and helpful

Verdict

Code works for its intended purpose. The cleanup issues should be fixed before merge (if this ever merges). Otherwise, script correctly validates the complete FUSE+R2 flow.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 5, 2025

Open in StackBlitz

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

commit: 74e6fa9

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

🐳 Docker Image Published

FROM cloudflare/sandbox:0.0.0-pr-191-66be952

Version: 0.0.0-pr-191-66be952

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

@ghostwriternr ghostwriternr marked this pull request as draft November 10, 2025 12:42
Base automatically changed from feat/bucket-mounting to main November 17, 2025 16:13
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