Skip to content

fix(list_models): respect API key profile filtering in full model data mode#1480

Merged
looplj merged 2 commits into
looplj:unstablefrom
djdembeck:fix/list-models-api-key-profile-filtering
Apr 25, 2026
Merged

fix(list_models): respect API key profile filtering in full model data mode#1480
looplj merged 2 commits into
looplj:unstablefrom
djdembeck:fix/list-models-api-key-profile-filtering

Conversation

@djdembeck

Copy link
Copy Markdown
Contributor

Purpose/Goal

Fix list models API key profile filtering when full model info setting is enabled.

Summary

When include=full or DefaultModelAPIIncludeAll=true, the list_models endpoint was querying all enabled models from the database without respecting API key profile restrictions. This change ensures that API key profile visibility filtering is applied consistently in both basic and extended (full data) modes.

What Changed

  • internal/server/api/openai.go: Refactored ListModels handler to first fetch visible models via ModelService (which respects API key profiles), then conditionally enrich with database data based on needFullData flag
  • internal/server/api/openai_retrieve_test.go: Added comprehensive test coverage:
    • TestOpenAIHandlers_ListModels_ExtendedModeRespectsAPIKeyProfile: Verifies extended mode only returns models the API key has access to
    • TestOpenAIHandlers_ListModels_ExtendedModeFallsBackToBasicForMissingDBModel: Verifies graceful fallback for channel-only models
    • TestOpenAIHandlers_ListModels_ExtendedModeWithZeroAllowedModelsReturnsEmpty: Verifies empty response when API key has no matching models

Why This Matters

API key profile restrictions were being bypassed when users requested full model metadata. This fix ensures authorization boundaries are maintained regardless of the include parameter or server-side default settings.

Spirit/Intent

Purpose/Goal: Maintain security invariants while providing rich model metadata. The fix preserves the existing authorization model (API key profiles) while enabling the extended model data feature. Users with restricted API keys should only see models they're authorized to access, whether in basic or extended mode.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the ListModels endpoint in openai.go to ensure that model visibility is correctly handled by fetching enabled models through the ModelService before querying the database for extended metadata. It also adds comprehensive tests to verify that the extended mode respects API key profiles and handles missing database entries gracefully. Feedback includes suggestions to optimize the logic with an early return for empty model lists and to use the lo library for more idiomatic collection transformations.

Comment thread internal/server/api/openai.go
Comment thread internal/server/api/openai.go Outdated
Comment thread internal/server/api/openai.go Outdated
…ta query

When include=full is enabled, the ListModels endpoint was querying all
enabled models from the database without filtering by API key profile
visibility. This caused models the caller shouldn't see to be returned.

Fix by fetching visible models first (with profile filtering), then using
modelIDIn to query DB only for those IDs, merging extended data where
available and falling back to basic fields for profile-excluded models.
@djdembeck djdembeck force-pushed the fix/list-models-api-key-profile-filtering branch from 327ab4d to e2dd778 Compare April 24, 2026 21:40
Refactor ListModels to add early return for empty models and simplify
loops with lo.Map per code review feedback.

- Add early return when visibleModels is empty
- Replace manual for-append loops with lo.Map
- Minor alignment fix on ent.Query
@djdembeck djdembeck marked this pull request as ready for review April 24, 2026 21:53

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@djdembeck

Copy link
Copy Markdown
Contributor Author

confirmed working on my instance.

@looplj looplj merged commit c6f128a into looplj:unstable Apr 25, 2026
4 checks passed
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