🛡️ Sentinel: [CRITICAL] Fix predictable temporary filenames (Symlink attack)

.jules/sentinel.md

@@ -0,0 +1,4 @@
+## 2025-01-20 - Predictable Temporary Filenames Vulnerability
+**Vulnerability:** Found predictable temporary filenames being used during atomic file writes (`path + ".tmp"`) in `os.TempDir()` within `internal/burnrate/burnrate.go` and `internal/sparkline/sparkline.go`.
+**Learning:** Atomic file writes often use a predictable `.tmp` suffix to create a temporary file before renaming. In shared directories like `os.TempDir()`, this predictable naming allows attackers to pre-create the file as a symlink, tricking the application into overwriting unintended files (Symlink attack).
+**Prevention:** Always use `os.CreateTemp` with an unpredictable pattern (e.g., `filepath.Base(path)+"-*"`) for temporary files. Ensure the file is created in the same directory as the target (`filepath.Dir(path)`), explicitly set permissions with `f.Chmod` if needed, and call `f.Close()` before renaming to prevent cross-platform issues.

internal/burnrate/burnrate.go

@@ -64,8 +64,22 @@ func LoadOrCreateSnapshotAt(sessionID string, currentCost float64, now time.Time
 		return nil, false, err
 	}
 
-	tmpPath := path + ".tmp"
-	if err := os.WriteFile(tmpPath, data, 0644); err != nil {
+	f, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+"-*")
+	if err != nil {
+		return nil, false, err
+	}
+	tmpPath := f.Name()
+	if _, err := f.Write(data); err != nil {
+		f.Close()
+		os.Remove(tmpPath)
+		return nil, false, err
+	}
+	if err := f.Close(); err != nil {
+		os.Remove(tmpPath)
+		return nil, false, err
+	}
+	if err := os.Chmod(tmpPath, 0644); err != nil {
+		os.Remove(tmpPath)
 		return nil, false, err
 	}
 	if err := os.Rename(tmpPath, path); err != nil {

internal/sparkline/sparkline.go

@@ -138,11 +138,27 @@ func Save(sessionID, metric string, b *Buffer) {
 	if err != nil {
 		return
 	}
-	tmp := path + ".tmp"
-	if err := os.WriteFile(tmp, data, 0644); err != nil {
+	f, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+"-*")
+	if err != nil {
+		return
+	}
+	tmp := f.Name()
+	if _, err := f.Write(data); err != nil {
+		f.Close()
+		os.Remove(tmp)
+		return
+	}
+	if err := f.Close(); err != nil {
+		os.Remove(tmp)
+		return
+	}
+	if err := os.Chmod(tmp, 0644); err != nil {
+		os.Remove(tmp)
 		return
 	}
-	os.Rename(tmp, path)
+	if err := os.Rename(tmp, path); err != nil {
+		os.Remove(tmp)
+	}
 }
 
 // PushAndSave is a convenience that loads, pushes, saves, and returns the buffer