fix: guard diff output options

codex · Byron · commit 040de4b1985c · 2026-07-22T04:46:13.000+02:00
Reject unsafe diff options before revision parsing or Git invocation so callers cannot write command output to arbitrary filesystem paths. Cover commit and index diffs, including option-like revisions, and preserve an explicit allow_unsafe_options escape hatch. References GHSA-fjr4-x663-mwxc. Validated against Git baseline a23bace9.
diff --git a/git/diff.py b/git/diff.py
@@ -9,7 +9,7 @@
 import re
 import warnings
 
-from git.cmd import handle_process_output
+from git.cmd import Git, handle_process_output
 from git.compat import defenc
 from git.objects.blob import Blob
 from git.objects.util import mode_str_to_int
@@ -35,7 +35,6 @@
 if TYPE_CHECKING:
     from subprocess import Popen
 
-    from git.cmd import Git
     from git.objects.base import IndexObject
     from git.objects.commit import Commit
     from git.objects.tree import Tree
@@ -190,6 +189,7 @@ def diff(
         other: Union[DiffConstants, "Tree", "Commit", str, None] = INDEX,
         paths: Union[PathLike, List[PathLike], Tuple[PathLike, ...], None] = None,
         create_patch: bool = False,
+        allow_unsafe_options: bool = False,
         **kwargs: Any,
     ) -> "DiffIndex[Diff]":
         """Create diffs between two items being trees, trees and index or an index and
@@ -219,6 +219,10 @@ def diff(
             applied makes the self to other. Patches are somewhat costly as blobs have
             to be read and diffed.
 
+        :param allow_unsafe_options:
+            If ``True``, allow options such as ``--output`` that can write to arbitrary
+            filesystem paths.
+
         :param kwargs:
             Additional arguments passed to :manpage:`git-diff(1)`, such as ``R=True`` to
             swap both sides of the diff.
@@ -231,6 +235,12 @@ def diff(
             an instance of :class:`~git.objects.tree.Tree` or
             :class:`~git.objects.commit.Commit`, or a git command error will occur.
         """
+        if not allow_unsafe_options:
+            Git.check_unsafe_options(
+                options=Git._option_candidates([other], kwargs),
+                unsafe_options=self.repo.unsafe_git_revision_options,
+            )
+
         args: List[Union[PathLike, Diffable]] = []
         args.append("--abbrev=40")  # We need full shas.
         args.append("--full-index")  # Get full index paths, not only filenames.
diff --git a/git/index/base.py b/git/index/base.py
@@ -23,6 +23,7 @@
 from gitdb.db import MemoryDB
 
 from git.compat import defenc, force_bytes
+from git.cmd import Git
 import git.diff as git_diff
 from git.exc import CheckoutError, GitCommandError, GitError, InvalidGitRepositoryError
 from git.objects import Blob, Commit, Object, Submodule, Tree
@@ -1492,6 +1493,7 @@ def diff(
         ] = git_diff.INDEX,
         paths: Union[PathLike, List[PathLike], Tuple[PathLike, ...], None] = None,
         create_patch: bool = False,
+        allow_unsafe_options: bool = False,
         **kwargs: Any,
     ) -> git_diff.DiffIndex[git_diff.Diff]:
         """Diff this index against the working copy or a :class:`~git.objects.tree.Tree`
@@ -1504,6 +1506,12 @@ def diff(
             Will only work with indices that represent the default git index as they
             have not been initialized with a stream.
         """
+        if not allow_unsafe_options:
+            Git.check_unsafe_options(
+                options=Git._option_candidates([other], kwargs),
+                unsafe_options=self.repo.unsafe_git_revision_options,
+            )
+
         # Only run if we are the default repository index.
         if self._file_path != self._index_path():
             raise AssertionError("Cannot call %r on indices that do not represent the default git index" % self.diff())
@@ -1560,12 +1568,18 @@ def diff(
             # Invert the existing R flag.
             cur_val = kwargs.get("R", False)
             kwargs["R"] = not cur_val
-            return other.diff(self.INDEX, paths, create_patch, **kwargs)
+            return other.diff(
+                self.INDEX,
+                paths,
+                create_patch,
+                allow_unsafe_options=allow_unsafe_options,
+                **kwargs,
+            )
         # END diff against other item handling
 
         # If other is not None here, something is wrong.
         if other is not None:
             raise ValueError("other must be None, Diffable.INDEX, a Tree or Commit, was %r" % other)
 
         # Diff against working copy - can be handled by superclass natively.
-        return super().diff(other, paths, create_patch, **kwargs)
+        return super().diff(other, paths, create_patch, allow_unsafe_options=allow_unsafe_options, **kwargs)
diff --git a/test/test_diff.py b/test/test_diff.py
@@ -14,6 +14,7 @@
 
 from git import NULL_TREE, Diff, DiffIndex, Diffable, GitCommandError, Repo, Submodule
 from git.cmd import Git
+from git.exc import UnsafeOptionError
 
 from test.lib import StringProcessAdapter, TestBase, fixture, with_rw_directory
 
@@ -352,6 +353,25 @@ def test_diff_submodule(self):
         self.assertIsInstance(diff.a_blob.size, int)
         self.assertIsInstance(diff.b_blob.size, int)
 
+    def test_diff_rejects_unsafe_output_options(self):
+        commit = self.rorepo.head.commit
+
+        calls = (
+            lambda target: commit.diff(output=target),
+            lambda target: commit.diff(other=f"--output={target}"),
+            lambda target: self.rorepo.index.diff(NULL_TREE, output=target),
+            lambda target: self.rorepo.index.diff(f"--output={target}"),
+        )
+        for index, call in enumerate(calls):
+            target = osp.join(self.repo_dir, f"diff-output-{index}")
+            with self.assertRaises(UnsafeOptionError):
+                call(target)
+            self.assertFalse(osp.exists(target))
+
+        allowed_target = osp.join(self.repo_dir, "allowed-diff-output")
+        commit.diff(output=allowed_target, allow_unsafe_options=True)
+        self.assertTrue(osp.isfile(allowed_target))
+
     def test_diff_interface(self):
         """Test a few variations of the main diff routine."""
         assertion_map = {}
