Config: preserve env var references on write (#15600)
* Config: preserve env var references on write * Config: handle env refs in arrays
This commit is contained in:
@@ -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/<guildId>` 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.
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
157
src/config/io.ts
157
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<string, string>): 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<string>,
|
||||
): 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<string>): 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<string, string>,
|
||||
changedPaths: Set<string>,
|
||||
): 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<string, unknown> = {};
|
||||
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<void> {
|
||||
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<string, string> | null = null;
|
||||
let changedPaths: Set<string> | 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<string, string>();
|
||||
collectEnvRefPaths(resolvedIncludes, "", collected);
|
||||
if (collected.size > 0) {
|
||||
envRefMap = collected;
|
||||
changedPaths = new Set<string>();
|
||||
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,
|
||||
|
||||
@@ -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"]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user