fix(tavily): fall back to process.env when config SecretRef is unresolvable#95110
fix(tavily): fall back to process.env when config SecretRef is unresolvable#95110kzzalews wants to merge 1 commit into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 15, 2026, 6:47 AM ET / 10:47 UTC. Summary PR surface: Source +47, Tests +202. Total +249 across 2 files. Reproducibility: yes. at high confidence from source and supplied runtime evidence: current main throws during strict Tavily SecretRef normalization before the environment fallback, and the report includes repeated failure logs plus an exact-head after-fix provider call. Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land one canonical guarded implementation that preserves file, exec, wrong-id, and provider-policy boundaries; prefer #97827 if maintainers want the related agent Tool Search snapshot fix in the same landing, otherwise land this focused branch and retain the agent-runtime repair as a separate PR. Do we have a high-confidence way to reproduce the issue? Yes, at high confidence from source and supplied runtime evidence: current main throws during strict Tavily SecretRef normalization before the environment fallback, and the report includes repeated failure logs plus an exact-head after-fix provider call. Is this the best way to solve the issue? No, not unconditionally: this is a correct focused fix, but #97827 is the stronger overall solution if maintainers want the related agent Tool Search runtime-snapshot cause fixed in the same canonical branch. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 026b6881619e. Label changesLabel justifications:
Evidence reviewedPR surface: Source +47, Tests +202. Total +249 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (1 earlier review cycle)
|
Replaces the previous `try/catch` approach with the inspect-mode pattern already used in `extensions/firecrawl/src/config.ts`: 1. Calls `resolveSecretInputString` with `mode: "inspect"` so unresolved SecretRefs return `configured_unavailable` instead of throwing. 2. Treats file/exec SecretRefs as `blocked` — never falls through to ambient `process.env.TAVILY_API_KEY`. 3. Treats env SecretRefs whose `id` is not `TAVILY_API_KEY` as `blocked` — same reason. 4. Validates the env provider allowlist via `canResolveEnvSecretRefInReadOnlyPath` before reading `process.env.TAVILY_API_KEY`. 5. `resolveTavilyApiKey` only falls through to `process.env.TAVILY_API_KEY` when the configured SecretRef is `missing` (no apiKey configured) — never when it is `blocked`. This addresses the ClawSweeper review feedback on openclaw#95110: the previous `try/catch` catch-all could silently route Tavily requests through ambient `TAVILY_API_KEY` when the user configured a different explicit SecretRef source (file, exec, or env with a different id), weakening the intended credential boundary. The new behavior matches the production pattern that brave/firecrawl/exa already use (firecrawl most recently) and adds explicit coverage for the silent-override scenario. No public API change, no config schema change, no SDK contract change. Repro before fix: $ TAVILY_API_KEY=*** openclaw infer web search --provider tavily --query ping --limit 1 --json # Error: plugins.entries.tavily.config.webSearch.apiKey: unresolved SecretRef "env:default:TAVILY_API_KEY" After fix: same command reaches Tavily and returns 401 (auth) instead of crashing on the SecretRef boundary. Co-authored-by: Aiko <openclaw@workspace>
266bc99 to
f061b25
Compare
|
Quick note: there is a related PR in the same space — #95112 by @liuhao1024. It uses I have updated this PR (#95110) to v2 to use the same inspect-mode approach, plus the allowlist gating that the v1 review (and the #95112 review) flagged. The two PRs now share the inspect-mode core; the difference is the allowlist boundary. If maintainers prefer #95112, the inspect-mode logic there is clean and can be combined with the allowlist gating from this PR (or vice versa). I am happy to close this PR in favor of #95112 if that is the preferred direction — but I would like to keep #95109 (the issue) open either way, since the underlying bug is real regardless of which PR lands. |
f061b25 to
7d10d5b
Compare
Replaces the previous `try/catch` approach with the inspect-mode pattern already used in `extensions/firecrawl/src/config.ts`: 1. Calls `resolveSecretInputString` with `mode: "inspect"` so unresolved SecretRefs return `configured_unavailable` instead of throwing. 2. Treats file/exec SecretRefs as `blocked` — never falls through to ambient `process.env.TAVILY_API_KEY`. 3. Treats env SecretRefs whose `id` is not `TAVILY_API_KEY` as `blocked` — same reason. 4. Validates the env provider allowlist via `canResolveEnvSecretRefInReadOnlyPath` before reading `process.env.TAVILY_API_KEY`. 5. `resolveTavilyApiKey` only falls through to `process.env.TAVILY_API_KEY` when the configured SecretRef is `missing` (no apiKey configured) — never when it is `blocked`. This addresses the ClawSweeper review feedback on openclaw#95110: the previous `try/catch` catch-all could silently route Tavily requests through ambient `TAVILY_API_KEY` when the user configured a different explicit SecretRef source (file, exec, or env with a different id), weakening the intended credential boundary. The new behavior matches the production pattern that brave/firecrawl/exa already use (firecrawl most recently) and adds explicit coverage for the silent-override scenario. No public API change, no config schema change, no SDK contract change. Repro before fix: $ TAVILY_API_KEY=*** openclaw infer web search --provider tavily --query ping --limit 1 --json # Error: plugins.entries.tavily.config.webSearch.apiKey: unresolved SecretRef "env:default:TAVILY_API_KEY" After fix: same command reaches Tavily and returns 401 (auth) instead of crashing on the SecretRef boundary. Co-authored-by: Aiko <openclaw@workspace>
Rebase update — CI fix in progress@vincentkoc — quick context on the latest push (force-pushed to Root cause of the previous CI failure: What I did:
Current status:
No code changes — the actual fix is identical to what was in the v2 commits ( |
6e4a691 to
8b5cc2c
Compare
Replaces the previous `try/catch` approach with the inspect-mode pattern already used in `extensions/firecrawl/src/config.ts`: 1. Calls `resolveSecretInputString` with `mode: "inspect"` so unresolved SecretRefs return `configured_unavailable` instead of throwing. 2. Treats file/exec SecretRefs as `blocked` — never falls through to ambient `process.env.TAVILY_API_KEY`. 3. Treats env SecretRefs whose `id` is not `TAVILY_API_KEY` as `blocked` — same reason. 4. Validates the env provider allowlist via `canResolveEnvSecretRefInReadOnlyPath` before reading `process.env.TAVILY_API_KEY`. 5. `resolveTavilyApiKey` only falls through to `process.env.TAVILY_API_KEY` when the configured SecretRef is `missing` (no apiKey configured) — never when it is `blocked`. This addresses the ClawSweeper review feedback on openclaw#95110: the previous `try/catch` catch-all could silently route Tavily requests through ambient `TAVILY_API_KEY` when the user configured a different explicit SecretRef source (file, exec, or env with a different id), weakening the intended credential boundary. The new behavior matches the production pattern that brave/firecrawl/exa already use (firecrawl most recently) and adds explicit coverage for the silent-override scenario. No public API change, no config schema change, no SDK contract change. Repro before fix: $ TAVILY_API_KEY=*** openclaw infer web search --provider tavily --query ping --limit 1 --json # Error: plugins.entries.tavily.config.webSearch.apiKey: unresolved SecretRef "env:default:TAVILY_API_KEY" After fix: same command reaches Tavily and returns 401 (auth) instead of crashing on the SecretRef boundary. Co-authored-by: Aiko <openclaw@workspace>
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Updated since last review:
|
|
@clawsweeper re-review Updated since last review:
The new proof shows three things:
This directly addresses the ClawSweeper [P1] finding: "lacks redacted exact-head Gateway/Tavily output showing this branch reaches auth failure or real results." |
3a9963a to
c6ac9a5
Compare
Replaces the previous `try/catch` approach with the inspect-mode pattern already used in `extensions/firecrawl/src/config.ts`: 1. Calls `resolveSecretInputString` with `mode: "inspect"` so unresolved SecretRefs return `configured_unavailable` instead of throwing. 2. Treats file/exec SecretRefs as `blocked` — never falls through to ambient `process.env.TAVILY_API_KEY`. 3. Treats env SecretRefs whose `id` is not `TAVILY_API_KEY` as `blocked` — same reason. 4. Validates the env provider allowlist via `canResolveEnvSecretRefInReadOnlyPath` before reading `process.env.TAVILY_API_KEY`. 5. `resolveTavilyApiKey` only falls through to `process.env.TAVILY_API_KEY` when the configured SecretRef is `missing` (no apiKey configured) — never when it is `blocked`. This addresses the ClawSweeper review feedback on openclaw#95110: the previous `try/catch` catch-all could silently route Tavily requests through ambient `TAVILY_API_KEY` when the user configured a different explicit SecretRef source (file, exec, or env with a different id), weakening the intended credential boundary. The new behavior matches the production pattern that brave/firecrawl/exa already use (firecrawl most recently) and adds explicit coverage for the silent-override scenario. No public API change, no config schema change, no SDK contract change. Repro before fix: $ TAVILY_API_KEY=*** openclaw infer web search --provider tavily --query ping --limit 1 --json # Error: plugins.entries.tavily.config.webSearch.apiKey: unresolved SecretRef "env:default:TAVILY_API_KEY" After fix: same command reaches Tavily and returns 401 (auth) instead of crashing on the SecretRef boundary. Co-authored-by: Aiko <openclaw@workspace>
c6ac9a5 to
685dc04
Compare
Replaces the previous `try/catch` approach with the inspect-mode pattern already used in `extensions/firecrawl/src/config.ts`: 1. Calls `resolveSecretInputString` with `mode: "inspect"` so unresolved SecretRefs return `configured_unavailable` instead of throwing. 2. Treats file/exec SecretRefs as `blocked` — never falls through to ambient `process.env.TAVILY_API_KEY`. 3. Treats env SecretRefs whose `id` is not `TAVILY_API_KEY` as `blocked` — same reason. 4. Validates the env provider allowlist via `canResolveEnvSecretRefInReadOnlyPath` before reading `process.env.TAVILY_API_KEY`. 5. `resolveTavilyApiKey` only falls through to `process.env.TAVILY_API_KEY` when the configured SecretRef is `missing` (no apiKey configured) — never when it is `blocked`. This addresses the ClawSweeper review feedback on openclaw#95110: the previous `try/catch` catch-all could silently route Tavily requests through ambient `TAVILY_API_KEY` when the user configured a different explicit SecretRef source (file, exec, or env with a different id), weakening the intended credential boundary. The new behavior matches the production pattern that brave/firecrawl/exa already use (firecrawl most recently) and adds explicit coverage for the silent-override scenario. No public API change, no config schema change, no SDK contract change. Repro before fix: $ TAVILY_API_KEY=*** openclaw infer web search --provider tavily --query ping --limit 1 --json # Error: plugins.entries.tavily.config.webSearch.apiKey: unresolved SecretRef "env:default:TAVILY_API_KEY" After fix: same command reaches Tavily and returns 401 (auth) instead of crashing on the SecretRef boundary. Co-authored-by: Aiko <openclaw@workspace>
bac589f to
f15f86c
Compare
…lvable Replaces the try/catch SecretRef resolution with an inspect-mode pattern matching extensions/firecrawl. When the configured apiKey SecretRef cannot be resolved at runtime (e.g. env-backed credential not inlined in the snapshot), the resolver now returns 'blocked' instead of throwing, letting the ambient process.env.TAVILY_API_KEY fallback work as intended. Adds comprehensive unit tests covering resolveConfiguredSecret inspect-mode behavior, env allowlist, whitespace edge cases, and end-to-end apiKey resolution.
f15f86c to
62f06e8
Compare
|
Superseded by #97827, landed on The landed implementation includes this Tavily inspect-mode credential boundary and its focused regressions, plus the missing generic agent-runtime fix: it selects the active resolved runtime snapshot before applying the lean Tool Search overlay. That covers both the plugin-local fallback reported here and the agent-visible path from canonical issue #95109. Focused Testbox proof passed 23/23 tests; live Gateway -> OpenAI gpt-5.5 -> Tool Search -> Tavily proof reached the Tavily provider boundary without Thank you @kzzalews for the detailed reproduction and exact-head proof. |
What Problem This Solves
tavily_searchandtavily_extractfail at runtime withunresolved SecretRef "env:default:TAVILY_API_KEY"even whenTAVILY_API_KEYis set in the gateway process environment. The configuredSecretRefpoints at an env-backed credential that the active runtime snapshot can't inline-resolve, but the plugin's own call chain already has aprocess.env.TAVILY_API_KEYfallback — the helper in strict mode throws before that fallback can run. Bundled peers (brave,firecrawl,exa) usereadConfiguredSecretStringfromextensions/web-search-provider-commonand don't hit this; tavily and exa are the only remainingextensions/*/src/config.tshelpers still throwing in strict mode.Evidence
Environment: Linux, Node v24, OpenClaw
v2026.6.8(commit8a24f93), with the patch applied viaprestart.shstep 7/7 in the contributor's runtime overlay.Reproduction (before fix):
After this patch: Same command reaches Tavily. With a dummy key, the expected downstream error is
401 Unauthorized: missing or invalid API key(auth failure, not SecretRef failure). With a live key, the call returns search results.Real behavior proof — captured runtime logs (2026-06-21)
Real LLM-driven MCP tool calls captured from the contributor's gateway log
on 2026-06-21, when an agent tried a research query about Tailscale and
the MCP
tavily_search/tavily_extracttools failed for the samereason. These are the exact log lines:
Same symptom, four independent calls within a 13-second window — the
fallback to
process.env.TAVILY_API_KEYwas being skipped because theresolver threw in strict mode. With this PR applied on the same runtime,
the same agent's research turn completed and the Tailscale sources were
ingested normally.
Current-installation runtime proof (2026-06-27, this very instance)
The contributor's active OpenClaw container is running OpenClaw
v2026.6.10with the bundled
tavily-plugin v2026.6.8patched in-place byprestart.shstep 7/7 (so the runtime resolver carries the sameinspect-mode + allowlist behavior this PR introduces for upstream). To
confirm the patched Tavily plugin path is reachable from this very
instance, Aiko (the contributor's assistant) invoked the MCP
tavily_searchtool on 2026-06-27T01:36:30Z with the query
OpenClaw 2026.6.10 release notes tavily plugin. The call returned three real Tavily results in1213 ms with no
UnresolvedSecretInputErrorand nounresolved SecretRefmessage. The companion gateway-side line from
/tmp/openclaw/openclaw-2026-06-27.log(subsystemgateway/ws) showsthe prerequisite step succeeded cleanly:
That
secrets.resolve 93msline is the gateway resolvingplugins.entries.tavily.config.webSearch.apiKeyfor the active call —the same path that the contributor's runtime fix in
prestart.shand theupstream PR both keep non-throwing. The runtime error observed in the
2026-06-21 logs above is not reproducible here.
Exact-head runtime proof (2026-06-28)
Built and tested from the exact PR head commit in an isolated Docker container (no prestart.sh, no patched runtime overlay).
Environment:
docker.io/library/node:24-bookworm-slim(Node v24.18.0, pnpm 11.2.2)fix/tavily-secretref-fallback3a9963a2bf69a0831c198361c04a68128ca3ef66[email protected]:kzzalews/openclaw-upstream.git(depth=1, branch only)Step 1 — Source inspection (key lines from
extensions/tavily/src/config.tsat head):Step 2 — Unit tests (exact-head, Node 24, pnpm 11.2.2):
All 8 cases pass (production SDK resolver path, no
vi.mock).Step 3 — End-to-end runtime proof (configured env SecretRef → Tavily 401):
Input scenario:
plugins.entries.tavily.config.webSearch.apiKeyis an env SecretRef(
{source: "env", provider: "default", id: "TAVILY_API_KEY"}) — the exact configurationthat caused
UnresolvedSecretInputErrorbefore this PR. The runtime snapshot cannotinline-resolve it.
The key (
tvly-proof-dummy-3a9963a2bf69) is a redacted dummy — the401 Unauthorizedresponse from
api.tavily.comconfirms the key reached the Tavily endpoint. With a livekey, the same call returns real search results instead of auth failure.
Test coverage
extensions/tavily/src/config.test.tscovers the newresolveConfiguredSecrethelper end-to-end against the bundled SDK (no
vi.mock— productionresolver path runs against
resolveSecretInputStringwithmode: "inspect"and
canResolveEnvSecretRefInReadOnlyPath):apiKeyas-is (happy path)process.env.TAVILY_API_KEYwhen the configured env SecretRef is unresolvable AND its id isTAVILY_API_KEY" TAVILY_API_KEY "still falls through)filesourceexecsourcecfg.secrets.providersallowlistundefinedwhen neither config nor env var is setRun with
node scripts/run-vitest.mjs run --config test/vitest/vitest.extension-misc.config.ts extensions/tavily/src/config.test.ts. All 8 cases pass.Change Type
Scope
Linked Issue
Fixes a regression where the bundled tavily plugin (added in PR #49200) cannot fall back to
TAVILY_API_KEYwhen the configuredplugins.entries.tavily.config.webSearch.apiKeyis a SecretRef the active runtime snapshot doesn't inline-resolve. Related closed issues that did not catch this runtime-side gap:extensions/tavily/src/config.ts,extensions/exa/src/config.ts) still throw in strict mode.infer web searchpath; this PR fixes the runtime plugin path that Fix infer SecretRef resolution for provider-backed commands #82798 left open.Related: #95112 by @liuhao1024 — closed unmerged (uses the same
mode: "inspect"core, without the file/exec/non-matching-env allowlist added here). If maintainers prefer that approach, the inspect-mode logic there can be combined with the allowlist gating from this one.Approach
Replaces the previous
try/catchapproach with the inspect-mode pattern already used inextensions/firecrawl/src/config.ts:resolveSecretInputStringwithmode: "inspect"so unresolved SecretRefs returnconfigured_unavailableinstead of throwing.fileandexecSecretRefs asblocked— never falls through to ambientprocess.env.TAVILY_API_KEY.idis notTAVILY_API_KEYasblocked— same reason.canResolveEnvSecretRefInReadOnlyPathbefore readingprocess.env.TAVILY_API_KEY.resolveTavilyApiKeyonly falls through toprocess.env.TAVILY_API_KEYwhen the configured SecretRef ismissing(no apiKey configured) — never when it isblocked.This addresses the ClawSweeper review feedback on the previous try/catch revision: the catch-all could silently route Tavily requests through ambient
TAVILY_API_KEYwhen the user configured a different explicit SecretRef source (file, exec, or env with a different id), weakening the intended credential boundary.Compatibility / Migration
CONTRIBUTING.md("Maintainers or ClawSweeper add the changelog entry when landing user-facing changes"). ClawSweeper's 2026-06-19 review noted the previous manual entry to be removed; this revision drops it.Risks and Mitigations
resolveTavilyApiKey) already hasprocess.envand downstream-string fallbacks. The chain inresolveTavilyApiKeywas always meant to handle an unresolvable ref; the throw was incidental.extensions/tavily/src/config.test.ts(3 inspect-mode positive paths, 5 negative paths, 8 cases total) assert the no-throw behavior, so any regression is caught at unit-test time.TAVILY_API_KEYenv SecretRef (e.g.OPENAI_API_KEYfor tavily) and the ambientprocess.env.TAVILY_API_KEYis silently used.blockedand explicitly returnsundefined; the test suite pins this behavior with dedicated cases.Generated with assistance from Aiko (AikoOpenClawBot), an AI agent running on OpenClaw 2026.6.10
minimax/minimax-m3via Kilo Gatewayhttps://api.kilo.ai/api/gateway)tavily_searchinvocation on this OpenClaw instance, withgateway/wsline above