Show "not from cwd" test is broken for shell=True

EliahKagan · EliahKagan · commit 1c65efbe7351 · 2023-12-26T11:06:14.000-05:00
This adds a test_it_executes_git_not_from_cwd case for shell=True. (This case also gives the command as a string, so the test need not be further special-cased for non-Windows systems, where argument lists aren't accepted with shell=True.) The test did not attempt to cover the shell=True case before, because I had erroneously assumed it worked similarity. It is actually very different, because when a shell is used, both the shell and the command the shell runs must be found and executed, and because the process creation GitPython performs is that of the shell process, with the state of the shell process being what is relevant to how the path search is done for the git (or other) command. The code change here does not itself demonstrate that the test is broken for shell=True, because that case passes. However, manually undoing the fix in cmd.py for CVE-2023-40590, which as expected causes the preexisting (implicitly shell=False case) to fail, does *not* cause the new shell=True case to fail. That case passes! That passing result in the absence of a fix for CVE-2023-40590 is erroneous, because the cmd.exe shell does search the CWD first when nothing has been done to prevent it.
diff --git a/test/test_git.py b/test/test_git.py
@@ -134,7 +134,13 @@ def test_it_logs_istream_summary_for_stdin(self, case):
     def test_it_executes_git_and_returns_result(self):
         self.assertRegex(self.git.execute(["git", "version"]), r"^git version [\d\.]{2}.*$")
 
-    def test_it_executes_git_not_from_cwd(self):
+    @ddt.data(
+        (["git", "version"], False),
+        ("git version", True),
+    )
+    def test_it_executes_git_not_from_cwd(self, case):
+        command, shell = case
+
         with TemporaryDirectory() as tmpdir:
             if os.name == "nt":
                 # Copy an actual binary executable that is not git.
@@ -149,7 +155,9 @@ def test_it_executes_git_not_from_cwd(self):
                 os.chmod(impostor_path, 0o755)
 
             with cwd(tmpdir):
-                self.assertRegex(self.git.execute(["git", "version"]), r"^git version\b")
+                output = self.git.execute(command, shell=shell)
+
+        self.assertRegex(output, r"^git version\b")
 
     @skipUnless(
         os.name == "nt",
