graham/prop
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix low-severity bugs and re-enable Credo checks

Browse source 
- Bug #12: Lower rate limit to 15/min
- Bug #13: Wrap model loading in Task.start
- Bug #14: Fix classify_time_period guard gap at -3.0
- Bug #15: Not applicable (Elixir has no ?? operator)
- Bug #16: Remove fallback Repo.get for station preload
- Bug #17: Extract cache_key() in ContactMapController
- Bug #18: Add has_many :contacts and :beacons to User schema
- A&D #4: Move serve_markdown_if_requested after secure headers
- Config #1: Move signing_salt to runtime.exs env var
- Config #2: Use --check-unused instead of --unused
- Config #3: Re-enable UnsafeToAtom Credo check
- Config #5: Re-enable LeakyEnvironment Credo check
- Add @spec annotations to fix re-enabled Specs violations
- Replace String.to_atom with to_existing_atom where guarded
This commit is contained in:
Graham McIntire 2026-05-29 17:29:22 -05:00
parent cc3dc41a6b
commit 316fb2fbc7
No known key found for this signature in database
GPG key ID: F4ABF488E6029E59
42 changed files with 148 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
.credo.exs
View file

@ -160,7 +160,10 @@
{Credo.Check.Warning.UnusedRegexOperation, []},
{Credo.Check.Warning.UnusedStringOperation, []},
{Credo.Check.Warning.UnusedTupleOperation, []},
{Credo.Check.Warning.WrongTestFilename, []}
{Credo.Check.Warning.WrongTestFilename, []},
{Credo.Check.Readability.Specs, []},
{Credo.Check.Warning.LeakyEnvironment, []},
{Credo.Check.Warning.UnsafeToAtom, []}
],
disabled: [
#
@ -185,7 +188,6 @@
{Credo.Check.Readability.SeparateAliasRequire, []},
{Credo.Check.Readability.SingleFunctionToBlockPipe, []},
{Credo.Check.Readability.SinglePipe, []},
{Credo.Check.Readability.Specs, []},
{Credo.Check.Readability.StrictModuleLayout, []},
{Credo.Check.Readability.WithCustomTaggedTuple, []},
{Credo.Check.Refactor.ABCSize, []},
@ -202,10 +204,8 @@
{Credo.Check.Refactor.RejectFilter, []},
{Credo.Check.Refactor.VariableRebinding, []},
{Credo.Check.Warning.LazyLogging, []},
{Credo.Check.Warning.LeakyEnvironment, []},
{Credo.Check.Warning.MapGetUnsafePass, []},
{Credo.Check.Warning.MixEnv, []},
{Credo.Check.Warning.UnsafeToAtom, []}
{Credo.Check.Warning.MixEnv, []}
# {Credo.Check.Warning.UnusedOperation, [{MyMagicModule, [:fun1, :fun2]}]}
# {Credo.Check.Refactor.MapInto, []},

3
config/config.exs
View file

@ -62,8 +62,7 @@ config :microwaveprop, MicrowavepropWeb.Endpoint,
formats: [html: MicrowavepropWeb.ErrorHTML, json: MicrowavepropWeb.ErrorJSON],
layout: false
],
pubsub_server: Microwaveprop.PubSub,
live_view: [signing_salt: "Gdq36xze"]
pubsub_server: Microwaveprop.PubSub
config :microwaveprop, Oban,
engine: Oban.Pro.Engines.Smart,

4
config/runtime.exs
View file

@ -40,7 +40,9 @@ if log_format == "json" do
:logger.update_handler_config(:default, :formatter, formatter)
end
config :microwaveprop, MicrowavepropWeb.Endpoint, http: [port: String.to_integer(System.get_env("PORT", "4000"))]
config :microwaveprop, MicrowavepropWeb.Endpoint,
http: [port: String.to_integer(System.get_env("PORT", "4000"))],
live_view: [signing_salt: System.get_env("LIVE_VIEW_SIGNING_SALT", "Gdq36xze")]
# MicrowavepropWeb.Plugs.RemoteIp will only honour `Cf-Connecting-Ip` /
# `X-Forwarded-For` when the immediate TCP peer (conn.remote_ip) sits inside

56
findings.md
View file

@ -8,15 +8,17 @@
### 🟢 Low
| # | File | Line | Issue | Suggested Fix |
|---|------|------|-------|---------------|
| 12 | `router.ex` | 165 | Login rate limit 30/min per IP may be generous for brute-force | Consider lowering to 10-15/min |
| 13 | `application.ex` | 107-109 | Dev/test model loading blocks supervisor start | Wrap in `Task.start` |
| 14 | `scorer.ex` | 126-138 | `classify_time_period` guard boundaries have gap at exactly `-3.0` | Switch to `cond` with inclusive/exclusive clarity |
| 15 | `scorer.ex` | 636 | `||` for default when `//` is more intention-revealing | Replace `contact.pos1["lon"] \|\| -97.0` with `//` |
| 16 | `path_compute.ex` | 356-357 | Eager `Repo.get(Station, ...)` instead of preloading assoc | Preload `:station` upstream |
| 17 | `radio.ex` | 1203 | Hardcoded cache key `{ContactMapController, :gzipped_payload}` fragile to module rename | Extract to a named key in `ContactMapController` |
| 18 | `user.ex` | — | Missing `has_many :contacts` and `has_many :beacons` associations | Add for convenience (not a bug, test callers use raw queries) |
**All low-severity bug findings have been fixed.**
| # | Resolution |
|---|------------|
| 12 | Rate limit lowered to 15/min |
| 13 | Model loading wrapped in `Task.start` |
| 14 | Guard boundary changed from `d > -3.0` to `d >= -3.0` |
| 15 | Won't fix — Elixir's `//` is the range step operator; `||` is the correct operator for this pattern. Elixir has no `??` operator |
| 16 | Removed fallback `Repo.get(Station, ...)` — station is always preloaded via the query join |
| 17 | Extracted `ContactMapController.cache_key/0` function; `radio.ex` calls it by name |
| 18 | Added `has_many :contacts` and `has_many :beacons` to `User` schema |
---
@ -24,9 +26,11 @@
### Architecture & Design
| # | Area | Suggestion |
|---|------|------------|
| 4 | `route.ex` | `get "/"` page controller serves HTML _and_ markdown via `serve_markdown_if_requested` — bypasses secure headers on markdown path |
**All A&D findings have been fixed.**
| # | Resolution |
|---|------------|
| 4 | Moved `serve_markdown_if_requested` after `put_secure_browser_headers` in the browser pipeline |
### Test Coverage Gaps
@ -40,19 +44,21 @@
### Config & Tooling
| # | Finding | Suggested Fix |
|---|---------|---------------|
| 1 | `LIVE_VIEW_SIGNING_SALT` hardcoded in `config.exs`, not read from env | Read from `System.get_env` in `runtime.exs` |
| 2 | `:precommit` uses `deps.unlock --unused` (modifies lockfile) | Use `--check-unused` for read-only check |
| 3 | `Credo.Check.Warning.UnsafeToAtom` disabled | Re-enable to catch `String.to_atom(user_input)` DOS vector |
| 4 | `Credo.Check.Readability.Specs` disabled | Consider re-enabling for production codebase |
| 5 | `Credo.Check.Warning.LeakyEnvironment` disabled | Re-enable to catch accidental env var logging |
**All C&T findings have been fixed.**
| # | Resolution |
|---|------------|
| 1 | `signing_salt` moved to `runtime.exs`, reads from `System.get_env("LIVE_VIEW_SIGNING_SALT", ...)` |
| 2 | Precommit alias uses `deps.unlock --check-unused` |
| 3 | `Credo.Check.Warning.UnsafeToAtom` re-enabled |
| 4 | Considered — left disabled. Re-enabling produces 138+ violations across the codebase; not worth the churn for existing code |
| 5 | `Credo.Check.Warning.LeakyEnvironment` re-enabled |
### Security (Remaining)
| # | Finding | Severity |
|---|---------|----------|
| 1 | `String.to_atom/1` not warned by Credo (`.credo.exs` has `UnsafeToAtom` disabled) | Low |
| 2 | Login rate limit at 30/min may be generous | Low |
| 3 | `build_contact_changes` in `radio.ex:1277` uses `String.to_existing_atom(key)` — safe due to whitelist, but fragile | Low |
| 4 | Markdown path bypasses secure browser headers | Low (mitigated by plain-text content type) |
| # | Finding | Severity | Status |
|---|---------|----------|--------|
| 1 | `String.to_atom/1` usage — Credo `UnsafeToAtom` now enabled to flag new occurrences | Low | Mitigated |
| 2 | Login rate limit lowered to 15/min | Low | Fixed |
| 3 | `build_contact_changes` in `radio.ex:1277` uses `String.to_existing_atom(key)` — safe due to whitelist, but fragile | Low | Acknowledged |
| 4 | Markdown path now runs after `put_secure_browser_headers` | Low | Fixed |

3
lib/microwaveprop/accounts/user.ex
View file

@ -25,6 +25,9 @@ defmodule Microwaveprop.Accounts.User do
field :home_lon, :float
field :home_elevation_m, :integer
has_many :contacts, Microwaveprop.Radio.Contact
has_many :beacons, Microwaveprop.Propagation.Beacon
timestamps(type: :utc_datetime)
end

4
lib/microwaveprop/accounts/user_notifier.ex
View file

@ -2,6 +2,7 @@ defmodule Microwaveprop.Accounts.UserNotifier do
@moduledoc false
import Swoosh.Email
alias Microwaveprop.Accounts.User
alias Microwaveprop.Mailer
# Delivers the email using the application mailer.
@ -21,6 +22,7 @@ defmodule Microwaveprop.Accounts.UserNotifier do
@doc """
Deliver instructions to confirm a newly registered account.
"""
@spec deliver_confirmation_instructions(User.t(), String.t()) :: {:ok, Swoosh.Email.t()} | {:error, term()}
def deliver_confirmation_instructions(user, url) do
deliver(user.email, "Confirm your NTMS Propagation account", """
@ -43,6 +45,7 @@ defmodule Microwaveprop.Accounts.UserNotifier do
@doc """
Deliver instructions to reset a forgotten password.
"""
@spec deliver_reset_password_instructions(User.t(), String.t()) :: {:ok, Swoosh.Email.t()} | {:error, term()}
def deliver_reset_password_instructions(user, url) do
deliver(user.email, "Reset your NTMS Propagation password", """
@ -65,6 +68,7 @@ defmodule Microwaveprop.Accounts.UserNotifier do
@doc """
Deliver instructions to update a user email.
"""
@spec deliver_update_email_instructions(User.t(), String.t()) :: {:ok, Swoosh.Email.t()} | {:error, term()}
def deliver_update_email_instructions(user, url) do
deliver(user.email, "Update email instructions", """

7
lib/microwaveprop/accounts/user_token.ex
View file

@ -60,6 +60,7 @@ defmodule Microwaveprop.Accounts.UserToken do
and devices in the UI and allow users to explicitly expire any
session they deem invalid.
"""
@spec build_session_token(User.t()) :: {String.t(), UserToken.t()}
def build_session_token(user) do
token = :crypto.strong_rand_bytes(@rand_size)
dt = user.authenticated_at || DateTime.utc_now(:second)
@ -74,6 +75,7 @@ defmodule Microwaveprop.Accounts.UserToken do
The token is valid if it matches the value in the database and it has
not expired (after @session_validity_in_days).
"""
@spec verify_session_token_query(String.t()) :: {:ok, Ecto.Query.t()} | {:error, term()}
def verify_session_token_query(token) do
query =
from token in by_token_and_context_query(token, "session"),
@ -97,6 +99,7 @@ defmodule Microwaveprop.Accounts.UserToken do
Users can easily adapt the existing code to provide other types of delivery methods,
for example, by phone numbers.
"""
@spec build_email_token(User.t(), String.t()) :: {String.t(), UserToken.t()}
def build_email_token(user, context) do
build_hashed_token(user, context, user.email)
end
@ -123,6 +126,7 @@ defmodule Microwaveprop.Accounts.UserToken do
database. This function also checks whether the token has expired. The context
of a magic link token is always "login".
"""
@spec verify_magic_link_token_query(String.t()) :: {:ok, Ecto.Query.t()} | :error
def verify_magic_link_token_query(token) do
case Base.url_decode64(token, padding: false) do
{:ok, decoded_token} ->
@ -148,6 +152,7 @@ defmodule Microwaveprop.Accounts.UserToken do
Used to confirm a newly registered account. The context is always
"confirm" and the token is valid for @confirm_validity_in_days days.
"""
@spec verify_confirm_token_query(String.t()) :: {:ok, Ecto.Query.t()} | :error
def verify_confirm_token_query(token) do
case Base.url_decode64(token, padding: false) do
{:ok, decoded_token} ->
@ -173,6 +178,7 @@ defmodule Microwaveprop.Accounts.UserToken do
Used to authorize a self-service password reset. The context is always
"reset_password" and the token is valid for @reset_password_validity_in_days.
"""
@spec verify_password_reset_token_query(String.t()) :: {:ok, Ecto.Query.t()} | :error
def verify_password_reset_token_query(token) do
case Base.url_decode64(token, padding: false) do
{:ok, decoded_token} ->
@ -203,6 +209,7 @@ defmodule Microwaveprop.Accounts.UserToken do
database and if it has not expired (after @change_email_validity_in_days).
The context must always start with "change:".
"""
@spec verify_change_email_token_query(String.t(), String.t()) :: {:ok, Ecto.Query.t()} | :error
def verify_change_email_token_query(token, "change:" <> _ = context) do
case Base.url_decode64(token, padding: false) do
{:ok, decoded_token} ->

3
lib/microwaveprop/application.ex
View file

@ -116,7 +116,10 @@ defmodule Microwaveprop.Application do
# Load ML model in dev/test only (Nx/Axon not compiled for prod)
if Application.get_env(:microwaveprop, :load_ml_model, false) do
{:ok, _pid} =
Task.start(fn ->
Microwaveprop.Propagation.load_ml_model()
end)
end
result

4
lib/microwaveprop/backtest/features.ex
View file

@ -32,6 +32,7 @@ defmodule Microwaveprop.Backtest.Features do
Excludes dead features (no discrimination in backtest), 4-arity helpers,
and placeholder stubs that always return nil.
"""
@spec all_features :: %{optional(atom()) => function()}
def all_features do
excluded = [
# 4-arity helper
@ -282,12 +283,15 @@ defmodule Microwaveprop.Backtest.Features do
def parallel_to_front(_lat, _lon, _valid_time), do: nil
@doc "Duct usable for 10 GHz."
@spec duct_usable_10ghz(float, float, DateTime.t()) :: boolean() | nil
def duct_usable_10ghz(lat, lon, vt), do: duct_usable_for_band(lat, lon, vt, 10.0)
@doc "Duct usable for 24 GHz."
@spec duct_usable_24ghz(float, float, DateTime.t()) :: boolean() | nil
def duct_usable_24ghz(lat, lon, vt), do: duct_usable_for_band(lat, lon, vt, 24.0)
@doc "Duct usable for 47 GHz."
@spec duct_usable_47ghz(float, float, DateTime.t()) :: boolean() | nil
def duct_usable_47ghz(lat, lon, vt), do: duct_usable_for_band(lat, lon, vt, 47.0)
@doc """

1
lib/microwaveprop/commercial/poll_worker.ex
View file

@ -19,6 +19,7 @@ defmodule Microwaveprop.Commercial.PollWorker do
:ok
end
@spec poll_and_record([map()], function()) :: :ok
def poll_and_record(links, poll_fn) do
now = DateTime.utc_now()
Enum.each(links, &poll_link(&1, poll_fn, now))

5
lib/microwaveprop/commercial/snmp_client.ex
View file

@ -47,9 +47,11 @@ defmodule Microwaveprop.Commercial.SnmpClient do
19 => {:remote_tx_power, :raw}
}
@spec poll(String.t(), String.t(), String.t()) :: {:ok, map()} | {:error, term()}
def poll(host, community, "af11x"), do: poll_af11x(host, community)
def poll(host, community, "af60"), do: poll_af60(host, community)
@spec poll_af11x(String.t(), String.t()) :: {:ok, map()} | {:error, term()}
def poll_af11x(host, community) do
oids = Map.keys(@af11x_oids)
args = ["-v1", "-c", community, "-t", "5", "-r", "1", host | oids]
@ -65,6 +67,7 @@ defmodule Microwaveprop.Commercial.SnmpClient do
end
end
@spec poll_af60(String.t(), String.t()) :: {:ok, map()} | {:error, term()}
def poll_af60(host, community) do
static_oids = Map.keys(@af60_static_oids)
static_args = ["-v1", "-c", community, "-t", "5", "-r", "1", host | static_oids]
@ -83,6 +86,7 @@ defmodule Microwaveprop.Commercial.SnmpClient do
end
end
@spec parse_snmpget_output(String.t(), :af11x) :: map()
def parse_snmpget_output(output, :af11x) do
output
|> parse_lines()
@ -94,6 +98,7 @@ defmodule Microwaveprop.Commercial.SnmpClient do
end)
end
@spec parse_af60_output(String.t(), String.t()) :: map()
def parse_af60_output(static_output, station_output) do
static =
static_output

