Release from GitHub Actions with trusted publishing, and package only tracked files

.github/workflows/release.yml

@@ -0,0 +1,74 @@
+name: Release
+
+# Pushing a v* tag builds the gem from that commit and publishes it. This is
+# the workflow shape RubyGems recommends: one job, in a reviewer-gated
+# environment, using rubygems/release-gem, which runs `rake release` with
+# short-lived OIDC credentials from RubyGems trusted publishing and uploads a
+# sigstore attestation. Because the tag already exists, `rake release` skips
+# tagging and pushing git and only builds and pushes the gem.
+#
+# RELEASING.md covers cutting a release, verifying it, recovery, and the
+# one-time GitHub and RubyGems setup.
+on:
+  push:
+    tags: [ "v*" ]
+
+permissions: {}
+
+concurrency:
+  group: release
+  cancel-in-progress: false
+
+jobs:
+  release:
+    name: Publish to RubyGems and create the GitHub Release
+    runs-on: ubuntu-latest
+    environment: release-rubygems
+    permissions:
+      contents: write # gh release create
+      id-token: write # RubyGems trusted publishing and sigstore signing
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          fetch-depth: 0
+          persist-credentials: false
+
+      - name: Check that the tag names the version and is on main
+        id: version
+        env:
+          REF_NAME: ${{ github.ref_name }}
+        run: |
+          version="$(ruby -Ilib -rupright/version -e 'puts Upright::VERSION')"
+          if [ "v${version}" != "$REF_NAME" ]; then
+            echo "::error::Tag ${REF_NAME} does not match Upright::VERSION ${version}"
+            exit 1
+          fi
+          if ! git merge-base --is-ancestor "$GITHUB_SHA" origin/main; then
+            echo "::error::Tagged commit ${GITHUB_SHA} is not on origin/main"
+            exit 1
+          fi
+          echo "version=${version}" >> "$GITHUB_OUTPUT"
+          echo "prerelease=$(ruby -e 'puts Gem::Version.new(ARGV[0]).prerelease?' "$version")" >> "$GITHUB_OUTPUT"
+
+      # No bundler cache in the job that builds and publishes: a restored cache
+      # would be loaded by the process that builds the gem.
+      - uses: ruby/setup-ruby@95ef2b042f9d7a56d8268cba8559e2842e2ad01b # v1.321.0
+        with:
+          ruby-version: "4.0"
+          bundler-cache: false
+
+      - run: bundle install
+
+      - name: Build, publish and attest the gem
+        uses: rubygems/release-gem@7f9650160c1a4e7989fdc9855807bdbd421d8b6b # v1.4.1
+
+      - name: Create the GitHub Release with the gem attached
+        env:
+          GH_TOKEN: ${{ github.token }}
+          REF_NAME: ${{ github.ref_name }}
+          VERSION: ${{ steps.version.outputs.version }}
+          PRERELEASE: ${{ steps.version.outputs.prerelease }}
+        run: |
+          flags=(--verify-tag --generate-notes)
+          if [ "$PRERELEASE" = "true" ]; then flags+=(--prerelease); fi
+          gh release create "$REF_NAME" "pkg/upright-${VERSION}.gem" "${flags[@]}"

CHANGELOG.md

@@ -4,6 +4,18 @@
 
 ### Security
 
+- Release from GitHub Actions instead of a laptop. `bin/release` built the gem
+  from whatever was in the working tree, pushed it with a personal RubyGems
+  OTP, and only then committed and tagged, so a tag and its gem did not have
+  to match. The gemspec globbed the filesystem, which put the ignored
+  `config/credentials/development.key` and `test.key` into the public 0.2.0 and
+  0.3.0 gems. Treat both keys as disclosed. The gemspec now takes its file list
+  from `git ls-files`, a test checks that an ignored key cannot be packaged,
+  and `.github/workflows/release.yml` builds each `v*` tag from a clean
+  checkout of that commit, checks that the tag matches `Upright::VERSION` and
+  is on `main`, publishes through RubyGems trusted publishing with a sigstore
+  attestation, and creates the GitHub Release with the gem attached. See
+  `RELEASING.md`.
 - Stop serving the Playwright Trace Viewer from Upright's origin, and stop
   vendoring it. The viewer serialised a trace's tags and attributes into
   `text/html` on the admin origin, so a crafted trace ZIP executed script there

CONTRIBUTING.md

@@ -33,3 +33,9 @@ bin/rails test
 ```
 
 Playwright integration tests require Docker. Start the Playwright server with `bin/services` before running the full suite.
+
+## Releasing
+
+Releases are built and published by GitHub Actions from a tag on `main`, not
+from a laptop. `RELEASING.md` describes the workflow, the version bump and tag
+steps, verification, recovery, and the one-time repository setup.

RELEASING.md

@@ -0,0 +1,103 @@
+# Releasing upright
+
+A release is a `vX.Y.Z` tag on `main`. Pushing the tag runs
+`.github/workflows/release.yml`, which has one job in the `release-rubygems`
+environment. After a reviewer approves it, the job:
+
+1. Checks that the tag matches `Upright::VERSION` and that the tagged commit
+   is on `origin/main`.
+2. Runs `rubygems/release-gem`. That action obtains a short-lived credential
+   from RubyGems trusted publishing, runs `rake release` (which builds the gem
+   into `pkg/`, skips tagging because the tag exists, and pushes the gem with
+   a sigstore attestation), and waits until rubygems.org serves the version.
+3. Creates the GitHub Release for the tag with generated notes and the built
+   gem attached.
+
+The gemspec lists files with `git ls-files`, so nothing untracked or ignored
+can enter the package. `test/packaging_test.rb` checks this in CI. RubyGems
+builds reproducibly by default, so `gem build upright.gemspec` at the tag
+produces the same bytes as the published gem.
+
+## Cutting a release
+
+1. On a branch, set the new version in `lib/upright/version.rb`, run
+   `bundle install` to refresh `Gemfile.lock`, move the CHANGELOG's Unreleased
+   notes under the new version, open a PR, and merge it.
+
+   A `playwright-ruby-client` constraint change is not a routine bump.
+   `Upright::PLAYWRIGHT_VERSION`, the gemspec constraint, `package.json`, the
+   install generator's Dockerfile and `test/dummy/docker-compose.yml` change
+   together.
+2. On an up-to-date `main` checkout, run `rake tag`. It stops if the tree is
+   dirty, the branch is not `main`, `HEAD` differs from `origin/main`, or the
+   tag already exists. Otherwise it creates the annotated tag and pushes it.
+3. Approve the `release-rubygems` environment when the workflow pauses.
+4. When the run finishes, check the result:
+
+   ```sh
+   gem fetch upright -v X.Y.Z
+   gh release download vX.Y.Z --pattern '*.gem' --output github-upright-X.Y.Z.gem
+   sha256sum upright-X.Y.Z.gem github-upright-X.Y.Z.gem
+   ```
+
+   The two digests must be equal. The gem's page on rubygems.org shows the
+   provenance from the attestation, naming this repository, the workflow and
+   the tag.
+
+Push one tag at a time. The workflow's concurrency group keeps one pending
+run, so a second tag pushed while a run is in progress replaces the queued
+one.
+
+## Recovery
+
+| State | What to do |
+|---|---|
+| The run failed before `gem push` | Fix on `main`, delete the tag, tag again. Deleting is allowed only because nothing was published. |
+| The gem was pushed, then creating the GitHub Release failed | Do not re-run: `gem push` refuses an existing version. Create the release by hand: `gem fetch upright -v X.Y.Z` then `gh release create vX.Y.Z upright-X.Y.Z.gem --verify-tag --generate-notes`. |
+| A published release is bad | Do not move or delete the tag. Publish a new patch version. Yank only for a security problem in the published gem. |
+
+## One-time setup
+
+Set `RELEASE_REVIEWER` to the GitHub login of the person who approves
+releases.
+
+1. **Default workflow token.** Make the default `GITHUB_TOKEN` read-only. The
+   release job declares the permissions it needs.
+
+   ```sh
+   gh api -X PUT repos/basecamp/upright/actions/permissions/workflow \
+     -f default_workflow_permissions=read
+   ```
+
+2. **Environment `release-rubygems`.** One required reviewer,
+   `prevent_self_review: false` so the releaser can approve their own release,
+   `can_admins_bypass: false`, and a deployment branch policy that allows only
+   `v*` tags.
+
+   ```sh
+   reviewer_id=$(gh api "users/${RELEASE_REVIEWER}" --jq .id)
+   gh api -X PUT repos/basecamp/upright/environments/release-rubygems \
+     --input - <<JSON
+   { "reviewers": [{ "type": "User", "id": ${reviewer_id} }],
+     "prevent_self_review": false,
+     "can_admins_bypass": false,
+     "deployment_branch_policy": { "protected_branches": false, "custom_branch_policies": true } }
+   JSON
+   gh api -X POST repos/basecamp/upright/environments/release-rubygems/deployment-branch-policies \
+     -f name='v*' -f type=tag
+   ```
+
+3. **Tag rulesets.** Two rulesets on `refs/tags/v*` with enforcement
+   `active`: one restricts creation, with the releaser as the only bypass
+   actor; the other blocks update and deletion with no bypass actors.
+
+4. **RubyGems trusted publisher.** On rubygems.org, in the `upright` gem's
+   trusted publishing settings, add a GitHub Actions publisher with repository
+   `basecamp/upright`, workflow `release.yml` and environment
+   `release-rubygems`. Then remove any long-lived API keys from the owner
+   accounts and confirm every owner has MFA enabled, so the workflow is the
+   only way to push.
+
+5. **Pinned actions.** After this workflow is merged, enable "Require actions
+   to be pinned to a full-length commit SHA" in the repository's Actions
+   settings.

Rakefile

@@ -6,3 +6,23 @@ load "rails/tasks/engine.rake"
 require "bundler/gem_tasks"
 
 Dir.glob("lib/tasks/**/*.rake").each { |r| load r }
+
+desc "Tag v<current version> and push it, which starts the Release workflow"
+task :tag do
+  require_relative "lib/upright/version"
+  tag = "v#{Upright::VERSION}"
+
+  abort "tag: the working tree has uncommitted changes" unless `git status --porcelain`.empty?
+
+  branch = `git rev-parse --abbrev-ref HEAD`.strip
+  abort "tag: run this on main (currently on #{branch})" unless branch == "main"
+
+  sh "git fetch origin main --quiet"
+  abort "tag: HEAD differs from origin/main; pull or push first" unless `git rev-parse HEAD`.strip == `git rev-parse origin/main`.strip
+  abort "tag: #{tag} already exists locally" unless `git tag --list #{tag}`.strip.empty?
+  abort "tag: #{tag} already exists on origin" unless `git ls-remote --tags origin refs/tags/#{tag}`.strip.empty?
+
+  sh "git tag --annotate #{tag} --message 'upright #{tag}'"
+  sh "git push origin #{tag}"
+  puts "Pushed #{tag}. Approve the release-rubygems environment when the Release workflow pauses."
+end

test/packaging_test.rb

@@ -0,0 +1,41 @@
+require "test_helper"
+
+# The public 0.2.0 and 0.3.0 gems shipped config/credentials/development.key and
+# test.key because the gemspec globbed the filesystem instead of reading the git
+# index. These hold that line.
+class PackagingTest < ActiveSupport::TestCase
+  ROOT = Upright::Engine.root
+
+  test "packages only files tracked by git" do
+    assert_empty packaged_files - tracked_files
+  end
+
+  test "an ignored credential key can't enter the package" do
+    key = ROOT.join("config/credentials/test.key")
+    created = !key.exist?
+
+    if created
+      key.dirname.mkpath
+      key.write(SecureRandom.hex(16))
+    end
+
+    assert_not_includes packaged_files, "config/credentials/test.key"
+  ensure
+    key.delete if created
+  end
+
+  test "packages the engine's runtime files" do
+    assert_includes packaged_files, "lib/upright.rb"
+    assert_includes packaged_files, "config/routes.rb"
+    assert_includes packaged_files, "app/controllers/upright/application_controller.rb"
+  end
+
+  private
+    def packaged_files
+      Gem::Specification.load(ROOT.join("upright.gemspec").to_s).files
+    end
+
+    def tracked_files
+      IO.popen(%w[git ls-files -z], chdir: ROOT.to_s) { |ls| ls.readlines("\x0", chomp: true) }
+    end
+end

upright.gemspec

@@ -14,9 +14,13 @@ Gem::Specification.new do |spec|
   spec.metadata["source_code_uri"] = "https://github.com/basecamp/upright"
   spec.metadata["changelog_uri"]   = "https://github.com/basecamp/upright/blob/main/CHANGELOG.md"
 
-  spec.files = Dir.chdir(File.expand_path(__dir__)) do
-    Dir["{app,config,db,lib,public}/**/*", "LICENSE.md", "Rakefile", "README.md"]
-  end
+  # The manifest is the git index, not a filesystem glob. A glob packages whatever
+  # is present in the working tree, which is how the ignored
+  # config/credentials/*.key files reached the public 0.2.0 and 0.3.0 gems.
+  # test/packaging_test.rb checks this.
+  spec.files = IO.popen(%w[git ls-files -z], chdir: __dir__, err: IO::NULL) do |ls|
+    ls.readlines("\x0", chomp: true)
+  end.grep(%r{\A(?:app|config|db|lib|public)/|\A(?:LICENSE\.md|Rakefile|README\.md)\z})
 
   spec.required_ruby_version = ">= 3.4"