diff --git a/docs/channels/slack.md b/docs/channels/slack.md index 05573d2843df..92e2c43cbcd2 100644 --- a/docs/channels/slack.md +++ b/docs/channels/slack.md @@ -268,13 +268,17 @@ the enterprise account with the same Request URL path: allowFrom: ["*"], groupPolicy: "allowlist", channels: { - C0123456789: { requireMention: true }, + "team:T0123456789:channel:C0123456789": { requireMention: true }, }, }, }, } ``` +For each selected workspace, open it in Slack's web app and copy the `T...` +workspace ID from `https://app.slack.com/client/T.../...`. Use that workspace ID +with the channel's `C...` ID in every qualified policy key, as shown above. + At startup, OpenClaw uses Slack `auth.test` to detect whether the token belongs to a workspace installation or an Enterprise Grid org-wide installation. No installation-mode setting is required. Slack remains the source of truth for @@ -324,14 +328,18 @@ validated listener-owned client remains in the active event turn. The in-memory send queue and thread-participation records are partitioned by that event's workspace; the client itself is never serialized or persisted. -Channel policy keys accept raw stable Slack channel IDs, `channel:`, or the -`"*"` wildcard. `dm.groupChannels` accepts raw stable channel IDs or -`channel:`, but not `"*"`. OpenClaw normalizes the ID forms to the raw -channel ID for runtime matching; the channel prefixes `slack:`, `group:`, and -`mpim:` fail startup. +Enterprise channel policy keys must use +`team::channel:` or the `"*"` wildcard. +`dm.groupChannels` requires the workspace-qualified form and does not accept +`"*"`. A delivered Enterprise event never falls back from its qualified +workspace and channel identity to a bare channel ID. Workspace installations +retain raw stable channel IDs and `channel:` compatibility. The channel +prefixes `slack:`, `group:`, and `mpim:` fail startup. -User policy entries in `allowFrom`, `reactionAllowlist`, and per-channel `users` -accept raw stable Slack user IDs, `slack:`, `user:`, or `"*"`. +Enterprise user policy entries in `allowFrom`, `reactionAllowlist`, and +per-channel `users` must use `team::user:` or `"*"`. A +workspace-scoped sender never matches a bare user ID. Workspace installations +retain raw stable user IDs, `slack:`, and `user:` compatibility. Enterprise `toolsBySender` keys accept raw stable user IDs, `id:`, `channel:slack:`, or `"*"`. Names, slugs, display names, and email addresses fail startup. IDs must use Slack's canonical uppercase prefix and body @@ -351,8 +359,9 @@ rejected before authorization or system-event handling. Enterprise DMs support the same `disabled`, `open`, `allowlist`, and `pairing` policies as workspace installs. Pairing approvals are stored as `team::user:` and are applied only to events from that -workspace. Explicit account `allowFrom` entries remain organization-wide; -channel and sender policy continues to apply to channel messages. +workspace. Explicit account `allowFrom` entries use the same qualified form and +apply only to that workspace; channel and sender policy continues to apply to +channel messages. ## Install @@ -1321,7 +1330,7 @@ Current Slack message actions include `send`, `upload-file`, `download-file`, `r - `allowlist` - `disabled` - Channel allowlist lives under `channels.slack.channels` and **must use stable Slack channel IDs** (for example `C12345678`) as config keys. + Channel allowlist lives under `channels.slack.channels` and **must use stable Slack channel IDs** (for example `C12345678`) as config keys. Enterprise Grid org installs require `team::channel:` so policies cannot cross workspace boundaries. Runtime note: if `channels.slack` is completely missing (env-only setup), runtime falls back to `groupPolicy="allowlist"` and logs a warning (even if `channels.defaults.groupPolicy` is set). @@ -1954,7 +1963,7 @@ Primary reference: [Configuration reference - Slack](/gateway/config-channels#sl Check, in order: - `groupPolicy` - - channel allowlist (`channels.slack.channels`) — **keys must be channel IDs** (`C12345678`), not names (`#channel-name`). Name-based keys silently fail under `groupPolicy: "allowlist"` because channel routing is ID-first by default. To find an ID: right-click the channel in Slack → **Copy link** — the `C...` value at the end of the URL is the channel ID. + - channel allowlist (`channels.slack.channels`) — **keys must be channel IDs** (`C12345678`) or workspace-qualified channel targets (`team::channel:`), not names (`#channel-name`). Name-based keys silently fail under `groupPolicy: "allowlist"` because channel routing is ID-first by default. To find an ID: right-click the channel in Slack → **Copy link** — the `C...` value at the end of the URL is the channel ID. - `requireMention` - per-channel `users` allowlist - `messages.groupChat.visibleReplies`: normal group/channel requests default to `"automatic"`. If you opted into `"message_tool"` and logs show assistant text with no `message(action=send)` call, the model missed the visible message-tool path. Final text stays private in this mode; inspect the gateway verbose log for suppressed payload metadata, or set it to `"automatic"` if you want every normal assistant final reply posted through the legacy path. diff --git a/docs/gateway/config-channels.md b/docs/gateway/config-channels.md index ca7d27163026..722895ce4cee 100644 --- a/docs/gateway/config-channels.md +++ b/docs/gateway/config-channels.md @@ -493,9 +493,11 @@ WhatsApp runs through the gateway's web channel (Baileys Web). It starts automat - Slack detects Enterprise Grid org-wide installations automatically from the bot token with `auth.test`; no installation-mode setting is required. Enterprise DMs support `disabled`, `open`, `allowlist`, and workspace-scoped - `pairing`. Channel and user policies must use stable Slack IDs; mutable names - and unsupported channel prefixes fail startup. Mention-pattern channel - scopes and static route-binding peers use workspace-qualified Slack targets. + `pairing`. Channel and user policies must use + `team::channel:` or `team::user:`; + bare IDs, mutable names, and unsupported channel prefixes fail startup. + Mention-pattern channel scopes and static route-binding peers use + workspace-qualified Slack targets. Direct Socket Mode or HTTP messages, mentions, workspace-qualified actions, deferred delivery, proactive sends, supported event listeners and interactions, static route bindings, and Slack-native approvals from diff --git a/extensions/slack/src/action-runtime.ts b/extensions/slack/src/action-runtime.ts index 387cee5112f5..e6cb787549df 100644 --- a/extensions/slack/src/action-runtime.ts +++ b/extensions/slack/src/action-runtime.ts @@ -11,6 +11,8 @@ import type { ResolvedSlackAccount } from "./accounts.js"; import { parseSlackBlocksInput } from "./blocks-input.js"; import type { SlackConversationInfo } from "./channel-type.js"; import { assertSlackDetachedTargetAllowed } from "./detached-target-admission.js"; +import { buildSlackChannelIdCandidates } from "./group-policy.js"; +import { getSlackInstallationKind } from "./installation-identity-state.js"; import { SLACK_TEXT_LIMIT } from "./limits.js"; import { resolveSlackChannelConfig } from "./monitor/channel-config.js"; import { isSlackChannelAllowedByPolicy } from "./monitor/policy.js"; @@ -282,6 +284,7 @@ function resolveSlackChannelReadPolicy(params: { account: ResolvedSlackAccount; cfg: OpenClawConfig; channelId: string; + teamId?: string; channelName?: string; conversationReadOrigin?: ConversationReadInvocationOrigin; metadataResolved?: boolean; @@ -290,6 +293,8 @@ function resolveSlackChannelReadPolicy(params: { const channels = params.account.config.channels; const channelKeys = Object.keys(channels ?? {}); const channelConfig = resolveSlackChannelConfig({ + teamId: params.teamId, + allowUnscoped: getSlackInstallationKind(params.account.accountId) !== "enterprise", channelId: params.channelId, channelName: params.channelName, channels, @@ -344,7 +349,7 @@ function resolveSlackChannelReadPolicy(params: { params.account.config.dm?.enabled !== false && params.account.config.dm?.groupEnabled === true && (params.currentConversation || - isSlackGroupDmTargetConfigured(params.account, params.channelId)), + isSlackGroupDmTargetConfigured(params.account, params.channelId, params.teamId)), shouldResolveName, }; } @@ -461,16 +466,26 @@ async function assertSlackReadTargetAllowed(params: { } } -function isSlackGroupDmTargetConfigured(account: ResolvedSlackAccount, channelId: string): boolean { +function isSlackGroupDmTargetConfigured( + account: ResolvedSlackAccount, + channelId: string, + teamId?: string, +): boolean { const entries = account.config.dm?.groupChannels ?? []; if (entries.length === 0) { return true; } + const candidates = new Set( + buildSlackChannelIdCandidates(channelId, teamId, { + allowUnscoped: getSlackInstallationKind(account.accountId) !== "enterprise", + }).map((candidate) => candidate.toLowerCase()), + ); const target = channelId.trim().toLowerCase(); return entries.some((entry) => { const candidate = String(entry).trim().toLowerCase(); return ( candidate === "*" || + candidates.has(candidate) || candidate === target || candidate === `slack:${target}` || candidate === `channel:${target}` || diff --git a/extensions/slack/src/doctor.test.ts b/extensions/slack/src/doctor.test.ts index 1b8f6615f490..2080b49bf8de 100644 --- a/extensions/slack/src/doctor.test.ts +++ b/extensions/slack/src/doctor.test.ts @@ -174,6 +174,19 @@ describe("slack doctor", () => { ).toBe(true); }); + it("accepts workspace-qualified channel and user ids as stable policy entries", async () => { + const warnings = await collectSlackWarnings({ + allowFrom: ["team:T11111111:user:U01234567"], + channels: { + "team:T11111111:channel:C01234567": { + users: ["team:T11111111:user:U01234567"], + }, + }, + }); + + expect(warnings).toEqual([]); + }); + it("warns for name-keyed allowlist channels but accepts routed ID forms (#81665)", async () => { const warnings = await collectSlackWarnings({ channels: { @@ -183,9 +196,11 @@ describe("slack doctor", () => { c0al2gdua7k: {}, "channel:C0AL2GDUA7L": {}, "channel:c0al2gdua7m": {}, + "team:T11111111:channel:C0AL2GDUA7S": {}, D0AL2GDUA7Q: {}, "channel:d0al2gdua7r": {}, "channel:dabcdefgh": {}, + "team:T11111111:channel:D0AL2GDUA7T": {}, "channel:customers": {}, "CHANNEL:C0AL2GDUA7N": {}, "channel:C0al2gdua7p": {}, @@ -208,10 +223,11 @@ describe("slack doctor", () => { const dmWarnings = warnings.filter((warning) => warning.includes("is a Slack DM conversation ID"), ); - expect(dmWarnings).toHaveLength(3); + expect(dmWarnings).toHaveLength(4); expect(dmWarnings[0]).toContain('channels.slack.channels."D0AL2GDUA7Q"'); expect(dmWarnings[1]).toContain('channels.slack.channels."channel:d0al2gdua7r"'); expect(dmWarnings[2]).toContain('channels.slack.channels."channel:dabcdefgh"'); + expect(dmWarnings[3]).toContain('channels.slack.channels."team:T11111111:channel:D0AL2GDUA7T"'); expect(dmWarnings[0]).toContain("channels.slack.dmPolicy"); }); diff --git a/extensions/slack/src/doctor.ts b/extensions/slack/src/doctor.ts index a4dd71fe939f..122ea2990e32 100644 --- a/extensions/slack/src/doctor.ts +++ b/extensions/slack/src/doctor.ts @@ -14,6 +14,7 @@ import { } from "./doctor-contract.js"; import { probeSlack } from "./probe.js"; import { isSlackMutableAllowEntry } from "./security-doctor.js"; +import { parseSlackTarget } from "./target-parsing.js"; const collectSlackMutableAllowlistWarnings = createDangerousNameMatchingMutableAllowlistWarningCollector({ @@ -45,7 +46,9 @@ const SLACK_CHANNEL_NAME_RE = /^[\p{L}\p{M}\p{N}_-]{1,80}$/u; const SLACK_CHANNEL_NAME_ALPHANUMERIC_RE = /[\p{L}\p{N}]/u; function looksLikeSlackChannelId(channelKey: string): boolean { + const workspaceChannelId = parseWorkspaceQualifiedChannelId(channelKey); return ( + (workspaceChannelId !== undefined && /^[CG]/i.test(workspaceChannelId)) || SLACK_CANONICAL_CHANNEL_ID_RE.test(channelKey) || SLACK_LOWERCASE_CHANNEL_ID_RE.test(channelKey) || SLACK_PREFIXED_CANONICAL_CHANNEL_ID_RE.test(channelKey) || @@ -54,11 +57,26 @@ function looksLikeSlackChannelId(channelKey: string): boolean { } function looksLikeSlackDmId(channelKey: string): boolean { + const workspaceChannelId = parseWorkspaceQualifiedChannelId(channelKey); return ( - SLACK_CANONICAL_DM_ID_RE.test(channelKey) || SLACK_PREFIXED_LOWERCASE_DM_ID_RE.test(channelKey) + (workspaceChannelId !== undefined && /^D/i.test(workspaceChannelId)) || + SLACK_CANONICAL_DM_ID_RE.test(channelKey) || + SLACK_PREFIXED_LOWERCASE_DM_ID_RE.test(channelKey) ); } +function parseWorkspaceQualifiedChannelId(channelKey: string): string | undefined { + if (!/^team:/i.test(channelKey)) { + return undefined; + } + try { + const target = parseSlackTarget(channelKey); + return target?.kind === "channel" && target.teamId ? target.id : undefined; + } catch { + return undefined; + } +} + function looksLikeSlackChannelNameKey(channelKey: string): boolean { const name = channelKey.startsWith("#") ? channelKey.slice(1) : channelKey; return ( diff --git a/extensions/slack/src/group-policy.test.ts b/extensions/slack/src/group-policy.test.ts index 832c2aba2437..b81faad5fd53 100644 --- a/extensions/slack/src/group-policy.test.ts +++ b/extensions/slack/src/group-policy.test.ts @@ -2,6 +2,7 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { describe, expect, it } from "vitest"; import { resolveSlackGroupRequireMention, resolveSlackGroupToolPolicy } from "./group-policy.js"; +import { registerSlackInstallationState } from "./installation-identity-state.js"; const cfg = { channels: { @@ -54,6 +55,160 @@ describe("slack group policy", () => { expect(wildcardTools).toEqual({ deny: ["exec"] }); }); + it.each([ + { configuredChannelId: "C01234567", groupId: "c01234567" }, + { configuredChannelId: "c01234567", groupId: "C01234567" }, + { configuredChannelId: "channel:C01234567", groupId: "c01234567" }, + { configuredChannelId: "channel:c01234567", groupId: "C01234567" }, + ])( + "applies $configuredChannelId channel and sender policies to session $groupId", + ({ configuredChannelId, groupId }) => { + const channelPolicyCfg = { + channels: { + slack: { + channels: { + [configuredChannelId]: { + requireMention: false, + tools: { allow: ["message.send"] }, + toolsBySender: { + "id:user:alice": { allow: ["sessions.list"] }, + }, + }, + "*": { + requireMention: true, + tools: { deny: ["exec"] }, + }, + }, + }, + }, + } as OpenClawConfig; + + expect(resolveSlackGroupRequireMention({ cfg: channelPolicyCfg, groupId })).toBe(false); + expect( + resolveSlackGroupToolPolicy({ + cfg: channelPolicyCfg, + groupId, + senderId: "user:bob", + }), + ).toEqual({ allow: ["message.send"] }); + expect( + resolveSlackGroupToolPolicy({ + cfg: channelPolicyCfg, + groupId, + senderId: "user:alice", + }), + ).toEqual({ allow: ["sessions.list"] }); + }, + ); + + it("scopes Enterprise mention and tool policies to the event workspace", () => { + const installationState = registerSlackInstallationState("default", "enterprise"); + const enterpriseCfg = { + channels: { + slack: { + channels: { + "team:T11111111:channel:C01234567": { + requireMention: false, + tools: { allow: ["message.send"] }, + }, + "team:T22222222:channel:C01234567": { + requireMention: true, + tools: { deny: ["exec"] }, + }, + }, + }, + }, + } as OpenClawConfig; + + try { + expect( + resolveSlackGroupRequireMention({ + cfg: enterpriseCfg, + groupId: "C01234567", + groupSpace: "T11111111", + }), + ).toBe(false); + expect( + resolveSlackGroupToolPolicy({ + cfg: enterpriseCfg, + groupId: "C01234567", + groupSpace: "T11111111", + }), + ).toEqual({ allow: ["message.send"] }); + expect( + resolveSlackGroupRequireMention({ + cfg: enterpriseCfg, + groupId: "C01234567", + groupSpace: "T22222222", + }), + ).toBe(true); + expect( + resolveSlackGroupToolPolicy({ + cfg: enterpriseCfg, + groupId: "C01234567", + groupSpace: "T22222222", + }), + ).toEqual({ deny: ["exec"] }); + } finally { + installationState.release(); + } + }); + + it("retains bare channel policy matching for workspace installs", () => { + const installationState = registerSlackInstallationState("default", "workspace"); + const workspaceCfg = { + channels: { + slack: { + channels: { + C01234567: { + requireMention: false, + tools: { allow: ["message.send"] }, + }, + }, + }, + }, + } as OpenClawConfig; + + try { + expect( + resolveSlackGroupRequireMention({ + cfg: workspaceCfg, + groupId: "C01234567", + groupSpace: "T11111111", + }), + ).toBe(false); + expect( + resolveSlackGroupToolPolicy({ + cfg: workspaceCfg, + groupId: "C01234567", + groupSpace: "T11111111", + }), + ).toEqual({ allow: ["message.send"] }); + } finally { + installationState.release(); + } + }); + + it("prefers the exact channel ID when case variants have different policies", () => { + const caseSensitiveCfg = { + channels: { + slack: { + channels: { + c01234567: { tools: { allow: ["message.send"] } }, + C01234567: { tools: { deny: ["exec"] } }, + }, + }, + }, + } as OpenClawConfig; + + expect(resolveSlackGroupToolPolicy({ cfg: caseSensitiveCfg, groupId: "c01234567" })).toEqual({ + allow: ["message.send"], + }); + expect(resolveSlackGroupToolPolicy({ cfg: caseSensitiveCfg, groupId: "C01234567" })).toEqual({ + deny: ["exec"], + }); + }); + it("keeps wildcard fields hidden by a matched whole entry", () => { const partialCfg = { channels: { diff --git a/extensions/slack/src/group-policy.ts b/extensions/slack/src/group-policy.ts index 5779b7477db4..7b9dd1e8a607 100644 --- a/extensions/slack/src/group-policy.ts +++ b/extensions/slack/src/group-policy.ts @@ -9,8 +9,10 @@ import { type ScopeNode, type ScopeTree, } from "openclaw/plugin-sdk/channel-policy"; +import { buildChannelKeyCandidates } from "openclaw/plugin-sdk/channel-targets"; import { normalizeHyphenSlug } from "openclaw/plugin-sdk/string-normalization-runtime"; import { mergeSlackAccountConfig, resolveDefaultSlackAccountId } from "./accounts.js"; +import { getSlackInstallationKind } from "./installation-identity-state.js"; type SlackChannelPolicyEntry = { requireMention?: boolean; @@ -18,6 +20,40 @@ type SlackChannelPolicyEntry = { toolsBySender?: GroupToolPolicyBySenderConfig; }; +export function buildSlackChannelIdCandidates( + channelId: string | null | undefined, + teamId?: string | null, + options?: { allowUnscoped?: boolean }, +): string[] { + const trimmedId = channelId?.trim(); + if (!trimmedId) { + return []; + } + const lowercaseId = trimmedId.toLowerCase(); + const uppercaseId = trimmedId.toUpperCase(); + const exactTeamId = teamId || undefined; + const lowercaseTeamId = exactTeamId?.toLowerCase(); + const uppercaseTeamId = exactTeamId?.toUpperCase(); + // Inbound Slack IDs are uppercase, but persisted session group IDs are lowercase. + const scopedCandidates = buildChannelKeyCandidates( + exactTeamId ? `team:${exactTeamId}:channel:${trimmedId}` : undefined, + lowercaseTeamId ? `team:${lowercaseTeamId}:channel:${lowercaseId}` : undefined, + uppercaseTeamId ? `team:${uppercaseTeamId}:channel:${uppercaseId}` : undefined, + ); + if (exactTeamId && options?.allowUnscoped !== true) { + return scopedCandidates; + } + return buildChannelKeyCandidates( + ...scopedCandidates, + trimmedId, + lowercaseId, + uppercaseId, + `channel:${trimmedId}`, + `channel:${lowercaseId}`, + `channel:${uppercaseId}`, + ); +} + export function buildSlackChannelPolicyScope(params: { channels?: Record; candidates: readonly string[]; @@ -49,14 +85,14 @@ function resolveSlackGroupPolicyScope(params: ChannelGroupContext) { const channels = mergeSlackAccountConfig(params.cfg, accountId).channels as | Record | undefined; - const channelId = params.groupId?.trim(); const channelName = params.groupChannel?.replace(/^#/, ""); - const candidates = [ - channelId, + const allowUnscoped = getSlackInstallationKind(accountId) !== "enterprise"; + const candidates = buildChannelKeyCandidates( + ...buildSlackChannelIdCandidates(params.groupId, params.groupSpace, { allowUnscoped }), channelName ? `#${channelName}` : undefined, channelName, normalizeHyphenSlug(channelName), - ].filter((candidate): candidate is string => Boolean(candidate)); + ); return buildSlackChannelPolicyScope({ channels, candidates }); } diff --git a/extensions/slack/src/monitor/allow-list.test.ts b/extensions/slack/src/monitor/allow-list.test.ts index 3761b12342f4..a056a8c0402c 100644 --- a/extensions/slack/src/monitor/allow-list.test.ts +++ b/extensions/slack/src/monitor/allow-list.test.ts @@ -63,4 +63,65 @@ describe("slack/allow-list", () => { false, ); }); + + it("matches a workspace-qualified user only in that workspace", () => { + const allowList = ["team:t11111111:user:u01234567"]; + + expect( + resolveSlackAllowListMatch({ + allowList, + teamId: "T11111111", + id: "U01234567", + }), + ).toEqual({ + allowed: true, + matchKey: "team:t11111111:user:u01234567", + matchSource: "workspace-id", + }); + expect( + resolveSlackAllowListMatch({ + allowList, + teamId: "T22222222", + id: "U01234567", + }), + ).toEqual({ allowed: false }); + expect( + resolveSlackAllowListMatch({ + allowList: ["u01234567"], + teamId: "T22222222", + id: "U01234567", + }), + ).toEqual({ allowed: false }); + expect( + resolveSlackAllowListMatch({ + allowList: ["u01234567"], + teamId: "T22222222", + id: "U01234567", + allowUnscoped: true, + }), + ).toEqual({ allowed: true, matchKey: "u01234567", matchSource: "id" }); + }); + + it("matches a workspace-qualified bot only in that workspace", () => { + const allowList = ["team:t11111111:user:b01234567"]; + + expect( + resolveSlackAllowListMatch({ + allowList, + teamId: "T11111111", + id: "B01234567", + }), + ).toEqual({ + allowed: true, + matchKey: "team:t11111111:user:b01234567", + matchSource: "workspace-id", + }); + expect( + resolveSlackAllowListMatch({ + allowList, + teamId: "T22222222", + id: "B01234567", + }), + ).toEqual({ allowed: false }); + }); }); diff --git a/extensions/slack/src/monitor/allow-list.ts b/extensions/slack/src/monitor/allow-list.ts index 6281629a62b5..c184f1b59624 100644 --- a/extensions/slack/src/monitor/allow-list.ts +++ b/extensions/slack/src/monitor/allow-list.ts @@ -10,6 +10,7 @@ import { normalizeStringEntries, normalizeStringEntriesLower, } from "openclaw/plugin-sdk/string-normalization-runtime"; +import { parseSlackTarget } from "../target-parsing.js"; const SLACK_SLUG_CACHE_MAX = 512; const slackSlugCache = new Map(); @@ -44,26 +45,50 @@ export function normalizeSlackAllowOwnerEntry(entry: string): string | undefined if (!trimmed || trimmed === "*") { return undefined; } + try { + const target = parseSlackTarget(trimmed); + if (target?.kind === "user" && target.teamId) { + return target.id.toLowerCase(); + } + } catch { + return undefined; + } const withoutPrefix = trimmed.replace(/^(slack:|user:)/, ""); return /^u[a-z0-9]+$/.test(withoutPrefix) ? withoutPrefix : undefined; } export type SlackAllowListMatch = AllowlistMatch< - "wildcard" | "id" | "prefixed-id" | "prefixed-user" | "name" | "prefixed-name" | "slug" + | "wildcard" + | "workspace-id" + | "id" + | "prefixed-id" + | "prefixed-user" + | "name" + | "prefixed-name" + | "slug" >; type SlackAllowListSource = Exclude; export function resolveSlackAllowListMatch(params: { allowList: readonly string[]; + teamId?: string; id?: string; name?: string; allowNameMatching?: boolean; + allowUnscoped?: boolean; }): SlackAllowListMatch { const compiledAllowList = compileAllowlist(params.allowList); + const teamId = normalizeOptionalLowercaseString(params.teamId); const id = normalizeOptionalLowercaseString(params.id); const name = normalizeOptionalLowercaseString(params.name); const slug = normalizeSlackSlug(name); - const candidates: Array<{ value?: string; source: SlackAllowListSource }> = [ + const scopedCandidates: Array<{ value?: string; source: SlackAllowListSource }> = [ + { + value: teamId && id ? `team:${teamId}:user:${id}` : undefined, + source: "workspace-id", + }, + ]; + const unscopedCandidates: Array<{ value?: string; source: SlackAllowListSource }> = [ { value: id, source: "id" }, { value: id ? `slack:${id}` : undefined, source: "prefixed-id" }, { value: id ? `user:${id}` : undefined, source: "prefixed-user" }, @@ -75,6 +100,10 @@ export function resolveSlackAllowListMatch(params: { ] satisfies Array<{ value?: string; source: SlackAllowListSource }>) : []), ]; + const candidates = + teamId && params.allowUnscoped !== true + ? scopedCandidates + : [...scopedCandidates, ...unscopedCandidates]; return resolveCompiledAllowlistMatch({ compiledAllowlist: compiledAllowList, candidates, @@ -83,18 +112,22 @@ export function resolveSlackAllowListMatch(params: { export function allowListMatches(params: { allowList: string[]; + teamId?: string; id?: string; name?: string; allowNameMatching?: boolean; + allowUnscoped?: boolean; }) { return resolveSlackAllowListMatch(params).allowed; } export function resolveSlackUserAllowed(params: { allowList?: Array; + teamId?: string; userId?: string; userName?: string; allowNameMatching?: boolean; + allowUnscoped?: boolean; }) { const allowList = normalizeAllowListLower(params.allowList); if (allowList.length === 0) { @@ -102,8 +135,37 @@ export function resolveSlackUserAllowed(params: { } return allowListMatches({ allowList, + teamId: params.teamId, id: params.userId, name: params.userName, allowNameMatching: params.allowNameMatching, + allowUnscoped: params.allowUnscoped, + }); +} + +export function resolveSlackUserAllowListForTeam(params: { + allowList?: Array; + teamId?: string; + preserveUnmatchedScopedEntries?: boolean; + allowUnscoped?: boolean; +}): string[] { + const allowList = normalizeAllowListLower(params.allowList); + const teamId = normalizeOptionalLowercaseString(params.teamId); + return allowList.flatMap((entry) => { + if (entry === "*") { + return [entry]; + } + if (!entry.startsWith("team:")) { + return params.allowUnscoped === true || params.preserveUnmatchedScopedEntries ? [entry] : []; + } + try { + const target = parseSlackTarget(entry); + if (target?.kind === "user" && target.teamId?.toLowerCase() === teamId) { + return params.allowUnscoped === true ? [target.id.toLowerCase()] : [entry]; + } + return params.preserveUnmatchedScopedEntries ? [entry] : []; + } catch { + return params.preserveUnmatchedScopedEntries ? [entry] : []; + } }); } diff --git a/extensions/slack/src/monitor/auth.test.ts b/extensions/slack/src/monitor/auth.test.ts index f25c0e1c4c1d..5e9396d218fd 100644 --- a/extensions/slack/src/monitor/auth.test.ts +++ b/extensions/slack/src/monitor/auth.test.ts @@ -36,6 +36,7 @@ function makeAuthorizeCtx(params?: { resolveChannelName?: ( channelId: string, ) => Promise<{ name?: string; type?: "im" | "mpim" | "channel" | "group" }>; + installationIdentity?: SlackMonitorContext["installationIdentity"]; }) { return { allowFrom: params?.allowFrom ?? [], @@ -46,6 +47,10 @@ function makeAuthorizeCtx(params?: { channelsConfig: params?.channelsConfig ?? {}, channelsConfigKeys: Object.keys(params?.channelsConfig ?? {}), defaultRequireMention: true, + installationIdentity: params?.installationIdentity ?? { + kind: "workspace", + teamId: "T_MAIN", + }, isChannelAllowed: vi.fn(() => true), resolveUserName: vi.fn( params?.resolveUserName ?? ((_) => Promise.resolve({ name: undefined })), @@ -119,16 +124,33 @@ describe("resolveSlackEffectiveAllowFrom", () => { includePairingStore: true, eventScope: { teamId: "T11111111", client: {} as never }, }), - ).resolves.toEqual(["uconfig123", "u11111111"]); + ).resolves.toEqual(["team:t11111111:user:u11111111"]); await expect( resolveSlackEffectiveAllowFrom(ctx, { includePairingStore: true, eventScope: { teamId: "T22222222", client: {} as never }, }), - ).resolves.toEqual(["uconfig123", "u22222222"]); + ).resolves.toEqual(["team:t22222222:user:u22222222"]); await expect( resolveSlackEffectiveAllowFrom(ctx, { includePairingStore: true }), - ).resolves.toEqual(["uconfig123"]); + ).resolves.toEqual([]); + }); + + it("keeps only configured users for the current Enterprise workspace", async () => { + const ctx = makeSlackCtx(["team:T11111111:user:U01234567"]); + ctx.installationIdentity = { kind: "enterprise", enterpriseId: "E11111111" }; + + await expect( + resolveSlackEffectiveAllowFrom(ctx, { + eventScope: { teamId: "T11111111", client: {} as never }, + }), + ).resolves.toEqual(["team:t11111111:user:u01234567"]); + await expect( + resolveSlackEffectiveAllowFrom(ctx, { + eventScope: { teamId: "T22222222", client: {} as never }, + }), + ).resolves.toEqual([]); + await expect(resolveSlackEffectiveAllowFrom(ctx)).resolves.toEqual([]); }); }); @@ -514,6 +536,47 @@ describe("resolveSlackCommandIngress", () => { ({ resolveSlackCommandIngress } = await import("./auth.js")); }); + it.each([ + ["allows the workspace-qualified user in its workspace", "T11111111", "allow", true], + ["blocks the same bare user ID in another workspace", "T22222222", "block", false], + ] as const)("%s", async (_name, teamId, decision, allowed) => { + const result = await resolveSlackCommandIngress({ + ctx: makeAuthorizeCtx({ + installationIdentity: { kind: "enterprise", enterpriseId: "E11111111" }, + }), + teamId, + senderId: "U01234567", + channelType: "channel", + channelId: "C01234567", + ownerAllowFromLower: [], + channelUsers: ["team:T11111111:user:U01234567"], + allowTextCommands: false, + hasControlCommand: false, + }); + + expect(result.senderAccess.decision).toBe(decision); + expect(result.senderAccess.gate?.allowed).toBe(allowed); + }); + + it("does not authorize a bare user ID for an Enterprise workspace event", async () => { + const result = await resolveSlackCommandIngress({ + ctx: makeAuthorizeCtx({ + installationIdentity: { kind: "enterprise", enterpriseId: "E11111111" }, + }), + teamId: "T11111111", + senderId: "U01234567", + channelType: "channel", + channelId: "C01234567", + ownerAllowFromLower: [], + channelUsers: ["U01234567"], + allowTextCommands: false, + hasControlCommand: false, + }); + + expect(result.senderAccess.decision).toBe("block"); + expect(result.senderAccess.gate?.allowed).toBe(false); + }); + it("does not authorize commands when sender denial stops before the command gate", async () => { const result = await resolveSlackCommandIngress({ ctx: makeAuthorizeCtx(), diff --git a/extensions/slack/src/monitor/auth.ts b/extensions/slack/src/monitor/auth.ts index 6aa2eceb8d8b..9ac9da72603b 100644 --- a/extensions/slack/src/monitor/auth.ts +++ b/extensions/slack/src/monitor/auth.ts @@ -18,10 +18,10 @@ import { logVerbose } from "openclaw/plugin-sdk/runtime-env"; import { parseSlackTarget } from "../target-parsing.js"; import { allowListMatches, - normalizeAllowList, normalizeAllowListLower, normalizeSlackAllowOwnerEntry, normalizeSlackSlug, + resolveSlackUserAllowListForTeam, } from "./allow-list.js"; import { resolveSlackChannelConfig } from "./channel-config.js"; import { inferSlackChannelType } from "./channel-type.js"; @@ -79,6 +79,14 @@ function normalizeSlackStableEntry(entry: string): string | null { if (!normalized) { return null; } + try { + const target = parseSlackTarget(normalized); + if (target?.kind === "user" && target.teamId) { + return target.normalized; + } + } catch { + return null; + } const userId = normalizeSlackUserId(normalized); return isSlackStableUserId(userId) ? userId : null; } @@ -125,8 +133,17 @@ const slackIngressIdentity = defineStableChannelIngressIdentity({ })), }); -function createSlackIngressSubject(params: { senderId: string; senderName?: string }) { - const senderId = normalizeSlackUserId(params.senderId); +function createSlackIngressSubject(params: { + senderId: string; + senderName?: string; + teamId?: string; + workspaceScoped?: boolean; +}) { + const bareSenderId = normalizeSlackUserId(params.senderId); + const senderId = + params.workspaceScoped && params.teamId + ? `team:${params.teamId.toLowerCase()}:user:${bareSenderId}` + : bareSenderId; const senderName = params.senderName?.trim().toLowerCase(); const senderNameSlug = senderName ? normalizeSlackSlug(senderName) : undefined; return { @@ -178,15 +195,20 @@ function pruneChannelMembersCache(cache: Map { try { const target = parseSlackTarget(entry); - return target?.kind === "user" && target.teamId?.toLowerCase() === teamId ? [target.id] : []; + return target?.kind === "user" && target.teamId?.toLowerCase() === normalizedTeamId + ? [entry] + : []; } catch { return []; } @@ -322,9 +350,11 @@ export async function authorizeSlackBotRoomMessage(params: { channelUserAllowList.length > 0 && allowListMatches({ allowList: channelUserAllowList, + teamId: params.eventScope?.teamId ?? params.ctx.teamId, id: params.senderId, name: params.senderName, allowNameMatching: params.ctx.allowNameMatching, + allowUnscoped: params.ctx.installationIdentity?.kind !== "enterprise", }) ) { return true; @@ -370,6 +400,7 @@ function slackIngressConversationKind( export async function resolveSlackCommandIngress(params: { ctx: SlackMonitorContext; + teamId?: string; senderId: string; senderName?: string; channelType: SlackIngressChannelType; @@ -387,19 +418,29 @@ export async function resolveSlackCommandIngress(params: { }) { const isDirectMessage = params.channelType === "im"; const isGroupDm = params.channelType === "mpim"; - const channelUsers = normalizeAllowListLower(params.channelUsers); - const channelUsersConfigured = !isDirectMessage && !isGroupDm && channelUsers.length > 0; + const teamId = params.teamId ?? params.ctx.teamId; + const allowUnscoped = params.ctx.installationIdentity?.kind !== "enterprise"; + const ownerAllowFrom = resolveSlackUserAllowListForTeam({ + allowList: params.ownerAllowFromLower, + teamId, + allowUnscoped, + }); + const channelUsers = resolveSlackUserAllowListForTeam({ + allowList: params.channelUsers, + teamId, + allowUnscoped, + }); + const channelUsersConfigured = + !isDirectMessage && !isGroupDm && normalizeAllowListLower(params.channelUsers).length > 0; // MPIM ingress is group-shaped, but its sender policy is DM-owned. Callers // pass configured allowFrom without pairing-store approvals for this path. - const groupAllowFrom = isGroupDm - ? params.ownerAllowFromLower - : channelUsersConfigured - ? channelUsers - : []; + const groupAllowFrom = isGroupDm ? ownerAllowFrom : channelUsersConfigured ? channelUsers : []; const result = await createSlackIngressResolver(params.ctx).message({ subject: createSlackIngressSubject({ senderId: params.senderId, senderName: params.senderName, + teamId, + workspaceScoped: !allowUnscoped, }), conversation: { kind: slackIngressConversationKind(params.channelType), @@ -418,13 +459,13 @@ export async function resolveSlackCommandIngress(params: { ...(params.activation ? { activation: params.activation } : {}), }, mentionFacts: params.mentionFacts, - allowFrom: isDirectMessage ? ["*"] : params.ownerAllowFromLower, + allowFrom: isDirectMessage ? ["*"] : ownerAllowFrom, groupAllowFrom, command: { allowTextCommands: params.allowTextCommands, hasControlCommand: params.hasControlCommand, modeWhenAccessGroupsOff: params.modeWhenAccessGroupsOff, - ...(isDirectMessage ? { commandOwnerAllowFrom: params.ownerAllowFromLower } : {}), + ...(isDirectMessage ? { commandOwnerAllowFrom: ownerAllowFrom } : {}), }, }); return result; @@ -432,6 +473,7 @@ export async function resolveSlackCommandIngress(params: { async function decideSlackSystemIngress(params: { ctx: SlackMonitorContext; + teamId?: string; senderId: string; senderName?: string; channelType: SlackIngressChannelType; @@ -442,12 +484,24 @@ async function decideSlackSystemIngress(params: { }): Promise { const isDirectMessage = params.channelType === "im"; const isGroupDm = params.channelType === "mpim"; - const channelUsers = normalizeAllowListLower(params.channelUsers); - const channelUsersConfigured = !isDirectMessage && !isGroupDm && channelUsers.length > 0; + const teamId = params.teamId ?? params.ctx.teamId; + const allowUnscoped = params.ctx.installationIdentity?.kind !== "enterprise"; + const ownerAllowFromLower = resolveSlackUserAllowListForTeam({ + allowList: params.ownerAllowFromLower, + teamId, + allowUnscoped, + }); + const channelUsers = resolveSlackUserAllowListForTeam({ + allowList: params.channelUsers, + teamId, + allowUnscoped, + }); + const channelUsersConfigured = + !isDirectMessage && !isGroupDm && normalizeAllowListLower(params.channelUsers).length > 0; const ownerAllowFrom = params.interactiveEvent && channelUsersConfigured - ? params.ownerAllowFromLower.filter((entry) => entry !== "*") - : params.ownerAllowFromLower; + ? ownerAllowFromLower.filter((entry) => entry !== "*") + : ownerAllowFromLower; const hasAnyCommandAllowlist = ownerAllowFrom.length > 0 || channelUsersConfigured; const groupAllowFrom = (() => { if (isDirectMessage) { @@ -462,12 +516,14 @@ async function decideSlackSystemIngress(params: { if (channelUsersConfigured) { return channelUsers; } - return params.channelId ? ["*"] : wildcardWhenOpen(params.ownerAllowFromLower); + return params.channelId ? ["*"] : wildcardWhenOpen(ownerAllowFromLower); })(); const result = await createSlackIngressResolver(params.ctx).message({ subject: createSlackIngressSubject({ senderId: params.senderId, senderName: params.senderName, + teamId, + workspaceScoped: !allowUnscoped, }), conversation: { kind: slackIngressConversationKind(params.channelType), @@ -483,14 +539,14 @@ async function decideSlackSystemIngress(params: { ? "allowlist" : params.interactiveEvent && hasAnyCommandAllowlist ? "open" - : channelUsersConfigured || (!params.channelId && params.ownerAllowFromLower.length > 0) + : channelUsersConfigured || (!params.channelId && ownerAllowFromLower.length > 0) ? "allowlist" : "open", policy: { groupAllowFromFallbackToAllowFrom: false, mutableIdentifierMatching: params.ctx.allowNameMatching ? "enabled" : "disabled", }, - allowFrom: isDirectMessage ? wildcardWhenOpen(params.ownerAllowFromLower) : ownerAllowFrom, + allowFrom: isDirectMessage ? wildcardWhenOpen(ownerAllowFromLower) : ownerAllowFrom, groupAllowFrom, command: params.interactiveEvent && hasAnyCommandAllowlist @@ -545,6 +601,7 @@ export async function authorizeSlackSystemEventSender(params: { channelType = normalizeSlackChannelType(resolvedTypeSource, channelId); if ( !params.ctx.isChannelAllowed({ + teamId: params.eventScope?.teamId ?? params.ctx.teamId, channelId, channelName, channelType, @@ -602,6 +659,8 @@ export async function authorizeSlackSystemEventSender(params: { }); const channelConfig = channelId ? resolveSlackChannelConfig({ + teamId: params.eventScope?.teamId ?? params.ctx.teamId, + allowUnscoped: params.ctx.installationIdentity?.kind !== "enterprise", channelId, channelName, channels: params.ctx.channelsConfig, @@ -614,6 +673,7 @@ export async function authorizeSlackSystemEventSender(params: { Array.isArray(channelConfig?.users) && channelConfig.users.length > 0; const decision = await decideSlackSystemIngress({ ctx: params.ctx, + teamId: params.eventScope?.teamId ?? params.ctx.teamId, senderId, senderName, channelType: ingressChannelType, diff --git a/extensions/slack/src/monitor/channel-config.ts b/extensions/slack/src/monitor/channel-config.ts index 243705fb6135..c4c6be5e2575 100644 --- a/extensions/slack/src/monitor/channel-config.ts +++ b/extensions/slack/src/monitor/channel-config.ts @@ -10,9 +10,8 @@ import type { SlackChannelConfig, } from "openclaw/plugin-sdk/config-contracts"; import { mergePairLoopGuardConfig } from "openclaw/plugin-sdk/pair-loop-guard-runtime"; -import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime"; -import { buildSlackChannelPolicyScope } from "../group-policy.js"; -import { normalizeSlackSlug } from "./allow-list.js"; +import { buildSlackChannelIdCandidates, buildSlackChannelPolicyScope } from "../group-policy.js"; +import { normalizeSlackSlug, resolveSlackUserAllowListForTeam } from "./allow-list.js"; export type SlackChannelConfigResolved = { allowed: boolean; @@ -64,6 +63,8 @@ export function resolveSlackChannelLabel(params: { channelId?: string; channelNa } export function resolveSlackChannelConfig(params: { + teamId?: string; + allowUnscoped?: boolean; channelId: string; channelName?: string; channels?: SlackChannelConfigEntries; @@ -83,22 +84,10 @@ export function resolveSlackChannelConfig(params: { const keys = channelKeys ?? Object.keys(entries); const normalizedName = channelName ? normalizeSlackSlug(channelName) : ""; const directName = channelName ? channelName.trim() : ""; - // Slack always delivers channel IDs in uppercase (e.g. C0ABC12345) but - // operators commonly write them in lowercase in their config. Add both - // case variants so the lookup is case-insensitive without requiring a full - // entry-scan. buildChannelKeyCandidates deduplicates identical keys. - const channelIdLower = normalizeLowercaseStringOrEmpty(channelId); - const channelIdUpper = channelId.toUpperCase(); - const channelTarget = `channel:${channelId}`; - const channelTargetLower = `channel:${channelIdLower}`; - const channelTargetUpper = `channel:${channelIdUpper}`; const candidates = buildChannelKeyCandidates( - channelId, - channelIdLower !== channelId ? channelIdLower : undefined, - channelIdUpper !== channelId ? channelIdUpper : undefined, - channelTarget, - channelTargetLower !== channelTarget ? channelTargetLower : undefined, - channelTargetUpper !== channelTarget ? channelTargetUpper : undefined, + ...buildSlackChannelIdCandidates(channelId, params.teamId, { + allowUnscoped: params.allowUnscoped, + }), allowNameMatching ? (channelName ? `#${directName}` : undefined) : undefined, allowNameMatching ? directName : undefined, allowNameMatching ? normalizedName : undefined, @@ -130,7 +119,14 @@ export function resolveSlackChannelConfig(params: { fallback?.botLoopProtection, matched?.botLoopProtection, ); - const users = firstDefined(resolved.users, fallback?.users); + const users = resolveSlackUserAllowListForTeam({ + allowList: firstDefined(resolved.users, fallback?.users), + teamId: params.teamId, + allowUnscoped: params.allowUnscoped, + // Keeping unmatched entries preserves the configured allowlist gate; strict + // workspace ingress treats bare and differently scoped values as non-matching. + preserveUnmatchedScopedEntries: true, + }); const skills = firstDefined(resolved.skills, fallback?.skills); const systemPrompt = firstDefined(resolved.systemPrompt, fallback?.systemPrompt); const presenceEvents = firstDefined(resolved.presenceEvents, fallback?.presenceEvents); @@ -141,7 +137,7 @@ export function resolveSlackChannelConfig(params: { replyToMode, allowBots, botLoopProtection, - users, + users: users.length > 0 ? users : undefined, skills, systemPrompt, presenceEvents, diff --git a/extensions/slack/src/monitor/context.test.ts b/extensions/slack/src/monitor/context.test.ts index 6d1feddd911f..1866887466e3 100644 --- a/extensions/slack/src/monitor/context.test.ts +++ b/extensions/slack/src/monitor/context.test.ts @@ -13,6 +13,7 @@ function createTestContext(params?: { groupDmChannels?: string[]; appClient?: App["client"]; apiAppId?: string; + channelsConfig?: Record; }) { return createSlackMonitorContext({ cfg: { @@ -37,6 +38,7 @@ function createTestContext(params?: { groupDmEnabled: params?.groupDmEnabled ?? false, groupDmChannels: params?.groupDmChannels ?? [], defaultRequireMention: true, + channelsConfig: params?.channelsConfig, groupPolicy: "allowlist", useAccessGroups: true, reactionMode: "off", @@ -149,6 +151,46 @@ describe("createSlackMonitorContext isChannelAllowed", () => { expect(ctx.isChannelAllowed({ channelId: "G456", channelType: "mpim" })).toBe(true); expect(ctx.isChannelAllowed({ channelId: "G999", channelType: "mpim" })).toBe(false); }); + + it("matches workspace-qualified channel and group DM policies", () => { + const ctx = createTestContext({ + groupDmEnabled: true, + groupDmChannels: ["team:T11111111:channel:G01234567"], + channelsConfig: { + "team:T11111111:channel:C01234567": { enabled: true }, + "team:T22222222:channel:C01234567": { enabled: false }, + }, + }); + + expect( + ctx.isChannelAllowed({ + teamId: "T11111111", + channelId: "C01234567", + channelType: "channel", + }), + ).toBe(true); + expect( + ctx.isChannelAllowed({ + teamId: "T22222222", + channelId: "C01234567", + channelType: "channel", + }), + ).toBe(false); + expect( + ctx.isChannelAllowed({ + teamId: "T11111111", + channelId: "G01234567", + channelType: "mpim", + }), + ).toBe(true); + expect( + ctx.isChannelAllowed({ + teamId: "T22222222", + channelId: "G01234567", + channelType: "mpim", + }), + ).toBe(false); + }); }); describe("createSlackMonitorContext resolveSlackSystemEventSessionKey", () => { diff --git a/extensions/slack/src/monitor/context.ts b/extensions/slack/src/monitor/context.ts index d7a62d98c6c4..9b6c8bcb05a5 100644 --- a/extensions/slack/src/monitor/context.ts +++ b/extensions/slack/src/monitor/context.ts @@ -18,6 +18,7 @@ import { normalizeOptionalString, } from "openclaw/plugin-sdk/string-coerce-runtime"; import { formatSlackError } from "../errors.js"; +import { buildSlackChannelIdCandidates } from "../group-policy.js"; import type { SlackMessageEvent } from "../types.js"; import { createSlackAgentViewState } from "./agent-view-state.js"; import { normalizeAllowList, normalizeAllowListLower, normalizeSlackSlug } from "./allow-list.js"; @@ -116,6 +117,7 @@ export type SlackMonitorContext = { eventScope?: SlackEventScope; }) => string; isChannelAllowed: (params: { + teamId?: string; channelId?: string; channelName?: string; channelType?: SlackMessageEvent["channel_type"]; @@ -374,6 +376,7 @@ export function createSlackMonitorContext(params: { }); const isChannelAllowed = (p: { + teamId?: string; channelId?: string; channelName?: string; channelType?: SlackMessageEvent["channel_type"]; @@ -392,7 +395,9 @@ export function createSlackMonitorContext(params: { if (isGroupDm && groupDmChannels.length > 0) { const candidates = [ - p.channelId, + ...buildSlackChannelIdCandidates(p.channelId, p.teamId, { + allowUnscoped: params.installationIdentity?.kind !== "enterprise", + }), p.channelName ? `#${p.channelName}` : undefined, p.channelName, p.channelName ? normalizeSlackSlug(p.channelName) : undefined, @@ -409,6 +414,8 @@ export function createSlackMonitorContext(params: { if (isRoom && p.channelId) { const channelConfig = resolveSlackChannelConfig({ + teamId: p.teamId, + allowUnscoped: params.installationIdentity?.kind !== "enterprise", channelId: p.channelId, channelName: p.channelName, channels: params.channelsConfig, @@ -433,7 +440,7 @@ export function createSlackMonitorContext(params: { if (shouldDrop) { if (explicitlyDisabled) { const reason = "channel_not_allowed"; - const warningKey = `${params.accountId}:${p.channelId}:${reason}`; + const warningKey = `${params.accountId}:${p.teamId ? `${p.teamId}:` : ""}${p.channelId}:${reason}`; if (!channelDenialWarnings.peek(warningKey)) { channelDenialWarnings.check(warningKey); logger.warn( diff --git a/extensions/slack/src/monitor/dm-auth.test.ts b/extensions/slack/src/monitor/dm-auth.test.ts index e9df4881181b..acee17596483 100644 --- a/extensions/slack/src/monitor/dm-auth.test.ts +++ b/extensions/slack/src/monitor/dm-auth.test.ts @@ -76,6 +76,31 @@ describe("authorizeSlackDirectMessage", () => { }); }); + it("allows bare user ids for workspace-install DMs", async () => { + const params = makeParams("allowlist"); + params.ctx.installationIdentity = { kind: "workspace", teamId: "T11111111" }; + params.eventScope = { teamId: "T11111111", client: {} as never }; + params.allowFromLower = ["u123"]; + + await expect(authorizeSlackDirectMessage(params)).resolves.toBe(true); + + expect(params.onUnauthorized).not.toHaveBeenCalled(); + }); + + it("keeps bare user ids scoped out of Enterprise DMs", async () => { + const params = makeParams("allowlist"); + params.ctx.installationIdentity = { kind: "enterprise", enterpriseId: "E11111111" }; + params.eventScope = { teamId: "T11111111", client: {} as never }; + params.allowFromLower = ["u123"]; + + await expect(authorizeSlackDirectMessage(params)).resolves.toBe(false); + + expect(params.onUnauthorized).toHaveBeenCalledWith({ + allowMatchMeta: "matchKey=none matchSource=none", + senderName: "Alice", + }); + }); + it("creates independent pairing requests for the same user in two Grid workspaces", async () => { const pendingCodes = new Map(); upsertChannelPairingRequestMock.mockImplementation( diff --git a/extensions/slack/src/monitor/dm-auth.ts b/extensions/slack/src/monitor/dm-auth.ts index 26371566796f..53b5b9a9c117 100644 --- a/extensions/slack/src/monitor/dm-auth.ts +++ b/extensions/slack/src/monitor/dm-auth.ts @@ -33,9 +33,11 @@ export async function authorizeSlackDirectMessage(params: { const senderName = sender?.name ?? undefined; const allowMatch = resolveSlackAllowListMatch({ allowList: params.allowFromLower, + teamId: params.eventScope?.teamId ?? params.ctx.teamId, id: params.senderId, name: senderName, allowNameMatching: params.ctx.allowNameMatching, + allowUnscoped: params.ctx.installationIdentity?.kind !== "enterprise", }); const allowMatchMeta = formatAllowlistMatchMeta(allowMatch); if (allowMatch.allowed) { diff --git a/extensions/slack/src/monitor/enterprise-install.test.ts b/extensions/slack/src/monitor/enterprise-install.test.ts index 8c9de0c3526c..46ce5ac69f94 100644 --- a/extensions/slack/src/monitor/enterprise-install.test.ts +++ b/extensions/slack/src/monitor/enterprise-install.test.ts @@ -92,16 +92,18 @@ describe("assertEnterpriseSlackPolicyConfig", () => { assertEnterpriseSlackPolicyConfig({ accountId: "org", config: { - allowFrom: ["U01234567", "slack:W01234567", "user:U12345678"], - dm: { groupChannels: ["G01234567", "channel:G12345678"] }, + allowFrom: ["team:T01234567:user:U01234567"], + dm: { + groupChannels: ["team:T01234567:channel:G01234567"], + }, mentionPatterns: { mode: "allow", allowIn: ["team:T01234567:channel:C01234567"], denyIn: ["team:T12345678:channel:C12345678"], }, channels: { - C01234567: { - users: ["U01234567", "slack:W01234567", "user:U12345678"], + "team:T01234567:channel:C01234567": { + users: ["team:T01234567:user:U01234567", "team:T01234567:user:B01234567"], toolsBySender: { U01234567: {}, "id:W01234567": {}, @@ -109,9 +111,11 @@ describe("assertEnterpriseSlackPolicyConfig", () => { "*": {}, }, }, - "channel:C12345678": {}, + "team:T12345678:channel:C12345678": {}, "*": {}, }, + reactionNotifications: "allowlist", + reactionAllowlist: ["team:T01234567:user:U01234567"], }, }), ).not.toThrow(); @@ -138,6 +142,25 @@ describe("assertEnterpriseSlackPolicyConfig", () => { ).toThrow(/cannot use dangerouslyAllowNameMatching/); }); + it.each<[string, SlackAccountConfig]>([ + ["channel ID", { channels: { C01234567: {} } }], + ["allowFrom user ID", { allowFrom: ["U01234567"] }], + ["group DM channel ID", { dm: { groupChannels: ["G01234567"] } }], + ["reaction user ID", { reactionNotifications: "allowlist", reactionAllowlist: ["U01234567"] }], + [ + "per-channel user ID", + { + channels: { + "team:T01234567:channel:C01234567": { users: ["U01234567"] }, + }, + }, + ], + ])("rejects unscoped Enterprise %s", (_label, config) => { + expect(() => assertEnterpriseSlackPolicyConfig({ accountId: "org", config })).toThrow( + /Slack Enterprise Grid/, + ); + }); + it.each<[string, SlackAccountConfig]>([ ["channels key", { channels: { general: {} } }], ["prefixed channels key", { channels: { "channel:general": {} } }], @@ -190,7 +213,7 @@ describe("assertEnterpriseSlackPolicyConfig", () => { accountId: "org", config: { channels: { - C01234567: { + "team:T01234567:channel:C01234567": { toolsBySender: { [entry]: { deny: ["exec"] }, "*": { allow: ["exec"] }, diff --git a/extensions/slack/src/monitor/enterprise-install.ts b/extensions/slack/src/monitor/enterprise-install.ts index 92db54138b0c..65b58452b416 100644 --- a/extensions/slack/src/monitor/enterprise-install.ts +++ b/extensions/slack/src/monitor/enterprise-install.ts @@ -41,9 +41,12 @@ export type SlackAuthTestIdentity = { }; const SLACK_CHANNEL_ID_RE = /^[CDG][A-Z0-9]{8,}$/; -const SLACK_USER_ID_RE = /^[UW][A-Z0-9]{8,}$/; +const SLACK_USER_ID_RE = /^[BUW][A-Z0-9]{8,}$/; -function isStableSlackChannelEntry(value: unknown, options?: { allowWildcard?: boolean }): boolean { +function isWorkspaceScopedSlackChannelEntry( + value: unknown, + options?: { allowWildcard?: boolean }, +): boolean { if (typeof value !== "string") { return false; } @@ -51,14 +54,10 @@ function isStableSlackChannelEntry(value: unknown, options?: { allowWildcard?: b if (normalized === "*") { return options?.allowWildcard === true; } - const prefixed = /^channel:([CDG][A-Z0-9]{8,})$/.exec(normalized); - if (prefixed?.[1]) { - return true; - } - return SLACK_CHANNEL_ID_RE.test(normalized); + return isWorkspaceQualifiedSlackTarget(normalized, "channel"); } -function isStableSlackAllowlistUserEntry(value: unknown): boolean { +function isWorkspaceScopedSlackAllowlistUserEntry(value: unknown): boolean { if (typeof value !== "string") { return false; } @@ -66,8 +65,7 @@ function isStableSlackAllowlistUserEntry(value: unknown): boolean { if (normalized === "*") { return true; } - const prefixed = /^(?:slack|user):([UW][A-Z0-9]{8,})$/.exec(normalized); - return Boolean(prefixed?.[1]) || SLACK_USER_ID_RE.test(normalized); + return isWorkspaceQualifiedSlackTarget(normalized, "user"); } function isStableSlackToolsBySenderEntry(value: unknown): boolean { @@ -136,30 +134,30 @@ export function assertEnterpriseSlackPolicyConfig(params: { assertStableEntries({ values: config.allowFrom, path: `channels.slack.accounts.${accountId}.allowFrom`, - predicate: isStableSlackAllowlistUserEntry, + predicate: isWorkspaceScopedSlackAllowlistUserEntry, }); assertStableEntries({ values: config.dm?.groupChannels, path: `channels.slack.accounts.${accountId}.dm.groupChannels`, - predicate: (value) => isStableSlackChannelEntry(value), + predicate: (value) => isWorkspaceScopedSlackChannelEntry(value), }); if (config.reactionNotifications === "allowlist") { assertStableEntries({ values: config.reactionAllowlist, path: `channels.slack.accounts.${accountId}.reactionAllowlist`, - predicate: isStableSlackAllowlistUserEntry, + predicate: isWorkspaceScopedSlackAllowlistUserEntry, }); } for (const [channelKey, channel] of Object.entries(config.channels ?? {})) { - if (!isStableSlackChannelEntry(channelKey, { allowWildcard: true })) { + if (!isWorkspaceScopedSlackChannelEntry(channelKey, { allowWildcard: true })) { throw new Error( - `Slack Enterprise Grid org installs require stable Slack channel IDs; invalid channels key ${JSON.stringify(channelKey)}`, + `Slack Enterprise Grid org installs require stable Slack channel IDs with workspace scope; invalid channels key ${JSON.stringify(channelKey)}`, ); } assertStableEntries({ values: channel?.users, path: `channels.slack.accounts.${accountId}.channels.${channelKey}.users`, - predicate: isStableSlackAllowlistUserEntry, + predicate: isWorkspaceScopedSlackAllowlistUserEntry, }); assertStableEntries({ values: Object.keys(channel?.toolsBySender ?? {}), diff --git a/extensions/slack/src/monitor/events/channels.ts b/extensions/slack/src/monitor/events/channels.ts index 40cf48052079..8e9a6d4dacaa 100644 --- a/extensions/slack/src/monitor/events/channels.ts +++ b/extensions/slack/src/monitor/events/channels.ts @@ -32,6 +32,7 @@ export function registerSlackChannelEvents(params: { }) => { if ( !ctx.isChannelAllowed({ + teamId: paramsLocal.eventScope?.teamId ?? ctx.teamId, channelId: paramsLocal.channelId, channelName: paramsLocal.channelName, channelType: "channel", diff --git a/extensions/slack/src/monitor/events/interactions.block-actions.ts b/extensions/slack/src/monitor/events/interactions.block-actions.ts index d957d0196cc8..2c7ea6e8c30d 100644 --- a/extensions/slack/src/monitor/events/interactions.block-actions.ts +++ b/extensions/slack/src/monitor/events/interactions.block-actions.ts @@ -910,6 +910,8 @@ async function resolveSlackBlockActionCommandAuthorized(params: { let channelUsers: Array = []; if (isRoom && params.parsed.channelId) { const channelConfig = resolveSlackChannelConfig({ + teamId: params.eventScope?.teamId ?? params.ctx.teamId, + allowUnscoped: params.ctx.installationIdentity?.kind !== "enterprise", channelId: params.parsed.channelId, channelName: params.auth.channelName, channels: params.ctx.channelsConfig, @@ -922,6 +924,7 @@ async function resolveSlackBlockActionCommandAuthorized(params: { const commandIngress = await resolveSlackCommandIngress({ ctx: params.ctx, + teamId: params.eventScope?.teamId ?? params.ctx.teamId, senderId: params.parsed.userId, senderName, channelType: params.auth.channelType ?? "channel", diff --git a/extensions/slack/src/monitor/events/reactions.ts b/extensions/slack/src/monitor/events/reactions.ts index f2efc8a78a37..70ca13acdc4e 100644 --- a/extensions/slack/src/monitor/events/reactions.ts +++ b/extensions/slack/src/monitor/events/reactions.ts @@ -15,6 +15,7 @@ import { function shouldEmitSlackReactionNotification(params: { ctx: SlackMonitorContext; event: SlackReactionEvent; + eventScope?: SlackEventScope; actorName?: string; }) { const { ctx, event, actorName } = params; @@ -31,9 +32,11 @@ function shouldEmitSlackReactionNotification(params: { } return allowListMatches({ allowList, + teamId: params.eventScope?.teamId ?? ctx.teamId, id: event.user, name: actorName, allowNameMatching: ctx.allowNameMatching, + allowUnscoped: ctx.installationIdentity?.kind !== "enterprise", }); } return ctx.reactionMode === "all"; @@ -88,6 +91,7 @@ export function registerSlackReactionEvents(params: { !shouldEmitSlackReactionNotification({ ctx, event, + eventScope, actorName: actorInfo?.name, }) ) { diff --git a/extensions/slack/src/monitor/message-handler/prepare.test.ts b/extensions/slack/src/monitor/message-handler/prepare.test.ts index 199407911483..157ec54b3288 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.test.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.test.ts @@ -434,6 +434,58 @@ describe("slack prepareSlackMessage inbound contract", () => { }); }); + it("applies workspace-qualified channel users during message ingress", async () => { + const channelsConfig = { + "team:T123ENTERPRISE:channel:C123CHANNEL": { + enabled: true, + requireMention: false, + users: ["team:T123ENTERPRISE:user:U123"], + }, + "team:T456ENTERPRISE:channel:C123CHANNEL": { + enabled: true, + requireMention: false, + users: ["team:T456ENTERPRISE:user:U456"], + }, + }; + const ctx = createInboundSlackCtx({ + cfg: { channels: { slack: { enabled: true, groupPolicy: "allowlist" } } }, + channelsConfig, + defaultRequireMention: false, + groupPolicy: "allowlist", + }); + ctx.resolveChannelName = async () => ({ name: "general", type: "channel" }); + ctx.resolveUserName = async () => ({ name: "Alice" }); + const account = createSlackAccount({ groupPolicy: "allowlist", channels: channelsConfig }); + const message = createSlackMessage({ + channel: "C123CHANNEL", + channel_type: "channel", + user: "U123", + text: "hello", + }); + + const allowed = await prepareSlackMessage({ + ctx, + account, + message, + opts: { + source: "message", + eventScope: { teamId: "T123ENTERPRISE", client: ctx.app.client }, + }, + }); + const blocked = await prepareSlackMessage({ + ctx, + account, + message, + opts: { + source: "message", + eventScope: { teamId: "T456ENTERPRISE", client: ctx.app.client }, + }, + }); + + assertPrepared(allowed, "workspace-qualified channel user"); + expect(blocked).toBeNull(); + }); + it("applies workspace-qualified Enterprise mention pattern policy", async () => { const cfg = { messages: { groupChat: { mentionPatterns: ["\\bbill\\b"] } }, diff --git a/extensions/slack/src/monitor/message-handler/prepare.ts b/extensions/slack/src/monitor/message-handler/prepare.ts index d11ee3f60274..77d482de6f0b 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.ts @@ -521,6 +521,8 @@ async function resolveSlackConversationContext(params: { const isRoomish = isRoom || isGroupDm; const channelConfig = isRoom ? resolveSlackChannelConfig({ + teamId: params.eventScope?.teamId ?? ctx.teamId, + allowUnscoped: ctx.installationIdentity?.kind !== "enterprise", channelId: message.channel, channelName, channels: ctx.channelsConfig, @@ -585,6 +587,7 @@ async function authorizeSlackInboundMessage(params: { if ( !ctx.isChannelAllowed({ + teamId: params.eventScope?.teamId ?? ctx.teamId, channelId: message.channel, channelName, channelType: resolvedChannelType, @@ -1075,6 +1078,7 @@ export async function prepareSlackMessage(params: { isRoom && Array.isArray(channelConfig?.users) && channelConfig.users.length > 0; const messageIngress = await resolveSlackCommandIngress({ ctx, + teamId: opts.eventScope?.teamId ?? ctx.teamId, senderId, senderName: senderNameForAuth, channelType: conversation.resolvedChannelType ?? "channel", @@ -1693,7 +1697,7 @@ export async function prepareSlackMessage(params: { const pinnedMainDmOwner = isDirectMessage ? resolvePinnedMainDmOwnerFromAllowlist({ dmScope: cfg.session?.dmScope, - allowFrom: ctx.allowFrom, + allowFrom: allowFromLower, normalizeEntry: normalizeSlackAllowOwnerEntry, }) : null; diff --git a/extensions/slack/src/monitor/monitor.test.ts b/extensions/slack/src/monitor/monitor.test.ts index 5e0de04909d5..9b319f7f92eb 100644 --- a/extensions/slack/src/monitor/monitor.test.ts +++ b/extensions/slack/src/monitor/monitor.test.ts @@ -162,6 +162,93 @@ describe("resolveSlackChannelConfig", () => { }); }); + it("prefers a workspace-qualified channel over the same channel ID in another workspace", () => { + const channels = { + "team:T11111111:channel:C01234567": { enabled: true, requireMention: false }, + "team:T22222222:channel:C01234567": { enabled: false, requireMention: true }, + }; + + expectSlackChannelConfig( + resolveSlackChannelConfig({ + teamId: "T11111111", + channelId: "C01234567", + channels, + }), + { + allowed: true, + requireMention: false, + matchKey: "team:T11111111:channel:C01234567", + matchSource: "direct", + }, + ); + expectSlackChannelConfig( + resolveSlackChannelConfig({ + teamId: "T22222222", + channelId: "C01234567", + channels, + }), + { + allowed: false, + requireMention: true, + matchKey: "team:T22222222:channel:C01234567", + matchSource: "direct", + }, + ); + }); + + it("does not match a bare channel ID when workspace scope is required", () => { + const channels = { C01234567: { enabled: true, requireMention: false } }; + + expectSlackChannelConfig( + resolveSlackChannelConfig({ + teamId: "T11111111", + channelId: "C01234567", + channels, + }), + { allowed: false, requireMention: true }, + ); + expectSlackChannelConfig( + resolveSlackChannelConfig({ + teamId: "T11111111", + allowUnscoped: true, + channelId: "C01234567", + channels, + }), + { + allowed: true, + requireMention: false, + matchKey: "C01234567", + matchSource: "direct", + }, + ); + }); + + it("matches per-channel users only in their selected workspace", () => { + const channels = { + "team:T11111111:channel:C01234567": { + users: ["team:T11111111:user:U01234567", "team:T22222222:user:U12345678", "U23456789"], + }, + "team:T22222222:channel:C01234567": { + users: ["team:T11111111:user:U01234567", "team:T22222222:user:U12345678", "U23456789"], + }, + }; + + expect( + resolveSlackChannelConfig({ + teamId: "T11111111", + channelId: "C01234567", + channels, + })?.users, + ).toEqual(["team:t11111111:user:u01234567", "team:t22222222:user:u12345678", "u23456789"]); + expect( + resolveSlackChannelConfig({ + teamId: "T22222222", + channelId: "C01234567", + channels, + })?.users, + ).toEqual(["team:t11111111:user:u01234567", "team:t22222222:user:u12345678", "u23456789"]); + }); + it("blocks channel-name route matches by default", () => { const res = resolveSlackChannelConfig({ channelId: "C1", diff --git a/extensions/slack/src/monitor/slash.ts b/extensions/slack/src/monitor/slash.ts index 3b4f227cc69e..cf9cd3c4c3f8 100644 --- a/extensions/slack/src/monitor/slash.ts +++ b/extensions/slack/src/monitor/slash.ts @@ -479,6 +479,7 @@ export async function registerSlackMonitorSlashCommands(params: { if ( !ctx.isChannelAllowed({ + teamId: eventScope?.teamId ?? ctx.teamId, channelId: command.channel_id, channelName: channelInfo?.name, channelType, @@ -538,6 +539,8 @@ export async function registerSlackMonitorSlashCommands(params: { if (isRoom) { channelConfig = resolveSlackChannelConfig({ + teamId: eventScope?.teamId ?? ctx.teamId, + allowUnscoped: ctx.installationIdentity?.kind !== "enterprise", channelId: command.channel_id, channelName: channelInfo?.name, channels: ctx.channelsConfig, @@ -579,6 +582,7 @@ export async function registerSlackMonitorSlashCommands(params: { const senderName = sender?.name ?? command.user_name ?? command.user_id; const slashIngress = await resolveSlackCommandIngress({ ctx, + teamId: eventScope?.teamId ?? ctx.teamId, senderId: command.user_id, senderName, channelType: channelType ?? "channel", diff --git a/extensions/slack/src/resolve-channels.test.ts b/extensions/slack/src/resolve-channels.test.ts index 132b2054323b..b9d9157393f9 100644 --- a/extensions/slack/src/resolve-channels.test.ts +++ b/extensions/slack/src/resolve-channels.test.ts @@ -45,6 +45,21 @@ describe("resolveSlackChannelAllowlist", () => { expect(list).not.toHaveBeenCalled(); }); + it("preserves workspace-qualified channel ids without listing a workspace", async () => { + const list = vi.fn(); + const res = await resolveSlackChannelAllowlist({ + token: "xoxb-test", + entries: ["team:T11111111:channel:C01234567", "team:T22222222:channel:C01234567"], + client: { conversations: { list } } as never, + }); + + expect(res.map((entry) => entry.id)).toEqual([ + "team:T11111111:channel:C01234567", + "team:T22222222:channel:C01234567", + ]); + expect(list).not.toHaveBeenCalled(); + }); + it("resolves by name and prefers active channels", async () => { const client = { conversations: { diff --git a/extensions/slack/src/resolve-channels.ts b/extensions/slack/src/resolve-channels.ts index f66f45363c73..cee5cc0b5a5d 100644 --- a/extensions/slack/src/resolve-channels.ts +++ b/extensions/slack/src/resolve-channels.ts @@ -4,6 +4,7 @@ import { resolveDirectoryAllowlistEntries } from "openclaw/plugin-sdk/directory- import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime"; import { createSlackLookupClient } from "./client.js"; import { collectSlackCursorPages } from "./cursor-pages.js"; +import { formatSlackTarget, parseSlackTarget } from "./target-parsing.js"; export type SlackChannelLookup = { id: string; @@ -20,6 +21,25 @@ export type SlackChannelResolution = { archived?: boolean; }; +function resolveWorkspaceQualifiedChannel(input: string): SlackChannelResolution | undefined { + if (!/^team:/i.test(input)) { + return undefined; + } + try { + const target = parseSlackTarget(input); + if (target?.kind !== "channel" || !target.teamId) { + return undefined; + } + return { + input, + resolved: true, + id: formatSlackTarget({ teamId: target.teamId, kind: "channel", id: target.id }), + }; + } catch { + return undefined; + } +} + function parseSlackChannelMention(raw: string): { id?: string; name?: string } { const trimmed = raw.trim(); if (!trimmed) { @@ -90,26 +110,35 @@ export async function resolveSlackChannelAllowlist(params: { entries: string[]; client?: WebClient; }): Promise { - const parsedEntries = params.entries.map((input) => ({ + const workspaceResolved = params.entries.map(resolveWorkspaceQualifiedChannel); + const lookupEntries = params.entries.filter((_, index) => !workspaceResolved[index]); + if (lookupEntries.length === 0) { + return workspaceResolved.filter( + (entry): entry is SlackChannelResolution => entry !== undefined, + ); + } + const parsedEntries = lookupEntries.map((input) => ({ input, parsed: parseSlackChannelMention(input), })); if (parsedEntries.every((entry) => Boolean(entry.parsed.id))) { - return parsedEntries.map(({ input, parsed }) => ({ + const resolved = parsedEntries.map(({ input, parsed }) => ({ input, resolved: true, id: parsed.id, name: parsed.name, })); + let resolvedIndex = 0; + return workspaceResolved.map((entry) => entry ?? resolved[resolvedIndex++]!); } const client = params.client ?? createSlackLookupClient(params.token); const channels = await listSlackChannels(client); - return resolveDirectoryAllowlistEntries< + const resolved = resolveDirectoryAllowlistEntries< { id?: string; name?: string }, SlackChannelLookup, SlackChannelResolution >({ - entries: params.entries, + entries: lookupEntries, lookup: channels, parseInput: parseSlackChannelMention, findById: (lookup, id) => lookup.find((channel) => channel.id === id), @@ -138,4 +167,6 @@ export async function resolveSlackChannelAllowlist(params: { }, buildUnresolved: (input) => ({ input, resolved: false }), }); + let resolvedIndex = 0; + return workspaceResolved.map((entry) => entry ?? resolved[resolvedIndex++]!); } diff --git a/extensions/slack/src/resolve-users.test.ts b/extensions/slack/src/resolve-users.test.ts index d3b3a5cd2d13..9d5c5c1f53c4 100644 --- a/extensions/slack/src/resolve-users.test.ts +++ b/extensions/slack/src/resolve-users.test.ts @@ -75,6 +75,21 @@ describe("resolveSlackUserAllowlist", () => { }); }); + it("preserves workspace-qualified user ids without listing a workspace", async () => { + const list = vi.fn(); + const res = await resolveSlackUserAllowlist({ + token: "xoxb-test", + entries: ["team:T11111111:user:U01234567", "team:T22222222:user:U01234567"], + client: { users: { list } } as never, + }); + + expect(res.map((entry) => entry.id)).toEqual([ + "team:T11111111:user:U01234567", + "team:T22222222:user:U01234567", + ]); + expect(list).not.toHaveBeenCalled(); + }); + it("keeps unresolved users", async () => { const client = { users: { diff --git a/extensions/slack/src/resolve-users.ts b/extensions/slack/src/resolve-users.ts index 4c97a76914ec..01b52ad2856b 100644 --- a/extensions/slack/src/resolve-users.ts +++ b/extensions/slack/src/resolve-users.ts @@ -7,6 +7,7 @@ import { } from "openclaw/plugin-sdk/string-coerce-runtime"; import { createSlackLookupClient } from "./client.js"; import { collectSlackCursorPages } from "./cursor-pages.js"; +import { formatSlackTarget, parseSlackTarget } from "./target-parsing.js"; export type SlackUserLookup = { id: string; @@ -30,6 +31,25 @@ export type SlackUserResolution = { note?: string; }; +function resolveWorkspaceQualifiedUser(input: string): SlackUserResolution | undefined { + if (!/^team:/i.test(input)) { + return undefined; + } + try { + const target = parseSlackTarget(input); + if (target?.kind !== "user" || !target.teamId) { + return undefined; + } + return { + input, + resolved: true, + id: formatSlackTarget({ teamId: target.teamId, kind: "user", id: target.id }), + }; + } catch { + return undefined; + } +} + function parseSlackUserInput(raw: string): { id?: string; name?: string; email?: string } { const trimmed = raw.trim(); if (!trimmed) { @@ -138,14 +158,19 @@ export async function resolveSlackUserAllowlist(params: { entries: string[]; client?: WebClient; }): Promise { + const workspaceResolved = params.entries.map(resolveWorkspaceQualifiedUser); + const lookupEntries = params.entries.filter((_, index) => !workspaceResolved[index]); + if (lookupEntries.length === 0) { + return workspaceResolved.filter((entry): entry is SlackUserResolution => entry !== undefined); + } const client = params.client ?? createSlackLookupClient(params.token); const users = await listSlackUsers(client); - return resolveDirectoryAllowlistEntries< + const resolved = resolveDirectoryAllowlistEntries< { id?: string; name?: string; email?: string }, SlackUserLookup, SlackUserResolution >({ - entries: params.entries, + entries: lookupEntries, lookup: users, parseInput: parseSlackUserInput, findById: (lookup, id) => lookup.find((user) => user.id === id), @@ -181,4 +206,6 @@ export async function resolveSlackUserAllowlist(params: { }, buildUnresolved: (input) => ({ input, resolved: false }), }); + let resolvedIndex = 0; + return workspaceResolved.map((entry) => entry ?? resolved[resolvedIndex++]!); } diff --git a/extensions/slack/src/security-doctor.ts b/extensions/slack/src/security-doctor.ts index 28398cf40cb4..85d21c614b82 100644 --- a/extensions/slack/src/security-doctor.ts +++ b/extensions/slack/src/security-doctor.ts @@ -1,7 +1,22 @@ // Slack plugin module implements security doctor behavior. import { buildMutableAllowEntryDetector } from "openclaw/plugin-sdk/channel-policy"; +import { parseSlackTarget } from "./target-parsing.js"; -export const isSlackMutableAllowEntry = buildMutableAllowEntryDetector({ +const isSlackMutableUnqualifiedAllowEntry = buildMutableAllowEntryDetector({ stableIdPattern: /^(?:(?:(?:[sS][lL][aA][cC][kK]|[uU][sS][eE][rR]):)?(?:[UWBCGDT][A-Z0-9]{2,}|[A-Za-z0-9]{8,})|<@[A-Za-z0-9]{8,}>)$/, }); + +export function isSlackMutableAllowEntry(entry: string): boolean { + if (/^team:/i.test(entry)) { + try { + const target = parseSlackTarget(entry); + if (target?.kind === "user" && target.teamId) { + return false; + } + } catch { + // Invalid qualified entries remain mutable so Doctor reports them. + } + } + return isSlackMutableUnqualifiedAllowEntry(entry); +} diff --git a/extensions/slack/src/target-parsing.ts b/extensions/slack/src/target-parsing.ts index 73b6415d7a9f..f8caaef9ba83 100644 --- a/extensions/slack/src/target-parsing.ts +++ b/extensions/slack/src/target-parsing.ts @@ -20,7 +20,7 @@ export type SlackTargetParseOptions = MessagingTargetParseOptions; // Letter-leading folded IDs are indistinguishable from supported channel names. // Doctor reports that ambiguity; runtime repairs only the digit-leading form. const SLACK_CHANNEL_API_ID_RE = /^[CDG][0-9][A-Z0-9]{7,}$/i; -const SLACK_USER_API_ID_RE = /^[UW][A-Z0-9]{8,}$/i; +const SLACK_USER_API_ID_RE = /^[BUW][A-Z0-9]{8,}$/i; const SLACK_QUALIFIED_TARGET_RE = /^team:([^:]+):(user|channel):([^:]+)$/i; function decodeSlackTargetPart(raw: string): string | undefined { @@ -44,7 +44,7 @@ function parseQualifiedSlackTarget(raw: string): SlackTarget | undefined { const teamId = decodeSlackTargetPart(match[1] ?? ""); const kind = match[2]?.toLowerCase() as SlackTargetKind | undefined; const id = decodeSlackTargetPart(match[3] ?? ""); - const idPattern = kind === "user" ? /^[UW][A-Z0-9]+$/i : /^[CDG][A-Z0-9]+$/i; + const idPattern = kind === "user" ? /^[BUW][A-Z0-9]+$/i : /^[CDG][A-Z0-9]+$/i; if (!teamId || !/^T[A-Z0-9]+$/i.test(teamId) || !kind || !id || !idPattern.test(id)) { throw new Error("Invalid Slack workspace-qualified target"); } @@ -68,7 +68,7 @@ export function formatSlackTarget(params: { if (!teamId) { return params.explicitKind ? `${params.kind}:${id}` : id; } - const idPattern = params.kind === "user" ? /^[UW][A-Z0-9]+$/i : /^[CDG][A-Z0-9]+$/i; + const idPattern = params.kind === "user" ? /^[BUW][A-Z0-9]+$/i : /^[CDG][A-Z0-9]+$/i; if (!/^T[A-Z0-9]+$/i.test(teamId) || !idPattern.test(id)) { throw new Error("Invalid Slack workspace-qualified target"); } diff --git a/extensions/slack/src/targets.test.ts b/extensions/slack/src/targets.test.ts index 50d269b2114a..e2a8676fafc4 100644 --- a/extensions/slack/src/targets.test.ts +++ b/extensions/slack/src/targets.test.ts @@ -65,6 +65,13 @@ describe("parseSlackTarget", () => { raw: "team:T789:user:U012", normalized: "team:t789:user:u012", }); + expect(parseSlackTarget("team:T789:user:B345")).toEqual({ + kind: "user", + id: "B345", + teamId: "T789", + raw: "team:T789:user:B345", + normalized: "team:t789:user:b345", + }); }); it("formats bare and structurally valid workspace-qualified targets", () => { @@ -72,6 +79,9 @@ describe("parseSlackTarget", () => { "team:T123:channel:C456", ); expect(formatSlackTarget({ kind: "channel", id: "C456" })).toBe("C456"); + expect(formatSlackTarget({ teamId: "T123", kind: "user", id: "B456" })).toBe( + "team:T123:user:B456", + ); expect(() => formatSlackTarget({ teamId: "E123", kind: "channel", id: "C456" })).toThrow( "Invalid Slack workspace-qualified target", );