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

default config file name #318

Open
davidak opened this issue Jun 18, 2018 · 22 comments
Open

default config file name #318

davidak opened this issue Jun 18, 2018 · 22 comments
Labels
enhancement New feature or request

Comments

@davidak
Copy link

davidak commented Jun 18, 2018

Is your feature request related to a problem? Please describe.
The doc say i should create a YAML config file, but not how to name it. https://bandit.readthedocs.io/en/latest/config.html

Describe the solution you'd like
Please recommend a sane default name, so it is consistent in any project and can be found by CI etc.

I recommend using .bandit.yml because it is hidden on Linux, UNIX and macOS and has an extension.

Describe alternatives you've considered
I don't see any alternatives. Leaving it as is leads to chaos!

Additional context
Codacy says: "You can also use custom .bandit or bandit.yml config file."
I have also seen bandit.yaml in earlier issues.

For a sane solution, i look what similar tools do.

Most .name for INI-style config or .name.yml for YAML config.

pylintrc or .pylintrc: https://pylint.readthedocs.io/en/latest/user_guide/run.html#command-line-options

.flake8

.pycodestyle or config in setup.cfg or tox.ini: http://pycodestyle.pycqa.org/en/latest/intro.html#configuration

.pydocstyle, .pydocstyle.ini, .pydocstylerc, .pydocstylerc.ini: http://www.pydocstyle.org/en/2.1.1/usage.html#configuration-files

.coveragerc

.travis.yml, .circleci, .github, .appveyor.yml

@davidak
Copy link
Author

davidak commented Jun 18, 2018

bandit.yaml is specified in https://github.com/PyCQA/bandit/blob/master/bandit/cli/main.py#L31 but get not loaded, also no hint in readme, doc or help output, so i supposed there is no default... is it an error that the file get not loaded or a feature?

@lukehinds
Copy link
Member

lukehinds commented Jun 19, 2018

bandit.yaml is commonly used in OpenStack, but as it stands its ultimately up to the user by means of the -c arg.

Having said that, I am not strongly opposed to using a recommended default of .bandit.yml in the docs as long as its clear its a recommended naming convention and not a direct map to bandits chosen config (otherwise folks will assume their bandit config will auto load as they used the default naming convention).

@lukehinds lukehinds added the enhancement New feature or request label Jun 19, 2018
@davidak
Copy link
Author

davidak commented Jun 19, 2018

folks will assume their bandit config will auto load

i think such behavior was present in an earlyer version. why was that changed?

@lukehinds
Copy link
Member

lukehinds commented Jun 19, 2018

I have no recollection of there ever been a default, its always been None as far as I remember:

action='store', default=None, type=str,

@xuhcc
Copy link

xuhcc commented Dec 9, 2018

There is a section in readme file which suggests using a .bandit file:

https://github.com/PyCQA/bandit/blob/master/README.rst#per-project-command-line-args

To use this, put a .bandit file in your project's directory.

This sounds like .bandit is a default name for config file.

@rooterkyberian
Copy link

I find it tremendously useful when a tool has a well known default config filename, since I can quickly see if project is using a particular tool, and what proves even more useful: search thru github repositories by filename (e.g. filename:.bandit.yaml) to see different, working configurations example.

@lukehinds
Copy link
Member

ok, if someone can make a patch making bandit.yml the default, please go ahead.

But it's imperative we keep the -e arg present for those who have already implemented their own config naming.

@ericwb
Copy link
Member

ericwb commented Mar 1, 2019

.bandit is for command line options. It is a INI file format, not YAML.

bandit.yml or whatever *.yml a user passes to -c is for more extensive customization of test plugins.

Please do not try to combine or change the naming. If anything, the documentation should be made more clear as evident by the comments in this issue.

@rooterkyberian
Copy link

Whats is wrong with combining the two in single yml? as you can see it is confusing for the users (myself included) and trying to fix that with additional documentation when combining the two seems to be

  • simpler (1 file, with a default name, instead of 2 with some loose naming recommendations)
    • default filename makes it much simpler to search for on github as well for "most common configuration options"
  • reduced amount documentation required to read (&maintain)
  • will be backwards compatible
    • .bandit (ini) users will still be able to use it as they did in the past
    • YAML config users shouldn't have conflicting keys in the first place

What are the drawbacks that I'm not seeing that out weight the benefits?

@bittner
Copy link
Contributor

bittner commented Mar 14, 2019

I'd prefer to see a [bandit] section read from my tox.ini file, as do other tools (like pytest, flake8, behave, etc.), which can help to reduce clutter in Python projects.

Also, the options available in a configuration file need to be fully documented, ideally also printed by the CLI --help option (or so). See also #396.

@bittner
Copy link
Contributor

bittner commented May 10, 2019

@ericwb, can you explain what "distant future" is intended to mean? It sounds a bit like this request is meant to die slowly. Personally, I'd prefer to see:

  • either close this issue with "won't fix" if you don't want it in the code base,
  • or say, "a PR is welcome if it implements (this and that)".

@bsolomon1124
Copy link

Was there ever a PR that implemented this?

As of 8127716, looks like no:

# https://github.com/PyCQA/bandit/blob/8127716355048540e4baf6e1f9735583008c8714/bandit/cli/main.py#L158
parser.add_argument(
    '-c', '--configfile', dest='config_file',
    action='store', default=None, type=str,
    help='optional config file to use for selecting plugins and '
         'overriding defaults'
)

And yet the README instructs:

To use this, put a .bandit file in your project's directory.

To use what? Make a .bandit file that is ignored by default?

@jdrowne
Copy link

jdrowne commented Jan 26, 2021

Bump. As of this writing, documentation does not say the config file must be explicitly set via -c.

@lcnittl
Copy link

lcnittl commented Feb 21, 2022

Also jumping in here: What is the default name for YAML style supposed to be?

We are planning to place this file in a few dozens of repos – for now forcedly with --config option. We want to drop --config after the (default named) config file becomes loaded by default – and I would prefer to rather not have to rename that file again in the near future.

Therefore, big +1 for #318 (comment)

Any (official) comment is highly appreciated!

Cheers, lcnittl

@calliecameron
Copy link

Trying out a few things, looks like the current behaviour is:

  • if you specify a file to check e.g. bandit foo.py, no default config is used.
  • if you specify a folder to check e.g. bandit -r ., it uses .bandit in the specified directory, which it expects to be an INI file.

But the docs say .bandit is supposed to be a YAML file, and don't mention INI files at all.

Would be good to use .bandit by default in the single-file case too, since that's how e.g. vscode's linter calls bandit by default.

@bittner
Copy link
Contributor

bittner commented Feb 27, 2022

According to the source code there are two different files in play:

  1. An INI-style confiiguration file (see bandit.cli.main, lines 50+), which defaults to .bandit. It handles the general CLI options (skipped and included tests, targets, recursive, profile, etc.)
  2. The scanning behavior configuration file (see bandit,core.config, lines 43+), with no default name, which can be either a TOML or a YAML file. This is the file which is suggested to generate by what is described in the documentation.

Both the docs and the docstrings in the source code are in parts out of sync with the implementation. This has been criticized with several issues in the past. Information about using the INI-style configuration is mostly found in issues that report problems of use.

The docs originally covered only the scanning configuration, and the INI-style configuration was exclusively mentioned in the project README. This has recently been consolidated (#773), but more work needs to be done to make the documentation usable. Anyone who can read code and edit .rst files can contribute to improve the situation.

The confusion in the documentation is also reflected in the code. The project comes from sysadmin efforts at OpenStack and could do with some refactoring that makes the code more elegant, straight-forward and easier to understand and maintain. In my eyes, it may also make sense to consolidate the configuration into a single configuration file (the now popular pyproject.toml might be a good candidate for a default). Again, contributions will probably make the difference.

@acdha
Copy link

acdha commented Mar 17, 2022

@bittner Thank you for digging into that — I had wasted a bit of time on the YAML-formatted .bandit the documentation misleadingly refers to, found #401 but missed that pyproject.toml is ignored by default, and then realized that my INI-formatted .bandit settings were being overwritten because GitLab SaST tool adds the -x flag using an environmental variable (SAST_BANDIT_EXCLUDED_PATHS).

Strong +1 for enabling pyproject.toml by default or #736.

@bittner
Copy link
Contributor

bittner commented Mar 18, 2022

@acdha As a side note, if you have a GitLab Ultimate license I recommend using GitLab's SAST integration. Their scanning container that has Bandit on board does a parsing and transformation of Bandit's scan report. Only this way GitLab will correctly recognize the report content. You won't get the details in GitLab's Security dashboard, otherwise.

@smsearcy
Copy link

@bittner GitLab is the reasons I was looking for a default configuration file and ended up here. I want to disable the assert_used checks for our pytest files, but without a default configuration file to configure scanning I don't see how I can configure how GitLab behaves.

@arischow
Copy link

arischow commented Jul 1, 2022

My workaround: exclude the whole tests path from bandit by configuring the variable SAST_BANDIT_EXCLUDED_PATHS in .gitlab-ci.yml.

Reference: https://docs.gitlab.com/ee/user/application_security/sast/#vulnerability-filters

@bersbersbers
Copy link

bersbersbers commented Sep 13, 2023

What would help tremendously is if the .bandit INI file understood the --configfile parameter like so:

[bandit]
configfile = pyproject.toml

This would solve so many problems at once:

  • As long as they are using -r, people would not have to use -c anywhere.
  • They could still freely choose the name of the config file, without having to have a default config file name at all.
  • It's 100% backwards compatible since config is not parsed as of today. I understand that "breaking people's CI" is a huge concern, and AFAICS, this approach would not do that at all.

@bersbersbers
Copy link

Whoever is active in this repository might take a look at my PR which has been sitting waiting for a review for some 9 months :)

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

No branches or pull requests