Slack doc review notification#23313
Conversation
✅ Netlify Preview
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for cockroachdb-api-docs canceled.
|
✅ Deploy Preview for cockroachdb-interactivetutorials-docs canceled.
|
Sends a notification to #doc_review channel when docs-prs team is requested as a reviewer on a PR. The notification includes: - PR title and number - Author - Total lines changed - Direct link to the PR Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
taroface
force-pushed
the
slack-doc-review-notification
branch
from
c46704f to
12f5e9f
Compare
Files changed:
|
ebembi-crdb
left a comment
ebembi-crdb
left a comment
There was a problem hiding this comment.
A few issues to address before merging:
JSON injection via PR title
The curl payload constructs JSON by interpolating environment variables directly into a heredoc string. If a PR title contains a double quote or backslash, the JSON will be malformed. A crafted title could inject arbitrary JSON keys. Use jq to safely build the payload:
run: |
jq -n \
--arg title "$PR_TITLE" \
--arg url "$PR_URL" \
--arg author "$PR_AUTHOR" \
--arg number "$PR_NUMBER" \
--arg lines "$LINES_CHANGED" \
'{pr_title: $title, pr_url: $url, pr_author: $author, pr_number: $number, lines_changed: $lines}' \
| curl -X POST "$SLACK_WEBHOOK_URL" \
-H 'Content-Type: application/json' \
-d @-Webhook URL is hardcoded in plaintext
The Slack webhook URL is committed directly into the workflow file. Anyone with read access to the repo can use it to post arbitrary messages to the Slack channel. This should be stored as a GitHub Actions secret (e.g., ${{ secrets.SLACK_DOC_REVIEW_WEBHOOK_URL }}).
The gh api call is unnecessary
The "Get PR details" step shells out to gh api to fetch additions/deletions, but github.event.pull_request in a review_requested event already includes additions and deletions. The whole first step could be replaced with:
LINES_CHANGED: ${{ github.event.pull_request.additions + github.event.pull_request.deletions }}No error handling on the curl call
If the Slack webhook is down or returns an error, the workflow succeeds silently. Adding --fail-with-body to the curl call would surface failures.
- Use jq to safely construct JSON payload, preventing injection attacks - Move webhook URL to GitHub secret instead of hardcoding - Remove unnecessary gh api call, use github.event.pull_request directly - Add --fail-with-body to curl for proper error handling Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
|
@ebembi-crdb TFTR! Let me know if this looks good now. |
2 participants
Add a GitHub Action that sends a notification to Slack channel
#doc_reviewwhendocs-prsis requested as a reviewer on adocsPR.