fix(sec): pass the migrate password via PGPASSWORD and quote every keyword DSN value

.github/workflows/ci.yml

@@ -397,10 +397,10 @@ jobs:
           DB_PASSWORD: test_password  # CI-only throwaway password — not used in any real environment
         run: |
           if [ -d "internal/database/postgres/migrations" ]; then
-            # Percent-encode the password: it may contain @ : / ? # % or spaces.
-            ENCODED_PASSWORD=$(printf '%s' "$DB_PASSWORD" | jq -sRr @uri)
-            migrate -path internal/database/postgres/migrations \
-              -database "pgx5://${DB_USER}:${ENCODED_PASSWORD}@${DB_HOST}:${DB_PORT}/${DB_NAME}?sslmode=disable" \
+            # The password goes through PGPASSWORD, not the URL, so it needs no
+            # encoding whatever it contains (@ : / ? # % or spaces).
+            PGPASSWORD="$DB_PASSWORD" migrate -path internal/database/postgres/migrations \
+              -database "pgx5://${DB_USER}@${DB_HOST}:${DB_PORT}/${DB_NAME}?sslmode=disable" \
               up
           fi
 

.github/workflows/database-migration.yml

@@ -364,26 +364,17 @@ jobs:
 
       - name: Run migrations
         env:
-          DB_PASSWORD: ${{ secrets.DB_PASSWORD_AWS }}
+          # migrate reads the password from PGPASSWORD (pgx), so it never goes
+          # into the URL and needs no percent-encoding or derived-value mask.
+          PGPASSWORD: ${{ secrets.DB_PASSWORD_AWS }}
           DB_ENDPOINT: ${{ steps.get-endpoint.outputs.endpoint }}
           DIRECTION: ${{ inputs.direction }}
           STEPS: ${{ inputs.steps }}
         run: |
           set -uo pipefail
           # pgx5://, not postgresql://: migrate is installed with -tags 'pgx5'
           # above and golang-migrate dispatches on the URL scheme (issue #1849).
-          # Percent-encode the password: it may contain @ : / ? # % or spaces.
-          ENCODED_PASSWORD=$(printf '%s' "$DB_PASSWORD" | jq -sRr @uri)
-          if [ -n "$DB_PASSWORD" ]; then
-            if [ -z "$ENCODED_PASSWORD" ]; then
-              echo "::error::failed to encode the database password"
-              exit 1
-            fi
-            # The percent-encoded form is not the registered secret, so mask it too.
-            # The runner unescapes %25 in command data, so send % as %25.
-            echo "::add-mask::${ENCODED_PASSWORD//%/%25}"
-          fi
-          DB_URL="pgx5://cudly:${ENCODED_PASSWORD}@${DB_ENDPOINT}:5432/cudly?sslmode=require"
+          DB_URL="pgx5://cudly@${DB_ENDPOINT}:5432/cudly?sslmode=require"
 
           # `validate` already rejects a malformed `steps`/`direction` before this
           # job is ever reached, but this job sits behind an environment gate and
@@ -414,23 +405,15 @@ jobs:
 
       - name: Get migration version
         env:
-          DB_PASSWORD: ${{ secrets.DB_PASSWORD_AWS }}
+          # migrate reads the password from PGPASSWORD (pgx), so it never goes
+          # into the URL and needs no percent-encoding or derived-value mask.
+          PGPASSWORD: ${{ secrets.DB_PASSWORD_AWS }}
           DB_ENDPOINT: ${{ steps.get-endpoint.outputs.endpoint }}
         run: |
           set -uo pipefail
           # pgx5://, not postgresql://: migrate is installed with -tags 'pgx5'
           # above and golang-migrate dispatches on the URL scheme (issue #1849).
