fix(web/tavily): authenticate via Authorization header, config-driven - #66599
fix(web/tavily): authenticate via Authorization header, config-driven#66599zivisaiah wants to merge 1 commit into
Conversation
Related to merged #24658: that was the older |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
Critical
- None
Warnings
- None
Assessment
Fixes Tavily authentication to use Authorization: Bearer header (the documented method) with a config-driven fallback to legacy in-body api_key field. Header auth is the correct default as it allows upstream credential proxies to rewrite headers.
Looks Good
- New
TAVILY_AUTH_STYLEconfig option with documented values - Clean conditional:
headervsbodyauth styles - Minimal diff, focused fix with no side effects
- Good inline documentation
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting the current Tavily plugin rather than the removed inline helper; plugins/web/tavily/provider.py:54-59 confirms main still sends api_key in the JSON body.
Problems
- The new
TAVILY_AUTH_STYLEat PR diff right line 61 is a non-secret behavioral environment variable.AGENTS.md:102-107requires this class of setting to useconfig.yamlrather than.env. - The proxy-only claim is incomplete:
plugins/web/tavily/provider.py:46-51still rejects an unsetTAVILY_API_KEY, and Tavily selection checks it attools/web_tools.py:242,334-335. - The PR changes no tests. Existing coverage asserts body auth in
tests/tools/test_web_tools_tavily.py:32-48; add mocked assertions for default header auth, no body key, and any retained compatibility path.
Suggested changes
- Use a documented
webconfig setting for legacy selection, or omit the legacy switch. - Define and test the proxy-only credential contract.
Automated hermes-sweeper review.
| ) | ||
|
|
||
| base_url = get_provider_env("TAVILY_BASE_URL") or "https://api.tavily.com" | ||
| auth_style = (get_provider_env("TAVILY_AUTH_STYLE") or "header").strip().lower() |
There was a problem hiding this comment.
TAVILY_AUTH_STYLE is a user-facing, non-secret behavior switch. Please move any retained legacy-auth selection to a documented web config.yaml setting (or remove the compatibility mode); AGENTS.md:102-107 reserves .env for credentials.
SummaryThree open PRs modify the Tavily provider but address distinct causes: #32798 changes search-request defaults, #66599 changes API-key transport, and #67202 adds proxy routing. The complete diffs show adjacent changes rather than competing implementations. Related pull requests
Suggested consolidationKeep #32798, #66599, and #67202 open with the distinct salvage paths identified by their contributor reviews. Their complete diffs address request defaults, authentication transport, and proxy routing respectively, so none is supported as a duplicate; author action is required on each remaining review concern. Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 7 kB of PR diffs, 5 kB of issue/PR text, 4 kB of discussion (5 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
What
The Tavily web provider sends the API key inside the JSON request body (
payload["api_key"]). This change sends it as anAuthorization: Bearer <key>header instead, and adds aTAVILY_AUTH_STYLEenv var (headerdefault,bodyfor the legacy path) so nothing breaks for anyone relying on the old behavior.Why
Matches Tavily's current docs. Tavily's API reference now documents Bearer header auth as the standard method (
Authorization: Bearer tvly-...). The in-bodyapi_keyfield is legacy.Works behind a credential proxy. Some deployments route agent egress through a credential-injecting proxy that attaches the real key at the network edge so the bot never holds it. A proxy can rewrite request headers but not JSON request bodies, so body-only auth cannot be injected.
Precedence matters, and body currently wins. I verified empirically against the live Tavily API: when a request carries both a placeholder
api_keyin the body and a valid key in theAuthorizationheader, Tavily uses the body value and returns401 Unauthorized. Header-only auth returns200. So simply adding a header is not enough, the body key has to be dropped, which is what this patch does by default.Changes
plugins/web/tavily/provider.py:_tavily_requestsends the key as a Bearer header by default, or in the body whenTAVILY_AUTH_STYLE=body. Backward compatible.Testing
Header auth verified end to end against
api.tavily.com(search returns real results). Legacybodymode preserves the prior request shape.