Fail open on invalid local settings with in-app diagnostics

src/config.test.ts

@@ -94,6 +94,29 @@ describe("loadConfig", () => {
     }
   });
 
+  test("threads local settings diagnostics on unconfigured early return", async () => {
+    const cwd = await emptyCwd();
+    try {
+      await mkdir(join(cwd, ".corbits"), { recursive: true });
+      await writeFile(
+        join(cwd, ".corbits", "settings.json"),
+        JSON.stringify({ unknownKey: true, anotherJunk: 1 }),
+      );
+      const result = await loadConfig(["--cwd", cwd, "do it"], {
+        globalSettingsPath: NO_SETTINGS,
+        allowUnconfigured: true,
+      });
+      expect(result.configured).toBe(false);
+      if (result.configured === false) {
+        expect(result.settingsDiagnostics).toBeDefined();
+        expect(result.settingsDiagnostics!.length).toBeGreaterThan(0);
+        expect(result.settingsDiagnostics!.some((d) => /unknown/i.test(d.message))).toBe(true);
+      }
+    } finally {
+      await rm(cwd, { recursive: true, force: true });
+    }
+  });
+
   test("UnconfiguredConfig.globalSettingsPath reflects --config path, not the global default", async () => {
     const cwd = await emptyCwd();
     try {

src/config/index.ts

@@ -34,6 +34,8 @@ import { xaiUserIdFromAccessToken } from "../auth/xai/session.js";
 import {
   globalSettingsPath,
   loadLocalSettings,
+  loadLocalSettingsResult,
+  type SettingsLoadDiagnostic,
   loadSettings,
   localSettingsPath,
   normalizeOpenAICompatibleBaseURL,
@@ -245,6 +247,11 @@ export type Config = {
    * providerCatalogToSettings (or re-read disk) before any persist.
    */
   settings?: Settings;
+  /**
+   * Fail-open diagnostics from local settings load (unknown keys, invalid JSON,
+   * stripped credentials). Shown on the main TUI so startup never hard-crashes.
+   */
+  settingsDiagnostics?: SettingsLoadDiagnostic[];
 };
 
 // Returned by loadConfig when no provider is configured and allowUnconfigured is
@@ -262,6 +269,12 @@ export type UnconfiguredConfig = {
   globalSettingsPath: string;
   // The original error message, used for non-TUI (exec) error output.
   providerError: string;
+  /**
+   * Fail-open diagnostics from local settings load. Still threaded on the
+   * unconfigured path so junk local files surface via stderr/banner rather
+   * than disappearing when provider setup fails early.
+   */
+  settingsDiagnostics?: SettingsLoadDiagnostic[];
 };
 
 export type LoadConfigOptions = {
@@ -403,7 +416,10 @@ export async function loadConfig(
   // The per-repo selection file still applies on top of a --config source: that
   // file supplies provider definitions, while .corbits/settings.json supplies
   // the provider/model selection. CLI --provider/--model override both.
-  const local = await loadLocalSettings(localSettingsPath(cwd));
+  // Fail open on unknown/invalid local keys — never crash startup.
+  const localResult = await loadLocalSettingsResult(localSettingsPath(cwd));
+  const local = localResult.settings;
+  const settingsDiagnostics = localResult.diagnostics;
 
   const profile = await resolveProfile(cwd, profileFlag);
 
@@ -448,6 +464,9 @@ export async function loadConfig(
       command,
       globalSettingsPath: effectiveSettingsPath,
       providerError: err instanceof Error ? err.message : String(err),
+      // Keep diagnostics even when provider setup fails early so junk local
+      // files still reach stderr (exec) / banner (TUI after onboarding).
+      ...(settingsDiagnostics.length > 0 ? { settingsDiagnostics } : {}),
     };
   }
 
@@ -499,6 +518,7 @@ export async function loadConfig(
     // Codex/xAI providers that are never written to settings.json. Not safe
     // to persist as-is — use providerCatalogToSettings or re-read disk.
     ...(settingsForResolution !== null ? { settings: settingsForResolution } : {}),
+    ...(settingsDiagnostics.length > 0 ? { settingsDiagnostics } : {}),
   };
 }
 

src/config/settings.ts

@@ -6,7 +6,7 @@ import { dirname, join } from "node:path";
 import { type } from "arktype";
 
 import { SETTINGS_DIR_NAME } from "../branding.js";
-import { REASONING_EFFORTS, type ReasoningEffort } from "../provider/reasoning-effort.js";
+import { REASONING_EFFORTS, isReasoningEffort, type ReasoningEffort } from "../provider/reasoning-effort.js";
 import { isSessionMode, type SessionMode } from "./session-mode.js";
 
 // A configured inference provider. `apiKey` is secret and lives only in the
@@ -650,36 +650,177 @@ export async function loadSettings(path: string): Promise<Settings | null> {
   };
 }
 
-export async function loadLocalSettings(path: string): Promise<LocalSettings | null> {
+/** Diagnostic produced when settings fail open instead of crashing startup. */
+export type SettingsLoadDiagnostic = {
+  path: string;
+  message: string;
+  /** Actionable recommendation for the user. */
+  fix: string;
+};
+
+export type LocalSettingsLoadResult = {
+  settings: LocalSettings | null;
+  diagnostics: SettingsLoadDiagnostic[];
+};
+
+const LOCAL_ALLOWED_KEYS = new Set<string>(LOCAL_SETTINGS_OPTIONAL_KEYS);
+const LOCAL_CREDENTIAL_KEYS = new Set([
+  "apiKey",
+  "api_key",
+  "token",
+  "secret",
+  "password",
+  "authorization",
+]);
+
+/** Pick known local-settings fields from a raw object (strict or fail-open). */
+function pickLocalFields(
+  s: Record<string, unknown>,
+  mode: "strict" | "coerce",
+): OptionalLocalSettingsFields {
+  if (mode === "strict") {
+    return {
+      provider: s.provider as string | undefined,
+      model: s.model as string | undefined,
+      reasoningEffort: s.reasoningEffort as ReasoningEffort | undefined,
+      mcpServers: s.mcpServers !== undefined ? normalizeMcpServers(s.mcpServers) : undefined,
+      sessionMode:
+        s.sessionMode === "single" || s.sessionMode === "orchestrator" ? s.sessionMode : undefined,
+      env: s.env as Record<string, string> | undefined,
+    };
+  }
+  return {
+    provider: typeof s.provider === "string" ? s.provider : undefined,
+    model: typeof s.model === "string" ? s.model : undefined,
+    reasoningEffort: isReasoningEffort(s.reasoningEffort) ? s.reasoningEffort : undefined,
+    mcpServers: s.mcpServers !== undefined ? normalizeMcpServers(s.mcpServers) : undefined,
+    sessionMode:
+      s.sessionMode === "single" || s.sessionMode === "orchestrator" ? s.sessionMode : undefined,
+    env:
+      s.env !== undefined && typeof s.env === "object" && s.env !== null && !Array.isArray(s.env)
+        ? Object.fromEntries(
+            Object.entries(s.env as Record<string, unknown>).filter(
+              (e): e is [string, string] => typeof e[1] === "string",
+            ),
+          )
+        : undefined,
+  };
+}
+
+function coerceLocalSettings(path: string, parsed: unknown): LocalSettingsLoadResult {
+  const diagnostics: SettingsLoadDiagnostic[] = [];
+  if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) {
+    return {
+      settings: null,
+      diagnostics: [
+        {
+          path,
+          message: `Local settings in ${path} is not a JSON object.`,
+          fix: `Edit ${path} to a JSON object with only: provider, model, reasoningEffort, mcpServers, sessionMode, env.`,
+        },
+      ],
+    };
+  }
+  const s = parsed as Record<string, unknown>;
+  // Valid strict path still returns cleanly with no diagnostics.
+  if (isLocalSettings(parsed)) {
+    return { settings: pickDefined(pickLocalFields(s, "strict")), diagnostics: [] };
+  }
+
+  const unknownKeys = Object.keys(s).filter((k) => !LOCAL_ALLOWED_KEYS.has(k));
+  const credentialKeys = unknownKeys.filter(
+    (k) => LOCAL_CREDENTIAL_KEYS.has(k) || /key|token|secret|password/i.test(k),
+  );
+  const otherUnknown = unknownKeys.filter((k) => !credentialKeys.includes(k));
+  if (credentialKeys.length > 0) {
+    diagnostics.push({
+      path,
+      message: `Ignored credential field(s) in local settings (${credentialKeys.join(", ")}).`,
+      fix: "Keep credentials out of local .corbits/settings.json — store API keys via provider settings / keychain, not local selection files.",
+    });
+  }
+  if (otherUnknown.length > 0) {
+    diagnostics.push({
+      path,
+      message: `Ignored unknown local settings key(s): ${otherUnknown.join(", ")}.`,
+      fix: `Remove unknown keys from ${path}. Allowed keys: ${[...LOCAL_ALLOWED_KEYS].join(", ")}.`,
+    });
+  }
+
+  const optional = pickLocalFields(s, "coerce");
+  if (s.mcpServers !== undefined && optional.mcpServers === undefined) {
+    diagnostics.push({
+      path,
+      message: `mcpServers in ${path} was invalid and was ignored.`,
+      fix: "Use an object map of MCP server entries (command/args or url).",
+    });
+  }
+  if (s.reasoningEffort !== undefined && optional.reasoningEffort === undefined) {
+    diagnostics.push({
+      path,
+      message: `reasoningEffort in ${path} was invalid and was ignored.`,
+      fix: `Use one of: ${REASONING_EFFORTS.join(", ")}.`,
+    });
+  }
+  if (diagnostics.length === 0) {
+    // Shape failed isLocalSettings for another reason (e.g. wrong types).
+    diagnostics.push({
+      path,
+      message: `Local settings in ${path} had invalid values and were partially ignored.`,
+      fix: `Edit ${path}: only "provider", "model", "reasoningEffort", "mcpServers", "sessionMode", and "env" are allowed (no credentials).`,
+    });
+  }
+  const settings = pickDefined(optional);
+  return { settings: Object.keys(settings).length > 0 ? settings : null, diagnostics };
+}
+
+export async function loadLocalSettingsResult(path: string): Promise<LocalSettingsLoadResult> {
   let raw: string;
   try {
     raw = await readFile(path, "utf8");
   } catch (err) {
-    if (isENOENT(err)) return null;
+    if (isENOENT(err)) return { settings: null, diagnostics: [] };
     throw err;
   }
   let parsed: unknown;
   try {
     parsed = JSON.parse(raw);
   } catch {
-    throw new Error(`Invalid JSON in local settings file: ${path}`);
+    return {
+      settings: null,
+      diagnostics: [
+        {
+          path,
+          message: `Invalid JSON in local settings file: ${path}`,
+          fix: `Fix JSON syntax in ${path}, or delete the file to fall back to global settings only.`,
+        },
+      ],
+    };
   }
-  if (!isLocalSettings(parsed)) {
-    throw new Error(
-      `Invalid local settings in ${path}: only "provider", "model", "reasoningEffort", "mcpServers", "sessionMode", and "env" are allowed (no credentials).`,
-    );
+  return coerceLocalSettings(path, parsed);
+}
+
+export async function loadLocalSettings(path: string): Promise<LocalSettings | null> {
+  // Fail open: never throw for schema/unknown-key problems. Callers that need
+  // diagnostics should use loadLocalSettingsResult.
+  const { settings } = await loadLocalSettingsResult(path);
+  return settings;
+}
+
+// Resolve the base for a read-modify-write of the local selection file.
+// Absent file → empty base (create OK). Partial fail-open → cleaned fields.
+// Invalid JSON / unreadable / fully unusable → null so the caller skips the
+// write instead of collapsing to {} and wiping the file.
+export async function loadLocalSettingsWriteBase(path: string): Promise<LocalSettings | null> {
+  try {
+    const result = await loadLocalSettingsResult(path);
+    if (result.settings !== null) return result.settings;
+    // Absent (ENOENT) returns null settings with empty diagnostics.
+    if (result.diagnostics.length === 0) return {};
+    return null;
+  } catch {
+    return null;
   }
-  const s = parsed as Record<string, unknown>;
-  const optional: OptionalLocalSettingsFields = {
-    provider: s.provider as string | undefined,
-    model: s.model as string | undefined,
-    reasoningEffort: s.reasoningEffort as ReasoningEffort | undefined,
-    mcpServers: s.mcpServers !== undefined ? normalizeMcpServers(s.mcpServers) : undefined,
-    sessionMode:
-      s.sessionMode === "single" || s.sessionMode === "orchestrator" ? s.sessionMode : undefined,
-    env: s.env as Record<string, string> | undefined,
-  };
-  return pickDefined(optional);
 }
 
 // Resolve the base for a read-modify-write of the global settings file.

src/index.ts

@@ -20,6 +20,16 @@ export async function mainWithRunners(
   runners: Runners,
 ): Promise<number> {
   const config = await loadConfig(argv, { allowUnconfigured: true });
+  // Exec has no Ink banner; unconfigured TUI goes to onboarding without the
+  // main-screen notice. Surface fail-open diagnostics on stderr for those
+  // paths so junk local files are never silent.
+  const surfaceDiagnosticsOnStderr =
+    config.command === "exec" || !config.configured;
+  if (surfaceDiagnosticsOnStderr && config.settingsDiagnostics !== undefined) {
+    for (const d of config.settingsDiagnostics) {
+      process.stderr.write(`settings: ${d.message}\n  fix: ${d.fix}\n`);
+    }
+  }
   // Always the TRUE global settings file, never config.globalSettingsPath —
   // that's the --config override file when one was given, and splitting
   // telemetry across two files means the installationId lands somewhere the