summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorZhijiang Hu <hu.zhijiang@zte.com.cn>2018-01-05 06:29:13 +0000
committerGerrit Code Review <gerrit@opnfv.org>2018-01-05 06:29:13 +0000
commit431ed2baa96538fd48ebc1e7df8454b60b903de1 (patch)
treeea38d4d130fa8915802e790593a3407dc83ae242
parent49205e7200afd0ce286d2f685093948cb1f7bf3d (diff)
parent2334b3c550c217308efbaf3f4f22718c3b3d0466 (diff)
Merge "Fix security risks about shell=True"
-rw-r--r--deploy/utils.py5
-rw-r--r--tests/unit/test_utils.py19
2 files changed, 10 insertions, 14 deletions
diff --git a/deploy/utils.py b/deploy/utils.py
index 55fbc53a..d0e67359 100644
--- a/deploy/utils.py
+++ b/deploy/utils.py
@@ -124,10 +124,9 @@ def ipmi_reboot_node(host, user, passwd, boot_source=None):
def run_shell(cmd, check=False):
- process = subprocess.Popen(cmd,
+ process = subprocess.Popen(cmd.split(),
stdout=subprocess.PIPE,
- stderr=subprocess.PIPE,
- shell=True)
+ stderr=subprocess.PIPE)
while process.poll() is None:
LD(process.stdout.readline().strip())
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index e3b9dff7..4998a447 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -183,19 +183,16 @@ def test_ipmi_reboot_node(mock_getstatusoutput, mock_err_exit,
@pytest.mark.parametrize('cmd, check, expect', [
- ('cd /home', False, 0),
- ('cd /home', True, 0),
+ ('ls /home', False, 0),
+ ('ls /home', True, 0),
('test_command', False, 127),
('test_command', True, 127)])
-@mock.patch('deploy.utils.err_exit')
-def test_run_shell(mock_err_exit, cmd, check, expect):
- ret = run_shell(cmd, check=check)
- if check:
- if cmd == 'cd /home':
- mock_err_exit.assert_not_called()
- elif cmd == 'test_command':
- mock_err_exit.assert_called_once()
- assert ret == expect
+def test_run_shell(cmd, check, expect):
+ try:
+ ret = run_shell(cmd, check=check)
+ assert ret == expect
+ except OSError:
+ assert cmd == 'test_command'
@pytest.mark.parametrize('scenario', [