Skip to content

Commit 0f0cecd

Browse files
authored
Discord: enforce strict DM component allowlist auth (openclaw#49997)
* Discord: enforce strict DM component allowlist auth * Discord: align model picker fallback routing * changelog Signed-off-by: joshavant <830519+joshavant@users.noreply.github.com> --------- Signed-off-by: joshavant <830519+joshavant@users.noreply.github.com>
1 parent 7b151af commit 0f0cecd

5 files changed

Lines changed: 106 additions & 18 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ Docs: https://docs.openclaw.ai
132132
- Models/chat commands: keep `/model ...@YYYYMMDD` version suffixes intact by default, but still honor matching stored numeric auth-profile overrides for the same provider. (#48896) Thanks @Alix-007.
133133
- Gateway/channels: serialize per-account channel startup so overlapping starts do not boot the same provider twice, preventing MS Teams `EADDRINUSE` crash loops during startup and restart. (#49583) Thanks @sudie-codes.
134134
- Tests/OpenAI Codex auth: align login expectations with the default `gpt-5.4` model so CI coverage stays consistent with the current OpenAI Codex default. (#44367) Thanks @jrrcdev.
135+
- Discord: enforce strict DM component allowlist auth (#49997) Thanks @joshavant.
135136

136137
### Fixes
137138

extensions/discord/src/monitor/agent-components-helpers.ts

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,21 @@ async function ensureDmComponentAuthorized(params: {
429429
replyOpts: { ephemeral?: boolean };
430430
}) {
431431
const { ctx, interaction, user, componentLabel, replyOpts } = params;
432+
const allowFromPrefixes = ["discord:", "user:", "pk:"];
433+
const resolveAllowMatch = (entries: string[]) => {
434+
const allowList = normalizeDiscordAllowList(entries, allowFromPrefixes);
435+
return allowList
436+
? resolveDiscordAllowListMatch({
437+
allowList,
438+
candidate: {
439+
id: user.id,
440+
name: user.username,
441+
tag: formatDiscordUserTag(user),
442+
},
443+
allowNameMatching: isDangerousNameMatchingEnabled(ctx.discordConfig),
444+
})
445+
: { allowed: false };
446+
};
432447
const dmPolicy = ctx.dmPolicy ?? "pairing";
433448
if (dmPolicy === "disabled") {
434449
logVerbose(`agent ${componentLabel}: blocked (DM policy disabled)`);
@@ -444,24 +459,27 @@ async function ensureDmComponentAuthorized(params: {
444459
return true;
445460
}
446461

462+
if (dmPolicy === "allowlist") {
463+
const allowMatch = resolveAllowMatch(ctx.allowFrom ?? []);
464+
if (allowMatch.allowed) {
465+
return true;
466+
}
467+
logVerbose(`agent ${componentLabel}: blocked DM user ${user.id} (not in allowFrom)`);
468+
try {
469+
await interaction.reply({
470+
content: `You are not authorized to use this ${componentLabel}.`,
471+
...replyOpts,
472+
});
473+
} catch {}
474+
return false;
475+
}
476+
447477
const storeAllowFrom = await readStoreAllowFromForDmPolicy({
448478
provider: "discord",
449479
accountId: ctx.accountId,
450480
dmPolicy,
451481
});
452-
const effectiveAllowFrom = [...(ctx.allowFrom ?? []), ...storeAllowFrom];
453-
const allowList = normalizeDiscordAllowList(effectiveAllowFrom, ["discord:", "user:", "pk:"]);
454-
const allowMatch = allowList
455-
? resolveDiscordAllowListMatch({
456-
allowList,
457-
candidate: {
458-
id: user.id,
459-
name: user.username,
460-
tag: formatDiscordUserTag(user),
461-
},
462-
allowNameMatching: isDangerousNameMatchingEnabled(ctx.discordConfig),
463-
})
464-
: { allowed: false };
482+
const allowMatch = resolveAllowMatch([...(ctx.allowFrom ?? []), ...storeAllowFrom]);
465483
if (allowMatch.allowed) {
466484
return true;
467485
}

extensions/discord/src/monitor/monitor.test.ts

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,14 @@ describe("agent components", () => {
191191
expect(reply).toHaveBeenCalledTimes(1);
192192
expect(reply.mock.calls[0]?.[0]?.content).toContain("Pairing code: PAIRCODE");
193193
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
194+
expect(readAllowFromStoreMock).toHaveBeenCalledWith({
195+
provider: "discord",
196+
accountId: "default",
197+
dmPolicy: "pairing",
198+
});
194199
});
195200

196-
it("blocks DM interactions when only pairing store entries match in allowlist mode", async () => {
197-
readAllowFromStoreMock.mockResolvedValue(["123456789"]);
201+
it("blocks DM interactions in allowlist mode when sender is not in configured allowFrom", async () => {
198202
const button = createAgentComponentButton({
199203
cfg: createCfg(),
200204
accountId: "default",
@@ -210,6 +214,62 @@ describe("agent components", () => {
210214
expect(readAllowFromStoreMock).not.toHaveBeenCalled();
211215
});
212216

217+
it("authorizes DM interactions from pairing-store entries in pairing mode", async () => {
218+
readAllowFromStoreMock.mockResolvedValue(["123456789"]);
219+
const button = createAgentComponentButton({
220+
cfg: createCfg(),
221+
accountId: "default",
222+
dmPolicy: "pairing",
223+
});
224+
const { interaction, defer, reply } = createDmButtonInteraction();
225+
226+
await button.run(interaction, { componentId: "hello" } as ComponentData);
227+
228+
expect(defer).toHaveBeenCalledWith({ ephemeral: true });
229+
expect(reply).toHaveBeenCalledWith({ content: "✓" });
230+
expect(enqueueSystemEventMock).toHaveBeenCalled();
231+
expect(upsertPairingRequestMock).not.toHaveBeenCalled();
232+
expect(readAllowFromStoreMock).toHaveBeenCalledWith({
233+
provider: "discord",
234+
accountId: "default",
235+
dmPolicy: "pairing",
236+
});
237+
});
238+
239+
it("allows DM component interactions in open mode without reading pairing store", async () => {
240+
readAllowFromStoreMock.mockResolvedValue(["123456789"]);
241+
const button = createAgentComponentButton({
242+
cfg: createCfg(),
243+
accountId: "default",
244+
dmPolicy: "open",
245+
});
246+
const { interaction, defer, reply } = createDmButtonInteraction();
247+
248+
await button.run(interaction, { componentId: "hello" } as ComponentData);
249+
250+
expect(defer).toHaveBeenCalledWith({ ephemeral: true });
251+
expect(reply).toHaveBeenCalledWith({ content: "✓" });
252+
expect(enqueueSystemEventMock).toHaveBeenCalled();
253+
expect(readAllowFromStoreMock).not.toHaveBeenCalled();
254+
});
255+
256+
it("blocks DM component interactions in disabled mode without reading pairing store", async () => {
257+
readAllowFromStoreMock.mockResolvedValue(["123456789"]);
258+
const button = createAgentComponentButton({
259+
cfg: createCfg(),
260+
accountId: "default",
261+
dmPolicy: "disabled",
262+
});
263+
const { interaction, defer, reply } = createDmButtonInteraction();
264+
265+
await button.run(interaction, { componentId: "hello" } as ComponentData);
266+
267+
expect(defer).toHaveBeenCalledWith({ ephemeral: true });
268+
expect(reply).toHaveBeenCalledWith({ content: "DM interactions are disabled." });
269+
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
270+
expect(readAllowFromStoreMock).not.toHaveBeenCalled();
271+
});
272+
213273
it("matches tag-based allowlist entries for DM select menus", async () => {
214274
const select = createAgentSelectMenu({
215275
cfg: createCfg(),
@@ -225,6 +285,7 @@ describe("agent components", () => {
225285
expect(defer).toHaveBeenCalledWith({ ephemeral: true });
226286
expect(reply).toHaveBeenCalledWith({ content: "✓" });
227287
expect(enqueueSystemEventMock).toHaveBeenCalled();
288+
expect(readAllowFromStoreMock).not.toHaveBeenCalled();
228289
});
229290

230291
it("accepts cid payloads for agent button interactions", async () => {
@@ -244,6 +305,7 @@ describe("agent components", () => {
244305
expect.stringContaining("hello_cid"),
245306
expect.any(Object),
246307
);
308+
expect(readAllowFromStoreMock).not.toHaveBeenCalled();
247309
});
248310

249311
it("keeps malformed percent cid values without throwing", async () => {
@@ -263,6 +325,7 @@ describe("agent components", () => {
263325
expect.stringContaining("hello%2G"),
264326
expect.any(Object),
265327
);
328+
expect(readAllowFromStoreMock).not.toHaveBeenCalled();
266329
});
267330
});
268331

extensions/discord/src/monitor/native-command-ui.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
type DiscordModelPickerPreferenceScope,
3939
} from "./model-picker-preferences.js";
4040
import {
41+
DISCORD_MODEL_PICKER_CUSTOM_ID_KEY,
4142
loadDiscordModelPickerData,
4243
parseDiscordModelPickerData,
4344
renderDiscordModelPickerModelsView,
@@ -949,7 +950,7 @@ class DiscordCommandArgFallbackButton extends Button {
949950

950951
class DiscordModelPickerFallbackButton extends Button {
951952
label = "modelpick";
952-
customId = "modelpick:seed=btn";
953+
customId = `${DISCORD_MODEL_PICKER_CUSTOM_ID_KEY}:seed=btn`;
953954
private ctx: DiscordModelPickerContext;
954955
private safeInteractionCall: SafeDiscordInteractionCall;
955956
private dispatchCommandInteraction: DispatchDiscordCommandInteraction;
@@ -977,7 +978,7 @@ class DiscordModelPickerFallbackButton extends Button {
977978
}
978979

979980
class DiscordModelPickerFallbackSelect extends StringSelectMenu {
980-
customId = "modelpick:seed=sel";
981+
customId = `${DISCORD_MODEL_PICKER_CUSTOM_ID_KEY}:seed=sel`;
981982
options = [];
982983
private ctx: DiscordModelPickerContext;
983984
private safeInteractionCall: SafeDiscordInteractionCall;

extensions/discord/src/monitor/native-command.model-picker.test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,12 @@ describe("Discord model picker interactions", () => {
246246
const select = createDiscordModelPickerFallbackSelect(context);
247247

248248
expect(button.customId).not.toBe(select.customId);
249-
expect(button.customId.split(":")[0]).toBe(select.customId.split(":")[0]);
249+
expect(button.customId.split(":")[0]).toBe(
250+
modelPickerModule.DISCORD_MODEL_PICKER_CUSTOM_ID_KEY,
251+
);
252+
expect(select.customId.split(":")[0]).toBe(
253+
modelPickerModule.DISCORD_MODEL_PICKER_CUSTOM_ID_KEY,
254+
);
250255
});
251256

252257
it("ignores interactions from users other than the picker owner", async () => {

0 commit comments

Comments
 (0)