feat(extensions): implement cryptographic integrity verification for extension updates (#21772)

packages/core/src/services/keychainService.test.ts

@@ -13,6 +13,9 @@ import {
   afterEach,
   type Mock,
 } from 'vitest';
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import { spawnSync } from 'node:child_process';
 import { KeychainService } from './keychainService.js';
 import { coreEvents } from '../utils/events.js';
 import { debugLogger } from '../utils/debugLogger.js';
@@ -53,6 +56,21 @@ vi.mock('../utils/debugLogger.js', () => ({
   debugLogger: { log: vi.fn() },
 }));
 
+vi.mock('node:os', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:os')>();
+  return { ...actual, platform: vi.fn() };
+});
+
+vi.mock('node:child_process', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:child_process')>();
+  return { ...actual, spawnSync: vi.fn() };
+});
+
+vi.mock('node:fs', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:fs')>();
+  return { ...actual, existsSync: vi.fn(), promises: { ...actual.promises } };
+});
+
 describe('KeychainService', () => {
   let service: KeychainService;
   const SERVICE_NAME = 'test-service';
@@ -65,6 +83,9 @@ describe('KeychainService', () => {
     service = new KeychainService(SERVICE_NAME);
     passwords = {};
 
+    vi.mocked(os.platform).mockReturnValue('linux');
+    vi.mocked(fs.existsSync).mockReturnValue(true);
+
     // Stateful mock implementation for native keychain
     mockKeytar.setPassword?.mockImplementation((_svc, acc, val) => {
       passwords[acc] = val;
@@ -197,6 +218,90 @@ describe('KeychainService', () => {
     });
   });
 
+  describe('macOS Keychain Probing', () => {
+    beforeEach(() => {
+      vi.mocked(os.platform).mockReturnValue('darwin');
+    });
+
+    it('should skip functional test and fallback if security default-keychain fails', async () => {
+      vi.mocked(spawnSync).mockReturnValue({
+        status: 1,
+        stderr: 'not found',
+        stdout: '',
+        output: [],
+        pid: 123,
+        signal: null,
+      });
+
+      const available = await service.isAvailable();
+
+      expect(available).toBe(true);
+      expect(vi.mocked(spawnSync)).toHaveBeenCalledWith(
+        'security',
+        ['default-keychain'],
+        expect.any(Object),
+      );
+      expect(mockKeytar.setPassword).not.toHaveBeenCalled();
+      expect(FileKeychain).toHaveBeenCalled();
+      expect(debugLogger.log).toHaveBeenCalledWith(
+        expect.stringContaining('MacOS default keychain not found'),
+      );
+    });
+
+    it('should skip functional test and fallback if security default-keychain returns non-existent path', async () => {
+      vi.mocked(spawnSync).mockReturnValue({
+        status: 0,
+        stdout: '  "/non/existent/path"  \n',
+        stderr: '',
+        output: [],
+        pid: 123,
+        signal: null,
+      });
+      vi.mocked(fs.existsSync).mockReturnValue(false);
+
+      const available = await service.isAvailable();
+
+      expect(available).toBe(true);
+      expect(fs.existsSync).toHaveBeenCalledWith('/non/existent/path');
+      expect(mockKeytar.setPassword).not.toHaveBeenCalled();
+      expect(FileKeychain).toHaveBeenCalled();
+    });
+
+    it('should proceed with functional test if valid default keychain is found', async () => {
+      vi.mocked(spawnSync).mockReturnValue({
+        status: 0,
+        stdout: '"/path/to/valid.keychain"',
+        stderr: '',
+        output: [],
+        pid: 123,
+        signal: null,
+      });
+      vi.mocked(fs.existsSync).mockReturnValue(true);
+
+      const available = await service.isAvailable();
+
+      expect(available).toBe(true);
+      expect(mockKeytar.setPassword).toHaveBeenCalled();
+      expect(FileKeychain).not.toHaveBeenCalled();
+    });
+
+    it('should handle unquoted paths from security output', async () => {
+      vi.mocked(spawnSync).mockReturnValue({
+        status: 0,
+        stdout: '  /path/to/valid.keychain  \n',
+        stderr: '',
+        output: [],
+        pid: 123,
+        signal: null,
+      });
+      vi.mocked(fs.existsSync).mockReturnValue(true);
+
+      await service.isAvailable();
+
+      expect(fs.existsSync).toHaveBeenCalledWith('/path/to/valid.keychain');
+    });
+  });
+
   describe('Password Operations', () => {
     beforeEach(async () => {
       await service.isAvailable();
@@ -223,6 +328,4 @@ describe('KeychainService', () => {
       expect(await service.getPassword('missing')).toBeNull();
     });
   });
-
-  // Removing 'When Unavailable' tests since the service is always available via fallback
 });

packages/core/src/services/keychainService.ts

@@ -5,6 +5,9 @@
  */
 
 import * as crypto from 'node:crypto';
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import { spawnSync } from 'node:child_process';
 import { coreEvents } from '../utils/events.js';
 import { KeychainAvailabilityEvent } from '../telemetry/types.js';
 import { debugLogger } from '../utils/debugLogger.js';
@@ -95,42 +98,56 @@ export class KeychainService {
 
   // High-level orchestration of the loading and testing cycle.
   private async initializeKeychain(): Promise<Keychain | null> {
-    let resultKeychain: Keychain | null = null;
     const forceFileStorage = process.env[FORCE_FILE_STORAGE_ENV_VAR] === 'true';
 
-    if (!forceFileStorage) {
-      try {
-        const keychainModule = await this.loadKeychainModule();
-        if (keychainModule) {
-          if (await this.isKeychainFunctional(keychainModule)) {
-            resultKeychain = keychainModule;
-          } else {
-            debugLogger.log('Keychain functional verification failed');
-          }
-        }
-      } catch (error) {
-        // Avoid logging full error objects to prevent PII exposure.
-        const message = error instanceof Error ? error.message : String(error);
-        debugLogger.log(
-          'Keychain initialization encountered an error:',
-          message,
-        );
-      }
-    }
+    // Try to get the native OS keychain unless file storage is requested.
+    const nativeKeychain = forceFileStorage
+      ? null
+      : await this.getNativeKeychain();
 
     coreEvents.emitTelemetryKeychainAvailability(
-      new KeychainAvailabilityEvent(
-        resultKeychain !== null && !forceFileStorage,
-      ),
+      new KeychainAvailabilityEvent(nativeKeychain !== null),
     );
 
-    // Fallback to FileKeychain if native keychain is unavailable or file storage is forced
-    if (!resultKeychain) {
-      resultKeychain = new FileKeychain();
-      debugLogger.log('Using FileKeychain fallback for secure storage.');
+    if (nativeKeychain) {
+      return nativeKeychain;
     }
 
-    return resultKeychain;
+    // If native failed or was skipped, return the secure file fallback.
+    debugLogger.log('Using FileKeychain fallback for secure storage.');
+    return new FileKeychain();
+  }
+
+  /**
+   * Attempts to load and verify the native keychain module (keytar).
+   */
+  private async getNativeKeychain(): Promise<Keychain | null> {
+    try {
+      const keychainModule = await this.loadKeychainModule();
+      if (!keychainModule) {
+        return null;
+      }
+
+      // Probing macOS prevents process-blocking popups when no keychain exists.
+      if (os.platform() === 'darwin' && !this.isMacOSKeychainAvailable()) {
+        debugLogger.log(
+          'MacOS default keychain not found; skipping functional verification.',
+        );
+        return null;
+      }
+
+      if (await this.isKeychainFunctional(keychainModule)) {
+        return keychainModule;
+      }
+
+      debugLogger.log('Keychain functional verification failed');
+      return null;
+    } catch (error) {
+      // Avoid logging full error objects to prevent PII exposure.
+      const message = error instanceof Error ? error.message : String(error);
+      debugLogger.log('Keychain initialization encountered an error:', message);
+      return null;
+    }
   }
 
   // Low-level dynamic loading and structural validation.
@@ -166,4 +183,36 @@ export class KeychainService {
 
     return deleted && retrieved === testPassword;
   }
+
+  /**
+   * MacOS-specific check to detect if a default keychain is available.
+   */
+  private isMacOSKeychainAvailable(): boolean {
+    // Probing via the `security` CLI avoids a blocking OS-level popup that
+    // occurs when calling keytar without a configured keychain.
+    const result = spawnSync('security', ['default-keychain'], {
+      encoding: 'utf8',
+      // We pipe stdout to read the path, but ignore stderr to suppress
+      // "keychain not found" errors from polluting the terminal.
+      stdio: ['ignore', 'pipe', 'ignore'],
+    });
+
+    // If the command fails or lacks output, no default keychain is configured.
+    if (result.error || result.status !== 0 || !result.stdout) {
+      return false;
+    }
+
+    // Validate that the returned path string is not empty.
+    const trimmed = result.stdout.trim();
+    if (!trimmed) {
+      return false;
+    }
+
+    // The output usually contains the path wrapped in double quotes.
+    const match = trimmed.match(/"(.*)"/);
+    const keychainPath = match ? match[1] : trimmed;
+
+    // Finally, verify the path exists on disk to ensure it's not a stale reference.
+    return !!keychainPath && fs.existsSync(keychainPath);
+  }
 }