fix: verify all value types and handle reload error after restore
Address CodeRabbit findings: - verifyCriticalFields now uses reflect.DeepEqual to catch corruption of booleans, integers, and other non-string fields - ReadInConfig error after backup restore is now logged instead of silenced
- commit
- 7d7aff9589ffe1e748e5e5f495e3acaf8ea74ccb
- author
- brice · · 2026-03-14 05:02 UTC
- parent
- 460e4c5
- signature
- unsigned
1 file · +7 -7
M internal/usecase/config/service.go +7 -7
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "context" | |
| 6 | 6 | "fmt" | |
| 7 | 7 | "os" | |
| 8 | + | "reflect" | |
| 8 | 9 | "strings" | |
| 9 | 10 | "sync" | |
| 10 | 11 | "sync/atomic" | |
| @@ -481,8 +482,9 @@ func (s *Service) Save(ctx context.Context) error { | |||
| 481 | 482 | log.Error().Err(err).Msg("config corruption detected after save, restoring backup") | |
| 482 | 483 | if restoreErr := restoreConfigBackup(configFile); restoreErr != nil { | |
| 483 | 484 | log.Error().Err(restoreErr).Msg("CRITICAL: failed to restore config backup") | |
| 485 | + | } else if reloadErr := s.viper.ReadInConfig(); reloadErr != nil { | |
| 486 | + | log.Error().Err(reloadErr).Msg("failed to reload config after restore") | |
| 484 | 487 | } | |
| 485 | - | _ = s.viper.ReadInConfig() | |
| 486 | 488 | return log.WrapErr(err, "config verification failed after save") | |
| 487 | 489 | } | |
| 488 | 490 | ||
| @@ -504,14 +506,12 @@ func (s *Service) snapshotCriticalFields() configSnapshot { | |||
| 504 | 506 | ||
| 505 | 507 | func (s *Service) verifyCriticalFields(snap configSnapshot) error { | |
| 506 | 508 | for key, oldVal := range snap.values { | |
| 507 | - | oldStr, ok := oldVal.(string) | |
| 508 | - | if !ok || oldStr == "" { | |
| 509 | + | if oldVal == nil { | |
| 509 | 510 | continue | |
| 510 | 511 | } | |
| 511 | - | ||
| 512 | - | newVal := s.viper.GetString(key) | |
| 513 | - | if newVal != oldStr { | |
| 514 | - | return fmt.Errorf("config key %q changed from %q to %q after save", key, oldStr, newVal) | |
| 512 | + | newVal := s.viper.Get(key) | |
| 513 | + | if !reflect.DeepEqual(oldVal, newVal) { | |
| 514 | + | return fmt.Errorf("config key %q changed from %v to %v after save", key, oldVal, newVal) | |
| 515 | 515 | } | |
| 516 | 516 | } | |
| 517 | 517 | ||
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "context" | 5 | "context" | ||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | "os" | 7 | "os" | ||
| 8 | + | "reflect" | |||
| 8 | "strings" | 9 | "strings" | ||
| 9 | "sync" | 10 | "sync" | ||
| 10 | "sync/atomic" | 11 | "sync/atomic" | ||
| @@ -481,8 +482,9 @@ func (s *Service) Save(ctx context.Context) error { | |||||
| 481 | log.Error().Err(err).Msg("config corruption detected after save, restoring backup") | 482 | log.Error().Err(err).Msg("config corruption detected after save, restoring backup") | ||
| 482 | if restoreErr := restoreConfigBackup(configFile); restoreErr != nil { | 483 | if restoreErr := restoreConfigBackup(configFile); restoreErr != nil { | ||
| 483 | log.Error().Err(restoreErr).Msg("CRITICAL: failed to restore config backup") | 484 | log.Error().Err(restoreErr).Msg("CRITICAL: failed to restore config backup") | ||
| 485 | + | } else if reloadErr := s.viper.ReadInConfig(); reloadErr != nil { | |||
| 486 | + | log.Error().Err(reloadErr).Msg("failed to reload config after restore") | |||
| 484 | } | 487 | } | ||
| 485 | - | _ = s.viper.ReadInConfig() | |||
| 486 | return log.WrapErr(err, "config verification failed after save") | 488 | return log.WrapErr(err, "config verification failed after save") | ||
| 487 | } | 489 | } | ||
| 488 | 490 | ||||
| @@ -504,14 +506,12 @@ func (s *Service) snapshotCriticalFields() configSnapshot { | |||||
| 504 | 506 | ||||
| 505 | func (s *Service) verifyCriticalFields(snap configSnapshot) error { | 507 | func (s *Service) verifyCriticalFields(snap configSnapshot) error { | ||
| 506 | for key, oldVal := range snap.values { | 508 | for key, oldVal := range snap.values { | ||
| 507 | - | oldStr, ok := oldVal.(string) | 509 | + | if oldVal == nil { |
| 508 | - | if !ok || oldStr == "" { | |||
| 509 | continue | 510 | continue | ||
| 510 | } | 511 | } | ||
| 511 | - | 512 | + | newVal := s.viper.Get(key) | |
| 512 | - | newVal := s.viper.GetString(key) | 513 | + | if !reflect.DeepEqual(oldVal, newVal) { |
| 513 | - | if newVal != oldStr { | 514 | + | return fmt.Errorf("config key %q changed from %v to %v after save", key, oldVal, newVal) |
| 514 | - | return fmt.Errorf("config key %q changed from %q to %q after save", key, oldStr, newVal) | |||
| 515 | } | 515 | } | ||
| 516 | } | 516 | } | ||
| 517 | 517 | ||||