mirror of
https://github.com/MervinPraison/PraisonAI.git
synced 2026-11-07 04:35:58 -10:00
Merge pull request #6083 from MervinPraison/claude/issue-6080-20261027
fix: enforce allow-always grant TTL on the in-memory hot path
This commit is contained in:
commit
b8fd43457d
2 files changed
+78
-11
No files matched your search
@@ -382,12 +382,16 @@ class PermissionAllowlist:
|
||||
path: Optional[Union[str, Path]] = None,
|
||||
) -> None:
|
||||
self._lock = threading.Lock()
|
||||
# In-memory mirror of grant keys (agent_id, tool_name, arg_signature)
|
||||
self._allowed: Set[tuple] = set()
|
||||
# In-memory mirror: (agent_id, tool_name, arg_signature) -> created_at.
|
||||
# Stamped so the grant TTL holds on the hot path for the whole process
|
||||
# lifetime, not only when the durable store is re-read on restart.
|
||||
self._allowed: Dict[tuple, float] = {}
|
||||
self._store: Optional[ScopedAllowlistStore] = None
|
||||
self._ttl_seconds = _DEFAULT_GRANT_TTL_SECONDS
|
||||
if durable:
|
||||
try:
|
||||
self._store = store or ScopedAllowlistStore(path=path)
|
||||
self._ttl_seconds = self._store.ttl_seconds
|
||||
self._rehydrate()
|
||||
except Exception:
|
||||
# Fail-closed on the persistence path: if the store cannot be
|
||||
@@ -409,13 +413,21 @@ class PermissionAllowlist:
|
||||
grant["tool_name"],
|
||||
grant.get("arg_signature") or "",
|
||||
)
|
||||
self._allowed.add(key)
|
||||
self._allowed[key] = float(grant.get("created_at") or time.time())
|
||||
if self._allowed:
|
||||
logger.info(
|
||||
"Rehydrated %d durable allow-list grant(s) from %s",
|
||||
len(self._allowed), self._store.path,
|
||||
)
|
||||
|
||||
def _evict_expired_locked(self) -> None:
|
||||
"""Drop grants older than the TTL from the in-memory mirror."""
|
||||
if self._ttl_seconds <= 0:
|
||||
return
|
||||
cutoff = time.time() - self._ttl_seconds
|
||||
for key in [k for k, ts in self._allowed.items() if ts <= cutoff]:
|
||||
del self._allowed[key]
|
||||
|
||||
# ── Scoped API ───────────────────────────────────────────────────
|
||||
def add_scoped(
|
||||
self,
|
||||
@@ -428,7 +440,7 @@ class PermissionAllowlist:
|
||||
"""Add a scoped grant, persisting it when durable."""
|
||||
key = (agent_id, tool_name, arg_signature or "")
|
||||
with self._lock:
|
||||
self._allowed.add(key)
|
||||
self._allowed[key] = time.time()
|
||||
if self._store is not None:
|
||||
try:
|
||||
self._store.add(
|
||||
@@ -457,11 +469,17 @@ class PermissionAllowlist:
|
||||
(empty signature) covers any arguments.
|
||||
"""
|
||||
sig = arg_signature or ""
|
||||
cutoff = time.time() - self._ttl_seconds if self._ttl_seconds > 0 else None
|
||||
with self._lock:
|
||||
# Only the candidate keys are checked (O(1)); expired ones are
|
||||
# evicted lazily. Full sweeps happen in list()/list_scoped().
|
||||
for a in (agent_id, ANY_AGENT):
|
||||
if (a, tool_name, "") in self._allowed:
|
||||
return True
|
||||
if sig and (a, tool_name, sig) in self._allowed:
|
||||
for key in ((a, tool_name, ""), (a, tool_name, sig) if sig else None):
|
||||
if key is None or key not in self._allowed:
|
||||
continue
|
||||
if cutoff is not None and self._allowed[key] <= cutoff:
|
||||
del self._allowed[key]
|
||||
continue
|
||||
return True
|
||||
return False
|
||||
|
||||
@@ -478,12 +496,11 @@ class PermissionAllowlist:
|
||||
if arg_signature is None:
|
||||
for key in list(self._allowed):
|
||||
if key[0] == agent_id and key[1] == tool_name:
|
||||
self._allowed.discard(key)
|
||||
del self._allowed[key]
|
||||
removed = True
|
||||
else:
|
||||
key = (agent_id, tool_name, arg_signature)
|
||||
if key in self._allowed:
|
||||
self._allowed.discard(key)
|
||||
if self._allowed.pop(key, None) is not None:
|
||||
removed = True
|
||||
if self._store is not None:
|
||||
try:
|
||||
@@ -508,6 +525,7 @@ class PermissionAllowlist:
|
||||
except Exception:
|
||||
logger.exception("Failed to list persisted grants; using memory")
|
||||
with self._lock:
|
||||
self._evict_expired_locked()
|
||||
items = [
|
||||
{
|
||||
"agent_id": a,
|
||||
@@ -530,7 +548,7 @@ class PermissionAllowlist:
|
||||
with self._lock:
|
||||
for key in list(self._allowed):
|
||||
if key[1] == tool_name:
|
||||
self._allowed.discard(key)
|
||||
del self._allowed[key]
|
||||
removed = True
|
||||
if self._store is not None:
|
||||
try:
|
||||
@@ -551,6 +569,7 @@ class PermissionAllowlist:
|
||||
grants are exposed via :meth:`list_scoped` / the ``grants`` view.
|
||||
"""
|
||||
with self._lock:
|
||||
self._evict_expired_locked()
|
||||
return sorted(
|
||||
{key[1] for key in self._allowed if key[0] == ANY_AGENT}
|
||||
)
|
||||
|
||||
@@ -378,3 +378,51 @@ def test_gateway_backend_forwards_authorized_reviewers(tmp_path):
|
||||
assert decision.approved is True
|
||||
|
||||
asyncio.run(go())
|
||||
|
||||
|
||||
def test_allowlist_hot_path_enforces_grant_ttl(tmp_path, monkeypatch):
|
||||
"""An allow-always grant expires in memory without a process restart."""
|
||||
import time as _time
|
||||
|
||||
from praisonai_bot.gateway.exec_approval import ScopedAllowlistStore
|
||||
|
||||
store = ScopedAllowlistStore(path=tmp_path / "allow.sqlite", ttl_seconds=60)
|
||||
al = PermissionAllowlist(store=store)
|
||||
al.add_scoped(agent_id="a1", tool_name="shell_exec", arg_signature="sig")
|
||||
assert al.allows(agent_id="a1", tool_name="shell_exec", arg_signature="sig")
|
||||
|
||||
now = _time.time()
|
||||
monkeypatch.setattr(_time, "time", lambda: now + 61)
|
||||
assert not al.allows(agent_id="a1", tool_name="shell_exec", arg_signature="sig")
|
||||
assert al.list_scoped() == []
|
||||
|
||||
|
||||
def test_allowlist_in_memory_grant_ttl(monkeypatch):
|
||||
"""Non-durable grants honour the default 90-day cap too."""
|
||||
import time as _time
|
||||
|
||||
al = PermissionAllowlist(durable=False)
|
||||
al.add("shell_exec")
|
||||
assert "shell_exec" in al
|
||||
now = _time.time()
|
||||
monkeypatch.setattr(_time, "time", lambda: now + 91 * 86400)
|
||||
assert "shell_exec" not in al
|
||||
assert al.list() == []
|
||||
|
||||
|
||||
def test_allowlist_rehydrate_keeps_original_grant_age(tmp_path, monkeypatch):
|
||||
"""Rehydrated grants keep their persisted created_at, not restart time."""
|
||||
import time as _time
|
||||
|
||||
from praisonai_bot.gateway.exec_approval import ScopedAllowlistStore
|
||||
|
||||
path = tmp_path / "allow.sqlite"
|
||||
PermissionAllowlist(store=ScopedAllowlistStore(path=path, ttl_seconds=60)).add(
|
||||
"shell_exec"
|
||||
)
|
||||
now = _time.time()
|
||||
monkeypatch.setattr(_time, "time", lambda: now + 30)
|
||||
al = PermissionAllowlist(store=ScopedAllowlistStore(path=path, ttl_seconds=60))
|
||||
assert "shell_exec" in al
|
||||
monkeypatch.setattr(_time, "time", lambda: now + 61)
|
||||
assert "shell_exec" not in al
|
||||
Reference in new issue
Block a user