From 26487f0ec57df3b6f8c2f8cb41d7cce4e56de918 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Thu, 8 Oct 2026 20:41:03 +0800 Subject: [PATCH 1/2] fix(cli-install): install npm CLIs to ~/.local when the npm global prefix is not writable installEnv() only redirected NPM_CONFIG_PREFIX inside the Docker container (CODEMAN_IN_CONTAINER=1). On a native install with a root-owned system node (prefix /usr) `npm install -g @deepseek-ai/dsh` run as the server user died with EACCES (exit 243), so DeepSeek, pi and other npm-based CLIs could not be installed from Settings. Ask npm for its global prefix and, when the server user cannot write it, redirect to $HOME/.local like the container path does. An operator-set NPM_CONFIG_PREFIX and a writable prefix are left alone. --- .changeset/install-npm-prefix-writable.md | 5 +++ src/web/routes/cli-registry-routes.ts | 54 ++++++++++++++++++++--- test/routes/cli-registry-routes.test.ts | 35 ++++++++++++++- 3 files changed, 87 insertions(+), 7 deletions(-) create mode 100644 .changeset/install-npm-prefix-writable.md diff --git a/.changeset/install-npm-prefix-writable.md b/.changeset/install-npm-prefix-writable.md new file mode 100644 index 00000000..75143f4c --- /dev/null +++ b/.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. diff --git a/src/web/routes/cli-registry-routes.ts b/src/web/routes/cli-registry-routes.ts index 3b75ffda..88f076ee 100644 --- a/src/web/routes/cli-registry-routes.ts +++ b/src/web/routes/cli-registry-routes.ts @@ -29,7 +29,9 @@ * write path (`registry-writer.ts` mirrors `custom-model-hosts.ts`). */ -import { spawn } from 'node:child_process'; +import { execFileSync, spawn } from 'node:child_process'; +import { accessSync, constants as fsConstants } from 'node:fs'; +import { join } from 'node:path'; 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 +209,54 @@ const CLI_INSTALL_TIMEOUT_MS = 300_000; */ const installsInFlight = new Set(); +/** + * 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. + */ +export function npmGlobalPrefixWritable(source: NodeJS.ProcessEnv): boolean { + try { + const prefix = execFileSync('npm', ['config', 'get', 'prefix'], { + env: source, + encoding: 'utf8', + timeout: 5_000, + stdio: ['ignore', 'pipe', 'ignore'], + }).trim(); + if (!prefix) return true; + // npm creates lib/node_modules under the prefix; check that directory when it exists, else the prefix. + for (const dir of [join(prefix, 'lib', 'node_modules'), prefix]) { + try { + accessSync(dir, fsConstants.W_OK); + return true; + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== 'ENOENT') return false; + } + } + return false; + } 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 = npmGlobalPrefixWritable +): NodeJS.ProcessEnv { const env: NodeJS.ProcessEnv = {}; for (const [key, value] of Object.entries(source)) { if (!key.startsWith('CODEMAN_')) env[key] = value; @@ -225,6 +267,8 @@ export function installEnv(source: NodeJS.ProcessEnv = process.env): NodeJS.Proc // 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`; + } else if (process.platform !== 'win32' && source.HOME && !source.NPM_CONFIG_PREFIX && !prefixWritable(env)) { + env.NPM_CONFIG_PREFIX = `${source.HOME}/.local`; } return env; } diff --git a/test/routes/cli-registry-routes.test.ts b/test/routes/cli-registry-routes.test.ts index bc33b1f8..03fe2650 100644 --- a/test/routes/cli-registry-routes.test.ts +++ b/test/routes/cli-registry-routes.test.ts @@ -15,7 +15,12 @@ import { EventEmitter } from 'node:events'; import { chmodSync, mkdirSync, readFileSync, rmSync, writeFileSync, statSync } from 'node:fs'; 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 +716,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 +724,32 @@ 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('npmGlobalPrefixWritable follows npm config and treats a probe failure as writable', () => { + // Real probe against this machine: must return a boolean and never throw. + expect(typeof npmGlobalPrefixWritable({ PATH: process.env.PATH })).toBe('boolean'); + // npm not found on PATH: cannot tell, so do not redirect. + expect(npmGlobalPrefixWritable({ PATH: '/nonexistent' })).toBe(true); + }); }); /** From c2e55fc21006e7f8d9ac9fd7c50181f2be1de23e Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:42:06 +0800 Subject: [PATCH 2/2] fix(cli-install): address review: drop every npm_config_prefix spelling, probe async - Delete every spelling of npm_config_prefix before setting NPM_CONFIG_PREFIX, in the Compose branch too: npm run exports the lowercase key and a sorting /bin/sh let it win. The operator guard stays on the uppercase key only. - A prefix that does not exist yet is judged by its nearest existing ancestor. - The probe is async (promisified execFile, fs.promises.access, SIGKILL on timeout), awaited before the spawn and only for commands that run npm. - Tests: lowercase/mixed-case keys, and the decision logic against a fake npm on PATH (writable, read-only, not yet created, npm missing). Docs: one clause in cli-registry.md. --- docs/cli-registry.md | 2 +- src/web/routes/cli-registry-routes.ts | 71 ++++++++++++++++++------ test/routes/cli-registry-routes.test.ts | 72 ++++++++++++++++++++++--- 3 files changed, 123 insertions(+), 22 deletions(-) diff --git a/docs/cli-registry.md b/docs/cli-registry.md index 10e49c6b..e9ef4bcb 100644 --- a/docs/cli-registry.md +++ b/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*. diff --git a/src/web/routes/cli-registry-routes.ts b/src/web/routes/cli-registry-routes.ts index 88f076ee..d0a9f116 100644 --- a/src/web/routes/cli-registry-routes.ts +++ b/src/web/routes/cli-registry-routes.ts @@ -29,9 +29,11 @@ * write path (`registry-writer.ts` mirrors `custom-model-hosts.ts`). */ -import { execFileSync, spawn } from 'node:child_process'; -import { accessSync, constants as fsConstants } from 'node:fs'; -import { join } from 'node:path'; +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'; @@ -209,31 +211,42 @@ const CLI_INSTALL_TIMEOUT_MS = 300_000; */ const installsInFlight = new Set(); +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 function npmGlobalPrefixWritable(source: NodeJS.ProcessEnv): boolean { +export async function npmGlobalPrefixWritable(source: NodeJS.ProcessEnv): Promise { try { - const prefix = execFileSync('npm', ['config', 'get', 'prefix'], { + const { stdout } = await execFileAsync('npm', ['config', 'get', 'prefix'], { env: source, encoding: 'utf8', timeout: 5_000, - stdio: ['ignore', 'pipe', 'ignore'], - }).trim(); + killSignal: 'SIGKILL', + }); + const prefix = stdout.trim(); if (!prefix) return true; - // npm creates lib/node_modules under the prefix; check that directory when it exists, else the prefix. - for (const dir of [join(prefix, 'lib', 'node_modules'), prefix]) { + // 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 { - accessSync(dir, fsConstants.W_OK); + 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; } } - return false; } catch { return true; } @@ -255,7 +268,7 @@ export function npmGlobalPrefixWritable(source: NodeJS.ProcessEnv): boolean { */ export function installEnv( source: NodeJS.ProcessEnv = process.env, - prefixWritable: (env: NodeJS.ProcessEnv) => boolean = npmGlobalPrefixWritable + prefixWritable: (env: NodeJS.ProcessEnv) => boolean = () => true ): NodeJS.ProcessEnv { const env: NodeJS.ProcessEnv = {}; for (const [key, value] of Object.entries(source)) { @@ -266,13 +279,40 @@ export function installEnv( // 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)) { - env.NPM_CONFIG_PREFIX = `${source.HOME}/.local`; + 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 { + 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; @@ -288,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 { + const env = await installEnvFor(command); return new Promise((resolve) => { let child: ReturnType; try { @@ -299,7 +340,7 @@ async function runInstallCommand(command: string): Promise { // 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 }); diff --git a/test/routes/cli-registry-routes.test.ts b/test/routes/cli-registry-routes.test.ts index 03fe2650..ba5ec7d9 100644 --- a/test/routes/cli-registry-routes.test.ts +++ b/test/routes/cli-registry-routes.test.ts @@ -12,7 +12,8 @@ */ 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 { @@ -744,11 +745,70 @@ describe('registry writes are serialized and never clobber a file the reader wou expect(installEnv({ PATH: '/usr/bin' }, () => false)).toEqual({ PATH: '/usr/bin' }); }); - it('npmGlobalPrefixWritable follows npm config and treats a probe failure as writable', () => { - // Real probe against this machine: must return a boolean and never throw. - expect(typeof npmGlobalPrefixWritable({ PATH: process.env.PATH })).toBe('boolean'); - // npm not found on PATH: cannot tell, so do not redirect. - expect(npmGlobalPrefixWritable({ PATH: '/nonexistent' })).toBe(true); + 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); + }); }); });