242e2faa51675494cbfa78a81f3ff47d81039863

Author
Marc Cornellà <marc@mcornella.com>
Committer
GitHub <noreply@github.com>
Date

Message

ci: improve security in project.yml workflow (#13329)

There is no inherent security vulnerability in the workflow, but there were
certain practices that increased latent risk. In this commit, we:

- Explicitly bind app token for each step that needs it, instead of setting it for
  all steps after "Store app token"
- Refactor "classify" step, to not rely on files passed around, and instead uses
  only awk script.
- Remove all instances of template injection within `run` scripts. There was nothing
  dangerous, but the practice is unsafe.
- Sanitize all unwanted characters from PR plugin and theme names.

References: W2M1-06 W2M1-07

Diff

This diff is truncated to protect this page.

  1diff --git a/.github/workflows/project.yml b/.github/workflows/project.yml
  2index ba971db15c4eaa3d0d92a1816632888cfef201d1..e6da2cbe5527722ee508d63e5fd45e7d0f1c8040 100644
  3--- a/.github/workflows/project.yml
  4+++ b/.github/workflows/project.yml
  5@@ -20,17 +20,15 @@ jobs:
  6         uses: step-security/harden-runner@f4a75cfd619ee5ce8d5b864b0d183aff3c69b55a # v2.13.1
  7         with:
  8           egress-policy: audit
  9-
 10       - name: Authenticate as @ohmyzsh
 11         id: generate-token
 12         uses: actions/create-github-app-token@67018539274d69449ef7c02e8e71183d1719ab42 # v2.1.4
 13         with:
 14           app-id: ${{ secrets.OHMYZSH_APP_ID }}
 15           private-key: ${{ secrets.OHMYZSH_APP_PRIVATE_KEY }}
 16-      - name: Store app token
 17-        run: echo "GH_TOKEN=${{ steps.generate-token.outputs.token }}" >> "$GITHUB_ENV"
 18       - name: Read project data
 19         env:
 20+          GH_TOKEN: ${{ steps.generate-token.outputs.token }}
 21           ORGANIZATION: ohmyzsh
 22           PROJECT_NUMBER: "1"
 23         run: |
 24@@ -53,14 +51,14 @@ jobs:
 25             }' -f org=$ORGANIZATION -F number=$PROJECT_NUMBER > project_data.json
 26 
 27           # Parse project data
 28-          cat >> $GITHUB_ENV <<EOF
 29+          cat >> "$GITHUB_ENV" <<EOF
 30           PROJECT_ID=$(jq '.data.organization.projectV2.id' project_data.json)
 31           PLUGIN_FIELD_ID=$(jq '.data.organization.projectV2.fields.nodes[] | select(.name == "Plugin") | .id' project_data.json)
 32           THEME_FIELD_ID=$(jq '.data.organization.projectV2.fields.nodes[] | select(.name == "Theme") | .id' project_data.json)
 33           EOF
 34-
 35       - name: Add to project
 36         env:
 37+          GH_TOKEN: ${{ steps.generate-token.outputs.token }}
 38           ISSUE_OR_PR_ID: ${{ github.event.issue.node_id || github.event.pull_request.node_id }}
 39         run: |
 40           item_id="$(gh api graphql -f query='
 41@@ -71,45 +69,51 @@ jobs:
 42                 }
 43               }
 44             }
 45-          ' -f project=$PROJECT_ID -f content=$ISSUE_OR_PR_ID --jq '.data.addProjectV2ItemById.item.id')"
 46+          ' -f project="$PROJECT_ID" -f content="$ISSUE_OR_PR_ID" --jq '.data.addProjectV2ItemById.item.id')"
 47 
 48           echo "ITEM_ID=$item_id" >> $GITHUB_ENV
 49-
 50       - name: Classify Pull Request
 51         if: github.event_name == 'pull_request_target'
 52+        env:
 53+          GH_TOKEN: ${{ steps.generate-token.outputs.token }}
 54+          PR_NUMBER: ${{ github.event.pull_request.number }}
 55         run: |
 56-          touch plugins.list themes.list
 57-
 58-          gh pr view ${{ github.event.pull_request.number }} \
 59-            --repo ${{ github.repository }} \
 60+          # Get the list of modified files in the PR, and extract plugins and themes
 61+          gh pr view "$PR_NUMBER" \
 62+            --repo "$GITHUB_REPOSITORY" \
 63             --json files --jq '.files.[].path' | awk -F/ '
 64+            BEGIN {
 65+              plugins = 0
 66+              themes = 0
 67+            }
 68             /^plugins\// {
 69-              plugins[$2] = 1
 70+              if (plugin == $2) next
 71+              plugin = $2
 72+              plugins++
 73             }
 74             /^themes\// {
 75               gsub(/\.zsh-theme$/, "", $2)
 76-              themes[$2] = 1
 77+              if (theme == $2) next
 78+              theme = $2
 79+              themes++
 80             }
 81             END {
 82-              for (plugin in plugins) {
 83-                print plugin >> "plugins.list"
 84+              # plugin and theme are values controlled by the PR author
 85+              # so we should sanitize them before using anywhere else
 86+              if (plugins == 1) {
 87+                gsub(/[^a-zA-Z0-9._-]/, "", plugin)
 88+                print "PLUGIN=" plugin
 89               }
 90-              for (theme in themes) {
 91-                print theme >> "themes.list"
 92+              if (themes == 1) {
 93+                gsub(/[^a-zA-Z0-9._-]/, "", theme)
 94+                print "THEME=" theme
 95               }
 96             }
 97-          '
 98-          # If only one plugin is modified, add it to the plugin field
 99-          if [[ $(wc -l < plugins.list) = 1 ]]; then
100-            echo "PLUGIN=$(cat plugins.list)" >> $GITHUB_ENV
101-          fi
102-          # If only one theme is modified, add it to the theme field
103-          if [[ $(wc -l < themes.list) = 1 ]]; then
104-            echo "THEME=$(cat themes.list)" >> $GITHUB_ENV