From 4249e5f50de99812459cbc572b2eb7192e1929da Mon Sep 17 00:00:00 2001 From: Matthias Wenz Date: Fri, 20 Mar 2026 08:22:09 +0100 Subject: [PATCH 1/5] Fix login: prefer complete URL for browser, handle Ctrl+C during prompt - Show clean verification_uri to the user, but open verification_uri_complete (with code pre-filled) in the browser - Make waitForEnter context-aware so Ctrl+C exits cleanly instead of leaving the process stuck on a blocking tty read Fixes regression from #728 where the browser-opened URL lost the auth code parameter. Co-Authored-By: Claude Opus 4.6 (1M context) Entire-Checkpoint: 1518a16ea037 --- cmd/entire/cli/login.go | 45 ++++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/cmd/entire/cli/login.go b/cmd/entire/cli/login.go index f2ceb6aed..780d78097 100644 --- a/cmd/entire/cli/login.go +++ b/cmd/entire/cli/login.go @@ -67,25 +67,34 @@ func runLogin(ctx context.Context, outW, errW io.Writer, client deviceAuthClient } fmt.Fprintf(outW, "Device code: %s\n", start.UserCode) - approvalURL := start.VerificationURI - if approvalURL == "" { - approvalURL = start.VerificationURIComplete + + // Use the clean URL (without code) for display, but open the complete URL + // (with code pre-filled) in the browser for a smoother experience. + displayURL := start.VerificationURI + browserURL := start.VerificationURIComplete + if browserURL == "" { + browserURL = displayURL + } + if displayURL == "" { + displayURL = browserURL } if canPromptInteractively() { - fmt.Fprintf(outW, "Press Enter to open %s in your browser...", approvalURL) + fmt.Fprintf(outW, "Press Enter to open %s in your browser...", displayURL) // Read from /dev/tty so we get a real keypress and don't consume piped stdin. - waitForEnter() + if err := waitForEnter(ctx); err != nil { + return fmt.Errorf("wait for input: %w", err) + } fmt.Fprintln(outW) - if err := openURL(ctx, approvalURL); err != nil { + if err := openURL(ctx, browserURL); err != nil { fmt.Fprintf(errW, "Warning: failed to open browser: %v\n", err) fmt.Fprintln(outW, "Open the approval URL in your browser to continue.") } } else { - fmt.Fprintf(outW, "Approval URL: %s\n", approvalURL) + fmt.Fprintf(outW, "Approval URL: %s\n", displayURL) } fmt.Fprintln(outW, "Waiting for approval...") @@ -170,16 +179,28 @@ func waitForApproval(ctx context.Context, poller deviceAuthClient, deviceCode st // waitForEnter reads a line from /dev/tty, blocking until the user presses Enter. // If /dev/tty cannot be opened (e.g. on Windows), it returns immediately. -func waitForEnter() { +// Returns ctx.Err() if the context is cancelled before the user presses Enter. +func waitForEnter(ctx context.Context) error { tty, err := os.Open("/dev/tty") if err != nil { - return + return nil } defer tty.Close() - reader := bufio.NewReader(tty) - if _, err = reader.ReadString('\n'); err != nil { - return + done := make(chan error, 1) + go func() { + reader := bufio.NewReader(tty) + _, err := reader.ReadString('\n') + done <- err + }() + + select { + case <-ctx.Done(): + // Close tty to unblock the reading goroutine. + _ = tty.Close() + return fmt.Errorf("interrupted: %w", ctx.Err()) + case <-done: + return nil } } From f1f9963dc534acac5bebe180f3fc7f63cdc83bb5 Mon Sep 17 00:00:00 2001 From: Matthias Wenz Date: Fri, 20 Mar 2026 08:22:18 +0100 Subject: [PATCH 2/5] Add logout command to remove stored credentials Adds `entire logout` which deletes the auth token from the OS keyring, fully reversing `entire login`. Co-Authored-By: Claude Opus 4.6 (1M context) Entire-Checkpoint: 43f1c3795354 --- cmd/entire/cli/logout.go | 27 ++++++++++++++++++ cmd/entire/cli/logout_test.go | 52 +++++++++++++++++++++++++++++++++++ cmd/entire/cli/root.go | 1 + 3 files changed, 80 insertions(+) create mode 100644 cmd/entire/cli/logout.go create mode 100644 cmd/entire/cli/logout_test.go diff --git a/cmd/entire/cli/logout.go b/cmd/entire/cli/logout.go new file mode 100644 index 000000000..de7080125 --- /dev/null +++ b/cmd/entire/cli/logout.go @@ -0,0 +1,27 @@ +package cli + +import ( + "fmt" + + apiurl "github.com/entireio/cli/cmd/entire/cli/api" + "github.com/entireio/cli/cmd/entire/cli/auth" + "github.com/spf13/cobra" +) + +func newLogoutCmd() *cobra.Command { + return &cobra.Command{ + Use: "logout", + Short: "Log out of Entire", + RunE: func(cmd *cobra.Command, _ []string) error { + store := auth.NewStore() + baseURL := apiurl.BaseURL() + + if err := store.DeleteToken(baseURL); err != nil { + return fmt.Errorf("remove auth token: %w", err) + } + + fmt.Fprintln(cmd.OutOrStdout(), "Logged out.") + return nil + }, + } +} diff --git a/cmd/entire/cli/logout_test.go b/cmd/entire/cli/logout_test.go new file mode 100644 index 000000000..0a9d4fe1a --- /dev/null +++ b/cmd/entire/cli/logout_test.go @@ -0,0 +1,52 @@ +package cli + +import ( + "bytes" + "strings" + "testing" +) + +func TestLogoutCmd_PrintsLoggedOut(t *testing.T) { + t.Parallel() + + cmd := newLogoutCmd() + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetArgs([]string{}) + + // RunE calls store.DeleteToken which hits the real keyring. + // On most CI/dev machines the keyring is available and DeleteToken + // is a no-op when no token exists, so this exercises the happy path. + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !strings.Contains(out.String(), "Logged out.") { + t.Fatalf("output = %q, want to contain %q", out.String(), "Logged out.") + } +} + +func TestLogoutCmd_IsRegistered(t *testing.T) { + t.Parallel() + + root := NewRootCmd() + found := false + for _, c := range root.Commands() { + if c.Use == "logout" { + found = true + break + } + } + if !found { + t.Fatal("logout command not registered on root") + } +} + +func TestLogoutCmd_HasShortDescription(t *testing.T) { + t.Parallel() + + cmd := newLogoutCmd() + if cmd.Short == "" { + t.Fatal("logout command has no short description") + } +} diff --git a/cmd/entire/cli/root.go b/cmd/entire/cli/root.go index 445fbf820..36680b954 100644 --- a/cmd/entire/cli/root.go +++ b/cmd/entire/cli/root.go @@ -88,6 +88,7 @@ func NewRootCmd() *cobra.Command { cmd.AddCommand(newDisableCmd()) cmd.AddCommand(newStatusCmd()) cmd.AddCommand(newLoginCmd()) + cmd.AddCommand(newLogoutCmd()) cmd.AddCommand(newHooksCmd()) cmd.AddCommand(newVersionCmd()) cmd.AddCommand(newExplainCmd()) From 7f77e310d8ca1237fa21eb3a649d3434615ec712 Mon Sep 17 00:00:00 2001 From: Matthias Wenz Date: Fri, 20 Mar 2026 08:32:02 +0100 Subject: [PATCH 3/5] Print complete auth URL on browser-open failure When the browser fails to open, print the full verification URL (with code pre-filled) so the user can copy-paste it directly. Co-Authored-By: Claude Opus 4.6 (1M context) Entire-Checkpoint: 6e7deeb97fd7 --- cmd/entire/cli/login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/entire/cli/login.go b/cmd/entire/cli/login.go index 780d78097..c70903f43 100644 --- a/cmd/entire/cli/login.go +++ b/cmd/entire/cli/login.go @@ -91,7 +91,7 @@ func runLogin(ctx context.Context, outW, errW io.Writer, client deviceAuthClient if err := openURL(ctx, browserURL); err != nil { fmt.Fprintf(errW, "Warning: failed to open browser: %v\n", err) - fmt.Fprintln(outW, "Open the approval URL in your browser to continue.") + fmt.Fprintf(outW, "Open the approval URL in your browser to continue: %s\n", browserURL) } } else { fmt.Fprintf(outW, "Approval URL: %s\n", displayURL) From 8131a03e15838f785a3682957a717ebf86f184b1 Mon Sep 17 00:00:00 2001 From: Matthias Wenz Date: Fri, 20 Mar 2026 08:54:39 +0100 Subject: [PATCH 4/5] Use mock keyring in logout tests Extract runLogout with a tokenDeleter interface so tests use a mock instead of the real OS keyring, preventing failures on CI. Co-Authored-By: Claude Opus 4.6 (1M context) Entire-Checkpoint: 1d26a3e7f936 --- cmd/entire/cli/logout.go | 26 +++++++++----- cmd/entire/cli/logout_test.go | 64 +++++++++++++++++++++++++---------- 2 files changed, 64 insertions(+), 26 deletions(-) diff --git a/cmd/entire/cli/logout.go b/cmd/entire/cli/logout.go index de7080125..e425e78b9 100644 --- a/cmd/entire/cli/logout.go +++ b/cmd/entire/cli/logout.go @@ -2,26 +2,34 @@ package cli import ( "fmt" + "io" apiurl "github.com/entireio/cli/cmd/entire/cli/api" "github.com/entireio/cli/cmd/entire/cli/auth" "github.com/spf13/cobra" ) +// tokenDeleter abstracts token removal so runLogout can be unit-tested +// without hitting the real OS keyring. +type tokenDeleter interface { + DeleteToken(baseURL string) error +} + func newLogoutCmd() *cobra.Command { return &cobra.Command{ Use: "logout", Short: "Log out of Entire", RunE: func(cmd *cobra.Command, _ []string) error { - store := auth.NewStore() - baseURL := apiurl.BaseURL() - - if err := store.DeleteToken(baseURL); err != nil { - return fmt.Errorf("remove auth token: %w", err) - } - - fmt.Fprintln(cmd.OutOrStdout(), "Logged out.") - return nil + return runLogout(cmd.OutOrStdout(), auth.NewStore(), apiurl.BaseURL()) }, } } + +func runLogout(outW io.Writer, store tokenDeleter, baseURL string) error { + if err := store.DeleteToken(baseURL); err != nil { + return fmt.Errorf("remove auth token: %w", err) + } + + fmt.Fprintln(outW, "Logged out.") + return nil +} diff --git a/cmd/entire/cli/logout_test.go b/cmd/entire/cli/logout_test.go index 0a9d4fe1a..01f96428f 100644 --- a/cmd/entire/cli/logout_test.go +++ b/cmd/entire/cli/logout_test.go @@ -2,30 +2,69 @@ package cli import ( "bytes" + "errors" "strings" "testing" ) -func TestLogoutCmd_PrintsLoggedOut(t *testing.T) { +type mockTokenDeleter struct { + deleted map[string]bool + failWith error +} + +func newMockTokenDeleter() *mockTokenDeleter { + return &mockTokenDeleter{deleted: make(map[string]bool)} +} + +func (m *mockTokenDeleter) DeleteToken(baseURL string) error { + if m.failWith != nil { + return m.failWith + } + m.deleted[baseURL] = true + return nil +} + +func TestRunLogout_DeletesTokenAndPrintsMessage(t *testing.T) { t.Parallel() - cmd := newLogoutCmd() + store := newMockTokenDeleter() var out bytes.Buffer - cmd.SetOut(&out) - cmd.SetArgs([]string{}) - // RunE calls store.DeleteToken which hits the real keyring. - // On most CI/dev machines the keyring is available and DeleteToken - // is a no-op when no token exists, so this exercises the happy path. - if err := cmd.Execute(); err != nil { + err := runLogout(&out, store, "https://entire.io") + if err != nil { t.Fatalf("unexpected error: %v", err) } + if !store.deleted["https://entire.io"] { + t.Fatal("expected token to be deleted for https://entire.io") + } + if !strings.Contains(out.String(), "Logged out.") { t.Fatalf("output = %q, want to contain %q", out.String(), "Logged out.") } } +func TestRunLogout_ReturnsErrorOnDeleteFailure(t *testing.T) { + t.Parallel() + + store := newMockTokenDeleter() + store.failWith = errors.New("keyring locked") + var out bytes.Buffer + + err := runLogout(&out, store, "https://entire.io") + if err == nil { + t.Fatal("expected error, got nil") + } + + if !strings.Contains(err.Error(), "keyring locked") { + t.Fatalf("error = %q, want to contain %q", err.Error(), "keyring locked") + } + + if strings.Contains(out.String(), "Logged out.") { + t.Fatal("should not print success message on error") + } +} + func TestLogoutCmd_IsRegistered(t *testing.T) { t.Parallel() @@ -41,12 +80,3 @@ func TestLogoutCmd_IsRegistered(t *testing.T) { t.Fatal("logout command not registered on root") } } - -func TestLogoutCmd_HasShortDescription(t *testing.T) { - t.Parallel() - - cmd := newLogoutCmd() - if cmd.Short == "" { - t.Fatal("logout command has no short description") - } -} From 86778ebd72ceca5234b6a6671e398cb1049e2345 Mon Sep 17 00:00:00 2001 From: Matthias Wenz Date: Fri, 20 Mar 2026 15:44:23 +0100 Subject: [PATCH 5/5] Only use verification_uri, never verification_uri_complete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Showing one URL but opening another is a phishing vector — an attacker who can tamper with the device auth response could set verification_uri_complete to a lookalike domain while the clean verification_uri looks legitimate in the terminal. Always show and open the same URL. The server can handle pre-filling the device code on its end. Co-Authored-By: Claude Opus 4.6 (1M context) Entire-Checkpoint: d9afebaaf0f8 --- cmd/entire/cli/login.go | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/cmd/entire/cli/login.go b/cmd/entire/cli/login.go index c70903f43..2c7036c29 100644 --- a/cmd/entire/cli/login.go +++ b/cmd/entire/cli/login.go @@ -68,19 +68,10 @@ func runLogin(ctx context.Context, outW, errW io.Writer, client deviceAuthClient fmt.Fprintf(outW, "Device code: %s\n", start.UserCode) - // Use the clean URL (without code) for display, but open the complete URL - // (with code pre-filled) in the browser for a smoother experience. - displayURL := start.VerificationURI - browserURL := start.VerificationURIComplete - if browserURL == "" { - browserURL = displayURL - } - if displayURL == "" { - displayURL = browserURL - } + approvalURL := start.VerificationURI if canPromptInteractively() { - fmt.Fprintf(outW, "Press Enter to open %s in your browser...", displayURL) + fmt.Fprintf(outW, "Press Enter to open %s in your browser and enter the generated device code...", approvalURL) // Read from /dev/tty so we get a real keypress and don't consume piped stdin. if err := waitForEnter(ctx); err != nil { @@ -89,12 +80,12 @@ func runLogin(ctx context.Context, outW, errW io.Writer, client deviceAuthClient fmt.Fprintln(outW) - if err := openURL(ctx, browserURL); err != nil { + if err := openURL(ctx, approvalURL); err != nil { fmt.Fprintf(errW, "Warning: failed to open browser: %v\n", err) - fmt.Fprintf(outW, "Open the approval URL in your browser to continue: %s\n", browserURL) + fmt.Fprintf(outW, "Open the approval URL in your browser to continue and enter the generated device code: %s\n", approvalURL) } } else { - fmt.Fprintf(outW, "Approval URL: %s\n", displayURL) + fmt.Fprintf(outW, "Approval URL: %s\n", approvalURL) } fmt.Fprintln(outW, "Waiting for approval...") @@ -183,9 +174,8 @@ func waitForApproval(ctx context.Context, poller deviceAuthClient, deviceCode st func waitForEnter(ctx context.Context) error { tty, err := os.Open("/dev/tty") if err != nil { - return nil + return nil //nolint:nilerr // tty unavailable (e.g. Windows) — skip prompt silently } - defer tty.Close() done := make(chan error, 1) go func() { @@ -200,6 +190,7 @@ func waitForEnter(ctx context.Context) error { _ = tty.Close() return fmt.Errorf("interrupted: %w", ctx.Err()) case <-done: + _ = tty.Close() return nil } }