Merge pull request #2271 from Keerthana-64/commit-identity-headers

fix(commit): reject identity fields that alter commit headers

Author: Byron

git/objects/commit.py

@@ -662,6 +662,10 @@ def create_from_tree(
         :return:
             :class:`Commit` object representing the new commit.
 
+        :raise ValueError:
+            If the name or email of the author or committer contains ``<``, ``>`` or a
+            line feed, as these would change the identity headers of the commit.
+
         :note:
             Additional information about the committer and author are taken from the
             environment or from the git configuration. See :manpage:`git-commit-tree(1)`
@@ -786,6 +790,16 @@ def create_from_tree(
     # { Serializable Implementation
 
     def _serialize(self, stream: BytesIO) -> "Commit":
+        # An identity is written as "name <email> date" on a single header line, so a
+        # line feed or an angle bracket inside a name or email moves those boundaries:
+        # it can add header lines, end the headers early, or present another email.
+        # Git drops these three characters when it writes an identity; refuse them
+        # here before anything is written.
+        for actor in (self.author, self.committer):
+            for value in (actor.name, actor.email):
+                if value and any(char in value for char in "<>\n"):
+                    raise ValueError("Commit identity %r must not contain '<', '>' or a line feed" % value)
+
         write = stream.write
         write(("tree %s\n" % self.tree).encode("ascii"))
         for p in self.parents:

test/test_commit.py

@@ -424,6 +424,49 @@ def test_invalid_commit(self):
         self.assertEqual(cmt.author.name, "E.Azer Ko�o�o�oculu", cmt.author.name)
         self.assertEqual(cmt.author.email, "[email protected]", cmt.author.email)
 
+    @with_rw_directory
+    def test_identity_cannot_alter_headers(self, rw_dir):
+        """A name or email must not add header lines or present another identity."""
+        rw_repo = Repo.init(osp.join(rw_dir, "test_identity_headers"))
+        path = osp.join(str(rw_repo.working_tree_dir), "hello.txt")
+        touch(path)
+        rw_repo.index.add([path])
+        tree = rw_repo.index.write_tree()
+        service = Actor("Service", "[email protected]")
+        forged = "committer Forged <[email protected]> 0 +0000"
+
+        for name, email in (
+            # A line feed ends the header line, so the remainder would become headers
+            # of its own, which Git reads before the committer written after them.
+            ("User <[email protected]> 0 +0000\n" + forged, "[email protected]"),
+            ("User", "[email protected]> 0 +0000\n" + forged),
+            # Angle brackets delimit the email, so these would present another one.
+            ("Forged <[email protected]>", "[email protected]"),
+            ("User", "[email protected]> <[email protected]"),
+            ("User>", "[email protected]"),
+            ("User", "<[email protected]"),
+        ):
+            with self.subTest(name=name, email=email):
+                identity = Actor(name, email)
+                with self.assertRaises(ValueError):
+                    Commit.create_from_tree(rw_repo, tree, "message", head=True, author=identity, committer=service)
+                with self.assertRaises(ValueError):
+                    Commit.create_from_tree(rw_repo, tree, "message", head=True, author=service, committer=identity)
+
+        # Nothing was committed along the way.
+        assert not rw_repo.head.is_valid()
+
+        # Other punctuation is still written as given.
+        author = Actor("Dr. J. O'Neil-Smith, Jr.", "[email protected]")
+        commit = Commit.create_from_tree(rw_repo, tree, "message", head=True, author=author, committer=service)
+        stored = Commit(rw_repo, commit.binsha)
+        self.assertEqual(stored.author, author)
+        self.assertEqual(stored.committer, service)
+        self.assertEqual(stored.message, "message")
+
+        with self.assertRaises(ValueError):
+            commit.replace(author=Actor("User\n" + forged, "[email protected]"))
+
     def test_gpgsig(self):
         cmt = self.rorepo.commit()
         with open(fixture_path("commit_with_gpgsig"), "rb") as fd: