-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for interactive app form descriptions #3763
Conversation
Allows providing longer descriptions as a form header in Markdown format in form.yml.erb for interactive app forms.
@@ -1,4 +1,6 @@ | |||
|
|||
<%= OodAppkit.markdown.render(@app.form_header).html_safe %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be html_safe
? I feel like that's a way to inject <script>
tags in an odd way.
Links I think maybe OK - but it seem to me that we should sanitize it in some way to be sure at least script
s don't work.
https://api.rubyonrails.org/classes/ActionView/Helpers/SanitizeHelper.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did test <script>
tags, and they seemed to be sanitized away, but I'll take another look at it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into it, I didn't try myself, I just think about it when i see html_safe
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced html_safe with sanitize. Using the defaults for now, which seemed to even sanitize away div
elements and id tags used in the tests. I haven't looked into whether the permitted tags list needs to be customized or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Realistically I'm only worried about script
tags.
Div elements and ids are sanitized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This PR fixes #3405 by implementing a
form_header
option for the form YMLs. The form header will be shown only in the interactive app forms, as seen in the example screenshots in #3405.Example usage: