diff --git a/tests/tools/test_approval_fallback_identity.py b/tests/tools/test_approval_fallback_identity.py index 03581b0e8649..a5a3f4116d4d 100644 --- a/tests/tools/test_approval_fallback_identity.py +++ b/tests/tools/test_approval_fallback_identity.py @@ -12,6 +12,10 @@ def _clear_state(): with approval._lock: approval._pending.clear() getattr(approval, "_pending_by_session", {}).clear() + approval._pending_loaded_home = None + path = approval._pending_path() + if path.exists(): + path.unlink() def _submit(command="rm -rf /tmp/a", **extra): @@ -111,18 +115,60 @@ def test_expired_request_fails_closed(self): assert approval._pending[request["request_id"]]["status"] == "expired" assert approval._pending[request["request_id"]]["resolution"] is None - def test_post_restart_in_memory_request_fails_closed(self): + def test_resolved_request_survives_process_restart(self): request = _submit() - with approval._lock: - approval._pending.clear() - getattr(approval, "_pending_by_session", {}).clear() - assert approval.resolve_gateway_approval( SESSION, "once", request_id=request["request_id"], request_hash=request["argument_hash"], - ) == 0 + ) == 1 + persisted = approval._pending_path().read_text(encoding="utf-8") + assert "rm -rf" not in persisted + with approval._lock: + approval._pending.clear() + getattr(approval, "_pending_by_session", {}).clear() + approval._pending_loaded_home = None + + restored = approval.get_pending_approval(request["request_id"]) + + assert restored is not None + assert restored["status"] == "resolved" + assert restored["resolution"] == "once" + + def test_pending_request_survives_process_restart(self): + request = _submit() + with approval._lock: + approval._pending.clear() + approval._pending_by_session.clear() + approval._pending_loaded_home = None + + restored = approval.get_pending_approval(request["request_id"]) + + assert restored is not None + assert restored["status"] == "pending" + assert restored["argument_hash"] == request["argument_hash"] + assert restored["session_key"] == SESSION + + def test_clear_session_removes_persisted_requests(self): + request = _submit() + + approval.clear_session(SESSION) + with approval._lock: + approval._pending.clear() + approval._pending_by_session.clear() + approval._pending_loaded_home = None + + assert approval.get_pending_approval(request["request_id"]) is None + + def test_malformed_requests_collection_loads_as_empty(self): + approval._pending_path().write_text('{"requests": 7}', encoding="utf-8") + with approval._lock: + approval._pending.clear() + approval._pending_by_session.clear() + approval._pending_loaded_home = None + + assert approval.get_pending_approval("missing") is None def test_legacy_session_resolution_is_fifo_and_resolve_all_is_preserved(self, caplog): caplog.set_level("INFO") diff --git a/tools/approval.py b/tools/approval.py index 3073450cf761..c94eb8603b30 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -1435,6 +1435,7 @@ def detect_dangerous_command(command: str) -> tuple: # only the compatibility ordering used by session-only /approve and /deny. _pending: dict[str, dict] = {} _pending_by_session: dict[str, list[str]] = {} +_pending_loaded_home: Optional[str] = None _session_approved: dict[str, set] = {} _session_yolo: set[str] = set() _permanent_approved: set = set() @@ -1549,23 +1550,88 @@ def _approval_identity_matches(request: dict, expected_identity: Optional[dict]) return True +def _pending_path(): + from hermes_constants import get_hermes_home + + return get_hermes_home() / "approval_requests.json" + + +def _load_pending_locked() -> None: + """Load unresolved fallback approvals for the active profile once.""" + global _pending_loaded_home + path = _pending_path() + home = str(path.parent) + if _pending_loaded_home == home: + return + _pending.clear() + _pending_by_session.clear() + try: + payload = json.loads(path.read_text(encoding="utf-8")) if path.exists() else {} + except (OSError, ValueError, TypeError): + payload = {} + if not isinstance(payload, dict): + payload = {} + requests = payload.get("requests", []) + if not isinstance(requests, list): + requests = [] + for request in requests: + if not isinstance(request, dict) or request.get("status") not in {"pending", "resolved"}: + continue + request_id = str(request.get("request_id") or "") + session_key = str(request.get("session_key") or "") + if not request_id or not session_key or request_id in _pending: + continue + _pending[request_id] = request + _pending_by_session.setdefault(session_key, []).append(request_id) + _pending_loaded_home = home + + +def _persist_pending_locked() -> None: + """Persist unresolved fallback approvals without raw tool arguments.""" + from utils import atomic_json_write + + safe_fields = { + "request_id", "session_key", "created_at", "expires_at", + "argument_hash", "operation", "tool_name", "policy_key", + "pattern_key", "pattern_keys", "requester", "channel", "status", + "resolution", "resolution_reason", "resolved_at", "resolution_mode", + "allow_permanent", "description", + } + records = [ + {key: value for key, value in request.items() if key in safe_fields} + for request in _pending.values() + if request.get("status") in {"pending", "resolved"} + ] + atomic_json_write( + _pending_path(), + {"requests": records}, + mode=0o600, + default=str, + ) + + def _prune_pending_locked() -> None: """Drop terminal fallback records and repair the session index. Callers hold ``_lock``. A just-expired record is left in place until the next safe-point call so legacy inspection can still report its status. """ + _load_pending_locked() + changed = False for request_id, request in list(_pending.items()): status = request.get("status") if status in {"pending", "resolved"}: try: if time.time() >= float(request["expires_at"]): request["status"] = "expired" + changed = True except (KeyError, TypeError, ValueError): request["status"] = "stale" + changed = True continue if status in {"consumed", "expired", "stale"}: _pending.pop(request_id, None) + changed = True for session_key, request_ids in list(_pending_by_session.items()): kept = [ request_id for request_id in request_ids @@ -1573,9 +1639,14 @@ def _prune_pending_locked() -> None: and _pending[request_id].get("session_key") == session_key ] if kept: + if kept != request_ids: + changed = True _pending_by_session[session_key] = kept else: _pending_by_session.pop(session_key, None) + changed = True + if changed: + _persist_pending_locked() def _mark_expired(request: dict) -> bool: @@ -1621,6 +1692,7 @@ def _resolve_fallback_approval( ) return 0 if not _mark_expired(request): + _persist_pending_locked() logger.warning( "approval_resolution outcome=stale resolution_mode=exact " "request_id=%s session_key=%s", @@ -1629,6 +1701,7 @@ def _resolve_fallback_approval( return 0 if request_hash and request_hash != request.get("argument_hash"): request["status"] = "stale" + _persist_pending_locked() logger.warning( "approval_resolution outcome=changed resolution_mode=exact " "request_id=%s session_key=%s", @@ -1661,6 +1734,7 @@ def _resolve_fallback_approval( "request_id=%s session_key=%s choice=%s", resolution_mode, request["request_id"], session_key, choice, ) + _persist_pending_locked() return len(targets) @@ -1749,13 +1823,16 @@ def consume_pending_approval( try: if time.time() >= float(request["expires_at"]): request["status"] = "expired" + _persist_pending_locked() return None except (KeyError, TypeError, ValueError): request["status"] = "stale" + _persist_pending_locked() return None stored_hash = request.get("argument_hash") if not request_hash or not stored_hash or request_hash != stored_hash: request["status"] = "stale" + _persist_pending_locked() logger.warning( "approval_resolution outcome=changed resolution_mode=consume " "request_id=%s session_key=%s", @@ -1811,6 +1888,7 @@ def submit_pending(session_key: str, approval: dict) -> Optional[dict]: }) _pending[request_id] = request _pending_by_session.setdefault(session_key, []).append(request_id) + _persist_pending_locked() return copy.deepcopy(request) @@ -1946,10 +2024,12 @@ def clear_session(session_key: str) -> None: if not session_key: return with _lock: + _prune_pending_locked() _session_approved.pop(session_key, None) _session_yolo.discard(session_key) for request_id in _pending_by_session.pop(session_key, []): _pending.pop(request_id, None) + _persist_pending_locked() entries = _gateway_queues.pop(session_key, []) for entry in entries: # Session-boundary cleanup should cancel any blocked approval waits