Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements support for file names containing spaces in the spellcheck GitHub Action. The changes convert the SOURCES_LIST variable from string concatenation to a bash array, which properly preserves spaces in file paths and prevents word splitting issues that would otherwise cause the action to fail when processing files with spaces in their names.
Changes:
- Convert
SOURCES_LISTfrom string to bash array to handle spaces in file names - Update array append operations to use proper bash array syntax with quoted elements
- Replace string emptiness checks with array length checks
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [ -n "$INPUT_OUTPUT_FILE" ] && [ -n "$SOURCES_LIST" ]; then | ||
| $COMMAND --config "$SPELLCHECK_CONFIG_FILE" --spellchecker "$SPELL_CHECKER" $TASK_NAME $SOURCES_LIST | tee "$INPUT_OUTPUT_FILE" | ||
| if [ -n "$INPUT_OUTPUT_FILE" ] && [ "${#SOURCES_LIST[@]}" -gt 0 ]; then | ||
| $COMMAND --config "$SPELLCHECK_CONFIG_FILE" --spellchecker "$SPELL_CHECKER" $TASK_NAME "${SOURCES_LIST[@]}" | tee "$INPUT_OUTPUT_FILE" |
There was a problem hiding this comment.
Variable TASK_NAME is derived directly from the untrusted action input INPUT_TASK_NAME and then expanded unquoted in the shell command at this line. If a workflow passes attacker-controlled data into task_name (for example using PR titles or other user input), shell metacharacters like $(...) or backticks in the value will be interpreted by the shell, allowing arbitrary commands to execute in the GitHub Actions runner. To prevent command injection, avoid interpolating untrusted input into a shell command string and instead pass it as a separately quoted argument (for example by building an argument list such as TASK_NAME_ARGS=(--name "$INPUT_TASK_NAME") and expanding it as "${TASK_NAME_ARGS[@]}").
There was a problem hiding this comment.
@akohout-hai what do you think about the suggestion from copilot, it follows your implementation pattern, so it would be okay IMHO. And I believe it is necessary to address the unsafe TASK_NAME
Currently, spaces in file names as mentioned in the Readme do not work due to word splitting in the entrypoint.
With these changes, the spell checker correctly processes files with spaces.