-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Replace spawn with execFile for memory-safe command execution #1068
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📋 Review SummaryThis PR introduces significant improvements to command execution security and reliability by replacing 🔍 General Feedback
🎯 Specific Feedback🔴 CriticalNo critical issues identified in this review. 🟡 HighNo high priority issues identified in this review. 🟢 Medium
🔵 Low
✅ Highlights
|
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
pomelo-nwu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary
spawnwithexecFileand 20MBmaxBufferto prevent memory exhaustionProblem
Using
spawnfor command execution could cause memory exhaustion when processing large outputs, potentially crashing the Node.js process. This was a critical security and stability risk.Solution
Migrated
ripgrepshell execution fromspawntoexecFilewith explicit buffer limits:Changes
Core Security Fix
ripgrepUtils.ts: ReplacespawnwithexecFileand 20MBmaxBuffershell-utils.ts: CentralizedexecCommandusingexecFilewith buffer limitsCross-Platform Improvements
.exehandling and PowerShell/cmd detectionUpdated Components
runRipgrepexecCommandImpact
Testing
Breaking Changes
None - all changes maintain existing API contracts.
Related issues