From 63fa8d6964270d6437c7e97171eb59a50aa5b851 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Thu, 18 Apr 2024 15:33:55 -0500 Subject: [PATCH] chore: prevent action bash escapes (#294) # Description @robertprast in #285 and @random-dudde over at https://github.com/random-dudde/retina/pull/1 are poking at the [commit-message](https://github.com/microsoft/retina/blob/30a128b985bc99fc8686ef21afa1cc7358dc7dfd/.github/workflows/commit-message.yaml) Action trying to pull off a bash escape exploit. The bash escape actually exists, due to the direct usage of the PR title: ```bash commit_msg_header="${{ github.event.pull_request.title }}" ``` **However, this is not readily exploitable** because we require approval to run workflows on _all_ external contributions. A maintainer would need to approve the workflow, which makes it unlikely that any useful bash could be sneakily stuffed in to the title and executed. Even getting workflow approval with a benign title and then updating it later is correctly handled by GH and requires a new maintainer approval: ![image](https://github.com/microsoft/retina/assets/2940321/0fcee51d-1f72-48c3-a961-41ef31124b78) preventing a TOCTOU malicious title swap. With that all said...unlikely does not mean impossible, and even though it is not a zero-click attack, xz showed us that social engineering can be extremely effective. This change removes the bash escape by staging the user-input in an [intermediate environment variable](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-an-intermediate-environment-variable) at the Job level. ## Related Issue If this pull request is related to any issue, please mention it here. Additionally, make sure that the issue is assigned to you before submitting this pull request. ## Checklist - [x] I have read the [contributing documentation](https://retina.sh/docs/contributing). - [x] I signed and signed-off the commits (`git commit -S -s ...`). See [this documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification) on signing commits. - [x] I have correctly attributed the author(s) of the code. - [x] I have tested the changes locally. - [x] I have followed the project's style guidelines. - [x] I have updated the documentation, if necessary. - [x] I have added tests, if applicable. ## Screenshots (if applicable) or Testing Completed Please add any relevant screenshots or GIFs to showcase the changes made. ## Additional Notes Add any additional notes or context about the pull request here. --- Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more information on how to contribute to this project. Signed-off-by: Evan Baker --- .github/workflows/commit-message.yaml | 53 ++++++++++++--------------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/.github/workflows/commit-message.yaml b/.github/workflows/commit-message.yaml index d775958300..de88a7d9e0 100644 --- a/.github/workflows/commit-message.yaml +++ b/.github/workflows/commit-message.yaml @@ -2,7 +2,7 @@ name: commit-message on: merge_group: pull_request: - branches: [ main ] + branches: [main] types: - opened - synchronize @@ -13,33 +13,26 @@ jobs: if: ${{ github.event_name != 'merge_group' }} runs-on: ubuntu-20.04 steps: - - name: verify_commit_message - run: | - if [[ "${{ github.event_name }}" == pull_request ]]; then - commit_msg_header="${{ github.event.pull_request.title }}" - else - # get first line of commit message - commit_msg_header=`echo "${{ github.event.head_commit.message }}" | head -n 1` - fi + - name: verify_commit_message + env: + TITLE: ${{ github.event.pull_request.title }} + run: | + commit_msg_type_regex='feat|fix|refactor|style|test|docs|build|tool|chore|deps' + commit_msg_scope_regex='.{1,20}' + commit_msg_subject_regex='.{1,150}' + commit_msg_regex="^(${commit_msg_type_regex})(\(${commit_msg_scope_regex}\))?: (${commit_msg_subject_regex})\$" + merge_msg_regex="^Merge branch '.+' into .+\$" + full_regex="(${commit_msg_regex})|(${merge_msg_regex})" - commit_msg_type_regex='feat|fix|refactor|style|test|docs|build|tool|chore|deps' - commit_msg_scope_regex='.{1,20}' - commit_msg_subject_regex='.{1,150}' - commit_msg_regex="^(${commit_msg_type_regex})(\(${commit_msg_scope_regex}\))?: (${commit_msg_subject_regex})\$" - merge_msg_regex="^Merge branch '.+' into .+\$" - full_regex="(${commit_msg_regex})|(${merge_msg_regex})" - - echo $commit_msg_header | grep -qP "$full_regex" || { - echo "ERROR: Invalid commit message header. Please fix format of your PR title or the commit pushed to main." - echo "Current value:" - echo "$commit_msg_header" - echo - echo "Examples of valid commits:" - echo 'example 1: "feat(cli): new feature"' - echo 'example 2: "fix(advanced-metrics): bug fix"' - echo 'example 3: "docs: update readme"' - echo - echo "Valid types are: $commit_msg_type_regex" - echo "For more details, see .github/workflows/commit-message.yaml" - exit 1 - } + grep -qP "$full_regex" <<< "$TITLE" || { + echo "ERROR: Invalid commit message header. Please fix format of your PR title." + echo + echo "Examples of valid commits:" + echo 'example 1: "feat(cli): new feature"' + echo 'example 2: "fix(advanced-metrics): bug fix"' + echo 'example 3: "docs: update readme"' + echo + echo "Valid types are: $commit_msg_type_regex" + echo "For more details, see .github/workflows/commit-message.yaml" + exit 1 + }