From 066a6dc7b081015309eca8f2af4703019ada8280 Mon Sep 17 00:00:00 2001 From: John ODonnell Date: Wed, 19 Apr 2023 10:25:47 -0400 Subject: [PATCH] Support non-TTY Stdin for Confirmation prompts Being unable to pipe responses to prompts is a known shortcoming of Survey: https://github.com/go-survey/survey/issues/394. We frequently pipe 'yes' responses to CLI confirmation prompts, and this commit supports ONLY those cases. --- CHANGELOG.md | 9 ++++++- go.mod | 2 +- pkg/cmd/init_test.go | 25 ++++++++++++++--- pkg/cmd/utils_test.go | 44 ++++++++++++++++++++++++++++++ pkg/prompts/prompt.go | 63 ++++++++++++++++++++++++++++--------------- 5 files changed, 117 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 312b6745..fd43ef39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Nothing should go in this section, please add to the latest unreleased version (and update the corresponding date), or add a new version. +## [8.0.8] - 2023-04-19 + +### Fixed +- Fixed piping input to `conjur init` confirmation prompts + [cyberark/conjur-cli-go#127](https://github.com/cyberark/conjur-cli-go/pull/127) + ## [8.0.7] - 2023-04-18 ### Fixed @@ -76,7 +82,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - Placeholder version to capture the reset of the repository -[Unreleased]: https://github.com/cyberark/conjur-cli-go/compare/v8.0.7...HEAD +[Unreleased]: https://github.com/cyberark/conjur-cli-go/compare/v8.0.8...HEAD +[8.0.8]: https://github.com/cyberark/conjur-cli-go/compare/v8.0.7...v8.0.8 [8.0.7]: https://github.com/cyberark/conjur-cli-go/compare/v8.0.6...v8.0.7 [8.0.6]: https://github.com/cyberark/conjur-cli-go/compare/v8.0.5...v8.0.6 [8.0.5]: https://github.com/cyberark/conjur-cli-go/compare/v8.0.4...v8.0.5 diff --git a/go.mod b/go.mod index 2ea4e64f..47a2ce24 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/creack/pty v1.1.18 github.com/cyberark/conjur-api-go v0.10.3-0.20230217154521-e01f713716d2 // Run "go get github.com/cyberark/conjur-api-go@main" to update github.com/hinshun/vt10x v0.0.0-20220301184237-5011da428d02 + github.com/mattn/go-isatty v0.0.8 github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 github.com/spf13/cobra v1.5.0 github.com/stretchr/testify v1.8.1 @@ -27,7 +28,6 @@ require ( github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect github.com/kr/text v0.2.0 // indirect github.com/mattn/go-colorable v0.1.2 // indirect - github.com/mattn/go-isatty v0.0.8 // indirect github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/sirupsen/logrus v1.8.1 // indirect diff --git a/pkg/cmd/init_test.go b/pkg/cmd/init_test.go index dd9e001a..97d870af 100644 --- a/pkg/cmd/init_test.go +++ b/pkg/cmd/init_test.go @@ -16,8 +16,12 @@ var initCmdTestCases = []struct { args []string out string promptResponses []promptResponse - beforeTest func(t *testing.T, conjurrcInTmpDir string) - assert func(t *testing.T, conjurrcInTmpDir string, stdout string) + // Being unable to pipe responses to prompts is a known shortcoming of Survey. + // https://github.com/go-survey/survey/issues/394 + // This flag is used to enable Pipe-based, and not PTY-based, tests. + pipe bool + beforeTest func(t *testing.T, conjurrcInTmpDir string) + assert func(t *testing.T, conjurrcInTmpDir string, stdout string) }{ { name: "help", @@ -89,6 +93,7 @@ appliance_url: http://conjur response: "N", }, }, + pipe: true, beforeTest: func(t *testing.T, conjurrcInTmpDir string) { os.WriteFile(conjurrcInTmpDir, []byte("something"), 0644) }, @@ -107,6 +112,7 @@ appliance_url: http://conjur response: "y", }, }, + pipe: true, beforeTest: func(t *testing.T, conjurrcInTmpDir string) { os.WriteFile(conjurrcInTmpDir, []byte("something"), 0644) }, @@ -168,6 +174,7 @@ appliance_url: http://host response: "y", }, }, + pipe: true, assert: func(t *testing.T, conjurrcInTmpDir string, stdout string) { assertCertWritten(t, conjurrcInTmpDir, stdout) }, @@ -181,6 +188,7 @@ appliance_url: http://host response: "N", }, }, + pipe: true, assert: func(t *testing.T, conjurrcInTmpDir string, stdout string) { fmt.Println(stdout) assert.Contains(t, stdout, "You decided not to trust the certificate") @@ -205,6 +213,7 @@ appliance_url: http://host }, }, { + name: "succeeds for self-signed certificate with --self-signed flag", args: []string{"init", "-u=https://self-signed.badssl.com", "-a=test-account", "--self-signed"}, promptResponses: []promptResponse{ { @@ -212,6 +221,7 @@ appliance_url: http://host response: "y", }, }, + pipe: true, assert: func(t *testing.T, conjurrcInTmpDir string, stdout string) { assert.Contains(t, stdout, "Warning: Using self-signed certificates is not recommended and could lead to exposure of sensitive data") assertCertWritten(t, conjurrcInTmpDir, stdout) @@ -313,7 +323,16 @@ func TestInitCmd(t *testing.T) { rootCmd.AddCommand(cmd) rootCmd.SetArgs(args) - out, _ := executeCommandForTestWithPromptResponses(t, rootCmd, tc.promptResponses) + var out string + if tc.pipe { + var content string + for _, pr := range tc.promptResponses { + content = fmt.Sprintf("%s%s\n", content, pr.response) + } + out, _ = executeCommandForTestWithPipeResponses(t, rootCmd, content) + } else { + out, _ = executeCommandForTestWithPromptResponses(t, rootCmd, tc.promptResponses) + } if tc.assert != nil { tc.assert(t, conjurrcInTmpDir, out) diff --git a/pkg/cmd/utils_test.go b/pkg/cmd/utils_test.go index 7a06e52f..63416f26 100644 --- a/pkg/cmd/utils_test.go +++ b/pkg/cmd/utils_test.go @@ -3,6 +3,7 @@ package cmd import ( "bytes" "io" + "io/ioutil" "os" "regexp" "testing" @@ -49,6 +50,49 @@ func executeCommandForTest(t *testing.T, c *cobra.Command, args ...string) (stri return stripAnsi(stdoutBuf.String()), stripAnsi(stderrBuf.String()), err } +func executeCommandForTestWithPipeResponses( + t *testing.T, cmd *cobra.Command, content string, +) (stdOut string, cmdErr error) { + t.Helper() + + mockHelpText(cmd) + + // Send CLI stdOut and stdErr to Pipes and consolidate + outR, outW, _ := os.Pipe() + errR, errW, _ := os.Pipe() + combinedOutReader := io.MultiReader(outR, errR) + + defer func(origOut *os.File) { os.Stdout = origOut }(os.Stdout) + defer func(origErr *os.File) { os.Stderr = origErr }(os.Stderr) + os.Stdout = outW + os.Stderr = errW + + // Receive CLI stdIn from Pipe + inR, inW, _ := os.Pipe() + _, err := inW.Write([]byte(content)) + if err != nil { + t.Fatal(err) + } + inW.Close() + + defer func(origIn *os.File) { os.Stdin = origIn }(os.Stdin) + os.Stdin = inR + + // Run CLI cmd + err = cmd.Execute() + + // Collect stdOut + outW.Close() + errW.Close() + out, outErr := ioutil.ReadAll(combinedOutReader) + if outErr != nil { + t.Fatal(outErr) + } + + // strip ansi from stdout and stderr because we're using promptui + return stripAnsi(string(out)), err +} + func executeCommandForTestWithPromptResponses( t *testing.T, cmd *cobra.Command, promptResponses []promptResponse, ) (stdOut string, cmdErr error) { diff --git a/pkg/prompts/prompt.go b/pkg/prompts/prompt.go index 264cabeb..055cf199 100644 --- a/pkg/prompts/prompt.go +++ b/pkg/prompts/prompt.go @@ -5,8 +5,10 @@ import ( "fmt" "net/url" "os" + "strings" "github.com/AlecAivazis/survey/v2" + "github.com/mattn/go-isatty" "github.com/spf13/cobra" ) @@ -32,46 +34,65 @@ func newAccountPrompt() *survey.Question { } } -func newFileExistsPrompt(filePath string) *survey.Question { - return &survey.Question{ - Prompt: &survey.Confirm{Message: fmt.Sprintf("File %s exists. Overwrite?", filePath)}, - Validate: survey.Required, +func stringToBool(s string) bool { + switch strings.ToLower(strings.TrimSpace(s)) { + case "y": + return true + case "yes": + return true + default: + return false } } -// AskToOverwriteFile presents a prompt to get confirmation from a user to overwrite a file -func AskToOverwriteFile(filePath string) error { - var userInput bool +func basicConfirm(message string) bool { + fmt.Fprintf(os.Stdout, "%s ", message) + var answer string + fmt.Scanln(&answer) + fmt.Fprintln(os.Stdout, "") + return stringToBool(answer) +} - q := newFileExistsPrompt(filePath) - err := survey.AskOne(q.Prompt, &userInput, survey.WithValidator(q.Validate), survey.WithShowCursor(true)) +func confirm(message string) (bool, error) { + var err error + var userInput bool - if userInput == false { - return fmt.Errorf("Not overwriting %s", filePath) + // When handling Pipe-based Stdin, we can't use Survey to gather responses. + // https://github.com/go-survey/survey/issues/394 + // Instead, use standard fmt funcs to prompt to Stdout and gather from Stdin. + if isatty.IsTerminal(os.Stdin.Fd()) { + q := &survey.Question{ + Prompt: &survey.Confirm{Message: message}, + Validate: survey.Required, + } + err = survey.AskOne(q.Prompt, &userInput, survey.WithValidator(q.Validate), survey.WithShowCursor(true)) + } else { + userInput = basicConfirm(message) } - return err + + return userInput, err } -func newTrustCertPrompt() *survey.Question { - return &survey.Question{ - Prompt: &survey.Confirm{Message: "Trust this certificate?"}, - Validate: survey.Required, +// AskToOverwriteFile presents a prompt to get confirmation from a user to overwrite a file +func AskToOverwriteFile(filePath string) error { + userInput, err := confirm(fmt.Sprintf("File %s exists. Overwrite?", filePath)) + + if !userInput { + return fmt.Errorf("Not overwriting %s", filePath) } + return err } // AskToTrustCert presents a prompt to get confirmation from a user to trust a certificate func AskToTrustCert(fingerprint string) error { - var userInput bool - warning := fmt.Sprintf("\nThe server's certificate fingerprint is %s.\n", fingerprint) + "Please verify this certificate on the appliance using command:\n" + "openssl x509 -fingerprint -noout -in ~conjur/etc/ssl/conjur.pem\n\n" os.Stdout.Write([]byte(warning)) - q := newTrustCertPrompt() - err := survey.AskOne(q.Prompt, &userInput, survey.WithValidator(q.Validate), survey.WithShowCursor(true)) + userInput, err := confirm("Trust this certificate?") - if userInput == false { + if !userInput { return errors.New("You decided not to trust the certificate.") } return err