From d536f59a75b4621463d667c034d97a2b819497c8 Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Fri, 12 Jun 2026 06:19:01 +0000
Subject: [PATCH] Fix TOCTOU vulnerability in SysctlJournal temp file creation

Co-authored-by: acebytes <2820910+acebytes@users.noreply.github.com>
---
 .jules/sentinel.md                            |  4 +++
 Sources/CacheoutHelperLib/SysctlJournal.swift | 35 +++++++++++++++----
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/.jules/sentinel.md b/.jules/sentinel.md
index fd58571..0a17000 100644
--- a/.jules/sentinel.md
+++ b/.jules/sentinel.md
@@ -31,3 +31,7 @@
 **Vulnerability:** External shell command executed in `listLocalSnapshots()` triggered a deadlock when `tmutil` output exceeded 64KB, because stdout and stderr were read synchronously inside the process termination handler.
 **Learning:** In Swift, reading from a process pipe synchronously inside a `terminationHandler` can result in a permanent deadlock if the child blocks writing to a full pipe, preventing it from exiting.
 **Prevention:** Asynchronously drain pipes continuously while the process is running using background queues.
+## 2026-05-02 - Fixed insecure file creation permissions
+**Vulnerability:** Insecure file creation permissions
+**Learning:** Data.write(to:) followed by FileManager.default.setAttributes creates a TOCTOU vulnerability because the file is initially created with the process's default umask before being restricted.
+**Prevention:** Always use POSIX open() with O_CREAT, O_EXCL, and the desired permissions when creating files securely to avoid TOCTOU vulnerabilities, then wrap the file descriptor in a FileHandle for writing.
diff --git a/Sources/CacheoutHelperLib/SysctlJournal.swift b/Sources/CacheoutHelperLib/SysctlJournal.swift
index a14c902..36a56c1 100644
--- a/Sources/CacheoutHelperLib/SysctlJournal.swift
+++ b/Sources/CacheoutHelperLib/SysctlJournal.swift
@@ -307,14 +307,35 @@ public final class SysctlJournal {
         do {
             let data = try PropertyListEncoder().encode(state)
 
-            // Write to temp file (non-atomic — we control the rename ourselves).
-            try data.write(to: tmpURL)
+            // Remove any stale temp file to prevent O_EXCL failure.
+            try? FileManager.default.removeItem(at: tmpURL)
 
-            // Set permissions to 0600 (root-only) on temp file before rename.
-            try FileManager.default.setAttributes(
-                [.posixPermissions: 0o600],
-                ofItemAtPath: tmpURL.path
-            )
+            // Securely create temp file with 0600 permissions to avoid TOCTOU.
+            let fd = tmpURL.withUnsafeFileSystemRepresentation { pathPtr -> Int32 in
+                guard let pathPtr = pathPtr else { return -1 }
+                return open(pathPtr, O_CREAT | O_WRONLY | O_EXCL | O_CLOEXEC, 0o600)
+            }
+
+            guard fd != -1 else {
+                let err = String(cString: strerror(errno))
+                logger.error("Failed to securely open temp file: \(err, privacy: .public)")
+                return false
+            }
+
+            let handle = FileHandle(fileDescriptor: fd, closeOnDealloc: true)
+            do {
+                if #available(macOS 10.15.4, *) {
+                    try handle.write(contentsOf: data)
+                    try handle.close()
+                } else {
+                    handle.write(data)
+                    handle.closeFile()
+                }
+            } catch {
+                logger.error("Failed to write to temp file: \(error.localizedDescription, privacy: .public)")
+                try? FileManager.default.removeItem(at: tmpURL)
+                return false
+            }
 
             // Atomic rename(2) — atomicity on APFS/HFS+.
             if rename(tmpURL.path, url.path) != 0 {