fix: require operator.admin for mutating internal /allowlist commands (#54097)

This commit is contained in:
Devin Robison
2026-03-24 17:05:59 -07:00
committed by GitHub
parent 561acd1675
commit 56eeec4099
2 changed files with 142 additions and 1 deletions

View File

@@ -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",

View File

@@ -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", () => {