diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cd274c..85e427d 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/services/api/app/core/auth.py b/services/api/app/core/auth.py index 92d10b8..5ccd693 100644 --- a/services/api/app/core/auth.py +++ b/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() diff --git a/services/api/app/routes/auth_settings.py b/services/api/app/routes/auth_settings.py index 9f80200..eac557b 100644 --- a/services/api/app/routes/auth_settings.py +++ b/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() diff --git a/services/api/app/routes/runtime.py b/services/api/app/routes/runtime.py index 24d1964..83efab1 100644 --- a/services/api/app/routes/runtime.py +++ b/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") diff --git a/services/api/app/routes/system.py b/services/api/app/routes/system.py index a51d604..4dfca6c 100644 --- a/services/api/app/routes/system.py +++ b/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") diff --git a/services/api/app/services/auth_settings_service.py b/services/api/app/services/auth_settings_service.py index 1333e07..466f67a 100644 --- a/services/api/app/services/auth_settings_service.py +++ b/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) diff --git a/services/api/tests/test_lockout_after_key.py b/services/api/tests/test_lockout_after_key.py new file mode 100644 index 0000000..98e08c1 --- /dev/null +++ b/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 diff --git a/services/api/tests/test_memory_reset_service.py b/services/api/tests/test_memory_reset_service.py index 47e6365..27ed935 100644 --- a/services/api/tests/test_memory_reset_service.py +++ b/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