Skip to content

feat(middleware): mode-aware in-memory rate limiter (#438) [stacked on #477] - #478

Open
olegbrok wants to merge 1 commit into
feat/security-audit-stackedfrom
feat/rate-limit-stacked
Open

feat(middleware): mode-aware in-memory rate limiter (#438) [stacked on #477]#478
olegbrok wants to merge 1 commit into
feat/security-audit-stackedfrom
feat/rate-limit-stacked

Conversation

@olegbrok

Copy link
Copy Markdown
Collaborator

Summary

PR-7 of the #433 web-exposed hardening track. Slows brute-force on login and burst-probing on admin endpoints. Stacked on #477#476#475#474#473#472.

Closes part of #433. Refs #438.

Buckets

Bucket Routes Limit Window Penalty
`auth` POST `/auth/login` + `/auth/password` 5 300s 900s lockout
`admin` `/admin/*` 30 60s none
`webhook` `/triggers/webhook/*` 60 60s none
`default` everything else 300 60s none (lan/web only)

Mode behaviour: trusted = off; lan = auth + admin only; web = all four.

IP attribution

Keys come from the canonical client IP from #442's resolver. Raw `X-Forwarded-For` is NOT trusted; an attacker on the public internet can't dodge the auth bucket by spoofing XFF (test `test_uses_canonical_ip_from_resolver` verifies this end-to-end).

Audit

Every 429 emits a `rate_limit.exceeded` event into the security audit (#440) with bucket, limit, ip, via_proxy, forwarded_chain, method, target. No-op when `security_audit` isn't on `app.state`.

Murzik review followups (from #438)

Tests (27 new)

  • `classify_request`: login POST/GET, password POST, admin prefix, webhook prefix, default, case-insensitive method
  • `InMemoryRateLimiter`: under-limit / at-limit / lockout persists / window rolls off / keys + buckets independent / unknown bucket passes / reset / custom buckets
  • `DEFAULT_BUCKETS`: auth has lockout, admin no lockout, default 300/60
  • Middleware: trusted pass-through, LAN throttles auth, LAN default off, web default active, audit emitted on 429, canonical-IP keying defeats XFF spoofing
  • E2E `create_api`: limiter on app.state, trusted-mode no throttle through TestClient

503 tests green across the full hardening suite + existing api tests.

Test plan

  • `pytest tests/test_rate_limit.py -v` (27 passing)
  • `pytest tests/test_*.py` (no regression)
  • `ruff check src/pinky_daemon/middleware/rate_limit.py src/pinky_daemon/api.py tests/test_rate_limit.py` (clean)
  • Manual: `PINKY_DEPLOYMENT_MODE=web`; hit `/auth/login` 6× → 6th gets 429 with `Retry-After` header
  • Manual: confirm `security_audit_log` row appears for the 429

🤖 Opened by Barsik

PR-7 of the #433 web-exposed hardening track. Slows brute-force on
login and burst-probing on admin endpoints. Stacked on #477#476#475#474#473#472.

Buckets
=======
============  ================================  =====  ======  =================
Bucket        Routes                            Limit  Window  Penalty
============  ================================  =====  ======  =================
auth          POST /auth/login + /auth/password 5      300s    900s lockout
admin         /admin/*                          30     60s     none
webhook       /triggers/webhook/*               60     60s     none
default       everything else                   300    60s     none (lan/web)
============  ================================  =====  ======  =================

Mode behaviour
--------------
* trusted — disabled (back-compat with Mac Mini)
* lan     — auth + admin only
* web     — all four buckets

IP attribution
--------------
Keys come from the canonical client IP from #442's resolver. raw
X-Forwarded-For is NOT trusted; an attacker on the public internet
can't dodge the auth bucket by spoofing XFF (test
test_uses_canonical_ip_from_resolver verifies this end-to-end).

Audit
-----
Every 429 emits a `rate_limit.exceeded` event into the security audit
(#440) with bucket, limit, ip, via_proxy, forwarded_chain, method,
target. No-op when security_audit isn't on app.state.

Implementation
==============
* InMemoryRateLimiter — sliding-window-counter; thread-safe via single
  mutex (microsecond hold time). Per-process state.
* BucketConfig dataclass — limit, window, optional penalty_seconds.
* classify_request(method, path) — pure function so route → bucket
  mapping is unit-testable.
* build_rate_limit_middleware(limiter=None) — closure; reads
  app.state.rate_limiter when no explicit limiter is passed so a
  process can hot-swap config.

Wiring
======
create_api installs InMemoryRateLimiter on app.state.rate_limiter and
registers the middleware. Order matters — runs after security headers
so 429 responses still get headers; before route handlers so admin
work doesn't kick off when limiter says no.

Out of scope (deferred)
=======================
* Login keying on (IP, normalized email) — needs the auth user model
  from #435. Today: IP only. TODO marker in the module.
* Per-user admin bucket — same dependency on #435.
* Webhook keying on token-hash-prefix — needs the token model from
  #435. Today: IP only.
* Redis backend for multi-worker — process-local only; #441 should
  add a guard once the backend lands.

Murzik review followups (from #438) — addressed
================================================
* [x] IP-keyed buckets use #442 resolver, NOT slowapi.get_remote_address
* [-] Login keys on (IP, email) — IP-only this PR; deferred to #435
* [x] Lockout window implemented (penalty_seconds on auth bucket)
* [x] Process-local backend documented; multi-worker guard deferred to #441
* [-] Webhook by token-prefix — IP-only this PR; deferred to #435
* [-] Admin per-user — IP-only this PR; deferred to #435
* [x] 429s emit audit events to #440

Tests (27 new)
==============
* classify_request: login POST/GET, password POST, admin prefix,
  webhook prefix, default, case-insensitive method.
* InMemoryRateLimiter: under-limit allowed, at-limit rejected, lockout
  persists for penalty seconds, window rolls off for non-lockout
  bucket, keys independent, buckets independent, unknown bucket
  passes, reset clears state, custom buckets.
* DEFAULT_BUCKETS: auth has lockout, admin no lockout, default 300/60.
* Middleware: trusted pass-through, LAN throttles auth, LAN default
  bucket off, web default bucket active, audit emitted on 429,
  canonical-IP keying defeats XFF spoofing.
* End-to-end create_api: app.state has limiter, trusted-mode no
  throttle through TestClient.

503 tests green across the full hardening suite + existing api tests.

Closes part of #433. Refs #438.

🤖 Authored by Barsik
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.

1 participant