fix: validate device login URLs before launching browser

README.md

@@ -41,6 +41,9 @@ go install github.com/linuxfoundation/lfx-cli/cmd/lfx@latest
 # Log in via the Auth0 Device Code flow.
 lfx auth login
 
+# Automatically open the HTTPS verification URL in your default browser.
+lfx auth login --web
+
 # Log in against a non-production environment (aliases accepted).
 lfx auth login --env staging   # also: stage, stg
 lfx auth login --env dev       # also: develop, development
@@ -60,6 +63,10 @@ lfx api /projects --field name=example   # auto-promotes to POST
 lfx api -X PUT /projects/123 --input - -H "If-Match: <ver>" < input.json  # Content-Type: application/json is added automatically
 ```
 
+Automatic browser opening accepts only absolute HTTPS URLs with a hostname.
+Other verification URI values are displayed for manual inspection without
+launching them; control characters in login instructions are escaped.
+
 Credentials (refresh token, cached access token) are stored in your
 operating system's credential store by default (macOS Keychain, Windows
 Credential Manager, Linux Secret Service/KWallet/`pass`). Which of these is

internal/commands/auth.go

@@ -12,11 +12,14 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net/url"
 	"os"
 	"os/exec"
 	"runtime"
+	"strconv"
 	"strings"
 	"time"
+	"unicode"
 
 	"github.com/linuxfoundation/lfx-cli/internal/credstore"
 	"github.com/urfave/cli/v3"
@@ -217,22 +220,8 @@ func loginWithDeviceCode(
 		return fmt.Errorf("request device code: %w", err)
 	}
 
-	fmt.Printf("First copy your one-time code: %s\n", resp.UserCode)
-	// VerificationURIComplete is optional per RFC 8628 §3.2; fall back to
-	// VerificationURI (always present) plus the user code if the IdP
-	// doesn't supply it.
-	verificationURI := resp.VerificationURIComplete
-	if verificationURI == "" {
-		verificationURI = resp.VerificationURI
-	}
-	if cmd.Bool(webFlagName) {
-		fmt.Printf("Opening %s in your browser...\n", verificationURI)
-		if err := openBrowser(verificationURI); err != nil {
-			fmt.Printf("Couldn't open browser automatically: %v\n", err)
-			fmt.Printf("Please visit: %s\n", verificationURI)
-		}
-	} else {
-		fmt.Printf("Then visit: %s\n", verificationURI)
+	if err := printDeviceCodeInstructions(os.Stdout, resp, cmd.Bool(webFlagName)); err != nil {
+		return fmt.Errorf("display login instructions: %w", err)
 	}
 	fmt.Println("Waiting for authentication...")
 
@@ -756,15 +745,68 @@ func base64URLDecode(s string) ([]byte, error) {
 	return base64.RawURLEncoding.DecodeString(s)
 }
 
-// openBrowser opens url in the user's default browser, following the
-// per-OS conventions used by tools like `gh`.
-func openBrowser(url string) error {
+// printDeviceCodeInstructions displays untrusted IdP fields safely, retaining
+// manual instructions when automatic browser opening is refused or fails.
+func printDeviceCodeInstructions(w io.Writer, resp *oauth2.DeviceAuthResponse, web bool) error {
+	if _, err := fmt.Fprintf(w, "First copy your one-time code: %s\n", terminalSafeText(resp.UserCode)); err != nil {
+		return err
+	}
+	// VerificationURIComplete is optional per RFC 8628 §3.2.
+	verificationURI := resp.VerificationURIComplete
+	if verificationURI == "" {
+		verificationURI = resp.VerificationURI
+	}
+	displayURI := terminalSafeText(verificationURI)
+	if web {
+		if _, err := fmt.Fprintf(w, "Opening %s in your browser...\n", displayURI); err != nil {
+			return err
+		}
+		if err := openBrowser(verificationURI); err != nil {
+			if _, writeErr := fmt.Fprintf(w, "Couldn't open browser automatically: %v\n", err); writeErr != nil {
+				return writeErr
+			}
+			_, writeErr := fmt.Fprintf(w, "Please visit: %s\n", displayURI)
+			return writeErr
+		}
+	} else {
+		_, err := fmt.Fprintf(w, "Then visit: %s\n", displayURI)
+		return err
+	}
+	return nil
+}
+
+// terminalSafeText escapes control characters rather than letting IdP-supplied
+// values execute terminal escape sequences or forge additional output lines.
+func terminalSafeText(s string) string {
+	if strings.ContainsFunc(s, unicode.IsControl) {
+		return strconv.QuoteToASCII(s)
+	}
+	return s
+}
+
+// validateBrowserURL restricts OS launchers to absolute HTTPS URLs. These
+// launchers also accept local files and custom schemes, so exec.Command alone
+// is not sufficient to make an untrusted verification URI safe to launch.
+func validateBrowserURL(uri string) error {
+	u, err := url.Parse(uri)
+	if err != nil || u.Scheme != "https" || u.Hostname() == "" || u.Opaque != "" || strings.ContainsFunc(uri, unicode.IsControl) {
+		return errors.New("refusing to open verification URI: expected an absolute HTTPS URL with a hostname")
+	}
+	return nil
+}
+
+// openBrowser opens a validated HTTPS URL in the user's default browser,
+// following the per-OS conventions used by tools like `gh`.
+func openBrowser(uri string) error {
+	if err := validateBrowserURL(uri); err != nil {
+		return err
+	}
 	switch runtime.GOOS {
 	case "darwin":
-		return exec.Command("open", url).Start()
+		return exec.Command("open", uri).Start()
 	case "windows":
-		return exec.Command("rundll32", "url.dll,FileProtocolHandler", url).Start()
+		return exec.Command("rundll32", "url.dll,FileProtocolHandler", uri).Start()
 	default:
-		return exec.Command("xdg-open", url).Start()
+		return exec.Command("xdg-open", uri).Start()
 	}
 }

internal/commands/auth_test.go

@@ -4,16 +4,127 @@
 package commands
 
 import (
+	"bytes"
 	"context"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
+	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/linuxfoundation/lfx-cli/internal/credstore"
 	"github.com/urfave/cli/v3"
+	"golang.org/x/oauth2"
 )
 
+func TestValidateBrowserURL(t *testing.T) {
+	tests := []struct {
+		name  string
+		uri   string
+		valid bool
+	}{
+		{name: "HTTPS", uri: "https://example.test/activate", valid: true},
+		{name: "complete URI", uri: "https://example.test/activate?user_code=ABCD-EFGH", valid: true},
+		{name: "port and fragment", uri: "https://example.test:8443/activate#code", valid: true},
+		{name: "IPv6", uri: "https://[::1]/activate", valid: true},
+		{name: "empty"},
+		{name: "HTTP", uri: "http://example.test/activate"},
+		{name: "UNC", uri: `\\example.test\share\update.exe`},
+		{name: "Windows path", uri: `C:\Temp\update.exe`},
+		{name: "local path", uri: "/Applications/Example.app"},
+		{name: "file", uri: "file:///Applications/Example.app"},
+		{name: "remote file", uri: "file://example.test/share/update.exe"},
+		{name: "custom scheme", uri: "ms-msdt:/id Example"},
+		{name: "javascript", uri: "javascript:alert(1)"},
+		{name: "relative", uri: "/activate"},
+		{name: "protocol relative", uri: "//example.test/activate"},
+		{name: "missing host", uri: "https:///activate"},
+		{name: "empty hostname", uri: "https://:443/activate"},
+		{name: "opaque", uri: "https:example.test/activate"},
+		{name: "launcher option", uri: "--help"},
+		{name: "leading whitespace", uri: " https://example.test/activate"},
+		{name: "invalid escape", uri: "https://example.test/%zz"},
+		{name: "backslash host", uri: `https://example.test\share\update.exe`},
+		{name: "terminal escape", uri: "https://example.test/\x1b[31m"},
+		{name: "newline", uri: "https://example.test/\nforged output"},
+		{name: "Unicode control", uri: "https://example.test/\u009b31m"},
+	}
+
+	// No launcher is available: a rejected URI must produce the validation
+	// error, not an exec error, proving the guard runs before process startup.
+	t.Setenv("PATH", t.TempDir())
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			err := validateBrowserURL(tc.uri)
+			if tc.valid {
+				if err != nil {
+					t.Fatalf("validateBrowserURL(%q): %v", tc.uri, err)
+				}
+				return
+			}
+			if err == nil {
+				t.Fatalf("validateBrowserURL(%q): expected rejection", tc.uri)
+			}
+			if launchErr := openBrowser(tc.uri); launchErr == nil || launchErr.Error() != err.Error() {
+				t.Fatalf("openBrowser(%q) = %v, want validation error %v", tc.uri, launchErr, err)
+			}
+		})
+	}
+}
+
+func TestPrintDeviceCodeInstructionsRejectsUnsafeURIs(t *testing.T) {
+	t.Setenv("PATH", t.TempDir())
+	for _, uri := range []string{
+		`\\example.test\share\update.exe`,
+		"file:///Applications/Example.app",
+		"ms-msdt:/id Example",
+		"https://example.test/\x1b[31m\nforged output",
+	} {
+		for _, complete := range []bool{false, true} {
+			t.Run(strconv.FormatBool(complete)+"/"+uri, func(t *testing.T) {
+				resp := &oauth2.DeviceAuthResponse{
+					UserCode:        "ABCD-EFGH",
+					VerificationURI: uri,
+				}
+				if complete {
+					resp.VerificationURI = "https://example.test/activate"
+					resp.VerificationURIComplete = uri
+				}
+				var output bytes.Buffer
+				if err := printDeviceCodeInstructions(&output, resp, true); err != nil {
+					t.Fatalf("printDeviceCodeInstructions: %v", err)
+				}
+				got := output.String()
+				if !strings.Contains(got, "refusing to open verification URI") {
+					t.Fatalf("missing browser refusal: %q", got)
+				}
+				if !strings.Contains(got, "Please visit: "+terminalSafeText(uri)+"\n") {
+					t.Fatalf("missing manual instructions: %q", got)
+				}
+				if strings.Contains(got, "\x1b") || strings.Contains(got, "\nforged output") {
+					t.Fatalf("unsafe terminal output: %q", got)
+				}
+			})
+		}
+	}
+}
+
+func TestPrintDeviceCodeInstructionsManual(t *testing.T) {
+	resp := &oauth2.DeviceAuthResponse{
+		UserCode:        "ABCD-EFGH\x1b[31m",
+		VerificationURI: "https://example.test/activate",
+	}
+	var output bytes.Buffer
+	if err := printDeviceCodeInstructions(&output, resp, false); err != nil {
+		t.Fatalf("printDeviceCodeInstructions: %v", err)
+	}
+	want := "First copy your one-time code: \"ABCD-EFGH\\x1b[31m\"\nThen visit: https://example.test/activate\n"
+	if got := output.String(); got != want {
+		t.Fatalf("instructions = %q, want %q", got, want)
+	}
+}
+
 // newTestCommand builds a *cli.Command with the --insecure-storage and
 // --backend flags registered (as the root `lfx` command does via
 // CredentialStoreFlags), parses args against it, and returns the parsed