Avoid potential XSS vulnerability in flash messages #2442
Merged
+7
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
flash messages were being assumed to be HTML-safe which feels too fragile as it relies on every developer remembering to escape any user input that's included in flash messages. fix by removing the `.html_safe` from the messages partial. the only place that seems to be relying on this behaviour is in `MeetingsController` where validation errors are joined into a single flash message using <br> tags. Updated this to just join the messages with commas instead.
venue gets validation as part of its belongs_to association which is required by default. this resulted in 2 validation error messages being shown: "Venue must be set" and "Venue can't be blank". remove the presence validator so we now get just "Venue must be set"
update meeting specs to reflect removal of duplicate validation in cbd78da
mikej
force-pushed
the
prevent-potential-xss-in-flash-messages
branch
from
fa2ba7e to
81c7982
Compare
| - if msg.is_a?(String) | ||
| .alert.alert-dismissible.fade.show.mb-0{ 'data-alert': '', class: "alert-#{name}", role: 'alert' } | ||
| = content_tag :div, msg.html_safe | ||
| = content_tag :div, msg |
Collaborator
There was a problem hiding this comment.
Ah, here we can simplify the markup, something like
Suggested change
| = content_tag :div, msg | |
| %div= msg |
| - msg.each do |message| | ||
| .alert.alert-dismissible.fade.show.mb-0{ 'data-alert': '', class: "alert-#{name}", role: 'alert' } | ||
| = content_tag :span, message.html_safe | ||
| = content_tag :span, message |
Collaborator
There was a problem hiding this comment.
Suggested change
| = content_tag :span, message | |
| %span= message |
mikej
added a commit
to mikej/codebar-planner
that referenced
this pull request
Jan 22, 2026
small improvement to the partial used for flash messages that was suggested in the review comments for codebar#2442
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Flash messages were being assumed to be HTML-safe which feels too fragile as it relies on every developer remembering to escape any user input that's included in flash messages.
Fix by removing the
.html_safefrom the messages partial.The only place that seems to be relying on this behaviour is in
MeetingsControllerwhere validation errors are joined into a single flash message using<br>tags. Updated this to just join the messages with commas instead.