From 1bc9fea16e76f82e23e496a64c756385a36473a6 Mon Sep 17 00:00:00 2001 From: Naresh Date: Wed, 5 Nov 2025 14:44:33 +0000 Subject: [PATCH 01/13] Add bucket mounting for S3-compatible storage 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. --- .changeset/bucket-mounting.md | 7 + package-lock.json | 25 +- packages/sandbox/Dockerfile | 12 + packages/sandbox/src/index.ts | 21 +- packages/sandbox/src/sandbox.ts | 275 ++++++++++++++++++ .../src/storage-mount/credential-detection.ts | 42 +++ packages/sandbox/src/storage-mount/errors.ts | 52 ++++ packages/sandbox/src/storage-mount/index.ts | 17 ++ .../src/storage-mount/provider-detection.ts | 140 +++++++++ packages/sandbox/src/storage-mount/types.ts | 17 ++ .../credential-detection.test.ts | 115 ++++++++ .../storage-mount/provider-detection.test.ts | 83 ++++++ packages/shared/src/errors/codes.ts | 6 + packages/shared/src/errors/contexts.ts | 23 ++ packages/shared/src/errors/index.ts | 3 + packages/shared/src/errors/status-map.ts | 4 + packages/shared/src/index.ts | 4 + packages/shared/src/types.ts | 89 ++++++ tests/e2e/bucket-mounting.test.ts | 214 ++++++++++++++ tests/e2e/test-worker/index.ts | 28 ++ vitest.e2e.config.ts | 4 + 21 files changed, 1159 insertions(+), 22 deletions(-) create mode 100644 .changeset/bucket-mounting.md create mode 100644 packages/sandbox/src/storage-mount/credential-detection.ts create mode 100644 packages/sandbox/src/storage-mount/errors.ts create mode 100644 packages/sandbox/src/storage-mount/index.ts create mode 100644 packages/sandbox/src/storage-mount/provider-detection.ts create mode 100644 packages/sandbox/src/storage-mount/types.ts create mode 100644 packages/sandbox/tests/storage-mount/credential-detection.test.ts create mode 100644 packages/sandbox/tests/storage-mount/provider-detection.test.ts create mode 100644 tests/e2e/bucket-mounting.test.ts diff --git a/.changeset/bucket-mounting.md b/.changeset/bucket-mounting.md new file mode 100644 index 00000000..659a66d4 --- /dev/null +++ b/.changeset/bucket-mounting.md @@ -0,0 +1,7 @@ +--- +'@cloudflare/sandbox': minor +--- + +Add S3-compatible bucket mounting + +Enable mounting S3-compatible buckets (R2, S3, GCS, MinIO, etc.) as local filesystem paths using s3fs-fuse. Supports automatic credential detection from environment variables and intelligent provider detection from endpoint URLs. diff --git a/package-lock.json b/package-lock.json index f22eca25..6ae7c662 100644 --- a/package-lock.json +++ b/package-lock.json @@ -180,7 +180,6 @@ "integrity": "sha512-e7jT4DxYvIDLk1ZHmU/m/mB19rex9sv0c2ftBtjSBv+kVM/902eh0fINUzD7UwLLNR+jU585GxUJ8/EBfAM5fw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@babel/code-frame": "^7.27.1", "@babel/generator": "^7.28.5", @@ -1129,8 +1128,7 @@ "resolved": "https://registry.npmjs.org/@cloudflare/workers-types/-/workers-types-4.20251014.0.tgz", "integrity": "sha512-tEW98J/kOa0TdylIUOrLKRdwkUw0rvvYVlo+Ce0mqRH3c8kSoxLzUH9gfCvwLe0M89z1RkzFovSKAW2Nwtyn3w==", "dev": true, - "license": "MIT OR Apache-2.0", - "peer": true + "license": "MIT OR Apache-2.0" }, "node_modules/@cspotcode/source-map-support": { "version": "0.8.1", @@ -2282,7 +2280,6 @@ "integrity": "sha512-/g2d4sW9nUDJOMz3mabVQvOGhVa4e/BN/Um7yca9Bb2XTzPPnfTWHWQg+IsEYO7M3Vx+EXvaM/I2pJWIMun1bg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@octokit/auth-token": "^4.0.0", "@octokit/graphql": "^7.1.0", @@ -3315,7 +3312,6 @@ "resolved": "https://registry.npmjs.org/@types/react/-/react-19.2.2.tgz", "integrity": "sha512-6mDvHUFSjyT2B2yeNx2nUgMxh9LtOWvkhIU3uePn2I2oyNymUAX1NIsdgviM4CH+JSrp2D2hsMvJOkxY+0wNRA==", "license": "MIT", - "peer": true, "dependencies": { "csstype": "^3.0.2" } @@ -3453,7 +3449,6 @@ "integrity": "sha512-oukfKT9Mk41LreEW09vt45f8wx7DordoWUZMYdY/cyAk7w5TWkTRCNZYF7sX7n2wB7jyGAl74OxgwhPgKaqDMQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/utils": "3.2.4", "pathe": "^2.0.3", @@ -3469,7 +3464,6 @@ "integrity": "sha512-dEYtS7qQP2CjU27QBC5oUOxLE/v5eLkGqPE0ZKEIDGMs4vKWe7IjgLOeauHsR0D5YuuycGRO5oSRXnwnmA78fQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/pretty-format": "3.2.4", "magic-string": "^0.30.17", @@ -3498,7 +3492,6 @@ "integrity": "sha512-hGISOaP18plkzbWEcP/QvtRW1xDXF2+96HbEX6byqQhAUbiS5oH6/9JwW+QsQCIYON2bI6QZBF+2PvOmrRZ9wA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/utils": "3.2.4", "fflate": "^0.8.2", @@ -3720,7 +3713,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "baseline-browser-mapping": "^2.8.19", "caniuse-lite": "^1.0.30001751", @@ -4973,6 +4965,7 @@ "resolved": "https://registry.npmjs.org/loose-envify/-/loose-envify-1.4.0.tgz", "integrity": "sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q==", "license": "MIT", + "peer": true, "dependencies": { "js-tokens": "^3.0.0 || ^4.0.0" }, @@ -6050,6 +6043,7 @@ "resolved": "https://registry.npmjs.org/object-assign/-/object-assign-4.1.1.tgz", "integrity": "sha512-rJgTQnkUnH1sFw8yT6VSU3zD3sWmu6sZhIseY8VX+GRu3P6F7Fu+JNDoXfklElbLJSnc3FUQHVe4cU5hj+BcUg==", "license": "MIT", + "peer": true, "engines": { "node": ">=0.10.0" } @@ -6471,7 +6465,6 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.0.tgz", "integrity": "sha512-tmbWg6W31tQLeB5cdIBOicJDJRR2KzXsV7uSK9iNfLWQ5bIZfxuPEHp7M8wiHyHnn0DD1i7w3Zmin0FtkrwoCQ==", "license": "MIT", - "peer": true, "engines": { "node": ">=0.10.0" } @@ -6493,7 +6486,8 @@ "version": "16.13.1", "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.13.1.tgz", "integrity": "sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ==", - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/react-katex": { "version": "3.1.0", @@ -6678,7 +6672,6 @@ "integrity": "sha512-iMmuD72XXLf26Tqrv1cryNYLX6NNPLhZ3AmNkSf8+xda0H+yijjGJ+wVT9UdBUHOpKzq9RjKtQKRCWoEKQQBZQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@oxc-project/types": "=0.95.0", "@rolldown/pluginutils": "1.0.0-beta.45" @@ -7339,7 +7332,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -7508,7 +7500,6 @@ "integrity": "sha512-ytQKuwgmrrkDTFP4LjR0ToE2nqgy886GpvRSpU0JAnrdBYppuY5rLkRUYPU1yCryb24SsKBTL/hlDQAEFVwtZg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "~0.25.0", "get-tsconfig": "^4.7.5" @@ -7705,7 +7696,6 @@ "integrity": "sha512-Wj7/AMtE9MRnAXa6Su3Lk0LNCfqDYgfwVjwRFVum9U7wsto1imuHqk4kTm7Jni+5A0Hn7dttL6O/zjvUvoo+8A==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "defu": "^6.1.4", "exsolve": "^1.0.7", @@ -7924,7 +7914,6 @@ "integrity": "sha512-ZWyE8YXEXqJrrSLvYgrRP7p62OziLW7xI5HYGWFzOvupfAlrLvURSzv/FyGyy0eidogEM3ujU+kUG1zuHgb6Ug==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -8041,7 +8030,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -8055,7 +8043,6 @@ "integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@types/chai": "^5.2.2", "@vitest/expect": "3.2.4", @@ -8211,7 +8198,6 @@ "dev": true, "hasInstallScript": true, "license": "Apache-2.0", - "peer": true, "bin": { "workerd": "bin/workerd" }, @@ -8740,7 +8726,6 @@ "integrity": "sha512-PEIGCY5tSlUt50cqyMXfCzX+oOPqN0vuGqWzbcJ2xvnkzkq46oOpz7dQaTDBdfICb4N14+GARUDw2XV2N4tvzg==", "devOptional": true, "license": "MIT", - "peer": true, "engines": { "node": ">=10.0.0" }, diff --git a/packages/sandbox/Dockerfile b/packages/sandbox/Dockerfile index 1ece6e81..b475de1a 100644 --- a/packages/sandbox/Dockerfile +++ b/packages/sandbox/Dockerfile @@ -113,6 +113,18 @@ ENV DEBIAN_FRONTEND=noninteractive # Set the sandbox version as an environment variable for version checking ENV SANDBOX_VERSION=${SANDBOX_VERSION} +# Install S3FS-FUSE for bucket mounting +RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ + --mount=type=cache,target=/var/lib/apt,sharing=locked \ + rm -f /etc/apt/apt.conf.d/docker-clean && \ + echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' >/etc/apt/apt.conf.d/keep-cache && \ + apt-get update && apt-get install -y --no-install-recommends \ + s3fs \ + fuse + +# Enable FUSE in container - allow non-root users to use FUSE +RUN sed -i 's/#user_allow_other/user_allow_other/' /etc/fuse.conf + # Install runtime packages and Python runtime libraries RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ --mount=type=cache,target=/var/lib/apt,sharing=locked \ diff --git a/packages/sandbox/src/index.ts b/packages/sandbox/src/index.ts index 5886d06d..bb55f4c6 100644 --- a/packages/sandbox/src/index.ts +++ b/packages/sandbox/src/index.ts @@ -17,20 +17,31 @@ export { getSandbox, Sandbox } from './sandbox'; // Export core SDK types for consumers export type { BaseExecOptions, + BucketCredentials, + BucketProvider, + CodeContext, + CreateContextOptions, ExecEvent, ExecOptions, ExecResult, + ExecutionResult, + ExecutionSession, FileChunk, FileMetadata, FileStreamEvent, + GitCheckoutResult, ISandbox, + ListFilesOptions, LogEvent, + MountBucketOptions, Process, ProcessOptions, ProcessStatus, + RunCodeOptions, + SandboxOptions, + SessionOptions, StreamOptions } from '@repo/shared'; -export * from '@repo/shared'; // Export type guards for runtime validation export { isExecResult, isProcess, isProcessStatus } from '@repo/shared'; // Export all client types from new architecture @@ -50,7 +61,6 @@ export type { // Git client types GitCheckoutRequest, - GitCheckoutResult, // Base client types HttpClientOptions as SandboxClientOptions, @@ -98,3 +108,10 @@ export { parseSSEStream, responseToAsyncIterable } from './sse-parser'; +// Export bucket mounting errors +export { + BucketMountError, + InvalidMountConfigError, + MissingCredentialsError, + S3FSMountError +} from './storage-mount/errors'; diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index 85e55543..05c6a6f6 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -1,6 +1,8 @@ import type { DurableObject } from 'cloudflare:workers'; import { Container, getContainer, switchPort } from '@cloudflare/containers'; import type { + BucketCredentials, + BucketProvider, CodeContext, CreateContextOptions, ExecEvent, @@ -9,6 +11,7 @@ import type { ExecutionResult, ExecutionSession, ISandbox, + MountBucketOptions, Process, ProcessOptions, ProcessStatus, @@ -25,6 +28,16 @@ import { CodeInterpreter } from './interpreter'; import { isLocalhostPattern } from './request-handler'; import { SecurityError, sanitizeSandboxId, validatePort } from './security'; import { parseSSEStream } from './sse-parser'; +import { + detectCredentials, + detectProviderFromUrl, + resolveS3fsOptions +} from './storage-mount'; +import { + InvalidMountConfigError, + S3FSMountError +} from './storage-mount/errors'; +import type { MountInfo } from './storage-mount/types'; import { SDK_VERSION } from './version'; export function getSandbox( @@ -82,6 +95,7 @@ export class Sandbox extends Container implements ISandbox { envVars: Record = {}; private logger: ReturnType; private keepAliveEnabled: boolean = false; + private activeMounts: Map = new Map(); constructor(ctx: DurableObjectState<{}>, env: Env) { super(ctx, env); @@ -195,11 +209,272 @@ export class Sandbox extends Container implements ISandbox { } } + /** + * Mount an S3-compatible bucket as a local directory using S3FS-FUSE + * + * Requires explicit endpoint URL. Credentials are auto-detected from environment + * variables or can be provided explicitly. + * + * @param bucket - Bucket name (e.g., 'my-data') + * @param mountPath - Absolute path in container to mount at (e.g., '/mnt/data') + * @param options - Configuration options with required endpoint + * @throws MissingCredentialsError if no credentials found in environment + * @throws S3FSMountError if S3FS mount command fails + * @throws InvalidMountConfigError if bucket name, mount path, or endpoint is invalid + * + * @example + * ```typescript + * // Cloudflare R2 + * await sandbox.mountBucket('my-bucket', '/mnt/data', { + * endpoint: 'https://abc123.r2.cloudflarestorage.com' + * }); + * + * // AWS S3 + * await sandbox.mountBucket('my-bucket', '/mnt/data', { + * endpoint: 'https://s3.us-west-2.amazonaws.com' + * }); + * + * // MinIO + * await sandbox.mountBucket('my-bucket', '/mnt/data', { + * endpoint: 'http://minio.local:9000' + * }); + * ``` + */ + async mountBucket( + bucket: string, + mountPath: string, + options: MountBucketOptions + ): Promise { + this.logger.info(`Mounting bucket ${bucket} to ${mountPath}`, { + endpoint: options.endpoint + }); + + // Validate options + this.validateMountOptions(bucket, mountPath, options); + + // Detect provider from explicit option or URL pattern + const provider: BucketProvider | null = + options.provider || detectProviderFromUrl(options.endpoint); + + this.logger.debug(`Detected provider: ${provider || 'unknown'}`, { + endpoint: options.endpoint, + explicitProvider: options.provider + }); + + // Detect credentials + const credentials = detectCredentials(options, this.envVars); + + // Inject credentials into environment + await this.injectCredentials(credentials); + + // Create mount directory + await this.exec(`mkdir -p ${mountPath}`); + + // Execute S3FS mount with provider-specific flags + await this.executeS3FSMount(bucket, mountPath, options, provider); + + // Track active mount (use mountPath as key to support multiple mounts of same bucket) + this.activeMounts.set(mountPath, { + bucket, + mountPath, + endpoint: options.endpoint, + provider, + credentials, + mounted: true + }); + + this.logger.info(`Successfully mounted bucket ${bucket} to ${mountPath}`); + } + + /** + * Manually unmount a bucket filesystem + * + * @param mountPath - Absolute path where the bucket is mounted + * @throws InvalidMountConfigError if mount path doesn't exist or isn't mounted + * + * @example + * ```typescript + * // Mount a bucket + * await sandbox.mountBucket('my-bucket', '/mnt/data', { + * endpoint: 'https://abc123.r2.cloudflarestorage.com' + * }); + * + * // Later, unmount it manually + * await sandbox.unmountBucket('/mnt/data'); + * ``` + */ + async unmountBucket(mountPath: string): Promise { + this.logger.info(`Unmounting bucket from ${mountPath}`); + + // Look up mount by path + const mountInfo = this.activeMounts.get(mountPath); + + // Throw error if mount doesn't exist + if (!mountInfo) { + throw new InvalidMountConfigError( + `No active mount found at path: ${mountPath}` + ); + } + + // Unmount the filesystem + try { + await this.exec(`fusermount -u ${mountPath}`); + mountInfo.mounted = false; + + // Remove from active mounts + this.activeMounts.delete(mountPath); + + this.logger.info(`Successfully unmounted bucket from ${mountPath}`); + } catch (error) { + const errorMsg = error instanceof Error ? error.message : String(error); + throw new S3FSMountError( + `Failed to unmount bucket from ${mountPath}: ${errorMsg}` + ); + } + } + + /** + * Validate mount options + */ + private validateMountOptions( + bucket: string, + mountPath: string, + options: MountBucketOptions + ): void { + // Require endpoint field + if (!options.endpoint) { + throw new InvalidMountConfigError( + 'Endpoint is required. Provide the full S3-compatible endpoint URL.\n' + + 'Examples:\n' + + ' - R2: https://YOUR-ACCOUNT-ID.r2.cloudflarestorage.com\n' + + ' - AWS S3: https://s3.REGION.amazonaws.com\n' + + ' - GCS: https://storage.googleapis.com\n' + + ' - MinIO: http://minio.local:9000' + ); + } + + // Basic URL validation + try { + new URL(options.endpoint); + } catch (error) { + throw new InvalidMountConfigError( + `Invalid endpoint URL: "${options.endpoint}". Must be a valid HTTP(S) URL.` + ); + } + + // Validate bucket name (S3-compatible naming rules) + const bucketNameRegex = /^[a-z0-9]([a-z0-9.-]{0,61}[a-z0-9])?$/; + if (!bucketNameRegex.test(bucket)) { + throw new InvalidMountConfigError( + `Invalid bucket name: "${bucket}". Bucket names must be 3-63 characters, ` + + `lowercase alphanumeric, dots, or hyphens, and cannot start/end with dots or hyphens.` + ); + } + + // Validate mount path is absolute + if (!mountPath.startsWith('/')) { + throw new InvalidMountConfigError( + `Mount path must be absolute (start with /): "${mountPath}"` + ); + } + + // Check for duplicate mount path + if (this.activeMounts.has(mountPath)) { + const existingMount = this.activeMounts.get(mountPath); + throw new InvalidMountConfigError( + `Mount path "${mountPath}" is already in use by bucket "${existingMount?.bucket}". ` + + `Unmount the existing bucket first or use a different mount path.` + ); + } + } + + /** + * Inject S3 credentials into environment + */ + private async injectCredentials( + credentials: BucketCredentials + ): Promise { + const credEnvVars: Record = { + AWS_ACCESS_KEY_ID: credentials.accessKeyId, + AWS_SECRET_ACCESS_KEY: credentials.secretAccessKey + }; + + if (credentials.sessionToken) { + credEnvVars.AWS_SESSION_TOKEN = credentials.sessionToken; + } + + await this.setEnvVars(credEnvVars); + } + + /** + * Execute S3FS mount command + */ + private async executeS3FSMount( + bucket: string, + mountPath: string, + options: MountBucketOptions, + provider: BucketProvider | null + ): Promise { + // Resolve s3fs options (provider defaults + user overrides) + const resolvedOptions = resolveS3fsOptions(provider, options.s3fsOptions); + + // Build s3fs mount command + const s3fsArgs: string[] = []; + + // Add resolved provider-specific and user options + s3fsArgs.push(...resolvedOptions); + + // Add read-only flag if requested + if (options.readOnly) { + s3fsArgs.push('ro'); + } + + // Add endpoint URL + s3fsArgs.push(`url=${options.endpoint}`); + + // Build final command + const optionsStr = s3fsArgs.join(','); + const mountCmd = `s3fs ${bucket} ${mountPath} -o ${optionsStr}`; + + this.logger.debug(`Executing mount command: ${mountCmd}`, { + provider, + resolvedOptions + }); + + // Execute mount command + const result = await this.exec(mountCmd); + + if (result.exitCode !== 0) { + throw new S3FSMountError( + `S3FS mount failed: ${result.stderr || result.stdout || 'Unknown error'}` + ); + } + + this.logger.debug('Mount command executed successfully'); + } + /** * Cleanup and destroy the sandbox container */ override async destroy(): Promise { this.logger.info('Destroying sandbox container'); + + // Unmount all mounted buckets + for (const [mountPath, mountInfo] of this.activeMounts.entries()) { + if (mountInfo.mounted) { + try { + this.logger.info(`Unmounting bucket ${mountInfo.bucket} from ${mountPath}`); + await this.exec(`fusermount -u ${mountPath}`); + mountInfo.mounted = false; + } catch (error) { + const errorMsg = error instanceof Error ? error.message : String(error); + this.logger.warn( + `Failed to unmount bucket ${mountInfo.bucket} from ${mountPath}: ${errorMsg}` + ); + } + } + } + await super.destroy(); } diff --git a/packages/sandbox/src/storage-mount/credential-detection.ts b/packages/sandbox/src/storage-mount/credential-detection.ts new file mode 100644 index 00000000..def3fcc9 --- /dev/null +++ b/packages/sandbox/src/storage-mount/credential-detection.ts @@ -0,0 +1,42 @@ +import type { BucketCredentials, MountBucketOptions } from '@repo/shared'; +import { MissingCredentialsError } from './errors'; + +/** + * Detect credentials for bucket mounting from environment variables + * Priority order: + * 1. Explicit options.credentials + * 2. Standard AWS env vars: AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY + * 3. Error: no credentials found + * + * @param options - Mount options + * @param envVars - Environment variables + * @returns Detected credentials + * @throws MissingCredentialsError if no credentials found + */ +export function detectCredentials( + options: MountBucketOptions, + envVars: Record +): BucketCredentials { + // Priority 1: Explicit credentials in options + if (options.credentials) { + return options.credentials; + } + + // Priority 2: Standard AWS env vars + const awsAccessKeyId = envVars.AWS_ACCESS_KEY_ID; + const awsSecretAccessKey = envVars.AWS_SECRET_ACCESS_KEY; + + if (awsAccessKeyId && awsSecretAccessKey) { + return { + accessKeyId: awsAccessKeyId, + secretAccessKey: awsSecretAccessKey, + sessionToken: envVars.AWS_SESSION_TOKEN + }; + } + + // No credentials found - throw error with helpful message + throw new MissingCredentialsError( + `No credentials found. Set AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY ` + + `environment variables, or pass explicit credentials in options.` + ); +} diff --git a/packages/sandbox/src/storage-mount/errors.ts b/packages/sandbox/src/storage-mount/errors.ts new file mode 100644 index 00000000..27d7a182 --- /dev/null +++ b/packages/sandbox/src/storage-mount/errors.ts @@ -0,0 +1,52 @@ +/** + * Bucket mounting error classes + * + * These are SDK-side validation errors that follow the same pattern as SecurityError. + * They are thrown before any container interaction occurs. + */ + +import { ErrorCode } from '@repo/shared/errors'; + +/** + * Base error for bucket mounting operations + */ +export class BucketMountError extends Error { + public readonly code: ErrorCode; + + constructor(message: string, code: ErrorCode = ErrorCode.BUCKET_MOUNT_ERROR) { + super(message); + this.name = 'BucketMountError'; + this.code = code; + } +} + +/** + * Thrown when S3FS mount command fails + */ +export class S3FSMountError extends BucketMountError { + constructor(message: string) { + super(message, ErrorCode.S3FS_MOUNT_ERROR); + this.name = 'S3FSMountError'; + } +} + +/** + * Thrown when no credentials found in environment + */ +export class MissingCredentialsError extends BucketMountError { + constructor(message: string) { + super(message, ErrorCode.MISSING_CREDENTIALS); + this.name = 'MissingCredentialsError'; + } +} + +/** + * Thrown when bucket name, mount path, or options are invalid + */ +export class InvalidMountConfigError extends BucketMountError { + constructor(message: string) { + super(message, ErrorCode.INVALID_MOUNT_CONFIG); + this.name = 'InvalidMountConfigError'; + } +} + diff --git a/packages/sandbox/src/storage-mount/index.ts b/packages/sandbox/src/storage-mount/index.ts new file mode 100644 index 00000000..aed05d12 --- /dev/null +++ b/packages/sandbox/src/storage-mount/index.ts @@ -0,0 +1,17 @@ +/** + * Bucket mounting functionality + */ + +export { detectCredentials } from './credential-detection'; +export { + BucketMountError, + InvalidMountConfigError, + MissingCredentialsError, + S3FSMountError +} from './errors'; +export { + detectProviderFromUrl, + getProviderFlags, + resolveS3fsOptions +} from './provider-detection'; +export type { MountInfo } from './types'; diff --git a/packages/sandbox/src/storage-mount/provider-detection.ts b/packages/sandbox/src/storage-mount/provider-detection.ts new file mode 100644 index 00000000..79df404f --- /dev/null +++ b/packages/sandbox/src/storage-mount/provider-detection.ts @@ -0,0 +1,140 @@ +/** + * Provider detection and s3fs flag configuration + * + * Based on s3fs-fuse documentation: + * https://github.com/s3fs-fuse/s3fs-fuse/wiki/Non-Amazon-S3 + */ + +import type { BucketProvider } from '@repo/shared'; + +/** + * Detect provider from endpoint URL using pattern matching + */ +export function detectProviderFromUrl(endpoint: string): BucketProvider | null { + try { + const url = new URL(endpoint); + const hostname = url.hostname.toLowerCase(); + + // Cloudflare R2 + if (hostname.includes('.r2.cloudflarestorage.com')) { + return 'r2'; + } + + // Backblaze B2 (check before S3 as it contains 's3.' in URL) + if (hostname.includes('.backblazeb2.com')) { + return 'backblaze'; + } + + // Wasabi (check before S3 as it contains 's3.' in URL) + if (hostname.includes('.wasabisys.com')) { + return 'wasabi'; + } + + // Amazon S3 + if (hostname.includes('.amazonaws.com') || hostname.startsWith('s3.')) { + return 's3'; + } + + // Google Cloud Storage + if (hostname === 'storage.googleapis.com') { + return 'gcs'; + } + + // MinIO (common patterns: port 9000 or 'minio' in hostname) + if (hostname.includes('minio') || url.port === '9000') { + return 'minio'; + } + + // DigitalOcean Spaces + if (hostname.includes('.digitaloceanspaces.com')) { + return 'digitalocean'; + } + + // Unknown provider + return null; + } catch { + // Invalid URL + return null; + } +} + +/** + * Get s3fs flags for a given provider + * + * Based on s3fs-fuse wiki recommendations: + * https://github.com/s3fs-fuse/s3fs-fuse/wiki/Non-Amazon-S3 + */ +export function getProviderFlags(provider: BucketProvider | null): string[] { + if (!provider) { + // Safe default for unknown providers + return ['use_path_request_style']; + } + + switch (provider) { + case 'r2': + // Cloudflare R2 requires nomixupload and endpoint=auto + return ['nomixupload', 'endpoint=auto']; + + case 's3': + // AWS S3 works with defaults (virtual-hosted style) + return []; + + case 'gcs': + // Google Cloud Storage works with defaults (s3fs 1.90+) + return []; + + case 'minio': + // MinIO requires path-style requests + return ['use_path_request_style']; + + case 'backblaze': + // Backblaze B2 works with defaults + return []; + + case 'wasabi': + // Wasabi works with defaults + return []; + + case 'digitalocean': + // DigitalOcean Spaces works with defaults + return []; + + case 'custom': + // Custom provider - user must specify all flags + return []; + + default: + // Fallback to safe defaults + return ['use_path_request_style']; + } +} + +/** + * Resolve s3fs options by combining provider defaults with user overrides + */ +export function resolveS3fsOptions( + provider: BucketProvider | null, + userOptions?: string[] +): string[] { + const providerFlags = getProviderFlags(provider); + + if (!userOptions || userOptions.length === 0) { + return providerFlags; + } + + // Merge provider flags with user options + // User options take precedence (come last in the array) + const allFlags = [...providerFlags, ...userOptions]; + + // Deduplicate flags (keep last occurrence) + const flagMap = new Map(); + + for (const flag of allFlags) { + // Split on '=' to get the flag name + const [flagName] = flag.split('='); + flagMap.set(flagName, flag); + } + + return Array.from(flagMap.values()); +} + diff --git a/packages/sandbox/src/storage-mount/types.ts b/packages/sandbox/src/storage-mount/types.ts new file mode 100644 index 00000000..0902f47e --- /dev/null +++ b/packages/sandbox/src/storage-mount/types.ts @@ -0,0 +1,17 @@ +/** + * Internal bucket mounting types + */ + +import type { BucketCredentials, BucketProvider } from '@repo/shared'; + +/** + * Internal tracking information for active mounts + */ +export interface MountInfo { + bucket: string; + mountPath: string; + endpoint: string; + provider: BucketProvider | null; + credentials: BucketCredentials; + mounted: boolean; +} diff --git a/packages/sandbox/tests/storage-mount/credential-detection.test.ts b/packages/sandbox/tests/storage-mount/credential-detection.test.ts new file mode 100644 index 00000000..92dd36da --- /dev/null +++ b/packages/sandbox/tests/storage-mount/credential-detection.test.ts @@ -0,0 +1,115 @@ +import { describe, expect, it } from 'vitest'; +import { detectCredentials } from '../../src/storage-mount/credential-detection'; + +describe('Credential Detection', () => { + it('should use explicit credentials from options', () => { + const envVars = {}; + const options = { + endpoint: 'https://test.r2.cloudflarestorage.com', + credentials: { + accessKeyId: 'explicit-key', + secretAccessKey: 'explicit-secret' + } + }; + + const credentials = detectCredentials(options, envVars); + + expect(credentials.accessKeyId).toBe('explicit-key'); + expect(credentials.secretAccessKey).toBe('explicit-secret'); + }); + + it('should detect standard AWS env vars', () => { + const envVars = { + AWS_ACCESS_KEY_ID: 'aws-key', + AWS_SECRET_ACCESS_KEY: 'aws-secret' + }; + const options = { endpoint: 'https://s3.us-west-2.amazonaws.com' }; + + const credentials = detectCredentials(options, envVars); + + expect(credentials.accessKeyId).toBe('aws-key'); + expect(credentials.secretAccessKey).toBe('aws-secret'); + }); + + it('should include session token if present', () => { + const envVars = { + AWS_ACCESS_KEY_ID: 'aws-key', + AWS_SECRET_ACCESS_KEY: 'aws-secret', + AWS_SESSION_TOKEN: 'session-token' + }; + const options = { endpoint: 'https://s3.us-west-2.amazonaws.com' }; + + const credentials = detectCredentials(options, envVars); + + expect(credentials.sessionToken).toBe('session-token'); + }); + + it('should prioritize explicit credentials over env vars', () => { + const envVars = { + AWS_ACCESS_KEY_ID: 'env-key', + AWS_SECRET_ACCESS_KEY: 'env-secret' + }; + const options = { + endpoint: 'https://test.r2.cloudflarestorage.com', + credentials: { + accessKeyId: 'explicit-key', + secretAccessKey: 'explicit-secret' + } + }; + + const credentials = detectCredentials(options, envVars); + + expect(credentials.accessKeyId).toBe('explicit-key'); + expect(credentials.secretAccessKey).toBe('explicit-secret'); + }); + + it('should throw error when no credentials found', () => { + const envVars = {}; + const options = { endpoint: 'https://test.r2.cloudflarestorage.com' }; + + expect(() => detectCredentials(options, envVars)) + .toThrow('No credentials found'); + }); + + it('should include helpful error message with env var hints', () => { + const envVars = {}; + const options = { endpoint: 'https://test.r2.cloudflarestorage.com' }; + + let thrownError: Error | null = null; + try { + detectCredentials(options, envVars); + } catch (error) { + thrownError = error as Error; + } + + expect(thrownError).toBeTruthy(); + if (thrownError) { + const message = thrownError.message; + expect(message).toContain('AWS_ACCESS_KEY_ID'); + expect(message).toContain('AWS_SECRET_ACCESS_KEY'); + expect(message).toContain('explicit credentials'); + } + }); + + it('should throw error when only access key is present', () => { + const envVars = { + AWS_ACCESS_KEY_ID: 'aws-key' + // Missing AWS_SECRET_ACCESS_KEY + }; + const options = { endpoint: 'https://test.r2.cloudflarestorage.com' }; + + expect(() => detectCredentials(options, envVars)) + .toThrow('No credentials found'); + }); + + it('should throw error when only secret key is present', () => { + const envVars = { + AWS_SECRET_ACCESS_KEY: 'aws-secret' + // Missing AWS_ACCESS_KEY_ID + }; + const options = { endpoint: 'https://test.r2.cloudflarestorage.com' }; + + expect(() => detectCredentials(options, envVars)) + .toThrow('No credentials found'); + }); +}); diff --git a/packages/sandbox/tests/storage-mount/provider-detection.test.ts b/packages/sandbox/tests/storage-mount/provider-detection.test.ts new file mode 100644 index 00000000..82f056ff --- /dev/null +++ b/packages/sandbox/tests/storage-mount/provider-detection.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from 'vitest'; +import { + detectProviderFromUrl, + getProviderFlags, + resolveS3fsOptions +} from '../../src/storage-mount/provider-detection'; + +describe('Provider Detection', () => { + describe('detectProviderFromUrl', () => { + it.each([ + ['https://abc123.r2.cloudflarestorage.com', 'r2'], + ['https://s3.us-west-2.amazonaws.com', 's3'], + ['https://storage.googleapis.com', 'gcs'], + ['http://minio.local:9000', 'minio'], + ['https://s3.us-west-001.backblazeb2.com', 'backblaze'], + ['https://s3.wasabisys.com', 'wasabi'], + ['https://nyc3.digitaloceanspaces.com', 'digitalocean'] + ])('should detect %s as %s', (url, expectedProvider) => { + expect(detectProviderFromUrl(url)).toBe(expectedProvider); + }); + + it.each([ + ['https://custom.storage.example.com'], + ['not-a-url'], + [''] + ])('should return null for unknown/invalid: %s', (url) => { + expect(detectProviderFromUrl(url)).toBe(null); + }); + }); + + describe('getProviderFlags', () => { + it.each([ + ['r2', ['nomixupload', 'endpoint=auto']], + ['s3', []], + ['gcs', []], + ['minio', ['use_path_request_style']], + ['backblaze', []], + ['wasabi', []], + ['digitalocean', []], + ['custom', []] + ])('should return correct flags for %s', (provider, expected) => { + expect(getProviderFlags(provider as any)).toEqual(expected); + }); + + it('should return safe defaults for unknown providers', () => { + expect(getProviderFlags(null)).toEqual(['use_path_request_style']); + }); + }); + + describe('resolveS3fsOptions', () => { + it('should use provider defaults when no user options', () => { + const options = resolveS3fsOptions('r2'); + expect(options).toEqual(['nomixupload', 'endpoint=auto']); + }); + + it('should merge provider flags with user options', () => { + const options = resolveS3fsOptions('r2', ['custom_flag']); + expect(options).toContain('nomixupload'); + expect(options).toContain('endpoint=auto'); + expect(options).toContain('custom_flag'); + }); + + it('should allow user options to override provider defaults', () => { + const options = resolveS3fsOptions('r2', ['endpoint=us-east']); + expect(options).toContain('nomixupload'); + expect(options).toContain('endpoint=us-east'); + expect(options).not.toContain('endpoint=auto'); + }); + + it('should deduplicate flags keeping last occurrence', () => { + const options = resolveS3fsOptions('minio', ['use_path_request_style', 'custom_flag']); + const count = options.filter(o => o === 'use_path_request_style').length; + expect(count).toBe(1); + expect(options).toContain('custom_flag'); + }); + + it('should use safe defaults for unknown providers', () => { + const options = resolveS3fsOptions(null, ['nomixupload']); + expect(options).toContain('use_path_request_style'); + expect(options).toContain('nomixupload'); + }); + }); +}); diff --git a/packages/shared/src/errors/codes.ts b/packages/shared/src/errors/codes.ts index 345b1d9c..eee2d594 100644 --- a/packages/shared/src/errors/codes.ts +++ b/packages/shared/src/errors/codes.ts @@ -79,6 +79,12 @@ export const ErrorCode = { GIT_CHECKOUT_FAILED: 'GIT_CHECKOUT_FAILED', GIT_OPERATION_FAILED: 'GIT_OPERATION_FAILED', + // Bucket mounting errors + BUCKET_MOUNT_ERROR: 'BUCKET_MOUNT_ERROR', + S3FS_MOUNT_ERROR: 'S3FS_MOUNT_ERROR', + MISSING_CREDENTIALS: 'MISSING_CREDENTIALS', + INVALID_MOUNT_CONFIG: 'INVALID_MOUNT_CONFIG', + // Code Interpreter Errors (503) INTERPRETER_NOT_READY: 'INTERPRETER_NOT_READY', diff --git a/packages/shared/src/errors/contexts.ts b/packages/shared/src/errors/contexts.ts index 935811ca..2b0ed705 100644 --- a/packages/shared/src/errors/contexts.ts +++ b/packages/shared/src/errors/contexts.ts @@ -125,6 +125,29 @@ export interface ValidationFailedContext { }>; } +/** + * Bucket mounting error contexts + */ +export interface BucketMountContext { + bucket: string; + mountPath: string; + endpoint: string; + stderr?: string; + exitCode?: number; +} + +export interface MissingCredentialsContext { + bucket: string; + endpoint: string; +} + +export interface InvalidMountConfigContext { + bucket?: string; + mountPath?: string; + endpoint?: string; + reason?: string; +} + /** * Generic error contexts */ diff --git a/packages/shared/src/errors/index.ts b/packages/shared/src/errors/index.ts index 8fd8a5eb..84d9c666 100644 --- a/packages/shared/src/errors/index.ts +++ b/packages/shared/src/errors/index.ts @@ -33,6 +33,7 @@ export { ErrorCode, type ErrorCode as ErrorCodeType } from './codes'; // Export context interfaces export type { + BucketMountContext, CodeExecutionContext, CommandErrorContext, CommandNotFoundContext, @@ -46,7 +47,9 @@ export type { GitRepositoryNotFoundContext, InternalErrorContext, InterpreterNotReadyContext, + InvalidMountConfigContext, InvalidPortContext, + MissingCredentialsContext, PortAlreadyExposedContext, PortErrorContext, PortNotExposedContext, diff --git a/packages/shared/src/errors/status-map.ts b/packages/shared/src/errors/status-map.ts index 33b3b7e2..370e31a9 100644 --- a/packages/shared/src/errors/status-map.ts +++ b/packages/shared/src/errors/status-map.ts @@ -25,6 +25,8 @@ export const ERROR_STATUS_MAP: Record = { [ErrorCode.INVALID_JSON_RESPONSE]: 400, [ErrorCode.NAME_TOO_LONG]: 400, [ErrorCode.VALIDATION_FAILED]: 400, + [ErrorCode.MISSING_CREDENTIALS]: 400, + [ErrorCode.INVALID_MOUNT_CONFIG]: 400, // 401 Unauthorized [ErrorCode.GIT_AUTH_FAILED]: 401, @@ -61,6 +63,8 @@ export const ERROR_STATUS_MAP: Record = { [ErrorCode.GIT_CHECKOUT_FAILED]: 500, [ErrorCode.GIT_OPERATION_FAILED]: 500, [ErrorCode.CODE_EXECUTION_ERROR]: 500, + [ErrorCode.BUCKET_MOUNT_ERROR]: 500, + [ErrorCode.S3FS_MOUNT_ERROR]: 500, [ErrorCode.UNKNOWN_ERROR]: 500, [ErrorCode.INTERNAL_ERROR]: 500 }; diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 718e743d..d7631056 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -44,6 +44,9 @@ export type { // Export all types from types.ts export type { BaseExecOptions, + // Bucket mounting types + BucketCredentials, + BucketProvider, ContextCreateResult, ContextDeleteResult, ContextListResult, @@ -69,6 +72,7 @@ export type { ListFilesResult, LogEvent, MkdirResult, + MountBucketOptions, MoveFileResult, PortCloseResult, // Port management result types diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index 8c9eb102..5a219c8b 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -667,6 +667,87 @@ export interface ExecutionSession { deleteCodeContext(contextId: string): Promise; } +// Bucket mounting types +/** + * Supported S3-compatible storage providers + */ +export type BucketProvider = + | 'r2' // Cloudflare R2 + | 's3' // Amazon S3 + | 'gcs' // Google Cloud Storage + | 'minio' // MinIO + | 'backblaze' // Backblaze B2 + | 'wasabi' // Wasabi + | 'digitalocean' // DigitalOcean Spaces + | 'custom'; // Custom S3-compatible provider + +/** + * Credentials for S3-compatible storage + */ +export interface BucketCredentials { + accessKeyId: string; + secretAccessKey: string; + sessionToken?: string; +} + +/** + * Options for mounting an S3-compatible bucket + */ +export interface MountBucketOptions { + /** + * S3-compatible endpoint URL + * + * Examples: + * - R2: 'https://abc123.r2.cloudflarestorage.com' + * - AWS S3: 'https://s3.us-west-2.amazonaws.com' + * - GCS: 'https://storage.googleapis.com' + * - MinIO: 'http://minio.local:9000' + * + * Required field + */ + endpoint: string; + + /** + * Optional provider hint for automatic s3fs flag configuration + * + * If not specified, will attempt to detect from endpoint URL. + * Use 'custom' if you want to manually specify all s3fs options. + * + * Examples: + * - 'r2' - Cloudflare R2 (adds nomixupload, endpoint=auto) + * - 's3' - Amazon S3 (standard configuration) + * - 'gcs' - Google Cloud Storage (no special flags needed) + * - 'minio' - MinIO (adds use_path_request_style) + */ + provider?: BucketProvider; + + /** + * Explicit credentials (overrides env var auto-detection) + */ + credentials?: BucketCredentials; + + /** + * Mount filesystem as read-only + * Default: false + */ + readOnly?: boolean; + + /** + * Advanced: Override or extend s3fs options + * + * These will be merged with provider-specific defaults. + * To override defaults completely, specify all options here. + * + * Common options: + * - 'use_path_request_style' - Use path-style URLs (bucket/path vs bucket.host/path) + * - 'nomixupload' - Disable mixed multipart uploads (needed for some providers) + * - 'nomultipart' - Disable all multipart operations + * - 'sigv2' - Use signature version 2 instead of v4 + * - 'no_check_certificate' - Skip SSL certificate validation (dev/testing only) + */ + s3fsOptions?: string[]; +} + // Main Sandbox interface export interface ISandbox { // Command execution @@ -722,6 +803,14 @@ export interface ISandbox { options?: { branch?: string; targetDir?: string } ): Promise; + // Bucket mounting operations + mountBucket( + bucket: string, + mountPath: string, + options: MountBucketOptions + ): Promise; + unmountBucket(mountPath: string): Promise; + // Session management createSession(options?: SessionOptions): Promise; diff --git a/tests/e2e/bucket-mounting.test.ts b/tests/e2e/bucket-mounting.test.ts new file mode 100644 index 00000000..7e03ae7b --- /dev/null +++ b/tests/e2e/bucket-mounting.test.ts @@ -0,0 +1,214 @@ +import { + afterAll, + afterEach, + beforeAll, + describe, + expect, + test, + vi +} from 'vitest'; +import { + cleanupSandbox, + createSandboxId, + createTestHeaders, + fetchWithStartup +} from './helpers/test-fixtures'; +import { + getTestWorkerUrl, + type WranglerDevRunner +} from './helpers/wrangler-runner'; + +/** + * E2E test for S3-compatible bucket mounting + * + * This test validates the complete bucket mounting workflow: + * 1. Mount an R2 bucket using explicit endpoint URL + * 2. Write files via the mounted filesystem + * 3. Read files back to verify + * 4. List directory contents + * + * Requires: + * - R2 bucket: sandbox-e2e-test + * - Credentials: AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY + * - Account: CLOUDFLARE_ACCOUNT_ID (used to construct endpoint) + * + * Note: This test requires FUSE device access and only runs in CI. + * Local wrangler dev doesn't expose /dev/fuse to containers. + */ +describe('Bucket Mounting E2E', () => { + const requiredEnvVars = [ + 'CLOUDFLARE_ACCOUNT_ID', + 'AWS_ACCESS_KEY_ID', + 'AWS_SECRET_ACCESS_KEY' + ]; + + // Check if we have credentials to run this test + const hasCredentials = requiredEnvVars.every((key) => !!process.env[key]); + + if (!hasCredentials) { + test.skip('Skipping E2E test - missing R2 credentials', () => { + console.log('\n⚠️ Bucket mounting E2E test requires R2 credentials:'); + requiredEnvVars.forEach((key) => { + console.log(` ${key}: ${process.env[key] ? '✓' : '✗ missing'}`); + }); + console.log('\nSet these environment variables to run E2E tests.\n'); + }); + return; + } + + // Skip test when running locally (requires FUSE device access only available in CI) + const isCI = !!process.env.TEST_WORKER_URL; + if (!isCI) { + test.skip('Skipping E2E test - requires FUSE device access (CI only)', () => { + console.log( + '\n⚠️ Bucket mounting E2E test requires FUSE device access (only available in CI)\n' + ); + }); + return; + } + + describe('local', () => { + let runner: WranglerDevRunner | null; + let workerUrl: string; + let currentSandboxId: string | null = null; + + const TEST_BUCKET = 'sandbox-e2e-test'; + const MOUNT_PATH = '/mnt/test-data'; + const TEST_FILE = `e2e-test-${Date.now()}.txt`; + const TEST_CONTENT = `Bucket mounting E2E test - ${new Date().toISOString()}`; + + beforeAll(async () => { + // Get test worker URL (CI: deployed URL, Local: spawns wrangler dev) + const result = await getTestWorkerUrl(); + workerUrl = result.url; + runner = result.runner; + + console.log(`\n🔧 E2E Test Configuration:`); + console.log(` Worker URL: ${workerUrl}`); + console.log(` Bucket: ${TEST_BUCKET}`); + console.log(` Mount Path: ${MOUNT_PATH}`); + console.log(` Test File: ${TEST_FILE}\n`); + }, 30000); + + afterEach(async () => { + // Cleanup sandbox after each test + if (currentSandboxId) { + await cleanupSandbox(workerUrl, currentSandboxId); + currentSandboxId = null; + } + }); + + afterAll(async () => { + // Stop wrangler dev (only in local mode) + if (runner) { + await runner.stop(); + } + }); + + test('should mount R2 bucket and perform file operations', async () => { + currentSandboxId = createSandboxId(); + const headers = createTestHeaders(currentSandboxId); + + console.log(`\n🪣 Step 1: Mounting bucket ${TEST_BUCKET}...`); + + // Mount the R2 bucket + const mountResponse = await vi.waitFor( + async () => + fetchWithStartup(`${workerUrl}/api/bucket/mount`, { + method: 'POST', + headers, + body: JSON.stringify({ + bucket: TEST_BUCKET, + mountPath: MOUNT_PATH, + options: { + endpoint: `https://${process.env.CLOUDFLARE_ACCOUNT_ID}.r2.cloudflarestorage.com` + } + }) + }), + { timeout: 60000, interval: 2000 } + ); + + expect(mountResponse.ok).toBe(true); + const mountResult = await mountResponse.json(); + expect(mountResult.success).toBe(true); + console.log(`✅ Bucket mounted successfully`); + + // Verify mount point exists + console.log(`\n📂 Step 2: Verifying mount point...`); + const verifyResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ + command: `test -d ${MOUNT_PATH} && echo "mounted"` + }) + }); + + const verifyResult = await verifyResponse.json(); + expect(verifyResult.stdout?.trim()).toBe('mounted'); + expect(verifyResult.exitCode).toBe(0); + console.log(`✅ Mount point verified at ${MOUNT_PATH}`); + + // Write test file to mounted bucket + console.log(`\n✏️ Step 3: Writing test file...`); + const writeResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ + command: `echo "${TEST_CONTENT}" > ${MOUNT_PATH}/${TEST_FILE}` + }) + }); + + const writeResult = await writeResponse.json(); + expect(writeResult.exitCode).toBe(0); + console.log(`✅ Test file written: ${TEST_FILE}`); + + // Read file back + console.log(`\n📖 Step 4: Reading file back...`); + const readResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ + command: `cat ${MOUNT_PATH}/${TEST_FILE}` + }) + }); + + const readResult = await readResponse.json(); + expect(readResult.exitCode).toBe(0); + expect(readResult.stdout?.trim()).toBe(TEST_CONTENT); + console.log(`✅ File content verified`); + console.log(` Content: "${readResult.stdout?.trim()}"`); + + // List directory contents + console.log(`\n📋 Step 5: Listing directory...`); + const lsResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ + command: `ls -lh ${MOUNT_PATH}/${TEST_FILE}` + }) + }); + + const lsResult = await lsResponse.json(); + expect(lsResult.exitCode).toBe(0); + expect(lsResult.stdout).toContain(TEST_FILE); + console.log(`✅ Directory listing successful`); + console.log(` ${lsResult.stdout?.trim()}`); + + // Cleanup: delete test file + console.log(`\n🧹 Step 6: Cleaning up test file...`); + const cleanupResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ + command: `rm -f ${MOUNT_PATH}/${TEST_FILE}` + }) + }); + + const cleanupResult = await cleanupResponse.json(); + expect(cleanupResult.exitCode).toBe(0); + console.log(`✅ Test file removed`); + + console.log(`\n✅ All E2E bucket mounting tests passed!\n`); + }, 120000); // 2 minute timeout + }); +}); diff --git a/tests/e2e/test-worker/index.ts b/tests/e2e/test-worker/index.ts index eb070c1e..e8c123d4 100644 --- a/tests/e2e/test-worker/index.ts +++ b/tests/e2e/test-worker/index.ts @@ -9,6 +9,10 @@ export { Sandbox }; interface Env { Sandbox: DurableObjectNamespace; + // R2 credentials for bucket mounting tests + CLOUDFLARE_ACCOUNT_ID?: string; + AWS_ACCESS_KEY_ID?: string; + AWS_SECRET_ACCESS_KEY?: string; } async function parseBody(request: Request): Promise { @@ -296,6 +300,30 @@ console.log('Terminal server on port ' + port); }); } + // Bucket mount + if (url.pathname === '/api/bucket/mount' && request.method === 'POST') { + // Pass R2 credentials from worker env to sandbox env + const sandboxEnvVars: Record = {}; + if (env.CLOUDFLARE_ACCOUNT_ID) { + sandboxEnvVars.CLOUDFLARE_ACCOUNT_ID = env.CLOUDFLARE_ACCOUNT_ID; + } + if (env.AWS_ACCESS_KEY_ID) { + sandboxEnvVars.AWS_ACCESS_KEY_ID = env.AWS_ACCESS_KEY_ID; + } + if (env.AWS_SECRET_ACCESS_KEY) { + sandboxEnvVars.AWS_SECRET_ACCESS_KEY = env.AWS_SECRET_ACCESS_KEY; + } + + if (Object.keys(sandboxEnvVars).length > 0) { + await sandbox.setEnvVars(sandboxEnvVars); + } + + await sandbox.mountBucket(body.bucket, body.mountPath, body.options); + return new Response(JSON.stringify({ success: true }), { + headers: { 'Content-Type': 'application/json' } + }); + } + // File read if (url.pathname === '/api/file/read' && request.method === 'POST') { const file = await executor.readFile(body.path); diff --git a/vitest.e2e.config.ts b/vitest.e2e.config.ts index f89ba9fd..82867bdb 100644 --- a/vitest.e2e.config.ts +++ b/vitest.e2e.config.ts @@ -1,5 +1,9 @@ +import { config } from 'dotenv'; import { defineConfig } from 'vitest/config'; +// Load environment variables from .env file +config(); + /** * E2E test configuration * From 79ee46984e660d3e4132911cdbffb9454c0c4d70 Mon Sep 17 00:00:00 2001 From: Naresh Date: Wed, 5 Nov 2025 15:22:12 +0000 Subject: [PATCH 02/13] Clean up bucket mounting code 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. --- .github/workflows/pullrequest.yml | 7 +- .github/workflows/release.yml | 4 +- CONTRIBUTING.md | 12 +++ packages/sandbox/src/sandbox.ts | 86 +++++-------------- packages/sandbox/src/storage-mount/errors.ts | 3 +- .../src/storage-mount/provider-detection.ts | 26 +----- .../credential-detection.test.ts | 15 ++-- .../storage-mount/provider-detection.test.ts | 22 +++-- tests/e2e/bucket-mounting.test.ts | 78 ++++------------- tests/integration/src/index.ts | 6 +- 10 files changed, 85 insertions(+), 174 deletions(-) diff --git a/.github/workflows/pullrequest.yml b/.github/workflows/pullrequest.yml index 3138cb55..d234473a 100644 --- a/.github/workflows/pullrequest.yml +++ b/.github/workflows/pullrequest.yml @@ -120,8 +120,8 @@ jobs: with: context: . file: packages/sandbox/Dockerfile - platforms: linux/amd64 # Explicit single-arch for compatibility with release-amd64 cache - load: true # Load into Docker daemon for local testing + platforms: linux/amd64 # Explicit single-arch for compatibility with release-amd64 cache + load: true # Load into Docker daemon for local testing tags: cloudflare/sandbox-test:${{ needs.unit-tests.outputs.version || '0.0.0' }} cache-from: | type=gha,scope=pr-${{ github.event.pull_request.number }}-amd64 @@ -152,6 +152,9 @@ jobs: env: TEST_WORKER_URL: ${{ steps.get-url.outputs.worker_url }} CI: true + CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} + AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} # Cleanup: Delete test worker and container (only for PR environments) - name: Cleanup test deployment diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 581118a6..2201f5aa 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -119,7 +119,7 @@ jobs: context: . file: packages/sandbox/Dockerfile platforms: linux/amd64 - push: false # Don't push, just cache + push: false # Don't push, just cache cache-from: type=gha,scope=release-amd64 cache-to: type=gha,mode=max,scope=release-amd64 build-args: | @@ -190,7 +190,7 @@ jobs: context: . file: packages/sandbox/Dockerfile platforms: linux/amd64 - push: false # Don't push, just cache + push: false # Don't push, just cache cache-from: type=gha,scope=release-amd64 cache-to: type=gha,mode=max,scope=release-amd64 build-args: | diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7baa6fe4..89837c2f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,17 +15,20 @@ Thank you for your interest in contributing to the Cloudflare Sandbox SDK! This 1. Fork the repository to your GitHub account 2. Clone your fork: + ```bash git clone https://github.com/YOUR-USERNAME/sandbox-sdk.git cd sandbox-sdk ``` 3. Install dependencies: + ```bash npm install ``` 4. Build the packages: + ```bash npm run build ``` @@ -40,6 +43,7 @@ Thank you for your interest in contributing to the Cloudflare Sandbox SDK! This ### Making Changes 1. Create a new branch for your changes: + ```bash git checkout -b feat/your-feature-name # or @@ -49,6 +53,7 @@ Thank you for your interest in contributing to the Cloudflare Sandbox SDK! This 2. Make your changes following our coding standards (see CLAUDE.md) 3. Run code quality checks: + ```bash npm run check # Linting + type checking npm run fix # Auto-fix linting issues @@ -73,6 +78,7 @@ Follow the [7 rules for great commit messages](https://cbea.ms/git-commit/): 7. Use the body to explain what and why vs. how Example: + ``` Add session isolation for concurrent executions @@ -90,11 +96,13 @@ npx changeset ``` This will interactively guide you through: + 1. Selecting which packages to include 2. Choosing the semantic version bump (`patch`, `minor`, or `major`) 3. Writing a description of your changes Use semantic versioning: + - `patch`: Bug fixes, minor improvements - `minor`: New features, non-breaking changes - `major`: Breaking changes @@ -104,6 +112,7 @@ The changeset bot will comment on your PR if a changeset is needed. ## Submitting a Pull Request 1. Push your branch to your fork: + ```bash git push origin feat/your-feature-name ``` @@ -119,6 +128,7 @@ The changeset bot will comment on your PR if a changeset is needed. ### Review Process A maintainer will review your PR and may: + - Request changes - Ask questions - Suggest improvements @@ -141,6 +151,7 @@ We use Biome for linting and formatting. Key guidelines: ### Unit Tests Located in `packages/*/tests/`: + - Test individual components in isolation - Mock external dependencies - Fast feedback loop @@ -150,6 +161,7 @@ Run with: `npm test` ### E2E Tests Located in `tests/e2e/`: + - Test full workflows against real Workers and containers - Require Docker - Slower but comprehensive diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index 05c6a6f6..f31016f7 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -67,9 +67,9 @@ export function getSandbox( }); } -export function connect( - stub: { fetch: (request: Request) => Promise } -) { +export function connect(stub: { + fetch: (request: Request) => Promise; +}) { return async (request: Request, port: number) => { // Validate port before routing if (!validatePort(port)) { @@ -215,30 +215,12 @@ export class Sandbox extends Container implements ISandbox { * Requires explicit endpoint URL. Credentials are auto-detected from environment * variables or can be provided explicitly. * - * @param bucket - Bucket name (e.g., 'my-data') - * @param mountPath - Absolute path in container to mount at (e.g., '/mnt/data') + * @param bucket - Bucket name + * @param mountPath - Absolute path in container to mount at * @param options - Configuration options with required endpoint * @throws MissingCredentialsError if no credentials found in environment * @throws S3FSMountError if S3FS mount command fails * @throws InvalidMountConfigError if bucket name, mount path, or endpoint is invalid - * - * @example - * ```typescript - * // Cloudflare R2 - * await sandbox.mountBucket('my-bucket', '/mnt/data', { - * endpoint: 'https://abc123.r2.cloudflarestorage.com' - * }); - * - * // AWS S3 - * await sandbox.mountBucket('my-bucket', '/mnt/data', { - * endpoint: 'https://s3.us-west-2.amazonaws.com' - * }); - * - * // MinIO - * await sandbox.mountBucket('my-bucket', '/mnt/data', { - * endpoint: 'http://minio.local:9000' - * }); - * ``` */ async mountBucket( bucket: string, @@ -264,8 +246,17 @@ export class Sandbox extends Container implements ISandbox { // Detect credentials const credentials = detectCredentials(options, this.envVars); - // Inject credentials into environment - await this.injectCredentials(credentials); + // Inject credentials into container environment + const credEnvVars: Record = { + AWS_ACCESS_KEY_ID: credentials.accessKeyId, + AWS_SECRET_ACCESS_KEY: credentials.secretAccessKey + }; + + if (credentials.sessionToken) { + credEnvVars.AWS_SESSION_TOKEN = credentials.sessionToken; + } + + await this.setEnvVars(credEnvVars); // Create mount directory await this.exec(`mkdir -p ${mountPath}`); @@ -288,20 +279,9 @@ export class Sandbox extends Container implements ISandbox { /** * Manually unmount a bucket filesystem - * + * * @param mountPath - Absolute path where the bucket is mounted * @throws InvalidMountConfigError if mount path doesn't exist or isn't mounted - * - * @example - * ```typescript - * // Mount a bucket - * await sandbox.mountBucket('my-bucket', '/mnt/data', { - * endpoint: 'https://abc123.r2.cloudflarestorage.com' - * }); - * - * // Later, unmount it manually - * await sandbox.unmountBucket('/mnt/data'); - * ``` */ async unmountBucket(mountPath: string): Promise { this.logger.info(`Unmounting bucket from ${mountPath}`); @@ -344,12 +324,7 @@ export class Sandbox extends Container implements ISandbox { // Require endpoint field if (!options.endpoint) { throw new InvalidMountConfigError( - 'Endpoint is required. Provide the full S3-compatible endpoint URL.\n' + - 'Examples:\n' + - ' - R2: https://YOUR-ACCOUNT-ID.r2.cloudflarestorage.com\n' + - ' - AWS S3: https://s3.REGION.amazonaws.com\n' + - ' - GCS: https://storage.googleapis.com\n' + - ' - MinIO: http://minio.local:9000' + 'Endpoint is required. Provide the full S3-compatible endpoint URL.' ); } @@ -388,24 +363,6 @@ export class Sandbox extends Container implements ISandbox { } } - /** - * Inject S3 credentials into environment - */ - private async injectCredentials( - credentials: BucketCredentials - ): Promise { - const credEnvVars: Record = { - AWS_ACCESS_KEY_ID: credentials.accessKeyId, - AWS_SECRET_ACCESS_KEY: credentials.secretAccessKey - }; - - if (credentials.sessionToken) { - credEnvVars.AWS_SESSION_TOKEN = credentials.sessionToken; - } - - await this.setEnvVars(credEnvVars); - } - /** * Execute S3FS mount command */ @@ -463,11 +420,14 @@ export class Sandbox extends Container implements ISandbox { for (const [mountPath, mountInfo] of this.activeMounts.entries()) { if (mountInfo.mounted) { try { - this.logger.info(`Unmounting bucket ${mountInfo.bucket} from ${mountPath}`); + this.logger.info( + `Unmounting bucket ${mountInfo.bucket} from ${mountPath}` + ); await this.exec(`fusermount -u ${mountPath}`); mountInfo.mounted = false; } catch (error) { - const errorMsg = error instanceof Error ? error.message : String(error); + const errorMsg = + error instanceof Error ? error.message : String(error); this.logger.warn( `Failed to unmount bucket ${mountInfo.bucket} from ${mountPath}: ${errorMsg}` ); diff --git a/packages/sandbox/src/storage-mount/errors.ts b/packages/sandbox/src/storage-mount/errors.ts index 27d7a182..bce38fb2 100644 --- a/packages/sandbox/src/storage-mount/errors.ts +++ b/packages/sandbox/src/storage-mount/errors.ts @@ -1,6 +1,6 @@ /** * Bucket mounting error classes - * + * * These are SDK-side validation errors that follow the same pattern as SecurityError. * They are thrown before any container interaction occurs. */ @@ -49,4 +49,3 @@ export class InvalidMountConfigError extends BucketMountError { this.name = 'InvalidMountConfigError'; } } - diff --git a/packages/sandbox/src/storage-mount/provider-detection.ts b/packages/sandbox/src/storage-mount/provider-detection.ts index 79df404f..5c3e9db6 100644 --- a/packages/sandbox/src/storage-mount/provider-detection.ts +++ b/packages/sandbox/src/storage-mount/provider-detection.ts @@ -15,96 +15,77 @@ export function detectProviderFromUrl(endpoint: string): BucketProvider | null { const url = new URL(endpoint); const hostname = url.hostname.toLowerCase(); - // Cloudflare R2 if (hostname.includes('.r2.cloudflarestorage.com')) { return 'r2'; } - // Backblaze B2 (check before S3 as it contains 's3.' in URL) if (hostname.includes('.backblazeb2.com')) { return 'backblaze'; } - // Wasabi (check before S3 as it contains 's3.' in URL) if (hostname.includes('.wasabisys.com')) { return 'wasabi'; } - // Amazon S3 if (hostname.includes('.amazonaws.com') || hostname.startsWith('s3.')) { return 's3'; } - // Google Cloud Storage if (hostname === 'storage.googleapis.com') { return 'gcs'; } - // MinIO (common patterns: port 9000 or 'minio' in hostname) if (hostname.includes('minio') || url.port === '9000') { return 'minio'; } - // DigitalOcean Spaces if (hostname.includes('.digitaloceanspaces.com')) { return 'digitalocean'; } - // Unknown provider return null; } catch { - // Invalid URL return null; } } /** * Get s3fs flags for a given provider - * + * * Based on s3fs-fuse wiki recommendations: * https://github.com/s3fs-fuse/s3fs-fuse/wiki/Non-Amazon-S3 */ export function getProviderFlags(provider: BucketProvider | null): string[] { if (!provider) { - // Safe default for unknown providers return ['use_path_request_style']; } switch (provider) { case 'r2': - // Cloudflare R2 requires nomixupload and endpoint=auto return ['nomixupload', 'endpoint=auto']; case 's3': - // AWS S3 works with defaults (virtual-hosted style) return []; case 'gcs': - // Google Cloud Storage works with defaults (s3fs 1.90+) return []; case 'minio': - // MinIO requires path-style requests return ['use_path_request_style']; case 'backblaze': - // Backblaze B2 works with defaults return []; case 'wasabi': - // Wasabi works with defaults return []; case 'digitalocean': - // DigitalOcean Spaces works with defaults return []; case 'custom': - // Custom provider - user must specify all flags return []; default: - // Fallback to safe defaults return ['use_path_request_style']; } } @@ -117,7 +98,7 @@ export function resolveS3fsOptions( userOptions?: string[] ): string[] { const providerFlags = getProviderFlags(provider); - + if (!userOptions || userOptions.length === 0) { return providerFlags; } @@ -128,7 +109,7 @@ export function resolveS3fsOptions( // Deduplicate flags (keep last occurrence) const flagMap = new Map(); - + for (const flag of allFlags) { // Split on '=' to get the flag name const [flagName] = flag.split('='); @@ -137,4 +118,3 @@ export function resolveS3fsOptions( return Array.from(flagMap.values()); } - diff --git a/packages/sandbox/tests/storage-mount/credential-detection.test.ts b/packages/sandbox/tests/storage-mount/credential-detection.test.ts index 92dd36da..df14d379 100644 --- a/packages/sandbox/tests/storage-mount/credential-detection.test.ts +++ b/packages/sandbox/tests/storage-mount/credential-detection.test.ts @@ -67,8 +67,9 @@ describe('Credential Detection', () => { const envVars = {}; const options = { endpoint: 'https://test.r2.cloudflarestorage.com' }; - expect(() => detectCredentials(options, envVars)) - .toThrow('No credentials found'); + expect(() => detectCredentials(options, envVars)).toThrow( + 'No credentials found' + ); }); it('should include helpful error message with env var hints', () => { @@ -98,8 +99,9 @@ describe('Credential Detection', () => { }; const options = { endpoint: 'https://test.r2.cloudflarestorage.com' }; - expect(() => detectCredentials(options, envVars)) - .toThrow('No credentials found'); + expect(() => detectCredentials(options, envVars)).toThrow( + 'No credentials found' + ); }); it('should throw error when only secret key is present', () => { @@ -109,7 +111,8 @@ describe('Credential Detection', () => { }; const options = { endpoint: 'https://test.r2.cloudflarestorage.com' }; - expect(() => detectCredentials(options, envVars)) - .toThrow('No credentials found'); + expect(() => detectCredentials(options, envVars)).toThrow( + 'No credentials found' + ); }); }); diff --git a/packages/sandbox/tests/storage-mount/provider-detection.test.ts b/packages/sandbox/tests/storage-mount/provider-detection.test.ts index 82f056ff..c3653469 100644 --- a/packages/sandbox/tests/storage-mount/provider-detection.test.ts +++ b/packages/sandbox/tests/storage-mount/provider-detection.test.ts @@ -19,13 +19,12 @@ describe('Provider Detection', () => { expect(detectProviderFromUrl(url)).toBe(expectedProvider); }); - it.each([ - ['https://custom.storage.example.com'], - ['not-a-url'], - [''] - ])('should return null for unknown/invalid: %s', (url) => { - expect(detectProviderFromUrl(url)).toBe(null); - }); + it.each([['https://custom.storage.example.com'], ['not-a-url'], ['']])( + 'should return null for unknown/invalid: %s', + (url) => { + expect(detectProviderFromUrl(url)).toBe(null); + } + ); }); describe('getProviderFlags', () => { @@ -68,8 +67,13 @@ describe('Provider Detection', () => { }); it('should deduplicate flags keeping last occurrence', () => { - const options = resolveS3fsOptions('minio', ['use_path_request_style', 'custom_flag']); - const count = options.filter(o => o === 'use_path_request_style').length; + const options = resolveS3fsOptions('minio', [ + 'use_path_request_style', + 'custom_flag' + ]); + const count = options.filter( + (o) => o === 'use_path_request_style' + ).length; expect(count).toBe(1); expect(options).toContain('custom_flag'); }); diff --git a/tests/e2e/bucket-mounting.test.ts b/tests/e2e/bucket-mounting.test.ts index 7e03ae7b..474b9593 100644 --- a/tests/e2e/bucket-mounting.test.ts +++ b/tests/e2e/bucket-mounting.test.ts @@ -21,48 +21,20 @@ import { /** * E2E test for S3-compatible bucket mounting * - * This test validates the complete bucket mounting workflow: - * 1. Mount an R2 bucket using explicit endpoint URL - * 2. Write files via the mounted filesystem - * 3. Read files back to verify - * 4. List directory contents - * - * Requires: - * - R2 bucket: sandbox-e2e-test - * - Credentials: AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY - * - Account: CLOUDFLARE_ACCOUNT_ID (used to construct endpoint) + * Requires environment variables: + * CLOUDFLARE_ACCOUNT_ID - Cloudflare account ID + * AWS_ACCESS_KEY_ID - R2 access key ID + * AWS_SECRET_ACCESS_KEY - R2 secret access key * * Note: This test requires FUSE device access and only runs in CI. * Local wrangler dev doesn't expose /dev/fuse to containers. */ describe('Bucket Mounting E2E', () => { - const requiredEnvVars = [ - 'CLOUDFLARE_ACCOUNT_ID', - 'AWS_ACCESS_KEY_ID', - 'AWS_SECRET_ACCESS_KEY' - ]; - - // Check if we have credentials to run this test - const hasCredentials = requiredEnvVars.every((key) => !!process.env[key]); - - if (!hasCredentials) { - test.skip('Skipping E2E test - missing R2 credentials', () => { - console.log('\n⚠️ Bucket mounting E2E test requires R2 credentials:'); - requiredEnvVars.forEach((key) => { - console.log(` ${key}: ${process.env[key] ? '✓' : '✗ missing'}`); - }); - console.log('\nSet these environment variables to run E2E tests.\n'); - }); - return; - } - // Skip test when running locally (requires FUSE device access only available in CI) const isCI = !!process.env.TEST_WORKER_URL; if (!isCI) { - test.skip('Skipping E2E test - requires FUSE device access (CI only)', () => { - console.log( - '\n⚠️ Bucket mounting E2E test requires FUSE device access (only available in CI)\n' - ); + test.skip('Skipping - requires FUSE device access (CI only)', () => { + // Test skipped in local development }); return; } @@ -78,20 +50,12 @@ describe('Bucket Mounting E2E', () => { const TEST_CONTENT = `Bucket mounting E2E test - ${new Date().toISOString()}`; beforeAll(async () => { - // Get test worker URL (CI: deployed URL, Local: spawns wrangler dev) const result = await getTestWorkerUrl(); workerUrl = result.url; runner = result.runner; - - console.log(`\n🔧 E2E Test Configuration:`); - console.log(` Worker URL: ${workerUrl}`); - console.log(` Bucket: ${TEST_BUCKET}`); - console.log(` Mount Path: ${MOUNT_PATH}`); - console.log(` Test File: ${TEST_FILE}\n`); }, 30000); afterEach(async () => { - // Cleanup sandbox after each test if (currentSandboxId) { await cleanupSandbox(workerUrl, currentSandboxId); currentSandboxId = null; @@ -99,19 +63,24 @@ describe('Bucket Mounting E2E', () => { }); afterAll(async () => { - // Stop wrangler dev (only in local mode) if (runner) { await runner.stop(); } }); - test('should mount R2 bucket and perform file operations', async () => { + test('should mount bucket and perform file operations', async () => { + // Verify required credentials are present + const requiredVars = ['CLOUDFLARE_ACCOUNT_ID', 'AWS_ACCESS_KEY_ID', 'AWS_SECRET_ACCESS_KEY']; + const missing = requiredVars.filter(v => !process.env[v]); + + if (missing.length > 0) { + throw new Error(`Missing required environment variables: ${missing.join(', ')}`); + } + currentSandboxId = createSandboxId(); const headers = createTestHeaders(currentSandboxId); - console.log(`\n🪣 Step 1: Mounting bucket ${TEST_BUCKET}...`); - - // Mount the R2 bucket + // Mount the bucket const mountResponse = await vi.waitFor( async () => fetchWithStartup(`${workerUrl}/api/bucket/mount`, { @@ -131,10 +100,8 @@ describe('Bucket Mounting E2E', () => { expect(mountResponse.ok).toBe(true); const mountResult = await mountResponse.json(); expect(mountResult.success).toBe(true); - console.log(`✅ Bucket mounted successfully`); // Verify mount point exists - console.log(`\n📂 Step 2: Verifying mount point...`); const verifyResponse = await fetch(`${workerUrl}/api/execute`, { method: 'POST', headers, @@ -146,10 +113,8 @@ describe('Bucket Mounting E2E', () => { const verifyResult = await verifyResponse.json(); expect(verifyResult.stdout?.trim()).toBe('mounted'); expect(verifyResult.exitCode).toBe(0); - console.log(`✅ Mount point verified at ${MOUNT_PATH}`); // Write test file to mounted bucket - console.log(`\n✏️ Step 3: Writing test file...`); const writeResponse = await fetch(`${workerUrl}/api/execute`, { method: 'POST', headers, @@ -160,10 +125,8 @@ describe('Bucket Mounting E2E', () => { const writeResult = await writeResponse.json(); expect(writeResult.exitCode).toBe(0); - console.log(`✅ Test file written: ${TEST_FILE}`); // Read file back - console.log(`\n📖 Step 4: Reading file back...`); const readResponse = await fetch(`${workerUrl}/api/execute`, { method: 'POST', headers, @@ -175,11 +138,8 @@ describe('Bucket Mounting E2E', () => { const readResult = await readResponse.json(); expect(readResult.exitCode).toBe(0); expect(readResult.stdout?.trim()).toBe(TEST_CONTENT); - console.log(`✅ File content verified`); - console.log(` Content: "${readResult.stdout?.trim()}"`); // List directory contents - console.log(`\n📋 Step 5: Listing directory...`); const lsResponse = await fetch(`${workerUrl}/api/execute`, { method: 'POST', headers, @@ -191,11 +151,8 @@ describe('Bucket Mounting E2E', () => { const lsResult = await lsResponse.json(); expect(lsResult.exitCode).toBe(0); expect(lsResult.stdout).toContain(TEST_FILE); - console.log(`✅ Directory listing successful`); - console.log(` ${lsResult.stdout?.trim()}`); // Cleanup: delete test file - console.log(`\n🧹 Step 6: Cleaning up test file...`); const cleanupResponse = await fetch(`${workerUrl}/api/execute`, { method: 'POST', headers, @@ -206,9 +163,6 @@ describe('Bucket Mounting E2E', () => { const cleanupResult = await cleanupResponse.json(); expect(cleanupResult.exitCode).toBe(0); - console.log(`✅ Test file removed`); - - console.log(`\n✅ All E2E bucket mounting tests passed!\n`); }, 120000); // 2 minute timeout }); }); diff --git a/tests/integration/src/index.ts b/tests/integration/src/index.ts index df78ff36..b9010029 100644 --- a/tests/integration/src/index.ts +++ b/tests/integration/src/index.ts @@ -1,8 +1,4 @@ -import { - getSandbox, - proxyToSandbox, - type Sandbox -} from '@cloudflare/sandbox'; +import { getSandbox, proxyToSandbox, type Sandbox } from '@cloudflare/sandbox'; import { codeExamples } from '../shared/examples'; import { executeCommand, From f456c4024c53cf80deae13a6e8542cb81091d0ae Mon Sep 17 00:00:00 2001 From: Naresh Date: Wed, 5 Nov 2025 15:41:28 +0000 Subject: [PATCH 03/13] Reduce supported providers to R2, S3, MinIO, GCS 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. --- .../src/storage-mount/provider-detection.ts | 24 ------------------- .../storage-mount/provider-detection.test.ts | 11 ++------- packages/shared/src/types.ts | 6 +---- 3 files changed, 3 insertions(+), 38 deletions(-) diff --git a/packages/sandbox/src/storage-mount/provider-detection.ts b/packages/sandbox/src/storage-mount/provider-detection.ts index 5c3e9db6..025863d8 100644 --- a/packages/sandbox/src/storage-mount/provider-detection.ts +++ b/packages/sandbox/src/storage-mount/provider-detection.ts @@ -19,14 +19,6 @@ export function detectProviderFromUrl(endpoint: string): BucketProvider | null { return 'r2'; } - if (hostname.includes('.backblazeb2.com')) { - return 'backblaze'; - } - - if (hostname.includes('.wasabisys.com')) { - return 'wasabi'; - } - if (hostname.includes('.amazonaws.com') || hostname.startsWith('s3.')) { return 's3'; } @@ -39,10 +31,6 @@ export function detectProviderFromUrl(endpoint: string): BucketProvider | null { return 'minio'; } - if (hostname.includes('.digitaloceanspaces.com')) { - return 'digitalocean'; - } - return null; } catch { return null; @@ -73,18 +61,6 @@ export function getProviderFlags(provider: BucketProvider | null): string[] { case 'minio': return ['use_path_request_style']; - case 'backblaze': - return []; - - case 'wasabi': - return []; - - case 'digitalocean': - return []; - - case 'custom': - return []; - default: return ['use_path_request_style']; } diff --git a/packages/sandbox/tests/storage-mount/provider-detection.test.ts b/packages/sandbox/tests/storage-mount/provider-detection.test.ts index c3653469..c3e9303f 100644 --- a/packages/sandbox/tests/storage-mount/provider-detection.test.ts +++ b/packages/sandbox/tests/storage-mount/provider-detection.test.ts @@ -11,10 +11,7 @@ describe('Provider Detection', () => { ['https://abc123.r2.cloudflarestorage.com', 'r2'], ['https://s3.us-west-2.amazonaws.com', 's3'], ['https://storage.googleapis.com', 'gcs'], - ['http://minio.local:9000', 'minio'], - ['https://s3.us-west-001.backblazeb2.com', 'backblaze'], - ['https://s3.wasabisys.com', 'wasabi'], - ['https://nyc3.digitaloceanspaces.com', 'digitalocean'] + ['http://minio.local:9000', 'minio'] ])('should detect %s as %s', (url, expectedProvider) => { expect(detectProviderFromUrl(url)).toBe(expectedProvider); }); @@ -32,11 +29,7 @@ describe('Provider Detection', () => { ['r2', ['nomixupload', 'endpoint=auto']], ['s3', []], ['gcs', []], - ['minio', ['use_path_request_style']], - ['backblaze', []], - ['wasabi', []], - ['digitalocean', []], - ['custom', []] + ['minio', ['use_path_request_style']] ])('should return correct flags for %s', (provider, expected) => { expect(getProviderFlags(provider as any)).toEqual(expected); }); diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index 5a219c8b..8eb121d0 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -675,11 +675,7 @@ export type BucketProvider = | 'r2' // Cloudflare R2 | 's3' // Amazon S3 | 'gcs' // Google Cloud Storage - | 'minio' // MinIO - | 'backblaze' // Backblaze B2 - | 'wasabi' // Wasabi - | 'digitalocean' // DigitalOcean Spaces - | 'custom'; // Custom S3-compatible provider + | 'minio'; // MinIO /** * Credentials for S3-compatible storage From d985e889543a13965d9ac1667a3b33411a089139 Mon Sep 17 00:00:00 2001 From: Naresh Date: Wed, 5 Nov 2025 15:54:35 +0000 Subject: [PATCH 04/13] Add bucket mounting support to ExecutionSession Enable bucket mounting/unmounting from session objects returned by createSession(). Sessions share the filesystem, so mount operations affect all sessions in the sandbox. --- packages/sandbox/src/sandbox.ts | 7 ++++++- packages/shared/src/types.ts | 10 ++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index f31016f7..3fc966d1 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -1431,7 +1431,12 @@ export class Sandbox extends Container implements ISandbox { this.codeInterpreter.runCodeStream(code, options), listCodeContexts: () => this.codeInterpreter.listCodeContexts(), deleteCodeContext: (contextId) => - this.codeInterpreter.deleteCodeContext(contextId) + this.codeInterpreter.deleteCodeContext(contextId), + + // Bucket mounting - sandbox-level operations + mountBucket: (bucket, mountPath, options) => + this.mountBucket(bucket, mountPath, options), + unmountBucket: (mountPath) => this.unmountBucket(mountPath) }; } diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index 8eb121d0..c1f94724 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -665,6 +665,14 @@ export interface ExecutionSession { ): Promise>; listCodeContexts(): Promise; deleteCodeContext(contextId: string): Promise; + + // Bucket mounting operations + mountBucket( + bucket: string, + mountPath: string, + options: MountBucketOptions + ): Promise; + unmountBucket(mountPath: string): Promise; } // Bucket mounting types @@ -705,9 +713,7 @@ export interface MountBucketOptions { /** * Optional provider hint for automatic s3fs flag configuration - * * If not specified, will attempt to detect from endpoint URL. - * Use 'custom' if you want to manually specify all s3fs options. * * Examples: * - 'r2' - Cloudflare R2 (adds nomixupload, endpoint=auto) From 0e1c63bb1a40f48226c4a42cab7447258b5c742d Mon Sep 17 00:00:00 2001 From: Naresh Date: Wed, 5 Nov 2025 16:38:11 +0000 Subject: [PATCH 05/13] Fix command injection and race conditions 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. --- .../src/services/file-service.ts | 36 ++++------ .../src/services/git-service.ts | 13 +--- .../sandbox-container/src/shell-escape.ts | 42 ----------- packages/sandbox/src/sandbox.ts | 72 ++++++++++++------- .../src/storage-mount/provider-detection.ts | 4 +- packages/shared/src/git.ts | 6 +- packages/shared/src/index.ts | 2 + packages/shared/src/shell-escape.ts | 8 +++ packages/shared/tests/git.test.ts | 49 ++++++++----- tests/e2e/bucket-mounting.test.ts | 12 +++- 10 files changed, 121 insertions(+), 123 deletions(-) delete mode 100644 packages/sandbox-container/src/shell-escape.ts create mode 100644 packages/shared/src/shell-escape.ts diff --git a/packages/sandbox-container/src/services/file-service.ts b/packages/sandbox-container/src/services/file-service.ts index de5601fb..4e4b43b8 100644 --- a/packages/sandbox-container/src/services/file-service.ts +++ b/packages/sandbox-container/src/services/file-service.ts @@ -1,4 +1,5 @@ import type { FileInfo, ListFilesOptions, Logger } from '@repo/shared'; +import { shellEscape } from '@repo/shared'; import type { FileNotFoundContext, FileSystemContext, @@ -69,17 +70,6 @@ export class FileService implements FileSystemOperations { this.manager = new FileManager(); } - /** - * Escape path for safe shell usage - * Uses single quotes to prevent variable expansion and command substitution - */ - private escapePath(path: string): string { - // Single quotes prevent all expansion ($VAR, `cmd`, etc.) - // To include a literal single quote, we end the quoted string, add an escaped quote, and start a new quoted string - // Example: path="it's" becomes 'it'\''s' - return `'${path.replace(/'/g, "'\\''")}'`; - } - async read( path: string, options: ReadOptions = {}, @@ -131,7 +121,7 @@ export class FileService implements FileSystemOperations { } // 3. Get file size using stat - const escapedPath = this.escapePath(path); + const escapedPath = shellEscape(path); const statCommand = `stat -c '%s' ${escapedPath} 2>/dev/null`; const statResult = await this.sessionManager.executeInSession( sessionId, @@ -369,7 +359,7 @@ export class FileService implements FileSystemOperations { // 2. Write file using SessionManager with base64 encoding // Base64 ensures binary files (images, PDFs, etc.) are written correctly // and avoids heredoc EOF collision issues - const escapedPath = this.escapePath(path); + const escapedPath = shellEscape(path); const base64Content = Buffer.from(content, 'utf-8').toString('base64'); const command = `echo '${base64Content}' | base64 -d > ${escapedPath}`; @@ -492,7 +482,7 @@ export class FileService implements FileSystemOperations { } // 4. Delete file using SessionManager with rm command - const escapedPath = this.escapePath(path); + const escapedPath = shellEscape(path); const command = `rm ${escapedPath}`; const execResult = await this.sessionManager.executeInSession( @@ -594,8 +584,8 @@ export class FileService implements FileSystemOperations { } // 3. Rename file using SessionManager with mv command - const escapedOldPath = this.escapePath(oldPath); - const escapedNewPath = this.escapePath(newPath); + const escapedOldPath = shellEscape(oldPath); + const escapedNewPath = shellEscape(newPath); const command = `mv ${escapedOldPath} ${escapedNewPath}`; const execResult = await this.sessionManager.executeInSession( @@ -696,8 +686,8 @@ export class FileService implements FileSystemOperations { // 3. Move file using SessionManager with mv command // mv is atomic on same filesystem, automatically handles cross-filesystem moves - const escapedSource = this.escapePath(sourcePath); - const escapedDest = this.escapePath(destinationPath); + const escapedSource = shellEscape(sourcePath); + const escapedDest = shellEscape(destinationPath); const command = `mv ${escapedSource} ${escapedDest}`; const execResult = await this.sessionManager.executeInSession( @@ -785,7 +775,7 @@ export class FileService implements FileSystemOperations { const args = this.manager.buildMkdirArgs(path, options); // 3. Build command string from args (skip 'mkdir' at index 0) - const escapedPath = this.escapePath(path); + const escapedPath = shellEscape(path); let command = 'mkdir'; if (options.recursive) { command += ' -p'; @@ -874,7 +864,7 @@ export class FileService implements FileSystemOperations { } // 2. Check if file/directory exists using SessionManager - const escapedPath = this.escapePath(path); + const escapedPath = shellEscape(path); const command = `test -e ${escapedPath}`; const execResult = await this.sessionManager.executeInSession( @@ -970,7 +960,7 @@ export class FileService implements FileSystemOperations { const statCmd = this.manager.buildStatArgs(path); // 4. Build command string (stat with format argument) - const escapedPath = this.escapePath(path); + const escapedPath = shellEscape(path); const command = `stat ${statCmd.args[0]} ${statCmd.args[1]} ${escapedPath}`; // 5. Get file stats using SessionManager @@ -1172,7 +1162,7 @@ export class FileService implements FileSystemOperations { } // 4. Build find command to list files - const escapedPath = this.escapePath(path); + const escapedPath = shellEscape(path); const basePath = path.endsWith('/') ? path.slice(0, -1) : path; // Use find with appropriate flags @@ -1349,7 +1339,7 @@ export class FileService implements FileSystemOperations { sessionId = 'default' ): Promise> { const encoder = new TextEncoder(); - const escapedPath = this.escapePath(path); + const escapedPath = shellEscape(path); return new ReadableStream({ start: async (controller) => { diff --git a/packages/sandbox-container/src/services/git-service.ts b/packages/sandbox-container/src/services/git-service.ts index b003c464..2360555f 100644 --- a/packages/sandbox-container/src/services/git-service.ts +++ b/packages/sandbox-container/src/services/git-service.ts @@ -1,7 +1,7 @@ // Git Operations Service import type { Logger } from '@repo/shared'; -import { sanitizeGitData } from '@repo/shared'; +import { sanitizeGitData, shellEscape } from '@repo/shared'; import type { GitErrorContext, ValidationFailedContext @@ -29,17 +29,10 @@ export class GitService { /** * Build a shell command string from an array of arguments - * Quotes arguments that contain spaces for safe shell execution + * Escapes all arguments to prevent command injection */ private buildCommand(args: string[]): string { - return args - .map((arg) => { - if (arg.includes(' ')) { - return `"${arg}"`; - } - return arg; - }) - .join(' '); + return args.map((arg) => shellEscape(arg)).join(' '); } /** diff --git a/packages/sandbox-container/src/shell-escape.ts b/packages/sandbox-container/src/shell-escape.ts deleted file mode 100644 index 983a0752..00000000 --- a/packages/sandbox-container/src/shell-escape.ts +++ /dev/null @@ -1,42 +0,0 @@ -/** - * Secure shell command utilities to prevent injection attacks - */ - -/** - * Escapes a string for safe use in shell commands. - * This follows POSIX shell escaping rules to prevent command injection. - * - * @param str - The string to escape - * @returns The escaped string safe for shell use - */ -export function escapeShellArg(str: string): string { - // If string is empty, return empty quotes - if (str === '') { - return "''"; - } - - // Check if string contains any characters that need escaping - // Safe characters: alphanumeric, dash, underscore, dot, slash - if (/^[a-zA-Z0-9._\-/]+$/.test(str)) { - return str; - } - - // For strings with special characters, use single quotes and escape single quotes - // Single quotes preserve all characters literally except the single quote itself - // To include a single quote, we end the quoted string, add an escaped quote, and start a new quoted string - return `'${str.replace(/'/g, "'\\''")}'`; -} - -/** - * Escapes a file path for safe use in shell commands. - * - * @param path - The file path to escape - * @returns The escaped path safe for shell use - */ -export function escapeShellPath(path: string): string { - // Normalize path to prevent issues with multiple slashes - const normalizedPath = path.replace(/\/+/g, '/'); - - // Apply standard shell escaping - return escapeShellArg(normalizedPath); -} diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index 3fc966d1..9905d460 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -20,7 +20,12 @@ import type { SessionOptions, StreamOptions } from '@repo/shared'; -import { createLogger, runWithLogger, TraceContext } from '@repo/shared'; +import { + createLogger, + runWithLogger, + shellEscape, + TraceContext +} from '@repo/shared'; import { type ExecuteResponse, SandboxClient } from './clients'; import type { ErrorResponse } from './errors'; import { CustomDomainRequiredError, ErrorCode } from './errors'; @@ -246,35 +251,52 @@ export class Sandbox extends Container implements ISandbox { // Detect credentials const credentials = detectCredentials(options, this.envVars); - // Inject credentials into container environment - const credEnvVars: Record = { - AWS_ACCESS_KEY_ID: credentials.accessKeyId, - AWS_SECRET_ACCESS_KEY: credentials.secretAccessKey - }; - - if (credentials.sessionToken) { - credEnvVars.AWS_SESSION_TOKEN = credentials.sessionToken; - } - - await this.setEnvVars(credEnvVars); - - // Create mount directory - await this.exec(`mkdir -p ${mountPath}`); - - // Execute S3FS mount with provider-specific flags - await this.executeS3FSMount(bucket, mountPath, options, provider); - - // Track active mount (use mountPath as key to support multiple mounts of same bucket) + // Reserve mount path immediately to prevent race conditions + // (two concurrent mount calls would both pass validation otherwise) this.activeMounts.set(mountPath, { bucket, mountPath, endpoint: options.endpoint, provider, credentials, - mounted: true + mounted: false }); - this.logger.info(`Successfully mounted bucket ${bucket} to ${mountPath}`); + try { + // Inject credentials into container environment + const credEnvVars: Record = { + AWS_ACCESS_KEY_ID: credentials.accessKeyId, + AWS_SECRET_ACCESS_KEY: credentials.secretAccessKey + }; + + if (credentials.sessionToken) { + credEnvVars.AWS_SESSION_TOKEN = credentials.sessionToken; + } + + await this.setEnvVars(credEnvVars); + + // Create mount directory + await this.exec(`mkdir -p ${shellEscape(mountPath)}`); + + // Execute S3FS mount with provider-specific flags + await this.executeS3FSMount(bucket, mountPath, options, provider); + + // Mark as successfully mounted + this.activeMounts.set(mountPath, { + bucket, + mountPath, + endpoint: options.endpoint, + provider, + credentials, + mounted: true + }); + + this.logger.info(`Successfully mounted bucket ${bucket} to ${mountPath}`); + } catch (error) { + // Clean up reservation on failure + this.activeMounts.delete(mountPath); + throw error; + } } /** @@ -298,7 +320,7 @@ export class Sandbox extends Container implements ISandbox { // Unmount the filesystem try { - await this.exec(`fusermount -u ${mountPath}`); + await this.exec(`fusermount -u ${shellEscape(mountPath)}`); mountInfo.mounted = false; // Remove from active mounts @@ -391,7 +413,7 @@ export class Sandbox extends Container implements ISandbox { // Build final command const optionsStr = s3fsArgs.join(','); - const mountCmd = `s3fs ${bucket} ${mountPath} -o ${optionsStr}`; + const mountCmd = `s3fs ${shellEscape(bucket)} ${shellEscape(mountPath)} -o ${optionsStr}`; this.logger.debug(`Executing mount command: ${mountCmd}`, { provider, @@ -423,7 +445,7 @@ export class Sandbox extends Container implements ISandbox { this.logger.info( `Unmounting bucket ${mountInfo.bucket} from ${mountPath}` ); - await this.exec(`fusermount -u ${mountPath}`); + await this.exec(`fusermount -u ${shellEscape(mountPath)}`); mountInfo.mounted = false; } catch (error) { const errorMsg = diff --git a/packages/sandbox/src/storage-mount/provider-detection.ts b/packages/sandbox/src/storage-mount/provider-detection.ts index 025863d8..15bfd161 100644 --- a/packages/sandbox/src/storage-mount/provider-detection.ts +++ b/packages/sandbox/src/storage-mount/provider-detection.ts @@ -15,11 +15,11 @@ export function detectProviderFromUrl(endpoint: string): BucketProvider | null { const url = new URL(endpoint); const hostname = url.hostname.toLowerCase(); - if (hostname.includes('.r2.cloudflarestorage.com')) { + if (hostname.endsWith('.r2.cloudflarestorage.com')) { return 'r2'; } - if (hostname.includes('.amazonaws.com') || hostname.startsWith('s3.')) { + if (hostname.endsWith('.amazonaws.com') || hostname.startsWith('s3.')) { return 's3'; } diff --git a/packages/shared/src/git.ts b/packages/shared/src/git.ts index e6164064..3a5de840 100644 --- a/packages/shared/src/git.ts +++ b/packages/shared/src/git.ts @@ -132,7 +132,11 @@ export class GitLogger implements Logger { } error(message: string, error?: Error, context?: Partial): void { - this.baseLogger.error(message, this.sanitizeError(error), this.sanitizeContext(context)); + this.baseLogger.error( + message, + this.sanitizeError(error), + this.sanitizeContext(context) + ); } child(context: Partial): Logger { diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 649e7ff0..bab36e5a 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -43,6 +43,8 @@ export type { StartProcessRequest, WriteFileRequest } from './request-types.js'; +// Export shell utilities +export { shellEscape } from './shell-escape.js'; // Export all types from types.ts export type { BaseExecOptions, diff --git a/packages/shared/src/shell-escape.ts b/packages/shared/src/shell-escape.ts new file mode 100644 index 00000000..c61fe787 --- /dev/null +++ b/packages/shared/src/shell-escape.ts @@ -0,0 +1,8 @@ +/** + * Escapes a string for safe use in shell commands using POSIX single-quote escaping. + * Prevents command injection by wrapping the string in single quotes and escaping + * any single quotes within the string. + */ +export function shellEscape(str: string): string { + return `'${str.replace(/'/g, "'\\''")}'`; +} diff --git a/packages/shared/tests/git.test.ts b/packages/shared/tests/git.test.ts index 2e44a78a..5c706a47 100644 --- a/packages/shared/tests/git.test.ts +++ b/packages/shared/tests/git.test.ts @@ -1,12 +1,12 @@ import { describe, expect, it, vi } from 'vitest'; -import { redactCredentials, sanitizeGitData, GitLogger } from '../src/git'; +import { GitLogger, redactCredentials, sanitizeGitData } from '../src/git'; import { createNoOpLogger } from '../src/logger'; describe('redactCredentials', () => { it('should redact credentials from URLs embedded in text', () => { - expect(redactCredentials('fatal: https://oauth2:token@github.com/repo.git')).toBe( - 'fatal: https://******@github.com/repo.git' - ); + expect( + redactCredentials('fatal: https://oauth2:token@github.com/repo.git') + ).toBe('fatal: https://******@github.com/repo.git'); expect(redactCredentials('https://user:pass@example.com/path')).toBe( 'https://******@example.com/path' ); @@ -17,17 +17,21 @@ describe('redactCredentials', () => { it('should handle multiple URLs in a single string', () => { expect( - redactCredentials('Error: https://token1@host1.com failed, tried https://token2@host2.com') - ).toBe('Error: https://******@host1.com failed, tried https://******@host2.com'); + redactCredentials( + 'Error: https://token1@host1.com failed, tried https://token2@host2.com' + ) + ).toBe( + 'Error: https://******@host1.com failed, tried https://******@host2.com' + ); }); it('should handle URLs in structured formats', () => { - expect(redactCredentials('{"url":"https://token@github.com/repo.git"}')).toBe( - '{"url":"https://******@github.com/repo.git"}' - ); - expect(redactCredentials('https://token@github.com/repo.git')).toBe( - 'https://******@github.com/repo.git' - ); + expect( + redactCredentials('{"url":"https://token@github.com/repo.git"}') + ).toBe('{"url":"https://******@github.com/repo.git"}'); + expect( + redactCredentials('https://token@github.com/repo.git') + ).toBe('https://******@github.com/repo.git'); }); }); @@ -37,15 +41,22 @@ describe('sanitizeGitData', () => { repoUrl: 'https://token@github.com/repo.git', stderr: 'fatal: https://user:pass@gitlab.com/project.git', customField: { nested: 'Error: https://oauth2:token@example.com/path' }, - urls: ['https://ghp_abc@github.com/private.git', 'https://github.com/public.git'], + urls: [ + 'https://ghp_abc@github.com/private.git', + 'https://github.com/public.git' + ], exitCode: 128 }; const sanitized = sanitizeGitData(data); expect(sanitized.repoUrl).toBe('https://******@github.com/repo.git'); - expect(sanitized.stderr).toBe('fatal: https://******@gitlab.com/project.git'); - expect(sanitized.customField.nested).toBe('Error: https://******@example.com/path'); + expect(sanitized.stderr).toBe( + 'fatal: https://******@gitlab.com/project.git' + ); + expect(sanitized.customField.nested).toBe( + 'Error: https://******@example.com/path' + ); expect(sanitized.urls[0]).toBe('https://******@github.com/private.git'); expect(sanitized.urls[1]).toBe('https://github.com/public.git'); expect(sanitized.exitCode).toBe(128); @@ -54,7 +65,9 @@ describe('sanitizeGitData', () => { it('should handle edge cases', () => { expect(sanitizeGitData(null)).toBe(null); expect(sanitizeGitData(undefined)).toBe(undefined); - expect(sanitizeGitData('https://token@github.com/repo.git')).toBe('https://******@github.com/repo.git'); + expect(sanitizeGitData('https://token@github.com/repo.git')).toBe( + 'https://******@github.com/repo.git' + ); }); }); @@ -64,7 +77,9 @@ describe('GitLogger', () => { const errorSpy = vi.spyOn(baseLogger, 'error'); const gitLogger = new GitLogger(baseLogger); - const error = new Error('Auth failed for https://token@github.com/repo.git'); + const error = new Error( + 'Auth failed for https://token@github.com/repo.git' + ); gitLogger.error('Git operation failed', error); expect(errorSpy).toHaveBeenCalledWith( diff --git a/tests/e2e/bucket-mounting.test.ts b/tests/e2e/bucket-mounting.test.ts index 474b9593..a7db3bfb 100644 --- a/tests/e2e/bucket-mounting.test.ts +++ b/tests/e2e/bucket-mounting.test.ts @@ -70,11 +70,17 @@ describe('Bucket Mounting E2E', () => { test('should mount bucket and perform file operations', async () => { // Verify required credentials are present - const requiredVars = ['CLOUDFLARE_ACCOUNT_ID', 'AWS_ACCESS_KEY_ID', 'AWS_SECRET_ACCESS_KEY']; - const missing = requiredVars.filter(v => !process.env[v]); + const requiredVars = [ + 'CLOUDFLARE_ACCOUNT_ID', + 'AWS_ACCESS_KEY_ID', + 'AWS_SECRET_ACCESS_KEY' + ]; + const missing = requiredVars.filter((v) => !process.env[v]); if (missing.length > 0) { - throw new Error(`Missing required environment variables: ${missing.join(', ')}`); + throw new Error( + `Missing required environment variables: ${missing.join(', ')}` + ); } currentSandboxId = createSandboxId(); From bb6701561e607a0478f4079d5370706b6956f59f Mon Sep 17 00:00:00 2001 From: Naresh Date: Wed, 5 Nov 2025 21:48:39 +0000 Subject: [PATCH 06/13] Use password files for s3fs credentials 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. --- .../tests/services/git-service.test.ts | 12 +-- packages/sandbox/src/sandbox.ts | 93 ++++++++++++++----- packages/sandbox/src/storage-mount/types.ts | 1 + 3 files changed, 79 insertions(+), 27 deletions(-) diff --git a/packages/sandbox-container/tests/services/git-service.test.ts b/packages/sandbox-container/tests/services/git-service.test.ts index 6b278427..593b6a48 100644 --- a/packages/sandbox-container/tests/services/git-service.test.ts +++ b/packages/sandbox-container/tests/services/git-service.test.ts @@ -105,14 +105,14 @@ describe('GitService', () => { expect(mockSessionManager.executeInSession).toHaveBeenNthCalledWith( 1, 'default', - 'git clone https://github.com/user/repo.git /workspace/repo' + "'git' 'clone' 'https://github.com/user/repo.git' '/workspace/repo'" ); // Verify SessionManager was called for getting current branch expect(mockSessionManager.executeInSession).toHaveBeenNthCalledWith( 2, 'default', - 'git branch --show-current', + "'git' 'branch' '--show-current'", '/workspace/repo' ); }); @@ -157,7 +157,7 @@ describe('GitService', () => { expect(mockSessionManager.executeInSession).toHaveBeenNthCalledWith( 1, 'session-123', - 'git clone --branch develop https://github.com/user/repo.git /tmp/custom-target' + "'git' 'clone' '--branch' 'develop' 'https://github.com/user/repo.git' '/tmp/custom-target'" ); }); @@ -273,7 +273,7 @@ describe('GitService', () => { // Verify SessionManager was called with correct parameters expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', - 'git checkout develop', + "'git' 'checkout' 'develop'", '/tmp/repo' ); }); @@ -336,7 +336,7 @@ describe('GitService', () => { expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', - 'git branch --show-current', + "'git' 'branch' '--show-current'", '/tmp/repo' ); }); @@ -379,7 +379,7 @@ describe('GitService', () => { expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', - 'git branch -a', + "'git' 'branch' '-a'", '/tmp/repo' ); }); diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index 9905d460..4c4a223f 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -251,6 +251,9 @@ export class Sandbox extends Container implements ISandbox { // Detect credentials const credentials = detectCredentials(options, this.envVars); + // Generate unique password file path + const passwordFilePath = this.generatePasswordFilePath(); + // Reserve mount path immediately to prevent race conditions // (two concurrent mount calls would both pass validation otherwise) this.activeMounts.set(mountPath, { @@ -259,27 +262,26 @@ export class Sandbox extends Container implements ISandbox { endpoint: options.endpoint, provider, credentials, + passwordFilePath, mounted: false }); try { - // Inject credentials into container environment - const credEnvVars: Record = { - AWS_ACCESS_KEY_ID: credentials.accessKeyId, - AWS_SECRET_ACCESS_KEY: credentials.secretAccessKey - }; + // Create password file with credentials + await this.createPasswordFile(passwordFilePath, bucket, credentials); + // Handle session token via environment (s3fs doesn't support in passwd file) if (credentials.sessionToken) { - credEnvVars.AWS_SESSION_TOKEN = credentials.sessionToken; + await this.setEnvVars({ + AWS_SESSION_TOKEN: credentials.sessionToken + }); } - await this.setEnvVars(credEnvVars); - // Create mount directory await this.exec(`mkdir -p ${shellEscape(mountPath)}`); - // Execute S3FS mount with provider-specific flags - await this.executeS3FSMount(bucket, mountPath, options, provider); + // Execute S3FS mount with password file + await this.executeS3FSMount(bucket, mountPath, options, provider, passwordFilePath); // Mark as successfully mounted this.activeMounts.set(mountPath, { @@ -288,11 +290,15 @@ export class Sandbox extends Container implements ISandbox { endpoint: options.endpoint, provider, credentials, + passwordFilePath, mounted: true }); this.logger.info(`Successfully mounted bucket ${bucket} to ${mountPath}`); } catch (error) { + // Clean up password file on failure + await this.deletePasswordFile(passwordFilePath); + // Clean up reservation on failure this.activeMounts.delete(mountPath); throw error; @@ -322,17 +328,15 @@ export class Sandbox extends Container implements ISandbox { try { await this.exec(`fusermount -u ${shellEscape(mountPath)}`); mountInfo.mounted = false; + } finally { + // Always cleanup password file, even if unmount fails + await this.deletePasswordFile(mountInfo.passwordFilePath); // Remove from active mounts this.activeMounts.delete(mountPath); - - this.logger.info(`Successfully unmounted bucket from ${mountPath}`); - } catch (error) { - const errorMsg = error instanceof Error ? error.message : String(error); - throw new S3FSMountError( - `Failed to unmount bucket from ${mountPath}: ${errorMsg}` - ); } + + this.logger.info(`Successfully unmounted bucket from ${mountPath}`); } /** @@ -385,6 +389,46 @@ export class Sandbox extends Container implements ISandbox { } } + /** + * Generate unique password file path for s3fs credentials + */ + private generatePasswordFilePath(): string { + const uuid = crypto.randomUUID(); + return `/tmp/.passwd-s3fs-${uuid}`; + } + + /** + * Create password file with s3fs credentials + * Format: bucket:accessKeyId:secretAccessKey + */ + private async createPasswordFile( + passwordFilePath: string, + bucket: string, + credentials: BucketCredentials + ): Promise { + const content = `${bucket}:${credentials.accessKeyId}:${credentials.secretAccessKey}`; + + await this.writeFile(passwordFilePath, content); + + await this.exec(`chmod 0600 ${shellEscape(passwordFilePath)}`); + + this.logger.debug(`Created password file: ${passwordFilePath}`); + } + + /** + * Delete password file + */ + private async deletePasswordFile(passwordFilePath: string): Promise { + try { + await this.exec(`rm -f ${shellEscape(passwordFilePath)}`); + this.logger.debug(`Deleted password file: ${passwordFilePath}`); + } catch (error) { + this.logger.warn(`Failed to delete password file ${passwordFilePath}`, { + error: error instanceof Error ? error.message : String(error) + }); + } + } + /** * Execute S3FS mount command */ @@ -392,7 +436,8 @@ export class Sandbox extends Container implements ISandbox { bucket: string, mountPath: string, options: MountBucketOptions, - provider: BucketProvider | null + provider: BucketProvider | null, + passwordFilePath: string ): Promise { // Resolve s3fs options (provider defaults + user overrides) const resolvedOptions = resolveS3fsOptions(provider, options.s3fsOptions); @@ -400,6 +445,9 @@ export class Sandbox extends Container implements ISandbox { // Build s3fs mount command const s3fsArgs: string[] = []; + // Add password file option FIRST + s3fsArgs.push(`passwd_file=${passwordFilePath}`); + // Add resolved provider-specific and user options s3fsArgs.push(...resolvedOptions); @@ -411,8 +459,8 @@ export class Sandbox extends Container implements ISandbox { // Add endpoint URL s3fsArgs.push(`url=${options.endpoint}`); - // Build final command - const optionsStr = s3fsArgs.join(','); + // Build final command with escaped options + const optionsStr = shellEscape(s3fsArgs.join(',')); const mountCmd = `s3fs ${shellEscape(bucket)} ${shellEscape(mountPath)} -o ${optionsStr}`; this.logger.debug(`Executing mount command: ${mountCmd}`, { @@ -438,7 +486,7 @@ export class Sandbox extends Container implements ISandbox { override async destroy(): Promise { this.logger.info('Destroying sandbox container'); - // Unmount all mounted buckets + // Unmount all mounted buckets and cleanup password files for (const [mountPath, mountInfo] of this.activeMounts.entries()) { if (mountInfo.mounted) { try { @@ -455,6 +503,9 @@ export class Sandbox extends Container implements ISandbox { ); } } + + // Always cleanup password file + await this.deletePasswordFile(mountInfo.passwordFilePath); } await super.destroy(); diff --git a/packages/sandbox/src/storage-mount/types.ts b/packages/sandbox/src/storage-mount/types.ts index 0902f47e..cf5a0962 100644 --- a/packages/sandbox/src/storage-mount/types.ts +++ b/packages/sandbox/src/storage-mount/types.ts @@ -13,5 +13,6 @@ export interface MountInfo { endpoint: string; provider: BucketProvider | null; credentials: BucketCredentials; + passwordFilePath: string; mounted: boolean; } From 9a340635912a01252d4dc5ddfaa63da3ac1950fe Mon Sep 17 00:00:00 2001 From: Naresh Date: Thu, 6 Nov 2025 10:15:55 +0000 Subject: [PATCH 07/13] Pass new secrets to the PR workflow --- .github/workflows/pullrequest.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/pullrequest.yml b/.github/workflows/pullrequest.yml index d234473a..3a361d5c 100644 --- a/.github/workflows/pullrequest.yml +++ b/.github/workflows/pullrequest.yml @@ -139,6 +139,14 @@ jobs: accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} command: deploy --name ${{ steps.env-name.outputs.worker_name }} workingDirectory: tests/e2e/test-worker + secrets: | + AWS_ACCESS_KEY_ID + AWS_SECRET_ACCESS_KEY + CLOUDFLARE_ACCOUNT_ID + env: + AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} # Construct worker URL from worker name - name: Get deployment URL From bab7454944231716cc1bb9f1cfcedf6affc5ff1f Mon Sep 17 00:00:00 2001 From: Naresh Date: Thu, 6 Nov 2025 11:35:51 +0000 Subject: [PATCH 08/13] Fix R2 endpoint conflict and unmount cleanup 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. --- packages/sandbox/src/sandbox.ts | 6 +++--- packages/sandbox/src/storage-mount/provider-detection.ts | 2 +- .../sandbox/tests/storage-mount/provider-detection.test.ts | 5 ++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index 4c4a223f..e9d6d398 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -328,12 +328,12 @@ export class Sandbox extends Container implements ISandbox { try { await this.exec(`fusermount -u ${shellEscape(mountPath)}`); mountInfo.mounted = false; + + // Only remove from tracking if unmount succeeded + this.activeMounts.delete(mountPath); } finally { // Always cleanup password file, even if unmount fails await this.deletePasswordFile(mountInfo.passwordFilePath); - - // Remove from active mounts - this.activeMounts.delete(mountPath); } this.logger.info(`Successfully unmounted bucket from ${mountPath}`); diff --git a/packages/sandbox/src/storage-mount/provider-detection.ts b/packages/sandbox/src/storage-mount/provider-detection.ts index 15bfd161..7da71458 100644 --- a/packages/sandbox/src/storage-mount/provider-detection.ts +++ b/packages/sandbox/src/storage-mount/provider-detection.ts @@ -50,7 +50,7 @@ export function getProviderFlags(provider: BucketProvider | null): string[] { switch (provider) { case 'r2': - return ['nomixupload', 'endpoint=auto']; + return ['nomixupload']; case 's3': return []; diff --git a/packages/sandbox/tests/storage-mount/provider-detection.test.ts b/packages/sandbox/tests/storage-mount/provider-detection.test.ts index c3e9303f..49a5c145 100644 --- a/packages/sandbox/tests/storage-mount/provider-detection.test.ts +++ b/packages/sandbox/tests/storage-mount/provider-detection.test.ts @@ -26,7 +26,7 @@ describe('Provider Detection', () => { describe('getProviderFlags', () => { it.each([ - ['r2', ['nomixupload', 'endpoint=auto']], + ['r2', ['nomixupload']], ['s3', []], ['gcs', []], ['minio', ['use_path_request_style']] @@ -42,13 +42,12 @@ describe('Provider Detection', () => { describe('resolveS3fsOptions', () => { it('should use provider defaults when no user options', () => { const options = resolveS3fsOptions('r2'); - expect(options).toEqual(['nomixupload', 'endpoint=auto']); + expect(options).toEqual(['nomixupload']); }); it('should merge provider flags with user options', () => { const options = resolveS3fsOptions('r2', ['custom_flag']); expect(options).toContain('nomixupload'); - expect(options).toContain('endpoint=auto'); expect(options).toContain('custom_flag'); }); From 7ca09b7079b08414334e194e4812ac50b7e98e1f Mon Sep 17 00:00:00 2001 From: Naresh Date: Thu, 6 Nov 2025 11:47:52 +0000 Subject: [PATCH 09/13] Remove MinIO as supported provider Port 9000 detection was unreliable and could match non-MinIO services. MinIO buckets still work via safe fallback defaults (use_path_request_style). --- packages/sandbox/src/storage-mount/provider-detection.ts | 7 ------- .../tests/storage-mount/provider-detection.test.ts | 8 +++----- packages/shared/src/types.ts | 7 ++----- 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/packages/sandbox/src/storage-mount/provider-detection.ts b/packages/sandbox/src/storage-mount/provider-detection.ts index 7da71458..0ae528de 100644 --- a/packages/sandbox/src/storage-mount/provider-detection.ts +++ b/packages/sandbox/src/storage-mount/provider-detection.ts @@ -27,10 +27,6 @@ export function detectProviderFromUrl(endpoint: string): BucketProvider | null { return 'gcs'; } - if (hostname.includes('minio') || url.port === '9000') { - return 'minio'; - } - return null; } catch { return null; @@ -58,9 +54,6 @@ export function getProviderFlags(provider: BucketProvider | null): string[] { case 'gcs': return []; - case 'minio': - return ['use_path_request_style']; - default: return ['use_path_request_style']; } diff --git a/packages/sandbox/tests/storage-mount/provider-detection.test.ts b/packages/sandbox/tests/storage-mount/provider-detection.test.ts index 49a5c145..60ebe725 100644 --- a/packages/sandbox/tests/storage-mount/provider-detection.test.ts +++ b/packages/sandbox/tests/storage-mount/provider-detection.test.ts @@ -10,8 +10,7 @@ describe('Provider Detection', () => { it.each([ ['https://abc123.r2.cloudflarestorage.com', 'r2'], ['https://s3.us-west-2.amazonaws.com', 's3'], - ['https://storage.googleapis.com', 'gcs'], - ['http://minio.local:9000', 'minio'] + ['https://storage.googleapis.com', 'gcs'] ])('should detect %s as %s', (url, expectedProvider) => { expect(detectProviderFromUrl(url)).toBe(expectedProvider); }); @@ -28,8 +27,7 @@ describe('Provider Detection', () => { it.each([ ['r2', ['nomixupload']], ['s3', []], - ['gcs', []], - ['minio', ['use_path_request_style']] + ['gcs', []] ])('should return correct flags for %s', (provider, expected) => { expect(getProviderFlags(provider as any)).toEqual(expected); }); @@ -59,7 +57,7 @@ describe('Provider Detection', () => { }); it('should deduplicate flags keeping last occurrence', () => { - const options = resolveS3fsOptions('minio', [ + const options = resolveS3fsOptions(null, [ 'use_path_request_style', 'custom_flag' ]); diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index c1f94724..a5234c10 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -682,8 +682,7 @@ export interface ExecutionSession { export type BucketProvider = | 'r2' // Cloudflare R2 | 's3' // Amazon S3 - | 'gcs' // Google Cloud Storage - | 'minio'; // MinIO + | 'gcs'; // Google Cloud Storage /** * Credentials for S3-compatible storage @@ -705,7 +704,6 @@ export interface MountBucketOptions { * - R2: 'https://abc123.r2.cloudflarestorage.com' * - AWS S3: 'https://s3.us-west-2.amazonaws.com' * - GCS: 'https://storage.googleapis.com' - * - MinIO: 'http://minio.local:9000' * * Required field */ @@ -716,10 +714,9 @@ export interface MountBucketOptions { * If not specified, will attempt to detect from endpoint URL. * * Examples: - * - 'r2' - Cloudflare R2 (adds nomixupload, endpoint=auto) + * - 'r2' - Cloudflare R2 (adds nomixupload) * - 's3' - Amazon S3 (standard configuration) * - 'gcs' - Google Cloud Storage (no special flags needed) - * - 'minio' - MinIO (adds use_path_request_style) */ provider?: BucketProvider; From b51b13ece2cc4b34e41791cab647b789aa59e378 Mon Sep 17 00:00:00 2001 From: Naresh Date: Thu, 6 Nov 2025 11:51:31 +0000 Subject: [PATCH 10/13] Consolidate Dockerfile apt-get layers Merge s3fs/fuse installation with runtime packages to reduce image layer count. --- packages/sandbox/Dockerfile | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/packages/sandbox/Dockerfile b/packages/sandbox/Dockerfile index b475de1a..69ade837 100644 --- a/packages/sandbox/Dockerfile +++ b/packages/sandbox/Dockerfile @@ -113,29 +113,21 @@ ENV DEBIAN_FRONTEND=noninteractive # Set the sandbox version as an environment variable for version checking ENV SANDBOX_VERSION=${SANDBOX_VERSION} -# Install S3FS-FUSE for bucket mounting -RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ - --mount=type=cache,target=/var/lib/apt,sharing=locked \ - rm -f /etc/apt/apt.conf.d/docker-clean && \ - echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' >/etc/apt/apt.conf.d/keep-cache && \ - apt-get update && apt-get install -y --no-install-recommends \ - s3fs \ - fuse - -# Enable FUSE in container - allow non-root users to use FUSE -RUN sed -i 's/#user_allow_other/user_allow_other/' /etc/fuse.conf - -# Install runtime packages and Python runtime libraries +# Install runtime packages and S3FS-FUSE for bucket mounting RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ --mount=type=cache,target=/var/lib/apt,sharing=locked \ rm -f /etc/apt/apt.conf.d/docker-clean && \ echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' >/etc/apt/apt.conf.d/keep-cache && \ apt-get update && apt-get install -y --no-install-recommends \ + s3fs fuse \ ca-certificates curl wget procps git unzip zip jq file \ libssl3 zlib1g libbz2-1.0 libreadline8 libsqlite3-0 \ libncursesw6 libtinfo6 libxml2 libxmlsec1 libffi8 liblzma5 libtk8.6 && \ update-ca-certificates +# Enable FUSE in container - allow non-root users to use FUSE +RUN sed -i 's/#user_allow_other/user_allow_other/' /etc/fuse.conf + # Copy pre-built Python from python-builder stage COPY --from=python-builder /usr/local/python /usr/local/python From a4c0313ce5184d05f30ba40a04849986def9ff3c Mon Sep 17 00:00:00 2001 From: Naresh Date: Mon, 17 Nov 2025 12:44:09 +0000 Subject: [PATCH 11/13] Reduce credential exposure in bucket mounting Remove credentials from MountInfo to minimize sensitive data in Durable Object memory. Password file provides sufficient access for s3fs without retaining credentials. Remove endpoint URL from mount debug log to prevent account ID exposure in production logs. --- packages/sandbox/src/sandbox.ts | 6 +++--- packages/sandbox/src/storage-mount/types.ts | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index e9d6d398..a4d1f12c 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -261,7 +261,6 @@ export class Sandbox extends Container implements ISandbox { mountPath, endpoint: options.endpoint, provider, - credentials, passwordFilePath, mounted: false }); @@ -289,7 +288,6 @@ export class Sandbox extends Container implements ISandbox { mountPath, endpoint: options.endpoint, provider, - credentials, passwordFilePath, mounted: true }); @@ -463,7 +461,9 @@ export class Sandbox extends Container implements ISandbox { const optionsStr = shellEscape(s3fsArgs.join(',')); const mountCmd = `s3fs ${shellEscape(bucket)} ${shellEscape(mountPath)} -o ${optionsStr}`; - this.logger.debug(`Executing mount command: ${mountCmd}`, { + this.logger.debug('Executing s3fs mount', { + bucket, + mountPath, provider, resolvedOptions }); diff --git a/packages/sandbox/src/storage-mount/types.ts b/packages/sandbox/src/storage-mount/types.ts index cf5a0962..cc8e3ddc 100644 --- a/packages/sandbox/src/storage-mount/types.ts +++ b/packages/sandbox/src/storage-mount/types.ts @@ -2,7 +2,7 @@ * Internal bucket mounting types */ -import type { BucketCredentials, BucketProvider } from '@repo/shared'; +import type { BucketProvider } from '@repo/shared'; /** * Internal tracking information for active mounts @@ -12,7 +12,6 @@ export interface MountInfo { mountPath: string; endpoint: string; provider: BucketProvider | null; - credentials: BucketCredentials; passwordFilePath: string; mounted: boolean; } From d96e6bfcf819c89e4508d2a360d953ca6f9f19ff Mon Sep 17 00:00:00 2001 From: Naresh Date: Mon, 17 Nov 2025 13:24:22 +0000 Subject: [PATCH 12/13] Fix hostname validation and logging Replace startsWith('s3.') with exact match for s3.amazonaws.com to prevent unintended domain matches. Remove endpoint URLs from mount logs to avoid exposing account IDs in production logs. --- packages/sandbox/src/sandbox.ts | 5 +---- packages/sandbox/src/storage-mount/provider-detection.ts | 6 +++++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index 161b45d5..8b602b3d 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -248,9 +248,7 @@ export class Sandbox extends Container implements ISandbox { mountPath: string, options: MountBucketOptions ): Promise { - this.logger.info(`Mounting bucket ${bucket} to ${mountPath}`, { - endpoint: options.endpoint - }); + this.logger.info(`Mounting bucket ${bucket} to ${mountPath}`); // Validate options this.validateMountOptions(bucket, mountPath, options); @@ -260,7 +258,6 @@ export class Sandbox extends Container implements ISandbox { options.provider || detectProviderFromUrl(options.endpoint); this.logger.debug(`Detected provider: ${provider || 'unknown'}`, { - endpoint: options.endpoint, explicitProvider: options.provider }); diff --git a/packages/sandbox/src/storage-mount/provider-detection.ts b/packages/sandbox/src/storage-mount/provider-detection.ts index 0ae528de..765a40ea 100644 --- a/packages/sandbox/src/storage-mount/provider-detection.ts +++ b/packages/sandbox/src/storage-mount/provider-detection.ts @@ -19,7 +19,11 @@ export function detectProviderFromUrl(endpoint: string): BucketProvider | null { return 'r2'; } - if (hostname.endsWith('.amazonaws.com') || hostname.startsWith('s3.')) { + // Match AWS S3: *.amazonaws.com or s3.amazonaws.com + if ( + hostname.endsWith('.amazonaws.com') || + hostname === 's3.amazonaws.com' + ) { return 's3'; } From 108844cc3a30328ff85183ff0537685d25a7bf97 Mon Sep 17 00:00:00 2001 From: Naresh Date: Mon, 17 Nov 2025 13:46:41 +0000 Subject: [PATCH 13/13] Remove session token support Session tokens cannot be supported with our password file approach. s3fs requires AWS credentials file format for session tokens, which would compromise security and create multi-bucket conflicts. --- packages/sandbox/src/sandbox.ts | 7 ------- packages/sandbox/src/storage-mount/credential-detection.ts | 3 +-- .../tests/storage-mount/credential-detection.test.ts | 5 +++-- packages/shared/src/types.ts | 1 - 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index 8b602b3d..fda5ec4c 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -282,13 +282,6 @@ export class Sandbox extends Container implements ISandbox { // Create password file with credentials await this.createPasswordFile(passwordFilePath, bucket, credentials); - // Handle session token via environment (s3fs doesn't support in passwd file) - if (credentials.sessionToken) { - await this.setEnvVars({ - AWS_SESSION_TOKEN: credentials.sessionToken - }); - } - // Create mount directory await this.exec(`mkdir -p ${shellEscape(mountPath)}`); diff --git a/packages/sandbox/src/storage-mount/credential-detection.ts b/packages/sandbox/src/storage-mount/credential-detection.ts index def3fcc9..e01032ec 100644 --- a/packages/sandbox/src/storage-mount/credential-detection.ts +++ b/packages/sandbox/src/storage-mount/credential-detection.ts @@ -29,8 +29,7 @@ export function detectCredentials( if (awsAccessKeyId && awsSecretAccessKey) { return { accessKeyId: awsAccessKeyId, - secretAccessKey: awsSecretAccessKey, - sessionToken: envVars.AWS_SESSION_TOKEN + secretAccessKey: awsSecretAccessKey }; } diff --git a/packages/sandbox/tests/storage-mount/credential-detection.test.ts b/packages/sandbox/tests/storage-mount/credential-detection.test.ts index df14d379..8276ea87 100644 --- a/packages/sandbox/tests/storage-mount/credential-detection.test.ts +++ b/packages/sandbox/tests/storage-mount/credential-detection.test.ts @@ -31,7 +31,7 @@ describe('Credential Detection', () => { expect(credentials.secretAccessKey).toBe('aws-secret'); }); - it('should include session token if present', () => { + it('should ignore session token in environment', () => { const envVars = { AWS_ACCESS_KEY_ID: 'aws-key', AWS_SECRET_ACCESS_KEY: 'aws-secret', @@ -41,7 +41,8 @@ describe('Credential Detection', () => { const credentials = detectCredentials(options, envVars); - expect(credentials.sessionToken).toBe('session-token'); + expect(credentials.accessKeyId).toBe('aws-key'); + expect(credentials.secretAccessKey).toBe('aws-secret'); }); it('should prioritize explicit credentials over env vars', () => { diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index 3eeafa68..9a670e43 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -712,7 +712,6 @@ export type BucketProvider = export interface BucketCredentials { accessKeyId: string; secretAccessKey: string; - sessionToken?: string; } /**