Skip to content

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

Description

@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

Activity

  1. arunsathiya commented on Dec 28, 2023

    @arunsathiya
    Contributor

    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.

  2. water-sucks commented on Dec 28, 2023

    @water-sucks
    Author

    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.

  3. arunsathiya commented on Dec 28, 2023

    @arunsathiya
    Contributor

    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

  4. williammartin commented on Jan 22, 2024

    @williammartin
    Member

    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 😬

  5. added
    priority-3Affects a small number of users or is largely cosmetic
    priority-2Affects more than a few users but doesn't prevent core functions
    and removed
    priority-3Affects a small number of users or is largely cosmetic
    on Jan 22, 2024
  6. williammartin commented on Jan 22, 2024

    @williammartin
    Member

    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.

  7. williammartin commented on Jan 22, 2024

    @williammartin
    Member

    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())
    }
    
  8. williammartin commented on Jan 22, 2024

    @williammartin
    Member

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

  9. added
    gh-authrelating to the gh auth command
    gh-configrelating to the gh config command
    on Jan 22, 2024
  10. williammartin commented on Jan 23, 2024

    @williammartin
    Member

    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.

  11. added a commit that references this issue on Jan 31, 2024
    3063a61
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workinggh-authrelating to the gh auth commandgh-configrelating to the gh config commandpriority-2Affects more than a few users but doesn't prevent core functions

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions