Skip to content

ci: pin actions to commit SHAs and gate the release job - #158

Closed
dinwwwh wants to merge 1 commit into
mainfrom
claude/quirky-wozniak-w0y81u
Closed

dinwwwh wants to merge 1 commit into
mainfrom
claude/quirky-wozniak-w0y81u

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Oct 13, 2026

Copy link
Copy Markdown
Member

Fixes two low-severity supply-chain gaps in CI. Library code under packages/ is not touched.

  1. Every action was pinned to a floating major tag, including the actions that run inside the privileged release job (contents: write + id-token: write, npm trusted publishing) and the daily sponsors-sync job (contents: write, pushes to main).
  2. The release job could be dispatched from any branch, and it had no environment, so the npm trusted publisher could not be bound to one.

Changes

Pin every uses: to a commit SHA. Each pin carries a # vX.Y.Z comment and stays on its current major. I resolved every SHA with git ls-remote --tags (peeled ^{} commit for annotated tags) and checked each pin against that output.

Action Pin
actions/checkout (v7) 3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
actions/checkout (v6, pullfrog.yml) d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0
pnpm/action-setup ea17c68df8912ef543352723c149a84f56e3d413 # v6.1.0
CodSpeedHQ/action f22792bfac16f3e14eb9fbea76f4a48e9cc22b93 # v4.19.1
codecov/codecov-action 303a32d7a59b442fa8d48b6a1cc6825c09c847a5 # v7.1.1
pullfrog/pullfrog ad4283a862b8377cb525a0f5bf30f17c023b18ea # v0.1.98

Each pin is the commit its floating major tag points at today, with one exception. The pnpm/action-setup v6 tag still points at v6.0.10. v6.1.0 is the newest release in v6: a maintainer-signed tag from 2026-09-05 whose one commit adds pnpm v12 support. This repo uses pnpm v12 (devEngines.packageManager: ^12.4.1), so I pinned v6.1.0. CI on this PR exercises it.

Dependabot. Added a github-actions entry (weekly) so the SHA pins and their version comments stay current. It has a 7-day cooldown, so a hijacked release has time to be caught before Dependabot proposes it.

sponsors-sync. Replaced EndBug/add-and-commit with a plain run: git step, so the write token no longer reaches a third-party action. The step commits and pushes only when something is staged. The author/committer (GitHub Actions <41898282+github-actions[bot]@users.noreply.github.com>) and the message (chore: sync sponsors) are unchanged. I ran the step against a scratch repo with a bare remote: with no changes it exits 0 without committing; with changes it commits and pushes with the same author and committer as earlier sponsor commits on main. persist-credentials is left at its default because the job still pushes.

release.yaml. The release job now has if: github.ref == 'refs/heads/main' and environment: npm-release. Because the job only runs on main, skip_git_check can no longer publish from another branch, so I kept that input. The ref check alone is only defense in depth, since anyone who can push a branch can edit the workflow on it. The real control is the protected environment plus an npm publisher bound to it (see follow-ups). This does not break current releases: GitHub creates npm-release unprotected on first use, and an npm trusted publisher with no environment set still accepts the token.

Test. Added tests/github-workflows.test.ts. It uses node:fs and line regexes, with no new dependency. It asserts that:

  • every non-local uses: is pinned to a 40-char SHA with a # vX.Y.Z comment
  • dependabot.yml tracks github-actions
  • the release job has environment: npm-release and the refs/heads/main guard
  • no workflow references EndBug/add-and-commit

All 4 tests fail against the old .github/ and pass with this change.

Manual follow-ups (settings outside git)

  • GitHub → Settings → Environments → npm-release: add required reviewers and limit deployment branches to main. The environment exists after the first release run, or you can create it by hand beforehand.
  • After this PR is merged: on npmjs.com, set environment npm-release on the trusted publisher of each @standard-server/* package (core, fetch, node, fastify, aws-lambda, peer, shared). ⚠️ Doing this before merge breaks releases, because the current workflow has no environment.
  • Consider a main branch ruleset that requires pull requests.

Verification

  • pnpm vitest run tests/github-workflows.test.ts: 4 passed
  • pnpm run test: 61 files, 1361 tests passed
  • pnpm run check (sherif, oxlint, oxfmt, tsc): all passed
  • All edited YAML files parse

🤖 Generated with Claude Code

https://claude.ai/code/session_01MuvzyD4oT6bQx43q8DnNBn


Generated by Claude Code

- Pin every `uses:` to a full commit SHA with a `# vX.Y.Z` comment,
  keeping each action's current major.
- Track `github-actions` in Dependabot so the pins stay current.
- Replace EndBug/add-and-commit in sponsors-sync with a plain git step,
  so the daily `contents: write` token no longer reaches a third-party
  action. Author, committer and message are unchanged.
- Run the release job only on `main` and in the `npm-release`
  environment, so a protected environment and an npm trusted publisher
  bound to it can gate publishing.
- Add a test that keeps these properties from regressing.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01MuvzyD4oT6bQx43q8DnNBn
@pkg-pr-new

pkg-pr-new Bot commented Oct 13, 2026

Copy link
Copy Markdown
@standard-server/aws-lambda

npm i https://pkg.pr.new/@standard-server/aws-lambda@158

@standard-server/core

npm i https://pkg.pr.new/@standard-server/core@158

@standard-server/fastify

npm i https://pkg.pr.new/@standard-server/fastify@158

@standard-server/fetch

npm i https://pkg.pr.new/@standard-server/fetch@158

@standard-server/node

npm i https://pkg.pr.new/@standard-server/node@158

@standard-server/peer

npm i https://pkg.pr.new/@standard-server/peer@158

@standard-server/shared

npm i https://pkg.pr.new/@standard-server/shared@158

commit: 25350ef

@codecov

codecov Bot commented Oct 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

⚠️ 5 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

✅ 26 untouched benchmarks
⏩ 108 skipped benchmarks1


Comparing claude/quirky-wozniak-w0y81u (25350ef) with main (31a8ef3)

Open in CodSpeed

Footnotes

  1. 108 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No blocking issues — every pinned SHA checks out against its upstream tag (annotated-tag peels included). One operational note on the npm trusted-publisher follow-up.

Reviewed changes

  • Action SHA pinning — all 16 uses: references across ci.yaml, codspeed.yaml, pullfrog.yml, release.yaml, and sponsors-sync.yaml are now full commit pins with # vX.Y.Z comments, and each resolves to the version it claims.
  • sponsors-sync.yaml — EndBug/add-and-commit is replaced by a run: step that stages, no-ops when nothing changed, and commits and pushes under the same identity, so the write token no longer reaches a third-party action.
  • release.yaml — the release job gains environment: npm-release and an if: github.ref == 'refs/heads/main' guard.
  • dependabot.yml — a weekly github-actions ecosystem entry with a 7-day cooldown keeps the new pins current.
  • tests/github-workflows.test.ts — asserts SHA pinning plus version comments, the release environment and branch guard, the Dependabot entry, and the absence of EndBug/add-and-commit.

ℹ️ Binding the npm publisher to npm-release is not an in-place edit

The post-merge follow-up says to "set environment npm-release on the trusted publisher" of each @standard-server/* package, but npm treats an existing trusted publisher connection as immutable — the environment is part of that fixed connection — so this step actually means deleting and recreating each connection. A recreated connection must complete a successful publish within 2 days or it expires and can no longer be used. Time the recreation for right before the next release so each new connection validates inside that window. The workflow change itself is safe to merge first: a publisher with no environment accepts a token that carries one, so releases keep working until the npm side is updated.

Technical details
# npm trusted-publisher environment binding

## Affected sites
- PR description, "Manual follow-ups" — "set environment `npm-release` on the trusted publisher of each `@standard-server/*` package" assumes an editable field.
- `release.yaml:38` — `environment: npm-release` (the workflow side is correct).

## Required outcome
- The follow-up should read: delete + recreate each package's trusted publisher with environment `npm-release`, immediately before a release.

## Evidence
- docs.npmjs.com/trusted-publishers: "Existing trusted publisher connections cannot be changed... To change them, delete the connection and create a new one."
- Same page: "A newly created trusted publisher configuration must complete its first successful publish within 2 days... If the configuration is not validated within 2 days, it expires and can no longer be used or edited."

## Open question for the human
- Confirm the npm web UI / `npm trust` CLI exposes no in-place environment edit for an existing connection; if it does, the delete/recreate caveat is moot.

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@dinwwwh dinwwwh closed this Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants