CI: Fix zizmor security findings in PR-triggered workflows#15788
Merged
kevinjqliu merged 1 commit intoapache:mainfrom Mar 27, 2026
Merged
CI: Fix zizmor security findings in PR-triggered workflows#15788kevinjqliu merged 1 commit intoapache:mainfrom
kevinjqliu merged 1 commit intoapache:mainfrom
Conversation
Contributor
Author
|
All these changes are for workflows triggered in CI, so in this PR we can verify that everything still runs successfully after the change |
- Add persist-credentials: false to all actions/checkout steps (artipacked) - Replace actions/cache with actions/cache/restore (cache-poisoning) - Add conditional actions/cache/save on push events to keep cache fresh - Add enable-cache: false to setup-uv in open-api workflow
kevinjqliu
force-pushed
the
kevinjqliu/more-zizmor
branch
from
f10e625 to
5b27645
Compare
kevinjqliu
commented
Mar 27, 2026
| - run: | | ||
| echo "Using the old version tag, as per git describe, of $(git describe)"; | ||
| - run: ./gradlew revapi --rerun-tasks | ||
| - uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5 |
Contributor
Author
There was a problem hiding this comment.
we should replace actions/cache with gradle/actions/setup-gradle so we dont need to manually configure cache key, tracking in #15789
kevinjqliu
force-pushed
the
kevinjqliu/more-zizmor
branch
from
15590a5 to
5b27645
Compare
stevenzwu
reviewed
Mar 27, 2026
| - name: Install uv | ||
| uses: astral-sh/setup-uv@5a095e7a2014a4212f075830d4f7277575a9d098 | ||
| with: | ||
| enable-cache: false |
Contributor
There was a problem hiding this comment.
this seems fine. open-api workflow runs in ~22s per CI check. So the performance gain from caching is probably not that important. This is simpler and secure.
We can add back cache/restore in the future if this workflow becomes a bottleneck.
This was referenced Mar 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fix security findings reported by zizmor in all 11 workflows that are triggered on
pull_request.Changes
1. Add
persist-credentials: falsetoactions/checkout— fixesartipacked(Medium)Files:
.github/workflows/api-binary-compatibility.yml.github/workflows/codeql.yml.github/workflows/delta-conversion-ci.yml.github/workflows/docs-ci.yml.github/workflows/flink-ci.yml.github/workflows/hive-ci.yml.github/workflows/java-ci.yml(3 jobs).github/workflows/kafka-connect-ci.yml.github/workflows/license-check.yml.github/workflows/open-api.yml.github/workflows/spark-ci.ymlWhy zizmor recommends this:
By default,
actions/checkoutpersists the GitHub token in the local git config (.git/config) of the checked-out repository. If a subsequent step uploads the workspace as an artifact, the token is included, potentially allowing an attacker to extract it and push malicious code. Settingpersist-credentials: falseensures the token is not written to disk after checkout.See: https://woodruffw.github.io/zizmor/audits/#artipacked
2. Replace
actions/cachewithactions/cache/restore+ conditionalactions/cache/save— fixescache-poisoning(High)Files:
.github/workflows/api-binary-compatibility.yml.github/workflows/delta-conversion-ci.yml(2 jobs).github/workflows/flink-ci.yml.github/workflows/hive-ci.yml.github/workflows/java-ci.yml.github/workflows/kafka-connect-ci.yml.github/workflows/spark-ci.ymlWhy zizmor recommends this:
actions/cacheboth restores and saves the cache. In workflows triggered bypull_request, a malicious PR could poison the shared cache by injecting compromised content that is then saved and restored by subsequent trusted runs (e.g., onpushtomain). Theactions/cacheaction has implicit save behavior in its post step that always runs, regardless of job outcome. Splitting intoactions/cache/restore(unconditional) andactions/cache/save(conditional ongithub.event_name == 'push') makes the intent explicit: PRs can only read the cache, while only trusted push events can write to it.See: https://woodruffw.github.io/zizmor/audits/#cache-poisoning
3. Add
enable-cache: falsetoastral-sh/setup-uv— fixescache-poisoning(High)Files:
.github/workflows/open-api.ymlWhy zizmor recommends this:
astral-sh/setup-uvusesactions/cacheinternally when caching is enabled. The same cache-poisoning risk applies: a PR-triggered workflow could save a poisoned uv cache. Disabling the built-in cache eliminates this vector.Scope
Only the 11 workflows triggered on
pull_requestare included in this PR. The remaining 6 workflows (labeler, jmh-benchmarks, publish-iceberg-rest-fixture-docker, publish-snapshot, recurring-jmh-benchmarks, site-ci) are not triggered by PRs and will be addressed separately.Testing
These changes are behavioral no-ops:
persist-credentials: false— no workflow step relies on the persisted git credentialsactions/cache/restore— equivalent toactions/cachewithlookup-only: true(which was already set); thelookup-onlyparameter is removed sincecache/restorenever saves by definition