Skip to content

Commit cca1cf9

Browse files
committed
fix: prevent unauthenticated database storage abuse in track-user endpoint
1 parent 22120f0 commit cca1cf9

4 files changed

Lines changed: 261 additions & 4 deletions

File tree

app/api/track-user/route.test.ts

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ vi.mock('@/lib/rate-limit', () => ({
99
check: vi.fn().mockResolvedValue(true),
1010
},
1111
}));
12+
1213
vi.mock('@/lib/mongodb', () => ({
1314
default: vi.fn(),
1415
}));
@@ -19,6 +20,20 @@ vi.mock('@/models/User', () => ({
1920
},
2021
}));
2122

23+
vi.mock('@/lib/github', () => ({
24+
fetchUserProfile: vi.fn().mockImplementation((username) => {
25+
const lower = username.toLowerCase();
26+
if (lower === 'octocat' || lower === 'torvalds' || lower === 'valid-user') {
27+
return Promise.resolve({ login: username });
28+
}
29+
return Promise.reject(new Error('User not found'));
30+
}),
31+
}));
32+
33+
import { fetchUserProfile } from '@/lib/github';
34+
import { trackUserProtection } from '@/services/security/track-user-protection';
35+
import { gitHubUserValidator } from '@/services/github/validate-user';
36+
2237
function makeRequest(body: Record<string, unknown>): Request {
2338
return new Request('http://localhost/api/track-user', {
2439
method: 'POST',
@@ -30,14 +45,74 @@ function makeRequest(body: Record<string, unknown>): Request {
3045
describe('POST /api/track-user', () => {
3146
beforeEach(() => {
3247
vi.clearAllMocks();
48+
trackUserProtection.reset();
49+
gitHubUserValidator.reset();
3350
});
3451

3552
afterEach(() => {
3653
// Clean up environment variables
3754
delete process.env.MONGODB_URI;
3855
});
3956

40-
describe('Validation', () => {
57+
describe('Abuse Protection & Validation (Issue #1980)', () => {
58+
// Scenario 1: Valid GitHub username (Stored)
59+
it('Scenario 1: allows and stores a valid GitHub username', async () => {
60+
process.env.MONGODB_URI = 'mongodb://localhost:27017/test';
61+
const response = await POST(makeRequest({ username: 'valid-user' }));
62+
63+
expect(response.status).toBe(200);
64+
const data = await response.json();
65+
expect(data.success).toBe(true);
66+
expect(User.updateOne).toHaveBeenCalledWith({ username: 'valid-user' }, expect.any(Object), {
67+
upsert: true,
68+
});
69+
});
70+
71+
// Scenario 2: Invalid username (Rejected)
72+
it('Scenario 2: rejects invalid GitHub username that does not exist', async () => {
73+
process.env.MONGODB_URI = 'mongodb://localhost:27017/test';
74+
const response = await POST(makeRequest({ username: 'non-existent-user-12345' }));
75+
76+
expect(response.status).toBe(400);
77+
const data = await response.json();
78+
expect(data.success).toBe(false);
79+
expect(data.error).toBe('Invalid GitHub username');
80+
expect(User.updateOne).not.toHaveBeenCalled();
81+
});
82+
83+
// Scenario 3: Random UUID (Rejected)
84+
it('Scenario 3: rejects random UUID format immediately at regex format stage', async () => {
85+
process.env.MONGODB_URI = 'mongodb://localhost:27017/test';
86+
const response = await POST(makeRequest({ username: 'invalid--username!123' }));
87+
88+
expect(response.status).toBe(400);
89+
const data = await response.json();
90+
expect(data.success).toBe(false);
91+
expect(data.error).toBe('Invalid GitHub username');
92+
expect(fetchUserProfile).not.toHaveBeenCalled(); // Blocked before API lookup!
93+
expect(User.updateOne).not.toHaveBeenCalled(); // Blocked before DB write!
94+
});
95+
96+
// Scenario 4: Duplicate tracking request (cooldown deduplication)
97+
it('Scenario 4: skips database write for duplicate tracking request within cooldown', async () => {
98+
process.env.MONGODB_URI = 'mongodb://localhost:27017/test';
99+
100+
// First tracking allowed
101+
const firstResponse = await POST(makeRequest({ username: 'valid-user' }));
102+
expect(firstResponse.status).toBe(200);
103+
expect(User.updateOne).toHaveBeenCalledTimes(1);
104+
105+
// Second tracking within cooldown
106+
const secondResponse = await POST(makeRequest({ username: 'valid-user' }));
107+
expect(secondResponse.status).toBe(200);
108+
const data = await secondResponse.json();
109+
expect(data.success).toBe(true);
110+
expect(data.message).toBe('User already tracked recently');
111+
expect(User.updateOne).toHaveBeenCalledTimes(1); // Not incremented!
112+
});
113+
});
114+
115+
describe('Validation Basic Checks', () => {
41116
it('returns 400 for malformed JSON request bodies', async () => {
42117
const malformedRequest = {
43118
json: vi.fn().mockRejectedValue(new SyntaxError('Unexpected token')),

app/api/track-user/route.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import dbConnect from '@/lib/mongodb';
33
import { User } from '@/models/User';
44
import { trackUserRateLimiter } from '@/lib/rate-limit';
55
import { getClientIp } from '@/utils/getClientIp';
6+
import { trackUserProtection } from '@/services/security/track-user-protection';
67

78
export async function POST(req: Request) {
89
// Get IP for rate limiting securely
@@ -38,6 +39,23 @@ export async function POST(req: Request) {
3839

3940
const trimmedUsername = username.trim().toLowerCase();
4041

42+
// Coordinate security validations and deduplication checks
43+
const validation = await trackUserProtection.verifyAndDeduplicate(trimmedUsername);
44+
if (!validation.allowed) {
45+
if (validation.reason === 'COOLDOWN_ACTIVE') {
46+
// Return 200 OK with duplicate track indicator to bypass write and keep response fast
47+
return NextResponse.json(
48+
{ success: true, message: 'User already tracked recently' },
49+
{ status: 200 }
50+
);
51+
}
52+
53+
return NextResponse.json(
54+
{ success: false, error: 'Invalid GitHub username' },
55+
{ status: 400 }
56+
);
57+
}
58+
4159
// If MONGODB_URI is not set, handle based on environment
4260
if (!process.env.MONGODB_URI) {
4361
// In production, this is a critical configuration failure
@@ -53,6 +71,7 @@ export async function POST(req: Request) {
5371

5472
// For development/non-production environments, bypass gracefully
5573
console.warn('MONGODB_URI is not set. Bypassing user tracking for local development.');
74+
trackUserProtection.recordWrite(trimmedUsername);
5675
return NextResponse.json({ success: true, bypassed: true });
5776
}
5877

@@ -70,11 +89,11 @@ export async function POST(req: Request) {
7089
},
7190
{ upsert: true }
7291
);
92+
93+
// Record successful database write
94+
trackUserProtection.recordWrite(trimmedUsername);
7395
} catch (upsertError) {
7496
// Gracefully handle MongoDB E11000 duplicate key race conditions under high concurrency.
75-
// Concurrent upserts for the same username can race on the unique index, causing
76-
// MongoDB to throw a duplicate key error (code 11000) for one of the requests.
77-
// We can safely treat this as a successful no-op because another request already created it.
7897
if (
7998
upsertError &&
8099
typeof upsertError === 'object' &&
@@ -88,6 +107,7 @@ export async function POST(req: Request) {
88107
(typeof err.message === 'string' && err.message.includes('username'));
89108

90109
if (isUsernameConflict) {
110+
trackUserProtection.recordWrite(trimmedUsername);
91111
return NextResponse.json({ success: true });
92112
}
93113
}

services/github/validate-user.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { fetchUserProfile } from '../../lib/github';
2+
import { TTLCache } from '../../lib/cache';
3+
4+
// 24-hour validation cache
5+
const VALIDATION_CACHE_TTL_MS = 24 * 60 * 60 * 1000;
6+
7+
export class GitHubUserValidator {
8+
private static instance: GitHubUserValidator;
9+
10+
// Cache stores username -> boolean existence status
11+
private cache = new TTLCache<boolean>(5000, 60 * 60 * 1000);
12+
13+
private constructor() {}
14+
15+
public static getInstance(): GitHubUserValidator {
16+
if (!GitHubUserValidator.instance) {
17+
GitHubUserValidator.instance = new GitHubUserValidator();
18+
}
19+
return GitHubUserValidator.instance;
20+
}
21+
22+
/**
23+
* Validates if the username represents a real, existing GitHub user.
24+
* Results are cached for 24 hours to reduce API call overhead.
25+
*/
26+
public async validateUser(username: string): Promise<boolean> {
27+
const sanitized = username.trim().toLowerCase();
28+
29+
// Check cache first
30+
const cachedStatus = this.cache.get(sanitized);
31+
if (cachedStatus !== null) {
32+
return cachedStatus;
33+
}
34+
35+
try {
36+
// Query the GitHub profile endpoint
37+
await fetchUserProfile(username, { bypassCache: false });
38+
39+
// User exists! Cache the result
40+
this.cache.set(sanitized, true, VALIDATION_CACHE_TTL_MS);
41+
return true;
42+
} catch (err: unknown) {
43+
const errMessage = err instanceof Error ? err.message : '';
44+
45+
if (errMessage.includes('User not found') || errMessage.includes('not found')) {
46+
// User does not exist. Cache the negative result as well to prevent brute force abuse
47+
this.cache.set(sanitized, false, VALIDATION_CACHE_TTL_MS);
48+
return false;
49+
}
50+
51+
// For temporary API errors/rate limits, do not cache the failure permanently
52+
throw err;
53+
}
54+
}
55+
56+
/**
57+
* Clears the validation cache (useful for tests).
58+
*/
59+
public reset(): void {
60+
this.cache.clear();
61+
}
62+
}
63+
64+
export const gitHubUserValidator = GitHubUserValidator.getInstance();
65+
export default gitHubUserValidator;
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import { gitHubUserValidator } from '../github/validate-user';
2+
3+
// GitHub username rules: alphanumeric or single hyphens, max 39 chars, cannot start/end with hyphen
4+
const GITHUB_USERNAME_REGEX = /^[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}$/i;
5+
6+
// 5 minutes write cooldown to deduplicate writes/updates
7+
const WRITE_COOLDOWN_MS = 5 * 60 * 1000;
8+
9+
export class TrackUserProtection {
10+
private static instance: TrackUserProtection;
11+
12+
// Map of username -> last database write timestamp
13+
private lastWriteTimes = new Map<string, number>();
14+
15+
private constructor() {}
16+
17+
public static getInstance(): TrackUserProtection {
18+
if (!TrackUserProtection.instance) {
19+
TrackUserProtection.instance = new TrackUserProtection();
20+
}
21+
return TrackUserProtection.instance;
22+
}
23+
24+
/**
25+
* Validates if the username format complies with official GitHub conventions.
26+
* Prevents random UUIDs/junk injections before any API/DB calls are triggered.
27+
*/
28+
public validateFormat(username: string): boolean {
29+
const trimmed = username.trim();
30+
if (!trimmed || trimmed.length > 39) {
31+
return false;
32+
}
33+
return GITHUB_USERNAME_REGEX.test(trimmed);
34+
}
35+
36+
/**
37+
* Checks if a database write is allowed for the username based on cooldown.
38+
* Prevents spam updates/concurrency write spikes for the same user.
39+
*/
40+
public isWriteAllowed(username: string): boolean {
41+
const sanitized = username.trim().toLowerCase();
42+
const lastWrite = this.lastWriteTimes.get(sanitized);
43+
if (!lastWrite) {
44+
return true;
45+
}
46+
return Date.now() - lastWrite >= WRITE_COOLDOWN_MS;
47+
}
48+
49+
/**
50+
* Records a successful database write event for the username.
51+
*/
52+
public recordWrite(username: string): void {
53+
const sanitized = username.trim().toLowerCase();
54+
this.lastWriteTimes.set(sanitized, Date.now());
55+
}
56+
57+
/**
58+
* Coordinates both format, cooldown, and existence checks.
59+
*/
60+
public async verifyAndDeduplicate(username: string): Promise<{
61+
allowed: boolean;
62+
reason?: 'INVALID_FORMAT' | 'COOLDOWN_ACTIVE' | 'USER_NOT_FOUND';
63+
remainingMs?: number;
64+
}> {
65+
// 1. Verify format
66+
if (!this.validateFormat(username)) {
67+
return { allowed: false, reason: 'INVALID_FORMAT' };
68+
}
69+
70+
const sanitized = username.trim().toLowerCase();
71+
72+
// 2. Check cooldown deduplication
73+
if (!this.isWriteAllowed(sanitized)) {
74+
const lastWrite = this.lastWriteTimes.get(sanitized) || 0;
75+
const remainingMs = Math.max(0, WRITE_COOLDOWN_MS - (Date.now() - lastWrite));
76+
return { allowed: false, reason: 'COOLDOWN_ACTIVE', remainingMs };
77+
}
78+
79+
// 3. Verify user exists on GitHub
80+
const exists = await gitHubUserValidator.validateUser(username);
81+
if (!exists) {
82+
return { allowed: false, reason: 'USER_NOT_FOUND' };
83+
}
84+
85+
return { allowed: true };
86+
}
87+
88+
/**
89+
* Resets active writes map (useful for testing).
90+
*/
91+
public reset(): void {
92+
this.lastWriteTimes.clear();
93+
}
94+
}
95+
96+
export const trackUserProtection = TrackUserProtection.getInstance();
97+
export default trackUserProtection;

0 commit comments

Comments
 (0)