fix(security): the desktop code scanning alerts

src/main/actions/ExportTimelineAction.ts

@@ -38,6 +38,7 @@ import {
 } from '../services/exportPlan.js';
 import type { ExportFormat, ProjectTimelineTransition } from '@shared/index.js';
 import { EXPORTS_FOLDER } from '@shared/constants/videoFiles.js';
+import { trimChar } from '../utils/trimChar.js';
 
 
 interface TimelineClipData {
@@ -482,9 +483,11 @@ export class ExportTimelineAction extends BaseAction<ExportTimelineInput, { clip
    * no separators, no drive letters, none of the characters Windows rejects.
    */
   private sanitizeOutputName(name?: string): string {
-    const cleaned = (name ?? '')
-      .replace(/[<>:"/\\|?*\x00-\x1f]/g, '')
-      .replace(/\.+$/, '')
+    const cleaned = trimChar(
+      (name ?? '').replace(/[<>:"/\\|?*\x00-\x1f]/g, ''),
+      '.',
+      'end',
+    )
       .trim()
       .slice(0, 120);
 

src/main/actions/MoveClipToGameAction.ts

@@ -4,6 +4,7 @@ import { BaseAction } from './BaseAction.js';
 import { AppDataSource, VIDEOS_ROOT } from '../data-source.js';
 import { Clip } from '../entity/Clip.js';
 import { cleanupEmptyFolders } from '../utils/cleanupEmptyFolders.js';
+import { gameFolderPath } from '../services/gameFolder.js';
 
 export interface MoveClipToGameInput {
   clipId: number;
@@ -30,11 +31,14 @@ export class MoveClipToGameAction extends BaseAction<MoveClipToGameInput, MoveCl
     const oldPath = clip.filePath;
     const filename = clip.filename;
 
-    // Determine new path
-    const targetDir = path.join(VIDEOS_ROOT, targetGame);
+    // Refused before anything touches the disk, since a name holding a
+    // separator is a path and this moves somebody's recording to it.
+    const targetDir = gameFolderPath(VIDEOS_ROOT, targetGame);
     await fs.mkdir(targetDir, { recursive: true });
 
-    let newPath = path.join(targetDir, filename);
+    // The filename comes from the database, which the scan filled from disk,
+    // but a basename of it costs nothing and keeps the file in that folder.
+    let newPath = path.join(targetDir, path.basename(filename));
     let counter = 1;
 
     // Handle filename conflicts

src/main/routes/clips.ts

@@ -67,6 +67,7 @@ import {
 } from '../actions/BatchOperationsAction.js';
 import { ImportFilesAction } from '../actions/ImportFilesAction.js';
 import { MoveClipToGameAction } from '../actions/MoveClipToGameAction.js';
+import { gameFolderProblem } from '../services/gameFolder.js';
 import { ExportAudioAction } from '../actions/ExportAudioAction.js';
 import { GetClipCollectionsAction } from '../actions/GetClipCollectionsAction.js';
 import { OpenFileInExplorerAction } from '../actions/OpenFileInExplorerAction.js';
@@ -433,6 +434,9 @@ clipsRouter.post('/:id/move-to-game', asyncHandler(async (req, res) => {
     return res.status(400).json({ error: 'targetGame is required' });
   }
 
+  const problem = gameFolderProblem(targetGame.trim());
+  if (problem) return res.status(400).json({ error: problem });
+
   const action = new MoveClipToGameAction();
   const { clip } = await action.execute({ clipId: id, targetGame: targetGame.trim() });
 

src/main/routes/tagPatterns.ts

@@ -2,22 +2,21 @@ import express from 'express';
 import { AppDataSource } from '../data-source.js';
 import { TagPattern } from '../entity/TagPattern.js';
 import { asyncHandler } from '../utils/asyncHandler.js';
-import { TagPatternDTO } from '@shared/index.js';
+import { TagPatternDTO, tagPatternProblem } from '@shared/index.js';
 
 export const tagPatternsRouter = express.Router();
 
-/** Guards against a rule that cannot compile being saved and then thrown on every match. */
+/**
+ * Guards against a rule that cannot compile, or one that could backtrack for
+ * ever, being saved and then run against every clip. See `tagPatternProblem`.
+ */
 function invalidExpression(patterns: unknown): string | null {
   if (!Array.isArray(patterns) || patterns.length === 0) {
     return 'A pattern needs at least one expression';
   }
   for (const source of patterns) {
-    if (typeof source !== 'string' || !source.trim()) return 'An expression cannot be empty';
-    try {
-      new RegExp(source, 'i');
-    } catch {
-      return `"${source}" is not a valid expression`;
-    }
+    const problem = tagPatternProblem(source);
+    if (problem) return problem;
   }
   return null;
 }

src/main/services/clipSearch.ts

@@ -28,6 +28,8 @@
  * module chose, and nothing the user types is ever an operator.
  */
 
+import { trimChar } from '../utils/trimChar.js';
+
 /**
  * A search that will not be run as a query.
  *
@@ -201,7 +203,7 @@ function termsOf(raw: string | undefined | null): string[] {
 
   return raw
     .split(/[^\p{L}\p{N}']+/u)
-    .map((term) => term.replace(/^'+|'+$/g, ''))
+    .map((term) => trimChar(term, "'"))
     .filter((term) => term.length >= MIN_TERM_LENGTH)
     .slice(0, MAX_TERMS);
 }

src/main/services/gameFolder.ts

@@ -0,0 +1,54 @@
+import path from 'node:path';
+
+/**
+ * Characters Windows will not take in a folder name, plus the control range.
+ * The two separators are the ones that matter: with either of them a game name
+ * is a path, and `..\..\Windows` joined onto the videos root is somewhere else.
+ */
+const PROHIBITED = /[/\\:"<>*?|\x00-\x1f]/;
+
+/** Device names Windows reserves in every folder, with or without an extension. */
+const RESERVED = /^(con|prn|aux|nul|com[1-9]|lpt[1-9])(\..*)?$/i;
+
+/**
+ * Why a name cannot be a game folder, or null when it can.
+ *
+ * Refused rather than cleaned. `safeName` in `capture/gameNames.ts` cleans,
+ * which is right for a name read off a store; this is a name somebody typed,
+ * and moving their clip into a folder spelled differently from what they typed
+ * would be a surprise found later.
+ */
+export function gameFolderProblem(name: string): string | null {
+  if (!name) return 'A game needs a name';
+  if (name === '.' || name === '..') return 'A game cannot be called that';
+  if (PROHIBITED.test(name)) return 'A game name cannot contain / \\ : " < > * ? or |';
+  // The watcher, the scan and the sweepers all skip dot folders on purpose, so a
+  // clip moved into one would vanish from the library while still on disk.
+  if (name.startsWith('.')) return 'A game name cannot start with a dot';
+  // Windows drops these on its own, so the folder would not have the name asked for.
+  const last = name[name.length - 1];
+  if (last === '.' || last === ' ') return 'A game name cannot end with a dot or a space';
+  if (RESERVED.test(name)) return 'Windows reserves that name';
+  return null;
+}
+
+/**
+ * The folder for a game, proved to sit directly inside the videos root.
+ *
+ * `gameFolderProblem` already rules out anything that could climb out, and
+ * this checks the result anyway: one guard is a single point of failure, and
+ * this is the line that decides where somebody's recording is moved to.
+ */
+export function gameFolderPath(videosRoot: string, name: string): string {
+  const problem = gameFolderProblem(name);
+  if (problem) throw new GameFolderError(problem);
+
+  const root = path.resolve(videosRoot);
+  const target = path.resolve(root, name);
+  if (!target.startsWith(root + path.sep) || path.dirname(target) !== root) {
+    throw new GameFolderError('A game folder has to sit inside the videos folder');
+  }
+  return target;
+}
+
+export class GameFolderError extends Error {}

src/main/utils/trimChar.ts

@@ -0,0 +1,16 @@
+/**
+ * Take a run of one character off either end of a string.
+ *
+ * A loop rather than `/^x+|x+$/`: an end-anchored `x+$` is retried from every
+ * position of a long run that turns out not to end the string, which is
+ * quadratic in the run's length, and both call sites trim text somebody typed.
+ */
+export function trimChar(value: string, char: string, ends: 'both' | 'end' = 'both'): string {
+  let start = 0;
+  let end = value.length;
+  if (ends === 'both') {
+    while (start < end && value[start] === char) start++;
+  }
+  while (end > start && value[end - 1] === char) end--;
+  return value.slice(start, end);
+}

src/shared/constants/tagPatternRules.ts

@@ -0,0 +1,85 @@
+/**
+ * What a tag rule is allowed to be.
+ *
+ * A tag rule is a regular expression the user writes, so compiling user input
+ * is the feature rather than a mistake, and escaping it would turn every rule
+ * into a literal. What can go wrong is an expression that backtracks without
+ * end: `(a+)+` against a long filename freezes the renderer on every clip it is
+ * matched to, for as long as it stays saved. So a rule is refused when it is
+ * long, and when it repeats something that already repeats, which is the shape
+ * behind nearly every catastrophic expression.
+ *
+ * Shared because the route, the DTO and the editor all ask the same question,
+ * and two answers to it is how a rule the editor accepted gets a 400.
+ */
+
+/** Far longer than any rule anybody has written, and short enough to reason about. */
+export const MAX_TAG_PATTERN_LENGTH = 200;
+
+/** Why an expression cannot be a tag rule, or null when it can. */
+export function tagPatternProblem(source: unknown): string | null {
+  if (typeof source !== 'string' || !source.trim()) return 'An expression cannot be empty';
+  if (source.length > MAX_TAG_PATTERN_LENGTH) {
+    return `An expression can be at most ${MAX_TAG_PATTERN_LENGTH} characters`;
+  }
+  try {
+    new RegExp(source, 'i');
+  } catch {
+    return `"${source}" is not a valid expression`;
+  }
+  if (repeatsARepetition(source)) {
+    return `"${source}" repeats something that already repeats, which can freeze the library`;
+  }
+  return null;
+}
+
+/**
+ * Does a quantified group contain a quantifier of its own?
+ *
+ * A scan rather than a regular expression, since a pattern that recognises
+ * nested quantifiers is itself the kind of pattern that backtracks. `?` is not
+ * counted: `(a+)?` matches the group once or not at all, which cannot explode.
+ */
+function repeatsARepetition(source: string): boolean {
+  const groups: boolean[] = [];
+  let closedGroupRepeats = false;
+
+  for (let i = 0; i < source.length; i++) {
+    const ch = source[i];
+
+    if (ch === '\\') {
+      i++;
+      closedGroupRepeats = false;
+      continue;
+    }
+    if (ch === '[') {
+      // A class is one character, whatever is inside it.
+      i++;
+      while (i < source.length && source[i] !== ']') {
+        if (source[i] === '\\') i++;
+        i++;
+      }
+      closedGroupRepeats = false;
+      continue;
+    }
+    if (ch === '(') {
+      groups.push(false);
+      closedGroupRepeats = false;
+      continue;
+    }
+    if (ch === ')') {
+      const repeats = groups.pop() ?? false;
+      if (repeats && groups.length) groups[groups.length - 1] = true;
+      closedGroupRepeats = repeats;
+      continue;
+    }
+
+    const quantifier = ch === '*' || ch === '+' || (ch === '{' && /\d/.test(source[i + 1] ?? ''));
+    if (quantifier) {
+      if (closedGroupRepeats) return true;
+      if (groups.length) groups[groups.length - 1] = true;
+    }
+    closedGroupRepeats = false;
+  }
+  return false;
+}

src/shared/dtos/tag/TagPatternDTO.ts

@@ -1,4 +1,5 @@
 import { BaseDTO } from '../BaseDTO.js';
+import { tagPatternProblem } from '../../constants/tagPatternRules.js';
 
 export type TagCategory = 'Gameplay' | 'Weapons' | 'Maps' | 'Modes' | 'Quality' | 'General';
 
@@ -53,13 +54,10 @@ export class TagPatternDTO extends BaseDTO<TagPatternDTO> {
     }
 
     // An expression that cannot compile would throw at match time, on every
-    // clip, for as long as it stays saved.
+    // clip, for as long as it stays saved; one that backtracks would hang there.
     for (const source of this.patterns ?? []) {
-      try {
-        new RegExp(source, 'i');
-      } catch {
-        errors.push(`"${source}" is not a valid expression`);
-      }
+      const problem = tagPatternProblem(source);
+      if (problem) errors.push(problem);
     }
 
     return { isValid: errors.length === 0, errors: errors.length ? errors : undefined };

src/shared/index.ts

@@ -32,6 +32,7 @@ export * from './constants/ui.js';
 
 // Which files are clips, so the scan and the watcher cannot disagree again
 export * from './constants/videoFiles.js';
+export * from './constants/tagPatternRules.js';
 
 // Where a window opens and closes around a moment, so the analysis and a
 // GoodBit made from one cannot disagree about the same reading