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

Bandit 1.6.0 no longer respects excluded directories #488

Closed
mattjegan opened this issue May 9, 2019 · 25 comments · Fixed by #489 or OperationCode/resources_api#158
Closed

Bandit 1.6.0 no longer respects excluded directories #488

mattjegan opened this issue May 9, 2019 · 25 comments · Fixed by #489 or OperationCode/resources_api#158
Labels
bug Something isn't working
Milestone

Comments

@mattjegan
Copy link

Describe the bug
Prior to the bandit 1.6.0 release, I was using bandit like so:

bandit -r . -x ./mymodule1/tests/,./mymodule2/tests/

However, with bandit 1.6.0 the ./mymodule1/tests/ and ./mymodule2/tests/ directories are included in the results. I have confirmed that you can exclude individual files still, just not directories.

To Reproduce
Steps to reproduce the behavior:

  1. Create a directory with a vulnerability in it somewhere
  2. Run bandir -r . -x ./your-new-dir/
  3. See vulnerability results that should be excluded

Expected behavior
Bandit should be excluding the directories entirely that are passed as args to the -x flag.

Bandit version

bandit 1.6.0
  python version = 3.6.5 (default, Jun 17 2018, 12:13:06) [GCC 4.2.1 Compatible Apple LLVM 9.1.0 (clang-902.0.39.2)]
@ericwb ericwb added the bug Something isn't working label May 9, 2019
@ericwb ericwb added this to the Release 1.6.1 milestone May 9, 2019
@ericwb
Copy link
Member

ericwb commented May 9, 2019

Most likely a regression caused by #450

@mattjegan
Copy link
Author

Thanks, I can try fix it in a couple hours

@Bobspadger
Copy link

I have also just run into this issue with 1.6.0 - my auto builds are now failing due to scanning excluded directories :)

@guywillett
Copy link

Likewise I have found that exclude: ./tests/ in my .bandit file no longer are respected.
I have tested several times with different paths and using exclude_dirs: instead of exclude: but to no avail.
A quick fix would be much appreciated! :-)

@PeterHamilton
Copy link

Yep, I just encountered this as well. Glad to see there's already an issue filed and a fix in the works. I'll temporarily downgrade to 1.5.1 until 1.6.1 is out.

@Bobspadger
Copy link

pinning to 1.5.1 worked out for me :)

keep up the good work!

wmfgerrit pushed a commit to wikimedia/operations-software-spicerack that referenced this issue May 9, 2019
* Due to a bug upstream bandit 1.6.0 doesn't honor the excluded
  directories, causing the failure of the bandit tox environments.
  See PyCQA/bandit#488
* Temproarily forcing bandit to be < 1.6.0 until the fix is available
  upstream.

Change-Id: I8627d2a4e0e4b7f604b4dea843b910bbc5d4de45
rbuysse added a commit to rbuysse/sawtooth-poet that referenced this issue May 9, 2019
Workaround for bug reported here: PyCQA/bandit#488

Signed-off-by: Ryan Beck-Buysse <rbuysse@bitwise.io>
rbuysse added a commit to rbuysse/sawtooth-poet that referenced this issue May 9, 2019
Workaround for bug reported here: PyCQA/bandit#488

Signed-off-by: Ryan Beck-Buysse <rbuysse@bitwise.io>
rbuysse added a commit to Cargill/sawtooth-core that referenced this issue May 9, 2019
Workaround for bug reported here: PyCQA/bandit#488

Signed-off-by: Ryan Beck-Buysse <rbuysse@bitwise.io>
@besnik
Copy link

besnik commented May 9, 2019

I confirm this bug.
Following works in 1.5 but fails with 1.6.0 (tests directory contains unit tests):
bandit --exclude tests -r .

openstack-gerrit pushed a commit to openstack/python-openstackclient that referenced this issue May 10, 2019
Bandit 1.6.0 introduces a regression[0] with the -x option, a fix
is expected to be included in 1.6.1 soon.

[0] PyCQA/bandit#488
[1] PyCQA/bandit#489

Change-Id: I110829ef960e3ee146f47871ef076491244bf4fa
Signed-off-by: Dean Troyer <dtroyer@gmail.com>
openstack-gerrit pushed a commit to openstack/openstack that referenced this issue May 10, 2019
* Update python-openstackclient from branch 'master'
  - Blacklist Bandit 1.6.0 due to directory exclusion bug
    
    Bandit 1.6.0 introduces a regression[0] with the -x option, a fix
    is expected to be included in 1.6.1 soon.
    
    [0] PyCQA/bandit#488
    [1] PyCQA/bandit#489
    
    Change-Id: I110829ef960e3ee146f47871ef076491244bf4fa
    Signed-off-by: Dean Troyer <dtroyer@gmail.com>
openstack-gerrit pushed a commit to openstack/neutron that referenced this issue May 10, 2019
Bandit 1.6.0 introduces a regression[0] with the -x option,
a fix is expected to be included in 1.6.1 soon.

[0] PyCQA/bandit#488
[1] PyCQA/bandit#489

Change-Id: Id944054deedd545c34fc28ccf043dd72e5f31220
openstack-gerrit pushed a commit to openstack/openstack that referenced this issue May 10, 2019
* Update neutron from branch 'master'
  - Blacklist bandit 1.6.0 due to directory exclusion bug
    
    Bandit 1.6.0 introduces a regression[0] with the -x option,
    a fix is expected to be included in 1.6.1 soon.
    
    [0] PyCQA/bandit#488
    [1] PyCQA/bandit#489
    
    Change-Id: Id944054deedd545c34fc28ccf043dd72e5f31220
openstack-gerrit pushed a commit to openstack/neutron-lib that referenced this issue May 10, 2019
Bandit 1.6.0 introduces a regression[0] with the -x option,
a fix is expected to be included in 1.6.1 soon.

[0] PyCQA/bandit#488
[1] PyCQA/bandit#489

Change-Id: Id29f06b68a95f53ad62bdc597bbb0f12bc4d6a60
openstack-gerrit pushed a commit to openstack/openstack that referenced this issue May 10, 2019
* Update neutron-lib from branch 'master'
  - Blacklist bandit 1.6.0 due to directory exclusion bug
    
    Bandit 1.6.0 introduces a regression[0] with the -x option,
    a fix is expected to be included in 1.6.1 soon.
    
    [0] PyCQA/bandit#488
    [1] PyCQA/bandit#489
    
    Change-Id: Id29f06b68a95f53ad62bdc597bbb0f12bc4d6a60
openstack-gerrit pushed a commit to openstack/keystone that referenced this issue May 11, 2019
There's a regression[0] in bandit 1.6.0 which causes bandit to stop
respecting excluded directories, and our tests throw a bunch of
violations. Blacklist this version, but allow newer versions as there is
already a pull request[1] to fix it, and I expect it will be included in
the next release.

[0] PyCQA/bandit#488
[1] PyCQA/bandit#489

Change-Id: Ie4dbfb3f54e4aac00e0537d5760b7a8fc81b35a2
openstack-gerrit pushed a commit to openstack/openstack that referenced this issue May 11, 2019
* Update keystone from branch 'master'
  - Blacklist bandit 1.6.0
    
    There's a regression[0] in bandit 1.6.0 which causes bandit to stop
    respecting excluded directories, and our tests throw a bunch of
    violations. Blacklist this version, but allow newer versions as there is
    already a pull request[1] to fix it, and I expect it will be included in
    the next release.
    
    [0] PyCQA/bandit#488
    [1] PyCQA/bandit#489
    
    Change-Id: Ie4dbfb3f54e4aac00e0537d5760b7a8fc81b35a2
openstack-gerrit pushed a commit to openstack/openstack that referenced this issue Jul 1, 2019
* Update networking-odl from branch 'master'
  - Blacklist bandit, bump neutron-lib and retire neutron-lbaas
    
    Bandit 1.6.0 introduces a regression[0] with the -x option,
    a fix is expected to be included in 1.6.1 soon. (Original commit is
    based on Id944054deedd545c34fc28ccf043dd72e5f31220)
    
    neutron-lbaas is retired [2], so networking-odl dependencies must be
    removed.
    Trunk constants were moved to neutron-lib from neutron with [3] from
    1.25.0, and were removed from neutron with [4], thus lower-constraints
    must point to at least 1.25.0.
    To pass the gate fix sphinx requirements for python>3.4
    (Original Change-Id: I6e709385fefe12123ecab150237956297cc7e09f)
    
    [0] PyCQA/bandit#488
    [1] PyCQA/bandit#489
    [2] http://lists.openstack.org/pipermail/openstack-discuss/2019-May/006158.html
    [3] https://review.opendev.org/636989
    [4] https://review.opendev.org/649672
    
    Change-Id: I698caa93a188f7be206c18d79152dd81eb4029d3
