From 62aea5f3f0c2d030e5a25b2baaf9c13cb4df6e4b Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Wed, 2 Sep 2026 12:45:04 -0500 Subject: [PATCH] credhelper: print the verification URL that carries the code The interactive prompt named codeResp.VerificationURI while openBrowser was handed verificationURL, the one with ?user_code= appended. Pressing Enter therefore always worked, which is why this went unnoticed; copying the printed URL instead landed on /device with no code. That matters more than it looks, because the branch tests the wrong thing. isTerminal(os.Stdin) asks whether stdin is a TTY, not whether a browser exists, so an SSH session on a headless box takes the headed path and is told to press Enter to open a browser it does not have. The non-interactive branch, which already printed the full URL, is only reached by piping stdin. Printing the code-carrying URL in both branches makes that mismatch moot rather than requiring a smarter predicate. Also give /device without a code the styled device-error page instead of bare text/plain, matching the expired-code path beside it, and stop renderError panicking when Templates is nil. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UAqi2hS2dhZoatqcWoYZQk --- pkg/appview/handlers/device.go | 19 ++++++++++++++++++- pkg/appview/handlers/device_test.go | 7 +++++++ pkg/credhelper/cmd_login.go | 6 +++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/pkg/appview/handlers/device.go b/pkg/appview/handlers/device.go index ee03531..6ce0ee7 100644 --- a/pkg/appview/handlers/device.go +++ b/pkg/appview/handlers/device.go @@ -200,7 +200,15 @@ func (h *DeviceApprovalPageHandler) ServeHTTP(w http.ResponseWriter, r *http.Req // Get user code from query userCode := r.URL.Query().Get("user_code") if userCode == "" { - http.Error(w, "user_code required", http.StatusBadRequest) + // The credential helper prints this URL without the code, so people + // arrive here by copying it by hand off a headless box. Bare text/plain + // gives them nothing to act on; the styled page at least names the way + // forward, same as the expired-code path just below. + // Older credential helpers print this address without the code, so + // people arrive here by copying it off a headless box. Point at the + // code their terminal did show rather than assuming the link they + // have carries it. + h.renderError(w, r, "This address is missing its one-time code. Your terminal shows the code when you run docker login: open the link printed with it, or run docker login again to start over.") return } @@ -455,6 +463,15 @@ func (h *DeviceApprovalPageHandler) renderError(w http.ResponseWriter, r *http.R Message: message, } + if h.Templates == nil { + // Degrade to plain text rather than panic on a nil template set. This + // page sits on the docker login path, so a rendering misconfiguration + // should still tell the user what happened. + slog.Error("Device error page has no templates configured", "component", "device/approve") + http.Error(w, message, http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "text/html; charset=utf-8") w.WriteHeader(http.StatusBadRequest) if err := h.Templates.ExecuteTemplate(w, "device-error", data); err != nil { diff --git a/pkg/appview/handlers/device_test.go b/pkg/appview/handlers/device_test.go index f1cc78a..9eee726 100644 --- a/pkg/appview/handlers/device_test.go +++ b/pkg/appview/handlers/device_test.go @@ -434,6 +434,13 @@ func TestDeviceApprovalPageHandler_MissingUserCode(t *testing.T) { if rr.Code != http.StatusBadRequest { t.Errorf("Expected status %d, got %d", http.StatusBadRequest, rr.Code) } + + // The body must tell the user how to recover. It used to be the bare + // string "user_code required", which is a dead end for someone who copied + // the verification URL off a headless box (BUGS finding 20). + if !strings.Contains(rr.Body.String(), "one-time code") { + t.Errorf("Expected an actionable message, got: %s", rr.Body.String()) + } } func TestDeviceApprovalPageHandler_MethodNotAllowed(t *testing.T) { diff --git a/pkg/credhelper/cmd_login.go b/pkg/credhelper/cmd_login.go index e6f2d11..38f476a 100644 --- a/pkg/credhelper/cmd_login.go +++ b/pkg/credhelper/cmd_login.go @@ -71,7 +71,11 @@ func runLogin(cmd *cobra.Command, args []string) error { if isTerminal(os.Stdin) { // Interactive: wait for Enter before opening browser - logInfof("Press Enter to open %s in your browser... ", codeResp.VerificationURI) + // Name the URL that carries the code, the same one openBrowser gets + // below. Printing the bare VerificationURI sent anyone who copied it by + // hand (headless box, browser did not open) to a page with no code and + // no way forward. + logInfof("Press Enter to open %s in your browser... ", verificationURL) reader := bufio.NewReader(os.Stdin) reader.ReadString('\n') //nolint:errcheck