From 18f2484080dc81a92166c9127d6d70b3beeaf52a Mon Sep 17 00:00:00 2001 From: Kenichi Kocha <64531971+kkocha@users.noreply.github.com> Date: Mon, 3 Apr 2023 23:40:37 +0900 Subject: [PATCH 1/5] docs: add commit SHA arg to gh browse help (#7267) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mislav Marohnić --- pkg/cmd/browse/browse.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index 160d72aa2..6a9456eb7 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -59,7 +59,7 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co cmd := &cobra.Command{ Long: "Open the GitHub repository in the web browser.", Short: "Open the repository in the browser", - Use: "browse [ | ]", + Use: "browse [ | | ]", Args: cobra.MaximumNArgs(1), Example: heredoc.Doc(` $ gh browse @@ -87,7 +87,8 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co "help:arguments": heredoc.Doc(` A browser location can be specified using arguments in the following format: - by number for issue or pull request, e.g. "123"; or - - by path for opening folders and files, e.g. "cmd/gh/main.go" + - by path for opening folders and files, e.g. "cmd/gh/main.go"; or + - by commit SHA `), "help:environment": heredoc.Doc(` To configure a web browser other than the default, use the BROWSER environment variable. From 6261bc697188321d43d18d915832255830175b45 Mon Sep 17 00:00:00 2001 From: Kenichi Kocha <64531971+kkocha@users.noreply.github.com> Date: Tue, 4 Apr 2023 10:36:09 +0900 Subject: [PATCH 2/5] Additional help doc and example for auth setup-git (#7243) --- pkg/cmd/auth/setupgit/setupgit.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/auth/setupgit/setupgit.go b/pkg/cmd/auth/setupgit/setupgit.go index a041ebfc1..a3c991f4f 100644 --- a/pkg/cmd/auth/setupgit/setupgit.go +++ b/pkg/cmd/auth/setupgit/setupgit.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/pkg/cmd/auth/shared" "github.com/cli/cli/v2/pkg/cmdutil" @@ -29,8 +30,26 @@ func NewCmdSetupGit(f *cmdutil.Factory, runF func(*SetupGitOptions) error) *cobr } cmd := &cobra.Command{ - Short: "Configure git to use GitHub CLI as a credential helper", Use: "setup-git", + Short: "Setup git with GitHub CLI", + Long: heredoc.Docf(` + This command configures git to use GitHub CLI as a credential helper. + For more information on git credential helpers please reference: + https://git-scm.com/docs/gitcredentials. + + By default, GitHub CLI will be set as the credential helper for all authenticated hosts. + If there is no authenticated hosts the command fails with an error. + + Alternatively, use the %[1]s--hostname%[1]s flag to specify a single host to be configured. + If the host is not authenticated with, the command fails with an error. + `, "`"), + Example: heredoc.Doc(` + # Configure git to use GitHub CLI as the credential helper for all authenticated hosts + $ gh auth setup-git + + # Configure git to use GitHub CLI as the credential helper for enterprise.internal host + $ gh auth setup-git --hostname enterprise.internal + `), RunE: func(cmd *cobra.Command, args []string) error { opts.gitConfigure = &shared.GitCredentialFlow{ Executable: f.Executable(), From 126341205500b7f69ecef223f661a30b3fa7df03 Mon Sep 17 00:00:00 2001 From: Kenichi Kocha <64531971+kkocha@users.noreply.github.com> Date: Tue, 4 Apr 2023 11:54:40 +0900 Subject: [PATCH 3/5] fix: make number arg, commit arg, and flags mutually exclusive (#7268) * make number, commit, and flags mutually exclusive * break condition into arg own check --- pkg/cmd/browse/browse.go | 4 ++++ pkg/cmd/browse/browse_test.go | 22 +++++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index 6a9456eb7..42664d77c 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -125,6 +125,10 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co return err } + if (isNumber(opts.SelectorArg) || isCommit(opts.SelectorArg)) && (opts.Branch != "" || opts.Commit != "") { + return cmdutil.FlagErrorf("%q is an invalid argument when using `--branch` or `--commit`", opts.SelectorArg) + } + if cmd.Flags().Changed("repo") { opts.GitClient = &remoteGitClient{opts.BaseRepo, opts.HttpClient} } diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index 440970f29..4995e5cdf 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -162,7 +162,27 @@ func TestNewCmdBrowse(t *testing.T) { }, { name: "passed both branch and commit flags", - cli: "main.go --branch main --comit=12a4", + cli: "main.go --branch main --commit=12a4", + wantsErr: true, + }, + { + name: "passed both number arg and branch flag", + cli: "1 --branch trunk", + wantsErr: true, + }, + { + name: "passed both number arg and commit flag", + cli: "1 --commit=12a4", + wantsErr: true, + }, + { + name: "passed both commit SHA arg and branch flag", + cli: "de07febc26e19000f8c9e821207f3bc34a3c8038 --branch trunk", + wantsErr: true, + }, + { + name: "passed both commit SHA arg and commit flag", + cli: "de07febc26e19000f8c9e821207f3bc34a3c8038 --commit=12a4", wantsErr: true, }, } From 5d56dfbf423ce95f73d65b741d2a6578cddca087 Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 4 Apr 2023 11:13:43 -0700 Subject: [PATCH 4/5] add --insecure-storage and make secure storage the default --- pkg/cmd/auth/login/login.go | 29 ++++++------ pkg/cmd/auth/login/login_test.go | 76 ++++++++++++++++++++------------ 2 files changed, 63 insertions(+), 42 deletions(-) diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 3eda42ae0..258853164 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -30,12 +30,12 @@ type LoginOptions struct { Interactive bool - Hostname string - Scopes []string - Token string - Web bool - GitProtocol string - SecureStorage bool + Hostname string + Scopes []string + Token string + Web bool + GitProtocol string + InsecureStorage bool } func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Command { @@ -124,7 +124,13 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm cmd.Flags().BoolVar(&tokenStdin, "with-token", false, "Read token from standard input") cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open a browser to authenticate") cmdutil.StringEnumFlag(cmd, &opts.GitProtocol, "git-protocol", "p", "", []string{"ssh", "https"}, "The protocol to use for git operations") - cmd.Flags().BoolVarP(&opts.SecureStorage, "secure-storage", "", false, "Save authentication credentials in secure credential store") + + // secure storage became the default on 2023/4/04; this flag is left as a no-op for backwards compatibility + var secureStorage bool + cmd.Flags().BoolVar(&secureStorage, "secure-storage", false, "Save authentication credentials in secure credential store") + _ = cmd.Flags().MarkHidden("secure-storage") + + cmd.Flags().BoolVar(&opts.InsecureStorage, "insecure-storage", false, "Save authentication credentials in plain text instead of credential store") return cmd } @@ -136,11 +142,6 @@ func loginRun(opts *LoginOptions) error { } authCfg := cfg.Authentication() - if opts.SecureStorage { - cs := opts.IO.ColorScheme() - fmt.Fprintf(opts.IO.ErrOut, "%s Using secure storage could break installed extensions\n", cs.WarningIcon()) - } - hostname := opts.Hostname if opts.Interactive && hostname == "" { var err error @@ -166,7 +167,7 @@ func loginRun(opts *LoginOptions) error { return fmt.Errorf("error validating token: %w", err) } // Adding a user key ensures that a nonempty host section gets written to the config file. - return authCfg.Login(hostname, "x-access-token", opts.Token, opts.GitProtocol, opts.SecureStorage) + return authCfg.Login(hostname, "x-access-token", opts.Token, opts.GitProtocol, !opts.InsecureStorage) } existingToken, _ := authCfg.Token(hostname) @@ -195,7 +196,7 @@ func loginRun(opts *LoginOptions) error { Prompter: opts.Prompter, GitClient: opts.GitClient, Browser: opts.Browser, - SecureStorage: opts.SecureStorage, + SecureStorage: !opts.InsecureStorage, }) } diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index 9f95cc9ee..72d796a39 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -178,16 +178,31 @@ func Test_NewCmdLogin(t *testing.T) { stdinTTY: true, cli: "--secure-storage", wants: LoginOptions{ - Interactive: true, - SecureStorage: true, + Interactive: true, }, }, { name: "nontty secure-storage", cli: "--secure-storage", wants: LoginOptions{ - Hostname: "github.com", - SecureStorage: true, + Hostname: "github.com", + }, + }, + { + name: "tty insecure-storage", + stdinTTY: true, + cli: "--insecure-storage", + wants: LoginOptions{ + Interactive: true, + InsecureStorage: true, + }, + }, + { + name: "nontty insecure-storage", + cli: "--insecure-storage", + wants: LoginOptions{ + Hostname: "github.com", + InsecureStorage: true, }, }, } @@ -251,10 +266,11 @@ func Test_loginRun_nontty(t *testing.T) { wantSecureToken string }{ { - name: "with token", + name: "insecure with token", opts: &LoginOptions{ - Hostname: "github.com", - Token: "abc123", + Hostname: "github.com", + Token: "abc123", + InsecureStorage: true, }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org")) @@ -262,11 +278,12 @@ func Test_loginRun_nontty(t *testing.T) { wantHosts: "github.com:\n oauth_token: abc123\n user: x-access-token\n", }, { - name: "with token and https git-protocol", + name: "insecure with token and https git-protocol", opts: &LoginOptions{ - Hostname: "github.com", - Token: "abc123", - GitProtocol: "https", + Hostname: "github.com", + Token: "abc123", + GitProtocol: "https", + InsecureStorage: true, }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org")) @@ -276,8 +293,9 @@ func Test_loginRun_nontty(t *testing.T) { { name: "with token and non-default host", opts: &LoginOptions{ - Hostname: "albert.wesker", - Token: "abc123", + Hostname: "albert.wesker", + Token: "abc123", + InsecureStorage: true, }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.ScopesResponder("repo,read:org")) @@ -309,8 +327,9 @@ func Test_loginRun_nontty(t *testing.T) { { name: "has admin scope", opts: &LoginOptions{ - Hostname: "github.com", - Token: "abc456", + Hostname: "github.com", + Token: "abc456", + InsecureStorage: true, }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,admin:org")) @@ -358,16 +377,14 @@ func Test_loginRun_nontty(t *testing.T) { { name: "with token and secure storage", opts: &LoginOptions{ - Hostname: "github.com", - Token: "abc123", - SecureStorage: true, + Hostname: "github.com", + Token: "abc123", }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org")) }, wantHosts: "github.com:\n user: x-access-token\n", wantSecureToken: "abc123", - wantStderr: "! Using secure storage could break installed extensions\n", }, } @@ -463,8 +480,9 @@ func Test_loginRun_Survey(t *testing.T) { { name: "hostname set", opts: &LoginOptions{ - Hostname: "rebecca.chambers", - Interactive: true, + Hostname: "rebecca.chambers", + Interactive: true, + InsecureStorage: true, }, wantHosts: heredoc.Doc(` rebecca.chambers: @@ -504,7 +522,8 @@ func Test_loginRun_Survey(t *testing.T) { git_protocol: https `), opts: &LoginOptions{ - Interactive: true, + Interactive: true, + InsecureStorage: true, }, prompterStubs: func(pm *prompter.PrompterMock) { pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) { @@ -543,7 +562,8 @@ func Test_loginRun_Survey(t *testing.T) { git_protocol: https `), opts: &LoginOptions{ - Interactive: true, + Interactive: true, + InsecureStorage: true, }, prompterStubs: func(pm *prompter.PrompterMock) { pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) { @@ -573,7 +593,8 @@ func Test_loginRun_Survey(t *testing.T) { git_protocol: ssh `), opts: &LoginOptions{ - Interactive: true, + Interactive: true, + InsecureStorage: true, }, prompterStubs: func(pm *prompter.PrompterMock) { pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) { @@ -593,9 +614,8 @@ func Test_loginRun_Survey(t *testing.T) { { name: "secure storage", opts: &LoginOptions{ - Hostname: "github.com", - Interactive: true, - SecureStorage: true, + Hostname: "github.com", + Interactive: true, }, prompterStubs: func(pm *prompter.PrompterMock) { pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) { @@ -617,7 +637,7 @@ func Test_loginRun_Survey(t *testing.T) { user: jillv git_protocol: https `), - wantErrOut: regexp.MustCompile("! Using secure storage could break installed extensions"), + wantErrOut: regexp.MustCompile("Logged in as jillv"), wantSecureToken: "def456", }, } From d9db81db4ff35d38fb8abf3fd27c46de532c198f Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 4 Apr 2023 11:13:53 -0700 Subject: [PATCH 5/5] fix apparent typo --- pkg/cmd/auth/token/token.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/auth/token/token.go b/pkg/cmd/auth/token/token.go index 5ae555bb6..fee8dc638 100644 --- a/pkg/cmd/auth/token/token.go +++ b/pkg/cmd/auth/token/token.go @@ -38,7 +38,7 @@ func NewCmdToken(f *cmdutil.Factory, runF func(*TokenOptions) error) *cobra.Comm cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance authenticated with") cmd.Flags().BoolVarP(&opts.SecureStorage, "secure-storage", "", false, "Search only secure credential store for authentication token") - _ = cmd.Flags().MarkHidden("secure-storeage") + _ = cmd.Flags().MarkHidden("secure-storage") return cmd }