diff --git a/CHANGELOG.md b/CHANGELOG.md index b1268c1c..c96b5ee0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,19 @@ PyPI version — not the changelog header. ## [2026-07-21] +**fix(hooks)** — two DPLAN-0253 backlog hardenings (DPLAN-0256 clear): +engine handler timeout + presence_gate PID-reuse defense. `_run_handler` now +runs handler-type hooks on a daemon thread joined with the hooks.json +`timeout` field (default 30s) — a hung handler returns TIMEOUT loud +(engine.jsonl + sound) and the event moves on; daemon thread chosen over +ThreadPoolExecutor so a stuck orphan can never hang interpreter exit. +presence_gate occupancy no longer trusts `os.kill(pid, 0)` alone: +`procStart` (CC session file) is matched against `/proc//stat` field 22 +so a kernel-recycled PID can't impersonate a dead session — closes the gap +before observe-only ever flips to enforcement. Missing procStart / non-Linux +falls back to liveness-only, logged. 15 new tests, suite 1272 green, +seedgo 31/31 both files. Built by @hooks. + **fix(trigger)** — runaway-log alerts get the 24h TTL every other mute already had (DPLAN-0256 backlog clear): `_write_alert()` hardcoded `expires_at: None`, so alerts.json entries nagged forever while medic branch mutes self-expired. diff --git a/src/aipass/hooks/apps/handlers/security/presence_gate.py b/src/aipass/hooks/apps/handlers/security/presence_gate.py index 8597415d..3dfb116e 100644 --- a/src/aipass/hooks/apps/handlers/security/presence_gate.py +++ b/src/aipass/hooks/apps/handlers/security/presence_gate.py @@ -1,11 +1,11 @@ # =================== AIPass ==================== # Name: presence_gate.py -# Version: 3.0.0 +# Version: 3.0.1 # Description: Single-session gate — blocks duplicate Claude runtimes per branch # Branch: hooks # Layer: apps/handlers/security # Created: 2026-06-29 -# Modified: 2026-07-13 +# Modified: 2026-07-21 # ============================================= """Single-session gate — blocks duplicate Claude runtimes per branch. @@ -27,6 +27,12 @@ including background sessions with agent_type "claude". Ships in OBSERVE-ONLY mode: logs would-block decisions to engine.jsonl but never actually blocks. Flip _OBSERVE_ONLY to False after soak period confirms zero false positives. + +Occupant PID claims are identity-checked, not just liveness-checked: +cc_sessions.find_live_for_cwd() cross-verifies each session's recorded +procStart against the live process's actual start time before treating +a PID as a genuine occupant, so a PID recycled after the original +session died can never satisfy a stale claim. """ import importlib diff --git a/src/aipass/hooks/apps/modules/cc_sessions.py b/src/aipass/hooks/apps/modules/cc_sessions.py index 77be13c3..47fcc144 100644 --- a/src/aipass/hooks/apps/modules/cc_sessions.py +++ b/src/aipass/hooks/apps/modules/cc_sessions.py @@ -1,11 +1,11 @@ # =================== AIPass ==================== # Name: cc_sessions.py -# Version: 3.0.0 +# Version: 3.1.0 # Description: CC-native session discovery, listing, and reclaim # Branch: hooks # Layer: apps/modules # Created: 2026-06-30 -# Modified: 2026-07-14 +# Modified: 2026-07-21 # ============================================= """Read Claude Code native session files (~/.claude/sessions/.json). @@ -68,6 +68,50 @@ def _pid_alive_windows(pid: int) -> bool: kernel32.CloseHandle(handle) +def _proc_start_ticks(pid: int) -> str | None: + """Read a live process's start time (field 22 of /proc//stat), in + clock ticks since boot. Linux only — returns None elsewhere or on any + read failure, so callers can fall back to liveness-only checking. + + This is the identity half of the occupant check: os.kill(pid, 0) proves + *a* process holds this PID right now, not that it's the *same* process + that wrote the session file. A PID recycled by the kernel after the + original session died would otherwise pass silently. + """ + if sys.platform != "linux": + return None + try: + raw = Path(f"/proc/{pid}/stat").read_text(encoding="utf-8") + # comm (field 2) is parenthesized and may itself contain ')' or + # whitespace — split on the LAST ')' to get past it safely. + fields_after_comm = raw.rsplit(")", 1)[1].split() + return fields_after_comm[19] # field 22 overall, starttime + except (OSError, IndexError) as exc: + logger.info("[CC_SESSIONS] Cannot read /proc/%d/stat: %s", pid, exc) + return None + + +def _session_pid_matches(session: dict) -> bool: + """Verify the live PID is still the same process that wrote this session + file — not an unrelated process that later reused the same PID number. + + Compares the session's recorded procStart (written by CC at session + start) against the live process's current start time. Sessions without + a recorded procStart (older/malformed files) or non-Linux hosts can't be + checked — falls back to liveness-only, the pre-hardening behavior. + """ + recorded = session.get("procStart") + if not recorded: + return True + pid = session.get("pid") + if not isinstance(pid, int): + return True + live_start = _proc_start_ticks(pid) + if live_start is None: + return True + return str(live_start) == str(recorded) + + def _is_pid_alive(pid: int) -> bool: """Check if a process with the given PID exists.""" if pid <= 1: @@ -146,7 +190,9 @@ def find_live_for_cwd(cwd: str) -> list[dict]: """Find all live CC sessions whose cwd matches the given directory. Compares resolved paths for robustness (symlinks, trailing slashes). - Only returns sessions whose PID is still alive. + Only returns sessions whose PID is both alive and identity-checked + against procStart, so a PID recycled after the original session died + is never mistaken for a live occupant. """ target = str(Path(cwd).resolve()) live = [] @@ -157,14 +203,21 @@ def find_live_for_cwd(cwd: str) -> list[dict]: if str(Path(session_cwd).resolve()) != target: continue pid = session.get("pid") - if pid and _is_pid_alive(pid): - live.append(session) - else: + if not pid or not _is_pid_alive(pid): logger.info( "[CC_SESSIONS] Stale session file for PID %s at %s", pid, session_cwd, ) + continue + if not _session_pid_matches(session): + logger.warning( + "[CC_SESSIONS] PID %s reused (procStart mismatch) — treating stale session as dead at %s", + pid, + session_cwd, + ) + continue + live.append(session) return live diff --git a/src/aipass/hooks/apps/modules/engine.py b/src/aipass/hooks/apps/modules/engine.py index af1133c3..379cd634 100644 --- a/src/aipass/hooks/apps/modules/engine.py +++ b/src/aipass/hooks/apps/modules/engine.py @@ -15,6 +15,7 @@ import json import os import subprocess import tempfile +import threading import time from pathlib import Path @@ -61,8 +62,14 @@ def _run_hook(hook_cmd: str, stdin_data: str, timeout_s: int = 30) -> dict: return {"exit_code": -1, "stdout": "", "stderr": str(exc), "elapsed_ms": round(elapsed_ms, 1)} -def _run_handler(handler_path: str, hook_data: dict) -> dict: - """Call a handler function directly (no subprocess). Module imports handler.""" +def _run_handler(handler_path: str, hook_data: dict, timeout_s: int = 30) -> dict: + """Call a handler function directly (no subprocess). Module imports handler. + + Runs the handler on a daemon thread and joins with a timeout so a hung + handler can never stall the event — the calling thread returns control + on expiry instead of blocking forever. The orphaned thread (if any) is + left to die with the process; it never blocks interpreter exit. + """ start = time.monotonic() try: module_path, func_name = handler_path.rsplit(".", 1) @@ -80,7 +87,34 @@ def _run_handler(handler_path: str, hook_data: dict) -> dict: } module = importlib.import_module(module_path) handler_func = getattr(module, func_name) - result = handler_func(hook_data) + + outcome = {} + + def _call(): + try: + outcome["result"] = handler_func(hook_data) + except Exception as exc: + logger.info("[HOOKS] handler %s raised on worker thread: %s", handler_path, exc) + outcome["error"] = exc # re-raised on the calling thread below + + worker = threading.Thread(target=_call, daemon=True) + worker.start() + worker.join(timeout_s) + + if worker.is_alive(): + elapsed_ms = (time.monotonic() - start) * 1000 + logger.error("[HOOKS] handler timeout after %ds: %s", timeout_s, handler_path) + return { + "exit_code": -1, + "stdout": "", + "stderr": "TIMEOUT", + "elapsed_ms": round(elapsed_ms, 1), + } + + if "error" in outcome: + raise outcome["error"] + + result = outcome["result"] elapsed_ms = (time.monotonic() - start) * 1000 return { "exit_code": result.get("exit_code", 0), @@ -257,12 +291,37 @@ def dispatch(event_type: str, stdin_data: str, config: dict) -> tuple[str, int]: ) continue + hook_timeout = hook_def.get("timeout", 30) if handler: - result = _run_handler(handler, parsed) + result = _run_handler(handler, parsed, timeout_s=hook_timeout) else: - hook_timeout = hook_def.get("timeout", 30) result = _run_hook(command, stdin_data, timeout_s=hook_timeout) + if result.get("stderr") == "TIMEOUT": + logger.error( + "[HOOKS] %s.%s TIMED OUT after %ds — never silently swallowed", + event_type, + hook_name, + hook_timeout, + ) + _log( + { + "ts": time.time(), + "event": event_type, + "hook": hook_name, + "action": "timeout", + "timeout_s": hook_timeout, + "elapsed_ms": result["elapsed_ms"], + } + ) + try: + from aipass.hooks.apps.sound import speak + + speak(f"{hook_name.replace('_', ' ')} timed out") + except Exception as exc: + logger.info("[HOOKS] sound playback failed for timeout %s.%s: %s", event_type, hook_name, exc) + continue + logger.info( "[HOOKS] %s.%s agent=%s exit=%d out=%db %dms", event_type, diff --git a/src/aipass/hooks/tests/test_cc_sessions.py b/src/aipass/hooks/tests/test_cc_sessions.py index 0d1c666c..57691188 100644 --- a/src/aipass/hooks/tests/test_cc_sessions.py +++ b/src/aipass/hooks/tests/test_cc_sessions.py @@ -29,6 +29,51 @@ class TestIsPidAlive: assert cc_sessions._is_pid_alive(42) is False +class TestProcStartTicks: + def test_current_process_returns_value_on_linux(self): + with patch("sys.platform", "linux"): + result = cc_sessions._proc_start_ticks(os.getpid()) + assert result is not None + assert result.isdigit() + + def test_matches_raw_proc_stat_field(self): + with patch("sys.platform", "linux"): + result = cc_sessions._proc_start_ticks(os.getpid()) + from pathlib import Path + + raw = Path(f"/proc/{os.getpid()}/stat").read_text(encoding="utf-8") + expected = raw.rsplit(")", 1)[1].split()[19] + assert result == expected + + def test_non_linux_returns_none(self): + with patch("sys.platform", "win32"): + assert cc_sessions._proc_start_ticks(os.getpid()) is None + + def test_missing_pid_returns_none(self): + with patch("sys.platform", "linux"): + assert cc_sessions._proc_start_ticks(999999999) is None + + +class TestSessionPidMatches: + def test_no_procstart_recorded_passes(self): + assert cc_sessions._session_pid_matches({"pid": os.getpid()}) is True + + def test_non_int_pid_passes(self): + assert cc_sessions._session_pid_matches({"pid": "not-an-int", "procStart": "123"}) is True + + def test_matching_procstart_passes(self): + with patch.object(cc_sessions, "_proc_start_ticks", return_value="11277752"): + assert cc_sessions._session_pid_matches({"pid": 123, "procStart": "11277752"}) is True + + def test_mismatched_procstart_fails(self): + with patch.object(cc_sessions, "_proc_start_ticks", return_value="99999999"): + assert cc_sessions._session_pid_matches({"pid": 123, "procStart": "11277752"}) is False + + def test_unreadable_live_start_falls_back_to_pass(self): + with patch.object(cc_sessions, "_proc_start_ticks", return_value=None): + assert cc_sessions._session_pid_matches({"pid": 123, "procStart": "11277752"}) is True + + class TestReadAllSessions: def test_reads_pid_files(self, tmp_path): session = {"pid": 1234, "sessionId": "abc", "cwd": "/tmp/branch", "kind": "interactive"} @@ -103,6 +148,47 @@ class TestFindLiveForCwd: result = cc_sessions.find_live_for_cwd("/tmp/hooks") assert result == [] + def test_excludes_reused_pid_with_mismatched_procstart(self, tmp_path): + s = { + "pid": os.getpid(), + "sessionId": "reused", + "cwd": "/tmp/hooks", + "kind": "interactive", + "procStart": "1", + } + (tmp_path / f"{os.getpid()}.json").write_text(json.dumps(s)) + with ( + patch.object(cc_sessions, "CC_SESSIONS_DIR", tmp_path), + patch.object(cc_sessions, "_proc_start_ticks", return_value="999999999"), + ): + result = cc_sessions.find_live_for_cwd("/tmp/hooks") + assert result == [] + + def test_includes_session_with_matching_procstart(self, tmp_path): + s = { + "pid": os.getpid(), + "sessionId": "genuine", + "cwd": "/tmp/hooks", + "kind": "interactive", + "procStart": "42", + } + (tmp_path / f"{os.getpid()}.json").write_text(json.dumps(s)) + with ( + patch.object(cc_sessions, "CC_SESSIONS_DIR", tmp_path), + patch.object(cc_sessions, "_proc_start_ticks", return_value="42"), + ): + result = cc_sessions.find_live_for_cwd("/tmp/hooks") + assert len(result) == 1 + assert result[0]["sessionId"] == "genuine" + + def test_includes_session_without_procstart_field(self, tmp_path): + s = {"pid": os.getpid(), "sessionId": "no-procstart", "cwd": "/tmp/hooks", "kind": "interactive"} + (tmp_path / f"{os.getpid()}.json").write_text(json.dumps(s)) + with patch.object(cc_sessions, "CC_SESSIONS_DIR", tmp_path): + result = cc_sessions.find_live_for_cwd("/tmp/hooks") + assert len(result) == 1 + assert result[0]["sessionId"] == "no-procstart" + class TestFindOccupant: def test_no_occupant_when_free(self, tmp_path): @@ -135,6 +221,17 @@ class TestFindOccupant: result = cc_sessions.find_occupant("/tmp/hooks") assert result is not None + def test_reused_pid_never_reported_as_occupant(self, tmp_path): + my_pid = os.getpid() + s = {"pid": my_pid, "sessionId": "stale-claim", "cwd": "/tmp/hooks", "kind": "interactive", "procStart": "1"} + (tmp_path / f"{my_pid}.json").write_text(json.dumps(s)) + with ( + patch.object(cc_sessions, "CC_SESSIONS_DIR", tmp_path), + patch.object(cc_sessions, "_proc_start_ticks", return_value="999999999"), + ): + result = cc_sessions.find_occupant("/tmp/hooks", exclude_pid=99999) + assert result is None + class TestReclaim: def test_reclaim_stops_live_sessions(self, tmp_path): diff --git a/src/aipass/hooks/tests/test_engine.py b/src/aipass/hooks/tests/test_engine.py index d30202a6..1d0c5a7f 100644 --- a/src/aipass/hooks/tests/test_engine.py +++ b/src/aipass/hooks/tests/test_engine.py @@ -937,6 +937,199 @@ class TestLayerATrustEnforcement: assert "ok" in result[0] +class TestRunHandlerTimeout: + """DPLAN-0256 backlog #1: handler-type hooks had no engine-side timeout — a hung + handler could stall the whole event. _run_handler now runs the handler on a + daemon thread and joins with a timeout instead of calling it inline.""" + + def test_handler_completes_within_timeout(self, mock_logger): + from aipass.hooks.apps.modules.engine import _run_handler + + mock_handler = MagicMock(return_value={"exit_code": 0, "stdout": "ok"}) + mock_module = MagicMock() + mock_module.handle = mock_handler + with patch("importlib.import_module", return_value=mock_module): + result = _run_handler("aipass.hooks.apps.handlers.notification.stop_sound.handle", {}, timeout_s=1) + assert result["exit_code"] == 0 + assert result["stdout"] == "ok" + + def test_handler_exceeding_timeout_returns_timeout_marker(self, mock_logger): + import time as time_module + + from aipass.hooks.apps.modules.engine import _run_handler + + def _slow_handler(_data): + time_module.sleep(1.3) + return {"exit_code": 0, "stdout": "too late"} + + mock_module = MagicMock() + mock_module.handle = _slow_handler + with patch("importlib.import_module", return_value=mock_module): + result = _run_handler("aipass.hooks.apps.handlers.fake.handle", {}, timeout_s=1) + assert result["exit_code"] == -1 + assert result["stderr"] == "TIMEOUT" + + def test_handler_timeout_does_not_block_caller(self, mock_logger): + """Regression: the caller must return promptly even if the handler thread never finishes.""" + import time as time_module + + from aipass.hooks.apps.modules.engine import _run_handler + + def _hangs_much_longer_than_timeout(_data): + time_module.sleep(3) + return {"exit_code": 0, "stdout": "should never see this"} + + mock_module = MagicMock() + mock_module.handle = _hangs_much_longer_than_timeout + with patch("importlib.import_module", return_value=mock_module): + start = time_module.monotonic() + result = _run_handler("aipass.hooks.apps.handlers.fake.handle", {}, timeout_s=1) + elapsed = time_module.monotonic() - start + assert elapsed < 2.0 + assert result["stderr"] == "TIMEOUT" + + def test_default_handler_timeout_is_30(self): + import inspect + + from aipass.hooks.apps.modules.engine import _run_handler + + sig = inspect.signature(_run_handler) + assert sig.parameters["timeout_s"].default == 30 + + def test_handler_exception_still_surfaces_as_error_not_timeout(self, mock_logger): + from aipass.hooks.apps.modules.engine import _run_handler + + def _boom(_data): + raise RuntimeError("handler blew up") + + mock_module = MagicMock() + mock_module.handle = _boom + with patch("importlib.import_module", return_value=mock_module): + result = _run_handler("aipass.hooks.apps.handlers.fake.handle", {}, timeout_s=1) + assert result["exit_code"] == -1 + assert "handler blew up" in result["stderr"] + assert result["stderr"] != "TIMEOUT" + + +class TestDispatchHandlerTimeout: + """Dispatch-level wiring for DPLAN-0256 backlog #1 — hooks.json timeout fields + are honored for handler-type hooks, a sane default applies when absent, and a + timeout fails loud (log + sound) without ever silently swallowing or stalling + the rest of the event.""" + + def test_handler_timeout_propagates_hook_def_value(self, mock_logger): + config = { + "hooks_enabled": True, + "UserPromptSubmit": { + "slow_handler": { + "enabled": True, + "handler": "aipass.hooks.apps.handlers.fake.handle", + "matcher": "", + "timeout": 5, + } + }, + } + with ( + patch("aipass.hooks.apps.modules.engine._log"), + patch("aipass.hooks.apps.modules.engine._run_handler") as mock_run, + ): + mock_run.return_value = {"exit_code": 0, "stdout": "ok", "stderr": "", "elapsed_ms": 5} + dispatch("UserPromptSubmit", "{}", config) + mock_run.assert_called_once_with("aipass.hooks.apps.handlers.fake.handle", {}, timeout_s=5) + + def test_handler_default_timeout_is_30_when_unset(self, mock_logger): + config = { + "hooks_enabled": True, + "UserPromptSubmit": { + "no_timeout_handler": { + "enabled": True, + "handler": "aipass.hooks.apps.handlers.fake.handle", + "matcher": "", + } + }, + } + with ( + patch("aipass.hooks.apps.modules.engine._log"), + patch("aipass.hooks.apps.modules.engine._run_handler") as mock_run, + ): + mock_run.return_value = {"exit_code": 0, "stdout": "ok", "stderr": "", "elapsed_ms": 5} + dispatch("UserPromptSubmit", "{}", config) + mock_run.assert_called_once_with("aipass.hooks.apps.handlers.fake.handle", {}, timeout_s=30) + + def test_timeout_logs_speaks_and_lets_dispatch_continue(self, mock_logger): + config = { + "hooks_enabled": True, + "UserPromptSubmit": { + "hung_handler": { + "enabled": True, + "handler": "aipass.hooks.apps.handlers.fake.handle", + "matcher": "", + }, + "next_handler": { + "enabled": True, + "handler": "aipass.hooks.apps.handlers.fake2.handle", + "matcher": "", + }, + }, + } + with ( + patch("aipass.hooks.apps.modules.engine._log") as mock_log, + patch("aipass.hooks.apps.modules.engine._run_handler") as mock_run, + patch("aipass.hooks.apps.sound.speak") as mock_speak, + ): + mock_run.side_effect = [ + {"exit_code": -1, "stdout": "", "stderr": "TIMEOUT", "elapsed_ms": 30000}, + {"exit_code": 0, "stdout": "survived", "stderr": "", "elapsed_ms": 5}, + ] + result = dispatch("UserPromptSubmit", "{}", config) + assert "survived" in result[0] + assert result[1] == 0 + mock_speak.assert_called_once() + log_calls = [c[0][0] for c in mock_log.call_args_list] + assert any(e.get("action") == "timeout" for e in log_calls if isinstance(e, dict)) + + def test_timed_out_handler_produces_no_output(self, mock_logger): + config = { + "hooks_enabled": True, + "UserPromptSubmit": { + "hung_handler": { + "enabled": True, + "handler": "aipass.hooks.apps.handlers.fake.handle", + "matcher": "", + }, + }, + } + with ( + patch("aipass.hooks.apps.modules.engine._log"), + patch("aipass.hooks.apps.modules.engine._run_handler") as mock_run, + patch("aipass.hooks.apps.sound.speak"), + ): + mock_run.return_value = {"exit_code": -1, "stdout": "", "stderr": "TIMEOUT", "elapsed_ms": 30000} + result = dispatch("UserPromptSubmit", "{}", config) + assert result == ("", 0) + + def test_command_type_timeout_also_fails_loud(self, mock_logger): + """The fail-loud path is shared by both hook types — command-type timeouts already + existed (subprocess timeout=), but were silent past the generic per-hook log line.""" + config = { + "hooks_enabled": True, + "PreToolUse": { + "slow_cmd": {"enabled": True, "command": "sleep 999", "matcher": ""}, + }, + } + with ( + patch("aipass.hooks.apps.modules.engine._log") as mock_log, + patch("aipass.hooks.apps.modules.engine._run_hook") as mock_run, + patch("aipass.hooks.apps.sound.speak") as mock_speak, + ): + mock_run.return_value = {"exit_code": -1, "stdout": "", "stderr": "TIMEOUT", "elapsed_ms": 30000} + result = dispatch("PreToolUse", '{"tool_name":"Edit"}', config) + assert result == ("", 0) + mock_speak.assert_called_once() + log_calls = [c[0][0] for c in mock_log.call_args_list] + assert any(e.get("action") == "timeout" for e in log_calls if isinstance(e, dict)) + + class TestJsonHandlerNotApplicable: """Hooks uses JSONL logging, not json_handler. These verify the log equivalent."""