mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-30 08:29:49 +00:00
fix(telegram): require sender ids for allowlist auth
This commit is contained in:
@@ -24,6 +24,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- 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.
|
||||
- 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.
|
||||
|
||||
@@ -138,7 +138,7 @@ Control how group/room messages are handled per channel:
|
||||
},
|
||||
telegram: {
|
||||
groupPolicy: "disabled",
|
||||
groupAllowFrom: ["123456789", "@username"],
|
||||
groupAllowFrom: ["123456789"], // numeric Telegram user id (wizard can resolve @username)
|
||||
},
|
||||
signal: {
|
||||
groupPolicy: "disabled",
|
||||
|
||||
@@ -112,7 +112,8 @@ Token resolution order is account-aware. In practice, config values win over env
|
||||
- `open` (requires `allowFrom` to include `"*"`)
|
||||
- `disabled`
|
||||
|
||||
`channels.telegram.allowFrom` accepts numeric IDs and usernames. `telegram:` / `tg:` prefixes are accepted and normalized.
|
||||
`channels.telegram.allowFrom` accepts numeric Telegram user IDs. `telegram:` / `tg:` prefixes are accepted and normalized.
|
||||
The onboarding wizard accepts `@username` input and resolves it to numeric IDs.
|
||||
|
||||
### Finding your Telegram user ID
|
||||
|
||||
@@ -681,9 +682,9 @@ Primary reference:
|
||||
- `channels.telegram.botToken`: bot token (BotFather).
|
||||
- `channels.telegram.tokenFile`: read token from file path.
|
||||
- `channels.telegram.dmPolicy`: `pairing | allowlist | open | disabled` (default: pairing).
|
||||
- `channels.telegram.allowFrom`: DM allowlist (ids/usernames). `open` requires `"*"`.
|
||||
- `channels.telegram.allowFrom`: DM allowlist (numeric Telegram user IDs). `open` requires `"*"`.
|
||||
- `channels.telegram.groupPolicy`: `open | allowlist | disabled` (default: allowlist).
|
||||
- `channels.telegram.groupAllowFrom`: group sender allowlist (ids/usernames).
|
||||
- `channels.telegram.groupAllowFrom`: group sender allowlist (numeric Telegram user IDs).
|
||||
- `channels.telegram.groups`: per-group defaults + allowlist (use `"*"` for global defaults).
|
||||
- `channels.telegram.groups.<id>.groupPolicy`: per-group override for groupPolicy (`open | allowlist | disabled`).
|
||||
- `channels.telegram.groups.<id>.requireMention`: mention gating default.
|
||||
|
||||
@@ -794,7 +794,9 @@ without WhatsApp/Telegram.
|
||||
|
||||
### Telegram what goes in allowFrom
|
||||
|
||||
`channels.telegram.allowFrom` is **the human sender's Telegram user ID** (numeric, recommended) or `@username`. It is not the bot username.
|
||||
`channels.telegram.allowFrom` is **the human sender's Telegram user ID** (numeric). It is not the bot username.
|
||||
|
||||
The onboarding wizard accepts `@username` input and resolves it to a numeric ID, but OpenClaw authorization uses numeric IDs only.
|
||||
|
||||
Safer (no third-party bot):
|
||||
|
||||
|
||||
@@ -115,7 +115,7 @@ async function promptTelegramAllowFrom(params: {
|
||||
let resolvedIds: string[] = [];
|
||||
while (resolvedIds.length === 0) {
|
||||
const entry = await prompter.text({
|
||||
message: "Telegram allowFrom (username or user id)",
|
||||
message: "Telegram allowFrom (numeric sender id; @username resolves to id)",
|
||||
placeholder: "@username",
|
||||
initialValue: existingAllowFrom[0] ? String(existingAllowFrom[0]) : undefined,
|
||||
validate: (value) => (String(value ?? "").trim() ? undefined : "Required"),
|
||||
|
||||
@@ -14,6 +14,18 @@ function normalizeAllowFromList(list: Array<string | number> | undefined | null)
|
||||
return list.map((v) => String(v).trim()).filter(Boolean);
|
||||
}
|
||||
|
||||
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);
|
||||
}
|
||||
|
||||
function classifyChannelWarningSeverity(message: string): SecurityAuditSeverity {
|
||||
const s = message.toLowerCase();
|
||||
if (
|
||||
@@ -353,10 +365,39 @@ export async function collectChannelSecurityFindings(params: {
|
||||
|
||||
const storeAllowFrom = await readChannelAllowFromStore("telegram").catch(() => []);
|
||||
const storeHasWildcard = storeAllowFrom.some((v) => String(v).trim() === "*");
|
||||
const invalidTelegramAllowFromEntries = new Set<string>();
|
||||
for (const entry of storeAllowFrom) {
|
||||
const normalized = normalizeTelegramAllowFromEntry(entry);
|
||||
if (!normalized || normalized === "*") {
|
||||
continue;
|
||||
}
|
||||
if (!isNumericTelegramUserId(normalized)) {
|
||||
invalidTelegramAllowFromEntries.add(normalized);
|
||||
}
|
||||
}
|
||||
const groupAllowFrom = Array.isArray(telegramCfg.groupAllowFrom)
|
||||
? telegramCfg.groupAllowFrom
|
||||
: [];
|
||||
const groupAllowFromHasWildcard = groupAllowFrom.some((v) => String(v).trim() === "*");
|
||||
for (const entry of groupAllowFrom) {
|
||||
const normalized = normalizeTelegramAllowFromEntry(entry);
|
||||
if (!normalized || normalized === "*") {
|
||||
continue;
|
||||
}
|
||||
if (!isNumericTelegramUserId(normalized)) {
|
||||
invalidTelegramAllowFromEntries.add(normalized);
|
||||
}
|
||||
}
|
||||
const dmAllowFrom = Array.isArray(telegramCfg.allowFrom) ? telegramCfg.allowFrom : [];
|
||||
for (const entry of dmAllowFrom) {
|
||||
const normalized = normalizeTelegramAllowFromEntry(entry);
|
||||
if (!normalized || normalized === "*") {
|
||||
continue;
|
||||
}
|
||||
if (!isNumericTelegramUserId(normalized)) {
|
||||
invalidTelegramAllowFromEntries.add(normalized);
|
||||
}
|
||||
}
|
||||
const anyGroupOverride = Boolean(
|
||||
groups &&
|
||||
Object.values(groups).some((value) => {
|
||||
@@ -366,6 +407,15 @@ export async function collectChannelSecurityFindings(params: {
|
||||
const group = value as Record<string, unknown>;
|
||||
const allowFrom = Array.isArray(group.allowFrom) ? group.allowFrom : [];
|
||||
if (allowFrom.length > 0) {
|
||||
for (const entry of allowFrom) {
|
||||
const normalized = normalizeTelegramAllowFromEntry(entry);
|
||||
if (!normalized || normalized === "*") {
|
||||
continue;
|
||||
}
|
||||
if (!isNumericTelegramUserId(normalized)) {
|
||||
invalidTelegramAllowFromEntries.add(normalized);
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
const topics = group.topics;
|
||||
@@ -378,6 +428,15 @@ export async function collectChannelSecurityFindings(params: {
|
||||
}
|
||||
const topic = topicValue as Record<string, unknown>;
|
||||
const topicAllow = Array.isArray(topic.allowFrom) ? topic.allowFrom : [];
|
||||
for (const entry of topicAllow) {
|
||||
const normalized = normalizeTelegramAllowFromEntry(entry);
|
||||
if (!normalized || normalized === "*") {
|
||||
continue;
|
||||
}
|
||||
if (!isNumericTelegramUserId(normalized)) {
|
||||
invalidTelegramAllowFromEntries.add(normalized);
|
||||
}
|
||||
}
|
||||
return topicAllow.length > 0;
|
||||
});
|
||||
}),
|
||||
@@ -386,6 +445,24 @@ export async function collectChannelSecurityFindings(params: {
|
||||
const hasAnySenderAllowlist =
|
||||
storeAllowFrom.length > 0 || groupAllowFrom.length > 0 || anyGroupOverride;
|
||||
|
||||
if (invalidTelegramAllowFromEntries.size > 0) {
|
||||
const examples = Array.from(invalidTelegramAllowFromEntries).slice(0, 5);
|
||||
const more =
|
||||
invalidTelegramAllowFromEntries.size > examples.length
|
||||
? ` (+${invalidTelegramAllowFromEntries.size - examples.length} more)`
|
||||
: "";
|
||||
findings.push({
|
||||
checkId: "channels.telegram.allowFrom.invalid_entries",
|
||||
severity: "warn",
|
||||
title: "Telegram allowlist contains non-numeric entries",
|
||||
detail:
|
||||
"Telegram sender authorization requires numeric Telegram user IDs. " +
|
||||
`Found non-numeric allowFrom entries: ${examples.join(", ")}${more}.`,
|
||||
remediation:
|
||||
"Replace @username entries with numeric Telegram user IDs (use onboarding to resolve), then re-run the audit.",
|
||||
});
|
||||
}
|
||||
|
||||
if (storeHasWildcard || groupAllowFromHasWildcard) {
|
||||
findings.push({
|
||||
checkId: "channels.telegram.groups.allowFrom.wildcard",
|
||||
@@ -394,7 +471,7 @@ export async function collectChannelSecurityFindings(params: {
|
||||
detail:
|
||||
'Telegram group sender allowlist contains "*", which allows any group member to run /… commands and control directives.',
|
||||
remediation:
|
||||
'Remove "*" from channels.telegram.groupAllowFrom and pairing store; prefer explicit user ids/usernames.',
|
||||
'Remove "*" from channels.telegram.groupAllowFrom and pairing store; prefer explicit numeric Telegram user IDs.',
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -1099,6 +1099,50 @@ describe("security audit", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("warns when Telegram allowFrom entries are non-numeric (legacy @username configs)", async () => {
|
||||
const prevStateDir = process.env.OPENCLAW_STATE_DIR;
|
||||
const tmp = await fs.mkdtemp(
|
||||
path.join(os.tmpdir(), "openclaw-security-audit-telegram-invalid-allowfrom-"),
|
||||
);
|
||||
process.env.OPENCLAW_STATE_DIR = tmp;
|
||||
await fs.mkdir(path.join(tmp, "credentials"), { recursive: true, mode: 0o700 });
|
||||
try {
|
||||
const cfg: OpenClawConfig = {
|
||||
channels: {
|
||||
telegram: {
|
||||
enabled: true,
|
||||
botToken: "t",
|
||||
groupPolicy: "allowlist",
|
||||
groupAllowFrom: ["@TrustedOperator"],
|
||||
groups: { "-100123": {} },
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const res = await runSecurityAudit({
|
||||
config: cfg,
|
||||
includeFilesystem: false,
|
||||
includeChannelSecurity: true,
|
||||
plugins: [telegramPlugin],
|
||||
});
|
||||
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
checkId: "channels.telegram.allowFrom.invalid_entries",
|
||||
severity: "warn",
|
||||
}),
|
||||
]),
|
||||
);
|
||||
} finally {
|
||||
if (prevStateDir == null) {
|
||||
delete process.env.OPENCLAW_STATE_DIR;
|
||||
} else {
|
||||
process.env.OPENCLAW_STATE_DIR = prevStateDir;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("adds a warning when deep probe fails", async () => {
|
||||
const cfg: OpenClawConfig = { gateway: { mode: "local" } };
|
||||
|
||||
|
||||
@@ -2,12 +2,34 @@ import type { AllowlistMatch } from "../channels/allowlist-match.js";
|
||||
|
||||
export type NormalizedAllowFrom = {
|
||||
entries: string[];
|
||||
entriesLower: string[];
|
||||
hasWildcard: boolean;
|
||||
hasEntries: boolean;
|
||||
invalidEntries: string[];
|
||||
};
|
||||
|
||||
export type AllowFromMatch = AllowlistMatch<"wildcard" | "id" | "username">;
|
||||
export type AllowFromMatch = AllowlistMatch<"wildcard" | "id">;
|
||||
|
||||
const warnedInvalidEntries = new Set<string>();
|
||||
|
||||
function warnInvalidAllowFromEntries(entries: string[]) {
|
||||
if (process.env.VITEST || process.env.NODE_ENV === "test") {
|
||||
return;
|
||||
}
|
||||
for (const entry of entries) {
|
||||
if (warnedInvalidEntries.has(entry)) {
|
||||
continue;
|
||||
}
|
||||
warnedInvalidEntries.add(entry);
|
||||
console.warn(
|
||||
[
|
||||
"[telegram] Invalid allowFrom entry:",
|
||||
JSON.stringify(entry),
|
||||
"- allowFrom/groupAllowFrom authorization requires numeric Telegram sender IDs only.",
|
||||
'If you had "@username" entries, re-run onboarding (it resolves @username to IDs) or replace them manually.',
|
||||
].join(" "),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
export const normalizeAllowFrom = (list?: Array<string | number>): NormalizedAllowFrom => {
|
||||
const entries = (list ?? []).map((value) => String(value).trim()).filter(Boolean);
|
||||
@@ -15,12 +37,16 @@ export const normalizeAllowFrom = (list?: Array<string | number>): NormalizedAll
|
||||
const normalized = entries
|
||||
.filter((value) => value !== "*")
|
||||
.map((value) => value.replace(/^(telegram|tg):/i, ""));
|
||||
const normalizedLower = normalized.map((value) => value.toLowerCase());
|
||||
const invalidEntries = normalized.filter((value) => !/^\d+$/.test(value));
|
||||
if (invalidEntries.length > 0) {
|
||||
warnInvalidAllowFromEntries([...new Set(invalidEntries)]);
|
||||
}
|
||||
const ids = normalized.filter((value) => /^\d+$/.test(value));
|
||||
return {
|
||||
entries: normalized,
|
||||
entriesLower: normalizedLower,
|
||||
entries: ids,
|
||||
hasWildcard,
|
||||
hasEntries: entries.length > 0,
|
||||
invalidEntries,
|
||||
};
|
||||
};
|
||||
|
||||
@@ -48,7 +74,7 @@ export const isSenderAllowed = (params: {
|
||||
senderId?: string;
|
||||
senderUsername?: string;
|
||||
}) => {
|
||||
const { allow, senderId, senderUsername } = params;
|
||||
const { allow, senderId } = params;
|
||||
if (!allow.hasEntries) {
|
||||
return true;
|
||||
}
|
||||
@@ -58,11 +84,7 @@ export const isSenderAllowed = (params: {
|
||||
if (senderId && allow.entries.includes(senderId)) {
|
||||
return true;
|
||||
}
|
||||
const username = senderUsername?.toLowerCase();
|
||||
if (!username) {
|
||||
return false;
|
||||
}
|
||||
return allow.entriesLower.some((entry) => entry === username || entry === `@${username}`);
|
||||
return false;
|
||||
};
|
||||
|
||||
export const resolveSenderAllowMatch = (params: {
|
||||
@@ -70,7 +92,7 @@ export const resolveSenderAllowMatch = (params: {
|
||||
senderId?: string;
|
||||
senderUsername?: string;
|
||||
}): AllowFromMatch => {
|
||||
const { allow, senderId, senderUsername } = params;
|
||||
const { allow, senderId } = params;
|
||||
if (allow.hasWildcard) {
|
||||
return { allowed: true, matchKey: "*", matchSource: "wildcard" };
|
||||
}
|
||||
@@ -80,15 +102,5 @@ export const resolveSenderAllowMatch = (params: {
|
||||
if (senderId && allow.entries.includes(senderId)) {
|
||||
return { allowed: true, matchKey: senderId, matchSource: "id" };
|
||||
}
|
||||
const username = senderUsername?.toLowerCase();
|
||||
if (!username) {
|
||||
return { allowed: false };
|
||||
}
|
||||
const entry = allow.entriesLower.find(
|
||||
(candidate) => candidate === username || candidate === `@${username}`,
|
||||
);
|
||||
if (entry) {
|
||||
return { allowed: true, matchKey: entry, matchSource: "username" };
|
||||
}
|
||||
return { allowed: false };
|
||||
};
|
||||
|
||||
@@ -248,7 +248,7 @@ describe("createTelegramBot", () => {
|
||||
|
||||
expect(replySpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
it("allows group messages from senders in allowFrom (by username) when groupPolicy is 'allowlist'", async () => {
|
||||
it("blocks group messages when allowFrom is configured with @username entries (numeric IDs required)", async () => {
|
||||
onSpy.mockReset();
|
||||
const replySpy = replyModule.__replySpy as unknown as ReturnType<typeof vi.fn>;
|
||||
replySpy.mockReset();
|
||||
@@ -276,7 +276,7 @@ describe("createTelegramBot", () => {
|
||||
getFile: async () => ({ download: async () => new Uint8Array() }),
|
||||
});
|
||||
|
||||
expect(replySpy).toHaveBeenCalledTimes(1);
|
||||
expect(replySpy).toHaveBeenCalledTimes(0);
|
||||
});
|
||||
it("allows group messages from telegram:-prefixed allowFrom entries when groupPolicy is 'allowlist'", async () => {
|
||||
onSpy.mockReset();
|
||||
|
||||
@@ -159,7 +159,7 @@ describe("createTelegramBot", () => {
|
||||
|
||||
// groupPolicy tests
|
||||
|
||||
it("matches usernames case-insensitively when groupPolicy is 'allowlist'", async () => {
|
||||
it("blocks @username allowFrom entries when groupPolicy is 'allowlist' (numeric IDs required)", async () => {
|
||||
onSpy.mockReset();
|
||||
const replySpy = replyModule.__replySpy as unknown as ReturnType<typeof vi.fn>;
|
||||
replySpy.mockReset();
|
||||
@@ -187,7 +187,7 @@ describe("createTelegramBot", () => {
|
||||
getFile: async () => ({ download: async () => new Uint8Array() }),
|
||||
});
|
||||
|
||||
expect(replySpy).toHaveBeenCalledTimes(1);
|
||||
expect(replySpy).toHaveBeenCalledTimes(0);
|
||||
});
|
||||
it("allows direct messages regardless of groupPolicy", async () => {
|
||||
onSpy.mockReset();
|
||||
|
||||
Reference in New Issue
Block a user