mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-21 18:08:05 -06:00
fix(config): reject scalar config roots instead of loading defaults (#124945)
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.
This commit is contained in:
committed by
GitHub
parent
c1d3b33aa7
commit
6575733bc0
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
+11
-25
@@ -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,
|
||||
|
||||
@@ -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<T>(run: (home: string) => Promise<T>): Promise<T> {
|
||||
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);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user