diff --git a/CLAUDE.md b/CLAUDE.md index 79da99cb..b048d593 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -217,6 +217,8 @@ npm run test:e2e -- -- tests/e2e/git-clone-workflow.test.ts -t 'should handle cl - Sequential execution (`singleFork: true`) to prevent container resource contention - Longer timeouts (2min per test) for container operations +**Build system trust:** The monorepo build system (turbo + npm workspaces) is robust and handles all package dependencies automatically. E2E tests always run against the latest built code - there's no need to manually rebuild or worry about stale builds unless explicitly working on the build setup itself. + **CI behavior:** E2E tests in CI (`pullrequest.yml`): 1. Build Docker image locally (`npm run docker:local`) diff --git a/package-lock.json b/package-lock.json index 870d390b..db3464b5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -181,7 +181,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", @@ -1130,8 +1129,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", @@ -2283,7 +2281,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", @@ -3316,7 +3313,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" } @@ -3454,7 +3450,6 @@ "integrity": "sha512-oukfKT9Mk41LreEW09vt45f8wx7DordoWUZMYdY/cyAk7w5TWkTRCNZYF7sX7n2wB7jyGAl74OxgwhPgKaqDMQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/utils": "3.2.4", "pathe": "^2.0.3", @@ -3470,7 +3465,6 @@ "integrity": "sha512-dEYtS7qQP2CjU27QBC5oUOxLE/v5eLkGqPE0ZKEIDGMs4vKWe7IjgLOeauHsR0D5YuuycGRO5oSRXnwnmA78fQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/pretty-format": "3.2.4", "magic-string": "^0.30.17", @@ -3499,7 +3493,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", @@ -3721,7 +3714,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "baseline-browser-mapping": "^2.8.19", "caniuse-lite": "^1.0.30001751", @@ -5137,6 +5129,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" }, @@ -6214,6 +6207,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" } @@ -6635,7 +6629,6 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.0.tgz", "integrity": "sha512-tmbWg6W31tQLeB5cdIBOicJDJRR2KzXsV7uSK9iNfLWQ5bIZfxuPEHp7M8wiHyHnn0DD1i7w3Zmin0FtkrwoCQ==", "license": "MIT", - "peer": true, "engines": { "node": ">=0.10.0" } @@ -6657,7 +6650,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", @@ -6842,7 +6836,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" @@ -7503,7 +7496,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -7672,7 +7664,6 @@ "integrity": "sha512-ytQKuwgmrrkDTFP4LjR0ToE2nqgy886GpvRSpU0JAnrdBYppuY5rLkRUYPU1yCryb24SsKBTL/hlDQAEFVwtZg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "~0.25.0", "get-tsconfig": "^4.7.5" @@ -7869,7 +7860,6 @@ "integrity": "sha512-Wj7/AMtE9MRnAXa6Su3Lk0LNCfqDYgfwVjwRFVum9U7wsto1imuHqk4kTm7Jni+5A0Hn7dttL6O/zjvUvoo+8A==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "defu": "^6.1.4", "exsolve": "^1.0.7", @@ -8088,7 +8078,6 @@ "integrity": "sha512-ZWyE8YXEXqJrrSLvYgrRP7p62OziLW7xI5HYGWFzOvupfAlrLvURSzv/FyGyy0eidogEM3ujU+kUG1zuHgb6Ug==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -8205,7 +8194,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -8219,7 +8207,6 @@ "integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@types/chai": "^5.2.2", "@vitest/expect": "3.2.4", @@ -8375,7 +8362,6 @@ "dev": true, "hasInstallScript": true, "license": "Apache-2.0", - "peer": true, "bin": { "workerd": "bin/workerd" }, @@ -8904,7 +8890,6 @@ "integrity": "sha512-PEIGCY5tSlUt50cqyMXfCzX+oOPqN0vuGqWzbcJ2xvnkzkq46oOpz7dQaTDBdfICb4N14+GARUDw2XV2N4tvzg==", "devOptional": true, "license": "MIT", - "peer": true, "engines": { "node": ">=10.0.0" }, diff --git a/packages/sandbox-container/src/handlers/file-handler.ts b/packages/sandbox-container/src/handlers/file-handler.ts index 062d0d96..187785fd 100644 --- a/packages/sandbox-container/src/handlers/file-handler.ts +++ b/packages/sandbox-container/src/handlers/file-handler.ts @@ -75,7 +75,7 @@ export class FileHandler extends BaseHandler { const body = await this.parseRequestBody(request); const result = await this.fileService.readFile(body.path, { - encoding: body.encoding || 'utf-8' + encoding: body.encoding }); if (result.success) { @@ -191,7 +191,7 @@ export class FileHandler extends BaseHandler { const body = await this.parseRequestBody(request); const result = await this.fileService.writeFile(body.path, body.content, { - encoding: body.encoding || 'utf-8' + encoding: body.encoding }); if (result.success) { diff --git a/packages/sandbox-container/src/services/file-service.ts b/packages/sandbox-container/src/services/file-service.ts index de5601fb..ba5942bb 100644 --- a/packages/sandbox-container/src/services/file-service.ts +++ b/packages/sandbox-container/src/services/file-service.ts @@ -219,10 +219,19 @@ export class FileService implements FileSystemOperations { !mimeType.includes('x-empty'); // 6. Read file with appropriate encoding - let content: string; + // Respect user's encoding preference if provided, otherwise use MIME-based detection let actualEncoding: 'utf-8' | 'base64'; + if (options.encoding === 'base64') { + actualEncoding = 'base64'; + } else if (options.encoding === 'utf-8' || options.encoding === 'utf8') { + actualEncoding = 'utf-8'; + } else { + // No explicit encoding requested - use MIME-based detection (original behavior) + actualEncoding = isBinary ? 'base64' : 'utf-8'; + } - if (isBinary) { + let content: string; + if (actualEncoding === 'base64') { // Binary files: read as base64, return as-is (DO NOT decode) const base64Command = `base64 -w 0 < ${escapedPath}`; const base64Result = await this.sessionManager.executeInSession( @@ -261,7 +270,6 @@ export class FileService implements FileSystemOperations { } content = base64Result.data.stdout.trim(); - actualEncoding = 'base64'; } else { // Text files: read normally const catCommand = `cat ${escapedPath}`; @@ -301,7 +309,6 @@ export class FileService implements FileSystemOperations { } content = catResult.data.stdout; - actualEncoding = 'utf-8'; } return { @@ -309,7 +316,7 @@ export class FileService implements FileSystemOperations { data: content, metadata: { encoding: actualEncoding, - isBinary, + isBinary: actualEncoding === 'base64', mimeType, size: fileSize } @@ -366,12 +373,41 @@ 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 + // 2. Write file using SessionManager with proper encoding handling const escapedPath = this.escapePath(path); - const base64Content = Buffer.from(content, 'utf-8').toString('base64'); - const command = `echo '${base64Content}' | base64 -d > ${escapedPath}`; + const encoding = options.encoding || 'utf-8'; + + let command: string; + + if (encoding === 'base64') { + // Content is already base64 encoded, validate and decode it directly to file + // Validate that content only contains valid base64 characters to prevent command injection + if (!/^[A-Za-z0-9+/=]*$/.test(content)) { + return { + success: false, + error: { + message: `Invalid base64 content for '${path}': must contain only A-Z, a-z, 0-9, +, /, =`, + code: ErrorCode.VALIDATION_FAILED, + details: { + validationErrors: [ + { + field: 'content', + message: 'Invalid base64 characters', + code: 'INVALID_BASE64' + } + ] + } satisfies ValidationFailedContext + } + }; + } + // Use printf to output base64 literally without trailing newline + command = `printf '%s' '${content}' | base64 -d > ${escapedPath}`; + } else { + // Encode text to base64 to safely handle shell metacharacters (quotes, backticks, $, etc.) + // and special characters (newlines, control chars, null bytes) in user content + const base64Content = Buffer.from(content, 'utf-8').toString('base64'); + command = `printf '%s' '${base64Content}' | base64 -d > ${escapedPath}`; + } const execResult = await this.sessionManager.executeInSession( sessionId, diff --git a/packages/sandbox-container/tests/services/file-service.test.ts b/packages/sandbox-container/tests/services/file-service.test.ts index 29c39463..1d6c9cb2 100644 --- a/packages/sandbox-container/tests/services/file-service.test.ts +++ b/packages/sandbox-container/tests/services/file-service.test.ts @@ -56,6 +56,37 @@ describe('FileService', () => { }); describe('read', () => { + // Helper to setup common mocks for read operations + function setupReadMocks( + fileSize: number, + mimeType: string, + commandOutput: string + ) { + // Mock exists check + mocked(mockSessionManager.executeInSession).mockResolvedValueOnce({ + success: true, + data: { exitCode: 0, stdout: '', stderr: '' } + } as ServiceResult); + + // Mock stat command + mocked(mockSessionManager.executeInSession).mockResolvedValueOnce({ + success: true, + data: { exitCode: 0, stdout: fileSize.toString(), stderr: '' } + } as ServiceResult); + + // Mock MIME type detection + mocked(mockSessionManager.executeInSession).mockResolvedValueOnce({ + success: true, + data: { exitCode: 0, stdout: mimeType, stderr: '' } + } as ServiceResult); + + // Mock file read command (base64 or cat) + mocked(mockSessionManager.executeInSession).mockResolvedValueOnce({ + success: true, + data: { exitCode: 0, stdout: commandOutput, stderr: '' } + } as ServiceResult); + } + it('should read text file with MIME type detection', async () => { const testPath = '/tmp/test.txt'; const testContent = 'Hello, World!'; @@ -350,6 +381,88 @@ describe('FileService', () => { expect(result.error.code).toBe('FILESYSTEM_ERROR'); } }); + + it('should force base64 encoding when explicitly requested', async () => { + const testPath = '/tmp/text.txt'; + const testContent = 'Hello World'; + const base64Content = Buffer.from(testContent).toString('base64'); + + setupReadMocks(11, 'text/plain', base64Content); + + const result = await fileService.read( + testPath, + { encoding: 'base64' }, + 'session-123' + ); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.data).toBe(base64Content); + expect(result.metadata?.encoding).toBe('base64'); + expect(result.metadata?.isBinary).toBe(true); + expect(result.metadata?.mimeType).toBe('text/plain'); + } + + expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( + 'session-123', + "base64 -w 0 < '/tmp/text.txt'" + ); + }); + + it('should force utf-8 encoding when explicitly requested', async () => { + const testPath = '/tmp/data.bin'; + const testContent = 'Some text content'; + + setupReadMocks(17, 'application/octet-stream', testContent); + + const result = await fileService.read( + testPath, + { encoding: 'utf-8' }, + 'session-123' + ); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.data).toBe(testContent); + expect(result.metadata?.encoding).toBe('utf-8'); + expect(result.metadata?.isBinary).toBe(false); + expect(result.metadata?.mimeType).toBe('application/octet-stream'); + } + + expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( + 'session-123', + "cat '/tmp/data.bin'" + ); + + // Also test 'utf8' alias works the same way + vi.clearAllMocks(); + setupReadMocks(17, 'application/octet-stream', testContent); + + const aliasResult = await fileService.read( + testPath, + { encoding: 'utf8' }, + 'session-123' + ); + + expect(aliasResult.success).toBe(true); + expect(aliasResult.metadata?.encoding).toBe('utf-8'); + }); + + it('should use MIME-based detection when no encoding specified', async () => { + const testPath = '/tmp/auto.json'; + const testContent = '{"key": "value"}'; + + setupReadMocks(16, 'application/json', testContent); + + const result = await fileService.read(testPath, {}, 'session-123'); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.data).toBe(testContent); + expect(result.metadata?.encoding).toBe('utf-8'); + expect(result.metadata?.isBinary).toBe(false); + } + }); }); describe('write', () => { @@ -378,13 +491,127 @@ describe('FileService', () => { expect(result.success).toBe(true); - // Verify SessionManager was called with base64 encoded content (cwd is undefined, so only 2 params) + // Verify SessionManager was called with base64 encoded content expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', - `echo '${base64Content}' | base64 -d > '/tmp/test.txt'` + `printf '%s' '${base64Content}' | base64 -d > '/tmp/test.txt'` ); }); + it('should support utf8 as alias for utf-8 encoding in write', async () => { + const testPath = '/tmp/test.txt'; + const testContent = 'Test content'; + const base64Content = Buffer.from(testContent, 'utf-8').toString( + 'base64' + ); + + mocked(mockSessionManager.executeInSession).mockResolvedValue({ + success: true, + data: { + exitCode: 0, + stdout: '', + stderr: '' + } + } as ServiceResult); + + const result = await fileService.write( + testPath, + testContent, + { encoding: 'utf8' }, + 'session-123' + ); + + expect(result.success).toBe(true); + + // Verify text encoding path is used (printf + base64) + expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( + 'session-123', + `printf '%s' '${base64Content}' | base64 -d > '/tmp/test.txt'` + ); + }); + + it('should write binary file with base64 encoding option', async () => { + const testPath = '/tmp/image.png'; + const binaryData = Buffer.from([0x89, 0x50, 0x4e, 0x47]); // PNG header + const base64Content = binaryData.toString('base64'); + + mocked(mockSessionManager.executeInSession).mockResolvedValue({ + success: true, + data: { + exitCode: 0, + stdout: '', + stderr: '' + } + } as ServiceResult); + + const result = await fileService.write( + testPath, + base64Content, + { encoding: 'base64' }, + 'session-123' + ); + + expect(result.success).toBe(true); + + // Verify that content is passed directly without re-encoding + expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( + 'session-123', + `printf '%s' '${base64Content}' | base64 -d > '/tmp/image.png'` + ); + }); + + it('should reject base64 content with invalid characters', async () => { + const testPath = '/tmp/test.txt'; + + const maliciousInputs = [ + "abc'; rm -rf / #", // Shell command injection + 'valid$(whoami)base64', // Command substitution + 'test\nmalicious', // Newline injection + 'test`whoami`test', // Backtick injection + 'test|whoami', // Pipe injection + 'test&whoami&' // Background command injection + ]; + + for (const maliciousContent of maliciousInputs) { + vi.clearAllMocks(); + + const result = await fileService.write( + testPath, + maliciousContent, + { encoding: 'base64' }, + 'session-123' + ); + + expect(result.success).toBe(false); + expect(result.error?.code).toBe('VALIDATION_FAILED'); + expect(mockSessionManager.executeInSession).not.toHaveBeenCalled(); + } + }); + + it('should accept valid base64 content with padding', async () => { + const testPath = '/tmp/test.txt'; + const validBase64 = 'SGVsbG8gV29ybGQ='; // "Hello World" with padding + + mocked(mockSessionManager.executeInSession).mockResolvedValue({ + success: true, + data: { + exitCode: 0, + stdout: '', + stderr: '' + } + } as ServiceResult); + + const result = await fileService.write( + testPath, + validBase64, + { encoding: 'base64' }, + 'session-123' + ); + + expect(result.success).toBe(true); + expect(mockSessionManager.executeInSession).toHaveBeenCalled(); + }); + it('should handle write errors', async () => { mocked(mockSessionManager.executeInSession).mockResolvedValue({ success: true, diff --git a/packages/sandbox/src/clients/file-client.ts b/packages/sandbox/src/clients/file-client.ts index ef302770..ab99cfcc 100644 --- a/packages/sandbox/src/clients/file-client.ts +++ b/packages/sandbox/src/clients/file-client.ts @@ -98,7 +98,7 @@ export class FileClient extends BaseHttpClient { path, content, sessionId, - encoding: options?.encoding ?? 'utf8' + encoding: options?.encoding }; const response = await this.post('/api/write', data); @@ -126,7 +126,7 @@ export class FileClient extends BaseHttpClient { const data = { path, sessionId, - encoding: options?.encoding ?? 'utf8' + encoding: options?.encoding }; const response = await this.post('/api/read', data);