Make beacon monitor tokens always retrievable #4

graham · 2026-08-30T18:45:22Z

graham commented

2026-08-30 18:45:22 +00:00

Monitor tokens are readable again

The monitor token was reveal-once: 20260824120000_hash_beacon_monitor_tokens replaced the column with a SHA-256 digest, so the plaintext existed only in the LiveView socket that minted it. An operator moving a station to new hardware — or re-flashing the one they have — had no way to read it back, and had to regenerate, knocking a working monitor offline to recover a credential that was never compromised.

This stores the token itself (unique-indexed) and renders it on the monitor page on every visit. Rotation stays available, but as a deliberate act rather than the price of not having written the token down.

beacon_monitor.ex — :token is a real field again; token_hash is gone
beacon_monitors.ex — get_monitor_by_token/1 matches the indexed column directly; regenerate_token/1 is a plain update
MonitorLive.Show — identity panel always renders the token + Copy button. preserve_token/2 and token_revealed?/1 are deleted; they existed only because a reload returned nothing
MonitorLive.New — the "shown only once" warning becomes an informational note pointing back at the monitor page

⚠️ Deployed monitors need their new token

A digest is one-way, so no existing token is recoverable. The migration issues every row a fresh token, and every deployed monitor will 401 until its operator pastes the new value. This was a deliberate choice over leaving legacy rows showing "regenerate to recover".

Deliberately not changed

The REST API is not widened. BeaconMonitorJSON.data/1 still omits the token, so only the create response carries it. index/show are reachable by any scoped API token, whereas the monitor page requires a session.
User API tokens (Accounts.UserApiToken) remain hashed. Separate system.

wgrib2 suite is CI-only

Wgrib2Test shells out to the real wgrib2 binary, which ships in the CI base image but is not on a typical dev machine — it contributed 12 failures to every local run, noise that buries real regressions.

Tagged @moduletag :wgrib2, excluded in test_helper.exs unless the binary is on PATH or CI is set. Sibling modules in that directory decode GRIB2 in pure Elixir and stay untagged.

One wrinkle worth a look during review: gating on $CI alone would have been wrong, because the workflow runs mix test inside docker run and the runner's CI variable does not reach the container — the suite would have silently skipped in CI too. So -e CI=true is passed into the container, and a wgrib2 missing from the image now fails loudly instead of vanishing into 22 excluded tests.

Verified both directions: default → 0 tests, 22 excluded; CI=true → they run.

Verification

mix format --check-formatted ✅ · mix credo --strict ✅ · mix xref --fail-above 4 ✅ · mix compile --warnings-as-errors ✅

Full suite: 4881 passed, 6 skipped, 22 excluded, zero failures (was 12 failures at baseline, all wgrib2).

