fix(commands): enforce allowlist writes from command origin (#72361) (thanks @coygeek)

This commit is contained in:
Peter Steinberger
2026-05-27 07:00:55 +01:00
parent 9f06b6cca2
commit 42a38d2b00
4 changed files with 72 additions and 14 deletions

View File

@@ -37,10 +37,12 @@ vi.mock("../../config/config.js", () => ({
transform: (
currentConfig: OpenClawConfig,
context: { snapshot: ConfigSnapshotMock; previousHash: string | null; attempt: number },
) => Promise<{ nextConfig: OpenClawConfig; result?: unknown }> | {
nextConfig: OpenClawConfig;
result?: unknown;
};
) =>
| Promise<{ nextConfig: OpenClawConfig; result?: unknown }>
| {
nextConfig: OpenClawConfig;
result?: unknown;
};
}) => {
const snapshot = (await readConfigFileSnapshotMock()) as ConfigSnapshotMock;
const previousHash = snapshot.hash ?? null;
@@ -586,6 +588,31 @@ describe("handleAllowlistCommand", () => {
expect(addChannelAllowFromStoreEntryMock).not.toHaveBeenCalled();
});
it("blocks cross-channel config edits when the command origin disables writes", async () => {
const cfg = {
commands: { text: true, config: true },
channels: {
telegram: { allowFrom: ["123"], configWrites: false },
discord: { allowFrom: ["456"], configWrites: true },
},
} as OpenClawConfig;
readConfigFileSnapshotMock.mockResolvedValueOnce({
valid: true,
parsed: structuredClone(cfg),
});
const params = buildAllowlistParams("/allowlist add dm --channel discord --config 789", cfg, {
Provider: "telegram",
Surface: "telegram",
});
params.command.senderIsOwner = true;
const result = await handleAllowlistCommand(params, true);
expect(result?.shouldContinue).toBe(false);
expect(result?.reply?.text).toContain("channels.telegram.accounts.default.configWrites=true");
expect(replaceConfigFileMock).not.toHaveBeenCalled();
expect(addChannelAllowFromStoreEntryMock).not.toHaveBeenCalled();
});
it("blocks store-targeted allowlist edits when channel configWrites is false", async () => {
const cfg = {
commands: { text: true, config: true },
@@ -628,6 +655,26 @@ describe("handleAllowlistCommand", () => {
expect(addChannelAllowFromStoreEntryMock).not.toHaveBeenCalled();
});
it("blocks cross-channel store edits when the command origin disables writes", async () => {
const cfg = {
commands: { text: true, config: true },
channels: {
telegram: { allowFrom: ["123"], configWrites: false },
discord: { allowFrom: ["456"], configWrites: true },
},
} as OpenClawConfig;
const params = buildAllowlistParams("/allowlist add dm --channel discord --store 789", cfg, {
Provider: "telegram",
Surface: "telegram",
});
params.command.senderIsOwner = true;
const result = await handleAllowlistCommand(params, true);
expect(result?.shouldContinue).toBe(false);
expect(result?.reply?.text).toContain("channels.telegram.accounts.default.configWrites=true");
expect(addChannelAllowFromStoreEntryMock).not.toHaveBeenCalled();
});
it("allows store-targeted allowlist edits when configWrites is true", async () => {
addChannelAllowFromStoreEntryMock.mockResolvedValueOnce({
changed: true,

View File

@@ -15,6 +15,7 @@ import {
normalizeOptionalString,
} from "../../shared/string-coerce.js";
import { normalizeStringEntries } from "../../shared/string-normalization.js";
import { resolveChannelAccountId, resolveCommandSurfaceChannel } from "./channel-context.js";
import {
rejectNonOwnerCommand,
rejectUnauthorizedCommand,
@@ -308,6 +309,13 @@ export const handleAllowlistCommand: CommandHandler = async (params, allowTextCo
parsedAccount: parsed.account,
ctxAccountId: params.ctx.AccountId,
});
const originChannelId =
params.command.channelId ?? normalizeChannelId(resolveCommandSurfaceChannel(params));
const originAccountId = resolveChannelAccountId({
cfg: params.cfg,
ctx: params.ctx,
command: params.command,
});
const plugin = getChannelPlugin(channelId);
if (parsed.action === "list") {
@@ -491,10 +499,11 @@ export const handleAllowlistCommand: CommandHandler = async (params, allowTextCo
const deniedText = resolveConfigWriteDeniedText({
cfg: params.cfg,
channel: params.command.channel,
channelId,
accountId,
originChannelId,
originAccountId,
gatewayClientScopes: params.ctx.GatewayClientScopes,
target: editResult.writeTarget,
fallbackChannelId: channelId,
});
if (deniedText) {
return {
@@ -566,10 +575,11 @@ export const handleAllowlistCommand: CommandHandler = async (params, allowTextCo
const storeDeniedText = resolveConfigWriteDeniedText({
cfg: params.cfg,
channel: params.command.channel,
channelId,
accountId,
originChannelId,
originAccountId,
gatewayClientScopes: params.ctx.GatewayClientScopes,
target: resolveExplicitConfigWriteTarget({ channelId, accountId }),
fallbackChannelId: channelId,
});
if (storeDeniedText) {
return {

View File

@@ -81,8 +81,8 @@ export const handleConfigCommand: CommandHandler = async (params, allowTextComma
const deniedText = resolveConfigWriteDeniedText({
cfg: params.cfg,
channel: params.command.channel,
channelId,
accountId: resolveChannelAccountId({
originChannelId: channelId,
originAccountId: resolveChannelAccountId({
cfg: params.cfg,
ctx: params.ctx,
command: params.command,

View File

@@ -9,14 +9,15 @@ import type { OpenClawConfig } from "../../config/types.openclaw.js";
export function resolveConfigWriteDeniedText(params: {
cfg: OpenClawConfig;
channel?: string | null;
channelId: ChannelId | null;
accountId?: string;
originChannelId: ChannelId | null;
originAccountId?: string;
gatewayClientScopes?: string[];
target: Parameters<typeof authorizeConfigWrite>[0]["target"];
fallbackChannelId?: ChannelId | null;
}): string | null {
const writeAuth = authorizeConfigWrite({
cfg: params.cfg,
origin: { channelId: params.channelId, accountId: params.accountId },
origin: { channelId: params.originChannelId, accountId: params.originAccountId },
target: params.target,
allowBypass: canBypassConfigWritePolicy({
channel: params.channel ?? "",
@@ -28,6 +29,6 @@ export function resolveConfigWriteDeniedText(params: {
}
return formatConfigWriteDeniedMessage({
result: writeAuth,
fallbackChannelId: params.channelId,
fallbackChannelId: params.fallbackChannelId ?? params.originChannelId,
});
}