Review comments

Author: spoorcc

CHANGELOG.rst

@@ -15,7 +15,7 @@ Release 0.14.0 (unreleased)
 * Fix ``dfetch add`` crashing with a ``ValueError`` when the remote URL has a trailing slash (#1137)
 * Fix unhelpful error message when a metadata file is malformed (#1145)
 * Fix arbitrary file write via malicious tar/zip symlink (#1152)
-* Prevent ssh command injection (#1152)
+* Prevent SSH command injection (#1152)
 
 Release 0.13.0 (released 2026-03-30)
 ====================================

dfetch/vcs/archive.py

@@ -510,15 +510,11 @@ def _check_no_member_traverses_symlink(members: list[tarfile.TarInfo]) -> None:
         and provides defence-in-depth on newer ones.
 
         Raises:
-            RuntimeError: When a non-symlink member's path passes through a
-                symlink member name.
+            RuntimeError: When a member's path passes through a symlink member
+                name, including symlink-through-symlink chains.
         """
-        symlink_names = {m.name for m in members if m.issym()}
-        if not symlink_names:
-            return
+        symlink_names: set[str] = set()
         for member in members:
-            if member.issym():
-                continue
             parts = pathlib.PurePosixPath(member.name).parts
             for depth in range(1, len(parts)):
                 parent = str(pathlib.PurePosixPath(*parts[:depth]))
@@ -527,6 +523,8 @@ def _check_no_member_traverses_symlink(members: list[tarfile.TarInfo]) -> None:
                         f"Archive member {member.name!r} would be written "
                         f"through symlink {parent!r}"
                     )
+            if member.issym():
+                symlink_names.add(str(pathlib.PurePosixPath(member.name)))
 
     @staticmethod
     def _check_tar_members(tf: tarfile.TarFile) -> None:

tests/test_archive.py

@@ -358,6 +358,33 @@ def _setup(tf: tarfile.TarFile) -> None:
         _check_tar_members(tf)
 
 
+def test_check_tar_members_rejects_two_step_symlink_attack_dotprefix():
+    """Two-step tar-slip via a ./ prefixed symlink name must be rejected."""
+
+    def _setup(tf: tarfile.TarFile) -> None:
+        _add_symlink(tf, "./project/link", "../../outside")
+        content = b"escaped"
+        info = tarfile.TarInfo("project/link/payload.txt")
+        info.size = len(content)
+        tf.addfile(info, io.BytesIO(content))
+
+    tf = _make_tar_with_member(_setup)
+    with pytest.raises(RuntimeError, match="symlink"):
+        _check_tar_members(tf)
+
+
+def test_check_tar_members_rejects_symlink_chain():
+    """A second symlink whose path passes through an earlier symlink must be rejected."""
+
+    def _setup(tf: tarfile.TarFile) -> None:
+        _add_symlink(tf, "project/link", "../../outside")
+        _add_symlink(tf, "project/link/evil", "../payload")
+
+    tf = _make_tar_with_member(_setup)
+    with pytest.raises(RuntimeError, match="symlink"):
+        _check_tar_members(tf)
+
+
 def test_check_tar_members_allows_file_next_to_symlink():
     """A file sibling to a symlink (not written through it) must be accepted."""
 

tests/test_git_vcs.py

@@ -271,3 +271,8 @@ def test_build_git_ssh_command(name, env_ssh, git_config_ssh, expected):
                     mock_logger.debug.assert_called_once()
                 else:
                     mock_logger.debug.assert_not_called()
+
+                if "injection" in name:
+                    mock_logger.warning.assert_called()
+                else:
+                    mock_logger.warning.assert_not_called()