Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Positrons pops up "code-oss-dev Safe Storage" dialog when launched from within VSCode #2274

Open
wesm opened this issue Feb 16, 2024 · 14 comments
Labels
area: builds Issues related to Builds category.

Comments

@wesm
Copy link
Contributor

wesm commented Feb 16, 2024

This comes up frequently when launching Positron from within VSCode

image
@juliasilge
Copy link
Contributor

This is the same as #1688

I think we have confirmed this only occurs for dev builds (does not impact users) and haven't been able to really consistently reproduce this.

@wesm do you have a pretty reliable set of actions that causes you to see this?

@wesm
Copy link
Contributor Author

wesm commented Feb 16, 2024

Yeah, if I close Positron, and then relaunch it from VSCode, I get it every time

@wesm
Copy link
Contributor Author

wesm commented Feb 16, 2024

Screen.Recording.2024-02-15.at.6.31.14.PM.mov

@wesm wesm added this to the Public Beta 2024 Q2 milestone Feb 20, 2024
@wesm
Copy link
Contributor Author

wesm commented Feb 21, 2024

This pop up is originating from this file:

https://github.com/microsoft/vscode/blob/main/src/vs/platform/encryption/electron-main/encryptionMainService.ts

the exact line that causes the pop-up to show up is

https://github.com/microsoft/vscode/blob/main/src/vs/platform/encryption/electron-main/encryptionMainService.ts#L64

	isEncryptionAvailable(): Promise<boolean> {
		return Promise.resolve(safeStorage.isEncryptionAvailable());
	}

See https://www.electronjs.org/docs/latest/api/safe-storage

On my computer, I have a bunch of Electron apps that seem to use this API:

image

If this quirk seems to only apply to running Positron in development mode, maybe we can just fix the "code-oss-dev" piece (somehow?) to say "Positron" instead and recommend that people authorize the keychain access?

See also microsoft/vscode-discussions#748

@wesm wesm added the area: builds Issues related to Builds category. label Feb 29, 2024
@juliasilge juliasilge modified the milestones: Public Beta 2024 Q2, Future Apr 1, 2024
@sharon-wang
Copy link
Member

Release build instance of this issue: #5485

@jonvanausdeln
Copy link
Contributor

Moving back into triage as we have at least 2 instances of this this in a release build.

@juliasilge
Copy link
Contributor

This may possibly be related to recent work from the Connections pane that uses the secret storage.

@dfalbel
Copy link
Contributor

dfalbel commented Dec 16, 2024

I investigated this a little and it doesn't seem to be related to the secret storage usage in the connections pane (implemented in #5456) as even after removing all the refrences to the Secret storage in that files we still get the modals.

@juliasilge
Copy link
Contributor

Thanks, @dfalbel! I'll move this back to triage for us to decide how to proceed.

Reported again in #5769

@jmcphers
Copy link
Collaborator

jmcphers commented Jan 7, 2025

I did some debugging here and discovered that it is in fact the Connections code that is triggering the dialog. Specifically this line:

const storedConnections: IConnectionMetadata[] = JSON.parse(
await this.secretStorageService.get(id) || '[]'
);

It is also true that removing this line doesn't make the dialog go away though. It just makes it appear later, because Quarto is also hitting the secret storage service later during startup.

https://github.com/quarto-dev/quarto/blob/57ed6fb2ab6066a6a4f0333c0c968586701e006a/apps/vscode/src/providers/zotero/zotero.ts#L362-L369

We need to deal with both of these usages if we want this prompt to go away.

Possible paths forward:

  • Modify both the Connections pane and the Quarto extension so that they remember whether or not there is anything in the secret storage, and only hit the storage key if there's actually something there. People who use a Zotero API key or stored connections will of course still see the prompt.
  • Modify the Connections pane as above, but change Quarto so that it doesn't poll for a Zotero key at all at startup (delay until it is needed)
  • Investigate whether there is some way we can reduce the amount of prompting that our use of the keychain is creating, probably by modifying the upstream code
  • Use a secret storage mechanism that doesn't hit the OS keychain at all (and therefore won't kick up OS security dialogs), e.g. rolling our own encrypted storage
  • Run away from home and live in the woods

@jmcphers
Copy link
Collaborator

jmcphers commented Jan 7, 2025

@dfalbel I think the right first step for now is to make sure the Connections pane only hits the secret storage service when we are sure there are connections to retrieve (we will have to remember that out of band) to prevent spurious prompts.

That will help a lot because (as I read it) the Quarto extension doesn't activate eagerly unless you are using R, so it should solve the prompt-on-boot issue entirely for Python-only folks.

We can follow up with an improvement to the Quarto extension that makes the polling for Zotero web API keys less aggressive.

@JosiahParry
Copy link

JosiahParry commented Jan 10, 2025

This has started happening to me since installing 2025.01.0 build 152. I believe my previous installation was from september or october 2024

edit: this is still occurring in the latest version i've installed

Positron Version: 2025.02.0 (Universal) build 79
Code - OSS Version: 1.96.0
Commit: a268bc8
Date: 2025-01-21T02:44:54.581Z
Electron: 32.2.6
Chromium: 128.0.6613.186
Node.js: 20.18.1
V8: 12.8.374.38-electron.0
OS: Darwin arm64 24.0.0

@jmcphers
Copy link
Collaborator

I filed an issue here for the Quarto extension: quarto-dev/quarto#659

@dfalbel dfalbel removed their assignment Feb 17, 2025
@juliasilge
Copy link
Contributor

We believe we have made the changes needed in Positron itself to reduce these prompts to access to safe storage. I'm moving this to verification but note that you will need to uninstall Quarto and maybe Jupyter extensions as noted in #6266.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: builds Issues related to Builds category.
Projects
None yet
Development

No branches or pull requests

7 participants