Commit efd3915bb0
efd3915bb0c78a786f37bf9ea46c1655dd23885e
parent: 882f09f5a0
Verified · cmc ci/build: success ci/test: success ci/vuln: success
cmc <hello@cleberg.net> · 2026-09-03 21:42 UTC
store: triggers refuse deleting an owner that still owns repositories
repos.owner_id is polymorphic over users and orgs, so no foreign key
can hold it, and integrity rested on the checks in DeleteUser and
DeleteOrg. Migration 0033 adds BEFORE DELETE triggers on users and orgs
that abort while repositories remain, so a direct or buggy delete
cannot orphan a repository either.
Closes #136
Layout: unified · split
internal/store/migrations/0033_owner_guards.down.sql
added
+2
| @@ -0,0 +1,2 @@ |
| 1 | DROP TRIGGER users_owning_repos; |
| 2 | DROP TRIGGER orgs_owning_repos; |
internal/store/migrations/0033_owner_guards.up.sql
added
+14
| @@ -0,0 +1,14 @@ |
| 1 | -- repos.owner_id is polymorphic over users and orgs, so no foreign key |
| 2 | -- can hold it; DeleteUser and DeleteOrg refuse while repositories remain. |
| 3 | -- These triggers make that refusal structural: a direct or buggy delete |
| 4 | -- cannot orphan a repository either. |
| 5 | CREATE TRIGGER users_owning_repos BEFORE DELETE ON users |
| 6 | WHEN EXISTS (SELECT 1 FROM repos WHERE owner_kind = 'user' AND owner_id = OLD.id) |
| 7 | BEGIN |
| 8 | SELECT RAISE(ABORT, 'user still owns repositories'); |
| 9 | END; |
| 10 | CREATE TRIGGER orgs_owning_repos BEFORE DELETE ON orgs |
| 11 | WHEN EXISTS (SELECT 1 FROM repos WHERE owner_kind = 'org' AND owner_id = OLD.id) |
| 12 | BEGIN |
| 13 | SELECT RAISE(ABORT, 'organization still owns repositories'); |
| 14 | END; |
internal/store/ownerguard_test.go
added
+50
| @@ -0,0 +1,50 @@ |
| 1 | package store |
| 2 | |
| 3 | import ( |
| 4 | "strings" |
| 5 | "testing" |
| 6 | ) |
| 7 | |
| 8 | // repos.owner_id is polymorphic, so no foreign key can hold it. The |
| 9 | // triggers from migration 0033 refuse deleting an owner that still owns |
| 10 | // repositories even when the application guards are bypassed (#136). |
| 11 | func TestOwnerDeleteRefusedWhileReposRemain(t *testing.T) { |
| 12 | s := open(t) |
| 13 | if err := s.MigrateUp(); err != nil { |
| 14 | t.Fatal(err) |
| 15 | } |
| 16 | uid, err := s.CreateUser("alice", false) |
| 17 | if err != nil { |
| 18 | t.Fatal(err) |
| 19 | } |
| 20 | repoID, err := s.CreateRepo("user", uid, "app", "public") |
| 21 | if err != nil { |
| 22 | t.Fatal(err) |
| 23 | } |
| 24 | if _, err := s.DB.Exec("DELETE FROM users WHERE id = ?", uid); err == nil || !strings.Contains(err.Error(), "owns repositories") { |
| 25 | t.Fatalf("direct user delete with repositories: err=%v", err) |
| 26 | } |
| 27 | oid, err := s.CreateOrg("theorg", uid) |
| 28 | if err != nil { |
| 29 | t.Fatal(err) |
| 30 | } |
| 31 | orgRepo, err := s.CreateRepo("org", oid, "site", "public") |
| 32 | if err != nil { |
| 33 | t.Fatal(err) |
| 34 | } |
| 35 | if _, err := s.DB.Exec("DELETE FROM orgs WHERE id = ?", oid); err == nil || !strings.Contains(err.Error(), "owns repositories") { |
| 36 | t.Fatalf("direct org delete with repositories: err=%v", err) |
| 37 | } |
| 38 | // Without repositories the deletes go through. |
| 39 | for _, id := range []int64{repoID, orgRepo} { |
| 40 | if err := s.DeleteRepo(id); err != nil { |
| 41 | t.Fatal(err) |
| 42 | } |
| 43 | } |
| 44 | if _, err := s.DB.Exec("DELETE FROM orgs WHERE id = ?", oid); err != nil { |
| 45 | t.Fatalf("org delete without repositories: %v", err) |
| 46 | } |
| 47 | if _, err := s.DB.Exec("DELETE FROM users WHERE id = ?", uid); err != nil { |
| 48 | t.Fatalf("user delete without repositories: %v", err) |
| 49 | } |
| 50 | } |