scan .erb files + add run_all_checks config

Co-authored-by: Gavin Morrice <gavin@gavinmorrice.com>
prontolabs · Dec 9, 2021 · 78c5b69 · 78c5b69
1 parent 9785357
commit 78c5b69
Show file tree

Hide file tree

Showing 23 changed files with 105 additions and 13 deletions.
diff --git a/README.md b/README.md
@@ -11,3 +11,14 @@ Pronto runner for [Brakeman](https://github.com/presidentbeef/brakeman), securit
 Brakeman [Confidence](https://github.com/presidentbeef/brakeman#confidence-levels) is mapped to severity levels on the
 messages generated by Pronto. High confidence maps to fatal, medium confidence maps to warning, and low confidence maps
 to info.
+
+## Run all checks
+
+Brakeman also includes some optional checks and by setting the following in your `.pronto.yml` you can run every check included in the gem:
+
+```yaml
+brakeman:
+  run_all_checks: true
+```
+
+(This is the equivalent of running `brakeman -A` on the command line.)
diff --git a/lib/pronto/brakeman.rb b/lib/pronto/brakeman.rb
@@ -4,23 +4,25 @@
 module Pronto
   class Brakeman < Runner
     def run
-      files = ruby_patches.map do |patch|
+      patches = ruby_patches | erb_patches
+      files = patches.map do |patch|
         patch.new_file_full_path.relative_path_from(repo_path).to_s
-      end
+      end.sort
 
       return [] unless files.any?
 
       output = ::Brakeman.run(app_path: repo_path,
                               output_formats: [:to_s],
-                              only_files: files)
-      messages_for(ruby_patches, output).compact
+                              only_files: files,
+                              run_all_checks: run_all_checks?)
+       messages_for(patches, output).compact
     rescue ::Brakeman::NoApplication
       []
     end
 
-    def messages_for(ruby_patches, output)
+    def messages_for(code_patches, output)
       output.filtered_warnings.map do |warning|
-        patch = patch_for_warning(ruby_patches, warning)
+        patch = patch_for_warning(code_patches, warning)
 
         next unless patch
         line = patch.added_lines.find do |added_line|
@@ -49,10 +51,27 @@ def severity_for_confidence(confidence_level)
       end
     end
 
-    def patch_for_warning(ruby_patches, warning)
-      ruby_patches.find do |patch|
+    def patch_for_warning(code_patches, warning)
+      code_patches.find do |patch|
         patch.new_file_full_path.to_s == warning.file.absolute
       end
     end
+
+    def run_all_checks?
+      pronto_brakeman_config['run_all_checks']
+    end
+
+    def pronto_brakeman_config
+      pronto_brakeman_config ||= Pronto::ConfigFile.new.to_h['brakeman'] || {}
+    end
+
+    def erb_patches
+      @erb_patches ||= Array(@patches).select { |patch| patch.additions > 0 }
+                                      .select { |patch| erb_file?(patch.new_file_full_path) }
+    end
+
+    def erb_file?(path)
+      File.extname(path) == '.erb'
+    end
   end
 end
diff --git a/lib/pronto/brakeman/version.rb b/lib/pronto/brakeman/version.rb
@@ -1,5 +1,5 @@
 module Pronto
   module BrakemanVersion
-    VERSION = '0.11.0'.freeze
+    VERSION = '0.11.1'.freeze
   end
 end
diff --git a/spec/fixtures/test.git/app/views/layouts/application.html.erb b/spec/fixtures/test.git/app/views/layouts/application.html.erb
@@ -8,7 +8,11 @@
 </head>
 <body>
 
-<%= yield %>
+  <%= link_to some_url, target: "_blank" do -%>
+    Uh oh
+  <% end %>
+
+ <%= yield %>
 
 </body>
 </html>
diff --git a/spec/fixtures/test.git/git/COMMIT_EDITMSG b/spec/fixtures/test.git/git/COMMIT_EDITMSG
@@ -0,0 +1 @@
+Fix typo
diff --git a/spec/fixtures/test.git/git/index b/spec/fixtures/test.git/git/index
diff --git a/spec/fixtures/test.git/git/logs/HEAD b/spec/fixtures/test.git/git/logs/HEAD
@@ -1,2 +1,4 @@
 0000000000000000000000000000000000000000 da701274f54ae0fde7878e9834e5fb2e1148d48b Mindaugas Mozūras <mindaugas.mozuras@gmail.com> 1429988205 +0300	commit (initial): Initial commit
 da701274f54ae0fde7878e9834e5fb2e1148d48b 225af1ab522457873a5994c150d7ad571ff260c0 Mindaugas Mozūras <mindaugas.mozuras@gmail.com> 1429988422 +0300	commit: Add unsafe redirect_to with params
+225af1ab522457873a5994c150d7ad571ff260c0 b09de21aa02cbb43386e3a1d7e7e0a628df7ca66 Noah Berman <noah-berman@cookpad.com> 1638890290 +0000	commit: Add a link that is vulnerable to reverse tabnabbing
+b09de21aa02cbb43386e3a1d7e7e0a628df7ca66 737a2ccda4d398b65fd784dd34940c1abffa67f4 Noah Berman <noah-berman@cookpad.com> 1639049703 +0000	commit: Fix typo
diff --git a/spec/fixtures/test.git/git/logs/refs/heads/master b/spec/fixtures/test.git/git/logs/refs/heads/master
@@ -1,2 +1,4 @@
 0000000000000000000000000000000000000000 da701274f54ae0fde7878e9834e5fb2e1148d48b Mindaugas Mozūras <mindaugas.mozuras@gmail.com> 1429988205 +0300	commit (initial): Initial commit
 da701274f54ae0fde7878e9834e5fb2e1148d48b 225af1ab522457873a5994c150d7ad571ff260c0 Mindaugas Mozūras <mindaugas.mozuras@gmail.com> 1429988422 +0300	commit: Add unsafe redirect_to with params
+225af1ab522457873a5994c150d7ad571ff260c0 b09de21aa02cbb43386e3a1d7e7e0a628df7ca66 Noah Berman <noah-berman@cookpad.com> 1638890290 +0000	commit: Add a link that is vulnerable to reverse tabnabbing
+b09de21aa02cbb43386e3a1d7e7e0a628df7ca66 737a2ccda4d398b65fd784dd34940c1abffa67f4 Noah Berman <noah-berman@cookpad.com> 1639049703 +0000	commit: Fix typo
diff --git a/spec/fixtures/test.git/git/objects/01/7afdbc86e35b2aeacb2c67673c66de76b2ef6f b/spec/fixtures/test.git/git/objects/01/7afdbc86e35b2aeacb2c67673c66de76b2ef6f
diff --git a/spec/fixtures/test.git/git/objects/1a/434a7ca370b8a4e7dbe269824b462c735b1106 b/spec/fixtures/test.git/git/objects/1a/434a7ca370b8a4e7dbe269824b462c735b1106
diff --git a/spec/fixtures/test.git/git/objects/33/2406c5ad51ed6976ac26bf363994aa9085989d b/spec/fixtures/test.git/git/objects/33/2406c5ad51ed6976ac26bf363994aa9085989d
diff --git a/spec/fixtures/test.git/git/objects/70/8b611a518b602dafee3fc62eb0e1e97e32a794 b/spec/fixtures/test.git/git/objects/70/8b611a518b602dafee3fc62eb0e1e97e32a794
diff --git a/spec/fixtures/test.git/git/objects/73/7a2ccda4d398b65fd784dd34940c1abffa67f4 b/spec/fixtures/test.git/git/objects/73/7a2ccda4d398b65fd784dd34940c1abffa67f4
diff --git a/spec/fixtures/test.git/git/objects/83/8853977ce26e7891807974dafaea83c9547587 b/spec/fixtures/test.git/git/objects/83/8853977ce26e7891807974dafaea83c9547587
diff --git a/spec/fixtures/test.git/git/objects/b0/9de21aa02cbb43386e3a1d7e7e0a628df7ca66 b/spec/fixtures/test.git/git/objects/b0/9de21aa02cbb43386e3a1d7e7e0a628df7ca66
@@ -0,0 +1,3 @@
+x�O͒�@�y��[��af��T@Yt5(�.{�F@ѭ��C6לҧ�n�TU��ŧ�5�HJ>RBh�#��$J�)Ѡi�G�䂭�;`���3fs!��R��8M�\X�#s���]޴6��g�
+k�R��)��uӔL?�����P�)
+ڃ�n?�3�]�]�[���x~0a�`3Bw�[�>���]/����0�� ݾ���$ZZ�ċ�峷��\3��Ǩď<�I�r[���KG >p7f�#/�S¨;��h�G;=�6����3�-OS������>ț�\��S.���4��s���g�t�n�7񣽆��{uĂ����,}�V�.�Kw-?�_Ks���[?���+q��E]B�c��smZL��Z�0����Ԙ$E���[#��

diff --git a/spec/fixtures/test.git/git/objects/c1/35cd22d47f5639e61723a47debd5a6d0fe3af0 b/spec/fixtures/test.git/git/objects/c1/35cd22d47f5639e61723a47debd5a6d0fe3af0
diff --git a/spec/fixtures/test.git/git/objects/dc/0e362153d2e76f0a7ea457d665b3eb9f4aaf83 b/spec/fixtures/test.git/git/objects/dc/0e362153d2e76f0a7ea457d665b3eb9f4aaf83
diff --git a/spec/fixtures/test.git/git/objects/eb/593c37df1cf6c6904adb9fa5f27f1757939802 b/spec/fixtures/test.git/git/objects/eb/593c37df1cf6c6904adb9fa5f27f1757939802
diff --git a/spec/fixtures/test.git/git/objects/ec/0e22e42de60b4180b3762557e605a3062c1d40 b/spec/fixtures/test.git/git/objects/ec/0e22e42de60b4180b3762557e605a3062c1d40
diff --git a/spec/fixtures/test.git/git/objects/fc/ce070251b258ea92c18cae48bacafb8179a140 b/spec/fixtures/test.git/git/objects/fc/ce070251b258ea92c18cae48bacafb8179a140
diff --git a/spec/fixtures/test.git/git/refs/heads/master b/spec/fixtures/test.git/git/refs/heads/master
@@ -1 +1 @@
-225af1ab522457873a5994c150d7ad571ff260c0
+737a2ccda4d398b65fd784dd34940c1abffa67f4
diff --git a/spec/pronto/brakeman_spec.rb b/spec/pronto/brakeman_spec.rb
@@ -3,6 +3,14 @@
 module Pronto
   describe Brakeman do
     let(:brakeman) { Brakeman.new(patches) }
+    let(:pronto_config) do
+      instance_double Pronto::ConfigFile, to_h: config_hash
+    end
+    let(:config_hash) { {} }
+
+    before do
+      allow(Pronto::ConfigFile).to receive(:new) { pronto_config }
+    end
 
     describe '#run' do
       subject { brakeman.run }
@@ -33,6 +41,48 @@ module Pronto
             'Possible security vulnerability: [Possible unprotected redirect](https://brakemanscanner.org/docs/warning_types/redirect/)'
         end
       end
+
+      context 'with a change to an erb file' do
+        context 'with brakeman not included in pronto config' do
+          let(:config_hash) { { 'foo' => {} } }
+          include_context 'test repo'
+          let(:patches) { repo.diff('b09de21aa02cbb43386e3a1d7e7e0a628df7ca66') }
+
+          it 'should disable all checks' do
+            expect(brakeman.run_all_checks?).to eq nil
+          end
+
+          its(:count) { should == 0 }
+        end
+
+        context 'with brakeman included in pronto config' do
+          context 'with run all checks disabled' do
+            let(:config_hash) { { 'brakeman' => { 'run_all_checks' => false } } }
+            include_context 'test repo'
+            let(:patches) { repo.diff('b09de21aa02cbb43386e3a1d7e7e0a628df7ca66') }
+
+            it 'should disable all checks' do
+              expect(brakeman.run_all_checks?).to eq false
+            end
+
+            its(:count) { should == 0 }
+          end
+
+          context 'with run all checks enabled' do
+            let(:config_hash) { { 'brakeman' => { 'run_all_checks' => true } } }
+            include_context 'test repo'
+            let(:patches) { repo.diff('b09de21aa02cbb43386e3a1d7e7e0a628df7ca66') }
+
+            it 'should enable all checks' do
+              expect(brakeman.run_all_checks?).to eq true
+            end
+            its(:count) { should == 1 }
+            it "should report a tabnabbing vulnerability" do
+              expect(subject.first.msg).to include("Possible security vulnerability: [When opening a link in a new tab without setting `rel:")
+            end
+          end
+        end
+      end
     end
 
     describe "#severity_for_confidence" do

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
@@ -13,6 +13,6 @@
 end
 
 RSpec.configure do |config|
-  config.expect_with(:rspec) { |c| c.syntax = :should }
-  config.mock_with(:rspec) { |c| c.syntax = :should }
+  config.expect_with(:rspec) { |c| c.syntax = [:should, :expect] }
+  config.mock_with(:rspec) { |c| c.syntax =  [:should, :expect] }
 end