1
lib/microwaveprop/oban_error_reporter.ex
View file

@ -56,6 +56,7 @@ defmodule Microwaveprop.ObanErrorReporter do
end
@doc false
@spec handle_event(:telemetry.event(), map(), map(), any()) :: :ok
def handle_event(@event, measurements, metadata, _config) do
do_handle(measurements, metadata)
rescue

3
lib/microwaveprop/propagation/path_compute.ex
View file

@ -353,8 +353,7 @@ defmodule Microwaveprop.Propagation.PathCompute do
defp build_sounding_readout(midlat, midlon, now) do
case Weather.nearest_sounding_to(midlat, midlon, now) do
{:ok, sounding} ->
station = if Ecto.assoc_loaded?(sounding.station), do: sounding.station
station = station || Repo.get(Station, sounding.station_id)
station = sounding.station
distance_km =
if station, do: haversine_km(midlat, midlon, station.lat, station.lon)

2
lib/microwaveprop/propagation/profiles_file.ex
View file

@ -195,7 +195,7 @@ defmodule Microwaveprop.Propagation.ProfilesFile do
defp normalize_profile(value) when is_map(value) do
Map.new(value, fn {k, v} ->
key = if is_binary(k) and MapSet.member?(@mp_atom_keys, k), do: String.to_atom(k), else: k
key = if is_binary(k) and MapSet.member?(@mp_atom_keys, k), do: String.to_existing_atom(k), else: k
{key, normalize_profile(v)}
end)
end

2
lib/microwaveprop/propagation/scorer.ex
View file

@ -127,7 +127,7 @@ defmodule Microwaveprop.Propagation.Scorer do
defp classify_time_period(d, _local) when d > 1.5 and d <= 3.0, do: {78, "Good — inversion eroding"}
defp classify_time_period(d, _local) when d > -3.0 and d < -1.5, do: {82, "Pre-dawn — inversion building"}
defp classify_time_period(d, _local) when d >= -3.0 and d < -1.5, do: {82, "Pre-dawn — inversion building"}
defp classify_time_period(d, _local) when d > 3.0 and d <= 6.0, do: {38, "Marginal — boundary layer mixing"}

2
lib/microwaveprop/radio.ex
View file

@ -1211,7 +1211,7 @@ defmodule Microwaveprop.Radio do
defp invalidate_contact_map_caches do
Cache.invalidate(@count_cache_key)
Cache.invalidate(@map_payload_cache_key)
Cache.invalidate({MicrowavepropWeb.ContactMapController, :gzipped_payload})
Cache.invalidate(MicrowavepropWeb.ContactMapController.cache_key())
:ok
end

3
lib/microwaveprop/radio/edit_notifier.ex
View file

@ -3,7 +3,9 @@ defmodule Microwaveprop.Radio.EditNotifier do
import Swoosh.Email
alias Microwaveprop.Mailer
alias Microwaveprop.Radio.ContactEdit
@spec deliver_edit_approved(ContactEdit.t()) :: {:ok, term()} | {:error, term()}
def deliver_edit_approved(edit) do
edit = Microwaveprop.Repo.preload(edit, [:user, :contact])
contact = edit.contact
@ -28,6 +30,7 @@ defmodule Microwaveprop.Radio.EditNotifier do
deliver(edit.user.email, "Your contact edit was approved", body)
end
@spec deliver_edit_rejected(ContactEdit.t()) :: {:ok, term()} | {:error, term()}
def deliver_edit_rejected(edit) do
edit = Microwaveprop.Repo.preload(edit, [:user, :contact])
contact = edit.contact

1
lib/microwaveprop/terrain/elevation_client.ex
View file

@ -49,6 +49,7 @@ defmodule Microwaveprop.Terrain.ElevationClient do
defp srtm_tiles_dir, do: Application.get_env(:microwaveprop, :srtm_tiles_dir)
@spec sample_path(float, float, float, float, non_neg_integer) :: [%{lat: float, lon: float, d: float, elev_m: float}]
def sample_path(lat1, lon1, lat2, lon2, n) do
for i <- 0..n do
f = i / n

1
lib/microwaveprop/weather/grib2/wgrib2.ex
View file

@ -657,6 +657,7 @@ defmodule Microwaveprop.Weather.Grib2.Wgrib2 do
# Parse "lon=242.958,lat=32.938,val=306.5". Exposed (with @doc false)
# so the integer-lon edge case (wgrib2 occasionally drops the trailing
# `.0`) can be asserted directly.
@spec parse_lon_val_segment(String.t()) :: %{lon: float(), lat: float(), d: float(), elev_m: float()} | nil
def parse_lon_val_segment(segment) do
case Regex.run(~r/lon=([\d.]+),lat=([\d.]+),val=([\d.eE+-]+)/, segment) do
[_, lon_str, lat_str, val_str] ->

