Browse Source

build(ci): Change `acceptance` workflow back to `pull_request` (#22344)

This addresses a security vulnerability with how we were using `pull_request_target` and `actions/checkout`. Please read #22432 for more information.

Closes #22432
Billy Vong 4 years ago
parent
commit
b1afda9010
2 changed files with 25 additions and 67 deletions
  1. 4 67
      .github/workflows/acceptance.yml
  2. 21 0
      .github/workflows/visual-diff.yml

+ 4 - 67
.github/workflows/acceptance.yml

@@ -11,12 +11,7 @@ on:
     branches:
       - master
       - releases/**
-  # XXX: We are using `pull_request_target` instead of `pull_request` because we want
-  # Visual Snapshots to run on forks.  It allows forks to access secrets safely by
-  # only running workflows from the main branch. Prefer to use `pull_request` when possible.
-  #
-  # See https://github.com/getsentry/sentry/pull/21600 for more details
-  pull_request_target:
+  pull_request:
 
 jobs:
   frontend:
@@ -28,17 +23,7 @@ jobs:
       VISUAL_HTML_ENABLE: 1
     steps:
       - uses: actions/checkout@v2
-        name: Checkout sentry (pull_request_target)
-        if: github.event.pull_request.head.ref != ''
-        with:
-          # Note this is required because of `pull_request_target`, which allows
-          # forks to access secrets safely by only running workflows from the main branch.
-          ref: ${{ github.event.pull_request.head.ref }}
-          repository: ${{ github.event.pull_request.head.repo.full_name }}
-
-      - uses: actions/checkout@v2
-        name: Checkout sentry (push)
-        if: github.event.pull_request.head.ref == ''
+        name: Checkout sentry
 
       - uses: volta-cli/action@v1
 
@@ -100,17 +85,7 @@ jobs:
 
     steps:
       - uses: actions/checkout@v2
-        name: Checkout sentry (pull_request_target)
-        if: github.event.pull_request.head.ref != ''
-        with:
-          # Note this is required because of `pull_request_target`, which allows
-          # forks to access secrets safely by only running workflows from the main branch.
-          ref: ${{ github.event.pull_request.head.ref }}
-          repository: ${{ github.event.pull_request.head.repo.full_name }}
-
-      - uses: actions/checkout@v2
-        name: Checkout sentry (push)
-        if: github.event.pull_request.head.ref == ''
+        name: Checkout sentry
 
       - uses: volta-cli/action@v1
 
@@ -153,6 +128,7 @@ jobs:
       - name: webpack
         env:
           SENTRY_INSTRUMENTATION: 1
+          # this is fine to not have for forks, it shouldn't fail
           SENTRY_WEBPACK_WEBHOOK_SECRET: ${{ secrets.SENTRY_WEBPACK_WEBHOOK_SECRET }}
         run: |
           yarn webpack --display errors-only
@@ -174,42 +150,3 @@ jobs:
         with:
           save-only: true
           snapshot-path: .artifacts/visual-snapshots
-
-  visual-diff:
-    # Note: `github.ref` is always `master` for `pull_request_target` event
-    # `github.head/base_ref` == '' for `push` event, we want to skip it on pushes
-    if: github.head_ref != ''
-    needs: [acceptance, frontend]
-    runs-on: ubuntu-16.04
-    timeout-minutes: 20
-
-    steps:
-      # We need to checkout the repository so that we can fetch the merge base from git
-      - uses: actions/checkout@v2
-        name: Checkout sentry
-        if: github.event.pull_request.head.ref != ''
-        with:
-          ref: ${{ github.event.pull_request.head.ref }}
-          repository: ${{ github.event.pull_request.head.repo.full_name }}
-          # fetch all history so we can use `git merge-base`
-          fetch-depth: 0
-
-      # Because of pull_request_target event, `github.event.pull_request.base` will refer to latest
-      # commit on main branch when opening the PR
-      - name: Get merge base
-        if: github.event.pull_request.head.ref != ''
-        id: merge-base
-        env:
-          REF: ${{ github.event.pull_request.head.ref }}
-        run: |
-          echo "::set-output name=sha::$(git merge-base origin/master $REF)"
-
-      - name: Diff snapshots
-        id: visual-snapshots-diff
-        uses: getsentry/action-visual-snapshot@v2
-        with:
-          merge-base: ${{ steps.merge-base.outputs.sha }}
-          api-token: ${{ secrets.VISUAL_SNAPSHOT_SECRET }}
-          github-token: ${{ secrets.GITHUB_TOKEN }}
-          gcs-bucket: 'sentry-visual-snapshots'
-          gcp-service-account-key: ${{ secrets.SNAPSHOT_GOOGLE_SERVICE_ACCOUNT_KEY }}

+ 21 - 0
.github/workflows/visual-diff.yml

@@ -0,0 +1,21 @@
+name: visual diff
+on:
+  workflow_run:
+    workflows:
+      - acceptance
+    types:
+      - completed
+
+jobs:
+  visual-diff:
+    runs-on: ubuntu-16.04
+    timeout-minutes: 20
+
+    steps:
+      - name: Diff snapshots
+        id: visual-snapshots-diff
+        uses: getsentry/action-visual-snapshot@v2
+        with:
+          api-token: ${{ secrets.VISUAL_SNAPSHOT_SECRET }}
+          gcs-bucket: 'sentry-visual-snapshots'
+          gcp-service-account-key: ${{ secrets.SNAPSHOT_GOOGLE_SERVICE_ACCOUNT_KEY }}