Skip to content

Harden S3 publish workflows against zizmor findings - #6543

Merged
bmribler merged 1 commit into
HDFGroup:developfrom
mkitti:mkitti/harden-workflows
Aug 4, 2026
Merged

Harden S3 publish workflows against zizmor findings#6543
bmribler merged 1 commit into
HDFGroup:developfrom
mkitti:mkitti/harden-workflows

Conversation

@mkitti

@mkitti mkitti commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Addresses GitHub Actions static-analysis (zizmor / CodeQL "GitHub Advanced Security") findings in the S3 publishing workflows. These are the two workflows that publish release artifacts and documentation to S3, and they carried the bulk of the template-injection findings.

Changes

.github/workflows/publish-release.yml (41 template-injection + 1 artipacked)

  • Hoist every inputs.* / secrets.* / vars.* value used inside run: blocks into job-level env: variables (USE_TAG, FILE_NAME, TARGET_DIR, DRY_RUN, S3_BUCKET, TARGET_PATH) and reference plain shell variables ($USE_TAG, …) instead of interpolating ${{ ... }} directly into the script body. This removes the code-injection vector where a value is expanded into the script before the shell runs.
  • Add persist-credentials: false to the actions/checkout step (artipacked). This workflow does not push to git; it only reads sources/scripts and uses separate AWS credentials.

.github/workflows/publish-branch.yml (1 artipacked)

  • Add persist-credentials: false to the actions/checkout step. Its S3 sync step already used env: variables.

No behavioral change

The same values are used — only sourced from the shell environment instead of direct ${{ }} expansion. if: conditions and action with: inputs are left as-is (those are not shell-injection contexts).

Verification

zizmor (v1.25.2, the pinned version from zizmor.yml) reports no findings on either file after the change.

Scope note

The remaining zizmor findings across other workflows (maven-*, java-*, additional artipacked, and excessive-permissions) are intentionally not included here. Those require per-workflow analysis — e.g. persist-credentials: false would break checkouts used for git push / gh-pages deploys, and minimal permissions: blocks need to be verified against each job's needs — and are better handled as separate, focused follow-ups.

🤖 Generated with Claude Code

Address GitHub Actions static-analysis (zizmor/CodeQL) findings in the
S3 publishing workflows:

- publish-release.yml: hoist every inputs/secrets/vars value used inside
  run blocks into job-level env vars and reference plain shell variables
  ($USE_TAG, $FILE_NAME, $TARGET_DIR, $S3_BUCKET, $TARGET_PATH, $DRY_RUN),
  eliminating 41 template-injection findings. Add persist-credentials:
  false to the checkout (artipacked).
- publish-branch.yml: add persist-credentials: false to the checkout
  (artipacked); its S3 sync already used env vars.

No behavioral change: the same values are used, only via the shell
environment instead of direct ${{ }} expansion into the script body.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mkitti
mkitti requested a review from hyoklee as a code owner July 20, 2026 21:17
Copilot AI review requested due to automatic review settings July 20, 2026 21:17
@github-project-automation github-project-automation Bot moved this to To be triaged in HDF5 - TRIAGE & TRACK Jul 20, 2026
@mkitti

mkitti commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Deconfliction with #6542 (hdf5.tag)

This PR and #6542 both modify the Process documentation step of .github/workflows/publish-release.yml, so they will conflict:

The clean merged result keeps this PR's job-level env: (so #6542's step-level env: FILE_NAME becomes redundant and is dropped) and adds the hdf5.tag check inside the already-hardened step. Regardless of merge order, resolve the Process documentation step to:

       - name: Process documentation
         run: |
           set -euo pipefail
           DOC_FILE="HDF5/${FILE_NAME}.doxygen.zip"
           if [ -f "$DOC_FILE" ]; then
             echo "📚 Processing documentation..."
             unzip -q "$DOC_FILE"
             if [ -d "${FILE_NAME}.doxygen" ]; then
               echo "✅ Documentation extracted successfully"
             else
               echo "❌ Documentation extraction failed"
               exit 1
             fi
+            # The doxygen tag file is published alongside the HTML docs
+            if [ -f "${FILE_NAME}.doxygen/hdf5.tag" ]; then
+              echo "✅ Found: hdf5.tag"
+            else
+              echo "❌ hdf5.tag not found in documentation"
+              exit 1
+            fi
           else
             echo "⚠️  Documentation file not found, skipping..."
           fi

Note: the added check references ${FILE_NAME} from the job-level env: introduced here, so no step-level env: is needed — that is the only substantive difference from #6542's version of the step. Everything else in #6542 (the docs/doxygen/CMakeLists.txt change and the ctest.yml guard) is untouched by this PR and merges without conflict.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the two S3 publishing GitHub Actions workflows against zizmor/CodeQL findings by removing ${{ ... }} expression interpolation from shell run: blocks (mitigating template-injection risks) and by disabling persisted checkout credentials where they are not needed.

Changes:

  • Hoists inputs.*, secrets.*, and vars.* used by shell scripts in publish-release.yml into job-level env: variables and references them via shell variables inside run: blocks.
  • Adds persist-credentials: false to actions/checkout in both publishing workflows to address the artipacked finding.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
.github/workflows/publish-release.yml Moves templated values out of run: blocks into job env: and disables persisted checkout credentials to mitigate template-injection and credential persistence findings.
.github/workflows/publish-branch.yml Disables persisted checkout credentials to reduce unnecessary credential exposure during S3 publishing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Checklist

This PR touches the following areas. Each needs a sign-off
from its listed owners before merging.

✅ All areas have been signed off.

@github-actions
github-actions Bot removed the request for review from glennsong09 July 20, 2026 21:33
@bmribler bmribler self-assigned this Aug 4, 2026
@bmribler
bmribler merged commit fe17c9b into HDFGroup:develop Aug 4, 2026
132 of 134 checks passed
@github-project-automation github-project-automation Bot moved this from To be triaged to Done in HDF5 - TRIAGE & TRACK Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants