Skip to content

feat: rate limit - #74

Draft
mmagician wants to merge 3 commits into
mainfrom
mmagician-claude/rate-limit-review-fixes
Draft

feat: rate limit#74
mmagician wants to merge 3 commits into
mainfrom
mmagician-claude/rate-limit-review-fixes

Conversation

@mmagician

Copy link
Copy Markdown
Collaborator

Draft PR against ev/fmt (PR #60) with the review fixes from #60 (comment) (review I posted locally).

Summary

  • Canonical gRPC status: trailers-only HTTP 200 with grpc-status: 8 (RESOURCE_EXHAUSTED) instead of HTTP 429 with grpc-status: 14 (UNAVAILABLE). Clients apply very different retry policies to these two codes.
  • X-Forwarded-For / X-Real-IP trust is now opt-in via trust_forwarded_headers (default false) + --rate-limit-trust-forwarded-headers CLI flag. Trusting these unconditionally let any direct client bypass the limiter by spoofing the header. Also removed the LOCALHOST fallback that coalesced unidentifiable traffic into a shared bucket.
  • IPv6 addresses are bucketed by /64 prefix before being keyed, so a single routable /64 cannot rotate through 2^64 addresses to dodge the limiter.
  • governor's dashmap state store is now periodically cleaned by a background tokio task calling retain_recent() every 60s, preventing unbounded memory growth.
  • requests_per_second / burst_size are now NonZeroU32 in the config and on the CLI, so --rate-limit-rps 0 fails at parse time instead of being silently clamped to 1.
  • governor and http are now referenced via [workspace.dependencies] to match the rest of the crate.
  • Unit tests for extract_ip precedence + trust-flag gating, IPv6 /64 bucketing, and a smoke test that over-quota requests hit RESOURCE_EXHAUSTED.

Test plan

  • cargo test --workspace --all-targets (12 tests pass, 7 new)
  • make lint (fmt + fix + clippy + toml + workspace-lints)
  • Manual test against a running node with a low --rate-limit-rps + grpcurl to confirm the RESOURCE_EXHAUSTED response shape
  • Verify X-Forwarded-For flag behavior behind nginx

Worktree: /home/marti/dev/note-transport-service/.claude/worktrees/rate-limit-review-fixes.
Branch: mmagician-claude/rate-limit-review-fixes, based off the ev/fmt head (93ffd74).

v0-e and others added 3 commits December 31, 2025 14:47
- Move governor/http to workspace [dependencies], matching convention.
- Return a trailers-only gRPC response with grpc-status 8 (RESOURCE_EXHAUSTED),
  which is the canonical mapping for rate-limit rejection. Previously we
  returned HTTP 429 with grpc-status 14 (UNAVAILABLE), which clients treat
  differently for retry/backoff purposes.
- Gate X-Forwarded-For / X-Real-IP trust behind a new
  `trust_forwarded_headers` flag (default false). Trusting these headers
  unconditionally lets any direct client bypass the limiter by spoofing the
  header. The flag also removes the localhost fallback, which previously
  coalesced all unidentifiable traffic into one shared bucket.
- Bucket IPv6 addresses by /64 prefix so a single routable prefix cannot
  rotate through 2^64 addresses to dodge the limiter.
- Spawn a tokio task that periodically calls retain_recent() on the
  governor state store, preventing unbounded dashmap growth.
- Use NonZeroU32 in RateLimitConfig and as the clap value type, so
  --rate-limit-rps 0 fails at parse time instead of silently becoming 1.
- Minor: inline the opt_nonzero helper in main, log "disabled" rather than
  "0s" for the disabled tcp-keepalive case.
- Add unit tests for extract_ip precedence, trust flag honored, IPv6 /64
  bucketing, and an end-to-end check that over-quota requests are rejected.
- Instead of letting unidentified traffic through unlimited, key it to a
  single shared bucket (UNKNOWN_IP_BUCKET = 0.0.0.0) so it is still
  rate-limited. In practice the branch should not fire because tonic
  always attaches TcpConnectInfo — the fallback is defensive.
- Bump retain_recent() interval from 60s to 300s. Entries are small and
  eviction eligibility depends on bucket replenishment, not wall-clock,
  so a 5-minute sweep is plenty.
@mmagician
mmagician changed the base branch from ev/fmt to main April 17, 2026 16:29
@mmagician mmagician changed the title review fixes: rate-limit (vs #60) feat: rate limit Apr 17, 2026
@mmagician mmagician linked an issue Apr 17, 2026 that may be closed by this pull request
@mmagician mmagician mentioned this pull request Apr 17, 2026
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.

Spam protection mechanism

3 participants