fix(control-panel): revoke owner-match scope for suspended installations (#953) - #966
Conversation
…ons (JSONbored#953) buildControlPanelAccessScope/buildControlPanelRoleSummary granted control-panel scope to a user's own installed repos via the owner-match branch (sameLogin(repo.owner, args.login)) without checking suspension — while the installation-id branch was already suspend-aware. Because the installation 'suspend' webhook does not flip repositories.isInstalled to false, a user who suspended the App kept control-panel access to their own repo's private data: exactly the access the suspend was meant to cut. Exclude any repo under a suspended account installation (suspendedAt set, same account login) from ownedInstalledRepos in both functions, so owner-match no longer overrides suspension. Adds coverage for the access-scope and role-summary paths. Closes JSONbored#953.
|
🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ Maintainer review — Ready to mergeReviewed 2 changed file(s) — two independent AI reviewers, synthesized. ✅ Safe to merge — both reviewers found no blocking issues. Summary The changes correctly prevent repos under suspended installations from granting control‑panel scope via owner‑match logic, fixing a security/permission regression. Added tests verify the new behavior. No new security concerns, build passes, and existing functionality is preserved. 💡 Nits — non-blocking, optional
|
|
Note Gittensory Gate skippedPR closed before full evaluation. No late first comment was created.
💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers. |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #966 +/- ##
=======================================
Coverage 96.64% 96.64%
=======================================
Files 110 110
Lines 15033 15039 +6
Branches 5432 5434 +2
=======================================
+ Hits 14528 14534 +6
Misses 104 104
Partials 401 401 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ghost
left a comment
There was a problem hiding this comment.
Both reviewers approved, all CI is green, and this is a small, low-risk change — auto-merging.
Summary
Fixes #953.
buildControlPanelAccessScope/buildControlPanelRoleSummarygranted control-panel scope to a user's own installed repos through the owner-match branch (sameLogin(repo.owner, args.login)) without checking suspension — even though the installation-id branch is already suspend-aware (accountInstallationIdsexcludessuspendedAt).Because the installation
suspendwebhook does not fliprepositories.isInstalledtofalse, a user who suspended the App still retained control-panel access to their own repo's private gittensory data — the exact access the suspend was meant to revoke.Fix
Compute
suspendedAccountInstallationIds(installations withsuspendedAtset for the same account login) and exclude any repo under such an installation fromownedInstalledRepos, in both functions. The owner-match branch can no longer override suspension; non-suspended owners are unaffected.Tests
buildControlPanelAccessScope: a suspended owner's repo yields an empty scope (no repos/installations/logins).buildControlPanelRoleSummary: a suspended owner gets noowner/maintainerrole and zero owned/account evidence.npx vitest run test/unit/control-panel-roles.test.ts→ 9 passed ·npx tsc --noEmit→ clean ·git diff --check→ clean.Closes #953.