Fix X-Forwarded-For bypass

[tembo]

Summary

Fixed rate limiting bypass vulnerability when TRUST_PROXY=true by:

• Added IP validation (isValidIp()) using IPv4/IPv6 regex to reject malformed or spoofed values
• Enhanced TRUST_PROXY=true mode to prefer platform-specific headers (cf-connecting-ip, x-vercel-forwarded-for) before falling back to generic X-Forwarded-For
• Applied IP validation to all proxy modes (cloudflare, vercel, true)
• Updated warning message to recommend secure platform-specific modes over generic TRUST_PROXY=true

Files modified:

• apps/web/src/routes/api/models.ts

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

---

Summary by cubic

Fix rate-limit bypass in /api/models by validating client IPs and gating extraction to trusted proxy modes. Blocks spoofed X-Forwarded-For and rejects requests when IP is missing or headers are untrusted.

• Bug Fixes
  • Validate IPv4/IPv6 with isValidIp() across cloudflare, vercel, and true modes.
  • In TRUST_PROXY=true, prefer cf-connecting-ip and x-vercel-forwarded-for; only fall back to X-Forwarded-For when valid.
  • Fail closed when TRUST_PROXY is unset or invalid, with warnings to use platform-specific modes.

^{Written for commit 782c70c. 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/routes/api/models.ts

[cubic-dev-ai]

17 issues found across 43 files

Confidence score: 2/5

• High-risk behavioral bug: apps/web/src/routes/api/workflow/generate-title.ts runs the OpenRouter fetch outside context.run(), so it may execute multiple times per workflow step and cause repeated external calls.
• Security/accuracy concerns in apps/web/src/routes/api/models.ts: client-controlled IP headers can be trusted behind generic proxies, and IPv4-mapped IPv6 addresses are rejected, which can break rate-limiting or access checks.
• Multiple medium issues (race conditions and accounting leaks) in apps/server/convex/lib/upstashUsage.ts and apps/server/convex/backgroundStream.ts increase regression risk around usage tracking and refunds.
• Pay close attention to apps/web/src/routes/api/workflow/generate-title.ts, apps/web/src/routes/api/models.ts, apps/server/convex/lib/upstashUsage.ts, apps/server/convex/backgroundStream.ts - repeated execution, IP trust/format handling, atomicity, and refund leakage.

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/routes/api/models.ts">

<violation number="1" location="apps/web/src/routes/api/models.ts:14">
P1: IPv4-mapped IPv6 addresses (e.g., `::ffff:192.168.1.1`) are rejected because the regex doesn't include `.` in the character class. These are common in practice — Node.js and many proxies report IPv4 addresses in this format. Affected users would get 400 errors.

Include `.` in the character class to support IPv4-mapped/embedded addresses.</violation>

<violation number="2" location="apps/web/src/routes/api/models.ts:99">
P1: When behind a generic reverse proxy (not Cloudflare/Vercel), `cf-connecting-ip` and `x-vercel-forwarded-for` are **not** stripped by the proxy and are fully client-controlled, while `x-forwarded-for` is typically managed by the proxy. Preferring these headers over `x-forwarded-for` in `TRUST_PROXY=true` mode actually introduces a *new* spoofing vector — an attacker can set `cf-connecting-ip` to rotate through arbitrary IPs and bypass rate limiting.

Consider only using `x-forwarded-for` in `TRUST_PROXY=true` mode, or at minimum, falling back to `x-forwarded-for` *first* and only use platform-specific headers when the corresponding platform mode is selected.</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 in production but never actually checked in the fallback path. The cookie fallback is unconditionally enabled in local dev (`IS_LOCAL_DEV`), making this env var a no-op. The condition should also require `ALLOW_AUTH_COOKIE_FALLBACK` to be true.</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: the server explicitly sets `error.name = "RateLimitError"` for this purpose. Matching on message content is brittle and will break if the server message format changes. Consider reverting to the name-based check, or at minimum matching on `"too many"` as a broader pattern.</violation>

<violation number="2" location="apps/web/src/components/app-sidebar.tsx:455">
P2: Hardcoded toast message loses the server-provided retry-after wait time. The server includes a specific duration (e.g., "in 30 seconds") in the error message, but this code discards it in favor of a generic "Please try again later." Consider using the actual error message: `toast.error(message)`.</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 whitespace collapsing, so removing tags like `<b> </b>` can leave multiple consecutive spaces in the sanitized title. Move this line before the whitespace collapse step, or re-collapse spaces afterward.</violation>
</file>

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

<violation number="1" location="apps/server/convex/cleanupAction.ts:38">
P2: Validation uses `Number.isFinite()` but the downstream mutation (`cleanupSoftDeletedRecords`) validates with `Number.isInteger()`. This allows fractional values (e.g. `1.5`) to pass action-level validation, only to fail inside the mutation. Use `Number.isInteger()` here to match the inner validation and fail early with a clear error.</violation>

<violation number="2" location="apps/server/convex/cleanupAction.ts:42">
P2: Same `Number.isFinite()` vs `Number.isInteger()` mismatch for `batchSize`. Use `Number.isInteger()` to match the downstream mutation's validation.</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 indent while unchanged lines in the same object block use 8-space indent. This creates a visible misalignment between state properties and action methods within the same `(set) => ({...})` callback.</violation>
</file>

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

<violation number="1" location="apps/web/src/lib/server-auth.test.ts:60">
P2: Weak test: the fallback cookie `fallback-token` is not a valid JWT, so this test would still pass even if the 4xx early-return were removed — the fallback path would also return `null` due to `isJwtNotExpired()` failing. Use `createJwt()` to create a valid fallback JWT (like the other fallback tests do), so the test truly verifies that a 401 prevents fallback.</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 handling: `forkMessage` still exposes raw `parsedError.message` to the user, while `editMessage` and `retryMessage` were updated in this PR to use `getUserFriendlyError()`. Apply the same sanitization here to avoid leaking internal error details.</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 negative deltas silently drops small adjustments. For example, `Math.ceil(-0.5)` → `0`, which triggers the `roundedDelta === 0` early return, discarding the adjustment entirely. Consider using `Math.round` or rounding away from zero (e.g., `Math.sign(delta) * Math.ceil(Math.abs(delta))`) to ensure small negative adjustments aren't lost.</violation>

<violation number="2" location="apps/server/convex/lib/upstashUsage.ts:189">
P2: Race condition: the negative-total floor check and the subsequent `SET key 0` are not atomic. A concurrent `INCRBY` between the first pipeline response and the second pipeline execution could be overwritten by the `SET 0`, silently losing that increment. Consider using a Lua script (via Upstash `EVAL`) to make the check-and-set atomic.</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:246">
P1: The OpenRouter `fetch` call and its response handling are outside any `context.run()` step. In Upstash Workflow, the endpoint is re-invoked for every step in the run, and code outside `context.run()` executes every time. This means this fetch will fire redundantly on every step replay (e.g., when "save-title" runs), wasting OpenRouter API credits and compute.

The codebase's own `cleanup.ts` workflow correctly wraps its fetch calls inside `context.run()`. Consider either:
1. Wrapping this block in `context.run("call-llm", async () => { ... })` to make it a durable step, or
2. Using `context.call()` which offloads the HTTP call to Upstash infrastructure entirely.</violation>
</file>

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

<violation number="1" location="apps/server/convex/users.ts:871">
P2: `batchSize` is not forwarded to `deleteUserChatReadStatuses` and `deleteUserPromptTemplates`, unlike all other step cases. The caller passes `batchSize` for every step, but these two silently ignore it and always use the default (100). Add `batchSize: args.batchSize` for consistency.</violation>
</file>

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

<violation number="1" location="apps/web/src/lib/redis.ts:62">
P2: `parseStreamMeta`: The string branch returns `JSON.parse(value) as StreamMeta` without validating the parsed result, but the object branch performs thorough field validation. This inconsistency means corrupted JSON in Redis could return an invalid `StreamMeta`. Consider reusing the object validation for the parsed JSON result.</violation>
</file>

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

<violation number="1" location="apps/server/convex/backgroundStream.ts:815">
P2: Reserved Upstash usage (1 cent) is leaked when the "No API key available" early return is hit. The refund logic is only in the try/catch block which starts after this return. Add a refund before returning.</violation>
</file>

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

[cubic-dev-ai]

P1: IPv4-mapped IPv6 addresses (e.g., ::ffff:192.168.1.1) are rejected because the regex doesn't include . in the character class. These are common in practice — Node.js and many proxies report IPv4 addresses in this format. Affected users would get 400 errors.

Include . in the character class to support IPv4-mapped/embedded addresses.

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/models.ts, line 14:

<comment>IPv4-mapped IPv6 addresses (e.g., `::ffff:192.168.1.1`) are rejected because the regex doesn't include `.` in the character class. These are common in practice — Node.js and many proxies report IPv4 addresses in this format. Affected users would get 400 errors.

Include `.` in the character class to support IPv4-mapped/embedded addresses.</comment>

<file context>
@@ -0,0 +1,194 @@
+
+// Basic IPv4 and IPv6 validation to reject obviously spoofed or malformed values.
+const IPV4_REGEX = /^(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)$/;
+const IPV6_REGEX = /^[\da-fA-F:]+$/;
+
+function isValidIp(value: string): boolean {
</file context>

[cubic-dev-ai]

P1: The OpenRouter fetch call and its response handling are outside any context.run() step. In Upstash Workflow, the endpoint is re-invoked for every step in the run, and code outside context.run() executes every time. This means this fetch will fire redundantly on every step replay (e.g., when "save-title" runs), wasting OpenRouter API credits and compute.

The codebase's own cleanup.ts workflow correctly wraps its fetch calls inside context.run(). Consider either:

1. Wrapping this block in context.run("call-llm", async () => { ... }) to make it a durable step, or
2. Using context.call() which offloads the HTTP call to Upstash infrastructure entirely.

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

<comment>The OpenRouter `fetch` call and its response handling are outside any `context.run()` step. In Upstash Workflow, the endpoint is re-invoked for every step in the run, and code outside `context.run()` executes every time. This means this fetch will fire redundantly on every step replay (e.g., when "save-title" runs), wasting OpenRouter API credits and compute.

The codebase's own `cleanup.ts` workflow correctly wraps its fetch calls inside `context.run()`. Consider either:
1. Wrapping this block in `context.run("call-llm", async () => { ... })` to make it a durable step, or
2. Using `context.call()` which offloads the HTTP call to Upstash infrastructure entirely.</comment>

<file context>
@@ -0,0 +1,438 @@
+		TITLE_STYLE_PROMPTS[length],
+	].join(" ");
+
+	let llmResponseStatus = 0;
+	let llmResponseBody: { choices?: Array<{ message?: { content?: string } }> } | null = null;
+	try {
</file context>

[cubic-dev-ai]

P2: ALLOW_AUTH_COOKIE_FALLBACK is defined and guarded in production but never actually checked in the fallback path. The cookie fallback is unconditionally enabled in local dev (IS_LOCAL_DEV), making this env var a no-op. The condition 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 in production but never actually checked in the fallback path. The cookie fallback is unconditionally enabled in local dev (`IS_LOCAL_DEV`), making this env var a no-op. The condition 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: Fragile string-based error detection: the server explicitly sets error.name = "RateLimitError" for this purpose. Matching on message content is brittle and will break if the server message format changes. Consider reverting to the name-based check, or at minimum matching on "too many" as a broader pattern.

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-based error detection: the server explicitly sets `error.name = "RateLimitError"` for this purpose. Matching on message content is brittle and will break if the server message format changes. Consider reverting to the name-based check, or at minimum matching on `"too many"` as a broader pattern.</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>

Suggested change

	if (message.toLowerCase().includes("too many title generations")) {
	if (error instanceof Error && error.name === "RateLimitError") {

[cubic-dev-ai]

P2: Math.ceil on negative deltas silently drops small adjustments. For example, Math.ceil(-0.5) → 0, which triggers the roundedDelta === 0 early return, discarding the adjustment entirely. Consider using Math.round or rounding away from zero (e.g., Math.sign(delta) * Math.ceil(Math.abs(delta))) to ensure small negative adjustments aren't lost.

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 negative deltas silently drops small adjustments. For example, `Math.ceil(-0.5)` → `0`, which triggers the `roundedDelta === 0` early return, discarding the adjustment entirely. Consider using `Math.round` or rounding away from zero (e.g., `Math.sign(delta) * Math.ceil(Math.abs(delta))`) to ensure small negative adjustments aren't lost.</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]

P2: batchSize is not forwarded to deleteUserChatReadStatuses and deleteUserPromptTemplates, unlike all other step cases. The caller passes batchSize for every step, but these two silently ignore it and always use the default (100). Add 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 871:

<comment>`batchSize` is not forwarded to `deleteUserChatReadStatuses` and `deleteUserPromptTemplates`, unlike all other step cases. The caller passes `batchSize` for every step, but these two silently ignore it and always use the default (100). Add `batchSize: args.batchSize` for consistency.</comment>

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

Suggested change

	return await ctx.runMutation(internal.users.deleteUserChatReadStatuses, {
	userId,
	});
	return await ctx.runMutation(internal.users.deleteUserChatReadStatuses, {
	userId,
	batchSize: args.batchSize,
	});

[cubic-dev-ai]

P2: parseStreamMeta: The string branch returns JSON.parse(value) as StreamMeta without validating the parsed result, but the object branch performs thorough field validation. This inconsistency means corrupted JSON in Redis could return an invalid StreamMeta. Consider reusing the object validation for the parsed JSON result.

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

<comment>`parseStreamMeta`: The string branch returns `JSON.parse(value) as StreamMeta` without validating the parsed result, but the object branch performs thorough field validation. This inconsistency means corrupted JSON in Redis could return an invalid `StreamMeta`. Consider reusing the object validation for the parsed JSON result.</comment>

<file context>
@@ -80,14 +32,53 @@ export interface StreamMeta {
+	if (!value) return null;
+	if (typeof value === "string") {
+		try {
+			return JSON.parse(value) as StreamMeta;
+		} catch {
+			return null;
</file context>

[cubic-dev-ai]

P3: Inconsistent indentation: the modified lines use 9-space indent while unchanged lines in the same object block use 8-space indent. This creates a visible misalignment between state properties and action methods within the same (set) => ({...}) callback.

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 indent while unchanged lines in the same object block use 8-space indent. This creates a visible misalignment between state properties and action methods within the same `(set) => ({...})` callback.</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