fix(gateway,cron): guard cron model-tool path + auto-resume loop breaker (#30719) - #247
Conversation
|
Review Complete Files Reviewed: 7 By Severity:
PR introduces a TOCTOU security gap in cron lifecycle guards allowing agents to bypass script-content checks, silent session cross-contamination via cached agent reuse across session_id boundaries, and an accidental regression in gateway secret redaction. Files Reviewed (7 files) |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: 🟠 High (78/100) — 1 high finding · 892 LOC across 7 files
Critical Issues
TOCTOU in cron lifecycle guard (finding-003) — The gateway lifecycle guard in cron/lifecycle_guard.py scans script content only at job creation time. When the script doesn't exist, the check silently passes with empty-string fallback. An agent can create a cron job referencing a non-existent file, then write lifecycle commands (e.g. systemctl restart) into it afterward. The scheduler's execution path has no lifecycle content check for no_agent=True scripts, making this the only defense layer — and it's trivially bypassable.
Session cross-contamination via cached agent reuse (finding-004) — The new 4-tuple session_id guard in gateway/run.py reuses cached agents across session switches but fails to update the agent's internal self.session_id. This causes conversation B's messages to be silently flushed to session A's database, corrupting both transcripts. The _last_flushed_db_idx reset at gateway/run.py:15097 compounds this by discarding prior flush state.
Moderate Issues
Secret redaction regression (finding-001) — The Slack xapp- token regex was accidentally dropped from _GATEWAY_SECRET_PATTERNS in gateway/run.py. The primary redaction path still catches it, but the defense-in-depth fallback no longer does.
| except OSError: | ||
| return "" |
There was a problem hiding this comment.
🟠 TOCTOU gap: cron lifecycle guard scans script at creation but script can be written later, bypassing the guard (security)
The gateway lifecycle guard in cron/lifecycle_guard.py checks script content at job creation time (called from cron.jobs.create_job:970). When the script file does not exist, _read_script_for_scanning() catches OSError and returns an empty string (line 108-109), falling back to prompt-only scanning. An agent can sequence cronjob creation (with a non-existent script and clean prompt) followed by write_file to populate the script with lifecycle commands (e.g., systemctl restart hermes-gateway). The cron scheduler (scheduler.py:_run_job_script) runs scripts without any lifecycle content check. For no_agent=True cron jobs, this is the ONLY defense against script-based lifecycle attacks — terminal_tool.py's execution guard does not apply to scheduler-executed scripts. A recurring cron spaced >60s apart can sustain a SIGTERM-respawn loop that evades the restart-loop breaker's 60s default window (defense-3 in gateway/restart_loop_guard.py), since boots spaced ~66s apart never accumulate 3 within the 60s window.
💡 Suggestion: Add a defense layer at script execution time in the cron scheduler. Options: (1) In cron/scheduler.py:_run_job_script, add a pre-execution lifecycle scan using the same _GATEWAY_LIFECYCLE_PATTERN regex before running the script. (2) At creation time, if the script doesn't exist and no_agent=True, reject the job creation or flag it for deferred re-scan. (3) Have create_job mark jobs with a pending_lifecycle_check flag when the script doesn't exist, and check it at execution time.
📋 Prompt for AI Agents
In cron/scheduler.py, add a lifecycle content check to _run_job_script() before executing the script. Import contains_gateway_lifecycle_command from cron.lifecycle_guard, read the script content (with errors='replace' for binary safety), and if a lifecycle command is detected, abort execution and log a warning. This closes the TOCTOU gap for all script execution paths regardless of when the script was written. Example: after resolving the script path and before subprocess execution, add: if contains_gateway_lifecycle_command(path.read_bytes().decode('utf-8', errors='replace')): return (False, 'Blocked: script contains gateway lifecycle command').
1 participant
Summary
Completes the NousResearch#30719 gateway restart-loop defenses. Defenses 1–2 already landed on
mainunder different names; this closes the two remaining gaps and hardens the filter.The foot-gun: an agent schedules
hermes gateway restart(orlaunchctl kickstart ai.hermes.gateway), the cron fires → SIGTERM → supervisor KeepAlive revives the gateway → auto-resume replays the offending session → the turn re-runs the same logic. A ~10s SIGTERM-respawn loop until broken by hand (real macOS repro, 2026-05-22).What was already on main
_HERMES_GATEWAY=1guard:hermes gateway stop|restartrefuse from inside the gateway, andterminal_toolhard-blocks lifecycle commands at execution time.hermes_cli.cron.cron_createrejected lifecycle prompts/scripts.What this PR adds
cronjobmodel tool callscron.jobs.create_jobdirectly, bypassing the CLI filter, so model-tool-scheduled lifecycle jobs were only caught at execution. Moved the filter into a sharedcron/lifecycle_guard.pyenforced atcreate_job— the single path every caller (CLI + model tool) hits._contains_gateway_lifecycle_commandis re-exported fromhermes_cli.cronsoterminal_tool's import is unchanged. Relative script paths resolve underHERMES_HOME/scriptsexactly as the scheduler runs them, so a bare script name is scanned as the file that actually executes.gateway/restart_loop_guard.pycounts restart-interrupted boots in a rolling window (gateway.restart_loop_guard, default 3 boots / 60s) and skips auto-resume for that boot once tripped. The gateway still comes up and serves real inbound messages — it just stops replaying the session that keeps killing it, putting a human back in the loop. Catches every SIGTERM source defenses 1–2 don't (e.g. a rawterminal("launchctl kickstart …")). Fails open on any state error.hermes gateway start; require the gateway identifier on the launchctl/systemctl branches (launchctl unload ai.hermes.update-checker.plistandsystemctl restart hermes-meta.serviceno longer false-positive); added the inversepkill gateway … hermestoken order; fixed the binary-script bypass (decodeerrors="replace"instead of swallowingUnicodeDecodeError).Changes
cron/lifecycle_guard.py(new): shared checker +check_gateway_lifecycleraisingGatewayLifecycleBlocked.cron/jobs.py: enforce the guard increate_job.hermes_cli/cron.py: re-export the checker; drop the now-redundant CLI-layer guard.gateway/restart_loop_guard.py(new) +gateway/run.py: loop breaker + wiring in_schedule_resume_pending_sessions.hermes_cli/config.py:gateway.restart_loop_guarddefaults.tests/hermes_cli/test_gateway_restart_loop.pyextended.Validation
cronjobcreate (prompt + script abuse)Credit
Design and much of defense-2 originate from PR NousResearch#33395 (@kshitijk4poor), which salvaged NousResearch#30728 (@SimoKiihamaki). Rebuilt against current
mainbecause defenses 1–2 had already landed under different names (_HERMES_GATEWAY), so cherry-picking NousResearch#33395 wholesale would have re-added a duplicateHERMES_IN_GATEWAYenv flag. Both authors preserved asCo-authored-by.Closes NousResearch#30719.
Infographic
Mirror-of: NousResearch#56240
NousResearch#56240