From 3700151ec07b714f179504fab6aaf430f04f7442 Mon Sep 17 00:00:00 2001 From: Brian Mendonca Date: Sun, 22 Feb 2026 00:51:40 -0700 Subject: [PATCH] Channels: fail closed when Slack/Discord config is missing --- docs/channels/discord.md | 2 +- docs/channels/slack.md | 2 +- .../monitor/provider.group-policy.test.ts | 29 ++++++++++++++ src/discord/monitor/provider.ts | 37 +++++++++++++---- .../monitor/provider.group-policy.test.ts | 29 ++++++++++++++ src/slack/monitor/provider.ts | 40 +++++++++++++++---- 6 files changed, 121 insertions(+), 18 deletions(-) create mode 100644 src/discord/monitor/provider.group-policy.test.ts create mode 100644 src/slack/monitor/provider.group-policy.test.ts diff --git a/docs/channels/discord.md b/docs/channels/discord.md index d725b5c2edd..6cdd3aa410c 100644 --- a/docs/channels/discord.md +++ b/docs/channels/discord.md @@ -425,7 +425,7 @@ Example: } ``` - If you only set `DISCORD_BOT_TOKEN` and do not create a `channels.discord` block, runtime fallback is `groupPolicy="open"` (with a warning in logs). + If you only set `DISCORD_BOT_TOKEN` and do not create a `channels.discord` block, runtime fallback is `groupPolicy="allowlist"` (with a warning in logs). diff --git a/docs/channels/slack.md b/docs/channels/slack.md index 0d0bba3cb27..13c53b02459 100644 --- a/docs/channels/slack.md +++ b/docs/channels/slack.md @@ -165,7 +165,7 @@ For actions/directory reads, user token can be preferred when configured. For wr Channel allowlist lives under `channels.slack.channels`. - Runtime note: if `channels.slack` is completely missing (env-only setup) and `channels.defaults.groupPolicy` is unset, runtime falls back to `groupPolicy="open"` and logs a warning. + Runtime note: if `channels.slack` is completely missing (env-only setup) and `channels.defaults.groupPolicy` is unset, runtime falls back to `groupPolicy="allowlist"` and logs a warning. Name/ID resolution: diff --git a/src/discord/monitor/provider.group-policy.test.ts b/src/discord/monitor/provider.group-policy.test.ts new file mode 100644 index 00000000000..50a3377f806 --- /dev/null +++ b/src/discord/monitor/provider.group-policy.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { __testing } from "./provider.js"; + +describe("resolveDiscordRuntimeGroupPolicy", () => { + it("fails closed when channels.discord is missing and no defaults are set", () => { + const resolved = __testing.resolveDiscordRuntimeGroupPolicy({ + providerConfigPresent: false, + }); + expect(resolved.groupPolicy).toBe("allowlist"); + expect(resolved.providerMissingFallbackApplied).toBe(true); + }); + + it("keeps open default when channels.discord is configured", () => { + const resolved = __testing.resolveDiscordRuntimeGroupPolicy({ + providerConfigPresent: true, + }); + expect(resolved.groupPolicy).toBe("open"); + expect(resolved.providerMissingFallbackApplied).toBe(false); + }); + + it("respects explicit provider policy", () => { + const resolved = __testing.resolveDiscordRuntimeGroupPolicy({ + providerConfigPresent: false, + groupPolicy: "disabled", + }); + expect(resolved.groupPolicy).toBe("disabled"); + expect(resolved.providerMissingFallbackApplied).toBe(false); + }); +}); diff --git a/src/discord/monitor/provider.ts b/src/discord/monitor/provider.ts index ff16a262145..bfe8880098d 100644 --- a/src/discord/monitor/provider.ts +++ b/src/discord/monitor/provider.ts @@ -21,6 +21,7 @@ import { } from "../../config/commands.js"; import type { OpenClawConfig, ReplyToMode } from "../../config/config.js"; import { loadConfig } from "../../config/config.js"; +import type { GroupPolicy } from "../../config/types.base.js"; import { danger, logVerbose, shouldLogVerbose, warn } from "../../globals.js"; import { formatErrorMessage } from "../../infra/errors.js"; import { createDiscordRetryRunner } from "../../infra/retry-policy.js"; @@ -170,6 +171,25 @@ function dedupeSkillCommandsForDiscord( return deduped; } +function resolveDiscordRuntimeGroupPolicy(params: { + providerConfigPresent: boolean; + groupPolicy?: GroupPolicy; + defaultGroupPolicy?: GroupPolicy; +}): { + groupPolicy: GroupPolicy; + providerMissingFallbackApplied: boolean; +} { + const groupPolicy = + params.groupPolicy ?? + params.defaultGroupPolicy ?? + (params.providerConfigPresent ? "open" : "allowlist"); + const providerMissingFallbackApplied = + !params.providerConfigPresent && + params.groupPolicy === undefined && + params.defaultGroupPolicy === undefined; + return { groupPolicy, providerMissingFallbackApplied }; +} + async function deployDiscordCommands(params: { client: Client; runtime: RuntimeEnv; @@ -253,16 +273,16 @@ export async function monitorDiscordProvider(opts: MonitorDiscordOpts = {}) { const dmConfig = discordCfg.dm; let guildEntries = discordCfg.guilds; const defaultGroupPolicy = cfg.channels?.defaults?.groupPolicy; - const groupPolicy = discordCfg.groupPolicy ?? defaultGroupPolicy ?? "open"; - if ( - discordCfg.groupPolicy === undefined && - discordCfg.guilds === undefined && - defaultGroupPolicy === undefined && - groupPolicy === "open" - ) { + const providerConfigPresent = cfg.channels?.discord !== undefined; + const { groupPolicy, providerMissingFallbackApplied } = resolveDiscordRuntimeGroupPolicy({ + providerConfigPresent, + groupPolicy: discordCfg.groupPolicy, + defaultGroupPolicy, + }); + if (providerMissingFallbackApplied) { runtime.log?.( warn( - 'discord: groupPolicy defaults to "open" when channels.discord is missing; set channels.discord.groupPolicy (or channels.defaults.groupPolicy) or add channels.discord.guilds to restrict access.', + 'discord: channels.discord is missing; defaulting groupPolicy to "allowlist" (guild messages blocked until explicitly configured).', ), ); } @@ -622,6 +642,7 @@ async function clearDiscordNativeCommands(params: { export const __testing = { createDiscordGatewayPlugin, dedupeSkillCommandsForDiscord, + resolveDiscordRuntimeGroupPolicy, resolveDiscordRestFetch, resolveThreadBindingsEnabled, }; diff --git a/src/slack/monitor/provider.group-policy.test.ts b/src/slack/monitor/provider.group-policy.test.ts new file mode 100644 index 00000000000..43bc8dfec54 --- /dev/null +++ b/src/slack/monitor/provider.group-policy.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { __testing } from "./provider.js"; + +describe("resolveSlackRuntimeGroupPolicy", () => { + it("fails closed when channels.slack is missing and no defaults are set", () => { + const resolved = __testing.resolveSlackRuntimeGroupPolicy({ + providerConfigPresent: false, + }); + expect(resolved.groupPolicy).toBe("allowlist"); + expect(resolved.providerMissingFallbackApplied).toBe(true); + }); + + it("keeps open default when channels.slack is configured", () => { + const resolved = __testing.resolveSlackRuntimeGroupPolicy({ + providerConfigPresent: true, + }); + expect(resolved.groupPolicy).toBe("open"); + expect(resolved.providerMissingFallbackApplied).toBe(false); + }); + + it("respects explicit global defaults", () => { + const resolved = __testing.resolveSlackRuntimeGroupPolicy({ + providerConfigPresent: false, + defaultGroupPolicy: "open", + }); + expect(resolved.groupPolicy).toBe("open"); + expect(resolved.providerMissingFallbackApplied).toBe(false); + }); +}); diff --git a/src/slack/monitor/provider.ts b/src/slack/monitor/provider.ts index 248728751e6..4d9d50331a9 100644 --- a/src/slack/monitor/provider.ts +++ b/src/slack/monitor/provider.ts @@ -11,6 +11,7 @@ import { } from "../../channels/allowlists/resolve-utils.js"; import { loadConfig } from "../../config/config.js"; import type { SessionScope } from "../../config/sessions.js"; +import type { GroupPolicy } from "../../config/types.base.js"; import { warn } from "../../globals.js"; import { installRequestBodyLimitGuard } from "../../infra/http-body.js"; import { normalizeMainKey } from "../../routing/session-key.js"; @@ -41,6 +42,25 @@ const { App, HTTPReceiver } = slackBolt; const SLACK_WEBHOOK_MAX_BODY_BYTES = 1024 * 1024; const SLACK_WEBHOOK_BODY_TIMEOUT_MS = 30_000; +function resolveSlackRuntimeGroupPolicy(params: { + providerConfigPresent: boolean; + groupPolicy?: GroupPolicy; + defaultGroupPolicy?: GroupPolicy; +}): { + groupPolicy: GroupPolicy; + providerMissingFallbackApplied: boolean; +} { + const groupPolicy = + params.groupPolicy ?? + params.defaultGroupPolicy ?? + (params.providerConfigPresent ? "open" : "allowlist"); + const providerMissingFallbackApplied = + !params.providerConfigPresent && + params.groupPolicy === undefined && + params.defaultGroupPolicy === undefined; + return { groupPolicy, providerMissingFallbackApplied }; +} + function parseApiAppIdFromAppToken(raw?: string) { const token = raw?.trim(); if (!token) { @@ -99,16 +119,16 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { const groupDmChannels = dmConfig?.groupChannels; let channelsConfig = slackCfg.channels; const defaultGroupPolicy = cfg.channels?.defaults?.groupPolicy; - const groupPolicy = slackCfg.groupPolicy ?? defaultGroupPolicy ?? "open"; - if ( - slackCfg.groupPolicy === undefined && - slackCfg.channels === undefined && - defaultGroupPolicy === undefined && - groupPolicy === "open" - ) { + const providerConfigPresent = cfg.channels?.slack !== undefined; + const { groupPolicy, providerMissingFallbackApplied } = resolveSlackRuntimeGroupPolicy({ + providerConfigPresent, + groupPolicy: slackCfg.groupPolicy, + defaultGroupPolicy, + }); + if (providerMissingFallbackApplied) { runtime.log?.( warn( - 'slack: groupPolicy defaults to "open" when channels.slack is missing; set channels.slack.groupPolicy (or channels.defaults.groupPolicy) or add channels.slack.channels to restrict access.', + 'slack: channels.slack is missing; defaulting groupPolicy to "allowlist" (group messages blocked until explicitly configured).', ), ); } @@ -363,3 +383,7 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { await app.stop().catch(() => undefined); } } + +export const __testing = { + resolveSlackRuntimeGroupPolicy, +};