From 707e08e8dab07c4b2cdb6811b166702f63d90c82 Mon Sep 17 00:00:00 2001 From: Laszlo Toth Date: Mon, 6 Jul 2026 10:42:27 +0200 Subject: [PATCH 1/4] fix: v1.1.1 reboot self-heal, caddy loop, secret persistence, root guard, qbit RAM Addresses the v1.1.0 issues reported on Discord (Loki/Ryder): - Reboot ordering: add a deunhealth sidecar that restarts qBittorrent when its healthcheck fails. depends_on ordering is ignored on host reboot, so a VPN-routed qbit can start before gluetun and lose its published WebUI port, and restart:unless-stopped can't fix it because qbit is up, just orphaned. Only qbit is labeled, so nothing else is ever auto-restarted. arrstack update injects the sidecar into existing VPN installs too. - Healthchecks: render the catalog health blocks (previously dead code) as container healthchecks using a portable curl-or-wget probe (arr/media images ship one or the other). Skip tcp/port-0 blocks (caddy, recyclarr). - Caddy crash-loop: emit a minimal :80 site in LAN-only mode with no local DNS instead of a 0-byte Caddyfile that makes caddy exit and restart-loop. - Secret persistence: reuse persisted api keys and a new secrets block (the Bazarr/translator AES key + Bazarr flask secret) across reconfigure/resume instead of rotating them out from under running containers. Stop the install with a fix message if admin.txt exists but is unreadable (root-owned) rather than silently generating a new password and locking the user out of qbit. - Root guard: warn in preflight and the wizard status strip, and clamp PUID/PGID away from 0 so LinuxServer.io containers do not run as root. - qBittorrent RAM: set enable_os_cache=false to bound the libtorrent-2.0 memory-mapped disk cache that can grow to many GB and OOM a small VM. - Tooling: add typescript to devDependencies so `bun run typecheck` works. 296 tests pass, typecheck clean. --- package.json | 5 +- src/catalog/services.yaml | 21 ++++++ src/cli.ts | 15 ++++- src/platform/preflight.ts | 21 ++++++ src/renderer/compose.ts | 52 ++++++++++++++- src/state/schema.ts | 12 ++++ src/state/store.ts | 21 ++++++ src/ui/wizard/Form.tsx | 1 + src/ui/wizard/StatusStrip.tsx | 8 ++- src/ui/wizard/useWizardState.ts | 68 +++++++++++++++---- src/usecase/install.ts | 18 +++-- src/usecase/update.ts | 15 ++++- src/version.ts | 2 +- src/wiring/qbittorrent.ts | 8 +++ templates/Caddyfile.hbs | 12 ++++ templates/compose.yml.hbs | 6 ++ tests/platform/preflight.test.ts | 28 ++++++++ tests/renderer/caddy.test.ts | 21 +++++- tests/renderer/compose.test.ts | 62 ++++++++++++++++++ tests/state/store.test.ts | 36 +++++++++- tests/ui/wizard/useWizardState.test.ts | 91 +++++++++++++++++++++++++- tests/usecase/update.test.ts | 29 ++++++++ tests/wiring/qbittorrent.test.ts | 21 ++++++ 23 files changed, 547 insertions(+), 26 deletions(-) diff --git a/package.json b/package.json index cbc1e59..a95c0b0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "arrstack-installer", - "version": "1.1.0", + "version": "1.1.1", "type": "module", "bin": { "arrstack": "src/cli.ts" @@ -17,7 +17,8 @@ "@types/bcryptjs": "^3.0.0", "@types/react": "^19.2.14", "bun-types": "^1.3.12", - "ink-testing-library": "^4.0.0" + "ink-testing-library": "^4.0.0", + "typescript": "^6.0.3" }, "dependencies": { "bcryptjs": "^3.0.3", diff --git a/src/catalog/services.yaml b/src/catalog/services.yaml index 159cc7b..d93f429 100644 --- a/src/catalog/services.yaml +++ b/src/catalog/services.yaml @@ -328,6 +328,27 @@ services: extraDevices: - /dev/net/tun:/dev/net/tun + - id: deunhealth + name: Deunhealth + # Restarts qBittorrent when Docker marks it unhealthy. On a host reboot the + # daemon ignores depends_on ordering, so the VPN-routed qbit can start + # before gluetun and lose its published WebUI port; a plain restart policy + # doesn't help because qbit is up, just orphaned from gluetun's netns. + # deunhealth watches the deunhealth.restart.on.unhealthy label (only qbit + # carries it) and restarts it once gluetun is healthy again. The renderer + # bind-mounts /var/run/docker.sock (ro) so it can talk to the daemon. + description: Auto-restarts qBittorrent if it goes unhealthy after a reboot + category: utility + image: qmcgaw/deunhealth + tag: "latest" + ports: [] + configPath: /config + mounts: {} + envVars: {} + dependsOn: [] + default: false + requiresAdminAuth: false + - id: dnsmasq name: dnsmasq description: LAN-wide DNS so "*." resolves to this host on every client diff --git a/src/cli.ts b/src/cli.ts index 7a051d1..706bc09 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -4,7 +4,7 @@ import { VERSION } from "./version.js"; import { render } from "ink"; import React from "react"; import { App } from "./ui/App.js"; -import { readState } from "./state/store.js"; +import { readState, adminTxtUnreadable } from "./state/store.js"; import { runDoctor } from "./usecase/doctor.js"; import { runUpdate } from "./usecase/update.js"; import { showPassword } from "./usecase/show-password.js"; @@ -50,6 +50,19 @@ program console.log(`Removed: ${path}`); } } + // A previous `sudo` install can leave admin.txt root-owned; a later + // non-sudo run then can't read it, would generate a NEW admin password, + // rewrite qBittorrent.conf, and lock the user out of the still-running qbit + // container. Stop early with a fix instead of silently rotating. + if (!opts.fresh && adminTxtUnreadable(installDir)) { + const adminTxt = join(installDir, "admin.txt"); + console.error( + `\nCannot read ${adminTxt}. It looks like a previous 'sudo arrstack install' left it owned by root.\n` + + `Fix ownership and re-run as your normal user (not sudo):\n` + + ` sudo chown $(id -u):$(id -g) ${adminTxt}\n` + ); + process.exit(1); + } const existing = opts.fresh ? null : readState(installDir); const { waitUntilExit } = render(React.createElement(App, { existingState: existing })); await waitUntilExit(); diff --git a/src/platform/preflight.ts b/src/platform/preflight.ts index b5ce2ab..229f340 100644 --- a/src/platform/preflight.ts +++ b/src/platform/preflight.ts @@ -9,6 +9,24 @@ export interface CheckResult { blocking: boolean; } +// Running the installer as root makes every LinuxServer.io container run as +// root (PUID/PGID 0), which breaks them ("usermod: user abc is currently used +// by process 1") and litters root-owned files. Non-blocking: some users only +// have root, and the wizard now clamps PUID/PGID away from 0 regardless, so +// this is a warning rather than a hard stop. +export function checkNotRoot(): CheckResult { + const uid = typeof process.getuid === "function" ? process.getuid() : -1; + const isRoot = uid === 0; + return { + name: "Not running as root", + ok: !isRoot, + message: isRoot + ? "Running as root. LinuxServer.io containers misbehave as root; re-run as a normal user in the 'docker' group (PUID/PGID default to 1000)." + : "Running as a normal user", + blocking: false, + }; +} + async function checkDiskSpace(path: string, minGb: number): Promise { const result = await exec(["df", "-Pk", path], { timeoutMs: 5000 }); const name = `Disk space on ${path}`; @@ -51,6 +69,9 @@ export async function runPreflight( ): Promise { const results: CheckResult[] = []; + // 0. Not running as root (warning only, non-blocking) + results.push(checkNotRoot()); + // 1. Docker installed const dockerInstalled = await isDockerInstalled(); results.push({ diff --git a/src/renderer/compose.ts b/src/renderer/compose.ts index 06efd8c..609fe63 100644 --- a/src/renderer/compose.ts +++ b/src/renderer/compose.ts @@ -84,6 +84,8 @@ interface ServiceContext { // forms can't be mixed on one service. dependsOnLongForm: boolean; healthcheck?: Healthcheck; + // Compose container labels (e.g. the deunhealth auto-restart marker on qbit). + labels: string[]; vpnNetwork: boolean; } @@ -202,6 +204,32 @@ const GLUETUN_HEALTHCHECK: Healthcheck = { start_period: "30s", }; +// Label read by the deunhealth sidecar: it restarts any container carrying it +// when Docker reports it unhealthy. We put it on qBittorrent only. +const DEUNHEALTH_LABEL = "deunhealth.restart.on.unhealthy=true"; + +// Turn a catalog `health` block into a container healthcheck. gluetun keeps its +// bespoke probe. For everything else we only render `http` blocks: the probe is +// curl-OR-wget (every arr/media image ships at least one; e.g. jellyseerr has +// only wget, bazarr only curl) hitting an unauthenticated endpoint, so a real +// 200 is required to count as healthy. `tcp` blocks (caddy, recyclarr) are +// skipped: a bare port-open check needs nc, which many images lack, and caddy's +// :80 can legitimately 404 on an unmatched host. Timings are generous so a slow +// first boot doesn't flap to unhealthy. +function buildHealthcheck(svc: Service): Healthcheck | undefined { + if (svc.id === "gluetun") return GLUETUN_HEALTHCHECK; + const h = svc.health; + if (!h || h.type !== "http" || !h.path || h.port <= 0) return undefined; + const url = `http://127.0.0.1:${h.port}${h.path}`; + return { + test: `["CMD-SHELL", "curl -fsS -o /dev/null ${url} || wget -q -O /dev/null ${url} || exit 1"]`, + interval: "30s", + timeout: "10s", + retries: 5, + start_period: "45s", + }; +} + // Translates the wizard's VPN state into the env vars gluetun expects // (VPN_SERVICE_PROVIDER / VPN_TYPE / WIREGUARD_* / SERVER_COUNTRIES / custom // endpoint tuple). Only emitted for the gluetun service and only when VPN @@ -265,11 +293,32 @@ export function buildComposeContext(services: Service[], opts: ComposeOptions): } const dependsOnLongForm = dependsOn.some((d) => d.condition !== undefined); - const healthcheck = svc.id === "gluetun" ? GLUETUN_HEALTHCHECK : undefined; + const healthcheck = buildHealthcheck(svc); + + // Only qBittorrent is auto-restarted by deunhealth. On a host reboot Docker + // ignores depends_on ordering, so qbit (network_mode: service:gluetun) can + // start before gluetun and end up orphaned from its netns with its WebUI + // port dead. restart: unless-stopped won't help because qbit is up, just + // broken. Its healthcheck flips it to unhealthy and deunhealth restarts it + // once gluetun is ready. Labeling only qbit means nothing else is touched. + const labels: string[] = []; + if (vpnNetwork) { + labels.push(DEUNHEALTH_LABEL); + } const caddyImage = resolveCaddyImage(svc, opts.remoteMode); const dataMounts = buildDataMounts(svc, opts.storageRoot, opts.extraPaths); + if (svc.id === "deunhealth") { + // deunhealth watches Docker's health status and restarts labeled + // containers, so it needs the daemon socket. Read-only is enough: the + // restart API call still works over a ro-mounted socket. + dataMounts.push({ + src: "/var/run/docker.sock", + dst: "/var/run/docker.sock", + mode: "ro", + }); + } if (svc.id === "caddy") { // The renderer writes {installDir}/Caddyfile to disk, but the container // needs it at /etc/caddy/Caddyfile. Without this explicit mount Caddy @@ -299,6 +348,7 @@ export function buildComposeContext(services: Service[], opts: ComposeOptions): dependsOn, dependsOnLongForm, healthcheck, + labels, vpnNetwork, }; }); diff --git a/src/state/schema.ts b/src/state/schema.ts index b4a1cdd..3e18898 100644 --- a/src/state/schema.ts +++ b/src/state/schema.ts @@ -57,6 +57,18 @@ export const StateSchema = z.object({ // from this list. English-only is the safest out-of-the-box default. subtitle_languages: z.array(z.string().length(2)).default(["en"]), api_keys: z.record(z.string(), z.string()), + // Long-lived generated secrets that are NOT service API keys but must still + // survive reconfigure/resume. Rotating these breaks a running stack: the + // AES-GCM key is shared by Bazarr and ai-subtitle-translator (rotating it + // orphans everything they encrypted at rest), and the Flask secret would + // invalidate all Bazarr sessions. Optional/defaulted so old state.json files + // (written before this field existed) still parse. + secrets: z + .object({ + translator_encryption_key: z.string().optional(), + bazarr_flask_secret: z.string().optional(), + }) + .default({}), install_started_at: z.string().datetime().optional(), install_completed_at: z.string().datetime().optional(), last_updated_at: z.string().datetime().optional(), diff --git a/src/state/store.ts b/src/state/store.ts index c0abeb8..3772795 100644 --- a/src/state/store.ts +++ b/src/state/store.ts @@ -14,6 +14,27 @@ export function readState(installDir: string): State | null { return StateSchema.parse(data); } +/** + * True when admin.txt exists but cannot be read. This happens when a prior + * `sudo arrstack install` left it owned by root and a later non-sudo run hits + * EACCES. The installer must NOT treat that as "no password" and silently + * generate a new one: the qBittorrent container from the earlier run still + * holds the old password, so a rotate rewrites qBittorrent.conf with a hash the + * running container never sees and the user is locked out. Callers should stop + * with an actionable message instead. A genuinely absent file returns false + * (that is a normal fresh install). + */ +export function adminTxtUnreadable(installDir: string): boolean { + const p = join(installDir, "admin.txt"); + if (!existsSync(p)) return false; + try { + readFileSync(p, "utf-8"); + return false; + } catch { + return true; + } +} + export function writeState(installDir: string, state: State): void { const statePath = join(installDir, STATE_FILE); const tmpPath = `${statePath}.tmp`; diff --git a/src/ui/wizard/Form.tsx b/src/ui/wizard/Form.tsx index d93a01d..332248f 100644 --- a/src/ui/wizard/Form.tsx +++ b/src/ui/wizard/Form.tsx @@ -427,6 +427,7 @@ export function Form({ initial, isReconfigure, onSubmit, onCancel }: FormProps) diskInfo={ws.diskInfo} dockerOk={ws.dockerOk} portsOk={ws.portsOk} + isRoot={ws.isRoot} gpuName={ws.detectedGpus.find((g) => g.vendor === ws.gpuVendor)?.name} caddyHttpPort={ws.caddyHttpPort} caddyHttpsPort={ws.caddyHttpsPort} diff --git a/src/ui/wizard/StatusStrip.tsx b/src/ui/wizard/StatusStrip.tsx index 541cb3f..7451e8c 100644 --- a/src/ui/wizard/StatusStrip.tsx +++ b/src/ui/wizard/StatusStrip.tsx @@ -7,13 +7,14 @@ interface StatusStripProps { diskInfo: Array<{ path: string; freeGb: number }>; dockerOk: boolean; portsOk: boolean; + isRoot?: boolean; gpuName?: string; caddyHttpPort?: number; caddyHttpsPort?: number; portConflicts?: string[]; } -export function StatusStrip({ diskInfo, dockerOk, portsOk, gpuName, caddyHttpPort, caddyHttpsPort, portConflicts }: StatusStripProps) { +export function StatusStrip({ diskInfo, dockerOk, portsOk, isRoot, gpuName, caddyHttpPort, caddyHttpsPort, portConflicts }: StatusStripProps) { const caddyNote = (!portsOk && caddyHttpPort && caddyHttpsPort) ? `Caddy:${caddyHttpPort}/${caddyHttpsPort}` : portsOk ? "80/443:free" : "80/443:in use"; @@ -31,6 +32,11 @@ export function StatusStrip({ diskInfo, dockerOk, portsOk, gpuName, caddyHttpPor {portConflicts && portConflicts.length > 0 && ( Port remaps: {portConflicts.join(", ")} )} + {isRoot && ( + + ⚠ Running as root: containers will use PUID/PGID 1000, not 0. Prefer running as a normal user in the 'docker' group. + + )} ); } diff --git a/src/ui/wizard/useWizardState.ts b/src/ui/wizard/useWizardState.ts index 4956a4b..38c3c4a 100644 --- a/src/ui/wizard/useWizardState.ts +++ b/src/ui/wizard/useWizardState.ts @@ -74,11 +74,15 @@ export interface WizardState { // Status (for status strip) dockerOk: boolean; portsOk: boolean; + isRoot: boolean; // installer running as root -> PUID/PGID footgun warning diskInfo: Array<{ path: string; freeGb: number }>; portConflicts: string[]; // human-readable conflict messages } -export function buildStateFromWizard(ws: WizardState): State { +export function buildStateFromWizard( + ws: WizardState, + existing?: Partial | null, +): State { const catalog = loadCatalog(); const enabledIds = ws.services.filter((s) => s.checked).map((s) => s.id); @@ -88,6 +92,16 @@ export function buildStateFromWizard(ws: WizardState): State { if (ws.remoteMode === "duckdns") enabledIds.push("duckdns-updater"); if (ws.localDnsEnabled && ws.localDnsInstallDnsmasq) enabledIds.push("dnsmasq"); if (ws.vpnMode === "gluetun" && !enabledIds.includes("gluetun")) enabledIds.push("gluetun"); + // deunhealth restarts the VPN-routed qBittorrent when it goes unhealthy after + // a reboot (Docker ignores depends_on ordering on daemon restart). Only + // needed when qbit actually runs inside gluetun's netns. + if ( + ws.vpnMode === "gluetun" && + enabledIds.includes("qbittorrent") && + !enabledIds.includes("deunhealth") + ) { + enabledIds.push("deunhealth"); + } // Bazarr+ bundle: when bazarr is checked, auto-add its dependencies if (enabledIds.includes("bazarr")) { @@ -101,10 +115,16 @@ export function buildStateFromWizard(ws: WizardState): State { .map((p) => p.trim()) .filter(Boolean); - const apiKeys: Record = {}; + // Reuse every api key persisted from a prior install/attempt, then generate + // one only for a service that declares apiKeyEnv and doesn't have one yet. + // Regenerating on reconfigure would rotate keys out from under running + // containers and break cross-service references (Prowlarr->arr, Jellyseerr, + // Bazarr) that still hold the old value. Bazarr's key has no apiKeyEnv and is + // minted in install.ts, but it lives in api_keys too so this carries it over. + const apiKeys: Record = { ...(existing?.api_keys ?? {}) }; for (const id of enabledIds) { const svc = catalog.find((s) => s.id === id); - if (svc?.apiKeyEnv) { + if (svc?.apiKeyEnv && !apiKeys[id]) { apiKeys[id] = generateApiKey(); } } @@ -176,6 +196,10 @@ export function buildStateFromWizard(ws: WizardState): State { .map((s) => s.trim().toLowerCase()) .filter((s) => /^[a-z]{2}$/.test(s)), api_keys: apiKeys, + // Carry persisted long-lived secrets forward so reconfigure/resume doesn't + // rotate the Bazarr/translator AES key or the Flask secret. install.ts + // fills these on first run and writes them back. + secrets: existing?.secrets ?? {}, }; } @@ -203,7 +227,7 @@ function readExistingAdminPassword(installDir: string): string | null { // Infrastructure: caddy, ddns containers, dnsmasq // Bazarr+ deps: flaresolverr, opensubtitles-scraper, ai-subtitle-translator (bundled with Bazarr+) const AUTO_MANAGED_SERVICES = new Set([ - "caddy", "cloudflare-ddns", "duckdns-updater", "dnsmasq", + "caddy", "cloudflare-ddns", "duckdns-updater", "dnsmasq", "deunhealth", "flaresolverr", "opensubtitles-scraper", "ai-subtitle-translator", ]); @@ -223,12 +247,27 @@ function buildInitialServices(existingEnabled?: string[]): WizardServiceItem[] { })); } +/** + * PUID/PGID must never be 0. Running the installer as root makes getuid()/ + * getgid() return 0, but LinuxServer.io containers break when run as root, so + * fall back to the conventional 1000. Also used to sanitize a persisted 0 left + * behind by an installer run under an older version that didn't clamp. + */ +export function safeSystemId(raw: number | undefined): number { + return raw === undefined || raw === 0 ? 1000 : raw; +} + export function useWizardState(existingState?: Partial | null) { const timezone = detectTimezone(); - const puid = - typeof process.getuid === "function" ? process.getuid() : 1000; - const pgid = - typeof process.getgid === "function" ? process.getgid() : 1000; + // Never default PUID/PGID to 0. Under sudo/as root, getuid()/getgid() return + // 0, but LinuxServer.io images break when run as root ("usermod: user abc is + // currently used by process 1") and write root-owned config/media. Fall back + // to the conventional 1000; config dirs are chowned to this PUID anyway. + const rawUid = typeof process.getuid === "function" ? process.getuid() : 1000; + const rawGid = typeof process.getgid === "function" ? process.getgid() : 1000; + const puid = safeSystemId(rawUid); + const pgid = safeSystemId(rawGid); + const isRoot = rawUid === 0; const defaultStorageRoot = `${process.env.HOME ?? "."}/arrstack/data`; const [storageRoot, setStorageRoot] = useState( @@ -294,8 +333,13 @@ export function useWizardState(existingState?: Partial | null) { ); const [tz, setTz] = useState(existingState?.timezone ?? timezone); - const [puidState, setPuid] = useState(existingState?.puid ?? puid); - const [pgidState, setPgid] = useState(existingState?.pgid ?? pgid); + // Reuse a persisted id on resume, but ignore a 0 left by an older root run. + const [puidState, setPuid] = useState( + existingState?.puid && existingState.puid !== 0 ? existingState.puid : puid, + ); + const [pgidState, setPgid] = useState( + existingState?.pgid && existingState.pgid !== 0 ? existingState.pgid : pgid, + ); const [vpnMode, setVpnMode] = useState(() => { // vpn.enabled is what controls whether we route qbittorrent through // gluetun; provider now holds a real VPN service name (mullvad, etc.), @@ -474,10 +518,11 @@ export function useWizardState(existingState?: Partial | null) { caddyHttpsPort, dockerOk, portsOk, + isRoot, diskInfo, portConflicts, }; - return buildStateFromWizard(ws); + return buildStateFromWizard(ws, existingState); } return { @@ -556,6 +601,7 @@ export function useWizardState(existingState?: Partial | null) { // Status dockerOk, portsOk, + isRoot, diskInfo, portConflicts, diff --git a/src/usecase/install.ts b/src/usecase/install.ts index 86045e4..e5d42eb 100644 --- a/src/usecase/install.ts +++ b/src/usecase/install.ts @@ -170,9 +170,17 @@ export async function runInstall( let pbkdf2Hash = ""; let bazarrHash = ""; let hostIp = "localhost"; - // Generated once, shared between the .env (-> ai-subtitle-translator) and - // Bazarr's config.yaml so AES-GCM encryption between the two matches. - const translatorEncryptionKey = randomBytes(32).toString("hex"); + // Long-lived secrets: reuse whatever a prior install persisted so a + // reconfigure/resume does NOT rotate them. The AES-GCM key is shared between + // the .env (-> ai-subtitle-translator) and Bazarr's config.yaml so their + // encryption matches (rotating it orphans data encrypted at rest); the Flask + // secret backs Bazarr's sessions. Both are written back into state.json below. + const secrets: State["secrets"] = { ...(state.secrets ?? {}) }; + const translatorEncryptionKey = + secrets.translator_encryption_key ?? randomBytes(32).toString("hex"); + secrets.translator_encryption_key = translatorEncryptionKey; + const bazarrFlaskSecret = secrets.bazarr_flask_secret ?? generateApiKey(); + secrets.bazarr_flask_secret = bazarrFlaskSecret; // Step 1: Create storage layout (primary root + tv/movies subdirs inside each // extra path so reconfigures that add drives also get their layout). @@ -313,7 +321,7 @@ export async function runInstall( username: state.admin.username, passwordHash: bazarrHash, bazarrApiKey: apiKeys["bazarr"], - flaskSecretKey: generateApiKey(), + flaskSecretKey: bazarrFlaskSecret, sonarrApiKey: apiKeys["sonarr"] ?? "", radarrApiKey: apiKeys["radarr"] ?? "", translatorEncryptionKey, @@ -384,6 +392,7 @@ export async function runInstall( writeState(installDir, { ...state, api_keys: apiKeys, + secrets, install_started_at: installStartedAt, }); // Persist the admin password early (mode 600). state.json never stores it, @@ -661,6 +670,7 @@ export async function runInstall( const finalState: State = { ...state, api_keys: apiKeys, + secrets, install_started_at: installStartedAt, install_completed_at: new Date().toISOString(), last_updated_at: new Date().toISOString(), diff --git a/src/usecase/update.ts b/src/usecase/update.ts index ad94c84..023956e 100644 --- a/src/usecase/update.ts +++ b/src/usecase/update.ts @@ -166,7 +166,20 @@ async function regenerateInstallerConfig( state: State, log: (line: string) => void ): Promise { - const services = getServicesByIds(state.services_enabled); + // Existing VPN installs from before the reboot fix don't list deunhealth in + // services_enabled. Inject it here so `arrstack update` deploys the sidecar + // that auto-restarts qBittorrent after a reboot (otherwise qbit would carry + // the restart label but have no watcher). Idempotent: skipped if present. + const enabledIds = [...state.services_enabled]; + if ( + state.vpn.enabled && + enabledIds.includes("qbittorrent") && + !enabledIds.includes("deunhealth") + ) { + enabledIds.push("deunhealth"); + log("[update] adding deunhealth sidecar (auto-restarts qBittorrent after a reboot)"); + } + const services = getServicesByIds(enabledIds); const composePath = join(installDir, "docker-compose.yml"); diff --git a/src/version.ts b/src/version.ts index f5da581..9db5e41 100644 --- a/src/version.ts +++ b/src/version.ts @@ -1 +1 @@ -export const VERSION = "1.1.0"; +export const VERSION = "1.1.1"; diff --git a/src/wiring/qbittorrent.ts b/src/wiring/qbittorrent.ts index 8e2adc9..81c76db 100644 --- a/src/wiring/qbittorrent.ts +++ b/src/wiring/qbittorrent.ts @@ -80,6 +80,14 @@ export async function configureQbit( max_ratio_enabled: false, max_seeding_time_enabled: false, utp_rate_limited: true, + // Disable the OS disk cache. qBittorrent 5.x ships on libtorrent 2.0, which + // does disk I/O via memory-mapped files; Linux counts the page-cache pages + // backing those mmaps against the container's RSS, so "memory use" climbs + // to many GB when seeding lots of data and can OOM a small Docker VM. + // Bypassing the OS cache keeps qBittorrent's footprint bounded (the single + // most effective lever for this; the old `disk_cache` knob is a no-op on + // libtorrent 2.0). Trades a little throughput for predictable RAM. + enable_os_cache: false, }; const prefsRes = await fetch(`${base}/api/v2/app/setPreferences`, { diff --git a/templates/Caddyfile.hbs b/templates/Caddyfile.hbs index 56fc6d6..223f275 100644 --- a/templates/Caddyfile.hbs +++ b/templates/Caddyfile.hbs @@ -57,3 +57,15 @@ http://{{id}}.{{../localDnsTld}} { } } {{/if}} +{{#unless localDnsEnabled}}{{#if (eq mode "none")}} +# LAN-only with no hostname routing: there are no vhosts to reverse-proxy. +# Without at least one site block the Caddyfile renders to zero bytes, and +# `caddy run` exits with "adapting config: EOF" (code 1); under +# restart: unless-stopped that becomes a permanent crash-loop. Emit a tiny +# always-valid site so Caddy has a parseable config, keeps a listener on :80 +# (so its healthcheck passes), and stays up. Services are still reached +# directly on their own host ports; this block just serves a hint. +:80 { + respond "arrstack is running. No reverse-proxy is configured (LAN-only, no local DNS), so reach each service directly on its host port." 200 +} +{{/if}}{{/unless}} diff --git a/templates/compose.yml.hbs b/templates/compose.yml.hbs index 35f78e5..89b3b80 100644 --- a/templates/compose.yml.hbs +++ b/templates/compose.yml.hbs @@ -26,6 +26,12 @@ services: retries: {{healthcheck.retries}} start_period: {{healthcheck.start_period}} {{/if}} +{{#if labels}} + labels: +{{#each labels}} + - "{{this}}" +{{/each}} +{{/if}} {{#if vpnNetwork}} network_mode: "service:gluetun" {{else}} diff --git a/tests/platform/preflight.test.ts b/tests/platform/preflight.test.ts index f911495..9cfbf20 100644 --- a/tests/platform/preflight.test.ts +++ b/tests/platform/preflight.test.ts @@ -1,5 +1,33 @@ import { test, expect, describe, afterEach } from "bun:test"; import { checkPortFree } from "../../src/platform/ports.js"; +import { checkNotRoot } from "../../src/platform/preflight.js"; + +describe("checkNotRoot", () => { + const origGetuid = process.getuid; + afterEach(() => { + // @ts-expect-error restore original (may be undefined on some platforms) + process.getuid = origGetuid; + }); + + test("passes as a normal user (non-zero uid)", () => { + // @ts-expect-error stub + process.getuid = () => 1000; + const r = checkNotRoot(); + expect(r.ok).toBe(true); + expect(r.blocking).toBe(false); + expect(r.name).toBe("Not running as root"); + }); + + test("fails (warning) when uid is 0, and never blocks", () => { + // @ts-expect-error stub + process.getuid = () => 0; + const r = checkNotRoot(); + expect(r.ok).toBe(false); + // A root warning must never hard-block the install. + expect(r.blocking).toBe(false); + expect(r.message.toLowerCase()).toContain("root"); + }); +}); describe("checkPortFree", () => { const servers: ReturnType[] = []; diff --git a/tests/renderer/caddy.test.ts b/tests/renderer/caddy.test.ts index bd4784d..8712765 100644 --- a/tests/renderer/caddy.test.ts +++ b/tests/renderer/caddy.test.ts @@ -5,10 +5,29 @@ import { getServicesByIds } from "../../src/catalog"; const services = getServicesByIds(["sonarr", "radarr", "jellyfin"]); describe("renderCaddyfile", () => { - test("LAN mode without local DNS emits an empty Caddyfile (direct IP access)", () => { + test("LAN mode without local DNS emits a minimal :80 site, not an empty file", () => { const output = renderCaddyfile(services, { mode: "none" }); + // There are no vhosts to reverse-proxy and no cert to issue... expect(output).not.toContain("reverse_proxy"); expect(output).not.toContain("tls {"); + // ...but the file must NOT be empty: a zero-byte Caddyfile makes + // `caddy run` exit with "adapting config: EOF" and crash-loop under + // restart: unless-stopped. A tiny always-valid :80 site keeps it up. + expect(output.trim().length).toBeGreaterThan(0); + expect(output).toContain(":80 {"); + expect(output).toContain("respond "); + }); + + test("the :80 fallback appears ONLY when there are no vhosts", () => { + // With local DNS the http vhosts are the config, so no fallback needed. + const withDns = renderCaddyfile(services, { + mode: "none", + localDns: { enabled: true, tld: "arrstack.local" }, + }); + expect(withDns).not.toContain(":80 {"); + // In remote modes the wildcard block is the config, so no fallback. + const remote = renderCaddyfile(services, { mode: "duckdns", domain: "x.duckdns.org" }); + expect(remote).not.toContain(":80 {"); }); test("LAN mode WITH local DNS emits hostname vhosts on HTTP", () => { diff --git a/tests/renderer/compose.test.ts b/tests/renderer/compose.test.ts index 130fccb..2f15544 100644 --- a/tests/renderer/compose.test.ts +++ b/tests/renderer/compose.test.ts @@ -270,3 +270,65 @@ describe("renderCompose", () => { expect(output).toContain("44"); }); }); + +describe("healthchecks + deunhealth reboot self-heal", () => { + test("http services get a curl-or-wget healthcheck from the catalog", () => { + const ctx = buildComposeContext(getServices(["sonarr"]), baseOpts); + const sonarr = ctx.services.find((s) => s.id === "sonarr")!; + expect(sonarr.healthcheck).toBeDefined(); + // Portable probe: some arr/media images ship only curl, some only wget. + expect(sonarr.healthcheck!.test).toContain("curl -fsS"); + expect(sonarr.healthcheck!.test).toContain("wget -q"); + expect(sonarr.healthcheck!.test).toContain("http://127.0.0.1:8989/ping"); + }); + + test("qBittorrent gets a healthcheck (needed for deunhealth to detect the orphaned netns)", () => { + const ctx = buildComposeContext(getServices(["qbittorrent"]), baseOpts); + const qbit = ctx.services.find((s) => s.id === "qbittorrent")!; + expect(qbit.healthcheck).toBeDefined(); + expect(qbit.healthcheck!.test).toContain("http://127.0.0.1:8080/"); + }); + + test("tcp/port-0 catalog health blocks are NOT rendered (caddy, recyclarr)", () => { + const ctx = buildComposeContext(getServices(["caddy", "recyclarr"]), baseOpts); + expect(ctx.services.find((s) => s.id === "caddy")!.healthcheck).toBeUndefined(); + expect(ctx.services.find((s) => s.id === "recyclarr")!.healthcheck).toBeUndefined(); + }); + + test("VPN on: qBittorrent is labeled for deunhealth auto-restart", () => { + const opts = { ...baseOpts, vpn: { enabled: true, provider: "mullvad" } }; + const ctx = buildComposeContext(getServices(["gluetun", "qbittorrent", "deunhealth"]), opts); + const qbit = ctx.services.find((s) => s.id === "qbittorrent")!; + expect(qbit.labels).toContain("deunhealth.restart.on.unhealthy=true"); + expect(renderCompose(getServices(["gluetun", "qbittorrent", "deunhealth"]), opts)).toContain( + "deunhealth.restart.on.unhealthy=true", + ); + }); + + test("VPN off: qBittorrent carries no deunhealth label (nothing to auto-restart)", () => { + const ctx = buildComposeContext(getServices(["qbittorrent"]), baseOpts); + const qbit = ctx.services.find((s) => s.id === "qbittorrent")!; + expect(qbit.labels).not.toContain("deunhealth.restart.on.unhealthy=true"); + }); + + test("deunhealth gets the docker socket (read-only) so it can restart containers", () => { + const ctx = buildComposeContext(getServices(["deunhealth"]), baseOpts); + const dh = ctx.services.find((s) => s.id === "deunhealth")!; + expect(dh.dataMounts).toContainEqual({ + src: "/var/run/docker.sock", + dst: "/var/run/docker.sock", + mode: "ro", + }); + // No other service should be handed the daemon socket. + const sonarr = buildComposeContext(getServices(["sonarr"]), baseOpts).services[0]; + expect(sonarr.dataMounts.some((m) => m.src.includes("docker.sock"))).toBe(false); + }); + + test("every service renders restart: unless-stopped (the reboot recovery floor)", () => { + const ids = ["qbittorrent", "gluetun", "sonarr", "caddy", "deunhealth"]; + const opts = { ...baseOpts, vpn: { enabled: true, provider: "mullvad" } }; + const output = renderCompose(getServices(ids), opts); + // One "restart: unless-stopped" per service. + expect(output.match(/restart: unless-stopped/g)?.length).toBe(ids.length); + }); +}); diff --git a/tests/state/store.test.ts b/tests/state/store.test.ts index 6c4df5a..17fd877 100644 --- a/tests/state/store.test.ts +++ b/tests/state/store.test.ts @@ -1,8 +1,8 @@ import { test, expect, afterEach } from "bun:test"; -import { mkdirSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { mkdirSync, rmSync, statSync, writeFileSync, chmodSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; -import { readState, writeState } from "../../src/state/store.js"; +import { readState, writeState, adminTxtUnreadable } from "../../src/state/store.js"; import type { State } from "../../src/state/schema.js"; const TMP = join(tmpdir(), `arrstack-state-test-${process.pid}`); @@ -24,6 +24,7 @@ const VALID_STATE: State = { pgid: 1000, subtitle_languages: ["en"], api_keys: {}, + secrets: {}, }; afterEach(() => { @@ -64,3 +65,34 @@ test("readState rejects state missing required fields", () => { writeFileSync(join(TMP, "state.json"), JSON.stringify({ schema_version: 1 })); expect(() => readState(TMP)).toThrow(); }); + +test("readState backfills secrets:{} for a state.json written before the field existed", () => { + mkdirSync(TMP, { recursive: true }); + const { secrets: _drop, ...legacy } = VALID_STATE as State & { secrets?: unknown }; + writeFileSync(join(TMP, "state.json"), JSON.stringify(legacy)); + const result = readState(TMP); + expect(result?.secrets).toEqual({}); +}); + +test("adminTxtUnreadable: false when admin.txt is absent (fresh install)", () => { + mkdirSync(TMP, { recursive: true }); + expect(adminTxtUnreadable(TMP)).toBe(false); +}); + +test("adminTxtUnreadable: false when admin.txt is readable", () => { + mkdirSync(TMP, { recursive: true }); + writeFileSync(join(TMP, "admin.txt"), "password: hunter2\n"); + expect(adminTxtUnreadable(TMP)).toBe(false); +}); + +test("adminTxtUnreadable: true when admin.txt exists but can't be read", () => { + // Root bypasses file permissions, so a chmod-000 file is still readable and + // this case can't be exercised. Skip rather than assert a false negative. + if (typeof process.getuid === "function" && process.getuid() === 0) return; + mkdirSync(TMP, { recursive: true }); + const p = join(TMP, "admin.txt"); + writeFileSync(p, "password: hunter2\n"); + chmodSync(p, 0o000); + expect(adminTxtUnreadable(TMP)).toBe(true); + chmodSync(p, 0o600); // let afterEach clean up +}); diff --git a/tests/ui/wizard/useWizardState.test.ts b/tests/ui/wizard/useWizardState.test.ts index b1385ed..dafd812 100644 --- a/tests/ui/wizard/useWizardState.test.ts +++ b/tests/ui/wizard/useWizardState.test.ts @@ -1,5 +1,23 @@ import { describe, test, expect } from "bun:test"; -import { buildStateFromWizard, type WizardState } from "../../../src/ui/wizard/useWizardState.js"; +import { + buildStateFromWizard, + safeSystemId, + type WizardState, +} from "../../../src/ui/wizard/useWizardState.js"; + +describe("safeSystemId (PUID/PGID must never be 0)", () => { + test("clamps 0 (root) to 1000 so LSIO containers don't run as root", () => { + expect(safeSystemId(0)).toBe(1000); + }); + test("clamps undefined to 1000", () => { + expect(safeSystemId(undefined)).toBe(1000); + }); + test("passes through a normal non-zero uid/gid", () => { + expect(safeSystemId(1000)).toBe(1000); + expect(safeSystemId(1001)).toBe(1001); + expect(safeSystemId(65534)).toBe(65534); + }); +}); function makeWizardState(overrides: Partial = {}): WizardState { return { @@ -294,6 +312,77 @@ describe("buildStateFromWizard", () => { expect(state.api_keys["prowlarr"]).toBeUndefined(); }); + test("reuses a persisted api_key instead of rotating it on reconfigure", () => { + const ws = makeWizardState({ + services: [{ id: "prowlarr", name: "Prowlarr", checked: true }], + }); + const state = buildStateFromWizard(ws, { + api_keys: { prowlarr: "EXISTING_KEY", bazarr: "BZ_KEY" }, + }); + // Rotating this would break Prowlarr<->arr cross-references. + expect(state.api_keys["prowlarr"]).toBe("EXISTING_KEY"); + // Keys minted outside the catalog (Bazarr, no apiKeyEnv) carry over too. + expect(state.api_keys["bazarr"]).toBe("BZ_KEY"); + }); + + test("still generates a fresh key for a newly-added apiKeyEnv service", () => { + const state = buildStateFromWizard( + makeWizardState({ services: [{ id: "prowlarr", name: "Prowlarr", checked: true }] }), + { api_keys: {} }, // resume with no prowlarr key yet + ); + expect(typeof state.api_keys["prowlarr"]).toBe("string"); + expect(state.api_keys["prowlarr"].length).toBeGreaterThan(0); + }); + + test("carries persisted long-lived secrets (AES + flask) forward", () => { + const state = buildStateFromWizard(makeWizardState(), { + secrets: { translator_encryption_key: "AESKEY", bazarr_flask_secret: "FLASK" }, + }); + expect(state.secrets?.translator_encryption_key).toBe("AESKEY"); + expect(state.secrets?.bazarr_flask_secret).toBe("FLASK"); + }); + + test("secrets defaults to an empty object on a fresh install", () => { + const state = buildStateFromWizard(makeWizardState()); + expect(state.secrets).toEqual({}); + }); + + test("auto-adds deunhealth when VPN routes qBittorrent (reboot self-heal)", () => { + const state = buildStateFromWizard( + makeWizardState({ + vpnMode: "gluetun", + vpnProvider: "mullvad", + vpnPrivateKey: "k", + vpnAddresses: "10.0.0.1/32", + services: [{ id: "qbittorrent", name: "qBittorrent", checked: true }], + }), + ); + expect(state.services_enabled).toContain("deunhealth"); + }); + + test("does NOT add deunhealth without VPN", () => { + const state = buildStateFromWizard( + makeWizardState({ + vpnMode: "none", + services: [{ id: "qbittorrent", name: "qBittorrent", checked: true }], + }), + ); + expect(state.services_enabled).not.toContain("deunhealth"); + }); + + test("does NOT add deunhealth when qBittorrent isn't enabled", () => { + const state = buildStateFromWizard( + makeWizardState({ + vpnMode: "gluetun", + vpnProvider: "mullvad", + vpnPrivateKey: "k", + vpnAddresses: "10.0.0.1/32", + services: [{ id: "sonarr", name: "Sonarr", checked: true }], + }), + ); + expect(state.services_enabled).not.toContain("deunhealth"); + }); + test("installer_version is a non-empty string", () => { const state = buildStateFromWizard(makeWizardState()); expect(typeof state.installer_version).toBe("string"); diff --git a/tests/usecase/update.test.ts b/tests/usecase/update.test.ts index 73a2a5a..19d999f 100644 --- a/tests/usecase/update.test.ts +++ b/tests/usecase/update.test.ts @@ -142,6 +142,35 @@ test("runUpdate logs warning and continues when a service health check fails", a expect(contents).toContain("did not become healthy"); }); +test("runUpdate injects the deunhealth sidecar into an existing VPN install's compose", async () => { + // Simulates a v1.1.0 VPN install whose state.json predates the reboot fix: + // services_enabled has qbittorrent + gluetun but no deunhealth. `arrstack + // update` must add the watcher so existing users get the fix without a fresh + // install. mullvad + a real key means resolveVpnWireguardKey makes no API call. + const dir = makeInstallDir(true, false); + writeState(dir, { + ...VALID_STATE, + install_dir: dir, + services_enabled: ["gluetun", "qbittorrent", "sonarr"], + vpn: { + enabled: true, + provider: "mullvad", + type: "wireguard", + private_key: "KEY==", + addresses: "10.0.0.2/32", + }, + }); + const { deps } = makeDeps(); + + await silently(() => runUpdate(dir, deps)); + + const compose = readFileSync(join(dir, "docker-compose.yml"), "utf-8"); + expect(compose).toContain("deunhealth:"); + expect(compose).toContain("deunhealth.restart.on.unhealthy=true"); + const logTxt = readFileSync(join(dir, "update.log"), "utf-8"); + expect(logTxt).toContain("adding deunhealth sidecar"); +}); + test("runUpdate summary reports changed service images", async () => { const dir = makeInstallDir(); let captureCount = 0; diff --git a/tests/wiring/qbittorrent.test.ts b/tests/wiring/qbittorrent.test.ts index 4a1e7b5..894d4da 100644 --- a/tests/wiring/qbittorrent.test.ts +++ b/tests/wiring/qbittorrent.test.ts @@ -67,4 +67,25 @@ describe("configureQbit login handling (qBittorrent 5.x)", () => { /incorrect username or password/, ); }); + + test("sends enable_os_cache:false to bound libtorrent-2.0 RAM growth", async () => { + let prefsJson: any = null; + globalThis.fetch = (async (url: any, init: any) => { + const u = String(url); + if (u.endsWith("/auth/login")) { + return new Response("", { + status: 204, + headers: { "set-cookie": "QBT_SID_8080=c; path=/" }, + }); + } + if (u.includes("setPreferences")) { + const body = new URLSearchParams(String(init?.body)); + prefsJson = JSON.parse(body.get("json") ?? "{}"); + } + return new Response("Ok.", { status: 200 }); + }) as any; + await configureQbit("admin", "pw", "http://qbit.test"); + expect(prefsJson).not.toBeNull(); + expect(prefsJson.enable_os_cache).toBe(false); + }); }); From 05e755286bbc0ad66584508fe4f7f903676a64a1 Mon Sep 17 00:00:00 2001 From: Laszlo Toth Date: Mon, 6 Jul 2026 11:05:52 +0200 Subject: [PATCH 2/4] fix: address Codex review on PR #2 - P1: VPN-routed qBittorrent healthcheck now probes tunnel connectivity (cp.cloudflare.com/generate_204) instead of loopback 8080. A loopback probe passes even when qbit is orphaned from gluetun's netns after a reboot, so deunhealth would never restart it; the tunnel probe only succeeds in gluetun's live netns, which is exactly when the published WebUI port also routes. - P2: arrstack doctor no longer fails on a non-blocking preflight check. The root-user warning (blocking: false) prints as a warning, not a failing exit. - P2: apply the unreadable-admin.txt guard to bare `arrstack` (the default TUI entrypoint) too, not just `arrstack install`, via a shared helper. - P2: on upgrade, backfill the translator ENCRYPTION_KEY and Bazarr flask secret from the existing .env / Bazarr config before generating new ones, so the first reconfigure after upgrading doesn't rotate keys and orphan data encrypted at rest. 301 tests pass, typecheck clean. --- src/cli.ts | 34 ++++++++++++++++---------- src/renderer/compose.ts | 27 ++++++++++++++++++++- src/usecase/doctor.ts | 8 +++++++ src/usecase/install.ts | 32 ++++++++++++++++++++++--- tests/renderer/compose.test.ts | 14 ++++++++++- tests/usecase/install.test.ts | 44 ++++++++++++++++++++++++++++++++-- 6 files changed, 139 insertions(+), 20 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 706bc09..a68c613 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -16,6 +16,23 @@ import { exec } from "./lib/exec.js"; import { existsSync } from "node:fs"; import { join } from "node:path"; +// A previous `sudo` install can leave admin.txt root-owned; a later non-sudo +// run then can't read it, would generate a NEW admin password, rewrite +// qBittorrent.conf, and lock the user out of the still-running qbit container. +// Stop early with a fix instead of silently rotating. Applies to every +// entrypoint that launches the wizard against an existing install (both +// `arrstack install` and bare `arrstack`). +function guardAdminTxtReadable(installDir: string): void { + if (!adminTxtUnreadable(installDir)) return; + const adminTxt = join(installDir, "admin.txt"); + console.error( + `\nCannot read ${adminTxt}. It looks like a previous 'sudo arrstack install' left it owned by root.\n` + + `Fix ownership and re-run as your normal user (not sudo):\n` + + ` sudo chown $(id -u):$(id -g) ${adminTxt}\n` + ); + process.exit(1); +} + const program = new Command(); program @@ -50,19 +67,9 @@ program console.log(`Removed: ${path}`); } } - // A previous `sudo` install can leave admin.txt root-owned; a later - // non-sudo run then can't read it, would generate a NEW admin password, - // rewrite qBittorrent.conf, and lock the user out of the still-running qbit - // container. Stop early with a fix instead of silently rotating. - if (!opts.fresh && adminTxtUnreadable(installDir)) { - const adminTxt = join(installDir, "admin.txt"); - console.error( - `\nCannot read ${adminTxt}. It looks like a previous 'sudo arrstack install' left it owned by root.\n` + - `Fix ownership and re-run as your normal user (not sudo):\n` + - ` sudo chown $(id -u):$(id -g) ${adminTxt}\n` - ); - process.exit(1); - } + // --fresh purges the dir (removing admin.txt) just above, so only guard a + // resume/reconfigure over an existing install. + if (!opts.fresh) guardAdminTxtReadable(installDir); const existing = opts.fresh ? null : readState(installDir); const { waitUntilExit } = render(React.createElement(App, { existingState: existing })); await waitUntilExit(); @@ -142,6 +149,7 @@ program // If no subcommand provided (just `arrstack`), launch the TUI program.action(async () => { const installDir = `${process.env.HOME}/arrstack`; + guardAdminTxtReadable(installDir); const existing = readState(installDir); const { waitUntilExit } = render(React.createElement(App, { existingState: existing })); await waitUntilExit(); diff --git a/src/renderer/compose.ts b/src/renderer/compose.ts index 609fe63..b29fdc3 100644 --- a/src/renderer/compose.ts +++ b/src/renderer/compose.ts @@ -230,6 +230,28 @@ function buildHealthcheck(svc: Service): Healthcheck | undefined { }; } +// Healthcheck for a VPN-routed service (qBittorrent inside gluetun's netns). +// A loopback WebUI probe is the WRONG signal here: after a reboot the container +// can be orphaned from gluetun's netns with its published port dead, yet its +// own 127.0.0.1:8080 still answers, so Docker keeps it `healthy` and deunhealth +// never restarts it. Probe external connectivity through the tunnel instead: it +// succeeds ONLY when the container sits in gluetun's live netns, which is +// exactly when the published WebUI port also routes correctly. Plain HTTP + a +// 204 endpoint so the busybox-wget fallback works without TLS; lenient timing +// (5 x 30s after a 60s grace) so a brief VPN reconnect doesn't trigger a +// needless restart while a genuine orphan still self-heals within a few minutes. +const VPN_CONNECTIVITY_URL = "http://cp.cloudflare.com/generate_204"; +function buildVpnHealthcheck(): Healthcheck { + const u = VPN_CONNECTIVITY_URL; + return { + test: `["CMD-SHELL", "curl -fsS -m 10 -o /dev/null ${u} || wget -q -T 10 -O /dev/null ${u} || exit 1"]`, + interval: "30s", + timeout: "15s", + retries: 5, + start_period: "60s", + }; +} + // Translates the wizard's VPN state into the env vars gluetun expects // (VPN_SERVICE_PROVIDER / VPN_TYPE / WIREGUARD_* / SERVER_COUNTRIES / custom // endpoint tuple). Only emitted for the gluetun service and only when VPN @@ -293,7 +315,10 @@ export function buildComposeContext(services: Service[], opts: ComposeOptions): } const dependsOnLongForm = dependsOn.some((d) => d.condition !== undefined); - const healthcheck = buildHealthcheck(svc); + // VPN-routed services need a tunnel-connectivity probe (see buildVpnHealthcheck) + // so deunhealth can actually detect the reboot orphaned-netns state; a plain + // loopback probe would pass even when the published port is dead. + const healthcheck = vpnNetwork ? buildVpnHealthcheck() : buildHealthcheck(svc); // Only qBittorrent is auto-restarted by deunhealth. On a host reboot Docker // ignores depends_on ordering, so qbit (network_mode: service:gluetun) can diff --git a/src/usecase/doctor.ts b/src/usecase/doctor.ts index 9231d01..f12502a 100644 --- a/src/usecase/doctor.ts +++ b/src/usecase/doctor.ts @@ -21,6 +21,10 @@ function fail(label: string, msg: string, hint?: string) { } } +function warn(label: string, msg: string) { + console.log(` ${YELLOW}⚠${RESET} ${label}: ${msg}`); +} + function section(title: string) { console.log(`\n${BOLD}${title}${RESET}`); } @@ -69,6 +73,10 @@ export async function runDoctor(installDir: string): Promise { for (const check of preflightResults) { if (check.ok) { pass(check.name, check.message); + } else if (!check.blocking) { + // Advisory only (e.g. running as root): surface it but don't flip doctor + // to a failing exit, since checkNotRoot() marks it blocking: false. + warn(check.name, check.message); } else { allOk = false; const hint = remediationHint(check); diff --git a/src/usecase/install.ts b/src/usecase/install.ts index e5d42eb..e6bdd09 100644 --- a/src/usecase/install.ts +++ b/src/usecase/install.ts @@ -1,4 +1,4 @@ -import { writeFileSync, mkdirSync, chownSync } from "node:fs"; +import { writeFileSync, readFileSync, mkdirSync, chownSync } from "node:fs"; import { join } from "node:path"; import { randomBytes } from "node:crypto"; @@ -148,6 +148,19 @@ function normalizeState(state: State): State { return state; } +// Recover a secret already written into an installer-owned file (.env, +// Bazarr config.yaml) so an upgrade to a version that persists it in state.json +// doesn't rotate it. Returns the trimmed first capture group, or undefined if +// the file is absent/unreadable or the pattern doesn't match. +export function readSecretFromFile(path: string, re: RegExp): string | undefined { + try { + const m = readFileSync(path, "utf-8").match(re); + return m?.[1]?.trim() || undefined; + } catch { + return undefined; + } +} + export async function runInstall( rawState: State, adminPassword: string, @@ -175,11 +188,24 @@ export async function runInstall( // the .env (-> ai-subtitle-translator) and Bazarr's config.yaml so their // encryption matches (rotating it orphans data encrypted at rest); the Flask // secret backs Bazarr's sessions. Both are written back into state.json below. + // Precedence: the value persisted in state.json (v1.1.1+) wins; otherwise, for + // an install created before the secrets field existed, recover the key already + // baked into the .env / Bazarr config so we do NOT rotate it (rotating orphans + // everything Bazarr and the translator encrypted at rest). Only mint a new one + // on a genuinely fresh install where neither exists. const secrets: State["secrets"] = { ...(state.secrets ?? {}) }; const translatorEncryptionKey = - secrets.translator_encryption_key ?? randomBytes(32).toString("hex"); + secrets.translator_encryption_key ?? + readSecretFromFile(join(installDir, ".env"), /^ENCRYPTION_KEY=(.+)$/m) ?? + randomBytes(32).toString("hex"); secrets.translator_encryption_key = translatorEncryptionKey; - const bazarrFlaskSecret = secrets.bazarr_flask_secret ?? generateApiKey(); + const bazarrFlaskSecret = + secrets.bazarr_flask_secret ?? + readSecretFromFile( + join(installDir, "config", "bazarr", "config", "config.yaml"), + /flask_secret_key:\s*(.+)/, + ) ?? + generateApiKey(); secrets.bazarr_flask_secret = bazarrFlaskSecret; // Step 1: Create storage layout (primary root + tv/movies subdirs inside each diff --git a/tests/renderer/compose.test.ts b/tests/renderer/compose.test.ts index 2f15544..b39694f 100644 --- a/tests/renderer/compose.test.ts +++ b/tests/renderer/compose.test.ts @@ -282,13 +282,25 @@ describe("healthchecks + deunhealth reboot self-heal", () => { expect(sonarr.healthcheck!.test).toContain("http://127.0.0.1:8989/ping"); }); - test("qBittorrent gets a healthcheck (needed for deunhealth to detect the orphaned netns)", () => { + test("non-VPN qBittorrent uses the loopback WebUI probe", () => { const ctx = buildComposeContext(getServices(["qbittorrent"]), baseOpts); const qbit = ctx.services.find((s) => s.id === "qbittorrent")!; expect(qbit.healthcheck).toBeDefined(); expect(qbit.healthcheck!.test).toContain("http://127.0.0.1:8080/"); }); + test("VPN-routed qBittorrent probes tunnel connectivity, not loopback", () => { + // A loopback WebUI probe passes even when qbit is orphaned from gluetun's + // netns after a reboot (its own 127.0.0.1 still answers), so deunhealth + // would never fire. The tunnel probe only succeeds in gluetun's live netns. + const opts = { ...baseOpts, vpn: { enabled: true, provider: "mullvad" } }; + const ctx = buildComposeContext(getServices(["gluetun", "qbittorrent"]), opts); + const qbit = ctx.services.find((s) => s.id === "qbittorrent")!; + expect(qbit.healthcheck).toBeDefined(); + expect(qbit.healthcheck!.test).toContain("cp.cloudflare.com/generate_204"); + expect(qbit.healthcheck!.test).not.toContain("127.0.0.1:8080"); + }); + test("tcp/port-0 catalog health blocks are NOT rendered (caddy, recyclarr)", () => { const ctx = buildComposeContext(getServices(["caddy", "recyclarr"]), baseOpts); expect(ctx.services.find((s) => s.id === "caddy")!.healthcheck).toBeUndefined(); diff --git a/tests/usecase/install.test.ts b/tests/usecase/install.test.ts index 1f1e737..f82d0a3 100644 --- a/tests/usecase/install.test.ts +++ b/tests/usecase/install.test.ts @@ -1,5 +1,8 @@ -import { describe, test, expect } from "bun:test"; -import { getHostIp } from "../../src/usecase/install.js"; +import { describe, test, expect, afterEach } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { getHostIp, readSecretFromFile } from "../../src/usecase/install.js"; import type { StepUpdate } from "../../src/usecase/install.js"; // Import the unexported runStep via a local re-implementation for isolation @@ -35,6 +38,43 @@ async function callStep( return { updates, threw }; } +describe("readSecretFromFile (legacy secret migration)", () => { + let dir = ""; + afterEach(() => { + if (dir) rmSync(dir, { recursive: true, force: true }); + dir = ""; + }); + + test("recovers ENCRYPTION_KEY from an existing .env so an upgrade doesn't rotate it", () => { + dir = mkdtempSync(join(tmpdir(), "arrstack-secret-")); + writeFileSync(join(dir, ".env"), "PUID=1000\nENCRYPTION_KEY=deadbeefcafe1234\nTZ=UTC\n"); + expect(readSecretFromFile(join(dir, ".env"), /^ENCRYPTION_KEY=(.+)$/m)).toBe( + "deadbeefcafe1234", + ); + }); + + test("recovers flask_secret_key from an existing Bazarr config.yaml", () => { + dir = mkdtempSync(join(tmpdir(), "arrstack-secret-")); + const cfg = join(dir, "config", "bazarr", "config"); + mkdirSync(cfg, { recursive: true }); + writeFileSync(join(cfg, "config.yaml"), "general:\n flask_secret_key: abc123secret\n"); + expect( + readSecretFromFile(join(cfg, "config.yaml"), /flask_secret_key:\s*(.+)/), + ).toBe("abc123secret"); + }); + + test("returns undefined when the file is missing (fresh install)", () => { + dir = mkdtempSync(join(tmpdir(), "arrstack-secret-")); + expect(readSecretFromFile(join(dir, ".env"), /^ENCRYPTION_KEY=(.+)$/m)).toBeUndefined(); + }); + + test("returns undefined when the pattern doesn't match", () => { + dir = mkdtempSync(join(tmpdir(), "arrstack-secret-")); + writeFileSync(join(dir, ".env"), "PUID=1000\nTZ=UTC\n"); + expect(readSecretFromFile(join(dir, ".env"), /^ENCRYPTION_KEY=(.+)$/m)).toBeUndefined(); + }); +}); + describe("getHostIp", () => { test("returns a non-empty string", async () => { const ip = await getHostIp(); From 8822c64aadd8a270be584201fa819ad8afba2c14 Mon Sep 17 00:00:00 2001 From: Laszlo Toth Date: Mon, 6 Jul 2026 11:31:50 +0200 Subject: [PATCH 3/4] fix: address 3-agent code review (VPN probe, deunhealth persistence, secret regex, tests) A multi-agent review of the v1.1.1 branch surfaced these (all low/medium, no criticals): - VPN qbit healthcheck now probes gluetun's control server (127.0.0.1:8000, via the shared netns) instead of a third-party host. The old external probe could not tell an orphaned netns from a real tunnel/WAN outage, so deunhealth would restart-loop qbit during a genuine outage. The control server is up whenever the gluetun container is up, independent of the tunnel, so a real outage no longer churns qbit; -f is omitted so an auth 401 still counts as reachable. Verified reachable from the running qbit container. - arrstack update now persists deunhealth into state.services_enabled, not just the rendered compose, so doctor can verify the sidecar and later updates don't re-add/re-log it (state/compose drift). - Harden the Bazarr flask_secret migration regex: anchored, same-line value, strips optional quotes, so a re-serialized quoted/empty/commented key falls through to generation instead of capturing the wrong text. - Tests: cover the doctor warn-vs-fail path (classifyCheck), the admin.txt guard message (adminTxtGuardMessage), flask regex edge cases, and the update no-re-add path; reframe the tautological restart test into a real reboot-recovery-trio assertion. 310 tests pass, typecheck clean. --- src/cli.ts | 15 ++++++--------- src/renderer/compose.ts | 33 ++++++++++++++++++--------------- src/state/store.ts | 13 +++++++++++++ src/usecase/doctor.ts | 15 +++++++++++++-- src/usecase/install.ts | 6 +++++- src/usecase/update.ts | 7 ++++++- tests/renderer/compose.test.ts | 24 ++++++++++++++++-------- tests/state/store.test.ts | 27 ++++++++++++++++++++++++++- tests/usecase/doctor.test.ts | 23 +++++++++++++++++++++++ tests/usecase/install.test.ts | 32 ++++++++++++++++++++++++++++---- tests/usecase/update.test.ts | 21 ++++++++++++++++++++- 11 files changed, 174 insertions(+), 42 deletions(-) create mode 100644 tests/usecase/doctor.test.ts diff --git a/src/cli.ts b/src/cli.ts index a68c613..e7f1ac2 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -4,7 +4,7 @@ import { VERSION } from "./version.js"; import { render } from "ink"; import React from "react"; import { App } from "./ui/App.js"; -import { readState, adminTxtUnreadable } from "./state/store.js"; +import { readState, adminTxtGuardMessage } from "./state/store.js"; import { runDoctor } from "./usecase/doctor.js"; import { runUpdate } from "./usecase/update.js"; import { showPassword } from "./usecase/show-password.js"; @@ -23,14 +23,11 @@ import { join } from "node:path"; // entrypoint that launches the wizard against an existing install (both // `arrstack install` and bare `arrstack`). function guardAdminTxtReadable(installDir: string): void { - if (!adminTxtUnreadable(installDir)) return; - const adminTxt = join(installDir, "admin.txt"); - console.error( - `\nCannot read ${adminTxt}. It looks like a previous 'sudo arrstack install' left it owned by root.\n` + - `Fix ownership and re-run as your normal user (not sudo):\n` + - ` sudo chown $(id -u):$(id -g) ${adminTxt}\n` - ); - process.exit(1); + const msg = adminTxtGuardMessage(installDir); + if (msg) { + console.error(`\n${msg}\n`); + process.exit(1); + } } const program = new Command(); diff --git a/src/renderer/compose.ts b/src/renderer/compose.ts index b29fdc3..4a86e46 100644 --- a/src/renderer/compose.ts +++ b/src/renderer/compose.ts @@ -231,24 +231,27 @@ function buildHealthcheck(svc: Service): Healthcheck | undefined { } // Healthcheck for a VPN-routed service (qBittorrent inside gluetun's netns). -// A loopback WebUI probe is the WRONG signal here: after a reboot the container -// can be orphaned from gluetun's netns with its published port dead, yet its -// own 127.0.0.1:8080 still answers, so Docker keeps it `healthy` and deunhealth -// never restarts it. Probe external connectivity through the tunnel instead: it -// succeeds ONLY when the container sits in gluetun's live netns, which is -// exactly when the published WebUI port also routes correctly. Plain HTTP + a -// 204 endpoint so the busybox-wget fallback works without TLS; lenient timing -// (5 x 30s after a 60s grace) so a brief VPN reconnect doesn't trigger a -// needless restart while a genuine orphan still self-heals within a few minutes. -const VPN_CONNECTIVITY_URL = "http://cp.cloudflare.com/generate_204"; +// A loopback WebUI probe is the WRONG signal: after a reboot the container can +// be orphaned from gluetun's netns with its published port dead, yet its own +// 127.0.0.1:8080 still answers, so Docker keeps it `healthy` and deunhealth +// never restarts it. Probe gluetun's control server instead. Because qbit shares +// gluetun's netns it can reach the control server on 127.0.0.1:8000, but ONLY +// while it sits in gluetun's LIVE netns; an orphaned/stale netns has no such +// listener, so the probe fails and deunhealth restarts qbit. Crucially the +// control server is up whenever the gluetun CONTAINER is up, independent of the +// tunnel/WAN, so a real VPN outage does NOT restart-loop qbit (unlike an +// external connectivity check), and there's no third-party dependency. `-f` is +// omitted so an auth 401 still counts as reachable; only a refused connection +// (orphaned netns) fails. curl is used directly (LSIO qbit ships it) since +// busybox wget can't distinguish a 401 from a refused connection. +const GLUETUN_CONTROL_URL = "http://127.0.0.1:8000/v1/publicip/ip"; function buildVpnHealthcheck(): Healthcheck { - const u = VPN_CONNECTIVITY_URL; return { - test: `["CMD-SHELL", "curl -fsS -m 10 -o /dev/null ${u} || wget -q -T 10 -O /dev/null ${u} || exit 1"]`, + test: `["CMD-SHELL", "curl -sS -m 10 -o /dev/null ${GLUETUN_CONTROL_URL} || exit 1"]`, interval: "30s", - timeout: "15s", - retries: 5, - start_period: "60s", + timeout: "10s", + retries: 3, + start_period: "40s", }; } diff --git a/src/state/store.ts b/src/state/store.ts index 3772795..f44ba94 100644 --- a/src/state/store.ts +++ b/src/state/store.ts @@ -35,6 +35,19 @@ export function adminTxtUnreadable(installDir: string): boolean { } } +// The actionable message to print when admin.txt can't be read, or null when +// it's fine to proceed. Kept here (not in cli.ts, which self-executes on import) +// so the guard's decision is unit-testable. cli.ts prints this and exits. +export function adminTxtGuardMessage(installDir: string): string | null { + if (!adminTxtUnreadable(installDir)) return null; + const p = join(installDir, "admin.txt"); + return ( + `Cannot read ${p}. It looks like a previous 'sudo arrstack install' left it owned by root.\n` + + `Fix ownership and re-run as your normal user (not sudo):\n` + + ` sudo chown $(id -u):$(id -g) ${p}` + ); +} + export function writeState(installDir: string, state: State): void { const statePath = join(installDir, STATE_FILE); const tmpPath = `${statePath}.tmp`; diff --git a/src/usecase/doctor.ts b/src/usecase/doctor.ts index f12502a..2c2a44c 100644 --- a/src/usecase/doctor.ts +++ b/src/usecase/doctor.ts @@ -25,6 +25,16 @@ function warn(label: string, msg: string) { console.log(` ${YELLOW}⚠${RESET} ${label}: ${msg}`); } +export type CheckOutcome = "pass" | "warn" | "fail"; + +// A passing check passes; a failing but non-blocking check (e.g. the root-user +// advisory) is only a warning and must NOT fail doctor; a failing blocking +// check is a hard failure. +export function classifyCheck(check: CheckResult): CheckOutcome { + if (check.ok) return "pass"; + return check.blocking ? "fail" : "warn"; +} + function section(title: string) { console.log(`\n${BOLD}${title}${RESET}`); } @@ -71,9 +81,10 @@ export async function runDoctor(installDir: string): Promise { skipPortChecks: true, }); for (const check of preflightResults) { - if (check.ok) { + const outcome = classifyCheck(check); + if (outcome === "pass") { pass(check.name, check.message); - } else if (!check.blocking) { + } else if (outcome === "warn") { // Advisory only (e.g. running as root): surface it but don't flip doctor // to a failing exit, since checkNotRoot() marks it blocking: false. warn(check.name, check.message); diff --git a/src/usecase/install.ts b/src/usecase/install.ts index e6bdd09..6945c46 100644 --- a/src/usecase/install.ts +++ b/src/usecase/install.ts @@ -203,7 +203,11 @@ export async function runInstall( secrets.bazarr_flask_secret ?? readSecretFromFile( join(installDir, "config", "bazarr", "config", "config.yaml"), - /flask_secret_key:\s*(.+)/, + // Bazarr rewrites this file at runtime, so be strict: anchor to the line, + // require a same-line (not newline-swallowed) value, and strip optional + // surrounding quotes. An empty/quoted/commented key then falls through to + // generation rather than capturing the wrong text. + /^\s*flask_secret_key:[ \t]*["']?([^"'\s]+)["']?\s*$/m, ) ?? generateApiKey(); secrets.bazarr_flask_secret = bazarrFlaskSecret; diff --git a/src/usecase/update.ts b/src/usecase/update.ts index 023956e..c388286 100644 --- a/src/usecase/update.ts +++ b/src/usecase/update.ts @@ -1,6 +1,6 @@ import { existsSync, openSync, writeSync, closeSync, fsyncSync, writeFileSync, readFileSync } from "node:fs"; import { join } from "node:path"; -import { readState } from "../state/store.js"; +import { readState, writeState } from "../state/store.js"; import { exec } from "../lib/exec.js"; import { getServicesByIds } from "../catalog/index.js"; import type { Service } from "../catalog/schema.js"; @@ -178,6 +178,11 @@ async function regenerateInstallerConfig( ) { enabledIds.push("deunhealth"); log("[update] adding deunhealth sidecar (auto-restarts qBittorrent after a reboot)"); + // Persist so state.json matches the deployed compose. doctor derives its + // service checklist from services_enabled (so it can actually verify the + // sidecar is running), and this stops every later update from re-adding and + // re-logging it. Written before the docker steps, same as the compose file. + writeState(installDir, { ...state, services_enabled: enabledIds }); } const services = getServicesByIds(enabledIds); diff --git a/tests/renderer/compose.test.ts b/tests/renderer/compose.test.ts index b39694f..a602665 100644 --- a/tests/renderer/compose.test.ts +++ b/tests/renderer/compose.test.ts @@ -289,15 +289,19 @@ describe("healthchecks + deunhealth reboot self-heal", () => { expect(qbit.healthcheck!.test).toContain("http://127.0.0.1:8080/"); }); - test("VPN-routed qBittorrent probes tunnel connectivity, not loopback", () => { + test("VPN-routed qBittorrent probes gluetun's control server, not loopback", () => { // A loopback WebUI probe passes even when qbit is orphaned from gluetun's - // netns after a reboot (its own 127.0.0.1 still answers), so deunhealth - // would never fire. The tunnel probe only succeeds in gluetun's live netns. + // netns after a reboot (its own 127.0.0.1:8080 still answers), so deunhealth + // would never fire. Probing gluetun's control server (127.0.0.1:8000, shared + // netns) only succeeds in the LIVE netns, and unlike an external check it + // stays healthy during a real tunnel outage (no pointless restart loop). const opts = { ...baseOpts, vpn: { enabled: true, provider: "mullvad" } }; const ctx = buildComposeContext(getServices(["gluetun", "qbittorrent"]), opts); const qbit = ctx.services.find((s) => s.id === "qbittorrent")!; expect(qbit.healthcheck).toBeDefined(); - expect(qbit.healthcheck!.test).toContain("cp.cloudflare.com/generate_204"); + expect(qbit.healthcheck!.test).toContain("127.0.0.1:8000"); + // -f omitted so an auth 401 still counts as reachable (only refused fails). + expect(qbit.healthcheck!.test).not.toContain("-f"); expect(qbit.healthcheck!.test).not.toContain("127.0.0.1:8080"); }); @@ -336,11 +340,15 @@ describe("healthchecks + deunhealth reboot self-heal", () => { expect(sonarr.dataMounts.some((m) => m.src.includes("docker.sock"))).toBe(false); }); - test("every service renders restart: unless-stopped (the reboot recovery floor)", () => { - const ids = ["qbittorrent", "gluetun", "sonarr", "caddy", "deunhealth"]; + test("VPN stack renders the full reboot-recovery trio together (sidecar + label + socket)", () => { + // All three must co-render or the self-heal is a no-op: the deunhealth + // watcher, the label that arms it on qbit, and the socket that lets it + // actually restart qbit. This fails if any leg of the v1.1.1 change regresses. + const ids = ["gluetun", "qbittorrent", "deunhealth"]; const opts = { ...baseOpts, vpn: { enabled: true, provider: "mullvad" } }; const output = renderCompose(getServices(ids), opts); - // One "restart: unless-stopped" per service. - expect(output.match(/restart: unless-stopped/g)?.length).toBe(ids.length); + expect(output).toContain("deunhealth:"); + expect(output).toContain("deunhealth.restart.on.unhealthy=true"); + expect(output).toContain("/var/run/docker.sock:/var/run/docker.sock:ro"); }); }); diff --git a/tests/state/store.test.ts b/tests/state/store.test.ts index 17fd877..c2f6c65 100644 --- a/tests/state/store.test.ts +++ b/tests/state/store.test.ts @@ -2,7 +2,12 @@ import { test, expect, afterEach } from "bun:test"; import { mkdirSync, rmSync, statSync, writeFileSync, chmodSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; -import { readState, writeState, adminTxtUnreadable } from "../../src/state/store.js"; +import { + readState, + writeState, + adminTxtUnreadable, + adminTxtGuardMessage, +} from "../../src/state/store.js"; import type { State } from "../../src/state/schema.js"; const TMP = join(tmpdir(), `arrstack-state-test-${process.pid}`); @@ -96,3 +101,23 @@ test("adminTxtUnreadable: true when admin.txt exists but can't be read", () => { expect(adminTxtUnreadable(TMP)).toBe(true); chmodSync(p, 0o600); // let afterEach clean up }); + +test("adminTxtGuardMessage: null when admin.txt is absent or readable", () => { + mkdirSync(TMP, { recursive: true }); + expect(adminTxtGuardMessage(TMP)).toBeNull(); + writeFileSync(join(TMP, "admin.txt"), "password: hunter2\n"); + expect(adminTxtGuardMessage(TMP)).toBeNull(); +}); + +test("adminTxtGuardMessage: actionable chown message when admin.txt is unreadable", () => { + if (typeof process.getuid === "function" && process.getuid() === 0) return; + mkdirSync(TMP, { recursive: true }); + const p = join(TMP, "admin.txt"); + writeFileSync(p, "password: hunter2\n"); + chmodSync(p, 0o000); + const msg = adminTxtGuardMessage(TMP); + expect(msg).not.toBeNull(); + expect(msg).toContain("chown"); + expect(msg).toContain(p); + chmodSync(p, 0o600); +}); diff --git a/tests/usecase/doctor.test.ts b/tests/usecase/doctor.test.ts new file mode 100644 index 0000000..35344c8 --- /dev/null +++ b/tests/usecase/doctor.test.ts @@ -0,0 +1,23 @@ +import { describe, test, expect } from "bun:test"; +import { classifyCheck } from "../../src/usecase/doctor.js"; +import type { CheckResult } from "../../src/platform/preflight.js"; + +function check(over: Partial): CheckResult { + return { name: "c", ok: true, message: "", blocking: true, ...over }; +} + +describe("classifyCheck", () => { + test("a passing check passes", () => { + expect(classifyCheck(check({ ok: true }))).toBe("pass"); + }); + + test("a failing NON-blocking check is a warning, not a failure (root advisory)", () => { + // This is the regression the review flagged: doctor must not exit non-zero + // just because the box is running as root. + expect(classifyCheck(check({ ok: false, blocking: false }))).toBe("warn"); + }); + + test("a failing blocking check is a hard failure", () => { + expect(classifyCheck(check({ ok: false, blocking: true }))).toBe("fail"); + }); +}); diff --git a/tests/usecase/install.test.ts b/tests/usecase/install.test.ts index f82d0a3..7005e1b 100644 --- a/tests/usecase/install.test.ts +++ b/tests/usecase/install.test.ts @@ -53,14 +53,38 @@ describe("readSecretFromFile (legacy secret migration)", () => { ); }); + const FLASK_RE = /^\s*flask_secret_key:[ \t]*["']?([^"'\s]+)["']?\s*$/m; + test("recovers flask_secret_key from an existing Bazarr config.yaml", () => { dir = mkdtempSync(join(tmpdir(), "arrstack-secret-")); const cfg = join(dir, "config", "bazarr", "config"); mkdirSync(cfg, { recursive: true }); - writeFileSync(join(cfg, "config.yaml"), "general:\n flask_secret_key: abc123secret\n"); - expect( - readSecretFromFile(join(cfg, "config.yaml"), /flask_secret_key:\s*(.+)/), - ).toBe("abc123secret"); + writeFileSync(join(cfg, "config.yaml"), "general:\n flask_secret_key: abc123secret\n ip: '*'\n"); + expect(readSecretFromFile(join(cfg, "config.yaml"), FLASK_RE)).toBe("abc123secret"); + }); + + test("strips surrounding quotes if Bazarr re-serializes the flask secret quoted", () => { + dir = mkdtempSync(join(tmpdir(), "arrstack-secret-")); + const cfg = join(dir, "config", "bazarr", "config"); + mkdirSync(cfg, { recursive: true }); + writeFileSync(join(cfg, "config.yaml"), "general:\n flask_secret_key: 'deadbeef99'\n"); + expect(readSecretFromFile(join(cfg, "config.yaml"), FLASK_RE)).toBe("deadbeef99"); + }); + + test("does NOT capture the next line when the flask secret is empty (falls through to generate)", () => { + dir = mkdtempSync(join(tmpdir(), "arrstack-secret-")); + const cfg = join(dir, "config", "bazarr", "config"); + mkdirSync(cfg, { recursive: true }); + writeFileSync(join(cfg, "config.yaml"), "general:\n flask_secret_key:\n ip: '*'\n"); + expect(readSecretFromFile(join(cfg, "config.yaml"), FLASK_RE)).toBeUndefined(); + }); + + test("ignores a commented-out flask_secret_key line", () => { + dir = mkdtempSync(join(tmpdir(), "arrstack-secret-")); + const cfg = join(dir, "config", "bazarr", "config"); + mkdirSync(cfg, { recursive: true }); + writeFileSync(join(cfg, "config.yaml"), "general:\n # flask_secret_key: OLDVALUE\n flask_secret_key: realkey42\n"); + expect(readSecretFromFile(join(cfg, "config.yaml"), FLASK_RE)).toBe("realkey42"); }); test("returns undefined when the file is missing (fresh install)", () => { diff --git a/tests/usecase/update.test.ts b/tests/usecase/update.test.ts index 19d999f..ac3bcce 100644 --- a/tests/usecase/update.test.ts +++ b/tests/usecase/update.test.ts @@ -3,7 +3,7 @@ import { mkdtempSync, rmSync, writeFileSync, readFileSync, existsSync, statSync import { join } from "node:path"; import { tmpdir } from "node:os"; import { runUpdate, type UpdateDeps } from "../../src/usecase/update.js"; -import { writeState } from "../../src/state/store.js"; +import { writeState, readState } from "../../src/state/store.js"; import type { State } from "../../src/state/schema.js"; const VALID_STATE: State = { @@ -169,6 +169,25 @@ test("runUpdate injects the deunhealth sidecar into an existing VPN install's co expect(compose).toContain("deunhealth.restart.on.unhealthy=true"); const logTxt = readFileSync(join(dir, "update.log"), "utf-8"); expect(logTxt).toContain("adding deunhealth sidecar"); + // state.json must be updated too, so doctor can verify the sidecar and a + // later update doesn't re-add/re-log it (compose/state drift). + expect(readState(dir)?.services_enabled).toContain("deunhealth"); +}); + +test("runUpdate does not re-add deunhealth when it's already in services_enabled", async () => { + const dir = makeInstallDir(true, false); + writeState(dir, { + ...VALID_STATE, + install_dir: dir, + services_enabled: ["gluetun", "qbittorrent", "deunhealth"], + vpn: { enabled: true, provider: "mullvad", type: "wireguard", private_key: "K==", addresses: "10.0.0.2/32" }, + }); + const { deps } = makeDeps(); + + await silently(() => runUpdate(dir, deps)); + + const logTxt = readFileSync(join(dir, "update.log"), "utf-8"); + expect(logTxt).not.toContain("adding deunhealth sidecar"); }); test("runUpdate summary reports changed service images", async () => { From 44a1c46e14cd797212e4d1ea659ef09f3224ccbf Mon Sep 17 00:00:00 2001 From: Laszlo Toth Date: Mon, 6 Jul 2026 13:17:07 +0200 Subject: [PATCH 4/4] fix: VPN qbit healthcheck also probes its own WebUI (Codex re-review) The control-server-only probe proves qbit is in gluetun's live netns (catches the reboot orphaned-netns case) but would stay healthy if qbit's WebUI process hangs while gluetun is up, so deunhealth would never restart a hung qbit. Require BOTH legs: gluetun control server reachable AND qbit's WebUI responding. Verified both legs pass on a live VPN-routed qbit container. 310 tests pass, typecheck clean. --- src/renderer/compose.ts | 16 +++++++++++++--- tests/renderer/compose.test.ts | 16 +++++++--------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/renderer/compose.ts b/src/renderer/compose.ts index 4a86e46..18979b7 100644 --- a/src/renderer/compose.ts +++ b/src/renderer/compose.ts @@ -245,9 +245,17 @@ function buildHealthcheck(svc: Service): Healthcheck | undefined { // (orphaned netns) fails. curl is used directly (LSIO qbit ships it) since // busybox wget can't distinguish a 401 from a refused connection. const GLUETUN_CONTROL_URL = "http://127.0.0.1:8000/v1/publicip/ip"; -function buildVpnHealthcheck(): Healthcheck { +function buildVpnHealthcheck(webuiPort: number): Healthcheck { + // Healthy requires BOTH legs. Leg 1 (gluetun's control server) proves qbit is + // in gluetun's LIVE netns, catching the reboot orphaned-netns case; no -f, so + // an auth 401 still counts as reachable, and it stays green during a real + // tunnel outage (the container is up). Leg 2 (qbit's own WebUI) catches a + // hung/crashed qbit while gluetun is up, which leg 1 alone would miss; -f so a + // non-2xx or timed-out UI fails. Either leg failing -> unhealthy -> deunhealth + // restarts qbit. + const webui = `http://127.0.0.1:${webuiPort}/`; return { - test: `["CMD-SHELL", "curl -sS -m 10 -o /dev/null ${GLUETUN_CONTROL_URL} || exit 1"]`, + test: `["CMD-SHELL", "curl -sS -m 10 -o /dev/null ${GLUETUN_CONTROL_URL} && curl -fsS -m 10 -o /dev/null ${webui} || exit 1"]`, interval: "30s", timeout: "10s", retries: 3, @@ -321,7 +329,9 @@ export function buildComposeContext(services: Service[], opts: ComposeOptions): // VPN-routed services need a tunnel-connectivity probe (see buildVpnHealthcheck) // so deunhealth can actually detect the reboot orphaned-netns state; a plain // loopback probe would pass even when the published port is dead. - const healthcheck = vpnNetwork ? buildVpnHealthcheck() : buildHealthcheck(svc); + const healthcheck = vpnNetwork + ? buildVpnHealthcheck(svc.health?.port ?? 8080) + : buildHealthcheck(svc); // Only qBittorrent is auto-restarted by deunhealth. On a host reboot Docker // ignores depends_on ordering, so qbit (network_mode: service:gluetun) can diff --git a/tests/renderer/compose.test.ts b/tests/renderer/compose.test.ts index a602665..8b1a4de 100644 --- a/tests/renderer/compose.test.ts +++ b/tests/renderer/compose.test.ts @@ -289,20 +289,18 @@ describe("healthchecks + deunhealth reboot self-heal", () => { expect(qbit.healthcheck!.test).toContain("http://127.0.0.1:8080/"); }); - test("VPN-routed qBittorrent probes gluetun's control server, not loopback", () => { - // A loopback WebUI probe passes even when qbit is orphaned from gluetun's - // netns after a reboot (its own 127.0.0.1:8080 still answers), so deunhealth - // would never fire. Probing gluetun's control server (127.0.0.1:8000, shared - // netns) only succeeds in the LIVE netns, and unlike an external check it - // stays healthy during a real tunnel outage (no pointless restart loop). + test("VPN-routed qBittorrent probes BOTH gluetun's control server and its own WebUI", () => { + // Leg 1 (control server, 8000) catches the reboot orphaned-netns case: a bare + // loopback WebUI probe would stay 'healthy' when orphaned (qbit's own 8080 + // still answers), and it holds during a real tunnel outage. Leg 2 (WebUI, + // 8080) catches a hung qbit while gluetun is up. Both must pass to be healthy. const opts = { ...baseOpts, vpn: { enabled: true, provider: "mullvad" } }; const ctx = buildComposeContext(getServices(["gluetun", "qbittorrent"]), opts); const qbit = ctx.services.find((s) => s.id === "qbittorrent")!; expect(qbit.healthcheck).toBeDefined(); expect(qbit.healthcheck!.test).toContain("127.0.0.1:8000"); - // -f omitted so an auth 401 still counts as reachable (only refused fails). - expect(qbit.healthcheck!.test).not.toContain("-f"); - expect(qbit.healthcheck!.test).not.toContain("127.0.0.1:8080"); + expect(qbit.healthcheck!.test).toContain("127.0.0.1:8080"); + expect(qbit.healthcheck!.test).toContain("&&"); // both legs required }); test("tcp/port-0 catalog health blocks are NOT rendered (caddy, recyclarr)", () => {