fix: a lockout stops wrong keys, never the right one; failures expire

CHANGELOG.md

@@ -6,6 +6,13 @@ a decision with visible consequences.
 
 ## Unreleased
 
+- **A lockout stops wrong keys, never the right one.** After five wrong keys from one address,
+  every key from that address was refused for 300 s, so one misconfigured caller behind a proxy
+  or the host network locked out the rest, and failures never expired. Now the key is checked
+  first: a right key always passes (and clears the count), a wrong one is counted and refused
+  while the address is locked, and failures older than 15 minutes no longer count. Applies to
+  the admin key, read keys, listener keys and the current-key confirmations.
+
 - **No model answer, no AI analysis.** When no model answered (Pi down, local model off, hosted
   call refused), evidence processing stored an empty `local_ai_proposal` analysis, so the
   evidence counted as analysed and was never analysed again. Now nothing is stored.

services/api/app/core/auth.py

@@ -1,9 +1,7 @@
 from app.core.db import SessionLocal
 from app.services.auth_settings_service import (
-    clear_failed_attempts,
-    get_lockout_status,
+    attempt,
     has_agent_key,
-    register_failed_attempt,
     verify_admin_key,
     verify_agent_access_key,
 )
@@ -17,15 +15,12 @@ def require_key(request: Request, x_memorygate_key: str | None = Header(None, al
     lock_scope = f"auth:{client_host}"
     db = SessionLocal()
     try:
-        remaining = get_lockout_status(lock_scope)
+        valid = verify_admin_key(db, x_memorygate_key)
+        remaining = attempt(lock_scope, valid)
         if remaining:
             raise HTTPException(429, f"Too many failed attempts. Try again in {remaining} seconds.")
-        if not verify_admin_key(db, x_memorygate_key):
-            remaining = register_failed_attempt(lock_scope)
-            if remaining:
-                raise HTTPException(429, f"Too many failed attempts. Try again in {remaining} seconds.")
+        if not valid:
             raise HTTPException(401, "missing or invalid X-MemoryGate-Key")
-        clear_failed_attempts(lock_scope)
         return "admin"
     finally:
         db.close()
@@ -42,18 +37,13 @@ def require_read_key(request: Request, x_memorygate_key: str | None = Header(Non
             lock_scope = f"read:{client_host}:{agent_id}"
         else:
             lock_scope = f"read-unknown-agent:{client_host}"
-        remaining = get_lockout_status(lock_scope)
+        tier = ("admin" if verify_admin_key(db, x_memorygate_key)
+                else "read" if verify_agent_access_key(db, x_memorygate_key, agent_id) else None)
+        remaining = attempt(lock_scope, tier is not None)
         if remaining:
             raise HTTPException(429, f"Too many failed attempts. Try again in {remaining} seconds.")
-        if verify_admin_key(db, x_memorygate_key):
-            clear_failed_attempts(lock_scope)
-            return "admin"
-        if verify_agent_access_key(db, x_memorygate_key, agent_id):
-            clear_failed_attempts(lock_scope)
-            return "read"
-        remaining = register_failed_attempt(lock_scope)
-        if remaining:
-            raise HTTPException(429, f"Too many failed attempts. Try again in {remaining} seconds.")
-        raise HTTPException(401, "missing or invalid read key")
+        if tier is None:
+            raise HTTPException(401, "missing or invalid read key")
+        return tier
     finally:
         db.close()

services/api/app/routes/auth_settings.py

@@ -6,12 +6,10 @@
     AgentKeyCreateRequest,
 )
 from app.services.auth_settings_service import (
-    clear_failed_attempts,
+    attempt,
     create_agent_access_key,
     generate_admin_key,
     get_auth_state,
-    get_lockout_status,
-    register_failed_attempt,
     set_admin_key,
     validate_new_admin_key,
     verify_admin_key,
@@ -36,19 +34,16 @@ def change_key(payload: AdminKeyUpdateRequest, request: Request):
     lock_scope = f"settings:{client_host}"
     db = SessionLocal()
     try:
-        remaining = get_lockout_status(lock_scope)
+        valid = verify_admin_key(db, payload.current_key)
+        remaining = attempt(lock_scope, valid)
         if remaining:
             raise HTTPException(429, f"Too many failed attempts. Try again in {remaining} seconds.")
-        if not verify_admin_key(db, payload.current_key):
-            remaining = register_failed_attempt(lock_scope)
-            if remaining:
-                raise HTTPException(429, f"Too many failed attempts. Try again in {remaining} seconds.")
+        if not valid:
             raise HTTPException(403, "current key is invalid")
         validation_error = validate_new_admin_key(payload.new_key)
         if validation_error:
             raise HTTPException(400, validation_error)
         set_admin_key(db, payload.new_key)
-        clear_failed_attempts(lock_scope)
         return {"status": "ok"}
     finally:
         db.close()
@@ -60,17 +55,14 @@ def rotate_key(payload: AdminKeyRotateRequest, request: Request):
     lock_scope = f"settings:{client_host}"
     db = SessionLocal()
     try:
-        remaining = get_lockout_status(lock_scope)
+        valid = verify_admin_key(db, payload.current_key)
+        remaining = attempt(lock_scope, valid)
         if remaining:
             raise HTTPException(429, f"Too many failed attempts. Try again in {remaining} seconds.")
-        if not verify_admin_key(db, payload.current_key):
-            remaining = register_failed_attempt(lock_scope)
-            if remaining:
-                raise HTTPException(429, f"Too many failed attempts. Try again in {remaining} seconds.")
+        if not valid:
             raise HTTPException(403, "current key is invalid")
         new_key = generate_admin_key(payload.length)
         set_admin_key(db, new_key)
-        clear_failed_attempts(lock_scope)
         return {"status": "ok", "new_key": new_key}
     finally:
         db.close()

services/api/app/routes/runtime.py

@@ -21,9 +21,7 @@
 from app.schemas.runtime import AgentContextRequest, IngestEventRequest, MemoryQuestionRequest
 from app.services import listener_forgetting
 from app.services.auth_settings_service import (
-    clear_failed_attempts,
-    get_lockout_status,
-    register_failed_attempt,
+    attempt,
 )
 from app.services.briefing import build_briefing
 from app.services.conversation_memory import citations
@@ -94,17 +92,14 @@ def _listener_source(db, source_key: str, request: Request, key: str | None, *,
         raise HTTPException(404, "Enabled evidence source not found" if require_enabled else "Evidence source not found")
     client_host = request.client.host if request.client else "unknown"
     lock_scope = f"listener:{client_host}:{source.source_key}"
-    remaining = get_lockout_status(lock_scope)
-    if remaining:
-        raise HTTPException(429, f"Too many failed attempts. Try again in {remaining} seconds.")
     secret = json.loads(source.secret_json or "{}").get("ingest_key", "")
     # Bytes, not str: compare_digest raises on a non-ASCII str, and a header can carry one.
-    if not secret or not key or not hmac.compare_digest(secret.encode(), key.encode()):
-        remaining = register_failed_attempt(lock_scope)
-        if remaining:
-            raise HTTPException(429, f"Too many failed attempts. Try again in {remaining} seconds.")
+    valid = bool(secret and key and hmac.compare_digest(secret.encode(), key.encode()))
+    remaining = attempt(lock_scope, valid)
+    if remaining:
+        raise HTTPException(429, f"Too many failed attempts. Try again in {remaining} seconds.")
+    if not valid:
         raise HTTPException(401, "invalid listener key")
-    clear_failed_attempts(lock_scope)
     return source, json.loads(source.config_json or "{}").get("agent_id", "default")
 
 

services/api/app/routes/system.py

@@ -4,9 +4,7 @@
 from app.schemas.auth_settings import AiRuntimeUpdateRequest, MemoryResetRequest
 from app.services.ai_runtime_service import get_runtime_status, update_runtime_config
 from app.services.auth_settings_service import (
-    clear_failed_attempts,
-    get_lockout_status,
-    register_failed_attempt,
+    attempt,
     verify_admin_key,
 )
 from app.services.backup_service import create_backup, list_backups, resolve_backup
@@ -56,15 +54,12 @@ def download_backup(filename: str):
 def _confirm_admin(db, current_key: str, request: Request, action: str) -> None:
     client_host = request.client.host if request.client else "unknown"
     scope = f"{action}:{client_host}"
-    remaining = get_lockout_status(scope)
+    valid = verify_admin_key(db, current_key)
+    remaining = attempt(scope, valid)
     if remaining:
         raise HTTPException(429, f"Too many failed attempts. Try again in {remaining} seconds.")
-    if not verify_admin_key(db, current_key):
-        remaining = register_failed_attempt(scope)
-        if remaining:
-            raise HTTPException(429, f"Too many failed attempts. Try again in {remaining} seconds.")
+    if not valid:
         raise HTTPException(403, "Current admin key is invalid.")
-    clear_failed_attempts(scope)
 
 
 @router.get("/ai-runtime")

services/api/app/services/auth_settings_service.py

@@ -15,6 +15,8 @@
 _SINGLETON_ID = "singleton"
 _MAX_FAILED_ATTEMPTS = 5
 _LOCKOUT_SECONDS = 300
+# Failures older than this no longer count toward a lockout.
+_FAILURE_WINDOW_SECONDS = 900
 _attempt_state: dict[str, dict[str, float | int]] = {}
 
 # An environment key is a machine secret, so it is held to a length floor rather
@@ -222,17 +224,30 @@ def get_lockout_status(scope: str) -> int:
 
 def register_failed_attempt(scope: str) -> int:
     now = time.time()
-    state = _attempt_state.get(scope, {"count": 0, "locked_until": 0.0})
+    state = _attempt_state.get(scope, {"count": 0, "first_at": now, "locked_until": 0.0})
     locked_until = float(state.get("locked_until", 0))
     if locked_until <= now:
+        if now - float(state.get("first_at", now)) > _FAILURE_WINDOW_SECONDS:
+            state["count"], state["first_at"] = 0, now
         state["count"] = int(state.get("count", 0)) + 1
         state["locked_until"] = 0.0
     if int(state["count"]) >= _MAX_FAILED_ATTEMPTS:
-        state["count"] = 0
+        state["count"], state["first_at"] = 0, now
         state["locked_until"] = now + _LOCKOUT_SECONDS
     _attempt_state[scope] = state
     return get_lockout_status(scope)
 
 
+def attempt(scope: str, valid: bool) -> int:
+    """Seconds the caller must wait, or 0. Called after the key was checked: a valid key
+    always passes and clears the count, so a caller that shares an address with someone
+    sending wrong keys is never locked out; a wrong key counts, and is refused while the
+    scope is locked. Failures older than the window no longer count."""
+    if valid:
+        clear_failed_attempts(scope)
+        return 0
+    return get_lockout_status(scope) or register_failed_attempt(scope)
+
+
 def clear_failed_attempts(scope: str) -> None:
     _attempt_state.pop(scope, None)

services/api/tests/test_lockout_after_key.py

@@ -0,0 +1,37 @@
+"""A lockout stops wrong keys, never the right one: callers behind one address (a proxy, the
+host network) are not locked out by someone else's mistakes, and old failures expire."""
+
+from app.services import auth_settings_service
+from memory_stack import stack as stack
+
+WRONG = {"X-MemoryGate-Key": "not-the-key-at-all", "X-Agent-Id": "owner"}
+
+
+def _wrong(stack):
+    return stack.http.get("/memory", headers=WRONG).status_code
+
+
+def test_the_right_key_passes_while_wrong_keys_from_the_same_address_are_locked_out(stack):
+    assert [_wrong(stack) for _ in range(5)] == [401, 401, 401, 401, 429]
+
+    assert stack.http.get("/memory", headers=stack.admin_headers()).status_code == 200
+    # The right key cleared the count, so a later wrong key starts from one again.
+    assert _wrong(stack) == 401
+
+
+def test_a_wrong_key_stays_refused_while_the_address_is_locked(stack):
+    for _ in range(5):
+        _wrong(stack)
+
+    assert _wrong(stack) == 429
+
+
+def test_failures_older_than_the_window_no_longer_count(stack, monkeypatch):
+    now = [1_000_000.0]
+    monkeypatch.setattr(auth_settings_service.time, "time", lambda: now[0])
+    for _ in range(4):
+        assert _wrong(stack) == 401
+
+    now[0] += auth_settings_service._FAILURE_WINDOW_SECONDS + 1
+
+    assert _wrong(stack) == 401

services/api/tests/test_memory_reset_service.py

@@ -218,10 +218,11 @@ def test_a_wrong_current_key_deletes_nothing_and_writes_no_backup(sessions, monk
         assert db.query(Memory).count() == 2
 
 
-def test_five_wrong_keys_lock_the_reset_even_for_the_right_key(sessions, monkeypatch):
+def test_five_wrong_keys_lock_out_wrong_keys_but_never_the_right_one(sessions, monkeypatch):
     client = _client(sessions, monkeypatch)
-    codes = [_reset(client, f"wrong-{n}").status_code for n in range(5)]
-    assert codes == [403, 403, 403, 403, 429]
-    assert _reset(client, ADMIN_KEY).status_code == 429
+    codes = [_reset(client, f"wrong-{n}").status_code for n in range(6)]
+    assert codes == [403, 403, 403, 403, 429, 429]
     with sessions() as db:
         assert db.query(Memory).count() == 2
+    # Someone else's wrong keys from the same address never lock out the owner's right key.
+    assert _reset(client, ADMIN_KEY).status_code == 200