From b99ac916b63c3c9c4a454f65ae892be1f20a6230 Mon Sep 17 00:00:00 2001 From: Nan Gao Date: Thu, 8 Oct 2026 22:35:19 +0800 Subject: [PATCH] fix(auth): count login lockouts in the database across Gateway replicas (#6501) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(auth): count login lockouts in the database across Gateway replicas POST /api/v1/auth/login/local throttled failed logins per client IP in a module-level dict, so with N Gateway replicas behind one load balancer an attacker got N x auth.local.max_login_attempts guesses and a lockout on one replica was invisible to the others. Move the counter behind a LoginThrottleStore (deerflow.persistence. login_throttle): MemoryLoginThrottleStore keeps the historical per-process semantics and 10k tracked-IP bound; SqlLoginThrottleStore shares one login_throttle row per IP through the application database, incrementing and starting the lock in a single dialect-native upsert, applying check decisions with compare-and-set predicates in a separate write transaction, and sweeping served locks plus 24h-idle counters in bounded batches. Both stores pin the existing contract: the lock starts when the count reaches max_login_attempts, the committed duration is honored when the policy shrinks mid-lock, a served sentence is never resurrected, tightening keeps the count, success resets the IP, and the 429 status and message are unchanged. New startup-only selector auth.local.throttle_storage (auto | memory | db, default auto = the database whenever database.backend is sqlite or postgres). The Gateway installs the store once at startup after the persistence engine; deps._validate_login_throttle_storage warns when a multi-process deployment keeps memory. Migration 0034_login_throttle creates the table; config_version 55 -> 56 in config.example.yaml and the Helm chart. Docs: README, CONFIGURATION.md, AUTH_DESIGN.md, config and migrations AGENTS.md, CHANGELOG. * docs(changelog): link the login throttle entry to #6501 * fix(auth): re-check expiry on the row the login-throttle sweep deletes The bounded cleanup in SqlLoginThrottleStore.record_failure deleted `WHERE ip IN (SELECT ip ... WHERE LIMIT 200)`. On PostgreSQL READ COMMITTED the expiry predicate applied only inside the snapshot-bound subquery: when the DELETE had to wait on a row lock held by a concurrent check() that was extending the same lock under a raised lockout_seconds, PostgreSQL re-evaluated only the outer WHERE on the updated tuple, IP membership still held for the old candidate, and the now-active lock was deleted; the next attempt from that IP got a clean record and reached password verification (review of #6501, P2). Repeat the served/stale predicate on the DELETE target as well (SqlLoginThrottleStore.sweep_statement), keeping the bounded candidate subquery, so the re-evaluation sees the updated row's expiry. The other writes already carry their full compare-and-set predicate on the target row: _discard and the check() UPDATE both use _matches (ip, fail_count, locked_at, lock_duration_seconds), so a tuple changed while they waited no longer matches and the stale decision is skipped. _discard keeps the equality compare-and-set rather than adding an expiry condition: its early-release branch deletes a lock whose committed sentence has not elapsed under the lowered live policy, which an expiry predicate would wrongly block, and the equality predicate already pins every input of the decision. reset stays unconditional by design. Tests: a dialect-compile test asserts the DELETE's own WHERE carries the expiry predicate for postgresql and sqlite (red on the previous HEAD), and a PostgreSQL-only test (skipped without TEST_POSTGRES_URI) reproduces the update-versus-sweep interleaving with two engines, waiting on pg_stat_activity for the sweep to block before the extender commits. Docs: sql.py concurrency contract and the migrations guide's 0034 bullet. * fix(auth): keep an active login lock's clock and fail closed on explicit db Address the "changes requested" review on #6501. Lock clock (confirmed defect): record_failure re-stamped locked_at = now for every failure on an already-locked row, so a sustained attacker's sentence slid forward with each attempt. Both stores now apply one rule in one atomic decision: new_count = fail_count + 1; below max_attempts the IP counts with no lock (a failure after the operator raised the threshold clears an existing lock); at or over it, a failure during an *active* lock keeps locked_at and the committed lock_duration unchanged, while a never-locked or served row starts anew with now / lockout_seconds (so the failure that reaches a raised threshold starts a fresh lock rather than keeping a stale locked_at). The SQL upsert expresses it as nested case() on the row's own values; the memory store mirrors it. The sweeps in both stores now leave the row being recorded to that decision instead of deleting a served lock or stale counter first, which split the contract (SQL would have produced a fresh (1, NULL, NULL) row). base.py states the rule. Explicit db downgrade: resolve_login_throttle_store raises SystemExit — the failure type of the other startup validations in app.gateway.deps — when throttle_storage='db' resolves to the shared table but no engine is initialised, so langgraph_runtime refuses to start instead of silently running every replica on per-process counters; the lazy bare-app path applies the same rule and installs nothing. 'auto' keeps warning and falling back. 'db' on a memory database still degrades with a warning (there is no shared table to insist on). Non-blocking items: _key() logs a warning when a client key is truncated to 255 characters (never the key itself); the sql.py docstring covers wall-clock locked_at across replicas (run NTP; skew shifts evaluation by that much) and why STALE_COUNTER_SECONDS=24h / SWEEP_BATCH_SIZE=200 were chosen; a store-level blocking-I/O anchor covers the SQL reset path; the _validate_login_throttle_storage warning was verified to render 'throttle_storage=memory' for the StrEnum (the gate test now passes the enum member and asserts that text). reset stays unconditional: a peer failure that commits after a reset leaves fail_count=1, one genuine failure after the cleared history, which a new test documents. Docs: CHANGELOG entry, config.example.yaml, CONFIGURATION.md, AUTH_DESIGN.md and the throttle_storage field description. * fix(auth): re-evaluate a login-throttle check whose compare-and-set missed SqlLoginThrottleStore.check applied its decision (commit the live duration, or clear a served / released lock) with a compare-and-set predicate on the snapshot it had read, but ignored the affected-row count. When a peer changed the row between the read and the write — an admitted wrong password incrementing fail_count on another replica, or a successful login clearing the row — the UPDATE or DELETE matched zero rows, yet check reported the decision as applied: it could return an extended 60s sentence while the row kept its 1s duration, so the next check cleared the lock and let password verification through; a live duration decrease could be dropped the same way; and a row a peer had just cleared was still reported as locked (#6501 review, P2). Every compare-and-set write in check now checks result.rowcount. On a miss the row is re-read and re-evaluated from the top, up to CHECK_CAS_ATTEMPTS (3) times, applying a fresh compare-and-set decision on the new snapshot and never clobbering a reset or replacement: a row that is gone is reported as allowed, a fresh peer lock is reported as locked, and an incremented count gets the extension or decrease committed on it. Exhausting the bound fails closed: nothing is written and the answer is the last snapshot's lock under the live policy, logged at debug. The read stays in its own short transaction and each write in a separate one. The evaluation is now one shared pure function, evaluate_check in base.py (CheckDecision), used by both stores so the contract cannot diverge. The memory store keeps a single pass: its compare-and-set cannot miss because there is no await between the read and the write on the loop thread, which its docstring now states. Tests (red first on df26c2acf) use a session proxy that runs the peer mutation on a second engine right after the snapshot read ends its transaction: the reviewer's raised-duration interleaving (59.5s reported and committed, the increment survives, a later check still locked), the mirrored decrease, early release on the fresh row, a reset landing between read and write (0.0, no row recreated), and the bound-exhausted fail-closed case. * docs(changelog): resolve merge markers left in the Unreleased section * test(migrations): pin 0035_login_throttle as the head in the 0034 watermark test Upstream #6477 added test_migration_0034_run_event_seq_watermark.py asserting 0034 is the single head; this branch chains 0035_login_throttle after it, so the test now pins the current head and walks the chain like its siblings. * perf(auth): one session per login-throttle check, one transaction per failure Two non-blocking performance suggestions from the #6501 review. Single session per check. The router probed the row with store.get() and then called store.check(), which re-read it: a recorded IP cost two connection-pool checkouts and two SELECTs per request on the unauthenticated login path, letting a lockout flood compete with legitimate traffic for Postgres connections. LoginThrottleStore.check now takes the policy as an async callable (LoginThrottlePolicy) and resolves it lazily inside the store: the probe, the policy resolution and the compare-and-set loop share one session, a clean IP returns 0.0 without ever calling the policy (the config.yaml read is still skipped for the common case), and a recorded IP resolves it exactly once, also across compare-and-set retries. The decision is made on a snapshot read after the resolution: 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 — so one session, but still a read after the yield. The memory store takes the same signature and re-reads its record after the await for the same reason. The router passes its asyncio.to_thread policy read as the callable and drops the separate probe; get stays on the protocol for tests and anchors. No short-TTL verdict cache: it would mask a successful login's reset for the TTL window, and the single session already halves the hot path. Sweep in the upsert's transaction, plus an index. record_failure ran the housekeeping DELETE in its own transaction and the upsert in a second one, two commits (two WAL flushes on PostgreSQL) per failed login; the sweep now rides in the upsert's transaction with one commit, the keep exclusion already protecting the row being recorded. The new ix_login_throttle_updated_at (ORM __table_args__ and the unreleased 0035 migration, created after the table and dropped before it, each guarded separately so a table left by an interrupted attempt still gets its index) serves the sweep's stale-counter predicate; the served-lock predicate scans a table the sweep itself keeps small. Tests (red first on a137fbc47): a clean IP never resolves the policy and a recorded IP resolves it exactly once (both stores), the SQL store opens exactly one session per check and commits once per record_failure (counting wrappers), the migration test asserts the index after upgrade, after completing a partial upgrade that lacked it, and gone after downgrade while compare_metadata stays empty; the router-level "mutated during policy resolution" tests now inject their mutation through the policy callable. * docs(changelog): note config_version 57 in the login throttle entry --- CHANGELOG.md | 24 + README.md | 2 +- backend/app/gateway/auth/login_throttle.py | 121 +++ backend/app/gateway/deps.py | 44 + backend/app/gateway/routers/auth.py | 167 ++-- backend/docs/AUTH_DESIGN.md | 4 +- backend/docs/CONFIGURATION.md | 2 + .../harness/deerflow/config/AGENTS.md | 2 +- .../harness/deerflow/config/auth_config.py | 50 +- .../deerflow/config/reload_boundary.py | 5 + .../persistence/login_throttle/__init__.py | 21 + .../persistence/login_throttle/base.py | 148 ++++ .../persistence/login_throttle/memory.py | 116 +++ .../persistence/login_throttle/model.py | 39 + .../persistence/login_throttle/sql.py | 286 +++++++ .../deerflow/persistence/migrations/AGENTS.md | 5 +- .../versions/0035_login_throttle.py | 68 ++ .../deerflow/persistence/models/__init__.py | 2 + .../blocking_io/test_login_local_throttle.py | 93 +- backend/tests/test_auth.py | 673 ++++++++------- backend/tests/test_login_throttle_config.py | 213 +++++ backend/tests/test_login_throttle_store.py | 807 ++++++++++++++++++ ...st_migration_0025_repair_run_change_seq.py | 2 +- ...migration_0027_0028_notification_outbox.py | 2 +- ...st_migration_0029_scheduler_agent_tasks.py | 2 +- ...migration_0032_activity_and_task_events.py | 6 +- ..._migration_0034_run_event_seq_watermark.py | 6 +- .../test_migration_0035_login_throttle.py | 182 ++++ .../tests/test_multi_worker_postgres_gate.py | 56 +- config.example.yaml | 38 +- deploy/helm/deer-flow/README.md | 2 +- deploy/helm/deer-flow/values.yaml | 2 +- 32 files changed, 2724 insertions(+), 466 deletions(-) create mode 100644 backend/app/gateway/auth/login_throttle.py create mode 100644 backend/packages/harness/deerflow/persistence/login_throttle/__init__.py create mode 100644 backend/packages/harness/deerflow/persistence/login_throttle/base.py create mode 100644 backend/packages/harness/deerflow/persistence/login_throttle/memory.py create mode 100644 backend/packages/harness/deerflow/persistence/login_throttle/model.py create mode 100644 backend/packages/harness/deerflow/persistence/login_throttle/sql.py create mode 100644 backend/packages/harness/deerflow/persistence/migrations/versions/0035_login_throttle.py create mode 100644 backend/tests/test_login_throttle_config.py create mode 100644 backend/tests/test_login_throttle_store.py create mode 100644 backend/tests/test_migration_0035_login_throttle.py 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