Skip to content

feat(webapp): share rate limit bucket across additional API keys per environment - #4508

Merged
carderne merged 2 commits into
mainfrom
feature/tri-12888-add-redis-cache-for-additional-api-key-environment-lookup
Aug 6, 2026
Merged

feat(webapp): share rate limit bucket across additional API keys per environment#4508
carderne merged 2 commits into
mainfrom
feature/tri-12888-add-redis-cache-for-additional-api-key-environment-lookup

Conversation

@carderne

@carderne carderne commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

What

Rate-limit the API by environment rather than per API key.

Previously the limiter keyed its bucket on the hash of the full Authorization header — one bucket per key. With additional environment API keys (tr_*_sk_*), an environment can mint many keys and each got its own full bucket, so more keys = higher effective rate limit. This collapses all of an environment's keys onto a single shared per-environment bucket, so the ceiling is exactly the configured limit regardless of key mix.

How

  • authorizationRateLimitMiddleware now lets the override return { config?, identifier? }. identifier, when present, is the rate limit bucket key; otherwise it falls back to the hashed Authorization header (unchanged legacy behavior, still used by engineRateLimiter and any unauthenticated fallthrough).
  • apiRateLimiter's override resolves the environment id and uses it as the identifier:
    • Additional keys (isAdditionalApiKey) resolve via a new resolveAdditionalApiKeyRateLimitScope() — a scope-agnostic keyHash → (environmentId, org limiter config) lookup. It is deliberately permissive (restricted keys resolve too) because it's used only for bucketing, never as an auth decision — request auth still goes through the RBAC bearer controller, which enforces scopes. Revoked/expired keys are excluded so they can't hold a bucket warm.
    • Root/legacy keys reuse the environment already resolved by authenticateAuthorizationHeader and key on environment.id too.
  • The identifier is always the stable environment id, never the secret key (which can rotate and would split the bucket).
  • The whole override result is cached per key by the existing SWR cache, so no extra per-request lookup and no separate Redis mapping is added.

Behavior notes

  • Root + additional keys of the same environment now share one bucket (ceiling = configured limit, not a multiple of it). Restricted additional keys are included — they were the biggest gap, since they authenticate via the RBAC controller and previously fell back to per-key buckets.
  • Public JWTs keep their existing fixed-window, per-token bucketing.
  • One-time bucket reset on deploy (bucket keys change); harmless.

Tests

  • New: two tokens resolving to the same identifier share one bucket.
  • New: with no identifier, bucketing stays per-key (legacy behavior preserved).
  • Updated existing override tests to the new { config } return shape.

Base: feat/multi-keys-surface. Closes TRI-12888.

@changeset-bot

changeset-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 5a6a191

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5647e820-5ca2-468c-abe4-381d0704f58e

📥 Commits

Reviewing files that changed from the base of the PR and between ee84c89 and 5a6a191.

📒 Files selected for processing (2)
  • apps/webapp/app/models/api-key.server.ts
  • apps/webapp/app/services/apiRateLimit.server.ts
💤 Files with no reviewable changes (2)
  • apps/webapp/app/models/api-key.server.ts
  • apps/webapp/app/services/apiRateLimit.server.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout. (18)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 12)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 12)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
  • GitHub Check: runops-guard / runops-guard
  • GitHub Check: report
  • GitHub Check: code-quality / code-quality
  • GitHub Check: Analyze (javascript-typescript)

Walkthrough

API rate limits now use environment-scoped buckets. A new resolver maps valid private API keys to an environment and limiter configuration. Rate-limit middleware carries optional identifiers through override resolution, caching, validation, and bucket checks. JWT limits retain fixed-window behavior. Limits presenter lookups now use environment identifiers instead of hashed API keys. Tests cover shared buckets, isolated token buckets, preview environments, and deleted projects.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: sharing API rate-limit buckets across additional API keys within an environment.
Description check ✅ Passed The description clearly explains the problem, implementation, behavior changes, and tests, but omits the template checklist and screenshots section.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/tri-12888-add-redis-cache-for-additional-api-key-environment-lookup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@carderne
carderne marked this pull request as ready for review August 5, 2026 09:20
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@carderne
carderne force-pushed the feature/tri-12888-add-redis-cache-for-additional-api-key-environment-lookup branch from ba817b9 to 316db63 Compare August 5, 2026 17:08
@pkg-pr-new

pkg-pr-new Bot commented Aug 5, 2026

Copy link
Copy Markdown

Open in StackBlitz

@trigger.dev/build

npm i https://pkg.pr.new/@trigger.dev/build@ee84c89

trigger.dev

npm i https://pkg.pr.new/trigger.dev@ee84c89

@trigger.dev/core

npm i https://pkg.pr.new/@trigger.dev/core@ee84c89

@trigger.dev/python

npm i https://pkg.pr.new/@trigger.dev/python@ee84c89

@trigger.dev/react-hooks

npm i https://pkg.pr.new/@trigger.dev/react-hooks@ee84c89

@trigger.dev/redis-worker

npm i https://pkg.pr.new/@trigger.dev/redis-worker@ee84c89

@trigger.dev/rsc

npm i https://pkg.pr.new/@trigger.dev/rsc@ee84c89

@trigger.dev/schema-to-json

npm i https://pkg.pr.new/@trigger.dev/schema-to-json@ee84c89

@trigger.dev/sdk

npm i https://pkg.pr.new/@trigger.dev/sdk@ee84c89

commit: ee84c89

devin-ai-integration[bot]

This comment was marked as resolved.

@carderne
carderne force-pushed the feature/tri-12888-add-redis-cache-for-additional-api-key-environment-lookup branch from 316db63 to cd2a0c0 Compare August 5, 2026 17:14
devin-ai-integration[bot]

This comment was marked as resolved.

@carderne
carderne force-pushed the feature/tri-12888-add-redis-cache-for-additional-api-key-environment-lookup branch from eb0e8f7 to e9ae728 Compare August 6, 2026 09:42
devin-ai-integration[bot]

This comment was marked as resolved.

@carderne
carderne force-pushed the feature/tri-12888-add-redis-cache-for-additional-api-key-environment-lookup branch 2 times, most recently from 10388e2 to 15b81de Compare August 6, 2026 10:21
Base automatically changed from feat/multi-keys-surface to main August 6, 2026 14:27
…environment

remove comments

fix(webapp): bucket restricted additional API keys by environment too

feat(webapp): bucket root API keys per environment too

fix(webapp): version rate-limit cache key and validate cached shape on read

use new resolver for api keys

fix(webapp): keep environment rate-limit buckets consistent

Use environment identifiers when displaying remaining API capacity and ignore additional keys tied to deleted projects.

Update .server-changes/additional-api-key-rate-limit-bucket.md

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>

fix(webapp): keep public API keys on separate rate-limit buckets

fix limits presenter for preview branches
@carderne
carderne force-pushed the feature/tri-12888-add-redis-cache-for-additional-api-key-environment-lookup branch from 15b81de to ee84c89 Compare August 6, 2026 14:27
coderabbitai[bot]

This comment was marked as resolved.

@carderne
carderne merged commit 088f68b into main Aug 6, 2026
36 checks passed
@carderne
carderne deleted the feature/tri-12888-add-redis-cache-for-additional-api-key-environment-lookup branch August 6, 2026 15:05
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.

2 participants