From 9cbfbd18e35e907b6dcb96ebff9bd03aa72a5732 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 10 Apr 2026 23:26:52 +0100 Subject: [PATCH] fix: resolve scoped group tool policies (#64491) --- src/agents/pi-tools-agent-config.test.ts | 28 ++++++ src/agents/pi-tools.policy.ts | 114 +++++++++++++++++------ src/config/group-policy.ts | 20 +++- 3 files changed, 130 insertions(+), 32 deletions(-) diff --git a/src/agents/pi-tools-agent-config.test.ts b/src/agents/pi-tools-agent-config.test.ts index d1fef30a85a..135b422d2c5 100644 --- a/src/agents/pi-tools-agent-config.test.ts +++ b/src/agents/pi-tools-agent-config.test.ts @@ -514,6 +514,34 @@ describe("Agent-specific tool filtering", () => { expect(names).not.toContain("exec"); }); + it("should prefer scoped group candidates before wildcard tool policy", () => { + const cfg: OpenClawConfig = { + channels: { + feishu: { + groups: { + "*": { + tools: { allow: ["read", "exec"] }, + }, + oc_group_chat: { + tools: { allow: ["read"] }, + }, + }, + }, + }, + }; + + const tools = createOpenClawCodingTools({ + config: cfg, + sessionKey: "agent:main:feishu:group:oc_group_chat:topic:om_topic_root:sender:ou_topic_user", + messageProvider: "feishu", + workspaceDir: "/tmp/test-feishu-wildcard-group", + agentDir: "/tmp/agent-feishu-wildcard", + }); + const names = tools.map((t) => t.name); + expect(names).toContain("read"); + expect(names).not.toContain("exec"); + }); + it("should resolve inherited group tool policy for subagent parent groups", () => { const cfg: OpenClawConfig = { channels: { diff --git a/src/agents/pi-tools.policy.ts b/src/agents/pi-tools.policy.ts index f43f91c34ae..91b986734cf 100644 --- a/src/agents/pi-tools.policy.ts +++ b/src/agents/pi-tools.policy.ts @@ -136,9 +136,47 @@ function normalizeProviderKey(value: string): string { return normalizeLowercaseStringOrEmpty(value); } +function collectUniqueStrings(values: Array): string[] { + const seen = new Set(); + const resolved: string[] = []; + for (const value of values) { + const trimmed = value?.trim(); + if (!trimmed || seen.has(trimmed)) { + continue; + } + seen.add(trimmed); + resolved.push(trimmed); + } + return resolved; +} + +function buildScopedGroupIdCandidates(groupId?: string | null): string[] { + const raw = groupId?.trim(); + if (!raw) { + return []; + } + const topicSenderMatch = raw.match(/^(.+):topic:([^:]+):sender:([^:]+)$/i); + if (topicSenderMatch) { + const [, chatId, topicId] = topicSenderMatch; + // Sender-scoped sessions still inherit topic/base group tool policies. + return collectUniqueStrings([raw, `${chatId}:topic:${topicId}`, chatId]); + } + const topicMatch = raw.match(/^(.+):topic:([^:]+)$/i); + if (topicMatch) { + const [, chatId, topicId] = topicMatch; + return collectUniqueStrings([`${chatId}:topic:${topicId}`, chatId]); + } + const senderMatch = raw.match(/^(.+):sender:([^:]+)$/i); + if (senderMatch) { + const [, chatId] = senderMatch; + return collectUniqueStrings([raw, chatId]); + } + return [raw]; +} + function resolveGroupContextFromSessionKey(sessionKey?: string | null): { channel?: string; - groupId?: string; + groupIds?: string[]; } { const raw = (sessionKey ?? "").trim(); if (!raw) { @@ -148,21 +186,22 @@ function resolveGroupContextFromSessionKey(sessionKey?: string | null): { const conversationKey = threadId ? baseSessionKey : raw; const conversation = parseRawSessionConversationRef(conversationKey); if (conversation) { - if (/:(?:sender|thread|topic):/iu.test(conversation.rawId)) { - const resolvedConversation = resolveSessionConversation({ - channel: conversation.channel, - kind: conversation.kind, - rawId: conversation.rawId, - }); - const groupId = resolvedConversation?.baseConversationId; - if (groupId) { - return { + const resolvedConversation = /:(?:sender|thread|topic):/iu.test(conversation.rawId) + ? resolveSessionConversation({ channel: conversation.channel, - groupId, - }; - } - } - return { channel: conversation.channel, groupId: conversation.rawId }; + kind: conversation.kind, + rawId: conversation.rawId, + }) + : null; + return { + channel: conversation.channel, + groupIds: collectUniqueStrings([ + ...buildScopedGroupIdCandidates(conversation.rawId), + resolvedConversation?.id, + resolvedConversation?.baseConversationId, + ...(resolvedConversation?.parentConversationCandidates ?? []), + ]), + }; } const base = conversationKey ?? raw; const parts = base.split(":").filter(Boolean); @@ -181,7 +220,10 @@ function resolveGroupContextFromSessionKey(sessionKey?: string | null): { if (!groupId) { return {}; } - return { channel: normalizeLowercaseStringOrEmpty(channel), groupId }; + return { + channel: normalizeLowercaseStringOrEmpty(channel), + groupIds: buildScopedGroupIdCandidates(groupId), + }; } function resolveProviderToolPolicy(params: { @@ -331,8 +373,12 @@ export function resolveGroupToolPolicy(params: { } const sessionContext = resolveGroupContextFromSessionKey(params.sessionKey); const spawnedContext = resolveGroupContextFromSessionKey(params.spawnedBy); - const groupId = params.groupId ?? sessionContext.groupId ?? spawnedContext.groupId; - if (!groupId) { + const groupIds = collectUniqueStrings([ + ...buildScopedGroupIdCandidates(params.groupId), + ...(sessionContext.groupIds ?? []), + ...(spawnedContext.groupIds ?? []), + ]); + if (groupIds.length === 0) { return undefined; } const channelRaw = params.messageProvider ?? sessionContext.channel ?? spawnedContext.channel; @@ -346,8 +392,8 @@ export function resolveGroupToolPolicy(params: { } catch { plugin = undefined; } - const toolsConfig = - plugin?.groups?.resolveToolPolicy?.({ + for (const groupId of groupIds) { + const toolsConfig = plugin?.groups?.resolveToolPolicy?.({ cfg: params.config, groupId, groupChannel: params.groupChannel, @@ -357,18 +403,24 @@ export function resolveGroupToolPolicy(params: { senderName: params.senderName, senderUsername: params.senderUsername, senderE164: params.senderE164, - }) ?? - resolveChannelGroupToolsPolicy({ - cfg: params.config, - channel, - groupId, - accountId: params.accountId, - senderId: params.senderId, - senderName: params.senderName, - senderUsername: params.senderUsername, - senderE164: params.senderE164, }); - return pickSandboxToolPolicy(toolsConfig); + const policy = pickSandboxToolPolicy(toolsConfig); + if (policy) { + return policy; + } + } + const configTools = resolveChannelGroupToolsPolicy({ + cfg: params.config, + channel, + groupId: groupIds[0], + groupIdCandidates: groupIds.slice(1), + accountId: params.accountId, + senderId: params.senderId, + senderName: params.senderName, + senderUsername: params.senderUsername, + senderE164: params.senderE164, + }); + return pickSandboxToolPolicy(configTools); } export { isToolAllowedByPolicies, isToolAllowedByPolicyName } from "./tool-policy-match.js"; diff --git a/src/config/group-policy.ts b/src/config/group-policy.ts index 5da82587048..00ac4951cbf 100644 --- a/src/config/group-policy.ts +++ b/src/config/group-policy.ts @@ -400,11 +400,29 @@ export function resolveChannelGroupToolsPolicy( cfg: OpenClawConfig; channel: GroupPolicyChannel; groupId?: string | null; + groupIdCandidates?: Array; accountId?: string | null; groupIdCaseInsensitive?: boolean; } & GroupToolPolicySender, ): GroupToolPolicyConfig | undefined { - const { groupConfig, defaultConfig } = resolveChannelGroupPolicy(params); + const groups = resolveChannelGroups(params.cfg, params.channel, params.accountId); + const groupIds = [ + params.groupId, + ...(Array.isArray(params.groupIdCandidates) ? params.groupIdCandidates : []), + ]; + let groupConfig: ChannelGroupConfig | undefined; + for (const rawGroupId of groupIds) { + const groupId = rawGroupId?.trim(); + if (!groupId) { + continue; + } + // Scoped ids can collapse to a parent group; try all exact matches before wildcard fallback. + groupConfig = resolveChannelGroupConfig(groups, groupId, params.groupIdCaseInsensitive); + if (groupConfig) { + break; + } + } + const defaultConfig = groups?.["*"]; const groupSenderPolicy = resolveToolsBySender({ toolsBySender: groupConfig?.toolsBySender, senderId: params.senderId,