From f409fc87b13e3928774a1011f091ed640b40104d Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Sun, 12 Feb 2023 00:38:45 -0500 Subject: [PATCH 1/8] Allow pipenv check inputs to be built from the lockfile categories instead of whats installed. --- pipenv/cli/command.py | 18 +++++++++++++++++- pipenv/core.py | 17 ++++++++++++++--- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/pipenv/cli/command.py b/pipenv/cli/command.py index 5eef271c7b..ca0fadfccc 100644 --- a/pipenv/cli/command.py +++ b/pipenv/cli/command.py @@ -1,6 +1,8 @@ import os import sys +import click + from pipenv import environments from pipenv.__version__ import __version__ from pipenv.cli.options import ( @@ -477,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-lock", + 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, @@ -493,6 +505,8 @@ def check( save_json="", audit_and_monitor=True, project=None, + use_lock=False, + categories="", **kwargs, ): """Checks for PyUp Safety security vulnerabilities and against PEP 508 markers provided in Pipfile.""" @@ -513,6 +527,8 @@ def check( audit_and_monitor=audit_and_monitor, safety_project=project, pypi_mirror=state.pypi_mirror, + use_lock=use_lock, + categories=categories, ) diff --git a/pipenv/core.py b/pipenv/core.py index d3b2882b6d..d4629e2418 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -2791,6 +2791,8 @@ def do_check( audit_and_monitor=True, safety_project=None, pypi_mirror=None, + use_lock=False, + categories="", ): import json @@ -2897,9 +2899,18 @@ 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 categories: + target_venv_packages = run_command( + _cmd + ["-m", "pipenv", "requirements", "--categories", f'"{categories}"'], is_verbose=project.s.is_verbose() + ) + elif use_lock: + target_venv_packages = run_command( + _cmd + ["-m", "pipenv", "requirements"], is_verbose=project.s.is_verbose() + ) + else: + target_venv_packages = run_command( + _cmd + ["-m", "pip", "list", "--format=freeze"], is_verbose=project.s.is_verbose() + ) temp_requirements = tempfile.NamedTemporaryFile( mode="w+", From 6895116960eb75eea6aefa164d24eec8d13be0dc Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Sun, 12 Feb 2023 00:44:12 -0500 Subject: [PATCH 2/8] fix lint --- pipenv/cli/command.py | 5 +---- pipenv/core.py | 7 ++++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/pipenv/cli/command.py b/pipenv/cli/command.py index ca0fadfccc..3d98ed6ea9 100644 --- a/pipenv/cli/command.py +++ b/pipenv/cli/command.py @@ -1,8 +1,6 @@ import os import sys -import click - from pipenv import environments from pipenv.__version__ import __version__ from pipenv.cli.options import ( @@ -482,7 +480,7 @@ def run(state, command, args): @option( "--use-lock", is_flag=True, - help="Whether to use the lockfile as input to check (instead of result from pip list)." + help="Whether to use the lockfile as input to check (instead of result from pip list).", ) @option( "--categories", @@ -785,7 +783,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) diff --git a/pipenv/core.py b/pipenv/core.py index d4629e2418..22872cb1b8 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -57,7 +57,6 @@ from pipenv.vendor.requirementslib.models.requirements import Requirement if MYPY_RUNNING: - TSourceDict = Dict[str, Union[str, bool]] @@ -2901,7 +2900,8 @@ def do_check( if categories: target_venv_packages = run_command( - _cmd + ["-m", "pipenv", "requirements", "--categories", f'"{categories}"'], is_verbose=project.s.is_verbose() + _cmd + ["-m", "pipenv", "requirements", "--categories", f'"{categories}"'], + is_verbose=project.s.is_verbose(), ) elif use_lock: target_venv_packages = run_command( @@ -2909,7 +2909,8 @@ def do_check( ) else: target_venv_packages = run_command( - _cmd + ["-m", "pip", "list", "--format=freeze"], is_verbose=project.s.is_verbose() + _cmd + ["-m", "pip", "list", "--format=freeze"], + is_verbose=project.s.is_verbose(), ) temp_requirements = tempfile.NamedTemporaryFile( From c880daac380b61cdb331f7b04232409122452bbd Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Sun, 12 Feb 2023 00:45:49 -0500 Subject: [PATCH 3/8] add news fragment. --- news/5600.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/5600.feature.rst diff --git a/news/5600.feature.rst b/news/5600.feature.rst new file mode 100644 index 0000000000..738d2574d6 --- /dev/null +++ b/news/5600.feature.rst @@ -0,0 +1 @@ +Passing ``--use-lock`` to ``pipenv check`` will check again lock file default packages, and specifies ``--categories`` allows specifying which lockfile categories to use. From 268ca642d8c1b407b9c81ff8bcb57820f4eb0390 Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Sun, 12 Feb 2023 00:57:00 -0500 Subject: [PATCH 4/8] update pipenv check documentation. --- docs/advanced.rst | 75 +++++++++++++++++++++-------------------------- 1 file changed, 33 insertions(+), 42 deletions(-) diff --git a/docs/advanced.rst b/docs/advanced.rst index d70c5caef1..951528a2d0 100644 --- a/docs/advanced.rst +++ b/docs/advanced.rst @@ -390,57 +390,48 @@ It can be used to specify multiple categories also. Pipenv includes the `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 virtualenv packages for what is installed and use this as the input to the safety command. +To have ``pipenv check`` use the ``Pipfile.lock`` to determine package inputs simply pass ``--use-lock`` to use the default packages +category, or pass the specific ``--categories`` you want to check against. - 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 `) - 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 `_ updates the ``safety`` database of - insecure Python packages and `makes it available to the - community for free `__. 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 `__. 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``. From 9401e0af1f4548167a20111fca4a352de47f41b5 Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Thu, 16 Feb 2023 21:22:47 -0500 Subject: [PATCH 5/8] Revise logic for pipenv check and change default behavior to check lockfile. --- news/5600.feature.rst | 4 +++- pipenv/cli/command.py | 6 +++--- pipenv/core.py | 10 +++++----- tests/integration/test_cli.py | 21 +++++++++++++++++++-- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/news/5600.feature.rst b/news/5600.feature.rst index 738d2574d6..4dfd108518 100644 --- a/news/5600.feature.rst +++ b/news/5600.feature.rst @@ -1 +1,3 @@ -Passing ``--use-lock`` to ``pipenv check`` will check again lock file default packages, and specifies ``--categories`` allows specifying which lockfile categories to use. +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. diff --git a/pipenv/cli/command.py b/pipenv/cli/command.py index 3d98ed6ea9..c8cefdb8dd 100644 --- a/pipenv/cli/command.py +++ b/pipenv/cli/command.py @@ -478,7 +478,7 @@ def run(state, command, args): "Safety will use safety-report.json as filename. Default: empty", ) @option( - "--use-lock", + "--use-installed", is_flag=True, help="Whether to use the lockfile as input to check (instead of result from pip list).", ) @@ -503,7 +503,7 @@ def check( save_json="", audit_and_monitor=True, project=None, - use_lock=False, + use_installed=False, categories="", **kwargs, ): @@ -525,7 +525,7 @@ def check( audit_and_monitor=audit_and_monitor, safety_project=project, pypi_mirror=state.pypi_mirror, - use_lock=use_lock, + use_installed=use_installed, categories=categories, ) diff --git a/pipenv/core.py b/pipenv/core.py index 22872cb1b8..c297e1a7d4 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -2790,7 +2790,7 @@ def do_check( audit_and_monitor=True, safety_project=None, pypi_mirror=None, - use_lock=False, + use_installed=False, categories="", ): import json @@ -2903,14 +2903,14 @@ def do_check( _cmd + ["-m", "pipenv", "requirements", "--categories", f'"{categories}"'], is_verbose=project.s.is_verbose(), ) - elif use_lock: + elif use_installed: target_venv_packages = run_command( - _cmd + ["-m", "pipenv", "requirements"], is_verbose=project.s.is_verbose() + _cmd + ["-m", "pip", "list", "--format=freeze"], + is_verbose=project.s.is_verbose(), ) else: target_venv_packages = run_command( - _cmd + ["-m", "pip", "list", "--format=freeze"], - is_verbose=project.s.is_verbose(), + _cmd + ["-m", "pipenv", "requirements"], is_verbose=project.s.is_verbose() ) temp_requirements = tempfile.NamedTemporaryFile( diff --git a/tests/integration/test_cli.py b/tests/integration/test_cli.py index 11fbca32b3..608ce5a684 100644 --- a/tests/integration/test_cli.py +++ b/tests/integration/test_cli.py @@ -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') @@ -158,11 +158,28 @@ 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 + c = p.pipenv('pip uninstall wheel') + 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: From b5e032b0c3c953c9037b2798293614d91a2242ed Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Fri, 17 Feb 2023 07:57:59 -0500 Subject: [PATCH 6/8] fix issue revealed by tests --- pipenv/core.py | 4 ++-- tests/integration/test_cli.py | 5 ----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/pipenv/core.py b/pipenv/core.py index c297e1a7d4..69fc9ff6e7 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -2900,7 +2900,7 @@ def do_check( if categories: target_venv_packages = run_command( - _cmd + ["-m", "pipenv", "requirements", "--categories", f'"{categories}"'], + ["pipenv", "requirements", "--categories", categories], is_verbose=project.s.is_verbose(), ) elif use_installed: @@ -2910,7 +2910,7 @@ def do_check( ) else: target_venv_packages = run_command( - _cmd + ["-m", "pipenv", "requirements"], is_verbose=project.s.is_verbose() + ["pipenv", "requirements"], is_verbose=project.s.is_verbose() ) temp_requirements = tempfile.NamedTemporaryFile( diff --git a/tests/integration/test_cli.py b/tests/integration/test_cli.py index 608ce5a684..15239f6555 100644 --- a/tests/integration/test_cli.py +++ b/tests/integration/test_cli.py @@ -173,11 +173,6 @@ def test_pipenv_check_check_lockfile_categories(pipenv_instance_pypi, category): c = p.pipenv(f'check --categories={category}') assert c.returncode != 0 assert 'wheel' in c.stdout - c = p.pipenv('pip uninstall wheel') - assert c.returncode == 0 - c = p.pipenv(f'check --categories={category}') - assert c.returncode != 0 - assert 'wheel' in c.stdout @pytest.mark.cli From 6131f8e5f9a214925034ecf41b54c964984bd9f2 Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Fri, 17 Feb 2023 08:06:38 -0500 Subject: [PATCH 7/8] fix docs for pipenv check changes. --- docs/advanced.rst | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/advanced.rst b/docs/advanced.rst index 951528a2d0..e4f49d5735 100644 --- a/docs/advanced.rst +++ b/docs/advanced.rst @@ -390,9 +390,11 @@ It can be used to specify multiple categories also. Pipenv includes the `safety `_ package, and will use it to scan your dependency graph for known security vulnerabilities! -By default ``pipenv check`` will scan the virtualenv packages for what is installed and use this as the input to the safety command. -To have ``pipenv check`` use the ``Pipfile.lock`` to determine package inputs simply pass ``--use-lock`` to use the default packages -category, or pass the specific ``--categories`` you want to check against. +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`` Example:: From fe4168244d01942912e14969aa1f8bb3aca86eec Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Fri, 17 Feb 2023 22:14:41 -0500 Subject: [PATCH 8/8] change conditional ordering in prep for supporting for future default categories env var. --- pipenv/core.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pipenv/core.py b/pipenv/core.py index 69fc9ff6e7..8d047aff5e 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -2898,14 +2898,14 @@ def do_check( if safety_project: options.append(f"--project={safety_project}") - if categories: + if use_installed: target_venv_packages = run_command( - ["pipenv", "requirements", "--categories", categories], + _cmd + ["-m", "pip", "list", "--format=freeze"], is_verbose=project.s.is_verbose(), ) - elif use_installed: + elif categories: target_venv_packages = run_command( - _cmd + ["-m", "pip", "list", "--format=freeze"], + ["pipenv", "requirements", "--categories", categories], is_verbose=project.s.is_verbose(), ) else: