From b86f87ec59ff90ee4b29d9f2197a646d8abd3cf1 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 5 May 2026 02:32:50 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20predictable=20temporary=20filenames=20(Symlink=20attac?= =?UTF-8?q?k)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed predictable temporary filenames being used during atomic file writes (`path + ".tmp"`) in `os.TempDir()` within `internal/burnrate/burnrate.go` and `internal/sparkline/sparkline.go`. 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). The code now correctly utilizes `os.CreateTemp` to securely create an unpredictable temporary file in the target directory before writing to it and renaming it. Co-authored-by: himattm <6266621+himattm@users.noreply.github.com> --- .jules/sentinel.md | 4 ++++ internal/burnrate/burnrate.go | 18 ++++++++++++++++-- internal/sparkline/sparkline.go | 22 +++++++++++++++++++--- 3 files changed, 39 insertions(+), 5 deletions(-) create mode 100644 .jules/sentinel.md diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..3781369 --- /dev/null +++ b/.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. diff --git a/internal/burnrate/burnrate.go b/internal/burnrate/burnrate.go index 8cb680c..176158b 100644 --- a/internal/burnrate/burnrate.go +++ b/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 { diff --git a/internal/sparkline/sparkline.go b/internal/sparkline/sparkline.go index f4c755c..c39ae12 100644 --- a/internal/sparkline/sparkline.go +++ b/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