Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
301 changes: 301 additions & 0 deletions ratelimit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,301 @@
// IP Rate Limiting System with VPN/Internal Subnet Detection
// Targets Carrier Grade NAT (CGNAT) 100.64.0.0/10 and other VPN/internal ranges

/**
* CIDR Block Configuration
* Allows configuration via environment variables with defaults for common VPN/internal subnets
*/
function getBlockedSubnets(): string[] {
const envSubnets = process.env.RATELIMIT_SUBNETS;
if (envSubnets) {
return envSubnets.split(",").map((s) => s.trim());
}
// Default: CGNAT range (RFC 6598) - 100.64.0.0/10
// Common VPN internal ranges
return [
"100.64.0.0/10", // CGNAT (RFC 6598)
"10.0.0.0/8", // RFC 1918 private
"172.16.0.0/12", // RFC 1918 private
"192.168.0.0/16", // RFC 1918 private
"fc00::/7", // IPv6 unique local
];
}

/**
* Parse CIDR notation into network address and prefix length
*/
function parseCidr(cidr: string): { network: bigint; mask: bigint; isV6: boolean } | null {
const [ipStr, prefixStr] = cidr.split("/");
if (!ipStr || !prefixStr) return null;

const prefix = parseInt(prefixStr, 10);
if (!Number.isFinite(prefix) || prefix < 0) return null;

// IPv6
if (ipStr.includes(":")) {
if (prefix > 128) return null;
const ip = parseIPv6(ipStr);
if (ip === null) return null;
const mask = prefix === 0 ? 0n : (~0n << (128n - BigInt(prefix))) & ((1n << 128n) - 1n);
return { network: ip & mask, mask, isV6: true };
}

// IPv4
if (prefix > 32) return null;
const ip = parseIPv4(ipStr);
if (ip === null) return null;
const mask = prefix === 0 ? 0n : (~0n << (32n - BigInt(prefix))) & 0xffffffffn;
return { network: ip & mask, mask, isV6: false };
}

/**
* Parse IPv4 address string to bigint
*/
function parseIPv4(ip: string): bigint | null {
const parts = ip.split(".");
if (parts.length !== 4) return null;

let result = 0n;
for (const part of parts) {
const num = parseInt(part, 10);
if (!Number.isFinite(num) || num < 0 || num > 255) return null;
result = (result << 8n) | BigInt(num);
}
return result;
}

/**
* Parse IPv6 address string to bigint
*/
function parseIPv6(ip: string): bigint | null {
// Handle :: abbreviation
const parts = ip.split(":");
const emptyIndex = parts.findIndex((p) => p === "");

// Expand :: to full 8 groups
const expanded: string[] = [];
if (emptyIndex !== -1 && parts.length < 8) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High ratelimit.ts:77

IPv6 addresses ending with :: (like fc00::) fail to parse. Splitting fc00:: produces ['fc00', '', ''], so the trailing empty string at index 2 is skipped, leaving only 7 groups. Consider filtering empty strings from the non-expansion path or handling trailing :: specially.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file ratelimit.ts around line 77:

IPv6 addresses ending with `::` (like `fc00::`) fail to parse. Splitting `fc00::` produces `['fc00', '', '']`, so the trailing empty string at index 2 is skipped, leaving only 7 groups. Consider filtering empty strings from the non-expansion path or handling trailing `::` specially.

Evidence trail:
ratelimit.ts lines 70-92 at commit a9d7b346d257a76be864164802ace2ce7b2b0a8f. The parseIPv6 function at line 72 splits by `:`, line 73 finds first empty index, lines 77-87 expand `::` but skip non-emptyIndex empty strings (line 84-86), resulting in only 7 groups for input `fc00::`.

const missing = 8 - parts.length + 1;
for (let i = 0; i < parts.length; i++) {
if (parts[i] === "" && i === emptyIndex) {
for (let j = 0; j < missing; j++) {
expanded.push("0");
}
} else if (parts[i] !== "") {
expanded.push(parts[i]);
}
}
} else {
expanded.push(...parts.filter((p) => p !== ""));
}

if (expanded.length !== 8) return null;

let result = 0n;
for (const part of expanded) {
const num = parseInt(part, 16);
if (!Number.isFinite(num) || num < 0 || num > 0xffff) return null;
result = (result << 16n) | BigInt(num);
}
return result;
}
Comment on lines +70 to +101
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

parseIPv6 incorrectly handles :: at the start or end of an address, breaking the default fc00::/7 subnet.

When :: appears at the start or end (e.g., "fc00::", "::1"), "fc00::".split(":") produces ["fc00", "", ""]. The code tracks only emptyIndex (the first ""), fills zeros there, then silently drops the second adjacent empty string with else if (parts[i] !== ""). This produces 7 groups instead of 8, causing parseIPv6 to return null. As a result, parseCidr("fc00::/7") returns null, ipInCidr returns false, and IPv6 ULA addresses are never rate-limited — the entire fc00::/7 default subnet is a no-op.

The fix is to split on "::" (two characters) instead of ":":

🐛 Proposed fix for parseIPv6
 function parseIPv6(ip: string): bigint | null {
-  // Handle :: abbreviation
-  const parts = ip.split(":");
-  const emptyIndex = parts.findIndex((p) => p === "");
-
-  // Expand :: to full 8 groups
-  const expanded: string[] = [];
-  if (emptyIndex !== -1 && parts.length < 8) {
-    const missing = 8 - parts.length + 1;
-    for (let i = 0; i < parts.length; i++) {
-      if (parts[i] === "" && i === emptyIndex) {
-        for (let j = 0; j < missing; j++) {
-          expanded.push("0");
-        }
-      } else if (parts[i] !== "") {
-        expanded.push(parts[i]);
-      }
-    }
-  } else {
-    expanded.push(...parts.filter((p) => p !== ""));
-  }
-
-  if (expanded.length !== 8) return null;
+  // Split on "::" to correctly handle leading, middle, and trailing abbreviations
+  const halves = ip.split("::");
+  if (halves.length > 2) return null; // More than one "::" is invalid
+
+  let groups: string[];
+  if (halves.length === 2) {
+    const left = halves[0] ? halves[0].split(":") : [];
+    const right = halves[1] ? halves[1].split(":") : [];
+    const zerosNeeded = 8 - left.length - right.length;
+    if (zerosNeeded < 0) return null;
+    groups = [...left, ...Array(zerosNeeded).fill("0"), ...right];
+  } else {
+    groups = ip.split(":");
+  }
+
+  if (groups.length !== 8) return null;

-  let result = 0n;
-  for (const part of expanded) {
+  let result = 0n;
+  for (const part of groups) {
     const num = parseInt(part, 16);
     if (!Number.isFinite(num) || num < 0 || num > 0xffff) return null;
     result = (result << 16n) | BigInt(num);
   }
   return result;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function parseIPv6(ip: string): bigint | null {
// Handle :: abbreviation
const parts = ip.split(":");
const emptyIndex = parts.findIndex((p) => p === "");
// Expand :: to full 8 groups
const expanded: string[] = [];
if (emptyIndex !== -1 && parts.length < 8) {
const missing = 8 - parts.length + 1;
for (let i = 0; i < parts.length; i++) {
if (parts[i] === "" && i === emptyIndex) {
for (let j = 0; j < missing; j++) {
expanded.push("0");
}
} else if (parts[i] !== "") {
expanded.push(parts[i]);
}
}
} else {
expanded.push(...parts.filter((p) => p !== ""));
}
if (expanded.length !== 8) return null;
let result = 0n;
for (const part of expanded) {
const num = parseInt(part, 16);
if (!Number.isFinite(num) || num < 0 || num > 0xffff) return null;
result = (result << 16n) | BigInt(num);
}
return result;
}
function parseIPv6(ip: string): bigint | null {
// Split on "::" to correctly handle leading, middle, and trailing abbreviations
const halves = ip.split("::");
if (halves.length > 2) return null; // More than one "::" is invalid
let groups: string[];
if (halves.length === 2) {
const left = halves[0] ? halves[0].split(":") : [];
const right = halves[1] ? halves[1].split(":") : [];
const zerosNeeded = 8 - left.length - right.length;
if (zerosNeeded < 0) return null;
groups = [...left, ...Array(zerosNeeded).fill("0"), ...right];
} else {
groups = ip.split(":");
}
if (groups.length !== 8) return null;
let result = 0n;
for (const part of groups) {
const num = parseInt(part, 16);
if (!Number.isFinite(num) || num < 0 || num > 0xffff) return null;
result = (result << 16n) | BigInt(num);
}
return result;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ratelimit.ts` around lines 70 - 101, The parseIPv6 function fails for
leading/trailing :: because it splits on ":" and drops adjacent empty segments;
fix by detecting and splitting on the literal "::" instead: if the IP contains
"::", split into left and right halves at "::", split each half by ":" (allowing
empty halves), compute how many zero groups to insert to reach 8 total groups,
then concatenate left groups + inserted "0" groups + right groups; validate that
the final array has exactly 8 hex groups and then parse each into a 16-bit value
as before (references: parseIPv6, expanded/result variables).


/**
* Check if an IP address is within a CIDR block
*/
function ipInCidr(ip: string, cidr: string): boolean {
const cidrInfo = parseCidr(cidr);
if (!cidrInfo) return false;

const { network, mask, isV6 } = cidrInfo;

// Parse the IP to check
const ipNum = isV6 ? parseIPv6(ip) : parseIPv4(ip);
if (ipNum === null) return false;

return (ipNum & mask) === network;
}

/**
* Check if an IP is in any of the blocked subnets
*/
export function isVpnOrInternalIp(ip: string): boolean {
// Skip invalid IPs
if (!ip || ip === "unknown" || ip === "127.0.0.1" || ip === "::1") {
return false;
}

const subnets = getBlockedSubnets();
for (const subnet of subnets) {
try {
if (ipInCidr(ip, subnet)) {
return true;
}
} catch {
// Invalid subnet format, skip
}
}
return false;
}

/**
* Rate Limiter Configuration for VPN/Internal IPs
*/
export interface VpnRateLimitConfig {
// Much stricter limits for VPN/internal IPs
vpnWindowMs: number;
vpnMaxRequests: number;
// Standard limits for regular IPs (passed through to existing rate limiter)
standardWindowMs: number;
standardMaxRequests: number;
}

export function getVpnRateLimitConfig(): VpnRateLimitConfig {
return {
vpnWindowMs: parsePositiveInt(process.env.VPN_RATELIMIT_WINDOW_MS, 60_000),
vpnMaxRequests: parsePositiveInt(process.env.VPN_RATELIMIT_MAX_REQ, 5),
standardWindowMs: 60_000,
standardMaxRequests: 60,
};
}

function parsePositiveInt(value: string | undefined, fallback: number): number {
const parsed = Number.parseInt(value ?? "", 10);
return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback;
}

/**
* Extended rate limiter that applies stricter limits to VPN/internal IPs
*/
export class VpnAwareRateLimiter {
private windows = new Map<string, number[]>();
private lastSweep = 0;
private config: VpnRateLimitConfig;

constructor(config?: Partial<VpnRateLimitConfig>) {
this.config = { ...getVpnRateLimitConfig(), ...config };
}

/**
* Check if a request should be rate limited
* Returns true if the request should be blocked
*/
isRateLimited(ip: string, isVpn: boolean): boolean {
const now = Date.now();
const key = isVpn ? `vpn:${ip}` : `std:${ip}`;
const windowMs = isVpn ? this.config.vpnWindowMs : this.config.standardWindowMs;
const maxRequests = isVpn ? this.config.vpnMaxRequests : this.config.standardMaxRequests;

// Periodic cleanup of old entries
if (now - this.lastSweep >= windowMs) {
this.sweep(now, windowMs);
}

const existing = this.windows.get(key) ?? [];
const recent = existing.filter((timestamp) => now - timestamp <= windowMs);

if (recent.length >= maxRequests) {
this.windows.set(key, recent);
return true;
}

recent.push(now);
this.windows.set(key, recent);
return false;
}

/**
* Get current request count for an IP
*/
getRequestCount(ip: string, isVpn: boolean): number {
const now = Date.now();
const key = isVpn ? `vpn:${ip}` : `std:${ip}`;
const windowMs = isVpn ? this.config.vpnWindowMs : this.config.standardWindowMs;

const existing = this.windows.get(key) ?? [];
return existing.filter((timestamp) => now - timestamp <= windowMs).length;
}

/**
* Clean up old entries to prevent memory leaks
*/
private sweep(now: number, windowMs: number): void {
for (const [key, timestamps] of this.windows) {
const recent = timestamps.filter((timestamp) => now - timestamp <= windowMs);
Comment on lines +222 to +224
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High ratelimit.ts:222

The sweep method uses the caller's windowMs to filter all entries, but VPN and standard IPs have different windows. A VPN request (short window) would incorrectly purge valid timestamps from standard IP entries (longer window). Consider checking each key's prefix to apply the correct window size during sweep.

-  private sweep(now: number, windowMs: number): void {
+  private sweep(now: number, _windowMs: number): void {
     for (const [key, timestamps] of this.windows) {
+      const entryWindowMs = key.startsWith("vpn:") ? this.config.vpnWindowMs : this.config.standardWindowMs;
-      const recent = timestamps.filter((timestamp) => now - timestamp <= windowMs);
+      const recent = timestamps.filter((timestamp) => now - timestamp <= entryWindowMs);
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file ratelimit.ts around lines 222-224:

The `sweep` method uses the caller's `windowMs` to filter all entries, but VPN and standard IPs have different windows. A VPN request (short window) would incorrectly purge valid timestamps from standard IP entries (longer window). Consider checking each key's prefix to apply the correct window size during sweep.

Evidence trail:
ratelimit.ts lines 183-192 (isRateLimited method showing different windowMs for VPN vs standard), lines 222-230 (sweep method applying single windowMs to all entries), lines 153-159 (config showing vpnWindowMs is configurable while standardWindowMs is hardcoded), lines 185, 212 (VPN and standard use different key prefixes but stored in same map)

if (recent.length === 0) {
this.windows.delete(key);
} else {
this.windows.set(key, recent);
}
}
this.lastSweep = now;
}

/**
* Get stats for monitoring
*/
getStats(): { totalEntries: number; vpnEntries: number; standardEntries: number } {
let vpnEntries = 0;
let standardEntries = 0;
for (const key of this.windows.keys()) {
if (key.startsWith("vpn:")) {
vpnEntries++;
} else {
standardEntries++;
}
}
return {
totalEntries: this.windows.size,
vpnEntries,
standardEntries,
};
}

/**
* Clear all rate limit data
*/
clear(): void {
this.windows.clear();
this.lastSweep = 0;
}
}

/**
* Middleware-style rate limit check that can be used in fetch handlers
* Returns a Response if rate limited, null if allowed
*/
export function checkVpnRateLimit(
ip: string,
limiter: VpnAwareRateLimiter,
log?: (level: string, component: string, message: string, meta?: Record<string, unknown>) => void,
): Response | null {
const isVpn = isVpnOrInternalIp(ip);
const isLimited = limiter.isRateLimited(ip, isVpn);

if (isLimited) {
const count = limiter.getRequestCount(ip, isVpn);
if (log) {
log("WARN", "ratelimit", `Rate limited ${isVpn ? "VPN/internal" : "regular"} IP`, {
ip,
isVpn,
count,
});
}
return new Response(
JSON.stringify({
error: "Too Many Requests",
retryAfter: Math.ceil(getVpnRateLimitConfig().vpnWindowMs / 1000),
}),
{
status: 429,
headers: {
"Content-Type": "application/json",
"Retry-After": String(Math.ceil(getVpnRateLimitConfig().vpnWindowMs / 1000)),
"Cache-Control": "no-store",
},
},
);
}

return null;
}
Comment on lines +267 to +301
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Retry-After always uses vpnWindowMs even when a standard IP is rate-limited, and getVpnRateLimitConfig() is called twice.

isVpn is already resolved in this function. The 429 response body and Retry-After header should reflect the actual window that applies to the IP type. If VPN_RATELIMIT_WINDOW_MS is configured shorter than WINDOW_MS, a standard IP that gets rate-limited will receive a Retry-After value that is too short and will keep retrying prematurely. The function also calls getVpnRateLimitConfig() redundantly on both lines 287 and 293.

♻️ Proposed fix
 export function checkVpnRateLimit(
   ip: string,
   limiter: VpnAwareRateLimiter,
   log?: (level: string, component: string, message: string, meta?: Record<string, unknown>) => void,
 ): Response | null {
   const isVpn = isVpnOrInternalIp(ip);
   const isLimited = limiter.isRateLimited(ip, isVpn);

   if (isLimited) {
     const count = limiter.getRequestCount(ip, isVpn);
+    const config = getVpnRateLimitConfig();
+    const windowMs = isVpn ? config.vpnWindowMs : config.standardWindowMs;
+    const retryAfterSecs = Math.ceil(windowMs / 1000);
     if (log) {
       log("WARN", "ratelimit", `Rate limited ${isVpn ? "VPN/internal" : "regular"} IP`, {
         ip,
         isVpn,
         count,
       });
     }
     return new Response(
       JSON.stringify({
         error: "Too Many Requests",
-        retryAfter: Math.ceil(getVpnRateLimitConfig().vpnWindowMs / 1000),
+        retryAfter: retryAfterSecs,
       }),
       {
         status: 429,
         headers: {
           "Content-Type": "application/json",
-          "Retry-After": String(Math.ceil(getVpnRateLimitConfig().vpnWindowMs / 1000)),
+          "Retry-After": String(retryAfterSecs),
           "Cache-Control": "no-store",
         },
       },
     );
   }

   return null;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function checkVpnRateLimit(
ip: string,
limiter: VpnAwareRateLimiter,
log?: (level: string, component: string, message: string, meta?: Record<string, unknown>) => void,
): Response | null {
const isVpn = isVpnOrInternalIp(ip);
const isLimited = limiter.isRateLimited(ip, isVpn);
if (isLimited) {
const count = limiter.getRequestCount(ip, isVpn);
if (log) {
log("WARN", "ratelimit", `Rate limited ${isVpn ? "VPN/internal" : "regular"} IP`, {
ip,
isVpn,
count,
});
}
return new Response(
JSON.stringify({
error: "Too Many Requests",
retryAfter: Math.ceil(getVpnRateLimitConfig().vpnWindowMs / 1000),
}),
{
status: 429,
headers: {
"Content-Type": "application/json",
"Retry-After": String(Math.ceil(getVpnRateLimitConfig().vpnWindowMs / 1000)),
"Cache-Control": "no-store",
},
},
);
}
return null;
}
export function checkVpnRateLimit(
ip: string,
limiter: VpnAwareRateLimiter,
log?: (level: string, component: string, message: string, meta?: Record<string, unknown>) => void,
): Response | null {
const isVpn = isVpnOrInternalIp(ip);
const isLimited = limiter.isRateLimited(ip, isVpn);
if (isLimited) {
const count = limiter.getRequestCount(ip, isVpn);
const config = getVpnRateLimitConfig();
const windowMs = isVpn ? config.vpnWindowMs : config.standardWindowMs;
const retryAfterSecs = Math.ceil(windowMs / 1000);
if (log) {
log("WARN", "ratelimit", `Rate limited ${isVpn ? "VPN/internal" : "regular"} IP`, {
ip,
isVpn,
count,
});
}
return new Response(
JSON.stringify({
error: "Too Many Requests",
retryAfter: retryAfterSecs,
}),
{
status: 429,
headers: {
"Content-Type": "application/json",
"Retry-After": String(retryAfterSecs),
"Cache-Control": "no-store",
},
},
);
}
return null;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ratelimit.ts` around lines 267 - 301, The function checkVpnRateLimit
determines isVpn but always uses getVpnRateLimitConfig().vpnWindowMs for the
response; fix it by selecting the correct windowMs based on isVpn and only
calling config getters once: fetch the VPN config via getVpnRateLimitConfig()
and the standard config via getRateLimitConfig() (or the appropriate standard
config getter used in this codebase) at the start of the rate-limit branch, set
const windowMs = isVpn ? vpnConfig.vpnWindowMs : standardConfig.windowMs, then
use windowMs to compute retryAfter in both the JSON body and the "Retry-After"
header and remove the duplicate getVpnRateLimitConfig() calls; update references
in the Response creation inside checkVpnRateLimit accordingly.

22 changes: 22 additions & 0 deletions server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ import {
type GameState,
type RoundState,
} from "./game.ts";
import {
isVpnOrInternalIp,
VpnAwareRateLimiter,
checkVpnRateLimit,
} from "./ratelimit.ts";

const VERSION = crypto.randomUUID().slice(0, 8);

Expand Down Expand Up @@ -96,6 +101,14 @@ const historyCache = new Map<string, { body: string; expiresAt: number }>();
let lastRateWindowSweep = 0;
let lastHistoryCacheSweep = 0;

// VPN-aware rate limiter for stricter limits on VPN/internal IPs
const vpnRateLimiter = new VpnAwareRateLimiter({
vpnWindowMs: parsePositiveInt(process.env.VPN_RATELIMIT_WINDOW_MS, 60_000),
vpnMaxRequests: parsePositiveInt(process.env.VPN_RATELIMIT_MAX_REQ, 10),
standardWindowMs: WINDOW_MS,
standardMaxRequests: HISTORY_LIMIT_PER_MIN,
});
Comment on lines +119 to +125
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

vpnMaxRequests default is inconsistent between server.ts (10) and ratelimit.ts's getVpnRateLimitConfig() (5).

getVpnRateLimitConfig() returns vpnMaxRequests: 5 as the fallback when VPN_RATELIMIT_MAX_REQ is unset, while server.ts passes 10 explicitly. The PR description documents the default as 10. Any code that calls new VpnAwareRateLimiter() with no arguments, or reads getVpnRateLimitConfig() directly, will get the inconsistent value of 5. Align the getVpnRateLimitConfig() default with the intended value.

🛠️ Proposed fix

In ratelimit.ts, line 156:

-    vpnMaxRequests: parsePositiveInt(process.env.VPN_RATELIMIT_MAX_REQ, 5),
+    vpnMaxRequests: parsePositiveInt(process.env.VPN_RATELIMIT_MAX_REQ, 10),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server.ts` around lines 104 - 110, The fallback default for vpnMaxRequests is
inconsistent: update getVpnRateLimitConfig() to use vpnMaxRequests: 10 (not 5)
so it matches the VpnAwareRateLimiter instantiation and the PR docs; locate the
getVpnRateLimitConfig function and change its default/fallback value for
VPN_RATELIMIT_MAX_REQ to 10 (and run quick tests or lint to ensure no other
callers expect 5).


function parsePositiveInt(value: string | undefined, fallback: number): number {
const parsed = Number.parseInt(value ?? "", 10);
return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback;
Expand Down Expand Up @@ -306,6 +319,15 @@ const server = Bun.serve<WsData>({
const url = new URL(req.url);
const ip = getClientIp(req, server);

// Check VPN/internal IP rate limiting (very strict limits for VPN/subnet IPs)
// Skip health check endpoint from VPN rate limiting
if (url.pathname !== "/healthz") {
const vpnRateLimitResponse = checkVpnRateLimit(ip, vpnRateLimiter, log);
if (vpnRateLimitResponse) {
return vpnRateLimitResponse;
}
}
Comment on lines +426 to +433
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Static asset requests (/assets/) are counted against the VPN rate limit budget.

The VPN limiter check runs at line 324 before the /assets/ handler at line 331. Every static asset fetch (JS, CSS, fonts, etc.) by a VPN/internal IP consumes one of the 10 allowed requests per window, which can exhaust the budget quickly for a normal page load and cause legitimate users to see 429s. Extend the exclusion to at least /assets/ (and possibly /healthz-style paths) before hitting the limiter.

🛠️ Suggested fix
-    if (url.pathname !== "/healthz") {
+    if (url.pathname !== "/healthz" && !url.pathname.startsWith("/assets/")) {
       const vpnRateLimitResponse = checkVpnRateLimit(ip, vpnRateLimiter, log);
       if (vpnRateLimitResponse) {
         return vpnRateLimitResponse;
       }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Check VPN/internal IP rate limiting (very strict limits for VPN/subnet IPs)
// Skip health check endpoint from VPN rate limiting
if (url.pathname !== "/healthz") {
const vpnRateLimitResponse = checkVpnRateLimit(ip, vpnRateLimiter, log);
if (vpnRateLimitResponse) {
return vpnRateLimitResponse;
}
}
// Check VPN/internal IP rate limiting (very strict limits for VPN/subnet IPs)
// Skip health check endpoint from VPN rate limiting
if (url.pathname !== "/healthz" && !url.pathname.startsWith("/assets/")) {
const vpnRateLimitResponse = checkVpnRateLimit(ip, vpnRateLimiter, log);
if (vpnRateLimitResponse) {
return vpnRateLimitResponse;
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server.ts` around lines 322 - 329, The VPN rate limiter is applied before
static assets, causing requests to /assets/* to consume the limited VPN budget;
update the early-exit check around checkVpnRateLimit(ip, vpnRateLimiter, log) to
also skip VPN limiting for requests whose url.pathname starts with "/assets/"
(and keep skipping "/healthz"), i.e. only call checkVpnRateLimit when
url.pathname !== "/healthz" AND !url.pathname.startsWith("/assets/"); modify the
block that invokes checkVpnRateLimit so static asset requests bypass the VPN
limiter.


if (url.pathname.startsWith("/assets/")) {
const path = `./public${url.pathname}`;
const file = Bun.file(path);
Expand Down