fix(update): avoid PATH lookup for systemd-run

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] };
 }

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`).

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(); }
+  });
+});