fix(security): gate slash/control commands
This commit is contained in:
@@ -226,56 +226,171 @@ 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 prevStateDir = process.env.CLAWDBOT_STATE_DIR;
|
||||
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-discord-"));
|
||||
process.env.CLAWDBOT_STATE_DIR = tmp;
|
||||
await fs.mkdir(path.join(tmp, "credentials"), { recursive: true, mode: 0o700 });
|
||||
try {
|
||||
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 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");
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
checkId: "channels.discord.commands.native.no_allowlists",
|
||||
severity: "warn",
|
||||
}),
|
||||
]),
|
||||
);
|
||||
} finally {
|
||||
if (prevStateDir == null) delete process.env.CLAWDBOT_STATE_DIR;
|
||||
else process.env.CLAWDBOT_STATE_DIR = prevStateDir;
|
||||
}
|
||||
});
|
||||
|
||||
it("flags Discord slash commands when access-group enforcement is disabled and no users allowlist exists", async () => {
|
||||
const prevStateDir = process.env.CLAWDBOT_STATE_DIR;
|
||||
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-discord-open-"));
|
||||
process.env.CLAWDBOT_STATE_DIR = tmp;
|
||||
await fs.mkdir(path.join(tmp, "credentials"), { recursive: true, mode: 0o700 });
|
||||
try {
|
||||
const cfg: ClawdbotConfig = {
|
||||
commands: { useAccessGroups: false },
|
||||
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],
|
||||
});
|
||||
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
checkId: "channels.discord.commands.native.unrestricted",
|
||||
severity: "critical",
|
||||
}),
|
||||
]),
|
||||
);
|
||||
} finally {
|
||||
if (prevStateDir == null) delete process.env.CLAWDBOT_STATE_DIR;
|
||||
else process.env.CLAWDBOT_STATE_DIR = prevStateDir;
|
||||
}
|
||||
});
|
||||
|
||||
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 prevStateDir = process.env.CLAWDBOT_STATE_DIR;
|
||||
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-slack-"));
|
||||
process.env.CLAWDBOT_STATE_DIR = tmp;
|
||||
await fs.mkdir(path.join(tmp, "credentials"), { recursive: true, mode: 0o700 });
|
||||
try {
|
||||
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 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");
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
checkId: "channels.slack.commands.slash.no_allowlists",
|
||||
severity: "warn",
|
||||
}),
|
||||
]),
|
||||
);
|
||||
} finally {
|
||||
if (prevStateDir == null) delete process.env.CLAWDBOT_STATE_DIR;
|
||||
else process.env.CLAWDBOT_STATE_DIR = prevStateDir;
|
||||
}
|
||||
});
|
||||
|
||||
it("flags Slack slash commands when access-group enforcement is disabled", async () => {
|
||||
const prevStateDir = process.env.CLAWDBOT_STATE_DIR;
|
||||
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-security-audit-slack-open-"));
|
||||
process.env.CLAWDBOT_STATE_DIR = tmp;
|
||||
await fs.mkdir(path.join(tmp, "credentials"), { recursive: true, mode: 0o700 });
|
||||
try {
|
||||
const cfg: ClawdbotConfig = {
|
||||
commands: { useAccessGroups: false },
|
||||
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],
|
||||
});
|
||||
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
checkId: "channels.slack.commands.slash.useAccessGroups_off",
|
||||
severity: "critical",
|
||||
}),
|
||||
]),
|
||||
);
|
||||
} finally {
|
||||
if (prevStateDir == null) delete process.env.CLAWDBOT_STATE_DIR;
|
||||
else process.env.CLAWDBOT_STATE_DIR = prevStateDir;
|
||||
}
|
||||
});
|
||||
|
||||
it("flags Telegram group commands without a sender allowlist", async () => {
|
||||
|
||||
@@ -20,7 +20,7 @@ import {
|
||||
readConfigSnapshotForAudit,
|
||||
} from "./audit-extra.js";
|
||||
import { readChannelAllowFromStore } from "../pairing/pairing-store.js";
|
||||
import { resolveNativeSkillsEnabled } from "../config/commands.js";
|
||||
import { resolveNativeCommandsEnabled, resolveNativeSkillsEnabled } from "../config/commands.js";
|
||||
import {
|
||||
formatOctal,
|
||||
isGroupReadable,
|
||||
@@ -381,6 +381,13 @@ async function collectChannelSecurityFindings(params: {
|
||||
}): Promise<SecurityAuditFinding[]> {
|
||||
const findings: SecurityAuditFinding[] = [];
|
||||
|
||||
const coerceNativeSetting = (value: unknown): boolean | "auto" | undefined => {
|
||||
if (value === true) return true;
|
||||
if (value === false) return false;
|
||||
if (value === "auto") return "auto";
|
||||
return undefined;
|
||||
};
|
||||
|
||||
const warnDmPolicy = async (input: {
|
||||
label: string;
|
||||
provider: ChannelId;
|
||||
@@ -465,6 +472,140 @@ async function collectChannelSecurityFindings(params: {
|
||||
: true;
|
||||
if (!configured) continue;
|
||||
|
||||
if (plugin.id === "discord") {
|
||||
const discordCfg =
|
||||
(account as { config?: Record<string, unknown> } | null)?.config ?? ({} as Record<
|
||||
string,
|
||||
unknown
|
||||
>);
|
||||
const nativeEnabled = resolveNativeCommandsEnabled({
|
||||
providerId: "discord",
|
||||
providerSetting: coerceNativeSetting(
|
||||
(discordCfg.commands as { native?: unknown } | undefined)?.native,
|
||||
),
|
||||
globalSetting: params.cfg.commands?.native,
|
||||
});
|
||||
const nativeSkillsEnabled = resolveNativeSkillsEnabled({
|
||||
providerId: "discord",
|
||||
providerSetting: coerceNativeSetting(
|
||||
(discordCfg.commands as { nativeSkills?: unknown } | undefined)?.nativeSkills,
|
||||
),
|
||||
globalSetting: params.cfg.commands?.nativeSkills,
|
||||
});
|
||||
const slashEnabled = nativeEnabled || nativeSkillsEnabled;
|
||||
if (slashEnabled) {
|
||||
const groupPolicy = (discordCfg.groupPolicy as string | undefined) ?? "allowlist";
|
||||
const guildEntries = (discordCfg.guilds as Record<string, unknown> | undefined) ?? {};
|
||||
const guildsConfigured = Object.keys(guildEntries).length > 0;
|
||||
const hasAnyUserAllowlist = Object.values(guildEntries).some((guild) => {
|
||||
if (!guild || typeof guild !== "object") return false;
|
||||
const g = guild as Record<string, unknown>;
|
||||
if (Array.isArray(g.users) && g.users.length > 0) return true;
|
||||
const channels = g.channels;
|
||||
if (!channels || typeof channels !== "object") return false;
|
||||
return Object.values(channels as Record<string, unknown>).some((channel) => {
|
||||
if (!channel || typeof channel !== "object") return false;
|
||||
const c = channel as Record<string, unknown>;
|
||||
return Array.isArray(c.users) && c.users.length > 0;
|
||||
});
|
||||
});
|
||||
const dmAllowFromRaw = (discordCfg.dm as { allowFrom?: unknown } | undefined)?.allowFrom;
|
||||
const dmAllowFrom = Array.isArray(dmAllowFromRaw) ? dmAllowFromRaw : [];
|
||||
const storeAllowFrom = await readChannelAllowFromStore("discord").catch(() => []);
|
||||
const ownerAllowFromConfigured =
|
||||
normalizeAllowFromList([...dmAllowFrom, ...storeAllowFrom]).length > 0;
|
||||
|
||||
const useAccessGroups = params.cfg.commands?.useAccessGroups !== false;
|
||||
if (!useAccessGroups && groupPolicy !== "disabled" && guildsConfigured && !hasAnyUserAllowlist) {
|
||||
findings.push({
|
||||
checkId: "channels.discord.commands.native.unrestricted",
|
||||
severity: "critical",
|
||||
title: "Discord slash commands are unrestricted",
|
||||
detail:
|
||||
'commands.useAccessGroups=false disables sender allowlists for Discord slash commands unless a per-guild/channel users allowlist is configured; with no users allowlist, any user in allowed guild channels can invoke /… commands.',
|
||||
remediation:
|
||||
'Set commands.useAccessGroups=true (recommended), or configure channels.discord.guilds.<id>.users (or channels.discord.guilds.<id>.channels.<channel>.users).',
|
||||
});
|
||||
} else if (
|
||||
useAccessGroups &&
|
||||
groupPolicy !== "disabled" &&
|
||||
guildsConfigured &&
|
||||
!ownerAllowFromConfigured &&
|
||||
!hasAnyUserAllowlist
|
||||
) {
|
||||
findings.push({
|
||||
checkId: "channels.discord.commands.native.no_allowlists",
|
||||
severity: "warn",
|
||||
title: "Discord slash commands have no allowlists",
|
||||
detail:
|
||||
"Discord slash commands are enabled, but neither an owner allowFrom list nor any per-guild/channel users allowlist is configured; /… commands will be rejected for everyone.",
|
||||
remediation:
|
||||
'Add your user id to channels.discord.dm.allowFrom (or approve yourself via pairing), or configure channels.discord.guilds.<id>.users.',
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (plugin.id === "slack") {
|
||||
const slackCfg =
|
||||
(account as { config?: Record<string, unknown>; dm?: Record<string, unknown> } | null)
|
||||
?.config ?? ({} as Record<string, unknown>);
|
||||
const nativeEnabled = resolveNativeCommandsEnabled({
|
||||
providerId: "slack",
|
||||
providerSetting: coerceNativeSetting(
|
||||
(slackCfg.commands as { native?: unknown } | undefined)?.native,
|
||||
),
|
||||
globalSetting: params.cfg.commands?.native,
|
||||
});
|
||||
const nativeSkillsEnabled = resolveNativeSkillsEnabled({
|
||||
providerId: "slack",
|
||||
providerSetting: coerceNativeSetting(
|
||||
(slackCfg.commands as { nativeSkills?: unknown } | undefined)?.nativeSkills,
|
||||
),
|
||||
globalSetting: params.cfg.commands?.nativeSkills,
|
||||
});
|
||||
const slashCommandEnabled =
|
||||
nativeEnabled ||
|
||||
nativeSkillsEnabled ||
|
||||
((slackCfg.slashCommand as { enabled?: unknown } | undefined)?.enabled === true);
|
||||
if (slashCommandEnabled) {
|
||||
const useAccessGroups = params.cfg.commands?.useAccessGroups !== false;
|
||||
if (!useAccessGroups) {
|
||||
findings.push({
|
||||
checkId: "channels.slack.commands.slash.useAccessGroups_off",
|
||||
severity: "critical",
|
||||
title: "Slack slash commands bypass access groups",
|
||||
detail:
|
||||
"Slack slash/native commands are enabled while commands.useAccessGroups=false; this can allow unrestricted /… command execution from channels/users you didn't explicitly authorize.",
|
||||
remediation: "Set commands.useAccessGroups=true (recommended).",
|
||||
});
|
||||
} else {
|
||||
const dmAllowFromRaw = (account as { dm?: { allowFrom?: unknown } } | null)?.dm?.allowFrom;
|
||||
const dmAllowFrom = Array.isArray(dmAllowFromRaw) ? dmAllowFromRaw : [];
|
||||
const storeAllowFrom = await readChannelAllowFromStore("slack").catch(() => []);
|
||||
const ownerAllowFromConfigured =
|
||||
normalizeAllowFromList([...dmAllowFrom, ...storeAllowFrom]).length > 0;
|
||||
const channels = (slackCfg.channels as Record<string, unknown> | undefined) ?? {};
|
||||
const hasAnyChannelUsersAllowlist = Object.values(channels).some((value) => {
|
||||
if (!value || typeof value !== "object") return false;
|
||||
const channel = value as Record<string, unknown>;
|
||||
return Array.isArray(channel.users) && channel.users.length > 0;
|
||||
});
|
||||
if (!ownerAllowFromConfigured && !hasAnyChannelUsersAllowlist) {
|
||||
findings.push({
|
||||
checkId: "channels.slack.commands.slash.no_allowlists",
|
||||
severity: "warn",
|
||||
title: "Slack slash commands have no allowlists",
|
||||
detail:
|
||||
"Slack slash/native commands are enabled, but neither an owner allowFrom list nor any channels.<id>.users allowlist is configured; /… commands will be rejected for everyone.",
|
||||
remediation:
|
||||
"Approve yourself via pairing (recommended), or set channels.slack.dm.allowFrom and/or channels.slack.channels.<id>.users.",
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const dmPolicy = plugin.security.resolveDmPolicy?.({
|
||||
cfg: params.cfg,
|
||||
accountId: defaultAccountId,
|
||||
|
||||
Reference in New Issue
Block a user