fix(codex): harden dashboard config editing

src/shared/compatible-cli-contracts.ts

@@ -0,0 +1,214 @@
+export interface CompatibleCliDocLink {
+  id: string;
+  label: string;
+  url: string;
+  category: 'overview' | 'configuration' | 'byok' | 'reference';
+  source: 'factory' | 'provider' | 'openai' | 'github';
+  description: string;
+}
+
+export interface CompatibleCliProviderDocLink {
+  provider: string;
+  label: string;
+  apiFormat: string;
+  url: string;
+}
+
+export interface CompatibleCliDocsReference {
+  providerValues: string[];
+  settingsHierarchy: string[];
+  notes: string[];
+  links: CompatibleCliDocLink[];
+  providerDocs: CompatibleCliProviderDocLink[];
+}
+
+export type CodexBinarySource = 'CCS_CODEX_PATH' | 'PATH' | 'missing';
+
+export interface CodexBinaryDiagnostics {
+  installed: boolean;
+  path: string | null;
+  installDir: string | null;
+  source: CodexBinarySource;
+  version: string | null;
+  overridePath: string | null;
+  supportsConfigOverrides: boolean;
+}
+
+export interface CodexConfigFileDiagnostics {
+  label: string;
+  path: string;
+  resolvedPath: string;
+  exists: boolean;
+  isSymlink: boolean;
+  isRegularFile: boolean;
+  sizeBytes: number | null;
+  mtimeMs: number | null;
+  parseError: string | null;
+  readError: string | null;
+}
+
+export interface CodexModelProviderDiagnostics {
+  name: string;
+  baseUrl: string | null;
+  envKey: string | null;
+  wireApi: string | null;
+  requiresOpenaiAuth: boolean;
+  supportsWebsockets: boolean;
+  hasQueryParams: boolean;
+  hasHttpHeaders: boolean;
+  usesExperimentalBearerToken: boolean;
+}
+
+export interface CodexFeatureFlagDiagnostics {
+  name: string;
+  state: 'enabled' | 'disabled' | 'custom';
+}
+
+export interface CodexProjectTrustDiagnostics {
+  path: string;
+  trustLevel: string;
+}
+
+export interface CodexMcpServerDiagnostics {
+  name: string;
+  transport: 'stdio' | 'streamable-http' | 'unknown';
+  enabled: boolean;
+  required: boolean;
+  startupTimeoutSec: number | null;
+  toolTimeoutSec: number | null;
+  enabledToolsCount: number;
+  disabledToolsCount: number;
+  usesInlineBearerToken: boolean;
+}
+
+export interface CodexSupportMatrixEntry {
+  id: string;
+  label: string;
+  supported: boolean;
+  notes: string;
+}
+
+export interface CodexUserConfigDiagnostics {
+  model: string | null;
+  modelReasoningEffort: string | null;
+  modelProvider: string | null;
+  activeProfile: string | null;
+  approvalPolicy: string | null;
+  sandboxMode: string | null;
+  webSearch: string | null;
+  toolOutputTokenLimit: number | null;
+  personality: string | null;
+  topLevelKeys: string[];
+  profileCount: number;
+  profileNames: string[];
+  modelProviderCount: number;
+  modelProviders: CodexModelProviderDiagnostics[];
+  featureCount: number;
+  enabledFeatures: CodexFeatureFlagDiagnostics[];
+  disabledFeatures: CodexFeatureFlagDiagnostics[];
+  trustedProjectCount: number;
+  untrustedProjectCount: number;
+  projectTrust: CodexProjectTrustDiagnostics[];
+  mcpServerCount: number;
+  mcpServers: CodexMcpServerDiagnostics[];
+}
+
+export interface CodexDashboardDiagnostics {
+  binary: CodexBinaryDiagnostics;
+  file: CodexConfigFileDiagnostics;
+  workspacePath: string;
+  config: CodexUserConfigDiagnostics;
+  supportMatrix: CodexSupportMatrixEntry[];
+  warnings: string[];
+  docsReference: CompatibleCliDocsReference;
+}
+
+export interface CodexRawConfigResponse {
+  path: string;
+  resolvedPath: string;
+  exists: boolean;
+  mtime: number;
+  rawText: string;
+  config: Record<string, unknown> | null;
+  parseError: string | null;
+}
+
+export interface CodexTopLevelSettingsPatch {
+  model?: string | null;
+  modelReasoningEffort?: string | null;
+  modelProvider?: string | null;
+  approvalPolicy?: string | null;
+  sandboxMode?: string | null;
+  webSearch?: string | null;
+  toolOutputTokenLimit?: number | null;
+  personality?: string | null;
+}
+
+export interface CodexProfilePatchValues extends CodexTopLevelSettingsPatch {}
+
+export interface CodexModelProviderPatchValues {
+  displayName?: string | null;
+  baseUrl?: string | null;
+  envKey?: string | null;
+  wireApi?: string | null;
+  requiresOpenaiAuth?: boolean | null;
+  supportsWebsockets?: boolean | null;
+}
+
+export interface CodexMcpServerPatchValues {
+  transport: 'stdio' | 'streamable-http';
+  command?: string | null;
+  args?: string[] | null;
+  url?: string | null;
+  enabled?: boolean | null;
+  required?: boolean | null;
+  startupTimeoutSec?: number | null;
+  toolTimeoutSec?: number | null;
+  enabledTools?: string[] | null;
+  disabledTools?: string[] | null;
+}
+
+export type CodexConfigPatchInput =
+  | {
+      kind: 'top-level';
+      expectedMtime?: number;
+      values: CodexTopLevelSettingsPatch;
+    }
+  | {
+      kind: 'project-trust';
+      expectedMtime?: number;
+      path: string;
+      trustLevel: string | null;
+    }
+  | {
+      kind: 'feature';
+      expectedMtime?: number;
+      feature: string;
+      enabled: boolean | null;
+    }
+  | {
+      kind: 'profile';
+      expectedMtime?: number;
+      action: 'set-active' | 'upsert' | 'delete';
+      name: string;
+      values?: CodexProfilePatchValues;
+      setAsActive?: boolean;
+    }
+  | {
+      kind: 'model-provider';
+      expectedMtime?: number;
+      action: 'upsert' | 'delete';
+      name: string;
+      values?: CodexModelProviderPatchValues;
+    }
+  | {
+      kind: 'mcp-server';
+      expectedMtime?: number;
+      action: 'upsert' | 'delete';
+      name: string;
+      values?: CodexMcpServerPatchValues;
+    };
+
+export interface CodexConfigPatchResult extends CodexRawConfigResponse {
+  success: true;
+}

src/shared/toml-object.ts

@@ -0,0 +1,36 @@
+import { parse } from 'smol-toml';
+
+export interface SafeTomlObjectParseResult {
+  config: Record<string, unknown> | null;
+  parseError: string | null;
+}
+
+function isTomlObject(value: unknown): value is Record<string, unknown> {
+  return typeof value === 'object' && value !== null && !Array.isArray(value);
+}
+
+export function parseTomlObject(rawText: string): Record<string, unknown> {
+  const trimmed = rawText.trim();
+  if (!trimmed) return {};
+
+  const parsed = parse(rawText);
+  if (!isTomlObject(parsed)) {
+    throw new Error('TOML root must be a table.');
+  }
+
+  return parsed;
+}
+
+export function safeParseTomlObject(rawText: string): SafeTomlObjectParseResult {
+  try {
+    return {
+      config: parseTomlObject(rawText),
+      parseError: null,
+    };
+  } catch (error) {
+    return {
+      config: null,
+      parseError: (error as Error).message,
+    };
+  }
+}

src/web-server/services/codex-dashboard-service.ts

@@ -134,8 +134,7 @@ function setEnumStringField(
   }
 
   const normalized = value.trim();
-  const currentValue = asString(target[key]);
-  if (!allowedValues.has(normalized) && normalized !== currentValue) {
+  if (!allowedValues.has(normalized)) {
     throw new TomlFileValidationError(
       `${label} must be one of: ${Array.from(allowedValues).join(', ')}.`
     );

src/web-server/services/compatible-cli-toml-file-service.ts

@@ -1,6 +1,7 @@
 import { promises as fs } from 'fs';
 import * as path from 'path';
-import { parse, stringify } from 'smol-toml';
+import { stringify } from 'smol-toml';
+import { parseTomlObject } from '../../shared/toml-object';
 
 export interface TomlFileDiagnostics {
   label: string;
@@ -67,6 +68,111 @@ async function statPath(filePath: string): Promise<import('fs').Stats | null> {
   }
 }
 
+async function resolveConflictMtime(filePath: string): Promise<number> {
+  const stat = await statPath(filePath);
+  return stat?.mtimeMs ?? Date.now();
+}
+
+async function acquireWriteLock(
+  lockPath: string,
+  targetPath: string,
+  fileLabel: string
+): Promise<() => Promise<void>> {
+  let handle: Awaited<ReturnType<typeof fs.open>> | null = null;
+  try {
+    handle = await fs.open(lockPath, 'wx', 0o600);
+  } catch (error) {
+    const code = (error as NodeJS.ErrnoException).code;
+    if (code === 'EEXIST') {
+      const existingLock = await statPath(lockPath);
+      if (existingLock?.isSymbolicLink()) {
+        throw new Error(`Refusing to write: ${fileLabel}.lock is a symlink.`);
+      }
+      if (existingLock && !existingLock.isFile()) {
+        throw new Error(`Refusing to write: ${fileLabel}.lock is not a regular file.`);
+      }
+      throw new TomlFileConflictError(
+        'File is currently being written by another request. Refresh and retry.',
+        await resolveConflictMtime(targetPath)
+      );
+    }
+    throw error;
+  }
+
+  return async () => {
+    if (!handle) return;
+    try {
+      await handle.close();
+    } finally {
+      try {
+        await fs.unlink(lockPath);
+      } catch (error) {
+        if ((error as NodeJS.ErrnoException).code !== 'ENOENT') {
+          throw error;
+        }
+      }
+    }
+  };
+}
+
+function ensureWritableTarget(
+  targetStat: import('fs').Stats | null,
+  fileLabel: string
+): import('fs').Stats | null {
+  if (!targetStat) return null;
+  if (targetStat.isSymbolicLink()) {
+    throw new Error(`Refusing to write: ${fileLabel} is a symlink.`);
+  }
+  if (!targetStat.isFile()) {
+    throw new Error(`Refusing to write: ${fileLabel} is not a regular file.`);
+  }
+  return targetStat;
+}
+
+function assertExpectedMtime(
+  targetStat: import('fs').Stats | null,
+  expectedMtime: number | undefined
+): void {
+  if (!targetStat) {
+    if (expectedMtime !== undefined) {
+      throw new TomlFileConflictError('File modified externally.', Date.now());
+    }
+    return;
+  }
+
+  if (typeof expectedMtime !== 'number' || !Number.isFinite(expectedMtime)) {
+    throw new TomlFileConflictError(
+      'File metadata not loaded. Refresh and retry.',
+      targetStat.mtimeMs
+    );
+  }
+  if (targetStat.mtimeMs !== expectedMtime) {
+    throw new TomlFileConflictError('File modified externally.', targetStat.mtimeMs);
+  }
+}
+
+async function verifyTargetUnchanged(
+  targetPath: string,
+  initialTargetStat: import('fs').Stats | null,
+  fileLabel: string
+): Promise<void> {
+  const currentTargetStat = ensureWritableTarget(await statPath(targetPath), fileLabel);
+
+  if (!initialTargetStat) {
+    if (currentTargetStat) {
+      throw new TomlFileConflictError('File modified externally.', currentTargetStat.mtimeMs);
+    }
+    return;
+  }
+
+  if (!currentTargetStat || currentTargetStat.mtimeMs !== initialTargetStat.mtimeMs) {
+    throw new TomlFileConflictError(
+      'File modified externally.',
+      currentTargetStat?.mtimeMs ?? Date.now()
+    );
+  }
+}
+
 export function parseTomlObjectText(
   rawText: string,
   fieldName = 'rawText'
@@ -75,21 +181,15 @@ export function parseTomlObjectText(
     throw new TomlFileValidationError(`${fieldName} must be a string.`);
   }
 
-  const trimmed = rawText.trim();
-  if (!trimmed) return {};
-
-  let parsed: unknown;
   try {
-    parsed = parse(rawText);
+    return parseTomlObject(rawText);
   } catch (error) {
-    throw new TomlFileValidationError(`Invalid TOML in ${fieldName}: ${(error as Error).message}`);
+    const message = (error as Error).message;
+    if (message === 'TOML root must be a table.') {
+      throw new TomlFileValidationError(`${fieldName} TOML root must be a table.`);
+    }
+    throw new TomlFileValidationError(`Invalid TOML in ${fieldName}: ${message}`);
   }
-
-  if (!isObject(parsed)) {
-    throw new TomlFileValidationError(`${fieldName} TOML root must be a table.`);
-  }
-
-  return parsed;
 }
 
 export function stringifyTomlObject(config: Record<string, unknown>): string {
@@ -170,45 +270,21 @@ export async function writeTomlFileAtomic(input: WriteTomlFileInput): Promise<Wr
 
   const targetPath = input.filePath;
   const targetDir = path.dirname(targetPath);
-  const tempPath = targetPath + '.tmp';
+  const tempPath = `${targetPath}.${process.pid}.${Date.now()}.tmp`;
+  const lockPath = `${targetPath}.lock`;
   const dirMode = input.dirMode ?? 0o700;
   const fileMode = input.fileMode ?? 0o600;
 
   await fs.mkdir(targetDir, { recursive: true, mode: dirMode });
 
-  const targetStat = await statPath(targetPath);
-  if (targetStat) {
-    if (targetStat.isSymbolicLink()) {
-      throw new Error(`Refusing to write: ${fileLabel} is a symlink.`);
-    }
-    if (!targetStat.isFile()) {
-      throw new Error(`Refusing to write: ${fileLabel} is not a regular file.`);
-    }
-
-    if (typeof input.expectedMtime !== 'number' || !Number.isFinite(input.expectedMtime)) {
-      throw new TomlFileConflictError(
-        'File metadata not loaded. Refresh and retry.',
-        targetStat.mtimeMs
-      );
-    }
-    if (Math.abs(targetStat.mtimeMs - input.expectedMtime) > 1000) {
-      throw new TomlFileConflictError('File modified externally.', targetStat.mtimeMs);
-    }
-  }
+  const releaseLock = await acquireWriteLock(lockPath, targetPath, fileLabel);
 
   let wroteTemp = false;
   try {
-    const existingTempStat = await statPath(tempPath);
-    if (existingTempStat) {
-      if (existingTempStat.isSymbolicLink()) {
-        throw new Error(`Refusing to write: ${fileLabel}.tmp is a symlink.`);
-      }
-      if (!existingTempStat.isFile()) {
-        throw new Error(`Refusing to write: ${fileLabel}.tmp is not a regular file.`);
-      }
-    }
+    const targetStat = ensureWritableTarget(await statPath(targetPath), fileLabel);
+    assertExpectedMtime(targetStat, input.expectedMtime);
 
-    await fs.writeFile(tempPath, input.rawText, { mode: fileMode });
+    await fs.writeFile(tempPath, input.rawText, { mode: fileMode, flag: 'wx' });
     wroteTemp = true;
 
     const tempStat = await fs.lstat(tempPath);
@@ -219,6 +295,7 @@ export async function writeTomlFileAtomic(input: WriteTomlFileInput): Promise<Wr
       throw new Error(`Refusing to write: ${fileLabel}.tmp is not a regular file.`);
     }
 
+    await verifyTargetUnchanged(targetPath, targetStat, fileLabel);
     await fs.rename(tempPath, targetPath);
     wroteTemp = false;
 
@@ -240,5 +317,7 @@ export async function writeTomlFileAtomic(input: WriteTomlFileInput): Promise<Wr
         }
       }
     }
