fix(doctor): resolve telegram allowFrom usernames

This commit is contained in:
Peter Steinberger
2026-02-14 16:47:13 +01:00
parent 50645b905b
commit 9e147f00b4
4 changed files with 375 additions and 5 deletions

View File

@@ -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.

View File

@@ -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<string, { allowFrom: string[] }> }
>;
accounts: Record<string, { allowFrom: string[] }>;
};
};
};
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();
}
});
});

View File

@@ -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<string, unknown>) => {
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<string, unknown>;
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<string, unknown>;
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<string, unknown>;
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<string, unknown>).allowFrom,
);
}
}
};
scanAccount("channels.telegram", telegram as unknown as Record<string, unknown>);
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<string, unknown>);
}
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<string | null> => {
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<string, unknown>, key: string) => {
const raw = holder[key];
if (!Array.isArray(raw)) {
return;
}
const out: Array<string | number> = [];
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<string | number> = [];
const seen = new Set<string>();
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<string, unknown>) => {
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<string, unknown>;
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<string, unknown>;
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<string, unknown>;
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<string, unknown>,
"allowFrom",
);
}
}
};
const telegram = next.channels?.telegram;
if (telegram && typeof telegram === "object" && !Array.isArray(telegram)) {
await repairAccount("channels.telegram", telegram as unknown as Record<string, unknown>);
const accounts = (telegram as Record<string, unknown>).accounts;
if (accounts && typeof accounts === "object" && !Array.isArray(accounts)) {
for (const key of Object.keys(accounts as Record<string, unknown>)) {
const account = (accounts as Record<string, unknown>)[key];
if (!account || typeof account !== "object" || Array.isArray(account)) {
continue;
}
await repairAccount(
`channels.telegram.accounts.${key}`,
account as Record<string, unknown>,
);
}
}
}
if (changes.length === 0) {
return { config: cfg, changes: [] };
}
return { config: next, changes };
}
async function maybeMigrateLegacyConfig(): Promise<string[]> {
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");

View File

@@ -70,8 +70,9 @@ export type TelegramAccountConfig = {
/** Control reply threading when reply tags are present (off|first|all). */
replyToMode?: ReplyToMode;
groups?: Record<string, TelegramGroupConfig>;
/** DM allowlist (numeric Telegram user IDs). Onboarding can resolve @username to IDs. */
allowFrom?: Array<string | number>;
/** Optional allowlist for Telegram group senders (user ids or usernames). */
/** Optional allowlist for Telegram group senders (numeric Telegram user IDs). */
groupAllowFrom?: Array<string | number>;
/**
* 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<string | number>;
/** Optional system prompt snippet for this topic. */
systemPrompt?: string;
@@ -169,7 +170,7 @@ export type TelegramGroupConfig = {
topics?: Record<string, TelegramTopicConfig>;
/** 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<string | number>;
/** Optional system prompt snippet for this group. */
systemPrompt?: string;