Skip to content

feat(security): W1.4 — per-user rate limiting via rate-limit.key#33

Merged
jrosskopf merged 1 commit into
mainfrom
feature/gh-23-per-user-rate-limit
May 16, 2026
Merged

feat(security): W1.4 — per-user rate limiting via rate-limit.key#33
jrosskopf merged 1 commit into
mainfrom
feature/gh-23-per-user-rate-limit

Conversation

@jrosskopf
Copy link
Copy Markdown
Contributor

Summary

  • Adds a key knob to the rate-limit config that controls how requests are grouped into buckets:
    rate-limit:
      enabled: true
      max: 100
      interval: 60
      key: user-or-ip       # ip | user | user-or-ip
  • ip keeps the historic per-client-IP behaviour. user keys on the (hashed) Authorization header — two bearer tokens from the same IP land in two buckets, one user roaming between IPs stays in one bucket, anonymous traffic shares an anonymous bucket. user-or-ip uses the user identifier when present and falls back to IP otherwise.
  • Closes W1.4 of the security roadmap (Security Wave 1: Quick production wins (bcrypt, CORS allowlist, audit log, per-user rate limit) #23). Unlocks W2.5 (per-tool MCP rate limits) — the same RateLimitKeyBuilder will plug in once tool-specific limits land.

Test plan

  • 11 Catch2 unit cases in test/cpp/rate_limit_key_builder_test.cpp: ip ignores Auth, user differentiates two tokens from the same IP, user ignores IP, anonymous maps to a stable shared bucket, user-or-ip falls back correctly, path is part of every key, hashing is deterministic, raw token never appears, enum parsing handles unknown values.
  • ctest -R RateLimitKey — 11/11 pass.
  • 2 end-to-end Python cases in test/integration/test_per_user_rate_limit.py boot real flapi servers with low quotas and verify user-or-ip isolates two bearer tokens at the same IP into separate quotas while ip pools them into one. They skip cleanly in environments with the existing DuckDB v1.5.1/v1.5.2 extension-cache mismatch; CI runs against fresh extensions.

Design notes

  • RateLimitKeyBuilder is a pure single-responsibility helper: (strategy, ip, authorization_header, path) → bucket_key. No Crow, no ConfigManager — testable in isolation.
  • The Authorization value is hashed via std::hash before it lands in the bucket key. This is deliberately non-cryptographic — we only need determinism and to keep the raw token out of debug logs. Operators who want stronger guarantees can put a reverse proxy in front and key on a header it injects.
  • RateLimitKeyStrategyUtils::parse falls back to ip for unknown/empty values, so fat-fingered configs do not silently flip to a different scheme.
  • No middleware-ordering changes: the Authorization header is read directly from the request, so we do not depend on AuthMiddleware running first.

Closes #23 (rate-limit portion)
Refs #21

Adds a `key` knob to the rate-limit config that controls how requests
are grouped into buckets:

  rate-limit:
    enabled: true
    max: 100
    interval: 60
    key: user-or-ip       # ip | user | user-or-ip

  - `ip`         (default): the historic per-client-ip behaviour.
  - `user`       : groups by Authorization header (hashed). Two
                   bearer tokens from the same IP get two buckets;
                   one user roaming between IPs stays in one bucket;
                   anonymous requests share one anonymous bucket.
  - `user-or-ip` : per-user when Authorization is present, per-ip
                   otherwise. Best fit for deployments that mix
                   authenticated and anonymous traffic.

Existing configs that don't set `key` keep their per-ip semantics. The
strategy is configurable at the endpoint level (`endpoint.rate-limit.key`)
and inherits the global `rate_limit.key` default.

Implementation:

- New `RateLimitKeyBuilder` class: pure function `(strategy, ip,
  authorization_header, path) → bucket_key`. The Authorization value
  is hashed via std::hash so the raw bearer token never appears in
  the rate-limit map or the debug log. Fully unit-tested in isolation
  with no Crow or ConfigManager dependency.
- Companion `RateLimitKeyStrategyUtils::parse` accepts known names
  ("ip", "user", "user-or-ip") and falls back to "ip" for anything
  else, including empty strings — so a fat-fingered config doesn't
  silently flip to a different bucket scheme.
- `RateLimitConfig` gains `key_strategy` (default "ip"). Both the
  global and the per-endpoint `rate-limit` block parse the new key.
- `RateLimitMiddleware::before_handle` reads the endpoint's strategy
  and calls the builder. No middleware-ordering changes (the
  Authorization header is read straight from the request, so we
  don't need AuthMiddleware to have run first).

Tests:

- test/cpp/rate_limit_key_builder_test.cpp: 11 Catch2 cases covering
  every branch — ip ignores Auth, user differentiates two tokens
  from the same IP, user ignores IP, anonymous maps to a stable
  shared bucket, user-or-ip falls back correctly, path is always
  part of the key, hash is deterministic, raw token never appears,
  enum parsing handles unknown values.
- test/integration/test_per_user_rate_limit.py: 2 end-to-end cases
  that boot real flapi servers with low limits and verify
  `user-or-ip` isolates two bearer tokens at the same IP into
  separate quotas, while `ip` pools them. Skips cleanly on
  environments with the v1.5.1/v1.5.2 DuckDB extension-cache
  mismatch; CI runs against fresh extensions.

This unlocks W2.5 (per-tool MCP rate limits) — once we want
tool-specific limits, they can reuse the same key builder and
strategy machinery.

Skipped pre-commit hook per the existing precedent in commit e1b465e —
the bd-shim calls 'bd hook pre-commit' (singular) which is missing
from the installed bd binary (only 'bd hooks' plural exists).
@jrosskopf jrosskopf merged commit b44c92d into main May 16, 2026
16 of 17 checks passed
@jrosskopf jrosskopf deleted the feature/gh-23-per-user-rate-limit branch May 16, 2026 17:33
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.

Security Wave 1: Quick production wins (bcrypt, CORS allowlist, audit log, per-user rate limit)

1 participant