fix(coding-agent): resolve web-search endpoints and keys from trusted env

[10kH]

Same trust-boundary class as #3262 / #3270 / #3276, applied to the web-search provider family.

Problem

Each provider resolves its endpoint and its auth material from the environment, then sends that credential to that endpoint:

provider	endpoint	credential	request
kimi.ts	:59 MOONSHOT_SEARCH_BASE_URL / KIMI_SEARCH_BASE_URL	:68 MOONSHOT_SEARCH_API_KEY / KIMI_SEARCH_API_KEY	:82 fetch(resolveBaseUrl()) + :87 Bearer
xai.ts	:75 XAI_SEARCH_BASE_URL	from auth storage	:417 fetch(responsesEndpoint()) + :420 Bearer
anthropic.ts	:279 ANTHROPIC_SEARCH_BASE_URL	:278 ANTHROPIC_SEARCH_API_KEY, falling back to the active model credentials	:118 fetch(url)

All were read through $env, the merged view that includes the caller's cwd/.env, so a repository could redirect search traffic and collect the user's search credentials — for xAI the bearer comes from the trusted credential store, so redirecting the base URL alone exfiltrates it.

Fix

Resolve through $credentialEnv, the resolver this codebase already designates for material that must not come from the caller's project .env. Name precedence inside each provider is preserved (MOONSHOT_* ahead of KIMI_*), and the launching shell, ~/.env, the GJC config .env and the agent .env keep working.

SearXNG deliberately excluded

searxng.ts is the fourth provider in this family and reads SEARXNG_ENDPOINT / SEARXNG_TOKEN / SEARXNG_BASIC_USERNAME / SEARXNG_BASIC_PASSWORD the same way — but its basic-auth path supports an intentionally empty username or password, and $credentialEnv drops empty values. I implemented it, and its two existing red-team cases (sends Basic auth when the password is intentionally empty, … username …) went red, so moving it would silently remove that capability. Binding SearXNG needs an empty-preserving trusted resolver, which is a primitive decision rather than something to invent inside this change — happy to follow your preference in a successor.

Tests

web-search-env-trust.test.ts spawns a probe with a controlled cwd (the env module parses projectEnv at load time, so the boundary is only observable across processes): built-in endpoints and no credentials by default, planted endpoints ignored, planted credentials ignored, inherited configuration honored, and a planted .env cannot override an inherited value.

Proof-first: reverting only the kimi resolver fails ignores search endpoints planted by the project .env (4 pass / 1 fail).

Verification

bun run check in packages/coding-agent (whole-package biome + tsc): exit 0. New suite: 5 pass / 0 fail. Search-related suites across the package: 640 pass / 0 fail (including the SearXNG red-team cases, which stay green because that file is untouched).

[Yeachan-Heo]

Adversarial review — exact head d3668076d0b9c18842ef5e30319d6c2f5606395b vs origin/dev 18f6935b7c5f9995b5478b2f0e083a2d4302736d

Verdict: MERGE_READY (do not merge yet — see Merge gating.)

Continuation of the same read-only env-trust batch as #3273/#3276/#3278/#3280/#3250.

Credential-to-endpoint pairings — traced individually, not taken from the table

The premise is that each provider sends its credential to an endpoint it also resolves from the environment. I confirmed all three at this head:

provider	endpoint resolution	credential	where they meet
kimi.ts	resolveBaseUrl() :63	findApiKey() :72	:92 fetch(resolveBaseUrl()) + :97 Bearer ${apiKey}
xai.ts	getBaseUrl() :79 → responsesEndpoint() :83	auth storage	:422 fetch(responsesEndpoint()) + :425 Bearer ${auth.bearer}
anthropic.ts	$credentialEnv("ANTHROPIC_SEARCH_BASE_URL") :279	:278	buildAnthropicAuthConfig(searchApiKey, searchBaseUrl) :287

The xAI case is the sharpest and the description is right to call it out: the bearer comes from the trusted credential store, so redirecting the base URL alone was sufficient to exfiltrate it — no planted key required. The Anthropic case is the tidiest to verify because key and URL are bound together in one buildAnthropicAuthConfig call, so there is no way for the fix to desynchronise them.

