fix: prevent config clobbering

This commit is contained in:
Peter Steinberger
2026-01-15 04:05:01 +00:00
parent bd467ff765
commit 31d3aef8d6
29 changed files with 975 additions and 380 deletions

View File

@@ -6,7 +6,12 @@ import { describe, expect, it, vi } from "vitest";
import { createClawdbotTools } from "./clawdbot-tools.js";
vi.mock("./tools/gateway.js", () => ({
callGatewayTool: vi.fn(async () => ({ ok: true })),
callGatewayTool: vi.fn(async (method: string) => {
if (method === "config.get") {
return { hash: "hash-1" };
}
return { ok: true };
}),
}));
describe("gateway tool", () => {
@@ -71,11 +76,13 @@ describe("gateway tool", () => {
raw,
});
expect(callGatewayTool).toHaveBeenCalledWith("config.get", expect.any(Object), {});
expect(callGatewayTool).toHaveBeenCalledWith(
"config.apply",
expect.any(Object),
expect.objectContaining({
raw: raw.trim(),
baseHash: "hash-1",
sessionKey: "agent:main:whatsapp:dm:+15555550123",
}),
);

View File

@@ -1,3 +1,5 @@
import crypto from "node:crypto";
import { Type } from "@sinclair/typebox";
import type { ClawdbotConfig } from "../../config/config.js";
@@ -33,6 +35,7 @@ const GatewayToolSchema = Type.Object({
timeoutMs: Type.Optional(Type.Number()),
// config.apply
raw: Type.Optional(Type.String()),
baseHash: Type.Optional(Type.String()),
// config.apply, update.run
sessionKey: Type.Optional(Type.String()),
note: Type.Optional(Type.String()),
@@ -125,6 +128,24 @@ export function createGatewayTool(opts?: {
}
if (action === "config.apply") {
const raw = readStringParam(params, "raw", { required: true });
let baseHash = readStringParam(params, "baseHash");
if (!baseHash) {
const snapshot = await callGatewayTool("config.get", gatewayOpts, {});
if (snapshot && typeof snapshot === "object") {
const hash = (snapshot as { hash?: unknown }).hash;
if (typeof hash === "string" && hash.trim()) {
baseHash = hash.trim();
} else {
const rawSnapshot = (snapshot as { raw?: unknown }).raw;
if (typeof rawSnapshot === "string") {
baseHash = crypto
.createHash("sha256")
.update(rawSnapshot)
.digest("hex");
}
}
}
}
const sessionKey =
typeof params.sessionKey === "string" && params.sessionKey.trim()
? params.sessionKey.trim()
@@ -137,6 +158,7 @@ export function createGatewayTool(opts?: {
: undefined;
const result = await callGatewayTool("config.apply", gatewayOpts, {
raw,
baseHash,
sessionKey,
note,
restartDelayMs,

View File

@@ -51,6 +51,10 @@ const SHELL_ENV_EXPECTED_KEYS = [
export type ParseConfigJson5Result = { ok: true; parsed: unknown } | { ok: false; error: string };
function hashConfigRaw(raw: string | null): string {
return crypto.createHash("sha256").update(raw ?? "").digest("hex");
}
export type ConfigIoDeps = {
fs?: typeof fs;
json5?: typeof JSON5;
@@ -263,6 +267,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
async function readConfigFileSnapshot(): Promise<ConfigFileSnapshot> {
const exists = deps.fs.existsSync(configPath);
if (!exists) {
const hash = hashConfigRaw(null);
const config = applyTalkApiKey(
applyModelDefaults(
applyContextPruningDefaults(applySessionDefaults(applyMessageDefaults({}))),
@@ -276,6 +281,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
parsed: {},
valid: true,
config,
hash,
issues: [],
legacyIssues,
};
@@ -283,6 +289,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
try {
const raw = deps.fs.readFileSync(configPath, "utf-8");
const hash = hashConfigRaw(raw);
const parsedRes = parseConfigJson5(raw, deps.json5);
if (!parsedRes.ok) {
return {
@@ -292,6 +299,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
parsed: {},
valid: false,
config: {},
hash,
issues: [{ path: "", message: `JSON5 parse failed: ${parsedRes.error}` }],
legacyIssues: [],
};
@@ -316,6 +324,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
parsed: parsedRes.parsed,
valid: false,
config: {},
hash,
issues: [{ path: "", message }],
legacyIssues: [],
};
@@ -338,6 +347,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
parsed: parsedRes.parsed,
valid: false,
config: resolvedConfig,
hash,
issues: validated.issues,
legacyIssues,
};
@@ -363,6 +373,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
),
),
),
hash,
issues: [],
legacyIssues,
};
@@ -374,6 +385,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
parsed: {},
valid: false,
config: {},
hash: hashConfigRaw(null),
issues: [{ path: "", message: `read failed: ${String(err)}` }],
legacyIssues: [],
};

28
src/config/merge-patch.ts Normal file
View File

@@ -0,0 +1,28 @@
type PlainObject = Record<string, unknown>;
function isPlainObject(value: unknown): value is PlainObject {
return typeof value === "object" && value !== null && !Array.isArray(value);
}
export function applyMergePatch(base: unknown, patch: unknown): unknown {
if (!isPlainObject(patch)) {
return patch;
}
const result: PlainObject = isPlainObject(base) ? { ...base } : {};
for (const [key, value] of Object.entries(patch)) {
if (value === null) {
delete result[key];
continue;
}
if (isPlainObject(value)) {
const baseValue = result[key];
result[key] = applyMergePatch(isPlainObject(baseValue) ? baseValue : {}, value);
continue;
}
result[key] = value;
}
return result;
}

View File

@@ -93,6 +93,7 @@ export type ConfigFileSnapshot = {
parsed: unknown;
valid: boolean;
config: ClawdbotConfig;
hash?: string;
issues: ConfigValidationIssue[];
legacyIssues: LegacyConfigIssue[];
};

View File

@@ -27,6 +27,8 @@ import {
ConfigApplyParamsSchema,
type ConfigGetParams,
ConfigGetParamsSchema,
type ConfigPatchParams,
ConfigPatchParamsSchema,
type ConfigSchemaParams,
ConfigSchemaParamsSchema,
type ConfigSchemaResponse,
@@ -201,6 +203,7 @@ export const validateSessionsCompactParams = ajv.compile<SessionsCompactParams>(
export const validateConfigGetParams = ajv.compile<ConfigGetParams>(ConfigGetParamsSchema);
export const validateConfigSetParams = ajv.compile<ConfigSetParams>(ConfigSetParamsSchema);
export const validateConfigApplyParams = ajv.compile<ConfigApplyParams>(ConfigApplyParamsSchema);
export const validateConfigPatchParams = ajv.compile<ConfigPatchParams>(ConfigPatchParamsSchema);
export const validateConfigSchemaParams = ajv.compile<ConfigSchemaParams>(ConfigSchemaParamsSchema);
export const validateWizardStartParams = ajv.compile<WizardStartParams>(WizardStartParamsSchema);
export const validateWizardNextParams = ajv.compile<WizardNextParams>(WizardNextParamsSchema);
@@ -272,6 +275,7 @@ export {
ConfigGetParamsSchema,
ConfigSetParamsSchema,
ConfigApplyParamsSchema,
ConfigPatchParamsSchema,
ConfigSchemaParamsSchema,
ConfigSchemaResponseSchema,
WizardStartParamsSchema,
@@ -338,6 +342,7 @@ export type {
ConfigGetParams,
ConfigSetParams,
ConfigApplyParams,
ConfigPatchParams,
ConfigSchemaParams,
ConfigSchemaResponse,
WizardStartParams,

View File

@@ -7,6 +7,7 @@ export const ConfigGetParamsSchema = Type.Object({}, { additionalProperties: fal
export const ConfigSetParamsSchema = Type.Object(
{
raw: NonEmptyString,
baseHash: Type.Optional(NonEmptyString),
},
{ additionalProperties: false },
);
@@ -14,6 +15,7 @@ export const ConfigSetParamsSchema = Type.Object(
export const ConfigApplyParamsSchema = Type.Object(
{
raw: NonEmptyString,
baseHash: Type.Optional(NonEmptyString),
sessionKey: Type.Optional(Type.String()),
note: Type.Optional(Type.String()),
restartDelayMs: Type.Optional(Type.Integer({ minimum: 0 })),
@@ -21,6 +23,14 @@ export const ConfigApplyParamsSchema = Type.Object(
{ additionalProperties: false },
);
export const ConfigPatchParamsSchema = Type.Object(
{
raw: NonEmptyString,
baseHash: Type.Optional(NonEmptyString),
},
{ additionalProperties: false },
);
export const ConfigSchemaParamsSchema = Type.Object({}, { additionalProperties: false });
export const UpdateRunParamsSchema = Type.Object(

View File

@@ -30,6 +30,7 @@ import {
import {
ConfigApplyParamsSchema,
ConfigGetParamsSchema,
ConfigPatchParamsSchema,
ConfigSchemaParamsSchema,
ConfigSchemaResponseSchema,
ConfigSetParamsSchema,
@@ -131,6 +132,7 @@ export const ProtocolSchemas: Record<string, TSchema> = {
ConfigGetParams: ConfigGetParamsSchema,
ConfigSetParams: ConfigSetParamsSchema,
ConfigApplyParams: ConfigApplyParamsSchema,
ConfigPatchParams: ConfigPatchParamsSchema,
ConfigSchemaParams: ConfigSchemaParamsSchema,
ConfigSchemaResponse: ConfigSchemaResponseSchema,
WizardStartParams: WizardStartParamsSchema,

View File

@@ -28,6 +28,7 @@ import type {
import type {
ConfigApplyParamsSchema,
ConfigGetParamsSchema,
ConfigPatchParamsSchema,
ConfigSchemaParamsSchema,
ConfigSchemaResponseSchema,
ConfigSetParamsSchema,
@@ -124,6 +125,7 @@ export type SessionsCompactParams = Static<typeof SessionsCompactParamsSchema>;
export type ConfigGetParams = Static<typeof ConfigGetParamsSchema>;
export type ConfigSetParams = Static<typeof ConfigSetParamsSchema>;
export type ConfigApplyParams = Static<typeof ConfigApplyParamsSchema>;
export type ConfigPatchParams = Static<typeof ConfigPatchParamsSchema>;
export type ConfigSchemaParams = Static<typeof ConfigSchemaParamsSchema>;
export type ConfigSchemaResponse = Static<typeof ConfigSchemaResponseSchema>;
export type WizardStartParams = Static<typeof WizardStartParamsSchema>;

View File

@@ -7,17 +7,63 @@ import {
validateConfigObject,
writeConfigFile,
} from "../config/config.js";
import { applyLegacyMigrations } from "../config/legacy.js";
import { applyMergePatch } from "../config/merge-patch.js";
import { buildConfigSchema } from "../config/schema.js";
import { loadClawdbotPlugins } from "../plugins/loader.js";
import {
ErrorCodes,
formatValidationErrors,
validateConfigGetParams,
validateConfigPatchParams,
validateConfigSchemaParams,
validateConfigSetParams,
} from "./protocol/index.js";
import type { BridgeMethodHandler } from "./server-bridge-types.js";
function resolveBaseHash(params: unknown): string | null {
const raw = (params as { baseHash?: unknown })?.baseHash;
if (typeof raw !== "string") return null;
const trimmed = raw.trim();
return trimmed ? trimmed : null;
}
function requireConfigBaseHash(
params: unknown,
snapshot: Awaited<ReturnType<typeof readConfigFileSnapshot>>,
): { ok: true } | { ok: false; error: { code: string; message: string } } {
if (!snapshot.exists) return { ok: true };
if (typeof snapshot.raw !== "string" || !snapshot.hash) {
return {
ok: false,
error: {
code: ErrorCodes.INVALID_REQUEST,
message: "config base hash unavailable; re-run config.get and retry",
},
};
}
const baseHash = resolveBaseHash(params);
if (!baseHash) {
return {
ok: false,
error: {
code: ErrorCodes.INVALID_REQUEST,
message: "config base hash required; re-run config.get and retry",
},
};
}
if (baseHash !== snapshot.hash) {
return {
ok: false,
error: {
code: ErrorCodes.INVALID_REQUEST,
message: "config changed since last load; re-run config.get and retry",
},
};
}
return { ok: true };
}
export const handleConfigBridgeMethods: BridgeMethodHandler = async (
_ctx,
_nodeId,
@@ -85,6 +131,11 @@ export const handleConfigBridgeMethods: BridgeMethodHandler = async (
},
};
}
const snapshot = await readConfigFileSnapshot();
const guard = requireConfigBaseHash(params, snapshot);
if (!guard.ok) {
return { ok: false, error: guard.error };
}
const rawValue = (params as { raw?: unknown }).raw;
if (typeof rawValue !== "string") {
return {
@@ -126,6 +177,87 @@ export const handleConfigBridgeMethods: BridgeMethodHandler = async (
}),
};
}
case "config.patch": {
if (!validateConfigPatchParams(params)) {
return {
ok: false,
error: {
code: ErrorCodes.INVALID_REQUEST,
message: `invalid config.patch params: ${formatValidationErrors(validateConfigPatchParams.errors)}`,
},
};
}
const snapshot = await readConfigFileSnapshot();
const guard = requireConfigBaseHash(params, snapshot);
if (!guard.ok) {
return { ok: false, error: guard.error };
}
if (!snapshot.valid) {
return {
ok: false,
error: {
code: ErrorCodes.INVALID_REQUEST,
message: "invalid config; fix before patching",
},
};
}
const rawValue = (params as { raw?: unknown }).raw;
if (typeof rawValue !== "string") {
return {
ok: false,
error: {
code: ErrorCodes.INVALID_REQUEST,
message: "invalid config.patch params: raw (string) required",
},
};
}
const parsedRes = parseConfigJson5(rawValue);
if (!parsedRes.ok) {
return {
ok: false,
error: {
code: ErrorCodes.INVALID_REQUEST,
message: parsedRes.error,
},
};
}
if (
!parsedRes.parsed ||
typeof parsedRes.parsed !== "object" ||
Array.isArray(parsedRes.parsed)
) {
return {
ok: false,
error: {
code: ErrorCodes.INVALID_REQUEST,
message: "config.patch raw must be an object",
},
};
}
const merged = applyMergePatch(snapshot.config, parsedRes.parsed);
const migrated = applyLegacyMigrations(merged);
const resolved = migrated.next ?? merged;
const validated = validateConfigObject(resolved);
if (!validated.ok) {
return {
ok: false,
error: {
code: ErrorCodes.INVALID_REQUEST,
message: "invalid config",
details: { issues: validated.issues },
},
};
}
await writeConfigFile(validated.config);
return {
ok: true,
payloadJSON: JSON.stringify({
ok: true,
path: CONFIG_PATH_CLAWDBOT,
config: validated.config,
}),
};
}
default:
return null;
}

View File

@@ -10,6 +10,7 @@ const BASE_METHODS = [
"config.get",
"config.set",
"config.apply",
"config.patch",
"config.schema",
"wizard.start",
"wizard.next",

View File

@@ -7,6 +7,8 @@ import {
validateConfigObject,
writeConfigFile,
} from "../../config/config.js";
import { applyLegacyMigrations } from "../../config/legacy.js";
import { applyMergePatch } from "../../config/merge-patch.js";
import { buildConfigSchema } from "../../config/schema.js";
import { scheduleGatewaySigusr1Restart } from "../../infra/restart.js";
import {
@@ -21,11 +23,62 @@ import {
formatValidationErrors,
validateConfigApplyParams,
validateConfigGetParams,
validateConfigPatchParams,
validateConfigSchemaParams,
validateConfigSetParams,
} from "../protocol/index.js";
import type { GatewayRequestHandlers } from "./types.js";
function resolveBaseHash(params: unknown): string | null {
const raw = (params as { baseHash?: unknown })?.baseHash;
if (typeof raw !== "string") return null;
const trimmed = raw.trim();
return trimmed ? trimmed : null;
}
function requireConfigBaseHash(
params: unknown,
snapshot: Awaited<ReturnType<typeof readConfigFileSnapshot>>,
respond: (ok: boolean, payload?: unknown, error?: unknown) => void,
): boolean {
if (!snapshot.exists) return true;
if (typeof snapshot.raw !== "string" || !snapshot.hash) {
respond(
false,
undefined,
errorShape(
ErrorCodes.INVALID_REQUEST,
"config base hash unavailable; re-run config.get and retry",
),
);
return false;
}
const baseHash = resolveBaseHash(params);
if (!baseHash) {
respond(
false,
undefined,
errorShape(
ErrorCodes.INVALID_REQUEST,
"config base hash required; re-run config.get and retry",
),
);
return false;
}
if (baseHash !== snapshot.hash) {
respond(
false,
undefined,
errorShape(
ErrorCodes.INVALID_REQUEST,
"config changed since last load; re-run config.get and retry",
),
);
return false;
}
return true;
}
export const configHandlers: GatewayRequestHandlers = {
"config.get": async ({ params, respond }) => {
if (!validateConfigGetParams(params)) {
@@ -93,6 +146,10 @@ export const configHandlers: GatewayRequestHandlers = {
);
return;
}
const snapshot = await readConfigFileSnapshot();
if (!requireConfigBaseHash(params, snapshot, respond)) {
return;
}
const rawValue = (params as { raw?: unknown }).raw;
if (typeof rawValue !== "string") {
respond(
@@ -129,6 +186,80 @@ export const configHandlers: GatewayRequestHandlers = {
undefined,
);
},
"config.patch": async ({ params, respond }) => {
if (!validateConfigPatchParams(params)) {
respond(
false,
undefined,
errorShape(
ErrorCodes.INVALID_REQUEST,
`invalid config.patch params: ${formatValidationErrors(validateConfigPatchParams.errors)}`,
),
);
return;
}
const snapshot = await readConfigFileSnapshot();
if (!requireConfigBaseHash(params, snapshot, respond)) {
return;
}
if (!snapshot.valid) {
respond(
false,
undefined,
errorShape(ErrorCodes.INVALID_REQUEST, "invalid config; fix before patching"),
);
return;
}
const rawValue = (params as { raw?: unknown }).raw;
if (typeof rawValue !== "string") {
respond(
false,
undefined,
errorShape(
ErrorCodes.INVALID_REQUEST,
"invalid config.patch params: raw (string) required",
),
);
return;
}
const parsedRes = parseConfigJson5(rawValue);
if (!parsedRes.ok) {
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, parsedRes.error));
return;
}
if (!parsedRes.parsed || typeof parsedRes.parsed !== "object" || Array.isArray(parsedRes.parsed)) {
respond(
false,
undefined,
errorShape(ErrorCodes.INVALID_REQUEST, "config.patch raw must be an object"),
);
return;
}
const merged = applyMergePatch(snapshot.config, parsedRes.parsed);
const migrated = applyLegacyMigrations(merged);
const resolved = migrated.next ?? merged;
const validated = validateConfigObject(resolved);
if (!validated.ok) {
respond(
false,
undefined,
errorShape(ErrorCodes.INVALID_REQUEST, "invalid config", {
details: { issues: validated.issues },
}),
);
return;
}
await writeConfigFile(validated.config);
respond(
true,
{
ok: true,
path: CONFIG_PATH_CLAWDBOT,
config: validated.config,
},
undefined,
);
},
"config.apply": async ({ params, respond }) => {
if (!validateConfigApplyParams(params)) {
respond(
@@ -141,6 +272,10 @@ export const configHandlers: GatewayRequestHandlers = {
);
return;
}
const snapshot = await readConfigFileSnapshot();
if (!requireConfigBaseHash(params, snapshot, respond)) {
return;
}
const rawValue = (params as { raw?: unknown }).raw;
if (typeof rawValue !== "string") {
respond(

View File

@@ -0,0 +1,176 @@
import { describe, expect, it } from "vitest";
import { connectOk, onceMessage, startServerWithClient } from "./test-helpers.js";
describe("gateway config.patch", () => {
it("merges patches without clobbering unrelated config", async () => {
const { server, ws } = await startServerWithClient();
await connectOk(ws);
const setId = "req-set";
ws.send(
JSON.stringify({
type: "req",
id: setId,
method: "config.set",
params: {
raw: JSON.stringify({
gateway: { mode: "local" },
channels: { telegram: { botToken: "token-1" } },
}),
},
}),
);
const setRes = await onceMessage<{ ok: boolean }>(ws, (o) => o.type === "res" && o.id === setId);
expect(setRes.ok).toBe(true);
const getId = "req-get";
ws.send(
JSON.stringify({
type: "req",
id: getId,
method: "config.get",
params: {},
}),
);
const getRes = await onceMessage<{ ok: boolean; payload?: { hash?: string } }>(
ws,
(o) => o.type === "res" && o.id === getId,
);
expect(getRes.ok).toBe(true);
const baseHash = getRes.payload?.hash;
expect(typeof baseHash).toBe("string");
const patchId = "req-patch";
ws.send(
JSON.stringify({
type: "req",
id: patchId,
method: "config.patch",
params: {
raw: JSON.stringify({
channels: {
telegram: {
groups: {
"*": { requireMention: false },
},
},
},
}),
baseHash,
},
}),
);
const patchRes = await onceMessage<{ ok: boolean }>(
ws,
(o) => o.type === "res" && o.id === patchId,
);
expect(patchRes.ok).toBe(true);
const get2Id = "req-get-2";
ws.send(
JSON.stringify({
type: "req",
id: get2Id,
method: "config.get",
params: {},
}),
);
const get2Res = await onceMessage<{
ok: boolean;
payload?: { config?: { gateway?: { mode?: string }; channels?: { telegram?: { botToken?: string } } } };
}>(ws, (o) => o.type === "res" && o.id === get2Id);
expect(get2Res.ok).toBe(true);
expect(get2Res.payload?.config?.gateway?.mode).toBe("local");
expect(get2Res.payload?.config?.channels?.telegram?.botToken).toBe("token-1");
ws.close();
await server.close();
});
it("requires base hash when config exists", async () => {
const { server, ws } = await startServerWithClient();
await connectOk(ws);
const setId = "req-set-2";
ws.send(
JSON.stringify({
type: "req",
id: setId,
method: "config.set",
params: {
raw: JSON.stringify({
gateway: { mode: "local" },
}),
},
}),
);
const setRes = await onceMessage<{ ok: boolean }>(ws, (o) => o.type === "res" && o.id === setId);
expect(setRes.ok).toBe(true);
const patchId = "req-patch-2";
ws.send(
JSON.stringify({
type: "req",
id: patchId,
method: "config.patch",
params: {
raw: JSON.stringify({ gateway: { mode: "remote" } }),
},
}),
);
const patchRes = await onceMessage<{ ok: boolean; error?: { message?: string } }>(
ws,
(o) => o.type === "res" && o.id === patchId,
);
expect(patchRes.ok).toBe(false);
expect(patchRes.error?.message).toContain("base hash");
ws.close();
await server.close();
});
it("requires base hash for config.set when config exists", async () => {
const { server, ws } = await startServerWithClient();
await connectOk(ws);
const setId = "req-set-3";
ws.send(
JSON.stringify({
type: "req",
id: setId,
method: "config.set",
params: {
raw: JSON.stringify({
gateway: { mode: "local" },
}),
},
}),
);
const setRes = await onceMessage<{ ok: boolean }>(ws, (o) => o.type === "res" && o.id === setId);
expect(setRes.ok).toBe(true);
const set2Id = "req-set-4";
ws.send(
JSON.stringify({
type: "req",
id: set2Id,
method: "config.set",
params: {
raw: JSON.stringify({
gateway: { mode: "remote" },
}),
},
}),
);
const set2Res = await onceMessage<{ ok: boolean; error?: { message?: string } }>(
ws,
(o) => o.type === "res" && o.id === set2Id,
);
expect(set2Res.ok).toBe(false);
expect(set2Res.error?.message).toContain("base hash");
ws.close();
await server.close();
});
});