Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

pipenv check looks at installed packages, not Pipfile.lock #5600

Closed
jacobfelknor opened this issue Feb 7, 2023 · 4 comments · Fixed by #5628
Closed

pipenv check looks at installed packages, not Pipfile.lock #5600

jacobfelknor opened this issue Feb 7, 2023 · 4 comments · Fixed by #5628
Labels
Contributor Candidate The issue has been identified/triaged and contributions are welcomed/encouraged. Type: Enhancement 💡 This is a feature or enhancement request.

Comments

@jacobfelknor
Copy link

Use Case

I would like to run pipenv check as a separate job from the build/test job inside a CI pipeline without rebuilding environment. I discovered that I must actually install all packages to a pipenv environment before using pipenv check. Ideally, I should be able to scan the dependencies inside Pipfile.lock without actually installing the whole environment.

I believe its misleading that right now pipenv is just acting as a "proxy" to safety, and by default checks an environment that may not match Pipfile.lock. By using pipenv check the assumption should be that it is checking the environment specified in Pipfile.lock and if you need to check an environment that deviates, you use safety directly.

I've traced the behavior down to these lines:

pipenv/pipenv/core.py

Lines 2900 to 2902 in 8939c86

target_venv_packages = run_command(
_cmd + ["-m", "pip", "list", "--format=freeze"], is_verbose=project.s.is_verbose()
)

Instead of generating the temp requirements.txt file from the current environment using pip list, can we instead generate the temp requirements.txt from Pipfile.lock? Something like

# this command should also respect the wishes of the --dev argument, if provided. Unsure on specifics of implementation
target_venv_packages = run_command(
        _cmd + ["-m", "pipenv", "requirements"], is_verbose=project.s.is_verbose()
    )

Workaround

I'm currently using the following workaround in my CI job, but would like to go through pipenv directly.

pipenv requirements --dev | safety check --stdin
@matteius matteius added Type: Enhancement 💡 This is a feature or enhancement request. Contributor Candidate The issue has been identified/triaged and contributions are welcomed/encouraged. labels Feb 11, 2023
@matteius
Copy link
Member

pipenv=2022.2.18 has made this the default behavior for pipenv check

@jacobfelknor
Copy link
Author

jacobfelknor commented Feb 22, 2023

@matteius

Thanks for taking this suggestion! I appreciate it. I have two quick pieces of feedback for even further improvement

  1. The --dev argument has no effect on the pipenv check command. If user specifies the --dev flag, we should generate the temp requirements file from pipenv requirements --dev.

    • EDIT: apologies, I now realize this is possible with the pipenv check --categories packages,dev-packages. Is this equivalent to what I was asking for?
  2. pipenv check still prints "Checking installed packages for vulnerabilities...", regardless of it's using the installed environment with --use-installed or if it's using the dependencies from Pipfile.lock. If using default behavior, this should read something like "Checking Pipfile.lock packages for vulnerabilities..."

@matteius matteius reopened this Feb 23, 2023
@ryan-rozario
Copy link
Contributor

@matteius Could I create a PR for the second suggestion since it is a simple condition that needs to be added.

@matteius
Copy link
Member

matteius commented Mar 9, 2023

@ryan-rozario Yes please, that would be a big help.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Contributor Candidate The issue has been identified/triaged and contributions are welcomed/encouraged. Type: Enhancement 💡 This is a feature or enhancement request.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants