T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/with_server.py:69
- Finding
- Server Child Processes May Survive Lifecycle Cleanup<![CDATA[ ## Vulnerability Details **File Location**: `scripts/with_server.py`, lines 69–100 **Vulnerability Type**: Incomplete process-tree termination **Risk Level**: Medium ### Vulnerable Code ```python process = subprocess.Popen( server['cmd'], shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE ) server_processes.append(process) # Wait for this server to be ready print(f"Waiting for server on port {server['port']}...") if not is_server_ready(server['port'], timeout=args.timeout): raise RuntimeError(f"Server failed to start on port {server['port']} within {args.timeout}s") print(f"Server ready on port {server['port']}") print(f"\nAll {len(servers)} server(s) ready") # Run the command print(f"Running: {' '.join(args.command)}\n") result = subprocess.run(args.command) sys.exit(result.returncode) finally: # Clean up all servers print(f"\nStopping {len(server_processes)} server(s)...") for i, process in enumerate(server_processes): try: process.terminate() process.wait(timeout=5) except subprocess.TimeoutExpired: process.kill() process.wait() print(f"Server {i+1} stopped") ``` ### Technical Analysis Each server command is launched with `shell=True`. Consequently, the `Popen` object can represent an intermediary shell rather than the actual development server or all processes created by the command. The cleanup routine sends termination or kill signals only to the process represented by that object. No separate process session or process group is created, and cleanup does not enumerate or terminate descendants. Commands involving package managers, shell pipelines, background jobs, or scripts that spawn child processes can therefore leave descendants running after the intermediary shell exits. Killing the shell does not reliably propagate termination to its children. The use of `stdout=subprocess.PIPE` and `stderr=subprocess.PIPE` without con ...[truncated 1979 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Create an isolated process group or session for every server and terminate the entire group during cleanup. On POSIX systems: 1. Pass `start_new_session=True` to `subprocess.Popen`. 2. On cleanup, send `SIGTERM` to the process group with `os.killpg(process.pid, signal.SIGTERM)`. 3. Wait for graceful shutdown. 4. If the timeout expires, send `SIGKILL` to the same process group. 5. Handle `ProcessLookupError` to account for processes that have already exited. Example: ```python import os import signal import subprocess process = subprocess.Popen( server['cmd'], shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, start_new_session=True, ) # During cleanup: try: os.killpg(process.pid, signal.SIGTERM) process.wait(timeout=5) except subprocess.TimeoutExpired: os.killpg(process.pid, signal.SIGKILL) process.wait() except ProcessLookupError: pass ``` Use an equivalent process-tree termination mechanism on Windows, such as a new process group combined with an appropriate job object or `taskkill /T`. Where possible, avoid `shell=True` and accept server commands as argument arrays. If shell syntax is required, document that the supplied command is trusted and still isolate its complete process group. Finally, inherit output streams, redirect them to files, or drain both pipes continuously so verbose servers cannot block when pipe buffers fill. ]]>