From cae4e4a53f5fc0a1220a6881567f3af5f0e9005e Mon Sep 17 00:00:00 2001 From: autodev Date: Tue, 30 Jun 2026 13:35:56 +0800 Subject: [PATCH] fix(memos-local-openclaw): prevent openclaw.json corruption when adding group:plugins The plugin used a non-global regex replace on the raw JSON text to append "group:plugins" to tools.allow. When models.providers.*.models[*].input appeared before tools.allow in the file (the default OpenClaw layout) and ended with the same JSON literal as the last tools.allow entry, the patch matched the first occurrence and corrupted models.*.input, e.g. "input": ["text", "image", "group:plugins"] which OpenClaw then rejects with "models.providers..models.0.input.2: Invalid input (allowed: \"text\", \"image\")". Replace the textual patch with a small parse-then-serialise helper (src/openclaw-config.ts :: ensureToolsAllowEntry) that only ever mutates tools.allow. Cover it with 8 unit tests including a direct regression for the input/allow shape in issue #1377. Fixes #1377 --- apps/memos-local-openclaw/index.ts | 17 +-- .../src/openclaw-config.ts | 69 +++++++++++ .../tests/openclaw-config.test.ts | 115 ++++++++++++++++++ 3 files changed, 189 insertions(+), 12 deletions(-) create mode 100644 apps/memos-local-openclaw/src/openclaw-config.ts create mode 100644 apps/memos-local-openclaw/tests/openclaw-config.test.ts diff --git a/apps/memos-local-openclaw/index.ts b/apps/memos-local-openclaw/index.ts index 2f46052ee..bf12004e3 100644 --- a/apps/memos-local-openclaw/index.ts +++ b/apps/memos-local-openclaw/index.ts @@ -31,6 +31,7 @@ import { SkillInstaller } from "./src/skill/installer"; import { Summarizer } from "./src/ingest/providers"; import { MEMORY_GUIDE_SKILL_MD } from "./src/skill/bundled-memory-guide"; import { Telemetry } from "./src/telemetry"; +import { ensureToolsAllowEntry } from "./src/openclaw-config"; /** Remove near-duplicate hits based on summary word overlap (>70%). Keeps first (highest-scored) hit. */ @@ -356,18 +357,10 @@ const memosLocalPlugin = { const openclawJsonPath = path.join(stateDir, "openclaw.json"); if (fs.existsSync(openclawJsonPath)) { const raw = fs.readFileSync(openclawJsonPath, "utf-8"); - const cfg = JSON.parse(raw); - const allow: string[] | undefined = cfg?.tools?.allow; - if (Array.isArray(allow) && allow.length > 0 && !allow.includes("group:plugins") && !allow.includes("*")) { - const lastEntry = JSON.stringify(allow[allow.length - 1]); - const patched = raw.replace( - new RegExp(`(${lastEntry})(\\s*\\])`), - `$1,\n "group:plugins"$2`, - ); - if (patched !== raw && patched.includes("group:plugins")) { - fs.writeFileSync(openclawJsonPath, patched, "utf-8"); - ctx.log.info("memos-local: added 'group:plugins' to tools.allow in openclaw.json"); - } + const patched = ensureToolsAllowEntry(raw, "group:plugins"); + if (patched !== raw) { + fs.writeFileSync(openclawJsonPath, patched, "utf-8"); + ctx.log.info("memos-local: added 'group:plugins' to tools.allow in openclaw.json"); } } } catch (e) { diff --git a/apps/memos-local-openclaw/src/openclaw-config.ts b/apps/memos-local-openclaw/src/openclaw-config.ts new file mode 100644 index 000000000..1725d09a8 --- /dev/null +++ b/apps/memos-local-openclaw/src/openclaw-config.ts @@ -0,0 +1,69 @@ +/** + * Helpers for safely mutating ~/.openclaw/openclaw.json from the plugin. + * + * Previously this lived inline in index.ts and used a hand-written regex + * on the raw JSON text. That approach corrupted the config when the same + * literal that ended `tools.allow` also appeared elsewhere in the file + * (e.g. inside `models.providers.*.models[*].input`). See issue #1377: + * memos-local-openclaw-plugin corrupts openclaw.json by inserting + * "group:plugins" into models[*].input. + * + * The fixed implementation parses the JSON, mutates the parsed object, + * and re-serialises it. That guarantees the new entry can only land in + * tools.allow. + */ + +/** Detect indent unit (2 / 4 spaces or tab) by sampling the first indented line. */ +function detectIndent(raw: string): string | number { + const match = raw.match(/\n([ \t]+)\S/); + if (!match) return 2; + const indent = match[1]; + if (indent.startsWith("\t")) return "\t"; + return indent.length; +} + +/** + * Ensure that `entry` is present in `tools.allow` inside the given raw + * openclaw.json text. Returns the (possibly updated) JSON text. + * + * Behaviour: + * - If parsing fails, the input is returned unchanged. + * - If `tools.allow` is missing, empty, contains `"*"`, or already + * contains `entry`, the input is returned unchanged (referentially + * equal to the input string) — callers can detect "no change" by + * identity comparison and skip the disk write. + * - Otherwise, the entry is appended to `tools.allow` and the result + * is re-serialised with the original indentation. The original + * trailing newline is preserved. + */ +export function ensureToolsAllowEntry(raw: string, entry: string): string { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return raw; + } + if (!parsed || typeof parsed !== "object") { + return raw; + } + const root = parsed as Record; + const tools = root.tools; + if (!tools || typeof tools !== "object") { + return raw; + } + const allow = (tools as Record).allow; + if (!Array.isArray(allow) || allow.length === 0) { + return raw; + } + if (allow.includes(entry) || allow.includes("*")) { + return raw; + } + + const next = { ...root, tools: { ...(tools as Record), allow: [...allow, entry] } }; + const indent = detectIndent(raw); + let serialised = JSON.stringify(next, null, indent as any); + if (raw.endsWith("\n") && !serialised.endsWith("\n")) { + serialised += "\n"; + } + return serialised; +} diff --git a/apps/memos-local-openclaw/tests/openclaw-config.test.ts b/apps/memos-local-openclaw/tests/openclaw-config.test.ts new file mode 100644 index 000000000..2705934dc --- /dev/null +++ b/apps/memos-local-openclaw/tests/openclaw-config.test.ts @@ -0,0 +1,115 @@ +import { describe, expect, it } from "vitest"; +import { ensureToolsAllowEntry } from "../src/openclaw-config"; + +describe("ensureToolsAllowEntry", () => { + it("adds the entry to tools.allow when missing", () => { + const raw = JSON.stringify( + { + tools: { + allow: ["file_search", "shell"], + }, + }, + null, + 2, + ); + + const patched = ensureToolsAllowEntry(raw, "group:plugins"); + expect(patched).not.toBe(raw); + + const parsed = JSON.parse(patched); + expect(parsed.tools.allow).toEqual(["file_search", "shell", "group:plugins"]); + }); + + it("does NOT modify any other array that happens to contain the same string as the last tools.allow entry", () => { + // Regression: issue #1377. The previous implementation used `raw.replace(new RegExp(`(${lastEntry})(\\s*\\])`))` + // which is non-global and matches the FIRST occurrence in the file. When models.providers.*.models[*].input + // appeared BEFORE tools.allow and ended with the same JSON value as the last tools.allow entry, it patched the + // wrong array, corrupting openclaw.json: + // "input": ["text", "image"] → "input": ["text", "image", "group:plugins"] ← BUG + // "allow": ["file_search", "image"] (was meant to be patched here) + const raw = JSON.stringify( + { + models: { + providers: { + qwen: { + models: [ + { + id: "qwen3.5-plus-search", + name: "qwen3.5-plus-search", + input: ["text", "image"], + }, + ], + }, + }, + }, + tools: { + allow: ["file_search", "image"], + }, + }, + null, + 2, + ); + + const patched = ensureToolsAllowEntry(raw, "group:plugins"); + const parsed = JSON.parse(patched); + + expect(parsed.models.providers.qwen.models[0].input).toEqual(["text", "image"]); + expect(parsed.tools.allow).toEqual(["file_search", "image", "group:plugins"]); + }); + + it("is a no-op when the entry is already present", () => { + const raw = JSON.stringify( + { + tools: { allow: ["file_search", "group:plugins"] }, + }, + null, + 2, + ); + expect(ensureToolsAllowEntry(raw, "group:plugins")).toBe(raw); + }); + + it("is a no-op when tools.allow contains a wildcard", () => { + const raw = JSON.stringify({ tools: { allow: ["*"] } }, null, 2); + expect(ensureToolsAllowEntry(raw, "group:plugins")).toBe(raw); + }); + + it("is a no-op when tools.allow is missing or empty", () => { + expect(ensureToolsAllowEntry(JSON.stringify({}), "group:plugins")).toBe(JSON.stringify({})); + const emptyAllow = JSON.stringify({ tools: { allow: [] } }, null, 2); + expect(ensureToolsAllowEntry(emptyAllow, "group:plugins")).toBe(emptyAllow); + }); + + it("preserves indentation style when rewriting", () => { + const raw = + "{\n" + + " \"tools\": {\n" + + " \"allow\": [\n" + + " \"file_search\",\n" + + " \"shell\"\n" + + " ]\n" + + " },\n" + + " \"models\": { \"providers\": {} }\n" + + "}\n"; + + const patched = ensureToolsAllowEntry(raw, "group:plugins"); + const parsed = JSON.parse(patched); + expect(parsed.tools.allow).toEqual(["file_search", "shell", "group:plugins"]); + // Detected 2-space indent should be preserved + expect(patched.split("\n").some((line) => line.startsWith(" \"allow\""))).toBe(true); + // Original trailing newline is kept + expect(patched.endsWith("\n")).toBe(true); + }); + + it("handles regex-special characters in the last tools.allow entry", () => { + // Previous regex approach used JSON.stringify(value) inline and did not escape regex metacharacters. + const raw = JSON.stringify({ tools: { allow: ["a", "weird.name+x"] } }, null, 2); + const patched = ensureToolsAllowEntry(raw, "group:plugins"); + const parsed = JSON.parse(patched); + expect(parsed.tools.allow).toEqual(["a", "weird.name+x", "group:plugins"]); + }); + + it("returns the input unchanged when JSON cannot be parsed", () => { + const raw = "not really json"; + expect(ensureToolsAllowEntry(raw, "group:plugins")).toBe(raw); + }); +});