fix(security): invalidate user sessions on password change

[aaight]

Summary

• Adds deleteUserSessions(userId, excludeToken?) repository function to usersRepository.ts that deletes all sessions for a given user
• Calls deleteUserSessions from the users.update tRPC mutation after a successful password hash update, ensuring all existing sessions are invalidated
• Adds unit tests for both the new repository function and the router-level session invalidation behavior

Security Fix

When an admin changed a user's password via users.update, all existing sessions for that target user remained valid — allowing continued access with the old credentials. After this fix, all sessions for the target user are deleted immediately after the password is updated.

Reference: https://trello.com/c/eL6bwZFo/573-2-sessions-not-invalidated-on-password-change-src-api-routers-usersts121-when-an-admin-changes-a-users-password-all-existing-ses

Key Decisions

• No excludeToken used in the router: Since the mutation is admin-only and typically targets another user, all of the target user's sessions are invalidated. If an admin changes their own password, they will be logged out — this is correct security behavior.
• Not wrapped in a transaction: Both operations are simple single-table queries; if deleteUserSessions fails after updateUser succeeds, the password is still updated, improving security posture. Sessions will expire naturally.
• excludeToken parameter kept for future use: The repository function supports an optional excludeToken for cases where the caller wants to preserve their own session.

Test plan

• deleteUserSessions unit tests: deletes all sessions for user, handles excludeToken conditional
• Router tests: sessions invalidated when password changes, not called for name/email/role changes
• All existing tests still pass (63 tests in changed files, 89 total in affected projects)
• TypeScript type check passes (zero errors)
• Biome lint passes (complexity warning is pre-existing config set to warn, not error)

🕵️ claude-code · claude-sonnet-4-6 · run details

[aaight]

CI Failures Resolved

Fixes Applied

• Added npm overrides for lodash@^4.18.1, lodash-es@^4.18.1, and brace-expansion@^2.0.3 to resolve high-severity CVEs in transitive dependencies
  • lodash@4.17.23 via archiver@7.0.1 → archiver-utils@5.0.2 — GHSA-r5fr-rjxr-66jc (Code Injection, CVSS 8.1)
  • lodash-es@4.17.23 via @llmist/cli → js-toml → chevrotain — same CVEs
  • brace-expansion@2.0.2 — GHSA-f886-m6hf-6m8v (moderate)

Root Cause

The npm audit --omit=dev --audit-level=high CI step introduced vulnerabilities in transitive dependencies that appeared in the advisory database after the base branch commit. The fix uses npm's overrides field to force patched versions without changing direct dependency versions or introducing breaking changes.

Verification

• npm audit --omit=dev --audit-level=high → found 0 vulnerabilities ✅
• Build passes: npm run build ✅
• TypeScript type check passes: npm run typecheck ✅
• Lint passes: npm run lint ✅
• All 7230 unit tests pass ✅
• Changes pushed to branch ✅

🕵️ claude-code · claude-sonnet-4-6 · run details

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[nhopeatall]

Summary

LGTM — Clean, well-scoped security fix that correctly invalidates user sessions on password change.

What I verified:

• Correctness: deleteUserSessions is called after updateUser succeeds, gated by updates.passwordHash !== undefined — only triggers on actual password changes, not name/email/role updates.
• Repository function: Proper Drizzle ORM usage with and(eq, ne) for the excludeToken branch. The ne import was already present in the file.
• No missed call sites: Password changes only occur in users.update — login.ts only compares passwords. No other mutation path needs this fix.
• Test coverage: Three new router tests (password change → invalidates, name change → no invalidation, email change → no invalidation) plus three repository tests covering both code paths.
• Mock reset: clearMocks: true in vitest config ensures mockDeleteUserSessions is properly reset between tests; the beforeEach re-sets the default resolved value.
• Design decisions: Not wrapping in a transaction is reasonable — if session deletion fails after password update, security posture is still improved (password changed, sessions expire naturally). Invalidating all sessions when admin changes their own password is correct security behavior.
• Package overrides: lodash 4.18.1 and brace-expansion 2.0.3 are valid published versions (verified against npm registry). These are appropriate security dependency bumps.
• CI: All 7 checks passing.

🕵️ claude-code · claude-opus-4-6 · run details

[nhopeatall]

Summary

LGTM — Clean, well-scoped security fix that correctly invalidates all sessions when an admin changes a user's password.

Verified:

• deleteUserSessions is correctly placed in the repository layer alongside existing session operations (deleteSession, deleteExpiredSessions), following the same patterns (uses getDb(), same sessions table, consistent Promise<void> return).
• The router checks updates.passwordHash !== undefined (not input.password), which correctly gates on the actual DB update field, ensuring session invalidation only triggers when a password hash was actually computed and written.
• The excludeToken branch uses proper and(eq(...), ne(...)) composition with operators already imported — no new dependencies introduced.
• The FK schema (sessions.userId → users.id with onDelete: 'cascade') means deleteUser already handles session cleanup on user deletion; this new function addresses the distinct case of password change without user deletion.
• Decision not to wrap in a transaction is sound: if deleteUserSessions fails after updateUser succeeds, the password is still changed (security improved) and sessions expire naturally.
• Tests cover both layers (repository + router) with positive and negative cases.
• All 7 CI checks pass.

The package.json override additions (lodash, brace-expansion) are unrelated to the security fix but are harmless dependency resolution overrides.

🕵️ claude-code · claude-opus-4-6 · run details