prop/findings.md
Graham McIntire c722a77dcd
Add test coverage for Mailer and AboutLive
- Mailer: apply_defaults/1 sets From and Reply-To headers
- AboutLive: content rendering, empty stats, and contact count display
- Fix has_many :beacons association (wrong module path)
- Fix router pipeline order: serve_markdown before accepts,
  after secure headers so markdown responses get security headers
- Fix notify_listener_test Process.sleep regression
- Update findings.md test coverage gap status
2026-05-29 17:56:03 -05:00

64 lines
2.6 KiB
Markdown

# Findings — Bugs & Improvements (Non-Critical)
## Bugs
### 🟡 Medium
**All medium findings have been fixed.**
### 🟢 Low
**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 |
---
## Improvements
### Architecture & Design
**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
| Module | Status | Coverage |
|--------|--------|----------|
| `lib/microwaveprop/ionosphere.ex` | **Covered** | `upsert_observations/2`, `latest_observation/1`, `nearest_foes/3` tested |
| `lib/microwaveprop/mailer.ex` | **Covered** | `apply_defaults/1` tested |
| `lib/microwaveprop/repo.ex` | Not worth testing | Trivial `use Ecto.Repo` one-liner |
| `lib/microwaveprop/space_weather.ex` | **Covered** | `upsert_kp/1`, `upsert_solar_flux/1`, `upsert_xray/1`, `latest_kp/0`, `latest_f107/0`, `latest_xray/0` tested |
| `about_live.ex` | **Covered** | Content rendering and database stats display tested |
### Config & Tooling
**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 | 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 |