fix(#592): clamp the COPILOT_* variables that bypass approval, run code or move the token

The COPILOT_ prefix admits every variable Copilot reads, and the 1.0.98
bundle reads far more than `copilot help environment` lists. Only
COPILOT_HOME, COPILOT_ALLOW_ALL and the BYOK trio were privileged, so a
non-granted multi-user owner could still hand back what the allowAll
clamp removed:

- approval: COPILOT_PLAN_THEN_AUTOPILOT, COPILOT_ASSISTED_APPROVAL
- code with no approval row: COPILOT_CLI_DIST_DIR runs <dir>/index.js
  in place of the CLI, the package caches choose the CLI build,
  COPILOT_EDITOR, COPILOT_PROVIDER_API_KEY_COMMAND and
  COPILOT_AHP_HOST_BIN are executed, skills, provider configs and hooks
  are loaded from the named places (the DSH_HOME class)
- token redirects: COPILOT_API_URL and the other API bases receive the
  CLI's sign-in token, which in multi-user mode is the operator's (all
  users share the OS account and ~/.copilot), and the rest of the BYOK
  provider family re-aims a Codeman custom endpoint (the
  DEEPSEEK_BASE_URL / OMP_AUTH_BROKER_URL class)

All are now privilegedEnvKeys. The registry schema capped that list at 8;
the cap is now 48, since a longer deny list only takes more away from
non-granted owners. The test sends one of each class through the real
schema (they all pass it) and the real clamp. The docs say the list is
read from the 1.0.98 bundle and that a variable a later release adds is
not covered until it is listed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer committed 2026-10-18 08:04:41 +02:00
1 parent ebfa4a01b3
commit 75186a5d97
6 files changed
+99 -9

No files matched your search

+1 -1
View File
@@ -172,7 +172,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph
**CLI registry** (`src/config/cli-registry/`): every run mode is a `CliEntry` (discovery, launch argv template, env handling, `capabilities`, and the `overlays` behind remote/docker pane commands). **No code outside `stock.ts` may branch on a CLI id**: use a capability field or a NAMED PROFILE (`profiles.ts`); `test/cli-registry-no-id-branching.test.ts` and `test/frontend-cli-no-id-branching.test.ts` enforce it. ⚠️ Config holds typed argv tokens, never shell text; literals are validated at LOAD time and a bad one rejects the whole entry. ⚠️ Keep `external`, `hooks` and `altScreen` independent; never derive one from another. ⚠️ Config regexes (`discovery.version.regex`, `capabilities.workDetect.workingLine`, `capabilities.workDetect.watchingLine`, `capabilities.workDetect.awaitingLine`, `capabilities.workDetect.blockedLine`, `capabilities.modelDetect.screenLine`) must compile through `compileVersionRegex()`. ⚠️ `privilegedParams[].param` names a LAUNCH PARAM, not the legacy `<Mode>Config` field (bridged only by `launch.legacyConfigAliases`); a wrong name silently clamps nothing. ⚠️ Resolve the registry AT CALL TIME, never in a module-level const. ⚠️ Remote claude/omp arms of `buildRemoteLaunchCommand` are not covered by the pane-command golden. `~/.codeman/clis.json` overrides entries. It is WRITTEN only by the opt-in CLI management routes (`cliManagementEnabled`, default OFF; `/api/clis`, `cli-registry-routes.ts`), and only through `mutateRegistryFile()` in `registry-writer.ts`, which serializes mutations and refuses (409) a file that does not parse or has group/world permission bits rather than overwriting it. Importing the registry still writes nothing. → [architecture-invariants#cli-registry](docs/architecture-invariants.md#cli-registry), `docs/cli-registry.md`
**External CLI modes (OpenCode, Codex, Gemini, Antigravity, Pi, Grok, DeepSeek, OMP, Copilot)**: `isExternalCliMode()` in `session.ts` gates Claude-specific behavior off (Ralph tracker, BashToolParser, token parsing, ❯ readiness; readiness is output stabilization); work detection is per-CLI `capabilities.workDetect` data, not this gate. All nine **require tmux, no direct PTY fallback** (secrets go via socket-scoped `tmux setenv`, never the command line). ⚠️ `run*()` in `session-ui.js` MUST unwrap the `{success,data}` envelope. ⚠️ **Codex uses predictive write-through echo, never the buffer overlay**: `_predictHookOnData` must never `return` (wire path stays byte-identical), and flushed text and a bracketed paste must go out as separate delayed writes. ⚠️ **Pi**: no bypass flag, never invent one; `approveProjectTrust` executes repo code, so it is in the clamp's **materialize** branch; never wire `--api-key`. ⚠️ **Grok**: `alwaysApprove` is stripped for non-granted owners (only-if-sent). ⚠️ **DeepSeek**: the agent is a PROFILE (Run gates on `isDeepSeekRunnable()`); the permission switch is the `DSH_PERMISSION_MODE` env var, so `clampEnvOverridesForOwner()` must DROP `DSH_PERMISSION_MODE`, `DSH_HOME` and `DEEPSEEK_BASE_URL` for non-granted owners; `hooksAvailableForMode()` is per-SESSION for it (pass `sessionHookOptions(session)`) and is never a stand-in for `mode === 'claude'`; answers come from `deepseek-transcript.ts`, paired by header `cwd` + boot window, never newest-mtime. ⚠️ **OMP**: `OMP_AUTH_BROKER_URL`/`_TOKEN` are clamped the same way. ⚠️ **Copilot**: `allowAll` (`--yolo`) is a `privilegedParam` and `COPILOT_HOME`/`COPILOT_ALLOW_ALL`/`COPILOT_PROVIDER_*`/`COPILOT_MODEL` are privileged env keys; the resume id must start with a letter or digit (`path-segment`), since `--resume`'s value is optional and `--resume --yolo` would otherwise make `--yolo` its own flag; `--name` is dropped next to `--resume`/`--continue` (Copilot refuses the pair); its mouse DECSETs are stripped (`strip-mux-and-mouse`) so a drag selects and the wheel is forwarded by `_shouldForwardWheelToApp` while `cliMouseTracking` is on; `composerReadyMark` is the footer hint `/ commands · ? help`, NOT `❯` (the folder-trust dialog draws `❯ 1. Yes`); `last-response` reads the pane's visible screen (`transcriptNeedsScreenCapture`) through a per-CLI `PANE_DIALECTS` row, because its stream is cursor moves, not lines (a prompt row scrolled off by a tall answer reads as the answer's tail unless a banner/trust dialog is on screen, and typed composer text is not a prompt); Docker seeds no Copilot credential (`config.json` holds a plain-text token without a keyring). → [architecture-invariants#external-cli-modes-opencode-codex-gemini-antigravity-pi-grok-deepseek-omp](docs/architecture-invariants.md#external-cli-modes-opencode-codex-gemini-antigravity-pi-grok-deepseek-omp)
**External CLI modes (OpenCode, Codex, Gemini, Antigravity, Pi, Grok, DeepSeek, OMP, Copilot)**: `isExternalCliMode()` in `session.ts` gates Claude-specific behavior off (Ralph tracker, BashToolParser, token parsing, ❯ readiness; readiness is output stabilization); work detection is per-CLI `capabilities.workDetect` data, not this gate. All nine **require tmux, no direct PTY fallback** (secrets go via socket-scoped `tmux setenv`, never the command line). ⚠️ `run*()` in `session-ui.js` MUST unwrap the `{success,data}` envelope. ⚠️ **Codex uses predictive write-through echo, never the buffer overlay**: `_predictHookOnData` must never `return` (wire path stays byte-identical), and flushed text and a bracketed paste must go out as separate delayed writes. ⚠️ **Pi**: no bypass flag, never invent one; `approveProjectTrust` executes repo code, so it is in the clamp's **materialize** branch; never wire `--api-key`. ⚠️ **Grok**: `alwaysApprove` is stripped for non-granted owners (only-if-sent). ⚠️ **DeepSeek**: the agent is a PROFILE (Run gates on `isDeepSeekRunnable()`); the permission switch is the `DSH_PERMISSION_MODE` env var, so `clampEnvOverridesForOwner()` must DROP `DSH_PERMISSION_MODE`, `DSH_HOME` and `DEEPSEEK_BASE_URL` for non-granted owners; `hooksAvailableForMode()` is per-SESSION for it (pass `sessionHookOptions(session)`) and is never a stand-in for `mode === 'claude'`; answers come from `deepseek-transcript.ts`, paired by header `cwd` + boot window, never newest-mtime. ⚠️ **OMP**: `OMP_AUTH_BROKER_URL`/`_TOKEN` are clamped the same way. ⚠️ **Copilot**: `allowAll` (`--yolo`) is a `privilegedParam` and `COPILOT_HOME`/`COPILOT_ALLOW_ALL`/`COPILOT_PROVIDER_*`/`COPILOT_MODEL` are privileged env keys, and so is every other `COPILOT_*` variable the CLI reads that switches approvals, runs code with no approval row or sends its sign-in token elsewhere (the 1.0.98 list is in `stock.ts`; the prefix admits whatever a later release adds, so re-read the bundle whenever the measured version moves); the resume id must start with a letter or digit (`path-segment`), since `--resume`'s value is optional and `--resume --yolo` would otherwise make `--yolo` its own flag; `--name` is dropped next to `--resume`/`--continue` (Copilot refuses the pair); its mouse DECSETs are stripped (`strip-mux-and-mouse`) so a drag selects and the wheel is forwarded by `_shouldForwardWheelToApp` while `cliMouseTracking` is on; `composerReadyMark` is the footer hint `/ commands · ? help`, NOT `❯` (the folder-trust dialog draws `❯ 1. Yes`); `last-response` reads the pane's visible screen (`transcriptNeedsScreenCapture`) through a per-CLI `PANE_DIALECTS` row, because its stream is cursor moves, not lines (a prompt row scrolled off by a tall answer reads as the answer's tail unless a banner/trust dialog is on screen, and typed composer text is not a prompt); Docker seeds no Copilot credential (`config.json` holds a plain-text token without a keyring). → [architecture-invariants#external-cli-modes-opencode-codex-gemini-antigravity-pi-grok-deepseek-omp](docs/architecture-invariants.md#external-cli-modes-opencode-codex-gemini-antigravity-pi-grok-deepseek-omp)
**DeepSeek web UI** (`POST`/`GET`/`DELETE /api/deepseek/web`, `deepseek-web-server.ts`): the Run menu's "DeepSeek web UI..." entry supervises ONE background `dsh web` child process, deliberately **NOT a shell session**. ⚠️ Every piece is load-bearing: exactly one server (a second click REUSES it), restarted when the browser authority changes (`--trusted-host`, last asker wins), killed on shutdown via `stopDeepSeekWeb()` (the detached child would otherwise outlive Codeman and hold its port), and failures returned to the caller. ⚠️ Never hardcode the port: search from 3080 across 40, detect free ports by BINDING, then wait for the server to really answer. ⚠️ Both `POST` and `DELETE` must stay behind `canUsernameRunPrivilegedCommands` (booting a profile runs its plugin code; the server is shared). → [architecture-invariants#deepseek-web-ui](docs/architecture-invariants.md#deepseek-web-ui)
+1 -1
View File
@@ -352,4 +352,4 @@ The rules the module keeps and the tests pin: it only ADDS (a name already defin
`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 <id>`, 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`). Its `altScreen` is `strip-mux-and-mouse`, like opencode: Copilot turns mouse tracking on itself, and left in, a plain drag is reported to the TUI instead of selecting (Auto Copy and copy-on-select did nothing); stripped, a drag is a local selection, clicks go through the hand-encoded tap, and the wheel is forwarded as SGR reports while `cliMouseTracking` is on. 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`.
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`, and so are the other `COPILOT_*` variables (read from the 1.0.98 bundle, far more than `copilot help environment` lists) that switch approvals, run code with no approval row or send the sign-in token elsewhere. The `COPILOT_` prefix admits whatever a later release adds, so the list needs a look whenever the measured version moves.
+1 -1
View File
@@ -48,7 +48,7 @@ Codeman never logs Copilot in or stores its credentials.
| --- | --- |
| Launch | `copilot`, in Copilot's own Manual Approval mode. |
| Run button | `copilot --yolo` (allow every tool, path and URL), like the other CLIs' bypass switches, so an unattended session never stops on an approval. |
| Permission clamp | `allowAll` is a `privilegedParam`. A multi-user owner without the bypass grant has it forced back to `false`; an absent config is never materialized (a bare `copilot` is already safe). `COPILOT_HOME`, `COPILOT_ALLOW_ALL`, `COPILOT_PROVIDER_*` and `COPILOT_MODEL` are privileged env keys. |
| Permission clamp | `allowAll` is a `privilegedParam`. A multi-user owner without the bypass grant has it forced back to `false`; an absent config is never materialized (a bare `copilot` is already safe). `COPILOT_HOME`, `COPILOT_ALLOW_ALL`, `COPILOT_PROVIDER_*` and `COPILOT_MODEL` are privileged env keys, and so is every other `COPILOT_*` variable the CLI reads that switches approvals (`COPILOT_PLAN_THEN_AUTOPILOT`), runs code with no approval row (`COPILOT_CLI_DIST_DIR`, the package caches, `COPILOT_EDITOR`, `COPILOT_PROVIDER_API_KEY_COMMAND`, skills, hooks) or sends the sign-in token to another host (`COPILOT_API_URL`, `COPILOT_GH_HOST`, ...): the list is in `stock.ts`, read from the 1.0.98 bundle. A variable a later Copilot release adds is not covered until it is listed there. |
| Model | `--model <id>`, sent only by an API caller that sets `copilotConfig.model` (the value must start with a letter or digit, like `resumeSessionId`). Nothing in the UI sets it: a saved custom endpoint sets `COPILOT_MODEL` instead (see below). |
| Resume | `--resume <value>`, sent only by an API caller that sets `copilotConfig.resumeSessionId` (Past Sessions does not). The value (an id, an id prefix or a session name) must be one plain word starting with a letter or digit: `--resume`'s value is optional, so `--resume --yolo` would otherwise make `--yolo` its own flag and bypass the clamp. |
| Continue | `--continue`, from **Past Sessions**: it resumes the most recent conversation Copilot has for that directory. Nothing sets it on a respawn. Skipped when a resume value is set. |
+4 -2
View File
@@ -464,8 +464,10 @@ const capabilitiesSchema = z
)
.max(8),
// Exact env var NAMES, not prefixes: this list is a targeted deny, and a prefix here
// would let one entry silently strip a whole namespace off every owner's overrides.
privilegedEnvKeys: z.array(envName).max(8),
// would let one entry silently strip a whole namespace off every owner's overrides. The
// bound is generous because a longer list only takes more away from non-granted owners:
// copilot's COPILOT_ prefix covers dozens of variables that run code or redirect its token.
privilegedEnvKeys: z.array(envName).max(48),
gates: z.record(z.string(), z.object({ minVersion: z.string().max(20), failClosed: z.boolean() }).strict()),
maxFrameBytes: z.number().int().positive().optional(),
newline: z.enum(['line-feed', 'esc-enter']).optional(),
+43 -3
View File
@@ -1795,14 +1795,54 @@ const COPILOT: CliEntry = {
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.
// Every one of these matches the COPILOT_ allowedPrefix, so a non-granted multi-user owner
// must not be able to set it through envOverrides. Read from the 1.0.98 bundle (copilot reads
// far more COPILOT_* variables than `copilot help environment` lists), by what each hands back:
// - approval: COPILOT_ALLOW_ALL turns every tool on; COPILOT_PLAN_THEN_AUTOPILOT switches a
// plan into autopilot; COPILOT_ASSISTED_APPROVAL; COPILOT_HOME restates the permission config.
// - code at boot or without an approval row: COPILOT_CLI_DIST_DIR runs `<dir>/index.js` in place
// of the CLI, the package caches pick which CLI build runs, COPILOT_EDITOR,
// COPILOT_PROVIDER_API_KEY_COMMAND and COPILOT_AHP_HOST_BIN are run as commands, skills and
// provider configs are loaded from the named paths, and the hook switches widen what hooks run.
// - credential redirects: the Copilot and GitHub API bases receive the CLI's sign-in token, which
// in multi-user mode is the operator's (every user shares the OS account and ~/.copilot), and
// the BYOK provider family aims a Codeman custom endpoint elsewhere.
// The prefix stays because COPILOT_GITHUB_TOKEN and the harmless knobs live in it; a variable a
// later release adds is NOT covered until it is listed here.
privilegedEnvKeys: [
'COPILOT_HOME',
'COPILOT_ALLOW_ALL',
'COPILOT_PLAN_THEN_AUTOPILOT',
'COPILOT_ASSISTED_APPROVAL',
'COPILOT_CLI_DIST_DIR',
'COPILOT_CLI_RESOLVED_DIST_DIR',
'COPILOT_PKG_CACHE_HOME',
'COPILOT_CACHE_HOME',
'COPILOT_EDITOR',
'COPILOT_AHP_HOST_BIN',
'COPILOT_DEBUG_VOICE_ENGINE_SCRIPT',
'COPILOT_SKILLS_DIRS',
'COPILOT_PROVIDERS_CONFIG',
'COPILOT_HOOK_ALLOW_HTTP_AUTH_HOOKS',
'COPILOT_HOOK_ALLOW_LOCALHOST',
'COPILOT_E2E_TEST_HOOKS',
'COPILOT_API_URL',
'COPILOT_DEBUG_GITHUB_API_URL',
'COPILOT_GH_HOST',
'COPILOT_MC_BASE_URL',
'COPILOT_AHP_URL',
'COPILOT_DEBUG_ENTRA_BROKER_URL',
'COPILOT_CAFE_METRICS_URL',
'COPILOT_PROVIDER_BASE_URL',
'COPILOT_PROVIDER_API_KEY',
'COPILOT_PROVIDER_API_KEY_COMMAND',
'COPILOT_PROVIDER_BEARER_TOKEN',
'COPILOT_PROVIDER_HEADERS',
'COPILOT_PROVIDER_TYPE',
'COPILOT_PROVIDER_WIRE_MODEL',
'COPILOT_PROVIDER_MODEL_ID',
'COPILOT_PROVIDER_GHES_HOST',
'COPILOT_PROVIDER_GHES_TOKEN',
'COPILOT_MODEL',
],
// Grok/codex-shaped clamp: a bare spawn is already Manual Approval, so the multi-user clamp
+49 -1
View File
@@ -1,5 +1,7 @@
import { describe, it, expect } from 'vitest';
import { afterEach, beforeEach, describe, it, expect } from 'vitest';
import { STOCK_CLIS } from '../src/config/cli-registry/stock.js';
import { _clampEnvOverridesForOwner } from '../src/web/routes/session-routes.js';
import { CreateSessionSchema } from '../src/web/schemas.js';
const copilot = STOCK_CLIS.find((c) => (c.id as string) === 'copilot')!;
@@ -44,3 +46,49 @@ describe('GitHub Copilot environment and sync declarations', () => {
expect(copilot.overlays?.credStore).toBeUndefined();
});
});
describe('GitHub Copilot multi-user clamp: the env-var half', () => {
// COPILOT_ is an allowlisted prefix, so every variable below would otherwise pass the schema.
// Each one hands back what the allowAll clamp removed: an approval switch, code that runs with no
// approval row (a CLI build, an editor, a key command, skills, hooks), or the sign-in token sent to
// a host of the caller's choosing.
const DANGEROUS: Record<string, string> = {
COPILOT_ALLOW_ALL: 'true',
COPILOT_PLAN_THEN_AUTOPILOT: '1',
COPILOT_HOME: '/tmp/planted-copilot-home',
COPILOT_CLI_DIST_DIR: '/tmp/planted-dist',
COPILOT_PKG_CACHE_HOME: '/tmp/planted-cache',
COPILOT_CACHE_HOME: '/tmp/planted-cache',
COPILOT_EDITOR: 'sh -c planted',
COPILOT_PROVIDER_API_KEY_COMMAND: 'sh -c planted',
COPILOT_SKILLS_DIRS: '/tmp/planted-skills',
COPILOT_PROVIDERS_CONFIG: '/tmp/planted-providers.json',
COPILOT_API_URL: 'https://attacker.example',
COPILOT_GH_HOST: 'attacker.example',
COPILOT_PROVIDER_BASE_URL: 'https://attacker.example/v1',
COPILOT_PROVIDER_HEADERS: 'X-Planted: 1',
};
const ORIGINAL = process.env.CODEMAN_MULTIUSER;
beforeEach(() => {
process.env.CODEMAN_MULTIUSER = '1';
});
afterEach(() => {
if (ORIGINAL === undefined) delete process.env.CODEMAN_MULTIUSER;
else process.env.CODEMAN_MULTIUSER = ORIGINAL;
});
it('passes the env allowlist, so only the clamp stands between these and the pane', () => {
expect(() =>
CreateSessionSchema.parse({ workingDir: '/tmp', mode: 'copilot', envOverrides: DANGEROUS })
).not.toThrow();
});
it("strips every one of them from a non-granted owner's overrides and keeps the harmless knobs", async () => {
const out = await _clampEnvOverridesForOwner('nobody', {
...DANGEROUS,
COPILOT_GITHUB_TOKEN: 'own-token',
COPILOT_AUTO_UPDATE: 'false',
});
expect(out).toEqual({ COPILOT_GITHUB_TOKEN: 'own-token', COPILOT_AUTO_UPDATE: 'false' });
});
});