fix: Handle encoding parameter correctly in writeFile and readFile me…

[lacia-hIE]

Fix: Handle encoding parameter correctly in writeFile and readFile methods

Problem Description

The encoding parameter was not being handled correctly in both sandbox.writeFile and sandbox.readFile methods:

writeFile Issues

• Completely ignored the options.encoding parameter
• Always treated content as UTF-8, even when user specified base64 encoding
• Caused base64-encoded binary data (like images) to fail saving correctly

readFile Issues

• Completely ignored the options.encoding parameter
• Only used MIME type to automatically determine encoding
• Users couldn't force a specific encoding format

Fix Details

1. Fix FileService.write Method

File: packages/sandbox-container/src/services/file-service.ts

Before:

// Always treated content as UTF-8
const base64Content = Buffer.from(content, 'utf-8').toString('base64');
const command = `echo '${base64Content}' | base64 -d > ${escapedPath}`;

After:

const encoding = options.encoding || 'utf-8';

let command: string;

if (encoding === 'base64') {
  // Content is already base64 encoded, decode it directly to file
  command = `echo '${content}' | base64 -d > ${escapedPath}`;
} else {
  // Content is text, encode to base64 first to ensure safe shell handling
  const base64Content = Buffer.from(content, 'utf-8').toString('base64');
  command = `echo '${base64Content}' | base64 -d > ${escapedPath}`;
}

2. Fix FileService.read Method

File: packages/sandbox-container/src/services/file-service.ts

Before:

// Only MIME type based encoding decision
const isBinary = !mimeType.startsWith('text/') && ...;

if (isBinary) {
  // Force base64
} else {
  // Force UTF-8
}

After:

// Respect user's encoding preference if provided, otherwise use MIME-based detection
const requestedEncoding = options.encoding;

// Determine final encoding and binary flag
if (requestedEncoding === 'base64') {
  // User explicitly requested base64 - always use base64 regardless of MIME type
  actualEncoding = 'base64';
  isBinary = true;
} else if (requestedEncoding === 'utf-8' || requestedEncoding === 'utf8') {
  // User explicitly requested UTF-8 - always use UTF-8 regardless of MIME type
  actualEncoding = 'utf-8';
  isBinary = false;
} else {
  // No explicit encoding requested - use MIME-based detection (original behavior)
  actualEncoding = isBinaryByMime ? 'base64' : 'utf-8';
  isBinary = isBinaryByMime;
}

Usage Examples

writeFile After Fix

// Save binary data (like images)
const base64Image = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChAI9jYlkKQAAAABJRU5ErkJggg==';
await sandbox.writeFile('/app/image.png', base64Image, { encoding: 'base64' });

// Save text file (default behavior unchanged)
await sandbox.writeFile('/app/config.txt', 'Hello World', { encoding: 'utf-8' });

readFile After Fix

// Force base64 format for text files (for transmission scenarios)
const base64Content = await sandbox.readFile('/app/text.txt', { encoding: 'base64' });

// Force text mode for files that might be misidentified as binary
const textContent = await sandbox.readFile('/app/data.json', { encoding: 'utf-8' });

// Auto-detection (default behavior unchanged)
const autoContent = await sandbox.readFile('/app/file.txt'); // Auto-select encoding based on MIME type

Test Coverage

Existing test cases already cover these scenarios:

• packages/sandbox/tests/file-client.test.ts line 154: Tests base64 encoding write
• packages/sandbox/tests/file-client.test.ts line 251: Tests base64 encoding read
• packages/sandbox-container/tests/services/file-service.test.ts: Contains binary file handling tests

Backward Compatibility

✅ Fully backward compatible

• When encoding parameter is not specified, behavior is identical to before
• Existing code requires no modifications
• Only fixes the previously ignored encoding parameter

Impact Scope

• ✅ Fixed binary file (images, PDFs, etc.) saving via base64 encoding
• ✅ Added user control over file reading encoding method
• ✅ Maintained compatibility with all existing APIs
• ✅ Does not affect any existing functionality

Related Issues

This fix resolves the following user-reported problems:

• sandbox.writeFile with encoding: 'base64' parameter was not working correctly
• sandbox.readFile could not force a specific encoding format

Checklist

• Code changes completed
• Backward compatibility maintained
• Existing tests pass
• Functionality verified
• Documentation updated (this PR document)

---

Modified Files:

• packages/sandbox-container/src/services/file-service.ts

Change Type: Bug Fix

Priority: High (affects binary file handling functionality)

[changeset-bot]

⚠️ No Changeset found

Latest commit: b32abbd

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

This PR includes no changesets

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

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[ghostwriternr]

Thanks for sending this PR! I think this needs to be tested more - can you pls add tests and ensure you've tested them manually too. And add a changeset as well.

[ghostwriternr]

(sorry just setting up the ci properly for external contributors. re-opening this in a moment)

[lacia-hIE]

Summary

This PR addresses the reviewer feedback from #167 by adding security validation and proper encoding parameter handling in file operations.

Changes

1. Security: Base64 Content Validation

• Added validation for base64-encoded content in writeFile() to prevent command injection
• Validates content contains only valid base64 characters: [A-Za-z0-9+/=]
• Returns VALIDATION_FAILED error for malicious input like "abc'; rm -rf / #"
• Improved shell safety by changing from echo to printf '%s'

2. Feature: Respect Encoding Parameter

• Fixed readFile() to honor the encoding option (previously ignored)
• Supports forcing specific encoding regardless of MIME type:
  • encoding: 'base64' - read text files as base64
  • encoding: 'utf-8' - read binary files as text
• Maintains backward compatibility (auto-detection when no encoding specified)

3. Code Quality

• Removed unused variable declaration in read() method
• Added 9 comprehensive test cases (35/35 tests passing)

Test Coverage

Security Tests:

• ✅ Reject invalid base64 characters
• ✅ Reject shell metacharacters ($, ;, etc.)
• ✅ Accept valid base64 with padding

Encoding Tests:

• ✅ Force base64 encoding for text files
• ✅ Force UTF-8 encoding for binary files
• ✅ Support utf8 as alias for utf-8
• ✅ Auto-detection when no encoding specified

Backward Compatibility

✅ Fully compatible - no breaking changes

• Default behavior unchanged
• Existing code requires no modifications
• Only fixes previously ignored parameters

Files Changed

• packages/sandbox-container/src/services/file-service.ts (+24 lines)
• packages/sandbox-container/tests/services/file-service.test.ts (+276 lines)

Related

Addresses reviewer comments in #167:

• Validate and sanitize base64 content
• Remove unused variables

[ghostwriternr]

Hey @lacia-hIE ! Apologies for the long delay on this one, as well as for all the clutter with the repo setup for external contributors. The overall change is quite solid, and I just did some quick cleanup + importantly fixed e2e tests that were failing. See b32abbd. The change looks great overall and good to merge. Thanks for your contribution!