fix(security): codeql v4, SLSA provenance attestation, remove admin bypass#157
Conversation
- scorecard.yml: upgrade github/codeql-action/upload-sarif from v3 to v4 (v3 deprecated December 2026; SHA 7211b7c already used in security.yml) - release-consolidated.yml: add actions/attest-build-provenance@v4.1.0 to build-release job so every platform binary gets a SLSA provenance attestation on release; fixes Scorecard Signed-Releases score (0/10 → improving) - merge-own-pr.sh: extend to also relax/restore the "Protect main" ruleset (id 15969824) alongside the legacy branch-protection API; strict restore now sets bypass_actors: [] so admin bypass is removed after each merge, fixing Scorecard Branch-Protection "apply to administrators" warning Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reviewer's GuideUpdates security-related automation: aligns Scorecard CodeQL upload to v4, adds SLSA build-provenance attestations to release builds, and extends the merge helper script to temporarily relax and then strictly restore both legacy branch protection and the main-branch ruleset (removing admin bypass). Sequence diagram for updated merge-own-pr.sh branch protection handlingsequenceDiagram
actor Owner
participant merge_own_pr_sh as merge-own-pr.sh
participant GitHub_API
Owner->>merge_own_pr_sh: ./merge-own-pr.sh PR_NUMBER
activate merge_own_pr_sh
merge_own_pr_sh->>GitHub_API: PUT repos/docdyhr/batless/branches/main/protection (RELAXED_BP)
merge_own_pr_sh->>GitHub_API: PUT repos/docdyhr/batless/rulesets/15969824 (RELAXED_RULESET)
merge_own_pr_sh->>GitHub_API: PUT repos/docdyhr/batless/pulls/PR_NUMBER/merge (merge_method=squash)
merge_own_pr_sh->>GitHub_API: PUT repos/docdyhr/batless/branches/main/protection (STRICT_BP)
merge_own_pr_sh->>GitHub_API: PUT repos/docdyhr/batless/rulesets/15969824 (STRICT_RULESET, bypass_actors=[])
deactivate merge_own_pr_sh
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:
- The ruleset payloads in
merge-own-pr.shduplicate most fields betweenSTRICT_RULESETandRELAXED_RULESET; consider factoring out shared structure or generating them from a single template to reduce drift when the ruleset needs to change. - Both
REPOandRULESET_IDare hard-coded inmerge-own-pr.sh; adding a short comment on how to obtain/update the ruleset ID (or parameterizing these via environment variables) would make the script easier to maintain if the repo or ruleset changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The ruleset payloads in `merge-own-pr.sh` duplicate most fields between `STRICT_RULESET` and `RELAXED_RULESET`; consider factoring out shared structure or generating them from a single template to reduce drift when the ruleset needs to change.
- Both `REPO` and `RULESET_ID` are hard-coded in `merge-own-pr.sh`; adding a short comment on how to obtain/update the ruleset ID (or parameterizing these via environment variables) would make the script easier to maintain if the repo or ruleset changes.
## Individual Comments
### Comment 1
<location path=".github/scripts/merge-own-pr.sh" line_range="101" />
<code_context>
echo "Restoring branch protection..."
echo "$STRICT_BP" | gh api --method PUT "repos/$REPO/branches/main/protection" --input - > /dev/null
- echo "Branch protection restored (1 required review)."
+ echo "$STRICT_RULESET" | gh api --method PUT "repos/$REPO/rulesets/$RULESET_ID" --input - > /dev/null
+ echo "Branch protection restored (1 required review, no admin bypass)."
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Restoring strict settings overwrites the entire ruleset JSON, which may drop unrelated future changes
Because this uses `PUT` with the full JSON body, it will overwrite the entire ruleset. Any future changes made in the GitHub UI or via other tools (new rules, conditions, `bypass_actors`, etc.) would be removed when this script runs. To avoid that, either fetch the current ruleset and apply a minimal patch (only toggling the needed settings) or treat this script as the sole owner of that ruleset and document that it shouldn’t be edited elsewhere.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| echo "Restoring branch protection..." | ||
| echo "$STRICT_BP" | gh api --method PUT "repos/$REPO/branches/main/protection" --input - > /dev/null | ||
| echo "Branch protection restored (1 required review)." | ||
| echo "$STRICT_RULESET" | gh api --method PUT "repos/$REPO/rulesets/$RULESET_ID" --input - > /dev/null |
There was a problem hiding this comment.
issue (bug_risk): Restoring strict settings overwrites the entire ruleset JSON, which may drop unrelated future changes
Because this uses PUT with the full JSON body, it will overwrite the entire ruleset. Any future changes made in the GitHub UI or via other tools (new rules, conditions, bypass_actors, etc.) would be removed when this script runs. To avoid that, either fetch the current ruleset and apply a minimal patch (only toggling the needed settings) or treat this script as the sole owner of that ruleset and document that it shouldn’t be edited elsewhere.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR strengthens repository security and supply-chain posture by updating CodeQL SARIF upload to CodeQL Action v4, adding SLSA provenance attestations for release artifacts, and tightening branch protection by removing admin bypass from the main branch ruleset restore flow.
Changes:
- Update Scorecard workflow to use
github/codeql-action/upload-sarifv4 (pinned SHA) to address CodeQL deprecation warnings. - Add
actions/attest-build-provenanceto the release build matrix to generate SLSA provenance attestations for packaged binaries. - Extend
merge-own-pr.shto relax/restore both legacy branch protection and the “Protect main” ruleset, restoring withbypass_actors: []to remove admin bypass.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/scorecard.yml | Bumps SARIF upload action to CodeQL Action v4 (pinned SHA). |
| .github/workflows/release-consolidated.yml | Adds job permissions and build provenance attestation for release artifacts. |
| .github/scripts/merge-own-pr.sh | Adds ruleset relax/restore alongside legacy protection; aims to eliminate admin bypass on restore. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| restore() { | ||
| echo "Restoring branch protection..." | ||
| echo "$STRICT_BP" | gh api --method PUT "repos/$REPO/branches/main/protection" --input - > /dev/null | ||
| echo "Branch protection restored (1 required review)." | ||
| echo "$STRICT_RULESET" | gh api --method PUT "repos/$REPO/rulesets/$RULESET_ID" --input - > /dev/null | ||
| echo "Branch protection restored (1 required review, no admin bypass)." |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 293afed12a
ℹ️ 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".
| - name: Attest build provenance | ||
| uses: actions/attest-build-provenance@a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32 # v4.1.0 | ||
| with: | ||
| subject-path: "batless-${{ matrix.target }}.*" |
There was a problem hiding this comment.
Publish provenance as a release asset
If the goal is to fix the Scorecard Signed-Releases result, this only uploads the provenance to GitHub's Attestations API and never attaches a signature/provenance file to the GitHub release. Scorecard's Signed-Releases check looks for signature/provenance filenames in the release assets (for example *.intoto.jsonl, *.sigstore.json, *.asc), while the later release step still uploads only batless-*/batless-*, so the next release will still have no asset that the check can count.
Useful? React with 👍 / 👎.
Summary
Three Scorecard issues fixed in one pass:
CodeQL deprecation warning —
scorecard.ymlwas the only workflow still oncodeql-actionv3 SHA. Updated to the v4 SHA already used everywhere else insecurity.yml.Signed-Releases (0/10) —
release-consolidated.ymlbuild-releasejob now callsactions/attest-build-provenance@v4.1.0immediately after packaging each platform binary, generating SLSA provenance attestations stored in the GitHub Attestations API. Scorecard will detect these on the next release.Branch-Protection "apply to administrators" (8→9/10) —
merge-own-pr.shextended to also relax/restore theProtect mainruleset (id 15969824) alongside the legacy branch-protection API. The strict-restore payload setsbypass_actors: [], so after this PR is merged the admin bypass actor is permanently removed from the ruleset, and Scorecard will no longer see'branch protection settings apply to administrators' is disabled.What won't change
🤖 Generated with Claude Code
Summary by Sourcery
Tighten repository security and supply-chain protections by updating security workflows, adding build provenance attestations, and ensuring branch protection rules fully apply to administrators.
New Features:
Enhancements: