fix(backend): keep Developer API conversation reads under one shared ceiling (#11682)

Follow-up to the Developer API read hardening from #8713 (landed in
af3ab1e).

## What this fixes

Splitting conversation reads into per-route policies gave each route its
own Redis bucket:

| | Before the split | On `main` today |
|---|---|---|
| `GET /v1/dev/user/conversations` | `dev:conversations_read` 60/hr |
`dev:conversations_read` 60/hr |
| `GET /v1/dev/user/conversations/{id}` | *same* 60/hr bucket |
`dev:conversation_detail_read` **separate** 60/hr |
| **Aggregate per key** | **60/hr** | **120/hr** |

Each policy is an independent counter
(`rl:{policy}:{prefix}:{uid}:{app_id}:{key_id}`), so adding a policy
adds budget rather than subdividing it.

## Honest scoping — this is small

I don't want to oversell it. The practical exposure delta is minor:

- Detail reads return **one** conversation. The list endpoint already
allows up to 100 per request (`backend/routers/developer.py:1241`), so
60 list req/hr ≈ 6,000 records/hr vs. the detail path's 60 records/hr —
roughly **1%** on top.
- The vector from the original incident (`GET
/v1/dev/user/conversations?limit=20` at ~1,300 req/hr) is the **list**
endpoint, and it is unchanged at 60/hr. That cap still does its job.

So this is not a live vulnerability, and "120 vs 60" counts requests,
which overstates it.

**The reason to fix it is structural:** nothing bounds the policy set.
Every future `dev:conversation_*_read` policy silently raises the
aggregate again, and no test or comment flags that. This caps it once,
in a way that keeps working as policies are added.

## Approach

Add `dev:conversation_reads_total` (60/hr). Every conversation read
charges it *before* its per-route budget:

- Aggregate returns to the pre-split 60/hr — no client that worked
before is affected.
- List / detail / transcript stay independently tunable underneath the
ceiling. This also answers the review question on #8743 about why detail
was given the same value as list: the per-route numbers are headroom,
the ceiling is the real limit.
- Transcript reads still charge their stricter 25/hr bucket on top.

**Tradeoff:** one extra Redis round trip on the two conversation read
routes. The transcript sub-budget already charges two buckets on a
single request, so the pattern isn't new — but it is a real cost on a
read path, and worth a maintainer's call.

## Tests

`test_conversation_reads_share_an_aggregate_ceiling` drives both routes
in alternation, so neither per-route bucket can be what stops the
caller, then asserts the aggregate is. It fails on the current wiring
with:

```
AssertionError: 120 != 60
```

Also added `dev:conversation_reads_total` to the existing policy-wiring
assertions, and updated the ordered policy list in
`test_dependency_async_boundaries.py` (the shared ceiling is charged
first, and still routes through `critical_executor`).

Verified per-file the way `backend/test.sh` runs in CI:
`test_rate_limiting.py`, `test_dependency_async_boundaries.py`,
`test_dev_api_conversations_poison.py`,
`test_dev_api_folder_filters.py`, `test_dev_api_lock_bypass.py` all
pass. `black --line-length 120 --skip-string-normalization` clean.

<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/BasedHardware/omi/pull/11682?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->




---

No existing class in `.github/failure-classes/` describes this mode —
subdividing a shared quota into per-route policies *adds* budget rather
than partitioning it. Declaring `none` rather than minting a class,
since reviewers asked for a declaration and not a registry change; happy
to switch to `new` and add the definition if maintainers would rather
have the class tracked.

Failure-Class: none

Author: RatnamOjha

backend/dependencies.py

@@ -339,12 +339,29 @@ def _require_conversations_read_scope(auth: ApiKeyAuth):
         )
 
 
+async def _check_conversation_read_budgets_async(
+    *,
+    request: Optional[Request],
+    auth: ApiKeyAuth,
+    route_policy_name: str,
+) -> None:
+    """Charge a conversation read against the shared ceiling, then its per-route budget.
+
+    The shared ceiling is checked first so sustained polling is rejected on the
+    aggregate budget regardless of which read route it targets. Without it, adding a
+    per-route policy would hand each key a fresh bucket and raise the total number of
+    conversation reads it can make -- the opposite of what these limits are for.
+    """
+    await _check_dev_api_key_rate_limit_async(request=request, auth=auth, policy_name="dev:conversation_reads_total")
+    await _check_dev_api_key_rate_limit_async(request=request, auth=auth, policy_name=route_policy_name)
+
+
 async def get_auth_with_conversations_read(
     auth: ApiKeyAuth = Depends(get_api_key_auth),
     request: Request = None,
 ) -> ApiKeyAuth:
     _require_conversations_read_scope(auth)
-    await _check_dev_api_key_rate_limit_async(request=request, auth=auth, policy_name="dev:conversations_read")
+    await _check_conversation_read_budgets_async(request=request, auth=auth, route_policy_name="dev:conversations_read")
     return auth
 
 
@@ -353,7 +370,9 @@ async def get_auth_with_conversation_detail_read(
     request: Request = None,
 ) -> ApiKeyAuth:
     _require_conversations_read_scope(auth)
-    await _check_dev_api_key_rate_limit_async(request=request, auth=auth, policy_name="dev:conversation_detail_read")
+    await _check_conversation_read_budgets_async(
+        request=request, auth=auth, route_policy_name="dev:conversation_detail_read"
+    )
     return auth
 
 

backend/tests/unit/test_dependency_async_boundaries.py

@@ -282,7 +282,10 @@ async def exercise() -> None:
         assert policies == [
             'mcp:memories_read',
             'mcp:memories_write',
+            # Each conversation read charges the shared ceiling before its per-route budget.
+            'dev:conversation_reads_total',
             'dev:conversations_read',
+            'dev:conversation_reads_total',
             'dev:conversation_detail_read',
             'dev:conversations',
             'dev:memories_read',

backend/tests/unit/test_rate_limiting.py

@@ -489,6 +489,7 @@ def test_all_router_policies_exist(self):
             "goals:advice",
             "goals:extract",
             "dev:conversations",
+            "dev:conversation_reads_total",
             "dev:conversations_read",
             "dev:conversation_detail_read",
             "dev:conversation_transcript_read",
@@ -561,6 +562,7 @@ def test_developer_dependencies_have_read_rate_limits(self):
         for policy in [
             "dev:memories_read",
             "dev:action_items_read",
+            "dev:conversation_reads_total",
             "dev:conversations_read",
             "dev:conversation_detail_read",
             "dev:conversation_transcript_read",
@@ -592,6 +594,62 @@ def test_developer_conversation_reads_emit_sanitized_audit_logs(self):
         self.assertNotIn("request.headers.get('Authorization'", dependencies_source)
         self.assertNotIn('request.headers.get("Authorization"', dependencies_source)
 
+    def test_conversation_reads_share_an_aggregate_ceiling(self):
+        """Per-route read policies must not raise the total reads a key can make.
+
+        Before list and detail were split into separate policies they shared one
+        60/hr bucket. Giving detail its own 60/hr policy without a shared ceiling
+        would let one key make 120 conversation reads an hour -- a loosening of the
+        exact limit #8713 asked to tighten. The shared ceiling is what prevents that,
+        so this drives both routes and asserts the aggregate, not the per-route, cap
+        is what stops the caller.
+        """
+        dependencies = importlib.import_module("dependencies")
+
+        auth = dependencies.ApiKeyAuth(
+            uid="uid1",
+            scopes=["conversations:read"],
+            app_id="test-app",
+            key_id="test-key",
+        )
+
+        counters: dict[str, int] = {}
+
+        def counting_limiter(*, prefix, uid, app_id, key_id, policy_name):
+            max_requests, _window = RATE_POLICIES[policy_name]
+            counters[policy_name] = counters.get(policy_name, 0) + 1
+            if counters[policy_name] > max_requests:
+                raise HTTPException(status_code=429, detail="Rate limit exceeded")
+
+        async def drive() -> int:
+            served = 0
+            # Alternate routes so neither per-route bucket can be what stops us.
+            for i in range(500):
+                dep = (
+                    dependencies.get_auth_with_conversations_read
+                    if i % 2 == 0
+                    else dependencies.get_auth_with_conversation_detail_read
+                )
+                try:
+                    await dep(auth)
+                except HTTPException as exc:
+                    self.assertEqual(exc.status_code, 429)
+                    break
+                served += 1
+            return served
+
+        with patch.object(dependencies, "check_api_key_rate_limit", counting_limiter):
+            served = asyncio.run(drive())
+
+        umbrella_max, _window = RATE_POLICIES["dev:conversation_reads_total"]
+        split_total = RATE_POLICIES["dev:conversations_read"][0] + RATE_POLICIES["dev:conversation_detail_read"][0]
+
+        self.assertEqual(served, umbrella_max)
+        self.assertLess(served, split_total, "per-route budgets must not sum into a higher effective ceiling")
+        # The shared ceiling, not a per-route budget, is what rejected the caller.
+        self.assertLessEqual(counters["dev:conversations_read"], RATE_POLICIES["dev:conversations_read"][0])
+        self.assertLessEqual(counters["dev:conversation_detail_read"], RATE_POLICIES["dev:conversation_detail_read"][0])
+
     def test_developer_rate_limit_failures_log_without_request(self):
         dependencies = importlib.import_module("dependencies")
         auth = dependencies.ApiKeyAuth(

backend/utils/rate_limit_config.py

@@ -118,6 +118,12 @@
     # MCP API-key contexts are keyed by app/key identity when available.
     "dev:memories_read": (120, 3600),
     "dev:action_items_read": (120, 3600),
+    # Conversation reads are limited in two tiers. Every conversation read consumes
+    # the shared "reads_total" ceiling plus its per-route budget, so splitting list
+    # and detail into separately tunable policies cannot raise the aggregate number
+    # of conversation reads one key can make. Transcript reads consume a third,
+    # stricter bucket on top of the other two.
+    "dev:conversation_reads_total": (60, 3600),
     "dev:conversations_read": (60, 3600),
     "dev:conversation_detail_read": (60, 3600),
     "dev:conversation_transcript_read": (25, 3600),