From b2c8af9d3131c71e2f31075f52f4cf2973b70269 Mon Sep 17 00:00:00 2001 From: "James M. Greene" Date: Fri, 17 Sep 2021 17:41:13 -0500 Subject: [PATCH] Secure early access staging deployment (#21450) * Explicitly ensure the early access parent directories are created when cloning * Use explicit --file flag with tar * Remove security hole for Staging deployment by concatenating archives * Fail the staging builds if *.js, .npmrc, or Procfile is changed in the open source repo * docker build: extract user-code in separate directory * Checkout PR base branch and install dependencies * Remove one-off package installs * Remove selective file checkout * Don't persist git cloning credentials It usually makes the clone of early access content fail in the later steps * Update .github/workflows/staging-deploy-pr-docker.yml Co-authored-by: James M. Greene * Update .github/workflows/staging-deploy-pr-docker.yml Co-authored-by: James M. Greene * Remove debugging step * Best practice: Use environment variables to avoid potential injection attacks if the data was user-controlled Co-authored-by: Mike Surowiec Co-authored-by: Robert Sese Co-authored-by: Robert Sese --- .github/workflows/staging-build-pr-docker.yml | 9 +- .github/workflows/staging-build-pr.yml | 32 ++++++- .../workflows/staging-deploy-pr-docker.yml | 70 +++++++------- .github/workflows/staging-deploy-pr.yml | 95 ++++++++++--------- script/early-access/clone-for-build.js | 3 + 5 files changed, 122 insertions(+), 87 deletions(-) diff --git a/.github/workflows/staging-build-pr-docker.yml b/.github/workflows/staging-build-pr-docker.yml index 570d9d6d97..6e9075b418 100644 --- a/.github/workflows/staging-build-pr-docker.yml +++ b/.github/workflows/staging-build-pr-docker.yml @@ -33,7 +33,7 @@ jobs: # Make sure only approved files are changed if it's in github/docs - name: Check changed files - if: github.repository == 'github/docs' && github.event.pull_request.user.login != 'Octomerger' + if: ${{ github.repository == 'github/docs' && github.event.pull_request.user.login != 'Octomerger' }} uses: dorny/paths-filter@eb75a1edc117d3756a18ef89958ee59f9500ba58 id: filter with: @@ -47,20 +47,23 @@ jobs: # Returns list of changed files matching each filter filters: | notAllowed: + - '*.js' - '*.mjs' - '*.ts' - '*.tsx' - '*.json' + - '.npmrc' + - 'script/**' - 'Dockerfile*' # When there are changes to files we can't accept - - name: 'Fail when not allowed files are changed' + - name: Fail when disallowed files are changed if: ${{ steps.filter.outputs.notAllowed == 'true' }} run: exit 1 - name: Create an archive run: | - tar -cf app.tar \ + tar -c --file=app.tar \ assets/ \ content/ \ stylesheets/ \ diff --git a/.github/workflows/staging-build-pr.yml b/.github/workflows/staging-build-pr.yml index e469ab411d..8528681bcc 100644 --- a/.github/workflows/staging-build-pr.yml +++ b/.github/workflows/staging-build-pr.yml @@ -26,6 +26,36 @@ jobs: - name: Check out repo uses: actions/checkout@5a4ac9002d0be2fb38bd78e4b4dbde5606d7042f + # Make sure only approved files are changed if it's in github/docs + - name: Check changed files + if: ${{ github.repository == 'github/docs' && github.event.pull_request.user.login != 'Octomerger' }} + uses: dorny/paths-filter@eb75a1edc117d3756a18ef89958ee59f9500ba58 + id: filter + with: + # Base branch used to get changed files + base: 'main' + + # Enables setting an output in the format in `${FILTER_NAME}_files + # with the names of the matching files formatted as JSON array + list-files: json + + # Returns list of changed files matching each filter + filters: | + notAllowed: + - '*.js' + - '*.mjs' + - '*.ts' + - '*.tsx' + - '*.json' + - '.npmrc' + - 'script/**' + - 'Procfile' + + # When there are changes to files we can't accept + - name: Fail when disallowed files are changed + if: ${{ steps.filter.outputs.notAllowed == 'true' }} + run: exit 1 + - name: Setup node uses: actions/setup-node@38d90ce44d5275ad62cc48384b3d8a58c500bb5f with: @@ -53,7 +83,7 @@ jobs: - name: Create an archive run: | - tar -cf app.tar \ + tar -c --file=app.tar \ node_modules/ \ .next/ \ assets/ \ diff --git a/.github/workflows/staging-deploy-pr-docker.yml b/.github/workflows/staging-deploy-pr-docker.yml index 92650f6b99..3a450ad0b0 100644 --- a/.github/workflows/staging-deploy-pr-docker.yml +++ b/.github/workflows/staging-deploy-pr-docker.yml @@ -19,6 +19,8 @@ permissions: statuses: write env: + EARLY_ACCESS_SCRIPT_PATH: script/early-access/clone-for-build.js + EARLY_ACCESS_SUPPORT_FILES: script/package.json # In this specific workflow relationship, the `github.event.workflow_run.pull_requests` # array will always contain only 1 item! Specifically, it will contain the PR associated # with the `github.event.workflow_run.head_branch` that triggered the preceding @@ -90,56 +92,49 @@ jobs: app_name: ${{ steps.create-app.outputs.app_name}} docker_image_id: ${{ steps.image-id.outputs.image_id}} steps: - - name: Dump event context - env: - GITHUB_EVENT_CONTEXT: ${{ toJSON(github.event) }} - run: echo "$GITHUB_EVENT_CONTEXT" - - name: Check out repo's default branch uses: actions/checkout@5a4ac9002d0be2fb38bd78e4b4dbde5606d7042f with: + # For enhanced security: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ persist-credentials: 'false' - - name: Download build artifact - uses: dawidd6/action-download-artifact@b9571484721e8187f1fd08147b497129f8972c74 - with: - workflow: ${{ github.event.workflow_run.workflow_id }} - run_id: ${{ github.event.workflow_run.id }} - name: pr_build_docker - path: ./ - - - name: Show contents - run: ls -l - - - name: Extract the archive - run: | - tar -xf app.tar -C ./ - rm app.tar - - - name: Show contents again - run: ls -l - - - if: ${{ github.repository == 'github/docs-internal' }} - name: Setup node to clone early access + - name: Setup node uses: actions/setup-node@38d90ce44d5275ad62cc48384b3d8a58c500bb5f with: node-version: 16.8.x cache: npm - # Add any dependencies that are needed for this workflow below - - if: ${{ github.repository == 'github/docs-internal' }} - name: Install temporary development-only dependencies - run: npm install --no-save rimraf dotenv + - name: Install dependencies + run: npm ci - if: ${{ github.repository == 'github/docs-internal' }} name: Clone early access - run: node script/early-access/clone-for-build.js + run: node ${{ env.EARLY_ACCESS_SCRIPT_PATH }} env: DOCUBOT_REPO_PAT: ${{ secrets.DOCUBOT_REPO_PAT }} GIT_BRANCH: ${{ github.event.workflow_run.head_branch }} - - name: Install one-off development-only dependencies - run: npm install --no-save --include=optional esm + - if: ${{ github.repository == 'github/docs-internal' }} + name: Create an archive for early access + run: | + tar -c --file=early-access.tar \ + assets/ \ + content/ \ + data/ + + # Download the previously built "app.tar" + - name: Download build artifact + uses: dawidd6/action-download-artifact@b9571484721e8187f1fd08147b497129f8972c74 + with: + workflow: ${{ github.event.workflow_run.workflow_id }} + run_id: ${{ github.event.workflow_run.id }} + name: pr_build + path: ./ + + # Append "early-access.tar" into "app.tar" + - if: ${{ github.repository == 'github/docs-internal' }} + name: Concatenate the archives + run: tar -A --file=app.tar early-access.tar - name: Create app id: create-app @@ -182,9 +177,13 @@ jobs: throw(err) } + - name: Extract user-changes to tmp directory + run: | + tar -x --file=app.tar -C ${{ runner.temp }}/ + - name: Build, tag, push, and release the Docker image run: | - docker image build --target production_early_access -t registry.heroku.com/${{ steps.create-app.outputs.app_name}}/web . + docker image build -f ${{ runner.temp }}/Dockerfile --target production_early_access -t registry.heroku.com/${{ steps.create-app.outputs.app_name}}/web ${{ runner.temp }} heroku container:login docker push registry.heroku.com/${{ steps.create-app.outputs.app_name }}/web heroku container:release web --app=${{ steps.create-app.outputs.app_name }} @@ -202,9 +201,6 @@ jobs: echo "::set-output name=image_id::$(docker image inspect registry.heroku.com/${{ steps.create-app.outputs.app_name }}/web --format={{.Id}})" exit 1 # Stop at this point, don't move on to prepare job - # TODO - heroku stuff - # - create a release based on the image - - name: Send Slack notification if workflow fails uses: someimportantcompany/github-actions-slack-message@0b470c14b39da4260ed9e3f9a4f1298a74ccdefd if: ${{ failure() }} diff --git a/.github/workflows/staging-deploy-pr.yml b/.github/workflows/staging-deploy-pr.yml index 330ee9c2da..8831358026 100644 --- a/.github/workflows/staging-deploy-pr.yml +++ b/.github/workflows/staging-deploy-pr.yml @@ -31,6 +31,7 @@ env: CONTEXT_NAME: '${{ github.workflow }} / deploy (${{ github.event.workflow_run.event }})' ACTIONS_RUN_LOG: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }} BUILD_ACTIONS_RUN_LOG: https://github.com/${{ github.repository }}/actions/runs/${{ github.event.workflow_run.id }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} jobs: notify-of-failed-builds: @@ -45,18 +46,13 @@ jobs: # Specifically omitting a concurrency group here in case the build was not # successful BECAUSE a subsequent build already canceled it steps: - - name: Dump GitHub context - env: - GITHUB_CONTEXT: ${{ toJSON(github) }} - run: echo "$GITHUB_CONTEXT" - - name: Send Slack notification if build workflow failed uses: someimportantcompany/github-actions-slack-message@0b470c14b39da4260ed9e3f9a4f1298a74ccdefd with: channel: ${{ secrets.DOCS_STAGING_DEPLOYMENT_FAILURES_SLACK_CHANNEL_ID }} bot-token: ${{ secrets.SLACK_DOCS_BOT_TOKEN }} color: failure - text: Staging build failed for PR ${{ env.PR_URL }} at commit ${{ github.event.workflow_run.head_sha }}. See ${{ env.BUILD_ACTIONS_RUN_LOG }}. This run was ${{ env.ACTIONS_RUN_LOG }}. + text: Staging build failed for PR ${{ env.PR_URL }} at commit ${{ env.HEAD_SHA }}. See ${{ env.BUILD_ACTIONS_RUN_LOG }}. This run was ${{ env.ACTIONS_RUN_LOG }}. check-pr-before-prepare: if: >- @@ -76,13 +72,15 @@ jobs: - name: Check pull request state id: check-pr uses: actions/github-script@2b34a689ec86a68d8ab9478298f91d5401337b7d + env: + PR_NUMBER: ${{ env.PR_NUMBER }} with: script: | const { owner, repo } = context.repo const { data: pullRequest } = await github.pulls.get({ owner, repo, - pull_number: ${{ github.event.workflow_run.pull_requests[0].number }} + pull_number: process.env.PR_NUMBER }) core.setOutput('state', pullRequest.state) @@ -102,33 +100,21 @@ jobs: env: CONTEXT_NAME: ${{ env.CONTEXT_NAME }} ACTIONS_RUN_LOG: ${{ env.ACTIONS_RUN_LOG }} + HEAD_SHA: ${{ env.HEAD_SHA }} with: script: | - const { CONTEXT_NAME, ACTIONS_RUN_LOG } = process.env + const { CONTEXT_NAME, ACTIONS_RUN_LOG, HEAD_SHA } = process.env const { owner, repo } = context.repo await github.repos.createCommitStatus({ owner, repo, - sha: '${{ github.event.workflow_run.head_sha }}', + sha: HEAD_SHA, context: CONTEXT_NAME, state: 'pending', description: 'The app is being deployed. See logs.', target_url: ACTIONS_RUN_LOG }) - - name: Download build artifact - uses: dawidd6/action-download-artifact@b9571484721e8187f1fd08147b497129f8972c74 - with: - workflow: ${{ github.event.workflow_run.workflow_id }} - run_id: ${{ github.event.workflow_run.id }} - name: pr_build - path: ./ - - - name: Extract the archive - run: | - tar -xf app.tar -C ./ - rm app.tar - - if: ${{ github.repository == 'github/docs-internal' }} name: Setup node to clone early access uses: actions/setup-node@38d90ce44d5275ad62cc48384b3d8a58c500bb5f @@ -143,10 +129,10 @@ jobs: files: ${{ env.EARLY_ACCESS_SCRIPT_PATH }} ${{ env.EARLY_ACCESS_SUPPORT_FILES }} token: ${{ secrets.GITHUB_TOKEN }} - # Add any dependencies that are needed for this workflow below + # Install any dependencies that are needed for the early access script - if: ${{ github.repository == 'github/docs-internal' }} - name: Install temporary development-only dependencies - run: npm install --no-save rimraf + name: Install temporary dependencies + run: npm install --no-save dotenv rimraf - if: ${{ github.repository == 'github/docs-internal' }} name: Clone early access @@ -155,19 +141,31 @@ jobs: DOCUBOT_REPO_PAT: ${{ secrets.DOCUBOT_REPO_PAT }} GIT_BRANCH: ${{ github.event.workflow_run.head_branch }} - # Remove any dependencies installed for this workflow below - if: ${{ github.repository == 'github/docs-internal' }} - name: Remove development-only dependencies - run: npm prune --production - - - if: ${{ github.repository == 'github/docs-internal' }} - name: Delete the script directory after cloning early access - run: rm -rf script/ - - - name: Create a gzipped archive + name: Create an archive for early access run: | - touch app.tar.gz - tar --exclude=app.tar.gz -czf app.tar.gz ./ + tar -c --file=early-access.tar \ + assets/ \ + content/ \ + data/ + + # Download the previously built "app.tar" + - name: Download build artifact + uses: dawidd6/action-download-artifact@b9571484721e8187f1fd08147b497129f8972c74 + with: + workflow: ${{ github.event.workflow_run.workflow_id }} + run_id: ${{ github.event.workflow_run.id }} + name: pr_build + path: ./ + + # Append "early-access.tar" into "app.tar" + - if: ${{ github.repository == 'github/docs-internal' }} + name: Concatenate the archives + run: tar -A --file=app.tar early-access.tar + + # Create "app.tar.gz" and delete "app.tar" + - name: Create a gzipped archive + run: gzip app.tar - name: Install Heroku client development-only dependency run: npm install --no-save heroku-client @@ -199,8 +197,10 @@ jobs: # See: https://devcenter.heroku.com/articles/build-and-release-using-the-api#sources-endpoint - name: Upload to the Heroku build source + env: + UPLOAD_URL: ${{ steps.build-source.outputs.upload_url }} run: | - curl '${{ steps.build-source.outputs.upload_url }}' \ + curl '$UPLOAD_URL' \ -X PUT \ -H 'Content-Type:' \ --data-binary @app.tar.gz @@ -211,14 +211,15 @@ jobs: env: CONTEXT_NAME: ${{ env.CONTEXT_NAME }} ACTIONS_RUN_LOG: ${{ env.ACTIONS_RUN_LOG }} + HEAD_SHA: ${{ env.HEAD_SHA }} with: script: | - const { CONTEXT_NAME, ACTIONS_RUN_LOG } = process.env + const { CONTEXT_NAME, ACTIONS_RUN_LOG, HEAD_SHA } = process.env const { owner, repo } = context.repo await github.repos.createCommitStatus({ owner, repo, - sha: '${{ github.event.workflow_run.head_sha }}', + sha: HEAD_SHA, context: CONTEXT_NAME, state: 'error', description: 'Failed to deploy. See logs.', @@ -247,13 +248,15 @@ jobs: - name: Check pull request state id: check-pr uses: actions/github-script@2b34a689ec86a68d8ab9478298f91d5401337b7d + env: + PR_NUMBER: ${{ env.PR_NUMBER }} with: script: | const { owner, repo } = context.repo const { data: pullRequest } = await github.pulls.get({ owner, repo, - pull_number: ${{ github.event.workflow_run.pull_requests[0].number }} + pull_number: process.env.PR_NUMBER }) core.setOutput('state', pullRequest.state) @@ -293,6 +296,7 @@ jobs: SOURCE_BLOB_URL: ${{ needs.prepare.outputs.source_blob_url }} CONTEXT_NAME: ${{ env.CONTEXT_NAME }} ACTIONS_RUN_LOG: ${{ env.ACTIONS_RUN_LOG }} + HEAD_SHA: ${{ env.HEAD_SHA }} with: script: | const { GITHUB_TOKEN, HEROKU_API_TOKEN } = process.env @@ -320,10 +324,8 @@ jobs: // instance to avoid versioning discrepancies. const octokit = getOctokit() - const actionsRunLog = 'https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}' - try { - const { PR_URL, SOURCE_BLOB_URL, CONTEXT_NAME, ACTIONS_RUN_LOG } = process.env + const { PR_URL, SOURCE_BLOB_URL, CONTEXT_NAME, ACTIONS_RUN_LOG, HEAD_SHA } = process.env const { owner, repo, pullNumber } = parsePrUrl(PR_URL) if (!owner || !repo || !pullNumber) { throw new Error(`'pullRequestUrl' input must match URL format 'https://github.com/github/(docs|docs-internal)/pull/123' but was '${PR_URL}'`) @@ -347,7 +349,7 @@ jobs: await github.repos.createCommitStatus({ owner, repo, - sha: '${{ github.event.workflow_run.head_sha }}', + sha: HEAD_SHA, context: CONTEXT_NAME, state: 'success', description: 'Successfully deployed! See logs.', @@ -396,14 +398,15 @@ jobs: env: CONTEXT_NAME: ${{ env.CONTEXT_NAME }} ACTIONS_RUN_LOG: ${{ env.ACTIONS_RUN_LOG }} + HEAD_SHA: ${{ env.HEAD_SHA }} with: script: | - const { CONTEXT_NAME, ACTIONS_RUN_LOG } = process.env + const { CONTEXT_NAME, ACTIONS_RUN_LOG, HEAD_SHA } = process.env const { owner, repo } = context.repo await github.repos.createCommitStatus({ owner, repo, - sha: '${{ github.event.workflow_run.head_sha }}', + sha: HEAD_SHA, context: CONTEXT_NAME, state: 'error', description: 'Failed to deploy. See logs.', diff --git a/script/early-access/clone-for-build.js b/script/early-access/clone-for-build.js index 0ae78c4ad8..5a2be19625 100755 --- a/script/early-access/clone-for-build.js +++ b/script/early-access/clone-for-build.js @@ -122,6 +122,9 @@ destinationDirNames.forEach((dirName) => { return } + // Ensure the base directory exists + fs.mkdirSync(path.join(process.cwd(), dirName), { recursive: true }) + // Move the directory from the cloned source to the destination fs.renameSync(sourceDir, destDir)