fix(approval): keep self-termination gated under the container-backend bypass (#71957)

[PRATHAMESH75]

Problem

Fixes #71957.

check_dangerous_command / check_all_command_guards skip dangerous-command approval entirely when terminal.backend is an isolated container backend (docker, singularity, modal, daytona), on the documented rationale that the container is the host-safety boundary — a destructive command inside the sandbox can't reach the host.

That rationale is sound for rm -rf / mkfs / dd, but it also silently waived the self-termination guards, whose rationale has nothing to do with host safety. Killing the agent's own gateway process is a self-inflicted service DoS / mid-session state loss that disrupts the operator's own running service whether or not it touches the host:

• pkill hermes, pkill -9 hermes, killall gateway, kill $(pgrep -f hermes)
• hermes gateway stop|restart, hermes update
• gateway run with &/disown/nohup/setsid
• docker compose down, docker restart/stop/kill (docker.sock is commonly mounted in the Compose deployment)
• launchctl bootout … ai.hermes.gateway

Because the documented, recommended Docker Compose deployment runs terminal.backend: docker, self-termination prevention was effectively disabled by default for a large share of gateway operators — an agent could restart or kill its own gateway with no approval prompt (exactly as reported in the issue).

Fix

Carve the self-disruption pattern family out of the container bypass in both shell-command entry points. Under a container backend these commands now route through the normal approval gate as they would on local/ssh; every other dangerous command is still waived exactly as before (verified by test). The change is purely additive gating — it never weakens any existing check.

Implementation notes:

• _SELF_DISRUPTION_DESCRIPTIONS names the subset of DANGEROUS_PATTERNS whose sole purpose is preventing self-service disruption (self-termination + gateway/container lifecycle teardown), distinguished from host-scoped destruction which the container boundary legitimately covers.
• The carve-out matches the self-disruption patterns directly (_matches_self_disruption_pattern) rather than relying on detect_dangerous_command's first match, so a command like pkill -9 hermes — which matches the generic "force kill processes" rule first — is still recognised as self-termination and gated.
• Scoped to the two shell-command guards (check_dangerous_command, check_all_command_guards); check_execute_code_guard operates on Python source, not shell command strings, and is left unchanged.

Tests

New tests/tools/test_container_self_disruption_bypass.py (17 tests):

• self-termination gated under every container backend (docker/singularity/modal/daytona);
• host-destructive rm -rf /workspace still bypassed under every container backend (callback never consulted);
• the pkill -9 hermes first-match edge case still gated;
• kill $(pgrep -f hermes) and hermes gateway restart gated;
• an interactive user who approves still gets the command run (only the prompt is re-introduced);
• local backend behaviour unchanged;
• a drift guard asserting every carve-out description still exists in DANGEROUS_PATTERNS.

ruff and the Windows-footgun check pass; pyproject.toml/uv.lock untouched.

Preflight note: the local affected-test run reports one macOS-only failure, test_approval.py::TestDetectDangerousRm::test_nonrecursive_verification_artifact_cleanup_is_not_dangerous — a pre-existing /tmp→/private/tmp verification-artifact false-fail that also fails on a clean upstream/main checkout with none of this change applied. It is unrelated to this PR.

[alt-glitch]

This was generated by AI during triage.

Related: #71957 identifies the container-bypass policy gap; #43157 guards own-PID termination unconditionally. This PR instead preserves the container bypass for host-destruction patterns while gating the self-disruption subset, so maintainer policy selection is still needed.

[PRATHAMESH75]

Confirming the triage note's read: this isn't a duplicate of #43157. #43157 guards own-PID termination unconditionally, which would also re-gate host-destruction patterns (rm -rf, mkfs, dd) inside a container backend — reversing the deliberate "the container is the host-safety boundary" bypass. This PR deliberately keeps that bypass intact and narrows the change to only the self-termination subset, whose rationale is self-inflicted-outage protection, not host safety.

So the two are different policy shapes, not redundant implementations, and which one lands is a maintainer policy call — I've left the container bypass otherwise untouched here so the choice stays open. No code change from this note.

[teknium1]

Thanks for preserving the existing host-destruction bypass while addressing a real shell-command gap: current main returns early for isolated containers before the self-disruption patterns can run (tools/approval.py:3080, tools/approval.py:3381).

Problems

• execute_code remains an equivalent bypass. It calls check_execute_code_guard before dispatch (tools/code_execution_tool.py:1237-1242), but that guard returns approved for isolated containers before evaluating the script (tools/approval.py:3863-3870). Arbitrary Python can invoke process-control APIs or subprocesses, so the proposed self-disruption guarantee is not preserved across this execution surface.
• The PR changes no docs, while the current container-bypass statements say checks are skipped (website/docs/user-guide/security.md:191; website/docs/guides/tips.md:220). Those statements would become inaccurate for the new exception.

Suggested changes

• Decide the intended execute_code behavior for isolated backends and add focused coverage for it, or explicitly narrow the feature contract to shell-command execution.
• Update both container-bypass callouts to state the self-disruption exception.

This is an automated hermes-sweeper review.

[PRATHAMESH75]

Thanks — both points addressed in 8596c624f.

• execute_code is an equivalent bypass surface. You're right: check_execute_code_guard shares the same _should_skip_container_guards early return (tools/approval.py:3736), so a script could subprocess.run("hermes gateway stop", shell=True) / os.system("pkill hermes") and evade the terminal-path carve-out. Applied the same carve-out there: when the container skip would apply, the script text is matched against _matches_self_disruption_pattern, and a self-disruption shell command routes through approval instead of being waived; ordinary scripts stay waived exactly as before. I chose to close the surface rather than only narrow the contract, since embedded shell self-termination is the realistic vector. I've kept the honest limitation the guard's docstring already states — a raw in-process kill (os.kill on a PID the script discovered) is not detectable by string patterns and remains out of scope; that's noted in the code comment. Scoped to _should_skip_container_guards, so vercel_sandbox (a separate always-skip with no local gateway to terminate) is intentionally unchanged.

  Coverage added in test_container_self_disruption_bypass.py: execute_code self-termination gated across all four container backends, hermes gateway stop gated in docker, ordinary scripts still waived, and vercel_sandbox still waived.

• Docs. Updated both container-bypass callouts to state the self-disruption exception — website/docs/user-guide/security.md and website/docs/guides/tips.md now note that killing the agent's own gateway/service (including execute_code scripts that issue such a command) still requires approval.

Full file green locally (the one unrelated failure is the known macOS /tmp→/private/tmp artifact-cleanup test).

[GottZ]

This was generated by AI during triage.

Summary

One PR addresses issue #71957. #71965 implements a targeted self-disruption exception to the container-backend bypass across shell-command and detectable execute_code paths, while preserving the bypass for unrelated host-destructive commands and documenting the exception.

Related pull requests

• fix(approval): keep self-termination gated under the container-backend bypass (#71957) #71965 best fix — (+330/-4) — best available fix; keep open: The diff re-gates self-termination and gateway/container lifecycle commands in check_dangerous_command, check_all_command_guards, and detectable shell commands embedded in execute_code, with coverage across all four affected container backends; unrelated destructive commands remain bypassed. Consistent with the automated keep_open review on fix(approval): keep self-termination gated under the container-backend bypass (#71957) #71965, its concrete salvage value is this narrow policy carve-out, and the updated diff now addresses that review's execute_code and documentation concerns, while explicitly leaving raw in-process os.kill detection out of scope.

Suggested consolidation

Keep #71965 open with a salvage path: retain its targeted self-disruption matcher, three guard-path carve-outs, focused regression tests, and updated container-bypass documentation. It is the only PR in this complex, closes #71957 directly, and is not a duplicate of #43157, whose broader own-PID policy would have a different effect; there are no duplicate PRs to close.

Complex graph

flowchart LR
    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
    I71957(["issue #71957 (open)"])
    P71965["PR #71965 (open)"]
    P71965 -->|best fix| I71957
    class I71957 open
    class P71965 open
    class P71965 best
    class P71965 target
    click I71957 "https://github.com/NousResearch/hermes-agent/issues/71957"
    click P71965 "https://github.com/NousResearch/hermes-agent/pull/71965"

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 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 20 kB of PR diffs, 7 kB of issue/PR text, 4 kB of discussion (4 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.