From d988c02ec7c1fdc89c485466eea6e7d724aa6349 Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Fri, 13 Nov 2026 05:07:47 +0000 Subject: [PATCH 01/10] fix(daemon): stop proves the running daemon's home from its process (HACK-2052) stop decided which Hack home hackd.service serves from the loaded unit configuration. After a drop-in that set another home is removed and the manager reloaded without a restart, that configuration says "this home" while the running daemon still serves the other one, and stop stopped it. With a live main process, stop now proves from the process itself (proveOwnedDaemonProcess: a hackd holding a socket in this home's daemon directory) and refuses, with the manual status and stop steps, when it is not proven. With no live process (a pending restart) the configuration still decides, so a stop still cancels the restart. The daemon install flows, which exercise the real protected system directory decision, run again on hosted Linux: the linux-process-lifetime job makes the runner's /usr and /opt owner-writable and runs them with HACK_TEST_REQUIRE_INSTALL_FLOWS=1, so a refusal fails rather than skips. Co-Authored-By: Claude --- .github/workflows/ci.yml | 15 +++ docs/architecture.md | 9 +- src/commands/daemon-systemd.ts | 176 +++++++++++++++++++++++-------- tests/daemon-systemd-cli.test.ts | 12 ++- tests/daemon-systemd.test.ts | 118 ++++++++++++++++++++- 5 files changed, 284 insertions(+), 46 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7bcd3d5d9..251405a88 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -818,3 +818,18 @@ jobs: run: bun install --frozen-lockfile - name: Run process and terminal regressions run: bun test tests/daemon-command.test.ts tests/daemon-orphan.test.ts tests/host-exec-lifetime.test.ts tests/host-exec-tty.test.ts tests/shell-observation.test.ts tests/shared-host-lifecycle.test.ts tests/project-shared-helpers.test.ts + # The daemon install flows run the real CLI, which checks the host's own + # system PATH (and what /usr/bin links into). GitHub's Ubuntu image + # leaves trees under /usr and /opt writable by everyone, so this one + # job makes them owner-writable, as on a stock install, and requires + # the flows to run: the protected system directory decision then has a + # hosted Linux check (HACK-2052). The regular test shards skip them. + - name: Harden runner system trees for the daemon install flows + run: | + sudo find /usr /opt -xdev -perm /022 \( -type f -o \( -type d ! -perm -1000 \) \) -print0 > "$RUNNER_TEMP/writable.list" + echo "removing group/other write from $(tr -cd '\0' < "$RUNNER_TEMP/writable.list" | wc -c) entries" + sudo xargs -0 -r chmod go-w < "$RUNNER_TEMP/writable.list" + - name: Run daemon install flows + env: + HACK_TEST_REQUIRE_INSTALL_FLOWS: "1" + run: bun test tests/daemon-systemd-cli.test.ts diff --git a/docs/architecture.md b/docs/architecture.md index a341ab2a1..1410b9d83 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -374,7 +374,14 @@ failed query whose only message names `hackd.service` itself as absent ("Unit hackd.service could not be found."), counts as nothing to do. `start` and `stop` act on the unit only when its effective environment serves the shell's Hack home. A unit that serves another Hack home is left alone: `start` starts this home's hackd directly (its socket, pid file and lock live in -this home's `daemon/` directory), and `stop` stops only this home's own hackd. +this home's `daemon/` directory), and `stop` stops only this home's own hackd. When the unit has a live main +process, `stop` proves from that process which Hack home it serves (a hackd +holding a socket in this home's `daemon/` directory), not from the loaded +configuration: a reload without a restart changes the configuration, not the +running daemon. A main process that is not proven to be this home's hackd is +refused, with the `systemctl --user status` and `stop` commands. With no live +process (a pending restart), the configuration decides, and `stop` still +cancels the restart. When the Hack home cannot be read with certainty (for example an `EnvironmentFile=`, an `ExecSearchPath=`, a `PassEnvironment=` naming `HACK_HOME`, `HOME` or `PATH`, a relative `HOME`, or an escaped quote or diff --git a/src/commands/daemon-systemd.ts b/src/commands/daemon-systemd.ts index 5047fdb36..93699f7e7 100644 --- a/src/commands/daemon-systemd.ts +++ b/src/commands/daemon-systemd.ts @@ -1,6 +1,7 @@ import { mkdir, rm, writeFile } from "node:fs/promises"; import { homedir } from "node:os"; import { dirname, resolve } from "node:path"; +import { GLOBAL_DAEMON_DIR_NAME } from "../constants.ts"; import type { DaemonPaths } from "../daemon/paths.ts"; import { type DaemonProcessProof, @@ -449,6 +450,11 @@ export async function stopViaSystemdIfInstalled(opts: { readonly hackHome: string; readonly deps?: SystemdDeps; readonly isPidInUnit?: (pid: number) => Promise; + /** + * Proves the unit's main process is this home's hackd; defaults to + * {@link proveOwnedDaemonProcess} on `/daemon`. Tests pin it. + */ + readonly proveDaemon?: (pid: number) => Promise; }): Promise<{ readonly handled: boolean; readonly ownedPid: boolean; @@ -474,35 +480,12 @@ export async function stopViaSystemdIfInstalled(opts: { return none; } if (!status.installed) { - // A failed read is not evidence that nothing runs: a pending - // Restart=on-failure may remain, so it is refused, not reported as - // stopped (HACK-2052). An observed inactive or not-found unit is - // nothing to do. - if (status.stateError !== null) { - return { - ...none, - refused: `Not stopping ${SYSTEMD_UNIT_NAME}: there is no unit file at ${unitPath}, but systemd could not report whether it still runs or will restart (${status.stateError}). Check: systemctl --user status ${SYSTEMD_UNIT_NAME}; to stop it by hand, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, - }; - } - if ( - status.activeState === null || - !MANAGER_STOPPABLE_STATES.has(status.activeState) - ) { - return none; - } - // Another home's unit, loaded from its own directory: not this shell's - // to stop or refuse over; this shell's direct hackd is still stopped. - const loadedHome = await readSystemdUnitHackHome({ deps }); - if ( - loadedHome.kind === "home" && - loadedHome.path !== resolve(opts.hackHome) - ) { - return none; - } - return { - ...none, - refused: `Not stopping ${SYSTEMD_UNIT_NAME}: systemd has it ${status.activeState}, but not from ${unitPath}, the unit Hack manages (it was removed or is loaded from another directory), so it cannot be checked. Review it with systemctl --user status ${SYSTEMD_UNIT_NAME}; to stop it anyway, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, - }; + return await stopWithoutUnitFile({ + status, + unitPath, + hackHome: opts.hackHome, + deps, + }); } // An unknown state (the unit-specific `systemctl show` failed) does not // prove the unit is stopped: stop it anyway, which is harmless when it is @@ -514,20 +497,35 @@ export async function stopViaSystemdIfInstalled(opts: { return none; } // The unit is a per-user singleton: stopping it from a shell with another - // Hack home would stop someone else's daemon (HACK-2052). Another home's - // unit is left alone (this shell's own direct hackd is still stopped - // through the proven-ownership path); an unreadable home is refused. - const unitHome = await checkUnitServesHome({ - hackHome: opts.hackHome, - deps, - action: "stopping", + // Hack home would stop someone else's daemon (HACK-2052). With a live main + // process, what it serves is proven from the process itself (its socket + // in this home's daemon directory), not from the loaded configuration, + // which a reload without a restart can change under a running daemon. + const mainProcess = await checkMainProcessServesHome({ + status, + daemonRoot: resolve(opts.hackHome, GLOBAL_DAEMON_DIR_NAME), + prove: opts.proveDaemon, }); - if (unitHome.kind === "foreign") { - logger.info({ message: unitHome.message }); - return none; - } - if (unitHome.kind === "unknown") { - return { ...none, refused: unitHome.message }; + if (mainProcess.kind === "refused") { + return { ...none, refused: mainProcess.message }; + } + if (mainProcess.kind === "none") { + // No live process (a pending restart, or starting up): only the loaded + // configuration says whose it is. Another home's unit is left alone + // (this shell's own direct hackd is still stopped through the + // proven-ownership path); an unreadable home is refused. + const unitHome = await checkUnitServesHome({ + hackHome: opts.hackHome, + deps, + action: "stopping", + }); + if (unitHome.kind === "foreign") { + logger.info({ message: unitHome.message }); + return none; + } + if (unitHome.kind === "unknown") { + return { ...none, refused: unitHome.message }; + } } const ownedPid = opts.pid !== null && @@ -554,6 +552,100 @@ export async function stopViaSystemdIfInstalled(opts: { return { handled: true, ownedPid, failed: null, refused: null }; } +/** + * `stop` when no unit file is at the managed path (HACK-2052): see the + * comments below. Never runs `systemctl stop`; it reports nothing to do or + * refuses with the manual steps. + */ +async function stopWithoutUnitFile(opts: { + readonly status: SystemdServiceStatus; + readonly unitPath: string; + readonly hackHome: string; + readonly deps: SystemdDeps; +}): Promise<{ + readonly handled: false; + readonly ownedPid: false; + readonly failed: null; + readonly refused: string | null; +}> { + const { status, unitPath, deps } = opts; + const none = { + handled: false, + ownedPid: false, + failed: null, + refused: null, + } as const; + // A failed read is not evidence that nothing runs: a pending + // Restart=on-failure may remain, so it is refused, not reported as + // stopped (HACK-2052). An observed inactive or not-found unit is + // nothing to do. + if (status.stateError !== null) { + return { + ...none, + refused: `Not stopping ${SYSTEMD_UNIT_NAME}: there is no unit file at ${unitPath}, but systemd could not report whether it still runs or will restart (${status.stateError}). Check: systemctl --user status ${SYSTEMD_UNIT_NAME}; to stop it by hand, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, + }; + } + if ( + status.activeState === null || + !MANAGER_STOPPABLE_STATES.has(status.activeState) + ) { + return none; + } + // Another home's unit, loaded from its own directory, with no live + // process (a pending restart): not this shell's to stop or refuse over. + // A live process is refused below either way: it cannot be checked + // without the unit file (HACK-2052). + const loadedHome = await readSystemdUnitHackHome({ deps }); + if ( + status.mainPid === null && + loadedHome.kind === "home" && + loadedHome.path !== resolve(opts.hackHome) + ) { + return none; + } + return { + ...none, + refused: `Not stopping ${SYSTEMD_UNIT_NAME}: systemd has it ${status.activeState}, but not from ${unitPath}, the unit Hack manages (it was removed or is loaded from another directory), so it cannot be checked. Review it with systemctl --user status ${SYSTEMD_UNIT_NAME}; to stop it anyway, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, + }; +} + +/** + * Whether the unit's live main process is this home's hackd, from evidence + * tied to that process ({@link proveOwnedDaemonProcess}: a hackd image + * holding a socket in `daemonRoot`), never from configuration (HACK-2052). + * `none`: no live main process (none reported, or it exited meanwhile). + * `refused`: a live process that is not proven to be this home's, with the + * manual status and stop steps. + */ +async function checkMainProcessServesHome(opts: { + readonly status: SystemdServiceStatus; + readonly daemonRoot: string; + readonly prove?: (pid: number) => Promise; +}): Promise< + | { readonly kind: "ours" | "none" } + | { readonly kind: "refused"; readonly message: string } +> { + const pid = opts.status.mainPid; + if (pid === null) { + return { kind: "none" }; + } + const proof = await ( + opts.prove ?? + ((candidate: number) => + proveOwnedDaemonProcess({ pid: candidate, daemonRoot: opts.daemonRoot })) + )(pid); + if (proof.kind === "owned") { + return { kind: "ours" }; + } + if (proof.reason === "not-running") { + return { kind: "none" }; + } + return { + kind: "refused", + message: `Not stopping ${SYSTEMD_UNIT_NAME}: its main process (pid ${pid}) is not proven to be the hackd of this Hack home (${opts.daemonRoot}): ${describeDaemonProcessRefusal(proof.reason)}. It may serve another Hack home (a reload changes the unit's configuration, not the running process). Check: systemctl --user status ${SYSTEMD_UNIT_NAME}; to stop it anyway, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, + }; +} + /** * When systemd runs an incompatible hackd from a unit that points at a * different, still existing `hack`, replacing the daemon would only make diff --git a/tests/daemon-systemd-cli.test.ts b/tests/daemon-systemd-cli.test.ts index 9b10cd224..f01837beb 100644 --- a/tests/daemon-systemd-cli.test.ts +++ b/tests/daemon-systemd-cli.test.ts @@ -106,7 +106,11 @@ const hostSystemPathProblem = dirs: [...RENDERED_SYSTEM_PATH_DIRECTORIES, ...UNSET_PATH_LOOKUP_DIRS], }) : null; -if (hostSystemPathProblem !== null && process.env.GITHUB_ACTIONS === "true") { +if ( + hostSystemPathProblem !== null && + process.env.GITHUB_ACTIONS === "true" && + process.env.HACK_TEST_REQUIRE_INSTALL_FLOWS !== "1" +) { console.warn( `daemon-systemd-cli: skipping the install flows on this host: ${hostSystemPathProblem}` ); @@ -114,9 +118,13 @@ if (hostSystemPathProblem !== null && process.env.GITHUB_ACTIONS === "true") { // Skipped only on GitHub Actions, whose image is known to fail the check; // on any other host a refusal fails these flows instead of hiding a // regression in the check itself (HACK-2142). +// HACK_TEST_REQUIRE_INSTALL_FLOWS=1 (the hardened linux-process-lifetime +// CI job) never skips: there a refusal fails the flows. const installFlowTest = test.skipIf( process.platform !== "linux" || - (hostSystemPathProblem !== null && process.env.GITHUB_ACTIONS === "true") + (hostSystemPathProblem !== null && + process.env.GITHUB_ACTIONS === "true" && + process.env.HACK_TEST_REQUIRE_INSTALL_FLOWS !== "1") ); let fixture: Fixture | null = null; diff --git a/tests/daemon-systemd.test.ts b/tests/daemon-systemd.test.ts index 7f3b3111f..c149eb1d3 100644 --- a/tests/daemon-systemd.test.ts +++ b/tests/daemon-systemd.test.ts @@ -21,6 +21,7 @@ import { withSystemdIncompatibleNextStep, } from "../src/commands/daemon-systemd.ts"; import { resolveDaemonPaths } from "../src/daemon/paths.ts"; +import type { DaemonProcessProof } from "../src/daemon/process.ts"; import { summarizeSystemdService } from "../src/daemon/service-summary.ts"; import { buildDaemonRepairMessage, @@ -116,6 +117,13 @@ type Fake = { managerEnvironment: string; /** The account's home in the password database (`getent passwd`). */ passwdHome: string | null; + /** + * What proving the unit's main process (MainPID) finds: this home's + * hackd, another home's (its socket elsewhere), or nothing readable. + */ + mainProcess: "ours" | "foreign" | "unknown"; + /** False: the unit reports no main process (e.g. a pending restart). */ + mainPidReported: boolean; /** Namespace settings (`RootDirectory=...` lines) systemd reports. */ namespaceLines: string[]; }; @@ -184,6 +192,8 @@ function fakeSystemd(opts: FakeOptions = {}): Fake { extraExecLines: [], managerEnvironment: `HOME=${root}\nPATH=${sysBin}\n`, passwdHome: root, + mainProcess: "ours", + mainPidReported: true, namespaceLines: [], deps: undefined as unknown as SystemdDeps, }; @@ -296,7 +306,7 @@ function fakeSystemd(opts: FakeOptions = {}): Fake { "LoadState=loaded", `ActiveState=${state.activeState ?? (state.failed ? "failed" : state.active ? "active" : "inactive")}`, `UnitFileState=${state.enabled ? "enabled" : "disabled"}`, - `MainPID=${state.active ? (opts.mainPid ?? 4242) : 0}`, + `MainPID=${state.active && state.mainPidReported ? (opts.mainPid ?? 4242) : 0}`, `NeedDaemonReload=${state.needsReload ? "yes" : "no"}`, `FragmentPath=${state.fragmentPath ?? unitPath}`, `DropInPaths=${state.dropInPaths}`, @@ -360,6 +370,27 @@ function fakeSystemd(opts: FakeOptions = {}): Fake { return state; } +/** The stop path's proof of the unit's main process, as the fake says. */ +function proveFor(fake: Fake): (pid: number) => Promise { + return (pid) => + Promise.resolve( + fake.mainProcess === "ours" + ? ({ + kind: "owned", + pid, + birth: "ticks:1", + image: { executable: program, argv: [program] }, + } as unknown as DaemonProcessProof) + : { + kind: "refused", + reason: + fake.mainProcess === "foreign" + ? "foreign-socket" + : "identity-unknown", + } + ); +} + function unitText(): string { const text = renderSystemdUnit({ hackBinPath: program }); if (!text) { @@ -1180,6 +1211,7 @@ linuxTest( fake.calls.length = 0; expect( await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -1195,6 +1227,7 @@ linuxTest( fake.calls.length = 0; expect( await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -1684,6 +1717,7 @@ linuxTest( fake.showFails = true; fake.calls.length = 0; const result = await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -1699,6 +1733,9 @@ linuxTest( "stop refuses a unit whose effective Hack home is another one", async () => { const fake = fakeSystemd({ active: true }); + // No live main process (as during a pending restart): the loaded + // configuration is all that says whose the unit is. + fake.mainPidReported = false; await installSystemdUnit({ unitPath, unitText: unitText(), @@ -1706,6 +1743,7 @@ linuxTest( }); const stop = () => stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -2756,6 +2794,7 @@ linuxTest( fake.calls.length = 0; const stop = await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -2824,6 +2863,7 @@ linuxTest( fake.active = true; fake.calls.length = 0; const stop = await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -3017,6 +3057,9 @@ linuxTest( // HACK_HOME set there (not in the unit) made stop and uninstall think the // unit served this shell's home. const fake = fakeSystemd({ active: true }); + // No live main process (as during a pending restart): the loaded + // configuration is all that says whose the unit is. + fake.mainPidReported = false; await installSystemdUnit({ unitPath, unitText: unitText(), @@ -3026,6 +3069,7 @@ linuxTest( fake.managerEnvironment = `HOME=${root}\nHACK_HOME=/srv/other-home\n`; fake.calls.length = 0; const stop = await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -3043,6 +3087,7 @@ linuxTest( expect( ( await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -3178,6 +3223,9 @@ linuxTest( // Second-family review: `UnsetEnvironment=HACK_HOME` removed a HACK_HOME // naming this shell's home, so hackd used `$HOME/.hack` of another HOME. const fake = fakeSystemd({ active: true }); + // No live main process (as during a pending restart): the loaded + // configuration is all that says whose the unit is. + fake.mainPidReported = false; await installSystemdUnit({ unitPath, unitText: unitText(), @@ -3185,6 +3233,7 @@ linuxTest( }); const stop = () => stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -3337,6 +3386,7 @@ linuxTest( fake.active = true; fake.calls.length = 0; const stop = await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -3356,6 +3406,7 @@ linuxTest( expect( ( await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -3401,6 +3452,7 @@ linuxTest( fake.active = true; fake.calls.length = 0; const stop = await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -3428,6 +3480,7 @@ linuxTest( const fake = fakeSystemd(); const stop = () => stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -3465,6 +3518,9 @@ linuxTest( // home fell back to the invoking shell's HOME; hackd uses the account's // home from the password database. const fake = fakeSystemd({ active: true }); + // No live main process (as during a pending restart): the loaded + // configuration is all that says whose the unit is. + fake.mainPidReported = false; await installSystemdUnit({ unitPath, unitText: unitText(), @@ -3474,6 +3530,7 @@ linuxTest( fake.unsetEnvironment = "HOME"; const stop = () => stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -3687,6 +3744,7 @@ linuxTest( fake.showFails = true; fake.calls.length = 0; const stop = await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: fake.deps, @@ -3727,6 +3785,7 @@ linuxTest( fake.showFails = false; expect( await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps: notFound, @@ -3761,6 +3820,7 @@ linuxTest( }); const stop = (deps: SystemdDeps) => stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), pid: null, hackHome: join(root, ".hack"), deps, @@ -3857,3 +3917,59 @@ linuxTest( ); } ); + +linuxTest( + "stop proves the running daemon's home from its process, not the loaded configuration", + async () => { + // Second-family review: after a drop-in that set another Hack home is + // removed and the manager reloaded without a restart, the loaded + // configuration says "this home" while the running daemon still serves + // the other one; stop decided from the configuration and stopped it. + const fake = fakeSystemd({ active: true }); + await installSystemdUnit({ + unitPath, + unitText: unitText(), + deps: fake.deps, + }); + const stop = () => + stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), + pid: null, + hackHome: join(root, ".hack"), + deps: fake.deps, + }); + // The loaded configuration is this home's (the drop-in is gone), but + // the live main process holds its socket elsewhere. + fake.active = true; + fake.mainProcess = "foreign"; + fake.calls.length = 0; + const foreign = await stop(); + expect(foreign.handled).toBe(false); + expect(foreign.refused).toContain("pid 4242"); + expect(foreign.refused).toContain( + "not proven to be the hackd of this Hack home" + ); + expect(foreign.refused).toContain("systemctl --user status hackd.service"); + expect(foreign.refused).toContain("systemctl --user stop hackd.service"); + expect(fake.calls).not.toContain("systemctl --user stop hackd.service"); + expect(fake.active).toBe(true); + // Process evidence that cannot be read: refused the same way. + fake.mainProcess = "unknown"; + const unknown = await stop(); + expect(unknown.refused).toContain("not proven"); + expect(fake.calls).not.toContain("systemctl --user stop hackd.service"); + // This home's own daemon: stopped. + fake.mainProcess = "ours"; + expect((await stop()).handled).toBe(true); + expect(fake.calls).toContain("systemctl --user stop hackd.service"); + // No live daemon, a restart pending: the configuration decides, and a + // stop still cancels the restart. + fake.active = true; + fake.mainPidReported = false; + fake.activeState = "activating"; + fake.mainProcess = "foreign"; + fake.calls.length = 0; + expect((await stop()).handled).toBe(true); + expect(fake.calls).toContain("systemctl --user stop hackd.service"); + } +); From cd24d822f972c683e013b7eab786c23bbfde9f9e Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Fri, 13 Nov 2026 05:12:38 +0000 Subject: [PATCH 02/10] fix(daemon): accept this home's daemon run by another install when stopping (HACK-2052) The image check accepted a Bun source run only with this CLI's entry file, so stop refused this home's daemon when the unit runs another install or checkout (a Bun launcher, the dev shim). For hackd.service's main process, which systemd names, any foreground daemon start counts as hackd (anyBunEntry); the socket in this home's daemon directory proves the home. A main process still activating that has not bound its socket is reported as possibly still starting, not as another home's. Co-Authored-By: Claude --- src/commands/daemon-systemd.ts | 28 +++++++++++- src/daemon/process.ts | 11 ++++- tests/daemon-systemd.test.ts | 83 +++++++++++++++++++++++++++++++++- 3 files changed, 118 insertions(+), 4 deletions(-) diff --git a/src/commands/daemon-systemd.ts b/src/commands/daemon-systemd.ts index 93699f7e7..7cc5fb9e4 100644 --- a/src/commands/daemon-systemd.ts +++ b/src/commands/daemon-systemd.ts @@ -4,8 +4,10 @@ import { dirname, resolve } from "node:path"; import { GLOBAL_DAEMON_DIR_NAME } from "../constants.ts"; import type { DaemonPaths } from "../daemon/paths.ts"; import { + type DaemonProcessProbe, type DaemonProcessProof, type DaemonProcessRefusal, + defaultDaemonIdentityExpectation, describeDaemonProcessRefusal, proveOwnedDaemonProcess, } from "../daemon/process.ts"; @@ -455,6 +457,8 @@ export async function stopViaSystemdIfInstalled(opts: { * {@link proveOwnedDaemonProcess} on `/daemon`. Tests pin it. */ readonly proveDaemon?: (pid: number) => Promise; + /** Test seam for the default proof's observations. */ + readonly probe?: DaemonProcessProbe; }): Promise<{ readonly handled: boolean; readonly ownedPid: boolean; @@ -505,6 +509,7 @@ export async function stopViaSystemdIfInstalled(opts: { status, daemonRoot: resolve(opts.hackHome, GLOBAL_DAEMON_DIR_NAME), prove: opts.proveDaemon, + probe: opts.probe, }); if (mainProcess.kind === "refused") { return { ...none, refused: mainProcess.message }; @@ -621,6 +626,7 @@ async function checkMainProcessServesHome(opts: { readonly status: SystemdServiceStatus; readonly daemonRoot: string; readonly prove?: (pid: number) => Promise; + readonly probe?: DaemonProcessProbe; }): Promise< | { readonly kind: "ours" | "none" } | { readonly kind: "refused"; readonly message: string } @@ -629,10 +635,22 @@ async function checkMainProcessServesHome(opts: { if (pid === null) { return { kind: "none" }; } + // systemd names this process; it may run another install or checkout + // than this CLI, so any foreground daemon start counts as hackd, and the + // socket in this home's daemon directory proves the home. + const expectation = { + ...(await defaultDaemonIdentityExpectation()), + anyBunEntry: true, + }; const proof = await ( opts.prove ?? ((candidate: number) => - proveOwnedDaemonProcess({ pid: candidate, daemonRoot: opts.daemonRoot })) + proveOwnedDaemonProcess({ + pid: candidate, + daemonRoot: opts.daemonRoot, + expectation, + probe: opts.probe, + })) )(pid); if (proof.kind === "owned") { return { kind: "ours" }; @@ -640,9 +658,15 @@ async function checkMainProcessServesHome(opts: { if (proof.reason === "not-running") { return { kind: "none" }; } + // Still starting: its socket may not be bound yet, which is not evidence + // of another home. + const why = + opts.status.activeState === "activating" + ? "systemd reports it still starting, so it may not have bound its socket yet; run hack daemon stop again in a moment" + : "it may serve another Hack home (a reload changes the unit's configuration, not the running process)"; return { kind: "refused", - message: `Not stopping ${SYSTEMD_UNIT_NAME}: its main process (pid ${pid}) is not proven to be the hackd of this Hack home (${opts.daemonRoot}): ${describeDaemonProcessRefusal(proof.reason)}. It may serve another Hack home (a reload changes the unit's configuration, not the running process). Check: systemctl --user status ${SYSTEMD_UNIT_NAME}; to stop it anyway, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, + message: `Not stopping ${SYSTEMD_UNIT_NAME}: its main process (pid ${pid}) is not proven to be the hackd of this Hack home (${opts.daemonRoot}): ${describeDaemonProcessRefusal(proof.reason)}. ${why}. Check: systemctl --user status ${SYSTEMD_UNIT_NAME}; to stop it anyway, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, }; } diff --git a/src/daemon/process.ts b/src/daemon/process.ts index 8e551d5da..73f6ef080 100644 --- a/src/daemon/process.ts +++ b/src/daemon/process.ts @@ -328,6 +328,14 @@ export const defaultDaemonProcessProbe: DaemonProcessProbe = { */ export interface DaemonIdentityExpectation { readonly entryFiles: ReadonlySet; + /** + * Accept a source run of any entry file (still a foreground daemon + * start). Only for a process whose identity comes from elsewhere, such + * as hackd.service's main process as systemd reports it: the unit may run + * another install or checkout than this CLI (a Bun launcher, the dev + * shim), and the socket proves which home it serves (HACK-2052). + */ + readonly anyBunEntry?: boolean; } let defaultExpectation: Promise | null = null; @@ -917,7 +925,8 @@ export function isHackDaemonImage(opts: { } return ( entry !== null && - opts.expectation.entryFiles.has(entry) && + (opts.expectation.anyBunEntry === true || + opts.expectation.entryFiles.has(entry)) && isForegroundDaemonStartArgs(argv.slice(entryIndex + 1)) ); } diff --git a/tests/daemon-systemd.test.ts b/tests/daemon-systemd.test.ts index c149eb1d3..a6091210f 100644 --- a/tests/daemon-systemd.test.ts +++ b/tests/daemon-systemd.test.ts @@ -21,7 +21,11 @@ import { withSystemdIncompatibleNextStep, } from "../src/commands/daemon-systemd.ts"; import { resolveDaemonPaths } from "../src/daemon/paths.ts"; -import type { DaemonProcessProof } from "../src/daemon/process.ts"; +import type { + DaemonProcessProbe, + DaemonProcessProof, +} from "../src/daemon/process.ts"; +import type { ProcessImage } from "../src/daemon/process-image.ts"; import { summarizeSystemdService } from "../src/daemon/service-summary.ts"; import { buildDaemonRepairMessage, @@ -3973,3 +3977,80 @@ linuxTest( expect(fake.calls).toContain("systemctl --user stop hackd.service"); } ); + +linuxTest( + "stop accepts this home's daemon run by another install, through the real proof", + async () => { + // Reviewer: the unit may run another hack than this CLI (a Bun launcher + // or the dev shim, `bun daemon start --foreground`); the + // image check only accepted this CLI's own entry file, so stop refused + // this home's daemon. The socket proves the home. + const child = Bun.spawn(["sleep", "30"], { + stdout: "ignore", + stderr: "ignore", + }); + try { + const fake = fakeSystemd({ active: true, mainPid: child.pid }); + await installSystemdUnit({ + unitPath, + unitText: unitText(), + deps: fake.deps, + }); + let image: ProcessImage = { + executable: "/usr/local/bin/bun", + argv: [ + "/usr/local/bin/bun", + "/srv/other-checkout/index.ts", + "daemon", + "start", + "--foreground", + ], + cwd: "/", + }; + let socketOwners = new Set([child.pid]); + const probe: DaemonProcessProbe = { + birth: () => Promise.resolve("ticks:1"), + image: () => Promise.resolve(image), + socketOwners: () => Promise.resolve(socketOwners), + }; + const stop = () => + stopViaSystemdIfInstalled({ + probe, + pid: null, + hackHome: join(root, ".hack"), + deps: fake.deps, + }); + fake.active = true; + expect((await stop()).handled).toBe(true); + // Not a hackd at all: refused. + fake.active = true; + image = { + executable: "/usr/bin/python3", + argv: ["/usr/bin/python3", "daemon", "start", "--foreground"], + cwd: "/", + }; + expect((await stop()).refused).toContain("not proven"); + // Still starting, socket not bound yet: says so instead of blaming + // another home. + image = { + executable: "/usr/local/bin/bun", + argv: [ + "/usr/local/bin/bun", + "/srv/x/index.ts", + "daemon", + "start", + "--foreground", + ], + cwd: "/", + }; + socketOwners = new Set(); + fake.activeState = "activating"; + const starting = await stop(); + expect(starting.refused).toContain("still starting"); + expect(starting.refused).not.toContain("may serve another Hack home"); + } finally { + child.kill(); + await child.exited; + } + } +); From 34cd1861dec33838704642dfb70df51844d48bdd Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Fri, 13 Nov 2026 05:19:16 +0000 Subject: [PATCH 03/10] fix(daemon): stop proves the hackd child when MainPID is a launcher (HACK-2052) A #! launcher that runs hackd as a child (no exec) keeps the interpreter as the unit's MainPID, so requiring MainPID itself to be hackd refused this home's own daemon. When MainPID is proven not to be a hackd, the hackd named by this home's pid file counts instead, if it is proven this home's (socket in its daemon directory) and runs in the unit's cgroup. Co-Authored-By: Claude --- src/commands/daemon-systemd.ts | 27 +++++++++++++++-- tests/daemon-systemd.test.ts | 53 ++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/src/commands/daemon-systemd.ts b/src/commands/daemon-systemd.ts index 7cc5fb9e4..5c8082e3a 100644 --- a/src/commands/daemon-systemd.ts +++ b/src/commands/daemon-systemd.ts @@ -510,6 +510,8 @@ export async function stopViaSystemdIfInstalled(opts: { daemonRoot: resolve(opts.hackHome, GLOBAL_DAEMON_DIR_NAME), prove: opts.proveDaemon, probe: opts.probe, + homePid: opts.pid, + isPidInUnit: opts.isPidInUnit, }); if (mainProcess.kind === "refused") { return { ...none, refused: mainProcess.message }; @@ -627,6 +629,9 @@ async function checkMainProcessServesHome(opts: { readonly daemonRoot: string; readonly prove?: (pid: number) => Promise; readonly probe?: DaemonProcessProbe; + /** This home's hackd per its pid file, if one is running. */ + readonly homePid: number | null; + readonly isPidInUnit?: (pid: number) => Promise; }): Promise< | { readonly kind: "ours" | "none" } | { readonly kind: "refused"; readonly message: string } @@ -642,7 +647,7 @@ async function checkMainProcessServesHome(opts: { ...(await defaultDaemonIdentityExpectation()), anyBunEntry: true, }; - const proof = await ( + const prove = opts.prove ?? ((candidate: number) => proveOwnedDaemonProcess({ @@ -650,14 +655,30 @@ async function checkMainProcessServesHome(opts: { daemonRoot: opts.daemonRoot, expectation, probe: opts.probe, - })) - )(pid); + })); + const proof = await prove(pid); if (proof.kind === "owned") { return { kind: "ours" }; } if (proof.reason === "not-running") { return { kind: "none" }; } + // A `#!` launcher that runs hackd as a child (no `exec`) is the main + // process itself: then the hackd named by this home's pid file must be + // proven this home's and be in the unit (its cgroup), still from process + // evidence, not configuration. + if ( + proof.reason === "not-hackd" && + opts.homePid !== null && + opts.homePid !== pid && + (await prove(opts.homePid)).kind === "owned" && + (await ( + opts.isPidInUnit ?? + ((child: number) => isPidInSystemdUnit({ pid: child })) + )(opts.homePid)) === true + ) { + return { kind: "ours" }; + } // Still starting: its socket may not be bound yet, which is not evidence // of another home. const why = diff --git a/tests/daemon-systemd.test.ts b/tests/daemon-systemd.test.ts index a6091210f..c43a91df6 100644 --- a/tests/daemon-systemd.test.ts +++ b/tests/daemon-systemd.test.ts @@ -4054,3 +4054,56 @@ linuxTest( } } ); + +linuxTest( + "stop proves the hackd child when the unit's main process is a launcher", + async () => { + // Codex review: a `#!` launcher that runs hackd as a child keeps the + // interpreter as MainPID; requiring MainPID itself to be hackd refused + // this home's own daemon. The pid file's hackd, proven this home's and + // in the unit, counts instead. + const fake = fakeSystemd({ active: true }); + await installSystemdUnit({ + unitPath, + unitText: unitText(), + deps: fake.deps, + }); + const child = 5555; + let childProof: "owned" | "foreign-socket" = "owned"; + let inUnit: boolean | null = true; + const proveDaemon = (pid: number): Promise => + Promise.resolve( + pid === child && childProof === "owned" + ? ({ + kind: "owned", + pid, + birth: "ticks:1", + image: { executable: program, argv: [program] }, + } as unknown as DaemonProcessProof) + : { + kind: "refused", + reason: pid === child ? childProof : "not-hackd", + } + ); + const stop = (pid: number | null) => + stopViaSystemdIfInstalled({ + proveDaemon, + isPidInUnit: () => Promise.resolve(inUnit), + pid, + hackHome: join(root, ".hack"), + deps: fake.deps, + }); + fake.active = true; + expect((await stop(child)).handled).toBe(true); + // The child is not in the unit: not this unit's daemon. + fake.active = true; + inUnit = false; + expect((await stop(child)).refused).toContain("not proven"); + // The child is another home's: refused. + inUnit = true; + childProof = "foreign-socket"; + expect((await stop(child)).refused).toContain("not proven"); + // No pid file hackd to prove: refused. + expect((await stop(null)).refused).toContain("not proven"); + } +); From 6db80d1aec6c4cf7b30a62e96412c11a6c6767c6 Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Fri, 13 Nov 2026 05:19:59 +0000 Subject: [PATCH 04/10] test(daemon): type the launcher test's refusal reason (HACK-2052) Co-Authored-By: Claude --- tests/daemon-systemd.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/daemon-systemd.test.ts b/tests/daemon-systemd.test.ts index c43a91df6..36e94a064 100644 --- a/tests/daemon-systemd.test.ts +++ b/tests/daemon-systemd.test.ts @@ -4082,7 +4082,7 @@ linuxTest( } as unknown as DaemonProcessProof) : { kind: "refused", - reason: pid === child ? childProof : "not-hackd", + reason: pid === child ? "foreign-socket" : "not-hackd", } ); const stop = (pid: number | null) => From a026d2339a88ad3b324cc5657dfeadb96ce1f53f Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Fri, 13 Nov 2026 05:22:16 +0000 Subject: [PATCH 05/10] fix(daemon): the launcher fallback requires the hackd to be its direct child (HACK-2052) The unit's cgroup also holds whatever its processes started, so this home's hackd started detached from a job of another home's daemon passed the launcher fallback, and stop stopped the other home's unit. The pid file's hackd must now also be the main process's direct child (/proc//stat parent pid). Co-Authored-By: Claude --- src/commands/daemon-systemd.ts | 15 ++++++++++++--- src/daemon/process-image.ts | 15 +++++++++++++++ tests/daemon-systemd.test.ts | 8 ++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/commands/daemon-systemd.ts b/src/commands/daemon-systemd.ts index 5c8082e3a..3b63dc69a 100644 --- a/src/commands/daemon-systemd.ts +++ b/src/commands/daemon-systemd.ts @@ -11,6 +11,7 @@ import { describeDaemonProcessRefusal, proveOwnedDaemonProcess, } from "../daemon/process.ts"; +import { readLinuxParentPid } from "../daemon/process-image.ts"; import { type DaemonServiceSummary, summarizeSystemdService, @@ -459,6 +460,8 @@ export async function stopViaSystemdIfInstalled(opts: { readonly proveDaemon?: (pid: number) => Promise; /** Test seam for the default proof's observations. */ readonly probe?: DaemonProcessProbe; + /** Test seam: a process's parent pid (defaults to /proc). */ + readonly parentPid?: (pid: number) => Promise; }): Promise<{ readonly handled: boolean; readonly ownedPid: boolean; @@ -512,6 +515,7 @@ export async function stopViaSystemdIfInstalled(opts: { probe: opts.probe, homePid: opts.pid, isPidInUnit: opts.isPidInUnit, + parentPid: opts.parentPid, }); if (mainProcess.kind === "refused") { return { ...none, refused: mainProcess.message }; @@ -632,6 +636,8 @@ async function checkMainProcessServesHome(opts: { /** This home's hackd per its pid file, if one is running. */ readonly homePid: number | null; readonly isPidInUnit?: (pid: number) => Promise; + /** Defaults to {@link readLinuxParentPid}; tests pin it. */ + readonly parentPid?: (pid: number) => Promise; }): Promise< | { readonly kind: "ours" | "none" } | { readonly kind: "refused"; readonly message: string } @@ -665,8 +671,10 @@ async function checkMainProcessServesHome(opts: { } // A `#!` launcher that runs hackd as a child (no `exec`) is the main // process itself: then the hackd named by this home's pid file must be - // proven this home's and be in the unit (its cgroup), still from process - // evidence, not configuration. + // proven this home's, be in the unit (its cgroup) and be the launcher's + // direct child. The cgroup alone also holds whatever the unit's processes + // started, such as this home's hackd started detached from a job of + // another home's daemon; still process evidence, not configuration. if ( proof.reason === "not-hackd" && opts.homePid !== null && @@ -675,7 +683,8 @@ async function checkMainProcessServesHome(opts: { (await ( opts.isPidInUnit ?? ((child: number) => isPidInSystemdUnit({ pid: child })) - )(opts.homePid)) === true + )(opts.homePid)) === true && + (await (opts.parentPid ?? readLinuxParentPid)(opts.homePid)) === pid ) { return { kind: "ours" }; } diff --git a/src/daemon/process-image.ts b/src/daemon/process-image.ts index dfd723c10..aac9953cc 100644 --- a/src/daemon/process-image.ts +++ b/src/daemon/process-image.ts @@ -292,6 +292,21 @@ export async function readLinuxProcessStartTicks( return ticks && DIGITS.test(ticks) ? ticks : null; } +/** + * The parent pid of `pid` on Linux (`/proc//stat` field 4), or null + * when it cannot be read (gone, or another platform). + */ +export async function readLinuxParentPid(pid: number): Promise { + const stat = await readTextFile(`/proc/${pid}/stat`); + if (stat === null) { + return null; + } + // Fields after the command name (in parentheses) are fixed: rest[0] is + // field 3 (state), rest[1] is field 4 (ppid). + const ppid = stat.slice(stat.lastIndexOf(")") + 2).split(" ")[1]; + return ppid && DIGITS.test(ppid) ? Number.parseInt(ppid, 10) : null; +} + /** * True when `pid` is a zombie on Linux (`/proc//stat` state `Z`): it * has exited and only waits for its parent to reap it. `kill(pid, 0)` still diff --git a/tests/daemon-systemd.test.ts b/tests/daemon-systemd.test.ts index 36e94a064..5fc8f6f53 100644 --- a/tests/daemon-systemd.test.ts +++ b/tests/daemon-systemd.test.ts @@ -4071,6 +4071,7 @@ linuxTest( const child = 5555; let childProof: "owned" | "foreign-socket" = "owned"; let inUnit: boolean | null = true; + let parent: number | null = 4242; const proveDaemon = (pid: number): Promise => Promise.resolve( pid === child && childProof === "owned" @@ -4089,12 +4090,19 @@ linuxTest( stopViaSystemdIfInstalled({ proveDaemon, isPidInUnit: () => Promise.resolve(inUnit), + parentPid: () => Promise.resolve(parent), pid, hackHome: join(root, ".hack"), deps: fake.deps, }); fake.active = true; expect((await stop(child)).handled).toBe(true); + // In the unit but not the launcher's child (started detached from a job + // inside it, then re-parented): refused. + fake.active = true; + parent = 1; + expect((await stop(child)).refused).toContain("not proven"); + parent = 4242; // The child is not in the unit: not this unit's daemon. fake.active = true; inUnit = false; From 669c955027bac9c6fcc2d9cb41be2558ccdc6b95 Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Fri, 13 Nov 2026 05:57:05 +0000 Subject: [PATCH 06/10] ci: give runner files owned by other accounts to root for the daemon install flows (HACK-2052) The hardened linux-process-lifetime run fixed the modes (817,281 entries) and the flows then met /usr/bin linking to a geckodriver owned by uid 1000, another regular account, which Hack correctly refuses. On a stock install everything under /usr and /opt is root's; the step now gives entries owned by neither root nor the runner user to root (chown -h, links themselves). Co-Authored-By: Claude --- .github/workflows/ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 251405a88..df800a343 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -829,6 +829,12 @@ jobs: sudo find /usr /opt -xdev -perm /022 \( -type f -o \( -type d ! -perm -1000 \) \) -print0 > "$RUNNER_TEMP/writable.list" echo "removing group/other write from $(tr -cd '\0' < "$RUNNER_TEMP/writable.list" | wc -c) entries" sudo xargs -0 -r chmod go-w < "$RUNNER_TEMP/writable.list" + # Files there owned by another regular account (such as a uid 1000 + # geckodriver reached from /usr/bin) go to root, as on a stock + # install; links are changed themselves, never followed. + sudo find /usr /opt -xdev ! -uid 0 ! -uid "$(id -u)" -print0 > "$RUNNER_TEMP/foreign.list" + echo "giving $(tr -cd '\0' < "$RUNNER_TEMP/foreign.list" | wc -c) entries owned by other accounts to root" + sudo xargs -0 -r chown -h root < "$RUNNER_TEMP/foreign.list" - name: Run daemon install flows env: HACK_TEST_REQUIRE_INSTALL_FLOWS: "1" From 611cea7011759ce1fb4d5c4dd9a4c13079d2b7d1 Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Fri, 13 Nov 2026 06:44:55 +0000 Subject: [PATCH 07/10] ci: re-own only other regular accounts' files for the daemon install flows (HACK-2052) System accounts' files stay, so the hosted run still exercises the protected system directory rule and setuid helpers keep their bits. Co-Authored-By: Claude --- .github/workflows/ci.yml | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index df800a343..7609e241b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -829,11 +829,16 @@ jobs: sudo find /usr /opt -xdev -perm /022 \( -type f -o \( -type d ! -perm -1000 \) \) -print0 > "$RUNNER_TEMP/writable.list" echo "removing group/other write from $(tr -cd '\0' < "$RUNNER_TEMP/writable.list" | wc -c) entries" sudo xargs -0 -r chmod go-w < "$RUNNER_TEMP/writable.list" - # Files there owned by another regular account (such as a uid 1000 + # Files there owned by another regular account (uid >= UID_MIN, as + # the product's isSystemAccount draws the line; such as a uid 1000 # geckodriver reached from /usr/bin) go to root, as on a stock - # install; links are changed themselves, never followed. - sudo find /usr /opt -xdev ! -uid 0 ! -uid "$(id -u)" -print0 > "$RUNNER_TEMP/foreign.list" - echo "giving $(tr -cd '\0' < "$RUNNER_TEMP/foreign.list" | wc -c) entries owned by other accounts to root" + # install. System accounts' files stay: the protected system + # directory rule must still meet them. Links are changed + # themselves, never followed. + uid_min=$(awk '$1 == "UID_MIN" { print $2 }' /etc/login.defs 2>/dev/null) + uid_min=${uid_min:-1000} + sudo find /usr /opt -xdev -uid +$((uid_min - 1)) ! -uid "$(id -u)" -print0 > "$RUNNER_TEMP/foreign.list" + echo "giving $(tr -cd '\0' < "$RUNNER_TEMP/foreign.list" | wc -c) entries owned by other regular accounts (uid >= $uid_min) to root" sudo xargs -0 -r chown -h root < "$RUNNER_TEMP/foreign.list" - name: Run daemon install flows env: From 1213425cbe29caa913359a91fa5576365fe3943d Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Fri, 13 Nov 2026 08:03:05 +0000 Subject: [PATCH 08/10] fix(daemon): leave another home's running daemon alone instead of refusing stop (HACK-2052) A unit main process proven to be a hackd holding another home's socket, under a configuration that also names another home, is that home's daemon: stop leaves the unit alone (also without the managed unit file), so hack daemon stop and restart still stop this home's own hackd. A proven foreign hackd under a configuration naming this home is refused with a "may still be starting, run stop again" hint; an unidentifiable process is still refused. The main process is read again just before systemctl stop, and a change is refused. Co-Authored-By: Claude Opus 5.5 --- docs/architecture.md | 17 +++- src/commands/daemon-systemd.ts | 146 ++++++++++++++++++++++++--------- tests/daemon-systemd.test.ts | 125 +++++++++++++++++++++++++++- 3 files changed, 240 insertions(+), 48 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 1410b9d83..a97603b6d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -378,10 +378,19 @@ this home's `daemon/` directory), and `stop` stops only this home's own hackd. W process, `stop` proves from that process which Hack home it serves (a hackd holding a socket in this home's `daemon/` directory), not from the loaded configuration: a reload without a restart changes the configuration, not the -running daemon. A main process that is not proven to be this home's hackd is -refused, with the `systemctl --user status` and `stop` commands. With no live -process (a pending restart), the configuration decides, and `stop` still -cancels the restart. +running daemon. A main process proven to be a hackd that holds another home's +socket is that home's daemon when the configuration also names another home: +the unit is left alone and this home's own hackd is still stopped, whether or +not the managed unit file is present. When the configuration names this home +instead (a reload without a restart), `stop` refuses: the daemon may still be +starting (it has not bound its socket yet), so it says to run `stop` again, +or it serves another home. A main process that cannot be identified (its +process details cannot be read, or it is not a hackd) is refused, with the +`systemctl --user status` and `stop` commands. With no live process (a pending +restart), the configuration decides, and `stop` still cancels the restart. +Just before `systemctl --user stop`, `stop` reads the unit's main process +again and refuses if it changed (a crash and restart while it was checked); +the window left is the few milliseconds between that read and the stop. When the Hack home cannot be read with certainty (for example an `EnvironmentFile=`, an `ExecSearchPath=`, a `PassEnvironment=` naming `HACK_HOME`, `HOME` or `PATH`, a relative `HOME`, or an escaped quote or diff --git a/src/commands/daemon-systemd.ts b/src/commands/daemon-systemd.ts index 3b63dc69a..bc36083ed 100644 --- a/src/commands/daemon-systemd.ts +++ b/src/commands/daemon-systemd.ts @@ -486,12 +486,23 @@ export async function stopViaSystemdIfInstalled(opts: { if (!status?.availability.available) { return none; } + const checkMainProcess = () => + checkMainProcessServesHome({ + status, + daemonRoot: resolve(opts.hackHome, GLOBAL_DAEMON_DIR_NAME), + prove: opts.proveDaemon, + probe: opts.probe, + homePid: opts.pid, + isPidInUnit: opts.isPidInUnit, + parentPid: opts.parentPid, + }); if (!status.installed) { return await stopWithoutUnitFile({ status, unitPath, hackHome: opts.hackHome, deps, + checkMainProcess, }); } // An unknown state (the unit-specific `systemctl show` failed) does not @@ -508,35 +519,21 @@ export async function stopViaSystemdIfInstalled(opts: { // process, what it serves is proven from the process itself (its socket // in this home's daemon directory), not from the loaded configuration, // which a reload without a restart can change under a running daemon. - const mainProcess = await checkMainProcessServesHome({ - status, - daemonRoot: resolve(opts.hackHome, GLOBAL_DAEMON_DIR_NAME), - prove: opts.proveDaemon, - probe: opts.probe, - homePid: opts.pid, - isPidInUnit: opts.isPidInUnit, - parentPid: opts.parentPid, + const decided = await decideUnitStop({ + mainProcess: await checkMainProcess(), + unitHome: () => + checkUnitServesHome({ + hackHome: opts.hackHome, + deps, + action: "stopping", + }), }); - if (mainProcess.kind === "refused") { - return { ...none, refused: mainProcess.message }; - } - if (mainProcess.kind === "none") { - // No live process (a pending restart, or starting up): only the loaded - // configuration says whose it is. Another home's unit is left alone - // (this shell's own direct hackd is still stopped through the - // proven-ownership path); an unreadable home is refused. - const unitHome = await checkUnitServesHome({ - hackHome: opts.hackHome, - deps, - action: "stopping", - }); - if (unitHome.kind === "foreign") { - logger.info({ message: unitHome.message }); - return none; - } - if (unitHome.kind === "unknown") { - return { ...none, refused: unitHome.message }; - } + if (decided.kind === "leave") { + logger.info({ message: decided.message }); + return none; + } + if (decided.kind === "refuse") { + return { ...none, refused: decided.message }; } const ownedPid = opts.pid !== null && @@ -556,6 +553,17 @@ export async function stopViaSystemdIfInstalled(opts: { refused: `${hooksError}. Stopping it could run what that configuration adds: fix it, run systemctl --user daemon-reload, then run hack daemon stop again`, }; } + // The checks above judged the main process systemd reported then; the + // stop acts on whatever runs when it lands. Read it again just before: + // a different process (a crash and restart meanwhile) is refused, so the + // window left is the few milliseconds between this read and the stop. + const latest = await readSystemdServiceStatus({ unitPath, deps }); + if (latest?.mainPid !== status.mainPid) { + return { + ...none, + refused: `Not stopping ${SYSTEMD_UNIT_NAME}: its main process changed while it was being checked (pid ${status.mainPid ?? "none"}, now ${latest?.mainPid ?? "unknown"}). Run hack daemon stop again; to stop it anyway, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, + }; + } const stopped = await systemdServiceAction({ action: "stop", deps }); if (!stopped.ok) { return { handled: false, ownedPid, failed: stopped.error, refused: null }; @@ -563,6 +571,43 @@ export async function stopViaSystemdIfInstalled(opts: { return { handled: true, ownedPid, failed: null, refused: null }; } +/** + * Whether `stop` may stop the installed unit, from its main process and, + * where that does not decide, its loaded configuration (HACK-2052): + * - this home's proven hackd: stop; + * - an unidentifiable process: refuse; + * - a proven hackd holding another home's socket: another home's daemon + * (`leave`, so the caller still stops this home's own hackd) when the + * configuration also names another home; refused when it names this + * home (a reload without a restart, or still starting) or is unreadable; + * - no live process (a pending restart): the configuration decides. + */ +async function decideUnitStop(opts: { + readonly mainProcess: MainProcessCheck; + readonly unitHome: () => ReturnType; +}): Promise< + | { readonly kind: "stop" } + | { readonly kind: "leave" | "refuse"; readonly message: string } +> { + const { mainProcess } = opts; + if (mainProcess.kind === "ours") { + return { kind: "stop" }; + } + if (mainProcess.kind === "refused") { + return { kind: "refuse", message: mainProcess.message }; + } + const unitHome = await opts.unitHome(); + if (unitHome.kind === "foreign") { + return { kind: "leave", message: unitHome.message }; + } + if (unitHome.kind === "unknown") { + return { kind: "refuse", message: unitHome.message }; + } + return mainProcess.kind === "foreign" + ? { kind: "refuse", message: mainProcess.message } + : { kind: "stop" }; +} + /** * `stop` when no unit file is at the managed path (HACK-2052): see the * comments below. Never runs `systemctl stop`; it reports nothing to do or @@ -573,6 +618,7 @@ async function stopWithoutUnitFile(opts: { readonly unitPath: string; readonly hackHome: string; readonly deps: SystemdDeps; + readonly checkMainProcess: () => Promise; }): Promise<{ readonly handled: false; readonly ownedPid: false; @@ -602,17 +648,21 @@ async function stopWithoutUnitFile(opts: { ) { return none; } - // Another home's unit, loaded from its own directory, with no live - // process (a pending restart): not this shell's to stop or refuse over. - // A live process is refused below either way: it cannot be checked - // without the unit file (HACK-2052). + // Another home's unit, loaded from its own directory (for example a + // shell whose HOME points elsewhere): not this shell's to stop or refuse + // over, when nothing runs (a pending restart) or what runs is proven to + // be a hackd holding another home's socket. Anything else is refused + // below: without the unit file it cannot be checked (HACK-2052). const loadedHome = await readSystemdUnitHackHome({ deps }); if ( - status.mainPid === null && loadedHome.kind === "home" && loadedHome.path !== resolve(opts.hackHome) ) { - return none; + const mainProcess = + status.mainPid === null ? null : await opts.checkMainProcess(); + if (mainProcess === null || mainProcess.kind === "foreign") { + return none; + } } return { ...none, @@ -625,9 +675,16 @@ async function stopWithoutUnitFile(opts: { * tied to that process ({@link proveOwnedDaemonProcess}: a hackd image * holding a socket in `daemonRoot`), never from configuration (HACK-2052). * `none`: no live main process (none reported, or it exited meanwhile). - * `refused`: a live process that is not proven to be this home's, with the - * manual status and stop steps. + * `foreign`: a live hackd proven to hold another home's socket, not this + * one's; the caller decides with the loaded configuration, and `message` + * is the refusal to give when they disagree. + * `refused`: a live process that cannot be identified (unreadable, not a + * hackd, or changed while checked), with the manual status and stop steps. */ +type MainProcessCheck = + | { readonly kind: "ours" | "none" } + | { readonly kind: "foreign" | "refused"; readonly message: string }; + async function checkMainProcessServesHome(opts: { readonly status: SystemdServiceStatus; readonly daemonRoot: string; @@ -638,10 +695,7 @@ async function checkMainProcessServesHome(opts: { readonly isPidInUnit?: (pid: number) => Promise; /** Defaults to {@link readLinuxParentPid}; tests pin it. */ readonly parentPid?: (pid: number) => Promise; -}): Promise< - | { readonly kind: "ours" | "none" } - | { readonly kind: "refused"; readonly message: string } -> { +}): Promise { const pid = opts.status.mainPid; if (pid === null) { return { kind: "none" }; @@ -688,6 +742,16 @@ async function checkMainProcessServesHome(opts: { ) { return { kind: "ours" }; } + const manual = `Check: systemctl --user status ${SYSTEMD_UNIT_NAME}; to stop it anyway, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`; + if (proof.reason === "foreign-socket") { + // A hackd without this home's socket: another home's, or one that has + // not bound its socket yet (just started, even if systemd already + // reports it active), so say both. + return { + kind: "foreign", + message: `Not stopping ${SYSTEMD_UNIT_NAME}: its main process (pid ${pid}) is a hackd that does not hold this Hack home's daemon socket (${opts.daemonRoot}), while the unit's configuration names this home. It may still be starting: run hack daemon stop again in a moment. Otherwise it serves another Hack home (a reload changes the unit's configuration, not the running process). ${manual}`, + }; + } // Still starting: its socket may not be bound yet, which is not evidence // of another home. const why = @@ -696,7 +760,7 @@ async function checkMainProcessServesHome(opts: { : "it may serve another Hack home (a reload changes the unit's configuration, not the running process)"; return { kind: "refused", - message: `Not stopping ${SYSTEMD_UNIT_NAME}: its main process (pid ${pid}) is not proven to be the hackd of this Hack home (${opts.daemonRoot}): ${describeDaemonProcessRefusal(proof.reason)}. ${why}. Check: systemctl --user status ${SYSTEMD_UNIT_NAME}; to stop it anyway, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, + message: `Not stopping ${SYSTEMD_UNIT_NAME}: its main process (pid ${pid}) is not proven to be the hackd of this Hack home (${opts.daemonRoot}): ${describeDaemonProcessRefusal(proof.reason)}. ${why}. ${manual}`, }; } diff --git a/tests/daemon-systemd.test.ts b/tests/daemon-systemd.test.ts index 5fc8f6f53..2f11114d4 100644 --- a/tests/daemon-systemd.test.ts +++ b/tests/daemon-systemd.test.ts @@ -3951,8 +3951,12 @@ linuxTest( expect(foreign.handled).toBe(false); expect(foreign.refused).toContain("pid 4242"); expect(foreign.refused).toContain( - "not proven to be the hackd of this Hack home" + "does not hold this Hack home's daemon socket" ); + expect(foreign.refused).toContain( + "the unit's configuration names this home" + ); + expect(foreign.refused).toContain("It may still be starting"); expect(foreign.refused).toContain("systemctl --user status hackd.service"); expect(foreign.refused).toContain("systemctl --user stop hackd.service"); expect(fake.calls).not.toContain("systemctl --user stop hackd.service"); @@ -3978,6 +3982,118 @@ linuxTest( } ); +linuxTest( + "a live daemon of another Hack home is left alone, not refused", + async () => { + // Lead review: a proven hackd holding another home's socket, under a + // unit whose configuration also names another home, is that home's + // daemon. Stop leaves the unit alone with no refusal; the caller then + // stops this home's own direct hackd (it exits 1 only on `refused`, + // and stops its pid itself when `ownedPid` is false). + const fake = fakeSystemd({ active: true }); + await installSystemdUnit({ + unitPath, + unitText: unitText(), + deps: fake.deps, + }); + const stop = () => + stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), + // This home's own hackd, started directly, outside the unit. + pid: 31_337, + isPidInUnit: () => Promise.resolve(false), + hackHome: join(root, ".hack"), + deps: fake.deps, + }); + const leftAlone = async (configure: () => void) => { + fake.active = true; + fake.activeState = null; + fake.mainProcess = "foreign"; + fake.unitEnvironment = "PATH=/usr/bin:/bin HACK_NO_INTERACTIVE=1"; + fake.unsetEnvironment = ""; + fake.managerEnvironment = `HOME=${root}\nPATH=/usr/bin:/bin\n`; + fake.passwdHome = root; + configure(); + fake.calls.length = 0; + expect(await stop()).toEqual({ + handled: false, + ownedPid: false, + failed: null, + refused: null, + }); + expect(fake.calls).not.toContain("systemctl --user stop hackd.service"); + expect(fake.active).toBe(true); + }; + // Another home through a drop-in, the manager's environment, an + // UnsetEnvironment of this home, or the account's home. + await leftAlone(() => { + fake.unitEnvironment = "HACK_NO_INTERACTIVE=1 HACK_HOME=/srv/other-home"; + }); + await leftAlone(() => { + fake.managerEnvironment = `HOME=${root}\nHACK_HOME=/srv/other-home\n`; + }); + await leftAlone(() => { + fake.unitEnvironment = `HACK_HOME=${join(root, ".hack")} HOME=/srv/other`; + fake.unsetEnvironment = "HACK_HOME"; + }); + await leftAlone(() => { + fake.unsetEnvironment = "HOME"; + fake.passwdHome = "/srv/account"; + }); + // Still refused: a process that cannot be identified, even under + // another home's configuration. + fake.unitEnvironment = "HACK_NO_INTERACTIVE=1 HACK_HOME=/srv/other-home"; + fake.unsetEnvironment = ""; + fake.passwdHome = root; + fake.mainProcess = "unknown"; + expect((await stop()).refused).toContain("not proven"); + // No unit file, the unit loaded from another home's directory and + // running: left alone too, and refused when unidentifiable. + await rm(unitPath); + fake.active = true; + fake.mainProcess = "foreign"; + fake.calls.length = 0; + expect((await stop()).refused).toBeNull(); + expect(fake.calls).not.toContain("systemctl --user stop hackd.service"); + fake.mainProcess = "unknown"; + expect((await stop()).refused).toContain("cannot be checked"); + } +); + +linuxTest( + "stop refuses when the unit's main process changes while it is checked", + async () => { + // Lead review: the proof covers the main process seen first; the stop + // acts on whatever runs when it lands. A different MainPID just before + // the stop (a crash and restart meanwhile) is refused. + const fake = fakeSystemd({ active: true }); + await installSystemdUnit({ + unitPath, + unitText: unitText(), + deps: fake.deps, + }); + let proofs = 0; + const proveDaemon = (pid: number) => { + proofs += 1; + // The process is replaced once it has been proven. + fake.mainPidReported = false; + return proveFor(fake)(pid); + }; + fake.calls.length = 0; + const changed = await stopViaSystemdIfInstalled({ + proveDaemon, + pid: null, + hackHome: join(root, ".hack"), + deps: fake.deps, + }); + expect(proofs).toBe(1); + expect(changed.handled).toBe(false); + expect(changed.refused).toContain("changed while it was being checked"); + expect(changed.refused).toContain("hack daemon stop again"); + expect(fake.calls).not.toContain("systemctl --user stop hackd.service"); + } +); + linuxTest( "stop accepts this home's daemon run by another install, through the real proof", async () => { @@ -4046,8 +4162,11 @@ linuxTest( socketOwners = new Set(); fake.activeState = "activating"; const starting = await stop(); - expect(starting.refused).toContain("still starting"); - expect(starting.refused).not.toContain("may serve another Hack home"); + expect(starting.refused).toContain("may still be starting"); + // Even once systemd reports it active (Type=simple), a hackd that has + // not bound its socket gets the same hint. + fake.activeState = "active"; + expect((await stop()).refused).toContain("may still be starting"); } finally { child.kill(); await child.exited; From 48e188d0e70ad0eaf863413b5aff8b7e4d088184 Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Fri, 13 Nov 2026 10:54:24 +0000 Subject: [PATCH 09/10] fix(daemon): refuse unknown unit state, and leave a unit alone only on proof of another home (HACK-2052) Second-family review of #753 at 1213425c: - A failed unit-state or MainPID query (and a failed final re-read) refuses with the manual commands; it no longer counts as "no process", so the configuration alone cannot authorize a stop. - A hackd without this home's socket is left alone only when it is proven to hold a socket in the daemon directory of the home the configuration names (or exited meanwhile); one still starting or with unreadable sockets is refused with the retry hint. - Without the managed unit file, a main process that exits during the proof is handled like no live process (left to its home). - The residual race is described as what it is, not a duration. Co-Authored-By: Claude Opus 5.5 --- docs/architecture.md | 34 +++++--- src/commands/daemon-systemd.ts | 152 ++++++++++++++++++++++++--------- tests/daemon-systemd.test.ts | 123 +++++++++++++++++++++++--- 3 files changed, 243 insertions(+), 66 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index a97603b6d..b8ea15c85 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -366,7 +366,7 @@ cancels a restart systemd has scheduled after a crash. When systemd runs or is about to restart `hackd.service` but the managed unit file is gone (or the unit is loaded from another directory), `stop` refuses and prints the `systemctl --user` commands to review and stop it, instead of reporting hackd as -stopped. `uninstall` does the same in that case instead of reporting +stopped, unless the unit is shown to be another home's (below). `uninstall` does the same in that case instead of reporting that nothing was installed. If systemd cannot report the unit's state at all (the query fails, times out, loses the user bus, or `systemctl` cannot be run), both refuse in the same way. Only a unit systemd reports as inactive or not found, or a @@ -378,19 +378,25 @@ this home's `daemon/` directory), and `stop` stops only this home's own hackd. W process, `stop` proves from that process which Hack home it serves (a hackd holding a socket in this home's `daemon/` directory), not from the loaded configuration: a reload without a restart changes the configuration, not the -running daemon. A main process proven to be a hackd that holds another home's -socket is that home's daemon when the configuration also names another home: -the unit is left alone and this home's own hackd is still stopped, whether or -not the managed unit file is present. When the configuration names this home -instead (a reload without a restart), `stop` refuses: the daemon may still be -starting (it has not bound its socket yet), so it says to run `stop` again, -or it serves another home. A main process that cannot be identified (its -process details cannot be read, or it is not a hackd) is refused, with the -`systemctl --user status` and `stop` commands. With no live process (a pending -restart), the configuration decides, and `stop` still cancels the restart. -Just before `systemctl --user stop`, `stop` reads the unit's main process -again and refuses if it changed (a crash and restart while it was checked); -the window left is the few milliseconds between that read and the stop. +running daemon. A hackd main process that does not hold this home's socket is +left alone only with positive evidence that it is another home's daemon: the +configuration names another home, and the process is proven (identity stable) +to hold a socket in that home's `daemon/` directory, or it exited while being +checked. Then the unit is left alone and this home's own hackd is still +stopped, whether or not the managed unit file is present. Without that +evidence `stop` refuses: the daemon may still be starting (it has not bound +its socket yet), its sockets may be unreadable, or a reload changed the +configuration to this home under a running daemon; the message says to run +`stop` again or how to stop it by hand. A main process that cannot be +identified (its process details cannot be read, or it is not a hackd) is +refused, with the `systemctl --user status` and `stop` commands. With no live +process (a pending restart), the configuration decides, and `stop` still +cancels the restart. A failed query of the unit's state or main process is +not "no process": it is refused with the manual commands, installed unit or +not. Just before `systemctl --user stop`, `stop` reads the unit's state again +and refuses if the main pid changed or the read fails. That narrows the gap +without closing it: `systemctl stop` acts on whatever runs when it lands, so a +change after that read, or a pid reused in between, is not detected. When the Hack home cannot be read with certainty (for example an `EnvironmentFile=`, an `ExecSearchPath=`, a `PassEnvironment=` naming `HACK_HOME`, `HOME` or `PATH`, a relative `HOME`, or an escaped quote or diff --git a/src/commands/daemon-systemd.ts b/src/commands/daemon-systemd.ts index bc36083ed..fc48bf51f 100644 --- a/src/commands/daemon-systemd.ts +++ b/src/commands/daemon-systemd.ts @@ -268,7 +268,13 @@ async function checkUnitServesHome(opts: { readonly action: "starting" | "stopping"; }): Promise< | { readonly kind: "ours" } - | { readonly kind: "foreign" | "unknown"; readonly message: string } + | { + readonly kind: "foreign"; + readonly message: string; + /** The Hack home the unit's configuration names. */ + readonly path: string; + } + | { readonly kind: "unknown"; readonly message: string } > { const unitHome = await readSystemdUnitHackHome({ deps: opts.deps }); const ours = resolve(opts.hackHome); @@ -277,6 +283,7 @@ async function checkUnitServesHome(opts: { ? { kind: "ours" } : { kind: "foreign", + path: unitHome.path, message: `${SYSTEMD_UNIT_NAME} runs hackd for ${unitHome.path}, not ${ours}; not ${opts.action} it from here`, }; } @@ -457,7 +464,10 @@ export async function stopViaSystemdIfInstalled(opts: { * Proves the unit's main process is this home's hackd; defaults to * {@link proveOwnedDaemonProcess} on `/daemon`. Tests pin it. */ - readonly proveDaemon?: (pid: number) => Promise; + readonly proveDaemon?: ( + pid: number, + daemonRoot: string + ) => Promise; /** Test seam for the default proof's observations. */ readonly probe?: DaemonProcessProbe; /** Test seam: a process's parent pid (defaults to /proc). */ @@ -505,9 +515,15 @@ export async function stopViaSystemdIfInstalled(opts: { checkMainProcess, }); } - // An unknown state (the unit-specific `systemctl show` failed) does not - // prove the unit is stopped: stop it anyway, which is harmless when it is - // already inactive. + // A failed state read reports no main process, which is not evidence + // that none runs: one serving another home could be stopped on the + // configuration alone. Refused with the manual steps (HACK-2052). + if (status.stateError !== null) { + return { + ...none, + refused: `Not stopping ${SYSTEMD_UNIT_NAME}: systemd could not report its state or main process (${status.stateError}), so whose daemon it runs cannot be checked. Check: systemctl --user status ${SYSTEMD_UNIT_NAME}; to stop it anyway, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, + }; + } if ( status.activeState !== null && !MANAGER_STOPPABLE_STATES.has(status.activeState) @@ -554,14 +570,19 @@ export async function stopViaSystemdIfInstalled(opts: { }; } // The checks above judged the main process systemd reported then; the - // stop acts on whatever runs when it lands. Read it again just before: - // a different process (a crash and restart meanwhile) is refused, so the - // window left is the few milliseconds between this read and the stop. + // stop acts on whatever runs when it lands. Read it again just before: a + // different main pid (a crash and restart meanwhile), or a read that + // fails, is refused. This narrows the gap without closing it: a change + // after this read, or the same pid reused in between, is not detected. const latest = await readSystemdServiceStatus({ unitPath, deps }); - if (latest?.mainPid !== status.mainPid) { + if ( + latest === null || + latest.stateError !== null || + latest.mainPid !== status.mainPid + ) { return { ...none, - refused: `Not stopping ${SYSTEMD_UNIT_NAME}: its main process changed while it was being checked (pid ${status.mainPid ?? "none"}, now ${latest?.mainPid ?? "unknown"}). Run hack daemon stop again; to stop it anyway, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, + refused: `Not stopping ${SYSTEMD_UNIT_NAME}: its main process changed while it was being checked, or could not be read again (pid ${status.mainPid ?? "none"}, now ${latest === null || latest.stateError !== null ? "unknown" : (latest.mainPid ?? "none")}). Run hack daemon stop again; to stop it anyway, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, }; } const stopped = await systemdServiceAction({ action: "stop", deps }); @@ -576,10 +597,12 @@ export async function stopViaSystemdIfInstalled(opts: { * where that does not decide, its loaded configuration (HACK-2052): * - this home's proven hackd: stop; * - an unidentifiable process: refuse; - * - a proven hackd holding another home's socket: another home's daemon - * (`leave`, so the caller still stops this home's own hackd) when the - * configuration also names another home; refused when it names this - * home (a reload without a restart, or still starting) or is unreadable; + * - a hackd without this home's socket: left alone (the caller still stops + * this home's own hackd) only with positive evidence that it is another + * home's: the configuration names that home and the process is proven + * to hold a socket in that home's daemon directory (or it exited + * meanwhile). Otherwise refused: it may still be starting, its sockets + * may be unreadable, or a reload changed the configuration under it; * - no live process (a pending restart): the configuration decides. */ async function decideUnitStop(opts: { @@ -597,15 +620,39 @@ async function decideUnitStop(opts: { return { kind: "refuse", message: mainProcess.message }; } const unitHome = await opts.unitHome(); - if (unitHome.kind === "foreign") { - return { kind: "leave", message: unitHome.message }; - } if (unitHome.kind === "unknown") { return { kind: "refuse", message: unitHome.message }; } - return mainProcess.kind === "foreign" - ? { kind: "refuse", message: mainProcess.message } - : { kind: "stop" }; + if (mainProcess.kind === "none") { + return unitHome.kind === "foreign" + ? { kind: "leave", message: unitHome.message } + : { kind: "stop" }; + } + if (unitHome.kind === "ours") { + return { kind: "refuse", message: mainProcess.message }; + } + return (await foreignHomeProven({ mainProcess, home: unitHome.path })) + ? { kind: "leave", message: unitHome.message } + : { + kind: "refuse", + message: mainProcess.unprovenMessage(unitHome.path), + }; +} + +/** + * Positive evidence that a hackd without this home's socket is `home`'s + * daemon: proven ({@link proveOwnedDaemonProcess}, identity stable) to hold + * a socket in that home's daemon directory, or gone meanwhile (then + * nothing runs, and the configuration decides, as for a pending restart). + */ +async function foreignHomeProven(opts: { + readonly mainProcess: Extract; + readonly home: string; +}): Promise { + const proof = await opts.mainProcess.proveIn( + resolve(opts.home, GLOBAL_DAEMON_DIR_NAME) + ); + return proof.kind === "owned" || proof.reason === "not-running"; } /** @@ -650,9 +697,10 @@ async function stopWithoutUnitFile(opts: { } // Another home's unit, loaded from its own directory (for example a // shell whose HOME points elsewhere): not this shell's to stop or refuse - // over, when nothing runs (a pending restart) or what runs is proven to - // be a hackd holding another home's socket. Anything else is refused - // below: without the unit file it cannot be checked (HACK-2052). + // over, when nothing runs (a pending restart, or its main process exited + // while checked) or what runs is proven to hold a socket in that home's + // daemon directory. Anything else is refused below: without the unit + // file it cannot be checked (HACK-2052). const loadedHome = await readSystemdUnitHackHome({ deps }); if ( loadedHome.kind === "home" && @@ -660,7 +708,12 @@ async function stopWithoutUnitFile(opts: { ) { const mainProcess = status.mainPid === null ? null : await opts.checkMainProcess(); - if (mainProcess === null || mainProcess.kind === "foreign") { + if ( + mainProcess === null || + mainProcess.kind === "none" || + (mainProcess.kind === "foreign" && + (await foreignHomeProven({ mainProcess, home: loadedHome.path }))) + ) { return none; } } @@ -675,20 +728,34 @@ async function stopWithoutUnitFile(opts: { * tied to that process ({@link proveOwnedDaemonProcess}: a hackd image * holding a socket in `daemonRoot`), never from configuration (HACK-2052). * `none`: no live main process (none reported, or it exited meanwhile). - * `foreign`: a live hackd proven to hold another home's socket, not this - * one's; the caller decides with the loaded configuration, and `message` - * is the refusal to give when they disagree. + * `foreign`: a live hackd that does not hold this home's socket. That is + * not yet evidence of another home (it may still be starting, or its + * sockets may be unreadable): the caller proves it against the home the + * configuration names with `proveIn`. `message` is the refusal when the + * configuration names this home; `unprovenMessage` when it names another + * home the process is not proven to serve. * `refused`: a live process that cannot be identified (unreadable, not a * hackd, or changed while checked), with the manual status and stop steps. */ type MainProcessCheck = - | { readonly kind: "ours" | "none" } - | { readonly kind: "foreign" | "refused"; readonly message: string }; + | { readonly kind: "ours" } + | { readonly kind: "none" } + | { readonly kind: "refused"; readonly message: string } + | { + readonly kind: "foreign"; + readonly message: string; + readonly unprovenMessage: (home: string) => string; + readonly proveIn: (daemonRoot: string) => Promise; + }; async function checkMainProcessServesHome(opts: { readonly status: SystemdServiceStatus; readonly daemonRoot: string; - readonly prove?: (pid: number) => Promise; + /** Proves `pid` is a hackd holding a socket in `daemonRoot`; tests pin it. */ + readonly prove?: ( + pid: number, + daemonRoot: string + ) => Promise; readonly probe?: DaemonProcessProbe; /** This home's hackd per its pid file, if one is running. */ readonly homePid: number | null; @@ -707,15 +774,16 @@ async function checkMainProcessServesHome(opts: { ...(await defaultDaemonIdentityExpectation()), anyBunEntry: true, }; - const prove = - opts.prove ?? - ((candidate: number) => - proveOwnedDaemonProcess({ - pid: candidate, - daemonRoot: opts.daemonRoot, - expectation, - probe: opts.probe, - })); + const proveAt = (candidate: number, daemonRoot: string) => + opts.prove + ? opts.prove(candidate, daemonRoot) + : proveOwnedDaemonProcess({ + pid: candidate, + daemonRoot, + expectation, + probe: opts.probe, + }); + const prove = (candidate: number) => proveAt(candidate, opts.daemonRoot); const proof = await prove(pid); if (proof.kind === "owned") { return { kind: "ours" }; @@ -746,9 +814,13 @@ async function checkMainProcessServesHome(opts: { if (proof.reason === "foreign-socket") { // A hackd without this home's socket: another home's, or one that has // not bound its socket yet (just started, even if systemd already - // reports it active), so say both. + // reports it active), or whose sockets cannot be read. The caller + // proves it against the home the configuration names. return { kind: "foreign", + proveIn: (daemonRoot) => proveAt(pid, daemonRoot), + unprovenMessage: (home) => + `Not stopping ${SYSTEMD_UNIT_NAME}: its main process (pid ${pid}) is a hackd that is not proven to hold a socket of this Hack home (${opts.daemonRoot}) or of the one its configuration names (${resolve(home, GLOBAL_DAEMON_DIR_NAME)}). It may still be starting: run hack daemon stop again in a moment. Otherwise its sockets could not be read. ${manual}`, message: `Not stopping ${SYSTEMD_UNIT_NAME}: its main process (pid ${pid}) is a hackd that does not hold this Hack home's daemon socket (${opts.daemonRoot}), while the unit's configuration names this home. It may still be starting: run hack daemon stop again in a moment. Otherwise it serves another Hack home (a reload changes the unit's configuration, not the running process). ${manual}`, }; } diff --git a/tests/daemon-systemd.test.ts b/tests/daemon-systemd.test.ts index 2f11114d4..9ca1349eb 100644 --- a/tests/daemon-systemd.test.ts +++ b/tests/daemon-systemd.test.ts @@ -96,6 +96,8 @@ type Fake = { enableError: string | null; /** Fails the unit-specific `show` while set. */ showFails: boolean; + /** Only the unit-state query (ActiveState, MainPID...) fails. */ + stateShowFails: boolean; /** Overrides ActiveState, e.g. "activating" during a restart delay. */ activeState: string | null; needsReload: boolean; @@ -125,7 +127,7 @@ type Fake = { * What proving the unit's main process (MainPID) finds: this home's * hackd, another home's (its socket elsewhere), or nothing readable. */ - mainProcess: "ours" | "foreign" | "unknown"; + mainProcess: "ours" | "foreign" | "unbound" | "exited" | "unknown"; /** False: the unit reports no main process (e.g. a pending restart). */ mainPidReported: boolean; /** Namespace settings (`RootDirectory=...` lines) systemd reports. */ @@ -183,6 +185,7 @@ function fakeSystemd(opts: FakeOptions = {}): Fake { failed: false, enableError: null, showFails: false, + stateShowFails: false, activeState: null, needsReload: false, unitEnvironment: `PATH=${sysBin} HACK_NO_INTERACTIVE=1`, @@ -303,6 +306,17 @@ function fakeSystemd(opts: FakeOptions = {}): Fake { ) ); } + if ( + verb === "show" && + state.stateShowFails && + args.some((arg) => arg.includes("MainPID")) + ) { + return Promise.resolve({ + exitCode: 1, + stdout: "", + stderr: "Failed to get properties: Connection timed out", + }); + } if (verb === "show") { return Promise.resolve( ok( @@ -375,10 +389,22 @@ function fakeSystemd(opts: FakeOptions = {}): Fake { } /** The stop path's proof of the unit's main process, as the fake says. */ -function proveFor(fake: Fake): (pid: number) => Promise { - return (pid) => - Promise.resolve( - fake.mainProcess === "ours" +/** + * The proof for the fake's main process, by daemon directory: `ours` holds + * this home's socket; `foreign` holds a socket of any other home (positive + * evidence of another home); `unbound` is a hackd holding neither (still + * starting, or sockets unreadable); `unknown` cannot be identified. + */ +function proveFor( + fake: Fake +): (pid: number, daemonRoot: string) => Promise { + return (pid, daemonRoot) => { + const thisHome = daemonRoot === join(root, ".hack", "daemon"); + const owned = + (fake.mainProcess === "ours" && thisHome) || + (fake.mainProcess === "foreign" && !thisHome); + return Promise.resolve( + owned ? ({ kind: "owned", pid, @@ -388,11 +414,14 @@ function proveFor(fake: Fake): (pid: number) => Promise { : { kind: "refused", reason: - fake.mainProcess === "foreign" - ? "foreign-socket" - : "identity-unknown", + fake.mainProcess === "unknown" + ? "identity-unknown" + : fake.mainProcess === "exited" + ? "not-running" + : "foreign-socket", } ); + }; } function unitText(): string { @@ -1727,12 +1756,61 @@ linuxTest( deps: fake.deps, }); expect(result.handled).toBe(false); - expect(result.refused).toContain("could not be read"); + expect(result.refused).toContain("cannot be checked"); expect(result.refused).toContain("systemctl --user stop hackd.service"); expect(fake.calls).not.toContain("systemctl --user stop hackd.service"); } ); +linuxTest( + "stop refuses when only the unit's state query fails, even with this home's configuration", + async () => { + // Second-family review blocker: a failed state query reported no main + // process, the environment query (which succeeded) named this home, + // and stop ran with no process proof; it could stop a daemon serving + // another home after a reload. + const fake = fakeSystemd({ active: true }); + await installSystemdUnit({ + unitPath, + unitText: unitText(), + deps: fake.deps, + }); + fake.active = true; + fake.stateShowFails = true; + fake.calls.length = 0; + let proofs = 0; + const result = await stopViaSystemdIfInstalled({ + proveDaemon: (pid, daemonRoot) => { + proofs += 1; + return proveFor(fake)(pid, daemonRoot); + }, + pid: null, + hackHome: join(root, ".hack"), + deps: fake.deps, + }); + expect(result.handled).toBe(false); + expect(result.refused).toContain( + "could not report its state or main process" + ); + expect(result.refused).toContain("systemctl --user stop hackd.service"); + expect(proofs).toBe(0); + expect(fake.calls).not.toContain("systemctl --user stop hackd.service"); + // The final re-read failing after a successful proof: refused too. + fake.stateShowFails = false; + const reread = await stopViaSystemdIfInstalled({ + proveDaemon: (pid, daemonRoot) => { + fake.stateShowFails = true; + return proveFor(fake)(pid, daemonRoot); + }, + pid: null, + hackHome: join(root, ".hack"), + deps: fake.deps, + }); + expect(reread.refused).toContain("could not be read again"); + expect(fake.calls).not.toContain("systemctl --user stop hackd.service"); + } +); + linuxTest( "stop refuses a unit whose effective Hack home is another one", async () => { @@ -4047,14 +4125,35 @@ linuxTest( fake.passwdHome = root; fake.mainProcess = "unknown"; expect((await stop()).refused).toContain("not proven"); + // Second-family review: holding no socket of this home is not evidence + // of another. A hackd that holds neither this home's socket nor one of + // the home the configuration names (still starting, or its sockets + // unreadable) is refused with the retry hint, not left alone. + fake.mainProcess = "unbound"; + const unbound = await stop(); + expect(unbound.refused).toContain( + "not proven to hold a socket of this Hack home" + ); + expect(unbound.refused).toContain("/srv/other-home/daemon"); + expect(unbound.refused).toContain("may still be starting"); + expect(fake.calls).not.toContain("systemctl --user stop hackd.service"); + // A main process that exited while it was checked: nothing runs, so the + // configuration decides, as for a pending restart (left alone). + fake.mainProcess = "exited"; + expect((await stop()).refused).toBeNull(); // No unit file, the unit loaded from another home's directory and - // running: left alone too, and refused when unidentifiable. + // running: left alone too when proven another home's or exited, and + // refused when unproven or unidentifiable. await rm(unitPath); fake.active = true; fake.mainProcess = "foreign"; fake.calls.length = 0; expect((await stop()).refused).toBeNull(); expect(fake.calls).not.toContain("systemctl --user stop hackd.service"); + fake.mainProcess = "exited"; + expect((await stop()).refused).toBeNull(); + fake.mainProcess = "unbound"; + expect((await stop()).refused).toContain("cannot be checked"); fake.mainProcess = "unknown"; expect((await stop()).refused).toContain("cannot be checked"); } @@ -4073,11 +4172,11 @@ linuxTest( deps: fake.deps, }); let proofs = 0; - const proveDaemon = (pid: number) => { + const proveDaemon = (pid: number, daemonRoot: string) => { proofs += 1; // The process is replaced once it has been proven. fake.mainPidReported = false; - return proveFor(fake)(pid); + return proveFor(fake)(pid, daemonRoot); }; fake.calls.length = 0; const changed = await stopViaSystemdIfInstalled({ From ea6883642188e6db6968b2738f374ccbb9b50ad3 Mon Sep 17 00:00:00 2001 From: Kit <344842+roodboi@users.noreply.github.com> Date: Fri, 13 Nov 2026 11:11:09 +0000 Subject: [PATCH 10/10] fix(daemon): refuse when the manager is unavailable on the final re-read (HACK-2052) A pending restart (no MainPID) whose final status read found the user manager unavailable compared null with null and stopped; an unavailable manager on that read is now refused like any other failed read. Co-Authored-By: Claude Opus 5.5 --- src/commands/daemon-systemd.ts | 6 +++-- tests/daemon-systemd.test.ts | 48 ++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/commands/daemon-systemd.ts b/src/commands/daemon-systemd.ts index fc48bf51f..b0d501baf 100644 --- a/src/commands/daemon-systemd.ts +++ b/src/commands/daemon-systemd.ts @@ -572,17 +572,19 @@ export async function stopViaSystemdIfInstalled(opts: { // The checks above judged the main process systemd reported then; the // stop acts on whatever runs when it lands. Read it again just before: a // different main pid (a crash and restart meanwhile), or a read that - // fails, is refused. This narrows the gap without closing it: a change + // fails (the manager unavailable, or the unit's state not reported), is + // refused, also when no main process was seen (a pending restart). This narrows the gap without closing it: a change // after this read, or the same pid reused in between, is not detected. const latest = await readSystemdServiceStatus({ unitPath, deps }); if ( latest === null || + !latest.availability.available || latest.stateError !== null || latest.mainPid !== status.mainPid ) { return { ...none, - refused: `Not stopping ${SYSTEMD_UNIT_NAME}: its main process changed while it was being checked, or could not be read again (pid ${status.mainPid ?? "none"}, now ${latest === null || latest.stateError !== null ? "unknown" : (latest.mainPid ?? "none")}). Run hack daemon stop again; to stop it anyway, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, + refused: `Not stopping ${SYSTEMD_UNIT_NAME}: its main process changed while it was being checked, or could not be read again (pid ${status.mainPid ?? "none"}, now ${latest === null || !latest.availability.available || latest.stateError !== null ? "unknown" : (latest.mainPid ?? "none")}). Run hack daemon stop again; to stop it anyway, run: systemctl --user stop ${SYSTEMD_UNIT_NAME}`, }; } const stopped = await systemdServiceAction({ action: "stop", deps }); diff --git a/tests/daemon-systemd.test.ts b/tests/daemon-systemd.test.ts index 9ca1349eb..3f79163bc 100644 --- a/tests/daemon-systemd.test.ts +++ b/tests/daemon-systemd.test.ts @@ -98,6 +98,11 @@ type Fake = { showFails: boolean; /** Only the unit-state query (ActiveState, MainPID...) fails. */ stateShowFails: boolean; + /** + * When set, the manager probe (`show --property=Version`) succeeds this + * many more times and then fails, as if the user bus went away. + */ + versionProbesBeforeFailure: number | null; /** Overrides ActiveState, e.g. "activating" during a restart delay. */ activeState: string | null; needsReload: boolean; @@ -186,6 +191,7 @@ function fakeSystemd(opts: FakeOptions = {}): Fake { enableError: null, showFails: false, stateShowFails: false, + versionProbesBeforeFailure: null, activeState: null, needsReload: false, unitEnvironment: `PATH=${sysBin} HACK_NO_INTERACTIVE=1`, @@ -228,6 +234,16 @@ function fakeSystemd(opts: FakeOptions = {}): Fake { const verbs = args.filter((a) => !a.startsWith("--")); const verb = verbs[0]; if (verb === "show" && args.includes("--property=Version")) { + if (state.versionProbesBeforeFailure !== null) { + if (state.versionProbesBeforeFailure <= 0) { + return Promise.resolve({ + exitCode: 1, + stdout: "", + stderr: "Failed to connect to bus: No such file or directory", + }); + } + state.versionProbesBeforeFailure -= 1; + } return Promise.resolve( opts.probeError ? { @@ -1808,6 +1824,38 @@ linuxTest( }); expect(reread.refused).toContain("could not be read again"); expect(fake.calls).not.toContain("systemctl --user stop hackd.service"); + // Second-family review: a pending restart (no MainPID, configuration + // naming this home) whose final re-read finds the manager unavailable + // was stopped, since both reads had no main pid. Refused now. + fake.stateShowFails = false; + fake.mainPidReported = false; + fake.activeState = "activating"; + fake.versionProbesBeforeFailure = 1; + fake.calls.length = 0; + const unavailable = await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), + pid: null, + hackHome: join(root, ".hack"), + deps: fake.deps, + }); + expect(unavailable.handled).toBe(false); + expect(unavailable.refused).toContain("could not be read again"); + expect(unavailable.refused).toContain( + "systemctl --user stop hackd.service" + ); + expect(fake.calls).not.toContain("systemctl --user stop hackd.service"); + // The same pending restart with the manager reachable: cancelled. + fake.versionProbesBeforeFailure = null; + expect( + ( + await stopViaSystemdIfInstalled({ + proveDaemon: proveFor(fake), + pid: null, + hackHome: join(root, ".hack"), + deps: fake.deps, + }) + ).handled + ).toBe(true); } );