Refactor NodeCG runtime preparation and update handling

- Updated paths and configurations in doctor.mjs and prepare-nodecg-runtime.mjs to use new build-config.mjs imports.
- Enhanced runtime installation checks and permissions validation.
- Introduced new update configuration management in update-config.ts, including loading and validating update settings.
- Implemented update service for managing update checks and downloads in update-service.ts.
- Replaced update-utils.ts with update-schema.ts for better structure and clarity in update handling.
- Added comprehensive tests for update download and settings management.
- Ensured secure handling of download URLs and improved error handling in update processes.

src/main/app/bootstrap.ts

@@ -5,7 +5,7 @@ import { getRuntimeConfig } from "../config/runtime-config";
 import { showFatalError, log } from "../errors/error-presenter";
 import { createNodecgProcessManager } from "../nodecg/process-manager";
 import { prepareUserNodecgRuntime } from "../nodecg/runtime-provisioner";
-import { scheduleUpdateCheck } from "../updates/update-manager";
+import { scheduleUpdateCheck } from "../updates/update-service";
 import { createLoadingWindow, createMainWindow } from "../windows/window-service";
 import { createApplicationController } from "./application-controller";
 import { getApplicationPaths } from "./paths";

src/main/app/paths.ts

@@ -19,10 +19,36 @@ export function getUserDataPath(appDataPath: string, userDataDirectoryName: stri
   return path.join(appDataPath, userDataDirectoryName);
 }
 
+export function getManagedNodecgRuntimePath(userDataPath: string): string {
+  return path.join(userDataPath, "nodecg");
+}
+
 export function getSourceNodecgRuntimePath(rootPath: string): string {
   return path.resolve(rootPath, "lib", "nodecg");
 }
 
+export function getDefaultUpdateConfigPath(rootPath: string): string {
+  return path.join(rootPath, "static", "updates.json");
+}
+
+export function getUpdateDownloadDirectory(tempDirectory: string): string {
+  return path.join(tempDirectory, "scoreko-updates");
+}
+
+export function getSafeChildPath(parentDirectory: string, fileName: string): string {
+  const resolvedParent = path.resolve(parentDirectory);
+  const resolvedChild = path.resolve(resolvedParent, fileName);
+  const relativePath = path.relative(resolvedParent, resolvedChild);
+  const isInsideParent =
+    relativePath.length > 0 && !relativePath.startsWith("..") && !path.isAbsolute(relativePath);
+
+  if (!isInsideParent) {
+    throw new Error(`Refusing to build a path outside ${resolvedParent}: ${fileName}`);
+  }
+
+  return resolvedChild;
+}
+
 export function getNodecgBaseUrl(nodecgPort: string): string {
   return `http://127.0.0.1:${nodecgPort}`;
 }

src/main/nodecg/runtime-provisioner.ts

@@ -1,6 +1,8 @@
 import fs from "node:fs";
 import path from "node:path";
 
