test(approval): patch _YOLO_MODE_FROZEN directly in test_yolo_overrides_cron_deny - #32019
Merged
Conversation
…es_cron_deny The test set HERMES_YOLO_MODE=1 via monkeypatch.setenv, expecting check_dangerous_command() to honor yolo and bypass cron_mode=deny. But tools.approval._YOLO_MODE_FROZEN is intentionally frozen at module import time (security: prevents prompt-injection runtime escalation). When CI imports the module BEFORE the test sets the env, the frozen value stays False and the yolo bypass never activates. Local runs missed this because the conftest leaked a non-empty HERMES_YOLO_MODE into the import-time env. CI's clean-env path exposed the bug deterministically on test (3) / test (4) shards. Fix: patch the module attribute directly via mock.patch.object so the test simulates process-startup-with-yolo regardless of import order. The behavior under test (yolo bypasses cron_mode=deny for non-hardline commands) is unchanged; the security invariant (_YOLO_MODE_FROZEN can't be set at runtime by skills) is preserved. Reproduced locally with: env -i HOME=$HOME PATH=$PATH python3 -m pytest tests/tools/test_cron_approval_mode.py -o 'addopts=' -v Without the fix: 1 failed, 23 passed. With the fix: 24 passed.
Collaborator
Contributor
🔎 Lint report:
|
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.
Summary
test_yolo_overrides_cron_denynow passes deterministically in CI. Pre-existing test-isolation bug exposed by CI's clean-env conftest — blocks every PR routed to thetest (3)/test (4)shard.Root cause
tools.approval._YOLO_MODE_FROZENis intentionally frozen at module import time to prevent prompt-injection escalation:The test set
HERMES_YOLO_MODE=1viamonkeypatch.setenvAFTER the module had already been imported, so the frozen value stayedFalseand the yolo-bypass path never activated. Result:check_dangerous_command("rm -rf /tmp/stuff", "local")returnedapproved=False, the test assert failed.Why local passes: the conftest setup leaks a non-empty
HERMES_YOLO_MODEinto the import-time env on developer machines. CI'senv -i HOME=$HOME PATH=$PATHclean-env path exposes the bug deterministically.Fix
Patch the module attribute directly via
mock.patch.object, simulating process-startup withHERMES_YOLO_MODE=1regardless of import order. The behavior under test (yolo bypassescron_mode=denyfor non-hardline commands) is unchanged. The security invariant (_YOLO_MODE_FROZENcan't be set at runtime by skills) is preserved.Changes
tests/tools/test_cron_approval_mode.py—test_yolo_overrides_cron_denyaddsmock.patch.object(tools.approval, "_YOLO_MODE_FROZEN", True)alongside the existing_get_cron_approval_modepatch.Validation
env -i HOME=$HOME PATH=$PATH pytest tests/tools/test_cron_approval_mode.py_YOLO_MODE_FROZENfrozenWhy fix-it-first PR
This is the second night in a row a different PR has been blocked by this same test failure (PR #32013 Windows freeze, PR #32017 KBI guard, PR #32018 bracketed-paste timeout all hit it). Per
references/green-ci-policy.md, pre-existing failures get a separate fix-it PR so the affected PRs can rebase onto a clean main.Will rebase #32013 / #32017 / #32018 onto this once it lands.
Infographic
https://v3b.fal.media/files/b/0a9b9e23/O6oTzesAd4CJlfSUJvOMc_C0Mg0cuv.png