From c1bf0fdba0fef48b45ff4df7b83f3ae6176f47fa Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 31 Oct 2026 15:04:26 +0530 Subject: [PATCH] fix(desktop-plugins): host reconcile treats Windows junctions as links The Webapp host's desktop-half pass asked Path.is_symlink() everywhere it means "is this a link", and that is False for a Windows junction. A junction needs no privilege to create, so an installed package whose desktop/ folder held one published the folder it points at into the served desktop-plugins root, which Electron's lstat walk (Node reports a junction as a symbolic link) refuses. A checkout junctioned into plugins/ also lost its linked marker, so the two writers disagreed about the same copy, and a junctioned copy whose package is gone was never pruned because rmtree refuses a junction. One predicate now covers both kinds (the existing pm.filesystem is_junction beside is_symlink) at every link check in the module: the plain-tree check, the linked-checkout test, the link-at-target test, publication and the prune. Follow-up to #146402. The is_symlink-only checks date from f9e923f190 (#145853) and ca8e43aea7 (#146178). --- hermes_cli/desktop_plugin_halves.py | 19 ++- .../test_desktop_plugin_halves_host_fs.py | 127 ++++++++++++++++++ 2 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 tests/hermes_cli/test_desktop_plugin_halves_host_fs.py diff --git a/hermes_cli/desktop_plugin_halves.py b/hermes_cli/desktop_plugin_halves.py index 93193985d2..f6aedae5ce 100644 --- a/hermes_cli/desktop_plugin_halves.py +++ b/hermes_cli/desktop_plugin_halves.py @@ -26,6 +26,7 @@ import tempfile import threading from pathlib import Path +from pm.filesystem import is_junction from utils import rmtree_readonly logger = logging.getLogger(__name__) @@ -42,6 +43,12 @@ _GIT_ORIGIN = re.compile(r'\[remote "origin"\][^[]*?url\s*=\s*(\S+)') ROOT_WRITER = threading.Lock() +def _is_link(path: Path) -> bool: + """A symlink or a Windows junction: Node's ``lstat`` reports both as a link, and Electron's + checks of the same folder go by that. ``Path.is_symlink()`` alone misses a junction.""" + return path.is_symlink() or is_junction(path) + + def _list_dirs(folder: Path, *, follow_links: bool = False) -> list[str]: """Folder names in *folder*. A package root follows links to folders (``ln -s ~/src/pkg ~/.hermes/plugins/`` is how a package is developed in place), the app root does not.""" @@ -135,13 +142,13 @@ def check_desktop_tree(source: Path, base: Path, *, linked_root: bool = False, *linked_root* accepts *source* itself being a link (a dev checkout linked into ``plugins/``); every entry inside it is still held to the rule. Folders named in *skip* are left out of the copy (:func:`root_layout_ignore`), so they are not walked.""" - if source.is_symlink() and not linked_root: + if _is_link(source) and not linked_root: return f"The desktop plugin contains a link ({source.relative_to(base)}); links are not installed." total = 0 for folder, dirs, files in os.walk(source): dirs[:] = [n for n in dirs if n not in skip] for entry in (Path(folder) / n for n in dirs + files): - if entry.is_symlink(): + if _is_link(entry): return f"The desktop plugin contains a link ({entry.relative_to(base)}); links are not installed." total += sum((Path(folder) / n).stat().st_size for n in files) if total > TREE_MAX_BYTES: @@ -165,7 +172,7 @@ def publish_desktop_tree(source_dir: Path, target: Path, marker: dict | None, ig (staged / PACKAGE_MARKER).unlink(missing_ok=True) else: _write_marker(staged, marker) - if target.is_symlink(): + if _is_link(target): target.unlink() elif target.exists(): rmtree_readonly(target) @@ -206,11 +213,11 @@ def materialize_desktop_half(package_dir: Path, app_root: Path, name: str) -> Pa target = app_root / name existing = read_package_marker(target) - linked = package_dir.is_symlink() + linked = _is_link(package_dir) # A link at ``/`` is never a copy this pass may keep or stamp: a marker-less one is # the user's own linked plugin, and one carrying a marker is an earlier copy of a linked checkout # that kept the link instead of the files (Electron's ``cp`` did), its marker inside the checkout. - target_is_link = target.is_symlink() + target_is_link = _is_link(target) if target_is_link and existing is None: return None marker = { @@ -307,7 +314,7 @@ def reconcile_desktop_halves_report(hermes_root: Path, app_root: Path) -> dict: folder = app_root / name marker = read_package_marker(folder) if marker and _source_gone(name, Path(marker["source"]) / "plugin.js"): - if folder.is_symlink(): + if _is_link(folder): folder.unlink(missing_ok=True) else: shutil.rmtree(folder, ignore_errors=True) diff --git a/tests/hermes_cli/test_desktop_plugin_halves_host_fs.py b/tests/hermes_cli/test_desktop_plugin_halves_host_fs.py new file mode 100644 index 0000000000..500ffd35c2 --- /dev/null +++ b/tests/hermes_cli/test_desktop_plugin_halves_host_fs.py @@ -0,0 +1,127 @@ +"""Host filesystem behaviour of the desktop-half pass. + +The host pass treats a Windows junction as the link it is, as Electron's ``lstat`` does. + +``Path.is_symlink()`` is False for a junction, and a junction needs no privilege to make on Windows, +so a check that only asks ``is_symlink()`` publishes whatever folder a junction points at. The +real-junction tests run on the Windows lane; the stubbed ones run the same contract on every host. +""" + +import json +import os +from pathlib import Path + +import pytest + +from hermes_cli import desktop_plugin_halves as halves +from hermes_cli.desktop_plugin_halves import PACKAGE_MARKER, reconcile_desktop_halves_report + + +def _package(folder: Path) -> None: + (folder / "desktop").mkdir(parents=True) + (folder / "plugin.yaml").write_text("name: media\n", encoding="utf-8") + (folder / "desktop" / "plugin.js").write_text("export default {}", encoding="utf-8") + + +def _junction(target: Path, link: Path) -> None: + import _winapi + + _winapi.CreateJunction(str(target), str(link)) + + +def _stub_junctions(monkeypatch, *paths: Path) -> None: + """Report *paths* as junctions (a plain folder on this host), the reparse tag Windows would give.""" + wanted = {os.fspath(p) for p in paths} + monkeypatch.setattr(halves, "is_junction", lambda p: os.fspath(p) in wanted) + + +def _secret_folder(tmp_path: Path) -> Path: + secret = tmp_path / "secret" + secret.mkdir() + (secret / "token.txt").write_text("host secret", encoding="utf-8") + return secret + + +@pytest.mark.platforms("windows") +def test_a_junction_inside_an_installed_desktop_folder_is_not_published(tmp_path): + root, app_root = tmp_path / "hermes", tmp_path / "hermes" / "desktop-plugins" + package = root / "plugins" / "media" + _package(package) + _junction(_secret_folder(tmp_path), package / "desktop" / "assets") + + report = reconcile_desktop_halves_report(root, app_root) + assert report["touched"] == [] + assert [entry["name"] for entry in report["refused"]] == ["media"] + assert not (app_root / "media").exists() + + +@pytest.mark.platforms("windows") +def test_a_dev_checkout_junctioned_into_plugins_is_published_as_linked(tmp_path): + """Junctions are how a checkout is linked in on Windows without Developer Mode. Electron's lstat + calls the folder a link and stamps ``linked``; the host must agree, or each writer re-copies + the other's half on every pass.""" + root, app_root = tmp_path / "hermes", tmp_path / "hermes" / "desktop-plugins" + checkout = tmp_path / "src" / "media" + _package(checkout) + (root / "plugins").mkdir(parents=True) + _junction(checkout, root / "plugins" / "media") + + assert reconcile_desktop_halves_report(root, app_root) == {"touched": [str(app_root / "media")], "refused": []} + marker = json.loads((app_root / "media" / PACKAGE_MARKER).read_text(encoding="utf-8")) + assert marker["linked"] is True + + +@pytest.mark.platforms("windows") +def test_pruning_a_junctioned_copy_removes_the_junction_not_the_folder_it_points_at(tmp_path): + """An old copy that kept a link to a checkout, its marker inside the checkout: once the package is + gone only the junction goes. ``shutil.rmtree`` refuses a junction, so it used to stay forever.""" + root, app_root = tmp_path / "hermes", tmp_path / "hermes" / "desktop-plugins" + checkout = tmp_path / "src" / "media" + checkout.mkdir(parents=True) + (checkout / "plugin.js").write_text("export default {}", encoding="utf-8") + marker = {"package": "media", "source": str(tmp_path / "gone" / "desktop"), "sourceMtimeMs": 1} + (checkout / PACKAGE_MARKER).write_text(json.dumps(marker), encoding="utf-8") + app_root.mkdir(parents=True) + _junction(checkout, app_root / "media") + + assert reconcile_desktop_halves_report(root, app_root)["touched"] == [str(app_root / "media")] + assert not os.path.lexists(app_root / "media") + assert (checkout / "plugin.js").read_text(encoding="utf-8") == "export default {}" + + +@pytest.mark.parametrize("junction", ["desktop", "desktop/assets"]) +def test_a_folder_the_host_reports_as_a_junction_is_refused_like_a_link(tmp_path, monkeypatch, junction): + root, app_root = tmp_path / "hermes", tmp_path / "hermes" / "desktop-plugins" + package = root / "plugins" / "media" + _package(package) + (package / "desktop" / "assets").mkdir() + _stub_junctions(monkeypatch, package / junction) + + report = reconcile_desktop_halves_report(root, app_root) + assert [entry["name"] for entry in report["refused"]] == ["media"] + assert "link (desktop" in report["refused"][0]["reason"] + assert not (app_root / "media").exists() + + +def test_a_package_folder_the_host_reports_as_a_junction_is_a_linked_checkout(tmp_path, monkeypatch): + root, app_root = tmp_path / "hermes", tmp_path / "hermes" / "desktop-plugins" + package = root / "plugins" / "media" + _package(package) + _stub_junctions(monkeypatch, package) + + assert reconcile_desktop_halves_report(root, app_root)["touched"] == [str(app_root / "media")] + marker = json.loads((app_root / "media" / PACKAGE_MARKER).read_text(encoding="utf-8")) + assert marker["linked"] is True + + +def test_a_plain_folder_the_host_reports_as_a_junction_at_the_target_is_left_alone(tmp_path, monkeypatch): + """A junction at ``/`` with no marker is the user's own linked plugin: the pass + neither publishes over it nor deletes it, as it does for a symlink.""" + root, app_root = tmp_path / "hermes", tmp_path / "hermes" / "desktop-plugins" + _package(root / "plugins" / "media") + (app_root / "media").mkdir(parents=True) + (app_root / "media" / "mine.txt").write_text("mine", encoding="utf-8") + _stub_junctions(monkeypatch, app_root / "media") + + assert reconcile_desktop_halves_report(root, app_root) == {"touched": [], "refused": []} + assert (app_root / "media" / "mine.txt").read_text(encoding="utf-8") == "mine"