Commit b383802109

b383802109ad3d655852d929816243fcf59209ed

parent: 884a5c558b

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-28 22:29 UTC

backup: remove a killed run's leftovers; refuse an archive inside server.root

Ref #259

Layout: unified · split

.gitbay/wiki/Admin.org +5 −1
@@ -444,7 +444,11 @@ One archive: a consistent SQLite snapshot (taken *before* the
444repositories are read, so the database never references objects the 444repositories are read, so the database never references objects the
445archive missed), every repository, and the SSH host keys. Excluded: 445archive missed), every repository, and the SSH host keys. Excluded:
446hook socket, regenerated hook scripts, WAL files. Safe to run against a 446hook socket, regenerated hook scripts, WAL files. Safe to run against a
447live daemon. 447live daemon. =--out= must be outside =server.root=, or the next full
448backup would carry the archive. Each run removes the snapshot
449directories (=.gitbay-snap-*=) and temporary archives (=.*.tmp-*=) a
450killed run left beside its archive once they are a day old, and prints
451each one it removes.
448 452
449=--verify= reads an archive back: the snapshot must pass SQLite's 453=--verify= reads an archive back: the snapshot must pass SQLite's
450integrity check, every repository the snapshot names must be in the 454integrity check, every repository the snapshot names must be in the
cmd/gitbayd/backup.go +39 −1
@@ -97,6 +97,13 @@ func runBackup(cfg config.Config, out string, dbOnly bool) error {
97 return fmt.Errorf("%s ends in .age but [backup] age_recipients is not set, so the archive would not be encrypted", out) 97 return fmt.Errorf("%s ends in .age but [backup] age_recipients is not set, so the archive would not be encrypted", out)
98 } 98 }
99 99
100 dir := filepath.Dir(out)
101 // An archive under the root would be in the next full backup's walk.
102 if config.Within(cfg.Server.Root, dir) {
103 return fmt.Errorf("%s is inside server.root %s; write the archive elsewhere", out, cfg.Server.Root)
104 }
105 removeStale(dir, time.Now().Add(-staleAge))
106
100 // Deletes, renames and transfers wait until the walk finishes, so 107 // Deletes, renames and transfers wait until the walk finishes, so
101 // every repository the snapshot names is still on disk when the walk 108 // every repository the snapshot names is still on disk when the walk
102 // reaches it (#259). A database-only archive reads no repository. 109 // reaches it (#259). A database-only archive reads no repository.
@@ -123,7 +130,6 @@ func runBackup(cfg config.Config, out string, dbOnly bool) error {
123 130
124 // 1. Consistent database snapshot, before any repository is read. It 131 // 1. Consistent database snapshot, before any repository is read. It
125 // goes in a fresh 0700 directory beside the archive. 132 // goes in a fresh 0700 directory beside the archive.
126 dir := filepath.Dir(out)
127 snapDir, err := os.MkdirTemp(dir, ".gitbay-snap-") 133 snapDir, err := os.MkdirTemp(dir, ".gitbay-snap-")
128 if err != nil { 134 if err != nil {
129 return err 135 return err
@@ -266,6 +272,38 @@ func runBackup(cfg config.Config, out string, dbOnly bool) error {
266 return nil 272 return nil
267} 273}
268 274
275// staleAge is how old a snapshot directory or temporary archive must be
276// before a later run removes it. A run that is still writing one is
277// younger than this.
278const staleAge = 24 * time.Hour
279
280// removeStale removes what a killed run left in dir: snapshot
281// directories and temporary archives last modified before cutoff.
282func removeStale(dir string, cutoff time.Time) {
283 ents, err := os.ReadDir(dir)
284 if err != nil {
285 return
286 }
287 for _, e := range ents {
288 name := e.Name()
289 snap := e.IsDir() && strings.HasPrefix(name, ".gitbay-snap-")
290 tmp := e.Type().IsRegular() && strings.HasPrefix(name, ".") && strings.Contains(name, ".tmp-")
291 if !snap && !tmp {
292 continue
293 }
294 info, err := e.Info()
295 if err != nil || !info.ModTime().Before(cutoff) {
296 continue
297 }
298 p := filepath.Join(dir, name)
299 if err := os.RemoveAll(p); err != nil {
300 fmt.Fprintf(os.Stderr, "removing stale %s: %v\n", p, err)
301 continue
302 }
303 fmt.Fprintf(os.Stderr, "removed stale %s\n", p)
304 }
305}
306
269// refNames are what a repository's refs are read from. WalkDir would 307// refNames are what a repository's refs are read from. WalkDir would
270// reach objects/ before packed-refs and refs/, so a push landing mid-walk 308// reach objects/ before packed-refs and refs/, so a push landing mid-walk
271// could leave an archived ref naming objects the archive lacks. addRefs 309// could leave an archived ref naming objects the archive lacks. addRefs
cmd/gitbayd/backup_test.go +64
@@ -653,3 +653,67 @@ func TestBackupNeedsNoKeyFile(t *testing.T) {
653 t.Fatalf("verify without the key file: %v", err) 653 t.Fatalf("verify without the key file: %v", err)
654 } 654 }
655} 655}
656
657// A run removes what a killed run left beside the archive once it is a
658// day old, and leaves younger ones, which may belong to a run under way.
659func TestBackupRemovesStaleTemporaries(t *testing.T) {
660 cfg := testConfig(t)
661 s, err := openStore(cfg)
662 if err != nil {
663 t.Fatal(err)
664 }
665 s.Close()
666 dir := t.TempDir()
667 old := time.Now().Add(-25 * time.Hour)
668 mk := func(name string, isDir bool, mtime time.Time) {
669 p := filepath.Join(dir, name)
670 if isDir {
671 if err := os.Mkdir(p, 0o700); err != nil {
672 t.Fatal(err)
673 }
674 } else if err := os.WriteFile(p, []byte("x"), 0o600); err != nil {
675 t.Fatal(err)
676 }
677 if err := os.Chtimes(p, mtime, mtime); err != nil {
678 t.Fatal(err)
679 }
680 }
681 mk(".gitbay-snap-old", true, old)
682 mk(".b.tar.gz.tmp-old", false, old)
683 mk(".gitbay-snap-new", true, time.Now())
684 mk(".b.tar.gz.tmp-new", false, time.Now())
685 mk(".keep", false, old)
686 if err := runBackup(cfg, filepath.Join(dir, "b.tar.gz"), true); err != nil {
687 t.Fatal(err)
688 }
689 got := leftovers(t, dir)
690 want := []string{".b.tar.gz.tmp-new", ".gitbay-snap-new", ".keep"}
691 sort.Strings(got)
692 if strings.Join(got, " ") != strings.Join(want, " ") {
693 t.Errorf("left %v, want %v", got, want)
694 }
695}
696
697// An archive written under server.root, directly or through a symlink,
698// would be in the next full backup, so it is refused.
699func TestBackupRefusesOutputInsideRoot(t *testing.T) {
700 cfg := testConfig(t)
701 s, err := openStore(cfg)
702 if err != nil {
703 t.Fatal(err)
704 }
705 s.Close()
706 link := filepath.Join(t.TempDir(), "link")
707 if err := os.Symlink(cfg.Server.Root, link); err != nil {
708 t.Fatal(err)
709 }
710 for _, out := range []string{
711 filepath.Join(cfg.Server.Root, "b.tar.gz"),
712 filepath.Join(cfg.Server.Root, "backups", "b.tar.gz"),
713 filepath.Join(link, "b.tar.gz"),
714 } {
715 if err := runBackup(cfg, out, true); err == nil || !strings.Contains(err.Error(), "inside server.root") {
716 t.Errorf("%s: %v", out, err)
717 }
718 }
719}
internal/config/config.go +3 −3
@@ -359,12 +359,12 @@ func Load(path string) (Config, error) {
359 return cfg, cfg.Validate() 359 return cfg, cfg.Validate()
360} 360}
361 361
362// within reports whether path is dir or below it. Both are compared as 362// Within reports whether path is dir or below it. Both are compared as
363// cleaned absolute paths (a relative path resolves against the working 363// cleaned absolute paths (a relative path resolves against the working
364// directory, same as every other path in this config), with symlinks 364// directory, same as every other path in this config), with symlinks
365// resolved where the path exists on disk, so a path that reaches into dir 365// resolved where the path exists on disk, so a path that reaches into dir
366// through a symlink, or through "..", is still reported as inside. 366// through a symlink, or through "..", is still reported as inside.
367func within(dir, path string) bool { 367func Within(dir, path string) bool {
368 dir, path = resolvePath(dir), resolvePath(path) 368 dir, path = resolvePath(dir), resolvePath(path)
369 rel, err := filepath.Rel(dir, path) 369 rel, err := filepath.Rel(dir, path)
370 return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) 370 return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
@@ -430,7 +430,7 @@ func (c Config) Validate() error {
430 switch { 430 switch {
431 case c.Server.SecretKeyFile == "": 431 case c.Server.SecretKeyFile == "":
432 errs = append(errs, errors.New("server.secret_key_file is required")) 432 errs = append(errs, errors.New("server.secret_key_file is required"))
433 case within(c.Server.Root, c.Server.SecretKeyFile): 433 case Within(c.Server.Root, c.Server.SecretKeyFile):
434 errs = append(errs, fmt.Errorf("server.secret_key_file %q is inside server.root: backups of the root would carry the key beside the values it seals", c.Server.SecretKeyFile)) 434 errs = append(errs, fmt.Errorf("server.secret_key_file %q is inside server.root: backups of the root would carry the key beside the values it seals", c.Server.SecretKeyFile))
435 } 435 }
436 if err := oneOf("ssh.mode", c.SSH.Mode, "embedded", "system"); err != nil { 436 if err := oneOf("ssh.mode", c.SSH.Mode, "embedded", "system"); err != nil {