sec(jobs): pass pg_dump password via PGPASSWORD env, not argv

internal/jobs/customer_backup_runner.go

@@ -50,6 +50,7 @@ import (
 	"io"
 	"log/slog"
 	"net/http"
+	"os"
 	"os/exec"
 	"strings"
 	"time"
@@ -105,11 +106,25 @@ type pgDumpRunner interface {
 type realPgDumpRunner struct{}
 
 func (realPgDumpRunner) Run(ctx context.Context, connURL string, w io.Writer) error {
+	// SEC-WORKER FINDING-2 (2026-05-29): split the customer's DB password
+	// out of the URL into PGPASSWORD env so it does NOT sit in argv (and
+	// therefore /proc/<pid>/cmdline + `ps aux` + kubectl describe crash
+	// archive) for the entire hourly backup window. Fail-open on parse
+	// error to avoid a single malformed connection_url stalling every
+	// customer's backup ladder.
+	dsn, pw, splitErr := splitPGPassword(connURL)
+	if splitErr != nil {
+		dsn = connURL
+		pw = ""
+	}
 	cmd := exec.CommandContext(ctx, "pg_dump",
 		"--no-owner", "--no-acl",
 		"--format=custom",
-		"-d", connURL,
+		"-d", dsn,
 	)
+	if pw != "" {
+		cmd.Env = append(os.Environ(), "PGPASSWORD="+pw)
+	}
 	cmd.Stdout = w
 	// Stderr goes to slog at the call site by buffering — we don't want a
 	// noisy pg_dump banner ("dumping contents of table ...") to flood

internal/jobs/customer_backup_runner_test.go

@@ -5,6 +5,9 @@ import (
 	"context"
 	"errors"
 	"io"
+	"os"
+	"path/filepath"
+	"runtime"
 	"sync"
 	"testing"
 	"time"
@@ -550,3 +553,150 @@ func TestRetentionCutoff_PositiveDaysIsBackInTime(t *testing.T) {
 		t.Errorf("pro 30d: cutoff = %v, want %v", got, want)
 	}
 }
+
+// installFakePgDump writes a shell-script "pg_dump" into a TempDir, prepends
+// it to PATH for the test's lifetime, and returns the script path so the
+// test can read back the recorded argv + env after invocation. The fake
+// prints argv to <dir>/argv.txt and env's PGPASSWORD value to
+// <dir>/pgpassword.txt, then exits 0 (success path) or 1 if the caller
+// passes failExitCode=true.
+//
+// Used by TestRealPgDumpRunner_* and TestDefaultPgDumpExec_*: those tests
+// exercise the SEC-WORKER FINDING-1 + FINDING-2 PGPASSWORD-env branches
+// in customer_backup_runner.go + platform_db_backup.go which require
+// actually spawning a pg_dump-named process.
+func installFakePgDump(t *testing.T, failExitCode bool) (dir string) {
+	t.Helper()
+	if runtime.GOOS == "windows" {
+		t.Skip("fake pg_dump script is shell-based; worker runs on linux/darwin only")
+	}
+	dir = t.TempDir()
+	exitCode := "0"
+	if failExitCode {
+		exitCode = "1"
+	}
+	// The script:
+	//   1. Writes every argv element (one per line) to argv.txt
+	//   2. Writes PGPASSWORD (or empty string) to pgpassword.txt
+	//   3. Writes a tiny stdout payload so callers that pipe stdout see bytes
+	//   4. Exits 0 (success) or 1 (caller-controlled failure)
+	script := "#!/bin/sh\n" +
+		"printf '%s\\n' \"$@\" > \"" + dir + "/argv.txt\"\n" +
+		"printf '%s' \"${PGPASSWORD:-}\" > \"" + dir + "/pgpassword.txt\"\n" +
+		"printf 'fakepgdumpbody'\n" +
+		"exit " + exitCode + "\n"
+	path := filepath.Join(dir, "pg_dump")
+	if err := os.WriteFile(path, []byte(script), 0o755); err != nil {
+		t.Fatalf("write fake pg_dump: %v", err)
+	}
+	oldPATH := os.Getenv("PATH")
+	t.Setenv("PATH", dir+string(os.PathListSeparator)+oldPATH)
+	return dir
+}
+
+func readFakePgDumpRecord(t *testing.T, dir string) (argv []string, pgpassword string) {
+	t.Helper()
+	argvBytes, err := os.ReadFile(filepath.Join(dir, "argv.txt"))
+	if err != nil {
+		t.Fatalf("read argv.txt: %v", err)
+	}
+	// Strip the trailing newline before splitting so the last entry isn't "".
+	argvStr := string(bytes.TrimRight(argvBytes, "\n"))
+	pgpassword = mustReadString(t, filepath.Join(dir, "pgpassword.txt"))
+	if argvStr == "" {
+		return nil, pgpassword
+	}
+	parts := bytes.Split([]byte(argvStr), []byte("\n"))
+	argv = make([]string, len(parts))
+	for i, b := range parts {
+		argv[i] = string(b)
+	}
+	return argv, pgpassword
+}
+
+func mustReadString(t *testing.T, path string) string {
+	t.Helper()
+	b, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("read %s: %v", path, err)
+	}
+	return string(b)
+}
+
+// TestRealPgDumpRunner_Run_PasswordMovesToEnv pins SEC-WORKER FINDING-2:
+// realPgDumpRunner must strip the password out of connURL and pass it via
+// PGPASSWORD env, NOT inside argv. This covers customer_backup_runner.go
+// lines 125-127 (the `if pw != ""` env-setting branch).
+func TestRealPgDumpRunner_Run_PasswordMovesToEnv(t *testing.T) {
+	dir := installFakePgDump(t, false)
+
+	const secret = "super-secret-pw-ZZZ"
+	connURL := "postgres://doadmin:" + secret + "@db.example.com:25060/app?sslmode=require"
+
+	var out bytes.Buffer
+	if err := (realPgDumpRunner{}).Run(context.Background(), connURL, &out); err != nil {
+		t.Fatalf("Run: %v", err)
+	}
+	if out.String() != "fakepgdumpbody" {
+		t.Errorf("stdout payload: got %q, want %q", out.String(), "fakepgdumpbody")
+	}
+
+	argv, pgpassword := readFakePgDumpRecord(t, dir)
+
+	// PGPASSWORD env must carry the secret.
+	if pgpassword != secret {
+		t.Errorf("PGPASSWORD env: got %q, want %q", pgpassword, secret)
+	}
+	// argv must NOT contain the literal password anywhere — this is THE
+	// security promise the PR is shipping.
+	for _, a := range argv {
+		if bytes.Contains([]byte(a), []byte(secret)) {
+			t.Errorf("argv leaks password: %q (full argv: %q)", a, argv)
+		}
+	}
+	// argv MUST still carry the stripped DSN (with userinfo password removed).
+	found := false
+	for _, a := range argv {
+		if a == "postgres://doadmin@db.example.com:25060/app?sslmode=require" {
+			found = true
+		}
+	}
+	if !found {
+		t.Errorf("argv missing stripped DSN; got: %q", argv)
+	}
+}
+
+// TestRealPgDumpRunner_Run_MalformedURLFailOpen pins the fail-open branch:
+// if splitPGPassword returns an error, the runner falls back to the original
+// connURL with no PGPASSWORD env. Covers customer_backup_runner.go lines
+// 116-119 (the `if splitErr != nil { dsn = connURL; pw = "" }` branch).
+func TestRealPgDumpRunner_Run_MalformedURLFailOpen(t *testing.T) {
+	dir := installFakePgDump(t, false)
+
+	// Same shape that splitPGPassword's TestSplitPGPassword malformed_url_fail_open
+	// case proves returns an error from url.Parse.
+	const malformed = "::::not a url"
+
+	var out bytes.Buffer
+	if err := (realPgDumpRunner{}).Run(context.Background(), malformed, &out); err != nil {
+		t.Fatalf("Run on malformed URL: %v", err)
+	}
+
+	argv, pgpassword := readFakePgDumpRecord(t, dir)
+
+	// Fail-open: no PGPASSWORD env is set because pw == "".
+	if pgpassword != "" {
+		t.Errorf("PGPASSWORD on fail-open: got %q, want empty", pgpassword)
+	}
+	// The original malformed URL is passed through to pg_dump argv unchanged
+	// (this is the "no regression" promise — same code path it has always run).
+	found := false
+	for _, a := range argv {
+		if a == malformed {
+			found = true
+		}
+	}
+	if !found {
+		t.Errorf("argv missing fail-open passthrough URL %q; got: %q", malformed, argv)
+	}
+}

internal/jobs/pgpw.go

@@ -0,0 +1,59 @@
+package jobs
+
+// pgpw.go — small helper used by every pg_dump call-site to pass the
+// Postgres password out-of-band (via PGPASSWORD env) instead of inside the
+// process-args connection URI.
+//
+// SEC-WORKER FINDING-1 + FINDING-2 (2026-05-29):
+//   - platform_db_backup.go ran `pg_dump <DSN-with-password>` for the
+//     daily 02:00 UTC platform-DB backup. The DSN with embedded
+//     doadmin password was visible in `ps aux` / /proc/<pid>/cmdline for
+//     the entire multi-minute backup window — any sidecar / debug shell /
+//     log-shipper / `kubectl describe` crash dump could read it.
+//   - customer_backup_runner.go ran `pg_dump -d <DSN-with-password>` for
+//     every per-customer hourly Pro/Team backup. Same surface, but the
+//     leaked secret is the customer's DB password (decrypted from
+//     resources.connection_url AES-GCM ciphertext).
+//
+// libpq honors PGPASSWORD via env. We strip the password from the URI
+// userinfo and set PGPASSWORD on the cmd.Env before exec — the password
+// no longer appears in cmdline.
+//
+// Conservative: if parsing fails, returns the original URL and "" — the
+// caller falls back to old behavior (no regression). Callers that want
+// hard-fail on parse can check the returned error.
+
+import (
+	"fmt"
+	"net/url"
+)
+
+// splitPGPassword returns the Postgres URL with the userinfo password
+// removed, plus the extracted password. If u has no password (e.g. SSL
+// cert auth) the returned password is "" and the URL is returned
+// unchanged. If u cannot be parsed as a URL, the input is returned
+// unchanged along with the parse error.
+//
+// Examples:
+//
+//	"postgres://u:p@h:5432/db?sslmode=require"
+//	  → ("postgres://u@h:5432/db?sslmode=require", "p", nil)
+//
+//	"postgres://u@h/db"        → ("postgres://u@h/db", "", nil)
+//	"postgres://h/db"          → ("postgres://h/db", "", nil)
+func splitPGPassword(rawURL string) (string, string, error) {
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		return rawURL, "", fmt.Errorf("parse pg url: %w", err)
+	}
+	if u.User == nil {
+		return rawURL, "", nil
+	}
+	pw, hasPW := u.User.Password()
+	if !hasPW {
+		return rawURL, "", nil
+	}
+	// Reconstruct userinfo with only the username.
+	u.User = url.User(u.User.Username())
+	return u.String(), pw, nil
+}

internal/jobs/pgpw_test.go

@@ -0,0 +1,92 @@
+package jobs
+
+import (
+	"strings"
+	"testing"
+)
+
+// TestSplitPGPassword pins the SEC-WORKER FINDING-1 + FINDING-2 fix:
+// pg_dump call sites move the Postgres password from process argv into
+// PGPASSWORD env. The helper must:
+//   1. Strip the password from a typical userinfo URL.
+//   2. Pass through unchanged when there is no password (cert auth,
+//      no user, malformed URL with fail-open).
+//   3. Never leak the literal password in the returned URL.
+func TestSplitPGPassword(t *testing.T) {
+	cases := []struct {
+		name    string
+		in      string
+		wantURL string
+		wantPW  string
+		wantErr bool
+	}{
+		{
+			name:    "userpass",
+			in:      "postgres://doadmin:abc123@host:25060/db?sslmode=require",
+			wantURL: "postgres://doadmin@host:25060/db?sslmode=require",
+			wantPW:  "abc123",
+		},
+		{
+			name:    "user_only_no_password",
+			in:      "postgres://doadmin@host:25060/db?sslmode=require",
+			wantURL: "postgres://doadmin@host:25060/db?sslmode=require",
+			wantPW:  "",
+		},
+		{
+			name:    "no_userinfo",
+			in:      "postgres://host:25060/db",
+			wantURL: "postgres://host:25060/db",
+			wantPW:  "",
+		},
+		{
+			name:    "empty",
+			in:      "",
+			wantURL: "",
+			wantPW:  "",
+		},
+		{
+			name:    "percent_encoded_password",
+			in:      "postgres://u:p%40ss%40word@h:5432/db",
+			wantURL: "postgres://u@h:5432/db",
+			wantPW:  "p@ss@word", // url.Userinfo.Password() decodes
+		},
+		{
+			name:    "malformed_url_fail_open",
+			in:      "::::not a url",
+			wantURL: "::::not a url",
+			wantPW:  "",
+			wantErr: true,
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			gotURL, gotPW, err := splitPGPassword(c.in)
+			if (err != nil) != c.wantErr {
+				t.Fatalf("err = %v, wantErr = %v", err, c.wantErr)
+			}
+			if gotURL != c.wantURL {
+				t.Errorf("URL\n  got:  %q\n  want: %q", gotURL, c.wantURL)
+			}
+			if gotPW != c.wantPW {
+				t.Errorf("password\n  got:  %q\n  want: %q", gotPW, c.wantPW)
+			}
+		})
+	}
+}
+
+// TestSplitPGPassword_NoLeak: the literal password substring must never
+// appear in the returned URL (this is THE point of the fix).
+func TestSplitPGPassword_NoLeak(t *testing.T) {
+	const secret = "ZZZ_NEVER_IN_URL_ZZZ"
+	in := "postgres://admin:" + secret + "@host:5432/db?sslmode=require"
+	gotURL, gotPW, err := splitPGPassword(in)
+	if err != nil {
+		t.Fatalf("splitPGPassword(%q) error: %v", in, err)
+	}
+	if strings.Contains(gotURL, secret) {
+		t.Errorf("returned URL %q still contains the password %q", gotURL, secret)
+	}
+	if gotPW != secret {
+		t.Errorf("password\n  got:  %q\n  want: %q", gotPW, secret)
+	}
+}

internal/jobs/platform_db_backup.go

@@ -623,18 +623,39 @@ func durationSeconds(d time.Duration) float64 {
 type defaultPgDumpExec struct{}
 
 // Dump runs pg_dump and streams its stdout to w.
+//
+// SEC-WORKER FINDING-1 (2026-05-29): the connection password is passed
+// to pg_dump out-of-band via PGPASSWORD env, NOT embedded in the URL on
+// argv. argv is world-readable via /proc/<pid>/cmdline for any sidecar /
+// debug shell / log-shipper / kube-exec process for the entire backup
+// window.
 func (defaultPgDumpExec) Dump(ctx context.Context, databaseURL string, w io.Writer) (int64, error) {
 	bin := os.Getenv("PG_DUMP_BIN")
 	if bin == "" {
 		bin = "pg_dump"
 	}
+	// Split password out of the URL → into PGPASSWORD env. If parse fails
+	// we fall back to the original URL (no regression): the same code path
+	// it has always run. The downside of fail-open is that a malformed
+	// URL would still leak; the alternative is hard-fail on every backup
+	// run because of one operator typo — caller chose the safer default.
+	dsn, pw, splitErr := splitPGPassword(databaseURL)
+	if splitErr != nil {
+		// non-fatal — fall back to original URL on argv
+		dsn = databaseURL
+		pw = ""
+	}
 	cmd := exec.CommandContext(ctx, bin,
 		"--no-owner",
 		"--no-acl",
 		"--format=custom",
 		"--compress=9",
-		databaseURL,
+		dsn,
 	)
+	if pw != "" {
+		// Inherit parent env so pg_dump still sees PATH, HOME, etc.
+		cmd.Env = append(os.Environ(), "PGPASSWORD="+pw)
+	}
 	// Capture stderr to a small buffer so a pg_dump failure produces a
 	// useful error message. stdout streams straight to w.
 	var stderr strings.Builder