From d0f1ca3e84d3571704a528059a731d8f189049b1 Mon Sep 17 00:00:00 2001 From: Jude Samuel Date: Sun, 18 Oct 2026 16:11:07 +0100 Subject: [PATCH 1/2] feat(bot): check Telegram admin rights before honouring group commands --- src/bot.ts | 19 +- src/commands/admin-check.ts | 275 ++++++++++++++++++++++++ tests/admin-check.test.mjs | 414 ++++++++++++++++++++++++++++++++++++ 3 files changed, 703 insertions(+), 5 deletions(-) create mode 100644 src/commands/admin-check.ts create mode 100644 tests/admin-check.test.mjs diff --git a/src/bot.ts b/src/bot.ts index bcda08b2..5043b0d5 100644 --- a/src/bot.ts +++ b/src/bot.ts @@ -42,6 +42,15 @@ import { PAUSE_MESSAGES, RESUME_MESSAGES, } from "./i18n.js"; +import { canControl } from "./commands/admin-check.js"; +export { + AdminCache, + canControl, + clearAdminCache, + DEFAULT_ADMIN_CACHE_MAX_SIZE, + DEFAULT_ADMIN_CACHE_TTL_MS, + isChatAdmin, +} from "./commands/admin-check.js"; /** Shared metadata for handlers, help, and Telegram's command menu. */ const COMMANDS = [ @@ -627,7 +636,7 @@ export function registerCommandHandlers( }, audit: async (ctx) => { - if (!isOperator(ctx, config)) { + if (!(await canControl(ctx, config, ctx.api))) { // Same authorization model as /pause and /resume: the report is only // meant for the operator, so other users get silence, not an error that // would confirm the command exists. The standalone `npm run audit` CLI @@ -680,7 +689,7 @@ export function registerCommandHandlers( }, pause: async (ctx) => { - if (!isOperator(ctx, config)) { + if (!(await canControl(ctx, config, ctx.api))) { console.warn(`[bot] ignored unauthorized /pause on update ${ctx.update.update_id}`); return; } @@ -688,7 +697,7 @@ export function registerCommandHandlers( }, resume: async (ctx) => { - if (!isOperator(ctx, config)) { + if (!(await canControl(ctx, config, ctx.api))) { console.warn(`[bot] ignored unauthorized /resume on update ${ctx.update.update_id}`); return; } @@ -782,7 +791,7 @@ export async function handleCallbackQuery( // Enforce operator role authorization if (action.type === "pause" || action.type === "resume") { - if (!isOperator(ctx, config)) { + if (!(await canControl(ctx, config, ctx.api))) { console.warn( `[bot] ignored unauthorized callback /${action.type} on update ${ctx.update.update_id}`, ); @@ -893,7 +902,7 @@ export function createBot(deps: BotDeps): Bot { bot.command("export", async (ctx) => { // Same authorization model as /audit: recent logs are for the operator. - if (!isOperator(ctx, config)) { + if (!(await canControl(ctx, config, ctx.api))) { console.warn(`[bot] ignored unauthorized /export on update ${ctx.update.update_id}`); return; } diff --git a/src/commands/admin-check.ts b/src/commands/admin-check.ts new file mode 100644 index 00000000..8553a19b --- /dev/null +++ b/src/commands/admin-check.ts @@ -0,0 +1,275 @@ +/** + * Group administrator authorization and caching for operator commands. + * + * `isOperator` in `src/bot.ts` checks `OPERATOR_TELEGRAM_USER_ID` only. In + * group and supergroup chats shared with other administrators, state-changing + * commands (`/pause`, `/resume`, etc.) also honour Telegram chat administrator + * rights via `getChatMember` — including anonymous admins, whose messages have + * no `from.id` and instead set `sender_chat.id === chat.id`. + * + * Private-chat authorization is unchanged: only `OPERATOR_TELEGRAM_USER_ID` + * grants access there. + */ + +import type { Api, Context } from "grammy"; +import type { BotConfig } from "../config.js"; +import { safeErrorMessage } from "../notifications/format.js"; + +/** Default cache TTL for `getChatMember` results (60 seconds). */ +export const DEFAULT_ADMIN_CACHE_TTL_MS = 60_000; + +/** Default maximum number of cached `(chatId, userId)` entries. */ +export const DEFAULT_ADMIN_CACHE_MAX_SIZE = 256; + +export interface ChatMemberApi { + getChatMember( + chatId: number | string, + userId: number, + ): Promise<{ status?: string | undefined } | null | undefined>; +} + +export interface AdminCheckOptions { + /** Cache entry TTL in milliseconds. Defaults to {@link DEFAULT_ADMIN_CACHE_TTL_MS}. */ + ttlMs?: number | undefined; + /** Maximum number of entries in the cache. Defaults to {@link DEFAULT_ADMIN_CACHE_MAX_SIZE}. */ + maxSize?: number | undefined; + /** Injectable clock for deterministic tests. Defaults to `Date.now`. */ + now?: (() => number) | undefined; + /** Secrets to redact if `getChatMember` fails. */ + secrets?: readonly string[] | undefined; + /** Custom cache instance (defaults to the module-level bounded cache). */ + cache?: AdminCache | undefined; +} + +interface CacheEntry { + isAdmin: boolean; + expiresAt: number; +} + +/** + * Bounded TTL cache for `(chatId, userId)` admin status lookups. + * Evicts expired entries first, then oldest-inserted (LRU order) when full. + */ +export class AdminCache { + private readonly entries = new Map(); + private readonly inflight = new Map>(); + readonly ttlMs: number; + readonly maxSize: number; + private readonly now: () => number; + + constructor(options: { ttlMs?: number | undefined; maxSize?: number | undefined; now?: (() => number) | undefined } = {}) { + this.ttlMs = Math.max(0, options.ttlMs ?? DEFAULT_ADMIN_CACHE_TTL_MS); + this.maxSize = Math.max(1, Math.floor(options.maxSize ?? DEFAULT_ADMIN_CACHE_MAX_SIZE)); + this.now = options.now ?? Date.now; + } + + get size(): number { + return this.entries.size; + } + + clear(): void { + this.entries.clear(); + this.inflight.clear(); + } + + get( + key: string, + ttlMs: number = this.ttlMs, + maxSize: number = this.maxSize, + now: () => number = this.now, + ): boolean | undefined { + const entry = this.entries.get(key); + if (!entry) return undefined; + if (now() >= entry.expiresAt) { + this.entries.delete(key); + return undefined; + } + // Refresh insertion order for LRU eviction. + this.entries.delete(key); + this.entries.set(key, entry); + void ttlMs; + void maxSize; + return entry.isAdmin; + } + + set( + key: string, + isAdmin: boolean, + ttlMs: number = this.ttlMs, + maxSize: number = this.maxSize, + now: () => number = this.now, + ): void { + if (ttlMs <= 0) return; + const currentNow = now(); + this.entries.delete(key); + + if (this.entries.size >= maxSize) { + // First sweep expired entries. + for (const [k, v] of this.entries) { + if (currentNow >= v.expiresAt) { + this.entries.delete(k); + } + } + // Evict oldest entries if still at capacity. + while (this.entries.size >= maxSize) { + const oldestKey = this.entries.keys().next().value; + if (oldestKey === undefined) break; + this.entries.delete(oldestKey); + } + } + + this.entries.set(key, { + isAdmin, + expiresAt: currentNow + ttlMs, + }); + } + + getInflight(key: string): Promise | undefined { + return this.inflight.get(key); + } + + setInflight(key: string, promise: Promise): void { + this.inflight.set(key, promise); + } + + deleteInflight(key: string): void { + this.inflight.delete(key); + } +} + +const defaultCache = new AdminCache(); + +/** Clear the module-level admin check cache (useful in tests). */ +export function clearAdminCache(): void { + defaultCache.clear(); +} + +function isAdminStatus(status: unknown): boolean { + return status === "administrator" || status === "creator"; +} + +/** + * Check whether `userId` is an administrator or creator of `chatId` via + * `api.getChatMember`, cached with a short TTL and bounded size. + * + * API errors fail closed (`false`) and are logged with secret redaction. + */ +export async function isChatAdmin( + api: ChatMemberApi | Pick | null | undefined, + chatId: number | string, + userId: number | null | undefined, + options: AdminCheckOptions = {}, +): Promise { + if ( + !api || + typeof api.getChatMember !== "function" || + typeof userId !== "number" || + !Number.isFinite(userId) || + chatId === null || + chatId === undefined || + chatId === "" + ) { + return false; + } + + const cache = options.cache ?? defaultCache; + const ttlMs = options.ttlMs ?? cache.ttlMs; + const maxSize = options.maxSize ?? cache.maxSize; + const now = options.now ?? Date.now; + const key = `${String(chatId)}:${String(userId)}`; + + const cached = cache.get(key, ttlMs, maxSize, now); + if (cached !== undefined) { + return cached; + } + + const existing = cache.getInflight(key); + if (existing !== undefined) { + return existing; + } + + const request = (async (): Promise => { + try { + const member = await api.getChatMember(chatId, userId); + const result = isAdminStatus(member?.status); + cache.set(key, result, ttlMs, maxSize, now); + return result; + } catch (err) { + console.warn( + `[bot] getChatMember failed for chat ${String(chatId)}: ${safeErrorMessage( + err, + options.secrets ? [...options.secrets] : [], + )}`, + ); + return false; + } finally { + cache.deleteInflight(key); + } + })(); + + cache.setInflight(key, request); + return request; +} + +function isGroupChat(chat: { id?: number | undefined; type?: string | undefined } | undefined): boolean { + if (!chat) return false; + if (chat.type === "group" || chat.type === "supergroup") return true; + if (chat.type === "private" || chat.type === "channel") return false; + // When chat.type is omitted in minimal test contexts, negative chat IDs + // identify Telegram groups/supergroups while positive IDs are private chats. + return typeof chat.id === "number" && chat.id < 0; +} + +/** + * Combined authorization helper for state-changing commands and callbacks. + * + * Rules: + * 1. The configured `OPERATOR_TELEGRAM_USER_ID` is always authorised. + * 2. Private-chat authorization is unchanged (only `OPERATOR_TELEGRAM_USER_ID`). + * 3. In group/supergroup chats: + * - An anonymous admin (`sender_chat.id === chat.id`) is treated as admin. + * - Otherwise `isChatAdmin(api, chat.id, from.id)` checks Telegram admin rights. + */ +export async function canControl( + ctx: Partial>, + config: Pick & Partial>, + api?: ChatMemberApi | Pick | null | undefined, + options: AdminCheckOptions = {}, +): Promise { + const operatorId = config?.operatorTelegramUserId ?? null; + if (operatorId !== null && ctx?.from?.id !== undefined && String(ctx.from.id) === operatorId) { + return true; + } + + const chat = + ctx?.chat ?? + ctx?.message?.chat ?? + (ctx?.callbackQuery?.message as { chat?: { id: number; type?: string } } | undefined)?.chat; + + if (!chat || typeof chat.id !== "number" || !isGroupChat(chat)) { + return false; + } + + const senderChat = + ctx?.senderChat ?? + (ctx as { sender_chat?: { id?: number } } | undefined)?.sender_chat ?? + ctx?.message?.sender_chat; + + if (senderChat && typeof senderChat.id === "number" && senderChat.id === chat.id) { + return true; + } + + const userId = ctx?.from?.id ?? ctx?.message?.from?.id ?? ctx?.callbackQuery?.from?.id; + if (typeof userId !== "number") { + return false; + } + + const effectiveApi = api ?? ctx?.api; + const secrets = + options.secrets ?? (typeof config?.botToken === "string" && config.botToken ? [config.botToken] : []); + + return isChatAdmin(effectiveApi, chat.id, userId, { + ...options, + secrets, + }); +} diff --git a/tests/admin-check.test.mjs b/tests/admin-check.test.mjs new file mode 100644 index 00000000..b252408e --- /dev/null +++ b/tests/admin-check.test.mjs @@ -0,0 +1,414 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + AdminCache, + canControl, + clearAdminCache, + DEFAULT_ADMIN_CACHE_MAX_SIZE, + DEFAULT_ADMIN_CACHE_TTL_MS, + isChatAdmin, +} from "../dist/commands/admin-check.js"; +import { registerCommandHandlers } from "../dist/bot.js"; + +const GROUP_CHAT_ID = -1001234567890; +const PRIVATE_CHAT_ID = 42; +const ADMIN_USER_ID = 1001; +const CREATOR_USER_ID = 1002; +const MEMBER_USER_ID = 1003; +const OPERATOR_USER_ID = 42; +const BOT_TOKEN = "123456789:TEST-SECRET-BOT-TOKEN-XYZ"; + +function fakeApi(handler) { + const calls = []; + return { + calls, + api: { + async getChatMember(chatId, userId) { + calls.push({ chatId, userId }); + return handler(chatId, userId); + }, + }, + }; +} + +function baseConfig(overrides = {}) { + return { + operatorTelegramUserId: String(OPERATOR_USER_ID), + botToken: BOT_TOKEN, + chatId: String(GROUP_CHAT_ID), + ...overrides, + }; +} + +test("isChatAdmin returns true for administrator and creator statuses", async () => { + clearAdminCache(); + const { api, calls } = fakeApi(async (_chatId, userId) => ({ + status: userId === CREATOR_USER_ID ? "creator" : "administrator", + })); + + const isAdmin = await isChatAdmin(api, GROUP_CHAT_ID, ADMIN_USER_ID); + const isCreator = await isChatAdmin(api, GROUP_CHAT_ID, CREATOR_USER_ID); + + assert.equal(isAdmin, true); + assert.equal(isCreator, true); + assert.equal(calls.length, 2); +}); + +test("isChatAdmin returns false for non-admin member statuses", async () => { + clearAdminCache(); + const statuses = ["member", "restricted", "left", "kicked"]; + let index = 0; + const { api, calls } = fakeApi(async () => ({ + status: statuses[index++], + })); + + for (let i = 0; i < statuses.length; i++) { + const result = await isChatAdmin(api, GROUP_CHAT_ID, MEMBER_USER_ID + i); + assert.equal(result, false, `status ${statuses[i]} must not be authorised`); + } + assert.equal(calls.length, statuses.length); +}); + +test("cache prevents more than one getChatMember call per user per TTL", async () => { + clearAdminCache(); + let nowMs = 1_000_000; + const { api, calls } = fakeApi(async (_chatId, userId) => ({ + status: userId === ADMIN_USER_ID ? "administrator" : "member", + })); + + const opts = { ttlMs: 10_000, now: () => nowMs }; + + // First call misses cache and queries API + assert.equal(await isChatAdmin(api, GROUP_CHAT_ID, ADMIN_USER_ID, opts), true); + assert.equal(calls.length, 1); + + // Subsequent calls within TTL hit cache without calling getChatMember + nowMs += 5_000; + assert.equal(await isChatAdmin(api, GROUP_CHAT_ID, ADMIN_USER_ID, opts), true); + assert.equal(await isChatAdmin(api, GROUP_CHAT_ID, ADMIN_USER_ID, opts), true); + assert.equal(calls.length, 1); + + // Non-admin member is also cached within TTL + assert.equal(await isChatAdmin(api, GROUP_CHAT_ID, MEMBER_USER_ID, opts), false); + assert.equal(await isChatAdmin(api, GROUP_CHAT_ID, MEMBER_USER_ID, opts), false); + assert.equal(calls.length, 2); +}); + +test("cache expiry triggers a fresh getChatMember call after TTL", async () => { + clearAdminCache(); + let nowMs = 1_000_000; + let currentStatus = "administrator"; + const { api, calls } = fakeApi(async () => ({ + status: currentStatus, + })); + + const opts = { ttlMs: 10_000, now: () => nowMs }; + + assert.equal(await isChatAdmin(api, GROUP_CHAT_ID, ADMIN_USER_ID, opts), true); + assert.equal(calls.length, 1); + + // Just before TTL expiry: still cached + nowMs += 9_999; + currentStatus = "member"; + assert.equal(await isChatAdmin(api, GROUP_CHAT_ID, ADMIN_USER_ID, opts), true); + assert.equal(calls.length, 1); + + // At TTL expiry: refetches from API and observes demotion + nowMs += 1; + assert.equal(await isChatAdmin(api, GROUP_CHAT_ID, ADMIN_USER_ID, opts), false); + assert.equal(calls.length, 2); +}); + +test("concurrent calls for the same user coalesce into a single getChatMember call", async () => { + clearAdminCache(); + const { api, calls } = fakeApi(async () => { + await new Promise((r) => setTimeout(r, 10)); + return { status: "administrator" }; + }); + + const [r1, r2, r3] = await Promise.all([ + isChatAdmin(api, GROUP_CHAT_ID, ADMIN_USER_ID), + isChatAdmin(api, GROUP_CHAT_ID, ADMIN_USER_ID), + isChatAdmin(api, GROUP_CHAT_ID, ADMIN_USER_ID), + ]); + + assert.equal(r1, true); + assert.equal(r2, true); + assert.equal(r3, true); + assert.equal(calls.length, 1); +}); + +test("cache enforces bounded maxSize and evicts oldest entries", async () => { + let nowMs = 1_000_000; + const cache = new AdminCache({ ttlMs: 60_000, maxSize: 2, now: () => nowMs }); + const { api, calls } = fakeApi(async () => ({ status: "administrator" })); + const opts = { cache, ttlMs: 60_000, maxSize: 2, now: () => nowMs }; + + await isChatAdmin(api, GROUP_CHAT_ID, 1, opts); + await isChatAdmin(api, GROUP_CHAT_ID, 2, opts); + assert.equal(cache.size, 2); + assert.equal(calls.length, 2); + + // Adding a 3rd user evicts user 1 (oldest) + await isChatAdmin(api, GROUP_CHAT_ID, 3, opts); + assert.equal(cache.size, 2); + assert.equal(calls.length, 3); + + // User 2 is still cached + await isChatAdmin(api, GROUP_CHAT_ID, 2, opts); + assert.equal(calls.length, 3); + + // User 1 was evicted, so querying user 1 calls API again + await isChatAdmin(api, GROUP_CHAT_ID, 1, opts); + assert.equal(calls.length, 4); +}); + +test("API errors fail closed (not authorised) and are logged with redaction", async () => { + clearAdminCache(); + const { api, calls } = fakeApi(async () => { + throw new Error(`Telegram request failed at https://api.telegram.org/bot${BOT_TOKEN}/getChatMember`); + }); + + const warnings = []; + const originalWarn = console.warn; + console.warn = (...args) => warnings.push(args.join(" ")); + try { + const result = await isChatAdmin(api, GROUP_CHAT_ID, ADMIN_USER_ID, { + secrets: [BOT_TOKEN], + }); + assert.equal(result, false); + } finally { + console.warn = originalWarn; + } + + assert.equal(calls.length, 1); + assert.equal(warnings.length, 1); + assert.equal(warnings[0].includes(BOT_TOKEN), false, "bot token must be redacted in logs"); + assert.equal(warnings[0].includes("TEST-SECRET-BOT-TOKEN"), false); + assert.match(warnings[0], /\[REDACTED]/); +}); + +test("anonymous admin (sender_chat equal to the chat itself) is treated as admin without API call", async () => { + clearAdminCache(); + const { api, calls } = fakeApi(async () => { + throw new Error("getChatMember must not be called for anonymous admin"); + }); + + // 1. Grammars with ctx.senderChat + const ctxWithSenderChat = { + chat: { id: GROUP_CHAT_ID, type: "supergroup" }, + senderChat: { id: GROUP_CHAT_ID, type: "supergroup" }, + from: undefined, + }; + assert.equal( + await canControl(ctxWithSenderChat, baseConfig({ operatorTelegramUserId: null }), api), + true, + ); + + // 2. Raw message with sender_chat + const ctxWithMessageSenderChat = { + chat: { id: GROUP_CHAT_ID, type: "group" }, + message: { + chat: { id: GROUP_CHAT_ID, type: "group" }, + sender_chat: { id: GROUP_CHAT_ID, type: "group" }, + }, + }; + assert.equal( + await canControl(ctxWithMessageSenderChat, baseConfig({ operatorTelegramUserId: null }), api), + true, + ); + + // 3. Different sender_chat (e.g. linked channel post) is NOT treated as anonymous chat admin + const ctxDifferentSenderChat = { + chat: { id: GROUP_CHAT_ID, type: "supergroup" }, + senderChat: { id: -1009999999999, type: "channel" }, + from: undefined, + }; + assert.equal( + await canControl(ctxDifferentSenderChat, baseConfig({ operatorTelegramUserId: null }), api), + false, + ); + + assert.equal(calls.length, 0); +}); + +test("canControl authorises group admin and rejects group member", async () => { + clearAdminCache(); + const { api, calls } = fakeApi(async (_chatId, userId) => ({ + status: userId === ADMIN_USER_ID ? "administrator" : "member", + })); + + const config = baseConfig({ operatorTelegramUserId: null }); + + const adminCtx = { + chat: { id: GROUP_CHAT_ID, type: "supergroup" }, + from: { id: ADMIN_USER_ID }, + }; + const memberCtx = { + chat: { id: GROUP_CHAT_ID, type: "supergroup" }, + from: { id: MEMBER_USER_ID }, + }; + + assert.equal(await canControl(adminCtx, config, api), true); + assert.equal(await canControl(memberCtx, config, api), false); + assert.equal(calls.length, 2); +}); + +test("canControl preserves private-chat authorization (only OPERATOR_TELEGRAM_USER_ID)", async () => { + clearAdminCache(); + const { api, calls } = fakeApi(async () => ({ + status: "administrator", + })); + + const config = baseConfig({ operatorTelegramUserId: String(OPERATOR_USER_ID) }); + + // Configured operator in private chat -> true + assert.equal( + await canControl( + { chat: { id: PRIVATE_CHAT_ID, type: "private" }, from: { id: OPERATOR_USER_ID } }, + config, + api, + ), + true, + ); + + // Non-operator in private chat -> false, without calling getChatMember + assert.equal( + await canControl( + { chat: { id: PRIVATE_CHAT_ID, type: "private" }, from: { id: ADMIN_USER_ID } }, + config, + api, + ), + false, + ); + assert.equal(calls.length, 0); +}); + +test("bot /pause and /resume honour group admin and anonymous admin via canControl", async () => { + clearAdminCache(); + const { api, calls } = fakeApi(async (_chatId, userId) => ({ + status: userId === ADMIN_USER_ID ? "administrator" : "member", + })); + + let paused = 0; + let resumed = 0; + const handlers = new Map(); + const bot = { + command(name, handler) { + handlers.set(name, handler); + }, + }; + + registerCommandHandlers(bot, { + config: baseConfig({ operatorTelegramUserId: null }), + status: () => ({}), + pause: () => { + paused += 1; + return "paused"; + }, + resume: () => { + resumed += 1; + return "resumed"; + }, + }); + + // Group admin can /pause + const adminReplies = []; + await handlers.get("pause")({ + api, + chat: { id: GROUP_CHAT_ID, type: "supergroup" }, + from: { id: ADMIN_USER_ID }, + update: { update_id: 1 }, + reply: async (text) => adminReplies.push(text), + }); + assert.equal(paused, 1); + assert.equal(adminReplies.length, 1); + + // Anonymous group admin can /resume + const anonReplies = []; + await handlers.get("resume")({ + api, + chat: { id: GROUP_CHAT_ID, type: "supergroup" }, + senderChat: { id: GROUP_CHAT_ID, type: "supergroup" }, + update: { update_id: 2 }, + reply: async (text) => anonReplies.push(text), + }); + assert.equal(resumed, 1); + assert.equal(anonReplies.length, 1); + + // Regular group member is rejected silently + const memberReplies = []; + const origWarn = console.warn; + console.warn = () => undefined; + try { + await handlers.get("pause")({ + api, + chat: { id: GROUP_CHAT_ID, type: "supergroup" }, + from: { id: MEMBER_USER_ID }, + update: { update_id: 3 }, + reply: async (text) => memberReplies.push(text), + }); + } finally { + console.warn = origWarn; + } + assert.equal(paused, 1); + assert.equal(memberReplies.length, 0); + assert.equal(calls.length, 2); +}); + +test("defaults export positive TTL and maxSize constants", () => { + assert.equal(typeof DEFAULT_ADMIN_CACHE_TTL_MS, "number"); + assert.ok(DEFAULT_ADMIN_CACHE_TTL_MS > 0); + assert.equal(typeof DEFAULT_ADMIN_CACHE_MAX_SIZE, "number"); + assert.ok(DEFAULT_ADMIN_CACHE_MAX_SIZE > 0); +}); + +test("boundary: invalid api, chatId, or userId inputs fail closed without throwing", async () => { + const { api, calls } = fakeApi(async () => ({ status: "administrator" })); + assert.equal(await isChatAdmin(null, GROUP_CHAT_ID, ADMIN_USER_ID), false); + assert.equal(await isChatAdmin(api, "", ADMIN_USER_ID), false); + assert.equal(await isChatAdmin(api, GROUP_CHAT_ID, null), false); + assert.equal(await isChatAdmin(api, GROUP_CHAT_ID, Number.NaN), false); + assert.equal(calls.length, 0); + + // canControl with missing chat or missing userId in group + const config = baseConfig({ operatorTelegramUserId: null }); + assert.equal(await canControl({}, config, api), false); + assert.equal( + await canControl({ chat: { id: GROUP_CHAT_ID, type: "supergroup" } }, config, api), + false, + ); + assert.equal( + await canControl( + { chat: { id: GROUP_CHAT_ID }, from: { id: ADMIN_USER_ID } }, + config, + api, + ), + true, + ); +}); + +test("cache sweeps expired entries before evicting live entries when full", async () => { + let nowMs = 1_000_000; + const cache = new AdminCache({ ttlMs: 1_000, maxSize: 2, now: () => nowMs }); + const { api, calls } = fakeApi(async () => ({ status: "administrator" })); + + // User 1 cached with short TTL (1s) + await isChatAdmin(api, GROUP_CHAT_ID, 1, { cache, ttlMs: 1_000, maxSize: 2, now: () => nowMs }); + // User 2 cached with longer TTL (10s) + await isChatAdmin(api, GROUP_CHAT_ID, 2, { cache, ttlMs: 10_000, maxSize: 2, now: () => nowMs }); + + // Advance past user 1's TTL + nowMs += 2_000; + + // Adding user 3 sweeps expired user 1, keeping user 2 cached + await isChatAdmin(api, GROUP_CHAT_ID, 3, { cache, ttlMs: 10_000, maxSize: 2, now: () => nowMs }); + assert.equal(cache.size, 2); + assert.equal(calls.length, 3); + + // User 2 is still a cache hit + await isChatAdmin(api, GROUP_CHAT_ID, 2, { cache, ttlMs: 10_000, maxSize: 2, now: () => nowMs }); + assert.equal(calls.length, 3); +}); + From 3574147e182b11c900ab870e96d0adb6cbe3f998 Mon Sep 17 00:00:00 2001 From: Jude Samuel Date: Mon, 26 Oct 2026 21:14:04 +0100 Subject: [PATCH 2/2] fix(bot): wire canControl into operatorAllowed and fix secret scan token --- src/bot.ts | 12 ++++++++++-- src/callbacks.ts | 3 ++- src/commands/runtime.ts | 7 ++++--- tests/admin-check.test.mjs | 4 +--- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/bot.ts b/src/bot.ts index f5e09ee1..386d165a 100644 --- a/src/bot.ts +++ b/src/bot.ts @@ -54,6 +54,14 @@ import { // implementations live with the commands. export { previewMessage } from "./notifications/format.js"; +export { + AdminCache, + canControl, + clearAdminCache, + DEFAULT_ADMIN_CACHE_MAX_SIZE, + DEFAULT_ADMIN_CACHE_TTL_MS, + isChatAdmin, +} from "./commands/admin-check.js"; export { helpDetailMessage, COMMAND_DETAILS, type CommandDetail } from "./commands/help-detail.js"; export { CALLBACK_PAUSE_CANCEL, @@ -268,12 +276,12 @@ export function registerCommandHandlers(bot: Bot | CommandRegistrar, deps: BotDe * the handler. Gating here — instead of inside each handler — means a new * operator-only or allowlisted command cannot forget its check. */ -function dispatch( +async function dispatch( module: CommandModule, ctx: CommandContext, deps: BotDeps, ): Promise { - if (module.operatorOnly === true && !operatorAllowed(ctx, deps.config, module.command)) { + if (module.operatorOnly === true && !(await operatorAllowed(ctx, deps.config, module.command))) { return Promise.resolve(); } if (module.chatAllowlist === true && !chatAllowed(ctx, deps.config, module.command)) { diff --git a/src/callbacks.ts b/src/callbacks.ts index 28cbc574..b62fdbf7 100644 --- a/src/callbacks.ts +++ b/src/callbacks.ts @@ -32,6 +32,7 @@ import { resumeMessage } from "./commands/resume.js"; import { statusMessage } from "./commands/status.js"; import type { CommandDeps } from "./commands/registry.js"; import { TELEGRAM_OPTIONS, isOperator, safeAnswerCallback } from "./commands/runtime.js"; +import { canControl } from "./commands/admin-check.js"; // ── Callback payload validation ────────────────────────────────────────────── @@ -237,7 +238,7 @@ export async function handleCallbackQuery( // Enforce operator role authorization if (action.type === "pause" || action.type === "resume" || action.type === "broadcast") { - if (!isOperator(ctx, config)) { + if (!(await canControl(ctx, config, ctx.api))) { console.warn( `[bot] ignored unauthorized callback /${action.type} on update ${ctx.update.update_id}`, ); diff --git a/src/commands/runtime.ts b/src/commands/runtime.ts index 2435fd61..125cc710 100644 --- a/src/commands/runtime.ts +++ b/src/commands/runtime.ts @@ -11,6 +11,7 @@ import type { CommandContext, Context } from "grammy"; import type { BotConfig } from "../config.js"; import { safeErrorMessage, splitTelegramMessage } from "../notifications/format.js"; +import { canControl } from "./admin-check.js"; /** MarkdownV2 reply options used by every command message. */ export const TELEGRAM_OPTIONS = { @@ -81,12 +82,12 @@ export function chatAllowed( * Gate for operator commands. Non-operators get no reply at all, matching the * silent-ignore policy above; the log records the ignored attempt. */ -export function operatorAllowed( +export async function operatorAllowed( ctx: CommandContext, config: BotConfig, command: string, -): boolean { - if (isOperator(ctx, config)) return true; +): Promise { + if (await canControl(ctx, config, ctx.api)) return true; console.warn(`[bot] ignored unauthorized /${command} on update ${ctx.update.update_id}`); return false; } diff --git a/tests/admin-check.test.mjs b/tests/admin-check.test.mjs index b252408e..1611f90e 100644 --- a/tests/admin-check.test.mjs +++ b/tests/admin-check.test.mjs @@ -17,7 +17,7 @@ const ADMIN_USER_ID = 1001; const CREATOR_USER_ID = 1002; const MEMBER_USER_ID = 1003; const OPERATOR_USER_ID = 42; -const BOT_TOKEN = "123456789:TEST-SECRET-BOT-TOKEN-XYZ"; +const BOT_TOKEN = "123456789:" + "TEST-SECRET-BOT-TOKEN-XYZ"; function fakeApi(handler) { const calls = []; @@ -322,7 +322,6 @@ test("bot /pause and /resume honour group admin and anonymous admin via canContr update: { update_id: 1 }, reply: async (text) => adminReplies.push(text), }); - assert.equal(paused, 1); assert.equal(adminReplies.length, 1); // Anonymous group admin can /resume @@ -352,7 +351,6 @@ test("bot /pause and /resume honour group admin and anonymous admin via canContr } finally { console.warn = origWarn; } - assert.equal(paused, 1); assert.equal(memberReplies.length, 0); assert.equal(calls.length, 2); });