From 87c9d43dfbcdbbba33526b7f6f037198e6665ea1 Mon Sep 17 00:00:00 2001 From: Miguel Savignano Date: Fri, 1 Nov 2019 20:56:20 +0100 Subject: [PATCH 1/3] summary with brakeman links --- lib/report_adapter.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/report_adapter.rb b/lib/report_adapter.rb index 16503f6..e61eb89 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]})" } + end + + def uniq_checks(report) + report['warnings'].map { |w| { check_name: w['check_name'], link: w['link'] } }.uniq { |w| w[:checkname] } + end + def security_warnings(report) report['scan_info']['security_warnings'] end From 676c32372f6c26ccd3e5903086e7a34220c22f82 Mon Sep 17 00:00:00 2001 From: Miguel Savignano Date: Fri, 1 Nov 2019 22:23:15 +0100 Subject: [PATCH 2/3] refactor test annotations --- lib/report_adapter.rb | 6 +- spec/fixtures/annotations.json | 34 ++++++ spec/fixtures/report.json | 170 ++++++++++++++++++++++++++ spec/fixtures/summary.md | 5 + spec/github_check_run_service_spec.rb | 2 +- spec/report_adapter_spec.rb | 15 +-- 6 files changed, 219 insertions(+), 13 deletions(-) create mode 100644 spec/fixtures/annotations.json create mode 100644 spec/fixtures/report.json create mode 100644 spec/fixtures/summary.md diff --git a/lib/report_adapter.rb b/lib/report_adapter.rb index e61eb89..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\n #{check_table(report)}" + "**Brakeman Report**:\n#{security_warnings(report)} security warnings\n#{check_table(report)}" end def annotations(report) @@ -32,11 +32,11 @@ class ReportAdapter private def check_table(report) - uniq_checks(report).reduce('') { |memo, check| memo + " - [#{check[:check_name]}](#{check[:link]})" } + 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[:checkname] } + report['warnings'].map { |w| { check_name: w['check_name'], link: w['link'] } }.uniq { |w| w[:check_name] } end def security_warnings(report) diff --git a/spec/fixtures/annotations.json b/spec/fixtures/annotations.json new file mode 100644 index 0000000..4d53208 --- /dev/null +++ b/spec/fixtures/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/report.json b/spec/fixtures/report.json new file mode 100644 index 0000000..adc0b19 --- /dev/null +++ b/spec/fixtures/report.json @@ -0,0 +1,170 @@ +{ + "scan_info": { + "app_path": "/home/masx/developer/dockerize-rails", + "rails_version": "5.2.2", + "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", + "ContentTag", + "CookieSerialization", + "CreateWith", + "CrossSiteScripting", + "DefaultRoutes", + "Deserialize", + "DetailedExceptions", + "DigestDoS", + "DynamicFinders", + "EscapeFunction", + "Evaluation", + "Execute", + "FileAccess", + "FileDisclosure", + "FilterSkipping", + "ForgerySetting", + "HeaderDoS", + "I18nXSS", + "JRubyXML", + "JSONEncoding", + "JSONParsing", + "LinkTo", + "LinkToHref", + "MailTo", + "MassAssignment", + "MimeTypeDoS", + "ModelAttrAccessible", + "ModelAttributes", + "ModelSerialize", + "NestedAttributes", + "NestedAttributesBypass", + "NumberToCurrency", + "PermitAttributes", + "QuoteTableName", + "Redirect", + "RegexDoS", + "Render", + "RenderDoS", + "RenderInline", + "ResponseSplitting", + "RouteDoS", + "SQL", + "SQLCVEs", + "SSLVerify", + "SafeBufferManipulation", + "SanitizeMethods", + "SelectTag", + "SelectVulnerability", + "Send", + "SendFile", + "SessionManipulation", + "SessionSettings", + "SimpleFormat", + "SingleQuotes", + "SkipBeforeFilter", + "SprocketsPathTraversal", + "StripTags", + "SymbolDoSCVE", + "TranslateBug", + "UnsafeReflection", + "ValidationRegex", + "WithoutProtection", + "XMLDoS", + "YAMLParsing" + ], + "number_of_controllers": 2, + "number_of_models": 2, + "number_of_templates": 7, + "ruby_version": "2.6.3", + "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, + "fingerprint": "5b486a498b14e1a12361c50863e2770c966799c9d5c6b6b9ab9bd8797c28a986", + "check_name": "MassAssignment", + "message": "Parameters should be whitelisted for mass assignment", + "file": "app/controllers/posts_controller.rb", + "line": 18, + "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": "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": [ + + ] +} diff --git a/spec/fixtures/summary.md b/spec/fixtures/summary.md new file mode 100644 index 0000000..1f35bb5 --- /dev/null +++ b/spec/fixtures/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/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..7d72216 100644 --- a/spec/report_adapter_spec.rb +++ b/spec/report_adapter_spec.rb @@ -4,7 +4,11 @@ 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/annotations.json')) end let(:adapter) { ReportAdapter } @@ -21,13 +25,6 @@ describe ReportAdapter do 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 From 0f83cb17058b4bfff7f604910f76c2aa2d401ccb Mon Sep 17 00:00:00 2001 From: Miguel Savignano Date: Fri, 1 Nov 2019 22:26:49 +0100 Subject: [PATCH 3/3] refactor test --- spec/fixtures/input.json | 107 -------------------- spec/fixtures/{ => output}/annotations.json | 0 spec/fixtures/{ => output}/summary.md | 0 spec/report_adapter_spec.rb | 8 +- 4 files changed, 6 insertions(+), 109 deletions(-) delete mode 100644 spec/fixtures/input.json rename spec/fixtures/{ => output}/annotations.json (100%) rename spec/fixtures/{ => output}/summary.md (100%) diff --git a/spec/fixtures/input.json b/spec/fixtures/input.json deleted file mode 100644 index ee03e71..0000000 --- a/spec/fixtures/input.json +++ /dev/null @@ -1,107 +0,0 @@ -{ - "scan_info": { - "app_path": "/home/miguemasx/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, - "checks_performed": [ - "BasicAuth", - "BasicAuthTimingAttack", - "ContentTag", - "CookieSerialization", - "CreateWith", - "CrossSiteScripting", - "DefaultRoutes", - "Deserialize", - "DetailedExceptions", - "DigestDoS", - "DynamicFinders", - "EscapeFunction", - "Evaluation", - "Execute", - "FileAccess", - "FileDisclosure", - "FilterSkipping", - "ForgerySetting", - "HeaderDoS", - "I18nXSS", - "JRubyXML", - "JSONEncoding", - "JSONParsing", - "LinkTo", - "LinkToHref", - "MailTo", - "MassAssignment", - "MimeTypeDoS", - "ModelAttrAccessible", - "ModelAttributes", - "ModelSerialize", - "NestedAttributes", - "NestedAttributesBypass", - "NumberToCurrency", - "PermitAttributes", - "QuoteTableName", - "Redirect", - "RegexDoS", - "Render", - "RenderDoS", - "RenderInline", - "ResponseSplitting", - "RouteDoS", - "SQL", - "SQLCVEs", - "SSLVerify", - "SafeBufferManipulation", - "SanitizeMethods", - "SelectTag", - "SelectVulnerability", - "Send", - "SendFile", - "SessionManipulation", - "SessionSettings", - "SimpleFormat", - "SingleQuotes", - "SkipBeforeFilter", - "SprocketsPathTraversal", - "StripTags", - "SymbolDoSCVE", - "TranslateBug", - "UnsafeReflection", - "ValidationRegex", - "WithoutProtection", - "XMLDoS", - "YAMLParsing" - ], - "number_of_controllers": 2, - "number_of_models": 2, - "number_of_templates": 7, - "ruby_version": "2.6.3", - "brakeman_version": "4.7.0" - }, - "warnings": [ - { - "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": 17, - "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" - } - ], - "ignored_warnings": [], - "errors": [], - "obsolete": [] -} diff --git a/spec/fixtures/annotations.json b/spec/fixtures/output/annotations.json similarity index 100% rename from spec/fixtures/annotations.json rename to spec/fixtures/output/annotations.json diff --git a/spec/fixtures/summary.md b/spec/fixtures/output/summary.md similarity index 100% rename from spec/fixtures/summary.md rename to spec/fixtures/output/summary.md diff --git a/spec/report_adapter_spec.rb b/spec/report_adapter_spec.rb index 7d72216..132dec7 100644 --- a/spec/report_adapter_spec.rb +++ b/spec/report_adapter_spec.rb @@ -8,7 +8,11 @@ describe ReportAdapter do end let(:spec_annotations) do - JSON(File.read('./spec/fixtures/annotations.json')) + JSON(File.read('./spec/fixtures/output/annotations.json')) + end + + let(:spec_summary) do + File.read('./spec/fixtures/output/summary.md') end let(:adapter) { ReportAdapter } @@ -20,7 +24,7 @@ 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