Repository navigation
gh auth login should error if the system keyring is unavailable rather than falling back to insecure storage #10108
Description
Description
Originally, the GitHub CLI only supported writing the token during obtained auth login, insecurely to a file. In fact it was only February 2023 that gh began using keyrings for secure storage. When this was added, it was opt-in via --secure-storage because the team didn't want to break existing workflows. Later, this was made the default and the previous insecure behaviour was opt in via --insecure-storage. In order to avoid breaking anyone who didn't have a system keyring set up correctly, gh would fallback to the insecure storage if the keyring was unavailable.
We've had so many issues of people complaining about gh falling back to insecure storage, not being particularly transparent about doing that, and gh giving no indication of why the secure storage didn't work (because the error is swallowed) that I think we should do something about it:
- gh auth saves plain text token, even though I have keyring #8954
- Clarify "an authentication token will be stored internally" -- where is the token stored? #5961
It would be nice to have telemetry on:
- How many / what percentage of people currently rely on the fallback behaviour of
auth login.
However, in the absence of this, my gut feeling is that it may still be a breaking change worth making, with a clear path to fixing users who are broken (--insecure-storage and possibly a config option), particularly in the interactive case.
What Options are on the Table?
Non-Interactivity
I don't think it's acceptable to break scripts out there that are calling gh auth login --with-token on environments that don't have a keyring available. However, we can consider some mitigations and nudges:
- We could add a
--no-insecure-fallbackoption for users who really, really don't want this to happen - We could add a
no_insecure_fallbackconfig option forgh configthat makes it persistent - We could add a warning on stderr, just in case someone looks at their script output and realises that something unexpected is happening
Interactivity
With the interactive flows we have a bit more flexibility around changing behaviour. I think there are three paths we could take here, each with a different tradeoff of safety vs convenience.
--no-insecure-fallback/no_insecure_fallback idea from above
As above, we could allow users to opt-in to turning this off. The downside of course is that this is opt-in, which even with the warning, still ends up with your token potentially being written to disk.
Re-prompt
On a failure to use the keyring we could offer a prompt along the lines of:
Failed to save the token securely due to <err>
Would you like to continue with insecure storage (y/N) ?
This is informative and allows for immediate recovery, which is nice since the user has probably gone through a whole oauth dance already.
Error
On a failure to use the keyring we could error out with the reason.
This might be immediately annoying to a user, but it's a pretty big nudge that they need to start using --insecure-storage from now on, which should be high friction.
My Recommendation
I think I would probably favour all the non-interactive options, and the last of the interactive options (erroring). I don't love that it's not "secure by default" across the board but I really can't see any way in which we can accept breaking the non-interactive usage, which could be large.
Both the re-prompt and error options would also help provide clarity to why keyring operations are failing, since the error is currently suppressed.
Activity
Stabledog commentedon Dec 20, 2024
I suppose it's too late to have the following discussion, but here goes anyway? Github has long provided ssh authentication with key pairs. It works fine, and is already in place. Very convenient. But
ghcan't use that mechanism. Why? Presumably some design meeting occurred where everyone agreed that "ssh key pairs is either insecure or inconvienent", but the solution that actually got implemented is worse in both dimensions.Oauth stuff should have been the fallback plan. Keypairs should have been the default mechanism.
ghwould then "just work" out of the box without the user having to jump through oauth hoops, and adoption of the tool would be orders of magnitude greater because users wouldn't groan internally thinking "oh God, now I have to get the keyring working in that docker container and yadayada. I guess I'll just create the new repo with the browser"If I were in charge of the gh project -- the very first priority for next release would be to support ssh key pairs as an auth mechanism, using the existing pair if the user has one set up.
williammartin commentedon Dec 20, 2024
Thanks for leaving your thoughts. I don't think it's too late to have the discussion. Regardless of the conclusion, having an open conversation around options is valuable.
Just to make sure we're on the same page,
ghwill work with SSH keys when interacting withgitservices on GitHub, but not with the API. I'm quite confident there was noghdesign meeting about this, but thatghwas built around the existing API authentication schemes and token based authentication is the only option there. I'm happy to go and dig into that a bit more and see whether SSH key based auth was ever considered for the API.I suspect in practice it's just not really worth the extra complexity. Off the top of my head, since there's no existing mechanism for SSH authentication over HTTP (mTLS aside which requires more than just the existing keypair), for this to work we would need to build some kind of custom flow where the server provides something for you to sign with your private key, in order to auth you and mint a token for you to use over HTTP? Then we'd likely also end up working through many of the same issues as tokens e.g. authz permissions/scopes. Then we also have to manage expiration and revocation of these tokens.
Am I missing something?
Edit: Some evidence for an API that works via private key signing of headers here: https://docs.chef.io/server/api_chef_server/#authentication-headers
Not for me it wouldn't 😅 I don't want to manage SSH keys at all, and if I did, I definitely wouldn't want to move my private key around different machines, or have to generate temporary ones and add them to the Web UI each time.
I'd be pretty surprised if adding SSH based authentication with the API made much movement on
ghusage at all, but I've certainly been wrong before. I'd definitely be interested in hearing feedback from users who don't useghbecause of the existing auth mechanisms.For what it's worth, you can set
GH_TOKENenv var if you don't want to bother with the keyring at all. This obviously has different tradeoffs, but just in case you didn't know.I don't really have a horse in this race, I want
ghto be as useful as we can make it. Since starting to write this message I dropped a message internally to see whether this has ever been discussed. I'll let you know if I come up with anything interesting.andyfeller commentedon Dec 20, 2024
I'm torn reading through this issue for a few different reasons:
I mention that only to say I appreciate the obvious time you've spent on the topic and trying to navigate all the different variables involved! ❤
I'll do my best to match that care as well as I can. 🙇
Being nearly 2 years since more secure handling of tokens was introduced, I genuinely think there has been sufficient time for our community to raise concerns and pain points with adopting the capability that I'd be open to sunsetting
--insecure-storageall together if there wasn't a meaningful case to preserve it. To be clear, I mean a well communicated, 6 month plan to remove the feature all together with sufficient time for users to raise feedback that we could address before doing so without having them to depend on unsupported, pinned versions ofgh.I see your feelings further below on the topic ("I don't think it's acceptable to break scripts out there that are calling
gh auth login --with-tokenon environments that don't have a keyring available."). I'm unsure if we have a list of scenarios where secure storage IS NOT possible that we must officially support, so I would appreciate more concrete details that we could talk towards.As you said, no telemetry on the internal fallback to insecure storage makes this a bit difficult.
The next best thing I can point to are public
gh auth login --insecure-storagereferences on GitHub where a very low number of results where people are explicitly stating or using the--insecure-storageoption.I would prefer we have the users and automation who really, really want insecure storage fallback to ask for it:
--insecure-fallback.I can raise the white flag 🙌 that there might be cases to still allow insecure storage, however I think a reasonable breakage that requires people to explicitly ask for the insecure fallback is acceptable as secure by default should matter.
This is probably the most congruent behavior given my feelings on non-interactive and I appreciate how the default value is
nohere 🧑🍳 💋Here's my best middle of the road recommendation as I struggle with my personal bias to sunset insecure storage all together:
Introduce a minor breaking change to
gh auth loginnon-interactive use that requires--insecure-fallbackflag to opt-in to insecure storageThis feels like a fair compromise of supporting use cases we don't understand while promoting secure by default.
Prepare communication of this minor breaking change in GitHub Blog and GitHub CLI repo before merging and releasing the feature.
I truly want anyone this will affect to have the most notice possible. This should really point people at a prepared
clidiscussion to understand current use and consolidate discussion.Introduce a prompt asking the user whether they want to fallback to insecure storage during interactive
gh auth loginwhere secure storage is unavailable and defaulting to no, setting the--insecure-fallbackbypasses the prompt and uses insecure storageThis begins to make the concern apparent while promoting secure by default while providing a consistent experience between TTY and non-TTY.
Begin an effort to gather user feedback around valid insecure storage use cases to work with others at GitHub to address.
For automation and developer environment use cases such as GitHub Actions or GitHub Codespaces, I'm hoping we can get some support from those teams. Beyond that, I don't know of any other situation we'd knowingly want to continue supporting insecure storage.
This feels like a necessary step regardless of continuing to support insecure storage or planning to sunset it.
williammartin commentedon Dec 20, 2024
Thanks for your thoughtful response. It seems to me like the fallback is the worst of both worlds, and the only reason to continue supporting any fallback behaviour is to avoid breaking existing workflows. Once people are broken by a change in
gh, it seems like they should just set--insecure-storage. This approach also has the benefit of not breaking anyone currently using the flag.I don't think that
--insecure-fallbackis anymore "secure by default" than--insecure-storage. At least the latter makes it clear exactly what the result is going to be to anyone reading it. I don't think it's likely that someone's system is going to suddenly go from non-working keyring to working keyring, and then they aren't taking advantage of it because they set--insecure-storage.In my mind we're supporting anyone who is working an environment without a keyring configured in a way we can
ghcan correctly interact with. I appreciate there aren't that many results in the GitHub search, but it's not uncommon that people report issues and they are redacting tokens from theirhosts.yml. Also, here's an example where someone wants a fully portableghinstallation.I'm strongly against removing the insecure storage functionality altogether. Our users know their security posture better than us, and I suspect if we don't support it, they are probably just going to end up writing their own token to the filesystem and loading it via
GH_TOKEN. I'm open to the idea that we entirely remove it as fallback behaviour.If we agreed that we would simply turn off the fallback, I'd also be amenable to adding
insecure_token_storageto the config, so that people don't have to indicate it every time. I'd also be amenable to rejecting that because we want--insecure-storageto have some friction.assignUser commentedon Jan 1, 2025
Thanks everyone for the detailed discussion and effort put forth to improve this!
I think it makes sense to keep insecure storage around with an explicit opt-in. Had the re-prompt existed when I first came across this I would have been satisfied (see #5961).
In general I think the handling of credential storage would benefit from some additional docs on what is required to make it work (mostly for linux I think) and potentially some config options to address situations where the default settings don't work. e.g. #5961 (comment)
To close this out I want to leave some positive feedback as well as that is usually sorely lacking in /issues: I use
ghdaily and its addition to my workflow has smoothed out the handling of different repos/forks/PR reviews etc.! So thank you and a happy new year!Faiqonli commentedon Jan 2, 2025
zimmerman-pliant commentedon Mar 27, 2025
Yes, I think this is the key thing that's needed here, especially for headless/SSH access to servers.
I haven't found a clear answer on how to get a secure credential store working when strictly SSHing into headless machines... in my case Ubuntu desktop or server installations.
This is where I'm most likely to use/need a command-line tool.
fix(ghauth): persist device-flow token with --insecure-storage (#111)
Make the plaintext credential fallback visible and refusable
cloud: store the sync token in the OS keyring on every platform (#37)
docs: three claims, made true (#411)