fix: fail-closed shared-session reply routing (#24571) (thanks @brandonwise)

This commit is contained in:
Peter Steinberger
2026-02-25 01:41:04 +00:00
parent e28803503d
commit 885452f5c1
8 changed files with 180 additions and 14 deletions

View File

@@ -49,6 +49,7 @@ Docs: https://docs.openclaw.ai
- Sandbox/FS bridge tests: add regression coverage for dash-leading basenames to confirm sandbox file reads resolve to absolute container paths (and avoid shell-option misdiagnosis for dashed filenames). (#25891) Thanks @albertlieyingadrian.
- Sandbox/Config: preserve `dangerouslyAllowReservedContainerTargets` and `dangerouslyAllowExternalBindSources` during sandbox docker config resolution so explicit bind-mount break-glass overrides reach runtime validation. (#25410) Thanks @skyer-jian.
- Routing/Session isolation: harden followup routing so explicit cross-channel origin replies never fall back to the active dispatcher on route failure, preserve queued overflow summary routing metadata (`channel`/`to`/`thread`) across followup drain, and prefer originating channel context over internal provider tags for embedded followup runs. This prevents webchat/control-ui context from hijacking Discord-targeted replies in shared sessions. (#25864) Thanks @Gamedesigner.
- Security/Routing: fail closed for shared-session cross-channel replies by binding outbound target resolution to the current turns source channel metadata (instead of stale session route fallbacks), and wire those turn-source fields through gateway + command delivery planners with regression coverage. (#24571) Thanks @brandonwise.
- Messaging tool dedupe: treat originating channel metadata as authoritative for same-target `message.send` suppression in proactive runs (heartbeat/cron/exec-event), including synthetic-provider contexts, so `delivery-mirror` transcript entries no longer cause duplicate Telegram sends. (#25835) Thanks @jadeathena84-arch.
- Cron/Heartbeat delivery: stop inheriting cached session `lastThreadId` for heartbeat-mode target resolution unless a thread/topic is explicitly requested, so announce-mode cron and heartbeat deliveries stay on top-level destinations instead of leaking into active conversation threads. (#25730) Thanks @markshields-tl.
- Heartbeat defaults/prompts: switch the implicit heartbeat delivery target from `last` to `none` (opt-in for external delivery), and use internal-only cron/exec heartbeat prompt wording when delivery is disabled so background checks do not nudge user-facing relay behavior. (#25871, #24638, #25851)

View File

@@ -191,6 +191,49 @@ describe("deliverAgentCommandResult", () => {
);
});
it("uses runContext turn source over stale session last route", async () => {
await runDelivery({
opts: {
message: "hello",
deliver: true,
runContext: {
messageChannel: "whatsapp",
currentChannelId: "+15559876543",
accountId: "work",
},
},
sessionEntry: {
lastChannel: "slack",
lastTo: "U_WRONG",
lastAccountId: "wrong",
} as SessionEntry,
});
expect(mocks.resolveOutboundTarget).toHaveBeenCalledWith(
expect.objectContaining({ channel: "whatsapp", to: "+15559876543", accountId: "work" }),
);
});
it("does not reuse session lastTo when runContext source omits currentChannelId", async () => {
await runDelivery({
opts: {
message: "hello",
deliver: true,
runContext: {
messageChannel: "whatsapp",
},
},
sessionEntry: {
lastChannel: "slack",
lastTo: "U_WRONG",
} as SessionEntry,
});
expect(mocks.resolveOutboundTarget).toHaveBeenCalledWith(
expect.objectContaining({ channel: "whatsapp", to: undefined }),
);
});
it("prefixes nested agent outputs with context", async () => {
const runtime = createRuntime();
await runDelivery({

View File

@@ -71,6 +71,10 @@ export async function deliverAgentCommandResult(params: {
const { cfg, deps, runtime, opts, sessionEntry, payloads, result } = params;
const deliver = opts.deliver === true;
const bestEffortDeliver = opts.bestEffortDeliver === true;
const turnSourceChannel = opts.runContext?.messageChannel ?? opts.messageChannel;
const turnSourceTo = opts.runContext?.currentChannelId ?? opts.to;
const turnSourceAccountId = opts.runContext?.accountId ?? opts.accountId;
const turnSourceThreadId = opts.runContext?.currentThreadTs ?? opts.threadId;
const deliveryPlan = resolveAgentDeliveryPlan({
sessionEntry,
requestedChannel: opts.replyChannel ?? opts.channel,
@@ -78,6 +82,10 @@ export async function deliverAgentCommandResult(params: {
explicitThreadId: opts.threadId,
accountId: opts.replyAccountId ?? opts.accountId,
wantsDelivery: deliver,
turnSourceChannel,
turnSourceTo,
turnSourceAccountId,
turnSourceThreadId,
});
let deliveryChannel = deliveryPlan.resolvedChannel;
const explicitChannelHint = (opts.replyChannel ?? opts.channel)?.trim();

View File

@@ -487,6 +487,16 @@ export const agentHandlers: GatewayRequestHandlers = {
typeof request.threadId === "string" && request.threadId.trim()
? request.threadId.trim()
: undefined;
const turnSourceChannel =
typeof request.channel === "string" && request.channel.trim()
? request.channel.trim()
: undefined;
const turnSourceTo =
typeof request.to === "string" && request.to.trim() ? request.to.trim() : undefined;
const turnSourceAccountId =
typeof request.accountId === "string" && request.accountId.trim()
? request.accountId.trim()
: undefined;
const deliveryPlan = resolveAgentDeliveryPlan({
sessionEntry,
requestedChannel: request.replyChannel ?? request.channel,
@@ -494,6 +504,10 @@ export const agentHandlers: GatewayRequestHandlers = {
explicitThreadId,
accountId: request.replyAccountId ?? request.accountId,
wantsDelivery,
turnSourceChannel,
turnSourceTo,
turnSourceAccountId,
turnSourceThreadId: explicitThreadId,
});
let resolvedChannel = deliveryPlan.resolvedChannel;

View File

@@ -96,4 +96,41 @@ describe("agent delivery helpers", () => {
expect(mocks.resolveOutboundTarget).not.toHaveBeenCalled();
expect(resolved.resolvedTo).toBe("+1555");
});
it("prefers turn-source delivery context over session last route", () => {
const plan = resolveAgentDeliveryPlan({
sessionEntry: {
sessionId: "s4",
updatedAt: 4,
deliveryContext: { channel: "slack", to: "U_WRONG", accountId: "wrong" },
},
requestedChannel: "last",
turnSourceChannel: "whatsapp",
turnSourceTo: "+17775550123",
turnSourceAccountId: "work",
accountId: undefined,
wantsDelivery: true,
});
expect(plan.resolvedChannel).toBe("whatsapp");
expect(plan.resolvedTo).toBe("+17775550123");
expect(plan.resolvedAccountId).toBe("work");
});
it("does not reuse mutable session to when only turnSourceChannel is provided", () => {
const plan = resolveAgentDeliveryPlan({
sessionEntry: {
sessionId: "s5",
updatedAt: 5,
deliveryContext: { channel: "slack", to: "U_WRONG" },
},
requestedChannel: "last",
turnSourceChannel: "whatsapp",
accountId: undefined,
wantsDelivery: true,
});
expect(plan.resolvedChannel).toBe("whatsapp");
expect(plan.resolvedTo).toBeUndefined();
});
});

View File

@@ -65,6 +65,15 @@ export function resolveAgentDeliveryPlan(params: {
normalizedTurnSource && isDeliverableMessageChannel(normalizedTurnSource)
? normalizedTurnSource
: undefined;
const turnSourceTo =
typeof params.turnSourceTo === "string" && params.turnSourceTo.trim()
? params.turnSourceTo.trim()
: undefined;
const turnSourceAccountId = normalizeAccountId(params.turnSourceAccountId);
const turnSourceThreadId =
params.turnSourceThreadId != null && params.turnSourceThreadId !== ""
? params.turnSourceThreadId
: undefined;
const baseDelivery = resolveSessionDeliveryTarget({
entry: params.sessionEntry,
@@ -72,9 +81,9 @@ export function resolveAgentDeliveryPlan(params: {
explicitTo,
explicitThreadId: params.explicitThreadId,
turnSourceChannel,
turnSourceTo: params.turnSourceTo,
turnSourceAccountId: params.turnSourceAccountId,
turnSourceThreadId: params.turnSourceThreadId,
turnSourceTo,
turnSourceAccountId,
turnSourceThreadId,
});
const resolvedChannel = (() => {

View File

@@ -470,4 +470,62 @@ describe("resolveSessionDeliveryTarget — cross-channel reply guard (#24152)",
expect(resolved.accountId).toBe("bot-123");
expect(resolved.threadId).toBe(42);
});
it("does not fall back to session target metadata when turnSourceChannel is set", () => {
const resolved = resolveSessionDeliveryTarget({
entry: {
sessionId: "sess-no-fallback",
updatedAt: 1,
lastChannel: "slack",
lastTo: "U_WRONG",
lastAccountId: "wrong-account",
lastThreadId: "1739142736.000100",
},
requestedChannel: "last",
turnSourceChannel: "whatsapp",
});
expect(resolved.channel).toBe("whatsapp");
expect(resolved.to).toBeUndefined();
expect(resolved.accountId).toBeUndefined();
expect(resolved.threadId).toBeUndefined();
expect(resolved.lastTo).toBeUndefined();
expect(resolved.lastAccountId).toBeUndefined();
expect(resolved.lastThreadId).toBeUndefined();
});
it("uses explicitTo even when turnSourceTo is omitted", () => {
const resolved = resolveSessionDeliveryTarget({
entry: {
sessionId: "sess-explicit-to",
updatedAt: 1,
lastChannel: "slack",
lastTo: "U_WRONG",
},
requestedChannel: "last",
explicitTo: "+15551234567",
turnSourceChannel: "whatsapp",
});
expect(resolved.channel).toBe("whatsapp");
expect(resolved.to).toBe("+15551234567");
});
it("still allows mismatched lastTo only from turn-scoped metadata", () => {
const resolved = resolveSessionDeliveryTarget({
entry: {
sessionId: "sess-mismatch-turn",
updatedAt: 1,
lastChannel: "slack",
lastTo: "U_WRONG",
},
requestedChannel: "telegram",
allowMismatchedLastTo: true,
turnSourceChannel: "whatsapp",
turnSourceTo: "+15550000000",
});
expect(resolved.channel).toBe("telegram");
expect(resolved.to).toBe("+15550000000");
});
});

View File

@@ -89,17 +89,13 @@ export function resolveSessionDeliveryTarget(params: {
const sessionLastChannel =
context?.channel && isDeliverableMessageChannel(context.channel) ? context.channel : undefined;
// When a turn-source channel is provided, use it instead of the session's
// 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;
// When a turn-source channel is provided, use only turn-scoped metadata.
// Falling back to mutable session fields would re-introduce routing races.
const hasTurnSourceChannel = params.turnSourceChannel != null;
const lastChannel = hasTurnSourceChannel ? params.turnSourceChannel : sessionLastChannel;
const lastTo = hasTurnSourceChannel ? params.turnSourceTo : context?.to;
const lastAccountId = hasTurnSourceChannel ? params.turnSourceAccountId : context?.accountId;
const lastThreadId = hasTurnSourceChannel ? params.turnSourceThreadId : context?.threadId;
const rawRequested = params.requestedChannel ?? "last";
const requested = rawRequested === "last" ? "last" : normalizeMessageChannel(rawRequested);