fix(auth): derive stable CSRF key from encryption key (closes #1120)

[cristim]

What

Fixes intermittent "CSRF validation failed" on deployed write operations (plan create, admin user-group update), for both Standard and Admin users. Closes #1120.

Root cause

Production constructed the auth service with no CSRFKey, so auth.NewService fell back to a random ephemeral key per process (internal/auth/service.go:107-118). CSRF tokens are HMAC-SHA256(csrfKey, rawSessionToken), minted at login and validated on every write. On a multi-instance Lambda fleet (and across cold-starts), a token minted by instance A was rejected by instance B because each had a different key. The frontend fetches the token once at login and reuses it (X-CSRF-Token), so it is only stable if the server key is stable.

Both production construction sites omitted CSRFKey: internal/server/app.go:460 (cold-start placeholder) and :691 (the request-serving service in reinitializeAfterConnect).

Fix

Derive a stable CSRFKey from the deploy-provided credential encryption key (already mandatory in production) via HKDF-SHA256 with a domain-separation label (auth.DeriveCSRFKey), and pass it into the request-serving auth service. Every instance and cold-start now derives the same key, so a token minted on one instance validates on another.

• No new secret / terraform change: the encryption key is already required and stable across the fleet.
• Fails closed: if the encryption key can't load, the app already refuses to start; DeriveCSRFKey also rejects an empty master secret.
• HKDF domain separation keeps the CSRF key cryptographically independent of the encryption key.
• The encryption-key load is memoized (sync.Once), so seeding the CSRF key before the credential store adds no extra work; it's loaded once and reused.

Verification (red -> green)

• New TestCSRFKey_CrossInstanceStable asserts the cross-instance invariant: a token minted by instance A validates on instance B sharing the same master secret, and is rejected by an instance built from a different secret.
• Proven the test catches the bug: a temporary simulation of the pre-fix wiring (services built with no CSRFKey -> random keys) made cross-instance validation fail (the exact QA symptom); with the stable derived key it passes. Temporary sim removed.
• go build ./... clean; go vet clean; go test ./internal/auth/ ./internal/server/ = 868 passed.

Notes

The unit test TestAuthServiceAdapter_ValidateCSRFToken reported as failing on base was already fixed on the current feat/multicloud-web-frontend tip (the mock pins a fixed CSRFKey); it passes here. The remaining and real defect was the production wiring, fixed by this PR.

[coderabbitai]

Warning

Review limit reached

@cristim, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 48 minutes and 50 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 47f33958-7d6c-4e50-b2e5-efcaf5f19f01

📥 Commits

Reviewing files that changed from the base of the PR and between 8e5f139 and a7db5e1.

📒 Files selected for processing (3)

• internal/auth/service_helpers.go
• internal/auth/service_security_test.go
• internal/server/app.go

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/csrf-validation-failures

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

Independent adversarial re-review — VERIFIED-CORRECT ✅

Traced the full request lifecycle against the committed head (046e3d0a3), not the PR description. The fix is correct and complete.

Routing: every write goes through the fixed-key service

The cold-start placeholder auth service (app.go:459, no CSRFKey → random key) is wired into the cold-start API handler, but that handler is fully replaced before any request reaches it:

• Both entrypoints call ensureDB before dispatching to app.API:
  • lambda.go:27 ensureDB → lambda.go:126 app.API.HandleRequest
  • http.go:162 ensureDB → http.go:172 app.API.HandleRequest
• ensureDB → reinitializeAfterConnect rebuilds app.Auth with the HKDF key (app.go:710) and rebuilds app.API with newAuthServiceAdapter(app.Auth) (app.go:827).
• ensureDB holds dbMu for its full duration and only sets dbConnected=true after reinitializeAfterConnect succeeds; on error it returns (lambda → error, http → 503) with no fallthrough to the placeholder.

So the placeholder/random-key service can never serve a real write. The only ensureDB-skipping path is handleVersion (http.go:185), which serves the public /version route only (no CSRF) — not a write path. handleScheduledHTTP calls ensureDB and is gated by a separate scheduledAuthMiddleware (not the CSRF token).

HKDF + fail-closed

• DeriveCSRFKey (service_helpers.go) = HKDF-SHA256, salt=nil, info="CUDly:csrf-key:v1" (domain separation), 32-byte output. Rejects an empty master secret with an error.
• Master secret = the credential encryption key from credentials.LoadKey (Secrets Manager ARN / Key Vault / GCP Secret Manager / raw hex env), memoized via cachedKeyOnce sync.Once → stable across the fleet and across cold-starts, not per-process. loadAndGuardEncryptionKey also rejects the all-zero dev key in prod.
• Fail-closed chain intact: empty/missing secret → LoadKey/DeriveCSRFKey error → reinitializeAfterConnect returns error → ensureDB errors → request aborts. No random-key fallback is reachable on the request-serving path.

Validation/mint unchanged

• ValidateCSRFToken (service.go:356) untouched — still recomputes HMAC-SHA256(csrfKey, rawSessionToken) and compares with subtle.ConstantTimeCompare. Constant-time compare preserved.
• deriveCSRFToken (the HMAC) unchanged; createSession mints via s.csrfKey, so login now mints under the stable derived key.
• FE is unchanged and needs no change (client.ts replays the login-issued X-CSRF-Token); the FE was never the defect.

Test red→green

• TestCSRFKey_CrossInstanceStable + TestDeriveCSRFKey_RejectsEmptySecret: pass on head (go build ./... + go test ./internal/auth/ green).
• Independent red check: a throwaway test simulating the exact pre-fix wiring (two NewService with no CSRFKey → random keys) confirmed A's token is rejected by B — the precise QA symptom (rows 547/548, 14.10). Removed after confirming.

Note (acceptable)

Tokens minted before deploy (old random key) will fail until re-login — inherent to any CSRF-key rotation, one-time per user.

Merge-readiness

Code is merge-ready. CI: sanity green. Caveat: the CodeRabbit "pass" is a rate-limit auto-reply, not a substantive review — CR hit the per-developer rate limit (reviews available ~01:00 UTC). A real @coderabbitai review should run after the window before merge.

Verdict: VERIFIED-CORRECT.

[coderabbitai]

Your plan includes 5 PR reviews per hour. More reviews will be available in 26 minutes and 18 seconds.

[cristim]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[cristim]

Gocyclo fix landed in a7db5e1.

(*Application).reinitializeAfterConnect was at complexity 11 after the CSRF HKDF branch was added in #1120. The CSRF key derivation and auth.Service construction were extracted into a new private method (*Application).initAuthService (complexity 3), bringing reinitializeAfterConnect down to 10 -- exactly at the project limit, not over it.

Verified locally:

• gocyclo -over 10 $(git ls-files "*.go" | grep -v _test.go | grep -v vendor/) reports zero violations
• go build ./... clean
• go test ./internal/server/... 365 tests pass

@coderabbitai review

[coderabbitai]

@cristim The extraction of CSRF key derivation + auth service construction into initAuthService is a clean refactor — keeping reinitializeAfterConnect at exactly the gocyclo-10 limit while isolating the new complexity in a focused, testable helper is the right call. Kicking off the full review now.

✅ Action performed

Full review finished.