diff --git a/.changeset/cli-catalog-consumers.md b/.changeset/cli-catalog-consumers.md index 46bb4a4b..a32197ae 100644 --- a/.changeset/cli-catalog-consumers.md +++ b/.changeset/cli-catalog-consumers.md @@ -17,15 +17,14 @@ The generator emits two committed artifacts, because neither consumer can import `config/clis.stock.json` for the Docker build, and a marked block inside `install.sh` itself, which runs via `curl | bash` before any checkout exists. The embedded copy is the FULL catalogue: an earlier attempt fetched it and fell back to a hardcoded two-CLI list, degrading -silently on an empty response, and there is no degraded mode to fall into now. An optional, -opt-in refresh (`CODEMAN_CLI_CATALOGUE_URL` or `CODEMAN_REFRESH_CLI_CATALOGUE=1`) warns loudly -on all three failure shapes. +silently on an empty response, and there is no degraded mode to fall into now — nor a network +fetch at all, since a `curl | bash` from master already carries a catalogue exactly as fresh as +the script itself. **Trust model is unchanged and now mechanical.** The server still never executes an entry's install command. `install.sh` executes only commands embedded in itself — same file, same TLS fetch, same commit as the `curl | bash` line that fetched it — and nothing pulled from the -network at install time is ever run: the two live in separate arrays and a test asserts the -refresh cannot write the executable one. +network at install time is ever run, because nothing is fetched at install time at all. **The agent image respects `enabled`.** The generated catalogue carries that flag, so a CLI shipping disabled is no longer baked into every image. It reads the stock catalogue rather than @@ -34,9 +33,9 @@ tagged `codeman/agent:base`. User-visible changes, all in the installer: -- The install menu is built from the catalogue, so it offers every enabled CLI that is not installed and ships an install command — five rather than the previous fixed two. Gemini had a command in the registry and appeared in no list in the script at all. +- The install menu is built from the catalogue, so it offers every enabled CLI with an install command that can drive a pane on its own — eight today, rather than the previous fixed two. Gemini had a command in the registry and appeared in no list in the script at all. DeepSeek is the one enabled CLI with a registry command that is deliberately NOT offered: `npm install -g @deepseek-ai/dsh` installs only the launcher, which ships no profile that can drive a terminal on its own, so choosing it used to leave the user with an AI CLI the installer considered "found" but that could not actually run anything. It still gets a hint pointing at its docs. - Its entries use the registry's labels ("Claude" rather than "Claude Code"), the same trade already made for `codeman doctor` rows. A suffix map would just be the hand-maintained list again. -- On a `wget`-only host the menu prints the commands instead of running them. The registry's commands call `curl`, whereas the two literals they replace went through `download_to_stdout`; rewriting `curl` to `wget` inside a string about to be executed is the wrong instinct. +- On a `wget`-only host, only the menu entries that actually need `curl` are held back (still shown as copy-paste hints); the `npm install -g` entries, which never needed it, are unaffected. Rewriting `curl` to `wget` inside a string about to be executed is the wrong instinct either way. - `CODEMAN_NONINTERACTIVE=1` still defaults to Claude Code, unchanged. `install.sh` remains bash 3.2 compatible (macOS ships it): parallel indexed arrays with diff --git a/CLAUDE.md b/CLAUDE.md index dc4b2fcb..7b9c945f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -452,6 +452,6 @@ Two constraints worth knowing before you touch them: the env-derived PTY buffer ## Scripts & Tunnel -**`install.sh`** (repo root, 104KB) is the public entry point: `curl -fsSL | bash` installs Node/tmux if missing, clones to `~/.codeman/app`, builds, and offers a systemd/launchd service. The network-access prompt is 3-way: **Tailscale** (loopback bind + guided `tailscale serve --bg ` HTTPS setup: install/login/operator/tailnet-HTTPS-toggle, then curl-verified end-to-end), **LAN** (0.0.0.0 + password prompt), or **local-only**; it preserves the existing binding on re-runs via `read_existing_binding()`. Tailscale state is detected dynamically from `tailscale serve status --json` (no marker files); the installer must NEVER `tailscale serve reset` or touch serve mappings other than 443→Codeman's port (users have unrelated serve config). `install.sh update`, `install.sh uninstall`, and `install.sh tailscale` (retrofit Tailscale access onto an existing install) also exist; `CODEMAN_NONINTERACTIVE=1` approves system changes for automation, `CODEMAN_TAILSCALE=1` presets the Tailscale choice (never installs Tailscale non-interactively). Its CLI knowledge is a GENERATED block (`npm run generate:cli-catalog`, markers in the file), not a hand-written list: detection, the install menu and the closing reminder all read it, which is what stops the class of bug upstream `b6d0f1fa` fixed by hand (a user with only omp installed being told no AI CLI was found). ⚠️ It must stay **bash 3.2** clean — macOS ships it and the documented install is `curl | bash` under `set -euo pipefail`, so `declare -A`, `mapfile`, namerefs, `${x,,}` and here-strings are all fatal there; CI runs `bash -n` plus a real `bash:3.2` container, since expanding an EMPTY array under `set -u` is a runtime abort `bash -n` cannot see. ⚠️ It executes ONLY commands from the embedded block (`CLI_INSTALL_CMD_TRUSTED`); nothing fetched by the optional catalogue refresh is ever run. +**`install.sh`** (repo root, ~112KB) is the public entry point: `curl -fsSL | bash` installs Node/tmux if missing, clones to `~/.codeman/app`, builds, and offers a systemd/launchd service. The network-access prompt is 3-way: **Tailscale** (loopback bind + guided `tailscale serve --bg ` HTTPS setup: install/login/operator/tailnet-HTTPS-toggle, then curl-verified end-to-end), **LAN** (0.0.0.0 + password prompt), or **local-only**; it preserves the existing binding on re-runs via `read_existing_binding()`. Tailscale state is detected dynamically from `tailscale serve status --json` (no marker files); the installer must NEVER `tailscale serve reset` or touch serve mappings other than 443→Codeman's port (users have unrelated serve config). `install.sh update`, `install.sh uninstall`, and `install.sh tailscale` (retrofit Tailscale access onto an existing install) also exist; `CODEMAN_NONINTERACTIVE=1` approves system changes for automation, `CODEMAN_TAILSCALE=1` presets the Tailscale choice (never installs Tailscale non-interactively). Its CLI knowledge is a GENERATED block (`npm run generate:cli-catalog`, markers in the file), not a hand-written list: detection, the install menu and the closing reminder all read it, which is what stops the class of bug upstream `b6d0f1fa` fixed by hand (a user with only omp installed being told no AI CLI was found). ⚠️ It must stay **bash 3.2** clean — macOS ships it and the documented install is `curl | bash` under `set -euo pipefail`, so `declare -A`, `mapfile`, namerefs, `${x,,}` and here-strings are all fatal there; CI runs `bash -n` plus a real `bash:3.2` container, since expanding an EMPTY array under `set -u` is a runtime abort `bash -n` cannot see. ⚠️ It executes ONLY commands from the embedded block (`CLI_INSTALL_CMD_TRUSTED`) — there is no network fetch of the catalogue at install time to worry about at all. Other key scripts: `scripts/tmux-manager.sh` (safe tmux mgmt), `scripts/tunnel.sh [quick|named] start|stop|status|url` (quick = random trycloudflare URL, default; `named setup|enable` = fixed-hostname tunnel via `scripts/codeman-tunnel-named.service`; bare `start|stop|url` still means quick), `scripts/run-beta.sh` (isolated beta instance), `scripts/build-agent-image.mjs` (docker base image), `scripts/self-update.sh` (detached updater). Production services: `scripts/codeman-web.service`, `scripts/codeman-tunnel.service`. **Always set `CODEMAN_PASSWORD`** before exposing via tunnel. diff --git a/config/clis.stock.json b/config/clis.stock.json index 382c25a7..89982217 100644 --- a/config/clis.stock.json +++ b/config/clis.stock.json @@ -181,7 +181,11 @@ "darwin": "npm install -g --ignore-scripts @earendil-works/pi-coding-agent" }, "npmPackage": "@earendil-works/pi-coding-agent", - "docsUrl": "https://pi.dev" + "docsUrl": "https://pi.dev", + "agentImageLayer": { + "kind": "dedicated", + "reason": "installed with --ignore-scripts in its own layer, so the flag cannot leak to the shared block" + } } } }, @@ -238,7 +242,11 @@ "darwin": "npm install -g @deepseek-ai/dsh" }, "npmPackage": "@deepseek-ai/dsh", - "docsUrl": "https://github.com/deepseek-ai/deepseek-harness" + "docsUrl": "https://github.com/deepseek-ai/deepseek-harness", + "agentImageLayer": { + "kind": "dedicated", + "reason": "needs pnpm alongside it (dsh plugin, issue #352) and a dsh-tui profile install" + } } } }, diff --git a/docker/agent.Dockerfile b/docker/agent.Dockerfile index 18dd9563..1c7899e8 100644 --- a/docker/agent.Dockerfile +++ b/docker/agent.Dockerfile @@ -58,7 +58,8 @@ RUN curl -fsSL https://antigravity.google/cli/install.sh | bash -s -- --dir /usr # Pi (pi.dev). Upstream documents --ignore-scripts (pi needs no lifecycle scripts); # kept out of the shared npm block above so the flag cannot silently change how the -# other four CLIs install. +# rest of that block's CLIs install — a fixed count would go stale here since +# CLI_NPM_PACKAGES (above) is now a generated, dynamic list rather than a hand-kept one. RUN npm install -g --ignore-scripts @earendil-works/pi-coding-agent \ && npm cache clean --force \ && pi --version diff --git a/docs/cli-registry.md b/docs/cli-registry.md index da9fa12a..d60d7319 100644 --- a/docs/cli-registry.md +++ b/docs/cli-registry.md @@ -137,9 +137,14 @@ registry exists to prevent. The install.sh copy is **embedded, not fetched**, and is the FULL catalogue. An earlier design fetched it and fell back to a hardcoded two-CLI list, which degraded silently on an empty -response; there is no degraded mode to fall into now. An optional, opt-in refresh -(`CODEMAN_CLI_CATALOGUE_URL`, or `CODEMAN_REFRESH_CLI_CATALOGUE=1`) exists for a stale local -copy, and warns on all three failure shapes — empty body, unparseable content, failed fetch. +response; there is no degraded mode to fall into now, and no network fetch either — a `curl | +bash` from master already carries a catalogue exactly as fresh as the script itself, so there is +nothing a refresh would buy that isn't already true. An earlier draft added an opt-in refresh +with a `TRUSTED`/`DISPLAY` array split to keep it from ever writing the executed command; it was +dropped before merge rather than shipped half-verified — the split's only actual write was the +label, `DISPLAY` never diverged from `TRUSTED` in practice, and the added surface (a second +array, a fetch path, three failure shapes to warn on) bought nothing the embedded copy didn't +already have. ### The install-command trust boundary @@ -147,12 +152,13 @@ Three rules, and the middle one is why the embed matters: 1. **The server never executes an entry's `install.command`.** Unchanged, and still enforced by nothing executing it: the field is display text (`CliDiscovery.install.command`). 2. **`install.sh` executes only commands embedded in itself.** Those arrive in the same file, over the same TLS fetch, in the same commit as the `curl \| bash` line that fetched the script — identical trust to the hardcoded vendor one-liners it replaces. -3. **Nothing fetched at install time is ever executed.** +3. **Nothing fetched at install time is ever executed.** There is no second code path that fetches anything after the script itself has been fetched. That is mechanical rather than a promise. `CLI_INSTALL_CMD_TRUSTED` is written only from the -generated block and is the only array the installer runs; `CLI_INSTALL_CMD_DISPLAY` is what the -refresh may rewrite. `test/install-sh-invariants.test.ts` asserts the split holds, that the -refresh never assigns into a `*_TRUSTED` array, and that it never `eval`s. +generated block and is the only array the installer ever runs or displays — there is no second +array a refresh could rewrite, because there is no refresh. `test/install-sh-invariants.test.ts` +asserts as much: the embedded commands are exactly the registry's, and nothing in `install.sh` +`eval`s. ### bash 3.2 diff --git a/docs/docker-cases.md b/docs/docker-cases.md index 760aa5bc..f930c126 100644 --- a/docs/docker-cases.md +++ b/docs/docker-cases.md @@ -34,8 +34,9 @@ must not change what is inside an image tagged `codeman/agent:base`, or two mach that tag hold different images and every cache decision downstream is a lie. Each entry's `enabled` flag IS honoured, so a CLI that ships disabled is never baked in. -Four CLIs keep hand-written layers, because the registry cannot express what makes them -special: +Five CLIs keep hand-written layers, because the registry cannot express what makes them +special (as a REGISTRY field now — `discovery.install.agentImageLayer` in `stock.ts` — rather +than an id-keyed table duplicated between the two producers of the image's build args): | CLI | Why it is not in the shared npm layer | | ------------- | ------------------------------------------------------------------------------------- | diff --git a/install.sh b/install.sh index f71a4a6c..97467eff 100755 --- a/install.sh +++ b/install.sh @@ -88,16 +88,16 @@ export PUPPETEER_SKIP_DOWNLOAD="${PUPPETEER_SKIP_DOWNLOAD:-1}" # # ⚠️ TRUST BOUNDARY: CLI_CMD_LINUX/CLI_CMD_DARWIN are the ONLY source of a command this # script will ever execute, and they arrive embedded in this file — same TLS fetch, same -# commit as the script itself. Nothing fetched at install time may write them; the -# refresh may only touch the *_DISPLAY copy. See cli_catalog_select_platform below. +# commit as the script itself. Nothing fetched at install time is ever executed; there is +# no network refresh of these arrays. See cli_catalog_select_platform below. CLI_IDS=('claude' 'shell' 'opencode' 'codex' 'gemini' 'antigravity' 'pi' 'grok' 'deepseek' 'omp') CLI_LABELS=('Claude' 'Shell' 'OpenCode' 'Codex' 'Gemini' 'Antigravity' 'Pi' 'Grok' 'DeepSeek' 'OMP') CLI_ENABLED=(1 1 1 1 1 1 1 1 1 1) CLI_KIND=('agent' 'shell' 'agent' 'agent' 'agent' 'agent' 'agent' 'agent' 'agent' 'agent') CLI_NPM=('@anthropic-ai/claude-code' '' 'opencode-ai' '@openai/codex' '@google/gemini-cli' '' '@earendil-works/pi-coding-agent' '' '@deepseek-ai/dsh' '') CLI_DOCS=('https://docs.claude.com/claude-code' '' 'https://opencode.ai/docs' 'https://developers.openai.com/codex/cli' 'https://github.com/google-gemini/gemini-cli' 'https://antigravity.google/cli' 'https://pi.dev' 'https://github.com/xai-org/grok-build' 'https://github.com/deepseek-ai/deepseek-harness' 'https://omp.sh') -CLI_CMD_LINUX=('curl -fsSL https://claude.ai/install.sh | bash' '' 'curl -fsSL https://opencode.ai/install | bash' 'npm install -g @openai/codex' 'npm install -g @google/gemini-cli' 'curl -fsSL https://antigravity.google/cli/install.sh | bash' 'npm install -g --ignore-scripts @earendil-works/pi-coding-agent' 'curl -fsSL https://x.ai/cli/install.sh | bash' 'npm install -g @deepseek-ai/dsh' 'curl -fsSL https://omp.sh/install | sh') -CLI_CMD_DARWIN=('curl -fsSL https://claude.ai/install.sh | bash' '' 'curl -fsSL https://opencode.ai/install | bash' 'npm install -g @openai/codex' 'npm install -g @google/gemini-cli' 'curl -fsSL https://antigravity.google/cli/install.sh | bash' 'npm install -g --ignore-scripts @earendil-works/pi-coding-agent' 'curl -fsSL https://x.ai/cli/install.sh | bash' 'npm install -g @deepseek-ai/dsh' 'brew install can1357/tap/omp') +CLI_CMD_LINUX=('curl -fsSL https://claude.ai/install.sh | bash' '' 'curl -fsSL https://opencode.ai/install | bash' 'npm install -g @openai/codex' 'npm install -g @google/gemini-cli' 'curl -fsSL https://antigravity.google/cli/install.sh | bash' 'npm install -g --ignore-scripts @earendil-works/pi-coding-agent' 'curl -fsSL https://x.ai/cli/install.sh | bash' '' 'curl -fsSL https://omp.sh/install | sh') +CLI_CMD_DARWIN=('curl -fsSL https://claude.ai/install.sh | bash' '' 'curl -fsSL https://opencode.ai/install | bash' 'npm install -g @openai/codex' 'npm install -g @google/gemini-cli' 'curl -fsSL https://antigravity.google/cli/install.sh | bash' 'npm install -g --ignore-scripts @earendil-works/pi-coding-agent' 'curl -fsSL https://x.ai/cli/install.sh | bash' '' 'brew install can1357/tap/omp') CLI_ALL_BINS=('claude' 'opencode' 'codex' 'gemini' 'agy' 'pi' 'grok' 'dsh' 'omp') CLI_BIN_OFF=(0 1 1 2 3 4 5 6 7 8) CLI_BIN_LEN=(1 0 1 1 1 1 1 1 1 1) @@ -438,7 +438,12 @@ _cli_index() { dsh_banner_probe() { local runner=() if command -v timeout &>/dev/null; then runner=(timeout 5); fi - "${runner[@]}" "$1" --help /dev/null | grep -qi "DeepSeek Harness" + # ⚠️ bash 3.2 (stock macOS): expanding an EMPTY array under `set -u` is an unbound-variable + # error, not a no-op — `${runner[@]}` alone abort­ed this whole probe with "runner[@]: + # unbound variable" whenever `timeout` was absent (i.e. exactly the host this comment is + # about). `${runner[@]+"${runner[@]}"}` expands to nothing when the array is empty and to + # the quoted elements otherwise, which is safe under `set -u` in both bash 3.2 and 4+. + ${runner[@]+"${runner[@]}"} "$1" --help /dev/null | grep -qi "DeepSeek Harness" } # Is "$2" really the CLI "$1" claims to be? @@ -535,21 +540,17 @@ get_cli_path() { # Pick this platform's install commands out of the generated per-platform arrays. # # ⚠️ THE TRUST BOUNDARY LIVES HERE, and it is mechanical rather than a promise: -# -# CLI_INSTALL_CMD_TRUSTED — written ONLY from CLI_CMD_LINUX/CLI_CMD_DARWIN, -# i.e. only from the block generated into this file. -# This is the sole array the installer ever executes. -# CLI_INSTALL_CMD_DISPLAY — the copy shown on screen. The optional catalogue -# refresh may rewrite it; it can never write TRUSTED. -# -# So a command that runs arrived in the same file, over the same TLS fetch, in -# the same commit as the `curl | bash` line that fetched this script. That is -# identical trust to the hardcoded vendor one-liners this replaces, and it is -# why nothing fetched at install time is ever executed. The server keeps its own, -# stricter rule unchanged: it never executes an entry's install command at all -# (see CliDiscovery.install.command in src/config/cli-registry/types.ts). +# CLI_INSTALL_CMD_TRUSTED is written ONLY from CLI_CMD_LINUX/CLI_CMD_DARWIN, i.e. +# only from the block generated into this file, and it is the sole array the +# installer ever executes or displays — there is no second copy a network +# refresh could rewrite. A command that runs therefore arrived in the same +# file, over the same TLS fetch, in the same commit as the `curl | bash` line +# that fetched this script. That is identical trust to the hardcoded vendor +# one-liners this replaces, and it is why nothing fetched at install time is +# ever executed. The server keeps its own, stricter rule unchanged: it never +# executes an entry's install command at all (see CliDiscovery.install.command +# in src/config/cli-registry/types.ts). CLI_INSTALL_CMD_TRUSTED=() -CLI_INSTALL_CMD_DISPLAY=() CLI_PLATFORM_DONE="" cli_catalog_select_platform() { [[ -n "$CLI_PLATFORM_DONE" ]] && return 0 @@ -565,7 +566,6 @@ cli_catalog_select_platform() { else CLI_INSTALL_CMD_TRUSTED[$i]="${CLI_CMD_LINUX[$i]}" fi - CLI_INSTALL_CMD_DISPLAY[$i]="${CLI_INSTALL_CMD_TRUSTED[$i]}" done } @@ -581,10 +581,14 @@ cli_catalog_names() { } # The "install one yourself" hints: every enabled CLI that is not installed, -# showing the DISPLAY command. An entry with no install command (DeepSeek ships -# no vendor one-liner) gets its docs URL instead of being silently omitted, -# which is what used to happen to Gemini — it had a command in the registry and -# appeared in no list in this script. +# showing the trusted install command. An entry with no install command gets +# its docs URL instead of being silently omitted, which is what used to +# happen to Gemini — it had a command in the registry and appeared in no list +# in this script. DeepSeek is the one entry that deliberately HAS a command in +# the registry but an empty one here: installing the launcher alone leaves +# nothing that can drive a pane, so the generator withholds the command for +# any launcherProfile entry (see installCommandFor in generate-cli-catalog.mts) +# and this hint falls through to the docs URL instead. cli_catalog_print_install_hints() { detect_all_clis local i @@ -592,99 +596,15 @@ cli_catalog_print_install_hints() { [[ "${CLI_ENABLED[$i]}" == "1" ]] || continue [[ "${CLI_BIN_LEN[$i]}" -gt 0 ]] || continue [[ -z "${CLI_FOUND_PATH[$i]}" ]] || continue - if [[ -n "${CLI_INSTALL_CMD_DISPLAY[$i]}" ]]; then - echo -e " ${CYAN}${CLI_INSTALL_CMD_DISPLAY[$i]}${NC} # ${CLI_LABELS[$i]}" + if [[ -n "${CLI_INSTALL_CMD_TRUSTED[$i]}" ]]; then + echo -e " ${CYAN}${CLI_INSTALL_CMD_TRUSTED[$i]}${NC} # ${CLI_LABELS[$i]}" elif [[ -n "${CLI_DOCS[$i]}" ]]; then echo -e " ${CLI_LABELS[$i]}: see ${CYAN}${CLI_DOCS[$i]}${NC}" fi done } -# Optional catalogue refresh — OPT-IN, and deliberately so. -# -# When you `curl | bash` from master the embedded catalogue is already exactly as -# fresh as the script that carries it, so a default-on refresh would buy nothing -# and add a network dependency plus a warning surface to every install. It exists -# for the case the embedded copy really can be stale: re-running an old local -# copy, or a fork. -# -# ⚠️ Only DISPLAY and detection-shaped fields are ever overwritten. TRUSTED is -# untouchable from here — see cli_catalog_select_platform. -# ⚠️ Called from main() only, AFTER check_curl_or_wget has set DOWNLOADER; that -# variable is otherwise unset under `set -u`. The guard below keeps a future -# caller that relocates this from dying instead of simply not refreshing. -cli_catalog_refresh() { - local url="${CODEMAN_CLI_CATALOGUE_URL:-}" - if [[ -z "$url" ]] && [[ "${CODEMAN_REFRESH_CLI_CATALOGUE:-0}" == "1" ]]; then - url="$(cli_catalog_default_url)" - fi - [[ -n "$url" ]] || return 0 - [[ -n "${DOWNLOADER:-}" ]] || return 0 - - local tmp - tmp="$(mktemp)" || return 0 - - if ! download "$url" "$tmp" 2>/dev/null; then - warn "CLI catalogue refresh could not fetch $url; using the catalogue built into this installer." - rm -f "$tmp" - return 0 - fi - if [[ ! -s "$tmp" ]]; then - # The failure shape that used to be SILENT: a plain network failure - # returning an empty body, where the old design fell back to a hardcoded - # two-CLI list and said nothing. There is no degraded list to fall back - # to now, and the fallback is announced either way. - warn "CLI catalogue refresh returned nothing (network failure?); using the catalogue built into this installer." - rm -f "$tmp" - return 0 - fi - - # Parsed with node into tab-separated records and read with `read`, never - # eval'd: this content came off the network. - local parsed count=0 id label - parsed="$(node -e ' - const fs = require("fs"); - let data; - try { data = JSON.parse(fs.readFileSync(process.argv[1], "utf8")); } catch { process.exit(2); } - if (!Array.isArray(data)) process.exit(2); - for (const e of data) { - if (!e || typeof e.id !== "string" || typeof e.label !== "string") continue; - if (/[\t\n]/.test(e.id) || /[\t\n]/.test(e.label)) continue; - console.log([e.id, e.label].join("\t")); - } - ' "$tmp" /dev/null)" || { - warn "CLI catalogue refresh returned unparseable content; using the catalogue built into this installer." - rm -f "$tmp" - return 0 - } - rm -f "$tmp" - - while IFS=$'\t' read -r id label; do - [[ -n "$id" ]] || continue - if _cli_index "$id"; then - CLI_LABELS[$CLI_IDX]="$label" - count=$((count + 1)) - fi - done <; npmPackage?: string; docsUrl?: string }; + install: { + command: Record; + npmPackage?: string; + docsUrl?: string; + agentImageLayer?: { kind: 'dedicated'; reason: string }; + }; }; } @@ -75,6 +80,7 @@ function toCatalogEntry(entry: CliEntry): CatalogEntry { command: { ...install.command } as Record, ...(install.npmPackage ? { npmPackage: install.npmPackage } : {}), ...(install.docsUrl ? { docsUrl: install.docsUrl } : {}), + ...(install.agentImageLayer ? { agentImageLayer: { ...install.agentImageLayer } } : {}), }, }, }; @@ -108,8 +114,19 @@ function shPath(dir: string, binary: string): string { * the exact platform, else linux, else whatever is declared. Resolved HERE, at generation * time, so that fallback logic stays in tested TypeScript instead of being reimplemented in * bash against an array the script would have to index by platform anyway. + * + * ⚠️ EMPTY for a `launcherProfile` entry (DeepSeek today), deliberately: `npm install -g + * @deepseek-ai/dsh` installs the LAUNCHER, not something that can drive a pane on its own — it + * ships only the `web`/`headless` profiles, neither of which is a terminal TUI. Emitting the + * command made the installer offer DeepSeek as a normal menu choice: picking it printed + * "DeepSeek installed at ...", counted as a found AI CLI, and left the user with a `dsh` that + * cannot actually run anything, with no mention of the Run dropdown's profile installer that + * fixes that. An empty command here means install.sh's menu-building loop (which requires a + * non-empty CLI_INSTALL_CMD_TRUSTED entry) skips it and the hint printer falls through to the + * docs URL instead — see cli_catalog_print_install_hints in install.sh. */ function installCommandFor(entry: CliEntry, platform: InstallPlatform): string { + if (entry.discovery.launcherProfile) return ''; const { command } = entry.discovery.install; return command[platform] ?? command.linux ?? Object.values(command)[0] ?? ''; } @@ -173,8 +190,8 @@ export function renderInstallShBlock(entries: CliEntry[] = STOCK_CLIS): string { '#', '# ⚠️ TRUST BOUNDARY: CLI_CMD_LINUX/CLI_CMD_DARWIN are the ONLY source of a command this', '# script will ever execute, and they arrive embedded in this file — same TLS fetch, same', - '# commit as the script itself. Nothing fetched at install time may write them; the', - '# refresh may only touch the *_DISPLAY copy. See cli_catalog_select_platform below.', + '# commit as the script itself. Nothing fetched at install time is ever executed; there is', + '# no network refresh of these arrays. See cli_catalog_select_platform below.', arr('CLI_IDS', ids), arr('CLI_LABELS', labels), arr('CLI_ENABLED', enabled), diff --git a/scripts/lib/cli-catalog.mjs b/scripts/lib/cli-catalog.mjs index 348efa90..6d2b8d6a 100644 --- a/scripts/lib/cli-catalog.mjs +++ b/scripts/lib/cli-catalog.mjs @@ -20,17 +20,16 @@ const CATALOG_PATH = fileURLToPath(new URL('../../config/clis.stock.json', impor * ⚠️ Filters on `enabled`. That is the field the earlier attempt's export omitted, which is * how a CLI that ships disabled still had its package baked into every image. * - * ⚠️ SPECIAL_CASES are excluded here and installed by their own hand-written Dockerfile - * layers, because the registry cannot express what makes them special — a flag, a companion - * package, or not being on npm at all. `test/docker-agent-image-coverage.test.ts` requires - * every one of them to carry a reason and to still be present in the Dockerfile, so an - * exclusion cannot quietly become an omission. + * ⚠️ An entry carrying `discovery.install.agentImageLayer` is excluded here and installed by + * its own hand-written Dockerfile layer instead, because the registry cannot express what + * makes it special — a flag, a companion package, or not being on npm at all. This used to be + * an id-keyed table duplicated between this file and `src/docker-hosts.ts` (exactly the shape + * `test/cli-registry-no-id-branching.test.ts` exists to forbid inside `src/`, which is why it + * was a blind spot rather than a pass — that test scans `src/` only). It is data now: both + * producers filter on the SAME field from the SAME catalogue entry, `reason` is required by + * `schema.ts`, and `test/docker-agent-image-coverage.test.ts` requires every one of them to + * still be present in the Dockerfile, so an exclusion cannot quietly become an omission. */ -export const AGENT_IMAGE_SPECIAL_CASES = { - pi: 'installed with --ignore-scripts in its own layer, so the flag cannot leak to the shared block', - deepseek: 'needs pnpm alongside it (dsh plugin, issue #352) and a dsh-tui profile install', -}; - /** Tokens allowed in an npm package name reaching a Dockerfile build arg unquoted. */ const SAFE_PACKAGE = /^[@A-Za-z0-9][@A-Za-z0-9/._-]*$/; @@ -38,13 +37,17 @@ export function agentImageNpmPackages(catalog) { const packages = []; for (const entry of catalog) { if (!entry.enabled) continue; - if (entry.id in AGENT_IMAGE_SPECIAL_CASES) continue; + if (entry.discovery?.install?.agentImageLayer) continue; const pkg = entry.discovery?.install?.npmPackage; if (!pkg) continue; // antigravity/grok/omp ship standalone installers, not npm if (!SAFE_PACKAGE.test(pkg)) { // The value is interpolated into a Dockerfile ARG that is expanded UNQUOTED (word // splitting is how the list becomes several arguments), so a token with whitespace or // shell metacharacters would change what the RUN line means. + // ⚠️ This exact regex is duplicated in `agentImageNpmPackages()` in + // `src/docker-hosts.ts` (that file cannot import this one — it is the TypeScript side of + // the same two-producers split this whole module exists for). Keep both literal patterns + // identical; `test/agent-image-build-args-parity.test.ts` pins that they are. throw new Error(`Refusing unsafe npm package name for "${entry.id}": ${JSON.stringify(pkg)}`); } packages.push(pkg); diff --git a/src/config/cli-registry/schema.ts b/src/config/cli-registry/schema.ts index 10f6d18c..a6ed5524 100644 --- a/src/config/cli-registry/schema.ts +++ b/src/config/cli-registry/schema.ts @@ -187,6 +187,13 @@ const discoverySchema = z .strict(), npmPackage: z.string().max(200).optional(), docsUrl: z.url().optional(), + // Requires a `reason` on purpose — see the field's own doc comment in types.ts. A + // dedicated agent-image layer with no stated reason is a silent id-keyed special case + // rebuilding itself inside the data this change moved it out of. + agentImageLayer: z + .object({ kind: z.literal('dedicated'), reason: z.string().min(1).max(300) }) + .strict() + .optional(), }) .strict(), }) diff --git a/src/config/cli-registry/stock.ts b/src/config/cli-registry/stock.ts index e75428da..29ce9065 100644 --- a/src/config/cli-registry/stock.ts +++ b/src/config/cli-registry/stock.ts @@ -621,6 +621,10 @@ const PI: CliEntry = { }, npmPackage: '@earendil-works/pi-coding-agent', docsUrl: 'https://pi.dev', + agentImageLayer: { + kind: 'dedicated', + reason: 'installed with --ignore-scripts in its own layer, so the flag cannot leak to the shared block', + }, }, }, launch: { @@ -835,6 +839,10 @@ const DEEPSEEK: CliEntry = { }, npmPackage: '@deepseek-ai/dsh', docsUrl: 'https://github.com/deepseek-ai/deepseek-harness', + agentImageLayer: { + kind: 'dedicated', + reason: 'needs pnpm alongside it (dsh plugin, issue #352) and a dsh-tui profile install', + }, }, }, launch: { diff --git a/src/config/cli-registry/types.ts b/src/config/cli-registry/types.ts index 174993a9..0b572986 100644 --- a/src/config/cli-registry/types.ts +++ b/src/config/cli-registry/types.ts @@ -230,6 +230,22 @@ export interface CliDiscovery { /** Package name for an npm-installable CLI. Display/tooling metadata only. */ npmPackage?: string; docsUrl?: string; + /** + * Present when the agent Docker image (`docker/agent.Dockerfile`) cannot install this + * CLI in the shared `npm install -g` layer with the rest and needs its own hand-written + * layer instead — a flag that would leak into the shared install (pi's `--ignore-scripts`), + * a companion package (deepseek's `pnpm`), or not being on npm at all (antigravity, grok, + * omp ship standalone installers). `reason` is REQUIRED, not decorative: it is what + * `test/docker-agent-image-coverage.test.ts` prints when a layer for this id goes missing + * from the Dockerfile, and it is what keeps this a data field rather than the id-keyed + * table it replaced (`AGENT_IMAGE_SPECIAL_CASE_IDS` in `docker-hosts.ts`, + * `AGENT_IMAGE_SPECIAL_CASES` in `scripts/lib/cli-catalog.mjs` — two copies kept in step by + * hand, outside stock.ts, which is exactly what this registry exists to prevent). + * `agentImageNpmPackages()` (docker-hosts.ts) and its `.mjs` mirror both filter on its + * presence rather than an id, so the shared npm layer and the special-case layers can never + * silently disagree about which CLI belongs in which. + */ + agentImageLayer?: { kind: 'dedicated'; reason: string }; }; } diff --git a/src/docker-hosts.ts b/src/docker-hosts.ts index 68719abb..d83ef866 100644 --- a/src/docker-hosts.ts +++ b/src/docker-hosts.ts @@ -508,6 +508,9 @@ export function agentImageBuildArgs( ]; } +/** Tokens allowed in an npm package name reaching a Dockerfile build arg unquoted. */ +const SAFE_PACKAGE = /^[@A-Za-z0-9][@A-Za-z0-9/._-]*$/; + /** * npm packages the agent image installs in its shared layer, from the STOCK catalogue. * @@ -515,23 +518,36 @@ export function agentImageBuildArgs( * change what lands inside an image tagged `codeman/agent:base`, or two machines holding that * same tag hold different images and every cache-hit decision downstream is a lie. * + * ⚠️ An entry carrying `discovery.install.agentImageLayer` is excluded here — see that field's + * doc comment in `types.ts` for why some CLIs need their own hand-written Dockerfile layer + * instead of the shared one, and `test/docker-agent-image-coverage.test.ts` for the guard that + * an exclusion here still lands in the Dockerfile somewhere. + * * ⚠️ This mirrors `agentImageNpmPackages()` in `scripts/lib/cli-catalog.mjs`, which the CLI * build path uses because a `.mjs` cannot import TypeScript. Two producers of one command - * line drift; `test/agent-image-build-args-parity.test.ts` is what stops them. + * line drift; `test/agent-image-build-args-parity.test.ts` is what stops them — including the + * SAFE_PACKAGE regex below, which is duplicated (not imported) in that file for the same + * reason and must stay byte-identical to it. */ export function agentImageNpmPackages(): string[] { - return STOCK_CLIS.filter((entry) => entry.enabled && !AGENT_IMAGE_SPECIAL_CASE_IDS.has(entry.id as string)) - .map((entry) => entry.discovery.install.npmPackage) - .filter((pkg): pkg is string => Boolean(pkg)); + const packages: string[] = []; + for (const entry of STOCK_CLIS) { + if (!entry.enabled || entry.discovery.install.agentImageLayer) continue; + const pkg = entry.discovery.install.npmPackage; + if (!pkg) continue; + // The value is interpolated into a Dockerfile ARG expanded UNQUOTED (word splitting is + // how the list becomes several arguments), so a token with whitespace or shell + // metacharacters would change what the RUN line means. The source is `stock.ts`, so the + // practical risk is nil, but this is the in-app auto-build path and the only one of the + // two producers where that had gone unchecked. + if (!SAFE_PACKAGE.test(pkg)) { + throw new Error(`Refusing unsafe npm package name for "${String(entry.id)}": ${JSON.stringify(pkg)}`); + } + packages.push(pkg); + } + return packages; } -/** - * CLIs the agent image installs in their OWN Dockerfile layer rather than the shared npm one. - * The registry cannot express what makes each special, so the layers stay hand-written and - * `test/docker-agent-image-coverage.test.ts` requires each to carry a reason and still exist. - */ -const AGENT_IMAGE_SPECIAL_CASE_IDS = new Set(['pi', 'deepseek']); - /** The `--build-arg` pairs the agent image takes. */ export function agentImageBuildArgPairs(): Array<[string, string]> { return [['CLI_NPM_PACKAGES', agentImageNpmPackages().join(' ')]]; diff --git a/test/agent-image-build-args-parity.test.ts b/test/agent-image-build-args-parity.test.ts index f3a1d695..89c0cdd1 100644 --- a/test/agent-image-build-args-parity.test.ts +++ b/test/agent-image-build-args-parity.test.ts @@ -77,4 +77,22 @@ describe('agent-image build args: the .mjs and the TS mirror agree', () => { expect(declared, 'the Dockerfile no longer declares CLI_NPM_PACKAGES').toBeDefined(); expect(declared).toBe(tsPackages().join(' ')); }); + + it('validates an unsafe package name with the SAME regex on both sides', () => { + // Equal OUTPUT on today's catalogue (asserted above) does not prove equal VALIDATION — a + // looser regex on one side would only show up the day someone ships a hostile package name. + // The regex is duplicated rather than shared (the .mjs side cannot import the .ts side, the + // whole reason this file exists), so pin the literal PATTERN text is identical between the + // two source files rather than trusting the comment that says so. + const tsSource = readFileSync(fileURLToPath(new URL('../src/docker-hosts.ts', import.meta.url)), 'utf-8'); + const mjsSource = readFileSync(fileURLToPath(new URL('../scripts/lib/cli-catalog.mjs', import.meta.url)), 'utf-8'); + const extract = (source: string, file: string): string => { + // Non-greedy to `/;` deliberately: the pattern itself contains a `/` (inside the + // character class), so a naive `[^/]+` stops at the wrong slash. + const m = /const SAFE_PACKAGE = (\/.+?\/);/.exec(source); + expect(m, `could not find the SAFE_PACKAGE regex literal in ${file}`).toBeDefined(); + return m![1]; + }; + expect(extract(tsSource, 'docker-hosts.ts')).toBe(extract(mjsSource, 'cli-catalog.mjs')); + }); }); diff --git a/test/docker-agent-image-coverage.test.ts b/test/docker-agent-image-coverage.test.ts index 18ffeefa..a6b8b0ca 100644 --- a/test/docker-agent-image-coverage.test.ts +++ b/test/docker-agent-image-coverage.test.ts @@ -16,7 +16,7 @@ import { describe, expect, it } from 'vitest'; import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; -import { AGENT_IMAGE_SPECIAL_CASES, agentImageNpmPackages } from '../scripts/lib/cli-catalog.mjs'; +import { agentImageNpmPackages } from '../scripts/lib/cli-catalog.mjs'; import { STOCK_CLIS } from '../src/config/cli-registry/stock.js'; const read = (rel: string): string => readFileSync(fileURLToPath(new URL(`../${rel}`, import.meta.url)), 'utf-8'); @@ -27,40 +27,53 @@ const INDEX_HTML = read('src/web/public/index.html'); const CATALOG = JSON.parse(read('config/clis.stock.json')) as Array<{ id: string; enabled: boolean; - discovery: { binaries: string[]; install: { npmPackage?: string } }; + discovery: { + binaries: string[]; + install: { npmPackage?: string; agentImageLayer?: { kind: 'dedicated'; reason: string } }; + }; }>; const enabledAgents = CATALOG.filter((e) => e.enabled && e.discovery.binaries.length > 0); +/** + * A layer's PROOF it installed the right thing, not merely a substring anywhere in the file. + * Every dedicated layer in agent.Dockerfile ends by running ` --version`, so anchoring + * on that (rather than `Dockerfile.includes(binary)`) survives a layer being deleted while its + * COMMENT — which also names the binary — is left behind. That gap is why this replaced the + * looser check. + */ +const hasVersionProof = (binary: string): boolean => AGENT_DOCKERFILE.includes(`${binary} --version`); + describe('docker agent image covers the catalogue', () => { - it('installs every enabled npm CLI, via the build arg or a documented special case', () => { + it('installs every enabled npm CLI, via the build arg or a documented dedicated layer', () => { const inBuildArg = new Set(agentImageNpmPackages(CATALOG)); const missing: string[] = []; for (const entry of enabledAgents) { const pkg = entry.discovery.install.npmPackage; if (!pkg) continue; // standalone installer, checked below if (inBuildArg.has(pkg)) continue; - if (entry.id in AGENT_IMAGE_SPECIAL_CASES) continue; + if (entry.discovery.install.agentImageLayer) continue; missing.push(`${entry.id} (${pkg})`); } expect( missing, - `npm CLI reaches neither the build arg nor a special case:\n ${missing.join('\n ')}\n` + - 'Add it to the arg (it is automatic) or give it a Dockerfile layer AND a reason in AGENT_IMAGE_SPECIAL_CASES.' + `npm CLI reaches neither the build arg nor a dedicated layer:\n ${missing.join('\n ')}\n` + + 'Add it to the arg (it is automatic) or give it a Dockerfile layer AND an agentImageLayer.reason in stock.ts.' ).toEqual([]); }); - it('gives every special case a reason and a real layer', () => { - for (const [id, reason] of Object.entries(AGENT_IMAGE_SPECIAL_CASES)) { - expect(reason.length, `${id} has an empty reason`).toBeGreaterThan(20); - const entry = CATALOG.find((e) => e.id === id); - expect(entry, `${id} is a special case but not in the catalogue`).toBeDefined(); - const pkg = entry?.discovery.install.npmPackage; - // Excluded from the shared arg, so it MUST appear in a hand-written layer, or it is - // simply not installed at all — an exclusion silently becoming an omission. + it('gives every dedicated-layer entry a reason and a real, provable layer', () => { + for (const entry of CATALOG) { + const layer = entry.discovery.install.agentImageLayer; + if (!layer) continue; + expect(layer.reason.length, `${entry.id} has an empty agentImageLayer.reason`).toBeGreaterThan(20); + const binary = entry.discovery.binaries[0]; + // Excluded from the shared arg, so it MUST appear in a hand-written layer that actually + // ran the binary, or it is simply not installed at all — an exclusion silently becoming + // an omission. expect( - AGENT_DOCKERFILE.includes(pkg ?? id), - `${id} is excluded from the arg but absent from the Dockerfile` + hasVersionProof(binary), + `${entry.id} is excluded from the arg but has no "${binary} --version" proof line in the Dockerfile` ).toBe(true); } }); @@ -70,8 +83,8 @@ describe('docker agent image covers the catalogue', () => { if (entry.discovery.install.npmPackage) continue; const binary = entry.discovery.binaries[0]; expect( - AGENT_DOCKERFILE.includes(binary), - `${entry.id} ships no npm package and no Dockerfile layer mentions "${binary}"` + hasVersionProof(binary), + `${entry.id} ships no npm package and no Dockerfile layer proves it ran "${binary} --version"` ).toBe(true); } }); diff --git a/test/install-sh-invariants.test.ts b/test/install-sh-invariants.test.ts index 8a4c2c8b..70a70915 100644 --- a/test/install-sh-invariants.test.ts +++ b/test/install-sh-invariants.test.ts @@ -93,9 +93,13 @@ describe('install.sh generated-catalogue block', () => { }); describe('install.sh trust boundary', () => { - // The whole point of splitting TRUSTED from DISPLAY: a command the installer EXECUTES must - // have arrived embedded in this file, over the same TLS fetch and in the same commit as the - // script itself. Anything pulled from the network at install time is display-only. + // A command the installer EXECUTES must have arrived embedded in this file, over the same + // TLS fetch and in the same commit as the script itself — there is no second, network-derived + // copy of these commands anywhere in the script (an earlier draft that added one, and split + // a TRUSTED/DISPLAY pair to keep the fetched copy display-only, was dropped before merge: + // see docs/cli-registry.md). These three assertions are what is left to guard now that the + // fetch path itself does not exist: everything the installer runs or shows still comes only + // from the generated block, and nothing in the file eval()s. it('writes CLI_INSTALL_CMD_TRUSTED only from the generated per-platform arrays', () => { const writes = CODE_LINES.filter((line) => /CLI_INSTALL_CMD_TRUSTED\s*\[[^\]]*\]\s*=/.test(line)); expect(writes.length, 'expected exactly the two platform assignments').toBe(2); @@ -106,23 +110,31 @@ describe('install.sh trust boundary', () => { } }); - it('never lets the refresh touch a *_TRUSTED array', () => { - const refresh = CODE.slice(CODE.indexOf('cli_catalog_refresh() {')); - const body = refresh.slice(0, refresh.indexOf('\n}\n')); - expect(body.length, 'could not isolate cli_catalog_refresh').toBeGreaterThan(0); - expect(/_TRUSTED\s*\[[^\]]*\]\s*=/.test(body), 'the refresh assigns into a TRUSTED array').toBe(false); + it('fetches no CLI catalogue over the network at install time', () => { + // The exact shape of the earlier, dropped design: a URL built from the repo/branch this + // script came from, an opt-in env var to enable it, and a `download()` call feeding + // straight into the trusted arrays. None of that exists in this file any more; this pins + // the absence so it cannot quietly come back without a reviewer noticing. + for (const needle of [ + 'cli_catalog_refresh', + 'cli_catalog_default_url', + 'CODEMAN_CLI_CATALOGUE_URL', + 'CODEMAN_REFRESH_CLI_CATALOGUE', + 'CLI_INSTALL_CMD_DISPLAY', + ]) { + expect(SOURCE.includes(needle), `${needle} should not exist — the catalogue refresh was dropped`).toBe(false); + } }); - it('never eval()s network-derived catalogue data', () => { - // Scoped to the refresh deliberately. install.sh has two long-standing, legitimate evals - // elsewhere (`eval "$(brew shellenv)"`, Homebrew's documented idiom, and one inside a - // node -e that reads `tailscale serve status`), and banning the word outright would flag - // those while saying nothing about the line that matters: `eval` on a fetched file would - // hand the shell to whatever answered the request. - const refresh = CODE.slice(CODE.indexOf('cli_catalog_refresh() {')); - const body = refresh.slice(0, refresh.indexOf('\n}\n')); - expect(body.length, 'could not isolate cli_catalog_refresh').toBeGreaterThan(0); - expect(/\beval\b/.test(body), 'the catalogue refresh eval()s something').toBe(false); + it('never eval()s anything', () => { + // install.sh has two long-standing, legitimate evals (`eval "$(brew shellenv)"`, Homebrew's + // documented idiom, and one inside a node -e that reads `tailscale serve status`), both of + // which operate on output this script itself produced, never on fetched content. With no + // network-derived catalogue left to eval, the word should not appear at all outside those. + const offenders = CODE_LINES.filter( + (line) => /\beval\b/.test(line) && !/eval "\$\(.*shellenv\)"/.test(line) && !line.includes('eval(process.argv') + ); + expect(offenders, `unexpected eval:\n ${offenders.join('\n ')}`).toEqual([]); }); it("redirects stdin for every command it executes on the user's behalf", () => { @@ -137,15 +149,6 @@ describe('install.sh trust boundary', () => { }); describe('install.sh runtime safety', () => { - it('guards the catalogue refresh on DOWNLOADER being set', () => { - // DOWNLOADER is assigned only by check_curl_or_wget, which only main() calls. Any path - // that reaches the refresh without it (the `tailscale` subcommand is one) would abort on - // an unbound variable under `set -u` rather than simply skipping the refresh. - const refresh = CODE.slice(CODE.indexOf('cli_catalog_refresh() {')); - const body = refresh.slice(0, refresh.indexOf('\n}\n')); - expect(body).toMatch(/\[\[\s*-n\s*"\$\{DOWNLOADER:-\}"\s*\]\]\s*\|\|\s*return 0/); - }); - it('can be sourced without installing anything', () => { // The bash 3.2 CI step sources this file to exercise detect_all_clis. Without the guard // the dispatch `case` at the tail would run a real install inside the container.