feat: add salvage functionality for security-sensitive values in GlobalContext

Author: Patrick-Erichsen

core/util/GlobalContext.test.ts

@@ -115,7 +115,7 @@ describe("GlobalContext", () => {
 
     expect(consoleWarnMock).toHaveBeenCalledWith(
       expect.stringContaining(
-        "Error updating global context, deleting corrupted file",
+        "Error updating global context, attempting to salvage security-sensitive values",
       ),
     );
 
@@ -174,4 +174,154 @@ describe("GlobalContext", () => {
   it("should not crash or throw when getting a key that hasn't been set", () => {
     expect(globalContext.get("hasAlreadyCreatedAPromptFile")).toBeUndefined();
   });
+
+  describe("salvage functionality", () => {
+    it("should salvage allowAnonymousTelemetry from corrupted sharedConfig", () => {
+      // Create a file with partially corrupted JSON that contains salvageable sharedConfig
+      const corruptedContent = `{
+  "indexingPaused": true,
+  "sharedConfig": {"allowAnonymousTelemetry": false},
+  "corrupted": { invalid json
+}`;
+      fs.writeFileSync(globalContextFilePath, corruptedContent);
+
+      const consoleWarnMock = jest
+        .spyOn(console, "warn")
+        .mockImplementation(() => {});
+
+      // Try to update - this should salvage the allowAnonymousTelemetry value
+      globalContext.update("indexingPaused", false);
+
+      expect(consoleWarnMock).toHaveBeenCalledWith(
+        expect.stringContaining(
+          "Error updating global context, attempting to salvage security-sensitive values",
+        ),
+      );
+
+      // Should have the new value
+      expect(globalContext.get("indexingPaused")).toBe(false);
+
+      // Should have salvaged the sharedConfig with allowAnonymousTelemetry
+      const salvaged = globalContext.get("sharedConfig");
+      expect(salvaged).toEqual({ allowAnonymousTelemetry: false });
+
+      consoleWarnMock.mockRestore();
+    });
+
+    it("should handle salvage when no sharedConfig is present", () => {
+      const corruptedContent = `{
+  "indexingPaused": true,
+  "corrupted": { invalid json
+}`;
+      fs.writeFileSync(globalContextFilePath, corruptedContent);
+
+      const consoleWarnMock = jest
+        .spyOn(console, "warn")
+        .mockImplementation(() => {});
+
+      globalContext.update("hasAlreadyCreatedAPromptFile", true);
+
+      expect(consoleWarnMock).toHaveBeenCalledWith(
+        expect.stringContaining(
+          "Error updating global context, attempting to salvage security-sensitive values",
+        ),
+      );
+
+      // Should have the new value
+      expect(globalContext.get("hasAlreadyCreatedAPromptFile")).toBe(true);
+
+      // Should not have any sharedConfig since none was salvageable
+      expect(globalContext.get("sharedConfig")).toBeUndefined();
+
+      consoleWarnMock.mockRestore();
+    });
+
+    it("should handle salvage when sharedConfig has invalid allowAnonymousTelemetry", () => {
+      const corruptedContent = `{
+  "indexingPaused": true,
+  "sharedConfig": {"allowAnonymousTelemetry": "not-a-boolean"},
+  "corrupted": { invalid json
+}`;
+      fs.writeFileSync(globalContextFilePath, corruptedContent);
+
+      const consoleWarnMock = jest
+        .spyOn(console, "warn")
+        .mockImplementation(() => {});
+
+      globalContext.update("indexingPaused", false);
+
+      // Should not have salvaged an invalid boolean value
+      const salvaged = globalContext.get("sharedConfig");
+      expect(salvaged).toBeUndefined();
+
+      consoleWarnMock.mockRestore();
+    });
+
+    it("should handle salvage with valid allowAnonymousTelemetry set to true", () => {
+      const corruptedContent = `{
+  "indexingPaused": true,
+  "sharedConfig": {"allowAnonymousTelemetry": true, "otherField": "value"},
+  "corrupted": { invalid json
+}`;
+      fs.writeFileSync(globalContextFilePath, corruptedContent);
+
+      const consoleWarnMock = jest
+        .spyOn(console, "warn")
+        .mockImplementation(() => {});
+
+      globalContext.update("hasAlreadyCreatedAPromptFile", true);
+
+      // Should have salvaged only the allowAnonymousTelemetry value
+      const salvaged = globalContext.get("sharedConfig");
+      expect(salvaged).toEqual({ allowAnonymousTelemetry: true });
+
+      consoleWarnMock.mockRestore();
+    });
+
+    it("should handle completely unparseable content gracefully", () => {
+      fs.writeFileSync(
+        globalContextFilePath,
+        "complete garbage not json at all",
+      );
+
+      const consoleWarnMock = jest
+        .spyOn(console, "warn")
+        .mockImplementation(() => {});
+
+      globalContext.update("indexingPaused", true);
+
+      // Should still work and create a new file
+      expect(globalContext.get("indexingPaused")).toBe(true);
+      expect(globalContext.get("sharedConfig")).toBeUndefined();
+
+      consoleWarnMock.mockRestore();
+    });
+
+    it("should preserve multiple salvageable values when updating different key", () => {
+      const corruptedContent = `{
+  "indexingPaused": true,
+  "sharedConfig": {"allowAnonymousTelemetry": false},
+  "hasAlreadyCreatedAPromptFile": true,
+  "corrupted": { invalid json
+}`;
+      fs.writeFileSync(globalContextFilePath, corruptedContent);
+
+      const consoleWarnMock = jest
+        .spyOn(console, "warn")
+        .mockImplementation(() => {});
+
+      // Update a different key - should salvage and preserve existing data
+      globalContext.update("showConfigUpdateToast", false);
+
+      // Should have the new value
+      expect(globalContext.get("showConfigUpdateToast")).toBe(false);
+
+      // Should have salvaged the security-sensitive value
+      expect(globalContext.get("sharedConfig")).toEqual({
+        allowAnonymousTelemetry: false,
+      });
+
+      consoleWarnMock.mockRestore();
+    });
+  });
 });

core/util/GlobalContext.ts

@@ -9,10 +9,6 @@ import {
   SharedConfigSchema,
 } from "../config/sharedConfig";
 
-import {
-  OAuthClientInformationFull,
-  OAuthTokens,
-} from "@modelcontextprotocol/sdk/shared/auth.js";
 import { getGlobalContextFilePath } from "./paths";
 
 export type GlobalContextModelSelections = Partial<
@@ -43,14 +39,6 @@ export type GlobalContextType = {
   isSupportedLanceDbCpuTargetForLinux: boolean;
   sharedConfig: SharedConfigSchema;
   failedDocs: SiteIndexingConfig[];
-  mcpOauthStorage: Record<
-    string,
-    {
-      clientInformation?: OAuthClientInformationFull;
-      tokens?: OAuthTokens;
-      codeVerifier?: string;
-    }
-  >;
   shownDeprecatedProviderWarnings: {
     [providerTitle: string]: boolean;
   };
@@ -84,8 +72,25 @@ export class GlobalContext {
         parsed = JSON.parse(data);
       } catch (e: any) {
         console.warn(
-          `Error updating global context, deleting corrupted file: ${e}`,
+          `Error updating global context, attempting to salvage security-sensitive values: ${e}`,
         );
+
+        // Attempt to salvage security-sensitive values before deleting
+        let salvaged: Partial<GlobalContextType> = {};
+        try {
+          // Try to partially parse the corrupted data to extract sharedConfig
+          const match = data.match(/"sharedConfig"\s*:\s*({[^}]*})/);
+          if (match) {
+            const sharedConfigObj = JSON.parse(match[1]);
+            const salvagedSharedConfig = salvageSharedConfig(sharedConfigObj);
+            if (Object.keys(salvagedSharedConfig).length > 0) {
+              salvaged.sharedConfig = salvagedSharedConfig;
+            }
+          }
+        } catch {
+          // If salvage fails, continue with empty salvaged object
+        }
+
         // Delete the corrupted file and recreate it fresh
         try {
           fs.unlinkSync(filepath);
@@ -94,17 +99,14 @@ export class GlobalContext {
             `Error deleting corrupted global context file: ${deleteError}`,
           );
         }
-        // Recreate the file with just the new value
-        fs.writeFileSync(
-          filepath,
-          JSON.stringify(
-            {
-              [key]: value,
-            },
-            null,
-            2,
-          ),
-        );
+
+        // Recreate the file with salvaged values plus the new value
+        const newData = {
+          ...salvaged,
+          [key]: value,
+        };
+
+        fs.writeFileSync(filepath, JSON.stringify(newData, null, 2));
         return;
       }