refactor: prevent custom field overriding prototype property

apps/client/src/common/hooks/useSocket.ts

@@ -139,7 +139,9 @@ export const setEventPlayback = {
   pause: () => sendSocket('pause', undefined),
 };
 
-export const useTimer = createSelector((state: RuntimeStore) => state.timer);
+export const useTimer = createSelector((state: RuntimeStore) => ({
+  ...state.timer,
+}));
 
 export const useClock = createSelector((state: RuntimeStore) => state.clock);
 

apps/server/src/adapters/WebsocketAdapter.ts

@@ -223,14 +223,9 @@ class SocketServer implements IAdapter {
 
   // message is any serializable value
   public sendAsJson<T extends MessageTag>(tag: T, payload: Pick<WsPacketToClient & { tag: T }, 'payload'>['payload']) {
-    const wss = this.wss;
-    if (!wss || wss.clients.size === 0) {
-      return;
-    }
-
     try {
       const stringifiedMessage = JSON.stringify({ tag, payload });
-      wss.clients.forEach((client) => {
+      this.wss?.clients.forEach((client) => {
         if (client.readyState === WebSocket.OPEN) {
           client.send(stringifiedMessage);
         }

apps/server/src/api-data/custom-fields/__tests__/customFields.parser.test.ts

@@ -126,4 +126,15 @@ describe('sanitiseCustomFields()', () => {
     const sanitationResult = sanitiseCustomFields(customFields);
     expect(sanitationResult).toStrictEqual(expectedCustomFields);
   });
+
+  it('filters keys that collide with Object prototype properties', () => {
+    const customFields: CustomFields = {
+      toString: { label: 'toString', type: 'text', colour: 'red' },
+      normalField: { label: 'normalField', type: 'text', colour: 'green' },
+    };
+    const sanitationResult = sanitiseCustomFields(customFields);
+    expect(sanitationResult).toStrictEqual({
+      normalField: { label: 'normalField', type: 'text', colour: 'green' },
+    });
+  });
 });

apps/server/src/api-data/custom-fields/customFields.parser.ts

@@ -1,5 +1,5 @@
 import { CustomField, CustomFields, DatabaseModel } from 'ontime-types';
-import { checkRegex, customFieldLabelToKey } from 'ontime-utils';
+import { checkRegex, customFieldLabelToKey, isObjectPrototypeKey } from 'ontime-utils';
 
 import type { ErrorEmitter } from '../../utils/parserUtils.js';
 
@@ -45,6 +45,7 @@ export function sanitiseCustomFields(data: object): CustomFields {
       'type' in data &&
       (data.type === 'text' || data.type === 'image') &&
       checkRegex.isAlphanumericWithSpace(data.label) &&
+      !isObjectPrototypeKey(key) &&
       key === customFieldLabelToKey(data.label)
     );
   }

apps/server/src/api-data/custom-fields/customFields.validation.ts

@@ -1,5 +1,5 @@
 import { body, param } from 'express-validator';
-import { checkRegex } from 'ontime-utils';
+import { checkRegex, customFieldLabelToKey, isObjectPrototypeKey } from 'ontime-utils';
 
 import { requestValidationFunction } from '../validation-utils/validationFunction.js';
 
@@ -9,7 +9,7 @@ export const validateCustomField = [
     .trim()
     .notEmpty()
     .custom((value) => {
-      return checkRegex.isAlphanumericWithSpace(value);
+      return checkRegex.isAlphanumericWithSpace(value) && !isObjectPrototypeKey(customFieldLabelToKey(value));
     }),
   body('type').isIn(['text', 'image']),
   body('colour').isString().trim(),
@@ -24,7 +24,7 @@ export const validateEditCustomField = [
     .trim()
     .notEmpty()
     .custom((value) => {
-      return checkRegex.isAlphanumericWithSpace(value);
+      return checkRegex.isAlphanumericWithSpace(value) && !isObjectPrototypeKey(customFieldLabelToKey(value));
     }),
   body('type').isIn(['text', 'image']),
   body('colour').isString().trim(),

apps/server/src/api-data/rundown/rundown.dao.ts

@@ -186,8 +186,7 @@ function remove(rundown: Rundown, entry: OntimeEntry) {
     // for ontime groups, we need to iterate through the children and delete them
     for (let i = 0; i < entry.entries.length; i++) {
       const nestedEntryId = entry.entries[i];
-      // nested entries are not part of the top-level order
+      deleteEntry(nestedEntryId);
-      deleteEntry(nestedEntryId, false);
     }
   } else if (entry.parent) {
     // at this point, we are handling entries inside a group, so we need to remove the reference
@@ -206,13 +205,10 @@ function remove(rundown: Rundown, entry: OntimeEntry) {
       edit(rundown, { id: parentGroup.id, entries: filteredEvents });
     }
   }
-
   deleteEntry(entry.id);
 
-  function deleteEntry(idToDelete: EntryId, shouldDeleteFromOrder: boolean = true) {
+  function deleteEntry(idToDelete: EntryId) {
-    if (shouldDeleteFromOrder) {
+    rundown.order = deleteById(rundown.order, idToDelete);
-      rundown.order = deleteById(rundown.order, idToDelete);
-    }
     delete rundown.entries[idToDelete];
   }
 }

apps/server/src/api-data/rundown/rundown.parser.ts

@@ -188,7 +188,7 @@ export function parseRundown(
  */
 export function sanitiseCustomFields(customFields: CustomFields, entry: OntimeEvent | OntimeMilestone | OntimeGroup) {
   for (const field in entry.custom) {
-    if (field in customFields) continue;
+    if (Object.hasOwn(customFields, field)) continue;
     delete entry.custom[field];
   }
   return entry;

apps/server/src/api-data/rundown/rundown.service.ts

@@ -16,7 +16,7 @@ import {
   isOntimeEvent,
   isOntimeGroup,
 } from 'ontime-types';
-import { customFieldLabelToKey, getInsertAfterId, resolveInsertParent } from 'ontime-utils';
+import { customFieldLabelToKey, getInsertAfterId, isObjectPrototypeKey, resolveInsertParent } from 'ontime-utils';
 
 import { sendRefetch } from '../../adapters/WebsocketAdapter.js';
 import { getDataProvider } from '../../classes/data-provider/DataProvider.js';
@@ -140,6 +140,7 @@ export async function batchEditEntries(ids: EntryId[], patch: Partial<OntimeEntr
 
   let batchDidInvalidate = false;
   const changedIds: EntryId[] = [];
+  const patchedEntries: OntimeEntry[] = [];
   for (let i = 0; i < ids.length; i++) {
     const currentId = ids[i];
     const currentEntry = rundown.entries[currentId];
@@ -164,20 +165,15 @@ export async function batchEditEntries(ids: EntryId[], patch: Partial<OntimeEntr
       continue;
     }
 
-    const { didInvalidate } = rundownMutation.edit(rundown, { ...patch, id: currentId });
+    const { entry, didInvalidate } = rundownMutation.edit(rundown, { ...patch, id: currentId });
 
     changedIds.push(currentId);
+    patchedEntries.push(entry);
 
     if (didInvalidate) {
       batchDidInvalidate = true;
     }
   }
-
-  // skip commit and notifications when the patch is no-op.
-  if (changedIds.length === 0) {
-    return getCurrentRundown() as Rundown;
-  }
-
   const { rundown: rundownResult, rundownMetadata, revision } = commit(batchDidInvalidate);
 
   // schedule the side effects
@@ -196,14 +192,7 @@ export async function batchEditEntries(ids: EntryId[], patch: Partial<OntimeEntr
  * Deletes a known entry from the current rundown
  */
 export async function deleteEntries(entryIds: EntryId[]): Promise<Rundown> {
-  const currentRundown = getCurrentRundown();
-  const shouldDelete = entryIds.some((entryId) => Object.hasOwn(currentRundown.entries, entryId));
-  if (!shouldDelete) {
-    return currentRundown as Rundown;
-  }
-
   const { rundown, commit } = createTransaction({ mutableRundown: true, mutableCustomFields: false });
-  const deletedIds: EntryId[] = [];
 
   for (let i = 0; i < entryIds.length; i++) {
     const entry = rundown.entries[entryIds[i]];
@@ -211,11 +200,6 @@ export async function deleteEntries(entryIds: EntryId[]): Promise<Rundown> {
       continue;
     }
     rundownMutation.remove(rundown, entry);
-    deletedIds.push(entry.id);
-  }
-
-  if (deletedIds.length === 0) {
-    return currentRundown as Rundown;
   }
 
   const { rundown: rundownResult, rundownMetadata, revision } = commit();
@@ -226,7 +210,7 @@ export async function deleteEntries(entryIds: EntryId[]): Promise<Rundown> {
     updateRuntimeOnChange(rundownMetadata);
 
     // notify timer and external services of change
-    notifyChanges(rundownMetadata, revision, { timer: deletedIds, external: true });
+    notifyChanges(rundownMetadata, revision, { timer: entryIds, external: true });
   });
 
   return rundownResult;
@@ -441,6 +425,9 @@ export async function createCustomField(customField: CustomField): Promise<Custo
   if (!key) {
     throw new Error('Unable to convert label to a valid key');
   }
+  if (isObjectPrototypeKey(key)) {
+    throw new Error('Label conflicts with a reserved field name');
+  }
 
   const { customFields, commit } = createTransaction({ mutableRundown: false, mutableCustomFields: true });
 
@@ -479,7 +466,7 @@ export async function editCustomField(
     mutableCustomFields: true,
   });
 
-  if (!(key in customFields)) {
+  if (!Object.hasOwn(customFields, key)) {
     throw new Error('Could not find label');
   }
 
@@ -488,6 +475,9 @@ export async function editCustomField(
   if (newField.type && existingField.type !== newField.type) {
     throw new Error('Change of field type is not allowed');
   }
+  if (newField.label && isObjectPrototypeKey(customFieldLabelToKey(newField.label))) {
+    throw new Error('Label conflicts with a reserved field name');
+  }
 
   const { oldKey, newKey } = customFieldMutation.edit(customFields, key, existingField, newField);
 
@@ -529,7 +519,7 @@ export async function deleteCustomField(key: CustomFieldKey, projectRundowns: Pr
     mutableRundown: true,
     mutableCustomFields: true,
   });
-  if (!(key in customFields)) {
+  if (!Object.hasOwn(customFields, key)) {
     return customFields;
   }
 
@@ -587,10 +577,8 @@ type NotifyChangesOptions = {
  * Notify services of changes in the rundown
  */
 function notifyChanges(rundownMetadata: RundownMetadata, revision: number, options: NotifyChangesOptions) {
-  const shouldNotifyTimer = options.timer === true || (Array.isArray(options.timer) && options.timer.length > 0);
-
   // notify timer service of changed events
-  if (shouldNotifyTimer) {
+  if (options.timer) {
     runtimeService.notifyOfChangedEvents(rundownMetadata);
   }
 

packages/utils/index.ts

@@ -61,7 +61,7 @@ export { checkRegex, regex } from './src/regex-utils/checkRegex.js';
 export { isColourHex } from './src/regex-utils/isColourHex.js';
 export { splitWhitespace } from './src/regex-utils/splitWhitespace.js';
 
-export { customFieldLabelToKey, customKeyFromLabel } from './src/customField-utils/customFieldUtils.js';
+export { customFieldLabelToKey, customKeyFromLabel, isObjectPrototypeKey } from './src/customField-utils/customFieldUtils.js';
 
 // helpers from externals
 export { deepmerge } from './src/externals/deepmerge.js';

packages/utils/src/customField-utils/customFieldUtils.ts

@@ -1,5 +1,7 @@
 import type { CustomFields } from 'ontime-types';
 
+const objectPrototypeKeys = new Set(Object.getOwnPropertyNames(Object.prototype));
+
 /**
  * Transforms an alphanumeric label with spaces into a valid key
  */
@@ -7,6 +9,13 @@ export function customFieldLabelToKey(label: string): string {
   return label.trim().replaceAll(' ', '_');
 }
 
+/**
+ * Detects keys that collide with Object prototype properties or methods
+ */
+export function isObjectPrototypeKey(key: string): boolean {
+  return objectPrototypeKeys.has(key);
+}
+
 /**
  * Finds an object key in the CustomFields object that matches the given label
  */