diff --git a/src/commands/doctor-config-flow.gateway-bind-persistence.test.ts b/src/commands/doctor-config-flow.gateway-bind-persistence.test.ts new file mode 100644 index 000000000000..2191a6597f00 --- /dev/null +++ b/src/commands/doctor-config-flow.gateway-bind-persistence.test.ts @@ -0,0 +1,65 @@ +// Verifies Doctor persists legacy gateway bind repairs through the real config writer. +import fs from "node:fs/promises"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { readConfigFileSnapshot } from "../config/config.js"; +import { withTempHome, writeOpenClawConfig } from "../config/test-helpers.js"; +import { runInitialConfigWriteHealth } from "../flows/doctor-health-contribution-runners.config.js"; +import type { DoctorHealthFlowContext } from "../flows/doctor-health-contribution-types.js"; +import type { RuntimeEnv } from "../runtime.js"; +import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js"; +import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js"; +import { createDoctorPrompter, type DoctorOptions } from "./doctor-prompter.js"; + +describe("Doctor gateway bind persistence", () => { + afterEach(() => { + closeOpenClawStateDatabaseForTest(); + }); + + it.each([ + ["localhost", "loopback"], + ["0.0.0.0", "lan"], + ] as const)("persists gateway bind %s as %s", async (legacyBind, canonicalBind) => { + await withTempHome(async (home) => { + const configPath = await writeOpenClawConfig(home, { + gateway: { mode: "local", bind: legacyBind }, + }); + const runtime: RuntimeEnv = { + error: vi.fn(), + exit: vi.fn(), + log: vi.fn(), + }; + const options: DoctorOptions = { nonInteractive: true, repair: true }; + const prompter = createDoctorPrompter({ runtime, options }); + const configResult = await loadAndMaybeMigrateDoctorConfig({ + options, + confirm: (params) => prompter.confirm(params), + runtime, + prompter, + }); + const ctx: DoctorHealthFlowContext = { + runtime, + options, + prompter, + configResult, + cfg: configResult.cfg, + cfgForPersistence: structuredClone(configResult.cfg), + sourceConfigValid: configResult.sourceConfigValid ?? true, + configPath, + stateDirExistedAtStart: true, + ...(configResult.runWithPluginMetadataSnapshot + ? { runWithPluginMetadataSnapshot: configResult.runWithPluginMetadataSnapshot } + : {}), + ...(configResult.invalidatePluginMetadataSnapshot + ? { invalidatePluginMetadataSnapshot: configResult.invalidatePluginMetadataSnapshot } + : {}), + }; + + await runInitialConfigWriteHealth(ctx); + + const snapshot = await readConfigFileSnapshot(); + expect(snapshot.valid).toBe(true); + expect(snapshot.config.gateway?.bind).toBe(canonicalBind); + expect(await fs.readFile(configPath, "utf-8")).not.toContain(`"bind": "${legacyBind}"`); + }); + }); +}); diff --git a/src/commands/doctor-config-preflight.test.ts b/src/commands/doctor-config-preflight.test.ts index 3380af1a1ef6..2251e9a566b3 100644 --- a/src/commands/doctor-config-preflight.test.ts +++ b/src/commands/doctor-config-preflight.test.ts @@ -4,6 +4,8 @@ import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { applyCliProfileEnv } from "../cli/profile.js"; import { promoteConfigSnapshotToLastKnownGood, readConfigFileSnapshot } from "../config/config.js"; +import { writeConfigHealthStateToStore } from "../config/io.health-state.js"; +import { createConfigHealthFingerprint } from "../config/io.observe-state.js"; import { withEnvOverride, withTempHome, writeOpenClawConfig } from "../config/test-helpers.js"; import { executeSqliteQueryTakeFirstSync, getNodeSqliteKysely } from "../infra/kysely-sync.js"; import type { DB as OpenClawStateKyselyDatabase } from "../state/openclaw-state-db.generated.js"; @@ -37,6 +39,36 @@ async function writeLegacyConfig(home: string): Promise { return legacyPath; } +async function seedLastKnownGood( + home: string, + configPath: string, + config: Record, +): Promise { + const raw = `${JSON.stringify(config, null, 2)}\n`; + const lastGoodPath = `${configPath}.last-good`; + await fs.writeFile(lastGoodPath, raw, "utf-8"); + const fingerprint = createConfigHealthFingerprint({ + raw, + parsed: config, + stat: await fs.stat(lastGoodPath), + }); + writeConfigHealthStateToStore( + { + env: { ...process.env, HOME: home }, + homedir: () => home, + logger: { warn: () => {} }, + }, + { + entries: { + [configPath]: { + lastKnownGood: fingerprint, + lastPromotedGood: fingerprint, + }, + }, + }, + ); +} + describe("runDoctorConfigPreflight", () => { afterEach(() => { closeOpenClawStateDatabaseForTest(); @@ -262,6 +294,66 @@ describe("runDoctorConfigPreflight", () => { }); }); + it.each([ + ["localhost", "loopback"], + ["0.0.0.0", "lan"], + ] as const)( + "migrates last-known-good gateway bind %s to %s before restoring", + async (legacyBind, canonicalBind) => { + await withTempHome(async (home) => { + const configPath = await writeOpenClawConfig(home, { + gateway: { mode: "local" }, + }); + await seedLastKnownGood(home, configPath, { + gateway: { mode: "local", bind: legacyBind }, + }); + const brokenRaw = '{ "gateway": { "mode": "local" },'; + await fs.writeFile(configPath, brokenRaw, "utf-8"); + + const repaired = await runDoctorConfigPreflight({ + migrateState: false, + migrateLegacyConfig: false, + repairPrefixedConfig: true, + invalidConfigNote: false, + }); + + expect(repaired.snapshot.valid).toBe(true); + expect(repaired.snapshot.config.gateway?.bind).toBe(canonicalBind); + const persisted = JSON.parse(await fs.readFile(configPath, "utf-8")) as { + gateway?: { bind?: string }; + }; + expect(persisted.gateway?.bind).toBe(canonicalBind); + }); + }, + ); + + it("preserves the active config when last-known-good cannot converge", async () => { + await withTempHome(async (home) => { + const configPath = await writeOpenClawConfig(home, { + gateway: { mode: "local" }, + }); + await seedLastKnownGood(home, configPath, { + gateway: { mode: "local", bind: "not-a-bind-mode" }, + }); + const brokenRaw = '{ "gateway": { "mode": "local" },'; + await fs.writeFile(configPath, brokenRaw, "utf-8"); + + const failure = await runDoctorConfigPreflight({ + migrateState: false, + migrateLegacyConfig: false, + repairPrefixedConfig: true, + invalidConfigNote: false, + }).then( + () => null, + (error: unknown) => error, + ); + + expect(failure).toBeInstanceOf(Error); + expect((failure as Error).message).toContain("Config could not be parsed or recovered"); + await expect(fs.readFile(configPath, "utf-8")).resolves.toBe(brokenRaw); + }); + }); + it("preserves and rejects unparseable config without last-known-good during repair preflight", async () => { await withTempHome(async (home) => { const configPath = path.join(home, ".openclaw", "openclaw.json"); diff --git a/src/commands/doctor/shared/config-flow-steps.ts b/src/commands/doctor/shared/config-flow-steps.ts index a73c8bb17186..cbce6402024c 100644 --- a/src/commands/doctor/shared/config-flow-steps.ts +++ b/src/commands/doctor/shared/config-flow-steps.ts @@ -5,13 +5,11 @@ import { stripUnknownConfigKeys } from "../../doctor-config-analysis.js"; import type { DoctorConfigPreflightResult } from "../../doctor-config-preflight.js"; import type { DoctorConfigMutationState } from "./config-mutation-state.js"; import { - classifyConfigPathMigrationOwnership, + classifyOtelGrpcMigrationOwnership, containsAuthoredInclude, } from "./include-migration-ownership.js"; import { migrateLegacyConfig } from "./legacy-config-migrate.js"; -const OTEL_GRPC_PROTOCOL_PATH = "diagnostics.otel.protocol"; - /** Apply legacy config migrations and update preview/fix state for doctor config flow. */ export function applyLegacyCompatibilityStep(params: { snapshot: DoctorConfigPreflightResult["snapshot"]; @@ -34,21 +32,24 @@ export function applyLegacyCompatibilityStep(params: { } const issueLines = formatConfigIssueLines(params.snapshot.legacyIssues, "-"); - if (params.snapshot.legacyIssues.some((issue) => issue.path === OTEL_GRPC_PROTOCOL_PATH)) { - const ownership = classifyConfigPathMigrationOwnership({ - snapshot: params.snapshot, - configPath: ["diagnostics", "otel", "protocol"], - }); + const otelOwnership = classifyOtelGrpcMigrationOwnership({ + snapshot: params.snapshot, + authoredConfig: params.snapshot.parsed, + resolvedConfig: params.snapshot.sourceConfig, + }); + if (otelOwnership) { + const ownership = otelOwnership; if (ownership.kind === "manual") { + const otelPath = "diagnostics.otel.protocol"; const targets = ownership.targetPaths.length > 0 - ? ` Inspect these candidate source files and remove or replace ${OTEL_GRPC_PROTOCOL_PATH} = "grpc" from every definition: ${ownership.targetPaths.join(", ")}.` - : ` Remove or replace ${OTEL_GRPC_PROTOCOL_PATH} = "grpc" in the owning $include directive or included file.`; + ? ` Inspect these candidate source files and remove or replace ${otelPath} = "grpc" from every definition: ${ownership.targetPaths.join(", ")}.` + : ` Remove or replace ${otelPath} = "grpc" in the owning $include directive or included file.`; return { state: params.state, issueLines: [ ...issueLines, - `- ${OTEL_GRPC_PROTOCOL_PATH}: Doctor cannot safely rewrite this $include ownership.${targets} No config files were changed.`, + `- ${otelPath}: Doctor cannot safely rewrite this $include ownership.${targets} No config files were changed.`, ], changeLines: [], blocksWrite: true, diff --git a/src/commands/doctor/shared/include-migration-ownership.test.ts b/src/commands/doctor/shared/include-migration-ownership.test.ts index 92121d5791d3..82cf08dd2e96 100644 --- a/src/commands/doctor/shared/include-migration-ownership.test.ts +++ b/src/commands/doctor/shared/include-migration-ownership.test.ts @@ -2,7 +2,7 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../../../config/types.openclaw.js"; import { - classifyConfigPathMigrationOwnership, + classifyOtelGrpcMigrationOwnership, isSingleTopLevelIncludeMigration, } from "./include-migration-ownership.js"; @@ -17,42 +17,40 @@ describe("include migration ownership", () => { const configDir = path.resolve("/tmp/openclaw-config"); const configPath = path.join(configDir, "openclaw.json"); const diagnosticsPath = path.join(configDir, "diagnostics.json5"); + const classifyOtelOwnership = ( + includeProvenance: NonNullable< + Parameters[0]["snapshot"]["includeProvenance"] + >, + ) => + classifyOtelGrpcMigrationOwnership({ + snapshot: { path: configPath, includeProvenance }, + authoredConfig: { diagnostics: { otel: { protocol: "grpc" } } }, + resolvedConfig: { diagnostics: { otel: { protocol: "grpc" } } }, + }); it("classifies direct config even when an unrelated include exists", () => { expect( - classifyConfigPathMigrationOwnership({ - snapshot: { - path: configPath, - includeProvenance: [ - { - path: ["agents"], - kind: "single", - hasSiblingOverrides: false, - targetPath: path.join(configDir, "agents.json5"), - }, - ], + classifyOtelOwnership([ + { + path: ["agents"], + kind: "single", + hasSiblingOverrides: false, + targetPath: path.join(configDir, "agents.json5"), }, - configPath: ["diagnostics", "otel", "protocol"], - }), + ]), ).toEqual({ kind: "direct" }); }); it("allows one internal top-level include that solely owns diagnostics", () => { expect( - classifyConfigPathMigrationOwnership({ - snapshot: { - path: configPath, - includeProvenance: [ - { - path: ["diagnostics"], - kind: "single", - hasSiblingOverrides: false, - targetPath: diagnosticsPath, - }, - ], + classifyOtelOwnership([ + { + path: ["diagnostics"], + kind: "single", + hasSiblingOverrides: false, + targetPath: diagnosticsPath, }, - configPath: ["diagnostics", "otel", "protocol"], - }), + ]), ).toEqual({ kind: "single-top-level-include", targetPath: diagnosticsPath }); }); @@ -124,12 +122,7 @@ describe("include migration ownership", () => { targetPaths: [path.resolve(configDir, "..", "external-diagnostics.json5")], }, ])("requires manual repair for $name ownership", ({ includeProvenance, targetPaths }) => { - expect( - classifyConfigPathMigrationOwnership({ - snapshot: { path: configPath, includeProvenance }, - configPath: ["diagnostics", "otel", "protocol"], - }), - ).toEqual({ kind: "manual", targetPaths }); + expect(classifyOtelOwnership(includeProvenance)).toEqual({ kind: "manual", targetPaths }); }); it("allows one isolated direct top-level string include", () => { diff --git a/src/commands/doctor/shared/include-migration-ownership.ts b/src/commands/doctor/shared/include-migration-ownership.ts index 455ac0684d18..915ceba15a0c 100644 --- a/src/commands/doctor/shared/include-migration-ownership.ts +++ b/src/commands/doctor/shared/include-migration-ownership.ts @@ -21,8 +21,10 @@ type ConfigPathMigrationOwnership = | { kind: "single-top-level-include"; targetPath: string } | { kind: "manual"; targetPaths: string[] }; +type OtelGrpcMigrationOwnership = ConfigPathMigrationOwnership | { kind: "resolved-only" }; + /** Classify whether Doctor can safely persist a migration at one resolved config path. */ -export function classifyConfigPathMigrationOwnership(params: { +function classifyConfigPathMigrationOwnership(params: { snapshot: Pick; configPath: readonly string[]; }): ConfigPathMigrationOwnership { @@ -57,6 +59,32 @@ export function classifyConfigPathMigrationOwnership(params: { return { kind: "manual", targetPaths }; } +function readOtelProtocol(config: unknown): unknown { + const root = isRecord(config) ? config : null; + const diagnostics = isRecord(root?.diagnostics) ? root.diagnostics : null; + const otel = isRecord(diagnostics?.otel) ? diagnostics.otel : null; + return otel?.protocol; +} + +/** Classify ownership for the sole legacy migration that consults resolved config values. */ +export function classifyOtelGrpcMigrationOwnership(params: { + snapshot: Pick; + authoredConfig: unknown; + resolvedConfig: unknown; +}): OtelGrpcMigrationOwnership | null { + if (readOtelProtocol(params.resolvedConfig) !== "grpc") { + return null; + } + const ownership = classifyConfigPathMigrationOwnership({ + snapshot: params.snapshot, + configPath: ["diagnostics", "otel", "protocol"], + }); + if (ownership.kind !== "direct") { + return ownership; + } + return readOtelProtocol(params.authoredConfig) === "grpc" ? ownership : { kind: "resolved-only" }; +} + export function isSingleTopLevelIncludeMigration(params: { parsed: unknown; sourceConfig: OpenClawConfig; diff --git a/src/config/io.compat.test.ts b/src/config/io.compat.test.ts index ec11b9795984..2807dadd0dd6 100644 --- a/src/config/io.compat.test.ts +++ b/src/config/io.compat.test.ts @@ -86,6 +86,33 @@ describe("config io paths", () => { }); }); + it.each(["lan", "loopback", "tailnet", "auto", "custom", undefined] as const)( + "keeps canonical gateway bind %s byte-identical during load", + async (bind) => { + await withTempHome(async (home) => { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + await fs.mkdir(path.dirname(configPath), { recursive: true }); + const gateway = { + mode: "local" as const, + ...(bind ? { bind } : {}), + ...(bind === "custom" ? { customBindHost: "127.0.0.1" } : {}), + }; + const raw = `${JSON.stringify({ gateway }, null, 2)}\n`; + await fs.writeFile(configPath, raw, "utf-8"); + const io = createConfigIO({ + configPath, + env: { HOME: home } as NodeJS.ProcessEnv, + homedir: () => home, + }); + + const config = io.loadConfig(); + + expect(config.gateway?.bind).toBe(bind); + await expect(fs.readFile(configPath, "utf-8")).resolves.toBe(raw); + }); + }, + ); + it("logs validation warnings with real line breaks", async () => { await withTempHome(async (home) => { const configPath = path.join(home, ".openclaw", "openclaw.json"); diff --git a/src/config/io.context.ts b/src/config/io.context.ts index 542ec8b558fc..6c9fff68db6c 100644 --- a/src/config/io.context.ts +++ b/src/config/io.context.ts @@ -2,6 +2,8 @@ import crypto from "node:crypto"; import { collectManifestModelIdNormalizationPolicies } from "@openclaw/model-catalog-core/provider-model-id-normalization"; import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../agents/agent-scope-config.js"; import { ensureOwnerDisplaySecret } from "../agents/owner-display.js"; +import { classifyOtelGrpcMigrationOwnership } from "../commands/doctor/shared/include-migration-ownership.js"; +import { applyLegacyDoctorMigrations } from "../commands/doctor/shared/legacy-config-compat.js"; import { loadShellEnvFallback, resolveShellEnvFallbackTimeoutMs, @@ -25,7 +27,13 @@ import { resolveConfigPathForDeps, } from "./io.read-helpers.js"; import { autoOwnerDisplaySecretByPath } from "./io.state.js"; -import type { ConfigIoFactoryOptions, NormalizedConfigIoDeps } from "./io.types.js"; +import type { + ConfigIoFactoryOptions, + ConfigRecoveryCandidate, + ConfigRecoveryCandidatePreparation, + NormalizedConfigIoDeps, +} from "./io.types.js"; +import { formatConfigIssueSummary } from "./issue-format.js"; import { migratePersistedImplicitMainRoster } from "./legacy.roster.js"; import { materializeRuntimeConfig } from "./materialize.js"; import { applyConfigOverrides } from "./runtime-overrides.js"; @@ -50,7 +58,9 @@ export type ConfigIoContext = { allowCurrentPluginMetadata?: boolean; }) => ValidationPluginMetadataSnapshotLoader; resolveRuntimePreflightSourceConfig: (candidate: OpenClawConfig) => OpenClawConfig; - resolveSuspiciousRecoveryBackupCandidate: (parsed: unknown) => OpenClawConfig | null; + prepareRecoveryBackupCandidate: ( + candidate: ConfigRecoveryCandidate, + ) => ConfigRecoveryCandidatePreparation; }; export function createConfigIoContext(options: ConfigIoFactoryOptions = {}): ConfigIoContext { @@ -132,10 +142,52 @@ export function createConfigIoContext(options: ConfigIoFactoryOptions = {}): Con return coerceConfig(migratePersistedImplicitMainRoster(resolution.resolvedConfigRaw).config); } - function resolveSuspiciousRecoveryBackupCandidate(parsed: unknown): OpenClawConfig | null { + function prepareRecoveryBackupCandidate( + candidate: ConfigRecoveryCandidate, + ): ConfigRecoveryCandidatePreparation { try { + const originalEnv = cloneEnvWithPlatformSemantics(deps.env); + const includeProvenance: NonNullable[number][] = []; + const originalResolvedIncludes = resolveConfigIncludesForRead( + candidate.parsed, + configPath, + { ...deps, env: originalEnv }, + undefined, + undefined, + undefined, + (event) => { + const { value: _value, ...ownership } = event; + includeProvenance.push(ownership); + }, + ); + const originalResolution = resolveConfigForRead( + originalResolvedIncludes, + originalEnv, + deps.lowerPrecedenceEnv, + ); + const otelOwnership = classifyOtelGrpcMigrationOwnership({ + snapshot: { path: configPath, includeProvenance }, + authoredConfig: candidate.parsed, + resolvedConfig: originalResolution.resolvedConfigRaw, + }); + if (otelOwnership && otelOwnership.kind !== "direct") { + return { + ok: false, + reason: + otelOwnership.kind === "resolved-only" + ? "candidate migration cannot persist an env-resolved diagnostics.otel.protocol repair" + : "candidate migration requires an include-owned diagnostics.otel.protocol repair", + }; + } + // Recovery is a migration boundary, not runtime compatibility: the canonical Doctor + // registry owns historical shapes before current-schema validation and any disk write. + const migrated = applyLegacyDoctorMigrations(candidate.parsed, { + authoredRaw: candidate.parsed, + resolvedRaw: originalResolution.resolvedConfigRaw, + }); + const authoredCandidate = migrated.next ?? candidate.parsed; const candidateEnv = cloneEnvWithPlatformSemantics(deps.env); - const resolved = resolveConfigIncludesForRead(parsed, configPath, { + const resolved = resolveConfigIncludesForRead(authoredCandidate, configPath, { ...deps, env: candidateEnv, }); @@ -149,12 +201,30 @@ export function createConfigIoContext(options: ConfigIoFactoryOptions = {}): Con env: candidateEnv, pluginValidation: options.pluginValidation, loadPluginMetadataSnapshot: pluginMetadata.load, - sourceRaw: parsed, + sourceRaw: authoredCandidate, preservedLegacyRootKeys: options.preservedLegacyRootKeys, }); - return validated.ok ? coerceConfig(effectiveConfigRaw) : null; - } catch { - return null; + if (!validated.ok) { + const issueSummary = formatConfigIssueSummary(validated.issues.slice(0, 3)) ?? ""; + const detail = issueSummary.length > 800 ? `${issueSummary.slice(0, 799)}…` : issueSummary; + return { + ok: false, + reason: `candidate remains invalid after legacy migration${detail ? `: ${detail}` : ""}`, + }; + } + return { + ok: true, + candidate: { + config: validated.config, + parsed: authoredCandidate, + raw: migrated.next + ? JSON.stringify(authoredCandidate, null, 2).trimEnd().concat("\n") + : candidate.raw, + }, + }; + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + return { ok: false, reason: `candidate preparation failed: ${detail}` }; } } @@ -166,7 +236,7 @@ export function createConfigIoContext(options: ConfigIoFactoryOptions = {}): Con finalizeLoadedRuntimeConfig, createValidationPluginMetadataSnapshotLoader, resolveRuntimePreflightSourceConfig, - resolveSuspiciousRecoveryBackupCandidate, + prepareRecoveryBackupCandidate, }; } diff --git a/src/config/io.factory.ts b/src/config/io.factory.ts index 60fb574c9e15..32808ca4843a 100644 --- a/src/config/io.factory.ts +++ b/src/config/io.factory.ts @@ -46,6 +46,7 @@ export function createConfigIO(options: ConfigIoFactoryOptions = {}) { deps: context.deps, snapshot: params.snapshot, reason: params.reason, + prepareCandidate: context.prepareRecoveryBackupCandidate, }), preserveConfigSnapshotAsClobbered: (snapshot: ConfigFileSnapshot) => preserveConfigSnapshotAsClobbered({ deps: context.deps, snapshot }), diff --git a/src/config/io.load.ts b/src/config/io.load.ts index d0839e57963b..df01ba722eab 100644 --- a/src/config/io.load.ts +++ b/src/config/io.load.ts @@ -155,8 +155,7 @@ export function loadConfigFromContext( configPath, raw, parsed, - validateBackupSync: (backup) => - context.resolveSuspiciousRecoveryBackupCandidate(backup.parsed) !== null, + prepareBackup: context.prepareRecoveryBackupCandidate, }); if (recovery.raw !== raw) { restoreEnvChangesIfUnchanged({ diff --git a/src/config/io.observe-recovery.test.ts b/src/config/io.observe-recovery.test.ts index ed2afbe9179e..8d2976d81541 100644 --- a/src/config/io.observe-recovery.test.ts +++ b/src/config/io.observe-recovery.test.ts @@ -24,6 +24,10 @@ import type { ConfigFileSnapshot } from "./types.js"; const CONFIG_CLOBBER_SNAPSHOT_LIMIT = 32; type ConfigHealthDatabase = Pick; type ObserveRecoveryDeps = Parameters[0]["deps"]; +const approveRecoveryCandidate = (candidate: T) => ({ + ok: true as const, + candidate, +}); function resolveLastKnownGoodConfigPath(configPath: string): string { return `${configPath}.last-good`; @@ -197,6 +201,7 @@ describe("config observe recovery", () => { configPath: params.configPath, raw: clobberedUpdateChannelRaw, parsed: clobberedUpdateChannelConfig, + prepareBackup: approveRecoveryCandidate, }); } @@ -211,6 +216,7 @@ describe("config observe recovery", () => { configPath: params.configPath, raw: params.raw, parsed: params.parsed, + prepareBackup: approveRecoveryCandidate, }); } @@ -223,6 +229,7 @@ describe("config observe recovery", () => { configPath: params.configPath, raw: clobberedUpdateChannelRaw, parsed: clobberedUpdateChannelConfig, + prepareBackup: approveRecoveryCandidate, }); } @@ -402,6 +409,7 @@ describe("config observe recovery", () => { configPath, raw: clobbered.raw, parsed: clobbered.parsed, + prepareBackup: approveRecoveryCandidate, }); expect((recovered.parsed as { gateway?: { mode?: string } }).gateway?.mode).toBe("local"); @@ -609,6 +617,133 @@ describe("config observe recovery", () => { }); }); + it.each([ + ["localhost", "loopback"], + ["0.0.0.0", "lan"], + ] as const)( + "migrates backup gateway bind %s to %s before recovery", + async (bind, canonicalBind) => { + await withSuiteHome(async (home) => { + const { io, configPath, warn } = createTestConfigIO(home); + await seedConfigBackup(configPath, { + gateway: { mode: "local", bind }, + }); + await fsp.writeFile( + `${configPath}.bak`, + `{\n // historical bind alias\n gateway: { mode: "local", bind: "${bind}" }\n}\n`, + "utf-8", + ); + const clobbered = await writeConfigRaw(configPath, { + meta: { lastTouchedVersion: "2026.5.28" }, + }); + + const snapshot = await io.readConfigFileSnapshot({ recoverSuspicious: true }); + + expect(snapshot.valid).toBe(true); + expect(snapshot.config.gateway?.mode).toBe("local"); + expect(snapshot.config.gateway?.bind).toBe(canonicalBind); + expect(JSON5.parse(await fsp.readFile(configPath, "utf-8"))).toMatchObject({ + gateway: { mode: "local", bind: canonicalBind }, + }); + await expect(fsp.readFile(configPath, "utf-8")).resolves.not.toBe(clobbered.raw); + await expect(listClobberFiles(configPath)).resolves.toHaveLength(1); + expectWarnContaining(warn, `Config write will strip JSON5 comments from ${configPath}.`); + }); + }, + ); + + it("does not flatten env- or include-owned bind aliases during backup recovery", async () => { + await withSuiteHome(async (home) => { + const env = { RECOVERY_BIND: "localhost" } as NodeJS.ProcessEnv; + const { io, configPath } = createTestConfigIO(home, vi.fn(), { env }); + await seedConfigBackup(configPath, { + gateway: { mode: "local", bind: "${RECOVERY_BIND}" }, + }); + const envOwnedCurrent = await writeConfigRaw(configPath, { + meta: { lastTouchedVersion: "2026.5.28" }, + }); + + await io.readConfigFileSnapshot({ recoverSuspicious: true }); + + await expect(fsp.readFile(configPath, "utf-8")).resolves.toBe(envOwnedCurrent.raw); + + await fsp.writeFile( + path.join(path.dirname(configPath), "legacy-bind.json5"), + '{ gateway: { mode: "local", bind: "localhost" } }\n', + "utf-8", + ); + await seedConfigBackup(configPath, { $include: "./legacy-bind.json5" }); + const includeOwnedCurrent = await writeConfigRaw(configPath, { + meta: { lastTouchedVersion: "2026.5.28" }, + }); + + await io.readConfigFileSnapshot({ recoverSuspicious: true }); + + await expect(fsp.readFile(configPath, "utf-8")).resolves.toBe(includeOwnedCurrent.raw); + }); + }); + + it("migrates directly authored OTel grpc config before backup recovery", async () => { + await withSuiteHome(async (home) => { + const { io, configPath } = createTestConfigIO(home); + await seedConfigBackup(configPath, { + gateway: { mode: "local" }, + diagnostics: { otel: { enabled: true, protocol: "grpc", traces: true } }, + }); + await writeConfigRaw(configPath, { + meta: { lastTouchedVersion: "2026.5.28" }, + }); + + const snapshot = await io.readConfigFileSnapshot({ recoverSuspicious: true }); + + expect(snapshot.config.diagnostics?.otel?.protocol).toBeUndefined(); + expect(snapshot.config.diagnostics?.otel?.enabled).toBe(false); + }); + }); + + it("does not persist env-resolved OTel grpc config during backup recovery", async () => { + await withSuiteHome(async (home) => { + const env = { OTEL_PROTOCOL: "grpc" } as NodeJS.ProcessEnv; + const { io, configPath } = createTestConfigIO(home, vi.fn(), { env }); + await seedConfigBackup(configPath, { + gateway: { mode: "local" }, + diagnostics: { otel: { enabled: true, protocol: "${OTEL_PROTOCOL}", traces: true } }, + }); + const active = await writeConfigRaw(configPath, { + meta: { lastTouchedVersion: "2026.5.28" }, + }); + + await io.readConfigFileSnapshot({ recoverSuspicious: true }); + + await expect(fsp.readFile(configPath, "utf-8")).resolves.toBe(active.raw); + await expect(listClobberFiles(configPath)).resolves.toHaveLength(0); + }); + }); + + it("does not flatten include-owned OTel settings during backup recovery", async () => { + await withSuiteHome(async (home) => { + const { io, configPath } = createTestConfigIO(home); + await fsp.mkdir(path.dirname(configPath), { recursive: true }); + await fsp.writeFile( + path.join(path.dirname(configPath), "legacy-otel.json5"), + '{ enabled: true, protocol: "grpc", traces: true }\n', + "utf-8", + ); + await seedConfigBackup(configPath, { + gateway: { mode: "local" }, + diagnostics: { otel: { $include: "./legacy-otel.json5" } }, + }); + const active = await writeConfigRaw(configPath, { + meta: { lastTouchedVersion: "2026.5.28" }, + }); + + await io.readConfigFileSnapshot({ recoverSuspicious: true }); + + await expect(fsp.readFile(configPath, "utf-8")).resolves.toBe(active.raw); + await expect(listClobberFiles(configPath)).resolves.toHaveLength(0); + }); + }); + it("does not auto-restore backup candidates rejected by the caller", async () => { await withSuiteHome(async (home) => { const { io, configPath } = createTestConfigIO(home); @@ -740,7 +875,7 @@ describe("config observe recovery", () => { "utf-8", ); const clobbered = await writeClobberedUpdateChannel(configPath); - const input = { deps, configPath, ...clobbered }; + const input = { deps, configPath, ...clobbered, prepareBackup: approveRecoveryCandidate }; const recovered = mode === "async" @@ -762,7 +897,7 @@ describe("config observe recovery", () => { const backup = { meta: { authoredBy: "operator" }, gateway: { mode: "local" } }; await seedConfigBackup(configPath, backup); const clobbered = await writeConfigRaw(configPath, { gateway: { mode: "local" } }); - const input = { deps, configPath, ...clobbered }; + const input = { deps, configPath, ...clobbered, prepareBackup: approveRecoveryCandidate }; const recovered = mode === "async" @@ -801,7 +936,12 @@ describe("config observe recovery", () => { }) as typeof fs.statSync, }, }; - const input = { deps: statDeps, configPath, ...clobbered }; + const input = { + deps: statDeps, + configPath, + ...clobbered, + prepareBackup: approveRecoveryCandidate, + }; const recovered = mode === "async" @@ -815,22 +955,22 @@ describe("config observe recovery", () => { it.each([ { - name: "records writeFile failure instead of falsely claiming restore succeeded", + name: "records atomic replace failure instead of falsely claiming restore succeeded", mode: "async", retry: false, }, { - name: "sync recovery records writeFileSync failure instead of falsely claiming restore succeeded", + name: "sync recovery records atomic replace failure instead of falsely claiming restore succeeded", mode: "sync", retry: false, }, { - name: "retries recovery on next launch after a failed writeFile restore", + name: "retries recovery on next launch after a failed atomic replace", mode: "async", retry: true, }, { - name: "sync recovery retries on next launch after a failed writeFileSync restore", + name: "sync recovery retries on next launch after a failed atomic replace", mode: "sync", retry: true, }, @@ -846,23 +986,28 @@ describe("config observe recovery", () => { ...deps.fs, promises: { ...deps.fs.promises, - writeFile: (target, data, options) => + rename: (source, target) => target === configPath ? Promise.reject(copyError) - : deps.fs.promises.writeFile(target, data, options), + : deps.fs.promises.rename(source, target), }, } : { ...deps.fs, - writeFileSync: (target, data, options) => { + renameSync: (source, target) => { if (target === configPath) { throw copyError; } - return deps.fs.writeFileSync(target, data, options); + return deps.fs.renameSync(source, target); }, }; const recover = (recoveryDeps: ObserveRecoveryDeps) => { - const input = { deps: recoveryDeps, configPath, ...clobbered }; + const input = { + deps: recoveryDeps, + configPath, + ...clobbered, + prepareBackup: approveRecoveryCandidate, + }; return mode === "async" ? maybeRecoverSuspiciousConfigRead(input) : maybeRecoverSuspiciousConfigReadSync(input); @@ -897,8 +1042,8 @@ describe("config observe recovery", () => { }); it.each([ - { name: "restores the exact async backup bytes approved by validation", mode: "async" }, - { name: "restores the exact sync backup bytes approved by validation", mode: "sync" }, + { name: "restores the exact async backup bytes approved by preparation", mode: "async" }, + { name: "restores the exact sync backup bytes approved by preparation", mode: "sync" }, ] as const)("$name", async ({ mode }) => { await withSuiteHome(async (home) => { const { deps, configPath } = makeDeps(home); @@ -907,22 +1052,22 @@ describe("config observe recovery", () => { const approvedRaw = await fsp.readFile(backupPath, "utf-8"); const replacementRaw = `${JSON.stringify({ gateway: { mode: "remote" } }, null, 2)}\n`; const clobbered = await writeClobberedUpdateChannel(configPath); - const input = { deps, configPath, ...clobbered }; + const input = { deps, configPath, ...clobbered, prepareBackup: approveRecoveryCandidate }; if (mode === "async") { await maybeRecoverSuspiciousConfigRead({ ...input, - validateBackup: async () => { - await fsp.writeFile(backupPath, replacementRaw, "utf-8"); - return true; + prepareBackup: (candidate) => { + fs.writeFileSync(backupPath, replacementRaw, "utf-8"); + return { ok: true, candidate }; }, }); } else { maybeRecoverSuspiciousConfigReadSync({ ...input, - validateBackupSync: () => { + prepareBackup: (candidate) => { fs.writeFileSync(backupPath, replacementRaw, "utf-8"); - return true; + return { ok: true, candidate }; }, }); } @@ -1091,6 +1236,7 @@ describe("config observe recovery", () => { issues: [{ path: "gateway.mode", message: "Expected string" }], }, reason: "test-invalid-config", + prepareCandidate: approveRecoveryCandidate, }); expect(restored).toBe(true); @@ -1103,6 +1249,41 @@ describe("config observe recovery", () => { }); }); + it("leaves the active config untouched when its owner rejects last-known-good recovery", async () => { + await withSuiteHome(async (home) => { + const { deps, configPath, warn } = makeDeps(home); + const snapshot = await makeSnapshot(configPath, { + gateway: { mode: "local" }, + }); + await expect( + promoteConfigSnapshotToLastKnownGood({ deps, snapshot, logger: deps.logger }), + ).resolves.toBe(true); + + const brokenRaw = "{ gateway: { mode: 123 } }\n"; + await fsp.writeFile(configPath, brokenRaw, "utf-8"); + const restored = await recoverConfigFromLastKnownGood({ + deps, + snapshot: { + ...snapshot, + raw: brokenRaw, + parsed: { gateway: { mode: 123 } }, + valid: false, + issues: [{ path: "gateway.mode", message: "Expected string" }], + }, + reason: "test-invalid-config", + prepareCandidate: () => ({ + ok: false, + reason: "candidate cannot converge under the current schema", + }), + }); + + expect(restored).toBe(false); + await expect(fsp.readFile(configPath, "utf-8")).resolves.toBe(brokenRaw); + await expect(listClobberFiles(configPath)).resolves.toHaveLength(0); + expectWarnContaining(warn, "candidate cannot converge under the current schema"); + }); + }); + it("warns when last-known-good promotion cannot tighten snapshot permissions", async () => { await withSuiteHome(async (home) => { const { deps, configPath, warn } = makeDeps(home); @@ -1148,6 +1329,7 @@ describe("config observe recovery", () => { issues: [{ path: "gateway.mode", message: "Expected string" }], }, reason: "test-invalid-config", + prepareCandidate: approveRecoveryCandidate, }), ).resolves.toBe(true); @@ -1226,6 +1408,7 @@ describe("config observe recovery", () => { deps, snapshot: { ...staleSnapshot, ...active, valid: false, issues: [issue] }, reason: "reload-invalid-config", + prepareCandidate: approveRecoveryCandidate, }); expect(restored).toBe(false); diff --git a/src/config/io.observe-recovery.ts b/src/config/io.observe-recovery.ts index e8be6500dfa8..89ce7ea0e4a3 100644 --- a/src/config/io.observe-recovery.ts +++ b/src/config/io.observe-recovery.ts @@ -1,5 +1,7 @@ // Observes and recovers config files that appear missing, corrupt, or clobbered. import type fs from "node:fs"; +import path from "node:path"; +import { replaceFileAtomic, replaceFileAtomicSync } from "../infra/replace-file.js"; import { isRecord } from "../utils.js"; import { appendConfigAuditRecord, appendConfigAuditRecordSync } from "./io.audit.js"; import { @@ -22,8 +24,13 @@ import { } from "./io.observe-state.js"; import { resolveConfigObserveSuspiciousReasons } from "./io.observe-suspicious.js"; import { hashConfigRaw, resolveConfigSnapshotHash } from "./io.read-helpers.js"; -import type { NormalizedConfigIoDeps } from "./io.types.js"; +import type { + ConfigRecoveryCandidatePreparation, + NormalizedConfigIoDeps, + PrepareConfigRecoveryCandidate, +} from "./io.types.js"; import { formatConfigIssueSummary } from "./issue-format.js"; +import { warnIfJSON5CommentsWillBeStripped } from "./json5-comments.js"; import { isPluginLocalInvalidConfigSnapshot, shouldAttemptLastKnownGoodRecovery, @@ -83,8 +90,7 @@ type ConfigReadRecoveryParams = { configPath: string; raw: string; parsed: unknown; - validateBackup?: (backup: { raw: string; parsed: unknown }) => Promise; - validateBackupSync?: (backup: { raw: string; parsed: unknown }) => boolean; + prepareBackup: PrepareConfigRecoveryCandidate; allowBackupRecovery?: () => Promise; }; @@ -93,6 +99,29 @@ type ConfigReadRecoveryResult = { parsed: unknown; }; +function createRecoveryCommitEffect(params: { + deps: ObserveRecoveryDeps; + configPath: string; + raw: string; +}): ConfigRecoveryEffect { + const options = { + filePath: params.configPath, + content: params.raw, + dirMode: 0o700, + mode: 0o600, + tempPrefix: path.basename(params.configPath), + fileSystem: params.deps.fs, + }; + return { + sync: () => { + replaceFileAtomicSync(options); + }, + async: async () => { + await replaceFileAtomic(options); + }, + }; +} + type ConfigObserveAuditRecordParams = Parameters[0]; function createConfigObserveAuditAppendParams( @@ -374,13 +403,14 @@ function* recoverSuspiciousConfigRead( return returnOriginalConfigRead(params); } const backupCandidate = { raw: backupRaw, parsed: backupParse.parsed }; - const validBackup = (yield { - sync: () => params.validateBackupSync?.(backupCandidate) ?? true, - async: () => params.validateBackup?.(backupCandidate) ?? true, - }) as boolean; - if (!validBackup) { + const prepared = (yield { + sync: () => params.prepareBackup(backupCandidate), + async: () => params.prepareBackup(backupCandidate), + }) as ConfigRecoveryCandidatePreparation; + if (!prepared.ok) { return returnOriginalConfigRead(params); } + const preparedCandidate = prepared.candidate; // Eligibility must describe the approved backup bytes, never an older healthy config. const backupStat = (yield createConfigRecoveryStatEffect(deps, backupPath)) as fs.Stats | null; const backup = createConfigHealthFingerprint({ @@ -413,11 +443,14 @@ function* recoverSuspiciousConfigRead( let restoredFromBackup = false; let restoreError: unknown; try { - const options = { encoding: "utf-8" as const, mode: 0o600 }; - yield { - sync: () => deps.fs.writeFileSync(configPath, backupRaw, options), - async: () => deps.fs.promises.writeFile(configPath, backupRaw, options), - }; + if (preparedCandidate.raw !== backupRaw) { + warnIfJSON5CommentsWillBeStripped({ + raw: backupRaw, + filePath: configPath, + warn: (message) => deps.logger.warn(message), + }); + } + yield createRecoveryCommitEffect({ deps, configPath, raw: preparedCandidate.raw }); const chmodParams = { deps, configPath, context: "backup restore" }; yield { sync: () => chmodConfigBestEffortSync(chmodParams), @@ -462,7 +495,7 @@ function* recoverSuspiciousConfigRead( }), ); } - return backupCandidate; + return preparedCandidate; } export async function promoteConfigSnapshotToLastKnownGood(params: { @@ -519,6 +552,7 @@ export async function recoverConfigFromLastKnownGood(params: { deps: ObserveRecoveryDeps; snapshot: ConfigFileSnapshot; reason: string; + prepareCandidate: PrepareConfigRecoveryCandidate; }): Promise { const { deps, snapshot } = params; if (!snapshot.exists || typeof snapshot.raw !== "string") { @@ -549,7 +583,18 @@ export async function recoverConfigFromLastKnownGood(params: { } catch { return false; } - const polluted = collectPollutedSecretPlaceholders(backupParsed); + // Historical bytes become live config only after their owner has migrated and validated them. + // This prevents Doctor recovery from exposing a schema-invalid intermediate file. + const originalCandidate = { raw: backupRaw, parsed: backupParsed }; + const prepared = params.prepareCandidate(originalCandidate); + if (!prepared.ok) { + deps.logger.warn( + `Config last-known-good recovery skipped: ${prepared.reason} (${params.reason})`, + ); + return false; + } + const recoveryCandidate = prepared.candidate; + const polluted = collectPollutedSecretPlaceholders(recoveryCandidate.parsed); if (polluted.length > 0) { deps.logger.warn( `Config last-known-good recovery skipped: redacted secret placeholder at ${polluted[0]}`, @@ -571,10 +616,18 @@ export async function recoverConfigFromLastKnownGood(params: { snapshot, observedAt: now, }); - await deps.fs.promises.writeFile(snapshot.path, backupRaw, { - encoding: "utf-8", - mode: 0o600, - }); + if (recoveryCandidate.raw !== backupRaw) { + warnIfJSON5CommentsWillBeStripped({ + raw: backupRaw, + filePath: snapshot.path, + warn: (message) => deps.logger.warn(message), + }); + } + await createRecoveryCommitEffect({ + deps, + configPath: snapshot.path, + raw: recoveryCandidate.raw, + }).async(); await chmodConfigBestEffort({ deps, configPath: snapshot.path, diff --git a/src/config/io.snapshot.ts b/src/config/io.snapshot.ts index 4ec0e4b51e7b..0563ee998198 100644 --- a/src/config/io.snapshot.ts +++ b/src/config/io.snapshot.ts @@ -244,9 +244,10 @@ export async function readConfigFileSnapshotInternal( configPath, raw, parsed: effectiveParsed, - validateBackup: async (backup) => { - recoveryCandidate = context.resolveSuspiciousRecoveryBackupCandidate(backup.parsed); - return recoveryCandidate !== null; + prepareBackup: (backup) => { + const prepared = context.prepareRecoveryBackupCandidate(backup); + recoveryCandidate = prepared.ok ? (prepared.candidate.config ?? null) : null; + return prepared; }, ...(allowSuspiciousRecovery ? { diff --git a/src/config/io.types.ts b/src/config/io.types.ts index 49f5475851b1..3a8318ed61ec 100644 --- a/src/config/io.types.ts +++ b/src/config/io.types.ts @@ -148,3 +148,17 @@ export type BestEffortConfigSnapshot = { config: OpenClawConfig; sourceConfig: OpenClawConfig; }; + +export type ConfigRecoveryCandidate = { + raw: string; + parsed: unknown; + config?: OpenClawConfig; +}; + +export type ConfigRecoveryCandidatePreparation = + | { ok: true; candidate: ConfigRecoveryCandidate } + | { ok: false; reason: string }; + +export type PrepareConfigRecoveryCandidate = ( + candidate: ConfigRecoveryCandidate, +) => ConfigRecoveryCandidatePreparation; diff --git a/src/gateway/server-network-runtime.e2e.test.ts b/src/gateway/server-network-runtime.e2e.test.ts index 6311bc9f761c..cb4796462e9e 100644 --- a/src/gateway/server-network-runtime.e2e.test.ts +++ b/src/gateway/server-network-runtime.e2e.test.ts @@ -116,4 +116,54 @@ describe("gateway network runtime", () => { envSnapshot.restore(); } }); + + it.each(["lan", "loopback", "tailnet", "auto", "custom", undefined] as const)( + "starts with persisted canonical bind %s without rewriting config", + async (bind) => { + const envSnapshot = captureEnv([...NETWORK_GATEWAY_ENV_KEYS]); + const tempHome = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-gw-bind-home-")); + let server: Awaited> | undefined; + + try { + for (const key of NETWORK_GATEWAY_ENV_KEYS) { + deleteTestEnvValue(key); + } + setTestEnvValue("HOME", tempHome); + setTestEnvValue("OPENCLAW_STATE_DIR", path.join(tempHome, ".openclaw")); + process.env.OPENCLAW_SKIP_CHANNELS = "1"; + process.env.OPENCLAW_SKIP_GMAIL_WATCHER = "1"; + process.env.OPENCLAW_SKIP_CRON = "1"; + process.env.OPENCLAW_SKIP_CANVAS_HOST = "1"; + process.env.OPENCLAW_SKIP_BROWSER_CONTROL_SERVER = "1"; + process.env.OPENCLAW_SKIP_PROVIDERS = "1"; + process.env.OPENCLAW_TEST_MINIMAL_GATEWAY = "1"; + process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = path.join(tempHome, "empty-bundled-plugins"); + await fs.mkdir(process.env.OPENCLAW_BUNDLED_PLUGINS_DIR, { recursive: true }); + + const token = `bind-token-${process.pid}-${process.env.VITEST_POOL_ID ?? "0"}`; + process.env.OPENCLAW_GATEWAY_TOKEN = token; + const configPath = path.join(tempHome, ".openclaw", "openclaw.json"); + const gateway = { + mode: "local" as const, + auth: { mode: "token" as const, token }, + ...(bind ? { bind } : {}), + ...(bind === "custom" ? { customBindHost: "127.0.0.1" } : {}), + }; + const raw = `${JSON.stringify({ gateway }, null, 2)}\n`; + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile(configPath, raw, { mode: 0o600 }); + setTestEnvValue("OPENCLAW_CONFIG_PATH", configPath); + + server = await startGatewayServer(await getFreeGatewayPort(), { + controlUiEnabled: false, + }); + + await expect(fs.readFile(configPath, "utf-8")).resolves.toBe(raw); + } finally { + await server?.close({ reason: "gateway bind persistence test complete" }); + await fs.rm(tempHome, { recursive: true, force: true }); + envSnapshot.restore(); + } + }, + ); });