Merge pull request #10209 from cli/wm/add-remote-check-to-secret

Require repo disambiguation for secret commands
This commit is contained in:
William Martin 2025-01-21 15:24:49 +01:00 committed by GitHub
commit ff922353bb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 756 additions and 40 deletions

View file

@ -0,0 +1,36 @@
# Set up env vars
env REPO=${SCRIPT_NAME}-${RANDOM_STRING}
# Use gh as a credential helper
exec gh auth setup-git
# Create a repository with a file so it has a default branch
exec gh repo create ${ORG}/${REPO} --add-readme --private
# Defer repo cleanup
defer gh repo delete --yes ${ORG}/${REPO}
# Create a fork
exec gh repo fork ${ORG}/${REPO} --org ${ORG} --fork-name ${REPO}-fork
# Defer fork cleanup
defer gh repo delete --yes ${ORG}/${REPO}-fork
# Sleep to allow the fork to be created before cloning
sleep 2
# Clone and move into the fork repo
exec gh repo clone ${ORG}/${REPO}-fork
cd ${REPO}-fork
# Secret list requires disambiguation
! exec gh secret list
stderr 'multiple remotes detected. please specify which repo to use by providing the -R, --repo argument'
# Secret set requires disambiguation
! exec gh secret set 'TEST_SECRET_NAME' --body 'TEST_SECRET_VALUE'
stderr 'multiple remotes detected. please specify which repo to use by providing the -R, --repo argument'
# Secret delete requires disambiguation
! exec gh secret delete 'TEST_SECRET_NAME'
stderr 'multiple remotes detected. please specify which repo to use by providing the -R, --repo argument'

View file

@ -69,6 +69,7 @@ func (rr *remoteResolver) Resolver() func() (context.Remotes, error) {
sort.Sort(resolvedRemotes)
// Filter remotes by hosts
// Note that this is not caching correctly: https://github.com/cli/cli/issues/10103
cachedRemotes := resolvedRemotes.FilterByHosts(hosts)
// Filter again by default host if one is set

View file

@ -30,7 +30,8 @@ func explainer() string {
- viewing and creating issues
- viewing and creating releases
- working with GitHub Actions
- adding repository and environment secrets`)
### NOTE: gh does not use the default repository for managing repository and environment secrets.`)
}
type iprompter interface {

View file

@ -382,7 +382,7 @@ func TestDefaultRun(t *testing.T) {
}
}
},
wantStdout: "This command sets the default remote repository to use when querying the\nGitHub API for the locally cloned repository.\n\ngh uses the default repository for things like:\n\n - viewing and creating pull requests\n - viewing and creating issues\n - viewing and creating releases\n - working with GitHub Actions\n - adding repository and environment secrets\n\n✓ Set OWNER2/REPO2 as the default repository for the current directory\n",
wantStdout: "This command sets the default remote repository to use when querying the\nGitHub API for the locally cloned repository.\n\ngh uses the default repository for things like:\n\n - viewing and creating pull requests\n - viewing and creating issues\n - viewing and creating releases\n - working with GitHub Actions\n\n### NOTE: gh does not use the default repository for managing repository and environment secrets.\n\n✓ Set OWNER2/REPO2 as the default repository for the current directory\n",
},
{
name: "interactive mode only one known host",
@ -456,7 +456,7 @@ func TestDefaultRun(t *testing.T) {
}
}
},
wantStdout: "This command sets the default remote repository to use when querying the\nGitHub API for the locally cloned repository.\n\ngh uses the default repository for things like:\n\n - viewing and creating pull requests\n - viewing and creating issues\n - viewing and creating releases\n - working with GitHub Actions\n - adding repository and environment secrets\n\n✓ Set OWNER2/REPO2 as the default repository for the current directory\n",
wantStdout: "This command sets the default remote repository to use when querying the\nGitHub API for the locally cloned repository.\n\ngh uses the default repository for things like:\n\n - viewing and creating pull requests\n - viewing and creating issues\n - viewing and creating releases\n - working with GitHub Actions\n\n### NOTE: gh does not use the default repository for managing repository and environment secrets.\n\n✓ Set OWNER2/REPO2 as the default repository for the current directory\n",
},
}

View file

@ -46,8 +46,19 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co
`),
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
// support `-R, --repo` override
// If the user specified a repo directly, then we're using the OverrideBaseRepoFunc set by EnableRepoOverride
// So there's no reason to use the specialised BaseRepoFunc that requires remote disambiguation.
opts.BaseRepo = f.BaseRepo
if !cmd.Flags().Changed("repo") {
// If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that errors if
// there might be multiple valid remotes.
opts.BaseRepo = shared.RequireNoAmbiguityBaseRepoFunc(opts.BaseRepo, f.Remotes)
// But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to
// resolve the ambiguity.
if opts.IO.CanPrompt() {
opts.BaseRepo = shared.PromptWhenAmbiguousBaseRepoFunc(opts.BaseRepo, f.IOStreams, f.Prompter)
}
}
if err := cmdutil.MutuallyExclusive("specify only one of `--org`, `--env`, or `--user`", opts.OrgName != "", opts.EnvName != "", opts.UserSecrets); err != nil {
return err
@ -88,6 +99,14 @@ func removeRun(opts *DeleteOptions) error {
return err
}
var baseRepo ghrepo.Interface
if secretEntity == shared.Repository || secretEntity == shared.Environment {
baseRepo, err = opts.BaseRepo()
if err != nil {
return err
}
}
secretApp, err := shared.GetSecretApp(opts.Application, secretEntity)
if err != nil {
return err
@ -97,14 +116,6 @@ func removeRun(opts *DeleteOptions) error {
return fmt.Errorf("%s secrets are not supported for %s", secretEntity, secretApp)
}
var baseRepo ghrepo.Interface
if secretEntity == shared.Repository || secretEntity == shared.Environment {
baseRepo, err = opts.BaseRepo()
if err != nil {
return err
}
}
cfg, err := opts.Config()
if err != nil {
return err

View file

@ -2,12 +2,17 @@ package delete
import (
"bytes"
"io"
"net/http"
"testing"
ghContext "github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/pkg/cmd/secret/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
@ -120,6 +125,108 @@ func TestNewCmdDelete(t *testing.T) {
}
}
func TestNewCmdDeleteBaseRepoFuncs(t *testing.T) {
remotes := ghContext.Remotes{
&ghContext.Remote{
Remote: &git.Remote{
Name: "origin",
},
Repo: ghrepo.New("owner", "fork"),
},
&ghContext.Remote{
Remote: &git.Remote{
Name: "upstream",
},
Repo: ghrepo.New("owner", "repo"),
},
}
tests := []struct {
name string
args string
prompterStubs func(*prompter.MockPrompter)
wantRepo ghrepo.Interface
wantErr error
}{
{
name: "when there is a repo flag provided, the factory base repo func is used",
args: "SECRET_NAME --repo owner/repo",
wantRepo: ghrepo.New("owner", "repo"),
},
{
name: "when there is no repo flag provided, and no prompting, the base func requiring no ambiguity is used",
args: "SECRET_NAME",
wantErr: shared.AmbiguousBaseRepoError{
Remotes: remotes,
},
},
{
name: "when there is no repo flag provided, and can prompt, the base func resolving ambiguity is used",
args: "SECRET_NAME",
prompterStubs: func(pm *prompter.MockPrompter) {
pm.RegisterSelect(
"Select a repo",
[]string{"owner/fork", "owner/repo"},
func(_, _ string, opts []string) (int, error) {
return prompter.IndexFor(opts, "owner/fork")
},
)
},
wantRepo: ghrepo.New("owner", "fork"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
var pm *prompter.MockPrompter
if tt.prompterStubs != nil {
ios.SetStdinTTY(true)
ios.SetStdoutTTY(true)
ios.SetStderrTTY(true)
pm = prompter.NewMockPrompter(t)
tt.prompterStubs(pm)
}
f := &cmdutil.Factory{
IOStreams: ios,
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("owner/repo")
},
Prompter: pm,
Remotes: func() (ghContext.Remotes, error) {
return remotes, nil
},
}
argv, err := shlex.Split(tt.args)
assert.NoError(t, err)
var gotOpts *DeleteOptions
cmd := NewCmdDelete(f, func(opts *DeleteOptions) error {
gotOpts = opts
return nil
})
// Require to support --repo flag
cmdutil.EnableRepoOverride(cmd, f)
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
_, err = cmd.ExecuteC()
require.NoError(t, err)
baseRepo, err := gotOpts.BaseRepo()
if tt.wantErr != nil {
require.Equal(t, tt.wantErr, err)
return
}
require.True(t, ghrepo.IsSame(tt.wantRepo, baseRepo))
})
}
}
func Test_removeRun_repo(t *testing.T) {
tests := []struct {
name string

View file

@ -11,6 +11,7 @@ import (
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/internal/tableprinter"
"github.com/cli/cli/v2/pkg/cmd/secret/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
@ -23,8 +24,10 @@ type ListOptions struct {
IO *iostreams.IOStreams
Config func() (gh.Config, error)
BaseRepo func() (ghrepo.Interface, error)
Now func() time.Time
Exporter cmdutil.Exporter
Prompter prompter.Prompter
Now func() time.Time
Exporter cmdutil.Exporter
OrgName string
EnvName string
@ -48,6 +51,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
Config: f.Config,
HttpClient: f.HttpClient,
Now: time.Now,
Prompter: f.Prompter,
}
cmd := &cobra.Command{
@ -63,8 +67,19 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
Aliases: []string{"ls"},
Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
// support `-R, --repo` override
// If the user specified a repo directly, then we're using the OverrideBaseRepoFunc set by EnableRepoOverride
// So there's no reason to use the specialised BaseRepoFunc that requires remote disambiguation.
opts.BaseRepo = f.BaseRepo
if !cmd.Flags().Changed("repo") {
// If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that errors if
// there might be multiple valid remotes.
opts.BaseRepo = shared.RequireNoAmbiguityBaseRepoFunc(opts.BaseRepo, f.Remotes)
// But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to
// resolve the ambiguity.
if opts.IO.CanPrompt() {
opts.BaseRepo = shared.PromptWhenAmbiguousBaseRepoFunc(opts.BaseRepo, f.IOStreams, f.Prompter)
}
}
if err := cmdutil.MutuallyExclusive("specify only one of `--org`, `--env`, or `--user`", opts.OrgName != "", opts.EnvName != "", opts.UserSecrets); err != nil {
return err

View file

@ -3,15 +3,19 @@ package list
import (
"bytes"
"fmt"
"io"
"net/http"
"net/url"
"strings"
"testing"
"time"
ghContext "github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/pkg/cmd/secret/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
@ -101,6 +105,108 @@ func Test_NewCmdList(t *testing.T) {
}
}
func TestNewCmdListBaseRepoFuncs(t *testing.T) {
remotes := ghContext.Remotes{
&ghContext.Remote{
Remote: &git.Remote{
Name: "origin",
},
Repo: ghrepo.New("owner", "fork"),
},
&ghContext.Remote{
Remote: &git.Remote{
Name: "upstream",
},
Repo: ghrepo.New("owner", "repo"),
},
}
tests := []struct {
name string
args string
prompterStubs func(*prompter.MockPrompter)
wantRepo ghrepo.Interface
wantErr error
}{
{
name: "when there is a repo flag provided, the factory base repo func is used",
args: "--repo owner/repo",
wantRepo: ghrepo.New("owner", "repo"),
},
{
name: "when there is no repo flag provided, and no prompting, the base func requiring no ambiguity is used",
args: "",
wantErr: shared.AmbiguousBaseRepoError{
Remotes: remotes,
},
},
{
name: "when there is no repo flag provided, and can prompt, the base func resolving ambiguity is used",
args: "",
prompterStubs: func(pm *prompter.MockPrompter) {
pm.RegisterSelect(
"Select a repo",
[]string{"owner/fork", "owner/repo"},
func(_, _ string, opts []string) (int, error) {
return prompter.IndexFor(opts, "owner/fork")
},
)
},
wantRepo: ghrepo.New("owner", "fork"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
var pm *prompter.MockPrompter
if tt.prompterStubs != nil {
ios.SetStdinTTY(true)
ios.SetStdoutTTY(true)
ios.SetStderrTTY(true)
pm = prompter.NewMockPrompter(t)
tt.prompterStubs(pm)
}
f := &cmdutil.Factory{
IOStreams: ios,
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("owner/repo")
},
Prompter: pm,
Remotes: func() (ghContext.Remotes, error) {
return remotes, nil
},
}
argv, err := shlex.Split(tt.args)
assert.NoError(t, err)
var gotOpts *ListOptions
cmd := NewCmdList(f, func(opts *ListOptions) error {
gotOpts = opts
return nil
})
// Require to support --repo flag
cmdutil.EnableRepoOverride(cmd, f)
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
_, err = cmd.ExecuteC()
require.NoError(t, err)
baseRepo, err := gotOpts.BaseRepo()
if tt.wantErr != nil {
require.Equal(t, tt.wantErr, err)
return
}
require.True(t, ghrepo.IsSame(tt.wantRepo, baseRepo))
})
}
}
func Test_listRun(t *testing.T) {
tests := []struct {
name string

View file

@ -9,6 +9,8 @@ import (
"os"
"strings"
"github.com/cli/cli/v2/internal/prompter"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/gh"
@ -27,7 +29,7 @@ type SetOptions struct {
IO *iostreams.IOStreams
Config func() (gh.Config, error)
BaseRepo func() (ghrepo.Interface, error)
Prompter iprompter
Prompter prompter.Prompter
RandomOverride func() io.Reader
@ -43,10 +45,6 @@ type SetOptions struct {
Application string
}
type iprompter interface {
Password(string) (string, error)
}
func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command {
opts := &SetOptions{
IO: f.IOStreams,
@ -77,6 +75,9 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command
# Read secret value from an environment variable
$ gh secret set MYSECRET --body "$ENV_VALUE"
# Set secret for a specific remote repository
$ gh secret set MYSECRET --repo origin/repo --body "$ENV_VALUE"
# Read secret value from a file
$ gh secret set MYSECRET < myfile.txt
@ -103,8 +104,19 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command
`),
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
// support `-R, --repo` override
// If the user specified a repo directly, then we're using the OverrideBaseRepoFunc set by EnableRepoOverride
// So there's no reason to use the specialised BaseRepoFunc that requires remote disambiguation.
opts.BaseRepo = f.BaseRepo
if !cmd.Flags().Changed("repo") {
// If they haven't specified a repo directly, then we will wrap the BaseRepoFunc in one that errors if
// there might be multiple valid remotes.
opts.BaseRepo = shared.RequireNoAmbiguityBaseRepoFunc(opts.BaseRepo, f.Remotes)
// But if we are able to prompt, then we will wrap that up in a BaseRepoFunc that can prompt the user to
// resolve the ambiguity.
if opts.IO.CanPrompt() {
opts.BaseRepo = shared.PromptWhenAmbiguousBaseRepoFunc(opts.BaseRepo, f.IOStreams, f.Prompter)
}
}
if err := cmdutil.MutuallyExclusive("specify only one of `--org`, `--env`, or `--user`", opts.OrgName != "", opts.EnvName != "", opts.UserSecrets); err != nil {
return err
@ -166,6 +178,27 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command
}
func setRun(opts *SetOptions) error {
orgName := opts.OrgName
envName := opts.EnvName
var host string
var baseRepo ghrepo.Interface
if orgName == "" && !opts.UserSecrets {
var err error
baseRepo, err = opts.BaseRepo()
if err != nil {
return err
}
host = baseRepo.RepoHost()
} else {
cfg, err := opts.Config()
if err != nil {
return err
}
host, _ = cfg.Authentication().DefaultHost()
}
secrets, err := getSecretsFromOptions(opts)
if err != nil {
return err
@ -177,25 +210,6 @@ func setRun(opts *SetOptions) error {
}
client := api.NewClientFromHTTP(c)
orgName := opts.OrgName
envName := opts.EnvName
var host string
var baseRepo ghrepo.Interface
if orgName == "" && !opts.UserSecrets {
baseRepo, err = opts.BaseRepo()
if err != nil {
return err
}
host = baseRepo.RepoHost()
} else {
cfg, err := opts.Config()
if err != nil {
return err
}
host, _ = cfg.Authentication().DefaultHost()
}
secretEntity, err := shared.GetSecretEntity(orgName, envName, opts.UserSecrets)
if err != nil {
return err

View file

@ -10,6 +10,8 @@ import (
"testing"
"github.com/MakeNowJust/heredoc"
ghContext "github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
@ -20,6 +22,7 @@ import (
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestNewCmdSet(t *testing.T) {
@ -221,6 +224,108 @@ func TestNewCmdSet(t *testing.T) {
}
}
func TestNewCmdSetBaseRepoFuncs(t *testing.T) {
remotes := ghContext.Remotes{
&ghContext.Remote{
Remote: &git.Remote{
Name: "origin",
},
Repo: ghrepo.New("owner", "fork"),
},
&ghContext.Remote{
Remote: &git.Remote{
Name: "upstream",
},
Repo: ghrepo.New("owner", "repo"),
},
}
tests := []struct {
name string
args string
prompterStubs func(*prompter.MockPrompter)
wantRepo ghrepo.Interface
wantErr error
}{
{
name: "when there is a repo flag provided, the factory base repo func is used",
args: "SECRET_NAME --repo owner/repo",
wantRepo: ghrepo.New("owner", "repo"),
},
{
name: "when there is no repo flag provided, and no prompting, the base func requiring no ambiguity is used",
args: "SECRET_NAME",
wantErr: shared.AmbiguousBaseRepoError{
Remotes: remotes,
},
},
{
name: "when there is no repo flag provided, and can prompt, the base func resolving ambiguity is used",
args: "SECRET_NAME",
prompterStubs: func(pm *prompter.MockPrompter) {
pm.RegisterSelect(
"Select a repo",
[]string{"owner/fork", "owner/repo"},
func(_, _ string, opts []string) (int, error) {
return prompter.IndexFor(opts, "owner/fork")
},
)
},
wantRepo: ghrepo.New("owner", "fork"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
var pm *prompter.MockPrompter
if tt.prompterStubs != nil {
ios.SetStdinTTY(true)
ios.SetStdoutTTY(true)
ios.SetStderrTTY(true)
pm = prompter.NewMockPrompter(t)
tt.prompterStubs(pm)
}
f := &cmdutil.Factory{
IOStreams: ios,
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("owner/repo")
},
Prompter: pm,
Remotes: func() (ghContext.Remotes, error) {
return remotes, nil
},
}
argv, err := shlex.Split(tt.args)
assert.NoError(t, err)
var gotOpts *SetOptions
cmd := NewCmdSet(f, func(opts *SetOptions) error {
gotOpts = opts
return nil
})
// Require to support --repo flag
cmdutil.EnableRepoOverride(cmd, f)
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
_, err = cmd.ExecuteC()
require.NoError(t, err)
baseRepo, err := gotOpts.BaseRepo()
if tt.wantErr != nil {
require.Equal(t, tt.wantErr, err)
return
}
require.True(t, ghrepo.IsSame(tt.wantRepo, baseRepo))
})
}
}
func Test_setRun_repo(t *testing.T) {
tests := []struct {
name string

View file

@ -0,0 +1,71 @@
package shared
import (
"errors"
"fmt"
ghContext "github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/pkg/iostreams"
)
type AmbiguousBaseRepoError struct {
Remotes ghContext.Remotes
}
func (e AmbiguousBaseRepoError) Error() string {
return "multiple remotes detected. please specify which repo to use by providing the -R, --repo argument"
}
type baseRepoFn func() (ghrepo.Interface, error)
type remotesFn func() (ghContext.Remotes, error)
func PromptWhenAmbiguousBaseRepoFunc(baseRepoFn baseRepoFn, ios *iostreams.IOStreams, prompter prompter.Prompter) baseRepoFn {
return func() (ghrepo.Interface, error) {
baseRepo, err := baseRepoFn()
if err != nil {
var ambiguousBaseRepoErr AmbiguousBaseRepoError
if !errors.As(err, &ambiguousBaseRepoErr) {
return nil, err
}
baseRepoOptions := make([]string, len(ambiguousBaseRepoErr.Remotes))
for i, remote := range ambiguousBaseRepoErr.Remotes {
baseRepoOptions[i] = ghrepo.FullName(remote)
}
fmt.Fprintf(ios.Out, "%s Multiple remotes detected. Due to the sensitive nature of secrets, requiring disambiguation.\n", ios.ColorScheme().WarningIcon())
selectedBaseRepo, err := prompter.Select("Select a repo", baseRepoOptions[0], baseRepoOptions)
if err != nil {
return nil, err
}
selectedRepo, err := ghrepo.FromFullName(baseRepoOptions[selectedBaseRepo])
if err != nil {
return nil, err
}
return selectedRepo, nil
}
return baseRepo, nil
}
}
// RequireNoAmbiguityBaseRepoFunc returns a function to resolve the base repo, ensuring that
// there was only one option, regardless of whether the base repo had been set.
func RequireNoAmbiguityBaseRepoFunc(baseRepo baseRepoFn, remotes remotesFn) baseRepoFn {
return func() (ghrepo.Interface, error) {
remotes, err := remotes()
if err != nil {
return nil, err
}
if remotes.Len() > 1 {
return nil, AmbiguousBaseRepoError{Remotes: remotes}
}
return baseRepo()
}
}

View file

@ -0,0 +1,249 @@
package shared_test
import (
"errors"
"testing"
ghContext "github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/pkg/cmd/secret/shared"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/stretchr/testify/require"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
)
func TestRequireNoAmbiguityBaseRepoFunc(t *testing.T) {
t.Parallel()
t.Run("succeeds when there is only one remote", func(t *testing.T) {
t.Parallel()
// Given there is only one remote
baseRepoFn := shared.RequireNoAmbiguityBaseRepoFunc(baseRepoStubFn, oneRemoteStubFn)
// When fetching the base repo
baseRepo, err := baseRepoFn()
// It succeeds and returns the inner base repo
require.NoError(t, err)
require.True(t, ghrepo.IsSame(ghrepo.New("owner", "repo"), baseRepo))
})
t.Run("returns specific error when there are multiple remotes", func(t *testing.T) {
t.Parallel()
// Given there are multiple remotes
baseRepoFn := shared.RequireNoAmbiguityBaseRepoFunc(baseRepoStubFn, twoRemotesStubFn)
// When fetching the base repo
_, err := baseRepoFn()
// It succeeds and returns the inner base repo
var multipleRemotesError shared.AmbiguousBaseRepoError
require.ErrorAs(t, err, &multipleRemotesError)
require.Equal(t, ghContext.Remotes{
{
Remote: &git.Remote{
Name: "origin",
},
Repo: ghrepo.New("owner", "fork"),
},
{
Remote: &git.Remote{
Name: "upstream",
},
Repo: ghrepo.New("owner", "repo"),
},
}, multipleRemotesError.Remotes)
})
t.Run("when the remote fetching function fails, it returns the error", func(t *testing.T) {
t.Parallel()
// Given the remote fetching function fails
baseRepoFn := shared.RequireNoAmbiguityBaseRepoFunc(baseRepoStubFn, errRemoteStubFn)
// When fetching the base repo
_, err := baseRepoFn()
// It returns the error
require.Equal(t, errors.New("test remote error"), err)
})
t.Run("when the wrapped base repo function fails, it returns the error", func(t *testing.T) {
t.Parallel()
// Given the wrapped base repo function fails
baseRepoFn := shared.RequireNoAmbiguityBaseRepoFunc(errBaseRepoStubFn, oneRemoteStubFn)
// When fetching the base repo
_, err := baseRepoFn()
// It returns the error
require.Equal(t, errors.New("test base repo error"), err)
})
}
func TestPromptWhenMultipleRemotesBaseRepoFunc(t *testing.T) {
t.Parallel()
t.Run("when there is no error from wrapped base repo func, then it succeeds without prompting", func(t *testing.T) {
t.Parallel()
ios, _, _, _ := iostreams.Test()
// Given the base repo function succeeds
baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(baseRepoStubFn, ios, nil)
// When fetching the base repo
baseRepo, err := baseRepoFn()
// It succeeds and returns the inner base repo
require.NoError(t, err)
require.True(t, ghrepo.IsSame(ghrepo.New("owner", "repo"), baseRepo))
})
t.Run("when the wrapped base repo func returns a specific error, then the prompter is used for disambiguation, with the remote ordering remaining unchanged", func(t *testing.T) {
t.Parallel()
ios, _, stdout, _ := iostreams.Test()
pm := prompter.NewMockPrompter(t)
pm.RegisterSelect(
"Select a repo",
[]string{"owner/fork", "owner/repo"},
func(_, def string, opts []string) (int, error) {
require.Equal(t, "owner/fork", def)
return prompter.IndexFor(opts, "owner/repo")
},
)
// Given the wrapped base repo func returns a specific error
baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(errMultipleRemotesStubFn, ios, pm)
// When fetching the base repo
baseRepo, err := baseRepoFn()
// It prints an informative message
require.Equal(t, "! Multiple remotes detected. Due to the sensitive nature of secrets, requiring disambiguation.\n", stdout.String())
// And it uses the prompter for disambiguation
require.NoError(t, err)
require.True(t, ghrepo.IsSame(ghrepo.New("owner", "repo"), baseRepo))
})
t.Run("when the prompter returns an error, then it is returned", func(t *testing.T) {
t.Parallel()
ios, _, _, _ := iostreams.Test()
// Given the prompter returns an error
pm := prompter.NewMockPrompter(t)
pm.RegisterSelect(
"Select a repo",
[]string{"owner/fork", "owner/repo"},
func(_, _ string, opts []string) (int, error) {
return 0, errors.New("test prompt error")
},
)
// Given the wrapped base repo func returns a specific error
baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(errMultipleRemotesStubFn, ios, pm)
// When fetching the base repo
_, err := baseRepoFn()
// It returns the error
require.Equal(t, errors.New("test prompt error"), err)
})
t.Run("when the wrapped base repo func returns a non-specific error, then it is returned", func(t *testing.T) {
t.Parallel()
ios, _, _, _ := iostreams.Test()
// Given the wrapped base repo func returns a non-specific error
baseRepoFn := shared.PromptWhenAmbiguousBaseRepoFunc(errBaseRepoStubFn, ios, nil)
// When fetching the base repo
_, err := baseRepoFn()
// It returns the error
require.Equal(t, errors.New("test base repo error"), err)
})
}
func TestMultipleRemotesErrorMessage(t *testing.T) {
err := shared.AmbiguousBaseRepoError{}
require.EqualError(t, err, "multiple remotes detected. please specify which repo to use by providing the -R, --repo argument")
}
func errMultipleRemotesStubFn() (ghrepo.Interface, error) {
remote1 := &ghContext.Remote{
Remote: &git.Remote{
Name: "origin",
},
Repo: ghrepo.New("owner", "fork"),
}
remote2 := &ghContext.Remote{
Remote: &git.Remote{
Name: "upstream",
},
Repo: ghrepo.New("owner", "repo"),
}
return nil, shared.AmbiguousBaseRepoError{
Remotes: ghContext.Remotes{
remote1,
remote2,
},
}
}
func baseRepoStubFn() (ghrepo.Interface, error) {
return ghrepo.New("owner", "repo"), nil
}
func oneRemoteStubFn() (ghContext.Remotes, error) {
remote := &ghContext.Remote{
Remote: &git.Remote{
Name: "origin",
},
Repo: ghrepo.New("owner", "repo"),
}
return ghContext.Remotes{
remote,
}, nil
}
func twoRemotesStubFn() (ghContext.Remotes, error) {
remote1 := &ghContext.Remote{
Remote: &git.Remote{
Name: "origin",
},
Repo: ghrepo.New("owner", "fork"),
}
remote2 := &ghContext.Remote{
Remote: &git.Remote{
Name: "upstream",
},
Repo: ghrepo.New("owner", "repo"),
}
return ghContext.Remotes{
remote1,
remote2,
}, nil
}
func errRemoteStubFn() (ghContext.Remotes, error) {
return nil, errors.New("test remote error")
}
func errBaseRepoStubFn() (ghrepo.Interface, error) {
return nil, errors.New("test base repo error")
}