From ac8495adaa563735e0f8b8d1b1fb2f3257ac9b80 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 22 Apr 2026 23:53:15 +0100 Subject: [PATCH] fix(config): write through single-file includes --- CHANGELOG.md | 1 + docs/gateway/configuration-reference.md | 2 + docs/gateway/configuration.md | 6 ++ src/config/mutate.test.ts | 110 +++++++++++++++++++- src/config/mutate.ts | 127 ++++++++++++++++++++++-- 5 files changed, 237 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c29eaea4249..3cac89fc51f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Docs: https://docs.openclaw.ai - Providers/Amazon Bedrock: use known context-window metadata for discovered models while keeping the unknown-model fallback conservative, so compaction and overflow handling improve for newer Bedrock models without overstating unlisted model limits. Thanks @wirjo. - Providers/Amazon Bedrock Mantle: refresh IAM-backed bearer tokens at runtime instead of baking discovery-time tokens into provider config, so long-lived Mantle sessions keep working after the initial token ages out. Thanks @wirjo. +- Config/includes: write through single-file top-level includes for isolated OpenClaw-owned mutations, so `plugins install` and `plugins update` update an included `plugins.json5` file instead of flattening modular `$include` configs. Fixes #41050 and #66048. - Codex harness: rotate the shared app-server websocket client when the configured bearer token changes, so auth-token refreshes reconnect with the new `Authorization` header instead of reusing a stale socket. (#70328) Thanks @Lucenx9. - Telegram/sandbox: keep Telegram bot DMs on per-account sender session keys even when `session.dmScope=main`, so sandbox/tool policy can distinguish Telegram-originated direct chats from the agent main session. - Config/models: merge provider-scoped model allowlist updates and protect model/provider map writes from accidental full replacement, adding `config set --merge` for additive updates and `--replace` for intentional clobbers. Fixes #65920, #68392, and #68653. diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index f5630c63bfa..eb55aded951 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -3903,6 +3903,8 @@ Split config into multiple files: - Sibling keys: merged after includes (override included values). - Nested includes: up to 10 levels deep. - Paths: resolved relative to the including file, but must stay inside the top-level config directory (`dirname` of `openclaw.json`). Absolute/`../` forms are allowed only when they still resolve inside that boundary. +- OpenClaw-owned writes that change only one top-level section backed by a single-file include write through to that included file. For example, `plugins install` updates `plugins: { $include: "./plugins.json5" }` in `plugins.json5` and leaves `openclaw.json` intact. +- Root includes, include arrays, and includes with sibling overrides are read-only for OpenClaw-owned writes; those writes fail closed instead of flattening the config. - Errors: clear messages for missing files, parse errors, and circular includes. --- diff --git a/docs/gateway/configuration.md b/docs/gateway/configuration.md index 72a4d54c898..c28c791b624 100644 --- a/docs/gateway/configuration.md +++ b/docs/gateway/configuration.md @@ -508,6 +508,12 @@ placeholders such as `***` or shortened token values. - **Sibling keys**: merged after includes (override included values) - **Nested includes**: supported up to 10 levels deep - **Relative paths**: resolved relative to the including file + - **OpenClaw-owned writes**: when a write changes only one top-level section + backed by a single-file include such as `plugins: { $include: "./plugins.json5" }`, + OpenClaw updates that included file and leaves `openclaw.json` intact + - **Unsupported write-through**: root includes, include arrays, and includes + with sibling overrides fail closed for OpenClaw-owned writes instead of + flattening the config - **Error handling**: clear errors for missing files, parse errors, and circular includes diff --git a/src/config/mutate.test.ts b/src/config/mutate.test.ts index b9b24a09d95..d20bd9360aa 100644 --- a/src/config/mutate.test.ts +++ b/src/config/mutate.test.ts @@ -1,4 +1,7 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import fs from "node:fs/promises"; +import path from "node:path"; +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { createSuiteTempRootTracker } from "../test-helpers/temp-dir.js"; import { ConfigMutationConflictError, mutateConfigFile, replaceConfigFile } from "./mutate.js"; import type { ConfigFileSnapshot, OpenClawConfig } from "./types.js"; @@ -13,6 +16,7 @@ vi.mock("./io.js", () => ioMocks); function createSnapshot(params: { hash: string; path?: string; + parsed?: unknown; sourceConfig: OpenClawConfig; runtimeConfig?: OpenClawConfig; }): ConfigFileSnapshot { @@ -23,7 +27,7 @@ function createSnapshot(params: { path: params.path ?? "/tmp/openclaw.json", exists: true, raw: "{}", - parsed: params.sourceConfig, + parsed: params.parsed ?? params.sourceConfig, sourceConfig, resolved: sourceConfig, valid: true, @@ -37,6 +41,16 @@ function createSnapshot(params: { } describe("config mutate helpers", () => { + const suiteRootTracker = createSuiteTempRootTracker({ prefix: "openclaw-config-mutate-" }); + + beforeAll(async () => { + await suiteRootTracker.setup(); + }); + + afterAll(async () => { + await suiteRootTracker.cleanup(); + }); + beforeEach(() => { vi.clearAllMocks(); ioMocks.resolveConfigSnapshotHash.mockImplementation( @@ -122,4 +136,96 @@ describe("config mutate helpers", () => { }, ); }); + + it("writes through a single-file top-level plugins include", async () => { + const home = await suiteRootTracker.make("include"); + const configPath = path.join(home, ".openclaw", "openclaw.json"); + const pluginsPath = path.join(home, ".openclaw", "config", "plugins.json5"); + await fs.mkdir(path.dirname(pluginsPath), { recursive: true }); + await fs.writeFile( + configPath, + `${JSON.stringify({ plugins: { $include: "./config/plugins.json5" } }, null, 2)}\n`, + "utf-8", + ); + await fs.writeFile( + pluginsPath, + `${JSON.stringify({ entries: { old: { enabled: true } } }, null, 2)}\n`, + "utf-8", + ); + const snapshot = createSnapshot({ + hash: "hash-include", + path: configPath, + parsed: { plugins: { $include: "./config/plugins.json5" } }, + sourceConfig: { + plugins: { + entries: { old: { enabled: true } }, + }, + }, + }); + + await replaceConfigFile({ + baseHash: snapshot.hash, + snapshot, + writeOptions: { expectedConfigPath: snapshot.path }, + nextConfig: { + plugins: { + entries: { + old: { enabled: true }, + demo: { enabled: true }, + }, + installs: { + demo: { + source: "npm", + spec: "demo", + installPath: "/tmp/demo", + }, + }, + }, + }, + }); + + expect(ioMocks.writeConfigFile).not.toHaveBeenCalled(); + await expect(fs.readFile(configPath, "utf-8")).resolves.toContain( + '"$include": "./config/plugins.json5"', + ); + await expect(fs.readFile(`${pluginsPath}.bak`, "utf-8")).resolves.toContain('"old"'); + const persistedPlugins = JSON.parse(await fs.readFile(pluginsPath, "utf-8")) as { + entries?: Record; + installs?: Record; + }; + expect(persistedPlugins.entries?.demo).toEqual({ enabled: true }); + expect(persistedPlugins.installs?.demo).toMatchObject({ source: "npm", spec: "demo" }); + }); + + it("falls back to the root writer when a plugins include write is not isolated", async () => { + const snapshot = createSnapshot({ + hash: "hash-multi", + path: "/tmp/openclaw.json", + parsed: { plugins: { $include: "./config/plugins.json5" }, gateway: { mode: "local" } }, + sourceConfig: { + gateway: { mode: "local" }, + plugins: { entries: {} }, + }, + }); + + await replaceConfigFile({ + snapshot, + writeOptions: { expectedConfigPath: snapshot.path }, + nextConfig: { + gateway: { mode: "local", port: 18789 }, + plugins: { entries: { demo: { enabled: true } } }, + }, + }); + + expect(ioMocks.writeConfigFile).toHaveBeenCalledWith( + { + gateway: { mode: "local", port: 18789 }, + plugins: { entries: { demo: { enabled: true } } }, + }, + { + baseSnapshot: snapshot, + expectedConfigPath: snapshot.path, + }, + ); + }); }); diff --git a/src/config/mutate.ts b/src/config/mutate.ts index 45fba7e5809..30c9de03888 100644 --- a/src/config/mutate.ts +++ b/src/config/mutate.ts @@ -1,3 +1,12 @@ +import crypto from "node:crypto"; +import fs from "node:fs/promises"; +import path from "node:path"; +import { isDeepStrictEqual } from "node:util"; +import { isPathInside } from "../security/scan-paths.js"; +import { isRecord } from "../utils.js"; +import { maintainConfigBackups } from "./backup-rotation.js"; +import { INCLUDE_KEY } from "./includes.js"; +import { createInvalidConfigError, formatInvalidConfigDetails } from "./io.invalid-config.js"; import { readConfigFileSnapshotForWrite, resolveConfigSnapshotHash, @@ -5,6 +14,7 @@ import { type ConfigWriteOptions, } from "./io.js"; import type { ConfigFileSnapshot, OpenClawConfig } from "./types.js"; +import { validateConfigObjectWithPlugins } from "./validation.js"; export type ConfigMutationBase = "runtime" | "source"; @@ -35,6 +45,97 @@ function assertBaseHashMatches(snapshot: ConfigFileSnapshot, expectedHash?: stri return currentHash; } +function getChangedTopLevelKeys(base: unknown, next: unknown): string[] { + if (!isRecord(base) || !isRecord(next)) { + return isDeepStrictEqual(base, next) ? [] : [""]; + } + const keys = new Set([...Object.keys(base), ...Object.keys(next)]); + return [...keys].filter((key) => !isDeepStrictEqual(base[key], next[key])); +} + +function getSingleTopLevelIncludeTarget(params: { + snapshot: ConfigFileSnapshot; + key: string; +}): string | null { + if (!isRecord(params.snapshot.parsed)) { + return null; + } + const authoredSection = params.snapshot.parsed[params.key]; + if (!isRecord(authoredSection)) { + return null; + } + const keys = Object.keys(authoredSection); + const includeValue = authoredSection[INCLUDE_KEY]; + if (keys.length !== 1 || typeof includeValue !== "string") { + return null; + } + + const rootDir = path.dirname(params.snapshot.path); + const resolved = path.normalize( + path.isAbsolute(includeValue) ? includeValue : path.resolve(rootDir, includeValue), + ); + if (!isPathInside(rootDir, resolved)) { + return null; + } + return resolved; +} + +async function writeJsonFileAtomic(filePath: string, value: unknown): Promise { + const dir = path.dirname(filePath); + const tmp = path.join( + dir, + `${path.basename(filePath)}.${process.pid}.${crypto.randomUUID()}.tmp`, + ); + try { + await fs.mkdir(dir, { recursive: true }); + await fs.writeFile(tmp, `${JSON.stringify(value, null, 2)}\n`, { + encoding: "utf-8", + mode: 0o600, + }); + await fs.access(filePath).then( + async () => await maintainConfigBackups(filePath, fs), + () => undefined, + ); + await fs.rename(tmp, filePath); + await fs.chmod(filePath, 0o600).catch(() => { + // best-effort + }); + } catch (err) { + await fs.unlink(tmp).catch(() => { + // best-effort + }); + throw err; + } +} + +async function tryWriteSingleTopLevelIncludeMutation(params: { + snapshot: ConfigFileSnapshot; + nextConfig: OpenClawConfig; +}): Promise { + const changedKeys = getChangedTopLevelKeys(params.snapshot.sourceConfig, params.nextConfig); + if (changedKeys.length !== 1 || changedKeys[0] === "") { + return false; + } + + const key = changedKeys[0]; + const includePath = getSingleTopLevelIncludeTarget({ snapshot: params.snapshot, key }); + if (!includePath || !isRecord(params.nextConfig) || !(key in params.nextConfig)) { + return false; + } + const nextConfigRecord = params.nextConfig as Record; + + const validated = validateConfigObjectWithPlugins(params.nextConfig); + if (!validated.ok) { + throw createInvalidConfigError( + params.snapshot.path, + formatInvalidConfigDetails(validated.issues), + ); + } + + await writeJsonFileAtomic(includePath, nextConfigRecord[key]); + return true; +} + export async function replaceConfigFile(params: { nextConfig: OpenClawConfig; baseHash?: string; @@ -47,11 +148,17 @@ export async function replaceConfigFile(params: { : await readConfigFileSnapshotForWrite(); const { snapshot, writeOptions } = prepared; const previousHash = assertBaseHashMatches(snapshot, params.baseHash); - await writeConfigFile(params.nextConfig, { - baseSnapshot: snapshot, - ...writeOptions, - ...params.writeOptions, + const wroteInclude = await tryWriteSingleTopLevelIncludeMutation({ + snapshot, + nextConfig: params.nextConfig, }); + if (!wroteInclude) { + await writeConfigFile(params.nextConfig, { + baseSnapshot: snapshot, + ...writeOptions, + ...params.writeOptions, + }); + } return { path: snapshot.path, previousHash, @@ -74,10 +181,16 @@ export async function mutateConfigFile(params: { const baseConfig = params.base === "runtime" ? snapshot.runtimeConfig : snapshot.sourceConfig; const draft = structuredClone(baseConfig) as OpenClawConfig; const result = (await params.mutate(draft, { snapshot, previousHash })) as T | undefined; - await writeConfigFile(draft, { - ...writeOptions, - ...params.writeOptions, + const wroteInclude = await tryWriteSingleTopLevelIncludeMutation({ + snapshot, + nextConfig: draft, }); + if (!wroteInclude) { + await writeConfigFile(draft, { + ...writeOptions, + ...params.writeOptions, + }); + } return { path: snapshot.path, previousHash,