From d462c67b826106d2c9c1d7dea61e36800ddbf547 Mon Sep 17 00:00:00 2001 From: martin veillette Date: Wed, 12 Aug 2026 14:04:36 -0400 Subject: [PATCH] Avoid CodeQL file-system-race in scaffold scripts. Align with SceneryStackTemplate: Dirent-based walks and try/catch reads so CodeQL stops flagging TOCTOU on local rename/scaffold tooling. Co-authored-by: Cursor --- scripts/rename-sim.ts | 56 +++++++++++++++++++++++++------------ scripts/scaffold-screens.ts | 22 ++++++++------- 2 files changed, 50 insertions(+), 28 deletions(-) diff --git a/scripts/rename-sim.ts b/scripts/rename-sim.ts index 5022f4e..fab8aa2 100644 --- a/scripts/rename-sim.ts +++ b/scripts/rename-sim.ts @@ -29,7 +29,7 @@ */ import { execFileSync } from "node:child_process"; -import { readdirSync, readFileSync, renameSync, statSync, writeFileSync } from "node:fs"; +import { readdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"; import { join, resolve } from "node:path"; // ── Argument parsing ────────────────────────────────────────────────────────── @@ -77,9 +77,18 @@ const TEXT_EXTS = new Set([".ts", ".js", ".json", ".html", ".md", ".css", ".svg" // ── Content replacements ────────────────────────────────────────────────────── // Sim-level only. Screen classes (SimScreen, SimModel, …) and `sim-screen/` are -// left for scaffold-screens. Longer strings must come first to avoid partial matches. - -const REPLACEMENTS: ReadonlyArray<[string, string]> = [ +// left for scaffold-screens. +// +// INVARIANT — order + token length: +// 1. Never add a bare "Sim" (or "sim" / "SIM") → prefix replacement. Overlapping +// prefixes (SimColors, SimConstants, …) would be corrupted by a short token. +// 2. Class / identifier tokens use `\b`-bounded regex so a future short token +// cannot match inside a longer identifier even if order slips. +// 3. Display / package strings stay plain substring replaces (longest first). +// 4. Keep identifier entries longest-first as defense in depth. + +/** Identifier tokens: search is matched with word boundaries (`\b…\b`). */ +const IDENTIFIER_REPLACEMENTS: ReadonlyArray<[string, string]> = [ // Shared / preferences (not per-screen) ["SimPreferencesModel", `${newPrefix}PreferencesModel`], ["SimPreferencesNode", `${newPrefix}PreferencesNode`], @@ -93,7 +102,10 @@ const REPLACEMENTS: ReadonlyArray<[string, string]> = [ ["simQueryParameters", `${newCamel}QueryParameters`], // SCREAMING_SNAKE identifier ["SIM_COMBO_BOX_OPTIONS", `${newSnake}_COMBO_BOX_OPTIONS`], - // Display strings (all locales + PWA) +]; + +/** Display / package strings: plain substring replace (longest first). */ +const STRING_REPLACEMENTS: ReadonlyArray<[string, string]> = [ ["SceneryStack Template", newName], ["Plantilla de Simulación", newName], ["Modèle de Simulation", newName], @@ -106,13 +118,22 @@ const REPLACEMENTS: ReadonlyArray<[string, string]> = [ // ── Utilities ───────────────────────────────────────────────────────────────── +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + function replaceAll(str: string, search: string, replacement: string): string { return str.split(search).join(replacement); } function applyReplacements(text: string): string { let result = text; - for (const [search, replacement] of REPLACEMENTS) { + for (const [search, replacement] of IDENTIFIER_REPLACEMENTS) { + if (search !== replacement) { + result = result.replace(new RegExp(`\\b${escapeRegExp(search)}\\b`, "g"), replacement); + } + } + for (const [search, replacement] of STRING_REPLACEMENTS) { if (search !== replacement) { result = replaceAll(result, search, replacement); } @@ -181,15 +202,14 @@ function updatePackageJson(): void { // ── Pass 1: update file contents ────────────────────────────────────────────── function processContents(dir: string): void { - for (const entry of readdirSync(dir)) { - if (SKIP_DIRS.has(entry)) { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (SKIP_DIRS.has(entry.name)) { continue; } - const full = join(dir, entry); - const stat = statSync(full); - if (stat.isDirectory()) { + const full = join(dir, entry.name); + if (entry.isDirectory()) { processContents(full); - } else if (TEXT_EXTS.has(fileExtension(entry))) { + } else if (entry.isFile() && TEXT_EXTS.has(fileExtension(entry.name))) { const original = readFileSync(full, "utf8"); const transformed = applyReplacements(original); if (transformed !== original) { @@ -208,16 +228,16 @@ interface RenameOp { } function collectRenames(dir: string, renameOps: RenameOp[]): void { - for (const entry of readdirSync(dir)) { - if (SKIP_DIRS.has(entry)) { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (SKIP_DIRS.has(entry.name)) { continue; } - const full = join(dir, entry); - if (statSync(full).isDirectory()) { + const full = join(dir, entry.name); + if (entry.isDirectory()) { collectRenames(full, renameOps); } - const newEntry = applyReplacements(entry); - if (newEntry !== entry) { + const newEntry = applyReplacements(entry.name); + if (newEntry !== entry.name) { renameOps.push({ from: full, to: join(dir, newEntry) }); } } diff --git a/scripts/scaffold-screens.ts b/scripts/scaffold-screens.ts index ad1169e..8254bb0 100644 --- a/scripts/scaffold-screens.ts +++ b/scripts/scaffold-screens.ts @@ -22,7 +22,7 @@ * --shared-model Emit src/common/model/SharedModel.ts; each screen model composes it */ -import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { basename, dirname, join, relative, resolve } from "node:path"; // ── Argument parsing ────────────────────────────────────────────────────────── @@ -234,11 +234,11 @@ function findPrototypeDir(): string { return preferred; } // Legacy post-rename layout: {id}-screen/ - for (const entry of readdirSync(SRC)) { - const full = join(SRC, entry); - if (!(statSync(full).isDirectory() && entry.endsWith("-screen"))) { + for (const entry of readdirSync(SRC, { withFileTypes: true })) { + if (!(entry.isDirectory() && entry.name.endsWith("-screen"))) { continue; } + const full = join(SRC, entry.name); const screens = readdirSync(full).filter((f) => f.endsWith("Screen.ts") && !f.includes("View")); if (screens.length === 1) { return full; @@ -250,11 +250,11 @@ function findPrototypeDir(): string { function walkFiles(dir: string): string[] { const out: string[] = []; - for (const entry of readdirSync(dir)) { - const full = join(dir, entry); - if (statSync(full).isDirectory()) { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + if (entry.isDirectory()) { out.push(...walkFiles(full)); - } else { + } else if (entry.isFile()) { out.push(full); } } @@ -556,10 +556,12 @@ function updateDocs(screens: ScreenSpec[], simPrefix: string): void { ]; for (const path of paths) { - if (!existsSync(path)) { + let original: string; + try { + original = readFileSync(path, "utf8"); + } catch { continue; } - const original = readFileSync(path, "utf8"); let text = original; if (basename(path) === "multi-screen.md") { for (const [from, to] of prose) {