Skip to content

Drive organization deletion to completion - #39

Open
vitramir wants to merge 3 commits into
mainfrom
organization-deletion
Open

Drive organization deletion to completion#39
vitramir wants to merge 3 commits into
mainfrom
organization-deletion

Conversation

@vitramir

@vitramir vitramir commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

DeleteOrganization performed no permission check at all — any authenticated caller who knew an organization's UUID could delete it — and deleted a single row, leaving every org-scoped record in every other service holding an organization_id that resolved to nothing. This makes deletion the lifecycle the spec describes.

Phase 1, in the request. The organization moves to deleting and its member and owner tuples go. Revoking access is not part of deleting the organization — it is what makes deleting its resources safe; without it a member creates an agent into an organization the cascade has already walked past.

Two details worth review here:

  • The cluster tuple deliberately survives. Cluster admins hold can_manage_organization through admin from cluster, and removing it would hide the teardown from the only people who can watch it.
  • Tuples are read back from Authorization, not derived from the memberships table. An installed app is written a member tuple directly and has no membership row, so deriving from the table would leave the app holding access to an organization being emptied.

Phase 2 is a background worker walking the configured teardown order, one step per claim, retrying a failing step with backoff and never advancing past it. ClaimDeletion uses FOR UPDATE SKIP LOCKED so a second replica works on a different organization rather than blocking.

Within step 6, Images and LLM precede Secrets — Secrets keeps its reference check and refuses to delete a secret an image or a subscription still names, so the reverse order stalls the cascade permanently. There is a test asserting that ordering, because it is the kind of thing a later edit silently breaks.

Phase 3 removes the remaining tuples, then the memberships and the row together. The slug is held until that commits, so a new organization cannot claim it while live exposures still answer on the old one.

UpdateOrganization now checks can_manage_organization rather than owner, which no cluster admin holds on any organization. The refusal to write to a deleting organization is a predicate on the UPDATE rather than a read before it, so a teardown starting mid-request cannot slip through the gap.

ListAccessibleOrganizations and ListMyMemberships filter deleting explicitly. Dropping the tuples is enough for a member, but a cluster admin's can_add_member resolves through admin from cluster for every organization on the platform, so state has to be checked rather than inferred from a missing tuple.

Tested against a real database. The lifecycle is SQL — an idempotent transition, a claim two replicas must not both take, a slug held by the row's continued existence — and a fake store proves none of it. CI gains a postgres service so the test runs rather than skipping silently, which is how DB-backed tests in this codebase have gone red unnoticed before.

Depends on authorization#44 for can_manage_organization, and on the twelve service PRs for the RPCs the cascade calls. Chart bumped to 0.2.0.

Part of organization settings and deletion.

DeleteOrganization performed no permission check at all -- any authenticated
caller who knew an organization's UUID could delete it -- and deleted a single
row, leaving every org-scoped record in every other service holding an
organization_id that resolved to nothing.

Deletion is now a lifecycle. Phase 1, in the request: the organization moves
to deleting and its member and owner tuples go. Revoking access is not part of
deleting the organization, it is what makes deleting its resources safe --
without it a member creates an agent into an organization the cascade has
already walked past. The cluster tuple deliberately stays, because cluster
admins hold can_manage_organization through admin from cluster and removing it
would hide the teardown from the only people who can watch it.

The tuples are read back from Authorization rather than derived from the
memberships table: an installed app is written a member tuple directly and has
no membership row.

Phase 2 is a background worker walking the configured teardown order, one step
per claim, retrying a failing step with backoff and never advancing past it.
Within step 6 Images and LLM precede Secrets, which refuses to delete a secret
an image or a subscription still names.

Phase 3 removes the remaining tuples, then the memberships and the row
together. The slug is held until that commits, so a new organization cannot
claim it while live exposures still answer on the old one.

UpdateOrganization now checks can_manage_organization rather than owner, which
no cluster admin holds on any organization. The refusal to write to a deleting
organization is a predicate on the UPDATE rather than a read before it, so a
teardown starting mid-request cannot slip through.

The deletion lifecycle is SQL, so it is tested against a real database, and CI
gains a postgres service so that test runs rather than skipping.
runners.proto imports agynio/api/runner/v1, which the path list does not name.
Without --include-imports the generated runners package references a package
that was never written.
A DB-backed test that skips is indistinguishable from one that passes in the
summary line, which is how tests here have gone red unnoticed. CI now checks
the database is up before running, and asserts the lifecycle test reports PASS
rather than SKIP.
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