From ddffe6de69806bcd1eb5526f957ff63aa8546d69 Mon Sep 17 00:00:00 2001 From: Jascha Date: Fri, 12 Dec 2025 15:48:26 +0100 Subject: [PATCH 1/3] Add go-expect based integration test for password resets --- go.mod | 2 + go.sum | 5 + internal/tiger/cmd/integration_test.go | 387 +++++++++++++++++++++++++ 3 files changed, 394 insertions(+) diff --git a/go.mod b/go.mod index 7b70464b..f8b910d8 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ tool ( require ( github.com/Masterminds/semver/v3 v3.4.0 + github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2 github.com/charmbracelet/bubbletea v1.3.10 github.com/cli/safeexec v1.0.1 github.com/fatih/color v1.18.0 @@ -55,6 +56,7 @@ require ( github.com/containerd/errdefs v1.0.0 // indirect github.com/containerd/errdefs/pkg v0.3.0 // indirect github.com/containerd/stargz-snapshotter/estargz v0.18.1 // indirect + github.com/creack/pty v1.1.17 // indirect github.com/danieljoos/wincred v1.2.3 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/distribution/reference v0.6.0 // indirect diff --git a/go.sum b/go.sum index d06379cc..b01604c0 100644 --- a/go.sum +++ b/go.sum @@ -8,6 +8,8 @@ github.com/Masterminds/semver/v3 v3.4.0 h1:Zog+i5UMtVoCU8oKka5P7i9q9HgrJeGzI9SA1 github.com/Masterminds/semver/v3 v3.4.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= +github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2 h1:+vx7roKuyA63nhn5WAunQHLTznkw5W8b1Xc0dNjp83s= +github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2/go.mod h1:HBCaDeC1lPdgDeDbhX8XFpy1jqjK0IBG8W5K+xYqA0w= github.com/RaveNoX/go-jsoncommentstrip v1.0.0/go.mod h1:78ihd09MekBnJnxpICcwzCMzGrKSKYe4AqU6PDYYpjk= github.com/adrg/xdg v0.5.3 h1:xRnxJXne7+oWDatRhR1JLnvuccuIeCoBu2rtuLqQB78= github.com/adrg/xdg v0.5.3/go.mod h1:nlTsY+NNiCBGCK2tpm09vRqfVzrc2fLmXGpBLF0zlTQ= @@ -56,6 +58,8 @@ github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3 github.com/containerd/stargz-snapshotter/estargz v0.18.1 h1:cy2/lpgBXDA3cDKSyEfNOFMA/c10O1axL69EU7iirO8= github.com/containerd/stargz-snapshotter/estargz v0.18.1/go.mod h1:ALIEqa7B6oVDsrF37GkGN20SuvG/pIMm7FwP7ZmRb0Q= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/creack/pty v1.1.17 h1:QeVUsEDNrLBW4tMgZHvxy18sKtr6VI492kBhUfhDJNI= +github.com/creack/pty v1.1.17/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMFLdQ= github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7MRLQK4X0bs= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -372,6 +376,7 @@ github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/ github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= diff --git a/internal/tiger/cmd/integration_test.go b/internal/tiger/cmd/integration_test.go index 8ee47f7f..de206802 100644 --- a/internal/tiger/cmd/integration_test.go +++ b/internal/tiger/cmd/integration_test.go @@ -6,11 +6,14 @@ import ( "encoding/json" "fmt" "os" + "os/exec" + "path/filepath" "regexp" "strings" "testing" "time" + expect "github.com/Netflix/go-expect" "github.com/spf13/viper" "github.com/timescale/tiger-cli/internal/tiger/api" @@ -2139,3 +2142,387 @@ func TestServiceForkIntegration(t *testing.T) { t.Logf("Logout successful") }) } + +// buildTigerBinary compiles the CLI binary for PTY-based testing +func buildTigerBinary(t *testing.T, tmpDir string) string { + t.Helper() + + binaryPath := filepath.Join(tmpDir, "tiger") + + // Find project root by looking for go.mod + projectRoot, err := findProjectRoot() + if err != nil { + t.Fatalf("Failed to find project root: %v", err) + } + + cmd := exec.Command("go", "build", "-o", binaryPath, "./cmd/tiger") + cmd.Dir = projectRoot + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("Failed to build tiger binary: %v\nOutput: %s", err, output) + } + + return binaryPath +} + +// findProjectRoot finds the project root directory by looking for go.mod +func findProjectRoot() (string, error) { + // Start from current working directory and walk up + dir, err := os.Getwd() + if err != nil { + return "", err + } + + for { + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { + return dir, nil + } + + parent := filepath.Dir(dir) + if parent == dir { + return "", fmt.Errorf("could not find go.mod in any parent directory") + } + dir = parent + } +} + +// buildTestEnv creates environment variables for PTY test binary +func buildTestEnv(tmpDir string) []string { + env := os.Environ() + // Override config directory to use test temp dir + env = append(env, "TIGER_CONFIG_DIR="+tmpDir) + // Disable analytics for tests + env = append(env, "TIGER_ANALYTICS=false") + // Use pgpass storage for easier testing (no keyring interaction) + env = append(env, "TIGER_PASSWORD_STORAGE=pgpass") + return env +} + +// TestDbConnectPasswordResetIntegration tests the interactive password reset flow +// using go-expect for PTY simulation +func TestDbConnectPasswordResetIntegration(t *testing.T) { + config.SetTestServiceName(t) + + // Check for required environment variables + publicKey := os.Getenv("TIGER_PUBLIC_KEY_INTEGRATION") + secretKey := os.Getenv("TIGER_SECRET_KEY_INTEGRATION") + if publicKey == "" || secretKey == "" { + t.Skip("Skipping integration test: TIGER_PUBLIC_KEY_INTEGRATION and TIGER_SECRET_KEY_INTEGRATION must be set") + } + + // Check if psql is available + if _, err := exec.LookPath("psql"); err != nil { + t.Skip("Skipping integration test: psql not available") + } + + // Set up isolated test environment with temporary config directory + tmpDir := setupIntegrationTest(t) + t.Logf("Using temporary config directory: %s", tmpDir) + + // Use pgpass storage for entire test so main process and subprocess use the same storage + os.Setenv("TIGER_PASSWORD_STORAGE", "pgpass") + t.Cleanup(func() { + os.Unsetenv("TIGER_PASSWORD_STORAGE") + }) + + var serviceID string + + // Always logout at the end to clean up credentials + defer func() { + t.Logf("Cleaning up authentication") + _, err := executeIntegrationCommand(t.Context(), "auth", "logout") + if err != nil { + t.Logf("Warning: Failed to logout: %v", err) + } + }() + + // Cleanup function to ensure service is deleted even if test fails + defer func() { + if serviceID != "" { + t.Logf("Cleaning up service: %s", serviceID) + _, err := executeIntegrationCommand( + t.Context(), + "service", "delete", serviceID, + "--confirm", + "--wait-timeout", "5m", + ) + if err != nil { + t.Logf("Warning: Failed to cleanup service %s: %v", serviceID, err) + } + } + }() + + t.Run("Login", func(t *testing.T) { + t.Logf("Logging in with public key: %s...", publicKey[:8]) + + // Login in main test process (uses test-specific keyring service name) + output, err := executeIntegrationCommand( + t.Context(), + "auth", "login", + "--public-key", publicKey, + "--secret-key", secretKey, + ) + + if err != nil { + t.Fatalf("Login failed: %v\nOutput: %s", err, output) + } + + if !strings.Contains(output, "Successfully logged in") { + t.Errorf("Login output doesn't contain success message: %s", output) + } + }) + + t.Run("CreateService", func(t *testing.T) { + t.Logf("Creating service for password reset test") + + output, err := executeIntegrationCommand( + t.Context(), + "service", "create", + "--wait-timeout", "15m", + "--no-set-default", + "--output", "json", + ) + + if err != nil { + t.Fatalf("Service creation failed: %v\nOutput: %s", err, output) + } + + // Extract service ID from JSON output + serviceID = extractServiceIDFromCreateOutput(t, output) + t.Logf("Created service with ID: %s", serviceID) + }) + + // Build tiger binary once for PTY testing (used by multiple subtests) + binaryPath := buildTigerBinary(t, tmpDir) + t.Logf("Built tiger binary at: %s", binaryPath) + + t.Run("SubprocessLogin", func(t *testing.T) { + // Run tiger auth login in subprocess environment to store credentials + // in the system keyring (which subprocess will use) + t.Logf("Running tiger auth login in subprocess environment") + + cmd := exec.Command(binaryPath, "auth", "login", + "--public-key", publicKey, + "--secret-key", secretKey, + ) + cmd.Env = buildTestEnv(tmpDir) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("Subprocess auth login failed: %v\nOutput: %s", err, output) + } + t.Logf("Subprocess auth login output: %s", output) + }) + + t.Run("VerifyInitialConnection", func(t *testing.T) { + if serviceID == "" { + t.Skip("No service ID available") + } + + t.Logf("Verifying initial connection works before deleting password") + + // Create PTY console + c, err := expect.NewConsole( + expect.WithDefaultTimeout(60*time.Second), + expect.WithStdout(os.Stdout), + ) + if err != nil { + t.Fatalf("Failed to create PTY console: %v", err) + } + defer c.Close() + + // Run tiger db connect + cmd := exec.Command(binaryPath, "db", "connect", serviceID) + cmd.Stdin = c.Tty() + cmd.Stdout = c.Tty() + cmd.Stderr = c.Tty() + cmd.Env = buildTestEnv(tmpDir) + + if err := cmd.Start(); err != nil { + t.Fatalf("Failed to start command: %v", err) + } + + // Should connect directly to psql (no menu since password is valid) + // Wait for psql prompt + t.Logf("Waiting for psql prompt...") + _, err = c.ExpectString("tsdb=>") + if err != nil { + t.Fatalf("Expected psql prompt, got error: %v", err) + } + t.Logf("Got psql prompt - connection works!") + + // Exit psql + c.SendLine("\\q") + cmd.Wait() + + t.Logf("Initial connection verified successfully") + }) + + t.Run("DeleteStoredPassword", func(t *testing.T) { + if serviceID == "" { + t.Skip("No service ID available") + } + + t.Logf("Deleting stored password to trigger recovery flow") + + // Get service details to remove password + cfg, err := config.Load() + if err != nil { + t.Fatalf("Failed to load config: %v", err) + } + + apiKey, projectID, err := config.GetCredentials() + if err != nil { + t.Fatalf("Failed to get credentials: %v", err) + } + + client, err := api.NewTigerClient(cfg, apiKey) + if err != nil { + t.Fatalf("Failed to create client: %v", err) + } + + resp, err := client.GetProjectsProjectIdServicesServiceIdWithResponse(t.Context(), projectID, serviceID) + if err != nil { + t.Fatalf("Failed to get service: %v", err) + } + + if resp.JSON200 == nil { + t.Fatalf("Service not found") + } + + service := *resp.JSON200 + + // Remove password from pgpass storage + storage := &common.PgpassStorage{} + if err := storage.Remove(service, "tsdbadmin"); err != nil { + t.Logf("Warning: Failed to remove password from pgpass (may not exist): %v", err) + } + + t.Logf("Password removed from storage") + }) + + t.Run("InteractivePasswordReset", func(t *testing.T) { + if serviceID == "" { + t.Skip("No service ID available") + } + + t.Logf("Running interactive password reset flow") + + // Create PTY console with timeout and stdout capture for debugging + c, err := expect.NewConsole( + expect.WithDefaultTimeout(90*time.Second), + expect.WithStdout(os.Stdout), // Echo output to test log + ) + if err != nil { + t.Fatalf("Failed to create PTY console: %v", err) + } + defer c.Close() + + // Set up command with PTY + cmd := exec.Command(binaryPath, "db", "connect", serviceID) + cmd.Stdin = c.Tty() + cmd.Stdout = c.Tty() + cmd.Stderr = c.Tty() + cmd.Env = buildTestEnv(tmpDir) + + if err := cmd.Start(); err != nil { + t.Fatalf("Failed to start command: %v", err) + } + + // Wait for the menu prompt (auth failure message comes before it) + t.Logf("Waiting for menu prompt...") + _, err = c.ExpectString("What would you like to do?") + if err != nil { + t.Fatalf("Expected menu prompt: %v", err) + } + t.Logf("Got menu prompt") + + // Select "Update/reset password" (option 2) + t.Logf("Selecting option 2 (Update/reset password)") + c.Send("2") + + // Expect password prompt + t.Logf("Waiting for password prompt...") + _, err = c.ExpectString("Enter new password") + if err != nil { + t.Fatalf("Expected password prompt: %v", err) + } + t.Logf("Got password prompt") + + // Send empty to auto-generate password + t.Logf("Sending empty password to auto-generate") + c.SendLine("") + + // Expect success message + t.Logf("Waiting for success message...") + _, err = c.ExpectString("updated successfully") + if err != nil { + t.Fatalf("Expected success message: %v", err) + } + t.Logf("Got success message") + + // psql launches - wait for any prompt indicator then exit + t.Logf("Waiting for psql to launch...") + // Give psql time to connect and show prompt + time.Sleep(2 * time.Second) + + // Exit psql with \q + t.Logf("Sending \\q to exit psql") + c.SendLine("\\q") + + // Wait for command to complete + err = cmd.Wait() + if err != nil { + t.Logf("Command exited with: %v (this may be expected)", err) + } + + t.Logf("Interactive password reset completed") + }) + + t.Run("VerifyPasswordReset", func(t *testing.T) { + if serviceID == "" { + t.Skip("No service ID available") + } + + t.Logf("Verifying password was reset successfully") + + // Test connection with the new password (should be saved in pgpass) + output, err := executeIntegrationCommand( + t.Context(), + "db", "test-connection", serviceID, + "--timeout", "30s", + ) + + if err != nil { + t.Fatalf("Connection failed after password reset: %v\nOutput: %s", err, output) + } + + if !strings.Contains(output, "Connection successful") { + t.Errorf("Expected successful connection, got: %s", output) + } + + t.Logf("Password reset verified - connection successful") + }) + + t.Run("DeleteService", func(t *testing.T) { + if serviceID == "" { + t.Skip("No service ID available") + } + + t.Logf("Deleting service: %s", serviceID) + + output, err := executeIntegrationCommand( + t.Context(), + "service", "delete", serviceID, + "--confirm", + "--wait-timeout", "5m", + ) + + if err != nil { + t.Fatalf("Service deletion failed: %v\nOutput: %s", err, output) + } + + // Clear serviceID so cleanup doesn't try to delete again + serviceID = "" + t.Logf("Service deleted successfully") + }) +} From 7cf6d3831256cced302683f857340356215ccc66 Mon Sep 17 00:00:00 2001 From: Jascha Date: Tue, 16 Dec 2025 14:52:03 +0100 Subject: [PATCH 2/3] Use stored credentials from config file when env var is given --- internal/tiger/cmd/integration_test.go | 29 +++++++------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/internal/tiger/cmd/integration_test.go b/internal/tiger/cmd/integration_test.go index de206802..967f4bcd 100644 --- a/internal/tiger/cmd/integration_test.go +++ b/internal/tiger/cmd/integration_test.go @@ -2187,7 +2187,7 @@ func findProjectRoot() (string, error) { } // buildTestEnv creates environment variables for PTY test binary -func buildTestEnv(tmpDir string) []string { +func buildTestEnv(tmpDir, publicKey, secretKey string) []string { env := os.Environ() // Override config directory to use test temp dir env = append(env, "TIGER_CONFIG_DIR="+tmpDir) @@ -2195,6 +2195,10 @@ func buildTestEnv(tmpDir string) []string { env = append(env, "TIGER_ANALYTICS=false") // Use pgpass storage for easier testing (no keyring interaction) env = append(env, "TIGER_PASSWORD_STORAGE=pgpass") + // Pass credentials via environment so subprocess uses them directly + // (bypasses keyring which may have different credentials) + env = append(env, "TIGER_PUBLIC_KEY="+publicKey) + env = append(env, "TIGER_SECRET_KEY="+secretKey) return env } @@ -2296,23 +2300,6 @@ func TestDbConnectPasswordResetIntegration(t *testing.T) { binaryPath := buildTigerBinary(t, tmpDir) t.Logf("Built tiger binary at: %s", binaryPath) - t.Run("SubprocessLogin", func(t *testing.T) { - // Run tiger auth login in subprocess environment to store credentials - // in the system keyring (which subprocess will use) - t.Logf("Running tiger auth login in subprocess environment") - - cmd := exec.Command(binaryPath, "auth", "login", - "--public-key", publicKey, - "--secret-key", secretKey, - ) - cmd.Env = buildTestEnv(tmpDir) - output, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("Subprocess auth login failed: %v\nOutput: %s", err, output) - } - t.Logf("Subprocess auth login output: %s", output) - }) - t.Run("VerifyInitialConnection", func(t *testing.T) { if serviceID == "" { t.Skip("No service ID available") @@ -2335,7 +2322,7 @@ func TestDbConnectPasswordResetIntegration(t *testing.T) { cmd.Stdin = c.Tty() cmd.Stdout = c.Tty() cmd.Stderr = c.Tty() - cmd.Env = buildTestEnv(tmpDir) + cmd.Env = buildTestEnv(tmpDir, publicKey, secretKey) if err := cmd.Start(); err != nil { t.Fatalf("Failed to start command: %v", err) @@ -2380,7 +2367,7 @@ func TestDbConnectPasswordResetIntegration(t *testing.T) { t.Fatalf("Failed to create client: %v", err) } - resp, err := client.GetProjectsProjectIdServicesServiceIdWithResponse(t.Context(), projectID, serviceID) + resp, err := client.GetServiceWithResponse(t.Context(), projectID, serviceID) if err != nil { t.Fatalf("Failed to get service: %v", err) } @@ -2422,7 +2409,7 @@ func TestDbConnectPasswordResetIntegration(t *testing.T) { cmd.Stdin = c.Tty() cmd.Stdout = c.Tty() cmd.Stderr = c.Tty() - cmd.Env = buildTestEnv(tmpDir) + cmd.Env = buildTestEnv(tmpDir, publicKey, secretKey) if err := cmd.Start(); err != nil { t.Fatalf("Failed to start command: %v", err) From bb374269f1909f5539c000d40e6ab8b82e44060a Mon Sep 17 00:00:00 2001 From: Jascha Date: Fri, 23 Jan 2026 14:19:40 +0100 Subject: [PATCH 3/3] Refactor stdin/password reading for better testability - Replace generic `readString` with specific `readLine` and `readPassword` functions for clearer intent and simpler call sites - Rename `checkStdinIsTTY` to `checkOutputIsTTY` to accurately reflect that it checks the output stream for TTY detection - Fix parameter ordering in `selectPasswordRecoveryOption` to be consistent (in, out) with other functions - Integration tests now use `buildRootCmd()` with cobra's SetIn/SetOut instead of spawning a subprocess, enabling direct PTY interaction - Remove `time.Sleep` calls from integration tests, use proper `ExpectString` waits instead - Remove unused helper functions: `buildTigerBinary`, `findProjectRoot`, `buildTestEnv` Co-Authored-By: Claude Opus 4.5 --- internal/tiger/cmd/auth.go | 11 +- internal/tiger/cmd/db.go | 21 ++-- internal/tiger/cmd/db_test.go | 25 ++--- internal/tiger/cmd/integration_test.go | 137 ++++++++---------------- internal/tiger/cmd/password_recovery.go | 15 +-- internal/tiger/cmd/root.go | 49 ++++++++- internal/tiger/cmd/service.go | 10 +- 7 files changed, 123 insertions(+), 145 deletions(-) diff --git a/internal/tiger/cmd/auth.go b/internal/tiger/cmd/auth.go index efffe31e..083f153e 100644 --- a/internal/tiger/cmd/auth.go +++ b/internal/tiger/cmd/auth.go @@ -1,7 +1,6 @@ package cmd import ( - "bufio" "context" "errors" "fmt" @@ -12,7 +11,6 @@ import ( "github.com/olekukonko/tablewriter" "github.com/spf13/cobra" - "golang.org/x/term" "golang.org/x/text/cases" "golang.org/x/text/language" @@ -286,12 +284,10 @@ func promptForCredentials(ctx context.Context, consoleURL string, creds credenti fmt.Printf("You can find your API credentials at: %s/dashboard/settings\n\n", consoleURL) - reader := bufio.NewReader(os.Stdin) - // Prompt for public key if missing if creds.publicKey == "" { fmt.Print("Enter your public key: ") - publicKey, err := readString(ctx, func() (string, error) { return reader.ReadString('\n') }) + publicKey, err := readLine(ctx, os.Stdin) if err != nil { return credentials{}, err } @@ -301,10 +297,7 @@ func promptForCredentials(ctx context.Context, consoleURL string, creds credenti // Prompt for secret key if missing if creds.secretKey == "" { fmt.Print("Enter your secret key: ") - password, err := readString(ctx, func() (string, error) { - val, err := term.ReadPassword(int(os.Stdin.Fd())) - return string(val), err - }) + password, err := readPassword(ctx, os.Stdin) if err != nil { return credentials{}, err } diff --git a/internal/tiger/cmd/db.go b/internal/tiger/cmd/db.go index 105c06bb..b4159dd3 100644 --- a/internal/tiger/cmd/db.go +++ b/internal/tiger/cmd/db.go @@ -6,6 +6,7 @@ import ( "encoding/base64" "errors" "fmt" + "io" "net/http" "os" "os/exec" @@ -15,7 +16,6 @@ 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/common" @@ -26,18 +26,9 @@ 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() (string, error) { - val, err := term.ReadPassword(int(os.Stdin.Fd())) - if err != nil { - return "", err - } - return string(val), nil + // checkOutputIsTTY can be overridden for testing to bypass TTY detection + checkOutputIsTTY = func(w io.Writer) bool { + return util.IsTerminal(w) } ) @@ -350,12 +341,12 @@ Examples: passwordToSave = envPassword } else { // Interactive prompt - check if we're in a terminal - if !checkStdinIsTTY() { + if !checkOutputIsTTY(cmd.OutOrStdout()) { return fmt.Errorf("TTY not detected - password required. Use --password flag or TIGER_NEW_PASSWORD environment variable") } fmt.Fprint(cmd.OutOrStdout(), "Enter password: ") - passwordToSave, err = readString(cmd.Context(), readPasswordFromTerminal) + passwordToSave, err = readPassword(cmd.Context(), cmd.InOrStdin()) if err != nil { return fmt.Errorf("failed to read password: %w", err) } diff --git a/internal/tiger/cmd/db_test.go b/internal/tiger/cmd/db_test.go index 410938e7..e3b89aef 100644 --- a/internal/tiger/cmd/db_test.go +++ b/internal/tiger/cmd/db_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "io" "os" "strings" "testing" @@ -1147,18 +1148,18 @@ func TestDBSavePassword_InteractivePrompt(t *testing.T) { testPassword := "interactive-password-999" // Mock TTY check to return true (simulate terminal) - originalCheckStdinIsTTY := checkStdinIsTTY - checkStdinIsTTY = func() bool { + originalCheckStdinIsTTY := checkOutputIsTTY + checkOutputIsTTY = func(w io.Writer) bool { return true } - defer func() { checkStdinIsTTY = originalCheckStdinIsTTY }() + defer func() { checkOutputIsTTY = originalCheckStdinIsTTY }() // Mock password reading to return our test password - originalReadPasswordFromTerminal := readPasswordFromTerminal - readPasswordFromTerminal = func() (string, error) { + originalReadPassword := readPassword + readPassword = func(ctx context.Context, in io.Reader) (string, error) { return testPassword, nil } - defer func() { readPasswordFromTerminal = originalReadPasswordFromTerminal }() + defer func() { readPassword = originalReadPassword }() // Execute save-password without --password flag or env var output, err := executeDBCommand(t.Context(), "db", "save-password") @@ -1225,18 +1226,18 @@ func TestDBSavePassword_InteractivePromptEmpty(t *testing.T) { os.Unsetenv("TIGER_NEW_PASSWORD") // Mock TTY check to return true (simulate terminal) - originalCheckStdinIsTTY := checkStdinIsTTY - checkStdinIsTTY = func() bool { + originalCheckStdinIsTTY := checkOutputIsTTY + checkOutputIsTTY = func(w io.Writer) bool { return true } - defer func() { checkStdinIsTTY = originalCheckStdinIsTTY }() + defer func() { checkOutputIsTTY = originalCheckStdinIsTTY }() // Mock password reading to return empty password - originalReadPasswordFromTerminal := readPasswordFromTerminal - readPasswordFromTerminal = func() (string, error) { + originalReadPassword := readPassword + readPassword = func(ctx context.Context, in io.Reader) (string, error) { return "", nil } - defer func() { readPasswordFromTerminal = originalReadPasswordFromTerminal }() + defer func() { readPassword = originalReadPassword }() // Execute the command _, err = executeDBCommand(t.Context(), "db", "save-password") diff --git a/internal/tiger/cmd/integration_test.go b/internal/tiger/cmd/integration_test.go index 967f4bcd..b6185ab4 100644 --- a/internal/tiger/cmd/integration_test.go +++ b/internal/tiger/cmd/integration_test.go @@ -7,7 +7,6 @@ import ( "fmt" "os" "os/exec" - "path/filepath" "regexp" "strings" "testing" @@ -2143,65 +2142,6 @@ func TestServiceForkIntegration(t *testing.T) { }) } -// buildTigerBinary compiles the CLI binary for PTY-based testing -func buildTigerBinary(t *testing.T, tmpDir string) string { - t.Helper() - - binaryPath := filepath.Join(tmpDir, "tiger") - - // Find project root by looking for go.mod - projectRoot, err := findProjectRoot() - if err != nil { - t.Fatalf("Failed to find project root: %v", err) - } - - cmd := exec.Command("go", "build", "-o", binaryPath, "./cmd/tiger") - cmd.Dir = projectRoot - output, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("Failed to build tiger binary: %v\nOutput: %s", err, output) - } - - return binaryPath -} - -// findProjectRoot finds the project root directory by looking for go.mod -func findProjectRoot() (string, error) { - // Start from current working directory and walk up - dir, err := os.Getwd() - if err != nil { - return "", err - } - - for { - if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { - return dir, nil - } - - parent := filepath.Dir(dir) - if parent == dir { - return "", fmt.Errorf("could not find go.mod in any parent directory") - } - dir = parent - } -} - -// buildTestEnv creates environment variables for PTY test binary -func buildTestEnv(tmpDir, publicKey, secretKey string) []string { - env := os.Environ() - // Override config directory to use test temp dir - env = append(env, "TIGER_CONFIG_DIR="+tmpDir) - // Disable analytics for tests - env = append(env, "TIGER_ANALYTICS=false") - // Use pgpass storage for easier testing (no keyring interaction) - env = append(env, "TIGER_PASSWORD_STORAGE=pgpass") - // Pass credentials via environment so subprocess uses them directly - // (bypasses keyring which may have different credentials) - env = append(env, "TIGER_PUBLIC_KEY="+publicKey) - env = append(env, "TIGER_SECRET_KEY="+secretKey) - return env -} - // TestDbConnectPasswordResetIntegration tests the interactive password reset flow // using go-expect for PTY simulation func TestDbConnectPasswordResetIntegration(t *testing.T) { @@ -2296,10 +2236,6 @@ func TestDbConnectPasswordResetIntegration(t *testing.T) { t.Logf("Created service with ID: %s", serviceID) }) - // Build tiger binary once for PTY testing (used by multiple subtests) - binaryPath := buildTigerBinary(t, tmpDir) - t.Logf("Built tiger binary at: %s", binaryPath) - t.Run("VerifyInitialConnection", func(t *testing.T) { if serviceID == "" { t.Skip("No service ID available") @@ -2318,16 +2254,20 @@ func TestDbConnectPasswordResetIntegration(t *testing.T) { defer c.Close() // Run tiger db connect - cmd := exec.Command(binaryPath, "db", "connect", serviceID) - cmd.Stdin = c.Tty() - cmd.Stdout = c.Tty() - cmd.Stderr = c.Tty() - cmd.Env = buildTestEnv(tmpDir, publicKey, secretKey) - - if err := cmd.Start(); err != nil { - t.Fatalf("Failed to start command: %v", err) + cmd, err := buildRootCmd(t.Context()) + if err != nil { + t.Fatalf("Failed to build root command: %v", err) } + cmd.SetArgs([]string{"db", "connect", serviceID}) + cmd.SetIn(c.Tty()) + cmd.SetOut(c.Tty()) + cmd.SetErr(c.Tty()) + // Run in goroutine since Execute() blocks + done := make(chan error, 1) + go func() { + done <- cmd.Execute() + }() // Should connect directly to psql (no menu since password is valid) // Wait for psql prompt t.Logf("Waiting for psql prompt...") @@ -2339,7 +2279,14 @@ func TestDbConnectPasswordResetIntegration(t *testing.T) { // Exit psql c.SendLine("\\q") - cmd.Wait() + select { + case err := <-done: + if err != nil { + t.Logf("Command finished with: %v", err) + } + case <-time.After(10 * time.Second): + t.Logf("Command still running after psql exit (may be expected)") + } t.Logf("Initial connection verified successfully") }) @@ -2405,21 +2352,25 @@ func TestDbConnectPasswordResetIntegration(t *testing.T) { defer c.Close() // Set up command with PTY - cmd := exec.Command(binaryPath, "db", "connect", serviceID) - cmd.Stdin = c.Tty() - cmd.Stdout = c.Tty() - cmd.Stderr = c.Tty() - cmd.Env = buildTestEnv(tmpDir, publicKey, secretKey) - - if err := cmd.Start(); err != nil { - t.Fatalf("Failed to start command: %v", err) + cmd, err := buildRootCmd(t.Context()) + if err != nil { + t.Fatalf("Failed to build root command: %v", err) } + cmd.SetArgs([]string{"db", "connect", serviceID}) + cmd.SetIn(c.Tty()) + cmd.SetOut(c.Tty()) + cmd.SetErr(c.Tty()) + + done := make(chan error, 1) + go func() { + done <- cmd.Execute() + }() // Wait for the menu prompt (auth failure message comes before it) t.Logf("Waiting for menu prompt...") - _, err = c.ExpectString("What would you like to do?") + _, err = c.ExpectString("Update/reset password") // Wait for actual option text if err != nil { - t.Fatalf("Expected menu prompt: %v", err) + t.Fatalf("Expected menu to render: %v", err) } t.Logf("Got menu prompt") @@ -2447,19 +2398,25 @@ func TestDbConnectPasswordResetIntegration(t *testing.T) { } t.Logf("Got success message") - // psql launches - wait for any prompt indicator then exit - t.Logf("Waiting for psql to launch...") - // Give psql time to connect and show prompt - time.Sleep(2 * time.Second) + // Wait for psql to launch and show prompt + t.Logf("Waiting for psql prompt...") + _, err = c.ExpectString("=>") + if err != nil { + t.Logf("Did not see psql prompt, continuing anyway: %v", err) + } // Exit psql with \q t.Logf("Sending \\q to exit psql") c.SendLine("\\q") // Wait for command to complete - err = cmd.Wait() - if err != nil { - t.Logf("Command exited with: %v (this may be expected)", err) + select { + case err := <-done: + if err != nil { + t.Logf("Command exited with: %v (may be expected)", err) + } + case <-time.After(10 * time.Second): + t.Logf("Timeout waiting for command to finish") } t.Logf("Interactive password reset completed") diff --git a/internal/tiger/cmd/password_recovery.go b/internal/tiger/cmd/password_recovery.go index 6f8d93a0..2edeec22 100644 --- a/internal/tiger/cmd/password_recovery.go +++ b/internal/tiger/cmd/password_recovery.go @@ -50,7 +50,7 @@ func connectWithPasswordMenu( fmt.Fprintf(cmd.ErrOrStderr(), "%s\nStored password is likely invalid or expired.\n\n", err.Error()) // Check if we're in a TTY for interactive menu - if !checkStdinIsTTY() { + if !checkOutputIsTTY(cmd.OutOrStdout()) { return fmt.Errorf("authentication failed and no TTY available for interactive password entry") } @@ -58,7 +58,7 @@ func connectWithPasswordMenu( // Only allow password reset for admin role canResetPassword := details.Role == "tsdbadmin" for { - option, err := selectPasswordRecoveryOption(cmd.ErrOrStderr(), canResetPassword) + option, err := selectPasswordRecoveryOption(cmd.InOrStdin(), cmd.ErrOrStderr(), canResetPassword) if err != nil { return err } @@ -67,7 +67,7 @@ func connectWithPasswordMenu( case optionEnterPassword: // Prompt for password fmt.Fprint(cmd.ErrOrStderr(), "Enter password: ") - password, err := readString(ctx, readPasswordFromTerminal) + password, err := readPassword(ctx, cmd.InOrStdin()) fmt.Fprintln(cmd.ErrOrStderr()) // newline after password entry if err != nil { if errors.Is(err, context.Canceled) { @@ -90,7 +90,7 @@ func connectWithPasswordMenu( case optionResetPassword: // Prompt and reset - password, err := promptAndResetPassword(ctx, cmd.ErrOrStderr(), client, service, details.Role) + password, err := promptAndResetPassword(ctx, cmd.InOrStdin(), cmd.ErrOrStderr(), client, service, details.Role) if err != nil { if errors.Is(err, context.Canceled) { return nil // user cancelled @@ -219,10 +219,10 @@ func (m passwordRecoveryModel) View() string { // selectPasswordRecoveryOption shows the interactive menu for password recovery // canResetPassword controls whether the "Update/reset password" option is shown -func selectPasswordRecoveryOption(out io.Writer, canResetPassword bool) (passwordRecoveryOption, error) { +func selectPasswordRecoveryOption(in io.Reader, out io.Writer, canResetPassword bool) (passwordRecoveryOption, error) { model := newPasswordRecoveryModel(canResetPassword) - program := tea.NewProgram(model, tea.WithOutput(out)) + program := tea.NewProgram(model, tea.WithInput(in), tea.WithOutput(out)) finalModel, err := program.Run() if err != nil { return optionExit, fmt.Errorf("failed to run password recovery menu: %w", err) @@ -314,13 +314,14 @@ func testSaveAndLaunchPsqlWithPassword( // Returns the new password on success. func promptAndResetPassword( ctx context.Context, + in io.Reader, out io.Writer, client api.ClientWithResponsesInterface, service api.Service, role string, ) (string, error) { fmt.Fprint(out, "Enter new password (leave empty to generate): ") - newPassword, err := readString(ctx, readPasswordFromTerminal) + newPassword, err := readPassword(ctx, in) fmt.Fprintln(out) // newline after password entry if err != nil { return "", fmt.Errorf("error reading password: %w", err) diff --git a/internal/tiger/cmd/root.go b/internal/tiger/cmd/root.go index c8512b3d..da64e1ee 100644 --- a/internal/tiger/cmd/root.go +++ b/internal/tiger/cmd/root.go @@ -1,22 +1,25 @@ package cmd import ( + "bufio" "context" "errors" "fmt" + "io" + "os" "strings" "time" "github.com/fatih/color" "github.com/spf13/cobra" "github.com/spf13/viper" - "go.uber.org/zap" - "github.com/timescale/tiger-cli/internal/tiger/analytics" "github.com/timescale/tiger-cli/internal/tiger/common" "github.com/timescale/tiger-cli/internal/tiger/config" "github.com/timescale/tiger-cli/internal/tiger/logging" "github.com/timescale/tiger-cli/internal/tiger/version" + "go.uber.org/zap" + "golang.org/x/term" ) func buildRootCmd(ctx context.Context) (*cobra.Command, error) { @@ -173,18 +176,19 @@ func Execute(ctx context.Context) error { return rootCmd.Execute() } -func readString(ctx context.Context, readFn func() (string, error)) (string, error) { +// readLine reads a line of text from the input, cancellable via context. +func readLine(ctx context.Context, in io.Reader) (string, error) { valCh := make(chan string) errCh := make(chan error) defer func() { close(valCh); close(errCh) }() go func() { - val, err := readFn() + val, err := bufio.NewReader(in).ReadString('\n') if err != nil { errCh <- err return } select { - case <-ctx.Done(): // don't return an empty value if the context is already canceled + case <-ctx.Done(): return default: } @@ -200,3 +204,38 @@ func readString(ctx context.Context, readFn func() (string, error)) (string, err return strings.TrimSpace(val), nil } } + +// readPassword reads a password with hidden input, cancellable via context. +// This is a package-level var so it can be overridden for testing. +var readPassword = func(ctx context.Context, in io.Reader) (string, error) { + f, ok := in.(*os.File) + if !ok { + return "", fmt.Errorf("password input requires a terminal file descriptor") + } + + valCh := make(chan string) + errCh := make(chan error) + defer func() { close(valCh); close(errCh) }() + go func() { + val, err := term.ReadPassword(int(f.Fd())) + if err != nil { + errCh <- err + return + } + select { + case <-ctx.Done(): + return + default: + } + valCh <- string(val) + }() + + select { + case <-ctx.Done(): + return "", ctx.Err() + case err := <-errCh: + return "", err + case val := <-valCh: + return strings.TrimSpace(val), nil + } +} diff --git a/internal/tiger/cmd/service.go b/internal/tiger/cmd/service.go index e775884a..45c5b054 100644 --- a/internal/tiger/cmd/service.go +++ b/internal/tiger/cmd/service.go @@ -1,12 +1,10 @@ package cmd import ( - "bufio" "context" "fmt" "io" "net/http" - "os" "regexp" "strings" "time" @@ -492,11 +490,12 @@ Examples: } } else if password == "" { // Interactive prompt - check if we're in a terminal - if !checkStdinIsTTY() { + if !checkOutputIsTTY(cmd.OutOrStdout()) { return fmt.Errorf("TTY not detected - use --new-password flag, --auto-generate flag, or TIGER_NEW_PASSWORD environment variable") } _, err := promptAndResetPassword( ctx, + cmd.InOrStdin(), statusOutput, cfg.Client, service, @@ -832,10 +831,7 @@ Examples: if !deleteConfirm { fmt.Fprintf(statusOutput, "Are you sure you want to delete service '%s'? This operation cannot be undone.\n", serviceID) fmt.Fprintf(statusOutput, "Type the service ID '%s' to confirm: ", serviceID) - confirmation, err := readString(cmd.Context(), func() (string, error) { - reader := bufio.NewReader(os.Stdin) - return reader.ReadString('\n') - }) + confirmation, err := readLine(cmd.Context(), cmd.InOrStdin()) if err != nil { return fmt.Errorf("failed to read confirmation: %w", err) }