+import { getManagedNodecgRuntimePath } from "../app/paths";
+
 type RuntimeProvisionerConfig = {
   sourceRuntimePath: string;
   userDataPath: string;
@@ -55,7 +57,7 @@ export function prepareUserNodecgRuntime({
   deps,
 }: RuntimeProvisionerConfig): PreparedNodecgRuntime {
   const resolvedDeps = resolveDeps(deps);
-  const targetRuntimePath = path.join(userDataPath, "nodecg");
+  const targetRuntimePath = getManagedNodecgRuntimePath(userDataPath);
 
   validateSourceRuntime(sourceRuntimePath, bundleName, resolvedDeps.existsSync);
   resolvedDeps.mkdirSync(targetRuntimePath, { recursive: true });

src/main/updates/update-config.ts

@@ -1,8 +1,8 @@
 import fs from "node:fs";
-import path from "node:path";
 
+import { getDefaultUpdateConfigPath } from "../app/paths";
 import { AppRuntimeConfig } from "../config/runtime-config";
-import { UpdateFileConfig } from "./update-utils";
+import { UpdateFileConfig, validateHttpUrl } from "./update-schema";
 
 const DEFAULT_UPDATE_ASSET_PATTERN = "Scoreko-setup-.*\\.exe$";
 
@@ -13,17 +13,24 @@ export type UpdateSettings = {
   assetPattern: string;
 };
 
+type UpdateConfigOptions = {
+  allowInsecureHttp: boolean;
+};
+
 export function loadUpdateSettings(
   appConfig: AppRuntimeConfig,
   rootPath: string,
   log: (...args: unknown[]) => void,
+  options: UpdateConfigOptions = { allowInsecureHttp: true },
 ): UpdateSettings {
   const fileConfig = readUpdateFileConfig(appConfig, rootPath, log);
+  const apiUrl = readOptionalHttpUrl(appConfig.updateApiUrl ?? fileConfig.apiUrl, options);
+  const releasePageUrl = readOptionalHttpUrl(appConfig.updateReleasePageUrl ?? fileConfig.releasePageUrl, options);
 
   return {
-    enabled: appConfig.updatesEnabled && (Boolean(fileConfig.enabled) || Boolean(appConfig.updateApiUrl)),
-    apiUrl: appConfig.updateApiUrl ?? readOptionalString(fileConfig.apiUrl),
-    releasePageUrl: appConfig.updateReleasePageUrl ?? readOptionalString(fileConfig.releasePageUrl),
+    enabled: appConfig.updatesEnabled && (Boolean(fileConfig.enabled) || Boolean(appConfig.updateApiUrl)) && Boolean(apiUrl),
+    ...(apiUrl ? { apiUrl } : {}),
+    ...(releasePageUrl ? { releasePageUrl } : {}),
     assetPattern:
       appConfig.updateAssetPattern || readOptionalString(fileConfig.assetPattern) || DEFAULT_UPDATE_ASSET_PATTERN,
   };
@@ -34,7 +41,7 @@ export function readUpdateFileConfig(
   rootPath: string,
   log: (...args: unknown[]) => void,
 ): UpdateFileConfig {
-  const configPath = appConfig.updateConfigPathOverride ?? path.join(rootPath, "static", "updates.json");
+  const configPath = appConfig.updateConfigPathOverride ?? getDefaultUpdateConfigPath(rootPath);
 
   if (!fs.existsSync(configPath)) {
     return {};
@@ -62,6 +69,15 @@ function normalizeUpdateFileConfig(value: unknown): UpdateFileConfig {
   };
 }
 
+function readOptionalHttpUrl(value: unknown, options: UpdateConfigOptions): string | undefined {
+  const rawValue = readOptionalString(value);
+  if (!rawValue) {
+    return undefined;
+  }
+
+  return validateHttpUrl(rawValue, options) ?? undefined;
+}
+
 function readOptionalString(value: unknown): string | undefined {
   return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined;
 }

src/main/updates/update-dialogs.ts

@@ -1,7 +1,7 @@
 import { BrowserWindow, dialog } from "electron";
 import type { MessageBoxOptions } from "electron";
 
-import { ReleaseUpdate } from "./update-utils";
+import { ReleaseUpdate } from "./update-schema";
 
 export type DownloadUpdateChoice = "download" | "open-release" | "dismiss";
 

src/main/updates/update-download.ts

@@ -1,34 +1,103 @@
 import fs from "node:fs";
-import path from "node:path";
-import { Readable } from "node:stream";
+import { Writable } from "node:stream";
 
-import { ReleaseUpdate, sanitizeFileName } from "./update-utils";
+import { getSafeChildPath, getUpdateDownloadDirectory } from "../app/paths";
+import { ReleaseUpdate, sanitizeFileName, validateHttpUrl } from "./update-schema";
 
 type UpdateDownloadConfig = {
   tempDirectory: string;
+  allowInsecureHttp: boolean;
 };
 
 export async function downloadInstaller(update: ReleaseUpdate, config: UpdateDownloadConfig): Promise<string> {
+  const downloadUrl = validateHttpUrl(update.installer.downloadUrl, {
+    allowInsecureHttp: config.allowInsecureHttp,
+  });
+
+  if (!downloadUrl) {
+    throw new Error("Update installer URL is invalid or uses an unsupported protocol.");
+  }
+
   const safeFileName = sanitizeFileName(update.installer.name);
-  const downloadDirectory = path.join(config.tempDirectory, "scoreko-updates");
-  const targetPath = path.join(downloadDirectory, safeFileName);
+  const downloadDirectory = getUpdateDownloadDirectory(config.tempDirectory);
+  const targetPath = getSafeChildPath(downloadDirectory, safeFileName);
+  const stagingPath = getSafeChildPath(downloadDirectory, `${safeFileName}.${process.pid}.${Date.now()}.download`);
 
   fs.mkdirSync(downloadDirectory, { recursive: true });
+  fs.rmSync(stagingPath, { force: true });
 
-  const response = await fetch(update.installer.downloadUrl);
+  const response = await fetch(downloadUrl);
   if (!response.ok || !response.body) {
     throw new Error(`Could not download update installer. HTTP ${response.status}.`);
   }
 
-  await new Promise<void>((resolve, reject) => {
-    const fileStream = fs.createWriteStream(targetPath);
-    const responseStream = Readable.fromWeb(response.body as Parameters<typeof Readable.fromWeb>[0]);
-
-    responseStream.on("error", reject);
-    fileStream.on("error", reject);
-    fileStream.on("finish", resolve);
-    responseStream.pipe(fileStream);
-  });
+  try {
+    await writeResponseBodyToFile(response.body, stagingPath);
+    fs.renameSync(stagingPath, targetPath);
+  } catch (error) {
+    fs.rmSync(stagingPath, { force: true });
+    throw error;
+  }
 
   return targetPath;
 }
+
+async function writeResponseBodyToFile(body: ReadableStream<Uint8Array>, filePath: string): Promise<void> {
+  const reader = body.getReader();
+  const fileStream = fs.createWriteStream(filePath, { flags: "wx" });
+
+  try {
+    while (true) {
+      const chunk = await reader.read();
+      if (chunk.done) {
+        break;
+      }
+
+      await writeChunk(fileStream, chunk.value);
+    }
+
+    await endStream(fileStream);
+  } catch (error) {
+    fileStream.destroy();
+    throw error;
+  } finally {
+    reader.releaseLock();
+  }
+}
+
+function writeChunk(stream: Writable, chunk: Uint8Array): Promise<void> {
+  if (stream.write(chunk)) {
+    return Promise.resolve();
+  }
+
+  return new Promise((resolve, reject) => {
+    const cleanup = (): void => {
+      stream.off("drain", onDrain);
+      stream.off("error", onError);
+    };
+    const onDrain = (): void => {
+      cleanup();
+      resolve();
+    };
+    const onError = (error: Error): void => {
+      cleanup();
+      reject(error);
+    };
+
+    stream.once("drain", onDrain);
+    stream.once("error", onError);
+  });
+}
+
+function endStream(stream: Writable): Promise<void> {
+  return new Promise((resolve, reject) => {
+    stream.end((error?: Error | null) => {
+      if (error) {
+        reject(error);
+        return;
+      }
+
+      resolve();
+    });
+  });
+}

src/main/updates/update-schema.ts

@@ -0,0 +1,203 @@
+export type GiteaReleaseAsset = {
+  name: string;
+  browserDownloadUrl: string;
+  size?: number;
+};
+
+export type GiteaRelease = {
+  tagName: string;
+  title?: string;
+  pageUrl?: string;
+  assets: GiteaReleaseAsset[];
+};
+
+export type InstallerAsset = {
+  name: string;
+  downloadUrl: string;
+  size?: number;
+};
+
+export type ReleaseUpdate = {
+  version: string;
+  title: string;
+  pageUrl?: string;
+  installer: InstallerAsset;
+};
+
+export type UpdateFileConfig = {
+  enabled?: unknown;
+  apiUrl?: unknown;
+  releasePageUrl?: unknown;
+  assetPattern?: unknown;
+};
+
+type UrlPolicy = {
+  allowInsecureHttp: boolean;
+};
+
+export function parseGiteaRelease(value: unknown): GiteaRelease | null {
+  if (!isRecord(value)) {
+    return null;
+  }
+
+  const tagName = readRequiredString(value.tag_name);
+  const assets = Array.isArray(value.assets) ? value.assets.map(parseGiteaReleaseAsset).filter(isPresent) : null;
+
+  if (!tagName || !assets) {
+    return null;
+  }
+
+  return {
+    tagName,
+    assets,
+    ...(readOptionalString(value.name) ? { title: readOptionalString(value.name) } : {}),
+    ...(readOptionalUrlString(value.html_url) ? { pageUrl: readOptionalUrlString(value.html_url) } : {}),
+  };
+}
+
+export function isVersionNewer(candidateVersion: string, currentVersion: string): boolean {
+  const candidate = normalizeVersion(candidateVersion);
+  const current = normalizeVersion(currentVersion);
+
+  for (let index = 0; index < Math.max(candidate.length, current.length); index += 1) {
+    const candidatePart = candidate[index] ?? 0;
+    const currentPart = current[index] ?? 0;
+
+    if (candidatePart > currentPart) {
+      return true;
+    }
+
+    if (candidatePart < currentPart) {
+      return false;
+    }
+  }
+
+  return false;
+}
+
+export function selectInstallerAsset(
+  release: GiteaRelease,
+  assetPattern: string,
+  policy: UrlPolicy = { allowInsecureHttp: true },
+): InstallerAsset | null {
+  const matcher = new RegExp(assetPattern, "i");
+
+  for (const asset of release.assets) {
+    if (!matcher.test(asset.name)) {
+      continue;
+    }
+
+    const downloadUrl = validateHttpUrl(asset.browserDownloadUrl, policy);
+    if (!downloadUrl) {
+      continue;
+    }
+
+    return {
+      name: asset.name,
+      downloadUrl,
+      ...(typeof asset.size === "number" ? { size: asset.size } : {}),
+    };
+  }
+
+  return null;
+}
+
+export function buildReleaseUpdate(
+  release: GiteaRelease,
+  currentVersion: string,
+  assetPattern: string,
+  policy: UrlPolicy = { allowInsecureHttp: true },
+): ReleaseUpdate | null {
+  const version = release.tagName.replace(/^v/i, "");
+  if (!version || !isVersionNewer(version, currentVersion)) {
+    return null;
+  }
+
+  const installer = selectInstallerAsset(release, assetPattern, policy);
+  if (!installer) {
+    return null;
+  }
+
+  const pageUrl = release.pageUrl ? validateHttpUrl(release.pageUrl, policy) ?? undefined : undefined;
+
+  return {
+    version,
+    title: release.title ?? `Scoreko ${version}`,
+    ...(pageUrl ? { pageUrl } : {}),
+    installer,
+  };
+}
+
+export function sanitizeFileName(fileName: string): string {
+  const sanitized = fileName.replace(/[<>:"/\\|?*\x00-\x1f]/g, "_").trim();
+  return sanitized.length > 0 ? sanitized : "scoreko-update-installer";
+}
+
+export function validateHttpUrl(value: string, policy: UrlPolicy): string | null {
+  try {
+    const url = new URL(value);
+
+    if (url.protocol === "https:" || (policy.allowInsecureHttp && url.protocol === "http:")) {
+      return url.toString();
+    }
+
+    return null;
+  } catch {
+    return null;
+  }
+}
+
+function parseGiteaReleaseAsset(value: unknown): GiteaReleaseAsset | null {
+  if (!isRecord(value)) {
+    return null;
+  }
+
+  const name = readRequiredString(value.name);
+  const browserDownloadUrl = readRequiredString(value.browser_download_url);
+
+  if (!name || !browserDownloadUrl) {
+    return null;
+  }
+
+  return {
+    name,
+    browserDownloadUrl,
+    ...(typeof value.size === "number" && value.size >= 0 ? { size: value.size } : {}),
+  };
+}
+
+function normalizeVersion(version: string): number[] {
+  return version
+    .trim()
+    .replace(/^v/i, "")
+    .split(/[+-]/)[0]
+    .split(".")
+    .map((part) => Number.parseInt(part, 10))
+    .map((part) => (Number.isFinite(part) ? part : 0));
+}
+
+function readRequiredString(value: unknown): string | null {
+  const text = readOptionalString(value);
+  return text && text.length > 0 ? text : null;
+}
+
+function readOptionalString(value: unknown): string | undefined {
+  return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined;
+}
+
+function readOptionalUrlString(value: unknown): string | undefined {
+  const rawValue = readOptionalString(value);
+  if (!rawValue) {
+    return undefined;
+  }
+
+  return validateHttpUrl(rawValue, { allowInsecureHttp: true }) ?? undefined;
+}
+
+function isRecord(value: unknown): value is Record<string, unknown> {
+  return typeof value === "object" && value !== null && !Array.isArray(value);
+}
+
+function isPresent<T>(value: T | null): value is T {
+  return value !== null;
+}

src/main/updates/update-service.ts

@@ -2,11 +2,11 @@ import { app, BrowserWindow, shell } from "electron";
 
 import { AppRuntimeConfig } from "../config/runtime-config";
 import { askToDownloadUpdate, askToInstallUpdate } from "./update-dialogs";
+import { loadUpdateSettings, UpdateSettings } from "./update-config";
 import { downloadInstaller } from "./update-download";
-import { loadUpdateSettings, UpdateSettings } from "./update-settings";
-import { buildReleaseUpdate, GiteaRelease } from "./update-utils";
+import { buildReleaseUpdate, GiteaRelease, parseGiteaRelease } from "./update-schema";
 
-type UpdateManagerConfig = {
+type UpdateServiceConfig = {
   appConfig: AppRuntimeConfig;
   rootPath: string;
   getParentWindow: () => BrowserWindow | null;
@@ -14,14 +14,19 @@ type UpdateManagerConfig = {
   log: (...args: unknown[]) => void;
 };
 
+type UpdateProtocolPolicy = {
+  allowInsecureHttp: boolean;
+};
+
 export function scheduleUpdateCheck({
   appConfig,
   rootPath,
   getParentWindow,
   beforeInstall,
   log,
-}: UpdateManagerConfig): void {
-  const settings = loadUpdateSettings(appConfig, rootPath, log);
+}: UpdateServiceConfig): void {
+  const protocolPolicy = getUpdateProtocolPolicy();
+  const settings = loadUpdateSettings(appConfig, rootPath, log, protocolPolicy);
 
   if (!settings.enabled || !settings.apiUrl) {
     log("Update checks disabled or not configured.");
@@ -29,7 +34,7 @@ export function scheduleUpdateCheck({
   }
 
   setTimeout(() => {
-    void checkForUpdates({ settings, getParentWindow, beforeInstall, log });
+    void checkForUpdates({ settings, getParentWindow, beforeInstall, log, protocolPolicy });
   }, appConfig.updateCheckDelayMs);
 }
 
@@ -38,11 +43,13 @@ async function checkForUpdates({
   getParentWindow,
   beforeInstall,
   log,
+  protocolPolicy,
 }: {
   settings: UpdateSettings;
   getParentWindow: () => BrowserWindow | null;
   beforeInstall: () => Promise<void>;
   log: (...args: unknown[]) => void;
+  protocolPolicy: UpdateProtocolPolicy;
 }): Promise<void> {
   try {
     if (!settings.apiUrl) {
@@ -50,7 +57,7 @@ async function checkForUpdates({
     }
 
     const release = await fetchLatestRelease(settings.apiUrl);
-    const update = buildReleaseUpdate(release, app.getVersion(), settings.assetPattern);
+    const update = buildReleaseUpdate(release, app.getVersion(), settings.assetPattern, protocolPolicy);
 
     if (!update) {
       log("No Scoreko update available.");
@@ -69,7 +76,10 @@ async function checkForUpdates({
       return;
     }
 
-    const installerPath = await downloadInstaller(update, { tempDirectory: app.getPath("temp") });
+    const installerPath = await downloadInstaller(update, {
+      tempDirectory: app.getPath("temp"),
+      allowInsecureHttp: protocolPolicy.allowInsecureHttp,
+    });
     const shouldInstall = await askToInstallUpdate(update, getParentWindow());
     if (!shouldInstall) {
       await shell.showItemInFolder(installerPath);
@@ -99,7 +109,12 @@ async function fetchLatestRelease(apiUrl: string): Promise<GiteaRelease> {
     throw new Error(`Gitea update check failed with HTTP ${response.status}.`);
   }
 
-  return (await response.json()) as GiteaRelease;
+  const release = parseGiteaRelease(await response.json());
+  if (!release) {
+    throw new Error("Gitea update metadata is invalid.");
+  }
+
+  return release;
 }
 
 async function openReleasePage(releasePageUrl: string | undefined): Promise<void> {
@@ -107,3 +122,9 @@ async function openReleasePage(releasePageUrl: string | undefined): Promise<void
     await shell.openExternal(releasePageUrl);
   }
 }
+
+function getUpdateProtocolPolicy(): UpdateProtocolPolicy {
+  return {
+    allowInsecureHttp: !app.isPackaged,
+  };
+}

src/main/updates/update-utils.ts

@@ -1,123 +0,0 @@
-export type GiteaReleaseAsset = {
-  name?: unknown;
-  browser_download_url?: unknown;
-  size?: unknown;
-};
-
-export type GiteaRelease = {
-  tag_name?: unknown;
-  name?: unknown;
-  html_url?: unknown;
-  assets?: unknown;
-};
-
-export type InstallerAsset = {
-  name: string;
-  downloadUrl: string;
-  size?: number;
-};
-
-export type ReleaseUpdate = {
-  version: string;
-  title: string;
-  pageUrl?: string;
-  installer: InstallerAsset;
-};
-
-export type UpdateFileConfig = {
-  enabled?: unknown;
-  apiUrl?: unknown;
-  releasePageUrl?: unknown;
-  assetPattern?: unknown;
-};
-
-export function isVersionNewer(candidateVersion: string, currentVersion: string): boolean {
-  const candidate = normalizeVersion(candidateVersion);
-  const current = normalizeVersion(currentVersion);
-
-  for (let index = 0; index < Math.max(candidate.length, current.length); index += 1) {
-    const candidatePart = candidate[index] ?? 0;
-    const currentPart = current[index] ?? 0;
-
-    if (candidatePart > currentPart) {
-      return true;
-    }
-
-    if (candidatePart < currentPart) {
-      return false;
-    }
-  }
-
-  return false;
-}
-
-export function getReleaseVersion(release: GiteaRelease): string | null {
-  const tagName = typeof release.tag_name === "string" ? release.tag_name.trim() : "";
-  return tagName.length > 0 ? tagName.replace(/^v/i, "") : null;
-}
-
-export function getReleaseTitle(release: GiteaRelease, version: string): string {
-  const releaseName = typeof release.name === "string" ? release.name.trim() : "";
-  return releaseName.length > 0 ? releaseName : `Scoreko ${version}`;
-}
-
-export function selectInstallerAsset(release: GiteaRelease, assetPattern: string): InstallerAsset | null {
-  const assets = Array.isArray(release.assets) ? release.assets : [];
-  const matcher = new RegExp(assetPattern, "i");
-
-  for (const asset of assets as GiteaReleaseAsset[]) {
-    const name = typeof asset.name === "string" ? asset.name : "";
-    const downloadUrl = typeof asset.browser_download_url === "string" ? asset.browser_download_url : "";
-
-    if (!name || !downloadUrl || !matcher.test(name)) {
-      continue;
-    }
-
-    return {
-      name,
-      downloadUrl,
-      ...(typeof asset.size === "number" ? { size: asset.size } : {}),
-    };
-  }
-
-  return null;
-}
-
-export function buildReleaseUpdate(
-  release: GiteaRelease,
-  currentVersion: string,
-  assetPattern: string,
-): ReleaseUpdate | null {
-  const version = getReleaseVersion(release);
-  if (!version || !isVersionNewer(version, currentVersion)) {
-    return null;
-  }
-
-  const installer = selectInstallerAsset(release, assetPattern);
-  if (!installer) {
-    return null;
-  }
-
-  const pageUrl = typeof release.html_url === "string" && release.html_url.length > 0 ? release.html_url : undefined;
-
-  return {
-    version,
-    title: getReleaseTitle(release, version),
-    pageUrl,
-    installer,
-  };
-}
-
-export function sanitizeFileName(fileName: string): string {
-  return fileName.replace(/[<>:"/\\|?*\x00-\x1f]/g, "_");
-}
-
-function normalizeVersion(version: string): number[] {
-  return version
-    .trim()
-    .replace(/^v/i, "")
-    .split(/[+-]/)[0]
-    .split(".")
-    .map((part) => Number.parseInt(part, 10))
-    .map((part) => (Number.isFinite(part) ? part : 0));
-}