arsulegai pushed a commit to arsulegai/sawtooth-sdk-python that referenced this issue Aug 25, 2019
Workaround for bug reported here: PyCQA/bandit#488

Signed-off-by: Richard Berg <rberg@bitwise.io>
@bittner
Copy link
Contributor

bittner commented Oct 10, 2019

Can anyone confirm this bug has been fixed?

I still run bandit<1.6 as the latest Bandit from PyPI scans the .tox/ folder. Here is an example that uses an absolute directory path to rule out location translation issues.

$ bandit --debug -r . -x $(pwd)/.tox
...
[node_visitor]	DEBUG	{'imports': {'sys', '_locale', 'locale'}, 'import_aliases': {}, 'node': <_ast.Load object at 0x7fc53651c278>, 'linerange': [0, 1], 'filename': './.tox/bandit/lib/python3.6/_bootlocale.py'}

@bittner
Copy link
Contributor

bittner commented Oct 10, 2019

Interestingly, specifying an absolute target path combined with absolute exclusion paths seems to work:

$ bandit -x $(pwd)/.tox/ -r $(pwd)
[main]	INFO	profile include tests: None
[main]	INFO	profile exclude tests: None
[main]	INFO	cli include tests: None
[main]	INFO	cli exclude tests: None
[main]	INFO	running on Python 3.6.8
Run started:2019-10-10 08:51:10.630871

