Conversation
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @TylerLeonhardtMatched files:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the secrets storage layer to enable cross-app shared secrets on Windows, allowing specific secrets to be stored in StorageScope.APPLICATION_SHARED so they can be used for SSO between VS Code and another app (e.g., Sessions/Agents).
Changes:
- Route reads/writes of a small allowlist of secret keys to
StorageScope.APPLICATION_SHAREDon Windows. - Add helper methods to centralize storage get/store logic for shared vs non-shared secrets.
- Listen for secret changes from both
APPLICATIONandAPPLICATION_SHAREDstorage scopes.
Show a summary per file
| File | Description |
|---|---|
src/vs/platform/secrets/common/secrets.ts |
Adds Windows-only cross-app shared secret routing via APPLICATION_SHARED and wires change events for both scopes. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/vs/platform/secrets/common/secrets.ts:210
- For keys written to
StorageScope.APPLICATION_SHARED,delete()andkeys()still operate only onStorageScope.APPLICATION, which will leave shared secrets undeleted/unlisted. On Windows this can prevent extensions (e.g. GitHub auth) from actually deleting the secret, and due to APPLICATION_SHARED fallback-to-APPLICATION behavior it may require removing from both scopes to guarantee the secret is gone.
private setValueInStorage(key: string, fullKey: string, value: string, storageService: IStorageService): void {
if (isWindows && CROSS_APP_SHARED_SECRET_KEYS.includes(key)) {
this._logService.trace(`[SecretStorageService] Setting value for cross-app shared secret: ${fullKey}`);
storageService.store(fullKey, value, StorageScope.APPLICATION_SHARED, StorageTarget.MACHINE);
return;
}
storageService.store(fullKey, value, StorageScope.APPLICATION, StorageTarget.MACHINE);
- Files reviewed: 1/1 changed files
- Comments generated: 1
| private getValueFromStorage(key: string, fullKey: string, storageService: IStorageService): string | undefined { | ||
| if (isWindows && CROSS_APP_SHARED_SECRET_KEYS.includes(key)) { | ||
| this._logService.trace(`[SecretStorageService] Fetching value for cross-app shared secret: ${fullKey}`); | ||
| return storageService.get(fullKey, StorageScope.APPLICATION_SHARED); | ||
| } | ||
| return storageService.get(fullKey, StorageScope.APPLICATION); | ||
| } |
There was a problem hiding this comment.
The new cross-app secret routing to StorageScope.APPLICATION_SHARED (plus change notifications wired up for that scope) isn’t covered by the existing BaseSecretStorageService unit tests. Consider adding tests that (on Windows, or conditionally when isWindows is true) verify: (1) shared keys are stored/read from APPLICATION_SHARED, (2) onDidChangeSecret fires for APPLICATION_SHARED changes, and (3) deleting a shared key removes it so subsequent reads return undefined (including when a fallback value exists in APPLICATION).
This issue also appears on line 204 of the same file.
No description provided.