Skip to content

ci(vouch): add Slack notification for new vouch requests - #6038

Merged
maruiz93 merged 1 commit into
fullsend-ai:mainfrom
maruiz93:notify-vouch-slack
Aug 11, 2026
Merged

ci(vouch): add Slack notification for new vouch requests#6038
maruiz93 merged 1 commit into
fullsend-ai:mainfrom
maruiz93:notify-vouch-slack

Conversation

@maruiz93

@maruiz93 maruiz93 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds a new GitHub Actions workflow (notify-vouch-slack.yml) that posts to Slack when a new vouch-request discussion is created
  • Follows the same pattern as notify-adr-slack.yml: curl + jq Block Kit payload using the existing SLACK_WEBHOOK_URL secret
  • Includes workflow_dispatch trigger for manual testing

Test plan

  • Tested Slack posting via workflow_dispatch on fork (pre-guard revision — before repository_owner == 'fullsend-ai' was added). The guard was added in response to review feedback; fork-based workflow_dispatch is now correctly skipped by the guard.
  • Verify notification fires when a real vouch-request discussion is created on upstream (post-merge)

🤖 Generated with Claude Code

@maruiz93
maruiz93 requested a review from a team as a code owner August 10, 2026 13:06
@maruiz93
maruiz93 force-pushed the notify-vouch-slack branch from 8b010b7 to 63deb6c Compare August 10, 2026 13:07
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add GitHub Actions workflow to notify Slack on new vouch-request discussions

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Post a Slack message when a new "vouch-request" GitHub Discussion is created.
• Reuse the existing Slack webhook secret with a curl+jq Block Kit payload.
• Add a workflow_dispatch trigger to manually test notifications.
Diagram

graph TD
  gh(("GitHub")) --> evt["Discussion created"] --> gate{"Category = vouch-request?"} --> wf["notify-vouch-slack.yml"] --> build["jq payload"] --> slack(("Slack"))
  user(("Maintainer")) --> dispatch["workflow_dispatch"] --> wf
  subgraph Legend
    direction LR
    _actor(("Actor")) ~~~ _step["Workflow step"] ~~~ _dec{"Decision"}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use slackapi/slack-github-action
  • ➕ Less custom shell scripting (no manual curl/jq)
  • ➕ Built-in Slack API payload handling and better error surfacing
  • ➖ Adds a third-party action dependency and version management
  • ➖ May still require custom Block Kit JSON for formatting parity
2. Centralize Slack notifications via a reusable workflow
  • ➕ Reduces duplication with notify-adr-slack.yml and future notification workflows
  • ➕ Standardizes payload formatting and secret handling
  • ➖ Slightly more indirection when debugging workflow runs
  • ➖ Requires factoring shared logic upfront, which may be overkill for two workflows
3. Route discussion events to a small service (webhook -> Slack)
  • ➕ More flexible filtering/routing, retries, and richer formatting
  • ➕ Decouples Slack logic from GitHub Actions runtime
  • ➖ Introduces new infra, hosting, and operational burden
  • ➖ More complex security and secret management

Recommendation: The PR’s approach (inline curl+jq with the existing SLACK_WEBHOOK_URL secret) is appropriate for a lightweight, low-dependency notification. If additional Slack notification workflows are expected, consider extracting the shared Slack-post logic into a reusable workflow to avoid drift between notify-adr-slack.yml and notify-vouch-slack.yml.

Files changed (1) +61 / -0

Other (1) +61 / -0
notify-vouch-slack.ymlAdd Slack notification workflow for new vouch-request discussions +61/-0

Add Slack notification workflow for new vouch-request discussions

• Introduces a new workflow that triggers on Discussion creation and posts a Slack message when the discussion category slug is "vouch-request". Adds a workflow_dispatch path with inputs to allow manual end-to-end testing using the same webhook secret.

.github/workflows/notify-vouch-slack.yml

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 10, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 1:07 PM UTC · Ended 1:08 PM UTC

Commit: 8b010b7 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 10, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:09 PM UTC · Completed 1:24 PM UTC

Commit: 63deb6c · View workflow run →

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Unvalidated Slack mrkdwn inputs 🐞 Bug ⛨ Security
Description
workflow_dispatch allows free-form username and discussion_url inputs that are inserted into a
Slack mrkdwn message without validation or escaping, so an authorized workflow dispatcher can
craft misleading links/mentions and spoof notifications. This creates an avoidable Slack abuse/spam
vector that doesn’t require changing workflow code—only manually triggering it with crafted inputs.
Code

.github/workflows/notify-vouch-slack.yml[R28-29]

+          DISC_URL: ${{ github.event_name == 'workflow_dispatch' && inputs.discussion_url || github.event.discussion.html_url }}
+          DISC_AUTHOR: ${{ github.event_name == 'workflow_dispatch' && inputs.username || github.event.discussion.user.login }}
Relevance

●●● Strong

Team has accepted workflow hardening against input/URL injection risks; validating/escaping Slack
mrkdwn inputs is aligned.

PR-#3820
PR-#225
PR-#191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow defines required, free-form workflow_dispatch inputs and uses them to set DISC_URL
and DISC_AUTHOR, which are then concatenated into a Slack mrkdwn string; there is no
validation/escaping step before posting to the webhook.

.github/workflows/notify-vouch-slack.yml[7-15]
.github/workflows/notify-vouch-slack.yml[28-29]
.github/workflows/notify-vouch-slack.yml[36-46]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The workflow’s `workflow_dispatch` inputs (`username`, `discussion_url`) are treated as trusted and are directly interpolated into a Slack Block Kit `mrkdwn` message. Because these inputs are free-form, any actor who is authorized to run `workflow_dispatch` can send misleading/abusive Slack notifications (e.g., formatted mentions, deceptive URLs).

## Issue Context
The discussion-triggered path uses GitHub-provided values (author login + discussion URL). The manual path is intended for testing, but currently enables arbitrary content to be posted to Slack using the shared webhook.

## Fix Focus Areas
- .github/workflows/notify-vouch-slack.yml[7-15]
- .github/workflows/notify-vouch-slack.yml[28-29]
- .github/workflows/notify-vouch-slack.yml[36-46]

## What to change
- Add shell validation for `workflow_dispatch` values before building the payload:
 - Require `DISC_AUTHOR` to match GitHub username rules (e.g., `^[A-Za-z0-9-]{1,39}$`).
 - Require `DISC_URL` to be a GitHub Discussions URL for the current repo (e.g., `^https://github.com/${GITHUB_REPOSITORY}/discussions/[0-9]+`), or remove inputs entirely and use fixed safe test values.
- Optionally reduce Slack formatting surface:
 - Wrap interpolated fields in backticks (for author), or avoid `mrkdwn` for user-controlled fields, or set `verbatim: true` on the `mrkdwn` text object if appropriate for your Slack rendering needs.
- If manual testing is needed only on forks, consider gating `workflow_dispatch` (e.g., only allow it when `github.repository_owner != 'fullsend-ai'`), while still allowing the discussion event on upstream.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
✅ Compliance rules (platform): 54 rules

Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread .github/workflows/notify-vouch-slack.yml
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review

Findings

High

  • [protected-path] .github/workflows/notify-vouch-slack.yml — PR adds a new file under .github/workflows/, a protected path requiring human approval. The PR has no linked issue providing explicit authorization for modifying governance/infrastructure files. Protected files: .github/workflows/notify-vouch-slack.yml.
    Remediation: Link a GitHub issue that authorizes this workflow addition, or obtain explicit human maintainer approval.

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run

Review

Findings

High

  • [protected-path] .github/workflows/notify-vouch-slack.yml — PR adds a new file under .github/workflows/, a protected path requiring human approval. The PR has no linked issue providing explicit authorization for modifying governance/infrastructure files. Protected files: .github/workflows/notify-vouch-slack.yml.
    Remediation: Link a GitHub issue that authorizes this workflow addition, or obtain explicit human maintainer approval.

Medium

  • [scope-authorization-tier] PR title uses feat: prefix, but COMMITS.md explicitly states feat is wrong for CI/infrastructure changes. The forbidden combinations table bans feat(ci), and COMMITS.md instructs reviewers to "flag violations as a required change — they are not cosmetic." This is a new GitHub Actions workflow (a CI/CD pipeline change) and the PR itself carries the component/ci label. Using feat will cause this CI workflow to appear under "Features" in the auto-generated release notes, misleading end users.
    Remediation: Change PR title to ci(vouch): add Slack notification for new vouch requests.

Low

  • [workflow-command-injection] .github/workflows/notify-vouch-slack.yml:40 — When DISC_AUTHOR fails validation, the raw (unsanitized) value is interpolated into a ::error:: workflow command. If the value contains %0A or literal newlines (possible via API-submitted workflow_dispatch inputs), an attacker could inject additional workflow commands into the log output. The same pattern applies to DISC_URL on line 44. Impact is minimal: permissions: {} grants no token, and workflow_dispatch requires repo write access.
    Remediation: Omit the invalid value from the ::error:: message entirely, e.g. echo '::error::Invalid username received'.

Labels: PR adds a new GitHub Actions workflow under .github/workflows/ for vouch-request Slack notifications. component/ci is already applied.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (2)

Review

Findings

High

  • [protected-path] .github/workflows/notify-vouch-slack.yml — PR adds a new file under .github/workflows/, a protected path requiring human approval. The PR has no linked issue providing explicit authorization for modifying governance/infrastructure files. Protected files: .github/workflows/notify-vouch-slack.yml.
    Remediation: Link a GitHub issue that authorizes this workflow addition, or obtain explicit human maintainer approval.

Low

  • [missing-authorization] Non-trivial change (61 new lines, new workflow file) with no linked issue.

  • [scope-authorization-tier] PR title uses feat: prefix, but COMMITS.md explicitly states feat is wrong for CI/infrastructure changes and lists ci as the correct type. The forbidden combinations table further bans feat(ci). Consider using ci(vouch): add Slack notification for new vouch requests.

  • [edge-case] .github/workflows/notify-vouch-slack.yml:20 — The job-level if condition does not include a github.repository_owner == 'fullsend-ai' guard. The sibling vouch-command.yml includes this check. While the SLACK_WEBHOOK_URL emptiness check handles missing secrets, the workflow still produces failed runs on forks.


Labels: PR adds a new GitHub Actions workflow under .github/workflows/


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the component/ci CI pipelines and checks label Aug 10, 2026
Comment thread .github/workflows/notify-vouch-slack.yml
@maruiz93
maruiz93 force-pushed the notify-vouch-slack branch from 63deb6c to ffe6240 Compare August 10, 2026 15:05
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 10, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:07 PM UTC · Completed 3:19 PM UTC

Commit: ffe6240 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Comment thread .github/workflows/notify-vouch-slack.yml

@ralphbean ralphbean left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Two notes inline.

Comment thread .github/workflows/notify-vouch-slack.yml
Comment thread .github/workflows/notify-vouch-slack.yml
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 11, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 9:11 AM UTC · Ended 9:18 AM UTC

Commit: eea5eb0 · View workflow run →

Notify the team when a new vouch-request discussion is created so
maintainers can review without polling GitHub. Follows the same
pattern as notify-adr-slack.yml (curl + jq Block Kit payload).
Includes workflow_dispatch trigger for testing.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Marta Anon <manon@redhat.com>
@maruiz93
maruiz93 force-pushed the notify-vouch-slack branch from eea5eb0 to 4ba57a8 Compare August 11, 2026 09:17
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 11, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:19 AM UTC · Completed 9:32 AM UTC

Commit: 4ba57a8 · View workflow run →

@maruiz93 maruiz93 changed the title feat: add Slack notification for new vouch requests ci(vouch): add Slack notification for new vouch requests Aug 11, 2026

@fullsend-ai-review fullsend-ai-review Bot 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.

See the review comment for full details.

@maruiz93
maruiz93 added this pull request to the merge queue Aug 11, 2026
Merged via the queue into fullsend-ai:main with commit 0435439 Aug 11, 2026
23 checks passed
@maruiz93
maruiz93 deleted the notify-vouch-slack branch August 11, 2026 09:38
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 11, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 9:40 AM UTC · Completed 9:51 AM UTC

Commit: 4ba57a8 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #6038 — ci(vouch): add Slack notification for new vouch requests

Human-authored PR (maruiz93, co-authored with Claude) adding a single new GitHub Actions workflow file (73 lines). Three review agent rounds, two human reviewers (waynesun09, ralphbean), and one other bot (qodo-code-review). Merged after ~20.5 hours.

Review agent performance

Strengths:

  • Correctly identified 5 distinct issues across 3 rounds: protected-path, missing-authorization, scope-authorization-tier (with good escalation from LOW→MEDIUM in round 2), missing repository_owner guard, and workflow-command-injection.
  • The workflow-command-injection finding (raw unsanitized values in ::error:: messages) was novel — neither human reviewer caught it.
  • Properly tracked resolution: the edge-case (missing owner guard) and missing-authorization findings were dropped in round 2 after the author addressed them.

Gaps vs. human reviewers:

  • Missed timeout-minutes CI convention (waynesun09 caught it). The repo documents a clear "Timeout policy" in docs/contributing/ci-workflows.md requiring every non-reusable workflow job to set timeout-minutes. The agent didn't cross-reference this convention. This is evidence for existing issue #4149 (review agent should cross-reference repo-documented pitfalls/checklists).
  • Missed test-plan logical inconsistency (waynesun09 caught it). The PR claimed fork-based workflow_dispatch testing, but the shipped repository_owner == 'fullsend-ai' guard makes fork-based testing impossible since the fork owner would be maruiz93. This is a logical reasoning gap not covered by existing issues — see proposal below.
  • Protected-path finding persisted across all 3 rounds including after ralphbean's human approval. The third review ran for ~14 minutes and produced only this already-satisfied finding. This is evidence for existing issues #2794 and #1500.

Rework rate

Four force pushes total. The author addressed agent, bot, and human findings across 3 iterations. The review agent's findings were actionable and drove real improvements (owner guard, input validation, command injection fix, commit prefix correction). No wasted rework from false positives — even the persistent protected-path finding was correct in requiring human approval.

Autonomy assessment

The review agent added clear value (command-injection catch humans missed) but also missed two issues humans caught (timeout-minutes, test-plan inconsistency). For CI workflow reviews specifically, the agent is not yet ready for increased autonomy — it needs better awareness of repo-specific conventions and cross-validation of PR claims against code logic.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/ci CI pipelines and checks Review effort 1/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants