Skip to content
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

Solving safe/unsafe defaults #233

Open
annevk opened this issue Jun 12, 2024 · 1 comment
Open

Solving safe/unsafe defaults #233

annevk opened this issue Jun 12, 2024 · 1 comment

Comments

@annevk
Copy link
Collaborator

annevk commented Jun 12, 2024

We have a problem currently that new Sanitizer() ends up being the same configuration for safe and unsafe, with completely different effects. This makes it really hard to design an API to manipulate the policy as the implications for safe and unsafe are different.

To solve this I think we could do the following:

  1. Introduce Sanitizer.default() that returns the configuration we want setHTML() to use when no configuration is provided.
  2. Make setHTML()'s SetHTMLOptions's sanitizer default to "default" rather than {}. setHTMLUnsafe() will continue to default to {}. "default" here is an enum value that means Sanitizer.default() and is needed to make things work IDL-wise. It will also provide us with a way forward for other named policies we might want to add in the future.
  3. new Sanitizer() now represents an empty policy. So if you pass that to setHTML() you only get the XSS-blocklist impacting you.
  4. We can continue to keep get() and getUnsafe(). get() is useful to see how your policy will be impacted by the XSS-blocklist.

This means that if you do setHTML("...", sanitizer: {}) you're only impacted by the XSS-blocklist. If you do setHTML("...") you get our more conservative "default" take.

Coupled with #229 I think this would provide all the building blocks we need.

@annevk
Copy link
Collaborator Author

annevk commented Nov 29, 2024

3/4 are addressed by 7e2f127 and 1 is noted. But 1/2 still need to be done.

And note that 4 was morphed into the clearly better removeUnsafe().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant