Skip to content

Commit

Permalink
#139 added possibility to use bash operators in values/default scripts
Browse files Browse the repository at this point in the history
  • Loading branch information
bugy committed Apr 5, 2021
1 parent 827f562 commit e26ecc8
Show file tree
Hide file tree
Showing 10 changed files with 206 additions and 61 deletions.
12 changes: 6 additions & 6 deletions samples/configs/parameterized.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@
{
"name": "Simple List",
"param": "--simple_list=",
"same_arg_param": true,
"type": "list",
"description": "Parameter Five",
"values": [
"val1",
"val3",
"some long value"
],
"same_arg_param": true
]
},
{
"name": "File upload",
Expand All @@ -48,13 +48,13 @@
"param": "--multiselect",
"type": "multiselect",
"description": "Multiselect list",
"multiselect_argument_type": "repeat_param_value",
"values": [
"Black cat",
"Brown dog",
"Green parrot",
"Red fox"
]
],
"multiselect_argument_type": "repeat_param_value"
},
{
"name": "Required Text",
Expand Down Expand Up @@ -111,7 +111,7 @@
"type": "list",
"description": "List parameter 2",
"values": {
"script": "ls /var"
"script": "ls /var | grep -v lock | grep -v backups"
}
},
{
Expand Down Expand Up @@ -238,9 +238,9 @@
},
{
"name": "Editable list",
"required": true,
"param": "--editable_list",
"type": "editable_list",
"required": true,
"values": [
"Value A",
"Value B",
Expand Down
9 changes: 5 additions & 4 deletions src/config/script/list_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ def get_values(self, parameter_values):

class ScriptValuesProvider(ValuesProvider):

def __init__(self, script) -> None:
script_output = process_utils.invoke(script)
def __init__(self, script, shell) -> None:
script_output = process_utils.invoke(script, shell=shell)
script_output = script_output.rstrip('\n')
self._values = [line for line in script_output.split('\n') if not is_empty(line)]

Expand All @@ -56,7 +56,7 @@ def get_values(self, parameter_values):

class DependantScriptValuesProvider(ValuesProvider):

def __init__(self, script, parameters_supplier) -> None:
def __init__(self, script, parameters_supplier, shell) -> None:
pattern = re.compile('\${([^}]+)\}')

search_start = 0
Expand All @@ -80,6 +80,7 @@ def __init__(self, script, parameters_supplier) -> None:
self._required_parameters = tuple(required_parameters)
self._script_template = script
self._parameters_supplier = parameters_supplier
self._shell = shell

def get_required_parameters(self):
return self._required_parameters
Expand All @@ -94,7 +95,7 @@ def get_values(self, parameter_values):
script = fill_parameter_values(parameters, self._script_template, parameter_values)

try:
script_output = process_utils.invoke(script)
script_output = process_utils.invoke(script, shell=self._shell)
except Exception as e:
LOGGER.warning('Failed to execute script. ' + str(e))
return []
Expand Down
20 changes: 13 additions & 7 deletions src/model/parameter_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,16 @@ def _create_values_provider(self, values_config, type, constant):
return ConstValuesProvider(values_config)

elif 'script' in values_config:
script = replace_auth_vars(values_config['script'], self._username, self._audit_name)
original_script = values_config['script']
has_variables = ('${' in original_script)

script = replace_auth_vars(original_script, self._username, self._audit_name)
shell = read_bool_from_config('shell', values_config, default=not has_variables)

if '${' not in script:
return ScriptValuesProvider(script)
return ScriptValuesProvider(script, shell)

return DependantScriptValuesProvider(script, self._parameters_supplier)
return DependantScriptValuesProvider(script, self._parameters_supplier, shell)

else:
message = 'Unsupported "values" format for ' + self.name
Expand Down Expand Up @@ -439,11 +443,13 @@ def _resolve_default(default, username, audit_name, working_dir):
if resolved_string_value == string_value:
resolved_string_value = replace_auth_vars(string_value, username, audit_name)

if not script:
return resolved_string_value
if script:
has_variables = string_value != resolved_string_value
shell = read_bool_from_config('shell', default, default=not has_variables)
output = process_utils.invoke(resolved_string_value, working_dir, shell=shell)
return output.strip()

output = process_utils.invoke(resolved_string_value, working_dir)
return output.strip()
return resolved_string_value


def _resolve_file_dir(config, key):
Expand Down
110 changes: 83 additions & 27 deletions src/tests/list_values_test.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,42 @@
import os
import unittest

from parameterized import parameterized

from config.script.list_values import DependantScriptValuesProvider, FilesProvider, ScriptValuesProvider
from tests import test_utils
from tests.test_utils import create_parameter_model
from utils import file_utils
from utils.process_utils import ExecutionException


class ScriptValuesProviderTest(unittest.TestCase):
def test_ls_3_files(self):
@parameterized.expand([(True,), (False,)])
def test_ls_3_files(self, shell):
test_utils.create_files(['f1', 'f2', 'f3'])
provider = ScriptValuesProvider('ls "' + test_utils.temp_folder + '"')
provider = ScriptValuesProvider('ls "' + test_utils.temp_folder + '"',
shell=shell)
self.assertEqual(['f1', 'f2', 'f3'], provider.get_values({}))

def test_ls_no_files(self):
provider = ScriptValuesProvider('ls "' + test_utils.temp_folder + '"')
@parameterized.expand([(True,), (False,)])
def test_ls_no_files(self, shell):
provider = ScriptValuesProvider('ls "' + test_utils.temp_folder + '"',
shell=shell)
self.assertEqual([], provider.get_values({}))

def test_ls_3_files_when_bash_operator(self):
test_utils.create_files(['f1', 'f2', 'f3'])
self.assertRaises(ExecutionException,
ScriptValuesProvider,
'ls "' + test_utils.temp_folder + '" | grep 2',
shell=False)

def test_ls_3_files_when_bash_operator_and_shell(self):
test_utils.create_files(['f1', 'f2', 'f3'])
provider = ScriptValuesProvider('ls "' + test_utils.temp_folder + '" | grep 2',
shell=True)
self.assertEqual(['f2'], provider.get_values({}))

def setUp(self) -> None:
super().setUp()

Expand All @@ -29,73 +49,109 @@ def tearDown(self) -> None:


class DependantScriptValuesProviderTest(unittest.TestCase):
def test_get_required_parameters_when_single_dependency(self):
values_provider = DependantScriptValuesProvider('ls ${param1}', self.create_parameters_supplier('param1'))

@parameterized.expand([(True,), (False,)])
def test_get_required_parameters_when_single_dependency(self, shell):
values_provider = DependantScriptValuesProvider(
'ls ${param1}',
self.create_parameters_supplier('param1'),
shell=shell)

self.assertCountEqual(['param1'], values_provider.get_required_parameters())

def test_get_required_parameters_when_single_dependency_and_many_params(self):
@parameterized.expand([(True,), (False,)])
def test_get_required_parameters_when_single_dependency_and_many_params(self, shell):
values_provider = DependantScriptValuesProvider(
'ls ${param1}',
self.create_parameters_supplier('param1', 'param2', 'param3'))
self.create_parameters_supplier('param1', 'param2', 'param3'),
shell=shell)
self.assertCountEqual(['param1'], values_provider.get_required_parameters())

def test_get_required_parameters_when_multiple_dependencies(self):
@parameterized.expand([(True,), (False,)])
def test_get_required_parameters_when_multiple_dependencies(self, shell):
values_provider = DependantScriptValuesProvider(
'ls ${param1}/${param2}',
self.create_parameters_supplier('param1', 'param2', 'param3'))
self.create_parameters_supplier('param1', 'param2', 'param3'),
shell=shell)
self.assertCountEqual(['param1', 'param2'], values_provider.get_required_parameters())

def test_get_values_when_no_values(self):
@parameterized.expand([(True,), (False,)])
def test_get_values_when_no_values(self, shell):
values_provider = DependantScriptValuesProvider(
'ls ${param1}',
self.create_parameters_supplier('param1'))
self.create_parameters_supplier('param1'),
shell=shell)
self.assertEqual([], values_provider.get_values({}))

def test_get_values_when_single_parameter(self):
@parameterized.expand([(True,), (False,)])
def test_get_values_when_single_parameter(self, shell):
values_provider = DependantScriptValuesProvider(
"echo '_${param1}_'",
self.create_parameters_supplier('param1'))
self.create_parameters_supplier('param1'),
shell=shell)
self.assertEqual(['_hello world_'], values_provider.get_values({'param1': 'hello world'}))

def test_get_values_when_multiple_parameters(self):
@parameterized.expand([(True,), (False,)])
def test_get_values_when_multiple_parameters(self, shell):
files_path = os.path.join(test_utils.temp_folder, 'path1', 'path2')
for i in range(0, 5):
file_utils.write_file(os.path.join(files_path, 'f' + str(i) + '.txt'), 'test')

values_provider = DependantScriptValuesProvider(
'ls ' + test_utils.temp_folder + '/${param1}/${param2}',
self.create_parameters_supplier('param1', 'param2'))
self.create_parameters_supplier('param1', 'param2'),
shell=shell)
self.assertEqual(['f0.txt', 'f1.txt', 'f2.txt', 'f3.txt', 'f4.txt'],
values_provider.get_values({'param1': 'path1', 'param2': 'path2'}))

