Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add toggle to control if Brakeman scans changed files, or all files #23

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 10 additions & 3 deletions lib/pronto/brakeman.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,16 @@ def run

return [] unless files.any?

output = ::Brakeman.run(app_path: repo_path,
output_formats: [:to_s],
only_files: files)
brakeman_options = {
app_path: repo_path,
output_formats: [:to_s],
}

if ENV.fetch('PRONTO_BRAKEMAN_ONLY_FILES', true) != '0'
brakeman_options[:only_files] = files
end

output = ::Brakeman.run(**brakeman_options)
messages_for(ruby_patches, output).compact
rescue ::Brakeman::NoApplication
[]
Expand Down
2 changes: 2 additions & 0 deletions spec/fixtures/files/brakeman-run-all-checks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
:run_all_checks: true
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,8 @@ class ApplicationController < ActionController::Base
def index
redirect_to params
end

def show
Account.find(params[:id])
end
end
3 changes: 3 additions & 0 deletions spec/fixtures/test.git/app/models/account.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class Account < ActiveRecord::Base
belongs_to :user
end
2 changes: 2 additions & 0 deletions spec/fixtures/test.git/app/models/user.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class User < ActiveRecord::Base
end
1 change: 1 addition & 0 deletions spec/fixtures/test.git/git/ORIG_HEAD
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
369f92e8243a2ee803ec3d042d7c55e853e50fce
2 changes: 2 additions & 0 deletions spec/fixtures/test.git/git/config
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@
logallrefupdates = true
ignorecase = true
precomposeunicode = true
[receive]
denyCurrentBranch = ignore
Binary file modified spec/fixtures/test.git/git/index
Binary file not shown.
2 changes: 2 additions & 0 deletions spec/fixtures/test.git/git/logs/HEAD
Original file line number Diff line number Diff line change
@@ -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 369f92e8243a2ee803ec3d042d7c55e853e50fce Daniel Holz <dgholz@gmail.com> 1590684278 +0100 push
369f92e8243a2ee803ec3d042d7c55e853e50fce 369f92e8243a2ee803ec3d042d7c55e853e50fce Daniel Holz <dgholz@gmail.com> 1590684354 +0100 reset: moving to HEAD
1 change: 1 addition & 0 deletions spec/fixtures/test.git/git/logs/refs/heads/master
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
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 369f92e8243a2ee803ec3d042d7c55e853e50fce Daniel Holz <dgholz@gmail.com> 1590684278 +0100 push
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
xU�MK1�q��xQ�ċ��Z(x+zYf��64�Yf����������I�����ÕOd�fS�T��䢒+����'��x\c��ɹ`����B�h�f(E�y���qX_�V����0˄�f���d ���Zc�
����Q��/m�2��螵�X��U��>�7��.�4X=s���t �e�e�W{�]��>n��|ўf
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
2 changes: 1 addition & 1 deletion spec/fixtures/test.git/git/refs/heads/master
Original file line number Diff line number Diff line change
@@ -1 +1 @@
225af1ab522457873a5994c150d7ad571ff260c0
369f92e8243a2ee803ec3d042d7c55e853e50fce
33 changes: 29 additions & 4 deletions spec/pronto/brakeman_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,26 +33,51 @@ module Pronto
'Possible security vulnerability: [Possible unprotected redirect](https://brakemanscanner.org/docs/warning_types/redirect/)'
end
end

context 'patches with an unscoped find' do
include_context 'test repo'
include_context 'brakeman runs all checks'
let(:patches) { repo.diff('369f92e^') }

its(:count) { should == 0 }
end

context 'patches with an unscoped find and scans all files' do
include_context 'test repo'
include_context 'brakeman runs all checks'
before(:context) do
@original_env = ENV.to_hash
ENV['PRONTO_BRAKEMAN_ONLY_FILES'] = '0'
end
after(:context) { ENV.replace @original_env }
let(:patches) { repo.diff('369f92e^') }

its(:count) { should == 1 }
its(:'first.msg') do
should =~
/^Possible security vulnerability: \[Unscoped call to /
end
end
end

describe "#severity_for_confidence" do
describe '#severity_for_confidence' do
subject { brakeman.severity_for_confidence(confidence_level) }

let(:patches) { nil }

context "when confidence is HIGH" do
context 'when confidence is HIGH' do
let(:confidence_level) { 0 }

it { should == :fatal }
end

context "when confidence is MEDIUM" do
context 'when confidence is MEDIUM' do
let(:confidence_level) { 1 }

it { should == :warning }
end

context "when confidence is anything else" do
context 'when confidence is anything else' do
let(:confidence_level) { 2 }

it { should == :info }
Expand Down
6 changes: 6 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
after { FileUtils.mv(dot_git, git) }
end

RSpec.shared_context 'brakeman runs all checks' do
let(:brakeman_config) { repo.path.join('config', 'brakeman.yml') }
before { FileUtils.cp('spec/fixtures/files/brakeman-run-all-checks.yml', brakeman_config) }
after { FileUtils.rm(brakeman_config) }
end

RSpec.configure do |config|
config.expect_with(:rspec) { |c| c.syntax = :should }
config.mock_with(:rspec) { |c| c.syntax = :should }
Expand Down