diff --git a/internal/daemon/server.go b/internal/daemon/server.go index 21c44e0d6..0f21a3f7c 100644 --- a/internal/daemon/server.go +++ b/internal/daemon/server.go @@ -9,6 +9,8 @@ import ( "os" "sync" "time" + + "github.com/Gitlawb/zero/internal/fsutil" ) // Server is the daemon control plane. Mirrors reference-daemon-code-agent-js/ @@ -208,7 +210,7 @@ func (s *Server) writeStatusFile() error { if err != nil { return err } - if err := os.WriteFile(s.opts.Paths.Status, data, 0o600); err != nil { + if err := fsutil.WriteFileAtomic(s.opts.Paths.Status, data, 0o600); err != nil { return fmt.Errorf("daemon: write status file: %w", err) } return nil diff --git a/internal/daemon/status_test.go b/internal/daemon/status_test.go new file mode 100644 index 000000000..feab5d201 --- /dev/null +++ b/internal/daemon/status_test.go @@ -0,0 +1,182 @@ +package daemon + +import ( + "encoding/json" + "os" + "path/filepath" + "runtime" + "sync" + "sync/atomic" + "testing" + "time" +) + +func TestStatusFileAtomicPublicationConcurrentReaders(t *testing.T) { + launcher, _ := seqLauncher(&fakeWorker{pid: 1}) + srv, paths := newTestServer(t, launcher) + srv.startedAt = time.Now().UTC().Truncate(time.Millisecond) + + // Publish initial status + if err := srv.writeStatusFile(); err != nil { + t.Fatalf("initial writeStatusFile: %v", err) + } + + var stop atomic.Bool + var readerErrors atomic.Int64 + var readCount atomic.Int64 + var wg sync.WaitGroup + + numReaders := 8 + for i := 0; i < numReaders; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for !stop.Load() { + data, err := os.ReadFile(paths.Status) + if err != nil { + // Transient Windows absent window or retryable read + continue + } + if len(data) == 0 { + readerErrors.Add(1) + t.Errorf("observed empty status file during concurrent read") + return + } + var sf StatusFile + if err := json.Unmarshal(data, &sf); err != nil { + readerErrors.Add(1) + t.Errorf("observed partial or corrupted status JSON: %v (raw: %q)", err, string(data)) + return + } + if sf.PID != os.Getpid() { + readerErrors.Add(1) + t.Errorf("invalid PID in status file: got %d, want %d", sf.PID, os.Getpid()) + return + } + if sf.Socket != paths.Socket { + readerErrors.Add(1) + t.Errorf("invalid socket in status file: got %q, want %q", sf.Socket, paths.Socket) + return + } + if sf.Version < 1 { + readerErrors.Add(1) + t.Errorf("invalid version in status file: %d", sf.Version) + return + } + readCount.Add(1) + } + }() + } + + // Repeatedly update status file to stress concurrent reader/writer synchronization + numUpdates := 150 + for i := 1; i <= numUpdates; i++ { + srv.opts.Version = i + srv.startedAt = time.Now().UTC().Add(time.Duration(i) * time.Second).Truncate(time.Millisecond) + if err := srv.writeStatusFile(); err != nil { + t.Fatalf("writeStatusFile iteration %d: %v", i, err) + } + } + + stop.Store(true) + wg.Wait() + + if readerErrors.Load() > 0 { + t.Fatalf("%d reader errors detected during atomic status publication", readerErrors.Load()) + } + if readCount.Load() == 0 { + t.Fatal("no successful concurrent reads completed") + } +} + +func TestStatusFileFaultInjectionPreservesExisting(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("directory permissions-based fault injection skipped on Windows") + } + + launcher, _ := seqLauncher(&fakeWorker{pid: 1}) + srv, paths := newTestServer(t, launcher) + + initialStartedAt := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC) + srv.opts.Version = 10 + srv.startedAt = initialStartedAt + + // 1. Initial successful status publication + if err := srv.writeStatusFile(); err != nil { + t.Fatalf("initial writeStatusFile: %v", err) + } + + initialData, err := os.ReadFile(paths.Status) + if err != nil { + t.Fatalf("ReadFile initial status: %v", err) + } + var initialStatus StatusFile + if err := json.Unmarshal(initialData, &initialStatus); err != nil { + t.Fatalf("Unmarshal initial status: %v", err) + } + if initialStatus.Version != 10 { + t.Fatalf("initial version = %d, want 10", initialStatus.Version) + } + + // 2. Inject fault: make parent directory read-only so sibling temp file creation fails + dir := filepath.Dir(paths.Status) + if err := os.Chmod(dir, 0o500); err != nil { + t.Fatalf("Chmod dir to 0500: %v", err) + } + defer func() { _ = os.Chmod(dir, 0o700) }() + + // 3. Attempt update with new version, which must fail + srv.opts.Version = 99 + srv.startedAt = time.Now().UTC() + err = srv.writeStatusFile() + if err == nil { + t.Fatal("expected writeStatusFile to fail on read-only directory") + } + + // 4. Restore directory permissions + if err := os.Chmod(dir, 0o700); err != nil { + t.Fatalf("restore Chmod dir: %v", err) + } + + // 5. Verify the old status document survived unharmed and was not truncated + survivingData, err := os.ReadFile(paths.Status) + if err != nil { + t.Fatalf("ReadFile surviving status: %v", err) + } + if len(survivingData) == 0 { + t.Fatal("status file was truncated in place during failed write") + } + + var survivingStatus StatusFile + if err := json.Unmarshal(survivingData, &survivingStatus); err != nil { + t.Fatalf("surviving status file is corrupted: %v (raw: %q)", err, string(survivingData)) + } + if survivingStatus.Version != 10 { + t.Fatalf("surviving version = %d, want 10 (old document should be preserved)", survivingStatus.Version) + } + if !survivingStatus.StartedAt.Equal(initialStartedAt) { + t.Fatalf("surviving startedAt = %v, want %v", survivingStatus.StartedAt, initialStartedAt) + } +} + +func TestStatusFilePermissions(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Unix file permission checks skipped on Windows") + } + + launcher, _ := seqLauncher(&fakeWorker{pid: 1}) + srv, paths := newTestServer(t, launcher) + srv.startedAt = time.Now().UTC() + + if err := srv.writeStatusFile(); err != nil { + t.Fatalf("writeStatusFile: %v", err) + } + + info, err := os.Stat(paths.Status) + if err != nil { + t.Fatalf("Stat status file: %v", err) + } + if perm := info.Mode().Perm(); perm != 0o600 { + t.Fatalf("status file permissions = %04o, want 0600", perm) + } +} diff --git a/internal/fsutil/atomic_test.go b/internal/fsutil/atomic_test.go new file mode 100644 index 000000000..68f509fd5 --- /dev/null +++ b/internal/fsutil/atomic_test.go @@ -0,0 +1,191 @@ +package fsutil + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "path/filepath" + "runtime" + "sync" + "sync/atomic" + "testing" +) + +func TestWriteFileAtomicBasic(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "test.txt") + content := []byte("hello atomic world") + + if err := WriteFileAtomic(path, content, 0o600); err != nil { + t.Fatalf("WriteFileAtomic: %v", err) + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if !bytes.Equal(data, content) { + t.Fatalf("content = %q, want %q", data, content) + } + + info, err := os.Stat(path) + if err != nil { + t.Fatalf("Stat: %v", err) + } + if runtime.GOOS != "windows" { + if got := info.Mode().Perm(); got != 0o600 { + t.Fatalf("permissions = %04o, want 0600", got) + } + } + + // Overwrite existing file + newContent := []byte("updated content") + if err := WriteFileAtomic(path, newContent, 0o600); err != nil { + t.Fatalf("WriteFileAtomic overwrite: %v", err) + } + + data, err = os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile after overwrite: %v", err) + } + if !bytes.Equal(data, newContent) { + t.Fatalf("content after overwrite = %q, want %q", data, newContent) + } + + // Verify no temporary files remain + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("ReadDir: %v", err) + } + if len(entries) != 1 || entries[0].Name() != "test.txt" { + t.Fatalf("unexpected directory entries: %+v", entries) + } +} + +func TestWriteFileAtomicConcurrentReaders(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "data.json") + + type record struct { + Seq int `json:"seq"` + Padding string `json:"padding"` + } + + initial := record{Seq: 0, Padding: "initial"} + initData, _ := json.Marshal(initial) + if err := WriteFileAtomic(path, initData, 0o600); err != nil { + t.Fatalf("initial write: %v", err) + } + + var stop atomic.Bool + var readerErrors atomic.Int64 + var readCount atomic.Int64 + var wg sync.WaitGroup + + // Launch concurrent reader goroutines + numReaders := 8 + for i := 0; i < numReaders; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for !stop.Load() { + data, err := os.ReadFile(path) + if err != nil { + // On Windows, ReplaceFileW may briefly leave dst absent + continue + } + if len(data) == 0 { + readerErrors.Add(1) + t.Errorf("observed empty file during concurrent read") + return + } + var rec record + if err := json.Unmarshal(data, &rec); err != nil { + readerErrors.Add(1) + t.Errorf("observed corrupted/partial JSON: %v (raw: %q)", err, string(data)) + return + } + if rec.Seq < 0 { + readerErrors.Add(1) + t.Errorf("invalid sequence number: %d", rec.Seq) + return + } + readCount.Add(1) + } + }() + } + + // Writer publishes new versions sequentially + numWrites := 150 + for i := 1; i <= numWrites; i++ { + rec := record{ + Seq: i, + Padding: fmt.Sprintf("payload iteration %d with extended text to ensure multi-byte write", i), + } + data, err := json.Marshal(rec) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + if err := WriteFileAtomic(path, data, 0o600); err != nil { + t.Fatalf("WriteFileAtomic iter %d: %v", i, err) + } + } + + stop.Store(true) + wg.Wait() + + if readerErrors.Load() > 0 { + t.Fatalf("%d reader errors observed during concurrent writes", readerErrors.Load()) + } + if readCount.Load() == 0 { + t.Fatal("no successful reads completed") + } +} + +func TestWriteFileAtomicFaultPreservesExistingFile(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("read-only directory permissions differ on Windows") + } + + dir := t.TempDir() + path := filepath.Join(dir, "target.txt") + initialContent := []byte("original protected content") + + if err := WriteFileAtomic(path, initialContent, 0o600); err != nil { + t.Fatalf("initial write: %v", err) + } + + // Make parent directory read-only to force failure during temp file creation + if err := os.Chmod(dir, 0o500); err != nil { + t.Fatalf("Chmod dir: %v", err) + } + defer func() { _ = os.Chmod(dir, 0o700) }() + + // Attempt overwrite which must fail + err := WriteFileAtomic(path, []byte("new doomed content"), 0o600) + if err == nil { + t.Fatal("expected WriteFileAtomic to fail on read-only directory") + } + + // Restore permissions to inspect destination + if err := os.Chmod(dir, 0o700); err != nil { + t.Fatalf("restore Chmod dir: %v", err) + } + + // Verify original file is intact and unchanged + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if !bytes.Equal(data, initialContent) { + t.Fatalf("original content altered: got %q, want %q", data, initialContent) + } +} + +func TestSyncDir(t *testing.T) { + dir := t.TempDir() + if err := SyncDir(dir); err != nil { + t.Fatalf("SyncDir on valid directory failed: %v", err) + } +} diff --git a/internal/fsutil/rename.go b/internal/fsutil/rename.go index 4f74e8544..19b8190e1 100644 --- a/internal/fsutil/rename.go +++ b/internal/fsutil/rename.go @@ -5,11 +5,68 @@ import ( "errors" "fmt" "os" + "path/filepath" "runtime" "syscall" "time" ) +// WriteFileAtomic writes data to a temporary file in the same directory as filename, +// flushes and syncs it to disk, and replaces filename atomically via ReplaceWithRetry. +func WriteFileAtomic(filename string, data []byte, perm os.FileMode) error { + dir := filepath.Dir(filename) + if err := os.MkdirAll(dir, 0o755); err != nil { + return err + } + mode := perm + info, err := os.Lstat(filename) + switch { + case err == nil: + if info.Mode().IsRegular() { + mode = info.Mode().Perm() + } + case !os.IsNotExist(err): + return err + } + tmpFile, err := os.CreateTemp(dir, ".zero-tmp-*") + if err != nil { + return err + } + tmpName := tmpFile.Name() + closed := false + defer func() { + if !closed { + _ = tmpFile.Close() + } + _ = os.Remove(tmpName) + }() + + if err := tmpFile.Chmod(mode); err != nil { + return err + } + if _, err := tmpFile.Write(data); err != nil { + return err + } + if err := tmpFile.Sync(); err != nil { + return err + } + closed = true + if err := tmpFile.Close(); err != nil { + return err + } + + replaceErr := ReplaceWithRetry(tmpName, filename, nil) + if replaceErr == nil || isCommittedReplacement(replaceErr) { + _ = SyncDir(dir) + } + return replaceErr +} + +func isCommittedReplacement(err error) bool { + var committed *CommittedReplacementCleanupError + return errors.As(err, &committed) +} + // CommittedReplacementCleanupError reports that a replacement was committed, // but the old destination retained at BackupPath could not be removed. Callers // must treat the replacement itself as successful and surface the cleanup @@ -84,3 +141,24 @@ func isWindowsSharingOrLockViolation(err error) bool { } return false } + +// SyncDir fsyncs a directory so a file create or rename within it is durable +// across crashes. On platforms that do not support directory syncing (such as +// Windows), it returns nil. +func SyncDir(dir string) error { + if runtime.GOOS == "windows" { + // Windows does not support fsync on a directory handle; the rename is + // best-effort durable there. + return nil + } + d, err := os.Open(dir) + if err != nil { + return nil + } + syncErr := d.Sync() + closeErr := d.Close() + if syncErr != nil { + return syncErr + } + return closeErr +}