## Monitor tokens are readable again The monitor token was reveal-once: `20260824120000_hash_beacon_monitor_tokens` replaced the column with a SHA-256 digest, so the plaintext existed only in the LiveView socket that minted it. An operator moving a station to new hardware — or re-flashing the one they have — had no way to read it back, and had to **regenerate**, knocking a working monitor offline to recover a credential that was never compromised. This stores the token itself (unique-indexed) and renders it on the monitor page on every visit. Rotation stays available, but as a deliberate act rather than the price of not having written the token down. - `beacon_monitor.ex` — `:token` is a real field again; `token_hash` is gone - `beacon_monitors.ex` — `get_monitor_by_token/1` matches the indexed column directly; `regenerate_token/1` is a plain update - `MonitorLive.Show` — identity panel always renders the token + Copy button. `preserve_token/2` and `token_revealed?/1` are deleted; they existed only because a reload returned nothing - `MonitorLive.New` — the "shown only once" warning becomes an informational note pointing back at the monitor page ### ⚠️ Deployed monitors need their new token A digest is one-way, so no existing token is recoverable. **The migration issues every row a fresh token, and every deployed monitor will 401 until its operator pastes the new value.** This was a deliberate choice over leaving legacy rows showing "regenerate to recover". ### Deliberately not changed - **The REST API is not widened.** `BeaconMonitorJSON.data/1` still omits the token, so only the create response carries it. `index`/`show` are reachable by any scoped API token, whereas the monitor page requires a session. - **User API tokens (`Accounts.UserApiToken`) remain hashed.** Separate system. ## wgrib2 suite is CI-only `Wgrib2Test` shells out to the real `wgrib2` binary, which ships in the CI base image but is not on a typical dev machine — it contributed **12 failures to every local run**, noise that buries real regressions. Tagged `@moduletag :wgrib2`, excluded in `test_helper.exs` unless the binary is on PATH or `CI` is set. Sibling modules in that directory decode GRIB2 in pure Elixir and stay untagged. One wrinkle worth a look during review: gating on `$CI` alone would have been **wrong**, because the workflow runs `mix test` inside `docker run` and the runner's `CI` variable does not reach the container — the suite would have silently skipped in CI too. So `-e CI=true` is passed into the container, and a `wgrib2` missing from the image now fails loudly instead of vanishing into 22 excluded tests. Verified both directions: default → `0 tests, 22 excluded`; `CI=true` → they run. ## Verification `mix format --check-formatted` ✅ · `mix credo --strict` ✅ · `mix xref --fail-above 4` ✅ · `mix compile --warnings-as-errors` ✅ Full suite: **4881 passed, 6 skipped, 22 excluded**, zero failures (was 12 failures at baseline, all wgrib2).

graham added 2 commits

2026-08-30 18:45:22 +00:00

Store beacon monitor tokens in full so operators can retrieve them 67223a6518

The monitor token was reveal-once: only its SHA-256 digest was persisted,
so the plaintext existed solely in the LiveView socket that minted it. An
operator moving a station to new hardware — or re-flashing the one they
have — had no way to read the token back, and had to regenerate it,
knocking the running monitor offline to recover a credential that had not
actually been compromised.

Store the token itself, unique-indexed, and render it on the monitor page
on every visit. Rotation stays available but becomes a deliberate act
rather than the price of having forgotten to write the token down.

A digest is one-way, so no existing monitor's token is recoverable. The
migration issues every row a fresh one; deployed monitors will 401 until
their operator pastes the new value.

The API is deliberately not widened: BeaconMonitorJSON.data/1 still omits
the token, so only the create response carries it. index/show are reachable
by any scoped API token, whereas the monitor page requires a session.

Reverts the storage side of 20260824120000_hash_beacon_monitor_tokens.
User API tokens (Accounts.UserApiToken) are a separate system and remain
hashed.

Tag the wgrib2 suite so it runs in CI, not on dev machines c91146c6c8

Wgrib2Test shells out to the real wgrib2 binary, which ships in the CI base
image but is not on a typical dev machine. Locally it contributed 12
failures to every run — noise that buries real regressions and trains you
to ignore a red suite.

Tag the module :wgrib2 and exclude it from test_helper unless the binary is
on PATH or CI is set. The sibling modules in that directory decode GRIB2 in
pure Elixir and stay untagged.

Gating on CI alone would have been wrong: the workflow runs mix test inside
docker run, and the runner's CI variable does not reach the container, so
the suite would have silently skipped in CI too. Pass CI=true into the
container as well, so a wgrib2 missing from the image fails loudly instead
of vanishing into 22 excluded tests.

Local run is now clean; mix test --include wgrib2 opts back in.

graham merged commit 9035703eaa into main

2026-08-30 18:46:01 +00:00

graham deleted branch feat/monitor-token-always-retrievable

2026-08-30 18:46:01 +00:00

graham referenced this pull request from a commit

2026-08-30 18:46:03 +00:00

Make beacon monitor tokens always retrievable (#4)

skippy-bot reviewed

2026-08-30 18:55:38 +00:00

skippy-bot left a comment

🤖 Skippy PR review

1 finding — none blocking.

Severity	Location	Issue
🟡 Warning	`lib/microwaveprop/beacon_monitors.ex:229`	Raw bearer bytes now reach Postgres — a non-UTF-8 token 500s instead of 401ing

Checked clean: migration up/down are symmetric and the ADD COLUMN's ACCESS EXCLUSIVE lock keeps a NULL token from slipping in mid-backfill; no remaining callers of the deleted hash_token/1, token_revealed?/1 or preserve_token/2; BeaconMonitorJSON.data/1 still omits the token and index.ex never rendered it, so the only new read surfaces are the owner/admin-gated monitor page; prod log level is :info and :filter_parameters covers token, so the plaintext does not land in request or Ecto logs; the wgrib2 gate is the only mix test invocation in the workflow and it now gets CI=true.

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

### 🤖 Skippy PR review **1 finding** — none blocking. | Severity | Location | Issue | | --- | --- | --- | | 🟡 Warning | `lib/microwaveprop/beacon_monitors.ex:229` | Raw bearer bytes now reach Postgres — a non-UTF-8 token 500s instead of 401ing | Checked clean: migration up/down are symmetric and the ADD COLUMN's ACCESS EXCLUSIVE lock keeps a NULL token from slipping in mid-backfill; no remaining callers of the deleted `hash_token/1`, `token_revealed?/1` or `preserve_token/2`; `BeaconMonitorJSON.data/1` still omits the token and `index.ex` never rendered it, so the only new read surfaces are the owner/admin-gated monitor page; prod log level is `:info` and `:filter_parameters` covers `token`, so the plaintext does not land in request or Ecto logs; the wgrib2 gate is the only `mix test` invocation in the workflow and it now gets `CI=true`. <sub>Reviewed `c91146c6c848`. Comment `skippy review` to re-run.</sub>

lib/microwaveprop/beacon_monitors.ex

					
				@ -224,3 +227,3 @@

				    now = DateTime.utc_now(:second)

				    case Repo.get_by(BeaconMonitor, token_hash: hash_token(token)) do

				    case Repo.get_by(BeaconMonitor, token: token) do

🟡 Warning — Raw bearer bytes now reach Postgres — a non-UTF-8 token 500s instead of 401ing

hash_token/1 used to normalise whatever arrived in the header into a 32-byte digest before it ever touched the database. Now the client's bytes go straight into a varchar comparison, and Postgres validates encoding on text parameters: Authorization: Bearer <invalid UTF-8> (scanner junk, a mangled paste, a truncated multibyte token) raises Postgrex.Error 22021 invalid byte sequence for encoding "UTF8", so MicrowavepropWeb.Api.MonitorAuth crashes and the unauthenticated ingest endpoint answers 500 rather than the 401 it is supposed to. Auth.extract_bearer/1 only checks the "Bearer " prefix and byte_size > 0, so nothing upstream filters it. One-line guard:

def get_monitor_by_token(token) when is_binary(token) do
  now = DateTime.utc_now(:second)

  with true <- String.valid?(token),
       %BeaconMonitor{} = monitor <- Repo.get_by(BeaconMonitor, token: token) do
    # existing revoked/expires_at clauses
  else
    _ -> nil
  end
end

**🟡 Warning — Raw bearer bytes now reach Postgres — a non-UTF-8 token 500s instead of 401ing** `hash_token/1` used to normalise whatever arrived in the header into a 32-byte digest before it ever touched the database. Now the client's bytes go straight into a `varchar` comparison, and Postgres validates encoding on text parameters: `Authorization: Bearer <invalid UTF-8>` (scanner junk, a mangled paste, a truncated multibyte token) raises `Postgrex.Error` 22021 `invalid byte sequence for encoding "UTF8"`, so `MicrowavepropWeb.Api.MonitorAuth` crashes and the unauthenticated ingest endpoint answers **500** rather than the 401 it is supposed to. `Auth.extract_bearer/1` only checks the `"Bearer "` prefix and `byte_size > 0`, so nothing upstream filters it. One-line guard: ```elixir def get_monitor_by_token(token) when is_binary(token) do now = DateTime.utc_now(:second) with true <- String.valid?(token), %BeaconMonitor{} = monitor <- Repo.get_by(BeaconMonitor, token: token) do # existing revoked/expires_at clauses else _ -> nil end end ```