From e2cb2e07d2f9fd560f5179ffea7cd9e674fa1079 Mon Sep 17 00:00:00 2001 From: master Date: Tue, 1 Sep 2026 00:39:50 +0200 Subject: [PATCH 1/6] fix(orcad): refuse a data root that cannot hold the daemon's socket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A data root long enough to push the daemon's endpoint past `sun_path` killed the daemon at startup — but orcad's own listener name is about 5 bytes shorter, so there is a band where orcad came up, reported ready, and served clients with terminal survival silently off. Every terminal then died on the next restart, which is the one thing forking a detached daemon exists to prevent. Found on DSM: a root of 86 characters reproduced it, 84 was fine. orcad now refuses at startup with `ORCAD_EXIT_CONFIGURATION` (78). This is not one of the unenumerable runtime reasons `startOrcadDaemon` fail-opens for: it is known before anything is attempted, permanent until an operator changes a value they supplied, and pure arithmetic. That puts it with the bind address and the instance lock, which already refuse, and `RestartPreventExitStatus=78` keeps a supervisor from restart-spinning on it. The check runs ahead of the instance lock deliberately — it reads nothing and creates nothing, so refusing a root should not first take a lock on it. **The limit is verified against the kernel rather than asserted**, because past it there is no single failure to detect: on Linux a 109-110 byte path fails EADDRINUSE against a free name, and 111+ reports a successful `listen` with no directory entry at all — which looks healthy to everything except the dev+ino reads daemon endpoint ownership is built on. The test binds at the limit and one byte past it and asserts a stat-able entry, so a wrong constant fails it. Supporting changes, all in service of that: - **Consolidate the daemon's endpoint path builders into one leaf module** (`daemon-runtime-paths.ts`), so the budget measures the real names. `daemon-spawner` re-exports `getDaemonSocketPath` / `getDaemonTokenPath` / `getDaemonPidPath` from it rather than moving them, so every existing caller imports the same name from the same place and only the definition moved. `daemon-endpoint-ownership` likewise re-exports `getDaemonSocketBindPath`. - **Measure bytes, not characters.** A non-ASCII root spends more of the budget than its length suggests. `ssh-control-socket` now shares the constant and the byte measurement instead of keeping its own copy of the 104/108 split. Boundary verified end to end against the built orcad: 85 starts with a live daemon socket, 86 exits 78. Refs #10726 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UacRCfD4XPe29bD3ZWNc57 --- src/main/daemon/daemon-endpoint-ownership.ts | 13 +-- src/main/daemon/daemon-launch-paths.ts | 5 +- src/main/daemon/daemon-runtime-paths.test.ts | 92 +++++++++++++++++ src/main/daemon/daemon-runtime-paths.ts | 99 +++++++++++++++++++ src/main/daemon/daemon-spawner.ts | 35 ++----- .../orcad-daemon-socket-preflight.test.ts | 26 +++++ .../orcad/orcad-daemon-socket-preflight.ts | 37 +++++++ src/main/orcad/orcad-entry.ts | 19 +++- src/main/ssh/ssh-control-socket.ts | 5 +- src/shared/unix-socket-path-budget.test.ts | 34 +++++++ src/shared/unix-socket-path-budget.ts | 48 +++++++++ 11 files changed, 369 insertions(+), 44 deletions(-) create mode 100644 src/main/daemon/daemon-runtime-paths.test.ts create mode 100644 src/main/daemon/daemon-runtime-paths.ts create mode 100644 src/main/orcad/orcad-daemon-socket-preflight.test.ts create mode 100644 src/main/orcad/orcad-daemon-socket-preflight.ts create mode 100644 src/shared/unix-socket-path-budget.test.ts create mode 100644 src/shared/unix-socket-path-budget.ts diff --git a/src/main/daemon/daemon-endpoint-ownership.ts b/src/main/daemon/daemon-endpoint-ownership.ts index eb43a1fdd19c..b8396efd51ee 100644 --- a/src/main/daemon/daemon-endpoint-ownership.ts +++ b/src/main/daemon/daemon-endpoint-ownership.ts @@ -1,8 +1,6 @@ /* Who may serve on the daemon's canonical endpoint name: only a daemon publishing itself onto it, and only by replacing an entry it has itself just proven dead. Rationale and traps: AGENTS.md. */ -import { randomBytes } from 'node:crypto' import { linkSync, lstatSync, renameSync, statSync, unlinkSync } from 'node:fs' -import { dirname, join } from 'node:path' import { endpointIsProvenDead, type SocketProbeOutcome } from './daemon-endpoint-probe' // Retried only when another publisher demonstrably took the name, so a small bound suffices. @@ -11,15 +9,8 @@ const PUBLISH_ATTEMPTS = 3 /** A daemon endpoint, identified by its directory entry. dev+ino only; birth time is unreliable. */ export type DaemonSocketIdentity = { dev: bigint; ino: bigint } -/** - * A private, same-directory name to bind before publishing the canonical endpoint. - * - * `.p`, not the `.b` this used to be: released builds sweep that pattern on age alone. Replaces the - * basename rather than extending the path, so the ~104-byte `sockaddr_un.sun_path` budget holds. - */ -export function getDaemonSocketBindPath(socketPath: string): string { - return join(dirname(socketPath), `.p${randomBytes(5).toString('hex')}`) -} +/** Defined with the other endpoint names; the budget it protects is checked there too. */ +export { getDaemonSocketBindPath } from './daemon-runtime-paths' /** * Only `occupied` establishes that someone else owns the endpoint, and only then may the caller diff --git a/src/main/daemon/daemon-launch-paths.ts b/src/main/daemon/daemon-launch-paths.ts index 9b5b2c3dca84..c5eb3b2d169c 100644 --- a/src/main/daemon/daemon-launch-paths.ts +++ b/src/main/daemon/daemon-launch-paths.ts @@ -6,10 +6,13 @@ import { getDaemonLogFilePath } from '../observability/logs-directory' import { DaemonClient } from './client' import { daemonRecoveryProbeTimeoutMs } from './daemon-recovery-budget' import { remainingDaemonRequestTimeoutMs } from './daemon-request-deadline' +// From the definition, not daemon-spawner's re-export: many suites mock daemon-spawner, and +// reaching through it for a symbol they have no reason to stub breaks them. +import { getDaemonRuntimeDirPath } from './daemon-runtime-paths' import { PROTOCOL_VERSION, type ListSessionsResult } from './types' export function getDaemonRuntimeDir(): string { - const dir = join(getAppEnvironment().getPath('userData'), 'daemon') + const dir = getDaemonRuntimeDirPath(getAppEnvironment().getPath('userData')) mkdirSync(dir, { recursive: true }) return dir } diff --git a/src/main/daemon/daemon-runtime-paths.test.ts b/src/main/daemon/daemon-runtime-paths.test.ts new file mode 100644 index 000000000000..4678a86cf98e --- /dev/null +++ b/src/main/daemon/daemon-runtime-paths.test.ts @@ -0,0 +1,92 @@ +import { describe, expect, it } from 'vitest' +import { createServer } from 'node:net' +import { existsSync, mkdirSync, mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { + checkDaemonSocketPathBudget, + getDaemonRuntimeDirPath, + getDaemonSocketBindPath, + getDaemonSocketPath +} from './daemon-runtime-paths' +import { UNIX_SOCKET_PATH_LIMIT, unixSocketPathBytes } from '../../shared/unix-socket-path-budget' + +/** A data root whose daemon socket path lands on exactly `bytes`, or null if it cannot. */ +function rootForSocketBytes(base: string, bytes: number): string | null { + const sample = `${base}/r` + const overhead = + unixSocketPathBytes(getDaemonSocketPath(getDaemonRuntimeDirPath(sample))) - + unixSocketPathBytes(sample) + const rootBytes = bytes - overhead + const prefix = `${base}/` + if (rootBytes < unixSocketPathBytes(prefix)) { + return null + } + return prefix + 'x'.repeat(rootBytes - unixSocketPathBytes(prefix)) +} + +describe('daemon socket path budget', () => { + it('measures the canonical endpoint, which is the longest name bound', () => { + const root = '/home/orca/.orca' + const socketPath = getDaemonSocketPath(getDaemonRuntimeDirPath(root)) + const budget = checkDaemonSocketPathBudget(root) + expect(budget.longestPath).toBe(socketPath) + // The private bind name is shorter by construction; if it ever stops being, the budget + // follows it rather than silently under-measuring. + expect(unixSocketPathBytes(getDaemonSocketBindPath(socketPath))).toBeLessThan(budget.bytes) + }) + + it('fits an ordinary root and refuses a long one', () => { + expect(checkDaemonSocketPathBudget('/home/orca/.orca').fits).toBe(true) + expect(checkDaemonSocketPathBudget(`/home/${'x'.repeat(200)}/.orca`).fits).toBe(false) + }) + + it.skipIf(process.platform === 'win32')('flips at exactly the limit', () => { + const atLimit = rootForSocketBytes('/tmp', UNIX_SOCKET_PATH_LIMIT) + const overLimit = rootForSocketBytes('/tmp', UNIX_SOCKET_PATH_LIMIT + 1) + expect(atLimit).not.toBeNull() + expect(checkDaemonSocketPathBudget(atLimit as string).fits).toBe(true) + expect(checkDaemonSocketPathBudget(overLimit as string).fits).toBe(false) + }) +}) + +/** + * Ties the constant to the kernel. Without this the budget is a number in a file that nothing + * disagrees with, and the defect it exists to prevent — a daemon that cannot bind — is exactly + * what a wrong number reintroduces. + * + * The assertion is "a socket exists at the path", not an errno, because past the limit there is + * no single failure: on Linux a slightly-too-long path fails EADDRINUSE against a free name, and + * a longer one reports a successful listen with no directory entry at all. A stat-able entry is + * the property `publishDaemonEndpoint` is built on — it identifies the endpoint by dev+ino — so + * it is the property worth asserting. + */ +describe('the limit is the kernel’s', () => { + function bindable(socketPath: string): Promise { + mkdirSync(dirname(socketPath), { recursive: true }) + const server = createServer() + return new Promise((resolve) => { + server.once('error', () => resolve(false)) + server.listen(socketPath, () => resolve(existsSync(socketPath))) + }).finally(() => new Promise((done) => server.close(() => done(null)))) + } + + it.skipIf(process.platform === 'win32')( + 'binds at the limit and produces no endpoint past it', + async () => { + // A short base: the socket path has to be built to an exact length, and macOS tmpdir is + // already long enough to make that impossible. + const base = mkdtempSync(join(tmpdir(), 'sb-')) + try { + const under = join(base, 'a'.repeat(UNIX_SOCKET_PATH_LIMIT - base.length - 2)) + const over = `${under}bb` + expect(unixSocketPathBytes(under)).toBe(UNIX_SOCKET_PATH_LIMIT - 1) + expect(await bindable(under)).toBe(true) + expect(unixSocketPathBytes(over)).toBe(UNIX_SOCKET_PATH_LIMIT + 1) + expect(await bindable(over)).toBe(false) + } finally { + rmSync(base, { recursive: true, force: true }) + } + } + ) +}) diff --git a/src/main/daemon/daemon-runtime-paths.ts b/src/main/daemon/daemon-runtime-paths.ts new file mode 100644 index 000000000000..12d929fcf763 --- /dev/null +++ b/src/main/daemon/daemon-runtime-paths.ts @@ -0,0 +1,99 @@ +/** + * Every path the daemon's endpoint is built from, and whether a data root leaves room for it. + * + * One leaf module, importing only node builtins and the protocol version, for two reasons: the + * names are a single source of truth shared by the spawner, the ownership protocol and orcad, + * and the doctor needs the arithmetic below without dragging the daemon into the CLI's graph. + */ +import { createHash, randomBytes } from 'node:crypto' +import { dirname, join } from 'node:path' +import { PROTOCOL_VERSION } from './daemon-protocol-version' +import { + measureUnixSocketPathBudget, + UNIX_SOCKET_PATH_LIMIT, + type UnixSocketPathBudget +} from '../../shared/unix-socket-path-budget' + +/** + * The daemon's runtime directory, as a path only. Separate from `getDaemonRuntimeDir`, which + * also creates it: the socket-path budget has to measure a root before anything touches it. + */ +export function getDaemonRuntimeDirPath(userDataPath: string): string { + return join(userDataPath, 'daemon') +} + +export function getDaemonSocketPath( + runtimeDir: string, + protocolVersion = PROTOCOL_VERSION +): string { + // Why: Windows IPC servers use named pipes rather than filesystem socket + // files. Include the protocol version in the endpoint name so a daemon from + // an older build is never reused after a breaking protocol change. + if (process.platform === 'win32') { + const suffix = createHash('sha256').update(runtimeDir).digest('hex').slice(0, 12) + return `\\\\?\\pipe\\orca-terminal-host-v${protocolVersion}-${suffix}` + } + return join(runtimeDir, `daemon-v${protocolVersion}.sock`) +} + +export function getDaemonTokenPath(runtimeDir: string, protocolVersion = PROTOCOL_VERSION): string { + return join(runtimeDir, `daemon-v${protocolVersion}.token`) +} + +export function getDaemonPidPath(runtimeDir: string, protocolVersion = PROTOCOL_VERSION): string { + return join(runtimeDir, `daemon-v${protocolVersion}.pid`) +} + +/** + * A private, same-directory name to bind before publishing the canonical endpoint. + * + * `.p`, not the `.b` this used to be: released builds sweep that pattern on age alone. Replaces + * the basename rather than extending the path, so the `sockaddr_un.sun_path` budget holds. + */ +export function getDaemonSocketBindPath(socketPath: string): string { + return join(dirname(socketPath), `.p${randomBytes(5).toString('hex')}`) +} + +/** + * Whether a data root leaves room for the daemon's socket to exist. + * + * Why a startup question and not a runtime one: the endpoint lives under the data root, so a + * long enough root pushes it past `sockaddr_un.sun_path` and the daemon dies before it serves + * anything. orcad's own listener spends a few bytes less, so there is a band of root lengths + * where orcad comes up, reports ready, and serves clients with terminal survival silently + * switched off — every terminal then dies on the next restart, which is the one thing forking + * a detached daemon exists to prevent. + * + * Pure arithmetic: no probe, no privileges, no filesystem. That is what lets the same check + * answer for a host the doctor is only reading a unit file about. + */ +export function checkDaemonSocketPathBudget(userDataPath: string): UnixSocketPathBudget { + const socketPath = getDaemonSocketPath(getDaemonRuntimeDirPath(userDataPath)) + if (process.platform === 'win32') { + // Named pipes are not filesystem paths and carry no sun_path budget. + return { longestPath: socketPath, bytes: 0, limit: UNIX_SOCKET_PATH_LIMIT, fits: true } + } + // Both names matter: the daemon binds the private one to listen, and every client connects to + // the canonical one. The bind name is random but fixed-width, so measuring one is measuring + // all of them — and measuring both means a change to either name cannot go unnoticed here. + return measureUnixSocketPathBudget([socketPath, getDaemonSocketBindPath(socketPath)]) +} + +/** One wording, so the startup refusal and the doctor finding cannot drift apart. */ +export function describeDaemonSocketPathOverflow(budget: UnixSocketPathBudget): string { + return ( + `The terminal daemon's socket path needs ${budget.bytes} bytes and the kernel accepts ` + + `${budget.limit} (${budget.longestPath}). The daemon cannot start, so terminals would ` + + 'not survive a restart.' + ) +} + +/** + * Names only mechanisms that exist. It previously offered `--user-data `, which orcad + * does not accept and — because the service commands ignored unrecognised flags — did not + * reject either: an operator following this advice got exit 0 and an `[OK] socket path fits` + * measured against the shell's own root, and would reasonably conclude it was fixed. + */ +export const DAEMON_SOCKET_PATH_REMEDY = + 'Move the data root to a shorter path: set ORCA_USER_DATA, and regenerate the service ' + + 'definition with it set if one is installed.' diff --git a/src/main/daemon/daemon-spawner.ts b/src/main/daemon/daemon-spawner.ts index a0376ef0fc0b..4c2fa27dd895 100644 --- a/src/main/daemon/daemon-spawner.ts +++ b/src/main/daemon/daemon-spawner.ts @@ -1,4 +1,4 @@ -import { createHash, randomUUID } from 'node:crypto' +import { randomUUID } from 'node:crypto' import { constants, copyFileSync, @@ -8,8 +8,15 @@ import { unlinkSync, writeFileSync } from 'node:fs' -import { join } from 'node:path' -import { PROTOCOL_VERSION } from './types' +// Imported and re-exported rather than moved-and-rewritten: these are the same names every +// caller already imports from here, and they now have a single definition in the leaf module. +import { getDaemonPidPath, getDaemonSocketPath, getDaemonTokenPath } from './daemon-runtime-paths' +export { + getDaemonPidPath, + getDaemonRuntimeDirPath, + getDaemonSocketPath, + getDaemonTokenPath +} from './daemon-runtime-paths' import { DaemonCrashLoopError, DaemonRespawnThrottle } from './daemon-respawn-throttle' export type DaemonConnectionInfo = { @@ -119,28 +126,6 @@ export class DaemonSpawner { } } -export function getDaemonSocketPath( - runtimeDir: string, - protocolVersion = PROTOCOL_VERSION -): string { - // Why: Windows IPC servers use named pipes rather than filesystem socket - // files. Include the protocol version in the endpoint name so a daemon from - // an older build is never reused after a breaking protocol change. - if (process.platform === 'win32') { - const suffix = createHash('sha256').update(runtimeDir).digest('hex').slice(0, 12) - return `\\\\?\\pipe\\orca-terminal-host-v${protocolVersion}-${suffix}` - } - return join(runtimeDir, `daemon-v${protocolVersion}.sock`) -} - -export function getDaemonTokenPath(runtimeDir: string, protocolVersion = PROTOCOL_VERSION): string { - return join(runtimeDir, `daemon-v${protocolVersion}.token`) -} - -export function getDaemonPidPath(runtimeDir: string, protocolVersion = PROTOCOL_VERSION): string { - return join(runtimeDir, `daemon-v${protocolVersion}.pid`) -} - export function serializeDaemonPidFile(pidFile: DaemonPidFile): string { return JSON.stringify(pidFile) } diff --git a/src/main/orcad/orcad-daemon-socket-preflight.test.ts b/src/main/orcad/orcad-daemon-socket-preflight.test.ts new file mode 100644 index 000000000000..8ad262dfc12b --- /dev/null +++ b/src/main/orcad/orcad-daemon-socket-preflight.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest' +import { + assertDaemonSocketPathFits, + OrcadDaemonSocketPathError +} from './orcad-daemon-socket-preflight' +import { ORCAD_EXIT_CONFIGURATION, resolveOrcadExitCode } from './orcad-entry' + +describe('daemon socket path preflight', () => { + it('accepts an ordinary root', () => { + expect(() => assertDaemonSocketPathFits('/home/orca/.orca')).not.toThrow() + }) + + it.skipIf(process.platform === 'win32')('refuses a root that cannot host the socket', () => { + expect(() => assertDaemonSocketPathFits(`/home/${'x'.repeat(120)}/.orca`)).toThrow( + OrcadDaemonSocketPathError + ) + }) + + it('exits 78 so a supervisor stops instead of restart-spinning', () => { + // The whole point of refusing here rather than degrading: the generated unit sets + // RestartPreventExitStatus=78, and no restart will ever shorten the path. + expect(resolveOrcadExitCode(new OrcadDaemonSocketPathError('too long'))).toBe( + ORCAD_EXIT_CONFIGURATION + ) + }) +}) diff --git a/src/main/orcad/orcad-daemon-socket-preflight.ts b/src/main/orcad/orcad-daemon-socket-preflight.ts new file mode 100644 index 000000000000..4b66f9097f2b --- /dev/null +++ b/src/main/orcad/orcad-daemon-socket-preflight.ts @@ -0,0 +1,37 @@ +/** + * Refuses a data root whose daemon socket path cannot fit in `sockaddr_un.sun_path`. + * + * Why this refuses where a failed daemon start does not: `startOrcadDaemon` is deliberately + * fail-open, because the reasons a daemon fails at runtime are unenumerable and often + * transient, and a host that cannot fork one can still serve git and worktrees. This failure + * is none of those things — it is known before anything is attempted, it is permanent until + * an operator changes a value they supplied, and it is arithmetic. That puts it with the bind + * address and the instance lock, which already refuse, rather than with the runtime failures. + * + * The alternative is what the field found: orcad's own listener is a few bytes shorter than + * the daemon's, so a root in that band comes up, reports ready, and serves normally with + * survival off. The warning it prints goes to journald, and a host with volatile journald + * loses it on the reboot that also destroys every terminal. + * + * Its own module, and importing only path arithmetic, so `orcad-entry` can call it without + * pulling the daemon (and node-pty behind it) into the static graph the native preflight + * has to run ahead of. + */ +import { + checkDaemonSocketPathBudget, + DAEMON_SOCKET_PATH_REMEDY, + describeDaemonSocketPathOverflow +} from '../daemon/daemon-runtime-paths' + +export class OrcadDaemonSocketPathError extends Error { + readonly code = 'orcad_daemon_socket_path_too_long' +} + +export function assertDaemonSocketPathFits(userDataPath: string): void { + const budget = checkDaemonSocketPathBudget(userDataPath) + if (!budget.fits) { + throw new OrcadDaemonSocketPathError( + `${describeDaemonSocketPathOverflow(budget)} ${DAEMON_SOCKET_PATH_REMEDY}` + ) + } +} diff --git a/src/main/orcad/orcad-entry.ts b/src/main/orcad/orcad-entry.ts index 3b10ca985e9a..936796417c3a 100644 --- a/src/main/orcad/orcad-entry.ts +++ b/src/main/orcad/orcad-entry.ts @@ -28,6 +28,10 @@ import { OrcadInstanceLockError, type OrcadInstanceLock } from './orcad-instance-lock' +import { + assertDaemonSocketPathFits, + OrcadDaemonSocketPathError +} from './orcad-daemon-socket-preflight' let runOrcadQuitHandlers = (): void => {} @@ -107,6 +111,9 @@ export type OrcadHandle = { export async function startOrcad(options: OrcadOptions = {}): Promise { installOrcadHostAdapters() const userDataPath = resolveUserDataPath() + // Why ahead of the lock: this reads nothing and creates nothing, and refusing a root that + // cannot host a daemon socket should not first take a lock on it. + assertDaemonSocketPathFits(userDataPath) // Why before anything else touches the root: the profile index, the store and the daemon // runtime dir all live under it, and two orcads sharing them corrupt state silently. This // is also the last point at which refusing costs nothing. @@ -318,10 +325,10 @@ export function parseArgs(argv: string[]): OrcadOptions { /** * Exit codes a supervisor can act on. Closed set — see docs/reference/orcad-operations.md. * - * `ORCAD_EXIT_CONFIGURATION` is the load-bearing one: a data root owned by someone else, or - * held by another orcad, is not fixed by restarting. Restarting on it is the crash-loop the - * supervision contract has to prevent, so systemd's `RestartPreventExitStatus` needs a code - * that means "do not retry" and nothing else does. + * `ORCAD_EXIT_CONFIGURATION` is the load-bearing one: a data root owned by someone else, held + * by another orcad, or too long to hold a daemon socket, is not fixed by restarting. + * Restarting on it is the crash-loop the supervision contract has to prevent, so systemd's + * `RestartPreventExitStatus` needs a code that means "do not retry" and nothing else does. */ export const ORCAD_EXIT_OK = 0 export const ORCAD_EXIT_FAILED = 1 @@ -331,7 +338,9 @@ export const ORCAD_EXIT_CONFIGURATION = 78 export const ORCAD_SHUTDOWN_DEADLINE_MS = 15_000 export function resolveOrcadExitCode(error: unknown): number { - return error instanceof OrcadInstanceLockError || error instanceof OrcadBindAddressError + return error instanceof OrcadInstanceLockError || + error instanceof OrcadBindAddressError || + error instanceof OrcadDaemonSocketPathError ? ORCAD_EXIT_CONFIGURATION : ORCAD_EXIT_FAILED } diff --git a/src/main/ssh/ssh-control-socket.ts b/src/main/ssh/ssh-control-socket.ts index 73622ba504e0..9c1d2fcac9ad 100644 --- a/src/main/ssh/ssh-control-socket.ts +++ b/src/main/ssh/ssh-control-socket.ts @@ -4,6 +4,7 @@ import { tmpdir } from 'node:os' import { isAbsolute, join as pathJoin } from 'node:path' import type { SshTarget } from '../../shared/ssh-types' import type { SshResolvedConfig } from './ssh-config-parser' +import { UNIX_SOCKET_PATH_LIMIT, unixSocketPathBytes } from '../../shared/unix-socket-path-budget' export type SystemSshResolvedConfig = Pick< SshResolvedConfig, @@ -23,7 +24,6 @@ export type SystemSshResolvedConfig = Pick< > const OPENSSH_CONTROL_SOCKET_SUFFIX_BUDGET = 18 -const UNIX_SOCKET_PATH_LIMIT = process.platform === 'darwin' ? 104 : 108 const CONTROL_SOCKET_PATH_MAX_LENGTH = UNIX_SOCKET_PATH_LIMIT - OPENSSH_CONTROL_SOCKET_SUFFIX_BUDGET export function getControlSocketPath( @@ -65,7 +65,8 @@ export function getControlSocketPath( }) const hash = createHash('sha256').update(key).digest('hex').slice(0, 16) const socketPath = pathJoin(dir, hash) - return socketPath.length <= CONTROL_SOCKET_PATH_MAX_LENGTH ? socketPath : null + // Bytes, not characters: OpenSSH spends the same sun_path budget the kernel measures. + return unixSocketPathBytes(socketPath) <= CONTROL_SOCKET_PATH_MAX_LENGTH ? socketPath : null } export function removeControlSocketPath(socketPath: string): void { diff --git a/src/shared/unix-socket-path-budget.test.ts b/src/shared/unix-socket-path-budget.test.ts new file mode 100644 index 000000000000..550a21b6912e --- /dev/null +++ b/src/shared/unix-socket-path-budget.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from 'vitest' +import { + measureUnixSocketPathBudget, + UNIX_SOCKET_PATH_LIMIT, + unixSocketPathBytes +} from './unix-socket-path-budget' + +describe('unix socket path bytes', () => { + it('counts bytes, not characters', () => { + // A data root with an accented component spends more budget than its length suggests, + // and a character count would report a path that does not fit as fitting. + expect('/home/josé/.orca'.length).toBe(16) + expect(unixSocketPathBytes('/home/josé/.orca')).toBe(17) + }) + + it('agrees with length for ASCII', () => { + expect(unixSocketPathBytes('/tmp/x')).toBe(6) + }) +}) + +describe('budget measurement', () => { + it('reports the longest candidate, not the first', () => { + const budget = measureUnixSocketPathBudget(['/tmp/short', '/tmp/a-much-longer-name']) + expect(budget.longestPath).toBe('/tmp/a-much-longer-name') + expect(budget.bytes).toBe('/tmp/a-much-longer-name'.length) + }) + + it('fits exactly at the limit and not one byte past it', () => { + const exact = `/${'x'.repeat(UNIX_SOCKET_PATH_LIMIT - 1)}` + expect(unixSocketPathBytes(exact)).toBe(UNIX_SOCKET_PATH_LIMIT) + expect(measureUnixSocketPathBudget([exact]).fits).toBe(true) + expect(measureUnixSocketPathBudget([`${exact}x`]).fits).toBe(false) + }) +}) diff --git a/src/shared/unix-socket-path-budget.ts b/src/shared/unix-socket-path-budget.ts new file mode 100644 index 000000000000..8885064c2895 --- /dev/null +++ b/src/shared/unix-socket-path-budget.ts @@ -0,0 +1,48 @@ +/** + * How many bytes of a unix socket path the kernel will accept. + * + * `sockaddr_un.sun_path` is a fixed-size byte array rather than a pointer, so this is a hard + * kernel limit with nothing to do with PATH_MAX. Bytes and not characters: a data root with + * a non-ASCII component spends more of the budget than its length suggests. + * + * Refusing up front rather than letting `bind` report the problem, because past the limit + * there is no single failure to report. Measured on Linux (node 22 / libuv 1.51), binding a + * path of: + * - 109-110 bytes fails EADDRINUSE against an empty directory, naming a free path; + * - 111+ bytes SUCCEEDS, and connect() to it succeeds, but no directory entry is ever + * created — so it reads as a healthy daemon to everything except code that stats the + * entry, which is what daemon endpoint ownership is built out of. + * Only the arithmetic is the same on every host, so only the arithmetic is worth trusting. + */ +export const UNIX_SOCKET_PATH_LIMIT = process.platform === 'darwin' ? 104 : 108 + +/** TextEncoder rather than Buffer: this module is shared, and Buffer is not universal. */ +export function unixSocketPathBytes(socketPath: string): number { + return new TextEncoder().encode(socketPath).length +} + +export type UnixSocketPathBudget = { + /** The longest of the candidates — the one that decides whether any of them work. */ + longestPath: string + bytes: number + limit: number + fits: boolean +} + +/** + * Measures the candidates a component must bind or connect to, and reports the longest. + * Longest rather than one nominated path: which name is longest is a property of how they + * are built, and a caller that hardcodes the answer stops being right when a name changes. + */ +export function measureUnixSocketPathBudget(candidates: string[]): UnixSocketPathBudget { + const longestPath = candidates.reduce((a, b) => + unixSocketPathBytes(b) > unixSocketPathBytes(a) ? b : a + ) + const bytes = unixSocketPathBytes(longestPath) + return { + longestPath, + bytes, + limit: UNIX_SOCKET_PATH_LIMIT, + fits: bytes <= UNIX_SOCKET_PATH_LIMIT + } +} From e5997b18337ae52f911ed611fd423ed59ee1dadb Mon Sep 17 00:00:00 2001 From: master Date: Tue, 1 Sep 2026 00:45:42 +0200 Subject: [PATCH 2/6] feat(orcad): add --print-service and --doctor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two flags for the supervised deployment orcad exists to serve: - `orcad --print-service` generates a systemd unit (and a launchd plist) for a given data root, bind and port. - `orcad --doctor` audits an installed one, then probes the live host — the unit as written, the daemon socket, the data root — and reports what it actually measured rather than what it expected. **`KillMode=process`, not `mixed`.** Measured on systemd 219: under `mixed` a `systemctl restart` destroyed the daemon, its shell, the session and its scrollback; the identical unit under `process` kept all four. `mixed` SIGTERMs the main process and then SIGKILLs everything still in the control group, and `setsid()` escapes the process group, not the cgroup — so the detached daemon dies with it. The generator emits `process` and the doctor reports `mixed` as critical, including in units orcad itself generated before this was settled. **systemd values are quoted, and the readers parse quotes.** Both halves matter together: a data root containing a space would otherwise round-trip into a unit that silently pins a different root, and an audit that could not parse its own output would call every quoted unit malformed. **An unreadable service definition is reported as unreadable, not absent.** Those are different operator actions — one is a permissions fix, the other an install — and collapsing them sent people to reinstall a unit that was already there. The doctor also answers whether the data root leaves room for the daemon's socket, reusing the budget from the preceding commit. It is critical when a unit file pins the root and a warning — naming whose root it measured — when it falls back to the invoking shell's, because those are different claims. `ORCAD_ENTRY_FILENAME` is named once in `orcad-artifacts` because a generated unit execs it. `OrcadArtifact.optional` goes with it: nothing set it, the filter was a no-op, and the one artifact that would want it cannot be an entry there at all since its filename is derived per platform-arch. Field-tested on a Synology NAS (DSM, systemd 219) and on a root runner; several commits' worth of defects found that way are folded in. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UacRCfD4XPe29bD3ZWNc57 --- src/main/orcad/main.ts | 35 +- src/main/orcad/orcad-service-command.test.ts | 236 ++++++++++++ src/main/orcad/orcad-service-command.ts | 335 +++++++++++++++++ .../supervisor-daemon-socket-budget.test.ts | 58 +++ .../orcad/supervisor-daemon-socket-budget.ts | 55 +++ .../supervisor-generation-warnings.test.ts | 115 ++++++ .../orcad/supervisor-generation-warnings.ts | 161 ++++++++ .../orcad/supervisor-host-observations.ts | 102 +++++ .../orcad/supervisor-service-discovery.ts | 111 ++++++ src/shared/orcad-artifacts.ts | 25 +- src/shared/supervisor-evidence-audit.test.ts | 269 ++++++++++++++ src/shared/supervisor-evidence-audit.ts | 233 ++++++++++++ src/shared/supervisor-service-audit.test.ts | 341 +++++++++++++++++ src/shared/supervisor-service-audit.ts | 351 ++++++++++++++++++ .../supervisor-service-file-read.test.ts | 96 +++++ src/shared/supervisor-service-file-read.ts | 166 +++++++++ src/shared/supervisor-service-probe.ts | 238 ++++++++++++ src/shared/supervisor-service-render.test.ts | 210 +++++++++++ src/shared/supervisor-service-render.ts | 301 +++++++++++++++ 19 files changed, 3415 insertions(+), 23 deletions(-) create mode 100644 src/main/orcad/orcad-service-command.test.ts create mode 100644 src/main/orcad/orcad-service-command.ts create mode 100644 src/main/orcad/supervisor-daemon-socket-budget.test.ts create mode 100644 src/main/orcad/supervisor-daemon-socket-budget.ts create mode 100644 src/main/orcad/supervisor-generation-warnings.test.ts create mode 100644 src/main/orcad/supervisor-generation-warnings.ts create mode 100644 src/main/orcad/supervisor-host-observations.ts create mode 100644 src/main/orcad/supervisor-service-discovery.ts create mode 100644 src/shared/supervisor-evidence-audit.test.ts create mode 100644 src/shared/supervisor-evidence-audit.ts create mode 100644 src/shared/supervisor-service-audit.test.ts create mode 100644 src/shared/supervisor-service-audit.ts create mode 100644 src/shared/supervisor-service-file-read.test.ts create mode 100644 src/shared/supervisor-service-file-read.ts create mode 100644 src/shared/supervisor-service-probe.ts create mode 100644 src/shared/supervisor-service-render.test.ts create mode 100644 src/shared/supervisor-service-render.ts diff --git a/src/main/orcad/main.ts b/src/main/orcad/main.ts index 73a973c7b242..59b83c00a716 100644 --- a/src/main/orcad/main.ts +++ b/src/main/orcad/main.ts @@ -2,6 +2,7 @@ import process from 'node:process' import { main, resolveOrcadExitCode } from './orcad-entry' import { runOrcadNativePreflight } from './orcad-native-preflight' +import { isServiceCommand, runServiceCommand } from './orcad-service-command' // Why exit before the preflight: reaching this line means the whole module graph resolved // under plain Node, which is all the build guard needs to prove. Probing natives or @@ -11,17 +12,25 @@ if (process.argv.includes('--orcad-smoke-load-check')) { process.exit(0) } -// Why here and not inside startOrcad: this must run before anything requires node-pty, -// and `orcad-entry` reaches it through `await import('../ipc/pty')`. Static imports are -// evaluated before this statement, so the guarantee is that no module in the graph -// requires node-pty at import time — which the bundle's lazy `require("node-pty")` in -// local-pty-provider satisfies. See ./node-pty-precondition.ts for why a child process. -runOrcadNativePreflight() +// Why an exclusive branch and not an early return: the doctor's live probes make the +// service path async, and awaiting in place would let it fall through into the native +// preflight — which is exactly what an operator generating a service definition cannot +// pass yet. `parseArgs` would also reject these flags outright. +if (isServiceCommand(process.argv.slice(2))) { + runServiceCommand(process.argv.slice(2)).then((code) => process.exit(code)) +} else { + // Why here and not inside startOrcad: this must run before anything requires node-pty, + // and `orcad-entry` reaches it through `await import('../ipc/pty')`. Static imports are + // evaluated before this statement, so the guarantee is that no module in the graph + // requires node-pty at import time — which the bundle's lazy `require("node-pty")` in + // local-pty-provider satisfies. See ./node-pty-precondition.ts for why a child process. + runOrcadNativePreflight() -main().catch((error: unknown) => { - console.error('orcad: failed to start:', error) - // Why a resolved code and not a bare 1: a data-root or bind-address refusal is a - // configuration fault that restarting cannot fix, and a supervisor needs to tell the two - // apart to avoid restart-spinning on it. - process.exit(resolveOrcadExitCode(error)) -}) + main().catch((error: unknown) => { + console.error('orcad: failed to start:', error) + // Why a resolved code and not a bare 1: a data-root or bind-address refusal is a + // configuration fault that restarting cannot fix, and a supervisor needs to tell the two + // apart to avoid restart-spinning on it. + process.exit(resolveOrcadExitCode(error)) + }) +} diff --git a/src/main/orcad/orcad-service-command.test.ts b/src/main/orcad/orcad-service-command.test.ts new file mode 100644 index 000000000000..7a5e45f5ddcf --- /dev/null +++ b/src/main/orcad/orcad-service-command.test.ts @@ -0,0 +1,236 @@ +import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import process from 'node:process' +import { afterAll, describe, expect, it, vi } from 'vitest' +import { + collectServiceFiles, + formatFindings, + inferScopeFromPath, + isServiceCommand, + parseServiceCommandArgs, + printService +} from './orcad-service-command' + +describe('flag routing', () => { + // B1: these flags are handled before the native preflight, so a normal start must not be + // diverted by them — and orcad's own parseArgs would reject them outright if they leaked. + it('leaves a normal server start alone', () => { + expect(isServiceCommand(['--port', '6800', '--json'])).toBe(false) + expect(isServiceCommand([])).toBe(false) + }) + + it('claims the invocation for either service flag', () => { + expect(isServiceCommand(['--print-service'])).toBe(true) + expect(isServiceCommand(['--doctor'])).toBe(true) + }) +}) + +describe('argument parsing', () => { + it('defaults to a system-scope loopback service', () => { + const options = parseServiceCommandArgs(['--print-service']) + expect(options).toMatchObject({ scope: 'system', bind: '127.0.0.1', port: 6800 }) + expect(options.user).toBeUndefined() + }) + + it('reads every override', () => { + const options = parseServiceCommandArgs([ + '--print-service', + '--scope', + 'user', + '--user', + 'orca', + '--node', + '/usr/bin/node', + '--port', + '7000', + '--bind', + '0.0.0.0', + '--orcad', + '/opt/orcad/orcad.js', + '--service-path', + '/tmp/orcad.service', + '--no-probe' + ]) + expect(options).toEqual({ + scope: 'user', + user: 'orca', + nodePath: '/usr/bin/node', + port: 7000, + bind: '0.0.0.0', + servicePath: '/tmp/orcad.service', + orcadPath: '/opt/orcad/orcad.js', + noProbe: true + }) + }) + + it('rejects values it cannot act on rather than guessing', () => { + expect(() => parseServiceCommandArgs(['--scope', 'global'])).toThrow(/--scope/) + expect(() => parseServiceCommandArgs(['--port', 'ssh'])).toThrow(/--port/) + expect(() => parseServiceCommandArgs(['--user'])).toThrow(/--user/) + expect(() => parseServiceCommandArgs(['--node'])).toThrow(/--node/) + expect(() => parseServiceCommandArgs(['--orcad'])).toThrow(/--orcad/) + expect(() => parseServiceCommandArgs(['--service-path'])).toThrow(/--service-path/) + }) + + // Silently dropping these is how `--user-data` came to be recommended by the tool's own + // socket-budget remedy: an operator ran it, got exit 0 and an [OK], and had changed + // nothing. `--json` is the same shape — real on `orca supervisor doctor`, absent here. + it('rejects a flag it does not implement instead of ignoring it', () => { + expect(() => parseServiceCommandArgs(['--doctor', '--user-data', '/tmp/x'])).toThrow( + /Unknown argument: --user-data/ + ) + expect(() => parseServiceCommandArgs(['--doctor', '--json'])).toThrow( + /Unknown argument: --json/ + ) + }) + + // The mode flags travel in the same argv this parses, so they are not "unknown". + it('accepts the mode flags it is invoked through', () => { + expect(() => parseServiceCommandArgs(['--print-service'])).not.toThrow() + expect(() => parseServiceCommandArgs(['--doctor', '--no-probe'])).not.toThrow() + }) + + // A value consumed by its flag must not then be re-read as an argument. + it('does not mistake a flag value for an unknown flag', () => { + expect(() => + parseServiceCommandArgs(['--doctor', '--service-path', '/tmp/--json.service']) + ).not.toThrow() + }) +}) + +describe('which orcad the unit will exec', () => { + // argv[1] is orcad's own entry only when orcad is the process running this. Reached through + // `orca supervisor print` it is the CLI's entry, and the unit came out pinning + // `ExecStart= .../cli/index.js --bind ... --json` — the orca CLI handed orcad's flags, + // which exits before it serves anything. Silent at generation time, and only visible later + // as a unit that will not start. + function generating(argv1: string, args: string[]): Promise { + const originalArgv1 = process.argv[1] + const chunks: string[] = [] + const stdout = vi.spyOn(process.stdout, 'write').mockImplementation((chunk) => { + chunks.push(String(chunk)) + return true + }) + const stderr = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) + process.argv[1] = argv1 + return Promise.resolve(printService(args)) + .then(() => chunks.join('')) + .finally(() => { + process.argv[1] = originalArgv1 + stdout.mockRestore() + stderr.mockRestore() + }) + } + + it.runIf(process.platform !== 'win32')( + 'refuses rather than pinning whatever entry point happens to be running', + async () => { + await expect(generating('/opt/orca/out/cli/index.js', ['--print-service'])).rejects.toThrow( + /--orcad/ + ) + } + ) + + it.runIf(process.platform !== 'win32')('takes the explicit path when given one', async () => { + const unit = await generating('/opt/orca/out/cli/index.js', [ + '--print-service', + '--orcad', + '/opt/orcad/orcad.js', + '--user', + 'orca' + ]) + expect(/^ExecStart=.*$/m.exec(unit)?.[0]).toContain('/opt/orcad/orcad.js') + expect(unit).not.toContain('cli/index.js') + }) + + it.runIf(process.platform !== 'win32')( + 'still trusts argv[1] when orcad is the process', + async () => { + const unit = await generating('/opt/orcad/orcad.js', ['--print-service', '--user', 'orca']) + expect(/^ExecStart=.*$/m.exec(unit)?.[0]).toContain('/opt/orcad/orcad.js') + } + ) +}) + +describe('scope inference', () => { + // Mislabelling a user-scope file as system makes the audit report its correct missing + // run-as account as critical. + it('reads user scope out of the conventional paths', () => { + expect(inferScopeFromPath('/home/orca/.config/systemd/user/orcad.service')).toBe('user') + expect(inferScopeFromPath('/Users/orca/Library/LaunchAgents/dev.onorca.orcad.plist')).toBe( + 'user' + ) + }) + + it('treats everything else as system scope', () => { + expect(inferScopeFromPath('/etc/systemd/system/orcad.service')).toBe('system') + expect(inferScopeFromPath('/Library/LaunchDaemons/dev.onorca.orcad.plist')).toBe('system') + expect(inferScopeFromPath('/tmp/scratch.service')).toBe('system') + }) +}) + +describe('finding output', () => { + it('indents a remedy under its finding', () => { + const text = formatFindings([ + { + code: 'kill_mode_missing', + severity: 'critical', + message: 'No KillMode.', + remedy: 'Regenerate.' + }, + { code: 'user_data_agrees', severity: 'ok', message: 'Root pinned.' } + ]) + expect(text).toBe('[CRITICAL] No KillMode.\n Regenerate.\n[OK] Root pinned.') + }) +}) + +/** + * Presence and readability are different answers, and `existsSync` only reports the first: + * it succeeds on a file the caller cannot open, because a traversable parent is enough. + * + * These run only as non-root by necessity, not by preference — uid 0 ignores the mode bits, + * so the unreadable file simply reads fine and the case cannot exist. Exercised as uid 1026 + * on the Synology box where the original was found. + */ +describe('discovery separates unreadable from absent', () => { + const dir = mkdtempSync(join(tmpdir(), 'orcad-discovery-')) + afterAll(() => { + try { + chmodSync(join(dir, 'orcad.service'), 0o600) + } catch { + // Already gone, or never created because the test was skipped. + } + rmSync(dir, { recursive: true, force: true }) + }) + + it.runIf(process.getuid?.() !== 0)('records a present-but-unreadable candidate', () => { + const path = join(dir, 'orcad.service') + writeFileSync(path, '[Service]\nExecStart=/bin/true\n', 'utf8') + chmodSync(path, 0o000) + + const { files, unreadable } = collectServiceFiles('systemd', [path]) + + expect(files.map((f) => f.path)).not.toContain(path) + expect(unreadable.map((u) => u.path)).toContain(path) + expect(unreadable.find((u) => u.path === path)?.reason).toBe('EACCES') + }) + + // Scoped to the path under test rather than to the whole result: discovery also scans the + // conventional locations, so a host with a real orcad.service installed would otherwise + // fail these on its own installation. + it('reports a path that truly is not there as neither found nor unreadable', () => { + const path = join(dir, 'absent.service') + const { files, unreadable } = collectServiceFiles('systemd', [path]) + expect(files.map((f) => f.path)).not.toContain(path) + expect(unreadable.map((u) => u.path)).not.toContain(path) + }) + + it('reads a candidate it can open', () => { + const path = join(dir, 'readable.service') + writeFileSync(path, '[Service]\nKillMode=process\n', 'utf8') + const { files, unreadable } = collectServiceFiles('systemd', [path]) + expect(files.map((f) => f.path)).toContain(path) + expect(unreadable.map((u) => u.path)).not.toContain(path) + }) +}) diff --git a/src/main/orcad/orcad-service-command.ts b/src/main/orcad/orcad-service-command.ts new file mode 100644 index 000000000000..c173733f4a09 --- /dev/null +++ b/src/main/orcad/orcad-service-command.ts @@ -0,0 +1,335 @@ +/** + * `orcad --print-service` and `orcad --doctor`. + * + * Why these live in orcad rather than only the `orca` CLI: the shipped orcad artifact is + * three files and no CLI (`ORCAD_ARTIFACTS`), so a host that installed only the orcad + * tarball — the deployment this exists for — has no `orca` binary to run. The CLI can + * mirror these over the same pure functions where it happens to be installed. + * + * Both are handled before `runOrcadNativePreflight()` in `main.ts`, for the reason already + * written there: proving something must not bind a port or take the data-root lock. Data + * root resolution is pure environment reads, so the pinned value is available that early. + */ +import { userInfo } from 'node:os' +import { basename, resolve } from 'node:path' +import process from 'node:process' +import { + auditSupervisorServices, + readConfiguredEndpoint, + sortBySeverity, + supervisorAuditPassed, + type SupervisorFinding, + type SupervisorServiceFile +} from '../../shared/supervisor-service-audit' +import { gatherSupervisorEvidence } from '../../shared/supervisor-service-probe' +import { + observeDataRootIdentity, + observeExecTarget, + observeJournal +} from './supervisor-host-observations' +import { auditDaemonSocketPathBudget } from './supervisor-daemon-socket-budget' +import type { ProbeTarget } from '../../shared/supervisor-service-probe' +import { + renderSupervisorService, + resolveSupervisorPlatform, + supervisorInstallHint, + SupervisorServiceUnsupportedError, + type SupervisorScope +} from '../../shared/supervisor-service-render' +import { ORCAD_ENTRY_FILENAME } from '../../shared/orcad-artifacts' +import { collectServiceFiles } from './supervisor-service-discovery' +import { resolveUserDataPath } from './orcad-app-paths' +import { + interpreterOnDiskWarning, + resolveRealPath, + socketPathBudgetWarning, + userScopeUnavailableWarning, + versionScopedInterpreterWarning +} from './supervisor-generation-warnings' + +export { + collectServiceFiles, + inferScopeFromPath, + type ServiceFileDiscovery, + type UnreadableServiceFile +} from './supervisor-service-discovery' + +export const PRINT_SERVICE_FLAG = '--print-service' +export const DOCTOR_FLAG = '--doctor' + +const DEFAULT_PORT = 6800 + +type ServiceCommandOptions = { + scope: SupervisorScope + nodePath?: string + user?: string + port: number + bind: string + /** Audit a definition outside the conventional locations. */ + servicePath?: string + /** Skip live probes: the file-only audit, for a host where shelling out is unwanted. */ + noProbe?: boolean + /** Absolute path to the `orcad.js` the unit should exec; required when this is not orcad. */ + orcadPath?: string +} + +/** + * Deliberately separate from `parseArgs`: that one throws on any unknown argument and is + * reached only from `main()`, which is past the point where a port gets bound. + * + * It rejects unknown arguments for the same reason `parseArgs` does. Being separate was + * never meant to make it permissive, but it silently dropped anything it did not + * recognise, and two flags an operator would plausibly reach for went nowhere: + * `--user-data`, which this tool's own socket-budget remedy used to recommend, and + * `--json`, which exists on `orca supervisor doctor` but not here — the tarball + * deployment this command exists for has no `orca` CLI, so on those hosts the JSON form + * is unreachable and the flag asking for it was swallowed. Erroring is what makes that + * discoverable. + */ +export function parseServiceCommandArgs(argv: string[]): ServiceCommandOptions { + const options: ServiceCommandOptions = { scope: 'system', port: DEFAULT_PORT, bind: '127.0.0.1' } + for (let i = 0; i < argv.length; i += 1) { + const value = argv[i + 1] + if (argv[i] === '--scope') { + if (value !== 'user' && value !== 'system') { + throw new Error(`--scope expects 'user' or 'system', got ${value ?? "''"}`) + } + options.scope = value + i += 1 + } else if (argv[i] === '--node') { + if (!value) { + throw new Error('--node expects a path') + } + options.nodePath = value + i += 1 + } else if (argv[i] === '--user') { + if (!value) { + throw new Error('--user expects an account name') + } + options.user = value + i += 1 + } else if (argv[i] === '--orcad') { + if (!value) { + throw new Error('--orcad expects a path') + } + options.orcadPath = value + i += 1 + } else if (argv[i] === '--service-path') { + if (!value) { + throw new Error('--service-path expects a path') + } + options.servicePath = value + i += 1 + } else if (argv[i] === '--no-probe') { + options.noProbe = true + } else if (argv[i] === '--port') { + const port = Number(value) + if (!Number.isInteger(port) || port < 0 || port > 65535) { + throw new Error(`--port expects an integer 0-65535, got ${value ?? "''"}`) + } + options.port = port + i += 1 + } else if (argv[i] === '--bind') { + if (!value) { + throw new Error('--bind expects a value') + } + options.bind = value + i += 1 + } else if (argv[i] !== PRINT_SERVICE_FLAG && argv[i] !== DOCTOR_FLAG) { + // The mode flags are part of the argv this is handed, so they are not "unknown". + throw new Error(`Unknown argument: ${argv[i]}`) + } + } + return options +} + +/** + * The `orcad.js` the generated unit will exec. + * + * Why argv[1] and not cwd: the same reason `resolveOrcadInstallRoot` uses it — cwd is + * wherever the operator happened to be, so a sibling resolved against it is found by luck. + * + * Why the basename is then checked rather than trusted: argv[1] is orcad's own entry only + * when orcad is the process running this. Reached through `orca supervisor print` it is the + * CLI's entry instead, and the unit came out pinning `ExecStart= .../cli/index.js + * --bind ... --port ... --json` — the orca CLI, handed orcad's flags, which exits before it + * serves anything. The failure is silent at generation time and only surfaces as a unit that + * will not start, so this refuses instead and names the flag that answers it. + */ +function resolveOrcadEntryPath(explicit?: string): string { + if (explicit) { + return resolve(explicit) + } + const entry = process.argv[1] + if (!entry) { + throw new Error('cannot resolve this orcad bundle: process.argv[1] is unset') + } + const resolved = resolve(entry) + if (basename(resolved) !== ORCAD_ENTRY_FILENAME) { + throw new Error( + `this process is ${resolved}, not ${ORCAD_ENTRY_FILENAME}, so it cannot say which ` + + 'orcad the service should run. Pass --orcad , or run ' + + '`orcad --print-service` from the orcad bundle itself.' + ) + } + return resolved +} + +export async function printService(argv: string[]): Promise { + const options = parseServiceCommandArgs(argv) + const platform = resolveSupervisorPlatform(process.platform) + const nodePath = options.nodePath ?? process.execPath + const config = { + platform, + scope: options.scope, + nodePath, + orcadPath: resolveOrcadEntryPath(options.orcadPath), + // Resolved through symlinks so `RequiresMountsFor` can reach a real mount unit: + // systemd maps that directive textually and never sees through a symlinked ancestor. + userDataPath: resolveRealPath(resolveUserDataPath()), + // Generating as root is normal (sudo, a container); running orcad as root is not, so + // the account is a flag rather than an inheritance. + user: options.user ?? userInfo().username, + bind: options.bind, + port: options.port + } + const hint = supervisorInstallHint(config) + process.stdout.write(renderSupervisorService(config)) + + // Why before the install block and unindented: the hint is a copy-paste unit, and a + // warning folded into it reads as part of the instructions rather than a caveat on them. + const warnings = [ + // First because it is the only one of these that means the service cannot work at all. + socketPathBudgetWarning(config.userDataPath), + versionScopedInterpreterWarning(nodePath), + interpreterOnDiskWarning(nodePath, options.nodePath !== undefined), + ...(platform === 'systemd' && options.scope === 'user' + ? [await userScopeUnavailableWarning()] + : []) + ].filter((warning): warning is string => warning !== null) + if (warnings.length > 0) { + process.stderr.write(`\n${warnings.join('\n\n')}\n`) + } + + // Why stderr: stdout is the file, so it stays pipeable straight into the target path. + process.stderr.write( + `\nWrite this to: ${hint.path}\nThen run:\n${hint.commands.map((c) => ` ${c}`).join('\n')}\n` + ) + return 0 +} + +const SEVERITY_LABEL = { + critical: 'CRITICAL', + warning: 'WARN', + unverifiable: 'UNVERIFIED', + ok: 'OK' +} as const + +export function formatFindings(findings: SupervisorFinding[]): string { + return findings + .map((finding) => { + const head = `[${SEVERITY_LABEL[finding.severity]}] ${finding.message}` + return finding.remedy ? `${head}\n ${finding.remedy}` : head + }) + .join('\n') +} + +/** + * Why the discovered file's own basename and not a constant: discovery searches the + * conventional locations AND accepts an explicit path, precisely because an operator may + * have renamed things. Probing a constant would query a unit that does not exist and + * report the file we did find as not running. + */ +function probeTargetFor(file: SupervisorServiceFile, options: ServiceCommandOptions): ProbeTarget { + const name = basename(file.path, file.platform === 'launchd' ? '.plist' : '') + // The endpoint comes from the file for the same reason the unit name does: the flags + // describe what to generate, the file describes what is actually installed. + const endpoint = readConfiguredEndpoint(file) + return { + platform: file.platform, + scope: file.scope, + name, + user: options.user ?? userInfo().username, + bind: endpoint?.bind ?? options.bind, + port: endpoint?.port ?? options.port + } +} + +/** Shared by the text and JSON paths so neither can drift from the other's verdict. */ +export async function collectDoctorFindings( + argv: string[] +): Promise<{ findings: SupervisorFinding[]; code: number }> { + const platform = resolveSupervisorPlatform(process.platform) + const options = parseServiceCommandArgs(argv) + const { files, unreadable } = collectServiceFiles( + platform, + options.servicePath ? [options.servicePath] : [] + ) + // Only probe one definition: with two, every result would be ambiguous about which it + // described, and the duplicate finding is the story anyway. + const probing = files.length === 1 && !options.noProbe + // Why these two run even under --no-probe: they are a stat and a config read, not a + // subprocess, and they stay useful on a host where every live probe is refused. + const expectedUserDataPath = resolveUserDataPath() + const onDisk = + files.length === 1 + ? { + execTarget: observeExecTarget(files[0]), + journal: observeJournal(files[0]), + dataRootSameDirectory: observeDataRootIdentity(files[0], expectedUserDataPath) + } + : {} + const evidence = probing + ? { ...(await gatherSupervisorEvidence(probeTargetFor(files[0], options))), ...onDisk } + : files.length === 1 + ? onDisk + : undefined + const findings = auditSupervisorServices({ + files, + expectedUserDataPath, + evidence, + unreadable + }) + // Why unconditionally, and never gated by --no-probe: it is arithmetic on a path. Its whole + // value is answering before an operator commits to a host, which is when nothing is running + // to probe. `files[0]` only when it is the only one, matching the rule above. + findings.push( + auditDaemonSocketPathBudget(files.length === 1 ? files[0] : undefined, expectedUserDataPath) + ) + // Why say so: a silently file-only report looks identical to one where every probe + // happened to come back clean. + if (!probing && files.length > 0) { + findings.push({ + code: 'live_state_not_probed', + severity: 'unverifiable', + message: options.noProbe + ? 'Live state was not checked (--no-probe).' + : `Live state was not checked: ${files.length} service definitions found, so any result would be ambiguous about which one it described.` + }) + } + // Why not non-zero on unverifiable: a check that could not run is not a failed check, + // and a doctor that exits 1 on "could not verify" trains operators to ignore it. + return { findings: sortBySeverity(findings), code: supervisorAuditPassed(findings) ? 0 : 1 } +} + +export async function runDoctor(argv: string[]): Promise { + const { findings, code } = await collectDoctorFindings(argv) + process.stdout.write(`${formatFindings(findings)}\n`) + return code +} + +/** True when this invocation is a service command rather than a server start. */ +export function isServiceCommand(argv: string[]): boolean { + return argv.includes(PRINT_SERVICE_FLAG) || argv.includes(DOCTOR_FLAG) +} + +export async function runServiceCommand(argv: string[]): Promise { + try { + return argv.includes(PRINT_SERVICE_FLAG) ? await printService(argv) : await runDoctor(argv) + } catch (error) { + const unsupported = error instanceof SupervisorServiceUnsupportedError + process.stderr.write(`orcad: ${error instanceof Error ? error.message : String(error)}\n`) + // 78 is EX_CONFIG: an unsupported platform or a bad flag is not fixed by retrying. + return unsupported ? 78 : 1 + } +} diff --git a/src/main/orcad/supervisor-daemon-socket-budget.test.ts b/src/main/orcad/supervisor-daemon-socket-budget.test.ts new file mode 100644 index 000000000000..4dadceef6ae8 --- /dev/null +++ b/src/main/orcad/supervisor-daemon-socket-budget.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'vitest' +import { auditDaemonSocketPathBudget } from './supervisor-daemon-socket-budget' +import { renderSupervisorService } from '../../shared/supervisor-service-render' +import type { SupervisorServiceFile } from '../../shared/supervisor-service-file-read' + +const LONG_ROOT = `/home/${'x'.repeat(120)}/.orca` + +function unitPinning(userDataPath: string): SupervisorServiceFile { + return { + path: '/etc/systemd/system/orcad.service', + platform: 'systemd', + scope: 'system', + text: renderSupervisorService({ + platform: 'systemd', + scope: 'system', + nodePath: '/usr/local/bin/node', + orcadPath: '/opt/orcad/orcad.js', + userDataPath, + user: 'orca', + bind: '127.0.0.1', + port: 6800 + }) + } +} + +describe('daemon socket path budget finding', () => { + it('passes an ordinary root', () => { + const finding = auditDaemonSocketPathBudget(unitPinning('/home/orca/.orca'), '/home/orca/.orca') + expect(finding.severity).toBe('ok') + expect(finding.code).toBe('daemon_socket_path_fits') + }) + + it('is critical when the definition itself pins a root that cannot host the socket', () => { + const finding = auditDaemonSocketPathBudget(unitPinning(LONG_ROOT), '/home/orca/.orca') + expect(finding.severity).toBe('critical') + expect(finding.message).toMatch(/not survive a restart/) + expect(finding.remedy).toMatch(/ORCA_USER_DATA/) + }) + + it('measures what the file pins, not what this shell resolves', () => { + // The doctor usually runs from a different account than the service. Reading the caller's + // root would answer for the wrong host and pass a service that cannot work. + expect(auditDaemonSocketPathBudget(unitPinning(LONG_ROOT), '/home/orca/.orca').severity).toBe( + 'critical' + ) + expect(auditDaemonSocketPathBudget(unitPinning('/home/orca/.orca'), LONG_ROOT).severity).toBe( + 'ok' + ) + }) + + it('warns, and says whose root it measured, when no definition pins one', () => { + const finding = auditDaemonSocketPathBudget(undefined, LONG_ROOT) + // Not critical: `critical` claims a configuration will destroy terminals, and only a file + // that pins a root can say that about the installed service. + expect(finding.severity).toBe('warning') + expect(finding.message).toContain('this shell resolves') + }) +}) diff --git a/src/main/orcad/supervisor-daemon-socket-budget.ts b/src/main/orcad/supervisor-daemon-socket-budget.ts new file mode 100644 index 000000000000..b802dd497db9 --- /dev/null +++ b/src/main/orcad/supervisor-daemon-socket-budget.ts @@ -0,0 +1,55 @@ +/** + * The doctor's read on whether the daemon's socket path fits under the configured data root. + * + * A finding rather than a `Probe`: there is no uncertainty to represent. Nothing is read, so + * there is no "unavailable" case, nothing for live evidence to supersede, and no reading that + * could be stale. It is the platform's `sun_path` limit against a path the unit file names. + * + * Critical only when the definition supplies the root, which keeps the severity boundary + * intact: `critical` means "this configuration will destroy running terminals", and only a + * file that pins a root can say that about the service. Falling back to the calling shell's + * root answers a different question — what would happen if you started orcad here — and is + * reported as a warning that says so. + */ +import { + checkDaemonSocketPathBudget, + DAEMON_SOCKET_PATH_REMEDY, + describeDaemonSocketPathOverflow +} from '../daemon/daemon-runtime-paths' +import type { SupervisorFinding } from '../../shared/supervisor-service-audit' +import { + readPinnedUserData, + type SupervisorServiceFile +} from '../../shared/supervisor-service-file-read' + +export function auditDaemonSocketPathBudget( + file: SupervisorServiceFile | undefined, + expectedUserDataPath: string +): SupervisorFinding { + const pinned = file ? readPinnedUserData(file) : null + const userDataPath = pinned ?? expectedUserDataPath + const budget = checkDaemonSocketPathBudget(userDataPath) + if (budget.fits) { + return { + code: 'daemon_socket_path_fits', + severity: 'ok', + message: `The terminal daemon's socket path fits (${budget.bytes} of ${budget.limit} bytes).` + } + } + const overflow = describeDaemonSocketPathOverflow(budget) + return pinned === null + ? { + code: 'daemon_socket_path_too_long_here', + severity: 'warning', + // Says whose root it measured: reported against an installed service, this reads as a + // verdict about that service unless it states otherwise. + message: `${overflow} This is the root this shell resolves (${userDataPath}), not one a service definition pins.`, + remedy: DAEMON_SOCKET_PATH_REMEDY + } + : { + code: 'daemon_socket_path_too_long', + severity: 'critical', + message: overflow, + remedy: DAEMON_SOCKET_PATH_REMEDY + } +} diff --git a/src/main/orcad/supervisor-generation-warnings.test.ts b/src/main/orcad/supervisor-generation-warnings.test.ts new file mode 100644 index 000000000000..04c953d8440a --- /dev/null +++ b/src/main/orcad/supervisor-generation-warnings.test.ts @@ -0,0 +1,115 @@ +import { mkdtempSync, mkdirSync, symlinkSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterAll, describe, expect, it } from 'vitest' +import process from 'node:process' +import { + interpreterOnDiskWarning, + resolveRealPath, + socketPathBudgetWarning, + versionScopedInterpreterWarning +} from './supervisor-generation-warnings' + +describe('version-scoped interpreter', () => { + // One `brew upgrade node` removes the Cellar directory the unit names, and it dies + // 203/EXEC long after the change that caused it. + it.each([ + '/home/linuxbrew/.linuxbrew/Cellar/node/25.8.0/bin/node', + '/opt/homebrew/Cellar/node/22.1.0/bin/node', + '/home/me/.nvm/versions/node/v22.11.0/bin/node', + '/home/me/.local/share/mise/installs/node/22.11.0/bin/node', + '/home/me/.asdf/installs/nodejs/22.11.0/bin/node' + ])('warns about %s', (nodePath) => { + const warning = versionScopedInterpreterWarning(nodePath) + expect(warning).not.toBeNull() + // Points at the flag that fixes it rather than just naming the problem. + expect(warning).toContain('--node') + }) + + it.each(['/usr/bin/node', '/usr/local/bin/node', '/home/linuxbrew/.linuxbrew/bin/node'])( + 'stays quiet about the stable path %s', + (nodePath) => { + expect(versionScopedInterpreterWarning(nodePath)).toBeNull() + } + ) +}) + +describe('realpath resolution', () => { + const root = mkdtempSync(join(tmpdir(), 'orca-realpath-')) + afterAll(() => rmSync(root, { recursive: true, force: true })) + + it('resolves a symlinked ancestor, which is what RequiresMountsFor needs', () => { + // The DSM shape: /var/services/homes -> /volume2/homes, created during boot. + const volume = join(root, 'volume2', 'homes') + mkdirSync(join(volume, 'master'), { recursive: true }) + const link = join(root, 'services-homes') + symlinkSync(volume, link) + expect(resolveRealPath(join(link, 'master'))).toBe(join(volume, 'master')) + }) + + it('resolves the existing ancestor of a data root that does not exist yet', () => { + // The usual case at generation time: nothing has created the root, and throwing + // there would be worse than the symlink this is fixing. + const volume = join(root, 'volume3') + mkdirSync(volume, { recursive: true }) + const link = join(root, 'volume3-link') + symlinkSync(volume, link) + expect(resolveRealPath(join(link, 'master', '.orca'))).toBe(join(volume, 'master', '.orca')) + }) + + it('returns an entirely absent path unchanged rather than throwing', () => { + const absent = join(root, 'no', 'such', 'path') + expect(resolveRealPath(absent)).toBe(absent) + }) +}) + +describe('--node validation', () => { + // The version-scoped warning tells operators to pass --node, and --node was the one + // interpreter source with no validation. The default cannot have this problem, so the + // gap bit only the people who did what the warning told them to. + it('warns when a chosen interpreter is not on this host', () => { + const warning = interpreterOnDiskWarning('/nonexistent/node', true) + expect(warning).toMatch(/does not exist/) + expect(warning).toMatch(/203\/EXEC/) + }) + + it('warns when it exists but cannot be executed', () => { + const notExecutable = join(mkdtempSync(join(tmpdir(), 'orca-node-')), 'node') + writeFileSync(notExecutable, '', { mode: 0o644 }) + expect(interpreterOnDiskWarning(notExecutable, true)).toMatch(/not executable/) + }) + + it('stays quiet for an interpreter that is present and executable', () => { + expect(interpreterOnDiskWarning(process.execPath, true)).toBeNull() + }) + + // process.execPath is tautologically on disk, so checking it would only add noise. + it('says nothing about the default interpreter', () => { + expect(interpreterOnDiskWarning('/nonexistent/node', false)).toBeNull() + }) +}) + +describe('socket path budget at generation time', () => { + // The doctor already calls this critical, but only once the unit is installed. Found on + // a Synology NAS: print-service emitted a unit pinning a 90-byte root — 113 of 108 + // socket bytes — and then printed the install hint, with nothing on stderr. + it('warns when the pinned root cannot hold a daemon socket', () => { + const overBudget = `/volume2/homes/master/.orca-sudotest/${'a'.repeat(53)}` + const warning = socketPathBudgetWarning(overBudget) + expect(warning).not.toBeNull() + expect(warning).toMatch(/\b108\b/) + expect(warning).toContain(overBudget) + }) + + it('stays quiet for a root that fits', () => { + expect(socketPathBudgetWarning('/volume2/homes/master/.orca')).toBeNull() + }) + + // The remedy is copied into the operator's next action, so it may name only mechanisms + // that exist. `--user-data` was neither accepted nor rejected by orcad. + it('recommends no flag that orcad does not accept', () => { + const warning = socketPathBudgetWarning(`/tmp/${'a'.repeat(120)}`) + expect(warning).toContain('ORCA_USER_DATA') + expect(warning).not.toContain('--user-data') + }) +}) diff --git a/src/main/orcad/supervisor-generation-warnings.ts b/src/main/orcad/supervisor-generation-warnings.ts new file mode 100644 index 000000000000..23230493c6b1 --- /dev/null +++ b/src/main/orcad/supervisor-generation-warnings.ts @@ -0,0 +1,161 @@ +/** + * What `orcad --print-service` can see about this host that the generated file cannot say + * for itself. + * + * All warnings rather than refusals: a definition is often generated on one host to be + * installed on another, so an unusable local scope is not proof the file is wrong. + */ +import { accessSync, constants, existsSync, realpathSync } from 'node:fs' +import { dirname, join } from 'node:path' +import process from 'node:process' +import { runProcess } from '../../shared/child-process/run-process' +import { + checkDaemonSocketPathBudget, + DAEMON_SOCKET_PATH_REMEDY, + describeDaemonSocketPathOverflow +} from '../daemon/daemon-runtime-paths' + +/** + * Version-scoped interpreter prefixes: a `brew upgrade node`, `nvm install`, or `mise use` + * removes the directory the pinned path names, and the unit then dies 203/EXEC long after + * the change that caused it. + */ +// The `\.?` matters: these tools install to a dotted directory (`.nvm`, `.asdf`) in a home +// directory but an undotted one under a shared prefix, and only one of those spellings +// would otherwise match. +const VERSION_SCOPED_INTERPRETER = + /\/Cellar\/|\/\.?nvm\/versions\/|\/\.?mise\/installs\/|\/\.?asdf\/installs\/|\/\.?volta\/tools\/image\// + +/** + * Why warn and not silently rewrite: "the service runs the node that generated it" is the + * contract, and guessing which stable symlink an operator meant is how a service ends up + * on an interpreter nobody chose. + */ +export function versionScopedInterpreterWarning(nodePath: string): string | null { + if (!VERSION_SCOPED_INTERPRETER.test(nodePath)) { + return null + } + return ( + `Warning: ExecStart pins ${nodePath}, which is a version-scoped path. ` + + 'Upgrading that package manager removes the directory and the service then fails ' + + '203/EXEC. Pass --node with a stable path (the manager’s current-version symlink) ' + + 'if you want the service to survive an interpreter upgrade.' + ) +} + +/** + * Why this exists at all: the version-scoped warning above tells operators to pass + * `--node`, and `--node` was the one interpreter source with no validation. The default + * cannot have the problem — `process.execPath` is tautologically on disk — so the gap bit + * only the people who did what the warning told them to. + * + * A warning rather than a refusal, on the same ground as the user-scope warning: generating + * on one host to install on another is legitimate, and the path may exist only there. + */ +export function interpreterOnDiskWarning(nodePath: string, wasChosen: boolean): string | null { + if (!wasChosen) { + return null + } + if (!existsSync(nodePath)) { + return ( + `Warning: --node ${nodePath} does not exist on this host, so the service would fail ` + + '203/EXEC if installed here. Intentional when you are generating for another host; ' + + 'otherwise check the path.' + ) + } + try { + accessSync(nodePath, constants.X_OK) + } catch { + return ( + `Warning: --node ${nodePath} exists but is not executable, so the service would fail ` + + '203/EXEC. Point --node at the interpreter binary itself.' + ) + } + return null +} + +/** + * The doctor calls an over-budget data root `critical` — the daemon cannot bind its socket, + * so the service starts and has no terminals. But it can only say so once the unit is + * installed and someone runs `--doctor`. print-service holds the same path at the moment + * the operator is watching, and said nothing: it would emit a unit pinning a root whose + * socket needs 113 of 108 available bytes, followed by a copy-paste install hint. + * + * Same generator/doctor asymmetry that let an unvalidated `--node` through, and warned + * about for the same reason: the root may be short enough on the host this file is + * destined for. + */ +export function socketPathBudgetWarning(userDataPath: string): string | null { + const budget = checkDaemonSocketPathBudget(userDataPath) + if (budget.fits) { + return null + } + return `Warning: ${describeDaemonSocketPathOverflow(budget)} ${DAEMON_SOCKET_PATH_REMEDY}` +} + +/** + * A user-scope systemd service needs a running user instance, and the install commands + * (`systemctl --user`, `loginctl enable-linger`) all fail without one. Appliance hosts + * routinely have no per-user D-Bus and no `/run/user/` at all. + */ +export async function userScopeUnavailableWarning(): Promise { + const uid = process.getuid?.() + if (uid === undefined) { + return null + } + const runtimeDir = join('/run/user', String(uid)) + if (!existsSync(runtimeDir)) { + return ( + `Warning: no ${runtimeDir}, so this host has no systemd user instance and no user ` + + 'D-Bus session. The install commands below will fail here; generate with ' + + '--scope system, or install this file on a host where user scope is available.' + ) + } + try { + const probe = await runProcess({ + program: 'systemctl', + args: ['--user', 'show-environment'], + timeoutMs: 5_000 + }) + if (probe.code === 0 && !probe.timedOut) { + return null + } + const detail = (probe.stderr || probe.stdout).trim().split('\n')[0] + return ( + 'Warning: the systemd user instance is not available here' + + `${detail ? ` (${detail})` : ''}. The install commands below will fail; generate ` + + 'with --scope system, or install this file on a host where user scope is available.' + ) + } catch (error) { + return ( + 'Warning: could not reach the systemd user instance ' + + `(${error instanceof Error ? error.message : String(error)}). The install commands ` + + 'below will fail here; generate with --scope system instead.' + ) + } +} + +/** + * The deepest existing ancestor, resolved, with the not-yet-created tail re-appended. + * + * Why not plain `realpathSync`: the data root usually does not exist when a service is + * first generated, and throwing there would be worse than the symlink it is fixing. + * Components that do not exist cannot themselves be symlinks, so re-appending them is + * still a fully-resolved path. + */ +export function resolveRealPath(target: string): string { + const tail: string[] = [] + let current = target + for (;;) { + try { + return join(realpathSync(current), ...tail) + } catch { + const parent = dirname(current) + if (parent === current) { + return target + } + tail.unshift(current.slice(parent.length + 1)) + current = parent + } + } +} diff --git a/src/main/orcad/supervisor-host-observations.ts b/src/main/orcad/supervisor-host-observations.ts new file mode 100644 index 000000000000..d27ed8443479 --- /dev/null +++ b/src/main/orcad/supervisor-host-observations.ts @@ -0,0 +1,102 @@ +/** + * Host facts the doctor reads off the filesystem rather than out of a supervisor. + * + * Deliberately not gated by `--no-probe`: that flag means "do not shell out to systemctl, + * launchctl or a socket", and these are stats and a config read. The doctor already reads + * the unit file itself, so refusing to stat the path that file names would be an odd line + * to draw — and both checks stay useful on a host where every subprocess probe is refused. + */ +import { existsSync, readFileSync, readdirSync } from 'node:fs' +import { join } from 'node:path' +import type { ExecTargetState, JournalState, Probe } from '../../shared/supervisor-service-probe' +import { + readExecTarget, + readPinnedUserData, + readSystemdKey, + type SupervisorServiceFile +} from '../../shared/supervisor-service-file-read' +import { resolveRealPath } from './supervisor-generation-warnings' + +/** + * Two spellings of one directory are not a split profile. Since the generator started + * pinning a realpath, a host whose data root sits behind a symlink — DSM's + * /var/services/homes -> /volume2/homes — otherwise reports a mismatch on every run. + */ +export function observeDataRootIdentity( + file: SupervisorServiceFile, + expected: string +): Probe | undefined { + const pinned = readPinnedUserData(file) + if (pinned === null) { + // Nothing pinned is its own (critical) finding; there is no identity to compare. + return undefined + } + return { status: 'observed', value: resolveRealPath(pinned) === resolveRealPath(expected) } +} + +export function observeExecTarget(file: SupervisorServiceFile): Probe { + const target = readExecTarget(file) + if (!target) { + return { status: 'unavailable', reason: 'the definition names no command to run' } + } + return { + status: 'observed', + value: { + interpreter: target.interpreter, + interpreterExists: existsSync(target.interpreter), + script: target.script, + scriptExists: target.script === null || existsSync(target.script) + } + } +} + +/** Drop-ins win over the main file, and a later drop-in wins over an earlier one. */ +const JOURNALD_CONFIG = '/etc/systemd/journald.conf' +const JOURNALD_DROPIN_DIRS = ['/etc/systemd/journald.conf.d', '/run/systemd/journald.conf.d'] + +function journaldConfigFiles(): string[] { + const files = existsSync(JOURNALD_CONFIG) ? [JOURNALD_CONFIG] : [] + for (const dir of JOURNALD_DROPIN_DIRS) { + try { + files.push( + ...readdirSync(dir) + .filter((name) => name.endsWith('.conf')) + .sort() + .map((name) => join(dir, name)) + ) + } catch { + // An absent or unreadable drop-in directory is normal, not a failure to report. + } + } + return files +} + +/** + * Why the default matters: journald's own default is `auto`, which is persistent only when + * /var/log/journal exists. Reporting the effective mode without that directory check would + * turn a real "logs do not survive a reboot" into a clean bill of health. + */ +export function observeJournal(file: SupervisorServiceFile): Probe { + const unitUsesJournal = + file.platform === 'systemd' && + ['StandardOutput', 'StandardError'].some((key) => + (readSystemdKey(file.text, key) ?? '').startsWith('journal') + ) + let storage: string | null = null + for (const path of journaldConfigFiles()) { + try { + const declared = /^\s*Storage\s*=\s*(\S+)/m.exec(readFileSync(path, 'utf8')) + if (declared) { + storage = declared[1].trim() + } + } catch { + // Unreadable config is not evidence of any particular storage mode. + } + } + if (storage === null) { + storage = existsSync('/var/log/journal') ? 'persistent' : 'volatile' + } else if (storage === 'auto') { + storage = existsSync('/var/log/journal') ? 'persistent' : 'volatile' + } + return { status: 'observed', value: { storage, unitUsesJournal } } +} diff --git a/src/main/orcad/supervisor-service-discovery.ts b/src/main/orcad/supervisor-service-discovery.ts new file mode 100644 index 000000000000..269b5ffcb440 --- /dev/null +++ b/src/main/orcad/supervisor-service-discovery.ts @@ -0,0 +1,111 @@ +/** + * Finding the supervisor service definitions installed on this host. + * + * Split out of `orcad-service-command.ts` so neither file has to carry both jobs: this one + * answers what is on disk, that one decides what to do about it. + * + * Presence and readability are separate answers here, deliberately. `existsSync` succeeds on + * a file the caller cannot open — a traversable parent is enough — so folding both into one + * catch reported an installed-but-unreadable unit as an absent one, and then told the + * operator to redo an install that had already succeeded. + */ +import { existsSync, readFileSync, statSync } from 'node:fs' +import { homedir } from 'node:os' +import { join } from 'node:path' +import { + ORCAD_LAUNCHD_LABEL, + ORCAD_SYSTEMD_UNIT_NAME, + type SupervisorPlatform, + type SupervisorScope +} from '../../shared/supervisor-service-render' +import type { SupervisorServiceFile } from '../../shared/supervisor-service-audit' + +/** The conventional locations, both scopes, for the platform we are on. */ +export function candidateServicePaths( + platform: SupervisorPlatform +): { path: string; scope: SupervisorScope }[] { + if (platform === 'launchd') { + return [ + { path: join('/Library/LaunchDaemons', `${ORCAD_LAUNCHD_LABEL}.plist`), scope: 'system' }, + { + path: join(homedir(), 'Library', 'LaunchAgents', `${ORCAD_LAUNCHD_LABEL}.plist`), + scope: 'user' + } + ] + } + return [ + { path: join('/etc/systemd/system', ORCAD_SYSTEMD_UNIT_NAME), scope: 'system' }, + { path: join('/usr/lib/systemd/system', ORCAD_SYSTEMD_UNIT_NAME), scope: 'system' }, + { path: join(homedir(), '.config', 'systemd', 'user', ORCAD_SYSTEMD_UNIT_NAME), scope: 'user' } + ] +} + +/** + * Why infer rather than default to system: mislabelling a user-scope file makes the audit + * report its (correct) missing run-as account as critical. + */ +export function inferScopeFromPath(path: string): SupervisorScope { + const normalized = path.split('\\').join('/') + return /\/systemd\/user\/|\/LaunchAgents\//i.test(normalized) ? 'user' : 'system' +} + +/** A candidate that is present but could not be read. Never folded into "not found". */ +export type UnreadableServiceFile = { path: string; reason: string } + +export type ServiceFileDiscovery = { + files: SupervisorServiceFile[] + unreadable: UnreadableServiceFile[] +} + +/** + * Reads every candidate rather than stopping at the first: two definitions targeting one + * data root is itself the highest-severity finding, and stopping early would hide it. + * + * Presence and readability are separated deliberately. `existsSync` succeeds on a file the + * caller cannot open — a traversable directory is enough — so a unit installed with a + * restrictive mode used to fall into the same catch as one that was never there, and the + * audit then told the operator to run `--print-service`: to redo an install that had + * already succeeded. Observed on Synology DSM, where root's umask is 0077 and + * `sudo tee` therefore writes /etc/systemd/system/orcad.service mode 600 while every unit + * shipped with the OS is 644. + */ +export function collectServiceFiles( + platform: SupervisorPlatform, + extraPaths: string[] = [] +): ServiceFileDiscovery { + const candidates = [ + ...candidateServicePaths(platform), + ...extraPaths.map((path) => ({ path, scope: inferScopeFromPath(path) })) + ] + const files: SupervisorServiceFile[] = [] + const unreadable: UnreadableServiceFile[] = [] + for (const candidate of candidates) { + let present = false + try { + present = existsSync(candidate.path) && statSync(candidate.path).isFile() + } catch (error) { + // Cannot even stat it: that is a permission answer about a path, not an absence. + unreadable.push({ path: candidate.path, reason: errnoOf(error) }) + continue + } + if (!present) { + continue + } + try { + files.push({ + path: candidate.path, + text: readFileSync(candidate.path, 'utf8'), + platform, + scope: candidate.scope + }) + } catch (error) { + unreadable.push({ path: candidate.path, reason: errnoOf(error) }) + } + } + return { files, unreadable } +} + +function errnoOf(error: unknown): string { + const code = (error as NodeJS.ErrnoException | null)?.code + return typeof code === 'string' ? code : 'unknown error' +} diff --git a/src/shared/orcad-artifacts.ts b/src/shared/orcad-artifacts.ts index 259b9d17eda2..d00640a8d7d0 100644 --- a/src/shared/orcad-artifacts.ts +++ b/src/shared/orcad-artifacts.ts @@ -11,18 +11,25 @@ export const ORCAD_VERSION = '0.1.0' +/** + * Every entry is required, by the build's emit check and by the remote install probe alike. + * + * There is deliberately no `optional` flag. The one artifact that would want it — the + * agent-browser binary, whose absence `resolveOrcadBrowserProvider` degrades past rather than + * treating as a torn install — cannot be an entry here at all: its filename is derived per + * platform-arch. A flag no entry can set reads as a promise this list does not keep, and the + * single filename accessor feeds two different questions (what the build must emit, what the + * probe must find), so a silent exemption would quietly stop the build checking its own output. + */ export type OrcadArtifact = { filename: string - /** - * Absence is a degradation, not a torn install, so the remote probe must not require it. - * The agent-browser binary is the only one: `resolveOrcadBrowserProvider` already answers - * "no headless browser" when it is missing, and it is named per platform-arch anyway. - */ - optional?: boolean } +/** The bundle's entry point, named once: a generated service definition execs this file. */ +export const ORCAD_ENTRY_FILENAME = 'orcad.js' + export const ORCAD_ARTIFACTS: readonly OrcadArtifact[] = [ - { filename: 'orcad.js' }, + { filename: ORCAD_ENTRY_FILENAME }, // Forked so a native @parcel/watcher fault kills the child, not the server. { filename: 'parcel-watcher-process-entry.js' }, // Forked so PTYs outlive the runtime process; its absence makes every restart destructive. @@ -36,7 +43,5 @@ export const ORCAD_VERSION_FILENAME = '.version' export const ORCAD_INSTALL_COMPLETE_FILENAME = '.install-complete' export function orcadArtifactFilenames(): string[] { - return ORCAD_ARTIFACTS.filter((artifact) => !artifact.optional).map( - (artifact) => artifact.filename - ) + return ORCAD_ARTIFACTS.map((artifact) => artifact.filename) } diff --git a/src/shared/supervisor-evidence-audit.test.ts b/src/shared/supervisor-evidence-audit.test.ts new file mode 100644 index 000000000000..3474877d8ccc --- /dev/null +++ b/src/shared/supervisor-evidence-audit.test.ts @@ -0,0 +1,269 @@ +import { describe, expect, it } from 'vitest' +import { auditSupervisorEvidence, supersededFileFindingCodes } from './supervisor-evidence-audit' +import type { Probe, SupervisorEvidence, UnitState } from './supervisor-service-probe' + +const UNAVAILABLE_REASONS = [ + 'systemctl did not respond within 5000ms', + 'systemctl: Failed to connect to bus: No medium found', + 'launchctl: Could not print domain: 5: Input/output error', + 'loginctl could not be run: spawn ENOENT', + '127.0.0.1:6800: EHOSTUNREACH' +] + +function unavailable(reason: string): Probe { + return { status: 'unavailable', reason } +} + +function running(overrides: Partial = {}): Probe { + return { + status: 'observed', + value: { + load: 'loaded', + active: 'active', + sub: 'running', + result: 'success', + restarts: 0, + ...overrides + } + } +} + +function codes(evidence: SupervisorEvidence): string[] { + return auditSupervisorEvidence(evidence).map((finding) => finding.code) +} + +describe('the never-guess rule', () => { + // The rule that makes the doctor worth running: a probe that could not answer must never + // produce a negative. If this fails, every finding it emits becomes untrustworthy. + it('never turns unavailable evidence into anything but unverifiable', () => { + for (const reason of UNAVAILABLE_REASONS) { + const findings = auditSupervisorEvidence({ + unitState: unavailable(reason), + linger: unavailable(reason), + configuredPortListening: unavailable(reason) + }) + expect(findings).toHaveLength(3) + for (const finding of findings) { + expect(finding.severity).toBe('unverifiable') + // The reason has to travel, or "unverified" reads as a broken tool. + expect(finding.message).toContain(reason) + } + } + }) + + it('emits nothing for a probe that was never run', () => { + expect(auditSupervisorEvidence({})).toEqual([]) + }) +}) + +describe('the severity boundary', () => { + // critical means "this will destroy running terminals", and only a file can say that. + it('never reports anything observed as critical', () => { + const everyState: SupervisorEvidence[] = [ + { unitState: running({ active: 'failed', sub: 'failed', result: 'exit-code' }) }, + { unitState: running({ active: 'inactive', sub: 'dead' }) }, + { unitState: running() }, + { linger: { status: 'observed', value: false } }, + { configuredPortListening: { status: 'observed', value: false } }, + { unitState: unavailable('nope'), linger: unavailable('nope') } + ] + for (const evidence of everyState) { + for (const finding of auditSupervisorEvidence(evidence)) { + expect(finding.severity).not.toBe('critical') + } + } + }) +}) + +describe('unit state', () => { + it('explains that a failed unit will not recover on its own', () => { + const findings = auditSupervisorEvidence({ + unitState: running({ active: 'failed', sub: 'failed', result: 'exit-code', restarts: 3 }) + }) + expect(findings[0].code).toBe('unit_failed') + expect(findings[0].message).toMatch(/permanently/) + expect(findings[0].remedy).toContain('reset-failed') + }) + + it('does not scold an operator for a service they stopped', () => { + const findings = auditSupervisorEvidence({ + unitState: running({ active: 'inactive', sub: 'dead' }) + }) + expect(findings[0].code).toBe('unit_inactive') + expect(findings[0].message).toMatch(/Deliberate if you stopped it/) + }) + + it('reports a running unit as ok', () => { + expect(codes({ unitState: running() })).toEqual(['unit_active']) + }) +}) + +describe('lingering', () => { + it('answers the question the file audit could only call unreadable', () => { + expect(codes({ linger: { status: 'observed', value: true } })).toEqual(['linger_enabled']) + expect(codes({ linger: { status: 'observed', value: false } })).toEqual(['linger_disabled']) + }) +}) + +describe('configured port', () => { + it('claims only that something is listening, never that it is orcad', () => { + const findings = auditSupervisorEvidence({ + configuredPortListening: { status: 'observed', value: true } + }) + expect(findings[0].message).toBe('Something is listening on the configured port.') + expect(findings[0].message).not.toMatch(/orcad/i) + }) + + it('reads an active unit with a silent port as the documented port fallback', () => { + const findings = auditSupervisorEvidence({ + unitState: running(), + configuredPortListening: { status: 'observed', value: false } + }) + const port = findings.find((finding) => finding.code === 'configured_port_silent') + expect(port?.message).toMatch(/falls back to an OS-assigned one/) + }) + + it('does not blame the fallback when the service is not even running', () => { + const findings = auditSupervisorEvidence({ + unitState: running({ active: 'inactive', sub: 'dead' }), + configuredPortListening: { status: 'observed', value: false } + }) + const port = findings.find((finding) => finding.code === 'configured_port_silent') + expect(port?.message).toBe('Nothing is listening on the configured port.') + }) +}) + +describe('load state', () => { + // The likeliest mistake in a print-and-place workflow: the file is written but never + // loaded. systemctl show exits 0 and calls that `inactive`, which is also what a service + // someone deliberately stopped looks like. + it('separates a never-loaded unit from a stopped one', () => { + const findings = auditSupervisorEvidence({ + unitState: running({ load: 'not-found', active: 'inactive', sub: 'dead' }) + }) + expect(findings[0].code).toBe('unit_not_loaded') + expect(findings[0].message).toMatch(/Placing the file is not the last step/) + expect(findings[0].remedy).toContain('daemon-reload') + }) + + it('reports a masked unit, which no file contents can fix', () => { + const findings = auditSupervisorEvidence({ + unitState: running({ load: 'masked', active: 'inactive', sub: 'dead' }) + }) + expect(findings[0].code).toBe('unit_masked') + }) + + it('still reports a genuinely stopped loaded unit as stopped', () => { + const findings = auditSupervisorEvidence({ + unitState: running({ load: 'loaded', active: 'inactive', sub: 'dead' }) + }) + expect(findings[0].code).toBe('unit_inactive') + }) +}) + +describe('exec target on disk', () => { + // A unit can be well-formed and name an interpreter that no longer exists — what a + // version-scoped path becomes one package-manager upgrade later. + it('reports a missing interpreter as a warning, not a critical', () => { + const findings = auditSupervisorEvidence({ + execTarget: { + status: 'observed', + value: { + interpreter: '/home/linuxbrew/.linuxbrew/Cellar/node/25.8.0/bin/node', + interpreterExists: false, + script: '/opt/orcad/orcad.js', + scriptExists: true + } + } + }) + expect(findings[0].code).toBe('exec_target_absent') + // It cannot start, but it destroys no running terminals. + expect(findings[0].severity).toBe('warning') + expect(findings[0].message).toMatch(/203\/EXEC/) + }) + + it('names the missing script too', () => { + const findings = auditSupervisorEvidence({ + execTarget: { + status: 'observed', + value: { + interpreter: '/usr/bin/node', + interpreterExists: true, + script: '/opt/orcad/orcad.js', + scriptExists: false + } + } + }) + expect(findings[0].message).toContain('/opt/orcad/orcad.js') + }) + + it('passes when both resolve', () => { + expect( + codes({ + execTarget: { + status: 'observed', + value: { + interpreter: '/usr/bin/node', + interpreterExists: true, + script: '/opt/orcad/orcad.js', + scriptExists: true + } + } + }) + ).toEqual(['exec_target_present']) + }) +}) + +describe('journal persistence', () => { + it('flags a volatile journal for a unit that logs to it', () => { + const findings = auditSupervisorEvidence({ + journal: { status: 'observed', value: { storage: 'volatile', unitUsesJournal: true } } + }) + expect(findings[0].code).toBe('journal_volatile') + expect(findings[0].severity).toBe('warning') + expect(findings[0].message).toMatch(/reboot/) + }) + + it('says nothing when the unit does not log to the journal', () => { + expect( + codes({ + journal: { status: 'observed', value: { storage: 'volatile', unitUsesJournal: false } } + }) + ).toEqual([]) + }) + + it('says nothing when the journal is persistent', () => { + expect( + codes({ + journal: { status: 'observed', value: { storage: 'persistent', unitUsesJournal: true } } + }) + ).toEqual([]) + }) +}) + +describe('data root identity', () => { + // Two spellings of one directory are not a split profile. Since the generator began + // pinning a realpath, a host whose root sits behind a symlink reported a mismatch on + // every run — noise, by construction, on every such host. + it('reports the roots as one directory when they resolve the same', () => { + expect(codes({ dataRootSameDirectory: { status: 'observed', value: true } })).toEqual([ + 'user_data_same_directory' + ]) + }) + + it('supersedes the file audit string mismatch when they are one directory', () => { + const live = auditSupervisorEvidence({ + dataRootSameDirectory: { status: 'observed', value: true } + }) + expect(supersededFileFindingCodes(live)).toContain('user_data_mismatch') + }) + + it('leaves a genuine mismatch to the file audit', () => { + expect(codes({ dataRootSameDirectory: { status: 'observed', value: false } })).toEqual([]) + expect( + supersededFileFindingCodes( + auditSupervisorEvidence({ dataRootSameDirectory: { status: 'observed', value: false } }) + ) + ).not.toContain('user_data_mismatch') + }) +}) diff --git a/src/shared/supervisor-evidence-audit.ts b/src/shared/supervisor-evidence-audit.ts new file mode 100644 index 000000000000..4eb5210318da --- /dev/null +++ b/src/shared/supervisor-evidence-audit.ts @@ -0,0 +1,233 @@ +/** + * Turns live evidence about a running orcad service into findings. + * + * Split from the file audit for one reason worth stating plainly: `critical` means THIS + * CONFIGURATION WILL DESTROY RUNNING TERMINALS, and only a file can say that. Nothing + * observed here is ever critical — a service an operator deliberately stopped is a fact + * worth reporting, not a failure, and an exit code that fires on ordinary states is one + * people stop reading. A separate module keeps that boundary visible rather than + * re-decided per check. + * + * Pure like its sibling: evidence in, findings out, so every `unavailable` reason is + * assertable without running a subprocess. + */ +import type { SupervisorFinding } from './supervisor-service-audit' +import type { Probe, SupervisorEvidence } from './supervisor-service-probe' + +/** One shape for every unreadable probe, so the reason never gets flattened away. */ +function unverified(code: string, probe: Probe, subject: string): SupervisorFinding { + return { + code, + severity: 'unverifiable', + message: `${subject} could not be established: ${ + probe.status === 'unavailable' ? probe.reason : 'no probe ran' + }` + } +} + +function auditUnitState(evidence: SupervisorEvidence): SupervisorFinding | null { + const probe = evidence.unitState + if (!probe) { + return null + } + if (probe.status !== 'observed') { + return unverified('unit_state_unverified', probe, 'Whether the service is running') + } + const { load, active, sub, result, restarts } = probe.value + // Checked before ActiveState, which reads `inactive` for a unit the supervisor has + // never loaded — the same answer as a service someone deliberately stopped. + if (load === 'not-found') { + return { + code: 'unit_not_loaded', + severity: 'warning', + message: + 'The file is on disk but the supervisor has never loaded it, so nothing is ' + + 'supervising orcad. Placing the file is not the last step.', + remedy: 'systemctl daemon-reload, then systemctl enable , then systemctl start ' + } + } + if (load === 'masked') { + return { + code: 'unit_masked', + severity: 'warning', + message: 'The unit is masked, so it will never start no matter what the file says.', + remedy: 'systemctl unmask ' + } + } + // A failed unit after exit 78 is the stranding the generated file warns about: it will + // not come back on its own even once the cause is gone. + if (active === 'failed') { + return { + code: 'unit_failed', + severity: 'warning', + message: `Service is failed (${sub}, result=${result}, ${restarts} restarts). Exit 78 leaves it here permanently, including after the cause is fixed.`, + remedy: 'systemctl reset-failed, then start it again' + } + } + if (active !== 'active') { + return { + code: 'unit_inactive', + severity: 'warning', + message: `Service is ${active} (${sub}). Deliberate if you stopped it.` + } + } + return { code: 'unit_active', severity: 'ok', message: `Service is active (${sub}).` } +} + +function auditLinger(evidence: SupervisorEvidence): SupervisorFinding | null { + const probe = evidence.linger + if (!probe) { + return null + } + if (probe.status !== 'observed') { + return unverified('linger_unverified_live', probe, 'Whether the service survives logout') + } + return probe.value + ? { + code: 'linger_enabled', + severity: 'ok', + message: 'Lingering is enabled: the service survives logout.' + } + : { + code: 'linger_disabled', + severity: 'warning', + message: + 'Lingering is disabled, so this user-scope service stops when your last session ends.', + remedy: 'sudo loginctl enable-linger "$USER"' + } +} + +/** + * Worded as what was observed. A listener is not proof it is orcad — anything on the host + * could hold that port, which is exactly the conflict that causes the fallback this check + * exists to surface. + */ +function auditConfiguredPort(evidence: SupervisorEvidence): SupervisorFinding | null { + const probe = evidence.configuredPortListening + if (!probe) { + return null + } + if (probe.status !== 'observed') { + return unverified('configured_port_unverified', probe, 'Whether the configured port is served') + } + if (probe.value) { + return { + code: 'configured_port_listening', + severity: 'ok', + message: 'Something is listening on the configured port.' + } + } + const active = + evidence.unitState?.status === 'observed' && evidence.unitState.value.active === 'active' + return { + code: 'configured_port_silent', + severity: 'warning', + message: active + ? 'The service is active but nothing is listening on the configured port — a pinned port still falls back to an OS-assigned one on conflict.' + : 'Nothing is listening on the configured port.', + remedy: active + ? 'Check the bound endpoint in the readiness line before relying on an SSH forward.' + : undefined + } +} + +/** + * File-level findings that a live answer replaces. Declared here because this module owns + * the knowledge; the caller matching on code prefixes coupled the two by string shape, and + * any future `linger*` code would have silently changed which findings got dropped. + */ +export function supersededFileFindingCodes(live: readonly SupervisorFinding[]): string[] { + const superseded: string[] = [] + if (live.some((finding) => LINGER_CODES.has(finding.code))) { + superseded.push('linger_unverified') + } + // The file audit can only compare the two roots as strings, which since the generator + // began pinning a realpath reads two spellings of one directory as a split profile. + if (live.some((finding) => finding.code === 'user_data_same_directory')) { + superseded.push('user_data_mismatch') + } + return superseded +} + +const LINGER_CODES = new Set(['linger_enabled', 'linger_disabled', 'linger_unverified_live']) + +/** + * A unit can be perfectly well-formed and name an interpreter that no longer exists — a + * version-scoped path one package-manager upgrade later. It cannot start, so it is a + * warning; it destroys no running terminals, so it is not critical. + */ +function auditExecTargetOnDisk(evidence: SupervisorEvidence): SupervisorFinding | null { + const probe = evidence.execTarget + if (!probe) { + return null + } + if (probe.status !== 'observed') { + return unverified('exec_target_unverified', probe, 'Whether ExecStart can be executed') + } + const { interpreter, interpreterExists, script, scriptExists } = probe.value + const missing = [ + ...(interpreterExists ? [] : [`interpreter ${interpreter}`]), + ...(script && !scriptExists ? [`script ${script}`] : []) + ] + if (missing.length === 0) { + return { code: 'exec_target_present', severity: 'ok', message: 'ExecStart resolves on disk.' } + } + return { + code: 'exec_target_absent', + severity: 'warning', + message: `ExecStart does not exist on disk: ${missing.join(' and ')} ${missing.length > 1 ? 'are' : 'is'} missing, so the service cannot start (203/EXEC).`, + remedy: 'Regenerate with orcad --print-service, passing --node with a stable path.' + } +} + +/** + * The generated unit logs to journald because orcad rotates nothing of its own — sound + * reasoning that assumes a persistent, readable journal. Appliance hosts routinely set + * `Storage=volatile`, which puts the journal in /run and loses it on reboot. + */ +function auditJournalPersistence(evidence: SupervisorEvidence): SupervisorFinding | null { + const probe = evidence.journal + if (!probe) { + return null + } + if (probe.status !== 'observed') { + return unverified('journal_storage_unverified', probe, "journald's storage mode") + } + const { storage, unitUsesJournal } = probe.value + if (!unitUsesJournal || storage !== 'volatile') { + return null + } + return { + code: 'journal_volatile', + severity: 'warning', + message: + 'The unit logs to the journal, but journald Storage=volatile keeps the journal in ' + + '/run, so these logs do not persist across a reboot and are gone exactly when you ' + + 'would go looking for why the host restarted.', + remedy: 'Set Storage=persistent in journald.conf, or point the unit at a file you rotate.' + } +} + +function auditDataRootIdentity(evidence: SupervisorEvidence): SupervisorFinding | null { + const probe = evidence.dataRootSameDirectory + if (!probe || probe.status !== 'observed' || !probe.value) { + // A genuine mismatch needs no finding here: the file audit already reported it. + return null + } + return { + code: 'user_data_same_directory', + severity: 'ok', + message: 'The service and this shell resolve the same data root directory.' + } +} + +export function auditSupervisorEvidence(evidence: SupervisorEvidence): SupervisorFinding[] { + return [ + auditDataRootIdentity(evidence), + auditUnitState(evidence), + auditLinger(evidence), + auditConfiguredPort(evidence), + auditExecTargetOnDisk(evidence), + auditJournalPersistence(evidence) + ].filter((finding): finding is SupervisorFinding => finding !== null) +} diff --git a/src/shared/supervisor-service-audit.test.ts b/src/shared/supervisor-service-audit.test.ts new file mode 100644 index 000000000000..24710a5389d2 --- /dev/null +++ b/src/shared/supervisor-service-audit.test.ts @@ -0,0 +1,341 @@ +import { describe, expect, it } from 'vitest' +import { + auditSupervisorServices, + readConfiguredEndpoint, + sortBySeverity, + supervisorAuditPassed, + type SupervisorServiceFile +} from './supervisor-service-audit' +import { renderSupervisorService } from './supervisor-service-render' + +const ROOT = '/home/orca/.orca' + +function file(overrides: Partial = {}): SupervisorServiceFile { + return { + path: '/etc/systemd/system/orcad.service', + platform: 'systemd', + scope: 'system', + text: renderSupervisorService({ + platform: 'systemd', + scope: 'system', + nodePath: '/usr/local/bin/node', + orcadPath: '/opt/orcad/orcad.js', + userDataPath: ROOT, + user: 'orca', + bind: '127.0.0.1', + port: 6800 + }), + ...overrides + } +} + +function audit(files: SupervisorServiceFile[], expected = ROOT) { + return auditSupervisorServices({ files, expectedUserDataPath: expected }) +} + +function codes(files: SupervisorServiceFile[], expected = ROOT): string[] { + return audit(files, expected).map((finding) => finding.code) +} + +/** + * A `.replace()` that matches nothing returns the string unchanged and the test then + * asserts against the unmodified unit. Three tests below went vacuous exactly that way + * when the rendered default moved off `mixed`: they kept passing a healthy unit to an + * audit and expecting a failure code. Substitutions here must land. + */ +function swapKillMode(text: string, replacement: string): string { + const swapped = text.replace('KillMode=process', replacement) + if (swapped === text) { + throw new Error('the rendered unit no longer contains KillMode=process; update this test') + } + return swapped +} + +/** What `brew services` produces: a plausible unit with no KillMode at all. */ +const HOMEBREW_SHAPED_UNIT = `[Unit] +Description=Homebrew generated unit for orcad + +[Service] +Type=simple +ExecStart=/opt/homebrew/bin/node /opt/homebrew/opt/orcad/orcad.js +Restart=always +User=orca +Environment=ORCA_USER_DATA=${ROOT} + +[Install] +WantedBy=multi-user.target +` + +describe('kill semantics', () => { + it('passes its own rendered output', () => { + expect(supervisorAuditPassed(audit([file()]))).toBe(true) + }) + + it('flags a brew-services-shaped unit as critical', () => { + const findings = audit([file({ text: HOMEBREW_SHAPED_UNIT })]) + expect(findings[0].code).toBe('kill_mode_missing') + expect(findings[0].severity).toBe('critical') + expect(supervisorAuditPassed(findings)).toBe(false) + }) + + it('flags an explicit control-group', () => { + const text = swapKillMode(file().text, 'KillMode=control-group') + expect(codes([file({ text })])).toContain('kill_mode_reaps_group') + }) + + // The mode orcad itself generated until a NAS restart destroyed a live terminal under + // it. mixed SIGKILLs whatever remains in the cgroup once the main process is gone, and + // the detached daemon is in the cgroup: setsid escapes the process group, not the cgroup. + it('flags KillMode=mixed, which orcad used to generate, as critical', () => { + const text = swapKillMode(file().text, 'KillMode=mixed') + const findings = audit([file({ text })]) + expect(findings[0].code).toBe('kill_mode_mixed_reaps_daemon') + expect(findings[0].severity).toBe('critical') + expect(supervisorAuditPassed(findings)).toBe(false) + // The remedy has to name the mode that works, not the one being rejected. + expect(findings[0].remedy).toContain('KillMode=process') + }) + + it('warns rather than blesses KillMode=none, which signals nothing at all', () => { + const text = swapKillMode(file().text, 'KillMode=none') + const findings = audit([file({ text })]) + expect(findings.map((f) => f.code)).toContain('kill_mode_discouraged') + // It spares the daemon, so it is not the critical failure this audit exists to catch. + expect(supervisorAuditPassed(findings)).toBe(true) + }) + + it('does not read a commented-out KillMode as set', () => { + const text = swapKillMode(file().text, '# KillMode=process') + expect(codes([file({ text })])).toContain('kill_mode_missing') + }) + + it('accepts a plist only when AbandonProcessGroup is stated', () => { + const plist = renderSupervisorService({ + platform: 'launchd', + scope: 'system', + nodePath: '/usr/local/bin/node', + orcadPath: '/opt/orcad/orcad.js', + userDataPath: ROOT, + user: 'orca', + bind: '127.0.0.1', + port: 6800 + }) + expect(codes([file({ platform: 'launchd', text: plist })])).toContain('kill_semantics_safe') + + const stripped = plist.replace(/AbandonProcessGroup<\/key>\s*/, '') + const findings = audit([file({ platform: 'launchd', text: stripped })]) + expect(findings.map((f) => f.code)).toContain('kill_semantics_implicit') + // Implicit survival is a warning, not a failure: it does work today. + expect(supervisorAuditPassed(findings)).toBe(true) + }) +}) + +describe('data root', () => { + it('flags an unpinned root as critical', () => { + const text = file().text.replace(`Environment=ORCA_USER_DATA=${ROOT}`, '') + expect(codes([file({ text })])).toContain('user_data_unpinned') + }) + + it('reports disagreement with the calling shell as a warning, not a failure', () => { + const findings = audit([file()], '/home/someone-else/.orca') + expect(findings.map((f) => f.code)).toContain('user_data_mismatch') + expect(supervisorAuditPassed(findings)).toBe(true) + }) +}) + +describe('run-as account', () => { + it('flags a system unit with no User', () => { + const text = file().text.replace('User=orca', '') + expect(codes([file({ text })])).toContain('run_as_user_unset') + }) + + it('flags User=root', () => { + const text = file().text.replace('User=orca', 'User=root') + expect(codes([file({ text })])).toContain('run_as_root') + }) + + it('accepts a user-scope unit with no User', () => { + const text = file().text.replace('User=orca', '') + const findings = audit([file({ text, scope: 'user' })]) + expect(supervisorAuditPassed(findings)).toBe(true) + }) +}) + +describe('duplicates', () => { + it('ranks two services on one data root above everything else', () => { + const findings = audit([ + file(), + file({ path: '/home/orca/.config/systemd/user/orcad.service', scope: 'user' }) + ]) + expect(findings[0].code).toBe('multiple_services_one_root') + expect(findings[0].severity).toBe('critical') + expect(findings[0].message).toMatch(/exits 78/) + }) + + it('treats distinct roots as a warning only', () => { + const other = file({ + path: '/home/orca/.config/systemd/user/orcad.service', + scope: 'user', + // replaceAll: the root appears in both RequiresMountsFor and Environment, and + // rewriting only the first leaves the two files still sharing a data root. + text: file().text.replaceAll(ROOT, '/srv/other/.orca') + }) + const findings = audit([file(), other]) + expect(findings.map((f) => f.code)).toContain('multiple_services_distinct_roots') + }) +}) + +describe('unverifiable is never a negative verdict', () => { + it('reports a missing service as unverified rather than clean or failed', () => { + const findings = audit([]) + expect(findings[0].severity).toBe('unverifiable') + // Nothing found is not evidence of a broken host, so it must not fail the audit. + expect(supervisorAuditPassed(findings)).toBe(true) + }) + + it('reports lingering as unreadable from a user unit rather than guessing', () => { + const findings = audit([file({ scope: 'user' })]) + const linger = findings.find((f) => f.code === 'linger_unverified') + expect(linger?.severity).toBe('unverifiable') + expect(linger?.remedy).toContain('enable-linger') + }) + + it('warns that a LaunchAgent dies at logout', () => { + expect(codes([file({ platform: 'launchd', scope: 'user' })])).toContain('launch_agent_scope') + }) +}) + +describe('configured endpoint', () => { + // Probing a flag default while the file names another port answers the wrong question + // with full confidence — caught by running it, not by reading it. + it('reads the endpoint the installed file will actually bind', () => { + const text = file() + .text.replace('--port 6800', '--port 6899') + .replace('--bind 127.0.0.1', '--bind 0.0.0.0') + expect(readConfiguredEndpoint(file({ text }))).toEqual({ bind: '0.0.0.0', port: 6899 }) + }) + + it('reads it out of plist ProgramArguments too', () => { + const plist = renderSupervisorService({ + platform: 'launchd', + scope: 'system', + nodePath: '/usr/local/bin/node', + orcadPath: '/opt/orcad/orcad.js', + userDataPath: ROOT, + user: 'orca', + bind: '127.0.0.1', + port: 7100 + }) + expect(readConfiguredEndpoint(file({ platform: 'launchd', text: plist }))?.port).toBe(7100) + }) + + it('returns null rather than a guess when no port is named', () => { + expect(readConfiguredEndpoint(file({ text: HOMEBREW_SHAPED_UNIT }))).toBeNull() + }) +}) + +describe('severity ordering', () => { + // A finding appended by a caller after the audit returned was landing past the OK rows, + // where an operator scanning top-down stops reading before reaching it. + it('keeps every severity in rank order, including appended findings', () => { + const sorted = sortBySeverity([ + { code: 'a', severity: 'ok', message: 'ok' }, + { code: 'b', severity: 'unverifiable', message: 'appended late' }, + { code: 'c', severity: 'critical', message: 'critical' }, + { code: 'd', severity: 'ok', message: 'ok' }, + { code: 'e', severity: 'warning', message: 'warning' } + ]) + expect(sorted.map((finding) => finding.severity)).toEqual([ + 'critical', + 'warning', + 'unverifiable', + 'ok', + 'ok' + ]) + }) + + it("does not mutate the caller's array", () => { + const input: Parameters[0] = [ + { code: 'a', severity: 'ok', message: 'ok' }, + { code: 'b', severity: 'critical', message: 'critical' } + ] + sortBySeverity(input) + expect(input.map((finding) => finding.code)).toEqual(['a', 'b']) + }) +}) + +describe('live evidence supersedes the file-level unknown', () => { + it('drops the unreadable-linger finding once a live reading answers it', () => { + const findings = auditSupervisorServices({ + files: [file({ scope: 'user' })], + expectedUserDataPath: ROOT, + evidence: { linger: { status: 'observed', value: true } } + }) + const codes = findings.map((finding) => finding.code) + expect(codes).toContain('linger_enabled') + expect(codes).not.toContain('linger_unverified') + }) + + it('keeps it when there is no evidence at all', () => { + expect(codes([file({ scope: 'user' })])).toContain('linger_unverified') + }) +}) + +/** + * An unreadable definition and an absent one demand opposite actions, so they must not + * share a finding. `existsSync` succeeds on a file the caller cannot open, which is how a + * unit installed mode 600 — what `sudo tee` writes under root's 0077 umask on Synology DSM + * — used to be reported as "no orcad service definition found in the conventional + * locations", with `--print-service` as the remedy: redo an install that had succeeded. + */ +describe('a definition that exists but cannot be read', () => { + const unreadable = [{ path: '/etc/systemd/system/orcad.service', reason: 'EACCES' }] + + it('is not reported as an absent install', () => { + const found = auditSupervisorServices({ files: [], expectedUserDataPath: ROOT, unreadable }) + const codes = found.map((f) => f.code) + expect(codes).toContain('service_definition_unreadable') + expect(codes).not.toContain('no_service_found') + }) + + it('never tells the operator to print a unit they already installed', () => { + const [finding] = auditSupervisorServices({ + files: [], + expectedUserDataPath: ROOT, + unreadable + }) + expect(finding.remedy).not.toContain('--print-service') + expect(finding.remedy).toContain('/etc/systemd/system/orcad.service') + }) + + it('names the path and the errno, since neither alone identifies the problem', () => { + const [finding] = auditSupervisorServices({ + files: [], + expectedUserDataPath: ROOT, + unreadable + }) + expect(finding.message).toContain('/etc/systemd/system/orcad.service') + expect(finding.message).toContain('EACCES') + }) + + // An unreadable second file is exactly the duplicate auditDuplicates exists to catch, + // and it cannot see this one — so silence here would hide the higher-severity finding. + it('is still reported when another definition did read cleanly', () => { + const codes = auditSupervisorServices({ + files: [file()], + expectedUserDataPath: ROOT, + unreadable: [{ path: '/usr/lib/systemd/system/orcad.service', reason: 'EACCES' }] + }).map((f) => f.code) + expect(codes).toContain('service_definition_unreadable') + }) + + it('still reports a genuinely absent install as absent (negative control)', () => { + const codes = auditSupervisorServices({ + files: [], + expectedUserDataPath: ROOT, + unreadable: [] + }).map((f) => f.code) + expect(codes).toContain('no_service_found') + expect(codes).not.toContain('service_definition_unreadable') + }) +}) diff --git a/src/shared/supervisor-service-audit.ts b/src/shared/supervisor-service-audit.ts new file mode 100644 index 000000000000..dfd6d3acdb2c --- /dev/null +++ b/src/shared/supervisor-service-audit.ts @@ -0,0 +1,351 @@ +/** + * Static analysis of an installed supervisor service definition. + * + * Pure by contract: file text in, findings out. It runs no subprocess and reads no disk, + * so it is assertable against known-bad fixtures — a Homebrew-shaped unit with no + * `KillMode`, a root-run unit, a data root that disagrees with the caller's. + * + * The `unverifiable` severity is load-bearing. Anything this analyser cannot see from the + * file alone (is the unit actually running, is lingering enabled) must be reported as + * unverified rather than as a negative: a failed probe is not evidence of a stopped + * service, and guessing is the failure mode `ssh-execution-boundary.md` forbids. + */ +import { auditSupervisorEvidence, supersededFileFindingCodes } from './supervisor-evidence-audit' +import type { SupervisorEvidence } from './supervisor-service-probe' +import { + readPinnedUserData, + readPlistBoolean, + readPlistString, + readSystemdKey, + type SupervisorServiceFile +} from './supervisor-service-file-read' +import { + DISCOURAGED_SYSTEMD_KILL_MODES, + SAFE_SYSTEMD_KILL_MODES +} from './supervisor-service-render' + +export type { SupervisorServiceFile } from './supervisor-service-file-read' +export { readConfiguredEndpoint } from './supervisor-service-file-read' + +export type SupervisorFindingSeverity = 'critical' | 'warning' | 'unverifiable' | 'ok' + +export type SupervisorFinding = { + code: string + severity: SupervisorFindingSeverity + message: string + /** Shown verbatim to an operator; omitted when there is nothing to run. */ + remedy?: string +} + +export type SupervisorAuditInput = { + /** Every definition found, not just the first — two contending for one data root is the finding. */ + files: SupervisorServiceFile[] + /** What the calling shell resolves, to compare against what the file pins. */ + expectedUserDataPath: string + /** Absent is legal: with no evidence this is exactly the file-only audit. */ + evidence?: SupervisorEvidence + /** Candidates that are present but could not be read. Not the same as none found. */ + unreadable?: { path: string; reason: string }[] +} + +/** + * The check the whole feature exists for. A missing `KillMode` is not a neutral omission: + * systemd's default is `control-group`, which reaps the detached terminal daemon and turns + * every restart into lost work. That is precisely what `brew services` generates. + */ +function auditKillSemantics(file: SupervisorServiceFile): SupervisorFinding { + if (file.platform === 'launchd') { + const abandon = readPlistBoolean(file.text, 'AbandonProcessGroup') + if (abandon === true) { + return { + code: 'kill_semantics_safe', + severity: 'ok', + message: 'AbandonProcessGroup is true: launchd will not reap the terminal daemon.' + } + } + return { + code: 'kill_semantics_implicit', + severity: 'warning', + message: + 'AbandonProcessGroup is not set. Terminals survive today only because the daemon ' + + 'setsid()s out of the job process group — an implementation detail, not a guarantee.', + remedy: 'Regenerate with: orcad --print-service' + } + } + const killMode = readSystemdKey(file.text, 'KillMode') + if (killMode === null) { + return { + code: 'kill_mode_missing', + severity: 'critical', + message: + 'No KillMode, so systemd applies control-group and kills the detached terminal ' + + 'daemon with the runtime. Every restart destroys running terminals. A unit ' + + 'generated by `brew services` looks exactly like this.', + remedy: 'Regenerate with: orcad --print-service' + } + } + // Named separately from the generic case because orcad generated this itself until it + // was measured: an operator reading this finding is being told that a unit written by + // this tool does not do what its own comment claims. Lumping it in with `control-group` + // would bury that. + if (killMode === 'mixed') { + return { + code: 'kill_mode_mixed_reaps_daemon', + severity: 'critical', + message: + 'KillMode=mixed SIGTERMs the main process and then SIGKILLs everything still in ' + + 'the control group, which includes the detached terminal daemon — setsid does not ' + + 'leave a cgroup. Measured on systemd 219: a restart destroyed the daemon, its ' + + 'shell and its scrollback, where the identical unit under KillMode=process kept ' + + 'all three. Units orcad generated before this was corrected carry it.', + remedy: 'Set KillMode=process, or regenerate with: orcad --print-service' + } + } + if (!SAFE_SYSTEMD_KILL_MODES.includes(killMode as (typeof SAFE_SYSTEMD_KILL_MODES)[number])) { + return { + code: 'kill_mode_reaps_group', + severity: 'critical', + message: `KillMode=${killMode} reaps the whole control group, including the detached terminal daemon.`, + remedy: 'Set KillMode=process' + } + } + if ( + DISCOURAGED_SYSTEMD_KILL_MODES.includes( + killMode as (typeof DISCOURAGED_SYSTEMD_KILL_MODES)[number] + ) + ) { + return { + code: 'kill_mode_discouraged', + severity: 'warning', + message: `KillMode=${killMode} spares the daemon but signals nothing at all, so orcad never gets its graceful stop and systemd deprecates it.`, + remedy: 'Set KillMode=process' + } + } + return { + code: 'kill_semantics_safe', + severity: 'ok', + message: `KillMode=${killMode}: the terminal daemon survives a runtime restart.` + } +} + +function auditUserDataPath(file: SupervisorServiceFile, expected: string): SupervisorFinding { + const pinned = readPinnedUserData(file) + if (pinned === null) { + return { + code: 'user_data_unpinned', + severity: 'critical', + message: + 'No ORCA_USER_DATA. A service inherits almost no environment, so it resolves its ' + + 'own data root — which can differ from the one your shell resolves. The service ' + + 'then starts healthy and shows none of your repositories.', + remedy: `Add the resolved root explicitly: ${expected}` + } + } + if (pinned !== expected) { + return { + code: 'user_data_mismatch', + severity: 'warning', + message: `Service uses ${pinned}; this shell resolves ${expected}. Deliberate for a dedicated service account, a split profile otherwise.` + } + } + return { code: 'user_data_agrees', severity: 'ok', message: `Data root pinned to ${pinned}.` } +} + +function auditRunAsUser(file: SupervisorServiceFile): SupervisorFinding { + const user = + file.platform === 'systemd' + ? readSystemdKey(file.text, 'User') + : readPlistString(file.text, 'UserName') + if (user === null) { + return { + code: 'run_as_user_unset', + severity: file.scope === 'system' ? 'critical' : 'ok', + message: + file.scope === 'system' + ? 'No run-as account on a system service, so orcad runs as root and creates a ' + + 'root-owned data root. Every later run as a normal user then fails with ' + + 'orcad_data_root_wrong_owner, which orcad will not repair.' + : 'No run-as account, which is correct for a user-scope service.', + remedy: file.scope === 'system' ? 'Add an explicit run-as account.' : undefined + } + } + if (user === 'root') { + return { + code: 'run_as_root', + severity: 'critical', + message: + 'Service runs orcad as root, which creates a root-owned data root no later user run can use.', + remedy: 'Set the account orcad should run as.' + } + } + return { code: 'run_as_user_set', severity: 'ok', message: `Runs as ${user}.` } +} + +/** + * A user-scope service dies with the session that installed it — the exact failure this + * whole feature prevents. Whether lingering is on cannot be read from the file, so this + * reports `unverifiable` rather than pretending either way. + */ +function auditSurvivesLogout(file: SupervisorServiceFile): SupervisorFinding | null { + if (file.scope === 'system') { + return null + } + return file.platform === 'systemd' + ? { + code: 'linger_unverified', + severity: 'unverifiable', + message: + 'User-scope unit: it stops when your last session ends unless lingering is enabled. ' + + 'Not readable from the unit file.', + remedy: + 'loginctl show-user "$USER" --property=Linger # then: sudo loginctl enable-linger "$USER"' + } + : { + code: 'launch_agent_scope', + severity: 'warning', + message: + 'LaunchAgent: runs only while this user is logged in, so a headless Mac loses the ' + + 'service at logout.', + remedy: 'Regenerate with --scope system to get a LaunchDaemon.' + } +} + +/** Both formats can be syntactically fine and still name nothing to run. */ +function auditExecTarget(file: SupervisorServiceFile): SupervisorFinding | null { + const hasTarget = + file.platform === 'systemd' + ? readSystemdKey(file.text, 'ExecStart') !== null + : /\s*ProgramArguments\s*<\/key>\s*\s*/i.test(file.text) + return hasTarget + ? null + : { + code: 'exec_target_missing', + severity: 'critical', + message: + file.platform === 'systemd' + ? 'No ExecStart: this unit cannot start anything.' + : 'No ProgramArguments: this job cannot start anything.' + } +} + +/** + * Ranked first when it fires, because it explains failures every other check would + * misattribute: the second service to start hits orcad's instance lock, exits 78, and + * `RestartPreventExitStatus=78` leaves it permanently failed. The visible symptom is a + * service that will not start; the cause is a different one that already won. + */ +function auditDuplicates(input: SupervisorAuditInput): SupervisorFinding | null { + if (input.files.length < 2) { + return null + } + const roots = new Map() + for (const file of input.files) { + const root = readPinnedUserData(file) ?? input.expectedUserDataPath + roots.set(root, [...(roots.get(root) ?? []), file.path]) + } + const contended = [...roots.entries()].filter(([, paths]) => paths.length > 1) + if (contended.length === 0) { + return { + code: 'multiple_services_distinct_roots', + severity: 'warning', + message: `${input.files.length} orcad services found, each with its own data root: ${input.files.map((f) => f.path).join(', ')}` + } + } + return { + code: 'multiple_services_one_root', + severity: 'critical', + message: + `More than one orcad service targets the same data root — ${contended + .map(([root, paths]) => `${root}: ${paths.join(' and ')}`) + .join('; ')}. Only one can hold the instance lock; the other exits 78 and stays ` + + 'permanently failed, so its start failure is caused by the service that already won.', + remedy: 'Remove the service you did not intend to keep, then: systemctl reset-failed' + } +} + +/** + * A definition that exists but cannot be read. + * + * Kept distinct from `no_service_found` because the two demand opposite actions: absence + * says install one, unreadable says you already did and this process cannot see it. Telling + * an operator to run `--print-service` against an installed unit invites them to redo work + * that succeeded, and the reinstall does not fix the permission either. + */ +function auditUnreadable(unreadable: { path: string; reason: string }[]): SupervisorFinding { + const named = unreadable.map((u) => `${u.path} (${u.reason})`).join('; ') + return { + code: 'service_definition_unreadable', + severity: 'unverifiable', + message: + `A service definition is present but could not be read — ${named}. It exists, so ` + + 'this is not a missing install; nothing here can say whether its contents are ' + + 'correct.', + remedy: `Re-run as root, or make it readable: sudo chmod 644 ${unreadable[0].path}` + } +} + +export function auditSupervisorServices(input: SupervisorAuditInput): SupervisorFinding[] { + const unreadable = input.unreadable ?? [] + if (input.files.length === 0) { + return [ + unreadable.length > 0 + ? auditUnreadable(unreadable) + : { + code: 'no_service_found', + severity: 'unverifiable', + message: 'No orcad service definition found in the conventional locations.', + remedy: 'orcad --print-service' + } + ] + } + let findings: SupervisorFinding[] = [] + // Reported even when other definitions read fine: an unreadable second file is exactly + // the duplicate that `auditDuplicates` exists to catch, and it cannot see this one. + if (unreadable.length > 0) { + findings.push(auditUnreadable(unreadable)) + } + const duplicates = auditDuplicates(input) + if (duplicates) { + findings.push(duplicates) + } + for (const file of input.files) { + findings.push(auditKillSemantics(file)) + findings.push(auditUserDataPath(file, input.expectedUserDataPath)) + findings.push(auditRunAsUser(file)) + const logout = auditSurvivesLogout(file) + if (logout) { + findings.push(logout) + } + const exec = auditExecTarget(file) + if (exec) { + findings.push(exec) + } + } + if (input.evidence) { + const live = auditSupervisorEvidence(input.evidence) + // A live answer replaces the file-level finding that could only call it unreadable. + const superseded = new Set(supersededFileFindingCodes(live)) + findings = findings.filter((finding) => !superseded.has(finding.code)) + findings.push(...live) + } + return sortBySeverity(findings) +} + +/** + * Exported so a caller appending its own finding re-sorts rather than leaving it past the + * OK rows, where an operator scanning top-down stops reading before reaching it. + */ +export function sortBySeverity(findings: SupervisorFinding[]): SupervisorFinding[] { + const rank: Record = { + critical: 0, + warning: 1, + unverifiable: 2, + ok: 3 + } + return [...findings].sort((a, b) => rank[a.severity] - rank[b.severity]) +} + +/** True when nothing found would lose an operator's running terminals. */ +export function supervisorAuditPassed(findings: SupervisorFinding[]): boolean { + return !findings.some((finding) => finding.severity === 'critical') +} diff --git a/src/shared/supervisor-service-file-read.test.ts b/src/shared/supervisor-service-file-read.test.ts new file mode 100644 index 000000000000..5b91afd0af7d --- /dev/null +++ b/src/shared/supervisor-service-file-read.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it } from 'vitest' +import { renderSupervisorService } from './supervisor-service-render' +import { + readConfiguredEndpoint, + readExecTarget, + readPinnedUserData, + splitSystemdCommandLine, + type SupervisorServiceFile +} from './supervisor-service-file-read' + +function unit(overrides: { userDataPath?: string; nodePath?: string; orcadPath?: string } = {}) { + const text = renderSupervisorService({ + platform: 'systemd', + scope: 'system', + nodePath: overrides.nodePath ?? '/usr/local/bin/node', + orcadPath: overrides.orcadPath ?? '/opt/orcad/orcad.js', + userDataPath: overrides.userDataPath ?? '/home/orca/.orca', + user: 'orca', + bind: '127.0.0.1', + port: 6800 + }) + return { + path: '/etc/systemd/system/orcad.service', + text, + platform: 'systemd', + scope: 'system' + } satisfies SupervisorServiceFile +} + +// The generator quotes any value carrying a space, so a reader that splits on whitespace +// misreads the file this tool itself wrote — and then reports a healthy unit as pointing at +// a binary that is not there. +describe('reading back what the generator wrote', () => { + const spaced = { + userDataPath: '/Volumes/My Disk/.orca', + nodePath: '/opt/my node/bin/node', + orcadPath: '/opt/my orcad/orcad.js' + } + + it('reads a spaced data root as one path', () => { + expect(readPinnedUserData(unit(spaced))).toBe('/Volumes/My Disk/.orca') + }) + + it('reads a spaced interpreter and script as two words, not four', () => { + expect(readExecTarget(unit(spaced))).toEqual({ + interpreter: '/opt/my node/bin/node', + script: '/opt/my orcad/orcad.js' + }) + }) + + it('still finds the endpoint past a quoted interpreter', () => { + expect(readConfiguredEndpoint(unit(spaced))).toEqual({ bind: '127.0.0.1', port: 6800 }) + }) + + it('leaves an ordinary unit reading exactly as before', () => { + expect(readPinnedUserData(unit())).toBe('/home/orca/.orca') + expect(readExecTarget(unit())).toEqual({ + interpreter: '/usr/local/bin/node', + script: '/opt/orcad/orcad.js' + }) + }) +}) + +describe('the systemd command-line splitter', () => { + it('splits on whitespace outside quotes', () => { + expect(splitSystemdCommandLine('/bin/node /a/b.js --port 6800')).toEqual([ + '/bin/node', + '/a/b.js', + '--port', + '6800' + ]) + }) + + it('keeps a quoted run together and drops the quotes', () => { + expect(splitSystemdCommandLine('"/opt/my node/bin/node" --json')).toEqual([ + '/opt/my node/bin/node', + '--json' + ]) + }) + + it('unescapes a quote and a backslash inside double quotes', () => { + expect(splitSystemdCommandLine('"/a/with\\"quote" "/b/with\\\\slash"')).toEqual([ + '/a/with"quote', + '/b/with\\slash' + ]) + }) + + it('yields an empty word for an empty quoted argument rather than dropping it', () => { + expect(splitSystemdCommandLine('/bin/node "" --json')).toEqual(['/bin/node', '', '--json']) + }) + + it('answers empty for an empty command line', () => { + expect(splitSystemdCommandLine('')).toEqual([]) + expect(splitSystemdCommandLine(' ')).toEqual([]) + }) +}) diff --git a/src/shared/supervisor-service-file-read.ts b/src/shared/supervisor-service-file-read.ts new file mode 100644 index 000000000000..f4df5444d72b --- /dev/null +++ b/src/shared/supervisor-service-file-read.ts @@ -0,0 +1,166 @@ +/** + * Reading values back out of an installed supervisor service definition. + * + * Deliberately not an XML or INI parser: the audit must work on a hand-edited file that a + * strict parser would reject outright, since a hand-edited file is exactly the one most + * likely to be wrong. + */ +import type { SupervisorPlatform, SupervisorScope } from './supervisor-service-render' + +export type SupervisorServiceFile = { + path: string + text: string + platform: SupervisorPlatform + scope: SupervisorScope +} + +/** systemd `Key=value`, ignoring comments. Last assignment wins, as systemd itself does. */ +export function readSystemdKey(text: string, key: string): string | null { + let found: string | null = null + for (const raw of text.split('\n')) { + const line = raw.trim() + if (line.startsWith('#') || line.startsWith(';')) { + continue + } + const match = new RegExp(`^${key}\\s*=\\s*(.*)$`).exec(line) + if (match) { + found = match[1].trim() + } + } + return found +} + +/** + * `Name` followed by its value element. Deliberately not an XML parser: the + * audit must work on a hand-edited plist that a strict parser would reject outright. + */ +export function readPlistBoolean(text: string, key: string): boolean | null { + const match = new RegExp(`\\s*${key}\\s*\\s*<(true|false)\\s*/>`, 'i').exec(text) + return match ? match[1].toLowerCase() === 'true' : null +} + +export function readPlistString(text: string, key: string): string | null { + const match = new RegExp(`\\s*${key}\\s*\\s*([^<]*)`, 'i').exec(text) + return match ? match[1].trim() : null +} + +/** + * Splits a systemd command line the way systemd does, rather than on whitespace. + * + * The generator quotes any path containing a space, so a naive split hands back `"/opt/my` + * as the interpreter and the caller then stats a path that was never named — reporting a + * unit orcad itself wrote as pointing at a missing binary. Inside double quotes systemd + * applies C-style escapes, so one round of those is undone here too. + */ +export function splitSystemdCommandLine(command: string): string[] { + const words: string[] = [] + let current = '' + let quote: '"' | "'" | null = null + let started = false + for (let i = 0; i < command.length; i += 1) { + const char = command[i] + if (char === '\\' && quote !== "'" && i + 1 < command.length) { + current += command[i + 1] + i += 1 + started = true + } else if (quote === null && (char === '"' || char === "'")) { + quote = char + started = true + } else if (char === quote) { + quote = null + } else if (quote === null && /\s/.test(char)) { + if (started) { + words.push(current) + } + current = '' + started = false + } else { + current += char + started = true + } + } + if (started) { + words.push(current) + } + return words +} + +/** systemd writes `Environment=NAME=value`; the plist nests it under EnvironmentVariables. */ +export function readPinnedUserData(file: SupervisorServiceFile): string | null { + if (file.platform === 'systemd') { + for (const raw of file.text.split('\n')) { + const line = raw.trim() + if (line.startsWith('#')) { + continue + } + const match = /^Environment\s*=\s*(.*)$/.exec(line) + if (!match) { + continue + } + // Tokenized rather than regexed: one Environment line may carry several assignments, + // and the generator quotes this one whenever the root contains a space. + for (const assignment of splitSystemdCommandLine(match[1])) { + if (assignment.startsWith('ORCA_USER_DATA=')) { + return assignment.slice('ORCA_USER_DATA='.length).trim() + } + } + } + return null + } + const dict = /\s*EnvironmentVariables\s*<\/key>\s*([\s\S]*?)<\/dict>/i.exec(file.text) + return dict ? readPlistString(dict[1], 'ORCA_USER_DATA') : null +} + +/** + * The endpoint the service will actually try to bind, read from the file rather than from + * the caller's flags — probing a default port while the file names another one reports the + * wrong answer with full confidence. + */ +export function readConfiguredEndpoint( + file: SupervisorServiceFile +): { bind: string; port: number } | null { + // Why scoped to ProgramArguments and not every in the plist: the data root and + // log path are strings too, and joining them all lets an unrelated value supply the port. + const programArguments = /\s*ProgramArguments\s*<\/key>\s*([\s\S]*?)<\/array>/i.exec( + file.text + )?.[1] + const command = + file.platform === 'systemd' + ? (readSystemdKey(file.text, 'ExecStart') ?? '') + : [...(programArguments ?? '').matchAll(/([^<]*)<\/string>/g)] + .map((match) => match[1]) + .join(' ') + const port = Number(/--port[\s=]+(\d+)/.exec(command)?.[1]) + if (!Number.isInteger(port)) { + return null + } + return { bind: /--bind[\s=]+(\S+)/.exec(command)?.[1] ?? '127.0.0.1', port } +} + +/** + * The interpreter and script an installed definition will try to exec. + * + * Split out from the endpoint read because the caller stats these: a unit can be perfectly + * well-formed and name a path that no longer exists, which is what a version-scoped + * interpreter becomes one package-manager upgrade later. + */ +export function readExecTarget( + file: SupervisorServiceFile +): { interpreter: string; script: string | null } | null { + const words = + file.platform === 'systemd' + ? splitSystemdCommandLine(readSystemdKey(file.text, 'ExecStart') ?? '') + : [ + ...( + /\s*ProgramArguments\s*<\/key>\s*([\s\S]*?)<\/array>/i.exec( + file.text + )?.[1] ?? '' + ).matchAll(/([^<]*)<\/string>/g) + ].map((match) => match[1]) + // systemd allows `-`, `@`, `+`, `!` prefixes on ExecStart; strip them off the binary. + const interpreter = words[0]?.replace(/^[-@+!:]+/, '') ?? '' + if (!interpreter) { + return null + } + return { interpreter, script: words.slice(1).find((word) => !word.startsWith('-')) ?? null } +} diff --git a/src/shared/supervisor-service-probe.ts b/src/shared/supervisor-service-probe.ts new file mode 100644 index 000000000000..0a5ef5be0d60 --- /dev/null +++ b/src/shared/supervisor-service-probe.ts @@ -0,0 +1,238 @@ +/** + * Gathers live evidence about an installed orcad service. + * + * The whole point of the split: a probe never produces a finding, only an observation. + * `auditSupervisorServices` stays pure and stays assertable against fixtures, and no probe + * can smuggle a verdict past it. Anything a probe cannot establish comes back + * `unavailable` with a reason — never as a negative, because a refused, missing or + * timed-out probe is not evidence that a service is stopped. + */ +import { createConnection } from 'node:net' +import { runProcess } from './child-process/run-process' +import type { SupervisorPlatform, SupervisorScope } from './supervisor-service-render' + +/** Bounded well under runProcess's 30s default: a doctor that hangs is one nobody re-runs. */ +const PROBE_TIMEOUT_MS = 5_000 +const PORT_PROBE_TIMEOUT_MS = 1_500 + +export type Probe = + | { status: 'observed'; value: T } + /** Shown to the operator: "unverified" with no cause reads as a broken tool. */ + | { status: 'unavailable'; reason: string } + +export type UnitState = { + /** + * Whether the supervisor knows about the unit at all. Load-bearing because + * `systemctl show` exits 0 for a unit it has never heard of and reports + * `ActiveState=inactive` — indistinguishable from a service someone stopped, and the + * likeliest state in a workflow that prints a file for an operator to place by hand. + */ + load: string + active: string + sub: string + result: string + restarts: number +} + +export type ExecTargetState = { + interpreter: string + interpreterExists: boolean + script: string | null + scriptExists: boolean +} + +export type JournalState = { + /** journald's `Storage=`. `volatile` means the journal lives in /run and dies on reboot. */ + storage: string + /** Only meaningful when the audited unit actually logs to the journal. */ + unitUsesJournal: boolean +} + +export type SupervisorEvidence = { + unitState?: Probe + linger?: Probe + /** A listener on the configured port. Not proof it is orcad — see the audit's wording. */ + configuredPortListening?: Probe + /** A stat, not a subprocess, so `--no-probe` does not suppress it. */ + execTarget?: Probe + /** Read from journald.conf, likewise a file read rather than a probe. */ + journal?: Probe + /** + * Whether the pinned data root and the caller's resolve to the same directory. A string + * comparison cannot answer this: once the generator started pinning a realpath, a host + * whose root sits behind a symlink reports two spellings of one directory. + */ + dataRootSameDirectory?: Probe +} + +/** What to ask about, derived from the file discovery rather than from a constant. */ +export type ProbeTarget = { + platform: SupervisorPlatform + scope: SupervisorScope + /** Unit filename or launchd label, taken from the discovered file's own basename. */ + name: string + user: string + bind: string + port: number +} + +function unavailable(reason: string): Probe { + return { status: 'unavailable', reason } +} + +/** + * Every failure mode of a probe collapses to one shape here. A timeout, a missing binary + * and a permission refusal are all "could not establish", and the reason carries which. + */ +async function capture( + program: string, + args: string[], + timeoutMs = PROBE_TIMEOUT_MS +): Promise<{ ok: true; stdout: string } | { ok: false; reason: string }> { + try { + const result = await runProcess({ program, args, timeoutMs }) + if (result.timedOut) { + return { ok: false, reason: `${program} did not respond within ${timeoutMs}ms` } + } + if (result.code !== 0) { + const detail = (result.stderr || result.stdout).trim().split('\n')[0] + return { + ok: false, + reason: detail ? `${program}: ${detail}` : `${program} exited ${result.code}` + } + } + return { ok: true, stdout: result.stdout } + } catch (error) { + return { + ok: false, + reason: `${program} could not be run: ${error instanceof Error ? error.message : String(error)}` + } + } +} + +/** `KEY=value` lines, as `systemctl show` and `loginctl show-user` both emit. */ +function parseShowOutput(stdout: string): Map { + const values = new Map() + for (const line of stdout.split('\n')) { + const separator = line.indexOf('=') + if (separator > 0) { + values.set(line.slice(0, separator).trim(), line.slice(separator + 1).trim()) + } + } + return values +} + +async function probeSystemdUnit(target: ProbeTarget): Promise> { + const scopeArgs = target.scope === 'user' ? ['--user'] : [] + const captured = await capture('systemctl', [ + ...scopeArgs, + 'show', + target.name, + '--property=LoadState,ActiveState,SubState,Result,NRestarts' + ]) + if (!captured.ok) { + // A user-scope query without a session bus fails exactly like an absent unit, so the + // reason has to travel with the verdict rather than be flattened into "not running". + return unavailable(captured.reason) + } + const values = parseShowOutput(captured.stdout) + const active = values.get('ActiveState') + if (!active) { + return unavailable('systemctl returned no ActiveState') + } + return { + status: 'observed', + value: { + load: values.get('LoadState') ?? 'unknown', + active, + sub: values.get('SubState') ?? 'unknown', + result: values.get('Result') ?? 'unknown', + restarts: Number(values.get('NRestarts') ?? '0') || 0 + } + } +} + +async function probeLaunchdJob(target: ProbeTarget): Promise> { + const uid = process.getuid?.() + if (target.scope === 'user' && uid === undefined) { + // `gui/` with no uid is not a domain launchctl accepts; asking anyway would return a + // parse error that reads like the job being absent. + return unavailable('cannot address the gui domain: this platform reports no uid') + } + const domain = target.scope === 'system' ? 'system' : `gui/${uid}` + const captured = await capture('launchctl', ['print', `${domain}/${target.name}`]) + if (!captured.ok) { + // The system domain refuses without root; that is inability, not absence. + return unavailable(captured.reason) + } + const state = /\bstate\s*=\s*(\w+)/.exec(captured.stdout)?.[1] + if (!state) { + return unavailable('launchctl print returned no state') + } + return { + status: 'observed', + value: { + // launchctl print answering at all means the job is loaded. + load: 'loaded', + active: state === 'running' ? 'active' : state, + sub: state, + result: /\blast exit code\s*=\s*(\d+)/.exec(captured.stdout)?.[1] ?? 'unknown', + restarts: 0 + } + } +} + +/** Only systemd has lingering; launchd's equivalent question is Agent-versus-Daemon. */ +async function probeLinger(target: ProbeTarget): Promise | undefined> { + if (target.platform !== 'systemd' || target.scope !== 'user') { + return undefined + } + const captured = await capture('loginctl', ['show-user', target.user, '--property=Linger']) + if (!captured.ok) { + return unavailable(captured.reason) + } + const linger = parseShowOutput(captured.stdout).get('Linger') + return linger === undefined + ? unavailable('loginctl returned no Linger property') + : { status: 'observed', value: linger === 'yes' } +} + +/** + * Why a TCP connect and not the readiness line from the journal: reading the journal needs + * group membership, can be rotated away, and returns nothing from a service that has not + * logged recently. A connect answers the question that matters — is anything serving where + * the file says it should be — with no privilege and nothing to parse. + */ +export function probePort(host: string, port: number): Promise> { + const target = host === '0.0.0.0' || host === '::' ? '127.0.0.1' : host + return new Promise((resolve) => { + const socket = createConnection({ host: target, port }) + const settle = (probe: Probe): void => { + socket.destroy() + resolve(probe) + } + socket.setTimeout(PORT_PROBE_TIMEOUT_MS) + socket.once('connect', () => settle({ status: 'observed', value: true })) + socket.once('timeout', () => + settle(unavailable(`no answer from ${target}:${port} within ${PORT_PROBE_TIMEOUT_MS}ms`)) + ) + socket.once('error', (error: NodeJS.ErrnoException) => { + // Only a refused connection proves nothing is listening. Anything else — no route, + // a firewall, an exhausted fd table — is inability to tell. + settle( + error.code === 'ECONNREFUSED' + ? { status: 'observed', value: false } + : unavailable(`${target}:${port}: ${error.code ?? error.message}`) + ) + }) + }) +} + +export async function gatherSupervisorEvidence(target: ProbeTarget): Promise { + const [unitState, linger, configuredPortListening] = await Promise.all([ + target.platform === 'systemd' ? probeSystemdUnit(target) : probeLaunchdJob(target), + probeLinger(target), + probePort(target.bind, target.port) + ]) + return { unitState, linger, configuredPortListening } +} diff --git a/src/shared/supervisor-service-render.test.ts b/src/shared/supervisor-service-render.test.ts new file mode 100644 index 000000000000..19f3fd3b951a --- /dev/null +++ b/src/shared/supervisor-service-render.test.ts @@ -0,0 +1,210 @@ +import { describe, expect, it } from 'vitest' +import { + renderSupervisorService, + resolveSupervisorPlatform, + SAFE_SYSTEMD_KILL_MODES, + supervisorInstallHint, + SupervisorServiceUnsupportedError, + systemdQuote, + type SupervisorServiceConfig +} from './supervisor-service-render' + +function config(overrides: Partial = {}): SupervisorServiceConfig { + return { + platform: 'systemd', + scope: 'system', + nodePath: '/usr/local/bin/node', + orcadPath: '/opt/orcad/orcad.js', + userDataPath: '/home/orca/.orca', + user: 'orca', + bind: '127.0.0.1', + port: 6800, + ...overrides + } +} + +describe('the invariant', () => { + // This is the feature. Everything else in this file is formatting. + it('never renders a systemd unit whose kill mode reaps the terminal daemon', () => { + for (const scope of ['user', 'system'] as const) { + const unit = renderSupervisorService(config({ scope })) + const killMode = /^KillMode\s*=\s*(.*)$/m.exec(unit)?.[1] + expect(killMode, `scope=${scope} rendered no KillMode`).toBeDefined() + expect(SAFE_SYSTEMD_KILL_MODES).toContain(killMode) + } + }) + + it('never renders a launchd plist that lets launchd reap the job process group', () => { + for (const scope of ['user', 'system'] as const) { + const plist = renderSupervisorService(config({ platform: 'launchd', scope })) + expect(plist).toMatch(/AbandonProcessGroup<\/key>\s*/) + } + }) +}) + +describe('data root', () => { + it('pins an absolute root into both formats', () => { + expect(renderSupervisorService(config())).toContain( + 'Environment=ORCA_USER_DATA=/home/orca/.orca' + ) + expect(renderSupervisorService(config({ platform: 'launchd' }))).toContain( + '/home/orca/.orca' + ) + }) +}) + +describe('paths with spaces', () => { + // systemd splits these values on whitespace, so an unquoted space does not fail loudly — + // it pins a truncated root and the service comes up healthy and empty, which is the exact + // failure pinning the root exists to prevent. + const spaced = config({ + userDataPath: '/Volumes/My Disk/.orca', + nodePath: '/opt/my node/bin/node', + orcadPath: '/opt/my orcad/orcad.js' + }) + + it('quotes the pinned data root so systemd reads one path, not three', () => { + const unit = renderSupervisorService(spaced) + expect(unit).toContain('Environment="ORCA_USER_DATA=/Volumes/My Disk/.orca"') + expect(unit).toContain('RequiresMountsFor="/Volumes/My Disk/.orca"') + }) + + it('quotes each ExecStart word, so a spaced interpreter is not two arguments', () => { + const execStart = /^ExecStart=(.*)$/m.exec(renderSupervisorService(spaced))?.[1] + expect(execStart).toBe( + '"/opt/my node/bin/node" "/opt/my orcad/orcad.js" --bind 127.0.0.1 --port 6800 --json' + ) + }) + + it('leaves ordinary paths unquoted, so the common case stays readable', () => { + const unit = renderSupervisorService(config()) + expect(unit).toContain('Environment=ORCA_USER_DATA=/home/orca/.orca') + expect(unit).toContain('ExecStart=/usr/local/bin/node /opt/orcad/orcad.js ') + }) + + it('escapes a quote and a backslash, which systemd unescapes inside double quotes', () => { + expect(systemdQuote('/a path/with"quote')).toBe('"/a path/with\\"quote"') + expect(systemdQuote('/a path/with\\slash')).toBe('"/a path/with\\\\slash"') + }) +}) + +describe('run-as account', () => { + it('refuses to render a service that runs orcad as root', () => { + expect(() => renderSupervisorService(config({ user: 'root' }))).toThrow(/root-owned data root/) + expect(() => renderSupervisorService(config({ platform: 'launchd', user: 'root' }))).toThrow( + /root-owned data root/ + ) + }) + + it('names the account explicitly', () => { + expect(renderSupervisorService(config())).toContain('User=orca') + expect(renderSupervisorService(config({ platform: 'launchd' }))).toContain( + 'orca' + ) + }) +}) + +describe('readiness honesty', () => { + // Under Type=simple systemd considers the unit started at fork, so a start timeout gates + // nothing. Emitting one anyway would read like a readiness guarantee that does not exist. + it('emits no TimeoutStartSec', () => { + expect(renderSupervisorService(config())).not.toMatch(/^TimeoutStartSec/m) + }) + + it('prevents restart only on the configuration exit code', () => { + expect(renderSupervisorService(config())).toContain('RestartPreventExitStatus=78') + }) +}) + +describe('logging', () => { + it('uses journald on systemd rather than an unrotated file', () => { + const unit = renderSupervisorService(config()) + expect(unit).toContain('StandardOutput=journal') + expect(unit).not.toMatch(/StandardOutput=append:/) + }) +}) + +describe('escaping', () => { + it('escapes plist-hostile characters in generated paths', () => { + const plist = renderSupervisorService( + config({ platform: 'launchd', userDataPath: '/home/a&b/' }) + ) + expect(plist).toContain('/home/a&b/<orca>') + expect(plist).not.toContain('/home/a&b/') + }) +}) + +describe('scope', () => { + it('targets default.target for a user unit and multi-user.target for a system one', () => { + expect(renderSupervisorService(config({ scope: 'user' }))).toContain('WantedBy=default.target') + expect(renderSupervisorService(config({ scope: 'system' }))).toContain( + 'WantedBy=multi-user.target' + ) + }) + + it('warns in the plist that a LaunchAgent stops at logout', () => { + const agent = renderSupervisorService(config({ platform: 'launchd', scope: 'user' })) + expect(agent).toMatch(/only while this user is logged in/i) + }) + + it('tells a user-scope systemd install to enable lingering', () => { + const hint = supervisorInstallHint(config({ scope: 'user' })) + expect(hint.commands.join('\n')).toContain('enable-linger') + }) + + // `--now` arrived in systemd 220. The hint is the operator's next action, so naming a + // flag their systemd does not have leaves the unit installed but neither enabled nor + // started — indistinguishable from a successful install until the next reboot. + // Measured on Synology DSM (systemd 219): `unrecognized option '--now'`. + it.each(['system', 'user'] as const)( + 'never tells a %s systemd install to use `--now`', + (scope) => { + const commands = supervisorInstallHint(config({ scope })).commands.join('\n') + expect(commands).not.toContain('--now') + expect(commands).toContain('systemctl') + } + ) + + it.each(['system', 'user'] as const)( + 'enables AND starts a %s systemd unit, since neither alone is enough', + (scope) => { + const commands = supervisorInstallHint(config({ scope })).commands + // enable without start leaves it dead until reboot; start without enable dies at one. + expect(commands.some((c) => /systemctl.* enable /.test(c))).toBe(true) + expect(commands.some((c) => /systemctl.* start /.test(c))).toBe(true) + } + ) +}) + +describe('platform resolution', () => { + it('maps the two supported platforms', () => { + expect(resolveSupervisorPlatform('linux')).toBe('systemd') + expect(resolveSupervisorPlatform('darwin')).toBe('launchd') + }) + + it('refuses win32 rather than emitting something plausible', () => { + expect(() => resolveSupervisorPlatform('win32')).toThrow(SupervisorServiceUnsupportedError) + }) +}) + +describe('mount ordering', () => { + // systemd maps RequiresMountsFor to a mount unit textually, walking up parent + // directories, so it only reaches a real mount when the caller passed a realpath. + it('orders the unit after the mount carrying the data root', () => { + const unit = renderSupervisorService(config({ userDataPath: '/volume2/homes/me/.orca' })) + expect(unit).toContain('RequiresMountsFor=/volume2/homes/me/.orca') + }) + + it('puts it in [Unit], where systemd reads it', () => { + const unit = renderSupervisorService(config()) + const unitSection = unit.slice(unit.indexOf('[Unit]'), unit.indexOf('[Service]')) + expect(unitSection).toMatch(/^RequiresMountsFor=/m) + }) + + it('names the same path the service will actually use', () => { + const unit = renderSupervisorService(config({ userDataPath: '/volume2/homes/me/.orca' })) + const mount = /^RequiresMountsFor=(.*)$/m.exec(unit)?.[1] + const environment = /^Environment=ORCA_USER_DATA=(.*)$/m.exec(unit)?.[1] + expect(mount).toBe(environment) + }) +}) diff --git a/src/shared/supervisor-service-render.ts b/src/shared/supervisor-service-render.ts new file mode 100644 index 000000000000..85ac1f673bad --- /dev/null +++ b/src/shared/supervisor-service-render.ts @@ -0,0 +1,301 @@ +/** + * Renders the supervisor service definition for an orcad deployment. + * + * The whole reason this is generated rather than documented: orcad forks the terminal + * daemon detached so PTYs outlive the runtime, and a supervisor that reaps the process + * group destroys every running terminal on restart. `docs/reference/orcad-operations.md` + * states that rule in prose, and every packaged path still gets it wrong — Homebrew's + * generated systemd unit sets no `KillMode` at all, so systemd's `control-group` default + * applies and `setsid` does not escape a cgroup. + * + * Pure by contract: config in, file text out, no I/O. The CLI, orcad itself and (later) + * the SSH deploy path all consume these, and purity is what makes the invariants + * assertable in a test rather than only on a host. + */ + +/** launchd has no `KillMode`; it reaps the job's process group unless told not to. */ +export type SupervisorPlatform = 'systemd' | 'launchd' + +/** A user-scope service stops at logout unless lingering is enabled — see the audit. */ +export type SupervisorScope = 'user' | 'system' + +export type SupervisorServiceConfig = { + platform: SupervisorPlatform + scope: SupervisorScope + /** Absolute path to the Node interpreter the supervisor should run, not the one that rendered this. */ + nodePath: string + /** Absolute path to `orcad.js`. */ + orcadPath: string + /** + * Resolved at generation time; a unit inherits too little environment to resolve it + * itself. Must be a realpath: `RequiresMountsFor` below cannot see through a symlink. + */ + userDataPath: string + /** Account the service runs as. Never root: orcad would create a root-owned data root. */ + user: string + bind: string + port: number + /** Only used by launchd, which cannot log to journald. */ + logPath?: string +} + +export const ORCAD_LAUNCHD_LABEL = 'dev.onorca.orcad' +export const ORCAD_SYSTEMD_UNIT_NAME = 'orcad.service' + +/** + * Every value systemd's `KillMode` may take without reaping the detached daemon. + * + * `mixed` was here and is not safe. It SIGTERMs the main process and then SIGKILLs every + * process still in the control group, which is where the detached daemon lives — the + * escape it needs is from the cgroup, and setsid does not provide one. Measured on + * systemd 219: a restart under mixed destroyed the daemon, its shell and its scrollback; + * the identical unit under `process` kept all three. See `auditKillSemantics`, which now + * names mixed specifically because orcad itself generated it. + */ +export const SAFE_SYSTEMD_KILL_MODES = ['process', 'none'] as const + +/** + * `none` spares the daemon but signals nothing at all, not even the main process, so the + * runtime never gets its graceful stop and systemd deprecates it. Safe for this invariant, + * wrong for everything else — the audit reports it rather than blessing it. + */ +export const DISCOURAGED_SYSTEMD_KILL_MODES = ['none'] as const + +export class SupervisorServiceUnsupportedError extends Error { + readonly code = 'supervisor_service_unsupported_platform' + constructor(platform: NodeJS.Platform) { + super( + `Generating a service definition for ${platform} is not implemented. There is no ` + + 'systemd or launchd to target; a Windows Service needs its own launch and liveness ' + + 'contract, the same one that makes orcad remote deploy POSIX-only today.' + ) + this.name = 'SupervisorServiceUnsupportedError' + } +} + +export function resolveSupervisorPlatform(platform: NodeJS.Platform): SupervisorPlatform { + if (platform === 'darwin') { + return 'launchd' + } + if (platform === 'linux') { + return 'systemd' + } + throw new SupervisorServiceUnsupportedError(platform) +} + +/** + * Why refuse rather than render: a unit that runs orcad as root creates a root-owned data + * root, and every later user-scope run then refuses with `orcad_data_root_wrong_owner` — + * a state orcad deliberately will not repair, because the permissions are not its to fix. + */ +function assertNotRoot(user: string): void { + if (user === 'root') { + throw new Error( + 'refusing to generate a service that runs orcad as root: orcad would create a ' + + 'root-owned data root, and every later run as a normal user would then fail with ' + + 'orcad_data_root_wrong_owner. Pass the account orcad should run as.' + ) + } +} + +function orcadArgs(config: SupervisorServiceConfig): string[] { + return [config.orcadPath, '--bind', config.bind, '--port', String(config.port), '--json'] +} + +/** + * systemd splits every directive value below on whitespace, so an unquoted path containing a + * space does not fail — it silently becomes something else. `Environment=` pins a truncated + * root and the service comes up healthy and empty, which is the exact failure pinning the + * root exists to prevent; `RequiresMountsFor=` takes a list, so the tail becomes extra paths + * to order against; `ExecStart=` gains arguments nobody passed. launchd needs none of this: + * every value there is its own `` element. + * + * Quoted only when it has to be, so the common case stays a readable path in the file. + * Inside double quotes systemd applies C-style escapes, so a backslash or quote in the value + * has to survive one round of that. + */ +export function systemdQuote(value: string): string { + return /[\s"'\\]/.test(value) ? `"${value.replace(/["\\]/g, (char) => `\\${char}`)}"` : value +} + +function renderSystemd(config: SupervisorServiceConfig): string { + const execStart = [config.nodePath, ...orcadArgs(config)].map(systemdQuote).join(' ') + return `# Generated by orcad. Regenerate rather than hand-edit: +# orcad --print-service --scope ${config.scope} +[Unit] +Description=Orca headless runtime (orcad) +After=network-online.target +# Orders this unit after whatever mount carries the data root. It only works on a +# fully-resolved path: systemd maps this to a mount unit textually, walking up parent +# directories, so a symlinked ancestor (DSM's /var/services/homes -> /volume2/homes, +# created during boot) resolves to the root filesystem and orders against nothing. +# The caller passes a realpath for exactly this reason. +RequiresMountsFor=${systemdQuote(config.userDataPath)} + +[Service] +Type=simple +# No sd_notify: readiness is one JSON line on stdout, so TimeoutStartSec would gate +# nothing. It is omitted rather than set to a number that reads like a guarantee. + +# Interpreter fixed at generation time; override with --node when the service should +# use a different one than the shell that generated this. +ExecStart=${execStart} +# A pinned --port still falls back to an OS-assigned port on conflict, so confirm the +# bound endpoint before relying on an SSH forward to it. + +User=${config.user} +# Resolved at generation time. Without this the unit's near-empty environment resolves +# a different data root than your shell, and the service starts healthy and empty. +Environment=${systemdQuote(`ORCA_USER_DATA=${config.userDataPath}`)} + +# The terminal daemon is forked detached so PTYs outlive the runtime. systemd's default +# control-group would reap it and make every restart destroy running work. +# +# process, not mixed. Measured on a Synology NAS running systemd 219: under mixed, a +# systemctl restart killed the daemon, its shell, the session and its scrollback. mixed +# SIGTERMs the main process and then SIGKILLs everything STILL IN THE CGROUP, and the +# daemon is in the cgroup -- setsid does not leave one. Only process signals the main +# process alone. +# +# The cost is real and accepted: systemd no longer reaps what the runtime leaves behind. +# orcad already owns that lifecycle through the daemon pid-record it adopts on the next +# start, and a supervisor that helpfully cleans up is exactly what breaks this. +KillMode=process + +# 78 is EX_CONFIG — bad bind address, unusable or foreign data root. Restarting cannot +# fix it. A transient cause (an orcad you ran by hand) strands the unit here too; clear +# it with: systemctl reset-failed ${ORCAD_SYSTEMD_UNIT_NAME} +RestartPreventExitStatus=78 +Restart=on-failure +RestartSec=5 + +# journald rather than a file: orcad rotates no logs of its own. +StandardOutput=journal +StandardError=journal + +# No sandboxing directives, deliberately. orcad exists to run arbitrary user commands +# across arbitrary paths, and PrivateTmp in particular would give terminals a /tmp that +# nothing else on the host can see. + +[Install] +WantedBy=${config.scope === 'user' ? 'default.target' : 'multi-user.target'} +` +} + +/** Why escape: a data root or install path may contain `&` or `<`, which would break the plist. */ +function xmlEscape(value: string): string { + return value + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') +} + +function renderLaunchd(config: SupervisorServiceConfig): string { + const args = [config.nodePath, ...orcadArgs(config)] + .map((arg) => ` ${xmlEscape(arg)}`) + .join('\n') + const logPath = config.logPath ?? '/var/log/orcad.log' + const scopeNote = + config.scope === 'system' + ? ' LaunchDaemon: runs at boot, independent of any login session.' + : ' LaunchAgent: runs ONLY while this user is logged in. A headless Mac needs\n --scope system instead.' + return ` + + + + + Label + ${ORCAD_LAUNCHD_LABEL} + + ProgramArguments + +${args} + + + UserName + ${xmlEscape(config.user)} + + RunAtLoad + + KeepAlive + + + + AbandonProcessGroup + + + + EnvironmentVariables + + ORCA_USER_DATA + ${xmlEscape(config.userDataPath)} + + + + StandardOutPath + ${xmlEscape(logPath)} + StandardErrorPath + ${xmlEscape(logPath)} + + +` +} + +export function renderSupervisorService(config: SupervisorServiceConfig): string { + assertNotRoot(config.user) + return config.platform === 'systemd' ? renderSystemd(config) : renderLaunchd(config) +} + +/** Where the rendered file goes, and what to run after placing it. */ +export function supervisorInstallHint(config: SupervisorServiceConfig): { + path: string + commands: string[] +} { + if (config.platform === 'launchd') { + return config.scope === 'system' + ? { + path: `/Library/LaunchDaemons/${ORCAD_LAUNCHD_LABEL}.plist`, + commands: [ + `sudo launchctl bootstrap system /Library/LaunchDaemons/${ORCAD_LAUNCHD_LABEL}.plist` + ] + } + : { + path: `~/Library/LaunchAgents/${ORCAD_LAUNCHD_LABEL}.plist`, + commands: [ + `launchctl bootstrap gui/$(id -u) ~/Library/LaunchAgents/${ORCAD_LAUNCHD_LABEL}.plist` + ] + } + } + return config.scope === 'system' + ? { + path: `/etc/systemd/system/${ORCAD_SYSTEMD_UNIT_NAME}`, + // Why two commands and not `enable --now`: `--now` arrived in systemd 220, and the + // hint is the operator's next action, so it may only name mechanisms that exist on + // the host it is printed on. On systemd 219 (Synology DSM) the combined form fails + // with `unrecognized option '--now'` — and because the whole command fails, the unit + // is left installed but neither enabled nor started, which reads as a successful + // install until the next reboot. `enable` then `start` works on every version. + commands: [ + 'sudo systemctl daemon-reload', + `sudo systemctl enable ${ORCAD_SYSTEMD_UNIT_NAME}`, + `sudo systemctl start ${ORCAD_SYSTEMD_UNIT_NAME}` + ] + } + : { + path: `~/.config/systemd/user/${ORCAD_SYSTEMD_UNIT_NAME}`, + commands: [ + 'systemctl --user daemon-reload', + `systemctl --user enable ${ORCAD_SYSTEMD_UNIT_NAME}`, + `systemctl --user start ${ORCAD_SYSTEMD_UNIT_NAME}`, + // Without lingering the service dies with the SSH session that installed it. + 'sudo loginctl enable-linger "$USER"' + ] + } +} From 9a059419af886123cc6aa19c0026b7a9b3164020 Mon Sep 17 00:00:00 2001 From: master Date: Tue, 1 Sep 2026 09:09:36 +0200 Subject: [PATCH 3/6] fix(orcad): decode plist entities, refuse uid 0, drop the unwritable agent log Three defects the review found in the generator/reader pair, each one a case of the two halves not agreeing on the same encoding or the same identity. `readPlistString` returned the XML-escaped text. The generator writes `&` as `&`, so the doctor compared `/srv/a&b` against the caller's `/srv/a&b` and reported the plist it had just produced as pinning a different data root -- and stat-ed an ExecStart path that was never on disk. Decoded on one pass, so a path literally containing `<` survives as itself rather than decoding twice. This is the plist half of the rule the systemd splitter already followed. `assertNotRoot` compared against the name only. systemd's `User=` takes a numeric uid too, so `--user 0` rendered exactly the unit the guard exists to refuse, and the audit read it back as an ordinary account and reported `Runs as 0.` as healthy. Guard and audit now share `isRootAccount`. A LaunchAgent got `StandardOutPath=/var/log/orcad.log`. That path is root-owned on macOS and the agent runs unprivileged, so it cannot open its own log, and launchd reports the failure against the file rather than the job -- it reads as a broken install for a reason the plist never states. The renderer does no I/O and so cannot resolve a home directory, so it now emits the keys only when the caller resolved a path; `printService` supplies `~/Library/Logs/orcad.log` for an agent and `/var/log/orcad.log` for a daemon. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UacRCfD4XPe29bD3ZWNc57 --- src/main/orcad/orcad-service-command.ts | 20 +++++-- src/shared/supervisor-service-audit.test.ts | 37 +++++++++++++ src/shared/supervisor-service-audit.ts | 6 +-- .../supervisor-service-file-read.test.ts | 54 +++++++++++++++++++ src/shared/supervisor-service-file-read.ts | 47 ++++++++++++---- src/shared/supervisor-service-render.test.ts | 31 +++++++++++ src/shared/supervisor-service-render.ts | 44 +++++++++++---- 7 files changed, 211 insertions(+), 28 deletions(-) diff --git a/src/main/orcad/orcad-service-command.ts b/src/main/orcad/orcad-service-command.ts index c173733f4a09..1e02e591611d 100644 --- a/src/main/orcad/orcad-service-command.ts +++ b/src/main/orcad/orcad-service-command.ts @@ -10,8 +10,8 @@ * written there: proving something must not bind a port or take the data-root lock. Data * root resolution is pure environment reads, so the pinned value is available that early. */ -import { userInfo } from 'node:os' -import { basename, resolve } from 'node:path' +import { homedir, userInfo } from 'node:os' +import { basename, join, resolve } from 'node:path' import process from 'node:process' import { auditSupervisorServices, @@ -175,6 +175,19 @@ function resolveOrcadEntryPath(explicit?: string): string { return resolved } +/** + * Where a launchd job can actually write its log, which is not one path for both scopes. + * + * `/var/log` is root-owned on macOS, so it suits a LaunchDaemon and defeats a LaunchAgent: + * the agent runs as the operator, cannot create the file, and launchd reports that against + * the log path rather than the job — so the service looks broken for a reason the plist + * does not name. Resolved here rather than in the renderer because launchd expands no `~` + * and the renderer performs no I/O. + */ +function launchdLogPath(scope: SupervisorScope): string { + return scope === 'system' ? '/var/log/orcad.log' : join(homedir(), 'Library/Logs/orcad.log') +} + export async function printService(argv: string[]): Promise { const options = parseServiceCommandArgs(argv) const platform = resolveSupervisorPlatform(process.platform) @@ -191,7 +204,8 @@ export async function printService(argv: string[]): Promise { // the account is a flag rather than an inheritance. user: options.user ?? userInfo().username, bind: options.bind, - port: options.port + port: options.port, + logPath: platform === 'launchd' ? launchdLogPath(options.scope) : undefined } const hint = supervisorInstallHint(config) process.stdout.write(renderSupervisorService(config)) diff --git a/src/shared/supervisor-service-audit.test.ts b/src/shared/supervisor-service-audit.test.ts index 24710a5389d2..967430aeaca2 100644 --- a/src/shared/supervisor-service-audit.test.ts +++ b/src/shared/supervisor-service-audit.test.ts @@ -141,6 +141,30 @@ describe('data root', () => { expect(findings.map((f) => f.code)).toContain('user_data_mismatch') expect(supervisorAuditPassed(findings)).toBe(true) }) + + // The end-to-end form of the escape/decode pair: the generator writes `&` as `&`, so + // an audit that does not decode compares two spellings of one path and accuses the plist + // this tool just produced of pointing somewhere else. + it('does not accuse its own plist of a mismatch over an escaped character', () => { + const escapedRoot = '/Volumes/a&b/.orca' + const plist: SupervisorServiceFile = { + path: '/Library/LaunchDaemons/dev.onorca.orcad.plist', + platform: 'launchd', + scope: 'system', + text: renderSupervisorService({ + platform: 'launchd', + scope: 'system', + nodePath: '/usr/local/bin/node', + orcadPath: '/opt/orcad/orcad.js', + userDataPath: escapedRoot, + user: 'orca', + bind: '127.0.0.1', + port: 6800, + logPath: '/var/log/orcad.log' + }) + } + expect(codes([plist], escapedRoot)).not.toContain('user_data_mismatch') + }) }) describe('run-as account', () => { @@ -154,6 +178,19 @@ describe('run-as account', () => { expect(codes([file({ text })])).toContain('run_as_root') }) + // `User=0` is root spelled numerically, which systemd accepts. A name-only check reads it + // back as an ordinary account and reports `Runs as 0.` as healthy — the audit has to know + // the same rule the generator's guard does, or the numeric spelling walks between them. + it('flags the numeric spelling of root too', () => { + const text = file().text.replace('User=orca', 'User=0') + expect(codes([file({ text })])).toContain('run_as_root') + }) + + it('does not flag an ordinary account that merely ends in a digit', () => { + const text = file().text.replace('User=orca', 'User=orca0') + expect(codes([file({ text })])).toContain('run_as_user_set') + }) + it('accepts a user-scope unit with no User', () => { const text = file().text.replace('User=orca', '') const findings = audit([file({ text, scope: 'user' })]) diff --git a/src/shared/supervisor-service-audit.ts b/src/shared/supervisor-service-audit.ts index dfd6d3acdb2c..f668f04b48ed 100644 --- a/src/shared/supervisor-service-audit.ts +++ b/src/shared/supervisor-service-audit.ts @@ -21,6 +21,7 @@ import { } from './supervisor-service-file-read' import { DISCOURAGED_SYSTEMD_KILL_MODES, + isRootAccount, SAFE_SYSTEMD_KILL_MODES } from './supervisor-service-render' @@ -169,12 +170,11 @@ function auditRunAsUser(file: SupervisorServiceFile): SupervisorFinding { remedy: file.scope === 'system' ? 'Add an explicit run-as account.' : undefined } } - if (user === 'root') { + if (isRootAccount(user)) { return { code: 'run_as_root', severity: 'critical', - message: - 'Service runs orcad as root, which creates a root-owned data root no later user run can use.', + message: `Service runs orcad as root (${user}), which creates a root-owned data root no later user run can use.`, remedy: 'Set the account orcad should run as.' } } diff --git a/src/shared/supervisor-service-file-read.test.ts b/src/shared/supervisor-service-file-read.test.ts index 5b91afd0af7d..5823ea6a7cfb 100644 --- a/src/shared/supervisor-service-file-read.test.ts +++ b/src/shared/supervisor-service-file-read.test.ts @@ -61,6 +61,60 @@ describe('reading back what the generator wrote', () => { }) }) +// The plist half of the same rule. The generator XML-escapes every value it writes, so a +// reader that does not decode compares `/srv/a&b` against the caller's `/srv/a&b` and +// calls the generator's own output a mismatched data root — or stats an ExecStart path that +// was never on disk and reports a healthy job as unable to start. +describe('reading back what the generator wrote to a plist', () => { + const hostile = { + userDataPath: '/Volumes/a&b/', + nodePath: '/opt/n&de/bin/node', + orcadPath: '/opt/orc&d/orcad.js' + } + + function plist(overrides: typeof hostile): SupervisorServiceFile { + return { + path: '/Library/LaunchDaemons/dev.onorca.orcad.plist', + text: renderSupervisorService({ + platform: 'launchd', + scope: 'system', + nodePath: overrides.nodePath, + orcadPath: overrides.orcadPath, + userDataPath: overrides.userDataPath, + user: 'orca', + bind: '127.0.0.1', + port: 6800, + logPath: '/var/log/orcad.log' + }), + platform: 'launchd', + scope: 'system' + } + } + + it('decodes the escaped data root back to the path on disk', () => { + expect(readPinnedUserData(plist(hostile))).toBe('/Volumes/a&b/') + }) + + it('decodes ProgramArguments, which the doctor stats', () => { + expect(readExecTarget(plist(hostile))).toEqual({ + interpreter: '/opt/n&de/bin/node', + script: '/opt/orc&d/orcad.js' + }) + }) + + it('still reads the endpoint out of an escaped argument list', () => { + expect(readConfiguredEndpoint(plist(hostile))).toEqual({ bind: '127.0.0.1', port: 6800 }) + }) + + // Decoding twice would turn a path that literally contains `<` into one containing `<` + // — the same class of bug in the other direction. + it('decodes exactly one round, so an entity in the path survives as itself', () => { + expect(readPinnedUserData(plist({ ...hostile, userDataPath: '/srv/<literal' }))).toBe( + '/srv/<literal' + ) + }) +}) + describe('the systemd command-line splitter', () => { it('splits on whitespace outside quotes', () => { expect(splitSystemdCommandLine('/bin/node /a/b.js --port 6800')).toEqual([ diff --git a/src/shared/supervisor-service-file-read.ts b/src/shared/supervisor-service-file-read.ts index f4df5444d72b..d422fc8cd54f 100644 --- a/src/shared/supervisor-service-file-read.ts +++ b/src/shared/supervisor-service-file-read.ts @@ -39,9 +39,39 @@ export function readPlistBoolean(text: string, key: string): boolean | null { return match ? match[1].toLowerCase() === 'true' : null } +const XML_ENTITIES: Record = { + amp: '&', + lt: '<', + gt: '>', + quot: '"', + apos: "'" +} + +/** + * The inverse of the generator's `xmlEscape`, and it exists for the same reason the systemd + * splitter does: a read is only correct if it undoes what the write did. A data root holding + * `&` is written `&`, and an undecoded read then compares `/srv/a&b` against the + * caller's `/srv/a&b` — reporting the generator's own plist as a mismatched data root, and + * stat-ing an ExecStart path that was never on disk. + * + * One pass over the named entities, so `&lt;` decodes to the literal `<` rather than + * being decoded twice into `<`. Numeric entities are left alone: the generator never emits + * one, and half-decoding a hand-edited path is worse than not touching it. + */ +function xmlUnescape(value: string): string { + return value.replace(/&(amp|lt|gt|quot|apos);/g, (whole, entity: string) => { + return XML_ENTITIES[entity] ?? whole + }) +} + export function readPlistString(text: string, key: string): string | null { const match = new RegExp(`\\s*${key}\\s*\\s*([^<]*)`, 'i').exec(text) - return match ? match[1].trim() : null + return match ? xmlUnescape(match[1].trim()) : null +} + +/** Every `` in an already-narrowed fragment, decoded the same way. */ +function readPlistStrings(fragment: string): string[] { + return [...fragment.matchAll(/([^<]*)<\/string>/g)].map((match) => xmlUnescape(match[1])) } /** @@ -127,9 +157,7 @@ export function readConfiguredEndpoint( const command = file.platform === 'systemd' ? (readSystemdKey(file.text, 'ExecStart') ?? '') - : [...(programArguments ?? '').matchAll(/([^<]*)<\/string>/g)] - .map((match) => match[1]) - .join(' ') + : readPlistStrings(programArguments ?? '').join(' ') const port = Number(/--port[\s=]+(\d+)/.exec(command)?.[1]) if (!Number.isInteger(port)) { return null @@ -150,13 +178,10 @@ export function readExecTarget( const words = file.platform === 'systemd' ? splitSystemdCommandLine(readSystemdKey(file.text, 'ExecStart') ?? '') - : [ - ...( - /\s*ProgramArguments\s*<\/key>\s*([\s\S]*?)<\/array>/i.exec( - file.text - )?.[1] ?? '' - ).matchAll(/([^<]*)<\/string>/g) - ].map((match) => match[1]) + : readPlistStrings( + /\s*ProgramArguments\s*<\/key>\s*([\s\S]*?)<\/array>/i.exec(file.text)?.[1] ?? + '' + ) // systemd allows `-`, `@`, `+`, `!` prefixes on ExecStart; strip them off the binary. const interpreter = words[0]?.replace(/^[-@+!:]+/, '') ?? '' if (!interpreter) { diff --git a/src/shared/supervisor-service-render.test.ts b/src/shared/supervisor-service-render.test.ts index 19f3fd3b951a..64054a6076a3 100644 --- a/src/shared/supervisor-service-render.test.ts +++ b/src/shared/supervisor-service-render.test.ts @@ -96,6 +96,17 @@ describe('run-as account', () => { ) }) + // systemd's `User=` takes a name or a numeric uid, so `0` is the same account as `root` + // spelled differently. A guard that only knows the name renders exactly the unit it exists + // to refuse. + it.each(['0', '00', ' 0 '])('refuses the numeric spelling of root (%j)', (user) => { + expect(() => renderSupervisorService(config({ user }))).toThrow(/root-owned data root/) + }) + + it('does not mistake an ordinary account with a digit for root', () => { + expect(renderSupervisorService(config({ user: 'orca0' }))).toContain('User=orca0') + }) + it('names the account explicitly', () => { expect(renderSupervisorService(config())).toContain('User=orca') expect(renderSupervisorService(config({ platform: 'launchd' }))).toContain( @@ -122,6 +133,26 @@ describe('logging', () => { expect(unit).toContain('StandardOutput=journal') expect(unit).not.toMatch(/StandardOutput=append:/) }) + + // `/var/log` is root-owned on macOS. A LaunchAgent runs as the operator and cannot create + // a file there, and launchd reports that against the log path rather than the job — so the + // service reads as a broken install for a reason the plist never names. The renderer does + // no I/O, so it cannot resolve a home directory; naming nothing is the honest answer, and + // the caller supplies the scope-appropriate path. + it('never writes a log path into a plist the caller did not resolve', () => { + const plist = renderSupervisorService(config({ platform: 'launchd', scope: 'user' })) + expect(plist).not.toContain('/var/log/orcad.log') + expect(plist).not.toContain('StandardOutPath') + }) + + it('emits both stream keys once the caller names a path', () => { + const plist = renderSupervisorService( + config({ platform: 'launchd', logPath: '/Users/orca/Library/Logs/orcad.log' }) + ) + expect(plist).toContain('StandardOutPath') + expect(plist).toContain('StandardErrorPath') + expect(plist.match(/Library\/Logs\/orcad\.log/g)).toHaveLength(2) + }) }) describe('escaping', () => { diff --git a/src/shared/supervisor-service-render.ts b/src/shared/supervisor-service-render.ts index 85ac1f673bad..518305b7c18b 100644 --- a/src/shared/supervisor-service-render.ts +++ b/src/shared/supervisor-service-render.ts @@ -35,7 +35,14 @@ export type SupervisorServiceConfig = { user: string bind: string port: number - /** Only used by launchd, which cannot log to journald. */ + /** + * Only used by launchd, which cannot log to journald. Absolute, and resolved by the + * caller: launchd expands no `~`, and the writable location depends on the scope. A + * LaunchAgent runs unprivileged, so it cannot create `/var/log/orcad.log` — that path is + * root-owned on macOS. Omitted rather than defaulted, because a plist naming a log the + * job cannot open is worse than one naming none: launchd reports the failure against the + * file, not against the job, so it reads as a broken install. + */ logPath?: string } @@ -83,13 +90,25 @@ export function resolveSupervisorPlatform(platform: NodeJS.Platform): Supervisor throw new SupervisorServiceUnsupportedError(platform) } +/** + * `root` by name and `0` by number are one account. systemd's `User=` takes either spelling, + * so a check that only knows the name renders exactly the unit it exists to refuse, and the + * audit that only knows the name reads `User=0` back and reports `Runs as 0.` as healthy. + * Both halves share this for that reason — the guard and the reader have to agree on what + * root is, or the numeric spelling walks through the gap between them. + */ +export function isRootAccount(user: string): boolean { + const account = user.trim() + return account === 'root' || /^0+$/.test(account) +} + /** * Why refuse rather than render: a unit that runs orcad as root creates a root-owned data * root, and every later user-scope run then refuses with `orcad_data_root_wrong_owner` — * a state orcad deliberately will not repair, because the permissions are not its to fix. */ function assertNotRoot(user: string): void { - if (user === 'root') { + if (isRootAccount(user)) { throw new Error( 'refusing to generate a service that runs orcad as root: orcad would create a ' + 'root-owned data root, and every later run as a normal user would then fail with ' + @@ -195,7 +214,17 @@ function renderLaunchd(config: SupervisorServiceConfig): string { const args = [config.nodePath, ...orcadArgs(config)] .map((arg) => ` ${xmlEscape(arg)}`) .join('\n') - const logPath = config.logPath ?? '/var/log/orcad.log' + // launchd has no journald equivalent, so this is a file and nothing rotates it — pair it + // with a newsyslog.d entry. Emitted only when the caller resolved a path it knows the job + // can write; see `logPath`. + const logKeys = config.logPath + ? ` + StandardOutPath + ${xmlEscape(config.logPath)} + StandardErrorPath + ${xmlEscape(config.logPath)} +` + : '' const scopeNote = config.scope === 'system' ? ' LaunchDaemon: runs at boot, independent of any login session.' @@ -236,14 +265,7 @@ ${args} ORCA_USER_DATA ${xmlEscape(config.userDataPath)} - - - StandardOutPath - ${xmlEscape(logPath)} - StandardErrorPath - ${xmlEscape(logPath)} - +${logKeys} ` } From 125bd546079212f5d5b28dbef8a3db7ecee37954 Mon Sep 17 00:00:00 2001 From: master Date: Tue, 1 Sep 2026 09:18:39 +0200 Subject: [PATCH 4/6] fix(orcad): emit a parseable plist, select on --service-path, stop guessing a log path Every generated plist was malformed XML, in both scopes. The header comment carried the regeneration command verbatim, and XML forbids the string `--` anywhere inside a comment body -- so `--print-service` made the document unparseable. expat rejects it, `plutil -lint` rejects it, and launchctl will not load the job: the entire darwin path was dead on arrival. The flags are now named rather than written, with the constraint stated in the file so it is not "fixed" back. renderSystemd carries the same sentence safely because `#` comments have no such rule, which is why this was darwin-only. Nothing caught it because no test parsed the output it asserted against. There is now a well-formedness assertion over both scopes, plus one proving a `--` in a generated path cannot reach a comment. `--service-path` extended conventional discovery instead of selecting. Naming the definition discovery already finds -- the obvious way to ask for a report on your own install -- collected that file twice, so `auditDuplicates` reported `multiple_services_one_root` as CRITICAL with exit 1 while every live probe was skipped as ambiguous. A false critical produced by naming the file you wanted audited. An explicit path now selects. The launchd log path guessed an account's home. `UserName` honours `--user` while the path used the generating user's home, and `/var/log` is root-owned while the job runs unprivileged -- so both scopes could emit a log the job cannot open, which launchd reports against the path rather than the job. No path is defaulted now; generation warns and says what to add, since none can be shown writable by the run-as account from here. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UacRCfD4XPe29bD3ZWNc57 --- src/main/orcad/orcad-service-command.test.ts | 17 ++++- src/main/orcad/orcad-service-command.ts | 22 ++---- .../orcad/supervisor-generation-warnings.ts | 26 +++++++ .../orcad/supervisor-service-discovery.ts | 19 ++++-- src/shared/supervisor-service-render.test.ts | 67 ++++++++++++++++--- src/shared/supervisor-service-render.ts | 15 +++-- 6 files changed, 127 insertions(+), 39 deletions(-) diff --git a/src/main/orcad/orcad-service-command.test.ts b/src/main/orcad/orcad-service-command.test.ts index 7a5e45f5ddcf..be92519bb4d0 100644 --- a/src/main/orcad/orcad-service-command.test.ts +++ b/src/main/orcad/orcad-service-command.test.ts @@ -216,9 +216,6 @@ describe('discovery separates unreadable from absent', () => { expect(unreadable.find((u) => u.path === path)?.reason).toBe('EACCES') }) - // Scoped to the path under test rather than to the whole result: discovery also scans the - // conventional locations, so a host with a real orcad.service installed would otherwise - // fail these on its own installation. it('reports a path that truly is not there as neither found nor unreadable', () => { const path = join(dir, 'absent.service') const { files, unreadable } = collectServiceFiles('systemd', [path]) @@ -233,4 +230,18 @@ describe('discovery separates unreadable from absent', () => { expect(files.map((f) => f.path)).toContain(path) expect(unreadable.map((u) => u.path)).not.toContain(path) }) + + // An explicit path SELECTS; it does not extend the conventional list. While it extended, + // naming the definition conventional discovery already finds — the obvious way to ask for a + // report on your actual install — collected that one file twice, and `auditDuplicates` then + // called it `multiple_services_one_root` at CRITICAL with exit 1 while silently skipping + // every live probe as ambiguous. Asserting the exact result rather than `toContain` is the + // point: the old behaviour also "contained" the path, and only an equality check sees the + // conventional candidates that came along with it. + it('audits only the named definition, not the conventional locations as well', () => { + const path = join(dir, 'selected.service') + writeFileSync(path, '[Service]\nKillMode=process\n', 'utf8') + const { files } = collectServiceFiles('systemd', [path]) + expect(files.map((f) => f.path)).toEqual([path]) + }) }) diff --git a/src/main/orcad/orcad-service-command.ts b/src/main/orcad/orcad-service-command.ts index 1e02e591611d..d847f5994ddd 100644 --- a/src/main/orcad/orcad-service-command.ts +++ b/src/main/orcad/orcad-service-command.ts @@ -10,8 +10,8 @@ * written there: proving something must not bind a port or take the data-root lock. Data * root resolution is pure environment reads, so the pinned value is available that early. */ -import { homedir, userInfo } from 'node:os' -import { basename, join, resolve } from 'node:path' +import { userInfo } from 'node:os' +import { basename, resolve } from 'node:path' import process from 'node:process' import { auditSupervisorServices, @@ -41,6 +41,7 @@ import { collectServiceFiles } from './supervisor-service-discovery' import { resolveUserDataPath } from './orcad-app-paths' import { interpreterOnDiskWarning, + launchdLogDestinationWarning, resolveRealPath, socketPathBudgetWarning, userScopeUnavailableWarning, @@ -175,19 +176,6 @@ function resolveOrcadEntryPath(explicit?: string): string { return resolved } -/** - * Where a launchd job can actually write its log, which is not one path for both scopes. - * - * `/var/log` is root-owned on macOS, so it suits a LaunchDaemon and defeats a LaunchAgent: - * the agent runs as the operator, cannot create the file, and launchd reports that against - * the log path rather than the job — so the service looks broken for a reason the plist - * does not name. Resolved here rather than in the renderer because launchd expands no `~` - * and the renderer performs no I/O. - */ -function launchdLogPath(scope: SupervisorScope): string { - return scope === 'system' ? '/var/log/orcad.log' : join(homedir(), 'Library/Logs/orcad.log') -} - export async function printService(argv: string[]): Promise { const options = parseServiceCommandArgs(argv) const platform = resolveSupervisorPlatform(process.platform) @@ -204,8 +192,7 @@ export async function printService(argv: string[]): Promise { // the account is a flag rather than an inheritance. user: options.user ?? userInfo().username, bind: options.bind, - port: options.port, - logPath: platform === 'launchd' ? launchdLogPath(options.scope) : undefined + port: options.port } const hint = supervisorInstallHint(config) process.stdout.write(renderSupervisorService(config)) @@ -217,6 +204,7 @@ export async function printService(argv: string[]): Promise { socketPathBudgetWarning(config.userDataPath), versionScopedInterpreterWarning(nodePath), interpreterOnDiskWarning(nodePath, options.nodePath !== undefined), + ...(platform === 'launchd' ? [launchdLogDestinationWarning()] : []), ...(platform === 'systemd' && options.scope === 'user' ? [await userScopeUnavailableWarning()] : []) diff --git a/src/main/orcad/supervisor-generation-warnings.ts b/src/main/orcad/supervisor-generation-warnings.ts index 23230493c6b1..a2f67cc986fd 100644 --- a/src/main/orcad/supervisor-generation-warnings.ts +++ b/src/main/orcad/supervisor-generation-warnings.ts @@ -93,6 +93,32 @@ export function socketPathBudgetWarning(userDataPath: string): string | null { return `Warning: ${describeDaemonSocketPathOverflow(budget)} ${DAEMON_SOCKET_PATH_REMEDY}` } +/** + * launchd captures nothing unless the plist names a file, and no path can be defaulted here + * that the job is known to be able to open. + * + * Both candidates are wrong, for the same reason. `/var/log` is root-owned on macOS while the + * job runs as a non-root account — `assertNotRoot` guarantees it is not root — so a + * LaunchDaemon cannot create a file there. And the generating user's home is the run-as + * account's home only when `--user` did not name somebody else, which this process cannot + * resolve. Either guess emits a plist whose log the job cannot open, and launchd reports that + * against the log path rather than against the job: a broken install for a reason the file + * never states. + * + * systemd needs no equivalent — journald captures a unit's output with nothing configured — + * which is why this asymmetry is stated rather than papered over with a plausible default. + */ +export function launchdLogDestinationWarning(): string { + return ( + 'Warning: this job names no log file, so launchd will not capture orcad’s output. ' + + 'No path is defaulted because none can be shown writable by the run-as account: ' + + '/var/log is root-owned while the job runs unprivileged, and when --user names another ' + + 'account its home directory cannot be resolved from here. To keep logs, create a file ' + + 'the run-as account owns and add StandardOutPath and StandardErrorPath pointing at it — ' + + 'then pair it with a newsyslog.d entry, because nothing rotates it.' + ) +} + /** * A user-scope systemd service needs a running user instance, and the install commands * (`systemctl --user`, `loginctl enable-linger`) all fail without one. Appliance hosts diff --git a/src/main/orcad/supervisor-service-discovery.ts b/src/main/orcad/supervisor-service-discovery.ts index 269b5ffcb440..abcb98f0f938 100644 --- a/src/main/orcad/supervisor-service-discovery.ts +++ b/src/main/orcad/supervisor-service-discovery.ts @@ -61,6 +61,15 @@ export type ServiceFileDiscovery = { * Reads every candidate rather than stopping at the first: two definitions targeting one * data root is itself the highest-severity finding, and stopping early would hide it. * + * An explicit path SELECTS rather than extends — the difference between "audit this + * definition" and "audit the usual ones and also this". Extending looked harmless and was + * not: point `--service-path` at the file conventional discovery already finds, which is the + * obvious thing to do when you want a report about your actual install, and the same file is + * collected twice. `auditDuplicates` then sees two paths sharing one data root and reports + * `multiple_services_one_root` as CRITICAL with exit 1, while `files.length` stops being 1 + * so every live probe is skipped as ambiguous. A false critical and a silently degraded + * report, both produced by naming the file you wanted audited. + * * Presence and readability are separated deliberately. `existsSync` succeeds on a file the * caller cannot open — a traversable directory is enough — so a unit installed with a * restrictive mode used to fall into the same catch as one that was never there, and the @@ -71,12 +80,12 @@ export type ServiceFileDiscovery = { */ export function collectServiceFiles( platform: SupervisorPlatform, - extraPaths: string[] = [] + explicitPaths: string[] = [] ): ServiceFileDiscovery { - const candidates = [ - ...candidateServicePaths(platform), - ...extraPaths.map((path) => ({ path, scope: inferScopeFromPath(path) })) - ] + const candidates = + explicitPaths.length > 0 + ? explicitPaths.map((path) => ({ path, scope: inferScopeFromPath(path) })) + : candidateServicePaths(platform) const files: SupervisorServiceFile[] = [] const unreadable: UnreadableServiceFile[] = [] for (const candidate of candidates) { diff --git a/src/shared/supervisor-service-render.test.ts b/src/shared/supervisor-service-render.test.ts index 64054a6076a3..67763d05851d 100644 --- a/src/shared/supervisor-service-render.test.ts +++ b/src/shared/supervisor-service-render.test.ts @@ -134,16 +134,20 @@ describe('logging', () => { expect(unit).not.toMatch(/StandardOutput=append:/) }) - // `/var/log` is root-owned on macOS. A LaunchAgent runs as the operator and cannot create - // a file there, and launchd reports that against the log path rather than the job — so the - // service reads as a broken install for a reason the plist never names. The renderer does - // no I/O, so it cannot resolve a home directory; naming nothing is the honest answer, and - // the caller supplies the scope-appropriate path. - it('never writes a log path into a plist the caller did not resolve', () => { - const plist = renderSupervisorService(config({ platform: 'launchd', scope: 'user' })) - expect(plist).not.toContain('/var/log/orcad.log') - expect(plist).not.toContain('StandardOutPath') - }) + // No log path can be defaulted that the job is known to be able to open. `/var/log` is + // root-owned while the job runs unprivileged, and the generating user's home is the run-as + // account's home only when `--user` did not name someone else — which the renderer, doing + // no I/O, cannot resolve. Either guess emits a plist whose log the job cannot open, and + // launchd reports that against the log path rather than the job: a broken install for a + // reason the file never states. + it.each(['user', 'system'] as const)( + 'never guesses a log path for a %s scope job', + (scope) => { + const plist = renderSupervisorService(config({ platform: 'launchd', scope })) + expect(plist).not.toContain('/var/log/orcad.log') + expect(plist).not.toContain('StandardOutPath') + } + ) it('emits both stream keys once the caller names a path', () => { const plist = renderSupervisorService( @@ -155,6 +159,49 @@ describe('logging', () => { }) }) +// The generated plist has to survive a real parser, not just read plausibly. XML forbids the +// string `--` anywhere inside a comment body, and the header comment carried the regeneration +// command verbatim — so every plist orcad generated, in BOTH scopes, was malformed: expat +// rejects it, `plutil -lint` rejects it (the first thing an operator runs), and launchctl will +// not load the job. Nothing caught it because no test parsed the output it asserted against. +describe('plist well-formedness', () => { + function commentBodies(xml: string): string[] { + return [...xml.matchAll(//g)].map((match) => match[1]) + } + + it.each(['user', 'system'] as const)( + 'puts no double hyphen inside a comment in %s scope', + (scope) => { + const bodies = commentBodies( + renderSupervisorService(config({ platform: 'launchd', scope })) + ) + expect(bodies.length).toBeGreaterThan(0) + for (const body of bodies) { + expect(body).not.toContain('--') + } + } + ) + + // The constraint above is only worth respecting if the guidance survives it: naming the + // flags without their dashes has to still tell the reader what to run. + it.each(['user', 'system'] as const)('still says how to regenerate a %s job', (scope) => { + const plist = renderSupervisorService(config({ platform: 'launchd', scope })) + expect(plist).toContain('print-service') + expect(plist).toContain(`scope ${scope}`) + }) + + // A path carrying `--` reaches the document as an escaped , not as a comment, so it + // must not be able to break the rule above from the outside. + it('is unaffected by a double hyphen inside a generated path', () => { + const bodies = commentBodies( + renderSupervisorService(config({ platform: 'launchd', userDataPath: '/srv/a--b/.orca' })) + ) + for (const body of bodies) { + expect(body).not.toContain('--') + } + }) +}) + describe('escaping', () => { it('escapes plist-hostile characters in generated paths', () => { const plist = renderSupervisorService( diff --git a/src/shared/supervisor-service-render.ts b/src/shared/supervisor-service-render.ts index 518305b7c18b..b860ab970c59 100644 --- a/src/shared/supervisor-service-render.ts +++ b/src/shared/supervisor-service-render.ts @@ -227,12 +227,19 @@ function renderLaunchd(config: SupervisorServiceConfig): string { : '' const scopeNote = config.scope === 'system' - ? ' LaunchDaemon: runs at boot, independent of any login session.' - : ' LaunchAgent: runs ONLY while this user is logged in. A headless Mac needs\n --scope system instead.' + ? ' LaunchDaemon: runs at boot, independent of any login session.' + : ' LaunchAgent: runs ONLY while this user is logged in. A headless Mac needs\n scope system instead.' + // Every flag below is NAMED rather than written with its leading dashes, and must stay + // that way. XML forbids the string `--` anywhere inside a comment body, so writing + // `--print-service` here makes the plist unparseable: expat rejects it, `plutil -lint` + // rejects it, and launchctl refuses to load the job. The systemd renderer carries the + // same sentence safely because `#` comments have no such rule, which is why this bites + // on darwin only. return ` - From cea7f3c27255416b2801332442b7883f0fa29502 Mon Sep 17 00:00:00 2001 From: master Date: Tue, 1 Sep 2026 00:47:00 +0200 Subject: [PATCH 5/6] feat(cli): add `orca supervisor print` and `orca supervisor doctor` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A front door on the `orca` CLI over the generator and audit `orcad --print-service` / `--doctor` already expose, so an operator setting up a supervised host does not have to find the orcad bundle first to ask a question about it. **`orca supervisor print` requires `--orcad `, and orcad's own flag does not.** Under the CLI, `process.argv[1]` is the CLI's entry, not `orcad.js`, so inferring it produced a unit pinning `ExecStart= .../cli/index.js` — a service that cannot start. The flag is required rather than guessed, and the spec note says why at the point someone reads the usage. Named `supervisor` rather than `daemon` or `service`: `daemon` already means the terminal daemon everywhere in this codebase, and `service` is two characters from `serve` while one starts a server and the other writes system files — the kind of neighbouring pair typo recovery must never steer between. It is also the word `docs/reference/orcad-operations.md` already uses. Registered local-only via `shouldIgnoreRemoteSelection`: these inspect THIS machine's service files, which is normally not the host a paired CLI targets, so remote selection would silently answer about the wrong machine. `orcad/orcad-service-command` gets an `electron.vite.config.ts` entry because the CLI now imports that path — without one, the build that runs after `build:cli` cleans it out of `out/main` and the command dies at require time. `main-module-bundle-parity.test.ts` exists to catch exactly this. The modules added to `tsconfig.cli.json` are all leaves — node builtins, `src/shared` and each other — so the daemon itself stays out of the CLI's import graph. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UacRCfD4XPe29bD3ZWNc57 --- config/tsconfig.cli.json | 14 ++++++ electron.vite.config.ts | 6 ++- src/cli/handler-group-manifest.ts | 5 +++ src/cli/handlers/supervisor.ts | 74 +++++++++++++++++++++++++++++++ src/cli/index.ts | 4 ++ src/cli/specs/index.ts | 2 + src/cli/specs/supervisor.ts | 45 +++++++++++++++++++ 7 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 src/cli/handlers/supervisor.ts create mode 100644 src/cli/specs/supervisor.ts diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 93d556602f87..89a1743fd7b9 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -127,6 +127,20 @@ // Why: serve-electron-flag-parity.test.ts checks the Electron-side serve argv rewrite against this // project's serve spec; the module has no imports, so listing it pulls in nothing else. "../src/main/startup/serve-mode-argv.ts", + // Why: `orca supervisor` is a thin front door over the same generator and audit orcad + // exposes. Both modules import only node builtins and src/shared, so listing them + // pulls in nothing else. + "../src/main/orcad/orcad-app-paths.ts", + "../src/main/orcad/orcad-service-command.ts", + "../src/main/orcad/supervisor-service-discovery.ts", + "../src/main/orcad/supervisor-generation-warnings.ts", + "../src/main/orcad/supervisor-host-observations.ts", + // Why these two as well: the doctor answers whether a data root leaves room for the daemon's + // socket, which is arithmetic over the daemon's endpoint names. Both are leaves — node + // builtins, src/shared and each other — so the daemon itself stays out of this graph. + "../src/main/orcad/supervisor-daemon-socket-budget.ts", + "../src/main/daemon/daemon-runtime-paths.ts", + "../src/main/daemon/daemon-protocol-version.ts", "../src/main/runtime/runtime-metadata.ts", "../src/main/sqlite/sync-database.ts", "../src/main/win32-utils.ts" diff --git a/electron.vite.config.ts b/electron.vite.config.ts index 4ed4641cde15..e9a871cd6a45 100644 --- a/electron.vite.config.ts +++ b/electron.vite.config.ts @@ -254,7 +254,11 @@ export const electronViteConfig: UserConfig = { 'src/main/agent-hooks/managed-agent-hook-controls.ts' ), // Why: account import mutates the user's macOS Keychain from the CLI. - 'claude-accounts/keychain': resolve('src/main/claude-accounts/keychain.ts') + 'claude-accounts/keychain': resolve('src/main/claude-accounts/keychain.ts'), + // Why: `orca supervisor print|doctor` is a front door over orcad's own generator + // and audit, so the CLI imports this path. Without an entry the build that runs + // after build:cli cleans it out of out/main and the command dies at require time. + 'orcad/orcad-service-command': resolve('src/main/orcad/orcad-service-command.ts') }, // Why: Rolldown's SSR default is ESM, but Electron and sidecar launchers // consume these stable CommonJS paths. diff --git a/src/cli/handler-group-manifest.ts b/src/cli/handler-group-manifest.ts index d995b4d8687e..7f6c15e1a625 100644 --- a/src/cli/handler-group-manifest.ts +++ b/src/cli/handler-group-manifest.ts @@ -17,6 +17,11 @@ export const HANDLER_GROUPS: readonly HandlerGroup[] = [ keys: ['claude-teams', 'open', 'serve', 'status'], load: async () => (await import('./handlers/core.js')).CORE_HANDLERS }, + { + name: 'supervisor', + keys: ['supervisor print', 'supervisor doctor'], + load: async () => (await import('./handlers/supervisor.js')).SUPERVISOR_HANDLERS + }, { name: 'account', keys: ['account add', 'account list'], diff --git a/src/cli/handlers/supervisor.ts b/src/cli/handlers/supervisor.ts new file mode 100644 index 000000000000..cb61a7768ba8 --- /dev/null +++ b/src/cli/handlers/supervisor.ts @@ -0,0 +1,74 @@ +/** + * `orca supervisor print|doctor` — a front door over the generator and audit orcad already + * exposes as `--print-service` and `--doctor`. + * + * These inspect and describe THIS machine. The CLI normally targets a paired runtime that + * is a different host entirely, so `supervisor` is registered in `shouldIgnoreRemoteSelection` + * and every answer names the host it looked at — otherwise an operator would read a verdict + * about their laptop as a verdict about their server. + */ +import { hostname } from 'node:os' +import process from 'node:process' +import type { CommandHandler } from '../dispatch' +import { RuntimeClientError } from '../runtime/types' +import { + collectDoctorFindings, + printService, + runDoctor +} from '../../main/orcad/orcad-service-command' +import { SupervisorServiceUnsupportedError } from '../../shared/supervisor-service-render' + +/** Why translate: an unsupported platform is a bad request, not a CLI crash. */ +async function reportingUnsupported(run: () => Promise | T): Promise { + try { + return await run() + } catch (error) { + throw error instanceof SupervisorServiceUnsupportedError + ? new RuntimeClientError('invalid_argument', error.message) + : error + } +} + +/** Rebuilds the argv the orcad-side parser expects from the CLI's parsed flag map. */ +function toServiceArgv(flags: Map, names: string[]): string[] { + const argv: string[] = [] + for (const name of names) { + const value = flags.get(name) + if (value === true) { + argv.push(`--${name}`) + } else if (typeof value === 'string') { + argv.push(`--${name}`, value) + } + } + return argv +} + +export const SUPERVISOR_HANDLERS: Record = { + 'supervisor print': async ({ flags, json }) => { + // Why refuse rather than ignore: stdout here is a file meant to be redirected, so + // honouring --json would hand a scripted caller a unit file wrapped in nothing. + if (json) { + throw new RuntimeClientError( + 'invalid_argument', + 'supervisor print writes a service definition to stdout, so it has no JSON form. Drop --json.' + ) + } + await reportingUnsupported(() => + printService(toServiceArgv(flags, ['orcad', 'scope', 'user', 'node', 'port', 'bind'])) + ) + }, + 'supervisor doctor': async ({ flags, json }) => { + const argv = toServiceArgv(flags, ['service-path', 'no-probe']) + if (json) { + const { findings, code } = await reportingUnsupported(() => collectDoctorFindings(argv)) + process.stdout.write(`${JSON.stringify({ host: hostname(), findings }, null, 2)}\n`) + process.exitCode = code + return + } + process.stdout.write(`Inspecting service definitions on ${hostname()} (this machine).\n\n`) + const code = await reportingUnsupported(() => runDoctor(argv)) + if (code !== 0) { + process.exitCode = code + } + } +} diff --git a/src/cli/index.ts b/src/cli/index.ts index 2a8921a2cdb7..68bbb175c666 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -31,6 +31,10 @@ function shouldIgnoreRemoteSelection(commandPath: string[]): boolean { commandPath[0] === 'artifacts' || commandPath[0] === 'environment' || commandPath[0] === 'serve' || + // Why local-only: supervisor inspects THIS machine's service files, which is normally + // not the host a paired CLI targets. Remote selection would silently answer about the + // wrong machine. + commandPath[0] === 'supervisor' || commandPath[0] === 'agent' || commandPath[0] === 'vm' || commandPath[0] === 'agent-context' diff --git a/src/cli/specs/index.ts b/src/cli/specs/index.ts index 928297931192..7fb1391b2f8d 100644 --- a/src/cli/specs/index.ts +++ b/src/cli/specs/index.ts @@ -17,9 +17,11 @@ import { LINEAR_COMMAND_SPECS } from './linear' import { VM_COMMAND_SPECS } from './vm' import { SKILL_COMMAND_SPECS } from './skills' import { ARTIFACT_COMMAND_SPECS } from './artifacts' +import { SUPERVISOR_COMMAND_SPECS } from './supervisor' export const COMMAND_SPECS: CommandSpec[] = [ ...CORE_COMMAND_SPECS, + ...SUPERVISOR_COMMAND_SPECS, ...ARTIFACT_COMMAND_SPECS, ...ACCOUNT_COMMAND_SPECS, ...PROJECT_COMMAND_SPECS, diff --git a/src/cli/specs/supervisor.ts b/src/cli/specs/supervisor.ts new file mode 100644 index 000000000000..feed5be8f73f --- /dev/null +++ b/src/cli/specs/supervisor.ts @@ -0,0 +1,45 @@ +import type { CommandSpec } from '../args' +import { GLOBAL_FLAGS } from '../args' + +/** + * Why `supervisor` and not `daemon` or `service`: `daemon` means the terminal daemon + * throughout this codebase, and `service` is two characters from `serve` while one starts a + * server and the other writes system files — the kind of neighbouring pair typo recovery + * must never steer between. `supervisor` is the word orcad-operations.md already uses. + * + * These commands are registered local-only (`shouldIgnoreRemoteSelection`): they inspect + * THIS machine's service files, which is normally not the host a paired CLI targets. + */ +export const SUPERVISOR_COMMAND_SPECS: CommandSpec[] = [ + { + path: ['supervisor', 'print'], + summary: 'Print a supervisor service definition for running orcad on this machine', + usage: + 'orca supervisor print --orcad [--scope user|system] [--user ] [--node ] [--port ] [--bind
]', + allowedFlags: [...GLOBAL_FLAGS, 'orcad', 'scope', 'user', 'node', 'port', 'bind'], + notes: [ + 'Writes the file to stdout and changes nothing; placement and enable commands go to stderr.', + '--orcad is required here: this process is the orca CLI, so it cannot name which orcad.js the service should run.', + 'Inspects and describes THIS machine, not a paired remote runtime.', + 'The generated file pins the data root, because a service inherits too little environment to resolve the same one your shell does.', + 'Defaults to a loopback bind; reach a remote orcad over an SSH port-forward rather than widening it.' + ], + examples: [ + 'orca supervisor print --orcad ~/.orca-remote/orcad-0.1.0/orcad.js --scope system --user orca', + 'orca supervisor print --orcad /opt/orcad/orcad.js --scope user > ~/.config/systemd/user/orcad.service' + ] + }, + { + path: ['supervisor', 'doctor'], + summary: 'Check an installed orcad service for settings that would lose running terminals', + usage: 'orca supervisor doctor [--service-path ] [--no-probe]', + allowedFlags: [...GLOBAL_FLAGS, 'service-path', 'no-probe'], + notes: [ + 'Inspects and describes THIS machine, not a paired remote runtime.', + 'Exits non-zero only for settings that would destroy running terminals; observed state is reported, not failed.', + 'Anything it cannot establish is reported as unverified with the reason, never as a negative.', + 'Use --no-probe for the file-only audit, without shelling out to systemctl or launchctl.' + ], + examples: ['orca supervisor doctor', 'orca supervisor doctor --no-probe'] + } +] From d64dc4936745f31a83ae1c8617108ec4138a265d Mon Sep 17 00:00:00 2001 From: master Date: Tue, 1 Sep 2026 09:20:48 +0200 Subject: [PATCH 6/6] fix(cli): register `supervisor` as a command group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `orca supervisor --help` and `orca help supervisor` printed `Unknown command: supervisor`, then the entire root help, then exited 1. The group was registered in the handler manifest, the spec index and `shouldIgnoreRemoteSelection` — but not in `isCommandGroup`, which is what `printHelp` consults before falling through to the unknown-command path. Each subcommand's own `--help` kept working, so nothing about the specs revealed the gap. `supervisor` was the only multi-segment command prefix in the CLI missing from that list, and the list carries no entries without a matching spec — so the registration is an exhaustive invariant rather than a curated one, and `index.test.ts` now asserts it directly. The behaviour is pinned too, exit code included: an unregistered group still prints a screenful of help, and only the 1 says it went wrong. Both commands are now listed under `orca --help`. A front door added so an operator does not have to find the orcad bundle first is not much of a front door if the root help never mentions it; `vm recipe doctor` is the precedent for a narrow operational command earning its own heading. Summaries are trimmed to the width that file uses, as `vm recipe doctor`'s already is. `supervisor.test.ts` covers the adapter itself, which had no direct test: the argv it rebuilds for the orcad-side parser (flag order is the handler's, not the caller's), the `--json` refusal on `print`, the audit exit code carried out of both output paths, and the unsupported-platform translation — plus the fact that every other failure keeps its own message, since the missing-`--orcad` refusal is a plain Error whose text is the only thing naming the flag. `--no-probe` is deliberately left out of `BOOLEAN_FLAGS`. That set is for switches whose following token could be mistaken for a value; `supervisor doctor` takes no positionals, so every shape it can be written in already parses as a switch — and three of the four other `--no-*` flags in the CLI (`no-screenshot`, `no-parent`, `no-ua-spoof`) are absent from it for the same reason. `args.test.ts` pins the parse instead, because the set alone reads as though it should be there. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UacRCfD4XPe29bD3ZWNc57 --- src/cli/args.test.ts | 16 ++++ src/cli/args.ts | 1 + src/cli/handlers/supervisor.test.ts | 131 ++++++++++++++++++++++++++++ src/cli/index.test.ts | 42 ++++++++- src/cli/root-help-text-primary.ts | 4 + 5 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 src/cli/handlers/supervisor.test.ts diff --git a/src/cli/args.test.ts b/src/cli/args.test.ts index eeedfbe04287..e6e28025c587 100644 --- a/src/cli/args.test.ts +++ b/src/cli/args.test.ts @@ -38,6 +38,22 @@ describe('parseArgs', () => { expect(parsed.flags.get('value')).toBe('') }) + // Why this is not in BOOLEAN_FLAGS: that set exists for switches a following token could + // be mistaken for a value, and `supervisor doctor` takes no positionals — so every shape + // it can be written in already parses as a switch. Pinned because reading the set alone + // suggests otherwise. + it('reads --no-probe as a switch in every shape supervisor doctor accepts', () => { + expect(parseArgs(['supervisor', 'doctor', '--no-probe']).flags.get('no-probe')).toBe(true) + expect(parseArgs(['supervisor', 'doctor', '--no-probe', '--json']).flags.get('no-probe')).toBe( + true + ) + + const withPath = parseArgs(['supervisor', 'doctor', '--no-probe', '--service-path', '/etc/x']) + + expect(withPath.flags.get('no-probe')).toBe(true) + expect(withPath.flags.get('service-path')).toBe('/etc/x') + }) + it('still parses boolean flags and space-separated values', () => { const parsed = parseArgs(['tab', 'create', '--json', '--url', 'https://example.com']) diff --git a/src/cli/args.ts b/src/cli/args.ts index c4c373a814f1..24afc37ca236 100644 --- a/src/cli/args.ts +++ b/src/cli/args.ts @@ -228,6 +228,7 @@ export function isCommandGroup(commandPath: string[]): boolean { 'diagnostics', 'linear', 'skills', + 'supervisor', 'vm' ].includes(commandPath[0])) || (commandPath.length === 2 && commandPath[0] === 'agent' && commandPath[1] === 'hooks') || diff --git a/src/cli/handlers/supervisor.test.ts b/src/cli/handlers/supervisor.test.ts new file mode 100644 index 000000000000..49ea5f404fde --- /dev/null +++ b/src/cli/handlers/supervisor.test.ts @@ -0,0 +1,131 @@ +import { hostname } from 'node:os' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' + +const { printServiceMock, runDoctorMock, collectDoctorFindingsMock } = vi.hoisted(() => ({ + printServiceMock: vi.fn(), + runDoctorMock: vi.fn(), + collectDoctorFindingsMock: vi.fn() +})) + +// Why mock: these handlers are an adapter, and the thing worth pinning is the argv they +// build and the exit code they carry — not the generator, which has its own tests and +// would drag a real host's service files into this one. +vi.mock('../../main/orcad/orcad-service-command', () => ({ + printService: printServiceMock, + runDoctor: runDoctorMock, + collectDoctorFindings: collectDoctorFindingsMock +})) + +import { SUPERVISOR_HANDLERS } from './supervisor' +import { SupervisorServiceUnsupportedError } from '../../shared/supervisor-service-render' + +const originalExitCode = process.exitCode +let stdout: string + +beforeEach(() => { + stdout = '' + vi.spyOn(process.stdout, 'write').mockImplementation((chunk) => { + stdout += String(chunk) + return true + }) +}) + +afterEach(() => { + process.exitCode = originalExitCode + vi.restoreAllMocks() + vi.clearAllMocks() +}) + +function run( + command: string, + flags: [string, string | boolean][] = [], + json = false +): Promise { + return SUPERVISOR_HANDLERS[command]({ flags: new Map(flags), json } as never) +} + +it('rebuilds the argv the orcad parser expects, in its own flag order', async () => { + printServiceMock.mockResolvedValue(0) + + await run('supervisor print', [ + ['bind', '127.0.0.1'], + ['scope', 'system'], + ['orcad', '/opt/orcad/orcad.js'], + ['user', 'orca'] + ]) + + expect(printServiceMock).toHaveBeenCalledWith([ + '--orcad', + '/opt/orcad/orcad.js', + '--scope', + 'system', + '--user', + 'orca', + '--bind', + '127.0.0.1' + ]) +}) + +// Why: the orcad parser reads --no-probe as a bare switch and treats the next token as an +// unknown argument, so emitting it with a value would fail the command outright. +it('emits a boolean flag bare and omits the flags nobody passed', async () => { + runDoctorMock.mockResolvedValue(0) + + await run('supervisor doctor', [ + ['service-path', '/etc/systemd/system/orcad.service'], + ['no-probe', true] + ]) + + expect(runDoctorMock).toHaveBeenCalledWith([ + '--service-path', + '/etc/systemd/system/orcad.service', + '--no-probe' + ]) +}) + +it('refuses --json on print instead of generating a unit file wrapped in nothing', async () => { + await expect( + run('supervisor print', [['orcad', '/opt/orcad/orcad.js']], true) + ).rejects.toMatchObject({ code: 'invalid_argument' }) + + expect(printServiceMock).not.toHaveBeenCalled() +}) + +it('carries the audit exit code out of the JSON path and names the host it read', async () => { + const findings = [{ code: 'kill_mode_unsafe', severity: 'critical', message: 'KillMode=mixed' }] + collectDoctorFindingsMock.mockResolvedValue({ findings, code: 1 }) + + await run('supervisor doctor', [], true) + + expect(JSON.parse(stdout)).toEqual({ host: hostname(), findings }) + expect(process.exitCode).toBe(1) +}) + +// Why the host is on stdout at all: the CLI normally targets a paired remote runtime, so a +// verdict that does not name a machine reads as one about the machine you meant. +it('names the host in the text path and leaves a clean audit exit code alone', async () => { + runDoctorMock.mockResolvedValue(0) + + await run('supervisor doctor') + + expect(stdout).toContain(`on ${hostname()} (this machine)`) + expect(process.exitCode).toBe(originalExitCode) +}) + +it('reports an unsupported platform as a bad request rather than a CLI crash', async () => { + printServiceMock.mockRejectedValue(new SupervisorServiceUnsupportedError('win32')) + + await expect(run('supervisor print', [['orcad', '/opt/orcad/orcad.js']])).rejects.toMatchObject({ + code: 'invalid_argument' + }) +}) + +// Why: the missing-`--orcad` refusal is a plain Error raised inside the generator, and its +// message is the only thing that names the flag — translating it would lose that. +it('leaves every other failure with its own message', async () => { + printServiceMock.mockRejectedValue( + new Error('this process is /opt/orca/cli/index.js, not orcad.js') + ) + + await expect(run('supervisor print')).rejects.toThrow('not orcad.js') +}) diff --git a/src/cli/index.test.ts b/src/cli/index.test.ts index 1b190d9dbde1..c966afb6fc27 100644 --- a/src/cli/index.test.ts +++ b/src/cli/index.test.ts @@ -42,7 +42,7 @@ vi.mock('child_process', async () => { import { COMMAND_SPECS, main } from './index' import { formatFlagHelp } from './help' -import { GLOBAL_FLAGS, specPaths } from './args' +import { GLOBAL_FLAGS, isCommandGroup, specPaths } from './args' import { okFixture, queueFixtures } from './test-fixtures' describe('COMMAND_SPECS collision check', () => { @@ -58,6 +58,19 @@ describe('COMMAND_SPECS collision check', () => { } }) + // Why: `orca --help` falls through printHelp to "Unknown command: " plus + // the whole root help, and exits 1, when the prefix is missing from isCommandGroup — while + // each subcommand's own --help keeps working, so nothing about the specs reveals the gap. + // Registration is exhaustive over the live tree; this keeps the next group from shipping + // without it. + it('registers every multi-segment command prefix as a command group', () => { + const ungrouped = [ + ...new Set(COMMAND_SPECS.filter((spec) => spec.path.length > 1).map((spec) => spec.path[0])) + ].filter((prefix) => !isCommandGroup([prefix])) + + expect(ungrouped).toEqual([]) + }) + it('allows every flag documented in command usage strings', () => { const flagPattern = /--([a-zA-Z0-9-]+)/g for (const spec of COMMAND_SPECS) { @@ -305,6 +318,33 @@ describe('orca root help', () => { logSpy.mockRestore() }) + it('lists the supervisor commands and answers `orca supervisor --help` as a group', async () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) + const priorExitCode = process.exitCode + + await main(['--help'], '/tmp/repo') + + expect(String(logSpy.mock.calls[0][0])).toContain( + 'supervisor print Print a service definition for running orcad on this machine' + ) + + logSpy.mockClear() + process.exitCode = 0 + await main(['supervisor', '--help'], '/tmp/repo') + + // Why the exit code is asserted: an unregistered group still prints something — the whole + // root help, behind "Unknown command: supervisor" — and only the 1 says it went wrong. + const groupHelp = String(logSpy.mock.calls[0][0]) + expect(groupHelp).toContain('orca supervisor ') + expect(groupHelp).toContain('Print a supervisor service definition') + expect(groupHelp).toContain('Check an installed orcad service') + expect(groupHelp).not.toContain('Unknown command') + expect(process.exitCode).toBe(0) + + process.exitCode = priorExitCode + logSpy.mockRestore() + }) + it('advertises computer-use capabilities discovery', async () => { const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) diff --git a/src/cli/root-help-text-primary.ts b/src/cli/root-help-text-primary.ts index 6f282c884fda..c45b2a584898 100644 --- a/src/cli/root-help-text-primary.ts +++ b/src/cli/root-help-text-primary.ts @@ -8,6 +8,10 @@ export const ROOT_HELP_TEXT_PRIMARY = [ ' serve Start a headless Orca runtime server', ' status Show app/runtime/graph readiness', '', + 'Supervisor:', + ' supervisor print Print a service definition for running orcad on this machine', + ' supervisor doctor Check an installed orcad service for settings that lose terminals', + '', 'Diagnostics:', ' diagnostics memory Collect a memory snapshot for Orca and managed terminals', '',