Checkmarx / ast-cli

A CLI project wrapping application security testing (AST) APIs
Apache License 2.0
37 stars 26 forks source link

[BUG] No report generated when thresholds fail the scan #415

Closed Thomgrus closed 2 years ago

Thomgrus commented 2 years ago

Describe the bug

Introduced in version 2.0.19

When we use scan command with

There is no report generated but in the case where the scan passed there is report.

Expected behavior

I think report must be generated even if scan fail because of thresholds like in version 2.0.18.

Actual behavior

Unlike in version 2.0.18 the scan will not generate report if thresholds fail the scan.

Steps to reproduce

  1. Create a project with a security issue SAST High for example
  2. Run a scan with --report-format summaryJSON and --threshold sast-high=0 options
  3. Notice there is no cx_result.json generated in .
  4. Re run the scan without --threshold sast-high=0 or after SAST High issue fixed
  5. Notice there is a cx_result.json generated in .

Environment

Additional comments

Here is my analyse:

The BUG was introduced in 2.0.19 in: | Ast 12482 add summary html for async scans (https://github.com/Checkmarx/ast-cli/pull/405)

The file internal/commands/scan.go was modified and the createReportsAfterScan method call was moved from the handleWait method to the end of the runCreateScanCommand method.

The problem here is the block

err = applyThreshold(cmd, resultsWrapper, scanResponseModel)
if err != nil {
    return err
}

before the call of createReportsAfterScan.

Here is the lines: https://github.com/Checkmarx/ast-cli/blob/c7a33215fbde596638016f29737e8d2a395c9563/internal/commands/scan.go#L1102-L1121

To fix it we may use this code:

AsyncFlag, _ := cmd.Flags().GetBool(commonParams.AsyncFlag)
if !AsyncFlag {
    waitDelay, _ := cmd.Flags().GetInt(commonParams.WaitDelayFlag)
    err = handleWait(cmd, scanResponseModel, waitDelay, timeoutMinutes, scansWrapper, resultsWrapper)
    if err != nil {
        return err
    }

    err = createReportsAfterScan(cmd, scanResponseModel.ID, scansWrapper, resultsWrapper)
    if err != nil {
        return err
    }

    err = applyThreshold(cmd, resultsWrapper, scanResponseModel)
    if err != nil {
        return err
    }
} else {
    err = createReportsAfterScan(cmd, scanResponseModel.ID, scansWrapper, resultsWrapper)
    if err != nil {
        return err
    }
}

return nil

Logs

2022/06/20 09:02:55 Scan status:  Running
2022/06/20 09:03:01 Scan status:  Running
2022/06/20 09:03:06 Scan Finished with status:  Completed
Threshold check finished with status Failed : sast-high: Limit = 0, Current = 1 |
Program exits with code:  1
Scan Failed
alex46300 commented 2 years ago

Thanks for the report ! I have the same problem !

pedrompflopes commented 2 years ago

Hi @Thomgrus Thanks for your awesome analysis. We will include this in the next releases.

Next time, please feel free to open a PR with your change.

Thanks!

Thomgrus commented 2 years ago

Hi @pedrompflopes,

Thank you, i can do a PR with this changes, references this issue ; no problem.

Ok i'll do it next time 🙂 (i just followed the documentation https://github.com/Checkmarx/ast-cli/blob/main/docs/contributing.md#getting-started)

Thanks !