Fix Pipenv-owned CodeQL reliability findings

news/+code-quality-reliability.bugfix.rst

@@ -0,0 +1,2 @@
+Fixed corrupt Pipfile and lockfile errors so they retain the affected path and
+backup location while reporting the file-specific error message.

pipenv/routines/check.py

@@ -74,7 +74,8 @@ def run_pep508_check(project, system, python):
 
 
 def check_pep508_requirements(project, results, quiet):
-    p = plette.Pipfile.load(open(project.pipfile_location))
+    with open(project.pipfile_location) as pipfile:
+        p = plette.Pipfile.load(pipfile)
     p = plette.Lockfile.with_meta_from(p)
     failed = False
 

pipenv/routines/scan.py

@@ -160,7 +160,8 @@ def run_pep508_check(project, system, python):
 
 def check_pep508_requirements(project, results, quiet):
     """Verify PEP 508 environment markers in Pipfile match the current environment."""
-    p = plette.Pipfile.load(open(project.pipfile_location))
+    with open(project.pipfile_location) as pipfile:
+        p = plette.Pipfile.load(pipfile)
     p = plette.Lockfile.with_meta_from(p)
     failed = False
 

pipenv/utils/exceptions.py

@@ -56,8 +56,7 @@ def show(self):
 
 class LockfileCorruptException(FileCorruptException):
     def __init__(self, path, backup_path=None):
-        self.message = self.get_message(path, backup_path=backup_path)
-        super().__init__(self.message)
+        super().__init__(path, backup_path=backup_path)
 
     def get_message(self, path, backup_path=None):
         message = f"ERROR: Failed to load lockfile at {path}"
@@ -68,14 +67,10 @@ def get_message(self, path, backup_path=None):
         message = f"{message}\nYour lockfile is corrupt, {msg}"
         return message
 
-    def show(self, path, backup_path=None):
-        print(self.message, file=sys.stderr, flush=True)
-
 
 class PipfileCorruptException(FileCorruptException):
     def __init__(self, path, backup_path=None):
-        self.message = self.get_message(path, backup_path=backup_path)
-        super().__init__(self.message)
+        super().__init__(path, backup_path=backup_path)
 
     def get_message(self, path, backup_path=None):
         message = f"ERROR: Failed to load Pipfile at {path}"
@@ -86,9 +81,6 @@ def get_message(self, path, backup_path=None):
         message = f"{message}\nYour Pipfile is corrupt, {msg}"
         return message
 
-    def show(self, path, backup_path=None):
-        print(self.message, file=sys.stderr, flush=True)
-
 
 class PipfileNotFound(FileNotFoundError):
     def __init__(self, path, *args, **kwargs):

pipenv/utils/locking.py

@@ -414,7 +414,7 @@ def load(cls, path: Optional[str], create: bool = True) -> "Lockfile":
             backup_path = f"{formatted_path}.bak"
 
             # Show error and create backup
-            LockfileCorruptException.show(formatted_path, backup_path=backup_path)
+            LockfileCorruptException(formatted_path, backup_path=backup_path).show()
             path_obj.rename(backup_path)
 
             # Try loading again after backing up corrupted file

tests/integration/test_project.py

@@ -29,8 +29,10 @@ def test_pipfile_envvar_expansion(pipenv_instance_pypi):
         os.environ["TEST_HOST"] = "localhost:5000"
         project = Project()
         assert project.sources.all[0]["url"] == "https://localhost:5000/simple"
-        assert "localhost:5000" not in str(Pipfile.load(open(p.pipfile_path)))
-        print(str(Pipfile.load(open(p.pipfile_path))))
+        with open(p.pipfile_path) as pipfile:
+            loaded = Pipfile.load(pipfile)
+        assert "localhost:5000" not in str(loaded)
+        print(str(loaded))
 
 
 @pytest.mark.project

tests/unit/test_routine_context.py

@@ -290,10 +290,14 @@ class TestFromCliKeywordOnly:
     def test_from_cli_positional_raises(self):
         # The first positional after cls would land on `system`.
         with pytest.raises(TypeError):
+            # Deliberately violate the keyword-only API to test its contract.
+            # codeql[py/call/wrong-arguments]
             RoutineContext.from_cli(True)  # type: ignore[misc]
 
     def test_from_cli_two_positionals_raises(self):
         with pytest.raises(TypeError):
+            # Deliberately violate the keyword-only API to test its contract.
+            # codeql[py/call/wrong-arguments]
             RoutineContext.from_cli(True, False)  # type: ignore[misc]
 
 

tests/unit/test_utils_exceptions.py

@@ -0,0 +1,28 @@
+from pipenv.utils.exceptions import (
+    LockfileCorruptException,
+    PipfileCorruptException,
+)
+
+
+def test_lockfile_corrupt_exception_preserves_specific_message(capsys, tmp_path):
+    path = tmp_path / "Pipfile.lock"
+    backup_path = tmp_path / "Pipfile.lock.bak"
+
+    error = LockfileCorruptException(path, backup_path=backup_path)
+    error.show()
+
+    assert "Failed to load lockfile" in error.message
+    assert str(path) in error.message
+    assert str(backup_path) in error.message
+    assert error.message in capsys.readouterr().err
+
+
+def test_pipfile_corrupt_exception_preserves_specific_message(capsys, tmp_path):
+    path = tmp_path / "Pipfile"
+
+    error = PipfileCorruptException(path)
+    error.show()
+
+    assert "Failed to load Pipfile" in error.message
+    assert str(path) in error.message
+    assert error.message in capsys.readouterr().err