Conversation
…mation
CI (rewritten): bare-Ruby matrix (3.1-head) proving the stdlib-only claim,
lint job with RuboCop + coverage thresholds + release-script fixture tests,
checksum-verified actionlint + pedantic zizmor, dependency review, and a
stable "CI" fan-in check for branch protection. Top-level permissions: {}
everywhere; every job grants exactly what it needs; every action pinned to
a full SHA under a minimum-age policy.
Release pipeline (test -> build -> publish -> confirm -> attest ->
github-release): unprivileged reproducible build (exact Ruby/RubyGems pins,
SOURCE_DATE_EPOCH from the commit), artifact identity flowing forward by
digest, OIDC trusted publishing gated by the release-rubygems environment,
and a total registry state machine (script/release/, 100% line+branch
covered) that makes re-runs idempotent and fails closed on any conflict.
Registry confirmation lives outside the credentialed job; only canonical,
registry-confirmed bytes are ever attested or attached to the GitHub
Release. Credentials are scrubbed unconditionally. A standalone recovery
workflow finishes attestation + Release from verified canonical registry
bytes when a published tag's workflow is defective.
Dependabot: weekly grouped updates with cooldowns, and zero-touch
auto-merge for bundler patch/minor only, built on a constrained
pull_request_target base workflow: no checkout, no PR-controlled code, API
re-validation of actor, head SHA, changed-file allowlist, and
Dependabot-signed commits. CODEOWNERS deliberately scoped so lockfile PRs
don't deadlock on code-owner review.
RELEASING.md documents the procedure, the one-time repo/RubyGems setup,
the recovery state table, and the honest admin-threat limitation.
There was a problem hiding this comment.
Pull request overview
Establishes CI, trusted publishing, automated releases/recovery, Dependabot automation, and release documentation.
Changes:
- Adds hardened CI and release workflows.
- Adds registry/package verification scripts and tests.
- Adds dependency automation, release tasks, and documentation.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
test/test_helper.rb |
Configures optional coverage. |
test/surfguard_test.rb |
Uses shared test setup. |
test/release/registry_test.rb |
Tests registry state transitions. |
surfguard.gemspec |
Updates metadata and package files. |
SECURITY.md |
Clarifies supported versions. |
script/release/verify_package.rb |
Verifies built gems. |
script/release/registry.rb |
Implements registry reconciliation. |
script/release/registry_download.rb |
Adds recovery download CLI. |
script/release/registry_confirm.rb |
Adds confirmation CLI. |
script/release/registry_check.rb |
Adds reconciliation CLI. |
RELEASING.md |
Documents release operations. |
README.md |
Adds CI and installation guidance. |
Rakefile |
Adds bump, lint, and tag tasks. |
Gemfile.lock |
Locks development dependencies. |
Gemfile |
Defines development dependencies. |
.rubocop.yml |
Configures project linting. |
.gitignore |
Tracks lockfile and ignores coverage. |
.github/workflows/release.yml |
Defines the release pipeline. |
.github/workflows/release-recovery.yml |
Defines exceptional recovery. |
.github/workflows/dependabot-auto-merge.yml |
Automates safe dependency merges. |
.github/workflows/ci.yml |
Expands and hardens CI. |
.github/release.yml |
Configures generated release notes. |
.github/dependabot.yml |
Configures dependency updates. |
.github/CODEOWNERS |
Protects release-sensitive paths. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd087c5ba6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 24 changed files in this pull request and generated 1 comment.
Suppressed comments (7)
.github/workflows/release-recovery.yml:62
- This action creates
tag_namefrom the repository's default branch when that tag is absent. A mistyped recovery version can therefore create a new tag/release instead of recovering the immutable published tag. Fail before attestation unless the exactrefs/tags/v${VERSION}ref already exists.
tag_name: v${{ inputs.version }}
RELEASING.md:53
- The command says verification must be constrained by source ref, but it omits the available
--source-refconstraint. As written, an attestation from the same workflow on another ref can satisfy this check. Add--source-ref refs/tags/vX.Y.Z.
- `gh attestation verify surfguard-X.Y.Z.gem --signer-workflow basecamp/surfguard/.github/workflows/release.yml` —
constrain by signer workflow and source ref, not just `--repo`.
script/release/verify_package.rb:31
- The package verifier's integrity, content allowlist, dependency, and isolated-install branches have no fixture tests under
test/. The 100% SimpleCov threshold does not cover this executable because it is never loaded by the suite, so regressions can pass normal PR CI and surface only during a release/rehearsal workflow. Add valid and invalid gem fixtures that exercise these checks.
package = check("archive integrity (Gem::Package#verify)") do
Gem::Package.new(gem_file).tap(&:verify)
Rakefile:35
- The new
bumpguards and write behavior have no automated tests. Because the Rakefile is loaded beforetest_helperstarts SimpleCov, the reported 100% threshold does not exercise this task. Add subprocess tests in a temporary repository for malformed/non-increasing versions, dirty trees, zero-write failures, and the successful version/lock rewrite.
task :bump, [ :version ] do |_t, args|
version = args[:version].to_s
abort "bump: version must be exact semver X.Y.Z (got #{version.inspect})" unless version.match?(/\A\d+\.\d+\.\d+\z/)
abort "bump: working tree must be clean" unless clean_tree?
Rakefile:56
- The release-critical
tagguards and push ordering are not covered by tests, and Rakefile execution is outside the SimpleCov window. Add temporary-repository tests that verify each guard performs no writes and that a successful run pushesmainbefore creating/pushing the tag.
task :tag do
version = surfguard_version
tag = "v#{version}"
abort "tag: working tree must be clean" unless clean_tree?
.github/workflows/release-recovery.yml:66
- The version validator rejects
-, so this expression is always false for every accepted input. RubyGems marks letter-bearing versions such as1.0.0.pre.1as prereleases, so recovery would publish them as normal GitHub Releases. ComputeGem::Version#prerelease?during validation and pass that result to this input.
prerelease: ${{ contains(inputs.version, '-') }}
.github/workflows/release.yml:270
- Checking only for
-does not implement RubyGems prerelease semantics:Gem::Version#prerelease?treats any letter-bearing version (for example,1.0.0.pre.1) as a prerelease. Such a tag builds successfully but this action publishes its GitHub Release as stable. Derive and expose the prerelease flag fromGem::Versionalongside the version output.
prerelease: ${{ contains(needs.build.outputs.version, '-') }}
Per Copilot and Codex review of #3: - Recovery workflow: gate behind a release-recovery environment (same reviewer as publishing); require the vX.Y.Z tag to exist on main before anything privileged; rebuild the gem from the tagged source with the tag's own toolchain pins and require rebuilt digest == canonical RubyGems digest before attesting, so the build-provenance attestation is honest rather than vouching for merely-downloaded bytes. - Prerelease detection uses Gem::Version#prerelease? (letter-bearing versions like 0.2.0.rc1), not a hyphen check, in both release paths. - Dependabot auto-merge: approval is created via the API pinned to the validated head commit after a last-instant head recheck; a new revoke job disables pending auto-merge when a non-Dependabot actor pushes to a Dependabot PR. - CI asserts zero runtime dependencies explicitly (dependency-review only screens vulnerable deps, not benign new runtime deps). - Registry download resolves relative and scheme-relative redirect Locations against the current URI before enforcing https-only. - Package verification is now a library (PackageVerification) under the coverage gate, with fixture tests for every check: corrupt archive, wrong name/version, runtime deps, missing/unexpected files, failing install/require probes. Rake bump/tag guards get subprocess tests in a throwaway repo with a local bare origin, asserting zero writes on every rejection and main-then-tag push ordering on success. - RELEASING.md: verification commands gain --source-ref and a recovery signer-workflow variant; recovery and Dependabot sections updated; one-time setup gains the release-recovery environment (created and read back).
|
Addressed all review feedback in 9408e2b (all 11 inline threads fixed and resolved). Copilot's suppressed comments are covered too: tag-existence guard before attestation, |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9408e2b22b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 27 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
RELEASING.md:160
- The Dependabot workflow relies on stale approvals being dismissed after a head update (
dependabot-auto-merge.yml:113-132), but this setup checklist never enables that rule. Following these instructions leaves the bot approval valid after a human push until the revoke job runs. Require dismissal of stale approvals in the main ruleset so the documented defense is actually configured.
5. **Main branch ruleset** — require PRs (≥ 1 approving review, code-owner
review), required status check **`CI`** bound to the GitHub Actions app
(integration_id **15368**) with strict up-to-date policy, block deletion +
force pushes. Bypass: the release team with `bypass_mode: pull_request`
(so Jeremy's own PRs don't deadlock on self-approval). Read back.
script/release/registry.rb:253
malformedis never reset after an intervening 404, retryable HTTP response, or network fault. Three malformed responses anywhere in the five-minute poll therefore abort as though they were consecutive, contrary toMALFORMED_LIMIT's documented semantics. Reset this counter when a non-malformed poll outcome intervenes, and cover that sequence in the fixture tests.
malformed += 1
raise if malformed >= MALFORMED_LIMIT
test/test_helper.rb:15
- These measured counts are stale now that the suite loads
registry.rbandpackage_verification.rb; coverage includes substantially more than 66 lines and 24 branches. Remove the volatile counts or update them so the coverage documentation matches the new suite.
# Measured actuals are 100/100 (66/66 lines, 24/24 branches), so the
# thresholds pin them there: any change that leaves a line or branch
# untested fails loudly.
…ding - Recovery is now two jobs mirroring the release pipeline's separation: unprivileged verify (tag proof, git-archive extraction of the tagged source so the dispatch revision's helpers keep running, rebuild with the tag's pins, canonical download, digest equality) and a reviewer-gated finish with no checkout that only attests/releases artifact bytes whose digest verify proved. - Recovery is preferably dispatched on the release tag so the attestation's source ref binds to refs/tags/vX.Y.Z and the documented --source-ref verification holds; the version input is cross-checked against the dispatch ref, and the environment gained a v* tag policy. Main dispatch remains the fallback for a defective tagged recovery workflow, documented as binding to refs/heads/main. - The auto-merge revoke job also dismisses standing bot approvals, so the workflow no longer depends solely on the dismiss-stale branch rule (which the live ruleset does set; RELEASING.md now documents it). - Package verification scrubs RUBYLIB from probe subprocesses so the require probe cannot resolve from the checkout. - The registry poll's malformed-body tolerance is now genuinely consecutive: the counter resets on any intervening non-malformed outcome, with a fixture test. - test_helper drops the stale hardcoded coverage counts.
|
Second round addressed in 8ff24fa. Copilot's suppressed comments are in too: the registry poll's malformed tolerance is now genuinely consecutive (counter resets on intervening non-malformed outcomes, with a fixture test), RELEASING.md's main-ruleset checklist now documents dismiss-stale-approvals (the live ruleset already sets it), and the stale hardcoded coverage counts are gone from test_helper. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 27 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Rakefile:47
- The release-critical fixture suite exercises only rejected bumps; no test runs a successful bump or verifies that Bundler actually refreshes the path gem's version in
Gemfile.lock. A regression in this command can therefore pass CI while producing an incomplete version bump. Add a fixture Gemfile/lock and a success test asserting both files are updated.
sh "bundle install --quiet" # refresh Gemfile.lock's surfguard version
.github/workflows/dependabot-auto-merge.yml:169
- The review-dismissal endpoint accepts
messagebut not aneventfield;event=DISMISSbelongs to review creation/submission. This request can therefore return 422, leaving the revoke workflow failed exactly when it is intended to remove the standing bot approval. Remove the extra field.
gh api --method PUT "repos/${REPO}/pulls/${PR_NUMBER}/reviews/${review_id}/dismissals" \
-f message="Dismissing the automated approval: a non-Dependabot actor pushed to this PR after it was granted." \
-f event=DISMISS
…ccess - The review-dismissal API takes only message; drop the event=DISMISS field that would 422 the revoke job exactly when it matters. - Add a bump success test: a bundler fixture (gemspec + Gemfile + lock, no remote deps) proves version.rb is rewritten and Gemfile.lock picks up the new version. Subprocesses invoke rake as a library and scrub every BUNDLE*/RUBYOPT/RUBYLIB knob so the fixture repo behaves like a developer shell rather than inheriting the test runner's bundle.
|
Round three (suppressed comments) addressed: removed the invalid |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 27 changed files in this pull request and generated no new comments.
Suppressed comments (3)
.github/workflows/release-recovery.yml:101
- The recovery job invokes
python3with PyYAML before any dependency setup, but the currentubuntu-latestinventory does not install PyYAML (Ansible's Python dependencies are isolated under pipx).import yamltherefore prevents recovery from reaching the rebuild. Parse this with the runner's Ruby stdlib YAML support, or explicitly install and pin the Python dependency.
echo "ruby=$(python3 -c 'import yaml; print(yaml.safe_load(open("tagged-src/.github/workflows/release.yml"))["env"]["RUBY_VERSION"])')"
echo "rubygems=$(python3 -c 'import yaml; print(yaml.safe_load(open("tagged-src/.github/workflows/release.yml"))["env"]["RUBYGEMS_VERSION"])')"
script/release/package_verification.rb:25
RUBYGEMS_GEMDEPSis still inherited by both subprocesses. When set (especially to-), RubyGems auto-discovers and activates a dependency file at startup, so the require probe can activate the checkout's Gemfile/path gem or fail on its dependencies instead of testing only the isolated installation. Clear it here astest/release/rake_tasks_test.rb:166already does for isolated subprocesses.
CLEAN_ENV = {
"RUBYOPT" => nil, "RUBYLIB" => nil,
"BUNDLE_GEMFILE" => nil, "BUNDLE_PATH" => nil
}.freeze
Rakefile:34
- This accepts versions with leading-zero numeric identifiers, such as
01.2.3, even though those are not valid semantic versions. Since this is the guard meant to enforce exactX.Y.ZSemVer before writing release files, reject leading zeros except for the identifier0.
abort "bump: version must be exact semver X.Y.Z (got #{version.inspect})" unless version.match?(/\A\d+\.\d+\.\d+\z/)
- The recovery pins step no longer needs python3/PyYAML (not guaranteed in the runner inventory): anchored sed reads the pins from our own tagged release.yml, validated non-empty before use. - Package verification also scrubs RUBYGEMS_GEMDEPS so the probes can't auto-activate a discovered dependency file instead of testing the isolated install. - bump rejects leading-zero numeric identifiers (01.2.3), with tests.
|
Round four (suppressed comments) addressed: the recovery pins step now uses anchored sed on our own release.yml instead of python3+PyYAML (validated non-empty before use), package-verification probes also scrub RUBYGEMS_GEMDEPS, and |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 27 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Rakefile:34
- This accepts leading-zero components such as
00.2.0, even though the task promises exact SemVer. RubyGems treats that as equivalent to0.2.0, so the task can create a noncanonical version/tag and then reject a later correction as non-increasing. Require each numeric component to be either0or a nonzero digit followed by digits.
# Exact SemVer: numeric identifiers may not have leading zeros.
.github/workflows/dependabot-auto-merge.yml:127
- The one-time setup never enables the repository's Allow auto-merge setting. That setting is disabled independently of token permissions, and
gh pr merge --autofails when it is off, so the advertised zero-touch Dependabot path will stop at this command even after the documented workflow-token configuration is applied. Enable and read backallow_auto_mergeas part of the setup procedure.
gh pr merge "$PR_URL" --auto --squash --match-head-commit "$EVENT_HEAD_SHA"
- gh pr merge --auto requires the repository's allow_auto_merge setting, which is independent of token permissions; the one-time setup now enables and reads it back (applied to the live repo). - Pin the already-correct semver guard behavior with a 00.2.0 rejection case (each numeric component is 0 or nonzero-leading; leading-zero components never matched).
|
Round five addressed: the repo's allow_auto_merge setting is now enabled and read back (live), and documented in the one-time setup — good catch, |
| - uses: ruby/setup-ruby@95ef2b042f9d7a56d8268cba8559e2842e2ad01b # v1.321.0 | ||
| with: | ||
| ruby-version: ${{ matrix.ruby }} | ||
| # No Gemfile: the gem is stdlib-only and rake/minitest ship with Ruby. | ||
| # Bare Ruby on purpose: prove the gem and its suite need nothing | ||
| # beyond the standard library (rake/minitest ship with Ruby). | ||
| bundler-cache: false |
Sets up the full release-engineering stack for surfguard (setup only — no v0.1.0 cut here).
CI (
ci.yml, rewritten)bundler-cache: false, no Gemfile) proving the stdlib-only claim as execution;headis non-blocking.lint: RuboCop (rubocop-37signals), coverage thresholds pinned at the measured 100% line / 100% branch, and fixture tests for the release scripts.lint-actions: checksum-verified actionlint (with ShellCheck) + authenticated pedantic zizmor over the whole tree (dependabot.ymlincluded). Both pass locally with zero unjustified findings.dependency-reviewon PRs (fail-on-scopes: development, runtime, unknown— zero runtime deps is a hard invariant).CIfan-in check for branch protection: fails on failure/cancelled, tolerates skipped.permissions: {}in every workflow; explicit per-job permissions; every action pinned to a full 40-char SHA under a ≥10-day minimum-age policy.Release pipeline (
release.yml)test → build → publish → confirm → attest → github-release;workflow_dispatchis always a no-publish rehearsal (test → build only).SOURCE_DATE_EPOCHfrom the commit — two local builds produce byte-identical digests), verifies the package end to end (Gem::Package#verify, exact archive contents, zero runtime deps, isolatedGEM_HOMEinstall + require), and emits the artifact digest that every downstream job re-asserts before acting.release-rubygemsenvironment + OIDC trusted publishing,id-token: writeonly, no checkout (release scripts travel by artifact from the trusted build checkout). Registry reconciliation is a total state machine on the exact v2 endpoint — 404 → push, 200-same-SHA → skip, everything else fails closed — making re-runs idempotent with no error-string matching anywhere. Credential scrub runsif: always().actions/attest—attest-build-provenanceis now a thin wrapper around it).fail_on_unmatched_files: true.cancel-in-progress: false; RELEASING.md documents the one-pending-run-per-group constraint (one release at a time).release-recovery.ymlcovers "push succeeded, downstream failed, tagged workflow defective": it downloads canonical bytes from RubyGems, verifies them against the registry-reported SHA, and finishes attestation + Release from those verified bytes. It never publishes.Release scripts (
script/release/)Stdlib-only, 100% line+branch covered, with fixture tests for every registry transition (404, 200-same, 200-diff, 200-wrong-version, 429, 5xx, malformed, network fault, timeout, redirect handling). All paths fail closed except the two defined proceed/skip transitions.
Dependabot
dependabot-auto-merge.yml: zero-touch auto-merge for bundler patch/minor only, on a constrainedpull_request_targetbase workflow — no checkout, no PR-controlled code anywhere; job-level dependabot gating plus independent API re-validation (actor, author, head-ref prefix, head-SHA TOCTOU guard with--match-head-commit, changed-file allowlist ofGemfile/Gemfile.lock, Dependabot-signed commits, in-repo head). Actions-ecosystem PRs are never auto-approved or auto-merged.*, noGemfile.lock) so lockfile PRs don't deadlock on code-owner review.Rake tasks
rake bump[X.Y.Z](exact semver, clean tree, strictly greater, rewrites version.rb + refreshes the lock, commits nothing) andrake tag(clean tree, on main, HEAD == origin/main after fetch, tag absent locally + remotely, pushes main before tagging). All guards verified to reject with zero writes.Docs
RELEASING.md: procedure, one-time setup (workflow token, release team, environment, tag/main rulesets, SHA-pinning, trusted publisher timing), the recovery state table, and the honest statement that repo-level controls cannot defend against malicious admins — routine release authority narrows to Jeremy; the 19-admin residual exposure needs organizational mitigation.
README gains the CI badge and a post-v0.1.0-marked Installation section; SECURITY.md's supported version is
mainuntil the first packaged release.