fix: validate tracked callsign before navigation #35

graham · 2026-09-15T06:32:26-05:00

graham commented

2026-09-15 06:32:26 -05:00

Fixes M5 from bugs.md.

handle_track_callsign/2 normalized the input but never validated it before push_patch("/#{callsign}"). A callsign containing / (the common W5ABC/M portable suffix), ?, # or % produced a path the router cannot match, so live_link_info!/3 raised and the LiveView died on a normal form submit. Because the bad callsign was assigned before the patch, it also poisoned every subsequent URL update.

Changes:

Route the non-empty branch through track_validated_callsign/2, which applies the same ParamUtils.valid_callsign?/1 contract as the callsign search box; invalid input gets an error flash and no navigation.
Empty input keeps its existing clear-tracking behavior.
push_patch now uses a verified ~p route, and track_validated_callsign only ever navigates for validated callsigns.

Tests: regression test in index_test.exs submits W5ABC/M, W5ABC?Q, and W5ABC#Q and asserts the view survives and a subsequent valid callsign still navigates.

Verification: focused callsign suites 170 passed, full suite 2236 passed, credo strict clean, dialyzer clean (43 pre-existing suppressed, no new warnings).

Fixes M5 from bugs.md. `handle_track_callsign/2` normalized the input but never validated it before `push_patch("/#{callsign}")`. A callsign containing `/` (the common `W5ABC/M` portable suffix), `?`, `#` or `%` produced a path the router cannot match, so `live_link_info!/3` raised and the LiveView died on a normal form submit. Because the bad callsign was assigned before the patch, it also poisoned every subsequent URL update. Changes: - Route the non-empty branch through `track_validated_callsign/2`, which applies the same `ParamUtils.valid_callsign?/1` contract as the callsign search box; invalid input gets an error flash and no navigation. - Empty input keeps its existing clear-tracking behavior. - `push_patch` now uses a verified `~p` route, and `track_validated_callsign` only ever navigates for validated callsigns. Tests: regression test in `index_test.exs` submits `W5ABC/M`, `W5ABC?Q`, and `W5ABC#Q` and asserts the view survives and a subsequent valid callsign still navigates. Verification: focused callsign suites 170 passed, full suite 2236 passed, credo strict clean, dialyzer clean (43 pre-existing suppressed, no new warnings).

graham added 1 commit

2026-09-15 06:32:26 -05:00

fix: validate tracked callsign before navigation

Elixir CI / Dialyzer (pull_request) Successful in 39s

Details

Elixir CI / Build and test (pull_request) Successful in 1m41s

Details

Elixir CI / Build and Push Docker Image (pull_request) Has been skipped

Details

skippy-bot/review Skippy review: 1 blocking finding open — see the PR thread

Details

b0ff0a5a54

skippy-bot reviewed

2026-09-15 06:35:07 -05:00

skippy-bot left a comment

🤖 Skippy PR review

1 finding — 1 blocking before merge.

Severity	Location	Issue
🟡 Warning	`lib/aprsme_web/live/map_live/events.ex:180`	Guard covers the form only — the URL entry point still poisons tracked_callsign

_{Reviewed b0ff0a5a54f6. Comment skippy review to re-run.}

### 🤖 Skippy PR review **1 finding** — 1 blocking before merge. | Severity | Location | Issue | | --- | --- | --- | | 🟡 Warning | `lib/aprsme_web/live/map_live/events.ex:180` | Guard covers the form only — the URL entry point still poisons tracked_callsign | <sub>Reviewed `b0ff0a5a54f6`. Comment `skippy review` to re-run.</sub>

lib/aprsme_web/live/map_live/events.ex

					
				@ -186,0 +177,4 @@

				  # and kills the view on a normal form submit — and assigning the bad callsign

				  # first would poison every later URL update. Same contract as the search box.

				  defp track_validated_callsign(callsign, socket) do

				    if ParamUtils.valid_callsign?(callsign) do

🟡 Warning — Guard covers the form only — the URL entry point still poisons tracked_callsign

Index.mount/3 (lib/aprsme_web/live/map_live/index.ex:59) assigns tracked_callsign straight from params["callsign"] || params["call"] with trim/upcase only, and Plug decodes query strings, so /?call=W5ABC/M (no encoding needed; /W5ABC%2FM also works) still puts "W5ABC/M" in the socket. The next update_url_with_current_state — trail-duration/hist change or clear-tracking, events.ex:417 — builds "/#{socket.assigns.tracked_callsign}?lat=…" as a plain string and push_patches it, no route matches two segments, live_link_info!/3 raises, and the view dies. Same crash this PR fixes, reached from a URL instead of the form (the comment here names that poison vector as in scope). Fix: validate in mount/3 before State.finalize — if ParamUtils.valid_callsign?(tracked_callsign), do: tracked_callsign, else: "" — so the bad value never reaches an assign.

**🟡 Warning — Guard covers the form only — the URL entry point still poisons tracked_callsign** `Index.mount/3` (`lib/aprsme_web/live/map_live/index.ex:59`) assigns `tracked_callsign` straight from `params["callsign"] || params["call"]` with trim/upcase only, and Plug decodes query strings, so `/?call=W5ABC/M` (no encoding needed; `/W5ABC%2FM` also works) still puts `"W5ABC/M"` in the socket. The next `update_url_with_current_state` — trail-duration/hist change or clear-tracking, `events.ex:417` — builds `"/#{socket.assigns.tracked_callsign}?lat=…"` as a plain string and `push_patch`es it, no route matches two segments, `live_link_info!/3` raises, and the view dies. Same crash this PR fixes, reached from a URL instead of the form (the comment here names that poison vector as in scope). Fix: validate in `mount/3` before `State.finalize` — `if ParamUtils.valid_callsign?(tracked_callsign), do: tracked_callsign, else: ""` — so the bad value never reaches an assign.

skippy-bot marked this conversation as resolved

graham referenced this pull request from a commit

2026-09-15 07:09:00 -05:00

fix: drop unroutable callsigns from URL params in mount

graham added 1 commit

2026-09-15 07:09:00 -05:00

fix: drop unroutable callsigns from URL params in mount

Elixir CI / Build and test (pull_request) Successful in 57s

Details

skippy-bot/review Skippy review: clean — no open findings

Details

Elixir CI / Dialyzer (pull_request) Successful in 2m49s

Details

Elixir CI / Build and Push Docker Image (pull_request) Has been skipped

Details

aced185d6b

PR #35 review: the form guard alone left the URL entry point open.
mount/3 assigned params["callsign"] || params["call"] after trim/upcase
only, so ?call=W5ABC/M (Plug decodes %2F variants too) still poisoned
tracked_callsign; the next update_url_with_current_state patched to
"/W5ABC/M?lat=…", no route matched two segments, live_link_info!/3
raised, and the view died.

mount now drops callsigns the router cannot embed as a single path
segment (containing /, ?, #, % or control chars) via the new
ParamUtils.routable_callsign?/1, so the bad value never reaches an
assign. Format validation intentionally stays in the track-callsign
form and the search box: mount must keep tracking routable callsigns
like TEST-STATUS that fail the callsign-format check (pinned by
TrackedCallsignOldPacketTest and CallsignViewTest).

Also removes five stale standalone section-marker comments in index.ex:
Styler mis-places them across the file whenever any edit forces a
rewrite, so mix format on any change to this file was unstable.

Verified: new mount regression tests fail pre-fix, pass post-fix; full
suite 2238 passed; mix format --check-formatted and credo --strict
clean.

skippy-bot commented

2026-09-15 07:11:07 -05:00

First-time contributor

Resolved 1 of 1 earlier finding — mount/3 now drops unroutable URL callsigns (routable_callsign?/1) before they reach an assign, so the /?call=W5ABC/M poison vector is closed. 0 still open.

Nothing new in b0ff0a5a..aced185d. I checked the guard's edges: values that stay a single path segment (.., spaces, raw invalid UTF-8 via %FF) still match /:callsign and don't raise in URI.parse/String.trim/upcase, so the new check is sufficient for the crash class it targets — the mount check being looser than the form's valid_callsign?/1 is deliberate and changes no failure path.

Resolved 1 of 1 earlier finding — `mount/3` now drops unroutable URL callsigns (`routable_callsign?/1`) before they reach an assign, so the `/?call=W5ABC/M` poison vector is closed. 0 still open. Nothing new in `b0ff0a5a..aced185d`. I checked the guard's edges: values that stay a single path segment (`..`, spaces, raw invalid UTF-8 via `%FF`) still match `/:callsign` and don't raise in `URI.parse`/`String.trim/upcase`, so the new check is sufficient for the crash class it targets — the mount check being looser than the form's `valid_callsign?/1` is deliberate and changes no failure path.

graham merged commit 4dc4e399f6 into main

2026-09-15 07:14:18 -05:00

graham deleted branch fix/m5-track-callsign-validation

2026-09-15 07:14:19 -05:00

graham referenced this pull request from a commit

2026-09-15 07:14:19 -05:00

fix: validate tracked callsign before navigation (#35)

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project