H-290: enforce Captain Skill tenant bindings (#51)

* H-290: enforce Captain Skill tenant consistency

* H-290: fix SQLite tenant migration coverage

* H-290: leave tenant constraints to migrations

---------

Co-authored-by: Rogee <rogee@ipao.vip>
This commit is contained in:
Rogee
2026-08-19 17:54:45 +08:00
committed by GitHub
co-authored by rogee
parent d54700ac13
commit be53f30394
5 changed files with 202 additions and 46 deletions
@@ -1,6 +1,8 @@
package database
import (
"fmt"
"net/url"
"os"
"path/filepath"
"testing"
@@ -12,69 +14,155 @@ import (
"gorm.io/driver/sqlite"
"gorm.io/gorm"
"gorm.io/gorm/logger"
"github.com/gochat/gochat/internal/model"
)
func TestCaptainSkillSchemaSQLiteRoundTrip(t *testing.T) {
db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)})
require.NoError(t, err)
require.NoError(t, db.AutoMigrate(&model.CaptainAssistant{}, &model.CaptainSkill{}, &model.CaptainSkillReference{}, &model.CaptainAssistantSkill{}))
for _, table := range []string{"captain_skills", "captain_skill_references", "captain_assistant_skills"} {
assert.True(t, db.Migrator().HasTable(table), table)
require.NoError(t, db.Migrator().DropTable(table))
assert.False(t, db.Migrator().HasTable(table), table)
const captainSkillMigrationBaseline = `
CREATE TABLE accounts (id INTEGER PRIMARY KEY);
CREATE TABLE captain_assistants (id INTEGER PRIMARY KEY, account_id INTEGER NOT NULL);
CREATE TABLE captain_skills (id INTEGER PRIMARY KEY, account_id INTEGER NOT NULL);
CREATE TABLE captain_assistant_skills (
id INTEGER PRIMARY KEY,
account_id INTEGER NOT NULL REFERENCES accounts(id) ON DELETE CASCADE,
assistant_id INTEGER NOT NULL REFERENCES captain_assistants(id) ON DELETE CASCADE,
skill_id INTEGER NOT NULL REFERENCES captain_skills(id) ON DELETE CASCADE,
created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
UNIQUE (assistant_id, skill_id)
);
CREATE INDEX idx_captain_assistant_skills_account ON captain_assistant_skills(account_id);
CREATE INDEX idx_captain_assistant_skills_skill ON captain_assistant_skills(skill_id);
`
func captainSkillMigrationDir(t *testing.T, dialect string) string {
t.Helper()
dir := t.TempDir()
for _, direction := range []string{"up", "down"} {
require.NoError(t, os.WriteFile(filepath.Join(dir, "000080_baseline."+direction+".sql"), []byte("SELECT 1;"), 0o600))
}
for _, direction := range []string{"up", "down"} {
name := "000081_enforce_captain_skill_tenant_consistency." + direction + ".sql"
parts := []string{"..", "..", "migrations"}
if dialect != "" {
parts = append(parts, dialect)
}
data, err := os.ReadFile(filepath.Join(append(parts, name)...))
require.NoError(t, err)
require.NoError(t, os.WriteFile(filepath.Join(dir, name), data, 0o600))
}
return dir
}
func openCaptainMigrationPostgres(t *testing.T) *gorm.DB {
t.Helper()
dsn := os.Getenv("GOCHAT_TEST_DB_URL")
if dsn == "" {
dsn = "host=localhost port=5432 user=postgres password=postgres dbname=gochat_test sslmode=disable"
}
db, err := gorm.Open(postgres.Open(dsn), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)})
func TestCaptainSkillSQLiteMigrationRoundTrip(t *testing.T) {
dbPath := filepath.Join(t.TempDir(), "captain-skill.db")
dbURL := "sqlite3://" + dbPath + "?_foreign_keys=on"
db, err := gorm.Open(sqlite.Open(dbPath+"?_foreign_keys=on"), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)})
require.NoError(t, err)
sqlDB, err := db.DB()
require.NoError(t, err)
sqlDB.SetMaxOpenConns(1)
t.Cleanup(func() { _ = sqlDB.Close() })
schema := "captain_skill_migration_" + time.Now().Format("20060102150405000000000")
require.NoError(t, db.Exec("CREATE SCHEMA "+schema).Error)
t.Cleanup(func() { _ = db.Exec("DROP SCHEMA " + schema + " CASCADE").Error })
require.NoError(t, db.Exec("SET search_path TO "+schema).Error)
return db
require.NoError(t, db.Exec(captainSkillMigrationBaseline).Error)
require.NoError(t, db.Exec("INSERT INTO accounts VALUES (1), (2); INSERT INTO captain_assistants VALUES (1, 1), (2, 2); INSERT INTO captain_skills VALUES (1, 1), (2, 2)").Error)
migrations := captainSkillMigrationDir(t, "sqlite")
require.NoError(t, ForceVersion(dbURL, migrations, 80))
require.NoError(t, MigrateSteps(dbURL, migrations, 1))
version, dirty, err := CurrentVersion(dbURL, migrations)
require.NoError(t, err)
assert.Equal(t, uint(81), version)
assert.False(t, dirty)
require.NoError(t, db.Exec("INSERT INTO captain_assistant_skills(id, account_id, assistant_id, skill_id) VALUES (1, 1, 1, 1)").Error)
assert.Error(t, db.Exec("INSERT INTO captain_assistant_skills(id, account_id, assistant_id, skill_id) VALUES (2, 1, 2, 1)").Error)
assert.Error(t, db.Exec("INSERT INTO captain_assistant_skills(id, account_id, assistant_id, skill_id) VALUES (3, 1, 1, 2)").Error)
require.NoError(t, MigrateSteps(dbURL, migrations, -1))
version, dirty, err = CurrentVersion(dbURL, migrations)
require.NoError(t, err)
assert.Equal(t, uint(80), version)
assert.False(t, dirty)
require.NoError(t, db.Exec("INSERT INTO captain_assistant_skills(id, account_id, assistant_id, skill_id) VALUES (2, 1, 2, 1), (3, 1, 1, 2)").Error)
assert.False(t, db.Migrator().HasIndex("captain_assistants", "idx_captain_assistants_account_id_id"))
assert.False(t, db.Migrator().HasIndex("captain_skills", "idx_captain_skills_account_id_id"))
}
func openCaptainMigrationPostgres(t *testing.T) (*gorm.DB, string) {
t.Helper()
dsn := os.Getenv("GOCHAT_TEST_DB_URL")
if dsn == "" {
dsn = "postgres://postgres:postgres@localhost:5432/gochat_test?sslmode=disable"
}
admin, err := gorm.Open(postgres.Open(dsn), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)})
require.NoError(t, err)
adminDB, err := admin.DB()
require.NoError(t, err)
t.Cleanup(func() { _ = adminDB.Close() })
schema := fmt.Sprintf("captain_skill_migration_%d", time.Now().UnixNano())
require.NoError(t, admin.Exec("CREATE SCHEMA "+schema).Error)
t.Cleanup(func() { _ = admin.Exec("DROP SCHEMA " + schema + " CASCADE").Error })
migrationURL, err := url.Parse(dsn)
require.NoError(t, err)
require.NotEmpty(t, migrationURL.Scheme, "GOCHAT_TEST_DB_URL must be a PostgreSQL URL")
query := migrationURL.Query()
query.Set("search_path", schema)
migrationURL.RawQuery = query.Encode()
db, err := gorm.Open(postgres.Open(migrationURL.String()), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)})
require.NoError(t, err)
sqlDB, err := db.DB()
require.NoError(t, err)
sqlDB.SetMaxOpenConns(1)
t.Cleanup(func() { _ = sqlDB.Close() })
require.NoError(t, db.Exec(captainSkillMigrationBaseline).Error)
return db, migrationURL.String()
}
func runCaptainSkillMigration(t *testing.T, fn func() error) {
t.Helper()
require.NoError(t, withSanitizedPQEnvironment(fn))
}
func TestCaptainSkillPostgresMigrationRoundTrip(t *testing.T) {
if os.Getenv("GOCHAT_TEST_DB") == "sqlite" {
t.Skip("PostgreSQL migration test")
}
db := openCaptainMigrationPostgres(t)
require.NoError(t, db.Exec("CREATE TABLE accounts (id SERIAL PRIMARY KEY); CREATE TABLE captain_assistants (id SERIAL PRIMARY KEY, account_id INTEGER NOT NULL);").Error)
db, dbURL := openCaptainMigrationPostgres(t)
require.NoError(t, db.Exec("INSERT INTO accounts VALUES (1), (2); INSERT INTO captain_assistants VALUES (1, 1), (2, 2); INSERT INTO captain_skills VALUES (1, 1), (2, 2)").Error)
migrations := captainSkillMigrationDir(t, "")
runCaptainSkillMigration(t, func() error { return ForceVersion(dbURL, migrations, 80) })
runCaptainSkillMigration(t, func() error { return MigrateSteps(dbURL, migrations, 1) })
up, err := os.ReadFile(filepath.Join("..", "..", "migrations", "000080_add_captain_skills.up.sql"))
require.NoError(t, err)
require.NoError(t, db.Exec(string(up)).Error)
for _, table := range []string{"captain_skills", "captain_skill_references", "captain_assistant_skills"} {
assert.True(t, db.Migrator().HasTable(table), table)
}
require.NoError(t, db.Exec("INSERT INTO accounts(id) VALUES (1); INSERT INTO captain_assistants(id, account_id) VALUES (1, 1);").Error)
require.NoError(t, db.Exec("INSERT INTO captain_skills(id, account_id, name, description, instructions_md, status) VALUES (1, 1, 'policy', 'Policy', 'Use it', 'active')").Error)
require.NoError(t, db.Exec("INSERT INTO captain_skill_references(skill_id, reference_key, content_md, position) VALUES (1, 'standard', 'Terms', 0)").Error)
require.NoError(t, db.Exec("INSERT INTO captain_assistant_skills(account_id, assistant_id, skill_id) VALUES (1, 1, 1)").Error)
assert.Error(t, db.Exec("INSERT INTO captain_skill_references(skill_id, reference_key, content_md, position) VALUES (1, 'standard', 'Duplicate', 1)").Error)
require.NoError(t, db.Exec("DELETE FROM captain_skills WHERE id = 1").Error)
for _, table := range []string{"captain_skill_references", "captain_assistant_skills"} {
var count int64
require.NoError(t, db.Table(table).Count(&count).Error)
assert.Zero(t, count, table)
}
require.NoError(t, db.Exec("INSERT INTO captain_assistant_skills(id, account_id, assistant_id, skill_id) VALUES (1, 1, 1, 1)").Error)
assert.Error(t, db.Exec("INSERT INTO captain_assistant_skills(id, account_id, assistant_id, skill_id) VALUES (2, 1, 2, 1)").Error)
assert.Error(t, db.Exec("INSERT INTO captain_assistant_skills(id, account_id, assistant_id, skill_id) VALUES (3, 1, 1, 2)").Error)
down, err := os.ReadFile(filepath.Join("..", "..", "migrations", "000080_add_captain_skills.down.sql"))
require.NoError(t, err)
require.NoError(t, db.Exec(string(down)).Error)
for _, table := range []string{"captain_skills", "captain_skill_references", "captain_assistant_skills"} {
assert.False(t, db.Migrator().HasTable(table), table)
runCaptainSkillMigration(t, func() error { return MigrateSteps(dbURL, migrations, -1) })
require.NoError(t, db.Exec("INSERT INTO captain_assistant_skills(id, account_id, assistant_id, skill_id) VALUES (2, 1, 2, 1), (3, 1, 1, 2)").Error)
var count int64
require.NoError(t, db.Raw("SELECT count(*) FROM pg_indexes WHERE schemaname = current_schema() AND indexname IN ('idx_captain_assistants_account_id_id', 'idx_captain_skills_account_id_id')").Scan(&count).Error)
assert.Zero(t, count)
}
func TestCaptainSkillPostgresMigrationRejectsDirtyBindingsAtomically(t *testing.T) {
if os.Getenv("GOCHAT_TEST_DB") == "sqlite" {
t.Skip("PostgreSQL migration test")
}
db, dbURL := openCaptainMigrationPostgres(t)
require.NoError(t, db.Exec("INSERT INTO accounts VALUES (1), (2); INSERT INTO captain_assistants VALUES (1, 1), (2, 2); INSERT INTO captain_skills VALUES (1, 1), (2, 2); INSERT INTO captain_assistant_skills(id, account_id, assistant_id, skill_id) VALUES (1, 1, 2, 1)").Error)
migrations := captainSkillMigrationDir(t, "")
runCaptainSkillMigration(t, func() error { return ForceVersion(dbURL, migrations, 80) })
err := withSanitizedPQEnvironment(func() error { return MigrateSteps(dbURL, migrations, 1) })
require.Error(t, err)
var count int64
require.NoError(t, db.Table("captain_assistant_skills").Count(&count).Error)
assert.Equal(t, int64(1), count)
require.NoError(t, db.Raw("SELECT count(*) FROM pg_indexes WHERE schemaname = current_schema() AND indexname IN ('idx_captain_assistants_account_id_id', 'idx_captain_skills_account_id_id')").Scan(&count).Error)
assert.Zero(t, count)
require.NoError(t, db.Raw("SELECT count(*) FROM pg_constraint WHERE connamespace = current_schema()::regnamespace AND conname IN ('fk_captain_assistant_skills_assistant_tenant', 'fk_captain_assistant_skills_skill_tenant')").Scan(&count).Error)
assert.Zero(t, count)
version, dirty, versionErr := CurrentVersion(dbURL, migrations)
require.NoError(t, versionErr)
assert.Equal(t, uint(81), version)
assert.True(t, dirty)
}
@@ -0,0 +1,6 @@
ALTER TABLE captain_assistant_skills
DROP CONSTRAINT IF EXISTS fk_captain_assistant_skills_assistant_tenant,
DROP CONSTRAINT IF EXISTS fk_captain_assistant_skills_skill_tenant;
DROP INDEX IF EXISTS idx_captain_assistants_account_id_id;
DROP INDEX IF EXISTS idx_captain_skills_account_id_id;
@@ -0,0 +1,12 @@
CREATE UNIQUE INDEX idx_captain_assistants_account_id_id
ON captain_assistants(account_id, id);
CREATE UNIQUE INDEX idx_captain_skills_account_id_id
ON captain_skills(account_id, id);
ALTER TABLE captain_assistant_skills
ADD CONSTRAINT fk_captain_assistant_skills_assistant_tenant
FOREIGN KEY (account_id, assistant_id)
REFERENCES captain_assistants(account_id, id) ON DELETE CASCADE,
ADD CONSTRAINT fk_captain_assistant_skills_skill_tenant
FOREIGN KEY (account_id, skill_id)
REFERENCES captain_skills(account_id, id) ON DELETE CASCADE;
@@ -0,0 +1,21 @@
CREATE TABLE captain_assistant_skills_000081 (
id INTEGER PRIMARY KEY AUTOINCREMENT,
account_id INTEGER NOT NULL REFERENCES accounts(id) ON DELETE CASCADE,
assistant_id INTEGER NOT NULL REFERENCES captain_assistants(id) ON DELETE CASCADE,
skill_id INTEGER NOT NULL REFERENCES captain_skills(id) ON DELETE CASCADE,
created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
UNIQUE (assistant_id, skill_id)
);
INSERT INTO captain_assistant_skills_000081
(id, account_id, assistant_id, skill_id, created_at)
SELECT id, account_id, assistant_id, skill_id, created_at
FROM captain_assistant_skills;
DROP TABLE captain_assistant_skills;
ALTER TABLE captain_assistant_skills_000081 RENAME TO captain_assistant_skills;
CREATE INDEX idx_captain_assistant_skills_account ON captain_assistant_skills(account_id);
CREATE INDEX idx_captain_assistant_skills_skill ON captain_assistant_skills(skill_id);
DROP INDEX idx_captain_assistants_account_id_id;
DROP INDEX idx_captain_skills_account_id_id;
@@ -0,0 +1,29 @@
CREATE UNIQUE INDEX idx_captain_assistants_account_id_id
ON captain_assistants(account_id, id);
CREATE UNIQUE INDEX idx_captain_skills_account_id_id
ON captain_skills(account_id, id);
CREATE TABLE captain_assistant_skills_000081 (
id INTEGER PRIMARY KEY AUTOINCREMENT,
account_id INTEGER NOT NULL REFERENCES accounts(id) ON DELETE CASCADE,
assistant_id INTEGER NOT NULL,
skill_id INTEGER NOT NULL,
created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
UNIQUE (assistant_id, skill_id),
CONSTRAINT fk_captain_assistant_skills_assistant_tenant
FOREIGN KEY (account_id, assistant_id)
REFERENCES captain_assistants(account_id, id) ON DELETE CASCADE,
CONSTRAINT fk_captain_assistant_skills_skill_tenant
FOREIGN KEY (account_id, skill_id)
REFERENCES captain_skills(account_id, id) ON DELETE CASCADE
);
INSERT INTO captain_assistant_skills_000081
(id, account_id, assistant_id, skill_id, created_at)
SELECT id, account_id, assistant_id, skill_id, created_at
FROM captain_assistant_skills;
DROP TABLE captain_assistant_skills;
ALTER TABLE captain_assistant_skills_000081 RENAME TO captain_assistant_skills;
CREATE INDEX idx_captain_assistant_skills_account ON captain_assistant_skills(account_id);
CREATE INDEX idx_captain_assistant_skills_skill ON captain_assistant_skills(skill_id);