From e875ce352985a6acc8c9d5721e91db4eefe15f5a Mon Sep 17 00:00:00 2001 From: DeliberateEnsemble Date: Wed, 9 Sep 2026 20:25:59 -0400 Subject: [PATCH] fix(doctor): detect disabled Claude hooks --- src/doberman/cli/doctor.py | 11 +++++ src/doberman/hosthooks/integrity.py | 34 +++++++++++++++- tests/unit/test_cli_doctor.py | 62 +++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 2 deletions(-) diff --git a/src/doberman/cli/doctor.py b/src/doberman/cli/doctor.py index e5377084..ce5e3c12 100644 --- a/src/doberman/cli/doctor.py +++ b/src/doberman/cli/doctor.py @@ -161,8 +161,19 @@ def _check_hook_integrity(path: str) -> CheckResult: name = "Hook integrity" statuses = check_all(path) + disabled = [s for s in statuses if s.disabled] diverged = [s for s in statuses if s.state == "diverged"] intact = [s for s in statuses if s.state == "intact"] + if disabled: + where = ", ".join(f"{s.host} {s.scope}" for s in disabled) + return CheckResult( + name, + CheckStatus.FAIL, + f"disabled ({where}) - Claude Code's top-level `disableAllHooks: true` " + "skips every hook; Doberman is not gating calls. Remove the setting and " + "re-run `doberman doctor`", + critical=True, + ) if diverged: where = ", ".join(f"{s.host} {s.scope}: {'/'.join(s.diverged_events)}" for s in diverged) critical = any(s.critical for s in diverged) diff --git a/src/doberman/hosthooks/integrity.py b/src/doberman/hosthooks/integrity.py index 0d520f64..b8a69b76 100644 --- a/src/doberman/hosthooks/integrity.py +++ b/src/doberman/hosthooks/integrity.py @@ -26,7 +26,7 @@ import hmac import json import os -from dataclasses import dataclass +from dataclasses import dataclass, replace from datetime import datetime, timezone from pathlib import Path from typing import Any @@ -64,6 +64,8 @@ class IntegrityStatus: critical: bool = False #: ISO timestamp of the last divergence noted for this entry, if any. divergence_seen: str | None = None + #: True when an applicable Claude settings file disables all hooks. + disabled: bool = False def manifest_path() -> Path: @@ -279,19 +281,47 @@ def _live_groups( return path, codex_doberman_groups(load_settings(path)) +def _claude_hooks_disabled(settings_path: Path) -> bool: + """Return whether this Claude settings file disables all hooks. + + The setting is intentionally matched as the JSON boolean ``true`` only. + Reading is best effort so a malformed or unreadable settings file retains + the existing integrity behavior and never breaks ``doctor``. + """ + try: + from doberman.hosthooks.install import load_settings + + return load_settings(settings_path).get("disableAllHooks") is True + except Exception: # noqa: BLE001 - diagnostics must never raise on bad settings + return False + + def check_all(project_root: str) -> list[IntegrityStatus]: """Verify every tracked scope for *project_root*. Never raises. One status per ``(host, scope)`` in :data:`_SCOPES`; a scope whose settings - file is unreadable yields ``absent`` rather than raising. + file is unreadable yields ``absent`` rather than raising. A true Claude + ``disableAllHooks`` setting is reported separately through ``disabled`` + without changing the registration state. """ out: list[IntegrityStatus] = [] + active_claude_indexes: list[int] = [] + claude_hooks_disabled = False for host, scope in _SCOPES: try: path, groups = _live_groups(host, scope, project_root) out.append(verify_install(host, scope, path, groups)) + if host == "claude": + if groups: + active_claude_indexes.append(len(out) - 1) + claude_hooks_disabled = claude_hooks_disabled or _claude_hooks_disabled(path) except Exception: # noqa: BLE001 - an unreadable settings file is "absent", never a crash out.append(IntegrityStatus(host, scope, "absent")) + if claude_hooks_disabled: + out = [ + replace(status, disabled=True) if index in active_claude_indexes else status + for index, status in enumerate(out) + ] return out diff --git a/tests/unit/test_cli_doctor.py b/tests/unit/test_cli_doctor.py index dbddca0b..a428ec4e 100644 --- a/tests/unit/test_cli_doctor.py +++ b/tests/unit/test_cli_doctor.py @@ -600,6 +600,68 @@ def test_doctor_integrity_intact(integrity_env: Path) -> None: assert "claude project" in r.detail +def test_doctor_integrity_fails_when_claude_hooks_are_disabled(integrity_env: Path) -> None: + assert CliRunner().invoke(app, ["install-hooks", "--path", str(integrity_env)]).exit_code == 0 + settings_path = resolve_settings_path("project", str(integrity_env)) + data = _json.loads(settings_path.read_text(encoding="utf-8")) + data["disableAllHooks"] = True + settings_path.write_text(_json.dumps(data), encoding="utf-8") + + r = _integrity(run_checks(str(integrity_env))) + status = next( + s + for s in integrity.check_all(str(integrity_env)) + if s.host == "claude" and s.scope == "project" + ) + + assert r.status is CheckStatus.FAIL + assert r.critical is True + assert "disableAllHooks" in r.detail + assert status.state == "intact" + assert status.disabled is True + + +@pytest.mark.parametrize("value", [False, "true", 1]) +def test_doctor_integrity_ignores_non_boolean_true_disable_setting( + integrity_env: Path, value +) -> None: + assert CliRunner().invoke(app, ["install-hooks", "--path", str(integrity_env)]).exit_code == 0 + settings_path = resolve_settings_path("project", str(integrity_env)) + data = _json.loads(settings_path.read_text(encoding="utf-8")) + data["disableAllHooks"] = value + settings_path.write_text(_json.dumps(data), encoding="utf-8") + + r = _integrity(run_checks(str(integrity_env))) + + assert r.status is CheckStatus.OK + assert "intact" in r.detail + + +def test_doctor_integrity_detects_disable_setting_in_merged_claude_scope( + integrity_env: Path, +) -> None: + assert CliRunner().invoke(app, ["install-hooks", "--path", str(integrity_env)]).exit_code == 0 + global_settings_path = resolve_settings_path("global", str(integrity_env)) + global_settings_path.parent.mkdir(parents=True, exist_ok=True) + global_settings_path.write_text('{"disableAllHooks": true}', encoding="utf-8") + + r = _integrity(run_checks(str(integrity_env))) + + assert r.status is CheckStatus.FAIL + assert r.critical is True + assert "disableAllHooks" in r.detail + + +def test_doctor_integrity_does_not_crash_on_malformed_settings(integrity_env: Path) -> None: + assert CliRunner().invoke(app, ["install-hooks", "--path", str(integrity_env)]).exit_code == 0 + settings_path = resolve_settings_path("project", str(integrity_env)) + settings_path.write_text("not json", encoding="utf-8") + + r = _integrity(run_checks(str(integrity_env))) + + assert isinstance(r.status, CheckStatus) + + def test_doctor_integrity_critical_divergence(integrity_env: Path) -> None: assert CliRunner().invoke(app, ["install-hooks", "--path", str(integrity_env)]).exit_code == 0 settings_path = resolve_settings_path("project", str(integrity_env))