# Findings — Bugs & Improvements (Non-Critical)

## Bugs

### 🟡 Medium

**All medium findings have been fixed.**

### 🟢 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) |

---

## Improvements

### 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 |

### Test Coverage Gaps

| Module | Status |
|--------|--------|
| `lib/microwaveprop/ionosphere.ex` | **Untested** |
| `lib/microwaveprop/mailer.ex` | **Untested** |
| `lib/microwaveprop/repo.ex` | **Untested** |
| `lib/microwaveprop/space_weather.ex` | **Untested** |
| `about_live.ex` | Real DB query logic, **untested** |

### 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 |

### 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) |