From 3f58e47c63912cb14936b65a2d133878e1771758 Mon Sep 17 00:00:00 2001 From: Farukest Date: Sun, 1 Mar 2026 01:54:27 +0300 Subject: [PATCH] fix: guard POSIX-only process functions for Windows compatibility os.setsid, os.killpg, and os.getpgid do not exist on Windows and raise AttributeError on import or first call. This breaks the terminal tool, code execution sandbox, process registry, and WhatsApp bridge on Windows. Added _IS_WINDOWS platform guard in all four affected files, following the pattern documented in CONTRIBUTING.md. On Windows, preexec_fn is set to None and process termination falls back to proc.terminate() / proc.kill() instead of process group signals. Files changed: - tools/environments/local.py (3 call sites) - tools/process_registry.py (2 call sites) - tools/code_execution_tool.py (3 call sites) - gateway/platforms/whatsapp.py (3 call sites) --- gateway/platforms/whatsapp.py | 15 ++++-- tests/tools/test_windows_compat.py | 80 ++++++++++++++++++++++++++++++ tools/code_execution_tool.py | 15 ++++-- tools/environments/local.py | 25 +++++++--- tools/process_registry.py | 10 +++- 5 files changed, 129 insertions(+), 16 deletions(-) create mode 100644 tests/tools/test_windows_compat.py diff --git a/gateway/platforms/whatsapp.py b/gateway/platforms/whatsapp.py index eb0d6f1b5..17bb3ecb4 100644 --- a/gateway/platforms/whatsapp.py +++ b/gateway/platforms/whatsapp.py @@ -19,7 +19,10 @@ import asyncio import json import logging import os +import platform import subprocess + +_IS_WINDOWS = platform.system() == "Windows" from pathlib import Path from typing import Dict, List, Optional, Any @@ -166,7 +169,7 @@ class WhatsAppAdapter(BasePlatformAdapter): ], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, - preexec_fn=os.setsid, + preexec_fn=None if _IS_WINDOWS else os.setsid, ) # Wait for bridge to be ready via HTTP health check @@ -211,13 +214,19 @@ class WhatsAppAdapter(BasePlatformAdapter): # Kill the entire process group so child node processes die too import signal try: - os.killpg(os.getpgid(self._bridge_process.pid), signal.SIGTERM) + if _IS_WINDOWS: + self._bridge_process.terminate() + else: + os.killpg(os.getpgid(self._bridge_process.pid), signal.SIGTERM) except (ProcessLookupError, PermissionError): self._bridge_process.terminate() await asyncio.sleep(1) if self._bridge_process.poll() is None: try: - os.killpg(os.getpgid(self._bridge_process.pid), signal.SIGKILL) + if _IS_WINDOWS: + self._bridge_process.kill() + else: + os.killpg(os.getpgid(self._bridge_process.pid), signal.SIGKILL) except (ProcessLookupError, PermissionError): self._bridge_process.kill() except Exception as e: diff --git a/tests/tools/test_windows_compat.py b/tests/tools/test_windows_compat.py new file mode 100644 index 000000000..ec04d2095 --- /dev/null +++ b/tests/tools/test_windows_compat.py @@ -0,0 +1,80 @@ +"""Tests for Windows compatibility of process management code. + +Verifies that os.setsid and os.killpg are never called unconditionally, +and that each module uses a platform guard before invoking POSIX-only functions. +""" + +import ast +import pytest +from pathlib import Path + +# Files that must have Windows-safe process management +GUARDED_FILES = [ + "tools/environments/local.py", + "tools/process_registry.py", + "tools/code_execution_tool.py", + "gateway/platforms/whatsapp.py", +] + +PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent + + +def _get_preexec_fn_values(filepath: Path) -> list: + """Find all preexec_fn= keyword arguments in Popen calls.""" + source = filepath.read_text(encoding="utf-8") + tree = ast.parse(source, filename=str(filepath)) + values = [] + for node in ast.walk(tree): + if isinstance(node, ast.keyword) and node.arg == "preexec_fn": + values.append(ast.dump(node.value)) + return values + + +class TestNoUnconditionalSetsid: + """preexec_fn must never be a bare os.setsid reference.""" + + @pytest.mark.parametrize("relpath", GUARDED_FILES) + def test_preexec_fn_is_guarded(self, relpath): + filepath = PROJECT_ROOT / relpath + if not filepath.exists(): + pytest.skip(f"{relpath} not found") + values = _get_preexec_fn_values(filepath) + for val in values: + # A bare os.setsid would be: Attribute(value=Name(id='os'), attr='setsid') + assert "attr='setsid'" not in val or "IfExp" in val or "None" in val, ( + f"{relpath} has unconditional preexec_fn=os.setsid" + ) + + +class TestIsWindowsConstant: + """Each guarded file must define _IS_WINDOWS.""" + + @pytest.mark.parametrize("relpath", GUARDED_FILES) + def test_has_is_windows(self, relpath): + filepath = PROJECT_ROOT / relpath + if not filepath.exists(): + pytest.skip(f"{relpath} not found") + source = filepath.read_text(encoding="utf-8") + assert "_IS_WINDOWS" in source, ( + f"{relpath} missing _IS_WINDOWS platform guard" + ) + + +class TestKillpgGuarded: + """os.killpg must always be behind a platform check.""" + + @pytest.mark.parametrize("relpath", GUARDED_FILES) + def test_no_unguarded_killpg(self, relpath): + filepath = PROJECT_ROOT / relpath + if not filepath.exists(): + pytest.skip(f"{relpath} not found") + source = filepath.read_text(encoding="utf-8") + lines = source.splitlines() + for i, line in enumerate(lines): + stripped = line.strip() + if "os.killpg" in stripped or "os.getpgid" in stripped: + # Check that there's an _IS_WINDOWS guard in the surrounding context + context = "\n".join(lines[max(0, i - 15):i + 1]) + assert "_IS_WINDOWS" in context or "else:" in context, ( + f"{relpath}:{i + 1} has unguarded os.killpg/os.getpgid call" + ) diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py index aa64c802f..8fb4b4431 100644 --- a/tools/code_execution_tool.py +++ b/tools/code_execution_tool.py @@ -20,6 +20,7 @@ Platform: Linux / macOS only (Unix domain sockets). Disabled on Windows. import json import logging import os +import platform import signal import socket import subprocess @@ -28,6 +29,8 @@ import tempfile import threading import time import uuid + +_IS_WINDOWS = platform.system() == "Windows" from typing import Any, Dict, List, Optional # Availability gate: UDS requires a POSIX OS @@ -405,7 +408,7 @@ def execute_code( stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.DEVNULL, - preexec_fn=os.setsid, + preexec_fn=None if _IS_WINDOWS else os.setsid, ) # --- Poll loop: watch for exit, timeout, and interrupt --- @@ -514,7 +517,10 @@ def execute_code( def _kill_process_group(proc, escalate: bool = False): """Kill the child and its entire process group.""" try: - os.killpg(os.getpgid(proc.pid), signal.SIGTERM) + if _IS_WINDOWS: + proc.terminate() + else: + os.killpg(os.getpgid(proc.pid), signal.SIGTERM) except (ProcessLookupError, PermissionError): try: proc.kill() @@ -527,7 +533,10 @@ def _kill_process_group(proc, escalate: bool = False): proc.wait(timeout=5) except subprocess.TimeoutExpired: try: - os.killpg(os.getpgid(proc.pid), signal.SIGKILL) + if _IS_WINDOWS: + proc.kill() + else: + os.killpg(os.getpgid(proc.pid), signal.SIGKILL) except (ProcessLookupError, PermissionError): try: proc.kill() diff --git a/tools/environments/local.py b/tools/environments/local.py index 6d7e8da3c..39586917a 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -1,12 +1,15 @@ """Local execution environment with interrupt support and non-blocking I/O.""" import os +import platform import shutil import signal import subprocess import threading import time +_IS_WINDOWS = platform.system() == "Windows" + from tools.environments.base import BaseEnvironment # Noise lines emitted by interactive shells when stdin is not a terminal. @@ -68,7 +71,7 @@ class LocalEnvironment(BaseEnvironment): stdout=subprocess.PIPE, stderr=subprocess.STDOUT, stdin=subprocess.PIPE if stdin_data is not None else subprocess.DEVNULL, - preexec_fn=os.setsid, + preexec_fn=None if _IS_WINDOWS else os.setsid, ) if stdin_data is not None: @@ -101,12 +104,15 @@ class LocalEnvironment(BaseEnvironment): while proc.poll() is None: if _interrupt_event.is_set(): try: - pgid = os.getpgid(proc.pid) - os.killpg(pgid, signal.SIGTERM) - try: - proc.wait(timeout=1.0) - except subprocess.TimeoutExpired: - os.killpg(pgid, signal.SIGKILL) + if _IS_WINDOWS: + proc.terminate() + else: + pgid = os.getpgid(proc.pid) + os.killpg(pgid, signal.SIGTERM) + try: + proc.wait(timeout=1.0) + except subprocess.TimeoutExpired: + os.killpg(pgid, signal.SIGKILL) except (ProcessLookupError, PermissionError): proc.kill() reader.join(timeout=2) @@ -116,7 +122,10 @@ class LocalEnvironment(BaseEnvironment): } if time.monotonic() > deadline: try: - os.killpg(os.getpgid(proc.pid), signal.SIGTERM) + if _IS_WINDOWS: + proc.terminate() + else: + os.killpg(os.getpgid(proc.pid), signal.SIGTERM) except (ProcessLookupError, PermissionError): proc.kill() reader.join(timeout=2) diff --git a/tools/process_registry.py b/tools/process_registry.py index bfdb8cd1d..4f50fe116 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -32,6 +32,7 @@ Usage: import json import logging import os +import platform import shlex import shutil import signal @@ -39,6 +40,8 @@ import subprocess import threading import time import uuid + +_IS_WINDOWS = platform.system() == "Windows" from dataclasses import dataclass, field from pathlib import Path from typing import Any, Dict, List, Optional @@ -192,7 +195,7 @@ class ProcessRegistry: stdout=subprocess.PIPE, stderr=subprocess.STDOUT, stdin=subprocess.PIPE, - preexec_fn=os.setsid, + preexec_fn=None if _IS_WINDOWS else os.setsid, ) session.process = proc @@ -544,7 +547,10 @@ class ProcessRegistry: elif session.process: # Local process -- kill the process group try: - os.killpg(os.getpgid(session.process.pid), signal.SIGTERM) + if _IS_WINDOWS: + session.process.terminate() + else: + os.killpg(os.getpgid(session.process.pid), signal.SIGTERM) except (ProcessLookupError, PermissionError): session.process.kill() elif session.env_ref and session.pid: