From 9e147f00b48e63e7be6964e0e2a97f2980854128 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 14 Feb 2026 16:47:13 +0100 Subject: [PATCH] fix(doctor): resolve telegram allowFrom usernames --- CHANGELOG.md | 2 +- src/commands/doctor-config-flow.e2e.test.ts | 82 +++++- src/commands/doctor-config-flow.ts | 289 ++++++++++++++++++++ src/config/types.telegram.ts | 7 +- 4 files changed, 375 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb8c9e3ee..41f0f0b11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Feishu/Security: harden media URL fetching against SSRF and local file disclosure. (#16285) Thanks @mbelinky. -- Telegram/Security: require numeric Telegram sender IDs for allowlist authorization (reject `@username` principals) and warn in `openclaw security audit` when legacy configs contain usernames. Thanks @vincentkoc. +- Telegram/Security: require numeric Telegram sender IDs for allowlist authorization (reject `@username` principals), auto-resolve `@username` to IDs in `openclaw doctor --fix` (when possible), and warn in `openclaw security audit` when legacy configs contain usernames. Thanks @vincentkoc. - Security/Skills: harden archive extraction for download-installed skills to prevent path traversal outside the target directory. Thanks @markmusson. - Security/Media: stream and bound URL-backed input media fetches to prevent memory exhaustion from oversized responses. Thanks @vincentkoc. - Security/Signal: harden signal-cli archive extraction during install to prevent path traversal outside the install root. diff --git a/src/commands/doctor-config-flow.e2e.test.ts b/src/commands/doctor-config-flow.e2e.test.ts index 548a77f9f..d544c5a42 100644 --- a/src/commands/doctor-config-flow.e2e.test.ts +++ b/src/commands/doctor-config-flow.e2e.test.ts @@ -1,6 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { withTempHome } from "../../test/helpers/temp-home.js"; import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js"; @@ -64,4 +64,84 @@ describe("doctor config flow", () => { }); }); }); + + it("resolves Telegram @username allowFrom entries to numeric IDs on repair", async () => { + const fetchSpy = vi.fn(async (url: string) => { + const u = String(url); + const chatId = new URL(u).searchParams.get("chat_id") ?? ""; + const id = + chatId.toLowerCase() === "@testuser" + ? 111 + : chatId.toLowerCase() === "@groupuser" + ? 222 + : chatId.toLowerCase() === "@topicuser" + ? 333 + : chatId.toLowerCase() === "@accountuser" + ? 444 + : null; + return { + ok: id != null, + json: async () => (id != null ? { ok: true, result: { id } } : { ok: false }), + } as unknown as Response; + }); + vi.stubGlobal("fetch", fetchSpy); + try { + await withTempHome(async (home) => { + const configDir = path.join(home, ".openclaw"); + await fs.mkdir(configDir, { recursive: true }); + await fs.writeFile( + path.join(configDir, "openclaw.json"), + JSON.stringify( + { + channels: { + telegram: { + botToken: "123:abc", + allowFrom: ["@testuser"], + groupAllowFrom: ["groupUser"], + groups: { + "-100123": { + allowFrom: ["tg:@topicUser"], + topics: { "99": { allowFrom: ["@accountUser"] } }, + }, + }, + accounts: { + alerts: { botToken: "456:def", allowFrom: ["@accountUser"] }, + }, + }, + }, + }, + null, + 2, + ), + "utf-8", + ); + + const result = await loadAndMaybeMigrateDoctorConfig({ + options: { nonInteractive: true, repair: true }, + confirm: async () => false, + }); + + const cfg = result.cfg as unknown as { + channels: { + telegram: { + allowFrom: string[]; + groupAllowFrom: string[]; + groups: Record< + string, + { allowFrom: string[]; topics: Record } + >; + accounts: Record; + }; + }; + }; + expect(cfg.channels.telegram.allowFrom).toEqual(["111"]); + expect(cfg.channels.telegram.groupAllowFrom).toEqual(["222"]); + expect(cfg.channels.telegram.groups["-100123"].allowFrom).toEqual(["333"]); + expect(cfg.channels.telegram.groups["-100123"].topics["99"].allowFrom).toEqual(["444"]); + expect(cfg.channels.telegram.accounts.alerts.allowFrom).toEqual(["444"]); + }); + } finally { + vi.unstubAllGlobals(); + } + }); }); diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 6f59e9906..09d5eef8e 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -11,6 +11,7 @@ import { readConfigFileSnapshot, } from "../config/config.js"; import { applyPluginAutoEnable } from "../config/plugin-auto-enable.js"; +import { listTelegramAccountIds, resolveTelegramAccount } from "../telegram/accounts.js"; import { note } from "../terminal/note.js"; import { isRecord, resolveHomeDir } from "../utils.js"; import { normalizeLegacyConfigValues } from "./doctor-legacy-config.js"; @@ -142,6 +143,273 @@ function noteOpencodeProviderOverrides(cfg: OpenClawConfig) { note(lines.join("\n"), "OpenCode Zen"); } +function normalizeTelegramAllowFromEntry(raw: unknown): string { + const base = typeof raw === "string" ? raw : typeof raw === "number" ? String(raw) : ""; + return base + .trim() + .replace(/^(telegram|tg):/i, "") + .trim(); +} + +function isNumericTelegramUserId(raw: string): boolean { + return /^\d+$/.test(raw); +} + +type TelegramAllowFromUsernameHit = { path: string; entry: string }; + +function scanTelegramAllowFromUsernameEntries(cfg: OpenClawConfig): TelegramAllowFromUsernameHit[] { + const hits: TelegramAllowFromUsernameHit[] = []; + const telegram = cfg.channels?.telegram; + if (!telegram) { + return hits; + } + + const scanList = (pathLabel: string, list: unknown) => { + if (!Array.isArray(list)) { + return; + } + for (const entry of list) { + const normalized = normalizeTelegramAllowFromEntry(entry); + if (!normalized || normalized === "*") { + continue; + } + if (isNumericTelegramUserId(normalized)) { + continue; + } + hits.push({ path: pathLabel, entry: String(entry).trim() }); + } + }; + + const scanAccount = (prefix: string, account: Record) => { + scanList(`${prefix}.allowFrom`, account.allowFrom); + scanList(`${prefix}.groupAllowFrom`, account.groupAllowFrom); + const groups = account.groups; + if (!groups || typeof groups !== "object" || Array.isArray(groups)) { + return; + } + const groupsRecord = groups as Record; + for (const groupId of Object.keys(groupsRecord)) { + const group = groupsRecord[groupId]; + if (!group || typeof group !== "object" || Array.isArray(group)) { + continue; + } + const groupRec = group as Record; + scanList(`${prefix}.groups.${groupId}.allowFrom`, groupRec.allowFrom); + const topics = groupRec.topics; + if (!topics || typeof topics !== "object" || Array.isArray(topics)) { + continue; + } + const topicsRecord = topics as Record; + for (const topicId of Object.keys(topicsRecord)) { + const topic = topicsRecord[topicId]; + if (!topic || typeof topic !== "object" || Array.isArray(topic)) { + continue; + } + scanList( + `${prefix}.groups.${groupId}.topics.${topicId}.allowFrom`, + (topic as Record).allowFrom, + ); + } + } + }; + + scanAccount("channels.telegram", telegram as unknown as Record); + + const accounts = telegram.accounts; + if (!accounts || typeof accounts !== "object" || Array.isArray(accounts)) { + return hits; + } + for (const key of Object.keys(accounts)) { + const account = accounts[key]; + if (!account || typeof account !== "object" || Array.isArray(account)) { + continue; + } + scanAccount(`channels.telegram.accounts.${key}`, account as Record); + } + + return hits; +} + +async function maybeRepairTelegramAllowFromUsernames(cfg: OpenClawConfig): Promise<{ + config: OpenClawConfig; + changes: string[]; +}> { + const hits = scanTelegramAllowFromUsernameEntries(cfg); + if (hits.length === 0) { + return { config: cfg, changes: [] }; + } + + const tokens = Array.from( + new Set( + listTelegramAccountIds(cfg) + .map((accountId) => resolveTelegramAccount({ cfg, accountId })) + .map((account) => (account.tokenSource === "none" ? "" : account.token)) + .map((token) => token.trim()) + .filter(Boolean), + ), + ); + + if (tokens.length === 0) { + return { + config: cfg, + changes: [ + `- Telegram allowFrom contains @username entries, but no Telegram bot token is configured; cannot auto-resolve (run onboarding or replace with numeric sender IDs).`, + ], + }; + } + + const resolveUserId = async (raw: string): Promise => { + const trimmed = raw.trim(); + if (!trimmed) { + return null; + } + const stripped = normalizeTelegramAllowFromEntry(trimmed); + if (!stripped || stripped === "*") { + return null; + } + if (isNumericTelegramUserId(stripped)) { + return stripped; + } + if (/\s/.test(stripped)) { + return null; + } + const username = stripped.startsWith("@") ? stripped : `@${stripped}`; + for (const token of tokens) { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 4000); + try { + const url = `https://api.telegram.org/bot${token}/getChat?chat_id=${encodeURIComponent(username)}`; + const res = await fetch(url, { signal: controller.signal }).catch(() => null); + if (!res || !res.ok) { + continue; + } + const data = (await res.json().catch(() => null)) as { + ok?: boolean; + result?: { id?: number | string }; + } | null; + const id = data?.ok ? data?.result?.id : undefined; + if (typeof id === "number" || typeof id === "string") { + return String(id); + } + } catch { + // ignore and try next token + } finally { + clearTimeout(timeout); + } + } + return null; + }; + + const changes: string[] = []; + const next = structuredClone(cfg); + + const repairList = async (pathLabel: string, holder: Record, key: string) => { + const raw = holder[key]; + if (!Array.isArray(raw)) { + return; + } + const out: Array = []; + const replaced: Array<{ from: string; to: string }> = []; + for (const entry of raw) { + const normalized = normalizeTelegramAllowFromEntry(entry); + if (!normalized) { + continue; + } + if (normalized === "*") { + out.push("*"); + continue; + } + if (isNumericTelegramUserId(normalized)) { + out.push(normalized); + continue; + } + const resolved = await resolveUserId(String(entry)); + if (resolved) { + out.push(resolved); + replaced.push({ from: String(entry).trim(), to: resolved }); + } else { + out.push(String(entry).trim()); + } + } + const deduped: Array = []; + const seen = new Set(); + for (const entry of out) { + const k = String(entry).trim(); + if (!k || seen.has(k)) { + continue; + } + seen.add(k); + deduped.push(entry); + } + holder[key] = deduped; + if (replaced.length > 0) { + for (const rep of replaced.slice(0, 5)) { + changes.push(`- ${pathLabel}: resolved ${rep.from} -> ${rep.to}`); + } + if (replaced.length > 5) { + changes.push(`- ${pathLabel}: resolved ${replaced.length - 5} more @username entries`); + } + } + }; + + const repairAccount = async (prefix: string, account: Record) => { + await repairList(`${prefix}.allowFrom`, account, "allowFrom"); + await repairList(`${prefix}.groupAllowFrom`, account, "groupAllowFrom"); + const groups = account.groups; + if (!groups || typeof groups !== "object" || Array.isArray(groups)) { + return; + } + const groupsRecord = groups as Record; + for (const groupId of Object.keys(groupsRecord)) { + const group = groupsRecord[groupId]; + if (!group || typeof group !== "object" || Array.isArray(group)) { + continue; + } + const groupRec = group as Record; + await repairList(`${prefix}.groups.${groupId}.allowFrom`, groupRec, "allowFrom"); + const topics = groupRec.topics; + if (!topics || typeof topics !== "object" || Array.isArray(topics)) { + continue; + } + const topicsRecord = topics as Record; + for (const topicId of Object.keys(topicsRecord)) { + const topic = topicsRecord[topicId]; + if (!topic || typeof topic !== "object" || Array.isArray(topic)) { + continue; + } + await repairList( + `${prefix}.groups.${groupId}.topics.${topicId}.allowFrom`, + topic as Record, + "allowFrom", + ); + } + } + }; + + const telegram = next.channels?.telegram; + if (telegram && typeof telegram === "object" && !Array.isArray(telegram)) { + await repairAccount("channels.telegram", telegram as unknown as Record); + const accounts = (telegram as Record).accounts; + if (accounts && typeof accounts === "object" && !Array.isArray(accounts)) { + for (const key of Object.keys(accounts as Record)) { + const account = (accounts as Record)[key]; + if (!account || typeof account !== "object" || Array.isArray(account)) { + continue; + } + await repairAccount( + `channels.telegram.accounts.${key}`, + account as Record, + ); + } + } + } + + if (changes.length === 0) { + return { config: cfg, changes: [] }; + } + return { config: next, changes }; +} + async function maybeMigrateLegacyConfig(): Promise { const changes: string[] = []; const home = resolveHomeDir(); @@ -271,6 +539,27 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { } } + if (shouldRepair) { + const repair = await maybeRepairTelegramAllowFromUsernames(candidate); + if (repair.changes.length > 0) { + note(repair.changes.join("\n"), "Doctor changes"); + candidate = repair.config; + pendingChanges = true; + cfg = repair.config; + } + } else { + const hits = scanTelegramAllowFromUsernameEntries(candidate); + if (hits.length > 0) { + note( + [ + `- Telegram allowFrom contains ${hits.length} non-numeric entries (e.g. ${hits[0]?.entry ?? "@"}); Telegram authorization requires numeric sender IDs.`, + `- Run "${formatCliCommand("openclaw doctor --fix")}" to auto-resolve @username entries to numeric IDs (requires a Telegram bot token).`, + ].join("\n"), + "Doctor warnings", + ); + } + } + const unknown = stripUnknownConfigKeys(candidate); if (unknown.removed.length > 0) { const lines = unknown.removed.map((path) => `- ${path}`).join("\n"); diff --git a/src/config/types.telegram.ts b/src/config/types.telegram.ts index 32f9d7044..756caecfc 100644 --- a/src/config/types.telegram.ts +++ b/src/config/types.telegram.ts @@ -70,8 +70,9 @@ export type TelegramAccountConfig = { /** Control reply threading when reply tags are present (off|first|all). */ replyToMode?: ReplyToMode; groups?: Record; + /** DM allowlist (numeric Telegram user IDs). Onboarding can resolve @username to IDs. */ allowFrom?: Array; - /** Optional allowlist for Telegram group senders (user ids or usernames). */ + /** Optional allowlist for Telegram group senders (numeric Telegram user IDs). */ groupAllowFrom?: Array; /** * Controls how group messages are handled: @@ -150,7 +151,7 @@ export type TelegramTopicConfig = { skills?: string[]; /** If false, disable the bot for this topic. */ enabled?: boolean; - /** Optional allowlist for topic senders (ids or usernames). */ + /** Optional allowlist for topic senders (numeric Telegram user IDs). */ allowFrom?: Array; /** Optional system prompt snippet for this topic. */ systemPrompt?: string; @@ -169,7 +170,7 @@ export type TelegramGroupConfig = { topics?: Record; /** If false, disable the bot for this group (and its topics). */ enabled?: boolean; - /** Optional allowlist for group senders (ids or usernames). */ + /** Optional allowlist for group senders (numeric Telegram user IDs). */ allowFrom?: Array; /** Optional system prompt snippet for this group. */ systemPrompt?: string;