fix(gateway): stop shared-main chat.send from inheriting stale external routes (#38418)

* fix(gateway): prevent webchat messages from cross-routing to external channels chat.send always originates from the webchat/control-UI surface. Previously, channel-scoped session keys (e.g. agent:main:slack:direct:U…) caused OriginatingChannel to inherit the session's stored external route, so the reply dispatcher would route responses to Slack/Telegram instead of back to the gateway connection. Remove the route-inheritance logic from chat.send and always set OriginatingChannel to INTERNAL_MESSAGE_CHANNEL ("webchat"). Closes #34647 Made-with: Cursor * Gateway: preserve configured-main connect gating * Gateway: cover connect-without-client routing * Gateway: add chat.send session key length limit * Gateway: cap chat.send session key schema * Gateway: bound chat.send session key parsing * Gateway: cover oversized chat.send session keys * Update CHANGELOG.md --------- Co-authored-by: SidQin-cyber <sidqin0410@gmail.com>
2026-03-06 21:59:08 -05:00
parent bf623a580b
commit 563a125c66
5 changed files with 213 additions and 64 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -204,6 +204,7 @@ Docs: https://docs.openclaw.ai
 - Agents/gateway config guidance: stop exposing `config.schema` through the agent `gateway` tool, remove prompt/docs guidance that told agents to call it, and keep agents on `config.get` plus `config.patch`/`config.apply` for config changes. (#7382) thanks @kakuteki.
 - Agents/failover: classify periodic provider limit exhaustion text (for example `Weekly/Monthly Limit Exhausted`) as `rate_limit` while keeping explicit `402 Payment Required` variants in billing, so failover continues without misclassifying billing-wrapped quota errors. (#33813) thanks @zhouhe-xydt.
 - Mattermost/interactive button callbacks: allow external callback base URLs and stop requiring loopback-origin requests so button clicks work when Mattermost reaches the gateway over Tailscale, LAN, or a reverse proxy. (#37543) thanks @mukhtharcm.
+- Gateway/chat.send route inheritance: keep explicit external delivery for channel-scoped sessions while preventing shared-main and other channel-agnostic webchat sessions from inheriting stale external routes, so Control UI replies stay on webchat without breaking selected channel-target sessions. (#34669) Thanks @vincentkoc.
 - Telegram/Discord media upload caps: make outbound uploads honor channel `mediaMaxMb` config, raise Telegram's default media cap to 100MB, and remove MIME fallback limits that kept some Telegram uploads at 16MB. Thanks @vincentkoc.
 - Skills/nano-banana-pro resolution override: respect explicit `--resolution` values during image editing and only auto-detect output size from input images when the flag is omitted. (#36880) Thanks @shuofengzhang and @vincentkoc.
 - Skills/openai-image-gen CLI validation: validate `--background` and `--style` inputs early, normalize supported values, and warn when those flags are ignored for incompatible models. (#36762) Thanks @shuofengzhang and @vincentkoc.
--- a/src/gateway/protocol/schema/logs-chat.ts
+++ b/src/gateway/protocol/schema/logs-chat.ts
@@ -1,5 +1,5 @@
 import { Type } from "@sinclair/typebox";
-import { NonEmptyString } from "./primitives.js";
+import { ChatSendSessionKeyString, NonEmptyString } from "./primitives.js";

 export const LogsTailParamsSchema = Type.Object(
  {
@@ -33,7 +33,7 @@ export const ChatHistoryParamsSchema = Type.Object(

 export const ChatSendParamsSchema = Type.Object(
  {
-    sessionKey: NonEmptyString,
+    sessionKey: ChatSendSessionKeyString,
    message: Type.String(),
    thinking: Type.Optional(Type.String()),
    deliver: Type.Optional(Type.Boolean()),
--- a/src/gateway/protocol/schema/primitives.ts
+++ b/src/gateway/protocol/schema/primitives.ts
@@ -3,6 +3,11 @@ import { SESSION_LABEL_MAX_LENGTH } from "../../../sessions/session-label.js";
 import { GATEWAY_CLIENT_IDS, GATEWAY_CLIENT_MODES } from "../client-info.js";

 export const NonEmptyString = Type.String({ minLength: 1 });
+export const CHAT_SEND_SESSION_KEY_MAX_LENGTH = 512;
+export const ChatSendSessionKeyString = Type.String({
+  minLength: 1,
+  maxLength: CHAT_SEND_SESSION_KEY_MAX_LENGTH,
+});
 export const SessionLabelString = Type.String({
  minLength: 1,
  maxLength: SESSION_LABEL_MAX_LENGTH,
--- a/src/gateway/server-methods/chat.directive-tags.test.ts
+++ b/src/gateway/server-methods/chat.directive-tags.test.ts
@@ -5,6 +5,8 @@ import { CURRENT_SESSION_VERSION } from "@mariozechner/pi-coding-agent";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import type { MsgContext } from "../../auto-reply/templating.js";
 import { GATEWAY_CLIENT_CAPS, GATEWAY_CLIENT_MODES } from "../protocol/client-info.js";
+import { ErrorCodes } from "../protocol/index.js";
+import { CHAT_SEND_SESSION_KEY_MAX_LENGTH } from "../protocol/schema/primitives.js";
 import type { GatewayRequestContext } from "./types.js";

 const mockState = vi.hoisted(() => ({
@@ -325,6 +327,34 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
    expect(extractFirstTextBlock(payload)).toBe("");
  });

+  it("rejects oversized chat.send session keys before dispatch", async () => {
+    createTranscriptFixture("openclaw-chat-send-session-key-too-long-");
+    const respond = vi.fn();
+    const context = createChatContext();
+
+    await chatHandlers["chat.send"]({
+      params: {
+        sessionKey: `agent:main:${"x".repeat(CHAT_SEND_SESSION_KEY_MAX_LENGTH)}`,
+        message: "hello",
+        idempotencyKey: "idem-session-key-too-long",
+      },
+      respond,
+      req: {} as never,
+      client: null as never,
+      isWebchatConnect: () => false,
+      context: context as GatewayRequestContext,
+    });
+
+    expect(respond).toHaveBeenCalledWith(
+      false,
+      undefined,
+      expect.objectContaining({
+        code: ErrorCodes.INVALID_REQUEST,
+      }),
+    );
+    expect(context.broadcast).not.toHaveBeenCalled();
+  });
+
  it("chat.inject strips external untrusted wrapper metadata from final payload text", async () => {
    createTranscriptFixture("openclaw-chat-inject-untrusted-meta-");
    const respond = vi.fn();
@@ -362,7 +392,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
    expect(extractFirstTextBlock(payload)).toBe("hello");
  });

-  it("chat.send inherits originating routing metadata from session delivery context", async () => {
+  it("chat.send keeps explicit delivery routes for channel-scoped sessions", async () => {
    createTranscriptFixture("openclaw-chat-send-origin-routing-");
    mockState.finalText = "ok";
    mockState.sessionEntry = {
@@ -400,7 +430,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
    );
  });

-  it("chat.send inherits Feishu routing metadata from session delivery context", async () => {
+  it("chat.send keeps explicit delivery routes for Feishu channel-scoped sessions", async () => {
    createTranscriptFixture("openclaw-chat-send-feishu-origin-routing-");
    mockState.finalText = "ok";
    mockState.sessionEntry = {
@@ -429,12 +459,13 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
      expect.objectContaining({
        OriginatingChannel: "feishu",
        OriginatingTo: "ou_feishu_direct_123",
+        ExplicitDeliverRoute: true,
        AccountId: "default",
      }),
    );
  });

-  it("chat.send inherits routing metadata for per-account channel-peer session keys", async () => {
+  it("chat.send keeps explicit delivery routes for per-account channel-peer sessions", async () => {
    createTranscriptFixture("openclaw-chat-send-per-account-channel-peer-routing-");
    mockState.finalText = "ok";
    mockState.sessionEntry = {
@@ -463,12 +494,13 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
      expect.objectContaining({
        OriginatingChannel: "telegram",
        OriginatingTo: "telegram:6812765697",
+        ExplicitDeliverRoute: true,
        AccountId: "account-a",
      }),
    );
  });

-  it("chat.send inherits routing metadata for legacy channel-peer session keys", async () => {
+  it("chat.send keeps explicit delivery routes for legacy channel-peer sessions", async () => {
    createTranscriptFixture("openclaw-chat-send-legacy-channel-peer-routing-");
    mockState.finalText = "ok";
    mockState.sessionEntry = {
@@ -497,12 +529,13 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
      expect.objectContaining({
        OriginatingChannel: "telegram",
        OriginatingTo: "telegram:6812765697",
+        ExplicitDeliverRoute: true,
        AccountId: "default",
      }),
    );
  });

-  it("chat.send inherits routing metadata for legacy channel-peer thread session keys", async () => {
+  it("chat.send keeps explicit delivery routes for legacy thread sessions", async () => {
    createTranscriptFixture("openclaw-chat-send-legacy-thread-channel-peer-routing-");
    mockState.finalText = "ok";
    mockState.sessionEntry = {
@@ -533,6 +566,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
      expect.objectContaining({
        OriginatingChannel: "telegram",
        OriginatingTo: "telegram:6812765697",
+        ExplicitDeliverRoute: true,
        AccountId: "default",
        MessageThreadId: "42",
      }),
@@ -657,6 +691,44 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
    );
  });

+  it("chat.send keeps configured main delivery inheritance when connect metadata omits client details", async () => {
+    createTranscriptFixture("openclaw-chat-send-config-main-connect-no-client-");
+    mockState.mainSessionKey = "work";
+    mockState.finalText = "ok";
+    mockState.sessionEntry = {
+      deliveryContext: {
+        channel: "whatsapp",
+        to: "whatsapp:+8613800138000",
+        accountId: "default",
+      },
+      lastChannel: "whatsapp",
+      lastTo: "whatsapp:+8613800138000",
+      lastAccountId: "default",
+    };
+    const respond = vi.fn();
+    const context = createChatContext();
+
+    await runNonStreamingChatSend({
+      context,
+      respond,
+      idempotencyKey: "idem-config-main-connect-no-client",
+      client: {
+        connect: {},
+      } as unknown,
+      sessionKey: "agent:main:work",
+      deliver: true,
+      expectBroadcast: false,
+    });
+
+    expect(mockState.lastDispatchCtx).toEqual(
+      expect.objectContaining({
+        OriginatingChannel: "whatsapp",
+        OriginatingTo: "whatsapp:+8613800138000",
+        AccountId: "default",
+      }),
+    );
+  });
+
  it("chat.send does not inherit external delivery context for non-channel custom sessions", async () => {
    createTranscriptFixture("openclaw-chat-send-custom-no-cross-route-");
    mockState.finalText = "ok";
--- a/src/gateway/server-methods/chat.ts
+++ b/src/gateway/server-methods/chat.ts
@@ -46,6 +46,7 @@ import {
  validateChatInjectParams,
  validateChatSendParams,
 } from "../protocol/index.js";
+import { CHAT_SEND_SESSION_KEY_MAX_LENGTH } from "../protocol/schema/primitives.js";
 import { getMaxChatHistoryMessagesBytes } from "../server-constants.js";
 import {
  capArrayByJsonBytes,
@@ -95,6 +96,118 @@ const CHANNEL_AGNOSTIC_SESSION_SCOPES = new Set([
 ]);
 const CHANNEL_SCOPED_SESSION_SHAPES = new Set(["direct", "dm", "group", "channel"]);

+type ChatSendDeliveryEntry = {
+  deliveryContext?: {
+    channel?: string;
+    to?: string;
+    accountId?: string;
+    threadId?: string | number;
+  };
+  lastChannel?: string;
+  lastTo?: string;
+  lastAccountId?: string;
+  lastThreadId?: string | number;
+};
+
+type ChatSendOriginatingRoute = {
+  originatingChannel: string;
+  originatingTo?: string;
+  accountId?: string;
+  messageThreadId?: string | number;
+  explicitDeliverRoute: boolean;
+};
+
+function resolveChatSendOriginatingRoute(params: {
+  client?: { mode?: string | null; id?: string | null } | null;
+  deliver?: boolean;
+  entry?: ChatSendDeliveryEntry;
+  hasConnectedClient?: boolean;
+  mainKey?: string;
+  sessionKey: string;
+}): ChatSendOriginatingRoute {
+  const shouldDeliverExternally = params.deliver === true;
+  if (!shouldDeliverExternally) {
+    return {
+      originatingChannel: INTERNAL_MESSAGE_CHANNEL,
+      explicitDeliverRoute: false,
+    };
+  }
+
+  const routeChannelCandidate = normalizeMessageChannel(
+    params.entry?.deliveryContext?.channel ?? params.entry?.lastChannel,
+  );
+  const routeToCandidate = params.entry?.deliveryContext?.to ?? params.entry?.lastTo;
+  const routeAccountIdCandidate =
+    params.entry?.deliveryContext?.accountId ?? params.entry?.lastAccountId ?? undefined;
+  const routeThreadIdCandidate =
+    params.entry?.deliveryContext?.threadId ?? params.entry?.lastThreadId;
+  if (params.sessionKey.length > CHAT_SEND_SESSION_KEY_MAX_LENGTH) {
+    return {
+      originatingChannel: INTERNAL_MESSAGE_CHANNEL,
+      explicitDeliverRoute: false,
+    };
+  }
+
+  const parsedSessionKey = parseAgentSessionKey(params.sessionKey);
+  const sessionScopeParts = (parsedSessionKey?.rest ?? params.sessionKey)
+    .split(":", 3)
+    .filter(Boolean);
+  const sessionScopeHead = sessionScopeParts[0];
+  const sessionChannelHint = normalizeMessageChannel(sessionScopeHead);
+  const normalizedSessionScopeHead = (sessionScopeHead ?? "").trim().toLowerCase();
+  const sessionPeerShapeCandidates = [sessionScopeParts[1], sessionScopeParts[2]]
+    .map((part) => (part ?? "").trim().toLowerCase())
+    .filter(Boolean);
+  const isChannelAgnosticSessionScope = CHANNEL_AGNOSTIC_SESSION_SCOPES.has(
+    normalizedSessionScopeHead,
+  );
+  const isChannelScopedSession = sessionPeerShapeCandidates.some((part) =>
+    CHANNEL_SCOPED_SESSION_SHAPES.has(part),
+  );
+  const hasLegacyChannelPeerShape =
+    !isChannelScopedSession &&
+    typeof sessionScopeParts[1] === "string" &&
+    sessionChannelHint === routeChannelCandidate;
+  const isFromWebchatClient =
+    isWebchatClient(params.client) || params.client?.mode === GATEWAY_CLIENT_MODES.UI;
+  const configuredMainKey = (params.mainKey ?? "main").trim().toLowerCase();
+  const isConfiguredMainSessionScope =
+    normalizedSessionScopeHead.length > 0 && normalizedSessionScopeHead === configuredMainKey;
+
+  // Keep explicit delivery for channel-scoped sessions, but refuse to inherit
+  // stale external routes for shared-main and other channel-agnostic webchat/UI
+  // turns where the session key does not encode the user's current target.
+  // Preserve the old configured-main contract: any connected non-webchat client
+  // may inherit the last external route even when client metadata is absent.
+  const canInheritDeliverableRoute = Boolean(
+    sessionChannelHint &&
+    sessionChannelHint !== INTERNAL_MESSAGE_CHANNEL &&
+    ((!isChannelAgnosticSessionScope && (isChannelScopedSession || hasLegacyChannelPeerShape)) ||
+      (isConfiguredMainSessionScope && params.hasConnectedClient && !isFromWebchatClient)),
+  );
+  const hasDeliverableRoute =
+    canInheritDeliverableRoute &&
+    routeChannelCandidate &&
+    routeChannelCandidate !== INTERNAL_MESSAGE_CHANNEL &&
+    typeof routeToCandidate === "string" &&
+    routeToCandidate.trim().length > 0;
+
+  if (!hasDeliverableRoute) {
+    return {
+      originatingChannel: INTERNAL_MESSAGE_CHANNEL,
+      explicitDeliverRoute: false,
+    };
+  }
+
+  return {
+    originatingChannel: routeChannelCandidate,
+    originatingTo: routeToCandidate,
+    accountId: routeAccountIdCandidate,
+    messageThreadId: routeThreadIdCandidate,
+    explicitDeliverRoute: true,
+  };
+}
+
 function stripDisallowedChatControlChars(message: string): string {
  let output = "";
  for (const char of message) {
@@ -864,62 +977,20 @@ export const chatHandlers: GatewayRequestHandlers = {
      );
      const commandBody = injectThinking ? `/think ${p.thinking} ${parsedMessage}` : parsedMessage;
      const clientInfo = client?.connect?.client;
-      const shouldDeliverExternally = p.deliver === true;
-      const routeChannelCandidate = normalizeMessageChannel(
-        entry?.deliveryContext?.channel ?? entry?.lastChannel,
-      );
-      const routeToCandidate = entry?.deliveryContext?.to ?? entry?.lastTo;
-      const routeAccountIdCandidate =
-        entry?.deliveryContext?.accountId ?? entry?.lastAccountId ?? undefined;
-      const routeThreadIdCandidate = entry?.deliveryContext?.threadId ?? entry?.lastThreadId;
-      const parsedSessionKey = parseAgentSessionKey(sessionKey);
-      const sessionScopeParts = (parsedSessionKey?.rest ?? sessionKey).split(":").filter(Boolean);
-      const sessionScopeHead = sessionScopeParts[0];
-      const sessionChannelHint = normalizeMessageChannel(sessionScopeHead);
-      const normalizedSessionScopeHead = (sessionScopeHead ?? "").trim().toLowerCase();
-      const sessionPeerShapeCandidates = [sessionScopeParts[1], sessionScopeParts[2]]
-        .map((part) => (part ?? "").trim().toLowerCase())
-        .filter(Boolean);
-      const isChannelAgnosticSessionScope = CHANNEL_AGNOSTIC_SESSION_SCOPES.has(
-        normalizedSessionScopeHead,
-      );
-      const isChannelScopedSession = sessionPeerShapeCandidates.some((part) =>
-        CHANNEL_SCOPED_SESSION_SHAPES.has(part),
-      );
-      const hasLegacyChannelPeerShape =
-        !isChannelScopedSession &&
-        typeof sessionScopeParts[1] === "string" &&
-        sessionChannelHint === routeChannelCandidate;
-      const clientMode = client?.connect?.client?.mode;
-      const isFromWebchatClient =
-        isWebchatClient(client?.connect?.client) || clientMode === GATEWAY_CLIENT_MODES.UI;
-      const configuredMainKey = (cfg.session?.mainKey ?? "main").trim().toLowerCase();
-      const isConfiguredMainSessionScope =
-        normalizedSessionScopeHead.length > 0 && normalizedSessionScopeHead === configuredMainKey;
-      // Channel-agnostic session scopes (main, direct:<peer>, etc.) can leak
-      // stale routes across surfaces. Allow configured main sessions from
-      // non-Webchat/UI clients (e.g., CLI, backend) to keep the last external route.
-      const canInheritDeliverableRoute = Boolean(
-        sessionChannelHint &&
-        sessionChannelHint !== INTERNAL_MESSAGE_CHANNEL &&
-        ((!isChannelAgnosticSessionScope &&
-          (isChannelScopedSession || hasLegacyChannelPeerShape)) ||
-          (isConfiguredMainSessionScope && client?.connect !== undefined && !isFromWebchatClient)),
-      );
-      const hasDeliverableRoute = Boolean(
-        shouldDeliverExternally &&
-        canInheritDeliverableRoute &&
-        routeChannelCandidate &&
-        routeChannelCandidate !== INTERNAL_MESSAGE_CHANNEL &&
-        typeof routeToCandidate === "string" &&
-        routeToCandidate.trim().length > 0,
-      );
-      const originatingChannel = hasDeliverableRoute
-        ? routeChannelCandidate
-        : INTERNAL_MESSAGE_CHANNEL;
-      const originatingTo = hasDeliverableRoute ? routeToCandidate : undefined;
-      const accountId = hasDeliverableRoute ? routeAccountIdCandidate : undefined;
-      const messageThreadId = hasDeliverableRoute ? routeThreadIdCandidate : undefined;
+      const {
+        originatingChannel,
+        originatingTo,
+        accountId,
+        messageThreadId,
+        explicitDeliverRoute,
+      } = resolveChatSendOriginatingRoute({
+        client: clientInfo,
+        deliver: p.deliver,
+        entry,
+        hasConnectedClient: client?.connect !== undefined,
+        mainKey: cfg.session?.mainKey,
+        sessionKey,
+      });
      // Inject timestamp so agents know the current date/time.
      // Only BodyForAgent gets the timestamp — Body stays raw for UI display.
      // See: https://github.com/moltbot/moltbot/issues/3658
@@ -936,7 +1007,7 @@ export const chatHandlers: GatewayRequestHandlers = {
        Surface: INTERNAL_MESSAGE_CHANNEL,
        OriginatingChannel: originatingChannel,
        OriginatingTo: originatingTo,
-        ExplicitDeliverRoute: hasDeliverableRoute,
+        ExplicitDeliverRoute: explicitDeliverRoute,
        AccountId: accountId,
        MessageThreadId: messageThreadId,
        ChatType: "direct",