From bbc679541d0492ba6cd65cf8e4645de23e2d3109 Mon Sep 17 00:00:00 2001 From: Arne Luenser Date: Wed, 29 Jul 2026 14:32:28 +0200 Subject: [PATCH 1/2] fix: make --yes skip the sign-in confirmation prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --yes was parsed and stored on CommandHelper as noConfirm, but nothing ever read that field, so the flag was a no-op across every cloudx command. `ory tunnel --yes` still stopped at "Do you want to sign in?" and waited on stdin, which never arrives in CI — the documented workaround was to pipe `yes` into the command. Confirmations now go through CommandHelper.Confirm, which returns true without prompting when --yes is set. Routing them through one method rather than fixing the single call site means the next prompt added cannot silently reintroduce the bug. Closes #219 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JYzGVwAKQ4ormxRHZg1eDu --- cmd/cloudx/client/api_key.go | 3 +- cmd/cloudx/client/command_helper.go | 13 ++++++ cmd/cloudx/client/confirm_test.go | 66 +++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 cmd/cloudx/client/confirm_test.go diff --git a/cmd/cloudx/client/api_key.go b/cmd/cloudx/client/api_key.go index 34a2e341..7d083bfb 100644 --- a/cmd/cloudx/client/api_key.go +++ b/cmd/cloudx/client/api_key.go @@ -10,7 +10,6 @@ import ( "time" cloud "github.com/ory/client-go" - "github.com/ory/x/cmdx" ) // CreateProjectAPIKey creates a project API key. If expiresIn is greater than @@ -93,7 +92,7 @@ func (h *CommandHelper) TemporaryAPIKey(ctx context.Context, name string, expire _, _ = fmt.Fprintf(h.VerboseErrWriter, "Because you are not authenticated, the Ory CLI can not configure your project automatically. You can still use the Ory Proxy / Ory Tunnel, but complex flows such as Social Sign In will not work. Remove the `--quiet` flag or run `ory auth login` to authenticate.") return "", noop, nil } else if errors.Is(err, ErrNotAuthenticated) { - ok, err := cmdx.AskScannerForConfirmation("To support complex flows such as Social Sign In, the Ory CLI can configure your project automatically. To do so, you need to be signed in. Do you want to sign in?", h.Stdin, h.VerboseErrWriter) + ok, err := h.Confirm("To support complex flows such as Social Sign In, the Ory CLI can configure your project automatically. To do so, you need to be signed in. Do you want to sign in?") if err != nil { return "", noop, err } diff --git a/cmd/cloudx/client/command_helper.go b/cmd/cloudx/client/command_helper.go index a36d0264..9a8c32e6 100644 --- a/cmd/cloudx/client/command_helper.go +++ b/cmd/cloudx/client/command_helper.go @@ -314,6 +314,19 @@ func (h *CommandHelper) determineProjectID(ctx context.Context, config *Config) return nil } +// Confirm asks the user to confirm message, returning true if they agree. +// +// When --yes is set the answer is yes without prompting. Every interactive +// confirmation in the cloudx commands must go through here, otherwise --yes +// silently does nothing and unattended use (CI, scripts) blocks forever on a +// prompt nobody can answer. +func (h *CommandHelper) Confirm(message string) (bool, error) { + if h.noConfirm { + return true, nil + } + return cmdx.AskScannerForConfirmation(message, h.Stdin, h.VerboseErrWriter) +} + func (h *CommandHelper) ProjectID() (string, error) { if h.projectID == uuid.Nil { return "", ErrProjectNotSet diff --git a/cmd/cloudx/client/confirm_test.go b/cmd/cloudx/client/confirm_test.go new file mode 100644 index 00000000..42c1bce3 --- /dev/null +++ b/cmd/cloudx/client/confirm_test.go @@ -0,0 +1,66 @@ +// Copyright © 2026 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package client + +import ( + "bufio" + "bytes" + "io" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// refusingReader fails the test if anything tries to read from it. Reading +// stdin is exactly what --yes has to avoid: in CI there is nobody to answer, +// so the command would block forever. +type refusingReader struct{ t *testing.T } + +func (r refusingReader) Read([]byte) (int, error) { + r.t.Error("stdin must not be read when --yes is set") + return 0, io.EOF +} + +// TestConfirm covers https://github.com/ory/cli/issues/219: --yes was parsed +// and stored but never read, so it did not skip any prompt. +func TestConfirm(t *testing.T) { + t.Run("case=--yes answers yes without prompting", func(t *testing.T) { + var out bytes.Buffer + h := &CommandHelper{ + noConfirm: true, + Stdin: bufio.NewReader(refusingReader{t}), + VerboseErrWriter: &out, + } + + ok, err := h.Confirm("do you want to sign in?") + require.NoError(t, err) + assert.True(t, ok) + assert.Empty(t, out.String(), "no prompt should be shown when --yes is set") + }) + + for _, tc := range []struct { + name string + input string + want bool + }{ + {name: "case=prompts and accepts yes", input: "y\n", want: true}, + {name: "case=prompts and accepts no", input: "n\n", want: false}, + } { + t.Run(tc.name, func(t *testing.T) { + var out bytes.Buffer + h := &CommandHelper{ + noConfirm: false, + Stdin: bufio.NewReader(strings.NewReader(tc.input)), + VerboseErrWriter: &out, + } + + ok, err := h.Confirm("do you want to sign in?") + require.NoError(t, err) + assert.Equal(t, tc.want, ok) + assert.Contains(t, out.String(), "do you want to sign in?") + }) + } +} From c0d09bee640f097ce8b87422bb54bb6a11105e47 Mon Sep 17 00:00:00 2001 From: Arne Luenser Date: Wed, 29 Jul 2026 15:02:53 +0200 Subject: [PATCH 2/2] fix: make the sign-in confirmation reachable again Routing the prompt through Confirm was not enough: TemporaryAPIKey gated on Authenticate, which *performs* the interactive sign-in rather than checking for it. It never returns ErrNotAuthenticated, so the confirmation branch was dead code and --yes still had nothing to skip. Unauthenticated users were signed in via a browser without ever being asked. The quiet branch was dead for the same reason: Authenticate returns a plain error under --quiet, which matched neither sentinel and fell through to the catch-all, aborting the tunnel instead of continuing without an API key. TemporaryAPIKey now gates on checkAuthenticated and handles --quiet explicitly, matching how GetAuthenticatedConfig branches on the same sentinels. Confirm now errors under --quiet instead of writing an invisible prompt to io.Discard and blocking on stdin, and --yes also skips the legacy-config "Press enter to continue..." dialog, which blocked every command before the flag was ever consulted. TestTemporaryAPIKeyConfirmation drives the real call site offline via the browser hook, so the prompt cannot silently become unreachable again. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JYzGVwAKQ4ormxRHZg1eDu --- cmd/cloudx/client/api_key.go | 20 ++++++--- cmd/cloudx/client/command_helper.go | 6 +++ cmd/cloudx/client/config.go | 12 +++-- cmd/cloudx/client/confirm_test.go | 70 +++++++++++++++++++++++++++-- 4 files changed, 95 insertions(+), 13 deletions(-) diff --git a/cmd/cloudx/client/api_key.go b/cmd/cloudx/client/api_key.go index 7d083bfb..470523ca 100644 --- a/cmd/cloudx/client/api_key.go +++ b/cmd/cloudx/client/api_key.go @@ -87,11 +87,19 @@ func (h *CommandHelper) TemporaryAPIKey(ctx context.Context, name string, expire return *h.projectAPIKey, noop, nil } - // For all other projects, except the playground, we need to authenticate. - if err := h.Authenticate(ctx); errors.Is(err, ErrNoConfigQuiet) { - _, _ = fmt.Fprintf(h.VerboseErrWriter, "Because you are not authenticated, the Ory CLI can not configure your project automatically. You can still use the Ory Proxy / Ory Tunnel, but complex flows such as Social Sign In will not work. Remove the `--quiet` flag or run `ory auth login` to authenticate.") - return "", noop, nil - } else if errors.Is(err, ErrNotAuthenticated) { + // For all other projects, except the playground, we need to authenticate. We + // may only *check* here: Authenticate performs the interactive sign-in, so + // calling it up front would sign the user in without ever asking them, and + // would make both branches below unreachable. + switch err := h.checkAuthenticated(ctx); { + case err == nil: + // Already signed in, nothing to do. + case errors.Is(err, ErrNoConfig), errors.Is(err, ErrNotAuthenticated), errors.Is(err, ErrReauthenticate): + if h.isQuiet { + _, _ = fmt.Fprintf(h.VerboseErrWriter, "Because you are not authenticated, the Ory CLI can not configure your project automatically. You can still use the Ory Proxy / Ory Tunnel, but complex flows such as Social Sign In will not work. Remove the `--quiet` flag or run `ory auth login` to authenticate.") + return "", noop, nil + } + ok, err := h.Confirm("To support complex flows such as Social Sign In, the Ory CLI can configure your project automatically. To do so, you need to be signed in. Do you want to sign in?") if err != nil { return "", noop, err @@ -105,7 +113,7 @@ func (h *CommandHelper) TemporaryAPIKey(ctx context.Context, name string, expire if err := h.Authenticate(ctx); err != nil { return "", noop, err } - } else if err != nil { + default: return "", noop, err } diff --git a/cmd/cloudx/client/command_helper.go b/cmd/cloudx/client/command_helper.go index 9a8c32e6..07f6ac47 100644 --- a/cmd/cloudx/client/command_helper.go +++ b/cmd/cloudx/client/command_helper.go @@ -324,6 +324,12 @@ func (h *CommandHelper) Confirm(message string) (bool, error) { if h.noConfirm { return true, nil } + if h.isQuiet { + // VerboseErrWriter is io.Discard in quiet mode, so prompting would block + // on stdin without the user ever seeing the question. Fail loudly instead + // of hanging. + return false, errors.New("can not ask for confirmation when --quiet is set, use --yes to confirm automatically") + } return cmdx.AskScannerForConfirmation(message, h.Stdin, h.VerboseErrWriter) } diff --git a/cmd/cloudx/client/config.go b/cmd/cloudx/client/config.go index ff8632ac..23e3a231 100644 --- a/cmd/cloudx/client/config.go +++ b/cmd/cloudx/client/config.go @@ -96,10 +96,14 @@ func (h *CommandHelper) getConfig() (*Config, error) { } _, _ = fmt.Fprintln(h.VerboseErrWriter, "Thanks for upgrading! You will now be prompted to log in to the Ory CLI through the Ory Console.") - _, _ = fmt.Fprintln(h.VerboseErrWriter, "Press enter to continue...") - _, err := h.Stdin.ReadString('\n') - if err != nil && err != io.EOF { - return nil, fmt.Errorf("unable to read from stdin: %w", err) + // --yes must skip this dialog as well, otherwise unattended use + // blocks on a prompt nobody can answer. + if !h.noConfirm { + _, _ = fmt.Fprintln(h.VerboseErrWriter, "Press enter to continue...") + _, err := h.Stdin.ReadString('\n') + if err != nil && err != io.EOF { + return nil, fmt.Errorf("unable to read from stdin: %w", err) + } } } fallthrough diff --git a/cmd/cloudx/client/confirm_test.go b/cmd/cloudx/client/confirm_test.go index 42c1bce3..71ac5506 100644 --- a/cmd/cloudx/client/confirm_test.go +++ b/cmd/cloudx/client/confirm_test.go @@ -6,7 +6,10 @@ package client import ( "bufio" "bytes" + "context" + "errors" "io" + "path/filepath" "strings" "testing" @@ -15,12 +18,12 @@ import ( ) // refusingReader fails the test if anything tries to read from it. Reading -// stdin is exactly what --yes has to avoid: in CI there is nobody to answer, -// so the command would block forever. +// stdin is exactly what --yes and --quiet have to avoid: in CI there is nobody +// to answer, so the command would block forever. type refusingReader struct{ t *testing.T } func (r refusingReader) Read([]byte) (int, error) { - r.t.Error("stdin must not be read when --yes is set") + r.t.Error("stdin must not be read without an interactive confirmation") return 0, io.EOF } @@ -63,4 +66,65 @@ func TestConfirm(t *testing.T) { assert.Contains(t, out.String(), "do you want to sign in?") }) } + + t.Run("case=--quiet does not block on stdin", func(t *testing.T) { + h := &CommandHelper{ + isQuiet: true, + Stdin: bufio.NewReader(refusingReader{t}), + VerboseErrWriter: io.Discard, + } + + ok, err := h.Confirm("do you want to sign in?") + assert.Error(t, err) + assert.False(t, ok) + }) +} + +// TestTemporaryAPIKeyConfirmation guards the wiring of the confirmation: the +// prompt must actually be reachable. Gating on Authenticate instead of +// checkAuthenticated performs the sign-in instead of checking for it, which +// makes the prompt (and with it --yes) dead code. +func TestTemporaryAPIKeyConfirmation(t *testing.T) { + errBrowserOpened := errors.New("browser opened") + + newHelper := func(t *testing.T, stdin io.Reader, out io.Writer, opts ...CommandHelperOption) *CommandHelper { + h, err := NewCommandHelper(context.Background(), append([]CommandHelperOption{ + WithConfigLocation(filepath.Join(t.TempDir(), "config.json")), + WithStdin(stdin), + WithVerboseErrWriter(out), + WithOpenBrowserHook(func(string) error { return errBrowserOpened }), + }, opts...)...) + require.NoError(t, err) + return h + } + + t.Run("case=asks before signing in", func(t *testing.T) { + var out bytes.Buffer + h := newHelper(t, strings.NewReader("n\n"), &out) + + key, cleanup, err := h.TemporaryAPIKey(context.Background(), "test", 0) + require.NoError(t, err) + require.NoError(t, cleanup()) + assert.Empty(t, key) + assert.Contains(t, out.String(), "Do you want to sign in?") + }) + + t.Run("case=--yes skips the prompt and signs in", func(t *testing.T) { + var out bytes.Buffer + h := newHelper(t, refusingReader{t}, &out, WithNoConfirm(true)) + + _, _, err := h.TemporaryAPIKey(context.Background(), "test", 0) + assert.ErrorIs(t, err, errBrowserOpened) + }) + + t.Run("case=--quiet continues without an API key", func(t *testing.T) { + var out bytes.Buffer + h := newHelper(t, refusingReader{t}, &out, WithQuiet(true)) + + key, cleanup, err := h.TemporaryAPIKey(context.Background(), "test", 0) + require.NoError(t, err) + require.NoError(t, cleanup()) + assert.Empty(t, key) + assert.Contains(t, out.String(), "Remove the `--quiet` flag") + }) }