ActionText::ContentHelper.allowed_tags (and attributes) are no longer easily added to

[brendon]

Since Rails 7.1 and this change in particular:

e8137c5#diff-3ae50dc6dc778a98f32121997298af3643f7ca400cd35638a1236948a10fedd3R58

We can no longer easily add additional allowed_tags to ActionText::ContentHelper because it's computed at runtime and there's no configuration option.

Expected behavior

One could previously do this in an initialiser to add additional tags:

ActionText::ContentHelper.allowed_tags.add 'iframe'

Actual behavior

Now that raises an error saying allowed_tags is nil because it has no default value.

System configuration

Rails version: 7.1+

Ruby version: Any

Probably the best way forward is to add this as a configuration option unless a better option exists?

https://stackoverflow.com/questions/77366033/allow-actiontext-tags-in-rails-7-1-with-new-sanitizers

@flavorjones, issue opened at your request :)

[brendon]

One proposed solution was this:

ActiveSupport.on_load :action_text_content do
  default_allowed_tags = Class.new.include(ActionText::ContentHelper).new.sanitizer_allowed_tags
  ActionText::ContentHelper.allowed_tags = default_allowed_tags.merge(['iframe', ...])
end

Having to initialise the module into a new class just to get at the defaults feels backward.

[flavorjones]

This has been challenging to get right because Action Text doesn't have a default set of allowed tags and attributes, it delegates that to the sanitizer, which is also configurable.

How would you feel if the recommended idiom for adding allowed tags and attrs was something like:

ActiveSupport.on_load :action_text_content do
  ActionText::ContentHelper.extra_allowed_tags = ["embed"]
  ActionText::ContentHelper.extra_allowed_attributes = ["foobar"]
end

The idea is that the allowlist would be the union of the sanitizer's allowlist and the "extra_allowed" values.

[brendon]

That would work in my case as I just want to add extra tags but I wonder about the case when someone wants to remove a default tag. I suppose it would be weird for them to be removing the trix defaults and they can manipulate the sanitiser defaults elsewhere.

Would it be better to make this an official configuration option? Something like:

config.action_text.extra_allowed_tags
config.action_text.extra_allowed_attributes

[Alexander-Senko]

the sanitizer, which is also configurable.

config.action_view.sanitized_allowed_attributes lacks a default value as well being nil. That should be fixed first, I guess.

[flavorjones]

@Alexander-Senko What I am trying to say is that Action View has no opinion on allowed attributes and tags; it relies on the underlying sanitizer's lists, and that's why the default value is now nil.

The problem I think we need to solve first is "how to safely modify the allowlist" and not "I need my code to know what the default is", because I imagine the most common reason people need to know that is ... to safely modify it. No?

[flavorjones]

To be clear, I want to solve both problems. I'm trying to share my mental models to move the conversation forward.

[Alexander-Senko]

config.action_view.sanitized_allowed_attributes lacks a default value as well being nil. That should be fixed first, I guess.

At the same time ActionView::Base.sanitized_allowed_attributes is set to defaults. Why is config.action_view.sanitized_allowed_attributes not? That looks inconsistent to me.

[flavorjones]

I agree it is inconsistent and I've said several times already in this thread that I want to fix it.

[rails-bot]

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 8-0-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

[brendon]

Not Stale