test: unsigned commit to verify CI check#304
Conversation
Reviewer's GuideAdds a GitHub Actions workflow that enforces all PR commits to be GPG/SSH verified and authored by the PR owner, failing the job with clear error messages when checks fail. Sequence diagram for the new commit verification GitHub ActionsequenceDiagram
actor Developer
participant GitHub
participant Workflow_commit_verification as Commit_Verification_Workflow
participant GitHub_API
Developer->>GitHub: open pull_request to main
GitHub-->>Workflow_commit_verification: trigger verify-commits job
Workflow_commit_verification->>GitHub_API: gh api /repos/{repo}/pulls/{number}/commits
GitHub_API-->>Workflow_commit_verification: list of commits with verification and author
alt [any commit not verified]
Workflow_commit_verification-->>GitHub: ::error::Commit {sha} is not signed
else [commit author mismatch]
Workflow_commit_verification-->>GitHub: ::error::Commit {sha} author does not match PR owner
else [all commits verified and authored by PR owner]
Workflow_commit_verification-->>GitHub: All commits are signed and authored by PR owner
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Requiring every commit in the PR to be authored by the PR owner will block common workflows like maintainers pushing to a contributor’s branch or multiple authors collaborating; consider relaxing this to only enforce signatures or to allow a configurable set of allowed authors.
- The authorship check may fail for merge commits, GitHub UI edits (e.g.,
web-flow), or bot commits where.author.logindiffers from the PR owner; you might want to explicitly skip those cases or filter only non-merge, human-authored commits.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Requiring every commit in the PR to be authored by the PR owner will block common workflows like maintainers pushing to a contributor’s branch or multiple authors collaborating; consider relaxing this to only enforce signatures or to allow a configurable set of allowed authors.
- The authorship check may fail for merge commits, GitHub UI edits (e.g., `web-flow`), or bot commits where `.author.login` differs from the PR owner; you might want to explicitly skip those cases or filter only non-merge, human-authored commits.
## Individual Comments
### Comment 1
<location path=".github/workflows/commit-verification.yml" line_range="28-37" />
<code_context>
+ env:
+ GH_TOKEN: ${{ github.token }}
+ PR_AUTHOR: ${{ github.event.pull_request.user.login }}
+ run: |
+ errors=$(gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/commits" \
+ -q '
+ .[] | .sha[:12] as $s |
+ (if .commit.verification.verified != true
+ then "::error::Commit \($s) is not signed (\(.commit.verification.reason))"
+ else empty end),
+ (if (.author.login // "") != env.PR_AUTHOR
+ then "::error::Commit \($s) author (\(.author.login // "unknown")) does not match PR owner (\(env.PR_AUTHOR))"
+ else empty end)
+ ')
+ if [ -n "${errors}" ]; then
+ echo "${errors}"
+ exit 1
+ fi
+ echo "All commits are signed and authored by ${PR_AUTHOR}."
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider adding shell strict mode to make the verification script more robust.
Without strict mode, failures from `gh api` or `jq` (e.g., missing `gh`, rate limits, malformed JSON) won’t cause the step to fail, and `errors` may end up empty or partial, letting the job appear to succeed. Adding `set -euo pipefail` near the top of the script ensures any unexpected command failure stops the step immediately.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| run: | | ||
| errors=$(gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/commits" \ | ||
| -q ' | ||
| .[] | .sha[:12] as $s | | ||
| (if .commit.verification.verified != true | ||
| then "::error::Commit \($s) is not signed (\(.commit.verification.reason))" | ||
| else empty end), | ||
| (if (.author.login // "") != env.PR_AUTHOR | ||
| then "::error::Commit \($s) author (\(.author.login // "unknown")) does not match PR owner (\(env.PR_AUTHOR))" | ||
| else empty end) |
There was a problem hiding this comment.
suggestion (bug_risk): Consider adding shell strict mode to make the verification script more robust.
Without strict mode, failures from gh api or jq (e.g., missing gh, rate limits, malformed JSON) won’t cause the step to fail, and errors may end up empty or partial, letting the job appear to succeed. Adding set -euo pipefail near the top of the script ensures any unexpected command failure stops the step immediately.
17dcc53 to
709895c
Compare
709895c to
bbd1046
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: SpaceFace02 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Commit verification.
Branch Protection rules must be configured for stricter gating.
Summary by Sourcery
CI: