Skip to content

fixed benthos slack notification to trigger on pushing a tag#411

Open
prakhargarg105 wants to merge 1 commit intomainfrom
fix_webhook
Open

fixed benthos slack notification to trigger on pushing a tag#411
prakhargarg105 wants to merge 1 commit intomainfrom
fix_webhook

Conversation

@prakhargarg105
Copy link
Contributor

fix benthos slack notification to trigger on pushing a tag since we don't do releases

text:
type: "mrkdwn"
text: "${{ github.event.release.body }}"
- type: "markdown"
Copy link

Choose a reason for hiding this comment

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

Bug: type: "markdown" is not a valid Slack Block Kit block type. The valid block types are header, section, divider, image, actions, context, input, file, and rich_text.

The original code correctly used a section block with mrkdwn text. This should be changed back to a section block with a nested text object of type: "mrkdwn", otherwise the changelog content will not render in the Slack notification (Slack silently drops unknown block types).

Comment on lines +52 to +54
echo "notes<<EOF" >> $GITHUB_OUTPUT
echo "$NOTES" >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT
Copy link

Choose a reason for hiding this comment

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

Security: GITHUB_OUTPUT delimiter injection. Using a static EOF delimiter means that if CHANGELOG.md happens to contain a line that is exactly EOF, the output is prematurely terminated and subsequent lines are injected as arbitrary workflow output variables.

Use a random delimiter instead, e.g.:

DELIMITER=$(openssl rand -hex 16)
echo "notes<<${DELIMITER}" >> $GITHUB_OUTPUT
echo "$NOTES" >> $GITHUB_OUTPUT
echo "${DELIMITER}" >> $GITHUB_OUTPUT

@claude
Copy link

claude bot commented Mar 25, 2026

Commits

  1. a1fbdf1 — "fixed benthos slack notification to trigger on pushing a tag since benthos doesn't have releases": Message must use imperative mood and sentence-case for plain format. Should be e.g. Fix benthos slack notification to trigger on tag push (capitalize first word, use "fix" not "fixed").

Review
Workflow changed from release-triggered to tag-push-triggered Slack notification, with changelog extraction from CHANGELOG.md and git-log fallback.

  1. Bug — invalid Slack block type (line 69): type: "markdown" is not a valid Slack Block Kit block type. Should be a section block with mrkdwn text, matching the original code. The changelog content will not render as-is.
  2. Security — GITHUB_OUTPUT delimiter injection (lines 52-54): Static EOF delimiter can be spoofed by CHANGELOG content. Use a random delimiter instead.

- Replace invalid markdown block type with section/mrkdwn
- Convert markdown headers (### Foo) to Slack bold (*Foo*)
- Disable link unfurling to suppress link previews
text: ":github: View Tag"
emoji: true
url: "${{ github.event.release.html_url }}"
url: "${{ github.server_url }}/${{ github.repository }}/releases/tag/${{ github.ref_name }}"
Copy link

Choose a reason for hiding this comment

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

Bug: This URL points to /releases/tag/${{ github.ref_name }}, which is a GitHub Releases page. Since the PR description states "we don't do releases" (and the workflow was specifically changed to trigger on tag push instead of release), this URL will 404 for tags that don't have a corresponding GitHub Release.

Consider linking to the tag ref instead, e.g. /${{ github.repository }}/tree/${{ github.ref_name }} or the compare view between this tag and the previous one.

@claude
Copy link

claude bot commented Mar 26, 2026

Commits

  1. fix(ci): fix Slack notify payload formatting and link previews — The commit message is misleading. It describes only "payload formatting and link previews" but the actual changes are much broader: switching the trigger from release: published to push: tags, adding a checkout step, adding changelog extraction logic from CHANGELOG.md/git-log, and restructuring the entire Slack payload. The message should reflect the primary change (e.g., triggering on tag push and extracting changelog from CHANGELOG.md).

Review
CI workflow change to switch Slack notification from release-triggered to tag-push-triggered with changelog extraction. One bug found.

  1. Dead link in "View Tag" button (line 85): The URL /releases/tag/${{ github.ref_name }} points to a GitHub Releases page, which will 404 since this repo does not use GitHub Releases (per the PR description).

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.

1 participant