Fix(escalation) close session id rotation bypass of alert rate limiting

[711nishtha]

What

Adds a global, non-session-keyed rate-limit backstop for operator alerts
(Slack/Discord/Teams/PagerDuty/email), alongside the existing per-session
limiter.

Why

check_rate_limit() is keyed entirely on session_id, which is
caller-supplied and unauthenticated — middleware/interceptor.py reads it
straight off the request body with no validation. That means the
per-session quota (escalation.rate_limit_max, default 3/hour) resets for
every new session_id, so an attacker can rotate the ID on every request
and trigger unlimited operator pages/notifications, even though no single
session ever exceeds its own quota.

For a tool whose entire purpose is alerting a human when someone's in
crisis, unlimited false pages is a real availability/trust problem — it
buries real alerts under noise (alert fatigue on the on-call channel).

How

• New _global_rate_limit_allows() in escalation/router.py: in-process
  sliding-window counter (deque of timestamps + lock), independent of
  session_id.
• escalate() now requires both the existing per-session check AND the
  new global check before firing alerts. Short-circuits so a
  session-limited event doesn't also burn a global slot.
• New config keys under escalation: — global_rate_limit_max (default
  100) and global_rate_limit_window_seconds (default 60). Set
  global_rate_limit_max: 0 to disable.
• New reason: "logged_alerts_globally_rate_limited" in the result dict
  so it's distinguishable from the existing per-session
  "logged_alerts_rate_limited" in logs/tests.
• Audit logging is untouched — every event is still persisted regardless
  of either rate limit, per the existing design intent (only alerting
  is throttled).

Known limitation

This backstop is in-process, so it's per-worker in a multi-process
deployment (gunicorn/uvicorn with multiple workers), not a hard global
cap across the whole fleet. It still closes the exploit for the default
single-process deployment. A Redis-backed version would be needed for a
true cross-process ceiling — happy to open that as a follow-up if wanted.

Testing

Added TestGlobalRateLimitBackstop in tests/test_router.py:

• rotating session_id no longer yields unlimited alerts
• global_rate_limit_max: 0 disables the backstop
• window expiry allows further alerts
• a session already blocked by its own quota doesn't consume a global slot

pytest -q → 421 passed, 16 skipped, 0 failed. ruff check clean.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

Warning

Review limit reached

@711nishtha, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 58 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fb67e047-632c-41ca-a66d-c8b2e51c1512

📥 Commits

Reviewing files that changed from the base of the PR and between 37d0f65 and 2e97429.

📒 Files selected for processing (4)

• humane_proxy/config.yaml
• humane_proxy/escalation/router.py
• humane_proxy/middleware/interceptor.py
• tests/test_router.py

📝 Walkthrough

Walkthrough

Adds a global, cross-session alert rate-limit backstop to the escalation router, configured via new config.yaml fields, enforced alongside the existing per-session limiter with refined suppression reasons/logging, and validated by a new test suite covering bypass prevention and window expiration.

Changes

Global Alert Rate Limit Backstop

Layer / File(s)	Summary
Global rate limit configuration humane_proxy/config.yaml	Adds global_rate_limit_max and global_rate_limit_window_seconds fields under escalation, with comments explaining bypass prevention via rotating session IDs and disabling via 0.
Global limiter implementation and integration humane_proxy/escalation/router.py	Adds a thread-safe sliding-window global timestamp store and helper functions, integrates the global check into escalate() alongside the per-session check, and distinguishes per-session vs global suppression in returned reasons and warning logs.
Global rate limit test suite tests/test_router.py	Adds TestGlobalRateLimitBackstop verifying bypass prevention across rotating sessions, disabling via 0, window expiry, and quota isolation between per-session and global limits.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant EscalationRouter
  participant PerSessionLimiter
  participant GlobalLimiter

  Caller->>EscalationRouter: escalate(session_id, event)
  EscalationRouter->>PerSessionLimiter: check_rate_limit(session_id)
  PerSessionLimiter-->>EscalationRouter: allowed/denied
  alt session allowed
    EscalationRouter->>GlobalLimiter: _global_rate_limit_allows()
    GlobalLimiter-->>EscalationRouter: allowed/denied
  end
  EscalationRouter-->>Caller: alerts_allowed + reason (per-session/GLOBAL/none)

Loading

Poem

A hop, a check, a quota tight,
No sneaky session slips past sight!
One global gate for all to share,
Rotating IDs won't fool the hare 🐇
Alerts now capped, both far and near—
Carrots safe another year! 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is clearly about closing the session-id rotation bypass in escalation alert rate limiting.
Description check	✅ Passed	The description matches the changeset by explaining the new global alert backstop, config keys, rationale, and tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/test_router.py (1)

137-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test couples global_max to live session_cap — fragile if rate_limit_max changes.

The test hardcodes global_max=5 but reads session_cap from the live store singleton. The first session_cap calls consume both per-session and global slots, so the test only passes when session_cap < 5. If rate_limit_max in config.yaml is changed to ≥ 5, those initial calls exhaust the global limiter and the final fresh["alerted"] is True assertion fails.

Setting global_max relative to session_cap eliminates the coupling.

♻️ Proposed fix

-        with patch.object(router_mod, "get_config", return_value=self._cfg(global_max=5)):
+        with patch.object(router_mod, "get_config", return_value=self._cfg(global_max=session_cap + 5)):

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_router.py` around lines 137 - 159, The test in
test_session_limited_event_does_not_consume_global_slot is fragile because it
hardcodes get_config(... global_max=5) while using the live session_cap from
get_store()._rate_limit_max, so the first loop can exhaust the global limiter
when session_cap changes. Update the test setup to derive global_max from
session_cap (or otherwise keep it safely above the per-session cap) before
calling escalate, so the fresh-session assertion remains valid regardless of
config changes.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/test_router.py`:
- Around line 137-159: The test in
test_session_limited_event_does_not_consume_global_slot is fragile because it
hardcodes get_config(... global_max=5) while using the live session_cap from
get_store()._rate_limit_max, so the first loop can exhaust the global limiter
when session_cap changes. Update the test setup to derive global_max from
session_cap (or otherwise keep it safely above the per-session cap) before
calling escalate, so the fresh-session assertion remains valid regardless of
config changes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3ac3ff5d-4620-4e13-8b86-c181b1c77dc2

📥 Commits

Reviewing files that changed from the base of the PR and between d394ebe and 37d0f65.

📒 Files selected for processing (3)

• humane_proxy/config.yaml
• humane_proxy/escalation/router.py
• tests/test_router.py

[Vishisht16]

Good job with the work and thanks for the contribution