mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
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 fromf9e923f190(#145853) andca8e43aea7(#146178).
This commit is contained in:
1 parent
04a979bddb
commit
c1bf0fdba0
2 files changed
+140
-6
No files matched your search
@@ -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 ``<app_root>/<name>`` 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)
|
||||
|
||||
@@ -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 ``<app_root>/<name>`` 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"
|
||||
Reference in new issue
Block a user