diff --git a/src/update/worker-launch.ts b/src/update/worker-launch.ts index 89811193e1f..e003514ec0f 100644 --- a/src/update/worker-launch.ts +++ b/src/update/worker-launch.ts @@ -1,4 +1,6 @@ import { spawnSync } from "node:child_process"; +import { accessSync, constants, statSync } from "node:fs"; +import { dirname } from "node:path"; /** * How to launch the dashboard update worker on POSIX. @@ -16,19 +18,83 @@ export const SYSTEMD_SCOPE_ARGS = ["--user", "--scope", "--quiet", "--collect", export interface WorkerLaunchContext { platform?: NodeJS.Platform; env?: NodeJS.ProcessEnv; - hasSystemdRun?: () => boolean; + resolveSystemdRun?: () => string | undefined; } -let systemdRunProbe: boolean | undefined; +// Absolute install paths only — PATH is never consulted, so a caller-controlled entry cannot +// redirect the launch. `/usr/local/bin` is where systemd lands when built or stowed outside the +// distro layout, and `/run/current-system/sw/bin` is the NixOS layout, where the binary lives +// nowhere else even though the user bus works. A candidate only counts when the binary and its +// directory are root-owned and not group/world-writable, so a lower-trust local actor cannot +// plant the launcher the scope probe execs. +const TRUSTED_SYSTEMD_RUN_PATHS = [ + "/usr/bin/systemd-run", "/bin/systemd-run", "/usr/local/bin/systemd-run", + "/run/current-system/sw/bin/systemd-run", +] as const; -function probeSystemdRun(): boolean { +export interface SystemdRunHooks { + isExecutableFile: (path: string) => boolean; + probeScope: (path: string) => boolean; +} + +const GROUP_OR_WORLD_WRITE = 0o022; + +// stat (follow) rather than lstat: a root-owned symlink to a user-writable directory must fail +// on the target's mode, not pass on the symlink's (mirrors isTrustedSystemPath in +// src/codex/desktop-app/linux.ts). +function rootOnlyWritable(path: string): boolean { + try { + const st = statSync(path); + return st.uid === 0 && (st.mode & GROUP_OR_WORLD_WRITE) === 0; + } catch { + return false; + } +} + +// "Executable" here includes trust: the binary and its directory must be root-owned and not +// group/world-writable. /usr/local/bin is group-writable on some systems, and a planted or +// replaced systemd-run there would be exec'd by the scope probe under the service account; +// the fallback is the plain detached spawn, so nothing breaks when it is skipped. +// Exported for unit tests. +export function isTrustedSystemdRunFile(path: string): boolean { + try { + accessSync(path, constants.X_OK); + const st = statSync(path); + return st.isFile() && st.uid === 0 && (st.mode & GROUP_OR_WORLD_WRITE) === 0 + && rootOnlyWritable(dirname(path)); + } catch { + return false; + } +} + +const systemdRunHooks: SystemdRunHooks = { + isExecutableFile: isTrustedSystemdRunFile, + // Run a real no-op scope rather than `--version`: a present binary without a reachable user + // bus would otherwise pass the probe and then fail to start the worker at all. + probeScope: path => { + const probe = spawnSync(path, [...SYSTEMD_SCOPE_ARGS, "true"], { stdio: "ignore", timeout: 5_000 }); + return !probe.error && probe.status === 0; + }, +}; + +let systemdRunProbe: string | null | undefined; + +export function resolveSystemdRun(hooks: SystemdRunHooks = systemdRunHooks): string | undefined { if (systemdRunProbe === undefined) { - // Run a real no-op scope rather than `--version`: a present binary without a reachable user - // bus would otherwise pass the probe and then fail to start the worker at all. - const probe = spawnSync("systemd-run", [...SYSTEMD_SCOPE_ARGS, "true"], { stdio: "ignore", timeout: 5_000 }); - systemdRunProbe = !probe.error && probe.status === 0; + systemdRunProbe = null; + for (const command of TRUSTED_SYSTEMD_RUN_PATHS) { + if (!hooks.isExecutableFile(command)) continue; + if (hooks.probeScope(command)) { + systemdRunProbe = command; + break; + } + } } - return systemdRunProbe; + return systemdRunProbe ?? undefined; +} + +export function resetSystemdRunProbeForTests(): void { + systemdRunProbe = undefined; } export function guiUpdateWorkerCommand( @@ -39,8 +105,9 @@ export function guiUpdateWorkerCommand( const platform = context.platform ?? process.platform; const env = context.env ?? process.env; const underSystemd = platform === "linux" && Boolean(env.INVOCATION_ID); - if (underSystemd && (context.hasSystemdRun ?? probeSystemdRun)()) { - return { command: "systemd-run", argv: [...SYSTEMD_SCOPE_ARGS, execPath, ...args] }; + const systemdRun = underSystemd ? (context.resolveSystemdRun ?? resolveSystemdRun)() : undefined; + if (systemdRun) { + return { command: systemdRun, argv: [...SYSTEMD_SCOPE_ARGS, execPath, ...args] }; } return { command: execPath, argv: [...args] }; } diff --git a/structure/ops/service-and-sidecars.md b/structure/ops/service-and-sidecars.md index 389e1d690cb..43350cd7a53 100644 --- a/structure/ops/service-and-sidecars.md +++ b/structure/ops/service-and-sidecars.md @@ -276,4 +276,4 @@ src/update/async-check.ts uses the existing owner-bound registry target with a b The desktop badge snapshot in src/update/desktop-badge.ts is process-local display state keyed by a Tauri session id. A 60-second shell heartbeat renews receipt time; entries expire after 180 seconds and the store retains at most 32 sessions. It is separate from the package version cache and from the updater job/ownership transaction. A proxy restart reports unknown until a bound desktop shell republishes; no update installation can be authorized by this snapshot. -On Linux, a dashboard update worker started from the systemd user service is launched through `systemd-run --user --scope --quiet --collect` (`src/update/worker-launch.ts`), so it leaves the service cgroup before the updater stops `opencodex-proxy.service`; the default `KillMode=control-group` otherwise kills it with the proxy (#5750). The path applies only when `INVOCATION_ID` is set and a no-op scope probe succeeds; every other case keeps the plain detached spawn. `--scope` moves `systemd-run` itself into the scope and then execs the worker, so the recorded PID is the worker's (`tests/update/update-worker-launch.test.ts`). +On Linux, a dashboard update worker started from the systemd user service is launched through an executable regular file at a trusted absolute path — `/usr/bin/systemd-run`, `/bin/systemd-run`, `/usr/local/bin/systemd-run` (local installs), or `/run/current-system/sw/bin/systemd-run` (the NixOS layout) — with `--user --scope --quiet --collect` (`src/update/worker-launch.ts`), so it leaves the service cgroup before the updater stops `opencodex-proxy.service`; the default `KillMode=control-group` otherwise kills it with the proxy (#5750). The inherited `PATH` is never searched, and each candidate must be a root-owned regular file in a directory that is also root-owned and not group/world-writable — a group-writable `/usr/local/bin` is skipped rather than exec'd under the service account. Candidates are tried in order and a path whose no-op scope probe fails falls through to the next trusted path; the probe applies only when `INVOCATION_ID` is set, and every other case keeps the plain detached spawn. `--scope` moves `systemd-run` itself into the scope and then execs the worker, so the recorded PID is the worker's (`tests/update/update-worker-launch.test.ts`). diff --git a/tests/update/update-worker-launch.test.ts b/tests/update/update-worker-launch.test.ts index 4132df54912..b9a66a4ec88 100644 --- a/tests/update/update-worker-launch.test.ts +++ b/tests/update/update-worker-launch.test.ts @@ -1,5 +1,12 @@ import { describe, expect, test } from "bun:test"; -import { guiUpdateWorkerCommand, SYSTEMD_SCOPE_ARGS } from "../../src/update/worker-launch"; +import { chmodSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + guiUpdateWorkerCommand, isTrustedSystemdRunFile, resolveSystemdRun, resetSystemdRunProbeForTests, + SYSTEMD_SCOPE_ARGS, +} from "../../src/update/worker-launch"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; // #5750: a worker spawned by the systemd user service must leave the service cgroup before the // updater stops that service, or systemd kills it along with the proxy. @@ -8,23 +15,122 @@ describe("dashboard update worker launch", () => { test("a systemd-started Linux proxy launches the worker in its own scope", () => { const launch = guiUpdateWorkerCommand("/usr/bin/bun", args, { - platform: "linux", env: { INVOCATION_ID: "abc" }, hasSystemdRun: () => true, + platform: "linux", env: { INVOCATION_ID: "abc", PATH: "/tmp/attacker:/usr/bin" }, + resolveSystemdRun: () => "/usr/bin/systemd-run", + }); + expect(launch).toEqual({ + command: "/usr/bin/systemd-run", argv: [...SYSTEMD_SCOPE_ARGS, "/usr/bin/bun", ...args], }); - expect(launch).toEqual({ command: "systemd-run", argv: [...SYSTEMD_SCOPE_ARGS, "/usr/bin/bun", ...args] }); }); test("without systemd-run, outside systemd, or off Linux the spawn is unchanged", () => { const plain = { command: "/usr/bin/bun", argv: args }; expect(guiUpdateWorkerCommand("/usr/bin/bun", args, { - platform: "linux", env: { INVOCATION_ID: "abc" }, hasSystemdRun: () => false, + platform: "linux", env: { INVOCATION_ID: "abc" }, resolveSystemdRun: () => undefined, })).toEqual(plain); let probed = false; expect(guiUpdateWorkerCommand("/usr/bin/bun", args, { - platform: "linux", env: {}, hasSystemdRun: () => { probed = true; return true; }, + platform: "linux", env: {}, resolveSystemdRun: () => { probed = true; return "/usr/bin/systemd-run"; }, })).toEqual(plain); expect(probed).toBe(false); expect(guiUpdateWorkerCommand("/usr/bin/bun", args, { - platform: "darwin", env: { INVOCATION_ID: "abc" }, hasSystemdRun: () => true, + platform: "darwin", env: { INVOCATION_ID: "abc" }, resolveSystemdRun: () => "/usr/bin/systemd-run", })).toEqual(plain); }); }); + +// The real resolver — not the context seam — must be the thing under test: PATH must stay +// unconsulted, only the trusted absolute candidates may be probed, and a failed probe must +// fall through rather than settle for the plain in-cgroup spawn. +describe("trusted systemd-run discovery", () => { + test("walks only the trusted candidates and ignores PATH", () => { + resetSystemdRunProbeForTests(); + const seen: string[] = []; + const found = resolveSystemdRun({ + isExecutableFile: path => { seen.push(path); return path === "/run/current-system/sw/bin/systemd-run"; }, + probeScope: () => true, + }); + expect(found).toBe("/run/current-system/sw/bin/systemd-run"); + expect(seen).toEqual([ + "/usr/bin/systemd-run", "/bin/systemd-run", "/usr/local/bin/systemd-run", + "/run/current-system/sw/bin/systemd-run", + ]); + expect(seen.every(path => path.startsWith("/"))).toBe(true); + }); + + test("a failed scope probe falls through to the next candidate", () => { + resetSystemdRunProbeForTests(); + const found = resolveSystemdRun({ + isExecutableFile: () => true, + probeScope: path => path !== "/usr/bin/systemd-run", + }); + expect(found).toBe("/bin/systemd-run"); + }); + + test("the probe is cached and reports undefined when nothing qualifies", () => { + resetSystemdRunProbeForTests(); + let calls = 0; + const hooks = { + isExecutableFile: () => { calls++; return false; }, + probeScope: () => { throw new Error("must not run"); }, + }; + expect(resolveSystemdRun(hooks)).toBeUndefined(); + expect(resolveSystemdRun(hooks)).toBeUndefined(); + expect(calls).toBe(4); + resetSystemdRunProbeForTests(); + }); +}); + +// The default trust check must run against the real filesystem, not a stubbed seam. uid/mode +// semantics are POSIX-only — on Windows statSync reports uid 0 and chmod is a no-op — and only +// a root-run suite can create a uid-0 fixture, so each case is gated on what the test user can +// actually arrange. +describe("isTrustedSystemdRunFile (real filesystem)", () => { + const posix = process.platform !== "win32"; + const itPosix = posix ? test : test.skip; + const getuid = (process as { getuid?: () => number }).getuid?.bind(process); + const itNonRoot = posix && getuid?.() !== 0 ? test : test.skip; + const itRoot = posix && getuid?.() === 0 ? test : test.skip; + + function fixture(): { dir: string; file: string; cleanup: () => void } { + const dir = mkdtempSync(join(tmpdir(), "ocx-systemd-run-trust-")); + const file = join(dir, "systemd-run"); + writeFileSync(file, "#!/bin/sh\nexit 0\n"); + chmodSync(file, 0o755); + return { dir, file, cleanup: () => removeTreeWithRetry(dir) }; + } + + itNonRoot("rejects an executable owned by the test user rather than root", () => { + const { file, cleanup } = fixture(); + try { expect(isTrustedSystemdRunFile(file)).toBe(false); } finally { cleanup(); } + }); + + itNonRoot("rejects non-executable and missing paths", () => { + const { dir, file, cleanup } = fixture(); + try { + chmodSync(file, 0o644); + expect(isTrustedSystemdRunFile(file)).toBe(false); + expect(isTrustedSystemdRunFile(join(dir, "absent"))).toBe(false); + expect(isTrustedSystemdRunFile(dir)).toBe(false); + } finally { cleanup(); } + }); + + itRoot("rejects a root-owned file inside a group/world-writable directory", () => { + const { dir, file, cleanup } = fixture(); + try { + chmodSync(dir, 0o777); + expect(isTrustedSystemdRunFile(file)).toBe(false); + } finally { + chmodSync(dir, 0o700); + cleanup(); + } + }); + + itRoot("accepts a root-owned executable in a root-only-writable directory", () => { + const { dir, file, cleanup } = fixture(); + try { + chmodSync(dir, 0o755); + expect(isTrustedSystemdRunFile(file)).toBe(true); + } finally { cleanup(); } + }); +});