From ffc465394ee1f657020085c652a84bb9d4a3192a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 13 Jan 2026 01:03:23 +0000 Subject: [PATCH] fix: enforce message context isolation --- CHANGELOG.md | 1 + docs/tools/index.md | 1 + src/agents/tools/message-tool.ts | 10 +-- .../outbound/message-action-runner.test.ts | 61 +++++++++++++++++++ src/infra/outbound/message-action-runner.ts | 58 ++++++++++++++++++ src/providers/dock.ts | 38 ++++++++++++ 6 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 src/infra/outbound/message-action-runner.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 935cedfe7..034d0cab1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Update: run `clawdbot doctor --non-interactive` during updates to avoid TTY hangs. (#781 — thanks @ronyrus) - Tools: allow Claude/Gemini tool param aliases (`file_path`, `old_string`, `new_string`) while enforcing required params at runtime. (#793 — thanks @hsrvc) - Gemini: downgrade tool-call history missing `thought_signature` to avoid INVALID_ARGUMENT errors. (#793 — thanks @hsrvc) +- Messaging: enforce context isolation for message tool sends across providers (normalized targets + tests). (#793 — thanks @hsrvc) ## 2026.1.12-3 diff --git a/docs/tools/index.md b/docs/tools/index.md index 3cfd44da6..9d2a4b707 100644 --- a/docs/tools/index.md +++ b/docs/tools/index.md @@ -179,6 +179,7 @@ Core actions: Notes: - `send` routes WhatsApp via the Gateway; other providers go direct. - `poll` uses the Gateway for WhatsApp and MS Teams; Discord polls go direct. +- When a message tool call is bound to an active chat session, sends are constrained to that session’s target to avoid cross-context leaks. ### `cron` Manage Gateway cron jobs and wakeups. diff --git a/src/agents/tools/message-tool.ts b/src/agents/tools/message-tool.ts index be1f9f56d..4ba4b062f 100644 --- a/src/agents/tools/message-tool.ts +++ b/src/agents/tools/message-tool.ts @@ -2,6 +2,10 @@ import { Type } from "@sinclair/typebox"; import type { ClawdbotConfig } from "../../config/config.js"; import { loadConfig } from "../../config/config.js"; +import { + GATEWAY_CLIENT_IDS, + GATEWAY_CLIENT_MODES, +} from "../../gateway/protocol/client-info.js"; import { runMessageAction } from "../../infra/outbound/message-action-runner.js"; import { listProviderMessageActions, @@ -12,10 +16,6 @@ import { type ProviderMessageActionName, } from "../../providers/plugins/types.js"; import { normalizeAccountId } from "../../routing/session-key.js"; -import { - GATEWAY_CLIENT_MODES, - GATEWAY_CLIENT_NAMES, -} from "../../utils/message-provider.js"; import type { AnyAgentTool } from "./common.js"; import { jsonResult, readNumberParam, readStringParam } from "./common.js"; @@ -184,7 +184,7 @@ export function createMessageTool(options?: MessageToolOptions): AnyAgentTool { url: readStringParam(params, "gatewayUrl", { trim: false }), token: readStringParam(params, "gatewayToken", { trim: false }), timeoutMs: readNumberParam(params, "timeoutMs"), - clientName: GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT, + clientName: GATEWAY_CLIENT_IDS.GATEWAY_CLIENT, clientDisplayName: "agent", mode: GATEWAY_CLIENT_MODES.BACKEND, }; diff --git a/src/infra/outbound/message-action-runner.test.ts b/src/infra/outbound/message-action-runner.test.ts new file mode 100644 index 000000000..1f982f35d --- /dev/null +++ b/src/infra/outbound/message-action-runner.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from "vitest"; + +import type { ClawdbotConfig } from "../../config/config.js"; +import { runMessageAction } from "./message-action-runner.js"; + +const slackConfig = { + slack: { + botToken: "xoxb-test", + appToken: "xapp-test", + }, +} as ClawdbotConfig; + +describe("runMessageAction context isolation", () => { + it("allows send when target matches current channel", async () => { + const result = await runMessageAction({ + cfg: slackConfig, + action: "send", + params: { + provider: "slack", + to: "#C123", + message: "hi", + }, + toolContext: { currentChannelId: "C123" }, + dryRun: true, + }); + + expect(result.kind).toBe("send"); + }); + + it("blocks send when target differs from current channel", async () => { + await expect( + runMessageAction({ + cfg: slackConfig, + action: "send", + params: { + provider: "slack", + to: "channel:C999", + message: "hi", + }, + toolContext: { currentChannelId: "C123" }, + dryRun: true, + }), + ).rejects.toThrow(/Cross-context messaging denied/); + }); + + it("blocks thread-reply when channelId differs from current channel", async () => { + await expect( + runMessageAction({ + cfg: slackConfig, + action: "thread-reply", + params: { + provider: "slack", + channelId: "C999", + message: "hi", + }, + toolContext: { currentChannelId: "C123" }, + dryRun: true, + }), + ).rejects.toThrow(/Cross-context messaging denied/); + }); +}); diff --git a/src/infra/outbound/message-action-runner.ts b/src/infra/outbound/message-action-runner.ts index 6f743a6ea..94b9b83ed 100644 --- a/src/infra/outbound/message-action-runner.ts +++ b/src/infra/outbound/message-action-runner.ts @@ -1,4 +1,5 @@ import type { AgentToolResult } from "@mariozechner/pi-agent-core"; +import { normalizeTargetForProvider } from "../../agents/pi-embedded-messaging.js"; import { readNumberParam, readStringArrayParam, @@ -125,6 +126,56 @@ function parseButtonsParam(params: Record): void { } } +const CONTEXT_GUARDED_ACTIONS = new Set([ + "send", + "poll", + "thread-create", + "thread-reply", + "sticker", +]); + +function resolveContextGuardTarget( + action: ProviderMessageActionName, + params: Record, +): string | undefined { + if (!CONTEXT_GUARDED_ACTIONS.has(action)) return undefined; + + if (action === "thread-reply" || action === "thread-create") { + return ( + readStringParam(params, "channelId") ?? readStringParam(params, "to") + ); + } + + return readStringParam(params, "to") ?? readStringParam(params, "channelId"); +} + +function enforceContextIsolation(params: { + provider: ProviderId; + action: ProviderMessageActionName; + params: Record; + toolContext?: ProviderThreadingToolContext; +}): void { + const currentTarget = params.toolContext?.currentChannelId?.trim(); + if (!currentTarget) return; + if (!CONTEXT_GUARDED_ACTIONS.has(params.action)) return; + + const target = resolveContextGuardTarget(params.action, params.params); + if (!target) return; + + const normalizedTarget = + normalizeTargetForProvider(params.provider, target) ?? target.toLowerCase(); + const normalizedCurrent = + normalizeTargetForProvider(params.provider, currentTarget) ?? + currentTarget.toLowerCase(); + + if (!normalizedTarget || !normalizedCurrent) return; + if (normalizedTarget === normalizedCurrent) return; + + throw new Error( + `Cross-context messaging denied: action=${params.action} target="${target}" while bound to "${currentTarget}" (provider=${params.provider}).`, + ); +} + async function resolveProvider( cfg: ClawdbotConfig, params: Record, @@ -150,6 +201,13 @@ export async function runMessageAction( readStringParam(params, "accountId") ?? input.defaultAccountId; const dryRun = Boolean(input.dryRun ?? readBooleanParam(params, "dryRun")); + enforceContextIsolation({ + provider, + action, + params, + toolContext: input.toolContext, + }); + const gateway = input.gateway ? { url: input.gateway.url, diff --git a/src/providers/dock.ts b/src/providers/dock.ts index 3c3f64eb7..d1267a671 100644 --- a/src/providers/dock.ts +++ b/src/providers/dock.ts @@ -102,6 +102,11 @@ const DOCKS: Record = { }, threading: { resolveReplyToMode: ({ cfg }) => cfg.telegram?.replyToMode ?? "first", + buildToolContext: ({ context, hasRepliedRef }) => ({ + currentChannelId: context.To?.trim() || undefined, + currentThreadTs: context.ReplyToId, + hasRepliedRef, + }), }, }, whatsapp: { @@ -142,6 +147,13 @@ const DOCKS: Record = { return [escaped, `@${escaped}`]; }, }, + threading: { + buildToolContext: ({ context, hasRepliedRef }) => ({ + currentChannelId: context.To?.trim() || undefined, + currentThreadTs: context.ReplyToId, + hasRepliedRef, + }), + }, }, discord: { id: "discord", @@ -175,6 +187,11 @@ const DOCKS: Record = { }, threading: { resolveReplyToMode: ({ cfg }) => cfg.discord?.replyToMode ?? "off", + buildToolContext: ({ context, hasRepliedRef }) => ({ + currentChannelId: context.To?.trim() || undefined, + currentThreadTs: context.ReplyToId, + hasRepliedRef, + }), }, }, slack: { @@ -246,6 +263,13 @@ const DOCKS: Record = { ) .filter(Boolean), }, + threading: { + buildToolContext: ({ context, hasRepliedRef }) => ({ + currentChannelId: context.To?.trim() || undefined, + currentThreadTs: context.ReplyToId, + hasRepliedRef, + }), + }, }, imessage: { id: "imessage", @@ -266,6 +290,13 @@ const DOCKS: Record = { groups: { resolveRequireMention: resolveIMessageGroupRequireMention, }, + threading: { + buildToolContext: ({ context, hasRepliedRef }) => ({ + currentChannelId: context.To?.trim() || undefined, + currentThreadTs: context.ReplyToId, + hasRepliedRef, + }), + }, }, msteams: { id: "msteams", @@ -280,6 +311,13 @@ const DOCKS: Record = { resolveAllowFrom: ({ cfg }) => cfg.msteams?.allowFrom ?? [], formatAllowFrom: ({ allowFrom }) => formatLower(allowFrom), }, + threading: { + buildToolContext: ({ context, hasRepliedRef }) => ({ + currentChannelId: context.To?.trim() || undefined, + currentThreadTs: context.ReplyToId, + hasRepliedRef, + }), + }, }, };