Skip to content

Commit

Permalink
chore: prevent action bash escapes (#294)
Browse files Browse the repository at this point in the history
# 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 <[email protected]>
  • Loading branch information
rbtr authored Apr 18, 2024
1 parent 30a128b commit 63fa8d6
Showing 1 changed file with 23 additions and 30 deletions.
53 changes: 23 additions & 30 deletions .github/workflows/commit-message.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: commit-message
on:
merge_group:
pull_request:
branches: [ main ]
branches: [main]
types:
- opened
- synchronize
Expand All @@ -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
}

0 comments on commit 63fa8d6

Please sign in to comment.