From 8e20dd22d8900ae0f76cfc38a7d9c81ee4f23f29 Mon Sep 17 00:00:00 2001 From: Josh Avant <830519+joshavant@users.noreply.github.com> Date: Sat, 7 Mar 2026 11:28:39 -0600 Subject: [PATCH] Secrets: harden SecretRef-safe models.json persistence (#38955) --- .secrets.baseline | 12 +- CHANGELOG.md | 1 + docs/cli/agent.md | 4 + docs/cli/models.md | 1 + docs/cli/secrets.md | 7 +- docs/concepts/models.md | 6 +- docs/gateway/configuration-reference.md | 4 +- docs/gateway/secrets.md | 7 +- .../reference/secretref-credential-surface.md | 2 + ...tref-user-supplied-credentials-matrix.json | 7 + extensions/diffs/src/http.ts | 11 +- src/agents/model-auth-env-vars.ts | 42 +++ src/agents/model-auth-markers.test.ts | 26 ++ src/agents/model-auth-markers.ts | 80 +++++ src/agents/model-auth.ts | 86 +---- src/agents/models-config.file-mode.test.ts | 43 +++ ...ssing-provider-apikey-from-env-var.test.ts | 114 +++++- ...s-config.providers.auth-provenance.test.ts | 121 +++++++ ...ig.providers.cloudflare-ai-gateway.test.ts | 76 ++++ ...ls-config.providers.discovery-auth.test.ts | 140 ++++++++ ...ls-config.providers.normalize-keys.test.ts | 27 ++ src/agents/models-config.providers.ts | 331 ++++++++++++------ ...els-config.runtime-source-snapshot.test.ts | 162 +++++++++ src/agents/models-config.ts | 138 ++++++-- .../models-config.write-serialization.test.ts | 55 +++ src/agents/pi-embedded-runner/model.test.ts | 72 ++++ src/agents/pi-embedded-runner/model.ts | 77 ++-- src/commands/agent.test.ts | 81 +++++ src/commands/agent.ts | 18 +- src/commands/models.list.e2e.test.ts | 55 ++- .../models/list.auth-overview.test.ts | 49 +++ src/commands/models/list.auth-overview.ts | 14 +- .../list.list-command.forward-compat.test.ts | 41 +++ src/commands/models/list.list-command.ts | 9 +- .../models/list.probe.targets.test.ts | 106 ++++++ src/commands/models/list.probe.ts | 4 +- src/commands/models/list.registry.ts | 9 +- src/commands/models/load-config.test.ts | 103 ++++++ src/commands/models/load-config.ts | 48 ++- src/config/config.identity-defaults.test.ts | 29 ++ src/config/config.ts | 1 + src/config/io.runtime-snapshot-write.test.ts | 65 ++++ src/config/io.ts | 4 + src/config/schema.help.quality.test.ts | 3 + src/config/schema.help.ts | 2 +- src/config/schema.hints.test.ts | 1 + src/config/types.models.ts | 2 +- src/config/zod-schema.core.ts | 2 +- .../monitor/message-handler.preflight.test.ts | 8 +- .../monitor/message-handler.queue.test.ts | 5 +- .../monitor/message-handler.test-helpers.ts | 3 +- ...rovider-usage.auth.normalizes-keys.test.ts | 73 ++++ src/infra/provider-usage.auth.ts | 15 +- src/line/bot-handlers.test.ts | 3 +- .../runner.deepgram.test.ts | 18 +- src/media-understanding/runner.entries.ts | 26 +- src/secrets/apply.test.ts | 53 +++ src/secrets/audit.test.ts | 269 +++++++++++++- src/secrets/audit.ts | 139 ++++++++ src/secrets/plan.test.ts | 16 + src/secrets/runtime-config-collectors-core.ts | 22 +- src/secrets/runtime.test.ts | 11 + src/secrets/storage-scan.ts | 27 ++ src/secrets/target-registry-data.ts | 13 + src/secrets/target-registry-pattern.test.ts | 11 + .../bot-native-commands.session-meta.test.ts | 2 +- 66 files changed, 2713 insertions(+), 299 deletions(-) create mode 100644 src/agents/model-auth-env-vars.ts create mode 100644 src/agents/model-auth-markers.test.ts create mode 100644 src/agents/model-auth-markers.ts create mode 100644 src/agents/models-config.file-mode.test.ts create mode 100644 src/agents/models-config.providers.auth-provenance.test.ts create mode 100644 src/agents/models-config.providers.cloudflare-ai-gateway.test.ts create mode 100644 src/agents/models-config.providers.discovery-auth.test.ts create mode 100644 src/agents/models-config.runtime-source-snapshot.test.ts create mode 100644 src/agents/models-config.write-serialization.test.ts create mode 100644 src/commands/models/load-config.test.ts diff --git a/.secrets.baseline b/.secrets.baseline index 8066ff84714..11a5d0f6cc3 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -9777,35 +9777,35 @@ "filename": "docs/gateway/configuration-reference.md", "hashed_secret": "ec3810e10fb78db55ce38b9c18d1c3eb1db739e0", "is_verified": false, - "line_number": 2039 + "line_number": 2041 }, { "type": "Secret Keyword", "filename": "docs/gateway/configuration-reference.md", "hashed_secret": "c1e6ee547fd492df1441ac492e8bb294974712bd", "is_verified": false, - "line_number": 2271 + "line_number": 2273 }, { "type": "Secret Keyword", "filename": "docs/gateway/configuration-reference.md", "hashed_secret": "45d676e7c6ab44cf4b8fa366ef2d8fccd3e6d6e6", "is_verified": false, - "line_number": 2399 + "line_number": 2401 }, { "type": "Secret Keyword", "filename": "docs/gateway/configuration-reference.md", "hashed_secret": "a219d7693c25cd2d93313512e200ff3eb374d281", "is_verified": false, - "line_number": 2652 + "line_number": 2654 }, { "type": "Secret Keyword", "filename": "docs/gateway/configuration-reference.md", "hashed_secret": "b6f56e5e92078ed7c078c46fbfeedcbe5719bc25", "is_verified": false, - "line_number": 2654 + "line_number": 2656 } ], "docs/gateway/configuration.md": [ @@ -14725,5 +14725,5 @@ } ] }, - "generated_at": "2026-03-07T11:12:54Z" + "generated_at": "2026-03-07T16:49:39Z" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 2971c411b94..d976e541ef6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,6 +110,7 @@ Docs: https://docs.openclaw.ai - TUI/session-key canonicalization: normalize `openclaw tui --session` values to lowercase so uppercase session names no longer drop real-time streaming updates due to gateway/TUI key mismatches. (#33866, #34013) thanks @lynnzc. - iMessage/echo loop hardening: strip leaked assistant-internal scaffolding from outbound iMessage replies, drop reflected assistant-content messages before they re-enter inbound processing, extend echo-cache text retention for delayed reflections, and suppress repeated loop traffic before it amplifies into queue overflow. (#33295) Thanks @joelnishanth. - Outbound/send config threading: pass resolved SecretRef config through outbound adapters and helper send paths so send flows do not reload unresolved runtime config. (#33987) Thanks @joshavant. +- Secrets/models.json persistence hardening: keep SecretRef-managed api keys + headers from persisting in generated models.json, expand audit/apply coverage, and harden marker handling/serialization. (#38955) Thanks @joshavant. - Sessions/subagent attachments: remove `attachments[].content.maxLength` from `sessions_spawn` schema to avoid llama.cpp GBNF repetition overflow, and preflight UTF-8 byte size before buffer allocation while keeping runtime file-size enforcement unchanged. (#33648) Thanks @anisoptera. - Runtime/tool-state stability: recover from dangling Anthropic `tool_use` after compaction, serialize long-running Discord handler runs without blocking new inbound events, and prevent stale busy snapshots from suppressing stuck-channel recovery. (from #33630, #33583) Thanks @kevinWangSheng and @theotarr. - ACP/Discord startup hardening: clean up stuck ACP worker children on gateway restart, unbind stale ACP thread bindings during Discord startup reconciliation, and add per-thread listener watchdog timeouts so wedged turns cannot block later messages. (#33699) Thanks @dutifulbob. diff --git a/docs/cli/agent.md b/docs/cli/agent.md index 0712a16661b..93c8d04b41a 100644 --- a/docs/cli/agent.md +++ b/docs/cli/agent.md @@ -22,3 +22,7 @@ openclaw agent --agent ops --message "Summarize logs" openclaw agent --session-id 1234 --message "Summarize inbox" --thinking medium openclaw agent --agent ops --message "Generate report" --deliver --reply-channel slack --reply-to "#reports" ``` + +## Notes + +- When this command triggers `models.json` regeneration, SecretRef-managed provider credentials are persisted as non-secret markers (for example env var names or `secretref-managed`), not resolved secret plaintext. diff --git a/docs/cli/models.md b/docs/cli/models.md index 700b562c353..e023784cc5e 100644 --- a/docs/cli/models.md +++ b/docs/cli/models.md @@ -38,6 +38,7 @@ Notes: - `models set ` accepts `provider/model` or an alias. - Model refs are parsed by splitting on the **first** `/`. If the model ID includes `/` (OpenRouter-style), include the provider prefix (example: `openrouter/moonshotai/kimi-k2`). - If you omit the provider, OpenClaw treats the input as an alias or a model for the **default provider** (only works when there is no `/` in the model ID). +- `models status` may show `marker()` in auth output for non-secret placeholders (for example `OPENAI_API_KEY`, `secretref-managed`, `minimax-oauth`, `qwen-oauth`, `ollama-local`) instead of masking them as secrets. ### `models status` diff --git a/docs/cli/secrets.md b/docs/cli/secrets.md index db5e9476c55..f90a5de8ec0 100644 --- a/docs/cli/secrets.md +++ b/docs/cli/secrets.md @@ -14,7 +14,7 @@ Use `openclaw secrets` to manage SecretRefs and keep the active runtime snapshot Command roles: - `reload`: gateway RPC (`secrets.reload`) that re-resolves refs and swaps runtime snapshot only on full success (no config writes). -- `audit`: read-only scan of configuration/auth stores and legacy residues for plaintext, unresolved refs, and precedence drift. +- `audit`: read-only scan of configuration/auth/generated-model stores and legacy residues for plaintext, unresolved refs, and precedence drift. - `configure`: interactive planner for provider setup, target mapping, and preflight (TTY required). - `apply`: execute a saved plan (`--dry-run` for validation only), then scrub targeted plaintext residues. @@ -62,8 +62,13 @@ Scan OpenClaw state for: - plaintext secret storage - unresolved refs - precedence drift (`auth-profiles.json` credentials shadowing `openclaw.json` refs) +- generated `agents/*/agent/models.json` residues (provider `apiKey` values and sensitive provider headers) - legacy residues (legacy auth store entries, OAuth reminders) +Header residue note: + +- Sensitive provider header detection is name-heuristic based (common auth/credential header names and fragments such as `authorization`, `x-api-key`, `token`, `secret`, `password`, and `credential`). + ```bash openclaw secrets audit openclaw secrets audit --check diff --git a/docs/concepts/models.md b/docs/concepts/models.md index 981bd95086c..2ad809d9599 100644 --- a/docs/concepts/models.md +++ b/docs/concepts/models.md @@ -212,6 +212,10 @@ is merged by default unless `models.mode` is set to `replace`. Merge mode precedence for matching provider IDs: -- Non-empty `apiKey`/`baseUrl` already present in the agent `models.json` win. +- Non-empty `baseUrl` already present in the agent `models.json` wins. +- Non-empty `apiKey` in the agent `models.json` wins only when that provider is not SecretRef-managed in current config/auth-profile context. +- SecretRef-managed provider `apiKey` values are refreshed from source markers (`ENV_VAR_NAME` for env refs, `secretref-managed` for file/exec refs) instead of persisting resolved secrets. - Empty or missing agent `apiKey`/`baseUrl` fall back to config `models.providers`. - Other provider fields are refreshed from config and normalized catalog data. + +This marker-based persistence applies whenever OpenClaw regenerates `models.json`, including command-driven paths like `openclaw agent`. diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index 5dbd1f40bba..249c35b7309 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -2004,7 +2004,9 @@ OpenClaw uses the pi-coding-agent model catalog. Add custom providers via `model - Use `authHeader: true` + `headers` for custom auth needs. - Override agent config root with `OPENCLAW_AGENT_DIR` (or `PI_CODING_AGENT_DIR`). - Merge precedence for matching provider IDs: - - Non-empty agent `models.json` `apiKey`/`baseUrl` win. + - Non-empty agent `models.json` `baseUrl` values win. + - Non-empty agent `apiKey` values win only when that provider is not SecretRef-managed in current config/auth-profile context. + - SecretRef-managed provider `apiKey` values are refreshed from source markers (`ENV_VAR_NAME` for env refs, `secretref-managed` for file/exec refs) instead of persisting resolved secrets. - Empty or missing agent `apiKey`/`baseUrl` fall back to `models.providers` in config. - Matching model `contextWindow`/`maxTokens` use the higher value between explicit config and implicit catalog values. - Use `models.mode: "replace"` when you want config to fully rewrite `models.json`. diff --git a/docs/gateway/secrets.md b/docs/gateway/secrets.md index 2956d53133e..3ef08267618 100644 --- a/docs/gateway/secrets.md +++ b/docs/gateway/secrets.md @@ -372,11 +372,16 @@ openclaw secrets audit --check Findings include: -- plaintext values at rest (`openclaw.json`, `auth-profiles.json`, `.env`) +- plaintext values at rest (`openclaw.json`, `auth-profiles.json`, `.env`, and generated `agents/*/agent/models.json`) +- plaintext sensitive provider header residues in generated `models.json` entries - unresolved refs - precedence shadowing (`auth-profiles.json` taking priority over `openclaw.json` refs) - legacy residues (`auth.json`, OAuth reminders) +Header residue note: + +- Sensitive provider header detection is name-heuristic based (common auth/credential header names and fragments such as `authorization`, `x-api-key`, `token`, `secret`, `password`, and `credential`). + ### `secrets configure` Interactive helper that: diff --git a/docs/reference/secretref-credential-surface.md b/docs/reference/secretref-credential-surface.md index d356e4f809e..dd1b5f1fd2f 100644 --- a/docs/reference/secretref-credential-surface.md +++ b/docs/reference/secretref-credential-surface.md @@ -23,6 +23,7 @@ Scope intent: [//]: # "secretref-supported-list-start" - `models.providers.*.apiKey` +- `models.providers.*.headers.*` - `skills.entries.*.apiKey` - `agents.defaults.memorySearch.remote.apiKey` - `agents.list[].memorySearch.remote.apiKey` @@ -98,6 +99,7 @@ Notes: - Auth-profile plan targets require `agentId`. - Plan entries target `profiles.*.key` / `profiles.*.token` and write sibling refs (`keyRef` / `tokenRef`). - Auth-profile refs are included in runtime resolution and audit coverage. +- For SecretRef-managed model providers, generated `agents/*/agent/models.json` entries persist non-secret markers (not resolved secret values) for `apiKey`/header surfaces. - For web search: - In explicit provider mode (`tools.web.search.provider` set), only the selected provider key is active. - In auto mode (`tools.web.search.provider` unset), `tools.web.search.apiKey` and provider-specific keys are active. diff --git a/docs/reference/secretref-user-supplied-credentials-matrix.json b/docs/reference/secretref-user-supplied-credentials-matrix.json index ac454a605a6..773ef8ab162 100644 --- a/docs/reference/secretref-user-supplied-credentials-matrix.json +++ b/docs/reference/secretref-user-supplied-credentials-matrix.json @@ -426,6 +426,13 @@ "secretShape": "secret_input", "optIn": true }, + { + "id": "models.providers.*.headers.*", + "configFile": "openclaw.json", + "path": "models.providers.*.headers.*", + "secretShape": "secret_input", + "optIn": true + }, { "id": "skills.entries.*.apiKey", "configFile": "openclaw.json", diff --git a/extensions/diffs/src/http.ts b/extensions/diffs/src/http.ts index df141ca0d38..445500b2340 100644 --- a/extensions/diffs/src/http.ts +++ b/extensions/diffs/src/http.ts @@ -176,12 +176,13 @@ function isLoopbackClientIp(clientIp: string): boolean { } function hasProxyForwardingHints(req: IncomingMessage): boolean { + const headers = req.headers ?? {}; return Boolean( - req.headers["x-forwarded-for"] || - req.headers["x-real-ip"] || - req.headers.forwarded || - req.headers["x-forwarded-host"] || - req.headers["x-forwarded-proto"], + headers["x-forwarded-for"] || + headers["x-real-ip"] || + headers.forwarded || + headers["x-forwarded-host"] || + headers["x-forwarded-proto"], ); } diff --git a/src/agents/model-auth-env-vars.ts b/src/agents/model-auth-env-vars.ts new file mode 100644 index 00000000000..c366138207c --- /dev/null +++ b/src/agents/model-auth-env-vars.ts @@ -0,0 +1,42 @@ +export const PROVIDER_ENV_API_KEY_CANDIDATES: Record = { + "github-copilot": ["COPILOT_GITHUB_TOKEN", "GH_TOKEN", "GITHUB_TOKEN"], + anthropic: ["ANTHROPIC_OAUTH_TOKEN", "ANTHROPIC_API_KEY"], + chutes: ["CHUTES_OAUTH_TOKEN", "CHUTES_API_KEY"], + zai: ["ZAI_API_KEY", "Z_AI_API_KEY"], + opencode: ["OPENCODE_API_KEY", "OPENCODE_ZEN_API_KEY"], + "qwen-portal": ["QWEN_OAUTH_TOKEN", "QWEN_PORTAL_API_KEY"], + volcengine: ["VOLCANO_ENGINE_API_KEY"], + "volcengine-plan": ["VOLCANO_ENGINE_API_KEY"], + byteplus: ["BYTEPLUS_API_KEY"], + "byteplus-plan": ["BYTEPLUS_API_KEY"], + "minimax-portal": ["MINIMAX_OAUTH_TOKEN", "MINIMAX_API_KEY"], + "kimi-coding": ["KIMI_API_KEY", "KIMICODE_API_KEY"], + huggingface: ["HUGGINGFACE_HUB_TOKEN", "HF_TOKEN"], + openai: ["OPENAI_API_KEY"], + google: ["GEMINI_API_KEY"], + voyage: ["VOYAGE_API_KEY"], + groq: ["GROQ_API_KEY"], + deepgram: ["DEEPGRAM_API_KEY"], + cerebras: ["CEREBRAS_API_KEY"], + xai: ["XAI_API_KEY"], + openrouter: ["OPENROUTER_API_KEY"], + litellm: ["LITELLM_API_KEY"], + "vercel-ai-gateway": ["AI_GATEWAY_API_KEY"], + "cloudflare-ai-gateway": ["CLOUDFLARE_AI_GATEWAY_API_KEY"], + moonshot: ["MOONSHOT_API_KEY"], + minimax: ["MINIMAX_API_KEY"], + nvidia: ["NVIDIA_API_KEY"], + xiaomi: ["XIAOMI_API_KEY"], + synthetic: ["SYNTHETIC_API_KEY"], + venice: ["VENICE_API_KEY"], + mistral: ["MISTRAL_API_KEY"], + together: ["TOGETHER_API_KEY"], + qianfan: ["QIANFAN_API_KEY"], + ollama: ["OLLAMA_API_KEY"], + vllm: ["VLLM_API_KEY"], + kilocode: ["KILOCODE_API_KEY"], +}; + +export function listKnownProviderEnvApiKeyNames(): string[] { + return [...new Set(Object.values(PROVIDER_ENV_API_KEY_CANDIDATES).flat())]; +} diff --git a/src/agents/model-auth-markers.test.ts b/src/agents/model-auth-markers.test.ts new file mode 100644 index 00000000000..e2225588df7 --- /dev/null +++ b/src/agents/model-auth-markers.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from "vitest"; +import { listKnownProviderEnvApiKeyNames } from "./model-auth-env-vars.js"; +import { isNonSecretApiKeyMarker, NON_ENV_SECRETREF_MARKER } from "./model-auth-markers.js"; + +describe("model auth markers", () => { + it("recognizes explicit non-secret markers", () => { + expect(isNonSecretApiKeyMarker(NON_ENV_SECRETREF_MARKER)).toBe(true); + expect(isNonSecretApiKeyMarker("qwen-oauth")).toBe(true); + expect(isNonSecretApiKeyMarker("ollama-local")).toBe(true); + }); + + it("recognizes known env marker names but not arbitrary all-caps keys", () => { + expect(isNonSecretApiKeyMarker("OPENAI_API_KEY")).toBe(true); + expect(isNonSecretApiKeyMarker("ALLCAPS_EXAMPLE")).toBe(false); + }); + + it("recognizes all built-in provider env marker names", () => { + for (const envVarName of listKnownProviderEnvApiKeyNames()) { + expect(isNonSecretApiKeyMarker(envVarName)).toBe(true); + } + }); + + it("can exclude env marker-name interpretation for display-only paths", () => { + expect(isNonSecretApiKeyMarker("OPENAI_API_KEY", { includeEnvVarName: false })).toBe(false); + }); +}); diff --git a/src/agents/model-auth-markers.ts b/src/agents/model-auth-markers.ts new file mode 100644 index 00000000000..0b3b4960eb8 --- /dev/null +++ b/src/agents/model-auth-markers.ts @@ -0,0 +1,80 @@ +import type { SecretRefSource } from "../config/types.secrets.js"; +import { listKnownProviderEnvApiKeyNames } from "./model-auth-env-vars.js"; + +export const MINIMAX_OAUTH_MARKER = "minimax-oauth"; +export const QWEN_OAUTH_MARKER = "qwen-oauth"; +export const OLLAMA_LOCAL_AUTH_MARKER = "ollama-local"; +export const NON_ENV_SECRETREF_MARKER = "secretref-managed"; // pragma: allowlist secret +export const SECRETREF_ENV_HEADER_MARKER_PREFIX = "secretref-env:"; // pragma: allowlist secret + +const AWS_SDK_ENV_MARKERS = new Set([ + "AWS_BEARER_TOKEN_BEDROCK", + "AWS_ACCESS_KEY_ID", + "AWS_PROFILE", +]); + +// Legacy marker names kept for backward compatibility with existing models.json files. +const LEGACY_ENV_API_KEY_MARKERS = [ + "GOOGLE_API_KEY", + "DEEPSEEK_API_KEY", + "PERPLEXITY_API_KEY", + "FIREWORKS_API_KEY", + "NOVITA_API_KEY", + "AZURE_OPENAI_API_KEY", + "AZURE_API_KEY", + "MINIMAX_CODE_PLAN_KEY", +]; + +const KNOWN_ENV_API_KEY_MARKERS = new Set([ + ...listKnownProviderEnvApiKeyNames(), + ...LEGACY_ENV_API_KEY_MARKERS, + ...AWS_SDK_ENV_MARKERS, +]); + +export function isAwsSdkAuthMarker(value: string): boolean { + return AWS_SDK_ENV_MARKERS.has(value.trim()); +} + +export function resolveNonEnvSecretRefApiKeyMarker(_source: SecretRefSource): string { + return NON_ENV_SECRETREF_MARKER; +} + +export function resolveNonEnvSecretRefHeaderValueMarker(_source: SecretRefSource): string { + return NON_ENV_SECRETREF_MARKER; +} + +export function resolveEnvSecretRefHeaderValueMarker(envVarName: string): string { + return `${SECRETREF_ENV_HEADER_MARKER_PREFIX}${envVarName.trim()}`; +} + +export function isSecretRefHeaderValueMarker(value: string): boolean { + const trimmed = value.trim(); + return ( + trimmed === NON_ENV_SECRETREF_MARKER || trimmed.startsWith(SECRETREF_ENV_HEADER_MARKER_PREFIX) + ); +} + +export function isNonSecretApiKeyMarker( + value: string, + opts?: { includeEnvVarName?: boolean }, +): boolean { + const trimmed = value.trim(); + if (!trimmed) { + return false; + } + const isKnownMarker = + trimmed === MINIMAX_OAUTH_MARKER || + trimmed === QWEN_OAUTH_MARKER || + trimmed === OLLAMA_LOCAL_AUTH_MARKER || + trimmed === NON_ENV_SECRETREF_MARKER || + isAwsSdkAuthMarker(trimmed); + if (isKnownMarker) { + return true; + } + if (opts?.includeEnvVarName === false) { + return false; + } + // Do not treat arbitrary ALL_CAPS values as markers; only recognize the + // known env-var markers we intentionally persist for compatibility. + return KNOWN_ENV_API_KEY_MARKERS.has(trimmed); +} diff --git a/src/agents/model-auth.ts b/src/agents/model-auth.ts index 68a117c96a9..b8b0ac9336b 100644 --- a/src/agents/model-auth.ts +++ b/src/agents/model-auth.ts @@ -16,6 +16,8 @@ import { resolveAuthProfileOrder, resolveAuthStorePathForDisplay, } from "./auth-profiles.js"; +import { PROVIDER_ENV_API_KEY_CANDIDATES } from "./model-auth-env-vars.js"; +import { OLLAMA_LOCAL_AUTH_MARKER } from "./model-auth-markers.js"; import { normalizeProviderId } from "./model-selection.js"; export { ensureAuthProfileStore, resolveAuthProfileOrder } from "./auth-profiles.js"; @@ -90,7 +92,7 @@ function resolveSyntheticLocalProviderAuth(params: { } return { - apiKey: "ollama-local", // pragma: allowlist secret + apiKey: OLLAMA_LOCAL_AUTH_MARKER, source: "models.providers.ollama (synthetic local key)", mode: "api-key", }; @@ -281,20 +283,14 @@ export function resolveEnvApiKey(provider: string): EnvApiKeyResult | null { return { apiKey: value, source }; }; - if (normalized === "github-copilot") { - return pick("COPILOT_GITHUB_TOKEN") ?? pick("GH_TOKEN") ?? pick("GITHUB_TOKEN"); - } - - if (normalized === "anthropic") { - return pick("ANTHROPIC_OAUTH_TOKEN") ?? pick("ANTHROPIC_API_KEY"); - } - - if (normalized === "chutes") { - return pick("CHUTES_OAUTH_TOKEN") ?? pick("CHUTES_API_KEY"); - } - - if (normalized === "zai") { - return pick("ZAI_API_KEY") ?? pick("Z_AI_API_KEY"); + const candidates = PROVIDER_ENV_API_KEY_CANDIDATES[normalized]; + if (candidates) { + for (const envVar of candidates) { + const resolved = pick(envVar); + if (resolved) { + return resolved; + } + } } if (normalized === "google-vertex") { @@ -304,65 +300,7 @@ export function resolveEnvApiKey(provider: string): EnvApiKeyResult | null { } return { apiKey: envKey, source: "gcloud adc" }; } - - if (normalized === "opencode") { - return pick("OPENCODE_API_KEY") ?? pick("OPENCODE_ZEN_API_KEY"); - } - - if (normalized === "qwen-portal") { - return pick("QWEN_OAUTH_TOKEN") ?? pick("QWEN_PORTAL_API_KEY"); - } - - if (normalized === "volcengine" || normalized === "volcengine-plan") { - return pick("VOLCANO_ENGINE_API_KEY"); - } - - if (normalized === "byteplus" || normalized === "byteplus-plan") { - return pick("BYTEPLUS_API_KEY"); - } - if (normalized === "minimax-portal") { - return pick("MINIMAX_OAUTH_TOKEN") ?? pick("MINIMAX_API_KEY"); - } - - if (normalized === "kimi-coding") { - return pick("KIMI_API_KEY") ?? pick("KIMICODE_API_KEY"); - } - - if (normalized === "huggingface") { - return pick("HUGGINGFACE_HUB_TOKEN") ?? pick("HF_TOKEN"); - } - - const envMap: Record = { - openai: "OPENAI_API_KEY", - google: "GEMINI_API_KEY", - voyage: "VOYAGE_API_KEY", - groq: "GROQ_API_KEY", - deepgram: "DEEPGRAM_API_KEY", - cerebras: "CEREBRAS_API_KEY", - xai: "XAI_API_KEY", - openrouter: "OPENROUTER_API_KEY", - litellm: "LITELLM_API_KEY", - "vercel-ai-gateway": "AI_GATEWAY_API_KEY", - "cloudflare-ai-gateway": "CLOUDFLARE_AI_GATEWAY_API_KEY", - moonshot: "MOONSHOT_API_KEY", - minimax: "MINIMAX_API_KEY", - nvidia: "NVIDIA_API_KEY", - xiaomi: "XIAOMI_API_KEY", - synthetic: "SYNTHETIC_API_KEY", - venice: "VENICE_API_KEY", - mistral: "MISTRAL_API_KEY", - opencode: "OPENCODE_API_KEY", - together: "TOGETHER_API_KEY", - qianfan: "QIANFAN_API_KEY", - ollama: "OLLAMA_API_KEY", - vllm: "VLLM_API_KEY", - kilocode: "KILOCODE_API_KEY", - }; - const envVar = envMap[normalized]; - if (!envVar) { - return null; - } - return pick(envVar); + return null; } export function resolveModelAuthMode( diff --git a/src/agents/models-config.file-mode.test.ts b/src/agents/models-config.file-mode.test.ts new file mode 100644 index 00000000000..af5719082da --- /dev/null +++ b/src/agents/models-config.file-mode.test.ts @@ -0,0 +1,43 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { resolveOpenClawAgentDir } from "./agent-paths.js"; +import { + CUSTOM_PROXY_MODELS_CONFIG, + installModelsConfigTestHooks, + withModelsTempHome as withTempHome, +} from "./models-config.e2e-harness.js"; +import { ensureOpenClawModelsJson } from "./models-config.js"; + +installModelsConfigTestHooks(); + +describe("models-config file mode", () => { + it("writes models.json with mode 0600", async () => { + if (process.platform === "win32") { + return; + } + await withTempHome(async () => { + await ensureOpenClawModelsJson(CUSTOM_PROXY_MODELS_CONFIG); + const modelsPath = path.join(resolveOpenClawAgentDir(), "models.json"); + const stat = await fs.stat(modelsPath); + expect(stat.mode & 0o777).toBe(0o600); + }); + }); + + it("repairs models.json mode to 0600 on no-content-change paths", async () => { + if (process.platform === "win32") { + return; + } + await withTempHome(async () => { + await ensureOpenClawModelsJson(CUSTOM_PROXY_MODELS_CONFIG); + const modelsPath = path.join(resolveOpenClawAgentDir(), "models.json"); + await fs.chmod(modelsPath, 0o644); + + const result = await ensureOpenClawModelsJson(CUSTOM_PROXY_MODELS_CONFIG); + expect(result.wrote).toBe(false); + + const stat = await fs.stat(modelsPath); + expect(stat.mode & 0o777).toBe(0o600); + }); + }); +}); diff --git a/src/agents/models-config.fills-missing-provider-apikey-from-env-var.test.ts b/src/agents/models-config.fills-missing-provider-apikey-from-env-var.test.ts index bb3ca7a7cbe..ff7f06b5c7f 100644 --- a/src/agents/models-config.fills-missing-provider-apikey-from-env-var.test.ts +++ b/src/agents/models-config.fills-missing-provider-apikey-from-env-var.test.ts @@ -4,6 +4,7 @@ import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; import { validateConfigObject } from "../config/validation.js"; import { resolveOpenClawAgentDir } from "./agent-paths.js"; +import { NON_ENV_SECRETREF_MARKER } from "./model-auth-markers.js"; import { CUSTOM_PROXY_MODELS_CONFIG, installModelsConfigTestHooks, @@ -166,7 +167,7 @@ describe("models-config", () => { const parsed = await readGeneratedModelsJson<{ providers: Record }>; }>(); - expect(parsed.providers.minimax?.apiKey).toBe("MINIMAX_API_KEY"); + expect(parsed.providers.minimax?.apiKey).toBe("MINIMAX_API_KEY"); // pragma: allowlist secret const ids = parsed.providers.minimax?.models?.map((model) => model.id); expect(ids).toContain("MiniMax-VL-01"); }); @@ -220,6 +221,117 @@ describe("models-config", () => { }); }); + it("replaces stale merged apiKey when provider is SecretRef-managed in current config", async () => { + await withTempHome(async () => { + await writeAgentModelsJson({ + providers: { + custom: { + baseUrl: "https://agent.example/v1", + apiKey: "STALE_AGENT_KEY", // pragma: allowlist secret + api: "openai-responses", + models: [{ id: "agent-model", name: "Agent model", input: ["text"] }], + }, + }, + }); + await ensureOpenClawModelsJson({ + models: { + mode: "merge", + providers: { + custom: { + ...createMergeConfigProvider(), + apiKey: { source: "env", provider: "default", id: "CUSTOM_PROVIDER_API_KEY" }, // pragma: allowlist secret + }, + }, + }, + }); + + const parsed = await readGeneratedModelsJson<{ + providers: Record; + }>(); + expect(parsed.providers.custom?.apiKey).toBe("CUSTOM_PROVIDER_API_KEY"); // pragma: allowlist secret + expect(parsed.providers.custom?.baseUrl).toBe("https://agent.example/v1"); + }); + }); + + it("replaces stale merged apiKey when provider is SecretRef-managed via auth-profiles", async () => { + await withTempHome(async () => { + const agentDir = resolveOpenClawAgentDir(); + await fs.mkdir(agentDir, { recursive: true }); + await fs.writeFile( + path.join(agentDir, "auth-profiles.json"), + `${JSON.stringify( + { + version: 1, + profiles: { + "minimax:default": { + type: "api_key", + provider: "minimax", + keyRef: { source: "env", provider: "default", id: "MINIMAX_API_KEY" }, // pragma: allowlist secret + }, + }, + }, + null, + 2, + )}\n`, + "utf8", + ); + await writeAgentModelsJson({ + providers: { + minimax: { + baseUrl: "https://api.minimax.io/anthropic", + apiKey: "STALE_AGENT_KEY", // pragma: allowlist secret + api: "anthropic-messages", + models: [{ id: "MiniMax-M2.5", name: "MiniMax M2.5", input: ["text"] }], + }, + }, + }); + + await ensureOpenClawModelsJson({ + models: { + mode: "merge", + providers: {}, + }, + }); + + const parsed = await readGeneratedModelsJson<{ + providers: Record; + }>(); + expect(parsed.providers.minimax?.apiKey).toBe("MINIMAX_API_KEY"); // pragma: allowlist secret + }); + }); + + it("replaces stale non-env marker when provider transitions back to plaintext config", async () => { + await withTempHome(async () => { + await writeAgentModelsJson({ + providers: { + custom: { + baseUrl: "https://agent.example/v1", + apiKey: NON_ENV_SECRETREF_MARKER, + api: "openai-responses", + models: [{ id: "agent-model", name: "Agent model", input: ["text"] }], + }, + }, + }); + + await ensureOpenClawModelsJson({ + models: { + mode: "merge", + providers: { + custom: { + ...createMergeConfigProvider(), + apiKey: "ALLCAPS_SAMPLE", // pragma: allowlist secret + }, + }, + }, + }); + + const parsed = await readGeneratedModelsJson<{ + providers: Record; + }>(); + expect(parsed.providers.custom?.apiKey).toBe("ALLCAPS_SAMPLE"); + }); + }); + it("uses config apiKey/baseUrl when existing agent values are empty", async () => { await withTempHome(async () => { const parsed = await runCustomProviderMergeTest({ diff --git a/src/agents/models-config.providers.auth-provenance.test.ts b/src/agents/models-config.providers.auth-provenance.test.ts new file mode 100644 index 00000000000..0a606762d66 --- /dev/null +++ b/src/agents/models-config.providers.auth-provenance.test.ts @@ -0,0 +1,121 @@ +import { mkdtempSync } from "node:fs"; +import { writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { captureEnv } from "../test-utils/env.js"; +import { + MINIMAX_OAUTH_MARKER, + NON_ENV_SECRETREF_MARKER, + QWEN_OAUTH_MARKER, +} from "./model-auth-markers.js"; +import { resolveImplicitProviders } from "./models-config.providers.js"; + +describe("models-config provider auth provenance", () => { + it("persists env keyRef and tokenRef auth profiles as env var markers", async () => { + const agentDir = mkdtempSync(join(tmpdir(), "openclaw-test-")); + const envSnapshot = captureEnv(["VOLCANO_ENGINE_API_KEY", "TOGETHER_API_KEY"]); + delete process.env.VOLCANO_ENGINE_API_KEY; + delete process.env.TOGETHER_API_KEY; + await writeFile( + join(agentDir, "auth-profiles.json"), + JSON.stringify( + { + version: 1, + profiles: { + "volcengine:default": { + type: "api_key", + provider: "volcengine", + keyRef: { source: "env", provider: "default", id: "VOLCANO_ENGINE_API_KEY" }, + }, + "together:default": { + type: "token", + provider: "together", + tokenRef: { source: "env", provider: "default", id: "TOGETHER_API_KEY" }, + }, + }, + }, + null, + 2, + ), + "utf8", + ); + try { + const providers = await resolveImplicitProviders({ agentDir }); + expect(providers?.volcengine?.apiKey).toBe("VOLCANO_ENGINE_API_KEY"); + expect(providers?.["volcengine-plan"]?.apiKey).toBe("VOLCANO_ENGINE_API_KEY"); + expect(providers?.together?.apiKey).toBe("TOGETHER_API_KEY"); + } finally { + envSnapshot.restore(); + } + }); + + it("uses non-env marker for ref-managed profiles even when runtime plaintext is present", async () => { + const agentDir = mkdtempSync(join(tmpdir(), "openclaw-test-")); + await writeFile( + join(agentDir, "auth-profiles.json"), + JSON.stringify( + { + version: 1, + profiles: { + "byteplus:default": { + type: "api_key", + provider: "byteplus", + key: "sk-runtime-resolved-byteplus", + keyRef: { source: "file", provider: "vault", id: "/byteplus/apiKey" }, + }, + "together:default": { + type: "token", + provider: "together", + token: "tok-runtime-resolved-together", + tokenRef: { source: "exec", provider: "vault", id: "providers/together/token" }, + }, + }, + }, + null, + 2, + ), + "utf8", + ); + + const providers = await resolveImplicitProviders({ agentDir }); + expect(providers?.byteplus?.apiKey).toBe(NON_ENV_SECRETREF_MARKER); + expect(providers?.["byteplus-plan"]?.apiKey).toBe(NON_ENV_SECRETREF_MARKER); + expect(providers?.together?.apiKey).toBe(NON_ENV_SECRETREF_MARKER); + }); + + it("keeps oauth compatibility markers for minimax-portal and qwen-portal", async () => { + const agentDir = mkdtempSync(join(tmpdir(), "openclaw-test-")); + await writeFile( + join(agentDir, "auth-profiles.json"), + JSON.stringify( + { + version: 1, + profiles: { + "minimax-portal:default": { + type: "oauth", + provider: "minimax-portal", + access: "access-token", + refresh: "refresh-token", + expires: Date.now() + 60_000, + }, + "qwen-portal:default": { + type: "oauth", + provider: "qwen-portal", + access: "access-token", + refresh: "refresh-token", + expires: Date.now() + 60_000, + }, + }, + }, + null, + 2, + ), + "utf8", + ); + + const providers = await resolveImplicitProviders({ agentDir }); + expect(providers?.["minimax-portal"]?.apiKey).toBe(MINIMAX_OAUTH_MARKER); + expect(providers?.["qwen-portal"]?.apiKey).toBe(QWEN_OAUTH_MARKER); + }); +}); diff --git a/src/agents/models-config.providers.cloudflare-ai-gateway.test.ts b/src/agents/models-config.providers.cloudflare-ai-gateway.test.ts new file mode 100644 index 00000000000..82a16dbcbee --- /dev/null +++ b/src/agents/models-config.providers.cloudflare-ai-gateway.test.ts @@ -0,0 +1,76 @@ +import { mkdtempSync } from "node:fs"; +import { writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { captureEnv } from "../test-utils/env.js"; +import { NON_ENV_SECRETREF_MARKER } from "./model-auth-markers.js"; +import { resolveImplicitProviders } from "./models-config.providers.js"; + +describe("cloudflare-ai-gateway profile provenance", () => { + it("prefers env keyRef marker over runtime plaintext for persistence", async () => { + const agentDir = mkdtempSync(join(tmpdir(), "openclaw-test-")); + const envSnapshot = captureEnv(["CLOUDFLARE_AI_GATEWAY_API_KEY"]); + delete process.env.CLOUDFLARE_AI_GATEWAY_API_KEY; + + await writeFile( + join(agentDir, "auth-profiles.json"), + JSON.stringify( + { + version: 1, + profiles: { + "cloudflare-ai-gateway:default": { + type: "api_key", + provider: "cloudflare-ai-gateway", + key: "sk-runtime-cloudflare", + keyRef: { source: "env", provider: "default", id: "CLOUDFLARE_AI_GATEWAY_API_KEY" }, + metadata: { + accountId: "acct_123", + gatewayId: "gateway_456", + }, + }, + }, + }, + null, + 2, + ), + "utf8", + ); + try { + const providers = await resolveImplicitProviders({ agentDir }); + expect(providers?.["cloudflare-ai-gateway"]?.apiKey).toBe("CLOUDFLARE_AI_GATEWAY_API_KEY"); + } finally { + envSnapshot.restore(); + } + }); + + it("uses non-env marker for non-env keyRef cloudflare profiles", async () => { + const agentDir = mkdtempSync(join(tmpdir(), "openclaw-test-")); + await writeFile( + join(agentDir, "auth-profiles.json"), + JSON.stringify( + { + version: 1, + profiles: { + "cloudflare-ai-gateway:default": { + type: "api_key", + provider: "cloudflare-ai-gateway", + key: "sk-runtime-cloudflare", + keyRef: { source: "file", provider: "vault", id: "/cloudflare/apiKey" }, + metadata: { + accountId: "acct_123", + gatewayId: "gateway_456", + }, + }, + }, + }, + null, + 2, + ), + "utf8", + ); + + const providers = await resolveImplicitProviders({ agentDir }); + expect(providers?.["cloudflare-ai-gateway"]?.apiKey).toBe(NON_ENV_SECRETREF_MARKER); + }); +}); diff --git a/src/agents/models-config.providers.discovery-auth.test.ts b/src/agents/models-config.providers.discovery-auth.test.ts new file mode 100644 index 00000000000..6e8ebfbc0ac --- /dev/null +++ b/src/agents/models-config.providers.discovery-auth.test.ts @@ -0,0 +1,140 @@ +import { mkdtempSync } from "node:fs"; +import { writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { NON_ENV_SECRETREF_MARKER } from "./model-auth-markers.js"; +import { resolveImplicitProviders } from "./models-config.providers.js"; + +describe("provider discovery auth marker guardrails", () => { + let originalVitest: string | undefined; + let originalNodeEnv: string | undefined; + let originalFetch: typeof globalThis.fetch | undefined; + + afterEach(() => { + if (originalVitest !== undefined) { + process.env.VITEST = originalVitest; + } else { + delete process.env.VITEST; + } + if (originalNodeEnv !== undefined) { + process.env.NODE_ENV = originalNodeEnv; + } else { + delete process.env.NODE_ENV; + } + if (originalFetch) { + globalThis.fetch = originalFetch; + } + }); + + function enableDiscovery() { + originalVitest = process.env.VITEST; + originalNodeEnv = process.env.NODE_ENV; + originalFetch = globalThis.fetch; + delete process.env.VITEST; + delete process.env.NODE_ENV; + } + + it("does not send marker value as vLLM bearer token during discovery", async () => { + enableDiscovery(); + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ data: [] }), + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const agentDir = mkdtempSync(join(tmpdir(), "openclaw-test-")); + await writeFile( + join(agentDir, "auth-profiles.json"), + JSON.stringify( + { + version: 1, + profiles: { + "vllm:default": { + type: "api_key", + provider: "vllm", + keyRef: { source: "file", provider: "vault", id: "/vllm/apiKey" }, + }, + }, + }, + null, + 2, + ), + "utf8", + ); + + const providers = await resolveImplicitProviders({ agentDir }); + expect(providers?.vllm?.apiKey).toBe(NON_ENV_SECRETREF_MARKER); + const request = fetchMock.mock.calls[0]?.[1] as + | { headers?: Record } + | undefined; + expect(request?.headers?.Authorization).toBeUndefined(); + }); + + it("does not call Hugging Face discovery with marker-backed credentials", async () => { + enableDiscovery(); + const fetchMock = vi.fn(); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const agentDir = mkdtempSync(join(tmpdir(), "openclaw-test-")); + await writeFile( + join(agentDir, "auth-profiles.json"), + JSON.stringify( + { + version: 1, + profiles: { + "huggingface:default": { + type: "api_key", + provider: "huggingface", + keyRef: { source: "exec", provider: "vault", id: "providers/hf/token" }, + }, + }, + }, + null, + 2, + ), + "utf8", + ); + + const providers = await resolveImplicitProviders({ agentDir }); + expect(providers?.huggingface?.apiKey).toBe(NON_ENV_SECRETREF_MARKER); + const huggingfaceCalls = fetchMock.mock.calls.filter(([url]) => + String(url).includes("router.huggingface.co"), + ); + expect(huggingfaceCalls).toHaveLength(0); + }); + + it("keeps all-caps plaintext API keys for authenticated discovery", async () => { + enableDiscovery(); + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ data: [{ id: "vllm/test-model" }] }), + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const agentDir = mkdtempSync(join(tmpdir(), "openclaw-test-")); + await writeFile( + join(agentDir, "auth-profiles.json"), + JSON.stringify( + { + version: 1, + profiles: { + "vllm:default": { + type: "api_key", + provider: "vllm", + key: "ALLCAPS_SAMPLE", + }, + }, + }, + null, + 2, + ), + "utf8", + ); + + await resolveImplicitProviders({ agentDir }); + const vllmCall = fetchMock.mock.calls.find(([url]) => String(url).includes(":8000")); + const request = vllmCall?.[1] as { headers?: Record } | undefined; + expect(request?.headers?.Authorization).toBe("Bearer ALLCAPS_SAMPLE"); + }); +}); diff --git a/src/agents/models-config.providers.normalize-keys.test.ts b/src/agents/models-config.providers.normalize-keys.test.ts index cccd54851d8..1271b30faed 100644 --- a/src/agents/models-config.providers.normalize-keys.test.ts +++ b/src/agents/models-config.providers.normalize-keys.test.ts @@ -3,6 +3,7 @@ import os from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; +import { NON_ENV_SECRETREF_MARKER } from "./model-auth-markers.js"; import { normalizeProviders } from "./models-config.providers.js"; describe("normalizeProviders", () => { @@ -73,4 +74,30 @@ describe("normalizeProviders", () => { await fs.rm(agentDir, { recursive: true, force: true }); } }); + + it("normalizes SecretRef-backed provider headers to non-secret marker values", async () => { + const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-agent-")); + try { + const providers: NonNullable["providers"]> = { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + headers: { + Authorization: { source: "env", provider: "default", id: "OPENAI_HEADER_TOKEN" }, + "X-Tenant-Token": { source: "file", provider: "vault", id: "/openai/token" }, + }, + models: [], + }, + }; + + const normalized = normalizeProviders({ + providers, + agentDir, + }); + expect(normalized?.openai?.headers?.Authorization).toBe("secretref-env:OPENAI_HEADER_TOKEN"); + expect(normalized?.openai?.headers?.["X-Tenant-Token"]).toBe(NON_ENV_SECRETREF_MARKER); + } finally { + await fs.rm(agentDir, { recursive: true, force: true }); + } + }); }); diff --git a/src/agents/models-config.providers.ts b/src/agents/models-config.providers.ts index 1c7ad06699c..62bdf70f04e 100644 --- a/src/agents/models-config.providers.ts +++ b/src/agents/models-config.providers.ts @@ -1,6 +1,6 @@ import type { OpenClawConfig } from "../config/config.js"; import type { ModelDefinitionConfig } from "../config/types.models.js"; -import { coerceSecretRef } from "../config/types.secrets.js"; +import { coerceSecretRef, resolveSecretInputRef } from "../config/types.secrets.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { DEFAULT_COPILOT_API_BASE_URL, @@ -41,6 +41,15 @@ import { buildHuggingfaceModelDefinition, } from "./huggingface-models.js"; import { discoverKilocodeModels } from "./kilocode-models.js"; +import { + MINIMAX_OAUTH_MARKER, + OLLAMA_LOCAL_AUTH_MARKER, + QWEN_OAUTH_MARKER, + isNonSecretApiKeyMarker, + resolveNonEnvSecretRefApiKeyMarker, + resolveNonEnvSecretRefHeaderValueMarker, + resolveEnvSecretRefHeaderValueMarker, +} from "./model-auth-markers.js"; import { resolveAwsSdkEnvVarName, resolveEnvApiKey } from "./model-auth.js"; import { OLLAMA_NATIVE_BASE_URL } from "./ollama-stream.js"; import { @@ -63,7 +72,6 @@ const MINIMAX_DEFAULT_MODEL_ID = "MiniMax-M2.5"; const MINIMAX_DEFAULT_VISION_MODEL_ID = "MiniMax-VL-01"; const MINIMAX_DEFAULT_CONTEXT_WINDOW = 200000; const MINIMAX_DEFAULT_MAX_TOKENS = 8192; -const MINIMAX_OAUTH_PLACEHOLDER = "minimax-oauth"; // Pricing per 1M tokens (USD) — https://platform.minimaxi.com/document/Price const MINIMAX_API_COST = { input: 0.3, @@ -133,7 +141,6 @@ const KIMI_CODING_DEFAULT_COST = { }; const QWEN_PORTAL_BASE_URL = "https://portal.qwen.ai/v1"; -const QWEN_PORTAL_OAUTH_PLACEHOLDER = "qwen-oauth"; const QWEN_PORTAL_DEFAULT_CONTEXT_WINDOW = 128000; const QWEN_PORTAL_DEFAULT_MAX_TOKENS = 8192; const QWEN_PORTAL_DEFAULT_COST = { @@ -404,35 +411,125 @@ function resolveAwsSdkApiKeyVarName(): string { return resolveAwsSdkEnvVarName() ?? "AWS_PROFILE"; } +function normalizeHeaderValues(params: { + headers: ProviderConfig["headers"] | undefined; + secretDefaults: + | { + env?: string; + file?: string; + exec?: string; + } + | undefined; +}): { headers: ProviderConfig["headers"] | undefined; mutated: boolean } { + const { headers } = params; + if (!headers) { + return { headers, mutated: false }; + } + let mutated = false; + const nextHeaders: Record[string]> = {}; + for (const [headerName, headerValue] of Object.entries(headers)) { + const resolvedRef = resolveSecretInputRef({ + value: headerValue, + defaults: params.secretDefaults, + }).ref; + if (!resolvedRef || !resolvedRef.id.trim()) { + nextHeaders[headerName] = headerValue; + continue; + } + mutated = true; + nextHeaders[headerName] = + resolvedRef.source === "env" + ? resolveEnvSecretRefHeaderValueMarker(resolvedRef.id) + : resolveNonEnvSecretRefHeaderValueMarker(resolvedRef.source); + } + if (!mutated) { + return { headers, mutated: false }; + } + return { headers: nextHeaders, mutated: true }; +} + +type ProfileApiKeyResolution = { + apiKey: string; + source: "plaintext" | "env-ref" | "non-env-ref"; + /** Optional secret value that may be used for provider discovery only. */ + discoveryApiKey?: string; +}; + +function toDiscoveryApiKey(value: string | undefined): string | undefined { + const trimmed = value?.trim(); + if (!trimmed || isNonSecretApiKeyMarker(trimmed)) { + return undefined; + } + return trimmed; +} + +function resolveApiKeyFromCredential( + cred: ReturnType["profiles"][string] | undefined, +): ProfileApiKeyResolution | undefined { + if (!cred) { + return undefined; + } + if (cred.type === "api_key") { + const keyRef = coerceSecretRef(cred.keyRef); + if (keyRef && keyRef.id.trim()) { + if (keyRef.source === "env") { + const envVar = keyRef.id.trim(); + return { + apiKey: envVar, + source: "env-ref", + discoveryApiKey: toDiscoveryApiKey(process.env[envVar]), + }; + } + return { + apiKey: resolveNonEnvSecretRefApiKeyMarker(keyRef.source), + source: "non-env-ref", + }; + } + if (cred.key?.trim()) { + return { + apiKey: cred.key, + source: "plaintext", + discoveryApiKey: toDiscoveryApiKey(cred.key), + }; + } + return undefined; + } + if (cred.type === "token") { + const tokenRef = coerceSecretRef(cred.tokenRef); + if (tokenRef && tokenRef.id.trim()) { + if (tokenRef.source === "env") { + const envVar = tokenRef.id.trim(); + return { + apiKey: envVar, + source: "env-ref", + discoveryApiKey: toDiscoveryApiKey(process.env[envVar]), + }; + } + return { + apiKey: resolveNonEnvSecretRefApiKeyMarker(tokenRef.source), + source: "non-env-ref", + }; + } + if (cred.token?.trim()) { + return { + apiKey: cred.token, + source: "plaintext", + discoveryApiKey: toDiscoveryApiKey(cred.token), + }; + } + } + return undefined; +} + function resolveApiKeyFromProfiles(params: { provider: string; store: ReturnType; -}): string | undefined { +}): ProfileApiKeyResolution | undefined { const ids = listProfilesForProvider(params.store, params.provider); for (const id of ids) { - const cred = params.store.profiles[id]; - if (!cred) { - continue; - } - if (cred.type === "api_key") { - if (cred.key?.trim()) { - return cred.key; - } - const keyRef = coerceSecretRef(cred.keyRef); - if (keyRef?.source === "env" && keyRef.id.trim()) { - return keyRef.id.trim(); - } - continue; - } - if (cred.type === "token") { - if (cred.token?.trim()) { - return cred.token; - } - const tokenRef = coerceSecretRef(cred.tokenRef); - if (tokenRef?.source === "env" && tokenRef.id.trim()) { - return tokenRef.id.trim(); - } - continue; + const resolved = resolveApiKeyFromCredential(params.store.profiles[id]); + if (resolved) { + return resolved; } } return undefined; @@ -484,6 +581,12 @@ function normalizeAntigravityProvider(provider: ProviderConfig): ProviderConfig export function normalizeProviders(params: { providers: ModelsConfig["providers"]; agentDir: string; + secretDefaults?: { + env?: string; + file?: string; + exec?: string; + }; + secretRefManagedProviders?: Set; }): ModelsConfig["providers"] { const { providers } = params; if (!providers) { @@ -505,18 +608,51 @@ export function normalizeProviders(params: { mutated = true; } let normalizedProvider = provider; - const configuredApiKey = normalizedProvider.apiKey; - - // Fix common misconfig: apiKey set to "${ENV_VAR}" instead of "ENV_VAR". - if ( - typeof configuredApiKey === "string" && - normalizeApiKeyConfig(configuredApiKey) !== configuredApiKey - ) { + const normalizedHeaders = normalizeHeaderValues({ + headers: normalizedProvider.headers, + secretDefaults: params.secretDefaults, + }); + if (normalizedHeaders.mutated) { mutated = true; - normalizedProvider = { - ...normalizedProvider, - apiKey: normalizeApiKeyConfig(configuredApiKey), - }; + normalizedProvider = { ...normalizedProvider, headers: normalizedHeaders.headers }; + } + const configuredApiKey = normalizedProvider.apiKey; + const configuredApiKeyRef = resolveSecretInputRef({ + value: configuredApiKey, + defaults: params.secretDefaults, + }).ref; + const profileApiKey = resolveApiKeyFromProfiles({ + provider: normalizedKey, + store: authStore, + }); + + if (configuredApiKeyRef && configuredApiKeyRef.id.trim()) { + const marker = + configuredApiKeyRef.source === "env" + ? configuredApiKeyRef.id.trim() + : resolveNonEnvSecretRefApiKeyMarker(configuredApiKeyRef.source); + if (normalizedProvider.apiKey !== marker) { + mutated = true; + normalizedProvider = { ...normalizedProvider, apiKey: marker }; + } + params.secretRefManagedProviders?.add(normalizedKey); + } else if (typeof configuredApiKey === "string") { + // Fix common misconfig: apiKey set to "${ENV_VAR}" instead of "ENV_VAR". + const normalizedConfiguredApiKey = normalizeApiKeyConfig(configuredApiKey); + if (normalizedConfiguredApiKey !== configuredApiKey) { + mutated = true; + normalizedProvider = { + ...normalizedProvider, + apiKey: normalizedConfiguredApiKey, + }; + } + if ( + profileApiKey && + profileApiKey.source !== "plaintext" && + normalizedConfiguredApiKey === profileApiKey.apiKey + ) { + params.secretRefManagedProviders?.add(normalizedKey); + } } // If a provider defines models, pi's ModelRegistry requires apiKey to be set. @@ -534,12 +670,11 @@ export function normalizeProviders(params: { normalizedProvider = { ...normalizedProvider, apiKey }; } else { const fromEnv = resolveEnvApiKeyVarName(normalizedKey); - const fromProfiles = resolveApiKeyFromProfiles({ - provider: normalizedKey, - store: authStore, - }); - const apiKey = fromEnv ?? fromProfiles; + const apiKey = fromEnv ?? profileApiKey?.apiKey; if (apiKey?.trim()) { + if (profileApiKey && profileApiKey.source !== "plaintext") { + params.secretRefManagedProviders?.add(normalizedKey); + } mutated = true; normalizedProvider = { ...normalizedProvider, apiKey }; } @@ -778,14 +913,8 @@ async function buildOllamaProvider( }; } -async function buildHuggingfaceProvider(apiKey?: string): Promise { - // Resolve env var name to value for discovery (GET /v1/models requires Bearer token). - const resolvedSecret = - apiKey?.trim() !== "" - ? /^[A-Z][A-Z0-9_]*$/.test(apiKey!.trim()) - ? (process.env[apiKey!.trim()] ?? "").trim() - : apiKey!.trim() - : ""; +async function buildHuggingfaceProvider(discoveryApiKey?: string): Promise { + const resolvedSecret = toDiscoveryApiKey(discoveryApiKey) ?? ""; const models = resolvedSecret !== "" ? await discoverHuggingfaceModels(resolvedSecret) @@ -946,10 +1075,24 @@ export async function resolveImplicitProviders(params: { const authStore = ensureAuthProfileStore(params.agentDir, { allowKeychainPrompt: false, }); + const resolveProviderApiKey = ( + provider: string, + ): { apiKey: string | undefined; discoveryApiKey?: string } => { + const envVar = resolveEnvApiKeyVarName(provider); + if (envVar) { + return { + apiKey: envVar, + discoveryApiKey: toDiscoveryApiKey(process.env[envVar]), + }; + } + const fromProfiles = resolveApiKeyFromProfiles({ provider, store: authStore }); + return { + apiKey: fromProfiles?.apiKey, + discoveryApiKey: fromProfiles?.discoveryApiKey, + }; + }; - const minimaxKey = - resolveEnvApiKeyVarName("minimax") ?? - resolveApiKeyFromProfiles({ provider: "minimax", store: authStore }); + const minimaxKey = resolveProviderApiKey("minimax").apiKey; if (minimaxKey) { providers.minimax = { ...buildMinimaxProvider(), apiKey: minimaxKey }; } @@ -958,34 +1101,26 @@ export async function resolveImplicitProviders(params: { if (minimaxOauthProfile.length > 0) { providers["minimax-portal"] = { ...buildMinimaxPortalProvider(), - apiKey: MINIMAX_OAUTH_PLACEHOLDER, + apiKey: MINIMAX_OAUTH_MARKER, }; } - const moonshotKey = - resolveEnvApiKeyVarName("moonshot") ?? - resolveApiKeyFromProfiles({ provider: "moonshot", store: authStore }); + const moonshotKey = resolveProviderApiKey("moonshot").apiKey; if (moonshotKey) { providers.moonshot = { ...buildMoonshotProvider(), apiKey: moonshotKey }; } - const kimiCodingKey = - resolveEnvApiKeyVarName("kimi-coding") ?? - resolveApiKeyFromProfiles({ provider: "kimi-coding", store: authStore }); + const kimiCodingKey = resolveProviderApiKey("kimi-coding").apiKey; if (kimiCodingKey) { providers["kimi-coding"] = { ...buildKimiCodingProvider(), apiKey: kimiCodingKey }; } - const syntheticKey = - resolveEnvApiKeyVarName("synthetic") ?? - resolveApiKeyFromProfiles({ provider: "synthetic", store: authStore }); + const syntheticKey = resolveProviderApiKey("synthetic").apiKey; if (syntheticKey) { providers.synthetic = { ...buildSyntheticProvider(), apiKey: syntheticKey }; } - const veniceKey = - resolveEnvApiKeyVarName("venice") ?? - resolveApiKeyFromProfiles({ provider: "venice", store: authStore }); + const veniceKey = resolveProviderApiKey("venice").apiKey; if (veniceKey) { providers.venice = { ...(await buildVeniceProvider()), apiKey: veniceKey }; } @@ -994,13 +1129,11 @@ export async function resolveImplicitProviders(params: { if (qwenProfiles.length > 0) { providers["qwen-portal"] = { ...buildQwenPortalProvider(), - apiKey: QWEN_PORTAL_OAUTH_PLACEHOLDER, + apiKey: QWEN_OAUTH_MARKER, }; } - const volcengineKey = - resolveEnvApiKeyVarName("volcengine") ?? - resolveApiKeyFromProfiles({ provider: "volcengine", store: authStore }); + const volcengineKey = resolveProviderApiKey("volcengine").apiKey; if (volcengineKey) { providers.volcengine = { ...buildDoubaoProvider(), apiKey: volcengineKey }; providers["volcengine-plan"] = { @@ -1009,9 +1142,7 @@ export async function resolveImplicitProviders(params: { }; } - const byteplusKey = - resolveEnvApiKeyVarName("byteplus") ?? - resolveApiKeyFromProfiles({ provider: "byteplus", store: authStore }); + const byteplusKey = resolveProviderApiKey("byteplus").apiKey; if (byteplusKey) { providers.byteplus = { ...buildBytePlusProvider(), apiKey: byteplusKey }; providers["byteplus-plan"] = { @@ -1020,9 +1151,7 @@ export async function resolveImplicitProviders(params: { }; } - const xiaomiKey = - resolveEnvApiKeyVarName("xiaomi") ?? - resolveApiKeyFromProfiles({ provider: "xiaomi", store: authStore }); + const xiaomiKey = resolveProviderApiKey("xiaomi").apiKey; if (xiaomiKey) { providers.xiaomi = { ...buildXiaomiProvider(), apiKey: xiaomiKey }; } @@ -1042,7 +1171,9 @@ export async function resolveImplicitProviders(params: { if (!baseUrl) { continue; } - const apiKey = resolveEnvApiKeyVarName("cloudflare-ai-gateway") ?? cred.key?.trim() ?? ""; + const envVarApiKey = resolveEnvApiKeyVarName("cloudflare-ai-gateway"); + const profileApiKey = resolveApiKeyFromCredential(cred)?.apiKey; + const apiKey = envVarApiKey ?? profileApiKey ?? ""; if (!apiKey) { continue; } @@ -1059,9 +1190,7 @@ export async function resolveImplicitProviders(params: { // Use the user's configured baseUrl (from explicit providers) for model // discovery so that remote / non-default Ollama instances are reachable. // Skip discovery when explicit models are already defined. - const ollamaKey = - resolveEnvApiKeyVarName("ollama") ?? - resolveApiKeyFromProfiles({ provider: "ollama", store: authStore }); + const ollamaKey = resolveProviderApiKey("ollama").apiKey; const explicitOllama = params.explicitProviders?.ollama; const hasExplicitModels = Array.isArray(explicitOllama?.models) && explicitOllama.models.length > 0; @@ -1070,7 +1199,7 @@ export async function resolveImplicitProviders(params: { ...explicitOllama, baseUrl: resolveOllamaApiBase(explicitOllama.baseUrl), api: explicitOllama.api ?? "ollama", - apiKey: ollamaKey ?? explicitOllama.apiKey ?? "ollama-local", + apiKey: ollamaKey ?? explicitOllama.apiKey ?? OLLAMA_LOCAL_AUTH_MARKER, }; } else { const ollamaBaseUrl = explicitOllama?.baseUrl; @@ -1083,7 +1212,7 @@ export async function resolveImplicitProviders(params: { if (ollamaProvider.models.length > 0 || ollamaKey || explicitOllama?.apiKey) { providers.ollama = { ...ollamaProvider, - apiKey: ollamaKey ?? explicitOllama?.apiKey ?? "ollama-local", + apiKey: ollamaKey ?? explicitOllama?.apiKey ?? OLLAMA_LOCAL_AUTH_MARKER, }; } } @@ -1091,23 +1220,16 @@ export async function resolveImplicitProviders(params: { // vLLM provider - OpenAI-compatible local server (opt-in via env/profile). // If explicitly configured, keep user-defined models/settings as-is. if (!params.explicitProviders?.vllm) { - const vllmEnvVar = resolveEnvApiKeyVarName("vllm"); - const vllmProfileKey = resolveApiKeyFromProfiles({ provider: "vllm", store: authStore }); - const vllmKey = vllmEnvVar ?? vllmProfileKey; + const { apiKey: vllmKey, discoveryApiKey } = resolveProviderApiKey("vllm"); if (vllmKey) { - const discoveryApiKey = vllmEnvVar - ? (process.env[vllmEnvVar]?.trim() ?? "") - : (vllmProfileKey ?? ""); providers.vllm = { - ...(await buildVllmProvider({ apiKey: discoveryApiKey || undefined })), + ...(await buildVllmProvider({ apiKey: discoveryApiKey })), apiKey: vllmKey, }; } } - const togetherKey = - resolveEnvApiKeyVarName("together") ?? - resolveApiKeyFromProfiles({ provider: "together", store: authStore }); + const togetherKey = resolveProviderApiKey("together").apiKey; if (togetherKey) { providers.together = { ...buildTogetherProvider(), @@ -1115,41 +1237,32 @@ export async function resolveImplicitProviders(params: { }; } - const huggingfaceKey = - resolveEnvApiKeyVarName("huggingface") ?? - resolveApiKeyFromProfiles({ provider: "huggingface", store: authStore }); + const { apiKey: huggingfaceKey, discoveryApiKey: huggingfaceDiscoveryApiKey } = + resolveProviderApiKey("huggingface"); if (huggingfaceKey) { - const hfProvider = await buildHuggingfaceProvider(huggingfaceKey); + const hfProvider = await buildHuggingfaceProvider(huggingfaceDiscoveryApiKey); providers.huggingface = { ...hfProvider, apiKey: huggingfaceKey, }; } - const qianfanKey = - resolveEnvApiKeyVarName("qianfan") ?? - resolveApiKeyFromProfiles({ provider: "qianfan", store: authStore }); + const qianfanKey = resolveProviderApiKey("qianfan").apiKey; if (qianfanKey) { providers.qianfan = { ...buildQianfanProvider(), apiKey: qianfanKey }; } - const openrouterKey = - resolveEnvApiKeyVarName("openrouter") ?? - resolveApiKeyFromProfiles({ provider: "openrouter", store: authStore }); + const openrouterKey = resolveProviderApiKey("openrouter").apiKey; if (openrouterKey) { providers.openrouter = { ...buildOpenrouterProvider(), apiKey: openrouterKey }; } - const nvidiaKey = - resolveEnvApiKeyVarName("nvidia") ?? - resolveApiKeyFromProfiles({ provider: "nvidia", store: authStore }); + const nvidiaKey = resolveProviderApiKey("nvidia").apiKey; if (nvidiaKey) { providers.nvidia = { ...buildNvidiaProvider(), apiKey: nvidiaKey }; } - const kilocodeKey = - resolveEnvApiKeyVarName("kilocode") ?? - resolveApiKeyFromProfiles({ provider: "kilocode", store: authStore }); + const kilocodeKey = resolveProviderApiKey("kilocode").apiKey; if (kilocodeKey) { providers.kilocode = { ...(await buildKilocodeProviderWithDiscovery()), apiKey: kilocodeKey }; } diff --git a/src/agents/models-config.runtime-source-snapshot.test.ts b/src/agents/models-config.runtime-source-snapshot.test.ts new file mode 100644 index 00000000000..6d6ea0284ee --- /dev/null +++ b/src/agents/models-config.runtime-source-snapshot.test.ts @@ -0,0 +1,162 @@ +import { describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../config/config.js"; +import { + clearConfigCache, + clearRuntimeConfigSnapshot, + loadConfig, + setRuntimeConfigSnapshot, +} from "../config/config.js"; +import { NON_ENV_SECRETREF_MARKER } from "./model-auth-markers.js"; +import { + installModelsConfigTestHooks, + withModelsTempHome as withTempHome, +} from "./models-config.e2e-harness.js"; +import { ensureOpenClawModelsJson } from "./models-config.js"; +import { readGeneratedModelsJson } from "./models-config.test-utils.js"; + +installModelsConfigTestHooks(); + +describe("models-config runtime source snapshot", () => { + it("uses runtime source snapshot markers when passed the active runtime config", async () => { + await withTempHome(async () => { + const sourceConfig: OpenClawConfig = { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, // pragma: allowlist secret + api: "openai-completions" as const, + models: [], + }, + }, + }, + }; + const runtimeConfig: OpenClawConfig = { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: "sk-runtime-resolved", // pragma: allowlist secret + api: "openai-completions" as const, + models: [], + }, + }, + }, + }; + + try { + setRuntimeConfigSnapshot(runtimeConfig, sourceConfig); + await ensureOpenClawModelsJson(loadConfig()); + + const parsed = await readGeneratedModelsJson<{ + providers: Record; + }>(); + expect(parsed.providers.openai?.apiKey).toBe("OPENAI_API_KEY"); // pragma: allowlist secret + } finally { + clearRuntimeConfigSnapshot(); + clearConfigCache(); + } + }); + }); + + it("uses non-env marker from runtime source snapshot for file refs", async () => { + await withTempHome(async () => { + const sourceConfig: OpenClawConfig = { + models: { + providers: { + moonshot: { + baseUrl: "https://api.moonshot.ai/v1", + apiKey: { source: "file", provider: "vault", id: "/moonshot/apiKey" }, + api: "openai-completions" as const, + models: [], + }, + }, + }, + }; + const runtimeConfig: OpenClawConfig = { + models: { + providers: { + moonshot: { + baseUrl: "https://api.moonshot.ai/v1", + apiKey: "sk-runtime-moonshot", // pragma: allowlist secret + api: "openai-completions" as const, + models: [], + }, + }, + }, + }; + + try { + setRuntimeConfigSnapshot(runtimeConfig, sourceConfig); + await ensureOpenClawModelsJson(loadConfig()); + + const parsed = await readGeneratedModelsJson<{ + providers: Record; + }>(); + expect(parsed.providers.moonshot?.apiKey).toBe(NON_ENV_SECRETREF_MARKER); + } finally { + clearRuntimeConfigSnapshot(); + clearConfigCache(); + } + }); + }); + + it("uses header markers from runtime source snapshot instead of resolved runtime values", async () => { + await withTempHome(async () => { + const sourceConfig: OpenClawConfig = { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions" as const, + headers: { + Authorization: { + source: "env", + provider: "default", + id: "OPENAI_HEADER_TOKEN", // pragma: allowlist secret + }, + "X-Tenant-Token": { + source: "file", + provider: "vault", + id: "/providers/openai/tenantToken", + }, + }, + models: [], + }, + }, + }, + }; + const runtimeConfig: OpenClawConfig = { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions" as const, + headers: { + Authorization: "Bearer runtime-openai-token", + "X-Tenant-Token": "runtime-tenant-token", + }, + models: [], + }, + }, + }, + }; + + try { + setRuntimeConfigSnapshot(runtimeConfig, sourceConfig); + await ensureOpenClawModelsJson(loadConfig()); + + const parsed = await readGeneratedModelsJson<{ + providers: Record }>; + }>(); + expect(parsed.providers.openai?.headers?.Authorization).toBe( + "secretref-env:OPENAI_HEADER_TOKEN", // pragma: allowlist secret + ); + expect(parsed.providers.openai?.headers?.["X-Tenant-Token"]).toBe(NON_ENV_SECRETREF_MARKER); + } finally { + clearRuntimeConfigSnapshot(); + clearConfigCache(); + } + }); + }); +}); diff --git a/src/agents/models-config.ts b/src/agents/models-config.ts index e31d61044c3..11832b30b15 100644 --- a/src/agents/models-config.ts +++ b/src/agents/models-config.ts @@ -1,9 +1,15 @@ import fs from "node:fs/promises"; import path from "node:path"; -import { type OpenClawConfig, loadConfig } from "../config/config.js"; +import { + getRuntimeConfigSnapshot, + getRuntimeConfigSourceSnapshot, + type OpenClawConfig, + loadConfig, +} from "../config/config.js"; import { applyConfigEnvVars } from "../config/env-vars.js"; import { isRecord } from "../utils.js"; import { resolveOpenClawAgentDir } from "./agent-paths.js"; +import { isNonSecretApiKeyMarker } from "./model-auth-markers.js"; import { normalizeProviders, type ProviderConfig, @@ -15,6 +21,7 @@ import { type ModelsConfig = NonNullable; const DEFAULT_MODE: NonNullable = "merge"; +const MODELS_JSON_WRITE_LOCKS = new Map>(); function resolvePreferredTokenLimit(explicitValue: number, implicitValue: number): number { // Keep catalog refresh behavior for stale low values while preserving @@ -141,8 +148,9 @@ async function resolveProvidersForModelsJson(params: { function mergeWithExistingProviderSecrets(params: { nextProviders: Record; existingProviders: Record[string]>; + secretRefManagedProviders: ReadonlySet; }): Record { - const { nextProviders, existingProviders } = params; + const { nextProviders, existingProviders, secretRefManagedProviders } = params; const mergedProviders: Record = {}; for (const [key, entry] of Object.entries(existingProviders)) { mergedProviders[key] = entry; @@ -159,7 +167,12 @@ function mergeWithExistingProviderSecrets(params: { continue; } const preserved: Record = {}; - if (typeof existing.apiKey === "string" && existing.apiKey) { + if ( + !secretRefManagedProviders.has(key) && + typeof existing.apiKey === "string" && + existing.apiKey && + !isNonSecretApiKeyMarker(existing.apiKey, { includeEnvVarName: false }) + ) { preserved.apiKey = existing.apiKey; } if (typeof existing.baseUrl === "string" && existing.baseUrl) { @@ -174,6 +187,7 @@ async function resolveProvidersForMode(params: { mode: NonNullable; targetPath: string; providers: Record; + secretRefManagedProviders: ReadonlySet; }): Promise> { if (params.mode !== "merge") { return params.providers; @@ -189,6 +203,7 @@ async function resolveProvidersForMode(params: { return mergeWithExistingProviderSecrets({ nextProviders: params.providers, existingProviders, + secretRefManagedProviders: params.secretRefManagedProviders, }); } @@ -200,45 +215,94 @@ async function readRawFile(pathname: string): Promise { } } +async function ensureModelsFileMode(pathname: string): Promise { + await fs.chmod(pathname, 0o600).catch(() => { + // best-effort + }); +} + +function resolveModelsConfigInput(config?: OpenClawConfig): OpenClawConfig { + const runtimeSource = getRuntimeConfigSourceSnapshot(); + if (!runtimeSource) { + return config ?? loadConfig(); + } + if (!config) { + return runtimeSource; + } + const runtimeResolved = getRuntimeConfigSnapshot(); + if (runtimeResolved && config === runtimeResolved) { + return runtimeSource; + } + return config; +} + +async function withModelsJsonWriteLock(targetPath: string, run: () => Promise): Promise { + const prior = MODELS_JSON_WRITE_LOCKS.get(targetPath) ?? Promise.resolve(); + let release: () => void = () => {}; + const gate = new Promise((resolve) => { + release = resolve; + }); + const pending = prior.then(() => gate); + MODELS_JSON_WRITE_LOCKS.set(targetPath, pending); + try { + await prior; + return await run(); + } finally { + release(); + if (MODELS_JSON_WRITE_LOCKS.get(targetPath) === pending) { + MODELS_JSON_WRITE_LOCKS.delete(targetPath); + } + } +} + export async function ensureOpenClawModelsJson( config?: OpenClawConfig, agentDirOverride?: string, ): Promise<{ agentDir: string; wrote: boolean }> { - const cfg = config ?? loadConfig(); + const cfg = resolveModelsConfigInput(config); const agentDir = agentDirOverride?.trim() ? agentDirOverride.trim() : resolveOpenClawAgentDir(); - - // Ensure config env vars (e.g. AWS_PROFILE, AWS_ACCESS_KEY_ID) are - // available in process.env before implicit provider discovery. Some - // callers (agent runner, tools) pass config objects that haven't gone - // through the full loadConfig() pipeline which applies these. - applyConfigEnvVars(cfg); - - const providers = await resolveProvidersForModelsJson({ cfg, agentDir }); - - if (Object.keys(providers).length === 0) { - return { agentDir, wrote: false }; - } - - const mode = cfg.models?.mode ?? DEFAULT_MODE; const targetPath = path.join(agentDir, "models.json"); - const mergedProviders = await resolveProvidersForMode({ - mode, - targetPath, - providers, + + return await withModelsJsonWriteLock(targetPath, async () => { + // Ensure config env vars (e.g. AWS_PROFILE, AWS_ACCESS_KEY_ID) are + // available in process.env before implicit provider discovery. Some + // callers (agent runner, tools) pass config objects that haven't gone + // through the full loadConfig() pipeline which applies these. + applyConfigEnvVars(cfg); + + const providers = await resolveProvidersForModelsJson({ cfg, agentDir }); + + if (Object.keys(providers).length === 0) { + return { agentDir, wrote: false }; + } + + const mode = cfg.models?.mode ?? DEFAULT_MODE; + const secretRefManagedProviders = new Set(); + + const normalizedProviders = + normalizeProviders({ + providers, + agentDir, + secretDefaults: cfg.secrets?.defaults, + secretRefManagedProviders, + }) ?? providers; + const mergedProviders = await resolveProvidersForMode({ + mode, + targetPath, + providers: normalizedProviders, + secretRefManagedProviders, + }); + const next = `${JSON.stringify({ providers: mergedProviders }, null, 2)}\n`; + const existingRaw = await readRawFile(targetPath); + + if (existingRaw === next) { + await ensureModelsFileMode(targetPath); + return { agentDir, wrote: false }; + } + + await fs.mkdir(agentDir, { recursive: true, mode: 0o700 }); + await fs.writeFile(targetPath, next, { mode: 0o600 }); + await ensureModelsFileMode(targetPath); + return { agentDir, wrote: true }; }); - - const normalizedProviders = normalizeProviders({ - providers: mergedProviders, - agentDir, - }); - const next = `${JSON.stringify({ providers: normalizedProviders }, null, 2)}\n`; - const existingRaw = await readRawFile(targetPath); - - if (existingRaw === next) { - return { agentDir, wrote: false }; - } - - await fs.mkdir(agentDir, { recursive: true, mode: 0o700 }); - await fs.writeFile(targetPath, next, { mode: 0o600 }); - return { agentDir, wrote: true }; } diff --git a/src/agents/models-config.write-serialization.test.ts b/src/agents/models-config.write-serialization.test.ts new file mode 100644 index 00000000000..a69fd43b830 --- /dev/null +++ b/src/agents/models-config.write-serialization.test.ts @@ -0,0 +1,55 @@ +import fs from "node:fs/promises"; +import { describe, expect, it, vi } from "vitest"; +import { + CUSTOM_PROXY_MODELS_CONFIG, + installModelsConfigTestHooks, + withModelsTempHome, +} from "./models-config.e2e-harness.js"; +import { ensureOpenClawModelsJson } from "./models-config.js"; +import { readGeneratedModelsJson } from "./models-config.test-utils.js"; + +installModelsConfigTestHooks(); + +describe("models-config write serialization", () => { + it("serializes concurrent models.json writes to avoid overlap", async () => { + await withModelsTempHome(async () => { + const first = structuredClone(CUSTOM_PROXY_MODELS_CONFIG); + const second = structuredClone(CUSTOM_PROXY_MODELS_CONFIG); + const firstModel = first.models?.providers?.["custom-proxy"]?.models?.[0]; + const secondModel = second.models?.providers?.["custom-proxy"]?.models?.[0]; + if (!firstModel || !secondModel) { + throw new Error("custom-proxy fixture missing expected model entries"); + } + firstModel.name = "Proxy A"; + secondModel.name = "Proxy B with longer name"; + + const originalWriteFile = fs.writeFile.bind(fs); + let inFlightWrites = 0; + let maxInFlightWrites = 0; + const writeSpy = vi.spyOn(fs, "writeFile").mockImplementation(async (...args) => { + inFlightWrites += 1; + if (inFlightWrites > maxInFlightWrites) { + maxInFlightWrites = inFlightWrites; + } + await new Promise((resolve) => setTimeout(resolve, 20)); + try { + return await originalWriteFile(...args); + } finally { + inFlightWrites -= 1; + } + }); + + try { + await Promise.all([ensureOpenClawModelsJson(first), ensureOpenClawModelsJson(second)]); + } finally { + writeSpy.mockRestore(); + } + + expect(maxInFlightWrites).toBe(1); + const parsed = await readGeneratedModelsJson<{ + providers: { "custom-proxy"?: { models?: Array<{ name?: string }> } }; + }>(); + expect(parsed.providers["custom-proxy"]?.models?.[0]?.name).toBe("Proxy B with longer name"); + }); + }); +}); diff --git a/src/agents/pi-embedded-runner/model.test.ts b/src/agents/pi-embedded-runner/model.test.ts index d23b68d32b6..ca12a76cb36 100644 --- a/src/agents/pi-embedded-runner/model.test.ts +++ b/src/agents/pi-embedded-runner/model.test.ts @@ -179,6 +179,28 @@ describe("buildInlineProviderModels", () => { expect(result).toHaveLength(1); expect(result[0].headers).toBeUndefined(); }); + + it("preserves literal marker-shaped headers in inline provider models", () => { + const providers: Parameters[0] = { + custom: { + headers: { + Authorization: "secretref-env:OPENAI_HEADER_TOKEN", + "X-Managed": "secretref-managed", + "X-Static": "tenant-a", + }, + models: [makeModel("custom-model")], + }, + }; + + const result = buildInlineProviderModels(providers); + + expect(result).toHaveLength(1); + expect(result[0].headers).toEqual({ + Authorization: "secretref-env:OPENAI_HEADER_TOKEN", + "X-Managed": "secretref-managed", + "X-Static": "tenant-a", + }); + }); }); describe("resolveModel", () => { @@ -223,6 +245,56 @@ describe("resolveModel", () => { }); }); + it("preserves literal marker-shaped provider headers in fallback models", () => { + const cfg = { + models: { + providers: { + custom: { + baseUrl: "http://localhost:9000", + headers: { + Authorization: "secretref-env:OPENAI_HEADER_TOKEN", + "X-Managed": "secretref-managed", + "X-Custom-Auth": "token-123", + }, + models: [makeModel("listed-model")], + }, + }, + }, + } as OpenClawConfig; + + const result = resolveModel("custom", "missing-model", "/tmp/agent", cfg); + + expect(result.error).toBeUndefined(); + expect((result.model as unknown as { headers?: Record }).headers).toEqual({ + Authorization: "secretref-env:OPENAI_HEADER_TOKEN", + "X-Managed": "secretref-managed", + "X-Custom-Auth": "token-123", + }); + }); + + it("drops marker headers from discovered models.json entries", () => { + mockDiscoveredModel({ + provider: "custom", + modelId: "listed-model", + templateModel: { + ...makeModel("listed-model"), + provider: "custom", + headers: { + Authorization: "secretref-env:OPENAI_HEADER_TOKEN", + "X-Managed": "secretref-managed", + "X-Static": "tenant-a", + }, + }, + }); + + const result = resolveModel("custom", "listed-model", "/tmp/agent"); + + expect(result.error).toBeUndefined(); + expect((result.model as unknown as { headers?: Record }).headers).toEqual({ + "X-Static": "tenant-a", + }); + }); + it("prefers matching configured model metadata for fallback token limits", () => { const cfg = { models: { diff --git a/src/agents/pi-embedded-runner/model.ts b/src/agents/pi-embedded-runner/model.ts index b846895d029..f1b31a5e49a 100644 --- a/src/agents/pi-embedded-runner/model.ts +++ b/src/agents/pi-embedded-runner/model.ts @@ -5,6 +5,7 @@ import type { ModelDefinitionConfig } from "../../config/types.js"; import { resolveOpenClawAgentDir } from "../agent-paths.js"; import { DEFAULT_CONTEXT_TOKENS } from "../defaults.js"; import { buildModelAliasLines } from "../model-alias-lines.js"; +import { isSecretRefHeaderValueMarker } from "../model-auth-markers.js"; import { normalizeModelCompat } from "../model-compat.js"; import { resolveForwardCompatModel } from "../model-forward-compat.js"; import { findNormalizedProviderValue, normalizeProviderId } from "../model-selection.js"; @@ -19,9 +20,29 @@ type InlineProviderConfig = { baseUrl?: string; api?: ModelDefinitionConfig["api"]; models?: ModelDefinitionConfig[]; - headers?: Record; + headers?: unknown; }; +function sanitizeModelHeaders( + headers: unknown, + opts?: { stripSecretRefMarkers?: boolean }, +): Record | undefined { + if (!headers || typeof headers !== "object" || Array.isArray(headers)) { + return undefined; + } + const next: Record = {}; + for (const [headerName, headerValue] of Object.entries(headers)) { + if (typeof headerValue !== "string") { + continue; + } + if (opts?.stripSecretRefMarkers && isSecretRefHeaderValueMarker(headerValue)) { + continue; + } + next[headerName] = headerValue; + } + return Object.keys(next).length > 0 ? next : undefined; +} + export { buildModelAliasLines }; function resolveConfiguredProviderConfig( @@ -46,16 +67,23 @@ function applyConfiguredProviderOverrides(params: { }): Model { const { discoveredModel, providerConfig, modelId } = params; if (!providerConfig) { - return discoveredModel; + return { + ...discoveredModel, + // Discovered models originate from models.json and may contain persistence markers. + headers: sanitizeModelHeaders(discoveredModel.headers, { stripSecretRefMarkers: true }), + }; } const configuredModel = providerConfig.models?.find((candidate) => candidate.id === modelId); - if ( - !configuredModel && - !providerConfig.baseUrl && - !providerConfig.api && - !providerConfig.headers - ) { - return discoveredModel; + const discoveredHeaders = sanitizeModelHeaders(discoveredModel.headers, { + stripSecretRefMarkers: true, + }); + const providerHeaders = sanitizeModelHeaders(providerConfig.headers); + const configuredHeaders = sanitizeModelHeaders(configuredModel?.headers); + if (!configuredModel && !providerConfig.baseUrl && !providerConfig.api && !providerHeaders) { + return { + ...discoveredModel, + headers: discoveredHeaders, + }; } return { ...discoveredModel, @@ -67,13 +95,13 @@ function applyConfiguredProviderOverrides(params: { contextWindow: configuredModel?.contextWindow ?? discoveredModel.contextWindow, maxTokens: configuredModel?.maxTokens ?? discoveredModel.maxTokens, headers: - providerConfig.headers || configuredModel?.headers + discoveredHeaders || providerHeaders || configuredHeaders ? { - ...discoveredModel.headers, - ...providerConfig.headers, - ...configuredModel?.headers, + ...discoveredHeaders, + ...providerHeaders, + ...configuredHeaders, } - : discoveredModel.headers, + : undefined, compat: configuredModel?.compat ?? discoveredModel.compat, }; } @@ -86,15 +114,22 @@ export function buildInlineProviderModels( if (!trimmed) { return []; } + const providerHeaders = sanitizeModelHeaders(entry?.headers); return (entry?.models ?? []).map((model) => ({ ...model, provider: trimmed, baseUrl: entry?.baseUrl, api: model.api ?? entry?.api, - headers: - entry?.headers || (model as InlineModelEntry).headers - ? { ...entry?.headers, ...(model as InlineModelEntry).headers } - : undefined, + headers: (() => { + const modelHeaders = sanitizeModelHeaders((model as InlineModelEntry).headers); + if (!providerHeaders && !modelHeaders) { + return undefined; + } + return { + ...providerHeaders, + ...modelHeaders, + }; + })(), })); }); } @@ -161,6 +196,8 @@ export function resolveModelWithRegistry(params: { } const configuredModel = providerConfig?.models?.find((candidate) => candidate.id === modelId); + const providerHeaders = sanitizeModelHeaders(providerConfig?.headers); + const modelHeaders = sanitizeModelHeaders(configuredModel?.headers); if (providerConfig || modelId.startsWith("mock-")) { return normalizeModelCompat({ id: modelId, @@ -180,9 +217,7 @@ export function resolveModelWithRegistry(params: { providerConfig?.models?.[0]?.maxTokens ?? DEFAULT_CONTEXT_TOKENS, headers: - providerConfig?.headers || configuredModel?.headers - ? { ...providerConfig?.headers, ...configuredModel?.headers } - : undefined, + providerHeaders || modelHeaders ? { ...providerHeaders, ...modelHeaders } : undefined, } as Model); } diff --git a/src/commands/agent.test.ts b/src/commands/agent.test.ts index 7ca6909af4a..baa58df2ef1 100644 --- a/src/commands/agent.test.ts +++ b/src/commands/agent.test.ts @@ -8,6 +8,7 @@ import { FailoverError } from "../agents/failover-error.js"; import { loadModelCatalog } from "../agents/model-catalog.js"; import * as modelSelectionModule from "../agents/model-selection.js"; import { runEmbeddedPiAgent } from "../agents/pi-embedded.js"; +import * as commandSecretGatewayModule from "../cli/command-secret-gateway.js"; import type { OpenClawConfig } from "../config/config.js"; import * as configModule from "../config/config.js"; import * as sessionsModule from "../config/sessions.js"; @@ -51,6 +52,8 @@ const runtime: RuntimeEnv = { }; const configSpy = vi.spyOn(configModule, "loadConfig"); +const readConfigFileSnapshotForWriteSpy = vi.spyOn(configModule, "readConfigFileSnapshotForWrite"); +const setRuntimeConfigSnapshotSpy = vi.spyOn(configModule, "setRuntimeConfigSnapshot"); const runCliAgentSpy = vi.spyOn(cliRunnerModule, "runCliAgent"); const deliverAgentCommandResultSpy = vi.spyOn(agentDeliveryModule, "deliverAgentCommandResult"); @@ -256,13 +259,91 @@ function createTelegramOutboundPlugin() { beforeEach(() => { vi.clearAllMocks(); + configModule.clearRuntimeConfigSnapshot(); runCliAgentSpy.mockResolvedValue(createDefaultAgentResult() as never); vi.mocked(runEmbeddedPiAgent).mockResolvedValue(createDefaultAgentResult()); vi.mocked(loadModelCatalog).mockResolvedValue([]); vi.mocked(modelSelectionModule.isCliProvider).mockImplementation(() => false); + readConfigFileSnapshotForWriteSpy.mockResolvedValue({ + snapshot: { valid: false, resolved: {} as OpenClawConfig }, + writeOptions: {}, + } as Awaited>); }); describe("agentCommand", () => { + it("sets runtime snapshots from source config before embedded agent run", async () => { + await withTempHome(async (home) => { + const store = path.join(home, "sessions.json"); + const loadedConfig = { + agents: { + defaults: { + model: { primary: "anthropic/claude-opus-4-5" }, + models: { "anthropic/claude-opus-4-5": {} }, + workspace: path.join(home, "openclaw"), + }, + }, + session: { store, mainKey: "main" }, + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, // pragma: allowlist secret + models: [], + }, + }, + }, + } as unknown as OpenClawConfig; + const sourceConfig = { + ...loadedConfig, + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, // pragma: allowlist secret + models: [], + }, + }, + }, + } as unknown as OpenClawConfig; + const resolvedConfig = { + ...loadedConfig, + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: "sk-resolved-runtime", // pragma: allowlist secret + models: [], + }, + }, + }, + } as unknown as OpenClawConfig; + + configSpy.mockReturnValue(loadedConfig); + readConfigFileSnapshotForWriteSpy.mockResolvedValue({ + snapshot: { valid: true, resolved: sourceConfig }, + writeOptions: {}, + } as Awaited>); + const resolveSecretsSpy = vi + .spyOn(commandSecretGatewayModule, "resolveCommandSecretRefsViaGateway") + .mockResolvedValueOnce({ + resolvedConfig, + diagnostics: [], + targetStatesByPath: {}, + hadUnresolvedTargets: false, + }); + + await agentCommand({ message: "hello", to: "+1555" }, runtime); + + expect(resolveSecretsSpy).toHaveBeenCalledWith({ + config: loadedConfig, + commandName: "agent", + targetIds: expect.any(Set), + }); + expect(setRuntimeConfigSnapshotSpy).toHaveBeenCalledWith(resolvedConfig, sourceConfig); + expect(vi.mocked(runEmbeddedPiAgent).mock.calls.at(-1)?.[0]?.config).toBe(resolvedConfig); + }); + }); + it("creates a session entry when deriving from --to", async () => { await withTempHome(async (home) => { const store = path.join(home, "sessions.json"); diff --git a/src/commands/agent.ts b/src/commands/agent.ts index 10582521b95..7ed147dd46f 100644 --- a/src/commands/agent.ts +++ b/src/commands/agent.ts @@ -57,7 +57,11 @@ import { formatCliCommand } from "../cli/command-format.js"; import { resolveCommandSecretRefsViaGateway } from "../cli/command-secret-gateway.js"; import { getAgentRuntimeCommandSecretTargetIds } from "../cli/command-secret-targets.js"; import { type CliDeps, createDefaultDeps } from "../cli/deps.js"; -import { loadConfig } from "../config/config.js"; +import { + loadConfig, + readConfigFileSnapshotForWrite, + setRuntimeConfigSnapshot, +} from "../config/config.js"; import { mergeSessionEntry, parseSessionThreadInfo, @@ -427,11 +431,23 @@ async function agentCommandInternal( } const loadedRaw = loadConfig(); + const sourceConfig = await (async () => { + try { + const { snapshot } = await readConfigFileSnapshotForWrite(); + if (snapshot.valid) { + return snapshot.resolved; + } + } catch { + // Fall back to runtime-loaded config when source snapshot is unavailable. + } + return loadedRaw; + })(); const { resolvedConfig: cfg, diagnostics } = await resolveCommandSecretRefsViaGateway({ config: loadedRaw, commandName: "agent", targetIds: getAgentRuntimeCommandSecretTargetIds(), }); + setRuntimeConfigSnapshot(cfg, sourceConfig); for (const entry of diagnostics) { runtime.log(`[secrets] ${entry}`); } diff --git a/src/commands/models.list.e2e.test.ts b/src/commands/models.list.e2e.test.ts index 1469effeff1..53a112d0451 100644 --- a/src/commands/models.list.e2e.test.ts +++ b/src/commands/models.list.e2e.test.ts @@ -5,6 +5,11 @@ let loadModelRegistry: typeof import("./models/list.registry.js").loadModelRegis let toModelRow: typeof import("./models/list.registry.js").toModelRow; const loadConfig = vi.fn(); +const readConfigFileSnapshotForWrite = vi.fn().mockResolvedValue({ + snapshot: { valid: false, resolved: {} }, + writeOptions: {}, +}); +const setRuntimeConfigSnapshot = vi.fn(); const ensureOpenClawModelsJson = vi.fn().mockResolvedValue(undefined); const resolveOpenClawAgentDir = vi.fn().mockReturnValue("/tmp/openclaw-agent"); const ensureAuthProfileStore = vi.fn().mockReturnValue({ version: 1, profiles: {} }); @@ -29,6 +34,8 @@ vi.mock("../config/config.js", () => ({ CONFIG_PATH: "/tmp/openclaw.json", STATE_DIR: "/tmp/openclaw-state", loadConfig, + readConfigFileSnapshotForWrite, + setRuntimeConfigSnapshot, })); vi.mock("../agents/models-config.js", () => ({ @@ -84,8 +91,16 @@ vi.mock("../agents/pi-model-discovery.js", () => { }); vi.mock("../agents/pi-embedded-runner/model.js", () => ({ - resolveModel: () => { - throw new Error("resolveModel should not be called from models.list tests"); + resolveModelWithRegistry: ({ + provider, + modelId, + modelRegistry, + }: { + provider: string; + modelId: string; + modelRegistry: { find: (provider: string, id: string) => unknown }; + }) => { + return modelRegistry.find(provider, modelId); }, })); @@ -114,6 +129,13 @@ beforeEach(() => { modelRegistryState.getAllError = undefined; modelRegistryState.getAvailableError = undefined; listProfilesForProvider.mockReturnValue([]); + ensureOpenClawModelsJson.mockClear(); + readConfigFileSnapshotForWrite.mockClear(); + readConfigFileSnapshotForWrite.mockResolvedValue({ + snapshot: { valid: false, resolved: {} }, + writeOptions: {}, + }); + setRuntimeConfigSnapshot.mockClear(); }); afterEach(() => { @@ -302,6 +324,35 @@ describe("models list/status", () => { await expect(loadModelRegistry({})).rejects.toThrow("model discovery unavailable"); }); + it("loadModelRegistry persists using source config snapshot when provided", async () => { + modelRegistryState.models = [OPENAI_MODEL]; + modelRegistryState.available = [OPENAI_MODEL]; + const sourceConfig = { + models: { providers: { openai: { apiKey: "$OPENAI_API_KEY" } } }, // pragma: allowlist secret + }; + const resolvedConfig = { + models: { providers: { openai: { apiKey: "sk-resolved-runtime-value" } } }, // pragma: allowlist secret + }; + + await loadModelRegistry(resolvedConfig as never, { sourceConfig: sourceConfig as never }); + + expect(ensureOpenClawModelsJson).toHaveBeenCalledTimes(1); + expect(ensureOpenClawModelsJson).toHaveBeenCalledWith(sourceConfig); + }); + + it("loadModelRegistry uses resolved config when no source snapshot is provided", async () => { + modelRegistryState.models = [OPENAI_MODEL]; + modelRegistryState.available = [OPENAI_MODEL]; + const resolvedConfig = { + models: { providers: { openai: { apiKey: "sk-resolved-runtime-value" } } }, // pragma: allowlist secret + }; + + await loadModelRegistry(resolvedConfig as never); + + expect(ensureOpenClawModelsJson).toHaveBeenCalledTimes(1); + expect(ensureOpenClawModelsJson).toHaveBeenCalledWith(resolvedConfig); + }); + it("toModelRow does not crash without cfg/authStore when availability is undefined", async () => { const row = toModelRow({ model: makeGoogleAntigravityTemplate( diff --git a/src/commands/models/list.auth-overview.test.ts b/src/commands/models/list.auth-overview.test.ts index bc23ff9351c..98906ced281 100644 --- a/src/commands/models/list.auth-overview.test.ts +++ b/src/commands/models/list.auth-overview.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest"; +import { NON_ENV_SECRETREF_MARKER } from "../../agents/model-auth-markers.js"; import { resolveProviderAuthOverview } from "./list.auth-overview.js"; describe("resolveProviderAuthOverview", () => { @@ -21,4 +22,52 @@ describe("resolveProviderAuthOverview", () => { expect(overview.profiles.labels[0]).toContain("token:ref(env:GITHUB_TOKEN)"); }); + + it("renders marker-backed models.json auth as marker detail", () => { + const overview = resolveProviderAuthOverview({ + provider: "openai", + cfg: { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + apiKey: NON_ENV_SECRETREF_MARKER, + models: [], + }, + }, + }, + } as never, + store: { version: 1, profiles: {} } as never, + modelsPath: "/tmp/models.json", + }); + + expect(overview.effective.kind).toBe("models.json"); + expect(overview.effective.detail).toContain(`marker(${NON_ENV_SECRETREF_MARKER})`); + expect(overview.modelsJson?.value).toContain(`marker(${NON_ENV_SECRETREF_MARKER})`); + }); + + it("keeps env-var-shaped models.json values masked to avoid accidental plaintext exposure", () => { + const overview = resolveProviderAuthOverview({ + provider: "openai", + cfg: { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + apiKey: "OPENAI_API_KEY", // pragma: allowlist secret + models: [], + }, + }, + }, + } as never, + store: { version: 1, profiles: {} } as never, + modelsPath: "/tmp/models.json", + }); + + expect(overview.effective.kind).toBe("models.json"); + expect(overview.effective.detail).not.toContain("marker("); + expect(overview.effective.detail).not.toContain("OPENAI_API_KEY"); + }); }); diff --git a/src/commands/models/list.auth-overview.ts b/src/commands/models/list.auth-overview.ts index 0fc2f9828c5..28880415eeb 100644 --- a/src/commands/models/list.auth-overview.ts +++ b/src/commands/models/list.auth-overview.ts @@ -6,12 +6,19 @@ import { resolveAuthStorePathForDisplay, resolveProfileUnusableUntilForDisplay, } from "../../agents/auth-profiles.js"; +import { isNonSecretApiKeyMarker } from "../../agents/model-auth-markers.js"; import { getCustomProviderApiKey, resolveEnvApiKey } from "../../agents/model-auth.js"; import type { OpenClawConfig } from "../../config/config.js"; import { shortenHomePath } from "../../utils.js"; import { maskApiKey } from "./list.format.js"; import type { ProviderAuthOverview } from "./list.types.js"; +function formatMarkerOrSecret(value: string): string { + return isNonSecretApiKeyMarker(value, { includeEnvVarName: false }) + ? `marker(${value.trim()})` + : maskApiKey(value); +} + function formatProfileSecretLabel(params: { value: string | undefined; ref: { source: string; id: string } | undefined; @@ -19,7 +26,8 @@ function formatProfileSecretLabel(params: { }): string { const value = typeof params.value === "string" ? params.value.trim() : ""; if (value) { - return params.kind === "token" ? `token:${maskApiKey(value)}` : maskApiKey(value); + const display = formatMarkerOrSecret(value); + return params.kind === "token" ? `token:${display}` : display; } if (params.ref) { const refLabel = `ref(${params.ref.source}:${params.ref.id})`; @@ -108,7 +116,7 @@ export function resolveProviderAuthOverview(params: { }; } if (customKey) { - return { kind: "models.json", detail: maskApiKey(customKey) }; + return { kind: "models.json", detail: formatMarkerOrSecret(customKey) }; } return { kind: "missing", detail: "missing" }; })(); @@ -137,7 +145,7 @@ export function resolveProviderAuthOverview(params: { ...(customKey ? { modelsJson: { - value: maskApiKey(customKey), + value: formatMarkerOrSecret(customKey), source: `models.json: ${shortenHomePath(params.modelsPath)}`, }, } diff --git a/src/commands/models/list.list-command.forward-compat.test.ts b/src/commands/models/list.list-command.forward-compat.test.ts index 2b2e8612782..4cef137d88a 100644 --- a/src/commands/models/list.list-command.forward-compat.test.ts +++ b/src/commands/models/list.list-command.forward-compat.test.ts @@ -2,11 +2,38 @@ import { describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => { const printModelTable = vi.fn(); + const sourceConfig = { + agents: { defaults: { model: { primary: "openai-codex/gpt-5.4" } } }, + models: { + providers: { + openai: { + apiKey: "$OPENAI_API_KEY", // pragma: allowlist secret + }, + }, + }, + }; + const resolvedConfig = { + agents: { defaults: { model: { primary: "openai-codex/gpt-5.4" } } }, + models: { + providers: { + openai: { + apiKey: "sk-resolved-runtime-value", // pragma: allowlist secret + }, + }, + }, + }; return { loadConfig: vi.fn().mockReturnValue({ agents: { defaults: { model: { primary: "openai-codex/gpt-5.4" } } }, models: { providers: {} }, }), + sourceConfig, + resolvedConfig, + loadModelsConfigWithSource: vi.fn().mockResolvedValue({ + sourceConfig, + resolvedConfig, + diagnostics: [], + }), ensureAuthProfileStore: vi.fn().mockReturnValue({ version: 1, profiles: {}, order: {} }), loadModelRegistry: vi .fn() @@ -58,6 +85,10 @@ vi.mock("./list.registry.js", async (importOriginal) => { }; }); +vi.mock("./load-config.js", () => ({ + loadModelsConfigWithSource: mocks.loadModelsConfigWithSource, +})); + vi.mock("./list.configured.js", () => ({ resolveConfiguredEntries: mocks.resolveConfiguredEntries, })); @@ -95,6 +126,16 @@ describe("modelsListCommand forward-compat", () => { expect(codex?.tags).not.toContain("missing"); }); + it("passes source config to model registry loading for persistence safety", async () => { + const runtime = { log: vi.fn(), error: vi.fn() }; + + await modelsListCommand({ json: true }, runtime as never); + + expect(mocks.loadModelRegistry).toHaveBeenCalledWith(mocks.resolvedConfig, { + sourceConfig: mocks.sourceConfig, + }); + }); + it("keeps configured local openai gpt-5.4 entries visible in --local output", async () => { mocks.resolveConfiguredEntries.mockReturnValueOnce({ entries: [ diff --git a/src/commands/models/list.list-command.ts b/src/commands/models/list.list-command.ts index 7e706469cea..afcd7b785d2 100644 --- a/src/commands/models/list.list-command.ts +++ b/src/commands/models/list.list-command.ts @@ -8,7 +8,7 @@ import { formatErrorWithStack } from "./list.errors.js"; import { loadModelRegistry, toModelRow } from "./list.registry.js"; import { printModelTable } from "./list.table.js"; import type { ModelRow } from "./list.types.js"; -import { loadModelsConfig } from "./load-config.js"; +import { loadModelsConfigWithSource } from "./load-config.js"; import { DEFAULT_PROVIDER, ensureFlagCompatibility, isLocalBaseUrl, modelKey } from "./shared.js"; export async function modelsListCommand( @@ -23,7 +23,10 @@ export async function modelsListCommand( ) { ensureFlagCompatibility(opts); const { ensureAuthProfileStore } = await import("../../agents/auth-profiles.js"); - const cfg = await loadModelsConfig({ commandName: "models list", runtime }); + const { sourceConfig, resolvedConfig: cfg } = await loadModelsConfigWithSource({ + commandName: "models list", + runtime, + }); const authStore = ensureAuthProfileStore(); const providerFilter = (() => { const raw = opts.provider?.trim(); @@ -39,7 +42,7 @@ export async function modelsListCommand( let availableKeys: Set | undefined; let availabilityErrorMessage: string | undefined; try { - const loaded = await loadModelRegistry(cfg); + const loaded = await loadModelRegistry(cfg, { sourceConfig }); modelRegistry = loaded.registry; models = loaded.models; availableKeys = loaded.availableKeys; diff --git a/src/commands/models/list.probe.targets.test.ts b/src/commands/models/list.probe.targets.test.ts index 91c449f10a4..bc50a8c2fb6 100644 --- a/src/commands/models/list.probe.targets.test.ts +++ b/src/commands/models/list.probe.targets.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { AuthProfileStore } from "../../agents/auth-profiles.js"; +import { OLLAMA_LOCAL_AUTH_MARKER } from "../../agents/model-auth-markers.js"; import type { OpenClawConfig } from "../../config/config.js"; let mockStore: AuthProfileStore; @@ -138,4 +139,109 @@ describe("buildProbeTargets reason codes", () => { expectLegacyMissingCredentialsError(plan.results[0], "unresolved_ref"); expect(plan.results[0]?.error).toContain("env:default:MISSING_ANTHROPIC_TOKEN"); }); + + it("skips marker-only models.json credentials when building probe targets", async () => { + const previousAnthropic = process.env.ANTHROPIC_API_KEY; + const previousAnthropicOauth = process.env.ANTHROPIC_OAUTH_TOKEN; + delete process.env.ANTHROPIC_API_KEY; + delete process.env.ANTHROPIC_OAUTH_TOKEN; + mockStore = { + version: 1, + profiles: {}, + order: {}, + }; + try { + const plan = await buildProbeTargets({ + cfg: { + models: { + providers: { + anthropic: { + baseUrl: "https://api.anthropic.com/v1", + api: "anthropic-messages", + apiKey: OLLAMA_LOCAL_AUTH_MARKER, + models: [], + }, + }, + }, + } as OpenClawConfig, + providers: ["anthropic"], + modelCandidates: ["anthropic/claude-sonnet-4-6"], + options: { + timeoutMs: 5_000, + concurrency: 1, + maxTokens: 16, + }, + }); + + expect(plan.targets).toEqual([]); + expect(plan.results).toEqual([]); + } finally { + if (previousAnthropic === undefined) { + delete process.env.ANTHROPIC_API_KEY; + } else { + process.env.ANTHROPIC_API_KEY = previousAnthropic; + } + if (previousAnthropicOauth === undefined) { + delete process.env.ANTHROPIC_OAUTH_TOKEN; + } else { + process.env.ANTHROPIC_OAUTH_TOKEN = previousAnthropicOauth; + } + } + }); + + it("does not treat arbitrary all-caps models.json apiKey values as markers", async () => { + const previousAnthropic = process.env.ANTHROPIC_API_KEY; + const previousAnthropicOauth = process.env.ANTHROPIC_OAUTH_TOKEN; + delete process.env.ANTHROPIC_API_KEY; + delete process.env.ANTHROPIC_OAUTH_TOKEN; + mockStore = { + version: 1, + profiles: {}, + order: {}, + }; + try { + const plan = await buildProbeTargets({ + cfg: { + models: { + providers: { + anthropic: { + baseUrl: "https://api.anthropic.com/v1", + api: "anthropic-messages", + apiKey: "ALLCAPS_SAMPLE", // pragma: allowlist secret + models: [], + }, + }, + }, + } as OpenClawConfig, + providers: ["anthropic"], + modelCandidates: ["anthropic/claude-sonnet-4-6"], + options: { + timeoutMs: 5_000, + concurrency: 1, + maxTokens: 16, + }, + }); + + expect(plan.results).toEqual([]); + expect(plan.targets).toHaveLength(1); + expect(plan.targets[0]).toEqual( + expect.objectContaining({ + provider: "anthropic", + source: "models.json", + label: "models.json", + }), + ); + } finally { + if (previousAnthropic === undefined) { + delete process.env.ANTHROPIC_API_KEY; + } else { + process.env.ANTHROPIC_API_KEY = previousAnthropic; + } + if (previousAnthropicOauth === undefined) { + delete process.env.ANTHROPIC_OAUTH_TOKEN; + } else { + process.env.ANTHROPIC_OAUTH_TOKEN = previousAnthropicOauth; + } + } + }); }); diff --git a/src/commands/models/list.probe.ts b/src/commands/models/list.probe.ts index 8a2ec87adcc..40eb6b99b9b 100644 --- a/src/commands/models/list.probe.ts +++ b/src/commands/models/list.probe.ts @@ -12,6 +12,7 @@ import { resolveAuthProfileOrder, } from "../../agents/auth-profiles.js"; import { describeFailoverError } from "../../agents/failover-error.js"; +import { isNonSecretApiKeyMarker } from "../../agents/model-auth-markers.js"; import { getCustomProviderApiKey, resolveEnvApiKey } from "../../agents/model-auth.js"; import { loadModelCatalog } from "../../agents/model-catalog.js"; import { @@ -373,7 +374,8 @@ export async function buildProbeTargets(params: { const envKey = resolveEnvApiKey(providerKey); const customKey = getCustomProviderApiKey(cfg, providerKey); - if (!envKey && !customKey) { + const hasUsableModelsJsonKey = Boolean(customKey && !isNonSecretApiKeyMarker(customKey)); + if (!envKey && !hasUsableModelsJsonKey) { continue; } diff --git a/src/commands/models/list.registry.ts b/src/commands/models/list.registry.ts index a4fd2cdf0f5..187b55176f5 100644 --- a/src/commands/models/list.registry.ts +++ b/src/commands/models/list.registry.ts @@ -94,8 +94,13 @@ function loadAvailableModels(registry: ModelRegistry): Model[] { } } -export async function loadModelRegistry(cfg: OpenClawConfig) { - await ensureOpenClawModelsJson(cfg); +export async function loadModelRegistry( + cfg: OpenClawConfig, + opts?: { sourceConfig?: OpenClawConfig }, +) { + // Persistence must be based on source config (pre-resolution) so SecretRef-managed + // credentials remain markers in models.json for command paths too. + await ensureOpenClawModelsJson(opts?.sourceConfig ?? cfg); const agentDir = resolveOpenClawAgentDir(); const authStorage = discoverAuthStorage(agentDir); const registry = discoverModels(authStorage, agentDir); diff --git a/src/commands/models/load-config.test.ts b/src/commands/models/load-config.test.ts new file mode 100644 index 00000000000..b8969fd4681 --- /dev/null +++ b/src/commands/models/load-config.test.ts @@ -0,0 +1,103 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + loadConfig: vi.fn(), + readConfigFileSnapshotForWrite: vi.fn(), + setRuntimeConfigSnapshot: vi.fn(), + resolveCommandSecretRefsViaGateway: vi.fn(), + getModelsCommandSecretTargetIds: vi.fn(), +})); + +vi.mock("../../config/config.js", () => ({ + loadConfig: mocks.loadConfig, + readConfigFileSnapshotForWrite: mocks.readConfigFileSnapshotForWrite, + setRuntimeConfigSnapshot: mocks.setRuntimeConfigSnapshot, +})); + +vi.mock("../../cli/command-secret-gateway.js", () => ({ + resolveCommandSecretRefsViaGateway: mocks.resolveCommandSecretRefsViaGateway, +})); + +vi.mock("../../cli/command-secret-targets.js", () => ({ + getModelsCommandSecretTargetIds: mocks.getModelsCommandSecretTargetIds, +})); + +import { loadModelsConfig, loadModelsConfigWithSource } from "./load-config.js"; + +describe("models load-config", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("returns source+resolved configs and sets runtime snapshot", async () => { + const sourceConfig = { + models: { + providers: { + openai: { + apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, // pragma: allowlist secret + }, + }, + }, + }; + const runtimeConfig = { + models: { providers: { openai: { apiKey: "sk-runtime" } } }, // pragma: allowlist secret + }; + const resolvedConfig = { + models: { providers: { openai: { apiKey: "sk-resolved" } } }, // pragma: allowlist secret + }; + const targetIds = new Set(["models.providers.*.apiKey"]); + const runtime = { log: vi.fn(), error: vi.fn(), exit: vi.fn() }; + + mocks.loadConfig.mockReturnValue(runtimeConfig); + mocks.readConfigFileSnapshotForWrite.mockResolvedValue({ + snapshot: { valid: true, resolved: sourceConfig }, + writeOptions: {}, + }); + mocks.getModelsCommandSecretTargetIds.mockReturnValue(targetIds); + mocks.resolveCommandSecretRefsViaGateway.mockResolvedValue({ + resolvedConfig, + diagnostics: ["diag-one", "diag-two"], + }); + + const result = await loadModelsConfigWithSource({ commandName: "models list", runtime }); + + expect(mocks.resolveCommandSecretRefsViaGateway).toHaveBeenCalledWith({ + config: runtimeConfig, + commandName: "models list", + targetIds, + }); + expect(mocks.setRuntimeConfigSnapshot).toHaveBeenCalledWith(resolvedConfig, sourceConfig); + expect(runtime.log).toHaveBeenNthCalledWith(1, "[secrets] diag-one"); + expect(runtime.log).toHaveBeenNthCalledWith(2, "[secrets] diag-two"); + expect(result).toEqual({ + sourceConfig, + resolvedConfig, + diagnostics: ["diag-one", "diag-two"], + }); + }); + + it("loadModelsConfig returns resolved config while preserving runtime snapshot behavior", async () => { + const sourceConfig = { models: { providers: {} } }; + const runtimeConfig = { + models: { providers: { openai: { apiKey: "sk-runtime" } } }, // pragma: allowlist secret + }; + const resolvedConfig = { + models: { providers: { openai: { apiKey: "sk-resolved" } } }, // pragma: allowlist secret + }; + const targetIds = new Set(["models.providers.*.apiKey"]); + + mocks.loadConfig.mockReturnValue(runtimeConfig); + mocks.readConfigFileSnapshotForWrite.mockResolvedValue({ + snapshot: { valid: true, resolved: sourceConfig }, + writeOptions: {}, + }); + mocks.getModelsCommandSecretTargetIds.mockReturnValue(targetIds); + mocks.resolveCommandSecretRefsViaGateway.mockResolvedValue({ + resolvedConfig, + diagnostics: [], + }); + + await expect(loadModelsConfig({ commandName: "models list" })).resolves.toBe(resolvedConfig); + expect(mocks.setRuntimeConfigSnapshot).toHaveBeenCalledWith(resolvedConfig, sourceConfig); + }); +}); diff --git a/src/commands/models/load-config.ts b/src/commands/models/load-config.ts index ead48fa8b8a..854cd5240da 100644 --- a/src/commands/models/load-config.ts +++ b/src/commands/models/load-config.ts @@ -1,15 +1,39 @@ import { resolveCommandSecretRefsViaGateway } from "../../cli/command-secret-gateway.js"; import { getModelsCommandSecretTargetIds } from "../../cli/command-secret-targets.js"; -import { loadConfig, type OpenClawConfig } from "../../config/config.js"; +import { + loadConfig, + readConfigFileSnapshotForWrite, + setRuntimeConfigSnapshot, + type OpenClawConfig, +} from "../../config/config.js"; import type { RuntimeEnv } from "../../runtime.js"; -export async function loadModelsConfig(params: { +export type LoadedModelsConfig = { + sourceConfig: OpenClawConfig; + resolvedConfig: OpenClawConfig; + diagnostics: string[]; +}; + +async function loadSourceConfigSnapshot(fallback: OpenClawConfig): Promise { + try { + const { snapshot } = await readConfigFileSnapshotForWrite(); + if (snapshot.valid) { + return snapshot.resolved; + } + } catch { + // Fall back to runtime-loaded config if source snapshot cannot be read. + } + return fallback; +} + +export async function loadModelsConfigWithSource(params: { commandName: string; runtime?: RuntimeEnv; -}): Promise { - const loadedRaw = loadConfig(); +}): Promise { + const runtimeConfig = loadConfig(); + const sourceConfig = await loadSourceConfigSnapshot(runtimeConfig); const { resolvedConfig, diagnostics } = await resolveCommandSecretRefsViaGateway({ - config: loadedRaw, + config: runtimeConfig, commandName: params.commandName, targetIds: getModelsCommandSecretTargetIds(), }); @@ -18,5 +42,17 @@ export async function loadModelsConfig(params: { params.runtime.log(`[secrets] ${entry}`); } } - return resolvedConfig; + setRuntimeConfigSnapshot(resolvedConfig, sourceConfig); + return { + sourceConfig, + resolvedConfig, + diagnostics, + }; +} + +export async function loadModelsConfig(params: { + commandName: string; + runtime?: RuntimeEnv; +}): Promise { + return (await loadModelsConfigWithSource(params)).resolvedConfig; } diff --git a/src/config/config.identity-defaults.test.ts b/src/config/config.identity-defaults.test.ts index 6d25e4c6d16..92a4769c1fd 100644 --- a/src/config/config.identity-defaults.test.ts +++ b/src/config/config.identity-defaults.test.ts @@ -154,6 +154,35 @@ describe("config identity defaults", () => { }); }); + it("accepts SecretRef values in model provider headers", async () => { + await withTempHome("openclaw-config-identity-", async (home) => { + const cfg = await writeAndLoadConfig(home, { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + headers: { + Authorization: { + source: "env", + provider: "default", + id: "OPENAI_HEADER_TOKEN", + }, + }, + models: [], + }, + }, + }, + }); + + expect(cfg.models?.providers?.openai?.headers?.Authorization).toEqual({ + source: "env", + provider: "default", + id: "OPENAI_HEADER_TOKEN", + }); + }); + }); + it("respects empty responsePrefix to disable identity defaults", async () => { await withTempHome("openclaw-config-identity-", async (home) => { const cfg = await writeAndLoadConfig(home, configWithDefaultIdentity({ responsePrefix: "" })); diff --git a/src/config/config.ts b/src/config/config.ts index dfe47d82f87..35fe656c666 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -3,6 +3,7 @@ export { clearRuntimeConfigSnapshot, createConfigIO, getRuntimeConfigSnapshot, + getRuntimeConfigSourceSnapshot, loadConfig, parseConfigJson5, readConfigFileSnapshot, diff --git a/src/config/io.runtime-snapshot-write.test.ts b/src/config/io.runtime-snapshot-write.test.ts index 0a37de08aaa..cca75174500 100644 --- a/src/config/io.runtime-snapshot-write.test.ts +++ b/src/config/io.runtime-snapshot-write.test.ts @@ -5,6 +5,7 @@ import { withTempHome } from "./home-env.test-harness.js"; import { clearConfigCache, clearRuntimeConfigSnapshot, + getRuntimeConfigSourceSnapshot, loadConfig, setRuntimeConfigSnapshot, writeConfigFile, @@ -12,6 +13,70 @@ import { import type { OpenClawConfig } from "./types.js"; describe("runtime config snapshot writes", () => { + it("returns the source snapshot when runtime snapshot is active", async () => { + await withTempHome("openclaw-config-runtime-source-", async () => { + const sourceConfig: OpenClawConfig = { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + models: [], + }, + }, + }, + }; + const runtimeConfig: OpenClawConfig = { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: "sk-runtime-resolved", + models: [], + }, + }, + }, + }; + try { + setRuntimeConfigSnapshot(runtimeConfig, sourceConfig); + expect(getRuntimeConfigSourceSnapshot()).toEqual(sourceConfig); + } finally { + clearRuntimeConfigSnapshot(); + clearConfigCache(); + } + }); + }); + + it("clears runtime source snapshot when runtime snapshot is cleared", async () => { + const sourceConfig: OpenClawConfig = { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + models: [], + }, + }, + }, + }; + const runtimeConfig: OpenClawConfig = { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: "sk-runtime-resolved", + models: [], + }, + }, + }, + }; + + setRuntimeConfigSnapshot(runtimeConfig, sourceConfig); + clearRuntimeConfigSnapshot(); + clearConfigCache(); + expect(getRuntimeConfigSourceSnapshot()).toBeNull(); + }); + it("preserves source secret refs when writeConfigFile receives runtime-resolved config", async () => { await withTempHome("openclaw-config-runtime-write-", async (home) => { const configPath = path.join(home, ".openclaw", "openclaw.json"); diff --git a/src/config/io.ts b/src/config/io.ts index a2a2af5d1b5..d8b90646d12 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -1345,6 +1345,10 @@ export function getRuntimeConfigSnapshot(): OpenClawConfig | null { return runtimeConfigSnapshot; } +export function getRuntimeConfigSourceSnapshot(): OpenClawConfig | null { + return runtimeConfigSourceSnapshot; +} + export function loadConfig(): OpenClawConfig { if (runtimeConfigSnapshot) { return runtimeConfigSnapshot; diff --git a/src/config/schema.help.quality.test.ts b/src/config/schema.help.quality.test.ts index ab685448fdf..f660af8831e 100644 --- a/src/config/schema.help.quality.test.ts +++ b/src/config/schema.help.quality.test.ts @@ -775,6 +775,9 @@ describe("config help copy quality", () => { it("documents auth/model root semantics and provider secret handling", () => { const providerKey = FIELD_HELP["models.providers.*.apiKey"]; expect(/secret|env|credential/i.test(providerKey)).toBe(true); + const modelsMode = FIELD_HELP["models.mode"]; + expect(modelsMode.includes("SecretRef-managed")).toBe(true); + expect(modelsMode.includes("preserve")).toBe(true); const bedrockRefresh = FIELD_HELP["models.bedrockDiscovery.refreshInterval"]; expect(/refresh|seconds|interval/i.test(bedrockRefresh)).toBe(true); diff --git a/src/config/schema.help.ts b/src/config/schema.help.ts index f13944cb127..f0d30c854e7 100644 --- a/src/config/schema.help.ts +++ b/src/config/schema.help.ts @@ -688,7 +688,7 @@ export const FIELD_HELP: Record = { models: "Model catalog root for provider definitions, merge/replace behavior, and optional Bedrock discovery integration. Keep provider definitions explicit and validated before relying on production failover paths.", "models.mode": - 'Controls provider catalog behavior: "merge" keeps built-ins and overlays your custom providers, while "replace" uses only your configured providers. In "merge", matching provider IDs preserve non-empty agent models.json apiKey/baseUrl values and fall back to config when agent values are empty or missing; matching model contextWindow/maxTokens use the higher value between explicit and implicit entries.', + 'Controls provider catalog behavior: "merge" keeps built-ins and overlays your custom providers, while "replace" uses only your configured providers. In "merge", matching provider IDs preserve non-empty agent models.json baseUrl values, while apiKey values are preserved only when the provider is not SecretRef-managed in current config/auth-profile context; SecretRef-managed providers refresh apiKey from current source markers, and matching model contextWindow/maxTokens use the higher value between explicit and implicit entries.', "models.providers": "Provider map keyed by provider ID containing connection/auth settings and concrete model definitions. Use stable provider keys so references from agents and tooling remain portable across environments.", "models.providers.*.baseUrl": diff --git a/src/config/schema.hints.test.ts b/src/config/schema.hints.test.ts index 41ac8b1aa5d..e21a330f2e6 100644 --- a/src/config/schema.hints.test.ts +++ b/src/config/schema.hints.test.ts @@ -135,6 +135,7 @@ describe("mapSensitivePaths", () => { expect(hints["channels.discord.accounts.*.token"]?.sensitive).toBe(true); expect(hints["channels.googlechat.serviceAccount"]?.sensitive).toBe(true); expect(hints["gateway.auth.token"]?.sensitive).toBe(true); + expect(hints["models.providers.*.headers.*"]?.sensitive).toBe(true); expect(hints["skills.entries.*.apiKey"]?.sensitive).toBe(true); }); }); diff --git a/src/config/types.models.ts b/src/config/types.models.ts index 4ef646cc48a..b881269d961 100644 --- a/src/config/types.models.ts +++ b/src/config/types.models.ts @@ -54,7 +54,7 @@ export type ModelProviderConfig = { auth?: ModelProviderAuthMode; api?: ModelApi; injectNumCtxForOpenAICompat?: boolean; - headers?: Record; + headers?: Record; authHeader?: boolean; models: ModelDefinitionConfig[]; }; diff --git a/src/config/zod-schema.core.ts b/src/config/zod-schema.core.ts index 733917e4dac..7ddef789282 100644 --- a/src/config/zod-schema.core.ts +++ b/src/config/zod-schema.core.ts @@ -234,7 +234,7 @@ export const ModelProviderSchema = z .optional(), api: ModelApiSchema.optional(), injectNumCtxForOpenAICompat: z.boolean().optional(), - headers: z.record(z.string(), z.string()).optional(), + headers: z.record(z.string(), SecretInputSchema.register(sensitive)).optional(), authHeader: z.boolean().optional(), models: z.array(ModelDefinitionSchema), }) diff --git a/src/discord/monitor/message-handler.preflight.test.ts b/src/discord/monitor/message-handler.preflight.test.ts index 4a6df90a245..1e4d9c5dddb 100644 --- a/src/discord/monitor/message-handler.preflight.test.ts +++ b/src/discord/monitor/message-handler.preflight.test.ts @@ -150,10 +150,14 @@ function createMessage(params: { id: string; channelId: string; content: string; - author: import("@buape/carbon").Message["author"]; + author: { + id: string; + bot: boolean; + username?: string; + }; mentionedUsers?: Array<{ id: string }>; mentionedEveryone?: boolean; - attachments?: import("@buape/carbon").Message["attachments"]; + attachments?: Array>; }): import("@buape/carbon").Message { return { id: params.id, diff --git a/src/discord/monitor/message-handler.queue.test.ts b/src/discord/monitor/message-handler.queue.test.ts index 9301aad473d..122ce852333 100644 --- a/src/discord/monitor/message-handler.queue.test.ts +++ b/src/discord/monitor/message-handler.queue.test.ts @@ -7,6 +7,7 @@ import { const preflightDiscordMessageMock = vi.hoisted(() => vi.fn()); const processDiscordMessageMock = vi.hoisted(() => vi.fn()); const eventualReplyDeliveredMock = vi.hoisted(() => vi.fn()); +type SetStatusFn = (patch: Record) => void; vi.mock("./message-handler.preflight.js", () => ({ preflightDiscordMessage: preflightDiscordMessageMock, @@ -45,7 +46,7 @@ function createPreflightContext(channelId = "ch-1") { } async function createLifecycleStopScenario(params: { - createHandler: (status: ReturnType) => { + createHandler: (status: SetStatusFn) => { handler: (data: never, opts: never) => Promise; stop: () => void; }; @@ -59,7 +60,7 @@ async function createLifecycleStopScenario(params: { createPreflightContext(contextParams.data.channel_id), ); - const setStatus = vi.fn(); + const setStatus = vi.fn(); const { handler, stop } = params.createHandler(setStatus); await expect(handler(createMessageData("m-1") as never, {} as never)).resolves.toBeUndefined(); diff --git a/src/discord/monitor/message-handler.test-helpers.ts b/src/discord/monitor/message-handler.test-helpers.ts index 3faf3ca856a..6084fc1a00e 100644 --- a/src/discord/monitor/message-handler.test-helpers.ts +++ b/src/discord/monitor/message-handler.test-helpers.ts @@ -1,5 +1,6 @@ import { vi } from "vitest"; import type { OpenClawConfig } from "../../config/types.js"; +import type { createDiscordMessageHandler } from "./message-handler.js"; import { createNoopThreadBindingManager } from "./thread-bindings.js"; export const DEFAULT_DISCORD_BOT_USER_ID = "bot-123"; @@ -9,7 +10,7 @@ export function createDiscordHandlerParams(overrides?: { setStatus?: (patch: Record) => void; abortSignal?: AbortSignal; workerRunTimeoutMs?: number; -}) { +}): Parameters[0] { const cfg: OpenClawConfig = { channels: { discord: { diff --git a/src/infra/provider-usage.auth.normalizes-keys.test.ts b/src/infra/provider-usage.auth.normalizes-keys.test.ts index bae5ae5a7d9..f30363205a9 100644 --- a/src/infra/provider-usage.auth.normalizes-keys.test.ts +++ b/src/infra/provider-usage.auth.normalizes-keys.test.ts @@ -2,6 +2,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { NON_ENV_SECRETREF_MARKER } from "../agents/model-auth-markers.js"; import { resolveProviderAuths } from "./provider-usage.auth.js"; describe("resolveProviderAuths key normalization", () => { @@ -403,4 +404,76 @@ describe("resolveProviderAuths key normalization", () => { expect(auths).toEqual([{ provider: "anthropic", token: "token-1" }]); }, {}); }); + + it("ignores marker-backed config keys for provider usage auth resolution", async () => { + await withSuiteHome( + async (home) => { + const modelDef = { + id: "test-model", + name: "Test Model", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 1024, + maxTokens: 256, + }; + await writeConfig(home, { + models: { + providers: { + minimax: { + baseUrl: "https://api.minimaxi.com", + models: [modelDef], + apiKey: NON_ENV_SECRETREF_MARKER, + }, + }, + }, + }); + + const auths = await resolveProviderAuths({ + providers: ["minimax"], + }); + expect(auths).toEqual([]); + }, + { + MINIMAX_API_KEY: undefined, + MINIMAX_CODE_PLAN_KEY: undefined, + }, + ); + }); + + it("keeps all-caps plaintext config keys eligible for provider usage auth resolution", async () => { + await withSuiteHome( + async (home) => { + const modelDef = { + id: "test-model", + name: "Test Model", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 1024, + maxTokens: 256, + }; + await writeConfig(home, { + models: { + providers: { + minimax: { + baseUrl: "https://api.minimaxi.com", + models: [modelDef], + apiKey: "ALLCAPS_SAMPLE", // pragma: allowlist secret + }, + }, + }, + }); + + const auths = await resolveProviderAuths({ + providers: ["minimax"], + }); + expect(auths).toEqual([{ provider: "minimax", token: "ALLCAPS_SAMPLE" }]); + }, + { + MINIMAX_API_KEY: undefined, + MINIMAX_CODE_PLAN_KEY: undefined, + }, + ); + }); }); diff --git a/src/infra/provider-usage.auth.ts b/src/infra/provider-usage.auth.ts index ff63c1570f1..6afa4bebaad 100644 --- a/src/infra/provider-usage.auth.ts +++ b/src/infra/provider-usage.auth.ts @@ -8,6 +8,7 @@ import { resolveApiKeyForProfile, resolveAuthProfileOrder, } from "../agents/auth-profiles.js"; +import { isNonSecretApiKeyMarker } from "../agents/model-auth-markers.js"; import { getCustomProviderApiKey } from "../agents/model-auth.js"; import { normalizeProviderId } from "../agents/model-selection.js"; import { loadConfig } from "../config/config.js"; @@ -103,7 +104,7 @@ function resolveProviderApiKeyFromConfigAndStore(params: { const cfg = loadConfig(); const key = getCustomProviderApiKey(cfg, params.providerId); - if (key) { + if (key && !isNonSecretApiKeyMarker(key)) { return key; } @@ -122,9 +123,17 @@ function resolveProviderApiKeyFromConfigAndStore(params: { return undefined; } if (cred.type === "api_key") { - return normalizeSecretInput(cred.key); + const key = normalizeSecretInput(cred.key); + if (key && !isNonSecretApiKeyMarker(key)) { + return key; + } + return undefined; } - return normalizeSecretInput(cred.token); + const token = normalizeSecretInput(cred.token); + if (token && !isNonSecretApiKeyMarker(token)) { + return token; + } + return undefined; } async function resolveOAuthToken(params: { diff --git a/src/line/bot-handlers.test.ts b/src/line/bot-handlers.test.ts index 52b2174fa1d..a6890a2d1d6 100644 --- a/src/line/bot-handlers.test.ts +++ b/src/line/bot-handlers.test.ts @@ -65,6 +65,7 @@ const { readAllowFromStoreMock, upsertPairingRequestMock } = vi.hoisted(() => ({ let handleLineWebhookEvents: typeof import("./bot-handlers.js").handleLineWebhookEvents; let createLineWebhookReplayCache: typeof import("./bot-handlers.js").createLineWebhookReplayCache; +type LineWebhookContext = Parameters[1]; const createRuntime = () => ({ log: vi.fn(), error: vi.fn(), exit: vi.fn() }); @@ -88,7 +89,7 @@ function createReplayMessageEvent(params: { } function createOpenGroupReplayContext( - processMessage: ReturnType, + processMessage: LineWebhookContext["processMessage"], replayCache: ReturnType, ): Parameters[1] { return { diff --git a/src/media-understanding/runner.deepgram.test.ts b/src/media-understanding/runner.deepgram.test.ts index 38df19b7432..253c8d6eefa 100644 --- a/src/media-understanding/runner.deepgram.test.ts +++ b/src/media-understanding/runner.deepgram.test.ts @@ -29,7 +29,10 @@ describe("runCapability deepgram provider options", () => { deepgram: { baseUrl: "https://provider.example", apiKey: "test-key", - headers: { "X-Provider": "1" }, + headers: { + "X-Provider": "1", + "X-Provider-Managed": "secretref-managed", + }, models: [], }, }, @@ -39,7 +42,10 @@ describe("runCapability deepgram provider options", () => { audio: { enabled: true, baseUrl: "https://config.example", - headers: { "X-Config": "2" }, + headers: { + "X-Config": "2", + "X-Config-Managed": "secretref-env:DEEPGRAM_HEADER_TOKEN", + }, providerOptions: { deepgram: { detect_language: true, @@ -52,7 +58,10 @@ describe("runCapability deepgram provider options", () => { provider: "deepgram", model: "nova-3", baseUrl: "https://entry.example", - headers: { "X-Entry": "3" }, + headers: { + "X-Entry": "3", + "X-Entry-Managed": "secretref-managed", + }, providerOptions: { deepgram: { detectLanguage: false, @@ -79,8 +88,11 @@ describe("runCapability deepgram provider options", () => { expect(seenBaseUrl).toBe("https://entry.example"); expect(seenHeaders).toMatchObject({ "X-Provider": "1", + "X-Provider-Managed": "secretref-managed", "X-Config": "2", + "X-Config-Managed": "secretref-env:DEEPGRAM_HEADER_TOKEN", "X-Entry": "3", + "X-Entry-Managed": "secretref-managed", }); expect(seenQuery).toMatchObject({ detect_language: false, diff --git a/src/media-understanding/runner.entries.ts b/src/media-understanding/runner.entries.ts index 8423ece464d..cdd9468c4a7 100644 --- a/src/media-understanding/runner.entries.ts +++ b/src/media-understanding/runner.entries.ts @@ -40,6 +40,26 @@ import { estimateBase64Size, resolveVideoMaxBase64Bytes } from "./video.js"; export type ProviderRegistry = Map; +function sanitizeProviderHeaders( + headers: Record | undefined, +): Record | undefined { + if (!headers) { + return undefined; + } + const next: Record = {}; + for (const [key, value] of Object.entries(headers)) { + if (typeof value !== "string") { + continue; + } + // Intentionally preserve marker-shaped values here. This path handles + // explicit config/runtime provider headers, where literal values may + // legitimately match marker patterns; discovered models.json entries are + // sanitized separately in the model registry path. + next[key] = value; + } + return Object.keys(next).length > 0 ? next : undefined; +} + function trimOutput(text: string, maxChars?: number): string { const trimmed = text.trim(); if (!maxChars || trimmed.length <= maxChars) { @@ -352,9 +372,9 @@ async function resolveProviderExecutionContext(params: { }); const baseUrl = params.entry.baseUrl ?? params.config?.baseUrl ?? providerConfig?.baseUrl; const mergedHeaders = { - ...providerConfig?.headers, - ...params.config?.headers, - ...params.entry.headers, + ...sanitizeProviderHeaders(providerConfig?.headers as Record | undefined), + ...sanitizeProviderHeaders(params.config?.headers as Record | undefined), + ...sanitizeProviderHeaders(params.entry.headers as Record | undefined), }; const headers = Object.keys(mergedHeaders).length > 0 ? mergedHeaders : undefined; return { apiKeys, baseUrl, headers }; diff --git a/src/secrets/apply.test.ts b/src/secrets/apply.test.ts index a8e5ecd0cf8..7f097ef5d43 100644 --- a/src/secrets/apply.test.ts +++ b/src/secrets/apply.test.ts @@ -149,6 +149,18 @@ function createOpenAiProviderTarget(params?: { }; } +function createOpenAiProviderHeaderTarget(params?: { + path?: string; + pathSegments?: string[]; +}): SecretsApplyPlan["targets"][number] { + return { + type: "models.providers.headers", + path: params?.path ?? "models.providers.openai.headers.x-api-key", + ...(params?.pathSegments ? { pathSegments: params.pathSegments } : {}), + ref: OPENAI_API_KEY_ENV_REF, + }; +} + function createOneWayScrubOptions(): NonNullable { return { scrubEnv: true, @@ -436,6 +448,47 @@ describe("secrets apply", () => { }); }); + it("applies model provider header targets", async () => { + await writeJsonFile(fixture.configPath, { + models: { + providers: { + openai: { + ...createOpenAiProviderConfig(), + headers: { + "x-api-key": "sk-header-plaintext", + }, + }, + }, + }, + }); + + const plan = createPlan({ + targets: [ + createOpenAiProviderHeaderTarget({ + pathSegments: ["models", "providers", "openai", "headers", "x-api-key"], + }), + ], + options: { + scrubEnv: false, + scrubAuthProfilesForProviderTargets: false, + scrubLegacyAuthJson: false, + }, + }); + + const nextConfig = await applyPlanAndReadConfig<{ + models?: { + providers?: { + openai?: { + headers?: Record; + }; + }; + }; + }>(fixture, plan); + expect(nextConfig.models?.providers?.openai?.headers?.["x-api-key"]).toEqual( + OPENAI_API_KEY_ENV_REF, + ); + }); + it("applies array-indexed targets for agent memory search", async () => { await fs.writeFile( fixture.configPath, diff --git a/src/secrets/audit.test.ts b/src/secrets/audit.test.ts index cd85d84d3d8..b797494d54a 100644 --- a/src/secrets/audit.test.ts +++ b/src/secrets/audit.test.ts @@ -10,10 +10,13 @@ type AuditFixture = { configPath: string; authStorePath: string; authJsonPath: string; + modelsPath: string; envPath: string; env: NodeJS.ProcessEnv; }; +const OPENAI_API_KEY_MARKER = "OPENAI_API_KEY"; // pragma: allowlist secret + async function writeJsonFile(filePath: string, value: unknown): Promise { await fs.writeFile(filePath, `${JSON.stringify(value, null, 2)}\n`, "utf8"); } @@ -27,9 +30,11 @@ function resolveRuntimePathEnv(): string { function hasFinding( report: Awaited>, - predicate: (entry: { code: string; file: string }) => boolean, + predicate: (entry: { code: string; file: string; jsonPath?: string }) => boolean, ): boolean { - return report.findings.some((entry) => predicate(entry as { code: string; file: string })); + return report.findings.some((entry) => + predicate(entry as { code: string; file: string; jsonPath?: string }), + ); } async function createAuditFixture(): Promise { @@ -38,6 +43,7 @@ async function createAuditFixture(): Promise { const configPath = path.join(stateDir, "openclaw.json"); const authStorePath = path.join(stateDir, "agents", "main", "agent", "auth-profiles.json"); const authJsonPath = path.join(stateDir, "agents", "main", "agent", "auth.json"); + const modelsPath = path.join(stateDir, "agents", "main", "agent", "models.json"); const envPath = path.join(stateDir, ".env"); await fs.mkdir(path.dirname(configPath), { recursive: true }); @@ -49,6 +55,7 @@ async function createAuditFixture(): Promise { configPath, authStorePath, authJsonPath, + modelsPath, envPath, env: { OPENCLAW_STATE_DIR: stateDir, @@ -64,7 +71,7 @@ async function seedAuditFixture(fixture: AuditFixture): Promise { openai: { baseUrl: "https://api.openai.com/v1", api: "openai-completions", - apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + apiKey: { source: "env", provider: "default", id: OPENAI_API_KEY_MARKER }, models: [{ id: "gpt-5", name: "gpt-5" }], }, }; @@ -85,7 +92,21 @@ async function seedAuditFixture(fixture: AuditFixture): Promise { version: 1, profiles: Object.fromEntries(seededProfiles), }); - await fs.writeFile(fixture.envPath, "OPENAI_API_KEY=sk-openai-plaintext\n", "utf8"); + await writeJsonFile(fixture.modelsPath, { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + apiKey: OPENAI_API_KEY_MARKER, + models: [{ id: "gpt-5", name: "gpt-5" }], + }, + }, + }); + await fs.writeFile( + fixture.envPath, + `${OPENAI_API_KEY_MARKER}=sk-openai-plaintext\n`, // pragma: allowlist secret + "utf8", + ); } describe("secrets audit", () => { @@ -254,4 +275,244 @@ describe("secrets audit", () => { const callCount = callLog.split("\n").filter((line) => line.trim().length > 0).length; expect(callCount).toBe(1); }); + + it("scans agent models.json files for plaintext provider apiKey values", async () => { + await writeJsonFile(fixture.modelsPath, { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + apiKey: "sk-models-plaintext", // pragma: allowlist secret + models: [{ id: "gpt-5", name: "gpt-5" }], + }, + }, + }); + + const report = await runSecretsAudit({ env: fixture.env }); + expect( + hasFinding( + report, + (entry) => + entry.code === "PLAINTEXT_FOUND" && + entry.file === fixture.modelsPath && + entry.jsonPath === "providers.openai.apiKey", + ), + ).toBe(true); + expect(report.filesScanned).toContain(fixture.modelsPath); + }); + + it("scans agent models.json files for plaintext provider header values", async () => { + await writeJsonFile(fixture.modelsPath, { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + apiKey: OPENAI_API_KEY_MARKER, + headers: { + Authorization: "Bearer sk-header-plaintext", // pragma: allowlist secret + }, + models: [{ id: "gpt-5", name: "gpt-5" }], + }, + }, + }); + + const report = await runSecretsAudit({ env: fixture.env }); + expect( + hasFinding( + report, + (entry) => + entry.code === "PLAINTEXT_FOUND" && + entry.file === fixture.modelsPath && + entry.jsonPath === "providers.openai.headers.Authorization", + ), + ).toBe(true); + }); + + it("does not flag non-sensitive routing headers in models.json", async () => { + await writeJsonFile(fixture.modelsPath, { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + apiKey: OPENAI_API_KEY_MARKER, + headers: { + "X-Proxy-Region": "us-west", + }, + models: [{ id: "gpt-5", name: "gpt-5" }], + }, + }, + }); + + const report = await runSecretsAudit({ env: fixture.env }); + expect( + hasFinding( + report, + (entry) => + entry.code === "PLAINTEXT_FOUND" && + entry.file === fixture.modelsPath && + entry.jsonPath === "providers.openai.headers.X-Proxy-Region", + ), + ).toBe(false); + }); + + it("does not flag models.json marker values as plaintext", async () => { + await writeJsonFile(fixture.modelsPath, { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + apiKey: OPENAI_API_KEY_MARKER, + models: [{ id: "gpt-5", name: "gpt-5" }], + }, + }, + }); + + const report = await runSecretsAudit({ env: fixture.env }); + expect( + hasFinding( + report, + (entry) => + entry.code === "PLAINTEXT_FOUND" && + entry.file === fixture.modelsPath && + entry.jsonPath === "providers.openai.apiKey", + ), + ).toBe(false); + }); + + it("flags arbitrary all-caps models.json apiKey values as plaintext", async () => { + await writeJsonFile(fixture.modelsPath, { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + apiKey: "ALLCAPS_SAMPLE", // pragma: allowlist secret + models: [{ id: "gpt-5", name: "gpt-5" }], + }, + }, + }); + + const report = await runSecretsAudit({ env: fixture.env }); + expect( + hasFinding( + report, + (entry) => + entry.code === "PLAINTEXT_FOUND" && + entry.file === fixture.modelsPath && + entry.jsonPath === "providers.openai.apiKey", + ), + ).toBe(true); + }); + + it("does not flag models.json header marker values as plaintext", async () => { + await writeJsonFile(fixture.modelsPath, { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + apiKey: OPENAI_API_KEY_MARKER, + headers: { + Authorization: "secretref-env:OPENAI_HEADER_TOKEN", // pragma: allowlist secret + "x-managed-token": "secretref-managed", // pragma: allowlist secret + }, + models: [{ id: "gpt-5", name: "gpt-5" }], + }, + }, + }); + + const report = await runSecretsAudit({ env: fixture.env }); + expect( + hasFinding( + report, + (entry) => + entry.code === "PLAINTEXT_FOUND" && + entry.file === fixture.modelsPath && + entry.jsonPath === "providers.openai.headers.Authorization", + ), + ).toBe(false); + expect( + hasFinding( + report, + (entry) => + entry.code === "PLAINTEXT_FOUND" && + entry.file === fixture.modelsPath && + entry.jsonPath === "providers.openai.headers.x-managed-token", + ), + ).toBe(false); + }); + + it("reports unresolved models.json SecretRef objects in provider headers", async () => { + await writeJsonFile(fixture.modelsPath, { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + apiKey: OPENAI_API_KEY_MARKER, + headers: { + Authorization: { + source: "env", + provider: "default", + id: "OPENAI_HEADER_TOKEN", // pragma: allowlist secret + }, + }, + models: [{ id: "gpt-5", name: "gpt-5" }], + }, + }, + }); + + const report = await runSecretsAudit({ env: fixture.env }); + expect( + hasFinding( + report, + (entry) => + entry.code === "REF_UNRESOLVED" && + entry.file === fixture.modelsPath && + entry.jsonPath === "providers.openai.headers.Authorization", + ), + ).toBe(true); + }); + + it("reports malformed models.json as unresolved findings", async () => { + await fs.writeFile(fixture.modelsPath, "{bad-json", "utf8"); + const report = await runSecretsAudit({ env: fixture.env }); + expect( + hasFinding( + report, + (entry) => entry.code === "REF_UNRESOLVED" && entry.file === fixture.modelsPath, + ), + ).toBe(true); + }); + + it("does not flag non-sensitive routing headers in openclaw config", async () => { + await writeJsonFile(fixture.configPath, { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + apiKey: { source: "env", provider: "default", id: OPENAI_API_KEY_MARKER }, + headers: { + "X-Proxy-Region": "us-west", + }, + models: [{ id: "gpt-5", name: "gpt-5" }], + }, + }, + }, + }); + await writeJsonFile(fixture.authStorePath, { + version: 1, + profiles: {}, + }); + await fs.writeFile(fixture.envPath, "", "utf8"); + + const report = await runSecretsAudit({ env: fixture.env }); + expect( + hasFinding( + report, + (entry) => + entry.code === "PLAINTEXT_FOUND" && + entry.file === fixture.configPath && + entry.jsonPath === "models.providers.openai.headers.X-Proxy-Region", + ), + ).toBe(false); + }); }); diff --git a/src/secrets/audit.ts b/src/secrets/audit.ts index 132ea4ac431..3215b3ce855 100644 --- a/src/secrets/audit.ts +++ b/src/secrets/audit.ts @@ -1,8 +1,13 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { + isNonSecretApiKeyMarker, + isSecretRefHeaderValueMarker, +} from "../agents/model-auth-markers.js"; import { normalizeProviderId } from "../agents/model-selection.js"; import { resolveStateDir, type OpenClawConfig } from "../config/config.js"; +import { coerceSecretRef } from "../config/types.secrets.js"; import { resolveSecretInputRef, type SecretRef } from "../config/types.secrets.js"; import { resolveConfigDir, resolveUserPath } from "../utils.js"; import { runTasksWithConcurrency } from "../utils/run-with-concurrency.js"; @@ -23,6 +28,7 @@ import { import { isNonEmptyString, isRecord } from "./shared.js"; import { describeUnknownError } from "./shared.js"; import { + listAgentModelsJsonPaths, listAuthProfileStorePaths, listLegacyAuthJsonPaths, parseEnvAssignmentValue, @@ -91,6 +97,40 @@ type AuditCollector = { }; const REF_RESOLVE_FALLBACK_CONCURRENCY = 8; +const ALWAYS_SENSITIVE_MODEL_PROVIDER_HEADER_NAMES = new Set([ + "authorization", + "proxy-authorization", + "x-api-key", + "api-key", + "apikey", + "x-auth-token", + "auth-token", + "x-access-token", + "access-token", + "x-secret-key", + "secret-key", +]); +const SENSITIVE_MODEL_PROVIDER_HEADER_NAME_FRAGMENTS = [ + "api-key", + "apikey", + "token", + "secret", + "password", + "credential", +]; + +function isLikelySensitiveModelProviderHeaderName(value: string): boolean { + const normalized = value.trim().toLowerCase(); + if (!normalized) { + return false; + } + if (ALWAYS_SENSITIVE_MODEL_PROVIDER_HEADER_NAMES.has(normalized)) { + return true; + } + return SENSITIVE_MODEL_PROVIDER_HEADER_NAME_FRAGMENTS.some((fragment) => + normalized.includes(fragment), + ); +} function addFinding(collector: AuditCollector, finding: SecretsAuditFinding): void { collector.findings.push(finding); @@ -192,6 +232,12 @@ function collectConfigSecrets(params: { target.value, target.entry.expectedResolvedValue, ); + if ( + target.entry.id === "models.providers.*.headers.*" && + !isLikelySensitiveModelProviderHeaderName(target.pathSegments.at(-1) ?? "") + ) { + continue; + } if (!hasPlaintext) { continue; } @@ -315,6 +361,93 @@ function collectAuthJsonResidue(params: { stateDir: string; collector: AuditColl } } +function collectModelsJsonSecrets(params: { + modelsJsonPath: string; + collector: AuditCollector; +}): void { + if (!fs.existsSync(params.modelsJsonPath)) { + return; + } + params.collector.filesScanned.add(params.modelsJsonPath); + const parsedResult = readJsonObjectIfExists(params.modelsJsonPath); + if (parsedResult.error) { + addFinding(params.collector, { + code: "REF_UNRESOLVED", + severity: "error", + file: params.modelsJsonPath, + jsonPath: "", + message: `Invalid JSON in models.json: ${parsedResult.error}`, + }); + return; + } + const parsed = parsedResult.value; + if (!parsed || !isRecord(parsed.providers)) { + return; + } + for (const [providerId, providerValue] of Object.entries(parsed.providers)) { + if (!isRecord(providerValue)) { + continue; + } + const apiKey = providerValue.apiKey; + if (coerceSecretRef(apiKey)) { + addFinding(params.collector, { + code: "REF_UNRESOLVED", + severity: "error", + file: params.modelsJsonPath, + jsonPath: `providers.${providerId}.apiKey`, + message: "models.json contains an unresolved SecretRef object; regenerate models.json.", + provider: providerId, + }); + } else if (isNonEmptyString(apiKey) && !isNonSecretApiKeyMarker(apiKey)) { + addFinding(params.collector, { + code: "PLAINTEXT_FOUND", + severity: "warn", + file: params.modelsJsonPath, + jsonPath: `providers.${providerId}.apiKey`, + message: "models.json provider apiKey is stored as plaintext.", + provider: providerId, + }); + } + + const headers = isRecord(providerValue.headers) ? providerValue.headers : undefined; + if (!headers) { + continue; + } + for (const [headerKey, headerValue] of Object.entries(headers)) { + const headerPath = `providers.${providerId}.headers.${headerKey}`; + if (coerceSecretRef(headerValue)) { + addFinding(params.collector, { + code: "REF_UNRESOLVED", + severity: "error", + file: params.modelsJsonPath, + jsonPath: headerPath, + message: + "models.json contains an unresolved SecretRef object for provider headers; regenerate models.json.", + provider: providerId, + }); + continue; + } + if (!isNonEmptyString(headerValue)) { + continue; + } + if (isSecretRefHeaderValueMarker(headerValue)) { + continue; + } + if (!isLikelySensitiveModelProviderHeaderName(headerKey)) { + continue; + } + addFinding(params.collector, { + code: "PLAINTEXT_FOUND", + severity: "warn", + file: params.modelsJsonPath, + jsonPath: headerPath, + message: "models.json provider header value is stored as plaintext.", + provider: providerId, + }); + } + } +} + async function collectUnresolvedRefFindings(params: { collector: AuditCollector; config: OpenClawConfig; @@ -497,6 +630,12 @@ export async function runSecretsAudit( defaults, }); } + for (const modelsJsonPath of listAgentModelsJsonPaths(config, stateDir)) { + collectModelsJsonSecrets({ + modelsJsonPath, + collector, + }); + } await collectUnresolvedRefFindings({ collector, config, diff --git a/src/secrets/plan.test.ts b/src/secrets/plan.test.ts index 95071d549e1..01ee81ea551 100644 --- a/src/secrets/plan.test.ts +++ b/src/secrets/plan.test.ts @@ -21,6 +21,22 @@ describe("secrets plan validation", () => { expect(resolved?.pathSegments).toEqual(["channels", "telegram", "botToken"]); }); + it("accepts model provider header targets with wildcard-backed paths", () => { + const resolved = resolveValidatedPlanTarget({ + type: "models.providers.headers", + path: "models.providers.openai.headers.x-api-key", + pathSegments: ["models", "providers", "openai", "headers", "x-api-key"], + providerId: "openai", + }); + expect(resolved?.pathSegments).toEqual([ + "models", + "providers", + "openai", + "headers", + "x-api-key", + ]); + }); + it("rejects target paths that do not match the registered shape", () => { const resolved = resolveValidatedPlanTarget({ type: "channels.telegram.botToken", diff --git a/src/secrets/runtime-config-collectors-core.ts b/src/secrets/runtime-config-collectors-core.ts index 085573173cc..504331f0a96 100644 --- a/src/secrets/runtime-config-collectors-core.ts +++ b/src/secrets/runtime-config-collectors-core.ts @@ -10,6 +10,7 @@ import { isRecord } from "./shared.js"; type ProviderLike = { apiKey?: unknown; + headers?: unknown; enabled?: unknown; }; @@ -24,18 +25,37 @@ function collectModelProviderAssignments(params: { context: ResolverContext; }): void { for (const [providerId, provider] of Object.entries(params.providers)) { + const providerIsActive = provider.enabled !== false; collectSecretInputAssignment({ value: provider.apiKey, path: `models.providers.${providerId}.apiKey`, expected: "string", defaults: params.defaults, context: params.context, - active: provider.enabled !== false, + active: providerIsActive, inactiveReason: "provider is disabled.", apply: (value) => { provider.apiKey = value; }, }); + const headers = isRecord(provider.headers) ? provider.headers : undefined; + if (!headers) { + continue; + } + for (const [headerKey, headerValue] of Object.entries(headers)) { + collectSecretInputAssignment({ + value: headerValue, + path: `models.providers.${providerId}.headers.${headerKey}`, + expected: "string", + defaults: params.defaults, + context: params.context, + active: providerIsActive, + inactiveReason: "provider is disabled.", + apply: (value) => { + headers[headerKey] = value; + }, + }); + } } } diff --git a/src/secrets/runtime.test.ts b/src/secrets/runtime.test.ts index e1ca5774a75..1d9189f843c 100644 --- a/src/secrets/runtime.test.ts +++ b/src/secrets/runtime.test.ts @@ -56,6 +56,13 @@ describe("secrets runtime snapshot", () => { openai: { baseUrl: "https://api.openai.com/v1", apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + headers: { + Authorization: { + source: "env", + provider: "default", + id: "OPENAI_PROVIDER_AUTH_HEADER", + }, + }, models: [], }, }, @@ -123,6 +130,7 @@ describe("secrets runtime snapshot", () => { config, env: { OPENAI_API_KEY: "sk-env-openai", // pragma: allowlist secret + OPENAI_PROVIDER_AUTH_HEADER: "Bearer sk-env-header", // pragma: allowlist secret GITHUB_TOKEN: "ghp-env-token", // pragma: allowlist secret REVIEW_SKILL_API_KEY: "sk-skill-ref", // pragma: allowlist secret MEMORY_REMOTE_API_KEY: "mem-ref-key", // pragma: allowlist secret @@ -162,6 +170,9 @@ describe("secrets runtime snapshot", () => { }); expect(snapshot.config.models?.providers?.openai?.apiKey).toBe("sk-env-openai"); + expect(snapshot.config.models?.providers?.openai?.headers?.Authorization).toBe( + "Bearer sk-env-header", + ); expect(snapshot.config.skills?.entries?.["review-pr"]?.apiKey).toBe("sk-skill-ref"); expect(snapshot.config.agents?.defaults?.memorySearch?.remote?.apiKey).toBe("mem-ref-key"); expect(snapshot.config.talk?.apiKey).toBe("talk-ref-key"); diff --git a/src/secrets/storage-scan.ts b/src/secrets/storage-scan.ts index ccbfc544f6d..557f611c006 100644 --- a/src/secrets/storage-scan.ts +++ b/src/secrets/storage-scan.ts @@ -1,5 +1,6 @@ import fs from "node:fs"; import path from "node:path"; +import { listAgentIds, resolveAgentDir } from "../agents/agent-scope.js"; import type { OpenClawConfig } from "../config/config.js"; import { resolveUserPath } from "../utils.js"; import { listAuthProfileStorePaths as listAuthProfileStorePathsFromAuthStorePaths } from "./auth-store-paths.js"; @@ -31,6 +32,32 @@ export function listLegacyAuthJsonPaths(stateDir: string): string[] { return out; } +export function listAgentModelsJsonPaths(config: OpenClawConfig, stateDir: string): string[] { + const paths = new Set(); + paths.add(path.join(resolveUserPath(stateDir), "agents", "main", "agent", "models.json")); + + const agentsRoot = path.join(resolveUserPath(stateDir), "agents"); + if (fs.existsSync(agentsRoot)) { + for (const entry of fs.readdirSync(agentsRoot, { withFileTypes: true })) { + if (!entry.isDirectory()) { + continue; + } + paths.add(path.join(agentsRoot, entry.name, "agent", "models.json")); + } + } + + for (const agentId of listAgentIds(config)) { + if (agentId === "main") { + paths.add(path.join(resolveUserPath(stateDir), "agents", "main", "agent", "models.json")); + continue; + } + const agentDir = resolveAgentDir(config, agentId); + paths.add(path.join(resolveUserPath(agentDir), "models.json")); + } + + return [...paths]; +} + export function readJsonObjectIfExists(filePath: string): { value: Record | null; error?: string; diff --git a/src/secrets/target-registry-data.ts b/src/secrets/target-registry-data.ts index 61ccb1f9b66..3be4992d28f 100644 --- a/src/secrets/target-registry-data.ts +++ b/src/secrets/target-registry-data.ts @@ -642,6 +642,19 @@ const SECRET_TARGET_REGISTRY: SecretTargetRegistryEntry[] = [ providerIdPathSegmentIndex: 2, trackProviderShadowing: true, }, + { + id: "models.providers.*.headers.*", + targetType: "models.providers.headers", + targetTypeAliases: ["models.providers.*.headers.*"], + configFile: "openclaw.json", + pathPattern: "models.providers.*.headers.*", + secretShape: SECRET_INPUT_SHAPE, + expectedResolvedValue: "string", + includeInPlan: true, + includeInConfigure: true, + includeInAudit: true, + providerIdPathSegmentIndex: 2, + }, { id: "skills.entries.*.apiKey", targetType: "skills.entries.apiKey", diff --git a/src/secrets/target-registry-pattern.test.ts b/src/secrets/target-registry-pattern.test.ts index 4739ca5776d..2cd3537fb53 100644 --- a/src/secrets/target-registry-pattern.test.ts +++ b/src/secrets/target-registry-pattern.test.ts @@ -39,6 +39,17 @@ describe("target registry pattern helpers", () => { expect(materializePathTokens(refTokens, ["anthropic"])).toBeNull(); }); + it("matches two wildcard captures in five-segment header paths", () => { + const tokens = parsePathPattern("models.providers.*.headers.*"); + const match = matchPathTokens( + ["models", "providers", "openai", "headers", "x-api-key"], + tokens, + ); + expect(match).toEqual({ + captures: ["openai", "x-api-key"], + }); + }); + it("expands wildcard and array patterns over config objects", () => { const root = { agents: { diff --git a/src/telegram/bot-native-commands.session-meta.test.ts b/src/telegram/bot-native-commands.session-meta.test.ts index ef6de63057b..1b05ddd0d9c 100644 --- a/src/telegram/bot-native-commands.session-meta.test.ts +++ b/src/telegram/bot-native-commands.session-meta.test.ts @@ -238,7 +238,7 @@ function createConfiguredAcpTopicBinding(boundSessionKey: string) { status: "active", boundAt: 0, }, - }; + } satisfies import("../acp/persistent-bindings.js").ResolvedConfiguredAcpBinding; } function expectUnauthorizedNewCommandBlocked(sendMessage: ReturnType) {