fix double-escaping in Slack webhook payloads - #2994
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR adjusts Slack webhook payload generation to avoid double-escaped HTML entities in event descriptions, aligning outbound Slack message text with Slack’s documented escaping rules.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Changes:
- Update Slack payload text generation to unescape HTML entities and then re-escape only Slack control characters (
&,<,>). - Update/rename the Slack webhook payload test to assert quotes are preserved while Slack control characters are escaped.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/models/webhook/delivery.rb | Changes Slack payload text escaping behavior to prevent double-escaped entities in Slack notifications. |
| test/models/webhook/delivery_test.rb | Updates Slack payload expectation and test name to validate the new escaping behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def slack_payload | ||
| text = event.description_for(nil).to_plain_text | ||
| text = CGI.unescapeHTML(event.description_for(nil).to_plain_text).gsub(/[&<>]/, SLACK_ESCAPE) |
There was a problem hiding this comment.
CGI.unescapeHTML does not recursively decode — it's a single gsub pass, so replaced text is never re-scanned. CGI.unescapeHTML("&quot;") returns ", not \" .
Since Event::Description#to_plain_text escapes every & in user input to &, one unescape pass is the exact inverse of ERB::Util#h, and the & ordering concern doesn't apply. I verified the suggested "unescape & last" implementation produces byte-identical output across all the cases here, including a title containing a literal ".
Added a regression test covering that case.
Event::Description#to_plain_text HTML-escapes its output, so Slack notifications showed literal entities like " and &basecamp#39;. Slack renders plain text rather than HTML and only wants &, <, and > converted to entities: https://docs.slack.dev/messaging/formatting-message-text/#escaping Undo the HTML escaping with CGI.unescapeHTML, then escape just the three Slack control characters. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3ae07b3 to
0a1862a
Compare
What
Event::Description#to_plain_textHTML-escapes its output, so Slack notifications showed literal entities like"and'. Slack renders plain text rather than HTML and only wants&,<, and>converted to entities:https://docs.slack.dev/messaging/formatting-message-text/#escaping
Undo the HTML escaping with
CGI.unescapeHTML, then escape just the three Slack control characters.