Fail closed instead of rewriting clobbered settings

Author: TheGreatAxios

src/config/index.ts

@@ -695,16 +695,16 @@ export async function loadConfig(
   };
   const settings =
     configPath !== undefined
-      ? await loadSettingsRecoveringClobberedOAuthSelection(
-          configPath,
-          projectedOAuthProviders,
-        ).then((s) => {
+      ? await loadSettingsRecoveringClobberedOAuthSelection(configPath, projectedOAuthProviders, {
+          persist: false,
+        }).then((s) => {
           if (s === null) throw new Error(`--config file not found or empty: ${configPath}`);
           return s;
         })
       : await loadSettingsRecoveringClobberedOAuthSelection(
           effectiveSettingsPath,
           projectedOAuthProviders,
+          { persist: true },
         );
 
   // Track whether the effective value came from the persisted global default

src/config/settings.ts

@@ -886,14 +886,14 @@ export async function loadSettings(path: string): Promise<Settings | null> {
 export async function loadSettingsRecoveringClobberedOAuthSelection(
   path: string,
   recoverableOAuthProviders: Record<string, ProviderSettings>,
+  options: { persist: boolean },
 ): Promise<Settings | null> {
   const parsed = await loadSettingsJSON(path);
   if (parsed === null) return null;
   if (isClobberedLocalSelection(parsed)) {
-    const recovered = recoverClobberedOAuthSelection(parsed, recoverableOAuthProviders) ?? {
-      providers: {},
-    };
-    await saveGlobalSettings(path, recovered);
+    const recovered = recoverClobberedOAuthSelection(parsed, recoverableOAuthProviders);
+    if (recovered === undefined) throw settingsSchemaError(path);
+    if (options.persist) await saveGlobalSettings(path, recovered);
     return recovered;
   }
   return loadStrictSettings(path, parsed);

src/settings.test.ts

@@ -628,7 +628,9 @@ describe("loaders", () => {
           ]),
         );
 
-        const recovered = await loadSettingsRecoveringClobberedOAuthSelection(path, projected);
+        const recovered = await loadSettingsRecoveringClobberedOAuthSelection(path, projected, {
+          persist: true,
+        });
         expect(recovered).toEqual({
           defaultProvider: "codex/work",
           providers: {
@@ -655,14 +657,18 @@ describe("loaders", () => {
         path,
         JSON.stringify({ provider: "codex/work", model: "gpt-special-custom" }),
       );
-      const recovered = await loadSettingsRecoveringClobberedOAuthSelection(path, {
-        "codex/work": {
-          baseURL: "https://chatgpt.com/backend-api",
-          apiKey: "oauth-token",
-          models: ["gpt-5.2-codex", "gpt-5.1-codex"],
-          defaultModel: "gpt-5.2-codex",
+      const recovered = await loadSettingsRecoveringClobberedOAuthSelection(
+        path,
+        {
+          "codex/work": {
+            baseURL: "https://chatgpt.com/backend-api",
+            apiKey: "oauth-token",
+            models: ["gpt-5.2-codex", "gpt-5.1-codex"],
+            defaultModel: "gpt-5.2-codex",
+          },
         },
-      });
+        { persist: true },
+      );
       expect(recovered).toEqual({
         defaultProvider: "codex/work",
         providers: {
@@ -689,32 +695,67 @@ describe("loaders", () => {
         JSON.stringify({ provider: "codex/work", model: "gpt-5.1-codex", apiKey: "nope" }),
       );
       await expect(
-        loadSettingsRecoveringClobberedOAuthSelection(path, {
-          "codex/work": {
-            baseURL: "https://chatgpt.com/backend-api",
-            apiKey: "oauth-token",
-            models: ["gpt-5.1-codex"],
+        loadSettingsRecoveringClobberedOAuthSelection(
+          path,
+          {
+            "codex/work": {
+              baseURL: "https://chatgpt.com/backend-api",
+              apiKey: "oauth-token",
+              models: ["gpt-5.1-codex"],
+            },
           },
-        }),
+          { persist: true },
+        ),
+      ).rejects.toThrow(/Invalid settings schema/);
+    } finally {
+      await rm(dir, { recursive: true, force: true });
+    }
+  });
+
+  test("loadSettings fails closed on unmatched OAuth selections without touching the file", async () => {
+    const dir = await mkdtemp(join(tmpdir(), "ic-settings-"));
+    try {
+      const path = join(dir, "settings.json");
+      const original = JSON.stringify({ provider: "codex/missing", model: "gpt-5.1-codex" });
+      await writeFile(path, original);
+      await expect(
+        loadSettingsRecoveringClobberedOAuthSelection(
+          path,
+          {
+            "codex/work": {
+              baseURL: "https://chatgpt.com/backend-api",
+              apiKey: "oauth-token",
+              models: ["gpt-5.1-codex"],
+            },
+          },
+          { persist: true },
+        ),
       ).rejects.toThrow(/Invalid settings schema/);
+      expect(await readFile(path, "utf8")).toBe(original);
     } finally {
       await rm(dir, { recursive: true, force: true });
     }
   });
 
-  test("loadSettings does not recover unmatched OAuth selections", async () => {
+  test("loadSettingsRecoveringClobberedOAuthSelection leaves the file unchanged when persist is false", async () => {
     const dir = await mkdtemp(join(tmpdir(), "ic-settings-"));
     try {
       const path = join(dir, "settings.json");
-      await writeFile(path, JSON.stringify({ provider: "codex/missing", model: "gpt-5.1-codex" }));
-      const recovered = await loadSettingsRecoveringClobberedOAuthSelection(path, {
-        "codex/work": {
-          baseURL: "https://chatgpt.com/backend-api",
-          apiKey: "oauth-token",
-          models: ["gpt-5.1-codex"],
+      const original = JSON.stringify({ provider: "codex/work", model: "gpt-5.1-codex" });
+      await writeFile(path, original);
+      const recovered = await loadSettingsRecoveringClobberedOAuthSelection(
+        path,
+        {
+          "codex/work": {
+            baseURL: "https://chatgpt.com/backend-api",
+            apiKey: "oauth-token",
+            models: ["gpt-5.1-codex"],
+          },
         },
-      });
-      expect(recovered).toEqual({ providers: {} });
+        { persist: false },
+      );
+      expect(recovered?.defaultProvider).toBe("codex/work");
+      expect(await readFile(path, "utf8")).toBe(original);
     } finally {
       await rm(dir, { recursive: true, force: true });
     }

tests/unit/config.test.ts

@@ -1,5 +1,5 @@
 import { test, expect } from "bun:test";
-import { mkdtemp, mkdir, writeFile, rm, symlink } from "node:fs/promises";
+import { mkdtemp, mkdir, readFile, writeFile, rm, symlink } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { loadConfig } from "../../src/config/index.js";
@@ -153,25 +153,26 @@ test("symlink alias of the default global settings path is not a programmatic ov
 });
 
 test("loadSettings recovery helper recovers only an exact clobbered local selection", async () => {
-  const { loadSettings, loadSettingsRecoveringClobberedOAuthSelection } =
+  const { loadSettingsRecoveringClobberedOAuthSelection } =
     await import("../../src/config/settings.js");
   const cwd = await mkdtemp(join(tmpdir(), "ic-unit-config-recovery-"));
   try {
     const clobberedPath = join(cwd, "clobbered.json");
-    await writeFile(clobberedPath, JSON.stringify({ provider: "openai", model: "gpt-5" }));
-    expect(await loadSettingsRecoveringClobberedOAuthSelection(clobberedPath, {})).toEqual({
-      providers: {},
-    });
-    expect(await loadSettings(clobberedPath)).toEqual({ providers: {} });
+    const clobbered = JSON.stringify({ provider: "openai", model: "gpt-5" });
+    await writeFile(clobberedPath, clobbered);
+    await expect(
+      loadSettingsRecoveringClobberedOAuthSelection(clobberedPath, {}, { persist: true }),
+    ).rejects.toThrow(/Invalid settings schema/);
+    expect(await readFile(clobberedPath, "utf8")).toBe(clobbered);
 
     const malformedPath = join(cwd, "malformed.json");
     await writeFile(
       malformedPath,
       JSON.stringify({ provider: "openai", model: "gpt-5", apiKey: "not-recoverable" }),
     );
-    await expect(loadSettingsRecoveringClobberedOAuthSelection(malformedPath, {})).rejects.toThrow(
-      /Invalid settings schema/,
-    );
+    await expect(
+      loadSettingsRecoveringClobberedOAuthSelection(malformedPath, {}, { persist: true }),
+    ).rejects.toThrow(/Invalid settings schema/);
   } finally {
     await rm(cwd, { recursive: true, force: true });
   }