fix(security): tighten telegram dm audit coverage

This commit is contained in:
Peter Steinberger
2026-04-29 02:04:14 +01:00
parent a968f4f437
commit 381c2e1d1a
11 changed files with 269 additions and 32 deletions

View File

@@ -1,4 +1,4 @@
import { describe, expect, it, vi } from "vitest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { OpenClawConfig } from "../runtime-api.js";
import type { ResolvedTelegramAccount } from "./accounts.js";
import { collectTelegramSecurityAuditFindings } from "./security-audit.js";
@@ -32,6 +32,11 @@ function getTelegramConfig(cfg: OpenClawConfig) {
}
describe("Telegram security audit findings", () => {
beforeEach(() => {
readChannelAllowFromStoreMock.mockReset();
readChannelAllowFromStoreMock.mockResolvedValue([]);
});
it("flags group commands without a sender allowlist", async () => {
const cfg: OpenClawConfig = {
channels: {
@@ -44,7 +49,6 @@ describe("Telegram security audit findings", () => {
},
};
readChannelAllowFromStoreMock.mockResolvedValue([]);
const findings = await collectTelegramSecurityAuditFindings({
cfg,
account: createTelegramAccount(getTelegramConfig(cfg)),
@@ -74,7 +78,6 @@ describe("Telegram security audit findings", () => {
},
};
readChannelAllowFromStoreMock.mockResolvedValue([]);
const findings = await collectTelegramSecurityAuditFindings({
cfg,
account: createTelegramAccount(getTelegramConfig(cfg)),
@@ -90,4 +93,61 @@ describe("Telegram security audit findings", () => {
]),
);
});
it("warns about invalid DM allowFrom entries even when groups are not enabled", async () => {
const cfg: OpenClawConfig = {
channels: {
telegram: {
enabled: true,
botToken: "t",
dmPolicy: "allowlist",
allowFrom: ["@TrustedOperator"],
groupPolicy: "allowlist",
},
},
};
const findings = await collectTelegramSecurityAuditFindings({
cfg,
account: createTelegramAccount(getTelegramConfig(cfg)),
accountId: "default",
});
expect(findings).toEqual([
expect.objectContaining({
checkId: "channels.telegram.allowFrom.invalid_entries",
severity: "warn",
}),
]);
expect(readChannelAllowFromStoreMock).not.toHaveBeenCalled();
});
it("warns about invalid DM allowFrom entries when text commands are disabled", async () => {
const cfg: OpenClawConfig = {
commands: { text: false },
channels: {
telegram: {
enabled: true,
botToken: "t",
dmPolicy: "allowlist",
allowFrom: ["@TrustedOperator"],
groupPolicy: "allowlist",
},
},
};
const findings = await collectTelegramSecurityAuditFindings({
cfg,
account: createTelegramAccount(getTelegramConfig(cfg)),
accountId: "default",
});
expect(findings).toEqual([
expect.objectContaining({
checkId: "channels.telegram.allowFrom.invalid_entries",
severity: "warn",
}),
]);
expect(readChannelAllowFromStoreMock).not.toHaveBeenCalled();
});
});

View File

@@ -24,6 +24,36 @@ function collectInvalidTelegramAllowFromEntries(params: { entries: unknown; targ
}
}
function appendInvalidTelegramAllowFromFinding(
findings: Array<{
checkId: string;
severity: "info" | "warn" | "critical";
title: string;
detail: string;
remediation?: string;
}>,
invalidTelegramAllowFromEntries: Set<string>,
) {
if (invalidTelegramAllowFromEntries.size === 0) {
return;
}
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 setup to resolve), then re-run the audit.",
});
}
export async function collectTelegramSecurityAuditFindings(params: {
cfg: OpenClawConfig;
accountId?: string | null;
@@ -36,13 +66,20 @@ export async function collectTelegramSecurityAuditFindings(params: {
detail: string;
remediation?: string;
}> = [];
if (params.cfg.commands?.text === false) {
return findings;
}
const telegramCfg = params.account.config ?? {};
const accountId =
normalizeOptionalString(params.accountId) ?? params.account.accountId ?? "default";
const invalidTelegramAllowFromEntries = new Set<string>();
collectInvalidTelegramAllowFromEntries({
entries: Array.isArray(telegramCfg.allowFrom) ? telegramCfg.allowFrom : [],
target: invalidTelegramAllowFromEntries,
});
if (params.cfg.commands?.text === false) {
appendInvalidTelegramAllowFromFinding(findings, invalidTelegramAllowFromEntries);
return findings;
}
const defaultGroupPolicy = params.cfg.channels?.defaults?.groupPolicy;
const groupPolicy =
(telegramCfg.groupPolicy as string | undefined) ?? defaultGroupPolicy ?? "allowlist";
@@ -51,6 +88,7 @@ export async function collectTelegramSecurityAuditFindings(params: {
const groupAccessPossible =
groupPolicy === "open" || (groupPolicy === "allowlist" && groupsConfigured);
if (!groupAccessPossible) {
appendInvalidTelegramAllowFromFinding(findings, invalidTelegramAllowFromEntries);
return findings;
}
@@ -60,7 +98,6 @@ export async function collectTelegramSecurityAuditFindings(params: {
const storeHasWildcard = storeAllowFrom.some(
(value) => (normalizeOptionalString(value) ?? "") === "*",
);
const invalidTelegramAllowFromEntries = new Set<string>();
collectInvalidTelegramAllowFromEntries({
entries: storeAllowFrom,
target: invalidTelegramAllowFromEntries,
@@ -75,10 +112,6 @@ export async function collectTelegramSecurityAuditFindings(params: {
entries: groupAllowFrom,
target: invalidTelegramAllowFromEntries,
});
collectInvalidTelegramAllowFromEntries({
entries: Array.isArray(telegramCfg.allowFrom) ? telegramCfg.allowFrom : [],
target: invalidTelegramAllowFromEntries,
});
let anyGroupOverride = false;
if (groups) {
@@ -119,23 +152,7 @@ export async function collectTelegramSecurityAuditFindings(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 setup to resolve), then re-run the audit.",
});
}
appendInvalidTelegramAllowFromFinding(findings, invalidTelegramAllowFromEntries);
if (storeHasWildcard || groupAllowFromHasWildcard) {
findings.push({