zozo123 commented on code in PR #74173:
URL: https://github.com/apache/airflow/pull/74173#discussion_r4225221915
##########
.github/workflows/ci-image-build.yml:
##########
@@ -321,6 +321,62 @@
docker rmi "${CACHE_FROM_IMAGE}"
shell: bash
if: always() && env.CACHE_FROM_IMAGE != ''
+ - name: "Export mount cache ${{ inputs.platform }}:${{
env.PYTHON_MAJOR_MINOR_VERSION }}"
+ env:
+ PYTHON_MAJOR_MINOR_VERSION: ${{ env.PYTHON_MAJOR_MINOR_VERSION }}
+ run: >
+ breeze ci-image export-mount-cache
+ --cache-file
/tmp/ci-cache-mount-save-v3-${PYTHON_MAJOR_MINOR_VERSION}.tar.gz
+ if: >
+ inputs.upload-mount-cache-artifact == 'true' &&
+ steps.stashed-image.outputs.reusable != 'true'
+ - name: >
+ Stash cache mount ${{ inputs.platform }}:${{
env.PYTHON_MAJOR_MINOR_VERSION }}
+ ${{ inputs.image-stash-ref != '' && format('for ref {0}',
inputs.image-stash-ref) || '' }}
+ uses:
apache/infrastructure-actions/stash/save@61dcea11f19e2bbe1263f14d72235e8da17d3ad0
# save/v1.0.0
+ with:
+ key: "ci-cache-mount-save-v3-${{ inputs.platform }}-${{
env.PYTHON_MAJOR_MINOR_VERSION }}\
+ ${{ inputs.image-stash-ref != '' && format('-{0}',
inputs.image-stash-ref) || '' }}"
+ path: "/tmp/ci-cache-mount-save-v3-${{
env.PYTHON_MAJOR_MINOR_VERSION }}.tar.gz"
+ if-no-files-found: 'error'
+ # A ref's cache is read by the next publish of that same ref, days
rather than hours
+ # later, so it gets the retention the ref's image gets rather than
the branch's.
+ retention-days: ${{ inputs.image-stash-ref != '' && '6' || '2' }}
+ if: >
+ inputs.upload-mount-cache-artifact == 'true' &&
+ steps.stashed-image.outputs.reusable != 'true'
+ # Every job that prepares the CI image would otherwise `docker image
load` the stash below,
+ # unpacking and checksumming each layer again; a copy of the image store
restores with one
+ # extraction. Taken while the freshly built layers are still in the page
cache, and after the
+ # mount cache export, as it drops the build cache that the export reads.
+ # Only PR-scoped snapshots: arbitrary checkout refs in privileged
dispatch/scheduled
+ # workflows must never publish a snapshot into the default branch's
cache scope.
+ - name: "Snapshot CI image ${{ inputs.platform }}:${{
env.PYTHON_MAJOR_MINOR_VERSION }}"
+ id: snapshot-export
+ continue-on-error: true
+ env:
+ PLATFORM: ${{ inputs.platform }}
+ run: >
+ ./scripts/ci/docker_data_root_snapshot.sh create
+
"/mnt/ci-image-snapshot-${PLATFORM//\//_}-${PYTHON_MAJOR_MINOR_VERSION}.tar.zst"
+ shell: bash
+ if: >
+ github.event_name == 'pull_request' &&
+ inputs.upload-image-artifact == 'true' && inputs.image-stash-ref ==
'' &&
+ steps.stashed-image.outputs.reusable != 'true'
+ - name: "Stash CI image snapshot ${{ inputs.platform }}:${{
env.PYTHON_MAJOR_MINOR_VERSION }}"
Review Comment:
The current CodeQL alert on the snapshot step (`ci-image-build.yml:406`) is
this same `actions/cache-poisoning/poisonable-step` rule. It is a false
positive here, and the fix is a dismissal, not a code change:
- **The step can't run on the triggers the alert names.** It runs only when
`github.event_name == 'pull_request'` and `checkout-ref` is empty or
`github.sha` (L419–422). The alert comes from `schedule` and
`workflow_dispatch`. The rule only checks whether the checkout step at L149 is
guarded by an access check it models (actor, author association, label,
permission, repository, environment). It never reads the flagged step's own
`if:`, so even `if: false` on this step keeps the alert (checked locally).
- **It writes no cache.** `create` archives the local Docker data root into
`runner.temp`, and the next step uploads it as an artifact that only this run
downloads (2-day retention). Even a cache written during a `pull_request` run
is scoped to `refs/pull/<n>/merge` and [can't be restored by the base
branch](https://docs.github.com/en/actions/reference/workflows-and-actions/dependency-caching).
- **Main already has this finding on this job.** I ran CodeQL 2.27.2 locally
with the same query. Main (`bf2de26c7b`) has 11 results, including the four
steps right after this checkout (L154–161: `free_up_disk_space.sh`,
`make_mnt_writeable.sh`, `move_docker_to_mnt.sh`, `./.github/actions/breeze`),
which run on every event. This head (`3793fc0ad5`) adds exactly one, the
snapshot step, and it's the only one of the 12 restricted to `pull_request`.
- **Why the inline revision had no alert.** `c16d395b5f` drew no annotation
because its inline `docker`/`git`/`tar`/`zstd` commands aren't in the rule's
model of running checked-out code (`./path` scripts, local actions, build tools
like `make` or `npm`). It wasn't meaningfully safer: on `pull_request` the job
has already run checked-out scripts (L154–161) before this step. Moving the
logic into `docker_data_root_snapshot.sh create`, as asked in review, is what
made the alert visible again.
The only code changes that clear it either go back to the inline version or
dodge the pattern while running the same file. Running the script from a
trusted second checkout still alerts. I don't have Security-tab access, so a
maintainer would need to dismiss it as a false positive.
---
Drafted-by: Claude Code (Opus 5.5) (no human review before posting)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
