patch: Sanitize commit subject in get_summary to prevent format_patch escapes

A malicious commit subject (e.g. ``x/../../x`` or ``x\..\..\x``) could
direct ``porcelain.format_patch`` to write its patch outside the
requested ``outdir`` because ``get_summary`` only replaced spaces with
dashes. Match git's ``format_sanitized_subject``: keep only
``[A-Za-z0-9._]``, collapse other character runs to ``-``, collapse
consecutive ``.``, length-limit, and trim trailing ``.``/``-``.

Author: jelmer

NEWS

@@ -1,3 +1,11 @@
+1.2.3	UNRELEASED
+
+ * SECURITY: Sanitize commit subjects used in
+   ``porcelain.format_patch`` filenames so a malicious subject (e.g.
+   ``x/../../x``) cannot direct the generated patch outside ``outdir``.
+   ``get_summary`` now matches git's ``format_sanitized_subject``.
+   (Jelmer Vernooij)
+
 1.2.2	2026-05-19
 
  * Normalise hex SHAs to binary in ``DiskObjectStore.contains_packed``

dulwich/patch.py

@@ -171,16 +171,66 @@ def write_commit_patch(
         f.write(version.encode(encoding) + b"\n")
 
 
+def _sanitize_subject_for_filename(text: str, max_length: int = 52) -> str:
+    """Sanitize a string for safe use as part of a filename.
+
+    Matches git's ``format_sanitized_subject`` behavior:
+
+    - Only ``[A-Za-z0-9._]`` are kept; other characters become ``-``
+      (collapsed across runs).
+    - Consecutive ``.`` are collapsed to a single ``.``.
+    - The result is truncated to ``max_length`` characters.
+    - Trailing ``.`` and ``-`` are stripped.
+
+    Args:
+      text: Input string (typically a commit subject line).
+      max_length: Maximum length of the returned string.
+
+    Returns: Sanitized string safe to embed in a filename.
+    """
+    result: list[str] = []
+    # 2 = initial, 1 = saw a non-title char, 0 = saw a title char
+    space = 2
+    i = 0
+    text_len = len(text)
+    while i < text_len:
+        c = text[i]
+        if ("A" <= c <= "Z") or ("a" <= c <= "z") or ("0" <= c <= "9") or c in "._":
+            if space == 1:
+                result.append("-")
+            space = 0
+            result.append(c)
+            if c == ".":
+                while i + 1 < text_len and text[i + 1] == ".":
+                    i += 1
+        else:
+            space |= 1
+        i += 1
+        if len(result) >= max_length:
+            break
+
+    return "".join(result)[:max_length].rstrip(".-")
+
+
 def get_summary(commit: "Commit") -> str:
     """Determine the summary line for use in a filename.
 
+    Sanitizes the commit subject so it is safe to use as a filename
+    component, matching git's ``format_sanitized_subject`` behavior:
+    characters outside ``[A-Za-z0-9._]`` are replaced with ``-`` (with
+    runs collapsed) and consecutive ``.`` are collapsed. The result is
+    also length-limited to prevent overly long filenames.
+
     Args:
       commit: Commit
-    Returns: Summary string
+    Returns: Sanitized summary string suitable for use as a filename
+      component.
     """
     decoded = commit.message.decode(errors="replace")
     lines = decoded.splitlines()
-    return lines[0].replace(" ", "-") if lines else ""
+    if not lines:
+        return ""
+    return _sanitize_subject_for_filename(lines[0])
 
 
 #  Unified Diff

tests/porcelain/__init__.py

@@ -3155,6 +3155,76 @@ def test_format_patch_no_commits(self) -> None:
         )
         self.assertEqual(patches, [])
 
