chore(deps): security advisory fixes + Better Auth 1.6 session-integrity repair#66
Merged
Merged
Conversation
Ran `npm install` + `npm audit fix` (no --force) to sync the dependency tree and pull patched versions within existing package.json ranges. Reduces `npm audit` findings from 8 to 2 (remaining 2 are a false positive). Fixed advisories: - better-auth 1.4.18 -> 1.6.23 (HIGH: stored XSS via javascript: redirect_uri in oidc-provider/mcp plugins, GHSA-86j7-9j95-vpqj; not directly exploitable here — app uses genericOAuth only) - undici 7.22.0 -> 7.28.0 (HIGH: TLS bypass, header injection, cache poisoning; transitive via jsdom) - vite 7.3.1 -> 8.1.4 (HIGH: Windows fs.deny bypass, NTLMv2 disclosure; dev-server only, transitive via vitest) - esbuild -> 0.28.1 (LOW: Windows dev-server file read) - js-yaml -> 4.3.0 (MODERATE: quadratic-complexity DoS) - @babel/core (LOW: file read via sourceMappingURL) - postcss (direct) -> 8.5.16 (MODERATE: CSS stringify XSS) Also synced stale tree to package.json: next 16.1.6 -> 16.2.10, jsdom 28 -> 29.1.1, tsx -> 4.22.1, @vitejs/plugin-react -> 6.0.2, vitest -> 4.1.6. Remaining 2 moderate findings are the postcss@8.4.31 copy bundled inside Next.js. npm's only offered fix is --force -> next@9.3.3, a breaking downgrade; deliberately NOT applied. Verified: `npm run build` (clean, TS ok), `npm run lint` (clean), `npm run test:run` (272 tests / 20 files passing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Update the /audit-deps command so future runs are handled cleanly: - Add a "sync check FIRST" step (`npm ls | grep invalid`, `npm ci`) since a stale node_modules relative to package.json was the main source of misleading audit results. - Ban `npm audit fix --force` and document the specific trap: it tries to downgrade next to 9.3.3 to "fix" a postcss advisory. - Document the accepted false positive: the 2 moderate postcss findings are the copy bundled inside Next.js, not the app's direct (patched) dep. - Add a major-bump procedure: watch ERESOLVE/invalid peers, verify with build, and revert cleanly (restore package.json + `git checkout` lock + reinstall) on failure. Record TypeScript 7 as blocked (breaks Next 16 build worker; rejected by typescript-eslint peer <6.1.0). - Assess real-world exposure, not just advisory ranges (e.g. better-auth oidc-provider/mcp advisories don't apply — app uses genericOAuth only). - Replace the generic dependency checklist with this repo's actual stack (Better Auth not next-auth; no jsonwebtoken/bcryptjs/Drizzle/AWS SDK). - Add verification of tests (`npm run test:run`) alongside build and lint. - Note the command performs no Ministry Platform writes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
….6 regression) The dependency security audit (720f39d) bumped better-auth 1.4.18 -> 1.6.23. As of 1.6, parseAdditionalUserInputFromProviderProfile strips any user additionalField declared with `input: false` BEFORE the user record is created. Our `userGuid` field (the MP User_GUID / OAuth sub) is populated server-side via mapProfileToUser, so `input: false` silently dropped it: session.user.userGuid became undefined, breaking every MP profile lookup — blank header avatar, non-functional user menu, and userId: null. Verified live: after the fix, a fresh login yields a session with populated userGuid + userId, and the avatar/user menu render correctly. Changes: - src/lib/auth.ts: flip userGuid to input:true; extract config to exported `userAdditionalFields` const with a warning comment explaining why. - src/auth.test.ts: add a regression guard that runs the REAL better-auth parse function (better-auth/db) against the REAL field config. Fails if the flag is flipped back OR if a future better-auth version changes provider- profile field handling. Proven to fail on input:false, pass on input:true. - .claude/references/auth.md: correct the previously-misleading input:false snippet, add a "Better Auth Upgrade Checklist" (manual login smoke test is the only thing that catches OAuth-runtime regressions), and elevate the in-memory-adapter (no database) issue to top refactor priority. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…g the user Before the better-auth 1.6 fix, a session without a userGuid rendered a dead header — the avatar/menu never mounted, so there was no sign-out control and the user was stuck. Add a defensive guard so an unusable session always has an exit, independent of the root cause: - AuthWrapper: when a session exists but has no userGuid, redirect to /session-error (in addition to the existing /signin redirect for no session). - New /session-error page: a minimal recovery screen with a handleSignOut form button, so the user can always sign out and re-authenticate. It lives outside the (web) route group, so it is not wrapped by AuthWrapper and cannot redirect-loop. - auth-wrapper.test.tsx: cover all three guard branches (no session -> /signin, no userGuid -> /session-error, healthy session -> renders children). - auth.md: document the recovery flow and the two new files. Verified: lint clean, 277 tests pass (+4), production build succeeds and emits the /session-error route. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Captures the better-auth 1.4->1.6 breaking change (input:false additionalFields stripped from OAuth provider profiles) and everything needed to bring a repo built on an older MPNext up to the current auth setup: the userGuid input:true fix, the real-library regression guard, the broken-session recovery guard (AuthWrapper -> /session-error), the upgrade checklist, and a Better Auth <-> MP identity mental-model appendix. Matches the structure of the existing port-mp-* playbooks (outcome, background, discovery, phased recipes, verify, done checklist). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves outstanding
npm auditsecurity advisories via lockfile updates, then fixes and hardens the Better Auth 1.6 regression that lockfile bump introduced — which silently broke the app's link to Ministry Platform (blank avatar, dead user menu, no way to sign out).What's here
1. Dependency security fixes (
720f39d,7787b4d)npm install+npm audit fix(no--force); advisories reduced 8 → 2 (remaining 2 are a false positive in Next.js's bundled postcss).audit-depscommand doc with lessons from the run.2. Better Auth 1.6
userGuidregression fix (c9d80d4)The root cause of the broken avatar/menu. As of better-auth 1.6,
parseAdditionalUserInputFromProviderProfilestrips any useradditionalFielddeclared withinput: falsebefore creating the user record. OuruserGuid(MP User_GUID / OAuth sub) is populated server-side viamapProfileToUser, soinput: falsesilently dropped it →session.user.userGuidundefined → every MP profile lookup failed.userGuidtoinput: true; extract to exporteduserAdditionalFields.better-auth/db) against the real config — fails if the flag is flipped back or if a future better-auth version changes provider-profile handling.auth.mdsnippet; added a Better Auth upgrade checklist and elevated the in-memory-adapter risk.3. Broken-session recovery guard (
ebe7a66)Before the fix, a session without
userGuidrendered a dead header with no sign-out control — users were trapped. Defensive net, independent of root cause:AuthWrapper: session with nouserGuid→ redirect to/session-error./session-errorrecovery page with ahandleSignOutform button, placed outside the(web)route group so it can't redirect-loop.auth-wrapper.test.tsxcovers all three guard branches.4. Upgrade playbook (
9d25cd4).claude/playbooks/upgrade-betterauth-1.6-session-integrity.md— brings any repo built on an older MPNext up to the current auth setup, matching the existingport-mp-*playbook structure.Verification
npm run lintclean, 277 tests pass (+5: field-persistence guard + 4 AuthWrapper branches),npm run buildsucceeds and emits/session-error.userGuid+userId, and the avatar/user menu render and function.🤖 Generated with Claude Code