Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 43 additions & 4 deletions cron/scheduler_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,16 +131,55 @@ def _windows_cron_python_invocation(python_exe: str) -> tuple[str, dict[str, str
interpreter = sibling

from hermes_cli._launchers import resolve_store_python
from pm.environments import selected_venv, site_packages as dependency_site
from pm.environments import committed_venv, site_packages as dependency_site

repo = Path(__file__).resolve().parents[1]
managed_python = resolve_store_python(repo)
if managed_python is not None:
# A packaged caller may hand us the old venv launcher; select bytes
# from the install record rather than interpreting relocated pyvenv.cfg.
dependencies = dependency_site(selected_venv(repo))

return str(managed_python), {"PYTHONPATH": os.pathsep.join([str(repo), str(dependencies)])}
#
# ``committed_venv``, not ``selected_venv``: with no generation recorded the
# latter falls back to ``base_venv`` and answers the leftover pre-PM
# ``<root>/venv``, which is built for whichever interpreter created it. Overlaying
# that on the managed store Python loads a cp311 ``pydantic_core`` on 3.14 and
# every script dies with ``No module named 'pydantic_core._pydantic_core'`` — the
# cron sibling of the gateway crash in #122183/#123650.
#
# Both "no generation" and an unreadable record (a corrupt facts.json makes
# ``committed_venv`` RAISE, pm/environments.py:179-193) mean this install has no
# tree of its own to offer. Falling through to the uv overlay below is the
# answer: the handed venv keeps its own interpreter and its own site-packages.
# Returning the bare store Python here would be WORSE than main — a store Python
# with nothing committed is refused by ``_require_own_dependencies`` with
# "hermes pm repair", and a cron script has no hermes_bootstrap to refuse it
# cleanly, so it runs and dies on its first third-party import. It also hands
# ``_windows_cron_bootstrap_argv`` an overlay with no site-packages entry, which
# logs a WARNING on every spawn (#123668 review).
#
# The catch is the repo's idiom for a PM read (environments_adopt.py:30,
# install.py:524), not bare ``Exception``: a malformed record degrades a job, but
# ``payload_venv``'s "payload environment escapes its root" is a fail-closed
# security check (pm/environments.py:92) and must not be downgraded to a warning.
# That one propagates, exactly as it does on the POSIX path today.
#
# The fall-through is safe because the interpreter handed to a cron script is
# ``sys.executable``, and on a managed install with nothing committed a booted
# process is by definition a venv interpreter (``_require_own_dependencies``,
# pm/environments.py:285-287) carrying its own packages. A stdlib venv here has
# no ``uv`` key, skips the overlay below, and keeps its own ``python.exe``.
try:
environment = committed_venv(repo)
except (OSError, RuntimeError, ValueError) as exc:
if "escapes its root" in str(exc):
raise
logger.warning(
"Windows cron script: could not read the committed dependency environment "
"for %s; falling back to the handed venv", repo, exc_info=True)
environment = None
if environment is not None:
return str(managed_python), {"PYTHONPATH": os.pathsep.join(
[str(repo), str(dependency_site(environment))])}

cfg = _read_windows_pyvenv_cfg(venv_dir)
home = cfg.get("home", "")
Expand Down
157 changes: 157 additions & 0 deletions tests/cron/test_cron_windows_venv_abi.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
"""Windows cron scripts resolve their dependency tree, and fall through when there is none.

``cron/scheduler_script.py::_windows_cron_python_invocation`` answers "which interpreter and
which site-packages?" for a cron script child. On a PM-managed install it read the dependency
tree through ``pm.environments.selected_venv``, which falls back to ``base_venv`` and answers
the leftover pre-PM ``<root>/venv`` when no generation is recorded. That tree is built for
whichever interpreter created it, so overlaying it on the managed store Python loads a cp311
``pydantic_core`` on 3.14 and every script dies with
``ModuleNotFoundError: No module named 'pydantic_core._pydantic_core'`` — the cron sibling of
the gateway crash in #122183/#123650, which no PR covered.

Two invariants:

1. With a generation committed, that generation is the tree the child gets — never the in-tree
venv.
2. With NO generation, or a record too corrupt to read, the managed branch falls through to the
uv overlay: the handed venv keeps its own interpreter and its own site-packages. Handing back
the bare store Python instead would be worse than main (``_require_own_dependencies`` refuses
it with "hermes pm repair" and a cron script has no bootstrap to refuse cleanly), and it
strips the overlay's site-packages entry, warning on every spawn.
"""

import os
import sys
from pathlib import Path

import pytest

pytestmark = pytest.mark.platforms("windows")


def _write_venv(venv: Path, base: Path, *, version: str | None = None) -> Path:
"""A fake Windows venv with the ``Scripts/python.exe`` shim a cron child is handed."""
(venv / "Lib" / "site-packages").mkdir(parents=True, exist_ok=True)
(venv / "Scripts").mkdir(parents=True, exist_ok=True)
base.mkdir(parents=True, exist_ok=True)
(venv / "Scripts" / "python.exe").write_text("", encoding="utf-8")
(base / "python.exe").write_text("", encoding="utf-8")
lines = [f"home = {base}", "uv = 0.11.14"]
if version:
lines.append(f"version_info = {version}")
(venv / "pyvenv.cfg").write_text("\n".join(lines) + "\n", encoding="utf-8")
return venv


def _site_packages_in(env_overlay: dict) -> Path | None:
for entry in env_overlay.get("PYTHONPATH", "").split(os.pathsep):
if Path(entry).name == "site-packages":
return Path(entry)
return None


def _raise_runtime_error(_root):
raise RuntimeError("cannot read dependency environment: facts.json")


def test_managed_install_uses_the_committed_generation_not_the_in_tree_venv(
tmp_path, monkeypatch,
):
"""The committed generation wins; the leftover pre-PM venv never reaches the child."""
from cron import scheduler_script as sched_script

stale = _write_venv(tmp_path / "repo" / "venv", tmp_path / "base")
committed = _write_venv(
tmp_path / "installs" / "gen" / "venv", tmp_path / "genbase",
version=f"{sys.version_info[0]}.{sys.version_info[1]}.7",
)
store = tmp_path / "store" / "python.exe"
store.parent.mkdir(parents=True)
store.write_text("", encoding="utf-8")
child = _write_venv(tmp_path / "child", tmp_path / "childbase")

monkeypatch.setattr("hermes_cli._launchers.resolve_store_python", lambda _root: store)
monkeypatch.setattr("pm.environments.committed_venv", lambda _root: committed)
monkeypatch.setattr("pm.environments.selected_venv", lambda _root: stale)

interpreter, env_overlay = sched_script._windows_cron_python_invocation(
str(child / "Scripts" / "python.exe")
)

assert interpreter == str(store)
overlay = _site_packages_in(env_overlay)
assert overlay is not None, "the committed generation must supply a site-packages entry"
assert overlay == committed / "Lib" / "site-packages"
assert not overlay.is_relative_to(stale)


def test_managed_install_without_a_generation_falls_through_to_the_handed_venv(
tmp_path, monkeypatch,
):
"""No generation, or an unreadable record: the handed venv keeps its own packages.

Both arms must land identically — the child gets its own base interpreter and its own
site-packages, not the bare store Python, which cannot serve a script with nothing
committed. Positive control for the fall-through: the handed venv here IS a uv venv
carrying its own site-packages, exactly the case that worked on main.
"""
from cron import scheduler_script as sched_script

stale = _write_venv(tmp_path / "repo" / "venv", tmp_path / "base")
store = tmp_path / "store" / "python.exe"
store.parent.mkdir(parents=True)
store.write_text("", encoding="utf-8")

for label, committed in (("none", lambda _root: None),
("unreadable", _raise_runtime_error)):
case = tmp_path / label
child = _write_venv(case / "child", case / "childbase")
monkeypatch.setattr("hermes_cli._launchers.resolve_store_python",
lambda _root, s=store: s)
monkeypatch.setattr("pm.environments.committed_venv", committed)
monkeypatch.setattr("pm.environments.selected_venv", lambda _root, t=stale: t)

interpreter, env_overlay = sched_script._windows_cron_python_invocation(
str(child / "Scripts" / "python.exe")
)

assert interpreter == str(case / "childbase" / "python.exe"), label
assert env_overlay["VIRTUAL_ENV"] == str(child), label
overlay = _site_packages_in(env_overlay)
assert overlay is not None, f"{label}: the overlay must carry site-packages"
assert overlay == child / "Lib" / "site-packages", label
assert not overlay.is_relative_to(stale), label
assert interpreter != str(store), label


def test_managed_fall_through_keeps_a_stdlib_venv_that_has_no_uv_key(tmp_path, monkeypatch):
"""The fall-through's other arm: a venv with no ``uv`` key keeps its own python.exe.

``_windows_cron_python_invocation`` only overlays when the cfg says ``uv``; a stdlib venv
has no such key, so the overlay is skipped and the handed interpreter is returned
untouched. That is correct — a stdlib venv's ``python.exe`` resolves its own
site-packages with no help — but it is the arm the uv fixture never exercised, and it
is the invariant that keeps the fall-through from returning the bare store Python.
"""
from cron import scheduler_script as sched_script

store = tmp_path / "store" / "python.exe"
store.parent.mkdir(parents=True)
store.write_text("", encoding="utf-8")
child = tmp_path / "child"
(child / "Lib" / "site-packages").mkdir(parents=True)
(child / "Scripts").mkdir(parents=True)
(child / "Scripts" / "python.exe").write_text("", encoding="utf-8")
# NOTE: no "uv" key — this is what makes it the stdlib arm.
(child / "pyvenv.cfg").write_text("home = C:/python\n", encoding="utf-8")

monkeypatch.setattr("hermes_cli._launchers.resolve_store_python", lambda _root: store)
monkeypatch.setattr("pm.environments.committed_venv", lambda _root: None)

interpreter, env_overlay = sched_script._windows_cron_python_invocation(
str(child / "Scripts" / "python.exe")
)

assert interpreter == str(child / "Scripts" / "python.exe")
assert env_overlay == {}
assert interpreter != str(store)
45 changes: 45 additions & 0 deletions tests/tools/test_local_env_blocklist.py
Original file line number Diff line number Diff line change
Expand Up @@ -928,6 +928,51 @@ def test_validated_runtime_venv_lexical_after_repo_recovery(self, tmp_path, monk
local_pythonpath._strip_hermes_owned_pythonpath(env)
assert env["PYTHONPATH"].split(os.pathsep) == ["/home/user/my-lib"]

def test_unreadable_pm_record_degrades_instead_of_raising(self, tmp_path, monkeypatch):
"""A malformed facts.json must not crash child-env finalization.

``_validated_runtime_venv`` runs inside ``_finalize_child_env`` — the guard path
every spawn surface shares — so an unguarded ``selected_venv`` turns an unreadable
PM record into a spawn-time traceback instead of a narrower strip list. The
``runtime_facts_path().is_file()`` gate suppresses the FALLBACK, never the RAISE
(pm/environments.py:179-193). Same defect and same fix as the cron sibling in
#122183.
"""
from tools.environments import local_pythonpath

def _boom(_root):
raise RuntimeError("cannot read dependency environment: facts.json")

monkeypatch.setattr(
"pm.environments.runtime_facts_path", lambda _root: tmp_path / "facts.json"
)
(tmp_path / "facts.json").write_text("{ not json", encoding="utf-8")
monkeypatch.setattr("pm.environments.selected_venv", _boom)

assert local_pythonpath._validated_runtime_venv({}) is None

def test_payload_escape_is_not_swallowed_by_the_degrade(self, tmp_path, monkeypatch):
"""The degrade path must not absorb a fail-closed security check.

``payload_venv`` raises "payload environment escapes its root" when a sealed
payload's manifest points outside its tree. Returning None here would silently
accept a tampered manifest, so the raise propagates — the narrow catch covers a
malformed RECORD, not a refusal by the resolver.
"""
from tools.environments import local_pythonpath

def _escape(_root):
raise RuntimeError("payload environment escapes its root")

monkeypatch.setattr(
"pm.environments.runtime_facts_path", lambda _root: tmp_path / "facts.json"
)
(tmp_path / "facts.json").write_text("{}", encoding="utf-8")
monkeypatch.setattr("pm.environments.selected_venv", _escape)

with pytest.raises(RuntimeError, match="escapes its root"):
local_pythonpath._validated_runtime_venv({})




Expand Down
15 changes: 14 additions & 1 deletion tools/environments/local_pythonpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,20 @@ def _validated_runtime_venv(env: dict) -> Path | None:

root = Path(__file__).resolve().parents[2]
if runtime_facts_path(root).is_file():
return selected_venv(root)
# ``selected_venv`` RAISES on an unreadable/out-of-tree record (pm/environments.py:
# 179-193) — the is_file() gate above suppresses the fallback, never the raise. This
# runs inside child-env finalization for EVERY spawn surface, so an unguarded read
# turns a malformed facts.json into a spawn-time crash. Degrade to no runtime venv
# (the entries below still come from ``sys.prefix``) instead; same shape as the
# cron sibling in #122183, which wraps the same resolver for the same reason.
try:
return selected_venv(root)
except (OSError, RuntimeError, ValueError) as exc:
if "escapes its root" in str(exc):
raise
logger.debug("PM dependency record unreadable; no runtime venv to strip",
exc_info=True)
return None
candidate = Path(env.get("VIRTUAL_ENV") or "")
if not env.get("VIRTUAL_ENV") or not any(
_same_path(candidate, root / "venv") for root in _state()._hermes_repo_root_aliases):
Expand Down