From 6b7f1ff06072858daa35b378be3544f932e6eabd Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 26 Nov 2024 22:08:45 +0100 Subject: [PATCH] Allow client fetch to use insecure credentials pattern --- git/client.go | 7 +------ git/client_test.go | 26 ++------------------------ pkg/cmd/issue/develop/develop_test.go | 5 ----- pkg/cmd/pr/checkout/checkout.go | 7 +++++-- pkg/cmd/repo/clone/clone_test.go | 2 -- pkg/cmd/repo/fork/fork_test.go | 8 -------- 6 files changed, 8 insertions(+), 47 deletions(-) diff --git a/git/client.go b/git/client.go index fc386d76f..d2919ad22 100644 --- a/git/client.go +++ b/git/client.go @@ -577,16 +577,11 @@ func (c *Client) AddRemote(ctx context.Context, name, urlStr string, trackingBra // Below are commands that make network calls and need authentication credentials supplied from gh. func (c *Client) Fetch(ctx context.Context, remote string, refspec string, mods ...CommandModifier) error { - host, err := c.CredentialPatternFromRemote(ctx, remote) - if err != nil { - return err - } - args := []string{"fetch", remote} if refspec != "" { args = append(args, refspec) } - cmd, err := c.AuthenticatedCommand(ctx, host, args...) + cmd, err := c.AuthenticatedCommand(ctx, InsecureAllMatchingCredentialsPattern, args...) if err != nil { return err } diff --git a/git/client_test.go b/git/client_test.go index d680c2f00..614f05995 100644 --- a/git/client_test.go +++ b/git/client_test.go @@ -1166,33 +1166,15 @@ func TestClientFetch(t *testing.T) { name: "accepts command modifiers", mods: []CommandModifier{WithRepoDir("/path/to/repo")}, commands: map[args]commandResult{ - `path/to/git remote get-url origin`: { - ExitStatus: 0, - Stdout: "https://github.com/cli/nonexistent.git", - }, - `path/to/git -C /path/to/repo -c credential.https://github.com.helper= -c credential.https://github.com.helper=!"gh" auth git-credential fetch origin trunk`: { + `path/to/git -C /path/to/repo -c credential.helper= -c credential.helper=!"gh" auth git-credential fetch origin trunk`: { ExitStatus: 0, }, }, }, - { - name: "git error on get-url", - commands: map[args]commandResult{ - `path/to/git remote get-url origin`: { - ExitStatus: 1, - Stderr: "get-url error message", - }, - }, - wantErrorMsg: "failed to run git: get-url error message", - }, { name: "git error on fetch", commands: map[args]commandResult{ - `path/to/git remote get-url origin`: { - ExitStatus: 0, - Stdout: "https://github.com/cli/nonexistent.git", - }, - `path/to/git -c credential.https://github.com.helper= -c credential.https://github.com.helper=!"gh" auth git-credential fetch origin trunk`: { + `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential fetch origin trunk`: { ExitStatus: 1, Stderr: "fetch error message", }, @@ -1237,10 +1219,6 @@ func TestClientPull(t *testing.T) { name: "accepts command modifiers", mods: []CommandModifier{WithRepoDir("/path/to/repo")}, commands: map[args]commandResult{ - `path/to/git remote get-url origin`: { - ExitStatus: 0, - Stdout: "https://github.com/cli/nonexistent.git", - }, `path/to/git -C /path/to/repo -c credential.helper= -c credential.helper=!"gh" auth git-credential pull --ff-only origin trunk`: { ExitStatus: 0, }, diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index 655668b46..abdebf0c8 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -314,7 +314,6 @@ func TestDevelopRun(t *testing.T) { ) }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git remote get-url origin`, 0, "https://github.com/OWNER/REPO.git") cs.Register(`git fetch origin \+refs/heads/my-issue-1:refs/remotes/origin/my-issue-1`, 0, "") }, expectedOut: "github.com/OWNER/REPO/tree/my-issue-1\n", @@ -361,7 +360,6 @@ func TestDevelopRun(t *testing.T) { ) }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git remote get-url origin`, 0, "https://github.com/cli/cli.git") cs.Register(`git fetch origin \+refs/heads/my-issue-1:refs/remotes/origin/my-issue-1`, 0, "") }, expectedOut: "github.com/OWNER2/REPO/tree/my-issue-1\n", @@ -400,7 +398,6 @@ func TestDevelopRun(t *testing.T) { ) }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git remote get-url origin`, 0, "https://github.com/cli/cli.git") cs.Register(`git fetch origin \+refs/heads/my-branch:refs/remotes/origin/my-branch`, 0, "") }, expectedOut: "github.com/OWNER/REPO/tree/my-branch\n", @@ -470,7 +467,6 @@ func TestDevelopRun(t *testing.T) { ) }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git remote get-url origin`, 0, "https://github.com/cli/cli.git") cs.Register(`git fetch origin \+refs/heads/my-branch:refs/remotes/origin/my-branch`, 0, "") cs.Register(`git rev-parse --verify refs/heads/my-branch`, 0, "") cs.Register(`git checkout my-branch`, 0, "") @@ -513,7 +509,6 @@ func TestDevelopRun(t *testing.T) { ) }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git remote get-url origin`, 0, "https://github.com/cli/cli.git") cs.Register(`git fetch origin \+refs/heads/my-branch:refs/remotes/origin/my-branch`, 0, "") cs.Register(`git rev-parse --verify refs/heads/my-branch`, 1, "") cs.Register(`git checkout -b my-branch --track origin/my-branch`, 0, "") diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 4a732a409..a4cb405c1 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -251,9 +251,12 @@ func executeCmds(client *git.Client, credentialPattern git.CredentialPattern, cm for _, args := range cmdQueue { var err error var cmd *git.Command - if args[0] == "fetch" || args[0] == "submodule" { + switch args[0] { + case "submodule": cmd, err = client.AuthenticatedCommand(context.Background(), credentialPattern, args...) - } else { + case "fetch": + cmd, err = client.AuthenticatedCommand(context.Background(), git.InsecureAllMatchingCredentialsPattern, args...) + default: cmd, err = client.Command(context.Background(), args...) } if err != nil { diff --git a/pkg/cmd/repo/clone/clone_test.go b/pkg/cmd/repo/clone/clone_test.go index fe122690b..471ed05dd 100644 --- a/pkg/cmd/repo/clone/clone_test.go +++ b/pkg/cmd/repo/clone/clone_test.go @@ -259,7 +259,6 @@ func Test_RepoClone_hasParent(t *testing.T) { cs.Register(`git clone https://github.com/OWNER/REPO.git`, 0, "") cs.Register(`git -C REPO remote add -t trunk upstream https://github.com/hubot/ORIG.git`, 0, "") - cs.Register(`git -C REPO remote get-url upstream`, 0, "https://github.com/hubot/ORIG.git") cs.Register(`git -C REPO fetch upstream`, 0, "") cs.Register(`git -C REPO remote set-branches upstream *`, 0, "") cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "") @@ -300,7 +299,6 @@ func Test_RepoClone_hasParent_upstreamRemoteName(t *testing.T) { cs.Register(`git clone https://github.com/OWNER/REPO.git`, 0, "") cs.Register(`git -C REPO remote add -t trunk test https://github.com/hubot/ORIG.git`, 0, "") - cs.Register(`git -C REPO remote get-url test`, 0, "https://github.com/hubot/ORIG.git") cs.Register(`git -C REPO fetch test`, 0, "") cs.Register(`git -C REPO remote set-branches test *`, 0, "") cs.Register(`git -C REPO config --add remote.test.gh-resolved base`, 0, "") diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 95b27ae60..0f94496f0 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -442,7 +442,6 @@ func TestRepoFork(t *testing.T) { execStubs: func(cs *run.CommandStubber) { cs.Register(`git clone --depth 1 https://github.com/someone/REPO\.git`, 0, "") cs.Register(`git -C REPO remote add upstream https://github\.com/OWNER/REPO\.git`, 0, "") - cs.Register(`git -C REPO remote get-url upstream`, 0, "https://github.com/OWNER/REPO.git") cs.Register(`git -C REPO fetch upstream`, 0, "") cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "") }, @@ -475,7 +474,6 @@ func TestRepoFork(t *testing.T) { execStubs: func(cs *run.CommandStubber) { cs.Register(`git clone https://github.com/gamehendge/REPO\.git`, 0, "") cs.Register(`git -C REPO remote add upstream https://github\.com/OWNER/REPO\.git`, 0, "") - cs.Register(`git -C REPO remote get-url upstream`, 0, "https://github.com/OWNER/REPO.git") cs.Register(`git -C REPO fetch upstream`, 0, "") cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "") }, @@ -492,7 +490,6 @@ func TestRepoFork(t *testing.T) { execStubs: func(cs *run.CommandStubber) { cs.Register(`git clone https://github.com/someone/REPO\.git`, 0, "") cs.Register(`git -C REPO remote add upstream https://github\.com/OWNER/REPO\.git`, 0, "") - cs.Register(`git -C REPO remote get-url upstream`, 0, "https://github.com/OWNER/REPO.git") cs.Register(`git -C REPO fetch upstream`, 0, "") cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "") }, @@ -529,7 +526,6 @@ func TestRepoFork(t *testing.T) { execStubs: func(cs *run.CommandStubber) { cs.Register(`git clone https://github.com/someone/REPO\.git`, 0, "") cs.Register(`git -C REPO remote add upstream https://github\.com/OWNER/REPO\.git`, 0, "") - cs.Register(`git -C REPO remote get-url upstream`, 0, "https://github.com/OWNER/REPO.git") cs.Register(`git -C REPO fetch upstream`, 0, "") cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "") }, @@ -554,7 +550,6 @@ func TestRepoFork(t *testing.T) { execStubs: func(cs *run.CommandStubber) { cs.Register(`git clone https://github.com/someone/REPO\.git`, 0, "") cs.Register(`git -C REPO remote add upstream https://github\.com/OWNER/REPO\.git`, 0, "") - cs.Register(`git -C REPO remote get-url upstream`, 0, "https://github.com/OWNER/REPO.git") cs.Register(`git -C REPO fetch upstream`, 0, "") cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "") }, @@ -591,7 +586,6 @@ func TestRepoFork(t *testing.T) { execStubs: func(cs *run.CommandStubber) { cs.Register(`git clone https://github.com/someone/REPO\.git`, 0, "") cs.Register(`git -C REPO remote add upstream https://github\.com/OWNER/REPO\.git`, 0, "") - cs.Register(`git -C REPO remote get-url upstream`, 0, "https://github.com/OWNER/REPO.git") cs.Register(`git -C REPO fetch upstream`, 0, "") cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "") }, @@ -607,7 +601,6 @@ func TestRepoFork(t *testing.T) { execStubs: func(cs *run.CommandStubber) { cs.Register(`git clone https://github.com/someone/REPO\.git`, 0, "") cs.Register(`git -C REPO remote add upstream https://github\.com/OWNER/REPO\.git`, 0, "") - cs.Register(`git -C REPO remote get-url upstream`, 0, "https://github.com/OWNER/REPO.git") cs.Register(`git -C REPO fetch upstream`, 0, "") cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "") }, @@ -698,7 +691,6 @@ func TestRepoFork(t *testing.T) { cs.Register(`git clone https://github.com/someone/REPO\.git`, 128, "") cs.Register(`git clone https://github.com/someone/REPO\.git`, 0, "") cs.Register(`git -C REPO remote add upstream https://github\.com/OWNER/REPO\.git`, 0, "") - cs.Register(`git -C REPO remote get-url upstream`, 0, "https://github.com/OWNER/REPO.git") cs.Register(`git -C REPO fetch upstream`, 0, "") cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "") },