ci: harden GitHub Actions workflows for OpenSSF Scorecard (pin, permissions, zizmor gate) - #3223
Conversation
build-main.yaml and ui-test.yaml (the latter added by the in-tree console
move) landed without following the repo's workflow conventions: no top-level
permissions block and actions referenced by moving tags. Together they zero out
two OpenSSF Scorecard checks the rest of the tree already satisfies:
- Token-Permissions: scored on the weakest workflow, so one workflow with an
undeclared (default-write) top-level token drops the whole check to 0.
- Pinned-Dependencies: the six tag-pinned action refs count as unpinned.
Fix both files (top-level read-only token; job-level packages:write kept where
needed; all action refs pinned to the SHA already used elsewhere in the tree)
and add a durable gate so it cannot recur:
- .github/zizmor.yml enables the two Scorecard-relevant audits (unpinned-uses,
excessive-permissions), clean tree-wide as of this commit. The remaining
audits carry pre-existing debt and are disabled with a TODO to burn down and
re-enable one at a time.
- .pre-commit-config.yaml runs zizmor (offline) on changed workflows.
- .github/workflows/zizmor.yml runs the same audit tree-wide on every PR that
touches a workflow, failing on any reintroduced regression.
Also wire scorecard.yml to an optional SCORECARD_TOKEN secret: the default
GITHUB_TOKEN cannot read branch-protection settings, so the Branch-Protection
check is scored on partial data. The token is a no-op until the secret is added.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Native Go fuzzing over the flag parsers in config.go, asserting the
invariants that matter rather than just no-panic:
- ParseShardIndex round-trips with ShardName (accepted input is the canonical
name of the returned index; index is non-negative).
- ShardName round-trips back through ParseShardIndex for every valid index.
- ParseShardCount: a non-auto success yields count >= 1; auto yields 0.
- ParsePinnedTenants: every entry in a success has a non-empty key mapped to a
valid shard name.
Adds an OSS-Fuzz-detectable fuzzing signal for the OpenSSF Scorecard Fuzzing
check while covering parsers that sit on the sharding hot path.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request hardens the repository's CI/CD pipeline by addressing specific OpenSSF Scorecard findings related to token permissions and dependency pinning. By enforcing stricter security policies and integrating automated gating via zizmor, the changes ensure that future workflow modifications maintain a high security posture. Additionally, new fuzz tests were added to critical configuration parsers to enhance robustness. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR tightens GitHub Actions workflow permissions, pins workflow actions, adds Scorecard token fallback, introduces Zizmor workflow scanning and config, and adds fuzz tests for shard configuration parsing helpers. ChangesCI Workflow Hardening and Zizmor Integration
Estimated code review effort: 2 (Simple) | ~15 minutes Fuzz Tests for Shard Config Parsing
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Events
participant ZizmorWorkflow as zizmor.yml workflow
participant ZizmorTool as zizmor CLI
GitHub->>ZizmorWorkflow: pull_request / push / workflow_dispatch
ZizmorWorkflow->>ZizmorWorkflow: checkout repo (persist-credentials disabled)
ZizmorWorkflow->>ZizmorTool: install pinned zizmor
ZizmorWorkflow->>ZizmorTool: run zizmor --offline .github/workflows/
ZizmorTool-->>ZizmorWorkflow: audit findings (fail on findings)
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces GitHub Actions workflow auditing using zizmor via a new configuration file and a pre-commit hook, alongside comprehensive fuzz tests for the fluxshardoperator configuration parsers. The feedback highlights an issue in the underlying ParsePinnedTenants implementation where keys with trailing spaces are incorrectly allowed, suggesting an additional assertion in the fuzz test to catch this. Additionally, the PR title and release-note block need to be updated to comply with the Repository Style Guide's Conventional Commits requirements.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| for k, v := range pinned { | ||
| if k == "" { | ||
| t.Fatalf("ParsePinnedTenants(%q) produced an empty tenant key", s) | ||
| } | ||
| if _, ok := ParseShardIndex(v); !ok { | ||
| t.Fatalf("ParsePinnedTenants(%q) mapped %q to invalid shard %q", s, k, v) | ||
| } | ||
| } |
There was a problem hiding this comment.
There is an issue in the underlying ParsePinnedTenants implementation where it only trims spaces from the entire pair before splitting on =, rather than trimming the individual key and value. This allows keys with trailing spaces (e.g., "a " from "a =shard0") to be successfully parsed and inserted into the map.
To catch this and ensure keys are properly trimmed, we should add an assertion in the fuzz test to verify that keys do not contain leading or trailing whitespace. Note that you will also need to import "strings" in this test file.
for k, v := range pinned {
if k == "" {
t.Fatalf("ParsePinnedTenants(%q) produced an empty tenant key", s)
}
if strings.TrimSpace(k) != k {
t.Fatalf("ParsePinnedTenants(%q) produced a key %q with leading/trailing spaces", s, k)
}
if _, ok := ParseShardIndex(v); !ok {
t.Fatalf("ParsePinnedTenants(%q) mapped %q to invalid shard %q", s, k, v)
}
}There was a problem hiding this comment.
Good catch, but this is pre-existing behavior in ParsePinnedTenants — this PR only adds the fuzz harness, it doesn't change the parser. It's also benign in practice: the key is a tenant namespace, and Kubernetes namespace names can't contain spaces, so an entry like "a "=shard0 can never match a real namespace and stays inert. The fuzz invariant here deliberately asserts structural validity (non-empty key mapped to a valid shard), not input normalization. Trimming key/value is a reasonable but separate change to config.go with its own regression test — out of scope for this scorecard-hardening PR.
| @@ -0,0 +1,37 @@ | |||
| # zizmor configuration — https://docs.zizmor.sh/configuration/ | |||
There was a problem hiding this comment.
According to the Repository Style Guide (under Commit and PR Requirements), each commit and release-note entry must follow the Conventional Commits format: type(scope): brief description.
The current PR title (ci: harden...) and the release-note block (ci: pin...) are missing the scope. Since ci is a valid scope under Meta, please update them to include the scope, for example:
- PR Title:
ci(ci): harden GitHub Actions workflows... - Release Note:
ci(ci): pin GitHub Actions...
References
- Each commit and release-note block must follow the Conventional Commits format: type(scope): brief description. (link)
There was a problem hiding this comment.
ci: is a valid Conventional Commits header — the scope is optional (type(scope): description, the parenthesized scope may be omitted). Both the PR title (ci: harden…) and the release-note (ci: pin…) already start with a valid type: prefix, so they're compliant as-is. No change needed.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/zizmor.yml (2)
34-49: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo
timeout-minuteson the job.If
zizmorhangs (or pipx install stalls), the job has no bound and will run until the default GitHub Actions job timeout (6 hours), consuming runner minutes unnecessarily for what should be a fast, offline, syntactic check.♻️ Add a bound on job duration
zizmor: name: Audit workflows runs-on: ubuntu-latest + timeout-minutes: 10 permissions: contents: read🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/zizmor.yml around lines 34 - 49, The zizmor audit job is missing a duration cap, so it can run until the default GitHub Actions timeout if the install or scan hangs. Add a job-level timeout to the zizmor job in the workflow so the offline check is bounded; keep the existing steps (`actions/checkout`, `pipx install zizmor==1.26.1`, and `zizmor --offline .github/workflows/`) unchanged.
45-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winzizmor version drift between CI and pre-commit hook.
This workflow installs
zizmor==1.26.1, but.pre-commit-config.yamlpins thezizmor-pre-commithook tov1.22.0. Since the file's own comment states the pre-commit hook is meant to catch the same regressions locally as this tree-wide gate, a stale pre-commit pin could miss checks/behavior added between 1.22.0 and 1.26.1, weakening the "runs on PR diff via pre-commit hook" defense-in-depth story described in the header comment.♻️ Align pre-commit hook version with CI
- repo: https://github.com/zizmorcore/zizmor-pre-commit - rev: v1.22.0 + rev: v1.26.1 hooks: - id: zizmor args: [--offline]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/zizmor.yml around lines 45 - 46, The zizmor version used in the CI workflow is ahead of the version pinned by the pre-commit hook, so align the hook with the workflow to keep local and CI checks equivalent. Update the `zizmor-pre-commit` entry in `.pre-commit-config.yaml` to match the `zizmor` version installed in the `Install zizmor` step, and keep the header comment’s intended “same regressions locally as CI” behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/zizmor.yml:
- Around line 34-49: The zizmor audit job is missing a duration cap, so it can
run until the default GitHub Actions timeout if the install or scan hangs. Add a
job-level timeout to the zizmor job in the workflow so the offline check is
bounded; keep the existing steps (`actions/checkout`, `pipx install
zizmor==1.26.1`, and `zizmor --offline .github/workflows/`) unchanged.
- Around line 45-46: The zizmor version used in the CI workflow is ahead of the
version pinned by the pre-commit hook, so align the hook with the workflow to
keep local and CI checks equivalent. Update the `zizmor-pre-commit` entry in
`.pre-commit-config.yaml` to match the `zizmor` version installed in the
`Install zizmor` step, and keep the header comment’s intended “same regressions
locally as CI” behavior consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 89aaca55-404c-49f7-9574-ca3259529f27
📒 Files selected for processing (7)
.github/workflows/build-main.yaml.github/workflows/scorecard.yml.github/workflows/ui-test.yaml.github/workflows/zizmor.yml.github/zizmor.yml.pre-commit-config.yamlinternal/fluxshardoperator/config_fuzz_test.go
VerdictLGTM with non-blocking notes Two MINOR issues worth resolving before or shortly after merge. Findings[MINOR] The pre-commit hook pins [MINOR]
Claim mismatchesAll claims verified. No non-OK entries.
Caveats
Recommended follow-ups
|
…-test Review feedback (non-blocking): - .pre-commit-config.yaml pinned zizmor at v1.22.0 while the CI gate installs 1.26.1; the zizmorcore/zizmor-pre-commit rev is the zizmor version 1:1, so the gap meant local and CI runs could differ. Align the hook to v1.26.1. - ui-test.yaml's checkout did not set persist-credentials: false, unlike the other two workflows hardened here (build-main.yaml, zizmor.yml). Set it so the token is not left in .git/config for the later pnpm install step to read. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: myasnikovdaniil <60174387+myasnikovdaniil@users.noreply.github.com>
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — the Actions hardening is correct end-to-end: every added action is pinned to a full commit SHA, permissions are least-privilege, and the zizmor gate genuinely fails on findings and is green tree-wide.
Business context: Harden GitHub Actions workflows for OpenSSF Scorecard — pin actions to commit SHAs, tighten workflow permissions to least-privilege, and add a zizmor static-analysis gate on workflow changes.
Verified
- All added action refs are pinned to full 40-char commit SHAs, consistent with the rest of the tree; no
pull_request_targetis introduced; the Scorecard PAT never runs on PRs (triggers are branch_protection_rule / schedule / workflow_dispatch), so there is no fork-exfiltration path; therepo_token: ${{ secrets.SCORECARD_TOKEN || github.token }}fallback is a no-op until the secret exists. - The zizmor gate exits non-zero on findings (checked against a forced finding) and is green across the tree, so it cannot silently pass.
- Job-level permissions override the new top-level block, so no existing job's effective token widens.
Non-blocking follow-ups
.github/workflows/zizmor.yml— thezizmorjob has notimeout-minutes; a hung install/scan would run to the 6h default (concurrency.cancel-in-progressonly cancels superseded runs, not a stuck one). Addtimeout-minutes: 10.internal/fluxshardoperator/config_fuzz_test.go—FuzzParsePinnedTenantsasserts non-empty keys and valid shard values but not key canonicalization;ParsePinnedTenants(config.go:146) trims the whole pair rather than the key, so"a =shard0"yields key"a "(trailing space). Pre-existing quirk in untouched code — if you tighten the invariant to catch it, also trim the key inParsePinnedTenants, otherwise a live fuzz run will surface a crasher..github/workflows/zizmor.yml— the gate triggers only on.github/workflows/**+.github/zizmor.ymlpath changes; if "Audit workflows" is ever made a required status check, PRs touching neither path would be held on a perpetually-missing check. Heads-up only.
The zizmor "Audit workflows" gate (added in #3223) is red on main: five actions/* references in the release workflows from #3017 are pinned to a floating tag instead of a commit SHA (blanket unpinned-uses policy, High severity): - nightly.yaml (x3) actions/checkout@v4 - promote-rc.yaml actions/checkout@v4 - retention.yaml actions/create-github-app-token@v1 Pin each to the SHA the rest of the repo already standardizes on: - actions/checkout -> 34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - actions/create-github-app-token -> d72941d797fd3113feb6b93fd0dec494b13a2547 # v1 `zizmor --offline .github/workflows/` now reports no findings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Reconciles the talos-image-cache conflicts created when #3254 merged to main: keep the Chainsaw _lib/ port (hack/e2e-chainsaw/_lib/talos-image-cache.sh, a superset of #3254's logic + the node-join diagnose helper), drop the BATS-era hack/e2e-apps/talos-image-cache.sh, and keep the _lib/-path versions of the mirror manifest, install bats, and unit test. Also picks up main's zizmor action-pinning (#3223, unblocks pre-commit/zizmor) and Talos v1.13.6 (#3240). Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
What this PR does
Restores the two OpenSSF Scorecard checks that dropped the aggregate score below our target, and adds a durable gate so they cannot silently regress again.
Root cause. 15 of 17 workflows already declare a read-only top-level token and pin their actions by SHA. Two did not —
build-main.yamlandui-test.yaml(the latter added by the in-tree console move) — and that was enough to zero out two checks:0.Fixes
build-main.yaml,ui-test.yaml: add a top-level read-onlypermissionsblock (job-levelpackages: writekept where the build cache needs it) and pin every action to the SHA already used elsewhere in the tree..github/zizmor.yml+.github/workflows/zizmor.yml+.pre-commit-config.yaml: add zizmor as a gate. It runs on the PR diff (pre-commit) and tree-wide on every workflow-touching PR, failing on any reintroduced unpinned action or missing/overly-broad permissions block. Scope is intentionally limited to the two Scorecard-relevant audits (unpinned-uses,excessive-permissions), which are clean tree-wide as of this PR; the remaining audits carry pre-existing debt and are disabled with a documented TODO to burn them down and re-enable one at a time.scorecard.yml: wire an optionalSCORECARD_TOKENsecret. The defaultGITHUB_TOKENcannot read branch-protection settings, so the Branch-Protection check is currently scored on partial data. The token is a no-op until the secret is added by a maintainer.internal/fluxshardoperator/config_fuzz_test.go: native Go fuzz targets for the shard config parsers (round-trip and range invariants, not just no-panic), giving the Scorecard Fuzzing check a real signal on a hot-path parser.Verification
zizmor --offline .github/workflows/→ No findings to report (gate green).go test ./internal/fluxshardoperator/ -run '^Fuzz'passes; a 15s live fuzz ofFuzzParsePinnedTenantsran 10.5k execs with no crashers.Follow-ups (tracked separately, not in this PR): merge Renovate's digest-pinning PRs to lift Pinned-Dependencies further, tighten branch protection on
mainand release branches, upgrade the OpenSSF Best Practices badge, and sign releases.Release note
Summary by CodeRabbit