Skip to content

fix(ng-dev): prevent shell command injection in git push command#3735

Merged
alan-agius4 merged 1 commit into
angular:mainfrom
josephperrott:fix-command-injection-checkout
Jun 5, 2026
Merged

fix(ng-dev): prevent shell command injection in git push command#3735
alan-agius4 merged 1 commit into
angular:mainfrom
josephperrott:fix-command-injection-checkout

Conversation

@josephperrott
Copy link
Copy Markdown
Member

This PR mitigates a command injection vulnerability where a malicious PR branch name could lead to arbitrary shell command execution when copy-pasting the printed ng-dev checkout or rebase commands.

This escapes the branch name before interpolating it into the manual git push command printed to the console, preventing a copy-paste command injection vulnerability.
@josephperrott josephperrott added the action: merge The PR is ready for merge by the caretaker label Jun 4, 2026
@josephperrott josephperrott requested a review from alan-agius4 June 4, 2026 22:02
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces escaping for the git branch name and the --force-with-lease flag to safely handle special characters (such as single quotes) in shell commands. The reviewer suggested a cleaner, more idiomatic approach of quoting the entire argument (e.g., 'HEAD:branch' and '--force-with-lease=branch:oid') rather than using partial quoting, and provided code suggestions to implement this across both modified files.

Comment thread ng-dev/pr/common/checkout-pr.ts
Comment thread ng-dev/pr/common/checkout-pr.ts
Comment thread ng-dev/pr/rebase/index.ts
Comment thread ng-dev/pr/rebase/index.ts
@alan-agius4 alan-agius4 merged commit 5b7bfdf into angular:main Jun 5, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants