Merge pull request #562: fix(cli-install): install npm CLIs to ~/.local when the npm global prefix is not writable

.changeset/install-npm-prefix-writable.md

@@ -0,0 +1,5 @@
+---
+'aicodeman': patch
+---
+
+Installing an npm-based CLI from Settings (DeepSeek's `dsh`, pi, ...) no longer fails with EACCES on a native install whose system node keeps its global prefix under `/usr`. When the npm global prefix is not writable by the Codeman user, the install now goes to `~/.local` (where Codeman already looks for CLIs), as the Docker deployment already did. A prefix you set yourself, or one that is writable, is left alone.

docs/cli-registry.md

@@ -25,7 +25,7 @@ Every run mode Codeman can launch — Claude Code, Terminal/Shell, OpenCode, Cod
 App Settings → Agents & CLIs → **CLI management** (`cliManagementEnabled`, default OFF; admin-only in multi-user mode) lists every entry with an installed/not-installed badge and:
 
 - toggles any entry on or off. A `kind: 'shell'` entry cannot be disabled, and the row shows no switch for it. A disabled CLI disappears from the Run menu, the welcome screen and the phone overview, and new session requests for it are rejected.
-- installs a missing **stock** CLI by running its shipped install command, after a confirm that names the exact command. Only one install per CLI runs at a time, and the command runs without any `CODEMAN_*` variable in its environment. A custom entry's install command is never executed.
+- installs a missing **stock** CLI by running its shipped install command, after a confirm that names the exact command. Only one install per CLI runs at a time, and the command runs without any `CODEMAN_*` variable in its environment. An `npm install -g` command is pointed at `~/.local` (which every resolver searches) when the npm global prefix is not writable by the server user, for example a system node under `/usr`; a writable prefix, a prefix that does not exist yet but could be created, and an explicit `NPM_CONFIG_PREFIX` are left alone. A custom entry's install command is never executed.
 - adds, edits and deletes **custom** entries (id, label, badge, binaries, launch argv). The server re-validates the whole assembled entry through `CliEntrySchema`, so the form cannot bypass the load-time rules.
 
 These are the only writes to `clis.json`. They are serialized, and a file that does not parse or has unsafe permissions is refused rather than overwritten; fix it (or `chmod 600` it) and retry. The HTTP routes are listed in `docs/api-reference.md` under *CLI management*.

src/web/routes/cli-registry-routes.ts

@@ -29,7 +29,11 @@
  * write path (`registry-writer.ts` mirrors `custom-model-hosts.ts`).
  */
 
-import { spawn } from 'node:child_process';
+import { execFile, spawn } from 'node:child_process';
+import { constants as fsConstants } from 'node:fs';
+import { access } from 'node:fs/promises';
+import { dirname, join } from 'node:path';
+import { promisify } from 'node:util';
 import type { FastifyInstance, FastifyRequest } from 'fastify';
 import { ApiErrorCode, createErrorResponse, getErrorMessage, type ApiResponse } from '../../types.js';
 import { getAuthUser, isAdmin, parseBody, readJsonConfig, SETTINGS_PATH } from '../route-helpers.js';
@@ -207,14 +211,65 @@ const CLI_INSTALL_TIMEOUT_MS = 300_000;
  */
 const installsInFlight = new Set<string>();
 
+const execFileAsync = promisify(execFile);
+
+/**
+ * True when `npm install -g` can write to this process's npm global prefix, or when that cannot
+ * be determined (then nothing is redirected: a wrong guess would move installs somewhere the
+ * user did not choose). `npm config get prefix` is asked rather than guessed from `process.execPath`
+ * because a user `.npmrc` / `NPM_CONFIG_PREFIX` can point it anywhere.
+ *
+ * Async so the server keeps serving while npm boots (130 to 240 ms), and killed with SIGKILL on
+ * timeout because `SIGTERM` alone leaves the wait running. A prefix that does not exist yet is
+ * judged by the nearest ancestor that does: npm creates the missing directories, so a user
+ * `.npmrc` pointing at `~/.npm-global` before it was made is not moved.
+ */
+export async function npmGlobalPrefixWritable(source: NodeJS.ProcessEnv): Promise<boolean> {
+  try {
+    const { stdout } = await execFileAsync('npm', ['config', 'get', 'prefix'], {
+      env: source,
+      encoding: 'utf8',
+      timeout: 5_000,
+      killSignal: 'SIGKILL',
+    });
+    const prefix = stdout.trim();
+    if (!prefix) return true;
+    // npm creates lib/node_modules under the prefix; walk up to the first directory that exists.
+    let dir = join(prefix, 'lib', 'node_modules');
+    for (;;) {
+      try {
+        await access(dir, fsConstants.W_OK);
+        return true;
+      } catch (err) {
+        if ((err as NodeJS.ErrnoException).code !== 'ENOENT') return false;
+        const parent = dirname(dir);
+        if (parent === dir) return false;
+        dir = parent;
+      }
+    }
+  } catch {
+    return true;
+  }
+}
+
 /**
  * The server's environment minus every `CODEMAN_*` variable. An install script is third-party
  * code, and those variables carry Codeman's own secrets and wiring (`CODEMAN_PASSWORD`, the
- * data dir, the tmux socket), none of which an installer needs. Inside the Docker Compose
- * container (`CODEMAN_IN_CONTAINER=1`) it also points `NPM_CONFIG_PREFIX` at `$HOME/.local`,
- * so an `npm install -g` lands on the persistent home mount instead of the image.
+ * data dir, the tmux socket), none of which an installer needs.
+ *
+ * It also points `NPM_CONFIG_PREFIX` at `$HOME/.local` so an `npm install -g` lands somewhere the
+ * server user can write and Codeman's resolvers already search (`~/.local/bin`):
+ *  - inside the Docker Compose container (`CODEMAN_IN_CONTAINER=1`), so installs survive an image
+ *    update (the image's own prefix is image content);
+ *  - on a native install whose npm global prefix is not writable by the server user (a system node
+ *    under `/usr`, installed by root). Without this `npm install -g` died with EACCES (exit 243),
+ *    e.g. DeepSeek's `npm install -g @deepseek-ai/dsh`. An explicit `NPM_CONFIG_PREFIX` the
+ *    operator set is respected, and so is a prefix that is writable (nvm, `~/.npm-global`, ...).
  */
-export function installEnv(source: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv {
+export function installEnv(
+  source: NodeJS.ProcessEnv = process.env,
+  prefixWritable: (env: NodeJS.ProcessEnv) => boolean = () => true
+): NodeJS.ProcessEnv {
   const env: NodeJS.ProcessEnv = {};
   for (const [key, value] of Object.entries(source)) {
     if (!key.startsWith('CODEMAN_')) env[key] = value;
@@ -224,11 +279,40 @@ export function installEnv(source: NodeJS.ProcessEnv = process.env): NodeJS.Proc
   // vanishes. HOME is the persistent bind mount and `~/.local/bin` is already on every resolver's search
   // list, so npm-based installs are redirected there. curl|bash installers already target HOME.
   if (source.CODEMAN_IN_CONTAINER === '1' && source.HOME) {
-    env.NPM_CONFIG_PREFIX = `${source.HOME}/.local`;
+    redirectNpmPrefix(env, source.HOME);
+  } else if (process.platform !== 'win32' && source.HOME && !source.NPM_CONFIG_PREFIX && !prefixWritable(env)) {
+    redirectNpmPrefix(env, source.HOME);
   }
   return env;
 }
 
+/**
+ * Point npm at `$HOME/.local`, dropping every spelling of the prefix key first. `npm run` exports a
+ * lowercase `npm_config_prefix`, npm reads `npm_config_*` case-insensitively, and when both spellings
+ * are present a `/bin/sh` that sorts its environment (bash) lets the older value win. The explicit
+ * operator guard in `installEnv` stays on the uppercase key only: npm always injects the lowercase one.
+ */
+function redirectNpmPrefix(env: NodeJS.ProcessEnv, home: string): void {
+  for (const key of Object.keys(env)) if (/^npm_config_prefix$/i.test(key)) delete env[key];
+  env.NPM_CONFIG_PREFIX = `${home}/.local`;
+}
+
+/**
+ * `installEnv` for this process, with the (async) npm prefix probe done first and only when the
+ * command runs npm at all: a `curl | bash` installer never pays for it.
+ */
+async function installEnvFor(command: string, source: NodeJS.ProcessEnv = process.env): Promise<NodeJS.ProcessEnv> {
+  const usesNpm = /\bnpm\b/.test(command);
+  const probeNeeded =
+    usesNpm &&
+    source.CODEMAN_IN_CONTAINER !== '1' &&
+    process.platform !== 'win32' &&
+    !!source.HOME &&
+    !source.NPM_CONFIG_PREFIX;
+  const writable = probeNeeded ? await npmGlobalPrefixWritable(installEnv(source)) : true;
+  return installEnv(source, () => writable);
+}
+
 interface InstallResult {
   code: number | null;
   output: string;
@@ -244,6 +328,7 @@ interface InstallResult {
  * CUSTOM entry can never reach this function at all — see the route's own guard below.
  */
 async function runInstallCommand(command: string): Promise<InstallResult> {
+  const env = await installEnvFor(command);
   return new Promise((resolve) => {
     let child: ReturnType<typeof spawn>;
     try {
@@ -255,7 +340,7 @@ async function runInstallCommand(command: string): Promise<InstallResult> {
         // out into package-manager children, and spawn's own `timeout` option signals
         // only the direct child, leaving survivors holding the pipes open forever.
         detached: true,
-        env: installEnv(),
+        env,
       });
     } catch (err) {
       resolve({ code: null, output: `spawn failed: ${getErrorMessage(err)}`, timedOut: false });

test/routes/cli-registry-routes.test.ts

@@ -12,10 +12,16 @@
  */
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import { EventEmitter } from 'node:events';
-import { chmodSync, mkdirSync, readFileSync, rmSync, writeFileSync, statSync } from 'node:fs';
+import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync, statSync } from 'node:fs';
+import { tmpdir } from 'node:os';
 import { dirname } from 'node:path';
 import { createRouteTestHarness } from './_route-test-utils.js';
-import { installEnv, registerCliRegistryRoutes, type CliListItem } from '../../src/web/routes/cli-registry-routes.js';
+import {
+  installEnv,
+  npmGlobalPrefixWritable,
+  registerCliRegistryRoutes,
+  type CliListItem,
+} from '../../src/web/routes/cli-registry-routes.js';
 import { SETTINGS_PATH } from '../../src/web/route-helpers.js';
 import { CreateSessionSchema } from '../../src/web/schemas.js';
 import { buildSpawnCommandFromRegistry } from '../../src/session-cli-registry-bridge.js';
@@ -711,7 +717,7 @@ describe('registry writes are serialized and never clobber a file the reader wou
     } finally {
       delete process.env.CODEMAN_TEST_SECRET;
     }
-    expect(installEnv({ CODEMAN_PASSWORD: 'x', HOME: '/h' })).toEqual({ HOME: '/h' });
+    expect(installEnv({ CODEMAN_PASSWORD: 'x', HOME: '/h' }, () => true)).toEqual({ HOME: '/h' });
   });
 
   it('redirects npm installs to the persistent HOME inside the Compose container', () => {
@@ -719,6 +725,91 @@ describe('registry writes are serialized and never clobber a file the reader wou
       installEnv({ CODEMAN_IN_CONTAINER: '1', HOME: '/home/codeman', NPM_CONFIG_PREFIX: '/opt/codeman-cli' })
     ).toEqual({ HOME: '/home/codeman', NPM_CONFIG_PREFIX: '/home/codeman/.local' });
   });
+
+  it('redirects npm installs to ~/.local on a native install whose global prefix is not writable', () => {
+    // A system node under /usr: `npm install -g` as the server user dies with EACCES (exit 243).
+    expect(installEnv({ HOME: '/home/dev', CODEMAN_PASSWORD: 'x' }, () => false)).toEqual({
+      HOME: '/home/dev',
+      NPM_CONFIG_PREFIX: '/home/dev/.local',
+    });
+  });
+
+  it('leaves a writable, explicit or undeterminable npm prefix alone on a native install', () => {
+    expect(installEnv({ HOME: '/home/dev' }, () => true)).toEqual({ HOME: '/home/dev' });
+    // An operator-set prefix wins even if it is not writable: it is theirs to fix.
+    expect(installEnv({ HOME: '/home/dev', NPM_CONFIG_PREFIX: '/opt/npm' }, () => false)).toEqual({
+      HOME: '/home/dev',
+      NPM_CONFIG_PREFIX: '/opt/npm',
+    });
+    // No HOME means nowhere to redirect to.
+    expect(installEnv({ PATH: '/usr/bin' }, () => false)).toEqual({ PATH: '/usr/bin' });
+  });
+
+  it('drops every spelling of npm_config_prefix when it redirects (npm run exports the lowercase one)', () => {
+    // npm reads npm_config_* case-insensitively and a sorting /bin/sh lets the older key win.
+    expect(installEnv({ HOME: '/h', npm_config_prefix: '/usr' }, () => false)).toEqual({
+      HOME: '/h',
+      NPM_CONFIG_PREFIX: '/h/.local',
+    });
+    expect(installEnv({ HOME: '/h', Npm_Config_Prefix: '/usr', NPM_CONFIG_PREFIX: '' }, () => false)).toEqual({
+      HOME: '/h',
+      NPM_CONFIG_PREFIX: '/h/.local',
+    });
+    expect(
+      installEnv({ CODEMAN_IN_CONTAINER: '1', HOME: '/h', npm_config_prefix: '/usr', NPM_CONFIG_PREFIX: '/opt/x' })
+    ).toEqual({ HOME: '/h', NPM_CONFIG_PREFIX: '/h/.local' });
+    // The operator guard is the uppercase key only: npm run always injects the lowercase one.
+    expect(installEnv({ HOME: '/h', npm_config_prefix: '/usr' }, () => true)).toEqual({
+      HOME: '/h',
+      npm_config_prefix: '/usr',
+    });
+  });
+
+  describe('npmGlobalPrefixWritable', () => {
+    let dir: string;
+    beforeEach(() => {
+      dir = mkdtempSync(`${tmpdir()}/npm-prefix-`);
+    });
+    afterEach(() => {
+      chmodSync(dir, 0o755);
+      rmSync(dir, { recursive: true, force: true });
+    });
+
+    /** A fake `npm` on PATH that prints `prefix` for `npm config get prefix`. */
+    function fakeNpm(prefix: string): NodeJS.ProcessEnv {
+      const bin = `${dir}/bin`;
+      mkdirSync(bin, { recursive: true });
+      writeFileSync(`${bin}/npm`, `#!/bin/sh\necho '${prefix}'\n`, { mode: 0o755 });
+      return { PATH: `${bin}:/usr/bin:/bin` };
+    }
+
+    it('is true for a writable prefix, whether or not lib/node_modules exists', async () => {
+      mkdirSync(`${dir}/w`);
+      expect(await npmGlobalPrefixWritable(fakeNpm(`${dir}/w`))).toBe(true);
+      mkdirSync(`${dir}/w/lib/node_modules`, { recursive: true });
+      expect(await npmGlobalPrefixWritable(fakeNpm(`${dir}/w`))).toBe(true);
+    });
+
+    it('is false for a prefix the server user cannot write', async () => {
+      if (process.getuid?.() === 0) return; // root can write anywhere
+      mkdirSync(`${dir}/ro`);
+      chmodSync(`${dir}/ro`, 0o555);
+      expect(await npmGlobalPrefixWritable(fakeNpm(`${dir}/ro`))).toBe(false);
+    });
+
+    it('judges a prefix that does not exist yet by its nearest existing ancestor', async () => {
+      // A user .npmrc with prefix=~/.npm-global before it was created: npm makes it, so do not move it.
+      expect(await npmGlobalPrefixWritable(fakeNpm(`${dir}/not/yet/made`))).toBe(true);
+      if (process.getuid?.() === 0) return;
+      mkdirSync(`${dir}/ro2`);
+      chmodSync(`${dir}/ro2`, 0o555);
+      expect(await npmGlobalPrefixWritable(fakeNpm(`${dir}/ro2/not/yet`))).toBe(false);
+    });
+
+    it('treats a probe failure (npm missing) as writable, and never throws', async () => {
+      expect(await npmGlobalPrefixWritable({ PATH: '/nonexistent' })).toBe(true);
+    });
+  });
 });
 
 /**