Skip to content

fix(terminal): block launchctl submit inside gateway - #75557

Closed
hermes-jeffrolc wants to merge 3 commits into
NousResearch:mainfrom
hermes-jeffrolc:fix/launchctl-submit-lifecycle-guard
Closed

fix(terminal): block launchctl submit inside gateway#75557
hermes-jeffrolc wants to merge 3 commits into
NousResearch:mainfrom
hermes-jeffrolc:fix/launchctl-submit-lifecycle-guard

Conversation

@hermes-jeffrolc

@hermes-jeffrolc hermes-jeffrolc commented Jul 31, 2026

Copy link
Copy Markdown

Summary

  • block command-position launchctl submit invocations from local terminal calls running inside the Hermes gateway
  • normalize common execution wrappers and the valid bash -c -- <payload> form before command detection
  • keep the guard ahead of approval/force=True, so a submitted KeepAlive job cannot repeatedly restart its own gateway host
  • preserve external-shell, non-local backend, neutral launchctl bootstrap/load, and existing lifecycle-command behavior

Why

launchctl submit jobs can outlive the terminal command and be relaunched by launchd. In #62891, a submitted helper repeatedly ran hermes gateway restart, causing 263 gateway restarts before the transient job was removed.

Label-only matching is not enough: a neutral label can execute the same payload. This patch blocks the unsafe submit primitive at the local gateway-hosted terminal boundary while leaving Hermes' supported internal restart path and normal external-shell administration untouched.

This is intentionally a foot-gun guard, not a general shell-security boundary. Indirect execution through arbitrary scripts/interpreters remains outside scope.

Closes #62891.

Test plan

  • HERMES_HOME=<temp> python -m pytest -q tests/hermes_cli/test_gateway_restart_loop.py tests/tools/test_terminal_tool.py tests/tools/test_terminal_tool_requirements.py tests/tools/test_terminal_tool_pty_fallback.py tests/cron — 493 passed
  • python -m ruff check cron/lifecycle_guard.py tools/terminal_tool.py tests/hermes_cli/test_gateway_restart_loop.py
  • git diff --check

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for narrowing this to the local in-gateway terminal boundary. The premise is current: tools/terminal_tool.py:2441-2455 only checks the lifecycle matcher, and cron/lifecycle_guard.py:55-65 does not include launchctl submit; the new guard at tools/terminal_tool.py:2457 covers that direct bypass before approval/force.

Problems

  • tests/hermes_cli/test_gateway_restart_loop.py:266 will fail: it looks for lowercase gateway would kill this command, but the existing error at tools/terminal_tool.py:2449 contains The gateway would kill this command with uppercase T.

Suggested changes

  • Normalize the asserted error with .lower() or use the emitted capitalization, then rerun the focused restart-loop test file.

Automated hermes-sweeper review.

@@ -244,6 +263,7 @@ def test_blocks_lifecycle_commands_inside_gateway(self, monkeypatch, cmd):

assert result["exit_code"] == 1
assert "Blocked" in result["error"]
assert "gateway would kill this command" in result["error"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion is case-sensitive, but the established terminal error says The gateway would kill this command (tools/terminal_tool.py:2449). Assert against result["error"].lower() or use the emitted capitalization; otherwise every case in this parametrized test fails.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management tool/terminal Terminal execution and process management needs-decision Awaiting maintainer decision before any implementation labels Jul 31, 2026
@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 31, 2026
@hermes-jeffrolc
hermes-jeffrolc force-pushed the fix/launchctl-submit-lifecycle-guard branch from 2caa781 to cb024d8 Compare July 31, 2026 18:26

Copy link
Copy Markdown
Contributor

The direct-submit guard is worth landing, but Closes #62891 is too strong at the current scope.

The second production occurrence documented on the issue used a neutral launchd registration with StartInterval 15, and this PR explicitly preserves neutral bootstrap and load. It also leaves outer-script indirection out of scope. That means the same failure family still has a verified path even if command-position launchctl submit is blocked.

I’d change this to Related to #62891, land the narrow local-terminal guard once the review test is corrected, and keep #62891 open with the remaining bootstrap and script path tracked. If broader closure is the goal, #63272’s execution-aware work is the relevant salvage source, but I’d split and re-review that rather than pull it in wholesale.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Nine PRs address or reference the gateway self-lifecycle failure family. #51003, #62896, #63272, #75557, and #75972 target launchd registration or script indirection; #20489, #56517, #57305, and #58289 cover adjacent uninstall, path-expansion, raw-PID, and systemd recovery gaps rather than the reported neutral-label persistent-job cause.

Related pull requests

Duplicates

#51003, #62896, #63272, and #75557 substantially overlap on blocking launchd-mediated gateway restart jobs and are superseded by #75972; #62896 supplies continuation normalization, #51003 supplies submit/bootstrap coverage, #63272 supplies the execution-aware recursive scanner, and #75557 supplies a narrower direct-submit guard. #20489, #56517, #57305, and #58289 are adjacent but not duplicates of that fix.

Suggested consolidation

Close #75557 as duplicate of #75972, with the chain #75557#75972; likewise close #51003, #62896, and #63272 as duplicates or incorporated predecessors of #75972. This departs from the visible keep_open review on #75557 because the later merged #75972 diff covers the exact direct-submit scope and additionally closes the documented neutral-label bootstrap and referenced-script paths that #75557 leaves open. Keep #20489, #56517, #57305, and #58289 separate for their explicitly identified salvage work rather than treating them as fixes for #62891.

Complex graph

flowchart TD
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I62891(["issue #62891 (closed)"])
    subgraph Dup51003 ["PRs duplicating each other"]
        P51003["PR #51003 (closed)"]
        P62896["PR #62896 (closed)"]
        P63272["PR #63272 (closed)"]
        P75557["PR #75557 (open)"]
        P75972["PR #75972 (merged)"]
    end
    P75557 -.->|partial| I62891
    class I62891 closed
    class P51003 closed
    class P62896 closed
    class P63272 closed
    class P75557 open
    class P75972 merged
    class P62896 best
    class P63272 best
    class P75972 best
    class P75557 target
    click I62891 "https://github.com/NousResearch/hermes-agent/issues/62891"
    click P51003 "https://github.com/NousResearch/hermes-agent/pull/51003"
    click P62896 "https://github.com/NousResearch/hermes-agent/pull/62896"
    click P63272 "https://github.com/NousResearch/hermes-agent/pull/63272"
    click P75557 "https://github.com/NousResearch/hermes-agent/pull/75557"
    click P75972 "https://github.com/NousResearch/hermes-agent/pull/75972"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 9 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 130 kB of PR diffs, 22 kB of issue/PR text, 18 kB of discussion (19 comments), 10 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@hermes-jeffrolc hermes-jeffrolc closed this by deleting the head repository Aug 4, 2026
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 needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users 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 sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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.

macOS gateway lifecycle guard can be bypassed via launchctl submit, creating an infinite restart loop

5 participants