From 6eb42593fa0e983402fa449e45c06d792c10bc6e Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 31 Mar 2026 16:45:46 +0900 Subject: [PATCH] fix(slack): restore plugin approval auth --- extensions/slack/src/approval-auth.test.ts | 28 ++++++++++++- extensions/slack/src/approval-auth.ts | 36 +++++++++++++--- extensions/slack/src/approval-native.test.ts | 44 ++++++++++++++++++++ extensions/slack/src/approval-native.ts | 4 +- 4 files changed, 103 insertions(+), 9 deletions(-) diff --git a/extensions/slack/src/approval-auth.test.ts b/extensions/slack/src/approval-auth.test.ts index f224c9b7ace..958f0fe8942 100644 --- a/extensions/slack/src/approval-auth.test.ts +++ b/extensions/slack/src/approval-auth.test.ts @@ -2,11 +2,14 @@ import { describe, expect, it } from "vitest"; import { slackApprovalAuth } from "./approval-auth.js"; describe("slackApprovalAuth", () => { - it("authorizes inferred Slack approvers by user id", () => { + it("authorizes general Slack approvers from allowFrom and defaultTo", () => { const cfg = { channels: { slack: { - execApprovals: { enabled: true, approvers: ["user:U123OWNER"] }, + allowFrom: ["slack:U123OWNER"], + dm: { allowFrom: ["<@U234DM>"] }, + defaultTo: "user:U345DEFAULT", + execApprovals: { enabled: true, approvers: ["user:U999EXEC"] }, }, }, }; @@ -20,6 +23,27 @@ describe("slackApprovalAuth", () => { }), ).toEqual({ authorized: true }); + expect( + slackApprovalAuth.authorizeActorAction({ + cfg, + senderId: "U345DEFAULT", + action: "approve", + approvalKind: "plugin", + }), + ).toEqual({ authorized: true }); + + expect( + slackApprovalAuth.authorizeActorAction({ + cfg, + senderId: "U999EXEC", + action: "approve", + approvalKind: "plugin", + }), + ).toEqual({ + authorized: false, + reason: "❌ You are not authorized to approve plugin requests on Slack.", + }); + expect( slackApprovalAuth.authorizeActorAction({ cfg, diff --git a/extensions/slack/src/approval-auth.ts b/extensions/slack/src/approval-auth.ts index 0254ce1831f..4a6a2a4789c 100644 --- a/extensions/slack/src/approval-auth.ts +++ b/extensions/slack/src/approval-auth.ts @@ -1,13 +1,39 @@ import { createResolvedApproverActionAuthAdapter, + resolveApprovalApprovers, } from "openclaw/plugin-sdk/approval-runtime"; -import { - getSlackExecApprovalApprovers, - normalizeSlackApproverId, -} from "./exec-approvals.js"; +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; +import { resolveSlackAccount } from "./accounts.js"; +import { normalizeSlackApproverId } from "./exec-approvals.js"; + +export function getSlackApprovalApprovers(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): string[] { + const account = resolveSlackAccount(params).config; + return resolveApprovalApprovers({ + allowFrom: account.allowFrom, + extraAllowFrom: account.dm?.allowFrom, + defaultTo: account.defaultTo, + normalizeApprover: normalizeSlackApproverId, + normalizeDefaultTo: normalizeSlackApproverId, + }); +} + +export function isSlackApprovalAuthorizedSender(params: { + cfg: OpenClawConfig; + accountId?: string | null; + senderId?: string | null; +}): boolean { + const senderId = params.senderId ? normalizeSlackApproverId(params.senderId) : undefined; + if (!senderId) { + return false; + } + return getSlackApprovalApprovers(params).includes(senderId); +} export const slackApprovalAuth = createResolvedApproverActionAuthAdapter({ channelLabel: "Slack", - resolveApprovers: ({ cfg, accountId }) => getSlackExecApprovalApprovers({ cfg, accountId }), + resolveApprovers: ({ cfg, accountId }) => getSlackApprovalApprovers({ cfg, accountId }), normalizeSenderId: (value) => normalizeSlackApproverId(value), }); diff --git a/extensions/slack/src/approval-native.test.ts b/extensions/slack/src/approval-native.test.ts index 8d5e16871c9..6a65fcb5d41 100644 --- a/extensions/slack/src/approval-native.test.ts +++ b/extensions/slack/src/approval-native.test.ts @@ -200,4 +200,48 @@ describe("slack native approval adapter", () => { }), ).toBe(false); }); + + it("keeps plugin approval auth independent from exec approvers", () => { + const cfg = buildConfig({ + allowFrom: ["U123OWNER"], + execApprovals: { + enabled: true, + approvers: ["U999EXEC"], + target: "both", + }, + }); + + expect( + slackNativeApprovalAdapter.auth.authorizeActorAction({ + cfg, + accountId: "default", + senderId: "U123OWNER", + action: "approve", + approvalKind: "plugin", + }), + ).toEqual({ authorized: true }); + + expect( + slackNativeApprovalAdapter.auth.authorizeActorAction({ + cfg, + accountId: "default", + senderId: "U999EXEC", + action: "approve", + approvalKind: "plugin", + }), + ).toEqual({ + authorized: false, + reason: "❌ You are not authorized to approve plugin requests on Slack.", + }); + + expect( + slackNativeApprovalAdapter.auth.authorizeActorAction({ + cfg, + accountId: "default", + senderId: "U999EXEC", + action: "approve", + approvalKind: "exec", + }), + ).toEqual({ authorized: true }); + }); }); diff --git a/extensions/slack/src/approval-native.ts b/extensions/slack/src/approval-native.ts index 6c5d1c085c9..bd1283c862a 100644 --- a/extensions/slack/src/approval-native.ts +++ b/extensions/slack/src/approval-native.ts @@ -10,9 +10,9 @@ import type { } from "openclaw/plugin-sdk/infra-runtime"; import { normalizeAccountId } from "openclaw/plugin-sdk/routing"; import { listSlackAccountIds } from "./accounts.js"; +import { isSlackApprovalAuthorizedSender } from "./approval-auth.js"; import { getSlackExecApprovalApprovers, - isSlackExecApprovalApprover, isSlackExecApprovalAuthorizedSender, isSlackExecApprovalClientEnabled, resolveSlackExecApprovalTarget, @@ -197,7 +197,7 @@ export const slackNativeApprovalAdapter = createApproverRestrictedNativeApproval isExecAuthorizedSender: ({ cfg, accountId, senderId }) => isSlackExecApprovalAuthorizedSender({ cfg, accountId, senderId }), isPluginAuthorizedSender: ({ cfg, accountId, senderId }) => - isSlackExecApprovalApprover({ cfg, accountId, senderId }), + isSlackApprovalAuthorizedSender({ cfg, accountId, senderId }), isNativeDeliveryEnabled: ({ cfg, accountId }) => isSlackExecApprovalClientEnabled({ cfg, accountId }), resolveNativeDeliveryMode: ({ cfg, accountId }) =>