diff --git a/src/config/io.compat.test.ts b/src/config/io.compat.test.ts index 563ea072a4a..885df65c505 100644 --- 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): Promise { @@ -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 () => { diff --git a/src/config/io.owner-display-secret.test.ts b/src/config/io.owner-display-secret.test.ts index d83c43b6be4..11849c34793 100644 --- 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 { - 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(), + persistInFlight: new Set(), + persistWarned: new Set(), + }; } -describe("config io owner display secret autofill", () => { - beforeEach(() => { - vi.useRealTimers(); +async function flushAsyncWork(): Promise { + 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); }); }); diff --git a/src/config/io.owner-display-secret.ts b/src/config/io.owner-display-secret.ts new file mode 100644 index 00000000000..c3829c6dacb --- /dev/null +++ b/src/config/io.owner-display-secret.ts @@ -0,0 +1,49 @@ +import type { OpenClawConfig } from "./config.js"; + +export type OwnerDisplaySecretPersistState = { + pendingByPath: Map; + persistInFlight: Set; + persistWarned: Set; +}; + +export function persistGeneratedOwnerDisplaySecret(params: { + config: OpenClawConfig; + configPath: string; + generatedSecret?: string; + logger: Pick; + state: OwnerDisplaySecretPersistState; + persistConfig: ( + config: OpenClawConfig, + options: { expectedConfigPath: string }, + ) => Promise; +}): 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; +} diff --git a/src/config/io.ts b/src/config/io.ts index 483e5f85136..161b6dcced4 100644 --- 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) {