From 8d65201a9379bc3341aacb9558f2f8d51fc94511 Mon Sep 17 00:00:00 2001 From: Igor Serganov Date: Fri, 25 Sep 2026 09:08:24 -0700 Subject: [PATCH] db: check errors in openDB On the SQLite path openDB called db.DB() and configured the pool before looking at the error from gorm.Open, and both backends discarded the error from db.DB(). A failed open returned a half-initialised *gorm.DB alongside the error, and a nil *sql.DB from db.DB() would have been dereferenced. Return early on either error so callers get a nil handle and the original error. --- hscontrol/db/db.go | 17 +++++++++++--- hscontrol/db/db_test.go | 51 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/hscontrol/db/db.go b/hscontrol/db/db.go index 1bd8dd010..3feacb36c 100644 --- a/hscontrol/db/db.go +++ b/hscontrol/db/db.go @@ -446,16 +446,23 @@ func openDB(cfg types.DatabaseConfig) (*gorm.DB, error) { Logger: dbLogger, }, ) + if err != nil { + return nil, err + } // The pure Go SQLite library does not handle locking in // the same way as the C based one and we can't use the gorm // connection pool as of 2022/02/23. - sqlDB, _ := db.DB() + sqlDB, err := db.DB() + if err != nil { + return nil, err + } + sqlDB.SetMaxIdleConns(1) sqlDB.SetMaxOpenConns(1) sqlDB.SetConnMaxIdleTime(time.Hour) - return db, err + return db, nil case types.DatabasePostgres: dbString := fmt.Sprintf( @@ -493,7 +500,11 @@ func openDB(cfg types.DatabaseConfig) (*gorm.DB, error) { return nil, err } - sqlDB, _ := db.DB() + sqlDB, err := db.DB() + if err != nil { + return nil, err + } + sqlDB.SetMaxIdleConns(cfg.Postgres.MaxIdleConnections) sqlDB.SetMaxOpenConns(cfg.Postgres.MaxOpenConnections) sqlDB.SetConnMaxIdleTime( diff --git a/hscontrol/db/db_test.go b/hscontrol/db/db_test.go index fb4e7b97c..2cd0d9edc 100644 --- a/hscontrol/db/db_test.go +++ b/hscontrol/db/db_test.go @@ -910,3 +910,54 @@ func TestInterruptedInitSchemaRecovers(t *testing.T) { require.NoError(t, err) }) } + +func TestOpenDBReturnsOpenError(t *testing.T) { + tests := []struct { + name string + cfg func(t *testing.T) types.DatabaseConfig + }{ + { + name: "sqlite-path-is-a-directory", + cfg: func(t *testing.T) types.DatabaseConfig { + t.Helper() + + return types.DatabaseConfig{ + Type: types.DatabaseSqlite, + Sqlite: types.SqliteConfig{Path: t.TempDir()}, + } + }, + }, + { + name: "postgres-unreachable-host", + cfg: func(t *testing.T) types.DatabaseConfig { + t.Helper() + + return types.DatabaseConfig{ + Type: types.DatabasePostgres, + Postgres: types.PostgresConfig{ + Host: "127.0.0.1", + Port: 1, + Name: "headscale", + User: "headscale", + Ssl: "false", + }, + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var ( + db *gorm.DB + err error + ) + + require.NotPanics(t, func() { + db, err = openDB(tt.cfg(t)) + }) + require.Error(t, err) + assert.Nil(t, db) + }) + } +}