fix(terminal): strict Linux-only gating for background-executor systemd scopes (#70716 follow-up)
Cross-platform hardening of @toprakeker's systemd cgroup isolation (PR #71378, landed via #81264): - Gate every scope-path branch on a new _IS_LINUX constant instead of 'not _IS_WINDOWS', so macOS (and any other POSIX platform) provably never touches systemd code — no probe subprocess, no scope argv, byte-identical legacy spawn. - Unit tests: darwin no-op guarantee (no probe exec, no scope argv build, legacy argv byte-identical, no unit recorded) and probe-returns-False off Linux. - New live Windows E2E (tests/tools/test_process_registry_windows_live.py, wired into the on-demand windows-venv-e2e lane): real spawn_local on windows-latest asserting jobs run exactly as before — spawned, output captured, exit code correct, systemd path never reached even under faked gateway identity. Refs #70716, #71378.
This commit is contained in:
@@ -70,3 +70,11 @@ jobs:
|
||||
uv run --no-sync python -m pytest \
|
||||
tests/gateway/test_telegram_closewait_windows_live.py \
|
||||
-o addopts= -v -p no:cacheprovider
|
||||
|
||||
- name: Run background-executor spawn parity live E2E (#70716)
|
||||
shell: bash
|
||||
run: |
|
||||
set -uo pipefail
|
||||
uv run --no-sync python -m pytest \
|
||||
tests/tools/test_process_registry_windows_live.py \
|
||||
-o addopts= -v -p no:cacheprovider
|
||||
|
||||
@@ -2285,6 +2285,77 @@ class TestSystemdCgroupIsolation:
|
||||
|
||||
assert pr._stop_systemd_unit("hermes-worker-gone.scope") is True
|
||||
|
||||
def test_darwin_never_takes_scope_path_even_with_systemd_run_on_path(
|
||||
self, registry, monkeypatch, _gateway_identity
|
||||
):
|
||||
"""macOS no-op guarantee (#70716 cross-platform audit).
|
||||
|
||||
With ``_IS_LINUX = False`` (darwin), the spawn path must be
|
||||
byte-identical to the legacy path even when a ``systemd-run``
|
||||
binary is somehow on PATH and the gateway identity checks pass:
|
||||
no probe, no wrapping, no unit recorded.
|
||||
"""
|
||||
import tools.process_registry as pr
|
||||
|
||||
fake_popen, captured = self._fake_popen_capture()
|
||||
|
||||
monkeypatch.setattr(pr, "_IS_LINUX", False)
|
||||
monkeypatch.setattr(pr, "_IS_WINDOWS", False)
|
||||
monkeypatch.setattr(pr, "_SYSTEMD_SCOPE_AVAILABLE", None)
|
||||
monkeypatch.setattr("tools.process_registry._find_shell", lambda: "/bin/bash")
|
||||
monkeypatch.setattr(
|
||||
"gateway.restart.is_gateway_supervisor_process", lambda: True
|
||||
)
|
||||
# If any branch consults the probe or builds a scope argv on darwin,
|
||||
# fail loudly.
|
||||
monkeypatch.setattr("shutil.which", lambda name: "/usr/local/bin/systemd-run")
|
||||
scope_builds = []
|
||||
real_build = pr._build_systemd_scope_argv
|
||||
monkeypatch.setattr(
|
||||
pr,
|
||||
"_build_systemd_scope_argv",
|
||||
lambda *a, **k: scope_builds.append(a) or real_build(*a, **k),
|
||||
)
|
||||
probe_runs = []
|
||||
|
||||
def fake_probe_run(argv, **kwargs):
|
||||
probe_runs.append(argv)
|
||||
return subprocess.CompletedProcess(args=argv, returncode=0)
|
||||
|
||||
monkeypatch.setattr("subprocess.run", fake_probe_run)
|
||||
|
||||
with (
|
||||
patch("subprocess.Popen", side_effect=fake_popen),
|
||||
patch("threading.Thread", return_value=MagicMock()),
|
||||
patch.object(registry, "_write_checkpoint"),
|
||||
):
|
||||
session = registry.spawn_local("echo hello", cwd="/tmp")
|
||||
|
||||
argv = captured["argv"]
|
||||
assert argv == ["/bin/bash", "-lic", "set +m; echo hello"], argv
|
||||
assert captured["start_new_session"] is True
|
||||
assert session.systemd_unit == ""
|
||||
assert scope_builds == [], "darwin must never build a systemd scope argv"
|
||||
assert probe_runs == [], "darwin must never run the systemd-run probe"
|
||||
|
||||
def test_probe_returns_false_off_linux(self, monkeypatch):
|
||||
"""``_systemd_run_user_scope_available`` is False on non-Linux even
|
||||
when a ``systemd-run`` binary exists on PATH."""
|
||||
import tools.process_registry as pr
|
||||
|
||||
monkeypatch.setattr(pr, "_IS_LINUX", False)
|
||||
monkeypatch.setattr(pr, "_SYSTEMD_SCOPE_AVAILABLE", None)
|
||||
monkeypatch.setattr("shutil.which", lambda name: "/usr/local/bin/systemd-run")
|
||||
probe_runs = []
|
||||
monkeypatch.setattr(
|
||||
"subprocess.run",
|
||||
lambda argv, **kwargs: probe_runs.append(argv)
|
||||
or subprocess.CompletedProcess(args=argv, returncode=0),
|
||||
)
|
||||
|
||||
assert pr._systemd_run_user_scope_available() is False
|
||||
assert probe_runs == [], "non-Linux must not exec the probe"
|
||||
|
||||
|
||||
class TestNotificationRedaction:
|
||||
"""Background-process notification delivery (completion_queue) applies the
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
"""LIVE Windows E2E for background-executor spawn parity (#70716 / PR salvage).
|
||||
|
||||
Runs ONLY on a real Windows host (the on-demand ``windows-venv-e2e.yml``
|
||||
lane). The systemd cgroup-isolation feature for local background executors
|
||||
must be a strict no-op on Windows: jobs spawn exactly as before, output is
|
||||
captured, exit codes are correct, and no systemd code path is ever reached
|
||||
— even when the process claims gateway identity.
|
||||
|
||||
These tests drive the REAL ``ProcessRegistry.spawn_local`` pipe path on the
|
||||
live Windows process table (real Popen, real Git Bash shell, real reader
|
||||
thread) — no mocked spawn.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
pytestmark = pytest.mark.skipif(
|
||||
sys.platform != "win32", reason="live Windows background-executor E2E"
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def registry(tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes-home"))
|
||||
import tools.process_registry as pr
|
||||
|
||||
reg = pr.ProcessRegistry()
|
||||
yield reg
|
||||
for sid in list(reg._running):
|
||||
try:
|
||||
reg.kill_process(sid)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
|
||||
def _wait_exit(reg, sid, timeout=60):
|
||||
deadline = time.time() + timeout
|
||||
while time.time() < deadline:
|
||||
sess = reg._finished.get(sid) or reg._running.get(sid)
|
||||
if sess is not None and sess.exited:
|
||||
return sess
|
||||
time.sleep(0.2)
|
||||
raise AssertionError(f"session {sid} did not exit within {timeout}s")
|
||||
|
||||
|
||||
class TestWindowsSpawnParity:
|
||||
def test_background_job_runs_output_and_exit_code_unchanged(self, registry):
|
||||
"""Plain background job: spawned, output captured, exit code correct."""
|
||||
session = registry.spawn_local("echo win-live-parity; exit 7")
|
||||
done = _wait_exit(registry, session.id)
|
||||
|
||||
assert done.exit_code == 7
|
||||
assert "win-live-parity" in done.output_buffer
|
||||
# The systemd scope identity must never be recorded on Windows.
|
||||
assert done.systemd_unit == ""
|
||||
|
||||
def test_gateway_identity_never_reaches_systemd_path_on_windows(
|
||||
self, registry, monkeypatch
|
||||
):
|
||||
"""Even with full (faked) gateway identity, the Windows spawn takes
|
||||
the legacy path: no scope argv is built, no probe runs, and the job
|
||||
behaves exactly as without the identity."""
|
||||
import tools.process_registry as pr
|
||||
|
||||
monkeypatch.setenv("_HERMES_GATEWAY", "1")
|
||||
monkeypatch.setattr(
|
||||
"gateway.status.get_running_pid",
|
||||
lambda *, cleanup_stale=False: os.getpid(),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"gateway.restart.is_gateway_supervisor_process", lambda: True
|
||||
)
|
||||
monkeypatch.setattr(pr, "_SYSTEMD_SCOPE_AVAILABLE", None)
|
||||
|
||||
scope_builds = []
|
||||
monkeypatch.setattr(
|
||||
pr,
|
||||
"_build_systemd_scope_argv",
|
||||
lambda *a, **k: scope_builds.append(a) or a[0],
|
||||
)
|
||||
|
||||
session = registry.spawn_local("echo win-live-gateway; exit 3")
|
||||
done = _wait_exit(registry, session.id)
|
||||
|
||||
assert done.exit_code == 3
|
||||
assert "win-live-gateway" in done.output_buffer
|
||||
assert done.systemd_unit == ""
|
||||
assert scope_builds == [], "Windows must never build a systemd scope argv"
|
||||
# The availability probe must not have flipped to True on Windows.
|
||||
assert pr._SYSTEMD_SCOPE_AVAILABLE is not True
|
||||
|
||||
def test_kill_process_windows_plain_path(self, registry):
|
||||
"""kill_process on Windows works without any systemd unit cleanup."""
|
||||
session = registry.spawn_local("sleep 60")
|
||||
time.sleep(1.0)
|
||||
result = registry.kill_process(session.id)
|
||||
assert result.get("status") in {"killed", "already_exited"}
|
||||
assert session.systemd_unit == ""
|
||||
@@ -43,6 +43,10 @@ import uuid
|
||||
from pathlib import Path
|
||||
|
||||
_IS_WINDOWS = platform.system() == "Windows"
|
||||
# systemd transient scopes exist only on Linux. Gate every scope-path branch
|
||||
# on this constant (not merely "not Windows") so macOS and other POSIX
|
||||
# platforms provably never touch systemd code (#70716 cross-platform audit).
|
||||
_IS_LINUX = platform.system() == "Linux"
|
||||
from tools.environments.local import _find_shell, _resolve_safe_cwd, _sanitize_subprocess_env
|
||||
from hermes_cli._subprocess_compat import windows_hide_flags
|
||||
from dataclasses import dataclass, field
|
||||
@@ -215,7 +219,7 @@ def _systemd_run_user_scope_available() -> bool:
|
||||
return False
|
||||
|
||||
available = False
|
||||
if not _IS_WINDOWS:
|
||||
if _IS_LINUX:
|
||||
try:
|
||||
import shutil
|
||||
|
||||
@@ -1098,7 +1102,7 @@ class ProcessRegistry:
|
||||
# Wrap the PTY command in a systemd scope so interactive
|
||||
# executors get their own cgroup, same as pipe mode.
|
||||
pty_in_supervised_gateway = (
|
||||
not _IS_WINDOWS and _is_supervised_gateway_process()
|
||||
_IS_LINUX and _is_supervised_gateway_process()
|
||||
)
|
||||
pty_use_systemd_scope = (
|
||||
pty_in_supervised_gateway and _systemd_run_user_scope_available()
|
||||
@@ -1176,7 +1180,7 @@ class ProcessRegistry:
|
||||
# cgroup (and the messaging control plane with it). This applies to
|
||||
# both pipe mode and the PTY path above.
|
||||
shell_argv = [user_shell, "-lic", f"set +m; {safe_command}"]
|
||||
in_supervised_gateway = not _IS_WINDOWS and _is_supervised_gateway_process()
|
||||
in_supervised_gateway = _IS_LINUX and _is_supervised_gateway_process()
|
||||
use_systemd_scope = (
|
||||
in_supervised_gateway and _systemd_run_user_scope_available()
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user