fix(secrets): handle file-keyring passphrase on headless tty

Author: salmonumbrella

internal/secrets/store.go

@@ -47,8 +47,10 @@ func keyringItem(key string, data []byte) keyring.Item {
 }
 
 const (
-	keyringPasswordEnv = "GOG_KEYRING_PASSWORD" //nolint:gosec // env var name, not a credential
-	keyringBackendEnv  = "GOG_KEYRING_BACKEND"  //nolint:gosec // env var name, not a credential
+	keyringPasswordEnv       = "GOG_KEYRING_PASSWORD"      //nolint:gosec // env var name, not a credential
+	keyringPasswordCompatEnv = "GOGCLI_KEYRING_PASSPHRASE" //nolint:gosec // env var name, not a credential
+	keyringPasswordLegacyEnv = "KEYRING_FILE_PASSPHRASE"   //nolint:gosec // env var name, not a credential
+	keyringBackendEnv        = "GOG_KEYRING_BACKEND"       //nolint:gosec // env var name, not a credential
 )
 
 var (
@@ -60,6 +62,8 @@ var (
 	errKeyringTimeout        = errors.New("keyring connection timed out")
 	openKeyringFunc          = openKeyring
 	keyringOpenFunc          = keyring.Open
+	openTTYFileFunc          = func() (*os.File, error) { return os.OpenFile("/dev/tty", os.O_RDWR, 0) }
+	terminalPromptFunc       = promptOnTerminal
 )
 
 type KeyringBackendInfo struct {
@@ -126,7 +130,7 @@ func fileKeyringPasswordFuncFrom(password string, passwordSet bool, isTTY bool)
 	}
 
 	if isTTY {
-		return keyring.TerminalPrompt
+		return terminalPromptFunc
 	}
 
 	return func(_ string) (string, error) {
@@ -135,8 +139,74 @@ func fileKeyringPasswordFuncFrom(password string, passwordSet bool, isTTY bool)
 }
 
 func fileKeyringPasswordFunc() keyring.PromptFunc {
-	password, passwordSet := os.LookupEnv(keyringPasswordEnv)
-	return fileKeyringPasswordFuncFrom(password, passwordSet, term.IsTerminal(int(os.Stdin.Fd())))
+	password, passwordSet := keyringPasswordFromEnv()
+	return fileKeyringPasswordFuncFrom(password, passwordSet, hasTerminalPromptInput())
+}
+
+func keyringPasswordFromEnvFrom(lookup func(string) (string, bool)) (string, bool) {
+	for _, envKey := range []string{keyringPasswordEnv, keyringPasswordCompatEnv, keyringPasswordLegacyEnv} {
+		if value, ok := lookup(envKey); ok {
+			return value, true
+		}
+	}
+
+	return "", false
+}
+
+func keyringPasswordFromEnv() (string, bool) {
+	return keyringPasswordFromEnvFrom(os.LookupEnv)
+}
+
+func hasTerminalPromptInputFrom(stdinIsTTY bool, openTTY func() error) bool {
+	if stdinIsTTY {
+		return true
+	}
+
+	if openTTY == nil {
+		return false
+	}
+
+	return openTTY() == nil
+}
+
+func hasTerminalPromptInput() bool {
+	return hasTerminalPromptInputFrom(term.IsTerminal(int(os.Stdin.Fd())), func() error {
+		tty, err := openTTYFileFunc()
+		if err != nil {
+			return err
+		}
+
+		return tty.Close()
+	})
+}
+
+func promptOnTerminal(prompt string) (string, error) {
+	tty, err := openTTYFileFunc()
+	if err != nil {
+		password, promptErr := keyring.TerminalPrompt(prompt)
+		if promptErr != nil {
+			return "", fmt.Errorf("prompt password from stdin: %w", promptErr)
+		}
+
+		return password, nil
+	}
+
+	defer tty.Close()
+
+	if _, err = fmt.Fprintf(tty, "%s: ", prompt); err != nil {
+		return "", fmt.Errorf("write tty prompt: %w", err)
+	}
+
+	password, err := term.ReadPassword(int(tty.Fd()))
+	if err != nil {
+		return "", fmt.Errorf("read tty password: %w", err)
+	}
+
+	if _, err = fmt.Fprintln(tty); err != nil {
+		return "", fmt.Errorf("write tty newline: %w", err)
+	}
+
+	return string(password), nil
 }
 
 func normalizeKeyringBackend(value string) string {

internal/secrets/store_more_test.go

@@ -13,7 +13,10 @@ import (
 	"github.com/steipete/gogcli/internal/config"
 )
 
-var errTestKeychain = errors.New("test -25308 error")
+var (
+	errTestKeychain = errors.New("test -25308 error")
+	errNoTTYForTest = errors.New("no tty")
+)
 
 func TestKeyringStore_ListDeleteDefault(t *testing.T) {
 	ring := keyring.NewArrayKeyring(nil)
@@ -126,6 +129,123 @@ func TestFileKeyringPasswordFuncFrom(t *testing.T) {
 	if _, err := fn("prompt"); err == nil || !errors.Is(err, errNoTTY) {
 		t.Fatalf("expected no TTY error, got: %v", err)
 	}
+
+	// No env var and TTY available uses terminal prompt function.
+	origTerminalPrompt := terminalPromptFunc
+	terminalPromptFunc = func(prompt string) (string, error) {
+		return "typed:" + prompt, nil
+	}
+
+	t.Cleanup(func() { terminalPromptFunc = origTerminalPrompt })
+
+	fn = fileKeyringPasswordFuncFrom("", false, true)
+	if got, err := fn("prompt"); err != nil {
+		t.Fatalf("expected terminal prompt success, got err: %v", err)
+	} else if got != "typed:prompt" {
+		t.Fatalf("unexpected prompt value: %q", got)
+	}
+}
+
+func TestKeyringPasswordFromEnvFrom(t *testing.T) {
+	tests := []struct {
+		name      string
+		env       map[string]string
+		wantValue string
+		wantSet   bool
+	}{
+		{
+			name:      "unset",
+			env:       map[string]string{},
+			wantValue: "",
+			wantSet:   false,
+		},
+		{
+			name: "canonical wins",
+			env: map[string]string{
+				keyringPasswordEnv:       "canon",
+				keyringPasswordCompatEnv: "compat",
+				keyringPasswordLegacyEnv: "legacy",
+			},
+			wantValue: "canon",
+			wantSet:   true,
+		},
+		{
+			name: "compat fallback",
+			env: map[string]string{
+				keyringPasswordCompatEnv: "compat",
+				keyringPasswordLegacyEnv: "legacy",
+			},
+			wantValue: "compat",
+			wantSet:   true,
+		},
+		{
+			name: "legacy fallback",
+			env: map[string]string{
+				keyringPasswordLegacyEnv: "legacy",
+			},
+			wantValue: "legacy",
+			wantSet:   true,
+		},
+		{
+			name: "empty canonical is intentional",
+			env: map[string]string{
+				keyringPasswordEnv: "",
+			},
+			wantValue: "",
+			wantSet:   true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotValue, gotSet := keyringPasswordFromEnvFrom(func(k string) (string, bool) {
+				v, ok := tt.env[k]
+				return v, ok
+			})
+			if gotValue != tt.wantValue || gotSet != tt.wantSet {
+				t.Fatalf("got (%q, %v), want (%q, %v)", gotValue, gotSet, tt.wantValue, tt.wantSet)
+			}
+		})
+	}
+}
+
+func TestHasTerminalPromptInputFrom(t *testing.T) {
+	t.Run("stdin tty", func(t *testing.T) {
+		called := false
+
+		got := hasTerminalPromptInputFrom(true, func() error {
+			called = true
+			return nil
+		})
+		if !got {
+			t.Fatalf("expected prompt input when stdin is tty")
+		}
+
+		if called {
+			t.Fatalf("did not expect openTTY call when stdin is tty")
+		}
+	})
+
+	t.Run("tty fallback works", func(t *testing.T) {
+		got := hasTerminalPromptInputFrom(false, func() error { return nil })
+		if !got {
+			t.Fatalf("expected prompt input from tty fallback")
+		}
+	})
+
+	t.Run("tty fallback unavailable", func(t *testing.T) {
+		got := hasTerminalPromptInputFrom(false, func() error { return errNoTTYForTest })
+		if got {
+			t.Fatalf("expected no prompt input without tty")
+		}
+	})
+
+	t.Run("no open func", func(t *testing.T) {
+		got := hasTerminalPromptInputFrom(false, nil)
+		if got {
+			t.Fatalf("expected no prompt input without openTTY function")
+		}
+	})
 }
 
 func TestKeyringStoreSetTokenErrors(t *testing.T) {