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

Cannot pass a baseline file to pre-commit hook (baseline does not apply when filename is given as target) #531

Open
sirosen opened this issue Aug 20, 2019 · 4 comments
Labels
bug Something isn't working

Comments

@sirosen
Copy link

sirosen commented Aug 20, 2019

Describe the bug

After creating bandit config and a baseline such that bandit -c .bandit.yaml -b .bandit_baseline.json -r . passes, I added my desired pre-commit configuration:

- repo: https://github.com/PyCQA/bandit
  rev: 1.6.2
  hooks:
    - id: bandit
      args: ["-c", ".bandit.yaml", "-b", ".bandit_baseline.json"]
      exclude: tests/.*$

and ran pre-commit run --all-files --show-diff-on-failure expecting it to pass.
However, it appears that the baseline does not apply when a file is passed to bandit (pre-commit works by passing staged files as arguments), so this fails.

To Reproduce
In an empty directory, create fail.py:

import subprocess
subprocess.run('echo hi', shell=True)

and run bandit to create a baseline:

bandit -f json -o .baseline.json -r .

Then try running bandit with that input baseline giving fail.py as an argument:

 bandit -b .baseline.json fail.py

this fails (unexpectedly!).

If you then create a baseline from fail.py, you'll get a file that works:

bandit -f json -o .baseline.json fail.py
bandit -b .baseline.json fail.py  # works!

The difference? ./fail.py vs fail.py:

61c61
<       "filename": "./fail.py",
---
>       "filename": "fail.py",

Expected behavior

With a baseline generated recursively over a repo, the pre-commit hook should pass.

In order for this to happen, running bandit to lint an explicit file with a baseline generated by a recursive walk needs to work.

@sirosen
Copy link
Author

sirosen commented Sep 9, 2019

In case anyone else is trying to figure this out, I've found that this can make bandit work with pre-commit:

bandit -c .bandit.yaml -f json -o .bandit_baseline.json \
        --exclude "tests/*" $(git ls-files) 

(where your excludes may not match mine)

This ensures that the paths are passed to bandit in a similar manner to what pre-commit will do, and easily excludes untracked virtualenvs and the like.

You could also use find . -type f | sed 's:\./::' or the like and expand the exclude listing, but that seems messy.

I stand by the opinion that bandit should normalize for ./ as a prefix internally, so that you can just call bandit -r . without explicitly passing a file list and you don't get these spurious failures.

@ericwb ericwb added the bug Something isn't working label Dec 14, 2020
@m-aciek
Copy link

m-aciek commented Mar 31, 2021

@sirosen So you've created a custom pre-commit script and use it instead of

  - repo: https://github.com/PyCQA/bandit
    rev: 1.7.0
    hooks:
      - id: bandit

?

@sirosen
Copy link
Author

sirosen commented Mar 31, 2021

No, I've changed how the baseline is generated. The sample I provided passes filenames for the baseline exactly the way that pre-commit will pass them to the hook.

@diegovalenzuelaiturra
Copy link

Hi, the following may be helpful to configure bandit, for example, to avoid raising B101 assert_used warnings on python tests

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
Development

No branches or pull requests

4 participants