Skip to content

[ci] enforce zizmor checks, drop codecov #346

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

Merged
jameslamb merged 7 commits into from
Apr 25, 2026
Merged

[ci] enforce zizmor checks, drop codecov #346

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9a3eb61
[ci] enforce zizmor checks, drop codecov
jameslamb Apr 9, 2026
8be41c9
add pre-commit job
jameslamb Apr 9, 2026
3d23a68
default to contents:read
jameslamb Apr 24, 2026
7b9ecdb
pin zizmor to SHA, document in CONTRIBUTING.md
jameslamb Apr 24, 2026
5b77622
Add link and instructions for installing pre-commit
jayqi Apr 24, 2026
e003d62
Remove accidental duplication
jayqi Apr 24, 2026
2f9df4e
Apply suggestion from @jameslamb
jameslamb Apr 25, 2026
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
52 changes: 39 additions & 13 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,27 @@ on:
- cron: "0 0 * * 1"
workflow_dispatch:

# automatically cancel in-progress builds if another commit is pushed
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

# default to 0 permissions
# (job-level overrides add the minimal permissions needed)
permissions:
contents: none
Copy link
Copy Markdown
Collaborator

@jayqi jayqi Apr 24, 2026

Choose a reason for hiding this comment

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

This feels a little like overkill to me. Feels like setting to read explicitly here should be fine? Seems like every job is going to need it anyways, and as a baseline, a job with read shouldn't be in danger of doing something problematic.

Copy link
Copy Markdown
Collaborator Author

@jameslamb jameslamb Apr 24, 2026

Choose a reason for hiding this comment

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

The re-actors/alls-green job in this workflow, for example, does not need to be able to read the repo's contents, only the statuses of workflows.

As a general rule I still prefer this pattern of zeroing out the permissions for the workflow and having to opt in each job with only the permissions it needs, but I'm fine to switch these to read. This is a public repo so allowing contents: read would be totally fine.

Copy link
Copy Markdown
Collaborator Author

@jameslamb jameslamb Apr 24, 2026

Choose a reason for hiding this comment

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

Pushed that change in 3d23a68


jobs:
lint:
name: lint
runs-on: ubuntu-latest
steps:
- name: check out repository
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
fetch-depth: 1
persist-credentials: false
- uses: pre-commit/action@2c7b3805fd2a0fd8c1884dcaebf91fc102a13ecd #v3.0.1
test:
name: ${{ matrix.os }} (R ${{ matrix.r-version }})
runs-on: ${{ matrix.os }}
Expand All @@ -21,42 +41,48 @@ jobs:
r-version: 'release'
- os: macos-latest
r-version: 'release'
permissions:
contents: read
steps:
- name: checkout repository
uses: actions/checkout@v4
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
fetch-depth: 1
persist-credentials: false
- name: If local, apt update
if: ${{ (env.ACT || false) && (matrix.os == 'ubuntu-latest')}}
run: sudo apt update
- name: Install Tidy Ubuntu
if: matrix.os == 'ubuntu-latest'
run: sudo apt install -y tidy
- name: set up R
uses: r-lib/actions/setup-r@v2
uses: r-lib/actions/setup-r@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
with:
r-version: ${{ matrix.r-version }}
- uses: r-lib/actions/setup-pandoc@v2
- uses: r-lib/actions/setup-tinytex@v2
- uses: r-lib/actions/setup-r-dependencies@v2
- uses: r-lib/actions/setup-pandoc@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
- uses: r-lib/actions/setup-tinytex@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
- uses: r-lib/actions/setup-r-dependencies@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
with:
extra-packages: any::rcmdcheck, any::covr
needs: check
- uses: r-lib/actions/check-r-package@v2
- uses: r-lib/actions/check-r-package@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
with:
args: 'c("--as-cran")'
error-on: '"warning"'
check-dir: '"check"'
upload-results: '"true"'
upload-snapshots: '"true"'
- name: upload coverage
if: ${{ matrix.os == 'macos-latest' && matrix.r-version == 'release' }}
shell: Rscript {0}
run: covr::codecov(token = "${{ secrets.CODECOV_TOKEN }}")
# https://github.community/t/is-it-possible-to-require-all-github-actions-tasks-to-pass-without-enumerating-them/117957/4?u=graingert
all-successful:
if: always()
runs-on: ubuntu-latest
needs: [test]
needs:
- lint
- test
permissions:
statuses: read
steps:
- name: Note that all tests succeeded
run: echo "🎉"
- name: Note that all tests succeeded
uses: re-actors/alls-green@05ac9388f0aebcb5727afa17fcccfecd6f8ec5fe # v1.2.2
with:
jobs: ${{ toJSON(needs) }}
55 changes: 40 additions & 15 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,28 @@ on:
- 'release/**'
workflow_dispatch:

# automatically cancel in-progress builds if another commit is pushed
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

# default to 0 permissions
# (job-level overrides add the minimal permissions needed)
permissions:
contents: none

jobs:
version_check:
name: Confirm Whole Version Number
runs-on: ubuntu-latest
permission:
contents: read
steps:
- name: checkout repository
uses: actions/checkout@v4
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
fetch-depth: 1
persist-credentials: false
- name: Display Version Found
shell: bash
run: grep -E "Version:" $GITHUB_WORKSPACE/DESCRIPTION
Expand All @@ -39,56 +54,69 @@ jobs:
- os: macos-latest
r-version: 'devel'
- os: windows-latest
r-version: 'devel'
r-version: 'devel'
permissions:
contents: read
steps:
- name: checkout repository
uses: actions/checkout@v4
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
fetch-depth: 1
persist-credentials: false
- name: If local, apt update
if: ${{ (env.ACT || false) && (matrix.os == 'ubuntu-latest')}}
run: sudo apt update
- name: Install Tidy Ubuntu
if: matrix.os == 'ubuntu-latest'
run: sudo apt install -y tidy
- name: set up R
uses: r-lib/actions/setup-r@v2
uses: r-lib/actions/setup-r@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
with:
r-version: ${{ matrix.r-version }}
- uses: r-lib/actions/setup-pandoc@v2
- uses: r-lib/actions/setup-tinytex@v2
- uses: r-lib/actions/setup-pandoc@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
- uses: r-lib/actions/setup-tinytex@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
- run: tlmgr --version
- name: Install additional LaTeX Packages
run: |
tlmgr update --self
tlmgr update --all
tlmgr install titling framed inconsolata
tlmgr install collection-fontsrecommended
- uses: r-lib/actions/setup-r-dependencies@v2
- uses: r-lib/actions/setup-r-dependencies@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
with:
extra-packages: any::rcmdcheck
needs: check
- uses: r-lib/actions/check-r-package@v2
- uses: r-lib/actions/check-r-package@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
with:
args: 'c("--as-cran")'
error-on: '"warning"'
check-dir: '"check"'
upload-results: '"true"'
upload-snapshots: '"true"'
all-successful:
if: always()
runs-on: ubuntu-latest
needs: [version_check, devel_test]
needs:
- devel_test
- version_check
permissions:
statuses: read
steps:
- name: Note that all tests succeeded
run: echo "🎉"
uses: re-actors/alls-green@05ac9388f0aebcb5727afa17fcccfecd6f8ec5fe # v1.2.2
with:
jobs: ${{ toJSON(needs) }}
build-tarball:
runs-on: ubuntu-latest
needs: [version_check]
permissions:
contents: read
steps:
- name: checkout repository
uses: actions/checkout@v4
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
fetch-depth: 1
persist-credentials: false
- name: If local, apt update
if: ${{ (env.ACT || false)}}
run: sudo apt update
Expand All @@ -104,12 +132,9 @@ jobs:
TAR_NAME+=".tar.gz"
echo "TAR_NAME=$TAR_NAME" >> $GITHUB_ENV
- name: Upload tarball to run
uses: actions/upload-artifact@v4
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
with:
name: ${{env.TAR_NAME}}
path: ${{github.workspace}}/${{env.TAR_NAME}}
if-no-files-found: error
compression-level: 0 # already compressed



31 changes: 25 additions & 6 deletions .github/workflows/smoke-tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ on:
- cron: '0 0 * * 0'
workflow_dispatch:

# automatically cancel in-progress builds if another commit is pushed
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

# default to 0 permissions
# (job-level overrides add the minimal permissions needed)
permissions:
contents: none

jobs:
test:
name: smoke-tests
Expand Down Expand Up @@ -74,13 +84,16 @@ jobs:
- withr
- xfun
- yaml
permissions:
contents: read
steps:
- name: Checkout repository
uses: actions/checkout@v4
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
fetch-depth: 1
persist-credentials: false
- name: set up R
uses: r-lib/actions/setup-r@v2
uses: r-lib/actions/setup-r@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
with:
r-version: 'release'
- name: If local, apt update
Expand All @@ -89,16 +102,22 @@ jobs:
- name: Install Tidy
run: sudo apt install -y tidy
- name: Install Deps For Pkgnet & ${{ matrix.test_pkg }}
uses: r-lib/actions/setup-r-dependencies@v2
uses: r-lib/actions/setup-r-dependencies@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
with:
extra-packages: ${{ matrix.test_pkg }}, local::.
- name: run smoke test
shell: Rscript {0}
run: "pkgnet::CreatePackageReport(pkg_name='${{ matrix.test_pkg }}', report_path='${{ github.workspace }}/${{ matrix.test_pkg }}_report.html')"
# https://github.community/t/is-it-possible-to-require-all-github-actions-tasks-to-pass-without-enumerating-them/117957/4?u=graingert
all-successful:
if: always()
runs-on: ubuntu-latest
needs: [test]
needs:
- test
permissions:
statuses: read
steps:
- name: Note that all smoke tests succeeded
run: echo "🚫🔥==🐻👍🏼👍🏼"
- name: Note that all tests succeeded
uses: re-actors/alls-green@05ac9388f0aebcb5727afa17fcccfecd6f8ec5fe # v1.2.2
with:
jobs: ${{ toJSON(needs) }}
37 changes: 27 additions & 10 deletions .github/workflows/website.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,44 +7,61 @@ on:
- 'v*'
workflow_dispatch:

# automatically cancel in-progress builds if another commit is pushed
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

# default to 0 permissions
# (job-level overrides add the minimal permissions needed)
permissions:
contents: none

