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 anerror
andExitCode
value, then the cleanup code in the function, such as thelog.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 thecli.ExitCoder
interface, for example, an error is created usingerrors.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