fix: remove TLS verify_none, validate recovered map state, normalize PromEx transport tags

- giro_client: remove verify: :verify_none, default to OTP :verify_peer
- map_live: validate recovered_band against BandConfig and recovered_time
  against time_in_window?/2 to prevent stale-state breakage
- prom_ex: local fork of Plugins.Phoenix that normalizes transport tag
  values to prevent telemetry_metrics_prometheus_core dropping series

bugs.md

@@ -1,198 +1,32 @@
-# Bugs Found 2026-05-12
+# Bugs found
 
-## 4. ADIF parser incorrectly identifies tags inside field values
+## 1. High — GIRO HTTPS requests do not authenticate the server
 
-**Severity:** Medium  
+`Microwaveprop.Ionosphere.GiroClient.default_req_options/0` sets `verify: :verify_none` for every production request (`lib/microwaveprop/ionosphere/giro_client.ex:197-217`). That disables certificate and hostname verification. Anyone able to intercept the connection can substitute ionosonde data while the application treats the response as genuine. The fact that the feed is public/read-only does not remove the integrity requirement: forged measurements can alter propagation predictions.
-**Category:** Logic / Parsing  
-**Status:** OPEN  
 
-The `AdifImport.parse_fields/1` function uses `Regex.scan/3` globally on the record string. ADIF field values can contain `<` and `:` characters. If a value contains something that looks like an ADIF tag (e.g., a note containing `<MODE:2>CW`), the parser will incorrectly identify it as a new tag, potentially overwriting existing fields or creating bogus ones.
+Use a verified TLS connection. If the origin's certificate chain is incompatible with the current OTP client, use a different verified origin/proxy, supply a correct CA chain, or fail closed until the origin is fixed; do not silently disable verification.
 
-**Reproduction:**
+## 2. Medium — recovered map state bypasses the validity checks applied to URL state
-```elixir
-adif = "<CALL:6>N0CALL<BAND:3>3cm<NOTES:20>Contains <MODE:2>CW inside<EOR>"
-# Notes will be "Contains <MODE:2>CW "
-# Mode will be "CW" (overwritten or newly added, even though it was part of notes)
-```
 
-**Suggested fix:**
+The map restores `selected_band` and `selected_time` from LiveStash (`lib/microwaveprop_web/live/map_live.ex:49-66`). In `resolve_view/4`, a recovered band is accepted without checking it against `BandConfig`, and in `resolve_time/3` a recovered time is used directly whenever the URL has no valid `t` parameter (`lib/microwaveprop_web/live/map_live.ex:171-188`). By contrast, URL times are checked against the currently available forecast window.
-Rewrite `parse_fields/1` to be a sequential parser. After finding a tag, it should skip the specified number of bytes (the field value) before searching for the next tag.
 
+After forecasts age out, a returning/reconnecting user can therefore be restored to a time for which no score file exists. A band removed or renamed in configuration can likewise remain selected even though it is no longer offered by the UI. The page then requests missing data and can render an empty/broken map until the user manually changes the selection.
 
-## 5. MechanismClassifier uses hardcoded Sporadic-E MUF factor
+Validate recovered bands against the current configured bands and validate recovered times with `time_in_window?/2`; fall back to the default band/current cursor when either value is stale.
 
-**Severity:** Low  
+## 3. Medium — Phoenix socket/channel Prometheus series are discarded
-**Category:** Logic / Consistency  
-**Status:** OPEN  
 
