fix(gateway): require admin for chat config writes

This commit is contained in:
Peter Steinberger
2026-03-07 19:38:40 +00:00
parent 724d2d58fa
commit 5f8f58ae25
7 changed files with 112 additions and 19 deletions

View File

@@ -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;

View File

@@ -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);

View File

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

View File

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