bugfixes

config/dev.exs

@@ -61,9 +61,9 @@ config :aprsme, AprsmeWeb.Endpoint,
   live_reload: [
     web_console_logger: true,
     patterns: [
-      ~r"priv/static/(?!uploads/).*(js|css|png|jpeg|jpg|gif|svg)$",
-      ~r"priv/gettext/.*(po)$",
-      ~r"lib/aprsme_web/(?:controllers|live|components|router)/?.*\.(ex|heex)$"
+      ~r"priv/static/(?!uploads/).*(js|css|png|jpeg|jpg|gif|svg)$"E,
+      ~r"priv/gettext/.*(po)$"E,
+      ~r"lib/aprsme_web/(?:controllers|live|components|router)/?.*\.(ex|heex)$"E
     ]
   ]
 

config/test.exs

@@ -10,7 +10,7 @@ pool_size =
   if System.get_env("MIX_TEST_COVERAGE") do
     2
   else
-    System.schedulers_online() * 16
+    min(System.schedulers_online() * 2, 20)
   end
 
 # In test we don't send emails.

findings.md

@@ -1,103 +0,0 @@
-# Findings: aprs.me Code Review
-
-## Security Findings
-
-### CRITICAL: Production secrets exposed on disk
-`.envrc` contains all production secrets in plaintext: APRS credentials, `DATABASE_URL`, `SECRET_KEY_BASE`, `RESEND_API_KEY`, `RELEASE_COOKIE`. Protected by `.gitignore` but accessible to anyone with filesystem access.
-
-### CRITICAL: APRS credentials in K8s manifest
-`k8s/deployment.yaml:64-67,136-143` — APRS callsign and password hardcoded as plaintext env vars instead of Kubernetes Secrets.
-
-### HIGH: Secret key base in git history
-`config/dev.exs:48`, `config/test.exs:40` — Development and test `secret_key_base` values committed since initial commit.
-
-### HIGH: SQL injection vector
-`lib/aprsme/release.ex:162` — `SQL.query!` uses string interpolation with env var input:
-```elixir
-SQL.query!(repo, "SET statement_timeout = '#{timeout_seconds}s'", [])
-```
-
-### MEDIUM: Session signed but not encrypted
-`lib/aprsme_web/endpoint.ex:7-13` — Session configured with `signing_salt` only, no `encryption_salt`. Session contents can be base64-decoded by anyone with cookie access.
-
-### MEDIUM: `raw()` HTML rendering in LiveView
-`lib/aprsme_web/live/info_live/show.ex:537,542,545` — APRS symbol rendering uses `raw()` to inject HTML. While content is escaped, `raw()` bypasses HEEx auto-escaping at template level.
-
-### MEDIUM: `raw()` SVG rendering
-`lib/aprsme_web/components/core_components.ex:50` — SVG content rendered via `raw()`. Risk is low (files are controlled vendored assets).
-
-### MEDIUM: `check_origin: false` in dev
-`config/dev.exs:45` — WebSocket origin checks disabled in development.
-
-### MEDIUM: Anonymous WebSocket connections
-`lib/aprsme_web/channels/mobile_user_socket.ex:21-29` — Mobile socket requires no authentication. Rate-limited to 30/min/IP but otherwise open.
-
-### MEDIUM: No auth on API endpoints
-`lib/aprsme_web/router.ex:131-136` — API v1 (`/api/v1/callsign/:cs`, `/api/v1/weather/nearby`) is anonymous. Rate-limited but no API keys.
-
-### MEDIUM: APRS-IS connection uses plain TCP
-`lib/aprsme/is/is.ex:215` — APRS passcode sent in cleartext over `:gen_tcp` (no SSL).
-
-### LOW: No `secure` flag on session cookie
-`lib/aprsme_web/endpoint.ex:8-13` — No `secure: true` in session options. Handled by Phoenix in practice but not explicit.
-
-### LOW: Password policy too lax
-`lib/aprsme/accounts/user.ex:73-75` — Character class requirements are commented out. Passwords only require 12+ characters.
-
-### LOW: Rate limiting not shared across cluster
-`config/runtime.exs:133` — `Hammer.Backend.ETS` is per-node. Multi-node deployments can exhaust per-node limits independently.
-
-### INFO: CSP allows `unsafe-inline` and `unsafe-eval`
-`lib/aprsme_web/router.ex:23-26` — Content Security Policy allows both, weakening XSS protection.
-
-### INFO: 27+ Sobelow findings pre-skipped
-`.sobelow-skips` — Reviewed and deemed acceptable (parameterized spatial queries, symbol rendering, upstream TLS).
-
----
-
-## Code Quality & Refactoring Findings
-
-### HIGH: `map_live/index.ex` is 2,104 lines
-Largest module in the project. Combines socket management, event handling, rendering, map state, and overlay logic. Extract into focused sub-modules (map_state, map_events, map_render).
-
-### HIGH: Duplicated `has_weather` logic
-`lib/aprsme/packet.ex:159` and `lib/aprsme/packet_consumer.ex` both maintain parallel `set_has_weather/1` logic. If weather detection rules change, one path will be missed.
-
-### HIGH: 17 modules missing `@moduledoc`
-Modules lacking documentation include: `Packet`, `PacketConsumer`, `DataExtended`, `BadPacket`, `Accounts.User`, `Accounts.UserNotifier`, `Packets`, and others.
-
-### MEDIUM: `require Logger` in function bodies (~75 occurrences)
-Several modules duplicate `require Logger` at module level AND inside function bodies (e.g., `packets.ex:17,27`, `packet_consumer.ex`). Consolidate to module top-level.
-
-### MEDIUM: `packets.ex` is 849 lines
-Mix of query functions, store logic, and validation. Consider splitting query/concerns into separate modules.
-
-### MEDIUM: Cache module has unnecessary GenServer overhead
-`lib/aprsme/cache.ex` — Wraps ETS operations in GenServer calls. GenServer passes through `%{}` state with no meaningful state management. Direct ETS access would be faster for get/put.
-
-### MEDIUM: Encoding vs EncodingUtils boundary unclear
-`encoding_utils.ex` (404 lines) wraps Gleam-based `Encoding` module but also contains substantive logic (`sanitize_packet`, `normalize_data_type`). Clarify responsibility boundary.
-
-### MEDIUM: Inline `import Ecto.Query` in function bodies
-16 occurrences across `mobile_channel.ex`, `packet_utils.ex`, `info_live/show.ex`, `packets.ex`. Move to module top-level.
-
-### MEDIUM: Large inline SQL in LiveViews
-`info_live/show.ex:369,392,484,491` — Complex ST_Distance calculations embedded in LiveView. Extract to query modules.
-
-### LOW: Geometry stored redundantly
-`packet.ex` stores both scalar `lat`/`lon` AND PostGIS `location` geometry. `maybe_create_geometry_from_lat_lon/1` duplicates the data. Consider if both are needed.
-
-### LOW: Behaviour only covers subset of functions
-`packets_behaviour.ex` defines 8 callbacks but `packets.ex` has far more public functions. Only partial mockability.
-
-### LOW: Test coverage at 91.13% (threshold 87%)
-Only 4.13% above threshold. Focus on uncovered error/rescue paths and edge cases.
-
-### LOW: Performance TODOs not tracked in code
-`TODO.md` items (table partitioning, ETS write path) not marked as `# TODO` in source code. Won't appear in IDE task lists.
-
----
-
-## Already Fixed
-
-- **`packets.html.heex` (unstyled `<h1>`)**: Dead template with bare `<h1>Packets</h1>` removed. The `/packets` route uses `PacketsLive.Index` LiveView, not this controller template.

