refactor: share matched group policy evaluation

extensions/nextcloud-talk/src/policy.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { resolveNextcloudTalkAllowlistMatch } from "./policy.js";
+import { resolveNextcloudTalkAllowlistMatch, resolveNextcloudTalkGroupAllow } from "./policy.js";
 
 describe("nextcloud-talk policy", () => {
   describe("resolveNextcloudTalkAllowlistMatch", () => {
@@ -30,4 +30,109 @@ describe("nextcloud-talk policy", () => {
       ).toBe(false);
     });
   });
+
+  describe("resolveNextcloudTalkGroupAllow", () => {
+    it("blocks disabled policy", () => {
+      expect(
+        resolveNextcloudTalkGroupAllow({
+          groupPolicy: "disabled",
+          outerAllowFrom: ["owner"],
+          innerAllowFrom: ["room-user"],
+          senderId: "owner",
+        }),
+      ).toEqual({
+        allowed: false,
+        outerMatch: { allowed: false },
+        innerMatch: { allowed: false },
+      });
+    });
+
+    it("allows open policy", () => {
+      expect(
+        resolveNextcloudTalkGroupAllow({
+          groupPolicy: "open",
+          outerAllowFrom: [],
+          innerAllowFrom: [],
+          senderId: "owner",
+        }),
+      ).toEqual({
+        allowed: true,
+        outerMatch: { allowed: true },
+        innerMatch: { allowed: true },
+      });
+    });
+
+    it("blocks allowlist mode when both outer and inner allowlists are empty", () => {
+      expect(
+        resolveNextcloudTalkGroupAllow({
+          groupPolicy: "allowlist",
+          outerAllowFrom: [],
+          innerAllowFrom: [],
+          senderId: "owner",
+        }),
+      ).toEqual({
+        allowed: false,
+        outerMatch: { allowed: false },
+        innerMatch: { allowed: false },
+      });
+    });
+
+    it("requires inner match when only room-specific allowlist is configured", () => {
+      expect(
+        resolveNextcloudTalkGroupAllow({
+          groupPolicy: "allowlist",
+          outerAllowFrom: [],
+          innerAllowFrom: ["room-user"],
+          senderId: "room-user",
+        }),
+      ).toEqual({
+        allowed: true,
+        outerMatch: { allowed: false },
+        innerMatch: { allowed: true, matchKey: "room-user", matchSource: "id" },
+      });
+    });
+
+    it("blocks when outer allowlist misses even if inner allowlist matches", () => {
+      expect(
+        resolveNextcloudTalkGroupAllow({
+          groupPolicy: "allowlist",
+          outerAllowFrom: ["team-owner"],
+          innerAllowFrom: ["room-user"],
+          senderId: "room-user",
+        }),
+      ).toEqual({
+        allowed: false,
+        outerMatch: { allowed: false },
+        innerMatch: { allowed: true, matchKey: "room-user", matchSource: "id" },
+      });
+    });
+
+    it("allows when both outer and inner allowlists match", () => {
+      expect(
+        resolveNextcloudTalkGroupAllow({
+          groupPolicy: "allowlist",
+          outerAllowFrom: ["team-owner"],
+          innerAllowFrom: ["room-user"],
+          senderId: "team-owner",
+        }),
+      ).toEqual({
+        allowed: false,
+        outerMatch: { allowed: true, matchKey: "team-owner", matchSource: "id" },
+        innerMatch: { allowed: false },
+      });
+
+      expect(
+        resolveNextcloudTalkGroupAllow({
+          groupPolicy: "allowlist",
+          outerAllowFrom: ["shared-user"],
+          innerAllowFrom: ["shared-user"],
+          senderId: "shared-user",
+        }),
+      ).toEqual({
+        allowed: true,
+        outerMatch: { allowed: true, matchKey: "shared-user", matchSource: "id" },
+        innerMatch: { allowed: true, matchKey: "shared-user", matchSource: "id" },
+      });
+    });
+  });
 });

extensions/nextcloud-talk/src/policy.ts

@@ -6,6 +6,7 @@ import type {
 } from "openclaw/plugin-sdk/nextcloud-talk";
 import {
   buildChannelKeyCandidates,
+  evaluateMatchedGroupAccessForPolicy,
   normalizeChannelSlug,
   resolveChannelEntryMatchWithFallback,
   resolveMentionGatingWithBypass,
@@ -128,19 +129,8 @@ export function resolveNextcloudTalkGroupAllow(params: {
   innerAllowFrom: Array<string | number> | undefined;
   senderId: string;
 }): { allowed: boolean; outerMatch: AllowlistMatch; innerMatch: AllowlistMatch } {
-  if (params.groupPolicy === "disabled") {
-    return { allowed: false, outerMatch: { allowed: false }, innerMatch: { allowed: false } };
-  }
-  if (params.groupPolicy === "open") {
-    return { allowed: true, outerMatch: { allowed: true }, innerMatch: { allowed: true } };
-  }
-
   const outerAllow = normalizeNextcloudTalkAllowlist(params.outerAllowFrom);
   const innerAllow = normalizeNextcloudTalkAllowlist(params.innerAllowFrom);
-  if (outerAllow.length === 0 && innerAllow.length === 0) {
-    return { allowed: false, outerMatch: { allowed: false }, innerMatch: { allowed: false } };
-  }
-
   const outerMatch = resolveNextcloudTalkAllowlistMatch({
     allowFrom: params.outerAllowFrom,
     senderId: params.senderId,
@@ -149,14 +139,32 @@ export function resolveNextcloudTalkGroupAllow(params: {
     allowFrom: params.innerAllowFrom,
     senderId: params.senderId,
   });
-  const allowed = resolveNestedAllowlistDecision({
-    outerConfigured: outerAllow.length > 0 || innerAllow.length > 0,
-    outerMatched: outerAllow.length > 0 ? outerMatch.allowed : true,
-    innerConfigured: innerAllow.length > 0,
-    innerMatched: innerMatch.allowed,
+  const access = evaluateMatchedGroupAccessForPolicy({
+    groupPolicy: params.groupPolicy,
+    allowlistConfigured: outerAllow.length > 0 || innerAllow.length > 0,
+    allowlistMatched: resolveNestedAllowlistDecision({
+      outerConfigured: outerAllow.length > 0 || innerAllow.length > 0,
+      outerMatched: outerAllow.length > 0 ? outerMatch.allowed : true,
+      innerConfigured: innerAllow.length > 0,
+      innerMatched: innerMatch.allowed,
+    }),
   });
 
-  return { allowed, outerMatch, innerMatch };
+  return {
+    allowed: access.allowed,
+    outerMatch:
+      params.groupPolicy === "open"
+        ? { allowed: true }
+        : params.groupPolicy === "disabled"
+          ? { allowed: false }
+          : outerMatch,
+    innerMatch:
+      params.groupPolicy === "open"
+        ? { allowed: true }
+        : params.groupPolicy === "disabled"
+          ? { allowed: false }
+          : innerMatch,
+  };
 }
 
 export function resolveNextcloudTalkMentionGate(params: {

src/plugin-sdk/group-access.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, it } from "vitest";
 import {
   evaluateGroupRouteAccessForPolicy,
+  evaluateMatchedGroupAccessForPolicy,
   evaluateSenderGroupAccess,
   evaluateSenderGroupAccessForPolicy,
   resolveSenderScopedGroupPolicy,
@@ -120,6 +121,64 @@ describe("evaluateGroupRouteAccessForPolicy", () => {
   });
 });
 
+describe("evaluateMatchedGroupAccessForPolicy", () => {
+  it("blocks disabled policy", () => {
+    expect(
+      evaluateMatchedGroupAccessForPolicy({
+        groupPolicy: "disabled",
+        allowlistConfigured: true,
+        allowlistMatched: true,
+      }),
+    ).toEqual({
+      allowed: false,
+      groupPolicy: "disabled",
+      reason: "disabled",
+    });
+  });
+
+  it("blocks allowlist without configured entries", () => {
+    expect(
+      evaluateMatchedGroupAccessForPolicy({
+        groupPolicy: "allowlist",
+        allowlistConfigured: false,
+        allowlistMatched: false,
+      }),
+    ).toEqual({
+      allowed: false,
+      groupPolicy: "allowlist",
+      reason: "empty_allowlist",
+    });
+  });
+
+  it("blocks unmatched allowlist sender", () => {
+    expect(
+      evaluateMatchedGroupAccessForPolicy({
+        groupPolicy: "allowlist",
+        allowlistConfigured: true,
+        allowlistMatched: false,
+      }),
+    ).toEqual({
+      allowed: false,
+      groupPolicy: "allowlist",
+      reason: "not_allowlisted",
+    });
+  });
+
+  it("allows open policy", () => {
+    expect(
+      evaluateMatchedGroupAccessForPolicy({
+        groupPolicy: "open",
+        allowlistConfigured: false,
+        allowlistMatched: false,
+      }),
+    ).toEqual({
+      allowed: true,
+      groupPolicy: "open",
+      reason: "allowed",
+    });
+  });
+});
+
 describe("evaluateSenderGroupAccess", () => {
   it("defaults missing provider config to allowlist", () => {
     const decision = evaluateSenderGroupAccess({

src/plugin-sdk/group-access.ts

@@ -27,6 +27,18 @@ export type GroupRouteAccessDecision = {
   reason: GroupRouteAccessReason;
 };
 
+export type MatchedGroupAccessReason =
+  | "allowed"
+  | "disabled"
+  | "empty_allowlist"
+  | "not_allowlisted";
+
+export type MatchedGroupAccessDecision = {
+  allowed: boolean;
+  groupPolicy: GroupPolicy;
+  reason: MatchedGroupAccessReason;
+};
+
 export function resolveSenderScopedGroupPolicy(params: {
   groupPolicy: GroupPolicy;
   groupAllowFrom: string[];
@@ -83,6 +95,43 @@ export function evaluateGroupRouteAccessForPolicy(params: {
   };
 }
 
+export function evaluateMatchedGroupAccessForPolicy(params: {
+  groupPolicy: GroupPolicy;
+  allowlistConfigured: boolean;
+  allowlistMatched: boolean;
+}): MatchedGroupAccessDecision {
+  if (params.groupPolicy === "disabled") {
+    return {
+      allowed: false,
+      groupPolicy: params.groupPolicy,
+      reason: "disabled",
+    };
+  }
+
+  if (params.groupPolicy === "allowlist") {
+    if (!params.allowlistConfigured) {
+      return {
+        allowed: false,
+        groupPolicy: params.groupPolicy,
+        reason: "empty_allowlist",
+      };
+    }
+    if (!params.allowlistMatched) {
+      return {
+        allowed: false,
+        groupPolicy: params.groupPolicy,
+        reason: "not_allowlisted",
+      };
+    }
+  }
+
+  return {
+    allowed: true,
+    groupPolicy: params.groupPolicy,
+    reason: "allowed",
+  };
+}
+
 export function evaluateSenderGroupAccessForPolicy(params: {
   groupPolicy: GroupPolicy;
   providerMissingFallbackApplied?: boolean;

src/plugin-sdk/index.ts

@@ -280,11 +280,14 @@ export {
 } from "./allow-from.js";
 export {
   evaluateGroupRouteAccessForPolicy,
+  evaluateMatchedGroupAccessForPolicy,
   evaluateSenderGroupAccess,
   evaluateSenderGroupAccessForPolicy,
   resolveSenderScopedGroupPolicy,
   type GroupRouteAccessDecision,
   type GroupRouteAccessReason,
+  type MatchedGroupAccessDecision,
+  type MatchedGroupAccessReason,
   type SenderGroupAccessDecision,
   type SenderGroupAccessReason,
 } from "./group-access.js";

src/plugin-sdk/nextcloud-talk.ts

@@ -37,6 +37,7 @@ export type { ChannelPlugin } from "../channels/plugins/types.plugin.js";
 export { createReplyPrefixOptions } from "../channels/reply-prefix.js";
 export type { OpenClawConfig } from "../config/config.js";
 export { mapAllowFromEntries } from "./channel-config-helpers.js";
+export { evaluateMatchedGroupAccessForPolicy } from "./group-access.js";
 export {
   GROUP_POLICY_BLOCKED_LABEL,
   resolveAllowlistProviderRuntimeGroupPolicy,

src/security/dm-policy-shared.test.ts

@@ -388,6 +388,38 @@ describe("security/dm-policy-shared", () => {
   });
 
   for (const channel of channels) {
+    it(`[${channel}] blocks groups when group allowlist is empty`, () => {
+      const decision = resolveDmGroupAccessDecision({
+        isGroup: true,
+        dmPolicy: "pairing",
+        groupPolicy: "allowlist",
+        effectiveAllowFrom: ["owner"],
+        effectiveGroupAllowFrom: [],
+        isSenderAllowed: () => false,
+      });
+      expect(decision).toEqual({
+        decision: "block",
+        reasonCode: DM_GROUP_ACCESS_REASON.GROUP_POLICY_EMPTY_ALLOWLIST,
+        reason: "groupPolicy=allowlist (empty allowlist)",
+      });
+    });
+
+    it(`[${channel}] allows groups when group policy is open`, () => {
+      const decision = resolveDmGroupAccessDecision({
+        isGroup: true,
+        dmPolicy: "pairing",
+        groupPolicy: "open",
+        effectiveAllowFrom: ["owner"],
+        effectiveGroupAllowFrom: [],
+        isSenderAllowed: () => false,
+      });
+      expect(decision).toEqual({
+        decision: "allow",
+        reasonCode: DM_GROUP_ACCESS_REASON.GROUP_POLICY_ALLOWED,
+        reason: "groupPolicy=open",
+      });
+    });
+
     it(`[${channel}] blocks DM allowlist mode when allowlist is empty`, () => {
       const decision = resolveDmGroupAccessDecision({
         isGroup: false,

src/security/dm-policy-shared.ts

@@ -2,6 +2,7 @@ import { mergeDmAllowFromSources, resolveGroupAllowFromSources } from "../channe
 import { resolveControlCommandGate } from "../channels/command-gating.js";
 import type { ChannelId } from "../channels/plugins/types.js";
 import { readChannelAllowFromStore } from "../pairing/pairing-store.js";
+import { evaluateMatchedGroupAccessForPolicy } from "../plugin-sdk/group-access.js";
 import { normalizeStringEntries } from "../shared/string-normalization.js";
 
 export function resolvePinnedMainDmOwnerFromAllowlist(params: {
@@ -118,22 +119,28 @@ export function resolveDmGroupAccessDecision(params: {
   const effectiveGroupAllowFrom = normalizeStringEntries(params.effectiveGroupAllowFrom);
 
   if (params.isGroup) {
-    if (groupPolicy === "disabled") {
-      return {
-        decision: "block",
-        reasonCode: DM_GROUP_ACCESS_REASON.GROUP_POLICY_DISABLED,
-        reason: "groupPolicy=disabled",
-      };
-    }
-    if (groupPolicy === "allowlist") {
-      if (effectiveGroupAllowFrom.length === 0) {
+    const groupAccess = evaluateMatchedGroupAccessForPolicy({
+      groupPolicy,
+      allowlistConfigured: effectiveGroupAllowFrom.length > 0,
+      allowlistMatched: params.isSenderAllowed(effectiveGroupAllowFrom),
+    });
+
+    if (!groupAccess.allowed) {
+      if (groupAccess.reason === "disabled") {
+        return {
+          decision: "block",
+          reasonCode: DM_GROUP_ACCESS_REASON.GROUP_POLICY_DISABLED,
+          reason: "groupPolicy=disabled",
+        };
+      }
+      if (groupAccess.reason === "empty_allowlist") {
         return {
           decision: "block",
           reasonCode: DM_GROUP_ACCESS_REASON.GROUP_POLICY_EMPTY_ALLOWLIST,
           reason: "groupPolicy=allowlist (empty allowlist)",
         };
       }
-      if (!params.isSenderAllowed(effectiveGroupAllowFrom)) {
+      if (groupAccess.reason === "not_allowlisted") {
         return {
           decision: "block",
           reasonCode: DM_GROUP_ACCESS_REASON.GROUP_POLICY_NOT_ALLOWLISTED,
@@ -141,6 +148,7 @@ export function resolveDmGroupAccessDecision(params: {
         };
       }
     }
+
     return {
       decision: "allow",
       reasonCode: DM_GROUP_ACCESS_REASON.GROUP_POLICY_ALLOWED,