You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
close the remaining cronjob tool update-path bypass for gateway lifecycle commands
reuse the current shared cron.lifecycle_guard.check_gateway_lifecycle implementation
validate the effective stored-plus-updated prompt and script before persisting a partial update
preserve remediation: an update may replace or clear previously stored unsafe content
cover launchd and systemd scripts, lifecycle prompts, stored unsafe jobs, and script clearing
Root Cause
Current main already protects job creation in cron.jobs.create_job, but tools.cronjob_tools.cronjob(action="update") only validates a replacement script path before calling update_job(). It does not scan the script contents, and unrelated partial updates do not revalidate lifecycle content already stored on the job.
That leaves an update-only route for scheduling commands such as:
The fix merges the current update over the stored job for the two execution-content fields, then calls the existing shared lifecycle guard immediately before update_job().
Independent systemd reproduction (twice in one day) plus a suggested systemctl --user regression test over in #51976. This PR would have blocked our exact case — the agent created a no_agent cron job whose script ran systemctl --user restart hermes-gateway.service, which the shared lifecycle regex matches. +1 to landing it; the script-contents scan on the cronjob tool's create/update path is the missing defense.
Thanks for documenting both the launchd and systemd reproductions. Current main has already salvaged the create-path portion in b48cacb97: cron/jobs.py:1147-1153 invokes the shared cron.lifecycle_guard.check_gateway_lifecycle, and tests/hermes_cli/test_gateway_restart_loop.py:450-484 covers the model-tool create path.
Problems
The update bypass remains on current main: tools/cronjob_tools.py:897-903 validates only the replacement script path, then tools/cronjob_tools.py:959 persists it without a lifecycle-content check.
The submitted helper placement is stale. Current main's guard is cron/lifecycle_guard.py:112-141; adding gateway/lifecycle_guard.py would duplicate the established implementation and miss its current path-resolution and decoding behavior.
Suggested changes
Port only the update protection onto current cron.lifecycle_guard.check_gateway_lifecycle, checking the effective stored-plus-updated prompt/script before update_job().
Keep the launchctl/systemd regression cases, but place coverage with the current lifecycle-guard tests.
Reworked this on current main per the sweeper review. I removed the stale duplicate helper/create/terminal changes and kept only the remaining update-path protection. The update flow now computes the effective stored-plus-updated prompt and script, passes both through cron.lifecycle_guard.check_gateway_lifecycle, and only then calls update_job(). Remediation by replacing or clearing unsafe content remains allowed. Regression coverage now lives with the lifecycle-guard tests and covers launchd, systemd, prompt updates, stored unsafe content on unrelated updates, and clearing an unsafe script. Focused result: 203 passed; full tests/cron: 693 passed. Commit: 9ea9128.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
comp/cronCron scheduler and job managementneeds-decisionAwaiting maintainer decision before any implementationP2Medium — degraded but workaround existssweeper:blast-moderateSweeper blast radius: moderate — a subsystem or single platformsweeper:risk-automationSweeper risk: may affect CI, automerge, label sync, or maintainer automationtype/bugSomething isn't working
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cronjobtool update-path bypass for gateway lifecycle commandscron.lifecycle_guard.check_gateway_lifecycleimplementationRoot Cause
Current
mainalready protects job creation incron.jobs.create_job, buttools.cronjob_tools.cronjob(action="update")only validates a replacement script path before callingupdate_job(). It does not scan the script contents, and unrelated partial updates do not revalidate lifecycle content already stored on the job.That leaves an update-only route for scheduling commands such as:
The fix merges the current update over the stored job for the two execution-content fields, then calls the existing shared lifecycle guard immediately before
update_job().Fixes #51976
Test Plan
Result on current
main:203 passed, 1 pre-existing deprecation warning.