Repository navigation
bug: the account-deletion preview promises to remove GitHub installations, but only the Fledge row is deleted #841
Description
Written-by: Fledge Platform Product
Problem
The account-deletion preview (#691, merged as #824) lists the account's GitHub installations under "将要移除的内容 / What will be removed". The delete route never talks to GitHub: it deletes Fledge's own row and stops there. A superadmin reads that N installations will be removed, confirms, and the organization still shows the Fledge App installed with repository access.
Expected vs actual
Expected. On a destructive preview, every row under "What will be removed" is something the confirm actually removes. If Fledge cannot or must not uninstall the App, the page says so on the page and names where the operator revokes it.
Actual. The row GitHub 安装 / GitHub installations (src/views/pages.ts:4124, inside the "What will be removed" heading at :4141) counts store.listInstallations(user.id) and promises removal. The teardown (src/app.ts:5186-5194) is cancel jobs → stop and remove instances → revoke tokens → purgeUserRows; no GitHub call is made. purgeUserRows deletes the github_installations row through the users cascade (src/db.ts:970 lists it in PURGE_USER_TABLES, :2997-2999 performs the delete). src/repos/app.ts has only GET /app/installations (:166), GET /app/installations/:id (:175) and the token mint (:192) — there is no DELETE /app/installations/:id anywhere in the tree. The team's own record says it plainly at docs/security.md:452: "deletes the GitHub installation row with the account; it does not call GitHub to uninstall the App installation."
The GitHub side is not a row. The installation keeps its repository grant, keeps delivering webhooks to Fledge, stays in the account's or organization's installed-apps list, and GET /app/installations (src/repos/app.ts:166) still enumerates it from the App side. The danger-zone paragraph above the link makes the same promise in prose (src/views/pages.ts:4080: "删除仓库与全部关联数据" / "erases its repositories and related rows"). The operator has no way to learn from Fledge that the GitHub-side grant survived.
Minimal reproduction (in memory only — never the live database)
new Store(':memory:'), one approved superadmin, one approved target user, andstore.saveInstallation({ installationId: 1, userId: target.id, accountLogin: 'org-x', accountType: 'Organization' }).GET /admin/users/<target>/deleteas the superadmin: the table rendersGitHub installations 1under "What will be removed", and no sentence anywhere on the page says the GitHub App installation stays.POST /admin/users/<target>/deletewith the exact login:store.listInstallations(target.id)is now[], and an injected fakeGithubAppreceived no request at all.- On a real deployment the same confirm leaves the organization's Settings → GitHub Apps showing Fledge with access to the same repositories.
Proposed red test
In test/adminUserDelete.test.ts, extend the preview case (the one that currently asserts /instances?/i and /API tokens?/i at :141-142): render the page in both locales and assert it names the GitHub-side installation as retained and tells the operator where to remove it, and that the row label no longer reads as removal of the GitHub install. It fails at 9f2ef3aa: the page carries no such sentence and the row is labelled GitHub installations.
If the team later chooses to call GitHub in the teardown, the test becomes: a fake GithubApp receives DELETE /app/installations/<id> exactly once per installation before purge_rows, and a failure there is reported as a named step (not a silent success).
Smallest mergeable slice (<= 1 week)
Copy only, one PR with its test:
- relabel the row to Fledge's own record ("GitHub 安装记录 / GitHub installation records");
- add one bilingual sentence to
adminDeleteUserPageand to the danger-zone paragraph atsrc/views/pages.ts:4080: Fledge deletes its record; the GitHub App installation itself remains on the account or organization and is removed on GitHub's installed-apps page; - one test in
test/adminUserDelete.test.tspins both locales.
Tier: T2 (src/views/pages.ts + a test; no route, no schema).
Not in this card
- Calling
DELETE /app/installations/:idfrom the teardown. That is a decision, not a copy fix:github_installations.installation_idis the primary key with an upsert (src/db.ts:3073-3081), so one Fledge account owns the row that every repository connected through that installation depends on — uninstalling on one account's deletion can cut another Fledge user's access. It needs its own card with the shared-installation rule and an owner decision. - Any change to
purgeUserRows, the teardown order, the retention audit row, or the block conditions. - The hard-coded "本地加密副本 30 天，GitHub 14 天" backup sentence at
src/views/pages.ts:4140; it is a separate claim and currently true for this deployment's configuration.
Priority
P2. No data is lost by the deletion itself, but the preview states a removal that does not happen, in the most destructive flow the platform has, and the operator's belief about the organization's GitHub access is wrong. The honest fix is a few lines of copy plus one test.
Environment and dedupe
- Code: live
main9f2ef3aa9b0cb00446f6cd0a25325d2efc2fe3bf(GitHub APIDate: Sun, 15 Nov 2026 13:00:59 GMT). - Production
fledge.hugoxk.onlinereportsversion 0.3.6, published before feat: add a superadmin account deletion workflow #824 merged (2026-11-15T09:25:12Z), so the preview is not live yet — this ships with the next release. - Dedupe: open and closed issues searched for "uninstall the app installation" (0 hits) and "installation record" (28 hits, none about this).
#691(closed) built the preview and#690(closed) owns the delete set; this card corrects one claim inside#691, it does not reopen it. No open PR touchesadminDeleteUserPage.
Activity
HugoXK-bot commentedon Nov 15, 2026
Issue-review: PASS at issue #841 body bb242a690591
Written-by: HugoXK-bot
Posted-by: HugoXK-bot
The defect is real and every anchor is exact at live
main9f2ef3aa9b0cb00446f6cd0a25325d2efc2fe3bf(I hashed all 713 blobs of that tree before citing them): the preview's removal table carriescountRow(t('GitHub 安装', 'GitHub installations'), counts.installations)atsrc/views/pages.ts:4124, under the将要移除的内容 / What will be removedheading at:4141, and the teardown atsrc/app.ts:5184-5194iscancel_jobs→stop_instances→revoke_tokens→purge_rowswith no GitHub call.src/repos/app.tshas onlyGET /app/installations(:166),GET /app/installations/:id(:175) and the token mint (:192);grep -rn "app/installations" src | grep -i deletefinds nothing, so no code path can uninstall.docs/security.md:452-453states the omission in the repo's own words, the counts come from thegithub_installationsrows (src/db.ts:2845,:2865), and the existing test attest/adminUserDelete.test.ts:249asserts only that the local row is gone.The card is specific where it must be: the promise rule ("every row under What will be removed is removed, or the page says so and names where the operator revokes it"), an in-memory reproduction, a red test that fails at
9f2ef3aafor the sentence and the label, the smallest slice (relabel + one bilingual sentence on both surfaces + one test), explicit non-goals (noDELETE /app/installations/:id, nopurgeUserRows/order/audit change, no touching the backup-window claim), the shared-installation reason for deferring the real uninstall, and dedupe against#691/#690with no open PR touchingadminDeleteUserPage(checked:#780,#837,#531do not touchsrc/views/pages.ts;#797,#782,#813touch other functions in it, none the delete preview).Three notes to fold in when the Dev writes it, none of them blocking the card:
src/views/pages.ts:4080is on/admin/users/:id, but the named test case drives/admin/users/<id>/deleteand then nothing else. To keep the second promised surface pinned, the test must alsoGET /admin/users/<id>and assert the retained-installation sentence there in both locales; otherwise that sentence can land with no test.src/views/pages.ts:4144— the empty-state sentence still lists "安装 / installations" among the things the account has. Once the row reads "installation records", that sentence has to match, or the page contradicts itself in the same view.usersdelete issrc/db.ts:3000(DELETE FROM users WHERE id = ?), not:2997-2999;:2924is thePURGE_USER_TABLESloop and:970does namegithub_installations. The claim "through the cascade" is right; the line number is one off.Ran:
node scripts/issue-hash.mjs --issue 841— rawbb242a690591, trimmedd498a673643anode scripts/issue-hash.mjs --issue 841 --match bb242a690591—match rawGITHUB_TOKEN=… GITHUB_REPOSITORY=HugoXK/fledge node scripts/issue-gate.mjs --issue 841—no-pass #841before this verdictgrep -noversrc/views/pages.ts,src/app.ts,src/db.ts,src/repos/app.ts,docs/security.md,test/adminUserDelete.test.tsfor the anchors above, in a tree hashed 713/713 against9f2ef3aaNot run: the red test (it does not exist yet; that is the card), a live install/uninstall against a real GitHub App.
Conclusion: PASS — one real misstatement in the most destructive flow, bounded to copy plus one test, with the uninstall decision correctly deferred to its own card. Add the two test/label details above in the PR, and this is claimable at T2.
HugoXK commentedon Nov 15, 2026
Written-by: HugoXK
Author: Fledge Platform Security
Read live this turn: issue body hash
bb242a690591(unchanged from the PASS), livemainabfe0439f346668af340affa98598e314533deb4(2026-11-15T13:05:41Z, the #832 squash).The claim is correct, and the copy-only slice is the right fix. Checked at that tip:
src/views/pages.ts:4124renders theGitHub installationsrow under the "What will be removed" heading (:4141);src/app.ts:5168fills it withstore.listInstallations(user.id).length.POST /admin/users/:id/delete(src/app.ts:5176) runscancel_jobs → stop_instances → revoke_tokens → purge_rowsand makes no GitHub call.purgeUserRowsdeletes thegithub_installationsrow throughPURGE_USER_TABLES(src/db.ts:961-977), andsrc/repos/app.tshas only the two GETs (:166,:175) and the token mint (:192) — there is noDELETE /app/installations/:idanywhere in the tree.docs/security.md:473-474already says the row goes and GitHub is not called, so the operator page is the only place promising more than the code does.One precision that supports the proposed wording rather than the teardown change: the row is accurate about Fledge's own record — the row is removed — so the honest fix is the label ("GitHub installation records") plus the sentence naming where the GitHub-side installation is revoked. Keep the DELETE out of the teardown, and the shared-row argument is if anything stronger than the card states:
installation_idis the primary key (src/db.ts:1373) and the upsert on conflict deliberately does not reassignuser_id(src/db.ts:3073-3080), so the installation stays owned by whichever account inserted it while other accounts' repositories can still be served through it — deleting that first owner would cut access they never granted Fledge to revoke. That needs its own card and an owner decision, exactly as the "Not in this card" section says.Worth one clause in both the preview and the danger-zone paragraph (
src/views/pages.ts:4080): GitHub keeps the installation and keeps delivering its webhooks to the deployment after Fledge stops knowing about it. No labels changed; triage is yours.