refactor(approval): extract is_approval_bypass_active(); use frozen-env bypass in codex routing
Self-review follow-up on the salvaged approval-routing fix.
The initial adaptation re-read os.getenv("HERMES_YOLO_MODE") at session-build
time. That diverges from the repo's security invariant: HERMES_YOLO_MODE is
frozen into tools.approval._YOLO_MODE_FROZEN at import time precisely so a skill
running mid-process cannot set the env var and instantly flip the approval
bypass (a prompt-injection escalation path). A live re-read re-opened that hole
for the codex routing path.
- Add tools.approval.is_approval_bypass_active() — the canonical three-source
bypass check (frozen --yolo/HERMES_YOLO_MODE + session /yolo + approvals.mode
off) in one place. This is the 4th inline copy of that OR-chain (the three
sites in approval.py and tui_gateway/server.py:3121 all use the same idiom);
the helper is the shared chokepoint they can collapse onto.
- codex_runtime.py now calls is_approval_bypass_active() instead of the
hand-rolled mode-or-session check plus a runtime env re-read.
- Update the env-yolo test to patch _YOLO_MODE_FROZEN (the canonical test
pattern, e.g. tests/tools/test_yolo_mode.py) rather than setenv, which is
dead-on-arrival against the frozen constant.
Fail-closed default preserved on every branch; 28 integration + 77 session/yolo
tests pass; E2E confirms the real exec decision flips decline->accept only when
bypass is active.
This commit is contained in:
parent
0b8e81996f
commit
b23e1c3077
3 changed files with 34 additions and 20 deletions
|
|
@ -269,32 +269,22 @@ def run_codex_app_server_turn(
|
|||
# requests through, so codex app-server exec / apply_patch requests
|
||||
# fail closed (silently decline) by default. When the user has
|
||||
# explicitly opted out of Hermes approvals — via `approvals.mode: off`
|
||||
# in config, the /yolo session toggle, or HERMES_YOLO_MODE=1 — honor
|
||||
# that and let codex's own sandbox permission profile
|
||||
# in config, the /yolo session toggle, or --yolo / HERMES_YOLO_MODE —
|
||||
# honor that and let codex's own sandbox permission profile
|
||||
# (~/.codex/config.toml) be the policy gate instead of double-gating
|
||||
# with a missing Hermes UI. Defaults (manual/smart/unset) preserve the
|
||||
# current fail-closed behavior — this is a no-op for those users.
|
||||
auto_approve_requests = False
|
||||
try:
|
||||
from tools.approval import (
|
||||
_get_approval_mode,
|
||||
is_current_session_yolo_enabled,
|
||||
)
|
||||
from tools.approval import is_approval_bypass_active
|
||||
|
||||
auto_approve_requests = (
|
||||
_get_approval_mode() == "off"
|
||||
or is_current_session_yolo_enabled()
|
||||
)
|
||||
auto_approve_requests = is_approval_bypass_active()
|
||||
except Exception:
|
||||
logger.debug(
|
||||
"codex app-server: approval-mode lookup failed; "
|
||||
"codex app-server: approval-bypass lookup failed; "
|
||||
"keeping fail-closed default",
|
||||
exc_info=True,
|
||||
)
|
||||
if not auto_approve_requests:
|
||||
auto_approve_requests = os.getenv(
|
||||
"HERMES_YOLO_MODE", ""
|
||||
).strip().lower() in {"1", "true", "yes", "on"}
|
||||
|
||||
def _on_codex_event(note: dict) -> None:
|
||||
# Bridge Codex app-server item/started notifications to Hermes
|
||||
|
|
|
|||
|
|
@ -410,14 +410,17 @@ class TestRunConversationCodexPath:
|
|||
assert routing.auto_approve_exec is False
|
||||
assert routing.auto_approve_apply_patch is False
|
||||
|
||||
def test_hermes_yolo_env_auto_approves_codex_server_requests(
|
||||
def test_frozen_yolo_env_auto_approves_codex_server_requests(
|
||||
self, monkeypatch
|
||||
):
|
||||
"""HERMES_YOLO_MODE should flow through to codex app-server routing so
|
||||
gateway/cron contexts do not fail closed when the user explicitly
|
||||
enabled yolo mode outside the CLI slash-command path."""
|
||||
"""--yolo / HERMES_YOLO_MODE (frozen into _YOLO_MODE_FROZEN at import
|
||||
time — a prompt-injection-safe process-scoped bypass) should flow
|
||||
through to codex app-server routing so gateway/cron contexts do not
|
||||
fail closed when the user launched with yolo mode."""
|
||||
import tools.approval as _approval
|
||||
|
||||
captured = self._capture_routing_agent(monkeypatch)
|
||||
monkeypatch.setenv("HERMES_YOLO_MODE", "1")
|
||||
monkeypatch.setattr(_approval, "_YOLO_MODE_FROZEN", True)
|
||||
with patch(
|
||||
"hermes_cli.config.load_config",
|
||||
return_value={"approvals": {"mode": "manual"}},
|
||||
|
|
|
|||
|
|
@ -1770,6 +1770,27 @@ def _get_approval_mode() -> str:
|
|||
return _normalize_approval_mode(mode)
|
||||
|
||||
|
||||
def is_approval_bypass_active() -> bool:
|
||||
"""Return True when the user has opted out of Hermes approval prompts.
|
||||
|
||||
Collapses the canonical three-source bypass check used across the codebase
|
||||
into one place:
|
||||
- process-scoped ``--yolo`` / ``HERMES_YOLO_MODE`` (frozen at import time
|
||||
so a mid-process skill can't flip it — a prompt-injection escalation
|
||||
path; see ``_YOLO_MODE_FROZEN`` above),
|
||||
- the session-scoped gateway ``/yolo`` toggle,
|
||||
- ``approvals.mode: off`` in config.
|
||||
|
||||
This is the pure-bypass sub-expression only. Callers that also honor a
|
||||
hardline blocklist / permanent allowlist must check those separately.
|
||||
"""
|
||||
return (
|
||||
_YOLO_MODE_FROZEN
|
||||
or is_current_session_yolo_enabled()
|
||||
or _get_approval_mode() == "off"
|
||||
)
|
||||
|
||||
|
||||
def _get_approval_timeout() -> int:
|
||||
"""Read the approval timeout from config. Defaults to 60 seconds."""
|
||||
try:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue