Skip to content

fix: release notification secrets context and script injection#50

Merged
abhizipstack merged 2 commits intomainfrom
fix/release-notification-actionlint
Apr 8, 2026
Merged

fix: release notification secrets context and script injection#50
abhizipstack merged 2 commits intomainfrom
fix/release-notification-actionlint

Conversation

@abhizipstack
Copy link
Copy Markdown
Contributor

@abhizipstack abhizipstack commented Apr 8, 2026

What

  • Fix secrets context usage in release notification workflow
  • Fix script injection in run: block
  • Add skip guard on Slack post step

Why

  • secrets context is not available in jobs.<id>.if (only github, inputs, needs, vars allowed) — actionlint was failing
  • Direct ${{ }} interpolation of release event data in run: block is a script injection risk
  • Slack post step should skip cleanly when no webhook is configured

How

  • Moved secrets.SLACK_WEBHOOK_URL check from job-level if to step-level env guard
  • Pass release event data (TAG, RELEASE_NAME, URL) via env: variables instead of inline ${{ }} in shell
  • Added if: ${{ steps.message.outputs.text != '' }} on "Post to Slack" step

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

  • No — workflow-only change. Release notification behavior is unchanged; it now correctly skips when no Slack webhook is configured instead of failing at job-level.

Database Migrations

  • None

Env Config

  • None — uses existing SLACK_WEBHOOK_URL secret

Relevant Docs

  • N/A

Related Issues or PRs

Dependencies Versions

  • None

Notes on Testing

  • Verify pre-commit.ci actionlint passes
  • Verify release notification works when Slack webhook is configured

Screenshots

N/A

Checklist

🤖 Generated with Claude Code

- Move secrets check from job-level if to step-level env (secrets
  context is not available in jobs.<id>.if — only github, inputs,
  needs, and vars are allowed)
- Pass release event data via env variables instead of direct ${{ }}
  interpolation in run block to prevent script injection
- Skip Slack post if no message was built

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@abhizipstack abhizipstack requested a review from a team as a code owner April 8, 2026 12:23
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 8, 2026

Greptile Summary

This PR fixes three security/correctness issues in the .github/workflows/release-notification.yml workflow that were introduced in PR #42:

  1. secrets context in job-level if: — moved the SLACK_WEBHOOK_URL check from job-level (where secrets context is unavailable) to a step-level env: guard, which is the recommended GitHub Actions pattern.
  2. Script injection — replaced inline ${{ github.event.release.* }} interpolation directly in the run: block with env: variables ($TAG, $RELEASE_NAME, $URL), eliminating the injection vector.
  3. Skip guard on "Post to Slack" — added if: ${{ steps.message.outputs.payload != '' }} so the step cleanly skips when the webhook secret is not configured.

The fixes are minimal and well-targeted. One minor discrepancy: the PR description's "How" section says the guard uses steps.message.outputs.text, but the actual code correctly gates on steps.message.outputs.payload — matching what the "Build Slack payload" step actually outputs.

Confidence Score: 5/5

Safe to merge — all changes are workflow-only, correctly fix the reported actionlint failures and script-injection risk, and introduce no new issues.

All three stated fixes are correctly implemented: the secrets-context misuse is resolved via the step-level env guard (a recommended GitHub Actions pattern), release event data is safely passed through env variables and jq --arg to prevent injection, and the Slack post step has a proper skip guard. No P0 or P1 findings remain.

No files require special attention.

Vulnerabilities

No security concerns identified. The PR actively improves security by eliminating a script-injection risk: release event data (tag_name, name, html_url) is now passed through env: variables and then via jq --arg (which escapes values properly), rather than being interpolated directly with ${{ }} in the run: shell block.

Important Files Changed

Filename Overview
.github/workflows/release-notification.yml Correctly fixes secrets-context misuse, script injection via env: variables, and adds a skip guard on the Slack post step. No remaining issues.

Sequence Diagram

sequenceDiagram
    participant GH as GitHub Release Event
    participant Step1 as Build Slack Payload
    participant Step2 as Post to Slack
    participant Slack as Slack Webhook

    GH->>Step1: release published (tag_name, name, html_url)
    Note over Step1: if: env.SLACK_WEBHOOK_URL != ''
    alt SLACK_WEBHOOK_URL secret is set
        Step1->>Step1: env vars TAG, RELEASE_NAME, URL bound from secrets/event
        Step1->>Step1: printf → TEXT, jq --arg → PAYLOAD (JSON-safe)
        Step1-->>Step2: outputs.payload = JSON string
        Note over Step2: if: steps.message.outputs.payload != ''
        Step2->>Slack: POST webhook with payload
    else SLACK_WEBHOOK_URL secret is empty
        Step1-->>Step2: skipped (outputs.payload = '')
        Note over Step2: skipped — no webhook configured
    end
Loading

Reviews (2): Last reviewed commit: "fix: build full JSON payload in shell to..." | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

@wicky-zipstack wicky-zipstack left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@tahierhussain tahierhussain left a comment

Choose a reason for hiding this comment

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

LGTM

Note: Please make sure to address the security issue raised by Greptile.

Use jq to build the Slack payload JSON in the shell step instead of
interpolating untrusted values into the payload block. This ensures
release names with quotes or backslashes produce valid JSON.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@abhizipstack
Copy link
Copy Markdown
Contributor Author

Greptile P1 addressed in 2bc148b — full JSON payload is now built in the shell step using jq which properly escapes all values. No untrusted data is interpolated in the payload: block anymore.

@abhizipstack abhizipstack merged commit 909e79c into main Apr 8, 2026
6 checks passed
@abhizipstack abhizipstack deleted the fix/release-notification-actionlint branch April 8, 2026 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants