Skip to content

Apply the Bearer token guard to AsyncClient.web_search and web_fetch - #705

Open
ckarnell wants to merge 1 commit into
ollama:mainfrom
ckarnell:fix/async-web-search-auth-guard
Open

Apply the Bearer token guard to AsyncClient.web_search and web_fetch#705
ckarnell wants to merge 1 commit into
ollama:mainfrom
ckarnell:fix/async-web-search-auth-guard

Conversation

@ckarnell

Copy link
Copy Markdown

Client.web_search and Client.web_fetch both check for a Bearer token before doing anything:

if not self._client.headers.get('authorization', '').startswith('Bearer '):
  raise ValueError('Authorization header with Bearer token is required for web search')

The AsyncClient versions of both methods have no such check. An async caller with no
OLLAMA_API_KEY set does not get that error. The request goes out to
https://ollama.com/api/web_search unauthenticated and comes back as a 401, so the same mistake
produces a clear local message on one client and a round trip plus a generic server error on the
other.

This looks like an oversight. The sync docstrings carry a Raises: section
for exactly this and the async ones do not, and tests/test_client.py has
test_client_web_search_requires_bearer_auth_header and the web_fetch twin covering the sync
path with no async equivalent, in a file that otherwise has 26 async tests.

To reproduce, with OLLAMA_API_KEY unset:

Client().web_search('test')        # ValueError, no request sent
await AsyncClient().web_search('test')   # request goes out, ResponseError 401

This adds the same guard and the matching Raises: docstring line to both async methods, and the
two async tests mirroring the existing sync ones. Both new tests fail without the change and pass
with it. Full suite is 95 passing, ruff check and ruff format --check both clean.

A note on severity. Not a security hole. Both clients talk to the same host, so no credential
goes anywhere new. The cost is a worse error message and a wasted round trip.

I did not check whether any other AsyncClient method has a sync-only guard. I only looked
at the two that hit the hosted endpoints.

Client.web_search and Client.web_fetch raise ValueError when no Bearer token is
configured. The AsyncClient versions had no such check, so an unconfigured async
caller sent an unauthenticated request to ollama.com and got a 401 back instead
of failing locally with an actionable message.

Adds the same guard and the matching Raises: docstring section to both async
methods, plus the two async tests that mirror the existing sync ones.
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.

1 participant