Skip to content

Commit

Permalink
Issue 5600 check command should be able to pull requirements from loc…
Browse files Browse the repository at this point in the history
…k file (#5607)

* Allow pipenv check inputs to be built from the lockfile categories instead of whats installed.

* fix lint

* add news fragment.

* update pipenv check documentation.

* Revise logic for pipenv check and change default behavior to check lockfile.

* fix issue revealed by tests

* fix docs for pipenv check changes.

* change conditional ordering in prep for supporting for future default categories env var.
  • Loading branch information
matteius committed Feb 18, 2023
1 parent 8877d79 commit ca436af
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 50 deletions.
77 changes: 35 additions & 42 deletions docs/advanced.rst
Original file line number Diff line number Diff line change
Expand Up @@ -390,57 +390,50 @@ It can be used to specify multiple categories also.
Pipenv includes the `safety <https://github.com/pyupio/safety>`_ package, and will use it to scan your dependency graph
for known security vulnerabilities!

Example::

$ cat Pipfile
[packages]
django = "==1.10.1"

$ pipenv check
Checking PEP 508 requirements...
Passed!
Checking installed package safety...

33075: django >=1.10,<1.10.3 resolved (1.10.1 installed)!
Django before 1.8.x before 1.8.16, 1.9.x before 1.9.11, and 1.10.x before 1.10.3, when settings.DEBUG is True, allow remote attackers to conduct DNS rebinding attacks by leveraging failure to validate the HTTP Host header against settings.ALLOWED_HOSTS.

33076: django >=1.10,<1.10.3 resolved (1.10.1 installed)!
Django 1.8.x before 1.8.16, 1.9.x before 1.9.11, and 1.10.x before 1.10.3 use a hardcoded password for a temporary database user created when running tests with an Oracle database, which makes it easier for remote attackers to obtain access to the database server by leveraging failure to manually specify a password in the database settings TEST dictionary.
By default ``pipenv check`` will scan the Pipfile.lock default packages group and use this as the input to the safety command.
To scan other package categories pass the specific ``--categories`` you want to check against.
To have ``pipenv check`` scan the virtualenv packages for what is installed and use this as the input to the safety command,
run``pipenv check --use-installed``.
Note: ``--use-installed`` was the default behavior in ``pipenv<=2023.2.4``

33300: django >=1.10,<1.10.7 resolved (1.10.1 installed)!
CVE-2017-7233: Open redirect and possible XSS attack via user-supplied numeric redirect URLs
============================================================================================

Django relies on user input in some cases (e.g.
:func:`django.contrib.auth.views.login` and :doc:`i18n </topics/i18n/index>`)
to redirect the user to an "on success" URL. The security check for these
redirects (namely ``django.utils.http.is_safe_url()``) considered some numeric
URLs (e.g. ``http:999999999``) "safe" when they shouldn't be.

Also, if a developer relies on ``is_safe_url()`` to provide safe redirect
targets and puts such a URL into a link, they could suffer from an XSS attack.
Example::

CVE-2017-7234: Open redirect vulnerability in ``django.views.static.serve()``
=============================================================================
$ pipenv install wheel==0.37.1
$ cat Pipfile.lock
...
"default": {
"wheel": {
"hashes": [
"sha256:4bdcd7d840138086126cd09254dc6195fb4fc6f01c050a1d7236f2630db1d22a",
"sha256:e9a504e793efbca1b8e0e9cb979a249cf4a0a7b5b8c9e8b65a5e39d49529c1c4"
],
"index": "pypi",
"version": "==0.37.1"
}
},
...

A maliciously crafted URL to a Django site using the
:func:`~django.views.static.serve` view could redirect to any other domain. The
view no longer does any redirects as they don't provide any known, useful
functionality.
$ pipenv check --use-lock
...
-> Vulnerability found in wheel version 0.37.1
Vulnerability ID: 51499
Affected spec: <0.38.1
ADVISORY: Wheel 0.38.1 includes a fix for CVE-2022-40898: An issue discovered in Python Packaging Authority (PyPA) Wheel 0.37.1 and earlier allows remote attackers to cause a denial of service
via attacker controlled input to wheel cli.https://pyup.io/posts/pyup-discovers-redos-vulnerabilities-in-top-python-packages
CVE-2022-40898
For more information, please visit https://pyup.io/v/51499/742

Note, however, that this view has always carried a warning that it is not
hardened for production use and should be used only as a development aid.
Scan was completed. 1 vulnerability was found.
...

✨🍰✨

.. note::

Each month, `PyUp.io <https://pyup.io>`_ updates the ``safety`` database of
insecure Python packages and `makes it available to the
community for free <https://pyup.io/safety/>`__. Pipenv
makes an API call to retrieve those results and use them
each time you run ``pipenv check`` to show you vulnerable
dependencies.
insecure Python packages and `makes it available to the open source
community for free <https://pyup.io/safety/>`__. Each time
you run ``pipenv check`` to show you vulnerable dependencies,
Pipenv makes an API call to retrieve and use those results.

For more up-to-date vulnerability data, you may also use your own safety
API key by setting the environment variable ``PIPENV_PYUP_API_KEY``.
Expand Down
3 changes: 3 additions & 0 deletions news/5600.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Behavior change for ``pipenv check`` now checks the default packages group of the lockfile.
Specifying ``--categories`` to override which categories to check against.
Pass ``--use-installed`` to get the prior behavior of checking the packages actually installed into the environment.
17 changes: 15 additions & 2 deletions pipenv/cli/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -479,13 +479,23 @@ def run(state, command, args):
help="Path to where output file will be placed, if the path is a directory, "
"Safety will use safety-report.json as filename. Default: empty",
)
@option(
"--use-installed",
is_flag=True,
help="Whether to use the lockfile as input to check (instead of result from pip list).",
)
@option(
"--categories",
is_flag=False,
default="",
help="Use the specified categories from the lockfile as input to check.",
)
@common_options
@system_option
@pass_state
def check(
state,
db=None,
style=False,
ignore=None,
output="screen",
key=None,
Expand All @@ -495,6 +505,8 @@ def check(
save_json="",
audit_and_monitor=True,
project=None,
use_installed=False,
categories="",
**kwargs,
):
"""Checks for PyUp Safety security vulnerabilities and against PEP 508 markers provided in Pipfile."""
Expand All @@ -515,6 +527,8 @@ def check(
audit_and_monitor=audit_and_monitor,
safety_project=project,
pypi_mirror=state.pypi_mirror,
use_installed=use_installed,
categories=categories,
)


Expand Down Expand Up @@ -771,7 +785,6 @@ def verify(state):
def requirements(
state, dev=False, dev_only=False, hash=False, exclude_markers=False, categories=""
):

from pipenv.utils.dependencies import convert_deps_to_pip

lockfile = state.project.load_lockfile(expand_env_vars=False)
Expand Down
20 changes: 16 additions & 4 deletions pipenv/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
from pipenv.vendor.requirementslib.models.requirements import Requirement

if MYPY_RUNNING:

TSourceDict = Dict[str, Union[str, bool]]


Expand Down Expand Up @@ -2791,6 +2790,8 @@ def do_check(
audit_and_monitor=True,
safety_project=None,
pypi_mirror=None,
use_installed=False,
categories="",
):
import json

Expand Down Expand Up @@ -2897,9 +2898,20 @@ def do_check(
if safety_project:
options.append(f"--project={safety_project}")

target_venv_packages = run_command(
_cmd + ["-m", "pip", "list", "--format=freeze"], is_verbose=project.s.is_verbose()
)
if use_installed:
target_venv_packages = run_command(
_cmd + ["-m", "pip", "list", "--format=freeze"],
is_verbose=project.s.is_verbose(),
)
elif categories:
target_venv_packages = run_command(
["pipenv", "requirements", "--categories", categories],
is_verbose=project.s.is_verbose(),
)
else:
target_venv_packages = run_command(
["pipenv", "requirements"], is_verbose=project.s.is_verbose()
)

temp_requirements = tempfile.NamedTemporaryFile(
mode="w+",
Expand Down
16 changes: 14 additions & 2 deletions tests/integration/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def test_pipenv_check(pipenv_instance_private_pypi):
with pipenv_instance_private_pypi() as p:
c = p.pipenv('install pyyaml')
assert c.returncode == 0
c = p.pipenv('check')
c = p.pipenv('check --use-installed')
assert c.returncode != 0
assert 'pyyaml' in c.stdout
c = p.pipenv('uninstall pyyaml')
Expand All @@ -158,11 +158,23 @@ def test_pipenv_check(pipenv_instance_private_pypi):
# the issue above is still not resolved.
# added also 51499
# https://github.com/pypa/wheel/issues/481
c = p.pipenv('check --ignore 35015 -i 51457 -i 51499')
c = p.pipenv('check --use-installed --ignore 35015 -i 51457 -i 51499')
assert c.returncode == 0
assert 'Ignoring' in c.stderr


@pytest.mark.cli
@pytest.mark.needs_internet(reason='required by check')
@pytest.mark.parametrize("category", ["CVE", "packages"])
def test_pipenv_check_check_lockfile_categories(pipenv_instance_pypi, category):
with pipenv_instance_pypi() as p:
c = p.pipenv(f'install wheel==0.37.1 --categories={category}')
assert c.returncode == 0
c = p.pipenv(f'check --categories={category}')
assert c.returncode != 0
assert 'wheel' in c.stdout


@pytest.mark.cli
def test_pipenv_clean(pipenv_instance_pypi):
with pipenv_instance_pypi(chdir=True) as p:
Expand Down

0 comments on commit ca436af

Please sign in to comment.