Merge branch 'trunk' into phillmv/improve-gh-at-inspect

This commit is contained in:
Phill MV 2024-11-27 16:40:22 -05:00
commit 1cbdcedc80
17 changed files with 564 additions and 114 deletions

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
}

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=

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 {

View file

@ -35,23 +35,23 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co
cmd := &cobra.Command{
Use: "delete [<cache-id>| <cache-key> | --all]",
Short: "Delete GitHub Actions caches",
Long: `
Delete GitHub Actions caches.
Long: heredoc.Docf(`
Delete GitHub Actions caches.
Deletion requires authorization with the "repo" scope.
`,
Deletion requires authorization with the %[1]srepo%[1]s scope.
`, "`"),
Example: heredoc.Doc(`
# Delete a cache by id
$ gh cache delete 1234
# Delete a cache by id
$ gh cache delete 1234
# Delete a cache by key
$ gh cache delete cache-key
# Delete a cache by key
$ gh cache delete cache-key
# Delete a cache by id in a specific repo
$ gh cache delete 1234 --repo cli/cli
# Delete a cache by id in a specific repo
$ gh cache delete 1234 --repo cli/cli
# Delete all caches
$ gh cache delete --all
# Delete all caches
$ gh cache delete --all
`),
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {

View file

@ -4,6 +4,7 @@ import (
"context"
"fmt"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/codespaces"
"github.com/cli/cli/v2/internal/codespaces/api"
"github.com/cli/cli/v2/internal/codespaces/portforwarder"
@ -20,9 +21,12 @@ func newRebuildCmd(app *App) *cobra.Command {
rebuildCmd := &cobra.Command{
Use: "rebuild",
Short: "Rebuild a codespace",
Long: `Rebuilding recreates your codespace. Your code and any current changes will be
preserved. Your codespace will be rebuilt using your working directory's
dev container. A full rebuild also removes cached Docker images.`,
Long: heredoc.Doc(`
Rebuilding recreates your codespace.
Your code and any current changes will be preserved. Your codespace will be rebuilt using
your working directory's dev container. A full rebuild also removes cached Docker images.
`),
Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
return app.Rebuild(cmd.Context(), selector, fullRebuild)

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

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 {

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`)

View file

@ -477,6 +477,21 @@ func createRun(opts *CreateOptions) error {
}
newRelease, err := createRelease(httpClient, baseRepo, params)
var errMissingRequiredWorkflowScope *errMissingRequiredWorkflowScope
if errors.As(err, &errMissingRequiredWorkflowScope) {
host := errMissingRequiredWorkflowScope.Hostname
refreshInstructions := fmt.Sprintf("gh auth refresh -h %[1]s -s workflow", host)
cs := opts.IO.ColorScheme()
var sb strings.Builder
sb.WriteString(fmt.Sprintf("%s Failed to create release, \"workflow\" scope may be required.\n", cs.WarningIcon()))
sb.WriteString(fmt.Sprintf("To request it, run:\n%s\n", cs.Bold(refreshInstructions)))
fmt.Fprint(opts.IO.ErrOut, sb.String())
return cmdutil.SilentError
}
if err != nil {
return err
}

View file

@ -10,6 +10,7 @@ import (
"path/filepath"
"testing"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/gh"
@ -1082,6 +1083,74 @@ func Test_createRun(t *testing.T) {
runStubs: defaultRunStubs,
wantErr: "cannot generate release notes from tag v1.2.3 as it does not exist locally",
},
{
name: "API returns 404, OAuth token has no workflow scope",
isTTY: false,
opts: CreateOptions{
TagName: "Does not matter",
},
runStubs: func(rs *run.CommandStubber) {
rs.Register(contentCmd, 0, "some tag message")
rs.Register(signatureCmd, 0, "")
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL("RepositoryFindRef"),
httpmock.StringResponse(`{"data":{"repository":{"ref": {"id": "tag id"}}}}`),
)
reg.Register(
httpmock.REST("POST", "repos/OWNER/REPO/releases"),
httpmock.StatusScopesResponder(404, `repo,read:org`))
},
wantStderr: heredoc.Doc(`
! Failed to create release, "workflow" scope may be required.
To request it, run:
gh auth refresh -h github.com -s workflow
`),
wantErr: cmdutil.SilentError.Error(),
},
{
name: "API returns 404, OAuth token has workflow scope",
isTTY: false,
opts: CreateOptions{
TagName: "Does not matter",
},
runStubs: func(rs *run.CommandStubber) {
rs.Register(contentCmd, 0, "some tag message")
rs.Register(signatureCmd, 0, "")
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL("RepositoryFindRef"),
httpmock.StringResponse(`{"data":{"repository":{"ref": {"id": "tag id"}}}}`),
)
reg.Register(
httpmock.REST("POST", "repos/OWNER/REPO/releases"),
httpmock.StatusScopesResponder(404, `repo,read:org,workflow`))
},
wantErr: "HTTP 404 (https://api.github.com/repos/OWNER/REPO/releases)",
},
{
name: "API returns 404, not an OAuth token",
isTTY: false,
opts: CreateOptions{
TagName: "Does not matter",
},
runStubs: func(rs *run.CommandStubber) {
rs.Register(contentCmd, 0, "some tag message")
rs.Register(signatureCmd, 0, "")
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL("RepositoryFindRef"),
httpmock.StringResponse(`{"data":{"repository":{"ref": {"id": "tag id"}}}}`),
)
reg.Register(
httpmock.REST("POST", "repos/OWNER/REPO/releases"),
httpmock.StatusStringResponse(404, `HTTP 404 (https://api.github.com/repos/OWNER/REPO/releases)`))
},
wantErr: "HTTP 404 (https://api.github.com/repos/OWNER/REPO/releases)",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -1115,7 +1184,6 @@ func Test_createRun(t *testing.T) {
err := createRun(&tt.opts)
if tt.wantErr != "" {
require.EqualError(t, err, tt.wantErr)
return
} else {
require.NoError(t, err)
}

View file

@ -8,12 +8,16 @@ import (
"io"
"net/http"
"net/url"
"slices"
"strings"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/release/shared"
"github.com/shurcooL/githubv4"
ghauth "github.com/cli/go-gh/v2/pkg/auth"
)
type tag struct {
@ -27,6 +31,14 @@ type releaseNotes struct {
var notImplementedError = errors.New("not implemented")
type errMissingRequiredWorkflowScope struct {
Hostname string
}
func (e errMissingRequiredWorkflowScope) Error() string {
return "workflow scope may be required"
}
func remoteTagExists(httpClient *http.Client, repo ghrepo.Interface, tagName string) (bool, error) {
gql := api.NewClientFromHTTP(httpClient)
qualifiedTagName := fmt.Sprintf("refs/tags/%s", tagName)
@ -174,6 +186,24 @@ func createRelease(httpClient *http.Client, repo ghrepo.Interface, params map[st
}
defer resp.Body.Close()
// Check if we received a 404 while attempting to create a release without
// the workflow scope, and if so, return an error message that explains a possible
// solution to the user.
//
// If the same file (with both the same path and contents) exists
// on another branch in the repo, releases with workflow file changes can be
// created without the workflow scope. Otherwise, the workflow scope is
// required to create the release, but the API does not indicate this criteria
// beyond returning a 404.
//
// https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/scopes-for-oauth-apps#available-scopes
if resp.StatusCode == http.StatusNotFound && !tokenHasWorkflowScope(resp) {
normalizedHostname := ghauth.NormalizeHostname(resp.Request.URL.Hostname())
return nil, &errMissingRequiredWorkflowScope{
Hostname: normalizedHostname,
}
}
success := resp.StatusCode >= 200 && resp.StatusCode < 300
if !success {
return nil, api.HandleHTTPError(resp)
@ -254,3 +284,18 @@ func deleteRelease(httpClient *http.Client, release *shared.Release) error {
}
return nil
}
// tokenHasWorkflowScope checks if the given http.Response's token has the workflow scope.
// Tokens that do not have OAuth scopes are assumed to have the workflow scope.
func tokenHasWorkflowScope(resp *http.Response) bool {
scopes := resp.Header.Get("X-Oauth-Scopes")
// Return true when no scopes are present - no scopes in this header
// means that the user is probably authenticating with a token type other
// than an OAuth token, and we don't know what this token's scopes actually are.
if scopes == "" {
return true
}
return slices.Contains(strings.Split(scopes, ","), "workflow")
}

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
}

View file

@ -6,6 +6,7 @@ import (
"net/http"
"strings"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmdutil"
@ -38,12 +39,14 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co
cmd := &cobra.Command{
Use: "delete [<repository>]",
Short: "Delete a repository",
Long: `Delete a GitHub repository.
Long: heredoc.Docf(`
Delete a GitHub repository.
With no argument, deletes the current repository. Otherwise, deletes the specified repository.
With no argument, deletes the current repository. Otherwise, deletes the specified repository.
Deletion requires authorization with the "delete_repo" scope.
To authorize, run "gh auth refresh -s delete_repo"`,
Deletion requires authorization with the %[1]sdelete_repo%[1]s scope.
To authorize, run %[1]sgh auth refresh -s delete_repo%[1]s
`, "`"),
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
if len(args) > 0 {

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
}

View file

@ -225,10 +225,16 @@ func GraphQLQuery(body string, cb func(string, map[string]interface{})) Responde
}
}
// ScopesResponder returns a response with a 200 status code and the given OAuth scopes.
func ScopesResponder(scopes string) func(*http.Request) (*http.Response, error) {
return StatusScopesResponder(http.StatusOK, scopes)
}
// StatusScopesResponder returns a response with the given status code and OAuth scopes.
func StatusScopesResponder(status int, scopes string) func(*http.Request) (*http.Response, error) {
return func(req *http.Request) (*http.Response, error) {
return &http.Response{
StatusCode: 200,
StatusCode: status,
Request: req,
Header: map[string][]string{
"X-Oauth-Scopes": {scopes},