From 28db3d1879c8b7e4313a51f4d7bb91efd3bef1cd Mon Sep 17 00:00:00 2001 From: Kyle Gani Date: Tue, 4 Aug 2026 11:52:24 +0200 Subject: [PATCH] fix(deploy): fail fast on non-TTY confirmation Co-Authored-By: Claude Fable 5 --- go.mod | 2 +- internal/commands/deploy.go | 42 ++++++++--- internal/commands/deploy_test.go | 106 ++++++++++++++++++++++++++++ internal/ui/commands/deploy.go | 20 ++++-- internal/ui/commands/deploy_test.go | 69 ++++++++++++++++++ internal/ui/displayconf.go | 7 ++ 6 files changed, 230 insertions(+), 16 deletions(-) create mode 100644 internal/commands/deploy_test.go diff --git a/go.mod b/go.mod index 64caca8..37f9a95 100644 --- a/go.mod +++ b/go.mod @@ -18,6 +18,7 @@ require ( github.com/pelletier/go-toml/v2 v2.2.4 github.com/sebdah/goldie/v2 v2.8.0 github.com/spf13/cobra v1.10.1 + github.com/spf13/pflag v1.0.10 github.com/spf13/viper v1.21.0 github.com/stretchr/testify v1.11.1 golang.org/x/sync v0.17.0 @@ -51,7 +52,6 @@ require ( github.com/sergi/go-diff v1.4.0 // indirect github.com/spf13/afero v1.15.0 // indirect github.com/spf13/cast v1.10.0 // indirect - github.com/spf13/pflag v1.0.10 // indirect github.com/stretchr/objx v0.5.3 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect diff --git a/internal/commands/deploy.go b/internal/commands/deploy.go index 754c5dc..16455db 100644 --- a/internal/commands/deploy.go +++ b/internal/commands/deploy.go @@ -13,18 +13,18 @@ import ( "github.com/cerebriumai/cerebrium/pkg/projectconfig" tea "github.com/charmbracelet/bubbletea" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) // NewDeployCmd creates a deploy command func NewDeployCmd() *cobra.Command { var ( - name string - disableSyntaxCheck bool - logLevel string - configFile string - disableConfirmation bool - disableBuildLogs bool - detach bool + name string + disableSyntaxCheck bool + logLevel string + configFile string + disableBuildLogs bool + detach bool ) cmd := &cobra.Command{ @@ -50,7 +50,7 @@ Example: configFile: configFile, disableBuildLogs: disableBuildLogs, detach: detach, - }, disableConfirmation) + }, confirmationDisabled(cmd.Flags())) }, } @@ -59,7 +59,8 @@ Example: cmd.Flags().BoolVar(&disableSyntaxCheck, "disable-syntax-check", false, "Flag to disable syntax check") cmd.Flags().StringVar(&logLevel, "log-level", "INFO", "Log level for deployment (DEBUG or INFO)") cmd.Flags().StringVar(&configFile, "config-file", "./cerebrium.toml", "Path to the cerebrium config TOML file") - cmd.Flags().BoolVarP(&disableConfirmation, "disable-confirmation", "y", false, "Disable confirmation prompt") + cmd.Flags().BoolP("disable-confirmation", "y", false, "Disable confirmation prompt") + cmd.Flags().Bool("yes", false, "Skip the confirmation prompt (alias of --disable-confirmation)") cmd.Flags().BoolVar(&disableBuildLogs, "disable-build-logs", false, "Disable build logs during deployment") cmd.Flags().BoolVar(&detach, "detach", false, "Kick off deployment and exit without waiting for build completion. The build will continue on the server and Ctrl+C will not cancel it.") @@ -75,6 +76,24 @@ type deployOptions struct { detach bool } +// confirmationDisabled resolves the effective confirmation setting from +// --disable-confirmation (-y) and its alias --yes +func confirmationDisabled(flags *pflag.FlagSet) bool { + disableConfirmation, _ := flags.GetBool("disable-confirmation") + yes, _ := flags.GetBool("yes") + return disableConfirmation || yes +} + +// validateConfirmationPrompt fails fast when a confirmation prompt would be +// required but stdin is not a TTY, so a closed or piped stdin can never hang +// the command or be mistaken for consent +func validateConfirmationPrompt(stdinIsTTY, disableConfirmation bool) error { + if disableConfirmation || stdinIsTTY { + return nil + } + return ui.NewValidationError(fmt.Errorf("unable to prompt for confirmation: stdin is not a TTY. Re-run with -y/--yes to skip confirmation")) +} + func runDeploy(cmd *cobra.Command, opts deployOptions, disableConfirmation bool) error { cmd.SilenceUsage = true @@ -84,6 +103,11 @@ func runDeploy(cmd *cobra.Command, opts deployOptions, disableConfirmation bool) return ui.NewValidationError(fmt.Errorf("failed to get display options: %w", err)) } + // Fail fast if we would need to prompt for confirmation but cannot + if err := validateConfirmationPrompt(displayOpts.StdinIsTTY, disableConfirmation); err != nil { + return err + } + // Get config from context (loaded once in root command) cfg, err := config.GetConfigFromContext(cmd) if err != nil { diff --git a/internal/commands/deploy_test.go b/internal/commands/deploy_test.go new file mode 100644 index 0000000..0b2779f --- /dev/null +++ b/internal/commands/deploy_test.go @@ -0,0 +1,106 @@ +package commands + +import ( + "errors" + "testing" + + "github.com/cerebriumai/cerebrium/internal/ui" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_validateConfirmationPrompt(t *testing.T) { + tcs := []struct { + name string + stdinIsTTY bool + disableConfirmation bool + expectError bool + }{ + { + name: "TTY with confirmation required - prompt allowed", + stdinIsTTY: true, + disableConfirmation: false, + expectError: false, + }, + { + name: "TTY with confirmation disabled - no prompt needed", + stdinIsTTY: true, + disableConfirmation: true, + expectError: false, + }, + { + name: "non-TTY with confirmation disabled - no prompt needed", + stdinIsTTY: false, + disableConfirmation: true, + expectError: false, + }, + { + name: "non-TTY with confirmation required - fail fast", + stdinIsTTY: false, + disableConfirmation: false, + expectError: true, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + err := validateConfirmationPrompt(tc.stdinIsTTY, tc.disableConfirmation) + + if !tc.expectError { + assert.NoError(t, err) + return + } + + require.Error(t, err) + + var uiErr *ui.UIError + require.True(t, errors.As(err, &uiErr), "guard should return a structured UIError") + assert.Equal(t, ui.ErrorTypeValidation, uiErr.Type) + assert.Contains(t, err.Error(), "stdin is not a TTY") + assert.Contains(t, err.Error(), "-y/--yes", "error should name the flag that skips confirmation") + }) + } +} + +func Test_confirmationDisabled(t *testing.T) { + tcs := []struct { + name string + args []string + expected bool + }{ + { + name: "no flags", + args: []string{}, + expected: false, + }, + { + name: "--disable-confirmation", + args: []string{"--disable-confirmation"}, + expected: true, + }, + { + name: "-y shorthand", + args: []string{"-y"}, + expected: true, + }, + { + name: "--yes alias", + args: []string{"--yes"}, + expected: true, + }, + { + name: "both flags", + args: []string{"--disable-confirmation", "--yes"}, + expected: true, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + cmd := NewDeployCmd() + require.NoError(t, cmd.ParseFlags(tc.args)) + + assert.Equal(t, tc.expected, confirmationDisabled(cmd.Flags())) + }) + } +} diff --git a/internal/ui/commands/deploy.go b/internal/ui/commands/deploy.go index dcd4a9c..03b30d2 100644 --- a/internal/ui/commands/deploy.go +++ b/internal/ui/commands/deploy.go @@ -1,6 +1,7 @@ package commands import ( + "bufio" "context" "errors" "fmt" @@ -1325,14 +1326,21 @@ func (m *DeployView) showDeploymentSummary() { fmt.Print("Do you want to deploy? (Y/n): ") } -// waitForConfirmation waits for user input in non-TTY mode +// waitForConfirmation waits for user input in simple output mode func (m *DeployView) waitForConfirmation() tea.Msg { - // Read a single line from stdin - var response string - fmt.Scanln(&response) //nolint:errcheck,gosec // User input handling, errors handled by empty response default + return readConfirmationResponse(os.Stdin) +} - // Default to "yes" if empty (just Enter pressed) - if response == "" || strings.ToLower(response) == "y" || strings.ToLower(response) == "yes" { +// readConfirmationResponse reads a single line from r and interprets it as a +// confirmation response. An empty line defaults to yes (the prompt is Y/n); +// EOF or any read error is a decline so a closed stdin can never consent +func readConfirmationResponse(r io.Reader) confirmationResponseMsg { + line, err := bufio.NewReader(r).ReadString('\n') + response := strings.ToLower(strings.TrimSpace(line)) + if err != nil && response == "" { + return confirmationResponseMsg{confirmed: false} + } + if response == "" || response == "y" || response == "yes" { return confirmationResponseMsg{confirmed: true} } return confirmationResponseMsg{confirmed: false} diff --git a/internal/ui/commands/deploy_test.go b/internal/ui/commands/deploy_test.go index ab373a3..f7d0c74 100644 --- a/internal/ui/commands/deploy_test.go +++ b/internal/ui/commands/deploy_test.go @@ -3,7 +3,10 @@ package commands import ( "context" "errors" + "io" + "strings" "testing" + "testing/iotest" "github.com/cerebriumai/cerebrium/internal/api" apimock "github.com/cerebriumai/cerebrium/internal/api/mock" @@ -1053,3 +1056,69 @@ func TestDeployView_View(t *testing.T) { assert.Contains(t, view, "cancelled") }) } + +func TestReadConfirmationResponse(t *testing.T) { + tcs := []struct { + name string + input io.Reader + confirmed bool + }{ + { + name: "explicit yes", + input: strings.NewReader("y\n"), + confirmed: true, + }, + { + name: "explicit yes uppercase", + input: strings.NewReader("Y\n"), + confirmed: true, + }, + { + name: "explicit yes word", + input: strings.NewReader("yes\n"), + confirmed: true, + }, + { + name: "yes without trailing newline", + input: strings.NewReader("y"), + confirmed: true, + }, + { + name: "empty line defaults to yes", + input: strings.NewReader("\n"), + confirmed: true, + }, + { + name: "whitespace-only line defaults to yes", + input: strings.NewReader(" \n"), + confirmed: true, + }, + { + name: "explicit no", + input: strings.NewReader("n\n"), + confirmed: false, + }, + { + name: "any other input declines", + input: strings.NewReader("maybe\n"), + confirmed: false, + }, + { + name: "EOF with no input declines", + input: strings.NewReader(""), + confirmed: false, + }, + { + name: "read error declines", + input: iotest.ErrReader(errors.New("stdin read failure")), + confirmed: false, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + msg := readConfirmationResponse(tc.input) + assert.Equal(t, tc.confirmed, msg.confirmed) + }) + } +} diff --git a/internal/ui/displayconf.go b/internal/ui/displayconf.go index f534278..6f499c3 100644 --- a/internal/ui/displayconf.go +++ b/internal/ui/displayconf.go @@ -21,6 +21,7 @@ func GetDisplayConfigContextKey() DisplayConfigContextKey { type DisplayConfig struct { DisableAnimation bool IsInteractive bool + StdinIsTTY bool } func (d DisplayConfig) SimpleOutput() bool { @@ -38,6 +39,10 @@ func NewDisplayConfig(cmd *cobra.Command, verbose bool) (DisplayConfig, error) { // We only check stdout because that's where the TUI output goes stdoutIsTTY := isatty.IsTerminal(os.Stdout.Fd()) + // Detect if stdin is a TTY + // Commands that prompt for input must fail fast when it is not + stdinIsTTY := isatty.IsTerminal(os.Stdin.Fd()) + // Check if stdout and stderr point to the same file // This matters for verbose mode: if they're separate, verbose logs won't interfere with TUI stderrRedirectedToStdout := false @@ -64,6 +69,7 @@ func NewDisplayConfig(cmd *cobra.Command, verbose bool) (DisplayConfig, error) { opts := DisplayConfig{ DisableAnimation: disableAnimation, IsInteractive: isInteractive, + StdinIsTTY: stdinIsTTY, } // Debug logging to help diagnose display options @@ -74,6 +80,7 @@ func NewDisplayConfig(cmd *cobra.Command, verbose bool) (DisplayConfig, error) { "disable-animation-flag", disableAnimationFlag, "verbose-flag", verbose, "stdout-is-tty", stdoutIsTTY, + "stdin-is-tty", stdinIsTTY, "stderr-is-tty", isatty.IsTerminal(os.Stderr.Fd()), "stderr-same-as-stdout", stderrRedirectedToStdout, "verbose-forces-simple", verboseForcesSimpleOutput,