From 1e4b3af8073b106f67e0f9300ff1efae9de0a413 Mon Sep 17 00:00:00 2001 From: Akash Saini Date: Fri, 23 Oct 2026 01:10:39 +0100 Subject: [PATCH] fix(import/d1): keep the destination password off psql's argv runPsqlFile passed the full connection string, password included, as psql's first argument, exposing the 2-hour import role credential to any local user via ps. Strip the password from the keyword/value or URI connection string and pass it to psql via PGPASSWORD instead. Co-Authored-By: Claude Opus 5.5 --- internal/import/d1/import.go | 102 +++++++++++++++++++++++++++++- internal/import/d1/import_test.go | 67 ++++++++++++++++++++ 2 files changed, 168 insertions(+), 1 deletion(-) diff --git a/internal/import/d1/import.go b/internal/import/d1/import.go index 973b7c03..ed19407a 100644 --- a/internal/import/d1/import.go +++ b/internal/import/d1/import.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net/url" "os" "path/filepath" "slices" @@ -673,9 +674,19 @@ func runPsqlFile(ctx context.Context, destURI, path string) error { return err } + // Command-line arguments are visible to every local user via ps, so the + // password travels in the child's environment instead. + conninfo, password, err := splitConnPassword(destURI) + if err != nil { + return err + } + return withConnectionRetry(ctx, func() error { // --single-transaction: roll back the whole file if any statement fails. - cmd := execabs.CommandContext(ctx, psqlPath, destURI, "-v", "ON_ERROR_STOP=1", "--single-transaction", "-f", path) + cmd := execabs.CommandContext(ctx, psqlPath, conninfo, "-v", "ON_ERROR_STOP=1", "--single-transaction", "-f", path) + if password != "" { + cmd.Env = append(os.Environ(), "PGPASSWORD="+password) + } out, err := cmd.CombinedOutput() if err != nil { return fmt.Errorf("psql %s: %w: %s", filepath.Base(path), err, string(out)) @@ -684,6 +695,95 @@ func runPsqlFile(ctx context.Context, destURI, path string) error { }) } +// splitConnPassword removes the password from a libpq connection string in +// either URI or keyword/value form and returns it separately. +func splitConnPassword(conninfo string) (string, string, error) { + if strings.HasPrefix(conninfo, "postgres://") || strings.HasPrefix(conninfo, "postgresql://") { + u, err := url.Parse(conninfo) + if err != nil { + return "", "", fmt.Errorf("parse destination URI: %w", err) + } + var password string + if u.User != nil { + password, _ = u.User.Password() + u.User = url.User(u.User.Username()) + } + q := u.Query() + if p := q.Get("password"); p != "" { + password = p + q.Del("password") + u.RawQuery = q.Encode() + } + return u.String(), password, nil + } + + var kept []string + var password string + i := 0 + for { + for i < len(conninfo) && isConnSpace(conninfo[i]) { + i++ + } + if i >= len(conninfo) { + break + } + start := i + for i < len(conninfo) && conninfo[i] != '=' && !isConnSpace(conninfo[i]) { + i++ + } + key := conninfo[start:i] + for i < len(conninfo) && isConnSpace(conninfo[i]) { + i++ + } + if i >= len(conninfo) || conninfo[i] != '=' { + return "", "", fmt.Errorf("malformed connection string: missing \"=\" after %q", key) + } + i++ + for i < len(conninfo) && isConnSpace(conninfo[i]) { + i++ + } + + var value strings.Builder + quoted := i < len(conninfo) && conninfo[i] == '\'' + if quoted { + i++ + } + closed := !quoted + for i < len(conninfo) { + c := conninfo[i] + if c == '\\' && i+1 < len(conninfo) { + value.WriteByte(conninfo[i+1]) + i += 2 + continue + } + if quoted && c == '\'' { + closed = true + i++ + break + } + if !quoted && isConnSpace(c) { + break + } + value.WriteByte(c) + i++ + } + if !closed { + return "", "", errors.New("malformed connection string: unterminated quoted value") + } + + if key == "password" { + password = value.String() + continue + } + kept = append(kept, strings.TrimSpace(conninfo[start:i])) + } + return strings.Join(kept, " "), password, nil +} + +func isConnSpace(c byte) bool { + return c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '\f' || c == '\v' +} + // Status returns migration state for status polling. func Status(org, database, branch, migrationID string) (*MigrationState, error) { return LoadState(org, database, branch, migrationID) diff --git a/internal/import/d1/import_test.go b/internal/import/d1/import_test.go index 957496d2..c579a274 100644 --- a/internal/import/d1/import_test.go +++ b/internal/import/d1/import_test.go @@ -3,10 +3,13 @@ package d1 import ( "context" "errors" + "strings" "testing" "time" + "github.com/jackc/pgx/v5/pgconn" ps "github.com/planetscale/cli/internal/planetscale" + "github.com/planetscale/cli/internal/postgres" ) type failingImportClient struct{} @@ -289,3 +292,67 @@ func TestWithConnectionRetryRespectsContext(t *testing.T) { t.Fatal("expected context or retry error") } } + +func TestSplitConnPassword(t *testing.T) { + cases := []struct { + name string + conninfo string + password string + }{ + { + name: "resolved role connection string", + conninfo: postgres.BuildConnectionString(&postgres.Config{ + Host: "aws.connect.psdb.cloud", + Port: 5432, + User: "d1_import_abc", + Password: `pscale_pw_it's a secret`, + Database: "postgres", + SSLMode: "verify-full", + Options: map[string]string{"sslrootcert": "system"}, + }), + password: `pscale_pw_it's a secret`, + }, + {name: "unquoted keyword password", conninfo: "host=localhost password=hunter2 dbname=app sslmode=disable", password: "hunter2"}, + {name: "spaces around equals", conninfo: "host = localhost password = 'a b' user=me sslmode=disable", password: "a b"}, + {name: "no password", conninfo: "host=localhost dbname=app sslmode=disable"}, + {name: "uri userinfo", conninfo: "postgresql://me:p%40ss@localhost:5432/app?sslmode=disable", password: "p@ss"}, + {name: "uri query password", conninfo: "postgres://me@localhost/app?password=qp&sslmode=disable", password: "qp"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + stripped, password, err := splitConnPassword(tc.conninfo) + if err != nil { + t.Fatalf("splitConnPassword: %v", err) + } + if password != tc.password { + t.Fatalf("password = %q, want %q", password, tc.password) + } + if tc.password != "" && strings.Contains(stripped, tc.password) { + t.Fatalf("stripped connection string still contains the password: %q", stripped) + } + + want, err := pgconn.ParseConfig(tc.conninfo) + if err != nil { + t.Fatalf("parse original: %v", err) + } + t.Setenv("PGPASSWORD", password) + got, err := pgconn.ParseConfig(stripped) + if err != nil { + t.Fatalf("parse stripped %q: %v", stripped, err) + } + if got.Host != want.Host || got.Port != want.Port || got.User != want.User || + got.Database != want.Database || got.Password != want.Password { + t.Fatalf("stripped %q parses to %s@%s:%d/%s, want %s@%s:%d/%s", + stripped, got.User, got.Host, got.Port, got.Database, want.User, want.Host, want.Port, want.Database) + } + }) + } +} + +func TestSplitConnPasswordMalformed(t *testing.T) { + for _, conninfo := range []string{"host=localhost password='unterminated", "host localhost"} { + if _, _, err := splitConnPassword(conninfo); err == nil { + t.Fatalf("splitConnPassword(%q): expected error", conninfo) + } + } +}