diff --git a/CHANGELOG.md b/CHANGELOG.md index fb7c3ba9402..ce7e683df38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Docs: https://docs.openclaw.ai - Security/proxy attachments: restore the shared media-store size cap for persisted browser proxy files so oversized payloads are rejected instead of overriding the intended 5 MB limit. (`GHSA-6rph-mmhp-h7h9`)(#43684) Thanks @tdjackey and @vincentkoc. - Security/host env: block inherited `GIT_EXEC_PATH` from sanitized host exec environments so Git helper resolution cannot be steered by host environment state. (`GHSA-jf5v-pqgw-gm5m`)(#43685) Thanks @zpbrent and @vincentkoc. - Security/session_status: enforce sandbox session-tree visibility and shared agent-to-agent access guards before reading or mutating target session state, so sandboxed subagents can no longer inspect parent session metadata or write parent model overrides via `session_status`. (`GHSA-wcxr-59v9-rxr8`)(#43754) Thanks @tdjackey and @vincentkoc. +- Models/secrets: enforce source-managed SecretRef markers in generated `models.json` so runtime-resolved provider secrets are not persisted when runtime projection is skipped. (#43759) Thanks @joshavant. ### Changes diff --git a/docs/cli/agent.md b/docs/cli/agent.md index 93c8d04b41a..430bdf50743 100644 --- a/docs/cli/agent.md +++ b/docs/cli/agent.md @@ -25,4 +25,5 @@ openclaw agent --agent ops --message "Generate report" --deliver --reply-channel ## 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. +- When this command triggers `models.json` regeneration, SecretRef-managed provider credentials are persisted as non-secret markers (for example env var names, `secretref-env:ENV_VAR_NAME`, or `secretref-managed`), not resolved secret plaintext. +- Marker writes are source-authoritative: OpenClaw persists markers from the active source config snapshot, not from resolved runtime secret values. diff --git a/docs/concepts/models.md b/docs/concepts/models.md index f87eead821c..6323feef04e 100644 --- a/docs/concepts/models.md +++ b/docs/concepts/models.md @@ -207,7 +207,7 @@ mode, pass `--yes` to accept defaults. ## Models registry (`models.json`) Custom providers in `models.providers` are written into `models.json` under the -agent directory (default `~/.openclaw/agents//models.json`). This file +agent directory (default `~/.openclaw/agents//agent/models.json`). This file is merged by default unless `models.mode` is set to `replace`. Merge mode precedence for matching provider IDs: @@ -215,7 +215,9 @@ Merge mode precedence for matching provider IDs: - 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. +- SecretRef-managed provider header values are refreshed from source markers (`secretref-env:ENV_VAR_NAME` for env refs, `secretref-managed` for file/exec refs). - 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`. +Marker persistence is source-authoritative: OpenClaw writes markers from the active source config snapshot (pre-resolution), not from resolved runtime secret values. +This 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 1e48f69d6f8..db5077aebcf 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -2014,9 +2014,11 @@ OpenClaw uses the pi-coding-agent model catalog. Add custom providers via `model - 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. + - SecretRef-managed provider header values are refreshed from source markers (`secretref-env:ENV_VAR_NAME` for env refs, `secretref-managed` for file/exec refs). - 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`. + - Marker persistence is source-authoritative: markers are written from the active source config snapshot (pre-resolution), not from resolved runtime secret values. ### Provider field details diff --git a/docs/reference/secretref-credential-surface.md b/docs/reference/secretref-credential-surface.md index 2a5fc5a66ac..76eb4ec2ae1 100644 --- a/docs/reference/secretref-credential-surface.md +++ b/docs/reference/secretref-credential-surface.md @@ -101,6 +101,7 @@ Notes: - 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. +- Marker persistence is source-authoritative: OpenClaw writes markers from the active source config snapshot (pre-resolution), not from resolved runtime secret values. - 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), only the first provider key that resolves by precedence is active. diff --git a/src/agents/models-config.plan.ts b/src/agents/models-config.plan.ts index 40777c2cd0d..601a0edfda1 100644 --- a/src/agents/models-config.plan.ts +++ b/src/agents/models-config.plan.ts @@ -6,6 +6,7 @@ import { type ExistingProviderConfig, } from "./models-config.merge.js"; import { + enforceSourceManagedProviderSecrets, normalizeProviders, resolveImplicitProviders, type ProviderConfig, @@ -86,6 +87,7 @@ async function resolveProvidersForMode(params: { export async function planOpenClawModelsJson(params: { cfg: OpenClawConfig; + sourceConfigForSecrets?: OpenClawConfig; agentDir: string; env: NodeJS.ProcessEnv; existingRaw: string; @@ -106,6 +108,8 @@ export async function planOpenClawModelsJson(params: { agentDir, env, secretDefaults: cfg.secrets?.defaults, + sourceProviders: params.sourceConfigForSecrets?.models?.providers, + sourceSecretDefaults: params.sourceConfigForSecrets?.secrets?.defaults, secretRefManagedProviders, }) ?? providers; const mergedProviders = await resolveProvidersForMode({ @@ -115,7 +119,14 @@ export async function planOpenClawModelsJson(params: { secretRefManagedProviders, explicitBaseUrlProviders: resolveExplicitBaseUrlProviders(cfg.models), }); - const nextContents = `${JSON.stringify({ providers: mergedProviders }, null, 2)}\n`; + const secretEnforcedProviders = + enforceSourceManagedProviderSecrets({ + providers: mergedProviders, + sourceProviders: params.sourceConfigForSecrets?.models?.providers, + sourceSecretDefaults: params.sourceConfigForSecrets?.secrets?.defaults, + secretRefManagedProviders, + }) ?? mergedProviders; + const nextContents = `${JSON.stringify({ providers: secretEnforcedProviders }, null, 2)}\n`; if (params.existingRaw === nextContents) { return { action: "noop" }; diff --git a/src/agents/models-config.providers.normalize-keys.test.ts b/src/agents/models-config.providers.normalize-keys.test.ts index f8422d797dd..b39705d8ec2 100644 --- a/src/agents/models-config.providers.normalize-keys.test.ts +++ b/src/agents/models-config.providers.normalize-keys.test.ts @@ -4,7 +4,10 @@ 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"; +import { + enforceSourceManagedProviderSecrets, + normalizeProviders, +} from "./models-config.providers.js"; describe("normalizeProviders", () => { it("trims provider keys so image models remain discoverable for custom providers", async () => { @@ -136,4 +139,38 @@ describe("normalizeProviders", () => { await fs.rm(agentDir, { recursive: true, force: true }); } }); + + it("ignores non-object provider entries during source-managed enforcement", () => { + const providers = { + openai: null, + moonshot: { + baseUrl: "https://api.moonshot.ai/v1", + api: "openai-completions", + apiKey: "sk-runtime-moonshot", // pragma: allowlist secret + models: [], + }, + } as unknown as NonNullable["providers"]>; + + const sourceProviders: NonNullable["providers"]> = { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, // pragma: allowlist secret + models: [], + }, + moonshot: { + baseUrl: "https://api.moonshot.ai/v1", + api: "openai-completions", + apiKey: { source: "env", provider: "default", id: "MOONSHOT_API_KEY" }, // pragma: allowlist secret + models: [], + }, + }; + + const enforced = enforceSourceManagedProviderSecrets({ + providers, + sourceProviders, + }); + expect((enforced as Record).openai).toBeNull(); + expect(enforced?.moonshot?.apiKey).toBe("MOONSHOT_API_KEY"); // pragma: allowlist secret + }); }); diff --git a/src/agents/models-config.providers.ts b/src/agents/models-config.providers.ts index c63ed6865a8..411072f2d7a 100644 --- a/src/agents/models-config.providers.ts +++ b/src/agents/models-config.providers.ts @@ -4,6 +4,7 @@ import { DEFAULT_COPILOT_API_BASE_URL, resolveCopilotApiToken, } from "../providers/github-copilot-token.js"; +import { isRecord } from "../utils.js"; import { normalizeOptionalSecretInput } from "../utils/normalize-secret-input.js"; import { ensureAuthProfileStore, listProfilesForProvider } from "./auth-profiles.js"; import { discoverBedrockModels } from "./bedrock-discovery.js"; @@ -70,6 +71,11 @@ export { resolveOllamaApiBase } from "./models-config.providers.discovery.js"; type ModelsConfig = NonNullable; export type ProviderConfig = NonNullable[string]; +type SecretDefaults = { + env?: string; + file?: string; + exec?: string; +}; const ENV_VAR_NAME_RE = /^[A-Z_][A-Z0-9_]*$/; @@ -97,13 +103,7 @@ function resolveAwsSdkApiKeyVarName(env: NodeJS.ProcessEnv = process.env): strin function normalizeHeaderValues(params: { headers: ProviderConfig["headers"] | undefined; - secretDefaults: - | { - env?: string; - file?: string; - exec?: string; - } - | undefined; + secretDefaults: SecretDefaults | undefined; }): { headers: ProviderConfig["headers"] | undefined; mutated: boolean } { const { headers } = params; if (!headers) { @@ -276,15 +276,155 @@ function normalizeAntigravityProvider(provider: ProviderConfig): ProviderConfig return normalizeProviderModels(provider, normalizeAntigravityModelId); } +function normalizeSourceProviderLookup( + providers: ModelsConfig["providers"] | undefined, +): Record { + if (!providers) { + return {}; + } + const out: Record = {}; + for (const [key, provider] of Object.entries(providers)) { + const normalizedKey = key.trim(); + if (!normalizedKey || !isRecord(provider)) { + continue; + } + out[normalizedKey] = provider; + } + return out; +} + +function resolveSourceManagedApiKeyMarker(params: { + sourceProvider: ProviderConfig | undefined; + sourceSecretDefaults: SecretDefaults | undefined; +}): string | undefined { + const sourceApiKeyRef = resolveSecretInputRef({ + value: params.sourceProvider?.apiKey, + defaults: params.sourceSecretDefaults, + }).ref; + if (!sourceApiKeyRef || !sourceApiKeyRef.id.trim()) { + return undefined; + } + return sourceApiKeyRef.source === "env" + ? sourceApiKeyRef.id.trim() + : resolveNonEnvSecretRefApiKeyMarker(sourceApiKeyRef.source); +} + +function resolveSourceManagedHeaderMarkers(params: { + sourceProvider: ProviderConfig | undefined; + sourceSecretDefaults: SecretDefaults | undefined; +}): Record { + const sourceHeaders = isRecord(params.sourceProvider?.headers) + ? (params.sourceProvider.headers as Record) + : undefined; + if (!sourceHeaders) { + return {}; + } + const markers: Record = {}; + for (const [headerName, headerValue] of Object.entries(sourceHeaders)) { + const sourceHeaderRef = resolveSecretInputRef({ + value: headerValue, + defaults: params.sourceSecretDefaults, + }).ref; + if (!sourceHeaderRef || !sourceHeaderRef.id.trim()) { + continue; + } + markers[headerName] = + sourceHeaderRef.source === "env" + ? resolveEnvSecretRefHeaderValueMarker(sourceHeaderRef.id) + : resolveNonEnvSecretRefHeaderValueMarker(sourceHeaderRef.source); + } + return markers; +} + +export function enforceSourceManagedProviderSecrets(params: { + providers: ModelsConfig["providers"]; + sourceProviders: ModelsConfig["providers"] | undefined; + sourceSecretDefaults?: SecretDefaults; + secretRefManagedProviders?: Set; +}): ModelsConfig["providers"] { + const { providers } = params; + if (!providers) { + return providers; + } + const sourceProvidersByKey = normalizeSourceProviderLookup(params.sourceProviders); + if (Object.keys(sourceProvidersByKey).length === 0) { + return providers; + } + + let nextProviders: Record | null = null; + for (const [providerKey, provider] of Object.entries(providers)) { + if (!isRecord(provider)) { + continue; + } + const sourceProvider = sourceProvidersByKey[providerKey.trim()]; + if (!sourceProvider) { + continue; + } + let nextProvider = provider; + let providerMutated = false; + + const sourceApiKeyMarker = resolveSourceManagedApiKeyMarker({ + sourceProvider, + sourceSecretDefaults: params.sourceSecretDefaults, + }); + if (sourceApiKeyMarker) { + params.secretRefManagedProviders?.add(providerKey.trim()); + if (nextProvider.apiKey !== sourceApiKeyMarker) { + providerMutated = true; + nextProvider = { + ...nextProvider, + apiKey: sourceApiKeyMarker, + }; + } + } + + const sourceHeaderMarkers = resolveSourceManagedHeaderMarkers({ + sourceProvider, + sourceSecretDefaults: params.sourceSecretDefaults, + }); + if (Object.keys(sourceHeaderMarkers).length > 0) { + const currentHeaders = isRecord(nextProvider.headers) + ? (nextProvider.headers as Record) + : undefined; + const nextHeaders = { + ...(currentHeaders as Record[string]>), + }; + let headersMutated = !currentHeaders; + for (const [headerName, marker] of Object.entries(sourceHeaderMarkers)) { + if (nextHeaders[headerName] === marker) { + continue; + } + headersMutated = true; + nextHeaders[headerName] = marker; + } + if (headersMutated) { + providerMutated = true; + nextProvider = { + ...nextProvider, + headers: nextHeaders, + }; + } + } + + if (!providerMutated) { + continue; + } + if (!nextProviders) { + nextProviders = { ...providers }; + } + nextProviders[providerKey] = nextProvider; + } + + return nextProviders ?? providers; +} + export function normalizeProviders(params: { providers: ModelsConfig["providers"]; agentDir: string; env?: NodeJS.ProcessEnv; - secretDefaults?: { - env?: string; - file?: string; - exec?: string; - }; + secretDefaults?: SecretDefaults; + sourceProviders?: ModelsConfig["providers"]; + sourceSecretDefaults?: SecretDefaults; secretRefManagedProviders?: Set; }): ModelsConfig["providers"] { const { providers } = params; @@ -434,7 +574,13 @@ export function normalizeProviders(params: { next[normalizedKey] = normalizedProvider; } - return mutated ? next : providers; + const normalizedProviders = mutated ? next : providers; + return enforceSourceManagedProviderSecrets({ + providers: normalizedProviders, + sourceProviders: params.sourceProviders, + sourceSecretDefaults: params.sourceSecretDefaults, + secretRefManagedProviders: params.secretRefManagedProviders, + }); } type ImplicitProviderParams = { diff --git a/src/agents/models-config.runtime-source-snapshot.test.ts b/src/agents/models-config.runtime-source-snapshot.test.ts index 4c5889769cc..cc033fb56a6 100644 --- a/src/agents/models-config.runtime-source-snapshot.test.ts +++ b/src/agents/models-config.runtime-source-snapshot.test.ts @@ -209,4 +209,152 @@ describe("models-config runtime source snapshot", () => { } }); }); + + it("keeps source markers when runtime projection is skipped for incompatible top-level shape", 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: [], + }, + }, + }, + gateway: { + auth: { + mode: "token", + }, + }, + }; + 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: [], + }, + }, + }, + gateway: { + auth: { + mode: "token", + }, + }, + }; + const incompatibleCandidate: 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(incompatibleCandidate); + + const parsed = await readGeneratedModelsJson<{ + providers: Record; + }>(); + expect(parsed.providers.openai?.apiKey).toBe("OPENAI_API_KEY"); // pragma: allowlist secret + } finally { + clearRuntimeConfigSnapshot(); + clearConfigCache(); + } + }); + }); + + it("keeps source header markers when runtime projection is skipped for incompatible top-level shape", 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: [], + }, + }, + }, + gateway: { + auth: { + mode: "token", + }, + }, + }; + 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: [], + }, + }, + }, + gateway: { + auth: { + mode: "token", + }, + }, + }; + const incompatibleCandidate: 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(incompatibleCandidate); + + 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 99714a1a792..3e013799b0b 100644 --- a/src/agents/models-config.ts +++ b/src/agents/models-config.ts @@ -42,15 +42,31 @@ async function writeModelsFileAtomic(targetPath: string, contents: string): Prom await fs.rename(tempPath, targetPath); } -function resolveModelsConfigInput(config?: OpenClawConfig): OpenClawConfig { +function resolveModelsConfigInput(config?: OpenClawConfig): { + config: OpenClawConfig; + sourceConfigForSecrets: OpenClawConfig; +} { const runtimeSource = getRuntimeConfigSourceSnapshot(); if (!config) { - return runtimeSource ?? loadConfig(); + const loaded = loadConfig(); + return { + config: runtimeSource ?? loaded, + sourceConfigForSecrets: runtimeSource ?? loaded, + }; } if (!runtimeSource) { - return config; + return { + config, + sourceConfigForSecrets: config, + }; } - return projectConfigOntoRuntimeSourceSnapshot(config); + const projected = projectConfigOntoRuntimeSourceSnapshot(config); + return { + config: projected, + // If projection is skipped (for example incompatible top-level shape), + // keep managed secret persistence anchored to the active source snapshot. + sourceConfigForSecrets: projected === config ? runtimeSource : projected, + }; } async function withModelsJsonWriteLock(targetPath: string, run: () => Promise): Promise { @@ -76,7 +92,8 @@ export async function ensureOpenClawModelsJson( config?: OpenClawConfig, agentDirOverride?: string, ): Promise<{ agentDir: string; wrote: boolean }> { - const cfg = resolveModelsConfigInput(config); + const resolved = resolveModelsConfigInput(config); + const cfg = resolved.config; const agentDir = agentDirOverride?.trim() ? agentDirOverride.trim() : resolveOpenClawAgentDir(); const targetPath = path.join(agentDir, "models.json"); @@ -87,6 +104,7 @@ export async function ensureOpenClawModelsJson( const existingModelsFile = await readExistingModelsFile(targetPath); const plan = await planOpenClawModelsJson({ cfg, + sourceConfigForSecrets: resolved.sourceConfigForSecrets, agentDir, env, existingRaw: existingModelsFile.raw,