Skip to content

Commit cd029a3

Browse files
authored
Fix/track user storage exhaustion (JhaSourav07#2155)
## Description Fixes JhaSourav07#1980 This PR addresses the database storage exhaustion vulnerability where arbitrary usernames could be inserted into the tracking database without validation. ### Modifications implemented: * **GitHub Username Formatting Gate**: Implemented strict GitHub username regex validation to block UUIDs/malformed strings immediately. * **GitHub Existence Check**: Confirms if the user exists on GitHub before committing database writes. * **Validation Cache**: Caches lookup results (both valid and invalid) for 24 hours to optimize external rate limits. * **Write Cooldown Deduplication**: Limits tracking writes to at most once every 5 minutes per user, gracefully skipping redundant DB upserts. ## Pillar - [ ] 🎨 Pillar 1 — New Theme Design - [ ] 📐 Pillar 2 — Geometric SVG Improvement - [ ] 🕐 Pillar 3 — Timezone Logic Optimization - [x] 🛠️ Other (Bug fix, refactoring, docs) ## Visual Preview *(N/A - Security backend/middleware hardening change. Complete test suite validation has been performed locally).* ## Checklist before requesting a review: - [x] I have read the `CONTRIBUTING.md` file. - [x] I have tested these changes locally (`localhost:3000/api/streak?user=YOUR_USERNAME`). - [x] I have run `npm run format` and `npm run lint` locally and resolved all errors (CI will fail otherwise). - [x] My commits follow the Conventional Commits format (e.g., `feat(themes): ...`, `fix(calculate): ...`). - [x] I have updated `README.md` if I added a new theme or URL parameter. - [x] I have started the repo. - [x] I have made sure that i have only one commit to merge in this PR. - [x] The SVG output matches the CommitPulse "premium quality" aesthetic standard (no raw elements, smooth animations, correct fonts). - [x] (Recommended) I joined the CommitPulse Discord community for contributor discussions, mentorship, and faster PR support.
2 parents 7817ae0 + cca1cf9 commit cd029a3

18 files changed

Lines changed: 1249 additions & 89 deletions

app/(root)/dashboard/[username]/page.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ describe('DashboardPage', () => {
100100
achievements: [],
101101
commitClock: [],
102102
graphData: { nodes: [], links: [] },
103+
lastSyncedAt: undefined,
103104
};
104105

105106
beforeEach(() => {

app/api/github/route.test.ts

Lines changed: 113 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,98 +1,166 @@
11
import { describe, it, expect, vi, beforeEach } from 'vitest';
22
import { GET } from './route';
3-
// Replace the real GitHub API with a fake function so tests can run without hitting real APIs
3+
4+
// Replace the real GitHub API with a fake function
45
vi.mock('../../../lib/github', () => ({
56
getFullDashboardData: vi.fn(),
67
}));
78

89
import { getFullDashboardData } from '../../../lib/github';
9-
10-
function makeRequest(params: Record<string, string> = {}): Request {
10+
import { quotaMonitor } from '@/services/github/quota-monitor';
11+
import { refreshPolicy } from '@/services/github/refresh-policy';
12+
import { refreshRateLimiter } from '@/services/github/refresh-rate-limiter';
13+
import { backgroundRefresh } from '@/services/github/background-refresh';
14+
15+
function makeRequest(
16+
params: Record<string, string> = {},
17+
headers: Record<string, string> = {}
18+
): Request {
1119
const url = new URL('http://localhost/api/github');
1220
for (const [key, value] of Object.entries(params)) {
1321
url.searchParams.set(key, value);
1422
}
15-
return new Request(url.toString());
23+
return new Request(url.toString(), {
24+
headers: new Headers(headers),
25+
});
1626
}
1727

1828
describe('GET /api/github', () => {
1929
beforeEach(() => {
2030
vi.clearAllMocks();
21-
vi.mocked(getFullDashboardData).mockResolvedValue({} as never);
31+
vi.mocked(getFullDashboardData).mockResolvedValue({
32+
profile: { lastSyncedAt: new Date().toISOString() },
33+
calendar: {},
34+
lastSyncedAt: new Date().toISOString(),
35+
} as unknown as Awaited<ReturnType<typeof getFullDashboardData>>);
36+
37+
quotaMonitor.reset();
38+
refreshPolicy.reset();
39+
refreshRateLimiter.reset();
40+
backgroundRefresh.reset();
2241
});
2342

24-
describe('cache bypass via ?refresh=true', () => {
25-
it('calls getFullDashboardData with { bypassCache: true } when ?refresh=true', async () => {
26-
await GET(makeRequest({ username: 'octocat', refresh: 'true' }));
43+
describe('Unrestricted Cache Bypass & Abuse Mitigation (Issue #1978)', () => {
44+
// Scenario 1: Normal cached request
45+
it('Scenario 1: serves cached data and checks SWR background refresh', async () => {
46+
// Mock data that is stale (15 minutes ago)
47+
const staleTime = new Date(Date.now() - 15 * 60 * 1000).toISOString();
48+
vi.mocked(getFullDashboardData).mockResolvedValue({
49+
profile: { lastSyncedAt: staleTime },
50+
calendar: {},
51+
} as unknown as Awaited<ReturnType<typeof getFullDashboardData>>);
2752

28-
expect(getFullDashboardData).toHaveBeenCalledWith('octocat', { bypassCache: true });
29-
});
53+
const triggerSpy = vi.spyOn(backgroundRefresh, 'triggerRefresh');
3054

31-
it('calls getFullDashboardData with { bypassCache: false } when refresh is omitted', async () => {
32-
await GET(makeRequest({ username: 'octocat' }));
33-
expect(getFullDashboardData).toHaveBeenCalledWith('octocat', {
34-
bypassCache: false,
35-
});
55+
const response = await GET(makeRequest({ username: 'torvalds' }));
56+
expect(response.status).toBe(200);
57+
expect(getFullDashboardData).toHaveBeenCalledWith('torvalds', { bypassCache: false });
58+
expect(triggerSpy).toHaveBeenCalledWith('torvalds');
3659
});
37-
it('returns 400 when username contains invalid characters', async () => {
38-
const response = await GET(makeRequest({ username: '@@@@@' }));
39-
const body = await response.json();
4060

41-
expect(response.status).toBe(400);
42-
expect(body.error).toContain('Invalid parameters');
61+
// Scenario 2: Single refresh request allowed
62+
it('Scenario 2: allows a single refresh request when limits are respected', async () => {
63+
const response = await GET(makeRequest({ username: 'torvalds', refresh: 'true' }));
64+
65+
expect(response.status).toBe(200);
66+
expect(getFullDashboardData).toHaveBeenCalledWith('torvalds', { bypassCache: true });
67+
expect(response.headers.get('X-Refresh-Status')).toBe('Fresh');
4368
});
4469

45-
it('returns 400 when username contains only whitespace', async () => {
46-
const response = await GET(makeRequest({ username: ' ' }));
47-
const body = await response.json();
70+
// Scenario 3: Repeated refresh within cooldown served from cache
71+
it('Scenario 3: serves cached response for repeated refresh requests within cooldown', async () => {
72+
// First refresh is allowed
73+
await GET(makeRequest({ username: 'torvalds', refresh: 'true' }));
74+
expect(getFullDashboardData).toHaveBeenLastCalledWith('torvalds', { bypassCache: true });
4875

49-
expect(response.status).toBe(400);
50-
expect(body.error).toContain('Invalid parameters');
76+
// Second refresh within cooldown (5 minutes)
77+
const response = await GET(makeRequest({ username: 'torvalds', refresh: 'true' }));
78+
79+
expect(response.status).toBe(200);
80+
// Cooldown fallback triggers cached read
81+
expect(getFullDashboardData).toHaveBeenLastCalledWith('torvalds', { bypassCache: false });
82+
expect(response.headers.get('X-Refresh-Status')).toBe('Cooldown-Served-Cached');
5183
});
5284

53-
it('returns 400 when username exceeds GitHub maximum length', async () => {
85+
// Scenario 4: Refresh rate limit exceeded per client IP
86+
it('Scenario 4: returns 429 when client refresh rate limit is exceeded', async () => {
87+
// Set rate limit to 2 per window for testing
88+
refreshRateLimiter.setLimit(2);
89+
90+
// Refresh 1
91+
await GET(
92+
makeRequest({ username: 'torvalds', refresh: 'true' }, { 'x-real-ip': '203.0.113.5' })
93+
);
94+
// Refresh 2
95+
await GET(
96+
makeRequest({ username: 'octocat', refresh: 'true' }, { 'x-real-ip': '203.0.113.5' })
97+
);
98+
99+
// Refresh 3 (exceeds limit of 2)
54100
const response = await GET(
55-
makeRequest({
56-
username: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
57-
})
101+
makeRequest({ username: 'torvalds', refresh: 'true' }, { 'x-real-ip': '203.0.113.5' })
58102
);
59103

104+
expect(response.status).toBe(429);
60105
const body = await response.json();
106+
expect(body.error).toContain('Refresh rate limit exceeded');
107+
});
61108

62-
expect(response.status).toBe(400);
63-
expect(body.error).toContain('Invalid parameters');
109+
// Scenario 5: Low GitHub quota blocks refresh
110+
it('Scenario 5: blocks manual refresh when remaining GitHub quota is low (<10%)', async () => {
111+
// Set global remaining quota to 400 out of 5000 (8%)
112+
quotaMonitor.setQuota(5000, 400, Date.now() + 60000);
113+
114+
const response = await GET(makeRequest({ username: 'torvalds', refresh: 'true' }));
115+
116+
expect(response.status).toBe(429);
117+
const body = await response.json();
118+
expect(body.error).toContain('quota is low');
119+
expect(getFullDashboardData).not.toHaveBeenCalled();
64120
});
65121

66-
// Test 1 — missing username → 400
67-
it('returns 400 when username is missing', async () => {
68-
const response = await GET(makeRequest());
122+
// Scenario 6: Background refresh execution
123+
it('Scenario 6: asynchronous background refresh completes successfully', async () => {
124+
const loadSpy = vi.spyOn(backgroundRefresh, 'triggerRefresh');
125+
126+
// Mock data that is stale
127+
const staleTime = new Date(Date.now() - 15 * 60 * 1000).toISOString();
128+
vi.mocked(getFullDashboardData).mockResolvedValue({
129+
profile: { lastSyncedAt: staleTime },
130+
calendar: {},
131+
} as unknown as Awaited<ReturnType<typeof getFullDashboardData>>);
132+
133+
await GET(makeRequest({ username: 'torvalds' }));
134+
135+
expect(loadSpy).toHaveBeenCalledWith('torvalds');
136+
});
137+
});
138+
139+
describe('Standard route behavior', () => {
140+
it('returns 400 when username contains invalid characters', async () => {
141+
const response = await GET(makeRequest({ username: '@@@@@' }));
69142
const body = await response.json();
70143

71144
expect(response.status).toBe(400);
72145
expect(body.error).toContain('Invalid parameters');
73146
});
74147

75-
it('returns 400 and skips GitHub when username format is invalid', async () => {
76-
const response = await GET(makeRequest({ username: 'bad user' }));
148+
it('returns 400 when username contains only whitespace', async () => {
149+
const response = await GET(makeRequest({ username: ' ' }));
77150
const body = await response.json();
78151

79152
expect(response.status).toBe(400);
80153
expect(body.error).toContain('Invalid parameters');
81-
expect(getFullDashboardData).not.toHaveBeenCalled();
82154
});
83155

84-
// Test 2 — valid username → 200
85-
it('returns 200 with JSON body for a valid username', async () => {
86-
vi.mocked(getFullDashboardData).mockResolvedValue({ profile: 'octocat' } as never);
87-
88-
const response = await GET(makeRequest({ username: 'octocat' }));
156+
it('returns 400 when username is missing', async () => {
157+
const response = await GET(makeRequest());
89158
const body = await response.json();
90159

91-
expect(response.status).toBe(200);
92-
expect(body).toEqual({ profile: 'octocat' });
160+
expect(response.status).toBe(400);
161+
expect(body.error).toContain('Invalid parameters');
93162
});
94163

95-
// Test 3 — throws 'User not found' → 404
96164
it('returns 404 when getFullDashboardData throws User not found', async () => {
97165
vi.mocked(getFullDashboardData).mockRejectedValue(new Error('User not found'));
98166

@@ -102,27 +170,5 @@ describe('GET /api/github', () => {
102170
expect(response.status).toBe(404);
103171
expect(body.error).toContain('User not found');
104172
});
105-
106-
// Test 4 — throws 'API limit reached' → 403
107-
it('returns 403 when getFullDashboardData throws API limit reached', async () => {
108-
vi.mocked(getFullDashboardData).mockRejectedValue(new Error('API limit reached'));
109-
110-
const response = await GET(makeRequest({ username: 'octocat' }));
111-
const body = await response.json();
112-
113-
expect(response.status).toBe(403);
114-
expect(body.error).toContain('rate limit');
115-
});
116-
117-
// Test 5 — throws generic error → 500
118-
it('returns 500 for a generic unexpected error', async () => {
119-
vi.mocked(getFullDashboardData).mockRejectedValue(new Error('Something went wrong'));
120-
121-
const response = await GET(makeRequest({ username: 'octocat' }));
122-
const body = await response.json();
123-
124-
expect(response.status).toBe(500);
125-
expect(body.error).toContain('Something went wrong');
126-
});
127173
});
128174
});

app/api/github/route.ts

Lines changed: 86 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,22 @@
33
import { NextResponse } from 'next/server';
44
import { getFullDashboardData } from '@/lib/github';
55
import { githubParamsSchema } from '@/lib/validations';
6+
import { getClientIp } from '@/utils/getClientIp';
7+
import { quotaMonitor } from '@/services/github/quota-monitor';
8+
import { refreshPolicy } from '@/services/github/refresh-policy';
9+
import { refreshRateLimiter } from '@/services/github/refresh-rate-limiter';
10+
import { backgroundRefresh } from '@/services/github/background-refresh';
11+
12+
function logSecurityEvent(event: string, details: Record<string, unknown>) {
13+
console.warn(
14+
JSON.stringify({
15+
timestamp: new Date().toISOString(),
16+
type: 'SECURITY_EVENT',
17+
event,
18+
...details,
19+
})
20+
);
21+
}
622

723
/**
824
* Returns GitHub dashboard data as JSON.
@@ -18,11 +34,12 @@ import { githubParamsSchema } from '@/lib/validations';
1834
* - 400 → Invalid query parameters
1935
* - 403 → GitHub API rate limit reached
2036
* - 404 → GitHub user not found
37+
* - 429 → Too many requests (Refresh rate limit or low quota)
2138
* - 500 → Internal server error
2239
*/
23-
2440
export async function GET(request: Request) {
2541
const { searchParams } = new URL(request.url);
42+
const ip = getClientIp(request);
2643

2744
const parseResult = githubParamsSchema.safeParse(Object.fromEntries(searchParams.entries()));
2845

@@ -35,16 +52,82 @@ export async function GET(request: Request) {
3552

3653
const { username, refresh } = parseResult.data;
3754

55+
// 1. Quota awareness check - if remaining quota is low, disable manual refresh
56+
if (refresh && quotaMonitor.isQuotaLow()) {
57+
logSecurityEvent('LOW_QUOTA_REFRESH_BLOCKED', {
58+
username,
59+
ip,
60+
remainingQuota: quotaMonitor.getQuota().remaining,
61+
});
62+
return NextResponse.json(
63+
{ error: 'GitHub API quota is low. Cache refresh temporarily disabled.' },
64+
{ status: 429 }
65+
);
66+
}
67+
68+
// 2. Separate Refresh Rate Limiter
69+
if (refresh) {
70+
const rateLimitCheck = refreshRateLimiter.checkLimit(ip);
71+
if (!rateLimitCheck.success) {
72+
logSecurityEvent('REFRESH_RATE_LIMIT_EXCEEDED', {
73+
username,
74+
ip,
75+
limit: rateLimitCheck.limit,
76+
});
77+
return NextResponse.json(
78+
{ error: 'Refresh rate limit exceeded. Please try again later.' },
79+
{
80+
status: 429,
81+
headers: {
82+
'X-RateLimit-Limit': rateLimitCheck.limit.toString(),
83+
'X-RateLimit-Remaining': rateLimitCheck.remaining.toString(),
84+
'X-RateLimit-Reset': rateLimitCheck.reset.toString(),
85+
},
86+
}
87+
);
88+
}
89+
}
90+
91+
// 3. Per-Username Refresh Cooldown
92+
let shouldBypassCache = refresh;
93+
if (refresh) {
94+
if (!refreshPolicy.isRefreshAllowed(username)) {
95+
logSecurityEvent('REFRESH_COOLDOWN_VIOLATION', {
96+
username,
97+
ip,
98+
remainingMs: refreshPolicy.getRemainingCooldown(username),
99+
});
100+
// Fallback: serve cached data instead of bypassing cache
101+
shouldBypassCache = false;
102+
} else {
103+
refreshPolicy.recordRefresh(username);
104+
}
105+
}
106+
38107
try {
39-
const data = await getFullDashboardData(username, { bypassCache: refresh });
40-
const cacheControl = refresh
108+
const data = await getFullDashboardData(username, { bypassCache: shouldBypassCache });
109+
110+
// 4. Stale-While-Revalidate background refresh for normal cached requests
111+
if (!shouldBypassCache) {
112+
const lastSynced = data.lastSyncedAt;
113+
if (backgroundRefresh.isStale(lastSynced)) {
114+
backgroundRefresh.triggerRefresh(username);
115+
}
116+
}
117+
118+
const cacheControl = shouldBypassCache
41119
? 'no-cache, no-store, must-revalidate'
42120
: 's-maxage=3600, stale-while-revalidate=86400';
43121

44122
return NextResponse.json(data, {
45123
status: 200,
46124
headers: {
47125
'Cache-Control': cacheControl,
126+
'X-Refresh-Status': shouldBypassCache
127+
? 'Fresh'
128+
: refresh
129+
? 'Cooldown-Served-Cached'
130+
: 'Cached',
48131
},
49132
});
50133
} catch (error: unknown) {

0 commit comments

Comments
 (0)