Add root dir to determine paths for sast changed files (#743)

Author: attiasas

commands/audit/audit.go

@@ -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)
@@ -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,
@@ -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 {

commands/audit/auditparams.go

@@ -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
@@ -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

jas/sast/sastscanner.go

@@ -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++
@@ -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))
@@ -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

jas/sast/sastscanner_test.go

@@ -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()
@@ -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 {