From 055e7a561f910aa7b9d73fca1a9ba8c1acdd725e Mon Sep 17 00:00:00 2001 From: Caleb Leak Date: Wed, 26 Nov 2025 20:16:42 -0800 Subject: [PATCH] fix(beads-mcp): prevent subprocess stdin inheritance breaking MCP protocol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When running as an MCP server, subprocesses must not inherit stdin because MCP uses stdin for JSON-RPC protocol communication. Inherited stdin causes subprocesses to block indefinitely waiting for input or steal bytes from the MCP protocol stream. Added stdin=subprocess.DEVNULL (or asyncio.subprocess.DEVNULL for async) to all subprocess calls: - server.py: _resolve_workspace_root() git subprocess - tools.py: _resolve_workspace_root() git subprocess - bd_client.py: _run_command(), _check_version(), add_dependency(), quickstart(), and init() async subprocess calls 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../beads-mcp/src/beads_mcp/bd_client.py | 6 + .../beads-mcp/src/beads_mcp/server.py | 1 + integrations/beads-mcp/src/beads_mcp/tools.py | 1 + .../beads-mcp/tests/test_subprocess_stdin.py | 142 ++++++++++++++++++ 4 files changed, 150 insertions(+) create mode 100644 integrations/beads-mcp/tests/test_subprocess_stdin.py diff --git a/integrations/beads-mcp/src/beads_mcp/bd_client.py b/integrations/beads-mcp/src/beads_mcp/bd_client.py index b2324d20..a20759b0 100644 --- a/integrations/beads-mcp/src/beads_mcp/bd_client.py +++ b/integrations/beads-mcp/src/beads_mcp/bd_client.py @@ -4,6 +4,7 @@ import asyncio import json import os import re +import sys from abc import ABC, abstractmethod from typing import Any, List, Optional @@ -290,6 +291,7 @@ class BdCliClient(BdClientBase): try: process = await asyncio.create_subprocess_exec( *cmd, + stdin=asyncio.subprocess.DEVNULL, # Prevent inheriting MCP's stdin stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=working_dir, @@ -333,6 +335,7 @@ class BdCliClient(BdClientBase): process = await asyncio.create_subprocess_exec( self.bd_path, "version", + stdin=asyncio.subprocess.DEVNULL, # Prevent inheriting MCP's stdin stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=self._get_working_dir(), @@ -583,6 +586,7 @@ class BdCliClient(BdClientBase): try: process = await asyncio.create_subprocess_exec( *cmd, + stdin=asyncio.subprocess.DEVNULL, # Prevent inheriting MCP's stdin stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=self._get_working_dir(), @@ -610,6 +614,7 @@ class BdCliClient(BdClientBase): try: process = await asyncio.create_subprocess_exec( *cmd, + stdin=asyncio.subprocess.DEVNULL, # Prevent inheriting MCP's stdin stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=self._get_working_dir(), @@ -754,6 +759,7 @@ class BdCliClient(BdClientBase): try: process = await asyncio.create_subprocess_exec( *cmd, + stdin=asyncio.subprocess.DEVNULL, # Prevent inheriting MCP's stdin stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, cwd=self._get_working_dir(), diff --git a/integrations/beads-mcp/src/beads_mcp/server.py b/integrations/beads-mcp/src/beads_mcp/server.py index 38cda41d..9de8e55c 100644 --- a/integrations/beads-mcp/src/beads_mcp/server.py +++ b/integrations/beads-mcp/src/beads_mcp/server.py @@ -218,6 +218,7 @@ def _resolve_workspace_root(path: str) -> str: text=True, check=False, shell=sys.platform == "win32", + stdin=subprocess.DEVNULL, # Prevent inheriting MCP's stdin ) if result.returncode == 0: return result.stdout.strip() diff --git a/integrations/beads-mcp/src/beads_mcp/tools.py b/integrations/beads-mcp/src/beads_mcp/tools.py index c33b2917..930249e1 100644 --- a/integrations/beads-mcp/src/beads_mcp/tools.py +++ b/integrations/beads-mcp/src/beads_mcp/tools.py @@ -124,6 +124,7 @@ def _resolve_workspace_root(path: str) -> str: text=True, check=False, shell=sys.platform == "win32", + stdin=subprocess.DEVNULL, # Prevent inheriting MCP's stdin ) if result.returncode == 0: return result.stdout.strip() diff --git a/integrations/beads-mcp/tests/test_subprocess_stdin.py b/integrations/beads-mcp/tests/test_subprocess_stdin.py new file mode 100644 index 00000000..833a80b5 --- /dev/null +++ b/integrations/beads-mcp/tests/test_subprocess_stdin.py @@ -0,0 +1,142 @@ +"""Tests for subprocess stdin handling to prevent MCP protocol interference. + +When running as an MCP server, subprocesses must NOT inherit stdin because: +1. MCP uses stdin for JSON-RPC protocol communication +2. Inherited stdin causes subprocesses to block waiting for input +3. Subprocesses may steal bytes from the MCP protocol stream + +These tests verify that all subprocess calls use stdin=DEVNULL. +""" + +import asyncio +import subprocess +import sys +from unittest.mock import AsyncMock, MagicMock, call, patch + +import pytest + + +class TestBdClientSubprocessStdin: + """Test that BdClient subprocess calls don't inherit stdin.""" + + @pytest.fixture + def bd_client(self): + """Create a BdClient instance for testing.""" + from beads_mcp.bd_client import BdClient + return BdClient(bd_path="/usr/bin/bd", beads_db="/tmp/test.db") + + @pytest.fixture + def mock_process(self): + """Create a mock subprocess process.""" + process = MagicMock() + process.returncode = 0 + process.communicate = AsyncMock(return_value=(b'{"id": "test-1"}', b"")) + return process + + @pytest.mark.asyncio + async def test_run_command_uses_devnull_stdin(self, bd_client, mock_process): + """Test that _run_command passes stdin=DEVNULL to prevent MCP stdin inheritance.""" + with patch("asyncio.create_subprocess_exec", return_value=mock_process) as mock_exec: + await bd_client._run_command("show", "test-1") + + # Verify stdin=DEVNULL was passed + mock_exec.assert_called_once() + call_kwargs = mock_exec.call_args.kwargs + assert call_kwargs.get("stdin") == asyncio.subprocess.DEVNULL, ( + "subprocess must use stdin=DEVNULL to prevent inheriting MCP's stdin stream" + ) + + @pytest.mark.asyncio + async def test_check_version_uses_devnull_stdin(self, bd_client, mock_process): + """Test that _check_version passes stdin=DEVNULL.""" + mock_process.communicate = AsyncMock(return_value=(b"bd version 0.9.5", b"")) + + with patch("asyncio.create_subprocess_exec", return_value=mock_process) as mock_exec: + await bd_client._check_version() + + mock_exec.assert_called_once() + call_kwargs = mock_exec.call_args.kwargs + assert call_kwargs.get("stdin") == asyncio.subprocess.DEVNULL, ( + "subprocess must use stdin=DEVNULL to prevent inheriting MCP's stdin stream" + ) + + @pytest.mark.asyncio + async def test_add_dependency_uses_devnull_stdin(self, bd_client, mock_process): + """Test that add_dependency passes stdin=DEVNULL.""" + from beads_mcp.models import AddDependencyParams + + with patch("asyncio.create_subprocess_exec", return_value=mock_process) as mock_exec: + params = AddDependencyParams(issue_id="bd-2", depends_on_id="bd-1", dep_type="blocks") + await bd_client.add_dependency(params) + + mock_exec.assert_called_once() + call_kwargs = mock_exec.call_args.kwargs + assert call_kwargs.get("stdin") == asyncio.subprocess.DEVNULL, ( + "subprocess must use stdin=DEVNULL to prevent inheriting MCP's stdin stream" + ) + + @pytest.mark.asyncio + async def test_quickstart_uses_devnull_stdin(self, bd_client, mock_process): + """Test that quickstart passes stdin=DEVNULL.""" + mock_process.communicate = AsyncMock(return_value=(b"# Quickstart guide", b"")) + + with patch("asyncio.create_subprocess_exec", return_value=mock_process) as mock_exec: + await bd_client.quickstart() + + mock_exec.assert_called_once() + call_kwargs = mock_exec.call_args.kwargs + assert call_kwargs.get("stdin") == asyncio.subprocess.DEVNULL, ( + "subprocess must use stdin=DEVNULL to prevent inheriting MCP's stdin stream" + ) + + @pytest.mark.asyncio + async def test_init_uses_devnull_stdin(self, bd_client, mock_process): + """Test that init passes stdin=DEVNULL.""" + mock_process.communicate = AsyncMock(return_value=(b"Initialized!", b"")) + + with patch("asyncio.create_subprocess_exec", return_value=mock_process) as mock_exec: + await bd_client.init() + + mock_exec.assert_called_once() + call_kwargs = mock_exec.call_args.kwargs + assert call_kwargs.get("stdin") == asyncio.subprocess.DEVNULL, ( + "subprocess must use stdin=DEVNULL to prevent inheriting MCP's stdin stream" + ) + + +class TestServerSubprocessStdin: + """Test that server.py subprocess calls don't inherit stdin.""" + + def test_resolve_workspace_root_uses_devnull_stdin(self): + """Test that _resolve_workspace_root passes stdin=DEVNULL to git subprocess.""" + from beads_mcp.server import _resolve_workspace_root + + with patch("subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0, stdout="/repo/root\n") + + _resolve_workspace_root("/some/path") + + mock_run.assert_called_once() + call_kwargs = mock_run.call_args.kwargs + assert call_kwargs.get("stdin") == subprocess.DEVNULL, ( + "git subprocess must use stdin=DEVNULL to prevent inheriting MCP's stdin stream" + ) + + +class TestToolsSubprocessStdin: + """Test that tools.py subprocess calls don't inherit stdin.""" + + def test_tools_resolve_workspace_root_uses_devnull_stdin(self): + """Test that tools._resolve_workspace_root passes stdin=DEVNULL to git subprocess.""" + from beads_mcp.tools import _resolve_workspace_root + + with patch("beads_mcp.tools.subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0, stdout="/repo/root\n") + + _resolve_workspace_root("/some/path") + + mock_run.assert_called_once() + call_kwargs = mock_run.call_args.kwargs + assert call_kwargs.get("stdin") == subprocess.DEVNULL, ( + "git subprocess must use stdin=DEVNULL to prevent inheriting MCP's stdin stream" + )