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

Git Audit - Diff mode #299

Open
wants to merge 17 commits into
base: dev
Choose a base branch
from
Open
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
9 changes: 9 additions & 0 deletions cli/docs/git/audit/help.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
package audit

import (
"github.com/jfrog/jfrog-cli-core/v2/plugins/components"
)

func GetDescription() string {
return "Audit your local git repository project for security issues."
}

func GetArguments() []components.Argument {
return []components.Argument{{Name: "target", Description: `Diff mode, run git diff between the cwd commit to the given target commit and audit the differences.
You can specify a commit hash, a branch name a tag name or a reference like HEAD~1.`}}
}
6 changes: 6 additions & 0 deletions cli/gitcommands.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ func getGitNameSpaceCommands() []components.Command {

func GitAuditCmd(c *components.Context) error {
gitAuditCmd := audit.NewGitAuditCommand()

if len(c.Arguments) > 0 {
// Diff mode
gitAuditCmd.SetDiffTarget(c.Arguments[0])
}

// Set connection params
serverDetails, err := createServerDetailsWithConfigOffer(c)
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions commands/audit/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ func RunJasScans(auditParallelRunner *utils.SecurityParallelRunner, auditParams
auditParams.resultsContext.Watches,
scanResults.GetTechnologies()...,
),
auditParams.filesToScan,
auditParams.Exclusions()...,
)
auditParallelRunner.ResultsMu.Unlock()
Expand Down Expand Up @@ -343,6 +344,7 @@ func createJasScansTasks(auditParallelRunner *utils.SecurityParallelRunner, scan
Module: *module,
ConfigProfile: auditParams.configProfile,
ScansToPerform: auditParams.ScansToPerform(),
FilesToScan: auditParams.filesToScan,
SecretsScanType: secrets.SecretsScannerType,
DirectDependencies: auditParams.DirectDependencies(),
ThirdPartyApplicabilityScan: auditParams.thirdPartyApplicabilityScan,
Expand Down
11 changes: 11 additions & 0 deletions commands/audit/auditparams.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ type AuditParams struct {
configProfile *xscservices.ConfigProfile
scanResultsOutputDir string
startTime time.Time
// Diff mode, scan only the files affected by the diff.
filesToScan []string
}

func NewAuditParams() *AuditParams {
Expand Down Expand Up @@ -133,3 +135,12 @@ func (params *AuditParams) createXrayGraphScanParams() *services.XrayGraphScanPa
ScanType: services.Dependency,
}
}

func (params *AuditParams) SetFilesToScan(filesToScan []string) *AuditParams {
params.filesToScan = filesToScan
return params
}

func (params *AuditParams) FilesToScan() []string {
return params.filesToScan
}
34 changes: 32 additions & 2 deletions commands/audit/scarunner.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"encoding/json"
"errors"
"fmt"
"strings"

"github.com/jfrog/jfrog-cli-security/commands/audit/sca/swift"

biutils "github.com/jfrog/build-info-go/utils"
Expand Down Expand Up @@ -92,6 +94,15 @@ func buildDepTreeAndRunScaScan(auditParallelRunner *utils.SecurityParallelRunner
log.Warn(fmt.Sprintf("Couldn't determine a package manager or build tool used by this project. Skipping the SCA scan in '%s'...", targetResult.Target))
continue
}
if partialScan, filterDescriptors := getTargetDescriptorsToScan(targetResult.ScaResults, auditParams.filesToScan); partialScan {
// Diff mode, scan only the files affected by the diff.
if len(filterDescriptors) == 0 {
log.Info(fmt.Sprintf("No changed descriptors found for %s. Skipping SCA scan...", targetResult.Target))
continue
} else {
log.Info(fmt.Sprintf("Partial SCA scan will be performed on the following descriptors: %v", filterDescriptors))
}
}
// Get the dependency tree for the technology in the working directory.
treeResult, bdtErr := buildDependencyTree(targetResult, auditParams)
if bdtErr != nil {
Expand All @@ -103,20 +114,39 @@ func buildDepTreeAndRunScaScan(auditParallelRunner *utils.SecurityParallelRunner
_ = targetResult.AddTargetError(fmt.Errorf("failed to build dependency tree: %s", bdtErr.Error()), auditParams.AllowPartialResults())
continue
}
// TODO: get dependency from the changes and filter tree only to contain the changed dependencies
// Filter the dependency tree to contain only the changed dependencies.

// Create sca scan task
auditParallelRunner.ScaScansWg.Add(1)
// defer auditParallelRunner.ScaScansWg.Done()
_, taskErr := auditParallelRunner.Runner.AddTaskWithError(executeScaScanTask(auditParallelRunner, serverDetails, auditParams, targetResult, treeResult), func(err error) {
_ = targetResult.AddTargetError(fmt.Errorf("Failed to execute SCA scan: %s", err.Error()), auditParams.AllowPartialResults())
_ = targetResult.AddTargetError(fmt.Errorf("failed to execute SCA scan: %s", err.Error()), auditParams.AllowPartialResults())
})
if taskErr != nil {
_ = targetResult.AddTargetError(fmt.Errorf("Failed to create SCA scan task: %s", taskErr.Error()), auditParams.AllowPartialResults())
_ = targetResult.AddTargetError(fmt.Errorf("failed to create SCA scan task: %s", taskErr.Error()), auditParams.AllowPartialResults())
auditParallelRunner.ScaScansWg.Done()
}
}
return
}

func getTargetDescriptorsToScan(scaResults *results.ScaScanResults, filesToScan []string) (bool, []string) {
if len(filesToScan) == 0 {
return false, scaResults.Descriptors
}
filteredDescriptors := datastructures.MakeSet[string]()
for _, fileToScan := range filesToScan {
for _, descriptor := range scaResults.Descriptors {
if strings.HasSuffix(descriptor, fileToScan) {
filteredDescriptors.Add(descriptor)
}
}
}
return true, filteredDescriptors.ToSlice()

}

func getRequestedDescriptors(params *AuditParams) map[techutils.Technology][]string {
requestedDescriptors := map[techutils.Technology][]string{}
if params.PipRequirementsFile() != "" {
Expand Down
47 changes: 47 additions & 0 deletions commands/git/audit/diffmode.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package audit

import (
"github.com/jfrog/jfrog-cli-security/utils/results"
"github.com/jfrog/jfrog-cli-security/utils/scm"
"github.com/jfrog/jfrog-client-go/utils/log"
)

func filterResultsNotInDiff(scanResults *results.SecurityCommandResults, changes *scm.ChangesRelevantToScan) (onlyResultsInDiff *results.SecurityCommandResults) {
if changes == nil || !changes.HasFileChanges() {
log.Debug("No diff targets to filter results")
return scanResults
}
diffDescriptors := getDescriptorsFromDiff(changes.GetChangedFilesPaths())
// Create a new results object with the same metadata
onlyResultsInDiff = results.NewCommandResults(scanResults.CmdType)
onlyResultsInDiff.CommandMetaData = scanResults.CommandMetaData
// Loop over the scan targets and filter out the results that are not in the diff
for _, target := range scanResults.Targets {
// Add scan target to the new results object with the same metadata and no results
filterTarget := onlyResultsInDiff.NewScanResults(target.ScanTarget)
filterTarget.Errors = target.Errors
// Go over the results and filter out the ones that are not in the diff
filterTarget.ScaResults = filterScaResultsNotInDiff(target.ScaResults, diffDescriptors)
filterTarget.JasResults = filterJasResultsNotInDiff(target.JasResults, changes)
}
return
}

func getDescriptorsFromDiff(diffTargets []string) (descriptors []string) {
return append(descriptors, diffTargets...)
}

// Filter SCA results that are not in the diff, if at least one SCA descriptor is in the diff, the target is in the diff
// TODO: when we can discover and match SCA issue to location at file level, we can improve filter capabilities
func filterScaResultsNotInDiff(scaResults *results.ScaScanResults, changedDescriptors []string) (filterResults *results.ScaScanResults) {
if len(changedDescriptors) == 0 {
log.Debug("No diff targets to filter SCA results")
return scaResults
}
log.Warn("Filtering SCA results based on diff is not fully supported yet, all SCA results at the file level are included if changed")
return scaResults
}

func filterJasResultsNotInDiff(jasResults *results.JasScansResults, changes *scm.ChangesRelevantToScan) (filterResults *results.JasScansResults) {
return jasResults
}
66 changes: 63 additions & 3 deletions commands/git/audit/gitaudit.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
return scmManager.GetSourceControlContext()
}

func toAuditParams(params GitAuditParams) *sourceAudit.AuditParams {
func toAuditParams(params GitAuditParams, changes *scm.ChangesRelevantToScan) *sourceAudit.AuditParams {
auditParams := sourceAudit.NewAuditParams()
// Connection params
auditParams.SetServerDetails(params.serverDetails).SetInsecureTls(params.serverDetails.InsecureTls).SetXrayVersion(params.xrayVersion).SetXscVersion(params.xscVersion)
Expand All @@ -90,6 +90,10 @@
log.Debug(fmt.Sprintf("Results context: %+v", resultContext))
// Scan params
auditParams.SetThreads(params.threads).SetWorkingDirs([]string{params.repositoryLocalPath}).SetExclusions(params.exclusions).SetScansToPerform(params.scansToPerform)
if changedPaths := changes.GetChangedFilesPaths(); len(changedPaths) > 0 {
log.Debug(fmt.Sprintf("Diff targets: %v", changedPaths))
auditParams.SetFilesToScan(changedPaths)
}
// Output params
auditParams.SetOutputFormat(params.outputFormat)
// Cmd information
Expand All @@ -100,8 +104,23 @@
}

func RunGitAudit(params GitAuditParams) (scanResults *results.SecurityCommandResults) {
// Get diff targets to scan if needed
getTargetIfDiffMode(params)
return results.NewCommandResults(utils.SourceCode)

diffTargets, err := getDiffTargets(params)

Check failure on line 111 in commands/git/audit/gitaudit.go

View workflow job for this annotation

GitHub Actions / Lint ubuntu

unreachable code

Check failure on line 111 in commands/git/audit/gitaudit.go

View workflow job for this annotation

GitHub Actions / Static-Check

unreachable code

Check failure on line 111 in commands/git/audit/gitaudit.go

View workflow job for this annotation

GitHub Actions / Lint windows

unreachable code

Check failure on line 111 in commands/git/audit/gitaudit.go

View workflow job for this annotation

GitHub Actions / Lint macos

unreachable code
if err != nil {
return results.NewCommandResults(utils.SourceCode).AddGeneralError(err, false)
}
if diffTargets != nil && !diffTargets.HasChanges() {
log.Warn("No changes detected in the diff, skipping the scan")
return results.NewCommandResults(utils.SourceCode)
}
log.Debug(fmt.Sprintf("Diff targets: %v", diffTargets))
// TODO: delete this line
return results.NewCommandResults(utils.SourceCode)
// Send scan started event
event := xsc.CreateAnalyticsEvent(services.CliProduct, services.CliEventType, params.serverDetails)

Check failure on line 123 in commands/git/audit/gitaudit.go

View workflow job for this annotation

GitHub Actions / Lint ubuntu

unreachable code

Check failure on line 123 in commands/git/audit/gitaudit.go

View workflow job for this annotation

GitHub Actions / Static-Check

unreachable code

Check failure on line 123 in commands/git/audit/gitaudit.go

View workflow job for this annotation

GitHub Actions / Lint windows

unreachable code

Check failure on line 123 in commands/git/audit/gitaudit.go

View workflow job for this annotation

GitHub Actions / Lint macos

unreachable code
event.GitInfo = &params.source
event.IsGitInfoFlow = true
multiScanId, startTime := xsc.SendNewScanEvent(
Expand All @@ -112,13 +131,54 @@
)
params.multiScanId = multiScanId
params.startTime = startTime
// Run the scan
scanResults = sourceAudit.RunAudit(toAuditParams(params))
// Run the scan and filter results not in diff
scanResults = filterResultsNotInDiff(sourceAudit.RunAudit(toAuditParams(params, diffTargets)), diffTargets)
// Send scan ended event
xsc.SendScanEndedWithResults(params.serverDetails, scanResults)
return scanResults
}

func getTargetIfDiffMode(params GitAuditParams) (changes []scm.FileDiffGenerator, err error) {
if params.diffTarget == "" {
return
}
gitManager, err := scm.NewGitManager(params.repositoryLocalPath)
if err != nil {
return
}
if params.commonAncestor, err = gitManager.GetCommonAncestor(params.diffTarget); err != nil {
return
}
log.Info(fmt.Sprintf("Diff mode: comparing '%s' against target '%s' (common ancestor '%s')", params.source.LastCommitHash, params.diffTarget, params.commonAncestor))
diff, err := gitManager.Diff(params.diffTarget)
if err != nil {
return
}
changes = scm.FilePatchToFileDiffGenerator(diff...)
for _, analyzedFile := range changes {
log.Info(fmt.Sprintf("Analyzing file: %s", analyzedFile.String()))
}
return
}

func getDiffTargets(params GitAuditParams) (changes *scm.ChangesRelevantToScan, err error) {
if params.diffTarget == "" {
return
}
gitManager, err := scm.NewGitManager(params.repositoryLocalPath)
if err != nil {
return
}
if params.commonAncestor, err = gitManager.GetCommonAncestor(params.diffTarget); err != nil {
return
}
log.Info(fmt.Sprintf("Diff mode: comparing '%s' against target '%s' (common ancestor '%s')", params.source.LastCommitHash, params.diffTarget, params.commonAncestor))
if relevantChanges, err := gitManager.ScanRelevantDiff(params.diffTarget, params.commonAncestor); err == nil {
changes = &relevantChanges
}
return
}

func (gaCmd *GitAuditCommand) getResultWriter(cmdResults *results.SecurityCommandResults) *output.ResultsWriter {
var messages []string
if !cmdResults.EntitledForJas {
Expand Down
10 changes: 9 additions & 1 deletion commands/git/audit/gitauditparams.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import (

type GitAuditParams struct {
// Git Params
source services.XscGitInfoContext
source services.XscGitInfoContext
diffTarget string
// Connection params
serverDetails *config.ServerDetails
// Violations params
Expand All @@ -31,12 +32,19 @@ type GitAuditParams struct {
repositoryLocalPath string
multiScanId string
startTime time.Time

commonAncestor string
}

func NewGitAuditParams() *GitAuditParams {
return &GitAuditParams{}
}

func (gap *GitAuditParams) SetDiffTarget(diffTarget string) *GitAuditParams {
gap.diffTarget = diffTarget
return gap
}

func (gap *GitAuditParams) SetServerDetails(serverDetails *config.ServerDetails) *GitAuditParams {
gap.serverDetails = serverDetails
return gap
Expand Down
1 change: 1 addition & 0 deletions commands/scan/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@ func (scanCmd *ScanCommand) createIndexerHandlerFunc(file *spec.File, cmdResults
scanCmd.resultsContext.Watches,
targetResults.GetTechnologies()...,
),
[]string{},
)
if err != nil {
return targetResults.AddTargetError(fmt.Errorf("failed to create jas scanner: %s", err.Error()), false)
Expand Down
11 changes: 8 additions & 3 deletions jas/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,12 @@ type JasScanner struct {
ServerDetails *config.ServerDetails
ScannerDirCleanupFunc func() error
EnvVars map[string]string
FilesToScan []string
Exclusions []string
MinSeverity severityutils.Severity
}

func CreateJasScanner(serverDetails *config.ServerDetails, validateSecrets bool, minSeverity severityutils.Severity, envVars map[string]string, exclusions ...string) (scanner *JasScanner, err error) {
func CreateJasScanner(serverDetails *config.ServerDetails, validateSecrets bool, minSeverity severityutils.Severity, envVars map[string]string, filesToScan []string, exclusions ...string) (scanner *JasScanner, err error) {
if serverDetails == nil {
err = errors.New(NoServerDetailsError)
return
Expand Down Expand Up @@ -75,6 +76,7 @@ func CreateJasScanner(serverDetails *config.ServerDetails, validateSecrets bool,
return fileutils.RemoveTempDir(tempDir)
}
scanner.ServerDetails = serverDetails
scanner.FilesToScan = filesToScan
scanner.Exclusions = exclusions
scanner.MinSeverity = minSeverity
return
Expand Down Expand Up @@ -289,7 +291,7 @@ var FakeBasicXrayResults = []services.ScanResponse{

func InitJasTest(t *testing.T) (*JasScanner, func()) {
assert.NoError(t, DownloadAnalyzerManagerIfNeeded(0))
scanner, err := CreateJasScanner(&FakeServerDetails, false, "", GetAnalyzerManagerXscEnvVars("", "", "", []string{}))
scanner, err := CreateJasScanner(&FakeServerDetails, false, "", GetAnalyzerManagerXscEnvVars("", "", "", []string{}), []string{})
assert.NoError(t, err)
return scanner, func() {
assert.NoError(t, scanner.ScannerDirCleanupFunc())
Expand Down Expand Up @@ -318,7 +320,10 @@ func ShouldSkipScanner(module jfrogappsconfig.Module, scanType jasutils.JasScanT
return false
}

func GetSourceRoots(module jfrogappsconfig.Module, scanner *jfrogappsconfig.Scanner) ([]string, error) {
func GetSourceRoots(module jfrogappsconfig.Module, scanner *jfrogappsconfig.Scanner, filesToScan ...string) ([]string, error) {
if len(filesToScan) > 0 {
return filesToScan, nil
}
root, err := filepath.Abs(module.SourceRoot)
if err != nil {
return []string{}, errorutils.CheckError(err)
Expand Down
1 change: 1 addition & 0 deletions jas/runner/jasrunner.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type JasRunnerParams struct {
AllowPartialResults bool

ScansToPerform []utils.SubScanType
FilesToScan []string

// Secret scan flags
SecretsScanType secrets.SecretsScanType
Expand Down
6 changes: 3 additions & 3 deletions jas/runner/jasrunner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestJasRunner_AnalyzerManagerNotExist(t *testing.T) {
defer func() {
assert.NoError(t, os.Unsetenv(coreutils.HomeDir))
}()
scanner, err := jas.CreateJasScanner(&jas.FakeServerDetails, false, "", jas.GetAnalyzerManagerXscEnvVars("", "", "", []string{}))
scanner, err := jas.CreateJasScanner(&jas.FakeServerDetails, false, "", jas.GetAnalyzerManagerXscEnvVars("", "", "", []string{}), []string{})
assert.NoError(t, err)
if scanner.AnalyzerManager.AnalyzerManagerFullPath, err = jas.GetAnalyzerManagerExecutable(); err != nil {
return
Expand All @@ -43,7 +43,7 @@ func TestJasRunner(t *testing.T) {
securityParallelRunnerForTest := utils.CreateSecurityParallelRunner(cliutils.Threads)
targetResults := results.NewCommandResults(utils.SourceCode).SetEntitledForJas(true).SetSecretValidation(true).NewScanResults(results.ScanTarget{Target: "target", Technology: techutils.Pip})

jasScanner, err := jas.CreateJasScanner(&jas.FakeServerDetails, false, "", jas.GetAnalyzerManagerXscEnvVars("", "", "", []string{}, targetResults.GetTechnologies()...))
jasScanner, err := jas.CreateJasScanner(&jas.FakeServerDetails, false, "", jas.GetAnalyzerManagerXscEnvVars("", "", "", []string{}, targetResults.GetTechnologies()...), []string{})
assert.NoError(t, err)

targetResults.NewScaScanResults(0, jas.FakeBasicXrayResults[0])
Expand All @@ -63,7 +63,7 @@ func TestJasRunner_AnalyzerManagerReturnsError(t *testing.T) {
assert.NoError(t, jas.DownloadAnalyzerManagerIfNeeded(0))

jfrogAppsConfigForTest, _ := jas.CreateJFrogAppsConfig(nil)
scanner, _ := jas.CreateJasScanner(&jas.FakeServerDetails, false, "", jas.GetAnalyzerManagerXscEnvVars("", "", "", []string{}))
scanner, _ := jas.CreateJasScanner(&jas.FakeServerDetails, false, "", jas.GetAnalyzerManagerXscEnvVars("", "", "", []string{}), []string{})
_, 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, jas.ParseAnalyzerManagerError(jasutils.Applicability, err), "failed to run Applicability scan")
Expand Down
Loading
Loading