Test results:
...
Code scanned:
	Total lines of code: 70
	Total lines skipped (#nosec): 0

Run metrics:
	Total issues (by severity):
		Undefined: 0.0
		Low: 2.0
		Medium: 0.0
		High: 0.0
	Total issues (by confidence):
		Undefined: 0.0
		Low: 0.0
		Medium: 0.0
		High: 2.0
Files skipped (0):

tox.ini

Also, specifying all absolute paths in a Tox target, a section in tox.ini, "fixes" the issue:

[testenv:bandit]
deps = bandit
commands = bandit -x {toxinidir}/.git,{toxinidir}/.tox,{toxinidir}/tests -r {toxinidir}
$ tox -e bandit
bandit installed: bandit==1.6.2,gitdb2==2.0.6,GitPython==3.0.3,pbr==5.4.3,PyYAML==5.1.2,six==1.12.0,smmap2==2.0.5,stevedore==1.31.0
bandit run-test-pre: PYTHONHASHSEED='2508293914'
bandit runtests: commands[0] | bandit --exclude /home/foo/example-flask/.git,/home/foo/example-flask/.tox,/home/foo/example-flask/tests -r /home/foo/example-flask
[main]	INFO	profile include tests: None
[main]	INFO	profile exclude tests: None
[main]	INFO	cli include tests: None
[main]	INFO	cli exclude tests: None
[main]	INFO	running on Python 3.6.8
Run started:2019-10-10 08:53:59.209246

Test results:
	No issues identified.

Code scanned:
	Total lines of code: 26
	Total lines skipped (#nosec): 0

Run metrics:
	Total issues (by severity):
		Undefined: 0.0
		Low: 0.0
		Medium: 0.0
		High: 0.0
	Total issues (by confidence):
		Undefined: 0.0
		Low: 0.0
		Medium: 0.0
		High: 0.0
Files skipped (0):
__________________________________________________ summary __________________________________________________
  bandit: commands succeeded
  congratulations :)

Looks like there is still some hidden issue that needs fixing.

@LefterisJP
Copy link

I am trying to use bandit 1.6.2 and excluded directories are not respected. This issue does not appear to be fixed.

@Enrico204
Copy link

@LefterisJP I'm using bandit 1.6.2 and it works, however it depends on how you specify all paths. I'm using all full relative paths (eg. bandit -x './tests' -r .). If I omit ./ at the beginning of the exclude directory, that directory is not considered in the exclusion. It seems also that this behavior is the same with the full path, as stated by @bittner . There is definitively an issue on path handling

@bsolomon1124
Copy link

bsolomon1124 commented Apr 1, 2020

This issue still exists nearly one year later on Bandit 1.6.2. Unclear why it is closed.

While it makes very little sense, -x or --exclude seems to require a ./-prefaced relative path.

E.g. if you have a venv in your current directory:

python -m bandit --exclude ./venv --recursive --output bandit.json --format json .

will work as intended whereas

python -m bandit --exclude venv --recursive --output bandit.json --format json .
python -m bandit --exclude "${PWD}/venv" --recursive --output bandit.json --format json .

Will attempt to scan all of venv/.

The current arg is:

    parser.add_argument(
        '-x', '--exclude', dest='excluded_paths', action='store',
        default=','.join(constants.EXCLUDE),
        help='comma-separated list of paths (glob patterns '
             'supported) to exclude from scan '
             '(note that these are in addition to the excluded '
             'paths provided in the config file) (default: ' +
        ','.join(constants.EXCLUDE) + ')'
    )

Which gets passed to discover_files():

b_mgr.discover_files(args.targets, args.recursive, args.excluded_paths)

Independent of this issue, it might be sensible to add venv to constants.EXCLUDE.

wmfgerrit pushed a commit to wikimedia/cumin that referenced this issue May 18, 2020
* Adapt bandit command parameters to make it work with the latest
  version of bandit allowing to relax the version constraint.
* See also: PyCQA/bandit#488

Change-Id: Ide0f7c05de141c6131e9340e61f821189ed503a4
@KayleeTheMech
Copy link

KayleeTheMech commented Feb 8, 2021

Still having this issue in 1.7.0, pinning to 1.5.1 doesn't help. For gitlab reasons I had to include the virtualenv inside the project_root and bandit ignores all attempts to ignore that folder...
Command:
bandit --exclude .pipenv/* -r -ll .

@scott-nordicwi
Copy link

Just confirming that this issue still persists in 1.7

@KayleeTheMech
Copy link

Coming back here include my workaround.

Using absolute paths has worked, but since that wasn't an option for gitlab i found that

bandit --exclude ./.pipenv/* -r -ll .

seems to respect the excluded folder.

@bsolomon1124
Copy link

@ericwb please reopen based on ample evidence above.

@dolfinus
Copy link

dolfinus commented Jul 8, 2021

I've faced just the same issue on 1.7. This command is hanging for several minutes:

bandit -lll -r -x tests .

as well as this one:

bandit -lll -r -x "tests/*" .

but this one works just as expected:

bandit -lll -r -x "./tests/*" .

@dromer
Copy link

dromer commented Aug 16, 2021

The help output says that it excludes .tox by default, but my tox-dir is still scanned which is quite useless.

This is certainly not resolved and needs to be reopened.

felnne added a commit to antarctica/metadata-library that referenced this issue Oct 6, 2021
See PyCQA/bandit#488 as a depressing issue.
openstack-mirroring pushed a commit to openstack/networking-odl that referenced this issue Nov 22, 2021
Bandit 1.6.0 introduces a regression[0] with the -x option,
a fix is expected to be included in 1.6.1 soon. (Original commit is
based on Id944054deedd545c34fc28ccf043dd72e5f31220)

neutron-lbaas is retired [2], so networking-odl dependencies must be
removed.
Trunk constants were moved to neutron-lib from neutron with [3] from
1.25.0, and were removed from neutron with [4], thus lower-constraints
must point to at least 1.25.0.
To pass the gate fix sphinx requirements for python>3.4
(Original Change-Id: I6e709385fefe12123ecab150237956297cc7e09f)

Conflicts:
.zuul.d/project.yaml
doc/requirements.txt
lower-constraints.txt
networking_odl/tests/unit/journal/test_full_sync.py
requirements.txt
test-requirements.txt

[0] PyCQA/bandit#488
[1] PyCQA/bandit#489
[2] http://lists.openstack.org/pipermail/openstack-discuss/2019-May/006158.html
[3] https://review.opendev.org/636989
[4] https://review.opendev.org/649672

Change-Id: I698caa93a188f7be206c18d79152dd81eb4029d3
openstack-mirroring pushed a commit to openstack/networking-odl that referenced this issue Jan 3, 2022
Bandit 1.6.0 introduces a regression[0] with the -x option,
a fix is expected to be included in 1.6.1 soon. (Original commit is
based on Id944054deedd545c34fc28ccf043dd72e5f31220)

neutron-lbaas is retired [2], so networking-odl dependencies must be
removed.
Trunk constants were moved to neutron-lib from neutron with [3] from
1.25.0, and were removed from neutron with [4], thus lower-constraints
must point to at least 1.25.0.
To pass the gate fix sphinx requirements for python>3.4
(Original Change-Id: I6e709385fefe12123ecab150237956297cc7e09f)

Conflicts:
.zuul.d/project.yaml
.zuul.d/jobs.yaml
doc/requirements.txt
requirements.txt

[0] PyCQA/bandit#488
[1] PyCQA/bandit#489
[2] http://lists.openstack.org/pipermail/openstack-discuss/2019-May/006158.html
[3] https://review.opendev.org/636989
[4] https://review.opendev.org/649672

Change-Id: I698caa93a188f7be206c18d79152dd81eb4029d3
jesper-friis pushed a commit to EMMC-ASBL/oteapi-core that referenced this issue Jan 13, 2022
@demberto
Copy link

Can confirm here than it still doesn't work when I add exclude tests to pyproject.toml

[tool.bandit]
exclude = "./tests/*"

@zelenyjan
Copy link

zelenyjan commented Apr 3, 2022

Bandit 1.7.4 (Python 3.10.4)

bandit -c pyproject.toml -r src

with config pyproject.toml

[tool.bandit]
exclude_dirs = ["*/tests/*"]

works as expected.

All exluded dirs are then:

['*/tests/*', '.svn', 'CVS', '.bzr', '.hg', '.git/*', '__pycache__/*', '.tox', '.eggs', '*.egg']

@bittner
Copy link
Contributor

bittner commented Apr 3, 2022

@zelenyjan There are a few interesting facts about the information your share:

  • Where did you get the exclude_dirs option from? It's neither in the documentation (which says, exclude) nor in the CLI help screen, Only from the code this can be seen. Which suggests that we have a bug in the docs.
  • Interestingly enough, the exclude_dirs option covers not only directories: You can specify a path to a single file, which then also gets excluded, specifically. (The naming seems a bit misleading.)
  • The Bandit source code suggests that you have a Bandit INI file present that gets read (because reading the excluded dirs from TOML doesn't seem to be logged, which is what my local tests confirm).
  • The option considered in an INI file is called exclude, which is consistent with the docs but inconsistent with what is considered from a TOML (or YAML) file.

Can you share your complete Bandit (and Tox) configuration, please?

Also, can you try to run against . instead of src (which will likely walk directories like .git, .tox and .venv if they are not ignored effectively)?

bandit -c pyproject.toml -r .

Related: #528

And yes, the implementation of the config parsing is super-complicated and probably warrants a complete rewrite. 😟

@zelenyjan
Copy link

I found exclude_dirs it in the code. -x "*/tests/*" from CLI was working so was trying to find out what's different.

pyproject.toml

[tool.bandit]
exclude_dirs = ["*/tests/*"]
skips = ["B410", "B106", "B308", "B703", "B311", "B324", "B105", "B110"]  # just for testing

tox.ini not really using tox. It's just for flake8 config for now

[flake8]
ignore = E203, E501, W503, B950, FS003, S101
max-line-length = 120
per-file-ignores = __init__.py:F401,tests/*.py: S101
exclude = .tox,.git,*/migrations/*,*/static/CACHE/*,docs,node_modules,venv
pytest-mark-no-parentheses = true
pytest-fixture-no-parentheses = true

for testing just created .venv and .tox folders and they are not excluded by defaul. The list of excluded dirs I posted is print(excluded_path_globs) on this line .

To exlude .tox and .venv dirs I have to set:

pyproject.toml

[tool.bandit]
exclude_dirs = ["*/tests/*", "*/.tox/*", "*/.venv/*"]

sigmavirus24 pushed a commit that referenced this issue Aug 11, 2022
* [docs] Use code-blocks for syntax highighting, un-inline hyperlinks

* [docs] Mention `exclude_dirs` option available in TOML and YAML

Make configuration examples easier to understand and use

Explain how to install TOML support

As discovered in #488 and reported in #528.
saruniitr added a commit to uktrade/lite-hmrc that referenced this issue Sep 6, 2023
Bandit not respecting values provided in .bandit file because of a known issue
PyCQA/bandit#488
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet