Add option to not sanitize indexed data on display

drunken monkey → made their first commit to this issue’s fork.

Thanks a lot for proposing this, it does seem to make sense. I just amended the title and description of the option in the UI slightly to read:

Allow any HTML
Allow all HTML. Use with caution: must only be used with previously sanitized data, for instance, when displaying a field that contains a rendered view mode.

However, I’m still not sure this is something we want to do. Reaching out to the Security Team to maybe give this a quick glance, just to make sure we won’t have a security issue filed in a few weeks to again revert this change.

I can see how:

• this would be a useful feature
• this should be safe in most cases
• this could lead to XSS in some cases

What permission is allowed to set the "Allow any HTML" checkbox?

I agree it should have a warning on it as you point out in #5.

drunken monkey → credited dalin → .

drunken monkey → credited nickdickinsonwilde → .

@greggles: Thanks a lot for your input!
The permission would just be administer views, I guess. Which I think should be sufficient, someone with this permission could wreak all kinds of havoc. Putting an additional restriction on this specific option would also be possible, though, of course.

Further suggestions from Slack:

1. dalin → : Does it need to be a user-changeable flag? Under the hood would we be able to know that this went through the rendering pipeline, so mark it in the index as being safe HTML.
2. nickdickinsonwilde → : What about documenting it and put it in the schema but not providing a UI option? That would seem to reduce risk of thoughtless clicking if you have to go and edit the config file?

1. does seem like a good option, to either only have the “Allow any HTML” option available for those fields marked as safe, or to even hide the setting for those altogether. It just would be a bit more work and might even require some architectural changes if we don’t just want to hard-code this specific field as “safe”.

2. would be a much quicker option to make sure people who use this know what they’re doing – do not add this option to the UI (or only if it is already active, to prevent users from accidentally reverting it) but document it in README.md. Then only advanced users will even be able to set it, and they should be very much aware that they are doing something potentially risky.
The problem then, of course, is that even advanced users will have a hard time finding this option (see #3220868: Create an “Advanced options” sub-module for hidden/advanced config → for this whole set of problems). Though I guess we could add a mention of this to the description for the setting and just refer users to the README.md for details?

@sonfd: What’s your opinion on these suggestions?
Other input of course also welcome!

The administer views permission definition includes restrict access: true. Our documented expectation is that people who have that are trusted with making potentially risky choices.

@greggles: Thanks, it’s good to know that this at least won’t cause a security advisory.

Currently, I’m still leaning towards option 2, however, of removing the setting from the UI and just mentioning it in README.md (or maybe, maybe, in the form element description). But would be great to still hear from @sonfd what he thinks of that.

I like option 1, but I don't understand the challenges required to implement. I don't think I like it if it adds a lot of complexity. And I worry about removing the option and defaulting to no sanitization even for fields we think are safe if it's already been out there sanitizing these fields.

RE: Option 2, hide the option from the UI - my personal feeling is that this just makes it more difficult and confusing to use this option, but doesn't really prevent it from being misused. I think folks will search for an answer and then just make the config edit because they saw it in some stack overflow answer without understanding it anyway. And maybe that is even worse with AI suggestions? I would personally feel better about showing the option with a warning, a situation where I can control the information the person gets before making the change. 🤷

But I think it's a close call and I'll support whatever path you choose.