fix(v1): сделать read-only свойством doctor, а sentinel-ошибки — решением
Два свойства были описаны в документации, но не обеспечены кодом.
1. doctor «не изменяет диагностируемую систему».
Принудительный skipServiceStart закрывал ровно одну ИЗВЕСТНУЮ мутацию —
рестарт сервисов. Всё остальное в smoke держалось на том, что автор правки
выбрал правильный раннер: `test -s`, `grep -q`, `stat`, `sudo -u ... test`
и `nft -c` шли через мутирующий namespace, хотя ничего не меняют. Ожидание
между попытками выполнялось подпроцессом `sleep` через runMutatingHidden,
то есть пауза между двумя чтениями объявлялась изменением системы.
Следствие: настоящая мутация, случайно добавленная в smoke, ничем бы от них
не отличалась и была бы разрешена в doctor молча — а включить guard было
нельзя, он отказал бы на первой же читающей команде.
Команды классифицированы честно, `sleep` заменён таймером, и doctor целиком
выполняется под тем же read-only guard, что и PHASE 0 установки. Guard
снимается в finally. Диагностика при этом не сузилась: слушатели, healthz,
права, machine auth, trafficStats, версия бинаря, семантика конфига и
синтаксис nft проверяются полностью.
2. reset-admin различает «администратора нет» и «база не ответила».
Слой данных специально возвращает разные sentinel'ы, но команда склеивала их
обычным `if err != nil { создать } else { обновить }`. Опасен здесь не
только нарушенный смысл: при транзиентном отказе чтения («database is
locked») ветка создания отрабатывала успешно, и в таблице оказывались ДВЕ
учётные записи администратора. GetAdminUser берёт First() и о второй строке
не сообщает — на сервере оставалась вторая рабочая учётка с паролем, уже
напечатанным на экран, и ни один запрос об этом не говорил.
Заодно исправлено проглатывание ошибки хеширования: в ветке обновления
стояло `hash, _ := util.HashPassword(password)` внутри литерала map. При
отказе bcrypt в password_hash уезжала пустая строка, а на экран печатался
пароль, которым войти уже невозможно — VerifyPassword отклоняет всё, что не
bcrypt. Команда восстановления доступа умела молча его отобрать.
Тесты: doctor-readonly.test.ts дополнен поведенческой проверкой guard и
контролем набора раннеров в smoke; apps/cmd/reset_test.go проверяет обе ветки
на настоящей SQLite и отказ чтения при полностью работоспособной базе — ровно
тот случай, который прежний код превращал во второго администратора. Добавлена
dao.CountAdminUsers: до неё появление дубликата было ненаблюдаемым.
This commit is contained in:
+77
-34
@@ -1,6 +1,7 @@
|
||||
package cmd
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"github.com/spf13/cobra"
|
||||
"hy2xs-admin/dao"
|
||||
@@ -34,6 +35,15 @@ const (
|
||||
resetPasswordLength = 24
|
||||
)
|
||||
|
||||
// adminLookup — способ узнать о существующей учётной записи администратора.
|
||||
//
|
||||
// Параметризовано ради теста на отказ хранилища. Отличить «ветку создания» от
|
||||
// «ветки обновления» при недоступной базе иначе нельзя: при по-настоящему
|
||||
// сломанной базе обе ветки заканчиваются ошибкой записи, и наблюдаемый
|
||||
// результат совпадает. Опасен же ровно транзиентный отказ, когда чтение упало,
|
||||
// а запись прошла.
|
||||
type adminLookup func() (entity.AdminUser, error)
|
||||
|
||||
func runReset(cmd *cobra.Command, args []string) {
|
||||
username, err := util.RandomString(resetUsernameLength)
|
||||
if err != nil {
|
||||
@@ -49,18 +59,69 @@ func runReset(cmd *cobra.Command, args []string) {
|
||||
fmt.Println(err.Error())
|
||||
os.Exit(1)
|
||||
}
|
||||
admin, err := dao.GetAdminUser("1 = 1")
|
||||
nowMs := time.Now().UnixMilli()
|
||||
|
||||
lookup := func() (entity.AdminUser, error) { return dao.GetAdminUser("1 = 1") }
|
||||
if err = resetAdminCredentials(lookup, username, password, time.Now().UnixMilli()); err != nil {
|
||||
fmt.Println(err.Error())
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
if err = dao.CloseSqliteDB(); err != nil {
|
||||
fmt.Println(err.Error())
|
||||
os.Exit(1)
|
||||
}
|
||||
fmt.Println(fmt.Sprintf("HY2XS admin Login Username: %s", username))
|
||||
fmt.Println(fmt.Sprintf("HY2XS admin Login Password: %s", password))
|
||||
}
|
||||
|
||||
func resetAdminCredentials(lookup adminLookup, username, password string, nowMs int64) error {
|
||||
// Хеш считается ОДИН раз и ДО записи.
|
||||
//
|
||||
// В ветке обновления стояло `hash, _ := util.HashPassword(password)` внутри
|
||||
// литерала map. Ошибка bcrypt проглатывалась, в password_hash уезжала пустая
|
||||
// строка, а на экран печатался пароль, которым войти уже невозможно:
|
||||
// VerifyPassword отклоняет всё, что не является bcrypt-хешем. То есть
|
||||
// команда восстановления доступа умела молча его отобрать.
|
||||
hash, err := util.HashPassword(password)
|
||||
if err != nil {
|
||||
return fmt.Errorf("не удалось захешировать пароль восстановления: %w", err)
|
||||
}
|
||||
|
||||
admin, err := lookup()
|
||||
|
||||
// Отказ хранилища — это не «администратора нет».
|
||||
//
|
||||
// Здесь стояло обычное `if err != nil { создать } else { обновить }`, хотя
|
||||
// слой данных специально различает ErrAdminUserNotFound и ErrStorage.
|
||||
// Склейка опасна не только нарушением смысла sentinel'ов: при транзиентном
|
||||
// отказе SQLite («database is locked») ветка создания отрабатывала успешно,
|
||||
// и в таблице оказывались ДВЕ учётные записи администратора.
|
||||
// `GetAdminUser("1 = 1").First()` дальше отдаёт произвольную из них, то есть
|
||||
// на сервере остаётся вторая рабочая учётка с паролем, который уже был
|
||||
// напечатан на экран.
|
||||
switch {
|
||||
case err == nil:
|
||||
tokenVersion := int64(1)
|
||||
if admin.TokenVersion != nil && *admin.TokenVersion > 0 {
|
||||
tokenVersion = *admin.TokenVersion + 1
|
||||
}
|
||||
if updateErr := dao.UpdateAdminUser([]int64{*admin.Id}, map[string]interface{}{
|
||||
"username": username,
|
||||
"password_hash": hash,
|
||||
"force_password_change": 1,
|
||||
"password_changed_at": nowMs,
|
||||
"token_version": tokenVersion,
|
||||
"status": 1,
|
||||
}); updateErr != nil {
|
||||
return fmt.Errorf("не удалось обновить учётную запись администратора: %w", updateErr)
|
||||
}
|
||||
return nil
|
||||
|
||||
case errors.Is(err, dao.ErrAdminUserNotFound):
|
||||
tokenVersion := int64(1)
|
||||
status := int64(1)
|
||||
forcePasswordChange := int64(1)
|
||||
passwordChangedAt := nowMs
|
||||
hash, hashErr := util.HashPassword(password)
|
||||
if hashErr != nil {
|
||||
fmt.Println(hashErr.Error())
|
||||
os.Exit(1)
|
||||
}
|
||||
adminUser := entity.AdminUser{
|
||||
Username: &username,
|
||||
PasswordHash: &hash,
|
||||
@@ -70,33 +131,15 @@ func runReset(cmd *cobra.Command, args []string) {
|
||||
TokenVersion: &tokenVersion,
|
||||
}
|
||||
if _, saveErr := dao.SaveAdminUser(adminUser); saveErr != nil {
|
||||
fmt.Println(saveErr.Error())
|
||||
os.Exit(1)
|
||||
}
|
||||
} else {
|
||||
tokenVersion := int64(1)
|
||||
if admin.TokenVersion != nil && *admin.TokenVersion > 0 {
|
||||
tokenVersion = *admin.TokenVersion + 1
|
||||
}
|
||||
if err = dao.UpdateAdminUser([]int64{*admin.Id}, map[string]interface{}{
|
||||
"username": username,
|
||||
"password_hash": func() string {
|
||||
hash, _ := util.HashPassword(password)
|
||||
return hash
|
||||
}(),
|
||||
"force_password_change": 1,
|
||||
"password_changed_at": nowMs,
|
||||
"token_version": tokenVersion,
|
||||
"status": 1,
|
||||
}); err != nil {
|
||||
fmt.Println(err.Error())
|
||||
os.Exit(1)
|
||||
return fmt.Errorf("не удалось создать учётную запись администратора: %w", saveErr)
|
||||
}
|
||||
return nil
|
||||
|
||||
default:
|
||||
return fmt.Errorf(
|
||||
"не удалось прочитать учётную запись администратора: база данных не ответила. "+
|
||||
"Сброс не выполнен: создавать вторую учётную запись при недоступной базе нельзя: %w",
|
||||
err,
|
||||
)
|
||||
}
|
||||
if err = dao.CloseSqliteDB(); err != nil {
|
||||
fmt.Println(err.Error())
|
||||
os.Exit(1)
|
||||
}
|
||||
fmt.Println(fmt.Sprintf("HY2XS admin Login Username: %s", username))
|
||||
fmt.Println(fmt.Sprintf("HY2XS admin Login Password: %s", password))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,226 @@
|
||||
package cmd
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"hy2xs-admin/dao"
|
||||
"hy2xs-admin/model/entity"
|
||||
"hy2xs-admin/util"
|
||||
)
|
||||
|
||||
// reset-admin — команда восстановления доступа, и ошибиться ей дороже, чем
|
||||
// обычному обработчику: она печатает новые учётные данные на экран и на этом
|
||||
// основании оператор считает доступ восстановленным.
|
||||
//
|
||||
// Здесь закрепляются два дефекта:
|
||||
//
|
||||
// 1. отказ хранилища трактовался как «администратора нет», то есть переводил
|
||||
// команду в ветку СОЗДАНИЯ учётной записи;
|
||||
// 2. ошибка bcrypt в ветке обновления проглатывалась (`hash, _ := ...`), и в
|
||||
// password_hash уезжала пустая строка.
|
||||
|
||||
func newAdminDB(t *testing.T) {
|
||||
t.Helper()
|
||||
|
||||
dbPath := filepath.Join(t.TempDir(), "hy2xs-admin-test.db")
|
||||
if err := dao.InitSqliteDBAt(dbPath); err != nil {
|
||||
t.Fatalf("не удалось открыть тестовую базу: %v", err)
|
||||
}
|
||||
if err := dao.RunMigrations(); err != nil {
|
||||
t.Fatalf("не удалось применить миграции: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = dao.CloseSqliteDB() })
|
||||
}
|
||||
|
||||
func realLookup() (entity.AdminUser, error) {
|
||||
return dao.GetAdminUser("1 = 1")
|
||||
}
|
||||
|
||||
func countAdmins(t *testing.T) int64 {
|
||||
t.Helper()
|
||||
|
||||
count, err := dao.CountAdminUsers()
|
||||
if err != nil {
|
||||
t.Fatalf("не удалось посчитать администраторов: %v", err)
|
||||
}
|
||||
return count
|
||||
}
|
||||
|
||||
func TestResetCreatesAdminWhenNoneExists(t *testing.T) {
|
||||
newAdminDB(t)
|
||||
|
||||
if err := resetAdminCredentials(realLookup, "operator-1", "recovery-password-1", 1700000000000); err != nil {
|
||||
t.Fatalf("сброс не выполнен: %v", err)
|
||||
}
|
||||
|
||||
if got := countAdmins(t); got != 1 {
|
||||
t.Fatalf("ожидалась одна учётная запись, получено %d", got)
|
||||
}
|
||||
|
||||
admin, err := dao.GetAdminUser("1 = 1")
|
||||
if err != nil {
|
||||
t.Fatalf("учётная запись не читается: %v", err)
|
||||
}
|
||||
if *admin.Username != "operator-1" {
|
||||
t.Errorf("имя пользователя не записано: %q", *admin.Username)
|
||||
}
|
||||
if !util.VerifyPassword("recovery-password-1", *admin.PasswordHash) {
|
||||
t.Error("напечатанный пароль не подходит к сохранённому хешу")
|
||||
}
|
||||
if *admin.ForcePasswordChange != 1 {
|
||||
t.Error("восстановительный пароль обязан требовать смены при первом входе")
|
||||
}
|
||||
}
|
||||
|
||||
func TestResetUpdatesExistingAdminInPlace(t *testing.T) {
|
||||
newAdminDB(t)
|
||||
|
||||
if err := resetAdminCredentials(realLookup, "operator-1", "recovery-password-1", 1700000000000); err != nil {
|
||||
t.Fatalf("первичный сброс не выполнен: %v", err)
|
||||
}
|
||||
before, err := dao.GetAdminUser("1 = 1")
|
||||
if err != nil {
|
||||
t.Fatalf("учётная запись не читается: %v", err)
|
||||
}
|
||||
|
||||
if err := resetAdminCredentials(realLookup, "operator-2", "recovery-password-2", 1700000001000); err != nil {
|
||||
t.Fatalf("повторный сброс не выполнен: %v", err)
|
||||
}
|
||||
|
||||
if got := countAdmins(t); got != 1 {
|
||||
t.Fatalf("повторный сброс размножил учётные записи: %d", got)
|
||||
}
|
||||
|
||||
after, err := dao.GetAdminUser("1 = 1")
|
||||
if err != nil {
|
||||
t.Fatalf("учётная запись не читается: %v", err)
|
||||
}
|
||||
if *after.Id != *before.Id {
|
||||
t.Errorf("учётная запись пересоздана: было id=%d, стало id=%d", *before.Id, *after.Id)
|
||||
}
|
||||
if *after.Username != "operator-2" {
|
||||
t.Errorf("имя пользователя не обновлено: %q", *after.Username)
|
||||
}
|
||||
if !util.VerifyPassword("recovery-password-2", *after.PasswordHash) {
|
||||
t.Error("новый пароль не подходит к сохранённому хешу")
|
||||
}
|
||||
if util.VerifyPassword("recovery-password-1", *after.PasswordHash) {
|
||||
t.Error("прежний пароль всё ещё действует")
|
||||
}
|
||||
// Смена пароля обязана обесценивать выданные ранее токены.
|
||||
if *after.TokenVersion <= *before.TokenVersion {
|
||||
t.Errorf("token_version не увеличен: было %d, стало %d", *before.TokenVersion, *after.TokenVersion)
|
||||
}
|
||||
}
|
||||
|
||||
// Ключевая регрессия. База ПОЛНОСТЬЮ работоспособна, отказало только чтение —
|
||||
// ровно тот транзиентный случай («database is locked»), из-за которого прежний
|
||||
// код уходил в ветку создания и оставлял на сервере вторую рабочую учётку с
|
||||
// паролем, уже напечатанным на экран.
|
||||
func TestResetRefusesToCreateSecondAdminOnStorageFailure(t *testing.T) {
|
||||
newAdminDB(t)
|
||||
|
||||
if err := resetAdminCredentials(realLookup, "operator-1", "recovery-password-1", 1700000000000); err != nil {
|
||||
t.Fatalf("первичный сброс не выполнен: %v", err)
|
||||
}
|
||||
before, err := dao.GetAdminUser("1 = 1")
|
||||
if err != nil {
|
||||
t.Fatalf("учётная запись не читается: %v", err)
|
||||
}
|
||||
|
||||
failingLookup := func() (entity.AdminUser, error) {
|
||||
return entity.AdminUser{}, dao.ErrStorage
|
||||
}
|
||||
|
||||
err = resetAdminCredentials(failingLookup, "operator-2", "recovery-password-2", 1700000001000)
|
||||
if err == nil {
|
||||
t.Fatal("отказ хранилища обязан останавливать сброс, а не трактоваться как отсутствие администратора")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "база данных не ответила") {
|
||||
t.Errorf("сообщение не объясняет причину отказа: %v", err)
|
||||
}
|
||||
|
||||
if got := countAdmins(t); got != 1 {
|
||||
t.Fatalf("при отказе чтения создана вторая учётная запись: всего %d", got)
|
||||
}
|
||||
|
||||
after, err := dao.GetAdminUser("1 = 1")
|
||||
if err != nil {
|
||||
t.Fatalf("учётная запись не читается: %v", err)
|
||||
}
|
||||
if *after.Id != *before.Id || *after.Username != *before.Username {
|
||||
t.Error("существующая учётная запись изменена при отказе чтения")
|
||||
}
|
||||
if !util.VerifyPassword("recovery-password-1", *after.PasswordHash) {
|
||||
t.Error("прежний пароль перестал действовать, хотя сброс не выполнялся")
|
||||
}
|
||||
if util.VerifyPassword("recovery-password-2", *after.PasswordHash) {
|
||||
t.Error("напечатанный при отказе пароль действует")
|
||||
}
|
||||
}
|
||||
|
||||
// «Записи нет» по-прежнему означает создание: строгость к ErrStorage не имеет
|
||||
// права сломать штатный путь восстановления на пустой базе.
|
||||
func TestResetStillCreatesOnNotFoundSentinel(t *testing.T) {
|
||||
newAdminDB(t)
|
||||
|
||||
notFound := func() (entity.AdminUser, error) {
|
||||
return entity.AdminUser{}, dao.ErrAdminUserNotFound
|
||||
}
|
||||
|
||||
if err := resetAdminCredentials(notFound, "operator-1", "recovery-password-1", 1700000000000); err != nil {
|
||||
t.Fatalf("сброс на пустой базе не выполнен: %v", err)
|
||||
}
|
||||
if got := countAdmins(t); got != 1 {
|
||||
t.Fatalf("ожидалась одна учётная запись, получено %d", got)
|
||||
}
|
||||
}
|
||||
|
||||
// Sentinel'ы «нет записи» намеренно НЕСУТ ОДИНАКОВЫЙ ТЕКСТ: WrongPassword
|
||||
// уезжает в ответ Hysteria при неудачной machine-auth, и менять его ради
|
||||
// внутренней аккуратности было бы изменением внешнего контракта. Поэтому
|
||||
// различать их можно только через errors.Is, и решение о ветке обязано
|
||||
// опираться на идентичность значения, а не на строку.
|
||||
func TestAdminSentinelsAreDistinguishableOnlyByIdentity(t *testing.T) {
|
||||
if dao.ErrAdminUserNotFound.Error() != dao.ErrPeerNotFound.Error() {
|
||||
t.Log("тексты sentinel'ов разошлись; сравнение по идентичности остаётся обязательным")
|
||||
}
|
||||
if errors.Is(dao.ErrAdminUserNotFound, dao.ErrPeerNotFound) {
|
||||
t.Error("sentinel'ы разных таблиц неразличимы по идентичности")
|
||||
}
|
||||
if errors.Is(dao.ErrStorage, dao.ErrAdminUserNotFound) {
|
||||
t.Error("отказ хранилища опознаётся как отсутствие записи")
|
||||
}
|
||||
if dao.IsNotFound(dao.ErrStorage) {
|
||||
t.Error("IsNotFound истинна для отказа хранилища")
|
||||
}
|
||||
}
|
||||
|
||||
// Ошибка хеширования не имеет права превратиться в пустой password_hash.
|
||||
func TestResetRefusesWhenPasswordCannotBeHashed(t *testing.T) {
|
||||
newAdminDB(t)
|
||||
|
||||
if err := resetAdminCredentials(realLookup, "operator-1", "recovery-password-1", 1700000000000); err != nil {
|
||||
t.Fatalf("первичный сброс не выполнен: %v", err)
|
||||
}
|
||||
|
||||
// HashPassword отклоняет пароль короче шести символов.
|
||||
err := resetAdminCredentials(realLookup, "operator-2", "abc", 1700000001000)
|
||||
if err == nil {
|
||||
t.Fatal("непригодный пароль обязан останавливать сброс")
|
||||
}
|
||||
|
||||
admin, getErr := dao.GetAdminUser("1 = 1")
|
||||
if getErr != nil {
|
||||
t.Fatalf("учётная запись не читается: %v", getErr)
|
||||
}
|
||||
if !util.IsBcryptHash(*admin.PasswordHash) {
|
||||
t.Errorf("в password_hash оказалась не-bcrypt строка: %q", *admin.PasswordHash)
|
||||
}
|
||||
if !util.VerifyPassword("recovery-password-1", *admin.PasswordHash) {
|
||||
t.Error("прежний пароль перестал действовать после неудачного сброса")
|
||||
}
|
||||
}
|
||||
@@ -20,6 +20,22 @@ func GetAdminUser(query interface{}, args ...interface{}) (entity.AdminUser, err
|
||||
return admin, nil
|
||||
}
|
||||
|
||||
// CountAdminUsers — сколько учётных записей администратора существует.
|
||||
//
|
||||
// Продукт допускает ровно одну, и это ЕДИНСТВЕННОЕ место, где такой вопрос
|
||||
// можно задать: `GetAdminUser` берёт First() и о наличии второй строки не
|
||||
// сообщает. Именно поэтому появление дубликата (ветка создания, выбранная при
|
||||
// отказе чтения) было ненаблюдаемым — вторая рабочая учётка с уже напечатанным
|
||||
// на экран паролем просто существовала, и никакой запрос об этом не говорил.
|
||||
func CountAdminUsers() (int64, error) {
|
||||
var count int64
|
||||
if tx := sqliteDB.Model(&entity.AdminUser{}).Count(&count); tx.Error != nil {
|
||||
logrus.Errorf("%v", tx.Error)
|
||||
return 0, ErrStorage
|
||||
}
|
||||
return count, nil
|
||||
}
|
||||
|
||||
func SaveAdminUser(admin entity.AdminUser) (int64, error) {
|
||||
if tx := sqliteDB.Save(&admin); tx.Error != nil {
|
||||
logrus.Errorf("%v", tx.Error)
|
||||
|
||||
Reference in New Issue
Block a user