diff --git a/analyze/analyze.go b/analyze/analyze.go index 7a91c06..31c64ea 100644 --- a/analyze/analyze.go +++ b/analyze/analyze.go @@ -175,7 +175,7 @@ func (a *Analyzer) AnalyzeOrg(ctx context.Context, org string, numberOfGoroutine defer reposWg.Done() repoNameWithOwner := repo.GetRepoIdentifier() obs.OnRepoStarted(repoNameWithOwner) - repoKey, err := a.cloneRepo(ctx, repo.BuildGitURL(a.ScmClient.GetProviderBaseURL()), a.ScmClient.GetToken(), "HEAD") + repoKey, err := a.cloneRepo(ctx, repo.BuildGitURL(a.ScmClient.GetProviderBaseURL()), a.ScmClient.GetToken(), defaultBranchCloneRef(repo)) if err != nil { log.Error().Err(err).Str("repo", repoNameWithOwner).Msg("failed to clone repo") obs.OnRepoError(repoNameWithOwner, err) @@ -450,7 +450,11 @@ func (a *Analyzer) AnalyzeRepo(ctx context.Context, repoString string, ref strin log.Debug().Msgf("Starting repository analysis for: %s/%s on %s", org, repoName, provider) obs.OnRepoStarted(repoString) - repoKey, err := a.cloneRepo(ctx, repo.BuildGitURL(a.ScmClient.GetProviderBaseURL()), a.ScmClient.GetToken(), ref) + cloneRef := ref + if ref == "" || ref == "HEAD" { + cloneRef = defaultBranchCloneRef(repo) + } + repoKey, err := a.cloneRepo(ctx, repo.BuildGitURL(a.ScmClient.GetProviderBaseURL()), a.ScmClient.GetToken(), cloneRef) if err != nil { obs.OnRepoError(repoString, err) return nil, err @@ -749,3 +753,15 @@ func (a *Analyzer) cloneRepo(ctx context.Context, gitURL string, token string, r } return key, nil } + +// defaultBranchCloneRef returns an explicit refs/heads/ ref derived +// from the SCM-provided default branch. Passing this to Clone avoids both +// the ls-remote in the HEAD discovery path and the ls-remote in the bare-ref +// resolver. Falls back to "HEAD" when the SCM didn't provide a default, +// which routes through Clone's discovery path for correctness. +func defaultBranchCloneRef(repo Repository) string { + if db := repo.GetDefaultBranch(); db != "" { + return "refs/heads/" + db + } + return "HEAD" +} diff --git a/providers/gitops/gitops.go b/providers/gitops/gitops.go index dd442d1..1b4b3fb 100644 --- a/providers/gitops/gitops.go +++ b/providers/gitops/gitops.go @@ -138,37 +138,47 @@ func (g *GitClient) Clone(ctx context.Context, clonePath string, url string, tok switch { case ref == "HEAD": - // Try "main" first (most common), then "master", then ls-remote as fallback - for _, branch := range []string{"main", "master"} { + // Discover the actual default branch via ls-remote HEAD symref. + // Doing this first (rather than fast-pathing to main/master) avoids + // analyzing the wrong branch on repositories whose default is not main. + discovered := discoverDefaultBranch(repo, token) + if discovered != "" { fetchOpts.RefSpecs = []config.RefSpec{ - config.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/remotes/origin/%s", branch, branch)), + config.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/remotes/origin/%s", discovered, discovered)), } err = repo.FetchContext(ctx, fetchOpts) if err == nil { - defaultBranch = branch - resolved.localRef = plumbing.ReferenceName("refs/remotes/origin/" + branch) - break - } - if classifyFetchError(err) != nil && !strings.Contains(err.Error(), "couldn't find remote ref") { - return classifyFetchError(err) + defaultBranch = discovered + resolved.localRef = plumbing.ReferenceName("refs/remotes/origin/" + discovered) + } else if cerr := classifyFetchError(err); cerr != nil && !strings.Contains(err.Error(), "couldn't find remote ref") { + return cerr } } if defaultBranch == "" { - // Neither main nor master — ls-remote to find actual default - discovered := discoverDefaultBranchFromURL(url, token) - if discovered != "" { + // Discovery failed (e.g. server didn't advertise HEAD, or fetch of + // discovered branch failed). Try common defaults before falling + // back to fetching all branches. + for _, branch := range []string{"main", "master"} { fetchOpts.RefSpecs = []config.RefSpec{ - config.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/remotes/origin/%s", discovered, discovered)), + config.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/remotes/origin/%s", branch, branch)), + } + err = repo.FetchContext(ctx, fetchOpts) + if err == nil { + defaultBranch = branch + resolved.localRef = plumbing.ReferenceName("refs/remotes/origin/" + branch) + break + } + if cerr := classifyFetchError(err); cerr != nil && !strings.Contains(err.Error(), "couldn't find remote ref") { + return cerr } - resolved.localRef = plumbing.ReferenceName("refs/remotes/origin/" + discovered) - } else { - fetchOpts.RefSpecs = []config.RefSpec{config.RefSpec("+refs/heads/*:refs/remotes/origin/*")} } + } + if defaultBranch == "" { + fetchOpts.RefSpecs = []config.RefSpec{config.RefSpec("+refs/heads/*:refs/remotes/origin/*")} err = repo.FetchContext(ctx, fetchOpts) if err := classifyFetchError(err); err != nil { return err } - defaultBranch = discovered } default: resolved, err = resolveRemoteRef(repo, url, token, ref) @@ -772,20 +782,10 @@ func peelToCommit(store storer.EncodedObjectStorer, hash plumbing.Hash) (plumbin } } -// looksLikeSHA returns true if s looks like a full-length git commit SHA. -// discoverDefaultBranch uses remote.List to find the HEAD symref target. -// Returns empty string if it can't be determined. -// discoverDefaultBranchFromURL does a lightweight ls-remote to find the HEAD symref. -func discoverDefaultBranchFromURL(url string, token string) string { - store := memory.NewStorage() - repo, err := gogit.Init(store, nil) - if err != nil { - return "" - } - _, err = repo.CreateRemote(&config.RemoteConfig{Name: "origin", URLs: []string{url}}) - if err != nil { - return "" - } +// discoverDefaultBranch performs a lightweight ls-remote on the repo's origin +// remote and returns the branch name that HEAD points to. Returns "" when the +// server doesn't advertise a HEAD symref or the listing fails. +func discoverDefaultBranch(repo *gogit.Repository, token string) string { remote, err := repo.Remote("origin") if err != nil { return "" diff --git a/providers/gitops/gitops_test.go b/providers/gitops/gitops_test.go index 0b04ed2..fb67567 100644 --- a/providers/gitops/gitops_test.go +++ b/providers/gitops/gitops_test.go @@ -157,6 +157,34 @@ func TestClassifyFetchError(t *testing.T) { assert.Error(t, err) } +// TestDiscoverDefaultBranchPrefersRemoteHEAD is the regression for #436: +// when a repo has both `main` and another branch, and HEAD points to the +// other branch, discoverDefaultBranch must report the HEAD-pointed branch +// rather than fast-pathing to "main". Clone() consults this helper before +// falling back to "main"/"master", so getting this right ensures HEAD +// analysis follows the actual default branch. +func TestDiscoverDefaultBranchPrefersRemoteHEAD(t *testing.T) { + dir := t.TempDir() + remote, err := gogit.PlainInit(dir, false) + require.NoError(t, err) + + writeRepoFile(t, dir, "action.yml", "name: initial\n") + initialCommit := commitAll(t, remote, "initial commit") + + require.NoError(t, remote.Storer.SetReference( + plumbing.NewHashReference(plumbing.ReferenceName("refs/heads/main"), initialCommit), + )) + require.NoError(t, remote.Storer.SetReference( + plumbing.NewHashReference(plumbing.ReferenceName("refs/heads/zip-zip"), initialCommit), + )) + require.NoError(t, remote.Storer.SetReference( + plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.ReferenceName("refs/heads/zip-zip")), + )) + + client := createTestClientRepo(t, dir) + assert.Equal(t, "zip-zip", discoverDefaultBranch(client, "")) +} + func TestResolveRemoteRefBareTagPrefersTag(t *testing.T) { remotePath, _ := createTestRemoteRepo(t) repo := createTestClientRepo(t, remotePath)