-
Notifications
You must be signed in to change notification settings - Fork 22
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 Violations support for JAS scanners #241
base: dev
Are you sure you want to change the base?
Changes from 128 commits
650cc2b
8c2e4bd
7a4f5a1
c664895
dcd923e
03d0904
a16d7f2
8d3cd77
2c0b972
748a9b0
20eba2b
1c43d27
cc4720c
6065dd2
bb89eec
49617de
256064d
5a7bf61
88074b8
42b2fde
0698069
10e85c0
586aaa3
1a9f913
3173b9e
638c075
12ae850
1283811
c71e6a1
6778de2
7b9a482
0f94b02
e10abad
dac3af2
df8e5c2
f8257b5
2588355
72fbe1f
ffa905f
fdd290f
f16184c
dd56650
e429235
468b368
8ad6e2c
26a350f
ffbf1d0
4a2f2e9
87749e0
154fc6b
04596c4
68f608f
af658ab
a69b532
892fa96
501cdf7
2f850a4
8b7aba3
0c83518
9faea6d
069dbae
98971f6
8231b90
952d30d
e580111
aff7661
34b3781
0a118f4
50721c7
bb0f250
87e3b39
c5d16fa
e9b5e38
c5989a8
0b8810b
c40a287
e277cff
f2ed7e9
1583071
13c5bc3
b90ff34
c2a2622
a10364b
a20fa43
f9060e5
2e4a959
6614703
5f7f30e
5910589
1ab19ef
e901cd4
82fdf45
5d48264
099946b
5d5f8db
1bd432f
848d30b
7d61201
72c24b1
d632cb7
74fc2ef
baeae02
625913e
430b001
58f23a7
d288e4c
c390cd1
2017948
3100399
3507b9c
46019b3
113b34e
994ae2d
8deb9c0
40f074c
c5265b1
8e4055f
aab3114
a87dc1b
f6d4b07
d219f16
be4a8e8
bd0bd1b
1a010ae
0421990
3e7f307
359395f
ad9c8da
b98b4fc
628439e
8644e6f
3feccb7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -30,10 +30,12 @@ import ( | |||||
"github.com/jfrog/jfrog-client-go/xray" | ||||||
"github.com/jfrog/jfrog-client-go/xray/services" | ||||||
xscservices "github.com/jfrog/jfrog-client-go/xsc/services" | ||||||
xscutils "github.com/jfrog/jfrog-client-go/xsc/services/utils" | ||||||
) | ||||||
|
||||||
type AuditCommand struct { | ||||||
watches []string | ||||||
gitRepoHttpsCloneUrl string | ||||||
projectKey string | ||||||
targetRepoPath string | ||||||
IncludeVulnerabilities bool | ||||||
|
@@ -53,6 +55,11 @@ func (auditCmd *AuditCommand) SetWatches(watches []string) *AuditCommand { | |||||
return auditCmd | ||||||
} | ||||||
|
||||||
func (auditCmd *AuditCommand) SetGitRepoHttpsCloneUrl(gitRepoHttpsCloneUrl string) *AuditCommand { | ||||||
auditCmd.gitRepoHttpsCloneUrl = gitRepoHttpsCloneUrl | ||||||
return auditCmd | ||||||
} | ||||||
|
||||||
func (auditCmd *AuditCommand) SetProject(project string) *AuditCommand { | ||||||
auditCmd.projectKey = project | ||||||
return auditCmd | ||||||
|
@@ -88,16 +95,62 @@ func (auditCmd *AuditCommand) SetThreads(threads int) *AuditCommand { | |||||
return auditCmd | ||||||
} | ||||||
|
||||||
func (auditCmd *AuditCommand) CreateCommonGraphScanParams() *scangraph.CommonGraphScanParams { | ||||||
commonParams := &scangraph.CommonGraphScanParams{ | ||||||
RepoPath: auditCmd.targetRepoPath, | ||||||
Watches: auditCmd.watches, | ||||||
ScanType: services.Dependency, | ||||||
func (auditCmd *AuditCommand) CreateAuditResultsContext(serverDetails *config.ServerDetails) (context results.ResultContext) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can unify CreateAuditResultsContext and CreateResultsContext since CreateResultsContext is called once only and in CreateAuditResultsContext we only passing the params from auditCmd. We can simply extract them to variable and save another function. |
||||||
return CreateResultsContext( | ||||||
serverDetails, | ||||||
auditCmd.GetXrayVersion(), | ||||||
auditCmd.watches, | ||||||
auditCmd.targetRepoPath, | ||||||
auditCmd.projectKey, | ||||||
auditCmd.gitRepoHttpsCloneUrl, | ||||||
auditCmd.IncludeVulnerabilities, | ||||||
auditCmd.IncludeLicenses, | ||||||
) | ||||||
} | ||||||
|
||||||
// Create a results context based on the provided parameters. resolves conflicts between the parameters based on the retrieved platform watches. | ||||||
func CreateResultsContext(serverDetails *config.ServerDetails, xrayVersion string, watches []string, artifactoryRepoPath, projectKey, gitRepoHttpsCloneUrl string, includeVulnerabilities, includeLicenses bool) (context results.ResultContext) { | ||||||
context = results.ResultContext{ | ||||||
RepoPath: artifactoryRepoPath, | ||||||
Watches: watches, | ||||||
ProjectKey: projectKey, | ||||||
IncludeVulnerabilities: shouldIncludeVulnerabilities(includeVulnerabilities, watches, artifactoryRepoPath, projectKey, ""), | ||||||
IncludeLicenses: includeLicenses, | ||||||
} | ||||||
if err := clientutils.ValidateMinimumVersion(clientutils.Xray, xrayVersion, services.MinXrayVersionGitRepoKey); err != nil { | ||||||
// Git repo key is not supported by the Xray version. | ||||||
return | ||||||
} | ||||||
if gitRepoHttpsCloneUrl == "" { | ||||||
// No git repo key was provided, no need to check anything else. | ||||||
log.Debug("Git repo key was not provided, jas violations will not be checked.") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
return | ||||||
} | ||||||
// Get the defined and active watches from the platform. | ||||||
manager, err := xsc.CreateXscService(serverDetails) | ||||||
if err != nil { | ||||||
log.Warn(fmt.Sprintf("Failed to create Xray services manager: %s", err.Error())) | ||||||
return | ||||||
} | ||||||
if context.PlatformWatches, err = manager.GetResourceWatches(xscutils.GetGitRepoUrlKey(gitRepoHttpsCloneUrl), projectKey); err != nil { | ||||||
log.Warn(fmt.Sprintf("Failed to get active defined watches: %s", err.Error())) | ||||||
return | ||||||
} | ||||||
// Set git repo key and check if it has any watches defined in the platform. | ||||||
context.GitRepoHttpsCloneUrl = gitRepoHttpsCloneUrl | ||||||
if len(context.PlatformWatches.GitRepositoryWatches) == 0 { | ||||||
log.Debug(fmt.Sprintf("Git repo key was provided (%s) but no watches were defined in the platform. ignoring git repo key...", context.GitRepoHttpsCloneUrl)) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the log should be something like: |
||||||
context.GitRepoHttpsCloneUrl = "" | ||||||
} | ||||||
commonParams.ProjectKey = auditCmd.projectKey | ||||||
commonParams.IncludeVulnerabilities = auditCmd.IncludeVulnerabilities | ||||||
commonParams.IncludeLicenses = auditCmd.IncludeLicenses | ||||||
return commonParams | ||||||
// We calculate again this time also taking into account the final git repo key value. | ||||||
// (if there are no watches defined on the git repo and no other context was given, we should include vulnerabilities) | ||||||
context.IncludeVulnerabilities = shouldIncludeVulnerabilities(includeVulnerabilities, watches, artifactoryRepoPath, projectKey, context.GitRepoHttpsCloneUrl) | ||||||
return | ||||||
} | ||||||
|
||||||
// If the user requested to include vulnerabilities, or if the user didn't provide any watches, project key, artifactory repo path or git repo key, we should include vulnerabilities. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the comment here should be: |
||||||
func shouldIncludeVulnerabilities(includeVulnerabilities bool, watches []string, artifactoryRepoPath, projectKey, gitRepoHttpsCloneUrl string) bool { | ||||||
return includeVulnerabilities || !(len(watches) > 0 || projectKey != "" || artifactoryRepoPath != "" || gitRepoHttpsCloneUrl != "") | ||||||
} | ||||||
|
||||||
func (auditCmd *AuditCommand) Run() (err error) { | ||||||
|
@@ -124,7 +177,7 @@ func (auditCmd *AuditCommand) Run() (err error) { | |||||
SetMinSeverityFilter(auditCmd.minSeverityFilter). | ||||||
SetFixableOnly(auditCmd.fixableOnly). | ||||||
SetGraphBasicParams(auditCmd.AuditBasicParams). | ||||||
SetCommonGraphScanParams(auditCmd.CreateCommonGraphScanParams()). | ||||||
SetResultsContext(auditCmd.CreateAuditResultsContext(serverDetails)). | ||||||
SetThirdPartyApplicabilityScan(auditCmd.thirdPartyApplicabilityScan). | ||||||
SetThreads(auditCmd.Threads). | ||||||
SetScansResultsOutputDir(auditCmd.scanResultsOutputDir).SetStartTime(startTime).SetMultiScanId(multiScanId) | ||||||
|
@@ -144,13 +197,10 @@ func (auditCmd *AuditCommand) Run() (err error) { | |||||
messages = []string{coreutils.PrintTitle("The ‘jf audit’ command also supports JFrog Advanced Security features, such as 'Contextual Analysis', 'Secret Detection', 'IaC Scan' and ‘SAST’.\nThis feature isn't enabled on your system. Read more - ") + coreutils.PrintLink(utils.JasInfoURL)} | ||||||
} | ||||||
if err = output.NewResultsWriter(auditResults). | ||||||
SetHasViolationContext(auditCmd.HasViolationContext()). | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: er set the Violation context in auditParams, but how does it get passed to the resultsWriter so we know we are in a violation context? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. based on the value in the results - I added |
||||||
SetIncludeVulnerabilities(auditCmd.IncludeVulnerabilities). | ||||||
SetIncludeLicenses(auditCmd.IncludeLicenses). | ||||||
SetOutputFormat(auditCmd.OutputFormat()). | ||||||
SetPrintExtendedTable(auditCmd.PrintExtendedTable). | ||||||
SetExtraMessages(messages). | ||||||
SetSubScansPreformed(auditCmd.ScansToPerform()). | ||||||
SetSubScansPerformed(auditCmd.ScansToPerform()). | ||||||
PrintScanResults(); err != nil { | ||||||
return errors.Join(err, auditResults.GetErrors()) | ||||||
} | ||||||
|
@@ -170,10 +220,6 @@ func (auditCmd *AuditCommand) CommandName() string { | |||||
return "generic_audit" | ||||||
} | ||||||
|
||||||
func (auditCmd *AuditCommand) HasViolationContext() bool { | ||||||
return len(auditCmd.watches) > 0 || auditCmd.projectKey != "" || auditCmd.targetRepoPath != "" | ||||||
} | ||||||
|
||||||
// Runs an audit scan based on the provided auditParams. | ||||||
// Returns an audit Results object containing all the scan results. | ||||||
// If the current server is entitled for JAS, the advanced security results will be included in the scan results. | ||||||
|
@@ -192,14 +238,14 @@ func RunAudit(auditParams *AuditParams) (cmdResults *results.SecurityCommandResu | |||||
var jasScanner *jas.JasScanner | ||||||
var generalJasScanErr error | ||||||
if jasScanner, generalJasScanErr = RunJasScans(auditParallelRunner, auditParams, cmdResults, jfrogAppsConfig); generalJasScanErr != nil { | ||||||
cmdResults.AddGeneralError(fmt.Errorf("An error has occurred during JAS scan process. JAS scan is skipped for the following directories: %s\n%s", strings.Join(cmdResults.GetTargetsPaths(), ","), generalJasScanErr.Error()), auditParams.AllowPartialResults()) | ||||||
cmdResults.AddGeneralError(fmt.Errorf("error has occurred during JAS scan process. JAS scan is skipped for the following directories: %s\n%s", strings.Join(cmdResults.GetTargetsPaths(), ","), generalJasScanErr.Error()), auditParams.AllowPartialResults()) | ||||||
} | ||||||
if auditParams.Progress() != nil { | ||||||
auditParams.Progress().SetHeadlineMsg("Scanning for issues") | ||||||
} | ||||||
// The sca scan doesn't require the analyzer manager, so it can run separately from the analyzer manager download routine. | ||||||
if generalScaScanError := buildDepTreeAndRunScaScan(auditParallelRunner, auditParams, cmdResults); generalScaScanError != nil { | ||||||
cmdResults.AddGeneralError(fmt.Errorf("An error has occurred during SCA scan process. SCA scan is skipped for the following directories: %s\n%s", strings.Join(cmdResults.GetTargetsPaths(), ","), generalScaScanError.Error()), auditParams.AllowPartialResults()) | ||||||
cmdResults.AddGeneralError(fmt.Errorf("error has occurred during SCA scan process. SCA scan is skipped for the following directories: %s\n%s", strings.Join(cmdResults.GetTargetsPaths(), ","), generalScaScanError.Error()), auditParams.AllowPartialResults()) | ||||||
} | ||||||
go func() { | ||||||
auditParallelRunner.ScaScansWg.Wait() | ||||||
|
@@ -234,7 +280,19 @@ func RunJasScans(auditParallelRunner *utils.SecurityParallelRunner, auditParams | |||||
return | ||||||
} | ||||||
auditParallelRunner.ResultsMu.Lock() | ||||||
jasScanner, err = jas.CreateJasScanner(serverDetails, scanResults.SecretValidation, auditParams.minSeverityFilter, jas.GetAnalyzerManagerXscEnvVars(auditParams.GetMultiScanId(), scanResults.GetTechnologies()...), auditParams.Exclusions()...) | ||||||
jasScanner, err = jas.CreateJasScanner( | ||||||
serverDetails, | ||||||
scanResults.SecretValidation, | ||||||
auditParams.minSeverityFilter, | ||||||
jas.GetAnalyzerManagerXscEnvVars( | ||||||
auditParams.GetMultiScanId(), | ||||||
jas.GetGitRepoUrlKey(auditParams.resultsContext.GitRepoHttpsCloneUrl), | ||||||
auditParams.resultsContext.ProjectKey, | ||||||
auditParams.resultsContext.Watches, | ||||||
scanResults.GetTechnologies()..., | ||||||
), | ||||||
auditParams.Exclusions()..., | ||||||
) | ||||||
auditParallelRunner.ResultsMu.Unlock() | ||||||
if err != nil { | ||||||
generalError = fmt.Errorf("failed to create jas scanner: %s", err.Error()) | ||||||
|
@@ -276,7 +334,7 @@ func createJasScansTasks(auditParallelRunner *utils.SecurityParallelRunner, scan | |||||
Scanner: scanner, | ||||||
Module: *module, | ||||||
ConfigProfile: auditParams.configProfile, | ||||||
ScansToPreform: auditParams.ScansToPerform(), | ||||||
ScansToPerform: auditParams.ScansToPerform(), | ||||||
SecretsScanType: secrets.SecretsScannerType, | ||||||
DirectDependencies: auditParams.DirectDependencies(), | ||||||
ThirdPartyApplicabilityScan: auditParams.thirdPartyApplicabilityScan, | ||||||
|
@@ -310,11 +368,13 @@ func initAuditCmdResults(params *AuditParams) (cmdResults *results.SecurityComma | |||||
cmdResults.SetXscVersion(params.GetXscVersion()) | ||||||
cmdResults.SetMultiScanId(params.GetMultiScanId()) | ||||||
cmdResults.SetStartTime(params.StartTime()) | ||||||
// Send entitlement requests | ||||||
cmdResults.SetResultsContext(params.resultsContext) | ||||||
|
||||||
xrayManager, err := xrayutils.CreateXrayServiceManager(serverDetails) | ||||||
if err != nil { | ||||||
return cmdResults.AddGeneralError(err, false) | ||||||
} | ||||||
// Send entitlement requests | ||||||
entitledForJas, err := isEntitledForJas(xrayManager, params) | ||||||
if err != nil { | ||||||
return cmdResults.AddGeneralError(err, false) | ||||||
|
@@ -330,11 +390,11 @@ func initAuditCmdResults(params *AuditParams) (cmdResults *results.SecurityComma | |||||
// No SCA targets were detected, add the root directory as a target for JAS scans. | ||||||
cmdResults.NewScanResults(results.ScanTarget{Target: params.workingDirs[0]}) | ||||||
} | ||||||
scanInfo, err := coreutils.GetJsonIndent(cmdResults) | ||||||
scanInfo, err := coreutils.GetJsonIndent(cmdResults.GetTargets()) | ||||||
if err != nil { | ||||||
return | ||||||
} | ||||||
log.Info(fmt.Sprintf("Preforming scans on %d targets:\n%s", len(cmdResults.Targets), scanInfo)) | ||||||
log.Info(fmt.Sprintf("Performing scans on %d targets:\n%s", len(cmdResults.Targets), scanInfo)) | ||||||
return | ||||||
} | ||||||
|
||||||
|
@@ -350,14 +410,14 @@ func detectScanTargets(cmdResults *results.SecurityCommandResults, params *Audit | |||||
log.Warn("Couldn't detect technologies in", requestedDirectory, "directory.", err.Error()) | ||||||
continue | ||||||
} | ||||||
// Create scans to preform | ||||||
// Create scans to perform | ||||||
for tech, workingDirs := range techToWorkingDirs { | ||||||
if tech == techutils.Dotnet { | ||||||
// We detect Dotnet and Nuget the same way, if one detected so does the other. | ||||||
// We don't need to scan for both and get duplicate results. | ||||||
continue | ||||||
} | ||||||
// No technology was detected, add scan without descriptors. (so no sca scan will be preformed and set at target level) | ||||||
// No technology was detected, add scan without descriptors. (so no sca scan will be performed and set at target level) | ||||||
if len(workingDirs) == 0 { | ||||||
// Requested technology (from params) descriptors/indicators were not found or recursive scan with NoTech value, add scan without descriptors. | ||||||
cmdResults.NewScanResults(results.ScanTarget{Target: requestedDirectory, Technology: tech}) | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: why did we take off the || shouldIncludeVulnerabilities(c) part only from Audit but not from Scan or DockerScan?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because only in audit we are passing
git repo url
but you are right, we can remove those as well