fix: address second order command injection #2
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Potential fix for https://github.com/Lendable/changed-files/security/code-scanning/1
How to generally fix the problem:
To fix this vulnerability, all user-controlled arguments that are used as git ref names or branch names should be validated or sanitized before being passed into any git command. Specifically, you should ensure that neither
head.refnor any other PR-controlled argument can start with-(which would treat the argument as a git option), and ideally check that it matches the allowed format for git ref names.Single best way to fix:
Implement a function (e.g.,
isValidGitRef) that validates candidate branch names before using them as git arguments. The function should ensure:-..., not ending with.lock, etc.) for increased robustness, but at minimum, prevent leading dashes.Specific regions to change:
src/commitSha.ts: Before using any value fromgithub.context.payload.pull_request?.head?.refor...gitFetchExtraArgs, ensure that each ref or extra arg is validated for safety.gitFetchorgitFetchSubmodulesingetSHAForPullRequestEvent. Add a validation for the relevant variables (currentBranch, items ingitFetchExtraArgs), and throw an error if the check fails.What is needed:
isSafeGitArg(val: string): booleaninsrc/utils.ts, just above thegitFetchfunction.src/commitSha.ts, before passingcurrentBranchor anything fromgitFetchExtraArgsto git, assert all are safe usingisSafeGitArg. If an unsafe value is found, throw with a clear message.Suggested fixes powered by Copilot Autofix. Review carefully before merging.