feat(actions): support explicit commit range verification for non-PR workflows - #38
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Shallow checkouts can omit unmerged commits from verification, and plain branch names fail fallback resolution.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds explicit commit-range signature verification for non-PR workflows.
Changes:
- Adds
head-shaandbase-refinputs. - Implements commit-range verification and shared commit validation.
- Adds tests for the new execution paths.
File summaries
| File | Description |
|---|---|
action.yml |
Defines commit-range inputs and token default. |
verify-signed-commit-authors.mjs |
Implements range resolution, fetching, and verification. |
verify-signed-commit-authors.test.mjs |
Tests PR and commit-range behavior. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 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.
🟡 Changes recommended
Fully qualified branch refs can fail resolution after being fetched into an origin remote-tracking ref.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Ambiguous short base refs can resolve to a same-named tag instead of the intended branch.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The PR head tamper check incorrectly rejects equivalent uppercase SHA input.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/actions/verify-signed-commit-authors/verify-signed-commit-authors.mjs:38
- The tamper check compares SHA text case-sensitively, even though
FULL_SHA_PATTERNaccepts uppercase hexadecimal. A valid uppercase spelling of the PR head is therefore rejected as an override. Compare normalized SHA values so equivalent hashes are accepted.
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
PR head validation contradicts the stated contract, and valid non-branch refs cannot be fetched correctly.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/actions/verify-signed-commit-authors/verify-signed-commit-authors.mjs:175
base-refis documented as a commit ref, andresolveBaseCommitaccepts fully qualified refs, but this fetch path treats every non-SHA as a branch. For example, a missingrefs/tags/v1is fetched asrefs/heads/refs/tags/v1, so a valid tag ref can never be retrieved and range verification fails. Fetch non-branchrefs/*explicitly while preserving the existing branch mappings.
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
| const rawHead = process.env.SIGNED_COMMIT_HEAD_SHA?.trim(); | ||
| if (rawHead) { | ||
| fail( | ||
| 'head-sha cannot be specified on a pull request event; commits are determined from the pull request payload.', | ||
| ); | ||
| } | ||
|
|
||
| const prNumber = validateInteger('pull request number', pr.number); | ||
| const prCommitCount = validateInteger( | ||
| 'pull request commit count', | ||
| pr.commits, | ||
| ); | ||
| const prHeadSha = validateSha('pull request head SHA', pr.head?.sha); | ||
| const prBaseSha = validateSha('pull request base SHA', pr.base?.sha); |
There was a problem hiding this comment.
i've updated the pr description, that is not the desired contract. for pull requests a head sha should not be specified it should come from the pr.
Summary
Adds optional
head-shaandbase-refinputs toverify-signed-commit-authorsso it can verify commit ranges outside ofpull_requestevents (e.g.workflow_dispatch). This enablesmono'sdev-releaseworkflow to verify that all unmerged commits on a branch are signed by allowed team members before building and publishing Docker images.Changes
action.yml:head-shaandbase-refinputs.github-tokendefault to${{ github.token }}and updated its description to note its use for unshallowing and fetching commit history in commit-range mode.verify-signed-commit-authors.mjs:head-shaon apull_requestevent is now rejected outright.base-ref..head-sha. Inputs are validated (40-char hex SHA normalized to lowercase, safe ref pattern, no dash prefix). Ifhead-shais already merged intobase-ref, it verifies ancestor relationship and cleanly passes with zero commits.actions/checkoutdefaultfetch-depth: 1). Automatically unshallows viaoriginifGITHUB_TOKENis present, or fails closed if the repository remains shallow, preventing truncatedgit rev-listhistory from bypassing signature checks.base-refbefore querying Git so unqualified names (e.g.mainorrefs/heads/main) fall back torefs/remotes/origin/mainand cannot be hijacked by same-named tags (refs/tags/main).verifyCommitshelper between PR mode and commit-range mode.verify-signed-commit-authors.test.mjs:head-shaon PR events, unqualified branch resolution (main), qualified branch resolution (refs/heads/main), same-named tag disambiguation, uppercase SHA normalization, shallow repository unshallowing, and failing closed when shallow.