From fcf7952547076c1abd07118db4fdc1698c25608e Mon Sep 17 00:00:00 2001 From: per1234 Date: Sat, 25 Jun 2022 13:30:54 -0700 Subject: [PATCH 1/2] Refactor signing certificate handling in "Arduino IDE" workflow Previously, there was some code duplication of the complex code signing certificate handling commands, which made the related code more difficult to understand, maintain, and develop. The cause of this duplication is that there is a separate certificate for each operating system, each of which is stored in separate repository secrets, as well as a different certificate file extension for each OS. Since the secret names and file extensions are associated with the operating system, it is most logical to define them via attributes alongside the operating system definition in the job matrix configuration already used to generate the parallel job runs for native build on each OS. That done, the certificate handling commands are universal and the system can easily expand to additional host targets (e.g., Apple M1) as time goes on. --- .github/workflows/build.yml | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d50842822..16566f96e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -17,13 +17,22 @@ env: jobs: build: + name: build (${{ matrix.config.os }}) if: github.repository == 'arduino/arduino-ide' strategy: matrix: config: - os: windows-2019 + certificate-secret: WINDOWS_SIGNING_CERTIFICATE_PFX # Name of the secret that contains the certificate. + certificate-password-secret: WINDOWS_SIGNING_CERTIFICATE_PASSWORD # Name of the secret that contains the certificate password. + certificate-extension: pfx # File extension for the certificate. - os: ubuntu-18.04 # https://github.com/arduino/arduino-ide/issues/259 - os: macos-latest + # APPLE_SIGNING_CERTIFICATE_P12 secret was produced by following the procedure from: + # https://www.kencochrane.com/2020/08/01/build-and-sign-golang-binaries-for-macos-with-github-actions/#exporting-the-developer-certificate + certificate-secret: APPLE_SIGNING_CERTIFICATE_P12 + certificate-password-secret: KEYCHAIN_PASSWORD + certificate-extension: p12 runs-on: ${{ matrix.config.os }} timeout-minutes: 90 @@ -59,23 +68,14 @@ jobs: if [ $IS_FORK = true ]; then echo "Skipping the app signing: building from a fork." else - if [ "${{ runner.OS }}" = "macOS" ]; then - export CSC_LINK="${{ runner.temp }}/signing_certificate.p12" - # APPLE_SIGNING_CERTIFICATE_P12 secret was produced by following the procedure from: - # https://www.kencochrane.com/2020/08/01/build-and-sign-golang-binaries-for-macos-with-github-actions/#exporting-the-developer-certificate - echo "${{ secrets.APPLE_SIGNING_CERTIFICATE_P12 }}" | base64 --decode > "$CSC_LINK" - - export CSC_KEY_PASSWORD="${{ secrets.KEYCHAIN_PASSWORD }}" - - elif [ "${{ runner.OS }}" = "Windows" ]; then - export CSC_LINK="${{ runner.temp }}/signing_certificate.pfx" - npm config set msvs_version 2017 --global - echo "${{ secrets.WINDOWS_SIGNING_CERTIFICATE_PFX }}" | base64 --decode > "$CSC_LINK" - - export CSC_KEY_PASSWORD="${{ secrets.WINDOWS_SIGNING_CERTIFICATE_PASSWORD }}" - fi + export CSC_LINK="${{ runner.temp }}/signing_certificate.${{ matrix.config.certificate-extension }}" + echo "${{ secrets[matrix.config.certificate-secret] }}" | base64 --decode > "$CSC_LINK" + export CSC_KEY_PASSWORD="${{ secrets[matrix.config.certificate-password-secret] }}" fi + if [ "${{ runner.OS }}" = "Windows" ]; then + npm config set msvs_version 2017 --global + fi npx node-gyp install yarn --cwd ./electron/packager/ yarn --cwd ./electron/packager/ package From abf4aedf13f1bb06512cc054b346a3cd4539f03c Mon Sep 17 00:00:00 2001 From: per1234 Date: Sat, 25 Jun 2022 11:55:06 -0700 Subject: [PATCH 2/2] Enable "Arduino IDE" workflow use by contributors GitHub Actions workflows may require access to privileged information in order to perform certain operations. GitHub provides the capability for doing this via "repository secrets". For security reasons, repository secrets are only accessible to a GitHub Actions workflow run when it is triggered by an event from within the repository containing the secret. This means that a workflow which requires such secrets would fail when run in a fork (unless the fork owner was able to set up their own secrets with suitable values). In order to make the relevant components of the CI system friendly for use in forks by contributors validating their work in preparation for submitting a PR, when the operations that require access to a secret are supplemental, those operations should be configured to only run from branches of the parent repository. Due to its unfortunate monolithic design, in addition to operations useful to contributors, the "Arduino IDE" workflow contains several such supplemental operations: - Code signing - Publishing release artifacts to Arduino's server Some attempt was previously made to configure the workflow to skip these operations when run in forks, but that configuration was not done correctly. This made the workflow only usable by contributors with a deep enough understanding of GitHub Actions to be able to make the necessary modifications provisionally every time they needed to use the workflow. The average contributor would not be capable or willing to do this, which might result in PRs being submitted in a less validated state, increasing the burden on maintainers. The specific misconfigurations: **`build` job was conditional on the workflow running from `arduino/arduino-ide`** The job itself can run just fine in a fork, so there is no reason to impose this restriction. Since the time this conditional was added, some changes have been made to the GitHub Actions system which makes this sort of configuration unnecessary: - GitHub Actions is globally disabled in forks by default - Workflows which contain a `schedule` trigger (as is the case with this one) are individually disabled by default, requiring the repository owner to enable it specifically even after enabling GitHub Actions in general. This means this workflow will never run unexpectedly in a fork. The fork owner will always have intentionally enabled it. So this conditional can be removed completely. **Code signing was conditional on PR being submitted from a branch of the base repo** This would cause a spurious failure of the signing operation on PRs made within the contributor's fork when the signing secrets were not defined. The more appropriate condition of whether the signing secrets are defined or not is now used. The environment variable name has been updated accordingly. **`release` job was conditional on running from `arduino/arduino-ide`** The GitHub release creation step of this job can run in any repository. It is only the step that uploads to Arduino's AWS server which would only make sense to run from `arduino/arduino-ide`. So the conditional is moved to the AWS upload step, allowing contributors to test the workflow's release operation in their forks to validate related proposals. --- .github/workflows/build.yml | 10 +++++----- electron/build/scripts/notarize.js | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 16566f96e..c7e5758d3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -18,7 +18,6 @@ env: jobs: build: name: build (${{ matrix.config.os }}) - if: github.repository == 'arduino/arduino-ide' strategy: matrix: config: @@ -62,11 +61,11 @@ jobs: AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} IS_NIGHTLY: ${{ github.event_name == 'schedule' || (github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') }} IS_RELEASE: ${{ startsWith(github.ref, 'refs/tags/') }} - IS_FORK: ${{ github.event.pull_request.head.repo.fork == true }} + CAN_SIGN: ${{ secrets[matrix.config.certificate-secret] != '' }} run: | # See: https://www.electron.build/code-signing - if [ $IS_FORK = true ]; then - echo "Skipping the app signing: building from a fork." + if [ $CAN_SIGN = false ]; then + echo "Skipping the app signing: certificate not provided." else export CSC_LINK="${{ runner.temp }}/signing_certificate.${{ matrix.config.certificate-extension }}" echo "${{ secrets[matrix.config.certificate-secret] }}" | base64 --decode > "$CSC_LINK" @@ -188,7 +187,7 @@ jobs: release: needs: changelog - if: github.repository == 'arduino/arduino-ide' && startsWith(github.ref, 'refs/tags/') + if: startsWith(github.ref, 'refs/tags/') runs-on: ubuntu-latest steps: - name: Download [GitHub Actions] @@ -213,6 +212,7 @@ jobs: body: ${{ needs.changelog.outputs.BODY }} - name: Publish Release [S3] + if: github.repository == 'arduino/arduino-ide' uses: docker://plugins/s3 env: PLUGIN_SOURCE: '${{ env.JOB_TRANSFER_ARTIFACT }}/*' diff --git a/electron/build/scripts/notarize.js b/electron/build/scripts/notarize.js index c13111854..05a7b64b3 100644 --- a/electron/build/scripts/notarize.js +++ b/electron/build/scripts/notarize.js @@ -6,8 +6,8 @@ exports.default = async function notarizing(context) { console.log('Skipping notarization: not on CI.'); return; } - if (process.env.IS_FORK === 'true') { - console.log('Skipping the app notarization: building from a fork.'); + if (process.env.CAN_SIGN === 'false') { + console.log('Skipping the app notarization: certificate was not provided.'); return; } const { electronPlatformName, appOutDir } = context;