Add default ini file + enable recursive option in ini #503

Babarberousse · 2019-06-08T22:26:02Z

I think making recursive a default would make sense. However it could break some ci pipelines so maybe in a future PR.

Also, I made .bandit the default ini file, which means users who do not specify an ini file and don't have a .bandit file will now get a warning. Not sure we should keep it.

ericwb

This change results in an warning message every time for the typical case where bandit is run with no .bandit file, yet does provide a target.

[utils]	WARNING	Unable to parse config file .bandit or missing [bandit] section

I'd rather not see warning messages when the arguments given to the CLI are correct and complete.

bittner · 2019-10-10T07:45:17Z

@Babarberousse Can you rebase your PR to make it ready for merging?

Also, you may address @ericwb's concern and remove the chatty warning. We may still have that, but maybe show it only with --verbose or --debug or so.

bittner · 2019-10-10T07:59:57Z

bandit/cli/main.py

@@ -246,7 +246,7 @@ def main():
                           '(only JSON-formatted files are accepted)'
    )
    parser.add_argument(
-        '--ini', dest='ini_path', action='store', default=None,
+        '--ini', dest='ini_path', action='store', default='.bandit',


This limits the default to just a single possibility.

Other popular tools, such as flake8, behave, etc., allow the configuration to be discovered from various popular configuration files (to save the project home directory from being polluted with numerous configuration files).

I tend to place all configuration into a tox.ini file, for example. This makes sense as Tox is my main driver and single point of entry for running all tests and checks on Python projects.

bittner · 2019-10-16T08:05:44Z

bandit/cli/main.py

+
+    :return: the filename as a string or `None` if no ini file found
+    """
+    for filename in [".bandit", "setup.cfg", "tox.ini"]:


You are awesome! 🥇

bittner · 2019-10-16T08:17:30Z

This change results in an warning message every time for the typical case where bandit is run with no .bandit file, yet does provide a target.
[utils]	WARNING	Unable to parse config file .bandit or missing [bandit] section
I'd rather not see warning messages when the arguments given to the CLI are correct and complete.

@ericwb This seems fixed now.

Babarberousse · 2019-10-17T22:49:14Z

I think there are still a few problems, although not related to this pull request. The recursive parameter is set to False by default when parsing empty CLI args. Given that CLI args prevail over ini files, this means recursivity cannot be set through ini file. I think recursivity argument could be a store_const=True instead of a store_true so it is set to None without CLI args.

Also, there's a bit of redundancy between my find_ini_files function and the _get_options_from_ini function.

I'll try to look at these issues this week-end and try to make it a bit cleaner.

Babarberousse · 2019-10-20T22:24:32Z

I went a bit further than what my initial PR was supposed to be and:

Made recursivity true as a default
fixed the issue with the broken bool params in the ini files. The default values for bool params are not set with argparse anymore as it would have prevented the ini file from overriding these default values. I suspect some non-bool params still don't work although I haven't tested them (aggregates, level, confidence,...)
Abstracted some existing argparse logic into new functions.
Added support for setup.cfg and tox.ini
Removed parser.set_defaults lines as I couldn't figure out what they were meant to do. Please confirm you are ok with this removal.

bittner · 2022-01-10T00:36:53Z

@Babarberousse could you rebase this PR and try to get it merged? 🎸 🚀

This still seems like a desperately needed UX improvement to me.

bittner · 2022-04-03T21:51:16Z

Note to self: This PR would address issues #318 and #736.

Babarberousse · 2022-04-28T07:56:03Z

I don't have much time at the moment but I'll try to get it done in the next few weeks.

Fixed bool params not working in ini file

Babarberousse · 2022-05-26T08:35:24Z

I went a bit further than what my initial PR was supposed to be and:

Made recursivity true as a default

fixed the issue with the broken bool params in the ini files. The default values for bool params are not set with argparse anymore as it would have prevented the ini file from overriding these default values. I suspect some non-bool params still don't work although I haven't tested them (aggregates, level, confidence,...)

Abstracted some existing argparse logic into new functions.

Added support for setup.cfg and tox.ini

Removed parser.set_defaults lines as I couldn't figure out what they were meant to do. Please confirm you are ok with this removal.

I kept recursivity as false as a default and kept the parser.set_defaults() lines to reduce the scope of this PR.

Babarberousse force-pushed the add_recursivity branch 5 times, most recently from de54e38 to ccb9da0 Compare June 9, 2019 01:56

Babarberousse force-pushed the add_recursivity branch from ccb9da0 to 5c9a2f6 Compare June 23, 2019 20:54

ericwb requested changes Aug 18, 2019

View reviewed changes

bittner reviewed Oct 10, 2019

View reviewed changes

Babarberousse force-pushed the add_recursivity branch 5 times, most recently from 4bf77d6 to e61f86c Compare October 15, 2019 21:14

bittner reviewed Oct 16, 2019

View reviewed changes

bittner approved these changes Oct 16, 2019

View reviewed changes

Babarberousse force-pushed the add_recursivity branch 4 times, most recently from b2f927d to 8256521 Compare October 20, 2019 22:16

Babarberousse force-pushed the add_recursivity branch from 8256521 to a82aa79 Compare October 20, 2019 22:51

Babarberousse requested a review from ericwb October 23, 2019 15:16

Babarberousse force-pushed the add_recursivity branch from 0b45f74 to 1fd34cc Compare January 25, 2020 09:43

Babarberousse force-pushed the add_recursivity branch from 6c8d418 to a07bb75 Compare May 26, 2022 08:29

Babarberousse requested review from lukehinds and sigmavirus24 as code owners May 26, 2022 08:29

Babarberousse added 2 commits May 26, 2022 10:30

Added test for recursivity param

6fe4930

Added default ini files and targets

4374cb8

Fixed bool params not working in ini file

Babarberousse force-pushed the add_recursivity branch from a07bb75 to 4374cb8 Compare May 26, 2022 08:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add default ini file + enable recursive option in ini #503

Add default ini file + enable recursive option in ini #503

Babarberousse commented Jun 8, 2019

ericwb left a comment

bittner commented Oct 10, 2019

bittner Oct 10, 2019 •

edited

bittner Oct 16, 2019

bittner commented Oct 16, 2019

Babarberousse commented Oct 17, 2019

Babarberousse commented Oct 20, 2019 •

edited

bittner commented Jan 10, 2022

bittner commented Apr 3, 2022

Babarberousse commented Apr 28, 2022

Babarberousse commented May 26, 2022

Add default ini file + enable recursive option in ini #503

Are you sure you want to change the base?

Add default ini file + enable recursive option in ini #503

Conversation

Babarberousse commented Jun 8, 2019

ericwb left a comment

Choose a reason for hiding this comment

bittner commented Oct 10, 2019

bittner Oct 10, 2019 • edited

Choose a reason for hiding this comment

bittner Oct 16, 2019

Choose a reason for hiding this comment

bittner commented Oct 16, 2019

Babarberousse commented Oct 17, 2019

Babarberousse commented Oct 20, 2019 • edited

bittner commented Jan 10, 2022

bittner commented Apr 3, 2022

Babarberousse commented Apr 28, 2022

Babarberousse commented May 26, 2022

bittner Oct 10, 2019 •

edited

Babarberousse commented Oct 20, 2019 •

edited