def test_get_values_when_parameter_repeats(self):
@parameterized.expand([(True,), (False,)])
def test_get_values_when_parameter_repeats(self, shell):
values_provider = DependantScriptValuesProvider(
"echo '_${param1}_\n' 'test\n' '+${param1}+'",
self.create_parameters_supplier('param1'))
self.create_parameters_supplier('param1'),
shell=shell)
self.assertEqual(['_123_', ' test', ' +123+'], values_provider.get_values({'param1': '123'}))

def test_get_values_when_numeric_parameter(self):
@parameterized.expand([(True,), (False,)])
def test_get_values_when_numeric_parameter(self, shell):
values_provider = DependantScriptValuesProvider(
"echo '_${param1}_'",
self.create_parameters_supplier('param1'))
self.create_parameters_supplier('param1'),
shell=shell)
self.assertEqual(['_123_'], values_provider.get_values({'param1': 123}))

def test_get_values_when_newline_response(self):
@parameterized.expand([(True,), (False,)])
def test_get_values_when_newline_response(self, shell):
values_provider = DependantScriptValuesProvider(
"ls '${param1}'",
self.create_parameters_supplier('param1'))
self.create_parameters_supplier('param1'),
shell=shell)
self.assertEqual([], values_provider.get_values({'param1': test_utils.temp_folder}))

def test_no_code_injection(self):
@parameterized.expand([(True, ['1', '2']), (False, ['1 && echo 2'])])
def test_no_code_injection_for_and_operator(self, shell, expected_values):
values_provider = DependantScriptValuesProvider(
"echo ${param1}",
self.create_parameters_supplier('param1'))
self.assertEqual(['1 && echo 2'], values_provider.get_values({'param1': '1 && echo 2'}))
self.create_parameters_supplier('param1'),
shell=shell)
self.assertEqual(expected_values, values_provider.get_values({'param1': '1 && echo 2'}))

@parameterized.expand([(True, ['y2', 'y3']), (False, [])])
def test_no_code_injection_for_pipe_operator(self, shell, expected_values):
test_utils.create_files(['x1', 'y2', 'y3'])

values_provider = DependantScriptValuesProvider(
"ls ${param1}",
self.create_parameters_supplier('param1'),
shell=shell)
self.assertEqual(expected_values, values_provider.get_values({'param1': test_utils.temp_folder + ' | grep y'}))

def test_script_fails(self):
@parameterized.expand([(True,), (False,)])
def test_script_fails(self, shell):
values_provider = DependantScriptValuesProvider(
"echo2 ${param1}",
self.create_parameters_supplier('param1'))
self.create_parameters_supplier('param1'),
shell=shell)
self.assertEqual([], values_provider.get_values({'param1': 'abc'}))

def setUp(self):
Expand Down
Loading

0 comments on commit e26ecc8

Please sign in to comment.