jobs:
build-website:
name: build website docs on website_docs_update branch
runs-on: macos-latest
permissions:
contents: read
steps:
- name: checkout repository
uses: actions/checkout@v4
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
fetch-depth: 0 # pull all tags and other history
fetch-depth: 1
persist-credentials: false
- name: Get Latest Version Tag
id: previoustag
uses: WyriHaximus/github-action-get-previous-tag@v1.4.0
uses: WyriHaximus/github-action-get-previous-tag@61819f33034117e6c686e6a31dba995a85afc9de # v2.0.0
with:
prefix: 'v'
- name: Git Checkout Branch From Latest Version Tag
env:
PREVIOUS_TAG: ${{ steps.previoustag.outputs.tag }}
run: |
git config --local user.name "$GITHUB_ACTOR"
git config --local user.email "$GITHUB_ACTOR@users.noreply.github.com"
git checkout -b website_docs_update ${{steps.previoustag.outputs.tag}}
git checkout -b website_docs_update "${PREVIOUS_TAG}"
- name: set up R
uses: r-lib/actions/setup-r@v2
uses: r-lib/actions/setup-r@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
with:
r-version: 'release'
- uses: r-lib/actions/setup-pandoc@v2
- uses: r-lib/actions/setup-tinytex@v2
- uses: r-lib/actions/setup-r-dependencies@v2
- uses: r-lib/actions/setup-pandoc@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
- uses: r-lib/actions/setup-tinytex@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
- uses: r-lib/actions/setup-r-dependencies@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2.11.4
with:
packages: pkgdown
- name: Build Site
run: pkgdown::build_site()
shell: Rscript {0}
- name: Save Site Docs Articfact
uses: actions/upload-artifact@v4
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
with:
name: "built_website_docs_${{steps.previoustag.outputs.tag}}"
path: ${{github.workspace}}/docs/
- name: Commit website doc changes (overwrite if existing)
env:
PREVIOUS_TAG: ${{ steps.previoustag.outputs.tag }}
run: |
git add docs/\*
git commit -m "Update website documentation to ${{steps.previoustag.outputs.tag}}" || echo "No changes to commit"
git commit -m "Update website documentation to ${PREVIOUS_TAG}" || echo "No changes to commit"
git push -f origin website_docs_update
6 changes: 6 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
repos:
- repo: https://github.com/zizmorcore/zizmor-pre-commit
rev: 'v1.23.1'
hooks:
- id: zizmor
Copy link
Copy Markdown
Collaborator

@jayqi jayqi Apr 24, 2026

Choose a reason for hiding this comment

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

Are we asking developers to install pre-commit so that they run zizmor on pre-commit? Otherwise, wouldn't it be better to just install and run the zizmor CLI, or to use zizmorcore/zizmore-action? This is adding pre-commit as a dependency in the chain.

Also the pre-commit package being installed here also isn't pinned to a commit hash.

Copy link
Copy Markdown
Collaborator Author

@jameslamb jameslamb Apr 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

It's convenient to be able to run zizmor locally so you don't have to iterate with pushes + CI runs, but definitely not required. If you're not touching the GitHub Actions configs (which would be the case for any outside contributor, I'm guessing, as they'd probably only touch R code), then you don't need to care about it.

I chose a general task runner like pre-commit for that instead of, say, a shell script running the zizmor CLI because I could imagine wanting more tools like this in the future. In {uptasticsearch} Austin and I use pre-commit to also run codespell (typos), shellcheck (shell code correctness/portability), and shfmt (shell code autoformatting): https://github.com/uptake/uptasticsearch/blob/main/.pre-commit-config.yaml

It's convenient to have those things bundled together, and pre-commit takes care of things like installing the tools on different platforms.

If you'd prefer not to have this here I'm fine to switch to the zizmor GitHub action, would you like me to do that?

Also the pre-commit package being installed here also isn't pinned to a commit hash.

The pre-commit GitHub Actions I've introduced here is:

- uses: pre-commit/action@2c7b3805fd2a0fd8c1884dcaebf91fc102a13ecd #v3.0.1

I'm unsure how tightly that controls the pre-commit Python package and its dependencies or how to pin those too.

Or did you mean by this comment that you'd want the zizmor-pre-commit here to be pinned to a commit instead of rev: 'v1.23.1'?

Copy link
Copy Markdown
Collaborator

@jayqi jayqi Apr 24, 2026

Choose a reason for hiding this comment

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

I don't have a super strong position on whether we use the tools directly or if we use pre-commit. If we do want to use pre-commit, it probably makes sense to document that in CONTRIBUTING.md so that it's more clear and standardized how to use it and what the expectations are? I'm okay with treating that as a separate PR though, but just want to point out that adopting pre-commit would make the most sense if we're intentionally using it as designed.

Or did you mean by this comment that you'd want the zizmor-pre-commit here to be pinned to a commit instead of rev: 'v1.23.1'?

Yes, I think we should pin zizmor-pre-commit in the pre-commit manifest here if we're generally looking to harden against supply chain attacks where GitHub releases are modified.

Copy link
Copy Markdown
Collaborator Author

@jameslamb jameslamb Apr 24, 2026

Choose a reason for hiding this comment

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

Ok, I've just pushed a commit pinning the pre-commit hook to a SHA and documenting how to use it in CONTRIBUTING.md.

7b9ecdb

Note that with that change, it'll now need to be manually updated to pull in new versions... pre-commit autoupdate replaces the commit SHA with a tag like v.1.24.1.

I haven't pinned the zizmor Python package to a SHA (it'd be a checksum of the wheel contents, not a git commit) because that package publishes different wheels for different architectures:

image

https://pypi.org/project/zizmor/#files

It's == pinned to a version in the hook: https://github.com/zizmorcore/zizmor-pre-commit/blob/a4727cbbcd26d7098e96b9cb738169b59711ae51/pyproject.toml#L6

I suspect it's possible to do with platform markers but would be pretty annoying to update. And I REALLY think that'd be overkill, zizmor is probably better hardened against supply chain attacks than 99.9% of other packages on PyPI.

If you want to make any changes to the CONTRIBUTING.md or other details here, totally fine with me if you just want to push them directly to the branch. I'm interested in reducing our attack surface here and using zizmor to ensure we maintain those improvements, and don't have strongly-held opinions about how that's accomplished.

Copy link
Copy Markdown
Collaborator

@jayqi jayqi Apr 24, 2026

Choose a reason for hiding this comment

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

I think it's fine for PyPI packages to be pinned to versions and not SHA hashes. I'm pretty sure PyPI packages are not mutable. Once you've published a version, you can't publish a new distribution over that version number. The main thing is that anything that installs directly from a GitHub repository needs to be hardened, because GitHub does not do anything like that.

Copy link
Copy Markdown
Collaborator Author

@jameslamb jameslamb Apr 25, 2026

Choose a reason for hiding this comment

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

Ok thanks, yes I agree we can feel confident about not pinning the zizmor Python package any further.

Everything else I'm about to write is just for your interest, and shouldn't affect this PR at all.

Once you've published a version, you can't publish a new distribution over that version number.

There are a couple ways this is not technically true:

  1. a pin like ==1.24.1 would also match the version 1.24.1.0, which could be uploaded later
  2. PyPI doesn't prevent you from uploading more files to an existing release, including an old one... for example, you could publish an sdist of zizmor 1.24.1 and then much later upload a new wheel with that same version
  3. PyPI allows deletions of individual files, which can work like that but in reverse... if you've uploaded only wheels, you could upload an sdist and then delete the wheels (so installers would now be pulling in the sdist)

It's for reasons like this that pinning to a SHA that's a checksum of file contents is safer than pinning to version numbers.

There's more discussion about this here:

But still, anyway, the current state of this PR is more than enough for our purposes here and a net improvement in this repo's security posture.

1 change: 0 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
[![CRAN\_Download\_Badge](https://cranlogs.r-pkg.org/badges/grand-total/pkgnet)](https://cran.r-project.org/package=pkgnet)
[![GitHub Actions Build Status](https://github.com/uptake/pkgnet/actions/workflows/ci.yml/badge.svg?branch=main)](https://github.com/uptake/pkgnet/actions/workflows/ci.yml?query=branch%3Amain)
[![Appveyor Build status](https://ci.appveyor.com/api/projects/status/github/uptake/pkgnet?branch=main&svg=true)](https://ci.appveyor.com/project/jameslamb/pkgnet)
[![codecov](https://codecov.io/gh/uptake/pkgnet/branch/main/graph/badge.svg)](https://app.codecov.io/gh/uptake/pkgnet)
[![Lifecycle badge](https://lifecycle.r-lib.org/articles/figures/lifecycle-stable.svg)](https://lifecycle.r-lib.org/articles/stages.html)

## Introduction
Expand Down
Loading