mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-05 02:40:24 +00:00
fix(telegram): check chat allowlist before sender allowlist in group policy
When groupPolicy is "allowlist", the sender allowlist empty-entries guard ran before the chat-level allowlist check. This caused groups that were explicitly configured in the groups config to be silently rejected when no allowFrom / groupAllowFrom entries existed. Move the checkChatAllowlist block before the sender allowlist guard and introduce a chatExplicitlyAllowed flag that distinguishes a dedicated group entry (groupConfig is set) from a wildcard-only match. When the chat is explicitly allowed and no sender entries exist, skip the sender check entirely — the group ID itself acts as the authorization. Fixes #30613.
This commit is contained in:
committed by
Peter Steinberger
parent
60f8e832e0
commit
8247c25a32
270
src/telegram/group-access.policy-access.test.ts
Normal file
270
src/telegram/group-access.policy-access.test.ts
Normal file
@@ -0,0 +1,270 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import type { TelegramAccountConfig } from "../config/types.js";
|
||||
import { evaluateTelegramGroupPolicyAccess } from "./group-access.js";
|
||||
|
||||
/**
|
||||
* Minimal stubs shared across tests.
|
||||
*/
|
||||
const baseCfg = {
|
||||
channels: { telegram: {} },
|
||||
} as unknown as OpenClawConfig;
|
||||
|
||||
const baseTelegramCfg: TelegramAccountConfig = {
|
||||
groupPolicy: "allowlist",
|
||||
} as unknown as TelegramAccountConfig;
|
||||
|
||||
const emptyAllow = { entries: [], hasWildcard: false, hasEntries: false, invalidEntries: [] };
|
||||
const senderAllow = {
|
||||
entries: ["111"],
|
||||
hasWildcard: false,
|
||||
hasEntries: true,
|
||||
invalidEntries: [],
|
||||
};
|
||||
|
||||
describe("evaluateTelegramGroupPolicyAccess – chat allowlist vs sender allowlist ordering", () => {
|
||||
it("allows a group explicitly listed in groups config even when no allowFrom entries exist", () => {
|
||||
// Issue #30613: a group configured with a dedicated entry (groupConfig set)
|
||||
// should be allowed even without any allowFrom / groupAllowFrom entries.
|
||||
const result = evaluateTelegramGroupPolicyAccess({
|
||||
isGroup: true,
|
||||
chatId: "-100123456",
|
||||
cfg: baseCfg,
|
||||
telegramCfg: baseTelegramCfg,
|
||||
effectiveGroupAllow: emptyAllow,
|
||||
senderId: "999",
|
||||
senderUsername: "user",
|
||||
resolveGroupPolicy: () => ({
|
||||
allowlistEnabled: true,
|
||||
allowed: true,
|
||||
groupConfig: { requireMention: false }, // dedicated entry — not just wildcard
|
||||
}),
|
||||
enforcePolicy: true,
|
||||
useTopicAndGroupOverrides: false,
|
||||
enforceAllowlistAuthorization: true,
|
||||
allowEmptyAllowlistEntries: false,
|
||||
requireSenderForAllowlistAuthorization: true,
|
||||
checkChatAllowlist: true,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ allowed: true, groupPolicy: "allowlist" });
|
||||
});
|
||||
|
||||
it("still blocks when only wildcard match and no allowFrom entries", () => {
|
||||
// groups: { "*": ... } with no allowFrom → wildcard does NOT bypass sender checks.
|
||||
const result = evaluateTelegramGroupPolicyAccess({
|
||||
isGroup: true,
|
||||
chatId: "-100123456",
|
||||
cfg: baseCfg,
|
||||
telegramCfg: baseTelegramCfg,
|
||||
effectiveGroupAllow: emptyAllow,
|
||||
senderId: "999",
|
||||
senderUsername: "user",
|
||||
resolveGroupPolicy: () => ({
|
||||
allowlistEnabled: true,
|
||||
allowed: true,
|
||||
groupConfig: undefined, // wildcard match only — no dedicated entry
|
||||
}),
|
||||
enforcePolicy: true,
|
||||
useTopicAndGroupOverrides: false,
|
||||
enforceAllowlistAuthorization: true,
|
||||
allowEmptyAllowlistEntries: false,
|
||||
requireSenderForAllowlistAuthorization: true,
|
||||
checkChatAllowlist: true,
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
allowed: false,
|
||||
reason: "group-policy-allowlist-empty",
|
||||
groupPolicy: "allowlist",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects a group NOT in groups config", () => {
|
||||
const result = evaluateTelegramGroupPolicyAccess({
|
||||
isGroup: true,
|
||||
chatId: "-100999999",
|
||||
cfg: baseCfg,
|
||||
telegramCfg: baseTelegramCfg,
|
||||
effectiveGroupAllow: emptyAllow,
|
||||
senderId: "999",
|
||||
senderUsername: "user",
|
||||
resolveGroupPolicy: () => ({
|
||||
allowlistEnabled: true,
|
||||
allowed: false,
|
||||
}),
|
||||
enforcePolicy: true,
|
||||
useTopicAndGroupOverrides: false,
|
||||
enforceAllowlistAuthorization: true,
|
||||
allowEmptyAllowlistEntries: false,
|
||||
requireSenderForAllowlistAuthorization: true,
|
||||
checkChatAllowlist: true,
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
allowed: false,
|
||||
reason: "group-chat-not-allowed",
|
||||
groupPolicy: "allowlist",
|
||||
});
|
||||
});
|
||||
|
||||
it("still enforces sender allowlist when checkChatAllowlist is disabled", () => {
|
||||
const result = evaluateTelegramGroupPolicyAccess({
|
||||
isGroup: true,
|
||||
chatId: "-100123456",
|
||||
cfg: baseCfg,
|
||||
telegramCfg: baseTelegramCfg,
|
||||
effectiveGroupAllow: emptyAllow,
|
||||
senderId: "999",
|
||||
senderUsername: "user",
|
||||
resolveGroupPolicy: () => ({
|
||||
allowlistEnabled: true,
|
||||
allowed: true,
|
||||
groupConfig: { requireMention: false },
|
||||
}),
|
||||
enforcePolicy: true,
|
||||
useTopicAndGroupOverrides: false,
|
||||
enforceAllowlistAuthorization: true,
|
||||
allowEmptyAllowlistEntries: false,
|
||||
requireSenderForAllowlistAuthorization: true,
|
||||
checkChatAllowlist: false,
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
allowed: false,
|
||||
reason: "group-policy-allowlist-empty",
|
||||
groupPolicy: "allowlist",
|
||||
});
|
||||
});
|
||||
|
||||
it("blocks unauthorized sender even when chat is explicitly allowed and sender entries exist", () => {
|
||||
const result = evaluateTelegramGroupPolicyAccess({
|
||||
isGroup: true,
|
||||
chatId: "-100123456",
|
||||
cfg: baseCfg,
|
||||
telegramCfg: baseTelegramCfg,
|
||||
effectiveGroupAllow: senderAllow, // entries: ["111"]
|
||||
senderId: "222", // not in senderAllow.entries
|
||||
senderUsername: "other",
|
||||
resolveGroupPolicy: () => ({
|
||||
allowlistEnabled: true,
|
||||
allowed: true,
|
||||
groupConfig: { requireMention: false },
|
||||
}),
|
||||
enforcePolicy: true,
|
||||
useTopicAndGroupOverrides: false,
|
||||
enforceAllowlistAuthorization: true,
|
||||
allowEmptyAllowlistEntries: false,
|
||||
requireSenderForAllowlistAuthorization: true,
|
||||
checkChatAllowlist: true,
|
||||
});
|
||||
|
||||
// Chat is explicitly allowed, but sender entries exist and sender is not in them.
|
||||
expect(result).toEqual({
|
||||
allowed: false,
|
||||
reason: "group-policy-allowlist-unauthorized",
|
||||
groupPolicy: "allowlist",
|
||||
});
|
||||
});
|
||||
|
||||
it("allows when groupPolicy is open regardless of allowlist state", () => {
|
||||
const result = evaluateTelegramGroupPolicyAccess({
|
||||
isGroup: true,
|
||||
chatId: "-100123456",
|
||||
cfg: baseCfg,
|
||||
telegramCfg: { groupPolicy: "open" } as unknown as TelegramAccountConfig,
|
||||
effectiveGroupAllow: emptyAllow,
|
||||
senderId: "999",
|
||||
senderUsername: "user",
|
||||
resolveGroupPolicy: () => ({
|
||||
allowlistEnabled: false,
|
||||
allowed: false,
|
||||
}),
|
||||
enforcePolicy: true,
|
||||
useTopicAndGroupOverrides: false,
|
||||
enforceAllowlistAuthorization: true,
|
||||
allowEmptyAllowlistEntries: false,
|
||||
requireSenderForAllowlistAuthorization: true,
|
||||
checkChatAllowlist: true,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ allowed: true, groupPolicy: "open" });
|
||||
});
|
||||
|
||||
it("rejects when groupPolicy is disabled", () => {
|
||||
const result = evaluateTelegramGroupPolicyAccess({
|
||||
isGroup: true,
|
||||
chatId: "-100123456",
|
||||
cfg: baseCfg,
|
||||
telegramCfg: { groupPolicy: "disabled" } as unknown as TelegramAccountConfig,
|
||||
effectiveGroupAllow: emptyAllow,
|
||||
senderId: "999",
|
||||
senderUsername: "user",
|
||||
resolveGroupPolicy: () => ({
|
||||
allowlistEnabled: false,
|
||||
allowed: false,
|
||||
}),
|
||||
enforcePolicy: true,
|
||||
useTopicAndGroupOverrides: false,
|
||||
enforceAllowlistAuthorization: true,
|
||||
allowEmptyAllowlistEntries: false,
|
||||
requireSenderForAllowlistAuthorization: true,
|
||||
checkChatAllowlist: true,
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
allowed: false,
|
||||
reason: "group-policy-disabled",
|
||||
groupPolicy: "disabled",
|
||||
});
|
||||
});
|
||||
|
||||
it("allows non-group messages without any checks", () => {
|
||||
const result = evaluateTelegramGroupPolicyAccess({
|
||||
isGroup: false,
|
||||
chatId: "12345",
|
||||
cfg: baseCfg,
|
||||
telegramCfg: baseTelegramCfg,
|
||||
effectiveGroupAllow: emptyAllow,
|
||||
senderId: "999",
|
||||
senderUsername: "user",
|
||||
resolveGroupPolicy: () => ({
|
||||
allowlistEnabled: true,
|
||||
allowed: false,
|
||||
}),
|
||||
enforcePolicy: true,
|
||||
useTopicAndGroupOverrides: false,
|
||||
enforceAllowlistAuthorization: true,
|
||||
allowEmptyAllowlistEntries: false,
|
||||
requireSenderForAllowlistAuthorization: true,
|
||||
checkChatAllowlist: true,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ allowed: true, groupPolicy: "allowlist" });
|
||||
});
|
||||
|
||||
it("allows authorized sender in wildcard-matched group with sender entries", () => {
|
||||
const result = evaluateTelegramGroupPolicyAccess({
|
||||
isGroup: true,
|
||||
chatId: "-100123456",
|
||||
cfg: baseCfg,
|
||||
telegramCfg: baseTelegramCfg,
|
||||
effectiveGroupAllow: senderAllow, // entries: ["111"]
|
||||
senderId: "111", // IS in senderAllow.entries
|
||||
senderUsername: "user",
|
||||
resolveGroupPolicy: () => ({
|
||||
allowlistEnabled: true,
|
||||
allowed: true,
|
||||
groupConfig: undefined, // wildcard only
|
||||
}),
|
||||
enforcePolicy: true,
|
||||
useTopicAndGroupOverrides: false,
|
||||
enforceAllowlistAuthorization: true,
|
||||
allowEmptyAllowlistEntries: false,
|
||||
requireSenderForAllowlistAuthorization: true,
|
||||
checkChatAllowlist: true,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ allowed: true, groupPolicy: "allowlist" });
|
||||
});
|
||||
});
|
||||
@@ -130,14 +130,40 @@ export const evaluateTelegramGroupPolicyAccess = (params: {
|
||||
if (groupPolicy === "disabled") {
|
||||
return { allowed: false, reason: "group-policy-disabled", groupPolicy };
|
||||
}
|
||||
// Check chat-level allowlist first so that groups explicitly listed in the
|
||||
// `groups` config are not blocked by the sender-level "empty allowlist" guard.
|
||||
let chatExplicitlyAllowed = false;
|
||||
if (params.checkChatAllowlist) {
|
||||
const groupAllowlist = params.resolveGroupPolicy(params.chatId);
|
||||
if (groupAllowlist.allowlistEnabled && !groupAllowlist.allowed) {
|
||||
return { allowed: false, reason: "group-chat-not-allowed", groupPolicy };
|
||||
}
|
||||
// The chat is explicitly allowed when it has a dedicated entry in the groups
|
||||
// config (groupConfig is set). A wildcard ("*") match alone does not count
|
||||
// because it only enables the group — sender-level filtering still applies.
|
||||
if (groupAllowlist.allowlistEnabled && groupAllowlist.allowed && groupAllowlist.groupConfig) {
|
||||
chatExplicitlyAllowed = true;
|
||||
}
|
||||
}
|
||||
if (groupPolicy === "allowlist" && params.enforceAllowlistAuthorization) {
|
||||
const senderId = params.senderId ?? "";
|
||||
if (params.requireSenderForAllowlistAuthorization && !senderId) {
|
||||
return { allowed: false, reason: "group-policy-allowlist-no-sender", groupPolicy };
|
||||
}
|
||||
if (!params.allowEmptyAllowlistEntries && !params.effectiveGroupAllow.hasEntries) {
|
||||
// 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
|
||||
) {
|
||||
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({
|
||||
@@ -149,11 +175,5 @@ export const evaluateTelegramGroupPolicyAccess = (params: {
|
||||
return { allowed: false, reason: "group-policy-allowlist-unauthorized", groupPolicy };
|
||||
}
|
||||
}
|
||||
if (params.checkChatAllowlist) {
|
||||
const groupAllowlist = params.resolveGroupPolicy(params.chatId);
|
||||
if (groupAllowlist.allowlistEnabled && !groupAllowlist.allowed) {
|
||||
return { allowed: false, reason: "group-chat-not-allowed", groupPolicy };
|
||||
}
|
||||
}
|
||||
return { allowed: true, groupPolicy };
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user