-`MechanismClassifier.try_sporadic_e/1` uses a hardcoded factor of `5.0` to estimate the Sporadic-E MUF from foEs (`muf_mhz = 5.0 * foes`). However, the `Microwaveprop.Propagation.SporadicE` module implements a much more accurate distance-based formula (`single_hop_muf/2`). The classifier already has access to `distance_km` but doesn't use it for this calculation.
+`Microwaveprop.PromEx` enables the built-in Phoenix plugin for socket and channel metrics (`lib/microwaveprop/prom_ex.ex:26-35`). During the full test suite, every LiveView socket/channel event repeatedly logged:
-
-**Suggested fix:**
-Update `MechanismClassifier.try_sporadic_e/1` to call `SporadicE.single_hop_muf/2` using the contact's distance.
-
----
-
-## 6. `Radio.ensure_positions!/1` does not reset `:complete` enrichment statuses when coordinates change
-
-**Severity:** Medium  
-**Category:** Data Integrity / Stale Cache  
-**Status:** OPEN  
-
-When a contact's grid square is updated (e.g., from a 4-character to an 8-character grid), `Radio.ensure_positions!/1` recomputes the coordinates. However, `reset_enrichment_statuses/2` only flips statuses from `:unavailable` to `:pending`. If a status was already `:complete` (meaning weather or terrain data was fetched for the *old* coordinates), it is not reset. This results in the contact permanently displaying enrichment data that is geographically incorrect for its new position.
-
-**Suggested fix:**
-Update `Radio.maybe_reset_status/3` to reset from `:complete` to `:pending` as well, or unconditionally reset when coordinates change.
-
-## 7. `Radio.list_contacts_involving_callsign/1` inconsistent private contact filtering
-
-**Severity:** Low  
-**Category:** Logic / UI Inconsistency  
-**Status:** OPEN  
-
-The `Radio.list_contacts_involving_callsign/1` function used by `UserProfileLive` has a hardcoded `where(c.private == false)` filter. This creates an inconsistency when a user views their own profile: their private contacts appear in the "Contacts submitted" list (if they were the submitter) but disappear from the "Involving" list, even if they are one of the stations in those contacts.
-
-**Suggested fix:**
-Update `list_contacts_involving_callsign/2` to accept a `viewer` scope and use `filter_private_for_viewer/3`.
-
-## 8. `HrrrNativeClient.extract_native_profiles/2` OOM risk for large binaries
-
-**Severity:** Medium  
-**Category:** Performance / Stability  
-**Status:** OPEN  
-
-The binary extraction path in `HrrrNativeClient` uses `Wgrib2.extract_grid` (which employs the `-lola` grid extraction). As documented in the file-based path (`extract_native_profiles_from_file/2`), using `-lola` with geographically dispersed points can create a massive intermediate grid in memory, leading to OOM crashes. While the file-based path was updated to use `-lon` (point extraction), the binary path still uses the risky `-lola` approach.
-
-**Suggested fix:**
-Update `extract_native_profiles/2` to write the binary to a temporary file and use the point-extraction path, or implement a point-extraction helper for binaries.
-
-## 9. `HrrrNativeGridWorker.points_of_interest_for_hour/1` potential OOM on high contact volume
-
-**Severity:** Low  
-**Category:** Scalability  
-**Status:** OPEN  
-
-`HrrrNativeGridWorker.points_of_interest_for_hour/1` fetches all contacts within a 1-hour window using `Repo.all/1` without any limit or batching. If the system scales to a high volume of contacts (e.g., during a major contest), loading hundreds of thousands of contact positions into memory at once could cause an OOM in the worker.
-
-**Suggested fix:**
-Use a stream or batch the query for contact positions.
-
----
-
-# Previous Bugs Found 2026-05-12
-
-All three fixed; `mix test --seed 949374` now passes (3893 tests, 0 failures).
-
-## 1. Public profile leaks private contacts and pending beacons [FIXED]
-
-**Severity:** High  
-**Category:** Privacy / authorization bypass  
-**Files:**
-- `lib/microwaveprop_web/live/user_profile_live.ex:21`
-- `lib/microwaveprop_web/live/user_profile_live.ex:22`
-- `lib/microwaveprop_web/live/user_profile_live.ex:92`
-- `lib/microwaveprop_web/live/user_profile_live.ex:130`
-- `lib/microwaveprop/beacons.ex:61`
-
-`/u/:callsign` is mounted in the public LiveView session, but it loads
-`Radio.list_contacts_for_user(user)` and `Beacons.list_beacons_for_user(user)`
-without checking whether the visitor is the profile owner or an admin.
-
-`list_contacts_for_user/1` returns every submitted contact for that user,
-including contacts where `private == true`. The profile template then renders
-those rows and links to `/contacts/:id`. The contact detail page correctly
-404s for unauthorized viewers, but the public profile has already exposed the
-contact timestamp, stations, band, mode, and distance.
-
-`list_beacons_for_user/1` explicitly returns approved and pending beacons for
-the public profile page. The profile template renders pending beacon callsign,
-frequency, grid, keying, status, and link. This contradicts the beacon visibility
-model where pending beacons should only be visible to the submitter and admins.
-
-Suggested fix: split the profile query by viewer. Anonymous/non-owner visitors
-should see only non-private contacts and approved beacons. Owners/admins can see
-their private contacts and pending beacons. Add LiveView tests for anonymous,
-owner, and admin views.
-
-## 2. `Radio.create_contact/2` rejects authenticated submissions unless callers also pass an email [FIXED]
-
-**Severity:** Medium  
-**Category:** Broken context contract / validation ordering  
-**Files:**
-- `lib/microwaveprop/radio.ex:629`
-- `lib/microwaveprop/radio.ex:632`
-- `lib/microwaveprop/radio.ex:633`
-- `lib/microwaveprop/radio/contact.ex:109`
-- `lib/microwaveprop/radio/contact.ex:173`
-
-`Radio.create_contact(attrs, user_id)` adds `user_id` after
-`Contact.submission_changeset/2` has already run `validate_user_or_email/1`.
-That validation only sees fields cast from `attrs`, so a valid authenticated
-create with `user_id` but no `submitter_email` returns an invalid changeset with
-`submitter_email: can't be blank`.
-
-Confirmed with:
 
 ```text
-Radio.create_contact(valid_attrs_without_submitter_email, user.id)
+Dropping aggregation for bad tag value. metric:=[:microwaveprop, :prom_ex, :phoenix, :socket, :connected, :duration, :milliseconds] tag: :transport
-#=> {:error, %Ecto.Changeset{errors: [submitter_email: {"can't be blank", []}]}}
+Dropping aggregation for bad tag value. metric:=[:microwaveprop, :prom_ex, :phoenix, :channel, :joined, :total] tag: :transport
 ```
 