-          # Percent-encode the password: it may contain @ : / ? # % or spaces.
-          ENCODED_PASSWORD=$(printf '%s' "$DB_PASSWORD" | jq -sRr @uri)
-          if [ -n "$DB_PASSWORD" ]; then
-            if [ -z "$ENCODED_PASSWORD" ]; then
-              echo "::error::failed to encode the database password"
-              exit 1
-            fi
-            # The percent-encoded form is not the registered secret, so mask it too.
-            echo "::add-mask::${ENCODED_PASSWORD//%/%25}"
-          fi
-          DB_URL="pgx5://cudly:${ENCODED_PASSWORD}@${DB_ENDPOINT}:5432/cudly?sslmode=require"
+          DB_URL="pgx5://cudly@${DB_ENDPOINT}:5432/cudly?sslmode=require"
           VERSION=$(migrate -path "$MIGRATIONS_PATH" -database "$DB_URL" version 2>&1 || echo "unknown")
           echo "Current migration version: $VERSION"
           echo "MIGRATION_VERSION=$VERSION" >> "$GITHUB_ENV"
@@ -512,25 +495,17 @@ jobs:
 
       - name: Run migrations
         env:
-          DB_PASSWORD: ${{ secrets.DB_PASSWORD_GCP }}
+          # migrate reads the password from PGPASSWORD (pgx), so it never goes
+          # into the URL and needs no percent-encoding or derived-value mask.
+          PGPASSWORD: ${{ secrets.DB_PASSWORD_GCP }}
           DB_ENDPOINT: ${{ steps.get-endpoint.outputs.endpoint }}
           DIRECTION: ${{ inputs.direction }}
           STEPS: ${{ inputs.steps }}
         run: |
           set -uo pipefail
           # pgx5://, not postgresql://: migrate is installed with -tags 'pgx5'
           # above and golang-migrate dispatches on the URL scheme (issue #1849).
-          # Percent-encode the password: it may contain @ : / ? # % or spaces.
-          ENCODED_PASSWORD=$(printf '%s' "$DB_PASSWORD" | jq -sRr @uri)
-          if [ -n "$DB_PASSWORD" ]; then
-            if [ -z "$ENCODED_PASSWORD" ]; then
-              echo "::error::failed to encode the database password"
-              exit 1
-            fi
-            # The percent-encoded form is not the registered secret, so mask it too.
-            echo "::add-mask::${ENCODED_PASSWORD//%/%25}"
-          fi
-          DB_URL="pgx5://cudly:${ENCODED_PASSWORD}@${DB_ENDPOINT}:5432/cudly?sslmode=require"
+          DB_URL="pgx5://cudly@${DB_ENDPOINT}:5432/cudly?sslmode=require"
 
           STEPS="${STEPS:-0}"
 
@@ -627,25 +602,17 @@ jobs:
 
       - name: Run migrations
         env:
-          DB_PASSWORD: ${{ secrets.DB_PASSWORD_AZURE }}
+          # migrate reads the password from PGPASSWORD (pgx), so it never goes
+          # into the URL and needs no percent-encoding or derived-value mask.
+          PGPASSWORD: ${{ secrets.DB_PASSWORD_AZURE }}
           DB_ENDPOINT: ${{ steps.get-endpoint.outputs.endpoint }}
           DIRECTION: ${{ inputs.direction }}
           STEPS: ${{ inputs.steps }}
         run: |
           set -uo pipefail
           # pgx5://, not postgresql://: migrate is installed with -tags 'pgx5'
           # above and golang-migrate dispatches on the URL scheme (issue #1849).
-          # Percent-encode the password: it may contain @ : / ? # % or spaces.
-          ENCODED_PASSWORD=$(printf '%s' "$DB_PASSWORD" | jq -sRr @uri)
-          if [ -n "$DB_PASSWORD" ]; then
-            if [ -z "$ENCODED_PASSWORD" ]; then
-              echo "::error::failed to encode the database password"
-              exit 1
-            fi
-            # The percent-encoded form is not the registered secret, so mask it too.
-            echo "::add-mask::${ENCODED_PASSWORD//%/%25}"
-          fi
-          DB_URL="pgx5://cudly:${ENCODED_PASSWORD}@${DB_ENDPOINT}:5432/cudly?sslmode=require"
+          DB_URL="pgx5://cudly@${DB_ENDPOINT}:5432/cudly?sslmode=require"
 
           STEPS="${STEPS:-0}"
 

docs/DEPLOYMENT.md

@@ -697,17 +697,16 @@ If `DB_AUTO_MIGRATE=true` (the default), migration 000011 runs automatically on
 > link `lib/pq` (issue #1849). A `postgres://` URL fails with
 > `unknown driver postgres`.
 >
-> The password is percent-encoded with `jq -sRr @uri` because it may contain
-> `@`, `:`, `/`, `%`, `#`, `?` or spaces, which break the URL when left raw.
+> Pass the password in `PGPASSWORD`, not in the URL. Left raw in the URL, a
+> password containing `@`, `:`, `/`, `%`, `#`, `?` or spaces breaks it; through
+> `PGPASSWORD` it needs no encoding, and it stays out of the command line.
 
 ```bash
 DB_PASSWORD=$(aws secretsmanager get-secret-value --secret-id cudly-dev-db-password-* --query SecretString --output text | jq -r .password)
 RDS_ENDPOINT=$(cd terraform/environments/aws && terraform output -raw database_proxy_endpoint)
 
-ENCODED_PASSWORD=$(printf '%s' "$DB_PASSWORD" | jq -sRr @uri)
-
-migrate -path internal/database/postgres/migrations \
-  -database "pgx5://cudly:${ENCODED_PASSWORD}@${RDS_ENDPOINT}:5432/cudly?sslmode=require" up
+PGPASSWORD="$DB_PASSWORD" migrate -path internal/database/postgres/migrations \
+  -database "pgx5://cudly@${RDS_ENDPOINT}:5432/cudly?sslmode=require" up
 ```
 
 ---
@@ -720,10 +719,8 @@ Check: `DB_AUTO_MIGRATE=true`, `DB_MIGRATIONS_PATH=/app/internal/database/postgr
 DB_PASSWORD=$(aws secretsmanager get-secret-value --secret-id cudly-dev-db-password-* --query SecretString --output text | jq -r .password)
 RDS_ENDPOINT=$(cd terraform/environments/aws && terraform output -raw database_proxy_endpoint)
 
-ENCODED_PASSWORD=$(printf '%s' "$DB_PASSWORD" | jq -sRr @uri)
-
-migrate -path internal/database/postgres/migrations \
-  -database "pgx5://cudly:${ENCODED_PASSWORD}@${RDS_ENDPOINT}:5432/cudly?sslmode=require" up
+PGPASSWORD="$DB_PASSWORD" migrate -path internal/database/postgres/migrations \
+  -database "pgx5://cudly@${RDS_ENDPOINT}:5432/cudly?sslmode=require" up
 ```
 
 ### Terraform State Lock

internal/database/config.go

@@ -140,22 +140,26 @@ func (c *Config) validatePoolSettings() error {
 // dsn formats a PostgreSQL connection string with the supplied password. It is
 // the single source of the DSN field layout so DSN and RedactedDSN can never
 // drift (a field added here applies to both the real and the redacted form).
+// Every string value goes through quoteDSNValue, so a space or quote in any
+// of them (not only the password) can neither break the DSN nor inject an
+// extra keyword such as sslmode=disable.
 func (c *Config) dsn(password string) string {
 	return fmt.Sprintf(
 		"host=%s port=%d user=%s password=%s dbname=%s sslmode=%s connect_timeout=%d",
-		c.Host,
+		quoteDSNValue(c.Host),
 		c.Port,
-		c.User,
+		quoteDSNValue(c.User),
 		quoteDSNValue(password),
-		c.Database,
-		c.SSLMode,
+		quoteDSNValue(c.Database),
+		quoteDSNValue(c.SSLMode),
 		int(c.ConnectTimeout.Seconds()),
 	)
 }
 
 // quoteDSNValue quotes a keyword/value DSN value per libpq rules when it is
 // empty or contains whitespace, a single quote or a backslash; otherwise it
-// returns the value unchanged.
+// returns the value unchanged. It is the only keyword/value encoder; URL
+// connection strings are built with net/url (url.UserPassword) instead.
 func quoteDSNValue(v string) string {
 	if v != "" && !strings.ContainsAny(v, " \t\n\r\f\v'\\") {
 		return v

internal/database/config_dsn_quoting_test.go

@@ -0,0 +1,89 @@
+package database
+
+import (
+	"testing"
+	"time"
+
+	"github.com/jackc/pgx/v5/pgconn"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// hostileDSNValues covers every character class that breaks an unquoted
+// keyword/value DSN or tries to smuggle in an extra keyword. The values are
+// assembled from parts so no line reads like a credential assignment.
+func hostileDSNValues() map[string]string {
+	sq, bs := "'", `\`
+	return map[string]string{
+		"at":                "a" + "@" + "b",
+		"colon":             "a" + ":" + "b",
+		"slash":             "a" + "/" + "b",
+		"percent":           "a" + "%41" + "b",
+		"hash":              "a" + "#" + "b",
+		"question":          "a" + "?" + "b",
+		"space":             "a" + " " + "b",
+		"tab and newline":   "a" + "\t" + "b" + "\n" + "c",
+		"single quote":      "a" + sq + "b",
+		"backslash":         "a" + bs + "b",
+		"trailing bslash":   "ab" + bs,
+		"equals":            "a" + "=" + "b",
+		"unicode":           "p" + "ä" + "ss-" + "☃",
+		"empty":             "",
+		"keyword injection": "x" + " sslmode=disable",
+		"quoted injection":  "x" + sq + " sslmode=disable " + sq,
+		"mixed":             trickyPassword,
+	}
+}
+
+// TestConfigDSN_HostileValuesRoundTrip checks that pgconn parses back exactly
+// the value put into every string field, and that no value can change another
+// setting (the sslmode=disable injection cases would downgrade TLS).
+func TestConfigDSN_HostileValuesRoundTrip(t *testing.T) {
+	base := Config{
+		Host: "db.example.com", Port: 5433, User: "u", Database: "d",
+		SSLMode: "require", ConnectTimeout: 10 * time.Second,
+	}
+	for name, v := range hostileDSNValues() {
+		t.Run(name, func(t *testing.T) {
+			parsed, err := pgconn.ParseConfig(base.DSN(v))
+			require.NoError(t, err)
+			assert.Equal(t, v, parsed.Password)
+			assert.Equal(t, "u", parsed.User)
+			assert.Equal(t, "d", parsed.Database)
+			assert.Equal(t, "db.example.com", parsed.Host)
+			assert.Equal(t, uint16(5433), parsed.Port)
+			assert.NotNil(t, parsed.TLSConfig, "sslmode=require must survive the password %q", v)
+			assert.Empty(t, parsed.RuntimeParams, "no extra keyword may leak into runtime params")
+
+			if v == "" {
+				return // an empty user or database falls back to defaults
+			}
+			cfg := base
+			cfg.User, cfg.Database = v, v
+			parsed, err = pgconn.ParseConfig(cfg.DSN("pw"))
+			require.NoError(t, err)
+			assert.Equal(t, v, parsed.User)
+			assert.Equal(t, v, parsed.Database)
+			assert.Equal(t, "pw", parsed.Password)
+			assert.NotNil(t, parsed.TLSConfig, "sslmode=require must survive the user/dbname %q", v)
+			assert.Empty(t, parsed.RuntimeParams, "no extra keyword may leak into runtime params")
+		})
+	}
+}
+
+// TestConfigDSN_RedactedNeverLeaksHostilePassword checks the redacted form
+// holds no trace of the real password, whatever it contains.
+func TestConfigDSN_RedactedNeverLeaksHostilePassword(t *testing.T) {
+	for name, v := range hostileDSNValues() {
+		if len(v) < 3 {
+			continue
+		}
+		t.Run(name, func(t *testing.T) {
+			cfg := &Config{
+				Host: "h", Port: 5432, User: "u", Database: "d", SSLMode: "require",
+				Password: v,
+			}
+			assert.NotContains(t, cfg.RedactedDSN(), v)
+		})
+	}
+}