Implement RequireScriptNonce #201
Conversation
This new linter allows us to look for script tags with missing CSP nonce attributes
|
👋 Hello @exterm and @jhottenstein - what are your thoughts on rolling this check into erb-lint? If you don't think it's in line with the goals of the linter, I can just extract it to a module for my own use :) |
| context 'usage of javascript_tag helper with a well formed nonce tag' do | ||
| let(:file) { <<~FILE } | ||
| <br /> | ||
| <%= javascript_tag do nonce: true %> |
There was a problem hiding this comment.
Should not this be:
| <%= javascript_tag do nonce: true %> | |
| <%= javascript_tag nonce: true do %> |
?
| context 'usage of javascript_include_tag helper with a well formed nonce tag' do | ||
| let(:file) { <<~FILE } | ||
| <br /> | ||
| <%= javascript_include_tag "script" nonce: true %> |
There was a problem hiding this comment.
There is a syntax error here. I was expecting this to fail
| <%= javascript_include_tag "script" nonce: true %> | |
| <%= javascript_include_tag "script", nonce: true %> |
There was a problem hiding this comment.
Good catch! Not sure why the parser didn't choke on this one, but it is updated now :)
| # type attribute = something other than text/javascript | ||
| next if type_attribute && | ||
| type_attribute.value_node.present? && | ||
| type_attribute.value_node.to_a[1] != 'text/javascript' |
There was a problem hiding this comment.
Should we check "application/javascript" as well?
We should also extract this check to a private method with a meaningful name
There was a problem hiding this comment.
All set! application/javascript has been added, and the logic here (and below) has been extracted to private methods.
| nonce_attribute = tag.attributes['nonce'] | ||
| nonce_present = nonce_attribute.present? && nonce_attribute.value_node.present? | ||
|
|
||
| next if nonce_present |
There was a problem hiding this comment.
Same here to a private metod
| send_node&.method_name?(:javascript_include_tag) || | ||
| send_node&.method_name?(:javascript_pack_tag) | ||
|
|
||
| next if source.include?("nonce: true") |
There was a problem hiding this comment.
"nonce" => true is also valid so we probably should check that.
There was a problem hiding this comment.
Good point - instead of handling all of the possible syntaxes (hash rockets, symbols, single & double quotes, etc) we made it a bit more generic and pass if the word nonce is present in any form.
|
👋 Hey @rafaelfranca thanks for merging this! Would you possibly be able to cut a release for this? |
This new linter allows us to look for script tags with missing CSP nonce attributes