diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cf9047d7..6265aa2ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -274,6 +274,10 @@ - **The hint for reading scans on the device points to the right switch**, Settings, Modules, Labs, in every language. - **Help links open the right page** instead of going through redirects. +- **A directory setting below `/proc` no longer freezes the server.** Node's + way of creating a folder with its parents loops forever there on Linux, + so a `DB_BACKUP_DIR` or `GEOLITE2_DIR` pointing below it stopped the + process. Such a directory now reads as not writable. ### Security diff --git a/src/lib/database-backup/__tests__/dump-core.test.ts b/src/lib/database-backup/__tests__/dump-core.test.ts index 6cb5c1a34..14b9cc9d8 100644 --- a/src/lib/database-backup/__tests__/dump-core.test.ts +++ b/src/lib/database-backup/__tests__/dump-core.test.ts @@ -9,6 +9,7 @@ * of the rule: the restore script cannot import TypeScript, so these are the * tests that keep its plain-JS reading of both from drifting. */ +import { execFileSync } from "node:child_process"; import { chmodSync, createReadStream, @@ -24,6 +25,7 @@ import { import { readdir } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { fileURLToPath } from "node:url"; import { Readable } from "node:stream"; import { afterEach, describe, expect, it, vi } from "vitest"; @@ -53,7 +55,9 @@ import { selectForDeletion, verifyArchive, instanceIdFromEnv, + isWritableDirectory, listBucketDumps, + makeDirectory, type DirectoryProbe, type DumpSet, type Env, @@ -1025,3 +1029,46 @@ describe("whether the dump directory survives the container", () => { ); }); }); + +describe("making the backup directory", () => { + const root = mkdtempSync(join(tmpdir(), "hl-mkdir-")); + afterEach(() => { + for (const name of readdirSync(root)) { + rmSync(join(root, name), { recursive: true, force: true }); + } + }); + + it("creates every missing level and leaves an existing one alone", () => { + const dir = join(root, "a", "b", "c"); + makeDirectory(dir); + expect(statSync(dir).isDirectory()).toBe(true); + makeDirectory(dir); + expect(isWritableDirectory(dir)).toBe(true); + }); + + it("answers false below a file", () => { + writeFileSync(join(root, "file"), ""); + expect(isWritableDirectory(join(root, "file", "below"))).toBe(false); + }); + + // Node's recursive mkdir loops forever below /proc, inside one native call, + // so this runs in a child with a deadline: a regression fails here instead + // of freezing the test worker. + it.runIf(process.platform === "linux")( + "answers false below /proc instead of hanging", + () => { + const core = fileURLToPath(new URL("../dump-core.mjs", import.meta.url)); + const out = execFileSync( + process.execPath, + [ + "--input-type=module", + "-e", + `const { isWritableDirectory } = await import(${JSON.stringify(core)}); +console.log(isWritableDirectory("/proc/healthlog-cannot-write-here/nested"));`, + ], + { encoding: "utf8", timeout: 10_000 }, + ); + expect(out.trim()).toBe("false"); + }, + ); +}); diff --git a/src/lib/database-backup/dump-core.mjs b/src/lib/database-backup/dump-core.mjs index 740921876..95f17b269 100644 --- a/src/lib/database-backup/dump-core.mjs +++ b/src/lib/database-backup/dump-core.mjs @@ -43,7 +43,7 @@ import { statSync, } from "node:fs"; import { readFile, rename, rm, stat, writeFile } from "node:fs/promises"; -import { basename, join } from "node:path"; +import { basename, dirname, join, resolve } from "node:path"; import { pipeline } from "node:stream/promises"; import { createGunzip } from "node:zlib"; @@ -208,6 +208,41 @@ export function resolveBackupSchedule(env = process.env) { }; } +/** + * Create `dir` and its missing parents, one level at a time from the nearest + * one that exists. Throws what `mkdir` throws. + * + * Not `mkdirSync(dir, { recursive: true })`: on Linux, below a pseudo file + * system such as `/proc`, the child answers ENOENT while its parent answers + * EEXIST, and Node's recursive walk retries that pair forever inside one + * native call, so no timer, signal handler or test timeout ever runs again. + * A `DB_BACKUP_DIR` set to such a path froze the worker. Here every level is + * one plain `mkdir`, and the walk is as long as the path. + * + * @param {string} dir + */ +export function makeDirectory(dir) { + /** @type {string[]} */ + const missing = []; + let current = resolve(dir); + while (!existsSync(current)) { + missing.push(current); + const parent = dirname(current); + if (parent === current) break; + current = parent; + } + for (const path of missing.reverse()) { + try { + mkdirSync(path); + } catch (err) { + // Created in between by someone else: that is what was wanted. + if (/** @type {NodeJS.ErrnoException} */ (err).code !== "EEXIST") { + throw err; + } + } + } +} + /** * Whether `dir` exists (or can be created) and this process may write in it. * Never throws. @@ -216,7 +251,7 @@ export function resolveBackupSchedule(env = process.env) { */ export function isWritableDirectory(dir) { try { - mkdirSync(dir, { recursive: true }); + makeDirectory(dir); if (!statSync(dir).isDirectory()) return false; accessSync(dir, fsConstants.W_OK | fsConstants.X_OK); return true; diff --git a/src/lib/geo/__tests__/geolite2-fetch.test.ts b/src/lib/geo/__tests__/geolite2-fetch.test.ts index f9ea0572d..d6e50c542 100644 --- a/src/lib/geo/__tests__/geolite2-fetch.test.ts +++ b/src/lib/geo/__tests__/geolite2-fetch.test.ts @@ -28,6 +28,7 @@ vi.mock("@/lib/geo", async (importOriginal) => { import { buildMaxmindDownloadUrl, fetchGeoLite2Databases, + makeDirectory, MAXMIND_DOWNLOAD_BASE, } from "@/lib/geo/geolite2-fetch"; @@ -282,3 +283,28 @@ describe("fetchGeoLite2Databases — gating", () => { ); }); }); + +describe("makeDirectory", () => { + it("creates every missing level and accepts an existing one", async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "hl-geo-mkdir-")); + try { + const dir = path.join(root, "a", "b"); + await makeDirectory(dir); + await makeDirectory(dir); + expect(fs.statSync(dir).isDirectory()).toBe(true); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + // The recursive form never settles below /proc; this one rejects. + it.runIf(process.platform === "linux")( + "rejects below /proc instead of never settling", + async () => { + await expect( + makeDirectory("/proc/healthlog-cannot-write-here/nested"), + ).rejects.toMatchObject({ code: "ENOENT" }); + }, + 5_000, + ); +}); diff --git a/src/lib/geo/geolite2-fetch.ts b/src/lib/geo/geolite2-fetch.ts index 86106b1b4..c520781dd 100644 --- a/src/lib/geo/geolite2-fetch.ts +++ b/src/lib/geo/geolite2-fetch.ts @@ -227,6 +227,37 @@ async function downloadAndExtractEdition( return tmpDest; } +/** + * Create `dir` and its missing parents one level at a time. Not + * `fsp.mkdir(dir, { recursive: true })`: below `/proc` on Linux Node's + * recursive walk retries ENOENT/EEXIST forever and the promise never settles. + * Same walk as `makeDirectory` in `src/lib/database-backup/dump-core.mjs`, + * which this module cannot import (its `child_process` import would undo the + * bounded trace described above). + */ +export async function makeDirectory(dir: string): Promise { + const missing: string[] = []; + let current = path.resolve(dir); + for (;;) { + try { + await fsp.stat(current); + break; + } catch { + missing.push(current); + const parent = path.dirname(current); + if (parent === current) break; + current = parent; + } + } + for (const p of missing.reverse()) { + try { + await fsp.mkdir(p); + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== "EEXIST") throw err; + } + } +} + /** * Fetch both GeoLite2 databases and place them in `geoLiteDir()`. * @@ -255,7 +286,7 @@ export async function fetchGeoLite2Databases( const dir = geoLiteDir(); try { - await fsp.mkdir(dir, { recursive: true }); + await makeDirectory(dir); } catch (err) { return fail(`geolite2 directory not writable (${dir})`, err); }