fix(config): never generate a secret the staging/prod validators reject

internal/config/defaults_helpers.go

@@ -133,19 +133,37 @@ func UseRandSource(r io.Reader) (restore func()) {
 // from the scoped source (crypto/rand by default, see UseRandSource). Used for
 // auto-generating JWT keys and other secrets in dev mode. The source is never
 // logged or included in an error.
+//
+// A generated secret must pass the validators it will later face: a random
+// string occasionally spells one of insecurePatterns ("test", "admin", ...) or
+// a placeholder, and in staging/prod the validator then rejects a value the
+// CLI itself just made (and build has already persisted it to .env.secrets, so
+// the project stays broken). Such a draw is discarded and the next bytes of the
+// source are used, so a seeded source (plan and apply) still yields one value.
 func generateSecureRandom(length int) (string, error) {
 	src := io.Reader(rand.Reader)
 	if p := randSource.Load(); p != nil {
 		src = *p
 	}
-	// Allocate enough bytes to guarantee the encoded output is >= length.
-	b := make([]byte, length)
-	if _, err := io.ReadFull(src, b); err != nil {
-		return "", fmt.Errorf("generating secret: %w", err)
-	}
-	encoded := base64.RawURLEncoding.EncodeToString(b)
-	if len(encoded) < length {
-		return encoded, nil
+	for attempt := 0; attempt < secretAttempts; attempt++ {
+		// Allocate enough bytes to guarantee the encoded output is >= length.
+		b := make([]byte, length)
+		if _, err := io.ReadFull(src, b); err != nil {
+			return "", fmt.Errorf("generating secret: %w", err)
+		}
+		encoded := base64.RawURLEncoding.EncodeToString(b)
+		if len(encoded) >= length {
+			encoded = encoded[:length]
+		}
+		if !isInsecurePassword(encoded) && !isPlaceholderSecret(encoded) {
+			return encoded, nil
+		}
 	}
-	return encoded[:length], nil
+	return "", fmt.Errorf("generating secret: %d draws in a row matched a blocked pattern", secretAttempts)
 }
+
+// secretAttempts bounds the resampling in generateSecureRandom. A fresh draw is
+// rejected with probability ~1e-4, so a second rejection is already a
+// one-in-100-million event; a source that keeps producing rejected values (a
+// stuck fixed reader) fails loudly instead of looping.
+const secretAttempts = 32

internal/config/defaults_helpers_test.go

@@ -7,6 +7,7 @@ package config
 // Constraints: tests in this file must not run in parallel with each other.
 
 import (
+	"encoding/base64"
 	"errors"
 	"strings"
 	"testing"
@@ -68,3 +69,92 @@ func TestUseRandSourceErrorHidesSource(t *testing.T) {
 		t.Fatalf("want generating-secret error, got %v", err)
 	}
 }
+
+// fixedThenReader serves head once, then falls through to the countReader.
+type fixedThenReader struct {
+	head []byte
+	then countReader
+}
+
+func (r *fixedThenReader) Read(p []byte) (int, error) {
+	if len(r.head) > 0 {
+		n := copy(p, r.head)
+		r.head = r.head[n:]
+		return n, nil
+	}
+	return r.then.Read(p)
+}
+
+// encodedBytes returns the raw bytes whose base64url form starts with prefix
+// (padded with 'A' to a multiple of four), so a test can force the exact
+// string generateSecureRandom first sees.
+func encodedBytes(t *testing.T, prefix string, length int) []byte {
+	t.Helper()
+	enc := prefix + strings.Repeat("A", (4-len(prefix)%4)%4)
+	for len(enc) < length*4/3 {
+		enc += "AAAA"
+	}
+	raw, err := base64.RawURLEncoding.DecodeString(enc[:len(enc)/4*4])
+	if err != nil {
+		t.Fatal(err)
+	}
+	return raw[:length]
+}
+
+// A random draw that spells a blocked pattern (about 1 in 10^4 per project, the
+// flake in TestSurfaceIdentity) is discarded, so the secret always passes the
+// staging/prod validators.
+func TestGenerateSecureRandomSkipsBlockedPatterns(t *testing.T) {
+	for _, bad := range []string{"xxTeStxx", "AdMiNxx", "xxhasuraxx", "12345xxx", "change-me-"} {
+		for _, n := range []int{24, 44} {
+			src := &fixedThenReader{head: encodedBytes(t, bad, n)}
+			restore := UseRandSource(src)
+			s, err := generateSecureRandom(n)
+			restore()
+			if err != nil || len(s) != n {
+				t.Fatalf("%q len %d: %q, %v", bad, n, s, err)
+			}
+			if len(src.head) != 0 {
+				t.Fatalf("%q len %d: the forced draw was not consumed", bad, n)
+			}
+			if isInsecurePassword(s) || isPlaceholderSecret(s) {
+				t.Errorf("%q len %d: generated %q, which the validators reject", bad, n, s)
+			}
+		}
+	}
+}
+
+// Plan and apply read one seeded stream: skipping a bad draw must be the same
+// decision both times, so the two still produce one secret.
+func TestGenerateSecureRandomSkipIsDeterministic(t *testing.T) {
+	gen := func() string {
+		restore := UseRandSource(&fixedThenReader{head: encodedBytes(t, "xxtestxx", 24)})
+		defer restore()
+		s, err := generateSecureRandom(24)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return s
+	}
+	if a, b := gen(), gen(); a != b {
+		t.Errorf("seeded source, different secrets: %q vs %q", a, b)
+	}
+}
+
+// A source that only ever yields a blocked value fails instead of looping.
+func TestGenerateSecureRandomStuckSourceErrors(t *testing.T) {
+	restore := UseRandSource(&repeatReader{b: encodedBytes(t, "xxtestxx", 24)})
+	defer restore()
+	if s, err := generateSecureRandom(24); err == nil {
+		t.Fatalf("want an error, got %q", s)
+	}
+}
+
+type repeatReader struct{ b []byte }
+
+func (r *repeatReader) Read(p []byte) (int, error) {
+	for i := range p {
+		p[i] = r.b[i%len(r.b)]
+	}
+	return len(p), nil
+}