mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(windows): MSIX terminal and MCP children get node, npm and python copied outside the package
In the process tree that the Store app's execution alias starts, a program whose own executable is outside WindowsApps cannot start one inside it. cmd.exe gets "Access is denied" from the packaged node.exe, and npm starts every package bin through cmd.exe. So every npx MCP server died, and so did each `npm exec` in the terminal on a machine without its own Node. A venv's Scripts\python.exe redirector exits 101 when it starts the packaged interpreter, which broke `python -m venv`. A copy of the same files in the writable store starts without error. PM already makes one for Python builds (#135236). That code moves from pm._uv._toolchain to pm.user_facing.writable_copy. pm.user_facing.copies swaps each packaged npm, node and python dir for that copy, made on first use from the verified entry: a copy plus a digest check, no download. Two places use it: - the terminal's fallback dirs (user_facing_path), behind the user's own tools; - Hermes's npx/npm/node for a bare `command: npx` MCP server, which stays Hermes's own as on main. When lazy installs are off, PM makes no copy and the packaged dir stays. Live check on Windows 11 arm64 (Store build 2026.6819.1065.0), in the alias-started process tree: cmd.exe -> npx.cmd -> node.exe fails with "Access is denied" from the package, and passes from a copy of the same node and npm dirs in the user profile.
This commit is contained in:
4 files changed
+172
-31
No files matched your search
@@ -3,12 +3,10 @@ from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from pm import paths
|
||||
from pm.filesystem import long_root, native
|
||||
from pm.lock import Facts
|
||||
from pm.filesystem import native
|
||||
from pm.package import InstallError
|
||||
from pm.registry import get_package
|
||||
from pm.store import Store, current_target
|
||||
from pm.store import current_target
|
||||
|
||||
|
||||
def _toolchain(*, realize: bool = True, explicit: bool = False) -> tuple[Path, Path] | None:
|
||||
@@ -20,10 +18,8 @@ def _toolchain(*, realize: bool = True, explicit: bool = False) -> tuple[Path, P
|
||||
packaged interpreter dies with exit 101 (#135236). Hermes itself still enters those venvs
|
||||
on the packaged interpreter (``pm.environments.venv_command``).
|
||||
"""
|
||||
from pm.install import (
|
||||
_install, _installed_location, _lockfile, _refuse_lazy,
|
||||
ensure, lazy_installs_allowed, sealed,
|
||||
)
|
||||
from pm.install import _installed_location, _lockfile, ensure, sealed
|
||||
from pm.user_facing import writable_copy
|
||||
|
||||
if realize:
|
||||
ensure("uv", explicit=explicit)
|
||||
@@ -39,20 +35,10 @@ def _toolchain(*, realize: bool = True, explicit: bool = False) -> tuple[Path, P
|
||||
return None
|
||||
raise InstallError(name, "pinned tool is unavailable", "run `hermes pm install`")
|
||||
if name == "python" and target.startswith("win32") and sealed():
|
||||
# Spelled like Store spells its root, so the comparison and the copy's file calls agree.
|
||||
writable = long_root(paths.writable_store_root())
|
||||
if location[1].root != writable:
|
||||
copied = _installed_location(package, lockfile, target, verify=explicit, roots=(writable,))
|
||||
if copied is None:
|
||||
if not realize:
|
||||
return None
|
||||
if not explicit and not lazy_installs_allowed():
|
||||
raise _refuse_lazy(name, "a writable Python is required for bundled builds")
|
||||
copy_store = Store(writable)
|
||||
copy_facts = Facts(writable / "facts.json")
|
||||
_install(package, lockfile, copy_facts, copy_store, target, copy_from=location)
|
||||
copied = copy_facts, copy_store
|
||||
location = copied
|
||||
location = writable_copy(name, realize=realize, explicit=explicit,
|
||||
why="a writable Python is required for bundled builds")
|
||||
if location is None:
|
||||
return None
|
||||
facts, store = location
|
||||
binary = package.binary(store.entry(facts.get(name)["entry"]), target)
|
||||
if binary is None or not binary.is_file():
|
||||
|
||||
+106
-8
@@ -5,12 +5,109 @@ code resolves the pinned binaries. Terminal commands belong to the user and the
|
||||
run the tools the user installed, and Hermes's tools are the fallback for a machine without
|
||||
them. Hermes's own operations must not depend on this PATH: they resolve pinned tools
|
||||
through PM (``pm.env_for``, ``pm.installed_package``).
|
||||
|
||||
A sealed Windows payload's store sits under ``WindowsApps``. In the process tree that the
|
||||
app's execution alias starts, a program whose own executable is outside the package cannot
|
||||
start one inside it: cmd.exe running npm's ``npx.cmd`` gets "Access is denied" from the
|
||||
packaged ``node.exe``, and a venv's ``Scripts\\python.exe`` redirector exits 101 when it
|
||||
starts the packaged Python. A copy of the same files outside the package starts without
|
||||
error, so there :func:`copies` swaps those dirs for PM's verified copies.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
# Packages whose packaged dirs a sealed Windows payload swaps for writable copies.
|
||||
COPIED = ("npm", "node", "python")
|
||||
|
||||
|
||||
def applies() -> bool:
|
||||
"""True for a sealed payload on Windows, the only layout with the restriction above."""
|
||||
from pm.install import sealed
|
||||
from pm.store import current_target
|
||||
|
||||
return current_target().startswith("win32") and sealed()
|
||||
|
||||
|
||||
def writable_copy(name: str, *, realize: bool = True, explicit: bool = False,
|
||||
why: str = "a copy outside the package is required"):
|
||||
"""``(facts, store)`` of PM's verified copy of the installed *name* in the writable store.
|
||||
|
||||
The first call copies the installed entry and checks its digest. It downloads nothing.
|
||||
If the install already lives in the writable store, that install is the answer. Returns
|
||||
``None`` when *name* is not installed, or when no copy exists and *realize* is false.
|
||||
Raises the lazy-install refusal, naming *why*, when a copy is needed and lazy installs
|
||||
are off.
|
||||
"""
|
||||
from pm import paths
|
||||
from pm.filesystem import long_root
|
||||
from pm.install import _install, _installed_location, _lockfile, _refuse_lazy, lazy_installs_allowed
|
||||
from pm.lock import Facts
|
||||
from pm.registry import get_package
|
||||
from pm.store import Store, current_target
|
||||
|
||||
package, lockfile, target = get_package(name), _lockfile(), current_target()
|
||||
location = _installed_location(package, lockfile, target)
|
||||
if location is None:
|
||||
return None
|
||||
# Spelled like Store spells its root, so the comparison and the copy's file calls agree.
|
||||
writable = long_root(paths.writable_store_root())
|
||||
if location[1].root == writable:
|
||||
return location
|
||||
copied = _installed_location(package, lockfile, target, verify=explicit, roots=(writable,))
|
||||
if copied is not None or not realize:
|
||||
return copied
|
||||
if not explicit and not lazy_installs_allowed():
|
||||
raise _refuse_lazy(name, why)
|
||||
copy_store, copy_facts = Store(writable), Facts(writable / "facts.json")
|
||||
_install(package, lockfile, copy_facts, copy_store, target, copy_from=location)
|
||||
return copy_facts, copy_store
|
||||
|
||||
|
||||
def _dirs(facts, store, name: str) -> list[str]:
|
||||
"""The PATH dirs that *store*'s install of *name* contributes, natively spelled."""
|
||||
from pm.filesystem import native
|
||||
|
||||
dirs = facts.env_for(name, store.root).get("PATH") or []
|
||||
return [native(d) for d in ([dirs] if isinstance(dirs, str) else dirs) if d]
|
||||
|
||||
|
||||
def _sealed_dirs(name: str) -> list[str]:
|
||||
from pm.install import _facts, _store
|
||||
|
||||
facts, store = _facts(), _store()
|
||||
return _dirs(facts, store, name) if facts.installed(name, None, store.root) else []
|
||||
|
||||
|
||||
def _copy_dirs(name: str) -> list[str]:
|
||||
"""The PATH dirs of *name*'s writable copy, made on first use, or ``[]`` when PM may not make one."""
|
||||
from pm.package import InstallError
|
||||
|
||||
try:
|
||||
location = writable_copy(name)
|
||||
except (InstallError, OSError) as exc:
|
||||
LOG.debug("user-facing %s stays the packaged copy: %s", name, exc)
|
||||
return []
|
||||
return _dirs(*location, name) if location is not None else []
|
||||
|
||||
|
||||
def copies(dirs: list[str]) -> list[str]:
|
||||
"""*dirs* with each packaged npm, node and python dir swapped for its writable copy on a
|
||||
sealed Windows payload. A packaged dir stays when PM may not copy it (lazy installs off)."""
|
||||
if not applies():
|
||||
return list(dirs)
|
||||
swap: dict[str, list[str]] = {}
|
||||
for name in COPIED:
|
||||
sealed = _sealed_dirs(name)
|
||||
copied = _copy_dirs(name) if sealed else []
|
||||
if copied:
|
||||
swap.update((os.path.normcase(d), copied) for d in sealed)
|
||||
return list(dict.fromkeys(c for d in dirs for c in swap.get(os.path.normcase(d), [d])))
|
||||
|
||||
|
||||
def _python_stub_dir(directory: str) -> bool:
|
||||
"""True for the Windows app-alias dir when its ``python.exe`` is only the Store stub.
|
||||
@@ -27,27 +124,28 @@ def _python_stub_dir(directory: str) -> bool:
|
||||
return True
|
||||
|
||||
|
||||
def _python_dirs() -> set[str]:
|
||||
def _python_dirs() -> list[str]:
|
||||
from pm.install import env_for
|
||||
|
||||
path = env_for("python", base_env={"PATH": ""}).get("PATH", "")
|
||||
return {os.path.normcase(d) for d in path.split(os.pathsep) if d}
|
||||
return [d for d in env_for("python", base_env={"PATH": ""}).get("PATH", "").split(os.pathsep) if d]
|
||||
|
||||
|
||||
def user_facing_path(path: str) -> str:
|
||||
"""*path* with Hermes's store tool dirs moved behind the user's own entries.
|
||||
"""*path* with Hermes's store tool dirs (their writable copies, see :func:`copies`) moved
|
||||
behind the user's own entries.
|
||||
|
||||
Hermes's Python stops just before the app-alias dir that holds the Store stub, so a
|
||||
machine without Python still runs a real one.
|
||||
"""
|
||||
from pm.install import _store_path_dirs
|
||||
|
||||
ours = _store_path_dirs()
|
||||
if not ours:
|
||||
store = _store_path_dirs()
|
||||
if not store:
|
||||
return path
|
||||
keys = {os.path.normcase(d) for d in ours}
|
||||
ours = copies(store)
|
||||
keys = {os.path.normcase(d) for d in (*store, *ours)}
|
||||
kept = [p for p in path.split(os.pathsep) if p and os.path.normcase(p) not in keys]
|
||||
python = _python_dirs()
|
||||
python = {os.path.normcase(d) for d in copies(_python_dirs())}
|
||||
cut = next((i for i, p in enumerate(kept) if _python_stub_dir(p)), len(kept))
|
||||
early = [d for d in ours if os.path.normcase(d) in python]
|
||||
late = [d for d in ours if os.path.normcase(d) not in python]
|
||||
|
||||
@@ -13,7 +13,7 @@ def test_user_tools_come_before_hermes_and_python_stops_at_the_store_stub(tmp_pa
|
||||
stub = tmp_path / "AppData" / "Local" / "Microsoft" / "WindowsApps"
|
||||
stub.mkdir(parents=True)
|
||||
monkeypatch.setattr(pm.install, "_store_path_dirs", lambda: [npm, node, python, rg])
|
||||
monkeypatch.setattr(pm.user_facing, "_python_dirs", lambda: {os.path.normcase(python)})
|
||||
monkeypatch.setattr(pm.user_facing, "_python_dirs", lambda: [python])
|
||||
path = os.pathsep.join([npm, node, python, rg, "/usr/bin", str(stub), "/home/u/bin"])
|
||||
|
||||
# The stub alias would open the Store, so a machine without Python still runs ours.
|
||||
@@ -27,3 +27,46 @@ def test_user_tools_come_before_hermes_and_python_stops_at_the_store_stub(tmp_pa
|
||||
|
||||
monkeypatch.setattr(pm.install, "_store_path_dirs", list)
|
||||
assert user_facing_path(path) == path
|
||||
|
||||
|
||||
def test_a_sealed_windows_payload_swaps_packaged_tools_for_writable_copies(tmp_path, monkeypatch):
|
||||
"""cmd.exe cannot start an exe inside WindowsApps; a verified copy outside it can."""
|
||||
from pm import paths, registry
|
||||
from pm.lock import Facts, Lockfile
|
||||
from pm.packages import Nodejs, Python
|
||||
from pm.store import current_target, tree_digest
|
||||
from pm.user_facing import copies
|
||||
|
||||
shipped, writable = tmp_path / "payload" / "tools", tmp_path / "home" / "tools"
|
||||
target, digest = current_target(), "1" * 64
|
||||
lock, facts = Lockfile(tmp_path / "lock.json"), Facts(shipped / "facts.json")
|
||||
bins = {}
|
||||
for package in (Nodejs(), Python()):
|
||||
package.verify = lambda entry, target: ""
|
||||
monkeypatch.setitem(registry._packages, package.name, package)
|
||||
lock.set_pin(package.name, "test", {target: {"url": "https://test.invalid", "sha256": digest}})
|
||||
entry = shipped / package.store_entry("test", target)
|
||||
binary = package.binary(entry, target)
|
||||
binary.parent.mkdir(parents=True)
|
||||
binary.write_bytes(b"pinned " + package.name.encode())
|
||||
facts.record(package.name, "test", entry.name, package.env(entry, target), shipped,
|
||||
target=target, artifacts=[digest], digest=tree_digest(entry))
|
||||
bins[package.name] = {root: str(package.binary(root / entry.name, target).parent)
|
||||
for root in (shipped, writable)}
|
||||
lock.save()
|
||||
monkeypatch.setattr(paths, "lockfile_path", lambda: lock.path)
|
||||
monkeypatch.setattr(paths, "store_root", lambda: shipped)
|
||||
monkeypatch.setattr(paths, "writable_store_root", lambda: writable)
|
||||
monkeypatch.setattr(pm.user_facing, "applies", lambda: True)
|
||||
path = os.pathsep.join([bins["python"][shipped], bins["node"][shipped], "/usr/bin"])
|
||||
|
||||
# Lazy installs off: PM makes no copy, so the packaged dirs stay as the fallback.
|
||||
monkeypatch.setattr(pm.install, "lazy_installs_allowed", lambda: False)
|
||||
assert user_facing_path(path).split(os.pathsep) == [
|
||||
"/usr/bin", bins["python"][shipped], bins["node"][shipped]]
|
||||
assert not writable.exists()
|
||||
|
||||
monkeypatch.setattr(pm.install, "lazy_installs_allowed", lambda: True)
|
||||
assert user_facing_path(path).split(os.pathsep) == [
|
||||
"/usr/bin", bins["python"][writable], bins["node"][writable]]
|
||||
assert copies([bins["node"][shipped]]) == [bins["node"][writable]]
|
||||
@@ -261,6 +261,18 @@ def _managed_launcher(command: str) -> Optional[tuple[str, list[str]]]:
|
||||
return executable, dirs
|
||||
|
||||
|
||||
def _outside_package(launcher: str, managed: str, dirs: list[str]) -> tuple[str, list[str]]:
|
||||
"""Hermes's npm/node *launcher* and toolchain *dirs*, swapped for PM's copies outside a
|
||||
sealed Windows payload (``pm.user_facing.copies``). npm starts each package bin through
|
||||
cmd.exe, and cmd.exe cannot start the packaged ``node.exe``."""
|
||||
from pm.user_facing import copies
|
||||
|
||||
moved = copies(dirs)
|
||||
if moved == dirs:
|
||||
return managed, dirs
|
||||
return shutil.which(launcher, path=os.pathsep.join(moved)) or managed, moved
|
||||
|
||||
|
||||
def _is_hermes_managed_bin_dir(directory: str) -> bool:
|
||||
"""True for the bin dirs Hermes' bootstrap prepends to this process's PATH: anything
|
||||
under the active hermes home (the sealed payload's venv, ``<home>/bin``, PM store
|
||||
@@ -342,6 +354,8 @@ def _resolve_stdio_command(command: str, env: dict) -> tuple[str, dict]:
|
||||
managed = _managed_launcher(launcher) if launcher in _MANAGED_LAUNCHERS else None
|
||||
if managed is not None:
|
||||
resolved_command, dirs = managed
|
||||
if _MANAGED_LAUNCHERS[launcher] == "npm":
|
||||
resolved_command, dirs = _outside_package(launcher, resolved_command, dirs)
|
||||
# Moved to the front even when already on PATH behind a user's copy.
|
||||
keys = {os.path.normcase(d) for d in dirs}
|
||||
rest = [p for p in resolved_env.get("PATH", "").split(os.pathsep) if p and os.path.normcase(p) not in keys]
|
||||
|
||||
Reference in new issue
Block a user