Add Rails/OutputSafety Cop

[josh]

This adds a new cop tighten security around Rails output safety helpers like .html_safe and raw. While you may need .html_safe in a handful of cases, it was too easy to casually use without careful review. Instead of just using content_tag(:div, something) in a safe way in a helper method, "<div>#{something}</div>".html_safe would be used instead. In instances .html_safe were is necessary, we use an inline # rubocop:disable Rails/OutputSafety comment after security review.

[bbatsov]

Add here some examples of code the cop would consider offensive. See other cops for reference.

[bbatsov]

You might want to add some rule about this in the Rails style guide document as well. Such changes require a changelog entry.

[josh]

You might want to add some rule about this in the Rails style guide document as well

Wheres that at?

Such changes require a changelog entry.

👍

[josh]

@bbatsov pushed some changes based on your suggestions.

[bbatsov]

Wheres that at?

https://github.com/bbatsov/rails-style-guide

Apart from the failing build, the cops looks OK to me. You'll have to rebase, though. What's the reason why this is disabled by default? I'm guessing it produces some false positives, right?

[josh]

Apart from the failing build, the cops looks OK to me. You'll have to rebase, though.

👍 I'm on it.

What's the reason why this is disabled by default? I'm guessing it produces some false positives, right?

I'll just move it to enabled by default.

[josh]

@bbatsov updated with changes.

[bbatsov]

👍 Thanks!

[lucascaton]

Hi @josh,

Any suggestions for the following case?

= f.submit '▶'.html_safe

Cheers.

[mikegee]

How about using the decoded character?

= f.submit '▶'

[lucascaton]

Thanks @mikegee, but I was wondering if using a non-ASCII character wouldn't be a bad practice.

[josh]

If you really need encoded entities, consider something like = f.submit html_encode('▶'). Others reading your views have no idea what ▶ means.

[lucascaton]

Thanks @josh!

Did you mean html_escape? Because either html_escape or h compiles to the following HTML:

<input type="submit" name="commit" value="▶" />

While = f.submit '▶'.html_safe generates a non-ASCII character (which is considered a good practice afaik):

<input type="submit" name="commit" value="&#9654;" />

Cheers.

[lastobelus]

I'm also wondering how I'm supposed to have helpers return html entities, or add html entities to strings in helpers???

[lastobelus]

I think the cop should allow raw() for uninterpolated strings.

[mdavidn]

Butting heads against this cop with ' '.html_safe. There's no Rails helper for HTML entities. Uninterpolated strings should not be a violation, as there's no risk of code injection.

Here's a contrived example. (I know there's a CSS alternative. The actual use case is more complex.)

safe_join(words, ' '.html_safe)