Fix potential sql injection

app/helpers/stats_helper.rb

@@ -55,13 +55,7 @@ module StatsHelper
 
   def distance_traveled(user, stat)
     distance_unit = user.safe_settings.distance_unit
-
-    value =
-      if distance_unit == 'mi'
-        (stat.distance / 1609.34).round(2)
-      else
-        (stat.distance / 1000).round(2)
-      end
+    value = Stat.convert_distance(stat.distance, distance_unit).round
 
     "#{number_with_delimiter(value)} #{distance_unit}"
   end
@@ -103,7 +97,7 @@ module StatsHelper
     stat.toponyms.count { _1['country'] }
   end
 
-  def x_than_prevopis_countries_visited(stat, previous_stat)
+  def x_than_previous_countries_visited(stat, previous_stat)
     return '' unless previous_stat
 
     previous_countries = previous_stat.toponyms.count { _1['country'] }
@@ -123,16 +117,9 @@ module StatsHelper
     return 'N/A' unless peak && peak[1].positive?
 
     date = Date.new(stat.year, stat.month, peak[0])
-    distance_km = (peak[1] / 1000).round(2)
     distance_unit = stat.user.safe_settings.distance_unit
 
-    distance_value =
-      if distance_unit == 'mi'
-        (peak[1] / 1609.34).round(2)
-      else
-        distance_km
-      end
-
+    distance_value = Stat.convert_distance(peak[1], distance_unit).round
     text = "#{date.strftime('%B %d')} (#{distance_value} #{distance_unit})"
 
     link_to text, map_url(start_at: date.beginning_of_day, end_at: date.end_of_day), class: 'underline'

app/jobs/cache/preheating_job.rb

@@ -13,19 +13,19 @@ class Cache::PreheatingJob < ApplicationJob
 
       Rails.cache.write(
         "dawarich/user_#{user.id}_points_geocoded_stats",
-        StatsQuery.new(user).send(:cached_points_geocoded_stats),
+        StatsQuery.new(user).cached_points_geocoded_stats,
         expires_in: 1.day
       )
 
       Rails.cache.write(
         "dawarich/user_#{user.id}_countries_visited",
-        user.send(:countries_visited_uncached),
+        user.countries_visited_uncached,
         expires_in: 1.day
       )
 
       Rails.cache.write(
         "dawarich/user_#{user.id}_cities_visited",
-        user.send(:cities_visited_uncached),
+        user.cities_visited_uncached,
         expires_in: 1.day
       )
     end

app/models/user.rb

@@ -131,6 +131,19 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength
     Time.zone.name
   end
 
+  def countries_visited_uncached
+    points
+      .without_raw_data
+      .where.not(country_name: [nil, ''])
+      .distinct
+      .pluck(:country_name)
+      .compact
+  end
+
+  def cities_visited_uncached
+    points.where.not(city: [nil, '']).distinct.pluck(:city).compact
+  end
+
   private
 
   def create_api_key
@@ -168,17 +181,4 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength
     Users::MailerSendingJob.set(wait: 9.days).perform_later(id, 'post_trial_reminder_early')
     Users::MailerSendingJob.set(wait: 14.days).perform_later(id, 'post_trial_reminder_late')
   end
-
-  def countries_visited_uncached
-    points
-      .without_raw_data
-      .where.not(country_name: [nil, ''])
-      .distinct
-      .pluck(:country_name)
-      .compact
-  end
-
-  def cities_visited_uncached
-    points.where.not(city: [nil, '']).distinct.pluck(:city).compact
-  end
 end

app/queries/hexagon_query.rb

@@ -18,18 +18,21 @@ class HexagonQuery
   end
 
   def call
-    ActiveRecord::Base.connection.execute(build_hexagon_sql)
+    binds = []
+    user_sql = build_user_filter(binds)
+    date_filter = build_date_filter(binds)
+
+    sql = build_hexagon_sql(user_sql, date_filter)
+
+    ActiveRecord::Base.connection.exec_query(sql, 'hexagon_sql', binds)
   end
 
   private
 
-  def build_hexagon_sql
-    user_filter = user_id ? "user_id = #{user_id}" : '1=1'
-    date_filter = build_date_filter
-
+  def build_hexagon_sql(user_sql, date_filter)
     <<~SQL
       WITH bbox_geom AS (
-        SELECT ST_MakeEnvelope(#{min_lon}, #{min_lat}, #{max_lon}, #{max_lat}, 4326) as geom
+        SELECT ST_MakeEnvelope($1, $2, $3, $4, 4326) as geom
       ),
       bbox_utm AS (
         SELECT
@@ -44,7 +47,7 @@ class HexagonQuery
           id,
           timestamp
         FROM points
-        WHERE #{user_filter}
+        WHERE #{user_sql}
           #{date_filter}
           AND ST_Intersects(
             lonlat,
@@ -53,9 +56,9 @@ class HexagonQuery
       ),
       hex_grid AS (
         SELECT
-          (ST_HexagonGrid(#{hex_size}, bbox_utm.geom_utm)).geom as hex_geom_utm,
-          (ST_HexagonGrid(#{hex_size}, bbox_utm.geom_utm)).i as hex_i,
-          (ST_HexagonGrid(#{hex_size}, bbox_utm.geom_utm)).j as hex_j
+          (ST_HexagonGrid($5, bbox_utm.geom_utm)).geom as hex_geom_utm,
+          (ST_HexagonGrid($5, bbox_utm.geom_utm)).i as hex_i,
+          (ST_HexagonGrid($5, bbox_utm.geom_utm)).j as hex_j
         FROM bbox_utm
       ),
       hexagons_with_points AS (
@@ -88,17 +91,58 @@ class HexagonQuery
         row_number() OVER (ORDER BY point_count DESC) as id
       FROM hexagon_stats
       ORDER BY point_count DESC
-      LIMIT #{MAX_HEXAGONS_PER_REQUEST};
+      LIMIT $6;
     SQL
   end
 
-  def build_date_filter
+  def build_user_filter(binds)
+    # Add bbox coordinates: min_lon, min_lat, max_lon, max_lat
+    binds << min_lon
+    binds << min_lat
+    binds << max_lon
+    binds << max_lat
+
+    # Add hex_size
+    binds << hex_size
+
+    # Add limit
+    binds << MAX_HEXAGONS_PER_REQUEST
+
+    if user_id
+      binds << user_id
+      'user_id = $7'
+    else
+      '1=1'
+    end
+  end
+
+  def build_date_filter(binds)
     return '' unless start_date || end_date
 
     conditions = []
-    conditions << "timestamp >= EXTRACT(EPOCH FROM '#{start_date}'::timestamp)" if start_date
-    conditions << "timestamp <= EXTRACT(EPOCH FROM '#{end_date}'::timestamp)" if end_date
+    current_param_index = user_id ? 8 : 7 # Account for bbox, hex_size, limit, and potential user_id
+
+    if start_date
+      start_timestamp = parse_date_to_timestamp(start_date)
+      binds << start_timestamp
+      conditions << "timestamp >= $#{current_param_index}"
+      current_param_index += 1
+    end
+
+    if end_date
+      end_timestamp = parse_date_to_timestamp(end_date)
+      binds << end_timestamp
+      conditions << "timestamp <= $#{current_param_index}"
+    end
 
     conditions.any? ? "AND #{conditions.join(' AND ')}" : ''
   end
+
+  def parse_date_to_timestamp(date_string)
+    # Convert ISO date string to timestamp integer
+    Time.parse(date_string).to_i
+  rescue ArgumentError => e
+    ExceptionReporter.call(e, "Invalid date format: #{date_string}")
+    raise ArgumentError, "Invalid date format: #{date_string}"
+  end
 end

app/queries/stats_query.rb

@@ -17,21 +17,19 @@ class StatsQuery
     }
   end
 
-  private
-
-  attr_reader :user
-
   def cached_points_geocoded_stats
-    sql = ActiveRecord::Base.sanitize_sql_array([
-      <<~SQL.squish,
-        SELECT
-          COUNT(reverse_geocoded_at) as geocoded,
-          COUNT(CASE WHEN geodata = '{}'::jsonb THEN 1 END) as without_data
-        FROM points
-        WHERE user_id = ?
-      SQL
-      user.id
-    ])
+    sql = ActiveRecord::Base.sanitize_sql_array(
+      [
+        <<~SQL.squish,
+          SELECT
+            COUNT(reverse_geocoded_at) as geocoded,
+            COUNT(CASE WHEN geodata = '{}'::jsonb THEN 1 END) as without_data
+          FROM points
+          WHERE user_id = ?
+        SQL
+        user.id
+      ]
+    )
 
     result = Point.connection.select_one(sql)
 
@@ -40,4 +38,8 @@ class StatsQuery
       without_data: result['without_data'].to_i
     }
   end
+
+  private
+
+  attr_reader :user
 end

app/views/stats/_month.html.erb

@@ -45,7 +45,7 @@
       <%= countries_visited(stat) %>
     </div>
     <div class="stat-desc">
-      <%= x_than_prevopis_countries_visited(stat, previous_stat) %>
+      <%= x_than_previous_countries_visited(stat, previous_stat) %>
     </div>
   </div>
 </div>

config/routes.rb

@@ -70,7 +70,7 @@ Rails.application.routes.draw do
     end
   end
   get 'stats/:year', to: 'stats#show', constraints: { year: /\d{4}/ }
-  get 'stats/:year/:month', to: 'stats#month', constraints: { year: /\d{4}/, month: /\d{1,2}/ }
+  get 'stats/:year/:month', to: 'stats#month', constraints: { year: /\d{4}/, month: /(0?[1-9]|1[0-2])/ }
   put 'stats/:year/:month/update',
       to: 'stats#update',
       as: :update_year_month_stats,

spec/jobs/users/mailer_sending_job_spec.rb

@@ -108,7 +108,7 @@ RSpec.describe Users::MailerSendingJob, type: :job do
     end
 
     context 'when user is deleted' do
-      it 'raises ActiveRecord::RecordNotFound' do
+      it 'does not raise an error' do
         user.destroy
 
         expect do

spec/services/cache/clean_spec.rb

@@ -59,6 +59,25 @@ RSpec.describe Cache::Clean do
       expect(Rails.cache.exist?(user_2_points_geocoded_stats_key)).to be false
     end
 
+    it 'deletes countries and cities cache for all users' do
+      Rails.cache.write(user_1_countries_key, %w[USA Canada])
+      Rails.cache.write(user_2_countries_key, %w[France Germany])
+      Rails.cache.write(user_1_cities_key, ['New York', 'Toronto'])
+      Rails.cache.write(user_2_cities_key, %w[Paris Berlin])
+
+      expect(Rails.cache.exist?(user_1_countries_key)).to be true
+      expect(Rails.cache.exist?(user_2_countries_key)).to be true
+      expect(Rails.cache.exist?(user_1_cities_key)).to be true
+      expect(Rails.cache.exist?(user_2_cities_key)).to be true
+
+      described_class.call
+
+      expect(Rails.cache.exist?(user_1_countries_key)).to be false
+      expect(Rails.cache.exist?(user_2_countries_key)).to be false
+      expect(Rails.cache.exist?(user_1_cities_key)).to be false
+      expect(Rails.cache.exist?(user_2_cities_key)).to be false
+    end
+
     it 'logs cache cleaning process' do
       expect(Rails.logger).to receive(:info).with('Cleaning cache...')
       expect(Rails.logger).to receive(:info).with('Cache cleaned')