🛡️ Sentinel: [CRITICAL] Fix Process pipe deadlock DoS vulnerabilities

[NSEvent]

🚨 Severity: CRITICAL
💡 Vulnerability: When executing shell commands via Foundation.Process, standard streams were left unread before calling waitUntilExit(). If a child process output exceeded the OS pipe buffer (~64KB), the child blocked writing, and the parent blocked waiting for exit, resulting in a deadlock and application freeze (Denial of Service).
🎯 Impact: Attackers or malformed inputs could intentionally output large data to stderr/stdout during macro or webhook execution to trigger an application deadlock and crash. Unhandled try? process.run() followed by waitUntilExit() also introduced uncatchable Obj-C exception crashes.
🔧 Fix: Moved pipe.fileHandleForReading.readDataToEndOfFile() before waitUntilExit() in TriggerKit and OBS integrations. Replaced unread Pipe() initializations with FileHandle.nullDevice. Added proper do-catch launch validation before waitUntilExit().
✅ Verification: Statically validated all modified files for bracket/parenthesis syntax correctness. Verified that waitUntilExit() is never called with unread pipes or after failed process launches.

---

PR created automatically by Jules for task 7345657549228566205 started by @NSEvent

Summary by CodeRabbit

• Bug Fixes
  • Improved subprocess output collection to ensure stdout/stderr are drained reliably and avoid timing-related stalls.
  • Suppressed diagnostic output during discovery by discarding tailscale status errors.
  • Improved local binary detection by treating missing mediamtx as unavailable and reading command output in a safer order.
  • Reduced noise when checking whether ports are reachable by silencing nc output.

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: deaca318-12b2-41d9-bc0c-af6b60bd44f5

📥 Commits

Reviewing files that changed from the base of the PR and between da2231d and 5f936b5.

📒 Files selected for processing (2)

• TriggerKit/Sources/TriggerKitRuntime/AutomationExecutor.swift
• XboxControllerMapper/XboxControllerMapperTests/OBSWebSocketLiveIntegrationTests.swift

🚧 Files skipped from review as they are similar to previous changes (1)

• XboxControllerMapper/XboxControllerMapperTests/OBSWebSocketLiveIntegrationTests.swift

---

📝 Walkthrough

Walkthrough

Subprocess helpers adjust output-draining order and termination waits. Tailscale and network checks suppress unused output, while binary resolution handles execution failures and reads command output before termination checks.

Changes

Subprocess Output Handling

Layer / File(s)	Summary
Runtime pipe draining TriggerKit/Sources/TriggerKitRuntime/AutomationExecutor.swift	runProcess and childPIDs(of:) drain pipe data before waiting; ShellCommandRunner.run() explicitly waits before reading remaining buffered output.
Command stream handling XboxControllerMapper/XboxControllerMapper/Services/Input/UniversalControlMouseRelay.swift, XboxControllerMapper/XboxControllerMapperTests/OBSWebSocketLiveIntegrationTests.swift	Tailscale stderr and nc output are redirected to the null device; which failures return nil, with stdout read before termination checks.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• NSEvent/xbox-controller-mapper#85: Addresses the same subprocess pipe-draining deadlock pattern.
• NSEvent/xbox-controller-mapper#94: Updates the same runtime and binary-resolution subprocess handling.
• NSEvent/xbox-controller-mapper#97: Applies similar pipe-draining and which output-order changes.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: fixing Process pipe deadlock vulnerabilities causing DoS.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch sentinel/fix-process-pipe-deadlock-dos-7345657549228566205

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@XboxControllerMapper/XboxControllerMapperTests/OBSWebSocketLiveIntegrationTests.swift`:
- Around line 168-179: Make the return contract of resolveBinaryPath consistent
with its declared throws -> String signature: replace the nil return in the
which.run() catch branch with appropriate error propagation, and preserve the
existing successful path returning the resolved binary path for
start(binaryPath:).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 83b157ae-7d6f-4b91-b75a-0e52ed9a1570

📥 Commits

Reviewing files that changed from the base of the PR and between 5cdd395 and da2231d.

📒 Files selected for processing (3)

• TriggerKit/Sources/TriggerKitRuntime/AutomationExecutor.swift
• XboxControllerMapper/XboxControllerMapper/Services/Input/UniversalControlMouseRelay.swift
• XboxControllerMapper/XboxControllerMapperTests/OBSWebSocketLiveIntegrationTests.swift