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

Bug: pre-commit hook failing when local pip-audit command works #770

Open
3 tasks done
r-findley opened this issue Apr 29, 2024 · 8 comments
Open
3 tasks done

Bug: pre-commit hook failing when local pip-audit command works #770

r-findley opened this issue Apr 29, 2024 · 8 comments
Labels
bug-candidate Might be a bug. needs-response Needs response from the reporter.

Comments

@r-findley
Copy link

Pre-submission checks

  • I am not filing an auditing error (false positive or negative). These must be reported to pypa/advisory-database instead.
  • I agree to follow the PSF Code of Conduct.
  • I have looked through the open issues for a duplicate report.

Expected behavior

I expected pip-audit to pass the checks when using the pre-commit hook in my repository.

Actual behavior

After getting the following failure:

pip-audit................................................................Failed
- hook id: pip-audit
- exit code: 1

Found 1 known vulnerability in 1 package
Name Version ID                  Fix Versions
---- ------- ------------------- ------------
idna 3.6     GHSA-jjg7-2v4v-x38h 3.7

I performed an upgrade to the idna library to version 3.7 and ran the commit again. This still failed with the above error. I therefore ran the command

pip-audit

directly in the virtual environment and the pip-audit passed successfully.

Reproduction steps

  1. Make commits to repository for an Azure Function that is using an idna version below 3.7 while using the pre-commit hook.
  2. Upgrade idna to the version suggested by the pip-audit failure
  3. Make the commit again

Logs

No response

Additional context

I am running this on a MacBook Pro using Sonoma 14.4.1 while a co-worker is using Windows 11 OS Build 22631.3447

OS name, version, and architecture

No response

pip-audit version

2.7.2

pip version

23.3.1

Python version

3.11.7

@r-findley r-findley added the bug-candidate Might be a bug. label Apr 29, 2024
@woodruffw
Copy link
Member

Thanks for the report @r-findley!

Just to make sure I understand: what you're seeing here is idna==3.6 when run via the pre-commit hook, but not when auditing the environment directly, right?

Are you able to share more about your dependency/requirements set? I'm curious if idna==3.6 is being selected during initial resolution due to an upper bound (or aggressive caching) somewhere.

@woodruffw woodruffw added the needs-response Needs response from the reporter. label Apr 29, 2024
@r-findley
Copy link
Author

Hello @woodruffw! I appreciate the quick reply. I'll paste my requirements.txt info below. You are correct though. When I ran the pre-commit hooks during the commit, I got the warning about idna being vulnerable at 3.6. I did an update to idna to the 3.7 but never actually updated my requirements.txt file. You'll see below this shows idna at 3.7.

azure-functions         1.18.0
bandit                  1.7.8
black                   24.3.0
boolean.py              4.0
CacheControl            0.14.0
certifi                 2024.2.2
cfgv                    3.4.0
charset-normalizer      3.3.2
click                   8.1.7
cyclonedx-python-lib    6.4.4
defusedxml              0.7.1
distlib                 0.3.8
et-xmlfile              1.1.0
filelock                3.13.1
html5lib                1.1
identify                2.5.35
idna                    3.7
iniconfig               2.0.0
isort                   5.13.2
license-expression      30.3.0
markdown-it-py          3.0.0
mdurl                   0.1.2
msgpack                 1.0.8
mypy-extensions         1.0.0
nodeenv                 1.8.0
numpy                   1.26.4
openpyxl                3.1.2
packageurl-python       0.15.0
packaging               24.0
pandas                  2.2.1
pathspec                0.12.1
pbr                     6.0.0
pip                     23.3.1
pip-api                 0.0.33
pip_audit               2.7.2
pip-requirements-parser 32.0.1
platformdirs            4.2.0
pluggy                  1.4.0
pre-commit              3.6.2
py-serializable         1.0.2
Pygments                2.17.2
pyparsing               3.1.2
pytest                  8.1.1
python-dateutil         2.9.0.post0
python-dotenv           1.0.1
pytz                    2024.1
PyYAML                  6.0.1
requests                2.31.0
rich                    13.7.1
setuptools              69.0.2
six                     1.16.0
sortedcontainers        2.4.0
stevedore               5.2.0
toml                    0.10.2
tzdata                  2024.1
urllib3                 2.2.1
virtualenv              20.25.1
webencodings            0.5.1

Even after performing the install of 3.7, the pre-commit is failing. But a run of pip-audit in the terminal is working fine and not flagging any vulnerabilities in idna 3.7.

@woodruffw woodruffw removed the needs-response Needs response from the reporter. label Apr 29, 2024
@woodruffw
Copy link
Member

Got it, thank you!

Hmm, that's odd. Could you share your pre-commit config for pip-audit? I'm curious if you're currently configuring it to run on the requirements.txt (or similar) rather than on your environment.

(FWIW, I know relatively little about pre-commit, but I've seen similar things before where people run pip-audit and get different results because some invocations run it against their requirements input versus their current environment 😅)

@r-findley
Copy link
Author

Here's my .pre-commit-config.yaml. I commented out the pip-audit one since it was not working correctly. One note - I just removed the flag for the requirements.txt because my coworker on a Windows machine was running into an issue where the pre-commit would hang indefinitely if he used the actual requirements file. When I was running it, it failed either way.

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
  rev: v2.3.0
  hooks:
    - id: check-yaml
    - id: check-json
    - id: detect-private-key
    - id: flake8
      args: ["--max-line-length=88", "--ignore=E203"]
    - id: no-commit-to-branch
      args: ['--branch', 'master', '--branch', 'development']
    - id: requirements-txt-fixer
    - id: trailing-whitespace
- repo: https://github.com/psf/black
  rev: 22.10.0
  hooks:
    - id: black
- repo: https://github.com/PyCQA/bandit
  rev: 1.7.5
  hooks:
  - id: bandit
    args: ["-c", "bandit.yaml", "-r", "."]
# - repo: https://github.com/pypa/pip-audit
#   rev: v2.7.2
  # hooks:
  #   -   id: pip-audit
# - repo: local
#   hooks:
#   - id: pytest
#     name: pytest
#     entry: pytest
#     language: system
#     pass_filenames: false
#     always_run: true

@woodruffw
Copy link
Member

This is where my knowledge of pre-commit is lacking, but I suspect that what's happening here is that pre-commit is actually running within its own environment, and so running pip-audit without any arguments is really just auditing the pre-commit environment rather than your intended development/deployment environment.

I'm not sure how to easily verify this, though 😅

TL;DR: I think that, unfortunately, you need to use -r requirements.txt (or . for pyproject.toml mode) in pre-commit. With that, it's insufficient to just update your dev environment with pip update or similar -- the requirements.txt always needs to be kept up-to-date in order for pre-commit to not flag any findings within it.

(I'm curious to hear more about the hang with Windows as well -- that sounds a lot like #756, which hasn't made it into a release yet.)

@r-findley
Copy link
Author

Ah you may be right. We had removed the -r requirements.txt call due to the Windows issue (It may be #756 though I can't recall exactly if that's what my co-worker mentioned). I added the flag back to my recent commit and it worked properly. I believe that means it was an issue on our end though I'll keep an eye on my work and the team's to see if we run into the issue after #756 is addressed.

@woodruffw
Copy link
Member

Sounds good! I'll try and make a release containing #756 today.

@woodruffw
Copy link
Member

The fix in #756 has been released! Can you give the latest version of pip-audit a try and see if it resolves the hang you've seen?

@woodruffw woodruffw added the needs-response Needs response from the reporter. label May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-candidate Might be a bug. needs-response Needs response from the reporter.
Projects
None yet
Development

No branches or pull requests

2 participants