diff --git a/hermes_cli/plugins_cmd_catalog.py b/hermes_cli/plugins_cmd_catalog.py index 7c8f10bfb1..7ca07f9a83 100644 --- a/hermes_cli/plugins_cmd_catalog.py +++ b/hermes_cli/plugins_cmd_catalog.py @@ -357,12 +357,20 @@ def _carry_user_files(old: Path, new: Path, local: Optional[list[str]]) -> None: such as ``data/``; descendants of those entries are copied and win over same-path files in the new tree. ``None`` means there is no git checkout, so user-state files absent from the new tree are carried while executable/declarative plugin surfaces remain revision-owned. If a - user-owned path cannot be represented safely in the new tree, fail before publication rather - than silently dropping it. + user-owned path cannot be represented safely in the new tree (a layout clash, or a symlink in a + git checkout's untracked/ignored set), fail before publication rather than silently dropping it. """ from hermes_cli.plugins_cmd import PluginOperationError keep = {Path(rel) for rel in local or ()} + linked: list[str] = [] + + def _user_link(rel: Path) -> None: + # Git-owned user state that is a symlink is refused, never followed: a link injected after the + # installer's scan could point outside the plugin root past the guard (which skips links). + # Links under node_modules/ are reproducible install artefacts (.bin shims), not user state. + if local is not None and not keep.isdisjoint((rel, *rel.parents)) and "node_modules" not in rel.parts: + linked.append(rel.as_posix()) def _walk_error(exc: OSError) -> None: raise PluginOperationError(f"Could not preserve user files from '{old}': {exc}") from exc @@ -374,15 +382,16 @@ def _carry_user_files(old: Path, new: Path, local: Optional[list[str]]) -> None: for dirpath, dirnames, filenames in os.walk(old, onerror=_walk_error): here = Path(dirpath) - dirnames[:] = [ - name - for name in dirnames - if not _skip_preserve(name) + walk = [] + for name in dirnames: # Without git, top-level revision-owned dirs are never carried: do not even walk them. - and not (local is None and here == old and name in _NO_GIT_REVISION_DIRS) - and not (here / name).is_symlink() - and not is_junction(here / name) - ] + if _skip_preserve(name) or (local is None and here == old and name in _NO_GIT_REVISION_DIRS): + continue + if (here / name).is_symlink() or is_junction(here / name): + _user_link((here / name).relative_to(old)) + continue + walk.append(name) + dirnames[:] = walk for name in filenames: # Skipped directories are pruned above, so only the file name itself needs checking. if _skip_preserve(name): @@ -400,9 +409,11 @@ def _carry_user_files(old: Path, new: Path, local: Optional[list[str]]) -> None: src_mode = src.lstat().st_mode except OSError as exc: raise PluginOperationError(f"Could not preserve user file '{rel}': {exc}") from exc - # Only regular files are durable state. FIFOs, sockets and devices are runtime objects, - # and a symlink injected after the installer's first scan could point outside the plugin - # root past the guard (which skips links), so neither branch carries them. + # Only regular files are durable state. FIFOs, sockets and devices are runtime objects. + # Symlinks are never carried; in a git checkout a user-owned one fails the update below. + if stat.S_ISLNK(src_mode): + _user_link(rel) + continue if not stat.S_ISREG(src_mode): continue @@ -441,6 +452,12 @@ def _carry_user_files(old: Path, new: Path, local: Optional[list[str]]) -> None: if dst.is_symlink() or dst.is_file(): dst.unlink() shutil.copy2(src, dst, follow_symlinks=False) + if linked: + raise PluginOperationError( + f"Cannot preserve symlinked user file(s) {', '.join(sorted(linked))}: links are not followed " + "into an update. Replace each with a regular file (or remove it) and retry. " + "The installed plugin was left unchanged." + ) class RepinResult(NamedTuple): diff --git a/tests/hermes_cli/test_plugins_cmd_catalog.py b/tests/hermes_cli/test_plugins_cmd_catalog.py index 73d55c38f1..a1d52a8612 100644 --- a/tests/hermes_cli/test_plugins_cmd_catalog.py +++ b/tests/hermes_cli/test_plugins_cmd_catalog.py @@ -279,6 +279,20 @@ def test_repin_keeps_a_wholly_ignored_data_dir_in_a_git_checkout(world): assert _head(target) == world["state"]["pin"] assert (target / "data" / "db" / "index.db").read_text() == "user data" + # A symlink in the ignored set is never followed into the update: it fails closed, naming the path, + # and the live plugin stays at its current revision with its user data. + published = world["state"]["pin"] + outside = repo.parent / "outside.yaml" + outside.write_text("secret") + (target / "data" / "link.yaml").symlink_to(outside) + (repo / "__init__.py").write_text("def register(ctx):\n pass # v4\n") + world["state"]["pin"] = _commit(repo, "v4") + result = pc.dashboard_update_user_plugin("cat-plugin") + assert result["ok"] is False and "data/link.yaml" in result["error"] + assert _head(target) == published + assert (target / "data" / "link.yaml").is_symlink() + assert (target / "data" / "db" / "index.db").read_text() == "user data" + def test_kill_list_covers_update_enable_and_load_of_an_installed_plugin(world, tmp_path, monkeypatch): """A URL install whose name lands on the kill list AFTER install must stop pulling, cannot be enabled diff --git a/website/docs/user-guide/features/plugin-catalog.md b/website/docs/user-guide/features/plugin-catalog.md index b9f0a847f4..581999769e 100644 --- a/website/docs/user-guide/features/plugin-catalog.md +++ b/website/docs/user-guide/features/plugin-catalog.md @@ -196,6 +196,8 @@ catalog moved (via a reviewed PR), prepares and dependency-validates the new SHA before publishing it. Your enabled/disabled state is preserved, and so are files the plugin's repo does not track (the `config.yaml` created from its `.example`, data files, `.env`). +Symlinks among those untracked files are never followed into the new code: the +update stops before publishing and names them, so replace each with a regular file. For monorepo/subdirectory installs, which do not carry a local Git checkout, update preserves user-state files the new revision does not ship. Plugin code and control surfaces remain revision-owned and are not resurrected from the old install: