fix: Don’t leak STRIP_TRANSIENTS action (#961)

* fix(master): Don’t crash master if action or payload missing

Return error instead of crashing when the master receives an undefined action or an action without a `payload` field.

* fix(client): Don’t forward `STRIP_TRANSIENTS` action to transports

Author: delucis

src/client/client.test.ts

@@ -22,6 +22,7 @@ import {
   gameEvent,
   patch,
 } from '../core/action-creators';
+import * as Actions from '../core/action-types';
 import Debug from './debug/Debug.svelte';
 import { error } from '../core/logger';
 import type { LogEntry, State, SyncInfo } from '../types';
@@ -144,7 +145,7 @@ describe('multiplayer', () => {
 
     beforeAll(() => {
       client = Client({
-        game: { moves: { A: () => {} } },
+        game: { moves: { A: () => {}, Invalid: () => INVALID_MOVE } },
         multiplayer: SocketIO({ server: host + ':' + port }),
       });
       client.start();
@@ -173,6 +174,18 @@ describe('multiplayer', () => {
       expect(client.transport.onAction).toHaveBeenCalled();
     });
 
+    test('strip transients action not sent to transport', () => {
+      jest.spyOn(client.transport, 'onAction');
+      const state = { G: {}, ctx: { phase: '' }, plugins: {} };
+      const filteredMetadata = [];
+      client.store.dispatch(sync({ state, filteredMetadata } as SyncInfo));
+      client.moves.Invalid();
+      expect(client.transport.onAction).not.toHaveBeenCalledWith(
+        expect.any(Object),
+        { type: Actions.STRIP_TRANSIENTS }
+      );
+    });
+
     test('Sends and receives chat messages', () => {
       jest.spyOn(client.transport, 'onAction');
       client.updatePlayerID('0');

src/client/client.ts

@@ -43,7 +43,10 @@ type ClientAction =
   | ActionShape.Sync
   | ActionShape.Update
   | ActionShape.Patch;
-type Action = CredentialedActionShape.Any | ClientAction;
+type Action =
+  | CredentialedActionShape.Any
+  | ActionShape.StripTransients
+  | ClientAction;
 
 export interface DebugOpt {
   target?: HTMLElement;
@@ -287,7 +290,10 @@ export class _ClientImpl<G extends any = any> {
       const baseState = store.getState();
       const result = next(action);
 
-      if (!('clientOnly' in action)) {
+      if (
+        !('clientOnly' in action) &&
+        action.type !== Actions.STRIP_TRANSIENTS
+      ) {
         this.transport.onAction(baseState, action);
       }
 

src/master/master.test.ts

@@ -233,6 +233,18 @@ describe('update', () => {
     ]);
   });
 
+  test('missing action', async () => {
+    const { error } = await master.onUpdate(null, 0, 'matchID', '0');
+    expect(sendAll).not.toHaveBeenCalled();
+    expect(error).toBe('missing action or action payload');
+  });
+
+  test('missing action payload', async () => {
+    const { error } = await master.onUpdate({}, 0, 'matchID', '0');
+    expect(sendAll).not.toHaveBeenCalled();
+    expect(error).toBe('missing action or action payload');
+  });
+
   test('invalid matchID', async () => {
     await master.onUpdate(action, 0, 'default:unknown', '1');
     expect(sendAll).not.toHaveBeenCalled();

src/master/master.ts

@@ -161,6 +161,10 @@ export class Master {
     matchID: string,
     playerID: string
   ): Promise<void | { error: string }> {
+    if (!credAction || !credAction.payload) {
+      return { error: 'missing action or action payload' };
+    }
+
     let metadata: Server.MatchData | undefined;
     if (StorageAPI.isSynchronous(this.storageAPI)) {
       ({ metadata } = this.storageAPI.fetch(matchID, { metadata: true }));

src/types.ts

@@ -431,7 +431,9 @@ export namespace ActionShape {
   export type Redo = StripCredentials<CredentialedActionShape.Redo>;
   // Private type used only for internal error processing.
   // Included here to preserve type-checking of reducer inputs.
-  type _StripTransients = ReturnType<typeof ActionCreators.stripTransients>;
+  export type StripTransients = ReturnType<
+    typeof ActionCreators.stripTransients
+  >;
   export type Any =
     | MakeMove
     | GameEvent
@@ -443,7 +445,7 @@ export namespace ActionShape {
     | Undo
     | Redo
     | Plugin
-    | _StripTransients;
+    | StripTransients;
 }
 
 export namespace ActionPayload {