security: input validation & injection fixes (XSS, constantize, open redirects, CSV, SQL)

app/controllers/api/v1/transactions_controller.rb

@@ -270,7 +270,7 @@ def apply_filters(query)
     end
 
     def apply_search(query)
-      search_term = "%#{params[:search]}%"
+      search_term = "%#{ActiveRecord::Base.sanitize_sql_like(params[:search])}%"
 
       query.joins(:entry)
            .left_joins(:merchant)

app/controllers/concerns/accountable_resource.rb

@@ -48,7 +48,7 @@ def create
       @account.lock_saved_attributes!
     end
 
-    redirect_to account_params[:return_to].presence || @account, notice: t("accounts.create.success", type: accountable_type.name.underscore.humanize)
+    redirect_to safe_return_to_path || @account, notice: t("accounts.create.success", type: accountable_type.name.underscore.humanize)
   end
 
   def update
@@ -98,6 +98,23 @@ def set_manageable_account
       require_account_permission!(@account)
     end
 
+    # Sanitize return_to parameter to prevent XSS/open-redirect attacks.
+    # Only allow internal relative paths (starting with "/"), and reject any scheme/host.
+    def safe_return_to_path
+      return nil if params[:return_to].blank?
+
+      return_to = params[:return_to].to_s
+
+      begin
+        uri = URI.parse(return_to)
+        return nil if uri.scheme.present?
+        return nil unless return_to.start_with?("/")
+        return_to
+      rescue URI::InvalidURIError
+        nil
+      end
+    end
+
     def account_params
       params.require(:account).permit(
         :name, :balance, :subtype, :currency, :accountable_type, :return_to,

app/controllers/concerns/store_location.rb

@@ -24,8 +24,12 @@ def handle_not_found
   end
 
   def store_return_to
-    if params[:return_to].present?
-      session[:return_to] = params[:return_to]
+    return if params[:return_to].blank?
+
+    path = params[:return_to].to_s
+    # Only allow relative paths to prevent open redirect attacks
+    if path.start_with?("/") && !path.start_with?("//")
+      session[:return_to] = path
     end
   end
 

app/controllers/import/mappings_controller.rb

@@ -33,11 +33,21 @@ def create_when_empty
       mapping_params[:mappable_id] == mapping_class::CREATE_NEW_KEY
     end
 
+    ALLOWED_MAPPABLE_TYPES = %w[Category Tag Account].freeze
+    ALLOWED_MAPPING_TYPES = %w[
+      Import::CategoryMapping Import::TagMapping
+      Import::AccountMapping Import::AccountTypeMapping
+    ].freeze
+
     def mappable_class
-      mapping_params[:mappable_type]&.constantize
+      type = mapping_params[:mappable_type]
+      return nil unless type.present? && ALLOWED_MAPPABLE_TYPES.include?(type)
+      type.constantize
     end
 
     def mapping_class
-      mapping_params[:type]&.constantize
+      type = mapping_params[:type]
+      return nil unless type.present? && ALLOWED_MAPPING_TYPES.include?(type)
+      type.constantize
     end
 end

app/controllers/reports_controller.rb

@@ -663,13 +663,14 @@ def build_transactions_breakdown_for_export
       # Whitelist sort_direction to prevent SQL injection
       sort_direction = %w[asc desc].include?(params[:sort_direction]&.downcase) ? params[:sort_direction].upcase : "DESC"
 
+      direction = sort_direction.downcase.to_sym
       case sort_by
       when "date"
-        transactions.order("entries.date #{sort_direction}")
+        transactions.order("entries.date" => direction)
       when "amount"
-        transactions.order("entries.amount #{sort_direction}")
+        transactions.order("entries.amount" => direction)
       else
-        transactions.order("entries.date DESC")
+        transactions.order("entries.date" => :desc)
       end
     end
 

app/controllers/settings/guides_controller.rb

@@ -6,7 +6,11 @@ def show
       [ "Home", root_path ],
       [ "Guides", nil ]
     ]
-    markdown = Redcarpet::Markdown.new(Redcarpet::Render::HTML,
+    renderer = Redcarpet::Render::HTML.new(
+      filter_html: true,
+      link_attributes: { target: "_blank", rel: "noopener noreferrer" }
+    )
+    markdown = Redcarpet::Markdown.new(renderer,
       autolink: true,
       tables: true,
       fenced_code_blocks: true,

app/helpers/application_helper.rb

@@ -136,6 +136,7 @@ def markdown(text)
 
     renderer = Redcarpet::Render::HTML.new(
       hard_wrap: true,
+      filter_html: true,
       link_attributes: { target: "_blank", rel: "noopener noreferrer" }
     )
 
@@ -152,7 +153,11 @@ def markdown(text)
       footnotes: true
     )
 
-    markdown.render(text).html_safe
+    sanitize(
+      markdown.render(text),
+      tags: %w[p br strong em a ul ol li h1 h2 h3 h4 h5 h6 pre code blockquote table thead tbody tr th td span div sup del mark ins hr dt dd dl],
+      attributes: %w[href target rel class id]
+    )
   end
 
   # Generate the callback URL for Enable Banking OAuth (used in views and controller).

app/models/account_import.rb

@@ -1,11 +1,21 @@
 class AccountImport < Import
   OpeningBalanceError = Class.new(StandardError)
 
+  ALLOWED_ACCOUNTABLE_TYPES = %w[
+    Depository Investment Crypto
+    Property Vehicle OtherAsset
+    CreditCard Loan OtherLiability
+  ].freeze
+
   def import!
     transaction do
       rows.each do |row|
         mapping = mappings.account_types.find_by(key: row.entity_type)
-        accountable_class = mapping.value.constantize
+        type = mapping&.value
+        unless type.present? && ALLOWED_ACCOUNTABLE_TYPES.include?(type)
+          raise ArgumentError, "Invalid accountable type: #{type.inspect}"
+        end
+        accountable_class = type.constantize
 
         account = family.accounts.build(
           name: row.name,

app/models/family/data_exporter.rb

@@ -49,7 +49,7 @@ def generate_accounts_csv
         @family.accounts.includes(:accountable).find_each do |account|
           csv << [
             account.id,
-            account.name,
+            sanitize_csv(account.name),
             account.accountable_type,
             account.subtype,
             account.balance.to_s,
@@ -72,12 +72,12 @@ def generate_transactions_csv
           .find_each do |transaction|
             csv << [
               transaction.entry.date.iso8601,
-              transaction.entry.account.name,
+              sanitize_csv(transaction.entry.account.name),
               transaction.entry.amount.to_s,
-              transaction.entry.name,
-              transaction.category&.name,
-              transaction.tags.pluck(:name).join(","),
-              transaction.entry.notes,
+              sanitize_csv(transaction.entry.name),
+              sanitize_csv(transaction.category&.name),
+              transaction.tags.pluck(:name).map { |t| sanitize_csv(t) }.join(","),
+              sanitize_csv(transaction.entry.notes),
               transaction.entry.currency
             ]
           end
@@ -94,7 +94,7 @@ def generate_trades_csv
           .find_each do |trade|
             csv << [
               trade.entry.date.iso8601,
-              trade.entry.account.name,
+              sanitize_csv(trade.entry.account.name),
               trade.security.ticker,
               trade.qty.to_s,
               trade.price.to_s,
@@ -112,9 +112,9 @@ def generate_categories_csv
         # Only export categories belonging to this family
         @family.categories.includes(:parent).find_each do |category|
           csv << [
-            category.name,
+            sanitize_csv(category.name),
             category.color,
-            category.parent&.name,
+            sanitize_csv(category.parent&.name),
             category.lucide_icon
           ]
         end
@@ -128,7 +128,7 @@ def generate_rules_csv
         # Only export rules belonging to this family
         @family.rules.includes(conditions: :sub_conditions, actions: []).find_each do |rule|
           csv << [
-            rule.name,
+            sanitize_csv(rule.name),
             rule.resource_type,
             rule.active,
             rule.effective_date&.iso8601,
@@ -189,8 +189,8 @@ def generate_ndjson
             date: transaction.entry.date,
             amount: transaction.entry.amount,
             currency: transaction.entry.currency,
-            name: transaction.entry.name,
-            notes: transaction.entry.notes,
+            name: sanitize_csv(transaction.entry.name),
+            notes: sanitize_csv(transaction.entry.notes),
             excluded: transaction.entry.excluded,
             category_id: transaction.category_id,
             merchant_id: transaction.merchant_id,
@@ -234,7 +234,7 @@ def generate_ndjson
             date: entry.date,
             amount: entry.amount,
             currency: entry.currency,
-            name: entry.name,
+            name: sanitize_csv(entry.name),
             created_at: entry.created_at,
             updated_at: entry.updated_at
           }
@@ -271,7 +271,7 @@ def generate_ndjson
 
     def serialize_rule_for_export(rule)
       {
-        name: rule.name,
+        name: sanitize_csv(rule.name),
         resource_type: rule.resource_type,
         active: rule.active,
         effective_date: rule.effective_date&.iso8601,
@@ -350,4 +350,11 @@ def serialize_conditions_for_csv(conditions)
     def serialize_actions_for_csv(actions)
       actions.map { |a| serialize_action(a) }.to_json
     end
+
+    # Prevent CSV formula injection (CWE-1236)
+    # Values starting with =, +, -, @ can execute as formulas in Excel/Sheets
+    def sanitize_csv(value)
+      return value unless value.is_a?(String)
+      value.match?(/\A[=+\-@\t\r\n]/) ? "'" + value : value
+    end
 end

app/views/pages/changelog.html.erb

@@ -21,7 +21,9 @@
     </div>
     <div class="w-full md:w-2/3 text-secondary text-sm prose prose--github-release-notes">
       <h2 class="mb-5 text-xl text-primary"><%= @release_notes[:name] %></h2>
-      <%= (@release_notes[:body] || "No release notes available").html_safe %>
+      <%= sanitize(@release_notes[:body] || "No release notes available",
+                   tags: %w[h1 h2 h3 h4 h5 h6 p a ul ol li code pre blockquote strong em br img hr div span table thead tbody tr th td dl dt dd sup sub],
+                   attributes: %w[href src alt class id title]) %>
     </div>
   </div>
 </div>

test/models/account_import_test_security.rb

@@ -0,0 +1,18 @@
+require "test_helper"
+
+class AccountImportSecurityTest < ActiveSupport::TestCase
+  test "ALLOWED_ACCOUNTABLE_TYPES covers all expected types" do
+    expected = %w[
+      Depository Investment Crypto
+      Property Vehicle OtherAsset
+      CreditCard Loan OtherLiability
+    ]
+    assert_equal expected.sort, AccountImport::ALLOWED_ACCOUNTABLE_TYPES.sort
+  end
+
+  test "all ALLOWED_ACCOUNTABLE_TYPES are real constants" do
+    AccountImport::ALLOWED_ACCOUNTABLE_TYPES.each do |type|
+      assert_nothing_raised { type.constantize }
+    end
+  end
+end

test/models/family/data_exporter_test.rb

@@ -341,4 +341,24 @@ class Family::DataExporterTest < ActiveSupport::TestCase
       refute ndjson_content.include?(other_rule.name)
     end
   end
+
+  # CSV injection prevention (CWE-1236) ----------------------------------------
+
+  test "sanitize_csv prefixes formula-starting values with single quote" do
+    dangerous = { "=SUM(A1)" => "'=SUM(A1)", "+cmd" => "'+cmd", "-1+1" => "'-1+1", "@user" => "'@user" }
+    dangerous.each do |input, expected|
+      assert_equal expected, @exporter.send(:sanitize_csv, input), "Failed for: #{input}"
+    end
+  end
+
+  test "sanitize_csv leaves safe strings unchanged" do
+    %w[hello Normal 123 category].each do |safe|
+      assert_equal safe, @exporter.send(:sanitize_csv, safe)
+    end
+  end
+
+  test "sanitize_csv passes through non-string values unchanged" do
+    assert_nil @exporter.send(:sanitize_csv, nil)
+    assert_equal 42, @exporter.send(:sanitize_csv, 42)
+  end
 end