-
Notifications
You must be signed in to change notification settings - Fork 68
Multiline string for packages breaks everything #88
Description
If you have a lot of packages and use a multiline string for the packages parameter the setup fails, because bash attempts top run the second line as a command.
Instead of directly using the GitHub action variable interpolation you should pass the parameters as environment variables in order to prevent breakage (and command injection):
runs:
using: "composite"
steps:
- id: pre-cache
run: |
${GITHUB_ACTION_PATH}/pre_cache_action.sh \
~/cache-apt-pkgs \
"$VERSION" \
"$EXEC_INSTALL_SCRIPTS" \
"$DEBUG" \
"$PACKAGES"
echo "CACHE_KEY=$(cat ~/cache-apt-pkgs/cache_key.md5)" >> $GITHUB_ENV
shell: bash
env:
VERSION: "${{ inputs.version }}"
EXEC_INSTALL_SCRIPTS: "${{ inputs.execute_install_scripts }}"
DEBUG: "${{ inputs.debug }}"
PACKAGES: "${{ inputs.packages }}"Change here:
cache-apt-pkgs-action/action.yml
Lines 44 to 52 in 9b2b4f2
| - id: pre-cache | |
| run: | | |
| ${GITHUB_ACTION_PATH}/pre_cache_action.sh \ | |
| ~/cache-apt-pkgs \ | |
| "${{ inputs.version }}" \ | |
| "${{ inputs.execute_install_scripts }}" \ | |
| "${{ inputs.debug }}" \ | |
| ${{ inputs.packages }} | |
| echo "CACHE_KEY=$(cat ~/cache-apt-pkgs/cache_key.md5)" >> $GITHUB_ENV |
cache-apt-pkgs-action/action.yml
Lines 61 to 73 in 9b2b4f2
| - id: post-cache | |
| run: | | |
| ${GITHUB_ACTION_PATH}/post_cache_action.sh \ | |
| ~/cache-apt-pkgs \ | |
| / \ | |
| "${{ steps.load-cache.outputs.cache-hit }}" \ | |
| "${{ inputs.execute_install_scripts }}" \ | |
| "${{ inputs.debug }}" \ | |
| ${{ inputs.packages }} | |
| function create_list { local list=$(cat ~/cache-apt-pkgs/manifest_${1}.log | tr '\n' ','); echo ${list:0:-1}; }; | |
| echo "package-version-list=$(create_list main)" >> $GITHUB_OUTPUT | |
| echo "all-package-version-list=$(create_list all)" >> $GITHUB_OUTPUT | |
| shell: bash |
Note that simply adding " " around ${{ inputs.packages }} in the run command will still allow command injection if someone uses a " in the packages parameter. So using env vars is cleaner.
However, this then passes the packages as a single parameter, but since you then do this anyway it shouldn't matter (in fact make that simpler, since you merge it into a single string anyway):
cache-apt-pkgs-action/pre_cache_action.sh
Lines 28 to 31 in 9b2b4f2
| input_packages="${@:5}" | |
| # Trim commas, excess spaces, and sort. | |
| packages="$(normalize_package_list "${input_packages}")" |
cache-apt-pkgs-action/post_cache_action.sh
Lines 29 to 35 in 9b2b4f2
| packages="${@:6}" | |
| if [ "$cache_hit" == true ]; then | |
| ${script_dir}/restore_pkgs.sh "${cache_dir}" "${cache_restore_root}" "${execute_install_scripts}" "${debug}" | |
| else | |
| ${script_dir}/install_and_cache_pkgs.sh "${cache_dir}" "${debug}" ${packages} | |
| fi |
cache-apt-pkgs-action/install_and_cache_pkgs.sh
Lines 19 to 22 in 9b2b4f2
| input_packages="${@:3}" | |
| # Trim commas, excess spaces, and sort. | |
| normalized_packages="$(normalize_package_list "${input_packages}")" |