Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check the repository owner in CI tasks #1280

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Check the repository owner in CI tasks #1280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
032894a
CI: Test the repository owner before doing things involving secrets.
sauclovian-g Jan 30, 2025
e25928b
Add more comments.
sauclovian-g Jan 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/workflows/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,30 @@ conventions when picking with operating systems to test:
developer tools that we need, such as `llvm-link` and `llvm-as`.
* We do not currently run MIR-related tests on Windows, as it is not
straightforward to install `mir-json` on Windows.

Other notes:

* Workflow steps that depend on secrets (signing keys, access tokens,
etc.) or that do things that only make sense in the context of the
main repository need to be restricted so they aren't run in other
contexts.

It is apparently impossible to restrict scheduled workflows to the
original/parent repository; they will always also run in forks. See
[this discussion](https://github.com/orgs/community/discussions/16109).

Therefore while it's sometimes necessary/desirable to skip steps
when working on a pull request, it's not sufficient for scheduled
workflows. In particular, the condition
```github.event.pull_request.head.repo.fork == false``` isn't
sufficient.

Steps that require secrets should check the repository owner
(```github.repository_owner == 'GaloisInc'```). Checking the
repository identity (```github.repository == 'GaloisInc/crucible'```)
is also possible but less maintainable/robust/etc.

It isn't absolutely clear whether it's necessary to _also_ check the
pull request fork flag. For the time being we do so, partly to avoid
unforeseen complications and partly because this is what some other
projects apparently do.
3 changes: 3 additions & 0 deletions .github/workflows/crucible-go-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ jobs:
with:
nix_path: nixpkgs=channel:21.11
install_url: https://releases.nixos.org/nix/nix-2.10.3/install
# This token usage is to avoid GH rate-limiting; see commit
# 1696558326a8c57e1a9f0848aaaa1b8440294954. Apparently it'll
# work for anyone.
extra_nix_config: |
access-tokens = github.com=${{ secrets.GITHUB_TOKEN }}

Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/crucible-jvm-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ jobs:
with:
nix_path: nixpkgs=channel:21.11
install_url: https://releases.nixos.org/nix/nix-2.10.3/install
# This token usage is to avoid GH rate-limiting; see commit
# 1696558326a8c57e1a9f0848aaaa1b8440294954. Apparently it'll
# work for anyone.
extra_nix_config: |
access-tokens = github.com=${{ secrets.GITHUB_TOKEN }}
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/crucible-wasm-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ jobs:
with:
nix_path: nixpkgs=channel:21.11
install_url: https://releases.nixos.org/nix/nix-2.10.3/install
# This token usage is to avoid GH rate-limiting; see commit
# 1696558326a8c57e1a9f0848aaaa1b8440294954. Apparently it'll
# work for anyone.
extra_nix_config: |
access-tokens = github.com=${{ secrets.GITHUB_TOKEN }}

Expand Down
15 changes: 10 additions & 5 deletions .github/workflows/crux-llvm-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ jobs:
with:
nix_path: nixpkgs=channel:21.11
install_url: https://releases.nixos.org/nix/nix-2.10.3/install
# This token usage is to avoid GH rate-limiting; see commit
# 1696558326a8c57e1a9f0848aaaa1b8440294954. Apparently it'll
# work for anyone.
extra_nix_config: |
access-tokens = github.com=${{ secrets.GITHUB_TOKEN }}

Expand Down Expand Up @@ -223,7 +226,7 @@ jobs:
NAME="crux-llvm-${{ needs.config.outputs.crux-llvm-version }}-${{ matrix.os }}-${{ runner.arch }}"
echo "NAME=$NAME" >> $GITHUB_ENV
.github/ci.sh bundle_crux_llvm_files
if: github.repository == 'GaloisInc/crucible'
if: github.event.pull_request.head.repo.fork == false && github.repository_owner == 'GaloisInc'
env:
OS_TAG: ${{ matrix.os }}
ARCH_TAG: ${{ runner.arch }}
Expand All @@ -233,8 +236,10 @@ jobs:
# The SIGNING_PASSPHRASE and SIGNING_KEY secrets are only available on
# jobs run from the main repo, and as a result, they won't work when
# run from a fork. Signing binaries isn't essential to the rest of the
# workflow, so it is safe to skip this step on forks.
if: github.event.pull_request.head.repo.fork == false
# workflow, so it is safe to skip this step on forks. Note that it
# isn't sufficient to test the pull request info because this job is
# also scheduled. See notes in README.md.
if: github.event.pull_request.head.repo.fork == false && github.repository_owner == 'GaloisInc'
shell: bash
env:
SIGNING_PASSPHRASE: ${{ secrets.SIGNING_PASSPHRASE }}
Expand All @@ -243,7 +248,7 @@ jobs:
.github/ci.sh sign "${NAME}.tar.gz"

- uses: actions/upload-artifact@v4
if: github.repository == 'GaloisInc/crucible'
if: github.event.pull_request.head.repo.fork == false && github.repository_owner == 'GaloisInc'
with:
path: crux-llvm-*.tar.gz*
name: crux-llvm-${{ matrix.os }}-${{ matrix.ghc }}
Expand All @@ -260,7 +265,7 @@ jobs:
build-push-image:
runs-on: ubuntu-22.04
needs: [config]
if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || needs.config.outputs.release == 'true'
if: (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || needs.config.outputs.release == 'true') && github.repository_owner == 'GaloisInc'
Comment on lines 265 to +268
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build-push-image job in crux-mir-build.yml should be guarded in a similar way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, not sure how I missed that. I've added it

strategy:
fail-fast: false
matrix:
Expand Down
15 changes: 10 additions & 5 deletions .github/workflows/crux-mir-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ jobs:
with:
nix_path: nixpkgs=channel:21.11
install_url: https://releases.nixos.org/nix/nix-2.10.3/install
# This token usage is to avoid GH rate-limiting; see commit
# 1696558326a8c57e1a9f0848aaaa1b8440294954. Apparently it'll
# work for anyone.
extra_nix_config: |
access-tokens = github.com=${{ secrets.GITHUB_TOKEN }}

Expand Down Expand Up @@ -185,7 +188,7 @@ jobs:
NAME="crux-mir-${{ needs.config.outputs.crux-mir-version }}-${{ matrix.os }}-${{ runner.arch }}"
echo "NAME=$NAME" >> $GITHUB_ENV
.github/ci.sh bundle_crux_mir_files
if: github.repository == 'GaloisInc/crucible'
if: github.event.pull_request.head.repo.fork == false && github.repository_owner == 'GaloisInc'
env:
OS_TAG: ${{ matrix.os }}
ARCH_TAG: ${{ runner.arch }}
Expand All @@ -195,8 +198,10 @@ jobs:
# The SIGNING_PASSPHRASE and SIGNING_KEY secrets are only available on
# jobs run from the main repo, and as a result, they won't work when
# run from a fork. Signing binaries isn't essential to the rest of the
# workflow, so it is safe to skip this step on forks.
if: github.event.pull_request.head.repo.fork == false
# workflow, so it is safe to skip this step on forks. Note that it
# isn't sufficient to test the pull request info because this job is
# also scheduled. See notes in README.md.
if: github.event.pull_request.head.repo.fork == false && github.repository_owner == 'GaloisInc'
shell: bash
env:
SIGNING_PASSPHRASE: ${{ secrets.SIGNING_PASSPHRASE }}
Expand All @@ -205,7 +210,7 @@ jobs:
.github/ci.sh sign "${NAME}.tar.gz"

- uses: actions/upload-artifact@v4
if: github.repository == 'GaloisInc/crucible'
if: github.event.pull_request.head.repo.fork == false && github.repository_owner == 'GaloisInc'
with:
path: crux-mir-*.tar.gz*
name: crux-mir-${{ matrix.os }}-${{ matrix.ghc }}
Expand All @@ -222,7 +227,7 @@ jobs:
build-push-image:
runs-on: ubuntu-22.04
needs: [config]
if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || needs.config.outputs.release == 'true'
if: (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || needs.config.outputs.release == 'true') && github.repository_owner == 'GaloisInc'
strategy:
fail-fast: false
matrix:
Expand Down
Loading