fix(extensions): keep-config restore must not follow symlinks; split mode/time restore

Two hardening fixes to the reinstall keep-config restore loop:

1. Never write *through* a non-regular file at the config path. If dest_cfg
   is a symlink (copied with symlinks=True, or swapped in by a racing process
   between copytree and the restore), write_bytes() would follow it and
   clobber an arbitrary target outside the extension dir. Drop any symlink
   (unlink, never follow) or stray directory (rmtree) first, so write_bytes()
   always creates a fresh regular file — and skip the entry if the path can't
   be made safe rather than clobber.

2. Restore mode and mtime independently. The single try/except meant a chmod
   failure skipped os.utime (and vice versa); either can succeed on its own,
   so each gets its own guard.

Tests: a symlink-at-path case (proves the write doesn't follow the link to an
external victim; skipped where symlinks need privilege, runs on CI) and a
directory-at-path case (cross-platform: fails/crashes before the guard, passes
after). Behavior for the normal regular-file case is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: jawwad-ali

src/specify_cli/extensions/__init__.py

@@ -1441,9 +1441,27 @@ def install_from_directory(
         # path, which uses .backup/ rather than an in-place leftover.
         for name, (data, mode, atime, mtime) in preserved_configs.items():
             dest_cfg = dest_dir / name
+            # Never write *through* a symlink (or a non-regular-file) at this
+            # path: if dest_cfg is a symlink — copied with symlinks=True, or
+            # swapped in by a racing process between the copytree above and
+            # here — write_bytes() would follow it and clobber an arbitrary
+            # target outside the extension dir. Drop any non-regular entry
+            # first so write_bytes() always creates a fresh regular file.
+            try:
+                if dest_cfg.is_symlink():
+                    dest_cfg.unlink()  # remove the link itself, never follow it
+                elif dest_cfg.is_dir():
+                    shutil.rmtree(dest_cfg)
+            except OSError:
+                continue  # can't make the path safe — skip rather than clobber
             dest_cfg.write_bytes(data)
+            # Restore mode and timestamps independently: a failure to set one
+            # must not prevent the other, since either can succeed on its own.
             try:
                 os.chmod(dest_cfg, mode & 0o7777)  # permission bits only
+            except OSError:
+                pass
+            try:
                 os.utime(dest_cfg, (atime, mtime))  # and timestamps
             except OSError:
                 pass

tests/test_extensions.py

@@ -1591,6 +1591,98 @@ def test_reinstall_preserves_keep_config_leftover(self, extension_dir, project_d
         if platform.system() != "Windows":
             assert config_file.stat().st_mode & 0o777 == 0o600
 
+    def test_reinstall_config_restore_does_not_follow_symlink(
+        self, extension_dir, project_dir, monkeypatch
+    ):
+        """The keep-config restore must never write *through* a symlink. If the
+        config path is a symlink at restore time (e.g. swapped in between the
+        copytree and the restore), the write must not follow it and clobber the
+        external target — it must replace the link with a fresh regular file."""
+        # Probe symlink capability (Windows usually needs privilege).
+        probe_target = project_dir / "_probe_target"
+        probe_link = project_dir / "_probe_link"
+        try:
+            probe_target.write_text("x")
+            probe_link.symlink_to(probe_target)
+        except (OSError, NotImplementedError):
+            pytest.skip("symlinks not supported in this environment")
+        finally:
+            probe_link.unlink(missing_ok=True)
+            probe_target.unlink(missing_ok=True)
+
+        manager = ExtensionManager(project_dir)
+        manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
+
+        ext_dir = project_dir / ".specify" / "extensions" / "test-ext"
+        config_file = ext_dir / "test-ext-config.yml"
+        config_file.write_text("api_key: MY-CUSTOMIZED-VALUE")
+
+        # keep-config removal: registry entry dropped, config left in place.
+        assert manager.remove("test-ext", keep_config=True) is True
+
+        # A victim file outside the extension dir that the symlink points at.
+        victim = project_dir / "victim.txt"
+        victim.write_text("DO-NOT-OVERWRITE")
+
+        # Simulate the config path being a symlink to the victim right after the
+        # fresh copytree (the TOCTOU / symlinks=True case the guard defends).
+        real_copytree = shutil.copytree
+
+        def copytree_then_symlink(src, dst, *args, **kwargs):
+            result = real_copytree(src, dst, *args, **kwargs)
+            link = Path(dst) / "test-ext-config.yml"
+            if link.exists() or link.is_symlink():
+                link.unlink()
+            link.symlink_to(victim)
+            return result
+
+        monkeypatch.setattr(shutil, "copytree", copytree_then_symlink)
+
+        # Reinstall: restore must drop the symlink, not follow it.
+        manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
+
+        # The victim outside the extension dir is untouched.
+        assert victim.read_text() == "DO-NOT-OVERWRITE"
+        # The config path is now a regular file (not a symlink) with the
+        # preserved content written in place.
+        assert not config_file.is_symlink()
+        assert config_file.is_file()
+        assert "MY-CUSTOMIZED-VALUE" in config_file.read_text()
+
+    def test_reinstall_config_restore_replaces_directory_at_path(
+        self, extension_dir, project_dir, monkeypatch
+    ):
+        """Cross-platform guard check: if a directory occupies the config path at
+        restore time, write_bytes() would raise (uncaught, crashing the whole
+        reinstall). The guard must clear the directory and write a fresh regular
+        file. Exercises the same non-regular-file guard as the symlink case, but
+        needs no symlink privilege so it runs everywhere (incl. Windows)."""
+        manager = ExtensionManager(project_dir)
+        manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
+
+        ext_dir = project_dir / ".specify" / "extensions" / "test-ext"
+        config_file = ext_dir / "test-ext-config.yml"
+        config_file.write_text("api_key: MY-CUSTOMIZED-VALUE")
+        assert manager.remove("test-ext", keep_config=True) is True
+
+        # Simulate a directory occupying the config path right after copytree.
+        real_copytree = shutil.copytree
+
+        def copytree_then_dir(src, dst, *args, **kwargs):
+            result = real_copytree(src, dst, *args, **kwargs)
+            clash = Path(dst) / "test-ext-config.yml"
+            clash.mkdir()
+            (clash / "sentinel").write_text("stray")
+            return result
+
+        monkeypatch.setattr(shutil, "copytree", copytree_then_dir)
+
+        # Must not raise, and must restore a regular file with preserved content.
+        manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
+
+        assert config_file.is_file()
+        assert "MY-CUSTOMIZED-VALUE" in config_file.read_text()
+
 
 # ===== CommandRegistrar Tests =====