Plugin for using DOMPurify as Aurelia's HTML sanitizer

Thanks for another pointer in the right direction! Finally, it works, no matter where I put it:

import { Container, FrameworkConfiguration } from 'aurelia-framework';
import { HTMLSanitizer, SanitizeHTMLValueConverter } from 'aurelia-templating-resources';
import { DOMPurifySanitizer } from './dom-purify-sanitizer';

export function configure(config: FrameworkConfiguration): void {
  if (config.aurelia.resources['valueConverters']['sanitizeHTML']) {
    config.aurelia.resources['valueConverters']['sanitizeHTML'] = config.container.get(SanitizeHTMLValueConverter);
  }
}

HTMLSanitizer['get'] = (container: Container) => {
  return new DOMPurifySanitizer();
}
HTMLSanitizer['protocol:aurelia:resolver'] = true;

SanitizeHTMLValueConverter['get'] = (container: Container) => {
  return new SanitizeHTMLValueConverter(container.get(DOMPurifySanitizer));
}
SanitizeHTMLValueConverter['protocol:aurelia:resolver'] = true;

export { DOMPurifySanitizer };
4 Likes

seems a bit more involved than replacing the internal instance :wink:

Like making it configurable? :smiley: @appex/aurelia-dompurify - npm

1 Like

Hi @mroeling and @dabide ,

First of all, I am not a team member, so I cannot speak on behalf of the team here.

Personally, I disagree that Aureliaā€™s built-in HTML sanitizer needs to be addressed. The docs already clearly say, that itā€™s just a simple sanitizer for development purposes and that it is not suited for production usage. The docs also clearly state, that for production usage, a more sophisticated sanitizer should be chosen and it provides some hints on how to do that.

Binding: Basics - Element Content

1 Like

I can see both arguments.

My feeling/opinion is that it IS something that can be missed for some of us and can come back to be an issue that might/would/could be blamed on the default implementation being there.

Perhaps when building for prod (or any env) there could be a very visible warning that appears when the default, dev use only htmlsanitizer, is being used and warn it should be replaced. Having that buried in the docs could very easily be missed or forgotten about.

3 Likes

There is a rather large chance that all of us that used the built in sanitizer before the docs were changed two years ago, will continue using it without ever consulting the docs. The same will probably be the case for new developers working on existing codebases: Theyā€™ll just do it like it has been done before. For me, that is reason enough that this is a rather big problem. The warning thatā€™s printed in the logs (once) is easy to miss.

Also, I really donā€™t understand the rationale behind having an implementation ā€œfor development purposesā€. What need does that fill?

All in all, I think the availability of the sanitizeHTML value converter paired with the default implementation of HTMLSanitizercreates very dangerous false expectations, and something should be done about that.

3 Likes

I understand the documentation issue. I myself an a ā€œfreshā€ Aurelia user since last year. If something would change in the docs, I would probably not notice it myself, or it would be by chance, I guess.

I also understand the ā€œdevelopment implementationā€ argument. I agree that it could be misleading and that one would expect it to be safe anyway.

However, the title of this post says: ā€œUsing DOMPurify as Aureliaā€™s HTML sanitizerā€. Thatā€™s a step too far in my opinion. For two reasons.

The first reason is overhead. Well, DOMPurifyā€™s code is pretty small: about 8 KB minified. Nevertheless, itā€™s probably about 8 KB too large in projects where I donā€™t need itā€¦ The default implementation has some overhead as well, of course. I am not sure how much, however. And I am not sure how much of the current implementation would remain if DOMPurify would be integrated in Aurelia.

The second reason is freedom of choice. Should this actually be an issue that needs to be addressed by Aurelia? Or should it be addressed by the developer(s) that use Aurelia in their own projects instead? If I would actually prefer the sanitize-html package, for example, I would like to be able to include that instead of DOMPurify.

Therefore, if having the current default implementation for ā€œdevelopment purposesā€ is misleading and dangerous, and thus undesired, I would propose to completely remove it from Aurelia and only keep the architectural infrastructure intact. The default implementation should probably only throw errors that indicate that a fitting sanitizer needs to be used. That would force everybody who needs HTML sanitizing to also choose a fitting production-ready sanitizer. And it would keep Aureliaā€™s footprint as small as possible.

3 Likes

Aha, now I understand where youā€™re coming from. :slight_smile: I fully agree with your sentiment, and can see how you read the topic that way. It has now been edited to clarify what was meant: A presentation of my plugin to use DOMPurify as the HTMLSanitizer implementation.

4 Likes

I see now. Sorry that I didnā€™t read the full post as attentively as I normally do. If I had, I could have saved me some misplaced frustration. :relieved:

Your plugin looks interesting in several regards, especially the logic for replacing the default DI registration. It seems that that was somewhat more complicated than I would initially think. :open_mouth: :slightly_smiling_face:

4 Likes

Great to see such a vivid discussion and thourough explanations for both sides. It shows the commitment of the community :slight_smile:

Tbh, Iā€™m one of the I donā€™t know how many that have missed the note in the docs. And yes, that is totally my mistake.
Working on our project for nearly 3,5 years now, and almost going into production, Iā€™m glad that I at least try to follow important updates in the code packages we use, and of course in Aurelia as base for the frontend. But that might not be for everyone. And especially for a potentially vurlnerable part of the base code (wether it is meant for use in production production or not), I would suggest to at least add a console warning that mentions the status of the purifier, that it is not meant for production.

And will definitely give the plugin and/or code example a shot. Thanks for your contributions!

4 Likes