From b4ad4864b9457fb1662df1c4599769c7923f267d Mon Sep 17 00:00:00 2001 From: Jason Frey Date: Tue, 17 Sep 2024 17:13:40 -0400 Subject: [PATCH 1/2] Add specs for BrakemanFingerprintPatch --- .../brakeman_fingerprint_patch_spec.rb | 168 ++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 spec/lib/extensions/brakeman_fingerprint_patch_spec.rb diff --git a/spec/lib/extensions/brakeman_fingerprint_patch_spec.rb b/spec/lib/extensions/brakeman_fingerprint_patch_spec.rb new file mode 100644 index 00000000000..b3dde1f34f5 --- /dev/null +++ b/spec/lib/extensions/brakeman_fingerprint_patch_spec.rb @@ -0,0 +1,168 @@ +RSpec.describe BrakemanFingerprintPatch do + subject do + Class.new do + include BrakemanFingerprintPatch + attr_accessor :file + def initialize = @warning_code = 0 + def confidence = "High" + def location = {} + end.new + end + + shared_examples_for "handles brakeman pathing" do + before do + allow(Rails).to receive(:root).and_return(Pathname.new(core_path)) + allow(described_class).to receive(:rails_engine_paths).and_return([engine_path]) + end + + it "with an issue in the engine" do + subject.file = engine_issue + + expect(subject.file_string).to eq("(engine:manageiq-ui-classic) app/controllers/application_controller.rb") + expect(subject.fingerprint).to eq("1ea1c06c8976493622ad8c668f56df3a44aac997fabd57ab58fcf59a37712e56") + end + + it "with an issue in core" do + subject.file = core_issue + + expect(subject.file_string).to eq("lib/ansible/runner.rb") + expect(subject.fingerprint).to eq("f06e25d3b6fa417a80313b2ebd451fbbeac3670f03897e26e86983a5c29635c1") + end + end + + let(:core_path) { "/Users/user/dev/manageiq/" } + let(:core_issue) do + instance_double("Brakeman::FilePath", + :relative => "lib/ansible/runner.rb", + :absolute => "/Users/user/dev/manageiq/lib/ansible/runner.rb" + ) + end + + context "running from the core repo" do + context "with a git-based engine" do + context "in the system gem location" do + let(:engine_path) { "/Users/user/.gem/ruby/3.1.5/bundler/gems/manageiq-ui-classic-df1d9535ef51/" } + let(:engine_issue) do + instance_double("Brakeman::FilePath", + :relative => "../../.gem/ruby/3.1.5/bundler/gems/manageiq-ui-classic-df1d9535ef51/app/controllers/application_controller.rb", + :absolute => "/Users/user/.gem/ruby/3.1.5/bundler/gems/manageiq-ui-classic-df1d9535ef51/app/controllers/application_controller.rb" + ) + end + + include_examples "handles brakeman pathing" + end + + context "in a vendored gem location inside of the core repo" do # This is the core CI case + let(:engine_path) { "/Users/user/dev/manageiq/vendor/bundle/ruby/3.1.5/bundler/gems/manageiq-ui-classic-df1d9535ef51/" } + let(:engine_issue) do + instance_double("Brakeman::FilePath", + :relative => "vendor/bundle/ruby/3.1.5/bundler/gems/manageiq-ui-classic-df1d9535ef51/app/controllers/application_controller.rb", + :absolute => "/Users/user/dev/manageiq/vendor/bundle/ruby/3.1.5/bundler/gems/manageiq-ui-classic-df1d9535ef51/app/controllers/application_controller.rb" + ) + end + + include_examples "handles brakeman pathing" + end + end + + context "with a gem-based engine" do + context "in the system gem location" do + let(:engine_path) { "/Users/user/.gem/ruby/3.1.5/bundler/gems/manageiq-ui-classic-0.1.0/" } + let(:engine_issue) do + instance_double("Brakeman::FilePath", + :relative => "../../.gem/ruby/3.1.5/bundler/gems/manageiq-ui-classic-0.1.0/app/controllers/application_controller.rb", + :absolute => "/Users/user/.gem/ruby/3.1.5/bundler/gems/manageiq-ui-classic-0.1.0/app/controllers/application_controller.rb" + ) + end + + include_examples "handles brakeman pathing" + end + + context "in a vendored gem location inside of the core repo" do # This is a core CI case in a future where we might use a versioned gem + let(:engine_path) { "/Users/user/dev/manageiq/vendor/bundle/ruby/3.1.5/bundler/gems/manageiq-ui-classic-0.1.0/" } + let(:engine_issue) do + instance_double("Brakeman::FilePath", + :relative => "vendor/bundle/ruby/3.1.5/bundler/gems/manageiq-ui-classic-0.1.0/app/controllers/application_controller.rb", + :absolute => "/Users/user/dev/manageiq/vendor/bundle/ruby/3.1.5/bundler/gems/manageiq-ui-classic-0.1.0/app/controllers/application_controller.rb" + ) + end + + include_examples "handles brakeman pathing" + end + end + + context "with a path-based engine" do + context "which is a sibling of the core repo" do + let(:engine_path) { "/Users/user/dev/manageiq-ui-classic/" } + let(:engine_issue) do + instance_double("Brakeman::FilePath", + :relative => "../manageiq-ui-classic/app/controllers/application_controller.rb", + :absolute => "/Users/user/dev/manageiq-ui-classic/app/controllers/application_controller.rb" + ) + end + + include_examples "handles brakeman pathing" + end + + context "which is inside of the core repo" do + let(:engine_path) { "/Users/user/dev/manageiq/plugins/manageiq-ui-classic/" } + let(:engine_issue) do + instance_double("Brakeman::FilePath", + :relative => "plugins/manageiq-ui-classic/app/controllers/application_controller.rb", + :absolute => "/Users/user/dev/manageiq/plugins/manageiq-ui-classic/app/controllers/application_controller.rb" + ) + end + + include_examples "handles brakeman pathing" + end + end + end + + context "running from an engine repo" do + context "with a symlinked spec/manageiq dir" do # When symlinked, the paths appear the same as a path-based gem, so these are copies of the path-based tests above + context "which is a sibling of the core repo" do + let(:engine_path) { "/Users/user/dev/manageiq-ui-classic/" } + let(:engine_issue) do + instance_double("Brakeman::FilePath", + :relative => "../manageiq-ui-classic/app/controllers/application_controller.rb", + :absolute => "/Users/user/dev/manageiq-ui-classic/app/controllers/application_controller.rb" + ) + end + + include_examples "handles brakeman pathing" + end + + context "which is inside of the core repo" do + let(:engine_path) { "/Users/user/dev/manageiq/plugins/manageiq-ui-classic/" } + let(:engine_issue) do + instance_double("Brakeman::FilePath", + :relative => "plugins/manageiq-ui-classic/app/controllers/application_controller.rb", + :absolute => "/Users/user/dev/manageiq/plugins/manageiq-ui-classic/app/controllers/application_controller.rb" + ) + end + + include_examples "handles brakeman pathing" + end + end + + context "with a cloned spec/manageiq dir" do # This is also the CI case for a plugin + let(:core_path) { "/Users/user/dev/manageiq-ui-classic/spec/manageiq/" } + let(:core_issue) do + instance_double("Brakeman::FilePath", + :relative => "lib/ansible/runner.rb", + :absolute => "/Users/user/dev/manageiq-ui-classic/spec/manageiq/lib/ansible/runner.rb" + ) + end + + let(:engine_path) { "/Users/user/dev/manageiq-ui-classic/" } + let(:engine_issue) do + instance_double("Brakeman::FilePath", + :relative => "../../app/controllers/application_controller.rb", + :absolute => "/Users/user/dev/manageiq-ui-classic/app/controllers/application_controller.rb" + ) + end + + include_examples "handles brakeman pathing" + end + end +end From 00b8966a7f10d1192c462c513a5886cb9981c500 Mon Sep 17 00:00:00 2001 From: Jason Frey Date: Tue, 17 Sep 2024 17:15:18 -0400 Subject: [PATCH 2/2] Fix issue when running from an engine with a cloned spec/manageiq --- lib/extensions/brakeman_fingerprint_patch.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/extensions/brakeman_fingerprint_patch.rb b/lib/extensions/brakeman_fingerprint_patch.rb index dab14a39a33..18cc1bca84f 100644 --- a/lib/extensions/brakeman_fingerprint_patch.rb +++ b/lib/extensions/brakeman_fingerprint_patch.rb @@ -40,12 +40,12 @@ def self.rails_engine_paths # are standalone objects. def file_string engine_path = BrakemanFingerprintPatch.rails_engine_paths.detect { |p| self.file.absolute.start_with?(p) } - if engine_path + if engine_path.nil? || (Rails.root.to_s.start_with?(engine_path) && self.file.absolute.start_with?(Rails.root.to_s)) + self.file.relative + else engine_name = File.basename(engine_path).sub(/-\h+$/, "").sub(/-(?:\d+\.)+\d+$/, "") engine_relative = self.file.absolute.sub(engine_path, "") "(engine:#{engine_name}) #{engine_relative}" - else - self.file.relative end end @@ -65,6 +65,8 @@ def fingerprint def to_hash(absolute_paths: true) super.tap do |h| h[:file] = (absolute_paths ? self.file.absolute : file_string) + h[:file_rel] = self.file.relative + h[:file_abs] = self.file.absolute end end end