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 👍 / 👎.
Not sure why I assumed this would render.
This can happen if you run the update multiple times and it was already deleted from another run but not regenerated yet due to an early failure like from a conflict.
The client combines the key from this endpoint to then encrypt secrets into browser storage.
We use the cookie parser to remove the code-server token but by default it encodes using encodeURIComponent, which encodes more than is strictly necessary and can break proxied applications. Now we pass the cookies through unchanged (other than removing the code-server token). Fixes coder#7927.
With the update to node.js v24 in vscode and package.json some files have not been updated and still referenced version 22.x.
npm 12 rejects unknown CLI flags, so the npm install path crashes with EUNKNOWNCONFIG before installing. The flag has been a no-op since npm 7; remove it from install.sh and the postinstall guard.
One was updated by Dependabot, but not the others.
Installing on the wrong node version dies with "FORCE_NODE_VERSION: unbound variable" instead of explaining itself. The script runs under set -u and this is the one place the variable is read bare; the three other reads all default it.
npm install fails on Windows in postinstall with "mklink: command not found". mklink is a cmd.exe builtin rather than an executable, so the sh running this script cannot exec it, and every symlink the install needs is made this way. The doubled slashes are the usual MSYS escaping, and are safe here because this branch only runs when the OS was detected as cygwin or mingw.
…on-storage-service
11 participants
Fixes #