biome.json (1)

72-72: Use Biome overrides to limit the noImgElement rule disable to specific files.

Disabling noImgElement project-wide prevents Biome from encouraging Next.js's optimized <Image> component, which provides automatic lazy loading, responsive optimization, and improved performance (LCP, bandwidth).

Instead of a global disable, consider using Biome's overrides configuration to limit this to files that genuinely require raw <img> elements (e.g., QR code generation with data URLs):

"overrides": [
  {
    "includes": ["src/path/to/qr-code-files/**"],
    "linter": {
      "rules": {
        "performance": {
          "noImgElement": "off"
        }
      }
    }
  }
]

This keeps the rule active project-wide while carving out exceptions only where needed.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@biome.json` at line 72, The project currently disables the noImgElement rule
globally; instead add a Biome "overrides" entry in biome.json that keeps the
global rule enabled and only sets linter.rules.performance.noImgElement to "off"
for specific file globs via the "overrides" -> "includes" array (e.g., paths
that generate QR codes or use data-URL images). Locate the "noImgElement"
setting and replace the global disable with an overrides block using the
"overrides", "includes", and "linter" keys so only targeted files get
noImgElement: "off" while the rule remains active elsewhere.

packages/auth/src/totp.ts (1)

85-103: Early-exit loop leaks timing side-channel on recovery-code position — low risk, worth noting.

consumeRecoveryCode returns as soon as verifyPasswordHash matches. Because argon2 work is significant, an attacker with precise latency measurements can (probabilistically) infer the index of the matching stored hash. The risk is bounded — the attacker still needs a valid recovery code — but for defense in depth you can continue iterating all hashes and select the match at the end.

Not a blocker; treat as optional hardening.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/auth/src/totp.ts` around lines 85 - 103, The loop in
consumeRecoveryCode leaks which index matched by returning immediately after
verifyPasswordHash; instead, run verifyPasswordHash against every entry (using
normalized from normalizeRecoveryCode) and record whether any matched and the
matched index (or first match) but do not early-return; after the loop construct
remaining by removing the recorded matched index (if any) and then return {
valid: Boolean(matchFound), remaining }; ensure every verifyPasswordHash call is
awaited so total timing is consistent.

apps/start/src/routes/_login.verify.tsx (1)

65-98: Consider guarding the auto-submit against rapid re-fires.

onChange triggers mutation.mutate({ code: value }) whenever the value reaches length 6. If the mutation errors and the server leaves focus on the input (e.g. user pastes a new 6-digit code, or an autofill re-fills), this fires again automatically. Adding an || mutation.isPending guard here avoids racing with an in-flight request.

♻️ Suggested change

           onChange={(value) => {
             setCode(value);
-            if (value.length === 6) {
+            if (value.length === 6 && !mutation.isPending) {
               mutation.mutate({ code: value });
             }
           }}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/start/src/routes/_login.verify.tsx` around lines 65 - 98, The onChange
handler in the InputOTP block currently calls mutation.mutate({ code: value })
as soon as value.length === 6, allowing rapid re-fires; update the handler used
in the InputOTP onChange (the arrow that calls setCode and mutation.mutate) to
first check mutation.isPending and only call mutation.mutate when not pending
(e.g., guard with if (value.length === 6 && !mutation.isPending) ...), keeping
setCode behavior intact so the input still updates but preventing duplicate
in-flight submits.

apps/start/src/routes/_app.account.tsx (1)

5-12: Catch-all silently routes transient errors to onboarding.

.catch(() => []) treats every failure (network blip, auth expiration, 5xx) as "no organizations" and redirects the user to /onboarding/project. A user with existing orgs will land in onboarding until they retry. Consider letting the error propagate (TanStack Router will surface it via the nearest error boundary) or at least distinguish "empty list" from "fetch failed".

♻️ Suggested change

-    const organizations = await context.queryClient
-      .fetchQuery(
-        context.trpc.organization.list.queryOptions(undefined, {
-          staleTime: 0,
-          gcTime: 0,
-        }),
-      )
-      .catch(() => []);
+    const organizations = await context.queryClient.fetchQuery(
+      context.trpc.organization.list.queryOptions(undefined, {
+        staleTime: 0,
+        gcTime: 0,
+      }),
+    );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/start/src/routes/_app.account.tsx` around lines 5 - 12, The current
.catch(() => []) on the fetchQuery call for organizations masks all errors
(network/auth/5xx) and treats failures as "no organizations", forcing a redirect
to onboarding; remove the silent catch and let the error propagate from
context.queryClient.fetchQuery (using
context.trpc.organization.list.queryOptions) so TanStack Router can hit the
nearest error boundary, or if you must handle it here, replace the blanket
.catch with an explicit check that distinguishes an empty list from a failed
fetch (e.g., catch and rethrow the error or return a distinct sentinel like null
and handle that case where organizations is consumed) rather than returning []
for every failure.

packages/db/prisma/migrations/20260422194907_add_user_totp_and_2fa_challenge/migration.sql (1)

15-28: Add an index on expiresAt for cleanup queries.

Cleanup jobs that delete expired challenges (DELETE FROM two_factor_challenges WHERE "expiresAt" < now()) will do a full table scan without an index. Also worth considering: no uniqueness on userId — if the app doesn't purge prior challenges before inserting a new one, stale rows will accumulate per user.

♻️ Suggested addition

 CREATE INDEX "two_factor_challenges_userId_idx" ON "public"."two_factor_challenges"("userId");
+
+-- CreateIndex
+CREATE INDEX "two_factor_challenges_expiresAt_idx" ON "public"."two_factor_challenges"("expiresAt");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/db/prisma/migrations/20260422194907_add_user_totp_and_2fa_challenge/migration.sql`
around lines 15 - 28, Add an index on two_factor_challenges.expiresAt to avoid
full table scans for cleanup queries by creating e.g. an index named
two_factor_challenges_expiresAt_idx on the expiresAt column; additionally
consider enforcing uniqueness on two_factor_challenges.userId (or add a unique
index) if the application should only ever have one active challenge per user to
prevent stale rows accumulating (refer to table two_factor_challenges and
columns expiresAt and userId when making these changes).

apps/start/src/routes/_app.$organizationId.account._tabs.tsx (1)

22-34: Optional: consolidate navigation to a single relative form.

The special case for account (absolute) followed by a relative branch for other tabs works, but you could navigate to the parent index using a relative to: '.' (or to: '..' depending on layout/file-route relationship) to keep a single router.navigate call. Not a correctness issue — just removes the branch.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/start/src/routes/_app`.$organizationId.account._tabs.tsx around lines 22
- 34, handleTabChange currently branches to call router.navigate with an
absolute path for the 'account' tab and a relative path for other tabs; simplify
by using a single router.navigate call and pass a relative to value (e.g., to:
'.' or to: '..' depending on your route nesting) when tabId === 'account', and
include params: { organizationId } only when needed (or merge params into the
single call) so you remove the branch while preserving navigation to the parent
account index; update the function name handleTabChange and the router.navigate
invocation accordingly.

apps/start/src/modals/disable-two-factor.tsx (1)

47-54: Consider trimming and submit-on-Enter for better UX.

code is guarded with !code.trim() but the raw value is sent in mutate({ code }). Consider trimming before submit, and wrapping the input in a <form onSubmit> so pressing Enter works the same as clicking Disable.

♻️ Proposed change

-        <Button
-          disabled={!code.trim()}
-          loading={mutation.isPending}
-          onClick={() => mutation.mutate({ code })}
-          variant="destructive"
-        >
+        <Button
+          disabled={!code.trim()}
+          loading={mutation.isPending}
+          onClick={() => mutation.mutate({ code: code.trim() })}
+          variant="destructive"
+        >
           Disable
         </Button>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/start/src/modals/disable-two-factor.tsx` around lines 47 - 54, The
Button currently disables using !code.trim() but sends the raw code via
mutation.mutate({ code }) and doesn’t support Enter-to-submit; update the UI to
trim the value before calling mutation.mutate (use mutation.mutate({ code:
code.trim() }) or equivalent) and wrap the input and Button in a <form> with an
onSubmit handler that prevents default and triggers the same trimmed submit
logic so pressing Enter behaves identically to clicking the Disable Button
(ensure you keep using mutation.isPending for loading and preserve the disabled
condition using !code.trim()).

packages/db/prisma/schema.prisma (1)

150-159: Add an index on expiresAt to support cleanup of expired 2FA challenges.

Expired rows are only deleted during sign-in when the cookie points at them. If a user abandons the flow (closes the tab, loses the cookie), the row lingers. Without an index on expiresAt, a scheduled cleanup (deleteMany({ where: { expiresAt: { lt: now } } })) does a full scan as the table grows.

🗄️ Proposed change

 model TwoFactorChallenge {
   id        String   `@id`
   userId    String
   user      User     `@relation`(fields: [userId], references: [id], onDelete: Cascade)
   expiresAt DateTime
   createdAt DateTime `@default`(now())

   @@index([userId])
+  @@index([expiresAt])
   @@map("two_factor_challenges")
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/db/prisma/schema.prisma` around lines 150 - 159, Add an index on the
expiresAt column in the TwoFactorChallenge Prisma model so cleanup queries can
use an index; update the TwoFactorChallenge model (which currently has
@@index([userId]) and @@map("two_factor_challenges")) to include an index
directive for expiresAt (e.g., add @@index([expiresAt])) so deleteMany({ where:
{ expiresAt: { lt: now } } }) does not require a full table scan.

packages/trpc/src/routers/auth.ts (3)

435-485: Consider emailing the user on 2FA disable / recovery-code regeneration.

Disabling 2FA and regenerating recovery codes are security-sensitive actions. A notification email on these events is a standard best-practice (gives the user a chance to react to a compromised session). Not a blocker — just a recommended enhancement given sendEmail is already available in this file.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/trpc/src/routers/auth.ts` around lines 435 - 485, Add notification
emails when security-sensitive actions occur: in totpDisable and
totpRegenerateRecoveryCodes, after the DB changes (after db.userTotp.delete/...
and after db.userTotp.update(...)), call the existing sendEmail helper to notify
the user's email (accessible via ctx.session.userEmail or
ctx.session.user?.email) about the action, include context (action type "2FA
disabled" or "Recovery codes regenerated", timestamp, and IP/user-agent from ctx
if available), and ensure the mutations still return their existing payloads
even if email sending fails (catch/log email errors rather than throwing).

---

51-52: Operational: schedule cleanup of expired TwoFactorChallenge rows.

Expired rows are only deleted when a user returns to /signInTotp with a matching cookie. Abandoned challenges (user closes the tab) accumulate indefinitely. Combined with the suggested expiresAt index (see schema.prisma), a periodic deleteMany({ where: { expiresAt: { lt: new Date() } } }) job keeps the table bounded.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/trpc/src/routers/auth.ts` around lines 51 - 52, Add a periodic
cleanup job that removes expired TwoFactorChallenge rows (using Prisma's
deleteMany with where: { expiresAt: { lt: new Date() } }) so abandoned 2FA
challenges don't accumulate; schedule it to run at a reasonable interval (e.g.,
every few minutes) from wherever background jobs run in this service and
reference the same TTL/constants (TWO_FACTOR_CHALLENGE_TTL_SECONDS,
TWO_FACTOR_COOKIE) to keep behavior consistent with the sign-in flow and the
expiresAt index in schema.prisma.

---

356-401: Make totpSetup idempotent for not-yet-enabled rows.

Every call unconditionally overwrites secret and recoveryCodes for the user. Combined with setup-two-factor.tsx firing the mutation on mount (which also runs twice in StrictMode), a user can end up with a QR on screen that doesn't match the persisted secret, or have the secret silently rotated when they re-open the modal with their authenticator already paired.

Suggested behavior: if a UserTotp row exists and enabledAt is null, reuse the existing secret and return a fresh QR derived from it; only generate a new secret when none exists. This makes repeated calls safe.

🛡️ Proposed change

-    const secret = generateTotpSecret();
+    const secret =
+      existing && !existing.enabledAt ? decrypt(existing.secret) : generateTotpSecret();
     const otpauthUrl = buildOtpauthUrl({
       secret,
       accountName: user.email,
     });
     const qrDataUrl = await generateQrDataUrl(otpauthUrl);

-    await db.userTotp.upsert({
-      where: { userId },
-      create: {
-        userId,
-        secret: encrypt(secret),
-        recoveryCodes: [],
-      },
-      update: {
-        secret: encrypt(secret),
-        recoveryCodes: [],
-        enabledAt: null,
-      },
-    });
+    if (!existing) {
+      await db.userTotp.create({
+        data: { userId, secret: encrypt(secret), recoveryCodes: [] },
+      });
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/trpc/src/routers/auth.ts` around lines 356 - 401, The totpSetup
mutation currently always generates and persists a new secret
(db.userTotp.upsert), causing race/StrictMode issues; change it to be idempotent
by: when you fetch the existing row (db.userTotp.findUnique) if it exists and
existing.enabledAt is null and existing.secret is present, decrypt and reuse
that secret to build the otpauthUrl and qrDataUrl (use decrypt matching
encrypt), and avoid overwriting secret/recoveryCodes in the database; only
generate a new secret via generateTotpSecret(), encrypt(), and create the row
when no existing row exists. Update or replace the db.userTotp.upsert logic so
it only creates when missing and otherwise leaves the stored
secret/recoveryCodes intact (or only updates enabledAt), and ensure
generateQrDataUrl and buildOtpauthUrl use the reused/decrypted secret.

apps/start/src/modals/regenerate-recovery-codes.tsx (1)

47-57: Handle clipboard write failures.

navigator.clipboard.writeText returns a Promise and can reject (insecure context, permissions, some browsers). As written, the success toast fires even if the write fails, and the rejection will bubble as an unhandled promise rejection.

🛡️ Proposed fix

-          <Button
-            className="self-start"
-            onClick={() => {
-              navigator.clipboard.writeText(newCodes.join('\n'));
-              toast.success('Copied to clipboard');
-            }}
+          <Button
+            className="self-start"
+            onClick={async () => {
+              try {
+                await navigator.clipboard.writeText(newCodes.join('\n'));
+                toast.success('Copied to clipboard');
+              } catch {
+                toast.error('Could not copy to clipboard');
+              }
+            }}
             size="sm"
             variant="outline"
           >
             Copy all
           </Button>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/start/src/modals/regenerate-recovery-codes.tsx` around lines 47 - 57,
The onClick handler for the "Copy all" Button currently calls
navigator.clipboard.writeText(newCodes.join('\n')) and immediately shows
toast.success, which can produce unhandled rejections and false success
messages; update the onClick to handle the returned Promise from
navigator.clipboard.writeText (or first check for navigator.clipboard presence),
then show toast.success only on successful resolution and toast.error (or a
fallback alert) on rejection; reference the Button's onClick handler,
navigator.clipboard.writeText, and toast.success/toast.error when making the
change.

apps/start/src/modals/setup-two-factor.tsx (1)

162-172: Handle clipboard write failures.

Same as in regenerate-recovery-codes.tsx: navigator.clipboard.writeText returns a Promise that can reject (insecure context/permissions). As written, the success toast fires regardless, and a rejection becomes an unhandled promise rejection.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/start/src/modals/setup-two-factor.tsx` around lines 162 - 172, The
onClick handler for the "Copy all" Button uses
navigator.clipboard.writeText(codes.join('\n')) but doesn't handle rejections
and always calls toast.success; change the onClick to await or chain the Promise
from navigator.clipboard.writeText, call toast.success only on resolution, and
call toast.error (or similar) on rejection to avoid unhandled promise rejections
and surface failures; update the handler referenced on the Button (onClick) and
adjust uses of toast.success/toast.error accordingly.

apps/start/src/routes/_app.$organizationId.account._tabs.two-factor.tsx (1)

33-40: Avoid non-null assertion on enabledAt.

Line 35 uses status.data.enabledAt!. Narrow based on enabledAt itself instead of the enabled flag — the non-null assertion couples this render to a backend invariant. A simple check avoids the !:

♻️ Proposed change

-        {status.data.enabled ? (
+        {status.data.enabled && status.data.enabledAt ? (
           <EnabledView
-            enabledAt={status.data.enabledAt!}
+            enabledAt={status.data.enabledAt}
             remainingRecoveryCodes={status.data.remainingRecoveryCodes}
           />
         ) : (
           <DisabledView hasEmailProvider={status.data.hasEmailProvider} />
         )}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/start/src/routes/_app`.$organizationId.account._tabs.two-factor.tsx
around lines 33 - 40, The code currently forces a non-null assertion on
status.data.enabledAt in the EnabledView props; instead, change the conditional
to branch based on the presence of status.data.enabledAt (e.g.
status.data.enabledAt != null) and only render <EnabledView
enabledAt={status.data.enabledAt} .../> when enabledAt exists; keep rendering
<DisabledView hasEmailProvider={status.data.hasEmailProvider}/> otherwise, and
if EnabledView's prop type requires a non-null value, update its usage/prop type
to accept the checked value rather than using the `!` assertion.

Fix all unresolved CodeRabbit comments on this PR:

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