Delete the host scaffolding that guards nothing

src/config/configManager.ts

@@ -47,16 +47,6 @@ let configWriteQueue: Promise<void> = Promise.resolve()
 let pendingConfig: ConfigType | null = null
 let scheduledConfigWrite: Promise<void> | null = null
 
-async function writeConfig(normalizedConfig: ConfigType): Promise<void> {
-  const cleanedConfig = JSON.parse(
-    JSON.stringify(normalizedConfig, (key, value) => {
-      return key.startsWith("_") ? undefined : value
-    })
-  )
-
-  await writeJsonAtomic(configPath, cleanedConfig)
-}
-
 function scheduleConfigWrite(): Promise<void> {
   // Compared against null rather than tested for truthiness: the question is whether a write is already scheduled, not whether a promise is truthy (it always is).
   if (scheduledConfigWrite !== null) return scheduledConfigWrite
@@ -67,7 +57,10 @@ function scheduleConfigWrite(): Promise<void> {
     while (pendingConfig) {
       const nextConfig = pendingConfig
       pendingConfig = null
-      await writeConfig(nextConfig)
+      // Written as it stands: the only thing that ever reaches here is a normalizeConfig result,
+      // and that builds a fixed literal field by field, so the renderer's session-only markers
+      // (`_notifiedModUpdatesInstallations`, `_backgroundRevision`) are already gone.
+      await writeJsonAtomic(configPath, nextConfig)
     }
   })
 

src/ipc/handlers/pathsHandlers.ts

@@ -15,6 +15,7 @@ import type { WorkerDisposition } from "@src/ipc/workerManager"
 import { ConcurrencyLimiter } from "@domain/concurrencyLimiter"
 import { assertAllowedDownloadUrl, assertBoolean, assertInteger, assertPath, assertSafeFileName, assertSafeTaskId, comparablePath, isRecord, MAX_CUSTOM_ICON_BYTES } from "@src/ipc/validation"
 import { assertManagedDeletionPath, assertManagedPath } from "@src/ipc/pathPolicy"
+import { changePermissions } from "@src/ipc/permissions"
 import { getConfig } from "@src/config/configManager"
 import { assertVerifiedArtifact, getTrustedDownloadHash, recordVerifiedArtifact } from "@src/ipc/artifactVerification"
 import { getCachedOptimumManifest, getTrustedOverlayHash } from "@src/ipc/optimumManifest"
@@ -25,14 +26,12 @@ import { DEFAULT_COMPRESSION_LEVEL } from "@domain/config/defaults"
 import compressWorker from "@src/ipc/workers/compressWorker?modulePath"
 import extractWorker from "@src/ipc/workers/extractWorker?modulePath"
 import innoExtractWorker from "@src/ipc/workers/innoExtractWorker?modulePath"
-import changePermsWorker from "@src/ipc/workers/changePermsWorker?modulePath"
 import downloadWorkerPath from "@src/ipc/workers/downloadWorker?modulePath"
 
 const WORKER_TIMEOUTS_MS: Record<string, number> = {
   DOWNLOAD_ON_PATH: 45 * 60 * 1_000,
   EXTRACT_ON_PATH: 30 * 60 * 1_000,
   COMPRESS_ON_PATH: 30 * 60 * 1_000,
-  CHANGE_PERMS: 10 * 60 * 1_000,
   // Reading the payload out of the Windows installer. Measured at 41 seconds
   // for the 598 MB installer of 1.22.6 on a developer machine, so this leaves
   // room for a slow disk without leaving a stuck worker running for an hour.
@@ -69,6 +68,15 @@ const EXTRACT_INSTALLER_PAYLOAD = true
  */
 const RUN_INSTALLER_TIMEOUT_MS = 15 * 60 * 1_000
 
+/**
+ * Same standing as RUN_INSTALLER_TIMEOUT_MS: CHANGE_PERMS walks the tree on this thread
+ * rather than in a worker, so it is not the pool's timeout that bounds it. The value is the
+ * one the table used to carry. The entry cap in changePermissions bounds how many entries
+ * the walk may touch; this bounds how long it may take to touch them, which is the case a
+ * filesystem that stops answering produces.
+ */
+const CHANGE_PERMS_TIMEOUT_MS = 10 * 60 * 1_000
+
 /**
  * Nothing capped how many DOWNLOAD_ON_PATH/EXTRACT_ON_PATH/COMPRESS_ON_PATH calls the
  * renderer could fire at once (a mod update-all, say): each one spun up its own worker
@@ -114,19 +122,18 @@ app.on("before-quit", () => {
  * compression share a two-slot archive lane but use different worker scripts, so one idle
  * worker per archive operation keeps the combined idle count within that shared limit.
  *
- * CHANGE_PERMS and RUN_INSTALLER are 0 on purpose. Both run once per install with no burst
- * behind them, so pooling either would buy one saved worker spawn per game install and pay
- * for it with a resident idle isolate. They still go through the pooled protocol below so
- * there is only one worker protocol in the app; 0 just means every release terminates,
- * exactly like before this file started pooling anything. RUN_INSTALLER joining the archive
- * lane leaves those sums alone for the same reason: at 0 it never contributes an idle worker
- * to weigh against the two-slot limit.
+ * RUN_INSTALLER is 0 on purpose. It runs once per install with no burst behind it, so
+ * pooling it would buy one saved worker spawn per game install and pay for it with a
+ * resident idle isolate. It still goes through the pooled protocol below so there is only
+ * one worker protocol in the app; 0 just means every release terminates, exactly like
+ * before this file started pooling anything. RUN_INSTALLER joining the archive lane leaves
+ * those sums alone for the same reason: at 0 it never contributes an idle worker to weigh
+ * against the two-slot limit.
  */
 const WORKER_POOL_MAX_IDLE: Record<string, number> = {
   DOWNLOAD_ON_PATH: DOWNLOAD_CONCURRENCY_LIMIT,
   EXTRACT_ON_PATH: 1,
   COMPRESS_ON_PATH: 1,
-  CHANGE_PERMS: 0,
   RUN_INSTALLER: 0
 }
 
@@ -657,8 +664,18 @@ ipcMain.handle(IPC_CHANNELS.PATHS_MANAGER.CHANGE_PERMS, async (event, paths: str
 
   const safePaths = await Promise.all(paths.map((pathValue) => assertManagedPath(pathValue, "permissions path")))
   const safePerms = assertInteger(perms, "permissions", 0, 0o777)
+  const timeout = AbortSignal.timeout(CHANGE_PERMS_TIMEOUT_MS)
+
+  try {
+    await changePermissions({ paths: safePaths, perms: safePerms, signal: timeout })
+  } catch (err) {
+    // Same two texts the pooled worker reported, so the renderer's extract task reads a
+    // refusal exactly as it did. The reason behind them used to be dropped on the way out
+    // of the worker; at debug it is there for whoever reads the log afterwards.
+    logMessage("debug", `[back] [ipc] [ipc/handlers/pathsHandlers.ts] [CHANGE_PERMS] ${getErrorMessage(err)}`)
+    throw new Error(timeout.aborted ? "CHANGE_PERMS timed out" : "Changing permissions failed")
+  }
 
-  await runTrackedWorker(event, "permissions", undefined, changePermsWorker, { paths: safePaths, perms: safePerms }, "CHANGE_PERMS", () => true)
   return true
 })
 

src/ipc/permissions.ts

@@ -0,0 +1,112 @@
+/**
+ * Applying a permission bit to a folder tree.
+ *
+ * This exists for the Linux game folder, whose executables come out of the
+ * archive without the execute bit. A symbolic link inside the tree stops the
+ * whole run rather than being followed: chmod resolves links, so following one
+ * would apply the launcher's bits to a file outside the folder the user picked.
+ * A link that resolves to nothing has no such file behind it and is skipped.
+ *
+ * The walk is I/O and nothing else, so it runs on the main thread's event loop
+ * rather than in a worker: every step is an awaited syscall, and none of them
+ * holds the loop. Nothing here touches Electron.
+ */
+
+import { access, chmod, lstat, readdir } from "node:fs/promises"
+import { join } from "node:path"
+
+const MAX_ITEMS = 100_000
+
+/** The slice of the filesystem this walk needs, so a test can stand in a fake tree. `node:fs/promises` satisfies it as it is. */
+export interface PermissionsFileSystem {
+  lstat(path: string): Promise<{ isSymbolicLink(): boolean; isDirectory(): boolean; isFile(): boolean }>
+  readdir(path: string): Promise<string[]>
+  chmod(path: string, mode: number): Promise<void>
+  /** Follows links, unlike `lstat`. Refusing is what tells a link with nothing on the other end apart from a live one. */
+  access(path: string): Promise<void>
+}
+
+export interface ChangePermissionsOptions {
+  /** Roots to walk. Each is applied to itself and to everything beneath it. */
+  paths: readonly string[]
+  /** Mode passed straight to `chmod`. */
+  perms: number
+  /**
+   * The caller's bound on a tree that never ends. Checked before each entry, so a walk still
+   * making progress stops without touching the rest of the tree, and raced against the walk
+   * as a whole, so one syscall that never answers still settles the call.
+   */
+  signal?: AbortSignal
+  /** Filesystem to act on, defaulting to the real one. */
+  fileSystem?: PermissionsFileSystem
+}
+
+/**
+ * Applies `perms` to every path given and to everything under it.
+ *
+ * A path that cannot be stat'd is skipped, which is what lets a caller pass the
+ * union of the paths a Linux install might use without checking each one first.
+ *
+ * @param options Roots, mode, abort signal, and the filesystem to act on.
+ * @throws On a symbolic link whose target can be reached, on an entry that is
+ * neither a file nor a folder, once the tree passes the entry cap, and on an
+ * aborted signal. Nothing is rolled back: the caller reports the failure and
+ * the bits already applied stay applied.
+ */
+export async function changePermissions(options: ChangePermissionsOptions): Promise<void> {
+  const { paths, perms, signal, fileSystem = { lstat, readdir, chmod, access } } = options
+  let itemCount = 0
+
+  const visit = async (path: string): Promise<void> => {
+    signal?.throwIfAborted()
+
+    // Anything the filesystem will not describe is skipped rather than refused, missing paths
+    // included: the caller passes the union of the paths a Linux install might use and lets
+    // the walk sort out which of them are there.
+    const stats = await fileSystem.lstat(path).catch(() => null)
+    if (!stats) return
+
+    if (stats.isSymbolicLink()) {
+      // A link with nothing reachable on the other end is skipped rather than refused: a
+      // dangling target, a loop, a target behind a folder with no execute bit. The rule this
+      // guard exists for is about what chmod would resolve the link to, and there is nothing
+      // to resolve it to here, so a Linux install into a folder holding one stale link
+      // completes the way it always has. Neither answer applies a bit, so the gap between
+      // this call and the lstat above leaves nothing for the tree to change under.
+      const targetReachable = await fileSystem.access(path).then(
+        () => true,
+        () => false
+      )
+      if (targetReachable) throw new Error("Symbolic links are not allowed")
+      return
+    }
+
+    itemCount++
+    if (itemCount > MAX_ITEMS) throw new Error("Too many filesystem entries")
+
+    if (stats.isDirectory()) {
+      for (const item of await fileSystem.readdir(path)) await visit(join(path, item))
+    }
+
+    if (!stats.isDirectory() && !stats.isFile()) throw new Error("Unsupported filesystem entry")
+    await fileSystem.chmod(path, perms)
+  }
+
+  const walk = (async (): Promise<void> => {
+    for (const path of paths) await visit(path)
+  })()
+
+  if (!signal) return walk
+
+  // The check at the top of visit stops a walk that is still making syscalls. It never runs
+  // again once a single syscall stops answering, which is what a hard NFS or FUSE mount that
+  // goes away leaves behind, so the signal has to settle this call on its own as well.
+  // The walk is then abandoned rather than stopped, the same standing the worker thread this
+  // replaced had: its timeout fired on the main thread too, and the thread it gave up on was
+  // discarded rather than waited for.
+  const aborted = new Promise<never>((_, reject) => {
+    signal.addEventListener("abort", () => reject(signal.reason), { once: true })
+  })
+
+  return Promise.race([walk, aborted])
+}

src/ipc/validation.ts

@@ -300,13 +300,10 @@ export function validateGameInstallation(value: unknown): Pick<InstallationType,
     mesaGlThread: assertBoolean(value.mesaGlThread, "MESA GL thread flag"),
     envVars: assertBoundedString(value.envVars, "environment variables", 8_192),
     launchWrapper: assertBoundedString(value.launchWrapper ?? "", "launch wrapper", 4_096).trim(),
-    ...(value.gameVersionId === null
-      ? { gameVersionId: null }
-      : typeof value.gameVersionId === "string"
-        ? { gameVersionId: assertString(value.gameVersionId, "installation game version id", 128) }
-        : value.gameVersionId === undefined
-          ? {}
-          : { gameVersionId: assertString(value.gameVersionId, "installation game version id", 128) })
+    // Three outcomes, and only three: missing stays missing, so EXECUTE_GAME's correlation check
+    // can tell "no id sent" from "no version linked"; an explicit null is carried through; and
+    // anything else goes to assertString, which is what rejects a non-string rather than dropping it.
+    ...(value.gameVersionId === undefined ? {} : { gameVersionId: value.gameVersionId === null ? null : assertString(value.gameVersionId, "installation game version id", 128) })
   }
 }
 

