-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix: include API key in Ollama /api/tags requests #7903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Updated GetModelsOptions type to include optional apiKey for ollama provider - Modified getOllamaModels function to accept and use apiKey parameter in Authorization headers - Updated all callers (webviewMessageHandler, modelCache, native-ollama) to pass apiKey - Added test coverage for API key authentication Fixes #7902
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code because apparently I trust no one, not even myself.
Review Summary
The implementation correctly addresses issue #7902 by adding API key support to Ollama requests. The changes are clean and follow existing patterns.
Strengths:
- ✅ API key is properly included in Authorization headers for both /api/tags and /api/show endpoints
- ✅ All callers updated to pass the API key when available
- ✅ Type definitions correctly updated
- ✅ Comprehensive test coverage for the new functionality
Suggestions for improvement:
-
Error handling - Consider enhancing error handling in
getOllamaModels()to distinguish between authentication failures (401/403) and connection errors. This would help users understand if their API key is invalid vs if Ollama is simply unreachable. -
Native client compatibility - In
native-ollama.ts, verify that the ollama npm package actually supports and passes custom headers. The package documentation doesn't explicitly mention header support in the Config type. -
Test coverage - Consider adding tests for authentication failure scenarios to ensure proper error handling.
Overall, this is a solid implementation that solves the reported issue effectively.
daniel-lxs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR fixes an issue where the Ollama API key was not being included in the Authorization header when making requests to
/api/tagsand/api/showendpoints.Problem
When using a custom Ollama URL with an API key configured, the API key was not being sent in the Authorization header for requests to fetch available models. This prevented authentication with Ollama cloud or other authenticated Ollama instances.
Solution
GetModelsOptionstype to include an optionalapiKeyfield for the ollama providergetOllamaModelsfunction to accept and use the API key parameterwebviewMessageHandlerfor UI model fetchingmodelCachefor cached model fetchingnative-ollamaprovider for model fetchingTesting
Fixes #7902
Important
Fixes missing API key in Ollama requests by updating
getOllamaModels()to include Authorization header when API key is provided./api/tagsand/api/showrequests ingetOllamaModels().GetModelsOptionsto include optionalapiKeyfor Ollama.webviewMessageHandlerandnative-ollamato pass API key.getOllamaModels()inollama.tsnow acceptsapiKeyand includes it in request headers.fetchModel()innative-ollama.tsto use API key.ollama.test.tsto verify Authorization header inclusion when API key is provided.This description was created by
for 83ccedf. You can customize this summary. It will automatically update as commits are pushed.