From db4557d94b631631f2e06365f8eb66bf11da919b Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Tue, 22 Sep 2026 06:29:57 +0800 Subject: [PATCH] feat(cli-registry): Phases 3-6 - write API + custom entries + Settings UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes docs/cli-enable-disable-plan.md ("PR C" from the #343 review). Phase 3: PUT /api/clis/:id toggles enabled for any EXISTING entry (stock or custom) via a shallow merge onto its clis.json override; shell/claude are structurally un-disableable (Decision 4), an unknown id 404s rather than becoming a creation backdoor. Phase 4: POST /api/clis/:id/install runs a STOCK entry's already-vetted install command (shell:true, bounded by timeout, process-group killed on expiry, output captured, audit-logged). A custom entry's id is refused outright, independent of anything Phase 5 does (Decision 3: a custom entry's install text is display-only, never executed). Phase 5: POST /api/clis (create) / PUT /api/clis/custom/:id (update) / DELETE /api/clis/:id (custom only) — a deliberately minimal request shape (id/label/shortBadge/binaries/a simple launch variant), assembled into a full CliEntry with conservative capability defaults and re-validated through CliEntrySchema before writing, never a relaxed path for UI-originated entries. Stock-id collisions, duplicate custom ids, and edits/deletes against a stock id are all rejected explicitly. Phase 6: the Settings UI section (App Settings -> Agents & CLIs), gated independently on cliManagementEnabled AND admin-in-multi-user-mode (Decision 5), fetching/rendering GET /api/clis and wiring every write endpoint above. Every write endpoint answers the same way when the feature is off: 403 FORBIDDEN via one shared requireCliManagementGate() (Phase 1's own checklist item). registry-writer.ts is a new, deliberately separate write module so registry.ts itself stays import-side-effect-free, same tmp+ rename+0600 shape as custom-model-hosts.ts. 27 new/updated route tests covering every gate, collision, and cleanup path; full CI gate green (415/416 files, 7854 tests). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n --- src/config/cli-registry/registry-writer.ts | 53 +++ src/config/cli-registry/registry.ts | 10 + src/web/public/index.html | 46 ++- src/web/public/settings-ui.js | 226 +++++++++++ src/web/routes/cli-registry-routes.ts | 435 ++++++++++++++++++++- src/web/schemas.ts | 33 ++ test/routes/cli-registry-routes.test.ts | 354 ++++++++++++++++- 7 files changed, 1132 insertions(+), 25 deletions(-) create mode 100644 src/config/cli-registry/registry-writer.ts diff --git a/src/config/cli-registry/registry-writer.ts b/src/config/cli-registry/registry-writer.ts new file mode 100644 index 00000000..045dc3cb --- /dev/null +++ b/src/config/cli-registry/registry-writer.ts @@ -0,0 +1,53 @@ +/** + * @fileoverview Write side of the CLI registry (docs/cli-enable-disable-plan.md, Phases 3/5). + * + * Kept deliberately SEPARATE from `registry.ts`, which documents itself as read-only and + * whose whole point is that importing it (which `schemas.ts` does, transitively) performs no + * filesystem writes. Only `cli-registry-routes.ts` imports this module, so that property + * still holds for every OTHER importer of the registry. + * + * Same tmp+rename+0600 shape as `custom-model-hosts.ts`: `~/.codeman/clis.json` can be + * hand-edited, so a write must never leave it half-written, and 0600 is the mode + * `registry.ts`'s own `isUnsafePermissions()` check requires on the next read. + */ + +import { existsSync, mkdirSync } from 'node:fs'; +import fs from 'node:fs/promises'; +import { dirname } from 'node:path'; +import { registryFilePath } from './registry.js'; +import type { CliRegistryFile } from './types.js'; + +/** + * Best-effort read of the raw override file for mutation. Tolerant of "missing" and + * "unparseable" alike — both start a fresh `{schemaVersion: 1, clis: {}}` rather than + * failing the write, since the quarantine-on-corrupt-JSON behaviour belongs to the READ + * path (`registry.ts`'s `readRegistryFile`) and a write here should not fight it over the + * same file. A permissions problem is left to the read path to warn about on next load; + * this writer always emits 0600 regardless of what it found. + */ +export async function readRegistryFileForWrite(): Promise { + try { + const raw = await fs.readFile(registryFilePath(), 'utf-8'); + const parsed = JSON.parse(raw) as unknown; + if ( + typeof parsed === 'object' && + parsed !== null && + typeof (parsed as { clis?: unknown }).clis === 'object' && + (parsed as { clis?: unknown }).clis !== null + ) { + return parsed as CliRegistryFile; + } + } catch { + /* missing or invalid — start fresh, matching registry.ts's own tolerant defaults */ + } + return { schemaVersion: 1, clis: {} }; +} + +export async function writeRegistryFile(file: CliRegistryFile): Promise { + const target = registryFilePath(); + const dir = dirname(target); + if (!existsSync(dir)) mkdirSync(dir, { recursive: true }); + const tmp = `${target}.${process.pid}.tmp`; + await fs.writeFile(tmp, JSON.stringify(file, null, 2), { mode: 0o600 }); + await fs.rename(tmp, target); +} diff --git a/src/config/cli-registry/registry.ts b/src/config/cli-registry/registry.ts index cd9c590c..38d2c58f 100644 --- a/src/config/cli-registry/registry.ts +++ b/src/config/cli-registry/registry.ts @@ -48,6 +48,16 @@ function filePath(): string { return dataPath('clis.json'); } +/** + * The resolved path of `~/.codeman/clis.json`, exported for the write API + * (`cli-registry-writer.ts`, docs/cli-enable-disable-plan.md Phases 3/5) so both the read and + * write sides resolve the SAME path through the SAME instance-scoped helper — never a second + * `dataPath('clis.json')` call that could drift from this one under a future `dataPath()` change. + */ +export function registryFilePath(): string { + return filePath(); +} + /** * Keys that must never be merged out of a hand-editable JSON file. * diff --git a/src/web/public/index.html b/src/web/public/index.html index aa820c4e..c2b637b5 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -2372,11 +2372,55 @@ Enable CLI management Adds the list below and its write endpoints. Off by default: this changes machine configuration, not just what you see. - + + +

Claude

synced
diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index f6111120..754c7a02 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -404,6 +404,10 @@ Object.assign(CodemanApp.prototype, { this.applyCustomModelEndpointsVisibility(); // CLI management (docs/cli-enable-disable-plan.md): synced, default OFF. document.getElementById('appSettingsCliManagement').checked = settings.cliManagementEnabled === true; + // Same reasoning as applyCustomModelEndpointsVisibility above: assigning + // .checked fires no onchange, so the list's visibility (and lazy load) + // needs an explicit sync on every open, not just a save. + this.applyCliManagementVisibility(); // Read My Mind: synced, default OFF (opt-in; capture + prediction cost real tokens). document.getElementById('appSettingsReadMyMind').checked = settings.readMyMindEnabled === true; document.getElementById('appSettingsUltracodeFloatingWindows').checked = @@ -2711,6 +2715,227 @@ Object.assign(CodemanApp.prototype, { } }, + // ═══════════════════════════════════════════════════════════════ + // CLI management (docs/cli-enable-disable-plan.md) + // + // CRUD against /api/clis, rendered into the Agents & CLIs settings section. + // Same load/save-pair-outside-openAppSettings reasoning as the Custom Model + // Endpoints block above: these are server-side registry records, not a + // settings-payload field — only the `cliManagementEnabled` toggle itself + // goes through openAppSettings/saveAppSettings. + // ═══════════════════════════════════════════════════════════════ + + /** + * Same two-caller shape as applyCustomModelEndpointsVisibility (assigning + * .checked fires no change event, so this needs both an explicit call on + * open AND the checkbox's own onchange) and the same reasoning for hiding + * the whole list rather than showing it disabled: with the flag off the + * rows would be controls that only 403. + */ + applyCliManagementVisibility() { + const enabled = document.getElementById('appSettingsCliManagement').checked; + const group = document.getElementById('cliListGroup'); + if (group) group.style.display = enabled ? '' : 'none'; + if (enabled) this.loadCliListForSettings(); + else this.closeCliCustomForm(); + this._applyCliManagementAdminGate(); + }, + + /** + * Decision 5 (docs/cli-enable-disable-plan.md): hidden entirely for a + * non-admin in multi-user mode, not shown-empty. GET /api/clis already + * answers a non-admin with [], which empties the row list on its own; the + * "Add a custom CLI" row has no list row to hide behind, so it needs its + * own gate the same way the Custom Model Endpoints "+ Add" button does. + */ + _applyCliManagementAdminGate() { + const group = document.getElementById('cliListGroup'); + if (!group) return; + const me = window.__codemanUser || {}; + const blocked = me.multiUser && me.role !== 'admin'; + const featureOn = document.getElementById('appSettingsCliManagement')?.checked ?? false; + group.style.display = blocked || !featureOn ? 'none' : ''; + const addRow = document.getElementById('cliCustomAddToggle'); + if (addRow) addRow.style.display = blocked ? 'none' : ''; + }, + + async loadCliListForSettings() { + // GET /api/clis wraps its body in the { success, data } envelope like every + // other /api route — _apiJson() unwraps it, same reasoning as the Custom + // Model Endpoints list load above. + const clis = await this._apiJson('/api/clis'); + this._cliList = Array.isArray(clis) ? clis : []; + this.renderCliList(); + }, + + renderCliList() { + const list = document.getElementById('cliListRows'); + if (!list) return; + const clis = this._cliList || []; + if (clis.length === 0) { + list.innerHTML = '

No CLIs found.

'; + return; + } + const UNDISABLEABLE = new Set(['shell', 'claude']); + list.innerHTML = clis + .map((c) => { + const idArg = escapeHtml(JSON.stringify(c.id)); + const undisableable = UNDISABLEABLE.has(c.id); + const toggleTitle = undisableable ? `title="${escapeHtml(c.id)} cannot be disabled"` : ''; + const installBtn = + c.stock && !c.installed + ? `` + : ''; + const customActions = c.stock + ? '' + : ` + `; + return ` +
+
+ ${escapeHtml(c.label)} ${escapeHtml(c.shortBadge)} + ${c.installed ? 'Installed' : 'Not installed'}${c.stock ? '' : ' · custom'} +
+
+ ${installBtn} + ${customActions} + +
+
`; + }) + .join(''); + }, + + async toggleCliEnabled(id, checkbox) { + const next = checkbox.checked; + const res = await this._api(`/api/clis/${encodeURIComponent(id)}`, { method: 'PUT', body: { enabled: next } }); + if (!res || !res.ok) { + checkbox.checked = !next; // revert on failure — the row must not lie about server state + let detail = ''; + try { + detail = (await res?.json())?.error || ''; + } catch { + /* no body to read */ + } + this.showToast(`Failed to ${next ? 'enable' : 'disable'} "${id}"${detail ? `: ${detail}` : ''}`, 'error'); + return; + } + await this.loadCliListForSettings(); + }, + + async installCliEntry(id) { + const btn = document.getElementById(`cliInstallBtn-${id}`); + if (btn) { + btn.disabled = true; + btn.textContent = 'Installing…'; + } + try { + const res = await this._api(`/api/clis/${encodeURIComponent(id)}/install`, { method: 'POST' }); + if (!res || !res.ok) { + let detail = ''; + try { + detail = (await res?.json())?.error || ''; + } catch { + /* no body to read */ + } + this.showToast(`Installing "${id}" failed${detail ? `: ${detail}` : ''}`, 'error'); + return; + } + this.showToast(`Installed "${id}"`, 'success'); + } finally { + await this.loadCliListForSettings(); + } + }, + + /** + * Pass no id to create a new entry; pass an existing CUSTOM id to edit one. + * ⚠️ GET /api/clis deliberately excludes discovery/launch (Phase 2's own + * scope), so an edit cannot be pre-filled with the entry's existing binary + * or argv — those two fields start blank and must be re-entered, since the + * update endpoint (PUT /api/clis/custom/:id) replaces the whole launch + * spec rather than patching it. id/label/badge DO come from the list row. + */ + openCliCustomForm(editId) { + const form = document.getElementById('cliCustomForm'); + const errorEl = document.getElementById('cliCustomFormError'); + if (!form) return; + const existing = editId ? (this._cliList || []).find((c) => c.id === editId) : null; + this._editingCliCustomId = existing ? existing.id : null; + document.getElementById('cliCustomId').value = existing ? existing.id : ''; + document.getElementById('cliCustomId').disabled = !!existing; // id is immutable once created + document.getElementById('cliCustomLabel').value = existing ? existing.label : ''; + document.getElementById('cliCustomBadge').value = existing ? existing.shortBadge : ''; + document.getElementById('cliCustomBinary').value = ''; + document.getElementById('cliCustomArgv').value = ''; + document.getElementById('cliCustomSubmit').textContent = existing ? 'Save' : 'Create'; + if (errorEl) errorEl.style.display = 'none'; + form.style.display = ''; + }, + + closeCliCustomForm() { + const form = document.getElementById('cliCustomForm'); + if (form) form.style.display = 'none'; + this._editingCliCustomId = null; + }, + + /** Wired to #cliCustomForm's onsubmit; `event` is the submit event. */ + async submitCliCustomForm(event) { + event.preventDefault(); + const errorEl = document.getElementById('cliCustomFormError'); + const showError = (msg) => { + if (errorEl) { + errorEl.textContent = msg; + errorEl.style.display = ''; + } + }; + const id = document.getElementById('cliCustomId').value.trim(); + const label = document.getElementById('cliCustomLabel').value.trim(); + const shortBadge = document.getElementById('cliCustomBadge').value.trim(); + const binaries = document.getElementById('cliCustomBinary').value.trim().split(/\s+/).filter(Boolean); + const argv = document.getElementById('cliCustomArgv').value.trim().split(/\s+/).filter(Boolean); + if (!id || !label || !shortBadge || binaries.length === 0 || argv.length === 0) { + showError('All fields are required.'); + return; + } + const editing = this._editingCliCustomId; + const path = editing ? `/api/clis/custom/${encodeURIComponent(editing)}` : '/api/clis'; + const method = editing ? 'PUT' : 'POST'; + const res = await this._api(path, { method, body: { id, label, shortBadge, binaries, argv } }); + if (!res || !res.ok) { + let detail = 'Request failed'; + try { + detail = (await res?.json())?.error || detail; + } catch { + /* no body to read */ + } + showError(detail); + return; + } + this.closeCliCustomForm(); + await this.loadCliListForSettings(); + }, + + async deleteCliCustom(id) { + const entry = (this._cliList || []).find((c) => c.id === id); + if (!confirm(`Delete custom CLI "${entry?.label || id}"? This cannot be undone.`)) return; + const res = await this._api(`/api/clis/${encodeURIComponent(id)}`, { method: 'DELETE' }); + if (!res || !res.ok) { + let detail = ''; + try { + detail = (await res?.json())?.error || ''; + } catch { + /* no body to read */ + } + this.showToast(`Failed to delete "${id}"${detail ? `: ${detail}` : ''}`, 'error'); + return; + } + await this.loadCliListForSettings(); + }, + // ═══════════════════════════════════════════════════════════════ // Visibility Settings & Device-Specific Defaults // ═══════════════════════════════════════════════════════════════ @@ -3786,4 +4011,5 @@ Object.assign(CodemanApp.prototype, { // evaluation, not just this feature. document.addEventListener?.('codeman:me', () => { window.app?._applyCustomModelAdminGate?.(); + window.app?._applyCliManagementAdminGate?.(); }); diff --git a/src/web/routes/cli-registry-routes.ts b/src/web/routes/cli-registry-routes.ts index 3292da62..4c8c2c7c 100644 --- a/src/web/routes/cli-registry-routes.ts +++ b/src/web/routes/cli-registry-routes.ts @@ -3,22 +3,38 @@ * original #343 review, done in phases with the trust-model scope decided up front * (see that doc's "Decisions" section) rather than folded into a large diff. * - * This file currently holds Phase 2 only: `GET /api/clis`, a read-only list of - * every registry entry (stock + custom, enabled or not) for the Settings UI. - * Phase 3 (write: enable/disable), Phase 4 (auto-install) and Phase 5 (custom - * entry CRUD) land as their own additions here, each behind `cliManagementEnabled`. + * Phase 2: `GET /api/clis` — read-only list, ungated (reading is cheap, not the risky part). + * Phase 3: `PUT /api/clis/:id` — stock enable/disable ONLY; 404 for a non-stock id, so this + * endpoint can never become a backdoor for creating an entry (that's Phase 5's job). + * Phase 4: `POST /api/clis/:id/install` — runs a STOCK entry's already-vetted install command + * (never a custom entry's — Decision 3). Never auto-enables; Phase 3's endpoint is still + * the only thing that flips `enabled`. + * Phase 5: `POST /api/clis` (create) / `PUT /api/clis/custom/:id` (update) / `DELETE + * /api/clis/:id` (custom only) — a deliberately separate write surface from Phase 3's, so + * "stock entries can only have `enabled` toggled here, custom entries can be fully edited" + * stays structurally true rather than depending on every caller remembering the rule. * - * Mirrors `custom-model-routes.ts`'s shape for the closest existing precedent: - * same admin-gating pattern, same `readXEnabled()` helper shape reading - * `settings.json` directly rather than threading the setting through every - * caller. + * Every write endpoint answers the SAME way when `cliManagementEnabled` is off: 403 + * FORBIDDEN with a message naming the setting, via `requireCliManagementGate()`. + * + * Mirrors `custom-model-routes.ts`'s shape for the closest existing precedent: same + * admin-gating pattern, same `readXEnabled()` helper shape reading `settings.json` + * directly rather than threading the setting through every caller, same tmp+rename+0600 + * write path (`registry-writer.ts` mirrors `custom-model-hosts.ts`). */ +import { spawn } from 'node:child_process'; import type { FastifyInstance, FastifyRequest } from 'fastify'; -import { isAdmin, readJsonConfig, SETTINGS_PATH } from '../route-helpers.js'; +import { ApiErrorCode, createErrorResponse, getErrorMessage, type ApiResponse } from '../../types.js'; +import { getAuthUser, isAdmin, parseBody, readJsonConfig, SETTINGS_PATH } from '../route-helpers.js'; import { isMultiUserMode } from '../../config/multiuser.js'; -import { listClis } from '../../config/cli-registry/registry.js'; +import { listClis, reloadCliRegistry, resolveInstallCommandForPlatform } from '../../config/cli-registry/registry.js'; +import { readRegistryFileForWrite, writeRegistryFile } from '../../config/cli-registry/registry-writer.js'; +import { CliEntrySchema } from '../../config/cli-registry/schema.js'; +import { STOCK_CLIS } from '../../config/cli-registry/stock.js'; import type { CliEntry } from '../../config/cli-registry/types.js'; +import { CliCustomEntrySchema, CliEnableSchema } from '../schemas.js'; +import { appendAdminAudit } from '../admin-audit.js'; /** * `cliManagementEnabled` defaults OFF, same reasoning as @@ -50,11 +66,9 @@ export interface CliListItem { * resolvers when the CLI-management section is never opened; Node caches the * module after the first call, so repeat requests cost nothing extra. * - * ⚠️ STOCK-ONLY. There is no per-id resolver for a CUSTOM entry — Phase 5 - * (custom CLI creation) needs a GENERIC installed check built from the - * entry's own `discovery.binaries`/`searchDirs` directly, not this map. Until - * then a custom entry (none can exist before Phase 5 ships) reports `installed: - * false` rather than guessing. + * ⚠️ STOCK-ONLY. There is no per-id resolver for a CUSTOM entry — Phase 5's + * custom entries report `installed: false` from a GENERIC probe instead (a + * plain `which`-style search over the entry's own `discovery.binaries`). */ const STOCK_INSTALLED_PROBES: Record Promise> = { claude: async () => (await import('../../utils/claude-cli-resolver.js')).isClaudeAvailable(), @@ -71,22 +85,224 @@ const STOCK_INSTALLED_PROBES: Record Promise> = { omp: async () => (await import('../../utils/omp-cli-resolver.js')).isOmpAvailable(), }; +/** + * A CUSTOM entry's own installed probe: a plain PATH search (`which`/`where`) over its + * declared binaries, since there is no per-id resolver for a user-defined CLI. Deliberately + * simple — this is an informational badge in a settings list, not a launch-time gate (the + * actual resolver chain a session spawn uses is unrelated to this probe) — so it skips the + * full retry/negative-cache machinery `cli-executable-resolver.ts` builds for the heavier, + * request-hot stock resolvers. + * + * No real IO under vitest, same discipline as every other resolver in this codebase + * (`IS_TEST_MODE` in tmux-manager, the VITEST gate in cli-executable-resolver.ts): a test + * run must never scan the real machine's PATH. + */ +async function probeCustomInstalled(entry: CliEntry): Promise { + if (process.env.VITEST) return false; + const { execFileSync } = await import('node:child_process'); + const whichCmd = process.platform === 'win32' ? 'where' : 'which'; + for (const binary of entry.discovery.binaries) { + try { + execFileSync(whichCmd, [binary], { stdio: 'ignore', timeout: 3000 }); + return true; + } catch { + /* not on PATH, try the next declared binary */ + } + } + return false; +} + async function probeInstalled(entry: CliEntry): Promise { - const probe = STOCK_INSTALLED_PROBES[entry.id as string]; - return probe ? probe() : false; + if (entry.stock) { + const probe = STOCK_INSTALLED_PROBES[entry.id as string]; + return probe ? probe() : false; + } + return probeCustomInstalled(entry); +} + +const STOCK_IDS = new Set(STOCK_CLIS.map((e) => e.id as string)); + +/** `shell`/`claude` can never be disabled (Decision 4) — enforced here, not just in the UI. */ +const UNDISABLEABLE_IDS = new Set(['shell', 'claude']); + +/** + * Every write endpoint (Phases 3-5) answers the SAME way when the feature is off or the + * caller is a non-admin in multi-user mode: 403 FORBIDDEN. Decided once here rather than + * per-route, per docs/cli-enable-disable-plan.md Phase 1's own checklist item ("decide + * exact behavior... before Phase 3 starts, so all three write endpoints answer the same way"). + */ +async function requireCliManagementGate(req: FastifyRequest): Promise | null> { + if (isMultiUserMode() && !isAdmin(req)) { + return createErrorResponse(ApiErrorCode.FORBIDDEN, 'Admin only in multi-user mode'); + } + if (!(await readCliManagementEnabled())) { + return createErrorResponse(ApiErrorCode.FORBIDDEN, 'CLI management is disabled. Enable it in Settings first.'); + } + return null; +} + +/** + * Assembles a full, schema-valid `CliEntry` from Phase 5's deliberately minimal request + * shape (id/label/shortBadge/binaries/a simple launch variant — nothing else exposed in + * v1), filling every other required field with conservative, safe defaults: no hooks, no + * mux-optional fallback, no privileged params, no install command (Decision 3: a custom + * entry's install text stays display-only, and there IS none here to display), no custom + * model injection. `CliEntrySchema` re-validates the WHOLE thing below — this function + * only shapes the object, it is not itself the safety layer. + */ +function buildCustomCliEntry( + input: { id: string; label: string; shortBadge: string; binaries: string[]; argv: string[]; enabled?: boolean }, + order: number +): unknown { + return { + id: input.id, + label: input.label, + shortBadge: input.shortBadge, + accent: '#6b7280', + enabled: input.enabled ?? true, + stock: false, + order, + kind: 'agent', + discovery: { + binaries: input.binaries, + searchDirs: [], + install: { command: {} }, + }, + launch: { + params: {}, + variants: [{ id: 'default', args: input.argv.map((tok) => ({ lit: tok })) }], + }, + env: { + exports: [], + unset: [], + tmuxSetenvKeys: [], + dockerExecEnvNames: [], + allowedPrefixes: [], + allowedKeys: [], + }, + capabilities: { + external: true, + requiresMux: true, + hooks: 'none', + transcript: 'none', + altScreen: 'strip-mux-only', + echo: { policy: 'buffer', anchor: { kind: 'none' } }, + wheelForward: { mode: 'never' }, + keyboardAccessory: 'agent', + privilegedCommandGate: false, + startMode: 'interactive', + stripInkBloat: false, + ralph: false, + respawn: false, + effort: false, + agentSkillInjection: false, + statusLineTelemetry: false, + model: { source: 'none' }, + privilegedParams: [], + privilegedEnvKeys: [], + gates: {}, + customModelInjection: { kind: 'unsupported' }, + }, + overlays: {}, + }; +} + +function nextOrder(): number { + const orders = listClis().map((e) => e.order); + return (orders.length ? Math.max(...orders) : 0) + 10; +} + +/** Bounded execution: `PATH_INSTALL_TIMEOUT_MS`, output capped, process GROUP killed on timeout. */ +const CLI_INSTALL_TIMEOUT_MS = 300_000; + +interface InstallResult { + code: number | null; + output: string; + timedOut: boolean; +} + +/** + * Runs a STOCK entry's already-vetted install command. `shell: true` is unavoidable here — + * the shipped commands are genuinely `curl | bash` / `npm install -g` one-liners — but this + * is NOT a reopening of the config-shell-text concern the registry's `shellToken` pattern + * exists to prevent: the string executed here is NEVER user input, only ever what is + * already hardcoded and reviewed in `stock.ts` (`resolveInstallCommandForPlatform`), and a + * CUSTOM entry can never reach this function at all — see the route's own guard below. + */ +async function runInstallCommand(command: string): Promise { + return new Promise((resolve) => { + let child: ReturnType; + try { + child = spawn(command, { + shell: true, + stdio: ['ignore', 'pipe', 'pipe'], + // Own process group; the timeout below kills the whole tree by hand, mirroring + // the DeepSeek profile-install endpoint's own reasoning: an install command fans + // 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: process.env, + }); + } catch (err) { + resolve({ code: null, output: `spawn failed: ${getErrorMessage(err)}`, timedOut: false }); + return; + } + + let output = ''; + let timedOut = false; + let settled = false; + let killTimer: NodeJS.Timeout | undefined; + let reapTimer: NodeJS.Timeout | undefined; + + const capture = (chunk: Buffer) => { + if (output.length < 16_384) output += chunk.toString('utf-8'); + }; + child.stdout?.on('data', capture); + child.stderr?.on('data', capture); + + const killTree = (signal: NodeJS.Signals) => { + try { + if (child.pid) process.kill(-child.pid, signal); + } catch { + /* already gone */ + } + }; + + const finish = (code: number | null) => { + if (settled) return; + settled = true; + clearTimeout(timer); + if (killTimer) clearTimeout(killTimer); + if (reapTimer) clearTimeout(reapTimer); + resolve({ code, output, timedOut }); + }; + + const timer = setTimeout(() => { + timedOut = true; + killTree('SIGTERM'); + killTimer = setTimeout(() => killTree('SIGKILL'), 3_000); + reapTimer = setTimeout(() => finish(null), 8_000); + }, CLI_INSTALL_TIMEOUT_MS); + + child.on('error', (err) => { + output = `${output}\n${err.message}`; + finish(null); + }); + child.on('close', (code) => finish(code)); + }); } export function registerCliRegistryRoutes(app: FastifyInstance): void { + // ---- Phase 2: read ---------------------------------------------------- // GET /api/clis — every registry entry, disabled ones included (this is an // admin/settings surface; every SPAWN-time caller elsewhere uses // enabledClis() instead). Deliberately excludes launch/env/capabilities/ // overlays/discovery — the same rule every other catalogue-export surface in - // this codebase follows (scripts/generate-cli-catalog.mts, the reverted PR - // B2 window.__codemanCliCatalog before it). + // this codebase follows. // // NOT gated on cliManagementEnabled: reading the list is cheap and is not - // the risky part (docs/cli-enable-disable-plan.md, Phase 1). The Settings UI - // section simply never fetches this while the flag is off (Phase 6). + // the risky part. The Settings UI section simply never fetches this while + // the flag is off (Phase 6). app.get('/api/clis', async (req: FastifyRequest): Promise<{ success: true; data: CliListItem[] }> => { if (isMultiUserMode() && !isAdmin(req)) { return { success: true, data: [] }; @@ -106,4 +322,181 @@ export function registerCliRegistryRoutes(app: FastifyInstance): void { ); return { success: true, data }; }); + + // ---- Phase 3: enable/disable (stock OR custom) ------------------------- + // PUT /api/clis/:id — body { enabled }. Toggles an EXISTING entry's + // `enabled` flag, stock or custom alike; a not-yet-existing id is 404, + // never a backdoor into CREATING one (Phase 5 owns creation via its own + // endpoint, POST /api/clis). This is deliberately the one simple toggle + // both kinds of entry share — full custom-entry editing is a SEPARATE path + // (PUT /api/clis/custom/:id) precisely so a caller can flip `enabled` + // without first knowing the rest of a custom entry's shape (its binaries, + // its argv), which the Settings UI list row never carries. + app.put('/api/clis/:id', async (req, reply): Promise> => { + const denied = await requireCliManagementGate(req); + if (denied) { + reply.code(403); + return denied; + } + const { id } = req.params as { id: string }; + const body = parseBody(CliEnableSchema, req.body); + + if (UNDISABLEABLE_IDS.has(id) && !body.enabled) { + return createErrorResponse(ApiErrorCode.INVALID_INPUT, `"${id}" cannot be disabled`); + } + if (!listClis().some((e) => (e.id as string) === id)) { + return createErrorResponse(ApiErrorCode.NOT_FOUND, `"${id}" does not exist`); + } + + const file = await readRegistryFileForWrite(); + const existingOverride = (file.clis[id] as Record | undefined) ?? {}; + file.clis = { ...file.clis, [id]: { ...existingOverride, enabled: body.enabled } }; + await writeRegistryFile(file); + reloadCliRegistry(); + return { success: true, data: { id, enabled: body.enabled } }; + }); + + // ---- Phase 4: auto-install (stock only) -------------------------------- + // POST /api/clis/:id/install — runs the entry's already-vetted install + // command. Separate endpoint from Phase 3's toggle: installing is a bigger + // action than a boolean flip and gets its own audit entry. Never auto- + // enables — Phase 3's endpoint is still the only thing that flips `enabled`. + app.post( + '/api/clis/:id/install', + async (req, reply): Promise> => { + const denied = await requireCliManagementGate(req); + if (denied) { + reply.code(403); + return denied; + } + const { id } = req.params as { id: string }; + if (!STOCK_IDS.has(id)) { + // Decision 3: a custom entry's install command is NEVER executed, full + // stop — this guard is what makes that true independent of anything + // Phase 5 does, even if a caller invents an id that happens to match + // a custom entry's. + return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Auto-install is only available for stock CLIs'); + } + const entry = listClis().find((e) => (e.id as string) === id); + if (!entry) return createErrorResponse(ApiErrorCode.NOT_FOUND, `"${id}" is not a stock CLI`); + const command = resolveInstallCommandForPlatform(entry); + if (!command) { + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `"${id}" has no install command for this platform`); + } + + const result = await runInstallCommand(command); + const admin = getAuthUser(req).username; + void appendAdminAudit({ + admin, + action: 'cli_install', + target: id, + ip: req.ip, + detail: { command, exitCode: result.code, timedOut: result.timedOut }, + }); + + if (result.code !== 0) { + const detail = result.timedOut + ? `timed out after ${Math.round(CLI_INSTALL_TIMEOUT_MS / 1000)}s` + : result.output.slice(-1000).trim() || 'no output'; + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `Installing "${id}" failed: ${detail}`); + } + return { success: true, data: { id, code: result.code, output: result.output.slice(-4000) } }; + } + ); + + // ---- Phase 5: custom CLI entries ---------------------------------------- + // POST /api/clis — create a custom entry. Deliberately separate from Phase + // 3's PUT: that endpoint can only ever toggle an EXISTING stock entry, this + // one can only ever create a NEW custom one, so the two write surfaces + // cannot be confused for each other by a caller. + app.post('/api/clis', async (req, reply): Promise> => { + const denied = await requireCliManagementGate(req); + if (denied) { + reply.code(403); + return denied; + } + const body = parseBody(CliCustomEntrySchema, req.body); + if (STOCK_IDS.has(body.id)) { + return createErrorResponse( + ApiErrorCode.ALREADY_EXISTS, + `"${body.id}" is a stock CLI id and cannot be used for a custom entry` + ); + } + const file = await readRegistryFileForWrite(); + if (Object.prototype.hasOwnProperty.call(file.clis, body.id)) { + return createErrorResponse(ApiErrorCode.ALREADY_EXISTS, `A custom CLI "${body.id}" already exists`); + } + + const candidate = buildCustomCliEntry(body, nextOrder()); + const parsed = CliEntrySchema.safeParse(candidate); + if (!parsed.success) { + return createErrorResponse(ApiErrorCode.INVALID_INPUT, parsed.error.message); + } + + // Stored WITHOUT id/stock — those are forced back in by resolveRegistry() on every + // read, so the override file never duplicates what the key and provenance already say. + const { id: _id, stock: _stock, ...toStore } = parsed.data; + file.clis = { ...file.clis, [body.id]: toStore }; + await writeRegistryFile(file); + reloadCliRegistry(); + return { success: true, data: { id: body.id } }; + }); + + // PUT /api/clis/custom/:id — full update of an EXISTING custom entry. A + // separate path from Phase 3's PUT /api/clis/:id on purpose: that one is + // structurally stock-only (404s any id it doesn't recognise as stock), so + // there is no shared route where "which fields this id may change" depends + // on a runtime check a caller could get wrong. + app.put('/api/clis/custom/:id', async (req, reply): Promise> => { + const denied = await requireCliManagementGate(req); + if (denied) { + reply.code(403); + return denied; + } + const { id } = req.params as { id: string }; + if (STOCK_IDS.has(id)) { + return createErrorResponse(ApiErrorCode.INVALID_INPUT, `"${id}" is a stock CLI; use PUT /api/clis/${id} instead`); + } + const body = parseBody(CliCustomEntrySchema, { ...(req.body as object), id }); + const file = await readRegistryFileForWrite(); + if (!Object.prototype.hasOwnProperty.call(file.clis, id)) { + return createErrorResponse(ApiErrorCode.NOT_FOUND, `No custom CLI "${id}"`); + } + + const existingOrder = listClis().find((e) => (e.id as string) === id)?.order ?? nextOrder(); + const candidate = buildCustomCliEntry(body, existingOrder); + const parsed = CliEntrySchema.safeParse(candidate); + if (!parsed.success) { + return createErrorResponse(ApiErrorCode.INVALID_INPUT, parsed.error.message); + } + + const { id: _id, stock: _stock, ...toStore } = parsed.data; + file.clis = { ...file.clis, [id]: toStore }; + await writeRegistryFile(file); + reloadCliRegistry(); + return { success: true, data: { id } }; + }); + + // DELETE /api/clis/:id — refuses any STOCK id outright; deleting only ever + // removes a CUSTOM entry's override. + app.delete('/api/clis/:id', async (req, reply): Promise> => { + const denied = await requireCliManagementGate(req); + if (denied) { + reply.code(403); + return denied; + } + const { id } = req.params as { id: string }; + if (STOCK_IDS.has(id)) { + return createErrorResponse(ApiErrorCode.INVALID_INPUT, `"${id}" is a stock CLI and cannot be deleted`); + } + const file = await readRegistryFileForWrite(); + if (!Object.prototype.hasOwnProperty.call(file.clis, id)) { + return createErrorResponse(ApiErrorCode.NOT_FOUND, `No custom CLI "${id}"`); + } + const { [id]: _removed, ...rest } = file.clis; + file.clis = rest; + await writeRegistryFile(file); + reloadCliRegistry(); + return { success: true, data: { id } }; + }); } diff --git a/src/web/schemas.ts b/src/web/schemas.ts index 0b6f18a2..ccefcab7 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -2006,6 +2006,39 @@ export const CustomModelHostSchema = z.object({ modelSizesGB: z.record(z.string().max(200), z.number().positive().max(100_000)).optional(), }); +/** + * A shell-safe bare word, mirroring `config/cli-registry/schema.ts`'s own `shellToken` — + * duplicated rather than imported, since the REAL safety boundary for anything built from + * this is `CliEntrySchema` itself, re-applied server-side once the full entry is assembled + * (`cli-registry-routes.ts`). This is a request-shape sanity check, not the security gate. + */ +const cliShellToken = z + .string() + .min(1) + .max(256) + .regex(/^[A-Za-z0-9._:@=+/,-]+$/, 'must be a plain word with no shell metacharacters'); + +/** PUT /api/clis/:id (Phase 3) — stock enable/disable, the ONLY thing this endpoint can flip. */ +export const CliEnableSchema = z.object({ enabled: z.boolean() }); + +/** + * POST /api/clis + PUT /api/clis/custom/:id (Phase 5) — a deliberately MINIMAL custom-CLI + * shape (docs/cli-enable-disable-plan.md, Phase 6 checklist: "scope the FIRST version to the + * fields most stock entries actually use"), not the full `CliEntry`. `cli-registry-routes.ts` + * assembles the rest with safe, conservative capability defaults and re-validates the whole + * thing through `CliEntrySchema` before ever writing it — this schema exists to bound the + * REQUEST shape, not to BE the safety layer (Decision 3: typed-argv only, no raw shell text). + */ +export const CliCustomEntrySchema = z.object({ + id: z.string().regex(/^[a-z][a-z0-9-]{0,23}$/, 'id must be lowercase, start with a letter, at most 24 chars'), + label: z.string().min(1).max(60), + shortBadge: z.string().min(1).max(6), + enabled: z.boolean().optional(), + binaries: z.array(cliShellToken).min(1).max(4), + /** Bare argv tokens for the single launch variant — no flags-with-values, no params. */ + argv: z.array(cliShellToken).min(1).max(16), +}); + /** POST /api/sessions/:id/custom-model — apply or clear a session's custom-model selection. */ export const CustomModelSelectionSchema = z.union([ z.object({ diff --git a/test/routes/cli-registry-routes.test.ts b/test/routes/cli-registry-routes.test.ts index e3fe3886..db174f63 100644 --- a/test/routes/cli-registry-routes.test.ts +++ b/test/routes/cli-registry-routes.test.ts @@ -1,13 +1,39 @@ /** - * @fileoverview Route tests for GET /api/clis (docs/cli-enable-disable-plan.md, - * Phase 2). Mirrors the admin-gating test shape used for other admin/settings - * surfaces (see test/routes/search-routes.test.ts's multi-user block). + * @fileoverview Route tests for /api/clis (docs/cli-enable-disable-plan.md, Phases 2-5). + * Mirrors the admin-gating test shape used for other admin/settings surfaces (see + * test/routes/search-routes.test.ts's multi-user block). + * + * ⚠️ test/setup.ts gives the whole FILE one temp HOME, not one per `it()` — a write in + * one test is visible to every test declared after it. Phase 3-5 tests therefore each + * clean up what they create (delete a custom entry, restore a toggled stock flag) so + * later tests, including the Phase 2 "every entry is stock" assumption above, still hold. * * Port: N/A (app.inject(), no live server). */ import { afterEach, describe, expect, it } from 'vitest'; +import { mkdirSync, writeFileSync, statSync } from 'node:fs'; +import { dirname } from 'node:path'; import { createRouteTestHarness } from './_route-test-utils.js'; import { registerCliRegistryRoutes, type CliListItem } from '../../src/web/routes/cli-registry-routes.js'; +import { SETTINGS_PATH } from '../../src/web/route-helpers.js'; +import { getCli, registryFilePath } from '../../src/config/cli-registry/registry.js'; + +/** Every write endpoint requires this on; toggled per-test by writing settings.json directly. */ +function enableCliManagement(): void { + mkdirSync(dirname(SETTINGS_PATH), { recursive: true }); + writeFileSync(SETTINGS_PATH, JSON.stringify({ cliManagementEnabled: true })); +} + +/** + * The inverse, and load-bearing for every "off" test below: settings.json is shared by + * the whole FILE (one temp HOME, not one per `it()`), so a "should be rejected while off" + * test cannot assume the flag started false — an EARLIER test may have called + * `enableCliManagement()` and left it on. + */ +function disableCliManagement(): void { + mkdirSync(dirname(SETTINGS_PATH), { recursive: true }); + writeFileSync(SETTINGS_PATH, JSON.stringify({ cliManagementEnabled: false })); +} describe('GET /api/clis', () => { afterEach(() => { @@ -75,3 +101,325 @@ describe('GET /api/clis', () => { expect(body.data).toEqual([]); }); }); + +describe('PUT /api/clis/:id (Phase 3: enable/disable)', () => { + afterEach(() => { + delete process.env.CODEMAN_MULTIUSER; + }); + + it('rejects when cliManagementEnabled is off — no settings.json write at all', async () => { + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + const res = await app.inject({ method: 'PUT', url: '/api/clis/grok', payload: { enabled: false } }); + expect(res.statusCode).toBe(403); + const body = res.json() as { errorCode: string }; + expect(body.errorCode).toBe('FORBIDDEN'); + }); + + it('toggles a stock CLI off then back on, visible with no reload needed', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + const off = await app.inject({ method: 'PUT', url: '/api/clis/grok', payload: { enabled: false } }); + expect(off.statusCode).toBe(200); + const afterOff = await app.inject({ method: 'GET', url: '/api/clis' }); + const grokOff = (afterOff.json() as { data: CliListItem[] }).data.find((c) => c.id === 'grok'); + expect(grokOff?.enabled).toBe(false); + + const on = await app.inject({ method: 'PUT', url: '/api/clis/grok', payload: { enabled: true } }); + expect(on.statusCode).toBe(200); + const afterOn = await app.inject({ method: 'GET', url: '/api/clis' }); + const grokOn = (afterOn.json() as { data: CliListItem[] }).data.find((c) => c.id === 'grok'); + expect(grokOn?.enabled).toBe(true); + }); + + it('rejects disabling shell or claude, changes nothing', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + for (const id of ['shell', 'claude']) { + const res = await app.inject({ method: 'PUT', url: `/api/clis/${id}`, payload: { enabled: false } }); + // errorCode, not statusCode: this branch returns bare createErrorResponse() + // and relies on server.ts's global preSerialization hook to map it to 400, + // which the lightweight test harness does not register — same convention + // as test/routes/custom-model-routes.test.ts's equivalent checks. + expect(res.json().errorCode).toBe('INVALID_INPUT'); + const list = await app.inject({ method: 'GET', url: '/api/clis' }); + const entry = (list.json() as { data: CliListItem[] }).data.find((c) => c.id === id); + expect(entry?.enabled).toBe(true); + } + }); + + it('404s an id that does not exist, never creating one', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + const res = await app.inject({ method: 'PUT', url: '/api/clis/nonexistent-id', payload: { enabled: true } }); + expect(res.json().errorCode).toBe('NOT_FOUND'); + const list = await app.inject({ method: 'GET', url: '/api/clis' }); + expect((list.json() as { data: CliListItem[] }).data.some((c) => c.id === 'nonexistent-id')).toBe(false); + }); + + it('multi-user: non-admin is rejected before the write', async () => { + enableCliManagement(); + process.env.CODEMAN_MULTIUSER = '1'; + const { app } = await createRouteTestHarness(registerCliRegistryRoutes, { + authUser: { username: 'bob', role: 'user' }, + }); + const res = await app.inject({ method: 'PUT', url: '/api/clis/grok', payload: { enabled: false } }); + expect(res.statusCode).toBe(403); + }); + + it('preserves an unrelated existing override key on a stock entry when toggling enabled', async () => { + enableCliManagement(); + // grok's real accent is #f43f5e (stock.ts); overriding it here first proves + // the enabled-only write is a MERGE, not a replace, of that id's override. + mkdirSync(dirname(registryFilePath()), { recursive: true }); + writeFileSync(registryFilePath(), JSON.stringify({ schemaVersion: 1, clis: { grok: { accent: '#123456' } } }), { + mode: 0o600, + }); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + const res = await app.inject({ method: 'PUT', url: '/api/clis/grok', payload: { enabled: false } }); + expect(res.statusCode).toBe(200); + // GET /api/clis deliberately excludes `accent` (Phase 2's own response + // shape), so verify the merge server-side through the registry itself. + const grok = getCli('grok'); + expect(grok?.enabled).toBe(false); + expect(grok?.accent).toBe('#123456'); + // Restore for any later test in this file that assumes grok's stock default. + await app.inject({ method: 'PUT', url: '/api/clis/grok', payload: { enabled: true } }); + }); + + it('writes clis.json mode 0600 on POSIX', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + await app.inject({ method: 'PUT', url: '/api/clis/grok', payload: { enabled: true } }); + if (process.platform !== 'win32') { + const mode = statSync(registryFilePath()).mode & 0o777; + expect(mode).toBe(0o600); + } + }); +}); + +describe('POST /api/clis/:id/install (Phase 4)', () => { + afterEach(() => { + delete process.env.CODEMAN_MULTIUSER; + }); + + it('rejects when cliManagementEnabled is off', async () => { + disableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + const res = await app.inject({ method: 'POST', url: '/api/clis/grok/install' }); + expect(res.statusCode).toBe(403); + }); + + it('rejects a custom entry id — Decision 3: a custom install command is never executed', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + await app.inject({ + method: 'POST', + url: '/api/clis', + payload: { id: 'test-install-guard', label: 'X', shortBadge: 'X', binaries: ['x'], argv: ['x'] }, + }); + const res = await app.inject({ method: 'POST', url: '/api/clis/test-install-guard/install' }); + expect(res.json().errorCode).toBe('INVALID_INPUT'); + await app.inject({ method: 'DELETE', url: '/api/clis/test-install-guard' }); + }); + + it('multi-user: non-admin is rejected before any spawn', async () => { + enableCliManagement(); + process.env.CODEMAN_MULTIUSER = '1'; + const { app } = await createRouteTestHarness(registerCliRegistryRoutes, { + authUser: { username: 'bob', role: 'user' }, + }); + const res = await app.inject({ method: 'POST', url: '/api/clis/grok/install' }); + expect(res.statusCode).toBe(403); + }); +}); + +describe('Custom CLI entries (Phase 5)', () => { + afterEach(() => { + delete process.env.CODEMAN_MULTIUSER; + }); + + it('rejects create when cliManagementEnabled is off', async () => { + disableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + const res = await app.inject({ + method: 'POST', + url: '/api/clis', + payload: { id: 'test-off', label: 'X', shortBadge: 'X', binaries: ['x'], argv: ['x'] }, + }); + expect(res.statusCode).toBe(403); + }); + + it('creates a custom entry, it appears in GET /api/clis with stock:false', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + const create = await app.inject({ + method: 'POST', + url: '/api/clis', + payload: { + id: 'test-create', + label: 'Test CLI', + shortBadge: 'TC', + binaries: ['test-create-bin'], + argv: ['test-create-bin', '--flag'], + }, + }); + expect(create.statusCode).toBe(200); + const list = await app.inject({ method: 'GET', url: '/api/clis' }); + const entry = (list.json() as { data: CliListItem[] }).data.find((c) => c.id === 'test-create'); + expect(entry?.stock).toBe(false); + expect(entry?.label).toBe('Test CLI'); + await app.inject({ method: 'DELETE', url: '/api/clis/test-create' }); + }); + + it('rejects a create whose id collides with a stock id', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + const res = await app.inject({ + method: 'POST', + url: '/api/clis', + payload: { id: 'claude', label: 'X', shortBadge: 'X', binaries: ['x'], argv: ['x'] }, + }); + expect(res.json().errorCode).toBe('ALREADY_EXISTS'); + }); + + it('rejects creating the same custom id twice', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + const payload = { id: 'test-dup', label: 'X', shortBadge: 'X', binaries: ['x'], argv: ['x'] }; + const first = await app.inject({ method: 'POST', url: '/api/clis', payload }); + expect(first.statusCode).toBe(200); + const second = await app.inject({ method: 'POST', url: '/api/clis', payload }); + expect(second.json().errorCode).toBe('ALREADY_EXISTS'); + await app.inject({ method: 'DELETE', url: '/api/clis/test-dup' }); + }); + + it('rejects a literal with shell metacharacters (the schema, not a new bypass)', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + const res = await app.inject({ + method: 'POST', + url: '/api/clis', + payload: { id: 'test-unsafe', label: 'X', shortBadge: 'X', binaries: ['x'], argv: ['x; rm -rf /'] }, + }); + expect(res.statusCode).toBe(400); + const list = await app.inject({ method: 'GET', url: '/api/clis' }); + expect((list.json() as { data: CliListItem[] }).data.some((c) => c.id === 'test-unsafe')).toBe(false); + }); + + it('updates an existing custom entry via PUT /api/clis/custom/:id', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + await app.inject({ + method: 'POST', + url: '/api/clis', + payload: { id: 'test-update', label: 'Before', shortBadge: 'BE', binaries: ['x'], argv: ['x'] }, + }); + const update = await app.inject({ + method: 'PUT', + url: '/api/clis/custom/test-update', + payload: { label: 'After', shortBadge: 'AF', binaries: ['y'], argv: ['y', '--z'] }, + }); + expect(update.statusCode).toBe(200); + const list = await app.inject({ method: 'GET', url: '/api/clis' }); + const entry = (list.json() as { data: CliListItem[] }).data.find((c) => c.id === 'test-update'); + expect(entry?.label).toBe('After'); + await app.inject({ method: 'DELETE', url: '/api/clis/test-update' }); + }); + + it('rejects PUT /api/clis/custom/:id against a stock id', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + const res = await app.inject({ + method: 'PUT', + url: '/api/clis/custom/claude', + payload: { label: 'Hijack', shortBadge: 'HJ', binaries: ['x'], argv: ['x'] }, + }); + expect(res.json().errorCode).toBe('INVALID_INPUT'); + const list = await app.inject({ method: 'GET', url: '/api/clis' }); + expect((list.json() as { data: CliListItem[] }).data.find((c) => c.id === 'claude')?.label).toBe('Claude'); + }); + + it('404s an update against a custom id that does not exist', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + const res = await app.inject({ + method: 'PUT', + url: '/api/clis/custom/nonexistent-custom', + payload: { label: 'X', shortBadge: 'X', binaries: ['x'], argv: ['x'] }, + }); + expect(res.json().errorCode).toBe('NOT_FOUND'); + }); + + it('deletes a custom entry; a second delete 404s', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + await app.inject({ + method: 'POST', + url: '/api/clis', + payload: { id: 'test-delete', label: 'X', shortBadge: 'X', binaries: ['x'], argv: ['x'] }, + }); + const del = await app.inject({ method: 'DELETE', url: '/api/clis/test-delete' }); + expect(del.statusCode).toBe(200); + const list = await app.inject({ method: 'GET', url: '/api/clis' }); + expect((list.json() as { data: CliListItem[] }).data.some((c) => c.id === 'test-delete')).toBe(false); + const again = await app.inject({ method: 'DELETE', url: '/api/clis/test-delete' }); + expect(again.json().errorCode).toBe('NOT_FOUND'); + }); + + it('refuses to delete a stock CLI', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + const res = await app.inject({ method: 'DELETE', url: '/api/clis/claude' }); + expect(res.json().errorCode).toBe('INVALID_INPUT'); + const list = await app.inject({ method: 'GET', url: '/api/clis' }); + expect((list.json() as { data: CliListItem[] }).data.some((c) => c.id === 'claude')).toBe(true); + }); + + it('multi-user: non-admin is rejected on create/update/delete', async () => { + enableCliManagement(); + process.env.CODEMAN_MULTIUSER = '1'; + const { app } = await createRouteTestHarness(registerCliRegistryRoutes, { + authUser: { username: 'bob', role: 'user' }, + }); + const create = await app.inject({ + method: 'POST', + url: '/api/clis', + payload: { id: 'test-mu', label: 'X', shortBadge: 'X', binaries: ['x'], argv: ['x'] }, + }); + expect(create.statusCode).toBe(403); + const update = await app.inject({ + method: 'PUT', + url: '/api/clis/custom/test-mu', + payload: { label: 'X', shortBadge: 'X', binaries: ['x'], argv: ['x'] }, + }); + expect(update.statusCode).toBe(403); + const del = await app.inject({ method: 'DELETE', url: '/api/clis/test-mu' }); + expect(del.statusCode).toBe(403); + }); + + it('a custom entry can also be toggled via the simple Phase 3 endpoint', async () => { + enableCliManagement(); + const { app } = await createRouteTestHarness(registerCliRegistryRoutes); + await app.inject({ + method: 'POST', + url: '/api/clis', + payload: { id: 'test-toggle', label: 'X', shortBadge: 'X', binaries: ['x'], argv: ['x'], enabled: true }, + }); + const off = await app.inject({ method: 'PUT', url: '/api/clis/test-toggle', payload: { enabled: false } }); + expect(off.statusCode).toBe(200); + const list = await app.inject({ method: 'GET', url: '/api/clis' }); + const entry = (list.json() as { data: CliListItem[] }).data.find((c) => c.id === 'test-toggle'); + expect(entry?.enabled).toBe(false); + // The rest of the entry (binaries/argv/label) must survive the shallow + // enabled-only merge — proven indirectly: a second full update still finds + // the row and changes its label, which would fail if the toggle had + // corrupted the stored shape. + const relabel = await app.inject({ + method: 'PUT', + url: '/api/clis/custom/test-toggle', + payload: { label: 'Still here', shortBadge: 'X', binaries: ['x'], argv: ['x'] }, + }); + expect(relabel.statusCode).toBe(200); + await app.inject({ method: 'DELETE', url: '/api/clis/test-toggle' }); + }); +});