feat: restrict provider keys to specific models - #3418
Merged
Merged
Conversation
Adds an allowedModels list on provider_key so a credential whose upstream account only has a subset of the provider's catalogue is never picked by routing for a model it cannot serve. - gateway: BYOK + managed credential selection and routing availability (auto loop, unpinned routing, rate-limit/uptime fallbacks) skip keys whose allowedModels exclude the routed model - admin API: allowedModels CRUD with catalogue validation, save-time validation probes an allowed model instead of the default one, plus self-test and verify-models endpoints reporting per-model results - admin UI: searchable multi-select with comma-separated paste support, self-test and verify-models buttons with a per-model report Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Warning Review limit reached
Next review available in: 25 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
WalkthroughManaged provider credentials now support optional model allowlists. The API validates and probes selected models, gateway routing filters credentials by model, and the admin interface supports model selection, self-tests, and per-model verification. ChangesModel eligibility contract
Credential lifecycle and validation API
Model-aware provider routing
Admin credential management interface
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant ProviderCredentialsManager
participant AdminAPI
participant validateProviderKey
Admin->>ProviderCredentialsManager: select models and enter credential data
ProviderCredentialsManager->>AdminAPI: submit self-test or model verification
AdminAPI->>validateProviderKey: probe selected catalog model
validateProviderKey-->>AdminAPI: return validation result
AdminAPI-->>ProviderCredentialsManager: return per-model results
ProviderCredentialsManager-->>Admin: display verification status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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 |
There was a problem hiding this comment.
Pull request overview
Adds an allowedModels restriction to provider keys/credentials so the gateway can avoid selecting credentials that cannot serve a requested model, and updates admin APIs/UI to configure and validate that restriction.
Changes:
- Adds
allowed_models text[]toprovider_keyplus a sharedproviderKeyAllowsModel()helper. - Enforces model restrictions during routing/credential selection (BYOK and managed keys), and updates provider availability computation to be model-aware.
- Extends the admin provider-credentials API + UI with allowed-model selection, self-test, and per-model verification endpoints.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/db/src/schema.ts | Adds allowedModels column to the provider_key table schema. |
| packages/db/src/provider-key-allowed-models.ts | Introduces providerKeyAllowsModel() helper (NULL/empty = unrestricted). |
| packages/db/src/provider-key-allowed-models.spec.ts | Unit tests for providerKeyAllowsModel() semantics. |
| packages/db/src/index.ts | Re-exports the new helper from @llmgateway/db. |
| packages/db/migrations/meta/_journal.json | Records the new migration in the Drizzle journal. |
| packages/db/migrations/1785867564_orange_proudstar.sql | Migration adding allowed_models column to provider_key. |
| packages/actions/src/validate-provider-key.ts | Adds pinned-model validation support for probing specific models. |
| ee/admin/src/lib/admin-provider-credentials.ts | Adds typed client helpers for self-test and verify-models endpoints; threads allowedModels through mutations. |
| ee/admin/src/components/provider-credentials-manager.tsx | UI: allowed-model picker, self-test + verify-models actions, and table column showing restriction. |
| ee/admin/src/app/provider-credentials/page.tsx | Wires new admin client functions into the page. |
| apps/gateway/src/lib/managed-provider-key.spec.ts | Tests managed credential filtering/advertising behavior with allowedModels. |
| apps/gateway/src/lib/cached-queries.ts | Adds optional model filtering to findManagedProviderIds() and carries allowedModels through row reads. |
| apps/gateway/src/chat/tools/resolve-provider-context.ts | Filters BYOK keys by model restriction and passes model to managed credential selection. |
| apps/gateway/src/chat/tools/resolve-platform-credential.ts | Adds model-aware managed credential filtering. |
| apps/gateway/src/chat/chat.ts | Computes provider availability per model and filters restricted keys/credentials across routing paths. |
| apps/api/src/routes/admin-provider-credentials.ts | Admin API: store/validate allowedModels; add self-test + verify-models endpoints; catalog now includes provider model ids. |
| apps/api/src/routes/admin-provider-credentials.spec.ts | Tests for allowedModels normalization/validation + new probe endpoints. |
Suppressed comments (2)
apps/api/src/routes/admin-provider-credentials.ts:1008
- createCredential validates allowedModels without passing the credential’s managedCredentialValidationOptions (region/config). This can validate against the wrong regional mapping for region-scoped providers, and it also means the allowedModels validation and the subsequent save-time probe can disagree about what’s “available”.
validateAllowedModels(body.provider, allowedModels);
apps/api/src/routes/admin-provider-credentials.ts:1129
- updateCredential validates allowedModels without passing the effective managedCredentialValidationOptions (taking into account any region/config changes in the same PATCH). For region-scoped providers this can validate against a different region than the credential will actually use.
validateAllowedModels(existing.provider, allowedModels);
updates.allowedModels = allowedModels;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+356
to
+372
| function validateAllowedModels( | ||
| provider: string, | ||
| allowedModels: string[] | null, | ||
| ): void { | ||
| if (!allowedModels) { | ||
| return; | ||
| } | ||
| const unknown = allowedModels.filter( | ||
| (modelId) => | ||
| getPinnedValidationModel(provider as ProviderId, modelId) === null, | ||
| ); | ||
| if (unknown.length > 0) { | ||
| throw new HTTPException(400, { | ||
| message: `Not available from ${provider} per the catalogue: ${unknown.join(", ")}`, | ||
| }); | ||
| } | ||
| } |
Member
Author
There was a problem hiding this comment.
Fixed in 792501b — validateAllowedModels now takes the credential's managedCredentialValidationOptions (config + region) at both create and update, with update using the config/region the PATCH leaves in effect, so the catalogue check resolves the same regional mapping the save-time probe does.
Comment on lines
+1289
to
+1293
| const token = body.token?.trim() | ||
| ? body.token | ||
| : credential | ||
| ? readProviderKey(credential) | ||
| : undefined; |
Member
Author
There was a problem hiding this comment.
Fixed in 792501b — the trimmed token is now what the probe and error redaction use, matching the presence check.
Comment on lines
+119
to
+121
| options.model !== undefined | ||
| ? (key) => providerKeyAllowsModel(key.allowedModels, options.model!) | ||
| : undefined, |
Member
Author
There was a problem hiding this comment.
Fixed in 792501b — the model is captured into a local const before building the filter, no non-null assertion.
![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: 4
🧹 Nitpick comments (3)
ee/admin/src/components/provider-credentials-manager.tsx (1)
1455-1512: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnounce the probe result to assistive technology.
The self-test and verify-models results appear only after the async probe finishes. A screen reader user gets no announcement, because the container is not a live region. Add
aria-liveto a wrapper that is present before the result arrives. Also mark the two buttons witharia-busywhile their probe runs.♻️ Proposed change
- {selfTestOutcome ? ( + <div aria-live="polite" className="flex flex-col gap-2"> + {selfTestOutcome ? ( selfTestOutcome.error || !selfTestOutcome.result?.valid ? (Close the wrapper after the verify-models block:
</div> ) : null} + </div> </div>🤖 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 `@ee/admin/src/components/provider-credentials-manager.tsx` around lines 1455 - 1512, Wrap the self-test and verifyOutcome result section in a wrapper rendered before either async result is available, and add aria-live so assistive technology announces probe updates. Update both probe-triggering buttons to set aria-busy while their respective self-test or model-verification operation is running, preserving the existing result content and behavior.packages/actions/src/validate-provider-key.ts (1)
176-190: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared region and mapping resolution.
This block now exists three times in this file:
getValidationModel(Lines 52-60),getPinnedValidationModel(Lines 176-183), andvalidateProviderKey(Lines 298-305). The mapping lookup with region fallback is also duplicated at Lines 185-190 and Lines 331-337. Two helpers would remove all six copies and keep the region-fallback rule in one place.As per coding guidelines: "Apply DRY principles for code reuse, except that explicit model/provider mapping duplication is preferred in
packages/models" — this file is inpackages/actions, so the exemption does not apply.♻️ Proposed extraction
+function resolveSelectedRegion( + provider: ProviderId, + providerKeyOptions?: ProviderKeyOptions, +): string | undefined { + const providerDef = providers.find((p) => p.id === provider) as + ProviderDefinition | undefined; + const regionKey = providerDef?.regionConfig?.optionsKey; + return regionKey + ? ((providerKeyOptions as Record<string, string | undefined> | undefined)?.[ + regionKey + ] ?? providerDef?.regionConfig?.defaultRegion) + : undefined; +} + +function findProviderMapping( + modelDef: { providers: readonly unknown[] }, + provider: ProviderId, + region: string | undefined, +): ProviderModelMapping | undefined { + const mappings = modelDef.providers as readonly ProviderModelMapping[]; + return ( + mappings.find( + (p) => + p.providerId === provider && (p.region ?? null) === (region ?? null), + ) ?? mappings.find((p) => p.providerId === provider) + ); +}Then in
getPinnedValidationModel:- const providerDef = providers.find((p) => p.id === provider) as - ProviderDefinition | undefined; - const regionKey = providerDef?.regionConfig?.optionsKey; - const selectedRegion = regionKey - ? ((providerKeyOptions as Record<string, string | undefined> | undefined)?.[ - regionKey - ] ?? providerDef?.regionConfig?.defaultRegion) - : undefined; - - const mapping = (modelDef.providers.find( - (p) => - p.providerId === provider && - ((p as ProviderModelMapping).region ?? null) === (selectedRegion ?? null), - ) ?? modelDef.providers.find((p) => p.providerId === provider)) as - ProviderModelMapping | undefined; + const selectedRegion = resolveSelectedRegion(provider, providerKeyOptions); + const mapping = findProviderMapping(modelDef, provider, selectedRegion); if (!mapping) { return null; }🤖 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/actions/src/validate-provider-key.ts` around lines 176 - 190, Extract the repeated region resolution and provider-model mapping logic from getValidationModel, getPinnedValidationModel, and validateProviderKey into shared helpers in this file. Have the region helper resolve the configured option or default region, and have the mapping helper perform the region-specific lookup with fallback to the provider-only mapping; replace all six duplicated blocks while preserving their current behavior.Source: Coding guidelines
apps/api/src/routes/admin-provider-credentials.spec.ts (1)
1326-1386: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the pinned-model probe selection.
These tests cover normalization, catalogue rejection, and persistence. They cannot reach the new branch in
validateCredentialToken(Lines 450-463 ofapps/api/src/routes/admin-provider-credentials.ts), becauseisCredentialTestEnv()returns at Line 440 before the pinning logic runs. So the two decisions that the restriction exists for are unexercised:
- A restricted credential probes an allowed model, not the provider's default validation model.
- A restriction whose models are all non-chat-capable skips the probe instead of failing the save.
Mock
validateProviderKeyand assert thepinnedModelIdargument to cover both.🤖 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/admin-provider-credentials.spec.ts` around lines 1326 - 1386, Extend the provider-credential tests around validateCredentialToken to mock validateProviderKey and bypass the isCredentialTestEnv early return. Add coverage confirming restricted credentials pass an allowed chat-capable model as pinnedModelId instead of the default validation model, and that restrictions containing only non-chat-capable models skip probing rather than failing creation. Assert the mock calls and resulting save behavior for both cases.
🤖 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/api/src/routes/admin-provider-credentials.ts`:
- Line 1436: In the verification handler around normalizeAllowedModels, reject
an empty normalized modelIds array with a 400 response before evaluating results
or allValid. Apply the same guard to the corresponding flow around the
referenced later section, while preserving normal verification for non-empty
model lists.
In `@apps/gateway/src/chat/tools/resolve-platform-credential.ts`:
- Around line 77-82: Make the model parameter required in the platform
credential resolution API and update both chat credential call sites to pass the
selected model alongside selectionScope. In the managed credential filtering
logic, always apply providerKeyAllowsModel using model, removing the conditional
path that bypasses this check when model is omitted.
In `@apps/gateway/src/lib/managed-provider-key.spec.ts`:
- Around line 357-359: Replace the dynamic imports in
apps/gateway/src/lib/managed-provider-key.spec.ts at lines 357-359 and 385-387
with a shared module-scope resolvePlatformCredential import, and replace the
dynamic import at lines 421-422 with a module-scope providerKeyAllowsModel
import; update each test to use those top-level bindings.
In `@ee/admin/src/components/provider-credentials-manager.tsx`:
- Around line 1059-1084: Update handleSelfTest and handleVerifyModels to wrap
each awaited server action in try/catch/finally, preserving the existing outcome
handling for successful and failed results while ensuring
setSelfTestLoading(false) and setVerifyLoading(false) always execute in finally,
including when the invocation rejects.
---
Nitpick comments:
In `@apps/api/src/routes/admin-provider-credentials.spec.ts`:
- Around line 1326-1386: Extend the provider-credential tests around
validateCredentialToken to mock validateProviderKey and bypass the
isCredentialTestEnv early return. Add coverage confirming restricted credentials
pass an allowed chat-capable model as pinnedModelId instead of the default
validation model, and that restrictions containing only non-chat-capable models
skip probing rather than failing creation. Assert the mock calls and resulting
save behavior for both cases.
In `@ee/admin/src/components/provider-credentials-manager.tsx`:
- Around line 1455-1512: Wrap the self-test and verifyOutcome result section in
a wrapper rendered before either async result is available, and add aria-live so
assistive technology announces probe updates. Update both probe-triggering
buttons to set aria-busy while their respective self-test or model-verification
operation is running, preserving the existing result content and behavior.
In `@packages/actions/src/validate-provider-key.ts`:
- Around line 176-190: Extract the repeated region resolution and provider-model
mapping logic from getValidationModel, getPinnedValidationModel, and
validateProviderKey into shared helpers in this file. Have the region helper
resolve the configured option or default region, and have the mapping helper
perform the region-specific lookup with fallback to the provider-only mapping;
replace all six duplicated blocks while preserving their current behavior.
🪄 Autofix
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 Plus
Run ID: 040ba48f-cad9-4702-b1ca-a7fdab64020f
📒 Files selected for processing (18)
apps/api/src/routes/admin-provider-credentials.spec.tsapps/api/src/routes/admin-provider-credentials.tsapps/gateway/src/chat/chat.tsapps/gateway/src/chat/tools/resolve-platform-credential.tsapps/gateway/src/chat/tools/resolve-provider-context.tsapps/gateway/src/lib/cached-queries.tsapps/gateway/src/lib/managed-provider-key.spec.tsee/admin/src/app/provider-credentials/page.tsxee/admin/src/components/provider-credentials-manager.tsxee/admin/src/lib/admin-provider-credentials.tspackages/actions/src/validate-provider-key.tspackages/db/migrations/1785867564_orange_proudstar.sqlpackages/db/migrations/meta/1785867564_snapshot.jsonpackages/db/migrations/meta/_journal.jsonpackages/db/src/index.tspackages/db/src/provider-key-allowed-models.spec.tspackages/db/src/provider-key-allowed-models.tspackages/db/src/schema.ts
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lowlist # Conflicts: # apps/api/src/routes/admin-provider-credentials.ts
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- pass model to the two chat retry-path resolvePlatformCredential calls so alternate-key fallback also honors allowedModels; document why the option stays optional (non-chat endpoints only hold upstream ids) - validate allowedModels against the credential's config/region so region-scoped credentials check the mapping they will actually use - reject whitespace-only verify-models lists instead of allValid: true - use the trimmed token for test probes, matching the presence check - reset probe loading flags in finally so a rejected server action cannot wedge both buttons - aria-live region + aria-busy for the async probe results - dedupe region/mapping resolution in validate-provider-key.ts - top-level imports in the managed-provider-key spec - cover the pinned-model probe selection with a mocked live path - drop an unused map param that failed lint on main Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces the admin dialog's hand-rolled AllowedModelsPicker with the shared MultiModelSelector, fed with catalogue definitions filtered to the models the catalog endpoint reports live on the provider. The comma-separated paste support moves into the shared component so every consumer gets it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The model-filtered lookups reuse the existing swrWrap + Drizzle Redis cache paths (narrowing happens in memory after the cached row fetch), so a Postgres outage keeps serving them from the mirror. These tests pin that down for findManagedProviderIds and the filtered BYOK selection, including the text[] surviving the JSON round-trip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MultiModelSelector renders mapping-derived detail and needs full catalogue definitions, which is the wrong shape here — the admin deals in plain root model ids. Adds a lean shared MultiModelIdSelector (search, comma-separated paste, flagged unknown ids) and reverts the paste handler bolted onto MultiModelSelector. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
steebchen
enabled auto-merge
August 7, 2026 15:52
steebchen
disabled auto-merge
August 7, 2026 15:57
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.
Problem
Some upstream accounts only have a subset of a provider's catalogue enabled (e.g. a provider account that carries just a few models). The gateway had no way to know this, so auto-routing and fallback routing could pick a provider + credential that cannot serve the requested model, and the request would fail upstream instead of routing to a credential that works. Save-time credential validation also always probed the provider's default validation model — which is exactly the model such an account may not have — forcing admins to skip validation on the very credentials that needed it.
Approach
New nullable
allowed_models(text[]) column onprovider_key, holding canonical LLM Gateway model ids.NULL/empty means unrestricted; the sharedproviderKeyAllowsModel()helper (in@llmgateway/db) is the single comparison point.Gateway enforcement (applies to both BYOK keys and managed credentials):
resolveProviderContextfilters BYOK keys through the restriction (composed with the existing service-tier filter), so a restricted key is skipped in favor of a sibling key or — in hybrid mode — the credits fallback.resolvePlatformCredentialtakes a newmodeloption and skips managed credentials that exclude it, falling through to the next credential or the env vars. Filtering runs before variant narrowing, mirroring the existing filter semantics.findManagedProviderIdsaccepts an optionalmodelId, so a provider whose every managed credential excludes the model is not advertised for it (unless an env key can still serve it).chat.ts(auto-routing candidate loop, unpinned routing, rate-limit fallback, low-uptime fallback) now computeavailableProviders/providersWithKeysper model with restricted keys filtered out. For the auto loop this moved the availability computation inside the per-candidate loop; the extrafindManagedProviderIdscalls are served from the SWR cache.Admin API (
/admin/provider-credentials):allowedModelson create/update: trimmed, deduped, validated against the catalogue (every id must have a live mapping for the provider); an emptied list storesNULL.skipValidation.POST .../self-test: runs the save-time probe standalone against a stored credential or unsaved dialog values, reporting the outcome instead of gating a write.POST .../verify-models: probes each listed model through the credential (batches of 5, capped at 50) and returns a per-model report —inCatalog, livevalid/null(non-chat models can't be probed), status code and redacted upstream error.Admin UI (provider credentials dialog):
provider/model-prefixed ids) adds all entries at once, with unknown ids flagged inline.Notes for review
NULL, and a key that can serve nothing is only ever a misconfiguration.ALTER TABLE ... ADD COLUMN, generated viapnpm migrations.Verification
pnpm build— all 17 tasks pass.test:unitsemantics): 130 files / 2703 tests pass, including new specs for the helper,findManagedProviderIds/resolvePlatformCredentialmodel filtering, BYOK filtering, and the admin CRUD + self-test + verify-models endpoints.Screenshots
Credentials table — the new Models column: the managed credential is restricted to 3 models while the env key serves the full catalogue:
Model picker — searchable multi-select over the provider's catalogue (comma-separated paste fills it in one go):
Self-test + verify report — both probes run live through the key against the real upstream; every listed model verified green:
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes