Skip to content

Commit 856d525

Browse files
committed
refactor(workflows): consolidate GitHub->Slack notifications into one file
Address review feedback: - Merge closed-pr-comment.yml and slack-issue-notification.yml into a single github-slack-notifications.yml with two triggers (issues + issue_comment) and two jobs. One central place to control the GitHub -> Slack integration, as requested in review. - Remove internal incident references from the public workflow; the header now describes what the workflow does, not why it was created. No behavior change: both jobs emit the same 20-key payload the Slack workflow already consumes, gated and deduped exactly as before.
1 parent 57a2506 commit 856d525

2 files changed

Lines changed: 79 additions & 88 deletions

File tree

.github/workflows/closed-pr-comment.yml renamed to .github/workflows/github-slack-notifications.yml

Lines changed: 79 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,83 @@
1-
name: Closed PR Comment Redirect
1+
name: GitHub Slack Notifications
22

3-
# COE AI-7: alert users who comment on closed PRs to open a GitHub issue.
4-
# An external user reported the v1.4.8 SPII leak by commenting on the
5-
# already-closed revert PR. Closed-PR comments are not surfaced to oncall,
6-
# but issues are. This workflow:
7-
# 1. Replies to the commenter with a link to open a new issue.
8-
# 2. Notifies oncall via the existing SLACK_WEBHOOK_URL.
9-
# Maintainers (admin/maintain/write) are skipped so internal back-and-forth
10-
# doesn't trigger the redirect.
3+
# Central GitHub -> Slack integration point. Two triggers feed one shared
4+
# Slack workflow (via SLACK_WEBHOOK_URL), which branches on event_type:
5+
# - issues opened -> notify oncall of a new issue
6+
# - comments on closed PRs -> redirect the commenter to open an issue,
7+
# and notify oncall (closed-PR comments are
8+
# otherwise easy to miss)
9+
# Every payload sends the same key set so the Slack workflow can branch
10+
# reliably; fields that don't apply to an event are sent empty.
1111

1212
on:
13+
issues:
14+
types: [opened]
1315
issue_comment:
1416
types: [created]
1517

16-
permissions:
17-
pull-requests: write
18-
issues: read
19-
20-
# Serialize per-PR so the marker-comment dedup is race-free (otherwise two
21-
# rapid-fire comments could both see "no marker" and both post a redirect).
22-
# cancel-in-progress is false so the second run still executes after the
23-
# first finishes — we want to evaluate dedup against the just-posted marker.
24-
concurrency:
25-
group: closed-pr-comment-${{ github.event.issue.number }}
26-
cancel-in-progress: false
27-
2818
jobs:
29-
redirect:
19+
notify-issue-opened:
20+
if: github.event_name == 'issues'
3021
runs-on: ubuntu-latest
31-
# Only fire on PRs (issue_comment fires for issues too) that are closed.
22+
permissions: {}
23+
steps:
24+
- name: Send issue details to Slack
25+
# Attacker-controlled fields are passed through env: rather than
26+
# interpolated into the YAML payload, to prevent workflow injection.
27+
# For issue_opened, the issue_* fields carry the data and the
28+
# pr_*/comment_* fields are empty.
29+
env:
30+
REPOSITORY: ${{ github.repository }}
31+
CREATED_AT: ${{ github.event.issue.created_at }}
32+
ISSUE_NUMBER: ${{ github.event.issue.number }}
33+
ISSUE_TITLE: ${{ github.event.issue.title }}
34+
ISSUE_URL: ${{ github.event.issue.html_url }}
35+
ISSUE_AUTHOR: ${{ github.event.issue.user.login }}
36+
ISSUE_BODY: ${{ github.event.issue.body }}
37+
LABELS: ${{ join(github.event.issue.labels.*.name, ', ') }}
38+
uses: slackapi/slack-github-action@v2.1.1
39+
with:
40+
webhook: ${{ secrets.SLACK_WEBHOOK_URL }}
41+
webhook-type: webhook-trigger
42+
payload: |
43+
event_type: "issue_opened"
44+
repository: "${{ env.REPOSITORY }}"
45+
created_at: "${{ env.CREATED_AT }}"
46+
issue_number: "${{ env.ISSUE_NUMBER }}"
47+
issue_title: ${{ toJSON(env.ISSUE_TITLE) }}
48+
issue_url: "${{ env.ISSUE_URL }}"
49+
issue_author: "${{ env.ISSUE_AUTHOR }}"
50+
issue_body: ${{ toJSON(env.ISSUE_BODY) }}
51+
labels: ${{ toJSON(env.LABELS) }}
52+
pr_number: ""
53+
pr_title: ""
54+
pr_url: ""
55+
pr_author: ""
56+
pr_state: ""
57+
pr_closed_at: ""
58+
pr_merged_at: ""
59+
comment_id: ""
60+
comment_url: ""
61+
comment_author: ""
62+
comment_body: ""
63+
64+
closed-pr-comment-redirect:
65+
# Only fire on comments left on PRs (issue_comment fires for issues too)
66+
# that are already closed, and skip comments left by bots.
3267
if: >-
33-
github.event.issue.pull_request != null && github.event.issue.state == 'closed' && github.event.comment.user.type
34-
!= 'Bot'
68+
github.event_name == 'issue_comment' && github.event.issue.pull_request != null && github.event.issue.state ==
69+
'closed' && github.event.comment.user.type != 'Bot'
70+
runs-on: ubuntu-latest
71+
permissions:
72+
pull-requests: write
73+
issues: read
74+
# Serialize per-PR so the marker-comment dedup is race-free (otherwise two
75+
# rapid-fire comments could both see "no marker" and both post a redirect).
76+
# cancel-in-progress is false so the second run still executes after the
77+
# first finishes -- we want to evaluate dedup against the just-posted marker.
78+
concurrency:
79+
group: closed-pr-comment-${{ github.event.issue.number }}
80+
cancel-in-progress: false
3581
steps:
3682
- name: Check commenter permission
3783
id: perm
@@ -77,9 +123,9 @@ jobs:
77123
78124
- name: Post redirect comment
79125
if: steps.perm.outputs.skip != 'true' && steps.existing.outputs.already_posted != 'true'
80-
# The Slack alert is the load-bearing part of this workflow (it's
81-
# what closes the COE detection gap). If posting the bot reply
82-
# fails (rate limit, transient error), don't block oncall paging.
126+
# The Slack notification is the load-bearing part of this job. If
127+
# posting the bot reply fails (rate limit, transient error), don't
128+
# block the Slack notification.
83129
continue-on-error: true
84130
uses: actions/github-script@v9
85131
env:
@@ -124,16 +170,14 @@ jobs:
124170
core.setOutput('state', mergedAt ? 'merged' : 'closed');
125171
126172
- name: Notify Slack
127-
# Page oncall only on the FIRST external comment per PR (gated by
128-
# already_posted). Subsequent comments on the same PR don't page —
129-
# the redirect comment has already told the commenter to open an
130-
# issue, and issues page oncall via the issue path. This bounds
131-
# paging volume regardless of how chatty a thread becomes.
173+
# Notify oncall only on the FIRST external comment per PR (gated by
174+
# already_posted). Subsequent comments on the same PR don't notify --
175+
# the redirect comment has already directed the commenter to open an
176+
# issue, and issues notify oncall via the issue path. This bounds
177+
# notification volume regardless of how chatty a thread becomes.
132178
if: steps.perm.outputs.skip != 'true' && steps.existing.outputs.already_posted != 'true'
133179
# Attacker-controlled fields are passed through env: rather than
134180
# interpolated into the YAML payload, to prevent workflow injection.
135-
# Schema is uniform across event types: every workflow sends the
136-
# same 20 keys so Slack-side branching on event_type is reliable.
137181
# For closed-PR comments, the issue_* fields are empty (this isn't
138182
# an issue) and the pr_*/comment_* fields carry the real data.
139183
env:

.github/workflows/slack-issue-notification.yml

Lines changed: 0 additions & 53 deletions
This file was deleted.

0 commit comments

Comments
 (0)