fix(snapshot): propagate safe path close errors
- commit
- f2d254a6d0f6f022a5e9f984af21db2eae19632d
- author
- brice · · 2026-07-22 04:26 UTC
- parent
- ed8ebfe
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +177 -41
M internal/adapters/snapshot/atomic.go +57 -18
| @@ -67,13 +67,19 @@ func (r *Repository) ensurePrivateDirectoryPhase(dir, phase string) error { | |||
| 67 | 67 | return err | |
| 68 | 68 | } | |
| 69 | 69 | created := false | |
| 70 | - | if err := syscall.Mkdirat(parent, name, 0o700); err == nil { | |
| 70 | + | mkdirErr := syscall.Mkdirat(parent, name, 0o700) | |
| 71 | + | if mkdirErr == nil { | |
| 71 | 72 | created = true | |
| 72 | - | } else if err != syscall.EEXIST { | |
| 73 | - | _ = syscall.Close(parent) | |
| 73 | + | } | |
| 74 | + | parentCloseErr := r.closeDescriptor(parent, "close snapshot parent directory") | |
| 75 | + | if mkdirErr != nil && mkdirErr != syscall.EEXIST { | |
| 76 | + | err = mkdirErr | |
| 77 | + | joinCloseError(&err, "close snapshot parent directory", parentCloseErr) | |
| 74 | 78 | return err | |
| 75 | 79 | } | |
| 76 | - | _ = syscall.Close(parent) | |
| 80 | + | if parentCloseErr != nil { | |
| 81 | + | return fmt.Errorf("close snapshot parent directory: %w", parentCloseErr) | |
| 82 | + | } | |
| 77 | 83 | opened, err := r.openDirectory(dir) | |
| 78 | 84 | if err != nil { | |
| 79 | 85 | return err | |
| @@ -160,7 +166,7 @@ func (r *Repository) withAtomicTemp(dir string, data []byte, publish func(string | |||
| 160 | 166 | cleanup := func(cause error) error { | |
| 161 | 167 | if !closed { | |
| 162 | 168 | if err := r.closeFile(temp); err != nil { | |
| 163 | - | cause = errors.Join(cause, err) | |
| 169 | + | cause = errors.Join(cause, fmt.Errorf("close snapshot temporary file: %w", err)) | |
| 164 | 170 | } | |
| 165 | 171 | closed = true | |
| 166 | 172 | } | |
| @@ -182,7 +188,7 @@ func (r *Repository) withAtomicTemp(dir string, data []byte, publish func(string | |||
| 182 | 188 | } | |
| 183 | 189 | if err := r.closeFile(temp); err != nil { | |
| 184 | 190 | closed = true | |
| 185 | - | return cleanup(err) | |
| 191 | + | return cleanup(fmt.Errorf("close snapshot temporary file: %w", err)) | |
| 186 | 192 | } | |
| 187 | 193 | closed = true | |
| 188 | 194 | removeTemp, err := publish(tempPath) | |
| @@ -222,13 +228,36 @@ func (r *Repository) closeFile(f *os.File) error { | |||
| 222 | 228 | if r.hooks.closeFile != nil { | |
| 223 | 229 | injected = r.hooks.closeFile(f.Name()) | |
| 224 | 230 | } | |
| 225 | - | closeErr := f.Close() | |
| 226 | - | if injected != nil { | |
| 227 | - | return injected | |
| 231 | + | return errors.Join(injected, f.Close()) | |
| 232 | + | } | |
| 233 | + | ||
| 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 | |
| 253 | + | // close failures, including cleanup-only paths, without suppressing either. | |
| 254 | + | func joinCloseError(primary *error, operation string, closeErr error) { | |
| 255 | + | if closeErr != nil { | |
| 256 | + | *primary = errors.Join(*primary, fmt.Errorf("%s: %w", operation, closeErr)) | |
| 228 | 257 | } | |
| 229 | - | return closeErr | |
| 230 | 258 | } | |
| 231 | - | func (r *Repository) installImmutable(oldPath, newPath string) error { | |
| 259 | + | ||
| 260 | + | func (r *Repository) installImmutable(oldPath, newPath string) (err error) { | |
| 232 | 261 | if r.hooks.installImmutable != nil { | |
| 233 | 262 | if err := r.hooks.installImmutable(newPath); err != nil { | |
| 234 | 263 | return err | |
| @@ -238,12 +267,16 @@ func (r *Repository) installImmutable(oldPath, newPath string) error { | |||
| 238 | 267 | if err != nil { | |
| 239 | 268 | return err | |
| 240 | 269 | } | |
| 241 | - | defer syscall.Close(oldFD) | |
| 270 | + | defer func() { | |
| 271 | + | joinCloseError(&err, "close source snapshot parent directory", r.closeDescriptor(oldFD, "close source snapshot parent directory")) | |
| 272 | + | }() | |
| 242 | 273 | newFD, newName, err := r.repositoryParent(newPath) | |
| 243 | 274 | if err != nil { | |
| 244 | 275 | return err | |
| 245 | 276 | } | |
| 246 | - | defer syscall.Close(newFD) | |
| 277 | + | defer func() { | |
| 278 | + | joinCloseError(&err, "close destination snapshot parent directory", r.closeDescriptor(newFD, "close destination snapshot parent directory")) | |
| 279 | + | }() | |
| 247 | 280 | oldPtr, err := syscall.BytePtrFromString(oldName) | |
| 248 | 281 | if err != nil { | |
| 249 | 282 | return err | |
| @@ -258,7 +291,7 @@ func (r *Repository) installImmutable(oldPath, newPath string) error { | |||
| 258 | 291 | } | |
| 259 | 292 | return nil | |
| 260 | 293 | } | |
| 261 | - | func (r *Repository) rename(oldPath, newPath string) error { | |
| 294 | + | func (r *Repository) rename(oldPath, newPath string) (err error) { | |
| 262 | 295 | if r.hooks.rename != nil { | |
| 263 | 296 | if err := r.hooks.rename(newPath); err != nil { | |
| 264 | 297 | return err | |
| @@ -268,15 +301,19 @@ func (r *Repository) rename(oldPath, newPath string) error { | |||
| 268 | 301 | if err != nil { | |
| 269 | 302 | return err | |
| 270 | 303 | } | |
| 271 | - | defer syscall.Close(oldFD) | |
| 304 | + | defer func() { | |
| 305 | + | joinCloseError(&err, "close source snapshot parent directory", r.closeDescriptor(oldFD, "close source snapshot parent directory")) | |
| 306 | + | }() | |
| 272 | 307 | newFD, newName, err := r.repositoryParent(newPath) | |
| 273 | 308 | if err != nil { | |
| 274 | 309 | return err | |
| 275 | 310 | } | |
| 276 | - | defer syscall.Close(newFD) | |
| 311 | + | defer func() { | |
| 312 | + | joinCloseError(&err, "close destination snapshot parent directory", r.closeDescriptor(newFD, "close destination snapshot parent directory")) | |
| 313 | + | }() | |
| 277 | 314 | return syscall.Renameat(oldFD, oldName, newFD, newName) | |
| 278 | 315 | } | |
| 279 | - | func (r *Repository) remove(path string) error { | |
| 316 | + | func (r *Repository) remove(path string) (err error) { | |
| 280 | 317 | if r.hooks.remove != nil { | |
| 281 | 318 | if err := r.hooks.remove(path); err != nil { | |
| 282 | 319 | return err | |
| @@ -286,7 +323,9 @@ func (r *Repository) remove(path string) error { | |||
| 286 | 323 | if err != nil { | |
| 287 | 324 | return err | |
| 288 | 325 | } | |
| 289 | - | defer syscall.Close(fd) | |
| 326 | + | defer func() { | |
| 327 | + | joinCloseError(&err, "close snapshot parent directory", r.closeDescriptor(fd, "close snapshot parent directory")) | |
| 328 | + | }() | |
| 290 | 329 | if err := syscall.Unlinkat(fd, name); err != syscall.EISDIR { | |
| 291 | 330 | return err | |
| 292 | 331 | } | |
| @@ -67,13 +67,19 @@ func (r *Repository) ensurePrivateDirectoryPhase(dir, phase string) error { | |||||
| 67 | return err | 67 | return err | ||
| 68 | } | 68 | } | ||
| 69 | created := false | 69 | created := false | ||
| 70 | - | if err := syscall.Mkdirat(parent, name, 0o700); err == nil { | 70 | + | mkdirErr := syscall.Mkdirat(parent, name, 0o700) |
| 71 | + | if mkdirErr == nil { | |||
| 71 | created = true | 72 | created = true | ||
| 72 | - | } else if err != syscall.EEXIST { | 73 | + | } |
| 73 | - | _ = syscall.Close(parent) | 74 | + | parentCloseErr := r.closeDescriptor(parent, "close snapshot parent directory") |
| 75 | + | if mkdirErr != nil && mkdirErr != syscall.EEXIST { | |||
| 76 | + | err = mkdirErr | |||
| 77 | + | joinCloseError(&err, "close snapshot parent directory", parentCloseErr) | |||
| 74 | return err | 78 | return err | ||
| 75 | } | 79 | } | ||
| 76 | - | _ = syscall.Close(parent) | 80 | + | if parentCloseErr != nil { |
| 81 | + | return fmt.Errorf("close snapshot parent directory: %w", parentCloseErr) | |||
| 82 | + | } | |||
| 77 | opened, err := r.openDirectory(dir) | 83 | opened, err := r.openDirectory(dir) | ||
| 78 | if err != nil { | 84 | if err != nil { | ||
| 79 | return err | 85 | return err | ||
| @@ -160,7 +166,7 @@ func (r *Repository) withAtomicTemp(dir string, data []byte, publish func(string | |||||
| 160 | cleanup := func(cause error) error { | 166 | cleanup := func(cause error) error { | ||
| 161 | if !closed { | 167 | if !closed { | ||
| 162 | if err := r.closeFile(temp); err != nil { | 168 | if err := r.closeFile(temp); err != nil { | ||
| 163 | - | cause = errors.Join(cause, err) | 169 | + | cause = errors.Join(cause, fmt.Errorf("close snapshot temporary file: %w", err)) |
| 164 | } | 170 | } | ||
| 165 | closed = true | 171 | closed = true | ||
| 166 | } | 172 | } | ||
| @@ -182,7 +188,7 @@ func (r *Repository) withAtomicTemp(dir string, data []byte, publish func(string | |||||
| 182 | } | 188 | } | ||
| 183 | if err := r.closeFile(temp); err != nil { | 189 | if err := r.closeFile(temp); err != nil { | ||
| 184 | closed = true | 190 | closed = true | ||
| 185 | - | return cleanup(err) | 191 | + | return cleanup(fmt.Errorf("close snapshot temporary file: %w", err)) |
| 186 | } | 192 | } | ||
| 187 | closed = true | 193 | closed = true | ||
| 188 | removeTemp, err := publish(tempPath) | 194 | removeTemp, err := publish(tempPath) | ||
| @@ -222,13 +228,36 @@ func (r *Repository) closeFile(f *os.File) error { | |||||
| 222 | if r.hooks.closeFile != nil { | 228 | if r.hooks.closeFile != nil { | ||
| 223 | injected = r.hooks.closeFile(f.Name()) | 229 | injected = r.hooks.closeFile(f.Name()) | ||
| 224 | } | 230 | } | ||
| 225 | - | closeErr := f.Close() | 231 | + | return errors.Join(injected, f.Close()) |
| 226 | - | if injected != nil { | 232 | + | } |
| 227 | - | return injected | 233 | + | |
| 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 | |||
| 253 | + | // close failures, including cleanup-only paths, without suppressing either. | |||
| 254 | + | func joinCloseError(primary *error, operation string, closeErr error) { | |||
| 255 | + | if closeErr != nil { | |||
| 256 | + | *primary = errors.Join(*primary, fmt.Errorf("%s: %w", operation, closeErr)) | |||
| 228 | } | 257 | } | ||
| 229 | - | return closeErr | |||
| 230 | } | 258 | } | ||
| 231 | - | func (r *Repository) installImmutable(oldPath, newPath string) error { | 259 | + | |
| 260 | + | func (r *Repository) installImmutable(oldPath, newPath string) (err error) { | |||
| 232 | if r.hooks.installImmutable != nil { | 261 | if r.hooks.installImmutable != nil { | ||
| 233 | if err := r.hooks.installImmutable(newPath); err != nil { | 262 | if err := r.hooks.installImmutable(newPath); err != nil { | ||
| 234 | return err | 263 | return err | ||
| @@ -238,12 +267,16 @@ func (r *Repository) installImmutable(oldPath, newPath string) error { | |||||
| 238 | if err != nil { | 267 | if err != nil { | ||
| 239 | return err | 268 | return err | ||
| 240 | } | 269 | } | ||
| 241 | - | defer syscall.Close(oldFD) | 270 | + | defer func() { |
| 271 | + | joinCloseError(&err, "close source snapshot parent directory", r.closeDescriptor(oldFD, "close source snapshot parent directory")) | |||
| 272 | + | }() | |||
| 242 | newFD, newName, err := r.repositoryParent(newPath) | 273 | newFD, newName, err := r.repositoryParent(newPath) | ||
| 243 | if err != nil { | 274 | if err != nil { | ||
| 244 | return err | 275 | return err | ||
| 245 | } | 276 | } | ||
| 246 | - | defer syscall.Close(newFD) | 277 | + | defer func() { |
| 278 | + | joinCloseError(&err, "close destination snapshot parent directory", r.closeDescriptor(newFD, "close destination snapshot parent directory")) | |||
| 279 | + | }() | |||
| 247 | oldPtr, err := syscall.BytePtrFromString(oldName) | 280 | oldPtr, err := syscall.BytePtrFromString(oldName) | ||
| 248 | if err != nil { | 281 | if err != nil { | ||
| 249 | return err | 282 | return err | ||
| @@ -258,7 +291,7 @@ func (r *Repository) installImmutable(oldPath, newPath string) error { | |||||
| 258 | } | 291 | } | ||
| 259 | return nil | 292 | return nil | ||
| 260 | } | 293 | } | ||
| 261 | - | func (r *Repository) rename(oldPath, newPath string) error { | 294 | + | func (r *Repository) rename(oldPath, newPath string) (err error) { |
| 262 | if r.hooks.rename != nil { | 295 | if r.hooks.rename != nil { | ||
| 263 | if err := r.hooks.rename(newPath); err != nil { | 296 | if err := r.hooks.rename(newPath); err != nil { | ||
| 264 | return err | 297 | return err | ||
| @@ -268,15 +301,19 @@ func (r *Repository) rename(oldPath, newPath string) error { | |||||
| 268 | if err != nil { | 301 | if err != nil { | ||
| 269 | return err | 302 | return err | ||
| 270 | } | 303 | } | ||
| 271 | - | defer syscall.Close(oldFD) | 304 | + | defer func() { |
| 305 | + | joinCloseError(&err, "close source snapshot parent directory", r.closeDescriptor(oldFD, "close source snapshot parent directory")) | |||
| 306 | + | }() | |||
| 272 | newFD, newName, err := r.repositoryParent(newPath) | 307 | newFD, newName, err := r.repositoryParent(newPath) | ||
| 273 | if err != nil { | 308 | if err != nil { | ||
| 274 | return err | 309 | return err | ||
| 275 | } | 310 | } | ||
| 276 | - | defer syscall.Close(newFD) | 311 | + | defer func() { |
| 312 | + | joinCloseError(&err, "close destination snapshot parent directory", r.closeDescriptor(newFD, "close destination snapshot parent directory")) | |||
| 313 | + | }() | |||
| 277 | return syscall.Renameat(oldFD, oldName, newFD, newName) | 314 | return syscall.Renameat(oldFD, oldName, newFD, newName) | ||
| 278 | } | 315 | } | ||
| 279 | - | func (r *Repository) remove(path string) error { | 316 | + | func (r *Repository) remove(path string) (err error) { |
| 280 | if r.hooks.remove != nil { | 317 | if r.hooks.remove != nil { | ||
| 281 | if err := r.hooks.remove(path); err != nil { | 318 | if err := r.hooks.remove(path); err != nil { | ||
| 282 | return err | 319 | return err | ||
| @@ -286,7 +323,9 @@ func (r *Repository) remove(path string) error { | |||||
| 286 | if err != nil { | 323 | if err != nil { | ||
| 287 | return err | 324 | return err | ||
| 288 | } | 325 | } | ||
| 289 | - | defer syscall.Close(fd) | 326 | + | defer func() { |
| 327 | + | joinCloseError(&err, "close snapshot parent directory", r.closeDescriptor(fd, "close snapshot parent directory")) | |||
| 328 | + | }() | |||
| 290 | if err := syscall.Unlinkat(fd, name); err != syscall.EISDIR { | 329 | if err := syscall.Unlinkat(fd, name); err != syscall.EISDIR { | ||
| 291 | return err | 330 | return err | ||
| 292 | } | 331 | } | ||
M internal/adapters/snapshot/atomic_test.go +33 -0
| @@ -5,6 +5,8 @@ import ( | |||
| 5 | 5 | "errors" | |
| 6 | 6 | "os" | |
| 7 | 7 | "path/filepath" | |
| 8 | + | "strings" | |
| 9 | + | "syscall" | |
| 8 | 10 | "testing" | |
| 9 | 11 | ) | |
| 10 | 12 | ||
| @@ -75,6 +77,37 @@ func TestWithAtomicTempPreservesLifecycleOrder(t *testing.T) { | |||
| 75 | 77 | } | |
| 76 | 78 | } | |
| 77 | 79 | ||
| 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 | + | ||
| 78 | 111 | func TestWriteMutableDoesNotRemoveConsumedTemporaryFile(t *testing.T) { | |
| 79 | 112 | repo := NewRepository(privateDir(t)) | |
| 80 | 113 | path := filepath.Join(repo.dir, "HEAD") | |
| @@ -5,6 +5,8 @@ import ( | |||||
| 5 | "errors" | 5 | "errors" | ||
| 6 | "os" | 6 | "os" | ||
| 7 | "path/filepath" | 7 | "path/filepath" | ||
| 8 | + | "strings" | |||
| 9 | + | "syscall" | |||
| 8 | "testing" | 10 | "testing" | ||
| 9 | ) | 11 | ) | ||
| 10 | 12 | ||||
| @@ -75,6 +77,37 @@ func TestWithAtomicTempPreservesLifecycleOrder(t *testing.T) { | |||||
| 75 | } | 77 | } | ||
| 76 | } | 78 | } | ||
| 77 | 79 | ||||
| 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 | + | ||||
| 78 | func TestWriteMutableDoesNotRemoveConsumedTemporaryFile(t *testing.T) { | 111 | func TestWriteMutableDoesNotRemoveConsumedTemporaryFile(t *testing.T) { | ||
| 79 | repo := NewRepository(privateDir(t)) | 112 | repo := NewRepository(privateDir(t)) | ||
| 80 | path := filepath.Join(repo.dir, "HEAD") | 113 | path := filepath.Join(repo.dir, "HEAD") | ||
M internal/adapters/snapshot/path_security_test.go +24 -0
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "fmt" | |
| 7 | 7 | "os" | |
| 8 | 8 | "path/filepath" | |
| 9 | + | "strings" | |
| 9 | 10 | "testing" | |
| 10 | 11 | ) | |
| 11 | 12 | ||
| @@ -43,6 +44,29 @@ func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) { | |||
| 43 | 44 | } | |
| 44 | 45 | } | |
| 45 | 46 | ||
| 47 | + | func TestStatReturnsInjectedCloseErrorWithContext(t *testing.T) { | |
| 48 | + | repo := NewRepository(privateDir(t)) | |
| 49 | + | if err := repo.ensurePrivateDirectory(repo.dir); err != nil { | |
| 50 | + | t.Fatal(err) | |
| 51 | + | } | |
| 52 | + | path := filepath.Join(repo.dir, "state") | |
| 53 | + | if err := os.WriteFile(path, []byte("state"), 0o600); err != nil { | |
| 54 | + | t.Fatal(err) | |
| 55 | + | } | |
| 56 | + | closeCause := errors.New("injected stat close failure") | |
| 57 | + | repo.hooks.closeDescriptor = func(got string) error { | |
| 58 | + | if got == "close snapshot file" { | |
| 59 | + | return closeCause | |
| 60 | + | } | |
| 61 | + | return nil | |
| 62 | + | } | |
| 63 | + | ||
| 64 | + | _, err := repo.stat(path) | |
| 65 | + | if !errors.Is(err, closeCause) || !strings.Contains(err.Error(), "close snapshot file") { | |
| 66 | + | t.Fatalf("stat error = %v, want contextual close failure", err) | |
| 67 | + | } | |
| 68 | + | } | |
| 69 | + | ||
| 46 | 70 | func TestRepositoryLoadLegacyChargesUnrelatedRootEntries(t *testing.T) { | |
| 47 | 71 | repo := NewRepository(privateDir(t)) | |
| 48 | 72 | if err := os.MkdirAll(repo.dir, 0o700); err != nil { | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | "os" | 7 | "os" | ||
| 8 | "path/filepath" | 8 | "path/filepath" | ||
| 9 | + | "strings" | |||
| 9 | "testing" | 10 | "testing" | ||
| 10 | ) | 11 | ) | ||
| 11 | 12 | ||||
| @@ -43,6 +44,29 @@ func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) { | |||||
| 43 | } | 44 | } | ||
| 44 | } | 45 | } | ||
| 45 | 46 | ||||
| 47 | + | func TestStatReturnsInjectedCloseErrorWithContext(t *testing.T) { | |||
| 48 | + | repo := NewRepository(privateDir(t)) | |||
| 49 | + | if err := repo.ensurePrivateDirectory(repo.dir); err != nil { | |||
| 50 | + | t.Fatal(err) | |||
| 51 | + | } | |||
| 52 | + | path := filepath.Join(repo.dir, "state") | |||
| 53 | + | if err := os.WriteFile(path, []byte("state"), 0o600); err != nil { | |||
| 54 | + | t.Fatal(err) | |||
| 55 | + | } | |||
| 56 | + | closeCause := errors.New("injected stat close failure") | |||
| 57 | + | repo.hooks.closeDescriptor = func(got string) error { | |||
| 58 | + | if got == "close snapshot file" { | |||
| 59 | + | return closeCause | |||
| 60 | + | } | |||
| 61 | + | return nil | |||
| 62 | + | } | |||
| 63 | + | ||||
| 64 | + | _, err := repo.stat(path) | |||
| 65 | + | if !errors.Is(err, closeCause) || !strings.Contains(err.Error(), "close snapshot file") { | |||
| 66 | + | t.Fatalf("stat error = %v, want contextual close failure", err) | |||
| 67 | + | } | |||
| 68 | + | } | |||
| 69 | + | ||||
| 46 | func TestRepositoryLoadLegacyChargesUnrelatedRootEntries(t *testing.T) { | 70 | func TestRepositoryLoadLegacyChargesUnrelatedRootEntries(t *testing.T) { | ||
| 47 | repo := NewRepository(privateDir(t)) | 71 | repo := NewRepository(privateDir(t)) | ||
| 48 | if err := os.MkdirAll(repo.dir, 0o700); err != nil { | 72 | if err := os.MkdirAll(repo.dir, 0o700); err != nil { | ||
M internal/adapters/snapshot/repository.go +1 -0
| @@ -65,6 +65,7 @@ type repositoryHooks struct { | |||
| 65 | 65 | writeTemp func(string) error | |
| 66 | 66 | syncFile func(string) error | |
| 67 | 67 | closeFile func(string) error | |
| 68 | + | closeDescriptor func(string) error | |
| 68 | 69 | installImmutable func(string) error | |
| 69 | 70 | rename func(string) error | |
| 70 | 71 | syncDirectory func(string) error | |
| @@ -65,6 +65,7 @@ type repositoryHooks struct { | |||||
| 65 | writeTemp func(string) error | 65 | writeTemp func(string) error | ||
| 66 | syncFile func(string) error | 66 | syncFile func(string) error | ||
| 67 | closeFile func(string) error | 67 | closeFile func(string) error | ||
| 68 | + | closeDescriptor func(string) error | |||
| 68 | installImmutable func(string) error | 69 | installImmutable func(string) error | ||
| 69 | rename func(string) error | 70 | rename func(string) error | ||
| 70 | syncDirectory func(string) error | 71 | syncDirectory func(string) error | ||
M internal/adapters/snapshot/safe_path.go +62 -23
| @@ -25,17 +25,25 @@ func (r *Repository) repositoryFD(path string, finalFlags int, mode uint32) (*os | |||
| 25 | 25 | } | |
| 26 | 26 | if rel == "." { | |
| 27 | 27 | if finalFlags&syscall.O_DIRECTORY == 0 { | |
| 28 | - | _ = syscall.Close(root) | |
| 29 | - | return nil, fmt.Errorf("invalid snapshot root operation") | |
| 28 | + | err = fmt.Errorf("invalid snapshot root operation") | |
| 29 | + | joinCloseError(&err, "close snapshot root directory", r.closeDescriptor(root, "close snapshot root directory")) | |
| 30 | + | return nil, err | |
| 31 | + | } | |
| 32 | + | file := os.NewFile(uintptr(root), path) | |
| 33 | + | if file == nil { | |
| 34 | + | err = fmt.Errorf("open snapshot root") | |
| 35 | + | joinCloseError(&err, "close snapshot root directory", r.closeDescriptor(root, "close snapshot root directory")) | |
| 36 | + | return nil, err | |
| 30 | 37 | } | |
| 31 | - | return os.NewFile(uintptr(root), path), nil | |
| 38 | + | return file, nil | |
| 32 | 39 | } | |
| 33 | 40 | parts := strings.Split(filepath.Clean(rel), string(filepath.Separator)) | |
| 34 | 41 | fd := root | |
| 35 | 42 | for i, part := range parts { | |
| 36 | 43 | if part == "" || part == "." || part == ".." { | |
| 37 | - | _ = syscall.Close(fd) | |
| 38 | - | return nil, fmt.Errorf("invalid snapshot path") | |
| 44 | + | err = fmt.Errorf("invalid snapshot path") | |
| 45 | + | joinCloseError(&err, "close snapshot path directory", r.closeDescriptor(fd, "close snapshot path directory")) | |
| 46 | + | return nil, err | |
| 39 | 47 | } | |
| 40 | 48 | flags := syscall.O_RDONLY | syscall.O_CLOEXEC | syscall.O_NOFOLLOW | |
| 41 | 49 | if i != len(parts)-1 { | |
| @@ -44,18 +52,26 @@ func (r *Repository) repositoryFD(path string, finalFlags int, mode uint32) (*os | |||
| 44 | 52 | flags = finalFlags | syscall.O_CLOEXEC | syscall.O_NOFOLLOW | |
| 45 | 53 | } | |
| 46 | 54 | next, openErr := syscall.Openat(fd, part, flags, mode) | |
| 47 | - | _ = syscall.Close(fd) | |
| 55 | + | closeErr := r.closeDescriptor(fd, "close snapshot path parent directory") | |
| 48 | 56 | if openErr != nil { | |
| 49 | - | return nil, openErr | |
| 57 | + | err = openErr | |
| 58 | + | joinCloseError(&err, "close snapshot path parent directory", closeErr) | |
| 59 | + | return nil, err | |
| 60 | + | } | |
| 61 | + | if closeErr != nil { | |
| 62 | + | err = fmt.Errorf("close snapshot path parent directory: %w", closeErr) | |
| 63 | + | joinCloseError(&err, "close snapshot path child directory", r.closeDescriptor(next, "close snapshot path child directory")) | |
| 64 | + | return nil, err | |
| 50 | 65 | } | |
| 51 | 66 | fd = next | |
| 52 | 67 | } | |
| 53 | - | f := os.NewFile(uintptr(fd), path) | |
| 54 | - | if f == nil { | |
| 55 | - | _ = syscall.Close(fd) | |
| 56 | - | return nil, fmt.Errorf("open snapshot path") | |
| 68 | + | file := os.NewFile(uintptr(fd), path) | |
| 69 | + | if file == nil { | |
| 70 | + | err = fmt.Errorf("open snapshot path") | |
| 71 | + | joinCloseError(&err, "close snapshot path descriptor", r.closeDescriptor(fd, "close snapshot path descriptor")) | |
| 72 | + | return nil, err | |
| 57 | 73 | } | |
| 58 | - | return f, nil | |
| 74 | + | return file, nil | |
| 59 | 75 | } | |
| 60 | 76 | ||
| 61 | 77 | func (r *Repository) repositoryParent(path string) (int, string, error) { | |
| @@ -73,13 +89,21 @@ func (r *Repository) repositoryParent(path string) (int, string, error) { | |||
| 73 | 89 | } | |
| 74 | 90 | for _, part := range parts[:len(parts)-1] { | |
| 75 | 91 | if part == "" || part == "." || part == ".." { | |
| 76 | - | _ = syscall.Close(fd) | |
| 77 | - | return -1, "", fmt.Errorf("invalid snapshot path") | |
| 92 | + | err = fmt.Errorf("invalid snapshot path") | |
| 93 | + | joinCloseError(&err, "close snapshot parent directory", r.closeDescriptor(fd, "close snapshot parent directory")) | |
| 94 | + | return -1, "", err | |
| 78 | 95 | } | |
| 79 | 96 | next, openErr := syscall.Openat(fd, part, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0) | |
| 80 | - | _ = syscall.Close(fd) | |
| 97 | + | closeErr := r.closeDescriptor(fd, "close snapshot parent directory") | |
| 81 | 98 | if openErr != nil { | |
| 82 | - | return -1, "", openErr | |
| 99 | + | err = openErr | |
| 100 | + | joinCloseError(&err, "close snapshot parent directory", closeErr) | |
| 101 | + | return -1, "", err | |
| 102 | + | } | |
| 103 | + | if closeErr != nil { | |
| 104 | + | err = fmt.Errorf("close snapshot parent directory: %w", closeErr) | |
| 105 | + | joinCloseError(&err, "close snapshot child directory", r.closeDescriptor(next, "close snapshot child directory")) | |
| 106 | + | return -1, "", err | |
| 83 | 107 | } | |
| 84 | 108 | fd = next | |
| 85 | 109 | } | |
| @@ -105,13 +129,14 @@ func (r *Repository) openDirectory(path string) (*os.File, error) { | |||
| 105 | 129 | return r.repositoryFD(path, syscall.O_RDONLY|syscall.O_DIRECTORY, 0) | |
| 106 | 130 | } | |
| 107 | 131 | ||
| 108 | - | func (r *Repository) stat(path string) (syscall.Stat_t, error) { | |
| 132 | + | func (r *Repository) stat(path string) (st syscall.Stat_t, err error) { | |
| 109 | 133 | file, err := r.repositoryFD(path, syscall.O_RDONLY|syscall.O_NONBLOCK, 0) | |
| 110 | 134 | if err != nil { | |
| 111 | 135 | return syscall.Stat_t{}, err | |
| 112 | 136 | } | |
| 113 | - | defer file.Close() | |
| 114 | - | var st syscall.Stat_t | |
| 137 | + | defer func() { | |
| 138 | + | joinCloseError(&err, "close snapshot file", r.closeRepositoryFile(file, "close snapshot file")) | |
| 139 | + | }() | |
| 115 | 140 | if err := syscall.Fstat(int(file.Fd()), &st); err != nil { | |
| 116 | 141 | return syscall.Stat_t{}, err | |
| 117 | 142 | } | |
| @@ -120,13 +145,21 @@ func (r *Repository) stat(path string) (syscall.Stat_t, error) { | |||
| 120 | 145 | ||
| 121 | 146 | // createTempAt creates an exclusively owned temporary file in an already | |
| 122 | 147 | // validated directory. os.CreateTemp(path) would resolve that path again. | |
| 123 | - | func (r *Repository) createTempAt(dir string) (*os.File, error) { | |
| 148 | + | func (r *Repository) createTempAt(dir string) (temp *os.File, err error) { | |
| 124 | 149 | fd, err := r.openDirectory(dir) | |
| 125 | 150 | if err != nil { | |
| 126 | 151 | return nil, err | |
| 127 | 152 | } | |
| 128 | - | defer fd.Close() | |
| 129 | - | for i := 0; i < 100; i++ { | |
| 153 | + | defer func() { | |
| 154 | + | if closeErr := r.closeRepositoryFile(fd, "close snapshot temporary directory"); closeErr != nil { | |
| 155 | + | if temp != nil { | |
| 156 | + | joinCloseError(&err, "close snapshot temporary file after directory close failure", r.closeRepositoryFile(temp, "close snapshot temporary file after directory close failure")) | |
| 157 | + | temp = nil | |
| 158 | + | } | |
| 159 | + | joinCloseError(&err, "close snapshot temporary directory", closeErr) | |
| 160 | + | } | |
| 161 | + | }() | |
| 162 | + | for range 100 { | |
| 130 | 163 | var random [8]byte | |
| 131 | 164 | if _, err := rand.Read(random[:]); err != nil { | |
| 132 | 165 | return nil, err | |
| @@ -139,7 +172,13 @@ func (r *Repository) createTempAt(dir string) (*os.File, error) { | |||
| 139 | 172 | if err != nil { | |
| 140 | 173 | return nil, err | |
| 141 | 174 | } | |
| 142 | - | return os.NewFile(uintptr(tempFD), filepath.Join(dir, name)), nil | |
| 175 | + | temp = os.NewFile(uintptr(tempFD), filepath.Join(dir, name)) | |
| 176 | + | if temp == nil { | |
| 177 | + | err = fmt.Errorf("open snapshot temporary file") | |
| 178 | + | joinCloseError(&err, "close snapshot temporary descriptor", r.closeDescriptor(tempFD, "close snapshot temporary descriptor")) | |
| 179 | + | return nil, err | |
| 180 | + | } | |
| 181 | + | return temp, nil | |
| 143 | 182 | } | |
| 144 | 183 | return nil, fmt.Errorf("create snapshot temporary file: too many collisions") | |
| 145 | 184 | } | |
| @@ -25,17 +25,25 @@ func (r *Repository) repositoryFD(path string, finalFlags int, mode uint32) (*os | |||||
| 25 | } | 25 | } | ||
| 26 | if rel == "." { | 26 | if rel == "." { | ||
| 27 | if finalFlags&syscall.O_DIRECTORY == 0 { | 27 | if finalFlags&syscall.O_DIRECTORY == 0 { | ||
| 28 | - | _ = syscall.Close(root) | 28 | + | err = fmt.Errorf("invalid snapshot root operation") |
| 29 | - | return nil, fmt.Errorf("invalid snapshot root operation") | 29 | + | joinCloseError(&err, "close snapshot root directory", r.closeDescriptor(root, "close snapshot root directory")) |
| 30 | + | return nil, err | |||
| 31 | + | } | |||
| 32 | + | file := os.NewFile(uintptr(root), path) | |||
| 33 | + | if file == nil { | |||
| 34 | + | err = fmt.Errorf("open snapshot root") | |||
| 35 | + | joinCloseError(&err, "close snapshot root directory", r.closeDescriptor(root, "close snapshot root directory")) | |||
| 36 | + | return nil, err | |||
| 30 | } | 37 | } | ||
| 31 | - | return os.NewFile(uintptr(root), path), nil | 38 | + | return file, nil |
| 32 | } | 39 | } | ||
| 33 | parts := strings.Split(filepath.Clean(rel), string(filepath.Separator)) | 40 | parts := strings.Split(filepath.Clean(rel), string(filepath.Separator)) | ||
| 34 | fd := root | 41 | fd := root | ||
| 35 | for i, part := range parts { | 42 | for i, part := range parts { | ||
| 36 | if part == "" || part == "." || part == ".." { | 43 | if part == "" || part == "." || part == ".." { | ||
| 37 | - | _ = syscall.Close(fd) | 44 | + | err = fmt.Errorf("invalid snapshot path") |
| 38 | - | return nil, fmt.Errorf("invalid snapshot path") | 45 | + | joinCloseError(&err, "close snapshot path directory", r.closeDescriptor(fd, "close snapshot path directory")) |
| 46 | + | return nil, err | |||
| 39 | } | 47 | } | ||
| 40 | flags := syscall.O_RDONLY | syscall.O_CLOEXEC | syscall.O_NOFOLLOW | 48 | flags := syscall.O_RDONLY | syscall.O_CLOEXEC | syscall.O_NOFOLLOW | ||
| 41 | if i != len(parts)-1 { | 49 | if i != len(parts)-1 { | ||
| @@ -44,18 +52,26 @@ func (r *Repository) repositoryFD(path string, finalFlags int, mode uint32) (*os | |||||
| 44 | flags = finalFlags | syscall.O_CLOEXEC | syscall.O_NOFOLLOW | 52 | flags = finalFlags | syscall.O_CLOEXEC | syscall.O_NOFOLLOW | ||
| 45 | } | 53 | } | ||
| 46 | next, openErr := syscall.Openat(fd, part, flags, mode) | 54 | next, openErr := syscall.Openat(fd, part, flags, mode) | ||
| 47 | - | _ = syscall.Close(fd) | 55 | + | closeErr := r.closeDescriptor(fd, "close snapshot path parent directory") |
| 48 | if openErr != nil { | 56 | if openErr != nil { | ||
| 49 | - | return nil, openErr | 57 | + | err = openErr |
| 58 | + | joinCloseError(&err, "close snapshot path parent directory", closeErr) | |||
| 59 | + | return nil, err | |||
| 60 | + | } | |||
| 61 | + | if closeErr != nil { | |||
| 62 | + | err = fmt.Errorf("close snapshot path parent directory: %w", closeErr) | |||
| 63 | + | joinCloseError(&err, "close snapshot path child directory", r.closeDescriptor(next, "close snapshot path child directory")) | |||
| 64 | + | return nil, err | |||
| 50 | } | 65 | } | ||
| 51 | fd = next | 66 | fd = next | ||
| 52 | } | 67 | } | ||
| 53 | - | f := os.NewFile(uintptr(fd), path) | 68 | + | file := os.NewFile(uintptr(fd), path) |
| 54 | - | if f == nil { | 69 | + | if file == nil { |
| 55 | - | _ = syscall.Close(fd) | 70 | + | err = fmt.Errorf("open snapshot path") |
| 56 | - | return nil, fmt.Errorf("open snapshot path") | 71 | + | joinCloseError(&err, "close snapshot path descriptor", r.closeDescriptor(fd, "close snapshot path descriptor")) |
| 72 | + | return nil, err | |||
| 57 | } | 73 | } | ||
| 58 | - | return f, nil | 74 | + | return file, nil |
| 59 | } | 75 | } | ||
| 60 | 76 | ||||
| 61 | func (r *Repository) repositoryParent(path string) (int, string, error) { | 77 | func (r *Repository) repositoryParent(path string) (int, string, error) { | ||
| @@ -73,13 +89,21 @@ func (r *Repository) repositoryParent(path string) (int, string, error) { | |||||
| 73 | } | 89 | } | ||
| 74 | for _, part := range parts[:len(parts)-1] { | 90 | for _, part := range parts[:len(parts)-1] { | ||
| 75 | if part == "" || part == "." || part == ".." { | 91 | if part == "" || part == "." || part == ".." { | ||
| 76 | - | _ = syscall.Close(fd) | 92 | + | err = fmt.Errorf("invalid snapshot path") |
| 77 | - | return -1, "", fmt.Errorf("invalid snapshot path") | 93 | + | joinCloseError(&err, "close snapshot parent directory", r.closeDescriptor(fd, "close snapshot parent directory")) |
| 94 | + | return -1, "", err | |||
| 78 | } | 95 | } | ||
| 79 | next, openErr := syscall.Openat(fd, part, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0) | 96 | next, openErr := syscall.Openat(fd, part, syscall.O_RDONLY|syscall.O_DIRECTORY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0) | ||
| 80 | - | _ = syscall.Close(fd) | 97 | + | closeErr := r.closeDescriptor(fd, "close snapshot parent directory") |
| 81 | if openErr != nil { | 98 | if openErr != nil { | ||
| 82 | - | return -1, "", openErr | 99 | + | err = openErr |
| 100 | + | joinCloseError(&err, "close snapshot parent directory", closeErr) | |||
| 101 | + | return -1, "", err | |||
| 102 | + | } | |||
| 103 | + | if closeErr != nil { | |||
| 104 | + | err = fmt.Errorf("close snapshot parent directory: %w", closeErr) | |||
| 105 | + | joinCloseError(&err, "close snapshot child directory", r.closeDescriptor(next, "close snapshot child directory")) | |||
| 106 | + | return -1, "", err | |||
| 83 | } | 107 | } | ||
| 84 | fd = next | 108 | fd = next | ||
| 85 | } | 109 | } | ||
| @@ -105,13 +129,14 @@ func (r *Repository) openDirectory(path string) (*os.File, error) { | |||||
| 105 | return r.repositoryFD(path, syscall.O_RDONLY|syscall.O_DIRECTORY, 0) | 129 | return r.repositoryFD(path, syscall.O_RDONLY|syscall.O_DIRECTORY, 0) | ||
| 106 | } | 130 | } | ||
| 107 | 131 | ||||
| 108 | - | func (r *Repository) stat(path string) (syscall.Stat_t, error) { | 132 | + | func (r *Repository) stat(path string) (st syscall.Stat_t, err error) { |
| 109 | file, err := r.repositoryFD(path, syscall.O_RDONLY|syscall.O_NONBLOCK, 0) | 133 | file, err := r.repositoryFD(path, syscall.O_RDONLY|syscall.O_NONBLOCK, 0) | ||
| 110 | if err != nil { | 134 | if err != nil { | ||
| 111 | return syscall.Stat_t{}, err | 135 | return syscall.Stat_t{}, err | ||
| 112 | } | 136 | } | ||
| 113 | - | defer file.Close() | 137 | + | defer func() { |
| 114 | - | var st syscall.Stat_t | 138 | + | joinCloseError(&err, "close snapshot file", r.closeRepositoryFile(file, "close snapshot file")) |
| 139 | + | }() | |||
| 115 | if err := syscall.Fstat(int(file.Fd()), &st); err != nil { | 140 | if err := syscall.Fstat(int(file.Fd()), &st); err != nil { | ||
| 116 | return syscall.Stat_t{}, err | 141 | return syscall.Stat_t{}, err | ||
| 117 | } | 142 | } | ||
| @@ -120,13 +145,21 @@ func (r *Repository) stat(path string) (syscall.Stat_t, error) { | |||||
| 120 | 145 | ||||
| 121 | // createTempAt creates an exclusively owned temporary file in an already | 146 | // createTempAt creates an exclusively owned temporary file in an already | ||
| 122 | // validated directory. os.CreateTemp(path) would resolve that path again. | 147 | // validated directory. os.CreateTemp(path) would resolve that path again. | ||
| 123 | - | func (r *Repository) createTempAt(dir string) (*os.File, error) { | 148 | + | func (r *Repository) createTempAt(dir string) (temp *os.File, err error) { |
| 124 | fd, err := r.openDirectory(dir) | 149 | fd, err := r.openDirectory(dir) | ||
| 125 | if err != nil { | 150 | if err != nil { | ||
| 126 | return nil, err | 151 | return nil, err | ||
| 127 | } | 152 | } | ||
| 128 | - | defer fd.Close() | 153 | + | defer func() { |
| 129 | - | for i := 0; i < 100; i++ { | 154 | + | if closeErr := r.closeRepositoryFile(fd, "close snapshot temporary directory"); closeErr != nil { |
| 155 | + | if temp != nil { | |||
| 156 | + | joinCloseError(&err, "close snapshot temporary file after directory close failure", r.closeRepositoryFile(temp, "close snapshot temporary file after directory close failure")) | |||
| 157 | + | temp = nil | |||
| 158 | + | } | |||
| 159 | + | joinCloseError(&err, "close snapshot temporary directory", closeErr) | |||
| 160 | + | } | |||
| 161 | + | }() | |||
| 162 | + | for range 100 { | |||
| 130 | var random [8]byte | 163 | var random [8]byte | ||
| 131 | if _, err := rand.Read(random[:]); err != nil { | 164 | if _, err := rand.Read(random[:]); err != nil { | ||
| 132 | return nil, err | 165 | return nil, err | ||
| @@ -139,7 +172,13 @@ func (r *Repository) createTempAt(dir string) (*os.File, error) { | |||||
| 139 | if err != nil { | 172 | if err != nil { | ||
| 140 | return nil, err | 173 | return nil, err | ||
| 141 | } | 174 | } | ||
| 142 | - | return os.NewFile(uintptr(tempFD), filepath.Join(dir, name)), nil | 175 | + | temp = os.NewFile(uintptr(tempFD), filepath.Join(dir, name)) |
| 176 | + | if temp == nil { | |||
| 177 | + | err = fmt.Errorf("open snapshot temporary file") | |||
| 178 | + | joinCloseError(&err, "close snapshot temporary descriptor", r.closeDescriptor(tempFD, "close snapshot temporary descriptor")) | |||
| 179 | + | return nil, err | |||
| 180 | + | } | |||
| 181 | + | return temp, nil | |||
| 143 | } | 182 | } | ||
| 144 | return nil, fmt.Errorf("create snapshot temporary file: too many collisions") | 183 | return nil, fmt.Errorf("create snapshot temporary file: too many collisions") | ||
| 145 | } | 184 | } | ||