fix: require env resolution for web provider refs by lin-hongkuan · Pull Request #96400 · openclaw/openclaw

lin-hongkuan · 2026-06-24T12:18:34Z

What Problem This Solves

Fixes an issue where web provider credential detection treated env secret references such as ${CUSTOM_API_KEY} or $CUSTOM_API_KEY as literal configured credentials when the referenced environment variable was missing.

AI-assisted: yes. I reviewed the touched credential detection path and ran the focused validation below.

Why This Change Was Made

Recognized env secret refs now prove credential availability only through the env resolver, while non-env secret refs and normal literal strings keep their existing behavior. The same rule applies to primary and fallback credential sources.

User Impact

Web search/fetch provider setup no longer reports a provider as credentialed just because a missing env reference string is present in config. Operators get more accurate provider availability and fallback behavior.

Evidence

node scripts/run-vitest.mjs run --config test/vitest/vitest.unit.config.ts packages/web-content-core/src/provider-runtime-shared.test.ts
node_modules\.bin\oxfmt.cmd --check packages/web-content-core/src/provider-runtime-shared.ts packages/web-content-core/src/provider-runtime-shared.test.ts
git diff --check

Follow-up Evidence After Review

The review asked for real provider/config behavior proof and compatibility notes. I ran the shared web provider credential path directly with a configured web search provider using apiKey: "${CUSTOM_API_KEY}".

Live provider/config output from this branch:

{"label":"missing env ref","configuredApiKey":"${CUSTOM_API_KEY}","envHasCustomApiKey":false,"credentialed":false}
{"label":"resolved env ref","configuredApiKey":"${CUSTOM_API_KEY}","envHasCustomApiKey":true,"credentialed":true}
{"label":"literal compatibility","configuredApiKey":"literal-redacted-secret","envHasCustomApiKey":false,"credentialed":true}

Compatibility/auth-provider note: non-env SecretRefs (file / exec) and provider auth entries are still treated as configured credentials. Literal config strings are still accepted as before. This change only prevents env-style refs (${CUSTOM_API_KEY}, $CUSTOM_API_KEY, and legacy env markers) from being counted as a credential when env resolution actually misses.

Focused validation:

RUN  v4.1.8 C:/Users/lin20/Documents/New project 3/openclaw

Test Files  1 passed (1)
     Tests  9 passed (9)
  Start at  20:34:05
  Duration  674ms (transform 472ms, setup 366ms, import 13ms, tests 100ms, environment 0ms)

Checking formatting...

All matched files use the correct format.
Finished in 1195ms on 2 files using 32 threads.

git diff --check produced no output.

clawsweeper · 2026-06-24T12:20:06Z

Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 11:52 AM ET / 15:52 UTC.

Summary
The branch changes the shared web provider credential helper and tests so primary and fallback env-style SecretRefs only count when env resolution succeeds while literals and non-env SecretRefs still count.

PR surface: Source +4, Tests +37. Total +41 across 2 files.

Reproducibility: yes. Source inspection on current main shows env shorthand is parsed as a SecretRef and then still counted as a nonempty literal credential before env resolution; I did not run tests in this read-only review.

Review metrics: 1 noteworthy metric.

Env-ref credential gates: 2 changed: primary and fallback. Both credential sources now require env resolution for env-style refs, which is the compatibility-sensitive behavior maintainers need to accept before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

none.

Risk before merge

[P1] Existing configs that relied on unresolved env-ref strings being truthy may now fall back to another provider or no provider until the env value or provider auth is actually available.
[P2] The shared helper feeds both web_search and web_fetch provider availability, so maintainer acceptance should cover provider selection and auth-provider fallback semantics for both surfaces.

Maintainer options:

Accept stricter env-ref semantics (recommended)
Maintainers can accept that unresolved env-style refs no longer make web providers look credentialed, using the PR body's live output and helper tests as proof.
Request caller-level proof first
Maintainers can ask for focused web_search and web_fetch runtime proof if they want stronger confidence in provider-selection behavior before merge.
Pause if compatibility wins
If unresolved env-ref strings must keep selecting the configured provider, pause this PR and choose a diagnostic-only or opt-in strict path instead.

Next step before merge

No automated repair is needed; the remaining action is maintainer acceptance of the stricter compatibility and auth-provider behavior before merge.

Maintainer decision needed

Question: Should web provider credential detection stop treating unresolved env-style SecretRefs as configured credentials for both primary and fallback web provider sources?
Rationale: The code fix is narrow and well-covered, but it deliberately changes upgrade behavior for configs that previously selected providers from unresolved env-ref strings.
Likely owner: steipete — steipete both authored the shared helper refactor and is assigned to the PR, making him the best owner for the compatibility decision.
Options:
- Accept stricter env-ref semantics (recommended): Merge this PR and treat unresolved env-style refs as unavailable credentials while preserving literal strings and non-env SecretRefs.
- Ask for caller-level upgrade proof: Hold merge until web_search and web_fetch caller proof demonstrates the provider-selection behavior maintainers want for existing configs.
- Preserve compatibility by default: Pause this PR and choose a diagnostic or opt-in strict path if unresolved env-ref strings must keep selecting the configured provider.

Security
Cleared: The diff only changes TypeScript credential-detection logic and focused tests; it adds no dependencies, scripts, workflow permissions, downloads, or secret exposure paths.

Review details

Best possible solution:

Land the shared-helper fix after maintainers accept the stricter env-ref credential semantics; keep the rule centralized and preserve literal and non-env SecretRef behavior.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection on current main shows env shorthand is parsed as a SecretRef and then still counted as a nonempty literal credential before env resolution; I did not run tests in this read-only review.

Is this the best way to solve the issue?

Yes. The shared helper is the right fix boundary because both web_search and web_fetch call it; caller-level fixes would duplicate credential semantics and risk drift.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against ef53c20497e1.

Label changes

Label justifications:

P2: This is a normal-priority web provider credential detection fix with limited but real web-tool blast radius.
merge-risk: 🚨 compatibility: Existing configs that previously treated unresolved env-ref strings as truthy credentials can change provider selection after upgrade.
merge-risk: 🚨 auth-provider: The changed helper participates in web provider credential availability and provider-auth fallback decisions for web_search and web_fetch.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes redacted live output from the configured web provider credential path showing missing-env, resolved-env, and literal compatibility behavior after the fix.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes redacted live output from the configured web provider credential path showing missing-env, resolved-env, and literal compatibility behavior after the fix.

Evidence reviewed

PR surface:

Source +4, Tests +37. Total +41 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	6	2	+4
Tests	1	37	0	+37
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	43	2	+41

What I checked:

Current-main bug path: Current main parses an env-style raw value as a SecretRef, but then still normalizes the same raw string as a literal credential before env resolution, so an unresolved ${CUSTOM_API_KEY} or $CUSTOM_API_KEY can make the provider appear credentialed. (packages/web-content-core/src/provider-runtime-shared.ts:189, ef53c20497e1)
PR implementation: The PR skips literal credential fallback when the primary or fallback value parsed as any SecretRef, preserving non-env SecretRefs while forcing env refs through the resolver. (packages/web-content-core/src/provider-runtime-shared.ts:193, 04b343244125)
Regression coverage: The PR adds focused helper tests for missing primary env refs, missing fallback env refs, and literal-string compatibility. (packages/web-content-core/src/provider-runtime-shared.test.ts:83, 04b343244125)
Caller surface: web_search: web_search uses the shared helper for provider credential availability, configured values, fallback values, env probing, and provider auth profiles. (src/web-search/runtime.ts:86, ef53c20497e1)
Caller surface: web_fetch: web_fetch also uses the shared helper for provider credential availability, so the changed rule affects both web tool provider-selection surfaces. (src/web-fetch/runtime.ts:77, ef53c20497e1)
SecretRef contract: The docs define ${NAME} and $NAME as accepted shorthand strings on SecretInput fields, supporting treatment as env refs rather than literal credentials. Public docs: docs/gateway/secrets.md. (docs/gateway/secrets.md:118, ef53c20497e1)

Likely related people:

steipete: Peter Steinberger authored the refactor that centralized web provider credential logic into the shared helper and is the current assignee on this PR. (role: shared web runtime helper author and assigned reviewer; confidence: high; commits: 8838fdc91625; files: src/web-search/runtime.ts, src/web-fetch/runtime.ts, src/web/provider-runtime-shared.ts)
pgondhi987: Pavan Kumar Gondhi authored and merged the recent package move that owns the current-main package helper and web runtime file blame. (role: recent area contributor; confidence: medium; commits: 3ba72f624acc; files: packages/web-content-core/src/provider-runtime-shared.ts, src/web-search/runtime.ts, src/web-fetch/runtime.ts)
joshavant: joshavant authored the provider-based external secrets management work that introduced the broader SecretRef object and env-template contract this fix depends on. (role: SecretRef feature introducer; confidence: medium; commits: 4e7a833a24e7; files: src/config/types.secrets.ts, docs/gateway/secrets.md)

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-25T21:18:26.804Z sha 450a9f0 :: needs maintainer review before merge. :: none

steipete · 2026-07-09T16:10:27Z

Merged via squash.

Prepared head SHA: 04b343244125380888e6cabe474f41acba3b01bd
Landed commit: 148ec3282f071a9f63b4ae8d4dd9332bb37d3ba2

* fix: require env resolution for web provider refs * chore: retrigger PR checks --------- Co-authored-by: lin-hongkuan <[email protected]>

openclaw-barnacle Bot added the size: XS label Jun 24, 2026

steipete self-assigned this Jul 9, 2026

lin-hongkuan and others added 2 commits July 9, 2026 08:26

fix: require env resolution for web provider refs

d886b0c

chore: retrigger PR checks

04b3432

steipete force-pushed the codex/web-provider-env-ref-credentials branch from 450a9f0 to 04b3432 Compare July 9, 2026 15:31

steipete merged commit 148ec32 into openclaw:main Jul 9, 2026
81 checks passed

github-actions Bot mentioned this pull request Jul 9, 2026

📡 Upstream Digest — 2026-07-09 17:48 UTC curtismercier/openclaw-mods#1244

Open

This was referenced Jul 9, 2026

fix(models): preserve static provider catalog metadata #103051

Closed

fix(infra): drop replacement characters when response snippets truncate UTF-8 #102532

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: require env resolution for web provider refs#96400

fix: require env resolution for web provider refs#96400
steipete merged 2 commits into
openclaw:mainfrom
lin-hongkuan:codex/web-provider-env-ref-credentials

lin-hongkuan commented Jun 24, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

lin-hongkuan commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Follow-up Evidence After Review

Uh oh!

clawsweeper Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lin-hongkuan commented Jun 24, 2026 •

edited

Loading

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading