ci: harden the GitHub Actions workflows - #92
Merged
Conversation
The checkout/pnpm/setup-node/install block was copy-pasted across four jobs, with Node 22 hardcoded in three of them, and every install carried a HUSKY=0 prefix although the project has no husky and no prepare script. Extract the block into a local composite action, and make .nvmrc the single source of truth for the default Node version. The build-test matrix keeps its per-leg override. Refs certinia#91
A floating major tag runs whatever the owner last pushed to it, in a job that can reach the GITHUB_TOKEN. Pin each reference to the commit behind the tag it was already tracking, and record the version in a comment so a reader still knows what is installed. Dependabot already watches the github-actions ecosystem and updates a pinned SHA together with its comment. Refs certinia#91
The workflows ran with whatever GITHUB_TOKEN the repository default grants. ci.yml declared no permissions at all, so its three jobs got the default scope on every pull request, forks included. Deny everything at the top of each workflow and let each job ask for the scopes it reads. Every checkout now drops the token from .git/config after the clone, so no later step can reach it. Add the concurrency group and the per-job timeouts that only codeql.yml had, and cut the CodeQL timeout from 360 minutes to 30 - build-mode: none on a repository this size finishes in minutes, so the longer limit only delays the signal when a run wedges.
`run: ${{ matrix.command }}` pasted the expression into the script GitHub
generates. The values are workflow-authored, so this was not exploitable,
but it is the template-injection shape and a linter cannot tell the two
apart.
The matrix now carries the script name and the step passes it through the
environment, where the shell treats it as data.
The publish job held id-token: write at workflow scope, ran with no environment gate and never compared package.json with the tag it was publishing. A release cut from the wrong tag shipped silently. Move the OIDC and read scopes onto the job, put the job behind the npm-publish environment, and fail before the registry call when the tag and the manifest disagree. The tag reaches the shell through the environment, not through an interpolation. Add a concurrency group that queues rather than cancels: a publish that is stopped part-way is worse than one that waits.
The hardening in the previous commits is a snapshot. Without a check it decays: the next workflow to be added starts from whatever gets copied. actionlint catches the syntax and expression mistakes that only show up as a failed run. zizmor audits the security posture and reports through code scanning, which this repository already uses for CodeQL. Its config makes hash pinning a rule, so a floating tag fails the check instead of waiting for a reviewer to notice. actionlint comes from the release tarball, verified against a pinned checksum, rather than from an anonymous Docker Hub pull.
A pinned SHA does not follow its action, so pinning without an updater trades a re-tagging risk for a stale-dependency one. The github-actions ecosystem reads .github/workflows from '/' but not the composite actions, so those are listed as well. Hold every update for seven days before proposing it. A compromised release is usually yanked inside that window, and it clears the two zizmor findings the new lint job reports. Add the conventional-commit prefix the rest of the history uses, and cap the open pull requests per ecosystem.
Only unchanged files are skipped, so the result is the same. The cache goes under node_modules, which is already ignored, rather than dropping an .eslintcache in the project root.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
The composite action saved four repetitions of a four-line block. Against that it added a file to review, its own pin to keep fresh, an extra dependabot directory entry, and a level of indirection between a red job and the step that failed. The block is small and it is stable, so the duplication is cheaper than the abstraction. `.nvmrc` stays. It was the other half of the same commit and it does earn its keep: the Node version was written in three places. Refs certinia#91
This reverts commit bd3013cd08c1a92ff2df0e1dd9c78a3d8b7c86af. It does not belong on this branch. The request was to harden the GitHub Actions workflows; an eslint cache is neither a workflow nor a security change, and it is the only edit here that touched package.json. The cache also does nothing in CI, which is where this branch aims. Every job starts on a clean runner with no .eslintcache to restore, so the first and only run is always a full one.
As added, the job could not fail. zizmor-action defaults `advanced-security` to true, which runs zizmor with `--format=sarif`, and zizmor suppresses exit codes 11-14 in SARIF mode on purpose: SARIF consumers expect results in the document, not in the exit status. So a floating tag became a code-scanning alert on a green job. The hash-pin policy in .github/zizmor.yml was advisory, which is the one thing it was not meant to be. Measured on zizmor 1.29.0, one unpinned action, same config: default format exit 14 --format=sarif exit 0 `advanced-security: false` restores the console format and the exit code. `annotations: true` puts each finding on the pull request diff instead of the security tab, and needs advanced-security off because the two are mutually exclusive. Console format also uploads no SARIF, so the job no longer needs `security-events: write`. Refs certinia#91
Partially reverts c7b7355. The version check it added stays; the environment goes. It gates nothing. GitHub creates a referenced environment implicitly with no protection rules, so `npm-publish` reads as a release gate and is an empty one. The approval rule that would make it real lives in repository settings, not in this file. It also carries a risk that cannot be tested from here. A job with an `environment` gets an extra claim in its OIDC token, and its `sub` claim changes from the release tag to the environment. npm compares that token against the trusted publisher record, which 33de047 set up as org, repository and workflow with no environment. npm documents the environment field as optional but does not say whether an empty field accepts a token that names one. The only test is a real release, and it fails at the last step with the GitHub release already published. Adding the environment properly is four steps that belong together: create it, add the approval rule, set the name on the npm record, then add this line back. Refs certinia#91
This is the one job whose output reaches users, and a restored cache is writable by any run on the default branch. A poisoned pnpm store would reach the published tarball. Against that, a cold install costs seconds, once per release. Both cache switches are set, not one. `cache` is opt-in, but `package-manager-cache` defaults to true and auto-caches whenever `packageManager` names npm. Setting it here keeps the guarantee from resting on that field staying `pnpm`. Fixing it in the workflow rather than ignoring the finding in .github/zizmor.yml also keeps the alarm live: add a cache back and cache-poisoning fires again. The version check moves ahead of the install while the install is being edited. It is now the slow step, and there is no reason to pay for a cold one before finding out the tag is wrong. Refs certinia#91
The group was `publish-${{ github.ref }}`, and on a release event the ref
is the release tag. Two releases therefore land in two different groups
and publish at the same time, which is what the comment above the block
says must not happen. The group only ever matched a re-run of the same
tag.
A literal group name serialises them.
Refs certinia#91
Partially reverts 6496c3f. `open-pull-requests-limit: 5` is the default for version updates. `commit-message.prefix: 'chore(deps)'` is close to what dependabot already does. It detects conventional commits from the repository history and prefixes with `chore(deps)` on its own. Writing it out adds a line to maintain and changes nothing. Neither key was asked for, and both hid the two settings on this branch that are real: `cooldown` and `groups`. Refs certinia#91
b369cfc rewrote the trailing comment on `pull-requests: write` into two lines above it. The permission itself did not change, so no part of the hardening made the old comment wrong. The fork-PR caveat it added is true and worth knowing: a pull request from a fork gets a read-only token, so the summary comment is skipped there while the check still runs. That belongs in the pull request description, not in a comment on a line this branch does not touch. Refs certinia#91
Both workflows filtered their pull_request trigger to main and release/**, and CodeQL also skipped a pull request that touched only markdown. A required check that a pull request can skip never reports, so the pull request waits for it forever. 2488157 removed the same branch filter from ci.yml, after a pull request stacked on another feature branch merged with no checks at all. The push trigger keeps its filters and the weekly schedule keeps the CodeQL baseline; only the pull request path widens.
husky has never been in this repository, so the entry excluded nothing.
Provenance attaches a signed statement of the repository, commit and workflow run that built the tarball, which anyone can verify with `npm audit signatures`. It proves the published code came from this source and not from a machine holding a token. Both preconditions already hold: the job grants `id-token: write` for trusted publishing, and package.json carries the `repository` field that npm checks against the repository it builds from.
lcottercertinia
approved these changes
Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #91.
The four workflows worked, but they were written for function rather than for
supply-chain safety. This branch closes the gaps, and adds two linters so the
result cannot quietly rot.
What changes
Least privilege.
ci.ymlgranted nopermissionsat any level, so its threejobs ran with the repository default token on every pull request, including one
from a fork. Every workflow now grants nothing at workflow scope, and each job
asks for what it reads. Every
checkoutsetspersist-credentials: false, sothe token is no longer left in
.git/configfor later steps.Pinning. All thirteen action references move from floating major tags to
commit SHAs, each with a
# vX.Y.Zcomment so dependabot can still update it.CodeQL had already filed three alerts for this on #69.
Limits. Every job gains a
timeout-minutes; CodeQL drops from the starterdefault of 360 to 30.
ci.yml,dependency-review.ymland the newworkflow-lint.ymlgain concurrency groups that cancel superseded pull requestruns and leave
mainalone.Template-injection shape. The build matrix passed
${{ matrix.command }}straight into a shell. The values are workflow-authored, so it was not
exploitable, but a linter cannot tell that. The matrix now carries a script name
and the step runs
pnpm run "$TASK_SCRIPT"throughenv.The release job. A new step compares the release tag with
package.jsonbefore the install and fails on a mismatch. Caching is off, because this is the
one job whose output reaches users and a restored cache is writable by any run
on the default branch. The concurrency group is now literal, so two releases
serialise instead of running together. It also publishes with
--provenance, soeach tarball carries a signed statement of the repository, commit and workflow
run that built it, verifiable with
npm audit signatures. Both preconditionsalready held: the job grants
id-token: write, andpackage.jsonnames therepository npm checks against.
Trigger filters.
codeql.ymlanddependency-review.ymlfiltered theirpull_requesttrigger tomainandrelease/**, and CodeQL also skipped amarkdown-only pull request. A required check that a pull request can skip stays
pending forever — the defect 2488157 fixed for
ci.yml. Both now run on everypull request. Push filters and the weekly CodeQL baseline are unchanged.
Two new linters in
workflow-lint.yml: actionlint for correctness, zizmorfor the security shapes above.
.github/zizmor.ymlsetsunpinned-usestohash-pin, which makes the pinning self-enforcing rather than a review commentsomebody must remember to make.
Dependabot now watches the actions as well as npm, groups its pull requests,
and waits seven days before adopting a release.
Housekeeping.
.nvmrcreplaces the Node version written in three places, andthe dead
HUSKY=0prefix comes off the four installs — husky has never been inthis repository, so the
.husky/line in.npmignoregoes with it.Review found one real defect, in my own work
The zizmor job as first written could not fail.
zizmor-actiondefaultsadvanced-securityto true, which runs zizmor with--format=sarif, and zizmorsuppresses exit codes 11-14 in SARIF mode on purpose. A floating tag would have
become a code-scanning alert on a green job, so the hash-pin policy was
advisory — the one thing it was not meant to be.
Measured on zizmor 1.29.0, one unpinned action, same config: default format
exits 14,
--format=sarifexits 0. Fixed in c3dc660 withadvanced-security: falseandannotations: true.Why the history adds and then removes three things
Three commits on this branch are reverted by later commits on the same branch,
rather than rebased away. The earlier commits are pushed, and this repository
does not rewrite pushed history. The reasoning is worth keeping in any case:
setup-node-pnpmcomposite action saved four repetitions of afour-line block, and cost a file to review, its own pin to keep fresh, an
extra dependabot entry, and a level of indirection between a red job and the
step that failed.
.nvmrc, added in the same commit, stays.starts on a clean runner with no
.eslintcacheto restore.environment: npm-publishis explained below.No publish environment.
environment: npm-publishwas added and thenremoved. A referenced environment is created with no protection rules, so it
reads as a release gate and is an empty one, and it changes the OIDC token — the
submoves from the release tag to the environment name, which the trustedpublisher record does not name. Maintainers want no manual approval on a
release, so the record and the workflow now agree.
Needs a maintainer before or after merge
1. Required checks.
codeql.ymlanddependency-review.ymlno longer filtertheir
pull_requesttrigger by branch, and CodeQL no longer skips amarkdown-only pull request, so both are safe to make required. Branch protection
is not readable without admin, so somebody must set that.
Workflow Lintisequally safe to require: it carries no path filter for the same reason.
2.
publish.ymlcannot be exercised without cutting a release. It is coveredhere by actionlint, zizmor and an offline run of the version-check script only.
The OIDC path has never run either: trusted publishing landed in 33de047 on
2026-07-14, and the only release, 1.0.0, went out on 2026-03-20 with a token. The
next release is the first to use it. pnpm 10.30.2, the pinned version, is a
version that is known to work; pnpm 11 is known to break it, so hold the pin.
Worth checking after that release: whether the attestation actually landed.
Noted in passing, deliberately not fixed
.npmignoremay be redundant in full, sincepackage.jsonalready has afilesallowlist. Only its dead.husky/line is removed here.publish.ymlrunspnpm run build, andprepublishOnlyruns it again, sotscruns twice per release. Kept: the hook stops a manual publish shipping astale
dist/, and the explicit step makes a build failure its own red step.actionslanguage now overlaps zizmor. Both are worth keeping fornow — they find different things — but it is a duplication to watch.
env, not ausesref, sodependabot will not bump it. It needs a manual update. The alternative was
docker://rhysd/actionlint, which adds an anonymous Docker Hub pull to CI.dependency-review.ymlkeepspull-requests: write. On a fork pull requestthe token is read-only, so the summary comment is skipped there while the
check still runs.
Verification
Locally, on the final tree: actionlint 1.7.12 exit 0; zizmor 1.29.0 exit 0 with
--config .github/zizmor.yml;pnpm run lint,pnpm run build, 232 tests in12 suites, and 6 eval cases all pass.
No
CHANGELOG.mdentry: every CI-only commit in this repository's history hasleft it alone, and nothing here is visible to a user of the published package.