Skip to content

Commit e257825

Browse files
jawwad-aliclaude
andcommitted
fix(extensions): preserve-capture only from a real dir; reset non-dir dest_dir in main path
Two more symlink-safety fixes to the keep-config reinstall: 1. The preserved-config capture guarded on dest_dir.exists(), which is true for a symlink; iterating a symlinked dest_dir would follow the link and read arbitrary external files into preserved_configs. Require a real directory (is_dir() and not is_symlink()) so only genuine on-disk leftover configs are captured. 2. The main install path cleared dest_dir with shutil.rmtree(dest_dir), which raises on a symlink or file (NotADirectoryError) and didn't benefit from the _reset_dir behavior already used in rollback. Use self._reset_dir(dest_dir) so an unexpected occupant (symlink/file) is handled consistently instead of crashing on path type. Tests: a stray FILE at dest_dir is replaced cleanly (cross-platform; fails before with NotADirectoryError), and the capture ignores a symlinked dest_dir without reading/overwriting external files (skipped without symlink privilege, runs on CI). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5246aaa commit e257825

2 files changed

Lines changed: 66 additions & 3 deletions

File tree

‎src/specify_cli/extensions/__init__.py‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,7 +1519,11 @@ def install_from_directory(
15191519
# secrets (API keys); recreating them with default perms could widen
15201520
# access.
15211521
preserved_configs: dict[str, tuple[bytes, int, float, float]] = {}
1522-
if dest_dir.exists():
1522+
# Require a REAL directory, not a symlink: iterating a symlinked dest_dir
1523+
# would follow the link and read arbitrary external files into
1524+
# preserved_configs. This path only ever legitimately captures on-disk
1525+
# leftover configs inside the real extension directory.
1526+
if dest_dir.is_dir() and not dest_dir.is_symlink():
15231527
for cfg_file in dest_dir.iterdir():
15241528
if (
15251529
cfg_file.is_file()
@@ -1561,8 +1565,10 @@ def install_from_directory(
15611565

15621566
restored_ok = not preserved_configs
15631567
try:
1564-
if dest_dir.exists():
1565-
shutil.rmtree(dest_dir)
1568+
# Reset via _reset_dir (unlink a symlink / rmtree a real dir) rather
1569+
# than a bare rmtree, which raises on a symlink or file at dest_dir.
1570+
# Handles an unexpected occupant consistently with the rollback path.
1571+
self._reset_dir(dest_dir)
15661572
shutil.copytree(source_dir, dest_dir, ignore=ignore_fn)
15671573
# Restore every preserved config. A failure here is NOT swallowed: a
15681574
# config we promised to preserve but could not restore must fail the

‎tests/test_extensions.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1870,6 +1870,63 @@ def copytree_symlinks_destdir_first(src, dst, *args, **kwargs):
18701870
assert config_file.is_file()
18711871
assert "SECRET-VALUE" in config_file.read_text()
18721872

1873+
def test_reinstall_replaces_file_at_dest_dir(self, extension_dir, project_dir):
1874+
"""If a non-directory (a stray file) occupies dest_dir at reinstall, the
1875+
main install path clears it via _reset_dir rather than crashing on
1876+
shutil.rmtree (which raises NotADirectoryError on a file)."""
1877+
manager = ExtensionManager(project_dir)
1878+
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
1879+
ext_dir = project_dir / ".specify" / "extensions" / "test-ext"
1880+
# Drop the registry entry, then replace the extension dir with a file.
1881+
assert manager.remove("test-ext", keep_config=True) is True
1882+
shutil.rmtree(ext_dir)
1883+
ext_dir.write_text("not a directory")
1884+
assert ext_dir.is_file()
1885+
1886+
# Reinstall must succeed, replacing the file with a real extension dir.
1887+
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
1888+
assert ext_dir.is_dir() and not ext_dir.is_symlink()
1889+
assert (ext_dir / "extension.yml").is_file()
1890+
1891+
def test_preserve_capture_ignores_symlinked_dest_dir(
1892+
self, extension_dir, project_dir
1893+
):
1894+
"""The preserve-config capture must not follow a symlinked dest_dir and
1895+
read external files into preserved_configs. Skipped without symlink
1896+
privilege; runs on Linux CI."""
1897+
probe_t = project_dir / "_pt2"
1898+
probe_l = project_dir / "_pl2"
1899+
try:
1900+
probe_t.mkdir()
1901+
probe_l.symlink_to(probe_t, target_is_directory=True)
1902+
except (OSError, NotImplementedError):
1903+
pytest.skip("symlinks not supported in this environment")
1904+
finally:
1905+
if probe_l.is_symlink():
1906+
probe_l.unlink()
1907+
if probe_t.exists():
1908+
shutil.rmtree(probe_t, ignore_errors=True)
1909+
1910+
manager = ExtensionManager(project_dir)
1911+
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
1912+
ext_dir = project_dir / ".specify" / "extensions" / "test-ext"
1913+
assert manager.remove("test-ext", keep_config=True) is True
1914+
1915+
# External dir with a config-looking file that must NOT be captured.
1916+
external = project_dir / "external_configs"
1917+
external.mkdir()
1918+
(external / "test-ext-config.yml").write_text("api_key: EXTERNAL-SECRET")
1919+
# Swap dest_dir for a symlink to the external dir.
1920+
shutil.rmtree(ext_dir)
1921+
ext_dir.symlink_to(external, target_is_directory=True)
1922+
1923+
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
1924+
1925+
# The external secret was never captured, restored, or overwritten.
1926+
assert (external / "test-ext-config.yml").read_text() == "api_key: EXTERNAL-SECRET"
1927+
assert ext_dir.is_dir() and not ext_dir.is_symlink()
1928+
assert not (ext_dir / "test-ext-config.yml").exists()
1929+
18731930

18741931
# ===== CommandRegistrar Tests =====
18751932

0 commit comments

Comments
 (0)