fix(cron): scope lifecycle guard to real shell scripts; per-job cron flag via ContextVar

[samson-mak]

Summary

Fixes three approval-gate defects that appear after a gateway restart:

1. cron/lifecycle_guard.py — false-positive "cannot restart or stop the gateway" blocks

_iter_referenced_shell_scripts treated any full-path executable (/Users/.../venv/bin/python3, ~/.hermes/bin/claude, …) as a shell script to read and scan. Real binaries were decoded as UTF-8 with errors="replace" — which embeds NUL bytes — and the resolver then crashed with ValueError: embedded null byte (only OSError was caught). In the common case it returned unsafe=True, hard-blocking legitimate dispatch commands with a fake gateway-lifecycle verdict.

Fix: only treat files as shell scripts when they end in .sh/.bash/.zsh or their first 512 bytes carry a POSIX-shell shebang (_is_shell_script_file). Also fixed dot-source (Path(".").name is "" so . script.sh was never scanned) and added (OSError, ValueError) guards on all os.open/resolve paths.

2. cron/scheduler.py — process-wide HERMES_CRON_SESSION leak

run_job set os.environ["HERMES_CRON_SESSION"] = "1" process-wide. After a gateway restart, the long-lived scheduler process kept the flag set, so every live session (Slack/Discord/…) was misclassified as a cron job and execute_code was hard-denied via approvals.cron_mode: deny.

Fix: per-job ContextVar (HERMES_CRON_SESSION_CONTEXTVAR) set inside the lock-guarded try: and reset in finally: (with a None token guard), so an exception can't leak the terminal-cwd lock.

3. tools/approval.py — _is_cron_session()

All four approval-gating sites now check the ContextVar first with an env fallback (backward compatible for standalone cron processes).

Tests

New tests/cron/test_lifecycle_guard_regressions.py (12 tests):

• real full-path binary (venv/bin/python3 -c ...) → not flagged
• . /path/evil.sh and source /path/evil.sh → flagged
• bash /path/evil.sh → flagged
• large (>4 KB) extensionless #!/bin/sh script with a restart command → flagged
• real cron.scheduler.run_job exception path → ContextVar reset + writer lock released (no leak)

tests/tools/test_execute_code_approval_cluster.py: 21 passed — the 3 one-shot/smart-approval tests were failing on machines whose config.yaml allowlists execute_code (the permanent allowlist short-circuited the gateway approval branch). The gw_session fixture now snapshots/restores _permanent_approved (second commit), plus a new test_guard_permanent_allowlist_is_isolated pins the allowlist shortcut.

Suite results: test_lifecycle_guard_regressions.py 12 passed · test_terminal_cwd_lock.py 4 passed · test_cron_approval_mode.py 26 passed · test_execute_code_approval_cluster.py 21 passed.

Notes

• Reviewed by gpt-5.6-sol in 3 rounds (final verdict ACCEPT; no remaining findings).
• Reproduction & fix validated on a live gateway: after restart, HERMES_CRON_SESSION no longer set, full-path python3 no longer blocked.

[alt-glitch]

This was generated by AI during triage.

Related to #76773 and the cron ContextVar cluster (#56796, #58663). The lifecycle-guard patches differ on whether an extensionless text script without a shell shebang remains scanable, while this PR also contains the separate cron-context repair; please choose the intended guard policy before consolidating.

[teknium1]

Thanks for isolating the cron approval marker; the underlying leak is present on current main at cron/scheduler.py:3011-3014, and the existing worker handoff copies ContextVars at cron/scheduler.py:3564-3569.

Problems

• cron/lifecycle_guard.py:222-226 narrows referenced-script scanning to extensions or recognized shell shebangs. That drops directly executed extensionless text scripts without a shebang from the lifecycle scan, weakening the intentional defense-in-depth guard. The current implementation deliberately scans slash-containing paths (cron/lifecycle_guard.py:222-223), and the member note correctly flags this policy decision.
• tests/cron/test_lifecycle_guard_regressions.py:111-161 forces an exception before the agent worker submission, so it does not verify the actual scheduler worker propagation or concurrent gateway isolation.

Suggested changes

