From f53e10e3fd753d91fe1858d46511586458402f01 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 22:38:48 +0000 Subject: [PATCH] fix(config): fail closed on invalid config load (#9040, thanks @joetomasone) Land #9040 by @joetomasone. Add fail-closed config loading, compat coverage, and changelog entry for #5052. Co-authored-by: Joe Tomasone --- CHANGELOG.md | 1 + src/config/io.compat.test.ts | 4 +- src/config/io.ts | 5 +- src/config/io.validation-fails-closed.test.ts | 57 +++++++++++++++++++ 4 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 src/config/io.validation-fails-closed.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 58ef4307a30..4d77ffc4380 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Security/Config: fail closed when `loadConfig()` hits validation or read errors so invalid configs cannot silently fall back to permissive runtime defaults. (#9040) Thanks @joetomasone. - LINE/`requireMention` group gating: align inbound and reply-stage LINE group policy resolution across raw, `group:`, and `room:` keys (including account-scoped group config), preserve plugin-backed reply-stage fallback behavior, and add regression coverage for prefixed-only group/room config plus reply-stage policy resolution. (#35847) Thanks @kirisame-wang. - Onboarding/local setup: default unset local `tools.profile` to `coding` instead of `messaging`, restoring file/runtime tools for fresh local installs while preserving explicit user-set profiles. (from #38241, overlap with #34958) Thanks @cgdusek. - Gateway/Telegram stale-socket restart guard: only apply stale-socket restarts to channels that publish event-liveness timestamps, preventing Telegram providers from being misclassified as stale solely due to long uptime and avoiding restart/pairing storms after upgrade. (openclaw#38464) diff --git a/src/config/io.compat.test.ts b/src/config/io.compat.test.ts index f8cf21ea43b..b6a2f0ffcfc 100644 --- a/src/config/io.compat.test.ts +++ b/src/config/io.compat.test.ts @@ -138,7 +138,7 @@ describe("config io paths", () => { }); }); - it("logs invalid config path details and returns empty config", async () => { + it("logs invalid config path details and throws on invalid config", async () => { await withTempHome(async (home) => { const configDir = path.join(home, ".openclaw"); await fs.mkdir(configDir, { recursive: true }); @@ -159,7 +159,7 @@ describe("config io paths", () => { logger, }); - expect(io.loadConfig()).toEqual({}); + expect(() => io.loadConfig()).toThrow("Invalid config"); expect(logger.error).toHaveBeenCalledWith( expect.stringContaining(`Invalid config at ${configPath}:\\n`), ); diff --git a/src/config/io.ts b/src/config/io.ts index a7d486a77b8..f65e70d7a81 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -831,10 +831,11 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { } const error = err as { code?: string }; if (error?.code === "INVALID_CONFIG") { - return {}; + // Fail closed so invalid configs cannot silently fall back to permissive defaults. + throw err; } deps.logger.error(`Failed to read config at ${configPath}`, err); - return {}; + throw err; } } diff --git a/src/config/io.validation-fails-closed.test.ts b/src/config/io.validation-fails-closed.test.ts new file mode 100644 index 00000000000..efcb2b7378e --- /dev/null +++ b/src/config/io.validation-fails-closed.test.ts @@ -0,0 +1,57 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { clearConfigCache, loadConfig } from "./config.js"; +import { withTempHomeConfig } from "./test-helpers.js"; + +describe("config validation fail-closed behavior", () => { + beforeEach(() => { + clearConfigCache(); + vi.restoreAllMocks(); + }); + + it("throws INVALID_CONFIG instead of returning an empty config", async () => { + await withTempHomeConfig( + { + agents: { list: [{ id: "main" }] }, + nope: true, + channels: { + whatsapp: { + dmPolicy: "allowlist", + allowFrom: ["+1234567890"], + }, + }, + }, + async () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + let thrown: unknown; + try { + loadConfig(); + } catch (err) { + thrown = err; + } + + expect(thrown).toBeInstanceOf(Error); + expect((thrown as { code?: string } | undefined)?.code).toBe("INVALID_CONFIG"); + expect(spy).toHaveBeenCalled(); + }, + ); + }); + + it("still loads valid security settings unchanged", async () => { + await withTempHomeConfig( + { + agents: { list: [{ id: "main" }] }, + channels: { + whatsapp: { + dmPolicy: "allowlist", + allowFrom: ["+1234567890"], + }, + }, + }, + async () => { + const cfg = loadConfig(); + expect(cfg.channels?.whatsapp?.dmPolicy).toBe("allowlist"); + expect(cfg.channels?.whatsapp?.allowFrom).toEqual(["+1234567890"]); + }, + ); + }); +});