Description
Silent uninstall failure in installation_repositories.removed handler: when GitHub fires a removal event, the case-sensitive repository.update() call can match zero rows, returns no error, and responds HTTP 202 — confirming success for an operation that did nothing. The affected repository retains registered = true and continues ingesting data after the user has explicitly removed the GitHub App, constituting a privacy violation.
Root cause is a split between two insertion paths introduced in commit 1f9e3c3:
| Path |
Value stored |
Admin API registration (validateRepoFullName) |
owner/repo (lowercased) |
Webhook added event (line 47–53) |
Owner/Repo (raw from GitHub) |
PostgreSQL's text type is case-sensitive, so both rows satisfy the unique constraint independently. When the removed event arrives with Owner/Repo, repoRepo.update(repo.full_name, …) performs an exact PK match and only clears that row. The API-registered owner/repo row is never touched.
Even in a single-path lifecycle, any casing variant from GitHub (org rename, repo transfer) causes update() to silently affect 0 rows.
Steps to Reproduce
- Register a repo via the admin API → stored as
owner/repo (lowercased by validateRepoFullName).
- GitHub fires
installation_repositories.added for Owner/Repo → upsert inserts a second row Owner/Repo (no conflict — different casing).
- GitHub fires
installation_repositories.removed for Owner/Repo → repoRepo.update('Owner/Repo', …) clears only that row.
- Row
owner/repo remains with registered = true → miners continue running.
- No error is thrown; no warning is logged; GitHub receives HTTP 202.
Expected Behavior
After a installation_repositories.removed event, the repository row (regardless of stored casing) should have installationId = NULL and registered = false, and all data ingestion for that repository should stop immediately.
Actual Behavior
The case-sensitive repoRepo.update(repo.full_name, …) silently matches 0 rows when the stored casing differs from the event payload. The repository keeps registered = true and data ingestion continues indefinitely with no error or log warning.
Environment
- OS: any (server-side bug)
- Runtime/Node version: per project
.nvmrc
- Database: PostgreSQL (case-sensitive
text PK)
Additional Context
Affected file: packages/das/src/webhook/handlers/installation.handler.ts, lines 59–65
Vulnerable code:
// line 59-65 — repositories_removed handler
for (const repo of removed) {
await this.repoRepo.update(repo.full_name, { // exact, case-sensitive PK match
installationId: null,
registered: false,
});
}
Fix — mirror the installation.deleted pattern (lines 25–31) which already uses a case-insensitive query builder:
for (const repo of removed) {
await this.repoRepo
.createQueryBuilder()
.update()
.set({ installationId: null, registered: false })
.where("LOWER(repo_full_name) = LOWER(:name)", { name: repo.full_name })
.execute();
this.logger.log(`Stopped tracking repo: ${repo.full_name}`);
}
Also consider normalizing repo.full_name to lowercase on the added insertion path (line 48) so the split-world casing scenario cannot arise in the first place.
Description
Silent uninstall failure in
installation_repositories.removedhandler: when GitHub fires a removal event, the case-sensitiverepository.update()call can match zero rows, returns no error, and responds HTTP 202 — confirming success for an operation that did nothing. The affected repository retainsregistered = trueand continues ingesting data after the user has explicitly removed the GitHub App, constituting a privacy violation.Root cause is a split between two insertion paths introduced in commit
1f9e3c3:validateRepoFullName)owner/repo(lowercased)addedevent (line 47–53)Owner/Repo(raw from GitHub)PostgreSQL's
texttype is case-sensitive, so both rows satisfy the unique constraint independently. When theremovedevent arrives withOwner/Repo,repoRepo.update(repo.full_name, …)performs an exact PK match and only clears that row. The API-registeredowner/reporow is never touched.Even in a single-path lifecycle, any casing variant from GitHub (org rename, repo transfer) causes
update()to silently affect 0 rows.Steps to Reproduce
owner/repo(lowercased byvalidateRepoFullName).installation_repositories.addedforOwner/Repo→ upsert inserts a second rowOwner/Repo(no conflict — different casing).installation_repositories.removedforOwner/Repo→repoRepo.update('Owner/Repo', …)clears only that row.owner/reporemains withregistered = true→ miners continue running.Expected Behavior
After a
installation_repositories.removedevent, the repository row (regardless of stored casing) should haveinstallationId = NULLandregistered = false, and all data ingestion for that repository should stop immediately.Actual Behavior
The case-sensitive
repoRepo.update(repo.full_name, …)silently matches 0 rows when the stored casing differs from the event payload. The repository keepsregistered = trueand data ingestion continues indefinitely with no error or log warning.Environment
.nvmrctextPK)Additional Context
Affected file:
packages/das/src/webhook/handlers/installation.handler.ts, lines 59–65Vulnerable code:
Fix — mirror the
installation.deletedpattern (lines 25–31) which already uses a case-insensitive query builder:Also consider normalizing
repo.full_nameto lowercase on theaddedinsertion path (line 48) so the split-world casing scenario cannot arise in the first place.