Skip to content

Commit 98ab1f2

Browse files
samcoesamcoe
authored
Authenticate network git commands (cli#6541)
1 parent 649fb35 commit 98ab1f2

File tree

12 files changed

+104
-74
lines changed

12 files changed

+104
-74
lines changed

‎git/client.go

Lines changed: 22 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -317,16 +317,6 @@ func (c *Client) DeleteLocalBranch(ctx context.Context, branch string) error {
317317
return nil
318318
}
319319

320-
func (c *Client) HasLocalBranch(ctx context.Context, branch string) bool {
321-
args := []string{"rev-parse", "--verify", "refs/heads/" + branch}
322-
cmd, err := c.Command(ctx, args...)
323-
if err != nil {
324-
return false
325-
}
326-
_, err = cmd.Output()
327-
return err == nil
328-
}
329-
330320
func (c *Client) CheckoutBranch(ctx context.Context, branch string) error {
331321
args := []string{"checkout", branch}
332322
cmd, err := c.Command(ctx, args...)
@@ -354,27 +344,22 @@ func (c *Client) CheckoutNewBranch(ctx context.Context, remoteName, branch strin
354344
return nil
355345
}
356346

347+
func (c *Client) HasLocalBranch(ctx context.Context, branch string) bool {
348+
_, err := c.revParse(ctx, "--verify", "refs/heads/"+branch)
349+
return err == nil
350+
}
351+
357352
// ToplevelDir returns the top-level directory path of the current repository.
358353
func (c *Client) ToplevelDir(ctx context.Context) (string, error) {
359-
args := []string{"rev-parse", "--show-toplevel"}
360-
cmd, err := c.Command(ctx, args...)
361-
if err != nil {
362-
return "", err
363-
}
364-
out, err := cmd.Output()
354+
out, err := c.revParse(ctx, "--show-toplevel")
365355
if err != nil {
366356
return "", err
367357
}
368358
return firstLine(out), nil
369359
}
370360

371361
func (c *Client) GitDir(ctx context.Context) (string, error) {
372-
args := []string{"rev-parse", "--git-dir"}
373-
cmd, err := c.Command(ctx, args...)
374-
if err != nil {
375-
return "", err
376-
}
377-
out, err := cmd.Output()
362+
out, err := c.revParse(ctx, "--git-dir")
378363
if err != nil {
379364
return "", err
380365
}
@@ -383,12 +368,7 @@ func (c *Client) GitDir(ctx context.Context) (string, error) {
383368

384369
// Show current directory relative to the top-level directory of repository.
385370
func (c *Client) PathFromRoot(ctx context.Context) string {
386-
args := []string{"rev-parse", "--show-prefix"}
387-
cmd, err := c.Command(ctx, args...)
388-
if err != nil {
389-
return ""
390-
}
391-
out, err := cmd.Output()
371+
out, err := c.revParse(ctx, "--show-prefix")
392372
if err != nil {
393373
return ""
394374
}
@@ -398,12 +378,20 @@ func (c *Client) PathFromRoot(ctx context.Context) string {
398378
return ""
399379
}
400380

381+
func (c *Client) revParse(ctx context.Context, args ...string) ([]byte, error) {
382+
args = append([]string{"rev-parse"}, args...)
383+
cmd, err := c.Command(ctx, args...)
384+
if err != nil {
385+
return nil, err
386+
}
387+
return cmd.Output()
388+
}
389+
401390
// Below are commands that make network calls and need authentication credentials supplied from gh.
402391

403392
func (c *Client) Fetch(ctx context.Context, remote string, refspec string, mods ...CommandModifier) error {
404393
args := []string{"fetch", remote, refspec}
405-
// TODO: Use AuthenticatedCommand
406-
cmd, err := c.Command(ctx, args...)
394+
cmd, err := c.AuthenticatedCommand(ctx, args...)
407395
if err != nil {
408396
return err
409397
}
@@ -418,8 +406,7 @@ func (c *Client) Pull(ctx context.Context, remote, branch string, mods ...Comman
418406
if remote != "" && branch != "" {
419407
args = append(args, remote, branch)
420408
}
421-
// TODO: Use AuthenticatedCommand
422-
cmd, err := c.Command(ctx, args...)
409+
cmd, err := c.AuthenticatedCommand(ctx, args...)
423410
if err != nil {
424411
return err
425412
}
@@ -431,8 +418,7 @@ func (c *Client) Pull(ctx context.Context, remote, branch string, mods ...Comman
431418

432419
func (c *Client) Push(ctx context.Context, remote string, ref string, mods ...CommandModifier) error {
433420
args := []string{"push", "--set-upstream", remote, ref}
434-
// TODO: Use AuthenticatedCommand
435-
cmd, err := c.Command(ctx, args...)
421+
cmd, err := c.AuthenticatedCommand(ctx, args...)
436422
if err != nil {
437423
return err
438424
}
@@ -453,8 +439,7 @@ func (c *Client) Clone(ctx context.Context, cloneURL string, args []string, mods
453439
target = path.Base(strings.TrimSuffix(cloneURL, ".git"))
454440
}
455441
cloneArgs = append([]string{"clone"}, cloneArgs...)
456-
// TODO: Use AuthenticatedCommand
457-
cmd, err := c.Command(ctx, cloneArgs...)
442+
cmd, err := c.AuthenticatedCommand(ctx, cloneArgs...)
458443
if err != nil {
459444
return "", err
460445
}
@@ -474,8 +459,7 @@ func (c *Client) AddRemote(ctx context.Context, name, urlStr string, trackingBra
474459
args = append(args, "-t", branch)
475460
}
476461
args = append(args, "-f", name, urlStr)
477-
// TODO: Use AuthenticatedCommand
478-
cmd, err := c.Command(ctx, args...)
462+
cmd, err := c.AuthenticatedCommand(ctx, args...)
479463
if err != nil {
480464
return nil, err
481465
}

‎git/client_test.go

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,11 @@ func TestClientAuthenticatedCommand(t *testing.T) {
6767
{
6868
name: "adds credential helper config options",
6969
path: "path/to/gh",
70-
wantArgs: []string{"path/to/git", "-c", "credential.helper=", "-c", "credential.helper=!\"path/to/gh\" auth git-credential", "fetch"},
70+
wantArgs: []string{"path/to/git", "-c", "credential.helper=", "-c", `credential.helper=!"path/to/gh" auth git-credential`, "fetch"},
7171
},
7272
{
7373
name: "fallback when GhPath is not set",
74-
wantArgs: []string{"path/to/git", "-c", "credential.helper=", "-c", "credential.helper=!\"gh\" auth git-credential", "fetch"},
74+
wantArgs: []string{"path/to/git", "-c", "credential.helper=", "-c", `credential.helper=!"gh" auth git-credential`, "fetch"},
7575
},
7676
}
7777
for _, tt := range tests {
@@ -846,18 +846,18 @@ func TestClientFetch(t *testing.T) {
846846
}{
847847
{
848848
name: "fetch",
849-
wantCmdArgs: `path/to/git fetch origin trunk`,
849+
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential fetch origin trunk`,
850850
},
851851
{
852852
name: "accepts command modifiers",
853853
mods: []CommandModifier{WithRepoDir("/path/to/repo")},
854-
wantCmdArgs: `path/to/git -C /path/to/repo fetch origin trunk`,
854+
wantCmdArgs: `path/to/git -C /path/to/repo -c credential.helper= -c credential.helper=!"gh" auth git-credential fetch origin trunk`,
855855
},
856856
{
857857
name: "git error",
858858
cmdExitStatus: 1,
859859
cmdStderr: "git error message",
860-
wantCmdArgs: `path/to/git fetch origin trunk`,
860+
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential fetch origin trunk`,
861861
wantErrorMsg: "failed to run git: git error message",
862862
},
863863
}
@@ -891,18 +891,18 @@ func TestClientPull(t *testing.T) {
891891
}{
892892
{
893893
name: "pull",
894-
wantCmdArgs: `path/to/git pull --ff-only origin trunk`,
894+
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential pull --ff-only origin trunk`,
895895
},
896896
{
897897
name: "accepts command modifiers",
898898
mods: []CommandModifier{WithRepoDir("/path/to/repo")},
899-
wantCmdArgs: `path/to/git -C /path/to/repo pull --ff-only origin trunk`,
899+
wantCmdArgs: `path/to/git -C /path/to/repo -c credential.helper= -c credential.helper=!"gh" auth git-credential pull --ff-only origin trunk`,
900900
},
901901
{
902902
name: "git error",
903903
cmdExitStatus: 1,
904904
cmdStderr: "git error message",
905-
wantCmdArgs: `path/to/git pull --ff-only origin trunk`,
905+
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential pull --ff-only origin trunk`,
906906
wantErrorMsg: "failed to run git: git error message",
907907
},
908908
}
@@ -936,18 +936,18 @@ func TestClientPush(t *testing.T) {
936936
}{
937937
{
938938
name: "push",
939-
wantCmdArgs: `path/to/git push --set-upstream origin trunk`,
939+
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential push --set-upstream origin trunk`,
940940
},
941941
{
942942
name: "accepts command modifiers",
943943
mods: []CommandModifier{WithRepoDir("/path/to/repo")},
944-
wantCmdArgs: `path/to/git -C /path/to/repo push --set-upstream origin trunk`,
944+
wantCmdArgs: `path/to/git -C /path/to/repo -c credential.helper= -c credential.helper=!"gh" auth git-credential push --set-upstream origin trunk`,
945945
},
946946
{
947947
name: "git error",
948948
cmdExitStatus: 1,
949949
cmdStderr: "git error message",
950-
wantCmdArgs: `path/to/git push --set-upstream origin trunk`,
950+
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential push --set-upstream origin trunk`,
951951
wantErrorMsg: "failed to run git: git error message",
952952
},
953953
}
@@ -982,20 +982,20 @@ func TestClientClone(t *testing.T) {
982982
}{
983983
{
984984
name: "clone",
985-
wantCmdArgs: `path/to/git clone github.com/cli/cli`,
985+
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential clone github.com/cli/cli`,
986986
wantTarget: "cli",
987987
},
988988
{
989989
name: "accepts command modifiers",
990990
mods: []CommandModifier{WithRepoDir("/path/to/repo")},
991-
wantCmdArgs: `path/to/git -C /path/to/repo clone github.com/cli/cli`,
991+
wantCmdArgs: `path/to/git -C /path/to/repo -c credential.helper= -c credential.helper=!"gh" auth git-credential clone github.com/cli/cli`,
992992
wantTarget: "cli",
993993
},
994994
{
995995
name: "git error",
996996
cmdExitStatus: 1,
997997
cmdStderr: "git error message",
998-
wantCmdArgs: `path/to/git clone github.com/cli/cli`,
998+
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential clone github.com/cli/cli`,
999999
wantErrorMsg: "failed to run git: git error message",
10001000
},
10011001
}
@@ -1089,15 +1089,15 @@ func TestClientAddRemote(t *testing.T) {
10891089
url: "URL",
10901090
dir: "DIRECTORY",
10911091
branches: []string{},
1092-
wantCmdArgs: `path/to/git -C DIRECTORY remote add -f test URL`,
1092+
wantCmdArgs: `path/to/git -C DIRECTORY -c credential.helper= -c credential.helper=!"gh" auth git-credential remote add -f test URL`,
10931093
},
10941094
{
10951095
title: "fetch specific branches only",
10961096
name: "test",
10971097
url: "URL",
10981098
dir: "DIRECTORY",
10991099
branches: []string{"trunk", "dev"},
1100-
wantCmdArgs: `path/to/git -C DIRECTORY remote add -t trunk -t dev -f test URL`,
1100+
wantCmdArgs: `path/to/git -C DIRECTORY -c credential.helper= -c credential.helper=!"gh" auth git-credential remote add -t trunk -t dev -f test URL`,
11011101
},
11021102
}
11031103
for _, tt := range tests {

‎internal/run/stub.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ import (
88
"strings"
99
)
1010

11+
const (
12+
gitAuthRE = `-c credential.helper= -c credential.helper=!"[^"]+" auth git-credential `
13+
)
14+
1115
type T interface {
1216
Helper()
1317
Errorf(string, ...interface{})
@@ -71,6 +75,9 @@ func (cs *CommandStubber) Register(pattern string, exitStatus int, output string
7175
if len(pattern) < 1 {
7276
panic("cannot use empty regexp pattern")
7377
}
78+
if strings.HasPrefix(pattern, "git") {
79+
pattern = addGitAuthentication(pattern)
80+
}
7481
cs.stubs = append(cs.stubs, &commandStub{
7582
pattern: regexp.MustCompile(pattern),
7683
exitStatus: exitStatus,
@@ -114,3 +121,13 @@ func (s *commandStub) Output() ([]byte, error) {
114121
}
115122
return []byte(s.stdout), nil
116123
}
124+
125+
// Inject git authentication string for specific git commands.
126+
func addGitAuthentication(s string) string {
127+
pattern := regexp.MustCompile(`( fetch | pull | push | clone | remote add.+-f | submodule )`)
128+
loc := pattern.FindStringIndex(s)
129+
if loc == nil {
130+
return s
131+
}
132+
return s[:loc[0]+1] + gitAuthRE + s[loc[0]+1:]
133+
}

‎pkg/cmd/gist/clone/clone_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package clone
22

33
import (
44
"net/http"
5-
"strings"
65
"testing"
76

87
"github.com/cli/cli/v2/git"
@@ -26,7 +25,10 @@ func runCloneCommand(httpClient *http.Client, cli string) (*test.CmdOut, error)
2625
Config: func() (config.Config, error) {
2726
return config.NewBlankConfig(), nil
2827
},
29-
GitClient: &git.Client{GitPath: "some/path/git"},
28+
GitClient: &git.Client{
29+
GhPath: "some/path/gh",
30+
GitPath: "some/path/git",
31+
},
3032
}
3133

3234
cmd := NewCmdClone(fac, nil)
@@ -97,9 +99,7 @@ func Test_GistClone(t *testing.T) {
9799

98100
cs, restore := run.Stub()
99101
defer restore(t)
100-
cs.Register(`git clone`, 0, "", func(s []string) {
101-
assert.Equal(t, tt.want, strings.Join(s, " "))
102-
})
102+
cs.Register(tt.want, 0, "")
103103

104104
output, err := runCloneCommand(httpClient, tt.args)
105105
if err != nil {

‎pkg/cmd/issue/develop/develop_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,10 @@ func Test_developRun(t *testing.T) {
569569
return remotes, nil
570570
}
571571

572-
opts.GitClient = &git.Client{GitPath: "some/path/git"}
572+
opts.GitClient = &git.Client{
573+
GhPath: "some/path/gh",
574+
GitPath: "some/path/git",
575+
}
573576

574577
cmdStubs, cmdTeardown := run.Stub()
575578
defer cmdTeardown(t)

‎pkg/cmd/pr/checkout/checkout.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,13 @@ func localBranchExists(client *git.Client, b string) bool {
242242

243243
func executeCmds(client *git.Client, cmdQueue [][]string) error {
244244
for _, args := range cmdQueue {
245-
// TODO: Use AuthenticatedCommand
246-
cmd, err := client.Command(context.Background(), args...)
245+
var err error
246+
var cmd *git.Command
247+
if args[0] == "fetch" || args[0] == "submodule" {
248+
cmd, err = client.AuthenticatedCommand(context.Background(), args...)
249+
} else {
250+
cmd, err = client.Command(context.Background(), args...)
251+
}
247252
if err != nil {
248253
return err
249254
}

‎pkg/cmd/pr/checkout/checkout_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,10 @@ func Test_checkoutRun(t *testing.T) {
197197
return remotes, nil
198198
}
199199

200-
opts.GitClient = &git.Client{GitPath: "some/path/git"}
200+
opts.GitClient = &git.Client{
201+
GhPath: "some/path/gh",
202+
GitPath: "some/path/git",
203+
}
201204

202205
err := checkoutRun(opts)
203206
if (err != nil) != tt.wantErr {
@@ -236,7 +239,10 @@ func runCommand(rt http.RoundTripper, remotes context.Remotes, branch string, cl
236239
Branch: func() (string, error) {
237240
return branch, nil
238241
},
239-
GitClient: &git.Client{GitPath: "some/path/git"},
242+
GitClient: &git.Client{
243+
GhPath: "some/path/gh",
244+
GitPath: "some/path/git",
245+
},
240246
}
241247

242248
cmd := NewCmdCheckout(factory, nil)

‎pkg/cmd/pr/create/create_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,10 @@ func Test_createRun(t *testing.T) {
894894
return branch, nil
895895
}
896896
opts.Finder = shared.NewMockFinder(branch, nil, nil)
897-
opts.GitClient = &git.Client{GitPath: "some/path/git"}
897+
opts.GitClient = &git.Client{
898+
GhPath: "some/path/gh",
899+
GitPath: "some/path/git",
900+
}
898901
cleanSetup := func() {}
899902
if tt.setup != nil {
900903
cleanSetup = tt.setup(&opts, t)
@@ -1011,7 +1014,10 @@ func Test_determineTrackingBranch(t *testing.T) {
10111014

10121015
tt.cmdStubs(cs)
10131016

1014-
gitClient := &git.Client{GitPath: "some/path/git"}
1017+
gitClient := &git.Client{
1018+
GhPath: "some/path/gh",
1019+
GitPath: "some/path/git",
1020+
}
10151021
ref := determineTrackingBranch(gitClient, tt.remotes, "feature")
10161022
tt.assert(ref, t)
10171023
})

0 commit comments

Comments
 (0)