detached cb1999a

Merge pull request #121 from bnema/fix/drop-x-sys-snapshot

refactor(snapshot): replace x/sys filesystem calls with os.Root
commit
cb1999a9584ca20b293e4a85a7b32c3f976f75fc
author
bnema · · 2026-07-22 11:47 UTC
committer
GitHub · · 2026-07-22 11:47 UTC
merge of
59be6a2 + b649528 · diff against the first parent
signature
GPG UnverifiedB5690EEEBB952194 The key is unknown
18 files · +355 -298
Unified
UnifiedSplit
M go.mod +1 -4
@@ -2,10 +2,7 @@ module github.com/bnema/vev
22
33 go 1.26
44
5-require (
6- github.com/stretchr/testify v1.11.1
7- golang.org/x/sys v0.47.0
8-)
5+require github.com/stretchr/testify v1.11.1
96
107 require (
118 github.com/davecgh/go-spew v1.1.1 // indirect
@@ -2,10 +2,7 @@ module github.com/bnema/vev
2 2
3 go 1.263 go 1.26
4 4
5-require (5+require github.com/stretchr/testify v1.11.1
6- github.com/stretchr/testify v1.11.1
7- golang.org/x/sys v0.47.0
8-)
9 6
10 require (7 require (
11 github.com/davecgh/go-spew v1.1.1 // indirect8 github.com/davecgh/go-spew v1.1.1 // indirect
M go.sum +0 -2 generated file: collapsed
load diff
M internal/adapters/snapshot/atomic.go +62 -79
@@ -9,7 +9,6 @@ import (
99 "syscall"
1010
1111 "github.com/bnema/vev/pkg/safedir"
12- "golang.org/x/sys/unix"
1312 )
1413
1514 func (r *Repository) ensureSession(key string) error {
@@ -34,7 +33,7 @@ func (r *Repository) ensurePrivateDirectory(dir string) error {
3433 return r.ensurePrivateDirectoryPhase(dir, "snapshot directory")
3534 }
3635
37-func (r *Repository) ensurePrivateDirectoryPhase(dir, phase string) error {
36+func (r *Repository) ensurePrivateDirectoryPhase(dir, phase string) (err error) {
3837 // The configured root is the trust boundary. Once it is private, every
3938 // descendant is created through a pinned parent descriptor rather than via
4039 // a path that an attacker can replace between checks.
@@ -62,38 +61,31 @@ func (r *Repository) ensurePrivateDirectoryPhase(dir, phase string) error {
6261 if err := r.ensurePrivateDirectoryPhase(filepath.Dir(dir), "snapshot directory"); err != nil {
6362 return err
6463 }
65- parent, name, err := r.repositoryParent(dir)
64+ rel, ok := r.repositoryRelative(dir)
65+ if !ok {
66+ return fmt.Errorf("snapshot path outside repository")
67+ }
68+ root, err := r.openRoot()
6669 if err != nil {
6770 return err
6871 }
72+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
6973 created := false
70- mkdirErr := unix.Mkdirat(parent, name, 0o700)
74+ mkdirErr := root.Mkdir(rel, 0o700)
7175 if mkdirErr == nil {
7276 created = true
7377 }
74- parentCloseErr := r.closeDescriptor(parent, "close snapshot parent directory")
75- if mkdirErr != nil && !errors.Is(mkdirErr, syscall.EEXIST) {
76- err = mkdirErr
77- joinCloseError(&err, "close snapshot parent directory", parentCloseErr)
78- return err
79- }
80- if parentCloseErr != nil {
81- return fmt.Errorf("close snapshot parent directory: %w", parentCloseErr)
78+ if mkdirErr != nil && !errors.Is(mkdirErr, os.ErrExist) {
79+ return mkdirErr
8280 }
83- opened, err := r.openDirectory(dir)
81+ fi, err := root.Lstat(rel)
8482 if err != nil {
8583 return err
8684 }
87- var st syscall.Stat_t
88- statErr := syscall.Fstat(int(opened.Fd()), &st)
89- closeErr := opened.Close()
90- if statErr != nil {
91- return statErr
92- }
93- if closeErr != nil {
94- return closeErr
85+ if !fi.IsDir() {
86+ return fmt.Errorf("snapshot directory is not a directory")
9587 }
96- if int(st.Uid) != os.Geteuid() || st.Mode&0o777 != 0o700 {
88+ if !privateDirectory(fi) {
9789 return fmt.Errorf("snapshot directory is not private")
9890 }
9991 if created {
@@ -231,24 +223,6 @@ func (r *Repository) closeFile(f *os.File) error {
231223 return errors.Join(injected, f.Close())
232224 }
233225
234-// closeDescriptor closes a raw descriptor while keeping its close boundary
235-// fault-injectable for tests. Callers add operation context before returning.
236-func (r *Repository) closeDescriptor(fd int, operation string) error {
237- var injected error
238- if r.hooks.closeDescriptor != nil {
239- injected = r.hooks.closeDescriptor(operation)
240- }
241- return errors.Join(injected, syscall.Close(fd))
242-}
243-
244-func (r *Repository) closeRepositoryFile(f *os.File, operation string) error {
245- var injected error
246- if r.hooks.closeDescriptor != nil {
247- injected = r.hooks.closeDescriptor(operation)
248- }
249- return errors.Join(injected, f.Close())
250-}
251-
252226 // joinCloseError retains a primary operation failure and appends contextual
253227 // close failures, including cleanup-only paths, without suppressing either.
254228 func joinCloseError(primary *error, operation string, closeErr error) {
@@ -263,21 +237,20 @@ func (r *Repository) installImmutable(oldPath, newPath string) (err error) {
263237 return err
264238 }
265239 }
266- oldFD, oldName, err := r.repositoryParent(oldPath)
267- if err != nil {
268- return err
240+ oldRel, ok := r.repositoryRelative(oldPath)
241+ if !ok {
242+ return fmt.Errorf("snapshot path outside repository")
269243 }
270- defer func() {
271- joinCloseError(&err, "close source snapshot parent directory", r.closeDescriptor(oldFD, "close source snapshot parent directory"))
272- }()
273- newFD, newName, err := r.repositoryParent(newPath)
244+ newRel, ok := r.repositoryRelative(newPath)
245+ if !ok {
246+ return fmt.Errorf("snapshot path outside repository")
247+ }
248+ root, err := r.openRoot()
274249 if err != nil {
275250 return err
276251 }
277- defer func() {
278- joinCloseError(&err, "close destination snapshot parent directory", r.closeDescriptor(newFD, "close destination snapshot parent directory"))
279- }()
280- return unix.Linkat(oldFD, oldName, newFD, newName, 0)
252+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
253+ return root.Link(oldRel, newRel)
281254 }
282255 func (r *Repository) rename(oldPath, newPath string) (err error) {
283256 if r.hooks.rename != nil {
@@ -285,21 +258,20 @@ func (r *Repository) rename(oldPath, newPath string) (err error) {
285258 return err
286259 }
287260 }
288- oldFD, oldName, err := r.repositoryParent(oldPath)
289- if err != nil {
290- return err
261+ oldRel, ok := r.repositoryRelative(oldPath)
262+ if !ok {
263+ return fmt.Errorf("snapshot path outside repository")
264+ }
265+ newRel, ok := r.repositoryRelative(newPath)
266+ if !ok {
267+ return fmt.Errorf("snapshot path outside repository")
291268 }
292- defer func() {
293- joinCloseError(&err, "close source snapshot parent directory", r.closeDescriptor(oldFD, "close source snapshot parent directory"))
294- }()
295- newFD, newName, err := r.repositoryParent(newPath)
269+ root, err := r.openRoot()
296270 if err != nil {
297271 return err
298272 }
299- defer func() {
300- joinCloseError(&err, "close destination snapshot parent directory", r.closeDescriptor(newFD, "close destination snapshot parent directory"))
301- }()
302- return unix.Renameat(oldFD, oldName, newFD, newName)
273+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
274+ return root.Rename(oldRel, newRel)
303275 }
304276 func (r *Repository) remove(path string) (err error) {
305277 if r.hooks.remove != nil {
@@ -307,22 +279,16 @@ func (r *Repository) remove(path string) (err error) {
307279 return err
308280 }
309281 }
310- fd, name, err := r.repositoryParent(path)
282+ rel, ok := r.repositoryRelative(path)
283+ if !ok {
284+ return fmt.Errorf("snapshot path outside repository")
285+ }
286+ root, err := r.openRoot()
311287 if err != nil {
312288 return err
313289 }
314- defer func() {
315- joinCloseError(&err, "close snapshot parent directory", r.closeDescriptor(fd, "close snapshot parent directory"))
316- }()
317- unlinkErr := unix.Unlinkat(fd, name, 0)
318- if unlinkErr == nil {
319- return nil
320- }
321- var stat unix.Stat_t
322- if err := unix.Fstatat(fd, name, &stat, unix.AT_SYMLINK_NOFOLLOW); err != nil || !directoryUnlinkRetry(unlinkErr, uint32(stat.Mode)) {
323- return unlinkErr
324- }
325- return unix.Unlinkat(fd, name, unix.AT_REMOVEDIR)
290+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
291+ return root.Remove(rel)
326292 }
327293 func (r *Repository) syncDirectory(dir string) error {
328294 if r.hooks.syncDirectory != nil {
@@ -352,11 +318,28 @@ func (r *Repository) readOptionalBounded(path string) ([]byte, bool, error) {
352318 return data, err == nil, err
353319 }
354320
355-// readBounded opens the final component with O_NOFOLLOW, validates the opened
321+// readBounded rejects a final symlink explicitly (rejectFinalSymlink) before
322+// opening the component under the confined root, validates the opened
356323 // descriptor, then reads a hard bounded amount. It deliberately never trusts
357324 // path metadata obtained before opening the descriptor.
358-func (r *Repository) readBounded(path string) ([]byte, error) {
359- f, err := r.repositoryFD(path, syscall.O_RDONLY|syscall.O_NONBLOCK, 0)
325+func (r *Repository) readBounded(path string) (data []byte, err error) {
326+ root, err := r.openRoot()
327+ if err != nil {
328+ return nil, err
329+ }
330+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
331+ return r.readBoundedRoot(root, path)
332+}
333+
334+func (r *Repository) readBoundedRoot(root *os.Root, path string) ([]byte, error) {
335+ rel, ok := r.repositoryRelative(path)
336+ if !ok {
337+ return nil, fmt.Errorf("snapshot path outside repository")
338+ }
339+ if err := rejectFinalSymlink(root, rel); err != nil {
340+ return nil, err
341+ }
342+ f, err := root.OpenFile(rel, syscall.O_RDONLY|syscall.O_NONBLOCK, 0)
360343 if err != nil {
361344 return nil, err
362345 }
@@ -364,7 +347,7 @@ func (r *Repository) readBounded(path string) ([]byte, error) {
364347 }
365348
366349 // readBounded remains for focused descriptor-hardening tests. Repository code
367-// must use r.readBounded so intermediate components are pinned and no-follow.
350+// must use r.readBounded so every path stays confined to the pinned private root.
368351 func readBounded(path string) ([]byte, error) {
369352 fd, err := syscall.Open(path, syscall.O_RDONLY|syscall.O_CLOEXEC|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0)
370353 if err != nil {
@@ -9,7 +9,6 @@ import (
9 "syscall"9 "syscall"
10 10
11 "github.com/bnema/vev/pkg/safedir"11 "github.com/bnema/vev/pkg/safedir"
12- "golang.org/x/sys/unix"
13 )12 )
14 13
15 func (r *Repository) ensureSession(key string) error {14 func (r *Repository) ensureSession(key string) error {
@@ -34,7 +33,7 @@ func (r *Repository) ensurePrivateDirectory(dir string) error {
34 return r.ensurePrivateDirectoryPhase(dir, "snapshot directory")33 return r.ensurePrivateDirectoryPhase(dir, "snapshot directory")
35 }34 }
36 35
37-func (r *Repository) ensurePrivateDirectoryPhase(dir, phase string) error {36+func (r *Repository) ensurePrivateDirectoryPhase(dir, phase string) (err error) {
38 // The configured root is the trust boundary. Once it is private, every37 // The configured root is the trust boundary. Once it is private, every
39 // descendant is created through a pinned parent descriptor rather than via38 // descendant is created through a pinned parent descriptor rather than via
40 // a path that an attacker can replace between checks.39 // a path that an attacker can replace between checks.
@@ -62,38 +61,31 @@ func (r *Repository) ensurePrivateDirectoryPhase(dir, phase string) error {
62 if err := r.ensurePrivateDirectoryPhase(filepath.Dir(dir), "snapshot directory"); err != nil {61 if err := r.ensurePrivateDirectoryPhase(filepath.Dir(dir), "snapshot directory"); err != nil {
63 return err62 return err
64 }63 }
65- parent, name, err := r.repositoryParent(dir)64+ rel, ok := r.repositoryRelative(dir)
65+ if !ok {
66+ return fmt.Errorf("snapshot path outside repository")
67+ }
68+ root, err := r.openRoot()
66 if err != nil {69 if err != nil {
67 return err70 return err
68 }71 }
72+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
69 created := false73 created := false
70- mkdirErr := unix.Mkdirat(parent, name, 0o700)74+ mkdirErr := root.Mkdir(rel, 0o700)
71 if mkdirErr == nil {75 if mkdirErr == nil {
72 created = true76 created = true
73 }77 }
74- parentCloseErr := r.closeDescriptor(parent, "close snapshot parent directory")78+ if mkdirErr != nil && !errors.Is(mkdirErr, os.ErrExist) {
75- if mkdirErr != nil && !errors.Is(mkdirErr, syscall.EEXIST) {79+ return mkdirErr
76- err = mkdirErr
77- joinCloseError(&err, "close snapshot parent directory", parentCloseErr)
78- return err
79- }
80- if parentCloseErr != nil {
81- return fmt.Errorf("close snapshot parent directory: %w", parentCloseErr)
82 }80 }
83- opened, err := r.openDirectory(dir)81+ fi, err := root.Lstat(rel)
84 if err != nil {82 if err != nil {
85 return err83 return err
86 }84 }
87- var st syscall.Stat_t85+ if !fi.IsDir() {
88- statErr := syscall.Fstat(int(opened.Fd()), &st)86+ return fmt.Errorf("snapshot directory is not a directory")
89- closeErr := opened.Close()
90- if statErr != nil {
91- return statErr
92- }
93- if closeErr != nil {
94- return closeErr
95 }87 }
96- if int(st.Uid) != os.Geteuid() || st.Mode&0o777 != 0o700 {88+ if !privateDirectory(fi) {
97 return fmt.Errorf("snapshot directory is not private")89 return fmt.Errorf("snapshot directory is not private")
98 }90 }
99 if created {91 if created {
@@ -231,24 +223,6 @@ func (r *Repository) closeFile(f *os.File) error {
231 return errors.Join(injected, f.Close())223 return errors.Join(injected, f.Close())
232 }224 }
233 225
234-// closeDescriptor closes a raw descriptor while keeping its close boundary
235-// fault-injectable for tests. Callers add operation context before returning.
236-func (r *Repository) closeDescriptor(fd int, operation string) error {
237- var injected error
238- if r.hooks.closeDescriptor != nil {
239- injected = r.hooks.closeDescriptor(operation)
240- }
241- return errors.Join(injected, syscall.Close(fd))
242-}
243-
244-func (r *Repository) closeRepositoryFile(f *os.File, operation string) error {
245- var injected error
246- if r.hooks.closeDescriptor != nil {
247- injected = r.hooks.closeDescriptor(operation)
248- }
249- return errors.Join(injected, f.Close())
250-}
251-
252 // joinCloseError retains a primary operation failure and appends contextual226 // joinCloseError retains a primary operation failure and appends contextual
253 // close failures, including cleanup-only paths, without suppressing either.227 // close failures, including cleanup-only paths, without suppressing either.
254 func joinCloseError(primary *error, operation string, closeErr error) {228 func joinCloseError(primary *error, operation string, closeErr error) {
@@ -263,21 +237,20 @@ func (r *Repository) installImmutable(oldPath, newPath string) (err error) {
263 return err237 return err
264 }238 }
265 }239 }
266- oldFD, oldName, err := r.repositoryParent(oldPath)240+ oldRel, ok := r.repositoryRelative(oldPath)
267- if err != nil {241+ if !ok {
268- return err242+ return fmt.Errorf("snapshot path outside repository")
269 }243 }
270- defer func() {244+ newRel, ok := r.repositoryRelative(newPath)
271- joinCloseError(&err, "close source snapshot parent directory", r.closeDescriptor(oldFD, "close source snapshot parent directory"))245+ if !ok {
272- }()246+ return fmt.Errorf("snapshot path outside repository")
273- newFD, newName, err := r.repositoryParent(newPath)247+ }
248+ root, err := r.openRoot()
274 if err != nil {249 if err != nil {
275 return err250 return err
276 }251 }
277- defer func() {252+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
278- joinCloseError(&err, "close destination snapshot parent directory", r.closeDescriptor(newFD, "close destination snapshot parent directory"))253+ return root.Link(oldRel, newRel)
279- }()
280- return unix.Linkat(oldFD, oldName, newFD, newName, 0)
281 }254 }
282 func (r *Repository) rename(oldPath, newPath string) (err error) {255 func (r *Repository) rename(oldPath, newPath string) (err error) {
283 if r.hooks.rename != nil {256 if r.hooks.rename != nil {
@@ -285,21 +258,20 @@ func (r *Repository) rename(oldPath, newPath string) (err error) {
285 return err258 return err
286 }259 }
287 }260 }
288- oldFD, oldName, err := r.repositoryParent(oldPath)261+ oldRel, ok := r.repositoryRelative(oldPath)
289- if err != nil {262+ if !ok {
290- return err263+ return fmt.Errorf("snapshot path outside repository")
264+ }
265+ newRel, ok := r.repositoryRelative(newPath)
266+ if !ok {
267+ return fmt.Errorf("snapshot path outside repository")
291 }268 }
292- defer func() {269+ root, err := r.openRoot()
293- joinCloseError(&err, "close source snapshot parent directory", r.closeDescriptor(oldFD, "close source snapshot parent directory"))
294- }()
295- newFD, newName, err := r.repositoryParent(newPath)
296 if err != nil {270 if err != nil {
297 return err271 return err
298 }272 }
299- defer func() {273+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
300- joinCloseError(&err, "close destination snapshot parent directory", r.closeDescriptor(newFD, "close destination snapshot parent directory"))274+ return root.Rename(oldRel, newRel)
301- }()
302- return unix.Renameat(oldFD, oldName, newFD, newName)
303 }275 }
304 func (r *Repository) remove(path string) (err error) {276 func (r *Repository) remove(path string) (err error) {
305 if r.hooks.remove != nil {277 if r.hooks.remove != nil {
@@ -307,22 +279,16 @@ func (r *Repository) remove(path string) (err error) {
307 return err279 return err
308 }280 }
309 }281 }
310- fd, name, err := r.repositoryParent(path)282+ rel, ok := r.repositoryRelative(path)
283+ if !ok {
284+ return fmt.Errorf("snapshot path outside repository")
285+ }
286+ root, err := r.openRoot()
311 if err != nil {287 if err != nil {
312 return err288 return err
313 }289 }
314- defer func() {290+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
315- joinCloseError(&err, "close snapshot parent directory", r.closeDescriptor(fd, "close snapshot parent directory"))291+ return root.Remove(rel)
316- }()
317- unlinkErr := unix.Unlinkat(fd, name, 0)
318- if unlinkErr == nil {
319- return nil
320- }
321- var stat unix.Stat_t
322- if err := unix.Fstatat(fd, name, &stat, unix.AT_SYMLINK_NOFOLLOW); err != nil || !directoryUnlinkRetry(unlinkErr, uint32(stat.Mode)) {
323- return unlinkErr
324- }
325- return unix.Unlinkat(fd, name, unix.AT_REMOVEDIR)
326 }292 }
327 func (r *Repository) syncDirectory(dir string) error {293 func (r *Repository) syncDirectory(dir string) error {
328 if r.hooks.syncDirectory != nil {294 if r.hooks.syncDirectory != nil {
@@ -352,11 +318,28 @@ func (r *Repository) readOptionalBounded(path string) ([]byte, bool, error) {
352 return data, err == nil, err318 return data, err == nil, err
353 }319 }
354 320
355-// readBounded opens the final component with O_NOFOLLOW, validates the opened321+// readBounded rejects a final symlink explicitly (rejectFinalSymlink) before
322+// opening the component under the confined root, validates the opened
356 // descriptor, then reads a hard bounded amount. It deliberately never trusts323 // descriptor, then reads a hard bounded amount. It deliberately never trusts
357 // path metadata obtained before opening the descriptor.324 // path metadata obtained before opening the descriptor.
358-func (r *Repository) readBounded(path string) ([]byte, error) {325+func (r *Repository) readBounded(path string) (data []byte, err error) {
359- f, err := r.repositoryFD(path, syscall.O_RDONLY|syscall.O_NONBLOCK, 0)326+ root, err := r.openRoot()
327+ if err != nil {
328+ return nil, err
329+ }
330+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
331+ return r.readBoundedRoot(root, path)
332+}
333+
334+func (r *Repository) readBoundedRoot(root *os.Root, path string) ([]byte, error) {
335+ rel, ok := r.repositoryRelative(path)
336+ if !ok {
337+ return nil, fmt.Errorf("snapshot path outside repository")
338+ }
339+ if err := rejectFinalSymlink(root, rel); err != nil {
340+ return nil, err
341+ }
342+ f, err := root.OpenFile(rel, syscall.O_RDONLY|syscall.O_NONBLOCK, 0)
360 if err != nil {343 if err != nil {
361 return nil, err344 return nil, err
362 }345 }
@@ -364,7 +347,7 @@ func (r *Repository) readBounded(path string) ([]byte, error) {
364 }347 }
365 348
366 // readBounded remains for focused descriptor-hardening tests. Repository code349 // readBounded remains for focused descriptor-hardening tests. Repository code
367-// must use r.readBounded so intermediate components are pinned and no-follow.350+// must use r.readBounded so every path stays confined to the pinned private root.
368 func readBounded(path string) ([]byte, error) {351 func readBounded(path string) ([]byte, error) {
369 fd, err := syscall.Open(path, syscall.O_RDONLY|syscall.O_CLOEXEC|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0)352 fd, err := syscall.Open(path, syscall.O_RDONLY|syscall.O_CLOEXEC|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0)
370 if err != nil {353 if err != nil {
M internal/adapters/snapshot/atomic_test.go +0 -33
@@ -5,8 +5,6 @@ import (
55 "errors"
66 "os"
77 "path/filepath"
8- "strings"
9- "syscall"
108 "testing"
119 )
1210
@@ -77,37 +75,6 @@ func TestWithAtomicTempPreservesLifecycleOrder(t *testing.T) {
7775 }
7876 }
7977
80-func TestInstallImmutableJoinsPrimaryAndParentCloseErrors(t *testing.T) {
81- repo := NewRepository(privateDir(t))
82- dir := filepath.Join(repo.dir, "objects")
83- if err := repo.ensurePrivateDirectory(dir); err != nil {
84- t.Fatal(err)
85- }
86- oldPath := filepath.Join(dir, "old")
87- newPath := filepath.Join(dir, "new")
88- if err := os.WriteFile(oldPath, []byte("old"), 0o600); err != nil {
89- t.Fatal(err)
90- }
91- if err := os.WriteFile(newPath, []byte("new"), 0o600); err != nil {
92- t.Fatal(err)
93- }
94- closeCause := errors.New("injected source parent close failure")
95- repo.hooks.closeDescriptor = func(operation string) error {
96- if operation == "close source snapshot parent directory" {
97- return closeCause
98- }
99- return nil
100- }
101-
102- err := repo.installImmutable(oldPath, newPath)
103- if !errors.Is(err, syscall.EEXIST) || !errors.Is(err, closeCause) {
104- t.Fatalf("installImmutable error = %v, want link and close failures", err)
105- }
106- if !strings.Contains(err.Error(), "close source snapshot parent directory") {
107- t.Fatalf("installImmutable error = %v, want close operation context", err)
108- }
109-}
110-
11178 func TestWriteMutableDoesNotRemoveConsumedTemporaryFile(t *testing.T) {
11279 repo := NewRepository(privateDir(t))
11380 path := filepath.Join(repo.dir, "HEAD")
@@ -5,8 +5,6 @@ import (
5 "errors"5 "errors"
6 "os"6 "os"
7 "path/filepath"7 "path/filepath"
8- "strings"
9- "syscall"
10 "testing"8 "testing"
11 )9 )
12 10
@@ -77,37 +75,6 @@ func TestWithAtomicTempPreservesLifecycleOrder(t *testing.T) {
77 }75 }
78 }76 }
79 77
80-func TestInstallImmutableJoinsPrimaryAndParentCloseErrors(t *testing.T) {
81- repo := NewRepository(privateDir(t))
82- dir := filepath.Join(repo.dir, "objects")
83- if err := repo.ensurePrivateDirectory(dir); err != nil {
84- t.Fatal(err)
85- }
86- oldPath := filepath.Join(dir, "old")
87- newPath := filepath.Join(dir, "new")
88- if err := os.WriteFile(oldPath, []byte("old"), 0o600); err != nil {
89- t.Fatal(err)
90- }
91- if err := os.WriteFile(newPath, []byte("new"), 0o600); err != nil {
92- t.Fatal(err)
93- }
94- closeCause := errors.New("injected source parent close failure")
95- repo.hooks.closeDescriptor = func(operation string) error {
96- if operation == "close source snapshot parent directory" {
97- return closeCause
98- }
99- return nil
100- }
101-
102- err := repo.installImmutable(oldPath, newPath)
103- if !errors.Is(err, syscall.EEXIST) || !errors.Is(err, closeCause) {
104- t.Fatalf("installImmutable error = %v, want link and close failures", err)
105- }
106- if !strings.Contains(err.Error(), "close source snapshot parent directory") {
107- t.Fatalf("installImmutable error = %v, want close operation context", err)
108- }
109-}
110-
111 func TestWriteMutableDoesNotRemoveConsumedTemporaryFile(t *testing.T) {78 func TestWriteMutableDoesNotRemoveConsumedTemporaryFile(t *testing.T) {
112 repo := NewRepository(privateDir(t))79 repo := NewRepository(privateDir(t))
113 path := filepath.Join(repo.dir, "HEAD")80 path := filepath.Join(repo.dir, "HEAD")
M internal/adapters/snapshot/dirent_cookie_darwin.go +0 -7
@@ -3,7 +3,6 @@
33 package snapshot
44
55 import (
6- "errors"
76 "io"
87 "syscall"
98 "unsafe"
@@ -20,9 +19,3 @@ func directoryCookie(file maintenanceDirectory, _ *syscall.Dirent) (int64, error
2019 }
2120
2221 func drainMaintenanceDirentBatch() bool { return true }
23-
24-// Darwin reports EPERM, rather than EISDIR, when unlinkat without
25-// AT_REMOVEDIR targets a directory. The mode check preserves EPERM for files.
26-func directoryUnlinkRetry(err error, mode uint32) bool {
27- return errors.Is(err, syscall.EPERM) && mode&syscall.S_IFMT == syscall.S_IFDIR
28-}
@@ -3,7 +3,6 @@
3 package snapshot3 package snapshot
4 4
5 import (5 import (
6- "errors"
7 "io"6 "io"
8 "syscall"7 "syscall"
9 "unsafe"8 "unsafe"
@@ -20,9 +19,3 @@ func directoryCookie(file maintenanceDirectory, _ *syscall.Dirent) (int64, error
20 }19 }
21 20
22 func drainMaintenanceDirentBatch() bool { return true }21 func drainMaintenanceDirentBatch() bool { return true }
23-
24-// Darwin reports EPERM, rather than EISDIR, when unlinkat without
25-// AT_REMOVEDIR targets a directory. The mode check preserves EPERM for files.
26-func directoryUnlinkRetry(err error, mode uint32) bool {
27- return errors.Is(err, syscall.EPERM) && mode&syscall.S_IFMT == syscall.S_IFDIR
28-}
M internal/adapters/snapshot/dirent_cookie_darwin_test.go +0 -9
@@ -18,12 +18,3 @@ func TestDirectoryCookieUsesDescriptorEntryCount(t *testing.T) {
1818 t.Fatalf("directoryCookie() = %d, want descriptor entry count 42", got)
1919 }
2020 }
21-
22-func TestDirectoryUnlinkRetryRequiresDarwinDirectoryEPERM(t *testing.T) {
23- if directoryUnlinkRetry(syscall.EPERM, syscall.S_IFREG) {
24- t.Fatal("directoryUnlinkRetry(EPERM, regular file) = true, want false")
25- }
26- if !directoryUnlinkRetry(syscall.EPERM, syscall.S_IFDIR) {
27- t.Fatal("directoryUnlinkRetry(EPERM, directory) = false, want true")
28- }
29-}
@@ -18,12 +18,3 @@ func TestDirectoryCookieUsesDescriptorEntryCount(t *testing.T) {
18 t.Fatalf("directoryCookie() = %d, want descriptor entry count 42", got)18 t.Fatalf("directoryCookie() = %d, want descriptor entry count 42", got)
19 }19 }
20 }20 }
21-
22-func TestDirectoryUnlinkRetryRequiresDarwinDirectoryEPERM(t *testing.T) {
23- if directoryUnlinkRetry(syscall.EPERM, syscall.S_IFREG) {
24- t.Fatal("directoryUnlinkRetry(EPERM, regular file) = true, want false")
25- }
26- if !directoryUnlinkRetry(syscall.EPERM, syscall.S_IFDIR) {
27- t.Fatal("directoryUnlinkRetry(EPERM, directory) = false, want true")
28- }
29-}
M internal/adapters/snapshot/dirent_cookie_linux.go +1 -6
@@ -2,10 +2,7 @@
22
33 package snapshot
44
5-import (
6- "errors"
7- "syscall"
8-)
5+import "syscall"
96
107 const maintenanceDirentBufferSize = 8192
118
@@ -15,5 +12,3 @@ func directoryCookie(_ maintenanceDirectory, dirent *syscall.Dirent) (int64, err
1512 }
1613
1714 func drainMaintenanceDirentBatch() bool { return false }
18-
19-func directoryUnlinkRetry(err error, _ uint32) bool { return errors.Is(err, syscall.EISDIR) }
@@ -2,10 +2,7 @@
2 2
3 package snapshot3 package snapshot
4 4
5-import (5+import "syscall"
6- "errors"
7- "syscall"
8-)
9 6
10 const maintenanceDirentBufferSize = 81927 const maintenanceDirentBufferSize = 8192
11 8
@@ -15,5 +12,3 @@ func directoryCookie(_ maintenanceDirectory, dirent *syscall.Dirent) (int64, err
15 }12 }
16 13
17 func drainMaintenanceDirentBatch() bool { return false }14 func drainMaintenanceDirentBatch() bool { return false }
18-
19-func directoryUnlinkRetry(err error, _ uint32) bool { return errors.Is(err, syscall.EISDIR) }
M internal/adapters/snapshot/dirent_cookie_linux_test.go +0 -9
@@ -17,12 +17,3 @@ func TestDirectoryCookieUsesLinuxOffset(t *testing.T) {
1717 t.Fatalf("directoryCookie() = %d, want 42", got)
1818 }
1919 }
20-
21-func TestDirectoryUnlinkRetryRequiresLinuxEISDIR(t *testing.T) {
22- if directoryUnlinkRetry(syscall.EPERM, syscall.S_IFDIR) {
23- t.Fatal("directoryUnlinkRetry(EPERM, directory) = true, want false")
24- }
25- if !directoryUnlinkRetry(syscall.EISDIR, syscall.S_IFDIR) {
26- t.Fatal("directoryUnlinkRetry(EISDIR, directory) = false, want true")
27- }
28-}
@@ -17,12 +17,3 @@ func TestDirectoryCookieUsesLinuxOffset(t *testing.T) {
17 t.Fatalf("directoryCookie() = %d, want 42", got)17 t.Fatalf("directoryCookie() = %d, want 42", got)
18 }18 }
19 }19 }
20-
21-func TestDirectoryUnlinkRetryRequiresLinuxEISDIR(t *testing.T) {
22- if directoryUnlinkRetry(syscall.EPERM, syscall.S_IFDIR) {
23- t.Fatal("directoryUnlinkRetry(EPERM, directory) = true, want false")
24- }
25- if !directoryUnlinkRetry(syscall.EISDIR, syscall.S_IFDIR) {
26- t.Fatal("directoryUnlinkRetry(EISDIR, directory) = false, want true")
27- }
28-}
M internal/adapters/snapshot/load.go +11 -1
@@ -262,13 +262,23 @@ func marshalHead(generation uint64, digest ports.SnapshotDigest) []byte {
262262 return out
263263 }
264264 func (r *Repository) readHead(key string) (uint64, ports.SnapshotDigest, error) {
265+ return r.readHeadWithRoot(nil, key)
266+}
267+
268+func (r *Repository) readHeadWithRoot(root *os.Root, key string) (uint64, ports.SnapshotDigest, error) {
265269 path := r.headPath(key)
266270 if hook := r.hooks.beforeHeadRead; hook != nil {
267271 if err := hook(path); err != nil {
268272 return 0, ports.SnapshotDigest{}, err
269273 }
270274 }
271- data, err := r.readBounded(path)
275+ var data []byte
276+ var err error
277+ if root == nil {
278+ data, err = r.readBounded(path)
279+ } else {
280+ data, err = r.readBoundedRoot(root, path)
281+ }
272282 if err != nil {
273283 return 0, ports.SnapshotDigest{}, err
274284 }
@@ -262,13 +262,23 @@ func marshalHead(generation uint64, digest ports.SnapshotDigest) []byte {
262 return out262 return out
263 }263 }
264 func (r *Repository) readHead(key string) (uint64, ports.SnapshotDigest, error) {264 func (r *Repository) readHead(key string) (uint64, ports.SnapshotDigest, error) {
265+ return r.readHeadWithRoot(nil, key)
266+}
267+
268+func (r *Repository) readHeadWithRoot(root *os.Root, key string) (uint64, ports.SnapshotDigest, error) {
265 path := r.headPath(key)269 path := r.headPath(key)
266 if hook := r.hooks.beforeHeadRead; hook != nil {270 if hook := r.hooks.beforeHeadRead; hook != nil {
267 if err := hook(path); err != nil {271 if err := hook(path); err != nil {
268 return 0, ports.SnapshotDigest{}, err272 return 0, ports.SnapshotDigest{}, err
269 }273 }
270 }274 }
271- data, err := r.readBounded(path)275+ var data []byte
276+ var err error
277+ if root == nil {
278+ data, err = r.readBounded(path)
279+ } else {
280+ data, err = r.readBoundedRoot(root, path)
281+ }
272 if err != nil {282 if err != nil {
273 return 0, ports.SnapshotDigest{}, err283 return 0, ports.SnapshotDigest{}, err
274 }284 }
M internal/adapters/snapshot/maintenance_cursor.go +1 -7
@@ -187,15 +187,9 @@ func (r *Repository) readMaintenanceDirentWithDrain(dir string, limit int, curso
187187 continue
188188 }
189189 if err != nil {
190- // A final symlink is not traversed and is treated as a non-directory;
191- // deletion remains dirfd-relative and therefore unlinks only the link.
192- if errors.Is(err, syscall.ELOOP) {
193- entries = append(entries, maintenanceDirEntry{name: name})
194- continue
195- }
196190 return nil, false, maintenanceDirectoryError("stat maintenance directory entry", err)
197191 }
198- entries = append(entries, maintenanceDirEntry{name: name, isDir: stat.Mode&syscall.S_IFMT == syscall.S_IFDIR})
192+ entries = append(entries, maintenanceDirEntry{name: name, isDir: stat.IsDir()})
199193 }
200194 if len(entries) >= limit {
201195 if drainBuffer && len(entries) > limit {
@@ -187,15 +187,9 @@ func (r *Repository) readMaintenanceDirentWithDrain(dir string, limit int, curso
187 continue187 continue
188 }188 }
189 if err != nil {189 if err != nil {
190- // A final symlink is not traversed and is treated as a non-directory;
191- // deletion remains dirfd-relative and therefore unlinks only the link.
192- if errors.Is(err, syscall.ELOOP) {
193- entries = append(entries, maintenanceDirEntry{name: name})
194- continue
195- }
196 return nil, false, maintenanceDirectoryError("stat maintenance directory entry", err)190 return nil, false, maintenanceDirectoryError("stat maintenance directory entry", err)
197 }191 }
198- entries = append(entries, maintenanceDirEntry{name: name, isDir: stat.Mode&syscall.S_IFMT == syscall.S_IFDIR})192+ entries = append(entries, maintenanceDirEntry{name: name, isDir: stat.IsDir()})
199 }193 }
200 if len(entries) >= limit {194 if len(entries) >= limit {
201 if drainBuffer && len(entries) > limit {195 if drainBuffer && len(entries) > limit {
M internal/adapters/snapshot/maintenance_delete.go +2 -3
@@ -8,7 +8,6 @@ import (
88 "os"
99 "path/filepath"
1010 "strings"
11- "syscall"
1211 )
1312
1413 // Delete makes a session unavailable by durably moving it out of the canonical
@@ -142,10 +141,10 @@ func (r *Repository) maintainQuarantine(ctx context.Context, budget *int) (chang
142141 state.current = filepath.Dir(state.current)
143142 continue
144143 }
145- if err != nil && !errors.Is(err, syscall.ELOOP) {
144+ if err != nil {
146145 return changed, false, err
147146 }
148- if err != nil || stat.Mode&syscall.S_IFMT != syscall.S_IFDIR {
147+ if !stat.IsDir() {
149148 if !r.consumeQuarantineWork(budget, "remove") {
150149 return changed, false, nil
151150 }
@@ -8,7 +8,6 @@ import (
8 "os"8 "os"
9 "path/filepath"9 "path/filepath"
10 "strings"10 "strings"
11- "syscall"
12 )11 )
13 12
14 // Delete makes a session unavailable by durably moving it out of the canonical13 // Delete makes a session unavailable by durably moving it out of the canonical
@@ -142,10 +141,10 @@ func (r *Repository) maintainQuarantine(ctx context.Context, budget *int) (chang
142 state.current = filepath.Dir(state.current)141 state.current = filepath.Dir(state.current)
143 continue142 continue
144 }143 }
145- if err != nil && !errors.Is(err, syscall.ELOOP) {144+ if err != nil {
146 return changed, false, err145 return changed, false, err
147 }146 }
148- if err != nil || stat.Mode&syscall.S_IFMT != syscall.S_IFDIR {147+ if !stat.IsDir() {
149 if !r.consumeQuarantineWork(budget, "remove") {148 if !r.consumeQuarantineWork(budget, "remove") {
150 return changed, false, nil149 return changed, false, nil
151 }150 }
M internal/adapters/snapshot/maintenance_error.go +4 -0
@@ -26,6 +26,10 @@ func (e safePathError) Unwrap() error { return e.cause }
2626 // safeFilesystemError preserves error matching without including a filesystem
2727 // path from terminal-facing error text.
2828 func safeFilesystemError(err error) error {
29+ var linkError *os.LinkError
30+ if errors.As(err, &linkError) {
31+ return safePathError{op: linkError.Op, cause: linkError.Err}
32+ }
2933 var pathError *os.PathError
3034 if errors.As(err, &pathError) {
3135 return safePathError{op: pathError.Op, cause: pathError.Err}
@@ -26,6 +26,10 @@ func (e safePathError) Unwrap() error { return e.cause }
26 // safeFilesystemError preserves error matching without including a filesystem26 // safeFilesystemError preserves error matching without including a filesystem
27 // path from terminal-facing error text.27 // path from terminal-facing error text.
28 func safeFilesystemError(err error) error {28 func safeFilesystemError(err error) error {
29+ var linkError *os.LinkError
30+ if errors.As(err, &linkError) {
31+ return safePathError{op: linkError.Op, cause: linkError.Err}
32+ }
29 var pathError *os.PathError33 var pathError *os.PathError
30 if errors.As(err, &pathError) {34 if errors.As(err, &pathError) {
31 return safePathError{op: pathError.Op, cause: pathError.Err}35 return safePathError{op: pathError.Op, cause: pathError.Err}
M internal/adapters/snapshot/path_security_test.go +146 -14
@@ -7,9 +7,144 @@ import (
77 "os"
88 "path/filepath"
99 "strings"
10+ "syscall"
1011 "testing"
1112 )
1213
14+func TestRepositoryRejectsSymlinkedRoot(t *testing.T) {
15+ configuredRoot := filepath.Join(t.TempDir(), "configured")
16+ externalRoot := privateDir(t)
17+ if err := os.Symlink(externalRoot, configuredRoot); err != nil {
18+ t.Fatal(err)
19+ }
20+ if _, err := NewRepository(configuredRoot).openRoot(); err == nil {
21+ t.Fatal("openRoot succeeded through configured-root symlink")
22+ }
23+
24+ configuredRoot = privateDir(t)
25+ repo := NewRepository(configuredRoot)
26+ publication := repositoryPublication(t, "named", 1, []byte("configured"))
27+ if err := repo.Publish(context.Background(), publication); err != nil {
28+ t.Fatal(err)
29+ }
30+ external := NewRepository(externalRoot)
31+ externalPublication := repositoryPublication(t, publication.Name, 1, []byte("external"))
32+ if err := external.Publish(context.Background(), externalPublication); err != nil {
33+ t.Fatal(err)
34+ }
35+ if err := os.Rename(configuredRoot, configuredRoot+"-real"); err != nil {
36+ t.Fatal(err)
37+ }
38+ if err := os.Symlink(externalRoot, configuredRoot); err != nil {
39+ t.Fatal(err)
40+ }
41+
42+ if _, err := repo.Load(context.Background(), publication.Name); err == nil {
43+ t.Fatal("Load succeeded through replaced configured-root symlink")
44+ }
45+ if err := repo.Maintain(context.Background()); err == nil {
46+ t.Fatal("Maintain succeeded through replaced configured-root symlink")
47+ }
48+ loaded, err := external.Load(context.Background(), publication.Name)
49+ if err != nil {
50+ t.Fatal(err)
51+ }
52+ if string(loaded.Manifest) != string(externalPublication.Manifest) {
53+ t.Fatal("external repository changed")
54+ }
55+}
56+
57+func TestOpenRootRejectsUnsafePinnedRoot(t *testing.T) {
58+ dir := privateDir(t)
59+ if err := os.Mkdir(dir, 0o700); err != nil {
60+ t.Fatal(err)
61+ }
62+ if err := os.Chmod(dir, 0o755); err != nil {
63+ t.Fatal(err)
64+ }
65+ if _, err := NewRepository(dir).openRoot(); err == nil || !strings.Contains(err.Error(), "not private") {
66+ t.Fatalf("openRoot error = %v, want private-directory rejection", err)
67+ }
68+}
69+
70+func TestOpenRootDetectsReplacementAfterOpen(t *testing.T) {
71+ dir := privateDir(t)
72+ if err := os.Mkdir(dir, 0o700); err != nil {
73+ t.Fatal(err)
74+ }
75+ old := dir + "-old"
76+ replacementMarker := filepath.Join(dir, "replacement-marker")
77+ repo := NewRepository(dir)
78+ repo.hooks.afterOpenRoot = func() {
79+ repo.hooks.afterOpenRoot = nil
80+ if err := os.Rename(dir, old); err != nil {
81+ t.Fatal(err)
82+ }
83+ if err := os.Mkdir(dir, 0o700); err != nil {
84+ t.Fatal(err)
85+ }
86+ if err := os.WriteFile(replacementMarker, []byte("unchanged"), 0o600); err != nil {
87+ t.Fatal(err)
88+ }
89+ }
90+ if _, err := repo.openRoot(); !errors.Is(err, syscall.ESTALE) {
91+ t.Fatalf("openRoot error = %v, want ESTALE", err)
92+ }
93+ if got, err := os.ReadFile(replacementMarker); err != nil || string(got) != "unchanged" {
94+ t.Fatalf("replacement marker = %q, %v", got, err)
95+ }
96+ if _, err := os.Stat(filepath.Join(old, "replacement-marker")); !errors.Is(err, os.ErrNotExist) {
97+ t.Fatalf("pinned tree was operated on: %v", err)
98+ }
99+}
100+
101+func TestOpenDirectoryClosesChildWhenRootCloseFails(t *testing.T) {
102+ repo := NewRepository(privateDir(t))
103+ if err := os.Mkdir(repo.dir, 0o700); err != nil {
104+ t.Fatal(err)
105+ }
106+ child := filepath.Join(repo.dir, "child")
107+ if err := os.Mkdir(child, 0o700); err != nil {
108+ t.Fatal(err)
109+ }
110+ closeErr := errors.New("injected root close")
111+ repo.hooks.closeRoot = func() error { return closeErr }
112+ before := openDescriptorCount(t)
113+ for range 20 {
114+ file, err := repo.openDirectory(child)
115+ if file != nil || !errors.Is(err, closeErr) {
116+ t.Fatalf("openDirectory = (%v, %v), want nil and close error", file, err)
117+ }
118+ }
119+ if got := openDescriptorCount(t); got > before+4 {
120+ t.Fatalf("open descriptors after failed openDirectory = %d, before = %d", got, before)
121+ }
122+}
123+
124+func TestRootCloseErrorsJoinAndCloseRealRoot(t *testing.T) {
125+ repo := NewRepository(privateDir(t))
126+ if err := os.Mkdir(repo.dir, 0o700); err != nil {
127+ t.Fatal(err)
128+ }
129+ closeErr := errors.New("injected root close")
130+ repo.hooks.closeRoot = func() error { return closeErr }
131+ root, err := repo.openRoot()
132+ if err != nil {
133+ t.Fatal(err)
134+ }
135+ if err := repo.closeRoot(root); !errors.Is(err, closeErr) {
136+ t.Fatalf("closeRoot error = %v, want injected error", err)
137+ }
138+ if _, err := root.Stat("."); err == nil {
139+ t.Fatal("real root remained open after injected close error")
140+ }
141+
142+ _, err = repo.stat(filepath.Join(repo.dir, "missing"))
143+ if !errors.Is(err, closeErr) || !errors.Is(err, os.ErrNotExist) {
144+ t.Fatalf("stat error = %v, want primary and close errors", err)
145+ }
146+}
147+
13148 func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) {
14149 for _, target := range []string{repositoryGenerations, repositoryObjectsDir} {
15150 t.Run(target, func(t *testing.T) {
@@ -34,7 +169,6 @@ func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) {
34169 if _, err := repo.Load(context.Background(), publication.Name); err == nil {
35170 t.Fatal("Load succeeded through symlinked repository component")
36171 }
37- // Maintenance must reject the replacement rather than following it.
38172 if err := repo.Maintain(context.Background()); err == nil {
39173 t.Fatal("Maintain succeeded through symlinked repository component")
40174 }
@@ -46,26 +180,24 @@ func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) {
46180 }
47181 }
48182
49-func TestStatReturnsInjectedCloseErrorWithContext(t *testing.T) {
183+func TestFinalSymlinksRejectedByRootOperations(t *testing.T) {
50184 repo := NewRepository(privateDir(t))
51185 if err := repo.ensurePrivateDirectory(repo.dir); err != nil {
52186 t.Fatal(err)
53187 }
54- path := filepath.Join(repo.dir, "state")
55- if err := os.WriteFile(path, []byte("state"), 0o600); err != nil {
188+ target := filepath.Join(repo.dir, "target")
189+ if err := os.Mkdir(target, 0o700); err != nil {
56190 t.Fatal(err)
57191 }
58- closeCause := errors.New("injected stat close failure")
59- repo.hooks.closeDescriptor = func(got string) error {
60- if got == "close snapshot file" {
61- return closeCause
62- }
63- return nil
192+ link := filepath.Join(repo.dir, "link")
193+ if err := os.Symlink(target, link); err != nil {
194+ t.Fatal(err)
64195 }
65-
66- _, err := repo.stat(path)
67- if !errors.Is(err, closeCause) || !strings.Contains(err.Error(), "close snapshot file") {
68- t.Fatalf("stat error = %v, want contextual close failure", err)
196+ if _, err := repo.openDirectory(link); !errors.Is(err, syscall.ELOOP) || strings.Contains(err.Error(), repo.dir) {
197+ t.Fatalf("openDirectory error = %v, want sanitized ELOOP", err)
198+ }
199+ if _, err := repo.readBounded(link); !errors.Is(err, syscall.ELOOP) || strings.Contains(err.Error(), repo.dir) {
200+ t.Fatalf("readBounded error = %v, want sanitized ELOOP", err)
69201 }
70202 }
71203
@@ -7,9 +7,144 @@ import (
7 "os"7 "os"
8 "path/filepath"8 "path/filepath"
9 "strings"9 "strings"
10+ "syscall"
10 "testing"11 "testing"
11 )12 )
12 13
14+func TestRepositoryRejectsSymlinkedRoot(t *testing.T) {
15+ configuredRoot := filepath.Join(t.TempDir(), "configured")
16+ externalRoot := privateDir(t)
17+ if err := os.Symlink(externalRoot, configuredRoot); err != nil {
18+ t.Fatal(err)
19+ }
20+ if _, err := NewRepository(configuredRoot).openRoot(); err == nil {
21+ t.Fatal("openRoot succeeded through configured-root symlink")
22+ }
23+
24+ configuredRoot = privateDir(t)
25+ repo := NewRepository(configuredRoot)
26+ publication := repositoryPublication(t, "named", 1, []byte("configured"))
27+ if err := repo.Publish(context.Background(), publication); err != nil {
28+ t.Fatal(err)
29+ }
30+ external := NewRepository(externalRoot)
31+ externalPublication := repositoryPublication(t, publication.Name, 1, []byte("external"))
32+ if err := external.Publish(context.Background(), externalPublication); err != nil {
33+ t.Fatal(err)
34+ }
35+ if err := os.Rename(configuredRoot, configuredRoot+"-real"); err != nil {
36+ t.Fatal(err)
37+ }
38+ if err := os.Symlink(externalRoot, configuredRoot); err != nil {
39+ t.Fatal(err)
40+ }
41+
42+ if _, err := repo.Load(context.Background(), publication.Name); err == nil {
43+ t.Fatal("Load succeeded through replaced configured-root symlink")
44+ }
45+ if err := repo.Maintain(context.Background()); err == nil {
46+ t.Fatal("Maintain succeeded through replaced configured-root symlink")
47+ }
48+ loaded, err := external.Load(context.Background(), publication.Name)
49+ if err != nil {
50+ t.Fatal(err)
51+ }
52+ if string(loaded.Manifest) != string(externalPublication.Manifest) {
53+ t.Fatal("external repository changed")
54+ }
55+}
56+
57+func TestOpenRootRejectsUnsafePinnedRoot(t *testing.T) {
58+ dir := privateDir(t)
59+ if err := os.Mkdir(dir, 0o700); err != nil {
60+ t.Fatal(err)
61+ }
62+ if err := os.Chmod(dir, 0o755); err != nil {
63+ t.Fatal(err)
64+ }
65+ if _, err := NewRepository(dir).openRoot(); err == nil || !strings.Contains(err.Error(), "not private") {
66+ t.Fatalf("openRoot error = %v, want private-directory rejection", err)
67+ }
68+}
69+
70+func TestOpenRootDetectsReplacementAfterOpen(t *testing.T) {
71+ dir := privateDir(t)
72+ if err := os.Mkdir(dir, 0o700); err != nil {
73+ t.Fatal(err)
74+ }
75+ old := dir + "-old"
76+ replacementMarker := filepath.Join(dir, "replacement-marker")
77+ repo := NewRepository(dir)
78+ repo.hooks.afterOpenRoot = func() {
79+ repo.hooks.afterOpenRoot = nil
80+ if err := os.Rename(dir, old); err != nil {
81+ t.Fatal(err)
82+ }
83+ if err := os.Mkdir(dir, 0o700); err != nil {
84+ t.Fatal(err)
85+ }
86+ if err := os.WriteFile(replacementMarker, []byte("unchanged"), 0o600); err != nil {
87+ t.Fatal(err)
88+ }
89+ }
90+ if _, err := repo.openRoot(); !errors.Is(err, syscall.ESTALE) {
91+ t.Fatalf("openRoot error = %v, want ESTALE", err)
92+ }
93+ if got, err := os.ReadFile(replacementMarker); err != nil || string(got) != "unchanged" {
94+ t.Fatalf("replacement marker = %q, %v", got, err)
95+ }
96+ if _, err := os.Stat(filepath.Join(old, "replacement-marker")); !errors.Is(err, os.ErrNotExist) {
97+ t.Fatalf("pinned tree was operated on: %v", err)
98+ }
99+}
100+
101+func TestOpenDirectoryClosesChildWhenRootCloseFails(t *testing.T) {
102+ repo := NewRepository(privateDir(t))
103+ if err := os.Mkdir(repo.dir, 0o700); err != nil {
104+ t.Fatal(err)
105+ }
106+ child := filepath.Join(repo.dir, "child")
107+ if err := os.Mkdir(child, 0o700); err != nil {
108+ t.Fatal(err)
109+ }
110+ closeErr := errors.New("injected root close")
111+ repo.hooks.closeRoot = func() error { return closeErr }
112+ before := openDescriptorCount(t)
113+ for range 20 {
114+ file, err := repo.openDirectory(child)
115+ if file != nil || !errors.Is(err, closeErr) {
116+ t.Fatalf("openDirectory = (%v, %v), want nil and close error", file, err)
117+ }
118+ }
119+ if got := openDescriptorCount(t); got > before+4 {
120+ t.Fatalf("open descriptors after failed openDirectory = %d, before = %d", got, before)
121+ }
122+}
123+
124+func TestRootCloseErrorsJoinAndCloseRealRoot(t *testing.T) {
125+ repo := NewRepository(privateDir(t))
126+ if err := os.Mkdir(repo.dir, 0o700); err != nil {
127+ t.Fatal(err)
128+ }
129+ closeErr := errors.New("injected root close")
130+ repo.hooks.closeRoot = func() error { return closeErr }
131+ root, err := repo.openRoot()
132+ if err != nil {
133+ t.Fatal(err)
134+ }
135+ if err := repo.closeRoot(root); !errors.Is(err, closeErr) {
136+ t.Fatalf("closeRoot error = %v, want injected error", err)
137+ }
138+ if _, err := root.Stat("."); err == nil {
139+ t.Fatal("real root remained open after injected close error")
140+ }
141+
142+ _, err = repo.stat(filepath.Join(repo.dir, "missing"))
143+ if !errors.Is(err, closeErr) || !errors.Is(err, os.ErrNotExist) {
144+ t.Fatalf("stat error = %v, want primary and close errors", err)
145+ }
146+}
147+
13 func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) {148 func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) {
14 for _, target := range []string{repositoryGenerations, repositoryObjectsDir} {149 for _, target := range []string{repositoryGenerations, repositoryObjectsDir} {
15 t.Run(target, func(t *testing.T) {150 t.Run(target, func(t *testing.T) {
@@ -34,7 +169,6 @@ func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) {
34 if _, err := repo.Load(context.Background(), publication.Name); err == nil {169 if _, err := repo.Load(context.Background(), publication.Name); err == nil {
35 t.Fatal("Load succeeded through symlinked repository component")170 t.Fatal("Load succeeded through symlinked repository component")
36 }171 }
37- // Maintenance must reject the replacement rather than following it.
38 if err := repo.Maintain(context.Background()); err == nil {172 if err := repo.Maintain(context.Background()); err == nil {
39 t.Fatal("Maintain succeeded through symlinked repository component")173 t.Fatal("Maintain succeeded through symlinked repository component")
40 }174 }
@@ -46,26 +180,24 @@ func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) {
46 }180 }
47 }181 }
48 182
49-func TestStatReturnsInjectedCloseErrorWithContext(t *testing.T) {183+func TestFinalSymlinksRejectedByRootOperations(t *testing.T) {
50 repo := NewRepository(privateDir(t))184 repo := NewRepository(privateDir(t))
51 if err := repo.ensurePrivateDirectory(repo.dir); err != nil {185 if err := repo.ensurePrivateDirectory(repo.dir); err != nil {
52 t.Fatal(err)186 t.Fatal(err)
53 }187 }
54- path := filepath.Join(repo.dir, "state")188+ target := filepath.Join(repo.dir, "target")
55- if err := os.WriteFile(path, []byte("state"), 0o600); err != nil {189+ if err := os.Mkdir(target, 0o700); err != nil {
56 t.Fatal(err)190 t.Fatal(err)
57 }191 }
58- closeCause := errors.New("injected stat close failure")192+ link := filepath.Join(repo.dir, "link")
59- repo.hooks.closeDescriptor = func(got string) error {193+ if err := os.Symlink(target, link); err != nil {
60- if got == "close snapshot file" {194+ t.Fatal(err)
61- return closeCause
62- }
63- return nil
64 }195 }
65-196+ if _, err := repo.openDirectory(link); !errors.Is(err, syscall.ELOOP) || strings.Contains(err.Error(), repo.dir) {
66- _, err := repo.stat(path)197+ t.Fatalf("openDirectory error = %v, want sanitized ELOOP", err)
67- if !errors.Is(err, closeCause) || !strings.Contains(err.Error(), "close snapshot file") {198+ }
68- t.Fatalf("stat error = %v, want contextual close failure", err)199+ if _, err := repo.readBounded(link); !errors.Is(err, syscall.ELOOP) || strings.Contains(err.Error(), repo.dir) {
200+ t.Fatalf("readBounded error = %v, want sanitized ELOOP", err)
69 }201 }
70 }202 }
71 203
M internal/adapters/snapshot/publish.go +20 -4
@@ -37,10 +37,8 @@ func (r *Repository) Publish(ctx context.Context, publication ports.SnapshotPubl
3737 // The common replay path needs neither a manifest decode nor a reference
3838 // map. Checking the immutable bytes directly also keeps its descriptor
3939 // reads beneath the pinned repository root.
40- if generation, _, err := r.readHead(key); err == nil && generation == publication.Generation {
41- if current, err := r.readBounded(r.manifestPath(key, generation)); err == nil && bytes.Equal(current, publication.Manifest) {
42- return nil
43- }
40+ if unchanged, err := r.unchangedPublication(key, publication); err == nil && unchanged {
41+ return nil
4442 }
4543
4644 current, currentManifest, currentRefs, err := r.currentPublication(ctx, publication.Name, key)
@@ -147,6 +145,24 @@ func (r *Repository) Publish(ctx context.Context, publication ports.SnapshotPubl
147145 return nil
148146 }
149147
148+// unchangedPublication uses one root for the two reads in the replay fast
149+// path. The root is confined to this operation and is closed before return;
150+// all other repository operations retain their open-per-operation roots.
151+func (r *Repository) unchangedPublication(key string, publication ports.SnapshotPublication) (unchanged bool, err error) {
152+ root, err := r.openRoot()
153+ if err != nil {
154+ return false, err
155+ }
156+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
157+
158+ generation, _, err := r.readHeadWithRoot(root, key)
159+ if err != nil || generation != publication.Generation {
160+ return false, nil
161+ }
162+ current, err := r.readBoundedRoot(root, r.manifestPath(key, generation))
163+ return err == nil && bytes.Equal(current, publication.Manifest), nil
164+}
165+
150166 // currentPublication takes the normal HEAD path without loading every object
151167 // in the current generation. A damaged HEAD or manifest still uses the full
152168 // fallback scan, because only Load's object validation can establish a safe
@@ -37,10 +37,8 @@ func (r *Repository) Publish(ctx context.Context, publication ports.SnapshotPubl
37 // The common replay path needs neither a manifest decode nor a reference37 // The common replay path needs neither a manifest decode nor a reference
38 // map. Checking the immutable bytes directly also keeps its descriptor38 // map. Checking the immutable bytes directly also keeps its descriptor
39 // reads beneath the pinned repository root.39 // reads beneath the pinned repository root.
40- if generation, _, err := r.readHead(key); err == nil && generation == publication.Generation {40+ if unchanged, err := r.unchangedPublication(key, publication); err == nil && unchanged {
41- if current, err := r.readBounded(r.manifestPath(key, generation)); err == nil && bytes.Equal(current, publication.Manifest) {41+ return nil
42- return nil
43- }
44 }42 }
45 43
46 current, currentManifest, currentRefs, err := r.currentPublication(ctx, publication.Name, key)44 current, currentManifest, currentRefs, err := r.currentPublication(ctx, publication.Name, key)
@@ -147,6 +145,24 @@ func (r *Repository) Publish(ctx context.Context, publication ports.SnapshotPubl
147 return nil145 return nil
148 }146 }
149 147
148+// unchangedPublication uses one root for the two reads in the replay fast
149+// path. The root is confined to this operation and is closed before return;
150+// all other repository operations retain their open-per-operation roots.
151+func (r *Repository) unchangedPublication(key string, publication ports.SnapshotPublication) (unchanged bool, err error) {
152+ root, err := r.openRoot()
153+ if err != nil {
154+ return false, err
155+ }
156+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
157+
158+ generation, _, err := r.readHeadWithRoot(root, key)
159+ if err != nil || generation != publication.Generation {
160+ return false, nil
161+ }
162+ current, err := r.readBoundedRoot(root, r.manifestPath(key, generation))
163+ return err == nil && bytes.Equal(current, publication.Manifest), nil
164+}
165+
150 // currentPublication takes the normal HEAD path without loading every object166 // currentPublication takes the normal HEAD path without loading every object
151 // in the current generation. A damaged HEAD or manifest still uses the full167 // in the current generation. A damaged HEAD or manifest still uses the full
152 // fallback scan, because only Load's object validation can establish a safe168 // fallback scan, because only Load's object validation can establish a safe
M internal/adapters/snapshot/publish_efficiency_test.go +6 -2
@@ -35,13 +35,17 @@ func TestRepositoryPublishSkipsUnchangedTenThousandObjectHistory(t *testing.T) {
3535
3636 // Replaying the now-current generation still parses the 10k references, but
3737 // must not allocate one object-sized buffer per retained history entry.
38+ // The limit is 450, not lower: the os.Root-based fast path pays a constant
39+ // allocation cost for guarded reads and root identity validation, and -race
40+ // adds further overhead. A per-retained-object regression would exceed this
41+ // by orders of magnitude.
3842 allocations := testing.AllocsPerRun(3, func() {
3943 if err := repo.Publish(context.Background(), second); err != nil {
4044 panic(err)
4145 }
4246 })
43- if allocations > 300 {
44- t.Fatalf("unchanged 10k history Publish allocations = %.0f, want <= 300; retained objects were likely copied", allocations)
47+ if allocations > 450 {
48+ t.Fatalf("unchanged 10k history Publish allocations = %.0f, want <= 450; retained objects were likely copied", allocations)
4549 }
4650
4751 got, err := repo.Load(context.Background(), second.Name)
@@ -35,13 +35,17 @@ func TestRepositoryPublishSkipsUnchangedTenThousandObjectHistory(t *testing.T) {
35 35
36 // Replaying the now-current generation still parses the 10k references, but36 // Replaying the now-current generation still parses the 10k references, but
37 // must not allocate one object-sized buffer per retained history entry.37 // must not allocate one object-sized buffer per retained history entry.
38+ // The limit is 450, not lower: the os.Root-based fast path pays a constant
39+ // allocation cost for guarded reads and root identity validation, and -race
40+ // adds further overhead. A per-retained-object regression would exceed this
41+ // by orders of magnitude.
38 allocations := testing.AllocsPerRun(3, func() {42 allocations := testing.AllocsPerRun(3, func() {
39 if err := repo.Publish(context.Background(), second); err != nil {43 if err := repo.Publish(context.Background(), second); err != nil {
40 panic(err)44 panic(err)
41 }45 }
42 })46 })
43- if allocations > 300 {47+ if allocations > 450 {
44- t.Fatalf("unchanged 10k history Publish allocations = %.0f, want <= 300; retained objects were likely copied", allocations)48+ t.Fatalf("unchanged 10k history Publish allocations = %.0f, want <= 450; retained objects were likely copied", allocations)
45 }49 }
46 50
47 got, err := repo.Load(context.Background(), second.Name)51 got, err := repo.Load(context.Background(), second.Name)
M internal/adapters/snapshot/repository.go +5 -2
@@ -67,12 +67,15 @@ type repositoryHooks struct {
6767 writeTemp func(string) error
6868 syncFile func(string) error
6969 closeFile func(string) error
70- closeDescriptor func(string) error
7170 installImmutable func(string) error
7271 rename func(string) error
7372 syncDirectory func(string) error
7473 remove func(string) error
75- openMaintenanceDirectory func(string) (maintenanceDirectory, error)
74+ // afterOpenRoot and closeRoot make descriptor race and close-error paths
75+ // deterministic in package tests. closeRoot never replaces the real close.
76+ afterOpenRoot func()
77+ closeRoot func() error
78+ openMaintenanceDirectory func(string) (maintenanceDirectory, error)
7679 // beforeMaintenanceWork observes each budgeted quarantine filesystem step.
7780 // It is test-only instrumentation for hostile traversal bounds.
7881 beforeMaintenanceWork func(string)
@@ -67,12 +67,15 @@ type repositoryHooks struct {
67 writeTemp func(string) error67 writeTemp func(string) error
68 syncFile func(string) error68 syncFile func(string) error
69 closeFile func(string) error69 closeFile func(string) error
70- closeDescriptor func(string) error
71 installImmutable func(string) error70 installImmutable func(string) error
72 rename func(string) error71 rename func(string) error
73 syncDirectory func(string) error72 syncDirectory func(string) error
74 remove func(string) error73 remove func(string) error
75- openMaintenanceDirectory func(string) (maintenanceDirectory, error)74+ // afterOpenRoot and closeRoot make descriptor race and close-error paths
75+ // deterministic in package tests. closeRoot never replaces the real close.
76+ afterOpenRoot func()
77+ closeRoot func() error
78+ openMaintenanceDirectory func(string) (maintenanceDirectory, error)
76 // beforeMaintenanceWork observes each budgeted quarantine filesystem step.79 // beforeMaintenanceWork observes each budgeted quarantine filesystem step.
77 // It is test-only instrumentation for hostile traversal bounds.80 // It is test-only instrumentation for hostile traversal bounds.
78 beforeMaintenanceWork func(string)81 beforeMaintenanceWork func(string)
M internal/adapters/snapshot/repository_maintenance_test.go +7 -0
@@ -17,6 +17,13 @@ func TestSafeFilesystemErrorDoesNotExposePath(t *testing.T) {
1717 }
1818 }
1919
20+func TestSafeFilesystemErrorDoesNotExposeLinkErrorPath(t *testing.T) {
21+ err := safeFilesystemError(&os.LinkError{Op: "rename", Old: "/unsafe/old", New: "/unsafe/new", Err: os.ErrExist})
22+ if !errors.Is(err, os.ErrExist) || strings.Contains(err.Error(), "/unsafe/") {
23+ t.Fatalf("safe link error = %q", err)
24+ }
25+}
26+
2027 func TestRepositoryMaintenanceFilesystemErrorsHaveSafeContext(t *testing.T) {
2128 injected := errors.New("injected filesystem failure")
2229 cases := []struct {
@@ -17,6 +17,13 @@ func TestSafeFilesystemErrorDoesNotExposePath(t *testing.T) {
17 }17 }
18 }18 }
19 19
20+func TestSafeFilesystemErrorDoesNotExposeLinkErrorPath(t *testing.T) {
21+ err := safeFilesystemError(&os.LinkError{Op: "rename", Old: "/unsafe/old", New: "/unsafe/new", Err: os.ErrExist})
22+ if !errors.Is(err, os.ErrExist) || strings.Contains(err.Error(), "/unsafe/") {
23+ t.Fatalf("safe link error = %q", err)
24+ }
25+}
26+
20 func TestRepositoryMaintenanceFilesystemErrorsHaveSafeContext(t *testing.T) {27 func TestRepositoryMaintenanceFilesystemErrorsHaveSafeContext(t *testing.T) {
21 injected := errors.New("injected filesystem failure")28 injected := errors.New("injected filesystem failure")
22 cases := []struct {29 cases := []struct {
M internal/adapters/snapshot/safe_path.go +89 -116
@@ -9,108 +9,90 @@ import (
99 "path/filepath"
1010 "strings"
1111 "syscall"
12-
13- "golang.org/x/sys/unix"
1412 )
1513
16-// repositoryFD opens a repository-relative path one component at a time. Each
17-// directory is pinned by an O_NOFOLLOW descriptor before its child is opened,
18-// so a concurrent rename or symlink replacement cannot redirect an operation
19-// outside the repository.
20-func (r *Repository) repositoryFD(path string, finalFlags int, mode uint32) (*os.File, error) {
21- rel, ok := r.repositoryRelative(path)
22- if !ok {
23- return nil, fmt.Errorf("snapshot path outside repository")
24- }
25- root, err := syscall.Open(r.dir, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0)
14+func (r *Repository) openRoot() (*os.Root, error) {
15+ // os.OpenRoot follows a final symlink, so validate that its pinned directory
16+ // is still the non-symlink directory at the configured path.
17+ root, err := os.OpenRoot(r.dir)
2618 if err != nil {
2719 return nil, err
2820 }
29- if rel == "." {
30- if finalFlags&syscall.O_DIRECTORY == 0 {
31- err = fmt.Errorf("invalid snapshot root operation")
32- joinCloseError(&err, "close snapshot root directory", r.closeDescriptor(root, "close snapshot root directory"))
33- return nil, err
34- }
35- file := os.NewFile(uintptr(root), path)
36- if file == nil {
37- err = fmt.Errorf("open snapshot root")
38- joinCloseError(&err, "close snapshot root directory", r.closeDescriptor(root, "close snapshot root directory"))
39- return nil, err
40- }
41- return file, nil
42- }
43- parts := strings.Split(filepath.Clean(rel), string(filepath.Separator))
44- fd := root
45- for i, part := range parts {
46- if part == "" || part == "." || part == ".." {
47- err = fmt.Errorf("invalid snapshot path")
48- joinCloseError(&err, "close snapshot path directory", r.closeDescriptor(fd, "close snapshot path directory"))
49- return nil, err
50- }
51- flags := syscall.O_RDONLY | syscall.O_CLOEXEC | syscall.O_NOFOLLOW
52- if i != len(parts)-1 {
53- flags |= syscall.O_DIRECTORY
54- } else {
55- flags = finalFlags | syscall.O_CLOEXEC | syscall.O_NOFOLLOW
56- }
57- next, openErr := unix.Openat(fd, part, flags, mode)
58- closeErr := r.closeDescriptor(fd, "close snapshot path parent directory")
59- if openErr != nil {
60- err = openErr
61- joinCloseError(&err, "close snapshot path parent directory", closeErr)
62- return nil, err
63- }
64- if closeErr != nil {
65- err = fmt.Errorf("close snapshot path parent directory: %w", closeErr)
66- joinCloseError(&err, "close snapshot path child directory", r.closeDescriptor(next, "close snapshot path child directory"))
67- return nil, err
68- }
69- fd = next
21+ if hook := r.hooks.afterOpenRoot; hook != nil {
22+ hook()
7023 }
71- file := os.NewFile(uintptr(fd), path)
72- if file == nil {
73- err = fmt.Errorf("open snapshot path")
74- joinCloseError(&err, "close snapshot path descriptor", r.closeDescriptor(fd, "close snapshot path descriptor"))
75- return nil, err
24+ current, err := os.Lstat(r.dir)
25+ if err != nil {
26+ return nil, r.closeRootOnError(root, err)
27+ }
28+ if !current.IsDir() || current.Mode()&os.ModeSymlink != 0 {
29+ return nil, r.closeRootOnError(root, &os.PathError{Op: "open", Path: r.dir, Err: syscall.ELOOP})
30+ }
31+ pinned, err := root.Stat(".")
32+ if err != nil {
33+ return nil, r.closeRootOnError(root, err)
34+ }
35+ if !os.SameFile(current, pinned) {
36+ return nil, r.closeRootOnError(root, &os.PathError{Op: "open", Path: r.dir, Err: syscall.ESTALE})
37+ }
38+ if !privateDirectory(pinned) {
39+ return nil, r.closeRootOnError(root, fmt.Errorf("snapshot directory is not private"))
7640 }
77- return file, nil
41+ return root, nil
7842 }
7943
80-func (r *Repository) repositoryParent(path string) (int, string, error) {
81- rel, ok := r.repositoryRelative(path)
82- if !ok || rel == "." {
83- return -1, "", fmt.Errorf("snapshot path outside repository")
44+func privateDirectory(fi os.FileInfo) bool {
45+ st, ok := fi.Sys().(*syscall.Stat_t)
46+ return ok && fi.IsDir() && int(st.Uid) == os.Geteuid() && fi.Mode().Perm() == 0o700
47+}
48+
49+func (r *Repository) closeRoot(root *os.Root) error {
50+ var injected error
51+ if hook := r.hooks.closeRoot; hook != nil {
52+ injected = hook()
8453 }
85- parts := strings.Split(filepath.Clean(rel), string(filepath.Separator))
86- if len(parts) == 0 || parts[len(parts)-1] == "." || parts[len(parts)-1] == ".." {
87- return -1, "", fmt.Errorf("invalid snapshot path")
54+ return errors.Join(injected, root.Close())
55+}
56+
57+func (r *Repository) closeRootOnError(root *os.Root, err error) error {
58+ return errors.Join(err, r.closeRoot(root))
59+}
60+
61+// rejectFinalSymlink preserves the pre-os.Root guarantee that the final
62+// component is never a symlink. Root confinement is the hard boundary; this
63+// is defense-in-depth with a benign TOCTOU window inside a 0700 directory.
64+func rejectFinalSymlink(root *os.Root, rel string) error {
65+ fi, err := root.Lstat(rel)
66+ if err != nil {
67+ return err
68+ }
69+ if fi.Mode()&os.ModeSymlink != 0 {
70+ return &os.PathError{Op: "open", Path: rel, Err: syscall.ELOOP}
71+ }
72+ return nil
73+}
74+
75+func (r *Repository) openDirectory(path string) (file *os.File, err error) {
76+ rel, ok := r.repositoryRelative(path)
77+ if !ok {
78+ return nil, fmt.Errorf("snapshot path outside repository")
8879 }
89- fd, err := syscall.Open(r.dir, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0)
80+ root, err := r.openRoot()
9081 if err != nil {
91- return -1, "", err
82+ return nil, err
9283 }
93- for _, part := range parts[:len(parts)-1] {
94- if part == "" || part == "." || part == ".." {
95- err = fmt.Errorf("invalid snapshot path")
96- joinCloseError(&err, "close snapshot parent directory", r.closeDescriptor(fd, "close snapshot parent directory"))
97- return -1, "", err
98- }
99- next, openErr := unix.Openat(fd, part, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0)
100- closeErr := r.closeDescriptor(fd, "close snapshot parent directory")
101- if openErr != nil {
102- err = openErr
103- joinCloseError(&err, "close snapshot parent directory", closeErr)
104- return -1, "", err
105- }
106- if closeErr != nil {
107- err = fmt.Errorf("close snapshot parent directory: %w", closeErr)
108- joinCloseError(&err, "close snapshot child directory", r.closeDescriptor(next, "close snapshot child directory"))
109- return -1, "", err
84+ defer func() {
85+ closeErr := r.closeRoot(root)
86+ if closeErr != nil && file != nil {
87+ joinCloseError(&err, "close snapshot directory after root close failure", file.Close())
88+ file = nil
11089 }
111- fd = next
90+ joinCloseError(&err, "close snapshot root", closeErr)
91+ }()
92+ if err := rejectFinalSymlink(root, rel); err != nil {
93+ return nil, err
11294 }
113- return fd, parts[len(parts)-1], nil
95+ return root.OpenFile(rel, syscall.O_RDONLY|syscall.O_DIRECTORY, 0)
11496 }
11597
11698 func (r *Repository) repositoryRelative(path string) (string, bool) {
@@ -128,38 +110,35 @@ func (r *Repository) repositoryRelative(path string) (string, bool) {
128110 return rel, true
129111 }
130112
131-func (r *Repository) openDirectory(path string) (*os.File, error) {
132- return r.repositoryFD(path, syscall.O_RDONLY|syscall.O_DIRECTORY, 0)
133-}
134-
135-func (r *Repository) stat(path string) (st syscall.Stat_t, err error) {
136- file, err := r.repositoryFD(path, syscall.O_RDONLY|syscall.O_NONBLOCK, 0)
137- if err != nil {
138- return syscall.Stat_t{}, err
113+func (r *Repository) stat(path string) (fi os.FileInfo, err error) {
114+ rel, ok := r.repositoryRelative(path)
115+ if !ok {
116+ return nil, fmt.Errorf("snapshot path outside repository")
139117 }
140- defer func() {
141- joinCloseError(&err, "close snapshot file", r.closeRepositoryFile(file, "close snapshot file"))
142- }()
143- if err := syscall.Fstat(int(file.Fd()), &st); err != nil {
144- return syscall.Stat_t{}, err
118+ root, err := r.openRoot()
119+ if err != nil {
120+ return nil, err
145121 }
146- return st, nil
122+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
123+ return root.Lstat(rel)
147124 }
148125
149126 // createTempAt creates an exclusively owned temporary file in an already
150127 // validated directory. os.CreateTemp(path) would resolve that path again.
151128 func (r *Repository) createTempAt(dir string) (temp *os.File, err error) {
152- fd, err := r.openDirectory(dir)
129+ rel, ok := r.repositoryRelative(dir)
130+ if !ok {
131+ return nil, fmt.Errorf("snapshot path outside repository")
132+ }
133+ root, err := r.openRoot()
153134 if err != nil {
154135 return nil, err
155136 }
156137 defer func() {
157- if closeErr := r.closeRepositoryFile(fd, "close snapshot temporary directory"); closeErr != nil {
158- if temp != nil {
159- joinCloseError(&err, "close snapshot temporary file after directory close failure", r.closeRepositoryFile(temp, "close snapshot temporary file after directory close failure"))
160- temp = nil
161- }
162- joinCloseError(&err, "close snapshot temporary directory", closeErr)
138+ joinCloseError(&err, "close snapshot root", r.closeRoot(root))
139+ if err != nil && temp != nil {
140+ joinCloseError(&err, "close snapshot temporary file after root close failure", temp.Close())
141+ temp = nil
163142 }
164143 }()
165144 for range 100 {
@@ -168,19 +147,13 @@ func (r *Repository) createTempAt(dir string) (temp *os.File, err error) {
168147 return nil, err
169148 }
170149 name := ".tmp-" + hex.EncodeToString(random[:])
171- tempFD, err := unix.Openat(int(fd.Fd()), name, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_EXCL|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0o600)
172- if errors.Is(err, syscall.EEXIST) {
150+ temp, err = root.OpenFile(filepath.Join(rel, name), syscall.O_WRONLY|syscall.O_CREAT|syscall.O_EXCL, 0o600)
151+ if errors.Is(err, os.ErrExist) {
173152 continue
174153 }
175154 if err != nil {
176155 return nil, err
177156 }
178- temp = os.NewFile(uintptr(tempFD), filepath.Join(dir, name))
179- if temp == nil {
180- err = fmt.Errorf("open snapshot temporary file")
181- joinCloseError(&err, "close snapshot temporary descriptor", r.closeDescriptor(tempFD, "close snapshot temporary descriptor"))
182- return nil, err
183- }
184157 return temp, nil
185158 }
186159 return nil, fmt.Errorf("create snapshot temporary file: too many collisions")
@@ -9,108 +9,90 @@ import (
9 "path/filepath"9 "path/filepath"
10 "strings"10 "strings"
11 "syscall"11 "syscall"
12-
13- "golang.org/x/sys/unix"
14 )12 )
15 13
16-// repositoryFD opens a repository-relative path one component at a time. Each14+func (r *Repository) openRoot() (*os.Root, error) {
17-// directory is pinned by an O_NOFOLLOW descriptor before its child is opened,15+ // os.OpenRoot follows a final symlink, so validate that its pinned directory
18-// so a concurrent rename or symlink replacement cannot redirect an operation16+ // is still the non-symlink directory at the configured path.
19-// outside the repository.17+ root, err := os.OpenRoot(r.dir)
20-func (r *Repository) repositoryFD(path string, finalFlags int, mode uint32) (*os.File, error) {
21- rel, ok := r.repositoryRelative(path)
22- if !ok {
23- return nil, fmt.Errorf("snapshot path outside repository")
24- }
25- root, err := syscall.Open(r.dir, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0)
26 if err != nil {18 if err != nil {
27 return nil, err19 return nil, err
28 }20 }
29- if rel == "." {21+ if hook := r.hooks.afterOpenRoot; hook != nil {
30- if finalFlags&syscall.O_DIRECTORY == 0 {22+ hook()
31- err = fmt.Errorf("invalid snapshot root operation")
32- joinCloseError(&err, "close snapshot root directory", r.closeDescriptor(root, "close snapshot root directory"))
33- return nil, err
34- }
35- file := os.NewFile(uintptr(root), path)
36- if file == nil {
37- err = fmt.Errorf("open snapshot root")
38- joinCloseError(&err, "close snapshot root directory", r.closeDescriptor(root, "close snapshot root directory"))
39- return nil, err
40- }
41- return file, nil
42- }
43- parts := strings.Split(filepath.Clean(rel), string(filepath.Separator))
44- fd := root
45- for i, part := range parts {
46- if part == "" || part == "." || part == ".." {
47- err = fmt.Errorf("invalid snapshot path")
48- joinCloseError(&err, "close snapshot path directory", r.closeDescriptor(fd, "close snapshot path directory"))
49- return nil, err
50- }
51- flags := syscall.O_RDONLY | syscall.O_CLOEXEC | syscall.O_NOFOLLOW
52- if i != len(parts)-1 {
53- flags |= syscall.O_DIRECTORY
54- } else {
55- flags = finalFlags | syscall.O_CLOEXEC | syscall.O_NOFOLLOW
56- }
57- next, openErr := unix.Openat(fd, part, flags, mode)
58- closeErr := r.closeDescriptor(fd, "close snapshot path parent directory")
59- if openErr != nil {
60- err = openErr
61- joinCloseError(&err, "close snapshot path parent directory", closeErr)
62- return nil, err
63- }
64- if closeErr != nil {
65- err = fmt.Errorf("close snapshot path parent directory: %w", closeErr)
66- joinCloseError(&err, "close snapshot path child directory", r.closeDescriptor(next, "close snapshot path child directory"))
67- return nil, err
68- }
69- fd = next
70 }23 }
71- file := os.NewFile(uintptr(fd), path)24+ current, err := os.Lstat(r.dir)
72- if file == nil {25+ if err != nil {
73- err = fmt.Errorf("open snapshot path")26+ return nil, r.closeRootOnError(root, err)
74- joinCloseError(&err, "close snapshot path descriptor", r.closeDescriptor(fd, "close snapshot path descriptor"))27+ }
75- return nil, err28+ if !current.IsDir() || current.Mode()&os.ModeSymlink != 0 {
29+ return nil, r.closeRootOnError(root, &os.PathError{Op: "open", Path: r.dir, Err: syscall.ELOOP})
30+ }
31+ pinned, err := root.Stat(".")
32+ if err != nil {
33+ return nil, r.closeRootOnError(root, err)
34+ }
35+ if !os.SameFile(current, pinned) {
36+ return nil, r.closeRootOnError(root, &os.PathError{Op: "open", Path: r.dir, Err: syscall.ESTALE})
37+ }
38+ if !privateDirectory(pinned) {
39+ return nil, r.closeRootOnError(root, fmt.Errorf("snapshot directory is not private"))
76 }40 }
77- return file, nil41+ return root, nil
78 }42 }
79 43
80-func (r *Repository) repositoryParent(path string) (int, string, error) {44+func privateDirectory(fi os.FileInfo) bool {
81- rel, ok := r.repositoryRelative(path)45+ st, ok := fi.Sys().(*syscall.Stat_t)
82- if !ok || rel == "." {46+ return ok && fi.IsDir() && int(st.Uid) == os.Geteuid() && fi.Mode().Perm() == 0o700
83- return -1, "", fmt.Errorf("snapshot path outside repository")47+}
48+
49+func (r *Repository) closeRoot(root *os.Root) error {
50+ var injected error
51+ if hook := r.hooks.closeRoot; hook != nil {
52+ injected = hook()
84 }53 }
85- parts := strings.Split(filepath.Clean(rel), string(filepath.Separator))54+ return errors.Join(injected, root.Close())
86- if len(parts) == 0 || parts[len(parts)-1] == "." || parts[len(parts)-1] == ".." {55+}
87- return -1, "", fmt.Errorf("invalid snapshot path")56+
57+func (r *Repository) closeRootOnError(root *os.Root, err error) error {
58+ return errors.Join(err, r.closeRoot(root))
59+}
60+
61+// rejectFinalSymlink preserves the pre-os.Root guarantee that the final
62+// component is never a symlink. Root confinement is the hard boundary; this
63+// is defense-in-depth with a benign TOCTOU window inside a 0700 directory.
64+func rejectFinalSymlink(root *os.Root, rel string) error {
65+ fi, err := root.Lstat(rel)
66+ if err != nil {
67+ return err
68+ }
69+ if fi.Mode()&os.ModeSymlink != 0 {
70+ return &os.PathError{Op: "open", Path: rel, Err: syscall.ELOOP}
71+ }
72+ return nil
73+}
74+
75+func (r *Repository) openDirectory(path string) (file *os.File, err error) {
76+ rel, ok := r.repositoryRelative(path)
77+ if !ok {
78+ return nil, fmt.Errorf("snapshot path outside repository")
88 }79 }
89- fd, err := syscall.Open(r.dir, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0)80+ root, err := r.openRoot()
90 if err != nil {81 if err != nil {
91- return -1, "", err82+ return nil, err
92 }83 }
93- for _, part := range parts[:len(parts)-1] {84+ defer func() {
94- if part == "" || part == "." || part == ".." {85+ closeErr := r.closeRoot(root)
95- err = fmt.Errorf("invalid snapshot path")86+ if closeErr != nil && file != nil {
96- joinCloseError(&err, "close snapshot parent directory", r.closeDescriptor(fd, "close snapshot parent directory"))87+ joinCloseError(&err, "close snapshot directory after root close failure", file.Close())
97- return -1, "", err88+ file = nil
98- }
99- next, openErr := unix.Openat(fd, part, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0)
100- closeErr := r.closeDescriptor(fd, "close snapshot parent directory")
101- if openErr != nil {
102- err = openErr
103- joinCloseError(&err, "close snapshot parent directory", closeErr)
104- return -1, "", err
105- }
106- if closeErr != nil {
107- err = fmt.Errorf("close snapshot parent directory: %w", closeErr)
108- joinCloseError(&err, "close snapshot child directory", r.closeDescriptor(next, "close snapshot child directory"))
109- return -1, "", err
110 }89 }
111- fd = next90+ joinCloseError(&err, "close snapshot root", closeErr)
91+ }()
92+ if err := rejectFinalSymlink(root, rel); err != nil {
93+ return nil, err
112 }94 }
113- return fd, parts[len(parts)-1], nil95+ return root.OpenFile(rel, syscall.O_RDONLY|syscall.O_DIRECTORY, 0)
114 }96 }
115 97
116 func (r *Repository) repositoryRelative(path string) (string, bool) {98 func (r *Repository) repositoryRelative(path string) (string, bool) {
@@ -128,38 +110,35 @@ func (r *Repository) repositoryRelative(path string) (string, bool) {
128 return rel, true110 return rel, true
129 }111 }
130 112
131-func (r *Repository) openDirectory(path string) (*os.File, error) {113+func (r *Repository) stat(path string) (fi os.FileInfo, err error) {
132- return r.repositoryFD(path, syscall.O_RDONLY|syscall.O_DIRECTORY, 0)114+ rel, ok := r.repositoryRelative(path)
133-}115+ if !ok {
134-116+ return nil, fmt.Errorf("snapshot path outside repository")
135-func (r *Repository) stat(path string) (st syscall.Stat_t, err error) {
136- file, err := r.repositoryFD(path, syscall.O_RDONLY|syscall.O_NONBLOCK, 0)
137- if err != nil {
138- return syscall.Stat_t{}, err
139 }117 }
140- defer func() {118+ root, err := r.openRoot()
141- joinCloseError(&err, "close snapshot file", r.closeRepositoryFile(file, "close snapshot file"))119+ if err != nil {
142- }()120+ return nil, err
143- if err := syscall.Fstat(int(file.Fd()), &st); err != nil {
144- return syscall.Stat_t{}, err
145 }121 }
146- return st, nil122+ defer func() { joinCloseError(&err, "close snapshot root", r.closeRoot(root)) }()
123+ return root.Lstat(rel)
147 }124 }
148 125
149 // createTempAt creates an exclusively owned temporary file in an already126 // createTempAt creates an exclusively owned temporary file in an already
150 // validated directory. os.CreateTemp(path) would resolve that path again.127 // validated directory. os.CreateTemp(path) would resolve that path again.
151 func (r *Repository) createTempAt(dir string) (temp *os.File, err error) {128 func (r *Repository) createTempAt(dir string) (temp *os.File, err error) {
152- fd, err := r.openDirectory(dir)129+ rel, ok := r.repositoryRelative(dir)
130+ if !ok {
131+ return nil, fmt.Errorf("snapshot path outside repository")
132+ }
133+ root, err := r.openRoot()
153 if err != nil {134 if err != nil {
154 return nil, err135 return nil, err
155 }136 }
156 defer func() {137 defer func() {
157- if closeErr := r.closeRepositoryFile(fd, "close snapshot temporary directory"); closeErr != nil {138+ joinCloseError(&err, "close snapshot root", r.closeRoot(root))
158- if temp != nil {139+ if err != nil && temp != nil {
159- joinCloseError(&err, "close snapshot temporary file after directory close failure", r.closeRepositoryFile(temp, "close snapshot temporary file after directory close failure"))140+ joinCloseError(&err, "close snapshot temporary file after root close failure", temp.Close())
160- temp = nil141+ temp = nil
161- }
162- joinCloseError(&err, "close snapshot temporary directory", closeErr)
163 }142 }
164 }()143 }()
165 for range 100 {144 for range 100 {
@@ -168,19 +147,13 @@ func (r *Repository) createTempAt(dir string) (temp *os.File, err error) {
168 return nil, err147 return nil, err
169 }148 }
170 name := ".tmp-" + hex.EncodeToString(random[:])149 name := ".tmp-" + hex.EncodeToString(random[:])
171- tempFD, err := unix.Openat(int(fd.Fd()), name, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_EXCL|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0o600)150+ temp, err = root.OpenFile(filepath.Join(rel, name), syscall.O_WRONLY|syscall.O_CREAT|syscall.O_EXCL, 0o600)
172- if errors.Is(err, syscall.EEXIST) {151+ if errors.Is(err, os.ErrExist) {
173 continue152 continue
174 }153 }
175 if err != nil {154 if err != nil {
176 return nil, err155 return nil, err
177 }156 }
178- temp = os.NewFile(uintptr(tempFD), filepath.Join(dir, name))
179- if temp == nil {
180- err = fmt.Errorf("open snapshot temporary file")
181- joinCloseError(&err, "close snapshot temporary descriptor", r.closeDescriptor(tempFD, "close snapshot temporary descriptor"))
182- return nil, err
183- }
184 return temp, nil157 return temp, nil
185 }158 }
186 return nil, fmt.Errorf("create snapshot temporary file: too many collisions")159 return nil, fmt.Errorf("create snapshot temporary file: too many collisions")

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 2

Tags 13

↑↓ moveenter openesc close