+    def test_format_patch_subject_cannot_escape_outdir(self) -> None:
+        # A malicious commit subject must not be able to direct the
+        # generated patch file outside the requested output directory.
+        tree1 = Tree()
+        c1 = make_commit(tree=tree1, message=b"Initial commit")
+        self.repo.object_store.add_objects([(tree1, None), (c1, None)])
+
+        blob = Blob.from_string(b"data")
+        tree2 = Tree()
+        tree2.add(b"f.txt", 0o100644, blob.id)
+        # Subjects that try to traverse out of outdir via path separators
+        # or parent-directory components.
+        for evil in (b"x/../../x", b"x\\..\\..\\x", b"a:b"):
+            c2 = make_commit(
+                tree=tree2,
+                parents=[c1.id],
+                message=evil,
+            )
+            self.repo.object_store.add_objects(
+                [(blob, None), (tree2, None), (c2, None)]
+            )
+            self.repo[b"HEAD"] = c2.id
+
+            with tempfile.TemporaryDirectory() as tmpdir:
+                patches = porcelain.format_patch(
+                    self.repo.path,
+                    committish=c2.id,
+                    outdir=tmpdir,
+                )
+                self.assertEqual(len(patches), 1)
+                real_outdir = os.path.realpath(tmpdir)
+                real_patch = os.path.realpath(patches[0])
+                self.assertEqual(
+                    os.path.dirname(real_patch),
+                    real_outdir,
+                    f"patch for subject {evil!r} escaped outdir: {patches[0]}",
+                )
+                # Filename must not contain path separators or parent refs.
+                base = os.path.basename(patches[0])
+                self.assertNotIn("/", base)
+                self.assertNotIn("\\", base)
+                self.assertNotIn("..", base)
+
+    def test_format_patch_long_subject_truncated(self) -> None:
+        tree1 = Tree()
+        c1 = make_commit(tree=tree1, message=b"Initial commit")
+        self.repo.object_store.add_objects([(tree1, None), (c1, None)])
+
+        blob = Blob.from_string(b"data")
+        tree2 = Tree()
+        tree2.add(b"f.txt", 0o100644, blob.id)
+        c2 = make_commit(
+            tree=tree2,
+            parents=[c1.id],
+            message=b"a" * 500,
+        )
+        self.repo.object_store.add_objects([(blob, None), (tree2, None), (c2, None)])
+        self.repo[b"HEAD"] = c2.id
+
+        with tempfile.TemporaryDirectory() as tmpdir:
+            patches = porcelain.format_patch(
+                self.repo.path,
+                committish=c2.id,
+                outdir=tmpdir,
+            )
+            self.assertEqual(len(patches), 1)
+            # Whatever the cap is, an extremely long subject must not
+            # produce a pathologically long filename.
+            self.assertLess(len(os.path.basename(patches[0])), 100)
+
 
 class SymbolicRefTests(PorcelainTestCase):
     def test_set_wrong_symbolic_ref(self) -> None:

tests/test_patch.py

@@ -641,19 +641,64 @@ def test_object_diff_kind_change(self) -> None:
 
 
 class GetSummaryTests(TestCase):
-    def test_simple(self) -> None:
-        c = make_commit(
+    def _make_commit(self, message: bytes) -> Commit:
+        return make_commit(
             author=b"Jelmer <jelmer@samba.org>",
             committer=b"Jelmer <jelmer@samba.org>",
             author_time=1271350201,
             commit_time=1271350201,
             author_timezone=0,
             commit_timezone=0,
-            message=b"This is the first line\nAnd this is the second line.\n",
+            message=message,
             tree=Tree().id,
         )
+
+    def test_simple(self) -> None:
+        c = self._make_commit(
+            b"This is the first line\nAnd this is the second line.\n",
+        )
         self.assertEqual("This-is-the-first-line", get_summary(c))
 
+    def test_empty_message(self) -> None:
+        c = self._make_commit(b"")
+        self.assertEqual("", get_summary(c))
+
+    def test_path_traversal_unix(self) -> None:
+        c = self._make_commit(b"x/../../x\n")
+        # Path separators and consecutive dots must be sanitized so the
+        # result cannot escape the requested output directory.
+        summary = get_summary(c)
+        self.assertNotIn("/", summary)
+        self.assertNotIn("..", summary)
+        self.assertEqual("x-.-.-x", summary)
+
+    def test_path_traversal_windows(self) -> None:
+        c = self._make_commit(b"x\\..\\..\\x\n")
+        summary = get_summary(c)
+        self.assertNotIn("\\", summary)
+        self.assertNotIn("..", summary)
+        self.assertEqual("x-.-.-x", summary)
+
+    def test_colon_sanitized(self) -> None:
+        c = self._make_commit(b"feature: do something\n")
+        summary = get_summary(c)
+        self.assertNotIn(":", summary)
+        self.assertEqual("feature-do-something", summary)
+
+    def test_long_subject_truncated(self) -> None:
+        c = self._make_commit(b"a" * 500 + b"\n")
+        summary = get_summary(c)
+        self.assertLessEqual(len(summary), 64)
+
+    def test_trailing_dots_and_dashes_stripped(self) -> None:
+        c = self._make_commit(b"hello...\n")
+        self.assertEqual("hello", get_summary(c))
+
+    def test_only_problematic_chars(self) -> None:
+        c = self._make_commit(b"/\\:..\n")
+        # After sanitization there is nothing usable left.
+        self.assertEqual("", get_summary(c))
+
 
 class DiffAlgorithmTests(TestCase):
     """Tests for diff algorithm selection."""