fix(shell_hooks): parse hooks_auto_accept as strict bool/string, not bool() (#16322)

`_resolve_effective_accept()` used `return bool(cfg_val)` for the
`hooks_auto_accept` config key. In Python, `bool("false")` is `True`,
so a user setting `hooks_auto_accept: "false"` (quoted YAML string)
in `config.yaml` would silently enable auto-approval of every shell
hook, bypassing the consent prompt entirely.

Replace the coercion with the same type-aware parsing already used for
the HERMES_ACCEPT_HOOKS env var three lines above: bool passthrough,
strings checked against {1,true,yes,on} case-insensitively, everything
else (including "false", None, 0, ints) rejected.

Add TestHooksAutoAcceptParsing guarding the regression across all four
value shapes (bool, string-truthy, string-falsy, missing/None).

Reported by @sprmn24 in #16244.
This commit is contained in:
Teknium
2026-04-26 20:48:35 -07:00
committed by GitHub
parent 6993e566ba
commit e19854d893
2 changed files with 76 additions and 1 deletions

View File

@@ -240,3 +240,74 @@ class TestAllowlistOps:
and e.get("command") == str(script)
]
assert len(matching) == 1
# ── hooks_auto_accept config parsing ──────────────────────────────────────
class TestHooksAutoAcceptParsing:
"""Regression guard: YAML-string values must not silently auto-accept.
``bool("false")`` is ``True`` in Python, so the old ``return bool(cfg_val)``
path treated ``hooks_auto_accept: "false"`` (quoted YAML string) as a
truthy opt-in, silently bypassing user consent for every shell hook.
"""
def test_bool_true_accepts(self):
assert shell_hooks._resolve_effective_accept(
{"hooks_auto_accept": True}, accept_hooks_arg=False,
) is True
def test_bool_false_rejects(self):
assert shell_hooks._resolve_effective_accept(
{"hooks_auto_accept": False}, accept_hooks_arg=False,
) is False
def test_string_false_rejects(self):
# The bug: bool("false") is True. Must be parsed, not coerced.
assert shell_hooks._resolve_effective_accept(
{"hooks_auto_accept": "false"}, accept_hooks_arg=False,
) is False
def test_string_no_rejects(self):
assert shell_hooks._resolve_effective_accept(
{"hooks_auto_accept": "no"}, accept_hooks_arg=False,
) is False
def test_string_true_accepts(self):
assert shell_hooks._resolve_effective_accept(
{"hooks_auto_accept": "true"}, accept_hooks_arg=False,
) is True
def test_string_true_case_insensitive(self):
assert shell_hooks._resolve_effective_accept(
{"hooks_auto_accept": " TRUE "}, accept_hooks_arg=False,
) is True
def test_string_yes_on_one_accept(self):
for val in ("yes", "on", "1"):
assert shell_hooks._resolve_effective_accept(
{"hooks_auto_accept": val}, accept_hooks_arg=False,
) is True, val
def test_missing_key_rejects(self):
assert shell_hooks._resolve_effective_accept(
{}, accept_hooks_arg=False,
) is False
def test_none_rejects(self):
assert shell_hooks._resolve_effective_accept(
{"hooks_auto_accept": None}, accept_hooks_arg=False,
) is False
def test_integer_ignored(self):
# Only bool and str are honored; anything else (including 1) is False.
assert shell_hooks._resolve_effective_accept(
{"hooks_auto_accept": 1}, accept_hooks_arg=False,
) is False
def test_cli_arg_overrides_config(self):
assert shell_hooks._resolve_effective_accept(
{"hooks_auto_accept": "false"}, accept_hooks_arg=True,
) is True