Fix special characters in arguments

core/helpers/cli_parse_args.sh

@@ -35,11 +35,11 @@ _is_str_to_eval() {
   #   _is_str_to_eval <text>
   #
   # 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() {

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}")
 

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)
 )

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"