feat(action): install a verified release in one CI step - #70
Conversation
The documented use of this tool is a CI gate — `sync --dry-run` exiting 4,
`lint` in a pipeline, `audit` across a fleet — and nothing said how the binary
reaches the runner. The two routes that existed both misfit: `go install`
installs a Go toolchain to compile a dependency-free binary that was already
built six ways at release time, and a hand-rolled `curl | sh` step verifies
build provenance only on a runner where gh happens to be signed in.
- uses: jordonpeterson/codeowners-tool@v0
- run: codeowners-tool lint --dry-run --github-repo ${{ github.repository }}
setup.sh downloads nothing itself. install.sh already fetches, checksums and
attests the archive, is already shellcheck'd, and is already gated by
tools/supplychain; a second download here would be a second supply-chain
surface for the same bytes. setup.sh resolves WHICH release to ask for and
hands off, then refuses to export a binary that is not the build the job asked
for. Provenance is verified by default rather than warned about, because hosted
runners ship gh and the action hands it a token — the one place the check can
always run.
A full-version pin installs exactly that release. The action ships from this
repository, so `@v0.0.29` reads as a pin, and resolving it to the newest
release would make it a pin in name only — the version someone writes down
during an incident naming a build they never ran. Releases now also move the v0
tag that the recommended pin resolves through, and cut on a change to the
action, without which every consumer on @v0 would freeze at the commit it first
pointed at.
Tested at both seams: tools/action drives setup.sh against a stub install.sh
for version resolution, provenance passthrough, the PATH export and every
refusal, and a CI job runs the action as a consumer would, against real
releases, on Linux and macOS.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HA6pqDeLuGi6szcbQdkr16
There was a problem hiding this comment.
🟡 Changes recommended
The new action should avoid unnecessary runtime dependencies (bash) and should validate the installed binary’s reported version format before exporting it to later steps.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a composite GitHub Action to install a checksum/provenance-verified codeowners-tool release in a single CI step, and updates CI/release automation plus docs to support and validate that distribution path.
Changes:
- Add
action.yml+tools/action/setup.shand a focused Go test suite to validate version resolution, provenance passthrough, PATH/outputs export, and refusal cases. - Update CI to ShellCheck the new script and add an end-to-end workflow job that runs the action against real releases on Ubuntu/macOS.
- Update release workflow to cut releases when the action changes and to move the major
v0tag after publishing; update docs/README/CHANGELOG to document the CI install route.
File summaries
| File | Description |
|---|---|
| tools/action/version_test.go | Tests version resolution semantics (pinned tag vs @v0 vs explicit input) and refusal cases. |
| tools/action/setup.sh | Action implementation: resolves release tag, delegates to install.sh, validates installed build, exports PATH/outputs. |
| tools/action/setup_test.go | Harness to execute setup.sh against a stub installer and stub gh. |
| tools/action/manifest_test.go | Gates manifest input/env mapping, SHA pinning of third-party actions, release workflow invariants, and doc discoverability. |
| README.md | Adds a one-line pointer to the recommended CI usage (uses: ...@v0). |
| docs/LINTING.md | Updates linting CI snippet to include the action install step. |
| docs/INSTALL.md | Adds a “GitHub Actions” install section and documents inputs/outputs. |
| CHANGELOG.md | Documents the new composite action and the major-tag movement behavior. |
| action.yml | New composite action manifest wiring inputs/outputs and token for gh. |
| .github/workflows/release.yml | Triggers releases on action changes and force-moves the major tag after publishing. |
| .github/workflows/ci.yml | Adds ShellCheck coverage for setup.sh and an e2e job exercising the action on Ubuntu/macOS. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…no release Both from review on #70. `shell: bash` while the step's only command is `sh <script>`: the action asked the runner for an interpreter it never used, and would fail outright on the minimal Linux images self-hosted runners are often built from, which carry /bin/sh and no bash. The script is POSIX by contract — shellcheck gates it as sh — so the step is now `shell: sh`. The installed-build check only ran when a version was pinned. On the unpinned path — no version input, no gh to resolve one, install.sh picking "latest" itself — there was no tag to compare against, so whatever the binary reported was exported and announced. A release built without the -X stamp reports "dev", which is exactly the "a fleet cannot be asked which build it is running" failure the release workflow stamps the tag to prevent; the action would have put it on the PATH and named it in its own output. The build now has to name a release tag either way. Both are pinned by tests that fail without the fix: the manifest gate reads the step's shell, and the unpinned refusal was reproduced first — "codeowners-tool dev is on the PATH" — then mutation-tested by removing the check again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HA6pqDeLuGi6szcbQdkr16
There was a problem hiding this comment.
🟡 Changes recommended
setup.sh currently treats an explicit version: latest as unset and can incorrectly default to the pinned action tag, so the action may not install “latest” when requested.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 2
- Review effort level: Lite
From review on #70. `latest` fell through to the same branch as an unset input, so a workflow that pinned the action at @v0.0.9 and asked for `version: latest` was handed v0.0.9 — silently, because the ref happened to name a release. Pinning the action for stable behavior while floating the tool is a coherent thing to ask for, and the input said latest while the job ran something else. `latest` now resolves as latest whatever the ref is; the ref default applies only when the input is genuinely empty. The manifest description and docs/INSTALL.md state that precedence rather than leaving it to be inferred. The regression test reproduced the bug before the fix — the run resolved v0.0.9 and tripped the installed-vs-requested check against the newest release — and reverting the branch turns it red again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HA6pqDeLuGi6szcbQdkr16
There was a problem hiding this comment.
🟡 Changes recommended
install-dir should be validated to prevent newline injection into $GITHUB_PATH/$GITHUB_OUTPUT when exporting PATH and outputs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Lite
From review on #70, which flagged install-dir. The same hole was wider than that. $GITHUB_PATH and $GITHUB_OUTPUT are line-delimited, so a directory whose name contains a newline appends entries the action never produced — a second PATH entry pointing wherever the extra line says, ahead of everything the job runs afterwards. A workflow passing install-dir through from a reusable-workflow input or anything off github.event is where that stops being theoretical. Checking the version input showed the guard there was not one either: is_tag runs an anchored grep, and grep matches per LINE, so "v0.0.9\nevil" satisfied ^v[0-9]+\.[0-9]+\.[0-9]+$ on its first line and carried the rest through to install.sh. The anchors were never doing the work I assumed. Both now refuse a value containing a newline or carriage return, before anything is downloaded or installed. Neither a release tag nor a directory legitimately spans lines, so nothing valid is turned away. Tests reproduce each vector first — the malformed-version table gains the multi-line case, and a new test drives install-dir — and removing either guard turns them red again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HA6pqDeLuGi6szcbQdkr16
Found reviewing my own diff for the class 4f97451 fixed, rather than from a review comment. Refusing an input is not enough when the refusal echoes it. Both the version and provenance errors quote what they were given, and the runner reads ::workflow commands:: from the start of a line — so `version: "v0.0.9\n::error::forged"` was rejected and the rejection itself wrote the forged command into the job's log. The install-dir path was already safe by accident: its message names the input without quoting the value. All three inputs are now checked for a newline or carriage return before anything downstream can print them, and the refusal names the input rather than repeating what it held. The per-value guards stay where they were, covering what does not come from the workflow: the action ref, gh's answer, RUNNER_TEMP, and the version the installed binary reports. The test drives all three inputs and asserts the forged command never reaches the output; removing the up-front guards puts it back in two of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HA6pqDeLuGi6szcbQdkr16
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces a new supply-chain-sensitive installation path plus changes release/CI workflows, which warrants final human review despite strong test coverage.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
.github/workflows/release.yml:42
- The release workflow's paths filter includes
tools/action/**, which means changing onlytools/action/*_test.gowill cut a new release and force-move thev0tag even though neither the shipped action (action.yml+tools/action/setup.sh) nor the binary changed. That adds release noise and can advance the recommended@v0pin for changes that don't affect consumers.
Consider narrowing the trigger to just the shipped action files (e.g. tools/action/setup.sh and action.yml). Note: this will require updating the corresponding gate in tools/action/manifest_test.go that currently expects tools/action/ to be present.
- 'action.yml'
- 'tools/action/**'
docs/INSTALL.md:28
- This sentence says “One step”, but the example YAML shows two steps (one
uses:step to install, then a separaterun:step to use the tool). Tweaking the wording avoids confusion for readers skimming the page.
One step, and the binary is on the `PATH` for the rest of the job:
- Files reviewed: 11/11 changed files
- Comments generated: 0 new
- Review effort level: Lite
…ests From the fourth review pass on #70, which raised both of these as previously missed rather than as new comments. The paths filter globbed tools/action/**, so editing a _test.go file would cut a release and force-move the v0 tag — advancing the pin consumers resolve through onto a commit whose action is byte-identical. Narrowed to the two files that ship: action.yml and tools/action/setup.sh. The gate in manifest_test.go was too weak to catch it either way, since "tools/action/" is a substring of the narrower path. It now names the shipped files and rejects the glob outright. Also: docs/INSTALL.md said "One step" above an example showing two, the install and a use of it. It now says what it meant — add one step, and the binary is there for every step after it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HA6pqDeLuGi6szcbQdkr16
The documented use of this tool is a CI gate —
sync --dry-runexiting 4,lintin a pipeline,auditacross a fleet — and nothing said how the binary reaches the runner. The two routes that existed both misfit:go installinstalls a Go toolchain to compile a dependency-free binary that was already built six ways at release time, and a hand-rolledcurl | shstep verifies build provenance only on a runner whereghhappens to be signed in.The download stays in one place
setup.shdownloads nothing.install.shalready fetches, checksums and attests the archive, is already shellcheck'd, and is already gated bytools/supplychain; a second download here would be a second supply-chain surface for the same bytes.setup.shresolves which release to ask for and hands off, then refuses to export a binary that is not the build the job asked for.Provenance is verified by default rather than warned about: hosted runners ship
gh, and the action hands it a token, so this is the one place the check can always run.provenance: requiremakes it fatal even whereghis missing.A pin is a pin
The action ships from this repository, so
@v0.0.29reads as a version pin. Resolving it to the newest release would make it a pin in name only — the version someone writes down during an incident, naming a build they never ran. A full-version ref installs exactly that release;@v0takes the newest; an explicitversion: latestfloats regardless of the ref.That has a consequence for releases, so
release.ymlnow:v0tag the recommended pin resolves through, after the release publishes — without it every consumer on@v0freezes at the commit it first pointed at;action.ymlortools/action/setup.shchanges, since an action fix that cut no release would never reach that tag. Those two files and no glob over the package: its tests ship nothing, and releasing for them would advance the pin onto a commit whose action is byte-identical.v0does not exist yet. It is created by the first release after this merges, souses: ...@v0starts working then. The CI job below avoids the dependency by usinguses: ./.Testing
Both seams, because they fail differently.
tools/action/*_test.go(24 tests)setup.shagainst a stubinstall.sh: version resolution (explicit, action-ref-derived,gh-resolved,install.sh-resolved, and explicitlatest), provenance passthrough, thePATH/outputs export, and every refusal — malformed version, unknown provenance mode, Windows runner, failed install, a build that names no release, a build that is not the one requested, and any input spanning lines.ci.ymljobactionubuntu-latestandmacos-latest, withprovenance: require, then asserts a pinnedv0.0.9installs exactly that build.Every load-bearing assertion was mutation-tested: removing the check it guards turns it red again. That covers the installed-vs-requested check, the action-ref default, the
PATH-after-verify ordering, the release-tag check on the installed build, thelatest-beats-the-ref branch, and both line-spanning guards.Static gates live alongside them: the manifest must pass every
INPUT_*the script reads (a composite action passes nothing implicitly, and a missing mapping is silent), the step must not ask for a shell it never uses, anyuses:inaction.ymlmust be SHA-pinned since it executes in the consumer's job with the consumer's token, andrelease.ymlmust move the major tag and trigger on the action's shipped files without globbing its tests.Locally green:
go test -race ./...,shellcheck,actionlint, andmake docswith no diff.Review
Eight findings, all real, all fixed — six from Copilot, two from following its threads into code it hadn't flagged. Four were the same underlying mistake, worth naming for the reviewer: validation that was one layer shallower than it looked.
shell: bashfor a step whose only command issh— fails on minimal images that carry/bin/shand no bash.devand announce it as the installed version.version: latestfell through to the same branch as an unset input, so a workflow pinned at@v0.0.9asking forlatestsilently gotv0.0.9.install-dirreached the line-delimited$GITHUB_PATH, so a newline forged aPATHentry ahead of every later step.is_tag's anchored grep matches per line, sov0.0.9\nevilsatisfied^v[0-9]+\.[0-9]+\.[0-9]+$and carried the rest through. The anchors were never doing the work.::workflow commands::from the start of a line — so rejecting a crafted input was itself the injection.tools/action/**, releasing for test-only changes.docs/INSTALL.mdsaid "One step" above a two-step example.🤖 Generated with Claude Code
https://claude.ai/code/session_01HA6pqDeLuGi6szcbQdkr16