fix(discord): disambiguate allow-from DM targets (#74303)

* fix(discord): disambiguate allow-from DM targets

* test(discord): align allowFrom precedence

* docs(discord): clarify allowFrom id forms

* fix(discord): align allowFrom precedence

---------

Co-authored-by: 따온이네 맥북프로 <tulisy@ttaon-ine-ui-MacBookPro.local>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
Squirbie
2026-04-29 21:01:28 +09:00
committed by GitHub
parent 4f540c703f
commit 94a85e77de
17 changed files with 353 additions and 14 deletions

View File

@@ -18,6 +18,7 @@ Docs: https://docs.openclaw.ai
### Fixes
- Channels/Discord: treat bare numeric outbound targets that match the effective Discord DM allowlist as user DMs while preserving account-specific legacy `dm.allowFrom` precedence over inherited root `allowFrom`. (#74303) Thanks @Squirbie.
- Agents/auth: keep OAuth auth profiles inherited from the main agent read-through instead of copying refresh tokens into secondary agents, and refresh Codex app-server tokens against the owning store so multi-agent swarms avoid reused refresh-token failures. Fixes #74055. Thanks @ClarityInvest.
- Channels/Telegram: honor `ALL_PROXY` / `all_proxy` and service-level `OPENCLAW_PROXY_URL` when constructing the HTTP/1-only Telegram Bot API transport, so Windows and service installs that rely on those proxy settings no longer fall back to direct egress. Fixes #74014; refs #74086. Thanks @SymbolStar.
- Channels/Telegram: continue polling when `deleteWebhook` hits a transient network failure but `getWebhookInfo` confirms no webhook is configured, so startup does not retry cleanup forever after the webhook was already removed. Refs #74086; carries forward #47384. Thanks @clovericbot.

View File

@@ -402,7 +402,8 @@ Example:
Multi-account precedence:
- `channels.discord.accounts.default.allowFrom` applies only to the `default` account.
- Named accounts inherit `channels.discord.allowFrom` when their own `allowFrom` is unset.
- For one account, `allowFrom` takes precedence over legacy `dm.allowFrom`.
- Named accounts inherit `channels.discord.allowFrom` when their own `allowFrom` and legacy `dm.allowFrom` are unset.
- Named accounts do not inherit `channels.discord.accounts.default.allowFrom`.
DM target format for delivery:
@@ -410,7 +411,7 @@ Example:
- `user:<id>`
- `<@id>` mention
Bare numeric IDs are ambiguous and rejected unless an explicit user/channel target kind is provided.
Bare numeric IDs normally resolve as channel IDs when a channel default is active, but IDs listed in the account's effective DM `allowFrom` are treated as user DM targets for compatibility.
</Tab>

View File

@@ -42,6 +42,26 @@ export function mergeDiscordAccountConfig(
});
}
export function resolveDiscordAccountAllowFrom(params: {
cfg: OpenClawConfig;
accountId?: string | null;
}): string[] | undefined {
const accountId = normalizeAccountId(
params.accountId ?? resolveDefaultDiscordAccountId(params.cfg),
);
const accountConfig = resolveDiscordAccountConfig(params.cfg, accountId);
const rootConfig = params.cfg.channels?.discord as DiscordAccountConfig | undefined;
// New allowFrom wins over legacy dm.allowFrom within the same scope, while an
// account-local legacy list still wins over a root allowFrom inherited by merge.
return (
accountConfig?.allowFrom ??
accountConfig?.dm?.allowFrom ??
rootConfig?.allowFrom ??
rootConfig?.dm?.allowFrom
);
}
export function createDiscordActionGate(params: {
cfg: OpenClawConfig;
accountId?: string | null;

View File

@@ -3,7 +3,11 @@ import {
createResolvedDirectoryEntriesLister,
type DirectoryConfigParams,
} from "openclaw/plugin-sdk/directory-config-runtime";
import { mergeDiscordAccountConfig, resolveDefaultDiscordAccountId } from "./accounts.js";
import {
mergeDiscordAccountConfig,
resolveDefaultDiscordAccountId,
resolveDiscordAccountAllowFrom,
} from "./accounts.js";
function resolveDiscordDirectoryConfigAccount(
cfg: DirectoryConfigParams["cfg"],
@@ -14,6 +18,7 @@ function resolveDiscordDirectoryConfigAccount(
return {
accountId: resolvedAccountId,
config,
allowFrom: resolveDiscordAccountAllowFrom({ cfg, accountId: resolvedAccountId }) ?? [],
dm: config.dm,
};
}
@@ -24,13 +29,12 @@ export const listDiscordDirectoryPeersFromConfig = createResolvedDirectoryEntrie
kind: "user",
resolveAccount: (cfg, accountId) => resolveDiscordDirectoryConfigAccount(cfg, accountId),
resolveSources: (account) => {
const allowFrom = account.config.allowFrom ?? account.config.dm?.allowFrom ?? [];
const guildUsers = Object.values(account.config.guilds ?? {}).flatMap((guild) =>
(guild.users ?? []).concat(
Object.values(guild.channels ?? {}).flatMap((channel) => channel.users ?? []),
),
);
return [allowFrom, Object.keys(account.config.dms ?? {}), guildUsers];
return [account.allowFrom, Object.keys(account.config.dms ?? {}), guildUsers];
},
normalizeId: (raw) => {
const mention = raw.match(/^<@!?(\d+)>$/);

View File

@@ -77,6 +77,29 @@ describe("Discord directory contract", () => {
await expectDirectoryIds(listDiscordDirectoryGroupsFromConfig, cfg, ["channel:555"]);
});
it("uses account legacy dm.allowFrom before inherited root allowFrom", async () => {
const cfg = {
channels: {
discord: {
allowFrom: ["<@111>"],
accounts: {
work: {
dm: { allowFrom: ["<@222>"] },
},
},
},
},
} as unknown as OpenClawConfig;
const entries = await listDiscordDirectoryPeersFromConfig({
cfg,
accountId: "work",
query: null,
limit: null,
});
expect(entries.map((entry) => entry.id)).toEqual(["user:222"]);
});
it("applies query and limit filtering for config-backed directories", async () => {
const cfg = {
channels: {

View File

@@ -27,6 +27,25 @@ describe("discord target normalization", () => {
}
});
it("treats bare outbound IDs listed in allowFrom as DM targets", () => {
expect(normalizeDiscordOutboundTarget("1234567890", ["1234567890"])).toEqual({
ok: true,
to: "user:1234567890",
});
expect(normalizeDiscordOutboundTarget("2345678901", ["user:2345678901"])).toEqual({
ok: true,
to: "user:2345678901",
});
expect(normalizeDiscordOutboundTarget("3456789012", ["<@3456789012>"])).toEqual({
ok: true,
to: "user:3456789012",
});
expect(normalizeDiscordOutboundTarget("4567890123", ["*"])).toEqual({
ok: true,
to: "channel:4567890123",
});
});
it("detects Discord-style target identifiers", () => {
expect(looksLikeDiscordTargetId("<@!123456>")).toBe(true);
expect(looksLikeDiscordTargetId("user:123456")).toBe(true);

View File

@@ -9,10 +9,12 @@ export function normalizeDiscordMessagingTarget(raw: string): string | undefined
/**
* Normalize a Discord outbound target for delivery. Bare numeric IDs are
* prefixed with "channel:" to avoid the ambiguous-target error in
* parseDiscordTarget. All other formats pass through unchanged.
* parseDiscordTarget, unless the ID is explicitly configured as an allowed DM
* sender. All other formats pass through unchanged.
*/
export function normalizeDiscordOutboundTarget(
to?: string,
allowFrom?: readonly string[],
): { ok: true; to: string } | { ok: false; error: Error } {
const trimmed = to?.trim();
if (!trimmed) {
@@ -24,11 +26,48 @@ export function normalizeDiscordOutboundTarget(
};
}
if (/^\d+$/.test(trimmed)) {
if (allowFromContainsDiscordUserId(allowFrom, trimmed)) {
return { ok: true, to: `user:${trimmed}` };
}
return { ok: true, to: `channel:${trimmed}` };
}
return { ok: true, to: trimmed };
}
export function allowFromContainsDiscordUserId(
allowFrom: readonly string[] | undefined,
userId: string,
): boolean {
const normalizedUserId = userId.trim();
if (!normalizedUserId) {
return false;
}
return (allowFrom ?? []).some(
(entry) => normalizeAllowFromDiscordUserId(entry) === normalizedUserId,
);
}
function normalizeAllowFromDiscordUserId(entry: string): string | undefined {
const trimmed = entry.trim().toLowerCase();
if (!trimmed || trimmed === "*") {
return undefined;
}
const mentionMatch = /^<@!?(\d+)>$/.exec(trimmed);
if (mentionMatch) {
return mentionMatch[1];
}
// Accept both current and legacy allowFrom forms for Discord user IDs.
const prefixedMatch = /^(?:discord:)?user:(\d+)$/.exec(trimmed);
if (prefixedMatch) {
return prefixedMatch[1];
}
const discordMatch = /^discord:(\d+)$/.exec(trimmed);
if (discordMatch) {
return discordMatch[1];
}
return /^\d+$/.test(trimmed) ? trimmed : undefined;
}
export function looksLikeDiscordTargetId(raw: string): boolean {
const trimmed = raw.trim();
if (!trimmed) {

View File

@@ -49,6 +49,13 @@ describe("normalizeDiscordOutboundTarget", () => {
it("trims whitespace", () => {
expect(normalizeDiscordOutboundTarget(" 123 ")).toEqual({ ok: true, to: "channel:123" });
});
it("normalizes bare IDs in allowFrom to user: targets", () => {
expect(normalizeDiscordOutboundTarget("1470130713209602050", ["1470130713209602050"])).toEqual({
ok: true,
to: "user:1470130713209602050",
});
});
});
describe("discordOutbound", () => {
@@ -81,6 +88,18 @@ describe("discordOutbound", () => {
).toBe("visible");
});
it("uses allowFrom to disambiguate bare numeric DM delivery targets", () => {
expect(
discordOutbound.resolveTarget?.({
to: "1470130713209602050",
allowFrom: ["1470130713209602050"],
}),
).toEqual({
ok: true,
to: "user:1470130713209602050",
});
});
it("preserves Discord-native angle markup while stripping internal scaffolding", () => {
expect(
discordOutbound.sanitizeText?.({

View File

@@ -126,7 +126,7 @@ export const discordOutbound: ChannelOutboundAdapter = {
presentation,
});
},
resolveTarget: ({ to }) => normalizeDiscordOutboundTarget(to),
resolveTarget: ({ to, allowFrom }) => normalizeDiscordOutboundTarget(to, allowFrom),
sendPayload: async (ctx) =>
await sendDiscordOutboundPayload({
ctx,

View File

@@ -1,5 +1,6 @@
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-types";
import { afterEach, describe, expect, it, vi } from "vitest";
import { createDiscordPluginBase } from "./shared.js";
import { createDiscordPluginBase, discordConfigAdapter } from "./shared.js";
afterEach(() => {
vi.unstubAllEnvs();
@@ -56,3 +57,57 @@ describe("createDiscordPluginBase", () => {
expect(plugin.config.isEnabled?.(workAccount, cfg)).toBe(true);
});
});
describe("discordConfigAdapter", () => {
it("resolves top-level allowFrom before legacy dm.allowFrom", () => {
const cfg = {
channels: {
discord: {
accounts: {
default: {
allowFrom: ["123"],
dm: { allowFrom: ["456"] },
},
},
},
},
} as OpenClawConfig;
expect(discordConfigAdapter.resolveAllowFrom?.({ cfg, accountId: "default" })).toEqual(["123"]);
});
it("falls back to legacy dm.allowFrom", () => {
const cfg = {
channels: {
discord: {
accounts: {
default: {
dm: { allowFrom: ["456"] },
},
},
},
},
} as OpenClawConfig;
expect(discordConfigAdapter.resolveAllowFrom?.({ cfg, accountId: "default" })).toEqual(["456"]);
});
it("prefers account legacy dm.allowFrom over inherited root allowFrom", () => {
const cfg = {
channels: {
discord: {
allowFrom: ["root"],
accounts: {
work: {
dm: { allowFrom: ["account-legacy"] },
},
},
},
},
} as OpenClawConfig;
expect(discordConfigAdapter.resolveAllowFrom?.({ cfg, accountId: "work" })).toEqual([
"account-legacy",
]);
});
});

View File

@@ -9,6 +9,7 @@ import {
listDiscordAccountIds,
resolveDefaultDiscordAccountId,
resolveDiscordAccount,
resolveDiscordAccountAllowFrom,
resolveDiscordAccountDisabledReason,
type ResolvedDiscordAccount,
} from "./accounts.js";
@@ -16,6 +17,7 @@ import { getChatChannelMeta, type ChannelPlugin } from "./channel-api.js";
import { DiscordChannelConfigSchema } from "./config-schema.js";
import { normalizeCompatibilityConfig } from "./doctor-contract.js";
import { DISCORD_LEGACY_CONFIG_RULES } from "./doctor-shared.js";
import type { OpenClawConfig } from "./runtime-api.js";
import {
collectRuntimeConfigAssignments,
secretTargetRegistryEntries,
@@ -30,6 +32,10 @@ import { deriveLegacySessionChatType } from "./session-contract.js";
export const DISCORD_CHANNEL = "discord" as const;
type DiscordDoctorModule = typeof import("./doctor.js");
type DiscordConfigAccessorAccount = {
allowFrom: string[] | undefined;
defaultTo: string | undefined;
};
let discordDoctorModulePromise: Promise<DiscordDoctorModule> | undefined;
@@ -56,16 +62,31 @@ const discordDoctor: ChannelDoctorAdapter = {
},
};
export const discordConfigAdapter = createScopedChannelConfigAdapter<ResolvedDiscordAccount>({
function resolveDiscordConfigAccessorAccount(params: {
cfg: OpenClawConfig;
accountId?: string | null;
}): DiscordConfigAccessorAccount {
const account = resolveDiscordAccount(params);
return {
allowFrom: resolveDiscordAccountAllowFrom({ cfg: params.cfg, accountId: account.accountId }),
defaultTo: account.config.defaultTo,
};
}
export const discordConfigAdapter = createScopedChannelConfigAdapter<
ResolvedDiscordAccount,
DiscordConfigAccessorAccount
>({
sectionKey: DISCORD_CHANNEL,
listAccountIds: listDiscordAccountIds,
resolveAccount: adaptScopedAccountAccessor(resolveDiscordAccount),
resolveAccessorAccount: resolveDiscordConfigAccessorAccount,
inspectAccount: adaptScopedAccountAccessor(inspectDiscordAccount),
defaultAccountId: resolveDefaultDiscordAccountId,
clearBaseFields: ["token", "name"],
resolveAllowFrom: (account: ResolvedDiscordAccount) => account.config.dm?.allowFrom,
resolveAllowFrom: (account) => account.allowFrom,
formatAllowFrom: (allowFrom) => formatAllowFromLowercase({ allowFrom }),
resolveDefaultTo: (account: ResolvedDiscordAccount) => account.config.defaultTo,
resolveDefaultTo: (account) => account.defaultTo,
});
export function createDiscordPluginBase(params: {

View File

@@ -1,8 +1,9 @@
import type { DirectoryConfigParams } from "openclaw/plugin-sdk/directory-runtime";
import { buildMessagingTarget, type MessagingTarget } from "openclaw/plugin-sdk/messaging-targets";
import { resolveDiscordAccount } from "./accounts.js";
import { resolveDiscordAccount, resolveDiscordAccountAllowFrom } from "./accounts.js";
import { rememberDiscordDirectoryUser } from "./directory-cache.js";
import { listDiscordDirectoryPeersLive } from "./directory-live.js";
import { allowFromContainsDiscordUserId } from "./normalize.js";
import { parseDiscordSendTarget } from "./send-target-parsing.js";
import { type DiscordTargetParseOptions } from "./target-parsing.js";
@@ -23,6 +24,14 @@ export async function resolveDiscordTarget(
const likelyUsername = isLikelyUsername(trimmed);
const shouldLookup = isExplicitUserLookup(trimmed, parseOptions) || likelyUsername;
if (
/^\d+$/.test(trimmed) &&
parseOptions.defaultKind !== "user" &&
isConfiguredAllowedDiscordDmUser(trimmed, options)
) {
return buildMessagingTarget("user", trimmed, trimmed);
}
// Parse directly if it's already a known format. Use a safe parse so ambiguous
// numeric targets don't throw when we still want to attempt username lookup.
const directParse = safeParseDiscordTarget(trimmed, parseOptions);
@@ -87,6 +96,15 @@ function safeParseDiscordTarget(
}
}
function isConfiguredAllowedDiscordDmUser(input: string, options: DirectoryConfigParams): boolean {
const allowFrom =
resolveDiscordAccountAllowFrom({
cfg: options.cfg,
accountId: options.accountId,
}) ?? [];
return allowFromContainsDiscordUserId(allowFrom, input);
}
function isExplicitUserLookup(input: string, options: DiscordTargetParseOptions): boolean {
if (/^<@!?(\d+)>$/.test(input)) {
return true;

View File

@@ -114,6 +114,85 @@ describe("resolveDiscordTarget", () => {
expect(listPeers).not.toHaveBeenCalled();
});
it("treats bare numeric ids in allowFrom as users even when channels are the default", async () => {
const listPeers = vi.spyOn(directoryLive, "listDiscordDirectoryPeersLive");
const cfg = {
channels: {
discord: {
accounts: {
default: {
allowFrom: ["123"],
},
},
},
},
} as OpenClawConfig;
await expect(
resolveDiscordTarget("123", { cfg, accountId: "default" }, { defaultKind: "channel" }),
).resolves.toMatchObject({ kind: "user", id: "123", normalized: "user:123" });
expect(listPeers).not.toHaveBeenCalled();
});
it("uses legacy dm.allowFrom when disambiguating bare numeric ids", async () => {
const cfg = {
channels: {
discord: {
accounts: {
default: {
dm: { allowFrom: ["456"] },
},
},
},
},
} as OpenClawConfig;
await expect(
resolveDiscordTarget("456", { cfg, accountId: "default" }, { defaultKind: "channel" }),
).resolves.toMatchObject({ kind: "user", id: "456", normalized: "user:456" });
});
it("prefers top-level allowFrom over legacy dm.allowFrom for bare numeric ids", async () => {
const cfg = {
channels: {
discord: {
accounts: {
default: {
allowFrom: ["123"],
dm: { allowFrom: ["456"] },
},
},
},
},
} as OpenClawConfig;
await expect(
resolveDiscordTarget("456", { cfg, accountId: "default" }, { defaultKind: "channel" }),
).resolves.toMatchObject({ kind: "channel", id: "456", normalized: "channel:456" });
});
it("uses account legacy dm.allowFrom before inherited root allowFrom for bare numeric ids", async () => {
const cfg = {
channels: {
discord: {
allowFrom: ["123"],
accounts: {
work: {
dm: { allowFrom: ["456"] },
},
},
},
},
} as OpenClawConfig;
await expect(
resolveDiscordTarget("456", { cfg, accountId: "work" }, { defaultKind: "channel" }),
).resolves.toMatchObject({ kind: "user", id: "456", normalized: "user:456" });
await expect(
resolveDiscordTarget("123", { cfg, accountId: "work" }, { defaultKind: "channel" }),
).resolves.toMatchObject({ kind: "channel", id: "123", normalized: "channel:123" });
});
it("caches username lookups under the configured default account when accountId is omitted", async () => {
const cfg = {
channels: {

View File

@@ -185,4 +185,32 @@ describe("authorizeDiscordVoiceIngress", () => {
message: "You are not authorized to use this command.",
});
});
it("uses resolved account owner allowFrom over merged Discord config", async () => {
const access = await authorizeDiscordVoiceIngress({
cfg: baseCfg,
discordConfig: {
allowFrom: ["discord:u-root"],
guilds: {
g1: {
channels: {
c1: {},
},
},
},
} as DiscordAccountConfig,
groupPolicy: "allowlist",
guildId: "g1",
channelId: "c1",
channelSlug: "",
memberRoleIds: [],
ownerAllowFrom: ["discord:u-account"],
sender: {
id: "u-account",
name: "owner",
},
});
expect(access).toEqual({ ok: true });
});
});

View File

@@ -28,6 +28,7 @@ export async function authorizeDiscordVoiceIngress(params: {
scope?: "channel" | "thread";
channelLabel?: string;
memberRoleIds: string[];
ownerAllowFrom?: string[];
sender: { id: string; name?: string; tag?: string };
}): Promise<{ ok: true } | { ok: false; message: string }> {
const groupPolicy =
@@ -96,7 +97,11 @@ export async function authorizeDiscordVoiceIngress(params: {
});
const { ownerAllowList, ownerAllowed } = resolveDiscordOwnerAccess({
allowFrom: params.discordConfig.allowFrom ?? params.discordConfig.dm?.allowFrom ?? [],
allowFrom:
params.ownerAllowFrom ??
params.discordConfig.allowFrom ??
params.discordConfig.dm?.allowFrom ??
[],
sender: params.sender,
allowNameMatching: false,
});

View File

@@ -12,6 +12,7 @@ import {
} from "discord-api-types/v10";
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-types";
import type { DiscordAccountConfig } from "openclaw/plugin-sdk/config-types";
import { resolveDiscordAccountAllowFrom } from "../accounts.js";
import { formatMention } from "../mentions.js";
import { resolveDiscordChannelNameSafe } from "../monitor/channel-access.js";
import { resolveDiscordSenderIdentity } from "../monitor/sender-identity.js";
@@ -88,6 +89,10 @@ async function authorizeVoiceCommand(
scope: channelContext.isThreadChannel ? "thread" : "channel",
channelLabel: channelId ? formatMention({ channelId }) : "This channel",
memberRoleIds,
ownerAllowFrom: resolveDiscordAccountAllowFrom({
cfg: params.cfg,
accountId: params.accountId,
}),
sender: {
id: sender.id,
name: sender.name,

View File

@@ -23,6 +23,7 @@ import { parseTtsDirectives } from "openclaw/plugin-sdk/speech";
import { formatErrorMessage } from "openclaw/plugin-sdk/ssrf-runtime";
import { resolvePreferredOpenClawTmpDir } from "openclaw/plugin-sdk/temp-path";
import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime";
import { resolveDiscordAccountAllowFrom } from "../accounts.js";
import { formatMention } from "../mentions.js";
import { normalizeDiscordSlug, resolveDiscordOwnerAccess } from "../monitor/allow-list.js";
import { formatDiscordUserTag } from "../monitor/format.js";
@@ -340,7 +341,7 @@ export class DiscordVoiceManager {
this.botUserId = params.botUserId;
this.voiceEnabled = params.discordConfig.voice?.enabled !== false;
this.ownerAllowFrom =
params.discordConfig.allowFrom ?? params.discordConfig.dm?.allowFrom ?? [];
resolveDiscordAccountAllowFrom({ cfg: params.cfg, accountId: params.accountId }) ?? [];
}
setBotUserId(id?: string) {
@@ -749,6 +750,7 @@ export class DiscordVoiceManager {
channelSlug: entry.channelName ? normalizeDiscordSlug(entry.channelName) : "",
channelLabel: formatMention({ channelId: entry.channelId }),
memberRoleIds: speakerIdentity.memberRoleIds,
ownerAllowFrom: this.ownerAllowFrom,
sender: {
id: speakerIdentity.id,
name: speakerIdentity.name,