From 76497123566a1a9ef2a94db4deaaf7a9e301dc7c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 21:12:26 +0000 Subject: [PATCH] fix(config): degrade gracefully on missing env vars (#39050, thanks @akz142857) Co-authored-by: ziy --- CHANGELOG.md | 1 + src/config/env-substitution.test.ts | 80 ++++++++++++++++++++++++++++- src/config/env-substitution.ts | 48 ++++++++++++++--- src/config/env-vars.ts | 8 +++ src/config/io.ts | 55 ++++++++++---------- src/gateway/credentials.test.ts | 22 ++++++++ src/gateway/credentials.ts | 20 +++++++- 7 files changed, 195 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c17941983b..208c93a198a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -279,6 +279,7 @@ Docs: https://docs.openclaw.ai - Agents/OpenAI WS max-token zero forwarding: treat `maxTokens: 0` as an explicit value in websocket `response.create` payloads (instead of dropping it as falsy), with regression coverage for zero-token forwarding. (#39148) Thanks @scoootscooob. - Podman/.env gateway bind precedence: evaluate `OPENCLAW_GATEWAY_BIND` after sourcing `.env` in `run-openclaw-podman.sh` so env-file overrides are honored. (#38785) Thanks @majinyu666. - Models/default alias refresh: bump `gpt` to `openai/gpt-5.4` and Gemini defaults to `gemini-3.1` preview aliases (including normalization/default wiring) to track current model IDs. (#38638) Thanks @ademczuk. +- Config/env substitution degraded mode: convert missing `${VAR}` resolution in config reads from hard-fail to warning-backed degraded behavior, while preventing unresolved placeholders from being accepted as gateway credentials. (#39050) Thanks @akz142857. ## 2026.3.2 diff --git a/src/config/env-substitution.test.ts b/src/config/env-substitution.test.ts index 1b3c3f64f89..90db6a5e0e7 100644 --- a/src/config/env-substitution.test.ts +++ b/src/config/env-substitution.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from "vitest"; -import { MissingEnvVarError, resolveConfigEnvVars } from "./env-substitution.js"; +import { + type EnvSubstitutionWarning, + MissingEnvVarError, + containsEnvVarReference, + resolveConfigEnvVars, +} from "./env-substitution.js"; type SubstitutionScenario = { name: string; @@ -265,6 +270,79 @@ describe("resolveConfigEnvVars", () => { }); }); + describe("graceful missing env var handling (onMissing)", () => { + it("collects warnings and preserves placeholder when onMissing is set", () => { + const warnings: EnvSubstitutionWarning[] = []; + const result = resolveConfigEnvVars( + { key: "${MISSING_VAR}", present: "${PRESENT}" }, + { PRESENT: "ok" } as NodeJS.ProcessEnv, + { onMissing: (w) => warnings.push(w) }, + ); + expect(result).toEqual({ key: "${MISSING_VAR}", present: "ok" }); + expect(warnings).toEqual([{ varName: "MISSING_VAR", configPath: "key" }]); + }); + + it("collects multiple warnings across nested paths", () => { + const warnings: EnvSubstitutionWarning[] = []; + const result = resolveConfigEnvVars( + { + providers: { + tts: { apiKey: "${TTS_KEY}" }, + stt: { apiKey: "${STT_KEY}" }, + }, + gateway: { token: "${GW_TOKEN}" }, + }, + { GW_TOKEN: "secret" } as NodeJS.ProcessEnv, + { onMissing: (w) => warnings.push(w) }, + ); + expect(result).toEqual({ + providers: { + tts: { apiKey: "${TTS_KEY}" }, + stt: { apiKey: "${STT_KEY}" }, + }, + gateway: { token: "secret" }, + }); + expect(warnings).toHaveLength(2); + expect(warnings[0]).toEqual({ varName: "TTS_KEY", configPath: "providers.tts.apiKey" }); + expect(warnings[1]).toEqual({ varName: "STT_KEY", configPath: "providers.stt.apiKey" }); + }); + + it("still throws when onMissing is not set", () => { + expect(() => resolveConfigEnvVars({ key: "${MISSING}" }, {} as NodeJS.ProcessEnv)).toThrow( + MissingEnvVarError, + ); + }); + }); + + describe("containsEnvVarReference", () => { + it("detects unresolved env var placeholders", () => { + expect(containsEnvVarReference("${FOO}")).toBe(true); + expect(containsEnvVarReference("prefix-${VAR}-suffix")).toBe(true); + expect(containsEnvVarReference("${A}/${B}")).toBe(true); + expect(containsEnvVarReference("${_UNDERSCORE}")).toBe(true); + expect(containsEnvVarReference("${VAR_WITH_123}")).toBe(true); + }); + + it("returns false for non-matching patterns", () => { + expect(containsEnvVarReference("no-refs-here")).toBe(false); + expect(containsEnvVarReference("$VAR")).toBe(false); + expect(containsEnvVarReference("${lowercase}")).toBe(false); + expect(containsEnvVarReference("${MixedCase}")).toBe(false); + expect(containsEnvVarReference("${123INVALID}")).toBe(false); + expect(containsEnvVarReference("")).toBe(false); + }); + + it("returns false for escaped placeholders", () => { + expect(containsEnvVarReference("$${ESCAPED}")).toBe(false); + expect(containsEnvVarReference("prefix-$${ESCAPED}-suffix")).toBe(false); + }); + + it("detects references mixed with escaped placeholders", () => { + expect(containsEnvVarReference("$${ESCAPED} ${REAL}")).toBe(true); + expect(containsEnvVarReference("${REAL} $${ESCAPED}")).toBe(true); + }); + }); + describe("real-world config patterns", () => { it("substitutes provider, gateway, and base URL config values", () => { const scenarios: SubstitutionScenario[] = [ diff --git a/src/config/env-substitution.ts b/src/config/env-substitution.ts index 0c1b7e02603..cd44e4a5217 100644 --- a/src/config/env-substitution.ts +++ b/src/config/env-substitution.ts @@ -75,7 +75,22 @@ function parseEnvTokenAt(value: string, index: number): EnvToken | null { return null; } -function substituteString(value: string, env: NodeJS.ProcessEnv, configPath: string): string { +export type EnvSubstitutionWarning = { + varName: string; + configPath: string; +}; + +export type SubstituteOptions = { + /** When set, missing vars call this instead of throwing and the original placeholder is preserved. */ + onMissing?: (warning: EnvSubstitutionWarning) => void; +}; + +function substituteString( + value: string, + env: NodeJS.ProcessEnv, + configPath: string, + opts?: SubstituteOptions, +): string { if (!value.includes("$")) { return value; } @@ -98,6 +113,13 @@ function substituteString(value: string, env: NodeJS.ProcessEnv, configPath: str if (token?.kind === "substitution") { const envValue = env[token.name]; if (envValue === undefined || envValue === "") { + if (opts?.onMissing) { + opts.onMissing({ varName: token.name, configPath }); + // Preserve the original placeholder so the value is visibly unresolved. + chunks.push(`\${${token.name}}`); + i = token.end; + continue; + } throw new MissingEnvVarError(token.name, configPath); } chunks.push(envValue); @@ -136,20 +158,25 @@ export function containsEnvVarReference(value: string): boolean { return false; } -function substituteAny(value: unknown, env: NodeJS.ProcessEnv, path: string): unknown { +function substituteAny( + value: unknown, + env: NodeJS.ProcessEnv, + path: string, + opts?: SubstituteOptions, +): unknown { if (typeof value === "string") { - return substituteString(value, env, path); + return substituteString(value, env, path, opts); } if (Array.isArray(value)) { - return value.map((item, index) => substituteAny(item, env, `${path}[${index}]`)); + return value.map((item, index) => substituteAny(item, env, `${path}[${index}]`, opts)); } if (isPlainObject(value)) { const result: Record = {}; for (const [key, val] of Object.entries(value)) { const childPath = path ? `${path}.${key}` : key; - result[key] = substituteAny(val, env, childPath); + result[key] = substituteAny(val, env, childPath, opts); } return result; } @@ -163,9 +190,14 @@ function substituteAny(value: unknown, env: NodeJS.ProcessEnv, path: string): un * * @param obj - The parsed config object (after JSON5 parse and $include resolution) * @param env - Environment variables to use for substitution (defaults to process.env) + * @param opts - Options: `onMissing` callback to collect warnings instead of throwing. * @returns The config object with env vars substituted - * @throws {MissingEnvVarError} If a referenced env var is not set or empty + * @throws {MissingEnvVarError} If a referenced env var is not set or empty (unless `onMissing` is set) */ -export function resolveConfigEnvVars(obj: unknown, env: NodeJS.ProcessEnv = process.env): unknown { - return substituteAny(obj, env, ""); +export function resolveConfigEnvVars( + obj: unknown, + env: NodeJS.ProcessEnv = process.env, + opts?: SubstituteOptions, +): unknown { + return substituteAny(obj, env, "", opts); } diff --git a/src/config/env-vars.ts b/src/config/env-vars.ts index f9480b9f540..edfa44db302 100644 --- a/src/config/env-vars.ts +++ b/src/config/env-vars.ts @@ -3,6 +3,7 @@ import { isDangerousHostEnvVarName, normalizeEnvVarKey, } from "../infra/host-env-security.js"; +import { containsEnvVarReference } from "./env-substitution.js"; import type { OpenClawConfig } from "./types.js"; function isBlockedConfigEnvVar(key: string): boolean { @@ -75,6 +76,13 @@ export function applyConfigEnvVars( if (env[key]?.trim()) { continue; } + // Skip values containing unresolved ${VAR} references — applyConfigEnvVars runs + // before env substitution, so these would pollute process.env with literal placeholders + // (e.g. process.env.OPENCLAW_GATEWAY_TOKEN = "${VAULT_TOKEN}") which downstream auth + // resolution would accept as valid credentials. + if (containsEnvVarReference(value)) { + continue; + } env[key] = value; } } diff --git a/src/config/io.ts b/src/config/io.ts index 586dd9b3227..a7d486a77b8 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -30,6 +30,7 @@ import { } from "./defaults.js"; import { restoreEnvVarRefs } from "./env-preserve.js"; import { + type EnvSubstitutionWarning, MissingEnvVarError, containsEnvVarReference, resolveConfigEnvVars, @@ -630,6 +631,7 @@ export function parseConfigJson5( type ConfigReadResolution = { resolvedConfigRaw: unknown; envSnapshotForRestore: Record; + envWarnings: EnvSubstitutionWarning[]; }; function resolveConfigIncludesForRead( @@ -659,10 +661,16 @@ function resolveConfigForRead( applyConfigEnvVars(resolvedIncludes as OpenClawConfig, env); } + // Collect missing env var references as warnings instead of throwing, + // so non-critical config sections with unset vars don't crash the gateway. + const envWarnings: EnvSubstitutionWarning[] = []; return { - resolvedConfigRaw: resolveConfigEnvVars(resolvedIncludes, env), + resolvedConfigRaw: resolveConfigEnvVars(resolvedIncludes, env, { + onMissing: (w) => envWarnings.push(w), + }), // Capture env snapshot after substitution for write-time ${VAR} restoration. envSnapshotForRestore: { ...env } as Record, + envWarnings, }; } @@ -697,10 +705,16 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { } const raw = deps.fs.readFileSync(configPath, "utf-8"); const parsed = deps.json5.parse(raw); - const { resolvedConfigRaw: resolvedConfig } = resolveConfigForRead( + const readResolution = resolveConfigForRead( resolveConfigIncludesForRead(parsed, configPath, deps), deps.env, ); + const resolvedConfig = readResolution.resolvedConfigRaw; + for (const w of readResolution.envWarnings) { + deps.logger.warn( + `Config (${configPath}): missing env var "${w.varName}" at ${w.configPath} — feature using this value will be unavailable`, + ); + } warnOnConfigMiskeys(resolvedConfig, deps.logger); if (typeof resolvedConfig !== "object" || resolvedConfig === null) { return {}; @@ -906,30 +920,15 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { }; } - let readResolution: ConfigReadResolution; - try { - readResolution = resolveConfigForRead(resolved, deps.env); - } catch (err) { - const message = - err instanceof MissingEnvVarError - ? err.message - : `Env var substitution failed: ${String(err)}`; - return { - snapshot: { - path: configPath, - exists: true, - raw, - parsed: parsedRes.parsed, - resolved: coerceConfig(resolved), - valid: false, - config: coerceConfig(resolved), - hash, - issues: [{ path: "", message }], - warnings: [], - legacyIssues: [], - }, - }; - } + const readResolution = resolveConfigForRead(resolved, deps.env); + + // Convert missing env var references to config warnings instead of fatal errors. + // This allows the gateway to start in degraded mode when non-critical config + // sections reference unset env vars (e.g. optional provider API keys). + const envVarWarnings = readResolution.envWarnings.map((w) => ({ + path: w.configPath, + message: `Missing env var "${w.varName}" — feature using this value will be unavailable`, + })); const resolvedConfigRaw = readResolution.resolvedConfigRaw; // Detect legacy keys on resolved config, but only mark source-literal legacy @@ -949,7 +948,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { config: coerceConfig(resolvedConfigRaw), hash, issues: validated.issues, - warnings: validated.warnings, + warnings: [...validated.warnings, ...envVarWarnings], legacyIssues, }, }; @@ -981,7 +980,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { config: snapshotConfig, hash, issues: [], - warnings: validated.warnings, + warnings: [...validated.warnings, ...envVarWarnings], legacyIssues, }, envSnapshotForRestore: readResolution.envSnapshotForRestore, diff --git a/src/gateway/credentials.test.ts b/src/gateway/credentials.test.ts index d9b3fa26783..bb251682715 100644 --- a/src/gateway/credentials.test.ts +++ b/src/gateway/credentials.test.ts @@ -481,4 +481,26 @@ describe("resolveGatewayCredentialsFromValues", () => { password: "env-password", // pragma: allowlist secret }); }); + + it("rejects unresolved env var placeholders in config credentials", () => { + const resolved = resolveGatewayCredentialsFromValues({ + configToken: "${OPENCLAW_GATEWAY_TOKEN}", + configPassword: "${OPENCLAW_GATEWAY_PASSWORD}", + env: {} as NodeJS.ProcessEnv, + tokenPrecedence: "config-first", + passwordPrecedence: "config-first", // pragma: allowlist secret + }); + expect(resolved).toEqual({ token: undefined, password: undefined }); + }); + + it("accepts config credentials that do not contain env var references", () => { + const resolved = resolveGatewayCredentialsFromValues({ + configToken: "real-token-value", + configPassword: "real-password", // pragma: allowlist secret + env: {} as NodeJS.ProcessEnv, + tokenPrecedence: "config-first", + passwordPrecedence: "config-first", // pragma: allowlist secret + }); + expect(resolved).toEqual({ token: "real-token-value", password: "real-password" }); + }); }); diff --git a/src/gateway/credentials.ts b/src/gateway/credentials.ts index 42cc36676b9..4a88eb80015 100644 --- a/src/gateway/credentials.ts +++ b/src/gateway/credentials.ts @@ -1,4 +1,5 @@ import type { OpenClawConfig } from "../config/config.js"; +import { containsEnvVarReference } from "../config/env-substitution.js"; import { resolveSecretInputRef } from "../config/types.secrets.js"; export type ExplicitGatewayAuth = { @@ -56,6 +57,21 @@ export function trimToUndefined(value: unknown): string | undefined { return trimmed.length > 0 ? trimmed : undefined; } +/** + * Like trimToUndefined but also rejects unresolved env var placeholders (e.g. `${VAR}`). + * This prevents literal placeholder strings like `${OPENCLAW_GATEWAY_TOKEN}` from being + * accepted as valid credentials when the referenced env var is missing. + * Note: legitimate credential values containing literal `${UPPER_CASE}` patterns will + * also be rejected, but this is an extremely unlikely edge case. + */ +export function trimCredentialToUndefined(value: unknown): string | undefined { + const trimmed = trimToUndefined(value); + if (trimmed && containsEnvVarReference(trimmed)) { + return undefined; + } + return trimmed; +} + function firstDefined(values: Array): string | undefined { for (const value of values) { if (value) { @@ -123,8 +139,8 @@ export function resolveGatewayCredentialsFromValues(params: { const includeLegacyEnv = params.includeLegacyEnv ?? true; const envToken = readGatewayTokenEnv(env, includeLegacyEnv); const envPassword = readGatewayPasswordEnv(env, includeLegacyEnv); - const configToken = trimToUndefined(params.configToken); - const configPassword = trimToUndefined(params.configPassword); + const configToken = trimCredentialToUndefined(params.configToken); + const configPassword = trimCredentialToUndefined(params.configPassword); const tokenPrecedence = params.tokenPrecedence ?? "env-first"; const passwordPrecedence = params.passwordPrecedence ?? "env-first";