Skip to content

Bug: GitHub App Webhook silently drops entire batch of added/removed repositories if a single repo fails validation #6359

@AftAb-25

Description

@AftAb-25

Describe the bug

There is a massive data starvation and potential ghost-access vulnerability inside the GitHub App Webhook processor (internal/providers/github/webhook/app.go).

When a user modifies their GitHub App installation (e.g., granting or revoking access to bulk repositories), GitHub sends a installation_repositories webhook payload containing arrays of repositories_added and repositories_removed.

In processInstallationRepositoriesAppEvent(), the code iterates over addedRepos. If even a single repository fails validation in repositoryAdded() (for example, missing a valid name format), the function inexplicably returns an error and aborts the entire batch:

	results := make([]*processingResult, 0)
	for _, repo := range addedRepos {
		// caveat: we're accessing the database once for every
		// repository, which might be inefficient at scale.
		res, err := repositoryAdded(ctx, repo, installation)
		if err != nil {
			return nil, err // <--- ABORTS ENTIRE BATCH FOR ALL REPOS
		}

		results = append(results, res)
	}

	// Because of the abort above, removed repos in the same payload are NEVER processed
	for _, repo := range event.GetRepositoriesRemoved() {
		res := repositoryRemoved(repo)
		results = append(results, res)
	}

The Impact:

  1. GitHub Returns 500 & Infinite Retries: HandleGitHubAppWebhook receives the error and returns HTTP 500 to GitHub. GitHub will continually redeliver this same bad batch payload, repeatedly failing on the same bad repository.
  2. Valid Repositories Ignored: The rest of the perfectly valid repositories in the repositories_added array are permanently dropped and never onboarded into Minder.
  3. Data Leak / Ghost Access: Because the function aborts before iterating over GetRepositoriesRemoved(), any repositories the user explicitly revoked access to are completely ignored by Minder. Minder incorrectly retains database access tracking for these deleted repositories indefinitely.

Expected behavior

A failure to parse or handle a single repository inside a batch hook should not abort the entire payload. It should safely ignore/log the malfunctioning repository map and continue processing the rest of the valid added and removed arrays to ensure systems stay in sync.

Proposed Fix

In internal/providers/github/webhook/app.go, change return nil, err to continue and explicitly log the validation warning:

		res, err := repositoryAdded(ctx, repo, installation)
		if err != nil {
			zerolog.Ctx(ctx).Warn().Err(err).Msg("skipping invalid repository in installation payload")
			continue
		}

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions