-
-
Notifications
You must be signed in to change notification settings - Fork 5
feat: add SAST onboarding automation #72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| To configure the scanner for your repository's specific needs, | ||
| please review the workflow file and adjust as necessary." | ||
|
|
||
| git push origin "$BRANCH_NAME" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Documented opt-out mechanism not implemented in workflow
The PR template instructs users to add a .github/no-security-scanner file to "prevent future onboarding attempts", but the workflow never checks for this file's existence before creating the branch and PR. Teams that follow the opt-out instructions will continue receiving unwanted onboarding PRs, breaking the documented contract.
Additional Locations (1)
| if [ "${{ github.event_name }}" = "repository_dispatch" ]; then | ||
| ORG="${{ github.event.client_payload.organization }}" | ||
| REPO_NAME="${{ github.event.client_payload.repository }}" | ||
| REPO="$ORG/$REPO_NAME" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Unsanitized repository_dispatch payload enables command injection
Values from github.event.client_payload.organization and github.event.client_payload.repository are used directly in shell commands via ${{ }} expression syntax. Since repository_dispatch events can be triggered by external webhooks with arbitrary payloads, a malicious actor could inject shell commands through these fields. The values are used in API calls, git operations, and file paths without any validation or sanitization.
| repository: ${{ steps.target.outputs.repository }} | ||
| token: ${{ secrets.ONBOARDING_TOKEN }} | ||
| path: target-repo | ||
| ref: ${{ steps.target.outputs.base_branch }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Opt-out file check documented but never implemented
The PR body template tells users to add a .github/no-security-scanner file to prevent future onboarding attempts, but the workflow never checks for this file's existence. Repositories that have opted out will still receive onboarding PRs, causing frustration for teams that explicitly declined the scanner.
Additional Locations (1)
| # Copy the security scanner workflow template and replace placeholders | ||
| sed "s/{ DEFAULT_BRANCH }/$BASE_BRANCH/g" \ | ||
| ../scanner-repo/.github/templates/security-code-scanner.yml \ | ||
| > .github/workflows/security-code-scanner.yml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: sed fails when branch name contains forward slashes
The sed command uses / as a delimiter when substituting { DEFAULT_BRANCH } with $BASE_BRANCH. If the default branch name contains a forward slash (e.g., release/main or feature/default), the substitution will fail because the / in the branch name will be interpreted as a sed delimiter. Using a different delimiter like | or # would handle this edge case.
| echo "Repository empty status: $IS_EMPTY" | ||
| shell: bash | ||
| env: | ||
| GH_TOKEN: ${{ secrets.ONBOARDING_TOKEN }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Opt-out file check documented but never implemented
The PR body template instructs users to add a .github/no-security-scanner file to prevent future onboarding attempts, but the workflow never checks for this file's existence. The onboarding will proceed regardless of whether a team has opted out, making the documented opt-out mechanism non-functional.
Additional Locations (1)
| run: | | ||
| if [ "${{ github.event_name }}" = "repository_dispatch" ]; then | ||
| ORG="${{ github.event.client_payload.organization }}" | ||
| REPO_NAME="${{ github.event.client_payload.repository }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Script injection via unsanitized repository_dispatch payload
The workflow directly interpolates github.event.client_payload.organization and github.event.client_payload.repository into shell commands using ${{ }} syntax. Since repository_dispatch payloads can be controlled by external actors (anyone with a token that has write access), a malicious payload containing shell metacharacters like backticks or $() would execute arbitrary commands during workflow runs. This is a documented GitHub Actions script injection vulnerability pattern. Values should be passed via environment variables rather than direct interpolation.
| REPO_NAME="${{ github.event.client_payload.repository }}" | ||
| REPO="$ORG/$REPO_NAME" | ||
| else | ||
| REPO="${{ inputs.organization }}/${{ inputs.repository }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Shell command injection via unsanitized user inputs
The workflow directly interpolates user-controlled values from github.event.client_payload.organization, github.event.client_payload.repository, inputs.organization, and inputs.repository into shell commands using ${{ }} syntax. This is a known GitHub Actions security vulnerability allowing shell command injection. The repository_dispatch event's client_payload can be sent by anyone with repository access, and malicious values like foo; malicious_command would execute arbitrary commands. The safer approach is to pass these values through environment variables.
| # Copy the security scanner workflow template and replace placeholders | ||
| sed "s/{ DEFAULT_BRANCH }/$BASE_BRANCH/g" \ | ||
| ../scanner-repo/.github/templates/security-code-scanner.yml \ | ||
| > .github/workflows/security-code-scanner.yml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Sed command breaks on branch names containing slashes
The sed command uses / as the delimiter when replacing { DEFAULT_BRANCH } with $BASE_BRANCH. If the default branch name contains a / character (e.g., release/main or develop/stable), the sed substitution will fail or produce incorrect output because the / in the branch name will be interpreted as a delimiter boundary.
|
|
||
| 1. **Add a comment on this PR** explaining why your team is opting out | ||
| 2. **Close this PR** to prevent auto-merge | ||
| 3. **Add a `.github/no-security-scanner` file** to your repository to prevent future onboarding attempts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Opt-out file documented but never checked in workflow
The PR body template instructs teams to add a .github/no-security-scanner file to "prevent future onboarding attempts", but the workflow never actually checks for this file before proceeding with onboarding. This means the documented opt-out mechanism doesn't work - repositories that have added the file will still receive onboarding PRs.
Changes
Onboarding workflow (
.github/workflows/onboard-new-repo.yml)repository_dispatchevents from webhooksPR template improvements (
.github/templates/onboarding-pr-body-automated.md)Usage
Manual:
Automated:
Set up webhook to send
repository_dispatchwith:{ "event_type": "new_repository_created", "client_payload": { "organization": "MetaMask", "repository": "repo-name", "base_branch": "main" } } <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds an automated workflow to onboard repositories to the MetaMask Security Code Scanner, plus PR body templates and an example scanner workflow. > > - **Workflows**: > - Add onboarding workflow `/.github/workflows/onboard-new-repo.yml` to create PRs that add the scanner to target repos. > - Supports manual and `repository_dispatch` triggers, auto-detects default branch, handles empty repos, creates branch/PR, and substitutes variables in PR body. > - Add reusable scanner workflow template `/.github/templates/security-code-scanner.yml` with default `paths-ignored`, secrets, and `{ DEFAULT_BRANCH }` placeholder. > - **Templates**: > - Add automated PR body `/.github/templates/onboarding-pr-body-automated.md` with auto-merge notice, checklist, config examples, and variable `{{SECURITY_SCANNING_URL}}`. > - Add templates README `/.github/templates/README.md` describing usage and variables. > - **Examples**: > - Add example workflow `examples/security-code-scanner.yml` for reference usage. > - **Docs**: > - Update `CHANGELOG.md` with recent changes and fixes. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 6d3ad6bdd9fafa1b6fecb4482478abcf4e708dc6. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->