Merge dev to main: GCM AAD binding, mock type safety, dependency updates

cmd/cvert-ops/rotate.go

@@ -100,21 +100,22 @@ func rotateEncryptionKeys(ctx context.Context, pool *pgxpool.Pool, currentKey, p
 		return 0, fmt.Errorf("set bypass_rls: %w", err)
 	}
 
-	rows, err := tx.Query(ctx, "SELECT id, client_secret_enc FROM sso_connections")
+	rows, err := tx.Query(ctx, "SELECT id, org_id, client_secret_enc FROM sso_connections")
 	if err != nil {
 		return 0, fmt.Errorf("query sso_connections: %w", err)
 	}
 	defer rows.Close()
 
 	type pending struct {
-		id  string
-		enc []byte
+		id    string
+		orgID [16]byte
+		enc   []byte
 	}
 
 	var updates []pending
 	for rows.Next() {
 		var p pending
-		if err := rows.Scan(&p.id, &p.enc); err != nil {
+		if err := rows.Scan(&p.id, &p.orgID, &p.enc); err != nil {
 			return 0, fmt.Errorf("scan row: %w", err)
 		}
 		updates = append(updates, p)
@@ -125,12 +126,12 @@ func rotateEncryptionKeys(ctx context.Context, pool *pgxpool.Pool, currentKey, p
 
 	count := 0
 	for _, u := range updates {
-		plaintext, err := crypto.DecryptWithFallback(currentKey, previousKey, u.enc)
+		plaintext, err := crypto.DecryptWithFallback(currentKey, previousKey, u.enc, u.orgID[:])
 		if err != nil {
 			return 0, fmt.Errorf("decrypt row %s: %w", u.id, err)
 		}
 
-		newEnc, err := crypto.Encrypt(currentKey, plaintext)
+		newEnc, err := crypto.Encrypt(currentKey, plaintext, u.orgID[:])
 		if err != nil {
 			return 0, fmt.Errorf("re-encrypt row %s: %w", u.id, err)
 		}

cmd/cvert-ops/rotate_test.go

@@ -29,9 +29,9 @@ func TestRotateEncryptionKey_ReEncryptsAllValues(t *testing.T) {
 	}
 	orgID := org.ID
 
-	// Encrypt a secret with the old key.
+	// Encrypt a secret with the old key, bound to the org.
 	secret := []byte("my-client-secret")
-	enc, err := crypto.Encrypt(oldKey, secret)
+	enc, err := crypto.Encrypt(oldKey, secret, orgID[:])
 	if err != nil {
 		t.Fatalf("encrypt with old key: %v", err)
 	}
@@ -64,7 +64,7 @@ func TestRotateEncryptionKey_ReEncryptsAllValues(t *testing.T) {
 		t.Fatalf("read re-encrypted value: %v", err)
 	}
 
-	plaintext, err := crypto.Decrypt(newKey, reEncrypted)
+	plaintext, err := crypto.Decrypt(newKey, reEncrypted, orgID[:])
 	if err != nil {
 		t.Fatalf("decrypt with new key failed: %v", err)
 	}
@@ -73,7 +73,7 @@ func TestRotateEncryptionKey_ReEncryptsAllValues(t *testing.T) {
 	}
 
 	// Verify old key alone no longer works.
-	_, err = crypto.Decrypt(oldKey, reEncrypted)
+	_, err = crypto.Decrypt(oldKey, reEncrypted, orgID[:])
 	if err == nil {
 		t.Error("decrypt with old key should fail on re-encrypted data, but succeeded")
 	}

dev/specs/sso-secret-storage.md

@@ -0,0 +1,93 @@
+# SSO Secret Storage Architecture
+
+This document describes how CVErt Ops stores and manages user-provided secrets (specifically, OAuth/OIDC client secrets for enterprise SSO connections) in the production/SaaS configuration.
+
+## What's Encrypted
+
+The **only** user-input secret encrypted at rest in the database is `sso_connections.client_secret_enc` — the OIDC client secret that tenants provide when configuring enterprise SSO. It is stored as `BYTEA` in Postgres (migration `000028_sso_connections.up.sql`).
+
+## Encryption Scheme
+
+**AES-256-GCM** with random 12-byte nonces, implemented in `internal/crypto/aes.go`.
+
+- **Ciphertext format:** `nonce (12 bytes) || ciphertext + GCM authentication tag`
+- **Nonce source:** `crypto/rand.Reader` (OS CSPRNG)
+- **Library:** Go stdlib `crypto/aes` and `crypto/cipher` — no external crypto dependencies
+
+AES-256-GCM provides both confidentiality and integrity (authenticated encryption). An attacker who obtains a database dump cannot read or tamper with the client secrets without also possessing the encryption key.
+
+## Key Sourcing
+
+The encryption key is a raw 32-byte value provided as 64 hex characters via:
+
+1. **Startup:** The `SSO_ENCRYPTION_KEY` environment variable, parsed by `internal/config/reloadable.go`
+2. **Hot-reload:** A secrets file (one `KEY=VALUE` per line) can be reloaded at runtime via `SIGHUP` signal or the admin API reload endpoint. The key is swapped atomically using `atomic.Pointer` in `config.Holder`, so in-flight requests are never disrupted
+
+The API handler reads the active key via `srv.ssoEncryptionKey()` in `internal/api/sso.go`, which prefers the hot-reloadable config, falling back to the startup config value.
+
+## Key Rotation
+
+Key rotation uses a **dual-key** strategy with zero downtime:
+
+1. **Operator** generates a new 32-byte key (`openssl rand -hex 32`)
+2. **Operator** moves the current `SSO_ENCRYPTION_KEY` value to `SSO_ENCRYPTION_KEY_PREVIOUS` and sets the new key as `SSO_ENCRYPTION_KEY` in the secrets file
+3. **Operator** reloads config (SIGHUP or admin API)
+4. **During the transition window**, all decryption uses `crypto.DecryptWithFallback()` — tries the current key first, then falls back to the previous key on GCM authentication failure. Structural errors (truncated ciphertext, invalid key length) fail fast without attempting fallback
+5. **Operator** runs `cvert-ops rotate-encryption-key`, which re-encrypts every `sso_connections.client_secret_enc` row in a single Postgres transaction: decrypt with fallback, re-encrypt with current key
+6. **After re-encryption succeeds**, the operator removes `SSO_ENCRYPTION_KEY_PREVIOUS` and reloads config
+
+The re-encryption command is transactional — if it fails partway through, the transaction rolls back and all rows remain encrypted with the original key. Safe to retry.
+
+The full step-by-step procedure is documented in `docs/deployment/runbooks/secret-rotation.md`.
+
+## Security Boundaries and Assumptions
+
+| Boundary | Status |
+|----------|--------|
+| **Encryption at rest** | AES-256-GCM. Protects against database dump or backup theft |
+| **Tenant isolation** | Row-Level Security (RLS) on `sso_connections` + `org_id` scoping. One tenant cannot read another's encrypted secret |
+| **Key storage** | The encryption key lives in an environment variable or secrets file on the host. There is no KMS or HSM wrapping — compromise of the application server's environment means compromise of the key |
+| **Memory exposure** | The key is held in process memory as a `[32]byte`. Standard Go runtime — no `mlock` or secure memory wipe. Acceptable for non-HSM deployments |
+| **Rotation atomicity** | The `rotate-encryption-key` command runs in a single DB transaction. Failure leaves all rows encrypted with the old key (safe to retry) |
+| **No envelope encryption** | There is no KMS-wrapped DEK/KEK split. `SSO_ENCRYPTION_KEY` is the data encryption key directly. Key rotation therefore requires re-encrypting every row (currently only `sso_connections`, so the blast radius is small) |
+
+### Deployment expectation
+
+The security model assumes that the deployment environment adequately protects the `SSO_ENCRYPTION_KEY` value. In practice this means:
+
+- **Container deployments:** Use the platform's native secret injection (Kubernetes Secrets, Docker Swarm secrets, ECS task definition secrets, etc.)
+- **Cloud VMs:** Use a cloud secret manager (AWS Secrets Manager, GCP Secret Manager, Azure Key Vault) to inject the value into the environment at startup
+- **Self-hosted:** Ensure the secrets file has restrictive file permissions and is excluded from backups and version control
+
+If CVErt Ops later needs to support a managed SaaS model where the operator controls infrastructure, the natural upgrade path would be envelope encryption with a cloud KMS wrapping the SSO encryption key.
+
+## What's NOT Encrypted at Rest
+
+These values are **not** stored in the database — they live only in environment variables or the secrets file:
+
+- OAuth provider secrets (`GITHUB_CLIENT_SECRET`, `GOOGLE_CLIENT_SECRET`) — app-level config, not tenant-provided
+- JWT signing secrets (`JWT_SECRET`, `JWT_SECRET_PREVIOUS`)
+- SMTP credentials (`SMTP_PASSWORD`)
+
+These values are stored in the database but use **hashing, not encryption** (correct approach — they never need to be recovered in plaintext):
+
+- User passwords — argon2id
+- API key hashes
+
+## Key Files
+
+| File | Role |
+|------|------|
+| `internal/crypto/aes.go` | AES-256-GCM Encrypt / Decrypt / DecryptWithFallback |
+| `internal/config/reloadable.go` | Hot-reloadable config with atomic key swap |
+| `internal/api/sso.go` | SSO handler — encrypts on write, decrypts on read |
+| `cmd/cvert-ops/rotate.go` | CLI re-encryption command |
+| `migrations/000028_sso_connections.up.sql` | Schema with `client_secret_enc BYTEA` column + RLS |
+| `internal/store/queries/sso.sql` | sqlc queries (encrypted column passed as opaque bytes) |
+| `docs/deployment/runbooks/secret-rotation.md` | Operator-facing rotation procedures |
+
+## Dependencies
+
+- **Go stdlib crypto** (`crypto/aes`, `crypto/cipher`, `crypto/rand`) — no third-party crypto libraries
+- **pgx** for the rotation transaction
+- **Operator-managed key** — no external secrets manager SDK dependency

go.mod

@@ -17,7 +17,7 @@ require (
 	github.com/golang-migrate/migrate/v4 v4.19.1
 	github.com/google/uuid v1.6.0
 	github.com/jackc/pgx/v5 v5.9.1
-	github.com/lib/pq v1.12.0
+	github.com/lib/pq v1.12.3
 	github.com/pquerna/otp v1.5.0
 	github.com/prometheus/client_golang v1.23.2
 	github.com/sony/gobreaker/v2 v2.4.0
@@ -32,7 +32,7 @@ require (
 	golang.org/x/crypto v0.49.0
 	golang.org/x/oauth2 v0.36.0
 	golang.org/x/time v0.15.0
-	google.golang.org/genai v1.52.0
+	google.golang.org/genai v1.52.1
 )
 
 require (

go.sum

@@ -129,8 +129,8 @@ github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 h1:SOEGU9fKiNWd/HOJuq
 github.com/lann/builder v0.0.0-20180802200727-47ae307949d0/go.mod h1:dXGbAdH5GtBTC4WfIxhKZfyBF/HBFgRZSWwZ9g/He9o=
 github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 h1:P6pPBnrTSX3DEVR4fDembhRWSsG5rVo6hYhAB/ADZrk=
 github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0/go.mod h1:vmVJ0l/dxyfGW6FmdpVm2joNMFikkuWg0EoCKLGUMNw=
-github.com/lib/pq v1.12.0 h1:mC1zeiNamwKBecjHarAr26c/+d8V5w/u4J0I/yASbJo=
-github.com/lib/pq v1.12.0/go.mod h1:/p+8NSbOcwzAEI7wiMXFlgydTwcgTr3OSKMsD2BitpA=
+github.com/lib/pq v1.12.3 h1:tTWxr2YLKwIvK90ZXEw8GP7UFHtcbTtty8zsI+YjrfQ=
+github.com/lib/pq v1.12.3/go.mod h1:/p+8NSbOcwzAEI7wiMXFlgydTwcgTr3OSKMsD2BitpA=
 github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ81pIr0yLvtUWk2if982qA3F3QD6H4=
 github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2jmtg6P3p1VtQj7WsuWi/y4VnjVBn7F8KPB3I=
 github.com/magiconair/properties v1.8.10 h1:s31yESBquKXCV9a/ScB3ESkOjUYYv+X0rg8SYxI99mE=
@@ -276,8 +276,8 @@ golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gonum.org/v1/gonum v0.16.0 h1:5+ul4Swaf3ESvrOnidPp4GZbzf0mxVQpDCYUQE7OJfk=
 gonum.org/v1/gonum v0.16.0/go.mod h1:fef3am4MQ93R2HHpKnLk4/Tbh/s0+wqD5nfa6Pnwy4E=
-google.golang.org/genai v1.52.0 h1:ekVIxWHtLUNbt+v0WWi4j3JT4yrHDEbysMcHQcaCQoI=
-google.golang.org/genai v1.52.0/go.mod h1:A3kkl0nyBjyFlNjgxIwKq70julKbIxpSxqKO5gw/gmk=
+google.golang.org/genai v1.52.1 h1:dYoljKtLDXMiBdVaClSJ/ZPwZ7j1N0lGjMhwOKOQUlk=
+google.golang.org/genai v1.52.1/go.mod h1:A3kkl0nyBjyFlNjgxIwKq70julKbIxpSxqKO5gw/gmk=
 google.golang.org/genproto v0.0.0-20250603155806-513f23925822 h1:rHWScKit0gvAPuOnu87KpaYtjK5zBMLcULh7gxkCXu4=
 google.golang.org/genproto/googleapis/api v0.0.0-20260209200024-4cfbd4190f57 h1:JLQynH/LBHfCTSbDWl+py8C+Rg/k1OVH3xfcaiANuF0=
 google.golang.org/genproto/googleapis/api v0.0.0-20260209200024-4cfbd4190f57/go.mod h1:kSJwQxqmFXeo79zOmbrALdflXQeAYcUbgS7PbpMknCY=

internal/api/auth_mfa.go

@@ -374,7 +374,7 @@ func (srv *Server) verifyTOTP(ctx context.Context, userID uuid.UUID, code string
 		return false, fmt.Errorf("encryption key: %w", err)
 	}
 	prevKey := srv.ssoEncryptionKeyPrevious()
-	secretBytes, err := crypto.DecryptWithFallback(encKey, prevKey, cred.SecretEnc)
+	secretBytes, err := crypto.DecryptWithFallback(encKey, prevKey, cred.SecretEnc, userID[:])
 	if err != nil {
 		return false, fmt.Errorf("decrypt TOTP secret: %w", err)
 	}
@@ -580,7 +580,7 @@ func (srv *Server) mfaTOTPSetupHandler(ctx context.Context, input *mfaTOTPSetupI
 		slog.ErrorContext(ctx, "totp-setup: encryption key", "error", err)
 		return nil, huma.Error500InternalServerError("encryption key not configured")
 	}
-	secretEnc, err := crypto.Encrypt(encKey, []byte(key.Secret()))
+	secretEnc, err := crypto.Encrypt(encKey, []byte(key.Secret()), userID[:])
 	if err != nil {
 		slog.ErrorContext(ctx, "totp-setup: encrypt secret", "error", err)
 		return nil, huma.Error500InternalServerError("internal error")
@@ -645,7 +645,7 @@ func (srv *Server) mfaTOTPConfirmHandler(ctx context.Context, input *mfaTOTPConf
 		return nil, huma.Error500InternalServerError("internal error")
 	}
 	prevKey := srv.ssoEncryptionKeyPrevious()
-	secretBytes, err := crypto.DecryptWithFallback(encKey, prevKey, enrollClaims.SecretEnc)
+	secretBytes, err := crypto.DecryptWithFallback(encKey, prevKey, enrollClaims.SecretEnc, userID[:])
 	if err != nil {
 		slog.ErrorContext(ctx, "totp-confirm: decrypt secret", "error", err)
 		return nil, huma.Error500InternalServerError("internal error")
@@ -677,7 +677,7 @@ func (srv *Server) mfaTOTPConfirmHandler(ctx context.Context, input *mfaTOTPConf
 
 	// Re-encrypt secret for DB storage (enrollment cookie used same key, but
 	// re-encrypt to get a fresh nonce for defense in depth).
-	secretEncDB, err := crypto.Encrypt(encKey, secretBytes)
+	secretEncDB, err := crypto.Encrypt(encKey, secretBytes, userID[:])
 	if err != nil {
 		slog.ErrorContext(ctx, "totp-confirm: re-encrypt secret", "error", err)
 		return nil, huma.Error500InternalServerError("internal error")

internal/api/auth_mfa_test.go

@@ -55,7 +55,7 @@ func enrollTOTP(t *testing.T, ctx context.Context, srv *Server, userID uuid.UUID
 	if err != nil {
 		t.Fatalf("enrollTOTP: encryption key: %v", err)
 	}
-	secretEnc, err := crypto.Encrypt(encKey, []byte(secret))
+	secretEnc, err := crypto.Encrypt(encKey, []byte(secret), userID[:])
 	if err != nil {
 		t.Fatalf("enrollTOTP: encrypt: %v", err)
 	}

internal/api/oauth_oidc.go

@@ -53,7 +53,7 @@ func (srv *Server) oidcBuildOAuthConfig(ctx context.Context, conn *store.SSOConn
 	if err != nil {
 		return nil, nil, fmt.Errorf("encryption key: %w", err)
 	}
-	secret, err := crypto.DecryptWithFallback(key, srv.ssoEncryptionKeyPrevious(), conn.ClientSecretEnc)
+	secret, err := crypto.DecryptWithFallback(key, srv.ssoEncryptionKeyPrevious(), conn.ClientSecretEnc, conn.OrgID[:])
 	if err != nil {
 		return nil, nil, fmt.Errorf("decrypt secret: %w", err)
 	}

internal/api/sso.go

@@ -171,7 +171,7 @@ func (srv *Server) createSSOHandler(w http.ResponseWriter, r *http.Request) {
 		writeProblem(w, http.StatusInternalServerError, "server configuration error")
 		return
 	}
-	encSecret, err := crypto.Encrypt(key, []byte(req.ClientSecret))
+	encSecret, err := crypto.Encrypt(key, []byte(req.ClientSecret), orgID[:])
 	if err != nil {
 		slog.ErrorContext(r.Context(), "sso create: encrypt secret", "error", err)
 		writeProblem(w, http.StatusInternalServerError, "encryption error")
@@ -347,7 +347,7 @@ func (srv *Server) patchSSOHandler(w http.ResponseWriter, r *http.Request) {
 			writeProblem(w, http.StatusInternalServerError, "server configuration error")
 			return
 		}
-		secretEnc, err = crypto.Encrypt(key, []byte(*req.ClientSecret))
+		secretEnc, err = crypto.Encrypt(key, []byte(*req.ClientSecret), orgID[:])
 		if err != nil {
 			slog.ErrorContext(r.Context(), "sso patch: encrypt secret", "error", err)
 			writeProblem(w, http.StatusInternalServerError, "encryption error")

internal/crypto/aes.go

@@ -15,17 +15,18 @@ import (
 // authentication fails and previousKey is non-zero, it retries with
 // previousKey. This supports seamless encryption key rotation.
 // Structural errors (truncated ciphertext, invalid key) fail immediately
-// without attempting fallback.
-func DecryptWithFallback(currentKey, previousKey [32]byte, data []byte) ([]byte, error) {
-	plaintext, err := Decrypt(currentKey, data)
+// without attempting fallback. The aad (additional authenticated data) is
+// passed through to GCM and must match the value used during encryption.
+func DecryptWithFallback(currentKey, previousKey [32]byte, data []byte, aad []byte) ([]byte, error) {
+	plaintext, err := Decrypt(currentKey, data, aad)
 	if err == nil {
 		return plaintext, nil
 	}
 
 	// Only fall back on GCM authentication failure (wrong key).
 	// Structural errors (truncated ciphertext, invalid key) fail fast.
 	if previousKey != [32]byte{} && isGCMAuthError(err) {
-		plaintext, err2 := Decrypt(previousKey, data)
+		plaintext, err2 := Decrypt(previousKey, data, aad)
 		if err2 == nil {
 			return plaintext, nil
 		}
@@ -42,8 +43,10 @@ func isGCMAuthError(err error) bool {
 }
 
 // Encrypt encrypts plaintext using AES-256-GCM with a random nonce.
-// Returns nonce || ciphertext.
-func Encrypt(key [32]byte, plaintext []byte) ([]byte, error) {
+// Returns nonce || ciphertext. The aad (additional authenticated data) is
+// mixed into the GCM authentication tag, binding the ciphertext to a context
+// (e.g., an org_id or user_id). Pass nil for context-free encryption.
+func Encrypt(key [32]byte, plaintext []byte, aad []byte) ([]byte, error) {
 	block, err := aes.NewCipher(key[:])
 	if err != nil {
 		return nil, fmt.Errorf("aes new cipher: %w", err)
@@ -60,12 +63,13 @@ func Encrypt(key [32]byte, plaintext []byte) ([]byte, error) {
 	}
 
 	// Seal appends ciphertext to nonce, so result is nonce || ciphertext.
-	return gcm.Seal(nonce, nonce, plaintext, nil), nil
+	return gcm.Seal(nonce, nonce, plaintext, aad), nil
 }
 
 // Decrypt decrypts AES-256-GCM ciphertext produced by Encrypt.
-// Expects nonce (12 bytes) || ciphertext.
-func Decrypt(key [32]byte, data []byte) ([]byte, error) {
+// Expects nonce (12 bytes) || ciphertext. The aad must match the value
+// used during encryption; a mismatch causes an authentication failure.
+func Decrypt(key [32]byte, data []byte, aad []byte) ([]byte, error) {
 	block, err := aes.NewCipher(key[:])
 	if err != nil {
 		return nil, fmt.Errorf("aes new cipher: %w", err)
@@ -82,7 +86,7 @@ func Decrypt(key [32]byte, data []byte) ([]byte, error) {
 	}
 
 	nonce, ciphertext := data[:nonceSize], data[nonceSize:]
-	plaintext, err := gcm.Open(nil, nonce, ciphertext, nil)
+	plaintext, err := gcm.Open(nil, nonce, ciphertext, aad)
 	if err != nil {
 		return nil, fmt.Errorf("gcm decrypt: %w", err)
 	}