mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-07 22:44:16 +00:00
fix(config): degrade gracefully on missing env vars (#39050, thanks @akz142857)
Co-authored-by: ziy <ziyang.liu@wahool.com>
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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[] = [
|
||||
|
||||
@@ -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<string, unknown> = {};
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<string, string | undefined>;
|
||||
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<string, string | undefined>,
|
||||
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,
|
||||
|
||||
@@ -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" });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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>): 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";
|
||||
|
||||
|
||||
Reference in New Issue
Block a user