Repository navigation
fix(lifecycle_guard): allow bootstrap of an EXISTING non-gateway plist; surface guard blocks in execute_code - #767
Merged
Conversation
…t; surface guard blocks in execute_code Two defects, both reproduced on live code before any edit. 1. `launchctl bootstrap <existing plist>` was refused label-independently. `contains_launchctl_submit_command` treats submit and bootstrap label-independently because a NEW job's label is attacker-chosen (NousResearch#62891). That holds for `submit` (pure text) and for bootstrap of a path that does not exist yet, but not for a plist already on disk: launchd reads `Label` out of that file, so the label is a readable fact, not a claim. Measured inside the supervised gateway on the Studio: reloading the fleetreview router after a `plutil -replace` edit sudo launchctl bootout system/ai.hermes.fleetreview-router sudo launchctl bootstrap system /Library/LaunchDaemons/ai.hermes.fleetreview-router.plist was refused, though `ai.hermes.fleetreview-router` is not a gateway label. The workaround was the deprecated, label-gated `launchctl load -w`. `_bootstrap_targets_readable_non_gateway_plist` now allows bootstrap when every plist argument is an existing readable regular file whose Label is not a gateway label and whose Program/ProgramArguments do not reference a gateway entrypoint. Everything unreadable stays blocked: missing path, directory, FIFO, oversized (256 KiB cap), unparseable, missing Label, any unexpanded shell value in any argument, and `submit` in every shape. 2. execute_code turned a guard block into a silent success. The guard refuses a terminal() call by RETURNING a dict. Inside the sandbox that is a value, so the common script shape `r = terminal(cmd)` ran to completion and execute_code reported status=success, exit_code=0, output='' — a silent block, worse than the direct terminal tool, which at least prints why. Both refusal returns now carry `blocked_by`, and the generated hermes_tools stub raises ToolCallBlocked on that marker. Measured on the same input, before: status=success exit_code=0 output=''. After: status=error exit_code=1 with the refusal in the traceback. Verification (fork/main 9de4895, scripts/run_tests.sh, sandboxed HOME): * tests/cron/test_lifecycle_guard_bootstrap_existing_plist.py 31 passed * tests/tools/test_execute_code_surfaces_blocks.py 11 passed * full sweep 107 files, 2083 passed, 4 skipped Mutation proof, part 1 (9 mutants, per-guard): M1 drop gateway-label check KILLED (2) M2 drop argv-entrypoint check KILLED (1) M3 drop split-argv check KILLED (1) M4 drop missing-Label fail-closed KILLED (1) M5 drop unexpanded-shell check KILLED (1) M8 unreadable treated as safe KILLED (5) M9 any- instead of every-plist KILLED (6) M6 drop size bound SURVIVED - equivalent under overlap M7 drop S_ISREG check SURVIVED - equivalent under overlap M6/M7 are honestly reported, not waved through. The reader carries three overlapping bounds (st_size pre-check, bounded read loop, post-read length check); dropping any one still yields None, and only dropping all three goes red (verified: M6g KILLED). S_ISREG is likewise shadowed because os.read on a directory raises IsADirectoryError and a FIFO read returns empty. Both properties are pinned directly at the reader (test_oversized_file_reader_returns_none, test_fifo_argument_blocked, test_directory_named_like_a_plist_blocked) plus a positive control, so the property is gated even though no single mutation of its three implementations can be killed. M3 and M5 initially SURVIVED - real test gaps, not equivalents. Added test_gateway_entrypoint_split_across_argv_blocked and test_variable_in_domain_argument_blocked (`launchctl bootstrap gui/$UID <plist>`, the common spelling); both mutants then went red. Mutation proof, part 2 (6 mutants, both directions): stub not routed, helper never raises, helper raises on EVERYTHING, marker key renamed, submit-refusal unstamped, lifecycle-refusal unstamped - 6/6 KILLED. The raise-on-everything mutant is the load-bearing control: it reds test_ordinary_nonzero_exit_is_not_a_block, so an `exit 7` or a grep miss still returns normally. Against the real on-disk plists, self-identity ai.hermes.gateway: allowed bootstrap /Library/LaunchDaemons/ai.hermes.fleetreview-router.plist allowed bootout system/ai.hermes.fleetreview-router BLOCKED bootstrap ~/Library/LaunchAgents/ai.hermes.gateway.plist allowed bootstrap ~/Library/LaunchAgents/ai.hermes.gateway-daedalus.plist BLOCKED launchctl submit Residual risk: bootstrap now reads the plist at guard time, so a TOCTOU swap between the guard's read and launchd's read is possible. This requires local write access to the plist path, which already implies the ability to edit the gateway's own plist, so it does not widen the attack surface. Every other input remains fail-closed. Two pre-existing flakes surfaced under 64-way parallelism (test_media_delivery_parity, test_script_claim_heartbeat, test_scheduler_provider). Different tests each run; all pass in isolation on this branch; `git diff --name-only fork/main` is empty for each file, and test_media_delivery_parity flakes identically on a pristine fork/main worktree. Not caused by this change.
…and is blocked Review round 1 finding. The new allow-path asked "is this plist a gateway job?" (Label / entrypoint argv) but never "does loading this plist EXECUTE a gateway-lifecycle command?" — re-opening the NousResearch#62891 laundering shape with a file instead of a submit line, and needing no root: Label=ai.hermes.helper ProgramArguments=["/bin/sh","-c","launchctl kickstart -k system/<self>"] launchctl bootstrap gui/501 $TMPDIR/ai.hermes.helper.plist -> ALLOWED _plist_declares_gateway_job now also runs the flattened Program/ ProgramArguments through the existing lifecycle scanner, per token (sh -c payloads, referenced scripts) and joined (a command split across argv words). The scan is re-entrant (a plist can bootstrap a plist), so it is depth-bounded per thread and fails CLOSED at the bound. Verified: - incident shape (ai.hermes.fleetreview-router, argv /usr/bin/python3 /opt/router.py) stays ALLOWED. - 4 new witnesses: inline sh -c, referenced script, direct argv, mutual plist cycle -> all BLOCKED. - mutation matrix 6/6 killed, each reddening its own named witness: drop-check 4 red; drop-per-token 1; drop-joined 1; bound-disabled 1 (130 plist reads vs 3); depth-not-restored 5; bound-fails-open 1. - 35 tests in the file, 220 lifecycle/guard tests, 42 safe_command tests all pass.
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Sep 20, 2026
Collaborator
Author
|
Ejected from the merge queue 12:12 PT: merge_group run 35529883245 red on slice 3/16 only (runner studio-ang-ventures-hermes-agent-1). Two asyncio TimeoutErrors in tests/gateway/test_session_hygiene.py — outside this PR's diff, green on the PR's own run — while the Studio was under heavy local load (Apollo was running ~150 s real-clock suites there at the same minute). Wall-clock flake on a loaded self-hosted runner; re-enqueueing. Same run is also the first live proof of #766's split: slices 1–10 self-hosted, 11–16 ubuntu-latest, all green. — Apollo |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defects
Two, both reproduced on live code before any edit.
1.
launchctl bootstrap <existing plist>was refused label-independentlycontains_launchctl_submit_commandhandlessubmitandbootstraplabel-independently because a NEW job's label is attacker-chosen (NousResearch#62891). That reasoning is correct forsubmit(pure text) and for bootstrap of a path that does not exist yet — but not for a plist already on disk: launchd reads theLabelkey out of that file, so the label is a readable fact, not a claim.Measured inside the supervised gateway, reloading the fleetreview router after a
plutil -replaceedit:Refused, though
ai.hermes.fleetreview-routeris not a gateway label. The workaround was the deprecated, label-gatedlaunchctl load -w.2.
execute_codeturned a guard block into a silent successThe guard refuses a
terminal()call by returning a dict. Inside the sandbox that is a value, so the common script shaper = terminal(cmd)ran to completion andexecute_codereportedstatus=success, exit_code=0, output=''— a silent block, strictly worse than the direct terminal tool, which at least prints why.The change
_bootstrap_targets_readable_non_gateway_plistallows bootstrap when every plist argument is an existing readable regular file whoseLabelis not a gateway label and whoseProgram/ProgramArgumentsdo not reference a gateway entrypoint. Everything unreadable stays blocked: missing path, directory, FIFO, oversized (256 KiB cap), unparseable, missingLabel, any unexpanded shell value in any argument, andsubmitin every shape.Both refusal returns now carry
blocked_by, and the generatedhermes_toolsstub raisesToolCallBlockedon that marker.Before/after on the same input, part 2:
success''errorVerification
Base
fork/main9de4895,scripts/run_tests.sh, sandboxedHOME.tests/cron/test_lifecycle_guard_bootstrap_existing_plist.py— 31 passedtests/tools/test_execute_code_surfaces_blocks.py— 11 passedtests/cron/, approval, code-execution, terminal, gateway-restart) — 107 files, 2083 passed, 4 skippedMutation proof, part 1 — 9 mutants, one per guard
Labelfail-closedany-instead ofevery-plistS_ISREGcheckM6/M7 are reported honestly, not waved through. The reader carries three overlapping bounds (
st_sizepre-check, bounded read loop, post-read length check); dropping any one still yieldsNone, and only dropping all three goes red (verified:M6gKILLED).S_ISREGis likewise shadowed becauseos.readon a directory raisesIsADirectoryErrorand a FIFO read returns empty. Both properties are therefore pinned directly at the reader —test_oversized_file_reader_returns_none,test_fifo_argument_blocked,test_directory_named_like_a_plist_blocked, plus a positive control — so the property is gated even though no single mutation of its three implementations can be killed.M3 and M5 initially SURVIVED — real test gaps, not equivalents. Added
test_gateway_entrypoint_split_across_argv_blocked(a bare/usr/local/bin/hermes gateway runlauncher matches no marker token) andtest_variable_in_domain_argument_blocked(launchctl bootstrap gui/$UID <plist>, the common spelling, where the variable is in the domain not the path). Both mutants then went red.Mutation proof, part 2 — 6 mutants, both directions, 6/6 KILLED
stub not routed · helper never raises · helper raises on everything · marker key renamed · submit-refusal unstamped · lifecycle-refusal unstamped.
The raise-on-everything mutant is the load-bearing control: it reds
test_ordinary_nonzero_exit_is_not_a_block, proving anexit 7or a grep miss still returns normally rather than exploding.Against the real on-disk plists
Self-identity
ai.hermes.gateway:Residual risk
Bootstrap now reads the plist at guard time, so a TOCTOU swap between the guard's read and launchd's read is possible. It requires local write access to the plist path, which already implies the ability to edit the gateway's own plist — so it does not widen the attack surface. Every other input remains fail-closed.
Pre-existing flakes (not caused by this change)
Under 64-way parallelism,
test_media_delivery_parity,test_script_claim_heartbeatandtest_scheduler_providerflaked — different tests each run. All pass in isolation on this branch;git diff --name-only fork/mainis empty for each file; andtest_media_delivery_parityflakes identically on a pristinefork/mainworktree.What to check
_plist_declares_gateway_jobheuristic — is"gateway" in tokens and any("hermes" in token)the right breadth, or should it be tighter/looser?GATEWAY_LIFECYCLE_BLOCK_MARKERbelongs incron/lifecycle_guard.pyor somewhere more neutral, giventools/now imports it.