Fix insecure draft storage

[tembo]

Summary

Migrated prompt draft storage from localStorage to sessionStorage to mitigate security risks. Drafts containing sensitive user input are now automatically cleared when the browser tab closes, reducing exposure via XSS or compromised browser profiles.

Changes:

• Replaced localStorage with sessionStorage in apps/web/src/stores/prompt-draft.ts
• Reduced draft expiry from 7 days to 24 hours (defensive measure for session-scoped storage)
• Updated comments in apps/web/src/hooks/use-prompt-draft.ts and apps/web/src/stores/prompt-draft.ts to reflect security rationale

Files modified:

• apps/web/src/hooks/use-prompt-draft.ts
• apps/web/src/stores/prompt-draft.ts

Fixes OSS-34

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

---

Summary by cubic

Switched prompt draft storage to sessionStorage to limit sensitive data exposure. Drafts clear on tab close, expire after 24h, and the models API enforces IP rate limits with a 10s timeout (OSS-34).

^{Written for commit 394083f. 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/prompt-draft.ts
• apps/web/src/hooks/use-prompt-draft.ts

[cubic-dev-ai]

14 issues found across 45 files

Confidence score: 2/5

• High risk due to workflow misuse: apps/web/src/routes/api/workflow/generate-title.ts makes the OpenRouter fetch() outside context.run(), which can re-run on each step and cause repeated external calls and inconsistent behavior.
• Security/data handling concerns: draft migration leaves sensitive data in localStorage (apps/web/src/stores/prompt-draft.ts) and decrypted auth tokens are persisted as workflow step results in apps/web/src/routes/api/workflow/delete-account.ts.
• Concurrency/data integrity risk in apps/server/convex/lib/upstashUsage.ts where the negative-balance floor check is split across pipelines, allowing a race between read and write.
• Pay close attention to apps/web/src/routes/api/workflow/generate-title.ts, apps/web/src/stores/prompt-draft.ts, apps/web/src/routes/api/workflow/delete-account.ts, apps/server/convex/lib/upstashUsage.ts - replays/retries, sensitive data persistence, and race conditions.

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` still exposes raw `parsedError.message` to the user via toast, while `editMessage` and `retryMessage` in this same PR were updated to use `getUserFriendlyError()`. This can leak internal error details (API structures, DB field names, etc.) to the end user.</violation>
</file>

<file name="apps/web/src/stores/prompt-draft.ts">

<violation number="1" location="apps/web/src/stores/prompt-draft.ts:123">
P1: Switching to `sessionStorage` doesn't remove previously stored drafts from `localStorage`. Existing sensitive draft data under the `"openchat-prompt-drafts"` key will persist indefinitely in `localStorage`, undermining the security improvement this PR intends to make. Add a one-time migration cleanup to remove the old `localStorage` entry.</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 `fetch()` call to the OpenRouter API is not wrapped in a workflow step (`context.run()` or `context.call()`). In Upstash Workflow, the endpoint is re-invoked for each step, and only operations inside `context.run()`/`context.call()` are memoized and skipped on replay. This bare fetch will be re-executed every time the workflow replays for subsequent steps (e.g., `save-title`), causing duplicate LLM calls and wasted API credits.

Use `context.call()` which is specifically designed for external HTTP requests in workflows — it offloads the call to Upstash and automatically handles retries and replay.</violation>
</file>

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

<violation number="1" location="apps/server/convex/chats.ts:571">
P3: Indentation of `body` (2 tabs) doesn't match its sibling properties `method` and `headers` (3 tabs) in the fetch options object. This makes it look like `body` is outside the options object. Align it with the other properties.</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 and inconsistent rate-limit detection: this uses string matching on the error message, while the same file (line 520, bulk-delete handler) and the server (`throwRateLimitError`) use the structured `error.name === "RateLimitError"` check. The message-based check will silently break if the server wording changes, and the hardcoded toast loses the dynamic retry-after time the server provides.</violation>
</file>

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

<violation number="1" location="apps/server/convex/lib/upstashUsage.ts:189">
P1: Race condition (TOCTOU): The negative-balance floor check reads the total from the first pipeline, then issues a separate `SET key 0` in a second pipeline. A concurrent `INCRBY` between the two pipelines would be silently wiped out, causing usage data loss. Consider using a Lua script via `EVAL` to atomically check-and-floor:

["EVAL", "local v=redis.call('INCRBY',KEYS[1],ARGV[1]) if v<0 then redis.call('SET',KEYS[1],0) end return v", 1, key, roundedDelta]

</file>

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

<violation number="1" location="apps/web/src/lib/server-auth.ts:82">
P1: `ALLOW_AUTH_COOKIE_FALLBACK` is defined and guarded against production use, but never actually checked in the fallback code path. The cookie-based JWT fallback is unconditionally available in dev/test, defeating the purpose of the opt-in env var. The fallback guard should also require `ALLOW_AUTH_COOKIE_FALLBACK` to be `true`.</violation>
</file>

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

<violation number="1" location="apps/server/convex/lib/sanitize.ts:44">
P2: Regex-based HTML stripping `/<[^>]*>/g` will corrupt legitimate titles containing angle brackets (e.g., `"Understanding template<T>"` → `"Understanding template"`). For an AI chat app with code-related conversations, this is a realistic data loss scenario. Consider encoding angle brackets instead of removing them (e.g., replacing `<` with `&lt;`) or using a proper HTML sanitization library that distinguishes tags from text.</violation>
</file>

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

<violation number="1" location="scripts/dev.ts:25">
P2: The local variable `process` shadows the global `process` object. Rename it to avoid confusion and potential bugs if this function is later extended (e.g., someone adding `process.env` access inside `run` would silently get the subprocess object, not the global).</violation>
</file>

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

<violation number="1" location="apps/server/convex/cleanupAction.ts:38">
P2: Validation mismatch: `Number.isFinite` allows non-integer values (e.g. `1.5`) that the downstream mutation rejects with `Number.isInteger`. Use `Number.isInteger` here to match the mutation's validation and catch invalid input at the correct boundary.</violation>
</file>

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

<violation number="1" location="apps/web/src/routes/api/workflow/delete-account.ts:204">
P2: The decrypted auth token is returned from `context.run("resolve-auth", ...)`, which causes Upstash Workflow to persist it in plain text as a step result for replay. This partially undermines the encrypt/`getdel`/TTL protections in `workflow-auth-token.ts`. Consider restructuring so the token isn't stored as a step result — e.g., re-resolve the token inside each `context.run` step that needs it, or pass it via workflow request headers instead of step return values.</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:120">
P2: `runExportChatInline` should validate that `userId` is defined before proceeding. The function currently trusts its caller to always provide a `userId`, but its type signature allows `undefined`. Add a guard at the top of the function.</violation>
</file>

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

<violation number="1" location="apps/server/convex/users.ts:870">
P2: `batchSize` is not forwarded for `delete-chat-read-statuses` and `delete-prompt-templates` cases, unlike all other deletion steps. This prevents callers from controlling the batch size for these two steps. Pass `batchSize: args.batchSize` for consistency.</violation>
</file>

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

<violation number="1" location="apps/server/convex/chats.test.ts:1287">
P2: This test doesn't mock `fetch`, relying on the implementation short-circuiting before the API call. If the implementation order changes, this test will make a real HTTP request or fail unexpectedly. Mock `fetch` and assert it was **not** called — this both prevents accidental network calls and documents the expected behavior.</violation>
</file>

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

[cubic-dev-ai]

P1: Switching to sessionStorage doesn't remove previously stored drafts from localStorage. Existing sensitive draft data under the "openchat-prompt-drafts" key will persist indefinitely in localStorage, undermining the security improvement this PR intends to make. Add a one-time migration cleanup to remove the old localStorage entry.

Prompt for AI agents

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

<comment>Switching to `sessionStorage` doesn't remove previously stored drafts from `localStorage`. Existing sensitive draft data under the `"openchat-prompt-drafts"` key will persist indefinitely in `localStorage`, undermining the security improvement this PR intends to make. Add a one-time migration cleanup to remove the old `localStorage` entry.</comment>

<file context>
@@ -115,7 +120,7 @@ export const usePromptDraftStore = create<PromptDraftState>()(
 			{
 				name: "openchat-prompt-drafts",
-				storage: createJSONStorage(() => localStorage),
+				storage: createJSONStorage(() => sessionStorage),
 			},
 		),
</file context>

[cubic-dev-ai]

P1: The fetch() call to the OpenRouter API is not wrapped in a workflow step (context.run() or context.call()). In Upstash Workflow, the endpoint is re-invoked for each step, and only operations inside context.run()/context.call() are memoized and skipped on replay. This bare fetch will be re-executed every time the workflow replays for subsequent steps (e.g., save-title), causing duplicate LLM calls and wasted API credits.

Use context.call() which is specifically designed for external HTTP requests in workflows — it offloads the call to Upstash and automatically handles retries and replay.

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 `fetch()` call to the OpenRouter API is not wrapped in a workflow step (`context.run()` or `context.call()`). In Upstash Workflow, the endpoint is re-invoked for each step, and only operations inside `context.run()`/`context.call()` are memoized and skipped on replay. This bare fetch will be re-executed every time the workflow replays for subsequent steps (e.g., `save-title`), causing duplicate LLM calls and wasted API credits.

Use `context.call()` which is specifically designed for external HTTP requests in workflows — it offloads the call to Upstash and automatically handles retries and replay.</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: Race condition (TOCTOU): The negative-balance floor check reads the total from the first pipeline, then issues a separate SET key 0 in a second pipeline. A concurrent INCRBY between the two pipelines would be silently wiped out, causing usage data loss. Consider using a Lua script via EVAL to atomically check-and-floor:

["EVAL", "local v=redis.call('INCRBY',KEYS[1],ARGV[1]) if v<0 then redis.call('SET',KEYS[1],0) end return v", 1, key, roundedDelta]

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

<comment>Race condition (TOCTOU): The negative-balance floor check reads the total from the first pipeline, then issues a separate `SET key 0` in a second pipeline. A concurrent `INCRBY` between the two pipelines would be silently wiped out, causing usage data loss. Consider using a Lua script via `EVAL` to atomically check-and-floor:

["EVAL", "local v=redis.call('INCRBY',KEYS[1],ARGV[1]) if v<0 then redis.call('SET',KEYS[1],0) end return v", 1, key, roundedDelta]

<file context>
@@ -0,0 +1,200 @@
+				: typeof total === "string"
+					? Number.parseInt(total, 10)
+					: null;
+		if (typeof parsedTotal === "number" && Number.isFinite(parsedTotal) && parsedTotal < 0) {
+			await executePipeline([
+				["SET", key, 0],
</file context>

[cubic-dev-ai]

P1: ALLOW_AUTH_COOKIE_FALLBACK is defined and guarded against production use, but never actually checked in the fallback code path. The cookie-based JWT fallback is unconditionally available in dev/test, defeating the purpose of the opt-in env var. The fallback guard should also require ALLOW_AUTH_COOKIE_FALLBACK to be true.

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 defined and guarded against production use, but never actually checked in the fallback code path. The cookie-based JWT fallback is unconditionally available in dev/test, defeating the purpose of the opt-in env var. The fallback guard should also require `ALLOW_AUTH_COOKIE_FALLBACK` to be `true`.</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: Inconsistent error handling: forkMessage still exposes raw parsedError.message to the user via toast, while editMessage and retryMessage in this same PR were updated to use getUserFriendlyError(). This can leak internal error details (API structures, DB field names, etc.) to the end user.

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` still exposes raw `parsedError.message` to the user via toast, while `editMessage` and `retryMessage` in this same PR were updated to use `getUserFriendlyError()`. This can leak internal error details (API structures, DB field names, etc.) to the end user.</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: The decrypted auth token is returned from context.run("resolve-auth", ...), which causes Upstash Workflow to persist it in plain text as a step result for replay. This partially undermines the encrypt/getdel/TTL protections in workflow-auth-token.ts. Consider restructuring so the token isn't stored as a step result — e.g., re-resolve the token inside each context.run step that needs it, or pass it via workflow request headers instead of step return values.

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/delete-account.ts, line 204:

<comment>The decrypted auth token is returned from `context.run("resolve-auth", ...)`, which causes Upstash Workflow to persist it in plain text as a step result for replay. This partially undermines the encrypt/`getdel`/TTL protections in `workflow-auth-token.ts`. Consider restructuring so the token isn't stored as a step result — e.g., re-resolve the token inside each `context.run` step that needs it, or pass it via workflow request headers instead of step return values.</comment>

<file context>
@@ -0,0 +1,387 @@
+		return EMPTY_DELETE_RESULT;
+	}
+
+	const authToken = await context.run("resolve-auth", async () => {
+		return getWorkflowAuthToken(authTokenRef);
+	});
</file context>

