STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge commit from fork

Browse source 
Use only one pattern for credential helper matching
This commit is contained in:
Andy Feller 2024-11-27 15:57:08 -05:00 • committed by GitHub
commit 1fe14c956d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 401 additions and 92 deletions
Download patch file Download diff file Expand all files Collapse all files

84
git/client.go
View file

@ -16,6 +16,7 @@ import (
"strings"
"sync"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/safeexec"
)
@ -94,16 +95,65 @@ func (c *Client) Command(ctx context.Context, args ...string) (*Command, error)
return &Command{cmd}, nil
}
// CredentialPattern is used to inform AuthenticatedCommand which patterns Git should match
// against when trying to find credentials. It is a little over-engineered as a type because we
// want AuthenticatedCommand to have a clear compilation error when this is not provided,
// as opposed to using a string which might compile with `client.AuthenticatedCommand(ctx, "fetch")`.
//
// It is only usable when constructed by another function in the package because the empty pattern,
// without allMatching set to true, will result in an error in AuthenticatedCommand.
//
// Callers can currently opt-in to an slightly less secure mode for backwards compatibility by using
// AllMatchingCredentialsPattern.
type CredentialPattern struct {
allMatching bool // should only be constructable via AllMatchingCredentialsPattern
pattern string
}
// AllMatchingCredentialsPattern allows for setting gh as credential helper for all hosts.
// However, we should endeavour to remove it as it's less secure.
var AllMatchingCredentialsPattern = CredentialPattern{allMatching: true, pattern: ""}
var disallowedCredentialPattern = CredentialPattern{allMatching: false, pattern: ""}
// WM-TODO: Are there any funny remotes that might not resolve to a URL?
func CredentialPatternFromRemote(ctx context.Context, c *Client, remote string) (CredentialPattern, error) {
gitURL, err := c.GetRemoteURL(ctx, remote)
if err != nil {
return CredentialPattern{}, err
}
return CredentialPatternFromGitURL(gitURL)
}
func CredentialPatternFromGitURL(gitURL string) (CredentialPattern, error) {
normalizedURL, err := ParseURL(gitURL)
if err != nil {
return CredentialPattern{}, fmt.Errorf("failed to parse remote URL: %w", err)
}
return CredentialPattern{
pattern: strings.TrimSuffix(ghinstance.HostPrefix(normalizedURL.Host), "/"),
}, nil
}
// AuthenticatedCommand is a wrapper around Command that included configuration to use gh
// as the credential helper for git.
func (c *Client) AuthenticatedCommand(ctx context.Context, args ...string) (*Command, error) {
preArgs := []string{"-c", "credential.helper="}
func (c *Client) AuthenticatedCommand(ctx context.Context, credentialPattern CredentialPattern, args ...string) (*Command, error) {
if c.GhPath == "" {
// Assumes that gh is in PATH.
c.GhPath = "gh"
}
credHelper := fmt.Sprintf("!%q auth git-credential", c.GhPath)
preArgs = append(preArgs, "-c", fmt.Sprintf("credential.helper=%s", credHelper))
var preArgs []string
if credentialPattern == disallowedCredentialPattern {
return nil, fmt.Errorf("empty credential pattern is not allowed unless provided explicitly")
} else if credentialPattern == AllMatchingCredentialsPattern {
preArgs = []string{"-c", "credential.helper="}
preArgs = append(preArgs, "-c", fmt.Sprintf("credential.helper=%s", credHelper))
} else {
preArgs = []string{"-c", fmt.Sprintf("credential.%s.helper=", credentialPattern.pattern)}
preArgs = append(preArgs, "-c", fmt.Sprintf("credential.%s.helper=%s", credentialPattern.pattern, credHelper))
}
args = append(preArgs, args...)
return c.Command(ctx, args...)
}
@ -152,6 +202,19 @@ func (c *Client) UpdateRemoteURL(ctx context.Context, name, url string) error {
return nil
}
func (c *Client) GetRemoteURL(ctx context.Context, name string) (string, error) {
args := []string{"remote", "get-url", name}
cmd, err := c.Command(ctx, args...)
if err != nil {
return "", err
}
out, err := cmd.Output()
if err != nil {
return "", err
}
return strings.TrimSpace(string(out)), nil
}
func (c *Client) SetRemoteResolution(ctx context.Context, name, resolution string) error {
args := []string{"config", "--add", fmt.Sprintf("remote.%s.gh-resolved", name), resolution}
cmd, err := c.Command(ctx, args...)
@ -549,7 +612,7 @@ func (c *Client) Fetch(ctx context.Context, remote string, refspec string, mods
if refspec != "" {
args = append(args, refspec)
}
cmd, err := c.AuthenticatedCommand(ctx, args...)
cmd, err := c.AuthenticatedCommand(ctx, AllMatchingCredentialsPattern, args...)
if err != nil {
return err
}
@ -564,7 +627,7 @@ func (c *Client) Pull(ctx context.Context, remote, branch string, mods ...Comman
if remote != "" && branch != "" {
args = append(args, remote, branch)
}
cmd, err := c.AuthenticatedCommand(ctx, args...)
cmd, err := c.AuthenticatedCommand(ctx, AllMatchingCredentialsPattern, args...)
if err != nil {
return err
}
@ -576,7 +639,7 @@ func (c *Client) Pull(ctx context.Context, remote, branch string, mods ...Comman
func (c *Client) Push(ctx context.Context, remote string, ref string, mods ...CommandModifier) error {
args := []string{"push", "--set-upstream", remote, ref}
cmd, err := c.AuthenticatedCommand(ctx, args...)
cmd, err := c.AuthenticatedCommand(ctx, AllMatchingCredentialsPattern, args...)
if err != nil {
return err
}
@ -587,6 +650,13 @@ func (c *Client) Push(ctx context.Context, remote string, ref string, mods ...Co
}
func (c *Client) Clone(ctx context.Context, cloneURL string, args []string, mods ...CommandModifier) (string, error) {
// Note that even if this is an SSH clone URL, we are setting the pattern anyway.
// We could write some code to prevent this, but it also doesn't seem harmful.
pattern, err := CredentialPatternFromGitURL(cloneURL)
if err != nil {
return "", err
}
cloneArgs, target := parseCloneArgs(args)
cloneArgs = append(cloneArgs, cloneURL)
// If the args contain an explicit target, pass it to clone otherwise,
@ -601,7 +671,7 @@ func (c *Client) Clone(ctx context.Context, cloneURL string, args []string, mods
}
}
cloneArgs = append([]string{"clone"}, cloneArgs...)
cmd, err := c.AuthenticatedCommand(ctx, cloneArgs...)
cmd, err := c.AuthenticatedCommand(ctx, pattern, cloneArgs...)
if err != nil {
return "", err
}

335
git/client_test.go
View file

@ -64,17 +64,32 @@ func TestClientAuthenticatedCommand(t *testing.T) {
tests := []struct {
name string
path string
pattern CredentialPattern
wantArgs []string
wantErr error
}{
{
name: "adds credential helper config options",
name: "when credential pattern allows for anything, credential helper matches everything",
path: "path/to/gh",
pattern: AllMatchingCredentialsPattern,
wantArgs: []string{"path/to/git", "-c", "credential.helper=", "-c", `credential.helper=!"path/to/gh" auth git-credential`, "fetch"},
},
{
name: "when credential pattern is set, credential helper only matches that pattern",
path: "path/to/gh",
pattern: CredentialPattern{pattern: "https://github.com"},
wantArgs: []string{"path/to/git", "-c", "credential.https://github.com.helper=", "-c", `credential.https://github.com.helper=!"path/to/gh" auth git-credential`, "fetch"},
},
{
name: "fallback when GhPath is not set",
pattern: AllMatchingCredentialsPattern,
wantArgs: []string{"path/to/git", "-c", "credential.helper=", "-c", `credential.helper=!"gh" auth git-credential`, "fetch"},
},
{
name: "errors when attempting to use an empty pattern that isn't marked all matching",
pattern: CredentialPattern{allMatching: false, pattern: ""},
wantErr: fmt.Errorf("empty credential pattern is not allowed unless provided explicitly"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -82,9 +97,12 @@ func TestClientAuthenticatedCommand(t *testing.T) {
GhPath: tt.path,
GitPath: "path/to/git",
}
cmd, err := client.AuthenticatedCommand(context.Background(), "fetch")
assert.NoError(t, err)
assert.Equal(t, tt.wantArgs, cmd.Args)
cmd, err := client.AuthenticatedCommand(context.Background(), tt.pattern, "fetch")
if tt.wantErr != nil {
require.Equal(t, tt.wantErr, err)
return
}
require.Equal(t, tt.wantArgs, cmd.Args)
})
}
}
@ -1131,44 +1149,52 @@ func TestClientSetRemoteBranches(t *testing.T) {
func TestClientFetch(t *testing.T) {
tests := []struct {
name string
mods []CommandModifier
cmdExitStatus int
cmdStdout string
cmdStderr string
wantCmdArgs string
wantErrorMsg string
name string
mods []CommandModifier
commands mockedCommands
wantErrorMsg string
}{
{
name: "fetch",
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential fetch origin trunk`,
name: "fetch",
commands: map[args]commandResult{
`path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential fetch origin trunk`: {
ExitStatus: 0,
},
},
},
{
name: "accepts command modifiers",
mods: []CommandModifier{WithRepoDir("/path/to/repo")},
wantCmdArgs: `path/to/git -C /path/to/repo -c credential.helper= -c credential.helper=!"gh" auth git-credential fetch origin trunk`,
name: "accepts command modifiers",
mods: []CommandModifier{WithRepoDir("/path/to/repo")},
commands: map[args]commandResult{
`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",
cmdExitStatus: 1,
cmdStderr: "git error message",
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential fetch origin trunk`,
wantErrorMsg: "failed to run git: git error message",
name: "git error on fetch",
commands: map[args]commandResult{
`path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential fetch origin trunk`: {
ExitStatus: 1,
Stderr: "fetch error message",
},
},
wantErrorMsg: "failed to run git: fetch error message",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cmd, cmdCtx := createCommandContext(t, tt.cmdExitStatus, tt.cmdStdout, tt.cmdStderr)
cmdCtx := createMockedCommandContext(t, tt.commands)
client := Client{
GitPath: "path/to/git",
commandContext: cmdCtx,
}
err := client.Fetch(context.Background(), "origin", "trunk", tt.mods...)
assert.Equal(t, tt.wantCmdArgs, strings.Join(cmd.Args[3:], " "))
if tt.wantErrorMsg == "" {
assert.NoError(t, err)
require.NoError(t, err)
} else {
assert.EqualError(t, err, tt.wantErrorMsg)
require.EqualError(t, err, tt.wantErrorMsg)
}
})
}
@ -1176,44 +1202,52 @@ func TestClientFetch(t *testing.T) {
func TestClientPull(t *testing.T) {
tests := []struct {
name string
mods []CommandModifier
cmdExitStatus int
cmdStdout string
cmdStderr string
wantCmdArgs string
wantErrorMsg string
name string
mods []CommandModifier
commands mockedCommands
wantErrorMsg string
}{
{
name: "pull",
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential pull --ff-only origin trunk`,
name: "pull",
commands: map[args]commandResult{
`path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential pull --ff-only origin trunk`: {
ExitStatus: 0,
},
},
},
{
name: "accepts command modifiers",
mods: []CommandModifier{WithRepoDir("/path/to/repo")},
wantCmdArgs: `path/to/git -C /path/to/repo -c credential.helper= -c credential.helper=!"gh" auth git-credential pull --ff-only origin trunk`,
name: "accepts command modifiers",
mods: []CommandModifier{WithRepoDir("/path/to/repo")},
commands: map[args]commandResult{
`path/to/git -C /path/to/repo -c credential.helper= -c credential.helper=!"gh" auth git-credential pull --ff-only origin trunk`: {
ExitStatus: 0,
},
},
},
{
name: "git error",
cmdExitStatus: 1,
cmdStderr: "git error message",
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential pull --ff-only origin trunk`,
wantErrorMsg: "failed to run git: git error message",
name: "git error on pull",
commands: map[args]commandResult{
`path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential pull --ff-only origin trunk`: {
ExitStatus: 1,
Stderr: "pull error message",
},
},
wantErrorMsg: "failed to run git: pull error message",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cmd, cmdCtx := createCommandContext(t, tt.cmdExitStatus, tt.cmdStdout, tt.cmdStderr)
cmdCtx := createMockedCommandContext(t, tt.commands)
client := Client{
GitPath: "path/to/git",
commandContext: cmdCtx,
}
err := client.Pull(context.Background(), "origin", "trunk", tt.mods...)
assert.Equal(t, tt.wantCmdArgs, strings.Join(cmd.Args[3:], " "))
if tt.wantErrorMsg == "" {
assert.NoError(t, err)
require.NoError(t, err)
} else {
assert.EqualError(t, err, tt.wantErrorMsg)
require.EqualError(t, err, tt.wantErrorMsg)
}
})
}
@ -1221,44 +1255,52 @@ func TestClientPull(t *testing.T) {
func TestClientPush(t *testing.T) {
tests := []struct {
name string
mods []CommandModifier
cmdExitStatus int
cmdStdout string
cmdStderr string
wantCmdArgs string
wantErrorMsg string
name string
mods []CommandModifier
commands mockedCommands
wantErrorMsg string
}{
{
name: "push",
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential push --set-upstream origin trunk`,
name: "push",
commands: map[args]commandResult{
`path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential push --set-upstream origin trunk`: {
ExitStatus: 0,
},
},
},
{
name: "accepts command modifiers",
mods: []CommandModifier{WithRepoDir("/path/to/repo")},
wantCmdArgs: `path/to/git -C /path/to/repo -c credential.helper= -c credential.helper=!"gh" auth git-credential push --set-upstream origin trunk`,
name: "accepts command modifiers",
mods: []CommandModifier{WithRepoDir("/path/to/repo")},
commands: map[args]commandResult{
`path/to/git -C /path/to/repo -c credential.helper= -c credential.helper=!"gh" auth git-credential push --set-upstream origin trunk`: {
ExitStatus: 0,
},
},
},
{
name: "git error",
cmdExitStatus: 1,
cmdStderr: "git error message",
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential push --set-upstream origin trunk`,
wantErrorMsg: "failed to run git: git error message",
name: "git error on push",
commands: map[args]commandResult{
`path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential push --set-upstream origin trunk`: {
ExitStatus: 1,
Stderr: "push error message",
},
},
wantErrorMsg: "failed to run git: push error message",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cmd, cmdCtx := createCommandContext(t, tt.cmdExitStatus, tt.cmdStdout, tt.cmdStderr)
cmdCtx := createMockedCommandContext(t, tt.commands)
client := Client{
GitPath: "path/to/git",
commandContext: cmdCtx,
}
err := client.Push(context.Background(), "origin", "trunk", tt.mods...)
assert.Equal(t, tt.wantCmdArgs, strings.Join(cmd.Args[3:], " "))
if tt.wantErrorMsg == "" {
assert.NoError(t, err)
require.NoError(t, err)
} else {
assert.EqualError(t, err, tt.wantErrorMsg)
require.EqualError(t, err, tt.wantErrorMsg)
}
})
}
@ -1279,14 +1321,14 @@ func TestClientClone(t *testing.T) {
{
name: "clone",
args: []string{},
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential clone github.com/cli/cli`,
wantCmdArgs: `path/to/git -c credential.https://github.com.helper= -c credential.https://github.com.helper=!"gh" auth git-credential clone https://github.com/cli/cli`,
wantTarget: "cli",
},
{
name: "accepts command modifiers",
args: []string{},
mods: []CommandModifier{WithRepoDir("/path/to/repo")},
wantCmdArgs: `path/to/git -C /path/to/repo -c credential.helper= -c credential.helper=!"gh" auth git-credential clone github.com/cli/cli`,
wantCmdArgs: `path/to/git -C /path/to/repo -c credential.https://github.com.helper= -c credential.https://github.com.helper=!"gh" auth git-credential clone https://github.com/cli/cli`,
wantTarget: "cli",
},
{
@ -1294,19 +1336,19 @@ func TestClientClone(t *testing.T) {
args: []string{},
cmdExitStatus: 1,
cmdStderr: "git error message",
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential clone github.com/cli/cli`,
wantCmdArgs: `path/to/git -c credential.https://github.com.helper= -c credential.https://github.com.helper=!"gh" auth git-credential clone https://github.com/cli/cli`,
wantErrorMsg: "failed to run git: git error message",
},
{
name: "bare clone",
args: []string{"--bare"},
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential clone --bare github.com/cli/cli`,
wantCmdArgs: `path/to/git -c credential.https://github.com.helper= -c credential.https://github.com.helper=!"gh" auth git-credential clone --bare https://github.com/cli/cli`,
wantTarget: "cli.git",
},
{
name: "bare clone with explicit target",
args: []string{"cli-bare", "--bare"},
wantCmdArgs: `path/to/git -c credential.helper= -c credential.helper=!"gh" auth git-credential clone --bare github.com/cli/cli cli-bare`,
wantCmdArgs: `path/to/git -c credential.https://github.com.helper= -c credential.https://github.com.helper=!"gh" auth git-credential clone --bare https://github.com/cli/cli cli-bare`,
wantTarget: "cli-bare",
},
}
@ -1317,7 +1359,7 @@ func TestClientClone(t *testing.T) {
GitPath: "path/to/git",
commandContext: cmdCtx,
}
target, err := client.Clone(context.Background(), "github.com/cli/cli", tt.args, tt.mods...)
target, err := client.Clone(context.Background(), "https://github.com/cli/cli", tt.args, tt.mods...)
assert.Equal(t, tt.wantCmdArgs, strings.Join(cmd.Args[3:], " "))
if tt.wantErrorMsg == "" {
assert.NoError(t, err)
@ -1442,6 +1484,54 @@ func initRepo(t *testing.T, dir string) {
assert.NoError(t, err)
}
type args string
type commandResult struct {
ExitStatus int `json:"exitStatus"`
Stdout string `json:"out"`
Stderr string `json:"err"`
}
type mockedCommands map[args]commandResult
// TestCommandMocking is an invoked test helper that emulates expected behavior for predefined shell commands, erroring when unexpected conditions are encountered.
func TestCommandMocking(t *testing.T) {
if os.Getenv("GH_WANT_HELPER_PROCESS_RICH") != "1" {
return
}
jsonVar, ok := os.LookupEnv("GH_HELPER_PROCESS_RICH_COMMANDS")
if !ok {
fmt.Fprint(os.Stderr, "missing GH_HELPER_PROCESS_RICH_COMMANDS")
os.Exit(1)
}
var commands mockedCommands
if err := json.Unmarshal([]byte(jsonVar), &commands); err != nil {
fmt.Fprint(os.Stderr, "failed to unmarshal GH_HELPER_PROCESS_RICH_COMMANDS")
os.Exit(1)
}
// The discarded args are those for the go test binary itself, e.g. `-test.run=TestHelperProcessRich`
realArgs := os.Args[3:]
commandResult, ok := commands[args(strings.Join(realArgs, " "))]
if !ok {
fmt.Fprintf(os.Stderr, "unexpected command: %s\n", strings.Join(realArgs, " "))
os.Exit(1)
}
if commandResult.Stdout != "" {
fmt.Fprint(os.Stdout, commandResult.Stdout)
}
if commandResult.Stderr != "" {
fmt.Fprint(os.Stderr, commandResult.Stderr)
}
os.Exit(commandResult.ExitStatus)
}
func TestHelperProcess(t *testing.T) {
if os.Getenv("GH_WANT_HELPER_PROCESS") != "1" {
return
@ -1465,6 +1555,87 @@ func TestHelperProcess(t *testing.T) {
os.Exit(0)
}
func TestCredentialPatternFromGitURL(t *testing.T) {
tests := []struct {
name string
gitURL string
wantErr bool
wantCredentialPattern CredentialPattern
}{
{
name: "Given a well formed gitURL, it returns the corresponding CredentialPattern",
gitURL: "https://github.com/OWNER/REPO",
wantCredentialPattern: CredentialPattern{
pattern: "https://github.com",
allMatching: false,
},
},
{
name: "Given a malformed gitURL, it returns an error",
// This pattern is copied from the tests in ParseURL
// Unexpectedly, a non URL-like string did not error in ParseURL
gitURL: "ssh://git@[/tmp/git-repo",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
credentialPattern, err := CredentialPatternFromGitURL(tt.gitURL)
if tt.wantErr {
assert.ErrorContains(t, err, "failed to parse remote URL")
} else {
assert.NoError(t, err)
assert.Equal(t, tt.wantCredentialPattern, credentialPattern)
}
})
}
}
func TestCredentialPatternFromRemote(t *testing.T) {
tests := []struct {
name string
remote string
wantCredentialPattern CredentialPattern
wantErr bool
}{
{
name: "Given a well formed remote, it returns the corresponding CredentialPattern",
remote: "https://github.com/OWNER/REPO",
wantCredentialPattern: CredentialPattern{
pattern: "https://github.com",
allMatching: false,
},
},
{
name: "Given an error from GetRemoteURL, it returns that error",
remote: "foo remote",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var cmdCtx func(ctx context.Context, name string, args ...string) *exec.Cmd
if tt.wantErr {
_, cmdCtx = createCommandContext(t, 1, tt.remote, "GetRemoteURL error")
} else {
_, cmdCtx = createCommandContext(t, 0, tt.remote, "")
}
client := Client{
GitPath: "path/to/git",
commandContext: cmdCtx,
}
credentialPattern, err := CredentialPatternFromRemote(context.Background(), &client, tt.remote)
if tt.wantErr {
assert.ErrorContains(t, err, "GetRemoteURL error")
} else {
assert.NoError(t, err)
assert.Equal(t, tt.wantCredentialPattern, credentialPattern)
}
})
}
}
func createCommandContext(t *testing.T, exitStatus int, stdout, stderr string) (*exec.Cmd, commandCtx) {
cmd := exec.CommandContext(context.Background(), os.Args[0], "-test.run=TestHelperProcess", "--")
cmd.Env = []string{
@ -1479,3 +1650,21 @@ func createCommandContext(t *testing.T, exitStatus int, stdout, stderr string) (
return cmd
}
}
func createMockedCommandContext(t *testing.T, commands mockedCommands) commandCtx {
marshaledCommands, err := json.Marshal(commands)
require.NoError(t, err)
// invokes helper within current test binary, emulating desired behavior
return func(ctx context.Context, exe string, args ...string) *exec.Cmd {
cmd := exec.CommandContext(context.Background(), os.Args[0], "-test.run=TestCommandMocking", "--")
cmd.Env = []string{
"GH_WANT_HELPER_PROCESS_RICH=1",
fmt.Sprintf("GH_HELPER_PROCESS_RICH_COMMANDS=%s", string(marshaledCommands)),
}
cmd.Args = append(cmd.Args, exe)
cmd.Args = append(cmd.Args, args...)
return cmd
}
}

2
go.mod
View file

@ -11,7 +11,7 @@ require (
github.com/cenkalti/backoff/v4 v4.3.0
github.com/charmbracelet/glamour v0.7.0
github.com/charmbracelet/lipgloss v0.10.1-0.20240413172830-d0be07ea6b9c
github.com/cli/go-gh/v2 v2.11.0
github.com/cli/go-gh/v2 v2.11.1
github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24
github.com/cli/oauth v1.1.1
github.com/cli/safeexec v1.0.1

4
go.sum
View file

@ -95,8 +95,8 @@ github.com/charmbracelet/x/exp/term v0.0.0-20240425164147-ba2a9512b05f/go.mod h1
github.com/cli/browser v1.0.0/go.mod h1:IEWkHYbLjkhtjwwWlwTHW2lGxeS5gezEQBMLTwDHf5Q=
github.com/cli/browser v1.3.0 h1:LejqCrpWr+1pRqmEPDGnTZOjsMe7sehifLynZJuqJpo=
github.com/cli/browser v1.3.0/go.mod h1:HH8s+fOAxjhQoBUAsKuPCbqUuxZDhQ2/aD+SzsEfBTk=
github.com/cli/go-gh/v2 v2.11.0 h1:TERLYMMWderKBO3lBff/JIu2+eSly2oFRgN2WvO+3eA=
github.com/cli/go-gh/v2 v2.11.0/go.mod h1:MeRoKzXff3ygHu7zP+NVTT+imcHW6p3tpuxHAzRM2xE=
github.com/cli/go-gh/v2 v2.11.1 h1:amAyfqMWQTBdue8iTmDUegGZK7c8kk6WCxD9l/wLtGI=
github.com/cli/go-gh/v2 v2.11.1/go.mod h1:MeRoKzXff3ygHu7zP+NVTT+imcHW6p3tpuxHAzRM2xE=
github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24 h1:QDrhR4JA2n3ij9YQN0u5ZeuvRIIvsUGmf5yPlTS0w8E=
github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24/go.mod h1:rr9GNING0onuVw8MnracQHn7PcchnFlP882Y0II2KZk=
github.com/cli/oauth v1.1.1 h1:459gD3hSjlKX9B1uXBuiAMdpXBUQ9QGf/NDcCpoQxPs=

2
internal/run/stub.go
View file

@ -9,7 +9,7 @@ import (
)
const (
gitAuthRE = `-c credential.helper= -c credential.helper=!"[^"]+" auth git-credential `
gitAuthRE = `-c credential(?:\..+)?\.helper= -c credential(?:\..+)?\.helper=!"[^"]+" auth git-credential `
)
type T interface {

2
pkg/cmd/issue/develop/develop_test.go
View file

@ -249,7 +249,7 @@ func TestDevelopRun(t *testing.T) {
expectedOut: heredoc.Doc(`
Showing linked branches for OWNER/REPO#42
BRANCH URL
foo https://github.com/OWNER/REPO/tree/foo
bar https://github.com/OWNER/OTHER-REPO/tree/bar

20
pkg/cmd/pr/checkout/checkout.go
View file

@ -130,7 +130,14 @@ func checkoutRun(opts *CheckoutOptions) error {
cmdQueue = append(cmdQueue, []string{"submodule", "update", "--init", "--recursive"})
}
err = executeCmds(opts.GitClient, cmdQueue)
// Note that although we will probably be fetching from the headRemote, in practice, PR checkout can only
// ever point to one host, and we know baseRemote must be populated, where headRemote might be nil (e.g. when
// it was deleted).
credentialPattern, err := git.CredentialPatternFromRemote(context.Background(), opts.GitClient, baseRemote.Name)
if err != nil {
return err
}
err = executeCmds(opts.GitClient, credentialPattern, cmdQueue)
if err != nil {
return err
}
@ -240,13 +247,16 @@ func localBranchExists(client *git.Client, b string) bool {
return err == nil
}
func executeCmds(client *git.Client, cmdQueue [][]string) error {
func executeCmds(client *git.Client, credentialPattern git.CredentialPattern, cmdQueue [][]string) error {
for _, args := range cmdQueue {
var err error
var cmd *git.Command
if args[0] == "fetch" || args[0] == "submodule" {
cmd, err = client.AuthenticatedCommand(context.Background(), args...)
} else {
switch args[0] {
case "submodule":
cmd, err = client.AuthenticatedCommand(context.Background(), credentialPattern, args...)
case "fetch":
cmd, err = client.AuthenticatedCommand(context.Background(), git.AllMatchingCredentialsPattern, args...)
default:
cmd, err = client.Command(context.Background(), args...)
}
if err != nil {

40
pkg/cmd/pr/checkout/checkout_test.go
View file

@ -72,6 +72,32 @@ func Test_checkoutRun(t *testing.T) {
wantStderr string
wantErr bool
}{
{
name: "checkout with ssh remote URL",
opts: &CheckoutOptions{
SelectorArg: "123",
Finder: func() shared.PRFinder {
baseRepo, pr := stubPR("OWNER/REPO:master", "OWNER/REPO:feature")
finder := shared.NewMockFinder("123", pr, baseRepo)
return finder
}(),
Config: func() (gh.Config, error) {
return config.NewBlankConfig(), nil
},
Branch: func() (string, error) {
return "main", nil
},
},
remotes: map[string]string{
"origin": "OWNER/REPO",
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
cs.Register(`git remote get-url origin`, 0, "https://github.com/OWNER/REPO.git")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git checkout -b feature --track origin/feature`, 0, "")
},
},
{
name: "fork repo was deleted",
opts: &CheckoutOptions{
@ -94,6 +120,7 @@ func Test_checkoutRun(t *testing.T) {
"origin": "OWNER/REPO",
},
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/pull/123/head:feature`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 1, "")
cs.Register(`git checkout feature`, 0, "")
@ -124,6 +151,7 @@ func Test_checkoutRun(t *testing.T) {
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git show-ref --verify -- refs/heads/foobar`, 1, "")
cs.Register(`git remote get-url origin`, 0, "https://github.com/OWNER/REPO.git")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git checkout -b foobar --track origin/feature`, 0, "")
},
@ -151,6 +179,7 @@ func Test_checkoutRun(t *testing.T) {
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git config branch\.foobar\.merge`, 1, "")
cs.Register(`git remote get-url origin`, 0, "https://github.com/hubot/REPO.git")
cs.Register(`git fetch origin refs/pull/123/head:foobar`, 0, "")
cs.Register(`git checkout foobar`, 0, "")
cs.Register(`git config branch\.foobar\.remote https://github.com/hubot/REPO.git`, 0, "")
@ -276,6 +305,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git remote get-url origin`, 0, "https://github.com/OWNER/REPO.git")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
cs.Register(`git checkout -b feature --track origin/feature`, 0, "")
@ -296,6 +326,7 @@ func TestPRCheckout_existingBranch(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git remote get-url origin`, 0, "https://github.com/OWNER/REPO.git")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "")
cs.Register(`git checkout feature`, 0, "")
@ -329,6 +360,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git remote get-url origin`, 0, "https://github.com/OWNER/REPO.git")
cs.Register(`git fetch robot-fork \+refs/heads/feature:refs/remotes/robot-fork/feature`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
cs.Register(`git checkout -b feature --track robot-fork/feature`, 0, "")
@ -350,6 +382,7 @@ func TestPRCheckout_differentRepo(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git remote get-url origin`, 0, "https://github.com/OWNER/REPO.git")
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 1, "")
cs.Register(`git checkout feature`, 0, "")
@ -373,6 +406,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git remote get-url origin`, 0, "https://github.com/OWNER/REPO.git")
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n")
cs.Register(`git checkout feature`, 0, "")
@ -393,6 +427,7 @@ func TestPRCheckout_detachedHead(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git remote get-url origin`, 0, "https://github.com/OWNER/REPO.git")
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n")
cs.Register(`git checkout feature`, 0, "")
@ -413,6 +448,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git remote get-url origin`, 0, "https://github.com/OWNER/REPO.git")
cs.Register(`git fetch origin refs/pull/123/head`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n")
cs.Register(`git merge --ff-only FETCH_HEAD`, 0, "")
@ -450,6 +486,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git remote get-url origin`, 0, "https://github.com/OWNER/REPO.git")
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 1, "")
cs.Register(`git checkout feature`, 0, "")
@ -472,6 +509,7 @@ func TestPRCheckout_recurseSubmodules(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git remote get-url origin`, 0, "https://github.com/OWNER/REPO.git")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "")
cs.Register(`git checkout feature`, 0, "")
@ -494,6 +532,7 @@ func TestPRCheckout_force(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git remote get-url origin`, 0, "https://github.com/OWNER/REPO.git")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "")
cs.Register(`git checkout feature`, 0, "")
@ -517,6 +556,7 @@ func TestPRCheckout_detach(t *testing.T) {
defer cmdTeardown(t)
cs.Register(`git checkout --detach FETCH_HEAD`, 0, "")
cs.Register(`git remote get-url origin`, 0, "https://github.com/hubot/REPO.git")
cs.Register(`git fetch origin refs/pull/123/head`, 0, "")
output, err := runCommand(http, nil, "", `123 --detach`)

2
pkg/cmd/repo/create/create.go
View file

@ -676,7 +676,7 @@ func createFromLocal(opts *CreateOptions) error {
}
if opts.Push && repoType == bare {
cmd, err := opts.GitClient.AuthenticatedCommand(context.Background(), "push", baseRemote, "--mirror")
cmd, err := opts.GitClient.AuthenticatedCommand(context.Background(), git.AllMatchingCredentialsPattern, "push", baseRemote, "--mirror")
if err != nil {
return err
}

2
pkg/cmd/repo/sync/git.go
View file

@ -55,7 +55,7 @@ func (g *gitExecuter) CurrentBranch() (string, error) {
func (g *gitExecuter) Fetch(remote, ref string) error {
args := []string{"fetch", "-q", remote, ref}
cmd, err := g.client.AuthenticatedCommand(context.Background(), args...)
cmd, err := g.client.AuthenticatedCommand(context.Background(), git.AllMatchingCredentialsPattern, args...)
if err != nil {
return err
}