gh auth login hangs when config.yml is read-only, potentially producing an "auth schism"

[twz123]

Describe the bug

When config.yml is read-only, gh auth login hangs indefinitely after having stored the token in secure storage and printing ✓ Authentication complete. This can cause some confusion for users, as described in the linked discussion below.

Steps to reproduce the behavior

• gh wants GH_TOKEN env variable even when logged in #8347 (reply in thread)

Expected vs actual behavior

• No hangs, but a warning message printed to the console and continue with the process (e.g. by trying to write hosts.yml)
• Maybe a more consistent behavior of gh, as some parts of the CLI claim authentication is done while others ask for credentials.

Logs

None.

[williammartin]

Yeh the confusion here stems from the fact that ✓ Authentication complete. is printed once the oauth flow is completed (but before we store any configuration changes):

cli/pkg/cmd/auth/shared/login_flow.go

Line 159 in 06e438b

fmt.Fprintf(opts.IO.ErrOut, "%s Authentication complete.\n", cs.SuccessIcon())

In the case of refresh we actually print this message after the config changes have been made:

cli/pkg/cmd/auth/refresh/refresh.go

Line 198 in 06e438b

fmt.Fprintf(opts.IO.ErrOut, "%s Authentication complete.\n", cs.SuccessIcon())

Perhaps the simplest thing to do to remove the confusion is just to move that message out of the login flow and into the login command.

With regards the hang, I need to do some investigation. I'm surprised there is a hang rather than an error? For example, in #7360 there is an EPERM. It would be nice if you could recreate that scenario and maybe get an strace so we can see where it is hanging.

[twz123]

Did more investigation about the hang. When experiencing the hang, I was using the packaged gh binary of my distro, which is v2.29.0 currently. I never tried to re-auth with the current version without setting the custom GH_CONFIG_DIR. I actually mis-read the distro's gh version to be v2.39.0, so I thought I'd only be one point release behind, not ten minor releases 🙈.

I can repro the hang using 2.29.0, with both the distro's binary and the binary provided on the release download page. However, with 2.39.1, I get a proper error message, again both with the distro's unstable binary as well as with the binary from the release page:

$ ./gh auth login
? What account do you want to log into? GitHub.com
? You're already logged into github.com. Do you want to re-authenticate? Yes
? What is your preferred protocol for Git operations? SSH
? Generate a new SSH key to add to your GitHub account? No
? How would you like to authenticate GitHub CLI? Paste an authentication token
Tip: you can generate a Personal Access Token here https://github.com/settings/tokens
The minimum required scopes are 'repo', 'read:org'.
? Paste your authentication token: ****************************************
- gh config set -h github.com git_protocol ssh
✓ Configured git protocol
open /home/twieczorek/.config/gh/config.yml: read-only file system

I also tried strace, but there's not much enlightening in there, at least not for me. And in the end, the hang is no longer happening in current versions, so it doesn't really matter 🤷 🙂.

strace on v2.29.0 after pasting the token and hitting enter

16:49:50.653917 clock_gettime(CLOCK_MONOTONIC, {tv_sec=200513, tv_nsec=428598176}) = 0
16:49:50.654143 futex(0x2af2448, FUTEX_WAIT_PRIVATE, 0, NULL) = 0
16:49:50.796205 epoll_pwait(4, [], 128, 0, NULL, 0) = 0
16:49:50.796486 clock_gettime(CLOCK_MONOTONIC, {tv_sec=200513, tv_nsec=571145327}) = 0
16:49:50.796817 write(6, "\0", 1)       = 1
16:49:50.797001 futex(0x2af2448, FUTEX_WAIT_PRIVATE, 0, NULL) = 0
16:49:50.966991 epoll_pwait(4, [{events=EPOLLOUT, data={u32=1917058824, u64=140554721820424}}], 128, 0, NULL, 0) = 1
16:49:50.967341 clock_gettime(CLOCK_MONOTONIC, {tv_sec=200513, tv_nsec=742033267}) = 0
16:49:50.967640 write(6, "\0", 1)       = 1
16:49:50.967960 futex(0x2af2448, FUTEX_WAIT_PRIVATE, 0, NULL) = 0
16:49:50.968321 clock_gettime(CLOCK_MONOTONIC, {tv_sec=200513, tv_nsec=742953574}) = 0
16:49:50.968504 write(6, "\0", 1)       = 1
16:49:50.968793 futex(0x2af2448, FUTEX_WAIT_PRIVATE, 0, NULL) = 0
16:49:50.969149 clock_gettime(CLOCK_MONOTONIC, {tv_sec=200513, tv_nsec=743770933}) = 0
16:49:50.969279 write(6, "\0", 1)       = 1
16:49:50.969400 futex(0x2af2448, FUTEX_WAIT_PRIVATE, 0, NULL) = 0
16:49:50.970026 epoll_pwait(4, [], 128, 0, NULL, 0) = 0
16:49:50.970160 clock_gettime(CLOCK_MONOTONIC, {tv_sec=200513, tv_nsec=744793069}) = 0
16:49:50.970300 write(6, "\0", 1)       = 1
16:49:50.970418 futex(0x2af2448, FUTEX_WAIT_PRIVATE, 0, NULL) = 0
16:49:50.970964 clock_gettime(CLOCK_MONOTONIC, {tv_sec=200513, tv_nsec=745597159}) = 0
16:49:50.971109 write(6, "\0", 1)       = 1
16:49:50.971227 futex(0x2af2448, FUTEX_WAIT_PRIVATE, 0, NULL) = 0
16:49:50.971530 clock_gettime(CLOCK_MONOTONIC, {tv_sec=200513, tv_nsec=746156872}) = 0
16:49:50.971741 write(6, "\0", 1)       = 1
16:49:50.971850 futex(0x2af2448, FUTEX_WAIT_PRIVATE, 0, NULL) = ? ERESTARTSYS (To be restarted if SA_RESTART is set)
16:50:38.465989 --- SIGINT {si_signo=SIGINT, si_code=SI_KERNEL} ---
16:50:38.466153 rt_sigprocmask(SIG_UNBLOCK, [INT], NULL, 8) = 0
16:50:38.466298 getpid()                = 2318114
16:50:38.466397 gettid()                = 2318114
16:50:38.466512 tgkill(2318114, 2318114, SIGINT) = 0
16:50:38.466606 --- SIGINT {si_signo=SIGINT, si_code=SI_TKILL, si_pid=2318114, si_uid=2000} ---
16:50:38.466661 rt_sigaction(SIGINT, {sa_handler=SIG_DFL, sa_mask=~[], sa_flags=SA_RESTORER|SA_ONSTACK|SA_RESTART|SA_SIGINFO, sa_restorer=0x4685c0}, NULL, 8) = 0
16:50:38.466783 rt_sigprocmask(SIG_UNBLOCK, [INT], NULL, 8) = 0
16:50:38.466875 getpid()                = 2318114
16:50:38.466959 gettid()                = 2318114
16:50:38.467042 tgkill(2318114, 2318114, SIGINT) = 0
16:50:38.467132 --- SIGINT {si_signo=SIGINT, si_code=SI_TKILL, si_pid=2318114, si_uid=2000} ---
16:50:38.472346 +++ killed by SIGINT +++

[williammartin]

Thanks for investigation, nothing obvious in the strace as you say. I have some thoughts on how to debug that but since it's not occurring anymore I don't think it's worth us spending the time.

With this new error, it maybe is also clearer that something in fact went wrong even if it says ✓ Authentication complete. so maybe the change isn't necessary. I'm ambivalent.

[twz123]

so maybe the change isn't necessary. I'm ambivalent.

Yeah. The only thing remaining is the weird outcome that gh auth token works (and gh auth login also states "You're already logged into github.com. Do you want to re-authenticate?") while all the other commands ask for auth...

[williammartin]

Right, I forgot about that. Would it be possible for you to double check for me if that is still the case in the latest version?

By that I mean, logging out and then going through the log in process again, seeing it fail on config.yml.

I can't quite see how that would happen in the latest as the hosts.yml should be written before config.yml, and that is where the known hosts are enumerated. I'm just a bit concerned that the older version use in the original discussion might have some red herrings.

[twz123]

This can be triggered by simply removing the hosts.yml file (or maybe by just removing the github.com section from it?). I checked and in the current version that's not hanging, gh auth login writes hosts.yml, even in the case when config.yml can't be written to. So there's definitely extra user interaction necessary to trigger this.

$ ./gh version
gh version 2.39.1 (2023-11-14)
https://github.com/cli/cli/releases/tag/v2.39.1

$ ./gh auth status
github.com
  ✓ Logged in to github.com as twz123 (keyring)
  ✓ Git operations for github.com configured to use ssh protocol.
  ✓ Token: gho_************************************
  ✓ Token scopes: gist, read:org, repo

$ mv ~/.config/gh/hosts.yml ~/.config/gh/hosts.yml.foo

$ ./gh auth status
You are not logged into any GitHub hosts. Run gh auth login to authenticate.

$ ./gh auth login
? What account do you want to log into? GitHub.com
? You're already logged into github.com. Do you want to re-authenticate? (y/N)
^C

$ GH_TOKEN=$(./gh auth token) ./gh auth status
github.com
  ✓ Logged in to github.com as twz123 (GH_TOKEN)
  ✓ Git operations for github.com configured to use https protocol.
  ✓ Token: gho_************************************
  ✓ Token scopes: gist, read:org, repo

[williammartin]

Thanks for the extra investigation, super valuable.

Yeh, I understand how this would occur if the hosts.yml file were deleted. Ultimately, gh expects to own the configuration files and if you go messing around with them you're in for a bad time. I'm not against making changes to make this bifurcation less confusing but I reckon it'd be low priority given the user done goofed (assuming there's no way for this to happen normally in current versions).