Skip to content

Commit

Permalink
Merge pull request #201 from almostwhitehat/adding-require-nonce
Browse files Browse the repository at this point in the history
Implement RequireScriptNonce
  • Loading branch information
rafaelfranca authored Feb 22, 2021
2 parents a6fe90e + 3b5599a commit f4b9380
Show file tree
Hide file tree
Showing 3 changed files with 325 additions and 1 deletion.
41 changes: 40 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ linters:
| [DeprecatedClasses](#DeprecatedClasses) | No | warns about deprecated css classes |
| [ErbSafety](#ErbSafety) | No | detects unsafe interpolation of ruby data into various javascript contexts and enforce usage of safe helpers like `.to_json`. |
| [Rubocop](#Rubocop) | No | runs RuboCop rules on ruby statements found in ERB templates |

| [RequireScriptNonce](#RequireScriptNonce) | No | warns about missing [Content Security Policy nonces](https://guides.rubyonrails.org/security.html#content-security-policy) in script tags |

### DeprecatedClasses

Expand Down Expand Up @@ -340,6 +340,45 @@ Linter-Specific Option | Description
-----------------------|---------------------------------------------------------
`correction_style` | When configured with `cdata`, adds CDATA markers. When configured with `plain`, don't add makers. Defaults to `cdata`.

### RequireScriptNonce
This linter prevents the usage of HTML `<script>`, Rails `javascript_tag`, `javascript_include_tag` and `javascript_pack_tag` without a `nonce` argument. The purpose of such a check is to ensure that when [content securty policy](https://edgeguides.rubyonrails.org/security.html#content-security-policy) is implemented in an application, there is a means of discovering tags that need to be updated with a `nonce` argument to enable script execution at application runtime.

```
Bad ❌
<script>
alert(1)
</script>
Good ✅
<script nonce="<%= request.content_security_policy_nonce %>" >
alert(1)
</script>
```
```
Bad ❌
<%= javascript_tag do -%>
alert(1)
<% end -%>
Good ✅
<%= javascript_tag nonce: true do -%>
alert(1)
<% end -%>
```
```
Bad ❌
<%= javascript_include_tag "script" %>
Good ✅
<%= javascript_include_tag "script", nonce: true %>
```
```
Bad ❌
<%= javascript_pack_tag "script" %>
Good ✅
<%= javascript_pack_tag "script", nonce: true %>
```
### SelfClosingTag
This linter enforces self closing tag styles for void elements.
Expand Down
92 changes: 92 additions & 0 deletions lib/erb_lint/linters/require_script_nonce.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# frozen_string_literal: true

require 'better_html'
require 'better_html/tree/tag'

module ERBLint
module Linters
# Allow inline script tags in ERB that have a nonce attribute.
# This only validates inline <script> tags, as well as rails helpers like javascript_tag.
class RequireScriptNonce < Linter
include LinterRegistry

def run(processed_source)
parser = processed_source.parser

find_html_script_tags(parser)
find_rails_helper_script_tags(parser)
end

private

def find_html_script_tags(parser)
parser.nodes_with_type(:tag).each do |tag_node|
tag = BetterHtml::Tree::Tag.from_node(tag_node)
nonce_attribute = tag.attributes['nonce']

next if !html_javascript_tag?(tag) || nonce_present?(nonce_attribute)

add_offense(
tag_node.to_a[1].loc,
"Missing a nonce attribute. Use request.content_security_policy_nonce",
[nonce_attribute]
)
end
end

def nonce_present?(nonce_attribute)
nonce_attribute.present? && nonce_attribute.value_node.present?
end

def html_javascript_tag?(tag)
!tag.closing? &&
(tag.name == 'script' && !html_javascript_type_attribute?(tag))
end

def html_javascript_type_attribute?(tag)
type_attribute = tag.attributes['type']

type_attribute &&
type_attribute.value_node.present? &&
type_attribute.value_node.to_a[1] != 'text/javascript' &&
type_attribute.value_node.to_a[1] != 'application/javascript'
end

def find_rails_helper_script_tags(parser)
parser.ast.descendants(:erb).each do |erb_node|
indicator_node, _, code_node, _ = *erb_node
source = code_node.loc.source
ruby_node = extract_ruby_node(source)
send_node = ruby_node&.descendants(:send)&.first

next if code_comment?(indicator_node) ||
!ruby_node ||
!tag_helper?(send_node) ||
source.include?("nonce")

add_offense(
erb_node.loc,
"Missing a nonce attribute. Use nonce: true",
[erb_node, send_node]
)
end
end

def tag_helper?(send_node)
send_node&.method_name?(:javascript_tag) ||
send_node&.method_name?(:javascript_include_tag) ||
send_node&.method_name?(:javascript_pack_tag)
end

def code_comment?(indicator_node)
indicator_node&.loc&.source == '#'
end

def extract_ruby_node(source)
BetterHtml::TestHelper::RubyNode.parse(source)
rescue ::Parser::SyntaxError
nil
end
end
end
end
193 changes: 193 additions & 0 deletions spec/erb_lint/linters/require_script_nonce_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
# frozen_string_literal: true

require 'spec_helper'

describe ERBLint::Linters::RequireScriptNonce do
let(:linter_config) { described_class.config_schema.new }
let(:file_loader) { ERBLint::FileLoader.new('.') }
let(:linter) { described_class.new(file_loader, linter_config) }
let(:processed_source) { ERBLint::ProcessedSource.new('file.rb', file) }
let(:html_nonce_message) { 'Missing a nonce attribute. Use request.content_security_policy_nonce' }
let(:tag_helper_nonce_message) { 'Missing a nonce attribute. Use nonce: true' }

subject { linter.offenses }

before { linter.run(processed_source) }

describe 'Pure HTML Linting' do
let(:file) { "<script #{mime_type} #{nonce}>" }
let(:mime_type) { nil }

context 'when nonce is present' do
let(:nonce) { 'nonce="whatever"' }

context 'when MIME type is text/javascript' do
let(:mime_type) { 'type="text/javascript"' }

it "passes the nonce check" do
expect(subject).to(eq([]))
end
end

context 'when MIME type is application/javascript' do
let(:mime_type) { 'type="application/javascript"' }

it "passes the nonce check" do
expect(subject).to(eq([]))
end
end

context 'when MIME type is not specificed' do
let(:mime_type) { nil }

it "passes the nonce check" do
expect(subject).to(eq([]))
end
end

context 'when MIME type is not text/javascript' do
let(:mime_type) { 'type="text/whatever"' }

it "passes the nonce check" do
expect(subject).to(eq([]))
end
end

context 'when MIME type is not application/javascript' do
let(:mime_type) { 'type="application/whatever"' }

it "passes the nonce check" do
expect(subject).to(eq([]))
end
end
end

context 'when nonce has no value' do
let(:nonce) { 'nonce' }

context 'when MIME type is text/javascript' do
let(:mime_type) { 'type="text/javascript"' }

it "builds an offense for a HTML script tag with a missing nonce" do
expect(subject).to(eq([
build_offense(1..6, html_nonce_message),
]))
end
end

context 'when MIME type is application/javascript' do
let(:mime_type) { 'type="application/javascript"' }

it "builds an offense for a HTML script tag with a missing nonce" do
expect(subject).to(eq([
build_offense(1..6, html_nonce_message),
]))
end
end

context 'when MIME type is not specified' do
let(:mime_type) { nil }

it "builds an offense for a HTML script tag with a missing nonce" do
expect(subject).to(eq([
build_offense(1..6, html_nonce_message),
]))
end
end

context 'when MIME type is not text/javascript or application/javascript' do
let(:mime_type) { 'type="text/whatever"' }

it "passes the nonce check" do
expect(subject).to(eq([]))
end
end
end

context 'when nonce is not present' do
let(:nonce) { nil }

it "builds an offense for a HTML script tag with a missing nonce" do
expect(subject).to(eq([
build_offense(1..6, html_nonce_message),
]))
end
end
end

describe 'Javascript helper tags linting' do
context 'usage of javascript_tag helper without nonce' do
let(:file) { <<~FILE }
<br />
<%= javascript_tag do %>
FILE

it "builds an offense for a Rails helper script tag with a missing nonce" do
expect(subject).to(eq([build_offense(7..30, tag_helper_nonce_message)]))
end
end

context 'usage of javascript_include_tag helper without nonce' do
let(:file) { <<~FILE }
<br />
<%= javascript_include_tag "script" %>
FILE

it "builds an offense for a Rails helper script tag with a missing nonce" do
expect(subject).to(eq([build_offense(7..44, tag_helper_nonce_message)]))
end
end

context 'usage of javascript_pack_tag helper without nonce' do
let(:file) { <<~FILE }
<br />
<%= javascript_pack_tag "script" %>
FILE

it "builds an offense for a Rails helper script tag with a missing nonce" do
expect(subject).to(eq([build_offense(7..41, tag_helper_nonce_message)]))
end
end

context 'usage of javascript_tag helper with a nonce' do
let(:file) { <<~FILE }
<br />
<%= javascript_tag nonce: true do %>
FILE

it "passes the nonce check" do
expect(subject).to(eq([]))
end
end

context 'usage of javascript_include_tag helper with a nonce' do
let(:file) { <<~FILE }
<br />
<%= javascript_include_tag "script", nonce: true %>
FILE

it "passes the nonce check" do
expect(subject).to(eq([]))
end
end

context 'usage of javascript_pack_tag helper with a nonce' do
let(:file) { <<~FILE }
<br />
<%= javascript_pack_tag "script", nonce: true %>
FILE

it "passes the nonce check" do
expect(subject).to(eq([]))
end
end
end

def build_offense(range, message)
ERBLint::Offense.new(
linter,
processed_source.to_source_range(range),
message
)
end
end

0 comments on commit f4b9380

Please sign in to comment.