fix(tool/code_search): guard option-like refs in buildGrepArgs - #727
Merged
lizhengfeng101 merged 1 commit intoAug 5, 2026
Merged
Conversation
Moves the defense-in-depth check from gitGrep() into buildGrepArgs() so the guard is co-located with argument construction. Adds a comment explaining why --end-of-options can't be used here: git grep < 2.45 does not support --end-of-options before the revision. Adds TestBuildGrepArgs_RejectsOptionLikeRef. Signed-off-by: Aalhad <aalhadxx@users.noreply.github.com>
Contributor
|
🔍 OpenCodeReview found 1 issue(s) in this PR.
|
| @@ -1,4 +1,4 @@ | |||
| package tool | |||
| package tool | |||
Contributor
There was a problem hiding this comment.
[bug · medium]
BOM (Byte Order Mark) character added: The diff shows a BOM (U+FEFF) has been added before package tool. This can cause compilation issues with older Go versions or tools that don't expect a BOM. Go source files should not have a BOM as Go uses UTF-8 natively. This appears to be an accidental change.
aalhadxx
force-pushed
the
fix/code-search-end-of-options
branch
from
August 5, 2026 04:31
7736264 to
deaf560
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #645
Description
git grepon Git < 2.45 does not accept--end-of-optionsbetween the pattern and revision arguments. It treats--end-of-optionsas a revision and fails with:This PR removes
--end-of-optionsfrombuildGrepArgsforgit grepand moves the defense-in-depth check (reject refs starting with-) intobuildGrepArgs()so the guard is co-located with argument construction.Type of Change
How Has This Been Tested?
make testpasses locallyI ran the
internal/tooltests locally withgo test ./internal/tool/ -run 'CodeSearch|BuildGrepArgs|GitGrep'and all 25 tests passed. The new testTestBuildGrepArgs_RejectsOptionLikeRefspecifically covers the nil-return path when a ref starts with-.Describe the tests you ran to verify your changes:
TestBuildGrepArgs_RejectsOptionLikeRef— verifiesbuildGrepArgsreturnsnilfor option-like refsTestGitGrep_RejectsOptionLikeRef— verifiesgitGrepreturns the correct error stringTestGitGrep_OptionLikeRefDoesNotLaunchPager— verifies no file is created when an option-like ref is passedBuildGrepArgsandGitGreptests continue to passInclude relevant details about your test configuration:
Checklist
go fmt,go vet)Related Issues
Fixes #645
Addresses review feedback from @lizhengfeng101 in #663
Security Note
Upstream
validateReviewRefs(review_cmd.go:340-365) already rejects refs starting with-and verifies the ref resolves to a real commit viagit rev-parse --verify. The check insidebuildGrepArgsis defense-in-depth in case that function is ever called from a new code path. A comment explains why--end-of-optionscan't be used here:git grep < 2.45does not support--end-of-optionsbefore the revision.