bandit icon indicating copy to clipboard operation
bandit copied to clipboard

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

Open sirosen opened this issue 4 years ago • 4 comments

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 avatar Aug 20 '19 17:08 sirosen

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.

sirosen avatar Sep 09 '19 18:09 sirosen

@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

?

m-aciek avatar Mar 31 '21 12:03 m-aciek

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.

sirosen avatar Mar 31 '21 13:03 sirosen

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

  • https://github.com/PyCQA/bandit/issues/603#issuecomment-971057519

diegovalenzuelaiturra avatar Nov 17 '21 01:11 diegovalenzuelaiturra