fix(ingest): reject option-shaped --base-ref before invoking git#4
Merged
Merged
Conversation
git_heuristic concatenates the operator-supplied --base-ref into
"<ref>..HEAD" and "<ref>:<path>" and feeds them to `git diff` /
`git show` via exec.Command. There's no shell, so classic shell
injection was never on the table — but git's own argument parser
treats anything starting with `-` as an option flag regardless of
position. Today git diff happens to reject the option-shapes we
care about ("--upload-pack=…", "--exec-path=…", "-HEAD", "--"),
but relying on the downstream tool's error handling is fragile:
a future git version or a different subcommand could turn a
rejected flag into an accepted one.
Now safeGitRef rejects empty refs, refs equal to "--", and any ref
starting with "-" at the adapter boundary, returning a wrapped
ErrInvalidBaseRef so callers can distinguish input rejection from
git-side failure. The `git diff` invocation also gains a trailing
"--" so the rev/pathspec boundary is explicit, belt-and-braces.
Coverage gate: ingest 95.2% (target 87), global 88.0% (target 87).
tzone85
added a commit
that referenced
this pull request
Jun 11, 2026
Adds an Unreleased section to CHANGELOG.md covering the five sequential PRs that landed today: - #2 chore(security): bump x/sys for GO-2026-5024 - #3 fix(api): validate tenant id at route boundary - #4 fix(ingest): reject option-shaped --base-ref - #5 fix(mempalace): validate Kind/Tenant/Key against path traversal - #6 chore(supply-chain): pin govulncheck + base images by digest Updates README Status to mention the hardening pass and replaces a stale `§9` design-spec cross-reference with the section's actual name (per the project's doc-style rule against the § symbol). No code changes, no behaviour changes — pure documentation refresh for the audit story.
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
1 participant
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.
Summary
safeGitRefrejects empty refs,--, and any ref starting with-at the adapter boundary, returning a wrappedErrInvalidBaseRef.git diffinvocation gains a trailing--so the rev/pathspec boundary is explicit.exec.Command), so classic shell injection wasn't on the table. The actual risk: git's argument parser treats anything starting with-as an option flag regardless of position. Today git diff happens to reject--upload-pack=…,--exec-path=…,-HEAD,--— but relying on the downstream tool's error handling is fragile.TDD
RED first:
TestGitHeuristic_RejectsOptionLikeBaseReftable test for 4 option-shaped baserefs. Initial assertion againstErrInvalidBaseReffailed-to-compile (undefined symbol) — proves the test was driving the new sentinel, not git's downstream error.GREEN: defined
ErrInvalidBaseRef,safeGitRef, and the adapter-boundary check. All 4 sub-tests pass.Test plan
go test -race ./...— all packages greenbash scripts/cover_check.sh— PASS, ingest 95.2% (target 87), global 88.0% (target 87)golangci-lint run— 0 issues