refactor: share missing-sender matched allowlist evaluation

src/line/bot-handlers.test.ts

@@ -265,6 +265,40 @@ describe("handleLineWebhookEvents", () => {
     expect(readAllowFromStoreMock).toHaveBeenCalledWith("line", undefined, "default");
   });
 
+  it("blocks group messages without sender id when groupPolicy is allowlist", async () => {
+    const processMessage = vi.fn();
+    const event = {
+      type: "message",
+      message: { id: "m5a", type: "text", text: "hi" },
+      replyToken: "reply-token",
+      timestamp: Date.now(),
+      source: { type: "group", groupId: "group-1" },
+      mode: "active",
+      webhookEventId: "evt-5a",
+      deliveryContext: { isRedelivery: false },
+    } as MessageEvent;
+
+    await handleLineWebhookEvents([event], {
+      cfg: {
+        channels: { line: { groupPolicy: "allowlist", groupAllowFrom: ["user-5"] } },
+      },
+      account: {
+        accountId: "default",
+        enabled: true,
+        channelAccessToken: "token",
+        channelSecret: "secret",
+        tokenSource: "config",
+        config: { groupPolicy: "allowlist", groupAllowFrom: ["user-5"] },
+      },
+      runtime: createRuntime(),
+      mediaMaxBytes: 1,
+      processMessage,
+    });
+
+    expect(processMessage).not.toHaveBeenCalled();
+    expect(buildLineMessageContextMock).not.toHaveBeenCalled();
+  });
+
   it("does not authorize group messages from DM pairing-store entries when group allowlist is empty", async () => {
     readAllowFromStoreMock.mockResolvedValueOnce(["user-5"]);
     const processMessage = vi.fn();

src/line/bot-handlers.ts

@@ -30,7 +30,7 @@ import {
   readChannelAllowFromStore,
   upsertChannelPairingRequest,
 } from "../pairing/pairing-store.js";
-import { evaluateSenderGroupAccessForPolicy } from "../plugin-sdk/group-access.js";
+import { evaluateMatchedGroupAccessForPolicy } from "../plugin-sdk/group-access.js";
 import { resolveAgentRoute } from "../routing/resolve-route.js";
 import type { RuntimeEnv } from "../runtime.js";
 import {
@@ -345,29 +345,31 @@ async function shouldProcessLineEvent(
         return denied;
       }
     }
-    if (groupPolicy === "allowlist" && !senderId) {
-      logVerbose("Blocked line group message (no sender ID, groupPolicy: allowlist)");
-      return denied;
-    }
-    const senderGroupAccess = evaluateSenderGroupAccessForPolicy({
+    const senderGroupAccess = evaluateMatchedGroupAccessForPolicy({
       groupPolicy,
-      groupAllowFrom: effectiveGroupAllow.entries,
-      senderId,
-      isSenderAllowed: (candidateSenderId, allowFrom) =>
+      requireMatchInput: true,
+      hasMatchInput: Boolean(senderId),
+      allowlistConfigured: effectiveGroupAllow.entries.length > 0,
+      allowlistMatched:
+        Boolean(senderId) &&
         isSenderAllowed({
-          allow: normalizeAllowFrom(allowFrom),
-          senderId: candidateSenderId,
+          allow: effectiveGroupAllow,
+          senderId,
         }),
     });
     if (!senderGroupAccess.allowed && senderGroupAccess.reason === "disabled") {
       logVerbose("Blocked line group message (groupPolicy: disabled)");
       return denied;
     }
+    if (!senderGroupAccess.allowed && senderGroupAccess.reason === "missing_match_input") {
+      logVerbose("Blocked line group message (no sender ID, groupPolicy: allowlist)");
+      return denied;
+    }
     if (!senderGroupAccess.allowed && senderGroupAccess.reason === "empty_allowlist") {
       logVerbose("Blocked line group message (groupPolicy: allowlist, no groupAllowFrom)");
       return denied;
     }
-    if (!senderGroupAccess.allowed && senderGroupAccess.reason === "sender_not_allowlisted") {
+    if (!senderGroupAccess.allowed && senderGroupAccess.reason === "not_allowlisted") {
       logVerbose(`Blocked line group message from ${senderId} (groupPolicy: allowlist)`);
       return denied;
     }

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

@@ -150,6 +150,22 @@ describe("evaluateMatchedGroupAccessForPolicy", () => {
     });
   });
 
+  it("blocks allowlist when required match input is missing", () => {
+    expect(
+      evaluateMatchedGroupAccessForPolicy({
+        groupPolicy: "allowlist",
+        requireMatchInput: true,
+        hasMatchInput: false,
+        allowlistConfigured: true,
+        allowlistMatched: false,
+      }),
+    ).toEqual({
+      allowed: false,
+      groupPolicy: "allowlist",
+      reason: "missing_match_input",
+    });
+  });
+
   it("blocks unmatched allowlist sender", () => {
     expect(
       evaluateMatchedGroupAccessForPolicy({

src/plugin-sdk/group-access.ts

@@ -30,6 +30,7 @@ export type GroupRouteAccessDecision = {
 export type MatchedGroupAccessReason =
   | "allowed"
   | "disabled"
+  | "missing_match_input"
   | "empty_allowlist"
   | "not_allowlisted";
 
@@ -99,6 +100,8 @@ export function evaluateMatchedGroupAccessForPolicy(params: {
   groupPolicy: GroupPolicy;
   allowlistConfigured: boolean;
   allowlistMatched: boolean;
+  requireMatchInput?: boolean;
+  hasMatchInput?: boolean;
 }): MatchedGroupAccessDecision {
   if (params.groupPolicy === "disabled") {
     return {
@@ -109,6 +112,13 @@ export function evaluateMatchedGroupAccessForPolicy(params: {
   }
 
   if (params.groupPolicy === "allowlist") {
+    if (params.requireMatchInput && !params.hasMatchInput) {
+      return {
+        allowed: false,
+        groupPolicy: params.groupPolicy,
+        reason: "missing_match_input",
+      };
+    }
     if (!params.allowlistConfigured) {
       return {
         allowed: false,

src/telegram/group-access.policy-access.test.ts

@@ -180,6 +180,25 @@ describe("evaluateTelegramGroupPolicyAccess – chat allowlist vs sender allowli
     expect(result).toEqual({ allowed: true, groupPolicy: "allowlist" });
   });
 
+  it("blocks allowlist groups without sender identity before sender matching", () => {
+    const result = runAccess({
+      senderId: undefined,
+      senderUsername: undefined,
+      effectiveGroupAllow: senderAllow,
+      resolveGroupPolicy: () => ({
+        allowlistEnabled: true,
+        allowed: true,
+        groupConfig: { requireMention: false },
+      }),
+    });
+
+    expect(result).toEqual({
+      allowed: false,
+      reason: "group-policy-allowlist-no-sender",
+      groupPolicy: "allowlist",
+    });
+  });
+
   it("allows authorized sender in wildcard-matched group with sender entries", () => {
     const result = runAccess({
       effectiveGroupAllow: senderAllow, // entries: ["111"]

src/telegram/group-access.ts

@@ -7,6 +7,7 @@ import type {
   TelegramGroupConfig,
   TelegramTopicConfig,
 } from "../config/types.js";
+import { evaluateMatchedGroupAccessForPolicy } from "../plugin-sdk/group-access.js";
 import { isSenderAllowed, type NormalizedAllowFrom } from "./bot-access.js";
 import { firstDefined } from "./bot-access.js";
 
@@ -174,31 +175,29 @@ export const evaluateTelegramGroupPolicyAccess = (params: {
   }
   if (groupPolicy === "allowlist" && params.enforceAllowlistAuthorization) {
     const senderId = params.senderId ?? "";
-    if (params.requireSenderForAllowlistAuthorization && !senderId) {
+    const senderAuthorization = evaluateMatchedGroupAccessForPolicy({
+      groupPolicy,
+      requireMatchInput: params.requireSenderForAllowlistAuthorization,
+      hasMatchInput: Boolean(senderId),
+      allowlistConfigured:
+        chatExplicitlyAllowed ||
+        params.allowEmptyAllowlistEntries ||
+        params.effectiveGroupAllow.hasEntries,
+      allowlistMatched:
+        (chatExplicitlyAllowed && !params.effectiveGroupAllow.hasEntries) ||
+        isSenderAllowed({
+          allow: params.effectiveGroupAllow,
+          senderId,
+          senderUsername: params.senderUsername ?? "",
+        }),
+    });
+    if (!senderAuthorization.allowed && senderAuthorization.reason === "missing_match_input") {
       return { allowed: false, reason: "group-policy-allowlist-no-sender", groupPolicy };
     }
-    // Skip the "empty allowlist" guard when the chat itself is explicitly
-    // listed in the groups config — the group ID acts as the allowlist entry.
-    if (
-      !chatExplicitlyAllowed &&
-      !params.allowEmptyAllowlistEntries &&
-      !params.effectiveGroupAllow.hasEntries
-    ) {
+    if (!senderAuthorization.allowed && senderAuthorization.reason === "empty_allowlist") {
       return { allowed: false, reason: "group-policy-allowlist-empty", groupPolicy };
     }
-    // When the chat is explicitly allowed and there are no sender-level entries,
-    // skip the sender check — the group ID itself is the authorization.
-    if (chatExplicitlyAllowed && !params.effectiveGroupAllow.hasEntries) {
-      return { allowed: true, groupPolicy };
-    }
-    const senderUsername = params.senderUsername ?? "";
-    if (
-      !isSenderAllowed({
-        allow: params.effectiveGroupAllow,
-        senderId,
-        senderUsername,
-      })
-    ) {
+    if (!senderAuthorization.allowed && senderAuthorization.reason === "not_allowlisted") {
       return { allowed: false, reason: "group-policy-allowlist-unauthorized", groupPolicy };
     }
   }