[sonic_package_manager] replace shell=True

sonic_package_manager/service_creator/creator.py

@@ -2,10 +2,11 @@
 
 import contextlib
 import os
+import sys
 import stat
 import subprocess
 from collections import defaultdict
-from typing import Dict, Type
+from typing import Dict, Type, List
 
 import jinja2 as jinja2
 from config.config_mgmt import ConfigMgmt
@@ -96,22 +97,22 @@ def remove_if_exists(path):
     os.remove(path)
     log.info(f'removed {path}')
 
+def is_list_of_strings(command):
+    return isinstance(command, list) and all(isinstance(item, str) for item in command)
 
-def run_command(command: str):
+def run_command(command: List[str]):
     """ Run arbitrary bash command.
     Args:
-        command: String command to execute as bash script
+        command: List of Strings command to execute as bash script
     Raises:
         ServiceCreatorError: Raised when the command return code
                              is not 0.
     """
-
+    if not is_list_of_strings(command):
+        sys.exit("Input command should be a list of strings")
     log.debug(f'running command: {command}')
 
-    proc = subprocess.Popen(command,
-                            shell=True,
-                            executable='/bin/bash',
-                            stdout=subprocess.PIPE)
+    proc = subprocess.Popen(command, stdout=subprocess.PIPE)
     (_, _) = proc.communicate()
     if proc.returncode != 0:
         raise ServiceCreatorError(f'Failed to execute "{command}"')
@@ -647,4 +648,4 @@ def _post_operation_hook(self):
         """ Common operations executed after service is created/removed. """
 
         if not in_chroot():
-            run_command('systemctl daemon-reload')
+            run_command(['systemctl', 'daemon-reload'])

tests/sonic_package_manager/test_service_creator.py

@@ -1,8 +1,9 @@
 #!/usr/bin/env python
 
 import os
+import sys
 import copy
-from unittest.mock import Mock, MagicMock, call
+from unittest.mock import Mock, MagicMock, call, patch
 
 import pytest
 
@@ -62,6 +63,22 @@ def manifest():
         ]
     })
 
+def test_is_list_of_strings():
+    output = is_list_of_strings(['a', 'b', 'c'])
+    assert output is True
+
+    output = is_list_of_strings('abc')
+    assert output is False
+
+    output = is_list_of_strings(['a', 'b', 1])
+    assert output is False
+
+def test_run_command():
+    with pytest.raises(SystemExit) as e:
+        run_command('echo 1')
+
+    with pytest.raises(ServiceCreatorError) as e:
+        run_command([sys.executable, "-c", "import sys; sys.exit(6)"])
 
 @pytest.fixture()
 def service_creator(mock_feature_registry,
@@ -203,6 +220,11 @@ def test_service_creator_autocli(sonic_fs, manifest, mock_cli_gen,
         any_order=True
     )
 
+def test_service_creator_post_operation_hook(sonic_fs, manifest, mock_sonic_db, mock_config_mgmt, service_creator):
+    with patch('sonic_package_manager.service_creator.creator.run_command') as run_command:
+        with patch('sonic_package_manager.service_creator.creator.in_chroot', MagicMock(return_value=False)):
+            service_creator._post_operation_hook()
+            run_command.assert_called_with(['systemctl', 'daemon-reload'])
 
 def test_feature_registration(mock_sonic_db, manifest):
     mock_connector = Mock()