From 56eeec40992ac8d375e221606917960cbada26fd Mon Sep 17 00:00:00 2001 From: Devin Robison Date: Tue, 24 Mar 2026 17:05:59 -0700 Subject: [PATCH] fix: require operator.admin for mutating internal /allowlist commands (#54097) --- src/auto-reply/reply/commands-allowlist.ts | 15 ++- src/auto-reply/reply/commands.test.ts | 128 +++++++++++++++++++++ 2 files changed, 142 insertions(+), 1 deletion(-) diff --git a/src/auto-reply/reply/commands-allowlist.ts b/src/auto-reply/reply/commands-allowlist.ts index 7360fa20252..d0416a31ecc 100644 --- a/src/auto-reply/reply/commands-allowlist.ts +++ b/src/auto-reply/reply/commands-allowlist.ts @@ -18,7 +18,11 @@ import { normalizeOptionalAccountId, } from "../../routing/session-key.js"; import { normalizeStringEntries } from "../../shared/string-normalization.js"; -import { rejectUnauthorizedCommand, requireCommandFlagEnabled } from "./command-gates.js"; +import { + rejectUnauthorizedCommand, + requireCommandFlagEnabled, + requireGatewayClientScopeForInternalChannel, +} from "./command-gates.js"; import type { CommandHandler } from "./commands-types.js"; import { resolveConfigWriteDeniedText } from "./config-write-authorization.js"; @@ -383,6 +387,15 @@ export const handleAllowlistCommand: CommandHandler = async (params, allowTextCo return { shouldContinue: false, reply: { text: lines.join("\n") } }; } + const missingAdminScope = requireGatewayClientScopeForInternalChannel(params, { + label: "/allowlist write", + allowedScopes: ["operator.admin"], + missingText: "❌ /allowlist add|remove requires operator.admin for gateway clients.", + }); + if (missingAdminScope) { + return missingAdminScope; + } + const disabled = requireCommandFlagEnabled(params.cfg, { label: "/allowlist edits", configKey: "config", diff --git a/src/auto-reply/reply/commands.test.ts b/src/auto-reply/reply/commands.test.ts index 2e7461da57b..7df15a39f13 100644 --- a/src/auto-reply/reply/commands.test.ts +++ b/src/auto-reply/reply/commands.test.ts @@ -1463,6 +1463,134 @@ describe("handleCommands /allowlist", () => { }); } }); + + describe("operator.admin scope gating", () => { + it("blocks /allowlist add from internal gateway clients without operator.admin", async () => { + const cfg = { + commands: { text: true, config: true }, + channels: { telegram: { allowFrom: ["123"] } }, + } as OpenClawConfig; + const params = buildPolicyParams("/allowlist add dm channel=telegram 789", 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"); + expect(writeConfigFileMock).not.toHaveBeenCalled(); + expect(addChannelAllowFromStoreEntryMock).not.toHaveBeenCalled(); + }); + + it("allows /allowlist add from internal gateway clients with operator.admin", async () => { + validateConfigObjectWithPluginsMock.mockImplementation((config: unknown) => ({ + ok: true, + config, + })); + readConfigFileSnapshotMock.mockResolvedValueOnce({ + valid: true, + parsed: { + channels: { telegram: { allowFrom: ["123"] } }, + }, + }); + addChannelAllowFromStoreEntryMock.mockResolvedValueOnce({ + changed: true, + allowFrom: ["123", "789"], + }); + + const cfg = { + commands: { text: true, config: true }, + channels: { telegram: { allowFrom: ["123"] } }, + } as OpenClawConfig; + const params = buildPolicyParams("/allowlist add dm channel=telegram 789", 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(result.reply?.text).toContain("DM allowlist added"); + }); + + it("blocks /allowlist remove from internal gateway clients without operator.admin", async () => { + const cfg = { + commands: { text: true, config: true }, + channels: { telegram: { allowFrom: ["123", "789"] } }, + } as OpenClawConfig; + const params = buildPolicyParams("/allowlist remove dm channel=telegram 789", 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"); + expect(writeConfigFileMock).not.toHaveBeenCalled(); + expect(removeChannelAllowFromStoreEntryMock).not.toHaveBeenCalled(); + }); + + it("allows /allowlist remove from internal gateway clients with operator.admin", async () => { + validateConfigObjectWithPluginsMock.mockImplementation((config: unknown) => ({ + ok: true, + config, + })); + readConfigFileSnapshotMock.mockResolvedValueOnce({ + valid: true, + parsed: { + channels: { telegram: { allowFrom: ["123", "789"] } }, + }, + }); + removeChannelAllowFromStoreEntryMock.mockResolvedValueOnce({ + changed: true, + allowFrom: ["123"], + }); + + const cfg = { + commands: { text: true, config: true }, + channels: { telegram: { allowFrom: ["123", "789"] } }, + } as OpenClawConfig; + const params = buildPolicyParams("/allowlist remove dm channel=telegram 789", 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(result.reply?.text).toContain("DM allowlist removed"); + }); + + it("keeps /allowlist list accessible to internal operator.write clients", async () => { + readChannelAllowFromStoreMock.mockResolvedValueOnce(["456"]); + + const cfg = { + commands: { text: true }, + channels: { telegram: { allowFrom: ["123"] } }, + } as OpenClawConfig; + const params = buildPolicyParams("/allowlist list dm channel=telegram", 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("Channel: telegram"); + }); + }); }); describe("/models command", () => {