From cfa44ea6b4f7ad3210c65a2bc2e18b5754852615 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 24 Feb 2026 01:01:51 +0000 Subject: [PATCH] fix(security): make allowFrom id-only by default with dangerous name opt-in (#24907) * fix(channels): default allowFrom to id-only; add dangerous name opt-in * docs(security): align channel allowFrom docs with id-only default --- SECURITY.md | 1 + docs/channels/discord.md | 5 +- docs/channels/googlechat.md | 6 +- docs/channels/irc.md | 3 +- docs/channels/mattermost.md | 3 +- docs/channels/msteams.md | 7 +- docs/channels/slack.md | 2 + docs/cli/security.md | 3 +- docs/gateway/configuration-examples.md | 11 +- docs/gateway/configuration-reference.md | 8 +- extensions/googlechat/src/monitor.test.ts | 9 +- extensions/googlechat/src/monitor.ts | 14 +- extensions/irc/src/config-schema.ts | 1 + extensions/irc/src/inbound.ts | 4 + extensions/irc/src/normalize.test.ts | 11 +- extensions/irc/src/normalize.ts | 12 +- extensions/irc/src/policy.test.ts | 17 + extensions/irc/src/policy.ts | 13 +- extensions/irc/src/types.ts | 5 + extensions/mattermost/src/config-schema.ts | 1 + .../mattermost/src/mattermost/monitor.ts | 19 +- extensions/mattermost/src/types.ts | 5 + .../src/monitor-handler/message-handler.ts | 6 + extensions/msteams/src/policy.ts | 2 + src/channels/allowlist-match.ts | 3 +- src/commands/doctor-config-flow.ts | 422 ++++++++++++++++++ src/config/types.discord.ts | 5 + src/config/types.googlechat.ts | 5 + src/config/types.msteams.ts | 5 + src/config/types.slack.ts | 5 + src/config/zod-schema.providers-core.ts | 4 + src/discord/monitor.test.ts | 37 +- src/discord/monitor/agent-components.ts | 12 + src/discord/monitor/allow-list.ts | 83 ++-- src/discord/monitor/listeners.ts | 4 + .../monitor/message-handler.preflight.ts | 16 +- .../monitor/message-handler.process.ts | 1 + src/discord/monitor/monitor.test.ts | 14 +- src/discord/monitor/native-command.ts | 30 +- src/discord/monitor/provider.ts | 2 + src/discord/voice/command.ts | 15 +- src/security/audit-channel.ts | 34 +- src/security/audit.test.ts | 37 ++ src/slack/monitor/allow-list.test.ts | 11 +- src/slack/monitor/allow-list.ts | 20 +- src/slack/monitor/auth.ts | 4 +- src/slack/monitor/channel-config.ts | 2 + src/slack/monitor/context.ts | 3 + .../monitor/message-handler/prepare.test.ts | 1 + src/slack/monitor/message-handler/prepare.ts | 4 + src/slack/monitor/monitor.test.ts | 1 + src/slack/monitor/provider.ts | 1 + src/slack/monitor/slash.ts | 3 + 53 files changed, 852 insertions(+), 100 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index bf6917656ca..dbe2ad84a97 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -95,6 +95,7 @@ OpenClaw does **not** model one gateway as a multi-tenant, adversarial user boun - Deployments where mutually untrusted/adversarial operators share one gateway host and config (for example, reports expecting per-operator isolation for `sessions.list`, `sessions.preview`, `chat.history`, or similar control-plane reads) - Prompt-injection-only attacks (without a policy/auth/sandbox boundary bypass) - Reports that require write access to trusted local state (`~/.openclaw`, workspace files like `MEMORY.md` / `memory/*.md`) +- Any report whose only claim is that an operator-enabled `dangerous*`/`dangerously*` config option weakens defaults (these are explicit break-glass tradeoffs by design) - Reports that depend on trusted operator-supplied configuration values to trigger availability impact (for example custom regex patterns). These may still be fixed as defense-in-depth hardening, but are not security-boundary bypasses. - Exposed secrets that are third-party/user-controlled credentials (not OpenClaw-owned and not granting access to OpenClaw-operated infrastructure/services) without demonstrated OpenClaw impact diff --git a/docs/channels/discord.md b/docs/channels/discord.md index 334c6d78ee5..108ef34d4ef 100644 --- a/docs/channels/discord.md +++ b/docs/channels/discord.md @@ -397,7 +397,8 @@ Example: `allowlist` behavior: - guild must match `channels.discord.guilds` (`id` preferred, slug accepted) - - optional sender allowlists: `users` (IDs or names) and `roles` (role IDs only); if either is configured, senders are allowed when they match `users` OR `roles` + - optional sender allowlists: `users` (stable IDs recommended) and `roles` (role IDs only); if either is configured, senders are allowed when they match `users` OR `roles` + - direct name/tag matching is disabled by default; enable `channels.discord.dangerouslyAllowNameMatching: true` only as break-glass compatibility mode - names/tags are supported for `users`, but IDs are safer; `openclaw security audit` warns when name/tag entries are used - if a guild has `channels` configured, non-listed channels are denied - if a guild has no `channels` block, all channels in that allowlisted guild are allowed @@ -768,7 +769,7 @@ Default slash command settings: Notes: - allowlists can use `pk:` - - member display names are matched by name/slug + - member display names are matched by name/slug only when `channels.discord.dangerouslyAllowNameMatching: true` - lookups use original message ID and are time-window constrained - if lookup fails, proxied messages are treated as bot messages and dropped unless `allowBots=true` diff --git a/docs/channels/googlechat.md b/docs/channels/googlechat.md index 818a8288f5d..13729257fe7 100644 --- a/docs/channels/googlechat.md +++ b/docs/channels/googlechat.md @@ -153,7 +153,8 @@ Configure your tunnel's ingress rules to only route the webhook path: Use these identifiers for delivery and allowlists: -- Direct messages: `users/` (recommended) or raw email `name@example.com` (mutable principal). +- Direct messages: `users/` (recommended). +- Raw email `name@example.com` is mutable and only used for direct allowlist matching when `channels.googlechat.dangerouslyAllowNameMatching: true`. - Deprecated: `users/` is treated as a user id, not an email allowlist. - Spaces: `spaces/`. @@ -171,7 +172,7 @@ Use these identifiers for delivery and allowlists: botUser: "users/1234567890", // optional; helps mention detection dm: { policy: "pairing", - allowFrom: ["users/1234567890", "name@example.com"], + allowFrom: ["users/1234567890"], }, groupPolicy: "allowlist", groups: { @@ -194,6 +195,7 @@ Notes: - Service account credentials can also be passed inline with `serviceAccount` (JSON string). - Default webhook path is `/googlechat` if `webhookPath` isn’t set. +- `dangerouslyAllowNameMatching` re-enables mutable email principal matching for allowlists (break-glass compatibility mode). - Reactions are available via the `reactions` tool and `channels action` when `actions.reactions` is enabled. - `typingIndicator` supports `none`, `message` (default), and `reaction` (reaction requires user OAuth). - Attachments are downloaded through the Chat API and stored in the media pipeline (size capped by `mediaMaxMb`). diff --git a/docs/channels/irc.md b/docs/channels/irc.md index 7496f574c4e..00403b6f92d 100644 --- a/docs/channels/irc.md +++ b/docs/channels/irc.md @@ -57,7 +57,8 @@ Config keys: - Per-channel controls (channel + sender + mention rules): `channels.irc.groups["#channel"]` - `channels.irc.groupPolicy="open"` allows unconfigured channels (**still mention-gated by default**) -Allowlist entries can use nick or `nick!user@host` forms. +Allowlist entries should use stable sender identities (`nick!user@host`). +Bare nick matching is mutable and only enabled when `channels.irc.dangerouslyAllowNameMatching: true`. ### Common gotcha: `allowFrom` is for DMs, not channels diff --git a/docs/channels/mattermost.md b/docs/channels/mattermost.md index 350fa8429c4..702f72cc01f 100644 --- a/docs/channels/mattermost.md +++ b/docs/channels/mattermost.md @@ -101,7 +101,8 @@ Notes: ## Channels (groups) - Default: `channels.mattermost.groupPolicy = "allowlist"` (mention-gated). -- Allowlist senders with `channels.mattermost.groupAllowFrom` (user IDs or `@username`). +- Allowlist senders with `channels.mattermost.groupAllowFrom` (user IDs recommended). +- `@username` matching is mutable and only enabled when `channels.mattermost.dangerouslyAllowNameMatching: true`. - Open channels: `channels.mattermost.groupPolicy="open"` (mention-gated). - Runtime note: if `channels.mattermost` is completely missing, runtime falls back to `groupPolicy="allowlist"` for group checks (even if `channels.defaults.groupPolicy` is set). diff --git a/docs/channels/msteams.md b/docs/channels/msteams.md index d8b9f0af865..9c4a583e1b5 100644 --- a/docs/channels/msteams.md +++ b/docs/channels/msteams.md @@ -87,7 +87,9 @@ Disable with: **DM access** - Default: `channels.msteams.dmPolicy = "pairing"`. Unknown senders are ignored until approved. -- `channels.msteams.allowFrom` accepts AAD object IDs, UPNs, or display names. The wizard resolves names to IDs via Microsoft Graph when credentials allow. +- `channels.msteams.allowFrom` should use stable AAD object IDs. +- UPNs/display names are mutable; direct matching is disabled by default and only enabled with `channels.msteams.dangerouslyAllowNameMatching: true`. +- The wizard can resolve names to IDs via Microsoft Graph when credentials allow. **Group access** @@ -454,7 +456,8 @@ Key settings (see `/gateway/configuration` for shared channel patterns): - `channels.msteams.webhook.port` (default `3978`) - `channels.msteams.webhook.path` (default `/api/messages`) - `channels.msteams.dmPolicy`: `pairing | allowlist | open | disabled` (default: pairing) -- `channels.msteams.allowFrom`: allowlist for DMs (AAD object IDs, UPNs, or display names). The wizard resolves names to IDs during setup when Graph access is available. +- `channels.msteams.allowFrom`: DM allowlist (AAD object IDs recommended). The wizard resolves names to IDs during setup when Graph access is available. +- `channels.msteams.dangerouslyAllowNameMatching`: break-glass toggle to re-enable mutable UPN/display-name matching. - `channels.msteams.textChunkLimit`: outbound text chunk size. - `channels.msteams.chunkMode`: `length` (default) or `newline` to split on blank lines (paragraph boundaries) before length chunking. - `channels.msteams.mediaAllowHosts`: allowlist for inbound attachment hosts (defaults to Microsoft/Teams domains). diff --git a/docs/channels/slack.md b/docs/channels/slack.md index beb79a511fc..869df30ad99 100644 --- a/docs/channels/slack.md +++ b/docs/channels/slack.md @@ -171,6 +171,7 @@ For actions/directory reads, user token can be preferred when configured. For wr - channel allowlist entries and DM allowlist entries are resolved at startup when token access allows - unresolved entries are kept as configured + - inbound authorization matching is ID-first by default; direct username/slug matching requires `channels.slack.dangerouslyAllowNameMatching: true` @@ -513,6 +514,7 @@ Primary reference: High-signal Slack fields: - mode/auth: `mode`, `botToken`, `appToken`, `signingSecret`, `webhookPath`, `accounts.*` - DM access: `dm.enabled`, `dmPolicy`, `allowFrom` (legacy: `dm.policy`, `dm.allowFrom`), `dm.groupEnabled`, `dm.groupChannels` + - compatibility toggle: `dangerouslyAllowNameMatching` (break-glass; keep off unless needed) - channel access: `groupPolicy`, `channels.*`, `channels.*.users`, `channels.*.requireMention` - threading/history: `replyToMode`, `replyToModeByChatType`, `thread.*`, `historyLimit`, `dmHistoryLimit`, `dms.*.historyLimit` - delivery: `textChunkLimit`, `chunkMode`, `mediaMaxMb`, `streaming`, `nativeStreaming` diff --git a/docs/cli/security.md b/docs/cli/security.md index e8b76c8e3e7..9b1cce7db79 100644 --- a/docs/cli/security.md +++ b/docs/cli/security.md @@ -32,8 +32,9 @@ It also flags `gateway.allowRealIpFallback=true` (header-spoofing risk if proxie It also warns when sandbox browser uses Docker `bridge` network without `sandbox.browser.cdpSourceRange`. It also warns when existing sandbox browser Docker containers have missing/stale hash labels (for example pre-migration containers missing `openclaw.browserConfigEpoch`) and recommends `openclaw sandbox recreate --browser --all`. It also warns when npm-based plugin/hook install records are unpinned, missing integrity metadata, or drift from currently installed package versions. -It warns when Discord allowlists (`channels.discord.allowFrom`, `channels.discord.guilds.*.users`, pairing store) use name or tag entries instead of stable IDs. +It warns when channel allowlists rely on mutable names/emails/tags instead of stable IDs (Discord, Slack, Google Chat, MS Teams, Mattermost, IRC scopes where applicable). It warns when `gateway.auth.mode="none"` leaves Gateway HTTP APIs reachable without a shared secret (`/tools/invoke` plus any enabled `/v1/*` endpoint). +Settings prefixed with `dangerous`/`dangerously` are explicit break-glass operator overrides; enabling one is not, by itself, a security vulnerability report. ## JSON output diff --git a/docs/gateway/configuration-examples.md b/docs/gateway/configuration-examples.md index 6d310b0a32d..d3838bbdae6 100644 --- a/docs/gateway/configuration-examples.md +++ b/docs/gateway/configuration-examples.md @@ -202,7 +202,7 @@ Save to `~/.openclaw/openclaw.json` and you can DM the bot from that number. discord: { enabled: true, token: "YOUR_DISCORD_BOT_TOKEN", - dm: { enabled: true, allowFrom: ["steipete"] }, + dm: { enabled: true, allowFrom: ["123456789012345678"] }, guilds: { "123456789012345678": { slug: "friends-of-openclaw", @@ -317,7 +317,7 @@ Save to `~/.openclaw/openclaw.json` and you can DM the bot from that number. allowFrom: { whatsapp: ["+15555550123"], telegram: ["123456789"], - discord: ["steipete"], + discord: ["123456789012345678"], slack: ["U123"], signal: ["+15555550123"], imessage: ["user@example.com"], @@ -461,7 +461,7 @@ Save to `~/.openclaw/openclaw.json` and you can DM the bot from that number. discord: { enabled: true, token: "YOUR_TOKEN", - dm: { allowFrom: ["yourname"] }, + dm: { allowFrom: ["123456789012345678"] }, }, }, } @@ -487,12 +487,15 @@ If more than one person can DM your bot (multiple entries in `allowFrom`, pairin discord: { enabled: true, token: "YOUR_DISCORD_BOT_TOKEN", - dm: { enabled: true, allowFrom: ["alice", "bob"] }, + dm: { enabled: true, allowFrom: ["123456789012345678", "987654321098765432"] }, }, }, } ``` +For Discord/Slack/Google Chat/MS Teams/Mattermost/IRC, sender authorization is ID-first by default. +Only enable direct mutable name/email/nick matching with each channel's `dangerouslyAllowNameMatching: true` if you explicitly accept that risk. + ### OAuth with API key failover ```json5 diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index 77379112907..58629f1db15 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -212,7 +212,7 @@ WhatsApp runs through the gateway's web channel (Baileys Web). It starts automat }, replyToMode: "off", // off | first | all dmPolicy: "pairing", - allowFrom: ["1234567890", "steipete"], + allowFrom: ["1234567890", "123456789012345678"], dm: { enabled: true, groupEnabled: false, groupChannels: ["openclaw-dm"] }, guilds: { "123456789012345678": { @@ -283,6 +283,7 @@ WhatsApp runs through the gateway's web channel (Baileys Web). It starts automat - `channels.discord.ui.components.accentColor` sets the accent color for Discord components v2 containers. - `channels.discord.voice` enables Discord voice channel conversations and optional auto-join + TTS overrides. - `channels.discord.streaming` is the canonical stream mode key. Legacy `streamMode` and boolean `streaming` values are auto-migrated. +- `channels.discord.dangerouslyAllowNameMatching` re-enables mutable name/tag matching (break-glass compatibility mode). **Reaction notification modes:** `off` (none), `own` (bot's messages, default), `all` (all messages), `allowlist` (from `guilds..users` on all messages). @@ -317,7 +318,8 @@ WhatsApp runs through the gateway's web channel (Baileys Web). It starts automat - Service account JSON: inline (`serviceAccount`) or file-based (`serviceAccountFile`). - Env fallbacks: `GOOGLE_CHAT_SERVICE_ACCOUNT` or `GOOGLE_CHAT_SERVICE_ACCOUNT_FILE`. -- Use `spaces/` or `users/` for delivery targets. +- Use `spaces/` or `users/` for delivery targets. +- `channels.googlechat.dangerouslyAllowNameMatching` re-enables mutable email principal matching (break-glass compatibility mode). ### Slack @@ -1490,7 +1492,7 @@ Controls elevated (host) exec access: enabled: true, allowFrom: { whatsapp: ["+15555550123"], - discord: ["steipete", "1234567890123"], + discord: ["1234567890123", "987654321098765432"], }, }, }, diff --git a/extensions/googlechat/src/monitor.test.ts b/extensions/googlechat/src/monitor.test.ts index 6eec88abbe4..2a4e9935e2c 100644 --- a/extensions/googlechat/src/monitor.test.ts +++ b/extensions/googlechat/src/monitor.test.ts @@ -2,8 +2,9 @@ import { describe, expect, it } from "vitest"; import { isSenderAllowed } from "./monitor.js"; describe("isSenderAllowed", () => { - it("matches allowlist entries with raw email", () => { - expect(isSenderAllowed("users/123", "Jane@Example.com", ["jane@example.com"])).toBe(true); + it("matches raw email entries only when dangerous name matching is enabled", () => { + expect(isSenderAllowed("users/123", "Jane@Example.com", ["jane@example.com"])).toBe(false); + expect(isSenderAllowed("users/123", "Jane@Example.com", ["jane@example.com"], true)).toBe(true); }); it("does not treat users/ entries as email allowlist (deprecated form)", () => { @@ -17,6 +18,8 @@ describe("isSenderAllowed", () => { }); it("rejects non-matching raw email entries", () => { - expect(isSenderAllowed("users/123", "jane@example.com", ["other@example.com"])).toBe(false); + expect(isSenderAllowed("users/123", "jane@example.com", ["other@example.com"], true)).toBe( + false, + ); }); }); diff --git a/extensions/googlechat/src/monitor.ts b/extensions/googlechat/src/monitor.ts index 689f10341c2..19bd2f6421a 100644 --- a/extensions/googlechat/src/monitor.ts +++ b/extensions/googlechat/src/monitor.ts @@ -287,6 +287,7 @@ export function isSenderAllowed( senderId: string, senderEmail: string | undefined, allowFrom: string[], + allowNameMatching = false, ) { if (allowFrom.includes("*")) { return true; @@ -305,8 +306,8 @@ export function isSenderAllowed( return normalizeUserId(withoutPrefix) === normalizedSenderId; } - // Raw email allowlist entries remain supported for usability. - if (normalizedEmail && isEmailLike(withoutPrefix)) { + // Raw email allowlist entries are a break-glass override. + if (allowNameMatching && normalizedEmail && isEmailLike(withoutPrefix)) { return withoutPrefix === normalizedEmail; } @@ -409,6 +410,7 @@ async function processMessageWithPipeline(params: { const senderId = sender?.name ?? ""; const senderName = sender?.displayName ?? ""; const senderEmail = sender?.email ?? undefined; + const allowNameMatching = account.config.dangerouslyAllowNameMatching === true; const allowBots = account.config.allowBots === true; if (!allowBots) { @@ -489,6 +491,7 @@ async function processMessageWithPipeline(params: { senderId, senderEmail, groupUsers.map((v) => String(v)), + allowNameMatching, ); if (!ok) { logVerbose(core, runtime, `drop group message (sender not allowed, ${senderId})`); @@ -508,7 +511,12 @@ async function processMessageWithPipeline(params: { warnDeprecatedUsersEmailEntries(core, runtime, effectiveAllowFrom); const commandAllowFrom = isGroup ? groupUsers.map((v) => String(v)) : effectiveAllowFrom; const useAccessGroups = config.commands?.useAccessGroups !== false; - const senderAllowedForCommands = isSenderAllowed(senderId, senderEmail, commandAllowFrom); + const senderAllowedForCommands = isSenderAllowed( + senderId, + senderEmail, + commandAllowFrom, + allowNameMatching, + ); const commandAuthorized = shouldComputeAuth ? core.channel.commands.resolveCommandAuthorizedFromAuthorizers({ useAccessGroups, diff --git a/extensions/irc/src/config-schema.ts b/extensions/irc/src/config-schema.ts index 34eb85b021d..74a7ac363af 100644 --- a/extensions/irc/src/config-schema.ts +++ b/extensions/irc/src/config-schema.ts @@ -46,6 +46,7 @@ export const IrcAccountSchemaBase = z .object({ name: z.string().optional(), enabled: z.boolean().optional(), + dangerouslyAllowNameMatching: z.boolean().optional(), host: z.string().optional(), port: z.number().int().min(1).max(65535).optional(), tls: z.boolean().optional(), diff --git a/extensions/irc/src/inbound.ts b/extensions/irc/src/inbound.ts index 7c82573b3bd..db2bd584f14 100644 --- a/extensions/irc/src/inbound.ts +++ b/extensions/irc/src/inbound.ts @@ -78,6 +78,7 @@ export async function handleIrcInbound(params: { const senderDisplay = message.senderHost ? `${message.senderNick}!${message.senderUser ?? "?"}@${message.senderHost}` : message.senderNick; + const allowNameMatching = account.config.dangerouslyAllowNameMatching === true; const dmPolicy = account.config.dmPolicy ?? "pairing"; const defaultGroupPolicy = resolveDefaultGroupPolicy(config); @@ -132,6 +133,7 @@ export async function handleIrcInbound(params: { const senderAllowedForCommands = resolveIrcAllowlistMatch({ allowFrom: message.isGroup ? effectiveGroupAllowFrom : effectiveAllowFrom, message, + allowNameMatching, }).allowed; const hasControlCommand = core.channel.text.hasControlCommand(rawBody, config as OpenClawConfig); const commandGate = resolveControlCommandGate({ @@ -153,6 +155,7 @@ export async function handleIrcInbound(params: { message, outerAllowFrom: effectiveGroupAllowFrom, innerAllowFrom: groupAllowFrom, + allowNameMatching, }); if (!senderAllowed) { runtime.log?.(`irc: drop group sender ${senderDisplay} (policy=${groupPolicy})`); @@ -167,6 +170,7 @@ export async function handleIrcInbound(params: { const dmAllowed = resolveIrcAllowlistMatch({ allowFrom: effectiveAllowFrom, message, + allowNameMatching, }).allowed; if (!dmAllowed) { if (dmPolicy === "pairing") { diff --git a/extensions/irc/src/normalize.test.ts b/extensions/irc/src/normalize.test.ts index a498ffaacd0..428f0015fd2 100644 --- a/extensions/irc/src/normalize.test.ts +++ b/extensions/irc/src/normalize.test.ts @@ -30,6 +30,8 @@ describe("irc normalize", () => { }; expect(buildIrcAllowlistCandidates(message)).toContain("alice!ident@example.org"); + expect(buildIrcAllowlistCandidates(message)).not.toContain("alice"); + expect(buildIrcAllowlistCandidates(message, { allowNameMatching: true })).toContain("alice"); expect( resolveIrcAllowlistMatch({ allowFrom: ["alice!ident@example.org"], @@ -38,9 +40,16 @@ describe("irc normalize", () => { ).toBe(true); expect( resolveIrcAllowlistMatch({ - allowFrom: ["bob"], + allowFrom: ["alice"], message, }).allowed, ).toBe(false); + expect( + resolveIrcAllowlistMatch({ + allowFrom: ["alice"], + message, + allowNameMatching: true, + }).allowed, + ).toBe(true); }); }); diff --git a/extensions/irc/src/normalize.ts b/extensions/irc/src/normalize.ts index 89d135dbfd7..90b731dcbbf 100644 --- a/extensions/irc/src/normalize.ts +++ b/extensions/irc/src/normalize.ts @@ -77,12 +77,15 @@ export function formatIrcSenderId(message: IrcInboundMessage): string { return base; } -export function buildIrcAllowlistCandidates(message: IrcInboundMessage): string[] { +export function buildIrcAllowlistCandidates( + message: IrcInboundMessage, + params?: { allowNameMatching?: boolean }, +): string[] { const nick = message.senderNick.trim().toLowerCase(); const user = message.senderUser?.trim().toLowerCase(); const host = message.senderHost?.trim().toLowerCase(); const candidates = new Set(); - if (nick) { + if (nick && params?.allowNameMatching === true) { candidates.add(nick); } if (nick && user) { @@ -100,6 +103,7 @@ export function buildIrcAllowlistCandidates(message: IrcInboundMessage): string[ export function resolveIrcAllowlistMatch(params: { allowFrom: string[]; message: IrcInboundMessage; + allowNameMatching?: boolean; }): { allowed: boolean; source?: string } { const allowFrom = new Set( params.allowFrom.map((entry) => entry.trim().toLowerCase()).filter(Boolean), @@ -107,7 +111,9 @@ export function resolveIrcAllowlistMatch(params: { if (allowFrom.has("*")) { return { allowed: true, source: "wildcard" }; } - const candidates = buildIrcAllowlistCandidates(params.message); + const candidates = buildIrcAllowlistCandidates(params.message, { + allowNameMatching: params.allowNameMatching, + }); for (const candidate of candidates) { if (allowFrom.has(candidate)) { return { allowed: true, source: candidate }; diff --git a/extensions/irc/src/policy.test.ts b/extensions/irc/src/policy.test.ts index be3f65e617e..4136466ca79 100644 --- a/extensions/irc/src/policy.test.ts +++ b/extensions/irc/src/policy.test.ts @@ -50,6 +50,14 @@ describe("irc policy", () => { }), ).toBe(false); + expect( + resolveIrcGroupSenderAllowed({ + groupPolicy: "allowlist", + message, + outerAllowFrom: ["alice!ident@example.org"], + innerAllowFrom: [], + }), + ).toBe(true); expect( resolveIrcGroupSenderAllowed({ groupPolicy: "allowlist", @@ -57,6 +65,15 @@ describe("irc policy", () => { outerAllowFrom: ["alice"], innerAllowFrom: [], }), + ).toBe(false); + expect( + resolveIrcGroupSenderAllowed({ + groupPolicy: "allowlist", + message, + outerAllowFrom: ["alice"], + innerAllowFrom: [], + allowNameMatching: true, + }), ).toBe(true); }); diff --git a/extensions/irc/src/policy.ts b/extensions/irc/src/policy.ts index 81828a5ac09..356f0fae7d8 100644 --- a/extensions/irc/src/policy.ts +++ b/extensions/irc/src/policy.ts @@ -142,16 +142,25 @@ export function resolveIrcGroupSenderAllowed(params: { message: IrcInboundMessage; outerAllowFrom: string[]; innerAllowFrom: string[]; + allowNameMatching?: boolean; }): boolean { const policy = params.groupPolicy ?? "allowlist"; const inner = normalizeIrcAllowlist(params.innerAllowFrom); const outer = normalizeIrcAllowlist(params.outerAllowFrom); if (inner.length > 0) { - return resolveIrcAllowlistMatch({ allowFrom: inner, message: params.message }).allowed; + return resolveIrcAllowlistMatch({ + allowFrom: inner, + message: params.message, + allowNameMatching: params.allowNameMatching, + }).allowed; } if (outer.length > 0) { - return resolveIrcAllowlistMatch({ allowFrom: outer, message: params.message }).allowed; + return resolveIrcAllowlistMatch({ + allowFrom: outer, + message: params.message, + allowNameMatching: params.allowNameMatching, + }).allowed; } return policy === "open"; } diff --git a/extensions/irc/src/types.ts b/extensions/irc/src/types.ts index 2da3d31bafc..03e2d3f5eb3 100644 --- a/extensions/irc/src/types.ts +++ b/extensions/irc/src/types.ts @@ -32,6 +32,11 @@ export type IrcNickServConfig = { export type IrcAccountConfig = { name?: string; enabled?: boolean; + /** + * Break-glass override: allow nick-only allowlist matching. + * Default behavior requires host/user-qualified identities. + */ + dangerouslyAllowNameMatching?: boolean; host?: string; port?: number; tls?: boolean; diff --git a/extensions/mattermost/src/config-schema.ts b/extensions/mattermost/src/config-schema.ts index 7628613a16b..bb0d99e5667 100644 --- a/extensions/mattermost/src/config-schema.ts +++ b/extensions/mattermost/src/config-schema.ts @@ -11,6 +11,7 @@ const MattermostAccountSchemaBase = z .object({ name: z.string().optional(), capabilities: z.array(z.string()).optional(), + dangerouslyAllowNameMatching: z.boolean().optional(), markdown: MarkdownConfigSchema, enabled: z.boolean().optional(), configWrites: z.boolean().optional(), diff --git a/extensions/mattermost/src/mattermost/monitor.ts b/extensions/mattermost/src/mattermost/monitor.ts index 2ae8388b0fb..3baa129d6fb 100644 --- a/extensions/mattermost/src/mattermost/monitor.ts +++ b/extensions/mattermost/src/mattermost/monitor.ts @@ -152,6 +152,7 @@ function isSenderAllowed(params: { senderId: string; senderName?: string; allowFrom: string[]; + allowNameMatching?: boolean; }): boolean { const allowFrom = params.allowFrom; if (allowFrom.length === 0) { @@ -162,10 +163,15 @@ function isSenderAllowed(params: { } const normalizedSenderId = normalizeAllowEntry(params.senderId); const normalizedSenderName = params.senderName ? normalizeAllowEntry(params.senderName) : ""; - return allowFrom.some( - (entry) => - entry === normalizedSenderId || (normalizedSenderName && entry === normalizedSenderName), - ); + return allowFrom.some((entry) => { + if (entry === normalizedSenderId) { + return true; + } + if (params.allowNameMatching !== true) { + return false; + } + return normalizedSenderName ? entry === normalizedSenderName : false; + }); } type MattermostMediaInfo = { @@ -206,6 +212,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} cfg, accountId: opts.accountId, }); + const allowNameMatching = account.config.dangerouslyAllowNameMatching === true; const botToken = opts.botToken?.trim() || account.botToken?.trim(); if (!botToken) { throw new Error( @@ -416,11 +423,13 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} senderId, senderName, allowFrom: effectiveAllowFrom, + allowNameMatching, }); const groupAllowedForCommands = isSenderAllowed({ senderId, senderName, allowFrom: effectiveGroupAllowFrom, + allowNameMatching, }); const commandGate = resolveControlCommandGate({ useAccessGroups, @@ -892,6 +901,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} senderId: userId, senderName, allowFrom: effectiveAllowFrom, + allowNameMatching, }); if (!allowed) { logVerboseMessage( @@ -927,6 +937,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} senderId: userId, senderName, allowFrom: effectiveGroupAllowFrom, + allowNameMatching, }); if (!allowed) { logVerboseMessage(`mattermost: drop reaction (groupPolicy=allowlist sender=${userId})`); diff --git a/extensions/mattermost/src/types.ts b/extensions/mattermost/src/types.ts index 7501cca3f31..150989b7b44 100644 --- a/extensions/mattermost/src/types.ts +++ b/extensions/mattermost/src/types.ts @@ -7,6 +7,11 @@ export type MattermostAccountConfig = { name?: string; /** Optional provider capability tags used for agent/runtime guidance. */ capabilities?: string[]; + /** + * Break-glass override: allow mutable identity matching (@username/display name) in allowlists. + * Default behavior is ID-only matching. + */ + dangerouslyAllowNameMatching?: boolean; /** Allow channel-initiated config writes (default: true). */ configWrites?: boolean; /** If false, do not start this Mattermost account. Default: true. */ diff --git a/extensions/msteams/src/monitor-handler/message-handler.ts b/extensions/msteams/src/monitor-handler/message-handler.ts index 56f9848dd71..332a354a2fc 100644 --- a/extensions/msteams/src/monitor-handler/message-handler.ts +++ b/extensions/msteams/src/monitor-handler/message-handler.ts @@ -145,10 +145,12 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) { if (dmPolicy !== "open") { const effectiveAllowFrom = [...allowFrom.map((v) => String(v)), ...storedAllowFrom]; + const allowNameMatching = msteamsCfg.dangerouslyAllowNameMatching === true; const allowMatch = resolveMSTeamsAllowlistMatch({ allowFrom: effectiveAllowFrom, senderId, senderName, + allowNameMatching, }); if (!allowMatch.allowed) { @@ -226,10 +228,12 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) { return; } if (effectiveGroupAllowFrom.length > 0) { + const allowNameMatching = msteamsCfg.dangerouslyAllowNameMatching === true; const allowMatch = resolveMSTeamsAllowlistMatch({ allowFrom: effectiveGroupAllowFrom, senderId, senderName, + allowNameMatching, }); if (!allowMatch.allowed) { log.debug?.("dropping group message (not in groupAllowFrom)", { @@ -248,12 +252,14 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) { allowFrom: effectiveDmAllowFrom, senderId, senderName, + allowNameMatching: msteamsCfg?.dangerouslyAllowNameMatching === true, }); const groupAllowedForCommands = isMSTeamsGroupAllowed({ groupPolicy: "allowlist", allowFrom: effectiveGroupAllowFrom, senderId, senderName, + allowNameMatching: msteamsCfg?.dangerouslyAllowNameMatching === true, }); const hasControlCommandInMessage = core.channel.text.hasControlCommand(text, cfg); const commandGate = resolveControlCommandGate({ diff --git a/extensions/msteams/src/policy.ts b/extensions/msteams/src/policy.ts index 6bab808ce91..a3545c0594f 100644 --- a/extensions/msteams/src/policy.ts +++ b/extensions/msteams/src/policy.ts @@ -209,6 +209,7 @@ export function resolveMSTeamsAllowlistMatch(params: { allowFrom: Array; senderId: string; senderName?: string | null; + allowNameMatching?: boolean; }): MSTeamsAllowlistMatch { return resolveAllowlistMatchSimple(params); } @@ -245,6 +246,7 @@ export function isMSTeamsGroupAllowed(params: { allowFrom: Array; senderId: string; senderName?: string | null; + allowNameMatching?: boolean; }): boolean { const { groupPolicy } = params; if (groupPolicy === "disabled") { diff --git a/src/channels/allowlist-match.ts b/src/channels/allowlist-match.ts index 09cc525dad1..74ed2c25931 100644 --- a/src/channels/allowlist-match.ts +++ b/src/channels/allowlist-match.ts @@ -26,6 +26,7 @@ export function resolveAllowlistMatchSimple(params: { allowFrom: Array; senderId: string; senderName?: string | null; + allowNameMatching?: boolean; }): AllowlistMatch<"wildcard" | "id" | "name"> { const allowFrom = params.allowFrom .map((entry) => String(entry).trim().toLowerCase()) @@ -44,7 +45,7 @@ export function resolveAllowlistMatchSimple(params: { } const senderName = params.senderName?.toLowerCase(); - if (senderName && allowFrom.includes(senderName)) { + if (params.allowNameMatching === true && senderName && allowFrom.includes(senderName)) { return { allowed: true, matchKey: senderName, matchSource: "name" }; } diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index cabae3922bf..229d3928655 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -578,6 +578,400 @@ function maybeRepairDiscordNumericIds(cfg: OpenClawConfig): { return { config: next, changes }; } +type MutableAllowlistHit = { + channel: string; + path: string; + entry: string; + dangerousFlagPath: string; +}; + +function collectProviderAccountScopes( + cfg: OpenClawConfig, + provider: string, +): Array<{ prefix: string; account: Record }> { + const scopes: Array<{ prefix: string; account: Record }> = []; + const channels = asObjectRecord(cfg.channels); + if (!channels) { + return scopes; + } + const providerCfg = asObjectRecord(channels[provider]); + if (!providerCfg) { + return scopes; + } + scopes.push({ prefix: `channels.${provider}`, account: providerCfg }); + const accounts = asObjectRecord(providerCfg.accounts); + if (!accounts) { + return scopes; + } + for (const key of Object.keys(accounts)) { + const account = asObjectRecord(accounts[key]); + if (!account) { + continue; + } + scopes.push({ prefix: `channels.${provider}.accounts.${key}`, account }); + } + return scopes; +} + +function isDiscordMutableAllowEntry(raw: string): boolean { + const text = raw.trim(); + if (!text || text === "*") { + return false; + } + const maybeMentionId = text.replace(/^<@!?/, "").replace(/>$/, ""); + if (/^\d+$/.test(maybeMentionId)) { + return false; + } + for (const prefix of ["discord:", "user:", "pk:"]) { + if (!text.startsWith(prefix)) { + continue; + } + return text.slice(prefix.length).trim().length === 0; + } + return true; +} + +function isSlackMutableAllowEntry(raw: string): boolean { + const text = raw.trim(); + if (!text || text === "*") { + return false; + } + const mentionMatch = text.match(/^<@([A-Z0-9]+)>$/i); + if (mentionMatch && /^[A-Z0-9]{8,}$/i.test(mentionMatch[1] ?? "")) { + return false; + } + const withoutPrefix = text.replace(/^(slack|user):/i, "").trim(); + if (/^[UWBCGDT][A-Z0-9]{2,}$/.test(withoutPrefix)) { + return false; + } + if (/^[A-Z0-9]{8,}$/i.test(withoutPrefix)) { + return false; + } + return true; +} + +function isGoogleChatMutableAllowEntry(raw: string): boolean { + const text = raw.trim(); + if (!text || text === "*") { + return false; + } + const withoutPrefix = text.replace(/^(googlechat|google-chat|gchat):/i, "").trim(); + if (!withoutPrefix) { + return false; + } + const withoutUsers = withoutPrefix.replace(/^users\//i, ""); + return withoutUsers.includes("@"); +} + +function isMSTeamsMutableAllowEntry(raw: string): boolean { + const text = raw.trim(); + if (!text || text === "*") { + return false; + } + const withoutPrefix = text.replace(/^(msteams|user):/i, "").trim(); + return /\s/.test(withoutPrefix) || withoutPrefix.includes("@"); +} + +function isMattermostMutableAllowEntry(raw: string): boolean { + const text = raw.trim(); + if (!text || text === "*") { + return false; + } + const normalized = text + .replace(/^(mattermost|user):/i, "") + .replace(/^@/, "") + .trim() + .toLowerCase(); + // Mattermost user IDs are stable 26-char lowercase/number tokens. + if (/^[a-z0-9]{26}$/.test(normalized)) { + return false; + } + return true; +} + +function isIrcMutableAllowEntry(raw: string): boolean { + const text = raw.trim().toLowerCase(); + if (!text || text === "*") { + return false; + } + const normalized = text + .replace(/^irc:/, "") + .replace(/^user:/, "") + .trim(); + return !normalized.includes("!") && !normalized.includes("@"); +} + +function addMutableAllowlistHits(params: { + hits: MutableAllowlistHit[]; + pathLabel: string; + list: unknown; + detector: (entry: string) => boolean; + channel: string; + dangerousFlagPath: string; +}) { + if (!Array.isArray(params.list)) { + return; + } + for (const entry of params.list) { + const text = String(entry).trim(); + if (!text || text === "*") { + continue; + } + if (!params.detector(text)) { + continue; + } + params.hits.push({ + channel: params.channel, + path: params.pathLabel, + entry: text, + dangerousFlagPath: params.dangerousFlagPath, + }); + } +} + +function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[] { + const hits: MutableAllowlistHit[] = []; + + for (const scope of collectProviderAccountScopes(cfg, "discord")) { + const dangerousFlagPath = `${scope.prefix}.dangerouslyAllowNameMatching`; + if (scope.account.dangerouslyAllowNameMatching === true) { + continue; + } + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.allowFrom`, + list: scope.account.allowFrom, + detector: isDiscordMutableAllowEntry, + channel: "discord", + dangerousFlagPath, + }); + const dm = asObjectRecord(scope.account.dm); + if (dm) { + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.dm.allowFrom`, + list: dm.allowFrom, + detector: isDiscordMutableAllowEntry, + channel: "discord", + dangerousFlagPath, + }); + } + const guilds = asObjectRecord(scope.account.guilds); + if (!guilds) { + continue; + } + for (const [guildId, guildRaw] of Object.entries(guilds)) { + const guild = asObjectRecord(guildRaw); + if (!guild) { + continue; + } + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.guilds.${guildId}.users`, + list: guild.users, + detector: isDiscordMutableAllowEntry, + channel: "discord", + dangerousFlagPath, + }); + const channels = asObjectRecord(guild.channels); + if (!channels) { + continue; + } + for (const [channelId, channelRaw] of Object.entries(channels)) { + const channel = asObjectRecord(channelRaw); + if (!channel) { + continue; + } + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.guilds.${guildId}.channels.${channelId}.users`, + list: channel.users, + detector: isDiscordMutableAllowEntry, + channel: "discord", + dangerousFlagPath, + }); + } + } + } + + for (const scope of collectProviderAccountScopes(cfg, "slack")) { + const dangerousFlagPath = `${scope.prefix}.dangerouslyAllowNameMatching`; + if (scope.account.dangerouslyAllowNameMatching === true) { + continue; + } + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.allowFrom`, + list: scope.account.allowFrom, + detector: isSlackMutableAllowEntry, + channel: "slack", + dangerousFlagPath, + }); + const dm = asObjectRecord(scope.account.dm); + if (dm) { + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.dm.allowFrom`, + list: dm.allowFrom, + detector: isSlackMutableAllowEntry, + channel: "slack", + dangerousFlagPath, + }); + } + const channels = asObjectRecord(scope.account.channels); + if (!channels) { + continue; + } + for (const [channelKey, channelRaw] of Object.entries(channels)) { + const channel = asObjectRecord(channelRaw); + if (!channel) { + continue; + } + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.channels.${channelKey}.users`, + list: channel.users, + detector: isSlackMutableAllowEntry, + channel: "slack", + dangerousFlagPath, + }); + } + } + + for (const scope of collectProviderAccountScopes(cfg, "googlechat")) { + const dangerousFlagPath = `${scope.prefix}.dangerouslyAllowNameMatching`; + if (scope.account.dangerouslyAllowNameMatching === true) { + continue; + } + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.groupAllowFrom`, + list: scope.account.groupAllowFrom, + detector: isGoogleChatMutableAllowEntry, + channel: "googlechat", + dangerousFlagPath, + }); + const dm = asObjectRecord(scope.account.dm); + if (dm) { + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.dm.allowFrom`, + list: dm.allowFrom, + detector: isGoogleChatMutableAllowEntry, + channel: "googlechat", + dangerousFlagPath, + }); + } + const groups = asObjectRecord(scope.account.groups); + if (!groups) { + continue; + } + for (const [groupKey, groupRaw] of Object.entries(groups)) { + const group = asObjectRecord(groupRaw); + if (!group) { + continue; + } + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.groups.${groupKey}.users`, + list: group.users, + detector: isGoogleChatMutableAllowEntry, + channel: "googlechat", + dangerousFlagPath, + }); + } + } + + for (const scope of collectProviderAccountScopes(cfg, "msteams")) { + const dangerousFlagPath = `${scope.prefix}.dangerouslyAllowNameMatching`; + if (scope.account.dangerouslyAllowNameMatching === true) { + continue; + } + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.allowFrom`, + list: scope.account.allowFrom, + detector: isMSTeamsMutableAllowEntry, + channel: "msteams", + dangerousFlagPath, + }); + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.groupAllowFrom`, + list: scope.account.groupAllowFrom, + detector: isMSTeamsMutableAllowEntry, + channel: "msteams", + dangerousFlagPath, + }); + } + + for (const scope of collectProviderAccountScopes(cfg, "mattermost")) { + const dangerousFlagPath = `${scope.prefix}.dangerouslyAllowNameMatching`; + if (scope.account.dangerouslyAllowNameMatching === true) { + continue; + } + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.allowFrom`, + list: scope.account.allowFrom, + detector: isMattermostMutableAllowEntry, + channel: "mattermost", + dangerousFlagPath, + }); + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.groupAllowFrom`, + list: scope.account.groupAllowFrom, + detector: isMattermostMutableAllowEntry, + channel: "mattermost", + dangerousFlagPath, + }); + } + + for (const scope of collectProviderAccountScopes(cfg, "irc")) { + const dangerousFlagPath = `${scope.prefix}.dangerouslyAllowNameMatching`; + if (scope.account.dangerouslyAllowNameMatching === true) { + continue; + } + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.allowFrom`, + list: scope.account.allowFrom, + detector: isIrcMutableAllowEntry, + channel: "irc", + dangerousFlagPath, + }); + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.groupAllowFrom`, + list: scope.account.groupAllowFrom, + detector: isIrcMutableAllowEntry, + channel: "irc", + dangerousFlagPath, + }); + const groups = asObjectRecord(scope.account.groups); + if (!groups) { + continue; + } + for (const [groupKey, groupRaw] of Object.entries(groups)) { + const group = asObjectRecord(groupRaw); + if (!group) { + continue; + } + addMutableAllowlistHits({ + hits, + pathLabel: `${scope.prefix}.groups.${groupKey}.allowFrom`, + list: group.allowFrom, + detector: isIrcMutableAllowEntry, + channel: "irc", + dangerousFlagPath, + }); + } + } + + return hits; +} + /** * Scan all channel configs for dmPolicy="open" without allowFrom including "*". * This configuration is rejected by the schema validator but can easily occur when @@ -1209,6 +1603,34 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { } } + const mutableAllowlistHits = scanMutableAllowlistEntries(candidate); + if (mutableAllowlistHits.length > 0) { + const channels = Array.from(new Set(mutableAllowlistHits.map((hit) => hit.channel))).toSorted(); + const exampleLines = mutableAllowlistHits + .slice(0, 8) + .map((hit) => `- ${hit.path}: ${hit.entry}`) + .join("\n"); + const remaining = + mutableAllowlistHits.length > 8 + ? `- +${mutableAllowlistHits.length - 8} more mutable allowlist entries.` + : null; + const flagPaths = Array.from(new Set(mutableAllowlistHits.map((hit) => hit.dangerousFlagPath))); + const flagHint = + flagPaths.length === 1 + ? flagPaths[0] + : `${flagPaths[0]} (and ${flagPaths.length - 1} other scope flags)`; + note( + [ + `- Found ${mutableAllowlistHits.length} mutable allowlist ${mutableAllowlistHits.length === 1 ? "entry" : "entries"} across ${channels.join(", ")} while name matching is disabled by default.`, + exampleLines, + ...(remaining ? [remaining] : []), + `- Option A (break-glass): enable ${flagHint}=true to keep name/email/nick matching.`, + "- Option B (recommended): resolve names/emails/nicks to stable sender IDs and rewrite the allowlist entries.", + ].join("\n"), + "Doctor warnings", + ); + } + const unknown = stripUnknownConfigKeys(candidate); if (unknown.removed.length > 0) { const lines = unknown.removed.map((path) => `- ${path}`).join("\n"); diff --git a/src/config/types.discord.ts b/src/config/types.discord.ts index a5ef6c6465a..0d795c94bb4 100644 --- a/src/config/types.discord.ts +++ b/src/config/types.discord.ts @@ -184,6 +184,11 @@ export type DiscordAccountConfig = { proxy?: string; /** Allow bot-authored messages to trigger replies (default: false). */ allowBots?: boolean; + /** + * Break-glass override: allow mutable identity matching (names/tags/slugs) in allowlists. + * Default behavior is ID-only matching. + */ + dangerouslyAllowNameMatching?: boolean; /** * Controls how guild channel messages are handled: * - "open": guild channels bypass allowlists; mention-gating applies diff --git a/src/config/types.googlechat.ts b/src/config/types.googlechat.ts index 75d7b0224a9..070bf379b3b 100644 --- a/src/config/types.googlechat.ts +++ b/src/config/types.googlechat.ts @@ -43,6 +43,11 @@ export type GoogleChatAccountConfig = { enabled?: boolean; /** Allow bot-authored messages to trigger replies (default: false). */ allowBots?: boolean; + /** + * Break-glass override: allow mutable principal matching (raw email entries) in allowlists. + * Default behavior is ID-only matching. + */ + dangerouslyAllowNameMatching?: boolean; /** Default mention requirement for space messages (default: true). */ requireMention?: boolean; /** diff --git a/src/config/types.msteams.ts b/src/config/types.msteams.ts index 359b9da8be9..94ac8a3696f 100644 --- a/src/config/types.msteams.ts +++ b/src/config/types.msteams.ts @@ -47,6 +47,11 @@ export type MSTeamsConfig = { enabled?: boolean; /** Optional provider capability tags used for agent/runtime guidance. */ capabilities?: string[]; + /** + * Break-glass override: allow mutable identity matching (display names/UPNs) in allowlists. + * Default behavior is ID-only matching. + */ + dangerouslyAllowNameMatching?: boolean; /** Markdown formatting overrides (tables). */ markdown?: MarkdownConfig; /** Allow channel-initiated config writes (default: true). */ diff --git a/src/config/types.slack.ts b/src/config/types.slack.ts index 323906cd311..560a76d141a 100644 --- a/src/config/types.slack.ts +++ b/src/config/types.slack.ts @@ -105,6 +105,11 @@ export type SlackAccountConfig = { userTokenReadOnly?: boolean; /** Allow bot-authored messages to trigger replies (default: false). */ allowBots?: boolean; + /** + * Break-glass override: allow mutable identity matching (name/slug) in allowlists. + * Default behavior is ID-only matching. + */ + dangerouslyAllowNameMatching?: boolean; /** Default mention requirement for channel messages (default: true). */ requireMention?: boolean; /** diff --git a/src/config/zod-schema.providers-core.ts b/src/config/zod-schema.providers-core.ts index 2aaf1de245d..bccbb5bdd35 100644 --- a/src/config/zod-schema.providers-core.ts +++ b/src/config/zod-schema.providers-core.ts @@ -331,6 +331,7 @@ export const DiscordAccountSchema = z token: z.string().optional().register(sensitive), proxy: z.string().optional(), allowBots: z.boolean().optional(), + dangerouslyAllowNameMatching: z.boolean().optional(), groupPolicy: GroupPolicySchema.optional().default("allowlist"), historyLimit: z.number().int().min(0).optional(), dmHistoryLimit: z.number().int().min(0).optional(), @@ -516,6 +517,7 @@ export const GoogleChatAccountSchema = z enabled: z.boolean().optional(), configWrites: z.boolean().optional(), allowBots: z.boolean().optional(), + dangerouslyAllowNameMatching: z.boolean().optional(), requireMention: z.boolean().optional(), groupPolicy: GroupPolicySchema.optional().default("allowlist"), groupAllowFrom: z.array(z.union([z.string(), z.number()])).optional(), @@ -612,6 +614,7 @@ export const SlackAccountSchema = z userToken: z.string().optional().register(sensitive), userTokenReadOnly: z.boolean().optional().default(true), allowBots: z.boolean().optional(), + dangerouslyAllowNameMatching: z.boolean().optional(), requireMention: z.boolean().optional(), groupPolicy: GroupPolicySchema.optional(), historyLimit: z.number().int().min(0).optional(), @@ -1059,6 +1062,7 @@ export const MSTeamsConfigSchema = z .object({ enabled: z.boolean().optional(), capabilities: z.array(z.string()).optional(), + dangerouslyAllowNameMatching: z.boolean().optional(), markdown: MarkdownConfigSchema, configWrites: z.boolean().optional(), appId: z.string().optional(), diff --git a/src/discord/monitor.test.ts b/src/discord/monitor.test.ts index 423cbb74d65..222911894a9 100644 --- a/src/discord/monitor.test.ts +++ b/src/discord/monitor.test.ts @@ -184,7 +184,7 @@ describe("discord allowlist helpers", () => { expect(normalizeDiscordSlug("Dev__Chat")).toBe("dev-chat"); }); - it("matches ids or names", () => { + it("matches ids by default and names only when enabled", () => { const allow = normalizeDiscordAllowList( ["123", "steipete", "Friends of OpenClaw"], ["discord:", "user:", "guild:", "channel:"], @@ -194,8 +194,12 @@ describe("discord allowlist helpers", () => { throw new Error("Expected allow list to be normalized"); } expect(allowListMatches(allow, { id: "123" })).toBe(true); - expect(allowListMatches(allow, { name: "steipete" })).toBe(true); - expect(allowListMatches(allow, { name: "friends-of-openclaw" })).toBe(true); + expect(allowListMatches(allow, { name: "steipete" })).toBe(false); + expect(allowListMatches(allow, { name: "friends-of-openclaw" })).toBe(false); + expect(allowListMatches(allow, { name: "steipete" }, { allowNameMatching: true })).toBe(true); + expect( + allowListMatches(allow, { name: "friends-of-openclaw" }, { allowNameMatching: true }), + ).toBe(true); expect(allowListMatches(allow, { name: "other" })).toBe(false); }); @@ -750,6 +754,31 @@ describe("discord reaction notification gating", () => { }, expected: true, }, + { + name: "allowlist mode does not match usernames by default", + input: { + mode: "allowlist" as const, + botId: "bot-1", + messageAuthorId: "user-1", + userId: "999", + userName: "trusted-user", + allowlist: ["trusted-user"] as string[], + }, + expected: false, + }, + { + name: "allowlist mode matches usernames when explicitly enabled", + input: { + mode: "allowlist" as const, + botId: "bot-1", + messageAuthorId: "user-1", + userId: "999", + userName: "trusted-user", + allowlist: ["trusted-user"] as string[], + allowNameMatching: true, + }, + expected: true, + }, ]); for (const testCase of cases) { @@ -870,6 +899,7 @@ function makeReactionClient(options?: { function makeReactionListenerParams(overrides?: { botUserId?: string; + allowNameMatching?: boolean; guildEntries?: Record; }) { return { @@ -877,6 +907,7 @@ function makeReactionListenerParams(overrides?: { accountId: "acc-1", runtime: {} as import("../runtime.js").RuntimeEnv, botUserId: overrides?.botUserId ?? "bot-1", + allowNameMatching: overrides?.allowNameMatching ?? false, guildEntries: overrides?.guildEntries, logger: { info: vi.fn(), diff --git a/src/discord/monitor/agent-components.ts b/src/discord/monitor/agent-components.ts index 4423e7796e6..112ab78f9ac 100644 --- a/src/discord/monitor/agent-components.ts +++ b/src/discord/monitor/agent-components.ts @@ -237,6 +237,7 @@ async function ensureGuildComponentMemberAllowed(params: { replyOpts: { ephemeral?: boolean }; componentLabel: string; unauthorizedReply: string; + allowNameMatching: boolean; }): Promise { const { interaction, @@ -275,6 +276,7 @@ async function ensureGuildComponentMemberAllowed(params: { name: user.username, tag: user.discriminator ? `${user.username}#${user.discriminator}` : undefined, }, + allowNameMatching: params.allowNameMatching, }); if (memberAllowed) { return true; @@ -299,6 +301,7 @@ async function ensureComponentUserAllowed(params: { replyOpts: { ephemeral?: boolean }; componentLabel: string; unauthorizedReply: string; + allowNameMatching: boolean; }): Promise { const allowList = normalizeDiscordAllowList(params.entry.allowedUsers, [ "discord:", @@ -315,6 +318,7 @@ async function ensureComponentUserAllowed(params: { name: params.user.username, tag: formatDiscordUserTag(params.user), }, + allowNameMatching: params.allowNameMatching, }); if (match.allowed) { return true; @@ -361,6 +365,7 @@ async function ensureAgentComponentInteractionAllowed(params: { replyOpts: params.replyOpts, componentLabel: params.componentLabel, unauthorizedReply: params.unauthorizedReply, + allowNameMatching: params.ctx.discordConfig?.dangerouslyAllowNameMatching === true, }); if (!memberAllowed) { return null; @@ -476,6 +481,7 @@ async function ensureDmComponentAuthorized(params: { name: user.username, tag: formatDiscordUserTag(user), }, + allowNameMatching: ctx.discordConfig?.dangerouslyAllowNameMatching === true, }) : { allowed: false }; if (allowMatch.allowed) { @@ -778,6 +784,7 @@ async function dispatchDiscordComponentEvent(params: { channelConfig, guildInfo, sender: { id: interactionCtx.user.id, name: interactionCtx.user.username, tag: senderTag }, + allowNameMatching: ctx.discordConfig?.dangerouslyAllowNameMatching === true, }); const storePath = resolveStorePath(ctx.cfg.session?.store, { agentId }); const envelopeOptions = resolveEnvelopeFormatOptions(ctx.cfg); @@ -975,6 +982,7 @@ async function handleDiscordComponentEvent(params: { replyOpts, componentLabel: params.componentLabel, unauthorizedReply, + allowNameMatching: params.ctx.discordConfig?.dangerouslyAllowNameMatching === true, }); if (!memberAllowed) { return; @@ -987,6 +995,7 @@ async function handleDiscordComponentEvent(params: { replyOpts, componentLabel: params.componentLabel, unauthorizedReply, + allowNameMatching: params.ctx.discordConfig?.dangerouslyAllowNameMatching === true, }); if (!componentAllowed) { return; @@ -1125,6 +1134,7 @@ async function handleDiscordModalTrigger(params: { replyOpts, componentLabel: "form", unauthorizedReply, + allowNameMatching: params.ctx.discordConfig?.dangerouslyAllowNameMatching === true, }); if (!memberAllowed) { return; @@ -1137,6 +1147,7 @@ async function handleDiscordModalTrigger(params: { replyOpts, componentLabel: "form", unauthorizedReply, + allowNameMatching: params.ctx.discordConfig?.dangerouslyAllowNameMatching === true, }); if (!componentAllowed) { return; @@ -1572,6 +1583,7 @@ class DiscordComponentModal extends Modal { replyOpts, componentLabel: "form", unauthorizedReply: "You are not authorized to use this form.", + allowNameMatching: this.ctx.discordConfig?.dangerouslyAllowNameMatching === true, }); if (!memberAllowed) { return; diff --git a/src/discord/monitor/allow-list.ts b/src/discord/monitor/allow-list.ts index da83cb556f4..c0bff421505 100644 --- a/src/discord/monitor/allow-list.ts +++ b/src/discord/monitor/allow-list.ts @@ -98,6 +98,7 @@ export function normalizeDiscordSlug(value: string) { export function allowListMatches( list: DiscordAllowList, candidate: { id?: string; name?: string; tag?: string }, + params?: { allowNameMatching?: boolean }, ) { if (list.allowAll) { return true; @@ -105,12 +106,14 @@ export function allowListMatches( if (candidate.id && list.ids.has(candidate.id)) { return true; } - const slug = candidate.name ? normalizeDiscordSlug(candidate.name) : ""; - if (slug && list.names.has(slug)) { - return true; - } - if (candidate.tag && list.names.has(normalizeDiscordSlug(candidate.tag))) { - return true; + if (params?.allowNameMatching === true) { + const slug = candidate.name ? normalizeDiscordSlug(candidate.name) : ""; + if (slug && list.names.has(slug)) { + return true; + } + if (candidate.tag && list.names.has(normalizeDiscordSlug(candidate.tag))) { + return true; + } } return false; } @@ -118,6 +121,7 @@ export function allowListMatches( export function resolveDiscordAllowListMatch(params: { allowList: DiscordAllowList; candidate: { id?: string; name?: string; tag?: string }; + allowNameMatching?: boolean; }): DiscordAllowListMatch { const { allowList, candidate } = params; if (allowList.allowAll) { @@ -126,13 +130,15 @@ export function resolveDiscordAllowListMatch(params: { if (candidate.id && allowList.ids.has(candidate.id)) { return { allowed: true, matchKey: candidate.id, matchSource: "id" }; } - const nameSlug = candidate.name ? normalizeDiscordSlug(candidate.name) : ""; - if (nameSlug && allowList.names.has(nameSlug)) { - return { allowed: true, matchKey: nameSlug, matchSource: "name" }; - } - const tagSlug = candidate.tag ? normalizeDiscordSlug(candidate.tag) : ""; - if (tagSlug && allowList.names.has(tagSlug)) { - return { allowed: true, matchKey: tagSlug, matchSource: "tag" }; + if (params.allowNameMatching === true) { + const nameSlug = candidate.name ? normalizeDiscordSlug(candidate.name) : ""; + if (nameSlug && allowList.names.has(nameSlug)) { + return { allowed: true, matchKey: nameSlug, matchSource: "name" }; + } + const tagSlug = candidate.tag ? normalizeDiscordSlug(candidate.tag) : ""; + if (tagSlug && allowList.names.has(tagSlug)) { + return { allowed: true, matchKey: tagSlug, matchSource: "tag" }; + } } return { allowed: false }; } @@ -142,16 +148,21 @@ export function resolveDiscordUserAllowed(params: { userId: string; userName?: string; userTag?: string; + allowNameMatching?: boolean; }) { const allowList = normalizeDiscordAllowList(params.allowList, ["discord:", "user:", "pk:"]); if (!allowList) { return true; } - return allowListMatches(allowList, { - id: params.userId, - name: params.userName, - tag: params.userTag, - }); + return allowListMatches( + allowList, + { + id: params.userId, + name: params.userName, + tag: params.userTag, + }, + { allowNameMatching: params.allowNameMatching }, + ); } export function resolveDiscordRoleAllowed(params: { @@ -176,6 +187,7 @@ export function resolveDiscordMemberAllowed(params: { userId: string; userName?: string; userTag?: string; + allowNameMatching?: boolean; }) { const hasUserRestriction = Array.isArray(params.userAllowList) && params.userAllowList.length > 0; const hasRoleRestriction = Array.isArray(params.roleAllowList) && params.roleAllowList.length > 0; @@ -188,6 +200,7 @@ export function resolveDiscordMemberAllowed(params: { userId: params.userId, userName: params.userName, userTag: params.userTag, + allowNameMatching: params.allowNameMatching, }) : false; const roleOk = hasRoleRestriction @@ -204,6 +217,7 @@ export function resolveDiscordMemberAccessState(params: { guildInfo?: DiscordGuildEntryResolved | null; memberRoleIds: string[]; sender: { id: string; name?: string; tag?: string }; + allowNameMatching?: boolean; }) { const channelUsers = params.channelConfig?.users ?? params.guildInfo?.users; const channelRoles = params.channelConfig?.roles ?? params.guildInfo?.roles; @@ -217,6 +231,7 @@ export function resolveDiscordMemberAccessState(params: { userId: params.sender.id, userName: params.sender.name, userTag: params.sender.tag, + allowNameMatching: params.allowNameMatching, }); return { channelUsers, channelRoles, hasAccessRestrictions, memberAllowed } as const; } @@ -225,6 +240,7 @@ export function resolveDiscordOwnerAllowFrom(params: { channelConfig?: DiscordChannelConfigResolved | null; guildInfo?: DiscordGuildEntryResolved | null; sender: { id: string; name?: string; tag?: string }; + allowNameMatching?: boolean; }): string[] | undefined { const rawAllowList = params.channelConfig?.users ?? params.guildInfo?.users; if (!Array.isArray(rawAllowList) || rawAllowList.length === 0) { @@ -241,6 +257,7 @@ export function resolveDiscordOwnerAllowFrom(params: { name: params.sender.name, tag: params.sender.tag, }, + allowNameMatching: params.allowNameMatching, }); if (!match.allowed || !match.matchKey || match.matchKey === "*") { return undefined; @@ -253,6 +270,7 @@ export function resolveDiscordCommandAuthorized(params: { allowFrom?: string[]; guildInfo?: DiscordGuildEntryResolved | null; author: User; + allowNameMatching?: boolean; }) { if (!params.isDirectMessage) { return true; @@ -261,11 +279,15 @@ export function resolveDiscordCommandAuthorized(params: { if (!allowList) { return true; } - return allowListMatches(allowList, { - id: params.author.id, - name: params.author.username, - tag: formatDiscordUserTag(params.author), - }); + return allowListMatches( + allowList, + { + id: params.author.id, + name: params.author.username, + tag: formatDiscordUserTag(params.author), + }, + { allowNameMatching: params.allowNameMatching }, + ); } export function resolveDiscordGuildEntry(params: { @@ -501,6 +523,7 @@ export function shouldEmitDiscordReactionNotification(params: { userName?: string; userTag?: string; allowlist?: string[]; + allowNameMatching?: boolean; }) { const mode = params.mode ?? "own"; if (mode === "off") { @@ -517,11 +540,15 @@ export function shouldEmitDiscordReactionNotification(params: { if (!list) { return false; } - return allowListMatches(list, { - id: params.userId, - name: params.userName, - tag: params.userTag, - }); + return allowListMatches( + list, + { + id: params.userId, + name: params.userName, + tag: params.userTag, + }, + { allowNameMatching: params.allowNameMatching }, + ); } return false; } diff --git a/src/discord/monitor/listeners.ts b/src/discord/monitor/listeners.ts index 0267a26c11e..9bdc7331224 100644 --- a/src/discord/monitor/listeners.ts +++ b/src/discord/monitor/listeners.ts @@ -37,6 +37,7 @@ type DiscordReactionListenerParams = { accountId: string; runtime: RuntimeEnv; botUserId?: string; + allowNameMatching: boolean; guildEntries?: Record; logger: Logger; }; @@ -178,6 +179,7 @@ async function runDiscordReactionHandler(params: { cfg: params.handlerParams.cfg, accountId: params.handlerParams.accountId, botUserId: params.handlerParams.botUserId, + allowNameMatching: params.handlerParams.allowNameMatching, guildEntries: params.handlerParams.guildEntries, logger: params.handlerParams.logger, }), @@ -191,6 +193,7 @@ async function handleDiscordReactionEvent(params: { cfg: LoadedConfig; accountId: string; botUserId?: string; + allowNameMatching: boolean; guildEntries?: Record; logger: Logger; }) { @@ -292,6 +295,7 @@ async function handleDiscordReactionEvent(params: { userName: user.username, userTag: formatDiscordUserTag(user), allowlist: guildInfo?.users, + allowNameMatching: params.allowNameMatching, }); const emitReactionWithAuthor = (message: { author?: User } | null) => { const { baseText } = resolveReactionBase(); diff --git a/src/discord/monitor/message-handler.preflight.ts b/src/discord/monitor/message-handler.preflight.ts index f343cb58328..67dfc58e1c1 100644 --- a/src/discord/monitor/message-handler.preflight.ts +++ b/src/discord/monitor/message-handler.preflight.ts @@ -190,6 +190,7 @@ export async function preflightDiscordMessage( name: sender.name, tag: sender.tag, }, + allowNameMatching: params.discordConfig?.dangerouslyAllowNameMatching === true, }) : { allowed: false }; const allowMatchMeta = formatAllowlistMatchMeta(allowMatch); @@ -563,6 +564,7 @@ export async function preflightDiscordMessage( guildInfo, memberRoleIds, sender, + allowNameMatching: params.discordConfig?.dangerouslyAllowNameMatching === true, }); if (!isDirectMessage) { @@ -572,11 +574,15 @@ export async function preflightDiscordMessage( "pk:", ]); const ownerOk = ownerAllowList - ? allowListMatches(ownerAllowList, { - id: sender.id, - name: sender.name, - tag: sender.tag, - }) + ? allowListMatches( + ownerAllowList, + { + id: sender.id, + name: sender.name, + tag: sender.tag, + }, + { allowNameMatching: params.discordConfig?.dangerouslyAllowNameMatching === true }, + ) : false; const useAccessGroups = params.cfg.commands?.useAccessGroups !== false; const commandGate = resolveControlCommandGate({ diff --git a/src/discord/monitor/message-handler.process.ts b/src/discord/monitor/message-handler.process.ts index 80a63fdf49c..8c0530b8135 100644 --- a/src/discord/monitor/message-handler.process.ts +++ b/src/discord/monitor/message-handler.process.ts @@ -199,6 +199,7 @@ export async function processDiscordMessage(ctx: DiscordMessagePreflightContext) channelConfig, guildInfo, sender: { id: sender.id, name: sender.name, tag: sender.tag }, + allowNameMatching: discordConfig?.dangerouslyAllowNameMatching === true, }); const storePath = resolveStorePath(cfg.session?.store, { agentId: route.agentId, diff --git a/src/discord/monitor/monitor.test.ts b/src/discord/monitor/monitor.test.ts index a834d3c7392..18fdce2e786 100644 --- a/src/discord/monitor/monitor.test.ts +++ b/src/discord/monitor/monitor.test.ts @@ -170,6 +170,7 @@ describe("agent components", () => { const select = createAgentSelectMenu({ cfg: createCfg(), accountId: "default", + discordConfig: { dangerouslyAllowNameMatching: true } as DiscordAccountConfig, dmPolicy: "allowlist", allowFrom: ["Alice#1234"], }); @@ -426,13 +427,20 @@ describe("resolveDiscordOwnerAllowFrom", () => { expect(result).toEqual(["123"]); }); - it("returns the normalized name slug for name matches", () => { - const result = resolveDiscordOwnerAllowFrom({ + it("returns the normalized name slug for name matches only when enabled", () => { + const defaultResult = resolveDiscordOwnerAllowFrom({ channelConfig: { allowed: true, users: ["Some User"] } as DiscordChannelConfigResolved, sender: { id: "999", name: "Some User" }, }); + expect(defaultResult).toBeUndefined(); - expect(result).toEqual(["some-user"]); + const enabledResult = resolveDiscordOwnerAllowFrom({ + channelConfig: { allowed: true, users: ["Some User"] } as DiscordChannelConfigResolved, + sender: { id: "999", name: "Some User" }, + allowNameMatching: true, + }); + + expect(enabledResult).toEqual(["some-user"]); }); }); diff --git a/src/discord/monitor/native-command.ts b/src/discord/monitor/native-command.ts index adad1be709f..fc2fdee2fb6 100644 --- a/src/discord/monitor/native-command.ts +++ b/src/discord/monitor/native-command.ts @@ -1276,11 +1276,15 @@ async function dispatchDiscordCommandInteraction(params: { ); const ownerOk = ownerAllowList && user - ? allowListMatches(ownerAllowList, { - id: sender.id, - name: sender.name, - tag: sender.tag, - }) + ? allowListMatches( + ownerAllowList, + { + id: sender.id, + name: sender.name, + tag: sender.tag, + }, + { allowNameMatching: discordConfig?.dangerouslyAllowNameMatching === true }, + ) : false; const guildInfo = resolveDiscordGuildEntry({ guild: interaction.guild ?? undefined, @@ -1363,11 +1367,15 @@ async function dispatchDiscordCommandInteraction(params: { ]; const allowList = normalizeDiscordAllowList(effectiveAllowFrom, ["discord:", "user:", "pk:"]); const permitted = allowList - ? allowListMatches(allowList, { - id: sender.id, - name: sender.name, - tag: sender.tag, - }) + ? allowListMatches( + allowList, + { + id: sender.id, + name: sender.name, + tag: sender.tag, + }, + { allowNameMatching: discordConfig?.dangerouslyAllowNameMatching === true }, + ) : false; if (!permitted) { commandAuthorized = false; @@ -1404,6 +1412,7 @@ async function dispatchDiscordCommandInteraction(params: { guildInfo, memberRoleIds, sender, + allowNameMatching: discordConfig?.dangerouslyAllowNameMatching === true, }); const authorizers = useAccessGroups ? [ @@ -1509,6 +1518,7 @@ async function dispatchDiscordCommandInteraction(params: { channelConfig, guildInfo, sender: { id: sender.id, name: sender.name, tag: sender.tag }, + allowNameMatching: discordConfig?.dangerouslyAllowNameMatching === true, }); const ctxPayload = finalizeInboundContext({ Body: prompt, diff --git a/src/discord/monitor/provider.ts b/src/discord/monitor/provider.ts index 8f3d5d7ac73..3d72677b465 100644 --- a/src/discord/monitor/provider.ts +++ b/src/discord/monitor/provider.ts @@ -559,6 +559,7 @@ export async function monitorDiscordProvider(opts: MonitorDiscordOpts = {}) { accountId: account.accountId, runtime, botUserId, + allowNameMatching: discordCfg.dangerouslyAllowNameMatching === true, guildEntries, logger, }), @@ -570,6 +571,7 @@ export async function monitorDiscordProvider(opts: MonitorDiscordOpts = {}) { accountId: account.accountId, runtime, botUserId, + allowNameMatching: discordCfg.dangerouslyAllowNameMatching === true, guildEntries, logger, }), diff --git a/src/discord/voice/command.ts b/src/discord/voice/command.ts index 7731b903d0e..831d7e42e91 100644 --- a/src/discord/voice/command.ts +++ b/src/discord/voice/command.ts @@ -156,6 +156,7 @@ async function authorizeVoiceCommand( guildInfo, memberRoleIds, sender, + allowNameMatching: params.discordConfig?.dangerouslyAllowNameMatching === true, }); const ownerAllowList = normalizeDiscordAllowList( @@ -163,11 +164,15 @@ async function authorizeVoiceCommand( ["discord:", "user:", "pk:"], ); const ownerOk = ownerAllowList - ? allowListMatches(ownerAllowList, { - id: sender.id, - name: sender.name, - tag: sender.tag, - }) + ? allowListMatches( + ownerAllowList, + { + id: sender.id, + name: sender.name, + tag: sender.tag, + }, + { allowNameMatching: params.discordConfig?.dangerouslyAllowNameMatching === true }, + ) : false; const authorizers = params.useAccessGroups diff --git a/src/security/audit-channel.ts b/src/security/audit-channel.ts index 05ff4616b31..f403f059ec1 100644 --- a/src/security/audit-channel.ts +++ b/src/security/audit-channel.ts @@ -178,10 +178,25 @@ export async function collectChannelSecurityFindings(params: { continue; } + const accountConfig = (account as { config?: Record } | null | undefined) + ?.config; + if (accountConfig?.dangerouslyAllowNameMatching === true) { + findings.push({ + checkId: `channels.${plugin.id}.allowFrom.dangerous_name_matching_enabled`, + severity: "info", + title: `${plugin.meta.label ?? plugin.id} dangerous name matching is enabled`, + detail: + "dangerouslyAllowNameMatching=true re-enables mutable name/email/tag matching for sender authorization. This is a break-glass compatibility mode, not a hardened default.", + remediation: + "Prefer stable sender IDs in allowlists, then disable dangerouslyAllowNameMatching.", + }); + } + if (plugin.id === "discord") { const discordCfg = (account as { config?: Record } | null)?.config ?? ({} as Record); + const dangerousNameMatchingEnabled = discordCfg.dangerouslyAllowNameMatching === true; const storeAllowFrom = await readChannelAllowFromStore("discord").catch(() => []); const discordNameBasedAllowEntries = new Set(); addDiscordNameBasedEntries({ @@ -236,13 +251,18 @@ export async function collectChannelSecurityFindings(params: { : ""; findings.push({ checkId: "channels.discord.allowFrom.name_based_entries", - severity: "warn", - title: "Discord allowlist contains name or tag entries", - detail: - "Discord name/tag allowlist matching uses normalized slugs and can collide across users. " + - `Found: ${examples.join(", ")}${more}.`, - remediation: - "Prefer stable Discord IDs (or <@id>/user:/pk:) in channels.discord.allowFrom and channels.discord.guilds.*.users.", + severity: dangerousNameMatchingEnabled ? "info" : "warn", + title: dangerousNameMatchingEnabled + ? "Discord allowlist uses break-glass name/tag matching" + : "Discord allowlist contains name or tag entries", + detail: dangerousNameMatchingEnabled + ? "Discord name/tag allowlist matching is explicitly enabled via dangerouslyAllowNameMatching. This mutable-identity mode is operator-selected break-glass behavior and out-of-scope for vulnerability reports by itself. " + + `Found: ${examples.join(", ")}${more}.` + : "Discord name/tag allowlist matching uses normalized slugs and can collide across users. " + + `Found: ${examples.join(", ")}${more}.`, + remediation: dangerousNameMatchingEnabled + ? "Prefer stable Discord IDs (or <@id>/user:/pk:), then disable dangerouslyAllowNameMatching." + : "Prefer stable Discord IDs (or <@id>/user:/pk:) in channels.discord.allowFrom and channels.discord.guilds.*.users, or explicitly opt in with dangerouslyAllowNameMatching=true if you accept the risk.", }); } const nativeEnabled = resolveNativeCommandsEnabled({ diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index 03af14cf86d..d5ae8a97762 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -1500,6 +1500,43 @@ describe("security audit", () => { }); }); + it("marks Discord name-based allowlists as break-glass when dangerous matching is enabled", async () => { + await withChannelSecurityStateDir(async () => { + const cfg: OpenClawConfig = { + channels: { + discord: { + enabled: true, + token: "t", + dangerouslyAllowNameMatching: true, + allowFrom: ["Alice#1234"], + }, + }, + }; + + const res = await runSecurityAudit({ + config: cfg, + includeFilesystem: false, + includeChannelSecurity: true, + plugins: [discordPlugin], + }); + + const finding = res.findings.find( + (entry) => entry.checkId === "channels.discord.allowFrom.name_based_entries", + ); + expect(finding).toBeDefined(); + expect(finding?.severity).toBe("info"); + expect(finding?.detail).toContain("out-of-scope"); + expect(res.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + checkId: "channels.discord.allowFrom.dangerous_name_matching_enabled", + severity: "info", + }), + ]), + ); + }); + }); + it("does not warn when Discord allowlists use ID-style entries only", async () => { await withChannelSecurityStateDir(async () => { const cfg: OpenClawConfig = { diff --git a/src/slack/monitor/allow-list.test.ts b/src/slack/monitor/allow-list.test.ts index dc37f318680..d6fdb7d9452 100644 --- a/src/slack/monitor/allow-list.test.ts +++ b/src/slack/monitor/allow-list.test.ts @@ -15,7 +15,7 @@ describe("slack/allow-list", () => { expect(normalizeSlackSlug(" #Ops.Room ")).toBe("#ops.room"); }); - it("matches wildcard, id, and prefixed name candidates", () => { + it("matches wildcard and id candidates by default", () => { expect(resolveSlackAllowListMatch({ allowList: ["*"], id: "u1", name: "alice" })).toEqual({ allowed: true, matchKey: "*", @@ -40,6 +40,15 @@ describe("slack/allow-list", () => { id: "u2", name: "alice", }), + ).toEqual({ allowed: false }); + + expect( + resolveSlackAllowListMatch({ + allowList: ["slack:alice"], + id: "u2", + name: "alice", + allowNameMatching: true, + }), ).toEqual({ allowed: true, matchKey: "slack:alice", diff --git a/src/slack/monitor/allow-list.ts b/src/slack/monitor/allow-list.ts index 356d95d3d0a..34aa9ed3914 100644 --- a/src/slack/monitor/allow-list.ts +++ b/src/slack/monitor/allow-list.ts @@ -25,6 +25,7 @@ export function resolveSlackAllowListMatch(params: { allowList: string[]; id?: string; name?: string; + allowNameMatching?: boolean; }): SlackAllowListMatch { const allowList = params.allowList; if (allowList.length === 0) { @@ -40,9 +41,13 @@ export function resolveSlackAllowListMatch(params: { { value: id, source: "id" }, { value: id ? `slack:${id}` : undefined, source: "prefixed-id" }, { value: id ? `user:${id}` : undefined, source: "prefixed-user" }, - { value: name, source: "name" }, - { value: name ? `slack:${name}` : undefined, source: "prefixed-name" }, - { value: slug, source: "slug" }, + ...(params.allowNameMatching === true + ? ([ + { value: name, source: "name" as const }, + { value: name ? `slack:${name}` : undefined, source: "prefixed-name" as const }, + { value: slug, source: "slug" as const }, + ] satisfies Array<{ value?: string; source: SlackAllowListMatch["matchSource"] }>) + : []), ]; for (const candidate of candidates) { if (!candidate.value) { @@ -59,7 +64,12 @@ export function resolveSlackAllowListMatch(params: { return { allowed: false }; } -export function allowListMatches(params: { allowList: string[]; id?: string; name?: string }) { +export function allowListMatches(params: { + allowList: string[]; + id?: string; + name?: string; + allowNameMatching?: boolean; +}) { return resolveSlackAllowListMatch(params).allowed; } @@ -67,6 +77,7 @@ export function resolveSlackUserAllowed(params: { allowList?: Array; userId?: string; userName?: string; + allowNameMatching?: boolean; }) { const allowList = normalizeAllowListLower(params.allowList); if (allowList.length === 0) { @@ -76,5 +87,6 @@ export function resolveSlackUserAllowed(params: { allowList, id: params.userId, name: params.userName, + allowNameMatching: params.allowNameMatching, }); } diff --git a/src/slack/monitor/auth.ts b/src/slack/monitor/auth.ts index 9b050f5a654..d8fa5e5b4e5 100644 --- a/src/slack/monitor/auth.ts +++ b/src/slack/monitor/auth.ts @@ -14,14 +14,16 @@ export function isSlackSenderAllowListed(params: { allowListLower: string[]; senderId: string; senderName?: string; + allowNameMatching?: boolean; }) { - const { allowListLower, senderId, senderName } = params; + const { allowListLower, senderId, senderName, allowNameMatching } = params; return ( allowListLower.length === 0 || allowListMatches({ allowList: allowListLower, id: senderId, name: senderName, + allowNameMatching, }) ); } diff --git a/src/slack/monitor/channel-config.ts b/src/slack/monitor/channel-config.ts index 7e2c6cb4c18..15ba7c3b146 100644 --- a/src/slack/monitor/channel-config.ts +++ b/src/slack/monitor/channel-config.ts @@ -47,6 +47,7 @@ export function shouldEmitSlackReactionNotification(params: { userId: string; userName?: string | null; allowlist?: Array | null; + allowNameMatching?: boolean; }) { const { mode, botId, messageAuthorId, userId, userName, allowlist } = params; const effectiveMode = mode ?? "own"; @@ -68,6 +69,7 @@ export function shouldEmitSlackReactionNotification(params: { allowList: users, id: userId, name: userName ?? undefined, + allowNameMatching: params.allowNameMatching, }); } return true; diff --git a/src/slack/monitor/context.ts b/src/slack/monitor/context.ts index 70dd8a80853..ecf04974937 100644 --- a/src/slack/monitor/context.ts +++ b/src/slack/monitor/context.ts @@ -68,6 +68,7 @@ export type SlackMonitorContext = { dmEnabled: boolean; dmPolicy: DmPolicy; allowFrom: string[]; + allowNameMatching: boolean; groupDmEnabled: boolean; groupDmChannels: string[]; defaultRequireMention: boolean; @@ -129,6 +130,7 @@ export function createSlackMonitorContext(params: { dmEnabled: boolean; dmPolicy: DmPolicy; allowFrom: Array | undefined; + allowNameMatching: boolean; groupDmEnabled: boolean; groupDmChannels: Array | undefined; defaultRequireMention?: boolean; @@ -391,6 +393,7 @@ export function createSlackMonitorContext(params: { dmEnabled: params.dmEnabled, dmPolicy: params.dmPolicy, allowFrom, + allowNameMatching: params.allowNameMatching, groupDmEnabled: params.groupDmEnabled, groupDmChannels, defaultRequireMention, diff --git a/src/slack/monitor/message-handler/prepare.test.ts b/src/slack/monitor/message-handler/prepare.test.ts index a4feb2e4b2e..07e6b834501 100644 --- a/src/slack/monitor/message-handler/prepare.test.ts +++ b/src/slack/monitor/message-handler/prepare.test.ts @@ -60,6 +60,7 @@ describe("slack prepareSlackMessage inbound contract", () => { dmEnabled: true, dmPolicy: "open", allowFrom: [], + allowNameMatching: false, groupDmEnabled: true, groupDmChannels: [], defaultRequireMention: params.defaultRequireMention ?? true, diff --git a/src/slack/monitor/message-handler/prepare.ts b/src/slack/monitor/message-handler/prepare.ts index c94ba9bbb70..39515ad621d 100644 --- a/src/slack/monitor/message-handler/prepare.ts +++ b/src/slack/monitor/message-handler/prepare.ts @@ -142,6 +142,7 @@ export async function prepareSlackMessage(params: { const allowMatch = resolveSlackAllowListMatch({ allowList: allowFromLower, id: directUserId, + allowNameMatching: ctx.allowNameMatching, }); const allowMatchMeta = formatAllowlistMatchMeta(allowMatch); if (!allowMatch.allowed) { @@ -244,6 +245,7 @@ export async function prepareSlackMessage(params: { allowList: channelConfig?.users, userId: senderId, userName: senderName, + allowNameMatching: ctx.allowNameMatching, }) : true; if (isRoom && !channelUserAuthorized) { @@ -263,6 +265,7 @@ export async function prepareSlackMessage(params: { allowList: allowFromLower, id: senderId, name: senderName, + allowNameMatching: ctx.allowNameMatching, }).allowed; const channelUsersAllowlistConfigured = isRoom && Array.isArray(channelConfig?.users) && channelConfig.users.length > 0; @@ -272,6 +275,7 @@ export async function prepareSlackMessage(params: { allowList: channelConfig?.users, userId: senderId, userName: senderName, + allowNameMatching: ctx.allowNameMatching, }) : false; const commandGate = resolveControlCommandGate({ diff --git a/src/slack/monitor/monitor.test.ts b/src/slack/monitor/monitor.test.ts index 399b9cc563f..2a6072d93dd 100644 --- a/src/slack/monitor/monitor.test.ts +++ b/src/slack/monitor/monitor.test.ts @@ -77,6 +77,7 @@ const baseParams = () => ({ dmEnabled: true, dmPolicy: "open" as const, allowFrom: [], + allowNameMatching: false, groupDmEnabled: true, groupDmChannels: [], defaultRequireMention: true, diff --git a/src/slack/monitor/provider.ts b/src/slack/monitor/provider.ts index 19eab0d18c4..dcec6074dea 100644 --- a/src/slack/monitor/provider.ts +++ b/src/slack/monitor/provider.ts @@ -210,6 +210,7 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { dmEnabled, dmPolicy, allowFrom, + allowNameMatching: slackCfg.dangerouslyAllowNameMatching === true, groupDmEnabled, groupDmChannels, defaultRequireMention: slackCfg.requireMention, diff --git a/src/slack/monitor/slash.ts b/src/slack/monitor/slash.ts index fa3dd66c477..92afc734a91 100644 --- a/src/slack/monitor/slash.ts +++ b/src/slack/monitor/slash.ts @@ -361,6 +361,7 @@ export async function registerSlackMonitorSlashCommands(params: { allowList: effectiveAllowFromLower, id: command.user_id, name: senderName, + allowNameMatching: ctx.allowNameMatching, }); const allowMatchMeta = formatAllowlistMatchMeta(allowMatch); if (!allowMatch.allowed) { @@ -446,6 +447,7 @@ export async function registerSlackMonitorSlashCommands(params: { allowList: channelConfig?.users, userId: command.user_id, userName: senderName, + allowNameMatching: ctx.allowNameMatching, }) : false; if (channelUsersAllowlistConfigured && !channelUserAllowed) { @@ -460,6 +462,7 @@ export async function registerSlackMonitorSlashCommands(params: { allowList: effectiveAllowFromLower, id: command.user_id, name: senderName, + allowNameMatching: ctx.allowNameMatching, }).allowed; // DMs: allow chatting in dmPolicy=open, but keep privileged command gating intact by setting // CommandAuthorized based on allowlists/access-groups (downstream decides which commands need it).