Surface closure checked mechanically. Remaining $env reads after the change:

• anthropic.ts:49 — ANTHROPIC_SEARCH_MODEL
• xai.ts:71 — PI_XAI_WEB_SEARCH_MODEL / XAI_WEB_SEARCH_MODEL
• kimi.ts — none

Those are model-name selectors, not endpoints or credentials. Correctly left alone: a planted model name cannot move a credential.

Alias and empty-value semantics

MOONSHOT_* stays ahead of KIMI_* in both resolveBaseUrl and findApiKey. The asTrimmed(...) ?? asTrimmed(...) shape is preserved exactly, and since $credentialEnv already trims and nulls empties (env.ts:119-120, 164-165), the outer asTrimmed is now redundant rather than load-bearing — harmless, and it keeps the diff minimal. An empty MOONSHOT_SEARCH_API_KEY still falls through to KIMI_SEARCH_API_KEY rather than short-circuiting.

isAvailable() was updated on both providers too. That matters: leaving it on $env would have let a planted .env flip a provider to "available" and change routing even though the credential no longer resolved.

SearXNG exclusion — I tried to falsify this and could not

This is the part of the PR most likely to be a rationalisation, so I checked it rather than accepting it. searxng.ts reads SEARXNG_ENDPOINT :84, SEARXNG_TOKEN :95, SEARXNG_BASIC_USERNAME :106, SEARXNG_BASIC_PASSWORD :117 from raw process.env — structurally the same endpoint+credential shape, so on the surface it looks like an omission.

It is not. findAuth() :131-135 branches on !== null, and buildBasicAuthValue() :121 encodes ${username}:${password} — so an empty string is a meaningful distinct value, not an absent one. The two named red-team cases exist exactly as described (web-search-searxng.test.ts:110 and :129) and assert Basic base64("alice:") and Basic base64(":s3cret").

Measured directly against the resolver:

SEARXNG_BASIC_PASSWORD=""  ->  process.env: ""   $credentialEnv: null

So binding SearXNG to $credentialEnv would silently delete a supported capability and turn those two red-team cases red. Deferring it pending an empty-preserving trusted resolver is the correct call, and naming it in the PR body rather than quietly skipping it is the right way to handle it. I ran web-search-searxng.test.ts at this head: 11 pass / 0 fail, consistent with the file being untouched.

Worth stating for the record: SearXNG remains genuinely exposed until that successor lands. Not this PR's debt, but it should not be forgotten.

Local verification at this exact head

• packages/coding-agent/test/web-search-env-trust.test.ts — 5 pass / 0 fail
• packages/coding-agent/test/tools/web-search-searxng.test.ts — 11 pass / 0 fail
• bun run --cwd packages/coding-agent check — exit 0 (one pre-existing biome info in an unrelated prototype-pollution test)

Credential leakage

Zero logging of any key, bearer, or base URL across all three changed files.

Test hermeticity

resolveIn() :59 deletes the seven target keys from the child env but does not isolate HOME or GJC_CODING_AGENT_DIR, so $credentialEnv's file sources (agentEnv, piEnv, homeEnv, homeShellEnv — env.ts:220-229) can still leak in. Same latent class I reported on #3273. Not blocking here, but note that #3284 fixes exactly this pattern in the two merged suites and this PR reintroduces it in a new one — worth folding env.HOME = tempDir(); env.GJC_CODING_AGENT_DIR = tempDir(); in before merge, or immediately after #3284 lands, so the batch converges instead of oscillating.

Overlap and CI

Source files disjoint from every other open env-trust PR (the only shared source file in the whole batch remains azure-openai-responses.ts between #3268 and #3272). I ran a real sequential merge trial of #3282 → #3283 → #3284 onto current dev: all three auto-merge cleanly, including the shared packages/coding-agent/CHANGELOG.md anchor. No ordering requirement.

Changelog entry lands in the correct Unreleased → Fixed block. Hosted CI at this exact head: 15 success, 5 skipped, 0 failures.

Pre-existing platform gap (not charged to this PR)

A planted GJC_CODING_AGENT_DIR still redirects getAgentDir() (dirs.ts:251) and can relocate the "trusted" agent .env into the repo. Untouched by this PR and equally defeats the already-merged trustedBrowserEnv. Separate follow-up.

Merge gating

MERGE_READY is a verdict on this head, not an instruction to merge. Post-#3275 dev CI is still nonterminal (test:@gajae-code/coding-agent:shard-3-of-8 and shard-7-of-8 in progress at review time). Hold until it is terminal and green.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[Yeachan-Heo]

Verdict: APPROVE — exact head 8ba9e5a7103692c2552e546ee158072975fc664b

Upgrading the prior COMMENTED/MERGE_READY (do-not-merge-yet, held for batch sequencing, not a technical defect) at d3668076d0b9c18842ef5e30319d6c2f5606395b to a terminal APPROVE now that CI is exact-head green.

Checks performed:

• Content identity confirmed, not assumed: diff <(git show d3668076 --format= -- packages/coding-agent/src/web/search/providers/anthropic.ts packages/coding-agent/src/web/search/providers/kimi.ts packages/coding-agent/src/web/search/providers/xai.ts packages/coding-agent/test/fixtures/web-search-env-probe.ts packages/coding-agent/test/web-search-env-trust.test.ts) <(git show 8ba9e5a7 --format= -- packages/coding-agent/src/web/search/providers/anthropic.ts packages/coding-agent/src/web/search/providers/kimi.ts packages/coding-agent/src/web/search/providers/xai.ts packages/coding-agent/test/fixtures/web-search-env-probe.ts packages/coding-agent/test/web-search-env-trust.test.ts) is empty — the PR's own commit is byte-for-byte identical to the version the prior adversarial review validated; only intervening independently-merged dev commits sit between the two heads (confirmed via git merge-base).
• Exact-head terminal CI: 15 success / 0 failure / 0 pending / 5 skipped (total 20). No pending/queued checks remain.

Prior adversarial findings stand unchanged against this head.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[Yeachan-Heo]

Adversarial batch re-review — exact head 8ba9e5a7103692c2552e546ee158072975fc664b

Verdict: MERGE_READY — held, do not merge (see gating).

Independent re-verification, not a restatement of the prior APPROVE.

Trust primitive verified against base, not assumed

$credentialEnv exists on origin/dev fbb5670 (packages/utils/src/env.ts:118) and resolves $inheritedEnv → live → agentEnv → piEnv → homeEnv → homeShellEnv, deliberately excluding projectEnv. filterCredentialInheritedEnv additionally drops any Bun.env entry whose value exactly matches cwd/.env, closing the "Bun pre-loaded the project .env" ambiguity. The batch's premise is therefore real, not cosmetic.

Per-symbol trace

• anthropic.ts: ANTHROPIC_SEARCH_API_KEY + ANTHROPIC_SEARCH_BASE_URL both moved. Correct pairing — moving the key without the base URL would still let a repo redirect the authenticated request.
• kimi.ts: all four (MOONSHOT_*/KIMI_* key and base URL) moved; asTrimmed composition preserved.
• xai.ts: XAI_SEARCH_BASE_URL moved; $env import retained for genuinely non-credential reads.
• isAvailable() predicates moved in lockstep with the resolvers, so availability cannot disagree with what actually authenticates.

Tests

web-search-env-trust.test.ts drives a child process with controlled cwd, and neutralizes HOME + GJC_CODING_AGENT_DIR — necessary because projectEnv/homeEnv are parsed at module load. Covers default, planted-project-.env, inherited-honored, and the precedence case where a planted value must not override an inherited one.

Local bun test in this review worktree failed only on Cannot find module '@gajae-code/utils' (workspace deps not installed in the review checkout) — an environment artifact, not a PR defect. Exact-head CI is authoritative here and is green, including test:packages/coding-agent/test/web-search-env-trust.test.ts, check:@gajae-code/coding-agent, native-build, cli-smoke.

Merge gating

dev CI is currently red (latest Dev CI run: failure). This is a durable non-resident hold on a technically clean PR — batch sequencing, not a defect in this change.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]