Skip to content

Commit bdf0688

Browse files
pjbgfpjbgf
authored
Merge pull request #1864 from pjbgf/v5-issue-55
storage: filesystem, Avoid overwriting loose obj files
2 parents 8ed442c + 5290e52 commit bdf0688

7 files changed

Lines changed: 252 additions & 5 deletions

File tree

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ require (
1313
github.com/emirpasic/gods v1.18.1
1414
github.com/gliderlabs/ssh v0.3.8
1515
github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376
16-
github.com/go-git/go-billy/v5 v5.6.2
16+
github.com/go-git/go-billy/v5 v5.8.0
1717
github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399
1818
github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8
1919
github.com/google/go-cmp v0.7.0

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ github.com/gliderlabs/ssh v0.3.8 h1:a4YXD1V7xMF9g5nTkdfnja3Sxy1PVDCj1Zg4Wb8vY6c=
2525
github.com/gliderlabs/ssh v0.3.8/go.mod h1:xYoytBv1sV0aL3CavoDuJIQNURXkkfPA/wxQ1pL1fAU=
2626
github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 h1:+zs/tPmkDkHx3U66DAb0lQFJrpS6731Oaa12ikc+DiI=
2727
github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376/go.mod h1:an3vInlBmSxCcxctByoQdvwPiA7DTK7jaaFDBTtu0ic=
28-
github.com/go-git/go-billy/v5 v5.6.2 h1:6Q86EsPXMa7c3YZ3aLAQsMA0VlWmy43r6FHqa/UNbRM=
29-
github.com/go-git/go-billy/v5 v5.6.2/go.mod h1:rcFC2rAsp/erv7CMz9GczHcuD0D32fWzH+MJAU+jaUU=
28+
github.com/go-git/go-billy/v5 v5.8.0 h1:I8hjc3LbBlXTtVuFNJuwYuMiHvQJDq1AT6u4DwDzZG0=
29+
github.com/go-git/go-billy/v5 v5.8.0/go.mod h1:RpvI/rw4Vr5QA+Z60c6d6LXH0rYJo0uD5SqfmrrheCY=
3030
github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399 h1:eMje31YglSBqCdIqdhKBW8lokaMrL3uTkpGYlE2OOT4=
3131
github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399/go.mod h1:1OCfN199q1Jm3HZlxleg+Dw/mwps2Wbk9frAWm+4FII=
3232
github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 h1:f+oWsMOmNPc8JmEHVZIycC7hBoQxHH9pNKQORJNozsQ=

‎storage/filesystem/dotgit/dotgit_test.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/go-git/go-git/v5/plumbing"
1919
"github.com/go-git/go-git/v5/storage"
2020
"github.com/stretchr/testify/assert"
21+
"github.com/stretchr/testify/require"
2122
. "gopkg.in/check.v1"
2223
)
2324

@@ -1094,3 +1095,61 @@ func (s *SuiteDotGit) TestSetPackedRef(c *C) {
10941095
c.Assert(err, IsNil)
10951096
c.Assert(looseCount, Equals, 1)
10961097
}
1098+
1099+
func TestIssue55(t *testing.T) {
1100+
t.Parallel()
1101+
1102+
writeObject := func(fs billy.Filesystem) {
1103+
t.Helper()
1104+
1105+
dir := New(fs)
1106+
err := dir.Initialize()
1107+
require.NoError(t, err)
1108+
1109+
w, err := dir.NewObject()
1110+
require.NoError(t, err)
1111+
1112+
err = w.WriteHeader(plumbing.BlobObject, 14)
1113+
require.NoError(t, err)
1114+
n, err := w.Write([]byte("this is a test"))
1115+
require.NoError(t, err)
1116+
assert.Equal(t, 14, n)
1117+
1118+
assert.Equal(t, "a8a940627d132695a9769df883f85992f0ff4a43", w.Hash().String())
1119+
1120+
err = w.Close()
1121+
require.NoError(t, err)
1122+
}
1123+
1124+
for _, tc := range []struct {
1125+
name string
1126+
fs billy.Filesystem
1127+
}{
1128+
{"BoundOS", osfs.New(t.TempDir(), osfs.WithBoundOS())},
1129+
{"ChrootOS", osfs.New(t.TempDir(), osfs.WithChrootOS())},
1130+
} {
1131+
t.Run(tc.name, func(t *testing.T) {
1132+
t.Parallel()
1133+
path := filepath.Join("objects", "a8", "a940627d132695a9769df883f85992f0ff4a43")
1134+
1135+
writeObject(tc.fs)
1136+
i, err := tc.fs.Stat(path)
1137+
require.NoError(t, err)
1138+
assert.Equal(t, int64(34), i.Size())
1139+
1140+
ro, err := isReadOnly(tc.fs, path)
1141+
require.NoError(t, err)
1142+
assert.True(t, ro, "file %q is not read-only", path)
1143+
1144+
// Recreate the same object.
1145+
writeObject(tc.fs)
1146+
i, err = tc.fs.Stat(path)
1147+
require.NoError(t, err)
1148+
assert.Equal(t, int64(34), i.Size())
1149+
1150+
ro, err = isReadOnly(tc.fs, path)
1151+
require.NoError(t, err)
1152+
assert.True(t, ro, "file %q is not read-only", path)
1153+
})
1154+
}
1155+
}

‎storage/filesystem/dotgit/writers.go‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package dotgit
33
import (
44
"fmt"
55
"io"
6+
"os"
67
"sync/atomic"
78

89
"github.com/go-git/go-git/v5/plumbing"
@@ -137,14 +138,22 @@ func (w *PackWriter) save() error {
137138
}
138139

139140
if err := w.encodeIdx(idx); err != nil {
141+
_ = idx.Close()
140142
return err
141143
}
142144

143145
if err := idx.Close(); err != nil {
144146
return err
145147
}
148+
fixPermissions(w.fs, fmt.Sprintf("%s.idx", base))
146149

147-
return w.fs.Rename(w.fw.Name(), fmt.Sprintf("%s.pack", base))
150+
packPath := fmt.Sprintf("%s.pack", base)
151+
if err := w.fs.Rename(w.fw.Name(), packPath); err != nil {
152+
return err
153+
}
154+
fixPermissions(w.fs, packPath)
155+
156+
return nil
148157
}
149158

150159
func (w *PackWriter) encodeIdx(writer io.Writer) error {
@@ -281,5 +290,17 @@ func (w *ObjectWriter) save() error {
281290
hex := w.Hash().String()
282291
file := w.fs.Join(objectsPath, hex[0:2], hex[2:hash.HexSize])
283292

284-
return w.fs.Rename(w.f.Name(), file)
293+
// Loose objects are content addressable, if they already exist
294+
// we can safely delete the temporary file and short-circuit the
295+
// operation.
296+
if _, err := w.fs.Stat(file); err == nil || os.IsExist(err) {
297+
return w.fs.Remove(w.f.Name())
298+
}
299+
300+
if err := w.fs.Rename(w.f.Name(), file); err != nil {
301+
return err
302+
}
303+
fixPermissions(w.fs, file)
304+
305+
return nil
285306
}

‎storage/filesystem/dotgit/writers_test.go‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,18 @@ import (
44
"fmt"
55
"io"
66
"os"
7+
"path/filepath"
78
"strconv"
9+
"testing"
810

11+
"github.com/go-git/go-billy/v5"
912
"github.com/go-git/go-billy/v5/osfs"
1013
"github.com/go-git/go-billy/v5/util"
1114
"github.com/go-git/go-git/v5/plumbing"
1215
"github.com/go-git/go-git/v5/plumbing/format/idxfile"
1316
"github.com/go-git/go-git/v5/plumbing/format/packfile"
17+
"github.com/stretchr/testify/assert"
18+
"github.com/stretchr/testify/require"
1419

1520
fixtures "github.com/go-git/go-git-fixtures/v4"
1621
. "gopkg.in/check.v1"
@@ -135,3 +140,78 @@ func (s *SuiteDotGit) TestPackWriterUnusedNotify(c *C) {
135140

136141
c.Assert(w.Close(), IsNil)
137142
}
143+
144+
func TestPackWriterPermissions(t *testing.T) {
145+
t.Parallel()
146+
147+
for _, tc := range []struct {
148+
name string
149+
fs billy.Filesystem
150+
}{
151+
{"BoundOS", osfs.New(t.TempDir(), osfs.WithBoundOS())},
152+
{"ChrootOS", osfs.New(t.TempDir(), osfs.WithChrootOS())},
153+
} {
154+
t.Run(tc.name, func(t *testing.T) {
155+
t.Parallel()
156+
157+
f := fixtures.Basic().One()
158+
159+
dot := New(tc.fs)
160+
require.NoError(t, dot.Initialize())
161+
162+
w, err := dot.NewObjectPack()
163+
require.NoError(t, err)
164+
165+
_, err = io.Copy(w, f.Packfile())
166+
require.NoError(t, err)
167+
168+
require.NoError(t, w.Close())
169+
170+
pfPath := filepath.Join("objects", "pack", fmt.Sprintf("pack-%s.pack", f.PackfileHash))
171+
idxPath := filepath.Join("objects", "pack", fmt.Sprintf("pack-%s.idx", f.PackfileHash))
172+
173+
ro, err := isReadOnly(tc.fs, pfPath)
174+
require.NoError(t, err)
175+
assert.True(t, ro, "file %q is not read-only", pfPath)
176+
177+
ro, err = isReadOnly(tc.fs, idxPath)
178+
require.NoError(t, err)
179+
assert.True(t, ro, "file %q is not read-only", idxPath)
180+
})
181+
}
182+
}
183+
184+
func TestObjectWriterPermissions(t *testing.T) {
185+
t.Parallel()
186+
187+
for _, tc := range []struct {
188+
name string
189+
fs billy.Filesystem
190+
}{
191+
{"BoundOS", osfs.New(t.TempDir(), osfs.WithBoundOS())},
192+
{"ChrootOS", osfs.New(t.TempDir(), osfs.WithChrootOS())},
193+
} {
194+
t.Run(tc.name, func(t *testing.T) {
195+
t.Parallel()
196+
197+
dot := New(tc.fs)
198+
require.NoError(t, dot.Initialize())
199+
200+
w, err := dot.NewObject()
201+
require.NoError(t, err)
202+
203+
err = w.WriteHeader(plumbing.BlobObject, 14)
204+
require.NoError(t, err)
205+
206+
_, err = w.Write([]byte("this is a test"))
207+
require.NoError(t, err)
208+
209+
require.NoError(t, w.Close())
210+
211+
path := filepath.Join("objects", "a8", "a940627d132695a9769df883f85992f0ff4a43")
212+
ro, err := isReadOnly(tc.fs, path)
213+
require.NoError(t, err)
214+
assert.True(t, ro, "file %q is not read-only", path)
215+
})
216+
}
217+
}

‎storage/filesystem/dotgit/writers_unix.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//go:build !windows
2+
3+
package dotgit
4+
5+
import (
6+
"github.com/go-git/go-billy/v5"
7+
"github.com/go-git/go-git/v5/utils/trace"
8+
)
9+
10+
func fixPermissions(fs billy.Filesystem, path string) {
11+
if chmodFS, ok := fs.(billy.Chmod); ok {
12+
if err := chmodFS.Chmod(path, 0o444); err != nil {
13+
trace.General.Printf("failed to chmod %s: %v", path, err)
14+
}
15+
}
16+
}
17+
18+
func isReadOnly(fs billy.Filesystem, path string) (bool, error) {
19+
fi, err := fs.Stat(path)
20+
if err != nil {
21+
return false, err
22+
}
23+
24+
if fi.Mode().Perm() == 0o444 {
25+
return true, nil
26+
}
27+
28+
return false, nil
29+
}

‎storage/filesystem/dotgit/writers_windows.go‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
//go:build windows
2+
3+
package dotgit
4+
5+
import (
6+
"fmt"
7+
"path/filepath"
8+
9+
"github.com/go-git/go-billy/v5"
10+
"github.com/go-git/go-git/v5/utils/trace"
11+
"golang.org/x/sys/windows"
12+
)
13+
14+
func fixPermissions(fs billy.Filesystem, path string) {
15+
fullpath := filepath.Join(fs.Root(), path)
16+
p, err := windows.UTF16PtrFromString(fullpath)
17+
if err != nil {
18+
trace.General.Printf("failed to chmod %s: %v", fullpath, err)
19+
return
20+
}
21+
22+
attrs, err := windows.GetFileAttributes(p)
23+
if err != nil {
24+
trace.General.Printf("failed to chmod %s: %v", fullpath, err)
25+
return
26+
}
27+
28+
if attrs&windows.FILE_ATTRIBUTE_READONLY != 0 {
29+
return
30+
}
31+
32+
err = windows.SetFileAttributes(p,
33+
attrs|windows.FILE_ATTRIBUTE_READONLY,
34+
)
35+
36+
if err != nil {
37+
trace.General.Printf("failed to chmod %s: %v", fullpath, err)
38+
}
39+
}
40+
41+
func isReadOnly(fs billy.Filesystem, path string) (bool, error) {
42+
fullpath := filepath.Join(fs.Root(), path)
43+
p, err := windows.UTF16PtrFromString(fullpath)
44+
if err != nil {
45+
return false, fmt.Errorf("%w: %q", err, fullpath)
46+
}
47+
48+
attrs, err := windows.GetFileAttributes(p)
49+
if err != nil {
50+
return false, fmt.Errorf("%w: %q", err, fullpath)
51+
}
52+
53+
if attrs&windows.FILE_ATTRIBUTE_READONLY != 0 {
54+
return true, nil
55+
}
56+
57+
return false, nil
58+
}

0 commit comments

Comments
 (0)