Skip to content

fix: block persistent gateway self-restart jobs - #63272

Closed
John-Lussier wants to merge 3 commits into
NousResearch:mainfrom
John-Lussier:fix/gateway-indirect-lifecycle-guard
Closed

fix: block persistent gateway self-restart jobs#63272
John-Lussier wants to merge 3 commits into
NousResearch:mainfrom
John-Lussier:fix/gateway-indirect-lifecycle-guard

Conversation

@John-Lussier

Copy link
Copy Markdown
Contributor

Summary

A real macOS gateway session scheduled a delayed shell helper through launchd's submitted-job primitive. launchd registered it as persistent (OnDemand=false), so the helper slept 45 seconds, restarted the gateway, exited successfully, and was relaunched. It produced 762 restart cycles before removal.

This patch closes the indirect execution path in the existing gateway lifecycle guard:

  • rejects persistent submitted launchd jobs from gateway terminal sessions
  • scans directly executed, shell-wrapped, -c, relative-cwd, executable-shebang, and nested shell scripts
  • detects hidden gateway lifecycle and persistent launchd submission operations inside referenced scripts
  • resolves scripts against the same live session cwd used for execution
  • uses bounded, nonblocking, regular-file-only reads with recursion limits
  • preserves safe shell scripts and quoted explanatory text
  • applies the same persistent-job rejection to cron script creation

Verification

  • 78 passedtests/hermes_cli/test_gateway_restart_loop.py
  • 679 passedtests/cron
  • 156 passed — focused terminal-tool suites
  • git diff --check

An independent adversarial review found hidden-script, live-cwd, parser, and unbounded-read gaps in the first realization; the second commit adds regression coverage and closes each one.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management tool/terminal Terminal execution and process management P2 Medium — degraded but workaround exists sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation labels Jul 12, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for pursuing a real gateway-loop gap. The premise is confirmed on current main: the gateway terminal guard only calls _contains_gateway_lifecycle_command at tools/terminal_tool.py:2275-2277, while the matcher at cron/lifecycle_guard.py:48-66 does not cover launchctl submit or referenced scripts.

Problems

  • In PR commit 1824941eff36044c28f32805276e644680a3a568, the new guard_cwd call does not pass prev_owner=prev_cwd_owner. Current main's resolver rejects stale shared env.cwd only when that ownership value is supplied (tools/terminal_tool.py:2001-2023); execution does supply it at tools/terminal_tool.py:2651-2655. The guard can scan a prior session's directory while the command executes in another one.
  • The new scanner uses host-side file reads, but SSH/Modal/Daytona are remote backends (tools/environments/base.py:1034-1041). Missing host paths are allowed, so referenced scripts on those backends are not inspected.
  • The cron creation path added at cron/lifecycle_guard.py:336-339 still uses only direct matchers, not the new recursive scanner; a cron wrapper script can still invoke a child containing the prohibited operation.

Suggested changes

  • Preserve the execution CWD ownership semantics in the guard and add a two-session regression.
  • Make script inspection backend-aware, with remote-backend coverage.
  • Reuse the recursive scanner for resolved cron scripts and test a nested wrapper.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
- Resolve guard cwd against get_session_cwd(session_key); fall back to env.cwd
  when no session record exists yet, matching current main's per-session cwd
  architecture.
- Make referenced-script reads backend-aware: local read first; if missing,
  fall back to env.execute('cat ...') for SSH/Modal/Daytona backends.
- Reuse the recursive scanner in check_gateway_lifecycle so nested cron
  wrapper scripts are caught, and resolve relative refs inside a script
  against that script's directory.
- Add regression tests for remote-backend reads, two-session cwd, and nested
  cron wrappers.

Verification: 80 passed tests/hermes_cli/test_gateway_restart_loop.py;
694 passed tests/cron; ruff + git diff --check clean.
@John-Lussier
John-Lussier force-pushed the fix/gateway-indirect-lifecycle-guard branch from 1824941 to 1c8721d Compare July 17, 2026 01:11
@John-Lussier

