[SCAN-1020] neon scram pgx (#5217)

* [SCAN-1020] Add Neon host detection for Postgres verification Identify *.neon.tech endpoints so verification can use a driver that accepts Neon's SCRAM iteration count (i=1). Co-authored-by: Cursor <[email protected]> * [SCAN-1020] Verify Neon Postgres hosts with pgx lib/pq rejects Neon's SCRAM i=1 handshake. Route *.neon.tech verification through pgx, which accepts any positive iteration count, and keep lib/pq for all other hosts. Co-authored-by: Cursor <[email protected]> * [SCAN-1020] Check pgx Close error for errcheck Co-authored-by: Cursor <[email protected]> * make bugbot happy --------- Co-authored-by: Cursor <[email protected]>
2026-08-19 14:45:40 -04:00
parent 925c1d0a61
commit 9d3f5d13af
3 changed files with 178 additions and 3 deletions
@@ -58,6 +58,7 @@ require (
 	github.com/googleapis/gax-go/v2 v2.16.0
 	github.com/hashicorp/go-retryablehttp v0.7.8
 	github.com/hashicorp/golang-lru/v2 v2.0.7
+	github.com/jackc/pgx/v5 v5.9.2
 	github.com/jedib0t/go-pretty/v6 v6.6.8
 	github.com/jlaffaye/ftp v0.2.0
 	github.com/joho/godotenv v1.5.1
@@ -225,6 +226,8 @@ require (
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
 	github.com/hashicorp/go-multierror v1.1.1 // indirect
+	github.com/jackc/pgpassfile v1.0.0 // indirect
+	github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect
 	github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect
 	github.com/jpillora/s3 v1.1.4 // indirect
 	github.com/kevinburke/ssh_config v1.2.0 // indirect
@@ -10,6 +10,8 @@ import (
 	"strings"
 	"time"

+	"github.com/jackc/pgx/v5"
+	"github.com/jackc/pgx/v5/pgconn"
 	"github.com/lib/pq"
 	regexp "github.com/wasilibs/go-re2"

@@ -173,7 +175,7 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete
 				break
 			}

-			isVerified, verificationErr := verifyPostgres(params)
+			isVerified, verificationErr := verifyPostgres(ctx, params)
 			result.Verified = isVerified
 			result.SetVerificationError(verificationErr, password)
 		}
@@ -252,6 +254,13 @@ func shouldIgnore(uri []byte, ignorePatterns []*regexp.Regexp) bool {
 	return false
 }

+// isNeonHost reports whether host is a Neon managed-Postgres endpoint.
+// Neon advertises SCRAM-SHA-256 with iteration count i=1; lib/pq rejects that,
+// so verification for these hosts uses pgx instead (SCAN-1020).
+func isNeonHost(host string) bool {
+	return strings.HasSuffix(strings.ToLower(host), ".neon.tech")
+}
+
 // getDeadlineInSeconds gets the deadline from the context in seconds. If there
 // is no deadline, false is returned. If the deadline is already exceeded, a
 // negative or 0 value will be returned.
@@ -275,7 +284,70 @@ func isErrorDatabaseNotFound(err error, dbName string) bool {
 	return strings.Contains(err.Error(), missingDbErrorText)
 }

-func verifyPostgres(params map[string]string) (bool, error) {
+func verifyPostgres(ctx context.Context, params map[string]string) (bool, error) {
+	// Neon (managed Postgres) advertises SCRAM-SHA-256 with iteration count i=1.
+	// lib/pq rejects iteration fields shorter than 6 chars, which traps these
+	// secrets in indeterminate reverification. pgx accepts any iterations > 0.
+	if isNeonHost(params[pgHost]) {
+		return verifyPostgresPgx(ctx, params)
+	}
+	return verifyPostgresPq(params)
+}
+
+// verifyPostgresPgx verifies credentials with jackc/pgx. Used for Neon hosts
+// where lib/pq's SCRAM client cannot complete the handshake (SCAN-1020).
+func verifyPostgresPgx(ctx context.Context, params map[string]string) (bool, error) {
+	conn, err := pgx.Connect(ctx, pgxConnString(params))
+	if err != nil {
+		return classifyPostgresVerifyError(err, params[pgDbname])
+	}
+	defer func() {
+		// Best-effort close after verification; the verify outcome is already decided.
+		if closeErr := conn.Close(ctx); closeErr != nil {
+			return
+		}
+	}()
+
+	if err := conn.Ping(ctx); err != nil {
+		return classifyPostgresVerifyError(err, params[pgDbname])
+	}
+	return true, nil
+}
+
+// pgxConnString builds a libpq-style connection string for pgx, omitting keys
+// that are detector-only (db_type) or libpq client options pgx would forward as
+// unrecognized server GUCs (requiressl). sslmode is already normalized in FromData.
+func pgxConnString(params map[string]string) string {
+	var connStr strings.Builder
+	for key, value := range params {
+		if key == pgDbType || key == pgRequiressl {
+			continue
+		}
+		fmt.Fprintf(&connStr, "%s='%s'", key, value)
+	}
+	return connStr.String()
+}
+
+func classifyPostgresVerifyError(err error, dbName string) (bool, error) {
+	var pgErr *pgconn.PgError
+	if errors.As(err, &pgErr) {
+		switch pgErr.Code {
+		case "28P01": // invalid_password
+			return false, nil
+		case "3D000": // invalid_catalog_name — authenticated, DB missing
+			return true, nil
+		}
+	}
+	if strings.Contains(err.Error(), "password authentication failed") {
+		return false, nil
+	}
+	if isErrorDatabaseNotFound(err, dbName) {
+		return true, nil
+	}
+	return false, err
+}
+
+func verifyPostgresPq(params map[string]string) (bool, error) {
 	if sslmode := params[pgSslmode]; sslmode == pgSslmodeAllow || sslmode == pgSslmodePrefer {
 		// pq doesn't support 'allow' or 'prefer'. If we find either of them, we'll just ignore it. This will trigger
 		// the same logic that is run if no sslmode is set at all (which mimics 'prefer', which is the default).
@@ -319,7 +391,7 @@ func verifyPostgres(params map[string]string) (bool, error) {
 		// connections are acceptable, so now we try a connection without SSL.
 		params[pgSslmode] = pgSslmodeDisable
 		defer delete(params, pgSslmode) // We want to return with the original params map intact (for ExtraData)
-		return verifyPostgres(params)
+		return verifyPostgresPq(params)
 	case isErrorDatabaseNotFound(err, params[pgDbname]):
 		return true, nil // If we know this, we were able to authenticate
 	default:
@@ -2,10 +2,12 @@ package postgres

 import (
 	"context"
+	"errors"
 	"fmt"
 	"testing"

 	"github.com/google/go-cmp/cmp"
+	"github.com/jackc/pgx/v5/pgconn"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"

@@ -130,6 +132,104 @@ func TestPostgres_ExtraData(t *testing.T) {
 	}
 }

+func TestIsNeonHost(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		host string
+		want bool
+	}{
+		{host: "ep-falling-feather-aimmxil4.c-4.us-east-1.aws.neon.tech", want: true},
+		{host: "EP.NEON.TECH", want: true},
+		{host: "db.example.com", want: false},
+		{host: "neon.tech.evil.com", want: false},
+		{host: "neon.tech", want: false},
+		{host: "", want: false},
+	} {
+		t.Run(tc.host, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, tc.want, isNeonHost(tc.host))
+		})
+	}
+}
+
+func TestPgxConnStringOmitsClientOnlyParams(t *testing.T) {
+	t.Parallel()
+
+	got := pgxConnString(map[string]string{
+		pgHost:       "ep-example.us-east-1.aws.neon.tech",
+		pgPort:       "5432",
+		pgUser:       "user",
+		pgPassword:   "secret",
+		pgDbname:     "neondb",
+		pgSslmode:    pgSslmodeRequire,
+		pgDbType:     "postgres",
+		pgRequiressl: "1",
+	})
+
+	assert.Contains(t, got, "host='ep-example.us-east-1.aws.neon.tech'")
+	assert.Contains(t, got, "sslmode='require'")
+	assert.NotContains(t, got, "db_type=")
+	assert.NotContains(t, got, "requiressl=")
+}
+
+func TestClassifyPostgresVerifyError(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name         string
+		err          error
+		dbName       string
+		wantVerified bool
+		wantErr      bool
+	}{
+		{
+			name:         "invalid password code",
+			err:          &pgconn.PgError{Code: "28P01", Message: "password authentication failed"},
+			wantVerified: false,
+			wantErr:      false,
+		},
+		{
+			name:         "missing database code",
+			err:          &pgconn.PgError{Code: "3D000", Message: `database "app" does not exist`},
+			dbName:       "app",
+			wantVerified: true,
+			wantErr:      false,
+		},
+		{
+			name:         "password failure by message",
+			err:          errors.New("password authentication failed for user \"x\""),
+			wantVerified: false,
+			wantErr:      false,
+		},
+		{
+			name:         "missing database by message",
+			err:          errors.New(`database "postgres" does not exist`),
+			wantVerified: true,
+			wantErr:      false,
+		},
+		{
+			name:         "indeterminate error",
+			err:          errors.New("connection refused"),
+			wantVerified: false,
+			wantErr:      true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			verified, err := classifyPostgresVerifyError(tc.err, tc.dbName)
+			assert.Equal(t, tc.wantVerified, verified)
+			if tc.wantErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
 func TestPostgres_FromDataWithIgnorePattern(t *testing.T) {
 	s := New(
 		WithIgnorePattern([]string{