Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 52 additions & 6 deletions tests/tools/test_approval_fallback_identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Comment thread
9thLevelSoftware marked this conversation as resolved.


def _submit(command="rm -rf /tmp/a", **extra):
Expand Down Expand Up @@ -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):
Comment thread
9thLevelSoftware marked this conversation as resolved.
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")
Expand Down
80 changes: 80 additions & 0 deletions tools/approval.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -1549,33 +1550,103 @@ 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)
Comment thread
9thLevelSoftware marked this conversation as resolved.
_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"}
]
Comment thread
9thLevelSoftware marked this conversation as resolved.
atomic_json_write(
_pending_path(),
{"requests": records},
mode=0o600,
default=str,
)
Comment thread
9thLevelSoftware marked this conversation as resolved.


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
if request_id in _pending
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:
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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)


Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)


Expand Down Expand Up @@ -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
Expand Down