From 6a6c17e3ac8ebab0be1cacb8f3facbd0a53588ae Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Wed, 12 Jan 2022 12:42:05 -0800 Subject: [PATCH 1/6] CI: Add bandit to pre-commit --- .github/workflows/pythonapp.yml | 2 +- .pre-commit-config.yaml | 8 +++++++- bandit.conf | 5 ++++- cve_bin_tool/available_fix/redhat_cve_tracker.py | 4 ++-- dev-requirements.txt | 1 + 5 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.github/workflows/pythonapp.yml b/.github/workflows/pythonapp.yml index d8d4443543..1b90df0781 100644 --- a/.github/workflows/pythonapp.yml +++ b/.github/workflows/pythonapp.yml @@ -17,7 +17,7 @@ jobs: strategy: fail-fast: false matrix: - tool: ['isort', 'black', 'pyupgrade', 'flake8', 'format_checkers'] + tool: ['isort', 'black', 'pyupgrade', 'flake8', 'format_checkers', 'bandit'] steps: - uses: actions/checkout@v2 - uses: actions/setup-python@v2 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 20aee44eeb..7378953fd8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,12 +14,18 @@ repos: hooks: - id: pyupgrade args: ["--py37-plus"] - + - repo: https://github.com/pycqa/flake8 rev: 4.0.1 hooks: - id: flake8 +- repo: https://github.com/PyCQA/bandit + rev: 1.7.1 + hooks: + - id: bandit + args: ["-c", "bandit.conf"] + - repo: local hooks: - id: format_checkers diff --git a/bandit.conf b/bandit.conf index 74b09e52ee..ee7e67e116 100644 --- a/bandit.conf +++ b/bandit.conf @@ -85,7 +85,10 @@ tests: # (optional) list skipped test IDs here, eg '[B101, B406]': -skips: ['B603', 'B607', 'B404'] +skips: ['B603', 'B607', 'B404', "B608"] +# B603, B607 and B404 are all subprocess-related. +# B608 should be re-enabled when multi-line issues can be marked with nosec + # Explantion: cve-bin-tool is at heart a shell script that calls other processes. # Switching to pure python has significant performance impacts. diff --git a/cve_bin_tool/available_fix/redhat_cve_tracker.py b/cve_bin_tool/available_fix/redhat_cve_tracker.py index 28c31ccc53..2fc1479cd6 100644 --- a/cve_bin_tool/available_fix/redhat_cve_tracker.py +++ b/cve_bin_tool/available_fix/redhat_cve_tracker.py @@ -74,8 +74,8 @@ def cve_info( def get_data(self, cve_number: str, product: str): try: - full_query = f"{RH_CVE_API}/{cve_number}.json" - response = request.urlopen(full_query).read().decode("utf-8") + full_query = f"{RH_CVE_API}/{cve_number}.json" # static https url above + response = request.urlopen(full_query).read().decode("utf-8") # nosec return loads(response) except error.HTTPError as e: LOGGER.debug(e) diff --git a/dev-requirements.txt b/dev-requirements.txt index 26b0a5edd8..0ed29c8424 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -2,3 +2,4 @@ black==21.12b0 isort==5.10.1 pre-commit==2.16.0 flake8==4.0.1 +bandit=1.7.1 From 758d60e5cc660697ad16c7fbe656cd20b16000ca Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Wed, 12 Jan 2022 13:04:42 -0800 Subject: [PATCH 2/6] ci: exclude test dirs from bandit scan --- bandit.conf | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bandit.conf b/bandit.conf index ee7e67e116..493ba613f9 100644 --- a/bandit.conf +++ b/bandit.conf @@ -92,6 +92,10 @@ skips: ['B603', 'B607', 'B404', "B608"] # Explantion: cve-bin-tool is at heart a shell script that calls other processes. # Switching to pure python has significant performance impacts. +exclude_dirs: + - "./test/" + - "./build/lib/test/" + ### (optional) plugin settings - some test plugins require configuration data ### that may be given here, per-plugin. All bandit test plugins have a built in ### set of sensible defaults and these will be used if no configuration is From ad50d08f30f579f54933049062cb55551280b787 Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Wed, 12 Jan 2022 13:13:43 -0800 Subject: [PATCH 3/6] ci: exclude test dirs from bandit scan --- bandit.conf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bandit.conf b/bandit.conf index 493ba613f9..959c40db4c 100644 --- a/bandit.conf +++ b/bandit.conf @@ -93,6 +93,8 @@ skips: ['B603', 'B607', 'B404', "B608"] # Switching to pure python has significant performance impacts. exclude_dirs: + - "test/" + - "/test/" - "./test/" - "./build/lib/test/" From a5f80e86a0b76b580b40b135fd1a8a6d931cf857 Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Thu, 13 Jan 2022 12:35:56 -0800 Subject: [PATCH 4/6] doc: add bandit to list of tools used --- CONTRIBUTING.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5df1066902..788c44d0ba 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -17,6 +17,7 @@ If you've already contributed to other open source projects, contributing to the - [Using pre-commit to run linters automatically](#using-pre-commit-to-run-linters-automatically) - [Running isort by itself](#running-isort-by-itself) - [Running black by itself](#running-black-by-itself) + - [Running bandit by itself](#running-bandit-by-itself) - [Other linting tools](#other-linting-tools) - [Making a new branch & pull request](#making-a-new-branch--pull-request) - [Commit message tips](#commit-message-tips) @@ -160,6 +161,7 @@ CVE Binary Tool uses a few tools to improve code quality and readability: - `black` provides automatic style formatting. This will give you basic [PEP8](https://www.python.org/dev/peps/pep-0008/) compliance. (PEP8 is where the default python style guide is defined.) - `flake8` provides additional code "linting" for more complex errors like unused imports. - `pyupgrade` helps us be forward compatible with new versions of python. +- `bandit` is more of a static analysis tool than a linter and helps us find potential security flaws in the code. We provide a `dev-requirements.txt` file which includes all the precise versions of tools as they'll be used in GitHub Actions. You an install them all using pip: @@ -206,6 +208,26 @@ files you've changed because you won't have to scroll through a pile of auto-formatting changes to find your own modifications. However, you can also specify a whole folder using ```./``` +### Running bandit by itself + +We have a configuration file for bandit called `bandit.conf` that you should use. This disables a few of the checkers and disables scanning of the test directory. + +To run it on all the code we scan, use the following: + +```bash +bandit -c bandit.conf -r cve_bin_tool/ +``` + +You can also run it on individual files: + +```bash +bandit -c bandit.conf filename.py +``` + +If you run it without the config file, it will run a few extra checkers and will run on test code, so you'll get additional warnings. + +Bandit helps you target manual code review, but bandit issues aren't always things that need to be fixed, just reviewed. If you have a bandit finding that doesn't actually need a fix, you can mark it as reviewed using a `# nosec` comment. If possible, include details as to why the bandit results are ok for future reviewers. For example, we have comments like `#nosec uses static https url above` in cases where bandit prompted us to review the variable being passed to urlopen(). + ### Other linting tools As well as `black` for automatically making sure code adheres to the style guide, we use `flake8` to help us find things like unused imports. The [flake8 documentation](https://flake8.pycqa.org/en/latest/user/index.html) covers what you need to know about running it. From 2bb8ff5096ef380e48f88d4d49cde90ce647396f Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Thu, 13 Jan 2022 13:29:29 -0800 Subject: [PATCH 5/6] ci: add urlopen & nosec to spelling list --- .github/actions/spelling/allow.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt index 181ef47e04..897fd4735f 100644 --- a/.github/actions/spelling/allow.txt +++ b/.github/actions/spelling/allow.txt @@ -233,6 +233,7 @@ nitishsaini nlk noopener noreferrer +nosec nowdailynever nplurals ntp @@ -339,6 +340,7 @@ unicode uniq unittest url +urlopen usecase username usr From 01602af2ea5c25edc3b44feaec36948d4364c395 Mon Sep 17 00:00:00 2001 From: Terri Oda Date: Wed, 26 Jan 2022 10:03:51 -0800 Subject: [PATCH 6/6] Update dev-requirements.txt Co-authored-by: Bread Genie <63963181+BreadGenie@users.noreply.github.com> --- dev-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 0ed29c8424..835e7b40f4 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -2,4 +2,4 @@ black==21.12b0 isort==5.10.1 pre-commit==2.16.0 flake8==4.0.1 -bandit=1.7.1 +bandit==1.7.1