fix(xray): write generated config atomically (#5494)

internal/xray/process.go

@@ -526,7 +526,7 @@ func (p *process) Start() (err error) {
 	if p.configPath != "" {
 		configPath = p.configPath
 	}
-	err = os.WriteFile(configPath, data, 0644)
+	err = writeFileAtomic(configPath, data, 0o600)
 	if err != nil {
 		return common.NewErrorf("Failed to write configuration file: %v", err)
 	}
@@ -546,6 +546,54 @@ func (p *process) Start() (err error) {
 	return nil
 }
 
+// writeFileAtomic writes data to path via a same-directory temp file that is
+// permissioned, synced, and renamed into place, so a crash can never leave a
+// partial config; the config holds credentials, hence the 0600 perm. After the
+// rename the parent directory is fsynced to persist the directory entry. That
+// final step is skipped on Windows, where directory fsync is unsupported and
+// os.Rename already uses replace-existing semantics.
+func writeFileAtomic(path string, data []byte, perm os.FileMode) (err error) {
+	dir := filepath.Dir(path)
+	tmp, err := os.CreateTemp(dir, ".config-*.tmp")
+	if err != nil {
+		return err
+	}
+	tmpPath := tmp.Name()
+	defer func() {
+		_ = tmp.Close()
+		if err != nil {
+			_ = os.Remove(tmpPath)
+		}
+	}()
+	if err = tmp.Chmod(perm); err != nil {
+		return err
+	}
+	if _, err = tmp.Write(data); err != nil {
+		return err
+	}
+	if err = tmp.Sync(); err != nil {
+		return err
+	}
+	if err = tmp.Close(); err != nil {
+		return err
+	}
+	if err = renameFile(tmpPath, path); err != nil {
+		return err
+	}
+	if runtime.GOOS == "windows" {
+		return nil
+	}
+	dirHandle, err := os.Open(dir)
+	if err != nil {
+		return err
+	}
+	err = dirHandle.Sync()
+	_ = dirHandle.Close()
+	return err
+}
+
+var renameFile = os.Rename
+
 func (p *process) startCommand(cmd *exec.Cmd) error {
 	p.mu.Lock()
 	p.cmd = cmd

internal/xray/process_test.go

@@ -3,6 +3,7 @@
 package xray
 
 import (
+	"errors"
 	"os"
 	"os/exec"
 	"os/signal"
@@ -15,6 +16,52 @@ import (
 	"github.com/op/go-logging"
 )
 
+func TestWriteFileAtomicModeAndRenameFailure(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "config.json")
+	if err := os.WriteFile(path, []byte("old"), 0o644); err != nil {
+		t.Fatalf("seed: %v", err)
+	}
+	if err := writeFileAtomic(path, []byte("new"), 0o600); err != nil {
+		t.Fatalf("writeFileAtomic: %v", err)
+	}
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("read: %v", err)
+	}
+	if string(data) != "new" {
+		t.Fatalf("content = %q, want new", data)
+	}
+	info, err := os.Stat(path)
+	if err != nil {
+		t.Fatalf("stat: %v", err)
+	}
+	if info.Mode().Perm() != 0o600 {
+		t.Fatalf("mode = %o, want 600", info.Mode().Perm())
+	}
+
+	originalRename := renameFile
+	renameFile = func(_, _ string) error { return errors.New("injected rename failure") }
+	t.Cleanup(func() { renameFile = originalRename })
+	if err := writeFileAtomic(path, []byte("partial"), 0o600); err == nil {
+		t.Fatal("rename failure = nil")
+	}
+	data, err = os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("read preserved file: %v", err)
+	}
+	if string(data) != "new" {
+		t.Fatalf("content after failed rename = %q, want committed content", data)
+	}
+	matches, err := filepath.Glob(filepath.Join(dir, ".config-*.tmp"))
+	if err != nil {
+		t.Fatalf("glob: %v", err)
+	}
+	if len(matches) != 0 {
+		t.Fatalf("temporary files leaked: %v", matches)
+	}
+}
+
 func TestStopWaitsForGracefulExit(t *testing.T) {
 	initProcessTestLogger(t)