From c96bf842701ac3660fde2708fb2ca1a2da3ef644 Mon Sep 17 00:00:00 2001 From: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Date: Fri, 22 May 2026 03:57:37 +0000 Subject: [PATCH] fix(clawsweeper): address review for automerge-openclaw-openclaw-83348 (1) --- .../monitor.inbound-system-event.test.ts | 67 +++++++++++++++++++ .../mattermost/src/mattermost/monitor.ts | 4 +- .../message-handler.authz.test.ts | 33 +++++++++ .../src/monitor-handler/message-handler.ts | 4 +- 4 files changed, 102 insertions(+), 6 deletions(-) diff --git a/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts b/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts index 495db52d56e..b6cbbc70134 100644 --- a/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts +++ b/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts @@ -444,6 +444,73 @@ describe("mattermost inbound user posts", () => { expect(ctx?.Provider).toBe("mattermost"); }); + it("does not drop inline command-looking group text from non-command-authorized senders", async () => { + const socket = new FakeWebSocket(); + const abortController = new AbortController(); + mockState.abortController = abortController; + const inlineCommandConfig: OpenClawConfig = { + commands: { useAccessGroups: true }, + channels: { + mattermost: { + enabled: true, + baseUrl: "https://mattermost.example.com", + botToken: "bot-token", + chatmode: "onmessage", + dmPolicy: "open", + groupPolicy: "open", + }, + }, + }; + const isControlCommandMessage = vi.fn(() => false); + const shouldComputeCommandAuthorized = vi.fn(() => true); + mockState.runtimeCore = createRuntimeCore(inlineCommandConfig, undefined, { + isControlCommandMessage, + shouldComputeCommandAuthorized, + shouldHandleTextCommands: () => true, + }); + + const monitor = monitorMattermostProvider({ + config: inlineCommandConfig, + runtime: testRuntime(), + abortSignal: abortController.signal, + webSocketFactory: () => socket, + }); + + await vi.waitFor(() => { + expect(socket.openListenerCount).toBeGreaterThan(0); + }); + socket.emitOpen(); + + await socket.emitMessage({ + event: "posted", + data: { + channel_id: "chan-1", + channel_name: "town-square", + channel_display_name: "Town Square", + sender_name: "alice", + post: JSON.stringify({ + id: "post-inline-command", + channel_id: "chan-1", + user_id: "user-1", + message: "hello /status", + create_at: 1_714_000_000_000, + }), + }, + broadcast: { + channel_id: "chan-1", + user_id: "user-1", + }, + }); + socket.emitClose(1000); + await monitor; + + expect(isControlCommandMessage).toHaveBeenCalledWith("hello /status", inlineCommandConfig); + expect(mockState.dispatchReplyFromConfig).toHaveBeenCalledTimes(1); + const ctx = mockState.dispatchReplyFromConfig.mock.calls.at(0)?.[0].ctx; + expect(ctx?.BodyForAgent).toBe("hello /status"); + expect(ctx?.CommandAuthorized).toBe(false); + }); + it("uses websocket channel type when REST channel lookup fails", async () => { const socket = new FakeWebSocket(); const abortController = new AbortController(); diff --git a/extensions/mattermost/src/mattermost/monitor.ts b/extensions/mattermost/src/mattermost/monitor.ts index 2a8f5f25bf4..8588f60be40 100644 --- a/extensions/mattermost/src/mattermost/monitor.ts +++ b/extensions/mattermost/src/mattermost/monitor.ts @@ -1320,8 +1320,6 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} }); const isControlCommand = allowTextCommands && core.channel.commands.isControlCommandMessage(rawText, cfg); - const shouldComputeCommandAuthorized = - allowTextCommands && core.channel.commands.shouldComputeCommandAuthorized(rawText, cfg); const accessDecision = await resolveMattermostMonitorInboundAccess({ account, cfg, @@ -1332,7 +1330,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} groupPolicy, readStoreAllowFrom: pairing.readAllowFromStore, allowTextCommands, - hasControlCommand: shouldComputeCommandAuthorized, + hasControlCommand: isControlCommand, eventKind: "message", mayPair: true, }); diff --git a/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts b/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts index 25afe57de58..e5d3ac9bd14 100644 --- a/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts +++ b/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts @@ -617,6 +617,39 @@ describe("msteams monitor handler authz", () => { expect(runtimeApiMockState.dispatchReplyFromConfigWithSettledDispatcher).not.toHaveBeenCalled(); }); + it("does not drop inline command-looking group text from non-command-authorized senders", async () => { + resetThreadMocks(); + const isControlCommandMessage = vi.fn(() => false); + const shouldComputeCommandAuthorized = vi.fn(() => true); + const { deps } = createDeps( + { + commands: { useAccessGroups: true }, + channels: { + msteams: { + groupPolicy: "open", + requireMention: false, + }, + }, + } as OpenClawConfig, + { + isControlCommandMessage, + shouldComputeCommandAuthorized, + }, + ); + + const handler = createMSTeamsMessageHandler(deps); + await handler(createAttackerGroupActivity({ text: "hello /status" })); + + expect(isControlCommandMessage).toHaveBeenCalledWith("hello /status", deps.cfg); + expect(runtimeApiMockState.dispatchReplyFromConfigWithSettledDispatcher).toHaveBeenCalledTimes( + 1, + ); + const dispatched = firstSettledDispatch(); + const ctxPayload = recordFromMockCall(dispatched.ctxPayload); + expect(ctxPayload.BodyForAgent).toBe("hello /status"); + expect(ctxPayload.CommandAuthorized).toBe(false); + }); + it("flushes pending group text before authorizing a bare abort without a mention", async () => { resetThreadMocks(); const isBareAbort = vi.fn((text?: string) => diff --git a/extensions/msteams/src/monitor-handler/message-handler.ts b/extensions/msteams/src/monitor-handler/message-handler.ts index 427f1778029..cc4bfe4612e 100644 --- a/extensions/msteams/src/monitor-handler/message-handler.ts +++ b/extensions/msteams/src/monitor-handler/message-handler.ts @@ -287,8 +287,6 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) { }); const isControlCommand = allowTextCommands && core.channel.commands.isControlCommandMessage(text, cfg); - const shouldComputeCommandAuthorized = - allowTextCommands && core.channel.commands.shouldComputeCommandAuthorized(text, cfg); const { dmPolicy, senderId, @@ -303,7 +301,7 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) { } = await resolveMSTeamsSenderAccess({ cfg, activity, - hasControlCommand: shouldComputeCommandAuthorized, + hasControlCommand: isControlCommand, }); const commandAuthorized = commandAccess.requested ? commandAccess.authorized : undefined; const effectiveDmAllowFrom = senderAccess.effectiveAllowFrom;