fix: Handle encoding parameter correctly in writeFile and readFile me… (#167)

* fix: Handle encoding parameter correctly in writeFile and readFile methods

* fix: validate base64 content and respect encoding parameter in file operations

- Add base64 content validation in writeFile to prevent command injection
  - Validate content contains only valid base64 characters (A-Z, a-z, 0-9, +, /, =)
  - Return VALIDATION_FAILED error for invalid content
  - Change from 'echo' to 'printf' for safer shell handling

- Support user-specified encoding in readFile
  - Respect encoding parameter (base64/utf-8/utf8) over MIME detection
  - Allow forcing base64 for text files or utf-8 for binary files
  - Maintain backward compatibility with auto-detection when no encoding specified

- Add comprehensive test coverage
  - 4 tests for encoding parameter support
  - 5 tests for base64 validation and security
  - All tests passing (35/35 in file-service.test.ts)

Addresses reviewer feedback from PR #167:
- Sanitize base64 content to prevent command injection attacks
- Remove unused variable declarations

* Update lockfile

* Clean up encoding logic and improve comments

Remove redundant intermediate variables and assignments in read/write.
Clarify shell command comments to accurately explain printf usage and
base64 encoding rationale. Use printf consistently for both paths.

* Document build system reliability in CLAUDE.md

* Remove encoding defaults to enable MIME auto-detection

Remove 'utf8'/'utf-8' defaults from SDK client and HTTP handler,
allowing file service to perform MIME-based detection when encoding
is not explicitly specified. This fixes binary file detection.

---------

Co-authored-by: luxuncang <[email protected]>
Co-authored-by: Naresh <[email protected]>

Author: 3 people

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`)

package-lock.json

@@ -75,7 +75,7 @@ export class FileHandler extends BaseHandler<Request, Response> {
     const body = await this.parseRequestBody<ReadFileRequest>(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<Request, Response> {
     const body = await this.parseRequestBody<WriteFileRequest>(request);
 
     const result = await this.fileService.writeFile(body.path, body.content, {
-      encoding: body.encoding || 'utf-8'
+      encoding: body.encoding
     });
 
     if (result.success) {

packages/sandbox-container/src/handlers/file-handler.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,15 +309,14 @@ export class FileService implements FileSystemOperations {
         }
 
         content = catResult.data.stdout;
-        actualEncoding = 'utf-8';
       }
 
       return {
         success: true,
         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,