+
+    await releaseLock();
   }
 }

src/web-server/services/compatible-cli-types.ts

@@ -1,3 +1,28 @@
+import type { CompatibleCliDocsReference } from '../../shared/compatible-cli-contracts';
+
+export type {
+  CompatibleCliDocLink,
+  CompatibleCliDocsReference,
+  CompatibleCliProviderDocLink,
+  CodexBinaryDiagnostics,
+  CodexBinarySource,
+  CodexConfigFileDiagnostics,
+  CodexConfigPatchInput,
+  CodexConfigPatchResult,
+  CodexDashboardDiagnostics,
+  CodexFeatureFlagDiagnostics,
+  CodexMcpServerDiagnostics,
+  CodexMcpServerPatchValues,
+  CodexModelProviderDiagnostics,
+  CodexModelProviderPatchValues,
+  CodexProfilePatchValues,
+  CodexProjectTrustDiagnostics,
+  CodexRawConfigResponse,
+  CodexSupportMatrixEntry,
+  CodexTopLevelSettingsPatch,
+  CodexUserConfigDiagnostics,
+} from '../../shared/compatible-cli-contracts';
+
 export type DroidBinarySource = 'CCS_DROID_PATH' | 'PATH' | 'missing';
 
 export interface DroidBinaryDiagnostics {
@@ -44,30 +69,6 @@ export interface DroidByokDiagnostics {
   customModels: DroidCustomModelDiagnostics[];
 }
 
-export interface CompatibleCliDocLink {
-  id: string;
-  label: string;
-  url: string;
-  category: 'overview' | 'configuration' | 'byok' | 'reference';
-  source: 'factory' | 'provider' | 'openai' | 'github';
-  description: string;
-}
-
-export interface CompatibleCliProviderDocLink {
-  provider: string;
-  label: string;
-  apiFormat: string;
-  url: string;
-}
-
-export interface CompatibleCliDocsReference {
-  providerValues: string[];
-  settingsHierarchy: string[];
-  notes: string[];
-  links: CompatibleCliDocLink[];
-  providerDocs: CompatibleCliProviderDocLink[];
-}
-
 export interface DroidDashboardDiagnostics {
   binary: DroidBinaryDiagnostics;
   files: {
@@ -88,183 +89,3 @@ export interface DroidRawSettingsResponse {
   settings: Record<string, unknown> | null;
   parseError: string | null;
 }
-
-export type CodexBinarySource = 'CCS_CODEX_PATH' | 'PATH' | 'missing';
-
-export interface CodexBinaryDiagnostics {
-  installed: boolean;
-  path: string | null;
-  installDir: string | null;
-  source: CodexBinarySource;
-  version: string | null;
-  overridePath: string | null;
-  supportsConfigOverrides: boolean;
-}
-
-export type CodexConfigFileDiagnostics = DroidConfigFileDiagnostics;
-
-export interface CodexModelProviderDiagnostics {
-  name: string;
-  baseUrl: string | null;
-  envKey: string | null;
-  wireApi: string | null;
-  requiresOpenaiAuth: boolean;
-  supportsWebsockets: boolean;
-  hasQueryParams: boolean;
-  hasHttpHeaders: boolean;
-  usesExperimentalBearerToken: boolean;
-}
-
-export interface CodexFeatureFlagDiagnostics {
-  name: string;
-  state: 'enabled' | 'disabled' | 'custom';
-}
-
-export interface CodexProjectTrustDiagnostics {
-  path: string;
-  trustLevel: string;
-}
-
-export interface CodexMcpServerDiagnostics {
-  name: string;
-  transport: 'stdio' | 'streamable-http' | 'unknown';
-  enabled: boolean;
-  required: boolean;
-  startupTimeoutSec: number | null;
-  toolTimeoutSec: number | null;
-  enabledToolsCount: number;
-  disabledToolsCount: number;
-  usesInlineBearerToken: boolean;
-}
-
-export interface CodexSupportMatrixEntry {
-  id: string;
-  label: string;
-  supported: boolean;
-  notes: string;
-}
-
-export interface CodexUserConfigDiagnostics {
-  model: string | null;
-  modelReasoningEffort: string | null;
-  modelProvider: string | null;
-  activeProfile: string | null;
-  approvalPolicy: string | null;
-  sandboxMode: string | null;
-  webSearch: string | null;
-  toolOutputTokenLimit: number | null;
-  personality: string | null;
-  topLevelKeys: string[];
-  profileCount: number;
-  profileNames: string[];
-  modelProviderCount: number;
-  modelProviders: CodexModelProviderDiagnostics[];
-  featureCount: number;
-  enabledFeatures: CodexFeatureFlagDiagnostics[];
-  disabledFeatures: CodexFeatureFlagDiagnostics[];
-  trustedProjectCount: number;
-  untrustedProjectCount: number;
-  projectTrust: CodexProjectTrustDiagnostics[];
-  mcpServerCount: number;
-  mcpServers: CodexMcpServerDiagnostics[];
-}
-
-export interface CodexDashboardDiagnostics {
-  binary: CodexBinaryDiagnostics;
-  file: CodexConfigFileDiagnostics;
-  workspacePath: string;
-  config: CodexUserConfigDiagnostics;
-  supportMatrix: CodexSupportMatrixEntry[];
-  warnings: string[];
-  docsReference: CompatibleCliDocsReference;
-}
-
-export interface CodexRawConfigResponse {
-  path: string;
-  resolvedPath: string;
-  exists: boolean;
-  mtime: number;
-  rawText: string;
-  config: Record<string, unknown> | null;
-  parseError: string | null;
-}
-
-export interface CodexTopLevelSettingsPatch {
-  model?: string | null;
-  modelReasoningEffort?: string | null;
-  modelProvider?: string | null;
-  approvalPolicy?: string | null;
-  sandboxMode?: string | null;
-  webSearch?: string | null;
-  toolOutputTokenLimit?: number | null;
-  personality?: string | null;
-}
-
-export interface CodexProfilePatchValues extends CodexTopLevelSettingsPatch {}
-
-export interface CodexModelProviderPatchValues {
-  displayName?: string | null;
-  baseUrl?: string | null;
-  envKey?: string | null;
-  wireApi?: string | null;
-  requiresOpenaiAuth?: boolean | null;
-  supportsWebsockets?: boolean | null;
-}
-
-export interface CodexMcpServerPatchValues {
-  transport: 'stdio' | 'streamable-http';
-  command?: string | null;
-  args?: string[] | null;
-  url?: string | null;
-  enabled?: boolean | null;
-  required?: boolean | null;
-  startupTimeoutSec?: number | null;
-  toolTimeoutSec?: number | null;
-  enabledTools?: string[] | null;
-  disabledTools?: string[] | null;
-}
-
-export type CodexConfigPatchInput =
-  | {
-      kind: 'top-level';
-      expectedMtime?: number;
-      values: CodexTopLevelSettingsPatch;
-    }
-  | {
-      kind: 'project-trust';
-      expectedMtime?: number;
-      path: string;
-      trustLevel: string | null;
-    }
-  | {
-      kind: 'feature';
-      expectedMtime?: number;
-      feature: string;
-      enabled: boolean | null;
-    }
-  | {
-      kind: 'profile';
-      expectedMtime?: number;
-      action: 'set-active' | 'upsert' | 'delete';
-      name: string;
-      values?: CodexProfilePatchValues;
-      setAsActive?: boolean;
-    }
-  | {
-      kind: 'model-provider';
-      expectedMtime?: number;
-      action: 'upsert' | 'delete';
-      name: string;
-      values?: CodexModelProviderPatchValues;
-    }
-  | {
-      kind: 'mcp-server';
-      expectedMtime?: number;
-      action: 'upsert' | 'delete';
-      name: string;
-      values?: CodexMcpServerPatchValues;
-    };
-
-export interface CodexConfigPatchResult extends CodexRawConfigResponse {
-  success: true;
-}