fix(config): replace TOKEN_SIGNING_SECRET entropy heuristic (#30)

* fix(config): replace TOKEN_SIGNING_SECRET entropy heuristic

The previous distinct-byte check was structurally broken in two
directions:

  - False positive: a real `openssl rand -hex 32` value is drawn
    from a 16-symbol alphabet; the expected number of distinct
    chars in 64 draws is ~15.75, so a genuinely random secret
    intermittently fell below the < 16 threshold and was rejected
    at startup under PROD_MODE=true.

  - False negative: literals like
    `0123456789abcdef0123456789abcdef` have exactly 16 distinct
    chars and passed the gate despite trivially repeating with
    period 16. A patterned secret with no actual entropy past the
    first 16 bytes would clear the check.

Replaced with a three-part shape check on the raw secret bytes:

  1. All-same byte (period 1) → "is N copies of a single byte".
  2. Repeating period < len(b) → "repeats with period P". Catches
     both the period-16 false-pass above and shorter shapes like
     `abcabc…`.
  3. Tiny alphabet (< 8 distinct values) → catches the
     `aaaa…b`-shaped near-miss and uneven-run-length shapes
     (`aaaaabbbbbcccccddddd…`) that defeat both the period and
     all-same checks. Threshold of 8 is the most defensive floor
     with zero practical false-positive rate on real random
     output: 32 hex chars over a 16-symbol alphabet expect ~14
     distinct, base64 / raw expect more.

The new check fires at the same two sites as the old one — the
warn path during secret parsing, and the hard-fail path under
PROD_MODE — so dev / single-replica deployments that intentionally
use a weak secret keep working under PROD_MODE=false with a
startup warning.

Tests:

  - TestLoad_SecretWeaknessWarning_PeriodicRepeat is the headline
    regression for the famous false-pass: rejects
    `0123456789abcdef0123456789abcdef`.
  - TestLoad_SecretWeaknessWarning_TinyAlphabet is a 4-case
    boundary table: 2-distinct rejects, 7-distinct rejects (just
    below the floor), 8-distinct passes (at the floor, true
    8-symbol non-periodic fixture), 15-distinct passes.
  - TestLoad_SecretWeaknessWarning_NonPeriodic covers the
    false-positive direction with frozen `openssl rand -hex 32`,
    base64-shaped, and alphanumeric-32 samples.
  - TestWeakSecretReason_FreshRandomNeverFlags is a 9000-sample
    experiment per CI run (1000 iterations × 3 encodings × 3
    raw-byte lengths) demonstrating zero false-positives on fresh
    crypto/rand output across raw, hex, and base64 shapes at
    lengths 32 / 48 / 64.
  - TestLoad_ProdMode_BlocksWeakSecret is now table-driven (3
    shapes × 2 modes = 6 sub-tests).
  - The setAllRequired test fixture's TOKEN_SIGNING_SECRET moved
    from the literal famous false-pass to a non-periodic
    real-random hex sample so every existing test runs against a
    secret the new gate accepts.
  - distinctByteCount removed entirely.

Docs aligned: specs.md, README.md, and docs/configuration.md
all describe the new semantics and point at
manifests/scripts/generate-signing-secret.sh as the canonical
generator path.

* docs: name the third weak-secret class (tiny alphabet) in prose

The validator rejects three weak-secret shapes — all-same byte,
short repeating period, AND tiny alphabet (< 8 distinct values).
The prose in README, docs/configuration.md, and specs.md
described only the first two; the tiny-alphabet floor (added in
this PR) was visible only via the inline Go code. A reader of
the docs alone now sees the full contract and the rationale for
each class.

* docs(README): correct generate-signing-secret.sh output description

The Demo stack section described the script as emitting "a 32-byte
random hex string". The script actually emits a 64-character
cryptographically-random base64 string (it pipes
`openssl rand -base64 48` through tr-d and head -c 64). Same
ballpark of bits of entropy, different encoding.

Author: babs

README.md

@@ -133,7 +133,7 @@ production posture.
 | **`OIDC_CLIENT_SECRET`** | IdP client secret |
 | **`PROXY_BASE_URL`** | Public URL of this proxy (audience-bound into every sealed token) |
 | **`UPSTREAM_MCP_URL`** | Upstream MCP URL with explicit path (`http://mcp:8000/mcp`); the path is the proxy's mount AND forwarded verbatim. Origin-only, fragment-bearing, or control-plane-colliding paths are rejected at startup |
-| **`TOKEN_SIGNING_SECRET`** | ≥ 32 bytes, AES-GCM key; byte-identical across replicas. Generate with `manifests/scripts/generate-signing-secret.sh`. Rotation procedure (with `TOKEN_SIGNING_SECRETS_PREVIOUS` for zero-downtime rollover) in [`docs/runbooks/key-rotation.md`](./docs/runbooks/key-rotation.md) |
+| **`TOKEN_SIGNING_SECRET`** | ≥ 32 bytes, AES-GCM key; byte-identical across replicas. **Generate with `manifests/scripts/generate-signing-secret.sh`** (64-char base64). The startup validator rejects three weak-secret shapes: all-same-byte, short-repeating-period, and tiny alphabet (< 8 distinct values). Under `PROD_MODE=true` weak secrets fail fast. Rotation procedure (with `TOKEN_SIGNING_SECRETS_PREVIOUS` for zero-downtime rollover) in [`docs/runbooks/key-rotation.md`](./docs/runbooks/key-rotation.md) |
 
 Optional knobs (rate limits, replay store tuning, header trust,
 observability, dev/compat) are documented in
@@ -258,8 +258,9 @@ Compose with Keycloak (pre-seeded realm + admin user), Redis, a
 minimal MCP server, and the proxy itself wired end-to-end. The
 `manifests/k8s/` set is split between reference YAML templates and a
 production-oriented kustomize overlay at
-`manifests/overlays/production`. `scripts/generate-signing-secret.sh`
-emits a 32-byte random hex string suitable for `TOKEN_SIGNING_SECRET`.
+`manifests/overlays/production`. `manifests/scripts/generate-signing-secret.sh`
+emits a 64-character cryptographically-random base64 string suitable
+for `TOKEN_SIGNING_SECRET`.
 
 ---
 

config/config.go

@@ -181,8 +181,9 @@ type Config struct {
 	// env: UPSTREAM_AUTHORIZATION_HEADER. Treat as a secret in
 	// deployment (mount from a Secret, not a ConfigMap).
 	UpstreamAuthorization string
-	// secretWeakWarning is non-empty when TOKEN_SIGNING_SECRET has low
-	// byte-entropy (fewer than 16 distinct bytes). Exposed via
+	// secretWeakWarning is non-empty when TOKEN_SIGNING_SECRET matches
+	// an obvious-weakness pattern (all-same byte, or short repeating
+	// period). Exposed via
 	// SecretWeaknessWarning() so the caller can emit a structured log
 	// event at startup without Load() taking a *zap.Logger dependency.
 	secretWeakWarning string
@@ -248,20 +249,18 @@ func Load() (*Config, error) {
 		return nil, fmt.Errorf("TOKEN_SIGNING_SECRET must be at least 32 bytes")
 	default:
 		c.TokenSigningSecret = []byte(secret)
-		// L1: count distinct bytes in the secret. A 32-byte string with <16
-		// unique byte values signals a human-picked or repeating pattern
-		// (e.g. "aaaaaaaa..."): the AES-GCM key derived from SHA-256 is
-		// still 256 bits wide, but the secret itself has far less effective
-		// entropy than its length suggests. Warn here unconditionally; the
-		// PROD_MODE violations block below promotes this to a hard error
-		// when strict mode is on, so dev / single-replica deployments that
-		// knowingly use a patterned secret keep working under
-		// PROD_MODE=false.
-		if distinct := distinctByteCount(c.TokenSigningSecret); distinct < 16 {
-			c.secretWeakWarning = fmt.Sprintf(
-				"TOKEN_SIGNING_SECRET has only %d distinct bytes (<16); effective entropy is much lower than its length suggests",
-				distinct,
-			)
+		// Detect obvious-weakness patterns (all-same byte, or short
+		// repeating period). The AES-GCM key derived via SHA-256 is
+		// still 256 bits wide, but the secret itself has near-zero
+		// effective entropy when it's `aaaa…` or `abcabc…`. Warn
+		// unconditionally; the PROD_MODE violations block below
+		// promotes it to a hard error when strict mode is on, so dev
+		// / single-replica work that knowingly uses a patterned
+		// secret keeps working under PROD_MODE=false. Use
+		// `manifests/scripts/generate-signing-secret.sh` to produce
+		// a known-good 64-char base64 value.
+		if reason := weakSecretReason(c.TokenSigningSecret); reason != "" {
+			c.secretWeakWarning = "TOKEN_SIGNING_SECRET " + reason
 		}
 	}
 
@@ -530,19 +529,18 @@ func Load() (*Config, error) {
 		if allowInsecureOIDCHTTP {
 			violations = append(violations, "OIDC_ALLOW_INSECURE_HTTP=true (cleartext OIDC exposes the client secret)")
 		}
-		// Promote the L1 low-entropy warning to a hard fail under
-		// PROD_MODE. A 32-byte secret with <16 distinct bytes signals
-		// a human-typed or repeating pattern; the AES-GCM key derived
-		// from SHA-256 stays 256 bits wide but the *secret* itself
-		// has far less effective entropy than its length suggests.
-		// Dev / single-replica work that intentionally uses a
-		// patterned secret should set PROD_MODE=false.
-		if d := distinctByteCount(c.TokenSigningSecret); d > 0 && d < 16 {
-			violations = append(violations, fmt.Sprintf("TOKEN_SIGNING_SECRET has %d distinct bytes (<16); patterned secret indicates a human-typed value", d))
+		// Promote the obvious-weakness warning to a hard fail under
+		// PROD_MODE. A patterned secret (all-same byte, or short
+		// repeating period) leaves the AES-GCM key derived from
+		// SHA-256 trivially recoverable. Dev / single-replica work
+		// that intentionally uses a patterned secret should set
+		// PROD_MODE=false.
+		if reason := weakSecretReason(c.TokenSigningSecret); reason != "" {
+			violations = append(violations, "TOKEN_SIGNING_SECRET "+reason)
 		}
 		for i, prev := range c.TokenSigningSecretsPrevious {
-			if d := distinctByteCount(prev); d > 0 && d < 16 {
-				violations = append(violations, fmt.Sprintf("TOKEN_SIGNING_SECRETS_PREVIOUS[%d] has %d distinct bytes (<16); rolling-rotation cutover would regress entropy floor", i, d))
+			if reason := weakSecretReason(prev); reason != "" {
+				violations = append(violations, fmt.Sprintf("TOKEN_SIGNING_SECRETS_PREVIOUS[%d] %s", i, reason))
 			}
 		}
 		if len(violations) > 0 {
@@ -567,17 +565,58 @@ func (c *Config) SecretWeaknessWarning() string {
 	return c.secretWeakWarning
 }
 
-// distinctByteCount returns the number of unique byte values in b.
-func distinctByteCount(b []byte) int {
+// weakSecretReason returns a non-empty human-readable reason when b
+// matches an obvious-weakness pattern, or "" when b looks sane.
+// Catches three classes:
+//
+//  1. All-same byte (period 1).
+//  2. Repeating period < len(b) — e.g. `abcabc…`,
+//     `0123456789abcdef0123456789abcdef`.
+//  3. Tiny alphabet (< 8 distinct bytes). Closes shapes that
+//     defeat the period check by having uneven run-lengths
+//     (`aaaa…b`, `aaaaabbbbbcccccddddd…`) but obviously cluster
+//     around a small symbol set.
+//
+// Threshold choice for class 3: 8 is the most defensive floor
+// that still has zero practical false-positive rate on real
+// random output. A 32-char hex secret over a 16-symbol alphabet
+// has expected ~14 distinct chars (P(<8 distinct in 64 chars) is
+// essentially zero); base64 (64 symbols) and raw bytes (256)
+// cluster even higher. 4 was the prior choice and accepted
+// shapes like 5-symbol clustered runs; 8 closes that and stays
+// well below the random-hex distribution.
+func weakSecretReason(b []byte) string {
+	if len(b) < 2 {
+		return ""
+	}
+	for period := 1; period*2 <= len(b); period++ {
+		repeats := true
+		for i := period; i < len(b); i++ {
+			if b[i] != b[i%period] {
+				repeats = false
+				break
+			}
+		}
+		if repeats {
+			if period == 1 {
+				return fmt.Sprintf("is %d copies of a single byte (effectively zero entropy)", len(b))
+			}
+			return fmt.Sprintf("repeats with period %d (truly random secrets are non-periodic; use manifests/scripts/generate-signing-secret.sh)", period)
+		}
+	}
+	const minDistinct = 8
 	var seen [256]bool
-	n := 0
+	distinct := 0
 	for _, v := range b {
 		if !seen[v] {
 			seen[v] = true
-			n++
+			distinct++
+			if distinct >= minDistinct {
+				return ""
+			}
 		}
 	}
-	return n
+	return fmt.Sprintf("uses only %d distinct byte values (use manifests/scripts/generate-signing-secret.sh)", distinct)
 }
 
 // validateRedisKeyPrefix enforces ASCII-printable only (no cluster-hash