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

Add option to control the exit code #424

Closed
norg opened this issue Dec 7, 2022 · 9 comments
Closed

Add option to control the exit code #424

norg opened this issue Dec 7, 2022 · 9 comments

Comments

@norg
Copy link

norg commented Dec 7, 2022

Is your feature request related to a problem? Please describe.

If you run pip-audit in a CI pipeline and want to further use the output, the pipeline fails if a vulnerability is found. Sometimes you want to use that output in a further pipeline step, thus the exit code 1 prevents that.

Describe the solution you'd like

A command line option to pass to control the exit code, maybe something like --no-fail or so if you don't want the exit code 1 if vulnerabilities are found.

Describe alternatives you've considered

The alternative is to workaround that in the CI pipeline. The issue with that is, that you might want to have still exit code 1 if pip-audit fails for a reason and the pipeline should not proceed.

Additional context

This was mentioned in #99 (comment) and while the workaround could be fine in some cases, it's far from ideal since it would also skip if pip-audit fails due to other issues or bugs.

Some other audit-tools provide such an option, for example npm audit uses the --audit-level to define the exit code.

@norg norg added the enhancement New feature or request label Dec 7, 2022
@woodruffw
Copy link
Member

woodruffw commented Dec 7, 2022

Hi @norg! Thanks for making this feature request.

I think we still want to shy away from making pip-audit's error code behavior configurable, since there's a common shell idiom for handling this case already (mentioned in #99 (comment)):

pip-audit || true

or, if you simply want to exit while swallowing the error:

pip-audit || exit 0

If you need the precise exit code while also continuing (which won't mean much for pip-audit, since it's always 1), you can do something like this:

pip-audit
exitcode="${?}"
if [[ "${exitcode}" -ne 0 ]]; then
    echo "pip-audit failed with: ${?}"
fi

(You may need set -e for that, depending on how your shell has been initialized.)

I hope one of those works for your use case. Separately, if you're using pip-audit in a CI, our gh-action-pip-audit action may be of interest to you (either as a reference or for direct use).

@woodruffw woodruffw removed the enhancement New feature or request label Dec 7, 2022
@norg
Copy link
Author

norg commented Dec 7, 2022

If you need the precise exit code while also continuing, you can do something like this:

if ! pip-audit; then
    echo "pip-audit failed with: ${?}"
fi

The issue with that workarounds is, that you swallow all errors. So for example you have exit code 1 if a vulnerability is found but you also have exit code 1 if there was a runtime error like pip audit not being able to download a specific dependency or timeout towards the database.

So for example this error should still be caught:

pip_audit._virtual_env.VirtualEnvError: Failed to install packages: ['/tmp/tmpgzjk0f__/bin/python3', '-m', 'pip', 'install', '/tmp/tmpypsxw7fq/foobar-1.2.3.tar.gz']

While I want to "ignore" the exit code 1 for the vulnerability found since I postprocess the json output file, I want to stop the CI when there is another error like the mentioned one. So it depends on WHY the exit code 1 appeared. In one case pip audit itself did run as expected (with the result of vulns found) and returns 1, in other cases there was an issue with pip audit itself and that I still want to catch and stop further processing.

@woodruffw
Copy link
Member

That's a reasonable point.

I still don't think that we want to make errors silence-able from within pip-audit itself, but maybe we can help your use case by distinguishing the error codes here.

@di how would you feel about us setting aside another exit code, one that indicates an error in pip-audit itself rather than a failed audit? One hiccup here is that Python's default unhandled exception exit code is already 1, so it might make sense to re-define "audit failed" as 2.

@di
Copy link
Member

di commented Dec 7, 2022

Sorry for the pushback -- we really don't want something like this to become an easy footgun for people and wind up in a situation where they think they're performing an audit and catching the result, but they aren't.

So it depends on WHY the exit code 1 appeared. In one case pip audit itself did run as expected (with the result of vulns found) and returns 1, in other cases there was an issue with pip audit itself and that I still want to catch and stop further processing.

If you're post-processing the output file, wouldn't the presence of the file (or lack thereof) be enough to determine if the invocation of pip-audit was successful or not?

@norg
Copy link
Author

norg commented Dec 7, 2022

Sorry for the pushback -- we really don't want something like this to become an easy footgun for people and wind up in a situation where they think they're performing an audit and catching the result, but they aren't.

I see your point although I would think if it's a dedicated commandline option that a user has to set explicitly, that this should be enough to avoid the confusion. Especially since it is something other audit tools do as well, for those scenarios. So only users that use this option can run into trouble and they did it on purpose.
(like other options, set it to false by default)

One other argument for that is, that you already support ignoring specific vuln IDs and you also give the option --strict to users as well :)

So it depends on WHY the exit code 1 appeared. In one case pip audit itself did run as expected (with the result of vulns found) and returns 1, in other cases there was an issue with pip audit itself and that I still want to catch and stop further processing.

If you're post-processing the output file, wouldn't the presence of the file (or lack thereof) be enough to determine if the invocation of pip-audit was successful or not?

I agree that helps for the postprocess and error out at that point. But what if the file was written although there was a different error in the pip-audit run?

Don't get me wrong, I just think that ignoring all exit code 1 scenarios with the workaround above is worse compared to adding an option to set one specific scenario to 0 instead of 1. This would not break any current setups and just give an option to users that have this specific requirement.

@norg
Copy link
Author

norg commented Dec 12, 2022

@di just a small update on your suggestion. This works fine for some scenarios but I had a run in CI where pip-audit ran into a timeout and ended with a traceback Errno 104 but still created the file. At least it's also easy to check if the file is empty, but wondering if there could be scenarios where the file is written, has partial content but would still be non valid json. So another validation wrapper would be required.

@woodruffw
Copy link
Member

At least it's also easy to check if the file is empty, but wondering if there could be scenarios where the file is written, has partial content but would still be non valid json. So another validation wrapper would be required.

Writing partial content should never happen, but I agree that the "empty file" case is a bug on our part. I'm going to file an issue to track that 🙂

@woodruffw
Copy link
Member

#431 is tracking the empty file bug.

@irishismyname
Copy link

irishismyname commented Jun 5, 2023

I realize this is closed, but I do think something simple like a --ci option that yields a normal exit code when a vulnerability is found would be really nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants