Merge branch 'dev' into frogbot-golang.org/x/net-78a6fe11f8021e24b7f1…

…96d7f85790d7

commands/scanpullrequest/scanallpullrequests.go

@@ -47,8 +47,8 @@ func scanAllPullRequests(repo utils.Repository, client vcsclient.VcsClient) (err
 			log.Info("Pull Request", pr.ID, "has already been scanned before. If you wish to scan it again, please comment \"rescan\".")
 			return
 		}
-		spr := &ScanPullRequestCmd{pullRequestDetails: pr}
-		if e = spr.Run(utils.RepoAggregator{repo}, client); e != nil {
+		repo.PullRequestDetails = pr
+		if e = scanPullRequest(&repo, client); e != nil {
 			// If error, write it in errList and continue to the next PR.
 			err = errors.Join(err, fmt.Errorf(errPullRequestScan, int(pr.ID), repo.RepoName, e.Error()))
 		}

commands/scanpullrequest/scanpullrequest.go

@@ -24,10 +24,7 @@ const (
 	frogbotCommentNotFound  = -1
 )
 
-type ScanPullRequestCmd struct {
-	// Optional provided pull request details, used in scan-all-pull-requests command.
-	pullRequestDetails vcsclient.PullRequestInfo
-}
+type ScanPullRequestCmd struct{}
 
 // Run ScanPullRequest method only works for a single repository scan.
 // Therefore, the first repository config represents the repository on which Frogbot runs, and it is the only one that matters.
@@ -42,21 +39,51 @@ func (cmd *ScanPullRequestCmd) Run(configAggregator utils.RepoAggregator, client
 		}
 	}
 
-	// PullRequestDetails can be defined already when using the scan-all-pull-requests command.
-	if cmd.pullRequestDetails.ID == utils.UndefinedPrID {
-		if cmd.pullRequestDetails, err = client.GetPullRequestByID(context.Background(), repoConfig.RepoOwner, repoConfig.RepoName, repoConfig.PullRequestID); err != nil {
-			return
-		}
+	if repoConfig.PullRequestDetails, err = client.GetPullRequestByID(context.Background(), repoConfig.RepoOwner, repoConfig.RepoName, int(repoConfig.PullRequestDetails.ID)); err != nil {
+		return
+	}
+
+	return scanPullRequest(repoConfig, client)
+}
+
+// Verify that the 'frogbot' GitHub environment was properly configured on the repository
+func verifyGitHubFrogbotEnvironment(client vcsclient.VcsClient, repoConfig *utils.Repository) error {
+	if repoConfig.APIEndpoint != "" && repoConfig.APIEndpoint != "https://api.github.com" {
+		// Don't verify 'frogbot' environment on GitHub on-prem
+		return nil
+	}
+	if _, exist := os.LookupEnv(utils.GitHubActionsEnv); !exist {
+		// Don't verify 'frogbot' environment on non GitHub Actions CI
+		return nil
+	}
+
+	// If the repository is not public, using 'frogbot' environment is not mandatory
+	repoInfo, err := client.GetRepositoryInfo(context.Background(), repoConfig.RepoOwner, repoConfig.RepoName)
+	if err != nil {
+		return err
+	}
+	if repoInfo.RepositoryVisibility != vcsclient.Public {
+		return nil
+	}
+
+	// Get the 'frogbot' environment info and make sure it exists and includes reviewers
+	repoEnvInfo, err := client.GetRepositoryEnvironmentInfo(context.Background(), repoConfig.RepoOwner, repoConfig.RepoName, "frogbot")
+	if err != nil {
+		return errors.New(err.Error() + "/n" + noGitHubEnvErr)
+	}
+	if len(repoEnvInfo.Reviewers) == 0 {
+		return errors.New(noGitHubEnvReviewersErr)
 	}
 
-	return scanPullRequest(repoConfig, client, cmd.pullRequestDetails)
+	return nil
 }
 
 // By default, includeAllVulnerabilities is set to false and the scan goes as follows:
 // a. Audit the dependencies of the source and the target branches.
 // b. Compare the vulnerabilities found in source and target branches, and show only the new vulnerabilities added by the pull request.
 // Otherwise, only the source branch is scanned and all found vulnerabilities are being displayed.
