fix(cron): stop lifecycle guard false-positives and crashes on .py/binary scripts - #77201
Closed
criptogus wants to merge 1 commit into
Closed
fix(cron): stop lifecycle guard false-positives and crashes on .py/binary scripts#77201criptogus wants to merge 1 commit into
criptogus wants to merge 1 commit into
Conversation
…nary scripts The gateway lifecycle guard (cron/lifecycle_guard.py) applied shell-style tokenization and script-reference resolution to non-shell content, with two regressions: NousResearch#77131 - every .py cron script using pathlib division was hard-blocked: Path.home() / ".hermes" / ".env" tokenizes the bare "/" operator as an executable path, which resolves to the filesystem root; the regular-file check then fails closed as unsafe. Since Python runs under the interpreter, never through a POSIX shell, the shell-script reference walk is a false-positive generator on Python sources. check_gateway_lifecycle now skips the walk for *.py scripts (the direct command regex still scans the full text), and _iter_referenced_shell_scripts skips pure-separator tokens. NousResearch#76762 - terminal commands invoking a binary by absolute path (e.g. /usr/bin/python3) crashed the guard with ValueError: embedded null byte: the walk read the binary's bytes, decoded them as text, and re-tokenized machine code; the recursion then hit Path.resolve() on a NUL-bearing path while only OSError was caught. _read_referenced_script now skips NUL-containing files (binaries are not referenced shell scripts) and resolve() tolerates ValueError. Shell scripts (.sh/.bash/.zsh) keep the full deep scan; literal lifecycle commands in .py scripts are still blocked by the direct regex. New tests cover all four behaviors.
Collaborator
|
Merged via #77332 using your implementation — your commits cherry-picked with authorship preserved. Your PR was the most thorough of the three competing fixes: it preserved |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What
The gateway lifecycle guard (
cron/lifecycle_guard.py) applied shell-style tokenization and script-reference resolution to non-shell content, producing two regressions:#77131 — every
.pycron script using pathlib division was hard-blockedPath.home() / ".hermes" / ".env"tokenizes the bare/operator as an executable path, which resolves to the filesystem root; the regular-file check then fails closed asunsafe. Since Python runs under the interpreter, never through a POSIX shell, the shell-script reference walk is a false-positive generator on Python sources.#76762 — terminal commands invoking a binary by absolute path crashed the guard
/usr/bin/python3 -c "print(1)"crashed withValueError: embedded null byte: the walk read the binary's bytes, decoded them as text, and re-tokenized machine code; the recursion then hitPath.resolve()on a NUL-bearing path while onlyOSErrorwas caught.Fix
check_gateway_lifecycle— skip the shell-script reference walk for*.pyscripts (the direct command regex still scans the full text, so a literal lifecycle command embedded in.pyis still blocked; non-regular/oversized files still fail closed via the sentinel)._iter_referenced_shell_scripts— skip pure-separator tokens (pathlib's/operator)._read_referenced_script— treat NUL-containing files (ELF/Mach-O/PE binaries) as "nothing to scan": binaries are not referenced shell scripts, and scanning them fed junk paths into the recursion._contains_unsafe_gateway_action— tolerateValueErroratPath.resolve()time (not justOSError).Shell scripts (
.sh/.bash/.zsh) keep the full deep scan; the referenced-script walk still catches a.shwrapper that invokes another lifecycle script.How to test
4 new regression tests:
.pywith pathlib division → not blocked (cron lifecycle guard blocks ALL .py no-agent job creation (pathlib '/' false positive) #77131).pywith literalos.system("hermes gateway restart")→ still blocked/usr/bin/python3 -c "print(1)"→ no crash, returns False (terminal tool: lifecycle_guard crashes on absolute-path executables (ValueError: embedded null byte), blocks all such commands #76762).shwrapper referencing a lifecycle script → still blocked (walk preserved)E2E verified:
cron.jobs.create_jobwith a pathlib-heavy.pyscript now succeeds; withevil.pyit is still rejected.Platforms tested
test_gateway_restart_loop.pysuite (82 passed), cron area (153 passed), terminal tool (129 passed; 1 pre-existing unrelated theme test failure that reproduces onmain).Closes #77131, closes #76762