diff --git a/docs/cli-registry.md b/docs/cli-registry.md index 4e7d454c..fff6c057 100644 --- a/docs/cli-registry.md +++ b/docs/cli-registry.md @@ -4,15 +4,15 @@ Every run mode Codeman can launch — Claude Code, Terminal/Shell, OpenCode, Cod ## Where it lives -| File | What it holds | -| ------------- | --------------------------------------------------------------------------------------------------- | -| `types.ts` | The `CliEntry` interface and everything under it. Read this first. | -| `stock.ts` | The shipped catalog. **The only file allowed to name a CLI id.** | -| `schema.ts` | Zod validation, including the cross-field checks that reject an incoherent entry at LOAD time. | -| `argv.ts` | The argv engine: the only code that turns typed tokens into a command string. | -| `patterns.ts` | The NAMED value patterns (`model`, `uuid`, `path-segment`, …) and the regex-compilation guard. | +| File | What it holds | +| ------------- | ------------------------------------------------------------------------------------------------- | +| `types.ts` | The `CliEntry` interface and everything under it. Read this first. | +| `stock.ts` | The shipped catalog. **The only file allowed to name a CLI id.** | +| `schema.ts` | Zod validation, including the cross-field checks that reject an incoherent entry at LOAD time. | +| `argv.ts` | The argv engine: the only code that turns typed tokens into a command string. | +| `patterns.ts` | The NAMED value patterns (`model`, `uuid`, `path-segment`, …) and the regex-compilation guard. | | `profiles.ts` | The names of behaviours that genuinely need code, kept import-free so `schema.ts` can validate one. | -| `registry.ts` | Loading, merging `~/.codeman/clis.json`, and the accessors (`getCli`, `enabledClis`). | +| `registry.ts` | Loading, merging `~/.codeman/clis.json`, and the accessors (`getCli`, `enabledClis`). | `src/session-cli-registry-bridge.ts` maps the legacy per-mode option bag onto the engine, and `src/utils/cli-resolver.ts` / `src/utils/cli-launcher.ts` do registry-driven binary resolution and launcher-profile dispatch. @@ -28,7 +28,7 @@ App Settings → Agents & CLIs → **CLI management** (`cliManagementEnabled`, d - installs a missing **stock** CLI by running its shipped install command, after a confirm that names the exact command. Only one install per CLI runs at a time, and the command runs without any `CODEMAN_*` variable in its environment. An `npm install -g` command is pointed at `~/.local` (which every resolver searches) when the npm global prefix is not writable by the server user, for example a system node under `/usr`; a writable prefix, a prefix that does not exist yet but could be created, and an explicit `NPM_CONFIG_PREFIX` are left alone. A custom entry's install command is never executed. - adds, edits and deletes **custom** entries (id, label, badge, binaries, launch argv). The server re-validates the whole assembled entry through `CliEntrySchema`, so the form cannot bypass the load-time rules. -These are the only writes to `clis.json`. They are serialized, and a file that does not parse or has unsafe permissions is refused rather than overwritten; fix it (or `chmod 600` it) and retry. The HTTP routes are listed in `docs/api-reference.md` under _CLI management_. +These are the only writes to `clis.json`. They are serialized, and a file that does not parse or has unsafe permissions is refused rather than overwritten; fix it (or `chmod 600` it) and retry. The HTTP routes are listed in `docs/api-reference.md` under *CLI management*. ## The shape of an entry @@ -67,7 +67,7 @@ Six capability fields carry a regular expression an override file can set: `disc `workingLine` is the one that matters most, because it is compiled once per session and then run against every accumulated PTY chunk and every pane capture. A nested quantifier there is a ReDoS against the event loop for the whole server, not just that session. The guard therefore runs in two places, and neither is redundant: `schema.ts` rejects the entry at LOAD time so a bad pattern never reaches a session, and `_workingLinePattern()` in `session.ts` compiles through the same helper so the runtime cannot end up with a pattern the schema would have refused. -`modelDetect.screenLine` names the model a session runs, for the tile grid's and the split pane's headers (`SessionState.displayModel`). It must have exactly ONE capture group, the model, which `schema.ts` checks at LOAD time, and it runs over the last `screenLines` (1 to 8, default 1) non-blank rows of the capture the idle/working probe already takes, joined with newlines so a pattern can anchor on the row above. Like `watchingLine`, the rows are pane text the agent writes most of, so a pattern must anchor on chrome only that CLI draws. The two stock ones, measured on live panes: dsh-TUI's status line on the row under its composer's rounded border (`╰─+╯\n ?()`, three rows), codex's ` ·` footer (its last row, or the row above codex 0.162's indented hint row, so a two-row window), and opencode's composer agent row (`┃ Build `, directly above the box's `╹` edge, eight rows because its home screen puts up to five rows of its own chrome below it). opencode's field is the model AND the provider, since only colour separates them on that row; the pattern takes the LAST such row in the window, so a composer-shaped row the agent prints higher up cannot stand in for it. A screen that does not match keeps the last model the session reported; a CLI without the field shows its launch model, if any. Claude needs none: its statusLine exporter reports `model.display_name` on every render. ⚠️ dsh-TUI's first field is the model only while its status bar's model field is on; switched off, it is the next field: the reasoning effort (` medium · `), the session mode, or the folder name. So a captured field is not taken when it is one of the CLI's declared `modelDetect.rejectWords` (single tokens, compared ignoring case; dsh lists every effort id its adapters offer and the shipped mode ids) or the session's own working-directory basename (the shared reader's rule, for every CLI). Anything else the pattern captures is the model, so the official `deepseek-chat` / `deepseek-reasoner` ids are read. +`modelDetect.screenLine` names the model a session runs, for the tile grid's and the split pane's headers (`SessionState.displayModel`). It must have exactly ONE capture group, the model, which `schema.ts` checks at LOAD time, and it runs over the last `screenLines` (1 to 8, default 1) non-blank rows of the capture the idle/working probe already takes, joined with newlines so a pattern can anchor on the row above. Like `watchingLine`, the rows are pane text the agent writes most of, so a pattern must anchor on chrome only that CLI draws. The two stock ones, measured on live panes: dsh-TUI's status line on the row under its composer's rounded border (`╰─+╯\n ?()`, three rows), codex's ` · ` footer (its last row, or the row above codex 0.162's indented hint row, so a two-row window), and opencode's composer agent row (`┃ Build `, directly above the box's `╹` edge, eight rows because its home screen puts up to five rows of its own chrome below it). opencode's field is the model AND the provider, since only colour separates them on that row; the pattern takes the LAST such row in the window, so a composer-shaped row the agent prints higher up cannot stand in for it. A screen that does not match keeps the last model the session reported; a CLI without the field shows its launch model, if any. Claude needs none: its statusLine exporter reports `model.display_name` on every render. ⚠️ dsh-TUI's first field is the model only while its status bar's model field is on; switched off, it is the next field: the reasoning effort (` medium · `), the session mode, or the folder name. So a captured field is not taken when it is one of the CLI's declared `modelDetect.rejectWords` (single tokens, compared ignoring case; dsh lists every effort id its adapters offer and the shipped mode ids) or the session's own working-directory basename (the shared reader's rule, for every CLI). Anything else the pattern captures is the model, so the official `deepseek-chat` / `deepseek-reasoner` ids are read. `modelDetect.configResolver` names a READER in `src/model-config-resolvers.ts` (a name, never code in config, like a launcher profile) that resolves the model the CLI's own config pins for one session, for while its screen names none (the `config` source of `displayModel`, ranked below any report from the running CLI). It runs at every pane start, attach and relaunch, with the session's own launch config and env, and must be read-only, bounded (probe before read, no synchronous filesystem call) and return the model id alone. The one stock reader, `deepseek-route` (`src/deepseek-route-config.ts`), resolves dsh-TUI's route the way dsh composes it for the session's profile under the session's `DSH_HOME`: the last of `profiles//cordis.patch.yml` and `$DSH_HOME/cordis.patch.yml` carrying `config` for the `dsh-tui` row counts, and only when it names both `provider` and `model`. Anything in doubt answers nothing: a half-pinned route, a profile without dsh-TUI, an unreadable, oversized or symlinked-out layer, a file beyond its narrow YAML subset. @@ -205,7 +205,7 @@ Measured 2026-10-11 against the installed builds or their source: every CLI in t The composed command line is interpolated into `bash -c "…"` inside tmux, which makes command construction a security boundary. Four independent layers keep config out of it: -1. **Config contains no shell text.** There is no `command: "..."` field anywhere in the schema. An entry declares a sequence of typed tokens; `argv.ts` is the only place that turns them into a string, and it owns every separator itself — one space between tokens, `||` between fallback variants. Neither can originate from config, because config has no field that could hold either. +1. **Config contains no shell text.** There is no `command: "..."` field anywhere in the schema. An entry declares a sequence of typed tokens; `argv.ts` is the only place that turns them into a string, and it owns every separator itself — one space between tokens, ` || ` between fallback variants. Neither can originate from config, because config has no field that could hold either. 2. **Every literal is validated at LOAD time** against a safe-word pattern (no space, quote, backtick, `$`, `;`, `&`, `|`, redirection, parens, braces, newline or backslash). A bad literal **rejects the whole entry** rather than being dropped, because a silently dropped flag would change security-relevant behaviour — losing `--no-approve` is not a cosmetic difference. 3. **Values resolve through NAMED patterns.** A value placeholder selects a `TokenPattern` (`model`, `uuid`, `slug`, `path-segment`, `tool-list`, …) from `patterns.ts`; config can never supply its own regex for a value, so a `clis.json` structurally cannot widen its own validation. A value that fails its pattern drops the whole argument, exactly as the hand-written builders did: an invalid `--model` omits `--model`, it never substitutes something else. 4. **Escaping is independent of validation.** `renderToken()` re-checks the resolved value before emitting it unquoted, and single-quotes anything else — so even a value that somehow bypassed validation is quoted, never concatenated raw. @@ -227,12 +227,12 @@ The names live in `profiles.ts`, which is kept free of imports so `schema.ts` ca DeepSeek is worth reading before assuming an entry looks like its siblings — the schema carries four extensions because of it. -| What it breaks | How the registry expresses it | -| -------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------ | -| `dsh` is a profile LAUNCHER, not the agent, so "installed" is not "runnable". | `discovery.launcherProfile` + `discovery.launcherTargetParam`. | +| What it breaks | How the registry expresses it | +| ------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------ | +| `dsh` is a profile LAUNCHER, not the agent, so "installed" is not "runnable". | `discovery.launcherProfile` + `discovery.launcherTargetParam`. | | Its permission switch is the **`DSH_PERMISSION_MODE` env var**, not a flag — the harness has none. | `env.configSetenv` (so the ordinary `privilegedParams` clamp still reaches it) **and** `capabilities.privilegedEnvKeys`. | -| It is the only non-claude mode with real hook signals, and for it that is a per-SESSION question. | `capabilities.hooks: 'supervised'` — a third state, not a boolean. | -| Its transcript is zstd session files, one frame per write. | `capabilities.transcript: 'deepseek-zstd'`. | +| It is the only non-claude mode with real hook signals, and for it that is a per-SESSION question. | `capabilities.hooks: 'supervised'` — a third state, not a boolean. | +| Its transcript is zstd session files, one frame per write. | `capabilities.transcript: 'deepseek-zstd'`. | ## Identity probes @@ -272,10 +272,10 @@ Two things need the catalogue but cannot import TypeScript, so `npm run generate (`scripts/generate-cli-catalog.mts`) emits two artifacts from `stock.ts`. Both are committed, and `test/cli-catalog-sync.test.ts` fails if either drifts from a fresh generation. -| Artifact | Consumer | Why it exists | -| ---------------------------------- | -------------------------------------------------------- | ------------------------------------------------------------------------------ | -| `config/clis.stock.json` | `scripts/lib/cli-catalog.mjs` (Docker build args), tests | A `.mjs` cannot import the registry. | -| a marked block inside `install.sh` | the installer itself | It runs via `curl \| bash` before any checkout exists, so it can read neither. | +| Artifact | Consumer | Why it exists | +| ------------------------------------ | ---------------------------------- | ---------------------------------------------------------------------------------- | +| `config/clis.stock.json` | `scripts/lib/cli-catalog.mjs` (Docker build args), tests | A `.mjs` cannot import the registry. | +| a marked block inside `install.sh` | the installer itself | It runs via `curl \| bash` before any checkout exists, so it can read neither. | Only `id`, `label`, `shortBadge`, `enabled`, `order`, `kind` and `discovery` are exported. `launch`, `env`, `capabilities` and `overlays` are spawn-time concerns the server alone @@ -334,11 +334,11 @@ A module-level const freezes at first import, and the failure is asymmetric: a C ## MCP server sync -`capabilities.mcpConfig` (`{ path, format, relocation? }`, `path` relative to the home directory) names the file a CLI keeps its user-level MCP server list in and the dialect it is written in. `src/mcp-sync.ts` reads that list from every ENABLED CLI that declares one, and that is installed or already has the file (a CLI that is neither is reported `absent`, never created), and adds any server a CLI is missing from the others. It writes other tools' own config, so it is **opt-in**: `mcpSyncEnabled` (synced, default OFF) gates `GET`/`POST /api/mcp-sync` (403 while off) and the Settings → Agents & CLIs → MCP servers controls. Declared today for claude, gemini, codex, opencode and antigravity (plus GitHub Copilot CLI as a sync-only target, below); every format was checked against what the CLI's own `mcp add` writes, except opencode's (documented, not installed to check). A CLI with no entry (pi, grok, omp, deepseek) is not guessed at: it is listed as `unsupported` in the result when enabled. Adding one is a registry entry plus a small adapter in `mcp-sync.ts`, and a verified fixture in `test/mcp-sync.test.ts`. +`capabilities.mcpConfig` (`{ path, format, relocation? }`, `path` relative to the home directory) names the file a CLI keeps its user-level MCP server list in and the dialect it is written in. `src/mcp-sync.ts` reads that list from every ENABLED CLI that declares one, and that is installed or already has the file (a CLI that is neither is reported `absent`, never created), and adds any server a CLI is missing from the others. It writes other tools' own config, so it is **opt-in**: `mcpSyncEnabled` (synced, default OFF) gates `GET`/`POST /api/mcp-sync` (403 while off) and the Settings → Agents & CLIs → MCP servers controls. Declared today for claude, gemini, codex, opencode and antigravity (and GitHub Copilot CLI, below); every format was checked against what the CLI's own `mcp add` writes, except opencode's (documented, not installed to check). A CLI with no entry (pi, grok, omp, deepseek) is not guessed at: it is listed as `unsupported` in the result when enabled. Adding one is a registry entry plus a small adapter in `mcp-sync.ts`, and a verified fixture in `test/mcp-sync.test.ts`. -**Tools that are not run modes, or that declare no `mcpConfig`.** GitHub Copilot CLI keeps an MCP list worth syncing and is a run mode, but its registry entry declares no `mcpConfig`; its sync target is the plain-data entry below. `src/mcp-sync-targets.ts` declares such tools as plain data (`MCP_SYNC_ONLY_TOOLS`: id, label, config path, dialect, relocation var, the binary whose presence means "installed"). They join the registry CLIs as sync targets (listed after them, so a registry CLI's definition wins a same-name difference), under the same rules: installed or already configured, otherwise `absent`. Copilot's dialect is `copilot-json` (`~/.copilot/mcp-config.json`, relocated by `COPILOT_HOME`; checked against `copilot mcp add` 1.0.94): `mcpServers`, each entry with `tools` (`["*"]` = all), `type` `local` | `http` | `sse`, `command`/`args`/`env` or `url`/`headers`. `copilot mcp disable` does not mark the entry: it lists the name under `disabledMcpServers` in `settings.json` beside the config. Sync reads that list (never writes it) so a disabled server is not copied, and reports the target `unreadable` if `settings.json` is not valid JSON rather than guessing. +**Tools that are not run modes, or that declare no `mcpConfig`.** `src/mcp-sync-targets.ts` declares such tools as plain data (`MCP_SYNC_ONLY_TOOLS`: id, label, config path, dialect, relocation var, the binary whose presence means "installed"). They join the registry CLIs as sync targets (listed after them, so a registry CLI's definition wins a same-name difference), under the same rules: installed or already configured, otherwise `absent`. GitHub Copilot CLI is both: its registry entry declares `mcpConfig`, so while the run mode is enabled the sync-only row is skipped, and it applies only if the run mode is disabled. Copilot's dialect is `copilot-json` (`~/.copilot/mcp-config.json`, relocated by `COPILOT_HOME`; checked against `copilot mcp add` 1.0.94): `mcpServers`, each entry with `tools` (`["*"]` = all), `type` `local` | `http` | `sse`, `command`/`args`/`env` or `url`/`headers`. `copilot mcp disable` does not mark the entry: it lists the name under `disabledMcpServers` in `settings.json` beside the config. Sync reads that list (never writes it) so a disabled server is not copied, and reports the target `unreadable` if `settings.json` is not valid JSON rather than guessing. -`relocation` (`{ envVar, path }`) names the env var the CLI itself reads to move that file: claude `CLAUDE_CONFIG_DIR` (`.claude.json` under it), codex `CODEX_HOME` (`config.toml`), opencode `XDG_CONFIG_HOME` (`opencode/opencode.json`), gemini `GEMINI_CLI_HOME` (`.gemini/settings.json`) and, for the sync-only Copilot CLI, `COPILOT_HOME` (`mcp-config.json`); antigravity follows `$HOME` only, so it declares none. The var is read from the SERVER process env at call time, which is the env the CLIs Codeman spawns inherit. An absolute value moves the file to `/`, an empty one counts as unset (as it does for each CLI), and anything else reports the target `skipped` with the reason instead of writing a file the CLI never reads. A per-session relocation (a session's own `CLAUDE_CONFIG_DIR` in `envOverrides`) is not followed: the sync only knows the server's environment. +`relocation` (`{ envVar, path }`) names the env var the CLI itself reads to move that file: claude `CLAUDE_CONFIG_DIR` (`.claude.json` under it), codex `CODEX_HOME` (`config.toml`), opencode `XDG_CONFIG_HOME` (`opencode/opencode.json`), gemini `GEMINI_CLI_HOME` (`.gemini/settings.json`) copilot `COPILOT_HOME` (`mcp-config.json`); antigravity follows `$HOME` only, so it declares none. The var is read from the SERVER process env at call time, which is the env the CLIs Codeman spawns inherit. An absolute value moves the file to `/`, an empty one counts as unset (as it does for each CLI), and anything else reports the target `skipped` with the reason instead of writing a file the CLI never reads. A per-session relocation (a session's own `CLAUDE_CONFIG_DIR` in `envOverrides`) is not followed: the sync only knows the server's environment. The rules the module keeps and the tests pin: it only ADDS (a name already defined, in any shape, is never edited or removed; a same-name difference is reported as a conflict); a server switched off in its own CLI is not copied; it never writes a file it could not parse (opencode JSONC with comments, a TOML file with a duplicate table) and re-parses the new text before writing; codex TOML is read with a real parser (`smol-toml`), so CRLF files and inline tables are handled; names such as `__proto__` are ignored and every table keyed by an untrusted name has no prototype; a symlinked config is written through, not replaced; a file that receives `env`/`headers` is left `0600`; only one apply runs at a time; and its result carries server names only, never env values or headers, and never file text: a parse failure is reported by line and column, not by the parser's message (smol-toml prints a code frame of the offending lines and V8's JSON errors quote source, either of which can hold a secret). The schema restricts `path` and `relocation.path` to a relative path without `..`, since sync writes to it. @@ -350,6 +350,6 @@ The rules the module keeps and the tests pin: it only ADDS (a name already defin ## GitHub Copilot CLI -`copilot` (npm `@github/copilot`) is a stock entry. A bare launch is `copilot`, in its own Manual Approval mode. The launch parameters follow the grok/omp shape: `model` (`--model`), `resumeId` (`--resume `, an id or id prefix; names are refused by the id pattern) and `continueSession` (`--continue`, skipped when a resume id is set), plus `allowAll` (`--yolo`: allow every tool, path and URL). The legacy config object is `copilotConfig` (`{ model, allowAll, resumeSessionId, continueSession }`). The Run button sends `allowAll: true` like the other agent CLIs' bypass switches, and `allowAll` is declared in `privilegedParams`, so a non-granted multi-user owner has it forced back to `false` (an absent config is never materialized: a bare spawn is already safe). Session history resume goes through `copilotConfig.resumeSessionId` / `continueSession`. The tab names the model it reads from the footer (`modelDetect`). It declares no `mcpConfig`. +`copilot` (npm `@github/copilot`) is a stock entry. A bare launch is `copilot`, in its own Manual Approval mode. The launch parameters follow the grok/omp shape: `model` (`--model`), `resumeId` (`--resume `, an id, id prefix or session name: a single word that starts with a letter or digit, so a flag-shaped value such as `--yolo` is dropped rather than read as its own flag) and `continueSession` (`--continue`, skipped when a resume id is set), plus `allowAll` (`--yolo`: allow every tool, path and URL). The legacy config object is `copilotConfig` (`{ model, allowAll, resumeSessionId, continueSession }`). The Run button sends `allowAll: true` like the other agent CLIs' bypass switches, and `allowAll` is declared in `privilegedParams`, so a non-granted multi-user owner has it forced back to `false` (an absent config is never materialized: a bare spawn is already safe). Session history resume goes through `copilotConfig.resumeSessionId` / `continueSession`. The tab names the model it reads from the footer (`modelDetect`). It declares `mcpConfig` (`~/.copilot/mcp-config.json`, relocated by `COPILOT_HOME`) and no `credStore`: its token sits in `config.json` in plain text on a host without a keyring, so a Docker case is not seeded and you sign in inside the container. `env.allowedKeys` is empty on purpose: a headless host sets `COPILOT_GITHUB_TOKEN`, which the `COPILOT_` prefix admits, and `GH_TOKEN`/`GITHUB_TOKEN` stay out of the one global allowlist. Measured on 1.0.94 inside tmux: an alternate-screen TUI with mouse tracking on, whose composer is the `❯` between two rules. While a turn runs the footer reads `◉ Working esc edit prompt` (the dot alternates `◉`/`◎`), which is the `workDetect` working line. Keystrokes sent with `tmux send-keys` reach the composer, Shift+Enter inserts a newline, and a shell tool call completes inside tmux; github/copilot-cli#4180 (a driven PTY ignores input) and #4223 (a shell command never completes under tmux) report both failing on 1.0.70 to 1.0.74 and neither reproduces on 1.0.94. `COPILOT_HOME` and `COPILOT_ALLOW_ALL` are privileged env keys, so a non-granted multi-user owner cannot set them through `envOverrides`. diff --git a/docs/wiki/Docker-Cases.md b/docs/wiki/Docker-Cases.md index 30812cca..b119fe39 100644 --- a/docs/wiki/Docker-Cases.md +++ b/docs/wiki/Docker-Cases.md @@ -118,6 +118,10 @@ invisible from the host (`pi -c` and `grok -c` inside a docker case see only tha container's history). OMP's `sessions/` is the exception and is shared read-write, because Codeman reads it host-side for history and resume. +GitHub Copilot CLI is not seeded: on a host without a keyring its sign-in token sits in plain +text in `~/.copilot/config.json`, and there is no Docker credential row for it. Sign in inside +the container (`copilot login`), or set `COPILOT_GITHUB_TOKEN` for the session. + **Git hosts.** The agent image can also include the GitHub CLI (`gh`) and the Azure CLI (`az`, with the `azure-devops` extension), off by default, and its git then uses them as credential helpers for github.com and Azure DevOps. When the matching switch is on, their sign-ins are diff --git a/src/config/cli-registry/stock.ts b/src/config/cli-registry/stock.ts index 6122da56..9704ad55 100644 --- a/src/config/cli-registry/stock.ts +++ b/src/config/cli-registry/stock.ts @@ -1684,13 +1684,15 @@ const COPILOT: CliEntry = { // `--yolo` (allow every tool, path and URL) is opt-in through `allowAll`, which the Run button // sends like the other agent CLIs' bypass switches; an absent config spawns a bare `copilot` // in its own Manual Approval mode. `--model` and `--resume=` are Copilot's own flags - // (`copilot --help`, 1.0.94). `--resume` takes an id, id prefix or session NAME, so the id - // pattern below is what keeps a name or a path out of it. + // (`copilot --help`, 1.0.94). `--resume` takes an id, id prefix or session NAME and its value + // is optional, so the token must be a single plain word that cannot start with `-`: a value + // like `--yolo` would otherwise be parsed as its own flag and get around the `allowAll` clamp. launch: { params: { allowAll: { type: 'bool' }, model: { type: 'token', pattern: 'model' }, - resumeId: { type: 'token', pattern: 'id-dotted' }, + // `path-segment`, not `id-dotted`: the leading alphanumeric keeps a flag-shaped value out. + resumeId: { type: 'token', pattern: 'path-segment' }, continueSession: { type: 'bool' }, // The tab's name, so the CLI's own session list reads the same as the tab. sessionName: { type: 'engine', source: 'sessionName' }, @@ -1736,12 +1738,14 @@ const COPILOT: CliEntry = { env: { exports: [{ name: 'COLORTERM', value: 'truecolor' }], unset: ['NO_COLOR'], - // Sign-in is the CLI's own (`copilot login`, kept under ~/.copilot); a token in the - // environment (COPILOT_GITHUB_TOKEN, GH_TOKEN, GITHUB_TOKEN) is what a headless host uses. + // Sign-in is the CLI's own (`copilot login`, kept under ~/.copilot). A headless host sets + // COPILOT_GITHUB_TOKEN, which the COPILOT_ prefix already admits. GH_TOKEN / GITHUB_TOKEN are + // deliberately NOT allowlisted: the env allowlist is one global list with no mode context, so + // admitting them would make them settable on every session (see allowedEnvPrefixes()). tmuxSetenvKeys: [], dockerExecEnvNames: [], allowedPrefixes: ['COPILOT_'], - allowedKeys: ['GH_TOKEN', 'GITHUB_TOKEN'], + allowedKeys: [], }, capabilities: { ...agentDefaults(), @@ -1764,6 +1768,13 @@ const COPILOT: CliEntry = { screenLine: String.raw`(?:tab next tab|\? help|esc edit prompt|github-mcp-server) {3,}([A-Za-z0-9][\w.:/@ -]{1,60}?) *(?:\n|$)`, screenLines: 2, }, + // `copilot mcp add` writes `~/.copilot/mcp-config.json`; COPILOT_HOME replaces ~/.copilot + // (checked: `COPILOT_HOME= copilot mcp list` reads ). + mcpConfig: { + path: '.copilot/mcp-config.json', + format: 'copilot-json', + relocation: { envVar: 'COPILOT_HOME', path: 'mcp-config.json' }, + }, // COPILOT_HOME can restate permissions and COPILOT_ALLOW_ALL turns every one on; both // already match the COPILOT_ allowedPrefix, so a non-granted multi-user owner must not // be able to set them through envOverrides. @@ -1794,7 +1805,9 @@ const COPILOT: CliEntry = { }, }, overlays: { - credStore: { rel: '.copilot', seedFiles: ['config.json', 'mcp-config.json'] }, + // No credStore: Docker seeding is CRED_STORES in docker-hosts.ts, which has no `.copilot` row, + // and the sign-in token sits in plain text in config.json on a host without a keyring, so it + // is not copied into a container by default. Sign in inside the container instead. }, }; diff --git a/src/mcp-sync-targets.ts b/src/mcp-sync-targets.ts index a2292f6e..8fd0ed2e 100644 --- a/src/mcp-sync-targets.ts +++ b/src/mcp-sync-targets.ts @@ -4,9 +4,12 @@ * `mcpSyncTargets()` (routes/mcp-sync-routes.ts) takes the registry's enabled CLIs that declare an * `mcpConfig`. Some tools read an MCP server list worth keeping in step with the others but are not * something Codeman launches, so they have no registry entry (and no id to branch on): GitHub - * Copilot CLI is the first. They are plain data here, take part only when installed or when their - * config file already exists (an absent tool is reported `absent`, never created), and sort after - * the registry CLIs, so when two definitions of a name differ the registry CLI's is the one copied. + * Copilot CLI is the first. It is also a run mode that declares its own `mcpConfig`, so while that + * entry is enabled this row is skipped (`mcpSyncOnlyTargets` drops ids the registry already + * supplies); it applies when the run mode is disabled but the tool is still on the machine. They are + * plain data here, take part only when installed or when their config file already exists (an + * absent tool is reported `absent`, never created), and sort after the registry CLIs, so when two + * definitions of a name differ the registry CLI's is the one copied. * * @module mcp-sync-targets */ diff --git a/src/web/public/i18n.js b/src/web/public/i18n.js index 42657a23..9ec154ba 100644 --- a/src/web/public/i18n.js +++ b/src/web/public/i18n.js @@ -254,6 +254,7 @@ 'Run Grok': '运行 Grok', 'Run DeepSeek': '运行 DeepSeek', 'Run OMP': '运行 OMP', + 'Run GitHub Copilot': '运行 GitHub Copilot', 'Run Shell': '运行 Shell', 'More tools': '更多工具', 'Select AI backend': '选择 AI 后端', diff --git a/src/web/routes/mcp-sync-routes.ts b/src/web/routes/mcp-sync-routes.ts index f6812568..3b1ab7d8 100644 --- a/src/web/routes/mcp-sync-routes.ts +++ b/src/web/routes/mcp-sync-routes.ts @@ -10,8 +10,8 @@ * only, never env values, headers or file content (a parse failure is reported by position). * * A CLI takes part when it is ENABLED in the registry, declares an `mcpConfig`, and is installed - * or already has its config file (Copilot CLI, which is not a registry CLI, takes part when installed - * or when its config file exists); one that is enabled but absent from the machine is reported + * or already has its config file (Copilot CLI declares its own `mcpConfig`; with its run mode disabled it still + * takes part as a sync-only tool when installed or when its config file exists); one that is enabled but absent from the machine is reported * `absent` and never created. Its file is located with this process's env (the env the CLIs * Codeman spawns inherit), so a relocation var such as `CODEX_HOME` is followed. */ @@ -39,8 +39,8 @@ export async function readMcpSyncEnabled(): Promise { /** * Enabled CLIs that declare an MCP config file, in registry order (first definition wins), then the - * sync-only tools (src/mcp-sync-targets.ts: Copilot CLI), which come last so a registry CLI's - * definition wins a same-name difference. + * sync-only tools (src/mcp-sync-targets.ts: Copilot CLI while its run mode is disabled), which come + * last so a registry CLI's definition wins a same-name difference. */ export function mcpSyncTargets(availability: Record): McpSyncTarget[] { const registry = enabledClis() diff --git a/src/web/schemas.ts b/src/web/schemas.ts index 49e11033..f58ba5d7 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -448,12 +448,13 @@ const CopilotConfigSchema = z .optional(), allowAll: z.boolean().optional(), continueSession: z.boolean().optional(), - // Ids only: --resume also matches session NAMES (arbitrary user strings), which this - // regex deliberately cannot express. + // A single plain word with a leading alphanumeric: --resume also matches session NAMES, so + // `my-feature` is accepted, but a value like `--yolo` must not be, because --resume's value is + // optional and the next token would be read as its own flag (bypassing the allowAll clamp). resumeSessionId: z .string() .max(100) - .regex(/^[a-zA-Z0-9._-]+$/) + .regex(/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/) .optional(), }) .optional(); diff --git a/test/cli-registry-spawn-golden.test.ts b/test/cli-registry-spawn-golden.test.ts index d16c35e5..fd5f052d 100644 --- a/test/cli-registry-spawn-golden.test.ts +++ b/test/cli-registry-spawn-golden.test.ts @@ -361,6 +361,17 @@ describe('copilot', () => { expect(cp({ resumeSessionId: 'my session; rm -rf /' })).toBe('copilot'); }); + // `--resume`'s value is optional, so `--resume --yolo` would make `--yolo` its own flag and get + // around the allowAll clamp. A resume value must start with an alphanumeric. + it('drops a resume value that would be read as a flag', () => { + for (const resumeSessionId of ['--yolo', '-n', '--allow-all-tools', '.', '..', '.hidden']) { + expect(cp({ resumeSessionId })).toBe('copilot'); + } + expect(cp({ allowAll: false, resumeSessionId: '--yolo' })).toBe('copilot'); + // A plain word is still passed through (a session NAME also resumes). + expect(cp({ resumeSessionId: 'my-feature' })).toBe('copilot --resume my-feature'); + }); + it('never puts a credential on the command line', () => { const cmd = cp({ allowAll: true, model: 'auto' }) ?? ''; expect(cmd).not.toContain('token'); diff --git a/test/copilot-custom-endpoint.test.ts b/test/copilot-custom-endpoint.test.ts index 822b07db..dee7611c 100644 --- a/test/copilot-custom-endpoint.test.ts +++ b/test/copilot-custom-endpoint.test.ts @@ -23,3 +23,24 @@ describe('GitHub Copilot custom endpoint (BYOK)', () => { } }); }); + +describe('GitHub Copilot environment and sync declarations', () => { + it('admits only its own COPILOT_ namespace, never the shared GitHub token names', () => { + // The env allowlist is one global list with no mode context, so a foreign key here would be + // settable on every session. COPILOT_GITHUB_TOKEN is already reachable through the prefix. + expect(copilot.env.allowedPrefixes).toEqual(['COPILOT_']); + expect(copilot.env.allowedKeys).toEqual([]); + }); + + it('declares its own MCP config so Settings does not list it as unsupported', () => { + expect(copilot.capabilities.mcpConfig).toEqual({ + path: '.copilot/mcp-config.json', + format: 'copilot-json', + relocation: { envVar: 'COPILOT_HOME', path: 'mcp-config.json' }, + }); + }); + + it('seeds no credentials into a Docker case', () => { + expect(copilot.overlays?.credStore).toBeUndefined(); + }); +}); diff --git a/test/env-overrides-schema.test.ts b/test/env-overrides-schema.test.ts index ca92d3dd..a3bacc91 100644 --- a/test/env-overrides-schema.test.ts +++ b/test/env-overrides-schema.test.ts @@ -51,6 +51,17 @@ describe('envOverrides exact-key allowlist', () => { ).toThrow(); }); + it('rejects the shared GitHub token names (Copilot reads COPILOT_GITHUB_TOKEN, in its own prefix)', () => { + for (const key of ['GH_TOKEN', 'GITHUB_TOKEN']) { + for (const mode of ['claude', 'shell', 'codex', 'copilot'] as const) { + expect(() => CreateSessionSchema.parse({ workingDir: '/tmp', mode, envOverrides: { [key]: 'x' } })).toThrow(); + } + } + expect(() => + CreateSessionSchema.parse({ workingDir: '/tmp', mode: 'copilot', envOverrides: { COPILOT_GITHUB_TOKEN: 'x' } }) + ).not.toThrow(); + }); + it('still blocks security-sensitive keys', () => { for (const key of ['PATH', 'LD_PRELOAD', 'NODE_OPTIONS', 'CODEMAN_MUX_NAME']) { expect(() => diff --git a/test/mcp-sync-registry.test.ts b/test/mcp-sync-registry.test.ts index 10dddba1..0afee135 100644 --- a/test/mcp-sync-registry.test.ts +++ b/test/mcp-sync-registry.test.ts @@ -18,7 +18,7 @@ function withMcp(mcpConfig: unknown) { describe('capabilities.mcpConfig', () => { it('is declared by exactly the CLIs whose format is verified', () => { const declared = STOCK_CLIS.filter((e) => e.capabilities.mcpConfig).map((e) => e.id as string); - expect(declared.sort()).toEqual(['antigravity', 'claude', 'codex', 'gemini', 'opencode']); + expect(declared.sort()).toEqual(['antigravity', 'claude', 'codex', 'copilot', 'gemini', 'opencode']); }); it('every stock declaration passes the schema, with a distinct file per CLI', () => { @@ -41,6 +41,7 @@ describe('capabilities.mcpConfig', () => { claude: { envVar: 'CLAUDE_CONFIG_DIR', path: '.claude.json' }, opencode: { envVar: 'XDG_CONFIG_HOME', path: 'opencode/opencode.json' }, codex: { envVar: 'CODEX_HOME', path: 'config.toml' }, + copilot: { envVar: 'COPILOT_HOME', path: 'mcp-config.json' }, gemini: { envVar: 'GEMINI_CLI_HOME', path: '.gemini/settings.json' }, antigravity: undefined, });