🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

1. 🟠 Client mode spoofing allows unauthorized inheritance of external delivery routes (Telegram/WhatsApp)

Description

The gateway uses isGatewayCliClient() (based solely on the client-supplied client.mode string) to decide whether a caller may inherit the configured main session’s last external delivery route.

In resolveChatSendOriginatingRoute():

• isGatewayCliClient(params.client) returns true if client.mode === "cli" (normalized)
• For configured main sessions, the code allows external-route inheritance when the caller is considered CLI:
  • canInheritConfiguredMainRoute = ... (isFromGatewayCliClient || !hasClientMetadata)
• params.client is derived from the WebSocket handshake ConnectParams.client, which is provided by the connecting client and is not cryptographically bound to “CLI-ness” or otherwise server-attested.

As a result, any client that can connect and call chat.send can set client.mode to "cli" and be treated as a CLI client, enabling inheritance of the last external route for configured main sessions (e.g., agent:main:<mainKey>). This can cause unintended external message dispatch and potential data disclosure to external channels stored in the main session’s deliveryContext (Telegram/WhatsApp/etc.).

Vulnerable decision point:

const isFromGatewayCliClient = isGatewayCliClient(params.client);
...
const canInheritConfiguredMainRoute =
  isConfiguredMainSessionScope &&
  params.hasConnectedClient &&
  (isFromGatewayCliClient || !hasClientMetadata);

isGatewayCliClient() implementation is also purely mode-based:

export function isGatewayCliClient(client?: GatewayClientInfoLike | null): boolean {
  return normalizeGatewayClientMode(client?.mode) === GATEWAY_CLIENT_MODES.CLI;
}

Other call sites of isGatewayCliClient appear non-security-sensitive (presence tracking, auth failure message hints), but this chat.send routing decision is security-sensitive.

Recommendation

Do not make authorization/routing decisions based on client-declared metadata fields (client.mode, client.id). Harden CLI detection and route inheritance.

Recommended options (in increasing robustness):

1. Server-attested connection kind

   • During the WS handshake, derive a server-side, non-user-controlled flag (e.g., conn.isBrowser = hasBrowserOriginHeader, conn.clientKind = "cli" | "browser" | ...) and store it in the connection context.
   • Gate configured-main inheritance on this server-side property rather than client.mode.

2. Bind “CLI eligibility” to pairing/device identity (best)

   • Persist an allowed set of client modes/IDs on the paired device record (or on issued device tokens).
   • On connect, reject or downgrade when a device attempts to change its mode to cli without re-approval.

3. Explicit permission/scope

   • Require a specific scope/permission (e.g., session.route.inherit_external) to inherit configured-main external routes.

Concrete example approach (server-attested flag):

​// During WS handshake (server-side):
const isBrowser = Boolean(requestOrigin); // or hasBrowserOriginHeader
clientContext.isGatewayCli = !isBrowser && connectParams.client.mode === GATEWAY_CLIENT_MODES.CLI;

​// In chat.ts:
const isFromGatewayCliClient = Boolean(params.clientContext?.isGatewayCli);

Additionally, consider removing the || !hasClientMetadata legacy bypass for configured-main inheritance (or restricting it to a very narrow, server-trusted compatibility path), since it also represents a spoofable/ambiguous classification if any pathway allows missing client metadata.

Analyzed PR: #43918 at commit 8d87283

_{Last updated on: 2026-03-12T11:00:53Z}

Greptile Summary

This PR fixes a routing bug where mode:UI gateway clients calling chat.send on a configured main session (e.g. agent:main:main) with deliver:true would incorrectly inherit the session's persisted external delivery route (e.g. Telegram), causing replies to be delivered to the wrong channel instead of staying on the internal UI surface.

The fix tightens the canInheritConfiguredMainRoute predicate in resolveChatSendOriginatingRoute so that configured-main route inheritance is limited to CLI callers or legacy callers with no client metadata — UI-mode clients are now explicitly excluded. Channel-scoped session inheritance behavior is unchanged.

Key changes:

• chat.ts: Introduces isFromGatewayCliClient and hasClientMetadata checks to gate configured-main route inheritance; only CLI or no-metadata clients inherit the persisted external route on main sessions.
• chat.directive-tags.test.ts: Adds a targeted regression test that correctly exercises the bug path (deliver:true + mode:UI client + agent:main:main session key), confirming OriginatingChannel stays as webchat and ExplicitDeliverRoute is false.

Minor observation: The pre-existing test "chat.send does not inherit external delivery context for UI clients on main sessions" (line 618) does not pass deliver:true, so it was always passing via the early-return path in resolveChatSendOriginatingRoute without ever exercising the route-inheritance logic. This means it was not an effective guard against the regression before this PR — the new test at line 659 is the first test to actually cover it.

Confidence Score: 4/5

• This PR is safe to merge; the logic change is well-scoped and the critical delivery-routing path is now properly gated.
• The core fix is logically correct and surgically scoped to configured-main sessions. CLI and legacy no-metadata callers retain existing inheritance behavior; channel-scoped session inheritance is untouched. The new regression test correctly exercises the actual bug path (deliver:true on a UI-mode client). Score is 4 rather than 5 because the pre-existing test (line 618) was not actually guarding this path — it was trivially passing via the early-return branch — so there is a small gap in test fidelity that the PR's new test remedies but the old test still leaves slightly misleading.
• No files require special attention beyond the minor test observation in chat.directive-tags.test.ts.

This is a comment left during a code review.
Path: src/gateway/server-methods/chat.directive-tags.test.ts
Line: 618-656

Comment:
**Pre-existing test provides only trivial coverage for this bug**

The pre-existing test `"chat.send does not inherit external delivery context for UI clients on main sessions"` (line 618) does **not** pass `deliver: true`. This means `shouldDeliverExternally` is `false`, triggering the early-return path in `resolveChatSendOriginatingRoute` before any route-inheritance logic is evaluated — so the test passes regardless of the inheritance fix and does not exercise the actual bug.

The new test added at line 659 correctly adds `deliver: true` to exercise the inheritance logic. It would be worth updating the pre-existing test (or adding a note) to clarify that it only validates the "no-deliver early return" path, not route inheritance. Additionally, this test's assertion does not check `ExplicitDeliverRoute`, unlike the new one — worth aligning for completeness:

```suggestion
    expect(mockState.lastDispatchCtx).toEqual(
      expect.objectContaining({
        OriginatingChannel: "webchat",
        OriginatingTo: undefined,
        ExplicitDeliverRoute: false,
        AccountId: undefined,
      }),
    );
```

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: 5713f7b}

Landed on main.

• Landed source commit: 8d87283
• Merge commit: 8582cb0