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

Delegated allowlists? #209

Closed
martincizek opened this issue Apr 16, 2021 · 1 comment
Closed

Delegated allowlists? #209

martincizek opened this issue Apr 16, 2021 · 1 comment
Labels

Comments

@martincizek
Copy link

Is it an acceptable practice to provide a custom list implementation in place of a static array to achieve a custom allow/deny decision? The code below works as expected:

allowlist[:protocols]["a"]["href"] = Class.new do
  def include?(p)
    return true if p == :relative
    p = p.strip.downcase
    return false unless p =~ /\A[a-z][a-z0-9\+\.\-]*\z/ # RFC 3986
    %w(data javascript vbscript).none?(p)
  end
end.new

We want to achieve similar <a href=...> link sanitization behavior like GitLab, whose devs decided to allow any protocol and implement a dedicated transformer instead. Which seems too complex and error-prone for our rather simple use case (Redmine CommonMark formatter).

But I haven't found anything in the Sanitize docs that would make me feel safe about relying on the include?() contract.

Based on your answer, I can submit a documentation PR eventually. And thanks for this great library btw. :)

@rgrove
Copy link
Owner

rgrove commented Apr 16, 2021

That's a clever solution, but I wouldn't recommend this.

While I don't foresee Sanitize changing anytime soon in a way that would break this, I consider the call to include? an internal implementation detail and not part of the public API contract, so there's no guarantee this won't break in a semver minor or patch release.

It might be a little more complicated, but I think the best way to do this is with a dedicated transformer like the GitLab solution. That said, if you prefer the solution above and are willing to accept the risk of breakage, I won't stop you!

And thanks for this great library btw. :)

I'm glad you're finding it useful! 🙂

@rgrove rgrove closed this as completed Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants