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,31 @@ def set_manageable_account
       require_account_permission!(@account)
     end
 
+    # Sanitize return_to parameter to prevent XSS/open-redirect attacks.
+    # Only allow internal relative paths (single leading "/"), and reject any scheme/host.
+    # Accepts return_to from either the top-level params or nested account_params.
+    def safe_return_to_path
+      raw = params[:return_to].presence || params.dig(:account, :return_to).presence
+      return nil if raw.blank?
+
+      return_to = raw.to_s
+
+      # Reject protocol-relative URLs like "//evil.example.com/path" that browsers
+      # treat as cross-origin even though they pass a naive start_with?("/") check.
+      return nil if return_to.start_with?("//")
+      return nil unless return_to.start_with?("/")
+
+      begin
+        uri = URI.parse(return_to)
+      rescue URI::InvalidURIError
+        return nil
+      end
+
+      return nil if uri.scheme.present? || uri.host.present?
+
+      return_to
+    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

@@ -660,16 +660,15 @@ def build_transactions_breakdown_for_export
       transactions = apply_transaction_filters(transactions)
 
       sort_by = params[:sort_by] || "date"
-      # Whitelist sort_direction to prevent SQL injection
-      sort_direction = %w[asc desc].include?(params[:sort_direction]&.downcase) ? params[:sort_direction].upcase : "DESC"
-
+      # Whitelist sort_direction (hash-based order() below also guards against SQL injection)
+      direction = %w[asc desc].include?(params[:sort_direction]&.downcase) ? params[:sort_direction].downcase.to_sym : :desc
       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 img 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 src alt title]
+    )
   end
 
   # Generate the callback URL for Enable Banking OAuth (used in views and controller).

app/models/account_import.rb

@@ -1,11 +1,17 @@
 class AccountImport < Import
   OpeningBalanceError = Class.new(StandardError)
 
+  # Delegate the allow-list to `Accountable::TYPES` so AccountImport and the
+  # Accountable concern cannot drift. Kept as a public constant because tests
+  # and other code may reference it.
+  ALLOWED_ACCOUNTABLE_TYPES = Accountable::TYPES
+
   def import!
     transaction do
       rows.each do |row|
         mapping = mappings.account_types.find_by(key: row.entity_type)
-        accountable_class = mapping.value.constantize
+        accountable_class = Accountable.from_type(mapping&.value)
+        raise ArgumentError, "Invalid accountable type: #{mapping&.value.inspect}" unless accountable_class
 
         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,
@@ -350,4 +350,17 @@ 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.
+    # \t, \r, \n are included because some spreadsheet parsers trim leading
+    # whitespace-like characters before evaluating the cell, so "=1+1" prefixed
+    # with a tab/newline would still trigger a formula. Leading literal spaces
+    # are NOT treated as bypasses by mainstream parsers today; if that changes,
+    # extend the character class to cover them.
+    # CSV-only — do not apply to JSON/NDJSON output (it would mutate user data).
+    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 dir]) %>
     </div>
   </div>
 </div>

test/models/account_import_test_security.rb

@@ -0,0 +1,20 @@
+require "test_helper"
+
+class AccountImportSecurityTest < ActiveSupport::TestCase
+  test "ALLOWED_ACCOUNTABLE_TYPES mirrors Accountable::TYPES (guards against drift)" do
+    assert_equal Accountable::TYPES.sort, AccountImport::ALLOWED_ACCOUNTABLE_TYPES.sort
+  end
+
+  test "all ALLOWED_ACCOUNTABLE_TYPES resolve via Accountable.from_type" do
+    AccountImport::ALLOWED_ACCOUNTABLE_TYPES.each do |type|
+      assert_not_nil Accountable.from_type(type), "#{type} should resolve to a class"
+    end
+  end
+
+  test "Accountable.from_type rejects unknown input" do
+    assert_nil Accountable.from_type("ActiveRecord::Base")
+    assert_nil Accountable.from_type("NotAClass")
+    assert_nil Accountable.from_type(nil)
+    assert_nil Accountable.from_type("")
+  end
+end

test/models/family/data_exporter_test.rb

@@ -341,4 +341,55 @@ 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
+
+  test "NDJSON preserves formula-prefixed names/notes verbatim (no sanitize_csv leak)" do
+    # Transaction with name/notes starting with CSV formula chars — NDJSON must
+    # round-trip them unchanged, otherwise export→import silently mutates data.
+    dangerous_name  = "=SUM(A1)"
+    dangerous_notes = "-1.5x leverage"
+    entry = @account.entries.create!(
+      date: Date.current,
+      name: dangerous_name,
+      amount: 10,
+      currency: "USD",
+      notes: dangerous_notes,
+      entryable: Transaction.new
+    )
+
+    zip_data = @exporter.generate_export
+
+    Zip::File.open_buffer(zip_data) do |zip|
+      ndjson = zip.read("all.ndjson")
+
+      transaction_line = ndjson.each_line.find do |line|
+        parsed = JSON.parse(line)
+        parsed["type"] == "Transaction" && parsed.dig("data", "entry_id") == entry.id
+      end
+
+      assert transaction_line, "Should find the exported transaction in all.ndjson"
+      data = JSON.parse(transaction_line)["data"]
+      assert_equal dangerous_name,  data["name"]
+      assert_equal dangerous_notes, data["notes"]
+    end
+  end
 end