Overview
POST /github/sync/:owner/:repo has no authentication, no authorization, and no allowlist of which repositories MergeFi is actually tracking — it triggers a full issue sync for whatever owner/repo the caller names:
// src/github/github.controller.ts:1-14
@ApiTags('github')
@Controller('github')
export class GithubController {
constructor(private readonly syncService: GithubSyncService) {}
@Post('sync/:owner/:repo')
sync(@Param('owner') owner: string, @Param('repo') repo: string) {
return this.syncService.syncRepository(owner, repo);
}
}GithubSyncService.syncRepository (github-sync.service.ts:82-115) fetches the repo's metadata and then pages through every issue in it via octokit.paginate.iterator(this.octokit.issues.listForRepo, { owner, repo, state: 'all', per_page: 100 }) — for a large public repository, that's a lot of REST calls. All of them go out under one shared, platform-wide credential:
// src/github/octokit.provider.ts:47-53
export function createGithubOctokit(configService: ConfigService<AppConfig, true>): Octokit {
const token = configService.get('github', { infer: true }).apiToken; // <- GITHUB_API_TOKEN, one token for the whole platform
return new RetryingOctokit({ ...(token ? { auth: token } : {}), /* ... */ });
}GitHub's REST API gives an authenticated token a shared budget — 5,000 requests/hour, per the retry-handler's own comment (octokit.provider.ts:24) — for every call made with that credential, regardless of which repository it's calling about. Since sync/:owner/:repo is open to anyone, with no check that owner/repo is a repository MergeFi's sponsors/maintainers actually use this platform for, an anonymous caller can trigger POST /github/sync/torvalds/linux (or repeatedly hit POST /github/sync/<any-repo-with-tens-of-thousands-of-issues>) and consume a meaningful fraction of that shared 5,000/hour budget on a repository with zero relationship to any real MergeFi bounty. GithubSyncInterruptedError's own doc comment (github-sync.service.ts:29-53) confirms this isn't just slow — a sync that runs out of retry budget mid-page throws an error naming exactly how far it got, meaning a legitimate sync for a repo that actually matters, running concurrently or shortly after an abuse-triggered sync has already burned through the shared rate limit, is the one that fails.
The route sits behind only the app's global ThrottlerGuard (app.module.ts:56, { ttl: 60_000, limit: 120 }) — 120 requests per IP per minute is nowhere close to what's needed to stop this: a single request to /github/sync/torvalds/linux alone, one request, triggers thousands of downstream GitHub API calls internally, so the platform's rate-limit exposure per externally-visible request is enormous and completely decoupled from the generic per-IP throttle guarding it.
This is distinct from the already-closed "GitHub sync has no rate-limit awareness or backoff" issue in this repo — that issue (and the retry/throttling plugins it produced, visible in octokit.provider.ts today) is about GithubSyncService behaving gracefully once a sync is already running and hits GitHub's own rate limit — real, valuable, and already shipped. This issue is about the fact that literally anyone can start an arbitrary, expensive sync against any public GitHub repository in the first place, with no relationship to MergeFi required — an access-control gap the existing backoff work doesn't and can't address, since backoff only kicks in once the abuse has already begun consuming the shared budget.
Requirements
- Require authentication (and likely a maintainer/admin role, not just "any logged-in user") on
POST /github/sync/:owner/:repo, consistent with the companion "no auth at all" issue's broader fix, but called out specifically here because this route's abuse cost (platform-wide GitHub API quota) is categorically different from a typical unauthenticated mutation and deserves its own scoped acceptance criteria.
- Consider constraining sync targets to repositories already known to MergeFi (i.e. already present in the
repositories table, or explicitly allowlisted by an admin action) rather than accepting an arbitrary owner/repo pair from the URL — a resync of a tracked repo is a legitimate operation; a first-time sync of an arbitrary repo probably shouldn't be a bare, unauthenticated POST.
- Add a per-route rate limit tighter than the global 120/minute default, given the disproportionate downstream cost of a single call to this endpoint — the generic
ThrottlerGuard bucket (companion issue: "Global rate limiting is a single flat IP-based bucket") is the wrong tool for a route whose cost-per-request is this asymmetric regardless of how that generic issue gets resolved.
- Add a test asserting an unauthenticated request to this route is rejected once the auth requirement lands, and (if the allowlist approach is adopted) a test asserting a sync attempt against an untracked repository is rejected or requires an explicit admin action to register it first.
Acceptance Criteria
Additional Notes
Precise references: src/github/github.controller.ts:1-14 (the unguarded route), src/github/github-sync.service.ts:82-160 (syncRepository/syncIssues, the expensive downstream work a single call triggers), src/github/octokit.provider.ts:17-24,47-53 (the shared platform-wide token and its documented 5,000/hour budget), src/app.module.ts:56 (the only guard in front of this route today, a generic 120-req/min-per-IP throttle with no awareness of this route's disproportionate cost).
Test/reproduction plan:
await request(app).post('/github/sync/torvalds/linux').expect(401);
// pre-fix: 202/200, and (in a live environment) triggers a real, large, expensive sync
// against a repository with zero relationship to this MergeFi instanceCross-references: distinct from the closed "GitHub sync has no rate-limit awareness or backoff against Octokit's REST API" issue (that issue hardened how a sync behaves once running; this issue is about who can start one and against what) and from the open "Global rate limiting is a single flat IP-based bucket" issue (that issue is about the generic throttle's per-route tuning in general; this issue calls out one specific route whose abuse cost is dramatically higher than the generic bucket accounts for, as a concrete instance worth fixing regardless of how that broader issue is eventually resolved). Also a specific instance of the general "no auth at all" issue's pattern, called out separately here because of its unusually asymmetric external-cost profile.
Overview
POST /github/sync/:owner/:repohas no authentication, no authorization, and no allowlist of which repositories MergeFi is actually tracking — it triggers a full issue sync for whateverowner/repothe caller names:GithubSyncService.syncRepository(github-sync.service.ts:82-115) fetches the repo's metadata and then pages through every issue in it viaoctokit.paginate.iterator(this.octokit.issues.listForRepo, { owner, repo, state: 'all', per_page: 100 })— for a large public repository, that's a lot of REST calls. All of them go out under one shared, platform-wide credential:GitHub's REST API gives an authenticated token a shared budget — 5,000 requests/hour, per the retry-handler's own comment (
octokit.provider.ts:24) — for every call made with that credential, regardless of which repository it's calling about. Sincesync/:owner/:repois open to anyone, with no check thatowner/repois a repository MergeFi's sponsors/maintainers actually use this platform for, an anonymous caller can triggerPOST /github/sync/torvalds/linux(or repeatedly hitPOST /github/sync/<any-repo-with-tens-of-thousands-of-issues>) and consume a meaningful fraction of that shared 5,000/hour budget on a repository with zero relationship to any real MergeFi bounty.GithubSyncInterruptedError's own doc comment (github-sync.service.ts:29-53) confirms this isn't just slow — a sync that runs out of retry budget mid-page throws an error naming exactly how far it got, meaning a legitimate sync for a repo that actually matters, running concurrently or shortly after an abuse-triggered sync has already burned through the shared rate limit, is the one that fails.The route sits behind only the app's global
ThrottlerGuard(app.module.ts:56,{ ttl: 60_000, limit: 120 }) — 120 requests per IP per minute is nowhere close to what's needed to stop this: a single request to/github/sync/torvalds/linuxalone, one request, triggers thousands of downstream GitHub API calls internally, so the platform's rate-limit exposure per externally-visible request is enormous and completely decoupled from the generic per-IP throttle guarding it.This is distinct from the already-closed "GitHub sync has no rate-limit awareness or backoff" issue in this repo — that issue (and the retry/throttling plugins it produced, visible in
octokit.provider.tstoday) is aboutGithubSyncServicebehaving gracefully once a sync is already running and hits GitHub's own rate limit — real, valuable, and already shipped. This issue is about the fact that literally anyone can start an arbitrary, expensive sync against any public GitHub repository in the first place, with no relationship to MergeFi required — an access-control gap the existing backoff work doesn't and can't address, since backoff only kicks in once the abuse has already begun consuming the shared budget.Requirements
POST /github/sync/:owner/:repo, consistent with the companion "no auth at all" issue's broader fix, but called out specifically here because this route's abuse cost (platform-wide GitHub API quota) is categorically different from a typical unauthenticated mutation and deserves its own scoped acceptance criteria.repositoriestable, or explicitly allowlisted by an admin action) rather than accepting an arbitraryowner/repopair from the URL — a resync of a tracked repo is a legitimate operation; a first-time sync of an arbitrary repo probably shouldn't be a bare, unauthenticatedPOST.ThrottlerGuardbucket (companion issue: "Global rate limiting is a single flat IP-based bucket") is the wrong tool for a route whose cost-per-request is this asymmetric regardless of how that generic issue gets resolved.Acceptance Criteria
POST /github/sync/:owner/:reporequires authentication and an appropriate role, not open to anonymous callers.Additional Notes
Precise references:
src/github/github.controller.ts:1-14(the unguarded route),src/github/github-sync.service.ts:82-160(syncRepository/syncIssues, the expensive downstream work a single call triggers),src/github/octokit.provider.ts:17-24,47-53(the shared platform-wide token and its documented 5,000/hour budget),src/app.module.ts:56(the only guard in front of this route today, a generic 120-req/min-per-IP throttle with no awareness of this route's disproportionate cost).Test/reproduction plan:
Cross-references: distinct from the closed "GitHub sync has no rate-limit awareness or backoff against Octokit's REST API" issue (that issue hardened how a sync behaves once running; this issue is about who can start one and against what) and from the open "Global rate limiting is a single flat IP-based bucket" issue (that issue is about the generic throttle's per-route tuning in general; this issue calls out one specific route whose abuse cost is dramatically higher than the generic bucket accounts for, as a concrete instance worth fixing regardless of how that broader issue is eventually resolved). Also a specific instance of the general "no auth at all" issue's pattern, called out separately here because of its unusually asymmetric external-cost profile.