Commit 72d22fe4f7
Verified · cmc
Layout: unified · split
.gitbay/wiki/Admin.org +7 −3
| @@ -467,9 +467,13 @@ automatic gc removes during the walk is skipped; each repository's refs | |||
| 467 | are archived before its objects, so the refs still find their objects, | 467 | are archived before its objects, so the refs still find their objects, |
| 468 | and =--verify= reports it if one does not. | 468 | and =--verify= reports it if one does not. |
| 469 | 469 | ||
| 470 | Verifying an archive of unknown origin is safe as an unprivileged | 470 | Verifying an archive of unknown origin: run it as an unprivileged |
| 471 | user: the connectivity check runs git against the archived | 471 | user. The connectivity check runs git with =--git-dir= on each |
| 472 | repositories' own extracted copies, nothing under =server.root=. | 472 | extracted repository, so a directory that is not a repository fails |
| 473 | rather than git checking an enclosing one, and =objects/info/alternates= | ||
| 474 | members are not extracted, so an archive cannot point git at object | ||
| 475 | stores elsewhere on the host. Git still reads each archived | ||
| 476 | repository's own =config=. | ||
| 473 | 477 | ||
| 474 | Archives carry a directory entry for every directory, including an | 478 | Archives carry a directory entry for every directory, including an |
| 475 | empty one, so a bare repository whose refs are all packed restores as | 479 | empty one, so a bare repository whose refs are all packed restores as |
cmd/gitbayd/backup.go +14
| @@ -489,6 +489,9 @@ func verifyBackup(path, identity string) error { | |||
| 489 | if !filepath.IsLocal(trimmed) { | 489 | if !filepath.IsLocal(trimmed) { |
| 490 | return fmt.Errorf("%s: member %q leaves the archive root", path, h.Name) | 490 | return fmt.Errorf("%s: member %q leaves the archive root", path, h.Name) |
| 491 | } | 491 | } |
| 492 | if alternates(trimmed) { | ||
| 493 | continue | ||
| 494 | } | ||
| 492 | dest := filepath.Join(tmp, filepath.FromSlash(trimmed)) | 495 | dest := filepath.Join(tmp, filepath.FromSlash(trimmed)) |
| 493 | switch h.Typeflag { | 496 | switch h.Typeflag { |
| 494 | case tar.TypeDir: | 497 | case tar.TypeDir: |
| @@ -563,6 +566,17 @@ func verifyBackup(path, identity string) error { | |||
| 563 | return nil | 566 | return nil |
| 564 | } | 567 | } |
| 565 | 568 | ||
| 569 | // alternates reports an archive member that would point git at object | ||
| 570 | // stores outside the extracted repository. gitbay writes none, and one in | ||
| 571 | // a hostile archive would have fsck read other paths, so verify leaves | ||
| 572 | // them out. The comparison ignores case, as a case-insensitive | ||
| 573 | // filesystem would. | ||
| 574 | func alternates(name string) bool { | ||
| 575 | name = strings.ToLower(filepath.ToSlash(filepath.Clean(name))) | ||
| 576 | return strings.HasSuffix(name, "/objects/info/alternates") || | ||
| 577 | strings.HasSuffix(name, "/objects/info/http-alternates") | ||
| 578 | } | ||
| 579 | |||
| 566 | // extractTo writes one archive member to dest, owner-only. | 580 | // extractTo writes one archive member to dest, owner-only. |
| 567 | func extractTo(r io.Reader, dest string) error { | 581 | func extractTo(r io.Reader, dest string) error { |
| 568 | if err := os.MkdirAll(filepath.Dir(dest), 0o700); err != nil { | 582 | if err := os.MkdirAll(filepath.Dir(dest), 0o700); err != nil { |
cmd/gitbayd/backup_test.go +55
| @@ -717,3 +717,58 @@ func TestBackupRefusesOutputInsideRoot(t *testing.T) { | |||
| 717 | } | 717 | } |
| 718 | } | 718 | } |
| 719 | } | 719 | } |
| 720 | |||
| 721 | // verify does not extract objects/info/alternates, so an archived | ||
| 722 | // repository cannot borrow objects from paths outside the archive. | ||
| 723 | func TestVerifyIgnoresAlternates(t *testing.T) { | ||
| 724 | cfg := testConfig(t) | ||
| 725 | st, err := openStore(cfg) | ||
| 726 | if err != nil { | ||
| 727 | t.Fatal(err) | ||
| 728 | } | ||
| 729 | uid, err := st.CreateUser("krz", false) | ||
| 730 | if err != nil { | ||
| 731 | t.Fatal(err) | ||
| 732 | } | ||
| 733 | if _, err := st.CreateRepo("user", uid, "thing", "public"); err != nil { | ||
| 734 | t.Fatal(err) | ||
| 735 | } | ||
| 736 | st.Close() | ||
| 737 | |||
| 738 | work := t.TempDir() | ||
| 739 | gitIn(t, work, "init", "-q", "-b", "main") | ||
| 740 | if err := os.WriteFile(filepath.Join(work, "a.txt"), []byte("a\n"), 0o644); err != nil { | ||
| 741 | t.Fatal(err) | ||
| 742 | } | ||
| 743 | gitIn(t, work, "add", "a.txt") | ||
| 744 | gitIn(t, work, "commit", "-q", "-m", "one") | ||
| 745 | dir := filepath.Join(cfg.Server.Root, "repos", "krz", "thing.git") | ||
| 746 | gitIn(t, work, "clone", "-q", "--bare", "--shared", work, dir) | ||
| 747 | if _, err := os.Stat(filepath.Join(dir, "objects", "info", "alternates")); err != nil { | ||
| 748 | t.Fatal(err) | ||
| 749 | } | ||
| 750 | gitIn(t, dir, "fsck", "--connectivity-only", "--no-progress") | ||
| 751 | |||
| 752 | archive := filepath.Join(t.TempDir(), "b.tar.gz") | ||
| 753 | if err := runBackup(cfg, archive, false); err != nil { | ||
| 754 | t.Fatal(err) | ||
| 755 | } | ||
| 756 | if err := verifyBackup(archive, ""); err == nil || !strings.Contains(err.Error(), "connectivity") { | ||
| 757 | t.Fatalf("verify of a repository whose objects are only in an alternate: %v", err) | ||
| 758 | } | ||
| 759 | } | ||
| 760 | |||
| 761 | func TestAlternatesMember(t *testing.T) { | ||
| 762 | for name, want := range map[string]bool{ | ||
| 763 | "repos/a/b.git/objects/info/alternates": true, | ||
| 764 | "repos/a/b.git/objects/info/./alternates": true, | ||
| 765 | "repos/a/b.git/objects/info/Alternates": true, | ||
| 766 | "repos/a/b.git/objects/info/http-alternates": true, | ||
| 767 | "repos/a/b.git/objects/info/packs": false, | ||
| 768 | "repos/a/b.git/refs/heads/alternates": false, | ||
| 769 | } { | ||
| 770 | if got := alternates(name); got != want { | ||
| 771 | t.Errorf("alternates(%q) = %v, want %v", name, got, want) | ||
| 772 | } | ||
| 773 | } | ||
| 774 | } | ||
internal/gitutil/fsck_test.go +20 −2
| @@ -14,7 +14,8 @@ func TestFsckConnectivityFindsAMissingObject(t *testing.T) { | |||
| 14 | write(t, dir, "a.txt", "a\n") | 14 | write(t, dir, "a.txt", "a\n") |
| 15 | git(t, dir, "add", "a.txt") | 15 | git(t, dir, "add", "a.txt") |
| 16 | git(t, dir, "commit", "-q", "-m", "one") | 16 | git(t, dir, "commit", "-q", "-m", "one") |
| 17 | if err := FsckConnectivity(dir); err != nil { | 17 | gitDir := filepath.Join(dir, ".git") |
| 18 | if err := FsckConnectivity(gitDir); err != nil { | ||
| 18 | t.Fatalf("intact repository: %v", err) | 19 | t.Fatalf("intact repository: %v", err) |
| 19 | } | 20 | } |
| 20 | out, err := exec.Command("git", "-C", dir, "rev-parse", "HEAD:a.txt").Output() | 21 | out, err := exec.Command("git", "-C", dir, "rev-parse", "HEAD:a.txt").Output() |
| @@ -25,7 +26,24 @@ func TestFsckConnectivityFindsAMissingObject(t *testing.T) { | |||
| 25 | if err := os.Remove(filepath.Join(dir, ".git", "objects", blob[:2], blob[2:])); err != nil { | 26 | if err := os.Remove(filepath.Join(dir, ".git", "objects", blob[:2], blob[2:])); err != nil { |
| 26 | t.Fatal(err) | 27 | t.Fatal(err) |
| 27 | } | 28 | } |
| 28 | if err := FsckConnectivity(dir); err == nil { | 29 | if err := FsckConnectivity(gitDir); err == nil { |
| 29 | t.Fatal("a repository missing a blob passed") | 30 | t.Fatal("a repository missing a blob passed") |
| 30 | } | 31 | } |
| 31 | } | 32 | } |
| 33 | |||
| 34 | // A directory that is not a repository fails, even inside one: git does | ||
| 35 | // not search upward and check the enclosing repository instead. | ||
| 36 | func TestFsckConnectivityRefusesANonRepository(t *testing.T) { | ||
| 37 | dir := t.TempDir() | ||
| 38 | git(t, dir, "init", "-q", "-b", "main") | ||
| 39 | write(t, dir, "a.txt", "a\n") | ||
| 40 | git(t, dir, "add", "a.txt") | ||
| 41 | git(t, dir, "commit", "-q", "-m", "one") | ||
| 42 | inner := filepath.Join(dir, "repos", "x.git") | ||
| 43 | if err := os.MkdirAll(inner, 0o755); err != nil { | ||
| 44 | t.Fatal(err) | ||
| 45 | } | ||
| 46 | if err := FsckConnectivity(inner); err == nil { | ||
| 47 | t.Fatal("an empty directory inside a repository passed") | ||
| 48 | } | ||
| 49 | } | ||
internal/gitutil/merge.go +4 −2
| @@ -68,9 +68,11 @@ func PruneNow(dir string) error { | |||
| 68 | 68 | ||
| 69 | // FsckConnectivity checks that every object reachable from the | 69 | // FsckConnectivity checks that every object reachable from the |
| 70 | // repository's refs is present, without reading blob contents. A backup | 70 | // repository's refs is present, without reading blob contents. A backup |
| 71 | // verify runs it on each archived repository (#259). | 71 | // verify runs it on each archived repository (#259). dir is the git |
| 72 | // directory itself; it is passed as --git-dir so that a directory that is | ||
| 73 | // not a repository fails instead of git checking one enclosing it. | ||
| 72 | func FsckConnectivity(dir string) error { | 74 | func FsckConnectivity(dir string) error { |
| 73 | cmd := exec.Command(toolpath.Look("git"), "-C", dir, "fsck", "--connectivity-only", "--no-progress", "--no-dangling") | 75 | cmd := exec.Command(toolpath.Look("git"), "--git-dir="+dir, "fsck", "--connectivity-only", "--no-progress", "--no-dangling") |
| 74 | if out, err := cmd.CombinedOutput(); err != nil { | 76 | if out, err := cmd.CombinedOutput(); err != nil { |
| 75 | return fmt.Errorf("fsck --connectivity-only: %v\n%s", err, out) | 77 | return fmt.Errorf("fsck --connectivity-only: %v\n%s", err, out) |
| 76 | } | 78 | } |