Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Use exit codes between 0 and 255 (inclusive) #4511

Merged
merged 2 commits into from
Jan 11, 2023

Conversation

candrews
Copy link
Contributor

@candrews candrews commented May 18, 2022

Fixes Issue

Shells express exit codes in the range of 0-255. Anything outside of that range is brought into that range by modding it 256.

For example:
-1 becomes 255
-2 becomes 254
-3 becomes 253

This need to convert exit codes in shell scripts to use dependency check is unexpected and confusing.

For that reason, returning exit codes outside of 0-255 is unusual and generally discouraged.

See: https://tldp.org/LDP/abs/html/exit-status.html

Description of Change

Therefore, use exit codes in the 0-255 range by changing all negative exit codes to positive ones (ex, -1 becomes 1).

With this change, exit code 1 was used for both help and CVSS score failure. Therefore, the CVSS score failure exit code was changed to 15.

Have test cases been added to cover the new functionality?

no

@boring-cyborg boring-cyborg bot added the cli changes to the cli label May 18, 2022
@candrews candrews force-pushed the patch-1 branch 2 times, most recently from 16511c1 to 4859d79 Compare May 18, 2022 15:31
@aikebah
Copy link
Collaborator

aikebah commented May 18, 2022

The only handling of return code in a script in my view would be a check for success or failure, which would mean comparison of the exit code to 0 (SUCCESS) and potentially a print of the non-zero exit code received for diagnostic purposes.
That check would be transparent to the exit codes used.

https://unix.stackexchange.com/questions/418784/what-is-the-min-and-max-values-of-exit-codes-in-linux
shows that what exit code ranges are available depend on the shell

Valid exit codes available to POSIX compliant systems is the full range of int as per https://pubs.opengroup.org/onlinepubs/9699919799/functions/_exit.html / https://pubs.opengroup.org/onlinepubs/9699919799/functions/exit.html
(with typically the least significant byte being made available to shell scripts)

@candrews
Copy link
Contributor Author

The only handling of return code in a script in my view would be a check for success or failure

My use case is that I want to differentiate between 3 exit reasons when using the --failOnCVSS argument:

  1. Dependency check ran and found no vulnerabilities
  2. Dependency check ran and found vulnerability
  3. Dependency checked failed to run for some reason

With this MR, I'd check for exit codes 0, 11, and anything else, respectively.

https://unix.stackexchange.com/questions/418784/what-is-the-min-and-max-values-of-exit-codes-in-linux
shows that what exit code ranges are available depend on the shell

Agreed. Most users use bash, dash, or busybox sh - and all of those truncate to 0-255. And that furthers my point that using exit codes outside of the 0-255 range results in unexpected behavior which is why its generally recommended to stick to that range.

with typically the least significant byte being made available to shell scripts

"Typically" indicates inconsistent behavior, which is generally a good idea to avoid.

Shells express exit codes in the range of 0-255. Anything outside of that range is brought into that range by modding it 256.

For example:
-1 becomes 255
-2 becomes 254
-3 becomes 253

This need to convert exit codes in shell scripts to use dependency check is unexpected and confusing.

For that reason, returning exit codes outside of 0-255 is unusual and generally discouraged. Therefore, use exit codes in the 0-255 range.

With this change, exit code 1 was used for both help and CVSS score
failure. Therefore, the CVSS score failure exit code was changed to 15.

See: https://tldp.org/LDP/abs/html/exit-status.html
@aikebah
Copy link
Collaborator

aikebah commented May 18, 2022

@jeremylong what do you think. Looks reasonable to me, but think we should postpone integrating for an 8.x release as it will break existing returncode handling in scripted runs.

@jeremylong jeremylong added this to the 8.0.0 milestone May 25, 2022
@jeremylong
Copy link
Owner

I agree with the PR - but it would be a breaking change. This will get merged, but we might have a release or two happen before we move to 8.0.0.

@jeremylong jeremylong changed the title Use exit codes between 0 and 255 (inclusive) fix: Use exit codes between 0 and 255 (inclusive) Nov 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli changes to the cli
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants