🛡️ Sentinel: [Medium] Fix Deadlock Vulnerability in Process Output Pipes

.jules/sentinel.md

@@ -12,3 +12,8 @@
 **Vulnerability:** A process can block (deadlock) when its stdout/stderr pipe fills before the parent reads it, because the child blocks on `write()` while the parent blocks on `waitUntilExit()`.
 **Learning:** `pipe.fileHandleForReading.readDataToEndOfFile()` after `process.waitUntilExit()` is the deadlock pattern. Default macOS pipe buffer is ~64KB.
 **Prevention:** Read the pipe before/concurrently-with waiting for exit. The simplest pattern is to perform the read inside the same background queue that calls `waitUntilExit()`, capturing the bytes for the caller to use after the dispatch group resolves.
+
+## 2024-05-11 - Swift Concurrency Deadlocks with `Process` Pipes
+**Vulnerability:** When executing a child `Process`, if a subprocess outputs more data than the OS pipe buffer limit (typically 64KB), the child blocks waiting for the parent to read. If the parent waits for the child to exit before reading, or reads one pipe synchronously to completion before reading the other, a deadlock occurs.
+**Learning:** `FileHandle.readToEnd()` is a synchronous, blocking function. Attempting to use it with `async let` causes compilation errors. The correct way to read pipes concurrently without deadlocks is to use `FileHandle.readabilityHandler` and safely accumulate data with a lock.
+**Prevention:** Drain both `stdout` and `stderr` pipes concurrently using `readabilityHandler` to prevent the child process from blocking on full buffers. Ensure the `readabilityHandler` is set to `nil` inside the `terminationHandler` and read any remaining data to avoid truncation.

Sources/Cacheout/Intervention/Tier2Interventions.swift

@@ -20,6 +20,18 @@ private enum XPCResult<T> {
     case failed(String)
 }
 
+private final class LockedData: @unchecked Sendable {
+    private var _data = Data()
+    private let lock = NSLock()
+
+    var data: Data { lock.withLock { _data } }
+
+    func append(_ newData: Data) {
+        lock.withLock { _data.append(newData) }
+    }
+}
+
+
 /// Thread-safe once-box for resuming a continuation exactly once.
 /// Uses an unfair lock for minimal overhead in the XPC callback path.
 private final class OnceResumer<T>: @unchecked Sendable {
@@ -731,39 +743,52 @@ public final class SnapshotCleanup: Intervention {
         return try await withCheckedThrowingContinuation { continuation in
             let resumer = ThrowingOnceResumer(continuation)
 
+            // Use readabilityHandler to read concurrently and avoid deadlocks
+            let stdoutData = LockedData()
+            let stderrData = LockedData()
+
+            stdoutHandle.readabilityHandler = { handle in
+                stdoutData.append(handle.availableData)
+            }
+
+            stderrHandle.readabilityHandler = { handle in
+                stderrData.append(handle.availableData)
+            }
+
             process.terminationHandler = { proc in
-                // If the timeout task already fired, map any exit to .timeout
-                // to avoid reporting a misleading SIGTERM/SIGKILL status.
+                stdoutHandle.readabilityHandler = nil
+                stderrHandle.readabilityHandler = nil
+
+                // Read remaining data to avoid truncation
+                if let remaining = try? stdoutHandle.readToEnd() {
+                    stdoutData.append(remaining)
+                }
+                if let remaining = try? stderrHandle.readToEnd() {
+                    stderrData.append(remaining)
+                }
+
                 if timedOutFlag.value {
                     resumer.resume(with: .failure(SnapshotError.timeout))
                     return
                 }
 
-                let data = stdoutHandle.readDataToEndOfFile()
-                let output = String(data: data, encoding: .utf8) ?? ""
-
-                // Check terminationStatus — non-zero indicates failure.
+                let output = String(data: stdoutData.data, encoding: .utf8) ?? ""
                 if proc.terminationStatus != 0 {
-                    let errData = stderrHandle.readDataToEndOfFile()
-                    let errOutput = String(data: errData, encoding: .utf8) ?? "unknown"
+                    let errOutput = String(data: stderrData.data, encoding: .utf8) ?? "unknown"
                     resumer.resume(with: .failure(SnapshotError.listFailed(
                         status: proc.terminationStatus, stderr: errOutput)))
                     return
                 }
 
-                // tmutil output lines like "com.apple.TimeMachine.2024-01-15-123456.local"
-                // Extract the date portion.
                 let dates = output.components(separatedBy: .newlines)
                     .compactMap { line -> String? in
                         let trimmed = line.trimmingCharacters(in: .whitespaces)
-                        // Extract date from "com.apple.TimeMachine.YYYY-MM-DD-HHMMSS.local"
                         guard trimmed.hasPrefix("com.apple.TimeMachine.") else { return nil }
                         let stripped = trimmed
                             .replacingOccurrences(of: "com.apple.TimeMachine.", with: "")
                             .replacingOccurrences(of: ".local", with: "")
                         return stripped.isEmpty ? nil : stripped
                     }
-
                 resumer.resume(with: .success(dates))
             }