lib/aprsme/application.ex

@@ -29,8 +29,6 @@ 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
@@ -184,8 +182,7 @@ defmodule Aprsme.Application do
   defp redis_children do
     Logger.info("Starting ETS-based caching and rate limiting")
 
-    # Create ETS tables for caching — use :public so the Cache GenServer (a separate process)
-    # can write to these tables; write_concurrency improves throughput under concurrent writes.
+    # Create public ETS tables so callers can use concurrent reads and writes directly.
     _ = :ets.new(:query_cache, [:set, :public, :named_table, read_concurrency: true, write_concurrency: true])
     _ = :ets.new(:device_cache, [:set, :public, :named_table, read_concurrency: true, write_concurrency: true])
     _ = :ets.new(:symbol_cache, [:set, :public, :named_table, read_concurrency: true, write_concurrency: true])

lib/aprsme/cache.ex

@@ -8,22 +8,30 @@ 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
-    GenServer.call(__MODULE__, {:get, cache_name, key})
+    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
 
   @doc """
@@ -33,28 +41,47 @@ 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
-    GenServer.call(__MODULE__, {:put, cache_name, key, value, opts})
+    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
 
   @doc """
   Delete a key from cache
   """
   def del(cache_name, key) do
-    GenServer.call(__MODULE__, {:del, cache_name, key})
+    :ets.delete(cache_name, key)
+    {:ok, true}
+  rescue
+    ArgumentError -> {:error, :no_cache}
   end
 
   @doc """
   Clear all keys from cache
   """
   def clear(cache_name) do
-    GenServer.call(__MODULE__, {:clear, cache_name})
+    :ets.delete_all_objects(cache_name)
+    {:ok, true}
+  rescue
+    ArgumentError -> {:error, :no_cache}
   end
 
   @doc """
   Get cache statistics (simplified for ETS)
   """
   def stats(cache_name) do
-    GenServer.call(__MODULE__, {:stats, cache_name})
+    case :ets.info(cache_name) do
+      :undefined -> {:error, :no_cache}
+      info when is_list(info) -> {:ok, %{size: Keyword.get(info, :size, 0)}}
+    end
   end
 
   @doc """
@@ -72,7 +99,14 @@ defmodule Aprsme.Cache do
   Get TTL for a key. Returns remaining milliseconds or nil if no TTL.
   """
   def ttl(cache_name, key) do
-    GenServer.call(__MODULE__, {:ttl, cache_name, key})
+    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
 
   @doc """
@@ -92,108 +126,4 @@ 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 =
-      case :ets.info(cache_name) do
-        :undefined -> {:error, :no_cache}
-        info when is_list(info) -> {:ok, %{size: Keyword.get(info, :size, 0)}}
-      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

lib/aprsme_web/live/info_live/show.ex

@@ -86,6 +86,8 @@ defmodule AprsmeWeb.InfoLive.Show do
       |> assign(:callsign, normalized_callsign)
       |> assign(:packet, packet)
       |> assign(:neighbors, neighbors)
+      |> assign(:heard_by_stations, [])
+      |> assign(:stations_heard_by, [])
       |> assign(:page_title, "APRS station #{normalized_callsign}")
       |> assign(:has_weather_packets, has_weather_packets)
       |> assign(:other_ssids, other_ssids)
@@ -398,11 +400,11 @@ defmodule AprsmeWeb.InfoLive.Show do
         MIN(pp.received_at) as first_heard,
         MAX(pp.received_at) as last_heard,
         COUNT(*) as packet_count,
-        MAX(lp.longest_path_packet_id) as longest_path_packet_id
+        lp.longest_path_packet_id
       FROM parsed_paths pp
       LEFT JOIN longest_paths lp ON lp.first_digipeater = pp.first_digipeater
       WHERE pp.first_digipeater IS NOT NULL
-      GROUP BY pp.first_digipeater
+      GROUP BY pp.first_digipeater, lp.longest_path_packet_id
     )
     SELECT
       ds.digipeater,

lib/aprsme_web/live/map_live/index.ex

@@ -36,8 +36,6 @@ defmodule AprsmeWeb.MapLive.Index do
 
   @impl true
   def mount(params, session, socket) do
