From 0b4d4f351d7193f6a6c55b89a753727a9bd9a66f Mon Sep 17 00:00:00 2001 From: Krishna Kumar Date: Thu, 11 Jun 2026 19:57:02 +0530 Subject: [PATCH] fix: bound CI analytics API fan-out --- app/api/ci-analytics/route.test.ts | 53 +++++++++++++++++++ app/api/ci-analytics/route.ts | 17 +++++- services/github/ci-analytics.security.test.ts | 41 ++++++++++++++ services/github/ci-analytics.ts | 14 +++-- 4 files changed, 119 insertions(+), 6 deletions(-) create mode 100644 app/api/ci-analytics/route.test.ts create mode 100644 services/github/ci-analytics.security.test.ts diff --git a/app/api/ci-analytics/route.test.ts b/app/api/ci-analytics/route.test.ts new file mode 100644 index 000000000..76882898c --- /dev/null +++ b/app/api/ci-analytics/route.test.ts @@ -0,0 +1,53 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { GET } from './route'; +import { RateLimiter } from '@/lib/rate-limit'; +import { fetchCIAnalytics } from '@/services/github/ci-analytics'; + +vi.mock('@/services/github/ci-analytics', () => ({ + fetchCIAnalytics: vi.fn(), +})); + +describe('GET /api/ci-analytics', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.spyOn(RateLimiter.prototype, 'check').mockResolvedValue(true); + }); + + it('rejects requests when the endpoint abuse budget is exhausted', async () => { + vi.spyOn(RateLimiter.prototype, 'check').mockResolvedValueOnce(false); + + const response = await GET(new Request('http://localhost/api/ci-analytics?username=octocat')); + + expect(response.status).toBe(429); + expect(fetchCIAnalytics).not.toHaveBeenCalled(); + }); + + it('uses a fixed endpoint bucket that cannot be rotated with usernames', async () => { + vi.mocked(fetchCIAnalytics).mockResolvedValue({} as never); + const checkSpy = vi.spyOn(RateLimiter.prototype, 'check').mockResolvedValue(true); + + await GET(new Request('http://localhost/api/ci-analytics?username=octocat')); + await GET(new Request('http://localhost/api/ci-analytics?username=torvalds')); + + expect(checkSpy).toHaveBeenNthCalledWith(1, 'ci-analytics'); + expect(checkSpy).toHaveBeenNthCalledWith(2, 'ci-analytics'); + }); + + it('rejects invalid GitHub usernames before the service fan-out', async () => { + const response = await GET( + new Request('http://localhost/api/ci-analytics?username=invalid%20username') + ); + + expect(response.status).toBe(400); + expect(fetchCIAnalytics).not.toHaveBeenCalled(); + }); + + it('fetches analytics for a validated username', async () => { + vi.mocked(fetchCIAnalytics).mockResolvedValue({ totalRuns: 0 } as never); + + const response = await GET(new Request('http://localhost/api/ci-analytics?username=octocat')); + + expect(response.status).toBe(200); + expect(fetchCIAnalytics).toHaveBeenCalledWith('octocat'); + }); +}); diff --git a/app/api/ci-analytics/route.ts b/app/api/ci-analytics/route.ts index 136ab33c3..22d4c6488 100644 --- a/app/api/ci-analytics/route.ts +++ b/app/api/ci-analytics/route.ts @@ -1,14 +1,29 @@ import { NextResponse } from 'next/server'; import { fetchCIAnalytics } from '@/services/github/ci-analytics'; +import { validateGitHubUsername } from '@/lib/validations'; +import { RateLimiter } from '@/lib/rate-limit'; + +const ciAnalyticsLimiter = new RateLimiter(10, 60_000, 1); export async function GET(request: Request) { + if (!(await ciAnalyticsLimiter.check('ci-analytics'))) { + return NextResponse.json( + { error: 'Too many requests. Please try again later.' }, + { status: 429 } + ); + } + const { searchParams } = new URL(request.url); - const username = searchParams.get('username'); + const username = searchParams.get('username')?.trim(); if (!username) { return NextResponse.json({ error: 'Username is required' }, { status: 400 }); } + if (!validateGitHubUsername(username)) { + return NextResponse.json({ error: 'Invalid GitHub username' }, { status: 400 }); + } + try { const data = await fetchCIAnalytics(username); return NextResponse.json(data); diff --git a/services/github/ci-analytics.security.test.ts b/services/github/ci-analytics.security.test.ts new file mode 100644 index 000000000..5b309e215 --- /dev/null +++ b/services/github/ci-analytics.security.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it, vi } from 'vitest'; + +vi.mock('@/lib/github', () => ({ + getGitHubTokens: vi.fn(() => ['test-token']), + fetchWithRetry: vi.fn(async (url: string) => { + if (url.includes('/users/') && url.includes('/repos?')) { + return new Response( + JSON.stringify( + Array.from({ length: 20 }, (_, index) => ({ + name: `repo-${index}`, + owner: { login: 'octocat' }, + fork: false, + })) + ), + { status: 200 } + ); + } + + return new Response( + JSON.stringify(url.includes('/runs') ? { workflow_runs: [] } : { workflows: [] }), + { + status: 200, + } + ); + }), +})); + +import { fetchWithRetry } from '@/lib/github'; +import { fetchCIAnalytics } from './ci-analytics'; + +describe('CI analytics request fan-out budget', () => { + it('caps workflow fetch targets even when a user has many repositories', async () => { + await fetchCIAnalytics(`security-budget-${Date.now()}`); + + const actionCalls = vi + .mocked(fetchWithRetry) + .mock.calls.filter(([url]) => String(url).includes('/actions/')); + + expect(actionCalls).toHaveLength(10); + }); +}); diff --git a/services/github/ci-analytics.ts b/services/github/ci-analytics.ts index 496aeafe6..abdbcbde8 100644 --- a/services/github/ci-analytics.ts +++ b/services/github/ci-analytics.ts @@ -12,6 +12,9 @@ import type { } from '@/types/ci-analytics'; const GITHUB_REST_URL = 'https://api.github.com'; +const MAX_REPO_PAGES = 2; +const MAX_ACTION_PAGES = 2; +const MAX_FETCH_TARGETS = 5; const cache = new DistributedCache(500); @@ -34,7 +37,7 @@ async function fetchAllPages(url: string, perPage = 100): Promise { let page = 1; let hasMore = true; - while (hasMore && page <= 3) { + while (hasMore && page <= MAX_REPO_PAGES) { const paginatedUrl = `${url}${url.includes('?') ? '&' : '?'}per_page=${perPage}&page=${page}`; const res = await fetchWithRetry(paginatedUrl, { headers: getHeaders(), cache: 'no-store' }); if (!res.ok) break; @@ -76,7 +79,7 @@ async function fetchActionsPages(url: string, dataField: string, perPage = 50 const results: T[] = []; let page = 1; - while (page <= 3) { + while (page <= MAX_ACTION_PAGES) { const paginatedUrl = `${url}${url.includes('?') ? '&' : '?'}per_page=${perPage}&page=${page}`; const res = await fetchWithRetry(paginatedUrl, { headers: getHeaders(), cache: 'no-store' }); if (!res.ok) break; @@ -369,9 +372,10 @@ async function fetchCIAnalyticsUncached(username: string): Promise= MAX_FETCH_TARGETS) break; fetchTargets.push({ owner: repo.owner, repo: repo.name, label: `${repo.owner}/${repo.name}` }); - if (repo.fork && repo.parent) { + if (repo.fork && repo.parent && fetchTargets.length < MAX_FETCH_TARGETS) { const parentLabel = `${repo.parent.owner.login}/${repo.parent.name}`; if (!fetchTargets.some((t) => t.label === parentLabel)) { fetchTargets.push({ @@ -422,7 +426,7 @@ async function fetchCIAnalyticsUncached(username: string): Promise r.name).slice(0, 10), + repos: repos.map((r) => r.name).slice(0, MAX_FETCH_TARGETS), branches: Array.from(allBranches).sort(), };