From dfb67da92cbaa9b28aa2e0eb289f608704a4cfbb Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Fri, 24 Apr 2026 15:58:47 +0200 Subject: [PATCH] Move shared keychain migration from renderer to main process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace crossAppIPC-based secret handshake with direct shared keychain writes in the main process: - MacOSCrossAppSecretSharing now reads safeStorage+SQLite and writes to shared keychain via SharedKeychainMainService (no crossAppIPC needed) - Code.app migrates on startup; Agents app spawns Code.app once if keychain is incomplete - NativeSecretStorageService no longer does migration — just reads/writes shared keychain for cross-app keys --- src/vs/code/electron-main/app.ts | 2 +- .../macOSCrossAppSecretSharing.ts | 306 +++++------------- .../electron-browser/secretStorageService.ts | 41 +-- 3 files changed, 86 insertions(+), 263 deletions(-) diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index f31253f480c..738f69e33c6 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -1275,12 +1275,12 @@ export class CodeApplication extends Disposable { this._register(new MacOSCrossAppSecretSharing( accessor.get(IStorageMainService), accessor.get(IEncryptionMainService), + accessor.get(ISharedKeychainMainService), accessor.get(IStateService), this.logService, this.environmentMainService, accessor.get(ILaunchMainService), this.lifecycleMainService, - crossAppIPCService, )); } diff --git a/src/vs/platform/secrets/electron-main/macOSCrossAppSecretSharing.ts b/src/vs/platform/secrets/electron-main/macOSCrossAppSecretSharing.ts index 6dfea9bc15a..c95e639f072 100644 --- a/src/vs/platform/secrets/electron-main/macOSCrossAppSecretSharing.ts +++ b/src/vs/platform/secrets/electron-main/macOSCrossAppSecretSharing.ts @@ -5,75 +5,49 @@ import { execFile } from 'child_process'; import { dirname } from '../../../base/common/path.js'; -import { Disposable, DisposableStore } from '../../../base/common/lifecycle.js'; +import { Disposable } from '../../../base/common/lifecycle.js'; import { ILogService } from '../../log/common/log.js'; import { IEncryptionMainService } from '../../encryption/common/encryptionService.js'; import { IStorageMainService } from '../../storage/electron-main/storageMainService.js'; -import { CROSS_APP_SHARED_SECRET_KEYS, secretStorageKey, readEncryptedSecret, writeEncryptedSecret } from '../common/secrets.js'; +import { CROSS_APP_SHARED_SECRET_KEYS, readEncryptedSecret } from '../common/secrets.js'; import { IStateService } from '../../state/node/state.js'; import { INodeProcess, isMacintosh } from '../../../base/common/platform.js'; import { IStorageMain } from '../../storage/electron-main/storageMain.js'; import { IEnvironmentMainService } from '../../environment/electron-main/environmentMainService.js'; import { ILaunchMainService } from '../../launch/electron-main/launchMainService.js'; import { ILifecycleMainService } from '../../lifecycle/electron-main/lifecycleMainService.js'; -import { ICrossAppIPCService } from '../../crossAppIpc/electron-main/crossAppIpcService.js'; +import { ISharedKeychainMainService } from '../common/sharedKeychainService.js'; -const MIGRATION_STATE_KEY = 'crossAppSecretSharing.migrationDone'; - -/** - * Message types exchanged between apps over crossAppIPC for secret sharing. - */ -const enum CrossAppSecretMessageType { - /** Agents → Host: Request secrets */ - SecretRequest = 'secrets/request', - /** Host → Agents: Response with secrets */ - SecretResponse = 'secrets/response', - /** Agents → Host: Confirms secrets were stored, both sides mark migration done */ - SecretAck = 'secrets/ack', -} - -interface CrossAppSecretMessage { - type: CrossAppSecretMessageType; - data?: Record; -} +const MIGRATION_STATE_KEY = 'sharedKeychain.migrationDone'; +const HOST_SPAWN_STATE_KEY = 'sharedKeychain.hostSpawnDone'; /** * Coordinates one-time secret migration between the VS Code app and the - * agents app using Electron's crossAppIPC (macOS only). + * agents app via the macOS shared keychain (macOS only). * - * **Demand-driven**: Only the agents app initiates migration. If it - * detects that migration hasn't been done yet, it: - * 1. Waits for the crossAppIPC connection (managed by ICrossAppIPCService). - * 2. Spawns Code.app with `--share-secrets-with-agents-app`, which - * either starts Code.app fresh or (if already running) forwards - * the arg to the existing instance via the node IPC socket. - * 3. Code.app creates its own crossAppIPC connection when it sees - * the arg, and the two connect. - * 4. Agents app sends `SecretRequest` → Code.app responds with - * `SecretResponse` → Agents app sends `SecretAck`. - * 5. Both sides mark migration as done. Code.app quits if it was - * launched solely for this purpose. + * Each app migrates its own secrets from safeStorage+SQLite into the + * shared keychain on startup. The agents app also spawns Code.app + * (once) with `--share-secrets-with-agents-app` to trigger Code's + * migration if the shared keychain doesn't yet contain all expected + * keys. * - * Security: crossAppIPC uses code-signature verification (Mach ports - * on macOS) — the kernel authenticates both endpoints. No secrets are - * ever in process args, files, or network. + * After migration, both apps read from and write to the shared keychain + * for cross-app secret keys (via {@link NativeSecretStorageService}). */ export class MacOSCrossAppSecretSharing extends Disposable { private readonly isEmbeddedApp: boolean; private readonly applicationStorage: IStorageMain; - private _onHostMigrationComplete: (() => void) | undefined; - private readonly hostHandshakeListeners = this._register(new DisposableStore()); constructor( storageMainService: IStorageMainService, private readonly encryptionMainService: IEncryptionMainService, + private readonly sharedKeychainMainService: ISharedKeychainMainService, private readonly stateService: IStateService, private readonly logService: ILogService, environmentMainService: IEnvironmentMainService, launchMainService: ILaunchMainService, lifecycleMainService: ILifecycleMainService, - private readonly crossAppIPCService: ICrossAppIPCService, ) { super(); this.isEmbeddedApp = !!(process as INodeProcess).isEmbeddedApp; @@ -87,143 +61,96 @@ export class MacOSCrossAppSecretSharing extends Disposable { lifecycleMainService: ILifecycleMainService, ): void { if (this.isEmbeddedApp) { - // Agents app: initiate migration if needed + // Agents app: migrate own secrets + spawn Code.app if needed this.initializeAsAgentsApp(); } else if (environmentMainService.args['share-secrets-with-agents-app']) { - // Code.app launched fresh with --share-secrets-with-agents-app: - // respond to the agents app's request, then quit if no other reason to stay + // Code.app launched with --share-secrets-with-agents-app: + // migrate secrets to shared keychain, then quit if no other reason to stay const hasOtherArgs = environmentMainService.args._.length > 0 || environmentMainService.args['folder-uri'] || environmentMainService.args['file-uri']; - this.initializeAsHostApp(hasOtherArgs ? undefined : () => { - this.logService.info('[CrossAppSecretSharing] Host app was launched for migration only, quitting'); - lifecycleMainService.quit(); + this.migrateSecrets().then(() => { + if (!hasOtherArgs) { + this.logService.info('[CrossAppSecretSharing] Host app was launched for migration only, quitting'); + lifecycleMainService.quit(); + } }); } else { - // Code.app already running: listen for --share-secrets-with-agents-app - // forwarded from a second instance via the launch service + // Code.app normal startup: migrate own secrets + this.migrateSecrets(); + // Also respond to spawn requests from the agents app this._register(launchMainService.onDidRequestShareSecrets(() => { - this.initializeAsHostApp(); + this.migrateSecrets(); })); } } private async initializeAsAgentsApp(): Promise { - if (!isMacintosh || !this.isEmbeddedApp) { + if (!isMacintosh) { return; } - if (this.isMigrationDone()) { - this.logService.trace('[CrossAppSecretSharing] Migration already done, skipping'); + // Migrate own secrets (if any) to shared keychain + await this.migrateSecrets(); + + // If we've already spawned Code.app before, don't do it again + if (this.stateService.getItem(HOST_SPAWN_STATE_KEY, false)) { return; } - // Wait for storage to be ready before we start — handleSecretResponse - // will write secrets into applicationStorage. - await this.applicationStorage.whenInit; - - if (!this.crossAppIPCService.initialized) { - this.logService.info('[CrossAppSecretSharing] crossAppIPC not initialized, skipping migration'); - return; - } - - this.logService.info('[CrossAppSecretSharing] Migration needed, starting...'); - - // Listen for connection — when connected, request secrets - this._register(this.crossAppIPCService.onDidConnect(isServer => { - this.logService.info(`[CrossAppSecretSharing] Connected (isServer=${isServer}), requesting secrets from host app`); - this.crossAppIPCService.sendMessage({ type: CrossAppSecretMessageType.SecretRequest }); - })); - - // Listen for messages - this._register(this.crossAppIPCService.onDidReceiveMessage(msg => { - const secretMsg = msg as CrossAppSecretMessage; - if (secretMsg?.type === CrossAppSecretMessageType.SecretResponse) { - this.handleSecretResponse(secretMsg.data ?? {}); - } - })); - - // If already connected (e.g. service was initialized before storage was ready), - // send the request immediately. - if (this.crossAppIPCService.connected) { - this.logService.info(`[CrossAppSecretSharing] Already connected (isServer=${this.crossAppIPCService.isServer}), requesting secrets from host app`); - this.crossAppIPCService.sendMessage({ type: CrossAppSecretMessageType.SecretRequest }); - } - - // Spawn Code.app with --share-secrets-with-agents-app - this.spawnHostApp(); - - // Timeout: if migration doesn't complete within 30s, give up - setTimeout(() => { - if (!this.isMigrationDone()) { - this.logService.warn('[CrossAppSecretSharing] Migration timed out'); - } - }, 30_000); - } - - private async initializeAsHostApp(onComplete?: () => void): Promise { - if (!isMacintosh || this.isEmbeddedApp) { - onComplete?.(); - return; - } - - if (this.isMigrationDone()) { - this.logService.trace('[CrossAppSecretSharing] Migration already done, skipping'); - onComplete?.(); - return; - } - - // Wait for application storage to be fully initialized before - // checking for secrets — storage may still be in-memory at this - // point during early startup. - await this.applicationStorage.whenInit; - - if (!this.hasAnySharedSecrets()) { - this.logService.trace('[CrossAppSecretSharing] No shared secrets to share, skipping'); - onComplete?.(); - return; - } - - if (!this.crossAppIPCService.initialized) { - this.logService.info('[CrossAppSecretSharing] crossAppIPC not initialized'); - onComplete?.(); - return; - } - - this._onHostMigrationComplete = onComplete; - - this.logService.info('[CrossAppSecretSharing] Host app responding to secret sharing request'); - - // Dispose previous listeners if initializeAsHostApp is called again - // (e.g. via repeated onDidRequestShareSecrets events). - this.hostHandshakeListeners.clear(); - - // Listen for messages from the agents app - this.hostHandshakeListeners.add(this.crossAppIPCService.onDidReceiveMessage(msg => { - const secretMsg = msg as CrossAppSecretMessage; - if (secretMsg?.type === CrossAppSecretMessageType.SecretRequest) { - this.handleSecretRequest(); - } else if (secretMsg?.type === CrossAppSecretMessageType.SecretAck) { - this.handleSecretAck(); - } - })); - - // If disconnected before ack, still allow the host to quit - this.hostHandshakeListeners.add(this.crossAppIPCService.onDidDisconnect(() => { - this._onHostMigrationComplete?.(); - this._onHostMigrationComplete = undefined; - })); - } - - private isMigrationDone(): boolean { - return this.stateService.getItem(MIGRATION_STATE_KEY, false); - } - - private hasAnySharedSecrets(): boolean { + // Check if the shared keychain has all expected keys + let needsHostMigration = false; for (const key of CROSS_APP_SHARED_SECRET_KEYS) { - if (this.applicationStorage.get(secretStorageKey(key)) !== undefined) { - return true; + if (await this.sharedKeychainMainService.get(key) === undefined) { + needsHostMigration = true; + break; } } - return false; + + if (needsHostMigration) { + this.logService.info('[CrossAppSecretSharing] Shared keychain incomplete, spawning host app'); + this.spawnHostApp(); + } + + // Mark that we've attempted the host spawn (don't retry on next startup) + this.stateService.setItem(HOST_SPAWN_STATE_KEY, true); + } + + /** + * Migrates this app's secrets from safeStorage+SQLite to the shared keychain. + * Idempotent — skips if already done. + */ + private async migrateSecrets(): Promise { + if (!isMacintosh) { + return; + } + + if (this.stateService.getItem(MIGRATION_STATE_KEY, false)) { + this.logService.trace('[CrossAppSecretSharing] Migration already done, skipping'); + return; + } + + await this.applicationStorage.whenInit; + + this.logService.info('[CrossAppSecretSharing] Starting shared keychain migration'); + + for (const key of CROSS_APP_SHARED_SECRET_KEYS) { + try { + const decrypted = await readEncryptedSecret( + key, + (fullKey) => this.applicationStorage.get(fullKey), + (value) => this.encryptionMainService.decrypt(value), + this.logService, + ); + if (decrypted !== undefined) { + await this.sharedKeychainMainService.set(key, decrypted); + this.logService.trace('[CrossAppSecretSharing] Migrated key to shared keychain:', key); + } + } catch (err) { + this.logService.error('[CrossAppSecretSharing] Failed to migrate key:', key, err); + } + } + + this.stateService.setItem(MIGRATION_STATE_KEY, true); + this.logService.info('[CrossAppSecretSharing] Migration complete'); } private spawnHostApp(): void { @@ -247,69 +174,4 @@ export class MacOSCrossAppSecretSharing extends Disposable { }); child.unref(); } - - private async handleSecretRequest(): Promise { - this.logService.info('[CrossAppSecretSharing] Host app handling secret request'); - - const secrets: Record = {}; - - for (const key of CROSS_APP_SHARED_SECRET_KEYS) { - try { - const decrypted = await readEncryptedSecret( - key, - (fullKey) => this.applicationStorage.get(fullKey), - (value) => this.encryptionMainService.decrypt(value), - this.logService, - ); - if (decrypted !== undefined) { - secrets[key] = decrypted; - } - } catch (err) { - this.logService.error('[CrossAppSecretSharing] Failed to read secret for key:', key, err); - } - } - - this.crossAppIPCService.sendMessage({ type: CrossAppSecretMessageType.SecretResponse, data: secrets }); - this.logService.info('[CrossAppSecretSharing] Sent secrets response with', Object.keys(secrets).length, 'keys'); - } - - private async handleSecretResponse(secrets: Record): Promise { - this.logService.info('[CrossAppSecretSharing] Agents app received', Object.keys(secrets).length, 'secrets'); - - for (const [key, value] of Object.entries(secrets)) { - if (!CROSS_APP_SHARED_SECRET_KEYS.includes(key)) { - this.logService.warn('[CrossAppSecretSharing] Ignoring unexpected key:', key); - continue; - } - - try { - await writeEncryptedSecret( - key, - value, - (fullKey, encrypted) => this.applicationStorage.set(fullKey, encrypted), - (v) => this.encryptionMainService.encrypt(v), - this.logService, - ); - } catch (err) { - this.logService.error('[CrossAppSecretSharing] Failed to store secret for key:', key, err); - } - } - - this.stateService.setItem(MIGRATION_STATE_KEY, true); - this.logService.info('[CrossAppSecretSharing] Migration complete'); - - // Tell the host app migration is done so it can also record it. - // Don't close here — let the host close first after receiving the ack. - this.crossAppIPCService.sendMessage({ type: CrossAppSecretMessageType.SecretAck }); - } - - private handleSecretAck(): void { - this.stateService.setItem(MIGRATION_STATE_KEY, true); - this.logService.info('[CrossAppSecretSharing] Host app received ack, migration complete on both sides'); - - const onComplete = this._onHostMigrationComplete; - this._onHostMigrationComplete = undefined; - - onComplete?.(); - } } diff --git a/src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts b/src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts index 81ccbb5355b..c8d4199b5bd 100644 --- a/src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts +++ b/src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts @@ -16,15 +16,11 @@ import { INotificationService, IPromptChoice } from '../../../../platform/notifi import { IOpenerService } from '../../../../platform/opener/common/opener.js'; import { BaseSecretStorageService, CROSS_APP_SHARED_SECRET_KEYS, ISecretStorageService } from '../../../../platform/secrets/common/secrets.js'; import { ISharedKeychainService } from '../../../../platform/secrets/common/sharedKeychainService.js'; -import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js'; +import { IStorageService } from '../../../../platform/storage/common/storage.js'; import { IJSONEditingService } from '../../configuration/common/jsonEditing.js'; -const MIGRATION_STORAGE_KEY = 'sharedKeychain.migrationDone'; - export class NativeSecretStorageService extends BaseSecretStorageService { - private readonly _migrationPromise: Promise; - constructor( @INotificationService private readonly _notificationService: INotificationService, @IDialogService private readonly _dialogService: IDialogService, @@ -42,43 +38,11 @@ export class NativeSecretStorageService extends BaseSecretStorageService { encryptionService, logService ); - - this._migrationPromise = this._doMigration(); - } - - private async _doMigration(): Promise { - if (this.type === 'in-memory') { - return; - } - - const storageService = await this.resolvedStorageService; - if (storageService.get(MIGRATION_STORAGE_KEY, StorageScope.APPLICATION) === '1') { - this._logService.trace('[NativeSecretStorageService] shared keychain migration already done'); - return; - } - - this._logService.trace('[NativeSecretStorageService] starting shared keychain migration'); - - for (const sharedKey of CROSS_APP_SHARED_SECRET_KEYS) { - try { - const value = await this._doGet(sharedKey); - if (value !== undefined) { - await this._sharedKeychainService.set(sharedKey, value); - this._logService.trace('[NativeSecretStorageService] shared keychain migration: migrated', sharedKey); - } - } catch (err) { - this._logService.error('[NativeSecretStorageService] migration failed for:', sharedKey, err); - } - } - - storageService.store(MIGRATION_STORAGE_KEY, '1', StorageScope.APPLICATION, StorageTarget.MACHINE); - this._logService.trace('[NativeSecretStorageService] shared keychain migration complete'); } override get(key: string): Promise { return this._sequencer.queue(key, async () => { if (this.type !== 'in-memory' && CROSS_APP_SHARED_SECRET_KEYS.includes(key)) { - await this._migrationPromise; // Try shared keychain first (no-op on non-macOS) const value = await this._sharedKeychainService.get(key); if (value !== undefined) { @@ -101,7 +65,6 @@ export class NativeSecretStorageService extends BaseSecretStorageService { }); return this._sequencer.queue(key, async () => { if (this.type !== 'in-memory' && CROSS_APP_SHARED_SECRET_KEYS.includes(key)) { - await this._migrationPromise; // Write to shared keychain (no-op on non-macOS) await this._sharedKeychainService.set(key, value); } @@ -113,7 +76,6 @@ export class NativeSecretStorageService extends BaseSecretStorageService { override delete(key: string): Promise { return this._sequencer.queue(key, async () => { if (this.type !== 'in-memory' && CROSS_APP_SHARED_SECRET_KEYS.includes(key)) { - await this._migrationPromise; // Delete from shared keychain (no-op on non-macOS) await this._sharedKeychainService.delete(key); } @@ -126,7 +88,6 @@ export class NativeSecretStorageService extends BaseSecretStorageService { return this._sequencer.queue('__keys__', async () => { const legacyKeys = await this._doGetKeys(); if (this.type !== 'in-memory') { - await this._migrationPromise; // Include any cross-app shared keys present in the shared keychain for (const sharedKey of CROSS_APP_SHARED_SECRET_KEYS) { const sharedValue = await this._sharedKeychainService.get(sharedKey);