From 2b5bb0cb6caa9bbcc21702d6f65c0f2cd45c8c1b Mon Sep 17 00:00:00 2001
From: Mai Bui <maibui@microsoft.com>
Date: Wed, 25 Jan 2023 16:41:08 -0500
Subject: [PATCH] Revert "[system-health] Remove subprocess with shell=True
 (#12572)" (#13505)

This reverts commit b3a81679684d64ff380ab6f7e761587d9281679b.
Due to issue https://github.com/sonic-net/sonic-buildimage/issues/13432
---
 src/system-health/health_checker/service_checker.py | 11 +++++------
 src/system-health/health_checker/sysmonitor.py      |  2 +-
 src/system-health/health_checker/utils.py           |  2 +-
 src/system-health/tests/test_system_health.py       |  4 ++--
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/system-health/health_checker/service_checker.py b/src/system-health/health_checker/service_checker.py
index ed6c7296fde3..df6005cc0f92 100644
--- a/src/system-health/health_checker/service_checker.py
+++ b/src/system-health/health_checker/service_checker.py
@@ -26,13 +26,13 @@ class ServiceChecker(HealthChecker):
     CRITICAL_PROCESSES_PATH = 'etc/supervisor/critical_processes'
 
     # Command to get merged directory of a container
-    GET_CONTAINER_FOLDER_CMD = ['docker', 'inspect', '', '--format', "{{.GraphDriver.Data.MergedDir}}"]
+    GET_CONTAINER_FOLDER_CMD = 'docker inspect {} --format "{{{{.GraphDriver.Data.MergedDir}}}}"'
 
     # Command to query the status of monit service.
-    CHECK_MONIT_SERVICE_CMD = ['systemctl', 'is-active', 'monit.service']
+    CHECK_MONIT_SERVICE_CMD = 'systemctl is-active monit.service'
 
     # Command to get summary of critical system service.
-    CHECK_CMD = ['monit', 'summary', '-B']
+    CHECK_CMD = 'monit summary -B'
     MIN_CHECK_CMD_LINES = 3
 
     # Expect status for different system service category.
@@ -172,8 +172,7 @@ def _update_container_critical_processes(self, container, critical_process_list)
         self.need_save_cache = True
 
     def _get_container_folder(self, container):
-        ServiceChecker.GET_CONTAINER_FOLDER_CMD[2] = str(container)
-        container_folder = utils.run_command(ServiceChecker.GET_CONTAINER_FOLDER_CMD)
+        container_folder = utils.run_command(ServiceChecker.GET_CONTAINER_FOLDER_CMD.format(container))
         if container_folder is None:
             return container_folder
 
@@ -339,7 +338,7 @@ def check_process_existence(self, container_name, critical_process_list, config,
                 # We are using supervisorctl status to check the critical process status. We cannot leverage psutil here because
                 # it not always possible to get process cmdline in supervisor.conf. E.g, cmdline of orchagent is "/usr/bin/orchagent",
                 # however, in supervisor.conf it is "/usr/bin/orchagent.sh"
-                cmd = ['docker', 'exec', str(container_name), 'bash', '-c', "supervisorctl status"]
+                cmd = 'docker exec {} bash -c "supervisorctl status"'.format(container_name)
                 process_status = utils.run_command(cmd)
                 if process_status is None:
                     for process_name in critical_process_list:
diff --git a/src/system-health/health_checker/sysmonitor.py b/src/system-health/health_checker/sysmonitor.py
index 651776d3e0f3..dde0b73d1bce 100755
--- a/src/system-health/health_checker/sysmonitor.py
+++ b/src/system-health/health_checker/sysmonitor.py
@@ -234,7 +234,7 @@ def get_app_ready_status(self, service):
 
     #Gets the service properties
     def run_systemctl_show(self, service):
-        command = ['systemctl', 'show', str(service), '--property=Id,LoadState,UnitFileState,Type,ActiveState,SubState,Result']
+        command = ('systemctl show {} --property=Id,LoadState,UnitFileState,Type,ActiveState,SubState,Result'.format(service))
         output = utils.run_command(command)
         srv_properties = output.split('\n')
         prop_dict = {}
diff --git a/src/system-health/health_checker/utils.py b/src/system-health/health_checker/utils.py
index 338ef1d3afe5..00e7754e1ec2 100644
--- a/src/system-health/health_checker/utils.py
+++ b/src/system-health/health_checker/utils.py
@@ -8,7 +8,7 @@ def run_command(command):
     :return: Output of the shell command.
     """
     try:
-        process = subprocess.Popen(command, universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        process = subprocess.Popen(command, shell=True, universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         return process.communicate()[0]
     except Exception:
         return None
diff --git a/src/system-health/tests/test_system_health.py b/src/system-health/tests/test_system_health.py
index 73beb99cae83..687781ea2a4f 100644
--- a/src/system-health/tests/test_system_health.py
+++ b/src/system-health/tests/test_system_health.py
@@ -536,10 +536,10 @@ def test_manager(mock_hw_info, mock_service_info, mock_udc_info):
     manager._set_system_led(chassis, manager.config, 'normal')
 
 def test_utils():
-    output = utils.run_command(['some', 'invalid', 'command'])
+    output = utils.run_command('some invalid command')
     assert not output
 
-    output = utils.run_command(['ls'])
+    output = utils.run_command('ls')
     assert output