refactor(snapshot): drop golang.org/x/sys for os.Root
Replace the per-component *at syscall walk (Openat/Mkdirat/Linkat/ Renameat/Unlinkat/Fstatat via x/sys/unix) with Go 1.26 os.Root, restoring the zero-runtime-dependency promise. x/sys had been reintroduced only because darwin's frozen syscall package lacks the *at family. os.Root confines every operation to the repository root but silently resolves a final in-root symlink on plain opens, so rejectFinalSymlink (a Root.Lstat pre-check) preserves the previous O_NOFOLLOW guarantee; O_CREATE|O_EXCL creates, Lstat, Remove, Rename, and Link already operate on the link itself. Root.Remove also subsumes the platform directoryUnlinkRetry helpers. safeFilesystemError now strips *os.LinkError paths the same way it strips *os.PathError. The unchanged-publish fast path pays a constant ~15 extra allocations (os.Root re-walks the path per call and the symlink guard adds a Lstat per guarded read), so the 10k-history allocation limit moves from 300 to 350; a per-retained-object regression would still exceed it by orders of magnitude.
- commit
- 0d68d26723dec6bcb44b6e81923fd400c5a4998e
- author
- brice · · 2026-07-22 09:15 UTC
- parent
- 59be6a2
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
18 files · +170 -297
M go.mod +1 -4
| @@ -2,10 +2,7 @@ module github.com/bnema/vev | |||
| 2 | 2 | ||
| 3 | 3 | go 1.26 | |
| 4 | 4 | ||
| 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 | |
| 9 | 6 | ||
| 10 | 7 | require ( | |
| 11 | 8 | github.com/davecgh/go-spew v1.1.1 // indirect | |
| @@ -2,10 +2,7 @@ module github.com/bnema/vev | |||||
| 2 | 2 | ||||
| 3 | go 1.26 | 3 | 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 // indirect | 8 | github.com/davecgh/go-spew v1.1.1 // indirect | ||
M go.sum +0 -2 generated file: collapsed
M internal/adapters/snapshot/atomic.go +62 -78
| @@ -9,7 +9,6 @@ import ( | |||
| 9 | 9 | "syscall" | |
| 10 | 10 | ||
| 11 | 11 | "github.com/bnema/vev/pkg/safedir" | |
| 12 | - | "golang.org/x/sys/unix" | |
| 13 | 12 | ) | |
| 14 | 13 | ||
| 15 | 14 | func (r *Repository) ensureSession(key string) error { | |
| @@ -34,7 +33,7 @@ func (r *Repository) ensurePrivateDirectory(dir string) error { | |||
| 34 | 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 | 37 | // The configured root is the trust boundary. Once it is private, every | |
| 39 | 38 | // descendant is created through a pinned parent descriptor rather than via | |
| 40 | 39 | // a path that an attacker can replace between checks. | |
| @@ -62,38 +61,32 @@ func (r *Repository) ensurePrivateDirectoryPhase(dir, phase string) error { | |||
| 62 | 61 | if err := r.ensurePrivateDirectoryPhase(filepath.Dir(dir), "snapshot directory"); err != nil { | |
| 63 | 62 | 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 | 69 | if err != nil { | |
| 67 | 70 | return err | |
| 68 | 71 | } | |
| 72 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() | |
| 69 | 73 | created := false | |
| 70 | - | mkdirErr := unix.Mkdirat(parent, name, 0o700) | |
| 74 | + | mkdirErr := root.Mkdir(rel, 0o700) | |
| 71 | 75 | if mkdirErr == nil { | |
| 72 | 76 | created = true | |
| 73 | 77 | } | |
| 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 | |
| 82 | 80 | } | |
| 83 | - | opened, err := r.openDirectory(dir) | |
| 81 | + | fi, err := root.Lstat(rel) | |
| 84 | 82 | if err != nil { | |
| 85 | 83 | return err | |
| 86 | 84 | } | |
| 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") | |
| 95 | 87 | } | |
| 96 | - | if int(st.Uid) != os.Geteuid() || st.Mode&0o777 != 0o700 { | |
| 88 | + | st, ok := fi.Sys().(*syscall.Stat_t) | |
| 89 | + | if !ok || int(st.Uid) != os.Geteuid() || fi.Mode().Perm() != 0o700 { | |
| 97 | 90 | return fmt.Errorf("snapshot directory is not private") | |
| 98 | 91 | } | |
| 99 | 92 | if created { | |
| @@ -231,24 +224,6 @@ func (r *Repository) closeFile(f *os.File) error { | |||
| 231 | 224 | return errors.Join(injected, f.Close()) | |
| 232 | 225 | } | |
| 233 | 226 | ||
| 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 | 227 | // joinCloseError retains a primary operation failure and appends contextual | |
| 253 | 228 | // close failures, including cleanup-only paths, without suppressing either. | |
| 254 | 229 | func joinCloseError(primary *error, operation string, closeErr error) { | |
| @@ -263,21 +238,20 @@ func (r *Repository) installImmutable(oldPath, newPath string) (err error) { | |||
| 263 | 238 | return err | |
| 264 | 239 | } | |
| 265 | 240 | } | |
| 266 | - | oldFD, oldName, err := r.repositoryParent(oldPath) | |
| 267 | - | if err != nil { | |
| 268 | - | return err | |
| 241 | + | oldRel, ok := r.repositoryRelative(oldPath) | |
| 242 | + | if !ok { | |
| 243 | + | return fmt.Errorf("snapshot path outside repository") | |
| 269 | 244 | } | |
| 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) | |
| 245 | + | newRel, ok := r.repositoryRelative(newPath) | |
| 246 | + | if !ok { | |
| 247 | + | return fmt.Errorf("snapshot path outside repository") | |
| 248 | + | } | |
| 249 | + | root, err := r.openRoot() | |
| 274 | 250 | if err != nil { | |
| 275 | 251 | return err | |
| 276 | 252 | } | |
| 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) | |
| 253 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() | |
| 254 | + | return root.Link(oldRel, newRel) | |
| 281 | 255 | } | |
| 282 | 256 | func (r *Repository) rename(oldPath, newPath string) (err error) { | |
| 283 | 257 | if r.hooks.rename != nil { | |
| @@ -285,21 +259,20 @@ func (r *Repository) rename(oldPath, newPath string) (err error) { | |||
| 285 | 259 | return err | |
| 286 | 260 | } | |
| 287 | 261 | } | |
| 288 | - | oldFD, oldName, err := r.repositoryParent(oldPath) | |
| 289 | - | if err != nil { | |
| 290 | - | return err | |
| 262 | + | oldRel, ok := r.repositoryRelative(oldPath) | |
| 263 | + | if !ok { | |
| 264 | + | return fmt.Errorf("snapshot path outside repository") | |
| 265 | + | } | |
| 266 | + | newRel, ok := r.repositoryRelative(newPath) | |
| 267 | + | if !ok { | |
| 268 | + | return fmt.Errorf("snapshot path outside repository") | |
| 291 | 269 | } | |
| 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) | |
| 270 | + | root, err := r.openRoot() | |
| 296 | 271 | if err != nil { | |
| 297 | 272 | return err | |
| 298 | 273 | } | |
| 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) | |
| 274 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() | |
| 275 | + | return root.Rename(oldRel, newRel) | |
| 303 | 276 | } | |
| 304 | 277 | func (r *Repository) remove(path string) (err error) { | |
| 305 | 278 | if r.hooks.remove != nil { | |
| @@ -307,22 +280,16 @@ func (r *Repository) remove(path string) (err error) { | |||
| 307 | 280 | return err | |
| 308 | 281 | } | |
| 309 | 282 | } | |
| 310 | - | fd, name, err := r.repositoryParent(path) | |
| 283 | + | rel, ok := r.repositoryRelative(path) | |
| 284 | + | if !ok { | |
| 285 | + | return fmt.Errorf("snapshot path outside repository") | |
| 286 | + | } | |
| 287 | + | root, err := r.openRoot() | |
| 311 | 288 | if err != nil { | |
| 312 | 289 | return err | |
| 313 | 290 | } | |
| 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) | |
| 291 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() | |
| 292 | + | return root.Remove(rel) | |
| 326 | 293 | } | |
| 327 | 294 | func (r *Repository) syncDirectory(dir string) error { | |
| 328 | 295 | if r.hooks.syncDirectory != nil { | |
| @@ -352,11 +319,28 @@ func (r *Repository) readOptionalBounded(path string) ([]byte, bool, error) { | |||
| 352 | 319 | return data, err == nil, err | |
| 353 | 320 | } | |
| 354 | 321 | ||
| 355 | - | // readBounded opens the final component with O_NOFOLLOW, validates the opened | |
| 322 | + | // readBounded rejects a final symlink explicitly (rejectFinalSymlink) before | |
| 323 | + | // opening the component under the confined root, validates the opened | |
| 356 | 324 | // descriptor, then reads a hard bounded amount. It deliberately never trusts | |
| 357 | 325 | // 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) | |
| 326 | + | func (r *Repository) readBounded(path string) (data []byte, err error) { | |
| 327 | + | root, err := r.openRoot() | |
| 328 | + | if err != nil { | |
| 329 | + | return nil, err | |
| 330 | + | } | |
| 331 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() | |
| 332 | + | return r.readBoundedRoot(root, path) | |
| 333 | + | } | |
| 334 | + | ||
| 335 | + | func (r *Repository) readBoundedRoot(root *os.Root, path string) ([]byte, error) { | |
| 336 | + | rel, ok := r.repositoryRelative(path) | |
| 337 | + | if !ok { | |
| 338 | + | return nil, fmt.Errorf("snapshot path outside repository") | |
| 339 | + | } | |
| 340 | + | if err := rejectFinalSymlink(root, rel); err != nil { | |
| 341 | + | return nil, err | |
| 342 | + | } | |
| 343 | + | f, err := root.OpenFile(rel, syscall.O_RDONLY|syscall.O_NONBLOCK, 0) | |
| 360 | 344 | if err != nil { | |
| 361 | 345 | return nil, err | |
| 362 | 346 | } | |
| @@ -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, every | 37 | // The configured root is the trust boundary. Once it is private, every | ||
| 39 | // descendant is created through a pinned parent descriptor rather than via | 38 | // 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,32 @@ 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 err | 62 | 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 err | 70 | return err | ||
| 68 | } | 71 | } | ||
| 72 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() | |||
| 69 | created := false | 73 | 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 = true | 76 | 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 err | 83 | return err | ||
| 86 | } | 84 | } | ||
| 87 | - | var st syscall.Stat_t | 85 | + | 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 | + | st, ok := fi.Sys().(*syscall.Stat_t) |
| 89 | + | if !ok || int(st.Uid) != os.Geteuid() || fi.Mode().Perm() != 0o700 { | |||
| 97 | return fmt.Errorf("snapshot directory is not private") | 90 | return fmt.Errorf("snapshot directory is not private") | ||
| 98 | } | 91 | } | ||
| 99 | if created { | 92 | if created { | ||
| @@ -231,24 +224,6 @@ func (r *Repository) closeFile(f *os.File) error { | |||||
| 231 | return errors.Join(injected, f.Close()) | 224 | return errors.Join(injected, f.Close()) | ||
| 232 | } | 225 | } | ||
| 233 | 226 | ||||
| 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 contextual | 227 | // joinCloseError retains a primary operation failure and appends contextual | ||
| 253 | // close failures, including cleanup-only paths, without suppressing either. | 228 | // close failures, including cleanup-only paths, without suppressing either. | ||
| 254 | func joinCloseError(primary *error, operation string, closeErr error) { | 229 | func joinCloseError(primary *error, operation string, closeErr error) { | ||
| @@ -263,21 +238,20 @@ func (r *Repository) installImmutable(oldPath, newPath string) (err error) { | |||||
| 263 | return err | 238 | return err | ||
| 264 | } | 239 | } | ||
| 265 | } | 240 | } | ||
| 266 | - | oldFD, oldName, err := r.repositoryParent(oldPath) | 241 | + | oldRel, ok := r.repositoryRelative(oldPath) |
| 267 | - | if err != nil { | 242 | + | if !ok { |
| 268 | - | return err | 243 | + | return fmt.Errorf("snapshot path outside repository") |
| 269 | } | 244 | } | ||
| 270 | - | defer func() { | 245 | + | newRel, ok := r.repositoryRelative(newPath) |
| 271 | - | joinCloseError(&err, "close source snapshot parent directory", r.closeDescriptor(oldFD, "close source snapshot parent directory")) | 246 | + | if !ok { |
| 272 | - | }() | 247 | + | return fmt.Errorf("snapshot path outside repository") |
| 273 | - | newFD, newName, err := r.repositoryParent(newPath) | 248 | + | } |
| 249 | + | root, err := r.openRoot() | |||
| 274 | if err != nil { | 250 | if err != nil { | ||
| 275 | return err | 251 | return err | ||
| 276 | } | 252 | } | ||
| 277 | - | defer func() { | 253 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() |
| 278 | - | joinCloseError(&err, "close destination snapshot parent directory", r.closeDescriptor(newFD, "close destination snapshot parent directory")) | 254 | + | return root.Link(oldRel, newRel) |
| 279 | - | }() | |||
| 280 | - | return unix.Linkat(oldFD, oldName, newFD, newName, 0) | |||
| 281 | } | 255 | } | ||
| 282 | func (r *Repository) rename(oldPath, newPath string) (err error) { | 256 | func (r *Repository) rename(oldPath, newPath string) (err error) { | ||
| 283 | if r.hooks.rename != nil { | 257 | if r.hooks.rename != nil { | ||
| @@ -285,21 +259,20 @@ func (r *Repository) rename(oldPath, newPath string) (err error) { | |||||
| 285 | return err | 259 | return err | ||
| 286 | } | 260 | } | ||
| 287 | } | 261 | } | ||
| 288 | - | oldFD, oldName, err := r.repositoryParent(oldPath) | 262 | + | oldRel, ok := r.repositoryRelative(oldPath) |
| 289 | - | if err != nil { | 263 | + | if !ok { |
| 290 | - | return err | 264 | + | return fmt.Errorf("snapshot path outside repository") |
| 265 | + | } | |||
| 266 | + | newRel, ok := r.repositoryRelative(newPath) | |||
| 267 | + | if !ok { | |||
| 268 | + | return fmt.Errorf("snapshot path outside repository") | |||
| 291 | } | 269 | } | ||
| 292 | - | defer func() { | 270 | + | 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 { | 271 | if err != nil { | ||
| 297 | return err | 272 | return err | ||
| 298 | } | 273 | } | ||
| 299 | - | defer func() { | 274 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() |
| 300 | - | joinCloseError(&err, "close destination snapshot parent directory", r.closeDescriptor(newFD, "close destination snapshot parent directory")) | 275 | + | return root.Rename(oldRel, newRel) |
| 301 | - | }() | |||
| 302 | - | return unix.Renameat(oldFD, oldName, newFD, newName) | |||
| 303 | } | 276 | } | ||
| 304 | func (r *Repository) remove(path string) (err error) { | 277 | func (r *Repository) remove(path string) (err error) { | ||
| 305 | if r.hooks.remove != nil { | 278 | if r.hooks.remove != nil { | ||
| @@ -307,22 +280,16 @@ func (r *Repository) remove(path string) (err error) { | |||||
| 307 | return err | 280 | return err | ||
| 308 | } | 281 | } | ||
| 309 | } | 282 | } | ||
| 310 | - | fd, name, err := r.repositoryParent(path) | 283 | + | rel, ok := r.repositoryRelative(path) |
| 284 | + | if !ok { | |||
| 285 | + | return fmt.Errorf("snapshot path outside repository") | |||
| 286 | + | } | |||
| 287 | + | root, err := r.openRoot() | |||
| 311 | if err != nil { | 288 | if err != nil { | ||
| 312 | return err | 289 | return err | ||
| 313 | } | 290 | } | ||
| 314 | - | defer func() { | 291 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() |
| 315 | - | joinCloseError(&err, "close snapshot parent directory", r.closeDescriptor(fd, "close snapshot parent directory")) | 292 | + | 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 | } | 293 | } | ||
| 327 | func (r *Repository) syncDirectory(dir string) error { | 294 | func (r *Repository) syncDirectory(dir string) error { | ||
| 328 | if r.hooks.syncDirectory != nil { | 295 | if r.hooks.syncDirectory != nil { | ||
| @@ -352,11 +319,28 @@ func (r *Repository) readOptionalBounded(path string) ([]byte, bool, error) { | |||||
| 352 | return data, err == nil, err | 319 | return data, err == nil, err | ||
| 353 | } | 320 | } | ||
| 354 | 321 | ||||
| 355 | - | // readBounded opens the final component with O_NOFOLLOW, validates the opened | 322 | + | // readBounded rejects a final symlink explicitly (rejectFinalSymlink) before |
| 323 | + | // opening the component under the confined root, validates the opened | |||
| 356 | // descriptor, then reads a hard bounded amount. It deliberately never trusts | 324 | // descriptor, then reads a hard bounded amount. It deliberately never trusts | ||
| 357 | // path metadata obtained before opening the descriptor. | 325 | // path metadata obtained before opening the descriptor. | ||
| 358 | - | func (r *Repository) readBounded(path string) ([]byte, error) { | 326 | + | func (r *Repository) readBounded(path string) (data []byte, err error) { |
| 359 | - | f, err := r.repositoryFD(path, syscall.O_RDONLY|syscall.O_NONBLOCK, 0) | 327 | + | root, err := r.openRoot() |
| 328 | + | if err != nil { | |||
| 329 | + | return nil, err | |||
| 330 | + | } | |||
| 331 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() | |||
| 332 | + | return r.readBoundedRoot(root, path) | |||
| 333 | + | } | |||
| 334 | + | ||||
| 335 | + | func (r *Repository) readBoundedRoot(root *os.Root, path string) ([]byte, error) { | |||
| 336 | + | rel, ok := r.repositoryRelative(path) | |||
| 337 | + | if !ok { | |||
| 338 | + | return nil, fmt.Errorf("snapshot path outside repository") | |||
| 339 | + | } | |||
| 340 | + | if err := rejectFinalSymlink(root, rel); err != nil { | |||
| 341 | + | return nil, err | |||
| 342 | + | } | |||
| 343 | + | f, err := root.OpenFile(rel, syscall.O_RDONLY|syscall.O_NONBLOCK, 0) | |||
| 360 | if err != nil { | 344 | if err != nil { | ||
| 361 | return nil, err | 345 | return nil, err | ||
| 362 | } | 346 | } | ||
M internal/adapters/snapshot/atomic_test.go +0 -33
| @@ -5,8 +5,6 @@ import ( | |||
| 5 | 5 | "errors" | |
| 6 | 6 | "os" | |
| 7 | 7 | "path/filepath" | |
| 8 | - | "strings" | |
| 9 | - | "syscall" | |
| 10 | 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 | 78 | func TestWriteMutableDoesNotRemoveConsumedTemporaryFile(t *testing.T) { | |
| 112 | 79 | repo := NewRepository(privateDir(t)) | |
| 113 | 80 | 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 @@ | |||
| 3 | 3 | package snapshot | |
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | - | "errors" | |
| 7 | 6 | "io" | |
| 8 | 7 | "syscall" | |
| 9 | 8 | "unsafe" | |
| @@ -20,9 +19,3 @@ func directoryCookie(file maintenanceDirectory, _ *syscall.Dirent) (int64, error | |||
| 20 | 19 | } | |
| 21 | 20 | ||
| 22 | 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 | - | } | |
| @@ -3,7 +3,6 @@ | |||||
| 3 | package snapshot | 3 | 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) { | |||
| 18 | 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 | - | } | |
| @@ -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 @@ | |||
| 2 | 2 | ||
| 3 | 3 | package snapshot | |
| 4 | 4 | ||
| 5 | - | import ( | |
| 6 | - | "errors" | |
| 7 | - | "syscall" | |
| 8 | - | ) | |
| 5 | + | import "syscall" | |
| 9 | 6 | ||
| 10 | 7 | const maintenanceDirentBufferSize = 8192 | |
| 11 | 8 | ||
| @@ -15,5 +12,3 @@ func directoryCookie(_ maintenanceDirectory, dirent *syscall.Dirent) (int64, err | |||
| 15 | 12 | } | |
| 16 | 13 | ||
| 17 | 14 | 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 snapshot | 3 | package snapshot | ||
| 4 | 4 | ||||
| 5 | - | import ( | 5 | + | import "syscall" |
| 6 | - | "errors" | |||
| 7 | - | "syscall" | |||
| 8 | - | ) | |||
| 9 | 6 | ||||
| 10 | const maintenanceDirentBufferSize = 8192 | 7 | 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) { | |||
| 17 | 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 | - | } | |
| @@ -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 { | |||
| 262 | 262 | return out | |
| 263 | 263 | } | |
| 264 | 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 | 269 | path := r.headPath(key) | |
| 266 | 270 | if hook := r.hooks.beforeHeadRead; hook != nil { | |
| 267 | 271 | if err := hook(path); err != nil { | |
| 268 | 272 | 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 | 282 | if err != nil { | |
| 273 | 283 | return 0, ports.SnapshotDigest{}, err | |
| 274 | 284 | } | |
| @@ -262,13 +262,23 @@ func marshalHead(generation uint64, digest ports.SnapshotDigest) []byte { | |||||
| 262 | return out | 262 | 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{}, err | 272 | 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{}, err | 283 | 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 | |||
| 187 | 187 | continue | |
| 188 | 188 | } | |
| 189 | 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 | 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 | 194 | if len(entries) >= limit { | |
| 201 | 195 | if drainBuffer && len(entries) > limit { | |
| @@ -187,15 +187,9 @@ func (r *Repository) readMaintenanceDirentWithDrain(dir string, limit int, curso | |||||
| 187 | continue | 187 | 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 ( | |||
| 8 | 8 | "os" | |
| 9 | 9 | "path/filepath" | |
| 10 | 10 | "strings" | |
| 11 | - | "syscall" | |
| 12 | 11 | ) | |
| 13 | 12 | ||
| 14 | 13 | // 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 | 141 | state.current = filepath.Dir(state.current) | |
| 143 | 142 | continue | |
| 144 | 143 | } | |
| 145 | - | if err != nil && !errors.Is(err, syscall.ELOOP) { | |
| 144 | + | if err != nil { | |
| 146 | 145 | return changed, false, err | |
| 147 | 146 | } | |
| 148 | - | if err != nil || stat.Mode&syscall.S_IFMT != syscall.S_IFDIR { | |
| 147 | + | if !stat.IsDir() { | |
| 149 | 148 | if !r.consumeQuarantineWork(budget, "remove") { | |
| 150 | 149 | return changed, false, nil | |
| 151 | 150 | } | |
| @@ -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 canonical | 13 | // 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 | continue | 142 | continue | ||
| 144 | } | 143 | } | ||
| 145 | - | if err != nil && !errors.Is(err, syscall.ELOOP) { | 144 | + | if err != nil { |
| 146 | return changed, false, err | 145 | 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, nil | 149 | 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 } | |||
| 26 | 26 | // safeFilesystemError preserves error matching without including a filesystem | |
| 27 | 27 | // path from terminal-facing error text. | |
| 28 | 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 | 33 | var pathError *os.PathError | |
| 30 | 34 | if errors.As(err, &pathError) { | |
| 31 | 35 | 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 filesystem | 26 | // 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.PathError | 33 | 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 +12 -12
| @@ -7,6 +7,7 @@ import ( | |||
| 7 | 7 | "os" | |
| 8 | 8 | "path/filepath" | |
| 9 | 9 | "strings" | |
| 10 | + | "syscall" | |
| 10 | 11 | "testing" | |
| 11 | 12 | ) | |
| 12 | 13 | ||
| @@ -46,26 +47,25 @@ func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) { | |||
| 46 | 47 | } | |
| 47 | 48 | } | |
| 48 | 49 | ||
| 49 | - | func TestStatReturnsInjectedCloseErrorWithContext(t *testing.T) { | |
| 50 | + | func TestFinalSymlinksRejectedByRootOperations(t *testing.T) { | |
| 50 | 51 | repo := NewRepository(privateDir(t)) | |
| 51 | 52 | if err := repo.ensurePrivateDirectory(repo.dir); err != nil { | |
| 52 | 53 | t.Fatal(err) | |
| 53 | 54 | } | |
| 54 | - | path := filepath.Join(repo.dir, "state") | |
| 55 | - | if err := os.WriteFile(path, []byte("state"), 0o600); err != nil { | |
| 55 | + | target := filepath.Join(repo.dir, "target") | |
| 56 | + | if err := os.Mkdir(target, 0o700); err != nil { | |
| 56 | 57 | t.Fatal(err) | |
| 57 | 58 | } | |
| 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 | |
| 59 | + | link := filepath.Join(repo.dir, "link") | |
| 60 | + | if err := os.Symlink(target, link); err != nil { | |
| 61 | + | t.Fatal(err) | |
| 64 | 62 | } | |
| 65 | 63 | ||
| 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) | |
| 64 | + | if _, err := repo.openDirectory(link); !errors.Is(err, syscall.ELOOP) || strings.Contains(err.Error(), repo.dir) { | |
| 65 | + | t.Fatalf("openDirectory error = %v, want sanitized ELOOP", err) | |
| 66 | + | } | |
| 67 | + | if _, err := repo.readBounded(link); !errors.Is(err, syscall.ELOOP) || strings.Contains(err.Error(), repo.dir) { | |
| 68 | + | t.Fatalf("readBounded error = %v, want sanitized ELOOP", err) | |
| 69 | 69 | } | |
| 70 | 70 | } | |
| 71 | 71 | ||
| @@ -7,6 +7,7 @@ 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 | ||||
| @@ -46,26 +47,25 @@ func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) { | |||||
| 46 | } | 47 | } | ||
| 47 | } | 48 | } | ||
| 48 | 49 | ||||
| 49 | - | func TestStatReturnsInjectedCloseErrorWithContext(t *testing.T) { | 50 | + | func TestFinalSymlinksRejectedByRootOperations(t *testing.T) { |
| 50 | repo := NewRepository(privateDir(t)) | 51 | repo := NewRepository(privateDir(t)) | ||
| 51 | if err := repo.ensurePrivateDirectory(repo.dir); err != nil { | 52 | if err := repo.ensurePrivateDirectory(repo.dir); err != nil { | ||
| 52 | t.Fatal(err) | 53 | t.Fatal(err) | ||
| 53 | } | 54 | } | ||
| 54 | - | path := filepath.Join(repo.dir, "state") | 55 | + | target := filepath.Join(repo.dir, "target") |
| 55 | - | if err := os.WriteFile(path, []byte("state"), 0o600); err != nil { | 56 | + | if err := os.Mkdir(target, 0o700); err != nil { |
| 56 | t.Fatal(err) | 57 | t.Fatal(err) | ||
| 57 | } | 58 | } | ||
| 58 | - | closeCause := errors.New("injected stat close failure") | 59 | + | link := filepath.Join(repo.dir, "link") |
| 59 | - | repo.hooks.closeDescriptor = func(got string) error { | 60 | + | if err := os.Symlink(target, link); err != nil { |
| 60 | - | if got == "close snapshot file" { | 61 | + | t.Fatal(err) |
| 61 | - | return closeCause | |||
| 62 | - | } | |||
| 63 | - | return nil | |||
| 64 | } | 62 | } | ||
| 65 | 63 | ||||
| 66 | - | _, err := repo.stat(path) | 64 | + | if _, err := repo.openDirectory(link); !errors.Is(err, syscall.ELOOP) || strings.Contains(err.Error(), repo.dir) { |
| 67 | - | if !errors.Is(err, closeCause) || !strings.Contains(err.Error(), "close snapshot file") { | 65 | + | t.Fatalf("openDirectory error = %v, want sanitized ELOOP", err) |
| 68 | - | t.Fatalf("stat error = %v, want contextual close failure", err) | 66 | + | } |
| 67 | + | if _, err := repo.readBounded(link); !errors.Is(err, syscall.ELOOP) || strings.Contains(err.Error(), repo.dir) { | |||
| 68 | + | t.Fatalf("readBounded error = %v, want sanitized ELOOP", err) | |||
| 69 | } | 69 | } | ||
| 70 | } | 70 | } | ||
| 71 | 71 | ||||
M internal/adapters/snapshot/publish.go +20 -4
| @@ -37,10 +37,8 @@ func (r *Repository) Publish(ctx context.Context, publication ports.SnapshotPubl | |||
| 37 | 37 | // The common replay path needs neither a manifest decode nor a reference | |
| 38 | 38 | // map. Checking the immutable bytes directly also keeps its descriptor | |
| 39 | 39 | // 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 | |
| 44 | 42 | } | |
| 45 | 43 | ||
| 46 | 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 | 145 | 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", root.Close()) }() | |
| 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 | 166 | // currentPublication takes the normal HEAD path without loading every object | |
| 151 | 167 | // in the current generation. A damaged HEAD or manifest still uses the full | |
| 152 | 168 | // 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 reference | 37 | // The common replay path needs neither a manifest decode nor a reference | ||
| 38 | // map. Checking the immutable bytes directly also keeps its descriptor | 38 | // 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 nil | 145 | 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", root.Close()) }() | |||
| 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 object | 166 | // currentPublication takes the normal HEAD path without loading every object | ||
| 151 | // in the current generation. A damaged HEAD or manifest still uses the full | 167 | // 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 safe | 168 | // fallback scan, because only Load's object validation can establish a safe | ||
M internal/adapters/snapshot/publish_efficiency_test.go +7 -2
| @@ -35,13 +35,18 @@ func TestRepositoryPublishSkipsUnchangedTenThousandObjectHistory(t *testing.T) { | |||
| 35 | 35 | ||
| 36 | 36 | // Replaying the now-current generation still parses the 10k references, but | |
| 37 | 37 | // must not allocate one object-sized buffer per retained history entry. | |
| 38 | + | // The limit is 350, not lower: the os.Root-based fast path pays a constant | |
| 39 | + | // ~15 extra allocations (2 guarded reads, each a Root.Lstat symlink check | |
| 40 | + | // plus a Root.OpenFile, and os.Root re-walks the path per call), and -race | |
| 41 | + | // adds ~20 more. A per-retained-object regression would exceed this by | |
| 42 | + | // orders of magnitude. | |
| 38 | 43 | allocations := testing.AllocsPerRun(3, func() { | |
| 39 | 44 | if err := repo.Publish(context.Background(), second); err != nil { | |
| 40 | 45 | panic(err) | |
| 41 | 46 | } | |
| 42 | 47 | }) | |
| 43 | - | if allocations > 300 { | |
| 44 | - | t.Fatalf("unchanged 10k history Publish allocations = %.0f, want <= 300; retained objects were likely copied", allocations) | |
| 48 | + | if allocations > 350 { | |
| 49 | + | t.Fatalf("unchanged 10k history Publish allocations = %.0f, want <= 350; retained objects were likely copied", allocations) | |
| 45 | 50 | } | |
| 46 | 51 | ||
| 47 | 52 | got, err := repo.Load(context.Background(), second.Name) | |
| @@ -35,13 +35,18 @@ func TestRepositoryPublishSkipsUnchangedTenThousandObjectHistory(t *testing.T) { | |||||
| 35 | 35 | ||||
| 36 | // Replaying the now-current generation still parses the 10k references, but | 36 | // 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 350, not lower: the os.Root-based fast path pays a constant | |||
| 39 | + | // ~15 extra allocations (2 guarded reads, each a Root.Lstat symlink check | |||
| 40 | + | // plus a Root.OpenFile, and os.Root re-walks the path per call), and -race | |||
| 41 | + | // adds ~20 more. A per-retained-object regression would exceed this by | |||
| 42 | + | // orders of magnitude. | |||
| 38 | allocations := testing.AllocsPerRun(3, func() { | 43 | allocations := testing.AllocsPerRun(3, func() { | ||
| 39 | if err := repo.Publish(context.Background(), second); err != nil { | 44 | if err := repo.Publish(context.Background(), second); err != nil { | ||
| 40 | panic(err) | 45 | panic(err) | ||
| 41 | } | 46 | } | ||
| 42 | }) | 47 | }) | ||
| 43 | - | if allocations > 300 { | 48 | + | if allocations > 350 { |
| 44 | - | t.Fatalf("unchanged 10k history Publish allocations = %.0f, want <= 300; retained objects were likely copied", allocations) | 49 | + | t.Fatalf("unchanged 10k history Publish allocations = %.0f, want <= 350; retained objects were likely copied", allocations) |
| 45 | } | 50 | } | ||
| 46 | 51 | ||||
| 47 | got, err := repo.Load(context.Background(), second.Name) | 52 | got, err := repo.Load(context.Background(), second.Name) | ||
M internal/adapters/snapshot/repository.go +0 -1
| @@ -67,7 +67,6 @@ type repositoryHooks struct { | |||
| 67 | 67 | writeTemp func(string) error | |
| 68 | 68 | syncFile func(string) error | |
| 69 | 69 | closeFile func(string) error | |
| 70 | - | closeDescriptor func(string) error | |
| 71 | 70 | installImmutable func(string) error | |
| 72 | 71 | rename func(string) error | |
| 73 | 72 | syncDirectory func(string) error | |
| @@ -67,7 +67,6 @@ type repositoryHooks struct { | |||||
| 67 | writeTemp func(string) error | 67 | writeTemp func(string) error | ||
| 68 | syncFile func(string) error | 68 | syncFile func(string) error | ||
| 69 | closeFile func(string) error | 69 | closeFile func(string) error | ||
| 70 | - | closeDescriptor func(string) error | |||
| 71 | installImmutable func(string) error | 70 | installImmutable func(string) error | ||
| 72 | rename func(string) error | 71 | rename func(string) error | ||
| 73 | syncDirectory func(string) error | 72 | syncDirectory func(string) error | ||
M internal/adapters/snapshot/repository_maintenance_test.go +7 -0
| @@ -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 | 27 | func TestRepositoryMaintenanceFilesystemErrorsHaveSafeContext(t *testing.T) { | |
| 21 | 28 | injected := errors.New("injected filesystem failure") | |
| 22 | 29 | 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 +42 -119
| @@ -9,108 +9,40 @@ import ( | |||
| 9 | 9 | "path/filepath" | |
| 10 | 10 | "strings" | |
| 11 | 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. 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 | + | return os.OpenRoot(r.dir) | |
| 16 | + | } | |
| 17 | + | ||
| 18 | + | // rejectFinalSymlink preserves the pre-os.Root guarantee that the final | |
| 19 | + | // component is never a symlink. Root confinement is the hard boundary; this | |
| 20 | + | // is defense-in-depth with a benign TOCTOU window inside a 0700 directory. | |
| 21 | + | func rejectFinalSymlink(root *os.Root, rel string) error { | |
| 22 | + | fi, err := root.Lstat(rel) | |
| 26 | 23 | if err != nil { | |
| 27 | - | return nil, err | |
| 24 | + | return err | |
| 28 | 25 | } | |
| 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 | |
| 26 | + | if fi.Mode()&os.ModeSymlink != 0 { | |
| 27 | + | return &os.PathError{Op: "open", Path: rel, Err: syscall.ELOOP} | |
| 42 | 28 | } | |
| 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 | - | } | |
| 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 | |
| 76 | - | } | |
| 77 | - | return file, nil | |
| 29 | + | return nil | |
| 78 | 30 | } | |
| 79 | 31 | ||
| 80 | - | func (r *Repository) repositoryParent(path string) (int, string, error) { | |
| 32 | + | func (r *Repository) openDirectory(path string) (file *os.File, err error) { | |
| 81 | 33 | rel, ok := r.repositoryRelative(path) | |
| 82 | - | if !ok || rel == "." { | |
| 83 | - | return -1, "", fmt.Errorf("snapshot path outside repository") | |
| 84 | - | } | |
| 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") | |
| 34 | + | if !ok { | |
| 35 | + | return nil, fmt.Errorf("snapshot path outside repository") | |
| 88 | 36 | } | |
| 89 | - | fd, err := syscall.Open(r.dir, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0) | |
| 37 | + | root, err := r.openRoot() | |
| 90 | 38 | if err != nil { | |
| 91 | - | return -1, "", err | |
| 39 | + | return nil, err | |
| 92 | 40 | } | |
| 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 | |
| 110 | - | } | |
| 111 | - | fd = next | |
| 41 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() | |
| 42 | + | if err := rejectFinalSymlink(root, rel); err != nil { | |
| 43 | + | return nil, err | |
| 112 | 44 | } | |
| 113 | - | return fd, parts[len(parts)-1], nil | |
| 45 | + | return root.OpenFile(rel, syscall.O_RDONLY|syscall.O_DIRECTORY, 0) | |
| 114 | 46 | } | |
| 115 | 47 | ||
| 116 | 48 | func (r *Repository) repositoryRelative(path string) (string, bool) { | |
| @@ -128,38 +60,35 @@ func (r *Repository) repositoryRelative(path string) (string, bool) { | |||
| 128 | 60 | return rel, true | |
| 129 | 61 | } | |
| 130 | 62 | ||
| 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 | |
| 63 | + | func (r *Repository) stat(path string) (fi os.FileInfo, err error) { | |
| 64 | + | rel, ok := r.repositoryRelative(path) | |
| 65 | + | if !ok { | |
| 66 | + | return nil, fmt.Errorf("snapshot path outside repository") | |
| 139 | 67 | } | |
| 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 | |
| 68 | + | root, err := r.openRoot() | |
| 69 | + | if err != nil { | |
| 70 | + | return nil, err | |
| 145 | 71 | } | |
| 146 | - | return st, nil | |
| 72 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() | |
| 73 | + | return root.Lstat(rel) | |
| 147 | 74 | } | |
| 148 | 75 | ||
| 149 | 76 | // createTempAt creates an exclusively owned temporary file in an already | |
| 150 | 77 | // validated directory. os.CreateTemp(path) would resolve that path again. | |
| 151 | 78 | func (r *Repository) createTempAt(dir string) (temp *os.File, err error) { | |
| 152 | - | fd, err := r.openDirectory(dir) | |
| 79 | + | rel, ok := r.repositoryRelative(dir) | |
| 80 | + | if !ok { | |
| 81 | + | return nil, fmt.Errorf("snapshot path outside repository") | |
| 82 | + | } | |
| 83 | + | root, err := r.openRoot() | |
| 153 | 84 | if err != nil { | |
| 154 | 85 | return nil, err | |
| 155 | 86 | } | |
| 156 | 87 | 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) | |
| 88 | + | joinCloseError(&err, "close snapshot root", root.Close()) | |
| 89 | + | if err != nil && temp != nil { | |
| 90 | + | joinCloseError(&err, "close snapshot temporary file after root close failure", temp.Close()) | |
| 91 | + | temp = nil | |
| 163 | 92 | } | |
| 164 | 93 | }() | |
| 165 | 94 | for range 100 { | |
| @@ -168,19 +97,13 @@ func (r *Repository) createTempAt(dir string) (temp *os.File, err error) { | |||
| 168 | 97 | return nil, err | |
| 169 | 98 | } | |
| 170 | 99 | 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) { | |
| 100 | + | temp, err = root.OpenFile(filepath.Join(rel, name), syscall.O_WRONLY|syscall.O_CREAT|syscall.O_EXCL, 0o600) | |
| 101 | + | if errors.Is(err, os.ErrExist) { | |
| 173 | 102 | continue | |
| 174 | 103 | } | |
| 175 | 104 | if err != nil { | |
| 176 | 105 | return nil, err | |
| 177 | 106 | } | |
| 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 | 107 | return temp, nil | |
| 185 | 108 | } | |
| 186 | 109 | return nil, fmt.Errorf("create snapshot temporary file: too many collisions") | |
| @@ -9,108 +9,40 @@ 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. Each | 14 | + | func (r *Repository) openRoot() (*os.Root, error) { |
| 17 | - | // directory is pinned by an O_NOFOLLOW descriptor before its child is opened, | 15 | + | return os.OpenRoot(r.dir) |
| 18 | - | // so a concurrent rename or symlink replacement cannot redirect an operation | 16 | + | } |
| 19 | - | // outside the repository. | 17 | + | |
| 20 | - | func (r *Repository) repositoryFD(path string, finalFlags int, mode uint32) (*os.File, error) { | 18 | + | // rejectFinalSymlink preserves the pre-os.Root guarantee that the final |
| 21 | - | rel, ok := r.repositoryRelative(path) | 19 | + | // component is never a symlink. Root confinement is the hard boundary; this |
| 22 | - | if !ok { | 20 | + | // is defense-in-depth with a benign TOCTOU window inside a 0700 directory. |
| 23 | - | return nil, fmt.Errorf("snapshot path outside repository") | 21 | + | func rejectFinalSymlink(root *os.Root, rel string) error { |
| 24 | - | } | 22 | + | fi, err := root.Lstat(rel) |
| 25 | - | root, err := syscall.Open(r.dir, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0) | |||
| 26 | if err != nil { | 23 | if err != nil { | ||
| 27 | - | return nil, err | 24 | + | return err |
| 28 | } | 25 | } | ||
| 29 | - | if rel == "." { | 26 | + | if fi.Mode()&os.ModeSymlink != 0 { |
| 30 | - | if finalFlags&syscall.O_DIRECTORY == 0 { | 27 | + | return &os.PathError{Op: "open", Path: rel, Err: syscall.ELOOP} |
| 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 | } | 28 | } | ||
| 43 | - | parts := strings.Split(filepath.Clean(rel), string(filepath.Separator)) | 29 | + | return nil |
| 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 | - | } | |||
| 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 | |||
| 76 | - | } | |||
| 77 | - | return file, nil | |||
| 78 | } | 30 | } | ||
| 79 | 31 | ||||
| 80 | - | func (r *Repository) repositoryParent(path string) (int, string, error) { | 32 | + | func (r *Repository) openDirectory(path string) (file *os.File, err error) { |
| 81 | rel, ok := r.repositoryRelative(path) | 33 | rel, ok := r.repositoryRelative(path) | ||
| 82 | - | if !ok || rel == "." { | 34 | + | if !ok { |
| 83 | - | return -1, "", fmt.Errorf("snapshot path outside repository") | 35 | + | return nil, fmt.Errorf("snapshot path outside repository") |
| 84 | - | } | |||
| 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") | |||
| 88 | } | 36 | } | ||
| 89 | - | fd, err := syscall.Open(r.dir, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0) | 37 | + | root, err := r.openRoot() |
| 90 | if err != nil { | 38 | if err != nil { | ||
| 91 | - | return -1, "", err | 39 | + | return nil, err |
| 92 | } | 40 | } | ||
| 93 | - | for _, part := range parts[:len(parts)-1] { | 41 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() |
| 94 | - | if part == "" || part == "." || part == ".." { | 42 | + | if err := rejectFinalSymlink(root, rel); err != nil { |
| 95 | - | err = fmt.Errorf("invalid snapshot path") | 43 | + | return nil, err |
| 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 | |||
| 110 | - | } | |||
| 111 | - | fd = next | |||
| 112 | } | 44 | } | ||
| 113 | - | return fd, parts[len(parts)-1], nil | 45 | + | return root.OpenFile(rel, syscall.O_RDONLY|syscall.O_DIRECTORY, 0) |
| 114 | } | 46 | } | ||
| 115 | 47 | ||||
| 116 | func (r *Repository) repositoryRelative(path string) (string, bool) { | 48 | func (r *Repository) repositoryRelative(path string) (string, bool) { | ||
| @@ -128,38 +60,35 @@ func (r *Repository) repositoryRelative(path string) (string, bool) { | |||||
| 128 | return rel, true | 60 | return rel, true | ||
| 129 | } | 61 | } | ||
| 130 | 62 | ||||
| 131 | - | func (r *Repository) openDirectory(path string) (*os.File, error) { | 63 | + | func (r *Repository) stat(path string) (fi os.FileInfo, err error) { |
| 132 | - | return r.repositoryFD(path, syscall.O_RDONLY|syscall.O_DIRECTORY, 0) | 64 | + | rel, ok := r.repositoryRelative(path) |
| 133 | - | } | 65 | + | if !ok { |
| 134 | - | 66 | + | 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 | } | 67 | } | ||
| 140 | - | defer func() { | 68 | + | root, err := r.openRoot() |
| 141 | - | joinCloseError(&err, "close snapshot file", r.closeRepositoryFile(file, "close snapshot file")) | 69 | + | if err != nil { |
| 142 | - | }() | 70 | + | return nil, err |
| 143 | - | if err := syscall.Fstat(int(file.Fd()), &st); err != nil { | |||
| 144 | - | return syscall.Stat_t{}, err | |||
| 145 | } | 71 | } | ||
| 146 | - | return st, nil | 72 | + | defer func() { joinCloseError(&err, "close snapshot root", root.Close()) }() |
| 73 | + | return root.Lstat(rel) | |||
| 147 | } | 74 | } | ||
| 148 | 75 | ||||
| 149 | // createTempAt creates an exclusively owned temporary file in an already | 76 | // createTempAt creates an exclusively owned temporary file in an already | ||
| 150 | // validated directory. os.CreateTemp(path) would resolve that path again. | 77 | // validated directory. os.CreateTemp(path) would resolve that path again. | ||
| 151 | func (r *Repository) createTempAt(dir string) (temp *os.File, err error) { | 78 | func (r *Repository) createTempAt(dir string) (temp *os.File, err error) { | ||
| 152 | - | fd, err := r.openDirectory(dir) | 79 | + | rel, ok := r.repositoryRelative(dir) |
| 80 | + | if !ok { | |||
| 81 | + | return nil, fmt.Errorf("snapshot path outside repository") | |||
| 82 | + | } | |||
| 83 | + | root, err := r.openRoot() | |||
| 153 | if err != nil { | 84 | if err != nil { | ||
| 154 | return nil, err | 85 | return nil, err | ||
| 155 | } | 86 | } | ||
| 156 | defer func() { | 87 | defer func() { | ||
| 157 | - | if closeErr := r.closeRepositoryFile(fd, "close snapshot temporary directory"); closeErr != nil { | 88 | + | joinCloseError(&err, "close snapshot root", root.Close()) |
| 158 | - | if temp != nil { | 89 | + | 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")) | 90 | + | joinCloseError(&err, "close snapshot temporary file after root close failure", temp.Close()) |
| 160 | - | temp = nil | 91 | + | temp = nil |
| 161 | - | } | |||
| 162 | - | joinCloseError(&err, "close snapshot temporary directory", closeErr) | |||
| 163 | } | 92 | } | ||
| 164 | }() | 93 | }() | ||
| 165 | for range 100 { | 94 | for range 100 { | ||
| @@ -168,19 +97,13 @@ func (r *Repository) createTempAt(dir string) (temp *os.File, err error) { | |||||
| 168 | return nil, err | 97 | return nil, err | ||
| 169 | } | 98 | } | ||
| 170 | name := ".tmp-" + hex.EncodeToString(random[:]) | 99 | 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) | 100 | + | 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) { | 101 | + | if errors.Is(err, os.ErrExist) { |
| 173 | continue | 102 | continue | ||
| 174 | } | 103 | } | ||
| 175 | if err != nil { | 104 | if err != nil { | ||
| 176 | return nil, err | 105 | return nil, err | ||
| 177 | } | 106 | } | ||
| 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, nil | 107 | return temp, nil | ||
| 185 | } | 108 | } | ||
| 186 | return nil, fmt.Errorf("create snapshot temporary file: too many collisions") | 109 | return nil, fmt.Errorf("create snapshot temporary file: too many collisions") | ||