Frogbot improvements (#5)

* Add support for xrUrl and rtUrl

* Add missing trailing slash to urls

* Update frogbot.yml

* Add missing error handling

Author: talarian1

Diff for: .github/workflows/frogbot.yml

@@ -23,11 +23,14 @@ jobs:
       - name: Scan Pull Request
         env:
           FROGBOT_JF_USER: ${{ secrets.FROGBOT_JF_USER }}
-          FROGBOT_JF_URL: ${{ secrets.FROGBOT_JF_URL }}
+          FROGBOT_JF_URL: ${{ secrets.FROGBOT_JF_XRAY_URL }}
+          FROGBOT_JF_XRAY_URL: ${{ secrets.FROGBOT_JF_URL }}
+          FROGBOT_JF_ARTIFACTORY_URL: ${{ secrets.FROGBOT_JF_ARTIFACTORY_URL }}
           FROGBOT_JF_PASSWORD: ${{ secrets.FROGBOT_JF_PASSWORD }}
-          FROGBOT_GIT_OWNER: $GITHUB_REPOSITORY_OWNER
-          FROGBOT_GIT_REPO: ${{ github.repository }}
-          FROGBOT_GIT_TOKEN: ${{secrets.GITHUB_TOKEN}}
-          FROGBOT_GIT_BASE_BRANCH: $GITHUB_BASE_REF
+          FROGBOT_GIT_OWNER: ${{ github.repository_owner }}
+          FROGBOT_GIT_REPO: ${{ github.event.repository.name }}
+          FROGBOT_GIT_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          FROGBOT_GIT_BASE_BRANCH: ${{ github.base_ref}}
           FROGBOT_PR: ${{ github.event.number }}
+          JFROG_CLI_LOG_LEVEL: DEBUG
         run: ./frogbot scan-pull-request

Diff for: README.md

@@ -9,15 +9,16 @@ Automated dependencies scanning using JFrog Xray.
 
 ## Usage
 
-- [Overview](#overview)
-- [Usage](#usage)
+- [FrogBot](#frogbot)
+  - [Project Status](#project-status)
+  - [Usage](#usage)
   - [Using Frogbot with GitHub Actions](#using-frogbot-with-github-actions)
   - [Using Frogbot with GitLab CI](#using-frogbot-with-gitlab-ci)
   - [Using Frogbot with Jenkins](#using-frogbot-with-jenkins)
   - [Download Frogbot through Artifactory](#download-frogbot-through-artifactory)
 - [Building and Testing the Sources](#building-and-testing-the-sources)
   - [Build Frogbot](#build-frogbot)
-  - [Tests](#test)
+  - [Tests](#tests)
 - [Code Contributions](#code-contributions)
 - [Release Notes](#release-notes)
 

Diff for: main.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"path"
 	"strconv"
 
 	"github.com/jfrog/frogbot/icons"
@@ -23,15 +24,17 @@ import (
 const (
 	frogbotVersion = "0.0.0"
 	// Env
-	jfrogUser     = "FROGBOT_JF_USER"
-	jfrogUrl      = "FROGBOT_JF_URL"
-	jfrogPassword = "FROGBOT_JF_PASSWORD"
-	jfrogToken    = "FROGBOT_JF_TOKEN"
-	gitRepoOwner  = "FROGBOT_GIT_OWNER"
-	gitRepo       = "FROGBOT_GIT_REPO"
-	gitToken      = "FROGBOT_GIT_TOKEN"
-	gitBaseBranch = "FROGBOT_GIT_BASE_BRANCH"
-	prID          = "FROGBOT_PR"
+	jfrogUser           = "FROGBOT_JF_USER"
+	jfrogUrl            = "FROGBOT_JF_URL"
+	jfrogXrayUrl        = "FROGBOT_JF_XRAY_URL"
+	jfrogArtifactoryUrl = "FROGBOT_JF_ARTIFACTORY_URL"
+	jfrogPassword       = "FROGBOT_JF_PASSWORD"
+	jfrogToken          = "FROGBOT_JF_TOKEN"
+	gitRepoOwner        = "FROGBOT_GIT_OWNER"
+	gitRepo             = "FROGBOT_GIT_REPO"
+	gitToken            = "FROGBOT_GIT_TOKEN"
+	gitBaseBranch       = "FROGBOT_GIT_BASE_BRANCH"
+	prID                = "FROGBOT_PR"
 )
 
 func main() {
@@ -79,61 +82,82 @@ func scanPullRequest(c *clitool.Context) error {
 
 	// Audit PR code
 	// TODO - fill contex according to env/flags
-	xrayScanParams := services.XrayGraphScanParams{}
+	xrayScanParams := services.XrayGraphScanParams{IncludeVulnerabilities: true}
 	wd, err := os.Getwd()
 	if err != nil {
 		return err
 	}
+	clientLog.Info("Auditing " + wd)
 	currentScan, err := runAudit(xrayScanParams, &server, wd)
+	if err != nil {
+		return err
+	}
 	// Audit target code
+	clientLog.Info("Auditing " + repo + " " + baseBranch)
 	previousScan, err := auditTarget(client, xrayScanParams, &server, repoOwner, repo, baseBranch)
 	if err != nil {
 		return err
 	}
 	// Get only the new issues added by this PR
-	violations := getNewViolations(previousScan[0], currentScan[0]) // TODO - handle array of scan results!
+	var vulnerabilitiesRows []xrayutils.VulnerabilityRow
+	// TODO - handle array of scan results!
+	if len(currentScan[0].Violations) > 0 {
+		vulnerabilitiesRows = getNewViolations(previousScan[0], currentScan[0])
+	} else if len(currentScan[0].Vulnerabilities) > 0 {
+		vulnerabilitiesRows = getNewVulnerabilities(previousScan[0], currentScan[0])
+	}
 	// Comment frogbot message on the PR
-	message := createPullRequestMessage(violations)
+	message := createPullRequestMessage(vulnerabilitiesRows)
 	return client.AddPullRequestComment(context.Background(), repoOwner, repo, message, pullRequestID)
 
 }
 
 func extractParamsFromEnv() (server coreconfig.ServerDetails, repoOwner, token, repo, baseBranch string, pullRequestID int, err error) {
-	url, exists := os.LookupEnv(jfrogUrl)
-	if !exists {
-		err = fmt.Errorf("%s is missing", jfrogUrl)
-		return
+	url := os.Getenv(jfrogUrl)
+	xrUrl := os.Getenv(jfrogXrayUrl)
+	rtUrl := os.Getenv(jfrogArtifactoryUrl)
+	if xrUrl != "" && rtUrl != "" {
+		server.XrayUrl = xrUrl
+		server.ArtifactoryUrl = rtUrl
+	} else {
+		if url == "" {
+			err = fmt.Errorf("%s or %s and %s are missing", url, xrUrl, rtUrl)
+			return
+		}
+		server.Url = url
+		server.XrayUrl = url + "/xray/"
+		server.ArtifactoryUrl = path.Join(url, "artifactory") + "/"
 	}
-	server.Url = url
-	password, passwordExists := os.LookupEnv(jfrogPassword)
-	user, userExists := os.LookupEnv(jfrogUser)
-	if passwordExists && userExists {
+
+	password := os.Getenv(jfrogPassword)
+	user := os.Getenv(jfrogUser)
+	if password != "" && user != "" {
 		server.User = user
 		server.Password = password
-	} else if accessToken, exists := os.LookupEnv(jfrogToken); exists {
+	} else if accessToken := os.Getenv(jfrogToken); accessToken != "" {
 		server.AccessToken = accessToken
 	} else {
 		err = fmt.Errorf("%s and %s or %s are missing", jfrogUser, jfrogPassword, jfrogToken)
 		return
 	}
-	if repoOwner, exists = os.LookupEnv(gitRepoOwner); !exists {
+	if repoOwner = os.Getenv(gitRepoOwner); repoOwner == "" {
 		err = fmt.Errorf("%s is missing", gitRepoOwner)
 		return
 	}
-	if repo, exists = os.LookupEnv(gitRepo); !exists {
+	if repo = os.Getenv(gitRepo); repo == "" {
 		err = fmt.Errorf("%s is missing", gitRepo)
 		return
 	}
-	if token, exists = os.LookupEnv(gitToken); !exists {
+	if token = os.Getenv(gitToken); token == "" {
 		err = fmt.Errorf("%s is missing", gitToken)
 		return
 	}
-	if baseBranch, exists = os.LookupEnv(gitBaseBranch); !exists {
+	if baseBranch = os.Getenv(gitBaseBranch); baseBranch == "" {
 		err = fmt.Errorf("%s is missing", gitBaseBranch)
 		return
 	}
-	pullRequestIDString, exists := os.LookupEnv(prID)
-	if !exists {
+	pullRequestIDString := os.Getenv(prID)
+	if pullRequestIDString == "" {
 		err = fmt.Errorf("%s is missing", prID)
 		return
 	}
@@ -169,8 +193,14 @@ func auditTarget(client vcsclient.VcsClient, xrayScanParams services.XrayGraphSc
 	if err != nil {
 		return
 	}
+	clientLog.Debug("Created temp working directory: " + tempWorkdir)
 	defer fileutils.RemoveTempDir(tempWorkdir)
+	clientLog.Debug(fmt.Sprintf("Downloading %s/%s , branch:%s to:%s", owner, repo, branch, tempWorkdir))
 	err = client.DownloadRepository(context.Background(), owner, repo, branch, tempWorkdir)
+	if err != nil {
+		return
+	}
+	clientLog.Debug("Downloaded target repository")
 	return runAudit(xrayScanParams, server, tempWorkdir)
 }
 
@@ -218,22 +248,19 @@ func getNewVulnerabilities(previousScan, currentScan services.ScanResponse) (new
 }
 
 func GetUniqueID(vulnerability xrayutils.VulnerabilityRow) string {
-	return vulnerability.IssueId + vulnerability.Components[0].Name
+	return vulnerability.ImpactedPackageName + vulnerability.ImpactedPackageVersion + vulnerability.IssueId
 
 }
 
 func createPullRequestMessage(vulnerabilitiesRows []xrayutils.VulnerabilityRow) string {
 	if len(vulnerabilitiesRows) == 0 {
 		return icons.GetIconTag(icons.NoVulnerabilityBannerSource)
 	}
-	tableHeder := `| SEVERITY | IMPACTED PACKAGE | IMPACTED PACKAGE  VERSION | FIXED VERSIONS | COMPONENT | COMPONENT VERSION | CVE 
-	:--: | -- | -- | -- | -- | :--: | --`
-	tableContent := `
-
-	
-	`
+	tableHeder := "\n| SEVERITY | IMPACTED PACKAGE | IMPACTED PACKAGE  VERSION | FIXED VERSIONS | COMPONENT | COMPONENT VERSION | CVE\n" +
+		":--: | -- | -- | -- | -- | :--: | --"
+	var tableContent string
 	for _, vulnerability := range vulnerabilitiesRows {
-		tableContent += fmt.Sprintf("| %s | %s | %s | %s | %s | %s | %s \n", icons.GetIconTag(icons.GetIconSource(vulnerability.Severity)), vulnerability.ImpactedPackageName,
+		tableContent += fmt.Sprintf("\n| %s | %s | %s | %s | %s | %s | %s ", icons.GetIconTag(icons.GetIconSource(vulnerability.Severity)), vulnerability.ImpactedPackageName,
 			vulnerability.ImpactedPackageVersion, vulnerability.FixedVersions, vulnerability.Components[0].Name, vulnerability.Components[0].Version, vulnerability.Cves[0].Id)
 	}
 	return icons.GetIconTag(icons.VulnerabilitiesBannerSource) + tableHeder + tableContent