[change_lister] Validate inputs and handle rev-parse error#478
[change_lister] Validate inputs and handle rev-parse error#478sai-miduthuri merged 4 commits intomainfrom
Conversation
Add validation that Remote and BaseBranch don't start with "-" to prevent potential git flag injection. Handle the error from `git rev-parse --is-shallow-repository` instead of silently discarding it, so environment misconfigurations surface clearly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ruct Move GitChangeLister to unexported gitChangeLister with a newGitChangeLister constructor that validates inputs using regex allowlists: remote and baseBranch must match [a-zA-Z0-9._/-], commit must be a 4-40 character hex hash. This prevents git flag injection via crafted branch names or commit values. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the GitChangeLister into an unexported gitChangeLister struct and introduces a newGitChangeLister constructor that validates the remote, base branch, and commit hash using regular expressions. It also adds error handling for the shallow repository check and updates associated tests. Feedback was provided to change the variable declaration of the lister in main.go from a concrete pointer to the ChangeLister interface to avoid "typed nil" interface issues, which can lead to unexpected behavior when checking for nil.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Sai Miduthuri <sai.miduthuri@anyscale.com>
andrew-anyscale
left a comment
andrew-anyscale
left a comment
There was a problem hiding this comment.
Were you encountering errors somewhere?
|
No, I wasn't encountering any errors outside of the ones fixed by your previous PR. I realized that there were some non-urgent bugs in the code as a part of my review there. |
2 participants
Issue
change_listerdoesn't verifyremoteandbranchinputsFix
git rev-parse --is-shallow-repositoryinstead of silently discarding it, so environment misconfigurations surface clearly.