diff --git a/CHANGELOG.md b/CHANGELOG.md index f6e811131e..5edfa00447 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -804,6 +804,29 @@ This release closes that milestone with **439 merged pull requests**. ### Fixed +- **auth:** Login lockouts are now counted once per client IP across every + Gateway replica. `POST /api/v1/auth/login/local` kept its failed-login + counter in a per-process dict, so with N replicas behind one load balancer an + attacker got N × `max_login_attempts` guesses and a lockout on one replica + was invisible to the others. The counter now lives behind a + `LoginThrottleStore`: the new `auth.local.throttle_storage` selector + (default `auto`) keeps it in the shared `login_throttle` table (migration + `0035_login_throttle`) whenever `database.backend` is `sqlite` or + `postgres`, and falls back to the in-process counter with a warning when + there is no database to share (a `memory` database, or a configured database + whose engine is not initialised); `memory` forces the historical per-process + behavior; `db` forces the table and refuses to start when the configured + database's engine is unavailable. The Gateway resolves the store once at + startup, right after the persistence engine; a bare app resolves it on the + first throttle call. Failures are counted with one atomic upsert that keeps + an active lock's start and committed duration (the sentence is "N seconds + after the lock started", not "after the last attempt"), the duration + committed at lock time is still honored when the policy changes mid-lock, a + successful login clears the IP everywhere, served locks and idle counters are + swept in bounded batches, and a declared multi-instance deployment that + keeps `memory` logs a startup warning. Status codes and messages of the + login endpoint are unchanged; `max_login_attempts` and `lockout_seconds` + stay live-read. `config_version` is now 57. ([#6501]) - **memory:** DeerMem's derived SQLite FTS5 retrieval index can now live outside the memory root, and a Gateway instance now notices facts another instance wrote. The index for every user was one SQLite database in WAL mode @@ -9259,4 +9282,5 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#6484]: https://github.com/bytedance/deer-flow/pull/6484 [#6494]: https://github.com/bytedance/deer-flow/pull/6494 [#6495]: https://github.com/bytedance/deer-flow/pull/6495 +[#6501]: https://github.com/bytedance/deer-flow/pull/6501 [#6506]: https://github.com/bytedance/deer-flow/pull/6506 diff --git a/README.md b/README.md index 735edb93ce..cd6b60086f 100644 --- a/README.md +++ b/README.md @@ -519,7 +519,7 @@ Browser login uses `HttpOnly` session cookies. The login page offers a "keep me DeerFlow still uses `Forwarded` / `X-Forwarded-*` headers to recover the browser-facing scheme and origin behind a proxy. The bundled nginx sets `X-Forwarded-Proto`, but preserves an upstream HTTPS value and does not overwrite every forwarded header. Configure the outer trusted proxy to replace or strip client-supplied forwarding headers before traffic reaches DeerFlow. > [!IMPORTANT] -> The Gateway still owns active run tasks in process, so production defaults to a single Gateway worker (`GATEWAY_WORKERS=1`). Multi-worker deployments require Postgres, the Redis stream bridge (`stream_bridge.type: redis`), `run_ownership.heartbeat_enabled: true`, and `run_events.backend: db`; process-local memory/JSONL event stores cannot enforce singleton delivery receipts across workers. Kubernetes replicas run one worker per Pod, which the worker count cannot see: declare them with `deployment.multi_instance: true` (or `DEER_FLOW_MULTI_INSTANCE=1`, which deploy tooling such as a Helm chart can set from its replica count) so the startup gate enforces the same prerequisites instead of staying inert. The bridge shares SSE delivery and bounded `Last-Event-ID` replay across workers. When a valid reconnect cursor has been trimmed, or a subscriber that already established an empty-stream wait falls behind before its first delivery, Memory and Redis emit a machine-readable SSE `gap` event instead of silently returning a partial replay; the Web UI reloads durable thread/event state and resumes from the retained tail. Lease reconciliation marks runs from dead workers as errors, persists their delivery receipts, publishes the terminal stream marker, schedules retained-stream cleanup, and updates the affected thread status. SSE, `/wait`, and internal stream consumers use `stream_bridge.heartbeat_interval_seconds` (default `15`) for idle liveness checks; changing it requires a Gateway restart. Malformed Redis reconnect IDs live-tail new events instead of replaying the retained buffer, and the rolling retained-buffer TTL (`stream_ttl_seconds`) remains a cleanup safety net rather than a run timeout. IM channel state and other process-local services still need their own multi-worker coordination. +> The Gateway still owns active run tasks in process, so production defaults to a single Gateway worker (`GATEWAY_WORKERS=1`). Multi-worker deployments require Postgres, the Redis stream bridge (`stream_bridge.type: redis`), `run_ownership.heartbeat_enabled: true`, and `run_events.backend: db`; process-local memory/JSONL event stores cannot enforce singleton delivery receipts across workers. Kubernetes replicas run one worker per Pod, which the worker count cannot see: declare them with `deployment.multi_instance: true` (or `DEER_FLOW_MULTI_INSTANCE=1`, which deploy tooling such as a Helm chart can set from its replica count) so the startup gate enforces the same prerequisites instead of staying inert. The bridge shares SSE delivery and bounded `Last-Event-ID` replay across workers. When a valid reconnect cursor has been trimmed, or a subscriber that already established an empty-stream wait falls behind before its first delivery, Memory and Redis emit a machine-readable SSE `gap` event instead of silently returning a partial replay; the Web UI reloads durable thread/event state and resumes from the retained tail. Lease reconciliation marks runs from dead workers as errors, persists their delivery receipts, publishes the terminal stream marker, schedules retained-stream cleanup, and updates the affected thread status. SSE, `/wait`, and internal stream consumers use `stream_bridge.heartbeat_interval_seconds` (default `15`) for idle liveness checks; changing it requires a Gateway restart. Malformed Redis reconnect IDs live-tail new events instead of replaying the retained buffer, and the rolling retained-buffer TTL (`stream_ttl_seconds`) remains a cleanup safety net rather than a run timeout. Failed-login counters and lockouts for `POST /api/v1/auth/login/local` (`auth.local.max_login_attempts` / `lockout_seconds`) are kept in the shared `login_throttle` table whenever the application database is SQLite or Postgres (`auth.local.throttle_storage: auto`, the default), so every replica enforces one lockout per client IP; `memory` keeps the historical per-process counter, which under N replicas hands an attacker N × `max_login_attempts` guesses and logs a startup warning. IM channel state and other process-local services still need their own multi-worker coordination. > > In single-process JSONL deployments, cancelling an admitted event-store mutation waits for its background file I/O, rollback, and bookkeeping to settle before releasing the thread write lock. This prevents an older cancelled write from recreating deleted records or rolling back a later successful write. Cancellation can therefore wait on slow storage; it does not stop an in-flight filesystem operation. Callers still waiting to acquire the lock can cancel without starting a mutation. A batch spanning multiple threads drains its current thread group before propagating cancellation; subsequent thread groups do not start. > diff --git a/backend/app/gateway/auth/login_throttle.py b/backend/app/gateway/auth/login_throttle.py new file mode 100644 index 0000000000..e84d396e75 --- /dev/null +++ b/backend/app/gateway/auth/login_throttle.py @@ -0,0 +1,121 @@ +"""Resolve and hold the Gateway's login throttle store. + +The auth router counts failed local logins through a +:class:`~deerflow.persistence.login_throttle.LoginThrottleStore`. Which +implementation backs it is decided once per process from +``auth.local.throttle_storage`` and ``database.backend`` +(:func:`deerflow.config.auth_config.resolve_login_throttle_storage`): + +- ``db`` (the ``auto`` result whenever an application database exists) — + :class:`SqlLoginThrottleStore` over the shared ``login_throttle`` table, so + every Gateway replica using that database enforces one lockout per IP; +- ``memory`` — :class:`MemoryLoginThrottleStore`, the historical per-process + dict; under several replicas an attacker gets N × ``max_login_attempts`` + guesses, which ``app.gateway.deps._validate_login_throttle_storage`` warns + about at startup. + +``langgraph_runtime`` installs the store right after the persistence engine +is initialised, mirroring ``get_local_provider``'s process-level cache. A bare +app that never ran that lifespan (tests, embedded routers) resolves lazily on +the first throttle call from the live config. Tests swap the store through +:func:`install_login_throttle_store` / :func:`reset_login_throttle_store`. +""" + +from __future__ import annotations + +import logging +import threading +from typing import Any + +from deerflow.config.auth_config import LocalAuthConfig, resolve_login_throttle_storage +from deerflow.persistence.login_throttle import LoginThrottleStore, MemoryLoginThrottleStore, SqlLoginThrottleStore + +logger = logging.getLogger(__name__) + +_store: LoginThrottleStore | None = None +_install_lock = threading.Lock() + + +def resolve_login_throttle_store(config: Any | None, *, session_factory: Any | None) -> LoginThrottleStore: + """Build the store ``config`` asks for, given the (possibly absent) ORM session factory. + + ``auto`` never raises: with no database to share, or a configured database + whose engine is not initialised (a bare app without the Gateway lifespan), + it degrades to the in-process counter with a warning so the login endpoint + keeps throttling. An explicit ``db`` is an operator statement that lockouts + must be shared: when the configured database's engine is unavailable this + raises ``SystemExit`` with an actionable message — the same failure type + as the other startup validations in ``app.gateway.deps`` — so + ``langgraph_runtime`` refuses to start instead of silently running every + replica on per-process counters. ``db`` on a ``memory`` database still + degrades with a warning (there is no shared table to insist on). + """ + local = getattr(getattr(config, "auth", None), "local", None) + selector = getattr(local, "throttle_storage", LocalAuthConfig.model_fields["throttle_storage"].default) + selector_value = str(getattr(selector, "value", selector)) + database_backend = getattr(getattr(config, "database", None), "backend", None) + resolved = resolve_login_throttle_storage(selector, database_backend) + if resolved == "memory": + if selector_value == "db": + logger.warning( + "auth.local.throttle_storage=db requires database.backend to be 'sqlite' or 'postgres' (got %r). Falling back to the in-process login throttle counter: lockouts are per Gateway process and are not shared across replicas.", + database_backend, + ) + return MemoryLoginThrottleStore() + if session_factory is None: + if selector_value == "db": + raise SystemExit( + f"auth.local.throttle_storage='db' requires the persistence engine for database.backend={database_backend!r} to be initialised before login " + "throttling starts, but no engine is available, so lockouts could not be shared across Gateway replicas. Initialise the database engine first " + "(the Gateway does this in langgraph_runtime), or set auth.local.throttle_storage to 'auto' (falls back to the in-process counter with a warning) or 'memory'." + ) + logger.warning( + "auth.local.throttle_storage=%s resolved to the shared database table, but no persistence engine is initialised. Falling back to the in-process login throttle counter for this process.", + selector_value, + ) + return MemoryLoginThrottleStore() + logger.info("auth.local.throttle_storage=%s resolved to the shared login_throttle table (database.backend=%s); login lockouts are enforced across every Gateway replica using this database.", selector_value, database_backend) + return SqlLoginThrottleStore(session_factory) + + +def installed_login_throttle_store() -> LoginThrottleStore | None: + """The store installed for this process, or ``None`` before startup / after reset.""" + return _store + + +def install_login_throttle_store(store: LoginThrottleStore) -> None: + """Make ``store`` the process-wide login throttle store (startup and test hook).""" + global _store + _store = store + + +def reset_login_throttle_store() -> None: + """Forget the installed store so the next throttle call resolves afresh.""" + global _store + _store = None + + +def resolve_and_install_from_live_config() -> LoginThrottleStore: + """Lazy fallback for apps without the Gateway lifespan: resolve from ``get_app_config()``. + + Synchronous and blocking (it reads ``config.yaml``); the router calls it + through ``asyncio.to_thread``. Only ``FileNotFoundError`` falls back to the + ``LocalAuthConfig`` defaults, matching ``_login_throttle_policy``: a + malformed config propagates rather than silently picking a store, and an + explicit ``db`` without an initialised engine fails closed exactly as it + does at Gateway startup (nothing is installed). Racing resolvers keep the + first installed store. + """ + from deerflow.config.app_config import get_app_config + from deerflow.persistence.engine import get_session_factory + + with _install_lock: + if _store is not None: + return _store + try: + config: Any = get_app_config() + except FileNotFoundError: + config = None + store = resolve_login_throttle_store(config, session_factory=get_session_factory()) + install_login_throttle_store(store) + return store diff --git a/backend/app/gateway/deps.py b/backend/app/gateway/deps.py index 693e75c085..2a07ae2831 100644 --- a/backend/app/gateway/deps.py +++ b/backend/app/gateway/deps.py @@ -259,6 +259,39 @@ def _validate_agent_storage(config: AppConfig) -> None: ) +def _validate_login_throttle_storage(config: AppConfig) -> None: + """Warn when a multi-process deployment counts login failures per process. + + ``auth.local.throttle_storage`` resolves to the shared ``login_throttle`` + table whenever an application database exists, so under the multi-process + gate (which already requires Postgres) only an explicit ``memory`` lands + here. That is not fatal — the throttle still works on every replica — but + with N replicas behind one load balancer an attacker gets N x + ``max_login_attempts`` guesses and a lockout on one replica is invisible + to the others, exactly the gap the shared table closes. Mirrors the + ``agent_storage.backend='file'`` divergence warning above. + """ + signal = _multi_process_signal(config) + if signal is None: + return + local = getattr(getattr(config, "auth", None), "local", None) + if local is None: + return + from deerflow.config.auth_config import LocalAuthConfig, resolve_login_throttle_storage + + selector = getattr(local, "throttle_storage", LocalAuthConfig.model_fields["throttle_storage"].default) + db_backend = getattr(getattr(config, "database", None), "backend", None) + if resolve_login_throttle_storage(selector, db_backend) == "memory": + logger.warning( + "%s with auth.local.throttle_storage=%s: failed-login counters and lockouts are kept per Gateway process, " + "so an attacker behind the load balancer gets N x max_login_attempts guesses and a lockout on one replica " + "is invisible to the others. Set auth.local.throttle_storage='auto' (or 'db') so the shared login_throttle " + "table in the application database enforces one limit per IP.", + signal[0], + str(getattr(selector, "value", selector)), + ) + + def _validate_memory_retrieval_index(config: AppConfig) -> None: """Warn when a declared multi-instance deployment keeps DeerMem's retrieval index on the shared memory volume. @@ -569,6 +602,8 @@ async def langgraph_runtime(app: FastAPI, startup_config: AppConfig) -> AsyncGen # Reject agent_storage.backend='db' on a non-durable database, and warn on # node-divergent file storage under multi-worker Postgres. _validate_agent_storage(startup_config) + # Warn when login lockouts stay per-process under several Gateway processes. + _validate_login_throttle_storage(startup_config) # Warn when a declared multi-instance deployment shares DeerMem's SQLite # retrieval index across instances through the memory volume. _validate_memory_retrieval_index(startup_config) @@ -640,6 +675,15 @@ async def langgraph_runtime(app: FastAPI, startup_config: AppConfig) -> AsyncGen # Initialize repositories — one get_session_factory() call for all. sf = get_session_factory() + + # The login throttle store is resolved once per process from the startup + # snapshot and the engine above (auth.local.throttle_storage is + # startup-only); the router reads it through the same hook tests use. + from app.gateway.auth.login_throttle import install_login_throttle_store, reset_login_throttle_store, resolve_login_throttle_store + + install_login_throttle_store(resolve_login_throttle_store(config, session_factory=sf)) + stack.callback(reset_login_throttle_store) + if sf is not None: from deerflow.persistence.feedback import FeedbackRepository from deerflow.persistence.personal_access_tokens import PersonalAccessTokenRepository diff --git a/backend/app/gateway/routers/auth.py b/backend/app/gateway/routers/auth.py index c0da7df496..35efe23584 100644 --- a/backend/app/gateway/routers/auth.py +++ b/backend/app/gateway/routers/auth.py @@ -16,6 +16,7 @@ from starlette.responses import RedirectResponse from app.gateway.auth import ( UserResponse, create_access_token, + login_throttle, ) from app.gateway.auth.config import get_auth_config from app.gateway.auth.errors import AuthErrorCode, AuthErrorResponse @@ -38,6 +39,7 @@ from app.gateway.csrf_middleware import CSRF_COOKIE_NAME, _request_origin, auth_ from app.gateway.deps import get_current_user_from_request, get_local_provider from app.gateway.utils import constant_time_equals from deerflow.config.auth_config import OIDCProviderConfig +from deerflow.persistence.login_throttle import LoginThrottleStore logger = logging.getLogger(__name__) @@ -160,27 +162,40 @@ def _set_session_cookie(response: Response, token: str, request: Request, *, rem # ── Rate Limiting ──────────────────────────────────────────────────────── -# In-process dict — not shared across workers. -# -# **Limitation**: with multi-worker deployments (e.g., gunicorn -w N), each -# worker maintains its own lockout table, so an attacker effectively gets -# N × max_login_attempts guesses before being locked out everywhere. For -# production multi-worker setups, replace this with a shared store (Redis, -# database-backed counter) to enforce a true per-IP limit. +# Failed logins are counted per client IP in a ``LoginThrottleStore`` +# (``deerflow.persistence.login_throttle``). The store is resolved once per +# process by ``app.gateway.auth.login_throttle`` — the Gateway lifespan +# installs it from the startup config and the persistence engine, a bare app +# resolves it lazily on the first throttle call — and ``auth.local. +# throttle_storage`` picks the implementation: the shared ``login_throttle`` +# table whenever an application database exists (every replica sharing that +# database enforces one lockout per IP), otherwise the historical in-process +# counter (per process: N replicas give an attacker N × max_login_attempts +# guesses; ``deps._validate_login_throttle_storage`` warns about that). # # The policy values are operator-configurable via auth.local.max_login_attempts / # auth.local.lockout_seconds (read live per call, matching _local_registration_enabled, # so a config reload applies to the next login without a Gateway restart). The # no-config.yaml fallback is the LocalAuthConfig model defaults — a single source # of truth, not a second copy of the numbers. +# +# A lock's stored duration always matches the policy the lock was last +# evaluated under (its creation counts as an evaluation, and every check that +# leaves the lock active commits the then-current duration, decreases +# included): a lowered lockout_seconds releases an active lock early, a raised +# one extends it — and a sentence that already served the last-evaluated +# duration is never resurrected. The stores implement that contract +# (``deerflow/persistence/login_throttle/base.py``); this module only sequences +# the calls and keeps the config read off the event loop. -# ip → (fail_count, locked_at, locked_duration). The stored duration always -# matches the policy the lock was last evaluated under (its creation counts -# as an evaluation, and every check that leaves the lock active commits the -# then-current duration, decreases included): a lowered lockout_seconds -# releases an active lock early, a raised one extends it — and a sentence -# that already served the last-evaluated duration is never resurrected. -_login_attempts: dict[str, tuple[int, float, float]] = {} + +async def _login_throttle_store() -> LoginThrottleStore: + """The process-wide store, resolving it off the loop on first use in a bare app.""" + store = login_throttle.installed_login_throttle_store() + if store is None: + # Resolution reads config.yaml (stat + hash); never on the loop. + store = await asyncio.to_thread(login_throttle.resolve_and_install_from_live_config) + return store def _login_throttle_policy() -> tuple[int, float]: @@ -271,99 +286,33 @@ def _get_client_ip(request: Request) -> str: async def _check_rate_limit(ip: str) -> None: """Raise 429 if the IP is currently locked out. - The record lookup comes before policy resolution on purpose: a clean IP - (no failed attempts recorded — the overwhelming majority of logins) must - not pay a config read, and ``get_app_config`` re-hashes config.yaml on - every call while this endpoint is unauthenticated. When a record exists - the policy is resolved off the event loop via ``asyncio.to_thread``: - every request from a recorded IP — including an already-locked attacker - flooding the endpoint — pays that read on the way to its answer, and the - stat + hash must not block the loop. + The policy is handed to the store as an async callable and resolved + lazily, inside the store's single session: a clean IP (no failed attempts + recorded — the overwhelming majority of logins) never pays the config + read, and ``get_app_config`` re-hashes config.yaml on every call while + this endpoint is unauthenticated. A recorded IP — including an + already-locked attacker flooding the endpoint — resolves it exactly once, + off the event loop via ``asyncio.to_thread`` (the stat + hash must not + block the loop), and the SQL store serves the probe, the resolution and + the decision from one connection-pool checkout instead of two. + + The policy read is a yield point: another request for the same IP may + delete or replace the record meanwhile, so the store decides on a fresh + read taken after it and guards its own mutations against that snapshot — + a record replaced mid-flight (e.g. a successful login followed by a new + failure) is never clobbered by a stale decision. """ - record = _login_attempts.get(ip) - if record is None: - return - max_attempts, lockout_seconds = await asyncio.to_thread(_login_throttle_policy) - # The await above is a yield point: while this coroutine was suspended, - # another request for the same IP may have deleted or replaced the record - # (the pre-async version was atomic on the loop). The pre-read served only - # as the cheap clean-IP skip; decide on a fresh snapshot from here on — - # everything below is synchronous, and every mutation is guarded by - # re-comparing against that snapshot so a record replaced mid-flight - # (e.g. a successful login followed by a new failure) is never clobbered. - record = _login_attempts.get(ip) - if record is None: - return - fail_count, locked_at, locked_duration = record - if fail_count < max_attempts: - return - if locked_at == 0.0: - # Over the *current* threshold but the lock never started under the - # threshold these failures accumulated under (the operator tightened - # max_login_attempts mid-count). Keep the record: the next failure - # starts the lock and a successful login clears it — deleting here - # would hand the IP a fresh budget under a stricter policy. - return - now = time.time() - if now >= locked_at + locked_duration: - # The lock served the full sentence of the duration in force when it - # started — a later duration increase must not resurrect it. - if _login_attempts.get(ip) == record: - del _login_attempts[ip] - return - if now < locked_at + lockout_seconds: - # Still locked. The sentence now follows the current duration, and - # that evaluation is committed — including decreases — so the stored - # sentence always matches the policy the lock was last evaluated - # under; a later raise can never resurrect time the lock already - # served under a shorter policy. - if lockout_seconds != locked_duration and _login_attempts.get(ip) == record: - _login_attempts[ip] = (fail_count, locked_at, lockout_seconds) + store = await _login_throttle_store() + + async def policy() -> tuple[int, float]: + return await asyncio.to_thread(_login_throttle_policy) + + remaining = await store.check(ip, policy=policy, now=time.time()) + if remaining > 0.0: raise HTTPException( status_code=429, detail="Too many login attempts. Try again later.", ) - # Original sentence still running, but the current (lowered) duration has - # already elapsed — release early. - if _login_attempts.get(ip) == record: - del _login_attempts[ip] - - -_MAX_TRACKED_IPS = 10000 - - -def _record_failure_under_policy(ip: str, max_attempts: int, lockout_seconds: float) -> None: - """Apply one failed login to the counter under an explicit policy.""" - # Evict expired lockouts when dict grows too large. Expiry is a property - # of each record's own committed sentence — `t > 0 and now >= t + d` — - # independent of the live threshold: a record locked under an old, lower - # threshold must still be swept once its sentence is served, even if the - # current max has moved past its count. Gating on the current threshold - # here would retain expired records while the capacity fallback below - # evicts live counters (they sort first), granting active offenders - # fresh budgets. - if len(_login_attempts) >= _MAX_TRACKED_IPS: - now = time.time() - expired = [k for k, (c, t, d) in _login_attempts.items() if t > 0.0 and now >= t + d] - for k in expired: - del _login_attempts[k] - # If still too large, evict cheapest-to-lose half ordered by each - # record's own expiry: never-locked counters (t + d == 0.0) first, - # then locked records whose committed sentence expires earliest. - if len(_login_attempts) >= _MAX_TRACKED_IPS: - by_time = sorted(_login_attempts.items(), key=lambda kv: kv[1][1] + kv[1][2]) - for k, _ in by_time[: len(by_time) // 2]: - del _login_attempts[k] - - record = _login_attempts.get(ip) - if record is None: - _login_attempts[ip] = (1, 0.0, 0.0) - else: - new_count = record[0] + 1 - if new_count >= max_attempts: - _login_attempts[ip] = (new_count, time.time(), lockout_seconds) - else: - _login_attempts[ip] = (new_count, 0.0, 0.0) async def _record_login_failure(ip: str) -> None: @@ -373,6 +322,7 @@ async def _record_login_failure(ip: str) -> None: this is the first config read for a previously clean IP, and the login endpoint is unauthenticated. """ + store = await _login_throttle_store() try: max_attempts, lockout_seconds = await asyncio.to_thread(_login_throttle_policy) except Exception: @@ -386,14 +336,15 @@ async def _record_login_failure(ip: str) -> None: from deerflow.config.auth_config import LocalAuthConfig fallback = LocalAuthConfig() - _record_failure_under_policy(ip, fallback.max_login_attempts, fallback.lockout_seconds) + await store.record_failure(ip, max_attempts=fallback.max_login_attempts, lockout_seconds=fallback.lockout_seconds, now=time.time()) raise - _record_failure_under_policy(ip, max_attempts, lockout_seconds) + await store.record_failure(ip, max_attempts=max_attempts, lockout_seconds=lockout_seconds, now=time.time()) -def _record_login_success(ip: str) -> None: - """Clear failure counter for the given IP on successful login.""" - _login_attempts.pop(ip, None) +async def _record_login_success(ip: str) -> None: + """Clear the failure counter for the given IP on successful login.""" + store = await _login_throttle_store() + await store.reset(ip) # ── Endpoints ───────────────────────────────────────────────────────────── @@ -419,7 +370,7 @@ async def login_local( detail=AuthErrorResponse(code=AuthErrorCode.INVALID_CREDENTIALS, message="Incorrect email or password").model_dump(), ) - _record_login_success(client_ip) + await _record_login_success(client_ip) token = create_access_token(str(user.id), token_version=user.token_version) _set_session_cookie(response, token, request, remember_me=remember_me) diff --git a/backend/docs/AUTH_DESIGN.md b/backend/docs/AUTH_DESIGN.md index b8f026a2f3..9d725d2f05 100644 --- a/backend/docs/AUTH_DESIGN.md +++ b/backend/docs/AUTH_DESIGN.md @@ -110,7 +110,7 @@ enum UserScope: - 成功后签发 JWT，放入 `access_token` HttpOnly cookie。 - 响应体只返回 `expires_in` 和 `needs_setup`，不返回 token。 -登录失败会按客户端 IP 计数。IP 解析只在 TCP peer 属于 `AUTH_TRUSTED_PROXIES` 时信任 `X-Real-IP`，不使用 `X-Forwarded-For`。阈值与锁定时长可通过 `auth.local.max_login_attempts`（默认 5）和 `auth.local.lockout_seconds`（默认 300 秒）配置，按次实时读取，改配置后下一次登录即生效，无需重启 Gateway（`max_login_attempts` 最小为 2：单次失败不得锁定 IP。时长热改按方向生效：下调可提前释放进行中的锁定、收紧阈值会保留已计数的失败；上调只延长仍在锁定期内的锁定，不会复活已服满原时长的锁定）。 +登录失败会按客户端 IP 计数。IP 解析只在 TCP peer 属于 `AUTH_TRUSTED_PROXIES` 时信任 `X-Real-IP`，不使用 `X-Forwarded-For`。阈值与锁定时长可通过 `auth.local.max_login_attempts`（默认 5）和 `auth.local.lockout_seconds`（默认 300 秒）配置，按次实时读取，改配置后下一次登录即生效，无需重启 Gateway（`max_login_attempts` 最小为 2：单次失败不得锁定 IP。时长热改按方向生效：下调可提前释放进行中的锁定、收紧阈值会保留已计数的失败；上调只延长仍在锁定期内的锁定，不会复活已服满原时长的锁定）。计数器的存放位置由 `auth.local.throttle_storage` 决定（启动时解析一次，改动需重启）：默认 `auto` 在 `database.backend` 为 `sqlite` / `postgres` 时使用应用数据库中的 `login_throttle` 表，所有共享该数据库的 Gateway 副本对同一 IP 执行同一份限制；`memory` 退回进程内计数器（N 个副本意味着攻击者有 N × `max_login_attempts` 次机会，且一个副本上的锁定对其他副本不可见，多实例部署会在启动时打 WARNING）；`db` 强制使用共享表，数据库为 `memory` 时降级为进程内计数器并打 WARNING，已配置的数据库引擎不可用时则拒绝启动（`auto` 在该情况下降级并打 WARNING）。锁定期内的再次失败不会重置 `locked_at`：锁定时长从锁定开始计算，而不是从最后一次尝试计算。 ### 注册 @@ -442,7 +442,7 @@ thread 归属读取 Gateway 所配置数据库（`config.yaml` 的 `database`） | 边界 | 当前行为 | 后续方向 | |---|---|---| | 无 admin 时注册普通用户 | 允许注册普通 `user` | 如产品要求先初始化 admin，给 `/register` 加 gate | -| 登录限速 | 进程内 dict，单 worker 精确，多 worker 近似 | Redis / DB-backed rate limiter | +| 登录限速 | 默认存放在应用数据库的 `login_throttle` 表（`auth.local.throttle_storage: auto`），多副本共享同一份计数；`memory` 为进程内计数器，多 worker 近似 | Redis-backed rate limiter；按账号而非仅按 IP 限速 | | OAuth / OIDC | 已实现通用 OIDC SSO（Keycloak, Google, Azure AD, Okta 等），支持 PKCE + nonce、auto-provisioning、email domain 限制（详见 [SSO.md](SSO.md)） | 支持 RP-initiated logout、自定义 scope 映射 | | IM 用户隔离 | `channel_connections` 绑定到 `users.id`；未绑定消息在 `require_bound_identity: true` 时被拒绝 | 更多渠道与审计能力 | | Internal Auth 终端直持 token | 平台可把共享密钥下发给终端，导致 `Owner-User-Id` 可伪造 | 仅平台后端持 token；终端走平台自己的认证 | diff --git a/backend/docs/CONFIGURATION.md b/backend/docs/CONFIGURATION.md index dd2f45afd8..341bb30ae7 100644 --- a/backend/docs/CONFIGURATION.md +++ b/backend/docs/CONFIGURATION.md @@ -584,6 +584,8 @@ Notes: - Set `multi_instance: true` (or export `DEER_FLOW_MULTI_INSTANCE=1`, which lets deploy tooling such as a Helm chart set it from its replica count) on every instance that shares one database. Startup then enforces the same prerequisites as `GATEWAY_WORKERS > 1`: `database.backend: postgres`, `run_events.backend: db`, `run_ownership.heartbeat_enabled: true`, and a Redis stream bridge (`stream_bridge.type: redis` or `DEER_FLOW_STREAM_BRIDGE_REDIS_URL`). It refuses an explicit `sandbox.ownership.type: memory`, process-local browser tools, and `scheduler.enabled: true` without `scheduler.multi_instance: true`. - `DEER_FLOW_MULTI_INSTANCE` treats blank, `0`, `false`, `no` and `off` as "not declared"; any other value declares a multi-instance deployment, so a typo fails closed. - `GET /health/ready` pings the Redis stream bridge on every probe and answers 503 (`stream_bridge: unreachable`) while Redis is down, so the orchestrator drains that instance instead of routing it runs it cannot publish or stream; the memory bridge reports `not_configured`. When `sandbox.provisioner_url` is set, the body also carries the provisioner's `/health` verdict (`provisioner: ok|unreachable`), which never changes the status code because every instance shares one provisioner. That probe follows the sandbox clients' proxy policy: loopback, private, link-local and cluster-local provisioner addresses bypass `HTTP_PROXY`, external hosts keep the environment's proxy settings. +- The declaration also drives the `agent_storage.backend: file` divergence warning, the inbound webhook dedupe warning, the `auth.local.throttle_storage: memory` warning, and the WeChat QR-login guard, which otherwise only look at the worker count. +- Login lockouts are shared through the database: `auth.local.throttle_storage` (startup-only) defaults to `auto`, which keeps the per-IP failed-login counters for `POST /api/v1/auth/login/local` in the `login_throttle` table whenever `database.backend` is `sqlite` or `postgres`, so every instance enforces one `max_login_attempts` limit per client IP and a lockout on one instance holds on all of them. `memory` keeps the historical per-process counter (N instances give an attacker N × `max_login_attempts` guesses; a declared multi-instance deployment logs a warning), and `db` forces the table (it falls back to memory with a warning when the database backend is `memory`, and refuses to start when the configured database's engine is unavailable; `auto` falls back with a warning in that case). `max_login_attempts` and `lockout_seconds` stay live-read. - The declaration also drives the `agent_storage.backend: file` divergence warning, the inbound webhook dedupe warning, and the WeChat QR-login guard, which otherwise only look at the worker count. - With the default DeerMem backend, the declaration also warns when the derived SQLite retrieval index sits inside the shared `storage_path` (the default `{storage_path}/.retrieval`). Set `memory.backend_config.retrieval_index_path` to an instance-local directory (relative values resolve against `storage_path`): SQLite WAL is unsupported on network filesystems, and the index is rebuilt from the Markdown facts at startup and re-synced per user scope after a peer writes. - Restart-required: the gate runs once at startup. Restart all Gateway instances together after changing it. diff --git a/backend/packages/harness/deerflow/config/AGENTS.md b/backend/packages/harness/deerflow/config/AGENTS.md index d25b160747..d9b63a7b5c 100644 --- a/backend/packages/harness/deerflow/config/AGENTS.md +++ b/backend/packages/harness/deerflow/config/AGENTS.md @@ -63,7 +63,7 @@ YAML loading and both middleware tool-call paths in `test_tool_output_config_lim **Config Hot-Reload Boundary**: Gateway dependencies route through `get_app_config()` on every request, so per-run fields like `models[*].max_tokens`, `summarization.*`, `title.*`, `memory.*`, `subagents.*`, `verification.*`, `tools[*]`, and the agent system prompt pick up `config.yaml` edits on the next message. `AppConfig` is intentionally **not** cached on `app.state` — `lifespan()` keeps a local `startup_config` variable for one-shot bootstrap work and passes it to `langgraph_runtime(app, startup_config)`. -Infrastructure fields are **restart-required**. The authoritative list lives in `packages/harness/deerflow/config/reload_boundary.py::STARTUP_ONLY_FIELDS` and is mirrored by the standardised `"startup-only:"` prefix on the corresponding `Field(description=...)` in `AppConfig` or an explicitly registered nested config model, so IDE hover on those fields surfaces the reason inline (no need to context-switch into this table). Currently registered: `plugins`, `database`, `checkpointer`, `run_events`, `agent_storage`, `stream_bridge`, `sandbox`, `skills.container_path`, `log_level`, `logging`, `channels`, `channel_connections`, `scheduler`, `mcp_tasks`, `subagent_runtime`, `subagent_batches`, `run_ownership`, `dedupe_storage`. Adding a new restart-required field requires updating the registry; drift is pinned by `tests/test_reload_boundary.py`. `scheduler.recursion_limit` is the exception inside that section: it is read from `get_app_config()` at each scheduled dispatch, so a YAML edit applies to the next run without restarting the poller. +Infrastructure fields are **restart-required**. The authoritative list lives in `packages/harness/deerflow/config/reload_boundary.py::STARTUP_ONLY_FIELDS` and is mirrored by the standardised `"startup-only:"` prefix on the corresponding `Field(description=...)` in `AppConfig` or an explicitly registered nested config model, so IDE hover on those fields surfaces the reason inline (no need to context-switch into this table). Currently registered: `plugins`, `database`, `checkpointer`, `run_events`, `agent_storage`, `stream_bridge`, `sandbox`, `skills.container_path`, `log_level`, `logging`, `channels`, `channel_connections`, `scheduler`, `mcp_tasks`, `subagent_runtime`, `subagent_batches`, `run_ownership`, `dedupe_storage`, `auth.local.throttle_storage` (the login throttle store is installed once by `langgraph_runtime`; `max_login_attempts` / `lockout_seconds` stay live-read). Adding a new restart-required field requires updating the registry; drift is pinned by `tests/test_reload_boundary.py`. `scheduler.recursion_limit` is the exception inside that section: it is read from `get_app_config()` at each scheduled dispatch, so a YAML edit applies to the next run without restarting the poller. **Persistence backend resolution**: the unified `database` section selects the Gateway's LangGraph checkpointer, LangGraph Store, and DeerFlow SQL repositories. diff --git a/backend/packages/harness/deerflow/config/auth_config.py b/backend/packages/harness/deerflow/config/auth_config.py index 0ff017eb96..671d0e90f0 100644 --- a/backend/packages/harness/deerflow/config/auth_config.py +++ b/backend/packages/harness/deerflow/config/auth_config.py @@ -2,10 +2,13 @@ from __future__ import annotations +from enum import StrEnum from typing import Literal from pydantic import BaseModel, Field +from deerflow.config.reload_boundary import format_field_description + class OIDCProviderConfig(BaseModel): """Configuration for a single OIDC identity provider (Keycloak, Google, Azure AD, etc.).""" @@ -67,6 +70,31 @@ class OIDCAuthConfig(BaseModel): ) +class LoginThrottleStorage(StrEnum): + """Where the per-IP failed-login counter behind ``POST /api/v1/auth/login/local`` lives.""" + + AUTO = "auto" + MEMORY = "memory" + DB = "db" + + +def resolve_login_throttle_storage(selector: LoginThrottleStorage | str | None, database_backend: str | None) -> Literal["memory", "db"]: + """Resolve ``auth.local.throttle_storage`` against ``database.backend``. + + ``auto`` (the default) shares the counter through the application database + whenever one exists (``sqlite`` or ``postgres``), so every Gateway replica + using that database enforces one lockout per IP; a ``memory`` database has + no shared table and falls back to the per-process counter. ``memory`` + always keeps the per-process counter. ``db`` asks for the shared table and + degrades to ``memory`` when there is no database to hold it — the caller + warns about that misconfiguration. + """ + value = str(selector.value if isinstance(selector, StrEnum) else selector or LoginThrottleStorage.AUTO) + if value == LoginThrottleStorage.MEMORY: + return "memory" + return "db" if database_backend in ("sqlite", "postgres") else "memory" + + class LocalAuthConfig(BaseModel): """Configuration for the built-in email/password authentication provider.""" @@ -88,9 +116,10 @@ class LocalAuthConfig(BaseModel): "Raise it when many users share an egress IP (corporate proxy / NAT); lower it for " "a stricter posture. Minimum 2: one failed attempt must never lock an IP, or a " "single typo would block everyone behind a shared egress — the strictest legal " - "value locks after the second failure. The counter is per-Gateway-worker " - "(in-process), so effective attempts in multi-worker deployments scale with " - "worker count." + "value locks after the second failure. Where the counter lives is decided by " + "throttle_storage: shared through the application database (the default whenever " + "one exists) every Gateway replica enforces one limit per IP; the in-process " + "counter is per worker, so effective attempts scale with the worker count." ), ) lockout_seconds: float = Field( @@ -99,6 +128,21 @@ class LocalAuthConfig(BaseModel): allow_inf_nan=False, description=("Seconds an IP stays locked out after reaching auth.local.max_login_attempts. Defaults preserve the historical hardcoded policy (5 minutes)."), ) + throttle_storage: LoginThrottleStorage = Field( + default=LoginThrottleStorage.AUTO, + description=format_field_description( + "auth.local.throttle_storage", + field_doc=( + "Where failed-login counters and lockouts for POST /api/v1/auth/login/local are kept. " + "'auto' uses the application database whenever database.backend is sqlite or postgres, so every " + "Gateway replica sharing that database enforces one lockout per client IP; with database.backend=memory " + "it falls back to an in-process counter. 'memory' forces the in-process counter (per process: N replicas " + "give an attacker N x max_login_attempts guesses and a lockout on one replica is invisible to the others). " + "'db' forces the shared table: it falls back to memory with a warning when the database backend is memory, and refuses to start when the configured " + "database's engine is unavailable." + ), + ), + ) class AuthAppConfig(BaseModel): diff --git a/backend/packages/harness/deerflow/config/reload_boundary.py b/backend/packages/harness/deerflow/config/reload_boundary.py index 6fb63f04e3..67ddd7bb39 100644 --- a/backend/packages/harness/deerflow/config/reload_boundary.py +++ b/backend/packages/harness/deerflow/config/reload_boundary.py @@ -95,6 +95,11 @@ STARTUP_ONLY_FIELDS: dict[str, str] = { "make_inbound_dedupe_store() resolves the inbound dedupe store once when ChannelService is constructed at startup; the store " "(in-process memory or shared Postgres) is captured onto ChannelManager and is not rebuilt on config.yaml edits." ), + "auth.local.throttle_storage": ( + "langgraph_runtime() resolves the login throttle store once at startup from this selector and the persistence engine " + "(app.gateway.auth.login_throttle); the store (in-process counter or the shared login_throttle table) is not rebuilt on " + "config.yaml edits, so switching it needs a Gateway restart. auth.local.max_login_attempts and lockout_seconds stay live-read." + ), } diff --git a/backend/packages/harness/deerflow/persistence/login_throttle/__init__.py b/backend/packages/harness/deerflow/persistence/login_throttle/__init__.py new file mode 100644 index 0000000000..2205e4afe7 --- /dev/null +++ b/backend/packages/harness/deerflow/persistence/login_throttle/__init__.py @@ -0,0 +1,21 @@ +"""Per-IP failed-login counter behind local login (``auth.local.throttle_storage``).""" + +from deerflow.persistence.login_throttle.base import CheckDecision, LoginThrottlePolicy, LoginThrottleRecord, LoginThrottleStore, evaluate_check +from deerflow.persistence.login_throttle.memory import MAX_TRACKED_IPS, MemoryLoginThrottleStore +from deerflow.persistence.login_throttle.model import LOGIN_THROTTLE_IP_LENGTH, LoginThrottleRow +from deerflow.persistence.login_throttle.sql import STALE_COUNTER_SECONDS, SWEEP_BATCH_SIZE, SqlLoginThrottleStore + +__all__ = [ + "LOGIN_THROTTLE_IP_LENGTH", + "MAX_TRACKED_IPS", + "STALE_COUNTER_SECONDS", + "SWEEP_BATCH_SIZE", + "CheckDecision", + "LoginThrottlePolicy", + "LoginThrottleRecord", + "LoginThrottleRow", + "LoginThrottleStore", + "MemoryLoginThrottleStore", + "SqlLoginThrottleStore", + "evaluate_check", +] diff --git a/backend/packages/harness/deerflow/persistence/login_throttle/base.py b/backend/packages/harness/deerflow/persistence/login_throttle/base.py new file mode 100644 index 0000000000..8d89bfe578 --- /dev/null +++ b/backend/packages/harness/deerflow/persistence/login_throttle/base.py @@ -0,0 +1,148 @@ +"""Contract for the per-IP failed-login counter behind ``POST /api/v1/auth/login/local``. + +A store keeps one record per client IP: how many consecutive logins failed, +and — once that count reached ``max_attempts`` — when the lock started and +the sentence committed for it. The policy values (``max_attempts``, +``lockout_seconds``) are *not* stored as configuration: the caller passes the +live values on every call so a ``config.yaml`` edit applies to the next login +without a restart, and the semantics below decide how an in-flight record +follows a changed policy. + +Semantics every implementation must share (pinned by +``tests/test_login_throttle_store.py`` against both stores): + +- ``record_failure`` increments the count. A new count below ``max_attempts`` + leaves the IP counting with no lock (so a failure after the operator + raised the threshold clears an existing lock). A new count at or over the + threshold locks: a failure recorded while a lock is *active* keeps the + lock's start and committed duration — the sentence is "N seconds after the + lock started", never "after the last attempt" — while a served or absent + lock starts anew *now* with ``lockout_seconds`` as its committed duration + (a record already over the threshold but never locked — the operator + tightened the policy mid-count — locks on that next failure). +- ``check`` allows a record below the current threshold (raising + ``max_attempts`` releases a lower count immediately) and one that is over + it but never locked (the count is kept, not reset). An active lock first + serves the duration committed for it: once that elapsed the record is + cleared, so a later raise of ``lockout_seconds`` never resurrects a served + sentence. While the committed sentence still runs, the lock follows the + *current* duration — a lowered value releases early, a raised value + extends — and that evaluation is committed to the record (decreases + included), so the stored sentence always matches the policy the lock was + last evaluated under. +- ``reset`` forgets the IP (successful login). +- ``check`` takes the policy as an async callable and resolves it lazily: + a clean IP (no record — the overwhelming majority of logins) returns + ``0.0`` without ever calling it, so the router's ``config.yaml`` read is + skipped there; a recorded IP resolves it exactly once, and the decision is + then made on a snapshot read *after* that resolution — the policy read is a + yield point, and a decision that needs no write has no compare-and-set to + catch a record a peer changed meanwhile. The SQL store does the probe, + the resolution and the decision in one session. +- ``get`` is a non-mutating probe (tests and anchors use it); the router no + longer needs it because ``check`` skips the policy for clean IPs itself. + +``now`` is an epoch timestamp supplied by the caller (defaulting to +``time.time()``) so replicas compare the same clock the lock was stamped with +and tests can freeze it. +""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable +from dataclasses import dataclass +from typing import Protocol + +#: Resolves ``(max_login_attempts, lockout_seconds)``; awaited by ``check`` at most once, never for a clean IP. +LoginThrottlePolicy = Callable[[], Awaitable[tuple[int, float]]] + + +@dataclass(frozen=True, slots=True) +class LoginThrottleRecord: + """One IP's throttle state. + + ``locked_at == 0.0`` means "counting, never locked"; a lock stores the + epoch timestamp it started at and the duration committed for it. + """ + + fail_count: int + locked_at: float = 0.0 + lock_duration: float = 0.0 + + @property + def locked(self) -> bool: + return self.locked_at > 0.0 + + @property + def expires_at(self) -> float: + """When the committed sentence ends (0.0 for a never-locked counter).""" + return self.locked_at + self.lock_duration if self.locked else 0.0 + + +@dataclass(frozen=True, slots=True) +class CheckDecision: + """What ``check`` concluded from one snapshot, and the write that must follow it. + + ``remaining`` is the answer (seconds still locked, ``0.0`` = allowed); + ``discard`` asks the store to clear the snapshot's record (served or + released early); ``commit_duration`` asks it to store the live duration + on the still-active lock. At most one of the two writes is set. Stores + apply the write with a compare-and-set against the snapshot it was + decided on, so a record a peer changed meanwhile is never clobbered. + """ + + remaining: float + discard: bool = False + commit_duration: float | None = None + + +def evaluate_check(record: LoginThrottleRecord, *, max_attempts: int, lockout_seconds: float, now: float) -> CheckDecision: + """The ``check`` contract on one snapshot, shared by every store (see the module docstring).""" + if record.fail_count < max_attempts: + return CheckDecision(0.0) + if not record.locked: + # Over the current threshold but the lock never started under the + # threshold these failures accumulated under (the operator tightened + # max_login_attempts mid-count). Keep the record: the next failure + # starts the lock and a successful login clears it — deleting here + # would hand the IP a fresh budget under a stricter policy. + return CheckDecision(0.0) + if now >= record.expires_at: + # The lock served the full sentence of the duration in force when it + # started — a later duration increase must not resurrect it. + return CheckDecision(0.0, discard=True) + if now < record.locked_at + lockout_seconds: + # Still locked. The sentence now follows the current duration, and that + # evaluation is committed — including decreases — so the stored sentence + # always matches the policy the lock was last evaluated under; a later + # raise can never resurrect time the lock already served under a shorter + # policy. + commit = lockout_seconds if lockout_seconds != record.lock_duration else None + return CheckDecision(record.locked_at + lockout_seconds - now, commit_duration=commit) + # Original sentence still running, but the current (lowered) duration has + # already elapsed — release early. + return CheckDecision(0.0, discard=True) + + +class LoginThrottleStore(Protocol): + """Async per-IP failed-login counter shared by every login replica using it.""" + + async def get(self, ip: str) -> LoginThrottleRecord | None: + """Return the record for ``ip`` without mutating anything, or ``None`` for a clean IP.""" + ... + + async def check(self, ip: str, *, policy: LoginThrottlePolicy, now: float | None = None) -> float: + """Return the seconds the IP stays locked under the resolved policy, ``0.0`` when it may log in. + + ``policy`` is awaited at most once and never for a clean IP. Clears + served / released locks as a side effect (see the module docstring). + """ + ... + + async def record_failure(self, ip: str, *, max_attempts: int, lockout_seconds: float, now: float | None = None) -> LoginThrottleRecord: + """Count one failed login under the given policy and return the new record.""" + ... + + async def reset(self, ip: str) -> None: + """Forget the IP after a successful login.""" + ... diff --git a/backend/packages/harness/deerflow/persistence/login_throttle/memory.py b/backend/packages/harness/deerflow/persistence/login_throttle/memory.py new file mode 100644 index 0000000000..d386b3211d --- /dev/null +++ b/backend/packages/harness/deerflow/persistence/login_throttle/memory.py @@ -0,0 +1,116 @@ +"""In-process failed-login counter — one per Gateway process, not shared. + +This is the historical ``app.gateway.routers.auth._login_attempts`` dict moved +behind :class:`~deerflow.persistence.login_throttle.base.LoginThrottleStore`. +With N Gateway replicas behind one load balancer each replica keeps its own +counter, so an attacker effectively gets N × ``max_login_attempts`` guesses +and a lockout on one replica is invisible to the others; use the SQL store +whenever an application database exists (``auth.local.throttle_storage``). +""" + +from __future__ import annotations + +import time +from dataclasses import replace + +from deerflow.persistence.login_throttle.base import LoginThrottlePolicy, LoginThrottleRecord, evaluate_check + +#: Upper bound on tracked IPs before the capacity sweep runs (historical constant). +MAX_TRACKED_IPS = 10000 + + +class MemoryLoginThrottleStore: + """Process-local counter with a bounded tracked-IP set.""" + + def __init__(self, *, max_tracked_ips: int = MAX_TRACKED_IPS) -> None: + self._records: dict[str, LoginThrottleRecord] = {} + self._max_tracked_ips = max_tracked_ips + + async def get(self, ip: str) -> LoginThrottleRecord | None: + return self._records.get(ip) + + async def check(self, ip: str, *, policy: LoginThrottlePolicy, now: float | None = None) -> float: + """Apply the shared ``check`` contract to this process's record. + + The policy is resolved lazily: a clean IP returns before ``policy`` is + awaited. That await is a yield point, so the record is read again + afterwards and the decision is made on that fresh snapshot — a record + a concurrent request cleared or replaced meanwhile is never judged + from the stale probe. The compare-and-set that follows cannot miss: + there is no ``await`` between the fresh read and the write and the + event loop thread holds the GIL, so no peer can change the record in + between — unlike the SQL store, whose writes race other replicas and + therefore check their affected-row count and re-evaluate. The guards + are kept only as a statement of the same discipline. + """ + if ip not in self._records: + return 0.0 + max_attempts, lockout_seconds = await policy() + record = self._records.get(ip) # fresh: the policy read yielded the loop + if record is None: + return 0.0 + now = time.time() if now is None else now + decision = evaluate_check(record, max_attempts=max_attempts, lockout_seconds=lockout_seconds, now=now) + if decision.discard: + self._discard(ip, record) + elif decision.commit_duration is not None and self._records.get(ip) == record: + self._records[ip] = replace(record, lock_duration=decision.commit_duration) + return decision.remaining + + async def record_failure(self, ip: str, *, max_attempts: int, lockout_seconds: float, now: float | None = None) -> LoginThrottleRecord: + now = time.time() if now is None else now + self._sweep_if_full(now, keep=ip) + record = self._records.get(ip) + if record is None: + new_record = LoginThrottleRecord(fail_count=1) + else: + new_count = record.fail_count + 1 + if new_count < max_attempts: + # Below the (possibly raised) threshold: counting, any lock is cleared. + new_record = LoginThrottleRecord(fail_count=new_count) + elif record.locked and now < record.expires_at: + # A failure during an active lock keeps the lock's start and its + # committed duration: the sentence is "N seconds after the lock + # started", not "after the last attempt" (#6501 review). + new_record = LoginThrottleRecord(fail_count=new_count, locked_at=record.locked_at, lock_duration=record.lock_duration) + else: + # Never locked, or the lock already served its sentence: start anew. + new_record = LoginThrottleRecord(fail_count=new_count, locked_at=now, lock_duration=lockout_seconds) + self._records[ip] = new_record + return new_record + + async def reset(self, ip: str) -> None: + self._records.pop(ip, None) + + def _discard(self, ip: str, snapshot: LoginThrottleRecord) -> None: + # Compare-and-delete: only remove the record the decision was made on. + if self._records.get(ip) == snapshot: + del self._records[ip] + + def _sweep_if_full(self, now: float, *, keep: str) -> None: + """Evict expired lockouts when the dict grows too large. + + ``keep`` is the IP whose failure is being recorded: its record is live + and ``record_failure`` decides its fate from its own values (a served + lock starts a fresh sentence), the same contract the SQL store keeps + by excluding the upserted row from its sweep. + + Expiry is a property of each record's own committed sentence — ``locked + and now >= expires_at`` — independent of the live threshold: a record + locked under an old, lower threshold must still be swept once its + sentence is served, even if the current max has moved past its count. + Gating on the current threshold would retain expired records while the + capacity fallback below evicts live counters (they sort first), + granting active offenders fresh budgets. + """ + if len(self._records) < self._max_tracked_ips: + return + for key in [k for k, record in self._records.items() if k != keep and record.locked and now >= record.expires_at]: + del self._records[key] + # If still too large, evict the cheapest-to-lose half ordered by each + # record's own expiry: never-locked counters (expires_at == 0.0) first, + # then locked records whose committed sentence expires earliest. + if len(self._records) >= self._max_tracked_ips: + by_expiry = sorted(((k, record) for k, record in self._records.items() if k != keep), key=lambda kv: kv[1].expires_at) + for key, _ in by_expiry[: len(by_expiry) // 2]: + del self._records[key] diff --git a/backend/packages/harness/deerflow/persistence/login_throttle/model.py b/backend/packages/harness/deerflow/persistence/login_throttle/model.py new file mode 100644 index 0000000000..b9a958eaad --- /dev/null +++ b/backend/packages/harness/deerflow/persistence/login_throttle/model.py @@ -0,0 +1,39 @@ +"""ORM model for the shared failed-login counter (``login_throttle``). + +One row per client IP that failed at least one local login. Replicas sharing +the application database read and write the same row, so a lockout is +enforced wherever the load balancer sends the next attempt. Rows are swept +by :class:`~deerflow.persistence.login_throttle.sql.SqlLoginThrottleStore` +once a lock has served its sentence or a never-locked counter went stale. +""" + +from __future__ import annotations + +from datetime import UTC, datetime + +from sqlalchemy import DateTime, Double, Index, Integer, PrimaryKeyConstraint, String +from sqlalchemy.orm import Mapped, mapped_column + +from deerflow.persistence.base import Base + +#: Longest row key. IPv6 fits in 45 characters; a trusted proxy may forward a +#: longer ``X-Real-IP`` and the store truncates it to this length. +LOGIN_THROTTLE_IP_LENGTH = 255 + + +class LoginThrottleRow(Base): + __tablename__ = "login_throttle" + + ip: Mapped[str] = mapped_column(String(LOGIN_THROTTLE_IP_LENGTH), nullable=False) + fail_count: Mapped[int] = mapped_column(Integer, nullable=False) + # Epoch seconds the lock started; NULL while the IP is only counting. + locked_at: Mapped[float | None] = mapped_column(Double, nullable=True) + # Sentence committed for the lock (follows the live policy while active). + lock_duration_seconds: Mapped[float | None] = mapped_column(Double, nullable=True) + updated_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False, default=lambda: datetime.now(UTC)) + + __table_args__ = ( + PrimaryKeyConstraint("ip", name="pk_login_throttle"), + # Serves the sweep's stale-counter predicate (``locked_at IS NULL AND updated_at <= cutoff``). + Index("ix_login_throttle_updated_at", "updated_at"), + ) diff --git a/backend/packages/harness/deerflow/persistence/login_throttle/sql.py b/backend/packages/harness/deerflow/persistence/login_throttle/sql.py new file mode 100644 index 0000000000..9244dee536 --- /dev/null +++ b/backend/packages/harness/deerflow/persistence/login_throttle/sql.py @@ -0,0 +1,286 @@ +"""SQL-backed failed-login counter shared across Gateway replicas. + +One ``login_throttle`` row per client IP in the application database (SQLite +or PostgreSQL). Every replica that shares the database sees the same row, so +``max_login_attempts`` is enforced once per IP instead of once per process. + +Concurrency contract: + +- ``record_failure`` is one dialect-native upsert (``INSERT ... ON CONFLICT + (ip) DO UPDATE``) that increments the counter and starts the lock in the + same statement, so racing failures never lose an increment and the + "count reached ``max_attempts``" decision is made on the row's own value, + not on a value read earlier. +- ``check`` works in one session (one pool checkout): it probes the row and + returns for a clean IP before resolving the policy, resolves the policy + exactly once otherwise, then re-reads the row — the policy read is a yield + point and a decision that needs no write has no compare-and-set to catch a + row a peer changed meanwhile — and applies its decision (clear a served + lock, commit a changed sentence) with a compare-and-set predicate on the + snapshot it decided on, in a *separate* write transaction. + Keeping the read transaction out of the write avoids SQLite's read→write + upgrade (``SQLITE_BUSY`` / ``BUSY_SNAPSHOT`` under contention) while the + predicate guarantees a racing success or failure is never clobbered — the + same compare-and-set discipline the memory store uses on its dict. Every + such write checks its affected-row count: a miss means a peer changed the + row between the read and the write (a failure incremented the count, a + success cleared it, a fresh lock started), and the decision is *not* taken + as applied — the row is re-read and re-evaluated, up to + :data:`CHECK_CAS_ATTEMPTS` times, so an extension or decrease of the + sentence that ``check`` reports is also what the row stores, a cleared row + is reported as allowed, and a fresh peer lock is reported as locked (#6501 + review). Exhausting the bound under sustained contention fails closed: + nothing is written and the answer is the last snapshot's lock under the + live policy. +- Cleanup is amortized into ``record_failure``: a bounded ``DELETE`` removes + locks whose sentence has elapsed and never-locked counters idle for + :data:`STALE_COUNTER_SECONDS` (the shared-table equivalent of the memory + store's capacity eviction) so the table cannot grow without bound. It runs + in the upsert's own transaction — one commit (one WAL flush on PostgreSQL) + per failed login, not two; the ``keep`` exclusion protects the row being + recorded — and ``ix_login_throttle_updated_at`` serves its stale-counter + predicate, while the served-lock predicate (``locked_at + + lock_duration_seconds <= now``) scans a table the sweep itself keeps + small. The candidates come from an ``IN (SELECT ... LIMIT n)`` subquery bound to the + statement snapshot, and the same expiry predicate is repeated on the + ``DELETE`` target: PostgreSQL READ COMMITTED re-evaluates only the + statement's *own* WHERE on a row it had to wait for, so without the outer + predicate a lock that a concurrent ``check`` extended (or a stale counter a + concurrent failure re-armed) between the snapshot and the row lock would + still be deleted. Every other write (``_discard``, the ``check`` UPDATE) + already carries its full compare-and-set predicate on the target row for + the same reason; ``reset`` is unconditional by design. + +A failure recorded while a lock is active keeps the lock's ``locked_at`` and +committed duration (the sentence is "N seconds after the lock started", not +"after the last attempt"); only a never-locked or already-served row gets a +fresh stamp. The upsert decides that from the row's own values in one +statement, so it stays atomic under racing failures. + +``locked_at`` is the wall clock (``time.time()``) of whichever replica +recorded the failure, and every replica evaluates expiry against its own +clock. Replicas sharing a database must therefore run NTP: a skew of a few +seconds shifts lock evaluation by that much, which is acceptable for +lockouts measured in minutes but is the inherent cost of a wall-clock +lockout shared across hosts. + +Housekeeping constants: the shared table, unlike the memory store's bounded +dict, has no implicit size limit, so every ``record_failure`` sweeps. +:data:`STALE_COUNTER_SECONDS` is 24 hours — the same guard the upload-staging +and project-document sweeps use for leftovers that may still be in use on +another replica — long enough that a legitimate user's typos a day apart are +not chained together, short enough to keep scanner one-offs from piling up. +:data:`SWEEP_BATCH_SIZE` (200) bounds the rows one failed login may delete, +so the write transaction stays short even if a backlog exists. + +Database errors propagate: the users table lives in the same database, so a +login cannot succeed without it anyway, and failing closed keeps the throttle +from silently handing out unlimited verification. +""" + +from __future__ import annotations + +import logging +import time +from datetime import UTC, datetime, timedelta + +from sqlalchemy import and_, case, delete, or_, select, update +from sqlalchemy.dialects.postgresql import insert as pg_insert +from sqlalchemy.dialects.sqlite import insert as sqlite_insert +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from deerflow.persistence.login_throttle.base import LoginThrottlePolicy, LoginThrottleRecord, evaluate_check +from deerflow.persistence.login_throttle.model import LOGIN_THROTTLE_IP_LENGTH, LoginThrottleRow + +logger = logging.getLogger(__name__) + +#: Never-locked counters untouched for this long are dropped by the sweep. +STALE_COUNTER_SECONDS = 24 * 60 * 60 +#: Rows one ``record_failure`` call may delete; bounds the write transaction. +SWEEP_BATCH_SIZE = 200 +#: Snapshot reads one ``check`` may make before it stops retrying a missed compare-and-set. +CHECK_CAS_ATTEMPTS = 3 + + +def _insert_for(session: AsyncSession): + dialect = session.get_bind().dialect.name + if dialect == "postgresql": + return pg_insert + if dialect == "sqlite": + return sqlite_insert + raise ValueError(f"Unsupported login throttle database dialect: {dialect}") + + +def _record(fail_count: int, locked_at: float | None, lock_duration: float | None) -> LoginThrottleRecord: + return LoginThrottleRecord(fail_count=int(fail_count), locked_at=float(locked_at or 0.0), lock_duration=float(lock_duration or 0.0)) + + +def _key(ip: str) -> str: + """Bound the row key to the column length; a trusted proxy may forward a token-like ``X-Real-IP``. + + The key itself is never logged: it may be an identifier the proxy did not + mean to expose, and it is unbounded. Truncation keeps the throttle + fail-closed (colliding prefixes share a stricter budget). + """ + if len(ip) <= LOGIN_THROTTLE_IP_LENGTH: + return ip + logger.warning("Login throttle client key of %d characters truncated to %d; check the trusted proxy's X-Real-IP value.", len(ip), LOGIN_THROTTLE_IP_LENGTH) + return ip[:LOGIN_THROTTLE_IP_LENGTH] + + +def _timestamp(now: float) -> datetime: + return datetime.fromtimestamp(now, UTC) + + +class SqlLoginThrottleStore: + """Persistence facade for ``login_throttle``.""" + + def __init__(self, session_factory: async_sessionmaker[AsyncSession]) -> None: + self._sf = session_factory + + async def get(self, ip: str) -> LoginThrottleRecord | None: + async with self._sf() as session: + row = (await session.execute(self._select(_key(ip)))).first() + return None if row is None else _record(*row) + + async def check(self, ip: str, *, policy: LoginThrottlePolicy, now: float | None = None) -> float: + key = _key(ip) + now = time.time() if now is None else now + async with self._sf() as session: + # Probe first: a clean IP never pays the policy read (the router's + # config.yaml stat + hash) nor a second query. + record = await self._read_snapshot(session, key) + if record is None: + return 0.0 + max_attempts, lockout_seconds = await policy() + # The policy read yielded the loop; decide on a snapshot taken after + # it (a decision that needs no write has no compare-and-set to catch + # a row a peer changed meanwhile). Same session, same pool checkout. + for _attempt in range(CHECK_CAS_ATTEMPTS): + record = await self._read_snapshot(session, key) + if record is None: + return 0.0 + decision = evaluate_check(record, max_attempts=max_attempts, lockout_seconds=lockout_seconds, now=now) + if decision.discard: + if await self._discard(session, key, record): + return 0.0 + elif decision.commit_duration is not None: + if await self._commit_duration(session, key, record, decision.commit_duration, now): + return decision.remaining + else: + return decision.remaining + # The compare-and-set missed: a peer changed the row between the + # snapshot and the write. Never report the stale decision as + # applied — re-read and decide again on the fresh row (gone -> + # allowed; a fresh lock -> locked; an incremented count -> the + # extension or decrease is committed on it). + # Sustained contention exhausted the bound. Nothing was written; fail + # closed on the last snapshot under the live policy. + remaining = max(0.0, record.locked_at + lockout_seconds - now) if record.locked else 0.0 + logger.debug("Login throttle check gave up after %d compare-and-set misses on a contended row; reporting %.1fs remaining from the last snapshot without writing.", CHECK_CAS_ATTEMPTS, remaining) + return remaining + + async def record_failure(self, ip: str, *, max_attempts: int, lockout_seconds: float, now: float | None = None) -> LoginThrottleRecord: + key = _key(ip) + now = time.time() if now is None else now + stamp = _timestamp(now) + async with self._sf() as session: + # Housekeeping rides in the upsert's transaction (one commit per + # failed login); ``keep`` leaves this IP's row to the upsert below. + await session.execute(self.sweep_statement(now, keep=key)) + insert = _insert_for(session) + # One atomic decision on the row's own values (see the module + # docstring): below the threshold -> counting, lock cleared; at or + # over it during an *active* lock -> keep the lock's start and + # committed duration; otherwise (never locked / served) -> stamp now. + reaches = LoginThrottleRow.fail_count + 1 >= max_attempts + active = and_(LoginThrottleRow.locked_at.is_not(None), LoginThrottleRow.locked_at + LoginThrottleRow.lock_duration_seconds > now) + stmt = insert(LoginThrottleRow).values(ip=key, fail_count=1, locked_at=None, lock_duration_seconds=None, updated_at=stamp) + stmt = stmt.on_conflict_do_update( + index_elements=[LoginThrottleRow.ip], + set_={ + "fail_count": LoginThrottleRow.fail_count + 1, + "locked_at": case((and_(reaches, active), LoginThrottleRow.locked_at), (reaches, now), else_=None), + "lock_duration_seconds": case((and_(reaches, active), LoginThrottleRow.lock_duration_seconds), (reaches, lockout_seconds), else_=None), + "updated_at": stamp, + }, + ).returning(LoginThrottleRow.fail_count, LoginThrottleRow.locked_at, LoginThrottleRow.lock_duration_seconds) + row = (await session.execute(stmt)).one() + await session.commit() + return _record(*row) + + async def reset(self, ip: str) -> None: + async with self._sf() as session: + await session.execute(delete(LoginThrottleRow).where(LoginThrottleRow.ip == _key(ip))) + await session.commit() + + @staticmethod + def _select(key: str): + return select(LoginThrottleRow.fail_count, LoginThrottleRow.locked_at, LoginThrottleRow.lock_duration_seconds).where(LoginThrottleRow.ip == key) + + async def _read_snapshot(self, session: AsyncSession, key: str) -> LoginThrottleRecord | None: + """Read the row and end the read transaction before any write (see the module docstring).""" + row = (await session.execute(self._select(key))).first() + await session.rollback() + return None if row is None else _record(*row) + + @staticmethod + def _matches(key: str, snapshot: LoginThrottleRecord): + """Compare-and-set predicate: the row still is the one the decision was made on. + + Carried by the target row of every ``check`` write, so a database + that re-evaluates the WHERE after waiting on a concurrent writer + (PostgreSQL READ COMMITTED) sees the changed tuple and skips the + stale decision instead of applying it. + """ + return and_( + LoginThrottleRow.ip == key, + LoginThrottleRow.fail_count == snapshot.fail_count, + LoginThrottleRow.locked_at == snapshot.locked_at, + LoginThrottleRow.lock_duration_seconds == snapshot.lock_duration, + ) + + async def _discard(self, session: AsyncSession, key: str, snapshot: LoginThrottleRecord) -> bool: + """Compare-and-delete the snapshot's row; ``False`` when a peer changed it first.""" + result = await session.execute(delete(LoginThrottleRow).where(self._matches(key, snapshot))) + await session.commit() + return result.rowcount == 1 + + async def _commit_duration(self, session: AsyncSession, key: str, snapshot: LoginThrottleRecord, lock_duration: float, now: float) -> bool: + """Compare-and-set the live duration on the snapshot's row; ``False`` when a peer changed it first.""" + result = await session.execute(update(LoginThrottleRow).where(self._matches(key, snapshot)).values(lock_duration_seconds=lock_duration, updated_at=_timestamp(now))) + await session.commit() + return result.rowcount == 1 + + @staticmethod + def _expired(now: float): + """Rows the sweep may drop: served locks and never-locked counters idle past the stale window.""" + served = and_(LoginThrottleRow.locked_at.is_not(None), LoginThrottleRow.locked_at + LoginThrottleRow.lock_duration_seconds <= now) + stale = and_(LoginThrottleRow.locked_at.is_(None), LoginThrottleRow.updated_at <= _timestamp(now) - timedelta(seconds=STALE_COUNTER_SECONDS)) + return or_(served, stale) + + @classmethod + def sweep_statement(cls, now: float, *, keep: str | None = None): + """The bounded cleanup ``DELETE`` (see the module docstring's cleanup contract). + + ``keep`` is the key the calling ``record_failure`` is about to upsert: + that row is live and the upsert decides its fate from its own values + (a served lock starts a fresh sentence, a stale counter keeps counting), + exactly as the memory store does; sweeping it first would turn the + failure into a fresh ``(1, NULL, NULL)`` row and split the contract. + + The expiry predicate appears twice on purpose: once inside the + ``IN (SELECT ... LIMIT n)`` subquery that bounds the batch, and again + on the ``DELETE`` target. The subquery is evaluated against the + statement snapshot; PostgreSQL READ COMMITTED re-evaluates only the + outer WHERE on a row whose lock it had to wait for, and ``ip IN (...)`` + still holds for that old candidate — so without the repeated predicate + a lock that a concurrent ``check`` just extended (its UPDATE committed + while this DELETE waited) would be deleted and the next attempt from + that IP would reach password verification. SQLite serializes writers, + so there the repetition is merely redundant. + """ + candidates = cls._expired(now) if keep is None else and_(LoginThrottleRow.ip != keep, cls._expired(now)) + victims = select(LoginThrottleRow.ip).where(candidates).limit(SWEEP_BATCH_SIZE) + target = cls._expired(now) if keep is None else and_(LoginThrottleRow.ip != keep, cls._expired(now)) + return delete(LoginThrottleRow).where(LoginThrottleRow.ip.in_(victims), target) diff --git a/backend/packages/harness/deerflow/persistence/migrations/AGENTS.md b/backend/packages/harness/deerflow/persistence/migrations/AGENTS.md index 8e7f49e32a..18c86a2787 100644 --- a/backend/packages/harness/deerflow/persistence/migrations/AGENTS.md +++ b/backend/packages/harness/deerflow/persistence/migrations/AGENTS.md @@ -28,8 +28,8 @@ The empty-DB path keeps using `create_all` because `Base.metadata` is the only a `0026_mcp_task_lease_tokens` → `0027_notification_deliveries` → `0028_parked_attempts` → `0029_scheduler_agent_tasks` → `0030_notification_claim_tokens` → `0031_scheduled_streak_boundary` → -`0032_activity_and_task_events` → `0033_batch_result_artifact` → `0034_run_event_seq_watermark` -(current head). The preference +`0032_activity_and_task_events` → `0033_batch_result_artifact` → `0034_run_event_seq_watermark` → +`0035_login_throttle` (current head). The preference revision adds a separate owner/key table with a cascading users foreign key and does not alter users; the project-documents revision adds a new owner-scoped shelf table, and the MCP lease-token revision adds two nullable token columns to @@ -175,6 +175,7 @@ on installs that never enabled it. The convention is: - `migrations/versions/0028_parked_attempts.py` — adds `notification_deliveries.parked_attempts` so channel-down parking is capped; backfills with a temporary `server_default="0"` then drops it so the durable schema matches `create_all` (ORM Python-side `default=0` only). Chains after `0027_notification_deliveries` - `migrations/versions/0031_scheduled_streak_boundary.py` — adds nullable `scheduled_tasks.unmet_streak_after_seq` (occurrences at or below it never count toward the three-miss automatic pause; moved by goal/prompt/stop-condition/note edits, internal and never serialized) and nullable `scheduled_tasks.stop_condition` (the user's normalized "stop when …" rule, appended to the run message only at launch). Two nullable columns, so the bootstrap forward-compat floor is unchanged; chains after `0030_notification_claim_tokens`. Test: `tests/test_migration_0031_scheduled_streak_boundary.py` - `migrations/versions/0032_activity_and_task_events.py` — all schema of the scheduled-signals change in one revision: nullable `runs.origin_kind` (server-owned run origin; never backfilled, so legacy runs stay NULL, the "interactive or unknown" value) plus `ix_runs_thread_change_seq`, the per-user `thread_read_markers` (`(user_id, thread_id)` key, `seen_change_seq`) and `thread_read_versions` (`user_id` key, `version`) tables, and `scheduled_task_events` (lifecycle rows for the originating chat, unique on `(task_id, anchor, event)` as `uq_scheduled_task_event`, indexed on `(thread_id, created_at)` and `user_id`). `_helpers.py` guards only columns, so the revision guards every table and index on its own with a fresh inspector in both directions: a partially applied upgrade completes on the next start and an interrupted downgrade can run again. `RunRow` declares the index and column, and both new models are registered in `persistence/models`, because the empty-DB bootstrap uses `create_all` + `stamp head`. New tables and a nullable column, so the bootstrap forward-compat floor is unchanged; chains after `0031_scheduled_streak_boundary`. The `runs` index is built without `CONCURRENTLY` (a brief write block on large Postgres tables) and the SQLite downgrade rebuilds `runs`. Test: `tests/test_migration_0032_activity_and_task_events.py` +- `migrations/versions/0035_login_throttle.py` — creates `login_throttle` (`ip` PK, `fail_count`, nullable epoch `locked_at`, nullable `lock_duration_seconds`, `updated_at` + `ix_login_throttle_updated_at` for the sweep's stale-counter predicate), the shared failed-login counter behind `POST /api/v1/auth/login/local` selected by `auth.local.throttle_storage` (`persistence/login_throttle/`: `SqlLoginThrottleStore` increments and starts the lock in one dialect-native upsert, applies check decisions with compare-and-set predicates in a separate write transaction, and sweeps served locks plus 24h-idle counters in bounded batches — the sweep's `DELETE` repeats the expiry predicate on its target, not only `ip IN (SELECT … LIMIT n)`, because PostgreSQL READ COMMITTED re-evaluates only the statement's own WHERE on a row it waited for and would otherwise delete a lock a concurrent `check` just extended). Table create/drop is guarded with a fresh inspector in both directions. New table, so the bootstrap forward-compat floor is unchanged; chains after `0034_run_event_seq_watermark`. Test: `tests/test_migration_0035_login_throttle.py` - `persistence/bootstrap.py` — `bootstrap_schema(engine, backend=...)`, the three-branch provisioning decision, locked revision validation, and the narrow 0019 forward-compatibility exception - `extensions/loader.py::load_extensions` — registers each spec's `table_prefix` with `register_extension_table_prefix()` - Tests: `tests/test_persistence_bootstrap.py` (branches), `tests/test_persistence_bootstrap_concurrency.py` (concurrency), `tests/test_persistence_bootstrap_regression.py` (issue #3682), `tests/test_persistence_migrations_env.py` (filter, including extension-owned tables), `tests/test_extension_loader.py::TestTablePrefixRegistration` (spec-to-filter wiring), `tests/blocking_io/test_persistence_bootstrap.py` (asyncio.to_thread anchor), `tests/test_migration_0004_run_ownership_dedupe.py` + `tests/test_migration_0007_scheduled_run_active_dedupe.py` (dedupe-before-unique-index pre-steps), `tests/test_migration_0025_repair_run_change_seq.py` (issue #5516 skipped-revision heal) diff --git a/backend/packages/harness/deerflow/persistence/migrations/versions/0035_login_throttle.py b/backend/packages/harness/deerflow/persistence/migrations/versions/0035_login_throttle.py new file mode 100644 index 0000000000..742d6b3e14 --- /dev/null +++ b/backend/packages/harness/deerflow/persistence/migrations/versions/0035_login_throttle.py @@ -0,0 +1,68 @@ +"""Shared failed-login counter for cross-replica lockouts. + +Revision ID: 0035_login_throttle +Revises: 0034_run_event_seq_watermark + +Creates ``login_throttle``: one row per client IP that failed a local login, +keyed by ``ip``, with the consecutive failure count, the epoch timestamp the +lock started (NULL while only counting), the sentence committed for that lock +and ``updated_at``. Every Gateway replica sharing the application database +reads and writes the same row (``auth.local.throttle_storage``), replacing the +per-process dict that gave an attacker N x ``max_login_attempts`` guesses +behind a load balancer. + +``_helpers.py`` only guards columns, so the table is guarded with a fresh +inspector in both directions: an upgrade that finds the table (an interrupted +earlier attempt, or an empty-database bootstrap that already ran +``create_all``) completes, and a downgrade after the table is gone still +moves the version row. +""" + +from __future__ import annotations + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +revision: str = "0035_login_throttle" +down_revision: str | Sequence[str] | None = "0034_run_event_seq_watermark" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + +_TABLE = "login_throttle" +_INDEX = "ix_login_throttle_updated_at" + + +def _has_table() -> bool: + return sa.inspect(op.get_bind()).has_table(_TABLE) + + +def _has_index() -> bool: + if not _has_table(): + return False + return _INDEX in {index["name"] for index in sa.inspect(op.get_bind()).get_indexes(_TABLE)} + + +def upgrade() -> None: + if not _has_table(): + op.create_table( + _TABLE, + sa.Column("ip", sa.String(length=255), nullable=False), + sa.Column("fail_count", sa.Integer(), nullable=False), + sa.Column("locked_at", sa.Double(), nullable=True), + sa.Column("lock_duration_seconds", sa.Double(), nullable=True), + sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False), + sa.PrimaryKeyConstraint("ip", name="pk_login_throttle"), + ) + # Serves the sweep's stale-counter predicate; guarded separately so a + # table left behind by an interrupted attempt still gets its index. + if not _has_index(): + op.create_index(_INDEX, _TABLE, ["updated_at"]) + + +def downgrade() -> None: + if _has_index(): + op.drop_index(_INDEX, table_name=_TABLE) + if _has_table(): + op.drop_table(_TABLE) diff --git a/backend/packages/harness/deerflow/persistence/models/__init__.py b/backend/packages/harness/deerflow/persistence/models/__init__.py index 825ee5db59..28cd9236ca 100644 --- a/backend/packages/harness/deerflow/persistence/models/__init__.py +++ b/backend/packages/harness/deerflow/persistence/models/__init__.py @@ -22,6 +22,7 @@ from deerflow.persistence.channel_connections.model import ( ChannelOAuthStateRow, ) from deerflow.persistence.feedback.model import FeedbackRow +from deerflow.persistence.login_throttle.model import LoginThrottleRow from deerflow.persistence.managed_subagents.model import ManagedSubagentRow from deerflow.persistence.mcp_tasks.model import McpTaskRow from deerflow.persistence.models.run_event import RunEventRow, RunEventThreadSeqRow @@ -45,6 +46,7 @@ __all__ = [ "ChannelCredentialRow", "ChannelOAuthStateRow", "FeedbackRow", + "LoginThrottleRow", "McpTaskRow", "ManagedSubagentRow", "NotificationDeliveryRow", diff --git a/backend/tests/blocking_io/test_login_local_throttle.py b/backend/tests/blocking_io/test_login_local_throttle.py index 61dc48dd37..8629dd9c03 100644 --- a/backend/tests/blocking_io/test_login_local_throttle.py +++ b/backend/tests/blocking_io/test_login_local_throttle.py @@ -7,31 +7,69 @@ the policy — including an already-locked attacker flooding the endpoint on the way to its 429. Both resolution points (the rate-limit check and the failure recorder) offload via ``asyncio.to_thread``; if either regresses onto the event loop, the strict Blockbuster gate raises ``BlockingError``. + +The counter itself lives behind ``LoginThrottleStore``. The SQL store runs +every statement through the async session (aiosqlite / asyncpg), so the same +anchors run against it too: a synchronous driver call or file access sneaking +into the store would trip the gate here. The engine is built off the loop, +because ``create_async_engine`` resolves the SQLite path synchronously — test +fixture work, not the production path under test. """ from __future__ import annotations -import time +import asyncio +from collections.abc import AsyncIterator +from pathlib import Path import pytest +import pytest_asyncio +import sqlalchemy as sa from fastapi import HTTPException from fastapi.responses import Response from fastapi.security import OAuth2PasswordRequestForm +from sqlalchemy.ext.asyncio import AsyncEngine, async_sessionmaker, create_async_engine from starlette.requests import Request +# Pre-import so the router's lazy imports are cached no-ops under the gate. +import deerflow.persistence.models # noqa: F401 +from app.gateway.auth import login_throttle from app.gateway.routers import auth as auth_router +from deerflow.persistence.base import Base +from deerflow.persistence.login_throttle import LoginThrottleRow, LoginThrottleStore, MemoryLoginThrottleStore, SqlLoginThrottleStore pytestmark = pytest.mark.asyncio _CLIENT_IP = "203.0.113.9" +_POLICY = {"max_attempts": 5, "lockout_seconds": 3600.0} -@pytest.fixture(autouse=True) -def _throttle_state(monkeypatch): +def _build_sqlite_engine(path: Path) -> AsyncEngine: + """Create the schema with a sync engine and return the async one (runs in a worker thread).""" + sync_engine = sa.create_engine(f"sqlite:///{path.as_posix()}") + try: + Base.metadata.create_all(sync_engine, tables=[LoginThrottleRow.__table__]) + finally: + sync_engine.dispose() + return create_async_engine(f"sqlite+aiosqlite:///{path.as_posix()}") + + +@pytest_asyncio.fixture(params=["memory", "sql"]) +async def store(request, tmp_path, monkeypatch) -> AsyncIterator[LoginThrottleStore]: monkeypatch.delenv("AUTH_TRUSTED_PROXIES", raising=False) - auth_router._login_attempts.clear() - yield - auth_router._login_attempts.clear() + engine: AsyncEngine | None = None + if request.param == "memory": + store: LoginThrottleStore = MemoryLoginThrottleStore() + else: + engine = await asyncio.to_thread(_build_sqlite_engine, tmp_path / "throttle.db") + store = SqlLoginThrottleStore(async_sessionmaker(engine, expire_on_commit=False)) + login_throttle.install_login_throttle_store(store) + try: + yield store + finally: + login_throttle.reset_login_throttle_store() + if engine is not None: + await engine.dispose() def _request() -> Request: @@ -52,10 +90,11 @@ def _form() -> OAuth2PasswordRequestForm: return OAuth2PasswordRequestForm(username="user@example.com", password="wrong") -async def test_locked_ip_policy_resolution_does_not_block_loop() -> None: +async def test_locked_ip_policy_resolution_does_not_block_loop(store) -> None: """A locked IP floods the endpoint: every request resolves the policy on the way to 429, and that resolution must stay off the event loop.""" - auth_router._login_attempts[_CLIENT_IP] = (9, time.time(), 3600.0) # sentence running + for _ in range(_POLICY["max_attempts"]): + await store.record_failure(_CLIENT_IP, **_POLICY) # sentence running with pytest.raises(HTTPException) as exc_info: await auth_router.login_local(_request(), Response(), _form(), remember_me=True) @@ -63,7 +102,7 @@ async def test_locked_ip_policy_resolution_does_not_block_loop() -> None: assert exc_info.value.status_code == 429 -async def test_failed_login_recording_does_not_block_loop(monkeypatch) -> None: +async def test_failed_login_recording_does_not_block_loop(store, monkeypatch) -> None: """The wrong-password path resolves the policy again inside the recorder; counting must happen without blocking IO on the loop.""" @@ -72,10 +111,42 @@ async def test_failed_login_recording_does_not_block_loop(monkeypatch) -> None: return None monkeypatch.setattr(auth_router, "get_local_provider", lambda: _Provider()) - auth_router._login_attempts[_CLIENT_IP] = (1, 0.0, 0.0) # counting, not locked + await store.record_failure(_CLIENT_IP, **_POLICY) # counting, not locked with pytest.raises(HTTPException) as exc_info: await auth_router.login_local(_request(), Response(), _form(), remember_me=True) assert exc_info.value.status_code == 401 - assert auth_router._login_attempts[_CLIENT_IP][0] == 2 + assert (await store.get(_CLIENT_IP)).fail_count == 2 + + +async def test_successful_login_reset_does_not_block_loop(store, monkeypatch) -> None: + """The success path clears the counter through the store on the loop.""" + from app.gateway.auth.config import AuthConfig, set_auth_config + from app.gateway.auth.models import User + + class _Provider: + async def authenticate(self, credentials): + return User(email="user@example.com") + + set_auth_config(AuthConfig(jwt_secret="blocking-io-throttle-test-secret")) + monkeypatch.setattr(auth_router, "get_local_provider", lambda: _Provider()) + await store.record_failure(_CLIENT_IP, **_POLICY) + + result = await auth_router.login_local(_request(), Response(), _form(), remember_me=True) + + assert result.expires_in > 0 + assert await store.get(_CLIENT_IP) is None + + +async def test_store_reset_and_probe_do_not_block_loop(store) -> None: + """Store-level anchor for the success path: ``reset`` (DELETE) and the + ``get`` probe run through the async session with no sync driver call or + file access on the loop — the SQL variant is the one that matters.""" + await store.record_failure(_CLIENT_IP, **_POLICY) + assert (await store.get(_CLIENT_IP)).fail_count == 1 + + await store.reset(_CLIENT_IP) + + assert await store.get(_CLIENT_IP) is None + await store.reset(_CLIENT_IP) # idempotent on a clean IP diff --git a/backend/tests/test_auth.py b/backend/tests/test_auth.py index 152542df03..876ade42a0 100644 --- a/backend/tests/test_auth.py +++ b/backend/tests/test_auth.py @@ -6,6 +6,7 @@ from uuid import UUID, uuid4 import bcrypt import pytest +import pytest_asyncio from fastapi import FastAPI, HTTPException from fastapi.testclient import TestClient @@ -1173,77 +1174,171 @@ def test_login_response_includes_needs_setup(): # ── Rate Limiting ────────────────────────────────────────────────────────── +# +# The router counts failed logins through a ``LoginThrottleStore`` resolved +# once per process (``app.gateway.auth.login_throttle``). These tests install +# a store through that hook instead of poking router state, and run the +# router helpers against both the in-process counter and the shared SQL table +# so the two cannot drift apart. + +_THROTTLE_T0 = 1_700_000_000.0 +_THROTTLE_429_DETAIL = "Too many login attempts. Try again later." + + +def _install_throttle_store(store): + from app.gateway.auth import login_throttle + + login_throttle.install_login_throttle_store(store) + + +@pytest.fixture +def memory_throttle_store(): + """A fresh in-process counter installed as the router's store.""" + from app.gateway.auth import login_throttle + from deerflow.persistence.login_throttle import MemoryLoginThrottleStore + + store = MemoryLoginThrottleStore() + login_throttle.install_login_throttle_store(store) + try: + yield store + finally: + login_throttle.reset_login_throttle_store() + + +@pytest_asyncio.fixture(params=["memory", "sql"]) +async def throttle_store(request, tmp_path): + """The router's store, parametrized over both implementations.""" + from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine + + from app.gateway.auth import login_throttle + from deerflow.persistence.base import Base + from deerflow.persistence.login_throttle import LoginThrottleRow, MemoryLoginThrottleStore, SqlLoginThrottleStore + + engine = None + if request.param == "memory": + store = MemoryLoginThrottleStore() + else: + engine = create_async_engine(f"sqlite+aiosqlite:///{tmp_path / 'throttle.db'}") + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all, tables=[LoginThrottleRow.__table__]) + store = SqlLoginThrottleStore(async_sessionmaker(engine, expire_on_commit=False)) + login_throttle.install_login_throttle_store(store) + try: + yield store + finally: + login_throttle.reset_login_throttle_store() + if engine is not None: + await engine.dispose() + + +@pytest.fixture +def throttle_policy(): + """Set ``auth.local`` throttle knobs on the live app config; reset afterwards.""" + from deerflow.config.app_config import AppConfig, reset_app_config, set_app_config + from deerflow.config.auth_config import AuthAppConfig, LocalAuthConfig + from deerflow.config.sandbox_config import SandboxConfig + + def _set(*, max_attempts: int = 5, lockout_seconds: float = 60.0) -> None: + set_app_config( + AppConfig( + sandbox=SandboxConfig(use="test"), + auth=AuthAppConfig(local=LocalAuthConfig(max_login_attempts=max_attempts, lockout_seconds=lockout_seconds)), + ) + ) + + try: + yield _set + finally: + reset_app_config() + + +class _MutateDuringPolicyResolution: + """Store wrapper that mutates the record while ``check`` is suspended on the policy read. + + ``check`` probes the record (cheap clean-IP skip), resolves the policy — + an off-loop config read, so a yield point — and must then decide on a + fresh read. This wrapper runs ``mutation`` right after the policy callable + returns, standing in for a concurrent request that lands during the + resolution. + """ + + def __init__(self, inner, mutation): + self._inner = inner + self._mutation = mutation + + async def get(self, ip): + return await self._inner.get(ip) + + async def check(self, ip, *, policy, now=None): + async def racing_policy(): + resolved = await policy() + await self._mutation(self._inner) + return resolved + + return await self._inner.check(ip, policy=racing_policy, now=now) + + async def record_failure(self, ip, **kwargs): + return await self._inner.record_failure(ip, **kwargs) + + async def reset(self, ip): + await self._inner.reset(ip) @pytest.mark.asyncio -async def test_rate_limiter_allows_under_limit(): +async def test_rate_limiter_allows_under_limit(throttle_store): """Requests under the limit are allowed.""" - from app.gateway.routers.auth import _check_rate_limit, _login_attempts + from app.gateway.routers.auth import _check_rate_limit - _login_attempts.clear() await _check_rate_limit("192.168.1.1") # Should not raise @pytest.mark.asyncio -async def test_rate_limiter_blocks_after_max_failures(): - """IP is blocked after 5 consecutive failures.""" - from app.gateway.routers.auth import _check_rate_limit, _login_attempts, _record_login_failure +async def test_rate_limiter_blocks_after_max_failures(throttle_store): + """IP is blocked after 5 consecutive failures, with the historical 429 message.""" + from app.gateway.routers.auth import _check_rate_limit, _record_login_failure - _login_attempts.clear() ip = "10.0.0.1" for _ in range(5): await _record_login_failure(ip) with pytest.raises(HTTPException) as exc_info: await _check_rate_limit(ip) assert exc_info.value.status_code == 429 + assert exc_info.value.detail == _THROTTLE_429_DETAIL @pytest.mark.asyncio -async def test_rate_limiter_resets_on_success(): +async def test_rate_limiter_resets_on_success(throttle_store): """Successful login clears the failure counter.""" - from app.gateway.routers.auth import _check_rate_limit, _login_attempts, _record_login_failure, _record_login_success + from app.gateway.routers.auth import _check_rate_limit, _record_login_failure, _record_login_success - _login_attempts.clear() ip = "10.0.0.2" for _ in range(4): await _record_login_failure(ip) - _record_login_success(ip) + await _record_login_success(ip) await _check_rate_limit(ip) # Should not raise + assert await throttle_store.get(ip) is None @pytest.mark.asyncio -async def test_rate_limiter_honors_configured_attempts_and_lockout(monkeypatch): +async def test_rate_limiter_honors_configured_attempts_and_lockout(throttle_store, throttle_policy, monkeypatch): """auth.local.max_login_attempts / lockout_seconds drive the throttle policy.""" from app.gateway.routers import auth as auth_router - from app.gateway.routers.auth import _check_rate_limit, _login_attempts, _record_login_failure - from deerflow.config.app_config import AppConfig, reset_app_config, set_app_config - from deerflow.config.auth_config import AuthAppConfig, LocalAuthConfig - from deerflow.config.sandbox_config import SandboxConfig + from app.gateway.routers.auth import _check_rate_limit, _record_login_failure - _login_attempts.clear() - set_app_config( - AppConfig( - sandbox=SandboxConfig(use="test"), - auth=AuthAppConfig(local=LocalAuthConfig(max_login_attempts=2, lockout_seconds=60.0)), - ) - ) - try: - ip = "10.0.0.3" - await _record_login_failure(ip) - await _check_rate_limit(ip) # 1 failure < 2: allowed - await _record_login_failure(ip) - with pytest.raises(HTTPException) as exc_info: - await _check_rate_limit(ip) - assert exc_info.value.status_code == 429 - # The lockout window comes from lockout_seconds (60s), not the 300s - # default: at locked_at + 61 the lock must already be released. - _, locked_at, _ = _login_attempts[ip] - monkeypatch.setattr(auth_router.time, "time", lambda: locked_at + 61.0) + throttle_policy(max_attempts=2, lockout_seconds=60.0) + ip = "10.0.0.3" + await _record_login_failure(ip) + await _check_rate_limit(ip) # 1 failure < 2: allowed + await _record_login_failure(ip) + with pytest.raises(HTTPException) as exc_info: await _check_rate_limit(ip) - assert ip not in _login_attempts - finally: - reset_app_config() - _login_attempts.clear() + assert exc_info.value.status_code == 429 + # The lockout window comes from lockout_seconds (60s), not the 300s + # default: at locked_at + 61 the lock must already be released. + locked_at = (await throttle_store.get(ip)).locked_at + monkeypatch.setattr(auth_router.time, "time", lambda: locked_at + 61.0) + await _check_rate_limit(ip) + assert await throttle_store.get(ip) is None def test_rate_limiter_uses_defaults_when_config_unavailable(monkeypatch): @@ -1282,7 +1377,7 @@ def test_rate_limiter_malformed_config_propagates(monkeypatch): @pytest.mark.asyncio -async def test_rate_limiter_clean_ip_skips_config_read(monkeypatch): +async def test_rate_limiter_clean_ip_skips_config_read(throttle_store, monkeypatch): """A clean IP pays zero config reads: the record-None early return must come before policy resolution (get_app_config re-hashes config.yaml on every call, and login_local is an unauthenticated async endpoint).""" @@ -1293,12 +1388,11 @@ async def test_rate_limiter_clean_ip_skips_config_read(monkeypatch): raise AssertionError("config must not be read for a clean IP") monkeypatch.setattr(app_config_module, "get_app_config", _must_not_load) - auth_router._login_attempts.clear() await auth_router._check_rate_limit("192.0.2.7") # returns quietly → no config read @pytest.mark.asyncio -async def test_rate_limiter_policy_change_semantics(): +async def test_rate_limiter_policy_change_semantics(throttle_store, throttle_policy): """Pin the emergent semantics of a live-read policy under config reload. Raising max_login_attempts mid-lockout immediately unblocks IPs whose @@ -1307,51 +1401,35 @@ async def test_rate_limiter_policy_change_semantics(): Tightening the threshold keeps the accumulated count (see the dedicated test below); subsequent failures lock under the new, stricter policy. """ - from app.gateway.routers.auth import _check_rate_limit, _login_attempts, _record_login_failure - from deerflow.config.app_config import AppConfig, reset_app_config, set_app_config - from deerflow.config.auth_config import AuthAppConfig, LocalAuthConfig - from deerflow.config.sandbox_config import SandboxConfig + from app.gateway.routers.auth import _check_rate_limit, _record_login_failure - def _set_policy(max_attempts: int) -> None: - set_app_config( - AppConfig( - sandbox=SandboxConfig(use="test"), - auth=AuthAppConfig(local=LocalAuthConfig(max_login_attempts=max_attempts, lockout_seconds=60.0)), - ) - ) - - _login_attempts.clear() - try: - ip = "10.0.0.4" - _set_policy(2) - for _ in range(2): - await _record_login_failure(ip) - with pytest.raises(HTTPException): - await _check_rate_limit(ip) # locked under the old policy - - _set_policy(5) # operator raises the ceiling mid-lockout - await _check_rate_limit(ip) # immediately allowed: 2 < 5, no restart needed - - # max_login_attempts=1 never reaches the endpoint: config load rejects - # it (ge=2). A legal value of 1 previously disabled lockout entirely — - # the (1, 0.0) first-failure record expired immediately, and every - # subsequent failure re-created it, so the IP was never locked. - import pydantic - - with pytest.raises(pydantic.ValidationError): - _set_policy(1) - - # The strictest legal value still locks, at the second failure. - _login_attempts.pop(ip, None) - _set_policy(2) + ip = "10.0.0.4" + throttle_policy(max_attempts=2) + for _ in range(2): await _record_login_failure(ip) - await _check_rate_limit(ip) # 1 failure < 2: allowed - await _record_login_failure(ip) - with pytest.raises(HTTPException): - await _check_rate_limit(ip) - finally: - reset_app_config() - _login_attempts.clear() + with pytest.raises(HTTPException): + await _check_rate_limit(ip) # locked under the old policy + + throttle_policy(max_attempts=5) # operator raises the ceiling mid-lockout + await _check_rate_limit(ip) # immediately allowed: 2 < 5, no restart needed + + # max_login_attempts=1 never reaches the endpoint: config load rejects + # it (ge=2). A legal value of 1 previously disabled lockout entirely — + # the (1, 0.0) first-failure record expired immediately, and every + # subsequent failure re-created it, so the IP was never locked. + import pydantic + + with pytest.raises(pydantic.ValidationError): + throttle_policy(max_attempts=1) + + # The strictest legal value still locks, at the second failure. + await throttle_store.reset(ip) + throttle_policy(max_attempts=2) + await _record_login_failure(ip) + await _check_rate_limit(ip) # 1 failure < 2: allowed + await _record_login_failure(ip) + with pytest.raises(HTTPException): + await _check_rate_limit(ip) def test_local_auth_throttle_config_validation(): @@ -1375,7 +1453,7 @@ def test_local_auth_throttle_config_validation(): @pytest.mark.asyncio -async def test_rate_limiter_active_lockout_honors_live_lockout_seconds_change(monkeypatch): +async def test_rate_limiter_active_lockout_honors_live_lockout_seconds_change(throttle_store, throttle_policy, monkeypatch): """A live lockout_seconds change applies to in-flight lockouts, in the direction that serves the operator, without resurrecting served sentences. @@ -1386,67 +1464,52 @@ async def test_rate_limiter_active_lockout_honors_live_lockout_seconds_change(mo resurrection), and a raise while the lock is still active extends it. """ from app.gateway.routers import auth as auth_router - from app.gateway.routers.auth import _check_rate_limit, _login_attempts, _record_login_failure - from deerflow.config.app_config import AppConfig, reset_app_config, set_app_config - from deerflow.config.auth_config import AuthAppConfig, LocalAuthConfig - from deerflow.config.sandbox_config import SandboxConfig - - def _set_policy(lockout_seconds: float) -> None: - set_app_config( - AppConfig( - sandbox=SandboxConfig(use="test"), - auth=AuthAppConfig(local=LocalAuthConfig(max_login_attempts=2, lockout_seconds=lockout_seconds)), - ) - ) + from app.gateway.routers.auth import _check_rate_limit, _record_login_failure def _freeze_clock_at(t: float) -> None: monkeypatch.setattr(auth_router.time, "time", lambda: t) - _login_attempts.clear() - try: - ip = "10.0.0.5" + ip = "10.0.0.5" - # Lowering mid-lockout releases early. - _set_policy(60.0) - await _record_login_failure(ip) - await _record_login_failure(ip) - _, locked_at, _ = _login_attempts[ip] - _freeze_clock_at(locked_at + 2.0) - with pytest.raises(HTTPException): - await _check_rate_limit(ip) # 2s into the 60s window: still locked - _set_policy(1.0) # operator shortens the window - await _check_rate_limit(ip) # 2s > 1s: unlocked on the very next login - assert ip not in _login_attempts + # Lowering mid-lockout releases early. + throttle_policy(max_attempts=2, lockout_seconds=60.0) + await _record_login_failure(ip) + await _record_login_failure(ip) + locked_at = (await throttle_store.get(ip)).locked_at + _freeze_clock_at(locked_at + 2.0) + with pytest.raises(HTTPException): + await _check_rate_limit(ip) # 2s into the 60s window: still locked + throttle_policy(max_attempts=2, lockout_seconds=1.0) # operator shortens the window + await _check_rate_limit(ip) # 2s > 1s: unlocked on the very next login + assert await throttle_store.get(ip) is None - # Raising while the lock is still active extends it. - _set_policy(1.0) - await _record_login_failure(ip) - await _record_login_failure(ip) - _, locked_at, _ = _login_attempts[ip] - _freeze_clock_at(locked_at + 0.5) # still inside the 1s sentence - _set_policy(60.0) # operator lengthens the window mid-sentence - with pytest.raises(HTTPException): - await _check_rate_limit(ip) # active, and 0.5 < 60: extended - _freeze_clock_at(locked_at + 2.0) # past the original 1s sentence - with pytest.raises(HTTPException): - await _check_rate_limit(ip) # extended: 2 < 60, still locked + # Raising while the lock is still active extends it. + throttle_policy(max_attempts=2, lockout_seconds=1.0) + await _record_login_failure(ip) + await _record_login_failure(ip) + locked_at = (await throttle_store.get(ip)).locked_at + _freeze_clock_at(locked_at + 0.5) # still inside the 1s sentence + throttle_policy(max_attempts=2, lockout_seconds=60.0) # operator lengthens the window mid-sentence + with pytest.raises(HTTPException): + await _check_rate_limit(ip) # active, and 0.5 < 60: extended + _freeze_clock_at(locked_at + 2.0) # past the original 1s sentence + with pytest.raises(HTTPException): + await _check_rate_limit(ip) # extended: 2 < 60, still locked - # A sentence that already elapsed before the raise is not resurrected. - _set_policy(1.0) - await _record_login_failure(ip) - await _record_login_failure(ip) - _, locked_at, _ = _login_attempts[ip] - _freeze_clock_at(locked_at + 2.0) # past the 1s sentence, no check yet - _set_policy(60.0) # operator lengthens the window for *future* locks - await _check_rate_limit(ip) # the served 1s sentence is not resurrected - assert ip not in _login_attempts - finally: - reset_app_config() - _login_attempts.clear() + # A sentence that already elapsed before the raise is not resurrected. + throttle_policy(max_attempts=2, lockout_seconds=1.0) + await throttle_store.reset(ip) + await _record_login_failure(ip) + await _record_login_failure(ip) + locked_at = (await throttle_store.get(ip)).locked_at + _freeze_clock_at(locked_at + 2.0) # past the 1s sentence, no check yet + throttle_policy(max_attempts=2, lockout_seconds=60.0) # operator lengthens the window for *future* locks + await _check_rate_limit(ip) # the served 1s sentence is not resurrected + assert await throttle_store.get(ip) is None @pytest.mark.asyncio -async def test_rate_limiter_lowered_then_raised_duration_not_resurrected(monkeypatch): +async def test_rate_limiter_lowered_then_raised_duration_not_resurrected(throttle_store, throttle_policy, monkeypatch): """A shortened duration observed during the sentence is committed, so a later raise cannot resurrect time already served under the short policy. @@ -1455,220 +1518,147 @@ async def test_rate_limiter_lowered_then_raised_duration_not_resurrected(monkeyp +10s under the last-evaluated policy, so the +20s request must be allowed. """ from app.gateway.routers import auth as auth_router - from app.gateway.routers.auth import _check_rate_limit, _login_attempts, _record_login_failure - from deerflow.config.app_config import AppConfig, reset_app_config, set_app_config - from deerflow.config.auth_config import AuthAppConfig, LocalAuthConfig - from deerflow.config.sandbox_config import SandboxConfig + from app.gateway.routers.auth import _check_rate_limit, _record_login_failure - def _set_policy(lockout_seconds: float) -> None: - set_app_config( - AppConfig( - sandbox=SandboxConfig(use="test"), - auth=AuthAppConfig(local=LocalAuthConfig(max_login_attempts=2, lockout_seconds=lockout_seconds)), - ) - ) + def _freeze(t: float) -> None: + monkeypatch.setattr(auth_router.time, "time", lambda: t) - _login_attempts.clear() - try: - ip = "10.0.0.8" - _set_policy(60.0) - await _record_login_failure(ip) - await _record_login_failure(ip) - _, locked_at, _ = _login_attempts[ip] + ip = "10.0.0.8" + throttle_policy(max_attempts=2, lockout_seconds=60.0) + await _record_login_failure(ip) + await _record_login_failure(ip) + locked_at = (await throttle_store.get(ip)).locked_at - def _freeze(t: float) -> None: - monkeypatch.setattr(auth_router.time, "time", lambda: t) + _freeze(locked_at + 6.0) + throttle_policy(max_attempts=2, lockout_seconds=10.0) + with pytest.raises(HTTPException): + await _check_rate_limit(ip) # 6 < 10: still locked, and the 10s sentence is committed + assert (await throttle_store.get(ip)).lock_duration == 10.0 - _freeze(locked_at + 6.0) - _set_policy(10.0) - with pytest.raises(HTTPException): - await _check_rate_limit(ip) # 6 < 10: still locked, and the 10s sentence is committed - assert _login_attempts[ip][2] == 10.0 - - _freeze(locked_at + 20.0) - _set_policy(30.0) # raised after the 10s sentence was served at +10s - await _check_rate_limit(ip) # not resurrected: allowed - assert ip not in _login_attempts - finally: - reset_app_config() - _login_attempts.clear() + _freeze(locked_at + 20.0) + throttle_policy(max_attempts=2, lockout_seconds=30.0) # raised after the 10s sentence was served at +10s + await _check_rate_limit(ip) # not resurrected: allowed + assert await throttle_store.get(ip) is None @pytest.mark.asyncio -async def test_concurrent_checks_on_expired_lock_are_race_free(monkeypatch): +async def test_concurrent_checks_on_expired_lock_are_race_free(throttle_store, monkeypatch): """Synchronized checks of the same expired lock must all resolve cleanly. Policy resolution yields the event loop (asyncio.to_thread), so the sync version's atomicity is gone: with a pre-await snapshot only, concurrent checks of an expired record raced into double ``del`` — one request returned normally and the others raised KeyError (review reproduction). + The SQL store must likewise tolerate racing deletes of the same row. """ import asyncio from app.gateway.routers import auth as auth_router - from app.gateway.routers.auth import _check_rate_limit, _login_attempts + from app.gateway.routers.auth import _check_rate_limit from deerflow.config.auth_config import LocalAuthConfig def _defaults(): return LocalAuthConfig().max_login_attempts, LocalAuthConfig().lockout_seconds monkeypatch.setattr(auth_router, "_login_throttle_policy", _defaults) - _login_attempts.clear() - try: - _login_attempts["10.0.0.9"] = (5, 1.0, 1.0) # sentence long expired + ip = "10.0.0.9" + for _ in range(5): + await throttle_store.record_failure(ip, max_attempts=5, lockout_seconds=1.0, now=1.0) # sentence long expired - results = await asyncio.gather(*[_check_rate_limit("10.0.0.9") for _ in range(8)], return_exceptions=True) + results = await asyncio.gather(*[_check_rate_limit(ip) for _ in range(8)], return_exceptions=True) - assert all(result is None for result in results), results - assert "10.0.0.9" not in _login_attempts - finally: - _login_attempts.clear() + assert all(result is None for result in results), results + assert await throttle_store.get(ip) is None @pytest.mark.asyncio -async def test_check_survives_record_deleted_during_policy_resolution(monkeypatch): - """A record removed while the checker is suspended must not KeyError. +async def test_check_survives_record_deleted_during_policy_resolution(throttle_store, monkeypatch): + """A record removed while the checker is suspended must not fail. - Deterministic stand-in for the suspension-window interleaving: the policy - resolution itself removes the record, exactly like a concurrent success - login would while this coroutine sits in ``asyncio.to_thread``. + Deterministic stand-in for the suspension-window interleaving: a + concurrent successful login clears the record right after the pre-read, + exactly as it would while this coroutine sits in ``asyncio.to_thread``. """ from app.gateway.routers import auth as auth_router - from app.gateway.routers.auth import _check_rate_limit, _login_attempts, _record_login_success + from app.gateway.routers.auth import _check_rate_limit - def _policy_that_deletes_the_record(): - _record_login_success("10.0.0.10") - return 5, 300.0 + ip = "10.0.0.10" + for _ in range(5): + await throttle_store.record_failure(ip, max_attempts=5, lockout_seconds=1.0, now=1.0) # expired - monkeypatch.setattr(auth_router, "_login_throttle_policy", _policy_that_deletes_the_record) - _login_attempts.clear() - try: - _login_attempts["10.0.0.10"] = (5, 1.0, 1.0) # expired + async def _concurrent_success(inner): + await inner.reset(ip) - await _check_rate_limit("10.0.0.10") # pre-fix: KeyError + _install_throttle_store(_MutateDuringPolicyResolution(throttle_store, _concurrent_success)) + monkeypatch.setattr(auth_router, "_login_throttle_policy", lambda: (5, 300.0)) - assert "10.0.0.10" not in _login_attempts - finally: - _login_attempts.clear() + await _check_rate_limit(ip) # pre-fix: KeyError + + assert await throttle_store.get(ip) is None @pytest.mark.asyncio -async def test_check_never_clobbers_record_replaced_during_policy_resolution(monkeypatch): +async def test_check_never_clobbers_record_replaced_during_policy_resolution(throttle_store, monkeypatch): """A record replaced while the checker is suspended must survive intact. - The pre-await snapshot said "locked"; while suspended, a successful login - plus one new failure replaced the record with a fresh counter. The checker - must re-read and leave the fresh record alone instead of writing its - stale-snapshot decision over it. + The pre-read said "locked"; while suspended, a successful login plus one + new failure replaced the record with a fresh counter. The checker must + decide on a fresh read and leave the fresh record alone instead of writing + its stale-snapshot decision over it. """ from app.gateway.routers import auth as auth_router - from app.gateway.routers.auth import _check_rate_limit, _login_attempts + from app.gateway.routers.auth import _check_rate_limit + from deerflow.persistence.login_throttle import LoginThrottleRecord - def _policy_that_replaces_the_record(): - _login_attempts["10.0.0.11"] = (1, 0.0, 0.0) - return 2, 60.0 + ip = "10.0.0.11" + for _ in range(2): + await throttle_store.record_failure(ip, max_attempts=2, lockout_seconds=60.0, now=_THROTTLE_T0) # locked, sentence running + monkeypatch.setattr(auth_router.time, "time", lambda: _THROTTLE_T0 + 1.0) - monkeypatch.setattr(auth_router, "_login_throttle_policy", _policy_that_replaces_the_record) - _login_attempts.clear() - try: - _login_attempts["10.0.0.11"] = (2, 1.0, 1.0) # locked, sentence running + async def _success_then_one_failure(inner): + await inner.reset(ip) + await inner.record_failure(ip, max_attempts=2, lockout_seconds=60.0, now=_THROTTLE_T0 + 1.0) - await _check_rate_limit("10.0.0.11") # fresh read: (1, 0, 0) < max 2 → allowed + _install_throttle_store(_MutateDuringPolicyResolution(throttle_store, _success_then_one_failure)) + monkeypatch.setattr(auth_router, "_login_throttle_policy", lambda: (2, 60.0)) - assert _login_attempts["10.0.0.11"] == (1, 0.0, 0.0) - finally: - _login_attempts.clear() + await _check_rate_limit(ip) # fresh read: (1, 0, 0) < max 2 → allowed + + assert await throttle_store.get(ip) == LoginThrottleRecord(fail_count=1, locked_at=0.0, lock_duration=0.0) @pytest.mark.asyncio -async def test_rate_limiter_eviction_expires_by_stored_sentence_not_current_threshold(monkeypatch): - """The capacity sweep must expire records by their own committed sentence, - with no gate on the current threshold. - - A record locked under an old, lower threshold has a count below the live - max after the operator raises it; gating expiry on ``count >= max`` keeps - that served record resident while the capacity fallback evicts live - counters first (they sort earliest), handing an active offender a fresh - budget. Reproduction from review: cap 2, live ``(1, 0, 0)`` plus expired - ``(2, 10, 1)`` under max=3, clock at 100. - """ - from app.gateway.routers import auth as auth_router - from app.gateway.routers.auth import _login_attempts, _record_login_failure - from deerflow.config.app_config import AppConfig, reset_app_config, set_app_config - from deerflow.config.auth_config import AuthAppConfig, LocalAuthConfig - from deerflow.config.sandbox_config import SandboxConfig - - monkeypatch.setattr(auth_router, "_MAX_TRACKED_IPS", 2) - monkeypatch.setattr(auth_router.time, "time", lambda: 100.0) - set_app_config( - AppConfig( - sandbox=SandboxConfig(use="test"), - auth=AuthAppConfig(local=LocalAuthConfig(max_login_attempts=3, lockout_seconds=60.0)), - ) - ) - _login_attempts.clear() - try: - _login_attempts["live-counter"] = (1, 0.0, 0.0) # active offender, counting - _login_attempts["expired-lock"] = (2, 10.0, 1.0) # locked under old max=2; served at 11.0 - - await _record_login_failure("fresh-ip") # hits the capacity sweep - - assert "expired-lock" not in _login_attempts # served sentence is swept - assert _login_attempts["live-counter"] == (1, 0.0, 0.0) # live counter survives - assert _login_attempts["fresh-ip"][0] == 1 - finally: - reset_app_config() - _login_attempts.clear() - - -@pytest.mark.asyncio -async def test_rate_limiter_tightened_threshold_preserves_failures(): +async def test_rate_limiter_tightened_threshold_preserves_failures(throttle_store, throttle_policy): """Tightening max_login_attempts mid-count keeps the accumulated failures. An IP with four failures under max_login_attempts=5 must not get a fresh budget when the operator lowers the threshold to 2: the count stays, the next failure starts the lock, and a successful login still clears it. """ - from app.gateway.routers.auth import _check_rate_limit, _login_attempts, _record_login_failure, _record_login_success - from deerflow.config.app_config import AppConfig, reset_app_config, set_app_config - from deerflow.config.auth_config import AuthAppConfig, LocalAuthConfig - from deerflow.config.sandbox_config import SandboxConfig + from app.gateway.routers.auth import _check_rate_limit, _record_login_failure, _record_login_success - def _set_policy(max_attempts: int) -> None: - set_app_config( - AppConfig( - sandbox=SandboxConfig(use="test"), - auth=AuthAppConfig(local=LocalAuthConfig(max_login_attempts=max_attempts, lockout_seconds=60.0)), - ) - ) + ip = "10.0.0.6" + throttle_policy(max_attempts=5) + for _ in range(4): + await _record_login_failure(ip) # 4 failures: counting, never locked - _login_attempts.clear() - try: - ip = "10.0.0.6" - _set_policy(5) - for _ in range(4): - await _record_login_failure(ip) # 4 failures: counting, never locked + throttle_policy(max_attempts=2) # operator tightens the policy mid-count + await _check_rate_limit(ip) # allowed this once — but the count survives + assert (await throttle_store.get(ip)).fail_count == 4 - _set_policy(2) # operator tightens the policy mid-count - await _check_rate_limit(ip) # allowed this once — but the count survives - assert _login_attempts[ip][0] == 4 - - await _record_login_failure(ip) # 4 + 1 >= 2: locks on the very next failure - with pytest.raises(HTTPException): - await _check_rate_limit(ip) - - # A correct password still clears everything (no retroactive lockout - # of a legitimate user who fat-fingered the password four times). - _record_login_success(ip) + await _record_login_failure(ip) # 4 + 1 >= 2: locks on the very next failure + with pytest.raises(HTTPException): await _check_rate_limit(ip) - assert ip not in _login_attempts - finally: - reset_app_config() - _login_attempts.clear() + + # A correct password still clears everything (no retroactive lockout + # of a legitimate user who fat-fingered the password four times). + await _record_login_success(ip) + await _check_rate_limit(ip) + assert await throttle_store.get(ip) is None @pytest.mark.asyncio -async def test_rate_limiter_counts_failure_when_config_breaks(monkeypatch): +async def test_rate_limiter_counts_failure_when_config_breaks(throttle_store, monkeypatch): """A malformed config hot-edit must not hand out unlimited verification. Endpoint order per failed login: _check_rate_limit (clean IPs skip the @@ -1679,33 +1669,30 @@ async def test_rate_limiter_counts_failure_when_config_breaks(monkeypatch): re-raises; from then on the dirty IP's own check reads the broken config and fails closed — before authenticate. """ - from app.gateway.routers.auth import _check_rate_limit, _login_attempts, _record_login_failure + from app.gateway.routers.auth import _check_rate_limit, _record_login_failure from deerflow.config import app_config as app_config_module + from deerflow.persistence.login_throttle import LoginThrottleRecord def _malformed(): raise ValueError("config validation error") - _login_attempts.clear() monkeypatch.setattr(app_config_module, "get_app_config", _malformed) - try: - ip = "10.0.0.7" + ip = "10.0.0.7" - # First failed login: still allowed through to authenticate(), then - # the record call fails loudly — but the failure is counted first. - await _check_rate_limit(ip) # clean IP: no config read, allowed - with pytest.raises(ValueError, match="config validation error"): - await _record_login_failure(ip) - assert _login_attempts[ip] == (1, 0.0, 0.0) + # First failed login: still allowed through to authenticate(), then + # the record call fails loudly — but the failure is counted first. + await _check_rate_limit(ip) # clean IP: no config read, allowed + with pytest.raises(ValueError, match="config validation error"): + await _record_login_failure(ip) + assert await throttle_store.get(ip) == LoginThrottleRecord(fail_count=1, locked_at=0.0, lock_duration=0.0) - # Second login: the now-dirty IP's check reads the broken config and - # fails closed *before* authenticate() — no more password verification. - with pytest.raises(ValueError, match="config validation error"): - await _check_rate_limit(ip) - finally: - _login_attempts.clear() + # Second login: the now-dirty IP's check reads the broken config and + # fails closed *before* authenticate() — no more password verification. + with pytest.raises(ValueError, match="config validation error"): + await _check_rate_limit(ip) -def test_login_local_broken_config_fails_closed_after_first_failure(monkeypatch): +def test_login_local_broken_config_fails_closed_after_first_failure(memory_throttle_store, monkeypatch): """Route-level pin of the same sequence, through POST /login/local. The first wrong password is verified once and counted (the 500 comes from @@ -1714,6 +1701,8 @@ def test_login_local_broken_config_fails_closed_after_first_failure(monkeypatch) authenticate() is reached — no unlimited password verification while config.yaml stays malformed. """ + import asyncio + from fastapi import FastAPI from fastapi.testclient import TestClient @@ -1734,22 +1723,74 @@ def test_login_local_broken_config_fails_closed_after_first_failure(monkeypatch) monkeypatch.setattr(auth_router, "get_local_provider", lambda: _Provider()) monkeypatch.delenv("AUTH_TRUSTED_PROXIES", raising=False) - auth_router._login_attempts.clear() + + app = FastAPI() + app.include_router(auth_router.router) + with TestClient(app, raise_server_exceptions=False) as client: + first = client.post("/api/v1/auth/login/local", data={"username": "user@example.com", "password": "wrong"}) + assert first.status_code == 500 + assert calls["authenticate"] == 1 # verified once — and counted despite the raise + assert asyncio.run(memory_throttle_store.get("testclient")).fail_count == 1 + + second = client.post("/api/v1/auth/login/local", data={"username": "user@example.com", "password": "wrong-again"}) + assert second.status_code == 500 + assert calls["authenticate"] == 1 # fail-closed: no second verification + + +def test_login_local_lockout_follows_the_shared_database_across_replicas(tmp_path, monkeypatch): + """Route-level multi-replica reproduction through POST /login/local. + + Two Gateway replicas are modelled as two SQL stores over one SQLite file, + swapped in between requests: the failures counted by replica A lock the + client on replica B, and a lockout is enforced wherever the load balancer + sends the next attempt. Before this change each replica's in-process dict + gave an attacker N x max_login_attempts guesses. + """ + from fastapi import FastAPI + from fastapi.testclient import TestClient + from sqlalchemy import create_engine + from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine + + from app.gateway.auth import login_throttle + from app.gateway.routers import auth as auth_router + from deerflow.config.app_config import AppConfig, reset_app_config, set_app_config + from deerflow.config.auth_config import AuthAppConfig, LocalAuthConfig + from deerflow.config.sandbox_config import SandboxConfig + from deerflow.persistence.base import Base + from deerflow.persistence.login_throttle import LoginThrottleRow, SqlLoginThrottleStore + + class _Provider: + async def authenticate(self, credentials): + return None # every password is wrong + + monkeypatch.setattr(auth_router, "get_local_provider", lambda: _Provider()) + monkeypatch.delenv("AUTH_TRUSTED_PROXIES", raising=False) + set_app_config(AppConfig(sandbox=SandboxConfig(use="test"), auth=AuthAppConfig(local=LocalAuthConfig(max_login_attempts=3, lockout_seconds=300.0)))) + + db_path = tmp_path / "shared-throttle.db" + sync_engine = create_engine(f"sqlite:///{db_path.as_posix()}") + Base.metadata.create_all(sync_engine, tables=[LoginThrottleRow.__table__]) + sync_engine.dispose() + engines = [create_async_engine(f"sqlite+aiosqlite:///{db_path.as_posix()}") for _ in range(2)] + replica_a, replica_b = (SqlLoginThrottleStore(async_sessionmaker(engine, expire_on_commit=False)) for engine in engines) app = FastAPI() app.include_router(auth_router.router) try: - with TestClient(app, raise_server_exceptions=False) as client: - first = client.post("/api/v1/auth/login/local", data={"username": "user@example.com", "password": "wrong"}) - assert first.status_code == 500 - assert calls["authenticate"] == 1 # verified once — and counted despite the raise - assert auth_router._login_attempts["testclient"][0] == 1 + with TestClient(app) as client: + login_throttle.install_login_throttle_store(replica_a) + for _ in range(3): + assert client.post("/api/v1/auth/login/local", data={"username": "user@example.com", "password": "wrong"}).status_code == 401 - second = client.post("/api/v1/auth/login/local", data={"username": "user@example.com", "password": "wrong-again"}) - assert second.status_code == 500 - assert calls["authenticate"] == 1 # fail-closed: no second verification + login_throttle.install_login_throttle_store(replica_b) + blocked = client.post("/api/v1/auth/login/local", data={"username": "user@example.com", "password": "wrong"}) + assert blocked.status_code == 429 + assert blocked.json()["detail"] == _THROTTLE_429_DETAIL finally: - auth_router._login_attempts.clear() + login_throttle.reset_login_throttle_store() + reset_app_config() + for engine in engines: + engine.sync_engine.dispose() # ── Client IP extraction ───────────────────────────────────────────────── diff --git a/backend/tests/test_login_throttle_config.py b/backend/tests/test_login_throttle_config.py new file mode 100644 index 0000000000..cf18f8bf72 --- /dev/null +++ b/backend/tests/test_login_throttle_config.py @@ -0,0 +1,213 @@ +"""``auth.local.throttle_storage``: selector semantics and store resolution. + +``auto`` (the default) puts failed-login counters in the application database +whenever one exists (``database.backend`` sqlite or postgres), so every +Gateway replica sharing that database enforces one lockout per IP; a +``memory`` database has no shared table and falls back to the per-process +counter. ``memory`` forces the per-process counter; ``db`` forces the table +and degrades to memory with a warning when there is no database to hold it. +""" + +from __future__ import annotations + +import logging +from types import SimpleNamespace + +import pydantic +import pytest +from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine + +from app.gateway.auth import login_throttle +from deerflow.config.auth_config import LocalAuthConfig, LoginThrottleStorage, resolve_login_throttle_storage +from deerflow.config.reload_boundary import STARTUP_ONLY_FIELDS, STARTUP_ONLY_PREFIX +from deerflow.persistence.login_throttle import MemoryLoginThrottleStore, SqlLoginThrottleStore + + +@pytest.fixture(autouse=True) +def _no_installed_store(): + login_throttle.reset_login_throttle_store() + yield + login_throttle.reset_login_throttle_store() + + +@pytest.mark.parametrize( + ("selector", "database_backend", "expected"), + [ + ("auto", "sqlite", "db"), + ("auto", "postgres", "db"), + ("auto", "memory", "memory"), + ("auto", None, "memory"), + ("memory", "postgres", "memory"), + ("memory", "sqlite", "memory"), + ("db", "sqlite", "db"), + ("db", "postgres", "db"), + ("db", "memory", "memory"), + (LoginThrottleStorage.AUTO, "sqlite", "db"), + (LoginThrottleStorage.DB, "memory", "memory"), + ], +) +def test_resolve_login_throttle_storage(selector, database_backend, expected): + assert resolve_login_throttle_storage(selector, database_backend) == expected + + +def test_local_auth_config_selector_defaults_to_auto_and_validates(): + assert LocalAuthConfig().throttle_storage == LoginThrottleStorage.AUTO + assert LocalAuthConfig(throttle_storage="db").throttle_storage == LoginThrottleStorage.DB + assert LocalAuthConfig(throttle_storage="memory").throttle_storage == LoginThrottleStorage.MEMORY + with pytest.raises(pydantic.ValidationError): + LocalAuthConfig(throttle_storage="redis") + + +def test_throttle_storage_is_registered_as_startup_only(): + """The store is resolved once at Gateway startup; the policy knobs stay live-read.""" + field_path = "auth.local.throttle_storage" + assert field_path in STARTUP_ONLY_FIELDS + description = LocalAuthConfig.model_fields["throttle_storage"].description or "" + assert description.startswith(STARTUP_ONLY_PREFIX) + assert STARTUP_ONLY_FIELDS[field_path] in description + for live_field in ("max_login_attempts", "lockout_seconds"): + assert not (LocalAuthConfig.model_fields[live_field].description or "").startswith(STARTUP_ONLY_PREFIX) + + +def _config(selector: str, database_backend: str) -> SimpleNamespace: + return SimpleNamespace( + auth=SimpleNamespace(local=LocalAuthConfig(throttle_storage=selector)), + database=SimpleNamespace(backend=database_backend), + ) + + +@pytest.fixture +def session_factory(tmp_path): + engine = create_async_engine(f"sqlite+aiosqlite:///{tmp_path / 'resolver.db'}") + yield async_sessionmaker(engine, expire_on_commit=False) + engine.sync_engine.dispose() + + +def test_auto_with_a_database_resolves_the_sql_store(session_factory, caplog): + with caplog.at_level(logging.WARNING): + store = login_throttle.resolve_login_throttle_store(_config("auto", "sqlite"), session_factory=session_factory) + assert isinstance(store, SqlLoginThrottleStore) + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] + + +def test_auto_with_postgres_resolves_the_sql_store(session_factory): + store = login_throttle.resolve_login_throttle_store(_config("auto", "postgres"), session_factory=session_factory) + assert isinstance(store, SqlLoginThrottleStore) + + +def test_auto_with_a_memory_database_resolves_the_memory_store(caplog): + with caplog.at_level(logging.WARNING): + store = login_throttle.resolve_login_throttle_store(_config("auto", "memory"), session_factory=None) + assert isinstance(store, MemoryLoginThrottleStore) + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] + + +def test_explicit_memory_ignores_the_database(session_factory): + store = login_throttle.resolve_login_throttle_store(_config("memory", "postgres"), session_factory=session_factory) + assert isinstance(store, MemoryLoginThrottleStore) + + +def test_explicit_db_without_a_database_falls_back_to_memory_and_warns(caplog): + with caplog.at_level(logging.WARNING): + store = login_throttle.resolve_login_throttle_store(_config("db", "memory"), session_factory=None) + assert isinstance(store, MemoryLoginThrottleStore) + assert any("auth.local.throttle_storage=db" in r.message and "database.backend" in r.message for r in caplog.records) + + +def test_auto_resolution_without_an_engine_falls_back_to_memory_and_warns(caplog): + """``auto`` with a configured database but no initialised engine (bare app) keeps the login path working.""" + with caplog.at_level(logging.WARNING): + store = login_throttle.resolve_login_throttle_store(_config("auto", "sqlite"), session_factory=None) + assert isinstance(store, MemoryLoginThrottleStore) + assert any("engine" in r.message and "throttle_storage=auto" in r.message for r in caplog.records) + + +def test_explicit_db_without_an_engine_fails_closed_at_startup(): + """An operator who asked for the shared table must not silently get per-process counters. + + Same failure type as the other startup validations in ``app.gateway.deps`` + (``_validate_agent_storage`` / the multi-process gate), so ``langgraph_runtime`` + surfaces it as a startup failure instead of a first-login surprise. + """ + with pytest.raises(SystemExit, match="auth.local.throttle_storage='db'"): + login_throttle.resolve_login_throttle_store(_config("db", "sqlite"), session_factory=None) + with pytest.raises(SystemExit, match="database.backend"): + login_throttle.resolve_login_throttle_store(_config("db", "postgres"), session_factory=None) + assert login_throttle.installed_login_throttle_store() is None + + +def test_lazy_resolution_explicit_db_without_an_engine_fails_closed(monkeypatch): + """The bare-app path applies the same rule and installs nothing.""" + from deerflow.config import app_config as app_config_module + from deerflow.persistence import engine as engine_module + + monkeypatch.setattr(app_config_module, "get_app_config", lambda: _config("db", "sqlite")) + monkeypatch.setattr(engine_module, "get_session_factory", lambda: None) + with pytest.raises(SystemExit, match="auth.local.throttle_storage='db'"): + login_throttle.resolve_and_install_from_live_config() + assert login_throttle.installed_login_throttle_store() is None + + +def test_lazy_resolution_auto_without_an_engine_falls_back_to_memory(monkeypatch, caplog): + from deerflow.config import app_config as app_config_module + from deerflow.persistence import engine as engine_module + + monkeypatch.setattr(app_config_module, "get_app_config", lambda: _config("auto", "sqlite")) + monkeypatch.setattr(engine_module, "get_session_factory", lambda: None) + with caplog.at_level(logging.WARNING): + store = login_throttle.resolve_and_install_from_live_config() + assert isinstance(store, MemoryLoginThrottleStore) + assert login_throttle.installed_login_throttle_store() is store + assert any("throttle_storage=auto" in r.message for r in caplog.records) + + +def test_missing_config_resolves_the_memory_store(): + assert isinstance(login_throttle.resolve_login_throttle_store(None, session_factory=None), MemoryLoginThrottleStore) + + +def test_install_and_reset_hooks(): + assert login_throttle.installed_login_throttle_store() is None + store = MemoryLoginThrottleStore() + login_throttle.install_login_throttle_store(store) + assert login_throttle.installed_login_throttle_store() is store + login_throttle.reset_login_throttle_store() + assert login_throttle.installed_login_throttle_store() is None + + +def test_lazy_resolution_uses_live_config_and_installs_once(monkeypatch, session_factory): + """A bare app (no Gateway lifespan) resolves on first use and keeps that store.""" + from deerflow.config import app_config as app_config_module + from deerflow.persistence import engine as engine_module + + monkeypatch.setattr(app_config_module, "get_app_config", lambda: _config("auto", "sqlite")) + monkeypatch.setattr(engine_module, "get_session_factory", lambda: session_factory) + first = login_throttle.resolve_and_install_from_live_config() + assert isinstance(first, SqlLoginThrottleStore) + assert login_throttle.installed_login_throttle_store() is first + # A second resolver racing the first keeps the already installed store. + assert login_throttle.resolve_and_install_from_live_config() is first + + +def test_lazy_resolution_without_config_file_uses_defaults(monkeypatch): + from deerflow.config import app_config as app_config_module + from deerflow.persistence import engine as engine_module + + def _missing(): + raise FileNotFoundError("no config.yaml") + + monkeypatch.setattr(app_config_module, "get_app_config", _missing) + monkeypatch.setattr(engine_module, "get_session_factory", lambda: None) + assert isinstance(login_throttle.resolve_and_install_from_live_config(), MemoryLoginThrottleStore) + + +def test_lazy_resolution_propagates_a_malformed_config(monkeypatch): + """Like the policy read: a broken config.yaml fails loudly rather than silently picking a store.""" + from deerflow.config import app_config as app_config_module + + def _malformed(): + raise ValueError("config validation error") + + monkeypatch.setattr(app_config_module, "get_app_config", _malformed) + with pytest.raises(ValueError, match="config validation error"): + login_throttle.resolve_and_install_from_live_config() + assert login_throttle.installed_login_throttle_store() is None diff --git a/backend/tests/test_login_throttle_store.py b/backend/tests/test_login_throttle_store.py new file mode 100644 index 0000000000..ba4fc6f72e --- /dev/null +++ b/backend/tests/test_login_throttle_store.py @@ -0,0 +1,807 @@ +"""Login-throttle store contract: the in-process counter and the shared SQL table. + +Both stores must implement the semantics the auth router relied on while the +counter was a module-level dict: a lock starts when the failure count reaches +``max_attempts``; the duration committed at lock time is honored even when the +policy shrinks mid-lock; an active lock follows the live duration (decreases +included); a served sentence is never resurrected by a later raise; a +successful login resets the IP; and the memory store bounds its tracked-IP +set. The SQL store additionally shares that state across every Gateway +replica using one database, which is the whole point of the change (and the +red-on-main reproduction here: two SQL stores over one SQLite file agree on +the lock, two memory stores do not). +""" + +from __future__ import annotations + +import asyncio +import os +import time +import uuid +from collections.abc import AsyncIterator, Awaitable, Callable +from datetime import UTC, datetime +from pathlib import Path + +import pytest +import pytest_asyncio +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql, sqlite +from sqlalchemy.ext.asyncio import AsyncEngine, async_sessionmaker, create_async_engine +from support.postgres import asyncpg_test_url + +from deerflow.persistence.base import Base +from deerflow.persistence.login_throttle import ( + MAX_TRACKED_IPS, + STALE_COUNTER_SECONDS, + LoginThrottleRecord, + LoginThrottleRow, + LoginThrottleStore, + MemoryLoginThrottleStore, + SqlLoginThrottleStore, +) +from deerflow.persistence.postgres_schema import build_asyncpg_connect_args + +pytestmark = pytest.mark.asyncio + +T0 = 1_700_000_000.0 + + +async def _sqlite_engine(path: Path) -> AsyncEngine: + engine = create_async_engine(f"sqlite+aiosqlite:///{path.as_posix()}") + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all, tables=[LoginThrottleRow.__table__], checkfirst=True) + return engine + + +def _sql_store(engine: AsyncEngine) -> SqlLoginThrottleStore: + return SqlLoginThrottleStore(async_sessionmaker(engine, expire_on_commit=False)) + + +@pytest_asyncio.fixture(params=["memory", "sql"]) +async def store(request, tmp_path) -> AsyncIterator[LoginThrottleStore]: + if request.param == "memory": + yield MemoryLoginThrottleStore() + return + engine = await _sqlite_engine(tmp_path / "throttle.db") + try: + yield _sql_store(engine) + finally: + await engine.dispose() + + +def _policy(max_attempts: int, lockout_seconds: float) -> Callable[[], Awaitable[tuple[int, float]]]: + """A ``check`` policy callable for a known policy (the router wraps its config read the same way).""" + + async def resolve() -> tuple[int, float]: + return max_attempts, lockout_seconds + + return resolve + + +class _CountingPolicy: + """Policy callable that counts how often ``check`` resolved it.""" + + def __init__(self, max_attempts: int, lockout_seconds: float) -> None: + self.calls = 0 + self._policy = (max_attempts, lockout_seconds) + + async def __call__(self) -> tuple[int, float]: + self.calls += 1 + return self._policy + + +async def _lock(store: LoginThrottleStore, ip: str, *, max_attempts: int = 2, lockout_seconds: float = 60.0, now: float = T0) -> LoginThrottleRecord: + record = None + for _ in range(max_attempts): + record = await store.record_failure(ip, max_attempts=max_attempts, lockout_seconds=lockout_seconds, now=now) + assert record is not None + return record + + +# ── shared contract ───────────────────────────────────────────────────────── + + +async def test_clean_ip_has_no_record_and_is_allowed(store): + assert await store.get("192.0.2.1") is None + assert await store.check("192.0.2.1", policy=_policy(5, 300.0), now=T0) == 0.0 + + +async def test_lock_starts_when_failures_reach_max_attempts(store): + ip = "10.0.0.1" + for n in range(1, 5): + record = await store.record_failure(ip, max_attempts=5, lockout_seconds=300.0, now=T0 + n) + assert record == LoginThrottleRecord(fail_count=n, locked_at=0.0, lock_duration=0.0) + assert await store.check(ip, policy=_policy(5, 300.0), now=T0 + n) == 0.0 + record = await store.record_failure(ip, max_attempts=5, lockout_seconds=300.0, now=T0 + 5) + assert record == LoginThrottleRecord(fail_count=5, locked_at=T0 + 5, lock_duration=300.0) + assert await store.get(ip) == record + assert await store.check(ip, policy=_policy(5, 300.0), now=T0 + 6) == pytest.approx(299.0) + + +async def test_reset_clears_the_counter(store): + ip = "10.0.0.2" + for _ in range(4): + await store.record_failure(ip, max_attempts=5, lockout_seconds=300.0, now=T0) + await store.reset(ip) + assert await store.get(ip) is None + assert await store.check(ip, policy=_policy(5, 300.0), now=T0) == 0.0 + # Resetting an unknown IP is a no-op. + await store.reset("203.0.113.77") + + +async def test_expired_lock_is_cleared_on_check(store): + ip = "10.0.0.3" + await _lock(store, ip, lockout_seconds=60.0, now=T0) + assert await store.check(ip, policy=_policy(2, 60.0), now=T0 + 59.0) > 0.0 + assert await store.check(ip, policy=_policy(2, 60.0), now=T0 + 61.0) == 0.0 + assert await store.get(ip) is None + + +async def test_raised_threshold_unblocks_and_keeps_the_count(store): + """Raising max_login_attempts mid-lock immediately unblocks a lower count (#5108).""" + ip = "10.0.0.4" + await _lock(store, ip, max_attempts=2, lockout_seconds=60.0, now=T0) + assert await store.check(ip, policy=_policy(2, 60.0), now=T0 + 1) > 0.0 + assert await store.check(ip, policy=_policy(5, 60.0), now=T0 + 1) == 0.0 + assert (await store.get(ip)).fail_count == 2 + + +async def test_tightened_threshold_preserves_failures_and_locks_on_next(store): + ip = "10.0.0.5" + for _ in range(4): + await store.record_failure(ip, max_attempts=5, lockout_seconds=60.0, now=T0) + # Over the new threshold but never locked under it: allowed once, count kept. + assert await store.check(ip, policy=_policy(2, 60.0), now=T0 + 1) == 0.0 + assert (await store.get(ip)).fail_count == 4 + record = await store.record_failure(ip, max_attempts=2, lockout_seconds=60.0, now=T0 + 2) + assert record == LoginThrottleRecord(fail_count=5, locked_at=T0 + 2, lock_duration=60.0) + assert await store.check(ip, policy=_policy(2, 60.0), now=T0 + 3) > 0.0 + await store.reset(ip) + assert await store.check(ip, policy=_policy(2, 60.0), now=T0 + 3) == 0.0 + + +async def test_lowered_duration_releases_an_active_lock_early(store): + ip = "10.0.0.6" + await _lock(store, ip, lockout_seconds=60.0, now=T0) + assert await store.check(ip, policy=_policy(2, 60.0), now=T0 + 2) > 0.0 + assert await store.check(ip, policy=_policy(2, 1.0), now=T0 + 2) == 0.0 + assert await store.get(ip) is None + + +async def test_raised_duration_extends_an_active_lock(store): + ip = "10.0.0.7" + await _lock(store, ip, lockout_seconds=1.0, now=T0) + assert await store.check(ip, policy=_policy(2, 60.0), now=T0 + 0.5) == pytest.approx(59.5) + # The raise was committed while the lock was active, so it outlives the original 1s. + assert (await store.get(ip)).lock_duration == 60.0 + assert await store.check(ip, policy=_policy(2, 60.0), now=T0 + 2.0) == pytest.approx(58.0) + + +async def test_served_sentence_is_not_resurrected_by_a_later_raise(store): + ip = "10.0.0.8" + await _lock(store, ip, lockout_seconds=1.0, now=T0) + # No check happened while the 1s sentence ran; raising afterwards must not revive it. + assert await store.check(ip, policy=_policy(2, 60.0), now=T0 + 2.0) == 0.0 + assert await store.get(ip) is None + + +async def test_lowered_then_raised_duration_is_not_resurrected(store): + ip = "10.0.0.9" + await _lock(store, ip, lockout_seconds=60.0, now=T0) + assert await store.check(ip, policy=_policy(2, 10.0), now=T0 + 6.0) == pytest.approx(4.0) + assert (await store.get(ip)).lock_duration == 10.0 + assert await store.check(ip, policy=_policy(2, 30.0), now=T0 + 20.0) == 0.0 + assert await store.get(ip) is None + + +async def test_concurrent_failures_do_not_lose_increments(store): + """The upsert must be atomic: N racing failures count N, never fewer.""" + ip = "10.0.0.10" + await asyncio.gather(*[store.record_failure(ip, max_attempts=100, lockout_seconds=60.0, now=T0 + i) for i in range(25)]) + assert (await store.get(ip)).fail_count == 25 + + +async def test_concurrent_checks_of_an_expired_lock_all_resolve_cleanly(store): + ip = "10.0.0.11" + await _lock(store, ip, lockout_seconds=1.0, now=T0) + results = await asyncio.gather(*[store.check(ip, policy=_policy(2, 1.0), now=T0 + 5.0) for _ in range(8)], return_exceptions=True) + assert results == [0.0] * 8, results + assert await store.get(ip) is None + + +async def test_now_defaults_to_the_wall_clock(store): + ip = "10.0.0.12" + record = await store.record_failure(ip, max_attempts=2, lockout_seconds=60.0) + assert record.fail_count == 1 and record.locked_at == 0.0 + record = await store.record_failure(ip, max_attempts=2, lockout_seconds=60.0) + assert record.locked_at > T0 # a real timestamp, after this test was written + assert 0.0 < await store.check(ip, policy=_policy(2, 60.0)) <= 60.0 + + +async def test_failures_during_an_active_lock_do_not_slide_the_sentence(store): + """A failure recorded while the lock is active keeps the lock's start and committed duration. + + Review of #6501: re-stamping ``locked_at = now`` on every failure turned + "N seconds after the lock started" into "N seconds after the last + attempt", so a sustained attacker never served the sentence. + """ + ip = "10.0.0.20" + locked = await _lock(store, ip, max_attempts=2, lockout_seconds=60.0, now=T0) + assert locked == LoginThrottleRecord(fail_count=2, locked_at=T0, lock_duration=60.0) + for n, offset in enumerate((10.0, 20.0, 50.0), start=3): + record = await store.record_failure(ip, max_attempts=2, lockout_seconds=60.0, now=T0 + offset) + assert record == LoginThrottleRecord(fail_count=n, locked_at=T0, lock_duration=60.0) + assert await store.check(ip, policy=_policy(2, 60.0), now=T0 + 59.0) == pytest.approx(1.0) + assert await store.check(ip, policy=_policy(2, 60.0), now=T0 + 61.0) == 0.0 # expires at the original time + assert await store.get(ip) is None + + +async def test_failure_during_an_active_lock_keeps_the_committed_duration_not_the_live_one(store): + """The duration a ``check`` committed (here a lowered one) is what a later failure preserves.""" + ip = "10.0.0.21" + await _lock(store, ip, max_attempts=2, lockout_seconds=60.0, now=T0) + assert await store.check(ip, policy=_policy(2, 10.0), now=T0 + 2.0) == pytest.approx(8.0) # commits 10s + record = await store.record_failure(ip, max_attempts=2, lockout_seconds=300.0, now=T0 + 5.0) + assert record == LoginThrottleRecord(fail_count=3, locked_at=T0, lock_duration=10.0) + + +async def test_failure_after_a_served_lock_starts_a_fresh_sentence(store): + """A lock whose sentence elapsed (no check cleared it yet) is restarted by the next failure.""" + ip = "10.0.0.22" + await _lock(store, ip, max_attempts=2, lockout_seconds=1.0, now=T0) + record = await store.record_failure(ip, max_attempts=2, lockout_seconds=60.0, now=T0 + 5.0) + assert record == LoginThrottleRecord(fail_count=3, locked_at=T0 + 5.0, lock_duration=60.0) + assert await store.check(ip, policy=_policy(2, 60.0), now=T0 + 6.0) == pytest.approx(59.0) + + +async def test_raised_threshold_clears_the_lock_below_the_new_max_and_restarts_it_when_reached(store): + """Raising max_login_attempts mid-lock: a failure below the new threshold leaves the IP counting; + the failure that reaches the new threshold starts a fresh lock at that moment.""" + ip = "10.0.0.23" + await _lock(store, ip, max_attempts=2, lockout_seconds=60.0, now=T0) # (2, T0, 60) + record = await store.record_failure(ip, max_attempts=5, lockout_seconds=60.0, now=T0 + 1.0) + assert record == LoginThrottleRecord(fail_count=3, locked_at=0.0, lock_duration=0.0) # counting again under the raised max + assert await store.check(ip, policy=_policy(5, 60.0), now=T0 + 1.5) == 0.0 + record = await store.record_failure(ip, max_attempts=5, lockout_seconds=60.0, now=T0 + 2.0) + assert record == LoginThrottleRecord(fail_count=4, locked_at=0.0, lock_duration=0.0) + record = await store.record_failure(ip, max_attempts=5, lockout_seconds=60.0, now=T0 + 3.0) + assert record == LoginThrottleRecord(fail_count=5, locked_at=T0 + 3.0, lock_duration=60.0) # fresh lock, not the stale T0 + assert await store.check(ip, policy=_policy(5, 60.0), now=T0 + 4.0) == pytest.approx(59.0) + + +async def test_check_never_resolves_the_policy_for_a_clean_ip(store): + """The clean-IP skip survives the lazy policy: no record, no config read.""" + policy = _CountingPolicy(5, 300.0) + assert await store.check("192.0.2.200", policy=policy, now=T0) == 0.0 + assert policy.calls == 0 + + +async def test_check_resolves_the_policy_exactly_once_for_a_recorded_ip(store): + ip = "10.0.0.30" + await store.record_failure(ip, max_attempts=5, lockout_seconds=300.0, now=T0) + policy = _CountingPolicy(5, 300.0) + assert await store.check(ip, policy=policy, now=T0 + 1) == 0.0 + assert policy.calls == 1 + await _lock(store, ip, max_attempts=2, lockout_seconds=60.0, now=T0 + 2) + policy = _CountingPolicy(2, 60.0) + assert await store.check(ip, policy=policy, now=T0 + 3) > 0.0 + assert policy.calls == 1 + + +# ── memory store specifics ────────────────────────────────────────────────── + + +async def test_memory_eviction_expires_by_stored_sentence_not_current_threshold(): + """The capacity sweep expires records by their own committed sentence. + + A record locked under an old, lower threshold has a count below the live + max; gating expiry on ``count >= max`` would keep that served record + resident while the capacity fallback evicts live counters first (they sort + earliest), handing an active offender a fresh budget. + """ + store = MemoryLoginThrottleStore(max_tracked_ips=2) + # Seed the lock first: the capacity sweep runs inside record_failure once + # the set is full, and this test wants the sweep to run on the fresh IP. + await _lock(store, "expired-lock", max_attempts=2, lockout_seconds=1.0, now=10.0) # served at 11.0 + await store.record_failure("live-counter", max_attempts=3, lockout_seconds=60.0, now=90.0) + + await store.record_failure("fresh-ip", max_attempts=3, lockout_seconds=60.0, now=100.0) # hits the sweep + + assert await store.get("expired-lock") is None + assert await store.get("live-counter") == LoginThrottleRecord(fail_count=1, locked_at=0.0, lock_duration=0.0) + assert (await store.get("fresh-ip")).fail_count == 1 + + +async def test_memory_eviction_drops_the_cheapest_half_when_nothing_expired(): + store = MemoryLoginThrottleStore(max_tracked_ips=4) + await _lock(store, "lock-early", lockout_seconds=60.0, now=T0) + await _lock(store, "lock-late", lockout_seconds=600.0, now=T0) + await store.record_failure("counter-a", max_attempts=5, lockout_seconds=60.0, now=T0) + await store.record_failure("counter-b", max_attempts=5, lockout_seconds=60.0, now=T0) + + await store.record_failure("fresh-ip", max_attempts=5, lockout_seconds=60.0, now=T0 + 1) + + # Never-locked counters sort first (expiry key 0.0) and go; locks survive. + assert await store.get("counter-a") is None + assert await store.get("counter-b") is None + assert (await store.get("lock-early")).locked_at == T0 + assert (await store.get("lock-late")).locked_at == T0 + assert (await store.get("fresh-ip")).fail_count == 1 + + +async def test_memory_store_default_capacity_matches_the_historical_constant(): + assert MAX_TRACKED_IPS == 10000 + assert MemoryLoginThrottleStore()._max_tracked_ips == MAX_TRACKED_IPS + + +# ── SQL store specifics ───────────────────────────────────────────────────── + + +class _CountingSession: + """Session proxy counting ``commit()`` calls.""" + + def __init__(self, inner, counters: dict) -> None: + self._inner = inner + self._counters = counters + + async def __aenter__(self): + await self._inner.__aenter__() + return self + + async def __aexit__(self, *exc): + return await self._inner.__aexit__(*exc) + + async def commit(self) -> None: + self._counters["commits"] += 1 + await self._inner.commit() + + def __getattr__(self, name): + return getattr(self._inner, name) + + +def _counting_store(engine: AsyncEngine) -> tuple[SqlLoginThrottleStore, dict]: + """A SQL store whose session factory counts sessions opened and commits issued.""" + real = async_sessionmaker(engine, expire_on_commit=False) + counters = {"sessions": 0, "commits": 0} + + def factory(): + counters["sessions"] += 1 + return _CountingSession(real(), counters) + + return SqlLoginThrottleStore(factory), counters + + +async def test_sql_check_opens_exactly_one_session_per_call(tmp_path): + """A recorded IP no longer costs two pool checkouts (probe + decision) on the unauthenticated login path.""" + engine = await _sqlite_engine(tmp_path / "sessions.db") + try: + store, counters = _counting_store(engine) + ip = "10.0.0.31" + assert await store.check(ip, policy=_policy(5, 300.0), now=T0) == 0.0 # clean IP + assert counters["sessions"] == 1 + await _lock(store, ip, max_attempts=2, lockout_seconds=60.0, now=T0) + counters["sessions"] = 0 + assert await store.check(ip, policy=_policy(2, 300.0), now=T0 + 1) > 0.0 # recorded IP, duration committed + assert counters["sessions"] == 1 + counters["sessions"] = 0 + assert await store.check(ip, policy=_policy(2, 300.0), now=T0 + 2) > 0.0 # recorded IP, nothing to write + assert counters["sessions"] == 1 + finally: + await engine.dispose() + + +async def test_sql_record_failure_commits_one_transaction(tmp_path): + """Housekeeping rides in the upsert's transaction: one commit per failed login, not two.""" + engine = await _sqlite_engine(tmp_path / "commits.db") + try: + store, counters = _counting_store(engine) + await _lock(store, "other-served-lock", lockout_seconds=1.0, now=T0 - 10.0) + counters["commits"] = 0 + await store.record_failure("10.0.0.32", max_attempts=5, lockout_seconds=300.0, now=T0) + assert counters["commits"] == 1 + assert await store.get("other-served-lock") is None # the sweep still ran, inside that transaction + finally: + await engine.dispose() + + +async def test_sql_record_failure_sweeps_expired_locks_and_stale_counters(tmp_path): + engine = await _sqlite_engine(tmp_path / "sweep.db") + try: + store = _sql_store(engine) + await _lock(store, "expired-lock", lockout_seconds=1.0, now=T0) # served at T0 + 1 + await store.record_failure("stale-counter", max_attempts=5, lockout_seconds=60.0, now=T0 - STALE_COUNTER_SECONDS - 1) + await store.record_failure("live-counter", max_attempts=5, lockout_seconds=60.0, now=T0) + await _lock(store, "live-lock", lockout_seconds=600.0, now=T0) + + await store.record_failure("fresh-ip", max_attempts=5, lockout_seconds=60.0, now=T0 + 5) + + async with engine.connect() as conn: + ips = set((await conn.execute(sa.select(LoginThrottleRow.ip))).scalars()) + assert ips == {"live-counter", "live-lock", "fresh-ip"} + finally: + await engine.dispose() + + +async def test_sql_sweep_leaves_the_row_being_recorded_to_the_upsert(tmp_path): + """The sweep excludes the IP whose failure is being recorded, so the upsert applies the shared rule. + + A stale counter keeps counting (the memory store never expires counters on + its own either) instead of collapsing into a fresh ``(1, NULL, NULL)`` + row, while other expired rows are still swept by the same call. The + served-lock half of the rule is pinned for both stores by + ``test_failure_after_a_served_lock_starts_a_fresh_sentence``. + """ + engine = await _sqlite_engine(tmp_path / "keep.db") + try: + store = _sql_store(engine) + stale_now = T0 - STALE_COUNTER_SECONDS - 1 + for _ in range(4): + await store.record_failure("stale-counter", max_attempts=5, lockout_seconds=60.0, now=stale_now) + await _lock(store, "other-served-lock", lockout_seconds=1.0, now=T0 - 10.0) + + record = await store.record_failure("stale-counter", max_attempts=5, lockout_seconds=60.0, now=T0) + assert record == LoginThrottleRecord(fail_count=5, locked_at=T0, lock_duration=60.0) # 4 + 1 reaches the threshold + assert await store.get("other-served-lock") is None # swept by that same call + finally: + await engine.dispose() + + +async def test_sql_stores_truncate_overlong_keys_consistently(tmp_path, caplog): + """A trusted proxy can forward an arbitrarily long X-Real-IP; the row key is bounded and the truncation is logged.""" + import logging + + engine = await _sqlite_engine(tmp_path / "long.db") + try: + store = _sql_store(engine) + key = "x" * 400 + with caplog.at_level(logging.WARNING): + await _lock(store, key, lockout_seconds=60.0, now=T0) + truncation_warnings = [r.message for r in caplog.records if "truncat" in r.message.lower()] + assert truncation_warnings, caplog.records + assert "400" in truncation_warnings[0] and str(LoginThrottleRow.ip.type.length) in truncation_warnings[0] + assert key not in truncation_warnings[0] # never log the (possibly token-like) key itself + assert await store.check(key, policy=_policy(2, 60.0), now=T0 + 1) > 0.0 + async with engine.connect() as conn: + stored = (await conn.execute(sa.select(LoginThrottleRow.ip))).scalar_one() + assert len(stored) == LoginThrottleRow.ip.type.length + await store.reset(key) + assert await store.get(key) is None + finally: + await engine.dispose() + + +async def test_two_sql_stores_over_one_database_share_the_lock(tmp_path): + """Multi-replica reproduction: replica B sees the failures replica A counted. + + Two engines over one SQLite file stand in for two Gateway Pods sharing one + database. Before this change each Pod kept its own counter, so an attacker + behind a load balancer got N x max_login_attempts guesses and a lockout on + one Pod was invisible to the others. + """ + path = tmp_path / "shared.db" + engine_a = await _sqlite_engine(path) + engine_b = create_async_engine(f"sqlite+aiosqlite:///{path.as_posix()}") + try: + replica_a, replica_b = _sql_store(engine_a), _sql_store(engine_b) + ip = "198.51.100.7" + for _ in range(3): + await replica_a.record_failure(ip, max_attempts=5, lockout_seconds=300.0, now=T0) + for _ in range(2): + await replica_b.record_failure(ip, max_attempts=5, lockout_seconds=300.0, now=T0 + 1) + assert (await replica_a.get(ip)).fail_count == 5 + assert await replica_a.check(ip, policy=_policy(5, 300.0), now=T0 + 2) > 0.0 + assert await replica_b.check(ip, policy=_policy(5, 300.0), now=T0 + 2) > 0.0 + # A successful login on one replica releases the IP everywhere. + await replica_b.reset(ip) + assert await replica_a.check(ip, policy=_policy(5, 300.0), now=T0 + 3) == 0.0 + finally: + await engine_a.dispose() + await engine_b.dispose() + + +async def test_failure_committing_after_a_reset_counts_as_a_fresh_first_failure(tmp_path): + """A reset and a peer's failure serialize at the database; whichever commits last defines the state. + + A failure that commits after the successful login's reset leaves + ``fail_count = 1`` — one genuine failed attempt after the history was + cleared, not a resurrected counter or lock. (A failure that commits + before the reset is cleared by it.) The memory store has the same ordering + semantics; "a successful login clears the IP everywhere" holds either way. + """ + path = tmp_path / "reset-race.db" + engine_a = await _sqlite_engine(path) + engine_b = create_async_engine(f"sqlite+aiosqlite:///{path.as_posix()}") + try: + replica_a, replica_b = _sql_store(engine_a), _sql_store(engine_b) + ip = "198.51.100.9" + await _lock(replica_a, ip, max_attempts=5, lockout_seconds=300.0, now=T0) + await replica_a.reset(ip) # successful login on replica A + record = await replica_b.record_failure(ip, max_attempts=5, lockout_seconds=300.0, now=T0 + 1.0) # peer's wrong password lands after + assert record == LoginThrottleRecord(fail_count=1, locked_at=0.0, lock_duration=0.0) + assert await replica_a.get(ip) == record + assert await replica_a.check(ip, policy=_policy(5, 300.0), now=T0 + 2.0) == 0.0 + finally: + await engine_a.dispose() + await engine_b.dispose() + + +async def test_two_memory_stores_do_not_share_the_lock(): + """Documents the per-process behavior the SQL store exists to replace.""" + replica_a, replica_b = MemoryLoginThrottleStore(), MemoryLoginThrottleStore() + ip = "198.51.100.8" + for _ in range(5): + await replica_a.record_failure(ip, max_attempts=5, lockout_seconds=300.0, now=T0) + assert await replica_a.check(ip, policy=_policy(5, 300.0), now=T0 + 1) > 0.0 + assert await replica_b.check(ip, policy=_policy(5, 300.0), now=T0 + 1) == 0.0 + assert await replica_b.get(ip) is None + + +# ── sweep versus a concurrent lock extension ───────────────────────────────── + + +def _outer_where_without_candidate_subquery(sql: str) -> str: + """The DELETE's own WHERE clause with the ``IN (SELECT ...)`` candidate subquery replaced by ``(...)``.""" + outer = sql[sql.index("WHERE") :] + start = outer.index("IN (") + len("IN ") + depth, end = 0, start + for end in range(start, len(outer)): + depth += {"(": 1, ")": -1}.get(outer[end], 0) + if depth == 0: + break + return outer[:start] + "(...)" + outer[end + 1 :] + + +@pytest.mark.parametrize("dialect", [postgresql.dialect(), sqlite.dialect()], ids=["postgresql", "sqlite"]) +async def test_sweep_delete_repeats_the_expiry_predicate_on_the_deleted_row(dialect): + """The sweep's DELETE must re-check expiry on the row it deletes, not only IP membership. + + PostgreSQL READ COMMITTED re-evaluates only the DELETE's *own* WHERE on a + row it had to wait for; the ``IN (SELECT ... LIMIT n)`` candidate subquery is + bound to the statement snapshot. With ``ip IN (...)`` alone, a lock that a + concurrent ``check`` extended between the snapshot and the row lock would + still be deleted (review of #6501). Compiling the statement pins the guard + without a live PostgreSQL. + """ + sql = str(SqlLoginThrottleStore.sweep_statement(T0).compile(dialect=dialect)) + assert sql.startswith("DELETE FROM login_throttle") + assert "LIMIT" in sql # bounded candidate selection is kept + outer = _outer_where_without_candidate_subquery(sql) + assert "login_throttle.ip IN" in outer + assert "login_throttle.locked_at IS NOT NULL" in outer + assert "login_throttle.locked_at + login_throttle.lock_duration_seconds <=" in outer + assert "login_throttle.locked_at IS NULL" in outer + assert "login_throttle.updated_at <=" in outer + # The row being recorded is excluded on the target too, not only among the candidates. + with_keep = str(SqlLoginThrottleStore.sweep_statement(T0, keep="203.0.113.1").compile(dialect=dialect)) + assert "login_throttle.ip !=" in _outer_where_without_candidate_subquery(with_keep) + + +async def _wait_until_a_session_waits_on_a_lock(engine: AsyncEngine, *, timeout: float = 15.0) -> None: + """Block until another backend of this database is waiting on a lock (pg_stat_activity).""" + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + async with engine.connect() as conn: + waiting = await conn.scalar(sa.text("SELECT count(*) FROM pg_stat_activity WHERE datname = current_database() AND wait_event_type = 'Lock' AND pid <> pg_backend_pid()")) + if waiting: + return + await asyncio.sleep(0.05) + raise AssertionError("the sweeping replica never blocked on the extending replica's row lock") + + +async def test_postgres_sweep_keeps_a_lock_extended_by_a_concurrent_check(): + """Reproduces the #6501 review interleaving on a real PostgreSQL. + + Replica A evaluates an expiring lock under a raised ``lockout_seconds`` + and extends it (the UPDATE ``check`` issues) without committing yet. An + unrelated failed login on replica B sweeps: its snapshot still lists the IP + as expired, its DELETE waits on A's row lock, and once A commits the + re-evaluated row must survive — otherwise the next attempt from that IP + gets a clean record and reaches password verification. + """ + uri = os.environ.get("TEST_POSTGRES_URI") + if not uri: + pytest.skip("requires TEST_POSTGRES_URI (PostgreSQL READ COMMITTED sweep/update interleaving)") + schema = f"login_throttle_sweep_{uuid.uuid4().hex}" + engines = [create_async_engine(asyncpg_test_url(uri), connect_args=build_asyncpg_connect_args(schema)) for _ in range(2)] + engine_a, engine_b = engines + try: + async with engine_a.begin() as conn: + await conn.execute(sa.text(f'CREATE SCHEMA "{schema}"')) + await conn.run_sync(Base.metadata.create_all, tables=[LoginThrottleRow.__table__]) + session_factory_a = async_sessionmaker(engine_a, expire_on_commit=False) + replica_a, replica_b = _sql_store(engine_a), _sql_store(engine_b) + ip = "198.51.100.42" + await _lock(replica_a, ip, lockout_seconds=1.0, now=T0) # sentence ends at T0 + 1 + + async with session_factory_a() as extending: + # Replica A at T0 + 0.5 under lockout_seconds=60: the lock is still + # active, so check() commits the longer sentence — held open here. + await extending.execute(sa.update(LoginThrottleRow).where(LoginThrottleRow.ip == ip).values(lock_duration_seconds=60.0, updated_at=datetime.fromtimestamp(T0 + 0.5, UTC))) + # Replica B at T0 + 2: an unrelated failure sweeps; its snapshot sees + # the IP as expired and the DELETE blocks on A's row lock. + sweeping = asyncio.create_task(replica_b.record_failure("203.0.113.5", max_attempts=5, lockout_seconds=60.0, now=T0 + 2.0)) + await _wait_until_a_session_waits_on_a_lock(engine_a) + await extending.commit() + await sweeping + + assert await replica_a.get(ip) == LoginThrottleRecord(fail_count=2, locked_at=T0, lock_duration=60.0) + assert await replica_b.check(ip, policy=_policy(2, 60.0), now=T0 + 2.0) == pytest.approx(58.0) + assert (await replica_b.get("203.0.113.5")).fail_count == 1 # the sweeping failure was still counted + finally: + async with engine_a.begin() as conn: + await conn.execute(sa.text(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE')) + for engine in engines: + await engine.dispose() + + +# ── check(): compare-and-set misses between the snapshot read and the write ── + + +class _RacingSession: + """Session proxy that lets a peer act right after ``check`` ends its read transaction. + + ``check`` reads its snapshot and ends that transaction with ``rollback()`` + before writing. Running the peer mutation at that point — on another + engine over the same database — is exactly the reviewer's interleaving + (#6501): the compare-and-set that follows sees a row that changed since + the snapshot. The hook fires after the first read only unless + ``every_time`` is set, which forces a miss on every attempt. + """ + + def __init__(self, inner, on_read_end: Callable[[], Awaitable[None]], state, *, every_time: bool) -> None: + self._inner = inner + self._on_read_end = on_read_end + self._state = state + self._every_time = every_time + + async def __aenter__(self): + await self._inner.__aenter__() + return self + + async def __aexit__(self, *exc): + return await self._inner.__aexit__(*exc) + + async def rollback(self) -> None: + await self._inner.rollback() + if self._every_time or self._state["fired"] == 0: + self._state["fired"] += 1 + await self._on_read_end() + + def __getattr__(self, name): + return getattr(self._inner, name) + + +def _racing_store(engine: AsyncEngine, on_read_end: Callable[[], Awaitable[None]], *, every_time: bool = False) -> tuple[SqlLoginThrottleStore, dict]: + real = async_sessionmaker(engine, expire_on_commit=False) + state = {"fired": 0} + return SqlLoginThrottleStore(lambda: _RacingSession(real(), on_read_end, state, every_time=every_time)), state + + +async def _two_replicas(path: Path): + engine_a = await _sqlite_engine(path) + engine_b = create_async_engine(f"sqlite+aiosqlite:///{path.as_posix()}") + return engine_a, engine_b + + +async def test_check_recommits_a_raised_duration_after_a_concurrent_increment(tmp_path): + """Reviewer's interleaving: the extension must be committed, not merely reported. + + Lock (5, T, 1s); operator raises lockout_seconds to 60. Replica A reads + the row at T+0.5; before A's UPDATE an admitted wrong password on replica + B increments the count (keeping the active lock's 1s duration). A's + compare-and-set misses on ``fail_count``; it must re-read and commit the + 60s sentence on the fresh row, so a check at T+2 still reports locked. + """ + engine_a, engine_b = await _two_replicas(tmp_path / "raise.db") + try: + replica_b = _sql_store(engine_b) + ip = "198.51.100.50" + for _ in range(5): + await replica_b.record_failure(ip, max_attempts=5, lockout_seconds=1.0, now=T0) + + async def peer_failure() -> None: + await replica_b.record_failure(ip, max_attempts=5, lockout_seconds=1.0, now=T0 + 0.5) + + replica_a, state = _racing_store(engine_a, peer_failure) + policy = _CountingPolicy(5, 60.0) + assert await replica_a.check(ip, policy=policy, now=T0 + 0.5) == pytest.approx(59.5) + assert state["fired"] == 1 + assert policy.calls == 1 # resolved once, even though the compare-and-set retried + assert await replica_b.get(ip) == LoginThrottleRecord(fail_count=6, locked_at=T0, lock_duration=60.0) # increment kept, extension committed + assert await replica_b.check(ip, policy=_policy(5, 60.0), now=T0 + 2.0) == pytest.approx(58.0) + finally: + await engine_a.dispose() + await engine_b.dispose() + + +async def test_check_recommits_a_lowered_duration_after_a_concurrent_increment(tmp_path): + engine_a, engine_b = await _two_replicas(tmp_path / "lower.db") + try: + replica_b = _sql_store(engine_b) + ip = "198.51.100.51" + for _ in range(5): + await replica_b.record_failure(ip, max_attempts=5, lockout_seconds=60.0, now=T0) + + async def peer_failure() -> None: + await replica_b.record_failure(ip, max_attempts=5, lockout_seconds=60.0, now=T0 + 6.0) + + replica_a, state = _racing_store(engine_a, peer_failure) + assert await replica_a.check(ip, policy=_policy(5, 10.0), now=T0 + 6.0) == pytest.approx(4.0) + assert state["fired"] == 1 + assert await replica_b.get(ip) == LoginThrottleRecord(fail_count=6, locked_at=T0, lock_duration=10.0) # the decrease is committed + assert await replica_b.check(ip, policy=_policy(5, 30.0), now=T0 + 12.0) == 0.0 # 10s sentence served; not resurrected + finally: + await engine_a.dispose() + await engine_b.dispose() + + +async def test_check_releases_early_on_the_fresh_row_after_a_concurrent_increment(tmp_path): + engine_a, engine_b = await _two_replicas(tmp_path / "release.db") + try: + replica_b = _sql_store(engine_b) + ip = "198.51.100.52" + for _ in range(5): + await replica_b.record_failure(ip, max_attempts=5, lockout_seconds=60.0, now=T0) + + async def peer_failure() -> None: + await replica_b.record_failure(ip, max_attempts=5, lockout_seconds=60.0, now=T0 + 20.0) + + replica_a, state = _racing_store(engine_a, peer_failure) + # Lowered to 10s and 20s have passed: release early — on the row as it is now. + assert await replica_a.check(ip, policy=_policy(5, 10.0), now=T0 + 20.0) == 0.0 + assert state["fired"] == 1 + assert await replica_b.get(ip) is None + finally: + await engine_a.dispose() + await engine_b.dispose() + + +async def test_check_allows_when_a_reset_lands_between_read_and_write(tmp_path): + """A successful login on a peer clears the row mid-check: allowed, and nothing is recreated.""" + engine_a, engine_b = await _two_replicas(tmp_path / "reset.db") + try: + replica_b = _sql_store(engine_b) + ip = "198.51.100.53" + for _ in range(5): + await replica_b.record_failure(ip, max_attempts=5, lockout_seconds=1.0, now=T0) + + async def peer_success() -> None: + await replica_b.reset(ip) + + replica_a, state = _racing_store(engine_a, peer_success) + assert await replica_a.check(ip, policy=_policy(5, 60.0), now=T0 + 0.5) == 0.0 + assert state["fired"] == 1 + assert await replica_b.get(ip) is None + finally: + await engine_a.dispose() + await engine_b.dispose() + + +async def test_check_fails_closed_when_the_compare_and_set_keeps_missing(tmp_path, caplog): + """Sustained contention exhausts the bounded retries: report the last snapshot under the live policy, write nothing.""" + import logging + + from deerflow.persistence.login_throttle.sql import CHECK_CAS_ATTEMPTS + + engine_a, engine_b = await _two_replicas(tmp_path / "exhaust.db") + try: + replica_b = _sql_store(engine_b) + ip = "198.51.100.54" + for _ in range(5): + await replica_b.record_failure(ip, max_attempts=5, lockout_seconds=1.0, now=T0) + + async def peer_failure() -> None: + await replica_b.record_failure(ip, max_attempts=5, lockout_seconds=1.0, now=T0 + 0.5) + + replica_a, state = _racing_store(engine_a, peer_failure, every_time=True) + with caplog.at_level(logging.DEBUG, logger="deerflow.persistence.login_throttle.sql"): + remaining = await replica_a.check(ip, policy=_policy(5, 60.0), now=T0 + 0.5) + assert remaining == pytest.approx(59.5) # locked snapshot under the live 60s policy: fail closed + # One probe read before the policy is resolved, then one read per compare-and-set attempt. + assert state["fired"] == 1 + CHECK_CAS_ATTEMPTS + # Nothing was committed by the losing checks; every peer increment survived. + assert await replica_b.get(ip) == LoginThrottleRecord(fail_count=5 + 1 + CHECK_CAS_ATTEMPTS, locked_at=T0, lock_duration=1.0) + assert any("compare-and-set" in r.message for r in caplog.records if r.levelno == logging.DEBUG) + finally: + await engine_a.dispose() + await engine_b.dispose() diff --git a/backend/tests/test_migration_0025_repair_run_change_seq.py b/backend/tests/test_migration_0025_repair_run_change_seq.py index bd1e9083c2..8b66de78b8 100644 --- a/backend/tests/test_migration_0025_repair_run_change_seq.py +++ b/backend/tests/test_migration_0025_repair_run_change_seq.py @@ -48,7 +48,7 @@ def _seed_database_that_skipped_0023(db_path) -> None: # change_seq, so it goes before the column. conn.execute(sa.text("DROP INDEX IF EXISTS ix_runs_thread_change_seq")) conn.execute(sa.text("ALTER TABLE runs DROP COLUMN origin_kind")) - for table in ("thread_read_markers", "thread_read_versions", "scheduled_task_events"): + for table in ("login_throttle", "thread_read_markers", "thread_read_versions", "scheduled_task_events"): conn.execute(sa.text(f"DROP TABLE IF EXISTS {table}")) # Remove everything 0023_run_change_seq owns. conn.execute(sa.text("DROP INDEX IF EXISTS ix_runs_change_seq")) diff --git a/backend/tests/test_migration_0027_0028_notification_outbox.py b/backend/tests/test_migration_0027_0028_notification_outbox.py index e7b72b4465..2e024f31f2 100644 --- a/backend/tests/test_migration_0027_0028_notification_outbox.py +++ b/backend/tests/test_migration_0027_0028_notification_outbox.py @@ -23,7 +23,7 @@ OUTBOX = "0027_notification_deliveries" PARKED = "0028_parked_attempts" SCHEDULER = "0029_scheduler_agent_tasks" CLAIM_TOKENS = "0030_notification_claim_tokens" -HEAD = "0034_run_event_seq_watermark" +HEAD = "0035_login_throttle" PREVIOUS = "0026_mcp_task_lease_tokens" TABLE = "notification_deliveries" diff --git a/backend/tests/test_migration_0029_scheduler_agent_tasks.py b/backend/tests/test_migration_0029_scheduler_agent_tasks.py index 43673413b1..16fc385519 100644 --- a/backend/tests/test_migration_0029_scheduler_agent_tasks.py +++ b/backend/tests/test_migration_0029_scheduler_agent_tasks.py @@ -23,7 +23,7 @@ from deerflow.persistence.postgres_schema import build_asyncpg_connect_args from deerflow.persistence.scheduled_tasks import ScheduledTaskRepository REVISION = "0029_scheduler_agent_tasks" -CURRENT_HEAD = "0034_run_event_seq_watermark" +CURRENT_HEAD = "0035_login_throttle" NEXT = "0030_notification_claim_tokens" PREVIOUS = "0028_parked_attempts" TASK_FIELDS = {"origin_thread_id", "goal_objective", "max_runs", "end_at", "standing_notes"} diff --git a/backend/tests/test_migration_0032_activity_and_task_events.py b/backend/tests/test_migration_0032_activity_and_task_events.py index c6644be559..0d8d1c7ea0 100644 --- a/backend/tests/test_migration_0032_activity_and_task_events.py +++ b/backend/tests/test_migration_0032_activity_and_task_events.py @@ -18,7 +18,8 @@ from deerflow.persistence.postgres_schema import build_asyncpg_connect_args REVISION = "0032_activity_and_task_events" NEXT = "0033_batch_result_artifact" -CURRENT_HEAD = "0034_run_event_seq_watermark" +AFTER_NEXT = "0034_run_event_seq_watermark" +CURRENT_HEAD = "0035_login_throttle" PREVIOUS = "0031_scheduled_streak_boundary" TABLES = {"thread_read_markers", "thread_read_versions", "scheduled_task_events"} INDEXES = { @@ -34,7 +35,8 @@ async def test_0032_remains_in_the_single_migration_chain(): assert script.get_heads() == [CURRENT_HEAD] assert script.get_revision(REVISION).down_revision == PREVIOUS assert script.get_revision(NEXT).down_revision == REVISION - assert script.get_revision(CURRENT_HEAD).down_revision == NEXT + assert script.get_revision(AFTER_NEXT).down_revision == NEXT + assert script.get_revision(CURRENT_HEAD).down_revision == AFTER_NEXT # alembic_version.version_num is VARCHAR(32). assert len(REVISION) <= 32 diff --git a/backend/tests/test_migration_0034_run_event_seq_watermark.py b/backend/tests/test_migration_0034_run_event_seq_watermark.py index 1e2f9adfda..6bee769c1a 100644 --- a/backend/tests/test_migration_0034_run_event_seq_watermark.py +++ b/backend/tests/test_migration_0034_run_event_seq_watermark.py @@ -19,14 +19,16 @@ from deerflow.persistence.postgres_schema import build_asyncpg_connect_args REVISION = "0034_run_event_seq_watermark" PREVIOUS = "0033_batch_result_artifact" +CURRENT_HEAD = "0035_login_throttle" TABLE = "run_event_thread_seq" pytestmark = pytest.mark.asyncio -async def test_0034_is_the_single_migration_head(): +async def test_0034_remains_in_the_single_migration_chain(): script = ScriptDirectory(str(bootstrap._MIGRATIONS_DIR)) - assert script.get_heads() == [REVISION] + assert script.get_heads() == [CURRENT_HEAD] assert script.get_revision(REVISION).down_revision == PREVIOUS + assert script.get_revision(CURRENT_HEAD).down_revision == REVISION assert len(REVISION) <= 32 diff --git a/backend/tests/test_migration_0035_login_throttle.py b/backend/tests/test_migration_0035_login_throttle.py new file mode 100644 index 0000000000..f5c9b11621 --- /dev/null +++ b/backend/tests/test_migration_0035_login_throttle.py @@ -0,0 +1,182 @@ +"""Migration 0035: the shared ``login_throttle`` table for cross-replica lockouts.""" + +from __future__ import annotations + +import asyncio +import os +import uuid + +import pytest +import sqlalchemy as sa +from alembic import command +from alembic.autogenerate import compare_metadata +from alembic.migration import MigrationContext +from alembic.script import ScriptDirectory +from sqlalchemy.ext.asyncio import create_async_engine +from support.postgres import asyncpg_test_url + +from deerflow.persistence import bootstrap +from deerflow.persistence.base import Base +from deerflow.persistence.login_throttle import LoginThrottleRow +from deerflow.persistence.postgres_schema import build_asyncpg_connect_args + +REVISION = "0035_login_throttle" +PREVIOUS = "0034_run_event_seq_watermark" +TABLE = "login_throttle" +COLUMNS = {"ip", "fail_count", "locked_at", "lock_duration_seconds", "updated_at"} +INDEX = "ix_login_throttle_updated_at" +pytestmark = pytest.mark.asyncio + + +async def test_0035_is_the_single_head_and_chains_after_0034(): + script = ScriptDirectory(str(bootstrap._MIGRATIONS_DIR)) + assert script.get_heads() == [REVISION] + assert script.get_revision(REVISION).down_revision == PREVIOUS + # alembic_version.version_num is VARCHAR(32). + assert len(REVISION) <= 32 + + +def _engine(tmp_path, backend): + if backend == "sqlite": + return create_async_engine(f"sqlite+aiosqlite:///{tmp_path / 'migration.db'}"), None + uri = os.environ.get("TEST_POSTGRES_URI") + if not uri: + pytest.skip("requires TEST_POSTGRES_URI (real Postgres migration 0035)") + schema = f"login_throttle_{uuid.uuid4().hex}" + return create_async_engine(asyncpg_test_url(uri), connect_args=build_asyncpg_connect_args(schema)), schema + + +async def _shape(engine) -> dict: + async with engine.connect() as conn: + + def read(sync): + inspector = sa.inspect(sync) + tables = set(inspector.get_table_names()) + if TABLE not in tables: + return {"tables": tables} + return { + "tables": tables, + "columns": {col["name"]: col for col in inspector.get_columns(TABLE)}, + "pk": inspector.get_pk_constraint(TABLE)["constrained_columns"], + "indexes": {index["name"]: index["column_names"] for index in inspector.get_indexes(TABLE)}, + } + + return await conn.run_sync(read) + + +async def _orm_diff(engine) -> list: + """Alembic's own drift check, restricted to the table this revision owns.""" + + def only_login_throttle(obj, name, type_, reflected, compare_to): + if type_ == "table": + return name == TABLE + return getattr(getattr(obj, "table", None), "name", TABLE) == TABLE + + async with engine.connect() as conn: + + def diff(sync): + context = MigrationContext.configure(sync, opts={"include_object": only_login_throttle, "compare_type": True, "compare_server_default": True}) + return compare_metadata(context, Base.metadata) + + return await conn.run_sync(diff) + + +@pytest.mark.parametrize("backend", ["sqlite", "postgres"]) +async def test_0035_creates_the_table_matching_the_orm_and_downgrades_cleanly(tmp_path, backend): + engine, schema = _engine(tmp_path, backend) + cfg = bootstrap._get_alembic_config(engine, postgres_schema=schema or "") + try: + if schema: + async with engine.begin() as conn: + await conn.execute(sa.text(f'CREATE SCHEMA "{schema}"')) + await asyncio.to_thread(command.upgrade, cfg, PREVIOUS) + assert TABLE not in (await _shape(engine))["tables"] + + await asyncio.to_thread(command.upgrade, cfg, REVISION) + shape = await _shape(engine) + assert set(shape["columns"]) == COLUMNS + assert shape["pk"] == ["ip"] + assert shape["columns"]["fail_count"]["nullable"] is False + assert shape["columns"]["locked_at"]["nullable"] is True + assert shape["columns"]["lock_duration_seconds"]["nullable"] is True + assert shape["columns"]["updated_at"]["nullable"] is False + assert shape["indexes"][INDEX] == ["updated_at"] # serves the sweep's stale-counter predicate + # ``make migrate-rev`` would propose nothing: the revision matches the ORM model. + assert await _orm_diff(engine) == [] + + async with engine.begin() as conn: + await conn.execute(sa.insert(LoginThrottleRow).values(ip="198.51.100.1", fail_count=5, locked_at=1_700_000_000.5, lock_duration_seconds=300.0, updated_at=sa.func.now())) + with pytest.raises(sa.exc.IntegrityError): + async with engine.begin() as conn: + await conn.execute(sa.insert(LoginThrottleRow).values(ip="198.51.100.1", fail_count=1, updated_at=sa.func.now())) + async with engine.connect() as conn: + row = (await conn.execute(sa.text(f"SELECT fail_count, locked_at, lock_duration_seconds FROM {TABLE}"))).one() + assert tuple(row) == (5, 1_700_000_000.5, 300.0) + + await asyncio.to_thread(command.downgrade, cfg, PREVIOUS) + shape = await _shape(engine) + assert TABLE not in shape["tables"] + async with engine.connect() as conn: + # The index goes with the table on downgrade (named, so a leftover would be visible). + leftover = await conn.run_sync(lambda sync: [name for table in sa.inspect(sync).get_table_names() for name in {index["name"] for index in sa.inspect(sync).get_indexes(table)} if name == INDEX]) + assert leftover == [] + + # Downgrade then upgrade again is clean. + await asyncio.to_thread(command.upgrade, cfg, REVISION) + shape = await _shape(engine) + assert TABLE in shape["tables"] + assert INDEX in shape["indexes"] + finally: + if schema: + async with engine.begin() as conn: + await conn.execute(sa.text(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE')) + await engine.dispose() + + +@pytest.mark.parametrize("backend", ["sqlite", "postgres"]) +async def test_0035_completes_a_partially_applied_upgrade(tmp_path, backend): + """The table already exists (an interrupted earlier attempt): upgrade still lands.""" + engine, schema = _engine(tmp_path, backend) + cfg = bootstrap._get_alembic_config(engine, postgres_schema=schema or "") + try: + if schema: + async with engine.begin() as conn: + await conn.execute(sa.text(f'CREATE SCHEMA "{schema}"')) + await asyncio.to_thread(command.upgrade, cfg, PREVIOUS) + async with engine.begin() as conn: + # The table exists but its index was never built (an interrupted earlier attempt). + await conn.run_sync(Base.metadata.create_all, tables=[LoginThrottleRow.__table__]) + await conn.execute(sa.text(f"DROP INDEX {INDEX}")) + await conn.execute(sa.insert(LoginThrottleRow).values(ip="203.0.113.9", fail_count=2, updated_at=sa.func.now())) + + await asyncio.to_thread(command.upgrade, cfg, REVISION) + assert INDEX in (await _shape(engine))["indexes"] # the partial upgrade is completed + async with engine.connect() as conn: + assert (await conn.execute(sa.text(f"SELECT fail_count FROM {TABLE} WHERE ip='203.0.113.9'"))).scalar_one() == 2 + assert (await conn.execute(sa.text("SELECT version_num FROM alembic_version"))).scalar_one() == REVISION + + # A downgrade after someone already dropped the table still completes. + async with engine.begin() as conn: + await conn.execute(sa.text(f"DROP TABLE {TABLE}")) + await asyncio.to_thread(command.downgrade, cfg, PREVIOUS) + async with engine.connect() as conn: + assert (await conn.execute(sa.text("SELECT version_num FROM alembic_version"))).scalar_one() == PREVIOUS + finally: + if schema: + async with engine.begin() as conn: + await conn.execute(sa.text(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE')) + await engine.dispose() + + +async def test_bootstrap_provisions_the_table_on_an_empty_database_and_is_idempotent(tmp_path): + engine = create_async_engine(f"sqlite+aiosqlite:///{tmp_path / 'bootstrap.db'}") + try: + await bootstrap.bootstrap_schema(engine, backend="sqlite") + await bootstrap.bootstrap_schema(engine, backend="sqlite") + shape = await _shape(engine) + assert set(shape["columns"]) == COLUMNS + assert INDEX in shape["indexes"] + async with engine.connect() as conn: + assert (await conn.execute(sa.text("SELECT version_num FROM alembic_version"))).scalar_one() == REVISION + finally: + await engine.dispose() diff --git a/backend/tests/test_multi_worker_postgres_gate.py b/backend/tests/test_multi_worker_postgres_gate.py index 2493658d38..710c547927 100644 --- a/backend/tests/test_multi_worker_postgres_gate.py +++ b/backend/tests/test_multi_worker_postgres_gate.py @@ -18,7 +18,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest from fastapi import FastAPI -from app.gateway.deps import _enforce_postgres_for_multi_worker, _validate_agent_storage, _validate_memory_retrieval_index, langgraph_runtime +from app.gateway.deps import _enforce_postgres_for_multi_worker, _validate_agent_storage, _validate_login_throttle_storage, _validate_memory_retrieval_index, langgraph_runtime from app.gateway.routers.browser import _browser_tools_enabled from deerflow.config.database_config import DatabaseConfig from deerflow.config.deployment_config import MULTI_INSTANCE_ENV_VAR, DeploymentConfig, multi_instance_declaration @@ -710,6 +710,60 @@ def test_deployment_declaration_helpers(monkeypatch): # --------------------------------------------------------------------------- +# Login throttle storage warning (auth.local.throttle_storage) +# --------------------------------------------------------------------------- + + +def _with_throttle_storage(config, selector): + config.auth = SimpleNamespace(local=SimpleNamespace(throttle_storage=selector)) + return config + + +def _throttle_warnings(caplog): + return [r.message for r in caplog.records if "auth.local.throttle_storage" in r.message] + + +def test_login_throttle_warning_fires_for_a_declared_multi_instance_deployment_on_memory(caplog): + """Explicit memory counters under N replicas hand an attacker N x max_login_attempts guesses.""" + from deerflow.config.auth_config import LoginThrottleStorage + + with caplog.at_level("WARNING"): + _validate_login_throttle_storage(_with_throttle_storage(_cluster_ready(deployment_multi_instance=True), LoginThrottleStorage.MEMORY)) + messages = _throttle_warnings(caplog) + assert messages and "deployment.multi_instance=true" in messages[0] + assert "max_login_attempts" in messages[0] + # The selector is a StrEnum; the warning must render its value, not "LoginThrottleStorage.MEMORY". + assert "auth.local.throttle_storage=memory:" in messages[0] + assert "LoginThrottleStorage" not in messages[0] + + +def test_login_throttle_warning_names_the_worker_variable(monkeypatch, caplog): + monkeypatch.setenv("WEB_CONCURRENCY", "2") + with caplog.at_level("WARNING"): + _validate_login_throttle_storage(_with_throttle_storage(_cluster_ready(), "memory")) + messages = _throttle_warnings(caplog) + assert messages and "WEB_CONCURRENCY=2" in messages[0] + + +def test_login_throttle_auto_resolves_to_the_database_under_multi_instance_without_warning(caplog): + with caplog.at_level("WARNING"): + _validate_login_throttle_storage(_with_throttle_storage(_cluster_ready(deployment_multi_instance=True), "auto")) + _validate_login_throttle_storage(_with_throttle_storage(_cluster_ready(deployment_multi_instance=True), "db")) + assert _throttle_warnings(caplog) == [] + + +def test_login_throttle_memory_is_silent_for_a_single_instance(caplog): + with caplog.at_level("WARNING"): + _validate_login_throttle_storage(_with_throttle_storage(_config_with_backend("sqlite"), "memory")) + assert _throttle_warnings(caplog) == [] + + +def test_login_throttle_gate_tolerates_a_config_without_an_auth_section(caplog): + with caplog.at_level("WARNING"): + _validate_login_throttle_storage(_cluster_ready(deployment_multi_instance=True)) + assert _throttle_warnings(caplog) == [] + + # DeerMem retrieval index: a declared multi-instance deployment must keep the # derived SQLite index off the shared memory volume. # --------------------------------------------------------------------------- diff --git a/config.example.yaml b/config.example.yaml index f143359dff..48ee1ad76e 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -20,7 +20,7 @@ # ============================================================================ # Bump this number when the config schema changes. # Run `make config-upgrade` to merge new fields into your local config.yaml. -config_version: 56 +config_version: 57 # Optional operator-owned literal prompt extensions (see CONFIGURATION.md): # lead_prompt_overlay: @@ -3380,19 +3380,33 @@ authorization: # auth: # local: # allow_registration: false -# # Login throttling per client IP (in-process, per worker). Raise -# # max_login_attempts when many users share one egress IP (corporate -# # proxy / NAT) so shared-IP lockouts don't block the whole office; -# # lower it for a stricter posture (minimum 2 — one failed attempt must -# # never lock an IP, or a single typo would block the shared egress). -# # Defaults preserve the historical hardcoded policy (5 failures / -# # 5 minutes). Live-read per login, so a config reload applies without -# # a Gateway restart: lowering lockout_seconds releases an active lock -# # early and tightening max_login_attempts keeps already-counted -# # failures; a raised lockout_seconds extends a still-active lock but -# # never resurrects one that already expired. +# # Login throttling per client IP. Raise max_login_attempts when many +# # users share one egress IP (corporate proxy / NAT) so shared-IP +# # lockouts don't block the whole office; lower it for a stricter +# # posture (minimum 2 — one failed attempt must never lock an IP, or a +# # single typo would block the shared egress). Defaults preserve the +# # historical hardcoded policy (5 failures / 5 minutes). Live-read per +# # login, so a config reload applies without a Gateway restart: lowering +# # lockout_seconds releases an active lock early and tightening +# # max_login_attempts keeps already-counted failures; a raised +# # lockout_seconds extends a still-active lock but never resurrects one +# # that already expired. # # max_login_attempts: 5 # # lockout_seconds: 300 +# # Where the failed-login counters and lockouts live (restart required). +# # auto -- (default) the application database whenever +# # database.backend is sqlite or postgres, so every Gateway +# # replica sharing that database enforces one lockout per IP; +# # with database.backend: memory it falls back to an +# # in-process counter. +# # memory -- in-process counter only. Per process: N replicas give an +# # attacker N x max_login_attempts guesses and a lockout on +# # one replica is invisible to the others (a multi-instance +# # deployment logs a startup WARNING). +# # db -- the shared login_throttle table; falls back to memory with +# # a WARNING when database.backend is memory, and refuses to +# # start when the configured database's engine is unavailable. +# # throttle_storage: auto # # oidc: # enabled: true diff --git a/deploy/helm/deer-flow/README.md b/deploy/helm/deer-flow/README.md index 689d477e12..8a47b26300 100644 --- a/deploy/helm/deer-flow/README.md +++ b/deploy/helm/deer-flow/README.md @@ -135,7 +135,7 @@ they resolve from the `secrets` map): ```yaml config: | - config_version: 56 + config_version: 57 models: - name: gpt-4 use: langchain_openai:ChatOpenAI diff --git a/deploy/helm/deer-flow/values.yaml b/deploy/helm/deer-flow/values.yaml index 5a30e7f63d..a02b822ef6 100644 --- a/deploy/helm/deer-flow/values.yaml +++ b/deploy/helm/deer-flow/values.yaml @@ -329,7 +329,7 @@ ingress: # -- DeerFlow config.yaml content. Secrets MUST stay as $VAR references — never # inline literal secret values here. The default enables provisioner sandbox. config: | - config_version: 56 + config_version: 57 log_level: info recursion_limit: 100 max_recursion_limit: 1000