Skip to content

Commit

Permalink
Allow Rails html_safe for non-interpolated String literal receiver
Browse files Browse the repository at this point in the history
Tagging a string as html safe may be a security risk only when the string could contain random user input values.
Marking a statically coded string literal as html safe is a totally valid usage of html_safe.
  • Loading branch information
amatsuda authored and bbatsov committed Dec 21, 2018
1 parent 75414d8 commit 8435af4
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* [#6571](https://github.com/rubocop-hq/rubocop/issues/6571): Fix a false positive for `Layout/TrailingCommaInArguments` when a line break before a method call and `EnforcedStyleForMultiline` is set to `consistent_comma`. ([@koic][])
* [#6562](https://github.com/rubocop-hq/rubocop/pull/6562): Fix a false positive for `Style/MethodCallWithArgsParentheses` `omit_parentheses` enforced style after safe navigation call. ([@gsamokovarov][])
* [#6570](https://github.com/rubocop-hq/rubocop/pull/6570): Fix a false positive for `Style/MethodCallWithArgsParentheses` `omit_parentheses` enforced style while splatting the result of a method invocation. ([@gsamokovarov][])
* [#6594](https://github.com/rubocop-hq/rubocop/pull/6594): Fix a false positive for `Rails/OutputSafety` when the receiver is a non-interpolated string literal. ([@amatsuda][])

## 0.61.1 (2018-12-06)

Expand Down Expand Up @@ -3699,3 +3700,4 @@
[@tom-lord]: https://github.com/tom-lord
[@bayandin]: https://github.com/bayandin
[@nadiyaka]: https://github.com/nadiyaka
[@amatsuda]: https://github.com/amatsuda
6 changes: 6 additions & 0 deletions lib/rubocop/cop/rails/output_safety.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ class OutputSafety < Cop
MSG = 'Tagging a string as html safe may be a security risk.'.freeze

def on_send(node)
return if non_interpolated_string?(node)

return unless looks_like_rails_html_safe?(node) ||
looks_like_rails_raw?(node) ||
looks_like_rails_safe_concat?(node)
Expand All @@ -76,6 +78,10 @@ def on_send(node)

private

def non_interpolated_string?(node)
node.receiver && node.receiver.str_type? && !node.receiver.dstr_type?
end

def looks_like_rails_html_safe?(node)
node.receiver && node.method?(:html_safe) && !node.arguments?
end
Expand Down
14 changes: 10 additions & 4 deletions spec/rubocop/cop/rails/output_safety_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,20 @@
end

context 'when using `#html_safe`' do
it 'registers an offense for literal receiver and no argument' do
expect_offense(<<-RUBY.strip_indent)
it 'does not register an offense for static string literal receiver' do
expect_no_offenses(<<-RUBY.strip_indent)
"foo".html_safe
^^^^^^^^^ Tagging a string as html safe may be a security risk.
RUBY
end

it 'registers an offense for variable receiver and no argument' do
it 'registers an offense for dynamic string literal receiver' do
expect_offense(<<-'RUBY'.strip_indent)
"foo#{1}".html_safe
^^^^^^^^^ Tagging a string as html safe may be a security risk.
RUBY
end

it 'registers an offense for variable receiver' do
expect_offense(<<-RUBY.strip_indent)
foo.html_safe
^^^^^^^^^ Tagging a string as html safe may be a security risk.
Expand Down

0 comments on commit 8435af4

Please sign in to comment.