Skip to content

Commit 52133f5

Browse files
committed
fix(crafter): handle invalid merge branch config during repo open
Gracefully handle "branch config: invalid merge" errors from go-git when opening a repository, similar to the existing protection for remote listing. This prevents the crafter from crashing when the local git config has an invalid merge reference. Also upgrades go-git to v5.17.2. Closes #3031 Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
1 parent 1586bad commit 52133f5

4 files changed

Lines changed: 27 additions & 8 deletions

File tree

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ require (
354354
github.com/emicklei/go-restful/v3 v3.13.0 // indirect
355355
github.com/fsnotify/fsnotify v1.9.0 // indirect
356356
github.com/fsouza/fake-gcs-server v1.47.6
357-
github.com/go-git/go-git/v5 v5.17.1
357+
github.com/go-git/go-git/v5 v5.17.2
358358
github.com/go-kratos/aegis v0.2.0 // indirect
359359
github.com/go-logr/logr v1.4.3 // indirect
360360
github.com/go-logr/stdr v1.2.2 // indirect

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -449,8 +449,8 @@ github.com/go-git/go-billy/v5 v5.8.0/go.mod h1:RpvI/rw4Vr5QA+Z60c6d6LXH0rYJo0uD5
449449
github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399 h1:eMje31YglSBqCdIqdhKBW8lokaMrL3uTkpGYlE2OOT4=
450450
github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399/go.mod h1:1OCfN199q1Jm3HZlxleg+Dw/mwps2Wbk9frAWm+4FII=
451451
github.com/go-git/go-git/v5 v5.11.0/go.mod h1:6GFcX2P3NM7FPBfpePbpLd21XxsgdAt+lKqXmCUiUCY=
452-
github.com/go-git/go-git/v5 v5.17.1 h1:WnljyxIzSj9BRRUlnmAU35ohDsjRK0EKmL0evDqi5Jk=
453-
github.com/go-git/go-git/v5 v5.17.1/go.mod h1:pW/VmeqkanRFqR6AljLcs7EA7FbZaN5MQqO7oZADXpo=
452+
github.com/go-git/go-git/v5 v5.17.2 h1:B+nkdlxdYrvyFK4GPXVU8w1U+YkbsgciIR7f2sZJ104=
453+
github.com/go-git/go-git/v5 v5.17.2/go.mod h1:pW/VmeqkanRFqR6AljLcs7EA7FbZaN5MQqO7oZADXpo=
454454
github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
455455
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
456456
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=

pkg/attestation/crafter/crafter.go

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,12 @@ var errBranchInvalidMerge = errors.New("branch config: invalid merge")
310310

311311
// Returns the current directory git commit hash if possible
312312
// If we are not in a git repo it will return an empty string
313-
func gracefulGitRepoHead(path string) (*HeadCommit, error) {
313+
func gracefulGitRepoHead(path string, logger *zerolog.Logger) (*HeadCommit, error) {
314+
if logger == nil {
315+
l := zerolog.Nop()
316+
logger = &l
317+
}
318+
314319
repo, err := git.PlainOpenWithOptions(path, &git.PlainOpenOptions{
315320
// walk up the directory tree until we find a git repo
316321
DetectDotGit: true,
@@ -329,6 +334,15 @@ func gracefulGitRepoHead(path string) (*HeadCommit, error) {
329334
return nil, nil
330335
}
331336

337+
// Mitigate https://github.com/go-git/go-git/issues/331
338+
// This error is not exposed by go-git so we do a string comparison.
339+
// The upcoming go-git v6 no longer returns this error, so this
340+
// workaround can be removed after upgrading.
341+
if isBranchInvalidMergeError(err) {
342+
logger.Warn().Err(err).Msg("branch is invalid merge, skipping commit")
343+
return nil, nil
344+
}
345+
332346
return nil, fmt.Errorf("opening repository: %w", err)
333347
}
334348

@@ -361,7 +375,8 @@ func gracefulGitRepoHead(path string) (*HeadCommit, error) {
361375
// we do not care about that use-case, so we ignore the error
362376
// we compare by error string because go-git does not expose the error type
363377
// and errors.Is require the same instance of the error
364-
if err.Error() == errBranchInvalidMerge.Error() {
378+
if isBranchInvalidMergeError(err) {
379+
logger.Warn().Err(err).Msg("branch is invalid merge, skipping remotes")
365380
return c, nil
366381
}
367382

@@ -387,6 +402,10 @@ func gracefulGitRepoHead(path string) (*HeadCommit, error) {
387402
return c, nil
388403
}
389404

405+
func isBranchInvalidMergeError(err error) bool {
406+
return err.Error() == errBranchInvalidMerge.Error()
407+
}
408+
390409
// Clear any basic auth credentials from the remote URL
391410
func sanitizeRemoteURL(remoteURL string) (string, error) {
392411
uri, err := url.Parse(remoteURL)
@@ -409,7 +428,7 @@ func initialCraftingState(cwd string, opts *InitOpts) (*api.CraftingState, error
409428
return nil, errors.New("required init options not provided")
410429
}
411430
// Get git commit hash
412-
headCommit, err := gracefulGitRepoHead(cwd)
431+
headCommit, err := gracefulGitRepoHead(cwd, opts.Logger)
413432
if err != nil {
414433
return nil, fmt.Errorf("getting git commit hash: %w", err)
415434
}

pkg/attestation/crafter/crafter_unit_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ func (s *crafterUnitSuite) TestGitRepoHead() {
190190
require.NoError(s.T(), err)
191191
}
192192

193-
got, err := gracefulGitRepoHead(path)
193+
got, err := gracefulGitRepoHead(path, nil)
194194
if tc.wantErr {
195195
assert.Error(s.T(), err)
196196
return
@@ -358,7 +358,7 @@ func (s *crafterUnitSuite) TestGitRepoHeadWorktree() {
358358
out, err := cmd.CombinedOutput()
359359
require.NoError(s.T(), err, "git worktree add: %s", out)
360360

361-
got, err := gracefulGitRepoHead(worktreePath)
361+
got, err := gracefulGitRepoHead(worktreePath, nil)
362362
require.NoError(s.T(), err)
363363
require.NotNil(s.T(), got)
364364

0 commit comments

Comments
 (0)