Merge pull request cli#3075 from cli/credential-helper-absolute

Use absolute path when configuring gh as git credential

cmd/gh/main.go

@@ -188,10 +188,7 @@ func mainRun() exitCode {
 
 	newRelease := <-updateMessageChan
 	if newRelease != nil {
-		isHomebrew := false
-		if ghExe, err := os.Executable(); err == nil {
-			isHomebrew = isUnderHomebrew(ghExe)
-		}
+		isHomebrew := isUnderHomebrew(cmdFactory.Executable)
 		if isHomebrew && isRecentRelease(newRelease.PublishedAt) {
 			// do not notify Homebrew users before the version bump had a chance to get merged into homebrew-core
 			return exitOK

pkg/cmd/auth/login/login.go

@@ -23,6 +23,8 @@ type LoginOptions struct {
 	Config     func() (config.Config, error)
 	HttpClient func() (*http.Client, error)
 
+	MainExecutable string
+
 	Interactive bool
 
 	Hostname string
@@ -36,6 +38,8 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
 		IO:         f.IOStreams,
 		Config:     f.Config,
 		HttpClient: f.HttpClient,
+
+		MainExecutable: f.Executable,
 	}
 
 	var tokenStdin bool
@@ -189,6 +193,7 @@ func loginRun(opts *LoginOptions) error {
 		Interactive: opts.Interactive,
 		Web:         opts.Web,
 		Scopes:      opts.Scopes,
+		Executable:  opts.MainExecutable,
 	})
 }
 

pkg/cmd/auth/refresh/refresh.go

@@ -19,6 +19,8 @@ type RefreshOptions struct {
 	IO     *iostreams.IOStreams
 	Config func() (config.Config, error)
 
+	MainExecutable string
+
 	Hostname string
 	Scopes   []string
 	AuthFlow func(config.Config, *iostreams.IOStreams, string, []string) error
@@ -34,6 +36,7 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra.
 			_, err := authflow.AuthFlowWithConfig(cfg, io, hostname, "", scopes)
 			return err
 		},
+		MainExecutable: f.Executable,
 	}
 
 	cmd := &cobra.Command{

pkg/cmd/auth/shared/git_credential.go

@@ -15,6 +15,8 @@ import (
 )
 
 type GitCredentialFlow struct {
+	Executable string
+
 	shouldSetup bool
 	helper      string
 	scopes      []string
@@ -50,13 +52,26 @@ func (flow *GitCredentialFlow) ShouldSetup() bool {
 }
 
 func (flow *GitCredentialFlow) Setup(hostname, username, authToken string) error {
-	return GitCredentialSetup(hostname, username, authToken, flow.helper)
+	return flow.gitCredentialSetup(hostname, username, authToken)
 }
 
-func GitCredentialSetup(hostname, username, password, helper string) error {
-	if helper == "" {
+func (flow *GitCredentialFlow) gitCredentialSetup(hostname, username, password string) error {
+	if flow.helper == "" {
+		// first use a blank value to indicate to git we want to sever the chain of credential helpers
+		preConfigureCmd, err := git.GitCommand("config", "--global", gitCredentialHelperKey(hostname), "")
+		if err != nil {
+			return err
+		}
+		if err = run.PrepareCmd(preConfigureCmd).Run(); err != nil {
+			return err
+		}
+
 		// use GitHub CLI as a credential helper (for this host only)
-		configureCmd, err := git.GitCommand("config", "--global", gitCredentialHelperKey(hostname), "!gh auth git-credential")
+		configureCmd, err := git.GitCommand(
+			"config", "--global", "--add",
+			gitCredentialHelperKey(hostname),
+			fmt.Sprintf("!%s auth git-credential", shellQuote(flow.Executable)),
+		)
 		if err != nil {
 			return err
 		}
@@ -124,3 +139,10 @@ func isOurCredentialHelper(cmd string) bool {
 
 	return strings.TrimSuffix(filepath.Base(args[0]), ".exe") == "gh"
 }
+
+func shellQuote(s string) string {
+	if strings.ContainsAny(s, " $") {
+		return "'" + s + "'"
+	}
+	return s
+}

pkg/cmd/auth/shared/git_credential_test.go

@@ -12,21 +12,42 @@ func TestGitCredentialSetup_configureExisting(t *testing.T) {
 	cs.Register(`git credential reject`, 0, "")
 	cs.Register(`git credential approve`, 0, "")
 
-	if err := GitCredentialSetup("example.com", "monalisa", "PASSWD", "osxkeychain"); err != nil {
+	f := GitCredentialFlow{
+		Executable: "gh",
+		helper:     "osxkeychain",
+	}
+
+	if err := f.gitCredentialSetup("example.com", "monalisa", "PASSWD"); err != nil {
 		t.Errorf("GitCredentialSetup() error = %v", err)
 	}
 }
 
 func TestGitCredentialSetup_setOurs(t *testing.T) {
 	cs, restoreRun := run.Stub()
 	defer restoreRun(t)
-	cs.Register(`git config --global credential\.https://example\.com\.helper`, 0, "", func(args []string) {
-		if val := args[len(args)-1]; val != "!gh auth git-credential" {
+	cs.Register(`git config --global credential\.`, 0, "", func(args []string) {
+		if key := args[len(args)-2]; key != "credential.https://example.com.helper" {
+			t.Errorf("git config key was %q", key)
+		}
+		if val := args[len(args)-1]; val != "" {
 			t.Errorf("global credential helper configured to %q", val)
 		}
 	})
+	cs.Register(`git config --global --add credential\.`, 0, "", func(args []string) {
+		if key := args[len(args)-2]; key != "credential.https://example.com.helper" {
+			t.Errorf("git config key was %q", key)
+		}
+		if val := args[len(args)-1]; val != "!/path/to/gh auth git-credential" {
+			t.Errorf("global credential helper configured to %q", val)
+		}
+	})
+
+	f := GitCredentialFlow{
+		Executable: "/path/to/gh",
+		helper:     "",
+	}
 
-	if err := GitCredentialSetup("example.com", "monalisa", "PASSWD", ""); err != nil {
+	if err := f.gitCredentialSetup("example.com", "monalisa", "PASSWD"); err != nil {
 		t.Errorf("GitCredentialSetup() error = %v", err)
 	}
 }

pkg/cmd/auth/shared/login_flow.go

@@ -28,6 +28,7 @@ type LoginOptions struct {
 	Interactive bool
 	Web         bool
 	Scopes      []string
+	Executable  string
 
 	sshContext sshContext
 }
@@ -56,7 +57,7 @@ func Login(opts *LoginOptions) error {
 
 	var additionalScopes []string
 
-	credentialFlow := &GitCredentialFlow{}
+	credentialFlow := &GitCredentialFlow{Executable: opts.Executable}
 	if opts.Interactive && gitProtocol == "https" {
 		if err := credentialFlow.Prompt(hostname); err != nil {
 			return err

pkg/cmd/factory/default.go

@@ -44,6 +44,11 @@ func New(appVersion string) *cmdutil.Factory {
 	}
 	remotesFunc := rr.Resolver(hostOverride)
 
+	ghExecutable := "gh"
+	if exe, err := os.Executable(); err == nil {
+		ghExecutable = exe
+	}
+
 	return &cmdutil.Factory{
 		IOStreams: io,
 		Config:    configFunc,
@@ -70,5 +75,6 @@ func New(appVersion string) *cmdutil.Factory {
 			}
 			return currentBranch, nil
 		},
+		Executable: ghExecutable,
 	}
 }

pkg/cmdutil/factory.go

@@ -16,4 +16,7 @@ type Factory struct {
 	Remotes    func() (context.Remotes, error)
 	Config     func() (config.Config, error)
 	Branch     func() (string, error)
+
+	// Executable is the path to the currently invoked gh binary
+	Executable string
 }