From 5f8f58ae25e2a78f31b06edcf26532d634ca554e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 19:38:40 +0000 Subject: [PATCH] fix(gateway): require admin for chat config writes --- CHANGELOG.md | 1 + docs/gateway/configuration-reference.md | 2 +- docs/gateway/protocol.md | 4 ++ src/auto-reply/reply/command-gates.ts | 25 ++++++++++ src/auto-reply/reply/commands-approve.ts | 26 ++++------- src/auto-reply/reply/commands-config.ts | 14 +++++- src/auto-reply/reply/commands.test.ts | 59 ++++++++++++++++++++++++ 7 files changed, 112 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f35c5328152..6559fe86225 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -139,6 +139,7 @@ Docs: https://docs.openclaw.ai - Docs/security threat-model links: replace relative `.md` links with Mintlify-compatible root-relative routes in security docs to prevent broken internal navigation. (#27698) thanks @clawdoo. - Plugins/Update integrity drift: avoid false integrity drift prompts when updating npm-installed plugins from unpinned specs, while keeping drift checks for exact pinned versions. (#37179) Thanks @vincentkoc. - iOS/Voice timing safety: guard system speech start/finish callbacks to the active utterance to avoid misattributed start events during rapid stop/restart cycles. (#33304) thanks @mbelinky; original implementation direction by @ngutman. +- Gateway/chat.send command scopes: require `operator.admin` for persistent `/config set|unset` writes routed through gateway chat clients while keeping `/config show` available to normal write-scoped operator clients, preserving messaging-channel config command behavior without widening RPC write scope into admin config mutation. Thanks @tdjackey for reporting. - iOS/Talk incremental speech pacing: allow long punctuation-free assistant chunks to start speaking at safe whitespace boundaries so voice responses begin sooner instead of waiting for terminal punctuation. (#33305) thanks @mbelinky; original implementation by @ngutman. - iOS/Watch reply reliability: make watch session activation waiters robust under concurrent requests so status/send calls no longer hang intermittently, and align delegate callbacks with Swift 6 actor safety. (#33306) thanks @mbelinky; original implementation by @Rocuts. - Docs/tool-loop detection config keys: align `docs/tools/loop-detection.md` examples and field names with the current `tools.loopDetection` schema to prevent copy-paste validation failures from outdated keys. (#33182) Thanks @Mylszd. diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index 249c35b7309..e39d7777599 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -745,7 +745,7 @@ Include your own number in `allowFrom` to enable self-chat mode (ignores native - Override per channel: `channels.discord.commands.native` (bool or `"auto"`). `false` clears previously registered commands. - `channels.telegram.customCommands` adds extra Telegram bot menu entries. - `bash: true` enables `! ` for host shell. Requires `tools.elevated.enabled` and sender in `tools.elevated.allowFrom.`. -- `config: true` enables `/config` (reads/writes `openclaw.json`). +- `config: true` enables `/config` (reads/writes `openclaw.json`). For gateway `chat.send` clients, persistent `/config set|unset` writes also require `operator.admin`; read-only `/config show` stays available to normal write-scoped operator clients. - `channels..configWrites` gates config mutations per channel (default: true). - `allowFrom` is per-provider. When set, it is the **only** authorization source (channel allowlists/pairing and `useAccessGroups` are ignored). - `useAccessGroups: false` allows commands to bypass access-group policies when `allowFrom` is not set. diff --git a/docs/gateway/protocol.md b/docs/gateway/protocol.md index fe0ddb3f052..62a5adb1fef 100644 --- a/docs/gateway/protocol.md +++ b/docs/gateway/protocol.md @@ -149,6 +149,10 @@ Common scopes: - `operator.approvals` - `operator.pairing` +Method scope is only the first gate. Some slash commands reached through +`chat.send` apply stricter command-level checks on top. For example, persistent +`/config set` and `/config unset` writes require `operator.admin`. + ### Caps/commands/permissions (node) Nodes declare capability claims at connect time: diff --git a/src/auto-reply/reply/command-gates.ts b/src/auto-reply/reply/command-gates.ts index 721d9c1e261..49cf21c6861 100644 --- a/src/auto-reply/reply/command-gates.ts +++ b/src/auto-reply/reply/command-gates.ts @@ -1,6 +1,7 @@ import type { CommandFlagKey } from "../../config/commands.js"; import { isCommandFlagEnabled } from "../../config/commands.js"; import { logVerbose } from "../../globals.js"; +import { isInternalMessageChannel } from "../../utils/message-channel.js"; import type { ReplyPayload } from "../types.js"; import type { CommandHandlerResult, HandleCommandsParams } from "./commands-types.js"; @@ -17,6 +18,30 @@ export function rejectUnauthorizedCommand( return { shouldContinue: false }; } +export function requireGatewayClientScopeForInternalChannel( + params: HandleCommandsParams, + config: { + label: string; + allowedScopes: string[]; + missingText: string; + }, +): CommandHandlerResult | null { + if (!isInternalMessageChannel(params.command.channel)) { + return null; + } + const scopes = params.ctx.GatewayClientScopes ?? []; + if (config.allowedScopes.some((scope) => scopes.includes(scope))) { + return null; + } + logVerbose( + `Ignoring ${config.label} from gateway client missing scope: ${config.allowedScopes.join(" or ")}`, + ); + return { + shouldContinue: false, + reply: { text: config.missingText }, + }; +} + export function buildDisabledCommandReply(params: { label: string; configKey: CommandFlagKey; diff --git a/src/auto-reply/reply/commands-approve.ts b/src/auto-reply/reply/commands-approve.ts index 42e5b30a341..9773ba03ad5 100644 --- a/src/auto-reply/reply/commands-approve.ts +++ b/src/auto-reply/reply/commands-approve.ts @@ -1,10 +1,7 @@ import { callGateway } from "../../gateway/call.js"; import { logVerbose } from "../../globals.js"; -import { - GATEWAY_CLIENT_MODES, - GATEWAY_CLIENT_NAMES, - isInternalMessageChannel, -} from "../../utils/message-channel.js"; +import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../../utils/message-channel.js"; +import { requireGatewayClientScopeForInternalChannel } from "./command-gates.js"; import type { CommandHandler } from "./commands-types.js"; const COMMAND = "/approve"; @@ -86,18 +83,13 @@ export const handleApproveCommand: CommandHandler = async (params, allowTextComm return { shouldContinue: false, reply: { text: parsed.error } }; } - if (isInternalMessageChannel(params.command.channel)) { - const scopes = params.ctx.GatewayClientScopes ?? []; - const hasApprovals = scopes.includes("operator.approvals") || scopes.includes("operator.admin"); - if (!hasApprovals) { - logVerbose("Ignoring /approve from gateway client missing operator.approvals."); - return { - shouldContinue: false, - reply: { - text: "❌ /approve requires operator.approvals for gateway clients.", - }, - }; - } + const missingScope = requireGatewayClientScopeForInternalChannel(params, { + label: "/approve", + allowedScopes: ["operator.approvals", "operator.admin"], + missingText: "❌ /approve requires operator.approvals for gateway clients.", + }); + if (missingScope) { + return missingScope; } const resolvedBy = buildResolvedByLabel(params); diff --git a/src/auto-reply/reply/commands-config.ts b/src/auto-reply/reply/commands-config.ts index e8d04b160db..00ef8048efe 100644 --- a/src/auto-reply/reply/commands-config.ts +++ b/src/auto-reply/reply/commands-config.ts @@ -17,7 +17,11 @@ import { setConfigOverride, unsetConfigOverride, } from "../../config/runtime-overrides.js"; -import { rejectUnauthorizedCommand, requireCommandFlagEnabled } from "./command-gates.js"; +import { + rejectUnauthorizedCommand, + requireCommandFlagEnabled, + requireGatewayClientScopeForInternalChannel, +} from "./command-gates.js"; import type { CommandHandler } from "./commands-types.js"; import { parseConfigCommand } from "./config-commands.js"; import { parseDebugCommand } from "./debug-commands.js"; @@ -49,6 +53,14 @@ export const handleConfigCommand: CommandHandler = async (params, allowTextComma } if (configCommand.action === "set" || configCommand.action === "unset") { + const missingAdminScope = requireGatewayClientScopeForInternalChannel(params, { + label: "/config write", + allowedScopes: ["operator.admin"], + missingText: "❌ /config set|unset requires operator.admin for gateway clients.", + }); + if (missingAdminScope) { + return missingAdminScope; + } const channelId = params.command.channelId ?? normalizeChannelId(params.command.channel); const allowWrites = resolveChannelConfigWrites({ cfg: params.cfg, diff --git a/src/auto-reply/reply/commands.test.ts b/src/auto-reply/reply/commands.test.ts index 4d31b56a605..2c05690ebd0 100644 --- a/src/auto-reply/reply/commands.test.ts +++ b/src/auto-reply/reply/commands.test.ts @@ -13,6 +13,7 @@ import { updateSessionStore, type SessionEntry } from "../../config/sessions.js" import * as internalHooks from "../../hooks/internal-hooks.js"; import { clearPluginCommands, registerPluginCommand } from "../../plugins/commands.js"; import { typedCases } from "../../test-utils/typed-cases.js"; +import { INTERNAL_MESSAGE_CHANNEL } from "../../utils/message-channel.js"; import type { MsgContext } from "../templating.js"; import { resetBashChatCommandForTests } from "./bash-command.js"; import { handleCompactCommand } from "./commands-compact.js"; @@ -590,6 +591,64 @@ describe("handleCommands /config configWrites gating", () => { expect(result.shouldContinue).toBe(false); expect(result.reply?.text).toContain("Config writes are disabled"); }); + + it("blocks /config set from gateway clients without operator.admin", async () => { + const cfg = { + commands: { config: true, text: true }, + } as OpenClawConfig; + const params = buildParams('/config set messages.ackReaction=":)"', cfg, { + Provider: INTERNAL_MESSAGE_CHANNEL, + Surface: INTERNAL_MESSAGE_CHANNEL, + GatewayClientScopes: ["operator.write"], + }); + params.command.channel = INTERNAL_MESSAGE_CHANNEL; + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain("requires operator.admin"); + }); + + it("keeps /config show available to gateway operator.write clients", async () => { + const cfg = { + commands: { config: true, text: true }, + } as OpenClawConfig; + readConfigFileSnapshotMock.mockResolvedValueOnce({ + valid: true, + parsed: { messages: { ackreaction: ":)" } }, + }); + const params = buildParams("/config show messages.ackReaction", cfg, { + Provider: INTERNAL_MESSAGE_CHANNEL, + Surface: INTERNAL_MESSAGE_CHANNEL, + GatewayClientScopes: ["operator.write"], + }); + params.command.channel = INTERNAL_MESSAGE_CHANNEL; + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain("Config messages.ackreaction"); + }); + + it("keeps /config set working for gateway operator.admin clients", async () => { + const cfg = { + commands: { config: true, text: true }, + } as OpenClawConfig; + readConfigFileSnapshotMock.mockResolvedValueOnce({ + valid: true, + parsed: { messages: { ackReaction: ":)" } }, + }); + validateConfigObjectWithPluginsMock.mockImplementation((config: unknown) => ({ + ok: true, + config, + })); + const params = buildParams('/config set messages.ackReaction=":D"', cfg, { + Provider: INTERNAL_MESSAGE_CHANNEL, + Surface: INTERNAL_MESSAGE_CHANNEL, + GatewayClientScopes: ["operator.write", "operator.admin"], + }); + params.command.channel = INTERNAL_MESSAGE_CHANNEL; + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(writeConfigFileMock).toHaveBeenCalledOnce(); + expect(result.reply?.text).toContain("Config updated"); + }); }); describe("handleCommands bash alias", () => {