Copy link
Copy Markdown
Contributor Author

Rebased onto current origin/main and resolved the three concrete review items:

  1. Live session cwd: the guard now resolves against get_session_cwd(session_key) first; when no session record exists yet, it falls back to env.cwd. This matches the current per-session cwd architecture on main and closes the stale-shared-cwd bypass.

  2. Remote-backend script reads: referenced scripts are read locally first; if the local path is missing, the guard falls back to env.execute("cat ...") so SSH/Modal/Daytona backends are covered without penalizing local fast-path tests.

  3. Recursive cron scanner: check_gateway_lifecycle() now uses the recursive _contains_unsafe_gateway_action() scanner, so nested cron wrapper scripts (e.g. outer.shinner.sh) are caught. Relative references inside a script resolve against that script's directory.

Verification:

  • tests/hermes_cli/test_gateway_restart_loop.py80 passed (added regressions for remote reads, two-session cwd, nested cron wrappers).
  • tests/cron694 passed.
  • ruff check and git diff --check clean.

Fresh head: 1c8721d90.

teknium1 added a commit that referenced this pull request Aug 1, 2026
…bel-independent detection

Extends the shared _GATEWAY_LIFECYCLE_PATTERN (used by BOTH the cron
creation-time guard in cron/lifecycle_guard.py and the terminal
execution-time hard-block in tools/terminal_tool.py) so Branch B covers
launchctl submit and bootstrap alongside kickstart/unload/load/stop/
restart, and normalizes POSIX shell line continuations before matching
so the exact multi-line reported shape in #62891 cannot slip past.

Also extends the execution-aware, label-independent detector
(contains_launchctl_submit_command, cherry-picked from #63272) to cover
launchctl bootstrap, since a neutral label like ai.hermes.svc-reload-tmp
defeats any label-anchored regex — the second production reproduction.

Regression tests cover both sites, including
`launchctl submit -l com.foo -- /path/gateway` and the bootstrap
variant, plus outside-gateway pass-through.

Branch B regex extension and continuation normalization drawn from
PR #62896; bootstrap coverage and test shapes drawn from PR #51003.

Co-authored-by: JackJin <1037461232@qq.com>
Co-authored-by: joelbrilliant <joelbrilliant1@gmail.com>
@teknium1

teknium1 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Merged via PR #75972 (#75972) — your three commits (label-independent submit blocker + bounded recursive referenced-script scanner) were cherry-picked verbatim with your authorship preserved, extended to cover the bootstrap variant. Yours was the cleanest of the three candidate fixes. Fixes #62891. Thanks!

@teknium1 teknium1 closed this Aug 1, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…bel-independent detection

Extends the shared _GATEWAY_LIFECYCLE_PATTERN (used by BOTH the cron
creation-time guard in cron/lifecycle_guard.py and the terminal
execution-time hard-block in tools/terminal_tool.py) so Branch B covers
launchctl submit and bootstrap alongside kickstart/unload/load/stop/
restart, and normalizes POSIX shell line continuations before matching
so the exact multi-line reported shape in NousResearch#62891 cannot slip past.

Also extends the execution-aware, label-independent detector
(contains_launchctl_submit_command, cherry-picked from NousResearch#63272) to cover
launchctl bootstrap, since a neutral label like ai.hermes.svc-reload-tmp
defeats any label-anchored regex — the second production reproduction.

Regression tests cover both sites, including
`launchctl submit -l com.foo -- /path/gateway` and the bootstrap
variant, plus outside-gateway pass-through.

Branch B regex extension and continuation normalization drawn from
PR NousResearch#62896; bootstrap coverage and test shapes drawn from PR NousResearch#51003.

Co-authored-by: JackJin <1037461232@qq.com>
Co-authored-by: joelbrilliant <joelbrilliant1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants