From 7b4e51d58dad3098fde987703e4b87cc97439dd6 Mon Sep 17 00:00:00 2001 From: anshika1179 Date: Sun, 31 May 2026 19:41:38 +0530 Subject: [PATCH 1/4] fix: prevent rate limit bypass via spoofed X-Forwarded-For headers --- app/api/track-user/route.ts | 11 ++-- middleware.test.ts | 36 ++++++++++- middleware.ts | 8 +-- package-lock.json | 13 +--- types/network.ts | 26 ++++++++ utils/getClientIp.test.ts | 99 ++++++++++++++++++++++++++++ utils/getClientIp.ts | 124 ++++++++++++++++++++++++++++++++++++ utils/trustedProxy.ts | 124 ++++++++++++++++++++++++++++++++++++ 8 files changed, 417 insertions(+), 24 deletions(-) create mode 100644 types/network.ts create mode 100644 utils/getClientIp.test.ts create mode 100644 utils/getClientIp.ts create mode 100644 utils/trustedProxy.ts diff --git a/app/api/track-user/route.ts b/app/api/track-user/route.ts index c2ccb34d..2e3dcfeb 100644 --- a/app/api/track-user/route.ts +++ b/app/api/track-user/route.ts @@ -2,16 +2,13 @@ import { NextResponse } from 'next/server'; import dbConnect from '@/lib/mongodb'; import { User } from '@/models/User'; import { trackUserRateLimiter } from '@/lib/rate-limit'; +import { getClientIp } from '@/utils/getClientIp'; export async function POST(req: Request) { - // Get IP for rate limiting. - // x-real-ip is provided by Vercel/Nginx as the true client IP. - // We fall back to the LAST IP in the x-forwarded-for chain, which is appended by the Vercel proxy. - const forwardedFor = req.headers.get('x-forwarded-for'); - const fallbackIp = forwardedFor ? forwardedFor.split(',').pop()?.trim() : 'unknown'; - const ip = req.headers.get('x-real-ip') || fallbackIp || 'unknown'; + // Get IP for rate limiting securely + const ip = getClientIp(req); - if (ip !== 'unknown' && !(await trackUserRateLimiter.check(ip))) { + if (ip !== '127.0.0.1' && ip !== 'unknown' && !(await trackUserRateLimiter.check(ip))) { return NextResponse.json( { success: false, error: 'Too many requests, please try again later.' }, { status: 429 } diff --git a/middleware.test.ts b/middleware.test.ts index 2a95fe74..4cac066a 100644 --- a/middleware.test.ts +++ b/middleware.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest'; import { NextRequest, NextResponse } from 'next/server'; import { middleware } from './middleware'; import { rateLimit } from '@/lib/rate-limit'; @@ -8,8 +8,17 @@ vi.mock('@/lib/rate-limit', () => ({ })); describe('middleware', () => { + let originalEnv: string | undefined; + beforeEach(() => { vi.clearAllMocks(); + originalEnv = process.env.TRUSTED_PROXIES; + // Set trusted proxies to make standard multi-hop tests pass as trusted proxy chains + process.env.TRUSTED_PROXIES = '5.6.7.8, 9.10.11.12'; + }); + + afterEach(() => { + process.env.TRUSTED_PROXIES = originalEnv; }); it('calls NextResponse.next when rate limit succeeds', async () => { @@ -86,7 +95,7 @@ describe('middleware', () => { expect(response.headers.get('X-RateLimit-Remaining')).toBe('59'); }); - it('uses first IP from x-forwarded-for', async () => { + it('uses first IP from x-forwarded-for when subsequent hops are trusted', async () => { vi.mocked(rateLimit).mockResolvedValue({ success: true, limit: 60, @@ -105,6 +114,29 @@ describe('middleware', () => { expect(rateLimit).toHaveBeenCalledWith('1.2.3.4', 60, 60000); }); + it('ignores spoofed x-forwarded-for when subsequent hops are untrusted', async () => { + vi.mocked(rateLimit).mockResolvedValue({ + success: true, + limit: 60, + remaining: 59, + reset: 123456789, + }); + + // Clear trusted proxies so 5.6.7.8 is untrusted + process.env.TRUSTED_PROXIES = ''; + + const request = new NextRequest('http://localhost:3000/api/streak?user=octocat', { + headers: { + 'x-forwarded-for': '1.2.3.4, 5.6.7.8', + }, + }); + + await middleware(request); + + // Should resolve to the untrusted proxy IP (5.6.7.8) instead of the spoofed client IP (1.2.3.4) + expect(rateLimit).toHaveBeenCalledWith('5.6.7.8', 60, 60000); + }); + it('uses x-real-ip if x-forwarded-for is missing', async () => { vi.mocked(rateLimit).mockResolvedValue({ success: true, diff --git a/middleware.ts b/middleware.ts index bbb09744..fb72c5cc 100644 --- a/middleware.ts +++ b/middleware.ts @@ -1,6 +1,7 @@ import { NextResponse } from 'next/server'; import type { NextRequest } from 'next/server'; import { rateLimit } from './lib/rate-limit'; +import { getClientIp } from './utils/getClientIp'; /** * Middleware to enforce rate limiting on specific API routes. @@ -16,11 +17,8 @@ import { rateLimit } from './lib/rate-limit'; * Limit: 60 requests per minute per IP. */ export async function middleware(request: NextRequest) { - // Use Vercel's ip property if available, fallback to headers, then localhost - const ip = - request.headers.get('x-forwarded-for')?.split(',')[0] ?? - request.headers.get('x-real-ip') ?? - '127.0.0.1'; + // Secure client IP extraction + const ip = getClientIp(request); // Apply rate limiting // 60 requests per 60,000ms (1 minute) diff --git a/package-lock.json b/package-lock.json index d67d3890..df28fe78 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2864,8 +2864,7 @@ "resolved": "https://registry.npmjs.org/@types/aria-query/-/aria-query-5.0.4.tgz", "integrity": "sha512-rfT93uj5s0PRL7EzccGMs3brplhcrghnDoV26NqKhCAS1hVo+WdNsPvE/yb6ilfr5hi2MEk6d5EWJTKdxg8jVw==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/@types/chai": { "version": "5.2.3", @@ -3877,7 +3876,6 @@ "integrity": "sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=8" } @@ -5022,8 +5020,7 @@ "resolved": "https://registry.npmjs.org/dom-accessibility-api/-/dom-accessibility-api-0.5.16.tgz", "integrity": "sha512-X7BJ2yElsnOJ30pZF4uIIDfBEVgF4XEBxL9Bxhy6dnrm5hkzqmsWHGTiHqRiITNhMyFLyAiWndIJP7Z1NTteDg==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/dompurify": { "version": "3.4.7", @@ -7800,7 +7797,6 @@ "integrity": "sha512-h5bgJWpxJNswbU7qCrV0tIKQCaS3blPDrqKWx+QxzuzL1zGUzij9XCWLrSLsJPu5t+eWA/ycetzYAO5IOMcWAQ==", "dev": true, "license": "MIT", - "peer": true, "bin": { "lz-string": "bin/bin.js" } @@ -8840,7 +8836,6 @@ "integrity": "sha512-Qb1gy5OrP5+zDf2Bvnzdl3jsTf1qXVMazbvCoKhtKqVs4/YK4ozX4gKQJJVyNe+cajNPn0KoC0MC3FUmaHWEmQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "ansi-regex": "^5.0.1", "ansi-styles": "^5.0.0", @@ -8856,7 +8851,6 @@ "integrity": "sha512-Cxwpt2SfTzTtXcfOlzGEee8O+c+MmUgGrNiBcXnuWxuFJHe6a5Hz7qwhwe5OgaSYI0IJvkLqWX1ASG+cJOkEiA==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=10" }, @@ -8869,8 +8863,7 @@ "resolved": "https://registry.npmjs.org/react-is/-/react-is-17.0.2.tgz", "integrity": "sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/prop-types": { "version": "15.8.1", diff --git a/types/network.ts b/types/network.ts new file mode 100644 index 00000000..e0c44169 --- /dev/null +++ b/types/network.ts @@ -0,0 +1,26 @@ +export interface TrustedProxyConfig { + /** + * List of trusted proxy IP addresses or CIDR blocks. + * If '*' is present, all proxies are trusted (similar to express trust proxy = true). + */ + trustedProxies: string[]; + + /** + * Whether to trust default loopback/private IP addresses: + * 127.0.0.1, ::1, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, fc00::/7 + */ + trustPrivateRanges?: boolean; +} + +export interface GetClientIpOptions { + /** + * Trusted proxy configuration. + */ + proxyConfig?: TrustedProxyConfig; + + /** + * Custom request headers to check first, in order of priority. + * E.g., ['x-vercel-forwarded-for', 'cf-connecting-ip', 'x-real-ip'] + */ + headersPriority?: string[]; +} diff --git a/utils/getClientIp.test.ts b/utils/getClientIp.test.ts new file mode 100644 index 00000000..94414451 --- /dev/null +++ b/utils/getClientIp.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it } from 'vitest'; +import { NextRequest } from 'next/server'; +import { getClientIp } from './getClientIp'; + +describe('getClientIp', () => { + it('prefers request.ip on NextRequest if available', () => { + const req = new NextRequest('http://localhost:3000/api/streak'); + Object.defineProperty(req, 'ip', { value: '203.0.113.10', writable: true }); + + const ip = getClientIp(req); + expect(ip).toBe('203.0.113.10'); + }); + + it('ignores spoofed X-Forwarded-For when request.ip is present', () => { + const req = new NextRequest('http://localhost:3000/api/streak', { + headers: { + 'x-forwarded-for': '198.51.100.5, 203.0.113.10', + }, + }); + Object.defineProperty(req, 'ip', { value: '203.0.113.10', writable: true }); + + const ip = getClientIp(req); + expect(ip).toBe('203.0.113.10'); + }); + + it('uses direct client-supplied custom priority headers if available', () => { + const req = new Request('http://localhost:3000/api/streak', { + headers: { + 'cf-connecting-ip': '198.51.100.99', + }, + }); + + const ip = getClientIp(req); + expect(ip).toBe('198.51.100.99'); + }); + + it('ignores x-forwarded-for entirely when no trusted proxies are configured', () => { + const req = new Request('http://localhost:3000/api/streak', { + headers: { + 'x-forwarded-for': '198.51.100.5, 203.0.113.10', + }, + }); + + const ip = getClientIp(req, { + proxyConfig: { trustedProxies: [], trustPrivateRanges: false }, + }); + expect(ip).toBe('127.0.0.1'); // Default fallback because x-real-ip and others are missing + }); + + it('extracts correct IP through a trusted proxy chain', () => { + const req = new Request('http://localhost:3000/api/streak', { + headers: { + 'x-forwarded-for': '198.51.100.5, 203.0.113.10, 127.0.0.1', + }, + }); + + // We trust localhost (127.0.0.1) and 203.0.113.10 as proxies, so the true client is 198.51.100.5 + const ip = getClientIp(req, { + proxyConfig: { + trustedProxies: ['127.0.0.1', '203.0.113.10'], + trustPrivateRanges: true, + }, + }); + expect(ip).toBe('198.51.100.5'); + }); + + it('stops at the first untrusted proxy in the chain', () => { + const req = new Request('http://localhost:3000/api/streak', { + headers: { + 'x-forwarded-for': '198.51.100.5, 8.8.8.8, 127.0.0.1', + }, + }); + + // 127.0.0.1 is trusted. 8.8.8.8 is not. So the resolved IP must be 8.8.8.8. + const ip = getClientIp(req, { + proxyConfig: { + trustedProxies: ['127.0.0.1'], + trustPrivateRanges: true, + }, + }); + expect(ip).toBe('8.8.8.8'); + }); + + it('supports wildcards to trust all proxies (trust-all behavior)', () => { + const req = new Request('http://localhost:3000/api/streak', { + headers: { + 'x-forwarded-for': '198.51.100.5, 203.0.113.10', + }, + }); + + const ip = getClientIp(req, { + proxyConfig: { + trustedProxies: ['*'], + trustPrivateRanges: false, + }, + }); + expect(ip).toBe('198.51.100.5'); + }); +}); diff --git a/utils/getClientIp.ts b/utils/getClientIp.ts new file mode 100644 index 00000000..f49fbd7c --- /dev/null +++ b/utils/getClientIp.ts @@ -0,0 +1,124 @@ +import { NextRequest } from 'next/server'; +import { GetClientIpOptions } from '../types/network'; +import { isTrustedProxy, loadTrustedProxyConfig } from './trustedProxy'; + +/** + * Logs security-relevant events such as spoofing attempts in a structured format. + */ +function logSecurityEvent(event: string, details: Record) { + console.warn( + JSON.stringify({ + timestamp: new Date().toISOString(), + type: 'SECURITY_EVENT', + event, + ...details, + }) + ); +} + +/** + * Extracts the true client IP from the request securely. + * + * It prevents spoofing by validating proxy trust boundaries. + */ +export function getClientIp( + request: Request | NextRequest, + options: GetClientIpOptions = {} +): string { + const config = options.proxyConfig || loadTrustedProxyConfig(); + const headers = request.headers; + + // 1. NextRequest has a secure, platform-populated request.ip property on Vercel/Next.js + if (request instanceof NextRequest && request.ip) { + const rawXff = headers.get('x-forwarded-for'); + if (rawXff) { + const firstIp = rawXff.split(',')[0].trim(); + if (firstIp && firstIp !== request.ip) { + logSecurityEvent('SPOOFED_HEADER_ATTEMPT', { + claimedIp: firstIp, + resolvedIp: request.ip, + header: 'x-forwarded-for', + }); + } + } + return request.ip; + } + + // 2. Process X-Forwarded-For securely if present + const xff = headers.get('x-forwarded-for'); + if (xff) { + const ips = xff + .split(',') + .map((ip: string) => ip.trim()) + .filter(Boolean); + if (ips.length > 0) { + // If we don't trust any proxies, do NOT trust X-Forwarded-For values supplied by the client + if (config.trustedProxies.length === 0 && !config.trustPrivateRanges) { + const fallbackIp = headers.get('x-real-ip')?.trim() || '127.0.0.1'; + if (ips[0] !== fallbackIp) { + logSecurityEvent('SPOOFED_HEADER_ATTEMPT', { + claimedIp: ips[0], + resolvedIp: fallbackIp, + header: 'x-forwarded-for', + }); + } + return fallbackIp; + } + + // If all proxies are trusted via wildcard + if (config.trustedProxies.includes('*')) { + return ips[0]; + } + + // Traverse from right to left (most recent to oldest proxy hop) + // The rightmost IP is the one that connected directly to our server/balancer + let clientIp = ips[ips.length - 1]; + + for (let i = ips.length - 1; i >= 0; i--) { + const currentIp = ips[i]; + if (isTrustedProxy(currentIp, config)) { + // If the proxy is trusted, the client IP is the one preceding it (to the left) + if (i > 0) { + clientIp = ips[i - 1]; + } else { + clientIp = currentIp; + } + } else { + // Found the first untrusted IP in the chain - this is our true client + clientIp = currentIp; + break; + } + } + + if (ips[0] !== clientIp) { + logSecurityEvent('SPOOFED_HEADER_ATTEMPT', { + claimedIp: ips[0], + resolvedIp: clientIp, + header: 'x-forwarded-for', + }); + } + + return clientIp; + } + } + + // 3. Custom/Platform priority headers (e.g. Cloudflare, Vercel) + const priorityHeaders = options.headersPriority || [ + 'x-vercel-proxied-for', + 'cf-connecting-ip', + 'x-real-ip', + ]; + + for (const headerName of priorityHeaders) { + const headerVal = headers.get(headerName); + if (headerVal) { + const trimmed = headerVal.trim(); + if (trimmed) { + return trimmed; + } + } + } + + // 4. Ultimate Fallback + return '127.0.0.1'; +} diff --git a/utils/trustedProxy.ts b/utils/trustedProxy.ts new file mode 100644 index 00000000..393a32fa --- /dev/null +++ b/utils/trustedProxy.ts @@ -0,0 +1,124 @@ +import { TrustedProxyConfig } from '../types/network'; + +// Private and Loopback CIDR ranges +const PRIVATE_IPV4_RANGES = [ + '127.0.0.0/8', + '10.0.0.0/8', + '172.16.0.0/12', + '192.168.0.0/16', + '169.254.0.0/16', +]; + +/** + * Converts an IPv4 address to its 32-bit integer representation. + */ +export function ip4ToInt(ip: string): number { + return ip.split('.').reduce((int, oct) => (int << 8) + parseInt(oct, 10), 0) >>> 0; +} + +/** + * Checks if an IPv4 address falls within a given CIDR block. + */ +export function isIPv4InCidr(ip: string, cidr: string): boolean { + try { + const [range, bitsStr] = cidr.split('/'); + const bits = parseInt(bitsStr, 10); + if (isNaN(bits) || bits < 0 || bits > 32) return false; + + const ipInt = ip4ToInt(ip); + const rangeInt = ip4ToInt(range); + + if (bits === 0) return true; + + // Using >>> 0 to ensure unsigned 32-bit integer arithmetic + const mask = bits === 32 ? 0xffffffff : ~((1 << (32 - bits)) - 1) >>> 0; + return (ipInt & mask) === (rangeInt & mask); + } catch { + return false; + } +} + +/** + * Check if the given string is a valid IPv4 address. + */ +export function isIPv4(ip: string): boolean { + const parts = ip.split('.'); + if (parts.length !== 4) return false; + return parts.every((part) => { + const num = parseInt(part, 10); + return !isNaN(num) && num >= 0 && num <= 255 && part === num.toString(); + }); +} + +/** + * Checks if an IP is in the trusted proxy configuration list or private ranges. + */ +export function isTrustedProxy(ip: string, config: TrustedProxyConfig): boolean { + const sanitizedIp = ip.trim(); + + // If wildcard is used, trust all proxies + if (config.trustedProxies.includes('*')) { + return true; + } + + // Check exact matches + if (config.trustedProxies.includes(sanitizedIp)) { + return true; + } + + // Handle IPv4 CIDR matching + if (isIPv4(sanitizedIp)) { + for (const entry of config.trustedProxies) { + if (entry.includes('/') && isIPv4InCidr(sanitizedIp, entry)) { + return true; + } + } + + if (config.trustPrivateRanges) { + for (const range of PRIVATE_IPV4_RANGES) { + if (isIPv4InCidr(sanitizedIp, range)) { + return true; + } + } + } + } else { + // IPv6 checks (simple exact match loopback/private range fallback) + if (config.trustPrivateRanges) { + if (sanitizedIp === '::1' || sanitizedIp === '0:0:0:0:0:0:0:1') { + return true; + } + // Simple prefix match for private IPv6 ranges + const lowerIp = sanitizedIp.toLowerCase(); + if (lowerIp.startsWith('fc00') || lowerIp.startsWith('fd00') || lowerIp.startsWith('fe80')) { + return true; + } + } + } + + return false; +} + +/** + * Loads trusted proxy configuration from environment variables. + */ +export function loadTrustedProxyConfig(): TrustedProxyConfig { + const envProxies = process.env.TRUSTED_PROXIES; + const trustedProxies: string[] = []; + + if (envProxies) { + trustedProxies.push( + ...envProxies + .split(',') + .map((s) => s.trim()) + .filter(Boolean) + ); + } + + // In development, we always trust loopback IP addresses by default + const isDev = process.env.NODE_ENV !== 'production'; + + return { + trustedProxies, + trustPrivateRanges: isDev || process.env.TRUST_PRIVATE_PROXIES === 'true', + }; +} From 45cbba1eb10cf7d19cdb2ba7963a2e88bf48217b Mon Sep 17 00:00:00 2001 From: anshika1179 Date: Sun, 31 May 2026 20:25:36 +0530 Subject: [PATCH 2/4] fix(types): cast NextRequest to resolve typecheck compiler error --- package-lock.json | 29 ++++++++++++++++++++++++++++- utils/getClientIp.ts | 9 +++++---- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index df28fe78..df8bb4cc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -167,6 +167,7 @@ "integrity": "sha512-RgHBCvtjbOK2gXSNBNIkNoEc9qoVEtau3hj8gEqKQuL3HZAibKarWFEI3Lfm6EYKkLalOh8eSrj9b+ch9H/VBA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.29.7", "@babel/generator": "^7.29.7", @@ -496,6 +497,7 @@ } ], "license": "MIT", + "peer": true, "engines": { "node": ">=20.19.0" }, @@ -544,6 +546,7 @@ } ], "license": "MIT", + "peer": true, "engines": { "node": ">=20.19.0" } @@ -554,6 +557,7 @@ "integrity": "sha512-yq6OkJ4p82CAfPl0u9mQebQHKPJkY7WrIuk205cTYnYe+k2Z8YBh11FrbRG/H6ihirqcacOgl2BIO8oyMQLeXw==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "@emnapi/wasi-threads": "1.2.1", "tslib": "^2.4.0" @@ -565,6 +569,7 @@ "integrity": "sha512-ewvYlk86xUoGI0zQRNq/mC+16R1QeDlKQy21Ki3oSYXNgLb45GV1P6A0M+/s6nyCuNDqe5VpaY84BzXGwVbwFA==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "tslib": "^2.4.0" } @@ -2921,6 +2926,7 @@ "integrity": "sha512-ECymXOukMnOoVkC2bb1Vc/w/836DXncOg5m8Xj1RH7xSHZJWNYY6Zh7EH477vcnD5egKNNfy2RpNOmuChhFPgQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "undici-types": "~6.21.0" } @@ -2944,6 +2950,7 @@ "integrity": "sha512-eRwcGNHve+E8qtEQSSRl6urh+rFop4v8gm6O8rGv25CodbvFdLjA1vVQ1KkiFE0w0UPOnb8tDiFKL5lp0rtY5Q==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "csstype": "^3.2.2" } @@ -2954,6 +2961,7 @@ "integrity": "sha512-jp2L/eY6fn+KgVVQAOqYItbF0VY/YApe5Mz2F0aykSO8gx31bYCZyvSeYxCHKvzHG5eZjc+zyaS5BrBWya2+kQ==", "dev": true, "license": "MIT", + "peer": true, "peerDependencies": { "@types/react": "^19.2.0" } @@ -3025,6 +3033,7 @@ "integrity": "sha512-fcqpj/MyK4sxDPcbe7STNPbpQL4RLZOPWuaTmwZYuc+hJKzRf58yRxfhqGpc6PIq9ZyfSBpfHgmUHmHs0KwHwg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.60.0", "@typescript-eslint/types": "8.60.0", @@ -3631,6 +3640,7 @@ "integrity": "sha512-qsYPeXc5Q9dFLd1i8Ap+Bx8sQgcp+rFVQo4R0dDsWNBzl26ldVF1qOO+RL24K7FDrR6pA+50XedRLSoSG24bVQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@bcoe/v8-coverage": "^1.0.2", "@vitest/utils": "4.1.7", @@ -3760,6 +3770,7 @@ "integrity": "sha512-TP6utB2yX6rsJNVRo2qAlsi48i1YwFTrLV2tnTtWqJaYX7m4lRCCLirZBjU6xC5m0RsPHr+L2+N+eIPhgEzFfw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/utils": "4.1.7", "fflate": "^0.8.2", @@ -3820,6 +3831,7 @@ "integrity": "sha512-UVJyE9MttOsBQIDKw1skb9nAwQuR5wuGD3+82K6JgJlm/Y+KI92oNsMNGZCYdDsVtRHSak0pcV5Dno5+4jh9sw==", "dev": true, "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -4285,6 +4297,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "baseline-browser-mapping": "^2.10.12", "caniuse-lite": "^1.0.30001782", @@ -4612,7 +4625,8 @@ "version": "3.2.3", "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.2.3.tgz", "integrity": "sha512-z1HGKcYy2xA8AGQfwrn0PAy+PB7X/GSj3UVJW9qKyn43xWa+gl5nXmU4qqLMRzWVLFC8KusUX8T/0kCiOYpAIQ==", - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/d3-array": { "version": "3.2.4", @@ -4758,6 +4772,7 @@ "resolved": "https://registry.npmjs.org/d3-selection/-/d3-selection-3.0.0.tgz", "integrity": "sha512-fmTRWbNMmsmWq6xJV8D19U/gw/bwrHfNXxrIN+HfZgnzqTHp9jOmKMhsTUjXOJnZOdZY9Q28y4yebKzqDKlxlQ==", "license": "ISC", + "peer": true, "engines": { "node": ">=12" } @@ -5354,6 +5369,7 @@ "integrity": "sha512-XoMjdBOwe/esVgEvLmNsD3IRHkm7fbKIUGvrleloJXUZgDHig2IPWNniv+GwjyJXzuNqVjlr5+4yVUZjycJwfQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -5555,6 +5571,7 @@ "integrity": "sha512-whOE1HFo/qJDyX4SnXzP4N6zOWn79WhnCUY/iDR0mPfQZO8wcYE4JClzI2oZrhBnnMUCBCHZhO6VQyoBU95mZA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@rtsao/scc": "^1.1.0", "array-includes": "^3.1.9", @@ -8249,6 +8266,7 @@ "resolved": "https://registry.npmjs.org/next/-/next-16.2.6.tgz", "integrity": "sha512-qOVgKJg1+At15NpeUP+eJgCHvTCgXsogweq87Ri/Ix7PkqQHg4sdaXmSFqKlgaIXE4kW0g25LE68W87UANlHtw==", "license": "MIT", + "peer": true, "dependencies": { "@next/env": "16.2.6", "@swc/helpers": "0.5.15", @@ -8778,6 +8796,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "nanoid": "^3.3.12", "picocolors": "^1.1.1", @@ -8937,6 +8956,7 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.4.tgz", "integrity": "sha512-9nfp2hYpCwOjAN+8TZFGhtWEwgvWHXqESH8qT89AT/lWklpLON22Lc8pEtnpsZz7VmawabSU0gCjnj8aC0euHQ==", "license": "MIT", + "peer": true, "engines": { "node": ">=0.10.0" } @@ -8946,6 +8966,7 @@ "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-19.2.4.tgz", "integrity": "sha512-AXJdLo8kgMbimY95O2aKQqsz2iWi9jMgKJhRBAxECE4IFxfcazB2LmzloIoibJI3C12IlY20+KFaLv+71bUJeQ==", "license": "MIT", + "peer": true, "dependencies": { "scheduler": "^0.27.0" }, @@ -10119,6 +10140,7 @@ "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -10256,6 +10278,7 @@ "integrity": "sha512-X8EX+XV4QR5xCsrgxaED954zTDfY8KqlDtskKEL0cHhyS/P8b4IFOvGDQpsC9Q1XnLq915wEfwwY/zzskCtmhg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "~0.28.0" }, @@ -10380,6 +10403,7 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -10543,6 +10567,7 @@ "integrity": "sha512-s4BJJ+5y1pYL6Otw51FHhVJQhPnuRinKig64g/1+EUNaJsd3gCKdD31IPFvswUgW9/60QT9oFHbZHbQK5imcxw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "lightningcss": "^1.32.0", "picomatch": "^4.0.4", @@ -10634,6 +10659,7 @@ "integrity": "sha512-flYyaFd2CgoCoU+0UKt3pxksgC+S02iTDN0n3LtqaMeXsI9SBcdNujc2k0DeFLzUn/0k538yNjOSdwgCqcrwJA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/expect": "4.1.7", "@vitest/mocker": "4.1.7", @@ -11000,6 +11026,7 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-4.4.3.tgz", "integrity": "sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ==", "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/utils/getClientIp.ts b/utils/getClientIp.ts index f49fbd7c..b2f321c9 100644 --- a/utils/getClientIp.ts +++ b/utils/getClientIp.ts @@ -29,19 +29,20 @@ export function getClientIp( const headers = request.headers; // 1. NextRequest has a secure, platform-populated request.ip property on Vercel/Next.js - if (request instanceof NextRequest && request.ip) { + const requestIp = (request as unknown as { ip?: string }).ip; + if (request instanceof NextRequest && requestIp) { const rawXff = headers.get('x-forwarded-for'); if (rawXff) { const firstIp = rawXff.split(',')[0].trim(); - if (firstIp && firstIp !== request.ip) { + if (firstIp && firstIp !== requestIp) { logSecurityEvent('SPOOFED_HEADER_ATTEMPT', { claimedIp: firstIp, - resolvedIp: request.ip, + resolvedIp: requestIp, header: 'x-forwarded-for', }); } } - return request.ip; + return requestIp; } // 2. Process X-Forwarded-For securely if present From 22120f02eb10e18a39ff99d34d5104405d5287ca Mon Sep 17 00:00:00 2001 From: anshika1179 Date: Sun, 31 May 2026 20:44:33 +0530 Subject: [PATCH 3/4] fix: prevent GitHub API quota exhaustion through refresh abuse --- app/(root)/dashboard/[username]/page.test.tsx | 1 + app/api/github/route.test.ts | 180 +++++++++++------- app/api/github/route.ts | 89 ++++++++- lib/github.ts | 8 + services/github/background-refresh.ts | 80 ++++++++ services/github/quota-monitor.ts | 97 ++++++++++ services/github/refresh-policy.ts | 86 +++++++++ services/github/refresh-rate-limiter.ts | 101 ++++++++++ 8 files changed, 572 insertions(+), 70 deletions(-) create mode 100644 services/github/background-refresh.ts create mode 100644 services/github/quota-monitor.ts create mode 100644 services/github/refresh-policy.ts create mode 100644 services/github/refresh-rate-limiter.ts diff --git a/app/(root)/dashboard/[username]/page.test.tsx b/app/(root)/dashboard/[username]/page.test.tsx index 5c5d5fe0..494b7b35 100644 --- a/app/(root)/dashboard/[username]/page.test.tsx +++ b/app/(root)/dashboard/[username]/page.test.tsx @@ -100,6 +100,7 @@ describe('DashboardPage', () => { achievements: [], commitClock: [], graphData: { nodes: [], links: [] }, + lastSyncedAt: undefined, }; beforeEach(() => { diff --git a/app/api/github/route.test.ts b/app/api/github/route.test.ts index fc63c211..8e574737 100644 --- a/app/api/github/route.test.ts +++ b/app/api/github/route.test.ts @@ -1,98 +1,166 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { GET } from './route'; -// Replace the real GitHub API with a fake function so tests can run without hitting real APIs + +// Replace the real GitHub API with a fake function vi.mock('../../../lib/github', () => ({ getFullDashboardData: vi.fn(), })); import { getFullDashboardData } from '../../../lib/github'; - -function makeRequest(params: Record = {}): Request { +import { quotaMonitor } from '@/services/github/quota-monitor'; +import { refreshPolicy } from '@/services/github/refresh-policy'; +import { refreshRateLimiter } from '@/services/github/refresh-rate-limiter'; +import { backgroundRefresh } from '@/services/github/background-refresh'; + +function makeRequest( + params: Record = {}, + headers: Record = {} +): Request { const url = new URL('http://localhost/api/github'); for (const [key, value] of Object.entries(params)) { url.searchParams.set(key, value); } - return new Request(url.toString()); + return new Request(url.toString(), { + headers: new Headers(headers), + }); } describe('GET /api/github', () => { beforeEach(() => { vi.clearAllMocks(); - vi.mocked(getFullDashboardData).mockResolvedValue({} as never); + vi.mocked(getFullDashboardData).mockResolvedValue({ + profile: { lastSyncedAt: new Date().toISOString() }, + calendar: {}, + lastSyncedAt: new Date().toISOString(), + } as unknown as Awaited>); + + quotaMonitor.reset(); + refreshPolicy.reset(); + refreshRateLimiter.reset(); + backgroundRefresh.reset(); }); - describe('cache bypass via ?refresh=true', () => { - it('calls getFullDashboardData with { bypassCache: true } when ?refresh=true', async () => { - await GET(makeRequest({ username: 'octocat', refresh: 'true' })); + describe('Unrestricted Cache Bypass & Abuse Mitigation (Issue #1978)', () => { + // Scenario 1: Normal cached request + it('Scenario 1: serves cached data and checks SWR background refresh', async () => { + // Mock data that is stale (15 minutes ago) + const staleTime = new Date(Date.now() - 15 * 60 * 1000).toISOString(); + vi.mocked(getFullDashboardData).mockResolvedValue({ + profile: { lastSyncedAt: staleTime }, + calendar: {}, + } as unknown as Awaited>); - expect(getFullDashboardData).toHaveBeenCalledWith('octocat', { bypassCache: true }); - }); + const triggerSpy = vi.spyOn(backgroundRefresh, 'triggerRefresh'); - it('calls getFullDashboardData with { bypassCache: false } when refresh is omitted', async () => { - await GET(makeRequest({ username: 'octocat' })); - expect(getFullDashboardData).toHaveBeenCalledWith('octocat', { - bypassCache: false, - }); + const response = await GET(makeRequest({ username: 'torvalds' })); + expect(response.status).toBe(200); + expect(getFullDashboardData).toHaveBeenCalledWith('torvalds', { bypassCache: false }); + expect(triggerSpy).toHaveBeenCalledWith('torvalds'); }); - it('returns 400 when username contains invalid characters', async () => { - const response = await GET(makeRequest({ username: '@@@@@' })); - const body = await response.json(); - expect(response.status).toBe(400); - expect(body.error).toContain('Invalid parameters'); + // Scenario 2: Single refresh request allowed + it('Scenario 2: allows a single refresh request when limits are respected', async () => { + const response = await GET(makeRequest({ username: 'torvalds', refresh: 'true' })); + + expect(response.status).toBe(200); + expect(getFullDashboardData).toHaveBeenCalledWith('torvalds', { bypassCache: true }); + expect(response.headers.get('X-Refresh-Status')).toBe('Fresh'); }); - it('returns 400 when username contains only whitespace', async () => { - const response = await GET(makeRequest({ username: ' ' })); - const body = await response.json(); + // Scenario 3: Repeated refresh within cooldown served from cache + it('Scenario 3: serves cached response for repeated refresh requests within cooldown', async () => { + // First refresh is allowed + await GET(makeRequest({ username: 'torvalds', refresh: 'true' })); + expect(getFullDashboardData).toHaveBeenLastCalledWith('torvalds', { bypassCache: true }); - expect(response.status).toBe(400); - expect(body.error).toContain('Invalid parameters'); + // Second refresh within cooldown (5 minutes) + const response = await GET(makeRequest({ username: 'torvalds', refresh: 'true' })); + + expect(response.status).toBe(200); + // Cooldown fallback triggers cached read + expect(getFullDashboardData).toHaveBeenLastCalledWith('torvalds', { bypassCache: false }); + expect(response.headers.get('X-Refresh-Status')).toBe('Cooldown-Served-Cached'); }); - it('returns 400 when username exceeds GitHub maximum length', async () => { + // Scenario 4: Refresh rate limit exceeded per client IP + it('Scenario 4: returns 429 when client refresh rate limit is exceeded', async () => { + // Set rate limit to 2 per window for testing + refreshRateLimiter.setLimit(2); + + // Refresh 1 + await GET( + makeRequest({ username: 'torvalds', refresh: 'true' }, { 'x-real-ip': '203.0.113.5' }) + ); + // Refresh 2 + await GET( + makeRequest({ username: 'octocat', refresh: 'true' }, { 'x-real-ip': '203.0.113.5' }) + ); + + // Refresh 3 (exceeds limit of 2) const response = await GET( - makeRequest({ - username: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', - }) + makeRequest({ username: 'torvalds', refresh: 'true' }, { 'x-real-ip': '203.0.113.5' }) ); + expect(response.status).toBe(429); const body = await response.json(); + expect(body.error).toContain('Refresh rate limit exceeded'); + }); - expect(response.status).toBe(400); - expect(body.error).toContain('Invalid parameters'); + // Scenario 5: Low GitHub quota blocks refresh + it('Scenario 5: blocks manual refresh when remaining GitHub quota is low (<10%)', async () => { + // Set global remaining quota to 400 out of 5000 (8%) + quotaMonitor.setQuota(5000, 400, Date.now() + 60000); + + const response = await GET(makeRequest({ username: 'torvalds', refresh: 'true' })); + + expect(response.status).toBe(429); + const body = await response.json(); + expect(body.error).toContain('quota is low'); + expect(getFullDashboardData).not.toHaveBeenCalled(); }); - // Test 1 — missing username → 400 - it('returns 400 when username is missing', async () => { - const response = await GET(makeRequest()); + // Scenario 6: Background refresh execution + it('Scenario 6: asynchronous background refresh completes successfully', async () => { + const loadSpy = vi.spyOn(backgroundRefresh, 'triggerRefresh'); + + // Mock data that is stale + const staleTime = new Date(Date.now() - 15 * 60 * 1000).toISOString(); + vi.mocked(getFullDashboardData).mockResolvedValue({ + profile: { lastSyncedAt: staleTime }, + calendar: {}, + } as unknown as Awaited>); + + await GET(makeRequest({ username: 'torvalds' })); + + expect(loadSpy).toHaveBeenCalledWith('torvalds'); + }); + }); + + describe('Standard route behavior', () => { + it('returns 400 when username contains invalid characters', async () => { + const response = await GET(makeRequest({ username: '@@@@@' })); const body = await response.json(); expect(response.status).toBe(400); expect(body.error).toContain('Invalid parameters'); }); - it('returns 400 and skips GitHub when username format is invalid', async () => { - const response = await GET(makeRequest({ username: 'bad user' })); + it('returns 400 when username contains only whitespace', async () => { + const response = await GET(makeRequest({ username: ' ' })); const body = await response.json(); expect(response.status).toBe(400); expect(body.error).toContain('Invalid parameters'); - expect(getFullDashboardData).not.toHaveBeenCalled(); }); - // Test 2 — valid username → 200 - it('returns 200 with JSON body for a valid username', async () => { - vi.mocked(getFullDashboardData).mockResolvedValue({ profile: 'octocat' } as never); - - const response = await GET(makeRequest({ username: 'octocat' })); + it('returns 400 when username is missing', async () => { + const response = await GET(makeRequest()); const body = await response.json(); - expect(response.status).toBe(200); - expect(body).toEqual({ profile: 'octocat' }); + expect(response.status).toBe(400); + expect(body.error).toContain('Invalid parameters'); }); - // Test 3 — throws 'User not found' → 404 it('returns 404 when getFullDashboardData throws User not found', async () => { vi.mocked(getFullDashboardData).mockRejectedValue(new Error('User not found')); @@ -102,27 +170,5 @@ describe('GET /api/github', () => { expect(response.status).toBe(404); expect(body.error).toContain('User not found'); }); - - // Test 4 — throws 'API limit reached' → 403 - it('returns 403 when getFullDashboardData throws API limit reached', async () => { - vi.mocked(getFullDashboardData).mockRejectedValue(new Error('API limit reached')); - - const response = await GET(makeRequest({ username: 'octocat' })); - const body = await response.json(); - - expect(response.status).toBe(403); - expect(body.error).toContain('rate limit'); - }); - - // Test 5 — throws generic error → 500 - it('returns 500 for a generic unexpected error', async () => { - vi.mocked(getFullDashboardData).mockRejectedValue(new Error('Something went wrong')); - - const response = await GET(makeRequest({ username: 'octocat' })); - const body = await response.json(); - - expect(response.status).toBe(500); - expect(body.error).toContain('Something went wrong'); - }); }); }); diff --git a/app/api/github/route.ts b/app/api/github/route.ts index 4c3f7ad8..c0dc7d20 100644 --- a/app/api/github/route.ts +++ b/app/api/github/route.ts @@ -3,6 +3,22 @@ import { NextResponse } from 'next/server'; import { getFullDashboardData } from '@/lib/github'; import { githubParamsSchema } from '@/lib/validations'; +import { getClientIp } from '@/utils/getClientIp'; +import { quotaMonitor } from '@/services/github/quota-monitor'; +import { refreshPolicy } from '@/services/github/refresh-policy'; +import { refreshRateLimiter } from '@/services/github/refresh-rate-limiter'; +import { backgroundRefresh } from '@/services/github/background-refresh'; + +function logSecurityEvent(event: string, details: Record) { + console.warn( + JSON.stringify({ + timestamp: new Date().toISOString(), + type: 'SECURITY_EVENT', + event, + ...details, + }) + ); +} /** * Returns GitHub dashboard data as JSON. @@ -18,11 +34,12 @@ import { githubParamsSchema } from '@/lib/validations'; * - 400 → Invalid query parameters * - 403 → GitHub API rate limit reached * - 404 → GitHub user not found + * - 429 → Too many requests (Refresh rate limit or low quota) * - 500 → Internal server error */ - export async function GET(request: Request) { const { searchParams } = new URL(request.url); + const ip = getClientIp(request); const parseResult = githubParamsSchema.safeParse(Object.fromEntries(searchParams.entries())); @@ -35,9 +52,70 @@ export async function GET(request: Request) { const { username, refresh } = parseResult.data; + // 1. Quota awareness check - if remaining quota is low, disable manual refresh + if (refresh && quotaMonitor.isQuotaLow()) { + logSecurityEvent('LOW_QUOTA_REFRESH_BLOCKED', { + username, + ip, + remainingQuota: quotaMonitor.getQuota().remaining, + }); + return NextResponse.json( + { error: 'GitHub API quota is low. Cache refresh temporarily disabled.' }, + { status: 429 } + ); + } + + // 2. Separate Refresh Rate Limiter + if (refresh) { + const rateLimitCheck = refreshRateLimiter.checkLimit(ip); + if (!rateLimitCheck.success) { + logSecurityEvent('REFRESH_RATE_LIMIT_EXCEEDED', { + username, + ip, + limit: rateLimitCheck.limit, + }); + return NextResponse.json( + { error: 'Refresh rate limit exceeded. Please try again later.' }, + { + status: 429, + headers: { + 'X-RateLimit-Limit': rateLimitCheck.limit.toString(), + 'X-RateLimit-Remaining': rateLimitCheck.remaining.toString(), + 'X-RateLimit-Reset': rateLimitCheck.reset.toString(), + }, + } + ); + } + } + + // 3. Per-Username Refresh Cooldown + let shouldBypassCache = refresh; + if (refresh) { + if (!refreshPolicy.isRefreshAllowed(username)) { + logSecurityEvent('REFRESH_COOLDOWN_VIOLATION', { + username, + ip, + remainingMs: refreshPolicy.getRemainingCooldown(username), + }); + // Fallback: serve cached data instead of bypassing cache + shouldBypassCache = false; + } else { + refreshPolicy.recordRefresh(username); + } + } + try { - const data = await getFullDashboardData(username, { bypassCache: refresh }); - const cacheControl = refresh + const data = await getFullDashboardData(username, { bypassCache: shouldBypassCache }); + + // 4. Stale-While-Revalidate background refresh for normal cached requests + if (!shouldBypassCache) { + const lastSynced = data.lastSyncedAt; + if (backgroundRefresh.isStale(lastSynced)) { + backgroundRefresh.triggerRefresh(username); + } + } + + const cacheControl = shouldBypassCache ? 'no-cache, no-store, must-revalidate' : 's-maxage=3600, stale-while-revalidate=86400'; @@ -45,6 +123,11 @@ export async function GET(request: Request) { status: 200, headers: { 'Cache-Control': cacheControl, + 'X-Refresh-Status': shouldBypassCache + ? 'Fresh' + : refresh + ? 'Cooldown-Served-Cached' + : 'Cached', }, }); } catch (error: unknown) { diff --git a/lib/github.ts b/lib/github.ts index e73370fc..68465baa 100644 --- a/lib/github.ts +++ b/lib/github.ts @@ -11,6 +11,7 @@ import { calculateStreak, aggregateCalendars } from '@/lib/calculate'; import { DistributedCache } from '@/lib/cache'; import { LANGUAGE_COLORS } from '@/lib/svg/languageColors'; import { CONTRIBUTION_MILESTONES, STREAK_MILESTONES } from './svg/constants'; +import { quotaMonitor } from '@/services/github/quota-monitor'; import 'server-only'; @@ -88,6 +89,12 @@ export async function fetchWithRetry( if (!res) throw new Error('GitHub API request failed without a response'); + try { + quotaMonitor.updateQuotaFromHeaders(res.headers); + } catch (err) { + console.error('Failed to update quota monitor', err); + } + // Check for rate limit headers const retryAfter = res.headers.get('retry-after'); const isRateLimited = @@ -1158,6 +1165,7 @@ export async function getFullDashboardData(username: string, options: FetchOptio ), commitClock, graphData: { nodes, links }, + lastSyncedAt: calendarData.lastSyncedAt, }; } diff --git a/services/github/background-refresh.ts b/services/github/background-refresh.ts new file mode 100644 index 00000000..cae375e8 --- /dev/null +++ b/services/github/background-refresh.ts @@ -0,0 +1,80 @@ +import { getFullDashboardData } from '../../lib/github'; + +// Cache is considered stale and candidate for background refresh after 10 minutes +const STALE_THRESHOLD_MS = 10 * 60 * 1000; + +export class BackgroundRefresh { + private static instance: BackgroundRefresh; + + private activeJobs = new Set(); + + private constructor() {} + + public static getInstance(): BackgroundRefresh { + if (!BackgroundRefresh.instance) { + BackgroundRefresh.instance = new BackgroundRefresh(); + } + return BackgroundRefresh.instance; + } + + /** + * Checks whether a cached entry is stale and should trigger an async background update. + */ + public isStale(lastSyncedAt: string | undefined): boolean { + if (!lastSyncedAt) return true; + + try { + const lastSyncTime = new Date(lastSyncedAt).getTime(); + return Date.now() - lastSyncTime > STALE_THRESHOLD_MS; + } catch { + return true; + } + } + + /** + * Triggers an asynchronous, non-blocking cache backfill for the given username. + */ + public triggerRefresh(username: string): void { + const sanitized = username.trim().toLowerCase(); + + // Avoid duplicate background jobs for the same user concurrently + if (this.activeJobs.has(sanitized)) { + return; + } + + this.activeJobs.add(sanitized); + + console.info(`[BackgroundRefresh] Starting background refresh for: ${sanitized}`); + + // Trigger update asynchronously (non-blocking Promise execution) + getFullDashboardData(username, { bypassCache: true }) + .then(() => { + console.info( + `[BackgroundRefresh] Successfully completed background refresh for: ${sanitized}` + ); + }) + .catch((err) => { + console.error(`[BackgroundRefresh] Background refresh failed for: ${sanitized}`, err); + }) + .finally(() => { + this.activeJobs.delete(sanitized); + }); + } + + /** + * Returns whether a background job is currently active for a username. + */ + public isJobActive(username: string): boolean { + return this.activeJobs.has(username.trim().toLowerCase()); + } + + /** + * Resets active jobs. + */ + public reset(): void { + this.activeJobs.clear(); + } +} + +export const backgroundRefresh = BackgroundRefresh.getInstance(); +export default backgroundRefresh; diff --git a/services/github/quota-monitor.ts b/services/github/quota-monitor.ts new file mode 100644 index 00000000..31980f87 --- /dev/null +++ b/services/github/quota-monitor.ts @@ -0,0 +1,97 @@ +export class QuotaMonitor { + private static instance: QuotaMonitor; + + private limit = 5000; + private remaining = 5000; + private resetTime = 0; + private totalRefreshes = 0; + + private constructor() {} + + public static getInstance(): QuotaMonitor { + if (!QuotaMonitor.instance) { + QuotaMonitor.instance = new QuotaMonitor(); + } + return QuotaMonitor.instance; + } + + /** + * Updates the internal quota state using standard GitHub response headers. + */ + public updateQuotaFromHeaders(headers: Headers | Record): void { + const getHeader = (name: string): string | null => { + if (headers instanceof Headers) { + return headers.get(name); + } + return headers[name] || headers[name.toLowerCase()] || null; + }; + + const limitHeader = getHeader('x-ratelimit-limit'); + const remainingHeader = getHeader('x-ratelimit-remaining'); + const resetHeader = getHeader('x-ratelimit-reset'); + + if (limitHeader) { + const parsedLimit = parseInt(limitHeader, 10); + if (!isNaN(parsedLimit)) this.limit = parsedLimit; + } + + if (remainingHeader) { + const parsedRemaining = parseInt(remainingHeader, 10); + if (!isNaN(parsedRemaining)) this.remaining = parsedRemaining; + } + + if (resetHeader) { + const parsedReset = parseInt(resetHeader, 10); + if (!isNaN(parsedReset)) this.resetTime = parsedReset * 1000; // Convert to ms + } + } + + /** + * Updates quota manually (useful for mocking/testing). + */ + public setQuota(limit: number, remaining: number, resetTimeMs: number): void { + this.limit = limit; + this.remaining = remaining; + this.resetTime = resetTimeMs; + } + + /** + * Returns current quota information. + */ + public getQuota() { + return { + limit: this.limit, + remaining: this.remaining, + resetTime: this.resetTime, + totalRefreshes: this.totalRefreshes, + }; + } + + /** + * Tracks a successful refresh event. + */ + public incrementRefreshCount(): void { + this.totalRefreshes++; + } + + /** + * Returns true if remaining quota is less than 10%. + */ + public isQuotaLow(): boolean { + // If remaining is less than 10% of limit, flag quota as low + return this.remaining < this.limit * 0.1; + } + + /** + * Resets monitor stats. + */ + public reset(): void { + this.limit = 5000; + this.remaining = 5000; + this.resetTime = 0; + this.totalRefreshes = 0; + } +} + +export const quotaMonitor = QuotaMonitor.getInstance(); +export default quotaMonitor; diff --git a/services/github/refresh-policy.ts b/services/github/refresh-policy.ts new file mode 100644 index 00000000..987d54e8 --- /dev/null +++ b/services/github/refresh-policy.ts @@ -0,0 +1,86 @@ +import { quotaMonitor } from './quota-monitor'; + +export class RefreshPolicy { + private static instance: RefreshPolicy; + + // Cooldown in milliseconds (default 5 minutes) + private cooldownMs = 5 * 60 * 1000; + + // Map of username -> last successful refresh timestamp + private refreshTimes = new Map(); + + private constructor() {} + + public static getInstance(): RefreshPolicy { + if (!RefreshPolicy.instance) { + RefreshPolicy.instance = new RefreshPolicy(); + } + return RefreshPolicy.instance; + } + + /** + * Set custom cooldown duration in milliseconds. + */ + public setCooldown(ms: number): void { + this.cooldownMs = Math.max(0, ms); + } + + /** + * Returns whether a refresh is permitted for the given username. + * + * A refresh is allowed if: + * 1. The username has not been refreshed within the cooldown window. + * 2. The global GitHub API token quota is not low. + */ + public isRefreshAllowed(username: string): boolean { + const sanitized = username.trim().toLowerCase(); + + // 1. Check if global quota is dangerously low + if (quotaMonitor.isQuotaLow()) { + return false; + } + + // 2. Check per-username cooldown + const lastRefresh = this.refreshTimes.get(sanitized); + if (!lastRefresh) { + return true; + } + + return Date.now() - lastRefresh >= this.cooldownMs; + } + + /** + * Records a successful refresh event for the username. + */ + public recordRefresh(username: string): void { + const sanitized = username.trim().toLowerCase(); + this.refreshTimes.set(sanitized, Date.now()); + quotaMonitor.incrementRefreshCount(); + } + + /** + * Gets the remaining cooldown time in milliseconds for a username. + * Returns 0 if no cooldown is active. + */ + public getRemainingCooldown(username: string): number { + const sanitized = username.trim().toLowerCase(); + const lastRefresh = this.refreshTimes.get(sanitized); + if (!lastRefresh) { + return 0; + } + + const elapsed = Date.now() - lastRefresh; + return Math.max(0, this.cooldownMs - elapsed); + } + + /** + * Clears the refresh times map (useful for testing). + */ + public reset(): void { + this.refreshTimes.clear(); + this.cooldownMs = 5 * 60 * 1000; + } +} + +export const refreshPolicy = RefreshPolicy.getInstance(); +export default refreshPolicy; diff --git a/services/github/refresh-rate-limiter.ts b/services/github/refresh-rate-limiter.ts new file mode 100644 index 00000000..cb699e95 --- /dev/null +++ b/services/github/refresh-rate-limiter.ts @@ -0,0 +1,101 @@ +interface RefreshLimitRecord { + count: number; + windowStart: number; +} + +export class RefreshRateLimiter { + private static instance: RefreshRateLimiter; + + // Default limits: 3 refreshes per hour + private limit = 3; + private windowMs = 60 * 60 * 1000; // 1 hour + + private tracker = new Map(); + + private constructor() { + this.loadLimitFromEnv(); + } + + public static getInstance(): RefreshRateLimiter { + if (!RefreshRateLimiter.instance) { + RefreshRateLimiter.instance = new RefreshRateLimiter(); + } + return RefreshRateLimiter.instance; + } + + private loadLimitFromEnv(): void { + const envLimit = process.env.MAX_REFRESHES_PER_HOUR; + if (envLimit) { + const parsed = parseInt(envLimit, 10); + if (!isNaN(parsed) && parsed > 0) { + this.limit = parsed; + } + } + } + + /** + * Set custom rate limit parameters (useful for tests). + */ + public setLimit(limit: number, windowMs = 60 * 60 * 1000): void { + this.limit = limit; + this.windowMs = windowMs; + } + + /** + * Checks if an IP is allowed to perform a manual cache refresh. + */ + public checkLimit(ip: string): { + success: boolean; + limit: number; + remaining: number; + reset: number; + } { + this.loadLimitFromEnv(); // Ensure latest env config is applied + const now = Date.now(); + const clientKey = ip.trim(); + + let record = this.tracker.get(clientKey); + + // If window expired or new client, reset the window + if (!record || now - record.windowStart >= this.windowMs) { + record = { + count: 0, + windowStart: now, + }; + this.tracker.set(clientKey, record); + } + + const resetTime = record.windowStart + this.windowMs; + + if (record.count >= this.limit) { + return { + success: false, + limit: this.limit, + remaining: 0, + reset: resetTime, + }; + } + + // Increment count on checking (optimistic allocation) + record.count++; + + return { + success: true, + limit: this.limit, + remaining: this.limit - record.count, + reset: resetTime, + }; + } + + /** + * Clears the limiter state (useful for tests). + */ + public reset(): void { + this.tracker.clear(); + this.limit = 3; + this.windowMs = 60 * 60 * 1000; + } +} + +export const refreshRateLimiter = RefreshRateLimiter.getInstance(); +export default refreshRateLimiter; From cca1cf93cea7257829d829b0318ca4dce005e338 Mon Sep 17 00:00:00 2001 From: anshika1179 Date: Sun, 31 May 2026 20:57:43 +0530 Subject: [PATCH 4/4] fix: prevent unauthenticated database storage abuse in track-user endpoint --- app/api/track-user/route.test.ts | 77 ++++++++++++++++- app/api/track-user/route.ts | 26 +++++- services/github/validate-user.ts | 65 +++++++++++++++ services/security/track-user-protection.ts | 97 ++++++++++++++++++++++ 4 files changed, 261 insertions(+), 4 deletions(-) create mode 100644 services/github/validate-user.ts create mode 100644 services/security/track-user-protection.ts diff --git a/app/api/track-user/route.test.ts b/app/api/track-user/route.test.ts index 6269cabb..155e052a 100644 --- a/app/api/track-user/route.test.ts +++ b/app/api/track-user/route.test.ts @@ -9,6 +9,7 @@ vi.mock('@/lib/rate-limit', () => ({ check: vi.fn().mockResolvedValue(true), }, })); + vi.mock('@/lib/mongodb', () => ({ default: vi.fn(), })); @@ -19,6 +20,20 @@ vi.mock('@/models/User', () => ({ }, })); +vi.mock('@/lib/github', () => ({ + fetchUserProfile: vi.fn().mockImplementation((username) => { + const lower = username.toLowerCase(); + if (lower === 'octocat' || lower === 'torvalds' || lower === 'valid-user') { + return Promise.resolve({ login: username }); + } + return Promise.reject(new Error('User not found')); + }), +})); + +import { fetchUserProfile } from '@/lib/github'; +import { trackUserProtection } from '@/services/security/track-user-protection'; +import { gitHubUserValidator } from '@/services/github/validate-user'; + function makeRequest(body: Record): Request { return new Request('http://localhost/api/track-user', { method: 'POST', @@ -30,6 +45,8 @@ function makeRequest(body: Record): Request { describe('POST /api/track-user', () => { beforeEach(() => { vi.clearAllMocks(); + trackUserProtection.reset(); + gitHubUserValidator.reset(); }); afterEach(() => { @@ -37,7 +54,65 @@ describe('POST /api/track-user', () => { delete process.env.MONGODB_URI; }); - describe('Validation', () => { + describe('Abuse Protection & Validation (Issue #1980)', () => { + // Scenario 1: Valid GitHub username (Stored) + it('Scenario 1: allows and stores a valid GitHub username', async () => { + process.env.MONGODB_URI = 'mongodb://localhost:27017/test'; + const response = await POST(makeRequest({ username: 'valid-user' })); + + expect(response.status).toBe(200); + const data = await response.json(); + expect(data.success).toBe(true); + expect(User.updateOne).toHaveBeenCalledWith({ username: 'valid-user' }, expect.any(Object), { + upsert: true, + }); + }); + + // Scenario 2: Invalid username (Rejected) + it('Scenario 2: rejects invalid GitHub username that does not exist', async () => { + process.env.MONGODB_URI = 'mongodb://localhost:27017/test'; + const response = await POST(makeRequest({ username: 'non-existent-user-12345' })); + + expect(response.status).toBe(400); + const data = await response.json(); + expect(data.success).toBe(false); + expect(data.error).toBe('Invalid GitHub username'); + expect(User.updateOne).not.toHaveBeenCalled(); + }); + + // Scenario 3: Random UUID (Rejected) + it('Scenario 3: rejects random UUID format immediately at regex format stage', async () => { + process.env.MONGODB_URI = 'mongodb://localhost:27017/test'; + const response = await POST(makeRequest({ username: 'invalid--username!123' })); + + expect(response.status).toBe(400); + const data = await response.json(); + expect(data.success).toBe(false); + expect(data.error).toBe('Invalid GitHub username'); + expect(fetchUserProfile).not.toHaveBeenCalled(); // Blocked before API lookup! + expect(User.updateOne).not.toHaveBeenCalled(); // Blocked before DB write! + }); + + // Scenario 4: Duplicate tracking request (cooldown deduplication) + it('Scenario 4: skips database write for duplicate tracking request within cooldown', async () => { + process.env.MONGODB_URI = 'mongodb://localhost:27017/test'; + + // First tracking allowed + const firstResponse = await POST(makeRequest({ username: 'valid-user' })); + expect(firstResponse.status).toBe(200); + expect(User.updateOne).toHaveBeenCalledTimes(1); + + // Second tracking within cooldown + const secondResponse = await POST(makeRequest({ username: 'valid-user' })); + expect(secondResponse.status).toBe(200); + const data = await secondResponse.json(); + expect(data.success).toBe(true); + expect(data.message).toBe('User already tracked recently'); + expect(User.updateOne).toHaveBeenCalledTimes(1); // Not incremented! + }); + }); + + describe('Validation Basic Checks', () => { it('returns 400 for malformed JSON request bodies', async () => { const malformedRequest = { json: vi.fn().mockRejectedValue(new SyntaxError('Unexpected token')), diff --git a/app/api/track-user/route.ts b/app/api/track-user/route.ts index 2e3dcfeb..af350dec 100644 --- a/app/api/track-user/route.ts +++ b/app/api/track-user/route.ts @@ -3,6 +3,7 @@ import dbConnect from '@/lib/mongodb'; import { User } from '@/models/User'; import { trackUserRateLimiter } from '@/lib/rate-limit'; import { getClientIp } from '@/utils/getClientIp'; +import { trackUserProtection } from '@/services/security/track-user-protection'; export async function POST(req: Request) { // Get IP for rate limiting securely @@ -38,6 +39,23 @@ export async function POST(req: Request) { const trimmedUsername = username.trim().toLowerCase(); + // Coordinate security validations and deduplication checks + const validation = await trackUserProtection.verifyAndDeduplicate(trimmedUsername); + if (!validation.allowed) { + if (validation.reason === 'COOLDOWN_ACTIVE') { + // Return 200 OK with duplicate track indicator to bypass write and keep response fast + return NextResponse.json( + { success: true, message: 'User already tracked recently' }, + { status: 200 } + ); + } + + return NextResponse.json( + { success: false, error: 'Invalid GitHub username' }, + { status: 400 } + ); + } + // If MONGODB_URI is not set, handle based on environment if (!process.env.MONGODB_URI) { // In production, this is a critical configuration failure @@ -53,6 +71,7 @@ export async function POST(req: Request) { // For development/non-production environments, bypass gracefully console.warn('MONGODB_URI is not set. Bypassing user tracking for local development.'); + trackUserProtection.recordWrite(trimmedUsername); return NextResponse.json({ success: true, bypassed: true }); } @@ -70,11 +89,11 @@ export async function POST(req: Request) { }, { upsert: true } ); + + // Record successful database write + trackUserProtection.recordWrite(trimmedUsername); } catch (upsertError) { // Gracefully handle MongoDB E11000 duplicate key race conditions under high concurrency. - // Concurrent upserts for the same username can race on the unique index, causing - // MongoDB to throw a duplicate key error (code 11000) for one of the requests. - // We can safely treat this as a successful no-op because another request already created it. if ( upsertError && typeof upsertError === 'object' && @@ -88,6 +107,7 @@ export async function POST(req: Request) { (typeof err.message === 'string' && err.message.includes('username')); if (isUsernameConflict) { + trackUserProtection.recordWrite(trimmedUsername); return NextResponse.json({ success: true }); } } diff --git a/services/github/validate-user.ts b/services/github/validate-user.ts new file mode 100644 index 00000000..73f7f6a8 --- /dev/null +++ b/services/github/validate-user.ts @@ -0,0 +1,65 @@ +import { fetchUserProfile } from '../../lib/github'; +import { TTLCache } from '../../lib/cache'; + +// 24-hour validation cache +const VALIDATION_CACHE_TTL_MS = 24 * 60 * 60 * 1000; + +export class GitHubUserValidator { + private static instance: GitHubUserValidator; + + // Cache stores username -> boolean existence status + private cache = new TTLCache(5000, 60 * 60 * 1000); + + private constructor() {} + + public static getInstance(): GitHubUserValidator { + if (!GitHubUserValidator.instance) { + GitHubUserValidator.instance = new GitHubUserValidator(); + } + return GitHubUserValidator.instance; + } + + /** + * Validates if the username represents a real, existing GitHub user. + * Results are cached for 24 hours to reduce API call overhead. + */ + public async validateUser(username: string): Promise { + const sanitized = username.trim().toLowerCase(); + + // Check cache first + const cachedStatus = this.cache.get(sanitized); + if (cachedStatus !== null) { + return cachedStatus; + } + + try { + // Query the GitHub profile endpoint + await fetchUserProfile(username, { bypassCache: false }); + + // User exists! Cache the result + this.cache.set(sanitized, true, VALIDATION_CACHE_TTL_MS); + return true; + } catch (err: unknown) { + const errMessage = err instanceof Error ? err.message : ''; + + if (errMessage.includes('User not found') || errMessage.includes('not found')) { + // User does not exist. Cache the negative result as well to prevent brute force abuse + this.cache.set(sanitized, false, VALIDATION_CACHE_TTL_MS); + return false; + } + + // For temporary API errors/rate limits, do not cache the failure permanently + throw err; + } + } + + /** + * Clears the validation cache (useful for tests). + */ + public reset(): void { + this.cache.clear(); + } +} + +export const gitHubUserValidator = GitHubUserValidator.getInstance(); +export default gitHubUserValidator; diff --git a/services/security/track-user-protection.ts b/services/security/track-user-protection.ts new file mode 100644 index 00000000..8fd0cd49 --- /dev/null +++ b/services/security/track-user-protection.ts @@ -0,0 +1,97 @@ +import { gitHubUserValidator } from '../github/validate-user'; + +// GitHub username rules: alphanumeric or single hyphens, max 39 chars, cannot start/end with hyphen +const GITHUB_USERNAME_REGEX = /^[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}$/i; + +// 5 minutes write cooldown to deduplicate writes/updates +const WRITE_COOLDOWN_MS = 5 * 60 * 1000; + +export class TrackUserProtection { + private static instance: TrackUserProtection; + + // Map of username -> last database write timestamp + private lastWriteTimes = new Map(); + + private constructor() {} + + public static getInstance(): TrackUserProtection { + if (!TrackUserProtection.instance) { + TrackUserProtection.instance = new TrackUserProtection(); + } + return TrackUserProtection.instance; + } + + /** + * Validates if the username format complies with official GitHub conventions. + * Prevents random UUIDs/junk injections before any API/DB calls are triggered. + */ + public validateFormat(username: string): boolean { + const trimmed = username.trim(); + if (!trimmed || trimmed.length > 39) { + return false; + } + return GITHUB_USERNAME_REGEX.test(trimmed); + } + + /** + * Checks if a database write is allowed for the username based on cooldown. + * Prevents spam updates/concurrency write spikes for the same user. + */ + public isWriteAllowed(username: string): boolean { + const sanitized = username.trim().toLowerCase(); + const lastWrite = this.lastWriteTimes.get(sanitized); + if (!lastWrite) { + return true; + } + return Date.now() - lastWrite >= WRITE_COOLDOWN_MS; + } + + /** + * Records a successful database write event for the username. + */ + public recordWrite(username: string): void { + const sanitized = username.trim().toLowerCase(); + this.lastWriteTimes.set(sanitized, Date.now()); + } + + /** + * Coordinates both format, cooldown, and existence checks. + */ + public async verifyAndDeduplicate(username: string): Promise<{ + allowed: boolean; + reason?: 'INVALID_FORMAT' | 'COOLDOWN_ACTIVE' | 'USER_NOT_FOUND'; + remainingMs?: number; + }> { + // 1. Verify format + if (!this.validateFormat(username)) { + return { allowed: false, reason: 'INVALID_FORMAT' }; + } + + const sanitized = username.trim().toLowerCase(); + + // 2. Check cooldown deduplication + if (!this.isWriteAllowed(sanitized)) { + const lastWrite = this.lastWriteTimes.get(sanitized) || 0; + const remainingMs = Math.max(0, WRITE_COOLDOWN_MS - (Date.now() - lastWrite)); + return { allowed: false, reason: 'COOLDOWN_ACTIVE', remainingMs }; + } + + // 3. Verify user exists on GitHub + const exists = await gitHubUserValidator.validateUser(username); + if (!exists) { + return { allowed: false, reason: 'USER_NOT_FOUND' }; + } + + return { allowed: true }; + } + + /** + * Resets active writes map (useful for testing). + */ + public reset(): void { + this.lastWriteTimes.clear(); + } +} + +export const trackUserProtection = TrackUserProtection.getInstance(); +export default trackUserProtection;