From d8eb6aa8d3b6640792db63f97cc7f948eef01d11 Mon Sep 17 00:00:00 2001 From: joshavant <830519+joshavant@users.noreply.github.com> Date: Fri, 24 Jul 2026 17:22:13 -0500 Subject: [PATCH] fix(messages): authorize external targets before resolution --- .../plugins/message-action-dispatch.ts | 114 ++++++++++++++--- ...sage-action-runner.plugin-dispatch.test.ts | 121 ++++++++++++++++++ src/infra/outbound/message-action-runner.ts | 55 ++++++-- 3 files changed, 261 insertions(+), 29 deletions(-) diff --git a/src/channels/plugins/message-action-dispatch.ts b/src/channels/plugins/message-action-dispatch.ts index 829e3129a2f6..43057758251f 100644 --- a/src/channels/plugins/message-action-dispatch.ts +++ b/src/channels/plugins/message-action-dispatch.ts @@ -43,6 +43,14 @@ type ChannelMessageActionDispatchContext = Omit; + enforcement: Extract & { + pluginTrust: "external"; + }; +} { + return Boolean( + prepared && + prepared.origin !== "direct-operator" && + prepared.actionPolicy.kind === "conversation-read" && + prepared.enforcement.kind === "host-exact-current" && + prepared.enforcement.pluginTrust === "external", + ); +} + /** The sole host chokepoint before any read-capable plugin callback runs. */ function enforceMessageActionConversationReadGate(params: { ctx: ChannelMessageActionContext; @@ -573,6 +629,40 @@ function enforceMessageActionConversationReadGate(params: { } } +/** Authorizes and canonicalizes external exact-current targets before target resolution. */ +export function prepareExternalMessageActionTargetForResolution( + ctx: ChannelMessageActionDispatchContext, +): Record { + const prepared = prepareMessageActionReadContext(ctx); + if (!isExternalDelegatedMessageActionRead(prepared)) { + return ctx.params; + } + // External target resolution can execute plugin directory/provider lookups. + // Establish exact-current authority before that boundary, then recheck at dispatch. + const authorizedActionContext = attachExternalCurrentTargetSibling({ + ctx: prepared.actionContext, + plugin: prepared.plugin, + origin: prepared.origin, + actionPolicy: prepared.actionPolicy, + enforcement: prepared.enforcement, + }); + enforceMessageActionConversationReadGate({ + ctx: authorizedActionContext, + plugin: prepared.plugin, + origin: prepared.origin, + actionPolicy: prepared.actionPolicy, + enforcement: prepared.enforcement, + }); + return authorizedActionContext.params; +} + +/** Defers delegated external target interpretation to the attested Gateway boundary. */ +export function shouldDeferExternalMessageActionTargetResolution( + ctx: ChannelMessageActionDispatchContext, +): boolean { + return isExternalDelegatedMessageActionRead(prepareMessageActionReadContext(ctx)); +} + function requiresTrustedRequesterSender( ctx: ChannelMessageActionContext, plugin: ChannelPlugin, @@ -591,33 +681,15 @@ function requiresTrustedRequesterSender( export async function dispatchChannelMessageAction( ctx: ChannelMessageActionDispatchContext, ): Promise | null> { - const actionPolicy = resolveChannelMessageActionReadPolicy(ctx.action); - if (!actionPolicy) { + const prepared = prepareMessageActionReadContext(ctx); + if (!prepared) { return null; } - // The policy lookup is the runtime proof that this is a core-owned action. - const action = ctx.action as ChannelMessageActionName; - const registration = resolveChannelPluginRegistration(ctx.channel); - if (!registration) { - return null; - } - const { plugin } = registration; + const { actionContext, plugin, origin, actionPolicy, enforcement } = prepared; const actions = plugin.actions; if (!actions?.handleAction) { return null; } - const origin = resolveServerOwnedConversationReadOrigin(ctx.conversationReadOrigin); - const actionContext: ChannelMessageActionContext = { - ...ctx, - action, - // Plugins receive only the closed server-normalized classification. - conversationReadOrigin: origin, - }; - const enforcement = resolveMessageActionReadEnforcement({ - action: actionContext.action, - channel: actionContext.channel, - pluginOrigin: registration.origin, - }); const authorizedActionContext = attachExternalCurrentTargetSibling({ ctx: actionContext, plugin, 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 3fe7b82fdee0..a5a80c2616fc 100644 --- a/src/infra/outbound/message-action-runner.plugin-dispatch.test.ts +++ b/src/infra/outbound/message-action-runner.plugin-dispatch.test.ts @@ -776,6 +776,120 @@ describe("runMessageAction plugin dispatch", () => { ); }); + it("canonicalizes an external exact-current alias before legacy target resolution", async () => { + const looksLikeId = vi.fn((raw: string) => !/^room:/i.test(raw)); + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "actionhub", + source: "test", + origin: "config", + plugin: { + ...actionHubPlugin, + messaging: { + targetPrefixes: ["actionhub"], + normalizeTarget: (raw: string) => + raw.replace(/^room:/i, "actionhub:").replace(/^actionhub:/i, "actionhub:"), + targetResolver: { + looksLikeId, + }, + }, + }, + }, + ]), + ); + + await runMessageAction({ + cfg: { + channels: { + actionhub: { + enabled: true, + }, + }, + } as OpenClawConfig, + action: "pin", + params: { + channel: "actionhub", + target: "room:current", + messageId: "om_123", + }, + defaultAccountId: "default", + requesterAccountId: "default", + conversationReadOrigin: "delegated", + toolContext: { + currentChannelId: "actionhub:current", + currentChannelProvider: "actionhub", + currentChatType: "group", + }, + dryRun: false, + }); + + expect(looksLikeId).toHaveBeenCalledWith("actionhub:current", "actionhub:current"); + const call = readFirstPluginCall(handleAction); + expectRecordFields( + readRecordField(call, "params", "normalized plugin params"), + { + target: "actionhub:current", + to: "actionhub:current", + }, + "normalized plugin params", + ); + }); + + it.each([false, true])( + "rejects an external exact-current alias with the wrong account before target resolution (dryRun=%s)", + async (dryRun) => { + const looksLikeId = vi.fn(() => true); + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "actionhub", + source: "test", + origin: "config", + plugin: { + ...actionHubPlugin, + messaging: { + ...actionHubPlugin.messaging, + targetResolver: { + looksLikeId, + }, + }, + }, + }, + ]), + ); + + await expect( + runMessageAction({ + cfg: { + channels: { + actionhub: { + enabled: true, + }, + }, + } as OpenClawConfig, + action: "pin", + params: { + channel: "actionhub", + target: "room:current", + messageId: "om_123", + }, + defaultAccountId: "other", + requesterAccountId: "default", + conversationReadOrigin: "delegated", + toolContext: { + currentChannelId: "actionhub:current", + currentChannelProvider: "actionhub", + currentChatType: "group", + }, + dryRun, + }), + ).rejects.toThrow("requires the exact current conversation and account"); + expect(looksLikeId).not.toHaveBeenCalled(); + expect(handleAction).not.toHaveBeenCalled(); + }, + ); + 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(); @@ -892,12 +1006,18 @@ describe("runMessageAction plugin dispatch", () => { local: true, }), ); + const looksLikeId = vi.fn(() => true); const gatewayPlugin = createGatewayActionPlugin({ pluginId: "gatewaychat", label: "Gateway Chat", blurb: "Gateway Chat reaction test plugin.", actions: ["react"], capabilities: { chatTypes: ["direct"], reactions: true }, + messaging: { + targetResolver: { + looksLikeId, + }, + }, handleAction: handleActionEntry, }); setTestPlugin(gatewayPlugin, "gatewaychat"); @@ -965,6 +1085,7 @@ describe("runMessageAction plugin dispatch", () => { expect(gatewayParams).not.toHaveProperty("requesterAccountId"); expect(gatewayParams).not.toHaveProperty("requesterSenderId"); expect(gatewayParams).not.toHaveProperty("toolContext"); + expect(looksLikeId).not.toHaveBeenCalled(); expect(handleActionEntry).not.toHaveBeenCalled(); expectRecordFields( result, diff --git a/src/infra/outbound/message-action-runner.ts b/src/infra/outbound/message-action-runner.ts index 7cb7e45d6613..ea40a2c94a89 100644 --- a/src/infra/outbound/message-action-runner.ts +++ b/src/infra/outbound/message-action-runner.ts @@ -26,7 +26,11 @@ import { normalizeConversationReadInvocationOrigin, type ConversationReadInvocationOrigin, } from "../../channels/plugins/conversation-read-origin.js"; -import { dispatchChannelMessageAction } from "../../channels/plugins/message-action-dispatch.js"; +import { + dispatchChannelMessageAction, + prepareExternalMessageActionTargetForResolution, + shouldDeferExternalMessageActionTargetResolution, +} from "../../channels/plugins/message-action-dispatch.js"; import type { ChannelId, ChannelMessageActionName, @@ -1945,6 +1949,39 @@ export async function runMessageAction( params.accountId = accountId; } const dryRun = Boolean(input.dryRun ?? readBooleanParam(params, "dryRun")); + const delegatesActionToGateway = + Boolean(input.gateway) && + channelPlugin?.actions?.resolveExecutionMode?.({ action }) === "gateway"; + const defersExternalTargetResolution = + delegatesActionToGateway && + shouldDeferExternalMessageActionTargetResolution({ + channel, + action, + cfg, + params, + accountId: accountId ?? undefined, + conversationReadOrigin: normalizeConversationReadInvocationOrigin( + input.conversationReadOrigin, + ), + }); + if (!delegatesActionToGateway) { + const authorization = input.messageActionAuthorization; + params = prepareExternalMessageActionTargetForResolution({ + channel, + action, + cfg, + params, + accountId: accountId ?? undefined, + requesterAccountId: + authorization !== undefined + ? authorization.requesterAccountId + : (input.requesterAccountId ?? undefined), + conversationReadOrigin: normalizeConversationReadInvocationOrigin( + input.conversationReadOrigin, + ), + toolContext: authorization !== undefined ? authorization.toolContext : input.toolContext, + }); + } const normalizationPolicy = resolveAttachmentMediaPolicy({ sandboxRoot: input.sandboxRoot, mediaLocalRoots: getAgentScopedMediaLocalRoots(cfg, resolvedAgentId), @@ -2014,13 +2051,15 @@ export async function runMessageAction( await hydrateActionAttachmentParams(); } - const resolvedTarget = await resolveActionTarget({ - cfg, - channel, - action, - args: params, - accountId, - }); + const resolvedTarget = defersExternalTargetResolution + ? undefined + : await resolveActionTarget({ + cfg, + channel, + action, + args: params, + accountId, + }); enforceCrossContextPolicy({ channel,