• Preserve extensionless text-script scanning while skipping clearly binary headers (the related #76773 approach uses a NUL-byte header distinction).
• Add a real worker-handoff/concurrent-context regression around cron/scheduler.py:3568-3569.

Automated hermes-sweeper review.

[teknium1]

This excludes a directly executed extensionless text script with no shebang from scanning, so ./restart-helper can contain hermes gateway restart and evade the lifecycle guard. Please preserve scanning for text candidates and skip only clearly binary files (for example, a bounded NUL-byte header check).

[samson-mak]

Resolved in 7deebd1a0 — _is_shell_script_file now uses a bounded NUL-byte header check: text candidates (no NUL in first 512 bytes) are scanned, clearly binary files are skipped. ./restart-helper (extensionless, no shebang) is covered by the new test_extensionless_text_script_without_shebang_is_scanned.

[samson-mak]

Guard policy decision: bounded NUL-byte header check — scan text candidates (no NUL in first 512 bytes), skip clearly binary files. Landed in 7deebd1a0, aligned with the #76773 approach; ready to consolidate.

[samson-mak]

@teknium1 both findings addressed in 7deebd1a0. @alt-glitch guard policy decided.

1. lifecycle_guard — NUL-byte header check (_is_shell_script_file)
Read first 512 bytes; NUL present → binary → skip; no NUL → text candidate → scan. Preserves extensionless shebang-less text-script scanning (./restart-helper evasion case, new regression test) while keeping the binary NUL-crash fixes from f03e785. Also updated test_full_path_non_shell_binary_is_not_scanned: the old 2 MiB b"x" payload has no NUL byte, so under NUL detection it classifies as text and would trip the 1 MiB read cap (unsafe → block); payload is now NUL-padded.

2. Real worker-handoff regression — test_run_job_handoff_propagates_cron_context_and_isolates_concurrent_thread drives the actual cron.scheduler.run_job through the scheduler.py:3568-3569 copy_context submission: worker thread sees HERMES_CRON_SESSION_CONTEXTVAR True, a concurrent plain thread (live gateway session) sees False, scheduler thread resets to False after return.

Validation (Hermes venv, worktree at 7deebd1a0):

• test_lifecycle_guard_regressions.py + test_terminal_cwd_lock.py + test_cron_approval_mode.py → 44 passed
• test_execute_code_approval_cluster.py → 17 passed / 3 failed — identical at HEAD (pre-existing, unrelated)

Guard policy (for consolidation with #76773): bounded NUL-byte header check — scan text candidates, skip binary.

[mohitagrawal-marvis]

+1 — I hit this same bug in the wild and opened #77233 (now closed in favor of this PR), so I can confirm this fix is needed and correct.

Independent reproduction: a terminal command referencing a venv interpreter by path (e.g. .venv/bin/python -m src.main) crashed the guard with ValueError: embedded null byte — the referenced file was decoded as UTF-8, tokenized as shell text, and Path.resolve() blew up on the embedded NUL bytes. This broke legitimate cron/script dispatch immediately after the referenced-script scanning landed.

Validating the policy here: my own testing confirmed the exact behavior this PR implements — a NUL-byte-laden binary executable passes through cleanly (no crash, no false block), while real shell scripts (.sh/shebang) are still scanned and a lifecycle command embedded in them is still blocked. The scan-selection approach here (only treat .sh/.bash/.zsh or shebang-bearing files as scripts) is strictly better than my NUL-stripping approach, which still scanned binaries and could false-positive on a binary that merely contains the command string.

Also appreciated: the dot-source . script.sh fix and the HERMES_CRON_SESSION ContextVar leak fix riding along — both real.

[alt-glitch]

This was generated by AI during triage.

Related: #56796 and #58663 address the cron-session ContextVar family. This PR also repairs lifecycle-guard false positives and carries distinct cleanup coverage, so it is not a same-scope duplicate; maintainer comparison is needed.

[GottZ]

This was generated by AI during triage.

Graph note (no action implied — a maintainer has already reviewed this thread).

Our triage graph places this PR in a complex with 1 related pull request (#77233). They were checked against each other at the diff level and no consolidation is indicated — they address distinct causes.

Full neighbourhood: https://hermes-triage.gottz.de/?node=76797

This note exists so the relationship stays discoverable from the thread itself.

[alt-glitch]

This was generated by AI during triage.

Related: #77383 is the focused lifecycle-guard repair and #56796 is the focused cron ContextVar repair. This conflicting branch bundles both; please rebase and decide whether to split or consolidate.

[saved-j]

Hey! I ran into the same wall and made a complementary fix — there are a couple of holes left even with your scoping. Verified on a live machine (macOS, gateway under launchd with no HOME in the env):

1. RuntimeError: Could not determine home directory — expanduser() in _resolve_terminal_script_path() / _resolve_script_path() raises this (not ValueError) when a token starts with ~ and the env has no HOME. You catch (OSError, ValueError) — the RuntimeError slips through and kills the guard exactly like the NUL byte did.

2. tools/terminal_tool.py _read_script_in_env() — decodes any file up to 1 MB as UTF-8 with errors="replace", binaries included. Even with your iterator scoping: when the recursive scan walks a script that execs venv/bin/python inside, the binary gets decoded into garbage text containing gateway/restart — and the command gets falsely blocked with "cannot restart or stop the gateway". Your PR doesn't touch this file.

My PR (#78056): reject NUL bytes before touching pathlib/files + catch (OSError, ValueError, RuntimeError) at all four sites in lifecycle_guard.py, plus a NUL guard in _read_script_in_env (local and remote branches) matching the existing #76762 check.

On the live box: full-path python -c, ~ with no HOME, NUL-bearing tokens — all pass; real hermes gateway restart still blocked.

I think both PRs should land together — they fix adjacent layers of the same guard. Happy to rebase if needed.