From 4b111626cb2ab4c21dcbcf1348a76782c28636b1 Mon Sep 17 00:00:00 2001 From: Kelv Date: Thu, 27 Jun 2024 22:17:56 +0800 Subject: [PATCH] DEV: Remove invalid content_security_policy_script_src site setting values from DB (#27588) * DEV: add db migration to filter out invalid csp script source values * DEV: insert UserHistory row during data migration to track old value for content_security_policy_script_src site setting --- ...alid_csp_script_src_site_setting_values.rb | 37 ++++++++++ ...move_invalid_csp_script_src_values_spec.rb | 72 +++++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 db/migrate/20240627125112_remove_invalid_csp_script_src_site_setting_values.rb create mode 100644 spec/migrations/remove_invalid_csp_script_src_values_spec.rb diff --git a/db/migrate/20240627125112_remove_invalid_csp_script_src_site_setting_values.rb b/db/migrate/20240627125112_remove_invalid_csp_script_src_site_setting_values.rb new file mode 100644 index 00000000000..149f468999a --- /dev/null +++ b/db/migrate/20240627125112_remove_invalid_csp_script_src_site_setting_values.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class RemoveInvalidCspScriptSrcSiteSettingValues < ActiveRecord::Migration[7.0] + def up + prev_value = + DB.query_single( + "SELECT value FROM site_settings WHERE name = 'content_security_policy_script_src'", + ).first + + return if prev_value.blank? + + regex = + / + (?:\A'unsafe-eval'\z)| + (?:\A'wasm-unsafe-eval'\z)| + (?:\A'sha(?:256|384|512)-[A-Za-z0-9+\/\-_]+={0,2}'\z) + /x + new_value = prev_value.split("|").select { |substring| substring.match?(regex) }.uniq.join("|") + + return if new_value == prev_value + + DB.exec(<<~SQL, new_value:) + UPDATE site_settings + SET value = :new_value + WHERE name = 'content_security_policy_script_src' + SQL + + DB.exec(<<~SQL, prev_value:, new_value:) + INSERT INTO user_histories (action, subject, previous_value, new_value, admin_only, updated_at, created_at, acting_user_id) + VALUES (3, 'content_security_policy_script_src', :prev_value, :new_value, true, NOW(), NOW(), -1) + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/spec/migrations/remove_invalid_csp_script_src_values_spec.rb b/spec/migrations/remove_invalid_csp_script_src_values_spec.rb new file mode 100644 index 00000000000..f800898a9cb --- /dev/null +++ b/spec/migrations/remove_invalid_csp_script_src_values_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require Rails.root.join( + "db/migrate/20240627125112_remove_invalid_csp_script_src_site_setting_values.rb", + ) + +RSpec.describe RemoveInvalidCspScriptSrcSiteSettingValues do + let(:migrate) { described_class.new.up } + + context "when content_security_policy_script_src site setting is present" do + context "when value is present" do + let(:hash_1) { "'sha256-QFlnYO2Ll+rgFRKkUmtyRublBc7KFNsbzF7BzoCqjgA='" } + let(:hash_2) { "'sha384-oqVuAfXRKap7fdgcCY5uykM6+R9GqQ8K/uxy9rx7HNQlGYl1kPzQho1wx4JwY8wC'" } + let(:prev_value) do + "'unsafe-inline'|'wasm-unsafe-eval'|example.com|'unsafe-eval'|'sha256-bad+encoded+str=!'|#{hash_1}|#{hash_2}" + end + let(:new_value) { "'wasm-unsafe-eval'|'unsafe-eval'|#{hash_1}|#{hash_2}" } + let!(:site_setting) do + SiteSetting.create!( + name: "content_security_policy_script_src", + data_type: SiteSettings::TypeSupervisor.types[:simple_list], + value: prev_value, + ) + end + + it "keeps only valid script src values" do + silence_stdout { migrate } + expect(site_setting.reload.value).to eq new_value + end + + it "creates a new user history tracking the change in values" do + expect { silence_stdout { migrate } }.to change(UserHistory, :count).by 1 + expect(UserHistory.last).to have_attributes( + subject: "content_security_policy_script_src", + admin_only: true, + action: UserHistory.actions[:change_site_setting], + previous_value: prev_value, + new_value:, + ) + end + end + + context "when value is default" do + let!(:site_setting) do + SiteSetting.create!( + name: "content_security_policy_script_src", + data_type: SiteSettings::TypeSupervisor.types[:simple_list], + value: "", + ) + end + + it "does not update rows with the default empty string value" do + expect(site_setting.reload.value).to eq "" + end + + it "does not create a new user history" do + expect { silence_stdout { migrate } }.not_to change(UserHistory, :count) + end + end + end + + context "when content_security_policy_script_src site setting is not present" do + it "does not update" do + silence_stdout { migrate } + expect(SiteSetting.exists?(name: "content_security_policy_script_src")).to eq false + end + + it "does not create a new user history" do + expect { silence_stdout { migrate } }.not_to change(UserHistory, :count) + end + end +end