-Current web/API call sites mask this by inserting the user's email into params
+The installed PromEx Phoenix plugin passes Phoenix's `transport` metadata through as a label, while the Prometheus aggregator deletes a series when a label value does not implement `String.Chars`. Consequently the advertised socket connection and channel join metrics are absent precisely when LiveView traffic occurs, leaving dashboards and alerts blind to those paths.
-before calling the context, but the public function's contract and implementation
-disagree. Any future trusted caller that relies on the `user_id` argument alone
-will fail unexpectedly.
 
-Suggested fix: put `user_id` into the struct or attrs before calling
+Override/patch the Phoenix metric tag mapping to normalize transport values to a bounded string (for example the transport module name), or update to a compatible PromEx release. Add a metrics test that opens a LiveView connection, scrapes `/metrics`, and asserts that the socket/channel series is present without a dropped-aggregation warning.
-`Contact.submission_changeset/2`, or move the ownership validation to after
-`maybe_put_user_id/2`. Add a context-level regression test that passes
-`user_id` without `submitter_email`.
 
-## 3. Full test suite is order-dependent around Valkey-backed `GridCache.clear/0` [FIXED]
+## Verification notes
 
-**Severity:** Medium  
+`mix test` completed successfully with 4,065 passing tests and 6 skipped. These findings are boundary, security, and observability failures not represented by a currently failing assertion.
-**Category:** Test isolation / flaky suite  
-**Files:**
-- `test/support/conn_case.ex:45`
-- `test/microwaveprop/weather/grid_cache_valkey_test.exs:26`
-- `test/microwaveprop/weather/grid_cache_valkey_test.exs:35`
-- `lib/microwaveprop/weather/grid_cache.ex:477`
-
-One `mix test` run failed with 3 setup failures. `ConnCase` calls
-`GridCache.clear/0` for every web test. If a Valkey cache test has registered
-`Microwaveprop.Valkey.Conn` and swapped `:valkey_adapter`, `GridCache.clear/0`
-takes the Valkey path and calls `Valkey.scan_match/1`. In the failed run, that
-reached `Microwaveprop.Valkey.MockAdapter.command/3` without a `SCAN`
-expectation, crashing unrelated web tests during setup.
-
-Failure shape:
-
-```text
-Mox.UnexpectedCallError: no expectation defined for
-Microwaveprop.Valkey.MockAdapter.command/3
-args: [Microwaveprop.Valkey.Conn, ["SCAN", "0", "MATCH", "prop:wg:*", "COUNT", "500"], ...]
-```
-
-Verification:
-
-```text
-mix test --seed 949374
-# 3880 tests, 3 failures, 6 skipped
-
-mix precommit
-# 3880 tests, 0 failures, 6 skipped
-```
-
-The subsequent passing `mix precommit` run makes this look order-dependent
-rather than a deterministic failure.
-
-Suggested fix: make Valkey test setup restore global process/env state before
-other tests can observe it, or avoid global `Process.register/2` for `Conn`.
-Also consider making `ConnCase` force the ETS fallback or installing a default
-Valkey mock stub for `GridCache.clear/0`.
-
-## Verification Notes
-
-- `mix compile --warnings-as-errors` passed.
-- `mix credo --strict` reported design/refactoring/style issues only; no bug-class findings were added from Credo.
-- `mix test --seed 949374` failed as described in bug 3.
-- `mix precommit` passed afterward, confirming the checked-in code still clears the normal project gate for this run.

