ci: run shared Scorecard scans on pull requests - #745
Conversation
|
Warning Review limit reachedNext included review available in 52 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🧰 Additional context used🪛 GitHub Check: SonarCloud Code Analysis.github/workflows/scorecard-reusable.yml[warning] 99-100: Not enforcing HTTPS here might allow for redirections to insecure websites. Make sure it is safe here. [warning] 114-114: Not enforcing HTTPS here might allow for redirections to insecure websites. Make sure it is safe here. 🪛 zizmor (1.29.0).github/workflows/scorecard.yml[warning] 27-27: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment (undocumented-permissions) .github/workflows/scorecard-reusable.yml[warning] 23-23: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment (undocumented-permissions) [warning] 67-67: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment (undocumented-permissions) 🔇 Additional comments (2)
📝 SummarySummary by CodeRabbit
WalkthroughThe Scorecard workflows now run on pull requests and pushes to ChangesScorecard workflow execution
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change adds pull-request Scorecard analysis with scoped permissions and SARIF reporting while retaining default-branch publication behavior. No merge-blocking risk is currently identified. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
…e ruleset The "Shared scanner content safety" ruleset requires a Scorecard code-scanning analysis on the PR's head or merge commit. actions/checkout on a pull_request event checks out refs/pull/N/merge, which GitHub regenerates whenever main moves, so the analysis binds to a merge commit that stops matching before the PR can merge. Checking out github.event.pull_request.head.sha instead binds the upload to the head commit, which is stable for the life of the push. codeql-action's getRef() rewrites refs/pull/N/merge to refs/pull/N/head when HEAD differs from GITHUB_SHA, so the upload lands on the head SHA without any further change.
The "Shared scanner content safety" ruleset evaluates code scanning per (tool, category). scorecard-action forces Local mode on pull_request events, so a PR upload carries only supply-chain/local while the scheduled main run carries branch-protection and online-scm as well. Every PR therefore fails with "expecting 2 results from Scorecard". Local proof with the v5.5.0 CLI: a repo-mode scan at the PR head SHA skips the repo-state checks (2 categories, no branch-protection); only a HEAD scan yields all 3. So the PR job now installs the pinned scorecard CLI (release tarball, sha256-verified), runs exactly the seven repo-level checks at repository HEAD with the policy file taken from scorecard-action's pinned SHA, and appends those two runs to the scorecard-action SARIF before the existing upload. The local category remains the scorecard-action measurement of the checked-out PR tree. The step fails unless exactly three categories result. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
SonarCloud (githubactions:S6506) flags curl -L without a protocol restriction: a redirect could in principle land on plain HTTP. Both downloads are checksum-verified afterwards, so a downgrade could not smuggle content in, but there is no reason to permit one at all. Restrict the initial request and any redirect to HTTPS. Verified locally with curl 8.14: both URLs still fetch and both sha256 checks still pass under the restricted protocol set. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
…tia-scanner-in-governance-reusable Bring the Scorecard head-sha fix (#745) into this branch so the required 'scorecard / Run Scorecard PR' context can be produced on this PR's head.



Fleet's required Scorecard merge rule has no PR analysis: consumers call the shared publisher only on schedule, and its current implementation always attempts public publication. Add a PR-only analysis job that uploads actual SARIF without OIDC or publication to the public Scorecard dataset. Keep the existing publishing job and its name for non-PR events, with public publication restricted to the default branch.
Exercise the reusable workflow from standards itself on pull requests and main pushes. Scope write permissions to the calling/publishing jobs and retain locked action commits, bounded jobs, credential-free checkouts and SARIF evidence. Do not suppress scanner failures or lower security thresholds.
Validation: actionlint passes for both workflows. The repository's authoritative lock gate passes. The full local shell suite ran 39 test files: 37 passed initially; the two Mustfile-related failures identified inline pinning drift in the new steps and both pass after the pins were corrected. The same real OpenSSF PR scan and upload were exercised successfully in gitbot-fleet run 34101523253; the shared invocation still requires this PR's CI. OpenSSF labels its pull_request trigger experimental.
This is a prerequisite for restoring gitbot-fleet#512 to the centrally managed publisher after diagnosing its missing contents permission and a missing transitive runtime lock entry. Fleet's SARIF scan also exposed Pinned-Dependencies alerts for runtime-locked symbolic refs; those require evidence-based disposition or inline pin alignment, not blanket dismissal.
The shared PR scan and upload now pass in run 34102303989. Scorecard alert 830 was triaged as a false positive: security-events: write is scoped to the canonical scanner caller and is required for its SARIF upload. The reusable PR job has no OIDC or contents-write permission and runs only pinned actions. No rule threshold was changed.