Shred retired secrets and gate spending a member's token

[ppXD]

Summary

• Deleting a git provider connection leaves no secret decryptable and none of its own hooks or tokens live.
  • ProviderInstanceService.DeleteAsync (backend/src/CodeSpace.Core/Services/ProviderInstances/ProviderInstanceService.cs:235) first retires the connection's group and organization hooks (ProviderInstanceService.Retirement.cs:46).
  • Each hook is deleted at GitLab or GitHub with the token that registered it, best effort. This happens while that token still works: once the delete blanks it, nothing could remove a hook that keeps delivering the group's events. Registered rows are dropped, the rest are Cancelled, and every hook row the connection keeps has its secret_enc blanked.
  • Each credential is then retired through ICredentialRevocationService.RetireAsync: token blanked, status Revoked, row soft-deleted, act-as identities removed. oauth_client_secret_enc is nulled.
  • After commit, each OAuth credential's tokens are revoked at the provider (refresh token, then access token), best effort within a 30 s budget. A delete that rolls back therefore never ends a token. The revoke that Disconnect already did moves into the shared IOAuthGrantRevoker.
• A retirement revokes only the tokens it ends, never the person's whole grant.
  • Two teams may connect through the same OAuth App, because the unique index is per team. GitHub keeps one grant per user and app, so DELETE /applications/{client_id}/grant would end that person's token in every other team too.
  • OAuthRevokeInput.WholeGrant (backend/src/CodeSpace.Core/Services/OAuth/IOAuthClient.cs:75) defaults to true.
    • Disconnect passes true (CredentialService.cs:281) and keeps the grant delete it always sent.
    • The connection delete passes false (ProviderInstanceService.Retirement.cs:141).
  • When WholeGrant is false, GitHubOAuthClient.RevokeAsync (GitHubOAuthClient.cs:99) calls DELETE /applications/{client_id}/token with {"access_token": …}. It sends no refresh token, because GitHub has no endpoint that ends a refresh token alone.
  • GitLab revokes per token (RFC 7009), so it sends the same request either way.
  • A refresh in flight when the row is blanked follows the same rule.
    • OAuthTokenRefresher.RevokeIssuedTokensAsync (OAuthTokenRefresher.cs:146) revokes the pair the provider issued for a row that was blanked in the meantime, because nothing stores that pair.
    • It now reads the row's soft delete through ICredentialPayloadReader.IsRetiredAsync (CredentialPayloadReader.cs:17), the same detached read as the payload.
    • A retirement soft-deletes the row, so only the pair is ended. That covers a connection delete, a departure, a relink, or the sweep blanking a row the previous build deleted.
    • A Disconnect leaves the row listed, so it still ends the grant.
    • Before this change, a departure or relink racing a GitHub refresh ended the person's tokens in other teams, and this PR's delete would have done the same.
  • A pod still running the previous build always sends the grant delete for such a pair. Use the rollout order: stop old workers, then replace the API all at once. That way no old pod runs beside this build's delete or sweep.
• Retired secrets are crypto-shredded (backend/src/CodeSpace.Core/Persistence/DbUpFiles/0249_shred_retired_secrets.sql).
  • Variables: a deleted Secret variable holds no ciphertext. chk_variable_value_exclusive is relaxed for deleted rows only.
  • Storage credential revisions: a revision may be blanked exactly once, and only when its credential is Revoked. A superseded revision of an Active credential is kept, because a storage profile pins an exact revision and routed bytes still decrypt through it.
  • Deployment storage defaults: an envelope may be blanked exactly once, and only while no template names it.
    • StorageDefaultService.UpdateAsync (StorageDefaultService.cs:83) blanks the envelope that a rotation or a clear replaced, in the same transaction.
    • The materializer decrypts only the envelope its template names, read under a share lock.
  • Both guards still refuse every other UPDATE and DELETE, and refuse blank inserts.
  • The script backfills what retirement already left behind:
    • Revoked or soft-deleted credential tokens.
    • Deleted connections' client secrets.
    • Deleted connections' hooks, set to Cancelled with the secret blanked. external_id and owner_path are kept so an operator can remove a hook still at the provider by hand. Deliveries to it are refused as retired.
    • Deleted Secret variables.
    • Revisions of revoked storage credentials.
    • Default envelopes no template names.
  • The rollback note can now be run as written. The old CHECKs reject shredded rows with 23514, so the header gives exact NOT VALID statements and says to re-run 0126's and 0173's guard functions verbatim. A test runs those statements on a shredded database.
  • Rolling the application back does not need that note: the previous build runs on this schema. If the note was run, roll forward again with DELETE FROM schemaversions WHERE scriptname = '0249_shred_retired_secrets.sql' first (header, 0249_shred_retired_secrets.sql:64-70). Without that step this build could not retire a secret. A test re-applies the script this way.
