diff --git a/internal/store/migrations_test.go b/internal/store/migrations_test.go index fa0144b..fd0ba71 100644 --- a/internal/store/migrations_test.go +++ b/internal/store/migrations_test.go @@ -299,6 +299,124 @@ func TestOpenReadOnlyRefusesWrites(t *testing.T) { } } +// TestRunMigrationsIgnoresUnknownAppliedIDs simulates an older banger +// opening a DB that was written by a newer version: schema_migrations +// carries rows with ids the current binary's migrations slice doesn't +// know about. The runner must leave those rows alone and still apply +// any of its own known migrations that haven't been recorded yet. +// +// Without this behaviour, upgrading forward then downgrading back +// (or running two daemon versions against the same state dir) would +// either fail outright or start destructively reinterpreting rows. +func TestRunMigrationsIgnoresUnknownAppliedIDs(t *testing.T) { + db := openRawDB(t) + + // Bootstrap schema_migrations and pre-seed a row for a migration + // id the current binary doesn't know. Use a high id so it's + // clearly outside our slice. + if _, err := db.Exec(`CREATE TABLE IF NOT EXISTS schema_migrations ( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL, + applied_at TEXT NOT NULL + )`); err != nil { + t.Fatalf("seed schema_migrations: %v", err) + } + if _, err := db.Exec( + "INSERT INTO schema_migrations (id, name, applied_at) VALUES (?, ?, ?)", + 9001, "from-the-future", "2099-01-01T00:00:00Z", + ); err != nil { + t.Fatalf("seed future migration row: %v", err) + } + + if err := runMigrations(db); err != nil { + t.Fatalf("runMigrations: %v", err) + } + + // The alien row is untouched. + var name string + if err := db.QueryRow("SELECT name FROM schema_migrations WHERE id = 9001").Scan(&name); err != nil { + t.Fatalf("alien migration row disappeared: %v", err) + } + if name != "from-the-future" { + t.Fatalf("alien row name = %q, want 'from-the-future'", name) + } + + // Every known migration in our slice was applied — their rows + // should exist too. + for _, m := range migrations { + var got string + if err := db.QueryRow("SELECT name FROM schema_migrations WHERE id = ?", m.id).Scan(&got); err != nil { + t.Fatalf("migration %d not recorded despite unknown alien row: %v", m.id, err) + } + } +} + +// TestDropColumnIfExistsIsIdempotent pins the "run twice, no harm" +// property. A daemon that restarts after a successful migration 2 +// on a fresh install shouldn't fail because the column is already +// gone. migrateDropDeadImageColumns calls dropColumnIfExists, which +// must silently succeed when the column is absent. +func TestDropColumnIfExistsIsIdempotent(t *testing.T) { + db := openRawDB(t) + // Set up a tiny table with a known column we're going to drop. + if _, err := db.Exec(`CREATE TABLE throwaway (keeper TEXT, victim TEXT)`); err != nil { + t.Fatalf("CREATE: %v", err) + } + + run := func(label string) error { + tx, err := db.Begin() + if err != nil { + t.Fatalf("%s Begin: %v", label, err) + } + if err := dropColumnIfExists(tx, "throwaway", "victim"); err != nil { + _ = tx.Rollback() + return err + } + return tx.Commit() + } + + if err := run("first"); err != nil { + t.Fatalf("first dropColumnIfExists: %v", err) + } + // Second call against a table that no longer has the column. + if err := run("second"); err != nil { + t.Fatalf("second dropColumnIfExists (column already gone): %v", err) + } + + // The keeper column must still be there; victim is gone. + rows, err := db.Query("PRAGMA table_info(throwaway)") + if err != nil { + t.Fatalf("PRAGMA: %v", err) + } + defer rows.Close() + var haveKeeper, haveVictim bool + for rows.Next() { + var ( + cid int + name string + valueType string + notNull int + defaultV sql.NullString + pk int + ) + if err := rows.Scan(&cid, &name, &valueType, ¬Null, &defaultV, &pk); err != nil { + t.Fatalf("scan: %v", err) + } + switch name { + case "keeper": + haveKeeper = true + case "victim": + haveVictim = true + } + } + if !haveKeeper { + t.Fatal("keeper column disappeared — dropColumnIfExists is too aggressive") + } + if haveVictim { + t.Fatal("victim column survived — dropColumnIfExists didn't actually drop") + } +} + func TestRunMigrationsRejectsDuplicateID(t *testing.T) { db := openRawDB(t) orig := migrations diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 4da722e..8d91784 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "errors" + "os" "path/filepath" "reflect" "strconv" @@ -319,6 +320,58 @@ func TestStoreConfiguresSQLitePragmasOnPooledConnections(t *testing.T) { } } +// TestOpenRejectsCorruptDB pins the actionable-error contract when +// state.db exists on disk but isn't a valid SQLite file. Users can +// hit this after a disk-full crash mid-write, a copy that truncated, +// or accidental manual editing. banger must surface the error +// cleanly so the operator can delete-and-retry — never panic, never +// silently overwrite, never leak a partially-opened sql.DB handle. +func TestOpenRejectsCorruptDB(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path := filepath.Join(dir, "state.db") + garbage := []byte("this is definitely not a sqlite database") + if err := os.WriteFile(path, garbage, 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + s, err := Open(path) + if err == nil { + _ = s.Close() + t.Fatal("Open: want error on corrupt DB file") + } + + // The garbage bytes must still be there — Open must not have + // overwritten the file mid-attempt. A user recovering from a + // mid-write crash needs that invariant to hand the file to a + // tool like sqlite3_analyzer. + got, readErr := os.ReadFile(path) + if readErr != nil { + t.Fatalf("ReadFile: %v", readErr) + } + if string(got) != string(garbage) { + t.Fatalf("Open touched the garbage file: got %q, want %q", string(got), string(garbage)) + } +} + +// TestOpenReadOnlyRejectsMissingDB pins the "no silent creation" +// contract for the doctor path: OpenReadOnly against a path that +// doesn't exist must error, not create an empty DB that later reads +// would mistake for "no VMs yet." +func TestOpenReadOnlyRejectsMissingDB(t *testing.T) { + t.Parallel() + missing := filepath.Join(t.TempDir(), "never-existed.db") + s, err := OpenReadOnly(missing) + if err == nil { + _ = s.Close() + t.Fatal("OpenReadOnly: want error when the DB file doesn't exist") + } + if _, statErr := os.Stat(missing); !os.IsNotExist(statErr) { + t.Fatalf("OpenReadOnly silently created %q (stat err = %v)", missing, statErr) + } +} + func openTestStore(t *testing.T) *Store { t.Helper() store, err := Open(filepath.Join(t.TempDir(), "state.db"))