mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-30 06:14:23 +00:00
fix(discord): honor commands.allowFrom in guild slash auth (#38794)
* fix(discord): honor commands.allowFrom in guild slash auth * Update native-command.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update native-command.commands-allowfrom.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(discord): address slash auth review feedback * test(discord): add slash auth coverage for allowFrom variants * fix: add changelog entry for discord slash auth fix (#38794) (thanks @jskoiz) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Shadow <hi@shadowing.dev>
This commit is contained in:
@@ -244,6 +244,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Auto-reply/allowlist store account scoping: keep `/allowlist ... --store` writes scoped to the selected account and clear legacy unscoped entries when removing default-account store access, preventing cross-account default allowlist bleed-through from legacy pairing-store reads. Thanks @tdjackey for reporting and @vincentkoc for the fix.
|
||||
- Security/Nostr: harden profile mutation/import loopback guards by failing closed on non-loopback forwarded client headers (`x-forwarded-for` / `x-real-ip`) and rejecting `sec-fetch-site: cross-site`; adds regression coverage for proxy-forwarded and browser cross-site mutation attempts.
|
||||
- CLI/bootstrap Node version hint maintenance: replace hardcoded nvm `22` instructions in `openclaw.mjs` with `MIN_NODE_MAJOR` interpolation so future minimum-Node bumps keep startup guidance in sync automatically. (#39056) Thanks @onstash.
|
||||
- Discord/native slash command auth: honor `commands.allowFrom.discord` (and `commands.allowFrom["*"]`) in guild slash-command pre-dispatch authorization so allowlisted senders are no longer incorrectly rejected as unauthorized. (#38794) Thanks @jskoiz and @thewilloftheshadow.
|
||||
|
||||
## 2026.3.2
|
||||
|
||||
|
||||
245
src/discord/monitor/native-command.commands-allowfrom.test.ts
Normal file
245
src/discord/monitor/native-command.commands-allowfrom.test.ts
Normal file
@@ -0,0 +1,245 @@
|
||||
import { ChannelType } from "@buape/carbon";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { NativeCommandSpec } from "../../auto-reply/commands-registry.js";
|
||||
import * as dispatcherModule from "../../auto-reply/reply/provider-dispatcher.js";
|
||||
import type { OpenClawConfig } from "../../config/config.js";
|
||||
import * as pluginCommandsModule from "../../plugins/commands.js";
|
||||
import { createDiscordNativeCommand } from "./native-command.js";
|
||||
import { createNoopThreadBindingManager } from "./thread-bindings.js";
|
||||
|
||||
type MockCommandInteraction = {
|
||||
user: { id: string; username: string; globalName: string };
|
||||
channel: { type: ChannelType; id: string };
|
||||
guild: { id: string; name?: string } | null;
|
||||
rawData: { id: string; member: { roles: string[] } };
|
||||
options: {
|
||||
getString: ReturnType<typeof vi.fn>;
|
||||
getNumber: ReturnType<typeof vi.fn>;
|
||||
getBoolean: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
reply: ReturnType<typeof vi.fn>;
|
||||
followUp: ReturnType<typeof vi.fn>;
|
||||
client: object;
|
||||
};
|
||||
|
||||
function createInteraction(params?: {
|
||||
userId?: string;
|
||||
channelId?: string;
|
||||
guildId?: string;
|
||||
guildName?: string;
|
||||
}): MockCommandInteraction {
|
||||
return {
|
||||
user: {
|
||||
id: params?.userId ?? "123456789012345678",
|
||||
username: "discord-user",
|
||||
globalName: "Discord User",
|
||||
},
|
||||
channel: {
|
||||
type: ChannelType.GuildText,
|
||||
id: params?.channelId ?? "234567890123456789",
|
||||
},
|
||||
guild: {
|
||||
id: params?.guildId ?? "345678901234567890",
|
||||
name: params?.guildName ?? "Test Guild",
|
||||
},
|
||||
rawData: {
|
||||
id: "interaction-1",
|
||||
member: { roles: [] },
|
||||
},
|
||||
options: {
|
||||
getString: vi.fn().mockReturnValue(null),
|
||||
getNumber: vi.fn().mockReturnValue(null),
|
||||
getBoolean: vi.fn().mockReturnValue(null),
|
||||
},
|
||||
reply: vi.fn().mockResolvedValue({ ok: true }),
|
||||
followUp: vi.fn().mockResolvedValue({ ok: true }),
|
||||
client: {},
|
||||
};
|
||||
}
|
||||
|
||||
function createConfig(): OpenClawConfig {
|
||||
return {
|
||||
commands: {
|
||||
allowFrom: {
|
||||
discord: ["user:123456789012345678"],
|
||||
},
|
||||
},
|
||||
channels: {
|
||||
discord: {
|
||||
groupPolicy: "allowlist",
|
||||
guilds: {
|
||||
"345678901234567890": {
|
||||
channels: {
|
||||
"234567890123456789": {
|
||||
allow: true,
|
||||
requireMention: false,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
}
|
||||
|
||||
function createCommand(cfg: OpenClawConfig) {
|
||||
const commandSpec: NativeCommandSpec = {
|
||||
name: "status",
|
||||
description: "Status",
|
||||
acceptsArgs: false,
|
||||
};
|
||||
return createDiscordNativeCommand({
|
||||
command: commandSpec,
|
||||
cfg,
|
||||
discordConfig: cfg.channels?.discord ?? {},
|
||||
accountId: "default",
|
||||
sessionPrefix: "discord:slash",
|
||||
ephemeralDefault: true,
|
||||
threadBindings: createNoopThreadBindingManager("default"),
|
||||
});
|
||||
}
|
||||
|
||||
describe("Discord native slash commands with commands.allowFrom", () => {
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("authorizes guild slash commands when commands.allowFrom.discord matches the sender", async () => {
|
||||
const cfg = createConfig();
|
||||
const command = createCommand(cfg);
|
||||
const interaction = createInteraction();
|
||||
|
||||
vi.spyOn(pluginCommandsModule, "matchPluginCommand").mockReturnValue(null);
|
||||
const dispatchSpy = vi
|
||||
.spyOn(dispatcherModule, "dispatchReplyWithDispatcher")
|
||||
.mockResolvedValue({
|
||||
counts: {
|
||||
final: 1,
|
||||
block: 0,
|
||||
tool: 0,
|
||||
},
|
||||
} as never);
|
||||
|
||||
await (command as { run: (interaction: unknown) => Promise<void> }).run(interaction as unknown);
|
||||
|
||||
expect(dispatchSpy).toHaveBeenCalledTimes(1);
|
||||
expect(interaction.reply).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({ content: "You are not authorized to use this command." }),
|
||||
);
|
||||
});
|
||||
|
||||
it("authorizes guild slash commands from the global commands.allowFrom list when provider-specific allowFrom is missing", async () => {
|
||||
const cfg = createConfig();
|
||||
cfg.commands = {
|
||||
allowFrom: {
|
||||
"*": ["user:123456789012345678"],
|
||||
},
|
||||
};
|
||||
const command = createCommand(cfg);
|
||||
const interaction = createInteraction();
|
||||
|
||||
vi.spyOn(pluginCommandsModule, "matchPluginCommand").mockReturnValue(null);
|
||||
const dispatchSpy = vi
|
||||
.spyOn(dispatcherModule, "dispatchReplyWithDispatcher")
|
||||
.mockResolvedValue({
|
||||
counts: {
|
||||
final: 1,
|
||||
block: 0,
|
||||
tool: 0,
|
||||
},
|
||||
} as never);
|
||||
|
||||
await (command as { run: (interaction: unknown) => Promise<void> }).run(interaction as unknown);
|
||||
|
||||
expect(dispatchSpy).toHaveBeenCalledTimes(1);
|
||||
expect(interaction.reply).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({ content: "You are not authorized to use this command." }),
|
||||
);
|
||||
});
|
||||
|
||||
it("authorizes guild slash commands when commands.useAccessGroups is false and commands.allowFrom.discord matches the sender", async () => {
|
||||
const cfg = createConfig();
|
||||
cfg.commands = {
|
||||
...cfg.commands,
|
||||
useAccessGroups: false,
|
||||
};
|
||||
const command = createCommand(cfg);
|
||||
const interaction = createInteraction();
|
||||
|
||||
vi.spyOn(pluginCommandsModule, "matchPluginCommand").mockReturnValue(null);
|
||||
const dispatchSpy = vi
|
||||
.spyOn(dispatcherModule, "dispatchReplyWithDispatcher")
|
||||
.mockResolvedValue({
|
||||
counts: {
|
||||
final: 1,
|
||||
block: 0,
|
||||
tool: 0,
|
||||
},
|
||||
} as never);
|
||||
|
||||
await (command as { run: (interaction: unknown) => Promise<void> }).run(interaction as unknown);
|
||||
|
||||
expect(dispatchSpy).toHaveBeenCalledTimes(1);
|
||||
expect(interaction.reply).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({ content: "You are not authorized to use this command." }),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects guild slash commands when commands.allowFrom.discord does not match the sender", async () => {
|
||||
const cfg = createConfig();
|
||||
const command = createCommand(cfg);
|
||||
const interaction = createInteraction({ userId: "999999999999999999" });
|
||||
|
||||
vi.spyOn(pluginCommandsModule, "matchPluginCommand").mockReturnValue(null);
|
||||
const dispatchSpy = vi
|
||||
.spyOn(dispatcherModule, "dispatchReplyWithDispatcher")
|
||||
.mockResolvedValue({
|
||||
counts: {
|
||||
final: 1,
|
||||
block: 0,
|
||||
tool: 0,
|
||||
},
|
||||
} as never);
|
||||
|
||||
await (command as { run: (interaction: unknown) => Promise<void> }).run(interaction as unknown);
|
||||
|
||||
expect(dispatchSpy).not.toHaveBeenCalled();
|
||||
expect(interaction.reply).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
content: "You are not authorized to use this command.",
|
||||
ephemeral: true,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects guild slash commands when commands.useAccessGroups is false and commands.allowFrom.discord does not match the sender", async () => {
|
||||
const cfg = createConfig();
|
||||
cfg.commands = {
|
||||
...cfg.commands,
|
||||
useAccessGroups: false,
|
||||
};
|
||||
const command = createCommand(cfg);
|
||||
const interaction = createInteraction({ userId: "999999999999999999" });
|
||||
|
||||
vi.spyOn(pluginCommandsModule, "matchPluginCommand").mockReturnValue(null);
|
||||
const dispatchSpy = vi
|
||||
.spyOn(dispatcherModule, "dispatchReplyWithDispatcher")
|
||||
.mockResolvedValue({
|
||||
counts: {
|
||||
final: 1,
|
||||
block: 0,
|
||||
tool: 0,
|
||||
},
|
||||
} as never);
|
||||
|
||||
await (command as { run: (interaction: unknown) => Promise<void> }).run(interaction as unknown);
|
||||
|
||||
expect(dispatchSpy).not.toHaveBeenCalled();
|
||||
expect(interaction.reply).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
content: "You are not authorized to use this command.",
|
||||
ephemeral: true,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -20,6 +20,7 @@ import {
|
||||
} from "../../acp/persistent-bindings.route.js";
|
||||
import { resolveHumanDelayConfig } from "../../agents/identity.js";
|
||||
import { resolveChunkMode, resolveTextChunkLimit } from "../../auto-reply/chunk.js";
|
||||
import { resolveCommandAuthorization } from "../../auto-reply/command-auth.js";
|
||||
import type {
|
||||
ChatCommandDefinition,
|
||||
CommandArgDefinition,
|
||||
@@ -92,6 +93,46 @@ import { resolveDiscordThreadParentInfo } from "./threading.js";
|
||||
type DiscordConfig = NonNullable<OpenClawConfig["channels"]>["discord"];
|
||||
const log = createSubsystemLogger("discord/native-command");
|
||||
|
||||
function resolveDiscordNativeCommandAllowlistAccess(params: {
|
||||
cfg: OpenClawConfig;
|
||||
accountId?: string | null;
|
||||
sender: { id: string; name?: string; tag?: string };
|
||||
chatType: "direct" | "group" | "thread" | "channel";
|
||||
conversationId?: string;
|
||||
}) {
|
||||
const commandsAllowFrom = params.cfg.commands?.allowFrom;
|
||||
if (!commandsAllowFrom || typeof commandsAllowFrom !== "object") {
|
||||
return { configured: false, allowed: false } as const;
|
||||
}
|
||||
const configured =
|
||||
Array.isArray(commandsAllowFrom.discord) || Array.isArray(commandsAllowFrom["*"]);
|
||||
if (!configured) {
|
||||
return { configured: false, allowed: false } as const;
|
||||
}
|
||||
|
||||
const from =
|
||||
params.chatType === "direct"
|
||||
? `discord:${params.sender.id}`
|
||||
: `discord:${params.chatType}:${params.conversationId ?? "unknown"}`;
|
||||
const auth = resolveCommandAuthorization({
|
||||
ctx: {
|
||||
Provider: "discord",
|
||||
Surface: "discord",
|
||||
OriginatingChannel: "discord",
|
||||
AccountId: params.accountId ?? undefined,
|
||||
ChatType: params.chatType,
|
||||
From: from,
|
||||
SenderId: params.sender.id,
|
||||
SenderUsername: params.sender.name,
|
||||
SenderTag: params.sender.tag,
|
||||
},
|
||||
cfg: params.cfg,
|
||||
// We only want explicit commands.allowFrom authorization here.
|
||||
commandAuthorized: false,
|
||||
});
|
||||
return { configured: true, allowed: auth.isAuthorizedSender } as const;
|
||||
}
|
||||
|
||||
function buildDiscordCommandOptions(params: {
|
||||
command: ChatCommandDefinition;
|
||||
cfg: ReturnType<typeof loadConfig>;
|
||||
@@ -1297,6 +1338,23 @@ async function dispatchDiscordCommandInteraction(params: {
|
||||
},
|
||||
allowNameMatching,
|
||||
});
|
||||
const commandsAllowFromAccess = resolveDiscordNativeCommandAllowlistAccess({
|
||||
cfg,
|
||||
accountId,
|
||||
sender: {
|
||||
id: sender.id,
|
||||
name: sender.name,
|
||||
tag: sender.tag,
|
||||
},
|
||||
chatType: isDirectMessage
|
||||
? "direct"
|
||||
: isThreadChannel
|
||||
? "thread"
|
||||
: interaction.guild
|
||||
? "channel"
|
||||
: "group",
|
||||
conversationId: rawChannelId || undefined,
|
||||
});
|
||||
const guildInfo = resolveDiscordGuildEntry({
|
||||
guild: interaction.guild ?? undefined,
|
||||
guildEntries: discordConfig?.guilds,
|
||||
@@ -1418,10 +1476,20 @@ async function dispatchDiscordCommandInteraction(params: {
|
||||
});
|
||||
const authorizers = useAccessGroups
|
||||
? [
|
||||
{
|
||||
configured: commandsAllowFromAccess.configured,
|
||||
allowed: commandsAllowFromAccess.allowed,
|
||||
},
|
||||
{ configured: ownerAllowList != null, allowed: ownerOk },
|
||||
{ configured: hasAccessRestrictions, allowed: memberAllowed },
|
||||
]
|
||||
: [{ configured: hasAccessRestrictions, allowed: memberAllowed }];
|
||||
: [
|
||||
{
|
||||
configured: commandsAllowFromAccess.configured,
|
||||
allowed: commandsAllowFromAccess.allowed,
|
||||
},
|
||||
{ configured: hasAccessRestrictions, allowed: memberAllowed },
|
||||
];
|
||||
commandAuthorized = resolveCommandAuthorizedFromAuthorizers({
|
||||
useAccessGroups,
|
||||
authorizers,
|
||||
|
||||
Reference in New Issue
Block a user