fix(extensions): preserve keep-config leftover on reinstall (data loss)

src/specify_cli/extensions/__init__.py

@@ -1397,13 +1397,75 @@ def install_from_directory(
                 backup_config_dir.unlink()
             did_remove = self.remove(manifest.id)
 
+        # A prior `remove(keep_config=True)` leaves the top-level
+        # *-config.yml / *-config.local.yml in place while dropping the
+        # registry entry. A later reinstall is not a --force removal
+        # (did_remove is False), so the rmtree below would wipe those
+        # preserved configs and the backup-restore path never runs. Capture
+        # them here and write them back after the fresh copytree, mirroring
+        # the *-config filter the --force restore path uses.
+        # Capture (bytes, mode, atime, mtime) so restore preserves both
+        # permissions and timestamps — truly mirroring the --force restore's
+        # shutil.copy2 (which preserves mode/mtime). Config files may hold
+        # secrets (API keys); recreating them with default perms could widen
+        # access.
+        preserved_configs: dict[str, tuple[bytes, int, float, float]] = {}
+        if dest_dir.exists():
+            for cfg_file in dest_dir.iterdir():
+                if (
+                    cfg_file.is_file()
+                    and not cfg_file.is_symlink()
+                    and (
+                        cfg_file.name.endswith("-config.yml")
+                        or cfg_file.name.endswith("-config.local.yml")
+                    )
+                ):
+                    st = cfg_file.stat()
+                    preserved_configs[cfg_file.name] = (
+                        cfg_file.read_bytes(),
+                        st.st_mode,
+                        st.st_atime,
+                        st.st_mtime,
+                    )
+
         # Install extension (dest_dir computed above during self-install guard)
         if dest_dir.exists():
             shutil.rmtree(dest_dir)
 
         ignore_fn = self._load_extensionignore(source_dir)
         shutil.copytree(source_dir, dest_dir, ignore=ignore_fn)
 
+        # Restore configs preserved from a keep-config leftover (see above),
+        # preserving the original file mode and timestamps. The --force
+        # backup-restore below (did_remove) still takes precedence for that
+        # 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
+
         # Register commands with AI agents
         registered_commands = {}
         if register_commands:

tests/test_extensions.py

@@ -1557,6 +1557,132 @@ def test_config_backup_on_remove(self, extension_dir, project_dir):
         assert backup_file.exists()
         assert backup_file.read_text() == "test: config"
 
+    def test_reinstall_preserves_keep_config_leftover(self, extension_dir, project_dir):
+        """A reinstall after `remove(keep_config=True)` must not wipe the
+        preserved config. remove(keep_config=True) leaves the *-config.yml in
+        place and drops the registry entry; the subsequent reinstall is not a
+        --force removal, so the unconditional rmtree used to destroy it."""
+        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")
+        # Restrictive perms — a config may hold secrets; the restore must not
+        # widen them (POSIX only; Windows chmod ignores group/other bits).
+        config_file.chmod(0o600)
+        # Distinctive mtime to prove timestamps survive too (mirrors copy2).
+        old_mtime = 1_600_000_000
+        os.utime(config_file, (old_mtime, old_mtime))
+
+        # keep-config removal: registry entry dropped, config left in place.
+        assert manager.remove("test-ext", keep_config=True) is True
+        assert config_file.exists()
+
+        # Reinstall (no --force; registry now reports not-installed).
+        manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
+
+        # The user's config must survive the reinstall (was silently wiped).
+        assert config_file.exists()
+        assert "MY-CUSTOMIZED-VALUE" in config_file.read_text()
+        # ...and keep its original mtime (copy2-style timestamp preservation).
+        assert config_file.stat().st_mtime == old_mtime
+        # ...and its original (restrictive) mode, not default perms.
+        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 =====