Share secrets between Code and Agents app via macOS Keychain#308990
Share secrets between Code and Agents app via macOS Keychain#308990
Conversation
Add a shared keychain service that stores secrets directly in the macOS Keychain, allowing Code and its embedded Agents app to share auth tokens without re-authentication. Architecture: - ISharedKeychainService (common interface) with ISharedKeychainMainService running in the Electron main process, exposed to renderer via IPC - SharedKeychainMainService wraps @vscode/macos-keychain native addon - NativeSecretStorageService now writes to both the shared keychain and the legacy safeStorage+SQLite pipeline (for rollback safety) - On read, shared keychain is tried first, falling back to legacy Product configuration: - darwinSharedKeychainServiceName: per-flavor service name for data isolation between Stable/Insiders/Exploration - Access group auto-detected from entitlements by the native addon Key design decisions: - Shared keychain only used when type is 'persisted' (not in-memory) - BaseSecretStorageService refactored to expose protected _doGet/_doSet/ _doDelete/_doGetKeys for use by subclasses within sequencer tasks - Native addon is an optional dependency (macOS-only) Files: - build/azure-pipelines/darwin/app-entitlements.plist (keychain-access-groups) - src/vs/platform/secrets/common/sharedKeychainService.ts (interface) - src/vs/platform/secrets/electron-main/sharedKeychainMainService.ts (impl) - src/vs/workbench/services/secrets/electron-browser/sharedKeychainService.ts (IPC proxy) - src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts (wiring) Issue: #308028
There was a problem hiding this comment.
Pull request overview
Adds a macOS shared-keychain-backed secrets path so VS Code and the embedded Agents app can reuse authentication without re-prompting, while retaining the existing safeStorage+SQLite pipeline for fallback/rollback.
Changes:
- Introduces
ISharedKeychainService+ Electron main-process implementation (SharedKeychainMainService) backed by optional@vscode/macos-keychain, exposed to renderers via IPC. - Refactors
BaseSecretStorageServiceto expose protected_doGet/_doSet/_doDelete/_doGetKeyshelpers and updatesNativeSecretStorageServiceto read/write shared keychain first (persisted only) with legacy fallback. - Wires product/packaging: new
darwinSharedKeychainServiceName, macOS entitlements update, optional dependency + module packaging rules, and adds design/impl notes docs.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/workbench.desktop.main.ts | Loads shared keychain renderer-side IPC service. |
| src/vs/workbench/services/secrets/electron-browser/sharedKeychainService.ts | Registers shared keychain main-process remote service in the renderer. |
| src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts | Uses shared keychain (persisted) with legacy fallback; now dual-writes. |
| src/vs/sessions/sessions.desktop.main.ts | Loads shared keychain renderer-side IPC service for Agents window. |
| src/vs/platform/secrets/electron-main/sharedKeychainMainService.ts | Main-process service wrapping @vscode/macos-keychain. |
| src/vs/platform/secrets/common/sharedKeychainService.ts | New shared keychain service interfaces + decorators. |
| src/vs/platform/secrets/common/secrets.ts | Refactors BaseSecretStorageService to expose _do* helpers. |
| src/vs/code/electron-main/app.ts | Registers main-process shared keychain service + IPC channel. |
| src/vs/base/common/product.ts | Adds darwinSharedKeychainServiceName to product configuration typing. |
| src/typings/macos-keychain.d.ts | Provides TS typings for optional native addon. |
| product.json | Defines default shared keychain service name for macOS. |
| package.json / package-lock.json | Adds optional dependency on @vscode/macos-keychain. |
| build/azure-pipelines/darwin/app-entitlements.plist | Adds keychain-access-groups entitlement. |
| build/.moduleignore | Ensures the native .node binary is included in packaging. |
| secrets-sharing-spec.md / secrets-sharing-impl.md | Adds spec + implementation notes. |
Copilot's findings
Comments suppressed due to low confidence (6)
src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts:46
- Remove the
debugger;statement before shipping (it will trip linting and can pause execution in production builds).
override get(key: string): Promise<string | undefined> {
return this._sequencer.queue(key, async () => {
debugger;
if (this.type === 'persisted') {
src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts:70
- Remove the
debugger;statement before shipping (it will trip linting and can pause execution in production builds).
return this._sequencer.queue(key, async () => {
debugger;
if (this.type === 'persisted') {
// Write to shared keychain (no-op on non-macOS)
src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts:81
- Remove the
debugger;statement before shipping (it will trip linting and can pause execution in production builds).
override delete(key: string): Promise<void> {
return this._sequencer.queue(key, async () => {
debugger;
if (this.type === 'persisted') {
src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts:93
- Remove the
debugger;statement before shipping (it will trip linting and can pause execution in production builds).
override keys(): Promise<string[]> {
return this._sequencer.queue('__keys__', async () => {
debugger;
if (this.type === 'persisted') {
src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts:63
- This
_sequencer.queue(...)call is not returned/awaited. If the queued task throws/rejects (e.g.resolvedStorageServicefails or notification code errors), it can become an unhandled rejection and the caller won’t observe the failure. Consider folding the notification logic into the returned queue callback (singlequeuefor the key) or at least return/handle the promise.
this._sequencer.queue(key, async () => {
await this.resolvedStorageService;
if (this.type !== 'persisted' && !this._environmentService.useInMemorySecretStorage) {
this._logService.trace('[NativeSecretStorageService] Notifying user that secrets are not being stored on disk.');
src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts:74
- If
_sharedKeychainService.set(...)rejects (e.g. missing entitlements/addon load failure), the wholeset()rejects and_doSet(...)won’t run, which defeats the “write to legacy for rollback safety” goal and could break auth token persistence. Make shared-keychain writes best-effort here (catch/log and continue) so the legacy pipeline is still updated.
if (this.type === 'persisted') {
// Write to shared keychain (no-op on non-macOS)
await this._sharedKeychainService.set(key, value);
}
// Also write to legacy pipeline
await this._doSet(key, value);
- Files reviewed: 15/17 changed files
- Comments generated: 3
src/vs/workbench/services/secrets/electron-browser/secretStorageService.ts
Show resolved
Hide resolved
src/vs/platform/secrets/electron-main/sharedKeychainMainService.ts
Outdated
Show resolved
Hide resolved
| "url": "https://github.com/microsoft/vscode/issues" | ||
| }, | ||
| "optionalDependencies": { | ||
| "@vscode/macos-keychain": "microsoft/vscode-macos-keychain#85ade16af6da2125eb1d31241002a8a2e5ced710", |
There was a problem hiding this comment.
Using the GitHub shorthand microsoft/vscode-macos-keychain#... causes npm to resolve via git+ssh://git@github.com/... (see package-lock), which typically fails in CI/environments without GitHub SSH keys. Prefer an explicit git+https://... URL or (ideally) a published npm version to ensure reproducible installs.
| "@vscode/macos-keychain": "microsoft/vscode-macos-keychain#85ade16af6da2125eb1d31241002a8a2e5ced710", | |
| "@vscode/macos-keychain": "git+https://github.com/microsoft/vscode-macos-keychain.git#85ade16af6da2125eb1d31241002a8a2e5ced710", |
On first secret operation, migrate all existing secrets from the legacy safeStorage+SQLite pipeline into the shared macOS Keychain. This ensures the Agents app can read secrets that were stored before the shared keychain was introduced. - Migration is lazy (triggered on first get/set/delete/keys) - Guarded by a 'sharedKeychain.migrationDone' storage flag - Idempotent: keychain writes are upserts, re-running is safe - Best-effort per key: individual failures don't block the rest - Skipped when type is 'in-memory' - Also: make set() in SharedKeychainMainService best-effort (log, don't throw)
Fixes #308028
Add a shared keychain service that stores secrets directly in the macOS Keychain, allowing Code and its embedded Agents app to share auth tokens without re-authentication.
Architecture
ISharedKeychainService(common interface) withISharedKeychainMainServicerunning in the Electron main process, exposed to renderer via IPC (registerMainProcessRemoteService)SharedKeychainMainServicewraps the@vscode/macos-keychainnative addonNativeSecretStorageServicenow writes to both the shared keychain and the legacy safeStorage+SQLite pipeline (for rollback safety)type === 'persisted'(not in-memory)Product configuration
darwinSharedKeychainServiceName— per-flavor keychain service name for data isolation between Stable/Insiders/Exploration (set in product.json)keychain-access-groupsentitlement by the native addon at module load time (viaSecTaskCopyValueForEntitlement)Base class refactoring
BaseSecretStorageServicenow exposes protected_doGet/_doSet/_doDelete/_doGetKeysmethods that perform the actual safeStorage+SQLite operations without going through the sequencer, so subclasses can call them from within their own sequencer-queued tasks without deadlocking.Native addon
@vscode/macos-keychain— see https://github.com/microsoft/vscode-macos-keychainTODO
debuggerstatements