fix(discord): unify dm command auth gating

2026-03-01 23:50:17 +00:00
parent 577becf1ad
commit 50e2674dfc
5 changed files with 263 additions and 111 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -95,6 +95,7 @@ Docs: https://docs.openclaw.ai
 - Plugins/Discovery precedence: load bundled plugins before auto-discovered global extensions so bundled channel plugins win duplicate-ID resolution by default (explicit `plugins.load.paths` overrides remain highest precedence), with loader regression coverage. Landed from contributor PR #29710 by @Sid-Qin. Thanks @Sid-Qin.
 - Discord/Reconnect integrity: release Discord message listener lane immediately while preserving serialized handler execution, add HELLO-stall resume-first recovery with bounded fresh-identify fallback after repeated stalls, and extend lifecycle/listener regression coverage for forced reconnect scenarios. Landed from contributor PR #29508 by @cgdusek. Thanks @cgdusek.
 - Security/Skills: harden skill installer metadata parsing by rejecting unsafe installer specs (brew/node/go/uv/download) and constrain plugin-declared skill directories to the plugin root (including symlink-escape checks), with regression coverage.
+- Discord/DM command auth: unify DM allowlist + pairing-store authorization across message preflight and native command interactions so DM command gating is consistent for `open`/`pairing`/`allowlist` policies.
 - ACP/Harness thread spawn routing: force ACP harness thread creation through `sessions_spawn` (`runtime: "acp"`, `thread: true`) and explicitly forbid `message action=thread-create` for ACP harness requests, avoiding misrouted `Unknown channel` errors. (#30957) Thanks @dutifulbob.
 - Docs/ACP permissions: document the correct `permissionMode` default (`approve-reads`) and clarify non-interactive permission failure behavior/troubleshooting guidance. (#31044) Thanks @barronlroth.
 - Security/Logging utility hardening: remove `eval`-based command execution from `scripts/clawlog.sh`, switch to argv-safe command construction, and escape predicate literals for user-supplied search/category filters to block local command/predicate injection paths.
--- a/src/discord/monitor/dm-command-auth.test.ts
+++ b/src/discord/monitor/dm-command-auth.test.ts
@@ -0,0 +1,85 @@
+import { describe, expect, it } from "vitest";
+import { resolveDiscordDmCommandAccess } from "./dm-command-auth.js";
+
+describe("resolveDiscordDmCommandAccess", () => {
+  const sender = {
+    id: "123",
+    name: "alice",
+    tag: "alice#0001",
+  };
+
+  it("allows open DMs and keeps command auth enabled without allowlist entries", async () => {
+    const result = await resolveDiscordDmCommandAccess({
+      accountId: "default",
+      dmPolicy: "open",
+      configuredAllowFrom: [],
+      sender,
+      allowNameMatching: false,
+      useAccessGroups: true,
+      readStoreAllowFrom: async () => [],
+    });
+
+    expect(result.decision).toBe("allow");
+    expect(result.commandAuthorized).toBe(true);
+  });
+
+  it("marks command auth true when sender is allowlisted", async () => {
+    const result = await resolveDiscordDmCommandAccess({
+      accountId: "default",
+      dmPolicy: "open",
+      configuredAllowFrom: ["discord:123"],
+      sender,
+      allowNameMatching: false,
+      useAccessGroups: true,
+      readStoreAllowFrom: async () => [],
+    });
+
+    expect(result.decision).toBe("allow");
+    expect(result.commandAuthorized).toBe(true);
+  });
+
+  it("returns pairing decision and unauthorized command auth for unknown senders", async () => {
+    const result = await resolveDiscordDmCommandAccess({
+      accountId: "default",
+      dmPolicy: "pairing",
+      configuredAllowFrom: ["discord:456"],
+      sender,
+      allowNameMatching: false,
+      useAccessGroups: true,
+      readStoreAllowFrom: async () => [],
+    });
+
+    expect(result.decision).toBe("pairing");
+    expect(result.commandAuthorized).toBe(false);
+  });
+
+  it("authorizes sender from pairing-store allowlist entries", async () => {
+    const result = await resolveDiscordDmCommandAccess({
+      accountId: "default",
+      dmPolicy: "pairing",
+      configuredAllowFrom: [],
+      sender,
+      allowNameMatching: false,
+      useAccessGroups: true,
+      readStoreAllowFrom: async () => ["discord:123"],
+    });
+
+    expect(result.decision).toBe("allow");
+    expect(result.commandAuthorized).toBe(true);
+  });
+
+  it("keeps open DM command auth true when access groups are disabled", async () => {
+    const result = await resolveDiscordDmCommandAccess({
+      accountId: "default",
+      dmPolicy: "open",
+      configuredAllowFrom: [],
+      sender,
+      allowNameMatching: false,
+      useAccessGroups: false,
+      readStoreAllowFrom: async () => [],
+    });
+
+    expect(result.decision).toBe("allow");
+    expect(result.commandAuthorized).toBe(true);
+  });
+});
--- a/src/discord/monitor/dm-command-auth.ts
+++ b/src/discord/monitor/dm-command-auth.ts
@@ -0,0 +1,87 @@
+import { resolveCommandAuthorizedFromAuthorizers } from "../../channels/command-gating.js";
+import {
+  readStoreAllowFromForDmPolicy,
+  resolveDmGroupAccessWithLists,
+  type DmGroupAccessDecision,
+} from "../../security/dm-policy-shared.js";
+import { normalizeDiscordAllowList, resolveDiscordAllowListMatch } from "./allow-list.js";
+
+const DISCORD_ALLOW_LIST_PREFIXES = ["discord:", "user:", "pk:"];
+
+export type DiscordDmPolicy = "open" | "pairing" | "allowlist" | "disabled";
+
+export type DiscordDmCommandAccess = {
+  decision: DmGroupAccessDecision;
+  reason: string;
+  commandAuthorized: boolean;
+  allowMatch: ReturnType<typeof resolveDiscordAllowListMatch> | { allowed: false };
+};
+
+export async function resolveDiscordDmCommandAccess(params: {
+  accountId: string;
+  dmPolicy: DiscordDmPolicy;
+  configuredAllowFrom: string[];
+  sender: { id: string; name?: string; tag?: string };
+  allowNameMatching: boolean;
+  useAccessGroups: boolean;
+  readStoreAllowFrom?: () => Promise<string[]>;
+}): Promise<DiscordDmCommandAccess> {
+  const storeAllowFrom = params.readStoreAllowFrom
+    ? await params.readStoreAllowFrom().catch(() => [])
+    : await readStoreAllowFromForDmPolicy({
+        provider: "discord",
+        accountId: params.accountId,
+        dmPolicy: params.dmPolicy,
+      });
+
+  const access = resolveDmGroupAccessWithLists({
+    isGroup: false,
+    dmPolicy: params.dmPolicy,
+    allowFrom: params.configuredAllowFrom,
+    groupAllowFrom: [],
+    storeAllowFrom,
+    isSenderAllowed: (allowEntries) => {
+      const allowList = normalizeDiscordAllowList(allowEntries, DISCORD_ALLOW_LIST_PREFIXES);
+      const allowMatch = allowList
+        ? resolveDiscordAllowListMatch({
+            allowList,
+            candidate: params.sender,
+            allowNameMatching: params.allowNameMatching,
+          })
+        : { allowed: false };
+      return allowMatch.allowed;
+    },
+  });
+
+  const commandAllowList = normalizeDiscordAllowList(
+    access.effectiveAllowFrom,
+    DISCORD_ALLOW_LIST_PREFIXES,
+  );
+  const allowMatch = commandAllowList
+    ? resolveDiscordAllowListMatch({
+        allowList: commandAllowList,
+        candidate: params.sender,
+        allowNameMatching: params.allowNameMatching,
+      })
+    : { allowed: false };
+
+  const commandAuthorized = resolveCommandAuthorizedFromAuthorizers({
+    useAccessGroups: params.useAccessGroups,
+    authorizers: [
+      {
+        configured: access.effectiveAllowFrom.length > 0,
+        allowed: allowMatch.allowed,
+      },
+    ],
+    modeWhenAccessGroupsOff: "configured",
+  });
+  const effectiveCommandAuthorized =
+    access.decision === "allow" && params.dmPolicy === "open" ? true : commandAuthorized;
+
+  return {
+    decision: access.decision,
+    reason: access.reason,
+    commandAuthorized: effectiveCommandAuthorized,
+    allowMatch,
+  };
+}
--- a/src/discord/monitor/message-handler.preflight.ts
+++ b/src/discord/monitor/message-handler.preflight.ts
@@ -28,7 +28,6 @@ import { buildPairingReply } from "../../pairing/pairing-messages.js";
 import { upsertChannelPairingRequest } from "../../pairing/pairing-store.js";
 import { resolveAgentRoute } from "../../routing/resolve-route.js";
 import { DEFAULT_ACCOUNT_ID, resolveAgentIdFromSessionKey } from "../../routing/session-key.js";
-import { readStoreAllowFromForDmPolicy } from "../../security/dm-policy-shared.js";
 import { fetchPluralKitMessageInfo } from "../pluralkit.js";
 import { sendMessageDiscord } from "../send.js";
 import {
@@ -36,13 +35,13 @@ import {
  isDiscordGroupAllowedByPolicy,
  normalizeDiscordAllowList,
  normalizeDiscordSlug,
-  resolveDiscordAllowListMatch,
  resolveDiscordChannelConfigWithFallback,
  resolveDiscordGuildEntry,
  resolveDiscordMemberAccessState,
  resolveDiscordShouldRequireMention,
  resolveGroupDmAllow,
 } from "./allow-list.js";
+import { resolveDiscordDmCommandAccess } from "./dm-command-auth.js";
 import {
  formatDiscordUserTag,
  resolveDiscordSystemLocation,
@@ -174,6 +173,7 @@ export async function preflightDiscordMessage(
  }

  const dmPolicy = params.discordConfig?.dmPolicy ?? params.discordConfig?.dm?.policy ?? "pairing";
+  const useAccessGroups = params.cfg.commands?.useAccessGroups !== false;
  const resolvedAccountId = params.accountId ?? DEFAULT_ACCOUNT_ID;
  let commandAuthorized = true;
  if (isDirectMessage) {
@@ -181,69 +181,61 @@ export async function preflightDiscordMessage(
      logVerbose("discord: drop dm (dmPolicy: disabled)");
      return null;
    }
-    if (dmPolicy !== "open") {
-      const storeAllowFrom = await readStoreAllowFromForDmPolicy({
-        provider: "discord",
-        accountId: resolvedAccountId,
-        dmPolicy,
-      });
-      const effectiveAllowFrom = [...(params.allowFrom ?? []), ...storeAllowFrom];
-      const allowList = normalizeDiscordAllowList(effectiveAllowFrom, ["discord:", "user:", "pk:"]);
-      const allowMatch = allowList
-        ? resolveDiscordAllowListMatch({
-            allowList,
-            candidate: {
-              id: sender.id,
-              name: sender.name,
-              tag: sender.tag,
-            },
-            allowNameMatching: isDangerousNameMatchingEnabled(params.discordConfig),
-          })
-        : { allowed: false };
-      const allowMatchMeta = formatAllowlistMatchMeta(allowMatch);
-      const permitted = allowMatch.allowed;
-      if (!permitted) {
-        commandAuthorized = false;
-        if (dmPolicy === "pairing") {
-          const { code, created } = await upsertChannelPairingRequest({
-            channel: "discord",
-            id: author.id,
-            accountId: resolvedAccountId,
-            meta: {
-              tag: formatDiscordUserTag(author),
-              name: author.username ?? undefined,
-            },
-          });
-          if (created) {
-            logVerbose(
-              `discord pairing request sender=${author.id} tag=${formatDiscordUserTag(author)} (${allowMatchMeta})`,
-            );
-            try {
-              await sendMessageDiscord(
-                `user:${author.id}`,
-                buildPairingReply({
-                  channel: "discord",
-                  idLine: `Your Discord user id: ${author.id}`,
-                  code,
-                }),
-                {
-                  token: params.token,
-                  rest: params.client.rest,
-                  accountId: params.accountId,
-                },
-              );
-            } catch (err) {
-              logVerbose(`discord pairing reply failed for ${author.id}: ${String(err)}`);
-            }
-          }
-        } else {
+    const dmAccess = await resolveDiscordDmCommandAccess({
+      accountId: resolvedAccountId,
+      dmPolicy,
+      configuredAllowFrom: params.allowFrom ?? [],
+      sender: {
+        id: sender.id,
+        name: sender.name,
+        tag: sender.tag,
+      },
+      allowNameMatching: isDangerousNameMatchingEnabled(params.discordConfig),
+      useAccessGroups,
+    });
+    commandAuthorized = dmAccess.commandAuthorized;
+    if (dmAccess.decision !== "allow") {
+      const allowMatchMeta = formatAllowlistMatchMeta(
+        dmAccess.allowMatch.allowed ? dmAccess.allowMatch : undefined,
+      );
+      if (dmAccess.decision === "pairing") {
+        const { code, created } = await upsertChannelPairingRequest({
+          channel: "discord",
+          id: author.id,
+          accountId: resolvedAccountId,
+          meta: {
+            tag: formatDiscordUserTag(author),
+            name: author.username ?? undefined,
+          },
+        });
+        if (created) {
          logVerbose(
-            `Blocked unauthorized discord sender ${sender.id} (dmPolicy=${dmPolicy}, ${allowMatchMeta})`,
+            `discord pairing request sender=${author.id} tag=${formatDiscordUserTag(author)} (${allowMatchMeta})`,
          );
+          try {
+            await sendMessageDiscord(
+              `user:${author.id}`,
+              buildPairingReply({
+                channel: "discord",
+                idLine: `Your Discord user id: ${author.id}`,
+                code,
+              }),
+              {
+                token: params.token,
+                rest: params.client.rest,
+                accountId: params.accountId,
+              },
+            );
+          } catch (err) {
+            logVerbose(`discord pairing reply failed for ${author.id}: ${String(err)}`);
+          }
        }
-        return null;
+      } else {
+        logVerbose(
+          `Blocked unauthorized discord sender ${sender.id} (dmPolicy=${dmPolicy}, ${allowMatchMeta})`,
+        );
      }
-      commandAuthorized = true;
+      return null;
    }
  }

@@ -598,7 +590,6 @@ export async function preflightDiscordMessage(
          { allowNameMatching: isDangerousNameMatchingEnabled(params.discordConfig) },
        )
      : false;
-    const useAccessGroups = params.cfg.commands?.useAccessGroups !== false;
    const commandGate = resolveControlCommandGate({
      useAccessGroups,
      authorizers: [
--- a/src/discord/monitor/native-command.ts
+++ b/src/discord/monitor/native-command.ts
@@ -50,7 +50,6 @@ import { upsertChannelPairingRequest } from "../../pairing/pairing-store.js";
 import { resolveAgentRoute } from "../../routing/resolve-route.js";
 import { resolveAgentIdFromSessionKey } from "../../routing/session-key.js";
 import { buildUntrustedChannelMetadata } from "../../security/channel-metadata.js";
-import { readStoreAllowFromForDmPolicy } from "../../security/dm-policy-shared.js";
 import { chunkItems } from "../../utils/chunk-items.js";
 import { withTimeout } from "../../utils/with-timeout.js";
 import { loadWebMedia } from "../../web/media.js";
@@ -65,6 +64,7 @@ import {
  resolveDiscordMemberAccessState,
  resolveDiscordOwnerAllowFrom,
 } from "./allow-list.js";
+import { resolveDiscordDmCommandAccess } from "./dm-command-auth.js";
 import { resolveDiscordChannelInfo } from "./message-utils.js";
 import {
  readDiscordModelPickerRecentModels,
@@ -1357,56 +1357,44 @@ async function dispatchDiscordCommandInteraction(params: {
      await respond("Discord DMs are disabled.");
      return;
    }
-    if (dmPolicy !== "open") {
-      const storeAllowFrom = await readStoreAllowFromForDmPolicy({
-        provider: "discord",
-        accountId,
-        dmPolicy,
-      });
-      const effectiveAllowFrom = [
-        ...(discordConfig?.allowFrom ?? discordConfig?.dm?.allowFrom ?? []),
-        ...storeAllowFrom,
-      ];
-      const allowList = normalizeDiscordAllowList(effectiveAllowFrom, ["discord:", "user:", "pk:"]);
-      const permitted = allowList
-        ? allowListMatches(
-            allowList,
-            {
-              id: sender.id,
-              name: sender.name,
-              tag: sender.tag,
-            },
-            { allowNameMatching: isDangerousNameMatchingEnabled(discordConfig) },
-          )
-        : false;
-      if (!permitted) {
-        commandAuthorized = false;
-        if (dmPolicy === "pairing") {
-          const { code, created } = await upsertChannelPairingRequest({
-            channel: "discord",
-            id: user.id,
-            accountId,
-            meta: {
-              tag: sender.tag,
-              name: sender.name,
-            },
-          });
-          if (created) {
-            await respond(
-              buildPairingReply({
-                channel: "discord",
-                idLine: `Your Discord user id: ${user.id}`,
-                code,
-              }),
-              { ephemeral: true },
-            );
-          }
-        } else {
-          await respond("You are not authorized to use this command.", { ephemeral: true });
+    const dmAccess = await resolveDiscordDmCommandAccess({
+      accountId,
+      dmPolicy,
+      configuredAllowFrom: discordConfig?.allowFrom ?? discordConfig?.dm?.allowFrom ?? [],
+      sender: {
+        id: sender.id,
+        name: sender.name,
+        tag: sender.tag,
+      },
+      allowNameMatching: isDangerousNameMatchingEnabled(discordConfig),
+      useAccessGroups,
+    });
+    commandAuthorized = dmAccess.commandAuthorized;
+    if (dmAccess.decision !== "allow") {
+      if (dmAccess.decision === "pairing") {
+        const { code, created } = await upsertChannelPairingRequest({
+          channel: "discord",
+          id: user.id,
+          accountId,
+          meta: {
+            tag: sender.tag,
+            name: sender.name,
+          },
+        });
+        if (created) {
+          await respond(
+            buildPairingReply({
+              channel: "discord",
+              idLine: `Your Discord user id: ${user.id}`,
+              code,
+            }),
+            { ephemeral: true },
+          );
        }
-        return;
+      } else {
+        await respond("You are not authorized to use this command.", { ephemeral: true });
      }
-      commandAuthorized = true;
+      return;
    }
  }
  if (!isDirectMessage) {