Review comments

Author: spoorcc

dfetch/vcs/archive.py

@@ -387,17 +387,24 @@ def check_zip_members(zf: zipfile.ZipFile) -> list[zipfile.ZipInfo]:
 
     @staticmethod
     def _is_unsafe_symlink_target(target: str) -> bool:
-        r"""Return *True* when *target* is an absolute symlink destination.
+        r"""Return *True* when *target* is an unsafe symlink destination.
 
         Absolute targets (POSIX ``/`` or Windows drive/UNC anchors) are
-        always unsafe regardless of context.  Relative targets that contain
-        ``..`` components are NOT rejected here; they are validated after
-        extraction by :meth:`_check_symlinks_in_dest` using the manifest
-        root as the safety boundary.
+        always unsafe.  Relative targets containing ``..`` components are
+        also rejected: during :meth:`tarfile.TarFile.extractall` an
+        already-extracted symlink can be followed when writing a later
+        member, so a ``..``-based target can escape *dest_dir* before
+        :meth:`_check_symlinks_in_dest` runs.  That method remains as a
+        secondary check for any case not caught here.
         """
         posix = pathlib.PurePosixPath(target)
         win = pathlib.PureWindowsPath(target)
-        return posix.is_absolute() or bool(win.anchor)
+        return (
+            posix.is_absolute()
+            or bool(win.anchor)
+            or ".." in posix.parts
+            or ".." in win.parts
+        )
 
     @staticmethod
     def _check_symlinks_in_dest(dest_dir: str) -> None:

features/fetch-archive.feature

@@ -254,3 +254,4 @@ Feature: Fetching dependencies from an archive (tar/zip)
                             link.mk
                 dfetch.yaml
             """
+        And 'MyProject/SomeProject/sub/dir/link.mk' is a symlink pointing to '../../other/target.mk'

features/steps/generic_steps.py

@@ -363,6 +363,15 @@ def step_impl(_, name):
     assert os.path.exists(name), f"Expected {name} to exist, but it didn't!"
 
 
+@then("'{path}' is a symlink pointing to '{target}'")
+def step_impl(_, path, target):
+    assert os.path.islink(
+        path
+    ), f"Expected {path!r} to be a symbolic link, but it is not"
+    actual = os.readlink(path)
+    assert actual == target, f"Expected {path!r} to point to {target!r}, got {actual!r}"
+
+
 def multisub(patterns: List[Tuple[Pattern[str], str]], text: str) -> str:
     """Apply a list of tuples that each contain a regex + replace string."""
     for pattern, replace in patterns: