Audit changes - #2
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7bc869cac4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| + if (!this._useInMemoryStorage) { | ||
| + // Use persisted storage when not forced to use in-memory | ||
| + this._logService.trace(`[SecretStorageService] Using persisted storage`); | ||
| this._type = 'persisted'; | ||
| storageService = this._storageService; |
There was a problem hiding this comment.
When --enable-remote-storage is used in the browser, IEncryptionService.isEncryptionAvailable() is normally false, but this branch now selects persisted storage anyway; combined with the new encrypt/decrypt callbacks that return the value unchanged when encryption is unavailable, extension secrets such as GitHub or Copilot tokens are written as cleartext secret:// entries into the remote/browser storage database. The previous guard kept secrets in memory unless encryption was available, so this should either keep the in-memory fallback or route secrets through a server-side encrypted secret provider.
Useful? React with 👍 / 👎.
| + protected getStorage(scope: StorageScope): IStorage | undefined { | ||
| + switch (scope) { | ||
| + case StorageScope.APPLICATION: | ||
| + return this.applicationStorage; | ||
| + case StorageScope.PROFILE: | ||
| + return this.profileStorage; | ||
| + default: | ||
| + return this.workspaceStorage; |
There was a problem hiding this comment.
With remote storage enabled, callers using StorageScope.APPLICATION_SHARED fall through this switch to the default case and read/write the workspace database. The existing browser storage service creates a separate application-shared store because those keys must be shared across apps/workspaces, so routing them to workspace storage loses shared state and can mix cross-app secrets or mementos with per-workspace state; the remote service should create a remote application-shared storage client and return it explicitly for that scope.
Useful? React with 👍 / 👎.
1 participant
Fixes #