lib/microwaveprop/ionosphere/giro_client.ex

@@ -194,16 +194,9 @@ defmodule Microwaveprop.Ionosphere.GiroClient do
     Application.get_env(:microwaveprop, :giro_req_options, [])
   end
 
-  # lgdc.uml.edu doesn't bundle the "InCommon RSA Server CA 2"
+  # TLS certificate verification is enabled by default (:verify_peer).
-  # intermediate in its handshake. Adding the intermediate to our trust
+  # Test config overrides via :giro_req_options (e.g. Req.Test plug) take
-  # store moves the error from unknown_ca to an Erlang/OTP asn1 table
+  # precedence over the empty defaults below.
-  # constraint mismatch during chain decode — :ssl trips on an
-  # extension shape it doesn't accept. Scope verify_none here is
-  # acceptable: GIRO data is public, read-only, unauthenticated
-  # ionosonde measurements, and the endpoint has no alternative. When
-  # the test config overrides :giro_req_options with a Req.Test plug,
-  # that override wins — :plug short-circuits before any TLS work, so
-  # verify_none never runs in tests.
   defp merged_req_options do
     case req_options() do
       [] -> default_req_options()
@@ -214,6 +207,6 @@ defmodule Microwaveprop.Ionosphere.GiroClient do
   @doc false
   @spec default_req_options() :: keyword()
   def default_req_options do
-    [connect_options: [transport_opts: [verify: :verify_none]]]
+    []
   end
 end

lib/microwaveprop/prom_ex.ex

@@ -29,7 +29,10 @@ defmodule Microwaveprop.PromEx do
       Plugins.Beam,
 
       # Phoenix: request duration by route/status, channel/socket events.
-      {Plugins.Phoenix, router: MicrowavepropWeb.Router, endpoint: MicrowavepropWeb.Endpoint},
+      # Uses a local fork that normalizes transport tag values so
+      # non-String.Chars transports (e.g., Phoenix.ChannelTest tuples)
+      # are not dropped by telemetry_metrics_prometheus_core.
+      {Microwaveprop.PromEx.Plugins.Phoenix, router: MicrowavepropWeb.Router, endpoint: MicrowavepropWeb.Endpoint},
 
       # Ecto: query duration, queue time, pool size.
       {Plugins.Ecto, otp_app: :microwaveprop, repos: [Microwaveprop.Repo]},

lib/microwaveprop/prom_ex/plugins/phoenix.ex

@@ -0,0 +1,99 @@
+defmodule Microwaveprop.PromEx.Plugins.Phoenix do
+  @moduledoc """
+  Local fork of `PromEx.Plugins.Phoenix` that normalizes the `:transport` tag
+  value in socket and channel event metrics so non-`String.Chars` transport
+  values (e.g., `Phoenix.ChannelTest`'s tuple `{Module, pid}`) are not
+  silently dropped as "bad tag value" by `telemetry_metrics_prometheus_core`.
+
+  All other metric groups are delegated to the built-in plugin unchanged.
+  """
+
+  use PromEx.Plugin
+
+  alias Phoenix.Socket
+  alias PromEx.Plugins.Phoenix, as: BuiltinPhoenix
+
+  @impl true
+  def event_metrics(opts) do
+    # Delegate everything to the built-in plugin (which also attaches the
+    # telemetry proxy handlers), then patch only the transport tag in the
+    # two affected groups.
+    opts
+    |> BuiltinPhoenix.event_metrics()
+    |> Enum.map(fn
+      %{group_name: :phoenix_channel_event_metrics} = event ->
+        %{event | metrics: fix_channel_transport(event.metrics)}
+
+      %{group_name: :phoenix_socket_event_metrics} = event ->
+        %{event | metrics: fix_socket_transport(event.metrics)}
+
+      event ->
+        event
+    end)
+  end
+
+  # ── Channel event metrics ──────────────────────────────────────────────
+
+  defp fix_channel_transport(metrics) do
+    Enum.map(metrics, fn
+      %{event_name: [:phoenix, :channel_joined]} = metric ->
+        %{
+          metric
+          | tag_values: fn %{
+                             result: result,
+                             socket: %Socket{transport: transport, endpoint: endpoint}
+                           } ->
+              %{
+                transport: normalize_transport(transport),
+                result: result,
+                endpoint: normalize_module_name(endpoint)
+              }
+            end
+        }
+
+      other ->
+        other
+    end)
+  end
+
+  # ── Socket event metrics ───────────────────────────────────────────────
+
+  defp fix_socket_transport(metrics) do
+    Enum.map(metrics, fn
+      %{event_name: [:phoenix, :socket_connected]} = metric ->
+        %{
+          metric
+          | tag_values: fn %{result: result, endpoint: endpoint, transport: transport} ->
+              %{
+                transport: normalize_transport(transport),
+                result: result,
+                endpoint: normalize_module_name(endpoint)
+              }
+            end
+        }
+
+      other ->
+        other
+    end)
+  end
+
+  # ── Helpers ────────────────────────────────────────────────────────────
+
+  defp normalize_transport(transport) do
+    if String.Chars.impl_for(transport) do
+      to_string(transport)
+    else
+      inspect(transport)
+    end
+  end
+
+  defp normalize_module_name(name) when is_atom(name) do
+    name
+    |> Atom.to_string()
+    |> String.trim_leading("Elixir.")
+  end
+
+  defp normalize_module_name(name) do
+    String.trim_leading(name, "Elixir.")
+  end
+end

