diff --git a/build/.moduleignore b/build/.moduleignore index d199042ac93..f83624f893c 100644 --- a/build/.moduleignore +++ b/build/.moduleignore @@ -159,14 +159,6 @@ vsda/** @vscode/policy-watcher/index.d.ts !@vscode/policy-watcher/build/Release/vscode-policy-watcher.node -@vscode/macos-keychain/build/** -@vscode/macos-keychain/src/** -@vscode/macos-keychain/test/** -@vscode/macos-keychain/binding.gyp -@vscode/macos-keychain/README.md -@vscode/macos-keychain/index.d.ts -!@vscode/macos-keychain/build/Release/keychainNative.node - @vscode/windows-ca-certs/**/* !@vscode/windows-ca-certs/package.json !@vscode/windows-ca-certs/**/*.node diff --git a/build/azure-pipelines/darwin/app-entitlements.plist b/build/azure-pipelines/darwin/app-entitlements.plist index db62570f361..4073eafcf56 100644 --- a/build/azure-pipelines/darwin/app-entitlements.plist +++ b/build/azure-pipelines/darwin/app-entitlements.plist @@ -10,9 +10,5 @@ com.apple.security.automation.apple-events - keychain-access-groups - - $(TeamIdentifierPrefix)com.microsoft.vscode.shared-secrets - diff --git a/build/azure-pipelines/darwin/steps/product-build-darwin-compile.yml b/build/azure-pipelines/darwin/steps/product-build-darwin-compile.yml index f5972f6224d..a90e25aeabe 100644 --- a/build/azure-pipelines/darwin/steps/product-build-darwin-compile.yml +++ b/build/azure-pipelines/darwin/steps/product-build-darwin-compile.yml @@ -332,16 +332,6 @@ steps: BUILD_SOURCESDIRECTORY: $(Build.SourcesDirectory) displayName: ✍️ Codesign & Notarize - # Re-sign the app without the provisioning profile for tests. - # This strips the keychain-access-groups entitlement which requires a - # provisioning profile and is not needed for running tests. The codesign - # step reads from the archives packaged above which have the full entitlements. - - script: | - set -e - export CODESIGN_IDENTITY=$(security find-identity -v -p codesigning $(agent.tempdirectory)/buildagent.keychain | grep -oEi "([0-9A-F]{40})" | head -n 1) - DEBUG=electron-osx-sign* node build/darwin/sign.ts --skip-provisioning-profile $(agent.builddirectory) - displayName: Set Hardened Entitlements (for tests) - - ${{ if or(eq(parameters.VSCODE_RUN_ELECTRON_TESTS, true), eq(parameters.VSCODE_RUN_BROWSER_TESTS, true), eq(parameters.VSCODE_RUN_REMOTE_TESTS, true)) }}: - template: product-build-darwin-test.yml@self parameters: diff --git a/build/darwin/sign.ts b/build/darwin/sign.ts index fc53479855c..26e22aee08c 100644 --- a/build/darwin/sign.ts +++ b/build/darwin/sign.ts @@ -18,14 +18,7 @@ function getElectronVersion(): string { return target; } -const mainProvisioningProfilePath = path.join(baseDir, 'darwin', 'main.provisionprofile'); -const agentsProvisioningProfilePath = path.join(baseDir, 'darwin', 'agents.provisionprofile'); - -function hasProvisioningProfile(): boolean { - return fs.existsSync(mainProvisioningProfilePath); -} - -function getEntitlementsForFile(filePath: string, tempDir: string, useProvisioningProfile: boolean, teamId?: string): string { +function getEntitlementsForFile(filePath: string): string { if (filePath.includes(' Helper (GPU).app')) { return path.join(baseDir, 'azure-pipelines', 'darwin', 'helper-gpu-entitlements.plist'); } else if (filePath.includes(' Helper (Renderer).app')) { @@ -33,51 +26,7 @@ function getEntitlementsForFile(filePath: string, tempDir: string, useProvisioni } else if (filePath.includes(' Helper (Plugin).app')) { return path.join(baseDir, 'azure-pipelines', 'darwin', 'helper-plugin-entitlements.plist'); } - const entitlementsPath = path.join(baseDir, 'azure-pipelines', 'darwin', 'app-entitlements.plist'); - if (!useProvisioningProfile) { - // Without a provisioning profile, keychain-access-groups entitlement - // will cause signing failures. Strip it from the entitlements plist. - return getStrippedEntitlements(entitlementsPath, tempDir); - } - if (teamId) { - return getExpandedEntitlements(entitlementsPath, tempDir, teamId); - } - return entitlementsPath; -} - -let _strippedEntitlementsPath: string | undefined; - -/** - * Returns a path to a copy of the entitlements plist with the - * keychain-access-groups key removed. - */ -function getStrippedEntitlements(entitlementsPath: string, tempDir: string): string { - if (!_strippedEntitlementsPath) { - const content = fs.readFileSync(entitlementsPath, 'utf8'); - const stripped = content.replace( - /\s*keychain-access-groups<\/key>\s*[\s\S]*?<\/array>/, - '' - ); - _strippedEntitlementsPath = path.join(tempDir, 'app-entitlements-stripped.plist'); - fs.writeFileSync(_strippedEntitlementsPath, stripped); - } - return _strippedEntitlementsPath; -} - -let expandedEntitlementsPath: string | undefined; - -/** - * Returns a path to a copy of the entitlements plist with - * $(TeamIdentifierPrefix) expanded to the actual team identifier. - */ -function getExpandedEntitlements(entitlementsPath: string, tempDir: string, teamId: string): string { - if (!expandedEntitlementsPath) { - const content = fs.readFileSync(entitlementsPath, 'utf8'); - const expanded = content.replace(/\$\(TeamIdentifierPrefix\)/g, teamId + '.'); - expandedEntitlementsPath = path.join(tempDir, 'app-entitlements.plist'); - fs.writeFileSync(expandedEntitlementsPath, expanded); - } - return expandedEntitlementsPath; + return path.join(baseDir, 'azure-pipelines', 'darwin', 'app-entitlements.plist'); } async function retrySignOnKeychainError(fn: () => Promise, maxRetries: number = 3): Promise { @@ -109,7 +58,7 @@ async function retrySignOnKeychainError(fn: () => Promise, maxRetries: num throw lastError; } -async function main(buildDir?: string, skipProvisioningProfile?: boolean): Promise { +async function main(buildDir?: string): Promise { const tempDir = process.env['AGENT_TEMPDIRECTORY']; const arch = process.env['VSCODE_ARCH']; const identity = process.env['CODESIGN_IDENTITY']; @@ -129,42 +78,15 @@ async function main(buildDir?: string, skipProvisioningProfile?: boolean): Promi ? path.resolve(appRoot, appName, 'Contents', 'Applications', `${product.embedded.nameLong}.app`, 'Contents', 'Info.plist') : undefined; - const useProvisioningProfile = !skipProvisioningProfile && hasProvisioningProfile(); - const resolvedProvisioningProfile = useProvisioningProfile ? mainProvisioningProfilePath : undefined; - - let teamId: string | undefined; - if (resolvedProvisioningProfile) { - const profilePlist = await spawn('security', ['cms', '-D', '-i', resolvedProvisioningProfile]); - const teamIdMatch = /TeamIdentifier<\/key>\s*\s*(.*?)<\/string>/s.exec(profilePlist); - if (teamIdMatch) { - teamId = teamIdMatch[1]; - console.log(`Extracted TeamIdentifier from provisioning profile: ${teamId}`); - } else { - console.warn('Could not extract TeamIdentifier from provisioning profile; $(TeamIdentifierPrefix) will not be expanded'); - } - } - - // Embed the agents provisioning profile into the embedded app bundle - // before signing, since @electron/osx-sign only supports one top-level profile. - if (useProvisioningProfile && product.embedded && fs.existsSync(agentsProvisioningProfilePath)) { - const embeddedAppPath = path.join(appRoot, appName, 'Contents', 'Applications', `${product.embedded.nameLong}.app`); - if (fs.existsSync(embeddedAppPath)) { - const embeddedProfileDest = path.join(embeddedAppPath, 'Contents', 'embedded.provisionprofile'); - fs.copyFileSync(agentsProvisioningProfilePath, embeddedProfileDest); - console.log(`Embedded agents provisioning profile into ${embeddedProfileDest}`); - } - } - const appOpts: SignOptions = { app: path.join(appRoot, appName), platform: 'darwin', optionsForFile: (filePath) => ({ - entitlements: getEntitlementsForFile(filePath, tempDir, useProvisioningProfile, teamId), + entitlements: getEntitlementsForFile(filePath), hardenedRuntime: true, }), preAutoEntitlements: false, - preEmbedProvisioningProfile: !!resolvedProvisioningProfile, - provisioningProfile: resolvedProvisioningProfile, + preEmbedProvisioningProfile: false, keychain: path.join(tempDir, 'buildagent.keychain'), version: getElectronVersion(), identity, @@ -172,8 +94,7 @@ async function main(buildDir?: string, skipProvisioningProfile?: boolean): Promi // Only overwrite plist entries for x64 and arm64 builds, // universal will get its copy from the x64 build. - // Skip when re-signing (skipProvisioningProfile) since entries already exist. - if (arch !== 'universal' && !skipProvisioningProfile) { + if (arch !== 'universal') { await spawn('plutil', [ '-insert', 'NSAppleEventsUsageDescription', @@ -250,19 +171,10 @@ async function main(buildDir?: string, skipProvisioningProfile?: boolean): Promi } await retrySignOnKeychainError(() => sign(appOpts)); - - // Dump entitlements from the signed binary for diagnostic purposes - const mainBinary = path.join(appRoot, appName, 'Contents', 'MacOS', product.nameShort); - console.log(`Dumping entitlements from signed binary: ${mainBinary}`); - const entitlementsDump = await spawn('codesign', ['--display', '--entitlements', '-', '--xml', mainBinary]); - console.log(`Signed entitlements:\n${entitlementsDump}`); } if (import.meta.main) { - const args = process.argv.slice(2); - const skipProvisioningProfile = args.includes('--skip-provisioning-profile'); - const buildDir = args.filter(a => !a.startsWith('--'))[0]; - main(buildDir, skipProvisioningProfile).catch(async err => { + main(process.argv[2]).catch(async err => { console.error(err); const tempDir = process.env['AGENT_TEMPDIRECTORY']; if (tempDir) { diff --git a/package-lock.json b/package-lock.json index 884d9d05c71..477c2579e76 100644 --- a/package-lock.json +++ b/package-lock.json @@ -171,7 +171,6 @@ "yaserver": "^0.4.0" }, "optionalDependencies": { - "@vscode/macos-keychain": "^0.0.1", "windows-foreground-love": "0.6.1" } }, @@ -3812,31 +3811,6 @@ "url": "https://github.com/sponsors/isaacs" } }, - "node_modules/@vscode/macos-keychain": { - "version": "0.0.1", - "resolved": "https://registry.npmjs.org/@vscode/macos-keychain/-/macos-keychain-0.0.1.tgz", - "integrity": "sha512-8R5eKUZRoRUJvmoKgPrXFlEpBg6n8XKq0jyA85DLDuO1ZMbGuKsu2KsUCl7jWm06+h0ajZXUF0Z7dkk6j4IguA==", - "hasInstallScript": true, - "license": "MIT", - "optional": true, - "os": [ - "darwin" - ], - "dependencies": { - "bindings": "^1.5.0", - "node-addon-api": "^8.2.0" - } - }, - "node_modules/@vscode/macos-keychain/node_modules/node-addon-api": { - "version": "8.7.0", - "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-8.7.0.tgz", - "integrity": "sha512-9MdFxmkKaOYVTV+XVRG8ArDwwQ77XIgIPyKASB1k3JPq3M8fGQQQE3YpMOrKm6g//Ktx8ivZr8xo1Qmtqub+GA==", - "license": "MIT", - "optional": true, - "engines": { - "node": "^18 || ^20 || >= 21" - } - }, "node_modules/@vscode/native-watchdog": { "version": "1.4.6", "resolved": "https://registry.npmjs.org/@vscode/native-watchdog/-/native-watchdog-1.4.6.tgz", diff --git a/package.json b/package.json index 0ddf8d4a732..829ab071f3f 100644 --- a/package.json +++ b/package.json @@ -267,7 +267,6 @@ "url": "https://github.com/microsoft/vscode/issues" }, "optionalDependencies": { - "@vscode/macos-keychain": "^0.0.1", "windows-foreground-love": "0.6.1" } } diff --git a/product.json b/product.json index b8c702d3dbd..3b7b76b5ca3 100644 --- a/product.json +++ b/product.json @@ -26,7 +26,6 @@ "win32TunnelServiceMutex": "vscodeoss-tunnelservice", "win32TunnelMutex": "vscodeoss-tunnel", "darwinBundleIdentifier": "com.visualstudio.code.oss", - "darwinSharedKeychainServiceName": "com.visualstudio.code.oss.shared-secrets", "darwinProfileUUID": "47827DD9-4734-49A0-AF80-7E19B11495CC", "darwinProfilePayloadUUID": "CF808BE7-53F3-46C6-A7E2-7EDB98A5E959", "linuxIconName": "code-oss", diff --git a/src/typings/macos-keychain.d.ts b/src/typings/macos-keychain.d.ts deleted file mode 100644 index d10bdab322e..00000000000 --- a/src/typings/macos-keychain.d.ts +++ /dev/null @@ -1,15 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -// Type declarations for @vscode/macos-keychain. -// The package is an optional dependency (macOS-only native addon), so types -// are duplicated here to ensure TypeScript compilation succeeds on all platforms. - -declare module '@vscode/macos-keychain' { - export function keychainSet(service: string, account: string, value: string): void; - export function keychainGet(service: string, account: string): string | undefined; - export function keychainDelete(service: string, account: string): boolean; - export function keychainList(service: string): string[]; -} diff --git a/src/vs/base/common/product.ts b/src/vs/base/common/product.ts index 2e820ec314b..bc7c8f68eeb 100644 --- a/src/vs/base/common/product.ts +++ b/src/vs/base/common/product.ts @@ -224,7 +224,6 @@ export interface IProductConfiguration { readonly darwinUniversalAssetId?: string; readonly darwinBundleIdentifier?: string; readonly darwinSiblingBundleIdentifier?: string; - readonly darwinSharedKeychainServiceName?: string; readonly profileTemplatesUrl?: string; readonly commonlyUsedSettings?: string[]; diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index a2946dff2b5..bf4043983fd 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -37,8 +37,6 @@ import { DiagnosticsMainService, IDiagnosticsMainService } from '../../platform/ import { DialogMainService, IDialogMainService } from '../../platform/dialogs/electron-main/dialogMainService.js'; import { IEncryptionMainService } from '../../platform/encryption/common/encryptionService.js'; import { EncryptionMainService } from '../../platform/encryption/electron-main/encryptionMainService.js'; -import { ISharedKeychainMainService } from '../../platform/secrets/common/sharedKeychainService.js'; -import { SharedKeychainMainService } from '../../platform/secrets/electron-main/sharedKeychainMainService.js'; import { ipcBrowserViewChannelName } from '../../platform/browserView/common/browserView.js'; import { ipcBrowserViewGroupChannelName } from '../../platform/browserView/common/browserViewGroup.js'; import { BrowserViewMainService, IBrowserViewMainService } from '../../platform/browserView/electron-main/browserViewMainService.js'; @@ -1094,9 +1092,6 @@ export class CodeApplication extends Disposable { // Encryption services.set(IEncryptionMainService, new SyncDescriptor(EncryptionMainService)); - // Shared Keychain - services.set(ISharedKeychainMainService, new SyncDescriptor(SharedKeychainMainService)); - // Cross-app IPC services.set(ICrossAppIPCService, new SyncDescriptor(CrossAppIPCService)); @@ -1275,12 +1270,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, )); } @@ -1297,10 +1292,6 @@ export class CodeApplication extends Disposable { const encryptionChannel = ProxyChannel.fromService(accessor.get(IEncryptionMainService), disposables); mainProcessElectronServer.registerChannel('encryption', encryptionChannel); - // Shared Keychain - const sharedKeychainChannel = ProxyChannel.fromService(accessor.get(ISharedKeychainMainService), disposables); - mainProcessElectronServer.registerChannel('sharedKeychain', sharedKeychainChannel); - // Browser View const browserViewChannel = ProxyChannel.fromService(accessor.get(IBrowserViewMainService), disposables); mainProcessElectronServer.registerChannel(ipcBrowserViewChannelName, browserViewChannel); diff --git a/src/vs/platform/secrets/common/secrets.ts b/src/vs/platform/secrets/common/secrets.ts index 220442805fe..1840cdd6b28 100644 --- a/src/vs/platform/secrets/common/secrets.ts +++ b/src/vs/platform/secrets/common/secrets.ts @@ -76,6 +76,8 @@ export async function writeEncryptedSecret( /** * Secret keys that should be shared between the VS Code app and the agents app. + * When the agents app starts and doesn't have these secrets, it requests them + * from VS Code via crossAppIPC. */ export const CROSS_APP_SHARED_SECRET_KEYS: readonly string[] = [ '{"extensionId":"vscode.github-authentication","key":"github.auth"}', @@ -135,89 +137,65 @@ export class BaseSecretStorageService extends Disposable implements ISecretStora } get(key: string): Promise { - return this._sequencer.queue(key, () => this._doGet(key)); - } + return this._sequencer.queue(key, async () => { + const storageService = await this.resolvedStorageService; - /** - * Read from the safeStorage+SQLite pipeline without going through the sequencer. - * Must only be called from within a sequencer-queued task for the same key. - */ - protected async _doGet(key: string): Promise { - const storageService = await this.resolvedStorageService; - - try { - return await readEncryptedSecret( - key, - (fullKey) => this.getValueFromStorage(key, fullKey, storageService), - // If the storage service is in-memory, we don't need to decrypt - this._type === 'in-memory' ? (v) => Promise.resolve(v) : (v) => this._encryptionService.decrypt(v), - this._logService, - ); - } catch (e) { - this._logService.error(e); - this.delete(key); - return undefined; - } + try { + return await readEncryptedSecret( + key, + (fullKey) => this.getValueFromStorage(key, fullKey, storageService), + // If the storage service is in-memory, we don't need to decrypt + this._type === 'in-memory' ? (v) => Promise.resolve(v) : (v) => this._encryptionService.decrypt(v), + this._logService, + ); + } catch (e) { + this._logService.error(e); + this.delete(key); + return undefined; + } + }); } set(key: string, value: string): Promise { - return this._sequencer.queue(key, () => this._doSet(key, value)); - } + return this._sequencer.queue(key, async () => { + const storageService = await this.resolvedStorageService; - /** - * Write to the safeStorage+SQLite pipeline without going through the sequencer. - * Must only be called from within a sequencer-queued task for the same key. - */ - protected async _doSet(key: string, value: string): Promise { - const storageService = await this.resolvedStorageService; - - try { - await writeEncryptedSecret( - key, - value, - (fullKey, encrypted) => this.setValueInStorage(key, fullKey, encrypted, storageService), - // If the storage service is in-memory, we don't need to encrypt - this._type === 'in-memory' ? (v) => Promise.resolve(v) : (v) => this._encryptionService.encrypt(v), - this._logService, - ); - } catch (e) { - this._logService.error(e); - throw e; - } + try { + await writeEncryptedSecret( + key, + value, + (fullKey, encrypted) => this.setValueInStorage(key, fullKey, encrypted, storageService), + // If the storage service is in-memory, we don't need to encrypt + this._type === 'in-memory' ? (v) => Promise.resolve(v) : (v) => this._encryptionService.encrypt(v), + this._logService, + ); + } catch (e) { + this._logService.error(e); + throw e; + } + }); } delete(key: string): Promise { - return this._sequencer.queue(key, () => this._doDelete(key)); - } + return this._sequencer.queue(key, async () => { + const storageService = await this.resolvedStorageService; - /** - * Delete from the safeStorage+SQLite pipeline without going through the sequencer. - * Must only be called from within a sequencer-queued task for the same key. - */ - protected async _doDelete(key: string): Promise { - const storageService = await this.resolvedStorageService; - - const fullKey = secretStorageKey(key); - this._logService.trace('[secrets] deleting secret for key:', fullKey); - const scope = this.useSharedStorage(key) ? StorageScope.APPLICATION_SHARED : StorageScope.APPLICATION; - storageService.remove(fullKey, scope); - this._logService.trace('[secrets] deleted secret for key:', fullKey); + const fullKey = secretStorageKey(key); + this._logService.trace('[secrets] deleting secret for key:', fullKey); + const scope = this.useSharedStorage(key) ? StorageScope.APPLICATION_SHARED : StorageScope.APPLICATION; + storageService.remove(fullKey, scope); + this._logService.trace('[secrets] deleted secret for key:', fullKey); + }); } keys(): Promise { - return this._sequencer.queue('__keys__', () => this._doGetKeys()); - } - - /** - * List all secret keys from the safeStorage+SQLite pipeline without going through the sequencer. - * Must only be called from within a sequencer-queued task. - */ - protected async _doGetKeys(): Promise { - const storageService = await this.resolvedStorageService; - this._logService.trace('[secrets] fetching keys of all secrets'); - const allKeys = storageService.keys(StorageScope.APPLICATION, StorageTarget.MACHINE); - this._logService.trace('[secrets] fetched keys of all secrets'); - return allKeys.filter(key => key.startsWith(SECRET_STORAGE_PREFIX)).map(key => key.slice(SECRET_STORAGE_PREFIX.length)); + return this._sequencer.queue('__keys__', async () => { + const storageService = await this.resolvedStorageService; + this._logService.trace('[secrets] fetching keys of all secrets'); + const allKeys = storageService.keys(StorageScope.APPLICATION, StorageTarget.MACHINE); + this._logService.trace('[secrets] fetched keys of all secrets'); + return allKeys.filter(key => key.startsWith(SECRET_STORAGE_PREFIX)).map(key => key.slice(SECRET_STORAGE_PREFIX.length)); + }); } private getValueFromStorage(key: string, fullKey: string, storageService: IStorageService): string | undefined { diff --git a/src/vs/platform/secrets/common/sharedKeychainService.ts b/src/vs/platform/secrets/common/sharedKeychainService.ts deleted file mode 100644 index f99485e7900..00000000000 --- a/src/vs/platform/secrets/common/sharedKeychainService.ts +++ /dev/null @@ -1,25 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { createDecorator } from '../../instantiation/common/instantiation.js'; - -/** - * Provides shared keychain access between Code and the embedded Agents app - * via a macOS keychain access group. On non-macOS platforms the implementation - * is a no-op (returns undefined/empty for all operations). - */ -export const ISharedKeychainService = createDecorator('sharedKeychainService'); - -export interface ISharedKeychainService { - readonly _serviceBrand: undefined; - get(key: string): Promise; - set(key: string, value: string): Promise; - delete(key: string): Promise; - keys(): Promise; -} - -export const ISharedKeychainMainService = createDecorator('sharedKeychainMainService'); - -export interface ISharedKeychainMainService extends ISharedKeychainService { } diff --git a/src/vs/platform/secrets/electron-main/macOSCrossAppSecretSharing.ts b/src/vs/platform/secrets/electron-main/macOSCrossAppSecretSharing.ts index c95e639f072..6dfea9bc15a 100644 --- a/src/vs/platform/secrets/electron-main/macOSCrossAppSecretSharing.ts +++ b/src/vs/platform/secrets/electron-main/macOSCrossAppSecretSharing.ts @@ -5,49 +5,75 @@ import { execFile } from 'child_process'; import { dirname } from '../../../base/common/path.js'; -import { Disposable } from '../../../base/common/lifecycle.js'; +import { Disposable, DisposableStore } 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, readEncryptedSecret } from '../common/secrets.js'; +import { CROSS_APP_SHARED_SECRET_KEYS, secretStorageKey, readEncryptedSecret, writeEncryptedSecret } 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 { ISharedKeychainMainService } from '../common/sharedKeychainService.js'; +import { ICrossAppIPCService } from '../../crossAppIpc/electron-main/crossAppIpcService.js'; -const MIGRATION_STATE_KEY = 'sharedKeychain.migrationDone'; -const HOST_SPAWN_STATE_KEY = 'sharedKeychain.hostSpawnDone'; +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; +} /** * Coordinates one-time secret migration between the VS Code app and the - * agents app via the macOS shared keychain (macOS only). + * agents app using Electron's crossAppIPC (macOS only). * - * 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. + * **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. * - * After migration, both apps read from and write to the shared keychain - * for cross-app secret keys (via {@link NativeSecretStorageService}). + * 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. */ 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; @@ -61,96 +87,143 @@ export class MacOSCrossAppSecretSharing extends Disposable { lifecycleMainService: ILifecycleMainService, ): void { if (this.isEmbeddedApp) { - // Agents app: migrate own secrets + spawn Code.app if needed + // Agents app: initiate migration if needed this.initializeAsAgentsApp(); } else if (environmentMainService.args['share-secrets-with-agents-app']) { - // Code.app launched with --share-secrets-with-agents-app: - // migrate secrets to shared keychain, then quit if no other reason to stay + // 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 const hasOtherArgs = environmentMainService.args._.length > 0 || environmentMainService.args['folder-uri'] || environmentMainService.args['file-uri']; - this.migrateSecrets().then(() => { - if (!hasOtherArgs) { - this.logService.info('[CrossAppSecretSharing] Host app was launched for migration only, quitting'); - lifecycleMainService.quit(); - } + this.initializeAsHostApp(hasOtherArgs ? undefined : () => { + this.logService.info('[CrossAppSecretSharing] Host app was launched for migration only, quitting'); + lifecycleMainService.quit(); }); } else { - // Code.app normal startup: migrate own secrets - this.migrateSecrets(); - // Also respond to spawn requests from the agents app + // Code.app already running: listen for --share-secrets-with-agents-app + // forwarded from a second instance via the launch service this._register(launchMainService.onDidRequestShareSecrets(() => { - this.migrateSecrets(); + this.initializeAsHostApp(); })); } } private async initializeAsAgentsApp(): Promise { - if (!isMacintosh) { + if (!isMacintosh || !this.isEmbeddedApp) { return; } - // 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; - } - - // Check if the shared keychain has all expected keys - let needsHostMigration = false; - for (const key of CROSS_APP_SHARED_SECRET_KEYS) { - if (await this.sharedKeychainMainService.get(key) === undefined) { - needsHostMigration = true; - break; - } - } - - 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)) { + if (this.isMigrationDone()) { this.logService.trace('[CrossAppSecretSharing] Migration already done, skipping'); return; } + // Wait for storage to be ready before we start — handleSecretResponse + // will write secrets into applicationStorage. 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); - } + if (!this.crossAppIPCService.initialized) { + this.logService.info('[CrossAppSecretSharing] crossAppIPC not initialized, skipping migration'); + return; } - this.stateService.setItem(MIGRATION_STATE_KEY, true); - this.logService.info('[CrossAppSecretSharing] Migration complete'); + 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 { + for (const key of CROSS_APP_SHARED_SECRET_KEYS) { + if (this.applicationStorage.get(secretStorageKey(key)) !== undefined) { + return true; + } + } + return false; } private spawnHostApp(): void { @@ -174,4 +247,69 @@ 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/platform/secrets/electron-main/sharedKeychainMainService.ts b/src/vs/platform/secrets/electron-main/sharedKeychainMainService.ts deleted file mode 100644 index 3ebeb172d97..00000000000 --- a/src/vs/platform/secrets/electron-main/sharedKeychainMainService.ts +++ /dev/null @@ -1,92 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { isMacintosh } from '../../../base/common/platform.js'; -import { ISharedKeychainMainService } from '../common/sharedKeychainService.js'; -import { ILogService } from '../../log/common/log.js'; -import { IProductService } from '../../product/common/productService.js'; - -type KeychainModule = typeof import('@vscode/macos-keychain'); - -export class SharedKeychainMainService implements ISharedKeychainMainService { - declare readonly _serviceBrand: undefined; - - private _modulePromise: Promise | undefined; - private readonly serviceName: string; - private readonly enabled: boolean; - - constructor( - @IProductService productService: IProductService, - @ILogService private readonly logService: ILogService, - ) { - this.enabled = isMacintosh && !!productService.darwinSharedKeychainServiceName; - this.serviceName = productService.darwinSharedKeychainServiceName ?? ''; - } - - private getModule(): Promise { - if (!this._modulePromise) { - this._modulePromise = import('@vscode/macos-keychain'); - } - return this._modulePromise; - } - - async get(key: string): Promise { - if (!this.enabled) { - return undefined; - } - try { - const mod = await this.getModule(); - const value = mod.keychainGet(this.serviceName, key); - this.logService.trace('[SharedKeychainMainService] get:', key, value !== undefined ? '(found)' : '(not found)'); - return value; - } catch (err) { - this.logService.error('[SharedKeychainMainService] get failed:', key, err); - return undefined; - } - } - - async set(key: string, value: string): Promise { - if (!this.enabled) { - return; - } - try { - const mod = await this.getModule(); - mod.keychainSet(this.serviceName, key, value); - this.logService.trace('[SharedKeychainMainService] set:', key); - } catch (err) { - this.logService.error('[SharedKeychainMainService] set failed:', key, err); - } - } - - async delete(key: string): Promise { - if (!this.enabled) { - return false; - } - try { - const mod = await this.getModule(); - const deleted = mod.keychainDelete(this.serviceName, key); - this.logService.trace('[SharedKeychainMainService] delete:', key, deleted ? '(deleted)' : '(not found)'); - return deleted; - } catch (err) { - this.logService.error('[SharedKeychainMainService] delete failed:', key, err); - return false; - } - } - - async keys(): Promise { - if (!this.enabled) { - return []; - } - try { - const mod = await this.getModule(); - const result = mod.keychainList(this.serviceName); - this.logService.trace('[SharedKeychainMainService] keys: found', result.length, 'entries'); - return result; - } catch (err) { - this.logService.error('[SharedKeychainMainService] keys failed:', err); - return []; - } - } -} diff --git a/src/vs/sessions/sessions.desktop.main.ts b/src/vs/sessions/sessions.desktop.main.ts index 49e3d3f763a..24bb3646edb 100644 --- a/src/vs/sessions/sessions.desktop.main.ts +++ b/src/vs/sessions/sessions.desktop.main.ts @@ -58,7 +58,6 @@ import '../workbench/services/mcp/electron-browser/mcpWorkbenchManagementService import '../workbench/services/encryption/electron-browser/encryptionService.js'; import '../workbench/services/imageResize/electron-browser/imageResizeService.js'; import '../workbench/services/secrets/electron-browser/secretStorageService.js'; -import '../workbench/services/secrets/electron-browser/sharedKeychainService.js'; import '../workbench/services/localization/electron-browser/languagePackService.js'; import '../workbench/services/telemetry/electron-browser/telemetryService.js'; import '../workbench/services/extensions/electron-browser/extensionHostStarter.js'; diff --git a/src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts b/src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts index 8763ad91df8..228328d59bb 100644 --- a/src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts +++ b/src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { createSingleCallFunction } from '../../../../base/common/functional.js'; -import { isLinux, isMacintosh } from '../../../../base/common/platform.js'; +import { isLinux } from '../../../../base/common/platform.js'; import Severity from '../../../../base/common/severity.js'; import { localize } from '../../../../nls.js'; import { IDialogService } from '../../../../platform/dialogs/common/dialogs.js'; @@ -14,8 +14,7 @@ import { InstantiationType, registerSingleton } from '../../../../platform/insta import { ILogService } from '../../../../platform/log/common/log.js'; import { INotificationService, IPromptChoice } from '../../../../platform/notification/common/notification.js'; 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 { BaseSecretStorageService, ISecretStorageService } from '../../../../platform/secrets/common/secrets.js'; import { IStorageService } from '../../../../platform/storage/common/storage.js'; import { IJSONEditingService } from '../../configuration/common/jsonEditing.js'; @@ -27,7 +26,6 @@ export class NativeSecretStorageService extends BaseSecretStorageService { @IOpenerService private readonly _openerService: IOpenerService, @IJSONEditingService private readonly _jsonEditingService: IJSONEditingService, @INativeEnvironmentService private readonly _environmentService: INativeEnvironmentService, - @ISharedKeychainService private readonly _sharedKeychainService: ISharedKeychainService, @IStorageService storageService: IStorageService, @IEncryptionService encryptionService: IEncryptionService, @ILogService logService: ILogService @@ -40,20 +38,6 @@ export class NativeSecretStorageService extends BaseSecretStorageService { ); } - override get(key: string): Promise { - return this._sequencer.queue(key, async () => { - if (isMacintosh && this.type !== 'in-memory' && CROSS_APP_SHARED_SECRET_KEYS.includes(key)) { - // Try shared keychain first - const value = await this._sharedKeychainService.get(key); - if (value !== undefined) { - return value; - } - } - // Fall back to old safeStorage+SQLite pipeline - return this._doGet(key); - }); - } - override set(key: string, value: string): Promise { this._sequencer.queue(key, async () => { await this.resolvedStorageService; @@ -62,42 +46,10 @@ export class NativeSecretStorageService extends BaseSecretStorageService { this._logService.trace('[NativeSecretStorageService] Notifying user that secrets are not being stored on disk.'); await this.notifyOfNoEncryptionOnce(); } - }); - return this._sequencer.queue(key, async () => { - if (isMacintosh && this.type !== 'in-memory' && CROSS_APP_SHARED_SECRET_KEYS.includes(key)) { - // Write to shared keychain - await this._sharedKeychainService.set(key, value); - } - // Also write to legacy pipeline - await this._doSet(key, value); - }); - } - override delete(key: string): Promise { - return this._sequencer.queue(key, async () => { - if (isMacintosh && this.type !== 'in-memory' && CROSS_APP_SHARED_SECRET_KEYS.includes(key)) { - // Delete from shared keychain - await this._sharedKeychainService.delete(key); - } - // Delete from legacy pipeline - await this._doDelete(key); }); - } - override async keys(): Promise { - return this._sequencer.queue('__keys__', async () => { - const legacyKeys = await this._doGetKeys(); - if (isMacintosh && this.type !== 'in-memory') { - // 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); - if (sharedValue !== undefined && !legacyKeys.includes(sharedKey)) { - legacyKeys.push(sharedKey); - } - } - } - return legacyKeys; - }); + return super.set(key, value); } private notifyOfNoEncryptionOnce = createSingleCallFunction(() => this.notifyOfNoEncryption()); diff --git a/src/vs/workbench/services/secrets/electron-browser/sharedKeychainService.ts b/src/vs/workbench/services/secrets/electron-browser/sharedKeychainService.ts deleted file mode 100644 index 7930df4d693..00000000000 --- a/src/vs/workbench/services/secrets/electron-browser/sharedKeychainService.ts +++ /dev/null @@ -1,9 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { registerMainProcessRemoteService } from '../../../../platform/ipc/electron-browser/services.js'; -import { ISharedKeychainService } from '../../../../platform/secrets/common/sharedKeychainService.js'; - -registerMainProcessRemoteService(ISharedKeychainService, 'sharedKeychain'); diff --git a/src/vs/workbench/workbench.desktop.main.ts b/src/vs/workbench/workbench.desktop.main.ts index bbf06856477..4277093b103 100644 --- a/src/vs/workbench/workbench.desktop.main.ts +++ b/src/vs/workbench/workbench.desktop.main.ts @@ -59,7 +59,6 @@ import './services/mcp/electron-browser/mcpWorkbenchManagementService.js'; import './services/encryption/electron-browser/encryptionService.js'; import './services/imageResize/electron-browser/imageResizeService.js'; import './services/secrets/electron-browser/secretStorageService.js'; -import './services/secrets/electron-browser/sharedKeychainService.js'; import './services/localization/electron-browser/languagePackService.js'; import './services/telemetry/electron-browser/telemetryService.js'; import './services/extensions/electron-browser/extensionHostStarter.js';