diff --git a/CHANGELOG.md b/CHANGELOG.md index cf5963133..5809d0de4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Docs: https://docs.openclaw.ai - Sessions/Agents: pass `agentId` through status and usage transcript-resolution paths (auto-reply, gateway usage APIs, and session cost/log loaders) so non-default agents can resolve absolute session files without path-validation failures. (#15103) Thanks @jalehman. - Signal/Install: auto-install `signal-cli` via Homebrew on non-x64 Linux architectures, avoiding x86_64 native binary `Exec format error` failures on arm64/arm hosts. (#15443) Thanks @jogvan-k. - Discord: avoid misrouting numeric guild allowlist entries to `/channels/` by prefixing guild-only inputs with `guild:` during resolution. (#12326) Thanks @headswim. +- Config: preserve `${VAR}` env references when writing config files so `openclaw config set/apply/patch` does not persist secrets to disk. Thanks @thewilloftheshadow. - Web tools/web_fetch: prefer `text/markdown` responses for Cloudflare Markdown for Agents, add `cf-markdown` extraction for markdown bodies, and redact fetched URLs in `x-markdown-tokens` debug logs to avoid leaking raw paths/query params. (#15376) Thanks @Yaxuan42. - Config: keep legacy audio transcription migration strict by rejecting non-string/unsafe command tokens while still migrating valid custom script executables. (#5042) Thanks @shayan919293. diff --git a/src/config/env-substitution.ts b/src/config/env-substitution.ts index 97668a744..306be869c 100644 --- a/src/config/env-substitution.ts +++ b/src/config/env-substitution.ts @@ -92,6 +92,49 @@ function substituteString(value: string, env: NodeJS.ProcessEnv, configPath: str return chunks.join(""); } +export function containsEnvVarReference(value: string): boolean { + if (!value.includes("$")) { + return false; + } + + for (let i = 0; i < value.length; i += 1) { + const char = value[i]; + if (char !== "$") { + continue; + } + + const next = value[i + 1]; + const afterNext = value[i + 2]; + + // Escaped: $${VAR} -> ${VAR} + if (next === "$" && afterNext === "{") { + const start = i + 3; + const end = value.indexOf("}", start); + if (end !== -1) { + const name = value.slice(start, end); + if (ENV_VAR_NAME_PATTERN.test(name)) { + i = end; + continue; + } + } + } + + // Substitution: ${VAR} -> value + if (next === "{") { + const start = i + 2; + const end = value.indexOf("}", start); + if (end !== -1) { + const name = value.slice(start, end); + if (ENV_VAR_NAME_PATTERN.test(name)) { + return true; + } + } + } + } + + return false; +} + function substituteAny(value: unknown, env: NodeJS.ProcessEnv, path: string): unknown { if (typeof value === "string") { return substituteString(value, env, path); diff --git a/src/config/io.ts b/src/config/io.ts index 19b1f02e7..184f73942 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -25,7 +25,11 @@ import { applySessionDefaults, applyTalkApiKey, } from "./defaults.js"; -import { MissingEnvVarError, resolveConfigEnvVars } from "./env-substitution.js"; +import { + MissingEnvVarError, + containsEnvVarReference, + resolveConfigEnvVars, +} from "./env-substitution.js"; import { collectConfigEnvVars } from "./env-vars.js"; import { ConfigIncludeError, resolveConfigIncludes } from "./includes.js"; import { findLegacyConfigIssues } from "./legacy.js"; @@ -140,6 +144,132 @@ function createMergePatch(base: unknown, target: unknown): unknown { return patch; } +function collectEnvRefPaths(value: unknown, path: string, output: Map): void { + if (typeof value === "string") { + if (containsEnvVarReference(value)) { + output.set(path, value); + } + return; + } + if (Array.isArray(value)) { + value.forEach((item, index) => { + collectEnvRefPaths(item, `${path}[${index}]`, output); + }); + return; + } + if (isPlainObject(value)) { + for (const [key, child] of Object.entries(value)) { + const childPath = path ? `${path}.${key}` : key; + collectEnvRefPaths(child, childPath, output); + } + } +} + +function collectChangedPaths( + base: unknown, + target: unknown, + path: string, + output: Set, +): void { + if (Array.isArray(base) && Array.isArray(target)) { + const max = Math.max(base.length, target.length); + for (let index = 0; index < max; index += 1) { + const childPath = path ? `${path}[${index}]` : `[${index}]`; + if (index >= base.length || index >= target.length) { + output.add(childPath); + continue; + } + collectChangedPaths(base[index], target[index], childPath, output); + } + return; + } + if (isPlainObject(base) && isPlainObject(target)) { + const keys = new Set([...Object.keys(base), ...Object.keys(target)]); + for (const key of keys) { + const childPath = path ? `${path}.${key}` : key; + const hasBase = key in base; + const hasTarget = key in target; + if (!hasTarget || !hasBase) { + output.add(childPath); + continue; + } + collectChangedPaths(base[key], target[key], childPath, output); + } + return; + } + if (!isDeepStrictEqual(base, target)) { + output.add(path); + } +} + +function parentPath(value: string): string { + if (!value) { + return ""; + } + if (value.endsWith("]")) { + const index = value.lastIndexOf("["); + return index > 0 ? value.slice(0, index) : ""; + } + const index = value.lastIndexOf("."); + return index >= 0 ? value.slice(0, index) : ""; +} + +function isPathChanged(path: string, changedPaths: Set): boolean { + if (changedPaths.has(path)) { + return true; + } + let current = parentPath(path); + while (current) { + if (changedPaths.has(current)) { + return true; + } + current = parentPath(current); + } + return changedPaths.has(""); +} + +function restoreEnvRefsFromMap( + value: unknown, + path: string, + envRefMap: Map, + changedPaths: Set, +): unknown { + if (typeof value === "string") { + if (!isPathChanged(path, changedPaths)) { + const original = envRefMap.get(path); + if (original !== undefined) { + return original; + } + } + return value; + } + if (Array.isArray(value)) { + let changed = false; + const next = value.map((item, index) => { + const updated = restoreEnvRefsFromMap(item, `${path}[${index}]`, envRefMap, changedPaths); + if (updated !== item) { + changed = true; + } + return updated; + }); + return changed ? next : value; + } + if (isPlainObject(value)) { + let changed = false; + const next: Record = {}; + for (const [key, child] of Object.entries(value)) { + const childPath = path ? `${path}.${key}` : key; + const updated = restoreEnvRefsFromMap(child, childPath, envRefMap, changedPaths); + if (updated !== child) { + changed = true; + } + next[key] = updated; + } + return changed ? next : value; + } + return value; +} + async function rotateConfigBackups(configPath: string, ioFs: typeof fs.promises): Promise { if (CONFIG_BACKUP_COUNT <= 1) { return; @@ -552,9 +682,26 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { clearConfigCache(); let persistCandidate: unknown = cfg; const snapshot = await readConfigFileSnapshot(); + let envRefMap: Map | null = null; + let changedPaths: Set | null = null; if (snapshot.valid && snapshot.exists) { const patch = createMergePatch(snapshot.config, cfg); persistCandidate = applyMergePatch(snapshot.resolved, patch); + try { + const resolvedIncludes = resolveConfigIncludes(snapshot.parsed, configPath, { + readFile: (candidate) => deps.fs.readFileSync(candidate, "utf-8"), + parseJson: (raw) => deps.json5.parse(raw), + }); + const collected = new Map(); + collectEnvRefPaths(resolvedIncludes, "", collected); + if (collected.size > 0) { + envRefMap = collected; + changedPaths = new Set(); + collectChangedPaths(snapshot.config, cfg, "", changedPaths); + } + } catch { + envRefMap = null; + } } const validated = validateConfigObjectRawWithPlugins(persistCandidate); @@ -571,11 +718,13 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { } const dir = path.dirname(configPath); await deps.fs.promises.mkdir(dir, { recursive: true, mode: 0o700 }); + const outputConfig = + envRefMap && changedPaths + ? (restoreEnvRefsFromMap(validated.config, "", envRefMap, changedPaths) as OpenClawConfig) + : validated.config; // Do NOT apply runtime defaults when writing — user config should only contain // explicitly set values. Runtime defaults are applied when loading (issue #6070). - const json = JSON.stringify(stampConfigVersion(validated.config), null, 2) - .trimEnd() - .concat("\n"); + const json = JSON.stringify(stampConfigVersion(outputConfig), null, 2).trimEnd().concat("\n"); const tmp = path.join( dir, diff --git a/src/config/io.write-config.test.ts b/src/config/io.write-config.test.ts index cff5cd245..ca121c84a 100644 --- a/src/config/io.write-config.test.ts +++ b/src/config/io.write-config.test.ts @@ -44,4 +44,110 @@ describe("config io write", () => { expect(persisted).not.toHaveProperty("sessions.persistence"); }); }); + + it("preserves env var references when writing", async () => { + await withTempHome(async (home) => { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile( + configPath, + JSON.stringify( + { + agents: { + defaults: { + cliBackends: { + codex: { + env: { + OPENAI_API_KEY: "${OPENAI_API_KEY}", + }, + }, + }, + }, + }, + gateway: { port: 18789 }, + }, + null, + 2, + ), + "utf-8", + ); + + const io = createConfigIO({ + env: { OPENAI_API_KEY: "sk-secret" } as NodeJS.ProcessEnv, + homedir: () => home, + }); + + const snapshot = await io.readConfigFileSnapshot(); + expect(snapshot.valid).toBe(true); + + const next = structuredClone(snapshot.config); + next.gateway = { + ...next.gateway, + auth: { mode: "token" }, + }; + + await io.writeConfigFile(next); + + const persisted = JSON.parse(await fs.readFile(configPath, "utf-8")) as { + agents: { defaults: { cliBackends: { codex: { env: { OPENAI_API_KEY: string } } } } }; + gateway: { port: number; auth: { mode: string } }; + }; + expect(persisted.agents.defaults.cliBackends.codex.env.OPENAI_API_KEY).toBe( + "${OPENAI_API_KEY}", + ); + expect(persisted.gateway).toEqual({ + port: 18789, + auth: { mode: "token" }, + }); + }); + }); + + it("keeps env refs in arrays when appending entries", async () => { + await withTempHome(async (home) => { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile( + configPath, + JSON.stringify( + { + channels: { + discord: { + allowFrom: ["${DISCORD_USER_ID}", "123"], + }, + }, + }, + null, + 2, + ), + "utf-8", + ); + + const io = createConfigIO({ + env: { DISCORD_USER_ID: "999" } as NodeJS.ProcessEnv, + homedir: () => home, + }); + + const snapshot = await io.readConfigFileSnapshot(); + expect(snapshot.valid).toBe(true); + + const next = structuredClone(snapshot.config); + const allowFrom = Array.isArray(next.channels?.discord?.allowFrom) + ? next.channels?.discord?.allowFrom + : []; + next.channels = { + ...next.channels, + discord: { + ...next.channels?.discord, + allowFrom: [...allowFrom, "456"], + }, + }; + + await io.writeConfigFile(next); + + const persisted = JSON.parse(await fs.readFile(configPath, "utf-8")) as { + channels: { discord?: { allowFrom?: string[] } }; + }; + expect(persisted.channels.discord?.allowFrom).toEqual(["${DISCORD_USER_ID}", "123", "456"]); + }); + }); });