Repository navigation
fix(ci): resolve all zizmor security findings in GitHub Actions workflows - #2427
matthiasbruns merged 11 commits into
Conversation
✅ Deploy Preview for ocm-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdates many GitHub Actions workflows to disable credential persistence on checkout, replace secrets inheritance with explicit GITHUB_TOKEN passing, refactor inline workflow/context expressions into env-driven values, adjust per-module CI test invocations, and rework several release and tooling steps (including a new zizmor security workflow). ChangesWorkflow credential & env refactor
Sequence Diagram(s)(omitted — changes are workflow/config-focused and do not introduce new multi-component runtime control flow warranting a sequence diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
github-actions
Bot
added
component/github-actions
![coderabbitai[bot]](/web-api/iframe-proxy?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/website-publish-site.yaml (1)
103-103:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winResidual
${{ }}interpolation inwith:block is inconsistent with this PR's template-injection remediation.The PR moves inline context expressions out of
run:blocks, butcommit_messageon line 103 still embeds${{ github.event.head_commit.message }}directly as awith:input to a third-party action. Ifpeaceiris/actions-gh-pagespasses this value to a shell internally, a crafted commit message could trigger injection. The trigger is push-only, keeping risk low, but it is architecturally inconsistent with the rest of this PR.Consider the same pattern used elsewhere in this PR — declare the value in
env:and reference it from there:🛡️ Suggested fix
+ env: + COMMIT_MSG: ${{ github.event.head_commit.message }} - name: Publish as GitHub Pages uses: peaceiris/actions-gh-pages@4f9cc6602d3f66b9c108549d475ec49e8ef4d45e `#v4` with: github_token: ${{ steps.generate_token.outputs.token }} publish_dir: ./website/public - commit_message: ${{ github.event.head_commit.message }} + commit_message: ${{ env.COMMIT_MSG }} user_name: 'GitHub Actions Bot' user_email: '<41898282+github-actions[bot]@users.noreply.github.com>'Note: Moving to
env:does not fully eliminate the risk if the action itself shells out with the value unquoted, but it is consistent with the PR's remediation strategy and signals intent to future readers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/website-publish-site.yaml at line 103, The commit_message input still embeds the interpolation `${{ github.event.head_commit.message }}` directly into the `with:` block for the peaceiris/actions-gh-pages action; move that value into an environment variable and reference the env var from `with:` instead to match the PR's template-injection remediation. Specifically, set an env entry (e.g., `COMMIT_MESSAGE: ${{ github.event.head_commit.message }}`) in the workflow job or step, then change the `with:` key `commit_message` to reference the env var (e.g., `commit_message: ${{ env.COMMIT_MESSAGE }}`) so the value is provided via environment rather than inline interpolation. Ensure you update the step that uses peaceiris/actions-gh-pages and keep the variable name consistent across the step and job..github/workflows/release-go-submodule.yaml (2)
163-189:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
persist-credentials: falsewill makegit push origin "$tag"fail at authenticationThe token is removed during post-job cleanup. Set
persist-credentials: falseto opt-out. Whenpersist-credentials: falseis set, no credential helper is configured in git's local config, sogit push origin "$tag"on line 189 has no way to authenticate. TheGITHUB_TOKENenvironment variable in the step env block is consumed by theghCLI but is not automatically used by baregit. All actions/checkout steps that don't need to performgit pushshould havepersist-credentials: false. Workflows that need to push (deploy, etc.) should keep credentials available.Since this job intentionally pushes (tag creation), either:
- Remove
persist-credentials: falsefrom this checkout (the right call — this job needs to push), or- Keep
persist-credentials: falseand pass the token explicitly to the remote URL before pushing.🐛 Option 1 – remove the flag from the release job's checkout
- name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - with: - persist-credentials: false🐛 Option 2 – keep the flag and push via the token URL
git tag -a "$tag" -F .tagmsg - git push origin "$tag" + git remote set-url origin "https://x-access-token:${GITHUB_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" + git push origin "$tag"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-go-submodule.yaml around lines 163 - 189, The checkout step sets persist-credentials: false which removes the git credentials, causing git push origin "$tag" in the Create and Push Tag step to fail; fix by either removing the persist-credentials: false setting from the actions/checkout invocation so the job retains credentials for git pushes (preferred since this job creates tags), or if you must keep persist-credentials: false, before git push configure the remote to include the token (use the step env GITHUB_TOKEN) so git can authenticate (e.g., set origin URL with https://x-access-token:${GITHUB_TOKEN}@github.com/owner/repo.git or similar); update the workflow around the actions/checkout step and the Create and Push Tag step (git push origin "$tag") accordingly.
36-41:⚠️ Potential issue | 🔴 Critical
run:alongsideuses:is invalid –git fetch --tagswill never execute, breaking tag-based version logicEach step is either a shell script that will be executed, or an action that will be run.
uses:andrun:are mutually exclusive keys on a step. Placingrun: git fetch --tagsas a sibling key ofuses: actions/checkout@...is invalid; GitHub Actions will silently ignore therun:field and only execute the action.Without tags being fetched,
git tag --list "${prefix}[0-9]*"on line 68 returns nothing and all releases will compute from version0.0.0, discarding any existing history.The cleanest fix is
fetch-tags: truein thewith:block, which fetches tags even iffetch-depth > 0:🐛 Proposed fix
- name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 with: fetch-depth: 0 + fetch-tags: true persist-credentials: false - run: git fetch --tags🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-go-submodule.yaml around lines 36 - 41, The step currently mixes a `uses: actions/checkout@...` action with a sibling `run: git fetch --tags`, which is ignored; update the checkout step (the block using actions/checkout and keys `fetch-depth`/`persist-credentials`) to fetch tags by adding `fetch-tags: true` to its `with:` inputs so tags are available for subsequent `git tag --list` logic instead of relying on a separate `run` entry.
🧹 Nitpick comments (3)
.github/workflows/website-update-security-txt.yaml (1)
32-37: 💤 Low value
${{ secrets.SECURITY_TXT_READ }}is still interpolated inline in therun:shell.The PR's stated remediation for template-injection findings is to move
${{ … }}expressions out ofrun:blocks and intoenv:variables. This step was skipped here. Whilesecrets.*are trusted values (unlike user-controlledgithub.event.*inputs), migrating to anenv:binding is consistent with the stated approach and avoids the secret being expanded directly into the shell string.♻️ Proposed refactor
- name: Fetch security.txt + env: + SECURITY_TXT_READ: ${{ secrets.SECURITY_TXT_READ }} run: | curl -sSL \ -H "Accept: application/vnd.github+json" \ - -H "Authorization: Bearer ${{ secrets.SECURITY_TXT_READ }}" \ + -H "Authorization: Bearer ${SECURITY_TXT_READ}" \ https://raw.github.tools.sap/sgsc-engineering-and-automation/securitytxt/main/security.txt -o website/static/.well-known/security.txt🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/website-update-security-txt.yaml around lines 32 - 37, The Fetch security.txt step currently interpolates ${{ secrets.SECURITY_TXT_READ }} directly inside the run: shell string; change this to bind the secret in the step's env: (e.g., set an env entry like SECURITY_TXT_READ: ${{ secrets.SECURITY_TXT_READ }}) and update the run: curl command to reference the env var (e.g., use $SECURITY_TXT_READ) so the secret is not expanded inline in the shell command; keep the step name "Fetch security.txt" and the curl invocation but replace the inline interpolation with the env-based variable..github/workflows/publish-helminput-plugin-component.yaml (1)
75-80: 💤 Low valueLGTM — disabling Go module caching is correct for a publish workflow.
The best way to protect the integrity of releases is to avoid using GitHub Actions caching entirely for release workflows. While the PR description mentions PR-triggered workflows specifically, unconditional
cache: falseis the more defensible choice here because this workflow produces published artifacts (OCI components, attestations), and any build run — including a push-triggered one — could be a release candidate.Minor nit:
cache-dependency-path: bindings/go/helm/go.sumon line 79 is now a dead parameter (silently ignored whencache: false). Consider removing it to avoid confusion.🧹 Cleanup: remove the now-redundant `cache-dependency-path`
- name: Setup Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 with: go-version-file: bindings/go/helm/go.mod - cache-dependency-path: bindings/go/helm/go.sum cache: false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-helminput-plugin-component.yaml around lines 75 - 80, Remove the now-redundant cache-dependency-path parameter from the Setup Go step since cache: false makes it a dead/ignored option; locate the step using actions/setup-go (the "Setup Go" step) and delete the cache-dependency-path: bindings/go/helm/go.sum line, leaving cache: false in place to keep caching disabled for this publish workflow..github/workflows/kubernetes-controller.yml (1)
121-126: ⚡ Quick win
cache: falseis unconditional — disables caching for trusted builds too.Both the
buildjob (line 126) and theE2Ejob (line 244) setcache: falsefor all three triggers (pushto main/releases/v**,pull_request, andworkflow_call). The PR description says the intent is to address "PR-triggered" cache-poisoning, but the change removes caching for main-branch pushes andworkflow_callinvocations as well, where there is no untrusted-code execution.Access restrictions provide cache isolation and security by creating a logical boundary between different branches — the cache action first searches cache hits for a key in the branch containing the workflow run; if there are no hits in the current branch, it searches parent/upstream branches — but access is scoped to all workflows across runs of the same branch. The cross-branch contamination risk is real for PR builds (especially given documented "cache blasting" techniques) but does not apply when running on
pushto a trusted branch, where no attacker-controlled code runs.An additional security practice is to avoid caching privileged or sensitive workflows at all (e.g., release workflows). The
publishjob, which is the release-sensitive step, does not even callactions/setup-go, so the caching mitigation doesn't reduce risk there.Making the
cachesetting conditional would satisfy the security requirement on PRs while restoring performance for trusted builds:♻️ Proposed conditional caching for both `build` and `E2E` jobs
-# build job (line 122-126) - name: Setup Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 with: go-version-file: ${{ env.LOCATION }}/go.mod cache-dependency-path: ${{ env.LOCATION }}/go.sum - cache: false + cache: ${{ github.event_name != 'pull_request' }}-# E2E job (line 239-244) - name: Setup Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 with: go-version-file: ${{ env.LOCATION }}/go.mod cache-dependency-path: ${{ env.LOCATION }}/go.sum - cache: false + cache: ${{ github.event_name != 'pull_request' }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/kubernetes-controller.yml around lines 121 - 126, The workflow currently disables the setup-go cache unconditionally (cache: false) in the "Setup Go" step (uses: actions/setup-go...), which removes caching even for trusted builds; change the cache setting to be conditional so caching is enabled for trusted runs but disabled for PRs — replace cache: false with a conditional expression such as cache: ${{ github.event_name != 'pull_request' }} (or a tighter condition like enabling only on push to main/releases), updating the same "Setup Go" step in both the build and E2E jobs so PR-triggered runs remain uncached while trusted runs restore caching.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/cli-release.yml:
- Around line 119-120: The called workflow cli.yml is missing a secrets
declaration for GITHUB_TOKEN in its workflow_call block; add a secrets section
under workflow_call that declares GITHUB_TOKEN (required: true) so the caller
(cli-release.yml) can pass GITHUB_TOKEN into fields like repo-token,
github-token, password and env usage; update the workflow_call block in cli.yml
(look for the workflow_call:, inputs:, outputs: structure) to include secrets:
GITHUB_TOKEN: required: true.
---
Outside diff comments:
In @.github/workflows/release-go-submodule.yaml:
- Around line 163-189: The checkout step sets persist-credentials: false which
removes the git credentials, causing git push origin "$tag" in the Create and
Push Tag step to fail; fix by either removing the persist-credentials: false
setting from the actions/checkout invocation so the job retains credentials for
git pushes (preferred since this job creates tags), or if you must keep
persist-credentials: false, before git push configure the remote to include the
token (use the step env GITHUB_TOKEN) so git can authenticate (e.g., set origin
URL with https://x-access-token:${GITHUB_TOKEN}@github.com/owner/repo.git or
similar); update the workflow around the actions/checkout step and the Create
and Push Tag step (git push origin "$tag") accordingly.
- Around line 36-41: The step currently mixes a `uses: actions/checkout@...`
action with a sibling `run: git fetch --tags`, which is ignored; update the
checkout step (the block using actions/checkout and keys
`fetch-depth`/`persist-credentials`) to fetch tags by adding `fetch-tags: true`
to its `with:` inputs so tags are available for subsequent `git tag --list`
logic instead of relying on a separate `run` entry.
In @.github/workflows/website-publish-site.yaml:
- Line 103: The commit_message input still embeds the interpolation `${{
github.event.head_commit.message }}` directly into the `with:` block for the
peaceiris/actions-gh-pages action; move that value into an environment variable
and reference the env var from `with:` instead to match the PR's
template-injection remediation. Specifically, set an env entry (e.g.,
`COMMIT_MESSAGE: ${{ github.event.head_commit.message }}`) in the workflow job
or step, then change the `with:` key `commit_message` to reference the env var
(e.g., `commit_message: ${{ env.COMMIT_MESSAGE }}`) so the value is provided via
environment rather than inline interpolation. Ensure you update the step that
uses peaceiris/actions-gh-pages and keep the variable name consistent across the
step and job.
---
Nitpick comments:
In @.github/workflows/kubernetes-controller.yml:
- Around line 121-126: The workflow currently disables the setup-go cache
unconditionally (cache: false) in the "Setup Go" step (uses:
actions/setup-go...), which removes caching even for trusted builds; change the
cache setting to be conditional so caching is enabled for trusted runs but
disabled for PRs — replace cache: false with a conditional expression such as
cache: ${{ github.event_name != 'pull_request' }} (or a tighter condition like
enabling only on push to main/releases), updating the same "Setup Go" step in
both the build and E2E jobs so PR-triggered runs remain uncached while trusted
runs restore caching.
In @.github/workflows/publish-helminput-plugin-component.yaml:
- Around line 75-80: Remove the now-redundant cache-dependency-path parameter
from the Setup Go step since cache: false makes it a dead/ignored option; locate
the step using actions/setup-go (the "Setup Go" step) and delete the
cache-dependency-path: bindings/go/helm/go.sum line, leaving cache: false in
place to keep caching disabled for this publish workflow.
In @.github/workflows/website-update-security-txt.yaml:
- Around line 32-37: The Fetch security.txt step currently interpolates ${{
secrets.SECURITY_TXT_READ }} directly inside the run: shell string; change this
to bind the secret in the step's env: (e.g., set an env entry like
SECURITY_TXT_READ: ${{ secrets.SECURITY_TXT_READ }}) and update the run: curl
command to reference the env var (e.g., use $SECURITY_TXT_READ) so the secret is
not expanded inline in the shell command; keep the step name "Fetch
security.txt" and the curl invocation but replace the inline interpolation with
the env-based variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09a7bb0c-c2bf-41ae-8681-807822bc7736
📒 Files selected for processing (24)
.github/workflows/ci.yml.github/workflows/cli-release.yml.github/workflows/cli.yml.github/workflows/conformance.yml.github/workflows/controller-release.yml.github/workflows/jsonschema.yml.github/workflows/kubernetes-controller.yml.github/workflows/markdown.yml.github/workflows/publish-helminput-plugin-component.yaml.github/workflows/publish-ocm-component-version.yml.github/workflows/pull-request.yaml.github/workflows/release-branch.yml.github/workflows/release-candidate-version.yml.github/workflows/release-go-submodule.yaml.github/workflows/renovate.yml.github/workflows/reuse_helper_tool.yaml.github/workflows/sprint-hygiene.yaml.github/workflows/trigger-blackduck-scan.yaml.github/workflows/update-plugin-registry.yaml.github/workflows/website-live-test-install-script.yml.github/workflows/website-manual-update-cli-docs.yaml.github/workflows/website-publish-site.yaml.github/workflows/website-update-security-txt.yaml.github/workflows/website-verify-scripts.yml
frewilhelm
left a comment
frewilhelm
left a comment
There was a problem hiding this comment.
Based on the review / feedback here I will open a PR that will add Zizmor as a Job
i think this would make sense
Skarlso
left a comment
Skarlso
left a comment
There was a problem hiding this comment.
Looks okay aside from what Frederic already said.
matthiasbruns
left a comment
matthiasbruns
left a comment
There was a problem hiding this comment.
Overall very good cleanup and security fixes. I am just uncertain of this passing of the GITHUB_TOKEN
frewilhelm
left a comment
frewilhelm
left a comment
There was a problem hiding this comment.
do you plan to introduce zizmor action as a follow up then?
|
yeah exactly ill do a follow up PR. |
|
you know what, can also add it now... |
2ea654c
2ea654c to
f496d34
Compare
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ci.yml (1)
277-281:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the undefined
matrix.go-versionreferences from step names.Lines 277, 314, and 348 use
${{ matrix.go-version }}in the step name, but those matrices only definemodule. This creates a mismatch where the step name references an undefined variable. Either use a static step name like "Setup Go" or addgo-versionto each matrix if you need it interpolated.🤖 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/ci.yml around lines 277 - 281, The step names use an undefined matrix variable `${{ matrix.go-version }}` (seen in the step name "Setup Go ${{ matrix.go-version }}"), which causes CI interpolation errors because your matrices only define `module`; fix by either removing the interpolation and renaming the step to a static "Setup Go" (update all occurrences of the step name that reference `${{ matrix.go-version }}`) or add a `go-version` entry to each matrix that requires it so the `${{ matrix.go-version }}` reference is defined; apply the same change consistently for every occurrence of "Setup Go ${{ matrix.go-version }}" in the workflow..github/workflows/controller-release.yml (1)
60-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe git push in
create-tag.jswill fail due to missing credentials afterpersist-credentials: false.The
createAndPushTag()function in.github/scripts/create-tag.jsexecutesgit push origin refs/tags/${tag}, but withpersist-credentials: falseon the checkout steps (lines 66 and 255), the token is stripped from git config. Unlike the pattern used in other workflows likerelease-go-submodule.yml, this workflow does not re-inject credentials viagit remote set-url origin https://x-access-token:${TOKEN}@github.com/${REPO}.git.Add a step after checkout to re-inject the app token into the remote URL, or revert
persist-credentials: falsetotrueon the affected checkout steps (since the token here is a scoped app token, not the defaultGITHUB_TOKEN).🤖 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/controller-release.yml around lines 60 - 95, The checkout step sets persist-credentials: false which strips the token needed by the createAndPushTag() call in .github/scripts/create-tag.js (which runs git push), so either restore credentials on the checkout step (set persist-credentials: true) or add an immediate step after Checkout that re-injects the app token into the repo remote (run git remote set-url origin https://x-access-token:${{ steps.get_token.outputs.token }}@github.com/${{ github.repository }}.git using the same token output used elsewhere), ensuring the token is available for createAndPushTag() to push refs/tags/${tag}.
🧹 Nitpick comments (1)
.github/workflows/release-go-submodule.yaml (1)
178-190: 💤 Low valuePush-with-token works, but the token persists in
.git/config.Embedding
GITHUB_TOKENviagit remote set-urlis functional, but it leaveshttps://x-access-token:<token>@github.com/...in the runner's.git/configfor the remainder of the job. Since this is the last step and the runner is ephemeral, the practical risk is low. Optionally, you can avoid persisting the credential by passing it only on the push, e.g.:♻️ Optional: scope the credential to the single push
- git remote set-url origin "https://x-access-token:${GITHUB_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" - git push origin "$tag" + git -c http.extraheader="AUTHORIZATION: bearer ${GITHUB_TOKEN}" \ + push origin "refs/tags/${tag}"
http.extraheaderis the patternactions/checkoutitself uses, and it keeps the token out of.git/configand any URL that might end up in logs/diagnostics.🤖 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/release-go-submodule.yaml around lines 178 - 190, The workflow step "Create and Push Tag" currently exposes GITHUB_TOKEN by calling git remote set-url and leaving the token in .git/config; instead, avoid persisting the credential by passing the token only to the push command (e.g., use git -c http.extraheader or set GIT_ASKPASS/GIT_HTTP_* on the git push invocation) so the repository URL in .git/config is not modified—modify the run block that uses NEW_TAG, CHANGELOG_B64 and the git commands to remove git remote set-url and call git push with a transient credential transport that supplies "Authorization: bearer ${GITHUB_TOKEN}" only for that push.
🤖 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.
Inline comments:
In @.github/workflows/website-update-security-txt.yaml:
- Around line 32-39: The curl invocation in the "Fetch security.txt" step should
fail the job on HTTP 4xx/5xx responses; update the curl command used in that
step (the run block that currently uses curl -sSL ...) to include the
--fail-with-body flag so it exits nonzero on HTTP errors and prevents writing
error responses into website/static/.well-known/security.txt.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 277-281: The step names use an undefined matrix variable `${{
matrix.go-version }}` (seen in the step name "Setup Go ${{ matrix.go-version
}}"), which causes CI interpolation errors because your matrices only define
`module`; fix by either removing the interpolation and renaming the step to a
static "Setup Go" (update all occurrences of the step name that reference `${{
matrix.go-version }}`) or add a `go-version` entry to each matrix that requires
it so the `${{ matrix.go-version }}` reference is defined; apply the same change
consistently for every occurrence of "Setup Go ${{ matrix.go-version }}" in the
workflow.
In @.github/workflows/controller-release.yml:
- Around line 60-95: The checkout step sets persist-credentials: false which
strips the token needed by the createAndPushTag() call in
.github/scripts/create-tag.js (which runs git push), so either restore
credentials on the checkout step (set persist-credentials: true) or add an
immediate step after Checkout that re-injects the app token into the repo remote
(run git remote set-url origin https://x-access-token:${{
steps.get_token.outputs.token }}@github.com/${{ github.repository }}.git using
the same token output used elsewhere), ensuring the token is available for
createAndPushTag() to push refs/tags/${tag}.
---
Nitpick comments:
In @.github/workflows/release-go-submodule.yaml:
- Around line 178-190: The workflow step "Create and Push Tag" currently exposes
GITHUB_TOKEN by calling git remote set-url and leaving the token in .git/config;
instead, avoid persisting the credential by passing the token only to the push
command (e.g., use git -c http.extraheader or set GIT_ASKPASS/GIT_HTTP_* on the
git push invocation) so the repository URL in .git/config is not modified—modify
the run block that uses NEW_TAG, CHANGELOG_B64 and the git commands to remove
git remote set-url and call git push with a transient credential transport that
supplies "Authorization: bearer ${GITHUB_TOKEN}" only for that push.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e9a57b6a-a510-4892-bccc-ec5e2b14ee6b
📒 Files selected for processing (11)
.github/workflows/ci.yml.github/workflows/cli-release.yml.github/workflows/cli.yml.github/workflows/controller-release.yml.github/workflows/kubernetes-controller.yml.github/workflows/publish-helminput-plugin-component.yaml.github/workflows/release-go-submodule.yaml.github/workflows/renovate.yml.github/workflows/website-publish-site.yaml.github/workflows/website-update-security-txt.yaml.github/workflows/zizmor.yml
…lows
- Add persist-credentials: false to all actions/checkout steps (artipacked)
- Add cache: false to actions/setup-go in PR-triggered publish workflows (cache-poisoning)
- Move ${{ expression }} interpolations from run: blocks to env: vars (template-injection)
- Replace softprops/action-gh-release with gh release create (superfluous-actions)
- Replace secrets: inherit with explicit secret mappings where possible (secrets-inherit)
- Suppress pull_request_target findings with inline zizmor: ignore comments (dangerous-triggers)
where the trigger is intentional and no PR code is executed
- Add secrets declarations to workflow_call blocks in conformance.yml and
publish-helminput-plugin-component.yaml
Signed-off-by: Jakob Möller <[email protected]>
On-behalf-of: @SAP <[email protected]>
Cache-poisoning risk only applies to publish-eligible runs (push,
workflow_call, release). On pull_request runs the cache is read-only
from the base branch anyway, so keeping it enabled saves time.
cache: ${{ github.event_name == 'pull_request' }}
Evaluates true (cache on) for PR runs, false (cache off) for all
other triggers where artifacts may be published.
Signed-off-by: Jakob Möller <[email protected]>
On-behalf-of: @SAP <[email protected]>
Cache on PRs or feature branches can be poisoned and then consumed
by subsequent runs on main. Restrict cache to default branch only.
cache: ${{ github.ref_name == github.event.repository.default_branch }}
Signed-off-by: Jakob Möller <[email protected]>
On-behalf-of: @SAP <[email protected]>
Cache-poisoning concern was overstated — Go module cache keys are scoped to go.sum hash and fork PRs cannot write back to base cache. Let actions/setup-go manage caching with its defaults. Signed-off-by: Jakob Möller <[email protected]> On-behalf-of: @SAP <[email protected]>
- release-go-submodule: replace invalid sibling `run:` with `fetch-tags: true` in checkout step; fix git push auth by setting remote URL with token since persist-credentials: false removes credential helper - cli.yml: add missing `secrets: GITHUB_TOKEN` declaration to workflow_call block so callers can pass it explicitly - website-publish-site: move head_commit.message interpolation from `with:` to `env:` for consistency with template-injection remediation - website-update-security-txt: move secret from inline interpolation in `run:` block to `env:` binding On-behalf-of: @SAP <[email protected]> Signed-off-by: Jakob Möller <[email protected]> On-behalf-of: @SAP <[email protected]>
Adds zizmor as a CI job to continuously scan GitHub Actions workflows for security issues. Uses the official zizmor-action with SARIF upload for GitHub Advanced Security integration. Signed-off-by: Jakob Möller <[email protected]> On-behalf-of: @SAP <[email protected]>
Signed-off-by: Jakob Möller <[email protected]> On-behalf-of: @SAP <[email protected]>
Without this flag, curl exits 0 on HTTP 4xx/5xx and writes error HTML into security.txt. The workflow would then open a PR with malformed content. Signed-off-by: Jakob Möller <[email protected]> On-behalf-of: @SAP <[email protected]>
e78f915 to
e103bb7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/zizmor.yml (1)
15-18: 💤 Low valueOptional: drop
contents: readandactions: readfor public repo.Per the official
zizmor-actiondocs, for a public repo onlysecurity-events: writeis needed;contents: readandactions: readare only required for private/internal repos. If this repository is public, these two permissions can be removed to keep the job at minimum privilege. The current setup is harmless but slightly broader than needed.🔧 Proposed cleanup
permissions: security-events: write - contents: read - actions: 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 15 - 18, The permissions block grants extra scopes; remove the unnecessary permissions entries "contents: read" and "actions: read" and keep only "security-events: write" in the permissions mapping so the workflow runs with least privilege for a public repo; update the permissions stanza that currently contains the keys "security-events", "contents", and "actions" to only include "security-events: write"..github/workflows/ci.yml (1)
348-348: 💤 Low valuePre-existing:
matrix.go-versionis undefined in the step name.actionlint flags this here and at lines 277, 314 — the matrix only has a
moduleaxis, so${{ matrix.go-version }}resolves to an empty string and the step renders as "Setup Go ". Cosmetic only and not introduced by this PR; flagging because static analysis surfaced it. Safe to drop the interpolation or replace with${{ matrix.module }}.🤖 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/ci.yml at line 348, The step name currently uses an undefined matrix variable "matrix.go-version" (seen in the step name string "Setup Go ${{ matrix.go-version }}"); update the step name to remove the undefined interpolation or use the existing axis "matrix.module" instead (e.g., "Setup Go ${{ matrix.module }}" or simply "Setup Go") so the rendered step name is not empty and actionlint warnings are resolved—search for the exact step name "Setup Go ${{ matrix.go-version }}" to locate and edit the occurrences..github/workflows/release-go-submodule.yaml (1)
178-190: 💤 Low valueLGTM — token-in-remote-URL pattern is the right mitigation here.
persist-credentials: falseon the checkout prevents the credential helper from being available to all subsequent steps; setting the remote URL with the token only in the push step scopes credential exposure to this single step. The token comes from theactions/create-github-app-tokenstep (not the defaultGITHUB_TOKEN), which is what's required for cross-repo or fine-grained tag pushes.Minor nit: consider unsetting the remote URL afterward (
git remote set-url origin "https://github.com/${GITHUB_REPOSITORY}.git") to avoid leaving the token in.git/configif any later step in this job were ever added — currently this is the last step so it's purely defensive.🤖 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/release-go-submodule.yaml around lines 178 - 190, The job currently sets the remote URL with the token in the "Create and Push Tag" step using the GITHUB_TOKEN env and never restores it, so add a cleanup action after git push to reset the origin URL to the repository URL; specifically, after the git push that uses git remote set-url origin "https://x-access-token:${GITHUB_TOKEN}@github.com/${GITHUB_REPOSITORY}.git", run a command to restore origin to "https://github.com/${GITHUB_REPOSITORY}.git" (i.e., unset the tokenized remote) so the token is not left in .git/config for subsequent steps.
🤖 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.
Inline comments:
In @.github/workflows/cli-release.yml:
- Around line 150-161: Update the "Create RC Release" step so the gh release
create command explicitly targets the repository by adding --repo "${{
github.repository }}" to the command (the step that uses RC_TAG and RC_VERSION
and runs gh release create); also ensure the release_rc job includes a checkout
step if needed so artifacts and context are available, but at minimum add the
--repo flag to the gh release create invocation to make the target repository
deterministic.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 348: The step name currently uses an undefined matrix variable
"matrix.go-version" (seen in the step name string "Setup Go ${{
matrix.go-version }}"); update the step name to remove the undefined
interpolation or use the existing axis "matrix.module" instead (e.g., "Setup Go
${{ matrix.module }}" or simply "Setup Go") so the rendered step name is not
empty and actionlint warnings are resolved—search for the exact step name "Setup
Go ${{ matrix.go-version }}" to locate and edit the occurrences.
In @.github/workflows/release-go-submodule.yaml:
- Around line 178-190: The job currently sets the remote URL with the token in
the "Create and Push Tag" step using the GITHUB_TOKEN env and never restores it,
so add a cleanup action after git push to reset the origin URL to the repository
URL; specifically, after the git push that uses git remote set-url origin
"https://x-access-token:${GITHUB_TOKEN}@github.com/${GITHUB_REPOSITORY}.git",
run a command to restore origin to "https://github.com/${GITHUB_REPOSITORY}.git"
(i.e., unset the tokenized remote) so the token is not left in .git/config for
subsequent steps.
In @.github/workflows/zizmor.yml:
- Around line 15-18: The permissions block grants extra scopes; remove the
unnecessary permissions entries "contents: read" and "actions: read" and keep
only "security-events: write" in the permissions mapping so the workflow runs
with least privilege for a public repo; update the permissions stanza that
currently contains the keys "security-events", "contents", and "actions" to only
include "security-events: write".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: caac0514-4c5e-47b1-ad58-b479dfdc8fe4
📒 Files selected for processing (25)
.github/workflows/ci.yml.github/workflows/cli-release.yml.github/workflows/cli.yml.github/workflows/conformance.yml.github/workflows/controller-release.yml.github/workflows/jsonschema.yml.github/workflows/kubernetes-controller.yml.github/workflows/markdown.yml.github/workflows/publish-helminput-plugin-component.yaml.github/workflows/publish-ocm-component-version.yml.github/workflows/pull-request.yaml.github/workflows/release-branch.yml.github/workflows/release-candidate-version.yml.github/workflows/release-go-submodule.yaml.github/workflows/renovate.yml.github/workflows/reuse_helper_tool.yaml.github/workflows/sprint-hygiene.yaml.github/workflows/trigger-blackduck-scan.yaml.github/workflows/update-plugin-registry.yaml.github/workflows/website-live-test-install-script.yml.github/workflows/website-manual-update-cli-docs.yaml.github/workflows/website-publish-site.yaml.github/workflows/website-update-security-txt.yaml.github/workflows/website-verify-scripts.yml.github/workflows/zizmor.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/sprint-hygiene.yaml
- .github/workflows/renovate.yml
Signed-off-by: Jakob Möller <[email protected]> On-behalf-of: @SAP <[email protected]>
Signed-off-by: Jakob Möller <[email protected]> On-behalf-of: @SAP <[email protected]>
a7b7d97 to
2e4bf7f
Compare
…lows (#2427) <!-- markdownlint-disable MD041 --> #### What this PR does / why we need it - Add persist-credentials: false to all actions/checkout steps (artipacked) - Add cache: false to actions/setup-go in PR-triggered publish workflows (cache-poisoning) - Move ${{ expression }} interpolations from run: blocks to env: vars (template-injection) - Replace softprops/action-gh-release with gh release create (superfluous-actions) - Replace secrets: inherit with explicit secret mappings where possible (secrets-inherit) - Suppress pull_request_target findings with inline zizmor: ignore comments (dangerous-triggers) where the trigger is intentional and no PR code is executed - Add secrets declarations to workflow_call blocks in conformance.yml and publish-helminput-plugin-component.yaml #### Which issue(s) this PR fixes <!-- Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`. --> addresses some findings I found when scanning through the repo with [zizmor](https://docs.zizmor.sh/). Based on the review / feedback here I will open a PR that will add Zizmor as a Job #### Testing Im testing this all on this branch here. ##### Verification - [x] I have added/updated tests for my changes (see [Test Requirements](../CONTRIBUTING.md#test-requirements)) - [x] My changes do not decrease test coverage --------- Signed-off-by: Jakob Möller <[email protected]> 9c818a8
5 participants
What this PR does / why we need it
Which issue(s) this PR fixes
addresses some findings I found when scanning through the repo with zizmor. Based on the review / feedback here I will open a PR that will add Zizmor as a Job
Testing
Im testing this all on this branch here.
Verification