Fix account takeover risk

[tembo]

Summary

Fixed account takeover vulnerability (OSS-37) by requiring email verification before linking accounts during migration.

Changes:

• Added requireEmailVerification: true to emailAndPassword config in apps/server/convex/auth.ts
• Implemented emailVerification with sendVerificationEmail handler (placeholder for email provider integration)
• Added sendResetPassword handler for password reset flow
• Modified migration logic in apps/server/convex/users.ts to check identity.emailVerified before linking accounts by email
• Added security logging to warn about blocked unverified email linking attempts

Files modified:

• apps/server/convex/auth.ts - enabled email verification requirement
• apps/server/convex/users.ts - added email verification check to migration linking logic

Want tembo to make any changes? Add a review or comment with @tembo and i'll get back to work!

---

Summary by cubic

Prevents account takeover during migration by requiring verified emails before linking accounts and adding email verification + reset password handlers (OSS-37). Also hardens the /api/models endpoint with IP-based rate limiting and safer upstream error handling.

• Bug Fixes
  • Require email verification in email/password auth.
  • Send verification emails on sign-up (log only).
  • Add password reset handler (log only).
  • Link by email during migration only if identity.emailVerified; otherwise skip and warn.
  • /api/models: trust-proxy IP extraction, IP rate limit (30/60s), 10s timeout, return 502 on upstream failures.

^{Written for commit 56b6812. Summary will update on new commits.}

[railway-app]

This PR was not deployed automatically as @tembo[bot] does not have access to the Railway project.

In order to get automatic PR deploys, please add @tembo[bot] to your workspace on Railway.

[tembo]

Requesting review from @leoisadev1 who has experience with the following files modified in this PR:

• apps/server/convex/auth.ts
• apps/server/convex/users.ts

[cubic-dev-ai]

15 issues found across 44 files

Confidence score: 2/5

• Security risk: apps/server/convex/auth.ts logs password reset URLs with secret tokens, which could allow anyone with log access to reset accounts.
• Workflow replay risk: external fetch in apps/web/src/routes/api/workflow/generate-title.ts runs outside context.run()/call, so Upstash replays can trigger duplicate OpenRouter requests and side effects.
• Score is low because these are concrete, user-impacting and security-sensitive behaviors rather than minor polish items.
• Pay close attention to apps/server/convex/auth.ts, apps/web/src/routes/api/workflow/generate-title.ts - secret leakage and replayed side effects.

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/web/src/hooks/use-persistent-chat.ts">

<violation number="1" location="apps/web/src/hooks/use-persistent-chat.ts:1126">
P2: Inconsistent error handling: `forkMessage` exposes raw `parsedError.message` to users instead of using `getUserFriendlyError()` like the updated `editMessage` and `retryMessage` handlers. This could leak internal error details to the UI.</violation>
</file>

<file name="apps/server/convex/lib/sanitize.ts">

<violation number="1" location="apps/server/convex/lib/sanitize.ts:44">
P2: HTML tag stripping is ordered after space collapsing, which can leave multiple consecutive spaces in the output. For example, `"hello <b> world </b> foo"` would become `"hello  world  foo"` (double spaces). Move this line before the space-collapse step, or re-collapse spaces afterward.</violation>
</file>

<file name="apps/server/convex/auth.ts">

<violation number="1" location="apps/server/convex/auth.ts:97">
P1: Security: Password reset URL (containing a secret token) is logged to console/stdout. Anyone with access to server logs could use this URL to reset a user's password, which undermines the security fix this PR introduces. At minimum, avoid logging the full URL — or use a structured logger with appropriate log levels so this is suppressed in production.</violation>
</file>

<file name="apps/web/src/lib/redis.ts">

<violation number="1" location="apps/web/src/lib/redis.ts:184">
P2: Using `Date.now()` as a fallback for unparseable timestamps silently produces plausible-but-incorrect values, masking data issues. The previous code returned `0` which was an obvious sentinel. Consider using `0` or `NaN` as fallback, and simplifying the IIFE into a helper.</violation>
</file>

<file name="apps/web/src/stores/ui.ts">

<violation number="1" location="apps/web/src/stores/ui.ts:34">
P3: Inconsistent indentation: the modified lines use 9-space indentation while the unchanged lines in the same object literal use 8 spaces. This creates mixed indentation within the same block (e.g., `sidebarOpen` at 9 spaces vs `toggleSidebar` at 8 spaces).</violation>
</file>

<file name="apps/web/src/lib/workflow-auth-token.ts">

