mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-20 09:09:35 +02:00
fix(copilot): a model value must start with a letter or digit
Like resumeSessionId, model follows a flag, so --model --yolo made --yolo its own flag and bypassed the allowAll clamp. New model-plain token pattern, the same rule in the create schema, and golden and schema tests.
This commit is contained in:
1 parent
68af012845
commit
6cca5465a9
5 files changed
+45
-2
No files matched your search
@@ -20,6 +20,7 @@
|
||||
export type TokenPattern =
|
||||
| 'model'
|
||||
| 'model-claude'
|
||||
| 'model-plain'
|
||||
| 'model-pi'
|
||||
| 'id'
|
||||
| 'id-dotted'
|
||||
@@ -43,6 +44,9 @@ const PATTERNS: Record<TokenPattern, RegExp> = {
|
||||
model: /^[a-zA-Z0-9._\-/]+$/,
|
||||
// buildSpawnCommand's claude branch — `[` and `]` for bracketed model aliases
|
||||
'model-claude': /^[a-zA-Z0-9._\-[\]]+$/,
|
||||
// copilot --model. Same characters as `model`, but a leading alphanumeric: the value follows a
|
||||
// flag, so `--yolo` must not pass as a model id and become its own flag (the allowAll clamp bypass).
|
||||
'model-plain': /^[a-zA-Z0-9][a-zA-Z0-9._\-/]*$/,
|
||||
// buildPiCommand — `:` for a thinking suffix (`sonnet:high`), `/` for `provider/id`
|
||||
'model-pi': /^[a-zA-Z0-9._\-/:]+$/,
|
||||
// opencode --session, codex resume
|
||||
|
||||
@@ -1691,7 +1691,8 @@ const COPILOT: CliEntry = {
|
||||
launch: {
|
||||
params: {
|
||||
allowAll: { type: 'bool' },
|
||||
model: { type: 'token', pattern: 'model' },
|
||||
// `model-plain`, not `model`: the leading alphanumeric keeps a flag-shaped value out.
|
||||
model: { type: 'token', pattern: 'model-plain' },
|
||||
// `path-segment`, not `id-dotted`: the leading alphanumeric keeps a flag-shaped value out.
|
||||
resumeId: { type: 'token', pattern: 'path-segment' },
|
||||
continueSession: { type: 'bool' },
|
||||
|
||||
+3
-1
@@ -441,10 +441,12 @@ const GrokConfigSchema = z
|
||||
|
||||
const CopilotConfigSchema = z
|
||||
.object({
|
||||
// A leading alphanumeric, like resumeSessionId below: a value like `--yolo` would otherwise be read
|
||||
// as its own flag after `--model` (bypassing the allowAll clamp). `/` stays for provider/id.
|
||||
model: z
|
||||
.string()
|
||||
.max(100)
|
||||
.regex(/^[a-zA-Z0-9._\-/]+$/)
|
||||
.regex(/^[a-zA-Z0-9][a-zA-Z0-9._\-/]*$/)
|
||||
.optional(),
|
||||
allowAll: z.boolean().optional(),
|
||||
continueSession: z.boolean().optional(),
|
||||
|
||||
@@ -372,6 +372,17 @@ describe('copilot', () => {
|
||||
expect(cp({ resumeSessionId: 'my-feature' })).toBe('copilot --resume my-feature');
|
||||
});
|
||||
|
||||
// `--model --yolo` would make `--yolo` its own flag and get around the allowAll clamp, so a model id
|
||||
// starts with an alphanumeric too. A provider/id value (the `/`) is still accepted.
|
||||
it('drops a model value that would be read as a flag', () => {
|
||||
for (const model of ['--yolo', '-n', '--allow-all-tools', '.', '/etc/passwd']) {
|
||||
expect(cp({ model })).toBe('copilot');
|
||||
expect(cp({ allowAll: false, model })).toBe('copilot');
|
||||
}
|
||||
expect(cp({ model: 'claude-sonnet-5.5' })).toBe('copilot --model claude-sonnet-5.5');
|
||||
expect(cp({ model: 'openai/gpt-5' })).toBe('copilot --model openai/gpt-5');
|
||||
});
|
||||
|
||||
it('never puts a credential on the command line', () => {
|
||||
const cmd = cp({ allowAll: true, model: 'auto' }) ?? '';
|
||||
expect(cmd).not.toContain('token');
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
/**
|
||||
* @fileoverview copilotConfig on the session create body: `model` and `resumeSessionId` follow a flag
|
||||
* (`--model`, `--resume`), so a value that starts with `-` would be read as its own flag and get
|
||||
* around the allowAll clamp. Both need a leading letter or digit.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { CreateSessionSchema } from '../src/web/schemas.js';
|
||||
|
||||
const parse = (copilotConfig: unknown) =>
|
||||
CreateSessionSchema.safeParse({ workingDir: '/tmp', mode: 'copilot', copilotConfig });
|
||||
|
||||
describe('copilotConfig schema', () => {
|
||||
it.each(['--yolo', '-n', '--allow-all-tools', '.hidden', '/etc/passwd'])('rejects model %s', (model) => {
|
||||
expect(parse({ model }).success).toBe(false);
|
||||
});
|
||||
|
||||
it.each(['--yolo', '-n', '--allow-all-tools'])('rejects resumeSessionId %s', (resumeSessionId) => {
|
||||
expect(parse({ resumeSessionId }).success).toBe(false);
|
||||
});
|
||||
|
||||
it.each(['auto', 'claude-sonnet-5.5', 'openai/gpt-5'])('accepts model %s', (model) => {
|
||||
expect(parse({ model }).success).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user