Discord: enforce approver checks for text approvals (#56015)

* Discord: gate text approvals by approver policy

* Discord: require approvers for plugin text approvals

* Discord: preserve legacy text approval fallback

Author: jacobtomlinson

extensions/discord/src/exec-approvals.ts

@@ -11,6 +11,19 @@ export function isDiscordExecApprovalClientEnabled(params: {
   return Boolean(config?.enabled && (config.approvers?.length ?? 0) > 0);
 }
 
+export function isDiscordExecApprovalApprover(params: {
+  cfg: OpenClawConfig;
+  accountId?: string | null;
+  senderId?: string | null;
+}): boolean {
+  const senderId = params.senderId?.trim();
+  if (!senderId) {
+    return false;
+  }
+  const approvers = resolveDiscordAccount(params).config.execApprovals?.approvers ?? [];
+  return approvers.some((approverId) => String(approverId) === senderId);
+}
+
 export function shouldSuppressLocalDiscordExecApprovalPrompt(params: {
   cfg: OpenClawConfig;
   accountId?: string | null;

src/auto-reply/reply/commands-approve.ts

@@ -1,3 +1,7 @@
+import {
+  isDiscordExecApprovalApprover,
+  isDiscordExecApprovalClientEnabled,
+} from "../../../extensions/discord/api.js";
 import { callGateway } from "../../gateway/call.js";
 import { ErrorCodes } from "../../gateway/protocol/index.js";
 import { logVerbose } from "../../globals.js";
@@ -126,6 +130,8 @@ export const handleApproveCommand: CommandHandler = async (params, allowTextComm
     return { shouldContinue: false, reply: { text: parsed.error } };
   }
   const isPluginId = parsed.id.startsWith("plugin:");
+  let discordExecApprovalDeniedReply: { shouldContinue: false; reply: { text: string } } | null =
+    null;
 
   if (params.command.channel === "telegram") {
     const telegramApproverContext = {
@@ -162,6 +168,44 @@ export const handleApproveCommand: CommandHandler = async (params, allowTextComm
     }
   }
 
+  if (params.command.channel === "discord" && !isPluginId) {
+    const discordApproverContext = {
+      cfg: params.cfg,
+      accountId: params.ctx.AccountId,
+      senderId: params.command.senderId,
+    };
+    if (!isDiscordExecApprovalClientEnabled(discordApproverContext)) {
+      discordExecApprovalDeniedReply = {
+        shouldContinue: false,
+        reply: { text: "❌ Discord exec approvals are not enabled for this bot account." },
+      };
+    }
+    if (!discordExecApprovalDeniedReply && !isDiscordExecApprovalApprover(discordApproverContext)) {
+      discordExecApprovalDeniedReply = {
+        shouldContinue: false,
+        reply: { text: "❌ You are not authorized to approve exec requests on Discord." },
+      };
+    }
+  }
+
+  // Keep plugin-ID routing independent from exec approval client enablement so
+  // forwarded plugin approvals remain resolvable, but still require explicit
+  // Discord approver membership for security parity.
+  if (
+    params.command.channel === "discord" &&
+    isPluginId &&
+    !isDiscordExecApprovalApprover({
+      cfg: params.cfg,
+      accountId: params.ctx.AccountId,
+      senderId: params.command.senderId,
+    })
+  ) {
+    return {
+      shouldContinue: false,
+      reply: { text: "❌ You are not authorized to approve plugin requests on Discord." },
+    };
+  }
+
   const missingScope = requireGatewayClientScopeForInternalChannel(params, {
     label: "/approve",
     allowedScopes: ["operator.approvals", "operator.admin"],
@@ -194,6 +238,25 @@ export const handleApproveCommand: CommandHandler = async (params, allowTextComm
       };
     }
   } else {
+    if (discordExecApprovalDeniedReply) {
+      // Preserve the legacy unprefixed plugin fallback on Discord even when
+      // exec approvals are unavailable to this sender.
+      try {
+        await callApprovalMethod("plugin.approval.resolve");
+      } catch (pluginErr) {
+        if (isApprovalNotFoundError(pluginErr)) {
+          return discordExecApprovalDeniedReply;
+        }
+        return {
+          shouldContinue: false,
+          reply: { text: `❌ Failed to submit approval: ${String(pluginErr)}` },
+        };
+      }
+      return {
+        shouldContinue: false,
+        reply: { text: `✅ Approval ${parsed.decision} submitted for ${parsed.id}.` },
+      };
+    }
     try {
       await callApprovalMethod("exec.approval.resolve");
     } catch (err) {

src/auto-reply/reply/commands.test.ts

@@ -361,6 +361,24 @@ describe("/approve command", () => {
     } as OpenClawConfig;
   }
 
+  function createDiscordApproveCfg(
+    execApprovals: {
+      enabled: boolean;
+      approvers: string[];
+      target: "dm" | "channel" | "both";
+    } | null = { enabled: true, approvers: ["123"], target: "channel" },
+  ): OpenClawConfig {
+    return {
+      commands: { text: true },
+      channels: {
+        discord: {
+          allowFrom: ["*"],
+          ...(execApprovals ? { execApprovals } : {}),
+        },
+      },
+    } as OpenClawConfig;
+  }
+
   it("rejects invalid usage", async () => {
     const cfg = {
       commands: { text: true },
@@ -414,6 +432,139 @@ describe("/approve command", () => {
     );
   });
 
+  it("requires configured Discord approvers for exec approvals", async () => {
+    for (const testCase of [
+      {
+        name: "discord approvals disabled",
+        cfg: createDiscordApproveCfg(null),
+        senderId: "123",
+        expectedText: "Discord exec approvals are not enabled",
+        setup: () =>
+          callGatewayMock.mockRejectedValue(
+            gatewayError("unknown or expired approval id", "APPROVAL_NOT_FOUND"),
+          ),
+        expectedGatewayCalls: 1,
+      },
+      {
+        name: "discord non approver",
+        cfg: createDiscordApproveCfg({ enabled: true, approvers: ["999"], target: "channel" }),
+        senderId: "123",
+        expectedText: "not authorized to approve",
+        setup: () =>
+          callGatewayMock.mockRejectedValue(
+            gatewayError("unknown or expired approval id", "APPROVAL_NOT_FOUND"),
+          ),
+        expectedGatewayCalls: 1,
+      },
+      {
+        name: "discord approver",
+        cfg: createDiscordApproveCfg({ enabled: true, approvers: ["123"], target: "channel" }),
+        senderId: "123",
+        expectedText: "Approval allow-once submitted",
+        setup: () => callGatewayMock.mockResolvedValue({ ok: true }),
+        expectedGatewayCalls: 1,
+      },
+    ] as const) {
+      callGatewayMock.mockReset();
+      testCase.setup();
+      const params = buildParams("/approve abc12345 allow-once", testCase.cfg, {
+        Provider: "discord",
+        Surface: "discord",
+        SenderId: testCase.senderId,
+      });
+
+      const result = await handleCommands(params);
+      expect(result.shouldContinue, testCase.name).toBe(false);
+      expect(result.reply?.text, testCase.name).toContain(testCase.expectedText);
+      expect(callGatewayMock, testCase.name).toHaveBeenCalledTimes(testCase.expectedGatewayCalls);
+      if (testCase.expectedGatewayCalls > 0) {
+        expect(callGatewayMock, testCase.name).toHaveBeenCalledWith(
+          expect.objectContaining({
+            method:
+              testCase.name === "discord approver"
+                ? "exec.approval.resolve"
+                : "plugin.approval.resolve",
+            params: { id: "abc12345", decision: "allow-once" },
+          }),
+        );
+      }
+    }
+  });
+
+  it("preserves legacy unprefixed plugin approval fallback on Discord", async () => {
+    for (const testCase of [
+      {
+        name: "discord legacy plugin approval with exec approvals disabled",
+        cfg: createDiscordApproveCfg(null),
+        senderId: "123",
+      },
+      {
+        name: "discord legacy plugin approval for non approver",
+        cfg: createDiscordApproveCfg({ enabled: true, approvers: ["999"], target: "channel" }),
+        senderId: "123",
+      },
+    ] as const) {
+      callGatewayMock.mockReset();
+      callGatewayMock.mockResolvedValue({ ok: true });
+      const params = buildParams("/approve legacy-plugin-123 allow-once", testCase.cfg, {
+        Provider: "discord",
+        Surface: "discord",
+        SenderId: testCase.senderId,
+      });
+
+      const result = await handleCommands(params);
+      expect(result.shouldContinue, testCase.name).toBe(false);
+      expect(result.reply?.text, testCase.name).toContain("Approval allow-once submitted");
+      expect(callGatewayMock, testCase.name).toHaveBeenCalledTimes(1);
+      expect(callGatewayMock, testCase.name).toHaveBeenCalledWith(
+        expect.objectContaining({
+          method: "plugin.approval.resolve",
+          params: { id: "legacy-plugin-123", decision: "allow-once" },
+        }),
+      );
+    }
+  });
+
+  it("requires configured Discord approvers for plugin approvals", async () => {
+    for (const testCase of [
+      {
+        name: "discord plugin non approver",
+        cfg: createDiscordApproveCfg({ enabled: false, approvers: ["999"], target: "channel" }),
+        senderId: "123",
+        expectedText: "not authorized to approve plugin requests",
+        expectedGatewayCalls: 0,
+      },
+      {
+        name: "discord plugin approver",
+        cfg: createDiscordApproveCfg({ enabled: false, approvers: ["123"], target: "channel" }),
+        senderId: "123",
+        expectedText: "Approval allow-once submitted",
+        expectedGatewayCalls: 1,
+      },
+    ] as const) {
+      callGatewayMock.mockReset();
+      callGatewayMock.mockResolvedValue({ ok: true });
+      const params = buildParams("/approve plugin:abc123 allow-once", testCase.cfg, {
+        Provider: "discord",
+        Surface: "discord",
+        SenderId: testCase.senderId,
+      });
+
+      const result = await handleCommands(params);
+      expect(result.shouldContinue, testCase.name).toBe(false);
+      expect(result.reply?.text, testCase.name).toContain(testCase.expectedText);
+      expect(callGatewayMock, testCase.name).toHaveBeenCalledTimes(testCase.expectedGatewayCalls);
+      if (testCase.expectedGatewayCalls > 0) {
+        expect(callGatewayMock, testCase.name).toHaveBeenCalledWith(
+          expect.objectContaining({
+            method: "plugin.approval.resolve",
+            params: { id: "plugin:abc123", decision: "allow-once" },
+          }),
+        );
+      }
+    }
+  });
+
   it("rejects unauthorized or invalid Telegram /approve variants", async () => {
     for (const testCase of [
       {