Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions backend/internal/atomicfile/syncdir.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Package atomicfile contains the platform-specific durability steps used by
// same-directory atomic file replacements.
package atomicfile

// SyncDirectory makes a completed rename durable on platforms that support
// syncing directory handles. Platforms without that operation may implement
// this as a no-op after the renamed file itself has been synced.
func SyncDirectory(directory string) error {
return syncDirectory(directory)
}
18 changes: 18 additions & 0 deletions backend/internal/atomicfile/syncdir_nonwindows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//go:build !windows

package atomicfile

import "os"

func syncDirectory(directory string) error {
directoryHandle, err := os.Open(directory)
if err != nil {
return err
}
syncErr := directoryHandle.Sync()
closeErr := directoryHandle.Close()
if syncErr != nil {
return syncErr
}
return closeErr
}
16 changes: 16 additions & 0 deletions backend/internal/atomicfile/syncdir_nonwindows_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//go:build !windows

package atomicfile

import (
"os"
"path/filepath"
"testing"
)

func TestSyncDirectoryReturnsOpenErrors(t *testing.T) {
missing := filepath.Join(t.TempDir(), "missing")
if err := SyncDirectory(missing); !os.IsNotExist(err) {
t.Fatalf("SyncDirectory error = %v, want not-exist error", err)
}
}
9 changes: 9 additions & 0 deletions backend/internal/atomicfile/syncdir_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package atomicfile

import "testing"

func TestSyncDirectoryAcceptsExistingDirectory(t *testing.T) {
if err := SyncDirectory(t.TempDir()); err != nil {
t.Fatal(err)
}
}
10 changes: 10 additions & 0 deletions backend/internal/atomicfile/syncdir_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
//go:build windows

package atomicfile

func syncDirectory(string) error {
// os.File.Sync uses FlushFileBuffers on Windows. Directory handles opened
// through os.Open cannot be flushed and return ERROR_ACCESS_DENIED, even
// though the preceding rename has already completed.
return nil
}
15 changes: 2 additions & 13 deletions backend/internal/backup/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"sync"
"time"

"github.com/video-site/backend/internal/atomicfile"
"github.com/video-site/backend/internal/catalog"
"github.com/video-site/backend/internal/config"
"github.com/video-site/backend/internal/localpath"
Expand Down Expand Up @@ -1124,19 +1125,7 @@ func writeJSONAtomic(filePath string, value any, mode os.FileMode) error {
return err
}
removeTemporary = false
directoryHandle, err := os.Open(directory)
if err != nil {
return err
}
syncErr := directoryHandle.Sync()
closeErr := directoryHandle.Close()
if syncErr != nil {
return syncErr
}
if closeErr != nil {
return closeErr
}
return nil
return atomicfile.SyncDirectory(directory)
}

func metaPath(archivePath string) string {
Expand Down
9 changes: 9 additions & 0 deletions backend/internal/backuptransfer/permissions_nonwindows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//go:build !windows

package backuptransfer

import "os"

func stateFilePermissionsTooBroad(mode os.FileMode) bool {
return mode.Perm()&0o077 != 0
}
23 changes: 23 additions & 0 deletions backend/internal/backuptransfer/permissions_nonwindows_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//go:build !windows

package backuptransfer

import (
"os"
"testing"
)

func TestStateFilePermissionsTooBroad(t *testing.T) {
for _, test := range []struct {
mode uint32
broad bool
}{
{mode: 0o600, broad: false},
{mode: 0o640, broad: true},
{mode: 0o606, broad: true},
} {
if broad := stateFilePermissionsTooBroad(os.FileMode(test.mode)); broad != test.broad {
t.Fatalf("mode %#o broad = %t, want %t", test.mode, broad, test.broad)
}
}
}
12 changes: 12 additions & 0 deletions backend/internal/backuptransfer/permissions_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//go:build windows

package backuptransfer

import "os"

func stateFilePermissionsTooBroad(os.FileMode) bool {
// Windows FileMode permission bits are synthetic: writable files report
// 0666, and Chmod only changes the read-only attribute. The state directory
// ACL is the access-control boundary on this platform.
return false
}
14 changes: 14 additions & 0 deletions backend/internal/backuptransfer/permissions_windows_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//go:build windows

package backuptransfer

import (
"os"
"testing"
)

func TestStateFilePermissionsIgnoreSyntheticWindowsBits(t *testing.T) {
if stateFilePermissionsTooBroad(os.FileMode(0o666)) {
t.Fatal("synthetic Windows permission bits were rejected")
}
}
14 changes: 3 additions & 11 deletions backend/internal/backuptransfer/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strings"
"time"

"github.com/video-site/backend/internal/atomicfile"
"github.com/video-site/backend/internal/backup"
)

Expand Down Expand Up @@ -59,7 +60,7 @@ func readJSONFile(path string, destination any) error {
if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() {
return errors.New("backup transfer: state is not a regular file")
}
if info.Mode().Perm()&0o077 != 0 {
if stateFilePermissionsTooBroad(info.Mode()) {
return errors.New("backup transfer: state file permissions are too broad")
}
if info.Size() > maxStateFileBytes {
Expand Down Expand Up @@ -119,16 +120,7 @@ func writeJSONAtomic(path string, value any) error {
return err
}
removeTemporary = false
directoryHandle, err := os.Open(directory)
if err != nil {
return err
}
syncErr := directoryHandle.Sync()
closeErr := directoryHandle.Close()
if syncErr != nil {
return syncErr
}
return closeErr
return atomicfile.SyncDirectory(directory)
}

func validOpaqueID(value string) bool {
Expand Down
6 changes: 2 additions & 4 deletions backend/internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strings"
"time"

"github.com/video-site/backend/internal/atomicfile"
"github.com/video-site/backend/internal/localpath"
"github.com/video-site/backend/internal/schedule"
"gopkg.in/yaml.v3"
Expand Down Expand Up @@ -493,10 +494,7 @@ func writeFileAtomically(path string, data []byte, mode os.FileMode) error {
// Windows) reject syncing directory handles. The rename has already committed
// at this point, so returning an error would falsely tell callers that the
// save failed even though config.yaml was replaced.
if dirHandle, err := os.Open(dir); err == nil {
_ = dirHandle.Sync()
_ = dirHandle.Close()
}
_ = atomicfile.SyncDirectory(dir)
return nil
}

Expand Down
Loading