diff --git a/docs/decisions/0001-shell-and-codebase.md b/docs/decisions/0001-shell-and-codebase.md index 350cec3d..b8ab4322 100644 --- a/docs/decisions/0001-shell-and-codebase.md +++ b/docs/decisions/0001-shell-and-codebase.md @@ -14,6 +14,8 @@ So the question is no longer only "which shell". It is: Every number below was measured on 2026-08-16, on this machine, with the command named beside it. Where a number could not be measured, that is said instead of guessed. +> Note, 2026-08-29: the measurements below stand as they were taken, and the 7-Zip ones no longer describe the tree. PR #274 removed `node-7z` and `7zip-bin`, so the six `7za` binaries and the 9.4 MB `app.asar.unpacked` line are gone, a Linux `--dir` build is 9.5 MiB smaller, and the `7za l -slt` reader that Option A's cost paragraph leans on no longer exists: backups are gzipped tar read by `tar`, and the zips written before 1.7.0-beta.4 are read by `yauzl`. The test and coverage figures have moved a long way too. Nothing here is restated, because the argument was put to the deciders with the numbers of the day and rewriting it after the fact would misrepresent what was actually weighed. + ## What was measured ### The fork diff --git a/docs/vintage-story-quirks.md b/docs/vintage-story-quirks.md index 6bdebe11..f621312b 100644 --- a/docs/vintage-story-quirks.md +++ b/docs/vintage-story-quirks.md @@ -14,11 +14,11 @@ Field knowledge for anyone touching the game catalog, archive extraction, the in ## Archive -**A single wrapping folder gets flattened.** `contentRoot()` in `src/ipc/workers/extraction.ts:112-131` steps into an archive's contents only when the root holds exactly one entry and it is a directory; anything else (including an already-flat server archive) is left untouched. It is applied only to game archives, not server archives (`extraction.ts:261`: `copyTree(isGameArchive ? contentRoot(extractionRoot) : extractionRoot, outputPath)`). Covered by `tests/ipc/extraction.test.ts` ("unpacks a wrapped archive straight into the target folder" / "leaves a flat archive flat"). +**A single wrapping folder gets flattened.** `contentRoot()` in `src/ipc/workers/extraction.ts:112-131` steps into an archive's contents only when the root holds exactly one entry and it is a directory; anything else (including an already-flat server archive) is left untouched. It is applied only when the caller asks for it (`unwrapSingleRootFolder`, which the game version install passes and the backup restore does not: `copyTree(unwrapSingleRootFolder ? contentRoot(extractionRoot) : extractionRoot, outputPath)`). Covered by `tests/ipc/extraction.test.ts` ("unpacks a wrapped archive straight into the target folder" / "leaves a flat archive flat"). **Zero-size entries have to survive extraction as zero-size files, not be treated as errors or skipped.** `tests/ipc/extraction.test.ts:132-142` builds an archive containing an empty `assets/version-1.22.6.txt` (the version marker from the Catalog section above) and asserts the extracted file exists with `size === 0`. `validateTree` in `src/ipc/workers/extraction.ts` counts zero-byte files explicitly rather than special-casing them away. -**Vintage Story's tars have NUL bytes in their numeric header fields, which the bundled p7zip rejects; node-tar (and GNU tar) accept them, which is why `.tar.gz` extraction goes through the `tar` npm package instead of 7-Zip.** The reasoning is spelled out in `src/ipc/workers/extraction.ts:151-158`: "7-Zip is not used here. It needs two passes for a `.tar.gz`, and the bundled p7zip 16.02 cannot read the tar Vintage Story ships at all: its headers leave the numeric fields as NUL bytes, which GNU tar and node-tar accept and 7-Zip rejects with 'Is not archive'." Every other archive format goes through `node-7z`/`7zip-bin`; only tar.gz is routed to `tar.extract` (`extractTarGz`, same file). +**Vintage Story's tars have NUL bytes in their numeric header fields, which p7zip rejects and node-tar (and GNU tar) accept.** This is one of the reasons the launcher reads `.tar.gz` with the `tar` npm package, and it was the reason it never routed game archives through 7-Zip even while 7-Zip was still bundled. Since the launcher dropped `node-7z` and `7zip-bin` (issue #222), only two formats are read at all: gzipped tar through `tar` (`extractTarGz` in `src/ipc/workers/extraction.ts`, for game builds and for the backups the launcher writes) and zip through `yauzl` (`extractZip`, same file, for the backups made before that change). Anything else is refused by `validateArchive` in `src/ipc/archiveValidation.ts` rather than handed to a reader that would have to guess at it. ## Installer diff --git a/electron-builder.yml b/electron-builder.yml index f52ec9f0..ca3cd5ac 100644 --- a/electron-builder.yml +++ b/electron-builder.yml @@ -22,7 +22,6 @@ files: - "!node_modules/@napi-rs/lzma-*-musl{,/**/*}" asarUnpack: - resources/** - - node_modules/7zip-bin/** - node_modules/@napi-rs/lzma-*/** win: executableName: RiftLauncher diff --git a/package-lock.json b/package-lock.json index 664b9238..af9f1277 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,12 +11,10 @@ "dependencies": { "@electron-toolkit/utils": "^3.0.0", "@napi-rs/lzma": "1.5.1", - "7zip-bin": "^5.2.0", "electron-log": "^5.2.3", "electron-updater": "^6.8.9", "fs-extra": "^11.2.0", "json5": "^2.2.3", - "node-7z": "^3.0.0", "tar": "7.5.22", "write-file-atomic": "^8.0.0", "yauzl": "^3.4.0" @@ -31,7 +29,6 @@ "@testing-library/react": "^16.3.2", "@testing-library/user-event": "^14.6.4", "@types/node": "^20.14.8", - "@types/node-7z": "^2.1.11", "@types/react": "^18.3.3", "@types/react-dom": "^18.3.0", "@types/semver": "^7.5.8", @@ -3305,16 +3302,6 @@ "undici-types": "~6.21.0" } }, - "node_modules/@types/node-7z": { - "version": "2.1.11", - "resolved": "https://registry.npmjs.org/@types/node-7z/-/node-7z-2.1.11.tgz", - "integrity": "sha512-7gwLx44tqZqjyrvjkX41CWW4h7+aXrazFg/JR6N5g+R5BW1eqsNuw8SNLWrh7KcnfKhAYFiWyNb10ti5v5eCmQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "@types/node": "*" - } - }, "node_modules/@types/prop-types": { "version": "15.7.15", "resolved": "https://registry.npmjs.org/@types/prop-types/-/prop-types-15.7.15.tgz", @@ -3745,12 +3732,6 @@ "node": ">=10.0.0" } }, - "node_modules/7zip-bin": { - "version": "5.2.0", - "resolved": "https://registry.npmjs.org/7zip-bin/-/7zip-bin-5.2.0.tgz", - "integrity": "sha512-ukTPVhqG4jNzMro2qA9HSCSSVJN3aN7tlb+hfqYCt3ER0yWroeA2VR38MNrOHLQ/cVj+DaIMad0kFCtWWowh/A==", - "license": "MIT" - }, "node_modules/abbrev": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-4.0.0.tgz", @@ -8155,36 +8136,12 @@ "dev": true, "license": "MIT" }, - "node_modules/lodash.defaultsdeep": { - "version": "4.6.1", - "resolved": "https://registry.npmjs.org/lodash.defaultsdeep/-/lodash.defaultsdeep-4.6.1.tgz", - "integrity": "sha512-3j8wdDzYuWO3lM3Reg03MuQR957t287Rpcxp1njpEa8oDrikb+FwGdW3n+FELh/A6qib6yPit0j/pv9G/yeAqA==", - "license": "MIT" - }, - "node_modules/lodash.defaultto": { - "version": "4.14.0", - "resolved": "https://registry.npmjs.org/lodash.defaultto/-/lodash.defaultto-4.14.0.tgz", - "integrity": "sha512-G6tizqH6rg4P5j32Wy4Z3ZIip7OfG8YWWlPFzUFGcYStH1Ld0l1tWs6NevEQNEDnO1M3NZYjuHuraaFSN5WqeQ==", - "license": "MIT" - }, "node_modules/lodash.escaperegexp": { "version": "4.1.2", "resolved": "https://registry.npmjs.org/lodash.escaperegexp/-/lodash.escaperegexp-4.1.2.tgz", "integrity": "sha512-TM9YBvyC84ZxE3rgfefxUWiQKLilstD6k7PTGt6wfbtXF8ixIJLOL3VYyV/z+ZiPLsVxAsKAFVwWlWeb2Y8Yyw==", "license": "MIT" }, - "node_modules/lodash.flattendeep": { - "version": "4.4.0", - "resolved": "https://registry.npmjs.org/lodash.flattendeep/-/lodash.flattendeep-4.4.0.tgz", - "integrity": "sha512-uHaJFihxmJcEX3kT4I23ABqKKalJ/zDrDg0lsFtc1h+3uw49SIJ5beyhx5ExVRti3AvKoOJngIj7xz3oylPdWQ==", - "license": "MIT" - }, - "node_modules/lodash.isempty": { - "version": "4.4.0", - "resolved": "https://registry.npmjs.org/lodash.isempty/-/lodash.isempty-4.4.0.tgz", - "integrity": "sha512-oKMuF3xEeqDltrGMfDxAPGIVMSSRv8tbRSODbrs4KGsRRLEhrW8N8Rd4DRgB2+621hY8A8XwwrTVhXWpxFvMzg==", - "license": "MIT" - }, "node_modules/lodash.isequal": { "version": "4.5.0", "resolved": "https://registry.npmjs.org/lodash.isequal/-/lodash.isequal-4.5.0.tgz", @@ -8199,12 +8156,6 @@ "dev": true, "license": "MIT" }, - "node_modules/lodash.negate": { - "version": "3.0.2", - "resolved": "https://registry.npmjs.org/lodash.negate/-/lodash.negate-3.0.2.tgz", - "integrity": "sha512-JGJYYVslKYC0tRMm/7igfdHulCjoXjoganRNWM8AgS+RXfOvFnPkOveDhPI65F9aAypCX9QEEQoBqWf7Q6uAeA==", - "license": "MIT" - }, "node_modules/loose-envify": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/loose-envify/-/loose-envify-1.4.0.tgz", @@ -8525,24 +8476,6 @@ "dev": true, "license": "MIT" }, - "node_modules/node-7z": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/node-7z/-/node-7z-3.0.0.tgz", - "integrity": "sha512-KIznWSxIkOYO/vOgKQfJEaXd7rgoFYKZbaurainCEdMhYc7V7mRHX+qdf2HgbpQFcdJL/Q6/XOPrDLoBeTfuZA==", - "license": "ISC", - "dependencies": { - "debug": "^4.3.2", - "lodash.defaultsdeep": "^4.6.1", - "lodash.defaultto": "^4.14.0", - "lodash.flattendeep": "^4.4.0", - "lodash.isempty": "^4.4.0", - "lodash.negate": "^3.0.2", - "normalize-path": "^3.0.0" - }, - "engines": { - "node": ">=10" - } - }, "node_modules/node-abi": { "version": "4.33.0", "resolved": "https://registry.npmjs.org/node-abi/-/node-abi-4.33.0.tgz", @@ -8670,15 +8603,6 @@ "node": "^20.17.0 || >=22.9.0" } }, - "node_modules/normalize-path": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/normalize-path/-/normalize-path-3.0.0.tgz", - "integrity": "sha512-6eZs5Ls3WtCisHWp9S2GUy8dqkpGi4BVSz3GaqiE6ezub0512ESztXUwUB6C6IKbQkY2Pnb/mD4WYojCRwcwLA==", - "license": "MIT", - "engines": { - "node": ">=0.10.0" - } - }, "node_modules/normalize-url": { "version": "6.1.0", "resolved": "https://registry.npmjs.org/normalize-url/-/normalize-url-6.1.0.tgz", diff --git a/package.json b/package.json index 9e66eb71..9a34d37c 100644 --- a/package.json +++ b/package.json @@ -34,12 +34,10 @@ "dependencies": { "@electron-toolkit/utils": "^3.0.0", "@napi-rs/lzma": "1.5.1", - "7zip-bin": "^5.2.0", "electron-log": "^5.2.3", "electron-updater": "^6.8.9", "fs-extra": "^11.2.0", "json5": "^2.2.3", - "node-7z": "^3.0.0", "tar": "7.5.22", "write-file-atomic": "^8.0.0", "yauzl": "^3.4.0" @@ -54,7 +52,6 @@ "@testing-library/react": "^16.3.2", "@testing-library/user-event": "^14.6.4", "@types/node": "^20.14.8", - "@types/node-7z": "^2.1.11", "@types/react": "^18.3.3", "@types/react-dom": "^18.3.0", "@types/semver": "^7.5.8", diff --git a/scripts/fix-native-deps.js b/scripts/fix-native-deps.js index f446406d..bc542495 100644 --- a/scripts/fix-native-deps.js +++ b/scripts/fix-native-deps.js @@ -1,49 +1,23 @@ #!/usr/bin/env node /** - * `npm ci` on this project leaves two native-tooling gaps that have nothing - * to do with the code being worked on. Both are dev/test-only: packaged - * builds are unaffected because electron-builder and electron-vite handle - * them on their own. + * `npm ci` on this project leaves one native-tooling gap that has nothing to do + * with the code being worked on. It is dev/test-only: packaged builds are + * unaffected because electron-builder and electron-vite handle it on their own. * - * 1. node_modules/7zip-bin///7za(.exe) sometimes loses its - * executable bit in transit (registries and some npm/tar combinations do - * not reliably preserve unix permissions while packing/unpacking). Any - * code path that spawns it then fails with `EACCES`. See #30. + * As of Electron 42, the `electron` package shipped its own `postinstall` + * (`node install.js`) that downloaded the platform binary; that hook was + * removed from its package.json, so a plain `npm ci` no longer fetches + * node_modules/electron/dist at all. `npm run dev` and anything that spawns + * Electron then fails until install.js is run by hand. Cheap to close here + * since this script already runs on install. * - * 2. As of Electron 42, the `electron` package shipped its own - * `postinstall` (`node install.js`) that downloaded the platform binary; - * that hook was removed from its package.json, so a plain `npm ci` no - * longer fetches node_modules/electron/dist at all. `npm run dev` and - * anything that spawns Electron then fails until install.js is run by - * hand. Cheap to close here since this script already runs on install. - * - * Both fixes are idempotent and safe to run on every `npm install`. + * The fix is idempotent and safe to run on every `npm install`. */ -const { chmodSync, existsSync, statSync } = require("node:fs") +const { existsSync } = require("node:fs") const { join } = require("node:path") const { spawnSync } = require("node:child_process") -function restoreSevenZipExecutableBit() { - // .exe needs no unix executable bit, and Windows has no such concept. - if (process.platform === "win32") return - - let path7za - try { - ;({ path7za } = require("7zip-bin")) - } catch { - return // 7zip-bin is not installed; nothing to fix. - } - - if (!path7za || !existsSync(path7za)) return - - const isExecutable = (statSync(path7za).mode & 0o111) !== 0 - if (isExecutable) return - - chmodSync(path7za, 0o755) - console.log(`[fix-native-deps] restored executable bit on ${path7za}`) -} - function ensureElectronBinaryIsDownloaded() { const electronDir = join(__dirname, "..", "node_modules", "electron") const installScript = join(electronDir, "install.js") @@ -63,5 +37,4 @@ function ensureElectronBinaryIsDownloaded() { } } -restoreSevenZipExecutableBit() ensureElectronBinaryIsDownloaded() diff --git a/src/domain/config/defaults.ts b/src/domain/config/defaults.ts index 4f8f2236..d32f9b60 100644 --- a/src/domain/config/defaults.ts +++ b/src/domain/config/defaults.ts @@ -32,10 +32,12 @@ export const DEFAULT_CONFIG_BASE: Omit.zip". + // "_.tar.gz". const cleanInstallationName = cleanFolderName(installation.name) || installation.id.slice(0, 8) const dateStamp = formatTimestampForFilename(date) - const fileName = `${cleanInstallationName}_${dateStamp}.zip` + // Backups were zips up to 1.7.0-beta.4 and the restore still reads those, by + // the extension recorded with each one. New ones are gzipped tar. + const fileName = `${cleanInstallationName}_${dateStamp}.tar.gz` const outputFolder = await ports.paths.join([backupsFolder, INSTALLATIONS_BACKUP_SUBFOLDER, cleanInstallationName]) const archivePath = await ports.paths.join([outputFolder, fileName]) diff --git a/src/ipc/archiveValidation.ts b/src/ipc/archiveValidation.ts index 0ff1bead..1cbcce68 100644 --- a/src/ipc/archiveValidation.ts +++ b/src/ipc/archiveValidation.ts @@ -10,24 +10,21 @@ * only one. * * Called from inside the worker rather than from the IPC handler before it - * starts one: validateSevenZipArchive's own parse of a `7z l` listing can run - * to 100,000 entries over up to 4MB of text, real CPU work that has no - * business running on the main process's event loop when a worker thread is - * about to exist for this archive regardless. + * starts one: walking a table of contents can run to 100,000 entries, real CPU + * work that has no business running on the main process's event loop when a + * worker thread is about to exist for this archive regardless. * * Which reader runs is decided by the file name, which is exactly why the name * has to be the real one: a `.tar.gz` saved as `.zip` used to be handed to a * zip reader that could not read it, and the install died there. */ -import { spawn } from "node:child_process" import yauzl from "yauzl" import * as tar from "tar" import { isArchiveSymlink, isSafeArchiveEntry, isSafeTarEntryType, isTarGzName, MAX_ARCHIVE_ENTRY_BYTES, MAX_ARCHIVE_TOTAL_BYTES } from "./validation" const MAX_ARCHIVE_ENTRIES = 100_000 -const MAX_LISTING_BYTES = 4 * 1024 * 1024 function comparableArchiveEntry(entryName: string): string { const normalizedName = entryName.replaceAll("\\", "/") @@ -89,90 +86,11 @@ export function validateZipArchive(filePath: string): Promise { }) } -export function validateSevenZipArchive(filePath: string, sevenZipBin: string): Promise { - return new Promise((resolvePromise, rejectPromise) => { - const archiveLister = spawn(sevenZipBin, ["l", "-slt", "-bb0", filePath], { windowsHide: true }) - let output = "" - let settled = false - - const finish = (error?: Error): void => { - if (settled) return - settled = true - if (error) rejectPromise(error) - else resolvePromise() - } - - archiveLister.stdout.on("data", (chunk) => { - output += chunk.toString() - if (Buffer.byteLength(output, "utf8") > MAX_LISTING_BYTES) { - archiveLister.kill() - finish(new Error("Archive listing is too large")) - } - }) - archiveLister.stderr.on("data", () => {}) - archiveLister.on("error", (error) => finish(error)) - archiveLister.on("close", (code) => { - if (settled) return - if (code !== 0) { - finish(new Error("Archive could not be listed")) - return - } - - const records = output.split(/\r?\n\s*\r?\n/) - let entryCount = 0 - let totalUncompressedBytes = 0 - let firstRecord = true - const entryNames = new Set() - for (const record of records) { - const lines = record.split(/\r?\n/) - const pathLine = lines.find((line) => line.startsWith("Path = ")) - if (!pathLine) continue - if (firstRecord) { - firstRecord = false - continue - } - - const entryName = pathLine.slice("Path = ".length) - const sizeLine = lines.find((line) => line.startsWith("Size = ")) - const folderLine = lines.find((line) => line.startsWith("Folder = ")) - const isFolder = folderLine?.slice("Folder = ".length).trim() === "+" - const attributesLine = lines.find((line) => line.startsWith("Attributes = ")) - // Tar style listings report links in their own columns instead of the attributes one. - const linkLines = lines.filter((line) => line.startsWith("Symbolic Link = ") || line.startsWith("Hard Link = ")) - const entrySize = sizeLine ? Number(sizeLine.slice("Size = ".length)) : 0 - entryCount++ - totalUncompressedBytes += Number.isFinite(entrySize) && entrySize >= 0 ? entrySize : 0 - - if ( - entryCount > MAX_ARCHIVE_ENTRIES || - !isSafeArchiveEntry(entryName) || - entryNames.has(comparableArchiveEntry(entryName)) || - (attributesLine !== undefined && /(^|\s)L(\s|$)/.test(attributesLine)) || - linkLines.some((line) => line.slice(line.indexOf("= ") + 2).trim().length > 0) || - (!isFolder && (!sizeLine || !Number.isFinite(entrySize) || entrySize < 0)) || - entrySize > MAX_ARCHIVE_ENTRY_BYTES || - totalUncompressedBytes > MAX_ARCHIVE_TOTAL_BYTES - ) { - finish(new Error("Archive contains an unsafe entry")) - return - } - entryNames.add(comparableArchiveEntry(entryName)) - } - - finish() - }) - }) -} - /** - * Lists a gzipped tar and holds it to the same bounds as the other formats. + * Lists a gzipped tar and holds it to the same bounds as the zip reader. * - * 7-Zip is not used for this one. It would only see the gzip container and - * report the single `.tar` inside, and the bundled p7zip 16.02 cannot read the - * tars Vintage Story ships at all: their headers leave the numeric fields as - * NUL bytes, which GNU tar and node-tar accept and 7-Zip calls "Is not - * archive". The tar reader also states an entry's kind outright, so links are - * refused by type rather than by parsing an attributes column. + * The tar reader states an entry's kind outright, so links are refused by type + * rather than by reading an attributes column. */ export async function validateTarGzArchive(filePath: string): Promise { let entryCount = 0 @@ -221,12 +139,16 @@ export async function validateTarGzArchive(filePath: string): Promise { /** * Checks an archive with the reader its format actually needs. * + * Two formats reach the launcher and no others: gzipped tar, which the game + * builds ship as and which every backup is written as, and zip, which is what + * the backups made before that change still are. Anything else is refused here + * rather than handed to a reader that would have to guess at it. + * * @param filePath Archive on disk. Its name decides the reader. - * @param sevenZipBin 7-Zip binary, for the formats neither yauzl nor tar handles. * @throws When the archive cannot be read or holds an entry the launcher refuses to unpack. */ -export async function validateArchive(filePath: string, sevenZipBin: string): Promise { - if (filePath.toLowerCase().endsWith(".zip")) return validateZipArchive(filePath) +export async function validateArchive(filePath: string): Promise { if (isTarGzName(filePath)) return validateTarGzArchive(filePath) - return validateSevenZipArchive(filePath, sevenZipBin) + if (filePath.toLowerCase().endsWith(".zip")) return validateZipArchive(filePath) + throw new Error("Archive format is not supported") } diff --git a/src/ipc/handlers/pathsHandlers.ts b/src/ipc/handlers/pathsHandlers.ts index 740ccd45..6dd1c2b4 100644 --- a/src/ipc/handlers/pathsHandlers.ts +++ b/src/ipc/handlers/pathsHandlers.ts @@ -1,5 +1,4 @@ import { ipcMain, app, shell } from "electron" -import { path7za } from "7zip-bin" import fse from "fs-extra" import { open } from "node:fs/promises" import type { Stats } from "node:fs" @@ -27,7 +26,6 @@ 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 sevenZipBin = app.isPackaged ? path7za.replace("app.asar", "app.asar.unpacked") : path7za const WORKER_TIMEOUTS_MS: Record = { DOWNLOAD_ON_PATH: 45 * 60 * 1_000, EXTRACT_ON_PATH: 30 * 60 * 1_000, @@ -77,16 +75,18 @@ const RUN_INSTALLER_TIMEOUT_MS = 15 * 60 * 1_000 * first progress event, so this needs no renderer-side change to look right. * * Extraction and compression share one limiter instead of each getting their own: both - * spawn a 7-Zip subprocess and compete for the same CPU cores, so the resource that - * actually needs bounding is "concurrent 7-Zip processes", not "concurrent extractions" - * and "concurrent compressions" separately. + * inflate or deflate inside a worker thread and compete for the same CPU cores, so the + * resource that actually needs bounding is "concurrent archive workers", not "concurrent + * extractions" and "concurrent compressions" separately. This used to be written around + * the 7-Zip subprocesses both of them spawned; the work moved in process with #222, and + * the bound holds for the same reason it always did. * - * RUN_INSTALLER's payload read shares that lane too, even though it spawns no 7-Zip. What + * RUN_INSTALLER's payload read shares that lane too, even though it unpacks no archive. What * it does instead is decode the installer's LZMA2 payload with the plain-TypeScript decoder * in src/domain/inno and CRC32 every chunk on the way through, so it saturates a core inside * its worker thread for the whole read (41 seconds for a 598 MB installer, per - * WORKER_TIMEOUTS_MS above). Read the limit as "concurrent CPU-bound archive readers" rather - * than "concurrent 7-Zip processes" and it clearly belongs: an install starting while two mod + * WORKER_TIMEOUTS_MS above). Read the limit as "concurrent CPU-bound archive workers" and it + * clearly belongs: an install starting while two mod * extractions were already running used to put three of those on the machine at once, against * a bound written to allow two. */ @@ -163,7 +163,7 @@ function runTrackedWorker( const timeout = setTimeout( () => { // Never reused: the abandoned task is still running inside this thread (still - // holding a socket or a 7-Zip child), so its eventual message could still arrive + // holding a socket or an open archive), so its eventual message could still arrive // after some later task has been dispatched to the same worker. rejectOnce(new Error(`${operationName} timed out`), "discard") }, @@ -402,17 +402,18 @@ ipcMain.handle(IPC_CHANNELS.PATHS_MANAGER.DOWNLOAD_ON_PATH, async (event, id: st return downloadedPath }) -ipcMain.handle(IPC_CHANNELS.PATHS_MANAGER.EXTRACT_ON_PATH, async (event, id: string, filePath: string, outputPath: string, deleteZip: boolean): Promise => { +ipcMain.handle(IPC_CHANNELS.PATHS_MANAGER.EXTRACT_ON_PATH, async (event, id: string, filePath: string, outputPath: string, deleteZip: boolean, unwrapSingleRootFolder = false): Promise => { assertTrustedIpcSender(event) const safeId = assertSafeTaskId(id) const safeFilePath = await assertManagedPath(filePath, "archive path") const safeOutputPath = await assertManagedPath(outputPath, "output path", { allowMissing: true }) const shouldDeleteZip = assertBoolean(deleteZip, "delete archive flag") + const shouldUnwrapSingleRootFolder = assertBoolean(unwrapSingleRootFolder, "unwrap single root folder flag") if (resolve(safeFilePath) === resolve(safeOutputPath)) throw new TypeError("Archive and output paths must differ") // validateArchive runs inside the extraction worker now (workers/extraction.ts's - // runExtraction), not here: its 7z-listing parse is real CPU work that has no business - // blocking the main process's event loop. + // runExtraction), not here: walking a table of contents is real CPU work that has no + // business blocking the main process's event loop. logMessage("info", `[back] [ipc] [ipc/handlers/pathsHandlers.ts] [EXTRACT_ON_PATH] [${safeId}] Starting a bounded extraction.`) await archiveConcurrency.run(() => { @@ -422,7 +423,7 @@ ipcMain.handle(IPC_CHANNELS.PATHS_MANAGER.EXTRACT_ON_PATH, async (event, id: str safeId, IPC_CHANNELS.PATHS_MANAGER.EXTRACT_PROGRESS, extractWorker, - { filePath: safeFilePath, outputPath: safeOutputPath, deleteZip: shouldDeleteZip, sevenZipBin }, + { filePath: safeFilePath, outputPath: safeOutputPath, deleteZip: shouldDeleteZip, unwrapSingleRootFolder: shouldUnwrapSingleRootFolder }, "EXTRACT_ON_PATH", () => true ) @@ -598,7 +599,7 @@ ipcMain.handle( safeId, IPC_CHANNELS.PATHS_MANAGER.COMPRESS_PROGRESS, compressWorker, - { inputPath: safeInputPath, outputPath: safeOutputPath, outputFileName: safeOutputFileName, compressionLevel: safeCompressionLevel, sevenZipBin }, + { inputPath: safeInputPath, outputPath: safeOutputPath, outputFileName: safeOutputFileName, compressionLevel: safeCompressionLevel }, "COMPRESS_ON_PATH", () => true ) diff --git a/src/ipc/pathPolicy.ts b/src/ipc/pathPolicy.ts index 18392d06..8782cb33 100644 --- a/src/ipc/pathPolicy.ts +++ b/src/ipc/pathPolicy.ts @@ -96,8 +96,9 @@ function getLauncherFolders(): string[] { * writes mods, saves, client settings and game files under them, and an * installation the user put outside every configured folder still has to work. * A backup grants its own path only. Every archive the launcher makes is a - * single `.zip` file (see makeInstallationBackup), so a path *under* one is - * never a path the launcher meant to reach. + * single file, a `.tar.gz` now and a `.zip` for the ones written before + * 1.7.0-beta.4 (see makeInstallationBackup), so a path *under* one is never a + * path the launcher meant to reach. */ function getEntryGrants(config: ConfigType): PathGrant[] { return [ diff --git a/src/ipc/validation.ts b/src/ipc/validation.ts index b8254738..b767b47f 100644 --- a/src/ipc/validation.ts +++ b/src/ipc/validation.ts @@ -374,8 +374,8 @@ export function isRestoreWorkspaceName(installationName: string, candidateName: } /** - * True when `name` is a gzipped tar, which 7-Zip cannot unpack in one pass and, - * for the archives Vintage Story ships, cannot read at all. + * True when `name` is a gzipped tar: the game builds, and every backup written + * from 1.7.0-beta.4 on. It picks the reader, `tar` rather than `yauzl`. */ export function isTarGzName(name: unknown): boolean { return typeof name === "string" && /\.(?:tar\.gz|tgz)$/i.test(name) diff --git a/src/ipc/workers/compressWorker.ts b/src/ipc/workers/compressWorker.ts index 2044d981..c4b2f7e8 100644 --- a/src/ipc/workers/compressWorker.ts +++ b/src/ipc/workers/compressWorker.ts @@ -3,14 +3,13 @@ import { runCompression } from "@src/ipc/workers/compression" serveTasks( async (payload, onProgress) => { - const { inputPath, outputPath, outputFileName, compressionLevel, sevenZipBin } = payload as { + const { inputPath, outputPath, outputFileName, compressionLevel } = payload as { inputPath: string outputPath: string outputFileName: string compressionLevel?: number - sevenZipBin?: string } - await runCompression({ inputPath, outputPath, outputFileName, compressionLevel, sevenZipBin, onProgress }) + await runCompression({ inputPath, outputPath, outputFileName, compressionLevel, onProgress }) }, () => "Compression failed" ) diff --git a/src/ipc/workers/compression.ts b/src/ipc/workers/compression.ts index 4e0a0743..282306b3 100644 --- a/src/ipc/workers/compression.ts +++ b/src/ipc/workers/compression.ts @@ -5,22 +5,48 @@ * and innoExtraction.ts already use, so the same code can be driven from a test * or a script. Nothing here touches Electron or `worker_threads`. * - * The source tree is walked before 7-Zip is handed anything: a symbolic link or - * a device node in there would be followed into whatever it points at, and an - * unbounded tree would keep the worker busy indefinitely. + * Backups are written as gzipped tar through the same `tar` package the game + * archives are already read with. The source tree is walked before tar is + * handed anything: a symbolic link or a device node in there would be followed + * into whatever it points at, and an unbounded tree would keep the worker busy + * indefinitely. The walk also totals the bytes, which is what progress is + * measured against. */ -import type { EventEmitter } from "node:events" -import Seven from "node-7z" import fse from "fs-extra" import { join } from "node:path" +import * as tar from "tar" import { DEFAULT_COMPRESSION_LEVEL } from "@domain/config/defaults" +// Relative so the module stays importable from a plain test run, like extraction.ts. +import { MAX_ARCHIVE_TOTAL_BYTES } from "../validation" + const MAX_ITEMS = 100_000 -/** The `Seven.add` shape, so a test can drive the progress and end events without spawning 7-Zip. */ -export type ArchiveAdd = (archivePath: string, source: string, options: NonNullable[2]>) => EventEmitter +/** + * The link cache tar keeps while it packs, rigged never to report a hit. + * + * tar looks a file up in here by `dev:ino` whenever its `nlink` is above one, + * and on a hit writes the second name as a `Link` entry pointing at the first + * instead of writing its bytes again. The restore reader refuses `Link`, so + * that backup reports success and can never be put back. A cache that answers + * nothing sends every name down the ordinary file path, so both names come out + * of the archive as independent copies carrying their own bytes. + * + * Deduplicating filesystems hand out hard links on their own, so a player can + * have them without ever having made one. Refusing the source instead would + * cost them backups entirely; this costs them the sharing, which was a disk + * layout detail rather than anything the installation depends on. + * + * The key type matches tar's own `LinkCacheKey`, which the package does not + * re-export from its entry point. + */ +class UnsharedLinkCache extends Map<`${number}:${number}`, string> { + get(): undefined { + return undefined + } +} /** * Refuses a source tree holding anything but plain files and folders, or more @@ -28,11 +54,13 @@ export type ArchiveAdd = (archivePath: string, source: string, options: NonNulla * * @param root Folder about to be archived. * @param fileSystem Filesystem to walk, defaulting to the real one. + * @returns Total size in bytes of every plain file in the tree. * @throws When an entry is a symbolic link or a special file, or the tree is too large. */ -export function assertSafeCompressionTree(root: string, fileSystem: Pick = fse): void { +export function assertSafeCompressionTree(root: string, fileSystem: Pick = fse): number { const pending = [root] let itemCount = 0 + let totalBytes = 0 while (pending.length > 0) { const current = pending.pop() as string @@ -43,8 +71,12 @@ export function assertSafeCompressionTree(root: string, fileSystem: Pick MAX_ITEMS) throw new Error("Too many filesystem entries") if (stats.isDirectory()) { for (const child of fileSystem.readdirSync(current)) pending.push(join(current, child)) + } else { + totalBytes += stats.size } } + + return totalBytes } export interface CompressionOptions { @@ -54,12 +86,8 @@ export interface CompressionOptions { outputPath: string /** Archive file name, taken as given. */ outputFileName: string - /** 7-Zip `-mx` level, 0 to 9. */ + /** gzip level, 0 to 9. Same scale the config's compressionLevel has always carried. */ compressionLevel?: number - /** 7-Zip binary. */ - sevenZipBin?: string - /** Injected `Seven.add`, defaulting to the real one. */ - addArchive?: ArchiveAdd /** Called with 0 to 100 as the work advances, and once with 100 at the end. */ onProgress?: (progress: number) => void } @@ -68,50 +96,77 @@ export interface CompressionOptions { * Compresses one folder into one archive. * * @param options Source, destination, and how to report progress. - * @throws As a rejection, for an unsafe source or destination and for a 7-Zip - * failure alike. The caller reports "Compression failed" either way. + * @throws As a rejection, for an unsafe source or destination and for a failed + * write alike. The caller reports "Compression failed" either way. */ -export function runCompression(options: CompressionOptions): Promise { - const { inputPath, outputPath, outputFileName, compressionLevel = DEFAULT_COMPRESSION_LEVEL, sevenZipBin, addArchive = Seven.add, onProgress } = options - - return new Promise((resolvePromise, rejectPromise) => { - assertSafeCompressionTree(inputPath) - if (!fse.existsSync(inputPath) || !fse.lstatSync(inputPath).isDirectory()) throw new Error("Compression source must be a directory") - if (!fse.existsSync(outputPath)) fse.mkdirSync(outputPath, { recursive: true }) - if (fse.lstatSync(outputPath).isSymbolicLink() || !fse.lstatSync(outputPath).isDirectory()) throw new Error("Compression destination is unsafe") - - const archivePath = join(outputPath, outputFileName) - if (fse.existsSync(archivePath)) { - const archiveStats = fse.lstatSync(archivePath) - if (archiveStats.isSymbolicLink() || archiveStats.isDirectory()) throw new Error("Compression archive target is unsafe") +export async function runCompression(options: CompressionOptions): Promise { + const { inputPath, outputPath, outputFileName, compressionLevel = DEFAULT_COMPRESSION_LEVEL, onProgress } = options + + const totalBytes = assertSafeCompressionTree(inputPath) + // The restore reader holds an archive to this same total and refuses anything + // past it, so an installation over the cap would compress happily into a + // backup that can never be put back. Refusing here costs the player a failed + // backup; not refusing costs them a backup they only find out is useless on + // the day they need it, and one prune slot that an older, restorable backup + // used to hold. + if (totalBytes > MAX_ARCHIVE_TOTAL_BYTES) throw new Error("Compression source is too large") + if (!fse.existsSync(inputPath) || !fse.lstatSync(inputPath).isDirectory()) throw new Error("Compression source must be a directory") + if (!fse.existsSync(outputPath)) fse.mkdirSync(outputPath, { recursive: true }) + if (fse.lstatSync(outputPath).isSymbolicLink() || !fse.lstatSync(outputPath).isDirectory()) throw new Error("Compression destination is unsafe") + + const archivePath = join(outputPath, outputFileName) + if (fse.existsSync(archivePath)) { + const archiveStats = fse.lstatSync(archivePath) + if (archiveStats.isSymbolicLink() || archiveStats.isDirectory()) throw new Error("Compression archive target is unsafe") + } + + const entries = fse.readdirSync(inputPath) + let writtenBytes = 0 + let lastReportedProgress = 0 + + try { + await tar.create( + { + file: archivePath, + cwd: inputPath, + gzip: { level: compressionLevel }, + portable: true, + // Two names for one inode would otherwise become a Link entry the + // restore reader refuses. See UnsharedLinkCache above. + linkCache: new UnsharedLinkCache(), + // `entry.size` is not filled in yet when this runs, and the stat behind + // the entry is the same number the walk above totalled. Folders are + // skipped for the same reason the walk skipped them: their stat size is + // the directory's own on-disk size, which is not content. + onWriteEntry: (entry) => { + writtenBytes += entry.stat?.isFile() ? entry.stat.size : 0 + if (totalBytes <= 0) return + // The terminal 100 below is the only one this ever reports, so the + // running figure is capped a point short of it. + const progress = Math.min(99, Math.floor((writtenBytes / totalBytes) * 100)) + if (progress > lastReportedProgress) { + lastReportedProgress = progress + onProgress?.(progress) + } + } + }, + // A folder with nothing in it is still a folder worth backing up, and tar + // refuses an empty list of paths. "." archives the folder itself, which + // unpacks back to an empty folder rather than to nothing at all. + entries.length > 0 ? entries : ["."] + ) + } catch { + // tar opens the archive as soon as it starts, so a write that failed partway + // leaves a truncated file sitting in the backups folder. No backup record + // ever names it, which is exactly what pruning walks, so it would never be + // cleaned up and every retry would leave another one. + try { + fse.removeSync(archivePath) + } catch { + // Best effort. The compression failure below is the outcome that matters. } - const sourceGlob = join(inputPath, "*") - - const stream = addArchive(archivePath, sourceGlob, { - $bin: sevenZipBin, - $progress: true, - recursive: true, - method: [`x=${compressionLevel}`, "mt=on"] - }) - - let lastReportedProgress = 0 - - stream.on("progress", ({ percent }: { percent: unknown }) => { - const boundedPercent = Number(percent) - // The end event below owns the single terminal 100 report. - if (Number.isFinite(boundedPercent) && boundedPercent > lastReportedProgress && boundedPercent < 100) { - lastReportedProgress = boundedPercent - onProgress?.(boundedPercent) - } - }) - - stream.on("end", () => { - onProgress?.(100) - resolvePromise() - }) - - stream.on("error", () => { - rejectPromise(new Error("Compression failed")) - }) - }) + throw new Error("Compression failed") + } + + onProgress?.(100) } diff --git a/src/ipc/workers/extractWorker.ts b/src/ipc/workers/extractWorker.ts index 86ba73cd..c233dbc5 100644 --- a/src/ipc/workers/extractWorker.ts +++ b/src/ipc/workers/extractWorker.ts @@ -3,8 +3,8 @@ import { runExtraction } from "@src/ipc/workers/extraction" serveTasks( async (payload, onProgress) => { - const { filePath, outputPath, deleteZip, sevenZipBin } = payload as { filePath: string; outputPath: string; deleteZip: boolean; sevenZipBin: string } - await runExtraction({ filePath, outputPath, deleteArchive: deleteZip, sevenZipBin, onProgress }) + const { filePath, outputPath, deleteZip, unwrapSingleRootFolder } = payload as { filePath: string; outputPath: string; deleteZip: boolean; unwrapSingleRootFolder: boolean } + await runExtraction({ filePath, outputPath, deleteArchive: deleteZip, unwrapSingleRootFolder, onProgress }) }, (error) => (error instanceof Error ? error.message : "Extraction failed") ) diff --git a/src/ipc/workers/extraction.ts b/src/ipc/workers/extraction.ts index 7fb320da..392b8fe5 100644 --- a/src/ipc/workers/extraction.ts +++ b/src/ipc/workers/extraction.ts @@ -9,11 +9,12 @@ * write a single byte where the launcher keeps its files. */ -import Seven from "node-7z" import fse from "fs-extra" -import { createReadStream, mkdtempSync } from "node:fs" -import { isAbsolute, join, relative, resolve, sep } from "node:path" +import yauzl from "yauzl" +import { createReadStream, createWriteStream, mkdtempSync } from "node:fs" +import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path" import { tmpdir } from "node:os" +import type { Readable } from "node:stream" import * as tar from "tar" // Relative so the module stays importable from a plain test run, like validation.ts. @@ -130,39 +131,174 @@ export function contentRoot(root: string): string { return fse.lstatSync(candidate).isDirectory() ? candidate : root } -function extractWithSevenZip(filePath: string, destination: string, sevenZipBin: string, onProgress?: (progress: number) => void): Promise { +/** + * Where one archive entry is allowed to land, or nothing. + * + * The archive's table of contents was already checked for escaping names before + * a byte was written (validateArchive, in archiveValidation.ts), and yauzl + * refuses a leading "/" or a ".." segment on its own before either. This is the + * check the writer itself makes anyway, on the resolved path rather than on the + * name, because a writer is the wrong place to trust a name that came out of a + * file someone else wrote. + */ +export function resolveEntryDestination(destination: string, entryName: string): string { + const resolvedDestination = resolve(destination) + const target = resolve(resolvedDestination, entryName.replaceAll("\\", "/")) + const relativePath = relative(resolvedDestination, target) + if (!relativePath || relativePath === ".." || relativePath.startsWith(`..${sep}`) || isAbsolute(relativePath)) throw new Error("Archive entry escaped its root") + return target +} + +/** + * Unpacks a zip. + * + * Only the backups the launcher wrote before it moved to gzipped tar arrive + * here, and they have to keep restoring for as long as players still hold them. + * Reading is yauzl's, the same reader the mod archives and the pre-extraction + * table-of-contents check already run on, so no zip writer or external process + * is needed to keep the old format readable. + * + * Progress is counted in entries rather than bytes: the entry count is the one + * total a zip states up front, per entry sizes are what the archive claims + * rather than what comes out, and a backup's entries are of a similar size. + * + * Unix mode bits recorded in a legacy backup are deliberately not carried over, + * unlike the tar reader, which restores what the archive holds. An installation + * folder is game data with nothing executable in it; on Linux every extraction + * is followed by a blanket chmod to 0755 (startExtract in + * TaskManagerContext.tsx), which overwrites what either reader restored; and a + * mode read out of an archive written by a tool the launcher no longer ships is + * as easily 0 as it is useful, which would leave a save file unreadable. The + * attributes are still read, for the symlink check, which is the one thing in + * them worth acting on. + */ +export function extractZip(filePath: string, destination: string, onProgress?: (progress: number) => void): Promise { return new Promise((resolvePromise, rejectPromise) => { - const stream = Seven.extractFull(filePath, destination, { $bin: sevenZipBin, $progress: true, recursive: true }) - let lastReportedProgress = 0 + yauzl.open(filePath, { lazyEntries: true }, (openError, zipFile) => { + if (openError || !zipFile) { + rejectPromise(new Error("Extraction failed")) + return + } - stream.on("progress", ({ percent }) => { - const boundedPercent = Number(percent) - // runExtraction emits the terminal 100 after validation and copying. Do not - // let 7-Zip publish a second terminal event before that point. - if (Number.isFinite(boundedPercent) && boundedPercent > lastReportedProgress && boundedPercent < 100) { - lastReportedProgress = boundedPercent - onProgress?.(boundedPercent) + let settled = false + let extractedEntries = 0 + let lastReportedProgress = 0 + const totalEntries = zipFile.entryCount + // The pair the entry being written is streaming through, so a failure can + // stop them. Reassigned per entry, and left pointing at the last one. + let entryReader: Readable | undefined + let entryWriter: ReturnType | undefined + + const finish = (failure?: Error): void => { + if (settled) return + settled = true + if (failure) { + // Stop both ends before the caller wipes the temporary folder, the same + // reason extractTarGz does it: an open write handle in there turns the + // removal into an EBUSY on Windows, which then replaces the real error + // and leaves the folder behind. + entryReader?.unpipe() + entryReader?.destroy() + entryWriter?.destroy() + } + try { + zipFile.close() + } catch { + // Already closed after a parse error. The outcome below is what matters. + } + if (failure) rejectPromise(failure) + else resolvePromise() } - }) - stream.on("end", () => resolvePromise()) - stream.on("error", () => rejectPromise(new Error("Extraction failed"))) + const advance = (): void => { + extractedEntries++ + if (totalEntries <= 0) return + // runExtraction emits the terminal 100 after validation and copying, so + // the running figure stops a point short of it. + const progress = Math.min(99, Math.floor((extractedEntries / totalEntries) * 100)) + if (progress > lastReportedProgress) { + lastReportedProgress = progress + onProgress?.(progress) + } + } + + zipFile.on("entry", (entry: yauzl.Entry) => { + let target: string + try { + target = resolveEntryDestination(destination, entry.fileName) + } catch (error) { + finish(error as Error) + return + } + + if (entry.fileName.endsWith("/")) { + try { + fse.ensureDirSync(target) + } catch { + finish(new Error("Extraction failed")) + return + } + advance() + zipFile.readEntry() + return + } + + zipFile.openReadStream(entry, (streamError, readStream) => { + if (streamError || !readStream) { + finish(new Error("Extraction failed")) + return + } + + entryReader = readStream + let writeStream: ReturnType + try { + fse.ensureDirSync(dirname(target)) + writeStream = createWriteStream(target) + } catch { + finish(new Error("Extraction failed")) + return + } + entryWriter = writeStream + + readStream.on("error", () => finish(new Error("Extraction failed"))) + writeStream.on("error", () => finish(new Error("Extraction failed"))) + writeStream.on("close", () => { + if (settled) return + advance() + zipFile.readEntry() + }) + readStream.pipe(writeStream) + }) + }) + + zipFile.on("end", () => finish()) + zipFile.on("error", () => finish(new Error("Extraction failed"))) + zipFile.readEntry() + }) }) } /** - * Unpacks a gzipped tar. - * - * 7-Zip is not used here. It needs two passes for a `.tar.gz`, and the bundled - * p7zip 16.02 cannot read the tar Vintage Story ships at all: its headers leave - * the numeric fields as NUL bytes, which GNU tar and node-tar accept and 7-Zip - * rejects with "Is not archive". + * Unpacks a gzipped tar: the game builds, and every backup the launcher writes. * * Progress comes from the compressed bytes read, which is the only total known * up front. Anything that is not a plain file or folder fails the extraction - * rather than being skipped quietly. + * rather than being skipped quietly, and `preservePaths: false` is what keeps + * an absolute or climbing entry name from being written where it points. + * + * `strict` is what makes a failed entry a failed extraction. Left off, node-tar + * treats every per-entry write error as a warning: the entry is skipped, the + * stream still closes cleanly, and a run that hit a full disk halfway through + * comes back indistinguishable from one that unpacked everything. A restore + * believes that and deletes the installation the truncated tree replaced, so + * the archive has to fail closed here rather than downstream. It also upgrades + * the parser's own invalid-entry warning, a corrupt header whose entry is + * dropped just as quietly. Nothing else it turns fatal is reachable: an + * unsupported entry type is refused by the filter below, and an absolute name + * never gets this far, validateArchive having refused the archive by name + * before a byte was written. */ -function extractTarGz(filePath: string, destination: string, onProgress?: (progress: number) => void): Promise { +export function extractTarGz(filePath: string, destination: string, onProgress?: (progress: number) => void): Promise { return new Promise((resolvePromise, rejectPromise) => { const totalBytes = fse.statSync(filePath).size let readBytes = 0 @@ -189,6 +325,7 @@ function extractTarGz(filePath: string, destination: string, onProgress?: (progr const unpacker = tar.extract({ cwd: destination, preservePaths: false, + strict: true, filter: (_entryPath, entry): boolean => { if (isSafeTarEntryType("type" in entry ? entry.type : undefined)) return true unsafeEntry = new Error("Archive contains an unsafe entry") @@ -220,8 +357,13 @@ export interface ExtractionOptions { outputPath: string /** Whether the archive is deleted once its contents landed. */ deleteArchive: boolean - /** 7-Zip binary, used for every format outside the tar.gz family. */ - sevenZipBin: string + /** + * Whether a single wrapping folder is stepped into before the contents are + * copied out. Asked for by the game version install, whose Linux archives + * carry everything under `vintagestory/`, and never by a backup restore, + * whose archive holds an installation's contents at the root already. + */ + unwrapSingleRootFolder?: boolean /** Called with 0 to 100 as the work advances. */ onProgress?: (progress: number) => void } @@ -234,14 +376,14 @@ export interface ExtractionOptions { * files and folders, or busts the entry and size bounds. */ export async function runExtraction(options: ExtractionOptions): Promise { - const { filePath, outputPath, deleteArchive, sevenZipBin, onProgress } = options + const { filePath, outputPath, deleteArchive, unwrapSingleRootFolder = false, onProgress } = options let temporaryRoot: string | undefined // The first of two validation gates (see archiveValidation.ts's own comment): reads the // archive's table of contents and refuses it, before a single byte is written anywhere, // if it names an entry outside its root, repeats a name, carries a link, or busts the // entry/size bounds. - await validateArchive(filePath, sevenZipBin) + await validateArchive(filePath) try { assertNoSymlinkComponents(outputPath) @@ -252,15 +394,14 @@ export async function runExtraction(options: ExtractionOptions): Promise { const extractionRoot = join(temporaryRoot, "payload") fse.ensureDirSync(extractionRoot) - const isGameArchive = isTarGzName(filePath) - if (isGameArchive) await extractTarGz(filePath, extractionRoot, onProgress) - else await extractWithSevenZip(filePath, extractionRoot, sevenZipBin, onProgress) + if (isTarGzName(filePath)) await extractTarGz(filePath, extractionRoot, onProgress) + else await extractZip(filePath, extractionRoot, onProgress) validateTree(extractionRoot) - // Only the game archives wrap their contents in a folder. The zips reaching - // here are backups the launcher wrote itself, holding an installation's + // Only the game archives wrap their contents in a folder, and only their caller + // asks for that folder to be stepped into. A backup holds an installation's // contents at the root, and a restore has to put them back exactly as they were. - copyTree(isGameArchive ? contentRoot(extractionRoot) : extractionRoot, outputPath) + copyTree(unwrapSingleRootFolder ? contentRoot(extractionRoot) : extractionRoot, outputPath) if (deleteArchive) { assertNoSymlinkComponents(filePath) diff --git a/src/ipc/workers/workerHost.ts b/src/ipc/workers/workerHost.ts index f7abc22d..50fa55a4 100644 --- a/src/ipc/workers/workerHost.ts +++ b/src/ipc/workers/workerHost.ts @@ -77,9 +77,9 @@ export function serveTasks(handler: TaskHandler, describeFailure: FailureDescrib } const onProgress: ProgressReporter = (progress) => { - // node-7z and the stream readers this app uses can emit one last progress event - // after their own end event. Posting it was harmless while the worker died with - // its task right after; now the worker can already be sitting idle in the pool. + // The stream readers this app uses can emit one last progress event after their + // own end event. Posting it was harmless while the worker died with its task + // right after; now the worker can already be sitting idle in the pool. if (settled) return port.postMessage({ type: "progress", token, progress }) } diff --git a/src/preload/index.ts b/src/preload/index.ts index 47993927..7437d6e5 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -54,8 +54,8 @@ const api: BridgeAPI = { ensurePathExists: (path: string): Promise => ipcRenderer.invoke(IPC_CHANNELS.PATHS_MANAGER.ENSURE_PATH_EXISTS, path), openPathOnFileExplorer: (path: string): Promise => ipcRenderer.invoke(IPC_CHANNELS.PATHS_MANAGER.OPEN_PATH_ON_FILE_EXPLORER, path), downloadOnPath: (id: string, url: string, outputPath: string, fileName: string): Promise => ipcRenderer.invoke(IPC_CHANNELS.PATHS_MANAGER.DOWNLOAD_ON_PATH, id, url, outputPath, fileName), - extractOnPath: (id: string, filePath: string, outputPath: string, deleteZip: boolean): Promise => - ipcRenderer.invoke(IPC_CHANNELS.PATHS_MANAGER.EXTRACT_ON_PATH, id, filePath, outputPath, deleteZip), + extractOnPath: (id: string, filePath: string, outputPath: string, deleteZip: boolean, unwrapSingleRootFolder?: boolean): Promise => + ipcRenderer.invoke(IPC_CHANNELS.PATHS_MANAGER.EXTRACT_ON_PATH, id, filePath, outputPath, deleteZip, unwrapSingleRootFolder ?? false), runInstaller: (id: string, filePath: string, outputPath: string, deleteInstaller: boolean): Promise => ipcRenderer.invoke(IPC_CHANNELS.PATHS_MANAGER.RUN_INSTALLER, id, filePath, outputPath, deleteInstaller), compressOnPath: (id: string, inputPath: string, outputPath: string, outputFileName: string, compressionLevel?: number): Promise => diff --git a/src/preload/preload.d.ts b/src/preload/preload.d.ts index 0b78b56b..d9c510a9 100644 --- a/src/preload/preload.d.ts +++ b/src/preload/preload.d.ts @@ -54,7 +54,7 @@ declare global { ensurePathExists: (path: string) => Promise openPathOnFileExplorer: (path: string) => Promise downloadOnPath: (id: string, url: string, outputPath: string, fileName: string) => Promise - extractOnPath: (id: string, filePath: string, outputPath: string, deleteZip: boolean) => Promise + extractOnPath: (id: string, filePath: string, outputPath: string, deleteZip: boolean, unwrapSingleRootFolder?: boolean) => Promise runInstaller: (id: string, filePath: string, outputPath: string, deleteInstaller: boolean) => Promise compressOnPath: (id: string, inputPath: string, outputPath: string, outputFileName: string, compressionLevel?: number) => Promise onDownloadProgress: (callback: ProgressCallback) => Unsubscribe diff --git a/src/renderer/src/contexts/TaskManagerContext.tsx b/src/renderer/src/contexts/TaskManagerContext.tsx index 4b7bdd33..1bca1451 100644 --- a/src/renderer/src/contexts/TaskManagerContext.tsx +++ b/src/renderer/src/contexts/TaskManagerContext.tsx @@ -120,7 +120,8 @@ export interface TaskContextType { filePath: string, outputPath: string, deleteZip: boolean, - onFinish: (status: boolean, error: Error | null) => void + onFinish: (status: boolean, error: Error | null) => void, + unwrapSingleRootFolder?: boolean ): Promise startInstall( name: string, @@ -270,7 +271,8 @@ export const TaskProvider = ({ children }: { children: React.ReactNode }): JSX.E filePath: string, outputPath: string, deleteZip: boolean, - onFinish: (status: boolean, error: Error | null) => void + onFinish: (status: boolean, error: Error | null) => void, + unwrapSingleRootFolder = false ): Promise { const id = crypto.randomUUID() @@ -281,7 +283,7 @@ export const TaskProvider = ({ children }: { children: React.ReactNode }): JSX.E window.api.utils.logMessage("info", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startExtract] [${id}] [${filePath}] Extracting...`) if (showsStart(notifications)) addNotification(t("notifications.body.extracting", { extractName: name }), "info") - const result = await window.api.pathsManager.extractOnPath(id, filePath, outputPath, deleteZip) + const result = await window.api.pathsManager.extractOnPath(id, filePath, outputPath, deleteZip, unwrapSingleRootFolder) if (!result) throw new Error("Extraction failed") diff --git a/src/renderer/src/features/versions/adapters/install.ts b/src/renderer/src/features/versions/adapters/install.ts index 06e66d37..14bb5095 100644 --- a/src/renderer/src/features/versions/adapters/install.ts +++ b/src/renderer/src/features/versions/adapters/install.ts @@ -38,8 +38,11 @@ export function createInstallPorts({ startDownload, startExtract, startInstall, ) }, unpacker: { + // The Linux game archives carry everything under a single `vintagestory/` + // folder, which is the one case the extraction steps into. A backup restore + // asks for the opposite and gets the default. extractArchive: (request, onComplete) => - startExtract(taskName, unpackDescription, "progress", request.sourcePath, request.outputFolder, true, (status, error) => onComplete({ ok: status, error: error?.message })), + startExtract(taskName, unpackDescription, "progress", request.sourcePath, request.outputFolder, true, (status, error) => onComplete({ ok: status, error: error?.message }), true), runInstaller: (request, onComplete) => startInstall(taskName, unpackDescription, "progress", request.sourcePath, request.outputFolder, true, (status, error) => onComplete({ ok: status, error: error?.message })) } diff --git a/tests/domain/installations/backup.test.ts b/tests/domain/installations/backup.test.ts index 7631484e..5de9b7b6 100644 --- a/tests/domain/installations/backup.test.ts +++ b/tests/domain/installations/backup.test.ts @@ -66,7 +66,7 @@ function fakePorts(overrides: Partial = {}): MakeIn } function backup(id: string, overrides: { isDeleting?: boolean; isRestoring?: boolean } = {}): BackupRecord & { isDeleting?: boolean; isRestoring?: boolean } { - return { id, date: 1, path: `/backups/${id}.zip`, ...overrides } + return { id, date: 1, path: `/backups/${id}.tar.gz`, ...overrides } } function snapshot(overrides: Partial = {}): InstallationSnapshot { @@ -163,7 +163,7 @@ describe("makeInstallationBackup pruning", () => { assert.deepEqual(result.deletedBackupIds, ["b3", "b2"]) assert.deepEqual( trace.filter((entry) => entry.startsWith("remove:") || entry.startsWith("deleted:")), - ["remove:/backups/b3.zip", "deleted:b3", "remove:/backups/b2.zip", "deleted:b2"] + ["remove:/backups/b3.tar.gz", "deleted:b3", "remove:/backups/b2.tar.gz", "deleted:b2"] ) }) @@ -181,7 +181,7 @@ describe("makeInstallationBackup pruning", () => { it("stops at the first failed deletion and reports the ones already done", async () => { const installation = snapshot({ backupsLimit: 1, backups: [backup("b1"), backup("b2"), backup("b3")] }) - const ports = fakePorts({ fileSystem: fakeFileSystem({ removals: { "/backups/b2.zip": false } }) }) + const ports = fakePorts({ fileSystem: fakeFileSystem({ removals: { "/backups/b2.tar.gz": false } }) }) const result = await makeInstallationBackup(ports, { installation, backupsFolder: "/backups" }, recordingEvents()) @@ -203,7 +203,7 @@ describe("makeInstallationBackup pruning", () => { assert.deepEqual(result.deletedBackupIds, ["b2"]) assert.deepEqual( trace.filter((entry) => entry.startsWith("remove:") || entry.startsWith("deleted:")), - ["remove:/backups/b2.zip", "deleted:b2"] + ["remove:/backups/b2.tar.gz", "deleted:b2"] ) }) @@ -229,7 +229,7 @@ describe("makeInstallationBackup pruning", () => { assert.deepEqual(result.deletedBackupIds, ["b4", "b2"]) assert.deepEqual( trace.filter((entry) => entry.startsWith("remove:") || entry.startsWith("deleted:")), - ["remove:/backups/b4.zip", "deleted:b4", "remove:/backups/b2.zip", "deleted:b2"] + ["remove:/backups/b4.tar.gz", "deleted:b4", "remove:/backups/b2.tar.gz", "deleted:b2"] ) }) @@ -242,7 +242,7 @@ describe("makeInstallationBackup pruning", () => { assert.deepEqual(result.deletedBackupIds, ["b2"]) assert.deepEqual( trace.filter((entry) => entry.startsWith("remove:") || entry.startsWith("deleted:")), - ["remove:/backups/b2.zip", "deleted:b2"] + ["remove:/backups/b2.tar.gz", "deleted:b2"] ) }) }) @@ -253,11 +253,11 @@ describe("makeInstallationBackup archiving", () => { const result = await makeInstallationBackup(fakePorts({ archiver }), { installation: snapshot(), backupsFolder: "/backups" }) - assert.equal(requests[0]?.fileName, "My-Install-Test_2025-08-15_23-20-00.zip") + assert.equal(requests[0]?.fileName, "My-Install-Test_2025-08-15_23-20-00.tar.gz") assert.equal(requests[0]?.outputFolder, "/backups/Installations/My-Install-Test") assert.equal(requests[0]?.sourcePath, "/games/my-install") assert.equal(requests[0]?.compressionLevel, 5) - assert.equal(result.ok === true && result.backup.path, "/backups/Installations/My-Install-Test/My-Install-Test_2025-08-15_23-20-00.zip") + assert.equal(result.ok === true && result.backup.path, "/backups/Installations/My-Install-Test/My-Install-Test_2025-08-15_23-20-00.tar.gz") }) it("falls back to a slice of the installation id when the cleaned name is empty", async () => { @@ -265,9 +265,9 @@ describe("makeInstallationBackup archiving", () => { const result = await makeInstallationBackup(fakePorts({ archiver }), { installation: snapshot({ id: "installation-1", name: "***" }), backupsFolder: "/backups" }) - assert.equal(requests[0]?.fileName, "installa_2025-08-15_23-20-00.zip") + assert.equal(requests[0]?.fileName, "installa_2025-08-15_23-20-00.tar.gz") assert.equal(requests[0]?.outputFolder, "/backups/Installations/installa") - assert.equal(result.ok === true && result.backup.path, "/backups/Installations/installa/installa_2025-08-15_23-20-00.zip") + assert.equal(result.ok === true && result.backup.path, "/backups/Installations/installa/installa_2025-08-15_23-20-00.tar.gz") }) it("stamps the record with the clock time and a generated id", async () => { @@ -276,7 +276,7 @@ describe("makeInstallationBackup archiving", () => { assert.deepEqual(result.ok === true && result.backup, { id: "generated-id-1", date: FIXED_NOW, - path: "/backups/Installations/My-Install-Test/My-Install-Test_2025-08-15_23-20-00.zip" + path: "/backups/Installations/My-Install-Test/My-Install-Test_2025-08-15_23-20-00.tar.gz" }) }) @@ -287,9 +287,9 @@ describe("makeInstallationBackup archiving", () => { "exists:/games/my-install", "guard-acquire:Making and installation backup.", "started", - "remove:/backups/b1.zip", + "remove:/backups/b1.tar.gz", "deleted:b1", - "compress:/backups/Installations/My-Install-Test/My-Install-Test_2025-08-15_23-20-00.zip", + "compress:/backups/Installations/My-Install-Test/My-Install-Test_2025-08-15_23-20-00.tar.gz", "guard-release", "finished" ]) diff --git a/tests/domain/installations/restore.test.ts b/tests/domain/installations/restore.test.ts index 6f8d5995..0d28a1ba 100644 --- a/tests/domain/installations/restore.test.ts +++ b/tests/domain/installations/restore.test.ts @@ -10,7 +10,7 @@ const TOKEN = "token-1" const INSTALLATION_PATH = "/games/my-install" const STAGING_PATH = `${INSTALLATION_PATH}-restoring-${TOKEN}` const REPLACED_PATH = `${INSTALLATION_PATH}-replaced-${TOKEN}` -const ARCHIVE_PATH = "/backups/my-install.zip" +const ARCHIVE_PATH = "/backups/my-install.tar.gz" /** Everything the fakes wrote down, in the order it happened. */ let trace: string[] = [] diff --git a/tests/fixtures/build-fixtures.ts b/tests/fixtures/build-fixtures.ts index 8868e10a..3f694db9 100644 --- a/tests/fixtures/build-fixtures.ts +++ b/tests/fixtures/build-fixtures.ts @@ -1,8 +1,12 @@ /** - * Hand-builds the tiny zip fixtures under tests/fixtures/ that - * tests/ipc/modScan.test.ts reads readModArchive against, so the yauzl edge - * cases in src/ipc/adapters/modScan.ts are exercised against real archive - * bytes instead of only through the domain's fakes. + * Hand-builds the tiny zip fixtures under tests/fixtures/. + * + * Most of them are what tests/ipc/modScan.test.ts reads readModArchive + * against, so the yauzl edge cases in src/ipc/adapters/modScan.ts are + * exercised against real archive bytes instead of only through the domain's + * fakes. The last three are backups: the launcher wrote its backups as zip up to + * 1.7.0-beta.4, and tests/ipc/extraction.test.ts restores them here so a + * player's old backup keeps working long after the writer went away. * * This script is not run by the test suite. It is the documentation of what * each fixture contains: run it by hand after changing it, @@ -359,3 +363,54 @@ write("zip64.zip", assembleZip64([{ name: "modinfo.json", method: METHOD_STORE, // receives the error (no End of Central Directory Record signature found), // before a single "entry" event, exercising readModArchive's openErr branch. write("not-a-zip.bin", Buffer.from("this is not a zip archive, just some bytes", "utf8")) + +// --- legacy-backup.zip -------------------------------------------------- +// A backup the launcher wrote before it moved to gzipped tar: an installation's +// contents at the root of the archive, no wrapping folder, with a folder entry +// and a zero byte file in it. tests/ipc/extraction.test.ts restores this one and +// compares the tree byte for byte. +write( + "legacy-backup.zip", + assembleZip([ + { name: "Vintagestory", method: METHOD_STORE, realBytes: Buffer.from("elf", "utf8") }, + { name: "assets/", method: METHOD_STORE, realBytes: Buffer.alloc(0) }, + { name: "assets/version-1.22.6.txt", method: METHOD_STORE, realBytes: Buffer.alloc(0) }, + { name: "Mods/", method: METHOD_STORE, realBytes: Buffer.alloc(0) }, + { name: "Mods/notes.txt", method: METHOD_DEFLATE, realBytes: Buffer.from("a mod list worth keeping\n", "utf8") } + ]) +) + +// --- hostile-backup.zip ------------------------------------------------- +// The same shape, with three entry names that point outside the folder they +// would be unpacked into: a Windows drive letter, a climb with "..", and a +// unix absolute path. None is a name any writer produces by accident, and +// nothing may be written for any of them. +// +// All three are refused by yauzl's own validateFileName, which rejects +// /^[a-zA-Z]:/ exactly as it rejects a leading "/" and any ".." segment, so the +// read stops at the first entry and the launcher reports an archive it could +// not read. Nothing here reaches isSafeArchiveEntry; unsafe-name-backup.zip +// below is the one that does. +write( + "hostile-backup.zip", + assembleZip([ + { name: "C:/escaped-drive.txt", method: METHOD_STORE, realBytes: Buffer.from("drive letter", "utf8") }, + { name: "../escaped.txt", method: METHOD_STORE, realBytes: Buffer.from("climbed out", "utf8") }, + { name: "/etc/escaped-absolute.txt", method: METHOD_STORE, realBytes: Buffer.from("absolute", "utf8") } + ]) +) + +// --- unsafe-name-backup.zip --------------------------------------------- +// A NUL byte in the middle of an entry name. yauzl's validateFileName has +// nothing to say about one, so this is the archive that reaches the launcher's +// own isSafeArchiveEntry gate in validateZipArchive, which every name in +// hostile-backup.zip is stopped short of. An ordinary entry comes first, so the +// refusal is that gate deciding rather than the archive being unreadable from +// its first byte. +write( + "unsafe-name-backup.zip", + assembleZip([ + { name: "Vintagestory", method: METHOD_STORE, realBytes: Buffer.from("elf", "utf8") }, + { name: "Mods/notes\u0000.txt", method: METHOD_STORE, realBytes: Buffer.from("a name no writer produces by accident", "utf8") } + ]) +) diff --git a/tests/fixtures/hostile-backup.zip b/tests/fixtures/hostile-backup.zip new file mode 100644 index 00000000..f8198a3d Binary files /dev/null and b/tests/fixtures/hostile-backup.zip differ diff --git a/tests/fixtures/legacy-backup.zip b/tests/fixtures/legacy-backup.zip new file mode 100644 index 00000000..1845ecdb Binary files /dev/null and b/tests/fixtures/legacy-backup.zip differ diff --git a/tests/fixtures/unsafe-name-backup.zip b/tests/fixtures/unsafe-name-backup.zip new file mode 100644 index 00000000..22aadc91 Binary files /dev/null and b/tests/fixtures/unsafe-name-backup.zip differ diff --git a/tests/ipc/archiveValidation.test.ts b/tests/ipc/archiveValidation.test.ts index cf0a030f..064ef277 100644 --- a/tests/ipc/archiveValidation.test.ts +++ b/tests/ipc/archiveValidation.test.ts @@ -1,15 +1,14 @@ import assert from "node:assert/strict" -import { execFileSync } from "node:child_process" import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" import { join } from "node:path" import { afterEach, beforeEach, describe, it } from "vitest" import * as tar from "tar" -import { path7za } from "7zip-bin" import { validateArchive } from "../../src/ipc/archiveValidation" -const sevenZipBin = path7za +/** Committed zips, built by tests/fixtures/build-fixtures.ts. */ +const FIXTURES = join(__dirname, "..", "fixtures") let workspace: string @@ -38,53 +37,53 @@ describe("validateArchive", () => { it("accepts a gzipped tar, which is what the Linux game builds ship as", async () => { const archivePath = await makeTarGz("vs_client_linux-x64_1.22.6.tar.gz", ["vintagestory"]) - await assert.doesNotReject(async () => validateArchive(archivePath, sevenZipBin)) + await assert.doesNotReject(async () => validateArchive(archivePath)) }) it("accepts the same archive under the .tgz spelling", async () => { const archivePath = await makeTarGz("build.tgz", ["vintagestory"]) - await assert.doesNotReject(async () => validateArchive(archivePath, sevenZipBin)) + await assert.doesNotReject(async () => validateArchive(archivePath)) }) it("refuses a gzipped tar carrying a symbolic link", async () => { symlinkSync("/etc/passwd", workspacePath("source", "vintagestory", "escape.txt")) const archivePath = await makeTarGz("hostile.tar.gz", ["vintagestory"]) - await assert.rejects(validateArchive(archivePath, sevenZipBin), /unsafe entry/) + await assert.rejects(validateArchive(archivePath), /unsafe entry/) }) it("refuses a gzipped tar naming an entry outside its root", async () => { const archivePath = workspacePath("escaping.tar.gz") await tar.create({ file: archivePath, gzip: true, cwd: workspacePath("source"), portable: true, preservePaths: true }, ["../source/vintagestory/Vintagestory"]) - await assert.rejects(validateArchive(archivePath, sevenZipBin), /unsafe entry/) + await assert.rejects(validateArchive(archivePath), /unsafe entry/) }) it("refuses a file that only claims to be a gzipped tar", async () => { writeFileSync(workspacePath("liar.tar.gz"), "definitely not gzip") - await assert.rejects(validateArchive(workspacePath("liar.tar.gz"), sevenZipBin), /could not be read/) + await assert.rejects(validateArchive(workspacePath("liar.tar.gz")), /could not be read/) }) - it("still reads a zip with the zip reader", async () => { - const archivePath = workspacePath("backup.zip") - execFileSync(sevenZipBin, ["a", "-tzip", archivePath, workspacePath("source", "vintagestory")], { stdio: "ignore" }) + it("still reads a zip with the zip reader, which is what an old backup is", async () => { + await assert.doesNotReject(async () => validateArchive(join(FIXTURES, "legacy-backup.zip"))) + }) - await assert.doesNotReject(async () => validateArchive(archivePath, sevenZipBin)) + it("refuses a zip whose entries name places outside its root", async () => { + await assert.rejects(validateArchive(join(FIXTURES, "hostile-backup.zip")), /could not be read/) }) it("refuses a zip that cannot be parsed", async () => { writeFileSync(workspacePath("broken.zip"), "PK not really") - await assert.rejects(validateArchive(workspacePath("broken.zip"), sevenZipBin), /could not be read/) + await assert.rejects(validateArchive(workspacePath("broken.zip")), /could not be read/) }) - it("falls back to 7-Zip for anything else", async () => { - const archivePath = workspacePath("backup.7z") - execFileSync(sevenZipBin, ["a", "-t7z", archivePath, workspacePath("source", "vintagestory")], { stdio: "ignore" }) - - await assert.doesNotReject(async () => validateArchive(archivePath, sevenZipBin)) - await assert.rejects(validateArchive(workspacePath("source", "vintagestory", "Vintagestory"), sevenZipBin), /could not be listed/) + it("refuses anything that is neither a zip nor a gzipped tar", async () => { + // Two formats reach the launcher and no others, so a third is refused by + // name rather than handed to a reader that would have to guess at it. + await assert.rejects(validateArchive(workspacePath("source", "vintagestory", "Vintagestory")), /not supported/) + await assert.rejects(validateArchive(join(FIXTURES, "not-a-zip.bin")), /not supported/) }) }) diff --git a/tests/ipc/compression.test.ts b/tests/ipc/compression.test.ts index 2f05c5ee..2d69da16 100644 --- a/tests/ipc/compression.test.ts +++ b/tests/ipc/compression.test.ts @@ -1,23 +1,22 @@ import assert from "node:assert/strict" -import { EventEmitter } from "node:events" -import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs" +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, statSync, symlinkSync, truncateSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" import { join } from "node:path" +import { gunzipSync } from "node:zlib" import { afterEach, beforeEach, describe, it } from "vitest" +import * as tar from "tar" -import { assertSafeCompressionTree, runCompression, type ArchiveAdd } from "@src/ipc/workers/compression" +import { assertSafeCompressionTree, runCompression } from "@src/ipc/workers/compression" /** - * The backup compression, driven without spawning 7-Zip. + * The backup compression, against real archives. * - * `runCompression` takes the `Seven.add` call as a parameter, so these pin two - * separate things: the safety walk over the real source tree, which runs before - * 7-Zip is handed anything, and the invocation and message protocol, which is - * asserted on the recorded arguments instead of on an archive. + * Two separate things are pinned here: the safety walk over the source tree, + * which runs before tar is handed anything, and what the archive turns out to + * hold once it is written. * - * The walk is the half worth being fussy about. 7-Zip follows a symbolic link - * into whatever it points at, so a link inside a backup source would quietly - * pull files from outside the folder into the archive. + * The walk is the half worth being fussy about. A symbolic link inside a backup + * source would otherwise pull files from outside the folder into the archive. */ let workspace: string @@ -28,29 +27,16 @@ function workspacePath(...parts: string[]): string { return join(workspace, ...parts) } -interface Invocation { - archivePath: string - source: string - options: Record -} - -/** A stand-in for `Seven.add` that records its call and replays a scripted event sequence. */ -function fakeAdd(script: (stream: EventEmitter) => void): { add: ArchiveAdd; invocations: Invocation[] } { - const invocations: Invocation[] = [] - - const add: ArchiveAdd = (archivePath, sourceGlob, options) => { - invocations.push({ archivePath, source: sourceGlob, options: options as unknown as Record }) - const stream = new EventEmitter() - setImmediate(() => script(stream)) - return stream - } - - return { add, invocations } +/** Every entry name a written archive holds, sorted. */ +async function archiveEntryNames(archivePath: string): Promise { + const names: string[] = [] + await tar.list({ file: archivePath, onReadEntry: (entry) => void names.push(entry.path) }) + return names.sort() } const FAKE_ROOT = "/fake/root" -type FakeStats = { isSymbolicLink(): boolean; isDirectory(): boolean; isFile(): boolean } +type FakeStats = { isSymbolicLink(): boolean; isDirectory(): boolean; isFile(): boolean; size: number } type TreeFileSystem = Parameters[1] /** A filesystem that answers from the two functions given, for trees too large or too odd to build. */ @@ -63,15 +49,8 @@ function fakeStats(kind: "directory" | "file" | "other"): FakeStats { return { isSymbolicLink: (): boolean => false, isDirectory: (): boolean => kind === "directory", - isFile: (): boolean => kind === "file" - } -} - -/** Scripts a run that reports the given percentages and then ends. */ -function succeedsAt(...percentages: unknown[]): (stream: EventEmitter) => void { - return (stream) => { - for (const percent of percentages) stream.emit("progress", { percent }) - stream.emit("end") + isFile: (): boolean => kind === "file", + size: 0 } } @@ -91,8 +70,8 @@ afterEach(() => { }) describe("assertSafeCompressionTree", () => { - it("accepts a tree of plain files and folders", () => { - assert.doesNotThrow(() => assertSafeCompressionTree(source)) + it("accepts a tree of plain files and folders and totals their bytes", () => { + assert.equal(assertSafeCompressionTree(source), "elf".length + "1.22.6".length) }) it("refuses a symbolic link anywhere in the tree", () => { @@ -140,143 +119,155 @@ describe("assertSafeCompressionTree", () => { describe("runCompression", () => { it("archives the source contents, not the source folder itself", async () => { - const seven = fakeAdd(succeedsAt(30, 80)) + await runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.tar.gz" }) - await runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.zip", addArchive: seven.add }) - - assert.equal(seven.invocations.length, 1) - assert.equal(seven.invocations[0]?.archivePath, join(output, "backup.zip")) - // The trailing glob is what keeps the wrapping folder out of the archive, - // so a restore puts the files back where they came from. - assert.equal(seven.invocations[0]?.source, join(source, "*")) + // No "installation/" prefix anywhere: the wrapping folder is what a restore + // would otherwise put back one level too deep. + assert.deepEqual(await archiveEntryNames(join(output, "backup.tar.gz")), ["Vintagestory", "assets/", "assets/version.txt"]) }) - it("passes the compression level and the binary 7-Zip is asked for", async () => { - const seven = fakeAdd(succeedsAt()) - - await runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.zip", compressionLevel: 4, sevenZipBin: "/opt/7za", addArchive: seven.add }) + it("writes a gzip stream, which is what the restore reader expects", async () => { + await runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.tar.gz" }) - assert.deepEqual(seven.invocations[0]?.options, { $bin: "/opt/7za", $progress: true, recursive: true, method: ["x=4", "mt=on"] }) + const bytes = readFileSync(join(output, "backup.tar.gz")) + assert.deepEqual([bytes[0], bytes[1]], [0x1f, 0x8b]) + assert.equal(gunzipSync(bytes).length % 512, 0) }) - it("defaults to level 6 when the caller names none", async () => { - const seven = fakeAdd(succeedsAt()) - - await runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.zip", addArchive: seven.add }) - - assert.deepEqual(seven.invocations[0]?.options.method, ["x=6", "mt=on"]) - }) + it("honours the compression level it is given", async () => { + const compressible = workspacePath("compressible") + mkdirSync(compressible) + writeFileSync(join(compressible, "log.txt"), "the same sentence over and over. ".repeat(4_000)) - it("reports progress and always ends at 100", async () => { - const progress: number[] = [] - const seven = fakeAdd(succeedsAt(10, 55, 90)) + await runCompression({ inputPath: compressible, outputPath: output, outputFileName: "fastest.tar.gz", compressionLevel: 1 }) + await runCompression({ inputPath: compressible, outputPath: output, outputFileName: "smallest.tar.gz", compressionLevel: 9 }) - await runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.zip", addArchive: seven.add, onProgress: (value) => progress.push(value) }) + // Level 0 stores rather than deflates, so it is the one that says outright + // that the number reaches zlib at all instead of being quietly dropped. + await runCompression({ inputPath: compressible, outputPath: output, outputFileName: "stored.tar.gz", compressionLevel: 0 }) - assert.deepEqual(progress, [10, 55, 90, 100]) + const stored = statSync(join(output, "stored.tar.gz")).size + const fastest = statSync(join(output, "fastest.tar.gz")).size + const smallest = statSync(join(output, "smallest.tar.gz")).size + assert.equal(smallest <= fastest, true) + assert.equal(fastest < stored, true) }) - it("coalesces repeated percentages and reports completion once", async () => { + it("reports progress and always ends at 100, once", async () => { + const many = workspacePath("many-files") + mkdirSync(many) + for (let index = 0; index < 200; index++) writeFileSync(join(many, `file-${index}.bin`), Buffer.alloc(4_096, index % 251)) const progress: number[] = [] - const seven = fakeAdd(succeedsAt(0, 0, 10, 10, 50, 50, 100, 100)) - await runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.zip", addArchive: seven.add, onProgress: (value) => progress.push(value) }) + await runCompression({ inputPath: many, outputPath: output, outputFileName: "progress.tar.gz", onProgress: (value) => progress.push(value) }) - assert.deepEqual(progress, [10, 50, 100]) + assert.equal(progress.at(-1), 100) + assert.equal(progress.filter((value) => value === 100).length, 1) + assert.equal(new Set(progress).size, progress.length) + assert.deepEqual( + [...progress].sort((left, right) => left - right), + progress + ) + assert.equal( + progress.some((value) => value > 0 && value < 100), + true + ) }) - it("ignores a progress report that goes backwards, past 100, or is not a number", async () => { + it("reports the terminal 100 even for a source with nothing in it", async () => { + const empty = workspacePath("empty") + mkdirSync(empty) const progress: number[] = [] - const seven = fakeAdd(succeedsAt(40, 20, 140, "not a number", undefined, 60)) - await runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.zip", addArchive: seven.add, onProgress: (value) => progress.push(value) }) + await runCompression({ inputPath: empty, outputPath: output, outputFileName: "empty.tar.gz", onProgress: (value) => progress.push(value) }) - assert.deepEqual(progress, [40, 60, 100]) - }) - - it("fails when 7-Zip errors", async () => { - const seven = fakeAdd((stream) => stream.emit("error", new Error("7-Zip exited with code 2"))) - - await assert.rejects(runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.zip", addArchive: seven.add }), /Compression failed/) + assert.deepEqual(progress, [100]) }) it("creates the destination folder when it is missing", async () => { - const seven = fakeAdd(succeedsAt()) const missing = workspacePath("backups", "nested", "deeper") - await runCompression({ inputPath: source, outputPath: missing, outputFileName: "backup.zip", addArchive: seven.add }) + await runCompression({ inputPath: source, outputPath: missing, outputFileName: "backup.tar.gz" }) - assert.equal(seven.invocations[0]?.archivePath, join(missing, "backup.zip")) + assert.equal(statSync(join(missing, "backup.tar.gz")).isFile(), true) }) - it("refuses a source holding a symbolic link, without invoking 7-Zip", async () => { + it("refuses a source holding a symbolic link, without writing an archive", async () => { symlinkSync(workspacePath("backups"), join(source, "elsewhere")) - const seven = fakeAdd(succeedsAt()) - await assert.rejects(runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.zip", addArchive: seven.add }), /unsafe filesystem entry/) + await assert.rejects(runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.tar.gz" }), /unsafe filesystem entry/) - assert.deepEqual(seven.invocations, []) + assert.equal(statSync(output).isDirectory(), true) + assert.throws(() => statSync(join(output, "backup.tar.gz"))) }) it("refuses a source that is a file rather than a folder", async () => { - const seven = fakeAdd(succeedsAt()) - - await assert.rejects(runCompression({ inputPath: join(source, "Vintagestory"), outputPath: output, outputFileName: "backup.zip", addArchive: seven.add }), /must be a directory/) - - assert.deepEqual(seven.invocations, []) + await assert.rejects(runCompression({ inputPath: join(source, "Vintagestory"), outputPath: output, outputFileName: "backup.tar.gz" }), /must be a directory/) }) it("refuses a source that does not exist", async () => { - const seven = fakeAdd(succeedsAt()) - - await assert.rejects(runCompression({ inputPath: workspacePath("gone"), outputPath: output, outputFileName: "backup.zip", addArchive: seven.add })) - - assert.deepEqual(seven.invocations, []) + await assert.rejects(runCompression({ inputPath: workspacePath("gone"), outputPath: output, outputFileName: "backup.tar.gz" })) }) it("refuses a destination that is a symbolic link", async () => { const linked = workspacePath("linked-backups") symlinkSync(output, linked) - const seven = fakeAdd(succeedsAt()) - - await assert.rejects(runCompression({ inputPath: source, outputPath: linked, outputFileName: "backup.zip", addArchive: seven.add }), /destination is unsafe/) - assert.deepEqual(seven.invocations, []) + await assert.rejects(runCompression({ inputPath: source, outputPath: linked, outputFileName: "backup.tar.gz" }), /destination is unsafe/) }) it("refuses a destination that is a file", async () => { const asFile = workspacePath("not-a-folder") writeFileSync(asFile, "") - const seven = fakeAdd(succeedsAt()) - await assert.rejects(runCompression({ inputPath: source, outputPath: asFile, outputFileName: "backup.zip", addArchive: seven.add }), /destination is unsafe/) + await assert.rejects(runCompression({ inputPath: source, outputPath: asFile, outputFileName: "backup.tar.gz" }), /destination is unsafe/) }) it("refuses to write over a folder standing where the archive would go", async () => { - mkdirSync(join(output, "backup.zip")) - const seven = fakeAdd(succeedsAt()) + mkdirSync(join(output, "backup.tar.gz")) - await assert.rejects(runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.zip", addArchive: seven.add }), /archive target is unsafe/) - - assert.deepEqual(seven.invocations, []) + await assert.rejects(runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.tar.gz" }), /archive target is unsafe/) }) it("refuses to write through a symbolic link standing where the archive would go", async () => { writeFileSync(workspacePath("someone-elses-file"), "") - symlinkSync(workspacePath("someone-elses-file"), join(output, "backup.zip")) - const seven = fakeAdd(succeedsAt()) + symlinkSync(workspacePath("someone-elses-file"), join(output, "backup.tar.gz")) + + await assert.rejects(runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.tar.gz" }), /archive target is unsafe/) + assert.equal(readFileSync(workspacePath("someone-elses-file"), "utf8"), "") + }) + + it.skipIf(process.platform === "win32")("refuses a source past the total the restore reader will accept", async () => { + // A sparse file: 3 GiB by every stat the walk makes, no blocks on disk. The + // point is the size the reader would read back, and that is what stat says. + const huge = join(source, "world.vcdbs") + writeFileSync(huge, "") + truncateSync(huge, 3 * 1024 * 1024 * 1024) + + await assert.rejects(runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.tar.gz" }), /too large/) + + // Refused before anything was written, rather than after gigabytes of work. + assert.deepEqual(readdirSync(output), []) + }) + + it.skipIf(process.platform !== "linux" || process.getuid?.() === 0)("takes the half written archive away when the write fails", async () => { + // A file the safety walk can stat but tar cannot read, so the failure lands + // after tar has already created the archive and written the first bytes. + chmodSync(join(source, "Vintagestory"), 0o000) - await assert.rejects(runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.zip", addArchive: seven.add }), /archive target is unsafe/) + await assert.rejects(runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.tar.gz" }), /Compression failed/) - assert.deepEqual(seven.invocations, []) + // A failed backup leaves no record behind, and pruning only ever walks the + // records, so anything left here would stay for good and a retry would add + // one more beside it. + assert.deepEqual(readdirSync(output), []) }) it("overwrites a plain archive file already sitting there", async () => { - writeFileSync(join(output, "backup.zip"), "the previous backup") - const seven = fakeAdd(succeedsAt()) + writeFileSync(join(output, "backup.tar.gz"), "the previous backup") - await runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.zip", addArchive: seven.add }) + await runCompression({ inputPath: source, outputPath: output, outputFileName: "backup.tar.gz" }) - assert.equal(seven.invocations.length, 1) + assert.deepEqual(await archiveEntryNames(join(output, "backup.tar.gz")), ["Vintagestory", "assets/", "assets/version.txt"]) }) }) diff --git a/tests/ipc/extraction.test.ts b/tests/ipc/extraction.test.ts index ee8f0536..6d88e68e 100644 --- a/tests/ipc/extraction.test.ts +++ b/tests/ipc/extraction.test.ts @@ -1,22 +1,27 @@ import assert from "node:assert/strict" -import { execFileSync } from "node:child_process" -import { existsSync, linkSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, statSync, symlinkSync, writeFileSync } from "node:fs" +import { existsSync, linkSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, renameSync, rmSync, statSync, symlinkSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" -import { join } from "node:path" +import { join, relative, resolve } from "node:path" import { afterEach, beforeEach, describe, it, vi } from "vitest" import * as tar from "tar" -import { path7za } from "7zip-bin" -import { contentRoot, runExtraction, validateTree } from "../../src/ipc/workers/extraction" +import { contentRoot, extractTarGz, extractZip, resolveEntryDestination, runExtraction, validateTree } from "../../src/ipc/workers/extraction" +import { runCompression } from "../../src/ipc/workers/compression" +import { restoreInstallationBackup } from "../../src/domain/installations/restore" +import type { Extractor, FileSystem } from "../../src/domain/ports" /** - * These build their own archives, so they run anywhere without a network. + * These build their own archives, so they run anywhere without a network. The + * two zip fixtures are the exception, and they are committed rather than built: + * the whole point of keeping a zip reader is that backups written by a version + * of the launcher nobody runs any more still restore. See + * tests/fixtures/build-fixtures.ts for what is in them. * * The one test that needs a real Vintage Story archive is opt in: point * RIFT_E2E_ARCHIVE at a downloaded game tar.gz to run it. CI never does. */ -const sevenZipBin = path7za +const FIXTURES = join(__dirname, "..", "fixtures") let workspace: string @@ -123,7 +128,7 @@ describe("runExtraction on a gzipped tar", () => { writeTree(workspacePath("source"), { vintagestory: { Vintagestory: "elf", assets: { "version-1.22.6.txt": "", "seed.json": "{}" } } }) const archivePath = await makeTarGz("vs_client_linux-x64_1.22.6.tar.gz", workspacePath("source")) - await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, sevenZipBin }) + await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, unwrapSingleRootFolder: true }) // The listing is what says the wrapping "vintagestory" folder was stepped // into rather than copied along: it names every entry, so an extra folder @@ -138,7 +143,7 @@ describe("runExtraction on a gzipped tar", () => { writeTree(workspacePath("source"), { vintagestory: { Vintagestory: "elf", assets: { "version-1.22.6.txt": "" } } }) const archivePath = await makeTarGz("wrapped.tar.gz", workspacePath("source")) - await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, sevenZipBin }) + await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, unwrapSingleRootFolder: true }) const marker = workspacePath("target", "assets", "version-1.22.6.txt") assert.equal(existsSync(marker), true) @@ -149,7 +154,7 @@ describe("runExtraction on a gzipped tar", () => { writeTree(workspacePath("source"), { VintagestoryServer: "elf", assets: { "version-1.22.6.txt": "" } }) const archivePath = await makeTarGz("vs_server_linux-x64_1.22.6.tar.gz", workspacePath("source")) - await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, sevenZipBin }) + await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, unwrapSingleRootFolder: true }) assert.deepEqual(readdirSync(workspacePath("target")).sort(), ["VintagestoryServer", "assets"]) assert.equal(existsSync(workspacePath("target", "assets", "version-1.22.6.txt")), true) @@ -160,7 +165,7 @@ describe("runExtraction on a gzipped tar", () => { const archivePath = await makeTarGz("progress.tar.gz", workspacePath("source")) const reported: number[] = [] - await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, sevenZipBin, onProgress: (progress) => reported.push(progress) }) + await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, unwrapSingleRootFolder: true, onProgress: (progress) => reported.push(progress) }) assert.equal(reported.at(-1), 100) assert.equal(reported.filter((progress) => progress === 100).length, 1) @@ -176,8 +181,8 @@ describe("runExtraction on a gzipped tar", () => { const keptPath = await makeTarGz("kept.tar.gz", workspacePath("source")) const consumedPath = await makeTarGz("consumed.tar.gz", workspacePath("source")) - await runExtraction({ filePath: keptPath, outputPath: workspacePath("kept"), deleteArchive: false, sevenZipBin }) - await runExtraction({ filePath: consumedPath, outputPath: workspacePath("consumed"), deleteArchive: true, sevenZipBin }) + await runExtraction({ filePath: keptPath, outputPath: workspacePath("kept"), deleteArchive: false, unwrapSingleRootFolder: true }) + await runExtraction({ filePath: consumedPath, outputPath: workspacePath("consumed"), deleteArchive: true, unwrapSingleRootFolder: true }) assert.equal(existsSync(keptPath), true) assert.equal(existsSync(consumedPath), false) @@ -188,7 +193,7 @@ describe("runExtraction on a gzipped tar", () => { symlinkSync("/etc/passwd", workspacePath("source", "vintagestory", "escape.txt")) const archivePath = await makeTarGz("hostile.tar.gz", workspacePath("source")) - await assert.rejects(runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, sevenZipBin }), /unsafe entry/) + await assert.rejects(runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, unwrapSingleRootFolder: true }), /unsafe entry/) assert.equal(existsSync(workspacePath("target", "escape.txt")), false) assert.equal(existsSync(workspacePath("target", "Vintagestory")), false) }) @@ -199,7 +204,10 @@ describe("runExtraction on a gzipped tar", () => { // Caught by runExtraction's own pre-extraction validateArchive call, before it even // creates the target directory, let alone extracts (and its generic "Extraction failed") // is ever reached. - await assert.rejects(runExtraction({ filePath: workspacePath("broken.tar.gz"), outputPath: workspacePath("target"), deleteArchive: false, sevenZipBin }), /Archive could not be read/) + await assert.rejects( + runExtraction({ filePath: workspacePath("broken.tar.gz"), outputPath: workspacePath("target"), deleteArchive: false, unwrapSingleRootFolder: true }), + /Archive could not be read/ + ) assert.equal(existsSync(workspacePath("target")), false) }) @@ -215,48 +223,329 @@ describe("runExtraction on a gzipped tar", () => { writeTree(workspacePath("source"), { vintagestory: { Vintagestory: "elf" } }) const archivePath = await makeTarGz("clean.tar.gz", workspacePath("source")) - await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, sevenZipBin }) + await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, unwrapSingleRootFolder: true }) assert.deepEqual(readdirSync(temporaryRoot), []) }) }) -describe("runExtraction on a zip", () => { - it("keeps a zip's own shape, because those are the backups a restore puts back", async () => { - writeTree(workspacePath("source"), { vintagestory: { Vintagestory: "elf", assets: { "version-1.22.6.txt": "" } } }) - const archivePath = workspacePath("backup.zip") - execFileSync(sevenZipBin, ["a", "-tzip", archivePath, join(workspacePath("source"), "vintagestory")], { stdio: "ignore" }) +/** Every file in a tree, as a relative path to its bytes, so two trees can be compared outright. */ +function readTree(root: string): Record { + const files: Record = {} + const visit = (current: string): void => { + for (const name of readdirSync(current)) { + const entry = join(current, name) + if (lstatSync(entry).isDirectory()) visit(entry) + else files[relative(root, entry).replaceAll("\\", "/")] = readFileSync(entry, "base64") + } + } + visit(root) + return files +} + +describe("backup round trip", () => { + it("puts a freshly written backup back exactly as it was", async () => { + writeTree(workspacePath("installation"), { + Mods: { "carrycapacity.zip": "not really a mod", "notes.txt": "" }, + "clientsettings.json": "{}", + Saves: { "world.vcdbs": " binary-ish" } + }) + const before = readTree(workspacePath("installation")) + + await runCompression({ inputPath: workspacePath("installation"), outputPath: workspacePath("backups"), outputFileName: "backup.tar.gz" }) + await runExtraction({ filePath: workspacePath("backups", "backup.tar.gz"), outputPath: workspacePath("restored"), deleteArchive: false }) + + assert.deepEqual(readTree(workspacePath("restored")), before) + assert.deepEqual(readdirSync(workspacePath("restored")).sort(), readdirSync(workspacePath("installation")).sort()) + }) + + it("puts two names for one inode back as two ordinary files", async () => { + // A player does not have to have made a hard link to have one: a + // deduplicating filesystem hands them out on its own, and a mod copied twice + // can end up as two names over one inode. Left alone, tar writes the second + // name as a Link entry, the restore reader refuses Link, and the backup + // reports success while being impossible to put back. + writeTree(workspacePath("installation"), { Mods: { "carrycapacity.zip": "not really a mod" }, "clientsettings.json": "{}" }) + const firstName = workspacePath("installation", "Mods", "carrycapacity.zip") + const secondName = workspacePath("installation", "Mods", "carrycapacity-1.0.0.zip") + linkSync(firstName, secondName) + // The fixture is only worth anything if the two names really are one file. + assert.equal(lstatSync(firstName).ino, lstatSync(secondName).ino) + assert.equal(lstatSync(firstName).nlink, 2) + const before = readTree(workspacePath("installation")) + await runCompression({ inputPath: workspacePath("installation"), outputPath: workspacePath("backups"), outputFileName: "backup.tar.gz" }) + + // Nothing in the archive asks the reader to link one name to another: the + // second name carries its own bytes, which is what makes the backup + // self-contained and the restore below possible at all. + const archived: { path: string; type: string; size: number }[] = [] + await tar.list({ file: workspacePath("backups", "backup.tar.gz"), onReadEntry: (entry) => void archived.push({ path: entry.path, type: entry.type, size: entry.size }) }) + assert.deepEqual( + archived.filter((entry) => entry.type !== "File" && entry.type !== "Directory"), + [] + ) + assert.deepEqual( + archived.filter((entry) => entry.path.startsWith("Mods/carrycapacity")).map((entry) => entry.size), + [16, 16] + ) + + await runExtraction({ filePath: workspacePath("backups", "backup.tar.gz"), outputPath: workspacePath("restored"), deleteArchive: false }) + + assert.deepEqual(readTree(workspacePath("restored")), before) + // Independent copies, not the sharing put back: the launcher only promises + // the bytes, and the extraction's own tree check refuses a hard link anyway. + const restoredFirst = lstatSync(workspacePath("restored", "Mods", "carrycapacity.zip")) + const restoredSecond = lstatSync(workspacePath("restored", "Mods", "carrycapacity-1.0.0.zip")) + assert.equal(restoredFirst.nlink, 1) + assert.equal(restoredSecond.nlink, 1) + assert.notEqual(restoredFirst.ino, restoredSecond.ino) + }) + + it("puts a backup of an empty installation back as an empty folder", async () => { + mkdirSync(workspacePath("installation"), { recursive: true }) + + await runCompression({ inputPath: workspacePath("installation"), outputPath: workspacePath("backups"), outputFileName: "backup.tar.gz" }) + await runExtraction({ filePath: workspacePath("backups", "backup.tar.gz"), outputPath: workspacePath("restored"), deleteArchive: false }) + + assert.deepEqual(readdirSync(workspacePath("restored")), []) + }) + + it("never steps into a single folder a backup happens to hold, the way a game archive is stepped into", async () => { + // An installation whose only entry is a folder is not a wrapped archive: it + // is an installation with one folder in it, and a restore has to put that + // folder back rather than spill its contents over the installation root. + writeTree(workspacePath("installation"), { Mods: { "notes.txt": "one folder, nothing else" } }) + + await runCompression({ inputPath: workspacePath("installation"), outputPath: workspacePath("backups"), outputFileName: "backup.tar.gz" }) + await runExtraction({ filePath: workspacePath("backups", "backup.tar.gz"), outputPath: workspacePath("restored"), deleteArchive: false }) + + assert.deepEqual(readdirSync(workspacePath("restored")), ["Mods"]) + assert.equal(readFileSync(workspacePath("restored", "Mods", "notes.txt"), "utf8"), "one folder, nothing else") + }) +}) + +/** + * An archive whose second entry cannot be written, without needing a full disk + * or a permission the test runner may not be able to drop: "a" is a plain file, + * and "a/b" then asks for "a" to be a folder. The unpacker fails that one entry + * and carries on to the next, which is how a disk that fills partway through a + * multi-GB restore fails too, one entry at a time. + */ +async function makeConflictingTarGz(archiveName: string): Promise { + writeTree(workspacePath("conflict"), { a: "the file standing in the way", elsewhere: { b: "never lands" } }) + const archivePath = workspacePath(archiveName) + await tar.create( + { + file: archivePath, + cwd: workspacePath("conflict"), + gzip: true, + portable: true, + onWriteEntry: (entry) => { + if (entry.path === "elsewhere/b") entry.path = "a/b" + } + }, + ["a", "elsewhere/b"] + ) + return archivePath +} + +describe("a gzipped tar whose entries fail to land", () => { + it("fails the extraction rather than reporting a truncated tree as a whole one", async () => { + const archivePath = await makeConflictingTarGz("half-lands.tar.gz") + + await assert.rejects(runExtraction({ filePath: archivePath, outputPath: workspacePath("restored"), deleteArchive: false }), /Extraction failed/) + + // Nothing was copied out of the temporary folder, so the destination is as + // empty as it was: a partial tree is not a restore. + assert.deepEqual(readdirSync(workspacePath("restored")), []) + }) - await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, sevenZipBin }) + it("leaves the installation where it is, because the swap only happens once the extraction succeeded", async () => { + const archivePath = await makeConflictingTarGz("half-lands.tar.gz") + writeTree(workspacePath("my-install"), { "clientsettings.json": "the only copy of this" }) + + const fileSystem: FileSystem = { + exists: async (path: string): Promise => existsSync(path), + remove: async (path: string): Promise => { + rmSync(path, { recursive: true, force: true }) + return true + }, + move: async (from: string, to: string): Promise => { + renameSync(from, to) + return true + } + } + // The same shape extractWorker.ts gives the port: a rejection becomes a + // reported failure, anything else is a success. + const extractor: Extractor = { + extract: async (request, onComplete): Promise => { + try { + await runExtraction({ filePath: request.archivePath, outputPath: request.outputFolder, deleteArchive: false }) + onComplete({ ok: true }) + } catch (error) { + onComplete({ ok: false, error: (error as Error).message }) + } + } + } + + const result = await restoreInstallationBackup( + { fileSystem, extractor, ids: { newId: (): string => "token" }, closeGuard: { acquire: () => (): void => {} } }, + { + installation: { + id: "install-1", + name: "My install", + path: workspacePath("my-install"), + backupsLimit: 3, + compressionLevel: 6, + backups: [], + isBackingUp: false, + isPlaying: false, + isRestoringBackup: false + }, + backup: { id: "backup-1", date: 0, path: archivePath } + } + ) - assert.deepEqual(readdirSync(workspacePath("target")), ["vintagestory"]) - assert.equal(statSync(workspacePath("target", "vintagestory", "assets", "version-1.22.6.txt")).size, 0) + assert.equal(result.ok, false) + assert.equal(result.ok === false && result.reason, "extract-failed") + assert.equal(readFileSync(workspacePath("my-install", "clientsettings.json"), "utf8"), "the only copy of this") + assert.deepEqual(readdirSync(workspacePath("my-install")), ["clientsettings.json"]) }) +}) - // Two 7-Zip processes over a 2000 file archive run past the 5 s default on a - // Windows runner, where spawning and scanning cost far more than on Linux. - // The coalescing itself has nothing platform-specific in it, so the ubuntu - // run covers it; and PR #274 replaces this test outright, dropping 7-Zip for - // a yauzl reader that needs no process at all. - it.skipIf(process.platform === "win32")("coalesces 7-Zip progress and emits one terminal 100", async () => { - const source = workspacePath("many-files") - mkdirSync(source, { recursive: true }) - for (let index = 0; index < 2_000; index++) writeFileSync(join(source, `file-${index}.bin`), Buffer.alloc(2_048, index % 251)) +describe("runExtraction on a legacy zip backup", () => { + it("restores a backup written before the launcher moved off zip", async () => { + await runExtraction({ filePath: join(FIXTURES, "legacy-backup.zip"), outputPath: workspacePath("restored"), deleteArchive: false }) + + assert.deepEqual(readTree(workspacePath("restored")), { + Vintagestory: Buffer.from("elf").toString("base64"), + "assets/version-1.22.6.txt": "", + "Mods/notes.txt": Buffer.from("a mod list worth keeping\n").toString("base64") + }) + // A folder entry lands as a folder, and the zero byte marker inside it stays zero bytes. + assert.equal(lstatSync(workspacePath("restored", "assets")).isDirectory(), true) + assert.equal(statSync(workspacePath("restored", "assets", "version-1.22.6.txt")).size, 0) + }) + + it("keeps a zip's own shape, because those are the backups a restore puts back", async () => { + await runExtraction({ filePath: join(FIXTURES, "legacy-backup.zip"), outputPath: workspacePath("restored"), deleteArchive: false }) - const archivePath = workspacePath("progress.zip") - execFileSync(sevenZipBin, ["a", "-tzip", archivePath, source], { stdio: "ignore" }) + assert.deepEqual(readdirSync(workspacePath("restored")).sort(), ["Mods", "Vintagestory", "assets"]) + }) + + it("coalesces progress and emits one terminal 100", async () => { const reported: number[] = [] - await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, sevenZipBin, onProgress: (progress) => reported.push(progress) }) + await runExtraction({ filePath: join(FIXTURES, "legacy-backup.zip"), outputPath: workspacePath("restored"), deleteArchive: false, onProgress: (progress) => reported.push(progress) }) assert.equal(reported.at(-1), 100) assert.equal(reported.filter((progress) => progress === 100).length, 1) assert.equal(new Set(reported).size, reported.length) + assert.deepEqual( + [...reported].sort((left, right) => left - right), + reported + ) assert.equal( reported.some((progress) => progress > 0 && progress < 100), true ) }) + + it("deletes the archive when asked", async () => { + const consumedPath = workspacePath("consumed.zip") + writeFileSync(consumedPath, readFileSync(join(FIXTURES, "legacy-backup.zip"))) + + await runExtraction({ filePath: consumedPath, outputPath: workspacePath("restored"), deleteArchive: true }) + + assert.equal(existsSync(consumedPath), false) + }) + + it("refuses a zip naming entries outside the folder it unpacks into", async () => { + // The refusal comes from yauzl's own name validation, which stops the read + // at the first entry that names a drive, a "/" root or a ".." segment, so + // the launcher's table-of-contents pass reports an unreadable archive + // rather than an unsafe entry. Either way nothing is written, which is the + // property worth holding: the archive is refused before the output folder + // is even created. + await assert.rejects(runExtraction({ filePath: join(FIXTURES, "hostile-backup.zip"), outputPath: workspacePath("restored"), deleteArchive: false }), /could not be read/) + + assert.equal(existsSync(workspacePath("escaped.txt")), false) + assert.equal(existsSync(workspacePath("escaped-drive.txt")), false) + assert.equal(existsSync("/etc/escaped-absolute.txt"), false) + assert.equal(existsSync(workspacePath("restored")), false) + }) + + it("refuses an entry name yauzl accepts but the launcher does not", async () => { + // The other hostile fixture is stopped by yauzl's own name validation, so + // the launcher's isSafeArchiveEntry never gets a say on it. A NUL byte in + // the middle of a name is one yauzl has nothing to say about, which makes + // this the archive that pins that gate against real bytes. + await assert.rejects(runExtraction({ filePath: join(FIXTURES, "unsafe-name-backup.zip"), outputPath: workspacePath("restored"), deleteArchive: false }), /unsafe entry/) + + assert.equal(existsSync(workspacePath("restored")), false) + }) + + it("refuses an archive whose name says neither zip nor tar.gz", async () => { + await assert.rejects(runExtraction({ filePath: join(FIXTURES, "not-a-zip.bin"), outputPath: workspacePath("restored"), deleteArchive: false }), /not supported/) + }) +}) + +/** + * The unpackers' own look at an entry name, past the table-of-contents check + * that normally refuses these archives before either unpacker is reached. What + * is inside an archive is not something the launcher wrote, so both gates get + * pinned rather than only the first. + */ +describe("hostile entry names, straight at the unpackers", () => { + it("refuses to place a zip entry that climbs out, is absolute, or names a drive", () => { + const destination = workspacePath("destination", "inside") + + assert.throws(() => resolveEntryDestination(destination, "../escaped.txt"), /escaped its root/) + assert.throws(() => resolveEntryDestination(destination, "assets/../../escaped.txt"), /escaped its root/) + assert.throws(() => resolveEntryDestination(destination, "/etc/escaped-absolute.txt"), /escaped its root/) + // A backslash separator is a Windows path inside a zip, and is normalised + // before the comparison rather than taken as part of a file name. + assert.throws(() => resolveEntryDestination(destination, "..\\escaped.txt"), /escaped its root/) + assert.throws(() => resolveEntryDestination(destination, "."), /escaped its root/) + }) + + it("places an ordinary zip entry under the destination", () => { + const destination = workspacePath("destination", "inside") + + assert.equal(resolveEntryDestination(destination, "assets/version-1.22.6.txt"), join(destination, "assets", "version-1.22.6.txt")) + assert.equal(resolveEntryDestination(destination, "Mods\\notes.txt"), join(destination, "Mods", "notes.txt")) + }) + + it("writes nothing outside the destination for a zip full of escaping names", async () => { + const destination = workspacePath("destination", "inside") + mkdirSync(destination, { recursive: true }) + + await assert.rejects(extractZip(join(FIXTURES, "hostile-backup.zip"), destination)) + + assert.equal(existsSync(workspacePath("destination", "escaped.txt")), false) + assert.equal(existsSync(workspacePath("escaped.txt")), false) + assert.equal(existsSync("/etc/escaped-absolute.txt"), false) + assert.deepEqual(readdirSync(destination), []) + }) + + it("writes nothing outside the destination for a tar.gz climbing out with .. or an absolute path", async () => { + writeTree(workspacePath("source"), { "escaped.txt": "climbed out" }) + const archivePath = workspacePath("hostile-names.tar.gz") + await tar.create({ file: archivePath, gzip: true, cwd: workspacePath("source"), portable: true, preservePaths: true }, ["../source/escaped.txt", resolve(workspacePath("source", "escaped.txt"))]) + const destination = workspacePath("destination", "inside") + mkdirSync(destination, { recursive: true }) + + // The names are refused rather than quietly dropped: an entry the unpacker + // will not place is a failed extraction, not a smaller one. + await assert.rejects(extractTarGz(archivePath, destination), /Extraction failed/) + + assert.equal(existsSync(workspacePath("destination", "escaped.txt")), false) + assert.equal(existsSync(workspacePath("escaped.txt")), false) + // Whatever did land is under the destination, nowhere else. + for (const landed of readdirSync(destination, { recursive: true }) as string[]) { + assert.equal(resolve(destination, landed).startsWith(`${resolve(destination)}/`), true) + } + }) }) /** @@ -270,7 +559,7 @@ describe.skipIf(!realArchive)("runExtraction on a real Vintage Story archive", ( it("puts the game executable and the version marker straight in the target folder", { timeout: 600_000 }, async () => { const version = process.env.RIFT_E2E_VERSION ?? "" - await runExtraction({ filePath: realArchive as string, outputPath: workspacePath("target"), deleteArchive: false, sevenZipBin }) + await runExtraction({ filePath: realArchive as string, outputPath: workspacePath("target"), deleteArchive: false, unwrapSingleRootFolder: true }) const landed = readdirSync(workspacePath("target")) assert.equal(landed.includes("vintagestory"), false)