Skip to content

fix(tavily): fall back to process.env when config SecretRef is unresolvable#95110

Closed
kzzalews wants to merge 1 commit into
openclaw:mainfrom
kzzalews:fix/tavily-secretref-fallback
Closed

fix(tavily): fall back to process.env when config SecretRef is unresolvable#95110
kzzalews wants to merge 1 commit into
openclaw:mainfrom
kzzalews:fix/tavily-secretref-fallback

Conversation

@kzzalews

@kzzalews kzzalews commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

tavily_search and tavily_extract fail at runtime with unresolved SecretRef "env:default:TAVILY_API_KEY" even when TAVILY_API_KEY is set in the gateway process environment. The configured SecretRef points at an env-backed credential that the active runtime snapshot can't inline-resolve, but the plugin's own call chain already has a process.env.TAVILY_API_KEY fallback — the helper in strict mode throws before that fallback can run. Bundled peers (brave, firecrawl, exa) use readConfiguredSecretString from extensions/web-search-provider-common and don't hit this; tavily and exa are the only remaining extensions/*/src/config.ts helpers still throwing in strict mode.

Evidence

Environment: Linux, Node v24, OpenClaw v2026.6.8 (commit 8a24f93), with the patch applied via prestart.sh step 7/7 in the contributor's runtime overlay.

Reproduction (before fix):

export TAVILY_API_KEY=***
openclaw config set plugins.entries.tavily.config.webSearch.apiKey \
  --ref-provider default --ref-source env --ref-id 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". Resolve this command against an active gateway runtime snapshot before reading it.

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_extract tools failed for the same
reason. These are the exact log lines:

2026-06-21T12:05:16.462+00:00 [tools] tavily_search failed: plugins.entries.tavily.config.webSearch.apiKey: unresolved SecretRef "env:default:TAVILY_API_KEY". Resolve this command against an active gateway runtime snapshot before reading it. raw_params={"query":"Tailscale pricing 2026 Personal Pro Enterprise plans devices users","max_results":8,"include_answer":true}
2026-06-21T12:05:16.549+00:00 [tools] tavily_search failed: plugins.entries.tavily.config.webSearch.apiKey: unresolved SecretRef "env:default:TAVILY_API_KEY". Resolve this command against an active gateway runtime snapshot before reading it. raw_params={"query":"Tailscale features MagicDNS HTTPS TLS certificates HTTPS Let's Encrypt 2025","max_results":8,"include_answer":true}
2026-06-21T12:05:16.553+00:00 [tools] tavily_search failed: plugins.entries.tavily.config.webSearch.apiKey: unresolved SecretRef "env:default:TAVILY_API_KEY". Resolve this command against an active gateway runtime snapshot before reading it. raw_params={"query":"Tailscale Kubernetes operator k3s integration GitOps GitHub","max_results":6}
2026-06-21T12:05:29.366+00:00 [tools] tavily_extract failed: plugins.entries.tavily.config.webSearch.apiKey: unresolved SecretRef "env:default:TAVILY_API_KEY". Resolve this command against an active gateway runtime snapshot before reading it. raw_params={"urls":["https://tailscale.com/","https://tailscale.com/kb/"],"query":"Tailscale documentation features overview","chunks_per_source":3}

