fix(cli): include node bin dir in service unit PATH for fnm/nvm/asdf/volta/mise (#1000)

* fix(cli): prepend node bin dir to service unit PATH so npx-based agents resolve systemd/launchd units hard-coded a system-only PATH. Under per-user node managers (fnm, nvm, asdf, volta, mise), node lives in a versioned subdirectory like ~/.local/share/fnm/node-versions/v24.14.0/installation/bin, so the daemon could exec node (ExecStart resolves the absolute path) but could not exec npx — every ACP agent built with `npx -y <bridge-pkg>` (claude, codex, opencode, …) failed at spawn time with "peer disconnected before response". `captureLauncher()` already records the node binary path. Its parent dir is exactly where the matching npm/npx live, so prepend it to the unit's PATH (with dedup so /usr/local/bin etc. don't appear twice). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(cli): guard pathWithNodeBinDir against non-POSIX nodePath `path.dirname` returns `'.'` on POSIX when given a path with no `/` separator (e.g. a Windows-style `C:\…\node.exe`). Prepending `.` to a service unit's PATH would put CWD ahead of system dirs in the daemon context — a latent privilege-escalation footgun if any future caller ever passed a relative or bare-filename nodePath. Production is safe because `captureLauncher()` uses `process.execPath` (always absolute on Linux/macOS), so the guard is defensive, not a behavior change. Adds regression tests for both systemd and launchd, plus a previously missing launchd system-mode + versioned-bin-dir test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-21 22:35:07 +01:00
parent d14cda778e
commit 8a4ac6c570
2 changed files with 154 additions and 2 deletions
@@ -17,6 +17,14 @@ const BREW_LAUNCHER: LauncherInfo = {
  version: "0.49.0",
 };

+// Versioned bin dir typical of per-user node managers (fnm/nvm/asdf/volta/mise).
+const FNM_LAUNCHER: LauncherInfo = {
+  nodePath: "/home/alice/.local/share/fnm/node-versions/v24.14.0/installation/bin/node",
+  cliEntry:
+    "/home/alice/.local/share/fnm/node-versions/v24.14.0/installation/lib/node_modules/kandev/bin/cli.js",
+  kind: "npm",
+};
+
 describe("renderSystemdUnit", () => {
  it("renders a user unit with absolute paths and --headless", () => {
    const unit = renderSystemdUnit({
@@ -69,6 +77,65 @@ describe("renderSystemdUnit", () => {
    expect(unit).toContain("Environment=KANDEV_SERVER_PORT=9000");
  });

+  it("prepends launcher node bin dir to PATH so npx-based agents resolve under fnm/nvm/asdf/volta/mise", () => {
+    const unit = renderSystemdUnit({
+      launcher: FNM_LAUNCHER,
+      homeDir: "/home/alice/.kandev",
+      logDir: "/home/alice/.kandev/logs",
+      mode: "user",
+    });
+    expect(unit).toContain(
+      "Environment=PATH=/home/alice/.local/share/fnm/node-versions/v24.14.0/installation/bin:/usr/local/bin:/usr/bin:/bin:/opt/homebrew/bin:/home/linuxbrew/.linuxbrew/bin",
+    );
+  });
+
+  it("does not duplicate node bin dir when it is already on the system PATH", () => {
+    const unit = renderSystemdUnit({
+      launcher: NPM_LAUNCHER,
+      homeDir: "/home/alice/.kandev",
+      logDir: "/home/alice/.kandev/logs",
+      mode: "user",
+    });
+    // /usr/local/bin already in SYSTEMD_PATH — dirname(nodePath) must not double it.
+    expect(unit).toContain(
+      "Environment=PATH=/usr/local/bin:/usr/bin:/bin:/opt/homebrew/bin:/home/linuxbrew/.linuxbrew/bin",
+    );
+    expect(unit).not.toContain("/usr/local/bin:/usr/local/bin");
+  });
+
+  it("prepends node bin dir for system-mode units too", () => {
+    const unit = renderSystemdUnit({
+      launcher: FNM_LAUNCHER,
+      homeDir: "/var/lib/kandev",
+      logDir: "/var/lib/kandev/logs",
+      mode: "system",
+      systemUser: "alice",
+    });
+    expect(unit).toContain(
+      "Environment=PATH=/home/alice/.local/share/fnm/node-versions/v24.14.0/installation/bin:/usr/local/bin:/usr/bin:/bin:/opt/homebrew/bin:/home/linuxbrew/.linuxbrew/bin",
+    );
+  });
+
+  it("does not prepend '.' when nodePath has no POSIX separator (Windows-style or bare filename)", () => {
+    // path.dirname('C:\\...\\node.exe') === '.' on POSIX. Without the
+    // isAbsolute guard, this would put CWD first in the daemon's PATH —
+    // a privilege-escalation footgun the systemd unit must not introduce.
+    const unit = renderSystemdUnit({
+      launcher: {
+        nodePath: 'C:\\Program Files\\node "Node"\\node.exe',
+        cliEntry: "/home/alice/cli.js",
+        kind: "unknown",
+      },
+      homeDir: "/home/alice/.kandev",
+      logDir: "/home/alice/.kandev/logs",
+      mode: "user",
+    });
+    expect(unit).not.toMatch(/^Environment=PATH=\.:/m);
+    expect(unit).toContain(
+      "Environment=PATH=/usr/local/bin:/usr/bin:/bin:/opt/homebrew/bin:/home/linuxbrew/.linuxbrew/bin",
+    );
+  });
+
  it("quotes ExecStart paths that contain spaces", () => {
    const unit = renderSystemdUnit({
      launcher: {
@@ -104,6 +171,69 @@ describe("renderLaunchdPlist", () => {
    expect(plist).not.toContain("KANDEV_BUNDLE_DIR");
  });

+  it("prepends launcher node bin dir to PATH for plists too (fnm/nvm/asdf/volta/mise)", () => {
+    const plist = renderLaunchdPlist({
+      launcher: {
+        nodePath: "/Users/alice/.volta/tools/image/node/24.14.0/bin/node",
+        cliEntry: "/Users/alice/.volta/tools/image/packages/kandev/bin/cli.js",
+        kind: "npm",
+      },
+      homeDir: "/Users/alice/.kandev",
+      logDir: "/Users/alice/.kandev/logs",
+      mode: "user",
+    });
+    expect(plist).toContain(
+      "<key>PATH</key>\n      <string>/Users/alice/.volta/tools/image/node/24.14.0/bin:/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin</string>",
+    );
+  });
+
+  it("does not duplicate node bin dir in plist PATH when already present", () => {
+    const plist = renderLaunchdPlist({
+      launcher: BREW_LAUNCHER,
+      homeDir: "/Users/alice/.kandev",
+      logDir: "/Users/alice/.kandev/logs",
+      mode: "user",
+    });
+    // /opt/homebrew/bin already first in LAUNCHD_PATH — must not be doubled.
+    expect(plist).toContain(
+      "<key>PATH</key>\n      <string>/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin</string>",
+    );
+  });
+
+  it("prepends node bin dir for system-mode LaunchDaemons too", () => {
+    const plist = renderLaunchdPlist({
+      launcher: {
+        nodePath: "/Users/alice/.volta/tools/image/node/24.14.0/bin/node",
+        cliEntry: "/Users/alice/.volta/tools/image/packages/kandev/bin/cli.js",
+        kind: "npm",
+      },
+      homeDir: "/Library/Application Support/kandev",
+      logDir: "/Library/Logs/kandev",
+      mode: "system",
+      systemUser: "_kandev",
+    });
+    expect(plist).toContain(
+      "<key>PATH</key>\n      <string>/Users/alice/.volta/tools/image/node/24.14.0/bin:/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin</string>",
+    );
+  });
+
+  it("does not prepend '.' to plist PATH when nodePath has no POSIX separator", () => {
+    const plist = renderLaunchdPlist({
+      launcher: {
+        nodePath: "C:\\Program Files\\node\\node.exe",
+        cliEntry: "/home/alice/cli.js",
+        kind: "unknown",
+      },
+      homeDir: "/Users/alice/.kandev",
+      logDir: "/Users/alice/.kandev/logs",
+      mode: "user",
+    });
+    expect(plist).not.toMatch(/<key>PATH<\/key>\s*<string>\.:/);
+    expect(plist).toContain(
+      "<key>PATH</key>\n      <string>/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin</string>",
+    );
+  });
+
  it("escapes XML special characters in paths", () => {
    const plist = renderLaunchdPlist({
      launcher: {
@@ -1,3 +1,5 @@
+import path from "node:path";
+
 import type { LauncherInfo } from "./paths";

 export type UnitInputs = {
@@ -20,6 +22,26 @@ const SYSTEMD_PATH =
  "/usr/local/bin:/usr/bin:/bin:/opt/homebrew/bin:/home/linuxbrew/.linuxbrew/bin";
 const LAUNCHD_PATH = "/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin";

+// Prepend the launcher node's bin dir so `npm`/`npx` resolve under per-user
+// node managers (fnm, nvm, asdf, volta, mise), where node lives in a versioned
+// subdirectory not covered by the system PATH. ExecStart already points at this
+// node, so its parent dir is exactly where the matching npm/npx live — without
+// this, npx-based ACP agents (claude, codex, opencode) fail to spawn.
+//
+// The isAbsolute guard exists because `path.dirname` on POSIX returns `'.'`
+// for a path with no `/` separator (e.g. a Windows-style `C:\…\node.exe`
+// passed through). Prepending `.` would put CWD ahead of system dirs in the
+// daemon's PATH — a privilege-escalation footgun. Production callers always
+// pass `process.execPath` (absolute on Linux/macOS), so the guard only kicks
+// in for non-POSIX or relative inputs from future callers / tests.
+function pathWithNodeBinDir(basePath: string, nodePath: string): string {
+  const nodeBinDir = path.dirname(nodePath);
+  if (!path.isAbsolute(nodeBinDir)) return basePath;
+  const parts = basePath.split(":");
+  if (parts.includes(nodeBinDir)) return basePath;
+  return `${nodeBinDir}:${basePath}`;
+}
+
 /**
 * Marker substring baked into every unit/plist kandev writes. Used to safely
 * detect "this file is ours, overwriting is fine" vs "user has put something
@@ -45,7 +67,7 @@ export function renderSystemdUnit(input: UnitInputs): string {
  const env: string[] = [
    envLine("KANDEV_HOME_DIR", input.homeDir),
    envLine("KANDEV_LOG_LEVEL", "info"),
-    envLine("PATH", SYSTEMD_PATH),
+    envLine("PATH", pathWithNodeBinDir(SYSTEMD_PATH, input.launcher.nodePath)),
  ];
  if (input.port !== undefined) {
    env.push(envLine("KANDEV_SERVER_PORT", String(input.port)));
@@ -94,7 +116,7 @@ export function renderLaunchdPlist(input: UnitInputs): string {
  const envEntries: Array<[string, string]> = [
    ["KANDEV_HOME_DIR", input.homeDir],
    ["KANDEV_LOG_LEVEL", "info"],
-    ["PATH", LAUNCHD_PATH],
+    ["PATH", pathWithNodeBinDir(LAUNCHD_PATH, input.launcher.nodePath)],
  ];
  if (input.port !== undefined) {
    envEntries.push(["KANDEV_SERVER_PORT", String(input.port)]);