From 960dc3f98878ac91bb1764d58d890dbce1a049d9 Mon Sep 17 00:00:00 2001
From: Glucksberg <markuscontasul@gmail.com>
Date: Fri, 30 Jan 2026 15:10:39 +0000
Subject: [PATCH] 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
---
 src/commands/doctor-config-flow.test.ts | 46 ++++++++++++++++++++++++-
 src/commands/doctor-config-flow.ts      |  5 ++-
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts
index 6d775f6f1..64b3c857d 100644
--- a/src/commands/doctor-config-flow.test.ts
+++ b/src/commands/doctor-config-flow.test.ts
@@ -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");
diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts
index 694475267..dece4e118 100644
--- a/src/commands/doctor-config-flow.ts
+++ b/src/commands/doctor-config-flow.ts
@@ -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;