fix: rate-limit failed /v1/* bearer-token auth#227
Conversation
The daemon's `/v1/*` routes accepted unlimited authentication attempts. Local-only binding bounds the attack surface, but on a 0.0.0.0 bind (Windows containers) or from a co-resident process, a guesser had no cost to retry. Add a small per-IP failure counter: after 20 consecutive failures within 60s the client is locked out for 60s with a 429 + Retry-After response. A successful auth clears the counter, so legitimate clients are unaffected. Bounded LRU eviction keeps the table small. Includes unit tests for the limiter (no integration test since it would require standing up the full daemon).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 843cbd6327
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const preCheck = authLimiter.check(clientKey); | ||
| if (!preCheck.allowed) { | ||
| json( | ||
| res, | ||
| 429, |
There was a problem hiding this comment.
Because this pre-check runs before accepting a valid bearer token, 20 bad /v1/* requests from the same remoteAddress cause all later requests from that address to return 429, even when they include the configured token. In the normal loopback setup all local processes share 127.0.0.1/::1, so any unpaired local process can lock out the paired extension or CLI for the full timeout; the limiter can still count failed attempts, but valid tokens should bypass or clear the lockout.
Useful? React with 👍 / 👎.
1 participant
Summary
/v1/*routes accept an unbounded number of failed authentication attempts. The daemon binds 127.0.0.1 in the common case, so the practical reach is local processes that don't already hold a paired token — but on a 0.0.0.0 bind (Windows containers, perresolveDaemonListenHost) or from any co-resident process, a guesser had no cost to retry. Adding a small lockout is cheap defense-in-depth and combines naturally with the timing-safe compare in PR #226.Design
New
src/daemon/auth-rate-limit.tsexportsAuthRateLimiter:req.socket.remoteAddress. Null/empty keys are not tracked (always allowed).429with aRetry-Afterheader.now()for tests.src/daemon/server.tsconsults the limiter inside the existing/v1/*gate.Test plan
tests/daemon.auth-rate-limit.test.tscovers: under-threshold allow, exact-threshold trip + retry-after value, client isolation, success clearing the counter, failure-window reset, null-key handling, LRU eviction.pnpm -s check.Notes
Third of a small series from a security-oriented review. Sibling PRs: #225 (attachment path traversal) and #226 (timing-safe token compare). All three are independent — feel free to take any subset.