Add OSSF Scorecard workflow to gh-workflows module - #651
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
/cc @inteon |
|
@inteon, this should be ready for review now. |
|
@erikgb are we sure this will not run on private forks? |
That's a good point, but then we have the issue in cert-manager already. I will fix it. |
|
Fixed your remark now, @inteon. PTAL! 🙏 |
There was a problem hiding this comment.
Pull request overview
Adds an OSSF Scorecard GitHub Actions workflow to the gh-workflows module and wires it into the repo so downstream consumers can generate the same workflow into their repositories.
Changes:
- Introduces a new
scorecards.yamlworkflow template undermodules/gh-workflows/base/and a generated copy under.github/workflows/. - Adds a
generate-gh-workflowsmake target to copy the module’s base workflow files into the repository. - Updates Renovate configuration to ignore updates to the generated Scorecards workflow file.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| renovate-config.json5 | Ignores Renovate updates for the generated Scorecards workflow. |
| modules/gh-workflows/base/.github/workflows/scorecards.yaml | Adds the source-of-truth Scorecards workflow template used by the module. |
| modules/gh-workflows/01_mod.mk | Adds a generator target to copy module base workflows into the repo. |
| .github/workflows/scorecards.yaml | Adds the generated Scorecards workflow in the repository root. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| analysis: | ||
| name: Scorecards analysis | ||
| runs-on: ubuntu-latest | ||
| if: github.repository_owner == 'cert-manager' && github.ref_name == github.event.repository.default_branch |
Signed-off-by: Erik Godding Boye <egboye@gmail.com>
|
/cc @wallrj I have addressed the comments from Copilot now. |
wallrj
left a comment
There was a problem hiding this comment.
I reviewed this with a particular eye on whether it could break the private repos that import makefile-modules (e.g. jetstack-charts), and tested adoption locally: I pointed approver-policy's klone.yaml at erikgb/makefile-modules@add-scorecard (88efe0b) and ran make generate. The workflow installs cleanly at .github/workflows/scorecards.yaml, nothing else is touched, and it coexists happily with the repository-base files. So no throwaway PR needed.
Two big-picture observations:
- As far as I can tell no repo currently klones the
gh-workflowsmodule (GitHub code search only finds this repo's own Makefile), so merging this rolls nothing out by itself: each project needs an adoption PR adding the module to itsklone.yaml, and Renovate has nothing to update until then. Worth stating the adoption plan in the description. It also raises the question of whetherscorecards.yamlbelongs inrepository-base, which every project already imports — see inline comment. - Because adoption is opt-in and the job is guarded on
github.repository_owner == 'cert-manager', private jetstack repos can't be broken by this. But that same guard makes the workflow dead weight in any non-cert-manager importer — inline comment on that too.
Inline comments below; none of them are blockers.
with claude fable-5
| .PHONY: generate-gh-workflows | ||
| ## Generate base files in the repository | ||
| ## @category [shared] Generate/ Verify | ||
| generate-gh-workflows: |
There was a problem hiding this comment.
Unlike the govulncheck equivalent, there's no opt-out: the go module wraps its target in ifndef dont_generate_govulncheck. Without one, an importer that doesn't want this workflow can't delete it — make verify regenerates the file and fails forever. Suggest an equivalent dont_generate_gh_workflows guard.
| generate-gh-workflows: | ||
| cp -r $(generate_gh_workflows_base_dir)/. ./ | ||
|
|
||
| shared_generate_targets += generate-gh-workflows |
There was a problem hiding this comment.
Registering this in shared_generate_targets means a repo whose klone-bump PRs don't run make generate-klone && make generate (i.e. anything not using the shared Renovate preset's postUpgradeTasks) goes red on the bump PR: the module arrives but .github/workflows/scorecards.yaml doesn't exist yet, so the generate/verify diff fails until someone runs make generate locally. Worth a line in the adoption instructions.
| ## Generate base files in the repository | ||
| ## @category [shared] Generate/ Verify | ||
| generate-gh-workflows: | ||
| cp -r $(generate_gh_workflows_base_dir)/. ./ |
There was a problem hiding this comment.
Two things here:
- This is now the third copy of the copy-base-dir mechanism, after
generate-baseandgenerate-govulncheck; any fix (e.g. the opt-out above) has to land in three places. Given every project already importsrepository-base, did you consider puttingscorecards.yamlundermodules/repository-base/base/.github/workflows/instead? Zero new make code. - The unconditional copy silently overwrites any pre-existing
scorecards.yamlan importer maintains. That matches the existinggenerate-basebehaviour, so probably fine — just noting it for the adoption PRs.
| fix-pinact: | $(NEEDS_PINACT) | ||
| $(PINACT) run --fix --verify-comment | ||
|
|
||
| generate_gh_workflows_base_dir := $(dir $(lastword $(MAKEFILE_LIST)))/base/ |
There was a problem hiding this comment.
Nit: $(dir ...) already ends with /, so this expands to …/gh-workflows//base/ (and the recipe's $(var)/. adds another). Cosmetic, but it shows up in consumer logs; dropping the explicit / matches repository-base.
| analysis: | ||
| name: Scorecards analysis | ||
| runs-on: ubuntu-latest | ||
| if: github.repository_owner == 'cert-manager' && github.ref_name == github.event.repository.default_branch |
There was a problem hiding this comment.
The govulncheck workflow templates the repository via {{REPLACE:GH-REPOSITORY}} + sed rather than hard-coding the org. With this hard-coded guard, a non-cert-manager importer (private jetstack repos, forks) gets a workflow that triggers on every push to main/master but always skips: permanent "skipped" noise it can't remove without failing make verify. Templating the repo name (or the opt-out suggested in 01_mod.mk) would cover that.
| - name: "Upload to code-scanning" | ||
| uses: github/codeql-action/upload-sarif@7211b7c8077ea37d8641b6271f6a365a22a5fbfa # v4.36.0 | ||
| with: | ||
| sarif_file: results.sarif |
There was a problem hiding this comment.
The owner guard still admits private repos inside the cert-manager org, should any adopt this module. There, publish_results auto-disables, but this SARIF upload needs code scanning, which private repos only have with GitHub Advanced Security — without it this step would fail on every push to the default branch. I haven't verified the exact codeql-action v4 behaviour, so treat as a plausible failure mode; continue-on-error: true on this step would be a cheap belt-and-braces fix.
| # THIS FILE IS AUTOMATICALLY GENERATED. DO NOT EDIT. | ||
| # Edit https://github.com/cert-manager/makefile-modules/blob/main/modules/gh-workflows/base/.github/workflows/scorecards.yaml instead. | ||
|
|
||
| name: Scorecards supply-chain security |
There was a problem hiding this comment.
On the cert-manager manual upgrade you mention in the description: until scorecards.yml is deleted there, both files will run on every push to master — duplicate SARIF uploads and duplicate scorecard publishes. Worth making the deletion part of the same adoption PR so there's no window with both.
| // Exclude files that are sourced from makefile-modules and shouldn't be upgraded in projects using makefile-modules. | ||
| 'make/_shared/**', | ||
| '.github/workflows/govulncheck.yaml', | ||
| '.github/workflows/scorecards.yaml', |
There was a problem hiding this comment.
Two edges to this exclusion, neither blocking:
- It only protects repos that extend this shared preset. An importer with its own Renovate config will get action-pin PRs against the generated file, which the generate/verify check then rejects — recurring churn until it adds its own
ignorePathsentry. - Conversely, a preset-consuming repo with a hand-written
scorecards.yamlthat hasn't adopted the module stops receiving pin bumps for it, so its action pins quietly go stale.
This adds a slightly modified version of https://github.com/cert-manager/cert-manager/blob/6c9253775a067378b6d237982fab105601611901/.github/workflows/scorecards.yml to the gh-workflows module. This should ensure we get code scanning in all cert-manager projects using makefile-modules.
Compared with the original, I have modified the following: