test: isolate config io compatibility seams

2026-04-12 09:41:11 +00:00 · 2026-04-07 07:15:06 +01:00
parent b8451e26a3
commit 68e421c487
4 changed files with 218 additions and 130 deletions
--- a/src/config/io.compat.test.ts
+++ b/src/config/io.compat.test.ts
@@ -4,6 +4,7 @@ import path from "node:path";
 import { describe, expect, it, vi } from "vitest";
 import { VERSION } from "../version.js";
 import { createConfigIO } from "./io.js";
+import { normalizeExecSafeBinProfilesInConfig } from "./normalize-exec-safe-bin.js";
 import { parseOpenClawVersion } from "./version.js";

 async function withTempHome(run: (home: string) => Promise<void>): Promise<void> {
@@ -71,7 +72,6 @@ describe("config io paths", () => {
      const configPath = await writeConfig(home, ".openclaw", 19001);
      const io = createIoForHome(home);
      expect(io.configPath).toBe(configPath);
-      expect(io.loadConfig().gateway?.port).toBe(19001);
    });
  });

@@ -97,68 +97,52 @@ describe("config io paths", () => {
      const customPath = await writeConfig(home, ".openclaw", 20002, "custom.json");
      const io = createIoForHome(home, { OPENCLAW_CONFIG_PATH: customPath } as NodeJS.ProcessEnv);
      expect(io.configPath).toBe(customPath);
-      expect(io.loadConfig().gateway?.port).toBe(20002);
    });
  });

  it("normalizes safe-bin config entries at config load time", async () => {
-    await withTempHome(async (home) => {
-      const configDir = path.join(home, ".openclaw");
-      await fs.mkdir(configDir, { recursive: true });
-      const configPath = path.join(configDir, "openclaw.json");
-      await fs.writeFile(
-        configPath,
-        JSON.stringify(
+    const cfg = {
+      tools: {
+        exec: {
+          safeBinTrustedDirs: [" /custom/bin ", "", "/custom/bin", "/agent/bin"],
+          safeBinProfiles: {
+            " MyFilter ": {
+              allowedValueFlags: ["--limit", " --limit ", ""],
+            },
+          },
+        },
+      },
+      agents: {
+        list: [
          {
+            id: "ops",
            tools: {
              exec: {
-                safeBinTrustedDirs: [" /custom/bin ", "", "/custom/bin", "/agent/bin"],
+                safeBinTrustedDirs: [" /ops/bin ", "/ops/bin"],
                safeBinProfiles: {
-                  " MyFilter ": {
-                    allowedValueFlags: ["--limit", " --limit ", ""],
+                  " Custom ": {
+                    deniedFlags: ["-f", " -f ", ""],
                  },
                },
              },
            },
-            agents: {
-              list: [
-                {
-                  id: "ops",
-                  tools: {
-                    exec: {
-                      safeBinTrustedDirs: [" /ops/bin ", "/ops/bin"],
-                      safeBinProfiles: {
-                        " Custom ": {
-                          deniedFlags: ["-f", " -f ", ""],
-                        },
-                      },
-                    },
-                  },
-                },
-              ],
-            },
          },
-          null,
-          2,
-        ),
-        "utf-8",
-      );
-      const io = createIoForHome(home);
-      expect(io.configPath).toBe(configPath);
-      const cfg = io.loadConfig();
-      expect(cfg.tools?.exec?.safeBinProfiles).toEqual({
-        myfilter: {
-          allowedValueFlags: ["--limit"],
-        },
-      });
-      expect(cfg.tools?.exec?.safeBinTrustedDirs).toEqual(["/custom/bin", "/agent/bin"]);
-      expect(cfg.agents?.list?.[0]?.tools?.exec?.safeBinProfiles).toEqual({
-        custom: {
-          deniedFlags: ["-f"],
-        },
-      });
-      expect(cfg.agents?.list?.[0]?.tools?.exec?.safeBinTrustedDirs).toEqual(["/ops/bin"]);
+        ],
+      },
+    };
+    normalizeExecSafeBinProfilesInConfig(cfg);
+    expect(cfg.tools?.exec?.safeBinProfiles).toEqual({
+      myfilter: {
+        allowedValueFlags: ["--limit"],
+      },
    });
+    expect(cfg.tools?.exec?.safeBinTrustedDirs).toEqual(["/custom/bin", "/agent/bin"]);
+    expect(cfg.agents?.list?.[0]?.tools?.exec?.safeBinProfiles).toEqual({
+      custom: {
+        deniedFlags: ["-f"],
+      },
+    });
+    expect(cfg.agents?.list?.[0]?.tools?.exec?.safeBinTrustedDirs).toEqual(["/ops/bin"]);
  });

  it("logs invalid config path details and throws on invalid config", async () => {
--- a/src/config/io.owner-display-secret.test.ts
+++ b/src/config/io.owner-display-secret.test.ts
@@ -1,61 +1,132 @@
-import fs from "node:fs/promises";
-import path from "node:path";
-import { setTimeout as sleep } from "node:timers/promises";
-import { beforeEach, describe, expect, it, vi } from "vitest";
-import { withTempHome } from "./home-env.test-harness.js";
-import { createConfigIO } from "./io.js";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import type { OpenClawConfig } from "./config.js";
+import {
+  type OwnerDisplaySecretPersistState,
+  persistGeneratedOwnerDisplaySecret,
+} from "./io.owner-display-secret.js";

-async function waitForPersistedSecret(configPath: string, expectedSecret: string): Promise<void> {
-  const deadline = Date.now() + 3_000;
-  while (Date.now() < deadline) {
-    const raw = await fs.readFile(configPath, "utf-8");
-    let parsed: {
-      commands?: { ownerDisplaySecret?: string };
-    };
-    try {
-      parsed = JSON.parse(raw) as {
-        commands?: { ownerDisplaySecret?: string };
-      };
-    } catch {
-      await sleep(5);
-      continue;
-    }
-    if (parsed.commands?.ownerDisplaySecret === expectedSecret) {
-      return;
-    }
-    await sleep(5);
-  }
-  throw new Error("timed out waiting for ownerDisplaySecret persistence");
+function createState(): OwnerDisplaySecretPersistState {
+  return {
+    pendingByPath: new Map<string, string>(),
+    persistInFlight: new Set<string>(),
+    persistWarned: new Set<string>(),
+  };
 }

-describe("config io owner display secret autofill", () => {
-  beforeEach(() => {
-    vi.useRealTimers();
+async function flushAsyncWork(): Promise<void> {
+  await Promise.resolve();
+  await Promise.resolve();
+}
+
+describe("persistGeneratedOwnerDisplaySecret", () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
  });

-  it("auto-generates and persists commands.ownerDisplaySecret in hash mode", async () => {
-    await withTempHome("openclaw-owner-display-secret-", async (home) => {
-      const configPath = path.join(home, ".openclaw", "openclaw.json");
-      await fs.mkdir(path.dirname(configPath), { recursive: true });
-      await fs.writeFile(
-        configPath,
-        JSON.stringify({ commands: { ownerDisplay: "hash" } }, null, 2),
-        "utf-8",
-      );
+  it("persists generated owner display secrets once and clears state on success", async () => {
+    const state = createState();
+    const configPath = "/tmp/openclaw.json";
+    const config = {
+      commands: {
+        ownerDisplay: "hash",
+        ownerDisplaySecret: "generated-owner-secret",
+      },
+    } as OpenClawConfig;
+    const persistConfig = vi.fn(async () => undefined);

-      const io = createConfigIO({
-        env: {} as NodeJS.ProcessEnv,
-        homedir: () => home,
-        logger: { warn: () => {}, error: () => {} },
-      });
-      const cfg = io.loadConfig();
-      const secret = cfg.commands?.ownerDisplaySecret;
-
-      expect(secret).toMatch(/^[a-f0-9]{64}$/);
-      await waitForPersistedSecret(configPath, secret ?? "");
-
-      const cfgReloaded = io.loadConfig();
-      expect(cfgReloaded.commands?.ownerDisplaySecret).toBe(secret);
+    const result = persistGeneratedOwnerDisplaySecret({
+      config,
+      configPath,
+      generatedSecret: "generated-owner-secret",
+      logger: { warn: vi.fn() },
+      state,
+      persistConfig,
    });
+
+    expect(result).toBe(config);
+    expect(state.pendingByPath.get(configPath)).toBe("generated-owner-secret");
+    expect(state.persistInFlight.has(configPath)).toBe(true);
+    expect(persistConfig).toHaveBeenCalledTimes(1);
+    expect(persistConfig).toHaveBeenCalledWith(config, {
+      expectedConfigPath: configPath,
+    });
+
+    await flushAsyncWork();
+
+    expect(state.pendingByPath.has(configPath)).toBe(false);
+    expect(state.persistInFlight.has(configPath)).toBe(false);
+    expect(state.persistWarned.has(configPath)).toBe(false);
+  });
+
+  it("warns once and keeps the generated secret pending when persistence fails", async () => {
+    const state = createState();
+    const configPath = "/tmp/openclaw.json";
+    const config = {
+      commands: {
+        ownerDisplay: "hash",
+        ownerDisplaySecret: "generated-owner-secret",
+      },
+    } as OpenClawConfig;
+    const warn = vi.fn();
+    const persistConfig = vi.fn(async () => {
+      throw new Error("disk full");
+    });
+
+    persistGeneratedOwnerDisplaySecret({
+      config,
+      configPath,
+      generatedSecret: "generated-owner-secret",
+      logger: { warn },
+      state,
+      persistConfig,
+    });
+
+    await flushAsyncWork();
+
+    expect(warn).toHaveBeenCalledTimes(1);
+    expect(warn).toHaveBeenCalledWith(
+      expect.stringContaining("Failed to persist auto-generated commands.ownerDisplaySecret"),
+    );
+    expect(state.pendingByPath.get(configPath)).toBe("generated-owner-secret");
+    expect(state.persistInFlight.has(configPath)).toBe(false);
+    expect(state.persistWarned.has(configPath)).toBe(true);
+
+    persistGeneratedOwnerDisplaySecret({
+      config,
+      configPath,
+      generatedSecret: "generated-owner-secret",
+      logger: { warn },
+      state,
+      persistConfig,
+    });
+
+    await flushAsyncWork();
+
+    expect(warn).toHaveBeenCalledTimes(1);
+  });
+
+  it("clears pending state when no generated secret is present", () => {
+    const state = createState();
+    const configPath = "/tmp/openclaw.json";
+    state.pendingByPath.set(configPath, "stale-secret");
+    state.persistWarned.add(configPath);
+    const config = {
+      commands: {
+        ownerDisplay: "hash",
+        ownerDisplaySecret: "existing-secret",
+      },
+    } as OpenClawConfig;
+
+    const result = persistGeneratedOwnerDisplaySecret({
+      config,
+      configPath,
+      logger: { warn: vi.fn() },
+      state,
+      persistConfig: vi.fn(async () => undefined),
+    });
+
+    expect(result).toBe(config);
+    expect(state.pendingByPath.has(configPath)).toBe(false);
+    expect(state.persistWarned.has(configPath)).toBe(false);
  });
 });
--- a/src/config/io.owner-display-secret.ts
+++ b/src/config/io.owner-display-secret.ts
@@ -0,0 +1,49 @@
+import type { OpenClawConfig } from "./config.js";
+
+export type OwnerDisplaySecretPersistState = {
+  pendingByPath: Map<string, string>;
+  persistInFlight: Set<string>;
+  persistWarned: Set<string>;
+};
+
+export function persistGeneratedOwnerDisplaySecret(params: {
+  config: OpenClawConfig;
+  configPath: string;
+  generatedSecret?: string;
+  logger: Pick<typeof console, "warn">;
+  state: OwnerDisplaySecretPersistState;
+  persistConfig: (
+    config: OpenClawConfig,
+    options: { expectedConfigPath: string },
+  ) => Promise<unknown>;
+}): OpenClawConfig {
+  const { config, configPath, generatedSecret, logger, state, persistConfig } = params;
+  if (!generatedSecret) {
+    state.pendingByPath.delete(configPath);
+    state.persistWarned.delete(configPath);
+    return config;
+  }
+
+  state.pendingByPath.set(configPath, generatedSecret);
+  if (!state.persistInFlight.has(configPath)) {
+    state.persistInFlight.add(configPath);
+    void persistConfig(config, { expectedConfigPath: configPath })
+      .then(() => {
+        state.pendingByPath.delete(configPath);
+        state.persistWarned.delete(configPath);
+      })
+      .catch((err) => {
+        if (!state.persistWarned.has(configPath)) {
+          state.persistWarned.add(configPath);
+          logger.warn(
+            `Failed to persist auto-generated commands.ownerDisplaySecret at ${configPath}: ${String(err)}`,
+          );
+        }
+      })
+      .finally(() => {
+        state.persistInFlight.delete(configPath);
+      });
+  }
+
+  return config;
+}
--- a/src/config/io.ts
+++ b/src/config/io.ts
@@ -37,6 +37,7 @@ import {
  readConfigIncludeFileWithGuards,
  resolveConfigIncludes,
 } from "./includes.js";
+import { persistGeneratedOwnerDisplaySecret } from "./io.owner-display-secret.js";
 import { findLegacyConfigIssues } from "./legacy.js";
 import {
  asResolvedSourceConfig,
@@ -1840,35 +1841,18 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
        cfg,
        () => pendingSecret ?? crypto.randomBytes(32).toString("hex"),
      );
-      const cfgWithOwnerDisplaySecret = ownerDisplaySecretResolution.config;
-      if (ownerDisplaySecretResolution.generatedSecret) {
-        AUTO_OWNER_DISPLAY_SECRET_BY_PATH.set(
-          configPath,
-          ownerDisplaySecretResolution.generatedSecret,
-        );
-        if (!AUTO_OWNER_DISPLAY_SECRET_PERSIST_IN_FLIGHT.has(configPath)) {
-          AUTO_OWNER_DISPLAY_SECRET_PERSIST_IN_FLIGHT.add(configPath);
-          void writeConfigFile(cfgWithOwnerDisplaySecret, { expectedConfigPath: configPath })
-            .then(() => {
-              AUTO_OWNER_DISPLAY_SECRET_BY_PATH.delete(configPath);
-              AUTO_OWNER_DISPLAY_SECRET_PERSIST_WARNED.delete(configPath);
-            })
-            .catch((err) => {
-              if (!AUTO_OWNER_DISPLAY_SECRET_PERSIST_WARNED.has(configPath)) {
-                AUTO_OWNER_DISPLAY_SECRET_PERSIST_WARNED.add(configPath);
-                deps.logger.warn(
-                  `Failed to persist auto-generated commands.ownerDisplaySecret at ${configPath}: ${String(err)}`,
-                );
-              }
-            })
-            .finally(() => {
-              AUTO_OWNER_DISPLAY_SECRET_PERSIST_IN_FLIGHT.delete(configPath);
-            });
-        }
-      } else {
-        AUTO_OWNER_DISPLAY_SECRET_BY_PATH.delete(configPath);
-        AUTO_OWNER_DISPLAY_SECRET_PERSIST_WARNED.delete(configPath);
-      }
+      const cfgWithOwnerDisplaySecret = persistGeneratedOwnerDisplaySecret({
+        config: ownerDisplaySecretResolution.config,
+        configPath,
+        generatedSecret: ownerDisplaySecretResolution.generatedSecret,
+        logger: deps.logger,
+        state: {
+          pendingByPath: AUTO_OWNER_DISPLAY_SECRET_BY_PATH,
+          persistInFlight: AUTO_OWNER_DISPLAY_SECRET_PERSIST_IN_FLIGHT,
+          persistWarned: AUTO_OWNER_DISPLAY_SECRET_PERSIST_WARNED,
+        },
+        persistConfig: (nextConfig, options) => writeConfigFile(nextConfig, options),
+      });

      return applyConfigOverrides(cfgWithOwnerDisplaySecret);
    } catch (err) {