From 052d818569d0845ba26efc9214060d411354be69 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 26 Sep 2026 23:16:58 +0530 Subject: [PATCH] fix(plugins): never walk guard-excluded dirs when carrying user files The carry walk exempted links under tools.plugin_guard.EXCLUDED_DIRS from the symlink refusal but still descended into .venv/, node_modules/ and tool caches and copied every regular file below them. The staged tree got a venv with pyvenv.cfg but no bin/python and a node_modules without .bin shims; the carried node_modules also made _refresh_declared_dependencies skip `npm ci` when the lockfile was unchanged, publishing the broken copy. Base never carried ignored directories at all. Prune EXCLUDED_DIRS in the walk's directory filter so the rule lives in one place, and drop the now-unreachable EXCLUDED_DIRS clause in _user_link. node_modules stays in _NO_GIT_REVISION_DIRS: _revision_owned_without_git also uses it for a top-level *file* of that name, so it is not redundant. The ignored-data test now asserts .venv/ and node_modules/ are not carried while ignored user data still is. --- hermes_cli/plugins_cmd_catalog.py | 14 +++++++++----- tests/hermes_cli/test_plugins_cmd_catalog.py | 7 ++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/hermes_cli/plugins_cmd_catalog.py b/hermes_cli/plugins_cmd_catalog.py index a7ddbb4077..e904ee3191 100644 --- a/hermes_cli/plugins_cmd_catalog.py +++ b/hermes_cli/plugins_cmd_catalog.py @@ -359,6 +359,8 @@ def _carry_user_files(old: Path, new: Path, local: Optional[list[str]]) -> list[ tree are carried while executable/declarative plugin surfaces remain revision-owned. If a 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. + Dirs in ``tools.plugin_guard.EXCLUDED_DIRS`` (``.venv/``, ``node_modules/``, tool caches) are + install artefacts: they are neither carried nor inspected, so links inside them never stop an update. Returns the carried paths (POSIX, relative to the tree) so a later scan block can name them. """ from hermes_cli.plugins_cmd import PluginOperationError @@ -371,9 +373,7 @@ def _carry_user_files(old: Path, new: Path, local: Optional[list[str]]) -> list[ 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 the guard's excluded dirs (node_modules/.bin shims, .venv/bin/python) are - # reproducible install artefacts, not user state. - if local is not None and not keep.isdisjoint((rel, *rel.parents)) and EXCLUDED_DIRS.isdisjoint(rel.parts): + if local is not None and not keep.isdisjoint((rel, *rel.parents)): linked.append(rel.as_posix()) def _walk_error(exc: OSError) -> None: @@ -388,8 +388,12 @@ def _carry_user_files(old: Path, new: Path, local: Optional[list[str]]) -> list[ here = Path(dirpath) walk = [] for name in dirnames: - # Without git, top-level revision-owned dirs are never carried: do not even walk them. - if _skip_preserve(name) or (local is None and here == old and name in _NO_GIT_REVISION_DIRS): + # The guard's excluded dirs (.venv, node_modules, tool caches) are reproducible install + # artefacts, not user state: never walk or carry them, so the fresh tree rebuilds them whole + # (a partial copy has no bin/python or .bin shims and suppresses `npm ci`). Without git, + # top-level revision-owned dirs are never carried either. + if (_skip_preserve(name) or name in EXCLUDED_DIRS + 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)) diff --git a/tests/hermes_cli/test_plugins_cmd_catalog.py b/tests/hermes_cli/test_plugins_cmd_catalog.py index 06c0a47b6a..9f5e360302 100644 --- a/tests/hermes_cli/test_plugins_cmd_catalog.py +++ b/tests/hermes_cli/test_plugins_cmd_catalog.py @@ -264,7 +264,7 @@ def test_update_of_a_subdir_install_keeps_files_the_user_created_or_edited(world def test_repin_keeps_a_wholly_ignored_data_dir_in_a_git_checkout(world): """A single ``!! data/`` status entry must preserve every file below that ignored directory.""" repo = world["repo"] - (repo / ".gitignore").write_text("data/\n.venv/\n") + (repo / ".gitignore").write_text("data/\n.venv/\nnode_modules/\n") world["state"]["pin"] = _commit(repo, "ignore data") target = cat.install_catalog_entry(pc_cat.get_live_catalog_entry("cat-plugin"), force=False)[0] assert (target / ".git").exists() @@ -274,6 +274,9 @@ def test_repin_keeps_a_wholly_ignored_data_dir_in_a_git_checkout(world): # An ignored venv always holds symlinks (bin/python); it is a reproducible artefact, not user state. (target / ".venv" / "bin").mkdir(parents=True) (target / ".venv" / "bin" / "python").symlink_to("/usr/bin/python3") + (target / ".venv" / "pyvenv.cfg").write_text("home = /usr/bin") + (target / "node_modules" / "x").mkdir(parents=True) + (target / "node_modules" / "x" / "index.js").write_text("module.exports = 1") (repo / "__init__.py").write_text("def register(ctx):\n pass # v3\n") world["state"]["pin"] = _commit(repo, "v3") @@ -281,6 +284,8 @@ 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" + # Excluded dependency dirs are not carried at all: a partial copy would be a broken install. + assert not (target / ".venv").exists() and not (target / "node_modules").exists() # 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.