Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send detected secrets by email #438

Merged
merged 24 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 48 additions & 24 deletions commands/scanpullrequest/scanpullrequest.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func verifyGitHubFrogbotEnvironment(client vcsclient.VcsClient, repoConfig *util
// 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) error {
func scanPullRequest(repo *utils.Repository, client vcsclient.VcsClient) (err 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,
Expand All @@ -91,33 +91,44 @@ func scanPullRequest(repo *utils.Repository, client vcsclient.VcsClient) error {
log.Info("-----------------------------------------------------------")

// Audit PR code
vulnerabilitiesRows, iacRows, err := auditPullRequest(repo, client, pullRequestDetails)
vulnerabilitiesRows, iacRows, secretsRows, err := auditPullRequest(repo, client, pullRequestDetails)
if err != nil {
return err
return
}

if len(secretsRows) > 0 && repo.SmtpServer != "" {
omerzi marked this conversation as resolved.
Show resolved Hide resolved
prSourceDetails := repo.PullRequestDetails.Source
omerzi marked this conversation as resolved.
Show resolved Hide resolved
secretsEmailDetails := utils.NewSecretsEmailDetails(client, repo.GitProvider,
prSourceDetails.Owner, prSourceDetails.Repository, prSourceDetails.Name, repo.PullRequestDetails.URL,
secretsRows, repo.EmailDetails)
if err = utils.AlertSecretsExposed(secretsEmailDetails); err != nil {
return
}
}

// Delete previous Frogbot pull request message if exists
if err = deleteExistingPullRequestComment(repo, client); err != nil {
return err
return
}

// Create a pull request message
message := createPullRequestMessage(vulnerabilitiesRows, iacRows, repo.OutputWriter)

// Add comment to the pull request
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())
err = errors.New("couldn't add pull request comment: " + err.Error())
return
}

// Fail the Frogbot task if a security issue is found and Frogbot isn't configured to avoid the failure.
if repo.FailOnSecurityIssues != nil && *repo.FailOnSecurityIssues && len(vulnerabilitiesRows) > 0 {
err = errors.New(securityIssueFoundErr)
}
return err
return
}

