Skip to content

Commit

Permalink
start diff
Browse files Browse the repository at this point in the history
  • Loading branch information
attiasas committed Feb 3, 2025
1 parent 815ef39 commit 2516eb9
Show file tree
Hide file tree
Showing 9 changed files with 309 additions and 49 deletions.
50 changes: 50 additions & 0 deletions commands/git/audit/diffmode.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package audit

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

func filterResultsNotInDiff(scanResults *results.SecurityCommandResults, changes *gitutils.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) {
for _, target := range diffTargets {

Check failure on line 31 in commands/git/audit/diffmode.go

View workflow job for this annotation

GitHub Actions / Static-Check

S1011: should replace loop with `descriptors = append(descriptors, diffTargets...)` (gosimple)
descriptors = append(descriptors, target)
}
return
}

// 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 *gitutils.ChangesRelevantToScan) (filterResults *results.JasScansResults) {
return jasResults
}
33 changes: 30 additions & 3 deletions commands/git/audit/gitaudit.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func DetectGitInfo(wd string) (gitInfo *services.XscGitInfoContext, err error) {
return gitManager.GetGitContext()
}

func toAuditParams(params GitAuditParams) *sourceAudit.AuditParams {
func toAuditParams(params GitAuditParams, changes *gitutils.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,9 @@ func toAuditParams(params GitAuditParams) *sourceAudit.AuditParams {
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))
}
// Output params
auditParams.SetOutputFormat(params.outputFormat)
// Cmd information
Expand All @@ -100,6 +103,15 @@ func toAuditParams(params GitAuditParams) *sourceAudit.AuditParams {
}

func RunGitAudit(params GitAuditParams) (scanResults *results.SecurityCommandResults) {
// Get diff targets to scan if needed
diffTargets, err := getDiffTargets(params)
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)
}
// Send scan started event
event := xsc.CreateAnalyticsEvent(services.CliProduct, services.CliEventType, params.serverDetails)
event.GitInfo = &params.source
Expand All @@ -112,13 +124,28 @@ func RunGitAudit(params GitAuditParams) (scanResults *results.SecurityCommandRes
)
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 getDiffTargets(params GitAuditParams) (changes *gitutils.ChangesRelevantToScan, err error) {
if params.diffTarget == "" {
return
}
log.Info(fmt.Sprintf("Diff mode: comparing against target '%s'", params.diffTarget))
gitManager, err := gitutils.NewGitManager(params.repositoryLocalPath)
if err != nil {
return
}
if relevantChanges, err := gitManager.ScanRelevantDiff(params.diffTarget); err == nil {
changes = &relevantChanges
}
return
}

func (gaCmd *GitAuditCommand) getResultWriter(cmdResults *results.SecurityCommandResults) *output.ResultsWriter {
var messages []string
if !cmdResults.EntitledForJas {
Expand Down
135 changes: 135 additions & 0 deletions utils/gitutils/filediff.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
package gitutils

import (
"fmt"

goDiff "github.com/go-git/go-git/v5/plumbing/format/diff"

"github.com/jfrog/gofrog/datastructures"
"github.com/jfrog/jfrog-client-go/utils/log"
)

type ChangesRelevantToScan struct {
ChangedFiles []FileChanges
ChangedBinaries []string
}

func (c ChangesRelevantToScan) HasChanges() bool {
return c.HasFileChanges() || c.HasBinaryChanges()
}

func (c ChangesRelevantToScan) HasFileChanges() bool {
return len(c.ChangedFiles) > 0
}

func (c ChangesRelevantToScan) HasBinaryChanges() bool {
return len(c.ChangedBinaries) > 0
}

func (c ChangesRelevantToScan) GetChangedFilesPaths() (paths []string) {
for _, file := range c.ChangedFiles {
paths = append(paths, file.Path)
}
return
}

func (c ChangesRelevantToScan) GetFileChanges(path string) (changes *FileChanges) {
for _, file := range c.ChangedFiles {
if file.Path == path {
return &file
}
}
return
}

// FileChangeRanges represents the changes in the
type FileChanges struct {
Path string
Changes []Range
}

func (f FileChanges) String() string {
return fmt.Sprintf("%s: %v", f.Path, f.Changes)
}

func (f FileChanges) Contains(startRow, startCol, endRow, endCol int) bool {
for _, change := range f.Changes {
if change.Contains(startRow, startCol, endRow, endCol) {
return true
}
}
return false
}

func (f FileChanges) Overlaps(startRow, startCol, endRow, endCol int) bool {
for _, change := range f.Changes {
if change.Overlaps(startRow, startCol, endRow, endCol) {
return true
}
}
return false
}

type Range struct {
StartRow int
StartCol int
EndRow int
EndCol int
}

func (r Range) String() string {
return fmt.Sprintf("%d:%d-%d:%d", r.StartRow, r.StartCol, r.EndRow, r.EndCol)
}

// Contains checks if the range contains (fully) the given range
func (r Range) Contains(startRow, startCol, endRow, endCol int) bool {
return r.StartRow <= startRow && r.StartCol <= startCol && r.EndRow >= endRow && r.EndCol >= endCol
}

// Overlaps checks if the range overlaps (partially or fully) the given range
func (r Range) Overlaps(startRow, startCol, endRow, endCol int) bool {
return r.StartRow < endRow && r.EndRow > startRow && r.StartCol < endCol && r.EndCol > startCol
}

func detectRelevantChanges(filePatches []goDiff.FilePatch) (changes ChangesRelevantToScan, err error) {
binariesChanged := datastructures.MakeSet[string]()
// Go over the file patches and detect the relevant changes
for _, filePatch := range filePatches {
from, to := filePatch.Files()
fromPath := ""
if from != nil {
fromPath = from.Path()
}
toPath := ""
if to != nil {
toPath = to.Path()
}
log.Debug(fmt.Sprintf("Checking Diff between: %s (from) and %s (to)", fromPath, toPath))
// Get the relevant changes for the file
if filePatch.IsBinary() {
// Binary file, only file path is relevant
binariesChanged.Add(to.Path())
continue
}
if to == nil {
// Deleted file, not relevant to scan
continue
}
// Get the relevant changes in the file, if new file (from is nil) all the content is relevant
if fileChanges := processFileChunksForRelevantChanges(filePatch.Chunks(), from == nil); /*len(fileChanges) > 0*/ true {
changes.ChangedFiles = append(changes.ChangedFiles, FileChanges{Path: to.Path(), Changes: fileChanges})
}
}
changes.ChangedBinaries = binariesChanged.ToSlice()
return
}

func processFileChunksForRelevantChanges(fileChunks []goDiff.Chunk, isNewFile bool) (changes []Range) {

Check failure on line 127 in utils/gitutils/filediff.go

View workflow job for this annotation

GitHub Actions / Static-Check

`processFileChunksForRelevantChanges` - `isNewFile` is unused (unparam)
// SARIF locations start at 1
// row, col := 1, 1
for _, diffChunk := range fileChunks {
chunkContent := diffChunk.Content()
log.Debug(fmt.Sprintf("Chunk (type = %d): \"%s\"", diffChunk.Type(), chunkContent))
}
return
}
4 changes: 4 additions & 0 deletions utils/gitutils/filediff_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package gitutils

import (
)
62 changes: 35 additions & 27 deletions utils/gitutils/gitmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import (
"strings"

goGit "github.com/go-git/go-git/v5"

"github.com/go-git/go-git/v5/plumbing"
"github.com/go-git/go-git/v5/plumbing/format/diff"
"github.com/jfrog/jfrog-client-go/utils/log"
"github.com/jfrog/jfrog-client-go/xsc/services"
)
Expand Down Expand Up @@ -122,32 +123,6 @@ func (gm *GitManager) IsClean() (bool, error) {
return status.IsClean(), nil
}

func (gm *GitManager) Diff(reference string) (err error) {
// Get the current branch
currentBranch, err := gm.localGitRepository.Head()
if err != nil {
return
}
// Get the commit object of the current branch
currentCommit, err := gm.localGitRepository.CommitObject(currentBranch.Hash())
if err != nil {
return
}
// Get the commit object of the reference
referenceCommit, err := gm.localGitRepository.CommitObject(currentCommit.Hash)
if err != nil {
return
}
// Get the diff between the current branch and the reference
diff, err := currentCommit.Patch(referenceCommit)
if err != nil {
return
}
// Print the diff
fmt.Println(diff)
return
}

// Detect git information
func (gm *GitManager) GetGitContext() (gitInfo *services.XscGitInfoContext, err error) {
remoteUrl, err := getRemoteUrl(gm.remote)
Expand Down Expand Up @@ -200,6 +175,39 @@ func getRemoteUrl(remote *goGit.Remote) (remoteUrl string, err error) {
return remote.Config().URLs[0], nil
}

func (gm *GitManager) Diff(reference string) (changes []diff.FilePatch, err error) {
// Get the current branch
currentBranch, err := gm.localGitRepository.Head()
if err != nil {
return
}
// Get the commit object of the current branch
currentCommit, err := gm.localGitRepository.CommitObject(currentBranch.Hash())
if err != nil {
return
}
// Get the commit object of the reference
referenceCommit, err := gm.localGitRepository.CommitObject(plumbing.NewHash(reference))
if err != nil {
return
}
// Get the diff between the current branch and the reference
diff, err := currentCommit.Patch(referenceCommit)
if err != nil {
return
}
changes = diff.FilePatches()
return
}

func (gm *GitManager) ScanRelevantDiff(reference string) (changes ChangesRelevantToScan, err error) {
diff, err := gm.Diff(reference)
if err != nil {
return
}
return detectRelevantChanges(diff)
}

// Normalize the URL by removing protocol prefix and any trailing ".git"
func normalizeGitUrl(url string) string {
// jfrog-ignore - false positive, not used for communication
Expand Down
Loading

0 comments on commit 2516eb9

Please sign in to comment.