Skip to content

test(safety): make updater restart tests hermetic across platforms - #68988

Closed
hehehe0803 wants to merge 1 commit into
NousResearch:mainfrom
hehehe0803:test-safety/updater-restart-hermetic
Closed

hehehe0803 wants to merge 1 commit into
NousResearch:mainfrom
hehehe0803:test-safety/updater-restart-hermetic

Conversation

@hehehe0803

Copy link
Copy Markdown
Contributor

What

Make the updater restart tests hermetic. This extends the autouse live-system guard in tests/conftest.py so that while cmd_update / _cmd_update_impl is on the call stack, gateway/service/platform discovery, the restart/terminate boundaries, and the systemd/launchd unit-write boundary all return inert values. It also hardens the guard's command classifiers. Test-only — no production code changes.

Why

cmd_update discovers and restarts running gateways after its git and dependency work. The guard already blocked os.kill and systemctl mutations, but left discovery live. On a developer machine with a running gateway, an updater test whose mocks miss discovery can find that real gateway, classify it, and schedule a detached restart watcher that rewrites the developer's real user systemd unit to the test checkout's interpreter. Prior PRs closed narrower slices of this class: #23397 (block direct signals/systemd mutations), #22900 (stop run_gateway() tests rewriting the installed unit), #44267 (unit-generation test safety belt).

How it works — strictly additive, low overhead

  • Updater-frame detection is by name only (cmd_update/_cmd_update_impl in hermes_cli.main) — the fixture does not import hermes_cli.main, so non-updater test processes are unaffected (verified: a plain test process does not import hermes_cli.main).
  • The command classifiers were hardened and made robust: a read-only allowlist for a protected Hermes service target (unknown/mutating verbs fail closed); recursion through shell wrappers (sh/bash -c/-lc/-ic, cmd /c|/k, powershell -Command/-EncodedCommand); blocking of foreign, negative, and process-group (kill -9 -1) kill targets; and blocking of detached Hermes gateway spawns and launchctl kickstart/bootout / taskkill of a Hermes gateway.
  • The guard never blocks a command the prior guard allowed: read-only systemctl/launchctl probes, benign own-subtree kills, unrelated pkill, benign taskkill /IM, and all git commands pass through unchanged.

How to test

From a clean checkout (Linux, Python 3.11):

python -m pytest tests/test_live_system_guard_self_test.py tests/hermes_cli/test_update_autostash.py tests/test_live_system_guard_classifiers.py tests/test_live_system_guard_update_hermetic.py

Results on this branch: test_live_system_guard_self_test.py and test_update_autostash.py are unchanged from the pristine baseline (proving the change is strictly additive), and the new modules pass. The canary is load-bearing: disabling only the updater terminate_pid inert wrapper makes two tests fail. scripts/run_tests.sh (per-file isolation, matching CI) is the authoritative full-suite gate.

New synthetic tests (never discover, signal, spawn, or rewrite a real process/unit):

  • tests/test_live_system_guard_classifiers.py — pure classifier tests incl. shell-wrapper, negative-PID, and verb-allowlist edge cases.
  • wrapper-level wiring tests in tests/test_live_system_guard_self_test.py — assert launchctl/detached-spawn/taskkill are rejected before native execution, with pass-through cases.
  • tests/test_live_system_guard_update_hermetic.py — asserts no discovery/restart/unit-write boundary executes during cmd_update, plus the load-bearing terminate canary.

Platforms tested

Linux (Python 3.11). The launchd (macOS) and Windows taskkill/cmd/powershell classifiers are exercised as pure string tests; no native launchd/taskkill command is invoked by any test.

cmd_update discovers and restarts running gateways after its git and
dependency work. The autouse live-system guard blocked os.kill and
systemctl mutations but left gateway/service discovery live: on a
developer machine with a running gateway, an updater test whose mocks
miss discovery could find it and schedule a detached restart watcher
that rewrites the real user systemd unit. Prior PRs NousResearch#23397, NousResearch#22900,
NousResearch#44267 closed narrower slices of this class.

While cmd_update/_cmd_update_impl is on the stack, gateway/service/
platform discovery and the restart, terminate, and unit-write
boundaries now return inert values. The command classifiers are
hardened: a read-only allowlist for protected service targets;
recursion through shell wrappers (sh/bash -c/-lc, cmd /c, powershell
-Command/-EncodedCommand); blocking of foreign and negative/process-
group kill targets; and blocking of detached gateway spawns and
launchctl/taskkill of a Hermes gateway. The guard stays strictly
additive: read-only probes, benign kills, and git pass through.
Updater-frame detection is by name only (no hermes_cli.main import), so
non-updater test processes are unaffected.

Test-only; no product code. Adds synthetic classifier, wrapper-level
wiring, and load-bearing canary tests that never discover, signal,
spawn, or rewrite a real process or unit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015cwEx8CjYtTEvrnmCfp6Hw
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @hehehe0803 — closing after review: main's updater-restart guard already blocks all damage primitives this targets (kill/systemctl/hermes update spawns including shell wrappers), and explicitly documents the 'discovery without delivery is harmless' contract. The 426-line stack-frame-sniffing rewrite adds fragility without new protection, and is stale against the pruned suite. Appreciate the cross-platform testing intent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants