diff --git a/CHANGELOG.md b/CHANGELOG.md index f051950d860..8dcfe701e11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,7 @@ Docs: https://docs.openclaw.ai - Channels/allowlists: remove stale matcher caching so same-array allowlist edits and wildcard replacements take effect immediately, with regression coverage for in-place mutation cases. - Gateway/auth: fail closed when local `gateway.auth.*` SecretRefs are configured but unavailable, instead of silently falling back to `gateway.remote.*` credentials in local mode. Thanks @tdjackey. - Sandbox/fs bridge: pin staged writes to verified parent directories so temporary write files cannot materialize outside the allowed mount before atomic replace. Thanks @tdjackey. +- Commands/config writes: enforce `configWrites` against both the originating account and the targeted account scope for `/config` and config-backed `/allowlist` edits, blocking sibling-account mutations while preserving gateway `operator.admin` flows. Thanks @tdjackey for reporting. ## 2026.3.8 diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index 2a27470fd36..ae958788e2f 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -748,6 +748,7 @@ Include your own number in `allowFrom` to enable self-chat mode (ignores native - `bash: true` enables `! ` for host shell. Requires `tools.elevated.enabled` and sender in `tools.elevated.allowFrom.`. - `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). +- For multi-account channels, `channels..accounts..configWrites` also gates writes that target that account (for example `/allowlist --config --account ` or `/config set channels..accounts....`). - `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/tools/slash-commands.md b/docs/tools/slash-commands.md index dea4fb0d30f..d792398f1fa 100644 --- a/docs/tools/slash-commands.md +++ b/docs/tools/slash-commands.md @@ -123,6 +123,7 @@ Notes: - `/new ` accepts a model alias, `provider/model`, or a provider name (fuzzy match); if no match, the text is treated as the message body. - For full provider usage breakdown, use `openclaw status --usage`. - `/allowlist add|remove` requires `commands.config=true` and honors channel `configWrites`. +- In multi-account channels, config-targeted `/allowlist --account ` and `/config set channels..accounts....` also honor the target account's `configWrites`. - `/usage` controls the per-response usage footer; `/usage cost` prints a local cost summary from OpenClaw session logs. - `/restart` is enabled by default; set `commands.restart: false` to disable it. - Discord-only native command: `/vc join|leave|status` controls voice channels (requires `channels.discord.voice` and native commands; not available as text). diff --git a/src/auto-reply/reply/commands-allowlist.ts b/src/auto-reply/reply/commands-allowlist.ts index 766bb5f41b3..643d20143c6 100644 --- a/src/auto-reply/reply/commands-allowlist.ts +++ b/src/auto-reply/reply/commands-allowlist.ts @@ -1,5 +1,5 @@ import { getChannelDock } from "../../channels/dock.js"; -import { resolveChannelConfigWrites } from "../../channels/plugins/config-writes.js"; +import { authorizeConfigWrite } from "../../channels/plugins/config-writes.js"; import { listPairingChannels } from "../../channels/plugins/pairing.js"; import type { ChannelId } from "../../channels/plugins/types.js"; import { normalizeChannelId } from "../../channels/registry.js"; @@ -28,6 +28,7 @@ import { resolveSignalAccount } from "../../signal/accounts.js"; import { resolveSlackAccount } from "../../slack/accounts.js"; import { resolveSlackUserAllowlist } from "../../slack/resolve-users.js"; import { resolveTelegramAccount } from "../../telegram/accounts.js"; +import { isInternalMessageChannel } from "../../utils/message-channel.js"; import { resolveWhatsAppAccount } from "../../web/accounts.js"; import { rejectUnauthorizedCommand, requireCommandFlagEnabled } from "./command-gates.js"; import type { CommandHandler } from "./commands-types.js"; @@ -585,16 +586,25 @@ export const handleAllowlistCommand: CommandHandler = async (params, allowTextCo const shouldTouchStore = parsed.target !== "config" && listPairingChannels().includes(channelId); if (shouldUpdateConfig) { - const allowWrites = resolveChannelConfigWrites({ + const writeAuth = authorizeConfigWrite({ cfg: params.cfg, - channelId, - accountId: params.ctx.AccountId, + origin: { channelId, accountId: params.ctx.AccountId }, + targets: [{ channelId, accountId }], + allowBypass: + isInternalMessageChannel(params.command.channel) && + params.ctx.GatewayClientScopes?.includes("operator.admin") === true, }); - if (!allowWrites) { - const hint = `channels.${channelId}.configWrites=true`; + if (!writeAuth.allowed) { + const blocked = writeAuth.blockedScope?.scope; + const hint = + blocked?.channelId && blocked.accountId + ? `channels.${blocked.channelId}.accounts.${blocked.accountId}.configWrites=true` + : `channels.${blocked?.channelId ?? channelId}.configWrites=true`; return { shouldContinue: false, - reply: { text: `⚠️ Config writes are disabled for ${channelId}. Set ${hint} to enable.` }, + reply: { + text: `⚠️ Config writes are disabled for ${blocked?.channelId ?? channelId}. Set ${hint} to enable.`, + }, }; } diff --git a/src/auto-reply/reply/commands-config.ts b/src/auto-reply/reply/commands-config.ts index 00ef8048efe..edc0c145526 100644 --- a/src/auto-reply/reply/commands-config.ts +++ b/src/auto-reply/reply/commands-config.ts @@ -1,4 +1,7 @@ -import { resolveChannelConfigWrites } from "../../channels/plugins/config-writes.js"; +import { + authorizeConfigWrite, + resolveConfigWriteScopesFromPath, +} from "../../channels/plugins/config-writes.js"; import { normalizeChannelId } from "../../channels/registry.js"; import { getConfigValueAtPath, @@ -17,6 +20,7 @@ import { setConfigOverride, unsetConfigOverride, } from "../../config/runtime-overrides.js"; +import { isInternalMessageChannel } from "../../utils/message-channel.js"; import { rejectUnauthorizedCommand, requireCommandFlagEnabled, @@ -52,6 +56,7 @@ export const handleConfigCommand: CommandHandler = async (params, allowTextComma }; } + let parsedWritePath: string[] | undefined; if (configCommand.action === "set" || configCommand.action === "unset") { const missingAdminScope = requireGatewayClientScopeForInternalChannel(params, { label: "/config write", @@ -61,17 +66,41 @@ export const handleConfigCommand: CommandHandler = async (params, allowTextComma if (missingAdminScope) { return missingAdminScope; } + const parsedPath = parseConfigPath(configCommand.path); + if (!parsedPath.ok || !parsedPath.path) { + return { + shouldContinue: false, + reply: { text: `⚠️ ${parsedPath.error ?? "Invalid path."}` }, + }; + } + parsedWritePath = parsedPath.path; const channelId = params.command.channelId ?? normalizeChannelId(params.command.channel); - const allowWrites = resolveChannelConfigWrites({ + const writeAuth = authorizeConfigWrite({ cfg: params.cfg, - channelId, - accountId: params.ctx.AccountId, + origin: { channelId, accountId: params.ctx.AccountId }, + ...resolveConfigWriteScopesFromPath(parsedWritePath), + allowBypass: + isInternalMessageChannel(params.command.channel) && + params.ctx.GatewayClientScopes?.includes("operator.admin") === true, }); - if (!allowWrites) { - const channelLabel = channelId ?? "this channel"; - const hint = channelId - ? `channels.${channelId}.configWrites=true` - : "channels..configWrites=true"; + if (!writeAuth.allowed) { + if (writeAuth.reason === "ambiguous-target") { + return { + shouldContinue: false, + reply: { + text: "⚠️ Channel-initiated /config writes cannot replace channels, channel roots, or accounts collections. Use a more specific path or gateway operator.admin.", + }, + }; + } + const blocked = writeAuth.blockedScope?.scope; + const channelLabel = blocked?.channelId ?? channelId ?? "this channel"; + const hint = blocked?.channelId + ? blocked?.accountId + ? `channels.${blocked.channelId}.accounts.${blocked.accountId}.configWrites=true` + : `channels.${blocked.channelId}.configWrites=true` + : channelId + ? `channels.${channelId}.configWrites=true` + : "channels..configWrites=true"; return { shouldContinue: false, reply: { @@ -119,14 +148,7 @@ export const handleConfigCommand: CommandHandler = async (params, allowTextComma } if (configCommand.action === "unset") { - const parsedPath = parseConfigPath(configCommand.path); - if (!parsedPath.ok || !parsedPath.path) { - return { - shouldContinue: false, - reply: { text: `⚠️ ${parsedPath.error ?? "Invalid path."}` }, - }; - } - const removed = unsetConfigValueAtPath(parsedBase, parsedPath.path); + const removed = unsetConfigValueAtPath(parsedBase, parsedWritePath ?? []); if (!removed) { return { shouldContinue: false, @@ -151,14 +173,7 @@ export const handleConfigCommand: CommandHandler = async (params, allowTextComma } if (configCommand.action === "set") { - const parsedPath = parseConfigPath(configCommand.path); - if (!parsedPath.ok || !parsedPath.path) { - return { - shouldContinue: false, - reply: { text: `⚠️ ${parsedPath.error ?? "Invalid path."}` }, - }; - } - setConfigValueAtPath(parsedBase, parsedPath.path, configCommand.value); + setConfigValueAtPath(parsedBase, parsedWritePath ?? [], configCommand.value); const validated = validateConfigObjectWithPlugins(parsedBase); if (!validated.ok) { const issue = validated.issues[0]; diff --git a/src/auto-reply/reply/commands.test.ts b/src/auto-reply/reply/commands.test.ts index 0f526d6edaa..01e2c2238dd 100644 --- a/src/auto-reply/reply/commands.test.ts +++ b/src/auto-reply/reply/commands.test.ts @@ -682,6 +682,52 @@ describe("handleCommands /config configWrites gating", () => { expect(result.reply?.text).toContain("Config writes are disabled"); }); + it("blocks /config set when the target account disables writes", async () => { + const previousWriteCount = writeConfigFileMock.mock.calls.length; + const cfg = { + commands: { config: true, text: true }, + channels: { + telegram: { + configWrites: true, + accounts: { + work: { configWrites: false, enabled: true }, + }, + }, + }, + } as OpenClawConfig; + const params = buildPolicyParams( + "/config set channels.telegram.accounts.work.enabled=false", + cfg, + { + AccountId: "default", + Provider: "telegram", + Surface: "telegram", + }, + ); + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain("channels.telegram.accounts.work.configWrites=true"); + expect(writeConfigFileMock.mock.calls.length).toBe(previousWriteCount); + }); + + it("blocks ambiguous channel-root /config writes from channel commands", async () => { + const previousWriteCount = writeConfigFileMock.mock.calls.length; + const cfg = { + commands: { config: true, text: true }, + channels: { telegram: { configWrites: true } }, + } as OpenClawConfig; + const params = buildPolicyParams('/config set channels.telegram={"enabled":false}', cfg, { + Provider: "telegram", + Surface: "telegram", + }); + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain( + "cannot replace channels, channel roots, or accounts collections", + ); + expect(writeConfigFileMock.mock.calls.length).toBe(previousWriteCount); + }); + it("blocks /config set from gateway clients without operator.admin", async () => { const cfg = { commands: { config: true, text: true }, @@ -739,6 +785,49 @@ describe("handleCommands /config configWrites gating", () => { expect(writeConfigFileMock).toHaveBeenCalledOnce(); expect(result.reply?.text).toContain("Config updated"); }); + + it("keeps /config set working for gateway operator.admin on protected account paths", async () => { + readConfigFileSnapshotMock.mockResolvedValueOnce({ + valid: true, + parsed: { + channels: { + telegram: { + accounts: { + work: { enabled: true, configWrites: false }, + }, + }, + }, + }, + }); + validateConfigObjectWithPluginsMock.mockImplementation((config: unknown) => ({ + ok: true, + config, + })); + const params = buildParams( + "/config set channels.telegram.accounts.work.enabled=false", + { + commands: { config: true, text: true }, + channels: { + telegram: { + accounts: { + work: { enabled: true, configWrites: false }, + }, + }, + }, + } as OpenClawConfig, + { + 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(result.reply?.text).toContain("Config updated"); + const written = writeConfigFileMock.mock.calls.at(-1)?.[0] as OpenClawConfig; + expect(written.channels?.telegram?.accounts?.work?.enabled).toBe(false); + }); }); describe("handleCommands bash alias", () => { @@ -891,6 +980,31 @@ describe("handleCommands /allowlist", () => { }); }); + it("blocks config-targeted /allowlist edits when the target account disables writes", async () => { + const previousWriteCount = writeConfigFileMock.mock.calls.length; + const cfg = { + commands: { text: true, config: true }, + channels: { + telegram: { + configWrites: true, + accounts: { + work: { configWrites: false, allowFrom: ["123"] }, + }, + }, + }, + } as OpenClawConfig; + const params = buildPolicyParams("/allowlist add dm --account work --config 789", cfg, { + AccountId: "default", + Provider: "telegram", + Surface: "telegram", + }); + const result = await handleCommands(params); + + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain("channels.telegram.accounts.work.configWrites=true"); + expect(writeConfigFileMock.mock.calls.length).toBe(previousWriteCount); + }); + it("removes default-account entries from scoped and legacy pairing stores", async () => { removeChannelAllowFromStoreEntryMock .mockResolvedValueOnce({ diff --git a/src/channels/plugins/config-writes.ts b/src/channels/plugins/config-writes.ts index 87e220d7029..440e95dcfe9 100644 --- a/src/channels/plugins/config-writes.ts +++ b/src/channels/plugins/config-writes.ts @@ -12,6 +12,19 @@ function resolveAccountConfig(accounts: ChannelConfigWithAccounts["accounts"], a return resolveAccountEntry(accounts, accountId); } +export type ConfigWriteScope = { + channelId?: ChannelId | null; + accountId?: string | null; +}; + +export type ConfigWriteAuthorizationResult = + | { allowed: true } + | { + allowed: false; + reason: "ambiguous-target" | "origin-disabled" | "target-disabled"; + blockedScope?: { kind: "origin" | "target"; scope: ConfigWriteScope }; + }; + export function resolveChannelConfigWrites(params: { cfg: OpenClawConfig; channelId?: ChannelId | null; @@ -30,3 +43,86 @@ export function resolveChannelConfigWrites(params: { const value = accountConfig?.configWrites ?? channelConfig.configWrites; return value !== false; } + +export function authorizeConfigWrite(params: { + cfg: OpenClawConfig; + origin?: ConfigWriteScope; + targets?: ConfigWriteScope[]; + allowBypass?: boolean; + hasAmbiguousTarget?: boolean; +}): ConfigWriteAuthorizationResult { + if (params.allowBypass) { + return { allowed: true }; + } + if (params.hasAmbiguousTarget) { + return { allowed: false, reason: "ambiguous-target" }; + } + if ( + params.origin?.channelId && + !resolveChannelConfigWrites({ + cfg: params.cfg, + channelId: params.origin.channelId, + accountId: params.origin.accountId, + }) + ) { + return { + allowed: false, + reason: "origin-disabled", + blockedScope: { kind: "origin", scope: params.origin }, + }; + } + const seen = new Set(); + for (const target of params.targets ?? []) { + if (!target.channelId) { + continue; + } + const key = `${target.channelId}:${normalizeAccountId(target.accountId)}`; + if (seen.has(key)) { + continue; + } + seen.add(key); + if ( + !resolveChannelConfigWrites({ + cfg: params.cfg, + channelId: target.channelId, + accountId: target.accountId, + }) + ) { + return { + allowed: false, + reason: "target-disabled", + blockedScope: { kind: "target", scope: target }, + }; + } + } + return { allowed: true }; +} + +export function resolveConfigWriteScopesFromPath(path: string[]): { + targets: ConfigWriteScope[]; + hasAmbiguousTarget: boolean; +} { + if (path[0] !== "channels") { + return { targets: [], hasAmbiguousTarget: false }; + } + if (path.length < 2) { + return { targets: [], hasAmbiguousTarget: true }; + } + const channelId = path[1].trim().toLowerCase() as ChannelId; + if (!channelId) { + return { targets: [], hasAmbiguousTarget: true }; + } + if (path.length === 2) { + return { targets: [{ channelId }], hasAmbiguousTarget: true }; + } + if (path[2] !== "accounts") { + return { targets: [{ channelId }], hasAmbiguousTarget: false }; + } + if (path.length < 4) { + return { targets: [{ channelId }], hasAmbiguousTarget: true }; + } + return { + targets: [{ channelId, accountId: normalizeAccountId(path[3]) }], + hasAmbiguousTarget: false, + }; +} diff --git a/src/channels/plugins/plugins-core.test.ts b/src/channels/plugins/plugins-core.test.ts index 49012222982..8d574e472b4 100644 --- a/src/channels/plugins/plugins-core.test.ts +++ b/src/channels/plugins/plugins-core.test.ts @@ -20,7 +20,11 @@ import { } from "../../test-utils/channel-plugins.js"; import { withEnvAsync } from "../../test-utils/env.js"; import { getChannelPluginCatalogEntry, listChannelPluginCatalogEntries } from "./catalog.js"; -import { resolveChannelConfigWrites } from "./config-writes.js"; +import { + authorizeConfigWrite, + resolveChannelConfigWrites, + resolveConfigWriteScopesFromPath, +} from "./config-writes.js"; import { listDiscordDirectoryGroupsFromConfig, listDiscordDirectoryPeersFromConfig, @@ -325,6 +329,34 @@ describe("resolveChannelConfigWrites", () => { }); }); +describe("authorizeConfigWrite", () => { + it("blocks when a target account disables writes", () => { + const cfg = makeSlackConfigWritesCfg("work"); + expect( + authorizeConfigWrite({ + cfg, + origin: { channelId: "slack", accountId: "default" }, + targets: [{ channelId: "slack", accountId: "work" }], + }), + ).toEqual({ + allowed: false, + reason: "target-disabled", + blockedScope: { kind: "target", scope: { channelId: "slack", accountId: "work" } }, + }); + }); + + it("rejects ambiguous channel collection writes", () => { + expect(resolveConfigWriteScopesFromPath(["channels", "telegram"])).toEqual({ + targets: [{ channelId: "telegram" }], + hasAmbiguousTarget: true, + }); + expect(resolveConfigWriteScopesFromPath(["channels", "telegram", "accounts"])).toEqual({ + targets: [{ channelId: "telegram" }], + hasAmbiguousTarget: true, + }); + }); +}); + describe("directory (config-backed)", () => { it("lists Slack peers/groups from config", async () => { const cfg = {