Skip to content

Commit 2476b24

Browse files
authored
fix(api): add Zod validation and username sanitization to /api/compare (JhaSourav07#2093)
## Description Fixes JhaSourav07#2068 Replace manual null-check validation in `/api/compare` with `compareParamsSchema` (Zod v4) that validates GitHub username format via `GITHUB_USERNAME_REGEX`, enforces max length of 39 chars, and includes self-comparison check as a schema refinement. ### Changes - **`lib/validations.ts`** — Added `compareParamsSchema` and `CompareParams` type - **`app/api/compare/route.ts`** — Replaced manual validation with Zod `safeParse` + `error.flatten()` - **`app/api/compare/route.test.ts`** — New test file with 11 tests covering validation, error handling, and success paths ## Pillar - [x] 🛠️ Other (Bug fix, refactoring, docs) ## Visual Preview No visual changes — this is a validation/security fix for a JSON API endpoint. ## 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): ...`). - [ ] 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).
2 parents 7bb2eef + 38f117f commit 2476b24

3 files changed

Lines changed: 121 additions & 7 deletions

File tree

app/api/compare/route.test.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { describe, it, expect, vi, beforeEach } from 'vitest';
2+
import { GET } from './route';
3+
4+
vi.mock('@/lib/github', () => ({
5+
getFullDashboardData: vi.fn(),
6+
}));
7+
8+
import { getFullDashboardData } from '@/lib/github';
9+
10+
const makeRequest = (search: string) => new Request(`http://localhost:3000/api/compare?${search}`);
11+
12+
describe('GET /api/compare', () => {
13+
beforeEach(() => {
14+
vi.clearAllMocks();
15+
vi.mocked(getFullDashboardData).mockResolvedValue({
16+
calendar: { totalContributions: 50, weeks: [] },
17+
} as never);
18+
});
19+
20+
// ── Validation ────────────────────────────────────────────────────────────
21+
22+
it('returns 400 when user1 is missing', async () => {
23+
const res = await GET(makeRequest('user2=octocat'));
24+
expect(res.status).toBe(400);
25+
});
26+
27+
it('returns 400 when user2 is missing', async () => {
28+
const res = await GET(makeRequest('user1=octocat'));
29+
expect(res.status).toBe(400);
30+
});
31+
32+
it('returns 400 when both users are missing', async () => {
33+
const res = await GET(makeRequest(''));
34+
expect(res.status).toBe(400);
35+
});
36+
37+
it('returns 400 for invalid GitHub username format for user1', async () => {
38+
const res = await GET(makeRequest('user1=-invalid&user2=octocat'));
39+
expect(res.status).toBe(400);
40+
const data = await res.json();
41+
expect(data.details.fieldErrors.user1).toBeDefined();
42+
});
43+
44+
it('returns 400 for invalid GitHub username format for user2', async () => {
45+
const res = await GET(makeRequest('user1=octocat&user2=-invalid'));
46+
expect(res.status).toBe(400);
47+
const data = await res.json();
48+
expect(data.details.fieldErrors.user2).toBeDefined();
49+
});
50+
51+
it('returns 400 for username exceeding 39 characters', async () => {
52+
const res = await GET(makeRequest(`user1=${'a'.repeat(40)}&user2=octocat`));
53+
expect(res.status).toBe(400);
54+
});
55+
56+
it('returns 400 when comparing a user with themselves', async () => {
57+
const res = await GET(makeRequest('user1=octocat&user2=octocat'));
58+
expect(res.status).toBe(400);
59+
const data = await res.json();
60+
expect(data.details.fieldErrors.user2).toContain('Cannot compare a user with themselves.');
61+
});
62+
63+
it('returns 400 for self-comparison regardless of case', async () => {
64+
const res = await GET(makeRequest('user1=OctoCat&user2=octocat'));
65+
expect(res.status).toBe(400);
66+
});
67+
68+
// ── Success ──────────────────────────────────────────────────────────────
69+
70+
it('returns 200 with comparison data for valid users', async () => {
71+
const res = await GET(makeRequest('user1=alice&user2=bob'));
72+
expect(res.status).toBe(200);
73+
const data = await res.json();
74+
expect(data.user1).toBeDefined();
75+
expect(data.user2).toBeDefined();
76+
});
77+
78+
// ── Error handling ────────────────────────────────────────────────────────
79+
80+
it('returns 404 when user1 is not found on GitHub', async () => {
81+
vi.mocked(getFullDashboardData).mockRejectedValueOnce(new Error('Not found'));
82+
const res = await GET(makeRequest('user1=ghost123&user2=octocat'));
83+
expect(res.status).toBe(404);
84+
});
85+
86+
it('returns 404 when user2 is not found on GitHub', async () => {
87+
vi.mocked(getFullDashboardData)
88+
.mockResolvedValueOnce({ calendar: { totalContributions: 0, weeks: [] } } as never)
89+
.mockRejectedValueOnce(new Error('Not found'));
90+
const res = await GET(makeRequest('user1=octocat&user2=ghost123'));
91+
expect(res.status).toBe(404);
92+
});
93+
});

app/api/compare/route.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,23 @@
11
import { NextResponse } from 'next/server';
22
import { getFullDashboardData } from '@/lib/github';
3+
import { compareParamsSchema } from '@/lib/validations';
34

45
export const revalidate = 3600;
56

67
export async function GET(request: Request) {
78
const { searchParams } = new URL(request.url);
8-
const user1 = searchParams.get('user1');
9-
const user2 = searchParams.get('user2');
109

11-
if (!user1 || !user2) {
10+
const parseResult = compareParamsSchema.safeParse(Object.fromEntries(searchParams.entries()));
11+
12+
if (!parseResult.success) {
13+
const fieldErrors = parseResult.error.flatten();
1214
return NextResponse.json(
13-
{ error: 'Both user1 and user2 query parameters are required.' },
15+
{ error: 'Invalid parameters', details: fieldErrors },
1416
{ status: 400 }
1517
);
1618
}
1719

18-
if (user1.toLowerCase() === user2.toLowerCase()) {
19-
return NextResponse.json({ error: 'Cannot compare a user with themselves.' }, { status: 400 });
20-
}
20+
const { user1, user2 } = parseResult.data;
2121

2222
try {
2323
const [result1, result2] = await Promise.allSettled([

lib/validations.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,8 +423,29 @@ export const wrappedParamsSchema = z.object({
423423
height: dimensionParam('height', 80, 800),
424424
});
425425

426+
export const compareParamsSchema = z
427+
.object({
428+
user1: z
429+
.string({ error: 'Missing user1 parameter' })
430+
.trim()
431+
.min(1, { message: 'user1 is required' })
432+
.max(39, { message: 'GitHub username cannot exceed 39 characters' })
433+
.regex(GITHUB_USERNAME_REGEX, { message: 'Invalid GitHub username for user1' }),
434+
user2: z
435+
.string({ error: 'Missing user2 parameter' })
436+
.trim()
437+
.min(1, { message: 'user2 is required' })
438+
.max(39, { message: 'GitHub username cannot exceed 39 characters' })
439+
.regex(GITHUB_USERNAME_REGEX, { message: 'Invalid GitHub username for user2' }),
440+
})
441+
.refine((data) => data.user1.toLowerCase() !== data.user2.toLowerCase(), {
442+
message: 'Cannot compare a user with themselves.',
443+
path: ['user2'],
444+
});
445+
426446
export type StreakParams = z.infer<typeof streakParamsSchema>;
427447
export type GithubParams = z.infer<typeof githubParamsSchema>;
428448
export type OgParams = z.infer<typeof ogParamsSchema>;
429449
export type StatsParams = z.infer<typeof statsParamsSchema>;
430450
export type WrappedParams = z.infer<typeof wrappedParamsSchema>;
451+
export type CompareParams = z.infer<typeof compareParamsSchema>;

0 commit comments

Comments
 (0)