[cubic-dev-ai]

P2: runExportChatInline should validate that userId is defined before proceeding. The function currently trusts its caller to always provide a userId, but its type signature allows undefined. Add a guard at the top of the function.

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

<comment>`runExportChatInline` should validate that `userId` is defined before proceeding. The function currently trusts its caller to always provide a `userId`, but its type signature allows `undefined`. Add a guard at the top of the function.</comment>

<file context>
@@ -0,0 +1,226 @@
+	payload: ExportChatPayload,
+	authToken: string,
+): Promise<{ downloadUrl: string; byteLength: number; fileName: string }> {
+	const { chatId, userId, format = "markdown" } = payload;
+	const convexClient = createConvexServerClient(authToken);
+	const chatExportData = await convexClient.query(api.chats.getChatExportData, {
</file context>

[cubic-dev-ai]

P2: batchSize is not forwarded for delete-chat-read-statuses and delete-prompt-templates cases, unlike all other deletion steps. This prevents callers from controlling the batch size for these two steps. Pass batchSize: args.batchSize 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 for `delete-chat-read-statuses` and `delete-prompt-templates` cases, unlike all other deletion steps. This prevents callers from controlling the batch size for these two steps. Pass `batchSize: args.batchSize` 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]

P2: This test doesn't mock fetch, relying on the implementation short-circuiting before the API call. If the implementation order changes, this test will make a real HTTP request or fail unexpectedly. Mock fetch and assert it was not called — this both prevents accidental network calls and documents the expected behavior.

Prompt for AI agents

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

<comment>This test doesn't mock `fetch`, relying on the implementation short-circuiting before the API call. If the implementation order changes, this test will make a real HTTP request or fail unexpectedly. Mock `fetch` and assert it was **not** called — this both prevents accidental network calls and documents the expected behavior.</comment>

<file context>
@@ -1111,3 +1111,194 @@ describe('chats.checkExportRateLimit', () => {
+		expect(chat?.title).toBe('Helpful Testing Title');
+	});
+
+	it('does not overwrite existing custom titles unless forced', async () => {
+		await t.run(async (ctx) => {
+			await ctx.db.patch(chatId, { title: 'Custom Existing Title' });
</file context>

[cubic-dev-ai]

P3: Indentation of body (2 tabs) doesn't match its sibling properties method and headers (3 tabs) in the fetch options object. This makes it look like body is outside the options object. Align it with the other properties.

Prompt for AI agents

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

<comment>Indentation of `body` (2 tabs) doesn't match its sibling properties `method` and `headers` (3 tabs) in the fetch options object. This makes it look like `body` is outside the options object. Align it with the other properties.</comment>

<file context>
@@ -526,6 +526,87 @@ const TITLE_STYLE_PROMPTS: Record<"short" | "standard" | "long", string> = {
+				"HTTP-Referer": process.env.CONVEX_SITE_URL || "https://osschat.io",
+				"X-Title": "OSSChat",
+			},
+		body: JSON.stringify({
+			model: TITLE_MODEL_ID,
+			messages: [
</file context>

Suggested change

	body: JSON.stringify({
	body: JSON.stringify({

[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:54am