From 539ef46c7b53f3f1adaf206dcb2f68b2abd0ce0d Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 10 Jul 2026 09:03:57 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH]?= =?UTF-8?q?=20Fix=20Process()=20pipe=20buffer=20deadlock=20in=20Automation?= =?UTF-8?q?Executor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🚨 Severity: HIGH 💡 Vulnerability: `AutomationExecutor` was calling `process.waitUntilExit()` before reading the output from the `pipe`. 🎯 Impact: If a child process output exceeds the OS pipe buffer size (~64KB), the child blocks on writing to the pipe. If the parent calls `waitUntilExit()` while the child is blocked on writing, both processes wait on each other indefinitely (a deadlock/DoS condition). 🔧 Fix: Swapped the order to read pipe data (`readDataToEndOfFile()`) *before* invoking `process.waitUntilExit()` in `runProcess` and `childPIDs`. ✅ Verification: Code statically validated and Code Review passed. Co-authored-by: NSEvent <44446865+NSEvent@users.noreply.github.com> --- .Jules/sentinel.md | 4 ++++ .../Sources/TriggerKitRuntime/AutomationExecutor.swift | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.Jules/sentinel.md b/.Jules/sentinel.md index 70f67594..16e9fbbf 100644 --- a/.Jules/sentinel.md +++ b/.Jules/sentinel.md @@ -18,3 +18,7 @@ **Vulnerability:** The application was falling back to storing OBS passwords as plaintext strings inside exported/saved JSON models (`Macro.swift` and `SystemCommand.swift`) if saving to the macOS Keychain failed. **Learning:** Saving secrets to unencrypted formats simply because secure storage fails is a critical anti-pattern known as "failing open" that results in data exposure. **Prevention:** Always fail securely. If secure storage operations fail, discard the sensitive credential in memory rather than writing it insecurely to disk, even if it requires the user to re-authenticate later. +## 2026-06-28 - [Deadlock / DoS via Process() output buffer] +**Vulnerability:** Execution frameworks utilizing `Foundation.Process` combined with `Pipe()` were calling `process.waitUntilExit()` before reading the output `pipe.fileHandleForReading.readDataToEndOfFile()`. This allows a child process exceeding the OS pipe buffer size (~64KB) to block indefinitely while trying to write, creating a deadlock or DoS condition. +**Learning:** Process deadlocks can occur when output exceeds the pipe's internal buffer, because the operating system pauses the process until the buffer is read. Calling `waitUntilExit()` while the process is blocked on a write causes the parent process to hang forever. +**Prevention:** Always ensure you read pipe data (e.g., via `fileHandleForReading.readDataToEndOfFile()`) *before* invoking `process.waitUntilExit()`, or process reads asynchronously, to avoid deadlocking when capturing output. diff --git a/TriggerKit/Sources/TriggerKitRuntime/AutomationExecutor.swift b/TriggerKit/Sources/TriggerKitRuntime/AutomationExecutor.swift index 59854205..dee05f18 100644 --- a/TriggerKit/Sources/TriggerKitRuntime/AutomationExecutor.swift +++ b/TriggerKit/Sources/TriggerKitRuntime/AutomationExecutor.swift @@ -354,9 +354,11 @@ public final class AutomationExecutor { } catch { return .failure("\(name) launch failed: \(error.localizedDescription)") } - process.waitUntilExit() + // Read pipe data BEFORE waitUntilExit to avoid deadlock when + // output exceeds the pipe buffer size (~64KB). let data = pipe.fileHandleForReading.readDataToEndOfFile() + process.waitUntilExit() let output = String(data: data, encoding: .utf8)? .trimmingCharacters(in: .whitespacesAndNewlines) ?? "" @@ -731,11 +733,13 @@ private final class ShellCommandRunner: @unchecked Sendable { pgrep.standardError = FileHandle.nullDevice do { try pgrep.run() - pgrep.waitUntilExit() } catch { return [] } + + // Read pipe data BEFORE waitUntilExit to avoid deadlock let data = pipe.fileHandleForReading.readDataToEndOfFile() + pgrep.waitUntilExit() let output = String(data: data, encoding: .utf8) ?? "" return output .split(whereSeparator: \.isNewline) From 6904bd94846b7b95e0654f89562afe38ba60f3fa Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 10 Jul 2026 09:19:40 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH]?= =?UTF-8?q?=20Fix=20Process()=20pipe=20buffer=20deadlock=20in=20Automation?= =?UTF-8?q?Executor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🚨 Severity: HIGH 💡 Vulnerability: `AutomationExecutor` was calling `process.waitUntilExit()` before reading the output from the `pipe`. 🎯 Impact: If a child process output exceeds the OS pipe buffer size (~64KB), the child blocks on writing to the pipe. If the parent calls `waitUntilExit()` while the child is blocked on writing, both processes wait on each other indefinitely (a deadlock/DoS condition). 🔧 Fix: Swapped the order to read pipe data (`readDataToEndOfFile()`) *before* invoking `process.waitUntilExit()` in `runProcess` and `childPIDs`. Also fixed a flaky test for `testShellCommandCancellationStopsRunningProcess`. ✅ Verification: Code statically validated and Code Review passed. Co-authored-by: NSEvent <44446865+NSEvent@users.noreply.github.com> --- .../Sources/TriggerKitRuntime/AutomationExecutor.swift | 7 ++++++- .../TriggerKitRuntimeTests/AutomationExecutorTests.swift | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/TriggerKit/Sources/TriggerKitRuntime/AutomationExecutor.swift b/TriggerKit/Sources/TriggerKitRuntime/AutomationExecutor.swift index dee05f18..a44260bf 100644 --- a/TriggerKit/Sources/TriggerKitRuntime/AutomationExecutor.swift +++ b/TriggerKit/Sources/TriggerKitRuntime/AutomationExecutor.swift @@ -643,7 +643,12 @@ private final class ShellCommandRunner: @unchecked Sendable { } let timeout = DispatchTime.now() + step.timeoutSeconds - if semaphore.wait(timeout: timeout) == .timedOut { + let waitResult = semaphore.wait(timeout: timeout) + + // If the process hasn't exited but pipe buffer is filled, it could be deadlocked + // but since we read asynchronously via readabilityHandler, this shouldn't happen for the pipe. + + if waitResult == .timedOut { terminateProcessTree(process, signal: SIGTERM) _ = semaphore.wait(timeout: .now() + 1) if process.isRunning { diff --git a/TriggerKit/Tests/TriggerKitRuntimeTests/AutomationExecutorTests.swift b/TriggerKit/Tests/TriggerKitRuntimeTests/AutomationExecutorTests.swift index 4894b5b1..19bb79d3 100644 --- a/TriggerKit/Tests/TriggerKitRuntimeTests/AutomationExecutorTests.swift +++ b/TriggerKit/Tests/TriggerKitRuntimeTests/AutomationExecutorTests.swift @@ -464,7 +464,7 @@ final class AutomationExecutorTests: XCTestCase { ])) } - try? await Task.sleep(nanoseconds: 20_000_000) + try? await Task.sleep(nanoseconds: 250_000_000) // increase delay so process has time to start task.cancel() let result = await task.value