fix(doctor): preserve ${VAR} env var references when writing config

Previously, `doctor --fix` would resolve ${VAR} env var references
to their plaintext values before writing the config back to disk.
This leaked secrets (API keys, tokens, passwords) into openclaw.json.

The fix uses `snapshot.parsed` (pre-env-substitution) instead of
`snapshot.config` (post-substitution) as the base for modifications.
This ensures ${VAR} references are preserved when the config is
written back.

Fixes #4654
This commit is contained in:
Glucksberg 2026-01-30 15:10:39 +00:00
parent dcc4d47f35
commit 960dc3f988
2 changed files with 49 additions and 2 deletions

View File

@ -1,12 +1,56 @@
import fs from "node:fs/promises";
import path from "node:path";
import { describe, expect, it } from "vitest";
import { afterEach, describe, expect, it } from "vitest";
import { withTempHome } from "../../test/helpers/temp-home.js";
import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js";
describe("doctor config flow", () => {
// Issue #4654: doctor --fix should preserve ${VAR} env var references
it("preserves env var references in config values", async () => {
const originalEnv = process.env.TEST_SECRET_TOKEN;
process.env.TEST_SECRET_TOKEN = "super-secret-value-12345";
try {
await withTempHome(async (home) => {
const configDir = path.join(home, ".openclaw");
await fs.mkdir(configDir, { recursive: true });
// Write config with ${VAR} reference
await fs.writeFile(
path.join(configDir, "openclaw.json"),
JSON.stringify(
{
gateway: { auth: { mode: "token", token: "${TEST_SECRET_TOKEN}" } },
agents: { list: [{ id: "main" }] },
},
null,
2,
),
"utf-8",
);
const result = await loadAndMaybeMigrateDoctorConfig({
options: { nonInteractive: true, repair: true },
confirm: async () => true,
});
// The returned config should preserve the ${VAR} reference, NOT the resolved value
const gateway = (result.cfg as Record<string, unknown>).gateway as Record<string, unknown>;
const auth = gateway?.auth as Record<string, unknown>;
expect(auth?.token).toBe("${TEST_SECRET_TOKEN}");
// Ensure it's NOT the resolved value
expect(auth?.token).not.toBe("super-secret-value-12345");
});
} finally {
if (originalEnv === undefined) {
delete process.env.TEST_SECRET_TOKEN;
} else {
process.env.TEST_SECRET_TOKEN = originalEnv;
}
}
});
it("preserves invalid config for doctor repairs", async () => {
await withTempHome(async (home) => {
const configDir = path.join(home, ".openclaw");

View File

@ -184,7 +184,10 @@ export async function loadAndMaybeMigrateDoctorConfig(params: {
}
let snapshot = await readConfigFileSnapshot();
const baseCfg = snapshot.config ?? {};
// Use snapshot.parsed (pre-env-substitution) to preserve ${VAR} references when writing back.
// snapshot.config has env vars resolved, which would leak secrets if written to disk.
// See: https://github.com/moltbot/moltbot/issues/4654
const baseCfg = (snapshot.parsed ?? {}) as OpenClawConfig;
let cfg: OpenClawConfig = baseCfg;
let candidate = structuredClone(baseCfg) as OpenClawConfig;
let pendingChanges = false;