From d2daabb739e9b80d6c98e22a68581d29f594b764 Mon Sep 17 00:00:00 2001 From: mayor Date: Wed, 1 Apr 2026 23:56:58 +0000 Subject: [PATCH] fix: force VerifyServerDataDir fallback over TCP --- internal/doltserver/doltserver.go | 75 ++++++++++++++++- internal/doltserver/doltserver_test.go | 110 ++++++++++++++++++++++++- 2 files changed, 180 insertions(+), 5 deletions(-) diff --git a/internal/doltserver/doltserver.go b/internal/doltserver/doltserver.go index b5dcb1fdd6..7550649ed5 100644 --- a/internal/doltserver/doltserver.go +++ b/internal/doltserver/doltserver.go @@ -425,8 +425,46 @@ func buildDoltSQLCmd(ctx context.Context, config *Config, args ...string) *exec. cmd.Dir = config.DataDir setProcessGroup(cmd) - if config.IsRemote() && config.Password != "" { + if config.Password != "" { cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD="+config.Password) + } else if config.IsRemote() { + if inherited, ok := os.LookupEnv("DOLT_CLI_PASSWORD"); ok { + cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD="+inherited) + } else { + cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD=") + } + } else { + cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD=") + } + + return cmd +} + +// buildDoltSQLTCPClientCmd constructs a dolt sql command that always connects +// to the running server over TCP, even when the configured host is local. This +// is used by verification paths that must avoid embedded-mode fallback. +func buildDoltSQLTCPClientCmd(ctx context.Context, config *Config, args ...string) *exec.Cmd { + fullArgs := []string{ + "--host", config.EffectiveHost(), + "--port", strconv.Itoa(config.Port), + "--user", config.User, + "--no-tls", + "sql", + } + fullArgs = append(fullArgs, args...) + + cmd := exec.CommandContext(ctx, "dolt", fullArgs...) + cmd.Dir = config.DataDir + if config.Password != "" { + cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD="+config.Password) + } else if config.IsRemote() { + if inherited, ok := os.LookupEnv("DOLT_CLI_PASSWORD"); ok { + cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD="+inherited) + } else { + cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD=") + } + } else { + cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD=") } return cmd @@ -1018,7 +1056,7 @@ func VerifyServerDataDir(townRoot string) (bool, error) { return true, nil } - served, _, verifyErr := VerifyDatabases(townRoot) + served, verifyErr := queryServedDatabasesTCPClient(townRoot) if verifyErr != nil { return false, fmt.Errorf("could not query server databases: %w", verifyErr) } @@ -1041,6 +1079,39 @@ func VerifyServerDataDir(townRoot string) (bool, error) { return true, nil } +func queryServedDatabasesTCPClient(townRoot string) ([]string, error) { + config := DefaultConfig(townRoot) + if err := CheckServerReachable(townRoot); err != nil { + return nil, fmt.Errorf("server not reachable: %w", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + cmd := buildDoltSQLTCPClientCmd(ctx, config, + "-r", "json", + "-q", "SHOW DATABASES", + ) + + var stderrBuf bytes.Buffer + cmd.Stderr = &stderrBuf + output, queryErr := cmd.Output() + cancel() + if queryErr != nil { + stderrMsg := strings.TrimSpace(stderrBuf.String()) + errDetail := strings.TrimSpace(string(output)) + if stderrMsg != "" { + errDetail = errDetail + " (stderr: " + stderrMsg + ")" + } + return nil, fmt.Errorf("querying SHOW DATABASES: %w (output: %s)", queryErr, errDetail) + } + + served, parseErr := parseShowDatabases(output) + if parseErr != nil { + return nil, fmt.Errorf("parsing SHOW DATABASES output: %w", parseErr) + } + + return served, nil +} + // KillImposters finds and kills any dolt sql-server process on the configured // port that is NOT serving from the expected data directory. This handles the // case where another tool (e.g., bd) launched its own embedded Dolt server diff --git a/internal/doltserver/doltserver_test.go b/internal/doltserver/doltserver_test.go index a1dcdc8fbc..62ae29333e 100644 --- a/internal/doltserver/doltserver_test.go +++ b/internal/doltserver/doltserver_test.go @@ -3606,12 +3606,116 @@ func TestBuildDoltSQLCmd_RemoteNoPassword(t *testing.T) { ctx := t.Context() cmd := buildDoltSQLCmd(ctx, config, "-q", "SELECT 1") - // Should NOT have DOLT_CLI_PASSWORD in env + // Should set empty DOLT_CLI_PASSWORD to suppress interactive prompts. for _, env := range cmd.Env { - if strings.HasPrefix(env, "DOLT_CLI_PASSWORD=") { - t.Error("remote cmd without password should not have DOLT_CLI_PASSWORD env var") + if env == "DOLT_CLI_PASSWORD=" { + return } } + t.Error("remote cmd without password should set empty DOLT_CLI_PASSWORD env var") +} + +func TestBuildDoltSQLTCPClientCmd_Local(t *testing.T) { + config := &Config{ + Host: "", + Port: 3307, + User: "root", + DataDir: "/tmp/dolt-data", + } + + cmd := buildDoltSQLTCPClientCmd(t.Context(), config, "-q", "SELECT 1") + + if cmd.Dir != config.DataDir { + t.Errorf("cmd.Dir = %q, want %q", cmd.Dir, config.DataDir) + } + + wantArgs := []string{"dolt", "--host", "127.0.0.1", "--port", "3307", "--user", "root", "--no-tls", "sql", "-q", "SELECT 1"} + if len(cmd.Args) != len(wantArgs) { + t.Fatalf("len(cmd.Args) = %d, want %d; args=%v", len(cmd.Args), len(wantArgs), cmd.Args) + } + for i, want := range wantArgs { + if cmd.Args[i] != want { + t.Errorf("cmd.Args[%d] = %q, want %q", i, cmd.Args[i], want) + } + } + + found := false + for _, env := range cmd.Env { + if env == "DOLT_CLI_PASSWORD=" { + found = true + break + } + } + if !found { + t.Error("local explicit TCP cmd should set empty DOLT_CLI_PASSWORD env var") + } +} + +func TestBuildDoltSQLTCPClientCmd_Remote(t *testing.T) { + config := &Config{ + Host: "10.0.0.5", + Port: 3307, + User: "root", + Password: "secret", + DataDir: "/tmp/dolt-data", + } + + cmd := buildDoltSQLTCPClientCmd(t.Context(), config, "-q", "SELECT 1") + + if cmd.Dir != config.DataDir { + t.Errorf("cmd.Dir = %q, want %q", cmd.Dir, config.DataDir) + } + + wantArgs := []string{"dolt", "--host", "10.0.0.5", "--port", "3307", "--user", "root", "--no-tls", "sql", "-q", "SELECT 1"} + if len(cmd.Args) != len(wantArgs) { + t.Fatalf("len(cmd.Args) = %d, want %d; args=%v", len(cmd.Args), len(wantArgs), cmd.Args) + } + for i, want := range wantArgs { + if cmd.Args[i] != want { + t.Errorf("cmd.Args[%d] = %q, want %q", i, cmd.Args[i], want) + } + } + + found := false + for _, env := range cmd.Env { + if env == "DOLT_CLI_PASSWORD=secret" { + found = true + break + } + } + if !found { + t.Error("remote explicit TCP cmd should set DOLT_CLI_PASSWORD env var") + } +} + +func TestBuildDoltSQLTCPClientCmd_RemoteNoPasswordPreservesInheritedCredentials(t *testing.T) { + t.Setenv("DOLT_CLI_PASSWORD", "secret-from-env") + + config := &Config{ + Host: "10.0.0.5", + Port: 3307, + User: "root", + DataDir: "/tmp/dolt-data", + } + + cmd := buildDoltSQLTCPClientCmd(t.Context(), config, "-q", "SELECT 1") + + foundInherited := false + foundEmpty := false + for _, env := range cmd.Env { + if env == "DOLT_CLI_PASSWORD=secret-from-env" { + foundInherited = true + } + if env == "DOLT_CLI_PASSWORD=" { + foundEmpty = true + } + } + if !foundInherited { + t.Fatal("expected remote explicit TCP helper to preserve inherited DOLT_CLI_PASSWORD") + } + if foundEmpty { + t.Fatal("did not expect remote explicit TCP helper to overwrite inherited credentials with empty password") + } } // =============================================================================