src/ipc/workers/workerHost.ts

@@ -3,11 +3,11 @@
  * `workerData` and lets the thread fall idle so the parent can terminate it: it waits on
  * `parentPort` for one task at a time and stays alive for the next one.
  *
- * All five worker shims are this function plus a handler, so the protocol lives in exactly
+ * All four worker shims are this function plus a handler, so the protocol lives in exactly
  * one place on this side of the port: the token echo that lets the parent tell a live
  * task's messages from an abandoned one's, the guard that drops a progress report arriving
- * after its own task already settled, and the promise-or-value handling that lets the
- * synchronous permissions handler share a shape with the four asynchronous ones.
+ * after its own task already settled, and the promise-or-value handling that accepts a
+ * handler that answers without awaiting anything.
  *
  * Attaching the "message" listener is also what keeps the thread alive between tasks: a
  * started MessagePort refs the worker's event loop.
@@ -25,7 +25,7 @@ export type TaskHandler = (payload: Record<string, unknown>, onProgress: Progres
 
 /**
  * Turns a rejection into the message text the parent sees. Per worker on purpose:
- * extraction forwards its own reason, the other four report a fixed one, matching what
+ * extraction forwards its own reason, the other three report a fixed one, matching what
  * each already did before this file existed.
  */
 export type FailureDescriber = (error: unknown) => string

src/utils/logManager.ts

@@ -14,26 +14,11 @@ export function getErrorMessage(error: unknown): string {
   return redactSensitiveText(error.message)
 }
 
+/**
+ * The five modes are electron-log's own method names, so the mode indexes the logger directly.
+ * The optional call is the old `default:` branch: a mode that slipped past the type reads as an
+ * absent property and the line is dropped, rather than throwing inside a log call.
+ */
 export function logMessage(mode: ErrorTypes, message: string): void {
-  const safeMessage = redactSensitiveText(message)
-
-  switch (mode) {
-    case "error":
-      Logger.error(safeMessage)
-      break
-    case "warn":
-      Logger.warn(safeMessage)
-      break
-    case "info":
-      Logger.info(safeMessage)
-      break
-    case "debug":
-      Logger.debug(safeMessage)
-      break
-    case "verbose":
-      Logger.verbose(safeMessage)
-      break
-    default:
-      break
-  }
+  Logger[mode]?.(redactSensitiveText(message))
 }