From 075f8c67a56fd8f62fe28b0f74508d32cee4d11d Mon Sep 17 00:00:00 2001
From: "Valentin B." <703631+beeb@users.noreply.github.com>
Date: Tue, 10 Sep 2024 19:12:57 +0200
Subject: [PATCH] fix(git): config normalization for git sources (#3278)

When normalizing the git source config, the base and head refs should be normalized to commit hashes, in case
a branch or tag name was used. The `resolveAndSetCommit` function was returning a boolean value which should
indicate whether the input ref was changed from its original value. While this is in itself not a problem,
the caller (`normalizeConfig`) was using this boolean as an error marker, and returning early in case of `false`.

This meant that if the config was already containing a commit hash for the base or head ref, `resolveAndSetCommit`
would set the flag to `false` and `normalizeConfig` would early return erreneously. This caused the logic to find
the ancestor commit to be skipped which caused the bug in the issue #3220.

Since the `resolveAndSetCommit` function was only used in `normalizeConfig`, the signature has been changed to only
return the commit object and an error. The check for early return in `normalizeConfig` now instead relies on the
commit object being `nil` to indicate a failure to resolve the ref.

Refs: #3220
---
 pkg/sources/git/git.go | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/pkg/sources/git/git.go b/pkg/sources/git/git.go
index aefe9f6ef..b98cadbee 100644
--- a/pkg/sources/git/git.go
+++ b/pkg/sources/git/git.go
@@ -969,17 +969,17 @@ func (s *Git) ScanRepo(ctx context.Context, repo *git.Repository, repoPath strin
 // If either commit cannot be resolved, it returns early.
 // If both are resolved, it finds and sets the merge base in scanOptions.
 func normalizeConfig(scanOptions *ScanOptions, repo *git.Repository) error {
-	baseCommit, baseSet, err := resolveAndSetCommit(repo, &scanOptions.BaseHash)
+	baseCommit, err := resolveAndSetCommit(repo, &scanOptions.BaseHash)
 	if err != nil {
 		return err
 	}
 
-	headCommit, headSet, err := resolveAndSetCommit(repo, &scanOptions.HeadHash)
+	headCommit, err := resolveAndSetCommit(repo, &scanOptions.HeadHash)
 	if err != nil {
 		return err
 	}
 
-	if !(baseSet && headSet) {
+	if baseCommit == nil || headCommit == nil {
 		return nil
 	}
 
@@ -998,32 +998,31 @@ func normalizeConfig(scanOptions *ScanOptions, repo *git.Repository) error {
 }
 
 // resolveAndSetCommit resolves a Git reference to a commit object and updates the reference if it was not a direct hash.
-// Returns the commit object, a boolean indicating if the commit was successfully set, and any error encountered.
-func resolveAndSetCommit(repo *git.Repository, ref *string) (*object.Commit, bool, error) {
+// Returns the commit object and any error encountered.
+func resolveAndSetCommit(repo *git.Repository, ref *string) (*object.Commit, error) {
 	if repo == nil || ref == nil {
-		return nil, false, fmt.Errorf("repo and ref must be non-nil")
+		return nil, fmt.Errorf("repo and ref must be non-nil")
 	}
 	if len(*ref) == 0 {
-		return nil, false, nil
+		return nil, nil
 	}
 
 	originalRef := *ref
 	resolvedRef, err := resolveHash(repo, originalRef)
 	if err != nil {
-		return nil, false, fmt.Errorf("unable to resolve ref: %w", err)
+		return nil, fmt.Errorf("unable to resolve ref: %w", err)
 	}
 
 	commit, err := repo.CommitObject(plumbing.NewHash(resolvedRef))
 	if err != nil {
-		return nil, false, fmt.Errorf("unable to resolve commit: %w", err)
+		return nil, fmt.Errorf("unable to resolve commit: %w", err)
 	}
 
-	wasSet := originalRef != resolvedRef
-	if wasSet {
+	if originalRef != resolvedRef {
 		*ref = resolvedRef
 	}
 
-	return commit, wasSet, nil
+	return commit, nil
 }
 
 func resolveHash(repo *git.Repository, ref string) (string, error) {