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

Add logs and add error if serverDetails not provided #189

Merged
merged 5 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
58 changes: 37 additions & 21 deletions commands/audit/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ package audit
import (
"errors"
"fmt"

"github.com/jfrog/gofrog/log"
jfrogappsconfig "github.com/jfrog/jfrog-apps-config/go"
"github.com/jfrog/jfrog-cli-core/v2/utils/config"
"github.com/jfrog/jfrog-cli-core/v2/utils/coreutils"
"github.com/jfrog/jfrog-cli-security/jas"
"github.com/jfrog/jfrog-cli-security/jas/applicability"
Expand Down Expand Up @@ -194,17 +195,11 @@ func RunAudit(auditParams *AuditParams) (results *utils.Results, err error) {
if err != nil {
return results, fmt.Errorf("failed to create JFrogAppsConfig: %s", err.Error())
}
jasScanner := &jas.JasScanner{}
if results.ExtendedScanResults.EntitledForJas {
// Download (if needed) the analyzer manager and run scanners.
auditParallelRunner.JasWg.Add(1)
if _, jasErr := auditParallelRunner.Runner.AddTaskWithError(func(threadId int) error {
return downloadAnalyzerManagerAndRunScanners(auditParallelRunner, results, serverDetails, auditParams, jasScanner, jfrogAppsConfig, threadId)
}, auditParallelRunner.AddErrorToChan); jasErr != nil {
auditParallelRunner.AddErrorToChan(fmt.Errorf("failed to create AM downloading task, skipping JAS scans...: %s", jasErr.Error()))
}
var jasScanner *jas.JasScanner
var jasScanErr error
if jasScanner, jasScanErr = RunJasScans(auditParallelRunner, auditParams, results, jfrogAppsConfig); jasScanErr != nil {
auditParallelRunner.AddErrorToChan(jasScanErr)
}

// The sca scan doesn't require the analyzer manager, so it can run separately from the analyzer manager download routine.
if scaScanErr := buildDepTreeAndRunScaScan(auditParallelRunner, auditParams, results); scaScanErr != nil {
auditParallelRunner.AddErrorToChan(scaScanErr)
Expand All @@ -214,9 +209,8 @@ func RunAudit(auditParams *AuditParams) (results *utils.Results, err error) {
auditParallelRunner.JasWg.Wait()
// Wait for all jas scanners to complete before cleaning up scanners temp dir
auditParallelRunner.JasScannersWg.Wait()
cleanup := jasScanner.ScannerDirCleanupFunc
if cleanup != nil {
auditParallelRunner.AddErrorToChan(cleanup())
if jasScanner != nil && jasScanner.ScannerDirCleanupFunc != nil {
auditParallelRunner.AddErrorToChan(jasScanner.ScannerDirCleanupFunc())
}
close(auditParallelRunner.ErrorsQueue)
auditParallelRunner.Runner.Done()
Expand Down Expand Up @@ -244,19 +238,41 @@ func isEntitledForJas(xrayManager *xray.XrayServicesManager, auditParams *AuditP
return jas.IsEntitledForJas(xrayManager, auditParams.xrayVersion)
}

func downloadAnalyzerManagerAndRunScanners(auditParallelRunner *utils.SecurityParallelRunner, scanResults *utils.Results,
serverDetails *config.ServerDetails, auditParams *AuditParams, scanner *jas.JasScanner, jfrogAppsConfig *jfrogappsconfig.JFrogAppsConfig, threadId int) (err error) {
func RunJasScans(auditParallelRunner *utils.SecurityParallelRunner, auditParams *AuditParams, results *utils.Results, jfrogAppsConfig *jfrogappsconfig.JFrogAppsConfig) (jasScanner *jas.JasScanner, err error) {
if !results.ExtendedScanResults.EntitledForJas {
log.Info("Not entitled for JAS, skipping advance security scans...")
return
}
serverDetails, err := auditParams.ServerDetails()
if err != nil {
err = fmt.Errorf("failed to get server details: %s", err.Error())
return
}
jasScanner, err = jas.CreateJasScanner(jfrogAppsConfig, serverDetails, jas.GetAnalyzerManagerXscEnvVars(auditParams.commonGraphScanParams.MultiScanId, results.ExtendedScanResults.SecretValidation, results.GetScaScannedTechnologies()...), auditParams.Exclusions()...)
if err != nil {
err = fmt.Errorf("failed to create jas scanner: %s", err.Error())
return
} else if jasScanner == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under which circumstances that weren't exist before the jasScanner can be nil after we tried to create it? If it is not trivial to infer - please add a comment what can result this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look at CreateScanner. if len(serverDetails.Url) == 0 is true we log the usual warning and return nil scanner, we should not continue after that just like the check was at JasRunner. we need a way to separate the case that serverDetails is nil (err, should not be nil at this point) or the user did not configure the right things (URL, warning)

log.Debug("Jas scanner was not created, skipping advance security scans...")
return
}
auditParallelRunner.JasWg.Add(1)
if _, jasErr := auditParallelRunner.Runner.AddTaskWithError(func(threadId int) error {
return downloadAnalyzerManagerAndRunScanners(auditParallelRunner, jasScanner, results, auditParams, threadId)
}, auditParallelRunner.AddErrorToChan); jasErr != nil {
auditParallelRunner.AddErrorToChan(fmt.Errorf("failed to create AM downloading task, skipping JAS scans...: %s", jasErr.Error()))
}
return
}

func downloadAnalyzerManagerAndRunScanners(auditParallelRunner *utils.SecurityParallelRunner, scanner *jas.JasScanner, scanResults *utils.Results, auditParams *AuditParams, threadId int) (err error) {
defer func() {
auditParallelRunner.JasWg.Done()
}()
if err = jas.DownloadAnalyzerManagerIfNeeded(threadId); err != nil {
return fmt.Errorf("%s failed to download analyzer manager: %s", clientutils.GetLogMsgPrefix(threadId, false), err.Error())
}
scanner, err = jas.CreateJasScanner(scanner, jfrogAppsConfig, serverDetails, jas.GetAnalyzerManagerXscEnvVars(auditParams.commonGraphScanParams.MultiScanId, scanResults.ExtendedScanResults.SecretValidation, scanResults.GetScaScannedTechnologies()...), auditParams.Exclusions()...)
if err != nil {
return fmt.Errorf("failed to create jas scanner: %s", err.Error())
}
if err = runner.AddJasScannersTasks(auditParallelRunner, scanResults, auditParams.DirectDependencies(), serverDetails, auditParams.thirdPartyApplicabilityScan, scanner, applicability.ApplicabilityScannerType, secrets.SecretsScannerType, auditParallelRunner.AddErrorToChan, auditParams.ScansToPerform(), auditParams.configProfile, auditParams.scanResultsOutputDir); err != nil {
if err = runner.AddJasScannersTasks(auditParallelRunner, scanResults, auditParams.DirectDependencies(), auditParams.thirdPartyApplicabilityScan, scanner, applicability.ApplicabilityScannerType, secrets.SecretsScannerType, auditParallelRunner.AddErrorToChan, auditParams.ScansToPerform(), auditParams.configProfile, auditParams.scanResultsOutputDir); err != nil {
return fmt.Errorf("%s failed to run JAS scanners: %s", clientutils.GetLogMsgPrefix(threadId, false), err.Error())
}
return
Expand Down
8 changes: 5 additions & 3 deletions commands/scan/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,13 +451,15 @@ func (scanCmd *ScanCommand) createIndexerHandlerFunc(file *spec.File, entitledFo
log.Error(fmt.Sprintf("failed to create JFrogAppsConfig: %s", err.Error()))
indexedFileErrors[threadId] = append(indexedFileErrors[threadId], formats.SimpleJsonError{FilePath: filePath, ErrorMessage: err.Error()})
}
scanner := &jas.JasScanner{}
scanner, err = jas.CreateJasScanner(scanner, jfrogAppsConfig, scanCmd.serverDetails, jas.GetAnalyzerManagerXscEnvVars(scanResults.MultiScanId, validateSecrets, techutils.Technology(graphScanResults.ScannedPackageType)))
scanner, err := jas.CreateJasScanner(jfrogAppsConfig, scanCmd.serverDetails, jas.GetAnalyzerManagerXscEnvVars(scanResults.MultiScanId, validateSecrets, techutils.Technology(graphScanResults.ScannedPackageType)))
if err != nil {
log.Error(fmt.Sprintf("failed to create jas scanner: %s", err.Error()))
indexedFileErrors[threadId] = append(indexedFileErrors[threadId], formats.SimpleJsonError{FilePath: filePath, ErrorMessage: err.Error()})
} else if scanner == nil {
log.Debug(fmt.Sprintf("Jas scanner was not created for %s, skipping Jas scans", filePath))
return nil
}
err = runner.AddJasScannersTasks(jasFileProducerConsumer, &scanResults, &depsList, scanCmd.serverDetails, false, scanner, applicability.ApplicabilityDockerScanScanType, secrets.SecretsScannerDockerScanType, jasErrHandlerFunc, utils.GetAllSupportedScans(), nil, "")
err = runner.AddJasScannersTasks(jasFileProducerConsumer, &scanResults, &depsList, false, scanner, applicability.ApplicabilityDockerScanScanType, secrets.SecretsScannerDockerScanType, jasErrHandlerFunc, utils.GetAllSupportedScans(), nil, "")
if err != nil {
log.Error(fmt.Sprintf("scanning '%s' failed with error: %s", graph.Id, err.Error()))
indexedFileErrors[threadId] = append(indexedFileErrors[threadId], formats.SimpleJsonError{FilePath: filePath, ErrorMessage: err.Error()})
Expand Down
1 change: 1 addition & 0 deletions jas/analyzermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func GetAnalyzerManagerExecutable() (analyzerManagerPath string, err error) {
return
}
if !exists {
log.Debug(fmt.Sprintf("The analyzer manager executable was not found at %s", analyzerManagerPath))
err = errors.New("unable to locate the analyzer manager package. Advanced security scans cannot be performed without this package")
}
return analyzerManagerPath, err
Expand Down
27 changes: 18 additions & 9 deletions jas/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ import (
"gopkg.in/yaml.v3"
)

const (
NoServerUrlError = "To incorporate the ‘Advanced Security’ scans into the audit output make sure platform url is provided and valid (run 'jf c add' prior to 'jf audit' via CLI, or provide JF_URL via Frogbot)"
NoServerDetailsError = "Jfrog Server details are missing"
)

type JasScanner struct {
TempDir string
AnalyzerManager AnalyzerManager
Expand All @@ -41,17 +46,22 @@ type JasScanner struct {
Exclusions []string
}

func CreateJasScanner(scanner *JasScanner, jfrogAppsConfig *jfrogappsconfig.JFrogAppsConfig, serverDetails *config.ServerDetails, envVars map[string]string, exclusions ...string) (*JasScanner, error) {
var err error
if scanner.AnalyzerManager.AnalyzerManagerFullPath, err = GetAnalyzerManagerExecutable(); err != nil {
return scanner, err
func CreateJasScanner(jfrogAppsConfig *jfrogappsconfig.JFrogAppsConfig, serverDetails *config.ServerDetails, envVars map[string]string, exclusions ...string) (scanner *JasScanner, err error) {
if serverDetails == nil {
err = errors.New(NoServerDetailsError)
return
}
if len(serverDetails.Url) == 0 {
log.Warn(NoServerUrlError)
return
}
scanner = &JasScanner{}
if scanner.EnvVars, err = getJasEnvVars(serverDetails, envVars); err != nil {
return scanner, err
return
}
var tempDir string
if tempDir, err = fileutils.CreateTempDir(); err != nil {
return scanner, err
return
}
scanner.TempDir = tempDir
scanner.ScannerDirCleanupFunc = func() error {
Expand All @@ -60,7 +70,7 @@ func CreateJasScanner(scanner *JasScanner, jfrogAppsConfig *jfrogappsconfig.JFro
scanner.ServerDetails = serverDetails
scanner.JFrogAppsConfig = jfrogAppsConfig
scanner.Exclusions = exclusions
return scanner, err
return
}

func getJasEnvVars(serverDetails *config.ServerDetails, vars map[string]string) (map[string]string, error) {
Expand Down Expand Up @@ -215,8 +225,7 @@ func InitJasTest(t *testing.T, workingDirs ...string) (*JasScanner, func()) {
assert.NoError(t, DownloadAnalyzerManagerIfNeeded(0))
jfrogAppsConfigForTest, err := CreateJFrogAppsConfig(workingDirs)
assert.NoError(t, err)
scanner := &JasScanner{}
scanner, err = CreateJasScanner(scanner, jfrogAppsConfigForTest, &FakeServerDetails, GetAnalyzerManagerXscEnvVars("", false))
scanner, err := CreateJasScanner(jfrogAppsConfigForTest, &FakeServerDetails, GetAnalyzerManagerXscEnvVars("", false))
assert.NoError(t, err)
return scanner, func() {
assert.NoError(t, scanner.ScannerDirCleanupFunc())
Expand Down
8 changes: 3 additions & 5 deletions jas/runner/jasrunner.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"github.com/jfrog/gofrog/parallel"
jfrogappsconfig "github.com/jfrog/jfrog-apps-config/go"
"github.com/jfrog/jfrog-cli-core/v2/utils/config"
"github.com/jfrog/jfrog-cli-security/jas"
"github.com/jfrog/jfrog-cli-security/jas/applicability"
"github.com/jfrog/jfrog-cli-security/jas/iac"
Expand All @@ -20,11 +19,10 @@ import (
"golang.org/x/exp/slices"
)

func AddJasScannersTasks(securityParallelRunner *utils.SecurityParallelRunner, scanResults *utils.Results, directDependencies *[]string,
serverDetails *config.ServerDetails, thirdPartyApplicabilityScan bool, scanner *jas.JasScanner, scanType applicability.ApplicabilityScanType,
func AddJasScannersTasks(securityParallelRunner *utils.SecurityParallelRunner, scanResults *utils.Results, directDependencies *[]string, thirdPartyApplicabilityScan bool, scanner *jas.JasScanner, scanType applicability.ApplicabilityScanType,
secretsScanType secrets.SecretsScanType, errHandlerFunc func(error), scansToPreform []utils.SubScanType, configProfile *services.ConfigProfile, scansOutputDir string) (err error) {
if serverDetails == nil || len(serverDetails.Url) == 0 {
log.Warn("To incorporate the ‘Advanced Security’ scans into the audit output make sure platform url is provided and valid (run 'jf c add' prior to 'jf audit' via CLI, or provide JF_URL via Frogbot)")
// Set the analyzer manager executable path.
if scanner.AnalyzerManager.AnalyzerManagerFullPath, err = jas.GetAnalyzerManagerExecutable(); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we doing this here? what is changed in the logic that we didn't need it before and now we do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing has changed. the logic was just moved from one place to another. (Refactoring...)

return
}
// For docker scan we support only secrets and contextual scans.
Expand Down
25 changes: 14 additions & 11 deletions jas/runner/jasrunner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"github.com/stretchr/testify/assert"
)

func TestGetExtendedScanResults_AnalyzerManagerDoesntExist(t *testing.T) {
func TestJasRunner_AnalyzerManagerNotExist(t *testing.T) {
tmpDir, err := fileutils.CreateTempDir()
assert.NoError(t, err)
defer func() {
Expand All @@ -26,32 +26,35 @@ func TestGetExtendedScanResults_AnalyzerManagerDoesntExist(t *testing.T) {
defer func() {
assert.NoError(t, os.Unsetenv(coreutils.HomeDir))
}()
scanner := &jas.JasScanner{}
_, err = jas.CreateJasScanner(scanner, nil, &jas.FakeServerDetails, jas.GetAnalyzerManagerXscEnvVars("", false))
scanner, err := jas.CreateJasScanner(nil, &jas.FakeServerDetails, jas.GetAnalyzerManagerXscEnvVars("", false))
assert.NoError(t, err)
if scanner.AnalyzerManager.AnalyzerManagerFullPath, err = jas.GetAnalyzerManagerExecutable(); err != nil {
return
}
assert.Error(t, err)
assert.NotNil(t, scanner)
assert.ErrorContains(t, err, "unable to locate the analyzer manager package. Advanced security scans cannot be performed without this package")
}

func TestGetExtendedScanResults_ServerNotValid(t *testing.T) {
func TestJasRunner(t *testing.T) {
securityParallelRunnerForTest := utils.CreateSecurityParallelRunner(cliutils.Threads)
scanResults := &utils.Results{ScaResults: []*utils.ScaScanResult{{Technology: techutils.Pip, XrayResults: jas.FakeBasicXrayResults}}, ExtendedScanResults: &utils.ExtendedScanResults{}}

scanner := &jas.JasScanner{}
jasScanner, err := jas.CreateJasScanner(scanner, nil, &jas.FakeServerDetails, jas.GetAnalyzerManagerXscEnvVars("", false, scanResults.GetScaScannedTechnologies()...))
jfrogAppsConfigForTest, err := jas.CreateJFrogAppsConfig(nil)
assert.NoError(t, err)
jasScanner, err := jas.CreateJasScanner(jfrogAppsConfigForTest, &jas.FakeServerDetails, jas.GetAnalyzerManagerXscEnvVars("", false, scanResults.GetScaScannedTechnologies()...))
assert.NoError(t, err)
err = AddJasScannersTasks(securityParallelRunnerForTest, scanResults, &[]string{"issueId_1_direct_dependency", "issueId_2_direct_dependency"}, nil, false, jasScanner, applicability.ApplicabilityScannerType, secrets.SecretsScannerType, securityParallelRunnerForTest.AddErrorToChan, utils.GetAllSupportedScans(), nil, "")
err = AddJasScannersTasks(securityParallelRunnerForTest, scanResults, &[]string{"issueId_1_direct_dependency", "issueId_2_direct_dependency"}, false, jasScanner, applicability.ApplicabilityScannerType, secrets.SecretsScannerType, securityParallelRunnerForTest.AddErrorToChan, utils.GetAllSupportedScans(), nil, "")
assert.NoError(t, err)
}

func TestGetExtendedScanResults_AnalyzerManagerReturnsError(t *testing.T) {
func TestJasRunner_AnalyzerManagerReturnsError(t *testing.T) {
assert.NoError(t, jas.DownloadAnalyzerManagerIfNeeded(0))

jfrogAppsConfigForTest, _ := jas.CreateJFrogAppsConfig(nil)
scanner := &jas.JasScanner{}
scanner, _ = jas.CreateJasScanner(scanner, nil, &jas.FakeServerDetails, jas.GetAnalyzerManagerXscEnvVars("", false))
scanner, _ := jas.CreateJasScanner(nil, &jas.FakeServerDetails, jas.GetAnalyzerManagerXscEnvVars("", false))
_, err := applicability.RunApplicabilityScan(jas.FakeBasicXrayResults, []string{"issueId_2_direct_dependency", "issueId_1_direct_dependency"},
scanner, false, applicability.ApplicabilityScannerType, jfrogAppsConfigForTest.Modules[0], 0)

// Expect error:
assert.ErrorContains(t, err, "failed to run Applicability scan")
}
Loading