Same symptom, four independent calls within a 13-second window — the
fallback to process.env.TAVILY_API_KEY was being skipped because the
resolver 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.10
with the bundled tavily-plugin v2026.6.8 patched in-place by
prestart.sh step 7/7 (so the runtime resolver carries the same
inspect-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_search
tool on 2026-06-27T01:36:30Z with the query OpenClaw 2026.6.10 release notes tavily plugin. The call returned three real Tavily results in
1213 ms with no UnresolvedSecretInputError and no unresolved SecretRef
message. The companion gateway-side line from
/tmp/openclaw/openclaw-2026-06-27.log (subsystem gateway/ws) shows
the prerequisite step succeeded cleanly:

2026-06-27T01:36:14.935+00:00 [gateway/ws] ⇄ res ✓ secrets.resolve 93ms conn=94febf43…1322 id=efb0b53a…cc65

That secrets.resolve 93ms line is the gateway resolving
plugins.entries.tavily.config.webSearch.apiKey for the active call —
the same path that the contributor's runtime fix in prestart.sh and the
upstream 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:

  • Image: docker.io/library/node:24-bookworm-slim (Node v24.18.0, pnpm 11.2.2)
  • Platform: linux/amd64
  • Branch: fix/tavily-secretref-fallback
  • Commit: 3a9963a2bf69a0831c198361c04a68128ca3ef66
  • Source cloned from: [email protected]:kzzalews/openclaw-upstream.git (depth=1, branch only)

Step 1 — Source inspection (key lines from extensions/tavily/src/config.ts at head):

3:import { canResolveEnvSecretRefInReadOnlyPath } from "openclaw/plugin-sdk/extension-shared";
48:function resolveConfiguredSecret(
57:    mode: "inspect",
74:    !canResolveEnvSecretRefInReadOnlyPath({
88:  const resolved = resolveConfiguredSecret(

Step 2 — Unit tests (exact-head, Node 24, pnpm 11.2.2):

$ node scripts/run-vitest.mjs run --config test/vitest/vitest.extension-misc.config.ts \
    extensions/tavily/src/config.test.ts

 RUN  v4.1.8 /workspace

 ✓  extension-misc  extensions/tavily/src/config.test.ts (8 tests) 56ms

 Test Files  1 passed (1)
       Tests  8 passed (8)
    Start at  19:23:56
    Duration  844ms (transform 413ms, setup 236ms, import 392ms, tests 56ms, environment 0ms)

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.apiKey is an env SecretRef
({source: "env", provider: "default", id: "TAVILY_API_KEY"}) — the exact configuration
that caused UnresolvedSecretInputError before this PR. The runtime snapshot cannot
inline-resolve it.

$ node --import ./node_modules/tsx/dist/esm/index.cjs proof-runtime.mts

=== PR #95110 exact-head runtime proof ===
commit: 3a9963a2bf69a0831c198361c04a68128ca3ef66
branch: fix/tavily-secretref-fallback
timestamp: 2026-06-28T19:23:58.869Z

Input: configured env SecretRef {source:'env', provider:'default', id:'TAVILY_API_KEY'}
Env:   TAVILY_API_KEY = tvly-proof-dummy-3a9963a2bf69

resolveTavilyApiKey() returned: "tvly-proof-dummy-3a9963a2bf69"
  → Key resolved from process.env fallback (not thrown): OK

Making HTTP call to api.tavily.com with resolved key...
HTTP status: 401 Unauthorized
Response: {"detail":{"error":"Unauthorized: missing or invalid API key."}}

✓ PROOF COMPLETE: configured env SecretRef fallback passed key to Tavily
✓ Tavily returned auth failure (not UnresolvedSecretInputError)
✓ The PR fix (inspect-mode SecretRef resolver) works at runtime

The key (tvly-proof-dummy-3a9963a2bf69) is a redacted dummy — the 401 Unauthorized
response from api.tavily.com confirms the key reached the Tavily endpoint. With a live
key, the same call returns real search results instead of auth failure.

Test coverage

extensions/tavily/src/config.test.ts covers the new resolveConfiguredSecret
helper end-to-end against the bundled SDK (no vi.mock — production
resolver path runs against resolveSecretInputString with mode: "inspect"
and canResolveEnvSecretRefInReadOnlyPath):

  • returns the configured string apiKey as-is (happy path)
  • falls back to process.env.TAVILY_API_KEY when the configured env SecretRef is unresolvable AND its id is TAVILY_API_KEY
  • trims whitespace from the configured env SecretRef id before matching (" TAVILY_API_KEY " still falls through)
  • does NOT fall back when the configured SecretRef is a file source
  • does NOT fall back when the configured SecretRef is an exec source
  • does NOT fall back when the configured env SecretRef targets a different env var
  • does NOT fall back when the configured env SecretRef uses a provider not in cfg.secrets.providers allowlist
  • returns undefined when neither config nor env var is set

Run 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

  • Bug fix

Scope

  • Integrations (tavily web-search plugin)

Linked Issue

Fixes a regression where the bundled tavily plugin (added in PR #49200) cannot fall back to TAVILY_API_KEY when the configured plugins.entries.tavily.config.webSearch.apiKey is a SecretRef the active runtime snapshot doesn't inline-resolve. Related closed issues that did not catch this runtime-side gap:

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/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 and 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. Trims whitespace from the configured id before matching (YAML config may pad env var names).
  6. 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 the previous try/catch revision: the 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.

Compatibility / Migration

  • Backward compatible? Yes — behavior change is limited to the previously-broken unresolvable-env-ref case; success path is identical.
  • Config/env changes? No
  • Migration needed? No
  • CHANGELOG entry: NOT included in this PR, per 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

  • Risk: a stricter upstream caller relies on the resolver throwing so it can surface the error to the user.
    • Mitigation: the only caller in this file (resolveTavilyApiKey) already has process.env and downstream-string fallbacks. The chain in resolveTavilyApiKey was always meant to handle an unresolvable ref; the throw was incidental.
  • Risk: a future plugin re-introduces the same throw pattern.
    • Mitigation: the new tests in 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.
  • Risk: a future user configures a non-TAVILY_API_KEY env SecretRef (e.g. OPENAI_API_KEY for tavily) and the ambient process.env.TAVILY_API_KEY is silently used.
    • Mitigation: the new helper treats such refs as blocked and explicitly returns undefined; the test suite pins this behavior with dedicated cases.

Generated with assistance from Aiko (AikoOpenClawBot), an AI agent running on OpenClaw 2026.6.10

  • Model: minimax/minimax-m3 via Kilo Gateway
  • Provider: Kilo Gateway (https://api.kilo.ai/api/gateway)
  • Author: Karol Zalewski [email protected]
  • Local reproduction:
    • Isolated test pod reproduction (2026-06-21): gateway log timestamps cited above
    • Current-installation reproduction (2026-06-27): Aiko's MCP tavily_search invocation on this OpenClaw instance, with gateway/ws line above

@openclaw-barnacle openclaw-barnacle Bot added extensions: tavily size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 15, 2026, 6:47 AM ET / 10:47 UTC.

Summary
The branch changes Tavily API-key resolution to permit ambient TAVILY_API_KEY fallback only for a matching allowed unresolved env SecretRef, blocks other explicit refs, and adds focused regression tests.

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.

  • Credential fallback surfaces: 1 changed. The PR changes precedence for the Tavily API-key credential path, which is compatibility- and auth-sensitive even without a new config key.
  • Blocked fallback classes: 4 covered. Focused tests preserve fail-closed behavior for file refs, exec refs, wrong env ids, and disallowed env providers.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #95109
Summary: The open issue is the canonical Tavily strict SecretRef fallback report; this PR and the broader proof-positive agent-runtime PR are competing candidate fixes.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] Merging intentionally changes credential precedence: a matching permitted unresolved env SecretRef can use ambient TAVILY_API_KEY instead of failing, which requires explicit acceptance as part of the credential contract.
  • [P1] This focused branch does not include the separate agent Tool Search runtime-snapshot repair in fix: resolve Tavily SecretRefs in agent tools #97827, so choosing it requires preserving or splitting that broader fix rather than losing the distinct agent-runtime work.
  • [P1] The branch is behind current main and needs a final rebase or merge-result review, although the provided state reports it as mergeable and the Tavily patch remains bounded.

Maintainer options:

  1. Adopt the broader canonical branch (recommended)
    Land fix: resolve Tavily SecretRefs in agent tools #97827 after current-head review so both known Tavily SecretRef ownership layers are addressed and the overlapping branches can close.
  2. Land the focused resolver only
    Accept this branch's credential precedence and tests, while retaining the agent Tool Search snapshot correction as explicit follow-up work.
  3. Pause for a stricter credential contract
    Do not merge either branch if configured-but-unresolved env refs should fail closed rather than consult the matching ambient environment variable.

Next step before merge

  • No automated repair is needed; a maintainer should accept or reject the credential-boundary contract and select this branch or the broader proof-positive candidate as canonical.

Maintainer decision needed

  • Question: Which implementation should become canonical for the Tavily SecretRef report: this focused guarded resolver, or fix: resolve Tavily SecretRefs in agent tools #97827 with the additional agent Tool Search runtime-snapshot repair?
  • Rationale: Both branches adopt the same security-sensitive ambient-env fallback contract, but they differ materially in ownership scope; automation cannot decide whether maintainers want the adjacent agent-runtime correction coupled to this fix.
  • Likely owner: joshavant — Their history owns the strict-versus-inspect SecretRef contract and adjacent command-resolution behavior that governs the security boundary under review.
  • Options:
    • Land the broader guarded fix (recommended): Use fix: resolve Tavily SecretRefs in agent tools #97827 as canonical so the guarded Tavily fallback and agent Tool Search runtime-snapshot repair land together, then close the narrower candidates.
    • Land this focused fix: Merge this Tavily-only branch and require the distinct agent Tool Search runtime-snapshot repair to remain as a separately reviewed change.
    • Reject ambient env fallback: Pause both branches and define a strict fail-closed contract if an unresolved configured env SecretRef must never use the matching ambient environment variable.

Security
Cleared: No concrete security or supply-chain defect was found; the implementation narrows ambient fallback to a matching permitted env ref and tests the fail-closed boundaries.

Review details

Best 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 changes

Label justifications:

  • P2: This is a real bundled Tavily credential-resolution regression with limited provider-specific blast radius and a proven repair.
  • merge-risk: 🚨 compatibility: Existing configured-but-unresolved matching env refs change from throwing to using ambient TAVILY_API_KEY, altering upgrade-time credential precedence.
  • merge-risk: 🚨 auth-provider: The patch changes how Tavily selects an API credential when a configured SecretRef is unavailable in the active read-only path.
  • merge-risk: 🚨 security-boundary: The diff decides when an explicitly configured credential reference may be replaced by ambient process environment state, while deliberately blocking other ref classes.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): Exact-head terminal proof shows a configured matching env SecretRef resolves through TAVILY_API_KEY and reaches Tavily authentication instead of throwing, with private values redacted.
  • proof: sufficient: Contributor real behavior proof is sufficient. Exact-head terminal proof shows a configured matching env SecretRef resolves through TAVILY_API_KEY and reaches Tavily authentication instead of throwing, with private values redacted.
Evidence reviewed

PR surface:

Source +47, Tests +202. Total +249 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 62 15 +47
Tests 1 202 0 +202
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 264 15 +249

What I checked:

  • Current-main failure remains: Current main still strictly normalizes the configured Tavily API-key SecretRef before consulting process.env.TAVILY_API_KEY, so an unresolved ref can throw before the documented fallback. (extensions/tavily/src/config.ts:37, 026b6881619e)
  • Guarded PR implementation: The PR uses inspect-mode resolution and permits fallback only for an allowed env ref whose normalized id is TAVILY_API_KEY; file, exec, wrong-id, and disallowed-provider refs remain blocked. (extensions/tavily/src/config.ts:53, 62f06e884283)
  • Credential-boundary coverage: Tests cover literal credentials, matching env fallback, whitespace normalization, file and exec blocking, wrong env-id blocking, provider-policy blocking, and missing configuration. (extensions/tavily/src/config.test.ts:31, 62f06e884283)
  • Real behavior proof: The PR body provides exact-head Docker terminal output showing the configured env SecretRef resolves to a dummy environment key and reaches Tavily's endpoint, returning the expected 401 rather than UnresolvedSecretInputError. (extensions/tavily/src/config.ts:88, 62f06e884283)
  • Original Tavily provenance: The affected resolver and client path originate in the merged bundled Tavily implementation. (extensions/tavily/src/config.ts:1, b36e456b0916)
  • SecretRef contract provenance: The inspect-versus-strict resolver contract used by this repair traces to the SecretRef implementation work in commit 1769fb2aa1d6. (src/config/types.secrets.ts:224, 1769fb2aa1d6)

Likely related people:

  • lakshyaag-tavily: Authored the merged bundled Tavily plugin work that added the affected resolver, environment-key support, and client path. (role: introduced behavior; confidence: high; commits: b36e456b0916, 32b8ad0106e3; files: extensions/tavily/src/config.ts, extensions/tavily/src/tavily-client.ts)
  • gumadeiras: Merged the original Tavily plugin and contributed to its configuration and provider wiring history. (role: merger and adjacent contributor; confidence: high; commits: b36e456b0916, 76cf58c3531f, f88e3954428c; files: extensions/tavily/src/config.ts, extensions/tavily/src/tavily-client.ts, extensions/tavily/web-search-shared.ts)
  • joshavant: Authored the strict/inspect SecretRef behavior and provider-backed command resolution work that defines the credential-boundary contract used here. (role: adjacent SecretRef contract owner; confidence: high; commits: 1769fb2aa1d6, 2416de142194; files: src/config/types.secrets.ts, src/plugin-sdk/secret-input.ts, src/plugin-sdk/extension-shared.ts)
  • VACInc: Has merged Tavily runtime-config SecretRef history and owns the broader open candidate that covers both this resolver and the agent Tool Search snapshot path. (role: recent Tavily runtime contributor; confidence: medium; commits: 8de5a55317c3, 1555165d1698; files: extensions/tavily/src/tavily-search-tool.ts, extensions/tavily/src/tavily-extract-tool.ts, extensions/tavily/src/tavily-tools.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (1 earlier review cycle)
  • reviewed 2026-06-28T20:41:48.561Z sha 685dc04 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jun 19, 2026
kzzalews added a commit to kzzalews/openclaw-upstream that referenced this pull request Jun 19, 2026
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>
@kzzalews
kzzalews force-pushed the fix/tavily-secretref-fallback branch from 266bc99 to f061b25 Compare June 19, 2026 22:23
@kzzalews

Copy link
Copy Markdown
Contributor Author

Quick note: there is a related PR in the same space — #95112 by @liuhao1024. It uses mode: "inspect" (cleaner than the v1 try/catch this PR originally proposed) but without the file/exec/non-matching-env allowlist added in this v2.

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.

@kzzalews
kzzalews force-pushed the fix/tavily-secretref-fallback branch from f061b25 to 7d10d5b Compare June 20, 2026 19:07
kzzalews added a commit to kzzalews/openclaw-upstream that referenced this pull request Jun 20, 2026
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>
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 20, 2026
@kzzalews

Copy link
Copy Markdown
Contributor Author

Rebase update — CI fix in progress

@vincentkoc — quick context on the latest push (force-pushed to 7d10d5b875):

Root cause of the previous CI failure: test/scripts/security-sensitive-guard-workflow.test.ts:146 in our base (c4d1f37d) asserts expect(script).toContain("/memberships/") against the standalone scripts/github/security-sensitive-guard.mjs. Your PR #95114 (merged 2026-06-19 22:55:40 UTC) introduced the guard-shared.mjs helper and switched the test to assert against the combined guardSources (script + shared). Our branch was pushed at 22:23:34 UTC — about 30 minutes before your fix landed — so our base was missing the new test fixture.

What I did:

  1. Rebased fix/tavily-secretref-fallback onto current upstream/main (now 3a14f247). 296 commits ahead, clean (no conflicts — extensions/tavily/ was untouched on main in that window).
  2. Force-pushed with --force-with-lease (refused because of stale info, then plain --force); branch tip is now 7d10d5b875.
  3. Updated PR body to satisfy the "Real behavior proof" policy check: added ## What Problem This Solves and ## Evidence sections, plus a Risks-and-Mitigations bullet for the silent-override scenario that motivated the v2 redesign.

Current status:

  • Real behavior proof: 3 runs, last 2 SUCCESS (after body update).
  • security-sensitive-guard: SUCCESS (this is the check that was cascading into 4 failures on the previous run).
  • Two CI jobs (preflight, security-fast) are still queued — looks like a GitHub Actions capacity stall affecting several PRs in the queue right now, not specific to this PR.
  • mergeStateStatus: UNSTABLE (downgrades to CLEAN once the queued jobs resolve).

No code changes — the actual fix is identical to what was in the v2 commits (b68d37a0 + 7d10d5b8). Just keeping the branch in sync with main.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jun 20, 2026
@kzzalews
kzzalews force-pushed the fix/tavily-secretref-fallback branch from 6e4a691 to 8b5cc2c Compare June 21, 2026 03:59
kzzalews added a commit to kzzalews/openclaw-upstream that referenced this pull request Jun 21, 2026
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 clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 21, 2026
@kzzalews

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@kzzalews

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated since last review:

  1. Reverted manual CHANGELOG.md entry per CONTRIBUTING.md and your [P3] finding.
  2. Added a Current-installation runtime proof section (Aiko invoked MCP tavily_search on 2026-06-27T01:36:30Z, 3 results in 1213ms, no UnresolvedSecretInputError). PR body has been updated.

@clawsweeper clawsweeper Bot added status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. labels Jun 27, 2026
@kzzalews

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated since last review:

  • Added Exact-head runtime proof (2026-06-28) section with output from a clean, isolated Docker container (node:24-bookworm-slim, Node v24.18.0, pnpm 11.2.2), cloned directly from [email protected]:kzzalews/openclaw-upstream.git at commit 3a9963a2bf69a0831c198361c04a68128ca3ef66 with no prestart.sh or patched runtime overlay.

The new proof shows three things:

  1. Source inspectionmode: "inspect" and canResolveEnvSecretRefInReadOnlyPath are present in the PR head extensions/tavily/src/config.ts.
  2. Unit tests — all 8 cases pass on exact-head (node scripts/run-vitest.mjs run ... extensions/tavily/src/config.test.ts).
  3. End-to-end runtime proofresolveTavilyApiKey() called with a configured env SecretRef ({source:'env', provider:'default', id:'TAVILY_API_KEY'}) returns the env var value without throwing, and an HTTP call to api.tavily.com with the resolved (dummy) key returns 401 Unauthorized: missing or invalid API key — auth failure, not UnresolvedSecretInputError.

This directly addresses the ClawSweeper [P1] finding: "lacks redacted exact-head Gateway/Tavily output showing this branch reaches auth failure or real results."

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 28, 2026
@kzzalews
kzzalews force-pushed the fix/tavily-secretref-fallback branch from 3a9963a to c6ac9a5 Compare June 28, 2026 19:48
kzzalews added a commit to kzzalews/openclaw-upstream that referenced this pull request Jun 28, 2026
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 clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. labels Jun 28, 2026
@kzzalews
kzzalews force-pushed the fix/tavily-secretref-fallback branch from c6ac9a5 to 685dc04 Compare June 28, 2026 20:36
kzzalews added a commit to kzzalews/openclaw-upstream that referenced this pull request Jun 28, 2026
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 clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 28, 2026
@kzzalews
kzzalews force-pushed the fix/tavily-secretref-fallback branch 2 times, most recently from bac589f to f15f86c Compare July 14, 2026 21:00
…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.
@kzzalews
kzzalews force-pushed the fix/tavily-secretref-fallback branch from f15f86c to 62f06e8 Compare July 14, 2026 21:09
@steipete

Copy link
Copy Markdown
Contributor

Superseded by #97827, landed on main as 092bd16b79225ff2125e8cd99cc721fb1bfa2308.

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 UnresolvedSecretInputError: #97827 (comment)

Thank you @kzzalews for the detailed reproduction and exact-head proof.

@steipete steipete closed this Jul 15, 2026
@kzzalews
kzzalews deleted the fix/tavily-secretref-fallback branch July 15, 2026 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extensions: tavily merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants