From 6b37e050c586199139f666ee5c3e7ae888d3f9db Mon Sep 17 00:00:00 2001
From: Pixnop <77785313+Pixnop@users.noreply.github.com>
Date: Sun, 20 Sep 2026 12:27:50 +0200
Subject: [PATCH 1/4] Use the shared safe file name rule in the download worker

The worker carried its own copy of a rule validation.ts already states,
on an import line that already reached that module. The shared function
takes the caller's wording, so the refusal text is unchanged.

The local describe block went with it: tests/ipc-validation.test.ts
already pins every rule for the shared function, including the two
extensions the .zip suffix regression cost us.
---
 src/ipc/workers/download.ts | 23 ++++++++---------------
 tests/ipc/download.test.ts  | 29 +----------------------------
 2 files changed, 9 insertions(+), 43 deletions(-)

diff --git a/src/ipc/workers/download.ts b/src/ipc/workers/download.ts
index ba9096f5..7d6b097e 100644
--- a/src/ipc/workers/download.ts
+++ b/src/ipc/workers/download.ts
@@ -20,7 +20,7 @@ import fse from "fs-extra"
 import { join } from "node:path"
 
 // Relative so the module stays importable from a plain test run, like extraction.ts.
-import { assertAllowedDownloadUrl, assertAllowedRedirectUrl, optimumTestOrigin } from "../validation"
+import { assertAllowedDownloadUrl, assertAllowedRedirectUrl, assertSafeFileName, optimumTestOrigin } from "../validation"
 
 const MAX_DOWNLOAD_BYTES = 2 * 1024 * 1024 * 1024
 const DOWNLOAD_TIMEOUT_MS = 30_000
@@ -43,18 +43,6 @@ function isRedirectStatus(statusCode: number): boolean {
 /** Namespace used by temporary download siblings and the orphan sweep. */
 export const DOWNLOAD_TEMP_FILE_NAMESPACE = "riftlauncher"
 
-/**
- * The download is saved under exactly the name the caller asked for.
- *
- * It used to gain a `.zip` suffix here whatever the format really was, which is
- * how a Linux `.tar.gz` and a Windows `.exe` both ended up on disk as
- * `<version>.zip` and broke extraction and the installer alike.
- */
-export function assertSafeFileName(value: unknown): string {
-  if (typeof value !== "string" || value.length === 0 || value.length > 255 || value === "." || value === ".." || /[\\/\0]/.test(value)) throw new Error("Invalid download file name")
-  return value
-}
-
 /** The `https.request` shape, so a test can answer without a socket. */
 export type DownloadRequestFn = (url: URL, options: RequestOptions, callback: (response: IncomingMessage) => void) => ClientRequest
 
@@ -73,7 +61,12 @@ export interface DownloadOptions {
   url: unknown
   /** Folder the file lands in. Created when missing. */
   outputPath: string
-  /** Name to save under, exactly as given. */
+  /**
+   * Name to save under, exactly as given. It used to gain a `.zip` suffix
+   * whatever the format really was, which is how a Linux `.tar.gz` and a
+   * Windows `.exe` both ended up on disk as `<version>.zip` and broke
+   * extraction and the installer alike.
+   */
   fileName: unknown
   /** MD5 the finished file has to match, when the caller knows one. */
   expectedMd5?: unknown
@@ -117,7 +110,7 @@ export interface DownloadOptions {
 export function runDownload(options: DownloadOptions): Promise<string> {
   const { url, outputPath, fileName, expectedMd5, expectedSha256, maxBytes = MAX_DOWNLOAD_BYTES, request = nodeRequest, onProgress } = options
   const byteCeiling = Math.min(maxBytes, MAX_DOWNLOAD_BYTES)
-  const pathToDownload = join(outputPath, assertSafeFileName(fileName))
+  const pathToDownload = join(outputPath, assertSafeFileName(fileName, "download file name"))
   const temporaryPath = `${pathToDownload}.${DOWNLOAD_TEMP_FILE_NAMESPACE}.${process.pid}.${Date.now()}.part`
   const expectedDigest = typeof expectedSha256 === "string" ? expectedSha256 : expectedMd5
 
diff --git a/tests/ipc/download.test.ts b/tests/ipc/download.test.ts
index d4f236de..e423102f 100644
--- a/tests/ipc/download.test.ts
+++ b/tests/ipc/download.test.ts
@@ -9,7 +9,7 @@ import { afterEach, beforeEach, describe, it, vi } from "vitest"
 
 import type { ClientRequest, IncomingMessage } from "node:http"
 
-import { assertSafeFileName, DOWNLOAD_TEMP_FILE_NAMESPACE, runDownload, type DownloadRequestFn } from "@src/ipc/workers/download"
+import { DOWNLOAD_TEMP_FILE_NAMESPACE, runDownload, type DownloadRequestFn } from "@src/ipc/workers/download"
 
 /**
  * The download's own logic, driven without a socket.
@@ -137,33 +137,6 @@ afterEach(() => {
   rmSync(workspace, { recursive: true, force: true })
 })
 
-describe("assertSafeFileName", () => {
-  it("returns the name it was given", () => {
-    assert.equal(assertSafeFileName("vs_client_linux-x64_1.22.6.tar.gz"), "vs_client_linux-x64_1.22.6.tar.gz")
-  })
-
-  it("keeps the extension the caller asked for, whatever it is", () => {
-    // The suffix used to be forced to `.zip` here, which is how a tar.gz and an
-    // installer both landed on disk as `<version>.zip`.
-    assert.equal(assertSafeFileName("vs_setup_1.22.6.exe"), "vs_setup_1.22.6.exe")
-  })
-
-  for (const [label, value] of [
-    ["a non-string", 42],
-    ["an empty name", ""],
-    ["a name over 255 characters", "a".repeat(256)],
-    ["the current folder", "."],
-    ["the parent folder", ".."],
-    ["a POSIX separator", "nested/name.zip"],
-    ["a Windows separator", "nested\\name.zip"],
-    ["an embedded NUL", "name\0.zip"]
-  ] as const) {
-    it(`refuses ${label}`, () => {
-      assert.throws(() => assertSafeFileName(value), /Invalid download file name/)
-    })
-  }
-})
-
 describe("runDownload", () => {
   it("writes the response under the requested name and reports progress", async () => {
     const payload = body("vintage", "story")

From 4401d8733110b1761bddb64b5688b5cb1bc6cb27 Mon Sep 17 00:00:00 2001
From: Pixnop <77785313+Pixnop@users.noreply.github.com>
Date: Sun, 20 Sep 2026 12:35:56 +0200
Subject: [PATCH 2/4] Share one symlink ancestor walk between the path policy
 and the workers

The same walk was written twice, once in pathPolicy.ts for managed paths
and once in workers/extraction.ts for the eight worker call sites. Both
came from the fork seed, so neither was a chosen split, and a guard held
in two copies is a guard that can be tightened on one side only.

It lives in validation.ts now, which both sides already import and which
stays Electron-free. The refusal takes the caller's wording, so the path
policy keeps its "for managed paths" text, and it is a TypeError on both
sides now; nothing anywhere branches on the class.
---
 src/ipc/pathPolicy.ts             | 25 ++-------------------
 src/ipc/validation.ts             | 37 ++++++++++++++++++++++++++++++-
 src/ipc/workers/extraction.ts     | 27 ++++------------------
 src/ipc/workers/innoExtraction.ts |  3 ++-
 4 files changed, 44 insertions(+), 48 deletions(-)

diff --git a/src/ipc/pathPolicy.ts b/src/ipc/pathPolicy.ts
index 419de6b7..dd2d1b34 100644
--- a/src/ipc/pathPolicy.ts
+++ b/src/ipc/pathPolicy.ts
@@ -5,7 +5,7 @@ import { basename, dirname, join, resolve } from "node:path"
 import { getConfig } from "@src/config/configManager"
 import { MODS_FOLDER_NAME } from "@domain/mods/folder"
 import type { PathGrant } from "@src/ipc/validation"
-import { assertNonRootPath, comparablePath, isPathGranted, isRestoreWorkspaceName } from "@src/ipc/validation"
+import { assertNonRootPath, assertNoSymlinkComponents, comparablePath, isPathGranted, isRestoreWorkspaceName } from "@src/ipc/validation"
 
 type ApprovedPath = PathGrant & {
   expiresAt: number
@@ -153,27 +153,6 @@ function getProtectedPaths(config: ConfigType): string[] {
   return [app.getPath("userData"), app.getPath("appData"), app.getPath("home"), app.getAppPath(), ...getConfiguredFolders(config), ...getLauncherFolders()]
 }
 
-function assertNoSymlinkComponents(pathValue: string): void {
-  let current = resolve(pathValue)
-  let parent = dirname(current)
-
-  while (!fse.existsSync(current)) {
-    if (parent === current) break
-    current = parent
-    parent = dirname(current)
-  }
-
-  while (current !== parent) {
-    const stats = fse.lstatSync(current)
-    if (stats.isSymbolicLink()) throw new TypeError("Symbolic links are not allowed for managed paths")
-    current = parent
-    parent = dirname(current)
-  }
-
-  const rootStats = fse.lstatSync(current)
-  if (rootStats.isSymbolicLink()) throw new TypeError("Symbolic links are not allowed for managed paths")
-}
-
 export async function assertManagedPath(value: unknown, name = "path", options: PathPolicyOptions = {}): Promise<string> {
   const pathValue = resolve(assertNonRootPath(value, name))
   const config = await getConfig()
@@ -183,7 +162,7 @@ export async function assertManagedPath(value: unknown, name = "path", options:
   if (!isConfiguredPath && !isApprovedPath && !isRestoreWorkspace) throw new TypeError(`Unmanaged ${name}`)
 
   if (!options.allowMissing && !fse.existsSync(pathValue)) throw new TypeError(`Missing ${name}`)
-  if (!options.allowSymlinks) assertNoSymlinkComponents(pathValue)
+  if (!options.allowSymlinks) assertNoSymlinkComponents(pathValue, "Symbolic links are not allowed for managed paths")
   return pathValue
 }
 
diff --git a/src/ipc/validation.ts b/src/ipc/validation.ts
index fde6582b..d751c878 100644
--- a/src/ipc/validation.ts
+++ b/src/ipc/validation.ts
@@ -1,4 +1,5 @@
-import { isAbsolute, relative, resolve, sep } from "node:path"
+import { existsSync, lstatSync } from "node:fs"
+import { dirname, isAbsolute, relative, resolve, sep } from "node:path"
 import { fileURLToPath } from "node:url"
 
 import semver from "semver"
@@ -171,6 +172,40 @@ export function isPathWithin(root: string, candidate: string, allowRoot = true):
   return (allowRoot && relativePath === "") || (relativePath !== "" && relativePath !== ".." && !relativePath.startsWith(`..${sep}`) && !isAbsolute(relativePath))
 }
 
+/**
+ * Refuses a path any of whose existing ancestors is a symbolic link.
+ *
+ * The walk starts at the deepest component that exists, because a caller may
+ * name a file it is about to create, and climbs to the filesystem root, which
+ * is checked too. A link anywhere on that chain means the path the caller
+ * vetted and the path the filesystem opens are not the same path.
+ *
+ * Both sides of the host call this one: the path policy before it hands a
+ * managed path to a handler, and the extraction workers around every write
+ * they make. Two copies is how one side gets tightened and the other left
+ * stale, the same reason the archive size ceilings moved here in #362.
+ *
+ * @param message Wording for the refusal, so the path policy keeps its own.
+ */
+export function assertNoSymlinkComponents(pathValue: string, message = "Symbolic links are not allowed"): void {
+  let current = resolve(pathValue)
+  let parent = dirname(current)
+
+  while (!existsSync(current)) {
+    if (parent === current) return
+    current = parent
+    parent = dirname(current)
+  }
+
+  while (current !== parent) {
+    if (lstatSync(current).isSymbolicLink()) throw new TypeError(message)
+    current = parent
+    parent = dirname(current)
+  }
+
+  if (lstatSync(current).isSymbolicLink()) throw new TypeError(message)
+}
+
 /**
  * One entry of a path allow list.
  *
diff --git a/src/ipc/workers/extraction.ts b/src/ipc/workers/extraction.ts
index a609b6f7..6cfb4277 100644
--- a/src/ipc/workers/extraction.ts
+++ b/src/ipc/workers/extraction.ts
@@ -29,33 +29,14 @@ import * as tar from "tar"
 
 // Relative so the module stays importable from a plain test run, like validation.ts.
 import type { ArchiveSizeLimits } from "../validation"
-// The size ceilings used to be re-declared here as local copies of the two in
-// validation.ts. They are imported now: a backup is read under a different pair
-// (#362) and two places to change one number is how the pairs drift apart.
-import { archiveSizeLimits, isSafeTarEntryType, isTarGzName } from "../validation"
+// The size ceilings and the symlink walk used to be local copies of rules
+// validation.ts already states. They are imported now: a backup is read under a
+// different pair (#362) and two places to change one rule is how the pair drifts.
+import { archiveSizeLimits, assertNoSymlinkComponents, isSafeTarEntryType, isTarGzName } from "../validation"
 import { validateArchive } from "../archiveValidation"
 
 const MAX_ARCHIVE_ENTRIES = 100_000
 
-export function assertNoSymlinkComponents(pathValue: string): void {
-  let current = resolve(pathValue)
-  let parent = resolve(current, "..")
-  while (!fse.existsSync(current)) {
-    if (parent === current) return
-    current = parent
-    parent = resolve(current, "..")
-  }
-
-  while (current !== parent) {
-    const stats = fse.lstatSync(current)
-    if (stats.isSymbolicLink()) throw new Error("Symbolic links are not allowed")
-    current = parent
-    parent = resolve(current, "..")
-  }
-
-  if (fse.lstatSync(current).isSymbolicLink()) throw new Error("Symbolic links are not allowed")
-}
-
 export type ArchiveStats = { entries: number; bytes: number }
 
 export function validateTree(root: string, limits: ArchiveSizeLimits = archiveSizeLimits(false)): ArchiveStats {
diff --git a/src/ipc/workers/innoExtraction.ts b/src/ipc/workers/innoExtraction.ts
index 3ac3078c..986a4684 100644
--- a/src/ipc/workers/innoExtraction.ts
+++ b/src/ipc/workers/innoExtraction.ts
@@ -26,7 +26,8 @@ import { isInnoFormatError } from "../../domain/inno/errors"
 import type { InnoExtractionResult } from "../../domain/inno/extract"
 import type { Lzma2DecoderFactory } from "../../domain/inno/lzma"
 import type { InnoInstallerFile } from "../../domain/inno/ports"
-import { assertNoSymlinkComponents, copyTree, validateTree } from "./extraction"
+import { assertNoSymlinkComponents } from "../validation"
+import { copyTree, validateTree } from "./extraction"
 import { isNativeLzma2Error, loadNativeLzma2DecoderFactory } from "./nativeLzma2"
 
 /**

From 30afd363a19e4cfb7a1e570cb98d6138d427e326 Mon Sep 17 00:00:00 2001
From: Pixnop <77785313+Pixnop@users.noreply.github.com>
Date: Sun, 20 Sep 2026 12:43:55 +0200
Subject: [PATCH 3/4] Read a bounded response through one loop for both
 transports

The Electron and the Node transport each carried their own Content-Length
pre-check, status check and streamed byte-cap loop. Two transports is
deliberate (#76); two reading loops was not, and they had drifted: only
the Electron copy threw the typed BoundedResponseError that #442 added
for the release-notes classifier.

collectBounded now reads for both. The refusal messages are byte for byte
what they were, which netHandlers.fetchModDbListingArchive and the login
classifier both match on, and a non-2xx from either transport is now the
typed error.

loginFailureReason reads the status off that error instead of parsing the
digits back out of the message with an anchored regex. Same tokens for
every case the transport can produce, including a response with no status
line, which still lands on http-other. The module stays Electron-free:
the property is read structurally, the way codeOf already reads code.
The tests that stood in for the transport build the typed shape now.
---
 src/ipc/handlers/loginFailureReason.ts |  50 +++++----
 src/ipc/network.ts                     | 137 ++++++++++++-------------
 tests/ipc/accountHandlers.test.ts      |  34 +++---
 tests/ipc/loginFailureReason.test.ts   |  45 +++++---
 4 files changed, 135 insertions(+), 131 deletions(-)

diff --git a/src/ipc/handlers/loginFailureReason.ts b/src/ipc/handlers/loginFailureReason.ts
index 3da7c6e3..d9a6a101 100644
--- a/src/ipc/handlers/loginFailureReason.ts
+++ b/src/ipc/handlers/loginFailureReason.ts
@@ -19,11 +19,10 @@
  * into it. `code` and `name` are both public and writable, so a value that
  * merely looks like a Node enum member proves nothing about where it came
  * from: they are read as lookup keys into the tables below and never copied
- * into the answer. An HTTP status is parsed out of the message as a number
- * and then used the same way, because the message is written from the
- * response and a status the caller can influence is no safer than a field the
- * thrower can set. A key that is not in a table maps to that table's
- * catch-all token.
+ * into the answer. The HTTP status a refused response carries is used the same
+ * way, because it comes from the response and a status the caller can
+ * influence is no safer than a field the thrower can set. A key that is not in
+ * a table maps to that table's catch-all token.
  *
  * The other half of the answer is where the error came from. The handler's
  * catch wraps two very different things: the network round trip and the
@@ -82,23 +81,18 @@ export const STORAGE_MESSAGES = new Map<string, string>([
   ["A system password store is required for account storage", "no-system-password-store"]
 ])
 
-/** `Network request failed with status 503`. The digits are read as a number, never carried over as text. */
-const STATUS_MESSAGE = /^Network request failed with status (\d{3}|unknown)$/
-
 /**
  * The HTTP statuses the auth service actually answers with, each mapped to
  * the token that names it.
  *
- * Not `http-status-${status}`: `network.ts` writes that message from the
- * response's own status line, and `assertString` accepts `503` as a password,
- * so a login with that password and an outage on the other end used to put
- * the password in the log. The status is parsed into a number here and then
- * treated exactly like `code`, as a lookup key whose value never reaches the
- * answer. A status outside the table degrades to its range, which still
- * separates "the service rejected us" from "the service is broken", and
- * anything that is not a 4xx or 5xx (a redirect this transport does not
- * follow, or the literal `unknown` when the response had no status line at
- * all) is `http-other`.
+ * Not `http-status-${status}`: the status comes from the response's own status
+ * line, and `assertString` accepts `503` as a password, so a login with that
+ * password and an outage on the other end used to put the password in the log.
+ * The status is treated exactly like `code`, as a lookup key whose value never
+ * reaches the answer. A status outside the table degrades to its range, which
+ * still separates "the service rejected us" from "the service is broken", and
+ * anything that is not a 4xx or 5xx (a redirect this transport does not follow,
+ * or no status line at all) is `http-other`.
  */
 export const HTTP_STATUSES = new Map<number, string>([
   [400, "http-bad-request"],
@@ -193,17 +187,21 @@ export const ERROR_NAMES = new Map<string, string>([
   ["AbortError", "unclassified-AbortError"]
 ])
 
-/** Names an HTTP failure without ever formatting the status back into a string. */
-function httpReason(status: string | undefined): string {
-  const code = Number(status) // `unknown` and a missing group are both NaN, and every comparison below is false for NaN.
-  const named = HTTP_STATUSES.get(code)
+/** Names an HTTP failure without ever formatting the status back into a string. Every comparison is false for NaN, so a response with no status line lands on `http-other`. */
+function httpReason(status: number): string {
+  const named = HTTP_STATUSES.get(status)
   if (named) return named
-  if (code >= 400 && code < 500) return "http-4xx"
-  if (code >= 500 && code < 600) return "http-5xx"
+  if (status >= 400 && status < 500) return "http-4xx"
+  if (status >= 500 && status < 600) return "http-5xx"
 
   return "http-other"
 }
 
+/** The status a refusal carries, as a lookup key only: `network.ts` sets `statusCode` on every non-2xx it throws (`BoundedResponseError`), including when the response had no status line and `Number` reads it as NaN. Structural rather than `instanceof`, so this module stays Electron-free. */
+function statusOf(error: Error): number | undefined {
+  return "statusCode" in error ? Number((error as { statusCode?: unknown }).statusCode) : undefined
+}
+
 /** The `code` an error carries, as a lookup key only: a non-string is no key at all. */
 function codeOf(error: Error): string {
   const code: unknown = (error as { code?: unknown }).code
@@ -228,8 +226,8 @@ export function loginFailureReason(error: unknown): string {
   const known = NETWORK_MESSAGES.get(error.message)
   if (known) return known
 
-  const status = STATUS_MESSAGE.exec(error.message)
-  if (status) return httpReason(status[1])
+  const status = statusOf(error)
+  if (status !== undefined) return httpReason(status)
 
   const code = codeOf(error)
   if (code) return NETWORK_CODES.get(code) ?? "network-other"
diff --git a/src/ipc/network.ts b/src/ipc/network.ts
index 9f44d445..9f747d70 100644
--- a/src/ipc/network.ts
+++ b/src/ipc/network.ts
@@ -58,6 +58,64 @@ export class BoundedResponseError extends Error {
   }
 }
 
+/** What {@link collectBounded} reads off a response. Electron's `net` and Node's `http(s)` both answer this shape. */
+type BoundedResponse = NodeJS.EventEmitter & {
+  statusCode?: number
+  headers: Record<string, string | string[] | undefined>
+}
+
+/**
+ * Reads one response into `chunks` under `maxBytes`, then settles `finish`.
+ *
+ * The ceiling is checked twice: against the declared `Content-Length` before a
+ * byte is read, and against the running total as the chunks arrive, so a lying
+ * length costs the cap and not the heap. A non-2xx is a refusal rather than a
+ * body, carrying its status and headers for the callers that classify one (see
+ * {@link BoundedResponseError}); every other caller keeps matching on the
+ * message. Both transports below read through this, so no cap, status rule or
+ * refusal text can be tightened on one side only.
+ *
+ * @param cancel How this transport drops the exchange: `request.abort` for
+ * Electron's `net`, `request.destroy` for Node's `http(s)`.
+ * @param finish The caller's settle-once function, given the error on a refusal.
+ */
+function collectBounded(response: BoundedResponse, maxBytes: number, chunks: Buffer[], cancel: () => void, finish: (error?: Error) => void): void {
+  const contentLengthHeader = response.headers["content-length"]
+  const contentLength = Number(Array.isArray(contentLengthHeader) ? contentLengthHeader[0] : contentLengthHeader)
+
+  const refuse = (error: Error): void => {
+    cancel()
+    finish(error)
+  }
+
+  if (Number.isFinite(contentLength) && contentLength > maxBytes) {
+    refuse(new Error("Network response is too large"))
+    return
+  }
+
+  if (response.statusCode === undefined || response.statusCode < 200 || response.statusCode >= 300) {
+    refuse(new BoundedResponseError(`Network request failed with status ${response.statusCode ?? "unknown"}`, response.statusCode, response.headers))
+    return
+  }
+
+  let responseBytes = 0
+
+  response.on("data", (chunk: Buffer | string) => {
+    const chunkBuffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)
+    responseBytes += chunkBuffer.length
+
+    if (responseBytes > maxBytes) {
+      refuse(new Error("Network response is too large"))
+      return
+    }
+
+    chunks.push(chunkBuffer)
+  })
+  response.on("end", () => finish())
+  response.on("aborted", () => finish(new Error("Network response was aborted")))
+  response.on("error", (error: Error) => finish(error))
+}
+
 export function requestBoundedText(url: URL, options: BoundedRequestOptions = {}): Promise<string> {
   return requestBoundedBuffer(url, options).then((bytes) => bytes.toString("utf8"))
 }
@@ -74,7 +132,6 @@ export function requestBoundedBuffer(url: URL, options: BoundedRequestOptions =
 
   return new Promise((resolve, reject) => {
     let settled = false
-    let responseBytes = 0
     const chunks: Buffer[] = []
     const request = net.request({
       url: url.toString(),
@@ -100,39 +157,7 @@ export function requestBoundedBuffer(url: URL, options: BoundedRequestOptions =
       }
     }
 
-    request.on("response", (response) => {
-      const contentLengthHeader = response.headers["content-length"]
-      const contentLengthValue = Array.isArray(contentLengthHeader) ? contentLengthHeader[0] : contentLengthHeader
-      const contentLength = Number(contentLengthValue)
-
-      if (Number.isFinite(contentLength) && contentLength > maxBytes) {
-        request.abort()
-        finish(new Error("Network response is too large"))
-        return
-      }
-
-      if (response.statusCode === undefined || response.statusCode < 200 || response.statusCode >= 300) {
-        request.abort()
-        finish(new BoundedResponseError(`Network request failed with status ${response.statusCode ?? "unknown"}`, response.statusCode, response.headers))
-        return
-      }
-
-      response.on("data", (chunk: Buffer | string) => {
-        const chunkBuffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)
-        responseBytes += chunkBuffer.length
-
-        if (responseBytes > maxBytes) {
-          request.abort()
-          finish(new Error("Network response is too large"))
-          return
-        }
-
-        chunks.push(chunkBuffer)
-      })
-      response.on("end", () => finish())
-      response.on("aborted", () => finish(new Error("Network response was aborted")))
-      response.on("error", (error) => finish(error))
-    })
+    request.on("response", (response) => collectBounded(response, maxBytes, chunks, () => request.abort(), finish))
 
     request.on("error", (error) => finish(error))
     request.on("login", (_authInfo, callback) => callback())
@@ -159,11 +184,11 @@ export function requestBoundedBuffer(url: URL, options: BoundedRequestOptions =
  * multi-gigabyte file download needs a streamed-to-disk write and a
  * socket-inactivity timeout, not the wall-clock timeout this function keeps.
  *
- * Every guarantee `requestBoundedText` carries is preserved here and no
- * caller of `requestBoundedText` is affected, since that function is
- * untouched:
- *  - the response is capped at `maxBytes`, checked against `Content-Length`
- *    up front and against the running streamed total as chunks arrive;
+ * Every guarantee `requestBoundedText` carries is preserved here, because the
+ * reading itself is the same code: both transports hand their response to
+ * {@link collectBounded}, so the cap, the status rule and the refusal text
+ * cannot be tightened on one side only. What stays split is what genuinely
+ * differs, the transport and how it is cancelled. On top of that:
  *  - the whole exchange is bounded by `REQUEST_TIMEOUT_MS`, the same
  *    wall-clock timeout `requestBoundedText` uses (not `request.setTimeout`,
  *    which only measures socket inactivity and would let a slow-trickling
@@ -208,7 +233,6 @@ export function requestBoundedTextViaNode(url: URL, options: BoundedRequestOptio
 
   return new Promise((resolve, reject) => {
     let settled = false
-    let responseBytes = 0
     const chunks: Buffer[] = []
     // Whichever request is in flight right now (the CONNECT, or the real one), so the
     // one timeout below can abort it without knowing which phase it landed in.
@@ -231,38 +255,7 @@ export function requestBoundedTextViaNode(url: URL, options: BoundedRequestOptio
       finish(new Error("Network request timed out"))
     }, timeoutMs)
 
-    function onResponse(response: IncomingMessage): void {
-      const contentLengthHeader = response.headers["content-length"]
-      const contentLength = Number(contentLengthHeader)
-
-      if (Number.isFinite(contentLength) && contentLength > maxBytes) {
-        abortInFlight()
-        finish(new Error("Network response is too large"))
-        return
-      }
-
-      if (response.statusCode === undefined || response.statusCode < 200 || response.statusCode >= 300) {
-        abortInFlight()
-        finish(new Error(`Network request failed with status ${response.statusCode ?? "unknown"}`))
-        return
-      }
-
-      response.on("data", (chunk: Buffer | string) => {
-        const chunkBuffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)
-        responseBytes += chunkBuffer.length
-
-        if (responseBytes > maxBytes) {
-          abortInFlight()
-          finish(new Error("Network response is too large"))
-          return
-        }
-
-        chunks.push(chunkBuffer)
-      })
-      response.on("end", () => finish())
-      response.on("aborted", () => finish(new Error("Network response was aborted")))
-      response.on("error", (error) => finish(error))
-    }
+    const onResponse = (response: IncomingMessage): void => collectBounded(response, maxBytes, chunks, () => abortInFlight(), finish)
 
     // A tunneled request is always sent with node:http's own `request`, `agent` and all,
     // never `https.request`: the agent already hands back a socket doing TLS on its own
diff --git a/tests/ipc/accountHandlers.test.ts b/tests/ipc/accountHandlers.test.ts
index c6ec7c4c..6fb56818 100644
--- a/tests/ipc/accountHandlers.test.ts
+++ b/tests/ipc/accountHandlers.test.ts
@@ -57,6 +57,11 @@ const EMAIL = "player@example.invalid"
 const PASSWORD = "placeholder-password"
 const TWO_FACTOR_CODE = "123456"
 
+/** What `network.ts` rejects with for a non-2xx answer: the transport's own message, plus the status as a number (`BoundedResponseError`). */
+function refusedWithStatus(statusCode: number): Error {
+  return Object.assign(new Error(`Network request failed with status ${statusCode}`), { statusCode })
+}
+
 /** A body the service answers when the credentials are good. Placeholder values throughout. */
 const SUCCESS_BODY = JSON.stringify({
   valid: 1,
@@ -328,8 +333,8 @@ describe("LOGIN resolves a family status for a request failure it can classify",
   for (const [label, thrown, expectedStatus] of [
     ["a name that will not resolve", Object.assign(new Error("getaddrinfo ENOTFOUND auth3.vintagestory.at"), { code: "ENOTFOUND" }), "network-unreachable"],
     ["a certificate this machine will not accept", Object.assign(new Error("self signed certificate"), { code: "DEPTH_ZERO_SELF_SIGNED_CERT" }), "certificate-error"],
-    ["the service answering with a 503", new Error("Network request failed with status 503"), "service-error"],
-    ["the service answering with a 403", new Error("Network request failed with status 403"), "account-restricted"]
+    ["the service answering with a 503", refusedWithStatus(503), "service-error"],
+    ["the service answering with a 403", refusedWithStatus(403), "account-restricted"]
   ] as const) {
     it(`reports ${label} as ${expectedStatus}, not the generic failure`, async () => {
       vi.mocked(requestBoundedTextViaNode).mockRejectedValueOnce(thrown)
@@ -463,7 +468,7 @@ describe("LOGIN keeps credentials out of the log when it fails", () => {
     // matching status instead of throwing the generic failure (issue #481).
     for (const [thrown, expectedReason, expectedStatus] of [
       [Object.assign(new Error(`getaddrinfo ENOTFOUND while sending ${LEAKY_PASSWORD}`), { code: "ENOTFOUND" }), "network-ENOTFOUND", "network-unreachable"],
-      [new Error("Network request failed with status 503"), "http-unavailable", "service-error"],
+      [refusedWithStatus(503), "http-unavailable", "service-error"],
       [new Error("Network request timed out"), "timeout", "network-unreachable"]
     ] as const) {
       vi.mocked(requestBoundedTextViaNode).mockReset().mockRejectedValueOnce(thrown)
@@ -545,13 +550,12 @@ describe("LOGIN keeps credentials out of the log when it fails", () => {
   })
 
   it("logs no credential when the password is the same digits as the HTTP status", async () => {
-    // `assertString` accepts `503` as a password, and `network.ts` writes
-    // "Network request failed with status 503" from the response's own status
-    // line. A reason built by splicing those digits in therefore writes the
-    // password to the log the moment the auth service goes down, without the
-    // thrower doing anything unusual.
+    // `assertString` accepts `503` as a password, and the refusal `network.ts`
+    // throws carries the response's own status. A reason built by splicing
+    // those digits in therefore writes the password to the log the moment the
+    // auth service goes down, without the thrower doing anything unusual.
     const password = "503"
-    vi.mocked(requestBoundedTextViaNode).mockRejectedValueOnce(new Error(`Network request failed with status ${password}`))
+    vi.mocked(requestBoundedTextViaNode).mockRejectedValueOnce(refusedWithStatus(503))
 
     let result: AccountLoginResult | undefined
     const lines = await logLinesDuring(async () => {
@@ -573,13 +577,11 @@ describe("LOGIN keeps credentials out of the log when it fails", () => {
     // and an outage into one line, which is the first split a field report
     // needs. All three resolve rather than throw, each into its own family.
     for (const [status, expectedReason, expectedStatus] of [
-      ["401", "http-unauthorized", "account-restricted"],
-      ["429", "http-rate-limited", "service-error"],
-      ["503", "http-unavailable", "service-error"]
+      [401, "http-unauthorized", "account-restricted"],
+      [429, "http-rate-limited", "service-error"],
+      [503, "http-unavailable", "service-error"]
     ] as const) {
-      vi.mocked(requestBoundedTextViaNode)
-        .mockReset()
-        .mockRejectedValueOnce(new Error(`Network request failed with status ${status}`))
+      vi.mocked(requestBoundedTextViaNode).mockReset().mockRejectedValueOnce(refusedWithStatus(status))
 
       let result: AccountLoginResult | undefined
       const lines = await logLinesDuring(async () => {
@@ -591,7 +593,7 @@ describe("LOGIN keeps credentials out of the log when it fails", () => {
         lines.some((line) => line.includes(`Login failure reason: ${expectedReason}.`)),
         `no line named the reason ${expectedReason}: ${lines.join(" / ")}`
       )
-      assertNothingSecretIn(lines, [LEAKY_PASSWORD, status])
+      assertNothingSecretIn(lines, [LEAKY_PASSWORD, String(status)])
     }
   })
 })
diff --git a/tests/ipc/loginFailureReason.test.ts b/tests/ipc/loginFailureReason.test.ts
index a605e80b..ec5a3872 100644
--- a/tests/ipc/loginFailureReason.test.ts
+++ b/tests/ipc/loginFailureReason.test.ts
@@ -23,6 +23,16 @@ import {
  * Every password below is a placeholder that exists only to be asserted
  * absent.
  */
+/**
+ * What `network.ts` throws for a non-2xx answer, built by hand so this file
+ * stays Electron-free: `BoundedResponseError`'s message, plus the status as a
+ * number. The class sets `statusCode` even when the response carried no status
+ * line, so an undefined one is still an own property here.
+ */
+function refusedWithStatus(statusCode: number | undefined): Error {
+  return Object.assign(new Error(`Network request failed with status ${statusCode ?? "unknown"}`), { statusCode })
+}
+
 describe("loginFailureReason names what went wrong", () => {
   for (const [message, expected] of [
     ["Network request timed out", "timeout"],
@@ -37,27 +47,27 @@ describe("loginFailureReason names what went wrong", () => {
   }
 
   it("names the HTTP failure, so a 503 outage is not read as a wrong password", () => {
-    assert.equal(loginFailureReason(new Error("Network request failed with status 401")), "http-unauthorized")
-    assert.equal(loginFailureReason(new Error("Network request failed with status 429")), "http-rate-limited")
-    assert.equal(loginFailureReason(new Error("Network request failed with status 503")), "http-unavailable")
+    assert.equal(loginFailureReason(refusedWithStatus(401)), "http-unauthorized")
+    assert.equal(loginFailureReason(refusedWithStatus(429)), "http-rate-limited")
+    assert.equal(loginFailureReason(refusedWithStatus(503)), "http-unavailable")
   })
 
   it("never writes the status digits, which the response picks and a password can equal", () => {
-    // `network.ts` builds this message from the response's own status line,
-    // and `assertString` accepts `503` as a password, so a token built by
-    // splicing the digits in puts that password in the log the moment the
-    // service goes down. Every token below is a literal from the module.
-    for (const status of ["401", "403", "429", "500", "503", "418", "599", "302", "unknown"]) {
-      const reason = loginFailureReason(new Error(`Network request failed with status ${status}`))
-      assert.equal(reason.includes(status), false, `the status digits reached the reason: ${reason}`)
+    // The status comes from the response's own status line, and `assertString`
+    // accepts `503` as a password, so a token built by splicing the digits in
+    // puts that password in the log the moment the service goes down. Every
+    // token below is a literal from the module.
+    for (const status of [401, 403, 429, 500, 503, 418, 599, 302]) {
+      const reason = loginFailureReason(refusedWithStatus(status))
+      assert.equal(reason.includes(String(status)), false, `the status digits reached the reason: ${reason}`)
     }
   })
 
   it("degrades an unlisted status to its range, and anything else to http-other", () => {
-    assert.equal(loginFailureReason(new Error("Network request failed with status 418")), "http-4xx")
-    assert.equal(loginFailureReason(new Error("Network request failed with status 599")), "http-5xx")
-    assert.equal(loginFailureReason(new Error("Network request failed with status 302")), "http-other")
-    assert.equal(loginFailureReason(new Error("Network request failed with status unknown")), "http-other")
+    assert.equal(loginFailureReason(refusedWithStatus(418)), "http-4xx")
+    assert.equal(loginFailureReason(refusedWithStatus(599)), "http-5xx")
+    assert.equal(loginFailureReason(refusedWithStatus(302)), "http-other")
+    assert.equal(loginFailureReason(refusedWithStatus(undefined)), "http-other")
   })
 
   it("names the system error code when the socket is what failed", () => {
@@ -175,12 +185,13 @@ describe("loginFailureReason cannot carry a secret out", () => {
     assert.equal(reason, "storage-other")
   })
 
-  it("matches the status message whole, so a longer one is not sliced for its middle", () => {
+  it("reads the status off the error, not out of a message that merely looks like one", () => {
     const reason = loginFailureReason(new Error(`Network request failed with status 500 for body ${PASSWORD}`))
 
     assert.equal(reason.includes(PASSWORD), false)
-    // Anchored, so a message that merely starts like the known one is not
-    // matched and sliced: it falls through to the class name instead.
+    // Nothing is parsed out of the message text, so an error that only reads
+    // like a status refusal is not treated as one: it falls through to the
+    // class name, and no part of the message reaches the answer.
     assert.equal(reason, "unclassified-Error")
   })
 })

From 227effa3fb84981fa6778657860440f3e8f9ebfe Mon Sep 17 00:00:00 2001
From: Pixnop <77785313+Pixnop@users.noreply.github.com>
Date: Sun, 20 Sep 2026 13:11:42 +0200
Subject: [PATCH 4/4] Pin the status the Node transport carries on a refusal

loginFailureReason reads statusCode now instead of parsing the message,
so the message match alone no longer pins the login chain end to end: a
transport that answered a plain Error with the same text would send a
503 outage to unclassified and put the generic "Login failed" on screen
where the player should read that the service is down. The 404 case now
asserts the property as well as the message.
---
 tests/ipc/network.test.ts | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tests/ipc/network.test.ts b/tests/ipc/network.test.ts
index ada47729..aed4fc9c 100644
--- a/tests/ipc/network.test.ts
+++ b/tests/ipc/network.test.ts
@@ -104,13 +104,22 @@ describe("requestBoundedTextViaNode enforces its timeout", () => {
 })
 
 describe("requestBoundedTextViaNode rejects a non-2xx status", () => {
-  it("rejects a 404 rather than resolving its body", async () => {
+  it("rejects a 404 carrying the status, which is what classifies a login failure", async () => {
     const url = await startServer((_req, res) => {
       res.writeHead(404, { "Content-Type": "application/json" })
       res.end(JSON.stringify({ valid: 0, reason: "notfound" }))
     })
 
-    await assert.rejects(requestBoundedTextViaNode(url), /404/)
+    await assert.rejects(requestBoundedTextViaNode(url), (error: Error & { statusCode?: number }) => {
+      assert.match(error.message, /404/)
+      // The half the message alone cannot pin (issue #481): since
+      // `loginFailureReason` reads `statusCode` instead of parsing the
+      // message, a transport that answered a plain Error with the same text
+      // would classify a 503 outage as `unclassified` and put "Login failed"
+      // on screen where the player should read that the service is down.
+      assert.equal(error.statusCode, 404)
+      return true
+    })
   })
 })