Repository navigation
Commit f13cb22
authored
feat(security): per-token call rate limit for agent tokens (#1746)
## Description
Closes #1699. Adds a per-token call rate limit for agent tokens, built on the plan in the maintainer's comment on the issue.
- **Where it is enforced.** At the managed-client admission seam (`acquireAdmission`, before the concurrency registry), so it covers `call_tool_*`, direct routing mode, `code_execution` (each call a script makes counts) and the REST tool-call path with one check. `ListTools` and `Ping` are not counted, same as the concurrency limits.
- **The limit.** A token bucket per agent token: N calls per minute plus a burst. Off by default (absent or `per_minute: 0`). The limiter is in the repo (`internal/upstream/limiter/token_rate.go`, injectable clock), no new dependency. State is in memory, keyed by owner + token name (names are only unique per owner); idle buckets are swept. A restart refills every bucket.
- **Agent tokens only.** The check runs only for `AuthTypeAgent` callers with a token name. The admin API key, the Unix socket and unauthenticated callers go through unchanged.
- **Config.** `agent_token_rate_limit: {per_minute, burst}` is the default for every agent token. Hot-reloadable (goes through `applyConcurrencyLimits`; added to `undiffedHotConfigFields`). Negative values, values over 100000 and a `burst` without a `per_minute` are validation errors that name the field.
- **Per token.** `mcpproxy token create --rate-limit N [--rate-burst M]` and `rate_limit` on `POST /api/v1/tokens` store a budget on the token. It replaces the config default for that token; `per_minute: 0` exempts it. `GET /api/v1/tokens[/{name}]` and `token show` return it. The record gets one new omitempty field, so no storage migration.
- **Refusal.** A new `LimitError` reason `rate_limited` with scope `token` goes through the existing shed plumbing instead of a parallel path (a `Shed()` predicate replaces the hard-coded `queue_full || queue_timeout` checks):
- MCP: an `isError` tool result.
- REST: `429` with `Retry-After` (seconds, rounded up, minimum 1).
- Message: "Rate limit reached: this agent token may make N tool calls per minute (bursts of up to M) and has used its allowance. Retry in S second(s)."
- A refused call takes no concurrency slot and never reaches the upstream.
- **Record.** The refusal is one rejected `tool_call` in the activity log with reason `rate_limited`, written by the same observer as the concurrency sheds, so there is exactly one row per refusal and no second authz line. It carries the token name, prefix and client id (the observer now passes the attribution through). The audit-line `reason` vocabulary gains `rate_limited`, in both schema copies.
- **Metric.** `mcpproxy_tool_calls_rejected_total{reason="rate_limited",scope="token"}` (existing counter, new label values).
- Docs: `docs/features/agent-tokens.md` (Call Rate Limit), `docs/configuration.md`, `docs/configuration/config-file.md`, `docs/features/audit-log.md`, the spec 107 audit-line contract, CHANGELOG. OAS regenerated with `make swagger` (the diff is only the new config type and the two 429 descriptions).
Choices I made that you may want different:
- A token's own budget replaces the config default rather than stacking on it, and `0` is an explicit exemption. That gives a "no limit for this one batch token" without turning the default off for everyone. Say if you want the stricter of the two instead.
- There is no way to change a budget on an existing token (there is no token update route today). Re-create the token, or change the config default.
- Changing a budget keeps the calls a token has already spent (clamped to the new size); it does not refund them.
- `burst` defaults to `per_minute`, so `per_minute: 60` alone allows 60 calls at once, then one a second.
- `reason: rate_limited` is added to the shared `RejectedCount` in the activity summary, so it counts alongside the concurrency sheds. Say if you want it separate.
- Out of scope, as agreed on the issue: a tighter write/destructive cap, a daily budget, and a per-token metric label (token names would be unbounded cardinality, so the metric stays `scope="token"` without the name; the name is in the activity record).
## Testing
- [x] I have tested these changes locally
- [x] I have added/updated tests that prove my fix is effective or my feature works
- [x] All existing tests pass (see below)
- `internal/upstream/limiter`: bucket math with a fake clock (burst allowed, then refused with the exact `RetryAfter`, refill, per-key isolation, per-token override and exemption, changed budget, sweep, concurrent callers), error text and `Shed()`.
- `internal/upstream/managed`: enforcement at the seam: burst then refusal with `Retry-After`, refill, per-token isolation, off by default, admin and unauthenticated callers unaffected, a refused call takes no concurrency slot.
- `internal/server`: end to end against a counting upstream through `call_tool_read`, `code_execution`, direct mode and the REST typed error. The upstream is never reached after the budget is spent.
- `internal/runtime`: the rejected activity record names the token; a concurrency shed stays unattributed; a real runtime with a live HTTP upstream, the metric sink and a config reload; `IssueToken` validation and storage.
- `internal/httpapi`: `rate_limit` on create / get / list, validation, exemption, and the 429 + `Retry-After` mapping (1.5s becomes 2).
- `internal/config`, `internal/auth`, `internal/audit`, `internal/upstream`, `cmd/mcpproxy`: validation, JSON round trip, schema vocabulary, manager default follows reloads, CLI flags and output.
`go build ./...`, `go build -tags server ./...`, `go vet ./...` and `go vet -tags server ./...` are clean. `golangci-lint` v2.9.0 with `.github/.golangci.yml` reports 0 issues, bare and with `--build-tags server`. `go test -race -skip "E2E|Binary|MCPProtocol|TestInfoEndpoint|TestGracefulShutdownNoPanic|TestSocketInfoEndpoint" ./...` passes, as does the server-tag CI command for `./internal/{serveredition,config,oauth,server,httpapi,storage}/...` and `-shuffle=on` on the packages above.1 parent 0cfb7eb commit f13cb22
49 files changed
Lines changed: 2189 additions & 74 deletions
File tree
- cmd/mcpproxy
- docs
- configuration
- features
- schemas
- internal
- audit
- auth
- config
- contracts
- httpapi
- observability
- runtime
- server
- storage
- upstream
- limiter
- managed
- oas
- specs/107-server-edition-sso-hardening/contracts
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
91 | 91 | | |
92 | 92 | | |
93 | 93 | | |
| 94 | + | |
94 | 95 | | |
95 | 96 | | |
96 | 97 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
29 | 34 | | |
30 | 35 | | |
31 | 36 | | |
| |||
82 | 87 | | |
83 | 88 | | |
84 | 89 | | |
85 | | - | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
86 | 98 | | |
87 | 99 | | |
88 | 100 | | |
| |||
91 | 103 | | |
92 | 104 | | |
93 | 105 | | |
| 106 | + | |
| 107 | + | |
94 | 108 | | |
95 | 109 | | |
96 | 110 | | |
| |||
170 | 184 | | |
171 | 185 | | |
172 | 186 | | |
173 | | - | |
| 187 | + | |
174 | 188 | | |
175 | 189 | | |
176 | 190 | | |
| |||
188 | 202 | | |
189 | 203 | | |
190 | 204 | | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
191 | 214 | | |
192 | 215 | | |
193 | 216 | | |
| |||
208 | 231 | | |
209 | 232 | | |
210 | 233 | | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
211 | 237 | | |
212 | 238 | | |
213 | 239 | | |
| |||
257 | 283 | | |
258 | 284 | | |
259 | 285 | | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
260 | 289 | | |
261 | 290 | | |
262 | 291 | | |
263 | 292 | | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
264 | 335 | | |
265 | 336 | | |
266 | 337 | | |
| |||
428 | 499 | | |
429 | 500 | | |
430 | 501 | | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
431 | 505 | | |
432 | 506 | | |
433 | 507 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
171 | 171 | | |
172 | 172 | | |
173 | 173 | | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
338 | 338 | | |
339 | 339 | | |
340 | 340 | | |
341 | | - | |
| 341 | + | |
342 | 342 | | |
343 | 343 | | |
344 | 344 | | |
| |||
350 | 350 | | |
351 | 351 | | |
352 | 352 | | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
353 | 391 | | |
354 | 392 | | |
355 | 393 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
268 | 268 | | |
269 | 269 | | |
270 | 270 | | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
271 | 295 | | |
272 | 296 | | |
273 | 297 | | |
| |||
0 commit comments