Repository navigation
ci: add OpenSSF Scorecard workflow with published results - #406
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughAdds an OpenSSF Scorecard GitHub Actions workflow triggered weekly and on pushes to ChangesOpenSSF Scorecard workflow
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Preview DeploymentPreview URL: https://a14ec94f.bestax.pages.dev |
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 |
There was a problem hiding this comment.
Checkout pin is v7.0.0, not the repo's v7.0.1 — contradicts the PR's own claim — 🟡 Minor · Robustness
What: This pins actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0, which the GitHub API resolves to tag v7.0.0. Every other workflow in the repo (ci.yml, deploy.yml, visual-regression.yml, and ~12 more) pins 3d3c42e5aac5ba805825da76410c181273ba90b1, which is v7/v7.0.1. The PR body states this action "reuses the exact pin already in ci.yml" — it does not; the SHAs differ by one patch release.
Why it matters: Not a supply-chain risk (it's SHA-pinned to a real release), but it's a maintenance drift: the repo now carries two distinct # v7 pins for the same action, and the stated intent to match ci.yml isn't met. This looks like the SHA was copied verbatim from the OpenSSF starter template rather than from the repo.
Fix:
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 | |
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 |
API verification
repos/actions/checkout/git/ref/tags/v7 -> commit 3d3c42e5aac5ba805825da76410c181273ba90b1
repos/actions/checkout/git/ref/tags/v7.0.0 -> commit 9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 (this PR)
repos/actions/checkout/git/ref/tags/v7.0.1 -> commit 3d3c42e5aac5ba805825da76410c181273ba90b1 (ci.yml + rest of repo)
The other three pins (ossf/scorecard-action@2d11466… = v2.4.4, github/codeql-action/upload-sarif@e4fba86… = v4, actions/upload-artifact@043fb46… = v7) all verified correct.
There was a problem hiding this comment.
Deep review — 1 blocking · 2 advisory
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🟡 Minor | Robustness | actions/checkout pins v7.0.0 (9c091bb2), not the repo-wide v7.0.1 (3d3c42e5); PR body's "reuses the exact pin already in ci.yml" is inaccurate |
.github/workflows/scorecard.yml:36 |
| 2 | 🔵 Advisory | Correctness | PR body says top-level permissions: read-all "matches the pattern in ci.yml:13-14" — ci.yml is actually contents: read, not read-all. Cosmetic doc drift; the read-all value itself is correct/recommended for Scorecard |
.github/workflows/scorecard.yml:19 |
| 3 | 🔵 Advisory | Security | Job-level permissions fully replace (not merge with) the top-level read-all, so the analysis job effectively has only security-events: write, id-token: write, contents: read. Correct here (public repo — Scorecard reads public data without actions:read/contents:read token scopes), but if this repo ever goes private the job will silently under-scope |
.github/workflows/scorecard.yml:22-28 |
Overall: The change is sound and low-risk — a single, additive workflow file, correctly structured, with SARIF wired to both an artifact and the Security tab, publish_results correctly paired with id-token: write, a valid cron, and appropriate triggers (no pull_request, as intended). The one blocking item is a cosmetic-but-real pin inconsistency: three of the four SHA pins verified exactly against the GitHub API, but actions/checkout resolves to v7.0.0 rather than the v7.0.1 used everywhere else in the repo — worth aligning both to kill drift and to make the PR body's claim true. The human should focus first on that pin, then confirm the post-merge checklist (workflow runs green on main, results appear on scorecard.dev, SARIF in Security tab) since none of that can be exercised from a PR branch.
Residual risk:
- Wrong/forged SHA pins: refuted — all four dereferenced against the GitHub API;
scorecard-action(v2.4.4),codeql-action/upload-sarif(v4), andupload-artifact(v7) match exactly;checkoutis a real release (v7.0.0), just not the repo's chosen patch. - Missing permission causing a silent green-but-empty run: refuted for the current public repo —
security-events: write+id-token: write+contents: readcover SARIF upload andpublish_results;actions:read/contents:readtoken scopes are only needed on private repos (see advisory #3). - Post-merge behavior: genuinely unverifiable here — the
push/schedule/branch_protection_ruletriggers can't fire from a PR branch, so the "badge has data" outcome rests on the author's post-merge checklist, not on anything provable in this review.
🏄 Clean little set wave, bruh — one board's waxed a patch off from the rest of the quiver (v7.0.0 vs v7.0.1), snap it into line and this one's good to paddle out. Everything else pins solid.
|
Thanks — went through all three. Two are actionable, one I'm pushing back on with evidence. Finding 1 (🟡 Minor, pin drift) — disputing the premiseHalf right: All 16 occurrences across Repo-wide bumps are Dependabot's job ( Finding 2 (🔵 Advisory, PR-body drift) — fixedCorrect, and my error: Finding 3 (🔵 Advisory, job permissions replace rather than merge) — on the record, no changeAgreed on the mechanics and on the conclusion for today. Worth stating the trigger explicitly for whoever reads this later: if this repo ever goes private, the analysis job needs |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Runs the OpenSSF Scorecard analysis weekly, on pushes to main, and on branch protection rule changes. publish_results is required for the public scorecard.dev viewer and the README badge to have any data. No pull_request trigger — experimental upstream and unsupported on forks. All four actions are SHA-pinned per house convention. Refs #404
The pin I used (9c091bb2 / v7.0.0) came from a stale feature branch, not main. origin/main pins 3d3c42e5 in all 16 places. Deep review on #406 was right; my rebuttal was based on grepping the same stale branch.
|
Retracting my earlier pushback on finding 1 — the review was right and I was wrong. Fixed in the latest push. The pin is now How I got it wrongMy rebuttal cited this: That grep ran against the working tree, which had been switched to an unrelated feature branch based on an older Checked properly against
Knock-on fixThe same stale-branch mistake hit #411, which additionally had stale Findings 2 and 3 stand as previously addressed. |
Preview DeploymentPreview URL: https://459684b8.bestax.pages.dev |
|
deep-review: RE-REVIEW — verify the prior finding was properly addressed. Head is now rebased onto main. Your previous review flagged (🟡 Minor) that Verify independently, treating every claim in the PR body and comments as unverified:
|
|
@claude Please re-review this PR to verify that prior review feedback was properly addressed. The Context: your earlier deep review flagged (🟡 Minor) that Please verify independently and treat every claim in the PR body and comments as unverified:
Report anything still wrong. If something is fine, say so briefly rather than inventing findings. |
|
Claude finished @allxsmith's task in 3m 39s —— View job Re-review complete
PR is already merged, so I verified
1. Checkout pin — resolved, verified two ways without needing fresh API calls
All 18 occurrences repo-wide (17 pre-existing + this file) use that exact SHA. That SHA is the same one the first deep review in this thread already dereferenced via the live API and confirmed as 2.
|
|
🎉 This PR is included in version 4.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 5.8.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes part of #404 (the workflow half — the badge lands in the #403 README PR).
What
Adds
.github/workflows/scorecard.yml. Runs the OpenSSF Scorecard analysis weekly, on pushes tomain, and onbranch_protection_rulechanges; publishes results so the public viewer and the README badge have data; uploads SARIF to the Security tab alongside CodeQL.Why
Scorecard is a third-party, independently verifiable rating of exactly the posture this repo already invests in. It turns "we say we're hardened" into a number anyone can check — which is the whole premise of #403.
Notes on the choices
pull_requesttrigger. Experimental upstream and unsupported on forks.publish_results: truewithid-token: write— without both, the badge has no data at all.permissions: read-allat workflow level, with the three write scopes re-declared on the job only. Same least-privilege shape asci.yml:13-14(which usescontents: read);read-allis the value upstream recommends for Scorecard, which needs broad read access to score the repo.# vNcomments, per house convention.actions/checkoutreuses the exact pin already inci.yml;ossf/scorecard-actionv2.4.4 andgithub/codeql-action/upload-sarifv4 tags were dereferenced to their commit SHAs.Expected score
Roughly 7–8 / 10 initially. High on
Pinned-Dependencies,Token-Permissions,Signed-Releases,SAST,Dependency-Update-Tool,Security-Policy. Dragged down byFuzzing(not applicable to a component library — expect 0, ignore it) andCode-Review(Scorecard counts GitHub review approvals; our reviews are AI bots that don't leave approvals).Branch-Protectionshould now score better — required status checks and 1 required approval were applied tomainunder #405.Verification
prettier --checkpassesmain(thepushtrigger can't fire from a PR branch)Merge this before the #403 README PR — the Scorecard badge 404s until this has run on
mainat least once.Summary by CodeRabbit