Skip to content
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
11 changes: 9 additions & 2 deletions commands/audit/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ func (auditCmd *AuditCommand) Run() (err error) {
SetThirdPartyApplicabilityScan(auditCmd.thirdPartyApplicabilityScan).
SetThreads(auditCmd.Threads).
SetScansResultsOutputDir(auditCmd.scanResultsOutputDir).SetStartTime(startTime).SetMultiScanId(multiScanId).
SetSastChangedFilesMode(auditCmd.sastChangedFilesMode).SetSastRules(auditCmd.sastRules)
SetRootDir(auditCmd.rootDir).SetSastChangedFilesMode(auditCmd.sastChangedFilesMode).SetSastRules(auditCmd.sastRules)
auditParams.SetIsRecursiveScan(isRecursiveScan).SetExclusions(auditCmd.Exclusions())

auditResults := RunAudit(auditParams)
Expand Down Expand Up @@ -759,7 +759,7 @@ func createJasScansTask(auditParallelRunner *utils.SecurityParallelRunner, scanR
SignedDescriptions: getSignedDescriptions(auditParams.OutputFormat()),
SastRules: auditParams.SastRules(),
SastChangedFilesMode: auditParams.SastChangedFilesMode(),
SastChangedFiles: sast.SastChangedFilesForTarget(auditParams.SastChangedFilesMode(), scanResults.GitContext, targetResult.Target, scanResults.GetCommonParentPath()),
SastChangedFiles: sast.SastChangedFilesForTarget(auditParams.SastChangedFilesMode(), scanResults.GitContext, targetResult.Target, getRootDir(auditParams.rootDir, scanResults)),
ScanResults: targetResult,
TargetCount: len(scanResults.Targets),
TargetOutputDir: auditParams.scanResultsOutputDir,
Expand All @@ -775,6 +775,13 @@ func createJasScansTask(auditParallelRunner *utils.SecurityParallelRunner, scanR
}
}

func getRootDir(rootDir string, scanResults *results.SecurityCommandResults) string {
if rootDir != "" {
return rootDir
}
return scanResults.GetCommonParentPath()
}

func getSignedDescriptions(currentFormat format.OutputFormat) bool {
allowEmojis, err := strconv.ParseBool(os.Getenv(utils.IsAllowEmojis))
if err != nil {
Expand Down
10 changes: 10 additions & 0 deletions commands/audit/auditparams.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type AuditParams struct {
resultsContext results.ResultContext
gitContext *xscServices.XscGitInfoContext
workingDirs []string
rootDir string
installFunc func(tech string) error
fixableOnly bool
minSeverityFilter severityutils.Severity
Expand Down Expand Up @@ -71,6 +72,15 @@ func (params *AuditParams) WorkingDirs() []string {
return params.workingDirs
}

func (params *AuditParams) SetRootDir(rootDir string) *AuditParams {
params.rootDir = rootDir
return params
}

func (params *AuditParams) RootDir() string {
return params.rootDir
}

func (params *AuditParams) SetMultiScanId(msi string) *AuditParams {
params.multiScanId = msi
return params
Expand Down
49 changes: 7 additions & 42 deletions jas/sast/sastscanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,23 +216,15 @@ func (s sastChangedFileDropStats) anyDrops() bool {
}

// collectSastChangedAbsPaths maps repo-relative (or absolute-under-repo) changed file paths to clean absolute
// paths under commonAbs that belong to targetRel, deduplicating by absolute path.
func collectSastChangedAbsPaths(commonAbs, targetRel string, changedFiles []string) (out []string, stats sastChangedFileDropStats) {
seen := datastructures.MakeSet[string]()
for _, cf := range changedFiles {
cfSlash, ok := normalizeRepoRelativeChangedPath(commonAbs, cf)
if !ok {
log.Verbose(fmt.Sprintf("SAST changed files: invalid path: %s", cf))
stats.invalidPath++
continue
}
if !changedFileBelongsToTarget(targetRel, cfSlash) {
if !changedFileBelongsToTarget(targetRel, cf) {
log.Verbose(fmt.Sprintf("SAST changed files: outside target: %s", cf))
stats.outsideTarget++
continue
}
joined := filepath.Join(commonAbs, filepath.FromSlash(cfSlash))
absPath, err := filepath.Abs(filepath.Clean(joined))
absPath, err := filepath.Abs(filepath.Clean(filepath.Join(commonAbs, filepath.FromSlash(cf))))
if err != nil {
log.Verbose(fmt.Sprintf("SAST changed files: absolute path error: %s", err.Error()))
stats.absError++
Expand All @@ -254,29 +246,22 @@ func collectSastChangedAbsPaths(commonAbs, targetRel string, changedFiles []stri
return out, stats
}

// SastChangedFilesForTarget returns absolute paths of changed files under commonParent that belong to targetPath
// (paths from git are repo-relative or absolute under the repo). Only runs when changedFilesMode is true; only paths
// that exist on disk are returned. Returns nil if nothing matches or if gitCtx, commonParent, or targetPath are unusable.
func SastChangedFilesForTarget(changedFilesMode bool, gitCtx *xscservices.XscGitInfoContext, targetPath, commonParent string) []string {
// SastChangedFilesForTarget returns absolute paths of changed files under the root directory that belong to the target path.
func SastChangedFilesForTarget(changedFilesMode bool, gitCtx *xscservices.XscGitInfoContext, targetPath, rootDir string) []string {
if gitCtx == nil || !changedFilesMode {
return nil
}
if len(gitCtx.ChangedFiles) == 0 {
log.Debug("SAST changed files: git context has no changed files; skipping per-file roots")
return nil
}
if strings.TrimSpace(commonParent) == "" || strings.TrimSpace(targetPath) == "" {
if strings.TrimSpace(rootDir) == "" || strings.TrimSpace(targetPath) == "" {
log.Debug("SAST changed files: empty common parent or target path; skipping per-file roots")
return nil
}
commonAbs, err := filepath.Abs(filepath.Clean(commonParent))
if err != nil {
log.Debug(fmt.Sprintf("SAST changed files: could not resolve common parent: %s", err.Error()))
return nil
}
targetRel := filepath.ToSlash(utils.GetRelativePath(targetPath, commonParent))
targetRel := filepath.ToSlash(utils.GetRelativePath(targetPath, rootDir))
inputCount := len(gitCtx.ChangedFiles)
out, stats := collectSastChangedAbsPaths(commonAbs, targetRel, gitCtx.ChangedFiles)
out, stats := collectSastChangedAbsPaths(rootDir, targetRel, gitCtx.ChangedFiles)
if stats.anyDrops() {
log.Debug(fmt.Sprintf("SAST changed files: kept %d of %d changed-file entries (dropped: %d invalid/unsafe path, %d outside target, %d path resolution error, %d duplicate after normalization)",
len(out), inputCount, stats.invalidPath, stats.outsideTarget, stats.absError, stats.duplicate))
Expand All @@ -285,26 +270,6 @@ func SastChangedFilesForTarget(changedFilesMode bool, gitCtx *xscservices.XscGit
return out
}

func normalizeRepoRelativeChangedPath(commonAbs, cf string) (slashPath string, ok bool) {
cf = strings.TrimSpace(cf)
if cf == "" {
return "", false
}
if filepath.IsAbs(cf) {
cleaned := filepath.Clean(cf)
r, err := filepath.Rel(commonAbs, cleaned)
if err != nil {
return "", false
}
r = filepath.ToSlash(filepath.Clean(r))
if r == ".." || strings.HasPrefix(r, "../") {
return "", false
}
return r, true
}
return filepath.ToSlash(filepath.Clean(cf)), true
}

func changedFileBelongsToTarget(targetRel, cfSlash string) bool {
if targetRel == "" {
return true
Expand Down
33 changes: 17 additions & 16 deletions jas/sast/sastscanner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func TestSastRules(t *testing.T) {
assert.Equal(t, filepath.Join(scannerTempDir, "results.sarif"), sastScanManager.resultsFileName)
}

// xscGitInfoWithChanged builds an XscGitInfoContext the way the client defines it (GitDiffContext with changed_files).
// xscGitInfoWithChanged builds an XscGitInfoContext the way the client defines it (GitDiffContext with changed files).
// Must match the shape expected by SastChangedFilesForTarget in sastscanner.go.
func xscGitInfoWithChanged(t *testing.T, files ...string) *xscservices.XscGitInfoContext {
t.Helper()
Expand Down Expand Up @@ -237,61 +237,62 @@ func TestSastChangedFilesForTarget(t *testing.T) {
name string
gitCtx *xscservices.XscGitInfoContext
targetPath string
commonParent string
rootDir string
changedFilesMode bool
// wantEmpty: expect no file roots (nil or empty slice) when mode is off or there is nothing to return.
wantEmpty bool
want []string
}{
{name: "nil_context", gitCtx: nil, targetPath: base, commonParent: base, changedFilesMode: true, wantEmpty: true},
{name: "changed_files_mode_off", gitCtx: threeFiles, targetPath: modA, commonParent: base, changedFilesMode: false, wantEmpty: true},
{name: "empty_changed_files", gitCtx: xscGitInfoWithChanged(t), targetPath: modA, commonParent: base, changedFilesMode: true, wantEmpty: true},
{name: "empty_common_parent", gitCtx: threeFiles, targetPath: modA, commonParent: "", changedFilesMode: true, wantEmpty: true},
{name: "empty_target_path", gitCtx: threeFiles, targetPath: "", commonParent: base, changedFilesMode: true, wantEmpty: true},
{name: "nil_context", gitCtx: nil, targetPath: base, rootDir: base, changedFilesMode: true, wantEmpty: true},
{name: "changed_files_mode_off", gitCtx: threeFiles, targetPath: modA, rootDir: base, changedFilesMode: false, wantEmpty: true},
{name: "empty_changed_files", gitCtx: xscGitInfoWithChanged(t), targetPath: modA, rootDir: base, changedFilesMode: true, wantEmpty: true},
{name: "empty_root_dir", gitCtx: threeFiles, targetPath: modA, rootDir: "", changedFilesMode: true, wantEmpty: true},
{name: "empty_target_path", gitCtx: threeFiles, targetPath: "", rootDir: base, changedFilesMode: true, wantEmpty: true},
{
name: "target_is_common_parent_returns_all_as_abs",
name: "target_is_repo_root_returns_all_as_abs",
gitCtx: threeFiles,
targetPath: base,
commonParent: base,
rootDir: base,
changedFilesMode: true,
want: []string{filepath.Join(base, "modA", "a.go"), filepath.Join(base, "modA", "b.go"), filepath.Join(base, "modB", "x.go")},
},
{
name: "filters_to_modA_only",
gitCtx: threeFiles,
targetPath: modA,
commonParent: base,
rootDir: base,
changedFilesMode: true,
want: []string{filepath.Join(base, "modA", "a.go"), filepath.Join(base, "modA", "b.go")},
},
{
name: "prefix_foo_does_not_match_foobar",
gitCtx: &xscservices.XscGitInfoContext{GitDiffContext: xscservices.GitDiffContext{ChangedFiles: []string{"foo/x.go", "foobar/y.go"}}},
targetPath: filepath.Join(base, "foo"),
commonParent: base,
rootDir: base,
changedFilesMode: true,
want: []string{filepath.Join(base, "foo", "x.go")},
},
{
name: "absolute_changed_file_under_repo",
gitCtx: xscGitInfoWithChanged(t, filepath.Join(base, "modA", "abs.go")),
// belong-to-target matching uses repo-relative paths (as git reports); resolve to absolute under rootDir afterward.
name: "repo_relative_changed_file_under_target",
gitCtx: xscGitInfoWithChanged(t, "modA/abs.go"),
targetPath: modA,
commonParent: base,
rootDir: base,
changedFilesMode: true,
want: []string{filepath.Join(base, "modA", "abs.go")},
},
{
name: "deduplicates_same_paths",
gitCtx: &xscservices.XscGitInfoContext{GitDiffContext: xscservices.GitDiffContext{ChangedFiles: []string{"modA/a.go", "modA/a.go", "./modA/a.go"}}},
targetPath: modA,
commonParent: base,
rootDir: base,
changedFilesMode: true,
want: []string{filepath.Join(base, "modA", "a.go")},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := SastChangedFilesForTarget(tt.changedFilesMode, tt.gitCtx, tt.targetPath, tt.commonParent)
got := SastChangedFilesForTarget(tt.changedFilesMode, tt.gitCtx, tt.targetPath, tt.rootDir)
if tt.wantEmpty {
assert.Empty(t, got, "SastChangedFilesForTarget should not return any paths in this case")
} else {
Expand Down
Loading