diff --git a/src/line/bot-handlers.test.ts b/src/line/bot-handlers.test.ts index 3e52dd338fa..4f2ca707c8b 100644 --- a/src/line/bot-handlers.test.ts +++ b/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(); diff --git a/src/line/bot-handlers.ts b/src/line/bot-handlers.ts index 479cf0781e4..96d82afd33c 100644 --- a/src/line/bot-handlers.ts +++ b/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; } diff --git a/src/plugin-sdk/group-access.test.ts b/src/plugin-sdk/group-access.test.ts index b44c71c447e..fec5738e85d 100644 --- a/src/plugin-sdk/group-access.test.ts +++ b/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({ diff --git a/src/plugin-sdk/group-access.ts b/src/plugin-sdk/group-access.ts index ce80b04b1d7..5a58242338b 100644 --- a/src/plugin-sdk/group-access.ts +++ b/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, diff --git a/src/telegram/group-access.policy-access.test.ts b/src/telegram/group-access.policy-access.test.ts index 5683732476c..d32863318d2 100644 --- a/src/telegram/group-access.policy-access.test.ts +++ b/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"] diff --git a/src/telegram/group-access.ts b/src/telegram/group-access.ts index 19503b7fe39..e97251c950a 100644 --- a/src/telegram/group-access.ts +++ b/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 }; } }