// Downloads Pull Requests branches code and audits them
func auditPullRequest(repoConfig *utils.Repository, client vcsclient.VcsClient, pullRequestDetails vcsclient.PullRequestInfo) (vulnerabilitiesRows []formats.VulnerabilityOrViolationRow, iacRows []formats.IacSecretsRow, err error) {
func auditPullRequest(repoConfig *utils.Repository, client vcsclient.VcsClient, pullRequestDetails vcsclient.PullRequestInfo) (vulnerabilitiesRows []formats.VulnerabilityOrViolationRow, iacRows []formats.IacSecretsRow, secretsRows []formats.IacSecretsRow, err error) {
// Download source branch
sourceBranchInfo := pullRequestDetails.Source
sourceBranchWd, cleanupSource, err := utils.DownloadRepoToTempDir(client, sourceBranchInfo.Owner, sourceBranchInfo.Repository, sourceBranchInfo.Name)
Expand Down Expand Up @@ -157,8 +168,8 @@ func auditPullRequest(repoConfig *utils.Repository, client vcsclient.VcsClient,
}

// Set JAS output flags
extendedScanResults := sourceResults.ExtendedScanResults
repoConfig.OutputWriter.SetJasOutputFlags(extendedScanResults.EntitledForJas, len(extendedScanResults.ApplicabilityScanResults) > 0)
sourceScanResults := sourceResults.ExtendedScanResults
repoConfig.OutputWriter.SetJasOutputFlags(sourceScanResults.EntitledForJas, len(sourceScanResults.ApplicabilityScanResults) > 0)

// Get all issues that were found in the source branch
if repoConfig.IncludeAllVulnerabilities {
Expand All @@ -169,7 +180,8 @@ func auditPullRequest(repoConfig *utils.Repository, client vcsclient.VcsClient,
return
}
vulnerabilitiesRows = append(vulnerabilitiesRows, allIssuesRows...)
iacRows = append(iacRows, xrayutils.PrepareIacs(sourceResults.ExtendedScanResults.IacScanResults)...)
iacRows = append(iacRows, xrayutils.PrepareIacs(sourceScanResults.IacScanResults)...)
secretsRows = append(secretsRows, xrayutils.PrepareSecrets(sourceScanResults.SecretsScanResults)...)
continue
}

Expand All @@ -180,31 +192,41 @@ func auditPullRequest(repoConfig *utils.Repository, client vcsclient.VcsClient,
if err != nil {
return
}
targetScanResults := targetResults.ExtendedScanResults
var newIssuesRows []formats.VulnerabilityOrViolationRow
newIssuesRows, err = createNewIssuesRows(targetResults, sourceResults)
if err != nil {
return
}
vulnerabilitiesRows = append(vulnerabilitiesRows, newIssuesRows...)
iacRows = append(iacRows, createNewIacRows(targetResults.ExtendedScanResults.IacScanResults, sourceResults.ExtendedScanResults.IacScanResults)...)
iacRows = append(iacRows, createNewIacOrSecretsRows(targetScanResults.IacScanResults, sourceScanResults.IacScanResults, false)...)
secretsRows = append(secretsRows, createNewIacOrSecretsRows(targetScanResults.SecretsScanResults, sourceScanResults.SecretsScanResults, true)...)
}
return
}

func createNewIacRows(targetIacResults, sourceIacResults []xrayutils.IacOrSecretResult) []formats.IacSecretsRow {
targetIacRows := xrayutils.PrepareIacs(targetIacResults)
sourceIacRows := xrayutils.PrepareIacs(sourceIacResults)
targetIacVulnerabilitiesKeys := datastructures.MakeSet[string]()
for _, row := range targetIacRows {
targetIacVulnerabilitiesKeys.Add(row.File + row.Text)
}
var addedIacVulnerabilities []formats.IacSecretsRow
for _, row := range sourceIacRows {
if !targetIacVulnerabilitiesKeys.Exists(row.File + row.Text) {
addedIacVulnerabilities = append(addedIacVulnerabilities, row)
func createNewIacOrSecretsRows(targetIacOrSecretsResults, sourceIacOrSecretsResults []xrayutils.IacOrSecretResult, isSecret bool) []formats.IacSecretsRow {
omerzi marked this conversation as resolved.
Show resolved Hide resolved
var targetRows []formats.IacSecretsRow
var sourceRows []formats.IacSecretsRow
if isSecret {
targetRows = xrayutils.PrepareSecrets(targetIacOrSecretsResults)
sourceRows = xrayutils.PrepareSecrets(sourceIacOrSecretsResults)
} else {
targetRows = xrayutils.PrepareIacs(targetIacOrSecretsResults)
sourceRows = xrayutils.PrepareIacs(sourceIacOrSecretsResults)
}

targetIacOrSecretsVulnerabilitiesKeys := datastructures.MakeSet[string]()
for _, row := range targetRows {
targetIacOrSecretsVulnerabilitiesKeys.Add(row.File + row.Text)
}
var addedIacOrSecretsVulnerabilities []formats.IacSecretsRow
for _, row := range sourceRows {
if !targetIacOrSecretsVulnerabilitiesKeys.Exists(row.File + row.Text) {
addedIacOrSecretsVulnerabilities = append(addedIacOrSecretsVulnerabilities, row)
}
}
return addedIacVulnerabilities
return addedIacOrSecretsVulnerabilities
}

// Create vulnerabilities rows. The rows should contain only the new issues added by this PR
Expand Down Expand Up @@ -307,7 +329,9 @@ func deleteExistingPullRequestComment(repository *utils.Repository, client vcscl
log.Debug("Looking for an existing Frogbot pull request comment. Deleting it if it exists...")
comments, err := utils.GetSortedPullRequestComments(client, repository.RepoOwner, repository.RepoName, int(repository.PullRequestDetails.ID))
if err != nil {
return err
return fmt.Errorf(
"failed to get the pull request comments. the following details were used in order to fetch the comments: <%s/%s> pull request id: %d. the error received: %s",
omerzi marked this conversation as resolved.
Show resolved Hide resolved
omerzi marked this conversation as resolved.
Show resolved Hide resolved
repository.RepoOwner, repository.RepoName, int(repository.PullRequestDetails.ID), err.Error())
}

commentID := frogbotCommentNotFound
Expand Down
97 changes: 88 additions & 9 deletions commands/scanpullrequest/scanpullrequest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -683,8 +683,7 @@ func TestCreateNewIacRows(t *testing.T) {
Severity: "High",
File: "file1",
LineColumn: "1:10",
Type: "Secret",
Text: "Sensitive information",
Text: "aws violation",
},
},
sourceIacResults: []utils2.IacOrSecretResult{},
Expand All @@ -694,6 +693,71 @@ func TestCreateNewIacRows(t *testing.T) {
name: "No vulnerabilities in target IaC results",
targetIacResults: []utils2.IacOrSecretResult{},
sourceIacResults: []utils2.IacOrSecretResult{
{
Severity: "High",
File: "file1",
LineColumn: "1:10",
Text: "aws violation",
},
},
expectedAddedIacVulnerabilities: []formats.IacSecretsRow{
{
Severity: "High",
File: "file1",
LineColumn: "1:10",
Text: "aws violation",
SeverityNumValue: 10,
},
},
},
{
name: "Some new vulnerabilities in source IaC results",
targetIacResults: []utils2.IacOrSecretResult{
{
Severity: "High",
File: "file1",
LineColumn: "1:10",
Text: "aws violation",
},
},
sourceIacResults: []utils2.IacOrSecretResult{
{
Severity: "Medium",
File: "file2",
LineColumn: "2:5",
Text: "gcp violation",
},
},
expectedAddedIacVulnerabilities: []formats.IacSecretsRow{
{
Severity: "Medium",
SeverityNumValue: 8,
File: "file2",
LineColumn: "2:5",
Text: "gcp violation",
},
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
addedIacVulnerabilities := createNewIacOrSecretsRows(tc.targetIacResults, tc.sourceIacResults, false)
assert.ElementsMatch(t, tc.expectedAddedIacVulnerabilities, addedIacVulnerabilities)
})
}
}

func TestCreateNewSecretRows(t *testing.T) {
testCases := []struct {
name string
targetSecretsResults []utils2.IacOrSecretResult
sourceSecretsResults []utils2.IacOrSecretResult
expectedAddedSecretsVulnerabilities []formats.IacSecretsRow
}{
{
name: "No vulnerabilities in source secrets results",
targetSecretsResults: []utils2.IacOrSecretResult{
{
Severity: "High",
File: "file1",
Expand All @@ -702,7 +766,22 @@ func TestCreateNewIacRows(t *testing.T) {
Text: "Sensitive information",
},
},
expectedAddedIacVulnerabilities: []formats.IacSecretsRow{
sourceSecretsResults: []utils2.IacOrSecretResult{},
expectedAddedSecretsVulnerabilities: []formats.IacSecretsRow{},
},
{
name: "No vulnerabilities in target secrets results",
targetSecretsResults: []utils2.IacOrSecretResult{},
sourceSecretsResults: []utils2.IacOrSecretResult{
{
Severity: "High",
File: "file1",
LineColumn: "1:10",
Type: "Secret",
Text: "Sensitive information",
},
},
expectedAddedSecretsVulnerabilities: []formats.IacSecretsRow{
{
Severity: "High",
File: "file1",
Expand All @@ -714,8 +793,8 @@ func TestCreateNewIacRows(t *testing.T) {
},
},
{
name: "Some new vulnerabilities in source IaC results",
targetIacResults: []utils2.IacOrSecretResult{
name: "Some new vulnerabilities in source secrets results",
targetSecretsResults: []utils2.IacOrSecretResult{
{
Severity: "High",
File: "file1",
Expand All @@ -724,7 +803,7 @@ func TestCreateNewIacRows(t *testing.T) {
Text: "Sensitive information",
},
},
sourceIacResults: []utils2.IacOrSecretResult{
sourceSecretsResults: []utils2.IacOrSecretResult{
{
Severity: "Medium",
File: "file2",
Expand All @@ -733,7 +812,7 @@ func TestCreateNewIacRows(t *testing.T) {
Text: "Confidential data",
},
},
expectedAddedIacVulnerabilities: []formats.IacSecretsRow{
expectedAddedSecretsVulnerabilities: []formats.IacSecretsRow{
{
Severity: "Medium",
SeverityNumValue: 8,
Expand All @@ -748,8 +827,8 @@ func TestCreateNewIacRows(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
addedIacVulnerabilities := createNewIacRows(tc.targetIacResults, tc.sourceIacResults)
assert.ElementsMatch(t, tc.expectedAddedIacVulnerabilities, addedIacVulnerabilities)
addedIacVulnerabilities := createNewIacOrSecretsRows(tc.targetSecretsResults, tc.sourceSecretsResults, true)
assert.ElementsMatch(t, tc.expectedAddedSecretsVulnerabilities, addedIacVulnerabilities)
})
}
}
Expand Down
9 changes: 8 additions & 1 deletion commands/utils/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ const (
FixableOnlyEnv = "JF_FIXABLE_ONLY"
WatchesDelimiter = ","

// Email related environment variables
//#nosec G101 -- False positive - no hardcoded credentials.
SmtpAuthPassEnv = "JF_SMTP_PASS"
omerzi marked this conversation as resolved.
Show resolved Hide resolved
SmtpAuthUserEnv = "JF_SMTP_USER"
SmtpServerEnv = "JF_SMTP_SERVER"
EmailReceiversEnv = "JF_EMAIL_RECEIVERS"

//#nosec G101 -- False positive - no hardcoded credentials.
GitTokenEnv = "JF_GIT_TOKEN"
GitBaseBranchEnv = "JF_GIT_BASE_BRANCH"
Expand All @@ -75,7 +82,7 @@ const (
BranchNameTemplate = "frogbot-" + PackagePlaceHolder + "-" + BranchHashPlaceHolder
AggregatedBranchNameTemplate = "frogbot-update-" + BranchHashPlaceHolder + "-dependencies"
CommitMessageTemplate = "Upgrade " + PackagePlaceHolder + " to " + FixVersionPlaceHolder
PullRequestTitleTemplate = outputwriter.FrogbotPullRequestTitlePrefix + " Update version of " + PackagePlaceHolder + " to " + FixVersionPlaceHolder
PullRequestTitleTemplate = outputwriter.FrogbotTitlePrefix + " Update version of " + PackagePlaceHolder + " to " + FixVersionPlaceHolder
// Frogbot Git author details showed in commits
frogbotAuthorName = "JFrog-Frogbot"
frogbotAuthorEmail = "[email protected]"
Expand Down
Loading
Loading