From 769fe59f40f38e4aafc651fa32b5787ed74d6e0a Mon Sep 17 00:00:00 2001 From: Seongmin Lee Date: Tue, 28 Jul 2026 12:42:44 +0900 Subject: [PATCH] spindle/db: acl table migration Signed-off-by: Seongmin Lee --- spindle/db/db.go | 63 +++++++++ spindle/db/migrations_test.go | 259 ++++++++++++++++++++++++++++++++++ 2 files changed, 322 insertions(+) create mode 100644 spindle/db/migrations_test.go diff --git a/spindle/db/db.go b/spindle/db/db.go index 9e80d476..ec0cb1c0 100644 --- a/spindle/db/db.go +++ b/spindle/db/db.go @@ -329,9 +329,72 @@ func runMigrations(_ context.Context, conn *sql.Conn, logger *slog.Logger) error return err } + if err := orm.RunMigration(conn, logger, "switch-to-rbac-v2", func(tx *sql.Tx) error { + // NOTE: we are manually creating casbin table because DB migration will happen before rbac/v2 initialization. + if _, err := tx.Exec(` + CREATE TABLE acl_2( + p_type VARCHAR(32) DEFAULT '' NOT NULL, + v0 VARCHAR(255) DEFAULT '' NOT NULL, + v1 VARCHAR(255) DEFAULT '' NOT NULL, + v2 VARCHAR(255) DEFAULT '' NOT NULL, + v3 VARCHAR(255) DEFAULT '' NOT NULL, + v4 VARCHAR(255) DEFAULT '' NOT NULL, + v5 VARCHAR(255) DEFAULT '' NOT NULL, + CHECK (TYPEOF("p_type") = "text" AND + LENGTH("p_type") <= 32), + CHECK (TYPEOF("v0") = "text" AND + LENGTH("v0") <= 255), + CHECK (TYPEOF("v1") = "text" AND + LENGTH("v1") <= 255), + CHECK (TYPEOF("v2") = "text" AND + LENGTH("v2") <= 255), + CHECK (TYPEOF("v3") = "text" AND + LENGTH("v3") <= 255), + CHECK (TYPEOF("v4") = "text" AND + LENGTH("v4") <= 255), + CHECK (TYPEOF("v5") = "text" AND + LENGTH("v5") <= 255) + ); + `); err != nil { + return err + } + + // fresh spindles might not have acl table + hasAcl, err := tableExists(tx, "acl") + if err != nil { + return err + } + if !hasAcl { + return nil + } + + for _, role := range []string{"repo:owner", "repo:collaborator"} { + if _, err := tx.Exec(` + insert into acl_2 (p_type, v0, v1, v2) + select distinct 'g', v0, v3, v2 + from acl + where p_type = 'p' and v1 = 'thisserver' and v3 = ? + `, role); err != nil { + return err + } + } + return nil + }); err != nil { + return err + } + return nil } +func tableExists(tx *sql.Tx, name string) (bool, error) { + var exists bool + err := tx.QueryRow( + `select exists (select 1 from sqlite_master where type = 'table' and name = ?)`, + name, + ).Scan(&exists) + return exists, err +} + func hasUniqueIndex(tx *sql.Tx, table string, cols []string) (bool, error) { rows, err := tx.Query( `select name from pragma_index_list(?) where "unique" = 1`, diff --git a/spindle/db/migrations_test.go b/spindle/db/migrations_test.go new file mode 100644 index 00000000..1b7b5317 --- /dev/null +++ b/spindle/db/migrations_test.go @@ -0,0 +1,259 @@ +package db + +import ( + "context" + "database/sql" + "path/filepath" + "slices" + "testing" + + "github.com/bluesky-social/indigo/atproto/syntax" + "tangled.org/core/rbac/v2" +) + +// seedLegacyDB writes a pre-rbac/v2 spindle database: legacy `repos`, the tables that used to +// hold ACL state, and a casbin `acl` table as rbac.NewEnforcer would have left it. +func seedLegacyDB(t *testing.T, path string) { + t.Helper() + raw, err := sql.Open("sqlite3", path) + if err != nil { + t.Fatalf("open: %v", err) + } + defer raw.Close() + + if _, err := raw.Exec(` + create table migrations ( + id integer primary key autoincrement, + name text unique + ); + + create table known_dids (did text primary key); + + create table spindle_members ( + id integer primary key autoincrement, + did text not null, + rkey text not null, + instance text not null, + subject text not null, + created text not null default (strftime('%Y-%m-%dT%H:%M:%SZ', 'now')), + unique (did, rkey) + ); + + create table repos ( + id integer primary key autoincrement, + knot text not null, + owner text not null, + rkey text not null, + repo_did text, + created_at text, + addedAt text not null default (strftime('%Y-%m-%dT%H:%M:%SZ', 'now')), + unique(owner, rkey) + ); + + create table repo_collaborators ( + id integer primary key autoincrement, + owner_did text not null, + rkey text not null, + subject text not null, + repo_did text not null, + addedAt text not null default (strftime('%Y-%m-%dT%H:%M:%SZ', 'now')), + unique(owner_did, rkey) + ); + + create table acl ( + p_type text default '' not null, + v0 text default '' not null, + v1 text default '' not null, + v2 text default '' not null, + v3 text default '' not null, + v4 text default '' not null, + v5 text default '' not null + ); + + -- one repo_did with two rkeys (rename/alias siblings) plus a row that never got a did + insert into repos (knot, owner, rkey, repo_did, created_at) values + ('knot.test', 'did:plc:alice', 'old-rkey', 'did:plc:repo1', '2024-01-01T00:00:00Z'), + ('knot.test', 'did:plc:alice', 'new-rkey', 'did:plc:repo1', '2024-06-01T00:00:00Z'), + ('knot.test', 'did:plc:alice', 'no-did-rkey', null, null); + + -- did = whoever published the record (the spindle owner), subject = the member + insert into spindle_members (did, rkey, instance, subject) values + ('did:plc:owner', '3kmember001', 'spindle.test', 'did:plc:member'), + ('did:plc:owner', '3kmember002', 'spindle.test', 'did:plc:member2'); + + -- known_dids mixed collaborators in with members, so it must not seed the members table + insert into known_dids (did) values + ('did:plc:member'), + ('did:plc:bob'); + + insert into acl (p_type, v0, v1, v2, v3) values + ('g', 'did:plc:owner', 'server:owner', 'spindle:spindle.test', ''), + ('g', 'did:plc:member', 'server:member', 'spindle:spindle.test', ''), + ('g', 'did:plc:member2', 'server:member', 'spindle:spindle.test', ''), + -- repo policies, deliberately duplicated to exercise the distinct + ('p', 'did:plc:alice', 'thisserver', 'did:plc:repo1', 'repo:owner'), + ('p', 'did:plc:alice', 'thisserver', 'did:plc:repo1', 'repo:push'), + ('p', 'did:plc:alice', 'thisserver', 'did:plc:repo1', 'repo:settings'), + ('p', 'did:plc:bob', 'thisserver', 'did:plc:repo1', 'repo:collaborator'), + ('p', 'did:plc:bob', 'thisserver', 'did:plc:repo1', 'repo:push'), + ('p', 'server:owner', 'thisserver', 'did:plc:repo1', 'repo:delete'); + `); err != nil { + t.Fatalf("seed: %v", err) + } +} + +func TestMigrateLegacyAcl(t *testing.T) { + path := filepath.Join(t.TempDir(), "spindle.db") + seedLegacyDB(t, path) + + d, err := Make(context.Background(), path) + if err != nil { + t.Fatalf("Make: %v", err) + } + defer d.Close() + + t.Run("collapses repo_did siblings", func(t *testing.T) { + repo, err := d.GetRepoByDid("did:plc:repo1") + if err != nil { + t.Fatalf("GetRepoByDid: %v", err) + } + // prefers the row with the latest created_at, like the old CollapseRepoSiblings + if repo.Rkey != "new-rkey" { + t.Errorf("kept rkey %q, want new-rkey", repo.Rkey) + } + + var n int + if err := d.QueryRow(`select count(*) from repos`).Scan(&n); err != nil { + t.Fatalf("count repos: %v", err) + } + if n != 1 { + t.Errorf("repos has %d rows, want 1 (siblings collapsed, did-less row dropped)", n) + } + }) + + t.Run("seeds members from spindle_members", func(t *testing.T) { + members, err := d.ListAllowedMembers() + if err != nil { + t.Fatalf("ListAllowedMembers: %v", err) + } + + // `subject` is the member, `did` is the inviting owner - both are members + for _, want := range []syntax.DID{"did:plc:member", "did:plc:member2", "did:plc:owner"} { + if !slices.Contains(members, want) { + t.Errorf("members missing %s, got %v", want, members) + } + } + + // known_dids also held collaborators; seeding `members` from it would promote them + // to spindle members, i.e. let them register their own repos + if slices.Contains(members, "did:plc:bob") { + t.Errorf("collaborator did:plc:bob was granted spindle membership, got %v", members) + } + }) + + t.Run("rewrites acl into acl_2 grouping rows", func(t *testing.T) { + rows, err := d.Query(`select p_type, v0, v1, v2 from acl_2 order by v1, v0`) + if err != nil { + t.Fatalf("query acl_2: %v", err) + } + defer rows.Close() + + var got []string + for rows.Next() { + var pType, v0, v1, v2 string + if err := rows.Scan(&pType, &v0, &v1, &v2); err != nil { + t.Fatalf("scan: %v", err) + } + got = append(got, pType+"|"+v0+"|"+v1+"|"+v2) + } + if err := rows.Err(); err != nil { + t.Fatalf("rows: %v", err) + } + + want := []string{ + "g|did:plc:bob|repo:collaborator|did:plc:repo1", + "g|did:plc:alice|repo:owner|did:plc:repo1", + } + if len(got) != len(want) { + t.Fatalf("acl_2 rows = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("acl_2 row %d = %q, want %q", i, got[i], want[i]) + } + } + }) + + // the point of the rewrite: migrated grants have to actually enforce + t.Run("migrated grants enforce", func(t *testing.T) { + e, err := rbac.NewEnforcer(path) + if err != nil { + t.Fatalf("rbac.NewEnforcer: %v", err) + } + + for _, tc := range []struct { + name string + did syntax.DID + want bool + }{ + {"owner", "did:plc:alice", true}, + {"collaborator", "did:plc:bob", true}, + {"stranger", "did:plc:eve", false}, + } { + ok, err := e.IsRepoSecretsAllowed(tc.did, "did:plc:repo1") + if err != nil { + t.Fatalf("IsRepoSecretsAllowed(%s): %v", tc.name, err) + } + if ok != tc.want { + t.Errorf("IsRepoSecretsAllowed(%s) = %v, want %v", tc.name, ok, tc.want) + } + + ok, err = e.IsRepoCiTriggerAllowed(tc.did, "did:plc:repo1") + if err != nil { + t.Fatalf("IsRepoCiTriggerAllowed(%s): %v", tc.name, err) + } + if ok != tc.want { + t.Errorf("IsRepoCiTriggerAllowed(%s) = %v, want %v", tc.name, ok, tc.want) + } + } + }) + + // the base schema recreates these empty on every boot, so assert the stale rows are gone + // rather than the tables - reopen to get past the recreate. + t.Run("clears legacy acl tables", func(t *testing.T) { + d.Close() + reopened, err := Make(context.Background(), path) + if err != nil { + t.Fatalf("reopen: %v", err) + } + defer reopened.Close() + + for _, table := range []string{"known_dids", "repo_collaborators", "spindle_members"} { + var n int + if err := reopened.QueryRow(`select count(*) from ` + table).Scan(&n); err != nil { + t.Fatalf("count %s: %v", table, err) + } + if n != 0 { + t.Errorf("table %s still has %d stale rows", table, n) + } + } + }) +} + +// TestMigrateFreshDB covers the case where no `acl` table exists yet: it is created by the +// casbin adapter inside rbac.NewEnforcer, which runs after Make. +func TestMigrateFreshDB(t *testing.T) { + d, err := Make(context.Background(), filepath.Join(t.TempDir(), "spindle.db")) + if err != nil { + t.Fatalf("Make on fresh db: %v", err) + } + defer d.Close() + + members, err := d.ListAllowedMembers() + if err != nil { + t.Fatalf("ListAllowedMembers: %v", err) + } + if len(members) != 0 { + t.Errorf("fresh db has members %v, want none", members) + } +} -- 2.51.2