fix: validate numeric timestamps in backup filenames and use nanosecond precision
- Backup suffix now validated with strconv.ParseInt to reject non-numeric files - Sort by parsed timestamp instead of lexicographic string comparison - Use UnixNano() instead of Unix() to prevent collisions on rapid saves - Test asserts exact backup count (5) instead of permissive range
- commit
- d1dc76326f378af8cb04309e95d645c9866343f8
- author
- brice · · 2026-03-14 05:42 UTC
- parent
- 3b9844b
- signature
- unsigned
2 files · +39 -17
M internal/usecase/config/service.go +37 -14
| @@ -9,6 +9,7 @@ import ( | |||
| 9 | 9 | "path/filepath" | |
| 10 | 10 | "reflect" | |
| 11 | 11 | "slices" | |
| 12 | + | "strconv" | |
| 12 | 13 | "strings" | |
| 13 | 14 | "sync" | |
| 14 | 15 | "sync/atomic" | |
| @@ -575,7 +576,7 @@ func backupConfigFile(configFile string) error { | |||
| 575 | 576 | return fmt.Errorf("failed to read config for backup: %w", err) | |
| 576 | 577 | } | |
| 577 | 578 | ||
| 578 | - | backupPath := fmt.Sprintf("%s.bak.%d", configFile, time.Now().Unix()) | |
| 579 | + | backupPath := fmt.Sprintf("%s.bak.%d", configFile, time.Now().UnixNano()) | |
| 579 | 580 | if err := os.WriteFile(backupPath, src, 0600); err != nil { | |
| 580 | 581 | return fmt.Errorf("failed to write backup config: %w", err) | |
| 581 | 582 | } | |
| @@ -587,7 +588,7 @@ func backupConfigFile(configFile string) error { | |||
| 587 | 588 | } | |
| 588 | 589 | ||
| 589 | 590 | // cleanupOldBackups removes old backup files, keeping only the most recent `keep` backups. | |
| 590 | - | // It only removes files matching the pattern `<configFile>.bak.<timestamp>`. | |
| 591 | + | // It only removes files matching the pattern `<configFile>.bak.<numeric-timestamp>`. | |
| 591 | 592 | func cleanupOldBackups(configFile string, keep int) error { | |
| 592 | 593 | dir := filepath.Dir(configFile) | |
| 593 | 594 | base := filepath.Base(configFile) | |
| @@ -597,18 +598,29 @@ func cleanupOldBackups(configFile string, keep int) error { | |||
| 597 | 598 | } | |
| 598 | 599 | ||
| 599 | 600 | prefix := base + ".bak." | |
| 600 | - | backups := make([]string, 0) | |
| 601 | + | type tsBackup struct { | |
| 602 | + | ts int64 | |
| 603 | + | name string | |
| 604 | + | } | |
| 605 | + | var backups []tsBackup | |
| 601 | 606 | for _, entry := range entries { | |
| 602 | 607 | if entry.IsDir() { | |
| 603 | 608 | continue | |
| 604 | 609 | } | |
| 605 | - | if strings.HasPrefix(entry.Name(), prefix) { | |
| 606 | - | backups = append(backups, entry.Name()) | |
| 610 | + | name := entry.Name() | |
| 611 | + | if !strings.HasPrefix(name, prefix) { | |
| 612 | + | continue | |
| 613 | + | } | |
| 614 | + | suffix := strings.TrimPrefix(name, prefix) | |
| 615 | + | ts, err := strconv.ParseInt(suffix, 10, 64) | |
| 616 | + | if err != nil { | |
| 617 | + | continue | |
| 607 | 618 | } | |
| 619 | + | backups = append(backups, tsBackup{ts: ts, name: name}) | |
| 608 | 620 | } | |
| 609 | 621 | ||
| 610 | - | slices.SortFunc(backups, func(a, b string) int { | |
| 611 | - | return cmp.Compare(b, a) | |
| 622 | + | slices.SortFunc(backups, func(a, b tsBackup) int { | |
| 623 | + | return cmp.Compare(b.ts, a.ts) | |
| 612 | 624 | }) | |
| 613 | 625 | if len(backups) <= keep { | |
| 614 | 626 | return nil | |
| @@ -616,7 +628,7 @@ func cleanupOldBackups(configFile string, keep int) error { | |||
| 616 | 628 | ||
| 617 | 629 | var firstErr error | |
| 618 | 630 | for _, backup := range backups[keep:] { | |
| 619 | - | backupPath := filepath.Join(dir, backup) | |
| 631 | + | backupPath := filepath.Join(dir, backup.name) | |
| 620 | 632 | if err := os.Remove(backupPath); err != nil && firstErr == nil { | |
| 621 | 633 | firstErr = fmt.Errorf("failed to remove old backup %s: %w", backupPath, err) | |
| 622 | 634 | } | |
| @@ -634,23 +646,34 @@ func restoreConfigBackup(configFile string) error { | |||
| 634 | 646 | } | |
| 635 | 647 | ||
| 636 | 648 | prefix := base + ".bak." | |
| 637 | - | backups := make([]string, 0) | |
| 649 | + | type tsBackup struct { | |
| 650 | + | ts int64 | |
| 651 | + | name string | |
| 652 | + | } | |
| 653 | + | var backups []tsBackup | |
| 638 | 654 | for _, entry := range entries { | |
| 639 | 655 | if entry.IsDir() { | |
| 640 | 656 | continue | |
| 641 | 657 | } | |
| 642 | - | if strings.HasPrefix(entry.Name(), prefix) { | |
| 643 | - | backups = append(backups, entry.Name()) | |
| 658 | + | name := entry.Name() | |
| 659 | + | if !strings.HasPrefix(name, prefix) { | |
| 660 | + | continue | |
| 661 | + | } | |
| 662 | + | suffix := strings.TrimPrefix(name, prefix) | |
| 663 | + | ts, err := strconv.ParseInt(suffix, 10, 64) | |
| 664 | + | if err != nil { | |
| 665 | + | continue | |
| 644 | 666 | } | |
| 667 | + | backups = append(backups, tsBackup{ts: ts, name: name}) | |
| 645 | 668 | } | |
| 646 | 669 | ||
| 647 | - | slices.SortFunc(backups, func(a, b string) int { | |
| 648 | - | return cmp.Compare(b, a) | |
| 670 | + | slices.SortFunc(backups, func(a, b tsBackup) int { | |
| 671 | + | return cmp.Compare(b.ts, a.ts) | |
| 649 | 672 | }) | |
| 650 | 673 | ||
| 651 | 674 | backupPath := configFile + ".bak" | |
| 652 | 675 | if len(backups) > 0 { | |
| 653 | - | backupPath = filepath.Join(dir, backups[0]) | |
| 676 | + | backupPath = filepath.Join(dir, backups[0].name) | |
| 654 | 677 | } | |
| 655 | 678 | ||
| 656 | 679 | src, err := os.ReadFile(backupPath) | |
| @@ -9,6 +9,7 @@ import ( | |||||
| 9 | "path/filepath" | 9 | "path/filepath" | ||
| 10 | "reflect" | 10 | "reflect" | ||
| 11 | "slices" | 11 | "slices" | ||
| 12 | + | "strconv" | |||
| 12 | "strings" | 13 | "strings" | ||
| 13 | "sync" | 14 | "sync" | ||
| 14 | "sync/atomic" | 15 | "sync/atomic" | ||
| @@ -575,7 +576,7 @@ func backupConfigFile(configFile string) error { | |||||
| 575 | return fmt.Errorf("failed to read config for backup: %w", err) | 576 | return fmt.Errorf("failed to read config for backup: %w", err) | ||
| 576 | } | 577 | } | ||
| 577 | 578 | ||||
| 578 | - | backupPath := fmt.Sprintf("%s.bak.%d", configFile, time.Now().Unix()) | 579 | + | backupPath := fmt.Sprintf("%s.bak.%d", configFile, time.Now().UnixNano()) |
| 579 | if err := os.WriteFile(backupPath, src, 0600); err != nil { | 580 | if err := os.WriteFile(backupPath, src, 0600); err != nil { | ||
| 580 | return fmt.Errorf("failed to write backup config: %w", err) | 581 | return fmt.Errorf("failed to write backup config: %w", err) | ||
| 581 | } | 582 | } | ||
| @@ -587,7 +588,7 @@ func backupConfigFile(configFile string) error { | |||||
| 587 | } | 588 | } | ||
| 588 | 589 | ||||
| 589 | // cleanupOldBackups removes old backup files, keeping only the most recent `keep` backups. | 590 | // cleanupOldBackups removes old backup files, keeping only the most recent `keep` backups. | ||
| 590 | - | // It only removes files matching the pattern `<configFile>.bak.<timestamp>`. | 591 | + | // It only removes files matching the pattern `<configFile>.bak.<numeric-timestamp>`. |
| 591 | func cleanupOldBackups(configFile string, keep int) error { | 592 | func cleanupOldBackups(configFile string, keep int) error { | ||
| 592 | dir := filepath.Dir(configFile) | 593 | dir := filepath.Dir(configFile) | ||
| 593 | base := filepath.Base(configFile) | 594 | base := filepath.Base(configFile) | ||
| @@ -597,18 +598,29 @@ func cleanupOldBackups(configFile string, keep int) error { | |||||
| 597 | } | 598 | } | ||
| 598 | 599 | ||||
| 599 | prefix := base + ".bak." | 600 | prefix := base + ".bak." | ||
| 600 | - | backups := make([]string, 0) | 601 | + | type tsBackup struct { |
| 602 | + | ts int64 | |||
| 603 | + | name string | |||
| 604 | + | } | |||
| 605 | + | var backups []tsBackup | |||
| 601 | for _, entry := range entries { | 606 | for _, entry := range entries { | ||
| 602 | if entry.IsDir() { | 607 | if entry.IsDir() { | ||
| 603 | continue | 608 | continue | ||
| 604 | } | 609 | } | ||
| 605 | - | if strings.HasPrefix(entry.Name(), prefix) { | 610 | + | name := entry.Name() |
| 606 | - | backups = append(backups, entry.Name()) | 611 | + | if !strings.HasPrefix(name, prefix) { |
| 612 | + | continue | |||
| 613 | + | } | |||
| 614 | + | suffix := strings.TrimPrefix(name, prefix) | |||
| 615 | + | ts, err := strconv.ParseInt(suffix, 10, 64) | |||
| 616 | + | if err != nil { | |||
| 617 | + | continue | |||
| 607 | } | 618 | } | ||
| 619 | + | backups = append(backups, tsBackup{ts: ts, name: name}) | |||
| 608 | } | 620 | } | ||
| 609 | 621 | ||||
| 610 | - | slices.SortFunc(backups, func(a, b string) int { | 622 | + | slices.SortFunc(backups, func(a, b tsBackup) int { |
| 611 | - | return cmp.Compare(b, a) | 623 | + | return cmp.Compare(b.ts, a.ts) |
| 612 | }) | 624 | }) | ||
| 613 | if len(backups) <= keep { | 625 | if len(backups) <= keep { | ||
| 614 | return nil | 626 | return nil | ||
| @@ -616,7 +628,7 @@ func cleanupOldBackups(configFile string, keep int) error { | |||||
| 616 | 628 | ||||
| 617 | var firstErr error | 629 | var firstErr error | ||
| 618 | for _, backup := range backups[keep:] { | 630 | for _, backup := range backups[keep:] { | ||
| 619 | - | backupPath := filepath.Join(dir, backup) | 631 | + | backupPath := filepath.Join(dir, backup.name) |
| 620 | if err := os.Remove(backupPath); err != nil && firstErr == nil { | 632 | if err := os.Remove(backupPath); err != nil && firstErr == nil { | ||
| 621 | firstErr = fmt.Errorf("failed to remove old backup %s: %w", backupPath, err) | 633 | firstErr = fmt.Errorf("failed to remove old backup %s: %w", backupPath, err) | ||
| 622 | } | 634 | } | ||
| @@ -634,23 +646,34 @@ func restoreConfigBackup(configFile string) error { | |||||
| 634 | } | 646 | } | ||
| 635 | 647 | ||||
| 636 | prefix := base + ".bak." | 648 | prefix := base + ".bak." | ||
| 637 | - | backups := make([]string, 0) | 649 | + | type tsBackup struct { |
| 650 | + | ts int64 | |||
| 651 | + | name string | |||
| 652 | + | } | |||
| 653 | + | var backups []tsBackup | |||
| 638 | for _, entry := range entries { | 654 | for _, entry := range entries { | ||
| 639 | if entry.IsDir() { | 655 | if entry.IsDir() { | ||
| 640 | continue | 656 | continue | ||
| 641 | } | 657 | } | ||
| 642 | - | if strings.HasPrefix(entry.Name(), prefix) { | 658 | + | name := entry.Name() |
| 643 | - | backups = append(backups, entry.Name()) | 659 | + | if !strings.HasPrefix(name, prefix) { |
| 660 | + | continue | |||
| 661 | + | } | |||
| 662 | + | suffix := strings.TrimPrefix(name, prefix) | |||
| 663 | + | ts, err := strconv.ParseInt(suffix, 10, 64) | |||
| 664 | + | if err != nil { | |||
| 665 | + | continue | |||
| 644 | } | 666 | } | ||
| 667 | + | backups = append(backups, tsBackup{ts: ts, name: name}) | |||
| 645 | } | 668 | } | ||
| 646 | 669 | ||||
| 647 | - | slices.SortFunc(backups, func(a, b string) int { | 670 | + | slices.SortFunc(backups, func(a, b tsBackup) int { |
| 648 | - | return cmp.Compare(b, a) | 671 | + | return cmp.Compare(b.ts, a.ts) |
| 649 | }) | 672 | }) | ||
| 650 | 673 | ||||
| 651 | backupPath := configFile + ".bak" | 674 | backupPath := configFile + ".bak" | ||
| 652 | if len(backups) > 0 { | 675 | if len(backups) > 0 { | ||
| 653 | - | backupPath = filepath.Join(dir, backups[0]) | 676 | + | backupPath = filepath.Join(dir, backups[0].name) |
| 654 | } | 677 | } | ||
| 655 | 678 | ||||
| 656 | src, err := os.ReadFile(backupPath) | 679 | src, err := os.ReadFile(backupPath) | ||
M internal/usecase/config/service_test.go +2 -3
| @@ -1304,7 +1304,7 @@ port = 9999 | |||
| 1304 | 1304 | err := os.WriteFile(configFile, []byte(initialConfig), 0600) | |
| 1305 | 1305 | require.NoError(t, err) | |
| 1306 | 1306 | ||
| 1307 | - | for i := 1000; i < 1007; i++ { | |
| 1307 | + | for i := 1000000000000; i < 1000000000007; i++ { | |
| 1308 | 1308 | backupPath := fmt.Sprintf("%s.bak.%d", configFile, i) | |
| 1309 | 1309 | err := os.WriteFile(backupPath, []byte("old backup"), 0600) | |
| 1310 | 1310 | require.NoError(t, err) | |
| @@ -1326,8 +1326,7 @@ port = 9999 | |||
| 1326 | 1326 | ||
| 1327 | 1327 | matches, err := filepath.Glob(configFile + ".bak.*") | |
| 1328 | 1328 | require.NoError(t, err) | |
| 1329 | - | assert.LessOrEqual(t, len(matches), 5) | |
| 1330 | - | assert.GreaterOrEqual(t, len(matches), 1) | |
| 1329 | + | assert.Equal(t, 5, len(matches)) | |
| 1331 | 1330 | }) | |
| 1332 | 1331 | ||
| 1333 | 1332 | t.Run("network_groups are persisted to disk", func(t *testing.T) { | |
| @@ -1304,7 +1304,7 @@ port = 9999 | |||||
| 1304 | err := os.WriteFile(configFile, []byte(initialConfig), 0600) | 1304 | err := os.WriteFile(configFile, []byte(initialConfig), 0600) | ||
| 1305 | require.NoError(t, err) | 1305 | require.NoError(t, err) | ||
| 1306 | 1306 | ||||
| 1307 | - | for i := 1000; i < 1007; i++ { | 1307 | + | for i := 1000000000000; i < 1000000000007; i++ { |
| 1308 | backupPath := fmt.Sprintf("%s.bak.%d", configFile, i) | 1308 | backupPath := fmt.Sprintf("%s.bak.%d", configFile, i) | ||
| 1309 | err := os.WriteFile(backupPath, []byte("old backup"), 0600) | 1309 | err := os.WriteFile(backupPath, []byte("old backup"), 0600) | ||
| 1310 | require.NoError(t, err) | 1310 | require.NoError(t, err) | ||
| @@ -1326,8 +1326,7 @@ port = 9999 | |||||
| 1326 | 1326 | ||||
| 1327 | matches, err := filepath.Glob(configFile + ".bak.*") | 1327 | matches, err := filepath.Glob(configFile + ".bak.*") | ||
| 1328 | require.NoError(t, err) | 1328 | require.NoError(t, err) | ||
| 1329 | - | assert.LessOrEqual(t, len(matches), 5) | 1329 | + | assert.Equal(t, 5, len(matches)) |
| 1330 | - | assert.GreaterOrEqual(t, len(matches), 1) | |||
| 1331 | }) | 1330 | }) | ||
| 1332 | 1331 | ||||
| 1333 | t.Run("network_groups are persisted to disk", func(t *testing.T) { | 1332 | t.Run("network_groups are persisted to disk", func(t *testing.T) { | ||