harden: sanitize subprocess call in git.py (gitlab.bandit.B602)

[anupamme]

Summary

Harden input handling in lib/git.py (flagged by semgrep).

Vulnerability

Field	Value
ID	gitlab.bandit.B602
Severity	HIGH
Scanner	semgrep
Rule	gitlab.bandit.B602
File	lib/git.py:39
Assessment	Defensive hardening

Description: Found subprocess function Popen with shell=True. This is dangerous because this call will
spawn the command using a shell process. Doing so propagates current shell settings and
variables,
which makes it much easier for a malicious actor to execute commands. Use shell=False
instead.

Threat Model Context

This is a Python library - vulnerabilities affect applications that import this code.

Changes

• lib/git.py

Behavior Preservation

The change is scoped to 1 file on the vulnerable path; it only tightens handling of untrusted input and leaves valid inputs unaffected.

Security Invariant

Property: The security boundary is maintained under adversarial input

Regression test

import pytest
import subprocess
from unittest.mock import patch, MagicMock
import sys
import os

# Add lib to path to import git module
sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..'))

from lib.git import run_git_command

@pytest.mark.parametrize("payload", [
    # Exact exploit case - command injection
    "status; rm -rf /tmp/test",
    # Boundary case - multiple commands with shell metacharacters
    "log && cat /etc/passwd",
    # Valid input - should still work
    "status",
    # Additional adversarial case - environment variable injection
    "status; export MALICIOUS=1; echo $MALICIOUS",
])
def test_git_command_shell_injection_protection(payload):
    """Invariant: Git commands must execute without shell injection regardless of input"""
    
    # Mock subprocess.Popen to capture how it's called
    with patch('subprocess.Popen') as mock_popen:
        # Configure mock to return a successful process
        mock_process = MagicMock()
        mock_process.communicate.return_value = (b"output", b"")
        mock_process.returncode = 0
        mock_popen.return_value = mock_process
        
        try:
            # Call the actual production function
            run_git_command(payload)
            
            # Verify Popen was called with shell=False
            mock_popen.assert_called_once()
            call_args, call_kwargs = mock_popen.call_args
            
            # Security property: shell must be False
            assert call_kwargs.get('shell') == False, \
                f"shell=True detected with payload: {payload}"
                
            # Additional safety check: first argument should not contain shell metacharacters
            # when passed as a list (which it should be with shell=False)
            args = call_args[0]
            if isinstance(args, list):
                # With shell=False, command should be split
                assert len(args) > 0, "Command arguments list should not be empty"
                
        except Exception as e:
            # The function should handle adversarial input gracefully
            # without raising unexpected exceptions
            if "Invalid git command" not in str(e):
                pytest.fail(f"Unexpected exception with payload '{payload}': {e}")

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

---

This patch removes an exploit primitive — a code pattern that, while not independently exploitable today, could be chained with other weaknesses by automated exploit-development tooling. Proactive removal of such primitives raises the bar against increasingly capable automated attack tools.

---

Automated security fix by OrbisAI Security