From 153c4f088eccc39912082e2d153bc9293ec17ace Mon Sep 17 00:00:00 2001 From: joshavant <830519+joshavant@users.noreply.github.com> Date: Fri, 24 Jul 2026 17:04:26 -0500 Subject: [PATCH] fix(channels): preserve external current target aliases --- .../plugins/message-action-dispatch.ts | 92 +++++++++-- .../plugins/message-actions.security.test.ts | 146 ++++++++++++++++++ ...sage-action-runner.plugin-dispatch.test.ts | 37 +++++ 3 files changed, 264 insertions(+), 11 deletions(-) diff --git a/src/channels/plugins/message-action-dispatch.ts b/src/channels/plugins/message-action-dispatch.ts index 576402b6eb83..829e3129a2f6 100644 --- a/src/channels/plugins/message-action-dispatch.ts +++ b/src/channels/plugins/message-action-dispatch.ts @@ -339,6 +339,65 @@ function hasTargetInput(value: unknown): boolean { return typeof value === "number" && Number.isFinite(value); } +function attachExternalCurrentTargetSibling(params: { + ctx: ChannelMessageActionContext; + plugin: ChannelPlugin; + origin: ServerOwnedConversationReadOrigin; + actionPolicy: ChannelMessageActionReadPolicy; + enforcement: MessageActionReadEnforcement; +}): ChannelMessageActionContext { + if ( + params.origin === "direct-operator" || + params.actionPolicy.kind !== "conversation-read" || + params.enforcement.kind !== "host-exact-current" || + params.enforcement.pluginTrust !== "external" + ) { + return params.ctx; + } + const target = + typeof params.ctx.params.target === "string" ? params.ctx.params.target.trim() : ""; + if (!target) { + return params.ctx; + } + const mirroredTo = params.ctx.params.to; + if (typeof mirroredTo !== "string" || mirroredTo.trim() !== target) { + return params.ctx; + } + const providerPrefixes = params.plugin.messaging?.targetPrefixes; + const requestedTarget = normalizeHostConversationTarget({ + value: target, + channel: params.ctx.channel, + providerPrefixes, + }); + if (!requestedTarget) { + return params.ctx; + } + const trustedCurrentTarget = [ + params.ctx.toolContext?.currentMessagingTarget, + params.ctx.toolContext?.currentChannelId, + ].find((value) => { + const normalized = normalizeHostConversationTarget({ + value, + channel: params.ctx.channel, + providerPrefixes, + }); + return ( + normalized?.id === requestedTarget.id && + (!requestedTarget.kind || !normalized.kind || normalized.kind === requestedTarget.kind) + ); + }); + if (typeof trustedCurrentTarget !== "string" || !trustedCurrentTarget.trim()) { + return params.ctx; + } + return { + ...params.ctx, + params: { + ...params.ctx.params, + to: trustedCurrentTarget.trim(), + }, + }; +} + function isExactCurrentConversation(params: { ctx: ChannelMessageActionContext; plugin: ChannelPlugin; @@ -554,31 +613,42 @@ export async function dispatchChannelMessageAction( // Plugins receive only the closed server-normalized classification. conversationReadOrigin: origin, }; - enforceMessageActionConversationReadGate({ + const enforcement = resolveMessageActionReadEnforcement({ + action: actionContext.action, + channel: actionContext.channel, + pluginOrigin: registration.origin, + }); + const authorizedActionContext = attachExternalCurrentTargetSibling({ ctx: actionContext, plugin, origin, actionPolicy, - enforcement: resolveMessageActionReadEnforcement({ - action: actionContext.action, - channel: actionContext.channel, - pluginOrigin: registration.origin, - }), + enforcement, + }); + enforceMessageActionConversationReadGate({ + ctx: authorizedActionContext, + plugin, + origin, + actionPolicy, + enforcement, }); // Some plugin actions depend on the sender identity to enforce channel-local // trust. Reject tool-driven calls before invoking the action without it. if ( - requiresTrustedRequesterSender(actionContext, plugin) && - !actionContext.requesterSenderId?.trim() + requiresTrustedRequesterSender(authorizedActionContext, plugin) && + !authorizedActionContext.requesterSenderId?.trim() ) { throw new Error( - `Trusted sender identity is required for ${actionContext.channel}:${actionContext.action} in tool-driven contexts.`, + `Trusted sender identity is required for ${authorizedActionContext.channel}:${authorizedActionContext.action} in tool-driven contexts.`, ); } // `handleAction` may be broad; `supportsAction` lets plugins cheaply decline // action names before the dispatcher enters channel-specific behavior. - if (actions.supportsAction && !actions.supportsAction({ action: actionContext.action })) { + if ( + actions.supportsAction && + !actions.supportsAction({ action: authorizedActionContext.action }) + ) { return null; } - return await actions.handleAction(actionContext); + return await actions.handleAction(authorizedActionContext); } diff --git a/src/channels/plugins/message-actions.security.test.ts b/src/channels/plugins/message-actions.security.test.ts index 43a87b3c325c..0f5da9247de0 100644 --- a/src/channels/plugins/message-actions.security.test.ts +++ b/src/channels/plugins/message-actions.security.test.ts @@ -346,6 +346,30 @@ describe("dispatchChannelMessageAction conversation-read provenance", () => { expect(handleAction).not.toHaveBeenCalled(); }); + it("does not rewrite an untyped channelId mirror from a typed user target", async () => { + setReadPlugin(); + + await expect( + dispatchChannelMessageAction({ + channel: "discord", + action: "read", + cfg: {} as OpenClawConfig, + params: { + target: "123", + channelId: "123", + }, + accountId: "default", + requesterAccountId: "default", + conversationReadOrigin: "delegated", + toolContext: { + currentChannelProvider: "discord", + currentMessagingTarget: "user:123", + }, + }), + ).rejects.toThrow("requires the exact current conversation and account"); + expect(handleAction).not.toHaveBeenCalled(); + }); + it("keeps external conversation ids case-sensitive after prefix normalization", async () => { setReadPlugin(); @@ -671,6 +695,128 @@ describe("dispatchChannelMessageAction conversation-read provenance", () => { }, ); + it("replaces an external normalization mirror with the trusted current target", async () => { + setReadPlugin({ + channel: "nextcloud-talk", + origin: "workspace", + targetPrefixes: ["nextcloud-talk", "nc-talk", "nc"], + }); + + await dispatchChannelMessageAction({ + channel: "nextcloud-talk", + action: "read", + cfg: {} as OpenClawConfig, + params: { + target: "room:current", + to: "room:current", + }, + accountId: "default", + requesterAccountId: "default", + conversationReadOrigin: "delegated", + toolContext: { + currentChannelProvider: "nextcloud-talk", + currentChannelId: "nextcloud-talk:current", + currentChatType: "group", + }, + }); + + expect(handleAction.mock.calls[0]?.[0].params).toMatchObject({ + target: "nextcloud-talk:current", + to: "nextcloud-talk:current", + }); + expect(handleAction).toHaveBeenCalledOnce(); + }); + + it("rejects a conflicting trusted target kind even when an untyped sibling exists", async () => { + setReadPlugin({ + channel: "nextcloud-talk", + origin: "workspace", + targetPrefixes: ["nextcloud-talk", "nc-talk", "nc"], + }); + + await expect( + dispatchChannelMessageAction({ + channel: "nextcloud-talk", + action: "read", + cfg: {} as OpenClawConfig, + params: { + target: "room:current", + to: "room:current", + }, + accountId: "default", + requesterAccountId: "default", + conversationReadOrigin: "delegated", + toolContext: { + currentChannelProvider: "nextcloud-talk", + currentChannelId: "nextcloud-talk:current", + currentMessagingTarget: "channel:current", + currentChatType: "group", + }, + }), + ).rejects.toThrow("requires the exact current conversation and account"); + expect(handleAction).not.toHaveBeenCalled(); + }); + + it("rejects an external normalization mirror for a different conversation", async () => { + setReadPlugin({ + channel: "nextcloud-talk", + origin: "workspace", + targetPrefixes: ["nextcloud-talk", "nc-talk", "nc"], + }); + + await expect( + dispatchChannelMessageAction({ + channel: "nextcloud-talk", + action: "read", + cfg: {} as OpenClawConfig, + params: { + target: "room:other", + to: "room:other", + }, + accountId: "default", + requesterAccountId: "default", + conversationReadOrigin: "delegated", + toolContext: { + currentChannelProvider: "nextcloud-talk", + currentChannelId: "nextcloud-talk:current", + currentChatType: "group", + }, + }), + ).rejects.toThrow("requires the exact current conversation and account"); + expect(handleAction).not.toHaveBeenCalled(); + }); + + it("does not rewrite direct external targets from current context", async () => { + setReadPlugin({ + channel: "nextcloud-talk", + origin: "workspace", + targetPrefixes: ["nextcloud-talk", "nc-talk", "nc"], + }); + + await dispatchChannelMessageAction({ + channel: "nextcloud-talk", + action: "read", + cfg: {} as OpenClawConfig, + params: { + target: "room:other", + to: "room:other", + }, + accountId: "default", + conversationReadOrigin: "direct-operator", + toolContext: { + currentChannelProvider: "nextcloud-talk", + currentChannelId: "nextcloud-talk:current", + currentChatType: "group", + }, + }); + + expect(handleAction.mock.calls[0]?.[0].params).toMatchObject({ + target: "room:other", + to: "room:other", + }); + expect(handleAction).toHaveBeenCalledOnce(); + }); + it("does not let an external provider prefix erase a conflicting target kind", async () => { setReadPlugin({ channel: "nextcloud-talk", diff --git a/src/infra/outbound/message-action-runner.plugin-dispatch.test.ts b/src/infra/outbound/message-action-runner.plugin-dispatch.test.ts index 2990a9d9889a..3fe7b82fdee0 100644 --- a/src/infra/outbound/message-action-runner.plugin-dispatch.test.ts +++ b/src/infra/outbound/message-action-runner.plugin-dispatch.test.ts @@ -739,6 +739,43 @@ describe("runMessageAction plugin dispatch", () => { ); }); + it("preserves a trusted canonical sibling for a typed external current target", async () => { + await runMessageAction({ + cfg: { + channels: { + actionhub: { + enabled: true, + }, + }, + } as OpenClawConfig, + action: "pin", + params: { + channel: "actionhub", + target: "channel:current", + messageId: "om_123", + }, + defaultAccountId: "default", + requesterAccountId: "default", + conversationReadOrigin: "delegated", + toolContext: { + currentChannelId: "actionhub:current", + currentChannelProvider: "actionhub", + currentChatType: "channel", + }, + dryRun: false, + }); + + const call = readFirstPluginCall(handleAction); + expectRecordFields( + readRecordField(call, "params", "normalized plugin params"), + { + target: "actionhub:current", + to: "actionhub:current", + }, + "normalized plugin params", + ); + }); + it("preserves no-context owner Discord admin actions through the shared runner", async () => { const handleDiscordAction = vi.fn(async (ctx: ChannelMessageActionContext) => { const currentProvider = ctx.toolContext?.currentChannelProvider?.trim().toLowerCase();