fix(security): gate slash commands by sender
This commit is contained in:
@@ -5,6 +5,7 @@ import JSON5 from "json5";
|
||||
|
||||
import type { ClawdbotConfig, ConfigFileSnapshot } from "../config/config.js";
|
||||
import { createConfigIO } from "../config/config.js";
|
||||
import { resolveNativeSkillsEnabled } from "../config/commands.js";
|
||||
import { resolveOAuthDir } from "../config/paths.js";
|
||||
import { resolveDefaultAgentId } from "../agents/agent-scope.js";
|
||||
import { INCLUDE_KEY, MAX_INCLUDE_DEPTH } from "../config/includes.js";
|
||||
@@ -380,11 +381,70 @@ export async function collectPluginsTrustFindings(params: {
|
||||
const allow = params.cfg.plugins?.allow;
|
||||
const allowConfigured = Array.isArray(allow) && allow.length > 0;
|
||||
if (!allowConfigured) {
|
||||
const hasString = (value: unknown) => typeof value === "string" && value.trim().length > 0;
|
||||
const hasAccountStringKey = (account: unknown, key: string) =>
|
||||
Boolean(account && typeof account === "object" && hasString((account as Record<string, unknown>)[key]));
|
||||
|
||||
const discordConfigured =
|
||||
hasString(params.cfg.channels?.discord?.token) ||
|
||||
Boolean(
|
||||
params.cfg.channels?.discord?.accounts &&
|
||||
Object.values(params.cfg.channels.discord.accounts).some((a) => hasAccountStringKey(a, "token")),
|
||||
) ||
|
||||
hasString(process.env.DISCORD_BOT_TOKEN);
|
||||
|
||||
const telegramConfigured =
|
||||
hasString(params.cfg.channels?.telegram?.botToken) ||
|
||||
hasString(params.cfg.channels?.telegram?.tokenFile) ||
|
||||
Boolean(
|
||||
params.cfg.channels?.telegram?.accounts &&
|
||||
Object.values(params.cfg.channels.telegram.accounts).some(
|
||||
(a) => hasAccountStringKey(a, "botToken") || hasAccountStringKey(a, "tokenFile"),
|
||||
),
|
||||
) ||
|
||||
hasString(process.env.TELEGRAM_BOT_TOKEN);
|
||||
|
||||
const slackConfigured =
|
||||
hasString(params.cfg.channels?.slack?.botToken) ||
|
||||
hasString(params.cfg.channels?.slack?.appToken) ||
|
||||
Boolean(
|
||||
params.cfg.channels?.slack?.accounts &&
|
||||
Object.values(params.cfg.channels.slack.accounts).some(
|
||||
(a) => hasAccountStringKey(a, "botToken") || hasAccountStringKey(a, "appToken"),
|
||||
),
|
||||
) ||
|
||||
hasString(process.env.SLACK_BOT_TOKEN) ||
|
||||
hasString(process.env.SLACK_APP_TOKEN);
|
||||
|
||||
const skillCommandsLikelyExposed =
|
||||
(discordConfigured &&
|
||||
resolveNativeSkillsEnabled({
|
||||
providerId: "discord",
|
||||
providerSetting: params.cfg.channels?.discord?.commands?.nativeSkills,
|
||||
globalSetting: params.cfg.commands?.nativeSkills,
|
||||
})) ||
|
||||
(telegramConfigured &&
|
||||
resolveNativeSkillsEnabled({
|
||||
providerId: "telegram",
|
||||
providerSetting: params.cfg.channels?.telegram?.commands?.nativeSkills,
|
||||
globalSetting: params.cfg.commands?.nativeSkills,
|
||||
})) ||
|
||||
(slackConfigured &&
|
||||
resolveNativeSkillsEnabled({
|
||||
providerId: "slack",
|
||||
providerSetting: params.cfg.channels?.slack?.commands?.nativeSkills,
|
||||
globalSetting: params.cfg.commands?.nativeSkills,
|
||||
}));
|
||||
|
||||
findings.push({
|
||||
checkId: "plugins.extensions_no_allowlist",
|
||||
severity: "warn",
|
||||
severity: skillCommandsLikelyExposed ? "critical" : "warn",
|
||||
title: "Extensions exist but plugins.allow is not set",
|
||||
detail: `Found ${pluginDirs.length} extension(s) under ${extensionsDir}. Without plugins.allow, any discovered plugin id may load (depending on config and plugin behavior).`,
|
||||
detail:
|
||||
`Found ${pluginDirs.length} extension(s) under ${extensionsDir}. Without plugins.allow, any discovered plugin id may load (depending on config and plugin behavior).` +
|
||||
(skillCommandsLikelyExposed
|
||||
? "\nNative skill commands are enabled on at least one configured chat surface; treat unpinned/unallowlisted extensions as high risk."
|
||||
: ""),
|
||||
remediation: "Set plugins.allow to an explicit list of plugin ids you trust.",
|
||||
});
|
||||
}
|
||||
|
||||
@@ -3,6 +3,9 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import type { ClawdbotConfig } from "../config/config.js";
|
||||
import type { ChannelPlugin } from "../channels/plugins/types.js";
|
||||
import { runSecurityAudit } from "./audit.js";
|
||||
import { discordPlugin } from "../channels/plugins/discord.js";
|
||||
import { slackPlugin } from "../channels/plugins/slack.js";
|
||||
import { telegramPlugin } from "../channels/plugins/telegram.js";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
@@ -222,6 +225,97 @@ describe("security audit", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("flags Discord native commands without a guild user allowlist", async () => {
|
||||
const cfg: ClawdbotConfig = {
|
||||
channels: {
|
||||
discord: {
|
||||
enabled: true,
|
||||
token: "t",
|
||||
groupPolicy: "allowlist",
|
||||
guilds: {
|
||||
"123": {
|
||||
channels: {
|
||||
general: { allow: true },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const res = await runSecurityAudit({
|
||||
config: cfg,
|
||||
includeFilesystem: false,
|
||||
includeChannelSecurity: true,
|
||||
plugins: [discordPlugin],
|
||||
});
|
||||
|
||||
const finding = res.findings.find((f) => f.detail.includes("Discord slash commands"));
|
||||
expect(finding?.severity).toBe("critical");
|
||||
});
|
||||
|
||||
it("flags Slack slash commands without a channel users allowlist", async () => {
|
||||
const cfg: ClawdbotConfig = {
|
||||
channels: {
|
||||
slack: {
|
||||
enabled: true,
|
||||
botToken: "xoxb-test",
|
||||
appToken: "xapp-test",
|
||||
groupPolicy: "open",
|
||||
slashCommand: { enabled: true },
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const res = await runSecurityAudit({
|
||||
config: cfg,
|
||||
includeFilesystem: false,
|
||||
includeChannelSecurity: true,
|
||||
plugins: [slackPlugin],
|
||||
});
|
||||
|
||||
const finding = res.findings.find((f) => f.detail.includes("Slack slash commands"));
|
||||
expect(finding?.severity).toBe("critical");
|
||||
});
|
||||
|
||||
it("flags Telegram group commands without a sender allowlist", async () => {
|
||||
const prevStateDir = process.env.CLAWDBOT_STATE_DIR;
|
||||
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-telegram-"));
|
||||
process.env.CLAWDBOT_STATE_DIR = tmp;
|
||||
await fs.mkdir(path.join(tmp, "credentials"), { recursive: true, mode: 0o700 });
|
||||
try {
|
||||
const cfg: ClawdbotConfig = {
|
||||
channels: {
|
||||
telegram: {
|
||||
enabled: true,
|
||||
botToken: "t",
|
||||
groupPolicy: "allowlist",
|
||||
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.groups.allowFrom.missing",
|
||||
severity: "critical",
|
||||
}),
|
||||
]),
|
||||
);
|
||||
} finally {
|
||||
if (prevStateDir == null) delete process.env.CLAWDBOT_STATE_DIR;
|
||||
else process.env.CLAWDBOT_STATE_DIR = prevStateDir;
|
||||
}
|
||||
});
|
||||
|
||||
it("adds a warning when deep probe fails", async () => {
|
||||
const cfg: ClawdbotConfig = { gateway: { mode: "local" } };
|
||||
|
||||
@@ -380,6 +474,14 @@ describe("security audit", () => {
|
||||
});
|
||||
|
||||
it("flags extensions without plugins.allow", async () => {
|
||||
const prevDiscordToken = process.env.DISCORD_BOT_TOKEN;
|
||||
const prevTelegramToken = process.env.TELEGRAM_BOT_TOKEN;
|
||||
const prevSlackBotToken = process.env.SLACK_BOT_TOKEN;
|
||||
const prevSlackAppToken = process.env.SLACK_APP_TOKEN;
|
||||
delete process.env.DISCORD_BOT_TOKEN;
|
||||
delete process.env.TELEGRAM_BOT_TOKEN;
|
||||
delete process.env.SLACK_BOT_TOKEN;
|
||||
delete process.env.SLACK_APP_TOKEN;
|
||||
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-"));
|
||||
const stateDir = path.join(tmp, "state");
|
||||
await fs.mkdir(path.join(stateDir, "extensions", "some-plugin"), {
|
||||
@@ -387,20 +489,69 @@ describe("security audit", () => {
|
||||
mode: 0o700,
|
||||
});
|
||||
|
||||
const cfg: ClawdbotConfig = {};
|
||||
const res = await runSecurityAudit({
|
||||
config: cfg,
|
||||
includeFilesystem: true,
|
||||
includeChannelSecurity: false,
|
||||
stateDir,
|
||||
configPath: path.join(stateDir, "clawdbot.json"),
|
||||
try {
|
||||
const cfg: ClawdbotConfig = {};
|
||||
const res = await runSecurityAudit({
|
||||
config: cfg,
|
||||
includeFilesystem: true,
|
||||
includeChannelSecurity: false,
|
||||
stateDir,
|
||||
configPath: path.join(stateDir, "clawdbot.json"),
|
||||
});
|
||||
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ checkId: "plugins.extensions_no_allowlist", severity: "warn" }),
|
||||
]),
|
||||
);
|
||||
} finally {
|
||||
if (prevDiscordToken == null) delete process.env.DISCORD_BOT_TOKEN;
|
||||
else process.env.DISCORD_BOT_TOKEN = prevDiscordToken;
|
||||
if (prevTelegramToken == null) delete process.env.TELEGRAM_BOT_TOKEN;
|
||||
else process.env.TELEGRAM_BOT_TOKEN = prevTelegramToken;
|
||||
if (prevSlackBotToken == null) delete process.env.SLACK_BOT_TOKEN;
|
||||
else process.env.SLACK_BOT_TOKEN = prevSlackBotToken;
|
||||
if (prevSlackAppToken == null) delete process.env.SLACK_APP_TOKEN;
|
||||
else process.env.SLACK_APP_TOKEN = prevSlackAppToken;
|
||||
}
|
||||
});
|
||||
|
||||
it("flags unallowlisted extensions as critical when native skill commands are exposed", async () => {
|
||||
const prevDiscordToken = process.env.DISCORD_BOT_TOKEN;
|
||||
delete process.env.DISCORD_BOT_TOKEN;
|
||||
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-"));
|
||||
const stateDir = path.join(tmp, "state");
|
||||
await fs.mkdir(path.join(stateDir, "extensions", "some-plugin"), {
|
||||
recursive: true,
|
||||
mode: 0o700,
|
||||
});
|
||||
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ checkId: "plugins.extensions_no_allowlist", severity: "warn" }),
|
||||
]),
|
||||
);
|
||||
try {
|
||||
const cfg: ClawdbotConfig = {
|
||||
channels: {
|
||||
discord: { enabled: true, token: "t" },
|
||||
},
|
||||
};
|
||||
const res = await runSecurityAudit({
|
||||
config: cfg,
|
||||
includeFilesystem: true,
|
||||
includeChannelSecurity: false,
|
||||
stateDir,
|
||||
configPath: path.join(stateDir, "clawdbot.json"),
|
||||
});
|
||||
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
checkId: "plugins.extensions_no_allowlist",
|
||||
severity: "critical",
|
||||
}),
|
||||
]),
|
||||
);
|
||||
} finally {
|
||||
if (prevDiscordToken == null) delete process.env.DISCORD_BOT_TOKEN;
|
||||
else process.env.DISCORD_BOT_TOKEN = prevDiscordToken;
|
||||
}
|
||||
});
|
||||
|
||||
it("flags open groupPolicy when tools.elevated is enabled", async () => {
|
||||
|
||||
@@ -20,6 +20,7 @@ import {
|
||||
readConfigSnapshotForAudit,
|
||||
} from "./audit-extra.js";
|
||||
import { readChannelAllowFromStore } from "../pairing/pairing-store.js";
|
||||
import { resolveNativeSkillsEnabled } from "../config/commands.js";
|
||||
import {
|
||||
formatOctal,
|
||||
isGroupReadable,
|
||||
@@ -498,6 +499,81 @@ async function collectChannelSecurityFindings(params: {
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
if (plugin.id === "telegram") {
|
||||
const allowTextCommands = params.cfg.commands?.text !== false;
|
||||
if (!allowTextCommands) continue;
|
||||
|
||||
const telegramCfg =
|
||||
(account as { config?: Record<string, unknown> } | null)?.config ?? ({} as Record<
|
||||
string,
|
||||
unknown
|
||||
>);
|
||||
const groupPolicy = (telegramCfg.groupPolicy as string | undefined) ?? "allowlist";
|
||||
const groups = telegramCfg.groups as Record<string, unknown> | undefined;
|
||||
const groupsConfigured = Boolean(groups) && Object.keys(groups ?? {}).length > 0;
|
||||
const groupAccessPossible =
|
||||
groupPolicy === "open" || (groupPolicy === "allowlist" && groupsConfigured);
|
||||
if (!groupAccessPossible) continue;
|
||||
|
||||
const storeAllowFrom = await readChannelAllowFromStore("telegram").catch(() => []);
|
||||
const storeHasWildcard = storeAllowFrom.some((v) => String(v).trim() === "*");
|
||||
const groupAllowFrom = Array.isArray(telegramCfg.groupAllowFrom) ? telegramCfg.groupAllowFrom : [];
|
||||
const groupAllowFromHasWildcard = groupAllowFrom.some((v) => String(v).trim() === "*");
|
||||
const anyGroupOverride = Boolean(
|
||||
groups &&
|
||||
Object.values(groups).some((value) => {
|
||||
if (!value || typeof value !== "object") return false;
|
||||
const group = value as Record<string, unknown>;
|
||||
const allowFrom = Array.isArray(group.allowFrom) ? group.allowFrom : [];
|
||||
if (allowFrom.length > 0) return true;
|
||||
const topics = group.topics;
|
||||
if (!topics || typeof topics !== "object") return false;
|
||||
return Object.values(topics as Record<string, unknown>).some((topicValue) => {
|
||||
if (!topicValue || typeof topicValue !== "object") return false;
|
||||
const topic = topicValue as Record<string, unknown>;
|
||||
const topicAllow = Array.isArray(topic.allowFrom) ? topic.allowFrom : [];
|
||||
return topicAllow.length > 0;
|
||||
});
|
||||
}),
|
||||
);
|
||||
|
||||
const hasAnySenderAllowlist =
|
||||
storeAllowFrom.length > 0 || groupAllowFrom.length > 0 || anyGroupOverride;
|
||||
|
||||
if (storeHasWildcard || groupAllowFromHasWildcard) {
|
||||
findings.push({
|
||||
checkId: "channels.telegram.groups.allowFrom.wildcard",
|
||||
severity: "critical",
|
||||
title: "Telegram group allowlist contains wildcard",
|
||||
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.',
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!hasAnySenderAllowlist) {
|
||||
const providerSetting = (telegramCfg.commands as { nativeSkills?: unknown } | undefined)
|
||||
?.nativeSkills as any;
|
||||
const skillsEnabled = resolveNativeSkillsEnabled({
|
||||
providerId: "telegram",
|
||||
providerSetting,
|
||||
globalSetting: params.cfg.commands?.nativeSkills,
|
||||
});
|
||||
findings.push({
|
||||
checkId: "channels.telegram.groups.allowFrom.missing",
|
||||
severity: "critical",
|
||||
title: "Telegram group commands have no sender allowlist",
|
||||
detail:
|
||||
`Telegram group access is enabled but no sender allowlist is configured; this allows any group member to invoke /… commands` +
|
||||
(skillsEnabled ? " (including skill commands)." : "."),
|
||||
remediation:
|
||||
"Approve yourself via pairing (recommended), or set channels.telegram.groupAllowFrom (or per-group groups.<id>.allowFrom).",
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return findings;
|
||||
|
||||
Reference in New Issue
Block a user