Improve dependency analysis & completions

- Update Dockerfile for dependency analysis
- Rename GetScore function to GetScorecardResult
- Update README title and link to GitHub Dependency Review documentation
- Add environment variable for list of checks
- Add checks for valid repo
- Filter out dependencies that are not added
- Filter out dependencies that do

[Dockerfile-dependency-analysis]
- Rename file from `Dockerfile-dependency-analysis` to `Dependency-analysis.dockerfile`
[dependency-analysis/main_test.go]
- Change the name of the GetScore function to GetScorecardResult
- Lower the minimum score required in the test from `got.Score < tt.score` to `got.Score <= tt.score`
[dependency-analysis/README.md]
- Change the title of the README from `OpenSSF Dependency Analysis` to `OpenSSF Scorecard Dependency Analysis`
- Change the link to the GitHub Dependency Review documentation
- Change the action name to `ossf/scorecard-action/dependency-analysis@main`
[dependency-analysis/main.go]
- Convert the PR number to an integer
- Move the `octokit` initialization to a separate file
- Add an environment variable to get the list of checks
- Add a check for a valid repo
- Convert the PR number to an integer
- Add a function to get the HTML for vulnerabilities
- Add a function to get the scorecard result
- Filter out dependencies that are not added
- Filter out dependencies that do
[.github/workflows/publish-dependency-image.yml]
- Change the file name for the Dockerfile from `Dockerfile-dependency-analysis` to `Dependency-analysis.dockerfile`

Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com>

.github/workflows/publish-dependency-image.yml

@@ -40,6 +40,6 @@ jobs:
         with:
           context: .
           push: true
-          file: ./Dockerfile-dependency-analysis
+          file: ./Dependency-analysis.dockerfile
           tags: ${{ steps.meta.outputs.tags }}
           labels: ${{ steps.meta.outputs.labels }}

dependency-analysis/README.md

@@ -1,4 +1,4 @@
-# OpenSSF Dependency Analysis
+# OpenSSF Scorecard Dependency Analysis
 
 This repository contains the source code for the OpenSSF Dependency Analysis project. The aim of the project is to check the security posture of a project's dependencies using the [GitHub Dependency Graph API](https://docs.github.com/en/rest/dependency-graph/dependency-review?apiVersion=2022-11-28#get-a-diff-of-the-dependencies-between-commits) and the [Security Scorecards API](https://api.securityscorecards.dev).
 
@@ -9,7 +9,7 @@ The action will run on the latest commit on the default branch of the repository
 An example of the comment can be found [here](https://github.com/ossf-tests/vulpy/pull/2#issuecomment-1442310469).
 
 ## Prerequisites
-The actions require enabling the [GitHub Dependency](https://docs.github.com/en/code-security/supply-chain-security/understanding-your-software-supply-chain/about-dependency-review) for the repository.
+The actions require enabling the [GitHub Dependency Review](https://docs.github.com/en/code-security/supply-chain-security/understanding-your-software-supply-chain/about-dependency-review) for the repository.
 
 ### Configuration
 The action can be configured using the following inputs:
@@ -54,5 +54,5 @@ jobs:
           persist-credentials: false
 
       - name: Run dependency analysis
-        uses: github.com/ossf/scorecard-action/dependency-analysis@main # Replace with the latest release version.
+        uses: ossf/scorecard-action/dependency-analysis@main # Replace with the latest release version.
 ```

dependency-analysis/main.go

@@ -39,61 +39,58 @@ func main() {
 	token := os.Getenv("GITHUB_TOKEN")
 	pr := os.Getenv("GITHUB_PR_NUMBER")
 	ghUser := os.Getenv("GITHUB_ACTOR")
+	fileName := os.Getenv("SCORECARD_CHECKS")
 	if err := Validate(token, repo, commitSHA, pr); err != nil {
 		log.Fatal(err)
 	}
+	// convert pr to int
+	prInt, err := strconv.Atoi(pr)
+	if err != nil {
+		log.Fatal(err)
+	}
 
 	ownerRepo := strings.Split(repo, "/")
+	if len(ownerRepo) != 2 {
+		log.Fatal("invalid repo")
+	}
 	owner := ownerRepo[0]
 	repo = ownerRepo[1]
-	checks, err := GetScorecardChecks()
+	checks, err := GetScorecardChecks(fileName)
 	if err != nil {
 		log.Fatal(err)
 	}
 
-	defaultBranch, err := getDefaultBranch(owner, repo, token)
+	ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token})
+	tc := oauth2.NewClient(context.Background(), ts)
+	client := github.NewClient(tc)
+	defaultBranch, err := getDefaultBranch(owner, repo, client)
 	if err != nil {
 		log.Fatal(err)
 	}
 
-	ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token})
-	tc := oauth2.NewClient(context.Background(), ts)
-	client := github.NewClient(tc)
-	data, err := GetDependencyDiff(owner, repo, token, defaultBranch, commitSHA)
+	diff, err := GetDependencyDiff(owner, repo, token, defaultBranch, commitSHA)
 	if err != nil {
 		log.Fatal(err)
 	}
 
 	m := make(map[string]DependencyDiff)
-	for _, dep := range *data { //nolint:gocritic
+	for _, dep := range diff { //nolint:gocritic
 		m[dep.SourceRepositoryURL] = dep
 	}
 
 	for k, i := range m { //nolint:gocritic
 		url := strings.TrimPrefix(k, "https://")
-		scorecard, err := GetScore(url)
-		if err != nil && len(i.Vulnerabilities) > 0 {
-			sb := strings.Builder{}
-			sb.WriteString(fmt.Sprintf("<details><summary>Vulnerabilties %s</summary>\n </br>", i.SourceRepositoryURL))
-			sb.WriteString("<table>\n")
-			sb.WriteString("<tr>\n")
-			sb.WriteString("<th>Severity</th>\n")
-			sb.WriteString("<th>AdvisoryGHSAId</th>\n")
-			sb.WriteString("<th>AdvisorySummary</th>\n")
-			sb.WriteString("<th>AdvisoryUrl</th>\n")
-			sb.WriteString("</tr>\n")
-			for _, v := range i.Vulnerabilities {
-				sb.WriteString("<tr>\n")
-				sb.WriteString(fmt.Sprintf("<td>%s</td>\n", v.Severity))
-				sb.WriteString(fmt.Sprintf("<td>%s</td>\n", v.AdvisoryGHSAId))
-				sb.WriteString(fmt.Sprintf("<td>%s</td>\n", v.AdvisorySummary))
-				sb.WriteString(fmt.Sprintf("<td>%s</td>\n", v.AdvisoryURL))
+		scorecard, err := GetScorecardResult(url)
+		if err != nil {
+			if len(i.Vulnerabilities) > 0 {
+				vulnerabilities = GetVulnsHTML(i, vulnerabilities)
+				continue
 			}
-			sb.WriteString("</table>\n")
-			sb.WriteString("</details>\n")
-			vulnerabilities += sb.String()
 			continue
 		}
+		if len(i.Vulnerabilities) > 0 {
+			vulnerabilities = GetVulnsHTML(i, vulnerabilities)
+		}
 		scorecard.Checks = filter(scorecard.Checks, func(check Check) bool {
 			for _, c := range checks {
 				if check.Name == c {
@@ -105,11 +102,6 @@ func main() {
 		scorecard.Vulnerabilities = i.Vulnerabilities
 		result += GitHubIssueComment(&scorecard)
 	}
-	// convert pr to int
-	prInt, err := strconv.Atoi(pr)
-	if err != nil {
-		log.Fatal(err)
-	}
 	// create or update comment
 	if vulnerabilities == "" && result == "" {
 		return
@@ -124,15 +116,33 @@ func main() {
 	}
 }
 
+// GetVulnsHTML returns the vulnerabilities in HTML format.
+func GetVulnsHTML(i DependencyDiff, vulnerabilities string) string { //nolint:gocritic
+	sb := strings.Builder{}
+	sb.WriteString(fmt.Sprintf("<details><summary>Vulnerabilties %s</summary>\n </br>", i.SourceRepositoryURL))
+	sb.WriteString("<table>\n")
+	sb.WriteString("<tr>\n")
+	sb.WriteString("<th>Severity</th>\n")
+	sb.WriteString("<th>AdvisoryGHSAId</th>\n")
+	sb.WriteString("<th>AdvisorySummary</th>\n")
+	sb.WriteString("<th>AdvisoryUrl</th>\n")
+	sb.WriteString("</tr>\n")
+	for _, v := range i.Vulnerabilities {
+		sb.WriteString("<tr>\n")
+		sb.WriteString(fmt.Sprintf("<td>%s</td>\n", v.Severity))
+		sb.WriteString(fmt.Sprintf("<td>%s</td>\n", v.AdvisoryGHSAId))
+		sb.WriteString(fmt.Sprintf("<td>%s</td>\n", v.AdvisorySummary))
+		sb.WriteString(fmt.Sprintf("<td>%s</td>\n", v.AdvisoryURL))
+	}
+	sb.WriteString("</table>\n")
+	sb.WriteString("</details>\n")
+	vulnerabilities += sb.String()
+	return vulnerabilities
+}
+
 // getDefaultBranch gets the default branch of the repository.
-func getDefaultBranch(owner, repo, token string) (string, error) {
+func getDefaultBranch(owner, repo string, client *github.Client) (string, error) {
 	ctx := context.Background()
-	ts := oauth2.StaticTokenSource(
-		&oauth2.Token{AccessToken: token},
-	)
-	tc := oauth2.NewClient(ctx, ts)
-	client := github.NewClient(tc)
-
 	repository, _, err := client.Repositories.Get(ctx, owner, repo)
 	if err != nil {
 		return "", fmt.Errorf("failed to get repository: %w", err)
@@ -229,44 +239,43 @@ func GitHubIssueComment(checks *ScorecardResult) string {
 
 // GetDependencyDiff returns the dependency diff between two commits.
 // It returns an error if the dependency graph is not enabled.
-func GetDependencyDiff(owner, repo, token, base, head string) (*[]DependencyDiff, error) {
+func GetDependencyDiff(owner, repo, token, base, head string) ([]DependencyDiff, error) {
+	var data []DependencyDiff
 	message := "failed to get dependency diff, please enable dependency graph https://docs.github.com/en/code-security/supply-chain-security/understanding-your-software-supply-chain/configuring-the-dependency-graph" //nolint:lll
 	if owner == "" {
-		return nil, fmt.Errorf("owner is required") //nolint:goerr113
+		return data, fmt.Errorf("owner is required") //nolint:goerr113
 	}
 	if repo == "" {
-		return nil, fmt.Errorf("repo is required") //nolint:goerr113
+		return data, fmt.Errorf("repo is required") //nolint:goerr113
 	}
 	if token == "" {
-		return nil, fmt.Errorf("token is required") //nolint:goerr113
+		return data, fmt.Errorf("token is required") //nolint:goerr113
 	}
 	resp, err := GetGitHubDependencyDiff(owner, repo, token, base, head)
 	defer resp.Body.Close() //nolint:staticcheck
 
 	if resp.StatusCode != http.StatusOK {
 		// if the dependency graph is not enabled, we can't get the dependency diff
-		return nil,
+		return data,
 			fmt.Errorf(" %s: %v", message, resp.Status) //nolint:goerr113
 	}
 	if err != nil {
-		return nil, fmt.Errorf("failed to get dependency diff: %w", err)
+		return data, fmt.Errorf("failed to get dependency diff: %w", err)
 	}
 
-	var data []DependencyDiff
 	err = json.NewDecoder(resp.Body).Decode(&data)
 	if err != nil {
-		return nil, fmt.Errorf("failed to decode dependency diff: %w", err)
+		return data, fmt.Errorf("failed to decode dependency diff: %w", err)
 	}
 	// filter out the dependencies that are not added
 	var filteredData []DependencyDiff
 	for _, dep := range data { //nolint:gocritic
 		// also if the source repo doesn't start with GitHub.com, we can ignore it
-		if dep.ChangeType == "added" && dep.SourceRepositoryURL != "" &&
-			strings.HasPrefix(dep.SourceRepositoryURL, "https://github.com") {
+		if dep.ChangeType == "added" && strings.HasPrefix(dep.SourceRepositoryURL, "https://github.com") {
 			filteredData = append(filteredData, dep)
 		}
 	}
-	return &filteredData, nil
+	return filteredData, nil
 }
 
 // GetGitHubDependencyDiff returns the dependency diff between two commits.
@@ -301,8 +310,7 @@ func filter[T any](slice []T, f func(T) bool) []T {
 
 // GetScorecardChecks returns the list of checks to run.
 // This uses the SCORECARD_CHECKS environment variable to get the path to the checks list.
-func GetScorecardChecks() ([]string, error) {
-	fileName := os.Getenv("SCORECARD_CHECKS")
+func GetScorecardChecks(fileName string) ([]string, error) {
 	if fileName == "" {
 		// default to critical and high severity checks
 		return []string{
@@ -324,8 +332,8 @@ func GetScorecardChecks() ([]string, error) {
 	return checksFromFile, nil
 }
 
-// GetScore returns the scorecard result for a given repository.
-func GetScore(repo string) (ScorecardResult, error) {
+// GetScorecardResult returns the scorecard result for a given repository.
+func GetScorecardResult(repo string) (ScorecardResult, error) {
 	req, err := http.NewRequest("GET", fmt.Sprintf("https://api.securityscorecards.dev/projects/%s", repo), nil)
 	if err != nil {
 		return ScorecardResult{}, fmt.Errorf("failed to create request: %w", err)

dependency-analysis/main_test.go

@@ -76,21 +76,20 @@ func TestGetScorecardChecks(t *testing.T) { //nolint:paralleltest
 	}
 	for _, tt := range tests { //nolint:paralleltest
 		t.Run(tt.name, func(t *testing.T) {
+			dir, err := os.MkdirTemp("", "scorecard-checks")
+			defer os.RemoveAll(dir)
+			if err != nil {
+				t.Errorf("GetScorecardChecks() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
 			if tt.fileContent != "" {
-				dir, err := os.MkdirTemp("", "scorecard-checks")
-				if err != nil {
-					t.Errorf("GetScorecardChecks() error = %v, wantErr %v", err, tt.wantErr)
-					return
-				}
-				defer os.RemoveAll(dir)
-
 				if err := os.WriteFile(path.Join(dir, "scorecard.txt"), []byte(tt.fileContent), 0o644); err != nil { //nolint:gosec
 					t.Errorf("GetScorecardChecks() error = %v, wantErr %v", err, tt.wantErr)
 					return
 				}
 				t.Setenv("SCORECARD_CHECKS", path.Join(dir, "scorecard.txt"))
 			}
-			got, err := GetScorecardChecks()
+			got, err := GetScorecardChecks(path.Join(dir, "scorecard.txt"))
 			if (err != nil) != tt.wantErr {
 				t.Errorf("GetScorecardChecks() error = %v, wantErr %v", err, tt.wantErr)
 				return
@@ -132,13 +131,13 @@ func TestGetScore(t *testing.T) { //nolint:paralleltest
 
 	for _, tt := range tests { //nolint:paralleltest
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := GetScore(tt.args.repo)
+			got, err := GetScorecardResult(tt.args.repo)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("GetScore() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("GetScorecardResult() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
 			if got.Score < tt.score {
-				t.Errorf("GetScore() got = %v, want %v", got, tt.score)
+				t.Errorf("GetScorecardResult() got = %v, want %v", got, tt.score)
 			}
 		})
 	}