fix: add room membership checks. Broadcast errors to room only

packages/sdk-socket-server-next/e2e/api-config.test.ts

@@ -1,5 +1,5 @@
 import request from 'supertest';
-import { app } from '../src/api-config';
+import { app } from '../src/analytics-api';
 
 jest.mock('analytics-node');
 

packages/sdk-socket-server-next/jest.config.ts

@@ -0,0 +1,18 @@
+import baseConfig from '../../jest.config.base';
+
+module.exports = {
+  ...baseConfig,
+  testMatch: [
+    '**/__tests__/**/*.[jt]s?(x)',
+    '**/?(*.)+(spec|test).[tj]s?(x)',
+  ],
+  testPathIgnorePatterns: ['/node_modules/', '/dist/'],
+  setupFiles: ['<rootDir>/jest.setup.ts'],
+  moduleNameMapper: {
+    '^analytics-node$': '<rootDir>/e2e/analytics-node.ts',
+  },
+  clearMocks: true,
+  resetMocks: false,
+  restoreMocks: false,
+  watchman: false,
+};

packages/sdk-socket-server-next/jest.setup.ts

@@ -0,0 +1,12 @@
+/* eslint-disable node/no-process-env */
+// Jest setup: ensure required environment variables are present so that
+// `analytics-api` does not call `process.exit(1)` when it loads.
+process.env.REDIS_NODES =
+  process.env.REDIS_NODES ?? 'redis://localhost:6379';
+process.env.NODE_ENV = process.env.NODE_ENV ?? 'test';
+
+// Some tests mock `./analytics-api` so the side-effectful logger setup in
+// `./config` never runs. Initialise the logger here so `getLogger()` returns
+// a real winston logger from any module under test.
+// eslint-disable-next-line import/no-unassigned-import
+import './src/config';

packages/sdk-socket-server-next/src/protocol/handleChannelRejected.test.ts

@@ -0,0 +1,191 @@
+/* eslint-disable jsdoc/require-jsdoc */
+import { v4 as uuidv4 } from 'uuid';
+
+const mockPubClient = {
+  get: jest.fn(),
+  setex: jest.fn(),
+};
+
+jest.mock('../analytics-api', () => ({
+  pubClient: mockPubClient,
+}));
+
+jest.mock('@socket.io/redis-adapter', () => ({
+  createAdapter: jest.fn(),
+}));
+
+import {
+  handleChannelRejected,
+  ChannelRejectedParams,
+} from './handleChannelRejected';
+import { ChannelConfig } from './handleJoinChannel';
+
+function makeSocket({
+  rooms,
+  socketId = 'socket-id-1',
+}: {
+  rooms: string[];
+  socketId?: string;
+}) {
+  const broadcastToEmit = jest.fn();
+  return {
+    id: socketId,
+    request: { socket: { remoteAddress: '127.0.0.1' } },
+    rooms: new Set(rooms),
+    broadcast: {
+      to: jest.fn(() => ({ emit: broadcastToEmit })),
+    },
+  } as any;
+}
+
+describe('handleChannelRejected participant check (HackerOne 3604630)', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('rejects requests from a non-participant socket on a fresh channel', async () => {
+    const channelId = uuidv4();
+    const socket = makeSocket({ rooms: [] });
+
+    // No existing channelConfig: this is a fresh channelId an attacker is
+    // poking at.
+    mockPubClient.get.mockResolvedValueOnce(null);
+
+    const callback = jest.fn();
+    const params: ChannelRejectedParams = {
+      io: {} as any,
+      socket,
+      channelId,
+    };
+
+    await handleChannelRejected(params, callback);
+
+    expect(callback).toHaveBeenCalledWith('not authorized', undefined);
+    // Must not write any rejected entry to redis on a non-participant request.
+    expect(mockPubClient.setex).not.toHaveBeenCalled();
+    expect(socket.broadcast.to).not.toHaveBeenCalled();
+  });
+
+  it('rejects requests from a non-participant socket even when a channelConfig with no wallet exists', async () => {
+    const channelId = uuidv4();
+    const socket = makeSocket({ rooms: [] });
+
+    // Existing config but no wallet recorded yet (e.g. only the dapp has
+    // joined). The reconnect-and-reject flow only applies to wallets that
+    // had previously joined.
+    const existingConfig: ChannelConfig = {
+      clients: { dapp: 'dapp-socket-id', wallet: '' },
+      createdAt: 1,
+      updatedAt: 1,
+    };
+    mockPubClient.get.mockResolvedValueOnce(JSON.stringify(existingConfig));
+
+    const callback = jest.fn();
+    const params: ChannelRejectedParams = {
+      io: {} as any,
+      socket,
+      channelId,
+    };
+
+    await handleChannelRejected(params, callback);
+
+    expect(callback).toHaveBeenCalledWith('not authorized', undefined);
+    expect(mockPubClient.setex).not.toHaveBeenCalled();
+  });
+
+  it('rejects requests with an invalid channelId', async () => {
+    const socket = makeSocket({ rooms: [] });
+    const callback = jest.fn();
+    const params: ChannelRejectedParams = {
+      io: {} as any,
+      socket,
+      channelId: 'not-a-uuid',
+    };
+
+    await handleChannelRejected(params, callback);
+
+    expect(callback).toHaveBeenCalledWith('error_id', undefined);
+    expect(mockPubClient.get).not.toHaveBeenCalled();
+    expect(mockPubClient.setex).not.toHaveBeenCalled();
+  });
+
+  it('allows the request when the socket is a live in-room participant', async () => {
+    const channelId = uuidv4();
+    const socket = makeSocket({ rooms: [channelId] });
+
+    mockPubClient.get.mockResolvedValueOnce(null);
+    mockPubClient.setex.mockResolvedValueOnce('OK');
+
+    const callback = jest.fn();
+    const params: ChannelRejectedParams = {
+      io: {} as any,
+      socket,
+      channelId,
+    };
+
+    await handleChannelRejected(params, callback);
+
+    expect(mockPubClient.setex).toHaveBeenCalledTimes(1);
+    expect(callback).toHaveBeenCalledWith(null, { success: true });
+  });
+
+  it('allows the post-reconnect reject flow when channelConfig has a known wallet', async () => {
+    const channelId = uuidv4();
+    // Wallet has reconnected, so its socket is not in the room.
+    const socket = makeSocket({
+      rooms: [],
+      socketId: 'wallet-reconnected-socket-id',
+    });
+
+    const existingConfig: ChannelConfig = {
+      clients: {
+        wallet: 'previous-wallet-socket-id',
+        dapp: 'dapp-socket-id',
+      },
+      createdAt: 1,
+      updatedAt: 1,
+    };
+    mockPubClient.get.mockResolvedValueOnce(JSON.stringify(existingConfig));
+    mockPubClient.setex.mockResolvedValueOnce('OK');
+
+    const callback = jest.fn();
+    const params: ChannelRejectedParams = {
+      io: {} as any,
+      socket,
+      channelId,
+    };
+
+    await handleChannelRejected(params, callback);
+
+    expect(mockPubClient.setex).toHaveBeenCalledTimes(1);
+    const payload = JSON.parse(mockPubClient.setex.mock.calls[0][2]);
+    expect(payload.rejected).toBe(true);
+
+    expect(socket.broadcast.to).toHaveBeenCalledWith(channelId);
+    expect(callback).toHaveBeenCalledWith(null, { success: true });
+  });
+
+  it('does not modify a channel that is already in the ready state', async () => {
+    const channelId = uuidv4();
+    const socket = makeSocket({ rooms: [channelId] });
+
+    const existingConfig: ChannelConfig = {
+      clients: { wallet: 'wallet-id', dapp: 'dapp-id' },
+      ready: true,
+      createdAt: 1,
+      updatedAt: 1,
+    };
+    mockPubClient.get.mockResolvedValueOnce(JSON.stringify(existingConfig));
+
+    const callback = jest.fn();
+    const params: ChannelRejectedParams = {
+      io: {} as any,
+      socket,
+      channelId,
+    };
+
+    await handleChannelRejected(params, callback);
+
+    expect(mockPubClient.setex).not.toHaveBeenCalled();
+  });
+});

packages/sdk-socket-server-next/src/protocol/handleChannelRejected.ts

@@ -1,4 +1,5 @@
 import { Server, Socket } from 'socket.io';
+import { validate } from 'uuid';
 import { pubClient } from '../analytics-api';
 import { config } from '../config';
 import { getLogger } from '../logger';
@@ -27,13 +28,38 @@ export const handleChannelRejected = async (
   const socketId = socket.id;
   const clientIp = socket.request.socket.remoteAddress;
 
+  if (!validate(channelId)) {
+    logger.warn(`[handleChannelRejected] ${channelId} invalid channelId`, {
+      channelId,
+      socketId,
+      clientIp,
+    });
+    callback?.('error_id', undefined);
+    return;
+  }
+
   // Force keys into the same hash slot in Redis Cluster, using a hash tag (a substring enclosed in curly braces {})
   const channelConfigKey = `channel_config:{${channelId}}`;
   const existingConfig = await pubClient.get(channelConfigKey);
   let channelConfig: ChannelConfig | null = existingConfig
     ? (JSON.parse(existingConfig) as ChannelConfig)
     : null;
 
+  const isInRoom = socket.rooms.has(channelId);
+  const isKnownWalletParticipant = Boolean(channelConfig?.clients?.wallet);
+  if (!isInRoom && !isKnownWalletParticipant) {
+    logger.warn(
+      `[handleChannelRejected] non-participant rejected request for ${channelId}`,
+      {
+        channelId,
+        socketId,
+        clientIp,
+      },
+    );
+    callback?.('not authorized', undefined);
+    return;
+  }
+
   if (channelConfig) {
     logger.debug(
       `[handleChannelRejected] Channel already exists: ${channelId}`,

packages/sdk-socket-server-next/src/protocol/handleMessage.test.ts

@@ -0,0 +1,95 @@
+/* eslint-disable jsdoc/require-jsdoc */
+import { v4 as uuidv4 } from 'uuid';
+
+const mockPubClient = {
+  get: jest.fn(),
+  set: jest.fn(),
+  rpush: jest.fn(),
+  expire: jest.fn(),
+};
+
+jest.mock('../analytics-api', () => ({
+  pubClient: mockPubClient,
+}));
+
+jest.mock('../rate-limiter', () => ({
+  rateLimiterMessage: { consume: jest.fn().mockResolvedValue(undefined) },
+  resetRateLimits: jest.fn(),
+  increaseRateLimits: jest.fn(),
+  setLastConnectionErrorTimestamp: jest.fn(),
+}));
+
+jest.mock('@socket.io/redis-adapter', () => ({
+  createAdapter: jest.fn(),
+}));
+
+import { handleMessage, MessageParams } from './handleMessage';
+
+type Emit = jest.Mock;
+
+function makeSocket(channelId: string) {
+  const broadcastToEmit: Emit = jest.fn();
+  const broadcastEmit: Emit = jest.fn();
+
+  const broadcast = {
+    to: jest.fn(() => ({ emit: broadcastToEmit })),
+    emit: broadcastEmit,
+  };
+
+  return {
+    socket: {
+      id: 'socket-id-1',
+      handshake: { address: '127.0.0.1' },
+      request: { socket: { remoteAddress: '127.0.0.1' } },
+      rooms: new Set([channelId]),
+      broadcast,
+      emit: jest.fn(),
+    } as any,
+    broadcastToEmit,
+    broadcastEmit,
+    broadcastTo: broadcast.to,
+  };
+}
+
+describe('handleMessage error path', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('scopes the error broadcast to the channel room (regression: HackerOne 3604630)', async () => {
+    const channelId = uuidv4();
+    const { socket, broadcastTo, broadcastToEmit, broadcastEmit } =
+      makeSocket(channelId);
+
+    // Force handleMessage to throw inside its try-block by making
+    // pubClient.get reject.
+    mockPubClient.get.mockRejectedValueOnce(new Error('boom'));
+
+    const callback = jest.fn();
+
+    const params: MessageParams = {
+      io: {} as any,
+      socket,
+      channelId,
+      clientType: 'dapp',
+      context: 'dapp',
+      message: 'encrypted-string',
+      hasRateLimit: false,
+      callback,
+    };
+
+    await handleMessage(params);
+
+    expect(broadcastTo).toHaveBeenCalledWith(channelId);
+    expect(broadcastToEmit).toHaveBeenCalledWith(
+      `message-${channelId}`,
+      expect.objectContaining({ error: 'boom' }),
+    );
+
+    // CRITICAL: error must NOT be broadcast to all sockets (which would
+    // leak the active channel ID).
+    expect(broadcastEmit).not.toHaveBeenCalled();
+
+    expect(callback).toHaveBeenCalledWith('boom');
+  });
+});

packages/sdk-socket-server-next/src/protocol/handleMessage.ts

@@ -114,7 +114,7 @@ export const handleMessage = async ({
       ackId = uuidv4();
       // Store in the correct message queue
       const otherQueue = clientType === 'dapp' ? 'wallet' : 'dapp';
-      // Force keys into the same hash slot in Redis Cluster, using a hash tag (a substring enclosed in curly braces {})  
+      // Force keys into the same hash slot in Redis Cluster, using a hash tag (a substring enclosed in curly braces {})
       const queueKey = `queue:{${channelId}}:${otherQueue}`;
       const persistedMsg: QueuedMessage = {
         message,
@@ -174,8 +174,7 @@ export const handleMessage = async ({
       clientIp,
     });
 
-    // emit an error message back to the client, if appropriate
-    socket.broadcast.emit(`message-${channelId}`, {
+    socket.broadcast.to(channelId).emit(`message-${channelId}`, {
       error: error instanceof Error ? error.message : 'Unknown error occurred',
     });