Address PR review feedback for db save-password

- Add -p short flag for --password option
- Use term.ReadPassword instead of bufio.Scanner for secure password input
- Add TTY check before interactive prompt to fail gracefully in non-terminal environments
- Scope err variable to conditional block for cleaner code
- Output status messages to stderr instead of stdout for consistency
- Make TTY check and password reading testable via function variables
- Update tests to mock terminal behavior without requiring actual TTY
- Move TIGER_NEW_PASSWORD documentation from global to save-password section

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

Author: cevian

internal/tiger/cmd/db.go

@@ -1,7 +1,6 @@
 package cmd
 
 import (
-	"bufio"
 	"context"
 	"errors"
 	"fmt"
@@ -12,10 +11,12 @@ import (
 	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgconn"
 	"github.com/spf13/cobra"
+	"golang.org/x/term"
 
 	"github.com/timescale/tiger-cli/internal/tiger/api"
 	"github.com/timescale/tiger-cli/internal/tiger/config"
 	"github.com/timescale/tiger-cli/internal/tiger/password"
+	"github.com/timescale/tiger-cli/internal/tiger/util"
 )
 
 var (
@@ -24,6 +25,16 @@ var (
 
 	// getServiceDetailsFunc can be overridden for testing
 	getServiceDetailsFunc = getServiceDetails
+
+	// checkStdinIsTTY can be overridden for testing to bypass TTY detection
+	checkStdinIsTTY = func() bool {
+		return util.IsTerminal(os.Stdin)
+	}
+
+	// readPasswordFromTerminal can be overridden for testing to inject password input
+	readPasswordFromTerminal = func(fd int) ([]byte, error) {
+		return term.ReadPassword(fd)
+	}
 )
 
 func buildDbConnectionStringCmd() *cobra.Command {
@@ -301,37 +312,37 @@ Examples:
 				// Use environment variable
 				passwordToSave = envPassword
 			} else {
-				// Interactive prompt
+				// Interactive prompt - check if we're in a terminal
+				if !checkStdinIsTTY() {
+					return fmt.Errorf("TTY not detected - password required. Use --password flag or TIGER_NEW_PASSWORD environment variable")
+				}
+
 				fmt.Fprint(cmd.OutOrStdout(), "Enter password: ")
-				scanner := bufio.NewScanner(cmd.InOrStdin())
-				if scanner.Scan() {
-					passwordToSave = scanner.Text()
-				} else {
-					if err := scanner.Err(); err != nil {
-						return fmt.Errorf("failed to read password: %w", err)
-					}
-					return fmt.Errorf("failed to read password: EOF")
+				bytePassword, err := readPasswordFromTerminal(int(os.Stdin.Fd()))
+				if err != nil {
+					return fmt.Errorf("failed to read password: %w", err)
 				}
+				fmt.Fprintln(cmd.OutOrStdout()) // Print newline after hidden input
+				passwordToSave = string(bytePassword)
 				if passwordToSave == "" {
 					return fmt.Errorf("password cannot be empty")
 				}
 			}
 
 			// Save password using configured storage
 			storage := password.GetPasswordStorage()
-			err = storage.Save(service, passwordToSave, dbSavePasswordRole)
-			if err != nil {
+			if err := storage.Save(service, passwordToSave, dbSavePasswordRole); err != nil {
 				return fmt.Errorf("failed to save password: %w", err)
 			}
 
-			fmt.Fprintf(cmd.OutOrStdout(), "Password saved successfully for service %s (role: %s)\n",
+			fmt.Fprintf(cmd.ErrOrStderr(), "Password saved successfully for service %s (role: %s)\n",
 				*service.ServiceId, dbSavePasswordRole)
 			return nil
 		},
 	}
 
 	// Add flags for db save-password command
-	cmd.Flags().StringVar(&dbSavePasswordValue, "password", "", "Password to save")
+	cmd.Flags().StringVarP(&dbSavePasswordValue, "password", "p", "", "Password to save")
 	cmd.Flags().StringVar(&dbSavePasswordRole, "role", "tsdbadmin", "Database role/username")
 
 	return cmd

internal/tiger/cmd/db_test.go

@@ -1193,24 +1193,27 @@ func TestDBSavePassword_InteractivePrompt(t *testing.T) {
 
 	// Prepare the password input
 	testPassword := "interactive-password-999"
-	inputReader := strings.NewReader(testPassword + "\n")
 
-	// Build the command with custom stdin
-	testRoot := buildRootCmd()
-	buf := new(bytes.Buffer)
-	testRoot.SetOut(buf)
-	testRoot.SetErr(buf)
-	testRoot.SetIn(inputReader)                       // Use our reader as stdin
-	testRoot.SetArgs([]string{"db", "save-password"}) // No --password flag and no env var
+	// Mock TTY check to return true (simulate terminal)
+	originalCheckStdinIsTTY := checkStdinIsTTY
+	checkStdinIsTTY = func() bool {
+		return true
+	}
+	defer func() { checkStdinIsTTY = originalCheckStdinIsTTY }()
 
-	// Execute the command
-	err = testRoot.Execute()
+	// Mock password reading to return our test password
+	originalReadPasswordFromTerminal := readPasswordFromTerminal
+	readPasswordFromTerminal = func(fd int) ([]byte, error) {
+		return []byte(testPassword), nil
+	}
+	defer func() { readPasswordFromTerminal = originalReadPasswordFromTerminal }()
+
+	// Execute save-password without --password flag or env var
+	output, err := executeDBCommand("db", "save-password")
 	if err != nil {
 		t.Fatalf("Expected save-password to succeed with interactive input, got error: %v", err)
 	}
 
-	output := buf.String()
-
 	// Verify the prompt was shown
 	if !strings.Contains(output, "Enter password:") {
 		t.Errorf("Expected password prompt, got: %s", output)
@@ -1264,19 +1267,22 @@ func TestDBSavePassword_InteractivePromptEmpty(t *testing.T) {
 	// Make sure TIGER_NEW_PASSWORD is not set
 	os.Unsetenv("TIGER_NEW_PASSWORD")
 
-	// Prepare empty input (just a newline)
-	inputReader := strings.NewReader("\n")
+	// Mock TTY check to return true (simulate terminal)
+	originalCheckStdinIsTTY := checkStdinIsTTY
+	checkStdinIsTTY = func() bool {
+		return true
+	}
+	defer func() { checkStdinIsTTY = originalCheckStdinIsTTY }()
 
-	// Build the command with custom stdin
-	testRoot := buildRootCmd()
-	buf := new(bytes.Buffer)
-	testRoot.SetOut(buf)
-	testRoot.SetErr(buf)
-	testRoot.SetIn(inputReader)                       // Use our reader as stdin
-	testRoot.SetArgs([]string{"db", "save-password"}) // No --password flag or env var
+	// Mock password reading to return empty password
+	originalReadPasswordFromTerminal := readPasswordFromTerminal
+	readPasswordFromTerminal = func(fd int) ([]byte, error) {
+		return []byte(""), nil
+	}
+	defer func() { readPasswordFromTerminal = originalReadPasswordFromTerminal }()
 
 	// Execute the command
-	err = testRoot.Execute()
+	_, err = executeDBCommand("db", "save-password")
 	if err == nil {
 		t.Fatal("Expected error when user provides empty password interactively")
 	}

specs/spec.md

@@ -31,7 +31,6 @@ mv tiger /usr/local/bin/
 - `TIGER_CONFIG_DIR`: Configuration directory (default: ~/.config/tiger)
 - `TIGER_OUTPUT`: Default output format (json, yaml, table)
 - `TIGER_ANALYTICS`: Enable/disable usage analytics (true/false)
-- `TIGER_NEW_PASSWORD`: Password to use for save-password command (used when --password flag not provided)
 
 ### Configuration File
 
@@ -430,6 +429,12 @@ The `connect` and `psql` commands automatically handle authentication using:
 2. `PGPASSWORD` environment variable
 3. Interactive password prompt (if neither above is available)
 
+**Password Input for save-password:**
+The `save-password` command accepts passwords through three methods (in order of precedence):
+1. `--password` flag with explicit value (highest precedence)
+2. `TIGER_NEW_PASSWORD` environment variable
+3. Interactive prompt (if neither provided and stdin is a TTY)
+
 **Advanced psql Usage:**
 The `connect` and `psql` commands support passing additional flags directly to the psql client using the `--` separator. Any flags after `--` are passed through to psql unchanged, allowing full access to psql's functionality while maintaining tiger's connection and authentication handling.