-func scanPullRequest(repo *utils.Repository, client vcsclient.VcsClient, pullRequestDetails vcsclient.PullRequestInfo) error {
+func scanPullRequest(repo *utils.Repository, client vcsclient.VcsClient) error {
+	pullRequestDetails := repo.PullRequestDetails
 	log.Info(fmt.Sprintf("Scanning Pull Request #%d (from source branch: <%s/%s/%s> to target branch: <%s/%s/%s>)",
 		pullRequestDetails.ID,
 		pullRequestDetails.Source.Owner, pullRequestDetails.Source.Repository, pullRequestDetails.Source.Name,
@@ -78,7 +105,7 @@ func scanPullRequest(repo *utils.Repository, client vcsclient.VcsClient, pullReq
 	message := createPullRequestMessage(vulnerabilitiesRows, iacRows, repo.OutputWriter)
 
 	// Add comment to the pull request
-	if err = client.AddPullRequestComment(context.Background(), repo.RepoOwner, repo.RepoName, message, repo.PullRequestID); err != nil {
+	if err = client.AddPullRequestComment(context.Background(), repo.RepoOwner, repo.RepoName, message, int(pullRequestDetails.ID)); err != nil {
 		return errors.New("couldn't add pull request comment: " + err.Error())
 	}
 
@@ -180,38 +207,6 @@ func createNewIacRows(targetIacResults, sourceIacResults []xrayutils.IacOrSecret
 	return addedIacVulnerabilities
 }
 
-// Verify that the 'frogbot' GitHub environment was properly configured on the repository
-func verifyGitHubFrogbotEnvironment(client vcsclient.VcsClient, repoConfig *utils.Repository) error {
-	if repoConfig.APIEndpoint != "" && repoConfig.APIEndpoint != "https://api.github.com" {
-		// Don't verify 'frogbot' environment on GitHub on-prem
-		return nil
-	}
-	if _, exist := os.LookupEnv(utils.GitHubActionsEnv); !exist {
-		// Don't verify 'frogbot' environment on non GitHub Actions CI
-		return nil
-	}
-
-	// If the repository is not public, using 'frogbot' environment is not mandatory
-	repoInfo, err := client.GetRepositoryInfo(context.Background(), repoConfig.RepoOwner, repoConfig.RepoName)
-	if err != nil {
-		return err
-	}
-	if repoInfo.RepositoryVisibility != vcsclient.Public {
-		return nil
-	}
-
-	// Get the 'frogbot' environment info and make sure it exists and includes reviewers
-	repoEnvInfo, err := client.GetRepositoryEnvironmentInfo(context.Background(), repoConfig.RepoOwner, repoConfig.RepoName, "frogbot")
-	if err != nil {
-		return errors.New(err.Error() + "/n" + noGitHubEnvErr)
-	}
-	if len(repoEnvInfo.Reviewers) == 0 {
-		return errors.New(noGitHubEnvReviewersErr)
-	}
-
-	return nil
-}
-
 // Create vulnerabilities rows. The rows should contain only the new issues added by this PR
 func createNewIssuesRows(targetResults, sourceResults *audit.Results) (vulnerabilitiesRows []formats.VulnerabilityOrViolationRow, err error) {
 	targetScanAggregatedResults := aggregateScanResults(targetResults.ExtendedScanResults.XrayResults)
@@ -310,7 +305,7 @@ func createPullRequestMessage(vulnerabilitiesRows []formats.VulnerabilityOrViola
 
 func deleteExistingPullRequestComment(repository *utils.Repository, client vcsclient.VcsClient) error {
 	log.Debug("Looking for an existing Frogbot pull request comment. Deleting it if it exists...")
-	comments, err := utils.GetSortedPullRequestComments(client, repository.RepoOwner, repository.RepoName, repository.PullRequestID)
+	comments, err := utils.GetSortedPullRequestComments(client, repository.RepoOwner, repository.RepoName, int(repository.PullRequestDetails.ID))
 	if err != nil {
 		return err
 	}
@@ -325,7 +320,7 @@ func deleteExistingPullRequestComment(repository *utils.Repository, client vcscl
 	}
 
 	if commentID != frogbotCommentNotFound {
-		err = client.DeletePullRequestComment(context.Background(), repository.RepoOwner, repository.RepoName, repository.PullRequestID, commentID)
+		err = client.DeletePullRequestComment(context.Background(), repository.RepoOwner, repository.RepoName, int(repository.PullRequestDetails.ID), commentID)
 	}
 
 	return err

commands/scanpullrequest/scanpullrequest_test.go

@@ -582,6 +582,7 @@ func prepareConfigAndClient(t *testing.T, configPath string, server *httptest.Se
 			Token:       "123456",
 			APIEndpoint: server.URL,
 		},
+		PullRequestDetails: vcsclient.PullRequestInfo{ID: int64(1)},
 	}
 	utils.SetEnvAndAssert(t, map[string]string{utils.GitPullRequestIDEnv: "1"})
 
@@ -757,9 +758,9 @@ func TestDeletePreviousPullRequestMessages(t *testing.T) {
 	repository := &utils.Repository{
 		Params: utils.Params{
 			Git: utils.Git{
-				RepoName:      "repo",
-				RepoOwner:     "owner",
-				PullRequestID: 17,
+				RepoName:           "repo",
+				RepoOwner:          "owner",
+				PullRequestDetails: vcsclient.PullRequestInfo{ID: 17},
 			},
 		},
 		OutputWriter: &outputwriter.StandardOutput{},

commands/utils/params.go

@@ -24,9 +24,6 @@ import (
 const (
 	frogbotConfigDir  = ".frogbot"
 	FrogbotConfigFile = "frogbot-config.yml"
-
-	// Pull Request ID cannot be 0
-	UndefinedPrID = 0
 )
 
 var (
@@ -197,13 +194,14 @@ type Git struct {
 	PullRequestTitleTemplate string   `yaml:"pullRequestTitleTemplate,omitempty"`
 	EmailAuthor              string   `yaml:"emailAuthor,omitempty"`
 	AggregateFixes           bool     `yaml:"aggregateFixes,omitempty"`
-	PullRequestID            int
+	PullRequestDetails       vcsclient.PullRequestInfo
 }
 
 func (g *Git) setDefaultsIfNeeded(gitParamsFromEnv *Git) (err error) {
 	g.RepoOwner = gitParamsFromEnv.RepoOwner
 	g.GitProvider = gitParamsFromEnv.GitProvider
 	g.VcsInfo = gitParamsFromEnv.VcsInfo
+	g.PullRequestDetails = gitParamsFromEnv.PullRequestDetails
 	if g.RepoName == "" {
 		if gitParamsFromEnv.RepoName == "" {
 			return fmt.Errorf("repository name is missing. please set the repository name in your %s file or as the %s environment variable", FrogbotConfigFile, GitRepoEnv)
@@ -242,15 +240,6 @@ func (g *Git) setDefaultsIfNeeded(gitParamsFromEnv *Git) (err error) {
 			g.EmailAuthor = frogbotAuthorEmail
 		}
 	}
-	if g.PullRequestID == UndefinedPrID {
-		if idStr := getTrimmedEnv(GitPullRequestIDEnv); idStr != "" {
-			var idNum int
-			if idNum, err = strconv.Atoi(idStr); err != nil {
-				return fmt.Errorf("failed parsing pull request ID as a number. ID as string : %s", idStr)
-			}
-			g.PullRequestID = idNum
-		}
-	}
 	return
 }
 
@@ -430,6 +419,13 @@ func extractGitParamsFromEnvs() (*Git, error) {
 	if err = readParamFromEnv(GitProjectEnv, &gitEnvParams.Project); err != nil && gitEnvParams.GitProvider == vcsutils.AzureRepos {
 		return nil, err
 	}
+	if envPrId := getTrimmedEnv(GitPullRequestIDEnv); envPrId != "" {
+		var convertedPrId int
+		if convertedPrId, err = strconv.Atoi(envPrId); err != nil {
+			return nil, fmt.Errorf("failed parsing %s environment variable as a number. The received environment is : %s", GitPullRequestIDEnv, envPrId)
+		}
+		gitEnvParams.PullRequestDetails = vcsclient.PullRequestInfo{ID: int64(convertedPrId)}
+	}
 
 	return gitEnvParams, nil
 }

commands/utils/params_test.go

@@ -251,7 +251,7 @@ func extractAndAssertParamsFromEnv(t *testing.T, platformUrl, basicAuth bool) {
 		assert.Equal(t, "frogbot", configParams.RepoName)
 		assert.Equal(t, "123456789", configParams.Token)
 		assert.Equal(t, "dev", configParams.Branches[0])
-		assert.Equal(t, 1, configParams.PullRequestID)
+		assert.Equal(t, int64(1), configParams.PullRequestDetails.ID)
 	}
 }
 
@@ -342,7 +342,7 @@ func TestGenerateConfigAggregatorFromEnv(t *testing.T) {
 	assert.Equal(t, gitParams.Token, repo.Token)
 	assert.Equal(t, gitParams.APIEndpoint, repo.APIEndpoint)
 	assert.ElementsMatch(t, gitParams.Branches, repo.Branches)
-	assert.Equal(t, repo.PullRequestID, repo.PullRequestID)
+	assert.Equal(t, repo.PullRequestDetails.ID, repo.PullRequestDetails.ID)
 	assert.Equal(t, gitParams.GitProvider, repo.GitProvider)
 	assert.Equal(t, repo.BranchNameTemplate, repo.BranchNameTemplate)
 	assert.Equal(t, repo.CommitMessageTemplate, repo.CommitMessageTemplate)