Fix insecure localStorage usage

[tembo]

Summary

Migrated chat stream storage from localStorage to sessionStorage and excluded sensitive fields (content, reasoning, text) from persistence to prevent exposure via XSS or compromised browser profiles. Only non-sensitive metadata (IDs, timestamps, mode) is now persisted for stream resumption.

Changes:

• Switched storage from localStorage to sessionStorage in apps/web/src/stores/stream.ts
• Modified partialize function to exclude sensitive fields:
  • activeStream.content → empty string
  • activeStream.reasoning → empty string
  • pendingUserMessage.text → empty string
• Added inline comments explaining security rationale

Files modified:

• apps/web/src/stores/stream.ts

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

---

Summary by cubic

Switched chat stream persistence from localStorage to sessionStorage and stopped saving message content, reasoning, and text; only IDs, lastEventId, timestamps, and mode are stored to reduce exposure risk. Also merged main’s /api/models hardening: trust‑proxy IP extraction, 30 req/60s rate limiting, and a 10s upstream timeout with 502 on failures.

^{Written for commit 96febf4. 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/web/src/stores/stream.ts

[cubic-dev-ai]

16 issues found across 44 files

Confidence score: 2/5

• High risk due to Upstash Workflow handlers performing work/throwing outside context.run() in apps/web/src/routes/api/workflow/generate-title.ts and apps/web/src/routes/api/workflow/cleanup.ts, which can cause repeated retries or duplicate work.
• Convex strict validators and workflow data shape mismatches in apps/server/convex/backgroundStream.ts could break existing stream job queries at runtime.
• Several user-facing correctness issues remain (error sanitization inconsistencies in apps/web/src/hooks/use-persistent-chat.ts, fragile rate-limit detection in apps/web/src/components/app-sidebar.tsx, and unsafe optional userId casting in apps/web/src/routes/api/workflow/export-chat.ts).
• Pay close attention to apps/web/src/routes/api/workflow/generate-title.ts, apps/web/src/routes/api/workflow/cleanup.ts, apps/server/convex/backgroundStream.ts, apps/web/src/hooks/use-persistent-chat.ts, apps/web/src/components/app-sidebar.tsx, apps/web/src/routes/api/workflow/export-chat.ts - workflow step placement, strict validators, and user-facing error handling.

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/server/convex/lib/upstashUsage.ts">

<violation number="1" location="apps/server/convex/lib/upstashUsage.ts:145">
P1: Race condition: the negative-total floor is not atomic. Between the first pipeline (`INCRBY` returning a negative total) and the second pipeline (`SET key 0`), a concurrent `INCRBY` from another request could increment the counter. The `SET 0` then overwrites that increment, silently losing recorded usage.

Consider using a Lua script via `EVAL` to make the check-and-floor atomic, e.g.:

local val = redis.call('INCRBY', KEYS[1], ARGV[1])
redis.call('EXPIREAT', KEYS[1], ARGV[2])
if val < 0 then
redis.call('SET', KEYS[1], 0)
redis.call('EXPIREAT', KEYS[1], ARGV[2])
end
return val

</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 checked only in the production guard but never gates the actual fallback logic. The cookie fallback path (line 88–92) is controlled solely by `IS_LOCAL_DEV`, so this env var has no effect on whether the fallback is used. Either the fallback condition should incorporate `ALLOW_AUTH_COOKIE_FALLBACK`, or the unused variable and guard should be removed to avoid confusion.</violation>
</file>

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

<violation number="1" location="apps/server/convex/chats.ts:571">
P2: Inconsistent indentation: `body` is indented one level less than `method` and `headers` in the same object literal. This makes it look like `body` might be outside the fetch options, harming readability.</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:249">
P1: The OpenRouter fetch call is outside any `context.run()` / `context.call()` step in the Upstash Workflow handler. Per Upstash Workflow docs, the workflow endpoint is re-invoked from the top for every step—completed `context.run` steps are skipped via cached results, but bare code re-executes every time. This means the LLM call will fire on every step resumption/retry, causing duplicate API calls and non-deterministic behavior.

Wrap the HTTP call in `context.call()` (preferred for HTTP, offloads the wait to Upstash) or at minimum in `context.run()`:</violation>
</file>

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

<violation number="1" location="apps/server/convex/http.ts:97">
P2: Bearer token extraction is case-sensitive, but RFC 7235 specifies auth-scheme matching should be case-insensitive. A client sending `bearer` or `BEARER` would have its Authorization header silently ignored. Consider using a case-insensitive check.</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 reformatted lines use 9-space indent while surrounding unchanged lines in the same object use 8-space indent. This creates mixed indentation within the same block (e.g., state properties at 9 spaces vs. action methods at 8 spaces).</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 applied after the space-collapse step, so removing tags can leave uncollapsed consecutive spaces in the output (e.g., `"hello <b>world</b> foo"` → `"hello world foo"`). Re-collapse spaces after stripping tags to maintain the single-space invariant.</violation>
</file>

<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 sanitization: `forkMessage` still exposes raw `parsedError.message` to users, while `editMessage` and `retryMessage` were updated in this PR to use `getUserFriendlyError()`. This could leak internal error details (API structures, DB field names, service error formats) to the UI.</violation>
</file>

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

<violation number="1" location="apps/server/convex/cleanupAction.ts:38">
P2: Validation mismatch: `Number.isFinite()` is used here, but the downstream mutation (`crons.cleanupSoftDeletedRecords`) validates with `Number.isInteger()`. Non-integer values (e.g., `1.5`) will pass this check but be rejected by the mutation. Use `Number.isInteger()` to match the stricter validation and provide clearer error messages at the action boundary.</violation>

<violation number="2" location="apps/server/convex/cleanupAction.ts:42">
P2: Same validation mismatch for `batchSize`: use `Number.isInteger()` instead of `Number.isFinite()` to match the downstream mutation's validation.</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 matching replaces the robust `error.name === "RateLimitError"` check used elsewhere in this file (line 520). The server explicitly sets `error.name = "RateLimitError"` — use that instead. Also, the server message already contains a specific retry time (e.g., "in 5 seconds") which is lost by hardcoding "later".</violation>
</file>

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

<violation number="1" location="apps/web/src/routes/api/workflow/export-chat.ts:124">
P2: `userId` is optional in `ExportChatPayload` but is unsafely cast to `Id<"users">` without a guard. If `runExportChatInline` is ever called without `userId` set, `undefined as Id<"users">` will silently pass a bad value to Convex. Add a runtime check or make `userId` required in the function's expected input.</violation>
</file>

<file name="apps/web/src/routes/api/workflow/cleanup.ts">

<violation number="1" location="apps/web/src/routes/api/workflow/cleanup.ts:183">
P1: Throwing outside `context.run()` in the Upstash Workflow handler will cause infinite retries for non-transient errors (invalid payload, missing env var). These conditions will never self-resolve, so each retry wastes resources. Return a result object instead, matching the pattern in `delete-account.ts` which returns `EMPTY_DELETE_RESULT` for similar cases.</violation>
</file>

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

<violation number="1" location="apps/server/convex/users.ts:870">
P2: `batchSize` is not forwarded to `deleteUserChatReadStatuses` here, unlike the other four deletion steps which all pass `args.batchSize`. If a caller specifies a custom `batchSize`, it will silently be ignored for this step (and `delete-prompt-templates` below). Pass it through for consistency.</violation>
</file>

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

<violation number="1" location="apps/server/convex/backgroundStream.ts:36">
P1: The `streamOptionsValidator` is missing `jonMode` and `dynamicPrompt` fields that exist on stored `streamJobs` documents. Convex `v.object()` validators are strict and reject extra properties, so querying any existing job with these fields in `options` will throw a runtime validation error from the `returns` validator in `getStreamJob` and `getActiveStreamJob`.</violation>
</file>

<file name="apps/web/src/routes/api/models.ts">

<violation number="1" location="apps/web/src/routes/api/models.ts:10">
P2: `TRUST_PROXY=false` (or `no`, `0`) is not handled as a disabled state. These are common ways to explicitly disable a feature, but because they're truthy strings, `getClientIp` falls through to return `null` — causing every request to get a 400 when Redis is configured. Consider treating `'false'`, `'no'`, and `'0'` the same as unset.</violation>
</file>

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

[cubic-dev-ai]

P1: Race condition: the negative-total floor is not atomic. Between the first pipeline (INCRBY returning a negative total) and the second pipeline (SET key 0), a concurrent INCRBY from another request could increment the counter. The SET 0 then overwrites that increment, silently losing recorded usage.

Consider using a Lua script via EVAL to make the check-and-floor atomic, e.g.:

local val = redis.call('INCRBY', KEYS[1], ARGV[1])
redis.call('EXPIREAT', KEYS[1], ARGV[2])
if val < 0 then
  redis.call('SET', KEYS[1], 0)
  redis.call('EXPIREAT', KEYS[1], ARGV[2])
end
return val

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 145:

<comment>Race condition: the negative-total floor is not atomic. Between the first pipeline (`INCRBY` returning a negative total) and the second pipeline (`SET key 0`), a concurrent `INCRBY` from another request could increment the counter. The `SET 0` then overwrites that increment, silently losing recorded usage.

Consider using a Lua script via `EVAL` to make the check-and-floor atomic, e.g.:

local val = redis.call('INCRBY', KEYS[1], ARGV[1])
redis.call('EXPIREAT', KEYS[1], ARGV[2])
if val < 0 then
redis.call('SET', KEYS[1], 0)
redis.call('EXPIREAT', KEYS[1], ARGV[2])
end
return val

<file context>
@@ -0,0 +1,200 @@
+
+	try {
+		const expiresAt = getMidnightUtcEpochSeconds(dateKey);
+		const result = await executePipeline([
+			["INCRBY", key, roundedCents],
+			["EXPIREAT", key, expiresAt],
</file context>

[cubic-dev-ai]

P1: The OpenRouter fetch call is outside any context.run() / context.call() step in the Upstash Workflow handler. Per Upstash Workflow docs, the workflow endpoint is re-invoked from the top for every step—completed context.run steps are skipped via cached results, but bare code re-executes every time. This means the LLM call will fire on every step resumption/retry, causing duplicate API calls and non-deterministic behavior.

Wrap the HTTP call in context.call() (preferred for HTTP, offloads the wait to Upstash) or at minimum 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 249:

<comment>The OpenRouter fetch call is outside any `context.run()` / `context.call()` step in the Upstash Workflow handler. Per Upstash Workflow docs, the workflow endpoint is re-invoked from the top for every step—completed `context.run` steps are skipped via cached results, but bare code re-executes every time. This means the LLM call will fire on every step resumption/retry, causing duplicate API calls and non-deterministic behavior.

Wrap the HTTP call in `context.call()` (preferred for HTTP, offloads the wait to Upstash) or at minimum 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",
+			headers: {
</file context>

[cubic-dev-ai]

P1: Throwing outside context.run() in the Upstash Workflow handler will cause infinite retries for non-transient errors (invalid payload, missing env var). These conditions will never self-resolve, so each retry wastes resources. Return a result object instead, matching the pattern in delete-account.ts which returns EMPTY_DELETE_RESULT for similar cases.

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/cleanup.ts, line 183:

<comment>Throwing outside `context.run()` in the Upstash Workflow handler will cause infinite retries for non-transient errors (invalid payload, missing env var). These conditions will never self-resolve, so each retry wastes resources. Return a result object instead, matching the pattern in `delete-account.ts` which returns `EMPTY_DELETE_RESULT` for similar cases.</comment>

<file context>
@@ -0,0 +1,310 @@
+
+const workflow = serve<CleanupPayload>(async (context) => {
+	const payload = parseCleanupPayload(context.requestPayload);
+	if (!payload) {
+		throw new Error("Invalid cleanup payload");
+	}
</file context>

[cubic-dev-ai]

P1: The streamOptionsValidator is missing jonMode and dynamicPrompt fields that exist on stored streamJobs documents. Convex v.object() validators are strict and reject extra properties, so querying any existing job with these fields in options will throw a runtime validation error from the returns validator in getStreamJob and getActiveStreamJob.

Prompt for AI agents

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

<comment>The `streamOptionsValidator` is missing `jonMode` and `dynamicPrompt` fields that exist on stored `streamJobs` documents. Convex `v.object()` validators are strict and reject extra properties, so querying any existing job with these fields in `options` will throw a runtime validation error from the `returns` validator in `getStreamJob` and `getActiveStreamJob`.</comment>

<file context>
@@ -26,6 +33,14 @@ const chainOfThoughtPartValidator = v.object({
 	errorText: v.optional(v.string()),
 });
 
+const streamOptionsValidator = v.object({
+	enableReasoning: v.optional(v.boolean()),
+	reasoningEffort: v.optional(v.string()),
</file context>

Suggested change

	const streamOptionsValidator = v.object({
	enableReasoning: v.optional(v.boolean()),
	reasoningEffort: v.optional(v.string()),
	enableWebSearch: v.optional(v.boolean()),
	supportsToolCalls: v.optional(v.boolean()),
	maxSteps: v.optional(v.number()),
	});
	const streamOptionsValidator = v.object({
	enableReasoning: v.optional(v.boolean()),
	reasoningEffort: v.optional(v.string()),
	enableWebSearch: v.optional(v.boolean()),
	supportsToolCalls: v.optional(v.boolean()),
	maxSteps: v.optional(v.number()),
	// Deprecated: retained for backward compatibility with existing documents
	jonMode: v.optional(v.boolean()),
	dynamicPrompt: v.optional(v.boolean()),
	});

[cubic-dev-ai]

P2: ALLOW_AUTH_COOKIE_FALLBACK is checked only in the production guard but never gates the actual fallback logic. The cookie fallback path (line 88–92) is controlled solely by IS_LOCAL_DEV, so this env var has no effect on whether the fallback is used. Either the fallback condition should incorporate ALLOW_AUTH_COOKIE_FALLBACK, or the unused variable and guard should be removed to avoid confusion.

Prompt for AI agents

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

<comment>`ALLOW_AUTH_COOKIE_FALLBACK` is checked only in the production guard but never gates the actual fallback logic. The cookie fallback path (line 88–92) is controlled solely by `IS_LOCAL_DEV`, so this env var has no effect on whether the fallback is used. Either the fallback condition should incorporate `ALLOW_AUTH_COOKIE_FALLBACK`, or the unused variable and guard should be removed to avoid confusion.</comment>

<file context>
@@ -16,18 +16,81 @@ type AuthSessionResponse = {
+			if (response.status >= 400 && response.status < 500) {
+				return null;
+			}
+			if (!IS_LOCAL_DEV) return null;
+		} catch {
+			if (!IS_LOCAL_DEV) return null;
</file context>

[cubic-dev-ai]

P2: Fragile string matching replaces the robust error.name === "RateLimitError" check used elsewhere in this file (line 520). The server explicitly sets error.name = "RateLimitError" — use that instead. Also, the server message already contains a specific retry time (e.g., "in 5 seconds") which is lost by hardcoding "later".

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/app-sidebar.tsx, line 454:

<comment>Fragile string matching replaces the robust `error.name === "RateLimitError"` check used elsewhere in this file (line 520). The server explicitly sets `error.name = "RateLimitError"` — use that instead. Also, the server message already contains a specific retry time (e.g., "in 5 seconds") which is lost by hardcoding "later".</comment>

<file context>
@@ -422,28 +422,37 @@ export function AppSidebar({
-      if (error instanceof Error && error.name === "RateLimitError") {
-        toast.error(error.message);
+      const message = error instanceof Error ? error.message : "";
+      if (message.toLowerCase().includes("too many title generations")) {
+        toast.error("Too many title generations. Please try again later.");
       } else {
</file context>

[cubic-dev-ai]

P2: userId is optional in ExportChatPayload but is unsafely cast to Id<"users"> without a guard. If runExportChatInline is ever called without userId set, undefined as Id<"users"> will silently pass a bad value to Convex. Add a runtime check or make userId required in the function's expected input.

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/export-chat.ts, line 124:

<comment>`userId` is optional in `ExportChatPayload` but is unsafely cast to `Id<"users">` without a guard. If `runExportChatInline` is ever called without `userId` set, `undefined as Id<"users">` will silently pass a bad value to Convex. Add a runtime check or make `userId` required in the function's expected input.</comment>

<file context>
@@ -0,0 +1,226 @@
+	const convexClient = createConvexServerClient(authToken);
+	const chatExportData = await convexClient.query(api.chats.getChatExportData, {
+		chatId: chatId as Id<"chats">,
+		userId: userId as Id<"users">,
+	});
+	if (!chatExportData) {
</file context>

[cubic-dev-ai]

P2: batchSize is not forwarded to deleteUserChatReadStatuses here, unlike the other four deletion steps which all pass args.batchSize. If a caller specifies a custom batchSize, it will silently be ignored for this step (and delete-prompt-templates below). Pass it through for consistency.

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 870:

<comment>`batchSize` is not forwarded to `deleteUserChatReadStatuses` here, unlike the other four deletion steps which all pass `args.batchSize`. If a caller specifies a custom `batchSize`, it will silently be ignored for this step (and `delete-prompt-templates` below). Pass it through for consistency.</comment>

<file context>
@@ -598,18 +596,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]

P3: Inconsistent indentation: the reformatted lines use 9-space indent while surrounding unchanged lines in the same object use 8-space indent. This creates mixed indentation within the same block (e.g., state properties at 9 spaces vs. action methods 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 reformatted lines use 9-space indent while surrounding unchanged lines in the same object use 8-space indent. This creates mixed indentation within the same block (e.g., state properties at 9 spaces vs. action methods 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:55am