Skip to content

fix(sdk): delete the credentials a connection minted when it or its integration is removed - #2083

Open
GeiserX wants to merge 2 commits into
UsefulSoftwareCo:mainfrom
GeiserX:fix/connection-remove-credential-gc
Open

GeiserX wants to merge 2 commits into
UsefulSoftwareCo:mainfrom
GeiserX:fix/connection-remove-credential-gc

Conversation

@GeiserX

@GeiserX GeiserX commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Removing a connection dropped its row and left the credential it had minted in the provider. Nothing pointed at that item any more and nothing in the product could show it, so someone who disconnected an account still had that account's tokens held on their behalf with no way left to see them or ask for them to go. Removing an integration is the same leak in bulk. The cascade drops every member's connections under the slug, and every secret those connections minted stayed behind. The refresh token is the one that hurts, long-lived by design and unreachable from any row that survives.

Both removals now delete what the connection minted. The row's credential_write marker says which items those are, the same persisted metadata the rest of the code uses to decide what we own, because a minted id carries a per-attempt uuid and cannot be derived from the row. The integration path reads the doomed connection rows before the cascade destroys them, through the same tenant-reach handle that does the delete and with the same predicate, so it sees exactly the rows that are about to go and never one more. In both paths the deletion runs after the outermost transaction commits. A provider does not roll back with the database, and a restored row pointing at a destroyed secret would be worse than the orphan this removes, and unlike the orphan it could not be repaired.

Some things are deliberately left alone. An item a connection only referenced stays, because we never wrote it. A minted item another live connection still points at also stays, so removing one connection cannot break another. The OAuth app's own client secret belongs to the client and outlives the integration it was registered against. A v1-migrated secret_<hash> item is ours but carries no marker, so we still leave its value behind, and closing that needs a schema change rather than a looser rule here. The alias hold-back reads through the bound handle, so it cannot see an alias held by another member, and the removal will delete that item anyway.

Fifteen tests cover both paths: the pasted and OAuth mints, another member's connection under a removed integration, the referenced and aliased items that have to survive, a provider whose delete fails, and a rolled-back removal.

This is part of the credential-handling series described in #1585, which lists every change and the reasoning behind it.

Removing a connection dropped its rows but left the credential it had minted in
the provider, unreferenced and invisible. Removal now deletes the items the
connection minted, chosen by the row's credential_write marker, after the
removal has committed. Referenced items, items another connection still points
at, and non-writable providers are left alone; failures never fail the removal.
The integrations.remove half of the original fix follows separately.
…n is removed

Integration removal cascades every subject's connections under the slug but
left all the credentials they had minted in the provider. The doomed rows are
now read through the same tenant-reach handle and predicate that deletes them,
and their minted items are deleted after the outermost commit with the same
best-effort rules as connection removal.
@GeiserX GeiserX changed the title fix(sdk): delete the credentials a connection minted when it is removed fix(sdk): delete the credentials a connection minted when it or its integration is removed Sep 20, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant