From 288b9fbbb276973d084e06f93e8e19e18f42b7ae Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Mon, 23 Mar 2026 16:59:00 -0500 Subject: [PATCH] fix: address security vulnerabilities and concurrency issues - Fix SQL injection in partition_manager, db_optimizer, and release.ex - Fix XSS vulnerabilities with proper HTML escaping in LiveViews - Add proper error handling for email delivery functions - Fix race conditions with advisory locks and atomic operations - Replace unsupervised spawn/Task.start with supervised alternatives - Convert ETS operations to GenServer serialization for thread safety - Change ETS tables from :public to :protected access - Add client limits to prevent unbounded memory growth - Add PubSub cleanup in GenServer terminate callbacks - Fix device upsert to use atomic Repo.insert_all --- lib/aprsme/application.ex | 19 +- lib/aprsme/cache.ex | 175 ++++++++++++------ lib/aprsme/cluster/connection_manager.ex | 6 + lib/aprsme/cluster/leader_election.ex | 25 ++- lib/aprsme/cluster/packet_distributor.ex | 21 ++- lib/aprsme/db_optimizer.ex | 14 +- lib/aprsme/device_identification.ex | 24 ++- lib/aprsme/packets/prepared_queries.ex | 1 + lib/aprsme/partition_manager.ex | 39 +++- lib/aprsme/regex_cache.ex | 21 ++- lib/aprsme/release.ex | 8 +- lib/aprsme/signal_handler.ex | 4 +- lib/aprsme/spatial_pubsub.ex | 40 ++-- lib/aprsme/streaming_packets_pubsub.ex | 7 - lib/aprsme_web/components/core_components.ex | 2 + lib/aprsme_web/live/info_live/show.ex | 46 +---- lib/aprsme_web/live/map_live/data_builder.ex | 4 +- lib/aprsme_web/live/status_live/index.ex | 4 +- .../user_confirmation_instructions_live.ex | 36 ++-- .../live/user_forgot_password_live.ex | 36 ++-- lib/aprsme_web/live/user_registration_live.ex | 17 +- lib/aprsme_web/live/user_settings_live.ex | 18 +- 22 files changed, 359 insertions(+), 208 deletions(-) diff --git a/lib/aprsme/application.ex b/lib/aprsme/application.ex index 633b7d3..085b6ba 100644 --- a/lib/aprsme/application.ex +++ b/lib/aprsme/application.ex @@ -27,6 +27,8 @@ defmodule Aprsme.Application do Aprsme.CircuitBreaker, # Start regex cache for performance Aprsme.RegexCache, + # Start cache manager + Aprsme.Cache, # Start device cache manager Aprsme.DeviceCache, # Start broadcast task supervisor for async operations @@ -88,7 +90,9 @@ defmodule Aprsme.Application do env = Application.get_env(:aprsme, :env) if env != :test do - Task.start(fn -> Aprsme.DeviceIdentification.maybe_refresh_devices() end) + Aprsme.BroadcastTaskSupervisor.async_execute(fn -> + Aprsme.DeviceIdentification.maybe_refresh_devices() + end) end {:ok, sup} @@ -138,10 +142,6 @@ defmodule Aprsme.Application do cluster_children = if topologies == [] do [] - # libcluster supervisor - # Dynamic supervisor for processes managed by cluster leader - # Leader election process - # Connection manager that starts/stops APRS-IS based on leadership else [ {Cluster.Supervisor, [topologies, [name: Aprsme.ClusterSupervisor]]}, @@ -149,6 +149,7 @@ defmodule Aprsme.Application do Aprsme.Cluster.LeaderElection, Aprsme.Cluster.ConnectionManager, Aprsme.Cluster.PacketReceiver, + Aprsme.Cluster.PacketDistributor, Aprsme.ConnectionMonitor ] end @@ -189,10 +190,10 @@ defmodule Aprsme.Application do Logger.info("Starting ETS-based caching and rate limiting") # Create ETS tables for caching - :ets.new(:query_cache, [:set, :public, :named_table, read_concurrency: true]) - :ets.new(:device_cache, [:set, :public, :named_table, read_concurrency: true]) - :ets.new(:symbol_cache, [:set, :public, :named_table, read_concurrency: true]) - :ets.new(:aprsme, [:set, :public, :named_table, read_concurrency: true]) + :ets.new(:query_cache, [:set, :protected, :named_table, read_concurrency: true]) + :ets.new(:device_cache, [:set, :protected, :named_table, read_concurrency: true]) + :ets.new(:symbol_cache, [:set, :protected, :named_table, read_concurrency: true]) + :ets.new(:aprsme, [:set, :protected, :named_table, read_concurrency: true]) :ets.insert(:aprsme, {:message_number, 0}) [ diff --git a/lib/aprsme/cache.ex b/lib/aprsme/cache.ex index da00d41..5489d18 100644 --- a/lib/aprsme/cache.ex +++ b/lib/aprsme/cache.ex @@ -8,31 +8,22 @@ defmodule Aprsme.Cache do Expired entries are lazily evicted on read. """ + use GenServer + + def start_link(opts) do + GenServer.start_link(__MODULE__, opts, name: __MODULE__) + end + + @impl true + def init(_opts) do + {:ok, %{}} + end + @doc """ Get a value from cache. Returns `{:ok, nil}` for expired entries. """ def get(cache_name, key) do - case :ets.lookup(cache_name, key) do - [{^key, value, :infinity}] -> - {:ok, value} - - [{^key, value, expires_at}] -> - if System.monotonic_time(:millisecond) < expires_at do - {:ok, value} - else - :ets.delete(cache_name, key) - {:ok, nil} - end - - # Support legacy {key, value} tuples during transition - [{^key, value}] -> - {:ok, value} - - [] -> - {:ok, nil} - end - rescue - ArgumentError -> {:error, :no_cache} + GenServer.call(__MODULE__, {:get, cache_name, key}) end @doc """ @@ -42,47 +33,28 @@ defmodule Aprsme.Cache do * `:ttl` - Time to live in milliseconds. Use `Cache.to_timeout/1` for convenience. """ def put(cache_name, key, value, opts \\ []) do - expires_at = - case Keyword.get(opts, :ttl) do - nil -> :infinity - ttl when is_integer(ttl) and ttl > 0 -> System.monotonic_time(:millisecond) + ttl - _ -> :infinity - end - - :ets.insert(cache_name, {key, value, expires_at}) - {:ok, true} - rescue - ArgumentError -> {:error, :no_cache} + GenServer.call(__MODULE__, {:put, cache_name, key, value, opts}) end @doc """ Delete a key from cache """ def del(cache_name, key) do - :ets.delete(cache_name, key) - {:ok, true} - rescue - ArgumentError -> {:error, :no_cache} + GenServer.call(__MODULE__, {:del, cache_name, key}) end @doc """ Clear all keys from cache """ def clear(cache_name) do - :ets.delete_all_objects(cache_name) - {:ok, true} - rescue - ArgumentError -> {:error, :no_cache} + GenServer.call(__MODULE__, {:clear, cache_name}) end @doc """ Get cache statistics (simplified for ETS) """ def stats(cache_name) do - info = :ets.info(cache_name) - {:ok, %{size: Keyword.get(info, :size, 0)}} - rescue - ArgumentError -> {:error, :no_cache} + GenServer.call(__MODULE__, {:stats, cache_name}) end @doc """ @@ -100,14 +72,7 @@ defmodule Aprsme.Cache do Get TTL for a key. Returns remaining milliseconds or nil if no TTL. """ def ttl(cache_name, key) do - case :ets.lookup(cache_name, key) do - [{^key, _value, :infinity}] -> {:ok, nil} - [{^key, _value, expires_at}] -> {:ok, max(0, expires_at - System.monotonic_time(:millisecond))} - [{^key, _value}] -> {:ok, nil} - [] -> {:ok, nil} - end - rescue - ArgumentError -> {:ok, nil} + GenServer.call(__MODULE__, {:ttl, cache_name, key}) end @doc """ @@ -127,4 +92,110 @@ defmodule Aprsme.Cache do {:milliseconds, n}, acc -> acc + n end) end + + @impl true + def handle_call({:get, cache_name, key}, _from, state) do + result = + try do + case :ets.lookup(cache_name, key) do + [{^key, value, :infinity}] -> + {:ok, value} + + [{^key, value, expires_at}] -> + if System.monotonic_time(:millisecond) < expires_at do + {:ok, value} + else + :ets.delete(cache_name, key) + {:ok, nil} + end + + [{^key, value}] -> + {:ok, value} + + [] -> + {:ok, nil} + end + rescue + ArgumentError -> + {:error, :no_cache} + end + + {:reply, result, state} + end + + @impl true + def handle_call({:put, cache_name, key, value, opts}, _from, state) do + result = + try do + expires_at = + case Keyword.get(opts, :ttl) do + nil -> :infinity + ttl when is_integer(ttl) and ttl > 0 -> System.monotonic_time(:millisecond) + ttl + _ -> :infinity + end + + :ets.insert(cache_name, {key, value, expires_at}) + {:ok, true} + rescue + ArgumentError -> {:error, :no_cache} + end + + {:reply, result, state} + end + + @impl true + def handle_call({:del, cache_name, key}, _from, state) do + result = + try do + :ets.delete(cache_name, key) + {:ok, true} + rescue + ArgumentError -> {:error, :no_cache} + end + + {:reply, result, state} + end + + @impl true + def handle_call({:clear, cache_name}, _from, state) do + result = + try do + :ets.delete_all_objects(cache_name) + {:ok, true} + rescue + ArgumentError -> {:error, :no_cache} + end + + {:reply, result, state} + end + + @impl true + def handle_call({:stats, cache_name}, _from, state) do + result = + try do + info = :ets.info(cache_name) + {:ok, %{size: Keyword.get(info, :size, 0)}} + rescue + ArgumentError -> {:error, :no_cache} + end + + {:reply, result, state} + end + + @impl true + def handle_call({:ttl, cache_name, key}, _from, state) do + result = + try do + case :ets.lookup(cache_name, key) do + [{^key, _value, :infinity}] -> {:ok, nil} + [{^key, _value, expires_at}] -> {:ok, max(0, expires_at - System.monotonic_time(:millisecond))} + [{^key, _value}] -> {:ok, nil} + [] -> {:ok, nil} + end + rescue + ArgumentError -> {:ok, nil} + end + + {:reply, result, state} + end end diff --git a/lib/aprsme/cluster/connection_manager.ex b/lib/aprsme/cluster/connection_manager.ex index ba1b568..12771d5 100644 --- a/lib/aprsme/cluster/connection_manager.ex +++ b/lib/aprsme/cluster/connection_manager.ex @@ -56,6 +56,12 @@ defmodule Aprsme.Cluster.ConnectionManager do end end + @impl true + def terminate(_reason, _state) do + Phoenix.PubSub.unsubscribe(Aprsme.PubSub, "cluster:leadership") + :ok + end + defp leader_check do LeaderElection.leader?() catch diff --git a/lib/aprsme/cluster/leader_election.ex b/lib/aprsme/cluster/leader_election.ex index 0331787..4cbab09 100644 --- a/lib/aprsme/cluster/leader_election.ex +++ b/lib/aprsme/cluster/leader_election.ex @@ -123,12 +123,8 @@ defmodule Aprsme.Cluster.LeaderElection do @impl true def handle_info(:attempt_election, state) do - # First, try to clean up any stale registrations cleanup_stale_registrations() - # Check if we already hold the registration — re-registering the same - # name returns :no even for the same PID, which would incorrectly clear - # our leadership state. case :global.whereis_name(@election_key) do pid when pid == self() -> if !state.is_leader do @@ -140,7 +136,7 @@ defmodule Aprsme.Cluster.LeaderElection do {:noreply, %{state | is_leader: true, leader_node: node()}} _ -> - attempt_registration(state) + attempt_registration_atomic(state) end end @@ -205,8 +201,23 @@ defmodule Aprsme.Cluster.LeaderElection do defp verify_leadership(state), do: state - defp attempt_registration(state) do - case :global.register_name(@election_key, self(), &resolve_conflict/3) do + defp attempt_registration_atomic(state) do + result = + :global.trans({@election_key, node()}, fn -> + case :global.whereis_name(@election_key) do + :undefined -> + :global.register_name(@election_key, self(), &resolve_conflict/3) + + pid -> + if pid == self() do + :yes + else + :no + end + end + end) + + case result do :yes -> Logger.info("Elected as APRS-IS connection leader on node #{node()}") :persistent_term.put({__MODULE__, :is_leader}, true) diff --git a/lib/aprsme/cluster/packet_distributor.ex b/lib/aprsme/cluster/packet_distributor.ex index fff4ffe..82850fd 100644 --- a/lib/aprsme/cluster/packet_distributor.ex +++ b/lib/aprsme/cluster/packet_distributor.ex @@ -4,16 +4,20 @@ defmodule Aprsme.Cluster.PacketDistributor do This ensures all nodes can serve real-time updates via LiveView while only the leader maintains the APRS-IS connection. """ + use GenServer + alias Aprsme.Cluster.LeaderElection @pubsub_topic "cluster:packets" + def start_link(opts) do + GenServer.start_link(__MODULE__, opts, name: __MODULE__) + end + def distribute_packet(packet) do - # Only distribute if clustering is enabled and we're the leader cluster_enabled = Application.get_env(:aprsme, :cluster_enabled, false) if cluster_enabled and LeaderElection.leader_cached?() do - # Broadcast to all nodes including self Phoenix.PubSub.broadcast( Aprsme.PubSub, @pubsub_topic, @@ -27,10 +31,21 @@ defmodule Aprsme.Cluster.PacketDistributor do end def handle_distributed_packet({:distributed_packet, packet}) do - # Broadcast to local LiveView clients via both PubSub systems Aprsme.StreamingPacketsPubSub.broadcast_packet(packet) Aprsme.SpatialPubSub.broadcast_packet(packet) :ok end + + @impl true + def init(_opts) do + Phoenix.PubSub.subscribe(Aprsme.PubSub, @pubsub_topic) + {:ok, %{}} + end + + @impl true + def terminate(_reason, _state) do + Phoenix.PubSub.unsubscribe(Aprsme.PubSub, @pubsub_topic) + :ok + end end diff --git a/lib/aprsme/db_optimizer.ex b/lib/aprsme/db_optimizer.ex index ce50978..f79d99d 100644 --- a/lib/aprsme/db_optimizer.ex +++ b/lib/aprsme/db_optimizer.ex @@ -76,7 +76,8 @@ defmodule Aprsme.DbOptimizer do """ def analyze_table(table_name) do validate_identifier!(table_name) - SQL.query!(Repo, "ANALYZE #{table_name}", []) + quoted_name = quote_identifier(table_name) + SQL.query!(Repo, "ANALYZE #{quoted_name}", []) :ok rescue error -> @@ -90,15 +91,15 @@ defmodule Aprsme.DbOptimizer do """ def vacuum_table(table_name, opts \\ []) do validate_identifier!(table_name) + quoted_name = quote_identifier(table_name) full = Keyword.get(opts, :full, false) analyze = Keyword.get(opts, :analyze, true) vacuum_type = if full, do: "VACUUM FULL", else: "VACUUM" analyze_clause = if analyze, do: " ANALYZE", else: "" - query = "#{vacuum_type}#{analyze_clause} #{table_name}" + query = "#{vacuum_type}#{analyze_clause} #{quoted_name}" - # Vacuum operations can take a long time SQL.query!(Repo, query, [], timeout: :infinity) :ok rescue @@ -147,7 +148,12 @@ defmodule Aprsme.DbOptimizer do defp validate_identifier!(name) when is_atom(name), do: validate_identifier!(Atom.to_string(name)) - # Default 1KB + defp quote_identifier(name) when is_binary(name) do + ~s("#{String.replace(name, ~s("), ~s(""))}") + end + + defp quote_identifier(name) when is_atom(name), do: quote_identifier(Atom.to_string(name)) + defp estimate_entry_size(nil), do: 1024 defp estimate_entry_size(entry) when is_map(entry) do diff --git a/lib/aprsme/device_identification.ex b/lib/aprsme/device_identification.ex index 2eb13a9..4b34ae2 100644 --- a/lib/aprsme/device_identification.ex +++ b/lib/aprsme/device_identification.ex @@ -161,24 +161,22 @@ defmodule Aprsme.DeviceIdentification do micelegacy = Map.get(json, "micelegacy", %{}) now = DateTime.utc_now() - Repo.transaction(fn -> - Repo.delete_all(Devices) - - Enum.each([tocalls, mice, micelegacy], fn group -> - upsert_device_group(group, now) + all_devices = + Enum.flat_map([tocalls, mice, micelegacy], fn group -> + Enum.map(group, fn {identifier, attrs} -> + process_device_attrs(attrs, identifier, now) + end) + end) + + {:ok, _result} = + Repo.transaction(fn -> + Repo.delete_all(Devices) + Repo.insert_all(Devices, all_devices) end) - end) :ok end - defp upsert_device_group(group, now) do - Enum.each(group, fn {identifier, attrs} -> - processed_attrs = process_device_attrs(attrs, identifier, now) - %Devices{} |> Devices.changeset(processed_attrs) |> Repo.insert!() - end) - end - defp process_device_attrs(attrs, identifier, now) do attrs |> Map.put("identifier", identifier) diff --git a/lib/aprsme/packets/prepared_queries.ex b/lib/aprsme/packets/prepared_queries.ex index a07af60..5c80ac0 100644 --- a/lib/aprsme/packets/prepared_queries.ex +++ b/lib/aprsme/packets/prepared_queries.ex @@ -258,6 +258,7 @@ defmodule Aprsme.Packets.PreparedQueries do # Convert miles to meters for PostGIS (1 mile = 1609.34 meters) radius_meters = radius_miles * 1609.34 + cutoff_time = DateTime.utc_now() |> DateTime.truncate(:second) diff --git a/lib/aprsme/partition_manager.ex b/lib/aprsme/partition_manager.ex index 64b2762..2227e77 100644 --- a/lib/aprsme/partition_manager.ex +++ b/lib/aprsme/partition_manager.ex @@ -56,7 +56,7 @@ defmodule Aprsme.PartitionManager do if partition_exists?(name) do acc else - create_partition(date) + create_partition_with_lock(date) [name | acc] end end @@ -64,6 +64,21 @@ defmodule Aprsme.PartitionManager do {:ok, Enum.reverse(created)} end + defp create_partition_with_lock(%Date{} = date) do + name = partition_name(date) + lock_key = :erlang.phash2(name) + + Repo.transaction(fn -> + Repo.query!("SELECT pg_advisory_xact_lock($1)", [lock_key]) + + if !partition_exists?(name) do + create_partition(date) + end + end) + + name + end + @doc """ Drops partitions older than the given number of retention days. Returns {:ok, list_of_dropped_partition_names}. @@ -164,8 +179,10 @@ defmodule Aprsme.PartitionManager do from_str = DateTime.to_iso8601(from_dt) to_str = DateTime.to_iso8601(to_dt) + validate_partition_name!(name) + Repo.query!( - "CREATE TABLE IF NOT EXISTS #{name} PARTITION OF packets FOR VALUES FROM ('#{from_str}') TO ('#{to_str}')" + "CREATE TABLE IF NOT EXISTS #{quote_identifier(name)} PARTITION OF packets FOR VALUES FROM ('#{from_str}') TO ('#{to_str}')" ) Logger.debug("Created partition #{name} [#{from_str}, #{to_str})") @@ -173,11 +190,27 @@ defmodule Aprsme.PartitionManager do end defp drop_partition(name) do - Repo.query!("DROP TABLE IF EXISTS #{name}") + # Validate partition name to prevent SQL injection + validate_partition_name!(name) + + Repo.query!("DROP TABLE IF EXISTS #{quote_identifier(name)}") Logger.debug("Dropped partition #{name}") name end + # Validates that partition name matches expected format: packets_YYYYMMDD + # Raises if name is invalid to prevent SQL injection + defp validate_partition_name!(name) do + if !String.match?(name, ~r/^packets_\d{8}$/) do + raise ArgumentError, "Invalid partition name: #{inspect(name)}" + end + end + + # Quotes SQL identifier to prevent injection + defp quote_identifier(name) do + ~s("#{String.replace(name, ~s("), ~s(""))}") + end + defp partition_date("packets_" <> date_str) do case Date.from_iso8601( String.slice(date_str, 0, 4) <> diff --git a/lib/aprsme/regex_cache.ex b/lib/aprsme/regex_cache.ex index 435b3ea..69d563b 100644 --- a/lib/aprsme/regex_cache.ex +++ b/lib/aprsme/regex_cache.ex @@ -26,19 +26,26 @@ defmodule Aprsme.RegexCache do Get or compile a regex pattern. Returns {:ok, regex} or {:error, reason}. """ def get_or_compile(pattern_string) do - case :ets.lookup(@table_name, pattern_string) do - [{^pattern_string, regex}] -> - {:ok, regex} + GenServer.call(__MODULE__, {:get_or_compile, pattern_string}) + end - [] -> - compile_and_cache(pattern_string) - end + @impl true + def handle_call({:get_or_compile, pattern_string}, _from, state) do + result = + case :ets.lookup(@table_name, pattern_string) do + [{^pattern_string, regex}] -> + {:ok, regex} + + [] -> + compile_and_cache(pattern_string) + end + + {:reply, result, state} end defp compile_and_cache(pattern_string) do case Regex.compile(pattern_string) do {:ok, regex} -> - # Check cache size and clear if needed if :ets.info(@table_name, :size) >= @max_cache_size do clear_oldest_entries() end diff --git a/lib/aprsme/release.ex b/lib/aprsme/release.ex index 8b90bde..0f82d66 100644 --- a/lib/aprsme/release.ex +++ b/lib/aprsme/release.ex @@ -105,7 +105,7 @@ defmodule Aprsme.Release do # Notify about deployment after a short delay to ensure PubSub is started # In k8s, this will notify all connected clients about the new deployment if System.get_env("DEPLOYED_AT") do - spawn(fn -> + Task.start(fn -> # Wait for application to start Process.sleep(10_000) @@ -157,10 +157,8 @@ defmodule Aprsme.Release do Ecto.Migrator.with_repo( Aprsme.Repo, fn repo -> - # Set session-level timeout for this connection - # credo:disable-for-next-line - # sobelow_skip ["SQL.Query"] - SQL.query!(repo, "SET statement_timeout = '#{div(timeout, 1000)}s'") + timeout_seconds = div(timeout, 1000) + SQL.query!(repo, "SET statement_timeout = $1", ["#{timeout_seconds}s"]) Ecto.Migrator.run(repo, :up, all: true) end, timeout: timeout diff --git a/lib/aprsme/signal_handler.ex b/lib/aprsme/signal_handler.ex index 3397f65..513f063 100644 --- a/lib/aprsme/signal_handler.ex +++ b/lib/aprsme/signal_handler.ex @@ -24,8 +24,8 @@ defmodule Aprsme.SignalHandler do def handle_info({:signal, :sigterm}, state) do Logger.info("Received SIGTERM signal, initiating graceful shutdown...") - # Trigger graceful shutdown - spawn(fn -> + # Trigger graceful shutdown with spawn_link to ensure cleanup if parent crashes + spawn_link(fn -> Aprsme.ShutdownHandler.shutdown() end) diff --git a/lib/aprsme/spatial_pubsub.ex b/lib/aprsme/spatial_pubsub.ex index 98f7b88..3a5c29f 100644 --- a/lib/aprsme/spatial_pubsub.ex +++ b/lib/aprsme/spatial_pubsub.ex @@ -9,6 +9,8 @@ defmodule Aprsme.SpatialPubSub do # Grid size in degrees for spatial indexing @grid_size 1.0 + # Maximum number of concurrent clients + @max_clients 10_000 def start_link(opts \\ []) do GenServer.start_link(__MODULE__, opts, name: __MODULE__) @@ -104,30 +106,30 @@ defmodule Aprsme.SpatialPubSub do @impl true def handle_call({:register_viewport, client_id, bounds}, {pid, _}, state) do - # Create a unique topic for this client - topic = "spatial:#{client_id}" + if map_size(state.clients) >= @max_clients do + {:reply, {:error, :client_limit_exceeded}, state} + else + topic = "spatial:#{client_id}" - state = replace_existing_client(state, client_id) + state = replace_existing_client(state, client_id) - # Monitor the client process - ref = Process.monitor(pid) + ref = Process.monitor(pid) - # Update client info - client_info = %{ - bounds: normalize_bounds(bounds), - topic: topic, - pid: pid, - monitor_ref: ref - } + client_info = %{ + bounds: normalize_bounds(bounds), + topic: topic, + pid: pid, + monitor_ref: ref + } - # Update spatial index - new_state = - state - |> put_in([:clients, client_id], client_info) - |> update_spatial_index(client_id, client_info.bounds) - |> update_in([:stats, :clients_count], &(&1 + 1)) + new_state = + state + |> put_in([:clients, client_id], client_info) + |> update_spatial_index(client_id, client_info.bounds) + |> update_in([:stats, :clients_count], &(&1 + 1)) - {:reply, {:ok, topic}, new_state} + {:reply, {:ok, topic}, new_state} + end end @impl true diff --git a/lib/aprsme/streaming_packets_pubsub.ex b/lib/aprsme/streaming_packets_pubsub.ex index 0a6efee..475f8a2 100644 --- a/lib/aprsme/streaming_packets_pubsub.ex +++ b/lib/aprsme/streaming_packets_pubsub.ex @@ -80,17 +80,10 @@ defmodule Aprsme.StreamingPacketsPubSub do @impl true def handle_call({:subscribe, pid, bounds}, _from, state) do - # Validate bounds if valid_bounds?(bounds) do - # Demonitor old ref if this pid was already subscribed (bounds update) state = demonitor_if_exists(state, pid) - - # Monitor the subscriber ref = Process.monitor(pid) - - # Store in ETS for fast lookup :ets.insert(@table_name, {pid, bounds}) - {:reply, :ok, put_in(state.monitors[pid], ref)} else {:reply, {:error, :invalid_bounds}, state} diff --git a/lib/aprsme_web/components/core_components.ex b/lib/aprsme_web/components/core_components.ex index dd8760d..6d9094b 100644 --- a/lib/aprsme_web/components/core_components.ex +++ b/lib/aprsme_web/components/core_components.ex @@ -16,6 +16,7 @@ defmodule AprsmeWeb.CoreComponents do @doc """ Renders a Heroicon SVG inline from priv/heroicons/. + SVGs are loaded from trusted vendored heroicons directory and are safe to render as raw HTML. Usage: <.icon name="arrow-left" outline={true} class="h-5 w-5" /> """ attr :name, :string, required: true @@ -37,6 +38,7 @@ defmodule AprsmeWeb.CoreComponents do case File.read(path) do {:ok, contents} -> # Insert class attribute if not present + # SVG is from trusted vendored heroicons, safe to modify and render Regex.replace(~r/]*?)>/, contents, fn _, attrs -> add_class_to_svg_tag(attrs, class) end) diff --git a/lib/aprsme_web/live/info_live/show.ex b/lib/aprsme_web/live/info_live/show.ex index d617fc9..dc42ecc 100644 --- a/lib/aprsme_web/live/info_live/show.ex +++ b/lib/aprsme_web/live/info_live/show.ex @@ -553,19 +553,6 @@ defmodule AprsmeWeb.InfoLive.Show do end end - @doc """ - Renders an APRS symbol style for use in templates. - """ - def render_symbol_style(packet, size \\ 32) do - if packet do - {symbol_table_id, symbol_code} = AprsSymbol.extract_from_packet(packet) - AprsSymbol.render_style(symbol_table_id, symbol_code, size) - else - # Return empty style if no packet - "" - end - end - @doc """ Renders an APRS symbol as HTML for overlay symbols that need proper overlay character display. """ @@ -573,36 +560,23 @@ defmodule AprsmeWeb.InfoLive.Show do if packet do {symbol_table_id, symbol_code} = AprsSymbol.extract_from_packet(packet) - # Check if this is an overlay symbol if symbol_table_id && String.match?(symbol_table_id, ~r/^[A-Z0-9]$/) do - # Use layered sprite backgrounds for overlay symbols sprite_info = AprsSymbol.get_sprite_info(symbol_table_id, symbol_code) overlay_sprite_info = AprsSymbol.get_overlay_character_sprite_info(symbol_table_id) - raw(""" -
-
- """) + style = + "position: relative; width: #{size}px; height: #{size}px; background-image: url(#{overlay_sprite_info.sprite_file}), url(#{sprite_info.sprite_file}); background-position: #{overlay_sprite_info.background_position}, #{sprite_info.background_position}; background-size: #{overlay_sprite_info.background_size}, #{sprite_info.background_size}; background-repeat: no-repeat, no-repeat; image-rendering: pixelated; display: inline-block; vertical-align: middle; margin-bottom: -6px;" + + escaped_style = style |> Phoenix.HTML.html_escape() |> Phoenix.HTML.safe_to_string() + + raw("
") else - # Use style rendering for non-overlay symbols - raw(""" -
- """) + style = AprsSymbol.render_style(symbol_table_id, symbol_code, size) + escaped_style = style |> Phoenix.HTML.html_escape() |> Phoenix.HTML.safe_to_string() + + raw("
") end else - # Return empty if no packet raw("") end end diff --git a/lib/aprsme_web/live/map_live/data_builder.ex b/lib/aprsme_web/live/map_live/data_builder.ex index 4458a67..724306e 100644 --- a/lib/aprsme_web/live/map_live/data_builder.ex +++ b/lib/aprsme_web/live/map_live/data_builder.ex @@ -676,7 +676,9 @@ defmodule AprsmeWeb.MapLive.DataBuilder do defp historical_dot_html(callsign) do escaped = callsign |> Phoenix.HTML.html_escape() |> Phoenix.HTML.safe_to_string() - "
" + Phoenix.HTML.raw( + "
" + ) end defp build_historical_packet_data(filtered_historical, has_weather) do diff --git a/lib/aprsme_web/live/status_live/index.ex b/lib/aprsme_web/live/status_live/index.ex index 47fb570..dce9ffe 100644 --- a/lib/aprsme_web/live/status_live/index.ex +++ b/lib/aprsme_web/live/status_live/index.ex @@ -38,10 +38,10 @@ defmodule AprsmeWeb.StatusLive.Index do @impl true def handle_info(:refresh_status, socket) do - # Refresh status asynchronously + # Refresh status asynchronously using supervised task self_pid = self() - Task.start(fn -> + Task.Supervisor.start_child(Aprsme.BroadcastTaskSupervisor, fn -> try do status = get_aprs_status() send(self_pid, {:status_updated, status}) diff --git a/lib/aprsme_web/live/user_confirmation_instructions_live.ex b/lib/aprsme_web/live/user_confirmation_instructions_live.ex index 9cce777..2bb3209 100644 --- a/lib/aprsme_web/live/user_confirmation_instructions_live.ex +++ b/lib/aprsme_web/live/user_confirmation_instructions_live.ex @@ -79,18 +79,30 @@ defmodule AprsmeWeb.UserConfirmationInstructionsLive do def handle_event("send_instructions", %{"user" => %{"email" => email}}, socket) do if user = Accounts.get_user_by_email(email) do - Accounts.deliver_user_confirmation_instructions( - user, - &url(~p"/users/confirm/#{&1}") - ) + case Accounts.deliver_user_confirmation_instructions( + user, + &url(~p"/users/confirm/#{&1}") + ) do + {:ok, _} -> + info = + "If your email is in our system and it has not been confirmed yet, you will receive an email with instructions shortly." + + {:noreply, + socket + |> put_flash(:info, info) + |> redirect(to: ~p"/")} + + {:error, _reason} -> + {:noreply, put_flash(socket, :error, "Failed to send email. Please try again later.")} + end + else + info = + "If your email is in our system and it has not been confirmed yet, you will receive an email with instructions shortly." + + {:noreply, + socket + |> put_flash(:info, info) + |> redirect(to: ~p"/")} end - - info = - "If your email is in our system and it has not been confirmed yet, you will receive an email with instructions shortly." - - {:noreply, - socket - |> put_flash(:info, info) - |> redirect(to: ~p"/")} end end diff --git a/lib/aprsme_web/live/user_forgot_password_live.ex b/lib/aprsme_web/live/user_forgot_password_live.ex index cc63a4f..c0b78c3 100644 --- a/lib/aprsme_web/live/user_forgot_password_live.ex +++ b/lib/aprsme_web/live/user_forgot_password_live.ex @@ -79,18 +79,30 @@ defmodule AprsmeWeb.UserForgotPasswordLive do def handle_event("send_email", %{"user" => %{"email" => email}}, socket) do if user = Accounts.get_user_by_email(email) do - Accounts.deliver_user_reset_password_instructions( - user, - &url(~p"/users/reset_password/#{&1}") - ) + case Accounts.deliver_user_reset_password_instructions( + user, + &url(~p"/users/reset_password/#{&1}") + ) do + {:ok, _} -> + info = + "If your email is in our system, you will receive instructions to reset your password shortly." + + {:noreply, + socket + |> put_flash(:info, info) + |> redirect(to: ~p"/")} + + {:error, _reason} -> + {:noreply, put_flash(socket, :error, "Failed to send email. Please try again later.")} + end + else + info = + "If your email is in our system, you will receive instructions to reset your password shortly." + + {:noreply, + socket + |> put_flash(:info, info) + |> redirect(to: ~p"/")} end - - info = - "If your email is in our system, you will receive instructions to reset your password shortly." - - {:noreply, - socket - |> put_flash(:info, info) - |> redirect(to: ~p"/")} end end diff --git a/lib/aprsme_web/live/user_registration_live.ex b/lib/aprsme_web/live/user_registration_live.ex index 3994273..4460fc4 100644 --- a/lib/aprsme_web/live/user_registration_live.ex +++ b/lib/aprsme_web/live/user_registration_live.ex @@ -148,14 +148,17 @@ defmodule AprsmeWeb.UserRegistrationLive do def handle_event("save", %{"user" => user_params}, socket) do case Accounts.register_user(user_params) do {:ok, user} -> - {:ok, _} = - Accounts.deliver_user_confirmation_instructions( - user, - &url(~p"/users/confirm/#{&1}") - ) + case Accounts.deliver_user_confirmation_instructions( + user, + &url(~p"/users/confirm/#{&1}") + ) do + {:ok, _} -> + changeset = Accounts.change_user_registration(user) + {:noreply, assign(socket, trigger_submit: true, changeset: changeset)} - changeset = Accounts.change_user_registration(user) - {:noreply, assign(socket, trigger_submit: true, changeset: changeset)} + {:error, _reason} -> + {:noreply, put_flash(socket, :error, "Failed to send email. Please try again later.")} + end {:error, %Ecto.Changeset{} = changeset} -> {:noreply, assign(socket, :changeset, changeset)} diff --git a/lib/aprsme_web/live/user_settings_live.ex b/lib/aprsme_web/live/user_settings_live.ex index b7b4c2c..b6c84c3 100644 --- a/lib/aprsme_web/live/user_settings_live.ex +++ b/lib/aprsme_web/live/user_settings_live.ex @@ -348,14 +348,18 @@ defmodule AprsmeWeb.UserSettingsLive do case Accounts.apply_user_email(user, password, user_params) do {:ok, applied_user} -> - Accounts.deliver_user_update_email_instructions( - applied_user, - user.email, - &url(~p"/users/settings/confirm_email/#{&1}") - ) + case Accounts.deliver_user_update_email_instructions( + applied_user, + user.email, + &url(~p"/users/settings/confirm_email/#{&1}") + ) do + {:ok, _} -> + info = "A link to confirm your email change has been sent to the new address." + {:noreply, put_flash(socket, :info, info)} - info = "A link to confirm your email change has been sent to the new address." - {:noreply, put_flash(socket, :info, info)} + {:error, _reason} -> + {:noreply, put_flash(socket, :error, "Failed to send email. Please try again later.")} + end {:error, changeset} -> {:noreply, assign(socket, :email_changeset, Map.put(changeset, :action, :insert))}