Skip to content

CRITICAL: maintainer-discover sweep runs unbounded O(users x installs x repos) GitHub API calls per cron tick, enabling cross-installation rate limit exhaustion #784

Description

@namann5

Summary

The daily cron sweep (maintainer-discover.ts) fires force: true events for up to 100 users. Each discoverForUser() loads ALL global installations (not scoped to the user), then for each org installation where the user isn't admin, iterates every repository calling repos.getCollaboratorPermissionLevel(). This produces ~100k+ GitHub API calls per tick at moderate scale, consuming rate limits for organizations unrelated to the user being processed. The rate budget check fails open, allowing cold-cache stampedes.

Detailed Description

The sweep trigger

The sweep() function (lines 233-264) runs on cron 0 2 * * * (line 35). It:

  1. Loads up to 500 github_installation_users rows (line 239)
  2. Deduplicates to unique user IDs (lines 240-247)
  3. Fires a maintainer/discover Inngest event with force: true (line 259) for up to 100 users (line 250)

The per-user cost explosion

Each maintainer/discover event runs discoverForUser() (lines 49-231). The critical flaw:

// Lines 65-68: loads ALL installations globally — NOT scoped to user
const { data: installs } = await sb
  .from('github_installations')
  .select('id, account_login, account_type')
  .is('uninstalled_at', null);

For each installation, the function:

  • Calls octokit.orgs.getMembershipForUser() (1 API call per org install, lines 94-97)
  • If not admin, falls to Path 3 (lines 128-157): loads ALL repos for that install, then calls octokit.repos.getCollaboratorPermissionLevel() for every single repo

Concrete cost calculation

Per-user cost = O(all_installations) x O(repos_per_install)

Total per tick = min(100, unique_users) x active_installations x avg(repos_per_install)
               = 100 x 50 x 20 = 100,000 GitHub API calls

Each API call uses the target installation's installation token, consuming that org's rate limit.

Fail-open rate budget

In src/lib/github/rate-budget.ts:

  • Lines 52-54: When no budget is cached, returns { ok: true } — allows first sweep of fresh deployment to run with zero throttling
  • Lines 62-64: When cache is unreachable, also fails open

Force flag bypasses dedup

Line 259 uses force: true, bypassing the 1-hour dedup cache check (lines 58-63), ensuring every sweep tick runs full discovery for all 100 users.

Impact

  1. Cross-organization rate limit exhaustion: The sweep consumes GitHub API rate budgets for installations that have no relationship to the users being processed
  2. System-wide denial of service: When installation rate limits are exhausted, ALL operations using installation tokens fail — webhook processing, PR syncing, audit runs, and backfills
  3. Cascading Inngest delays: The function sleeps when budgets are low (lines 78-81), but accumulated sleep durations can block the Inngest concurrency slot (concurrency: { limit: 1 }, line 34), delaying all other maintainer/discover triggers
  4. Inefficient concurrency: Only one sweep runs at a time globally. A long-running sweep blocks sign-in bootstraps and installation.created webhook handlers

Affected Files

File Lines Description
src/inngest/functions/maintainer-discover.ts 233-264 sweep() loads 500 junction rows, fires force: true for 100 users
src/inngest/functions/maintainer-discover.ts 65-68 discoverForUser() queries ALL installations globally
src/inngest/functions/maintainer-discover.ts 73-82 Per-installation rate budget check (sleeps, never skips)
src/inngest/functions/maintainer-discover.ts 92-113 Org membership API call per installation
src/inngest/functions/maintainer-discover.ts 128-157 Per-repo collaborator permission check across all repos
src/inngest/functions/maintainer-discover.ts 259 force: true bypasses dedup cache
src/lib/github/rate-budget.ts 52-54 Fail-open when no budget cached
src/lib/github/rate-budget.ts 62-64 Fail-open when cache unreachable

Suggested Fix

A. Scope installations to the user

Only check installations the user already has a junction row for:

// CURRENT (lines 65-68): loads ALL installations globally
const { data: installs } = await sb
  .from('github_installations')
  .select('id, account_login, account_type')
  .is('uninstalled_at', null);

// FIXED: only load installs the user is already associated with
const { data: userInstalls } = await sb
  .from('github_installation_users')
  .select('installation_id, github_installations!inner(id, account_login, account_type, uninstalled_at)')
  .eq('user_id', userId);
const installs = (userInstalls ?? [])
  .map(r => r.github_installations)
  .filter(i => i && i.uninstalled_at === null);

B. Remove force: true from sweep events

Let the 1-hour dedup cache be respected. Process fewer users per tick (10-20 instead of 100).

C. Enforce a global API call budget per sweep

Add a step counter and cap total GitHub API calls per execution (e.g., max 5,000), then early-terminate.

D. Fail closed on unknown budget

For sweep operations, when checkRateBudget returns { ok: true, remaining: null }, treat it as { ok: false } to avoid cold-cache stampedes.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions