-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Switch to native bindings for macOS Keychain #7123
Comments
Switch from github.com/zalando/go-keyring to github.com/99designs/keyring that uses native bindings on macOS and Windows. Originally suggested in cli#7023 (comment) Fixes cli#7123
@YorikSar You are correct in that the prerequisites for this work are now in place. The team is unsure if now is the right time to take on this work as it would be a breaking change and likely require all our users to re-authenticate. |
@samcoe Thank you for your reply. I will add code that will convert go-keyring format (prefixed base64-encoded string) to the "new" format (plain data). That would allow to reuse existing token. After that all that user will have to do is authorise |
Switch from github.com/zalando/go-keyring to github.com/99designs/keyring that uses native bindings on macOS and Windows. On darwin, github.com/zalando/go-keyring encodes secret data in hex or base64, adding a prefix indicating that. If new library returns this prefix, convert the secret to plain data to keep using existing token and not require reauthentication. Originally suggested in cli#7023 (comment) Fixes cli#7123
Switch from github.com/zalando/go-keyring to github.com/99designs/keyring that uses native bindings on macOS and Windows. On darwin, github.com/zalando/go-keyring encodes secret data in hex or base64, adding a prefix indicating that. If new library returns this prefix, convert the secret to plain data to keep using existing token and not require reauthentication. Originally suggested in cli#7023 (comment) Fixes cli#7123
Switch from github.com/zalando/go-keyring to github.com/99designs/keyring that uses native bindings on macOS and Windows. On darwin, github.com/zalando/go-keyring encodes secret data in hex or base64, adding a prefix indicating that. If new library returns this prefix, convert the secret to plain data to keep using existing token and not require reauthentication. Originally suggested in cli#7023 (comment) Fixes cli#7123
Done. Please see the PR for the additional compatibility code. |
Thanks for the new commits! Without having to force people to re-authenticate we are far more interested in merging this work. Unfortunately our team is stretched so thin right now we can't give this work the QA attention it deserves. I'm going to mark this as blocked for now for us to revisit once we have more bandwidth towards the end of the year. |
Switch from github.com/zalando/go-keyring to github.com/99designs/keyring that uses native bindings on macOS and Windows. On darwin, github.com/zalando/go-keyring encodes secret data in hex or base64, adding a prefix indicating that. If new library returns this prefix, convert the secret to plain data to keep using existing token and not require reauthentication. Originally suggested in cli#7023 (comment) Fixes cli#7123
This isn't *great*, but it does significantly improve local developer ergonomics. Prior to this change, things worked just fine (WRT the keychain parts), but the ergonomics kinda sucked. This is because, by default, only the application that writes the keychain entry is allowed to read from it. So what would happen during development is that you'd compile, go through the oauth device flow then realize you have to fix some bug, fix the bug, then try to run the application again: 💥 macOS password prompt. The changes here in this commit work around the issue by using `security` (the macOS built-in CLI for handling this stuff). That results in the keychain entry always being read/written by a stable binary so debug builds/re-builds don't have issue. The downside here, is that "anyone" that can execute `security` can read the password. For what it's worth, this is exactly what `gh` does. See some references below: - cli/cli#7043 - https://github.com/zalando/go-keyring - cli/cli#7123 - cli/cli#7023 (comment)
Switch from github.com/zalando/go-keyring to github.com/99designs/keyring that uses native bindings on macOS and Windows. On darwin, github.com/zalando/go-keyring encodes secret data in hex or base64, adding a prefix indicating that. If new library returns this prefix, convert the secret to plain data to keep using existing token and not require reauthentication. Originally suggested in cli#7023 (comment) Fixes cli#7123
Switch from github.com/zalando/go-keyring to github.com/99designs/keyring that uses native bindings on macOS and Windows. On darwin, github.com/zalando/go-keyring encodes secret data in hex or base64, adding a prefix indicating that. If new library returns this prefix, convert the secret to plain data to keep using existing token and not require reauthentication. Originally suggested in cli#7023 (comment) Fixes cli#7123
Prerequisites:
Any reason why you are not using https://github.com/99designs/keyring besides the cgo problem? The zalando one forks a
security
command on MacOS, which is not a secure practice really.I have to grant access to the
security
cli for the github auth token access, andsecurity
can then be invoked with any other shell script after that, losing control of who I grant access to those creds.The 99designs lib does not have this problem, as it uses native API-s, so MacOS would prompt me to grant access to
gh
only.Using the
security
cli tool directly opens up people's hosts to malicious shell scripts also being able to use thesecurity
cli tool and gaining access to the credentials, partially defeating the purpose of storing those secrets in the keychain.Originally posted by @reegnz in #7023 (comment)
The text was updated successfully, but these errors were encountered: