fix(rate-limiting): enforce the per-user limit on chat sends, and record why the other two stay unwired (#16857)

[mrveiss]

Thinking Path

#16857: three instances of the shared autobot_shared.rate_limiter.RateLimiter were built (#4460) and never acquired, so the controls they imply never ran. Measured first: there is one shared implementation, so nothing needed consolidating; the gap is wiring only. ssot_config defines none of the rate_limit_* keys, so the tier defaults apply (authenticated: 60/min, 2000/h).

Whether each surface should be limited is a product decision (the issue's open question). Owner decisions, recorded on #16857:

Limiter	Outcome	Why
user_rate_limiter	Wired, per user, on the 7 chat message-send endpoints only	A route-wide cap would throttle the UI's own polling and reconnects
conversation_rate_limiter	Not wired: a security decision	Keyed by a conversation id the request supplies, it would be taken before the endpoint proves the caller owns that conversation. Any authenticated user could then spend another user's 60/min and lock the owner out with 429s: the limiter itself becomes a cross-user DoS. Ordering the ownership check first does not fix it, because that check has a log-only feature-flag mode in which it does not refuse. The per-user limit already bounds how fast any one caller sends to any conversation.
llm_rate_limiter	Not wired	Provider pacing is LLMCrossWorkerRateLimiter; two budgets over one limit would leave no authority. That limiter does not cover stream_completion in any of the 4 providers, and streaming is chat's default path. Recorded on #16538, the existing home for that block (one defect, one fix).

Each unwired instance now states why in its own module, which is the issue's stated resolution for "not wanted".

Hook point (owner decision): api/chat.py is pinned at its 2778-line ceiling and every send route lives there. The limit is a router-level dependency attached at mount time: route_dependencies.dependencies_for(name) is consulted by app_factory._register_routers. The registry 4-tuple shape is unchanged, and core_routers.py (590/600) is untouched. The dependency returns before resolving the caller unless the request is a POST to a named send endpoint, so no other chat route gains an auth requirement or a cap. It is imported eagerly, so a broken import fails startup instead of silently unmounting the chat router.

What Changed

• autobot-backend/api/chat_send_rate_limit.py (new): SEND_ENDPOINTS and enforce_chat_send_rate_limit. Past the limit it returns 429 with Retry-After. A caller without user_id (the internal-service identity) is not counted. A Redis outage fails open, per the shared limiter's policy.
• autobot-backend/initialization/router_registry/route_dependencies.py (new): a name-keyed mapping of router-level dependencies.
• autobot-backend/app_factory.py: passes dependencies=dependencies_for(name) to include_router for core routers.
• autobot-backend/utils/conversation_rate_limiter.py, autobot-backend/llm_shared/optimization/rate_limiter.py: comments recording why each instance is unwired.
• autobot-backend/api/chat_send_rate_limit_test.py (new): 7 tests. The shared limiter's real Lua window runs on fakeredis, and the assertions are HTTP status codes, not calls to acquire:
  • a user past the limit gets 429 with Retry-After;
  • one user's flood does not lock another user out of their conversation;
  • the body-sourced send endpoint is limited too;
  • a non-send route is never limited and never resolves the caller;
  • a caller without user_id is not counted;
  • SEND_ENDPOINTS is pinned against api/chat.py's POST routes, in two layers:
    • Source: every name is a router.post function in the file (AST).
    • Runtime (added after CodeRabbit review, 9532cbc39c): the real api.chat router is imported, and every name must be the __name__ of a registered POST route after its decorators. _is_send matches the runtime name, so this is the check that actually guards it. A decorator that stopped preserving __name__ would silently unlimit every send route while the source check stayed green.
    • Not done: driving 429s through all seven production handlers. That needs the whole chat stack mocked, and refusal is already proven through the same mounting path.

Verification

• Local, as a design aid (owner-sanctioned): the deployed /opt Python 3.14 interpreter, read-only, with the worktree on PYTHONPATH and test-only deps in a scratch dir (nothing written under /opt): 6 passed. Mutation: with the registry mapping emptied (limiter unwired), 2 failed, 4 passed; the two 429 tests catch an unwired limiter. /opt's packages sit below some declared floors, so CI is the evidence.
• Pre-push code-reviewer pass: CHANGES NEEDED. The HIGH finding (the cross-user lockout above) is resolved by not wiring the conversation limit. Router-wide auth widening and the lazy import that could silently unmount the chat router are both fixed.

Single-issue rationale: the sibling search (open issues on these files; rate limit, throttle, backoff) found none sharing this file set. The related stream_completion gap (#16538) is a provider-behaviour change with a different risk profile, so it is recorded there rather than batched here.

Closes #16857

Model Used

Claude Opus 5.5

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Chat messages are now subject to a per-user sending limit across conversations. The limit applies to direct-send routes as well as other chat-send routes, and each user’s activity is counted separately.
  • When a user reaches the limit, further sends are temporarily rejected and a retry time is provided. Other routes and requests without a user ID are not counted.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: bc1a58a1-d571-4e7b-b685-9a0a911f09fa

📥 Commits

Reviewing files that changed from the base of the PR and between 6ffa502 and 07199b1.

📒 Files selected for processing (3)

• autobot-backend/api/chat_send_rate_limit_test.py
• autobot-backend/app_factory.py
• autobot-backend/llm_shared/optimization/rate_limiter.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The change adds per-user rate limiting for selected POST chat endpoints. The chat router receives the limiter through router-level dependencies. Tests cover rate-limit responses, excluded routes, and caller identity handling.

Changes

Chat-send rate limiting

Layer / File(s)	Summary
Define and enforce the per-user chat-send limit autobot-backend/api/chat_send_rate_limit.py, autobot-backend/llm_shared/optimization/rate_limiter.py, autobot-backend/utils/conversation_rate_limiter.py	Adds POST endpoint matching and per-user limit enforcement. A rejected request receives HTTP 429 and a Retry-After header. Comments state that the shared LLM and conversation-scoped limiters are not acquired on this path.
Wire the dependency and test request behaviour autobot-backend/initialization/router_registry/route_dependencies.py, autobot-backend/app_factory.py, autobot-backend/api/chat_send_rate_limit_test.py	Maps the chat router to the limiter dependency and passes router dependencies during core router registration. Tests cover limits across conversations, direct-send requests, excluded routes, callers without user_id, and configured endpoint names.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 07199

The chat-send limit is wired to the production router, with no additional merge-blocking issue established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 07199

The change adds useful throttling without demonstrated expansion of access or cross-user budget consumption. It is not a universal abuse limit: authenticated identities without a user ID and requests during Redis failures remain unthrottled, and strict concurrent accounting is not fully established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — A caller's budget spans the selected send endpoints and conversations under the same authenticated user_id. The key contains no organization identifier, so that user's activity across organizations shares a budget. Request-supplied conversation IDs cannot directly consume another user's limiter budget.

Security Findings and Attack Paths

• inferred — A backend-accepted signed JWT lacking user_id skips this limiter even when it represents a non-service user. This leaves an authenticated abuse path outside the new control, but does not expand its previously unthrottled exposure or bypass authentication. The exact production reachability of the SLM-issued token example remains configuration-dependent.

Trust Boundaries and Controls

• observed — The new gate uses existing authentication rather than trusting a caller-supplied budget identifier. Authentication rejects invalid callers and restricts run-scoped and device-scoped tokens away from ordinary chat routes. The trusted internal API-key identity is deliberately exempt because it lacks user_id.

Resilience and Maintainability Implications

• observed — Redis unavailability or evaluation errors permit requests by existing shared policy. Such admissions are not backfilled on recovery. Successful acquisition accounts for admission rather than handler completion, with no compensating release for a failed or canceled handler.
• inferred — Atomic Lua execution prevents the ordinary concurrent check-then-record race, but sorted-set members are derived solely from tostring(now). Duplicate members would overwrite rather than count separate admissions. This is a pre-existing shared implementation limitation newly relied upon by chat; production collision frequency and strict concurrent enforcement are not established.

Hardening Proposals

• proposed — Normalize supported non-service identities to a stable, issuer-safe budget identity, and make service exemptions explicit rather than inferring them from missing user_id. Validate alternate supported token shapes through the production authentication path.
• proposed — Use a unique admission member while retaining the timestamp as its score, and validate same-timestamp concurrent admissions before treating this limiter as a strict abuse boundary.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 6 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarises the main change: enforcing the per-user limit on chat sends. It also identifies the documented treatment of the other rate limiters.
Linked Issues check	✅ Passed	The PR meets the coding requirements in #16857. It acquires user_rate_limiter on seven chat message-send endpoints and returns HTTP 429 with Retry-After when the limit is exceeded. The tests use t…
Out of Scope Changes check	✅ Passed	The changes remain within #16857. The router dependency registry and app_factory.py changes provide the required runtime wiring. The tests verify refusal behaviour and route boundaries. The comments…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 31a774f2-ea1d-4273-8d0b-2ea20cf83948

📥 Commits

Reviewing files that changed from the base of the PR and between 91a4160 and 6ffa502.

📒 Files selected for processing (6)

• autobot-backend/api/chat_send_rate_limit.py
• autobot-backend/api/chat_send_rate_limit_test.py
• autobot-backend/app_factory.py
• autobot-backend/initialization/router_registry/route_dependencies.py
• autobot-backend/llm_shared/optimization/rate_limiter.py
• autobot-backend/utils/conversation_rate_limiter.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.