mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-19 11:38:38 +00:00
fix(security): prevent cross-channel reply routing in shared sessions
This commit is contained in:
committed by
Peter Steinberger
parent
c7ae4ed04d
commit
455fbc6b6d
@@ -7,6 +7,7 @@ import {
|
|||||||
isDeliverableMessageChannel,
|
isDeliverableMessageChannel,
|
||||||
isGatewayMessageChannel,
|
isGatewayMessageChannel,
|
||||||
normalizeMessageChannel,
|
normalizeMessageChannel,
|
||||||
|
type DeliverableMessageChannel,
|
||||||
type GatewayMessageChannel,
|
type GatewayMessageChannel,
|
||||||
} from "../../utils/message-channel.js";
|
} from "../../utils/message-channel.js";
|
||||||
import type { OutboundTargetResolution } from "./targets.js";
|
import type { OutboundTargetResolution } from "./targets.js";
|
||||||
@@ -32,6 +33,20 @@ export function resolveAgentDeliveryPlan(params: {
|
|||||||
explicitThreadId?: string | number;
|
explicitThreadId?: string | number;
|
||||||
accountId?: string;
|
accountId?: string;
|
||||||
wantsDelivery: boolean;
|
wantsDelivery: boolean;
|
||||||
|
/**
|
||||||
|
* The channel that originated the current agent turn. When provided,
|
||||||
|
* overrides session-level `lastChannel` to prevent cross-channel reply
|
||||||
|
* routing in shared sessions (dmScope="main").
|
||||||
|
*
|
||||||
|
* @see https://github.com/openclaw/openclaw/issues/24152
|
||||||
|
*/
|
||||||
|
turnSourceChannel?: string;
|
||||||
|
/** Turn-source `to` — paired with `turnSourceChannel`. */
|
||||||
|
turnSourceTo?: string;
|
||||||
|
/** Turn-source `accountId` — paired with `turnSourceChannel`. */
|
||||||
|
turnSourceAccountId?: string;
|
||||||
|
/** Turn-source `threadId` — paired with `turnSourceChannel`. */
|
||||||
|
turnSourceThreadId?: string | number;
|
||||||
}): AgentDeliveryPlan {
|
}): AgentDeliveryPlan {
|
||||||
const requestedRaw =
|
const requestedRaw =
|
||||||
typeof params.requestedChannel === "string" ? params.requestedChannel.trim() : "";
|
typeof params.requestedChannel === "string" ? params.requestedChannel.trim() : "";
|
||||||
@@ -43,11 +58,24 @@ export function resolveAgentDeliveryPlan(params: {
|
|||||||
? params.explicitTo.trim()
|
? params.explicitTo.trim()
|
||||||
: undefined;
|
: undefined;
|
||||||
|
|
||||||
|
// Resolve turn-source channel for cross-channel safety.
|
||||||
|
const normalizedTurnSource = params.turnSourceChannel
|
||||||
|
? normalizeMessageChannel(params.turnSourceChannel)
|
||||||
|
: undefined;
|
||||||
|
const turnSourceChannel =
|
||||||
|
normalizedTurnSource && isDeliverableMessageChannel(normalizedTurnSource)
|
||||||
|
? normalizedTurnSource
|
||||||
|
: undefined;
|
||||||
|
|
||||||
const baseDelivery = resolveSessionDeliveryTarget({
|
const baseDelivery = resolveSessionDeliveryTarget({
|
||||||
entry: params.sessionEntry,
|
entry: params.sessionEntry,
|
||||||
requestedChannel: requestedChannel === INTERNAL_MESSAGE_CHANNEL ? "last" : requestedChannel,
|
requestedChannel: requestedChannel === INTERNAL_MESSAGE_CHANNEL ? "last" : requestedChannel,
|
||||||
explicitTo,
|
explicitTo,
|
||||||
explicitThreadId: params.explicitThreadId,
|
explicitThreadId: params.explicitThreadId,
|
||||||
|
turnSourceChannel,
|
||||||
|
turnSourceTo: params.turnSourceTo,
|
||||||
|
turnSourceAccountId: params.turnSourceAccountId,
|
||||||
|
turnSourceThreadId: params.turnSourceThreadId,
|
||||||
});
|
});
|
||||||
|
|
||||||
const resolvedChannel = (() => {
|
const resolvedChannel = (() => {
|
||||||
|
|||||||
@@ -357,3 +357,79 @@ describe("resolveSessionDeliveryTarget", () => {
|
|||||||
expect(resolved.threadId).toBe(1008013);
|
expect(resolved.threadId).toBe(1008013);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("resolveSessionDeliveryTarget — cross-channel reply guard (#24152)", () => {
|
||||||
|
it("uses turnSourceChannel over session lastChannel when provided", () => {
|
||||||
|
// Simulate: WhatsApp message originated the turn, but a Slack message
|
||||||
|
// arrived concurrently and updated lastChannel to "slack"
|
||||||
|
const resolved = resolveSessionDeliveryTarget({
|
||||||
|
entry: {
|
||||||
|
sessionId: "sess-shared",
|
||||||
|
updatedAt: 1,
|
||||||
|
lastChannel: "slack", // <- concurrently overwritten
|
||||||
|
lastTo: "U0AEMECNCBV", // <- Slack user (wrong target)
|
||||||
|
},
|
||||||
|
requestedChannel: "last",
|
||||||
|
turnSourceChannel: "whatsapp", // <- originated from WhatsApp
|
||||||
|
turnSourceTo: "+66972796305", // <- WhatsApp user (correct target)
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(resolved.channel).toBe("whatsapp");
|
||||||
|
expect(resolved.to).toBe("+66972796305");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls back to session lastChannel when turnSourceChannel is not set", () => {
|
||||||
|
const resolved = resolveSessionDeliveryTarget({
|
||||||
|
entry: {
|
||||||
|
sessionId: "sess-normal",
|
||||||
|
updatedAt: 1,
|
||||||
|
lastChannel: "telegram",
|
||||||
|
lastTo: "8587265585",
|
||||||
|
},
|
||||||
|
requestedChannel: "last",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(resolved.channel).toBe("telegram");
|
||||||
|
expect(resolved.to).toBe("8587265585");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("respects explicit requestedChannel over turnSourceChannel", () => {
|
||||||
|
const resolved = resolveSessionDeliveryTarget({
|
||||||
|
entry: {
|
||||||
|
sessionId: "sess-explicit",
|
||||||
|
updatedAt: 1,
|
||||||
|
lastChannel: "slack",
|
||||||
|
lastTo: "U12345",
|
||||||
|
},
|
||||||
|
requestedChannel: "telegram",
|
||||||
|
explicitTo: "8587265585",
|
||||||
|
turnSourceChannel: "whatsapp",
|
||||||
|
turnSourceTo: "+66972796305",
|
||||||
|
});
|
||||||
|
|
||||||
|
// Explicit requestedChannel "telegram" is not "last", so it takes priority
|
||||||
|
expect(resolved.channel).toBe("telegram");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("preserves turnSourceAccountId and turnSourceThreadId", () => {
|
||||||
|
const resolved = resolveSessionDeliveryTarget({
|
||||||
|
entry: {
|
||||||
|
sessionId: "sess-meta",
|
||||||
|
updatedAt: 1,
|
||||||
|
lastChannel: "slack",
|
||||||
|
lastTo: "U_WRONG",
|
||||||
|
lastAccountId: "wrong-account",
|
||||||
|
},
|
||||||
|
requestedChannel: "last",
|
||||||
|
turnSourceChannel: "telegram",
|
||||||
|
turnSourceTo: "8587265585",
|
||||||
|
turnSourceAccountId: "bot-123",
|
||||||
|
turnSourceThreadId: 42,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(resolved.channel).toBe("telegram");
|
||||||
|
expect(resolved.to).toBe("8587265585");
|
||||||
|
expect(resolved.accountId).toBe("bot-123");
|
||||||
|
expect(resolved.threadId).toBe(42);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -62,13 +62,41 @@ export function resolveSessionDeliveryTarget(params: {
|
|||||||
fallbackChannel?: DeliverableMessageChannel;
|
fallbackChannel?: DeliverableMessageChannel;
|
||||||
allowMismatchedLastTo?: boolean;
|
allowMismatchedLastTo?: boolean;
|
||||||
mode?: ChannelOutboundTargetMode;
|
mode?: ChannelOutboundTargetMode;
|
||||||
|
/**
|
||||||
|
* When set, this overrides the session-level `lastChannel` for "last"
|
||||||
|
* resolution. This prevents cross-channel reply routing when multiple
|
||||||
|
* channels share the same session (dmScope = "main") and an inbound
|
||||||
|
* message from a different channel updates `lastChannel` while an agent
|
||||||
|
* turn is still in flight.
|
||||||
|
*
|
||||||
|
* Callers should set this to the channel that originated the current
|
||||||
|
* agent turn so the reply always routes back to the correct channel.
|
||||||
|
*
|
||||||
|
* @see https://github.com/openclaw/openclaw/issues/24152
|
||||||
|
*/
|
||||||
|
turnSourceChannel?: DeliverableMessageChannel;
|
||||||
|
/** Turn-source `to` — paired with `turnSourceChannel`. */
|
||||||
|
turnSourceTo?: string;
|
||||||
|
/** Turn-source `accountId` — paired with `turnSourceChannel`. */
|
||||||
|
turnSourceAccountId?: string;
|
||||||
|
/** Turn-source `threadId` — paired with `turnSourceChannel`. */
|
||||||
|
turnSourceThreadId?: string | number;
|
||||||
}): SessionDeliveryTarget {
|
}): SessionDeliveryTarget {
|
||||||
const context = deliveryContextFromSession(params.entry);
|
const context = deliveryContextFromSession(params.entry);
|
||||||
const lastChannel =
|
const sessionLastChannel =
|
||||||
context?.channel && isDeliverableMessageChannel(context.channel) ? context.channel : undefined;
|
context?.channel && isDeliverableMessageChannel(context.channel) ? context.channel : undefined;
|
||||||
const lastTo = context?.to;
|
|
||||||
const lastAccountId = context?.accountId;
|
// When a turn-source channel is provided, use it instead of the session's
|
||||||
const lastThreadId = context?.threadId;
|
// mutable lastChannel. This prevents a concurrent inbound from a different
|
||||||
|
// channel from hijacking the reply target (cross-channel privacy leak).
|
||||||
|
const lastChannel = params.turnSourceChannel ?? sessionLastChannel;
|
||||||
|
const lastTo = params.turnSourceChannel ? (params.turnSourceTo ?? context?.to) : context?.to;
|
||||||
|
const lastAccountId = params.turnSourceChannel
|
||||||
|
? (params.turnSourceAccountId ?? context?.accountId)
|
||||||
|
: context?.accountId;
|
||||||
|
const lastThreadId = params.turnSourceChannel
|
||||||
|
? (params.turnSourceThreadId ?? context?.threadId)
|
||||||
|
: context?.threadId;
|
||||||
|
|
||||||
const rawRequested = params.requestedChannel ?? "last";
|
const rawRequested = params.requestedChannel ?? "last";
|
||||||
const requested = rawRequested === "last" ? "last" : normalizeMessageChannel(rawRequested);
|
const requested = rawRequested === "last" ? "last" : normalizeMessageChannel(rawRequested);
|
||||||
|
|||||||
Reference in New Issue
Block a user