update auth api#19
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis PR implements a complete email-based authentication system with password reset, email verification, and framework-aware auth delivery across Next.js, Nuxt, and SvelteKit. It introduces structured auth results via ChangesAuthentication System & Contracts
Forms System Generics & Sensitive Input Handling
Framework Adapter Updates
Core Auth Delivery & Email Templates
CLI Mail Scaffolding
Framework Blog Apps (Next.js, Nuxt, SvelteKit)
Documentation Updates
Testing Infrastructure
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Browser)
participant App as Auth App<br/>(Next.js/Nuxt/SvelteKit)
participant Auth as `@holo-js/auth`<br/>Runtime
participant Mail as `@holo-js/mail`<br/>Delivery
participant DB as Database
Client->>App: POST /api/register<br/>(email, password, name)
App->>Auth: register(input)
Auth->>DB: Check identifier<br/>Create user<br/>Generate verification token
Auth->>Mail: Send verification email<br/>(token, route, APP_URL)
Mail->>Client: Email with<br/>verification link
Auth->>App: { data: user,<br/>error: null }
App->>Client: 201 JSON<br/>+ session cookie<br/>+ redirectTo: /verify-email
Client->>App: GET /verify-email<br/>?token=...
App->>Client: Verify form page
Client->>App: POST /api/verify-email<br/>(token)
App->>Auth: verification.consume(token)
Auth->>DB: Mark email_verified_at<br/>Invalidate token
Auth->>App: { data: user,<br/>error: null }
App->>Client: 200 JSON<br/>+ redirectTo: /login
Client->>App: POST /api/login<br/>(email, password)
App->>Auth: login(credentials)
Auth->>DB: Verify credentials<br/>Check email verification
Auth->>App: { data: {<br/>user, sessionId,<br/>emailVerificationRequired: false,<br/>cookies },<br/>error: null }
App->>Client: 200 JSON + set-cookie<br/>+ redirectTo: /admin
Client->>App: GET /api/auth/user
App->>Auth: check() + user()
Auth->>App: { authenticated: true,<br/>user: {...},<br/>guard: 'web' }
App->>Client: 200 JSON
Estimated Code Review Effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly Related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/forms/src/client-security.ts (1)
27-32:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard
processbefore readingprocess.env.VITEST.
importSecurityClientModule()accessesprocess.envwithout checking ifprocessexists, which will throwReferenceErrorin browser environments. The server-side helper inpackages/forms/src/security.tsalready uses the correct guard pattern—apply the same pattern here.Suggested fix
async function importSecurityClientModule(): Promise<SecurityClientModule> { - if (process.env.VITEST) { + if (typeof process !== 'undefined' && process.env && process.env.VITEST) { return await import(/* `@vite-ignore` */ '@holo-js/security/client') } return await import('@holo-js/security/client') }🤖 Prompt for 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. In `@packages/forms/src/client-security.ts` around lines 27 - 32, importSecurityClientModule reads process.env.VITEST without guarding for browser contexts, which can raise ReferenceError; update the function (importSecurityClientModule) to first check typeof process !== 'undefined' and that process.env is available before accessing VITEST, and only use the vite-specific dynamic import when that guarded check passes (otherwise fall back to the normal import path), mirroring the guard pattern used in the server-side helper (security.ts).packages/auth/src/runtime.ts (2)
1976-2007:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHydrate the guard context before reading session state.
readGuardSessionState()now depends onbindings.context.getSessionId(guardName), but it never pulls the incoming session cookie into that context first. On a fresh request,impersonation(),stopImpersonating(), and the actor lookup insideimpersonate()can all incorrectly seenulluntil some earlieruser()/check()call happens.Suggested change
async function readGuardSessionState( guardName: string, ): Promise<{ readonly sessionId: string readonly record: AuthSessionRecord readonly payloads: SessionAuthPayloadMap readonly payload: SessionAuthPayload } | null> { const bindings = getRuntimeBindings() + await hydrateGuardContextFromRequest(guardName) const sessionId = bindings.context.getSessionId(guardName) if (!sessionId) { return null }🤖 Prompt for 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. In `@packages/auth/src/runtime.ts` around lines 1976 - 2007, readGuardSessionState currently calls bindings.context.getSessionId(guardName) without first hydrating the guard context from the incoming request, so on fresh requests getSessionId returns null; fix by invoking the context hydration API on bindings.context (e.g., call the existing hydrate/initialize method on bindings.context) before calling bindings.context.getSessionId(guardName) inside readGuardSessionState so the incoming session cookie is loaded into context and subsequent reads (impersonation(), stopImpersonating(), impersonate(), etc.) see the correct session.
2151-2192:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winHydrate and clear cookies in logout even on a fresh request.
logoutForGuard()still reads only the in-memory context. If the request hits/logoutbefore any prior auth call populated that context,sessionIdis empty, the server session is never invalidated, andbuildLogoutCookies()returns no local session/remember-me clears. That makes logout a no-op for the common standalone-endpoint case.Suggested change
async function logoutForGuard(guardName: string): Promise<AuthLogoutResult> { const bindings = getRuntimeBindings() + await hydrateGuardContextFromRequest(guardName) const guard = getGuardConfig(guardName) if (guard.driver === 'token') { bindings.context.setAccessToken?.(guardName) bindings.context.setCachedUser(guardName, null) return Object.freeze({ guard: guardName, cookies: Object.freeze([]), }) } - let clearSessionCookies = false + let clearSessionCookies = !!( + bindings.context.getSessionId(guardName) + || bindings.context.getRememberToken?.(guardName) + ) const sessionId = bindings.context.getSessionId(guardName)🤖 Prompt for 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. In `@packages/auth/src/runtime.ts` around lines 2151 - 2192, logoutForGuard currently only uses the in-memory context and returns early when bindings.context.getSessionId(guardName) is falsy, so logout can be a no-op for fresh requests; change logoutForGuard to attempt to hydrate the sessionId before treating it as absent: if bindings.context.getSessionId(guardName) is falsy, try to extract a session id from the incoming request (e.g., via bindings.context.getCookie('session') or a helper like bindings.context.getSessionIdFromRequest / bindings.session.findSessionForRequest) and set it into the context so the existing logic (bindings.session.read, bindings.session.invalidate, writeExistingSession and buildLogoutCookies) can operate and clear session/remember cookies correctly. Ensure you update the same context via bindings.context.setSessionId(guardName) when you hydrate the id so the rest of the function uses the populated value.
🟡 Minor comments (4)
apps/blog-next/app/login/page.tsx-16-16 (1)
16-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winImport
CSSPropertiestype directly —React.CSSPropertiesis not accessible without importing React.The
satisfies React.CSSPropertiessyntax requires theReactnamespace to be in scope. Withjsx: "react-jsx"and no explicitimport React, this type reference is undefined. The same issue occurs inapps/blog-next/app/forgot-password/page.tsxandapps/blog-next/app/reset-password/page.tsx.Fix
'use client' +import type { CSSProperties } from 'react' import Link from 'next/link' import { useRouter } from 'next/navigation' import { useForm } from '@holo-js/adapter-next/client' import { loginForm } from '@/lib/schemas/auth' const panelStyle = { display: 'grid', ... -} satisfies React.CSSProperties +} satisfies CSSProperties🤖 Prompt for 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. In `@apps/blog-next/app/login/page.tsx` at line 16, The use of "satisfies React.CSSProperties" fails because the React namespace isn't imported; import the CSSProperties type directly and use it instead: add "import type { CSSProperties } from 'react'" at the top of the file(s) and change "satisfies React.CSSProperties" to "satisfies CSSProperties" (repeat the same change in the other occurrences in the ForgotPassword and ResetPassword page components).apps/blog-sveltekit/src/routes/verify-email/+page.svelte-27-47 (1)
27-47:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle resend network/parse failures explicitly.
resendVerificationEmail()currently has nocatch, so fetch/JSON failures can surface as unhandled rejections without settingresendError.🧩 Suggested fix
try { const response = await fetch('/api/verify-email/resend', { method: 'POST', headers: { 'content-type': 'application/json', }, body: JSON.stringify(data.email ? { email: data.email } : {}), }) const payload = await response.json() if (payload?.ok === true) { resendMessage = payload.data?.message ?? 'A fresh verification email has been sent.' return } @@ const message = Array.isArray(payload?.errors?._root) ? payload.errors._root[0] : 'Could not send another verification email.' resendError = typeof message === 'string' ? message : 'Could not send another verification email.' + } catch { + resendError = 'Could not send another verification email.' } finally { resending = false }🤖 Prompt for 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. In `@apps/blog-sveltekit/src/routes/verify-email/`+page.svelte around lines 27 - 47, The resendVerificationEmail flow currently lacks a catch, so network or JSON parse errors can bubble up unhandled; wrap the fetch/JSON logic in a try/catch (keeping the existing finally that sets resending = false) and in the catch set resendError to a user-friendly message (or the caught error.message when available) and optionally log the error; update the block around the fetch/response parsing (the code that assigns payload and checks payload?.ok) to ensure all thrown errors are caught and converted into resendError so the UI shows the failure instead of letting the rejection propagate.apps/docs/docs/auth/local-auth.md-447-455 (1)
447-455:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRoute example should include forwarding auth cookies.
The docs correctly describe that
login()changes authenticated state and emits cookies (lines 216-217), yet the Route Integration example omitssession.cookiesfrom the response. Developers copying this example would execute successful login logic but fail to establish the authenticated session because the browser would not receive the session cookies.The returned
sessionobject includes acookiesarray (containing serialized Set-Cookie headers) that must be appended to the HTTP response for the session to be recognized on subsequent requests.📘 Suggested doc patch
+ const headers = new Headers() + for (const cookie of session.cookies) { + headers.append('set-cookie', cookie) + } + return Response.json({ ok: true, data: { redirectTo: session.emailVerificationRequired ? session.emailVerificationRoute ?? '/verify-email' : '/admin', }, - }) + }, { headers })🤖 Prompt for 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. In `@apps/docs/docs/auth/local-auth.md` around lines 447 - 455, The Route example returns Response.json(...) but omits forwarding the authentication Set-Cookie headers, so the browser never receives the session; update the Response.json call that returns redirectTo to include the session.cookies as Set-Cookie headers (use the Response/Headers API to add each value from session.cookies to the response headers) so the emitted cookies from login() are sent to the client; specifically modify the Response.json invocation that references session.emailVerificationRequired / session.emailVerificationRoute to also attach session.cookies as response headers.apps/blog-next/.env.example-15-27 (1)
15-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winResolve dotenv key-order warnings in the new env block.
Line 15 through Line 27 currently trigger
dotenv-linterUnorderedKeywarnings. Reordering these keys will keep lint output clean and avoid potential CI friction.Suggested reorder
-MAIL_MAILER= MAIL_FROM_ADDRESS= MAIL_FROM_NAME= -MAIL_LOG_BODIES= MAIL_HOST= -MAIL_PORT= -MAIL_SECURE= -MAIL_USERNAME= +MAIL_LOG_BODIES= +MAIL_MAILER= MAIL_PASSWORD= +MAIL_PORT= +MAIL_SECURE= +MAIL_USERNAME= -AUTH_SOCIAL_ENCRYPTION_KEY= AUTH_EMAIL_VERIFICATION_ROUTE=/verify-email AUTH_PASSWORD_RESET_ROUTE=/reset-password +AUTH_SOCIAL_ENCRYPTION_KEY=🤖 Prompt for 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. In `@apps/blog-next/.env.example` around lines 15 - 27, Reorder the env keys in the new block so they satisfy dotenv-linter's UnorderedKey rule: sort the variables (AUTH_SOCIAL_ENCRYPTION_KEY, AUTH_EMAIL_VERIFICATION_ROUTE, AUTH_PASSWORD_RESET_ROUTE, MAIL_MAILER, MAIL_FROM_ADDRESS, MAIL_FROM_NAME, MAIL_LOG_BODIES, MAIL_HOST, MAIL_PORT, MAIL_SECURE, MAIL_USERNAME, MAIL_PASSWORD) into a consistent alphabetical/grouped order (or match the project's existing env ordering) so the dotenv-linter warnings disappear; update the block containing those MAIL_* and AUTH_* keys accordingly.
🧹 Nitpick comments (14)
packages/cli/src/project/scaffold/framework-renderers.ts (1)
251-251: ⚡ Quick winRemove misleading
_storageEnabledparameter fromrenderSvelteViteConfig.Line 251 still accepts
_storageEnabled, but the function now builds a fixed externals list. Keeping the arg suggests conditional behavior that no longer exists and makes the call site harder to reason about.Proposed cleanup
-function renderSvelteViteConfig(_storageEnabled: boolean): string { +function renderSvelteViteConfig(): string {// outside this changed range, update the call site: { path: 'vite.config.ts', contents: renderSvelteViteConfig() },🤖 Prompt for 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. In `@packages/cli/src/project/scaffold/framework-renderers.ts` at line 251, The function renderSvelteViteConfig currently declares an unused parameter _storageEnabled; remove that parameter from the function signature and from any internal references so the function is defined as renderSvelteViteConfig(), then update all call sites (e.g., where { path: 'vite.config.ts', contents: renderSvelteViteConfig() } is constructed) to invoke it without an argument to reflect the fixed externals list.packages/security/src/client.ts (2)
8-10: 💤 Low valueMinor: field name
bindingsholds a normalizedSecurityClientConfig, notSecurityClientBindings.
RuntimeSecurityClientState.bindingsis typed asSecurityClientConfigand stores the post-normalization result, whileconfigureSecurityClientaccepts the rawSecurityClientBindings. The mismatch in naming is a small readability nit — consider renaming toconfig(or storing the raw bindings and normalizing on read) for clarity.Suggested rename
type RuntimeSecurityClientState = { - bindings?: SecurityClientConfig + config?: SecurityClientConfig }…with corresponding updates in
configureSecurityClient,getSecurityClientConfig, andresetSecurityClient.🤖 Prompt for 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. In `@packages/security/src/client.ts` around lines 8 - 10, RuntimeSecurityClientState.bindings currently holds the normalized SecurityClientConfig but is named `bindings`, which is misleading; rename the field to `config` (or alternatively store the raw SecurityClientBindings and normalize on access) and update all usages accordingly — specifically update the type RuntimeSecurityClientState, the functions configureSecurityClient, getSecurityClientConfig, and resetSecurityClient to read/write `config` (typed as SecurityClientConfig) so the name matches the stored value and keep normalization logic in configureSecurityClient if you choose the config approach.
12-17: 💤 Low valueConsider importing CSRF defaults from the shared constants to prevent silent drift.
Client-side CSRF defaults currently match the server-side defaults (
field: '_token',cookie: 'XSRF-TOKEN'). However, these values are hardcoded rather than imported from the shared constants defined inpackages/config/src/defaults.ts(DEFAULT_SECURITY_CSRF_FIELD, DEFAULT_SECURITY_CSRF_COOKIE). If server-side defaults are ever updated, the client will not automatically follow, creating a maintenance risk. Re-export or import these constants from the shared module to keep both in sync.🤖 Prompt for 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. In `@packages/security/src/client.ts` around lines 12 - 17, The DEFAULT_SECURITY_CLIENT_CONFIG currently hardcodes csrf.field and csrf.cookie; update it to import and use the shared DEFAULT_SECURITY_CSRF_FIELD and DEFAULT_SECURITY_CSRF_COOKIE constants (instead of literal '_token' and 'XSRF-TOKEN') so the client config stays in sync with server defaults—modify the DEFAULT_SECURITY_CLIENT_CONFIG definition to reference those imported constants for the csrf.field and csrf.cookie values.packages/adapter-sveltekit/package.json (1)
38-38: ⚡ Quick win
svelteshould be inpeerDependencies, notdependencies.Listing a framework as a direct
dependencyof an adapter library can cause duplicate svelte instances in consuming projects. Svelte's reactivity primitives (stores, context) are singleton-sensitive — two copies of svelte in the same process produce subtle, hard-to-debug breakage. Note that@holo-js/formscorrectly follows thepeerDependenciespattern already.♻️ Proposed fix
"dependencies": { "@holo-js/config": "^0.1.4", - "@holo-js/core": "^0.1.4", - "svelte": "catalog:" + "@holo-js/core": "^0.1.4" }, "peerDependencies": { - "@holo-js/forms": "^0.1.4" + "@holo-js/forms": "^0.1.4", + "svelte": "catalog:" }, "peerDependenciesMeta": { "@holo-js/forms": { "optional": true - } + }, + "svelte": { + "optional": false + } }, "devDependencies": { "@types/node": "^22.10.2", + "svelte": "catalog:", "tsup": "^8.3.5",🤖 Prompt for 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. In `@packages/adapter-sveltekit/package.json` at line 38, The package.json currently lists "svelte": "catalog:" under dependencies; remove that entry from "dependencies" and add the same "svelte": "catalog:" entry under "peerDependencies" (optionally also add it under "devDependencies" if the package needs Svelte for local tests/builds). Update package.json's dependency sections so the adapter does not bundle its own Svelte and consumers provide the Svelte version as a peer dependency.apps/blog-nuxt/pages/register.vue (1)
29-56: ⚡ Quick winAdd
autocompleteattributes so password managers handle registration correctly.Without explicit
autocompletehints, password managers will not reliably treat the password fields as a new-account flow and may auto-fill the wrong credential set. Addautocomplete="name",autocomplete="email", andautocomplete="new-password"on both password inputs so registration form filling/saving works as expected.♻️ Suggested change
- <input name="name" v-model="form.fields.name.value" `@blur`="form.fields.name.onBlur()" /> + <input name="name" autocomplete="name" v-model="form.fields.name.value" `@blur`="form.fields.name.onBlur()" /> @@ - <input name="email" type="email" v-model="form.fields.email.value" `@blur`="form.fields.email.onBlur()" /> + <input name="email" type="email" autocomplete="email" v-model="form.fields.email.value" `@blur`="form.fields.email.onBlur()" /> @@ - <input name="password" type="password" v-model="form.fields.password.value" `@blur`="form.fields.password.onBlur()" /> + <input name="password" type="password" autocomplete="new-password" v-model="form.fields.password.value" `@blur`="form.fields.password.onBlur()" /> @@ - <input - name="passwordConfirmation" - type="password" - v-model="form.fields.passwordConfirmation.value" - `@blur`="form.fields.passwordConfirmation.onBlur()" - /> + <input + name="passwordConfirmation" + type="password" + autocomplete="new-password" + v-model="form.fields.passwordConfirmation.value" + `@blur`="form.fields.passwordConfirmation.onBlur()" + />🤖 Prompt for 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. In `@apps/blog-nuxt/pages/register.vue` around lines 29 - 56, The form inputs lack autocomplete attributes which prevents password managers from recognizing this as a new-account flow; update the input elements bound to form.fields.name.value, form.fields.email.value, form.fields.password.value, and form.fields.passwordConfirmation.value to include autocomplete="name" for the name input, autocomplete="email" for the email input, and autocomplete="new-password" on both the password and passwordConfirmation inputs so password managers correctly offer to save new credentials.packages/forms/src/client.ts (2)
837-839: 💤 Low valueConfirm intent: successful local-only submissions clear
lastSubmission.When
applyServerStatereceives aFormSubmissionResultwhosevalidis true (the fallthrough branch),state.lastSubmissionis reset toundefined. This matches the updated tests inpackages/forms/tests/client.test.ts(lastSubmissionundefined post-validate / post-failed-submit), but it means consumers can no longer observe a successful local submission fromlastSubmission. Worth a short doc/JSDoc note onlastSubmissioninUseFormResultso consumers know it only reflects server failures orok: true/falsepayloads, not local validation success.🤖 Prompt for 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. In `@packages/forms/src/client.ts` around lines 837 - 839, The code resets state.lastSubmission to undefined on successful local validation in applyServerState (setting state.lastSubmission = normalizedSubmission.valid ? undefined : normalizedSubmission.fail()), which hides successful local-only submissions; add a concise JSDoc on UseFormResult.lastSubmission explaining it only reflects server-side failures or payloads with ok:true/false and does not record successful local-only validation, and update any relevant tests/comments to make this intended behavior explicit; reference applyServerState, state.lastSubmission, UseFormResult, and FormSubmissionResult so reviewers can locate where the behavior is enforced and where the doc should live.
118-225: 🏗️ Heavy liftDuplicated submission helpers between
client.tsandcontracts.ts.
normalizeStatus,serializeSubmissionState,createSubmission,createSuccessfulSubmission, andcreateFailedSubmissionare now defined twice (here and inpackages/forms/src/contracts.ts) with verbatim semantics for status validation and submission-result construction. Two copies will silently diverge — e.g., a future change to status validation or to the frozen result shape only in contracts.ts would not propagate here, breaking client/server consistency that the rest of the pipeline relies on.Consider extracting these into a shared internal module (e.g.,
packages/forms/src/internal/submission.ts) and importing from bothclient.tsandcontracts.ts. As a minor follow-up, the localcreateSuccessfulSubmission/createFailedSubmissionkeep an unusedschemaDefinitionparameter (void schemaDefinition) — drop it in the internal-only signatures since they don't need API parity with the public re-exports.🤖 Prompt for 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. In `@packages/forms/src/client.ts` around lines 118 - 225, The duplicated submission helpers (normalizeStatus, serializeSubmissionState, createSubmission, createSuccessfulSubmission, createFailedSubmission) in client.ts and contracts.ts should be moved into a single internal module (e.g., packages/forms/src/internal/submission.ts) and both files should import from it; create that module exporting normalizeStatus, serializeSubmissionState, createSubmission, and internal versions of createSuccessfulSubmission/createFailedSubmission that drop the unused schemaDefinition parameter, preserve existing behavior (status validation via normalizeStatus, createErrorBag usage, Object.freeze shapes, and fail/success helpers), and update client.ts and contracts.ts to import and use these exports so there is one authoritative implementation and no silent divergence.apps/blog-nuxt/pages/verify-email.vue (1)
21-45: Operational note: ensure the resend endpoint enforces rate limiting / throttling.The client allows the user to repeatedly click "Resend verification email"; the only client-side gate is the in-flight
resendingflag. If/api/verify-email/resenddoesn't apply a server-side throttle (similar to thethrottle: 'login'/throttle: 'register'patterns on the other endpoints), this becomes an easy lever for email-bomb / enumeration abuse. Worth confirming the resend route is wrapped with the same throttle/abuse-protection mechanism, and surfacing a cooldown message client-side when the server signals 429.🤖 Prompt for 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. In `@apps/blog-nuxt/pages/verify-email.vue` around lines 21 - 45, Client allows repeated "Resend" clicks and lacks handling for server-side rate limits; confirm the backend route handling POST /api/verify-email/resend is wrapped with the app's throttle/abuse-protection (same pattern used for 'throttle: "login"' / 'throttle: "register"') and, in resendVerificationEmail(), handle HTTP 429 responses by setting resendError.value to a cooldown-specific message and optionally disabling UI for the returned retry-after duration using the existing resending flag; reference the resendVerificationEmail function, resending and resendError reactive vars, and the /api/verify-email/resend endpoint when applying fixes.apps/blog-nuxt/server/api/login.post.ts (1)
29-29: ⚡ Quick winUse
appendResponseHeaderto safely append session cookies without clobbering upstream middleware.
event.node.res.setHeader('set-cookie', [...session.cookies])writes directly to Node's response and overwrites any priorSet-Cookieheaders (e.g., CSRF/session cookies from middleware). Prefer h3'sappendResponseHeaderso each session cookie is appended rather than replacing the entire Set-Cookie header array. This applies to login.post.ts, register.post.ts, and logout.post.ts.♻️ Suggested change
- event.node.res.setHeader('set-cookie', [...session.cookies]) + for (const cookie of session.cookies) { + appendResponseHeader(event, 'set-cookie', cookie) + }🤖 Prompt for 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. In `@apps/blog-nuxt/server/api/login.post.ts` at line 29, Replace the direct Node header write that clobbers existing Set-Cookie values by iterating session.cookies and calling h3's appendResponseHeader(event, 'set-cookie', cookie) for each cookie instead of event.node.res.setHeader('set-cookie', [...session.cookies]); update login.post.ts (and the same change in register.post.ts and logout.post.ts) to import appendResponseHeader from 'h3', remove the setHeader call, and append each value from session.cookies using appendResponseHeader(event, 'set-cookie', cookie) so upstream middleware cookies are preserved.apps/blog-sveltekit/src/routes/api/auth/user/+server.ts (1)
4-9: ⚡ Quick winResolve the current user once and derive
authenticatedfrom it.
check()already performs an auth lookup, so callingcheck()anduser()here does the session/token work twice. That can double-touch the session on every request to this endpoint.Suggested change
export async function GET() { + const currentUser = await user() return json({ - authenticated: await check(), + authenticated: currentUser !== null, guard: 'web', - user: await user(), + user: currentUser, }) }🤖 Prompt for 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. In `@apps/blog-sveltekit/src/routes/api/auth/user/`+server.ts around lines 4 - 9, Call the current user once and derive authenticated from that result: in GET(), await user() a single time into a variable (e.g., currentUser) and return authenticated: !!currentUser (or Boolean(currentUser)), guard: 'web', and user: currentUser instead of calling check() and user() separately so you don't perform the auth lookup twice; update references in the GET function accordingly (replace usages of check() and user() with the single currentUser variable).packages/adapter-nuxt/tests/module.test.ts (1)
1037-1041: ⚡ Quick winKeep the looser matcher, but assert
authRequestis passed.These changes correctly avoid brittle exact-object matching, but they now miss a guard for the new
authRequestplumbing introduced in this PR.Proposed test hardening
- expect(initializeHoloAdapterProject).toHaveBeenCalledWith('/tmp/nuxt-project', expect.objectContaining({ + expect(initializeHoloAdapterProject).toHaveBeenCalledWith('/tmp/nuxt-project', expect.objectContaining({ envName: 'test', preferCache: true, processEnv: process.env, + authRequest: expect.any(Object), })) - expect(initializeHoloAdapterProject).toHaveBeenCalledWith('/tmp/nuxt-project', expect.objectContaining({ + expect(initializeHoloAdapterProject).toHaveBeenCalledWith('/tmp/nuxt-project', expect.objectContaining({ envName: 'development', preferCache: false, processEnv: process.env, + authRequest: expect.any(Object), }))Also applies to: 1180-1184
🤖 Prompt for 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. In `@packages/adapter-nuxt/tests/module.test.ts` around lines 1037 - 1041, Keep the loose expect.objectContaining matcher but assert the new authRequest plumbing is passed: update the two assertions that call initializeHoloAdapterProject to include authRequest (e.g., expect.objectContaining({ envName: 'test', preferCache: true, processEnv: process.env, authRequest: expect.any(Function) })) so the test verifies authRequest is forwarded; apply the same change for the second occurrence that mirrors lines 1180-1184.apps/blog-next/app/login/page.tsx (1)
43-61: ⚡ Quick winAdd
autocompleteattributes to the email and password inputs.Without
autocomplete, password managers and browser autofill cannot reliably pre-fill credentials, which degrades UX on the most commonly visited auth page.✏️ Proposed fix
<input name="email" type="email" + autoComplete="email" value={form.fields.email.value} ... /> ... <input name="password" type="password" + autoComplete="current-password" value={form.fields.password.value} ... />🤖 Prompt for 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. In `@apps/blog-next/app/login/page.tsx` around lines 43 - 61, The email and password inputs (the elements with name="email" and name="password", using form.fields.email and form.fields.password) lack autocomplete attributes; add autocomplete="email" to the email input and autocomplete="current-password" to the password input (keeping the existing value, onInput and onBlur handlers intact) so browsers and password managers can reliably autofill credentials.apps/blog-sveltekit/src/routes/register/+page.svelte (1)
26-26: ⚡ Quick winMigrate
on:submitandon:blurto Svelte 5 event handler properties (onsubmit/onblur).The project uses Svelte 5.55.5, where
on:directive syntax is deprecated. All five instances in this file should migrate to the new event handler properties: oneon:submit(line 26) and fouron:blurdirectives (lines 29, 37, 45, 57). This removes deprecation warnings and ensures compatibility with future Svelte versions.Proposed migration
- <form class="stack" on:submit={(event) => { event.preventDefault(); form.submit() }}> + <form class="stack" onsubmit={(event) => { event.preventDefault(); form.submit() }}> <label class="field"> <span>Name</span> - <input name="name" bind:value={form.fields.name.value} on:blur={() => form.fields.name.onBlur()} /> + <input name="name" bind:value={form.fields.name.value} onblur={() => form.fields.name.onBlur()} />🤖 Prompt for 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. In `@apps/blog-sveltekit/src/routes/register/`+page.svelte at line 26, Replace deprecated Svelte "on:" directives with Svelte 5 event handler properties: change the form's on:submit handler to an onsubmit property that prevents default and calls form.submit(), and convert all four on:blur attributes on the input fields to onblur properties that call their existing blur handlers; ensure the handler bodies remain identical (e.g., event => { event.preventDefault(); form.submit() } becomes onsubmit={event => { event.preventDefault(); form.submit(); }} and each on:blur={...} becomes onblur={...}) so behavior is preserved and deprecation warnings are removed.tests/example-app-auth-flow.mjs (1)
112-134: ⚡ Quick winMake token wait timeout configurable at the helper boundary.
Line 112 hardcodes a 10s default, and all calls in this helper use that default. Exposing a timeout option from
assertExampleAppAuthFlowwill reduce CI flakiness under slower environments.Suggested change
export async function assertExampleAppAuthFlow({ baseUrl, getOutput, appName, + tokenWaitTimeoutMs = 10000, }) { @@ const verificationToken = ( await waitForOutputMatch( getOutput, authTokenPattern, registerOutputStart, + tokenWaitTimeoutMs, ) )[1] @@ const resentVerificationToken = ( await waitForOutputMatch( getOutput, authTokenPattern, resendOutputStart, + tokenWaitTimeoutMs, ) )[1] @@ const resetTokenMatch = await waitForOutputMatch( getOutput, authTokenPattern, outputStart, + tokenWaitTimeoutMs, )🤖 Prompt for 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. In `@tests/example-app-auth-flow.mjs` around lines 112 - 134, The helper hardcodes waitForOutputMatch's timeout (10000ms) making tests flaky; add a timeout option to assertExampleAppAuthFlow (e.g., authTimeoutMs) and pass it into waitForOutputMatch instead of the hardcoded default, keeping 10000ms as the default value; update any internal calls that use waitForOutputMatch (refer to waitForOutputMatch, assertExampleAppAuthFlow, authTokenPattern) to use the new authTimeoutMs parameter so callers can override the token wait timeout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c0f1beaf-afd0-4f1e-9ba0-55be8755592e
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (138)
apps/blog-next/.env.exampleapps/blog-next/app/admin/page.tsxapps/blog-next/app/api/auth/user/route.tsapps/blog-next/app/api/forgot-password/route.tsapps/blog-next/app/api/login/route.tsapps/blog-next/app/api/logout/route.tsapps/blog-next/app/api/register/route.tsapps/blog-next/app/api/reset-password/route.tsapps/blog-next/app/api/verify-email/resend/route.tsapps/blog-next/app/api/verify-email/route.tsapps/blog-next/app/forgot-password/page.tsxapps/blog-next/app/layout.tsxapps/blog-next/app/login/page.tsxapps/blog-next/app/register/page.tsxapps/blog-next/app/reset-password/page.tsxapps/blog-next/app/verify-email/page.tsxapps/blog-next/config/auth.tsapps/blog-next/config/mail.tsapps/blog-next/config/session.tsapps/blog-next/lib/schemas/auth.tsapps/blog-next/package.jsonapps/blog-next/server/models/User.tsapps/blog-next/tests/run.mjsapps/blog-next/tsconfig.jsonapps/blog-nuxt/.env.exampleapps/blog-nuxt/app.vueapps/blog-nuxt/config/auth.tsapps/blog-nuxt/config/mail.tsapps/blog-nuxt/lib/schemas/auth.tsapps/blog-nuxt/nuxt.config.tsapps/blog-nuxt/package.jsonapps/blog-nuxt/pages/forgot-password.vueapps/blog-nuxt/pages/login.vueapps/blog-nuxt/pages/register.vueapps/blog-nuxt/pages/reset-password.vueapps/blog-nuxt/pages/verify-email.vueapps/blog-nuxt/server/api/auth/user.get.tsapps/blog-nuxt/server/api/forgot-password.post.tsapps/blog-nuxt/server/api/login.post.tsapps/blog-nuxt/server/api/logout.post.tsapps/blog-nuxt/server/api/register.post.tsapps/blog-nuxt/server/api/reset-password.post.tsapps/blog-nuxt/server/api/verify-email.post.tsapps/blog-nuxt/server/api/verify-email/resend.post.tsapps/blog-nuxt/server/models/User.tsapps/blog-nuxt/tests/run.mjsapps/blog-sveltekit/.env.exampleapps/blog-sveltekit/config/auth.tsapps/blog-sveltekit/config/mail.tsapps/blog-sveltekit/config/session.tsapps/blog-sveltekit/package.jsonapps/blog-sveltekit/server/models/User.tsapps/blog-sveltekit/src/lib/schemas/auth.tsapps/blog-sveltekit/src/routes/+layout.svelteapps/blog-sveltekit/src/routes/api/auth/user/+server.tsapps/blog-sveltekit/src/routes/api/forgot-password/+server.tsapps/blog-sveltekit/src/routes/api/login/+server.tsapps/blog-sveltekit/src/routes/api/logout/+server.tsapps/blog-sveltekit/src/routes/api/register/+server.tsapps/blog-sveltekit/src/routes/api/reset-password/+server.tsapps/blog-sveltekit/src/routes/api/verify-email/+server.tsapps/blog-sveltekit/src/routes/api/verify-email/resend/+server.tsapps/blog-sveltekit/src/routes/forgot-password/+page.svelteapps/blog-sveltekit/src/routes/login/+page.svelteapps/blog-sveltekit/src/routes/register/+page.svelteapps/blog-sveltekit/src/routes/reset-password/+page.server.tsapps/blog-sveltekit/src/routes/reset-password/+page.svelteapps/blog-sveltekit/src/routes/verify-email/+page.server.tsapps/blog-sveltekit/src/routes/verify-email/+page.svelteapps/blog-sveltekit/tests/run.mjsapps/blog-sveltekit/vite.config.tsapps/docs/docs/auth/current-auth-client.mdapps/docs/docs/auth/email-verification.mdapps/docs/docs/auth/index.mdapps/docs/docs/auth/local-auth.mdapps/docs/docs/auth/password-reset.mdapps/docs/docs/security.mdpackage.jsonpackages/adapter-next/src/client.tspackages/adapter-next/src/next-headers-shim.d.tspackages/adapter-next/src/runtime.tspackages/adapter-next/tsup.config.tspackages/adapter-nuxt/package.jsonpackages/adapter-nuxt/src/module.tspackages/adapter-nuxt/src/runtime/composables/forms.tspackages/adapter-nuxt/src/runtime/composables/index.tspackages/adapter-nuxt/src/runtime/plugins/init.tspackages/adapter-nuxt/src/runtime/shims.d.tspackages/adapter-nuxt/tests/module.test.tspackages/adapter-nuxt/tsconfig.jsonpackages/adapter-sveltekit/package.jsonpackages/adapter-sveltekit/src/client.tspackages/adapter-sveltekit/src/index.tspackages/auth/src/client-runtime.tspackages/auth/src/client.tspackages/auth/src/contracts.tspackages/auth/src/index.tspackages/auth/src/runtime.tspackages/auth/tests/contracts.type.test.tspackages/auth/tests/docs-smoke.test.tspackages/auth/tests/package.test.tspackages/cli/src/cli.tspackages/cli/src/project/scaffold.tspackages/cli/src/project/scaffold/config-renderers.tspackages/cli/src/project/scaffold/framework-renderers.tspackages/cli/src/project/scaffold/project-renderers.tspackages/cli/src/project/shared.tspackages/cli/tests/cli.test.tspackages/cli/tests/mail-scaffold.test.tspackages/config/src/defaults.tspackages/config/src/types.tspackages/core/src/adapter.tspackages/core/src/portable/holo.tspackages/core/tests/adapter.test.tspackages/core/tests/auth-runtime.test.tspackages/core/tests/runtime.test.tspackages/forms/package.jsonpackages/forms/src/client-security.tspackages/forms/src/client.tspackages/forms/src/contracts.tspackages/forms/src/errors.tspackages/forms/src/index.tspackages/forms/src/schema.tspackages/forms/src/security-shared.tspackages/forms/src/security.tspackages/forms/tests/client.test.tspackages/forms/tests/client.type.test.tspackages/forms/tests/contracts.test.tspackages/forms/tests/security.test.tspackages/forms/tsup.config.tspackages/mail/src/runtime.tspackages/mail/tests/docs-smoke.test.tspackages/mail/tests/runtime.test.tspackages/notifications/tests/docs-smoke.test.tspackages/security/src/client.tspackages/validation/src/contracts-types.tspackages/validation/src/index.tstests/example-app-auth-flow.mjs
💤 Files with no reviewable changes (1)
- apps/blog-nuxt/nuxt.config.ts
Comment on lines
+4040
to
+4043
| const baseAuthContext = authModule.createAsyncAuthContext() | ||
| authContext = options.authRequest | ||
| ? attachAuthRequestAccessors(baseAuthContext, options.authRequest) | ||
| : baseAuthContext |
There was a problem hiding this comment.
Don't make authRequest sticky across the process.
These accessors are only sourced during the first initialization, but initializeHolo(...) reuses the cached runtime afterward. If a consumer passes closures over a concrete request, later requests will keep reading the first request's cookies/headers, which turns this API into a cross-request auth leak. This needs a per-request resolver or a refresh path when the runtime already exists.
Also applies to: 4236-4239
🤖 Prompt for 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.
In `@packages/core/src/portable/holo.ts` around lines 4040 - 4043, The current
code binds a concrete request into authContext by calling
authModule.createAsyncAuthContext() and then
attachAuthRequestAccessors(baseAuthContext, options.authRequest), which makes
authRequest sticky across subsequent initializeHolo calls; instead, make auth
resolution per-request by storing a resolver/factory (e.g., keep baseAuthContext
and, if options.authRequest exists, wrap accessors in a lazy resolver or
recreate/refresh authContext on each initialize) so the call sites
(initializeHolo, and the code paths using baseAuthContext / authContext created
via authModule.createAsyncAuthContext and attachAuthRequestAccessors) never
close over a single request; update the logic used at the other occurrence
(lines ~4236-4239) similarly to avoid cross-request cookies/headers leakage.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/forms/src/contracts.ts (1)
352-408:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNested
req.headersobjects can still hijack normal form validation.
isStructuredRequestLikeObject()relies onisRequestLikeHeaders(), and that helper still accepts any object. Inputs like{ req: { headers: { email: 'ava@example.com' } } }will normalize into a synthetic emptyRequestinstead of validating the original payload.Suggested hardening
function isRequestLikeHeaders(value: unknown): value is RequestLikeHeaders { - return value instanceof Headers || isHeadersTupleArray(value) || (!!value && typeof value === 'object') + if (value instanceof Headers || isHeadersTupleArray(value)) { + return true + } + + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return false + } + + return Object.values(value).every(entry => + typeof entry === 'string' + || typeof entry === 'undefined' + || (Array.isArray(entry) && entry.every(item => typeof item === 'string')) + ) }🤖 Prompt for 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. In `@packages/forms/src/contracts.ts` around lines 352 - 408, isStructuredRequestLikeObject/isRequestLikeInput are treating any plain object as headers because isRequestLikeHeaders is too permissive; this lets nested payloads like { req: { headers: { email: 'x' } } } masquerade as a Request. Fix by tightening isRequestLikeHeaders to only accept real header-like values (e.g., instanceof Headers or objects exposing header methods like get/forEach/entries) and/or explicitly reject plain POJOs (Object.getPrototypeOf(headers) === Object.prototype). Update isStructuredRequestLikeObject to call the stricter isRequestLikeHeaders and ensure checks in isRequestLikeInput (candidate.req, candidate.web?.request, candidate.node?.req) use that tightened validation so plain nested header objects no longer trigger Request normalization.packages/core/src/portable/holo.ts (1)
4086-4089:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftDon't bind request-scoped auth accessors into the cached runtime.
Lines 4086-4089 create
authContextonly once, butinitializeHolo()reuses the same runtime afterward. If a caller passes closures over a concrete request here, later requests will keep reading the first request's cookies/headers. This needs a per-request resolver, or a refresh path when reusing an existing runtime.🤖 Prompt for 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. In `@packages/core/src/portable/holo.ts` around lines 4086 - 4089, The code creates a single authContext by calling authModule.createAsyncAuthContext() and conditionally wrapping it with attachAuthRequestAccessors using options.authRequest, which binds request-scoped accessors into a cached runtime (see baseAuthContext, authContext, attachAuthRequestAccessors, initializeHolo); change this so the runtime does not capture a concrete request: either create auth accessors per incoming request (resolve attachAuthRequestAccessors at request time) or add a refresh/resolver path on the cached runtime that accepts the current options.authRequest and returns a fresh request-bound context; update initializeHolo to use the per-request resolver instead of reusing the single authContext.
🤖 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/auth/src/runtime.ts`:
- Around line 2216-2223: The code currently persists the user via
serializeUser(adapter, user, guard.provider) and then calls
createEmailVerificationFacade().create(...); if that call fails the user remains
in the DB. Make verification delivery non-fatal by wrapping the
createEmailVerificationFacade().create(...) call in error handling that
compensates on failure: if create(...) throws, delete the just-created user via
the adapter (use the adapter instance and the persisted user's id from
serialized) or otherwise roll back the created account/token so registration
does not leave a dangling user; alternatively consider creating the verification
token before final persistence and only persisting the user after verification
token creation succeeds. Ensure you reference serializeUser,
createEmailVerificationFacade().create, adapter and defaultGuard when
implementing the fix.
- Around line 933-941: The remember-me token is being read into bindings.context
via getRememberToken/setRememberToken but never used to rehydrate a session, so
requests with only the remember cookie remain signed out; after you obtain
rememberToken (inside the block that uses parseSetCookieDefinition,
resolveRequestCookie and setRememberToken) call the session restore API on
bindings.session to create/restore a session from that token and then populate
the context/session state (e.g., call a restore method such as
bindings.session.restoreFromRememberToken(rememberToken) or
bindings.session.createFromRememberToken(rememberToken), then set the resulting
session id or user on the context using bindings.context.setSession?.(guardName,
sessionId) or bindings.context.setUser?.(guardName, user)). If your session
implementation exposes a different method name, invoke that method and ensure
you set the session id/user into bindings.context so subsequent session-user
code no longer returns null.
In `@packages/forms/src/sensitiveInput.ts`:
- Around line 11-15: The denylist currently includes reset/verification
transport tokens ('token', 'verification_code', 'verificationCode',
'verification_token', 'verificationToken'), which breaks resubmission of
reset/verify flows; remove these entries from the default denylist array in
packages/forms/src/sensitiveInput.ts (the array that lists sensitive input keys)
so transport tokens are preserved across round-trips, leaving other truly
sensitive keys intact.
---
Duplicate comments:
In `@packages/core/src/portable/holo.ts`:
- Around line 4086-4089: The code creates a single authContext by calling
authModule.createAsyncAuthContext() and conditionally wrapping it with
attachAuthRequestAccessors using options.authRequest, which binds request-scoped
accessors into a cached runtime (see baseAuthContext, authContext,
attachAuthRequestAccessors, initializeHolo); change this so the runtime does not
capture a concrete request: either create auth accessors per incoming request
(resolve attachAuthRequestAccessors at request time) or add a refresh/resolver
path on the cached runtime that accepts the current options.authRequest and
returns a fresh request-bound context; update initializeHolo to use the
per-request resolver instead of reusing the single authContext.
In `@packages/forms/src/contracts.ts`:
- Around line 352-408: isStructuredRequestLikeObject/isRequestLikeInput are
treating any plain object as headers because isRequestLikeHeaders is too
permissive; this lets nested payloads like { req: { headers: { email: 'x' } } }
masquerade as a Request. Fix by tightening isRequestLikeHeaders to only accept
real header-like values (e.g., instanceof Headers or objects exposing header
methods like get/forEach/entries) and/or explicitly reject plain POJOs
(Object.getPrototypeOf(headers) === Object.prototype). Update
isStructuredRequestLikeObject to call the stricter isRequestLikeHeaders and
ensure checks in isRequestLikeInput (candidate.req, candidate.web?.request,
candidate.node?.req) use that tightened validation so plain nested header
objects no longer trigger Request normalization.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 74439413-5cde-4ccc-9b83-6ccf584b9bbc
📒 Files selected for processing (42)
apps/blog-next/app/api/forgot-password/route.tsapps/blog-next/app/api/login/route.tsapps/blog-next/app/api/register/route.tsapps/blog-next/app/api/reset-password/route.tsapps/blog-next/app/api/verify-email/route.tsapps/blog-next/app/verify-email/page.tsxapps/blog-next/config/session.tsapps/blog-nuxt/config/session.tsapps/blog-nuxt/server/api/forgot-password.post.tsapps/blog-nuxt/server/api/login.post.tsapps/blog-nuxt/server/api/register.post.tsapps/blog-nuxt/server/api/reset-password.post.tsapps/blog-nuxt/server/api/verify-email.post.tsapps/blog-nuxt/server/api/verify-email/resend.post.tsapps/blog-nuxt/tests/run.mjsapps/blog-sveltekit/config/session.tsapps/blog-sveltekit/src/routes/api/forgot-password/+server.tsapps/blog-sveltekit/src/routes/api/login/+server.tsapps/blog-sveltekit/src/routes/api/register/+server.tsapps/blog-sveltekit/src/routes/api/reset-password/+server.tsapps/blog-sveltekit/src/routes/api/verify-email/+server.tsapps/docs/docs/auth/index.mdpackages/adapter-next/src/client.tspackages/adapter-nuxt/src/module.tspackages/adapter-nuxt/src/runtime/composables/index.tspackages/adapter-nuxt/tests/module.test.tspackages/adapter-sveltekit/src/index.tspackages/adapter-sveltekit/src/sveltekit.d.tspackages/adapter-sveltekit/tests/adapter.test.tspackages/auth/src/contracts.tspackages/auth/src/runtime.tspackages/auth/tests/package.test.tspackages/cli/src/project/scaffold/config-renderers.tspackages/core/src/portable/holo.tspackages/core/tests/auth-runtime.test.tspackages/forms/src/client.tspackages/forms/src/contracts.tspackages/forms/src/index.tspackages/forms/src/sensitiveInput.tspackages/forms/tests/client.test.tspackages/forms/tests/contracts.test.tspackages/forms/tests/docs-examples.test.ts
✅ Files skipped from review due to trivial changes (5)
- apps/blog-sveltekit/src/routes/api/forgot-password/+server.ts
- apps/blog-nuxt/server/api/reset-password.post.ts
- apps/blog-nuxt/server/api/verify-email.post.ts
- packages/adapter-next/src/client.ts
- apps/blog-sveltekit/src/routes/api/register/+server.ts
🚧 Files skipped from review as they are similar to previous changes (21)
- apps/blog-sveltekit/src/routes/api/verify-email/+server.ts
- apps/blog-next/app/api/reset-password/route.ts
- apps/blog-next/app/api/forgot-password/route.ts
- apps/blog-nuxt/server/api/login.post.ts
- packages/adapter-sveltekit/src/index.ts
- packages/forms/tests/client.test.ts
- packages/adapter-nuxt/src/module.ts
- apps/blog-nuxt/server/api/register.post.ts
- apps/blog-sveltekit/src/routes/api/login/+server.ts
- packages/forms/src/index.ts
- apps/blog-next/app/api/login/route.ts
- apps/blog-nuxt/server/api/verify-email/resend.post.ts
- apps/blog-nuxt/server/api/forgot-password.post.ts
- apps/blog-next/app/api/verify-email/route.ts
- apps/blog-sveltekit/src/routes/api/reset-password/+server.ts
- packages/core/tests/auth-runtime.test.ts
- packages/cli/src/project/scaffold/config-renderers.ts
- packages/adapter-nuxt/src/runtime/composables/index.ts
- apps/blog-next/app/verify-email/page.tsx
- packages/forms/tests/contracts.test.ts
- packages/forms/src/client.ts
Comment on lines
+933
to
+941
| if (!bindings.context.getRememberToken?.(guardName)) { | ||
| const rememberCookie = parseSetCookieDefinition(bindings.session.rememberMeCookie('')) | ||
| if (rememberCookie) { | ||
| const rememberToken = await resolveRequestCookie(bindings, rememberCookie.name) | ||
| if (rememberToken) { | ||
| bindings.context.setRememberToken?.(guardName, rememberToken) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Remember-me cookies are hydrated but never restored into a session.
Lines 933-941 store the remember token in context, but the session-user path still returns null as soon as there is no sessionId and never consumes that token. A new request with only the remember-me cookie will still look signed out, so the remember login option does not survive SSR request boundaries.
🤖 Prompt for 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.
In `@packages/auth/src/runtime.ts` around lines 933 - 941, The remember-me token
is being read into bindings.context via getRememberToken/setRememberToken but
never used to rehydrate a session, so requests with only the remember cookie
remain signed out; after you obtain rememberToken (inside the block that uses
parseSetCookieDefinition, resolveRequestCookie and setRememberToken) call the
session restore API on bindings.session to create/restore a session from that
token and then populate the context/session state (e.g., call a restore method
such as bindings.session.restoreFromRememberToken(rememberToken) or
bindings.session.createFromRememberToken(rememberToken), then set the resulting
session id or user on the context using bindings.context.setSession?.(guardName,
sessionId) or bindings.context.setUser?.(guardName, user)). If your session
implementation exposes a different method name, invoke that method and ensure
you set the session id/user into bindings.context so subsequent session-user
code no longer returns null.
Comment on lines
+2216
to
+2223
| const serialized = serializeUser(adapter, user, guard.provider) | ||
| if (isEmailVerificationRequired()) { | ||
| await createEmailVerificationFacade().create(serialized, { | ||
| guard: defaultGuard, | ||
| }) | ||
| } | ||
|
|
||
| return serializeUser(adapter, user, guard.provider) | ||
| return serialized |
There was a problem hiding this comment.
Don't make verification delivery a fatal step after the user is already created.
Line 2215 has already persisted the account. If createEmailVerificationFacade().create(...) fails here, registration returns a 500 but leaves a real user behind, so retrying just hits registration_identifier_taken. This should either succeed and let resend recover delivery failures, or compensate by cleaning up the created account/token.
🤖 Prompt for 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.
In `@packages/auth/src/runtime.ts` around lines 2216 - 2223, The code currently
persists the user via serializeUser(adapter, user, guard.provider) and then
calls createEmailVerificationFacade().create(...); if that call fails the user
remains in the DB. Make verification delivery non-fatal by wrapping the
createEmailVerificationFacade().create(...) call in error handling that
compensates on failure: if create(...) throws, delete the just-created user via
the adapter (use the adapter instance and the persisted user's id from
serialized) or otherwise roll back the created account/token so registration
does not leave a dangling user; alternatively consider creating the verification
token before final persistence and only persisting the user after verification
token creation succeeds. Ensure you reference serializeUser,
createEmailVerificationFacade().create, adapter and defaultGuard when
implementing the fix.
Comment on lines
+11
to
+15
| 'token', | ||
| 'verification_code', | ||
| 'verificationCode', | ||
| 'verification_token', | ||
| 'verificationToken', |
There was a problem hiding this comment.
Keep reset/verification transport tokens out of the default global denylist.
These values are needed to resubmit reset/verify flows after any unrelated validation error. Stripping them from flashed values will drop the hidden token on the next round-trip and break the flow.
Suggested change
const DEFAULT_DONT_FLASH_FIELDS = Object.freeze([
'confirm_password',
'confirmPassword',
'current_password',
'currentPassword',
'new_password',
'newPassword',
'password',
'password_confirmation',
'passwordConfirmation',
- 'token',
'verification_code',
'verificationCode',
- 'verification_token',
- 'verificationToken',
])🤖 Prompt for 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.
In `@packages/forms/src/sensitiveInput.ts` around lines 11 - 15, The denylist
currently includes reset/verification transport tokens ('token',
'verification_code', 'verificationCode', 'verification_token',
'verificationToken'), which breaks resubmission of reset/verify flows; remove
these entries from the default denylist array in
packages/forms/src/sensitiveInput.ts (the array that lists sensitive input keys)
so transport tokens are preserved across round-trips, leaving other truly
sensitive keys intact.
This was referenced May 5, 2026
Merged
Merged
Merged
Merged
Merged
Merged
Merged
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 by CodeRabbit
New Features
Configuration
Improvements