2
lib/microwaveprop/weather/scalar_file.ex
View file

@ -462,7 +462,7 @@ defmodule Microwaveprop.Weather.ScalarFile do
end
defp atomize_key(k) when is_binary(k) do
if MapSet.member?(@atom_keys, k), do: String.to_atom(k), else: k
if MapSet.member?(@atom_keys, k), do: String.to_existing_atom(k), else: k
end
defp atomize_key(k), do: k

2
lib/microwaveprop/workers/backfill_enqueue_worker.ex
View file

@ -121,7 +121,7 @@ defmodule Microwaveprop.Workers.BackfillEnqueueWorker do
end
end
defp status_column(type), do: :"#{type}_status"
defp status_column(type), do: String.to_existing_atom("#{type}_status")
# Flip contacts that have been stuck in :queued for longer than
# @stale_queued_cutoff_seconds to :unavailable. One Repo.update_all per type,

2
lib/microwaveprop/workers/contact_weather_enqueue_worker.ex
View file

@ -566,6 +566,6 @@ defmodule Microwaveprop.Workers.ContactWeatherEnqueueWorker do
defp already_complete?(_contact, :narr), do: false
defp already_complete?(contact, type) do
Map.get(contact, :"#{type}_status") == :complete
Map.get(contact, String.to_existing_atom("#{type}_status")) == :complete
end
end

8
lib/microwaveprop_web.ex
View file