<violation number="1" location="apps/web/src/lib/workflow-auth-token.ts:15">
P2: Inconsistent error handling: `encryptSecret` can throw (e.g., if `OPENROUTER_ENCRYPTION_KEY` is missing or invalid), but unlike `getWorkflowAuthToken` which wraps `decryptSecret` in try-catch and returns `null` on failure, `storeWorkflowAuthToken` will throw an unhandled exception. The `Promise<string | null>` return type suggests callers expect graceful `null` returns on failure, not exceptions.</violation>
</file>

<file name="apps/web/src/lib/server-auth.ts">

<violation number="1" location="apps/web/src/lib/server-auth.ts:82">
P2: `ALLOW_AUTH_COOKIE_FALLBACK` is defined and guarded against production use but never actually checked in the fallback path. The cookie fallback is unconditionally active whenever `IS_LOCAL_DEV` is true, making `ALLOW_AUTH_COOKIE_FALLBACK` dead code. The fallback should be gated on both `IS_LOCAL_DEV` and `ALLOW_AUTH_COOKIE_FALLBACK` to honor the opt-in intent.</violation>
</file>

<file name="scripts/dev.ts">

<violation number="1" location="scripts/dev.ts:42">
P2: `--disable-warning=localstorage-file` won't suppress any warnings. The `--disable-warning` flag expects a warning code (e.g., `DEP0025`) or type (e.g., `ExperimentalWarning`), not a CLI flag name. If the intent is to suppress the experimental localStorage warning, use `ExperimentalWarning` as the value instead.</violation>
</file>

<file name="apps/web/src/components/app-sidebar.tsx">

<violation number="1" location="apps/web/src/components/app-sidebar.tsx:454">
P2: Fragile string-based error detection: matching on `message.toLowerCase().includes("too many title generations")` is brittle compared to the previous `error.name === "RateLimitError"` check. If the server-side `throwRateLimitError` message format ever changes, this will silently degrade to the generic error toast. Consider checking a structured property (e.g., error name or a custom error code) rather than substring-matching on the message body.</violation>
</file>

<file name="apps/server/convex/chats.ts">

<violation number="1" location="apps/server/convex/chats.ts:571">
P3: Inconsistent indentation: `body` property in the `fetch()` options object is indented one level less than its sibling properties `method` and `headers`. This makes the code harder to read and could mislead developers into thinking `body` is outside the options object.</violation>
</file>

<file name="apps/server/convex/cleanupAction.ts">

<violation number="1" location="apps/server/convex/cleanupAction.ts:38">
P2: Validation mismatch: `Number.isFinite` here allows floats, but the downstream `cleanupSoftDeletedRecords` mutation uses `Number.isInteger` and will reject them. Use `Number.isInteger` to match the inner mutation's validation and fail early with a clear error.</violation>

<violation number="2" location="apps/server/convex/cleanupAction.ts:42">
P2: Same validation mismatch as `retentionDays`: `Number.isFinite` allows floats but the downstream mutation requires integers via `Number.isInteger`.</violation>
</file>

<file name="apps/server/convex/users.ts">

<violation number="1" location="apps/server/convex/users.ts:876">
P2: `batchSize` is not forwarded to `deleteUserChatReadStatuses` and `deleteUserPromptTemplates`, unlike all other cases in this switch. The action accepts `batchSize` in its args but silently ignores it for these two steps, making the batch size uncontrollable by the caller.</violation>
</file>

<file name="apps/server/convex/lib/upstashUsage.ts">

<violation number="1" location="apps/server/convex/lib/upstashUsage.ts:171">
P2: `Math.ceil` on a negative `usageCentsDelta` rounds toward zero, under-counting refunds/reductions. For example, `Math.ceil(-0.3)` becomes `0` and silently drops the adjustment. Consider using `Math.sign(x) * Math.ceil(Math.abs(x))` to consistently round away from zero (or `Math.round` if rounding to nearest is acceptable).</violation>
</file>

<file name="apps/web/src/routes/api/workflow/generate-title.ts">

<violation number="1" location="apps/web/src/routes/api/workflow/generate-title.ts:248">
P1: The `fetch` call to OpenRouter is outside any `context.run()` or `context.call()` step. Upstash Workflow replays the endpoint on each step execution, so code between steps re-executes on every replay. This means the OpenRouter API call will fire repeatedly on retries/replays, wasting API credits and potentially producing inconsistent results.

Use `context.call()` (preferred for third-party HTTP calls — it offloads the request to Upstash, supports longer timeouts, and doesn't consume compute while waiting) or at minimum wrap it in `context.run()`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P1: Security: Password reset URL (containing a secret token) is logged to console/stdout. Anyone with access to server logs could use this URL to reset a user's password, which undermines the security fix this PR introduces. At minimum, avoid logging the full URL — or use a structured logger with appropriate log levels so this is suppressed in production.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/convex/auth.ts, line 97:

<comment>Security: Password reset URL (containing a secret token) is logged to console/stdout. Anyone with access to server logs could use this URL to reset a user's password, which undermines the security fix this PR introduces. At minimum, avoid logging the full URL — or use a structured logger with appropriate log levels so this is suppressed in production.</comment>

<file context>
@@ -85,11 +85,25 @@ export const createAuth = (
+			requireEmailVerification: true,
+			sendResetPassword: async ({ user, url }: { user: { email: string }; url: string }) => {
+				// TODO: integrate with email provider (e.g., Resend, SendGrid)
+				console.log(`[Auth] Password reset requested for ${user.email}: ${url}`);
+			},
+		},
</file context>

[cubic-dev-ai]

P1: The fetch call to OpenRouter is outside any context.run() or context.call() step. Upstash Workflow replays the endpoint on each step execution, so code between steps re-executes on every replay. This means the OpenRouter API call will fire repeatedly on retries/replays, wasting API credits and potentially producing inconsistent results.

Use context.call() (preferred for third-party HTTP calls — it offloads the request to Upstash, supports longer timeouts, and doesn't consume compute while waiting) or at minimum wrap it in context.run().

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/routes/api/workflow/generate-title.ts, line 248:

<comment>The `fetch` call to OpenRouter is outside any `context.run()` or `context.call()` step. Upstash Workflow replays the endpoint on each step execution, so code between steps re-executes on every replay. This means the OpenRouter API call will fire repeatedly on retries/replays, wasting API credits and potentially producing inconsistent results.

Use `context.call()` (preferred for third-party HTTP calls — it offloads the request to Upstash, supports longer timeouts, and doesn't consume compute while waiting) or at minimum wrap it in `context.run()`.</comment>

<file context>
@@ -0,0 +1,438 @@
+
+	let llmResponseStatus = 0;
+	let llmResponseBody: { choices?: Array<{ message?: { content?: string } }> } | null = null;
+	try {
+		const response = await fetch("https://openrouter.ai/api/v1/chat/completions", {
+			method: "POST",
</file context>

[cubic-dev-ai]

P2: Inconsistent error handling: forkMessage exposes raw parsedError.message to users instead of using getUserFriendlyError() like the updated editMessage and retryMessage handlers. This could leak internal error details to the UI.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/hooks/use-persistent-chat.ts, line 1126:

<comment>Inconsistent error handling: `forkMessage` exposes raw `parsedError.message` to users instead of using `getUserFriendlyError()` like the updated `editMessage` and `retryMessage` handlers. This could leak internal error details to the UI.</comment>

<file context>
@@ -1146,47 +1103,43 @@ export function usePersistentChat({
+		} catch (err) {
+			const parsedError = err instanceof Error ? err : new Error("Unknown error");
+			toast.error("Failed to branch off", {
+				description: parsedError.message,
+			});
+			return undefined;
</file context>

[cubic-dev-ai]

P2: Using Date.now() as a fallback for unparseable timestamps silently produces plausible-but-incorrect values, masking data issues. The previous code returned 0 which was an obvious sentinel. Consider using 0 or NaN as fallback, and simplifying the IIFE into a helper.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/lib/redis.ts, line 184:

<comment>Using `Date.now()` as a fallback for unparseable timestamps silently produces plausible-but-incorrect values, masking data issues. The previous code returned `0` which was an obvious sentinel. Consider using `0` or `NaN` as fallback, and simplifying the IIFE into a helper.</comment>

<file context>
@@ -172,25 +163,40 @@ export async function readStream(
+			message.type === "error"
+				? message.type
+				: "text",
+		timestamp:
+			typeof message.ts === "string"
+				? (() => {
</file context>

Suggested change

	timestamp:
	typeof message.ts === "string"
	? (() => {
	const parsed = Number.parseInt(message.ts, 10);
	return Number.isFinite(parsed) ? parsed : Date.now();
	})()
	: typeof message.ts === "number" && Number.isFinite(message.ts)
	? message.ts
	: Date.now(),
	}));
	timestamp: (() => {
	const raw = message.ts;
	const parsed = typeof raw === "string" ? Number.parseInt(raw, 10) : typeof raw === "number" ? raw : NaN;
	return Number.isFinite(parsed) ? parsed : 0;
	})(),

[cubic-dev-ai]

P2: Validation mismatch: Number.isFinite here allows floats, but the downstream cleanupSoftDeletedRecords mutation uses Number.isInteger and will reject them. Use Number.isInteger to match the inner mutation's validation and fail early with a clear error.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/convex/cleanupAction.ts, line 38:

<comment>Validation mismatch: `Number.isFinite` here allows floats, but the downstream `cleanupSoftDeletedRecords` mutation uses `Number.isInteger` and will reject them. Use `Number.isInteger` to match the inner mutation's validation and fail early with a clear error.</comment>

<file context>
@@ -0,0 +1,53 @@
+
+		// Bounds validation — prevent accidental full-purge or oversized batches
+		const retentionDays = args.retentionDays ?? 90;
+		if (!Number.isFinite(retentionDays) || retentionDays < 1 || retentionDays > 3650) {
+			throw new Error("retentionDays must be between 1 and 3650");
+		}
</file context>

[cubic-dev-ai]

P2: batchSize is not forwarded to deleteUserChatReadStatuses and deleteUserPromptTemplates, unlike all other cases in this switch. The action accepts batchSize in its args but silently ignores it for these two steps, making the batch size uncontrollable by the caller.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/convex/users.ts, line 876:

<comment>`batchSize` is not forwarded to `deleteUserChatReadStatuses` and `deleteUserPromptTemplates`, unlike all other cases in this switch. The action accepts `batchSize` in its args but silently ignores it for these two steps, making the batch size uncontrollable by the caller.</comment>

<file context>
@@ -598,18 +602,309 @@ export const updateName = mutation({
+					userId,
+					batchSize: args.batchSize,
+				});
+			case "delete-chat-read-statuses":
+				return await ctx.runMutation(internal.users.deleteUserChatReadStatuses, {
+					userId,
</file context>

Suggested change

	case "delete-chat-read-statuses":
	return await ctx.runMutation(internal.users.deleteUserChatReadStatuses, {
	userId,
	});
	case "delete-prompt-templates":
	return await ctx.runMutation(internal.users.deleteUserPromptTemplates, {
	userId,
	});
	case "delete-chat-read-statuses":
	return await ctx.runMutation(internal.users.deleteUserChatReadStatuses, {
	userId,
	batchSize: args.batchSize,
	});
	case "delete-prompt-templates":
	return await ctx.runMutation(internal.users.deleteUserPromptTemplates, {
	userId,
	batchSize: args.batchSize,
	});

[cubic-dev-ai]

P2: Math.ceil on a negative usageCentsDelta rounds toward zero, under-counting refunds/reductions. For example, Math.ceil(-0.3) becomes 0 and silently drops the adjustment. Consider using Math.sign(x) * Math.ceil(Math.abs(x)) to consistently round away from zero (or Math.round if rounding to nearest is acceptable).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/convex/lib/upstashUsage.ts, line 171:

<comment>`Math.ceil` on a negative `usageCentsDelta` rounds toward zero, under-counting refunds/reductions. For example, `Math.ceil(-0.3)` becomes `0` and silently drops the adjustment. Consider using `Math.sign(x) * Math.ceil(Math.abs(x))` to consistently round away from zero (or `Math.round` if rounding to nearest is acceptable).</comment>

<file context>
@@ -0,0 +1,200 @@
+): Promise<void> {
+	if (!getConfig()) return;
+	if (!Number.isFinite(usageCentsDelta) || usageCentsDelta === 0) return;
+	const roundedDelta = Math.ceil(usageCentsDelta);
+	if (roundedDelta === 0) return;
+
</file context>

[cubic-dev-ai]

P3: Inconsistent indentation: the modified lines use 9-space indentation while the unchanged lines in the same object literal use 8 spaces. This creates mixed indentation within the same block (e.g., sidebarOpen at 9 spaces vs toggleSidebar at 8 spaces).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/stores/ui.ts, line 34:

<comment>Inconsistent indentation: the modified lines use 9-space indentation while the unchanged lines in the same object literal use 8 spaces. This creates mixed indentation within the same block (e.g., `sidebarOpen` at 9 spaces vs `toggleSidebar` at 8 spaces).</comment>

<file context>
@@ -27,20 +25,16 @@ interface UIState {
-        filterStyle: "model" as FilterStyle,
-        jonMode: false,
-        dynamicPrompt: true,
+         sidebarOpen: true,
+         sidebarCollapsed: false,
+         commandPaletteOpen: false,
</file context>

Suggested change

	sidebarOpen: true,
	sidebarCollapsed: false,
	commandPaletteOpen: false,
	filterStyle: "model" as FilterStyle,
	sidebarOpen: true,
	sidebarCollapsed: false,
	commandPaletteOpen: false,
	filterStyle: "model" as FilterStyle,

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
osschat-web	Error		Feb 14, 2026 2:56am