config.yml is opened for writing on gh auth switch when it should not be

[water-sucks]

Describe the bug

Certain commands, like gh auth switch, appear to fail entirely with an error failing to open the config.yml when it is immutable (i.e. when using home-manager):

However, they do not actually fail, and the account is switched. This error is rather misleading, because I thought this was a fatal error at first and the account switch didn't happen when it did, and I don't believe that config.yml should ever be opened in situations like this where it never is touched at all.

This occurs on gh version 2.40.1.

Steps to reproduce the behavior

1. Make config.yml immutable. (chattr +i ~/.config/gh/config.yml for testing)
2. Login with multiple accounts
3. Run gh auth switch

Expected vs actual behavior

I expected the account to be switched without errors like this, and the config.yml to not be opened for writing at all.

Logs

X Failed to switch account for github.com to <account-name>
open /home/<username>/.config/gh/config.yml: read-only file system

[arunsathiya]

I can reproduce the file opening behavior that you mentioned, but..

However, they do not actually fail, and the account is switched. This error is rather misleading

..it seems that the switch action doesn't actually go through, thus matching the error message.

󰂃 98% ~ ❯ gh auth token
gho_redacted5sh
(base)

󰂃 98% ~ ❯ gh auth switch
X Failed to switch account for github.com to arunsathiya
open /Users/arun/.config/gh/hosts.yml: operation not permitted
(base)

󰂃 98% ~ ❯ gh auth token
gho_redacted5sh

We can notice that both tokens end in 5sh, thus indicating the same account.

[water-sucks]

Ah, in my (and most home-manager users') case, the hosts.yml file is mutable. Just not the config.yml file.

I can reproduce the same thing you did if hosts.yml is also immutable, but that's not the exact issue I was running into.

[arunsathiya]

Interesting. I have been using keyring/keychain for secure storage, so I was not familiar with hosts.yml vs config.yml differences.

When I used --insecure-storage flag in gh auth login, it stored credentials to hosts.yml, so I just assumed that config.yml has been phased out in favor of hosts.yml

[williammartin]

Hey, thanks for opening this issue and sorry for the inconvenience and delay (catching up on issues that need triage after the holidays).

I can reproduce this on Mac:

➜  ~ gh auth status
github.com
  ✓ Logged in to github.com account wilmartin_microsoft (keyring)
  - Active account: true
  - Git operations protocol: https
  - Token: gho_************************************
  - Token scopes: 'gist', 'read:org', 'read:project', 'repo', 'workflow'

  ✓ Logged in to github.com account williammartin (keyring)
  - Active account: false
  - Git operations protocol: https
  - Token: gho_************************************
  - Token scopes: 'gist', 'read:org', 'read:project', 'repo', 'workflow'
  
➜  ~ sudo chflags uchg ~/.config/gh/config.yml

➜  ~ gh auth switch
X Failed to switch account for github.com to williammartin
open /Users/williammartin/.config/gh/config.yml: operation not permitted

This is definitely a bug and I will look into it. As you noted, it shouldn't actually result in any functional problem because well, the config file shouldn't be written at all when switching.

---

When I used --insecure-storage flag in gh auth login, it stored credentials to hosts.yml, so I just assumed that config.yml has been phased out in favor of hosts.yml

The historical reason for the split as far as I know is that it should be possible to put the config in version control without worrying about tokens being committed. I don't know if that's true or not as no one from the team is still around 😬

[williammartin]

Actually there is a bug here as well because the rollback logic in case of error leaves things in an inconsistent state.

The problem is that the hosts file has been persisted while the config file hasn't. Thus the hosts file says the switch has taken place, but we've rolled back the active token to the previous user.

// We are currently williammartin
➜  ~ gh api /user | jq .login
"williammartin"

// We switch which fails
➜  ~ gh auth switch
X Failed to switch account for github.com to wilmartin_microsoft
open /Users/williammartin/.config/gh/config.yml: operation not permitted

// gh auth status says the switch was successful
➜  ~ gh auth status
github.com
  ✓ Logged in to github.com account wilmartin_microsoft (keyring)
  - Active account: true
  - Git operations protocol: https
  - Token: gho_************************************
  - Token scopes: 'gist', 'read:org', 'read:project', 'repo', 'workflow'

  ✓ Logged in to github.com account williammartin (keyring)
  - Active account: false
  - Git operations protocol: https
  - Token: gho_************************************
  - Token scopes: 'gist', 'read:org', 'read:project', 'repo', 'workflow'
 
// But the token is actually still for the old user
➜  ~ gh api /user | jq .login
"williammartin"

I've upgraded this to a p2 as a result.

[williammartin]

As far as I can tell this has been an issue for a long long time for any command that made changes to the hosts file. Checkout the difference between v2.13.0 and v2.14.0:

➜  cli git:(v2.13.0) ✗ ./bin/gh version
gh version 2.13.0 (2024-01-22)
https://github.com/cli/cli/releases/tag/v2.13.0
➜  cli git:(v2.13.0) ✗ ./bin/gh auth logout
✓ Logged out of github.com account 'williammartin'

➜  cli git:(v2.14.0) ✗ ./bin/gh version
gh version 2.14.0 (2024-01-22)
https://github.com/cli/cli/releases/tag/v2.14.0
➜  cli git:(v2.14.0) ✗ ./bin/gh auth logout
failed to write config, authentication configuration not updated: open /Users/williammartin/.config/gh/config.yml: operation not permitted

This change coincided with cli/go-gh#44 which moved config management from cli/cli into cli/go-gh in June 2022 and as far as I can tell the logic has been incorrect since then. The intention was that only if a file had changed, should it be persisted:

https://github.com/cli/go-gh/blob/bb3e52bc6da80e0dd96d7f7949917a21983a3176/pkg/config/config.go#L129-L145

But the thing is that it appears to me as if the general node is marked as modified if there is a hosts file:

https://github.com/cli/go-gh/blob/bb3e52bc6da80e0dd96d7f7949917a21983a3176/pkg/config/config.go#L181-L183

Here's a test that demonstrates this incorrect behaviour:

func TestIsModifiedOnLoad(t *testing.T) {
	// Given we have a persisted config and hosts file
	tempDir := t.TempDir()
	t.Setenv("GH_CONFIG_DIR", tempDir)

	require.NoError(t, writeFile(hostsConfigFile(), []byte(testHostsData())))
	require.NoError(t, writeFile(generalConfigFile(), []byte(testGlobalData())))

	// When we load that config
	cfg, err := load(generalConfigFile(), hostsConfigFile(), nil)
	require.NoError(t, err)

	// Then the general entries should be unmodified (because we didn't mutate anything)
	require.False(t, cfg.entries.IsModified())
}

[williammartin]

Created cli/go-gh#147 to address this.

[williammartin]

That PR has been merged, so when we ship a new version of go-gh we'll include it in the next CLI release which will be in the next week or two.