Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Nov 5, 2025

Fixes shell injection vulnerability in pre-push hook environment loading.

Problem:
The original export $(grep -v '^\#' .env.local | xargs) pattern was vulnerable to shell injection. Malicious content in .env.local like RUN_TESTS_ON_PUSH=true; rm -rf / could execute arbitrary commands.

Solution:
Uses the existing @dotenvx/dotenvx dependency to securely parse .env.local without shell evaluation:

  • npx dotenvx get safely extracts specific variables
  • npx dotenvx run executes commands with environment loaded securely
  • Eliminates shell injection attack vectors while maintaining functionality

Security Benefits:

  • No shell evaluation of .env.local contents
  • Prevents execution of embedded commands or scripts
  • Uses established, secure dotenv parsing library
  • Maintains backward compatibility with existing behavior

@daniel-lxs daniel-lxs requested review from cte, jr and mrubens as code owners November 5, 2025 20:05
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Nov 5, 2025
@roomote
Copy link

roomote bot commented Nov 5, 2025

Rooviewer Clock   See task on Roo Cloud

Review complete. The shell injection vulnerability has been resolved by replacing the unsafe pattern with dotenvx.

  • The source .env.local command still executes the file as shell code, allowing malicious commands to run
Previous reviews

Mention @roomote to ask your PR Fixer agent to address the feedback.

@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Nov 5, 2025
- Replace unsafe export $(grep -v '^#' .env.local | xargs) pattern
- Use dotenvx for secure environment variable parsing without shell evaluation
- Prevents execution of malicious commands in .env.local values
- Uses existing @dotenvx/dotenvx dependency already in project
- Maintains same functionality while eliminating injection vulnerability
@daniel-lxs daniel-lxs force-pushed the fix/shell-injection-husky-pre-push branch from e55b260 to 971eaf9 Compare November 5, 2025 20:16
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Nov 5, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 6, 2025
@mrubens mrubens merged commit 37ac53e into main Nov 6, 2025
15 checks passed
@mrubens mrubens deleted the fix/shell-injection-husky-pre-push branch November 6, 2025 21:40
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Nov 6, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Nov 6, 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 lgtm This PR has been approved by a maintainer PR - Needs Review size:S This PR changes 10-29 lines, ignoring generated files.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants