Skip to content

fix(limits): enforce route caps and close the places limits leaked - #94

Merged
fylorn merged 1 commit into
devfrom
fix/limit-enforcement
Oct 10, 2026
Merged

fylorn merged 1 commit into
devfrom
fix/limit-enforcement

Conversation

@fylorn

@fylorn fylorn commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Fixes eight places where the gateway's limits leaked or did nothing. Each one was checked against the code before it was changed.

What changes

  1. A limits load that fails follows security.rate_limit_fail_closed (server/middleware/api_key_auth.rs). The key's and the user's limits were loaded with unwrap_or_default, so a database error meant the request ran with no limits. Now: fail open (the default) runs the request without them and logs a warning with the error (gateway_limits_load_fail_open_total). Fail closed refuses it (gateway_limits_load_fail_closed_total, plus an audit row limits_unavailable).

    • Refusal shape. The brief asked for "the gateway's 503 error shape", but the existing fail-closed path (Redis down in check_limits) answers 429 + Retry-After: 30 (GatewayError::limiter_unavailable), and the setting's hint in Settings says so. The setting should give one answer, so this uses the same one. It is labelled limits_unavailable (Rate limited: limits_unavailable) and written in the caller's dialect. On MCP it is a JSON-RPC error with HTTP 429.
    • The refusal comes after the gateway-use check, so a key without access still gets its 403.
  2. A budget read that fails follows the same setting (common/limits/budget.rs). current_spend swallowed the Redis error with .ok() and returned 0, so the fail-closed branch in check_budget could never run. The error is now returned. Fail closed answers 429 budget_unavailable, fail open goes on.

  3. Route caps are enforced (new gateway/src/route_caps.rs, selection in proxy/routing.rs).

    • Where the meaning comes from. model_routes.rpm_cap / tpm_cap are documented in db/schema.sql ("routes at cap are filtered out the same way circuit-broken routes are") and in the route editor ("enforced via the same sliding-window engine that drives api-key/user RPM/TPM, just with a route key"). Both match the expected semantics.
    • How it works. A selected route goes through route_caps::admit before the request is sent. That is one atomic check of both caps, and it counts the request when neither cap is reached. A route at a cap is skipped like an open breaker and the next candidate is picked.
    • When every candidate is capped. If no route failed, the answer is 429 route:requests/1m or route:tokens/1m, with a Retry-After of when the first capped route has room. If some route did fail, its upstream error is the answer instead. This applies to buffered and streamed requests alike.
    • Counting. Same sliding-window engine as the other limits (sliding::admit_counters_at / record_counters_at, newly exposed). The window is one minute with 1-second buckets. Keys are ratelimit:{route:<route_id>}:ai_gateway:route:<route_id>:{requests|tokens}:60, so one hash tag per route keeps them cluster-safe.
    • Tokens. The TPM cap counts weighted tokens, added after the response in post_flight_account, like the other token limits.
    • Attempts. Every attempt sent to a route counts against its RPM cap, failover attempts included.
    • Redis failure. Follows rate_limit_fail_closed: fail closed answers 429 rate_limiter_unavailable, fail open does not apply the cap.
  4. Access is checked before the request counts (common/lifecycle, gateway/proxy/pipeline.rs, mcp-gateway/proxy.rs). The order is now check_budget (read-only) → check_access → check_limits on both gateways. The type states changed to match:

    • check_access(Raw) -> AccessChecked
    • check_limits(AccessChecked) -> Authorized
    • LimitsChecked is gone.

    The MCP tools/call path had the same ordering bug.

  5. Stale comments corrected. These comments described things that do not exist:

    • models/team.rs: there are no team caps.
    • limits/mod.rs: there is no LimitsCache, and nothing subscribes to limits:changed.
    • weight.rs: the same limits:changed point.
    • The module header (check_cap, plan.md).

    No features were added.

  6. One loader decides which roles apply at the gateway, for access and for limits (auth/rbac.rs).

    • The bug. compute_user_role_constraints read every role assignment (load_user_policy_documents), team-scoped grants included. Access read only global and team-attached roles (load_gateway_policy_documents). It now uses the gateway loader too, and the unused load_user_role_ids (whose comment claimed the rate limiter used it) is removed.

    • Why team-scoped grants are left out.

      • The user form says: "A role assigned at team scope grants administration of that team only. It does not grant access to models or MCP tools through the gateways." (users.scopeTeamNoGateway)
      • The teams page says: "All members automatically inherit the team's roles and permissions."
      • The schema says a team scope "applies only when the target subject … belongs to that team", and gateway requests carry no team.

      So: roles attached to a team apply (access and limits), and roles granted at team scope apply to neither.

    • What stays the same. Constraints already came only from the Allow statements for *_gateway:use. Limits remain per surface, not per model.

  7. Cache hits count their tokens (proxy/generate.rs). A hit now runs post_flight_account with the tokens the cached answer records, against the same user/key token rules and budgets the pre-flight checked. No route cap is involved, because no route answered. The cache stores only input/output totals, so all input counts as plain input. The cost a hit reports stays 0.

  8. Legacy monthly quota removed. gateway/quota.rs (QuotaManager), GatewayState.quota and its three calls in generate.rs are gone. It wrote quota:{…}:used:<month> against a limit nothing ever set. The Redis-cluster test drops its quota section and gains a route-cap script check.

Also: the Settings label and hint for security.rate_limit_fail_closed (en/zh) and its seeded description now describe everything the setting covers.

CHANGELOG [Unreleased] covers these:

  • Read before upgrading: route caps are now enforced, limit/budget store failures follow the setting, team-scoped roles no longer limit, cache hits count tokens.
  • Fixed: the access-before-limits order.
  • Changed: the quota removal (the quota:* keys can be deleted).

For the integrator (feat/key-usage-endpoint)

  • budget::current_spend behaviour changed, signature unchanged. It already returned Result<Vec<CapStatus>, fred::error::Error>, but it returned Ok with current: 0 when a read failed. It now returns Err. A usage reader that wants zeros on failure must handle the Err (the console handlers already unwrap_or_default(), so on a Redis failure they now show no caps rather than zeroed caps).
  • sliding::admit_at / record_at keep their signatures and now delegate to the new public sliding::admit_counters_at / record_counters_at (+ sliding::Counter). current_count and ResolvedRule are untouched.
  • api_key_auth.rs: the key's and user's constraints are now loaded by load_request_limits and resolved by limits_or_refusal. When loading fails open, GatewayRequestIdentity.key_constraints is empty, so a /v1/usage handler mounted behind this middleware would report no limits for that request. When it fails closed, the middleware refuses before the handler runs.
  • rbac::compute_user_role_constraints / compute_user_surface_constraints keep their signatures. They no longer include team-scoped role grants (item 6). rbac::load_user_role_ids is removed.
  • GatewayErrorResponse::in_dialect is now pub.
  • GatewayState.quota is removed.
  • Lifecycle: LimitsChecked is removed. check_access takes Raw and returns AccessChecked. check_limits takes AccessChecked and returns Authorized.

Not changed

  • Route caps count weighted tokens (as the brief expected and as the route editor says, "the same … engine that drives api-key/user RPM/TPM"), not the raw tokens an upstream's own TPM quota counts. If an operator wants to mirror a provider's raw TPM limit, that would need a separate setting.
  • A route's RPM count is taken at admission. A streamed request whose client leaves before the stream's first poll is counted without reaching the upstream.
  • The unused gateway::rate_limiter::RateLimiter (constructed into GatewayState, never called) is left alone. It is out of scope; removing it is a follow-up.

Verification

  • cargo fmt --all -- --check, cargo clippy --workspace --all-targets -- -D warnings: clean.
  • node web/scripts/check-i18n.mjs: parity holds (only two values changed; pnpm test / pnpm build not run).
  • cargo nextest run --workspace --lib --bins (proxy env unset): 791 passed
  • Integration (dedicated containers postgres:18-alpine :25432, redis:8-alpine :26379, clickhouse 26.8 :28123): cargo nextest run -p think-watch-test-support --run-ignored only --profile ci with SKIP_NETWORK_TESTS=1: 433 passed (the 11 new tests in limit_enforcement.rs and 2 in team_rbac.rs included)

New unit tests:

  • budget::a_failed_read_is_an_error_not_zero
  • check_budget::an_unreadable_budget_{refuses_when_failing_closed,passes_when_failing_open}
  • check_limits::an_unreachable_store_{refuses_when_failing_closed,passes_when_failing_open}
  • api_key_auth::{a_failed_limits_load_follows_the_fail_closed_setting,the_refusal_is_a_429_in_the_callers_format}
  • route_caps::* (hash-tag slot, counters per cap, fail open/closed, the all-capped refusal)

New integration tests in tests/limit_enforcement.rs:

  • DB failure loading limits, with fail closed and with fail open
  • Redis failure reading a budget, with fail closed and with fail open
  • A route at its RPM cap is skipped and the next route serves
  • All routes capped gives 429 with Retry-After (buffered and stream)
  • A TPM cap counts weighted tokens (2×7+3×3=23) after the response and then refuses
  • The route-cap scripts on one node
  • A refused model does not move the request counter
  • A refused MCP tool does not move the request counter
  • A cached answer moves the token rule and the budget counter

In tests/team_rbac.rs:

  • A team-attached role with a rate limit grants and limits
  • The same role granted at team scope neither grants nor limits

tests/redis_cluster.rs has the_route_cap_scripts_run_on_a_cluster (needs TEST_REDIS_CLUSTER_URL; not run locally).

Security notes

  • Every change narrows what a request can do or refuses it earlier: limits no longer fail open on store errors when fail-closed is set, route caps apply, and access is checked before counting.
  • Item 6 removes limits from users whose limiting role was granted only at team scope. That matches what those grants already did for access, and the CHANGELOG tells operators to re-grant globally if the limits were intended.
  • No credential is logged: the new warnings log the database/Redis error text, the API key's id (not the key) and route ids.
  • The new audit row carries the user, key id and lineage, as the existing rate_limiter_unavailable row does.

🤖 Generated with Claude Code

Each of these was a way past a limit, or a limit that did nothing:

- Loading a request's limits from Postgres fell back to "no limits" on
  any error, and a budget read swallowed Redis errors as 0, so the
  budget gate's fail-closed branch could never run. Both now follow
  security.rate_limit_fail_closed like the rate limiter does: open runs
  the request and logs the error, closed answers the existing
  limiter-unavailable 429 (limits_unavailable / budget_unavailable).
- model_routes.rpm_cap / tpm_cap were loaded and never read. They are
  now enforced as the schema and the route editor describe: a route at
  its cap is skipped like a broken one, and with every candidate capped
  the request gets 429 with Retry-After. Counted on the sliding-window
  engine under a {route:<id>} hash tag; tokens are weighted and added
  after the response.
- The rate-limit stage, which charges the request counters, ran before
  the model / tool access check on both gateways, so a refused request
  spent a request. The order is now budget, access, limits, and the
  lifecycle type states enforce it.
- Role-level limits were merged from every role assignment while gateway
  access reads only global and team-attached roles. A team-scoped grant
  administers the team and grants nothing at the gateway, as the UI
  says, so its limits no longer apply either: one loader decides both.
- A cache hit counted a request but no tokens, so repeating a cached
  request got round token limits and budgets. It now counts the tokens
  the cached answer records.
- The legacy monthly quota counted into quota:* keys against a limit
  nothing could set. Removed with its metric; budgets cover it.

Also corrects comments that described a limits cache, a limits:changed
subscriber and team budget caps, none of which exist.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fylorn
fylorn merged commit 235e38b into dev Oct 10, 2026
6 checks passed
@fylorn
fylorn deleted the fix/limit-enforcement branch October 10, 2026 15:22
fylorn added a commit that referenced this pull request Oct 10, 2026
Resolves the overlap with #94: cached answers are accounted once, through
post_flight_account, which also records the key and user usage counters
(the separate count_cache_hit_usage is gone); usage::record_request runs
after check_limits in the new budget, access, limits order; current_spend
propagates a counter read error. Removes the gateway RateLimiter that was
built into GatewayState and never called.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fylorn fylorn mentioned this pull request Oct 10, 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.

1 participant