gh repo fork no longer respects the git protocol of the original repo when git_protocol is not configured

Describe the bug

➜ gh version
gh version 2.49.0 (2024-04-30)
https://github.com/cli/cli/releases/tag/v2.49.0

In #1056, it was requested that gh repo fork respected the protocol of the existing remote i.e. if the remote was using ssh then the fork would as well. Later in #2711, it was requested that the configured git_protocol would take precedence over the remote, and only if no git_protocol was configured, we would use the protocol of the existing remote.

In #8246 the repo fork command was modified to call config.GitProtocol rather than directly accessing the git_protocol config key. The difference here is that the GitProtocol method always returns the configured value or the default of https. However, this means that the following code branch could never be exercised:

cli/pkg/cmd/repo/fork/fork.go

Lines 257 to 272 in 4896546

    
           if protocol == "" { // user has no set preference 
        
           	if remote, err := remotes.FindByRepo(repoToFork.RepoOwner(), repoToFork.RepoName()); err == nil { 
        
           		scheme := "" 
        
           		if remote.FetchURL != nil { 
        
           			scheme = remote.FetchURL.Scheme 
        
           		} 
        
           		if remote.PushURL != nil { 
        
           			scheme = remote.PushURL.Scheme 
        
           		} 
        
           		if scheme != "" { 
        
           			protocol = scheme 
        
           		} else { 
        
           			protocol = "https" 
        
           		} 
        
           	} 
        
           }

Steps to reproduce the behavior

Make a temp directory: cd $(mktemp -d)
Clone a repository via ssh: gh repo clone [email protected]:cli/cli.git
Navigate into the repository: cd cli
Fork the repository: gh repo fork --remote=true
List all remotes: git remote -v

Expected behaviour

The new fork uses the ssh protocol because the repo was originally cloned through that.

Actual behaviour

The fork was added using the https protocol:

❯ git remote -v
origin	https://github.com/williammartin/cli.git (fetch)
origin	https://github.com/williammartin/cli.git (push)
upstream	[email protected]:cli/cli.git (fetch)
upstream	[email protected]:cli/cli.git (push)

Extra Details

It's worth knowing that the git_protocol will not be configured in the following cases:

After logging in using --with-token and not providing --git-protocol e.g. echo $TOKEN | gh auth login --with-token
After logging in using the web flow with prompting disabled e.g. GH_PROMPT_DISABLED=true gh auth login
Running a command that uses git while using GH_TOKEN or another env var for authentication (and no entry in the hosts file)

Why didn't the tests catch this?

This branch is covered by tests but they didn't fail when the behaviour changed because the test setup doesn't accurately reflect reality. In reality, the git_protocol config value can only be https or ssh but the test sets it to the empty string on line 257:

cli/pkg/cmd/repo/fork/fork_test.go

Lines 243 to 264 in 4896546

    
           name: "implicit match, no configured protocol", 
        
           tty:  true, 
        
           opts: &ForkOptions{ 
        
           	Remote:     true, 
        
           	RemoteName: "fork", 
        
           }, 
        
           remotes: []*context.Remote{ 
        
           	{ 
        
           		Remote: &git.Remote{Name: "origin", PushURL: &url.URL{ 
        
           			Scheme: "ssh", 
        
           		}}, 
        
           		Repo: ghrepo.New("OWNER", "REPO"), 
        
           	}, 
        
           }, 
        
           cfgStubs: func(_ *testing.T, c config.Config) { 
        
           	c.Set("", "git_protocol", "") 
        
           }, 
        
           httpStubs: forkPost, 
        
           execStubs: func(cs *run.CommandStubber) { 
        
           	cs.Register(`git remote add fork git@github\.com:someone/REPO\.git`, 0, "") 
        
           }, 
        
           wantErrOut: "✓ Created fork someone/REPO\n✓ Added remote fork\n",

The consequence for this is that our config code treats this as a valid configuration with the value of empty string rather than returning https as it would when exercised through the CLI.

Removing this configuration stub fails as expected, with an attempt to add an https remote failing because the expectation is that we are adding an ssh remote:

			execStubs: func(cs *run.CommandStubber) {
				cs.Register(`git remote add fork git@github\.com:someone/REPO\.git`, 0, "")
			},

--- FAIL: TestRepoFork (0.01s)
    --- FAIL: TestRepoFork/implicit_match,_no_configured_protocol (0.00s)
        /Users/williammartin/workspace/cli/pkg/cmd/repo/fork/stub.go:27: unmatched stubs (1): git remote add fork git@github\.com:someone/REPO\.git
panic: no exec stub for `git remote add fork https://github.com/someone/REPO.git` [recovered]
	panic: no exec stub for `git remote add fork https://github.com/someone/REPO.git`

What should we do about this?

Although no one has complained that we've broken them, the previous behaviour seems reasonable and we did have a regression. I'm inclined to return this behaviour to it's previous state.

I have an additional reason to believe this is the right thing to do and that is because gh auth refresh offers configuration of the Git Credential Manager in cases where the git_protocol was not set and gh is not the credential manager e.g.:

➜  cat ~/.config/gh/hosts.yml
github.com:
    users:
        williammartin:
    user: williammartin

➜  gh auth refresh
? Authenticate Git with your GitHub credentials? (Y/n)

This is a departure from gh auth login which only prompts for this if the user selected the https protocol.

Navigation Menu

Repository navigation

`gh repo fork` no longer respects the git protocol of the original repo when `git_protocol` is not configured #9058

Describe the bug

Steps to reproduce the behavior

Expected behaviour

Actual behaviour

Extra Details

Why didn't the tests catch this?

What should we do about this?

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if protocol == "" { // user has no set preference
	if remote, err := remotes.FindByRepo(repoToFork.RepoOwner(), repoToFork.RepoName()); err == nil {
	scheme := ""
	if remote.FetchURL != nil {
	scheme = remote.FetchURL.Scheme
	}
	if remote.PushURL != nil {
	scheme = remote.PushURL.Scheme
	}
	if scheme != "" {
	protocol = scheme
	} else {
	protocol = "https"
	}
	}
	}

	name: "implicit match, no configured protocol",
	tty: true,
	opts: &ForkOptions{
	Remote: true,
	RemoteName: "fork",
	},
	remotes: []*context.Remote{
	{
	Remote: &git.Remote{Name: "origin", PushURL: &url.URL{
	Scheme: "ssh",
	}},
	Repo: ghrepo.New("OWNER", "REPO"),
	},
	},
	cfgStubs: func(_ *testing.T, c config.Config) {
	c.Set("", "git_protocol", "")
	},
	httpStubs: forkPost,
	execStubs: func(cs *run.CommandStubber) {
	cs.Register(`git remote add fork git@github\.com:someone/REPO\.git`, 0, "")
	},
	wantErrOut: "✓ Created fork someone/REPO\n✓ Added remote fork\n",

gh repo fork no longer respects the git protocol of the original repo when git_protocol is not configured #9058

Description

Describe the bug

Steps to reproduce the behavior

Expected behaviour

Actual behaviour

Extra Details

Activity

williammartin commented on May 7, 2024

Why didn't the tests catch this?

williammartin commented on May 7, 2024

What should we do about this?

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

`gh repo fork` no longer respects the git protocol of the original repo when `git_protocol` is not configured #9058