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 numeric options for severity and confidence #702

Conversation

nathanstocking
Copy link
Contributor

Adds two new command line arguments which allow the user to specify
severity level and confidence level with a key-value pair rather than
repeating a flag. This makes it easier to specify those values if using
an alternate interface which invokes Bandit's CLI. The previous
repeatable flags have been retained and existing workflows will not be
affected.

New arguments:

  • --severity-level: Takes an integer from 1 to 4 to set the level (1
    for undefined, 2 for low, 3 for medium, 4 for high). This has the same
    effect as the existing -l/--level option. If both options are specified,
    an error will be printed.

  • --confidence-level: Takes an integer from 1 to 4 to set the level (1
    for undefined, 2 for low, 3 for medium, 4 for high). This has the same
    effect as the existing -i/--confidence option. If both options are
    specified, an error will be printed.

@sigmavirus24
Copy link
Member

Thanks so much for taking the initiative on this. The approach with using mutually exclusive groups looks right to me. I'm wondering if the numbers are the right thing to use instead of a string (text). I'm thinking that --severity-level=MEDIUM (or all lower-case) is going to be far easier on users than --severity-level=3 (what even is 3?). We can translate things on the backend to make things nicer for humans.

I'd also switch from calling 1 "UNDEFINED" to "ALL" but it would also be good to call out (if we're presenting that as an option) how it differs from low.

@nathanstocking
Copy link
Contributor Author

Thanks so much for taking the initiative on this. The approach with using mutually exclusive groups looks right to me. I'm wondering if the numbers are the right thing to use instead of a string (text). I'm thinking that --severity-level=MEDIUM (or all lower-case) is going to be far easier on users than --severity-level=3 (what even is 3?). We can translate things on the backend to make things nicer for humans.
Good idea. I will modify it to do that instead. At the moment, I'm planning to use lowercase options only since argparse doesn't make it easy to allow case-insensitive options in the choices.
I'd also switch from calling 1 "UNDEFINED" to "ALL" but it would also be good to call out (if we're presenting that as an option) how it differs from low.
We could just remove it since undefined is the default option, but since it's theoretically possible for a finding to be sorted as undefined, I'm inclined to keep the option. I'll use "all" as the name and attempt to clarify what this means.

@nathanstocking
Copy link
Contributor Author

The latest commit implements the string options as suggested.

Thanks so much for taking the initiative on this. The approach with using mutually exclusive groups looks right to me. I'm wondering if the numbers are the right thing to use instead of a string (text). I'm thinking that --severity-level=MEDIUM (or all lower-case) is going to be far easier on users than --severity-level=3 (what even is 3?). We can translate things on the backend to make things nicer for humans.
Good idea. I will modify it to do that instead. At the moment, I'm planning to use lowercase options only since argparse doesn't make it easy to allow case-insensitive options in the choices.
I'd also switch from calling 1 "UNDEFINED" to "ALL" but it would also be good to call out (if we're presenting that as an option) how it differs from low.
We could just remove it since undefined is the default option, but since it's theoretically possible for a finding to be sorted as undefined, I'm inclined to keep the option. I'll use "all" as the name and attempt to clarify what this means.

@sigmavirus24
Copy link
Member

Why did you close this?

@nathanstocking
Copy link
Contributor Author

Why did you close this?
Sorry, I must have pressed the wrong button when posting the comment. The change has been implemented.

@nathanstocking nathanstocking reopened this Apr 1, 2021
Adds two new command line arguments which allow the user to specify
severity level and confidence level with a key-value pair rather than
repeating a flag. This makes it easier to specify those values if using
an alternate interface which invokes Bandit's CLI. The previous
repeatable flags have been retained and existing workflows will not be
affected.

New arguments:

 * --severity-level: Takes a string "all", "low", "medium", or "high" to set the level. This has the same
 effect as the existing -l/--level option. If both options are specified,
 an error will be printed.

 * --confidence-level: Takes a string "all", "low", "medium", or "high" to set the level.
 This has the same effect as the existing -i/--confidence option. If both options are
 specified, an error will be printed.

 * Help text for these parameters clarifies why 'all' and 'low' aren't
 the same although they will almost certainly produce the same set of results.
@nathanstocking nathanstocking force-pushed the dev/ns/numeric_severity_and_confidence branch from 3c4b378 to b3cff1a Compare April 1, 2021 19:40
@lukehinds lukehinds merged commit 1eff509 into PyCQA:master Apr 2, 2021
mikespallino pushed a commit to mikespallino/bandit that referenced this pull request Jan 7, 2022
Adds two new command line arguments which allow the user to specify
severity level and confidence level with a key-value pair rather than
repeating a flag. This makes it easier to specify those values if using
an alternate interface which invokes Bandit's CLI. The previous
repeatable flags have been retained and existing workflows will not be
affected.

New arguments:

 * --severity-level: Takes a string "all", "low", "medium", or "high" to set the level. This has the same
 effect as the existing -l/--level option. If both options are specified,
 an error will be printed.

 * --confidence-level: Takes a string "all", "low", "medium", or "high" to set the level.
 This has the same effect as the existing -i/--confidence option. If both options are
 specified, an error will be printed.

 * Help text for these parameters clarifies why 'all' and 'low' aren't
 the same although they will almost certainly produce the same set of results.

Co-authored-by: Nathan Stocking <nathan.stocking@microsoft.com>
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

Successfully merging this pull request may close these issues.

None yet

3 participants