Skip to content

Commit

Permalink
fix: diff scan fixes (#1381)
Browse files Browse the repository at this point in the history
* fix: support repos with no remote

* fix: make diff fingerprints match full scan

* fix: don't discard uncommited changes when diffing

* fix: still produce output when there are no files
  • Loading branch information
didroe authored Nov 8, 2023
1 parent f1685bd commit 35c287c
Show file tree
Hide file tree
Showing 12 changed files with 212 additions and 63 deletions.
75 changes: 36 additions & 39 deletions internal/commands/artifact/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ import (
"github.com/bearer/bearer/internal/types"
)

var ErrFileListEmpty = errors.New("couldn't find any files to scan in the specified directory, for diff scans this can mean the compared branches were identical")

// TargetKind represents what kind of artifact bearer scans
type TargetKind string

Expand Down Expand Up @@ -164,10 +162,6 @@ func (r *runner) Scan(ctx context.Context, opts flag.Options) ([]files.File, *ba
return nil, nil, err
}

if len(fileList.Files) == 0 {
return nil, nil, ErrFileListEmpty
}

orchestrator, err := orchestrator.New(
work.Repository{Dir: r.targetPath},
r.scanSettings,
Expand All @@ -185,19 +179,11 @@ func (r *runner) Scan(ctx context.Context, opts flag.Options) ([]files.File, *ba
outputhandler.StdErrLog(fmt.Sprintf("\nScanning base branch %s", opts.DiffBaseBranch))
}

if err := orchestrator.Scan(r.reportPath+".base", fileList.BaseFiles); err != nil {
return err
}

report := types.Report{Path: r.reportPath + ".base", Inputgocloc: r.goclocResult}

reportData, err := reportoutput.GetData(report, r.scanSettings, nil)
baseBranchFindings, err = r.scanBaseBranch(orchestrator, fileList)
if err != nil {
return err
}

baseBranchFindings = buildBaseBranchFindings(reportData, fileList)

if !opts.Quiet {
outputhandler.StdErrLog("\nScanning current branch")
}
Expand All @@ -214,6 +200,40 @@ func (r *runner) Scan(ctx context.Context, opts flag.Options) ([]files.File, *ba
return fileList.Files, baseBranchFindings, nil
}

func (r *runner) scanBaseBranch(
orchestrator *orchestrator.Orchestrator,
fileList *files.List,
) (*basebranchfindings.Findings, error) {
result := basebranchfindings.New(fileList)

if len(fileList.BaseFiles) == 0 {
return result, nil
}

if err := orchestrator.Scan(r.reportPath+".base", fileList.BaseFiles); err != nil {
return nil, err
}

report := types.Report{
Path: r.reportPath + ".base",
Inputgocloc: r.goclocResult,
HasFiles: len(fileList.BaseFiles) != 0,
}

reportData, err := reportoutput.GetData(report, r.scanSettings, nil)
if err != nil {
return nil, err
}

for _, findings := range reportData.FindingsBySeverity {
for _, finding := range findings {
result.Add(finding.Rule.Id, finding.Filename, finding.Sink.Start, finding.Sink.End)
}
}

return result, nil
}

func getIgnoredFingerprints(client *api.API, settings settings.Config) (
useCloudIgnores bool,
ignoredFingerprints map[string]ignoretypes.IgnoredFingerprint,
Expand Down Expand Up @@ -314,12 +334,6 @@ func Run(ctx context.Context, opts flag.Options) (err error) {

files, baseBranchFindings, err := r.Scan(ctx, opts)
if err != nil {
if errors.Is(err, ErrFileListEmpty) {
outputhandler.StdOutLog(err.Error())
os.Exit(0)
return
}

return fmt.Errorf("scan error: %w", err)
}

Expand Down Expand Up @@ -360,7 +374,7 @@ func (r *runner) Report(
startTime := time.Now()
cacheUsed := r.CacheUsed()

report := types.Report{Path: r.reportPath, Inputgocloc: r.goclocResult}
report := types.Report{Path: r.reportPath, Inputgocloc: r.goclocResult, HasFiles: len(files) != 0}

// if output is defined we want to write only to file
logger := outputhandler.StdOutLog
Expand Down Expand Up @@ -490,20 +504,3 @@ func FormatFoundLanguages(languages map[string]*gocloc.Language) (foundLanguages

return keys
}

func buildBaseBranchFindings(reportData *outputtypes.ReportData, fileList *files.List) *basebranchfindings.Findings {
result := basebranchfindings.New(fileList)

for _, findings := range reportData.FindingsBySeverity {
for _, finding := range findings {
result.Add(
finding.Rule.Id,
finding.Filename,
finding.Sink.Start,
finding.Sink.End,
)
}
}

return result
}
28 changes: 25 additions & 3 deletions internal/commands/process/gitrepository/gitrepository.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ type Repository struct {
rootPath,
targetPath,
gitTargetPath string
baseRemoteRefName plumbing.ReferenceName
headRef *plumbing.Reference
baseRemoteRefName,
baseLocalRefName plumbing.ReferenceName
headRef *plumbing.Reference
headCommit,
mergeBaseCommit *object.Commit
githubToken string
Expand Down Expand Up @@ -88,6 +89,7 @@ func New(
targetPath: targetPath,
gitTargetPath: gitTargetPath,
baseRemoteRefName: plumbing.NewRemoteReferenceName("origin", baseBranch),
baseLocalRefName: plumbing.NewBranchReferenceName(baseBranch),
headRef: headRef,
headCommit: headCommit,
githubToken: config.Scan.GithubToken,
Expand Down Expand Up @@ -304,6 +306,14 @@ func (repository *Repository) WithBaseBranch(body func() error) error {
return fmt.Errorf("error getting git worktree: %w", err)
}

status, err := worktree.Status()
if err != nil {
return err
}
if !status.IsClean() {
return errors.New("uncommitted changes found in worktree. commit or stash changes your changes and retry")
}

defer repository.restoreHead(worktree)

if err := worktree.Checkout(&git.CheckoutOptions{
Expand Down Expand Up @@ -390,14 +400,26 @@ func (repository *Repository) lookupMergeBaseHash(baseBranch string) (*plumbing.
log.Debug().Msg("finding merge base using local repository")

ref, err := repository.git.Reference(repository.baseRemoteRefName, true)
if err != nil {
if err != nil && err != plumbing.ErrReferenceNotFound {
if err == plumbing.ErrReferenceNotFound {
return nil, nil
}

return nil, fmt.Errorf("invalid ref: %w", err)
}

if err == plumbing.ErrReferenceNotFound {
log.Debug().Msg("remote ref not found, trying local ref")
ref, err = repository.git.Reference(repository.baseLocalRefName, true)
if err != nil {
if err == plumbing.ErrReferenceNotFound {
return nil, nil
}

return nil, fmt.Errorf("invalid ref: %w", err)
}
}

baseCommit, err := repository.git.CommitObject(ref.Hash())
if err != nil {
if err == plumbing.ErrObjectNotFound {
Expand Down
3 changes: 2 additions & 1 deletion internal/languages/testhelper/testhelper.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ func (runner *Runner) scanSingleFile(t *testing.T, testDataPath string, fileRela
runner.config.Scan.Target = testDataPath
reportData, err := output.GetData(
types.Report{
Path: detectorsReportPath,
Path: detectorsReportPath,
HasFiles: true,
},
runner.config,
nil,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func TestDataflowComponents(t *testing.T) {
return
}

if err = dataflow.AddReportData(output, settings.Config{}, false); err != nil {
if err = dataflow.AddReportData(output, settings.Config{}, false, true); err != nil {
t.Fatalf("failed to get dataflow output %s", err)
return
}
Expand Down
7 changes: 6 additions & 1 deletion internal/report/output/dataflow/dataflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,12 @@ func contains(detections []detections.DetectionType, detection detections.Detect
return false
}

func AddReportData(reportData *types.ReportData, config settings.Config, isInternal bool) error {
func AddReportData(reportData *types.ReportData, config settings.Config, isInternal, hasFiles bool) error {
if !hasFiles {
reportData.Dataflow = &types.DataFlow{}
return nil
}

dataTypesHolder := datatypes.New(config, isInternal)
risksHolder := risks.New(config, isInternal)
componentsHolder := components.New(isInternal)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func TestDataflowDataType(t *testing.T) {
return
}

if err = dataflow.AddReportData(output, test.Config, false); err != nil {
if err = dataflow.AddReportData(output, test.Config, false, true); err != nil {
t.Fatalf("failed to get dataflow output %s", err)
return
}
Expand Down
2 changes: 1 addition & 1 deletion internal/report/output/dataflow/risks/risks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func TestDataflowRisks(t *testing.T) {
return
}

if err = dataflow.AddReportData(output, test.Config, false); err != nil {
if err = dataflow.AddReportData(output, test.Config, false, true); err != nil {
t.Fatalf("failed to get dataflow output %s", err)
return
}
Expand Down
8 changes: 6 additions & 2 deletions internal/report/output/detectors/detectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ import (
"github.com/bearer/bearer/internal/util/output"
)

func AddReportData(reportData *types.ReportData, report globaltypes.Report, config settings.Config) error {
if !config.Scan.Quiet {
func AddReportData(
reportData *types.ReportData,
report globaltypes.Report,
config settings.Config,
) error {
if !config.Scan.Quiet && report.HasFiles {
output.StdErrLog("Running Detectors")
}

Expand Down
13 changes: 9 additions & 4 deletions internal/report/output/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ func GetData(
case flag.ReportDataFlow:
return data, err
case flag.ReportSecurity:
err = security.AddReportData(data, config, baseBranchFindings)
err = security.AddReportData(data, config, baseBranchFindings, report.HasFiles)
case flag.ReportSaaS:
if err = security.AddReportData(data, config, baseBranchFindings); err != nil {
if err = security.AddReportData(data, config, baseBranchFindings, report.HasFiles); err != nil {
return nil, err
}
err = saas.GetReport(data, config, false)
Expand All @@ -81,7 +81,12 @@ func UploadReportToCloud(report *types.ReportData, config settings.Config) {
}
}

func GetDataflow(reportData *types.ReportData, report globaltypes.Report, config settings.Config, isInternal bool) error {
func GetDataflow(
reportData *types.ReportData,
report globaltypes.Report,
config settings.Config,
isInternal bool,
) error {
if reportData.Detectors == nil {
if err := detectors.AddReportData(reportData, report, config); err != nil {
return err
Expand All @@ -90,7 +95,7 @@ func GetDataflow(reportData *types.ReportData, report globaltypes.Report, config
for _, detection := range reportData.Detectors {
detection.(map[string]interface{})["id"] = uuid.NewString()
}
return dataflow.AddReportData(reportData, config, isInternal)
return dataflow.AddReportData(reportData, config, isInternal, report.HasFiles)
}

func FormatOutput(
Expand Down
27 changes: 22 additions & 5 deletions internal/report/output/security/security.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,18 @@ func AddReportData(
reportData *outputtypes.ReportData,
config settings.Config,
baseBranchFindings *basebranchfindings.Findings,
hasFiles bool,
) error {
dataflow := reportData.Dataflow
summaryFindings := make(Findings)
ignoredSummaryFindings := make(IgnoredFindings)
reportData.FindingsBySeverity = summaryFindings
reportData.IgnoredFindingsBySeverity = ignoredSummaryFindings

if !hasFiles {
return nil
}

dataflow := reportData.Dataflow
if !config.Scan.Quiet {
output.StdErrLog("Evaluating rules")
}
Expand All @@ -102,8 +110,6 @@ func AddReportData(
)
}

reportData.FindingsBySeverity = summaryFindings
reportData.IgnoredFindingsBySeverity = ignoredSummaryFindings
reportData.ReportFailed = builtInFailed || failed
return nil
}
Expand Down Expand Up @@ -180,17 +186,19 @@ func evaluateRules(
sortByLineNumber(policyFailures)

for i, output := range policyFailures {
instanceID := instanceCount[output.Filename]
instanceCount[output.Filename]++

if baseBranchFindings != nil &&
baseBranchFindings.Consume(rule.Id, output.Filename, output.Sink.Start, output.Sink.End) {
continue
}

fingerprintId := fmt.Sprintf("%s_%s", rule.Id, output.Filename)
oldFingerprintId := fmt.Sprintf("%s_%s", rule.Id, output.FullFilename)
fingerprint := fmt.Sprintf("%x_%d", md5.Sum([]byte(fingerprintId)), instanceCount[output.Filename])
fingerprint := fmt.Sprintf("%x_%d", md5.Sum([]byte(fingerprintId)), instanceID)
oldFingerprint := fmt.Sprintf("%x_%d", md5.Sum([]byte(oldFingerprintId)), i)

instanceCount[output.Filename]++
fingerprints = append(fingerprints, fingerprint)
rawCodeExtract := codeExtract(output.FullFilename, output.Source, output.Sink)
codeExtract := getExtract(rawCodeExtract)
Expand Down Expand Up @@ -361,6 +369,15 @@ func getExtract(rawCodeExtract []file.Line) string {
func BuildReportString(reportData *outputtypes.ReportData, config settings.Config, lineOfCodeOutput *gocloc.Result) *strings.Builder {
reportStr := &strings.Builder{}

if len(reportData.Files) == 0 {
reportStr.WriteString(
"\ncouldn't find any files to scan in the specified directory, " +
"for diff scans this can mean the compared branches were identical",
)

return reportStr
}

reportStr.WriteString("\n\nSecurity Report\n")
reportStr.WriteString("\n=====================================")

Expand Down
Loading

0 comments on commit 35c287c

Please sign in to comment.