From 9e5d0aaa0542c0e861dbbb33d48225f4745f56a5 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Tue, 7 Nov 2023 16:54:44 -0500
Subject: [PATCH] Avoid subprocess-writable temp file race condition

This lets the Windows subprocess open or rename onto the temporary
file using a more robust approach than in b12fd4a, avoiding the
race condition described there, where the filename could be
inadvertently reused between deletion and recreation of the file.

This creates a context manager helper for the temporary index file
used in IndexFile.from_tree, whose implementation differs by
operating system:

- Delegating straightforwardly to NamedTempoaryFile on POSIX
  systems where an open file can replaced by having another file
  renamed to it (just as it can be deleted).

- Employing custom logic on Windows, using mkstemp, closing the
  temporary file without immediately deleting it (so it won't be
  reused by any process seeking to create a temporary file), and
  then deleting it on context manager exit.

IndexFile.from_tree now calls this helper instead of
NamedTemporaryFile. For convenience, the helper provides the path,
i.e. the "name", when entered, so tmp_index is now just that path.

(At least for now, this helper is implemented as a nonpublic
function in the git.index.base module, rather than in the
git.index.util module. If it were public in git.index.util like the
other facilities there, then some later changes to it, or its later
removal, would be a breaking change to GitPython. If it were
nonpublic in git.index.util, then this would not be a concern, but
it would be unintuitive for it to be accessed from code in the
git.index.base module. In the future, one way to address this might
be to have one or more nonpublic _util modules with public members.
Because it would still be a breaking change to drop existing public
util modules, that would be more utility modules in total, so such
a change isn't included here just for this one used-once function.)
---
 git/index/base.py | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/git/index/base.py b/git/index/base.py
index 4fa0714d8..6c6462039 100644
--- a/git/index/base.py
+++ b/git/index/base.py
@@ -6,7 +6,7 @@
 """Module containing IndexFile, an Index implementation facilitating all kinds of index
 manipulations such as querying and merging."""
 
-from contextlib import ExitStack
+import contextlib
 import datetime
 import glob
 from io import BytesIO
@@ -67,6 +67,7 @@
     BinaryIO,
     Callable,
     Dict,
+    Generator,
     IO,
     Iterable,
     Iterator,
@@ -96,6 +97,27 @@
 __all__ = ("IndexFile", "CheckoutError", "StageType")
 
 
+@contextlib.contextmanager
+def _named_temporary_file_for_subprocess(directory: PathLike) -> Generator[str, None, None]:
+    """Create a named temporary file git subprocesses can open, deleting it afterward.
+
+    :param directory: The directory in which the file is created.
+
+    :return: A context manager object that creates the file and provides its name on
+        entry, and deletes it on exit.
+    """
+    if os.name == "nt":
+        fd, name = tempfile.mkstemp(dir=directory)
+        os.close(fd)
+        try:
+            yield name
+        finally:
+            os.remove(name)
+    else:
+        with tempfile.NamedTemporaryFile(dir=directory) as ctx:
+            yield ctx.name
+
+
 class IndexFile(LazyMixin, git_diff.Diffable, Serializable):
     """An Index that can be manipulated using a native implementation in order to save
     git command function calls wherever possible.
@@ -359,11 +381,9 @@ def from_tree(cls, repo: "Repo", *treeish: Treeish, **kwargs: Any) -> "IndexFile
 
         # tmp file created in git home directory to be sure renaming
         # works - /tmp/ dirs could be on another device.
-        with ExitStack() as stack:
-            tmp_index = stack.enter_context(tempfile.NamedTemporaryFile(dir=repo.git_dir))
-            if os.name == "nt":
-                tmp_index.close()
-            arg_list.append("--index-output=%s" % tmp_index.name)
+        with contextlib.ExitStack() as stack:
+            tmp_index = stack.enter_context(_named_temporary_file_for_subprocess(repo.git_dir))
+            arg_list.append("--index-output=%s" % tmp_index)
             arg_list.extend(treeish)
 
             # Move current index out of the way - otherwise the merge may fail
@@ -373,7 +393,7 @@ def from_tree(cls, repo: "Repo", *treeish: Treeish, **kwargs: Any) -> "IndexFile
 
             stack.enter_context(TemporaryFileSwap(join_path_native(repo.git_dir, "index")))
             repo.git.read_tree(*arg_list, **kwargs)
-            index = cls(repo, tmp_index.name)
+            index = cls(repo, tmp_index)
             index.entries  # Force it to read the file as we will delete the temp-file.
             return index
             # END index merge handling