sourcegraph / cody

AI that knows your entire codebase
https://cody.dev
Apache License 2.0
2.26k stars 215 forks source link

CLI: replace keytar with custom solution #4750

Closed olafurpg closed 1 day ago

olafurpg commented 3 days ago

Previously, we used Keytar to write/read/delete secrets. This was problematic for two reasons:

This PR addresses both problems by replacing keytar with a custom solution that shells out to different secret managers (security on macOS, secret-tool on Linux, and powershell on Windows). See comment in secrets.ts for a more detailed reasoning why we went with this approach.

Test plan

Manually tested on macOS/Linux/Windows.

olafurpg commented 2 days ago

TODO: report helpful error messages when missing dependencies

Linux:

sudo apt install libsecret-tools
sudo apt install gnome-keyring

Windows with Admin Powershell

Install-Module -Name CredentialManager
github-actions[bot] commented 1 day ago

‼️ Hey @sourcegraph/cody-security, please review this PR carefully as it introduces the usage of an unsafe_ function or abuses PromptString.

olafurpg commented 1 day ago

Thank you for the review! I've addressed the feedback on shell quoting, the solution is now using spawn instead of exec. I will submit a request in #discuss-security to additionally review this functionality after the release is out (we won't make public noise about this feature until after security review is done)

olafurpg commented 1 day ago

CI failures are unrelated, posted about it here https://sourcegraph.slack.com/archives/C05AGQYD528/p1720014583901319

olafurpg commented 1 day ago

Rebase on top of main to fix CI https://github.com/sourcegraph/cody/pull/4768

olafurpg commented 1 day ago

Thank you everybody for the review! 🙇🏻 I will cut a release and get a second round of review from the security team CODY-2738 : Complete security review on Cody cli auth secret management