[Bug]: suspending the App does not revoke control-panel access to your own repos

[galuis116]

[Bug]: suspending the GitHub App does not revoke control-panel access to your own repos (owner-match scope ignores suspendedAt)

Summary

buildControlPanelAccessScope deliberately drops suspended installations
when computing a session's repo access — except one branch. The
ownedInstalledRepos filter grants scope via sameLogin(repo.owner, args.login)
without checking suspension, and nothing flips repositories.isInstalled to
false on the installation suspend webhook. So after a user suspends the App
(GitHub revokes the App's access), that user still retains control-panel access to
their own installed repo's private gittensory data — the access the suspend was
meant to cut.

Evidence

// src/services/control-panel-roles.ts — buildControlPanelAccessScope (and the identical block in
// buildControlPanelRoleSummary at lines 102-104)
const installedRepos = args.repositories.filter((repo) => repo.isInstalled);
const accountInstallations = args.installations.filter(
  (installation) => !installation.suspendedAt && sameLogin(installation.accountLogin, args.login),  // line 75: suspend-aware
);
const accountInstallationIds = new Set(accountInstallations.map((installation) => installation.id));
const ownedInstalledRepos = installedRepos.filter((repo) =>
  sameLogin(repo.owner, args.login)                                   // ← line 77: NOT suspend-filtered
  || (repo.installationId != null && accountInstallationIds.has(repo.installationId)));  // ← suspend-filtered (correct)
const scopedRepoNames = uniqueRepoNames([...ownedInstalledRepos.map((repo) => repo.fullName), ...maintainerRepos]);

The intent is explicit: line 75 filters !installation.suspendedAt, and the
second disjunct of ownedInstalledRepos honors it via accountInstallationIds.
The first disjunct (sameLogin(repo.owner, args.login)) is the oversight — it
admits any isInstalled repo the login owns, regardless of suspension.

Nothing un-installs the repos on suspend:

// src/queue/processors.ts — only `deleted` is special-cased; `suspend` falls through to upsertInstallation
if (eventName === "installation" && payload.action === "deleted" && payload.installation?.id) {
  await markInstallationDeleted(env, payload.installation.id);
  ...
  return;
}
await upsertInstallation(env, payload);   // records suspendedAt, but never touches repositories.isInstalled

scopedRepoNames becomes scope.repositoryFullNames, which is the set that
requireSessionRepoAccess (HTTP), canLoginAccessRepo / canWatchRepo (MCP +
issue-watch fan-out) consult to authorize private-repo data. So the stale
owner-scope grants the suspended repo's data on all of those surfaces.

Reachability

1. Alice installs the App on her private repo alice/secret (repo row:
   isInstalled: true, owner: "alice", installationId: X).
2. Alice (or her org admin) suspends the App. GitHub sends installation
   action suspend → processGitHubWebhook runs only upsertInstallation, which
   sets installations.suspendedAt but leaves repositories.isInstalled = true.
3. Alice opens the control panel and hits a private-repo route
   (/v1/repos/alice/secret/settings, /issue-quality, /skipped-pr-audit,
   etc.). buildControlPanelAccessScope(login="alice"):
   • accountInstallations excludes the suspended X → empty.
   • ownedInstalledRepos still matches sameLogin("alice", "alice") → alice/secret
     is in repositoryFullNames.
   • requireSessionRepoAccess admits her → the suspended repo's private data is
     served.

Why it's wrong

Suspending the App is GitHub's "revoke access without uninstalling" control — the
gittensory access scope is supposed to honor it (line 75 proves the intent). The
owner-match branch breaks that contract: a suspended repo's owner keeps reading
their repo's cached gittensory data (settings, issue-quality, audit/skipped-PR
feed, and issue-watch notifications) after access was revoked.

Scope / honest severity

The leak is bounded to the repo owner's own data (the branch only matches
repo.owner === args.login, i.e. personally-owned repos; org-owned repos go
through the suspend-safe accountLogins path, and live-verified write paths like
the BYOK key routes re-check GitHub permission and fail closed under suspension).
So this is a revocation-completeness / access-boundary correctness bug, not a
cross-tenant leak. It still matters: suspension is a security control, and the
product treats it as one everywhere else.

Suggested fix

Gate the owner-match on the user actually having a non-suspended installation, in
both buildControlPanelAccessScope and buildControlPanelRoleSummary:

const activeOwnerLogins = new Set(accountInstallations.map((i) => i.accountLogin.toLowerCase()));
const ownedInstalledRepos = installedRepos.filter((repo) =>
  (sameLogin(repo.owner, args.login) && activeOwnerLogins.has(repo.owner.toLowerCase()))
  || (repo.installationId != null && accountInstallationIds.has(repo.installationId)));

Since accountInstallations is already the suspend-filtered, login-matched set,
this preserves the owner fallback (for missing/odd installationId linkage) while
respecting suspension. (A more complete fix would also flip the installation's
repos to isInstalled: false on the suspend action and restore on unsuspend,
mirroring markInstallationDeleted — but the scope fix alone closes the access
leak.)

Test status

Not covered. control-panel-roles.test.ts, access-boundary.test.ts, and
issue-watch.test.ts contain zero suspend cases — the installation() test
factory never sets suspendedAt, so the owner-disjunct's suspend-blindness is
never exercised. A regression test should build the scope with a suspended
installation + an owner-owned isInstalled repo and assert the repo is NOT in
repositoryFullNames.

Confidence note

High on the mechanism (clear contract violation — the suspend filter sits two
lines above the leaking branch — reachable from a real webhook + real serving
route, with a clean fix and a complete test gap). Honest on severity: bounded
to the owner's own repo data, so a revocation-completeness defect rather than a
cross-tenant exposure.

Distinct from prior reports

Unrelated to the manual-retrigger refresh, the score-breakdown sanitizer gap,
predicted-gate, gate-403, or the issue-watch visibility gate (#742). This is the
installation suspend lifecycle, which has no special-casing at all today.