diff --git a/core/helpers/cli_parse_args.sh b/core/helpers/cli_parse_args.sh index 76b0aa1..6295c4e 100644 --- a/core/helpers/cli_parse_args.sh +++ b/core/helpers/cli_parse_args.sh @@ -35,11 +35,11 @@ _is_str_to_eval() { # _is_str_to_eval # # Examples: - # _is_str_to_eval 'export eval_this="foo"' - # _is_str_to_eval '# Foo\nexport eval_this="bar"' - # _is_str_to_eval '# Foo\n# export do_not_eval_this="bar"' + # _is_str_to_eval $'export eval_this="foo"' + # _is_str_to_eval $'# Foo\nexport eval_this="bar"' + # _is_str_to_eval $'# Foo\n# export do_not_eval_this="bar"' local -r text=$1 - [[ $(echo -e "$text" | grep -v '^ *#' | cut -f1 -d' ' | sort -u) == "export" ]] + [[ $(grep -vE '^[[:space:]]*($|#)' <<< "$text" | awk '{print $1}' | sort -u || true) == "export" ]] } eval_args() { diff --git a/scripts/doc_parser/docopt_ng/__init__.py b/scripts/doc_parser/docopt_ng/__init__.py index 247cd7f..96209ef 100644 --- a/scripts/doc_parser/docopt_ng/__init__.py +++ b/scripts/doc_parser/docopt_ng/__init__.py @@ -950,6 +950,18 @@ def show_log(message: str) -> None: print(f"{filename}:{line_number} - {message}", file=sys.stderr) +# cli customization: +def bash_quote(value: str) -> str: + """Return a single-quoted bash literal that is safe to `eval`. + + Single quotes disable all bash expansion (command substitution, `$` + expansion, backticks, escapes), so user input cannot inject code. The only + character with special meaning inside single quotes is the single quote + itself, which is escaped with the standard `'\\''` idiom. + """ + return "'" + value.replace("'", "'\\''") + "'" + + # cli customization: def convert_to_bash(parsed_options: ParsedOptions) -> str: import os @@ -967,27 +979,27 @@ def convert_to_bash(parsed_options: ParsedOptions) -> str: var_names_list.append(var_name_bash) if value is None: - value_bash: str = '""' + value_bash: str = "''" elif value is True: - value_bash: str = '"true"' + value_bash: str = "'true'" elif value is False: - value_bash: str = '"false"' + value_bash: str = "'false'" elif isinstance(value, list): - value_bash: str = "(" + " ".join(f'"{item}"' for item in value) + ")" + value_bash: str = "(" + " ".join(bash_quote(str(item)) for item in value) + ")" else: - value_bash: str = f'"{value}"' - - if value_bash.startswith('"='): - if len(var_name_bash) == 1: - # For example: `hello world -f="value"`, instead of `hello world -f "value"` - value_bash: str = f'"{value_bash[2::]}' - else: - # `-f` could be an alias for something longer (e.g., `--foo`; in this case, `var_name_bash` would be "foo") - # Since we don't know the original option name, we can't remove the equal sign. - show_log( - f'WARNING: The value for {key} starts with an equal sign: "{value}"\n' - "You may need to remove the equal sign when calling the command." - ) + str_value: str = str(value) + if str_value.startswith("="): + if len(var_name_bash) == 1: + # For example: `hello world -f="value"`, instead of `hello world -f "value"` + str_value = str_value[1:] + else: + # `-f` could be an alias for something longer (e.g., `--foo`; in this case, `var_name_bash` would be "foo") + # Since we don't know the original option name, we can't remove the equal sign. + show_log( + f'WARNING: The value for {key} starts with an equal sign: "{value}"\n' + "You may need to remove the equal sign when calling the command." + ) + value_bash: str = bash_quote(str_value) bash_vars_definition.append(f"export {var_name_bash:s}={value_bash:s}") diff --git a/scripts/tests/test_helper_files.sh b/scripts/tests/test_helper_files.sh index 690f4ef..dfc1526 100755 --- a/scripts/tests/test_helper_files.sh +++ b/scripts/tests/test_helper_files.sh @@ -15,6 +15,14 @@ TESTS_HELPERS_DIR="${TESTS_DIR}/core/helpers" source "${TESTS_DIR}/unit_test_helpers.sh" +# GNU `comm` validates that its inputs are sorted by whole line and aborts otherwise; BSD `comm` +# (macOS default) does not. The lists below are sorted by the file field only (to preserve each +# file's definition order), so on GNU we must disable that check. The flag does not exist on BSD. +comm_nocheck=() +if comm --nocheck-order /dev/null /dev/null &>/dev/null; then + comm_nocheck=(--nocheck-order) +fi + # Run tests # -------------------------------------------------------------------------------------------------- @@ -36,7 +44,7 @@ new_section_level_2 "Every function in a helper file should have a corresponding helper_functions=$(grep -rE '^[^ #]+() {' "$HELPERS_DIR") test_helper_functions=$(grep -r '^test_[a-zA-Z0-9_]*' "$TESTS_HELPERS_DIR" | sed 's:tests/::g ; s:test_::g') functions_without_test=$( - comm -23 \ + comm "${comm_nocheck[@]}" -23 \ <(echo "$helper_functions" | sort -t: -k1,1 --stable) \ <(echo "$test_helper_functions" | sort -t: -k1,1 --stable) ) diff --git a/tests/core/helpers/test_cli_parse_args.sh b/tests/core/helpers/test_cli_parse_args.sh index 91fa35e..15a0214 100755 --- a/tests/core/helpers/test_cli_parse_args.sh +++ b/tests/core/helpers/test_cli_parse_args.sh @@ -30,99 +30,136 @@ test_parse_args() { assertEquals "With the --help parameter, it returns the help text" "$expected" "$result" result=$(parse_args "$help_text" 'hello-world') - expected='# <<-- docopt parsed arguments -->> -export cmd1="false" -export cmd2="false" -export hello_world="true" -export many="false" -export my_cmd="false" -export my_param="123" + expected="# <<-- docopt parsed arguments -->> +export cmd1='false' +export cmd2='false' +export hello_world='true' +export many='false' +export my_cmd='false' +export my_param='123' export names=() -export pos1="" -export pos2="" -export positional_param="" -export some_flag="false" -# <<----------------------------->>' +export pos1='' +export pos2='' +export positional_param='' +export some_flag='false' +# <<----------------------------->>" assertEquals "$expected" "$result" result=$(parse_args "$help_text" 'hello-world' 'John Doe' '--some-flag') - expected='# <<-- docopt parsed arguments -->> -export cmd1="false" -export cmd2="false" -export hello_world="true" -export many="false" -export my_cmd="false" -export my_param="123" + expected="# <<-- docopt parsed arguments -->> +export cmd1='false' +export cmd2='false' +export hello_world='true' +export many='false' +export my_cmd='false' +export my_param='123' export names=() -export pos1="" -export pos2="" -export positional_param="John Doe" -export some_flag="true" -# <<----------------------------->>' +export pos1='' +export pos2='' +export positional_param='John Doe' +export some_flag='true' +# <<----------------------------->>" assertEquals "$expected" "$result" result=$(parse_args "$help_text" 'hello-world' 'many' 'Mr. Smith' 'Mrs. Smith') - expected='# <<-- docopt parsed arguments -->> -export cmd1="false" -export cmd2="false" -export hello_world="true" -export many="true" -export my_cmd="false" -export my_param="123" -export names=("Mr. Smith" "Mrs. Smith") -export pos1="" -export pos2="" -export positional_param="" -export some_flag="false" -# <<----------------------------->>' + expected="# <<-- docopt parsed arguments -->> +export cmd1='false' +export cmd2='false' +export hello_world='true' +export many='true' +export my_cmd='false' +export my_param='123' +export names=('Mr. Smith' 'Mrs. Smith') +export pos1='' +export pos2='' +export positional_param='' +export some_flag='false' +# <<----------------------------->>" assertEquals "$expected" "$result" result=$(parse_args "$help_text" 'hello-world' 'my-cmd' 12 34) - expected='# <<-- docopt parsed arguments -->> -export cmd1="false" -export cmd2="false" -export hello_world="true" -export many="false" -export my_cmd="true" -export my_param="123" + expected="# <<-- docopt parsed arguments -->> +export cmd1='false' +export cmd2='false' +export hello_world='true' +export many='false' +export my_cmd='true' +export my_param='123' export names=() -export pos1="12" -export pos2="34" -export positional_param="" -export some_flag="false" -# <<----------------------------->>' +export pos1='12' +export pos2='34' +export positional_param='' +export some_flag='false' +# <<----------------------------->>" assertEquals "$expected" "$result" result=$(parse_args "$help_text" 'hello-world' 'cmd2' 45 567) - expected='# <<-- docopt parsed arguments -->> -export cmd1="false" -export cmd2="true" -export hello_world="true" -export many="false" -export my_cmd="false" -export my_param="123" + expected="# <<-- docopt parsed arguments -->> +export cmd1='false' +export cmd2='true' +export hello_world='true' +export many='false' +export my_cmd='false' +export my_param='123' export names=() -export pos1="45" -export pos2="567" -export positional_param="" -export some_flag="false" -# <<----------------------------->>' +export pos1='45' +export pos2='567' +export positional_param='' +export some_flag='false' +# <<----------------------------->>" assertEquals "$expected" "$result" } test__is_str_to_eval() { + # shellcheck disable=SC2034 + local -r multi_line_exports="# foo +export x=123 +# bar +export abc=qwerty" + # shellcheck disable=SC2034 + local -r multi_line_exports_blank_lines="# foo + +export x=123 + +export abc=qwerty" + # shellcheck disable=SC2034 + local -r multi_line_mixed="export x=123 +abc=qwerty" + assertTrue '_is_str_to_eval "export xyz=1234"' assertFalse '_is_str_to_eval "# export xyz=1234"' - assertTrue '_is_str_to_eval "# foo\nexport x=123\n# bar\nexport abc=qwerty"' - assertFalse '_is_str_to_eval "export x=123\nabc=qwerty"' + + # shellcheck disable=SC2016 + assertTrue "multi-line all-export block is recognized" \ + '_is_str_to_eval "$multi_line_exports"' + + # shellcheck disable=SC2016 + assertFalse "block with non-export line is rejected" \ + '_is_str_to_eval "$multi_line_mixed"' + + # shellcheck disable=SC2016 + assertTrue "export block with blank lines is recognized" \ + '_is_str_to_eval "$multi_line_exports_blank_lines"' + + # all lines are comments/blank — grep -vE exits 1 (no matches); must return false without aborting + assertFalse "comments-only input returns false without aborting" \ + '_is_str_to_eval "# only a comment"' + + # value contains a literal backslash-n (from safe quoting) — must not split the line + assertTrue '_is_str_to_eval "export name=\"\\n\""' + assertTrue "_is_str_to_eval \"export name='\\\\n'\"" + assertTrue "_is_str_to_eval \"export name='\\n'\"" } test_eval_args() { local result expected assertTrue "[ -z ${xyz:-} ] && [ -z ${a:-} ]" - eval_args "# foo\nexport xyz=1234\n # bar \nexport a='bb'" - assertTrue "[ -n ${xyz:-} ] && [ -n ${a:-} ]" + eval_args "# foo +export xyz=1234 + # bar +export a='bb'" + assertTrue "[ \"${xyz:-}\" == '1234' ] && [ \"${a:-}\" == 'bb' ]" unset xyz a assertTrue "[ -z ${xyz:-} ] && [ -z ${a:-} ]" @@ -147,35 +184,35 @@ test__parse_help_from_file() { local result expected result=$(_parse_help_from_file "$MOCK_COMMAND_PATH" 'Foo') - expected='# <<-- docopt parsed arguments -->> -export cmd1="false" -export cmd2="false" -export hello_world="true" -export many="false" -export my_cmd="false" -export my_param="123" + expected="# <<-- docopt parsed arguments -->> +export cmd1='false' +export cmd2='false' +export hello_world='true' +export many='false' +export my_cmd='false' +export my_param='123' export names=() -export pos1="" -export pos2="" -export positional_param="Foo" -export some_flag="false" -# <<----------------------------->>' +export pos1='' +export pos2='' +export positional_param='Foo' +export some_flag='false' +# <<----------------------------->>" assertEquals "$expected" "$result" result=$(_parse_help_from_file "$MOCK_COMMAND_PATH" 'many' 'Foo' 'Bar Baz') - expected='# <<-- docopt parsed arguments -->> -export cmd1="false" -export cmd2="false" -export hello_world="true" -export many="true" -export my_cmd="false" -export my_param="123" -export names=("Foo" "Bar Baz") -export pos1="" -export pos2="" -export positional_param="" -export some_flag="false" -# <<----------------------------->>' + expected="# <<-- docopt parsed arguments -->> +export cmd1='false' +export cmd2='false' +export hello_world='true' +export many='true' +export my_cmd='false' +export my_param='123' +export names=('Foo' 'Bar Baz') +export pos1='' +export pos2='' +export positional_param='' +export some_flag='false' +# <<----------------------------->>" assertEquals "$expected" "$result" } @@ -205,6 +242,30 @@ pos2='CDE'" assertEquals "$expected" "$result" } +test_parse_help_does_not_evaluate_injected_code() { + local result + + # Values containing command substitution / expansions must be treated as + # literal strings during `eval`, never executed (see docopt bash_quote()). + # shellcheck disable=SC2016 # single quotes are intentional: keep the value literal + result=$("$MOCK_COMMAND_PATH" 'my-cmd' '$(echo pwned)' '`echo pwned`') + assertEquals "command substitution is not executed" \ + "pos1='\$(echo pwned)' +pos2='\`echo pwned\`'" "$result" + + # shellcheck disable=SC2016 # single quotes are intentional: keep the value literal + result=$("$MOCK_COMMAND_PATH" 'my-cmd' '$HOME' '${PATH}') + assertEquals "variable expansion is not performed" \ + "pos1='\$HOME' +pos2='\${PATH}'" "$result" + + # A literal single quote in the value must round-trip safely. + result=$("$MOCK_COMMAND_PATH" 'my-cmd' "it's" 'a b') + assertEquals "embedded single quote round-trips" \ + "pos1='it's' +pos2='a b'" "$result" +} + oneTimeSetUp() { export CLI_DIR=$PWD MOCK_COMMAND_PATH="tests/resources/commands/hello/hello-world.sh"