feat(limits): cap API key limits to member budget#2935
Merged
Conversation
Enforce a clear precedence between per-member (org/SSO) budgets and per-key usage limits: - Gateway: check the member budget before the per-key usage limits in every request surface (chat, embeddings, ocr, speech, moderations, videos, mcp), so a member who is over budget is denied first. - UI: the create/edit API-key dialogs show the owner's effective org limits and reject a key limit that exceeds them (recurring caps are compared by normalized hourly rate so different windows compare fairly). - API: keys create/update reject a key limit above the owner's effective member budget as a server-side guardrail. - Share the pure comparison helper via @llmgateway/shared so both the client bundle and the backend use one implementation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
WalkthroughThis PR adds shared member-budget validation for API key limits, enforces it in API routes and UI flows, reorders gateway checks to prioritize member budgets, enables spend-limit enforcement for responses endpoints, and seeds a default developer budget during SSO onboarding. ChangesMember budget enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/shared/src/member-budget-limits.spec.ts (1)
1-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a "month" unit test case.
Coverage is solid for day/week comparisons, but there's no case exercising
"month"normalization (720h approximation), which is the unit most likely to diverge from calendar-based runtime enforcement. Adding one would pin down expected behavior here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/member-budget-limits.spec.ts` around lines 1 - 116, The tests in validateApiKeyLimitsWithinMemberBudget cover day and week normalization but miss the month path. Add a month-based case in member-budget-limits.spec.ts that exercises ApiKeyLimitConstraints with periodUsageDurationUnit set to "month" and verifies the expected 720h normalization behavior in both allowed and rejected scenarios. Use the existing validateApiKeyLimitsWithinMemberBudget helper and the NO_LIMITS/member constraint patterns to keep the new test consistent with the current suite.apps/gateway/src/chat/chat.ts (1)
1889-1893: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReordering logic looks correct.
Member budget now gates before per-key usage limits, matching
assertMemberWithinBudget/assertApiKeyWithinUsageLimitssemantics.Separately: this exact 2-line enforcement sequence is duplicated verbatim across
chat.ts,embeddings.ts, and (with an added project lookup)mcp.ts. Consider extracting a single helper (e.g.assertMemberAndKeyWithinLimits(apiKey, organizationId)inapps/gateway/src/lib/api-key-usage-limits.ts) so future policy changes to this ordering only need to happen in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/gateway/src/chat/chat.ts` around lines 1889 - 1893, The member-budget-then-key-usage enforcement is duplicated in chat.ts, embeddings.ts, and mcp.ts, so future policy changes will be easy to miss. Extract the shared sequence into a single helper in api-key-usage-limits.ts, such as assertMemberAndKeyWithinLimits(apiKey, organizationId), and have the existing call sites use it instead of calling assertMemberWithinBudget and assertApiKeyWithinUsageLimits directly. Keep the helper responsible for preserving the current ordering and any project lookup needed by mcp.ts.apps/api/src/routes/keys-api.spec.ts (1)
473-492: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a matching "accepts limit at/below budget" test for PATCH.
The POST route has both reject and accept boundary tests, but PATCH only has the reject case. Consider mirroring the POST acceptance test (limit == budget) for the PATCH route to fully cover the boundary condition on this security-relevant path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/routes/keys-api.spec.ts` around lines 473 - 492, Add a PATCH boundary test alongside the existing limit rejection case in keys-api.spec.ts by mirroring the POST acceptance coverage: use the same API key route and update flow around the PATCH handler for the limit endpoint, but assert that setting usageLimit at or below the member budget succeeds. Refer to the existing PATCH test for /keys/api/limit/{id} and the nearby POST boundary tests to keep the new test aligned with the route’s budget validation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ui/src/components/api-keys/api-key-limits-dialog.tsx`:
- Around line 33-46: The dialog is using the current user’s budget via
useMyMemberBudget(organizationId), which can mismatch the server when editing
another member’s key. Update ApiKeyLimitsDialog to use the apiKey owner’s budget
instead by passing in or fetching the creator’s budget based on
apiKey.createdBy, and use that value for both the notice and client-side
validation.
In `@packages/shared/src/member-budget-limits.ts`:
- Around line 17-22: The month-to-hours mapping in PERIOD_UNIT_HOURS uses a
fixed 30-day approximation, which should be aligned with the calendar-month
logic used by the runtime period helpers. Update the month handling in
member-budget-limits.ts so the comparisons rely on the same calendar-month
window semantics as the existing period helper path, and make sure the duration
check logic in the budget limit comparison uses that shared month calculation
instead of a hardcoded 24 * 30 value.
---
Nitpick comments:
In `@apps/api/src/routes/keys-api.spec.ts`:
- Around line 473-492: Add a PATCH boundary test alongside the existing limit
rejection case in keys-api.spec.ts by mirroring the POST acceptance coverage:
use the same API key route and update flow around the PATCH handler for the
limit endpoint, but assert that setting usageLimit at or below the member budget
succeeds. Refer to the existing PATCH test for /keys/api/limit/{id} and the
nearby POST boundary tests to keep the new test aligned with the route’s budget
validation behavior.
In `@apps/gateway/src/chat/chat.ts`:
- Around line 1889-1893: The member-budget-then-key-usage enforcement is
duplicated in chat.ts, embeddings.ts, and mcp.ts, so future policy changes will
be easy to miss. Extract the shared sequence into a single helper in
api-key-usage-limits.ts, such as assertMemberAndKeyWithinLimits(apiKey,
organizationId), and have the existing call sites use it instead of calling
assertMemberWithinBudget and assertApiKeyWithinUsageLimits directly. Keep the
helper responsible for preserving the current ordering and any project lookup
needed by mcp.ts.
In `@packages/shared/src/member-budget-limits.spec.ts`:
- Around line 1-116: The tests in validateApiKeyLimitsWithinMemberBudget cover
day and week normalization but miss the month path. Add a month-based case in
member-budget-limits.spec.ts that exercises ApiKeyLimitConstraints with
periodUsageDurationUnit set to "month" and verifies the expected 720h
normalization behavior in both allowed and rejected scenarios. Use the existing
validateApiKeyLimitsWithinMemberBudget helper and the NO_LIMITS/member
constraint patterns to keep the new test consistent with the current suite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: dfc82e09-11ef-4ed6-a384-8ea74d6af73d
📒 Files selected for processing (17)
apps/api/src/routes/keys-api.spec.tsapps/api/src/routes/keys-api.tsapps/gateway/src/chat/chat.tsapps/gateway/src/embeddings/embeddings.tsapps/gateway/src/mcp/mcp.tsapps/gateway/src/moderations/moderations.tsapps/gateway/src/ocr/ocr.tsapps/gateway/src/speech/speech.tsapps/gateway/src/videos/videos.tsapps/ui/src/components/api-keys/api-key-limit-fields.tsxapps/ui/src/components/api-keys/api-key-limits-dialog.tsxapps/ui/src/components/api-keys/api-keys-list.tsxapps/ui/src/components/api-keys/create-api-key-dialog.tsxpackages/db/src/member-budget.tspackages/shared/src/index.tspackages/shared/src/member-budget-limits.spec.tspackages/shared/src/member-budget-limits.ts
Comment on lines
+33
to
+46
| organizationId: string; | ||
| } | ||
|
|
||
| export function ApiKeyLimitsDialog({ | ||
| apiKey, | ||
| children, | ||
| onSubmit, | ||
| organizationId, | ||
| }: ApiKeyLimitsDialogProps) { | ||
| const [open, setOpen] = useState(false); | ||
| const [isSubmitting, setIsSubmitting] = useState(false); | ||
| const [value, setValue] = useState(() => createApiKeyLimitFormValue(apiKey)); | ||
| const { data: memberBudgetData } = useMyMemberBudget(organizationId); | ||
| const memberBudget = memberBudgetData?.budget ?? null; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm whether backend member-budget lookup for existing-key
# limit updates uses the key owner (createdBy) vs. the authenticated caller.
rg -nP -C6 'validateApiKeyLimitsWithinMemberBudget|effective.*budget|createdBy' apps/api/src/routes/keys-api.tsRepository: theopenco/llmgateway
Length of output: 9220
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant frontend files and inspect the budget hook plus dialog usage.
git ls-files apps/ui/src/components/api-keys apps/ui/src/hooks | sed -n '1,200p'
echo '---'
ast-grep outline apps/ui/src/components/api-keys/api-key-limits-dialog.tsx --view expanded
echo '---'
ast-grep outline apps/ui/src/components/api-keys/api-keys-list.tsx --view expanded
echo '---'
rg -n -C4 'useMyMemberBudget|ApiKeyLimitsDialog|createdBy|creator|members/me|budget' apps/ui/src/components/api-keys apps/ui/src/hooks
echo '---'
ast-grep outline apps/ui/src/hooks/use-my-member-budget.ts --view expandedRepository: theopenco/llmgateway
Length of output: 24982
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the member-budget hook and the key-limits dialog implementation,
# then inspect the backend update route that enforces the budget.
sed -n '1,220p' apps/ui/src/hooks/useTeam.ts
echo '---'
sed -n '1,220p' apps/ui/src/components/api-keys/api-key-limits-dialog.tsx
echo '---'
sed -n '1880,1975p' apps/api/src/routes/keys-api.tsRepository: theopenco/llmgateway
Length of output: 8919
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for APIs that can fetch another member's budget or details.
rg -n -C3 'members/\{memberId\}|members/me|budget' apps/api/src/routes apps/ui/src/hooks/useTeam.ts apps/ui/src/lib apps/ui/src/components | sed -n '1,260p'Repository: theopenco/llmgateway
Length of output: 22416
Use the key owner’s budget here. useMyMemberBudget(organizationId) reads /team/{organizationId}/members/me, so admins editing another member’s key will see their own budget in the notice and client-side validation. The update path checks apiKey.createdBy’s effective budget, so this can disagree with the server. Pass the creator’s budget into this dialog or fetch it by member id.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ui/src/components/api-keys/api-key-limits-dialog.tsx` around lines 33 -
46, The dialog is using the current user’s budget via
useMyMemberBudget(organizationId), which can mismatch the server when editing
another member’s key. Update ApiKeyLimitsDialog to use the apiKey owner’s budget
instead by passing in or fetching the creator’s budget based on
apiKey.createdBy, and use that value for both the notice and client-side
validation.
Comment on lines
+17
to
+22
| const PERIOD_UNIT_HOURS: Record<ApiKeyPeriodDurationUnitValue, number> = { | ||
| hour: 1, | ||
| day: 24, | ||
| week: 24 * 7, | ||
| month: 24 * 30, | ||
| }; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how month/period windows are computed elsewhere for consistency with the 24*30 approximation
rg -nP -C4 '24\s*\*\s*30|PERIOD_UNIT_HOURS|getApiKeyCurrentPeriodState|getMemberPeriodSpend' apps/gateway/src/lib/api-key-usage-limits.tsRepository: theopenco/llmgateway
Length of output: 1110
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map and inspect the shared member budget limits implementation and the runtime period helpers.
printf '\n== packages/shared/src/member-budget-limits.ts outline ==\n'
ast-grep outline packages/shared/src/member-budget-limits.ts --view expanded || true
printf '\n== apps/gateway/src/lib/api-key-usage-limits.ts outline ==\n'
ast-grep outline apps/gateway/src/lib/api-key-usage-limits.ts --view expanded || true
printf '\n== db/runtime helpers search ==\n'
rg -n -C 3 'getApiKeyCurrentPeriodState|getMemberPeriodSpend|periodUsageDurationUnit|periodUsageDurationValue|ApiKeyPeriodDurationUnitValue' packages apps libs . -g '!**/node_modules/**' || trueRepository: theopenco/llmgateway
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== packages/db/src/api-key-period-limit.ts (90-160) ==\n'
sed -n '90,160p' packages/db/src/api-key-period-limit.ts
printf '\n== apps/gateway/src/lib/cached-queries.ts (839-930) ==\n'
sed -n '839,930p' apps/gateway/src/lib/cached-queries.ts
printf '\n== packages/shared/src/member-budget-limits.ts (17-101) ==\n'
sed -n '17,101p' packages/shared/src/member-budget-limits.tsRepository: theopenco/llmgateway
Length of output: 6613
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== packages/db/src/api-key-period-limit.ts (1-220) ==\n'
sed -n '1,220p' packages/db/src/api-key-period-limit.ts
printf '\n== apps/gateway/src/lib/cached-queries.ts (839-930) ==\n'
sed -n '839,930p' apps/gateway/src/lib/cached-queries.tsRepository: theopenco/llmgateway
Length of output: 5648
Align month comparisons with calendar-month runtime windows
month should use the same calendar-month math as the runtime period helpers; the fixed 24 * 30 approximation can drift on 28/29/31-day months and misclassify valid or invalid budget combinations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/shared/src/member-budget-limits.ts` around lines 17 - 22, The
month-to-hours mapping in PERIOD_UNIT_HOURS uses a fixed 30-day approximation,
which should be aligned with the calendar-month logic used by the runtime period
helpers. Update the month handling in member-budget-limits.ts so the comparisons
rely on the same calendar-month window semantics as the existing period helper
path, and make sure the duration check logic in the budget limit comparison uses
that shared month calculation instead of a hardcoded 24 * 30 value.
The /v1/responses POST and /compact handlers delegate the billable call to /v1/chat/completions, so member-budget and per-key usage limits were only enforced transitively. Enforce them explicitly up front in the shared authenticateRequest helper (opt-in, so the non-billable GET retrieval stays accessible when over budget), using the same SWR-cached queries as the other gateway paths, member budget before per-key limit. Images (/v1/images) and Anthropic (/v1/messages) forward within a few lines and translate errors into their own formats, so they remain covered by the inner chat handler. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When an SSO team is first connected (SSO provider registration), seed the org's default developer budget with a $500/month per-developer spend cap, so provisioned developers are bounded out of the box. Only applied when the org has no default developer budget configured yet; owners/admins can override it on the Team page. The seed write is audit-logged as its own organization.update event. The $500/month value is a shared constant in @llmgateway/shared so the API (which writes it) and the UI (which surfaces it in the default developer limits dialog) agree on one source of truth. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…der-user-limits # Conflicts: # apps/api/src/routes/sso.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes the relationship between per-member (org/SSO) budgets and per-key usage limits explicit and enforced, so an enterprise/SSO user whose org sets limits on them can only create API keys whose limits are at or below their own — and every billable gateway path enforces both, in the same priority order, via the SWR-cached queries.
Two guarantees:
Changes
assertMemberWithinBudgetnow runs ahead ofassertApiKeyWithinUsageLimitsin all seven direct request surfaces (chat, embeddings, ocr, speech, moderations, videos, mcp)./v1/responsesand/v1/responses/compacthandlers delegate the billable call to/v1/chat/completions, so they were only enforced transitively. They now enforce explicitly up front in the sharedauthenticateRequesthelper (opt-in, so the non-billableGET /v1/responses/:idretrieval stays accessible when over budget)./v1/imagesand/v1/messages(Anthropic) forward within a few lines and translate errors into their own formats, so they remain covered by the inner chat handler. All member-budget/limit reads go through the SWR-cached query helpers — no uncached relational reads on the hot path./team/{org}/members/me), show a notice of the org limits, and reject a key limit that exceeds them before submitting.POST /keys/apiandPATCH /keys/api/limit/{id}reject a key limit above the key owner's effective member budget (their own caps, or the org-wide default developer caps that SSO-provisioned members inherit).validateApiKeyLimitsWithinMemberBudget) lives in@llmgateway/sharedso the client bundle and the backend share one implementation. Recurring caps are compared by normalized hourly spend rate, so a key with a shorter window can't out-spend a longer member window.Billable path coverage
/v1/chat/completions/v1/embeddings,/v1/ocr,/v1/audio/speech,/v1/moderations,/v1/videos,/mcp/v1/responses,/v1/responses/compact/v1/images,/v1/messages(Anthropic)/v1/chat/completionscallNotes
Tests
packages/shared/src/member-budget-limits.spec.ts).apps/api/src/routes/keys-api.spec.ts).pnpm buildandpnpm formatpass.🤖 Generated with Claude Code
Summary by CodeRabbit