-    socket = setup_subscriptions(socket)
-
     # Basic setup
     deployed_at = Aprsme.Release.deployed_at()
     one_day_ago = TimeUtils.hours_ago(24)
@@ -86,6 +84,7 @@ defmodule AprsmeWeb.MapLive.Index do
 
     # Calculate initial bounds
     initial_bounds = BoundsUtils.calculate_bounds_from_center_and_zoom(final_map_center, final_map_zoom)
+    socket = setup_subscriptions(socket, initial_bounds)
 
     # Final socket assignment
     {:ok,
@@ -100,11 +99,11 @@ defmodule AprsmeWeb.MapLive.Index do
      })}
   end
 
-  defp setup_subscriptions(socket) do
-    do_setup_subscriptions(socket, connected?(socket))
+  defp setup_subscriptions(socket, initial_bounds) do
+    do_setup_subscriptions(socket, initial_bounds, connected?(socket))
   end
 
-  defp do_setup_subscriptions(socket, true) do
+  defp do_setup_subscriptions(socket, initial_bounds, true) do
     # Check if we should accept new connections
     if Application.get_env(:aprsme, :cluster_enabled, false) and
          not Aprsme.ConnectionMonitor.accepting_connections?() do
@@ -125,10 +124,10 @@ defmodule AprsmeWeb.MapLive.Index do
           Phoenix.PubSub.subscribe(Aprsme.PubSub, "connection:drain:#{Node.self()}")
         end
 
-      # Register with spatial PubSub (will get viewport later)
-      # Start with a default viewport that will be updated when we get actual bounds
-      default_bounds = %{north: 90.0, south: -90.0, east: 180.0, west: -180.0}
-      {:ok, spatial_topic} = Aprsme.SpatialPubSub.register_viewport(client_id, default_bounds)
+      # Register the actual initial viewport. Registering world bounds here used to
+      # allocate roughly 65,000 one-degree grid cells for every new connection,
+      # only to tear them down again after the client sent its real bounds.
+      {:ok, spatial_topic} = Aprsme.SpatialPubSub.register_viewport(client_id, initial_bounds)
 
       # Subscribe to the spatial topic for this client
       :ok = Phoenix.PubSub.subscribe(Aprsme.PubSub, spatial_topic)
@@ -140,7 +139,7 @@ defmodule AprsmeWeb.MapLive.Index do
       :ok = Phoenix.PubSub.subscribe(Aprsme.PubSub, "deployment_events")
 
       # Subscribe to StreamingPacketsPubSub with initial bounds
-      _ = Aprsme.StreamingPacketsPubSub.subscribe_to_bounds(self(), default_bounds)
+      _ = Aprsme.StreamingPacketsPubSub.subscribe_to_bounds(self(), initial_bounds)
 
       _ = Process.send_after(self(), :cleanup_old_packets, 60_000)
 
@@ -150,7 +149,7 @@ defmodule AprsmeWeb.MapLive.Index do
     end
   end
 
-  defp do_setup_subscriptions(socket, false), do: socket
+  defp do_setup_subscriptions(socket, _initial_bounds, false), do: socket
 
   defp setup_additional_subscriptions(socket) do
     do_setup_additional_subscriptions(socket, connected?(socket))

test/aprsme/connection_prevention_test.exs

@@ -19,13 +19,13 @@ defmodule Aprsme.ConnectionPreventionTest do
 
   test "APRS configuration is safe for testing" do
     # Verify server configuration is neutralized
-    server = Application.get_env(:aprsme, :aprsme_is_server)
+    server = Application.get_env(:aprsme, :aprs_is_server)
     # credo:disable-for-next-line Jump.CredoChecks.ConditionalAssertion
     assert server == nil or server == "mock.aprs.test"
     # server config is legitimately nil or mock value in test
 
     # Verify test credentials are used
-    login_id = Application.get_env(:aprsme, :aprsme_is_login_id)
+    login_id = Application.get_env(:aprsme, :aprs_is_login_id)
     assert login_id == "TEST"
 
     # Verify no real APRS servers in config