diff --git a/lib/report_adapter.rb b/lib/report_adapter.rb index 16503f6..3e57b09 100644 --- a/lib/report_adapter.rb +++ b/lib/report_adapter.rb @@ -13,7 +13,7 @@ class ReportAdapter end def summary(report) - "**Brakeman Report**: \n - #{security_warnings(report)} security warnings" + "**Brakeman Report**:\n#{security_warnings(report)} security warnings\n#{check_table(report)}" end def annotations(report) @@ -31,6 +31,14 @@ class ReportAdapter private + def check_table(report) + uniq_checks(report).reduce('') { |memo, check| memo + "- [#{check[:check_name]}](#{check[:link]})\n" } + end + + def uniq_checks(report) + report['warnings'].map { |w| { check_name: w['check_name'], link: w['link'] } }.uniq { |w| w[:check_name] } + end + def security_warnings(report) report['scan_info']['security_warnings'] end diff --git a/spec/fixtures/output/annotations.json b/spec/fixtures/output/annotations.json new file mode 100644 index 0000000..4d53208 --- /dev/null +++ b/spec/fixtures/output/annotations.json @@ -0,0 +1,34 @@ +[ + { + "path": "app/controllers/posts_controller.rb", + "start_line": 29, + "end_line": 29, + "annotation_level": "warning", + "title": "High - Evaluation", + "message": "User input in eval" + }, + { + "path": "app/controllers/posts_controller.rb", + "start_line": 18, + "end_line": 18, + "annotation_level": "warning", + "title": "High - MassAssignment", + "message": "Parameters should be whitelisted for mass assignment" + }, + { + "path": "app/controllers/posts_controller.rb", + "start_line": 19, + "end_line": 19, + "annotation_level": "warning", + "title": "High - MassAssignment", + "message": "Parameters should be whitelisted for mass assignment" + }, + { + "path": "app/controllers/posts_controller.rb", + "start_line": 13, + "end_line": 13, + "annotation_level": "warning", + "title": "Medium - SQL", + "message": "Possible SQL injection" + } +] diff --git a/spec/fixtures/output/summary.md b/spec/fixtures/output/summary.md new file mode 100644 index 0000000..1f35bb5 --- /dev/null +++ b/spec/fixtures/output/summary.md @@ -0,0 +1,5 @@ +**Brakeman Report**: +4 security warnings +- [Evaluation](https://brakemanscanner.org/docs/warning_types/dangerous_eval/) +- [MassAssignment](https://brakemanscanner.org/docs/warning_types/mass_assignment/) +- [SQL](https://brakemanscanner.org/docs/warning_types/sql_injection/) diff --git a/spec/fixtures/input.json b/spec/fixtures/report.json similarity index 50% rename from spec/fixtures/input.json rename to spec/fixtures/report.json index ee03e71..adc0b19 100644 --- a/spec/fixtures/input.json +++ b/spec/fixtures/report.json @@ -1,11 +1,11 @@ { "scan_info": { - "app_path": "/home/miguemasx/developer/dockerize-rails", + "app_path": "/home/masx/developer/dockerize-rails", "rails_version": "5.2.2", - "security_warnings": 1, - "start_time": "2019-10-25 11:25:31 +0200", - "end_time": "2019-10-25 11:25:31 +0200", - "duration": 0.108293375, + "security_warnings": 4, + "start_time": "2019-11-01 22:07:50 +0100", + "end_time": "2019-11-01 22:07:50 +0100", + "duration": 0.166390277, "checks_performed": [ "BasicAuth", "BasicAuthTimingAttack", @@ -78,9 +78,28 @@ "number_of_models": 2, "number_of_templates": 7, "ruby_version": "2.6.3", - "brakeman_version": "4.7.0" + "brakeman_version": "4.7.1" }, "warnings": [ + { + "warning_type": "Dangerous Eval", + "warning_code": 13, + "fingerprint": "490f917262d1384df60e24097536f6c9039e5f5ba11fe11bf0981109cd286fc5", + "check_name": "Evaluation", + "message": "User input in eval", + "file": "app/controllers/posts_controller.rb", + "line": 29, + "link": "https://brakemanscanner.org/docs/warning_types/dangerous_eval/", + "code": "eval(params)", + "render_path": null, + "location": { + "type": "method", + "class": "PostsController", + "method": "create" + }, + "user_input": "params", + "confidence": "High" + }, { "warning_type": "Mass Assignment", "warning_code": 70, @@ -88,7 +107,7 @@ "check_name": "MassAssignment", "message": "Parameters should be whitelisted for mass assignment", "file": "app/controllers/posts_controller.rb", - "line": 17, + "line": 18, "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", "code": "params.permit!", "render_path": null, @@ -99,9 +118,53 @@ }, "user_input": null, "confidence": "High" + }, + { + "warning_type": "Mass Assignment", + "warning_code": 70, + "fingerprint": "5b486a498b14e1a12361c50863e2770c966799c9d5c6b6b9ab9bd8797c28a986", + "check_name": "MassAssignment", + "message": "Parameters should be whitelisted for mass assignment", + "file": "app/controllers/posts_controller.rb", + "line": 19, + "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", + "code": "params.permit!", + "render_path": null, + "location": { + "type": "method", + "class": "PostsController", + "method": "new" + }, + "user_input": null, + "confidence": "High" + }, + { + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "6e457f0a360661641a71555352f13a7cb2d983916b936007fce4e3826b837402", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/controllers/posts_controller.rb", + "line": 13, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "User.where(\"#{params[:query]}\")", + "render_path": null, + "location": { + "type": "method", + "class": "PostsController", + "method": "show" + }, + "user_input": "params[:query]", + "confidence": "Medium" } ], - "ignored_warnings": [], - "errors": [], - "obsolete": [] + "ignored_warnings": [ + + ], + "errors": [ + + ], + "obsolete": [ + + ] } diff --git a/spec/github_check_run_service_spec.rb b/spec/github_check_run_service_spec.rb index 7fb5dac..fbd86b1 100644 --- a/spec/github_check_run_service_spec.rb +++ b/spec/github_check_run_service_spec.rb @@ -3,7 +3,7 @@ require './spec/spec_helper' describe GithubCheckRunService do - let(:brakeman_report) { JSON(File.read('./spec/fixtures/input.json')) } + let(:brakeman_report) { JSON(File.read('./spec/fixtures/report.json')) } let(:github_data) { { sha: 'sha', token: 'token', owner: 'owner', repo: 'repository_name' } } let(:service) { GithubCheckRunService.new(brakeman_report, github_data, ReportAdapter) } diff --git a/spec/report_adapter_spec.rb b/spec/report_adapter_spec.rb index 42bf2ed..132dec7 100644 --- a/spec/report_adapter_spec.rb +++ b/spec/report_adapter_spec.rb @@ -4,7 +4,15 @@ require './spec/spec_helper' describe ReportAdapter do let(:brakeman_report) do - JSON(File.read('./spec/fixtures/input.json')) + JSON(File.read('./spec/fixtures/report.json')) + end + + let(:spec_annotations) do + JSON(File.read('./spec/fixtures/output/annotations.json')) + end + + let(:spec_summary) do + File.read('./spec/fixtures/output/summary.md') end let(:adapter) { ReportAdapter } @@ -16,18 +24,11 @@ describe ReportAdapter do it '.summary' do result = adapter.summary(brakeman_report) - expect(result).to be_a(String) + expect(result).to eq(spec_summary) end it '.annotations' do result = adapter.annotations(brakeman_report) - expect(result).to eq([{ - 'path' => 'app/controllers/posts_controller.rb', - 'start_line' => 17, - 'annotation_level' => 'warning', - 'end_line' => 17, - 'title' => 'High - MassAssignment', - 'message' => 'Parameters should be whitelisted for mass assignment' - }]) + expect(result).to eq(spec_annotations) end end