• An hourly sweep shreds secrets the previous build retires after 0249.
  • 0249 runs once, when the first new pod starts. The previous build keeps retiring secrets without blanking them wherever it still runs: next to new pods during a rolling deploy, or on its own after a rollback.
  • RetiredSecretShredRecurringJob (0 * * * *) sends ShredRetiredSecretsCommand, a non-transactional system sweep. Its handler calls IRetiredSecretShredService.ShredAsync (backend/src/CodeSpace.Core/Services/Credentials/RetiredSecretShredService.cs:35).
  • The service runs one conditional UPDATE per kind, with exactly 0249's backfill predicates. It is idempotent, and a blanked row is never selected again.
  • Like 0249, the sweep does not delete a deleted connection's hooks at the provider or revoke its tokens. Only this build's delete does that.
• Listing what a credential can reach now requires its owner or repos.manage. ListAccessibleRepositoriesQuery implements IRequireCredentialSpend.
  • A Personal credential lists only for its owner or a repos.manage holder.
  • A team-service credential lists only for a repos.manage holder.
  • Anyone else gets 403 before the token reaches the provider.
  • The rule lives in CredentialOwnerOrPermission, shared with IRequireCredentialManagement. Instance Admins keep their Owner standing.

No API shape changes. Behaviour changes, all required by the security fix:

• A Viewer or Member asking for another member's personal credential's accessible repositories now gets 403.
• Deleting a provider instance now calls the provider's hook-delete endpoint and its token-revoke endpoint: GitHub DELETE /applications/{client_id}/token, GitLab POST /oauth/revoke. Neither the delete nor a refresh it overtakes calls GitHub's grant delete.
• Rotating or clearing a deployment storage default's secret makes the replaced secret unrecoverable.
• One new recurring job, RetiredSecretShredRecurringJob.
  • A scheduler running the previous build cannot load it, like any recurring job that build does not have. It retries 5 times, then disables the job.
  • The next start of a new-build worker registers it again. Roll workers with Recreate, or restart them once after the rollout.

Test plan

• Unit:
  • OAuthClientRevokeTests: a recording HttpMessageHandler asserts what leaves each client.
    • URL, method, body and Basic auth for each WholeGrant and hint case.
    • No request is sent for a GitHub refresh token outside a whole-grant revoke.
    • 204, 404 and 422 count as done; a 500 is surfaced.
    • GitLab sends the identical RFC 7009 request either way.
    • The default is true.
  • OAuthTokenRefresherTests: a theory checks the pair a refresh got for a row blanked meanwhile. When a Disconnect blanked it, the pair is revoked whole-grant. When a retirement blanked and soft-deleted it, the pair is revoked token-level.
  • OAuthGrantRevokerTests: the scope is passed through on every token.
  • RetiredSecretShredDispatchTests: job, command and handler (Rules 14/16).
  • Authorization inventory entry for the sweep.
  • Model payload checks match 0249; spend-gate wiring; variable retirement.
  • Full unit suite: 13722 passed, 1 skipped.
• Integration:
  • SharedOAuthAppRevocationFlowTests runs the real GitHubOAuthClient against a loopback GitHub that keeps grants per user and app. Two teams share a client id, and one person is connected in both.
    • Deleting team A's connection ends only team A's token: no grant delete is sent and team B's token stays live.
    • A Disconnect still ends the whole grant.
    • Race theory, through the real auth resolver, GitHub OAuth strategy and refresher. A retirement lands while GitHub holds team A's refresh, in two variants:
      • this build's connection delete;
      • the previous build's delete, replayed as SQL, followed by the sweep through the real mediator (mixed version).
    • In both variants the pair GitHub issued is ended with /token, no /grant is sent, and team B's token stays live.
    • Sibling test: a Disconnect landing during the refresh still revokes that pair with /grant.
  • CredentialPayloadReaderFlowTests: through the real detached reader, a row that is Active or Disconnected is not retired; a row that is soft-deleted or gone is.
  • Provider-instance delete:
    • retires credentials and the client secret;
    • revokes token-level (WholeGrant = false) after commit;
    • takes group hooks down with a live token, whether the provider accepts or refuses.
  • RetiredSecretShredSweepFlowTests (mixed version, simulated):
    • The previous build's own retirement statements, replayed as raw SQL against this schema, are accepted by it: its provider delete, variable delete, storage revoke and default rotation.
    • The sweep, sent through the real mediator, then blanks every one and touches no live row.
    • A second tick changes nothing.
  • Storage default envelope guard: one blanking, only while unnamed, no refill, no blank insert.
  • Rotate then clear a template's secret, and materialize a team after the rotation.
  • Variable shred; storage credential revision shred.
  • SweepCommandTransactionFlowTests sends the sweep through the real pipeline.
• Migration on a populated DB (RetiredSecretShredMigrationTests), through the production DbUpRunner on its own database:
  • A pre-0249 database is upgraded; the backfills and the guards are checked after the upgrade.
  • The documented rollback runs on a shredded database.
  • Rolling forward after that rollback re-applies 0249 and lets retirement blank again.
  • The same seed written after 0249 is shredded by the sweep under the upgrade's own assertions. Mutating a predicate fails this test.
• E2E:
  • HTTP, GitLab stub on a loopback port:
    • Delete revokes OAuth tokens at GitLab and leaves no token or client secret.
    • Delete with the provider down still retires everything.
    • Connection-wide delete sends the group-hook DELETE with the registering PAT.
  • HTTP, GitHub stub, two teams sharing an OAuth App:
    • Deleting one team's connection sends only DELETE /api/v3/applications/{client_id}/token with its own access token.
    • A later Disconnect in the other team sends the grant delete.
    • A delete that lands while a listing refreshes the token sends exactly two /token revokes: the team's token, then the pair GitHub just issued. No /grant is sent.
  • A Viewer or Member against another member's PAT gets 403; the owner and a repos.manage holder get 200.
  • Deleted-variable shred; storage default rotate and clear over /api/admin/storage-defaults.
  • RecurringJobWorkerSmokeE2ETests: the real worker host registers the sweep and it completes a tick.
• Mutations:
  • Forcing the refresher to token-level fails the Disconnect sibling.
  • Forcing it to whole-grant fails the race at the unit, integration and E2E tiers.
  • Inverting the reader query fails the gone-row case.
• Regression:
  • Integration namespaces: OAuth, ProviderInstances, Credentials, Identity, Providers, Binding, Auth, Repositories, Webhooks, Persistence, Variables, Hangfire, Authorization, Mediation, Users, Projects.
  • E2E namespaces: Repositories, Credentials, Auth.
• Staging:
  • After deploy, check provider-instance deletes for "Failed to delete remote connection webhook" warnings. Each one is a group hook to remove by hand at the provider. The same applies to every hook of a connection the previous build deleted during the rollout.
  • Check that RetiredSecretShredRecurringJob has a non-empty NextExecution once every worker runs this build.