Skip to content

fix: wire ADR-0012 per-user rate limit on /ask (#261)#290

Open
ckrough wants to merge 1 commit into
mainfrom
issue/261
Open

fix: wire ADR-0012 per-user rate limit on /ask (#261)#290
ckrough wants to merge 1 commit into
mainfrom
issue/261

Conversation

@ckrough

@ckrough ckrough commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

POST /api/v1/ask in the retriever service drove chat + embedding LLM-gateway calls with no per-user throttle: a single valid subscription token could drive unbounded paid inference (denial-of-wallet, security finding L1 from the LLM abuse-surface assessment #228). This wires the documented-but-unimplemented ADR-0012 rate limit (slowapi) ahead of the gateway call.

  • Per-user keying on AuthUser.sub, stashed onto request.state by a dependency chained off require_auth (one JWT decode, no divergent validation). Per-tenant limiting is a documented forward extension: the token carries no tenant claim today, so a per-tenant key would collapse to one global bucket.
  • Default 10/minute, enabled by default, disableable via RATE_LIMIT_ENABLED for ops/tests. Fail-closed: a malformed RATE_LIMIT_ASK crashes app startup (parse_many at wire time), and swallow_errors=False surfaces limiter errors rather than silently exempting.
  • 429 response carries the ADR-0012 JSON body and a Retry-After header, and exits cleanly through CORS.
  • Scope confined to services/retriever/.

Closes #261

Pipeline phases

  • 🟢 Plan / design triage — complexity score 3; one designer pass, blueprint adopted.
  • 🟢 Implement — implementer ran, tests written first (red-green TDD).
  • 🟢 Test-validation — red-green replay confirmed guard-detecting tests fail against pre-change code; 0 blocking, 1 advisory.
  • 🟢 Review-gate — APPROVE (1 cycle); all 7 acceptance criteria verified.
  • 🟡 Simplify — applied 1 change (tightened test_create_app_wires_limiter_and_handler to assert the real create_app(), dropping a masking try/except); delta re-verified via the full gate run.
  • 🟢 Gates — configured test + lint both exit 0.
  • 🟡 Security review — no blocking findings; 3 advisories recorded as accepted risk (per-instance N× is ADR-0012's accepted design + Redis follow-up; 2 LOW unreachable / fail-closed).
  • 🟢 PR opened — non-draft, referencing the issue.

Test results

Both configured gate commands from .daedalus/config.json were run from the worktree and exited 0.

  • lint(cd services/retriever && uv sync && ruff check + ruff format --check + mypy --strict) && (cd services/petdata && ... ruff + mypy + bandit) && (cd apps/stacker && npm ci && npm run check)exit 0. retriever: ruff clean, format clean, mypy Success: no issues found in 68 source files. petdata: clean. stacker svelte-check: 0 ERRORS 0 WARNINGS.
  • test(cd services/retriever && uv sync && pytest tests/ --ignore=tests/integration) && (cd apps/stacker && npm ci && npm run build)exit 0. retriever: 382 passed, 10 deselected, total coverage 89.45% (new rate_limit.py at 100%). stacker: ✓ built.

(petdata pytest is CI-delegated: it needs Postgres+pgvector on :5434, per .daedalus/config.json.)

Eval scores

No evals configured for this repo.

The retriever /api/v1/ask endpoint drove chat + embedding LLM-gateway
calls with no per-user throttle: a single valid subscription token could
drive unbounded paid inference (denial-of-wallet, security finding L1).

Wire the documented-but-unimplemented ADR-0012 rate limit (slowapi) ahead
of the gateway call, keyed on the authenticated user (AuthUser.sub) via a
request.state stash set by a dependency chained off require_auth (one JWT
decode). Default 10/minute, enabled by default, disableable via config.
A 429 returns the ADR-0012 body plus a Retry-After header and exits cleanly
through CORS. Per-tenant limiting is a documented forward extension (the
token carries no tenant claim today).

Refs #261
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add rate/quota limit to the retriever /ask inference path

1 participant