Skip to content

fix(gha): use sound condition gating for latest-tag step#40035

Merged
hainenber merged 1 commit into
masterfrom
chore/gha-latest-tag-sound-condition
May 12, 2026
Merged

fix(gha): use sound condition gating for latest-tag step#40035
hainenber merged 1 commit into
masterfrom
chore/gha-latest-tag-sound-condition

Conversation

@rusackas
Copy link
Copy Markdown
Member

SUMMARY

The if: on the Run latest-tag step in .github/workflows/latest-release-tag.yml used a ${{ }} template interpolation inside an expression context — the zizmor unsound-condition pattern. The expression is template-substituted before it is parsed, so the value of steps.latest-tag.outputs.SKIP_TAG is spliced in literally:

if: (! ${{ steps.latest-tag.outputs.SKIP_TAG }} )

becomes one of:

  • (! true ) → expression parses to false → step skipped ✓
  • (! false ) → expression parses to true → step runs ✓
  • (! )malformed expression when SKIP_TAG is empty/undefined → GitHub Actions treats it as falsy → step skipped ✗

That third case is reachable in practice. scripts/tag_latest_release.sh has a "no latest tags yet" path (scripts/tag_latest_release.sh:126-131) that calls run_git_tag and exits without ever writing SKIP_TAG to $GITHUB_OUTPUT. Under the workflow's --dry-run invocation, run_git_tag is a no-op, so the script exits with SKIP_TAG unset. The buggy condition then evaluates (! ) and skips the tag step on the very first release, exactly when it most needs to run.

This PR replaces the interpolated condition with a sound direct comparison:

if: steps.latest-tag.outputs.SKIP_TAG != 'true'

which:

  • evaluates correctly in all cases ('', 'false', 'true'),
  • matches the script's intent (skip only when SKIP_TAG is explicitly "true"),
  • mirrors the idiomatic GHA pattern used elsewhere in this repo.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — CI/workflow change only.

TESTING INSTRUCTIONS

GitHub Actions workflow conditions can't be unit-tested in the repo's test suite. The fix can be reasoned about against the GHA expression docs and validated by inspection of scripts/tag_latest_release.sh paths:

SKIP_TAG value Old condition (! ${{ ... }} ) New condition ... != 'true' Intent
"true" (equal / older release) (! true )false → skip 'true' != 'true'false → skip skip
"false" (newer release) (! false )true → run 'false' != 'true'true → run run
"" (no-latest-tags path) (! ) → malformed → skip '' != 'true'true → run run

ADDITIONAL INFORMATION

The same workflow has a separate finding at line 23:

source ./scripts/tag_latest_release.sh $(echo ${{ github.event.release.tag_name }}) --dry-run

${{ github.event.release.tag_name }} is interpolated directly into a shell command — a zizmor template-injection class issue. That is a distinct fix (move the tag name to an env: var and reference it as "$TAG_NAME") and is intentionally out of scope for this PR.

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
  • Introduces new feature or API
  • Removes existing feature or API

The `if:` on the `Run latest-tag` step used a `${{ }}` template
interpolation inside an expression context, which is the zizmor
"unsound-condition" pattern. The expression is template-substituted
*before* it is parsed, so when `steps.latest-tag.outputs.SKIP_TAG` is
empty (which happens on the "no latest tags yet" path of
`scripts/tag_latest_release.sh`, where `SKIP_TAG` is never written to
`$GITHUB_OUTPUT` before the script exits), the condition becomes the
malformed expression `(! )` and GitHub Actions evaluates it as falsy —
so the tag step is skipped on the very first release when it should
run.

Replace with a direct string comparison
`steps.latest-tag.outputs.SKIP_TAG != 'true'`, which:
- evaluates soundly in all cases (empty, "false", "true"),
- matches the script's intent (skip only when SKIP_TAG is explicitly
  "true"), and
- mirrors how other workflows in this repo gate on step outputs.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 11, 2026

Bito Automatic Review Skipped - Files Excluded

Bito didn't auto-review this change because all changed files are in the exclusion list for automatic reviews. No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change the excluded files settings here, or contact your Bito workspace admin at evan@preset.io.

@github-actions github-actions Bot added the github_actions Pull requests that update GitHub Actions code label May 11, 2026
@netlify
Copy link
Copy Markdown

netlify Bot commented May 11, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit feea615
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a022a16fa38c80008e5e0d5
😎 Deploy Preview https://deploy-preview-40035--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@hainenber
Copy link
Copy Markdown
Contributor

Unrelated but we should probably integrate zizmor in another PR to up the project's CI security posture

@hainenber hainenber merged commit 658907a into master May 12, 2026
62 of 63 checks passed
@hainenber hainenber deleted the chore/gha-latest-tag-sound-condition branch May 12, 2026 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github_actions Pull requests that update GitHub Actions code size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants