search

tylerwince / flake8-bandit

Automated security testing using bandit and flake8.
MIT License
111 stars 23 forks source link

fix: refactor from ConfigFileFinder to load_config #35

Closed lsorber closed 2 years ago

lsorber commented 2 years ago

Attempt at fixing https://github.com/tylerwince/flake8-bandit/issues/33 by refactoring from flake8<5.0.0's ConfigFileFinder to flake8>=5.0.0,<6.0.0's load_config.

calumy commented 2 years ago

This change now requires a .bandit file to be added; otherwise, this error is raised Unable to parse config file: The specified config file does not exist: .bandit. The Bandit docs suggest that the .bandit file is optional.

Is this change intentional, or is there a way that this can be avoided?

If this file is required, it might be worth updating the README.md to reflect this.

lsorber commented 2 years ago

@CalumY Apologies for the misleading output on stderr. A .bandit config file was and still is optional after this PR.

The message you're referring to was originally intended for when the [bandit] section of the .bandit file is missing, but this PR would also (erroneously) cause that message to be printed if the .bandit file itself is missing. This is entirely harmless. I've just pushed a commit that removes the message. Now, if the .bandit file is missing or not correctly formatted, its contents (if any) will silently be ignored.

calumy commented 2 years ago

@lsorber Thanks for sorting this!

calumy commented 2 years ago

Another possible way of sorting this would be to update the if statement to ignore the file not found error along with the unable to parse config file error. This would still allow an error when the [bandit] section of the .bandit file is missing as was originally intended but ignore the error if the .bandit file is missing altogether. 

if str(e) != "No section: 'bandit'" and str(e) != "The specified config file does not exist: .bandit":
    sys.stderr.write(f"Unable to parse config file: {e}")
lsorber commented 2 years ago

@CalumY Sure, that could be done, but OTOH I'm not sure it's this plugin's responsibility to verify the validity of the .bandit file. For example, the [bandit] section could exist but one of its settings may be misspelled. I'll leave it up to @tylerwince to decide on what to do with this.

davfsa commented 2 years ago

@tylerwince would it be possible to merge this and make a new release?