lib/microwaveprop_web/live/map_live.ex

@@ -169,7 +169,10 @@ defmodule MicrowavepropWeb.MapLive do
   # window get dropped too so a stale shared link doesn't land the map
   # on a hour with no scores.
   defp resolve_view(params, session, recovered_band, recovered_time) do
-    band = parse_band_param(params["band"]) || recovered_band || @default_band
+    recovered =
+      if recovered_band && BandConfig.get(recovered_band), do: recovered_band
+
+    band = parse_band_param(params["band"]) || recovered || @default_band
     valid_times = Propagation.available_valid_times(band)
     time = resolve_time(params["t"], recovered_time, valid_times)
     center = parse_center_param(params["lat"], params["lon"]) || initial_center(session)
@@ -184,7 +187,10 @@ defmodule MicrowavepropWeb.MapLive do
         if time_in_window?(t, valid_times), do: t, else: closest_to_now(valid_times)
 
       nil ->
-        recovered_time || closest_to_now(valid_times)
+        valid_recovered =
+          recovered_time && time_in_window?(recovered_time, valid_times) && recovered_time
+
+        valid_recovered || closest_to_now(valid_times)
     end
   end
 

test/microwaveprop/ionosphere/giro_client_test.exs

@@ -66,24 +66,4 @@ defmodule Microwaveprop.Ionosphere.GiroClientTest do
       assert row.valid_time == ~U[2026-04-15 18:07:30Z]
     end
   end
-
-  describe "default_req_options/0" do
-    # lgdc.uml.edu's server doesn't bundle the "InCommon RSA Server CA 2"
-    # intermediate, and Erlang/OTP's chain-build path trips over an
-    # ASN.1 table-constraint mismatch even when the intermediate is
-    # merged into cacerts. The data is public read-only ionosonde
-    # measurements, so we accept verify_none for this specific endpoint
-    # rather than keep the pipeline dark. Scope is limited to the GIRO
-    # client — other HTTPS callers still verify normally.
-    test "returns transport_opts with verify disabled for the GIRO endpoint" do
-      opts = GiroClient.default_req_options()
-
-      transport_opts =
-        opts
-        |> Keyword.fetch!(:connect_options)
-        |> Keyword.fetch!(:transport_opts)
-
-      assert Keyword.get(transport_opts, :verify) == :verify_none
-    end
-  end
 end

test/microwaveprop/prom_ex_test.exs

@@ -16,5 +16,18 @@ defmodule Microwaveprop.PromExTest do
       assert Microwaveprop.PromEx.InstrumentPlugin in plugins
       assert PromEx.Plugins.Beam in plugins
     end
+
+    test "uses the local Phoenix plugin fork instead of the built-in" do
+      plugins = Microwaveprop.PromEx.plugins()
+
+      plugin_modules =
+        Enum.map(plugins, fn
+          {mod, _opts} when is_atom(mod) -> mod
+          mod when is_atom(mod) -> mod
+        end)
+
+      refute PromEx.Plugins.Phoenix in plugin_modules
+      assert Microwaveprop.PromEx.Plugins.Phoenix in plugin_modules
+    end
   end
 end