fix(codex): #546 landing review follow-up

Pin the App Settings codexModel guard to the schema. The 28df21f4 landing fix
added a client check in saveAppSettings() that copies the pattern of
SettingsUpdateSchema.codexModel, so one bad character no longer 400s the whole
.strict() settings PUT behind a "Settings saved" toast. Nothing tied the copy
to the schema: a looser copy would bring the silent 400 back, and a stricter
one would refuse valid model ids.

The new test extracts the client pattern from the saveAppSettings() body,
checks it agrees with the schema on eight samples (empty, dotted, slashed,
colon, space, semicolon, leading dash, non-ASCII), and asserts the guard runs
before the localStorage write. Length is left out on purpose, since the
input's maxlength="100" covers .max(100). Both a loosened pattern and a guard
moved after the write turn the test red.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-10-09 06:11:18 +02:00
parent f79f530f93
commit 432bd5fc0a
@@ -1,6 +1,7 @@
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { readFileSync } from 'node:fs';
import { mkdir, writeFile, rm } from 'node:fs/promises';
import { dirname, join } from 'node:path';
import { dirname, join, resolve } from 'node:path';
import { homedir } from 'node:os';
import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js';
import { registerSessionRoutes } from '../../src/web/routes/session-routes.js';
@@ -11,6 +12,7 @@ import { buildCodexCommand } from '../../src/tmux-manager.js';
import { Session } from '../../src/session.js';
import { safeRmHomeTree } from '../mocks/index.js';
import { getDataDir } from '../../src/config/instance.js';
import { SettingsUpdateSchema } from '../../src/web/schemas.js';
vi.mock('../../src/utils/cli-launcher.js', async (importOriginal) => {
const actual = await importOriginal<typeof import('../../src/utils/cli-launcher.js')>();
@@ -153,4 +155,26 @@ describe('Codex launch defaults', () => {
await system.app.close();
}
});
it('the App Settings client check mirrors SettingsUpdateSchema.codexModel and runs before the local write', () => {
// SettingsUpdateSchema is .strict(), so a codexModel the schema refuses 400s the
// WHOLE settings PUT while the toast still says "Settings saved". saveAppSettings()
// refuses it client-side with a copy of the schema's pattern; a looser copy brings
// that silent 400 back, a stricter one refuses valid model ids. Length is left
// out on purpose: the input's maxlength="100" covers .max(100).
const src = readFileSync(resolve(import.meta.dirname, '../../src/web/public/settings-ui.js'), 'utf8');
const start = src.indexOf('async saveAppSettings() {');
expect(start).toBeGreaterThan(-1);
const body = src.slice(start);
const m = body.match(/if \(!\/(\^\[[^\]\n]+\]\*\$)\/\.test\(settings\.codexModel\)\)/);
expect(m).not.toBeNull();
const client = new RegExp(m![1]);
for (const v of ['', 'gpt-5.1', 'org/model_1-x', 'gpt-oss:20b', 'a b', 'bad;cmd', '-x', 'é']) {
expect(client.test(v), v).toBe(SettingsUpdateSchema.safeParse({ codexModel: v }).success);
}
const guardAt = body.indexOf(m![0]);
const writeAt = body.indexOf('this.saveAppSettingsToStorage(settings);');
expect(writeAt).toBeGreaterThan(-1);
expect(guardAt).toBeLessThan(writeAt);
});
});