refactor(backend): validate cookie-derived values in lastfm/spotify routes

[LucasSantana-Dev]

Summary

Fixes issue #1627 by replacing unsafe type assertions with explicit runtime validation for cookie-derived state parameters.

Changes

Problem: Both lastfm.ts and spotify.ts had unchecked type assertions:

const cookies = req.cookies as Record<string, unknown> | undefined

This cast was unsafe because:

1. Express already types req.cookies as Record<string, string> when cookie-parser middleware is used
2. Casting to Record<string, unknown> unnecessarily widens the type
3. The widening hides type safety and suggests the narrowing check (typeof stateFromCookie === 'string') is optional when it's actually critical for security

Solution: Remove the unsafe cast and add explicit zod-based validation:

1. Extract req.cookies[COOKIE_NAME] directly (properly typed as string | undefined)
2. Add stateCookieSchema using zod to validate the cookie value is a non-empty string
3. Use stateCookieSchema.safeParse() for explicit runtime validation
4. Maintain existing priority: query string state > cookie state

Security Impact

• Removes type-unsafe cast that could hide validation bugs
• Makes the state validation explicit and testable
• Maintains same defensive behavior: validates state before crypto operations
• Related to H2: timingSafeEqual crash on mismatched buffer lengths in lastfm.ts #1622 (timingSafeEqual crash prevention)

Testing

• Changes maintain existing test compatibility (no behavior change)
• Existing tests already validate state parameter handling with various edge cases
• State validation via zod ensures non-empty string requirement

Closes #1627

---

Summary by cubic

Validate OAuth state from cookies in Last.fm and Spotify callbacks with zod, and restore safe cookie access to avoid 500s. Behavior unchanged: query state takes precedence; cookie is cleared. Closes #1627.

• Refactors
  • Replace unsafe req.cookies cast with stateCookieSchema and zod.safeParse (non-empty string).
  • Use req.cookies?.[STATE_COOKIE], typing the raw value as unknown before validation.
  • Preserve precedence (query over cookie) and clear the cookie in the callback.

^{Written for commit 098cb67. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Improved reliability of Last.fm and Spotify account-linking during OAuth sign-in.
  • Added stronger validation for OAuth state values sourced from callback parameters and cookies.
  • Reduced callback failures caused by missing or malformed state, while preserving the existing redirect behavior when state is unavailable.

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

	Warnings
⚠️	User-facing change without a CHANGELOG.md update. Add a line under ## [Unreleased] if this should appear in release notes. (Or apply the skip-changelog label if this PR does not affect end users.)
⚠️	Branch fix-1627-route-casts doesn't follow the standard prefix convention (feature/, fix/, refactor/, chore/, docs/, ci/, test/, release/).

Generated by 🚫 dangerJS against 098cb67

[coderabbitai]

📝 Walkthrough

Walkthrough

Both Last.fm and Spotify OAuth callback routes now validate cookie-derived state values with Zod schemas. Each callback prefers the validated query state, otherwise falls back to a schema-checked cookie value before continuing the existing OAuth flow.

Changes

OAuth state cookie validation

Layer / File(s)	Summary
Last.fm callback state validation packages/backend/src/routes/lastfm.ts	Adds stateCookieSchema, separates query validation from state extraction, and uses schema-validated lastfm_state cookie data as the fallback state.
Spotify callback state validation packages/backend/src/routes/spotify.ts	Adds stateCookieSchema, separates query validation from state extraction, and uses schema-validated spotify_state cookie data as the fallback state.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: cubic-dev-ai

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR addresses #1627 by adding runtime validation for cookie-derived OAuth state in lastfm.ts and spotify.ts before use.
Out of Scope Changes check	✅ Passed	The changes stay focused on the Last.fm and Spotify route validation work and do not introduce unrelated scope.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly reflects the main backend change: adding validation for cookie-derived values in the Last.fm and Spotify routes.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-1627-route-casts

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/backend/src/routes/lastfm.ts`:
- Around line 216-217: The raw cookie value in the lastfm route is still
inferred as any before Zod parsing; make the untrusted boundary explicit by
typing the value read from req.cookies[LASTFM_STATE_COOKIE] as unknown in the
stateFromCookieRaw assignment, then keep using stateCookieSchema.safeParse on
that unknown input in the same validation flow.

In `@packages/backend/src/routes/spotify.ts`:
- Around line 214-215: The raw Spotify state cookie is still coming from
req.cookies as an untyped boundary, so make the untrusted value explicit by
anchoring stateFromCookieRaw as unknown before passing it into
stateCookieSchema.safeParse. Keep the existing Zod validation flow in
spotify.ts, but ensure the variable feeding the parser is typed as unknown so
the any inference is removed and the validation boundary is clear.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 42dde9ba-2764-468e-ade6-d18043728338

📥 Commits

Reviewing files that changed from the base of the PR and between 6d58671 and 699c15f.

📒 Files selected for processing (2)

• packages/backend/src/routes/lastfm.ts
• packages/backend/src/routes/spotify.ts

[cubic-dev-ai]

No issues found across 2 files

<sub>Requires human review: Changes involve OAuth callback authentication logic with explicit state validation using zod. Although the refactor improves type safety, it touches security-critical authentication flows where a bug could have severe consequences. Human review is warranted for this risk profile.

Re-trigger cubic</sub>

[cubic-dev-ai]

0 issues found across 2 files (changes from recent commits).

<sub>Auto-approved: Adds explicit zod validation for OAuth state cookies in lastfm and spotify routes, replacing unsafe type casts with safe runtime checks. No behavioral changes, only improved type safety.

Re-trigger cubic</sub>

[cubic-dev-ai]

0 issues found across 2 files (changes from recent commits).

<sub>Auto-approved: Removes unsafe type assertions and adds explicit Zod validation for cookie state in two OAuth callback routes. No logic changes, improves type safety and security with low risk.

Re-trigger cubic</sub>

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
80.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud