From 27164f42910d4c74a3631654e1ce494d9713211e Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 30 Aug 2026 14:55:43 +0000 Subject: [PATCH] fix(cli): stop install-service writing a unit that dies on upgrade and reboot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent defects left `threatcrushd.service` permanently unstartable on a real host — found at 6315 failed starts against a path last valid at 0.2.1. `resolveBinPath()` returned `process.argv[1]`, which under a pnpm global install is the version-stamped `.pnpm/@profullstack+threatcrush@X.Y.Z/...` directory. The next upgrade moves that directory, so ExecStart points at nothing and `Restart=on-failure` turns it into an endless loop. Rewrite it to the stable symlink pnpm keeps at the store root, and only when that symlink really exists. The unit also listed `/var/run/threatcrush` under ReadWritePaths. `/var/run` is a symlink to the `/run` tmpfs, so the directory `ensureSystemDirs()` created at install time was gone after the next reboot, and a missing ReadWritePaths entry fails the unit 226/NAMESPACE on every start. Use `RuntimeDirectory=` so systemd creates it per start, and stop pre-creating it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018NXz7vRz6C7vaGSwMgWZfD --- .../src/commands/__tests__/service.test.ts | 54 +++++++++++++++++++ apps/cli/src/commands/service.ts | 21 +++++++- apps/cli/src/systemd/threatcrushd.service | 8 ++- 3 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 apps/cli/src/commands/__tests__/service.test.ts diff --git a/apps/cli/src/commands/__tests__/service.test.ts b/apps/cli/src/commands/__tests__/service.test.ts new file mode 100644 index 0000000..827800a --- /dev/null +++ b/apps/cli/src/commands/__tests__/service.test.ts @@ -0,0 +1,54 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { stableBinPath } from "../service.js"; + +const UNIT = readFileSync( + join(__dirname, "..", "..", "systemd", "threatcrushd.service"), + "utf-8", +); + +describe("stableBinPath", () => { + // The bug this file exists for: `install-service` baked the version-stamped + // pnpm path into ExecStart, so the next upgrade left the unit pointing at a + // directory that no longer existed. On one host that was 6315 failed starts + // against a path last valid at 0.2.1. + it("rewrites a version-stamped pnpm path to the stable symlink", () => { + const root = "/home/ubuntu/.local/share/pnpm/global/5"; + const versioned = `${root}/.pnpm/@profullstack+threatcrush@0.11.6/node_modules/@profullstack/threatcrush/dist/index.js`; + + // The rewrite only takes effect when the stable path exists on disk, which + // it does not here, so we assert the shape of the rewrite itself. + const rewritten = versioned.replace(/\/\.pnpm\/[^/]+\/node_modules\//, "/node_modules/"); + expect(rewritten).toBe(`${root}/node_modules/@profullstack/threatcrush/dist/index.js`); + expect(rewritten).not.toMatch(/@\d+\.\d+\.\d+/); + }); + + it("leaves a path with no .pnpm segment untouched", () => { + // npm's global layout is already version-free. + const npmPath = "/usr/lib/node_modules/@profullstack/threatcrush/dist/index.js"; + expect(stableBinPath(npmPath)).toBe(npmPath); + }); + + it("falls back to the original path when the stable one is absent", () => { + const versioned = + "/nonexistent/.pnpm/@profullstack+threatcrush@0.11.6/node_modules/@profullstack/threatcrush/dist/index.js"; + expect(stableBinPath(versioned)).toBe(versioned); + }); +}); + +describe("systemd unit template", () => { + // /var/run is a symlink to the /run tmpfs, so a directory created at install + // time is gone after the next reboot — and a missing ReadWritePaths entry + // fails the unit 226/NAMESPACE on every start. + it("does not list a tmpfs path under ReadWritePaths", () => { + const readWrite = UNIT.split("\n").find((l) => l.startsWith("ReadWritePaths=")); + expect(readWrite).toBeDefined(); + expect(readWrite).not.toContain("/var/run/"); + expect(readWrite).not.toContain("/run/"); + }); + + it("lets systemd create the runtime directory on each start", () => { + expect(UNIT).toMatch(/^RuntimeDirectory=threatcrush$/m); + }); +}); diff --git a/apps/cli/src/commands/service.ts b/apps/cli/src/commands/service.ts index 2181444..5d894fd 100644 --- a/apps/cli/src/commands/service.ts +++ b/apps/cli/src/commands/service.ts @@ -14,10 +14,25 @@ function resolveTemplate(): string { return readFileSync(templatePath, 'utf-8'); } +// pnpm installs a global package under a version-stamped directory +// (`.pnpm/@profullstack+threatcrush@0.11.6/node_modules/…`) and points a stable +// symlink at it from the store root. Baking the version-stamped path into +// ExecStart means the very next upgrade leaves the unit pointing at a directory +// that no longer exists — the unit then fails on every start, forever, and +// `Restart=on-failure` turns that into a permanent loop. Observed in the wild at +// 6315 restarts against a path last valid at 0.2.1. +export function stableBinPath(binPath: string): string { + const stable = binPath.replace(/\/\.pnpm\/[^/]+\/node_modules\//, '/node_modules/'); + if (stable === binPath) return binPath; + // Only take the rewrite if the symlink is really there; a layout we have not + // seen is better served by the path we were actually invoked with. + return existsSync(stable) ? stable : binPath; +} + function resolveBinPath(): string { // When installed globally the script path is the CLI bin. const arg = process.argv[1]; - if (arg && existsSync(arg)) return arg; + if (arg && existsSync(arg)) return stableBinPath(arg); try { return execSync('command -v threatcrush', { encoding: 'utf-8' }).trim(); } catch { @@ -74,7 +89,9 @@ function ensureSystemDirs(): void { { path: '/etc/threatcrush/threatcrushd.conf.d', groupWritable: true }, { path: '/var/log/threatcrush', groupWritable: false }, { path: '/var/lib/threatcrush', groupWritable: false }, - { path: '/var/run/threatcrush', groupWritable: false }, + // /run/threatcrush is deliberately absent: it lives on a tmpfs, so creating + // it here only lasts until the next reboot. The unit's RuntimeDirectory= + // recreates it on every start instead. ]; let admGid: number | null = null; try { diff --git a/apps/cli/src/systemd/threatcrushd.service b/apps/cli/src/systemd/threatcrushd.service index 525af4b..3990d56 100644 --- a/apps/cli/src/systemd/threatcrushd.service +++ b/apps/cli/src/systemd/threatcrushd.service @@ -17,7 +17,13 @@ TimeoutStopSec=10 NoNewPrivileges=true ProtectSystem=full ProtectHome=read-only -ReadWritePaths=/etc/threatcrush /var/log/threatcrush /var/lib/threatcrush /var/run/threatcrush +ReadWritePaths=/etc/threatcrush /var/log/threatcrush /var/lib/threatcrush +# /var/run is a symlink to /run, which is a tmpfs — anything created there at +# install time is gone after the next reboot. Listing it under ReadWritePaths +# then makes the unit fail 226/NAMESPACE on every start. RuntimeDirectory has +# systemd create (and clean up) /run/threatcrush itself, on each start. +RuntimeDirectory=threatcrush +RuntimeDirectoryMode=0755 PrivateTmp=true # Log directly to journald