From 6575733bc03537bf7535a33fa87f49da3fb52637 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 16 Aug 2026 18:19:11 -0700 Subject: [PATCH] fix(config): reject scalar config roots instead of loading defaults (#124945) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A config file whose JSON5 root parsed to a scalar (null, a number, a bare string — the classic truncated/clobbered file) hit a special case in loadConfigFromContext that returned {} with a snapshot marked valid: true. Three failures in one: the process silently ran with defaults while the operator's real config (channels, auth, allowlists) disappeared; the valid snapshot could promote the corrupt file's fingerprint as lastKnownGood, poisoning the clobber-recovery machinery that exists to catch exactly this; and the load path contradicted the snapshot path, which correctly reported the same bytes invalid. Root cause: an early-return that collapsed invalid input into the empty-config success shape. Deleted; a scalar root now flows into validateConfigObjectWithPlugins, fails schema validation, records a valid:false snapshot, and throws INVALID_CONFIG — same as an array root, same as the snapshot path, handled by doctor's invalid-config recovery. Regression: new io.scalar-root.test.ts (null/number/string roots all throw INVALID_CONFIG; snapshot agrees invalid) — all 3 fail pre-fix. Updated the compat warning-fingerprint test that relied on null-root loading as defaults. --- src/config/io.compat.test.ts | 6 ++-- src/config/io.load.ts | 36 +++++++--------------- src/config/io.scalar-root.test.ts | 50 +++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 27 deletions(-) create mode 100644 src/config/io.scalar-root.test.ts diff --git a/src/config/io.compat.test.ts b/src/config/io.compat.test.ts index 588c420a08c5..9a8b9d0ae996 100644 --- a/src/config/io.compat.test.ts +++ b/src/config/io.compat.test.ts @@ -249,11 +249,13 @@ describe("config io paths", () => { load(); expect(logger.warn).toHaveBeenCalledTimes(3); + // A null root is invalid config (throws) and, like the invalid-port + // step above, preserves the logged-warning fingerprint. await fs.writeFile(configPath, "null"); - load(); + expect(load).toThrow(); await writeRemovedPlugin("google-gemini-cli-auth"); load(); - expect(logger.warn).toHaveBeenCalledTimes(4); + expect(logger.warn).toHaveBeenCalledTimes(3); }); }); diff --git a/src/config/io.load.ts b/src/config/io.load.ts index eb6f56ffe65f..7d406ad4f4b6 100644 --- a/src/config/io.load.ts +++ b/src/config/io.load.ts @@ -86,31 +86,17 @@ export function loadConfigFromContext( deps.logger.warn(`Config (${configPath}): ${diagnostic}`); } warnOnConfigMiskeys(validationConfigRaw, deps.logger); - if (typeof validationConfigRaw !== "object" || validationConfigRaw === null) { - loggedConfigWarningFingerprints.delete(configPath); - context.observeLoadConfigSnapshot( - createConfigFileSnapshot({ - path: configPath, - exists: true, - raw: snapshotRaw, - parsed: snapshotParsed, - sourceConfig: {}, - valid: true, - runtimeConfig: {}, - hash, - issues: [], - warnings: [], - legacyIssues: [], - }), - ); - return {}; - } - const duplicates = findDuplicateAgentDirs(validationConfigRaw as OpenClawConfig, { - env: deps.env, - homedir: deps.homedir, - }); - if (duplicates.length > 0) { - throw new DuplicateAgentDirError(duplicates); + // A scalar/null root (truncated or clobbered file) must fail validation + // below like any invalid config — never load as an empty config marked + // valid, which would run with defaults and poison lastKnownGood. + if (typeof validationConfigRaw === "object" && validationConfigRaw !== null) { + const duplicates = findDuplicateAgentDirs(validationConfigRaw as OpenClawConfig, { + env: deps.env, + homedir: deps.homedir, + }); + if (duplicates.length > 0) { + throw new DuplicateAgentDirError(duplicates); + } } const pluginMetadata = context.createValidationPluginMetadataSnapshotLoader({ effectiveConfigRaw, diff --git a/src/config/io.scalar-root.test.ts b/src/config/io.scalar-root.test.ts new file mode 100644 index 000000000000..7114f3c5a279 --- /dev/null +++ b/src/config/io.scalar-root.test.ts @@ -0,0 +1,50 @@ +// A scalar/null config root (truncated or clobbered file) must fail loading +// like any invalid config — never load as an empty config marked valid, which +// would run with defaults and poison the lastKnownGood clobber recovery. +import fs from "node:fs/promises"; +import path from "node:path"; +import { describe, expect, it, vi } from "vitest"; +import { withTempDir } from "../test-utils/temp-dir.js"; +import { createConfigIO } from "./io.factory.js"; +import { isInvalidConfigError } from "./io.invalid-config.js"; + +function withTempHome(run: (home: string) => Promise): Promise { + return withTempDir("openclaw-config-scalar-root-", run); +} + +describe("config load with a scalar root", () => { + it.each([ + { name: "null", raw: "null\n" }, + { name: "number", raw: "42\n" }, + { name: "string", raw: '"oops"\n' }, + ])("rejects a $name root as INVALID_CONFIG instead of loading defaults", async ({ raw }) => { + await withTempHome(async (home) => { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile(configPath, raw, "utf-8"); + const logger = { error: vi.fn(), warn: vi.fn() }; + const io = createConfigIO({ + configPath, + env: { HOME: home } as NodeJS.ProcessEnv, + homedir: () => home, + logger, + pluginValidation: "skip", + }); + + let thrown: unknown; + try { + io.loadConfig(); + } catch (err) { + thrown = err; + } + expect(thrown, "scalar config root must not load as defaults").toBeDefined(); + expect(isInvalidConfigError(thrown)).toBe(true); + + // The snapshot must agree: the file exists and is invalid, so the + // clobber-recovery machinery never records it as lastKnownGood. + const snapshot = await io.readConfigFileSnapshot(); + expect(snapshot.exists).toBe(true); + expect(snapshot.valid).toBe(false); + }); + }); +});