Improve security of github action #1822
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
bookernath
force-pushed
the
CATALYST-768
branch
from
9db1094 to
7c85082
Compare
| exit 1 | ||
| fi | ||
| echo "ref=${{ github.event.pull_request.head.ref }}" >> $GITHUB_OUTPUT | ||
|
|
||
| - name: Use commit SHA for filename |
There was a problem hiding this comment.
Was this removed intentionally?
| id: validate | ||
| run: | | ||
| # Validate ref name against allowed pattern (alphanumeric, dash, underscore, and forward slash only) | ||
| if ! [[ "${{ github.event.pull_request.head.ref }}" =~ ^[a-zA-Z0-9/_-]+$ ]]; then |
There was a problem hiding this comment.
With this line i think we should still env the ${{ github.event.pull_request.head.ref }} as if you enter something like "]]asdf" it can escape the shell condition which can lead to RCE.
Using intermediate env vars like here: https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#using-an-intermediate-environment-variable should treat any special characters as escaped which we can safely use later
There was a problem hiding this comment.
I think we could use the default environment variable GITHUB_HEAD_REF instead? Should probably have the same escaped value 🤔
⚡️🏠 Lighthouse reportLighthouse ran against https://catalyst-latest-6aebua6ms-bigcommerce-platform.vercel.app 🖥️ DesktopWe ran Lighthouse against the changes on a desktop and produced this report. Here's the summary:
📱 MobileWe ran Lighthouse against the changes on a mobile and produced this report. Here's the summary:
|
|
Will reopen against |
What/Why?
https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#understanding-the-risk-of-script-injections
Testing
N/A