Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Jul 24, 2025

Summary

This PR implements file size validation across all file reading operations in Roo-Code to prevent memory exhaustion and performance issues, as identified in #6155.

Changes

1. Created shared file size constants (src/services/constants/file-limits.ts)

  • MAX_FILE_SIZE_BYTES: 1MB for general files
  • MAX_CONFIG_FILE_SIZE_BYTES: 100KB for configuration files
  • MAX_GITIGNORE_FILE_SIZE_BYTES: 50KB for .gitignore files
  • MAX_CHECKPOINT_FILE_SIZE_BYTES: 5MB for checkpoint files
  • Added formatBytes() helper and FileSizeLimitError class

2. Added file size validation to:

  • roo-config service: readFileIfExists() now checks file size before reading
  • glob/list-files service: .gitignore file reading now validates size
  • ShadowCheckpointService: checkpoint file reading validates size
  • McpHub: all config file reading operations (10+ locations) validate size

3. Added comprehensive unit tests

  • Updated existing tests to mock fs.stat() calls
  • Added new tests for file size validation scenarios
  • Tests verify files exceeding limits are rejected with appropriate warnings

Testing

  • ✅ All unit tests pass for modified services
  • ✅ Linting passes
  • ✅ Type checking passes
  • ✅ Files at size limits are accepted
  • ✅ Files exceeding limits are rejected with console warnings

Notes

  • When files exceed size limits, operations return null or skip the file rather than throwing errors, maintaining backward compatibility
  • Different file types have appropriate size limits based on their typical usage
  • The implementation follows the existing pattern from code indexing service which already had file size validation

Fixes #6155


Important

Add file size validation to prevent memory exhaustion by setting limits for different file types and updating services to enforce these limits.

  • Behavior:
    • Implement file size validation across file reading operations to prevent memory exhaustion.
    • Return null or skip files exceeding size limits to maintain backward compatibility.
  • Constants:
    • Add MAX_FILE_SIZE_BYTES, MAX_CONFIG_FILE_SIZE_BYTES, MAX_GITIGNORE_FILE_SIZE_BYTES, MAX_CHECKPOINT_FILE_SIZE_BYTES in file-limits.ts.
    • Introduce FileSizeLimitError class for handling size violations.
  • Services:
    • Update readFileIfExists() in roo-config to check file size.
    • Add size validation in list-files for .gitignore files.
    • Validate checkpoint file size in ShadowCheckpointService.
    • Validate config file size in McpHub.
  • Testing:
    • Add unit tests for file size validation in list-files-gitignore-size.spec.ts and index.spec.ts.
    • Mock fs.stat() in tests to simulate file size scenarios.

This description was created by Ellipsis for 40a52ae. You can customize this summary. It will automatically update as commits are pushed.

@roomote roomote bot requested review from cte, jr and mrubens as code owners July 24, 2025 08:23
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jul 24, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 24, 2025
- Add MAX_FILE_SIZE_BYTES constants for different file types
- Add size validation to readFileIfExists() in roo-config service
- Add size validation to .gitignore reading in list-files service
- Add size validation to checkpoint file reading in ShadowCheckpointService
- Add size validation to MCP config reading in McpHub
- Add comprehensive tests for file size validation
- Add FileSizeLimitError class for better error handling

This prevents memory exhaustion and performance issues when reading
large files across various Roo-Code services.

Fixes #6155
@roomote roomote bot force-pushed the feature/add-file-size-validation branch from 026fb4d to 40a52ae Compare July 24, 2025 08:55
@daniel-lxs daniel-lxs moved this from Triage to PR [Changes Requested] in Roo Code Roadmap Jul 24, 2025
@daniel-lxs daniel-lxs moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap Jul 24, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jul 24, 2025
@daniel-lxs
Copy link
Member

Closing in favor of #6174

@daniel-lxs daniel-lxs closed this Jul 28, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Jul 28, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working PR - Needs Preliminary Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Bug: Missing File Size Validation in File Reading Operations

4 participants