Merge pull request 'fix: prevent command injection via shell=True (closes #1, #3)' (#13) from fix/issue-1 into main

Reviewed-on: https://git.home.ms/jarianc/Clover/pulls/13

tools/commandline_tool.py

@@ -2,15 +2,39 @@
 Command line execution tool for Clover - A terminal assistant for AI-powered project management
 """
 
+import shlex
 import subprocess
 import sys
 import os
 from pathlib import Path
 
+# Whitelist of allowed commands for safe execution
+ALLOWED_COMMANDS = {
+    "ls", "cat", "echo", "pwd", "date", "whoami", "id",
+    "git", "python", "python3", "node", "npm", "pip", "pip3",
+    "mkdir", "cp", "mv", "rm", "touch", "chmod", "chown",
+    "grep", "find", "head", "tail", "wc", "sort", "uniq", "diff",
+    "make", "cmake", "cargo", "go", "rustc",
+    "docker", "docker-compose",
+    "curl", "wget",
+    "ps", "top", "df", "free", "uname",
+    "which", "whereis", "type",
+    "test", "stat", "file",
+    "bash", "sh", "zsh",
+    "vim", "nano", "less", "more",
+    "tar", "zip", "unzip", "gzip", "gunzip",
+    "sed", "awk", "tr", "cut", "paste", "join", "comm",
+    "xargs", "tee", "yes", "seq", "bc",
+}
+
+
 def commandline(command, allow_execution=True):
     """
     Execute a system command with user permission.
 
+    Uses shell=False with shlex.split() to prevent command injection.
+    The first word of the command must be in the ALLOWED_COMMANDS whitelist.
+
     Args:
         command (str): The command to execute
         allow_execution (bool): Whether execution is allowed (default: True)
@@ -19,23 +43,40 @@ def commandline(command, allow_execution=True):
         str: Output of the command or permission prompt
 
     Raises:
-        PermissionError: If execution is not permitted
+        PermissionError: If execution is not permitted or command not whitelisted
         subprocess.CalledProcessError: If command execution fails
     """
     if not allow_execution:
         return f"Command execution denied. Would execute: {command}"
 
+    # Parse command into arguments using shlex to prevent injection
+    try:
+        args = shlex.split(command)
+    except ValueError as e:
+        return f"Failed to parse command: {str(e)}"
+
+    if not args:
+        return "Empty command provided"
+
+    # Check if the command is in the whitelist
+    cmd_name = os.path.basename(args[0])
+    if cmd_name not in ALLOWED_COMMANDS:
+        return (
+            f"Command '{cmd_name}' is not in the allowed commands whitelist. "
+            f"Allowed commands: {', '.join(sorted(ALLOWED_COMMANDS))}"
+        )
+
     try:
         print(f"Executing command: {command}")
 
-        # Execute the command
+        # Execute the command with shell=False to prevent injection
         result = subprocess.run(
-            command,
+            args,
-            shell=True,
+            shell=False,
             check=True,
             text=True,
             capture_output=True,
-            timeout=300  # 5 minute timeout
+            timeout=300,  # 5 minute timeout
         )
 
         return result.stdout
@@ -52,6 +93,7 @@ def commandline(command, allow_execution=True):
     except Exception as e:
         return f"Error executing command '{command}': {str(e)}"
 
+
 def safe_execute(command, permission_prompt=True):
     """
     Safely execute a system command with optional permission prompt.
@@ -66,11 +108,12 @@ def safe_execute(command, permission_prompt=True):
     if permission_prompt:
         print(f"Permission needed to run: {command}")
         response = input("Allow execution? (y/N): ")
-        if response.lower() not in ['y', 'yes']:
+        if response.lower() not in ["y", "yes"]:
             return "Execution denied by user"
 
     return commandline(command)
 
+
 # Example usage function
 def example_usage():
     """
@@ -86,5 +129,6 @@ def example_usage():
     result = commandline("ls -la")
     print(f"Directory listing: {result}")
 
+
 if __name__ == "__main__":
     example_usage()