Fix shallow file scans when merge base objects are missing#4900
Open
lawrence3699 wants to merge 1 commit intotrufflesecurity:mainfrom
Open
Fix shallow file scans when merge base objects are missing#4900lawrence3699 wants to merge 1 commit intotrufflesecurity:mainfrom
lawrence3699 wants to merge 1 commit intotrufflesecurity:mainfrom
Conversation
|
|
There was a problem hiding this comment.
Pull request overview
Fixes failures when scanning shallow local file:// git clones with --since-commit <base> --branch <head> by not aborting when the merge-base object is missing locally (but commit enumeration from head is still possible).
Changes:
- Treat
plumbing.ErrObjectNotFoundfromMergeBaseduringnormalizeConfigas non-fatal and continue with the resolved base ref/hash. - Add an end-to-end regression test that builds a shallow local clone scenario and asserts scanning finds feature-branch content without pulling in
main-only content.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/sources/git/git.go | Continue scanning when merge-base resolution fails due to missing objects in shallow clones. |
| pkg/sources/git/git_test.go | Add regression coverage for shallow local file:// clone scanning with base/head branch refs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+724
to
+745
| func TestScanRepo_ShallowLocalCloneWithBranchBaseRef(t *testing.T) { | ||
| ctx := context.Background() | ||
|
|
||
| sourceRepoPath := setupTestRepo(t, "source-repo") | ||
| assert.NoError(t, exec.Command("git", "-C", sourceRepoPath, "branch", "-M", "main").Run()) | ||
| addTestFileAndCommit(t, sourceRepoPath, "base.txt", "base one") | ||
| addTestFileAndCommit(t, sourceRepoPath, "base.txt", "base two") | ||
|
|
||
| assert.NoError(t, exec.Command("git", "-C", sourceRepoPath, "checkout", "-b", "feature").Run()) | ||
| addTestFileAndCommit(t, sourceRepoPath, "feature.txt", "feature secret\nsecret: AKIA1234567890123456\n") | ||
| assert.NoError(t, exec.Command("git", "-C", sourceRepoPath, "checkout", "main").Run()) | ||
| addTestFileAndCommit(t, sourceRepoPath, "main.txt", "main only") | ||
|
|
||
| originRepoPath := filepath.Join(t.TempDir(), "origin.git") | ||
| assert.NoError(t, exec.Command("git", "clone", "--bare", sourceRepoPath, originRepoPath).Run()) | ||
|
|
||
| ciRepoPath := filepath.Join(t.TempDir(), "ci-repo") | ||
| assert.NoError(t, exec.Command("git", "clone", "--depth=1", "--branch", "feature", "file://"+originRepoPath, ciRepoPath).Run()) | ||
| assert.NoError(t, exec.Command("git", "-C", ciRepoPath, "fetch", "--depth=1", "origin", "main:main").Run()) | ||
|
|
||
| preparedRepoPath, _, err := prepareRepoSinceCommit(ctx, "file://"+ciRepoPath, "", "main", false, false) | ||
| assert.NoError(t, err) |
Comment on lines
1217
to
+1221
| // If baseCommit is an ancestor of headCommit, update c.BaseRef to be the common ancestor. | ||
| mergeBase, err := headCommit.MergeBase(baseCommit) | ||
| if err != nil { | ||
| // Shallow local clones can omit the common ancestor object even when git can still | ||
| // enumerate the head-side commits to scan. In that case, keep the resolved base ref |
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.
Description:
Fixes #4895.
When
trufflehog git file://... --since-commit <base> --branch <head>runs against a shallow local clone,normalizeConfigcan fail while resolving the merge base because the common ancestor object is not present locally.In that case, Git can still enumerate the head-side commits to scan, so this change keeps the resolved base ref instead of aborting the scan when
MergeBasereturnsplumbing.ErrObjectNotFound.The new regression test covers the shallow
file://path end to end and asserts that the scan still reports the feature-side change without pulling in themain-only commit.Checklist:
go test ./pkg/sources/git -run TestScanRepo_ShallowLocalCloneWithBranchBaseRef -count=1,go test ./pkg/sources/git -run 'Test(GitConfigSanitization|ScanRepo_ShallowLocalCloneWithBranchBaseRef|GitConfigSanitizationWithBareRepo|GitConfigSecurityIsolation|GitConfigSanitizationWithStagedChanges|PrepareRepoErrorPaths|NormalizeFileURI|PrepareRepoWithNormalization|PrepareRepoWithNormalizationBare)$' -count=1,go test ./pkg/sources/git -run 'TestSource_Chunks_Integration/remote repo, main ahead of branch' -count=1)./scripts/lint.sh)Note
Medium Risk
Changes commit-range normalization for
--since-commitscans: shallow clones that can’t load merge-base objects will now continue with the resolved base ref, which could subtly affect which commits are included/excluded. Covered by a new end-to-end regression test for shallowfile://clones.Overview
Fixes shallow local
file://scans wherenormalizeConfigpreviously aborted ifMergeBasefailed withplumbing.ErrObjectNotFound; it now treats this as non-fatal and proceeds using the resolved base ref.Adds
TestScanRepo_ShallowLocalCloneWithBranchBaseRefto reproduce the CI-style shallow clone scenario and assert the scan still reports feature-branch content without pulling inmain-only commits.Reviewed by Cursor Bugbot for commit 5c91e5a. Bugbot is set up for automated code reviews on this repo. Configure here.