Ensure consistent exit behaviour for secure analyzers

Summary

While reviewing update database automatically for every run in the gitlab-depscan project, we became aware of inconsistent exit behaviour with the urfave/cli package when an error occurs during execution of an analyzer.

Nearly all of the secure analyzer projects use a main() function similar to the following:

func main() {
  ... 

  if err := app.Run(os.Args); err != nil {
    log.Fatal(err)
  }
}

The problem encountered is within the app.Run() function call above:

  • If the error returned from app.Run() implements the cli.ExitCoder interface, in other words, it provides an error and ExitCode value, then the cleanup code in the function, such as the log.Fatal(err) line above, will not be executed.

    An example of such an error could be a failure encountered while trying to exec a scanner tool process using cmd.Output()

  • If the error returned from app.Run() does not implement the cli.ExitCoder interface, for example, an error is created using errors.New("an error occurred"), then the cleanup code in the function will be executed.

This behaviour is surprising and confusing, and I think for the sake of consistency and maintainability, we should update the secure analyzers so that control is always returned to the code that calls app.Run(). This will allow us to maintain control over the exit process and perform any necessary cleanup, such as simply logging the error message using log.Fatal(err).

Related issues

See this discussion for more detail background on this issue.

Improvements

Improved consistency, maintainability and less chance of bugs in the future.

Involved components

All secure analyzers

Edited by Adam Cohen