Skip to content

Commit

Permalink
ci: Switch pull_request_target to pull_request (#66)
Browse files Browse the repository at this point in the history
With this change, GitHub will start to enforce "require approval"
settings for the repo, which `pull_request_target` allows PR authors to
get around.

To stop using `pull_request_target`, we also have to stop using `vars`,
which are not available on `pull_request` triggers. Instead, we use the
"environments" trick used by Shaka Packager's workflows before `vars`
was introduced to GitHub Actions. This is a safer system.
  • Loading branch information
joeyparrish authored Jan 17, 2025
1 parent 0f6154f commit e707575
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 24 deletions.
44 changes: 29 additions & 15 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,30 @@ on:
ref:
required: true
type: string
# If true, start a debug SSH server on failures.
debug:
required: false
type: boolean
default: false
# If true, enable self-hosted runners in the build matrix.
self_hosted:
required: false
type: boolean
default: false

# Runs on manual trigger.
workflow_dispatch:
inputs:
# If true, start a debug SSH server on failures.
debug:
required: false
type: boolean
default: false
# If true, enable self-hosted runners in the build matrix.
self_hosted:
required: false
type: boolean
default: false


# NOTE: The versions of the software we build are stored in versions.txt.
Expand All @@ -41,11 +62,9 @@ defaults:
shell: bash

jobs:
# Configure the build matrix based on repo variables. The list of objects in
# the build matrix contents can't be changed by conditionals, but it can be
# computed by another job and deserialized. This uses
# vars.ENABLE_SELF_HOSTED to determine the build matrix, based on the
# metadata in build-matrix.json.
# Configure the build matrix based on inputs. The list of objects in the
# build matrix contents can't be changed by conditionals, but it can be
# computed by another job and deserialized.
matrix_config:
runs-on: ubuntu-latest
outputs:
Expand All @@ -62,11 +81,11 @@ jobs:
shell: node {0}
run: |
const fs = require('fs');
const enableDebug = "${{ vars.ENABLE_DEBUG }}" != '';
const enableSelfHosted = "${{ vars.ENABLE_SELF_HOSTED }}" != '';
const enableDebug = ${{ inputs.debug }};
const enableSelfHosted = ${{ inputs.self_hosted }};
// Use ENABLE_SELF_HOSTED to decide what the build matrix below
// should include.
// Use enableSelfHosted to decide what the build matrix below should
// include.
const buildMatrix = JSON.parse(fs.readFileSync("${{ github.workspace }}/repo-src/build-matrix.json"));
const {hosted, selfHosted} = buildMatrix;
const matrix = enableSelfHosted ? hosted.concat(selfHosted) : hosted;
Expand All @@ -76,11 +95,6 @@ jobs:
process.env['GITHUB_OUTPUT'],
`MATRIX=${ JSON.stringify(matrix) }\n`);
// Output the debug flag directly.
fs.appendFileSync(
process.env['GITHUB_OUTPUT'],
`ENABLE_DEBUG=${ enableDebug }\n`);
// Log the outputs, for the sake of debugging this script.
console.log({enableDebug, enableSelfHosted, matrix});
Expand Down Expand Up @@ -212,4 +226,4 @@ jobs:
uses: mxschmitt/[email protected]
with:
limit-access-to-actor: true
if: failure() && vars.ENABLE_DEBUG != ''
if: failure() && inputs.debug
52 changes: 52 additions & 0 deletions .github/workflows/settings.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Copyright 2022 Google LLC
#
# Use of this source code is governed by a BSD-style
# license that can be found in the LICENSE file or at
# https://developers.google.com/open-source/licenses/bsd

# A reusable workflow to extract settings from a repository.
# To enable a setting, create a "GitHub Environment" with the same name.
#
# This enables per-repo settings that aren't copied to a fork. This is better
# than "vars" or "secrets", since those would require the use of
# `pull_request_target` instead of `pull_request` triggers, which come with
# additional risks such as the bypassing of "require approval" rules for
# workflows.
#
# Without a setting for flags like "self_hosted", test workflows for a fork
# would time out waiting for self-hosted runners that the fork doesn't have.
name: Settings

# Runs when called from another workflow.
on:
workflow_call:
outputs:
self_hosted:
description: "Enable jobs requiring a self-hosted runner."
value: ${{ jobs.settings.outputs.self_hosted }}
debug:
description: "Enable SSH debugging when a workflow fails."
value: ${{ jobs.settings.outputs.debug }}

jobs:
settings:
runs-on: ubuntu-latest
outputs:
self_hosted: ${{ steps.settings.outputs.self_hosted }}
debug: ${{ steps.settings.outputs.debug }}
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
steps:
- id: settings
run: |
environments=$(gh api /repos/${{ github.repository }}/environments)
for name in self_hosted debug; do
exists=$(echo $environments | jq ".environments[] | select(.name == \"$name\")")
if [[ "$exists" != "" ]]; then
echo "$name=true" >> $GITHUB_OUTPUT
echo "\"$name\" enabled."
else
echo "$name=" >> $GITHUB_OUTPUT
echo "\"$name\" disabled."
fi
done
22 changes: 13 additions & 9 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@
name: Test

# Runs when a PR is uploaded or revised. Builds ffmpeg and ffprobe on all OS &
# CPU combinations. Uses pull_request_target rather than pull_request for
# access to configuration variables. Since no secrets are accessed or passed
# to the build workflow, this is safe. Only the release workflow uses secrets.
# CPU combinations.
on:
pull_request_target:
pull_request:
types: [opened, synchronize, reopened]

# If another instance of this workflow is started for the same PR, cancel the
Expand All @@ -30,14 +28,20 @@ concurrency:
group: ${{ github.workflow }}-${{ github.event.number }}
cancel-in-progress: true

# NOTE: Set the repository variable ENABLE_DEBUG to enable debugging via tmate
# on failure.
# NOTE: Set the repository variable ENABLE_SELF_HOSTED to enable self-hosted
# runners such as linux-arm64. This is set on the official repo, but forks
# will have to opt in after setting up their own self-hosted runners.
# NOTE: Create an environment called "debug" to enable debugging via tmate on
# failure.
# NOTE: Create an environment called "self_hosted" to enable self-hosted
# runners from build-matrix.json.

jobs:
settings:
name: Settings
uses: ./.github/workflows/settings.yaml

build:
needs: settings
uses: ./.github/workflows/build.yaml
with:
ref: refs/pull/${{ github.event.number }}/merge
self_hosted: ${{ needs.settings.outputs.self_hosted != '' }}
debug: ${{ needs.settings.outputs.debug != '' }}

0 comments on commit e707575

Please sign in to comment.