diff --git a/src/server/management/config-routes.ts b/src/server/management/config-routes.ts index adf7c639253..45cfee41f76 100644 --- a/src/server/management/config-routes.ts +++ b/src/server/management/config-routes.ts @@ -772,7 +772,7 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise checked, + spawnWorkerFn: (jobId, runChannel, runRestart) => + spawnGuiUpdateWorker(jobId, runChannel, runRestart, { resolveSystemdRun: () => systemdRun }), }) }); } catch (err) { if (err instanceof UpdateJobError) { diff --git a/src/update/job.ts b/src/update/job.ts index fd160f8d568..ebcf73a8706 100644 --- a/src/update/job.ts +++ b/src/update/job.ts @@ -61,6 +61,7 @@ import { type NpmCachePreflightReason, } from "./npm-cache-preflight.mjs"; import { guiUpdateWorkerCommand } from "./worker-launch"; +import type { WorkerLaunchContext } from "./worker-launch"; import { withoutSiblingMarker } from "../codex/sibling-start"; const RELEASE_NOTES_URL = "https://github.com/lidge-jun/opencodex/releases/latest"; @@ -568,6 +569,7 @@ export function spawnGuiUpdateWorker( jobId: string, channel: Channel, restart: boolean, + context: WorkerLaunchContext = {}, ): UpdateWorkerProcess { const args = selfLaunchArgv([ "__gui-update-worker", @@ -576,7 +578,7 @@ export function spawnGuiUpdateWorker( restart ? "restart" : "no-restart", ]); if (process.platform !== "win32") { - const launch = guiUpdateWorkerCommand(process.execPath, args); + const launch = guiUpdateWorkerCommand(process.execPath, args, context); return spawn(launch.command, launch.argv, { detached: true, stdio: "ignore", diff --git a/src/update/worker-launch.ts b/src/update/worker-launch.ts index 89811193e1f..3c964c3b489 100644 --- a/src/update/worker-launch.ts +++ b/src/update/worker-launch.ts @@ -1,4 +1,6 @@ -import { spawnSync } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; +import { accessSync, constants, realpathSync, statSync } from "node:fs"; +import { dirname, isAbsolute } from "node:path"; /** * How to launch the dashboard update worker on POSIX. @@ -16,19 +18,157 @@ 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; + /** Async variant of probeScope; resolveSystemdRunAsync prefers it when present. */ + probeScopeAsync?: (path: string) => Promise; +} + +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). +export interface SystemdRunTrustDeps { + /** Test seam: canonicalizes the candidate before its substitution chain is checked. */ + realpathSync?: (path: string) => string; + /** Test seam: stats a resolved path for ownership and mode. */ + statSync?: (path: string) => { isFile(): boolean; uid: number; mode: number }; + /** Test seam: checks the candidate's executable bit. */ + accessSync?: (path: string, mode: number) => void; +} + +function rootOnlyWritable(path: string, stat: SystemdRunTrustDeps["statSync"] = statSync): boolean { + try { + const st = stat!(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, deps: SystemdRunTrustDeps = {}): boolean { + try { + if (!isAbsolute(path)) return false; + (deps.accessSync ?? accessSync)(path, constants.X_OK); + // The lexical path may be a symlink. Checking the link's own parent only proves + // the *entry* is pinned; the file it resolves to — and every ancestor able to + // substitute that resolved file — is what the scope probe will actually exec. + const realpath = deps.realpathSync ?? realpathSync; + const resolved = realpath(path); + const stat = deps.statSync ?? statSync; + const st = stat(resolved); + if (!(st.isFile() && st.uid === 0 && (st.mode & GROUP_OR_WORLD_WRITE) === 0)) { + return false; + } + for (const start of [dirname(path), dirname(resolved)]) { + for (let dir = start, previous = ""; dir !== previous; previous = dir, dir = dirname(dir)) { + if (!rootOnlyWritable(dir, stat)) return false; + } + } + return true; + } catch { + return false; + } +} + +/** Scope discovery gets only user-bus identity, never inherited management credentials. */ +function scopeProbeEnvironment(): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = { PATH: "/usr/bin:/bin" }; + for (const name of ["HOME", "USER", "LOGNAME", "XDG_RUNTIME_DIR", "DBUS_SESSION_BUS_ADDRESS"]) { + if (process.env[name] !== undefined) env[name] = process.env[name]; + } + return env; +} + +const systemdRunHooks: SystemdRunHooks = { + isExecutableFile: isTrustedSystemdRunFile, + // Run a real scope with the same absolute binary as its harmless version payload. + // Probing only the outer --version would not verify the user bus. + probeScope: path => { + const probe = spawnSync(path, [...SYSTEMD_SCOPE_ARGS, path, "--version"], { stdio: "ignore", timeout: 5_000, env: scopeProbeEnvironment() }); + return !probe.error && probe.status === 0; + }, + probeScopeAsync: path => new Promise(resolve => { + const probe = spawn(path, [...SYSTEMD_SCOPE_ARGS, path, "--version"], { stdio: "ignore", env: scopeProbeEnvironment() }); + probe.unref(); + const timer = setTimeout(() => { + try { probe.kill("SIGKILL"); } catch { /* failed termination is not a successful probe */ } + resolve(false); + }, 5_000); + timer.unref(); + probe.once("error", () => { clearTimeout(timer); resolve(false); }); + probe.once("close", code => { clearTimeout(timer); resolve(code === 0); }); + }), +}; + +let systemdRunProbe: string | null | undefined; +let systemdRunProbePending: Promise | 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; +} + +/** + * Management-request path variant. The synchronous resolver blocks the shared + * event loop for up to four sequential five-second scope probes on first use; + * the dashboard update route awaits this instead, so probing overlaps other + * requests. Concurrent first callers share one probe pass. + */ +export async function resolveSystemdRunAsync(hooks: SystemdRunHooks = systemdRunHooks): Promise { + if (systemdRunProbe !== undefined) return systemdRunProbe ?? undefined; + if (!systemdRunProbePending) { + systemdRunProbePending = (async () => { + const probeScope = hooks.probeScopeAsync ?? (async (path: string) => hooks.probeScope(path)); + for (const command of TRUSTED_SYSTEMD_RUN_PATHS) { + if (!hooks.isExecutableFile(command)) continue; + if (await probeScope(command)) { + return command; + } + } + return null; + })(); + } + const found = await systemdRunProbePending; + // Honor a cache the sync resolver may have filled while the probe ran — the + // older observation wins so every caller converges on one launcher. + if (systemdRunProbe === undefined) systemdRunProbe = found; + return systemdRunProbe ?? undefined; +} + +export function resetSystemdRunProbeForTests(): void { + systemdRunProbe = undefined; + systemdRunProbePending = undefined; } export function guiUpdateWorkerCommand( @@ -39,8 +179,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 d8702d82b46..8a2d164a7f6 100644 --- a/structure/ops/service-and-sidecars.md +++ b/structure/ops/service-and-sidecars.md @@ -332,4 +332,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's resolved target — plus every ancestor directory able to substitute it — must be root-owned and not group/world-writable: a trusted-path symlink into a user-replaceable directory is skipped, as is a group-writable `/usr/local/bin`, 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. The management route resolves the launcher with `resolveSystemdRunAsync` before spawning, so first-request probing overlaps other work instead of blocking the event loop for up to twenty seconds. `--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..661c4fea581 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, + resolveSystemdRunAsync, 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,225 @@ 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(); + }); + + test("resolveSystemdRunAsync shares one probe pass across concurrent first callers", async () => { + resetSystemdRunProbeForTests(); + let probes = 0; + const hooks = { + isExecutableFile: () => true, + probeScope: () => { throw new Error("sync probe must not run on the request path"); }, + probeScopeAsync: async (path: string) => { + probes++; + await new Promise(resolve => setTimeout(resolve, 5)); + return path === "/bin/systemd-run"; + }, + }; + const [first, second, third] = await Promise.all([ + resolveSystemdRunAsync(hooks), + resolveSystemdRunAsync(hooks), + resolveSystemdRunAsync(hooks), + ]); + expect(first).toBe("/bin/systemd-run"); + expect(second).toBe("/bin/systemd-run"); + expect(third).toBe("/bin/systemd-run"); + expect(probes).toBe(2); + // The resolved value is now cached: the sync resolver agrees without probing again. + expect(resolveSystemdRun(hooks)).toBe("/bin/systemd-run"); + expect(probes).toBe(2); + 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(); } + }); +}); + +/* + * A trusted-path symlink is only as strong as the file it resolves to and the + * directories able to substitute that file. The link's own parent being + * root-only is not enough — these run against stub seams so the substitution + * chain is exercised without needing a uid-0 fixture on disk. + */ +describe("isTrustedSystemdRunFile (resolved substitution chain)", () => { + const fileStat = (mode: number, uid = 0) => ({ isFile: () => true, isDirectory: () => false, uid, mode }); + const dirStat = (mode: number, uid = 0) => ({ isFile: () => false, isDirectory: () => true, uid, mode }); + const trustedDeps = { + accessSync: () => {}, + statSync: (path: string) => dirStat(0o755), + realpathSync: (path: string) => path, + }; + + test("rejects a trusted-dir symlink whose resolved target can be substituted", () => { + // /usr/bin/systemd-run -> /home/user/bin/systemd-run: the file itself is + // root-owned and mode-pinned, but /home/user/bin is user-writable, so the + // user can replace it outright. + const deps = { + ...trustedDeps, + realpathSync: () => "/home/user/bin/systemd-run", + statSync: (path: string) => + path === "/home/user/bin/systemd-run" ? fileStat(0o755) + : path === "/home/user/bin" ? dirStat(0o775) + : dirStat(0o755), + }; + expect(isTrustedSystemdRunFile("/usr/bin/systemd-run", deps)).toBe(false); + }); + + test("rejects when any resolved ancestor can be substituted, not just the parent", () => { + // Target dir is pinned, but /opt/vendor is world-writable: swapping + // /opt/vendor/tools there substitutes the binary below it. + const deps = { + ...trustedDeps, + realpathSync: () => "/opt/vendor/tools/systemd-run", + statSync: (path: string) => + path === "/opt/vendor/tools/systemd-run" ? fileStat(0o755) + : path === "/opt/vendor" ? dirStat(0o777) + : dirStat(0o755), + }; + expect(isTrustedSystemdRunFile("/usr/bin/systemd-run", deps)).toBe(false); + }); + + test("accepts a resolved chain that is root-owned and pinned end to end", () => { + const deps = { + ...trustedDeps, + realpathSync: () => "/usr/lib/systemd/systemd-run", + statSync: (path: string) => + path === "/usr/lib/systemd/systemd-run" ? fileStat(0o755) : dirStat(0o755), + }; + expect(isTrustedSystemdRunFile("/usr/bin/systemd-run", deps)).toBe(true); + }); + + test("rejects a non-root resolved target even inside a pinned chain", () => { + const deps = { + ...trustedDeps, + realpathSync: () => "/usr/lib/systemd/systemd-run", + statSync: (path: string) => + path === "/usr/lib/systemd/systemd-run" ? fileStat(0o755, 1000) : dirStat(0o755), + }; + expect(isTrustedSystemdRunFile("/usr/bin/systemd-run", deps)).toBe(false); + }); +}); + +test("launcher trust also checks lexical ancestors of a canonical system target", () => { + const candidate = "/usr/local/bin/systemd-run"; + const target = "/nix/store/systemd/bin/systemd-run"; + const deps = (bad: string | undefined) => ({ realpathSync: () => target, accessSync: () => {}, + statSync: (path: string) => ({ isFile: () => path === target, uid: 0, mode: path === bad ? 0o777 : 0o755 }) }); + expect(isTrustedSystemdRunFile(candidate, deps(undefined))).toBe(true); + expect(isTrustedSystemdRunFile(candidate, deps("/usr/local"))).toBe(false); + expect(isTrustedSystemdRunFile(candidate, deps("/nix/store"))).toBe(false); + expect(isTrustedSystemdRunFile("relative/systemd-run", deps(undefined))).toBe(false); +});