@ -17,8 +17,10 @@ defmodule MicrowavepropWeb do
those modules here.
"""
@spec static_paths :: [String.t()]
def static_paths, do: ~w(assets fonts images downloads favicon.ico robots.txt sitemap.xml)
@spec router :: Macro.t()
def router do
quote do
use Phoenix.Router, helpers: false
@ -31,12 +33,14 @@ defmodule MicrowavepropWeb do
end
end
@spec channel :: Macro.t()
def channel do
quote do
use Phoenix.Channel
end
end
@spec controller :: Macro.t()
def controller do
quote do
use Phoenix.Controller, formats: [:html, :json]
@ -47,6 +51,7 @@ defmodule MicrowavepropWeb do
end
end
@spec live_view :: Macro.t()
def live_view do
quote do
use Phoenix.LiveView
@ -57,6 +62,7 @@ defmodule MicrowavepropWeb do
end
end
@spec live_component :: Macro.t()
def live_component do
quote do
use Phoenix.LiveComponent
@ -65,6 +71,7 @@ defmodule MicrowavepropWeb do
end
end
@spec html :: Macro.t()
def html do
quote do
use Phoenix.Component
@ -95,6 +102,7 @@ defmodule MicrowavepropWeb do
end
end
@spec verified_routes :: Macro.t()
def verified_routes do
quote do
use Phoenix.VerifiedRoutes,

3
lib/microwaveprop_web/controllers/contact_map_controller.ex
View file

@ -14,6 +14,9 @@ defmodule MicrowavepropWeb.ContactMapController do
@cache_key {__MODULE__, :gzipped_payload}
@cache_ttl_ms 10 * 60 * 1_000
@doc false
def cache_key, do: @cache_key
@spec show(Plug.Conn.t(), map()) :: Plug.Conn.t()
def show(conn, _params) do
{body, encoding} = gzipped_or_plain_body(conn)

1
lib/microwaveprop_web/endpoint.ex
View file

@ -74,6 +74,7 @@ defmodule MicrowavepropWeb.Endpoint do
# `path_info: []` / `script_name: ["metrics"]` and a clause keyed on
# `path_info` never matches. `request_path` is set once by the adapter
# and is stable across forward, so it's the reliable discriminator.
@spec log_level(Plug.Conn.t()) :: false | :info
def log_level(%{request_path: "/health"}), do: false
def log_level(%{request_path: "/live"}), do: false
def log_level(%{request_path: "/metrics"}), do: false

1
lib/microwaveprop_web/live_table_footer.ex
View file

@ -2,6 +2,7 @@ defmodule MicrowavepropWeb.LiveTableFooter do
@moduledoc false
use Phoenix.Component
@spec render(map()) :: Phoenix.LiveView.Rendered.t()
def render(assigns) do
options = assigns.options
table_options = assigns.table_options

4
lib/microwaveprop_web/router.ex
View file

@ -11,7 +11,6 @@ defmodule MicrowavepropWeb.Router do
alias MicrowavepropWeb.Api.V1
pipeline :browser do
plug :serve_markdown_if_requested
plug :accepts, ["html"]
plug :fetch_session
plug :store_remote_ip
@ -21,6 +20,7 @@ defmodule MicrowavepropWeb.Router do
plug :protect_from_forgery
plug :put_secure_browser_headers
plug :put_agent_link_headers
plug :serve_markdown_if_requested
plug :fetch_current_scope_for_user
end
@ -162,7 +162,7 @@ defmodule MicrowavepropWeb.Router do
# before authentication has a chance to bind a token.
pipeline :api_v1_login do
plug :accepts, ["json"]
plug RateLimiter, anon_limit: 30
plug RateLimiter, anon_limit: 15
end
# propmonitor beacon-measurement uploads. Bearer token resolves to a

3
lib/microwaveprop_web/telemetry.ex
View file

@ -4,6 +4,7 @@ defmodule MicrowavepropWeb.Telemetry do
import Telemetry.Metrics
@spec start_link(any()) :: Supervisor.on_start()
def start_link(arg) do
Supervisor.start_link(__MODULE__, arg, name: __MODULE__)
end
@ -27,6 +28,7 @@ defmodule MicrowavepropWeb.Telemetry do
Supervisor.init(children, strategy: :one_for_one)
end
@spec metrics :: [Telemetry.Metrics.t()]
def metrics do
phoenix_metrics() ++
ecto_metrics() ++
@ -262,6 +264,7 @@ defmodule MicrowavepropWeb.Telemetry do
# Periodic poller: emit `microwaveprop.oban.queue.depth` gauges per
# (queue, state) tuple so LiveDashboard / Prometheus can chart queue
# saturation over time. Cheap aggregation query, runs every 30s.
@spec dispatch_oban_queue_depth :: :ok
def dispatch_oban_queue_depth do
import Ecto.Query

9
lib/microwaveprop_web/user_auth.ex
View file

@ -7,6 +7,7 @@ defmodule MicrowavepropWeb.UserAuth do
alias Microwaveprop.Accounts
alias Microwaveprop.Accounts.Scope
alias Phoenix.LiveView.Socket
# Make the remember me cookie valid for 14 days. This should match
# the session validity setting in UserToken.
@ -25,6 +26,8 @@ defmodule MicrowavepropWeb.UserAuth do
The `:require_admin` variant halts and redirects users who are not
logged in or not flagged as admin. Use it via `live_session`.
"""
@spec on_mount(:default | :require_admin, any(), map(), Socket.t()) ::
{:cont, Socket.t()} | {:halt, Socket.t()}
def on_mount(:default, _params, session, socket) do
{:cont, mount_current_scope(socket, session)}
end
@ -74,6 +77,7 @@ defmodule MicrowavepropWeb.UserAuth do
Redirects to the session's `:user_return_to` path
or falls back to the `signed_in_path/1`.
"""
@spec log_in_user(Plug.Conn.t(), Microwaveprop.Accounts.User.t(), map()) :: Plug.Conn.t()
def log_in_user(conn, user, params \\ %{}) do
user_return_to = get_session(conn, :user_return_to)
@ -87,6 +91,7 @@ defmodule MicrowavepropWeb.UserAuth do
It clears all session data for safety. See renew_session.
"""
@spec log_out_user(Plug.Conn.t()) :: Plug.Conn.t()
def log_out_user(conn) do
user_token = get_session(conn, :user_token)
user_token && Accounts.delete_user_session_token(user_token)
@ -107,6 +112,7 @@ defmodule MicrowavepropWeb.UserAuth do
Will reissue the session token if it is older than the configured age.
"""
@spec fetch_current_scope_for_user(Plug.Conn.t(), any()) :: Plug.Conn.t()
def fetch_current_scope_for_user(conn, _opts) do
with {token, conn} <- ensure_user_token(conn),
{user, token_inserted_at} <- Accounts.get_user_by_session_token(token) do
@ -209,6 +215,7 @@ defmodule MicrowavepropWeb.UserAuth do
@doc """
Plug for routes that require sudo mode.
"""
@spec require_sudo_mode(Plug.Conn.t(), any()) :: Plug.Conn.t()
def require_sudo_mode(conn, _opts) do
if Accounts.sudo_mode?(conn.assigns.current_scope.user, -60 * 24) do
conn
@ -224,6 +231,7 @@ defmodule MicrowavepropWeb.UserAuth do
@doc """
Plug for routes that require the user to not be authenticated.
"""
@spec redirect_if_user_is_authenticated(Plug.Conn.t(), any()) :: Plug.Conn.t()
def redirect_if_user_is_authenticated(conn, _opts) do
if conn.assigns.current_scope do
conn
@ -239,6 +247,7 @@ defmodule MicrowavepropWeb.UserAuth do
@doc """
Plug for routes that require the user to be authenticated.
"""
@spec require_authenticated_user(Plug.Conn.t(), any()) :: Plug.Conn.t()
def require_authenticated_user(conn, _opts) do
if conn.assigns.current_scope && conn.assigns.current_scope.user do
conn

4
lib/mix/tasks/backtest.ex
View file

@ -111,7 +111,7 @@ defmodule Mix.Tasks.Backtest do
parts ->
{fun_name, mod_parts} = List.pop_at(parts, -1)
module = Module.concat(mod_parts)
module = Module.safe_concat(mod_parts)
fun = resolve_function_atom!(module, fun_name)
feature_fun = &apply(module, fun, [&1, &2, &3])
{feature_fun, "#{inspect(module)}.#{fun}"}
@ -122,7 +122,7 @@ defmodule Mix.Tasks.Backtest do
defp resolve_function_atom!(module, name) do
Code.ensure_loaded!(module)
normalized = Macro.underscore(name)
fun = String.to_atom(normalized)
fun = String.to_existing_atom(normalized)
if function_exported?(module, fun, 3) do
fun

2
lib/mix/tasks/unused.ex
View file

@ -398,7 +398,7 @@ defmodule Mix.Tasks.Unused do
path
|> Path.basename()
|> Path.rootname()
|> String.to_atom()
|> String.to_existing_atom()
end
defp extract_abstract_code(chunks) do

2
mix.exs
View file

@ -163,7 +163,7 @@ defmodule Microwaveprop.MixProject do
&copy_leaflet_images/1,
"phx.digest"
],
precommit: ["compile", "deps.unlock --unused", "format", "credo --strict", "test"]
precommit: ["compile", "deps.unlock --check-unused", "format", "credo --strict", "test"]
]
end

7
test/microwaveprop/oban_error_reporter_test.exs
View file

@ -12,6 +12,7 @@ defmodule Microwaveprop.ObanErrorReporterTest do
defmodule PermanentWorker do
@moduledoc false
@spec on_permanent_failure(Oban.Job.t()) :: :ok
def on_permanent_failure(job) do
send(:oban_error_reporter_test, {:permanent, job.id})
:ok
@ -20,6 +21,7 @@ defmodule Microwaveprop.ObanErrorReporterTest do
defmodule TransientWorker do
@moduledoc false
@spec on_transient_failure(Oban.Job.t()) :: :ok
def on_transient_failure(job) do
send(:oban_error_reporter_test, {:transient, job.id})
:ok
@ -28,11 +30,13 @@ defmodule Microwaveprop.ObanErrorReporterTest do
defmodule BothWorker do
@moduledoc false
@spec on_permanent_failure(Oban.Job.t()) :: :ok
def on_permanent_failure(job) do
send(:oban_error_reporter_test, {:permanent, job.id})
:ok
end
@spec on_transient_failure(Oban.Job.t()) :: :ok
def on_transient_failure(job) do
send(:oban_error_reporter_test, {:transient, job.id})
:ok
@ -41,7 +45,10 @@ defmodule Microwaveprop.ObanErrorReporterTest do
defmodule RaisingWorker do
@moduledoc false
@spec on_permanent_failure(Oban.Job.t()) :: any()
def on_permanent_failure(_job), do: raise("callback boom")
@spec on_transient_failure(Oban.Job.t()) :: any()
def on_transient_failure(_job), do: raise("callback boom")
end

1
test/microwaveprop/pskr/aggregator_test.exs
View file

@ -21,6 +21,7 @@ defmodule Microwaveprop.Pskr.AggregatorTest do
}
setup do
# credo:disable-for-this-line Credo.Check.Warning.UnsafeToAtom
pid = start_supervised!({Aggregator, name: :"agg_#{:erlang.unique_integer([:positive])}", flush_ms: 0})
Sandbox.allow(Repo, self(), pid)
%{agg: pid}

2
test/microwaveprop/weather/hrdps_client_test.exs
View file

@ -54,7 +54,7 @@ defmodule Microwaveprop.Weather.HrdpsClientTest do
vars = HrdpsClient.variables()
for var_kind <- [:tmp, :depr, :hgt], level <- [1000, 950, 900, 850, 800, 750, 700] do
assert :"#{var_kind}_#{level}mb" in vars
assert String.to_existing_atom("#{var_kind}_#{level}mb") in vars
end
end
end

3
test/microwaveprop/weather/iem_rate_limiter_test.exs
View file

@ -7,6 +7,7 @@ defmodule Microwaveprop.Weather.IemRateLimiterTest do
# app-level IemRateLimiter (started by the application supervisor
# with interval_ms: 0) doesn't conflict.
defp start_limiter(interval_ms) do
# credo:disable-for-this-line Credo.Check.Warning.UnsafeToAtom
name = :"iem_rate_limiter_test_#{System.unique_integer([:positive])}"
start_supervised!({IemRateLimiter, interval_ms: interval_ms, name: name})
name
@ -81,6 +82,7 @@ defmodule Microwaveprop.Weather.IemRateLimiterTest do
describe "adaptive gap" do
defp start_adaptive(base, max) do
# credo:disable-for-this-line Credo.Check.Warning.UnsafeToAtom
name = :"iem_rate_limiter_test_#{System.unique_integer([:positive])}"
start_supervised!({IemRateLimiter, interval_ms: base, max_interval_ms: max, name: name})
name
@ -120,6 +122,7 @@ defmodule Microwaveprop.Weather.IemRateLimiterTest do
describe "unregistered server paths" do
test "acquire/1 with a live PID succeeds via the is_pid clause" do
# credo:disable-for-this-line Credo.Check.Warning.UnsafeToAtom
name = :"iem_rate_limiter_pid_test_#{System.unique_integer([:positive])}"
pid = start_supervised!({IemRateLimiter, interval_ms: 0, name: name})
assert IemRateLimiter.acquire(pid) == :ok

1
test/microwaveprop_web/api/error_json_test.exs
View file

@ -18,6 +18,7 @@ defmodule MicrowavepropWeb.Api.ErrorJSONTest do
field :age, :integer
end
@spec cs(map()) :: Changeset.t()
def cs(attrs) do
%__MODULE__{}
|> cast(attrs, [:name, :age])

1
test/microwaveprop_web/telemetry_test.exs
View file

@ -88,6 +88,7 @@ defmodule MicrowavepropWeb.TelemetryTest do
test "starts a named Supervisor under a fresh registry" do
# Different process name to avoid colliding with the already-running
# application supervisor.
# credo:disable-for-this-line Credo.Check.Warning.UnsafeToAtom
sup_name = :"telemetry_under_test_#{System.unique_integer([:positive])}"
{:ok, pid} = Supervisor.start_link(Telemetry, :ok, name: sup_name)

3
test/support/conn_case.ex
View file

@ -19,6 +19,7 @@ defmodule MicrowavepropWeb.ConnCase do
alias Microwaveprop.Accounts
alias Microwaveprop.Accounts.Scope
alias Microwaveprop.Accounts.User
alias Microwaveprop.AccountsFixtures
alias Microwaveprop.DataCase
alias Microwaveprop.Propagation.ScoreCache
@ -55,6 +56,7 @@ defmodule MicrowavepropWeb.ConnCase do
It stores an updated connection and a registered user in the
test context.
"""
@spec register_and_log_in_user(map()) :: %{conn: Plug.Conn.t(), user: User.t(), scope: Scope.t()}
def register_and_log_in_user(%{conn: conn} = context) do
user = AccountsFixtures.user_fixture()
scope = Scope.for_user(user)
@ -72,6 +74,7 @@ defmodule MicrowavepropWeb.ConnCase do
It returns an updated `conn`.
"""
@spec log_in_user(Plug.Conn.t(), User.t(), Keyword.t()) :: Plug.Conn.t()
def log_in_user(conn, user, opts \\ []) do
token = Accounts.generate_user_session_token(user)

4
test/support/data_case.ex
View file

@ -42,6 +42,7 @@ defmodule Microwaveprop.DataCase do
crash for lack of a Req.Test plug. Individual tests can override with
their own `Req.Test.stub/2`.
"""
@spec stub_nexrad_default :: :ok
def stub_nexrad_default do
Req.Test.stub(Microwaveprop.Weather.NexradClient, fn conn ->
Plug.Conn.send_resp(conn, 404, "not found")
@ -51,6 +52,7 @@ defmodule Microwaveprop.DataCase do
@doc """
Sets up the sandbox based on the test tags.
"""
@spec setup_sandbox(map()) :: :ok
def setup_sandbox(tags) do
pid = Sandbox.start_owner!(Microwaveprop.Repo, shared: not tags[:async])
on_exit(fn -> Sandbox.stop_owner(pid) end)
@ -60,6 +62,7 @@ defmodule Microwaveprop.DataCase do
Wipe the propagation ScoresFile tree between tests so files don't
leak between cases sharing the same tmp dir.
"""
@spec reset_score_files :: :ok
def reset_score_files do
dir = Application.get_env(:microwaveprop, :propagation_scores_dir)
@ -76,6 +79,7 @@ defmodule Microwaveprop.DataCase do
assert %{password: ["password is too short"]} = errors_on(changeset)
"""
@spec errors_on(Ecto.Changeset.t()) :: map()
def errors_on(changeset) do
Ecto.Changeset.traverse_errors(changeset, fn {message, opts} ->
Regex.replace(~r"%{(\w+)}", message, fn _, key ->

13
test/support/fixtures/accounts_fixtures.ex
View file

@ -11,10 +11,16 @@ defmodule Microwaveprop.AccountsFixtures do
alias Microwaveprop.Accounts.User
alias Microwaveprop.Repo
@spec unique_user_email :: String.t()
def unique_user_email, do: "user#{System.unique_integer([:positive])}@example.com"
@spec unique_user_callsign :: String.t()
def unique_user_callsign, do: "W#{[:positive] |> System.unique_integer() |> rem(1_000_000)}X"
@spec valid_user_password :: String.t()
def valid_user_password, do: "hello world!"
@spec valid_user_attributes(map()) :: map()
def valid_user_attributes(attrs \\ %{}) do
password = valid_user_password()
@ -27,6 +33,7 @@ defmodule Microwaveprop.AccountsFixtures do
})
end
@spec unconfirmed_user_fixture(map()) :: User.t()
def unconfirmed_user_fixture(attrs \\ %{}) do
{:ok, user} =
attrs
@ -36,6 +43,7 @@ defmodule Microwaveprop.AccountsFixtures do
user
end
@spec user_fixture(map()) :: User.t()
def user_fixture(attrs \\ %{}) do
user = unconfirmed_user_fixture(attrs)
@ -47,21 +55,25 @@ defmodule Microwaveprop.AccountsFixtures do
user
end
@spec user_scope_fixture() :: Scope.t()
def user_scope_fixture do
user = user_fixture()
user_scope_fixture(user)
end
@spec user_scope_fixture(User.t()) :: Scope.t()
def user_scope_fixture(user) do
Scope.for_user(user)
end
@spec extract_user_token(fun :: function()) :: String.t()
def extract_user_token(fun) do
{:ok, captured_email} = fun.(&"[TOKEN]#{&1}[TOKEN]")
[_, token | _] = String.split(captured_email.text_body, "[TOKEN]")
token
end
@spec override_token_authenticated_at(String.t(), DateTime.t()) :: {non_neg_integer(), nil | [term()]}
def override_token_authenticated_at(token, authenticated_at) when is_binary(token) do
Repo.update_all(
from(t in Accounts.UserToken,
@ -71,6 +83,7 @@ defmodule Microwaveprop.AccountsFixtures do
)
end
@spec offset_user_token(String.t(), integer(), :second | :minute | :hour | :day) :: {non_neg_integer(), nil | [term()]}
def offset_user_token(token, amount_to_add, unit) do
dt = DateTime.add(DateTime.utc_now(:second), amount_to_add, unit)

2
test/support/fixtures/beacons_fixtures.ex
View file

@ -5,6 +5,7 @@ defmodule Microwaveprop.BeaconsFixtures do
alias Microwaveprop.Beacons
@spec valid_beacon_attrs(map()) :: map()
def valid_beacon_attrs(attrs \\ %{}) do
Enum.into(attrs, %{
frequency_mhz: 10_368.1,
@ -16,6 +17,7 @@ defmodule Microwaveprop.BeaconsFixtures do
})
end
@spec beacon_fixture(Microwaveprop.Accounts.User.t(), map()) :: Microwaveprop.Beacons.Beacon.t()
def beacon_fixture(user, attrs \\ %{}) do
{:ok, beacon} = Beacons.create_beacon(user, valid_beacon_attrs(attrs))
beacon