feat: add retry logic to scorecard API for commit-specific queries (#13)

feat: retry logic, normalizeRepoName to scorecard api

when querying the Scorecard API with a specific commitSHA that returns 404,
retry the request without the commit parameter to get the latest scorecard
for the default branch.

extract fetchFromAPI helper for HTTP request logic
add retry-without-commit logic in getScoreFromAPI
update tests to cover retry behavior and edge cases

---------

Co-authored-by: Jayashree O <jaishu138@gmail.com>
Co-authored-by: Shreyas Pandya <pandyashreyas1@gmail.com>
Co-authored-by: Abhishek Agrawal <abhishek.yours4@gmail.com>
Co-authored-by: Anirudh Edpuganti <aniedpuganti@gmail.com>
Signed-off-by: Paul Joseph <k.paul.joseph@gmail.com>

Author: 5 people

pkg/certifier/scorecard/scorecardRunner.go

@@ -18,26 +18,38 @@ package scorecard
 import (
 	"context"
 	"fmt"
-	"io"
-	"net/http"
-	"net/url"
-	"os"
-	"time"
-
 	"github.com/guacsec/guac/pkg/logging"
 	"github.com/ossf/scorecard/v4/checker"
 	"github.com/ossf/scorecard/v4/checks"
 	"github.com/ossf/scorecard/v4/log"
 	sc "github.com/ossf/scorecard/v4/pkg"
+	"io"
+	"net/http"
+	"net/url"
+	"os"
+	"strings"
+	"time"
 )
 
+const githubPrefix = "github.com/"
+
 // scorecardRunner is a struct that implements the Scorecard interface.
 type scorecardRunner struct {
 	ctx context.Context
 }
 
+// normalizeRepoName ensures the repo name has the github.com/ prefix required by the Scorecard API.
+// If the prefix is missing, it is added. If the repo name already has the prefix, it is returned as-is.
+func normalizeRepoName(repoName string) string {
+	if strings.HasPrefix(repoName, githubPrefix) {
+		return repoName
+	}
+	return githubPrefix + repoName
+}
+
 func (s scorecardRunner) GetScore(repoName, commitSHA, tag string) (*sc.ScorecardResult, error) {
 	logger := logging.FromContext(s.ctx)
+	repoName = normalizeRepoName(repoName)
 
 	// First try API approach
 	logger.Debugf("Attempting to fetch scorecard from API for repo: %s, commit: %s", repoName, commitSHA)
@@ -67,30 +79,44 @@ func (s scorecardRunner) GetScore(repoName, commitSHA, tag string) (*sc.Scorecar
 func (s scorecardRunner) getScoreFromAPI(repoName, commitSHA, tag string) (*sc.ScorecardResult, error) {
 	logger := logging.FromContext(s.ctx)
 
-	// The Scorecard API only supports commit SHAs, not tags.
-	// If a tag is provided without a commitSHA, we cannot use the API
-	// and must fall back to local computation to avoid returning incorrect results.
+	// If tag is provided without a valid commitSHA, skip API and use local computation
+	// The API cannot resolve tags, but computeScore can look up the commit for a tag
 	if (commitSHA == "" || commitSHA == "HEAD") && tag != "" {
-		logger.Debugf("Cannot use API for tag %s without commit SHA - will fall back to local computation", tag)
-		return nil, fmt.Errorf("scorecard API does not support tags; commit SHA required for tag %s", tag)
+		logger.Debugf("Tag %s provided without commit SHA - skipping API, will use local computation", tag)
+		return nil, fmt.Errorf("tag provided without commit SHA; falling back to local computation for tag %s", tag)
 	}
 
-	url, err := url.JoinPath("https://api.securityscorecards.dev", "projects", repoName)
+	baseURL, err := url.JoinPath("https://api.securityscorecards.dev", "projects", repoName)
 	if err != nil {
 		return nil, err
 	}
 
+	// If commitSHA is provided, try with it first
 	if commitSHA != "" && commitSHA != "HEAD" {
-		url += "?commit=" + commitSHA
+		urlWithCommit := baseURL + "?commit=" + commitSHA
+		result, err := s.fetchFromAPI(urlWithCommit)
+		if err == nil {
+			return result, nil
+		}
+		logger.Debugf("API call with commit %s failed, retrying without commit: %v", commitSHA, err)
 	}
 
-	logger.Debugf("Making API request to: %s", url)
+	result, err := s.fetchFromAPI(baseURL)
+	if err != nil {
+		return nil, err
+	}
+	return result, nil
+}
+
+func (s scorecardRunner) fetchFromAPI(apiURL string) (*sc.ScorecardResult, error) {
+	logger := logging.FromContext(s.ctx)
+	logger.Debugf("Making API request to: %s", apiURL)
 
 	httpClient := &http.Client{
 		Timeout: 30 * time.Second,
 	}
 
-	req, err := http.NewRequestWithContext(s.ctx, http.MethodGet, url, nil)
+	req, err := http.NewRequestWithContext(s.ctx, http.MethodGet, apiURL, nil)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create request: %w", err)
 	}
@@ -111,7 +137,7 @@ func (s scorecardRunner) getScoreFromAPI(repoName, commitSHA, tag string) (*sc.S
 	logger.Debugf("API response status code: %d", resp.StatusCode)
 
 	if resp.StatusCode == http.StatusNotFound {
-		return nil, fmt.Errorf("Scorecard for repo %s not found in scorecard API", repoName)
+		return nil, fmt.Errorf("scorecard not found in API")
 	}
 
 	if resp.StatusCode >= 400 {
@@ -198,4 +224,4 @@ func NewScorecardRunner(ctx context.Context) (Scorecard, error) {
 	return scorecardRunner{
 		ctx,
 	}, nil
-}
+}

pkg/certifier/scorecard/scorecardRunner_test.go

@@ -66,8 +66,8 @@ func Test_scorecardRunner_GetScore(t *testing.T) {
 	}
 }
 
-// Test_scorecardRunner_getScoreFromAPI tests the early return logic
-// for tags without commit SHAs, which doesn't require network access.
+// Test_scorecardRunner_getScoreFromAPI tests the API fetch logic with retry behavior.
+// Tests that require network access use a well-known repo (ossf/scorecard).
 
 func Test_scorecardRunner_getScoreFromAPI(t *testing.T) {
 	tests := []struct {
@@ -79,20 +79,42 @@ func Test_scorecardRunner_getScoreFromAPI(t *testing.T) {
 		errContains string
 	}{
 		{
-			name:        "tag without commit SHA returns error",
-			repoName:    "test/repo",
+			name:      "valid repo without commit returns latest scorecard",
+			repoName:  "github.com/ossf/scorecard",
+			commitSHA: "",
+			tag:       "",
+			wantErr:   false,
+		},
+		{
+			name:        "tag without commit SHA skips API for local computation",
+			repoName:    "github.com/ossf/scorecard",
 			commitSHA:   "",
-			tag:         "v1.0.0",
+			tag:         "v4.10.4",
 			wantErr:     true,
-			errContains: "scorecard API does not support tags",
+			errContains: "tag provided without commit SHA",
 		},
 		{
-			name:        "tag with HEAD commit SHA returns error",
-			repoName:    "test/repo",
+			name:        "tag with HEAD skips API for local computation",
+			repoName:    "github.com/ossf/scorecard",
 			commitSHA:   "HEAD",
-			tag:         "v1.0.0",
+			tag:         "v4.10.4",
+			wantErr:     true,
+			errContains: "tag provided without commit SHA",
+		},
+		{
+			name:        "non-existent repo returns error",
+			repoName:    "github.com/nonexistent/nonexistent-repo-12345",
+			commitSHA:   "",
+			tag:         "",
 			wantErr:     true,
-			errContains: "scorecard API does not support tags",
+			errContains: "scorecard not found in API",
+		},
+		{
+			name:      "invalid commit SHA falls back to latest scorecard",
+			repoName:  "github.com/ossf/scorecard",
+			commitSHA: "0000000000000000000000000000000000000000",
+			tag:       "",
+			wantErr:   false,
 		},
 	}
 
@@ -101,8 +123,6 @@ func Test_scorecardRunner_getScoreFromAPI(t *testing.T) {
 			ctx := context.Background()
 			runner := scorecardRunner{ctx: ctx}
 
-			// Run the actual API call - these tests only cover edge cases
-			// that return early without making network requests
 			got, err := runner.getScoreFromAPI(tt.repoName, tt.commitSHA, tt.tag)
 
 			if (err != nil) != tt.wantErr {