From 5cbc3fbdcc13c3a5d6d0f565a1d1929d0e5e47ff Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Thu, 28 May 2026 20:17:51 +0530 Subject: [PATCH] fix(cli): /yolo in chat must enable session bypass, not just set env var MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CLI's in-chat `/yolo` toggle mutated `os.environ["HERMES_YOLO_MODE"]` but had no effect because `tools/approval.py:_YOLO_MODE_FROZEN` captures that env var once at module-import time (a deliberate security floor that keeps prompt-injected skills from flipping the bypass mid-run). By the time the user reaches `/yolo` in a running CLI session, `tools.approval` has already been imported, so the env flip after that is a silent no-op. Result: `/yolo` advertised "⚠ YOLO" in the status bar while every dangerous command still hit the approval prompt or got denied. Only `hermes --yolo` (set before tool imports), `HERMES_YOLO_MODE=1 hermes ...`, and `hermes config set approvals.mode off` actually bypassed. This patches the CLI to match what the gateway and TUI `/yolo` handlers already do, plus mirrors the TUI's session-rename YOLO transfer: * `_toggle_yolo()` now calls `enable_session_yolo(self.session_id)` / `disable_session_yolo(self.session_id)` instead of touching the env var. Matches `gateway/run.py:_handle_yolo_command` and the `tui_gateway/server.py` key=="yolo" branch. * Around each `run_conversation()` call, `run_agent()` now binds `set_current_session_key(self.session_id)` so `tools.approval.is_current_session_yolo_enabled()` resolves against the same key the toggle writes under, and resets it in `finally` so reused threads don't see stale identity. Matches the `tui_gateway/server.py` and `gateway/platforms/api_server.py` binding pattern. * New `_transfer_session_yolo()` helper carries YOLO bypass state across `self.session_id` reassignments — `/branch` forking into a new session id and the auto-compression sync that rotates into a fresh continuation session id. Without this, the same UX failure mode the rest of this fix addresses (silent `/yolo` no-op) would reappear after a single `/branch` or auto-compression event. Mirrors `tui_gateway/server.py` ~line 1297-1305. * New `_is_session_yolo_active()` helper replaces the two `bool(os.getenv("HERMES_YOLO_MODE"))` reads in the status-bar builders, so the badge reflects the actual bypass state. Uses `getattr(self, "session_id", None)` so status-bar test fixtures that bypass `__init__` via `HermesCLI.__new__(HermesCLI)` don't trip `AttributeError` (the builders swallow exceptions silently and lose every field after the failure). Still honors `_YOLO_MODE_FROZEN` so `hermes --yolo` keeps lighting it up. The `_YOLO_MODE_FROZEN` security freeze is preserved — env-var-based opt-in still only works when set before process start, which is the documented contract for `--yolo` / `HERMES_YOLO_MODE`. Closes #33925 --- cli.py | 122 +++++++++++++-- tests/cli/test_cli_yolo_toggle.py | 244 ++++++++++++++++++++++++++++++ 2 files changed, 355 insertions(+), 11 deletions(-) create mode 100644 tests/cli/test_cli_yolo_toggle.py diff --git a/cli.py b/cli.py index f0478d839c9..aeffd8bad8a 100644 --- a/cli.py +++ b/cli.py @@ -168,7 +168,7 @@ from hermes_cli.browser_connect import ( try_launch_chrome_debug, ) from hermes_cli.env_loader import load_hermes_dotenv -from utils import base_url_host_matches, is_truthy_value +from utils import base_url_host_matches _hermes_home = get_hermes_home() _project_env = Path(__file__).parent / '.env' @@ -3747,7 +3747,7 @@ class HermesCLI: percent_label = f"{percent}%" if percent is not None else "--" duration_label = snapshot["duration"] - yolo_active = bool(os.getenv("HERMES_YOLO_MODE")) + yolo_active = self._is_session_yolo_active() if width < 52: text = f"⚕ {snapshot['model_short']} · {duration_label}" if yolo_active: @@ -3808,7 +3808,7 @@ class HermesCLI: # line and produce duplicated status bar rows over long sessions. width = self._get_tui_terminal_width() duration_label = snapshot["duration"] - yolo_active = bool(os.getenv("HERMES_YOLO_MODE")) + yolo_active = self._is_session_yolo_active() if width < 52: frags = [ @@ -6907,6 +6907,7 @@ class HermesCLI: pass # Switch to the new session + self._transfer_session_yolo(self.session_id, new_session_id) self.session_id = new_session_id self.session_start = now self._pending_title = None @@ -9619,20 +9620,92 @@ class HermesCLI: } _cprint(labels.get(self.tool_progress_mode, "")) - def _toggle_yolo(self): - """Toggle YOLO mode — skip all dangerous command approval prompts.""" - import os - from hermes_cli.colors import Colors as _Colors + def _transfer_session_yolo(self, old_session_id: str, new_session_id: str) -> None: + """Move YOLO bypass state from an old session key to a new one. - current = is_truthy_value(os.environ.get("HERMES_YOLO_MODE")) - if current: - os.environ.pop("HERMES_YOLO_MODE", None) + Called whenever ``self.session_id`` is reassigned mid-run — ``/branch`` + forks into a new session, and auto-compression rotates the agent's + session id into a fresh continuation session. Without this transfer + the user's ``/yolo ON`` toggle would silently revert on the very next + turn (the same UX failure mode that motivated this entire fix), since + ``_session_yolo`` is keyed by session id. + + Mirrors ``tui_gateway/server.py`` (~line 1297-1305) which performs the + same transfer for the TUI's session-rename path. No-op when YOLO + wasn't enabled or when the ids match. + """ + if not old_session_id or not new_session_id or old_session_id == new_session_id: + return + try: + from tools.approval import ( + disable_session_yolo, + enable_session_yolo, + is_session_yolo_enabled, + ) + except Exception: + return + if is_session_yolo_enabled(old_session_id): + enable_session_yolo(new_session_id) + disable_session_yolo(old_session_id) + + def _is_session_yolo_active(self) -> bool: + """Whether YOLO bypass is currently enabled for this CLI session. + + Reads from ``tools.approval._session_yolo`` (the same set that + ``enable_session_yolo`` / ``disable_session_yolo`` write to) so the + status bar reflects the actual bypass state instead of a stale env + var. Also honors the process-start ``--yolo`` flag, which freezes + ``HERMES_YOLO_MODE`` into ``_YOLO_MODE_FROZEN`` before tool imports + happen. + """ + try: + from tools.approval import ( + _YOLO_MODE_FROZEN, + is_session_yolo_enabled, + ) + except Exception: + return False + if _YOLO_MODE_FROZEN: + return True + # Use ``getattr`` so test fixtures that build a CLI via ``__new__`` + # (skipping ``__init__``) don't trip an AttributeError here; the + # status-bar builders swallow exceptions silently but lose every + # field after the failure. + session_key = getattr(self, "session_id", None) or "default" + return is_session_yolo_enabled(session_key) + + def _toggle_yolo(self): + """Toggle YOLO mode — skip all dangerous command approval prompts. + + Per-session toggle that mirrors the gateway and TUI ``/yolo`` handlers + (see ``gateway/run.py:_handle_yolo_command`` and + ``tui_gateway/server.py`` key=="yolo"). We deliberately do NOT mutate + ``HERMES_YOLO_MODE`` here — that env var is read once at module import + time into ``tools.approval._YOLO_MODE_FROZEN`` to keep prompt-injected + skills from flipping the bypass mid-session, so setting it after CLI + startup is a silent no-op. Routing through ``enable_session_yolo`` / + ``disable_session_yolo`` gives the same auditable, per-session bypass + the other surfaces have. ``run_conversation`` binds + ``self.session_id`` as the active approval session key via + ``set_current_session_key`` so the bypass takes effect on the very + next dangerous command in this run. + """ + from hermes_cli.colors import Colors as _Colors + from tools.approval import ( + disable_session_yolo, + enable_session_yolo, + is_session_yolo_enabled, + ) + + session_key = self.session_id or "default" + if is_session_yolo_enabled(session_key): + disable_session_yolo(session_key) _cprint( f" ⚠ YOLO mode {_Colors.BOLD}{_Colors.RED}OFF{_Colors.RESET}" " — dangerous commands will require approval." ) else: - os.environ["HERMES_YOLO_MODE"] = "1" + enable_session_yolo(session_key) _cprint( f" ⚡ YOLO mode {_Colors.BOLD}{_Colors.GREEN}ON{_Colors.RESET}" " — all commands auto-approved. Use with caution." @@ -11769,6 +11842,23 @@ class HermesCLI: set_secret_capture_callback(self._secret_capture_callback) except Exception: pass + # Bind this turn's approval session key into the contextvar so + # ``tools.approval.is_current_session_yolo_enabled()`` resolves + # against the same key that ``/yolo`` toggles under (see + # ``_toggle_yolo`` → ``enable_session_yolo(self.session_id)``). + # Mirrors ``tui_gateway/server.py`` and ``gateway/run.py`` which + # bind the same contextvar before invoking the agent. + try: + from tools.approval import ( + reset_current_session_key, + set_current_session_key, + ) + _approval_session_token = set_current_session_key( + self.session_id or "default" + ) + except Exception: + reset_current_session_key = None # type: ignore[assignment] + _approval_session_token = None agent_message = _voice_prefix + message if _voice_prefix else message # Prepend pending model switch note so the model knows about the switch _msn = getattr(self, '_pending_model_switch_note', None) @@ -11810,6 +11900,15 @@ class HermesCLI: set_secret_capture_callback(None) except Exception: pass + # Release the per-turn approval session key. ``_session_yolo`` + # state itself is preserved across turns (so /yolo persists + # for the whole CLI run); we just unbind the contextvar so a + # reused thread doesn't see stale identity on its next run. + if _approval_session_token is not None and reset_current_session_key is not None: + try: + reset_current_session_key(_approval_session_token) + except Exception: + pass # Start agent in background thread (daemon so it cannot keep the # process alive when the user closes the terminal tab — SIGHUP @@ -11940,6 +12039,7 @@ class HermesCLI: and getattr(self.agent, "session_id", None) and self.agent.session_id != self.session_id ): + self._transfer_session_yolo(self.session_id, self.agent.session_id) self.session_id = self.agent.session_id self._pending_title = None diff --git a/tests/cli/test_cli_yolo_toggle.py b/tests/cli/test_cli_yolo_toggle.py new file mode 100644 index 00000000000..55ee4882ee6 --- /dev/null +++ b/tests/cli/test_cli_yolo_toggle.py @@ -0,0 +1,244 @@ +"""Regression tests for the CLI ``/yolo`` in-chat toggle. + +Pre-fix bug (issue #33925): ``cli.HermesCLI._toggle_yolo`` mutated only +``os.environ["HERMES_YOLO_MODE"]``. That env var is captured once at +module-import time into ``tools.approval._YOLO_MODE_FROZEN`` (security +hardening: stops prompt-injected skills from flipping the bypass mid-run), +so the post-startup toggle was a silent no-op. ``/yolo`` advertised "YOLO ON" +in the status bar while every dangerous command still hit the approval +prompt. Only ``hermes --yolo`` (process-start env), ``HERMES_YOLO_MODE=1``, +and ``hermes config set approvals.mode off`` actually bypassed. + +The fix routes the CLI toggle through ``enable_session_yolo`` / +``disable_session_yolo`` (matching the gateway and TUI ``/yolo`` paths) and +binds ``self.session_id`` as the active approval session key around each +``run_conversation`` call so ``is_current_session_yolo_enabled()`` resolves +against the same key the toggle writes under. + +We test ``_toggle_yolo`` and ``_is_session_yolo_active`` as unbound methods +against a minimal stand-in object that exposes only the attribute they +read (``session_id``). This avoids the heavy ``HermesCLI`` construction +path used in ``test_cli_init.py``, which is incompatible with this test +file's path layout — ``HermesCLI.__init__`` imports a lot of optional +state we don't need here. +""" + +import os +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + +import tools.approval as approval_module +from cli import HermesCLI + + +SESSION_KEY = "test-cli-yolo-session" + + +@pytest.fixture(autouse=True) +def _clear_approval_state(monkeypatch): + """Clear the YOLO bypass + env var around every test so cases are independent.""" + monkeypatch.delenv("HERMES_YOLO_MODE", raising=False) + approval_module.clear_session(SESSION_KEY) + approval_module.clear_session("default") + yield + approval_module.clear_session(SESSION_KEY) + approval_module.clear_session("default") + + +def _make_stand_in(session_id: str = SESSION_KEY) -> SimpleNamespace: + """Minimal stand-in exposing only ``session_id``. + + ``_toggle_yolo`` and ``_is_session_yolo_active`` are both pure methods + that only read ``self.session_id`` — no other CLI state is touched. + Calling them as unbound functions against this stand-in is equivalent + to invoking them on a fully-constructed ``HermesCLI`` for the + behaviour under test, and avoids the brittle prompt_toolkit / config + stubbing required to instantiate ``HermesCLI`` from this test file. + """ + return SimpleNamespace(session_id=session_id) + + +class TestToggleYoloIsSessionScoped: + """The CLI /yolo handler must mutate the session-yolo set, not the env var. + + The env var path is dead-on-arrival because ``_YOLO_MODE_FROZEN`` is + captured once at module import, long before the CLI's ``/yolo`` command + can run. + """ + + def test_toggle_yolo_enables_session_bypass(self): + stand_in = _make_stand_in() + + assert approval_module.is_session_yolo_enabled(SESSION_KEY) is False + + with patch("cli._cprint"): + HermesCLI._toggle_yolo(stand_in) + + assert approval_module.is_session_yolo_enabled(SESSION_KEY) is True + + def test_toggle_yolo_disables_session_bypass_on_second_call(self): + stand_in = _make_stand_in() + with patch("cli._cprint"): + HermesCLI._toggle_yolo(stand_in) # ON + assert approval_module.is_session_yolo_enabled(SESSION_KEY) is True + HermesCLI._toggle_yolo(stand_in) # OFF + assert approval_module.is_session_yolo_enabled(SESSION_KEY) is False + + def test_toggle_yolo_does_not_mutate_env_var(self): + """Toggling /yolo must not write ``HERMES_YOLO_MODE`` — that path is + frozen at import time and would mislead anyone reading the env later + (subprocesses, status bars wired to the env, the relaunch flag list).""" + stand_in = _make_stand_in() + with patch("cli._cprint"): + HermesCLI._toggle_yolo(stand_in) + + assert os.environ.get("HERMES_YOLO_MODE") is None + + def test_toggle_yolo_falls_back_to_default_when_session_id_missing(self): + """An edge case during CLI bootstrap: a ``/yolo`` triggered before the + session id is set should not blow up, and should land under the + ``default`` session key so the bypass still takes effect for any code + that resolves against the default key.""" + stand_in = _make_stand_in(session_id="") + with patch("cli._cprint"): + HermesCLI._toggle_yolo(stand_in) + + assert approval_module.is_session_yolo_enabled("default") is True + + def test_two_independent_sessions_are_isolated(self): + """``/yolo`` toggled in one session must not bypass approvals in + another session — mirrors the gateway-side invariant.""" + cli_a = _make_stand_in(session_id="session-yolo-a") + cli_b = _make_stand_in(session_id="session-yolo-b") + + try: + with patch("cli._cprint"): + HermesCLI._toggle_yolo(cli_a) + + assert approval_module.is_session_yolo_enabled("session-yolo-a") is True + assert approval_module.is_session_yolo_enabled("session-yolo-b") is False + finally: + approval_module.clear_session("session-yolo-a") + approval_module.clear_session("session-yolo-b") + + +class TestIsSessionYoloActiveHelper: + """The status-bar helper must read the live session-yolo state, not the + env var (which is the bug class this PR fixes).""" + + def test_helper_reflects_toggle(self): + stand_in = _make_stand_in() + + assert HermesCLI._is_session_yolo_active(stand_in) is False + + with patch("cli._cprint"): + HermesCLI._toggle_yolo(stand_in) + + assert HermesCLI._is_session_yolo_active(stand_in) is True + + with patch("cli._cprint"): + HermesCLI._toggle_yolo(stand_in) + + assert HermesCLI._is_session_yolo_active(stand_in) is False + + def test_helper_honors_frozen_yolo_mode(self): + """``hermes --yolo`` sets ``HERMES_YOLO_MODE`` before tool imports, so + ``_YOLO_MODE_FROZEN`` ends up True. The status bar should still + reflect YOLO on in that case even when the session toggle is off.""" + stand_in = _make_stand_in() + + with patch.object(approval_module, "_YOLO_MODE_FROZEN", True): + assert HermesCLI._is_session_yolo_active(stand_in) is True + + +class TestToggleYoloEndToEnd: + """End-to-end: a dangerous command must auto-approve through the same + ``check_all_command_guards`` path the terminal tool uses.""" + + def test_toggle_yolo_bypasses_dangerous_command_check(self): + stand_in = _make_stand_in() + + token = approval_module.set_current_session_key(SESSION_KEY) + try: + with patch("cli._cprint"): + HermesCLI._toggle_yolo(stand_in) # YOLO ON + + result = approval_module.check_all_command_guards( + "rm -rf /tmp/scratch-xyzzy", "local", + ) + assert result["approved"] is True, ( + f"YOLO toggle should auto-approve dangerous commands, got: {result}" + ) + finally: + approval_module.reset_current_session_key(token) + + +class TestIsSessionYoloActiveAttrSafety: + """The status-bar helper runs against partially-constructed CLI fixtures + (tests use ``HermesCLI.__new__(HermesCLI)`` to skip ``__init__``). It must + not raise ``AttributeError`` when ``session_id`` is absent — the + status-bar builders swallow exceptions silently and lose every field + after the failure, producing a regression that's hard to track back to + the helper.""" + + def test_helper_survives_missing_session_id_attr(self): + # SimpleNamespace WITHOUT session_id mimics __new__-built fixtures. + from types import SimpleNamespace + no_attr = SimpleNamespace() + # Must return False, not raise. + assert HermesCLI._is_session_yolo_active(no_attr) is False + + +class TestSessionRotationTransfersYolo: + """When the CLI's ``session_id`` rotates mid-run (``/branch``, auto + compression continuation), YOLO state keyed under the old id must move + to the new id. Otherwise the user's ``/yolo ON`` silently reverts on + the next turn — the same UX failure mode this PR set out to fix. + Mirrors ``tui_gateway/server.py`` ~line 1297-1305.""" + + def test_transfer_moves_yolo_to_new_session(self): + stand_in = _make_stand_in(session_id="old-id") + try: + approval_module.enable_session_yolo("old-id") + assert approval_module.is_session_yolo_enabled("old-id") is True + + HermesCLI._transfer_session_yolo(stand_in, "old-id", "new-id") + + assert approval_module.is_session_yolo_enabled("new-id") is True + assert approval_module.is_session_yolo_enabled("old-id") is False + finally: + approval_module.clear_session("old-id") + approval_module.clear_session("new-id") + + def test_transfer_is_noop_when_yolo_was_off(self): + stand_in = _make_stand_in(session_id="old-id") + try: + HermesCLI._transfer_session_yolo(stand_in, "old-id", "new-id") + assert approval_module.is_session_yolo_enabled("new-id") is False + assert approval_module.is_session_yolo_enabled("old-id") is False + finally: + approval_module.clear_session("old-id") + approval_module.clear_session("new-id") + + def test_transfer_is_noop_when_ids_match(self): + stand_in = _make_stand_in(session_id="same-id") + try: + approval_module.enable_session_yolo("same-id") + HermesCLI._transfer_session_yolo(stand_in, "same-id", "same-id") + # Must NOT have been disabled — same-id == same-id is a no-op, + # not a "disable then re-enable" round-trip. + assert approval_module.is_session_yolo_enabled("same-id") is True + finally: + approval_module.clear_session("same-id") + + def test_transfer_handles_empty_inputs_safely(self): + stand_in = _make_stand_in(session_id="x") + # Both directions of empty input should be safe no-ops; nothing + # to transfer from "" / to "". + HermesCLI._transfer_session_yolo(stand_in, "", "new") + HermesCLI._transfer_session_yolo(stand_in, "old", "") + # Neither key should have been touched. + assert approval_module.is_session_yolo_enabled("new") is False + assert approval_module.is_session_yolo_enabled("old") is False