fix(approval): honor bare YAML approvals.mode: off (#2620)
Cherry-picked from PR #2563 by tumf. YAML 1.1 parses unquoted 'off' as boolean False. Added _normalize_approval_mode() to map False -> 'off', True -> 'manual', and normalize string values. Includes regression tests.
This commit is contained in:
parent
d35df0db71
commit
7da0822456
2 changed files with 28 additions and 1 deletions
|
|
@ -4,6 +4,7 @@ from unittest.mock import patch as mock_patch
|
||||||
|
|
||||||
import tools.approval as approval_module
|
import tools.approval as approval_module
|
||||||
from tools.approval import (
|
from tools.approval import (
|
||||||
|
_get_approval_mode,
|
||||||
approve_session,
|
approve_session,
|
||||||
clear_session,
|
clear_session,
|
||||||
detect_dangerous_command,
|
detect_dangerous_command,
|
||||||
|
|
@ -16,6 +17,16 @@ from tools.approval import (
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class TestApprovalModeParsing:
|
||||||
|
def test_unquoted_yaml_off_boolean_false_maps_to_off(self):
|
||||||
|
with mock_patch("hermes_cli.config.load_config", return_value={"approvals": {"mode": False}}):
|
||||||
|
assert _get_approval_mode() == "off"
|
||||||
|
|
||||||
|
def test_string_off_still_maps_to_off(self):
|
||||||
|
with mock_patch("hermes_cli.config.load_config", return_value={"approvals": {"mode": "off"}}):
|
||||||
|
assert _get_approval_mode() == "off"
|
||||||
|
|
||||||
|
|
||||||
class TestDetectDangerousRm:
|
class TestDetectDangerousRm:
|
||||||
def test_rm_rf_detected(self):
|
def test_rm_rf_detected(self):
|
||||||
is_dangerous, key, desc = detect_dangerous_command("rm -rf /home/user")
|
is_dangerous, key, desc = detect_dangerous_command("rm -rf /home/user")
|
||||||
|
|
|
||||||
|
|
@ -280,12 +280,28 @@ def prompt_dangerous_approval(command: str, description: str,
|
||||||
sys.stdout.flush()
|
sys.stdout.flush()
|
||||||
|
|
||||||
|
|
||||||
|
def _normalize_approval_mode(mode) -> str:
|
||||||
|
"""Normalize approval mode values loaded from YAML/config.
|
||||||
|
|
||||||
|
YAML 1.1 treats bare words like `off` as booleans, so a config entry like
|
||||||
|
`approvals:\n mode: off` is parsed as False unless quoted. Treat that as the
|
||||||
|
intended string mode instead of falling back to manual approvals.
|
||||||
|
"""
|
||||||
|
if isinstance(mode, bool):
|
||||||
|
return "off" if mode is False else "manual"
|
||||||
|
if isinstance(mode, str):
|
||||||
|
normalized = mode.strip().lower()
|
||||||
|
return normalized or "manual"
|
||||||
|
return "manual"
|
||||||
|
|
||||||
|
|
||||||
def _get_approval_mode() -> str:
|
def _get_approval_mode() -> str:
|
||||||
"""Read the approval mode from config. Returns 'manual', 'smart', or 'off'."""
|
"""Read the approval mode from config. Returns 'manual', 'smart', or 'off'."""
|
||||||
try:
|
try:
|
||||||
from hermes_cli.config import load_config
|
from hermes_cli.config import load_config
|
||||||
config = load_config()
|
config = load_config()
|
||||||
return config.get("approvals", {}).get("mode", "manual")
|
mode = config.get("approvals", {}).get("mode", "manual")
|
||||||
|
return _normalize_approval_mode(mode)
|
||||||
except Exception:
|
except Exception:
|
||||||
return "manual"
|
return "manual"
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue