graham/prop
Watch
1
0
Fork
You've already forked prop
 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions

Fix remaining audit findings (bugs.md sweep) #27

Merged
graham merged 2 commits from fix/audit-remainder into main 2026-09-23 16:16:00 +00:00
Conversation 2 Commits 2 Files changed 97 +1873 -807
graham commented 2026-09-22 20:40:33 +00:00
Owner
Copy link

Second pass over tmp/bugs.md after PRs #18-#26. Fixes every remaining confirmed BUG/RISK/SECURITY finding across propagation core, weather clients, auth, workers, terrain/radio/import, the Rust grid worker, and test hygiene.

Highlights:

  • Markdown code spans no longer get emphasis/link rewriting inside backticks (/algo)
  • Freshness signal filters to HRRR source so HRDPS completions cannot mask a dead chain
  • Email confirmation/change links require a click (GET no longer consumes single-use tokens)
  • Registration no longer reveals whether an email/callsign is taken
  • Session cookie only re-sent on change; tile endpoints private; CSP enumerates hosts
  • PathCompute reads 9 pgrid points instead of the full 92k-cell decode; composite score computed once
  • CSV import handles quoted multi-line fields; SRTM/canopy downloads atomic
  • Rust: chain panics fail the row, f00 degrades on missing diagnostics, NEXRAD off runtime thread, idx cache pruned, SIGTERM requeues
  • grid_tasks terminal rows pruned after 7 days
  • Remaining contacts_dedup_idx test collisions stamped unique
  • Also resolves merge-conflict markers accidentally committed to CHANGELOG.md

make precommit green (format, tsc, tests 5015, deps.unlock, credo --strict, xref). cargo clippy -D warnings + cargo test clean.

Second pass over tmp/bugs.md after PRs #18-#26. Fixes every remaining confirmed BUG/RISK/SECURITY finding across propagation core, weather clients, auth, workers, terrain/radio/import, the Rust grid worker, and test hygiene. Highlights: - Markdown code spans no longer get emphasis/link rewriting inside backticks (/algo) - Freshness signal filters to HRRR source so HRDPS completions cannot mask a dead chain - Email confirmation/change links require a click (GET no longer consumes single-use tokens) - Registration no longer reveals whether an email/callsign is taken - Session cookie only re-sent on change; tile endpoints private; CSP enumerates hosts - PathCompute reads 9 pgrid points instead of the full 92k-cell decode; composite score computed once - CSV import handles quoted multi-line fields; SRTM/canopy downloads atomic - Rust: chain panics fail the row, f00 degrades on missing diagnostics, NEXRAD off runtime thread, idx cache pruned, SIGTERM requeues - grid_tasks terminal rows pruned after 7 days - Remaining contacts_dedup_idx test collisions stamped unique - Also resolves merge-conflict markers accidentally committed to CHANGELOG.md make precommit green (format, tsc, tests 5015, deps.unlock, credo --strict, xref). cargo clippy -D warnings + cargo test clean.
- Markdown: extract inline code spans before emphasis/link rewriting so
  underscores and asterisks inside backticks stay literal (/algo page)
- Freshness: latest_chain_completion_time filters source='hrrr' so HRDPS
  completions can't mask a dead HRRR chain
- Email tokens: GET renders a confirmation page; POST consumes the token
  (mail scanners no longer burn single-use links)
- Registration: uniform 'check your inbox' flash (no user enumeration)
- Session cookie only re-emitted when the session changes; tile endpoints
  marked private; CSP enumerates hosts instead of https: wildcards
- Invalid API bearer tokens rate-limited per IP; beacon monitor token
  redacted from logs; MetricsPlug uses compile-time env flag
- PathCompute reads 9 pgrid points instead of decoding the full grid;
  composite score computed once per band per point; band_summary names
  the actual band; SporadicE hop window shared with classifier;
  BandWeights refresh timestamp moved to atomics
- Weather: scalar cache invalidated per-source on propagation_ready;
  GEFS APCP divided by the fetched accumulation window; point-presence
  batch uses unnest+EXISTS; dead HrrrNativeClient duct code removed
- Workers: Pskr recalibration run row starts 'running'; contact import
  no-retry + failed-run marking; home-QTH lookup dedup + transient retry;
  Pskr aggregator chunked upserts + PINGRESP watchdog; Rover.Elevation
  uses Srtm.samples_for_tile; Cache eviction batched; PollWorker skips
  unknown stations; grid_tasks terminal rows pruned after 7 days
- Terrain/radio: CSV quote-aware record splitting; dedup lookup picks
  closest match in window; SRTM downloads atomic + missing-tile warning;
  canopy downloads via .part rename
- Rust: chain panics mark the row failed; f00 degrades on missing
  diagnostic inputs; NEXRAD decode on spawn_blocking; idx cache pruned;
  SIGTERM requeues in-flight row; duct clamps; rotated-pole spec check
- Tests: remaining contacts_dedup_idx collisions stamped unique;
  CHANGELOG merge-conflict markers resolved
skippy-bot reviewed 2026-09-22 20:52:10 +00:00
skippy-bot left a comment
Copy link

🤖 Skippy PR review

1 finding — none blocking.

Severity Location Issue
🔵 Suggestion lib/microwaveprop/terrain/srtm.ex:47 Orphaned .tmp. SRTM tiles have no sweeper

Reviewed 9c390a760569. Comment skippy review to re-run.

### 🤖 Skippy PR review **1 finding** — none blocking. | Severity | Location | Issue | | --- | --- | --- | | 🔵 Suggestion | `lib/microwaveprop/terrain/srtm.ex:47` | Orphaned `.tmp.` SRTM tiles have no sweeper | <sub>Reviewed `9c390a760569`. Comment `skippy review` to re-run.</sub> <!-- skippy-pr-review -->
lib/microwaveprop/terrain/srtm.ex
@ -44,0 +44,4 @@
# pods on NFS) never observe a partially-written tile; a stray
# .tmp file is ignored by readers and overwritten next time.
tmp = path <> ".tmp." <> unique_suffix()
File.write!(tmp, decompressed)
skippy-bot commented 2026-09-22 20:52:10 +00:00
First-time contributor
Copy link

🔵 Suggestion — Orphaned .tmp. SRTM tiles have no sweeper

The unique suffix means a .tmp. file is never reused, so a failed File.write!/File.rename! or a pod killed between the two lines leaves a full-size partial tile (up to ~26 MB for SRTM1) in the tiles dir forever. Nothing prunes srtm_tiles_dir — the .tmp.* sweeper in ScoreStore only walks the scores/profiles/scalar trees — and Canopy.BulkFetch, fixed in this same PR, uses a fixed .part name that the next run overwrites, so this path is the odd one out (the comment above claims the file is "overwritten next time", which the unique suffix prevents). Fix: use a fixed path <> ".part" name like canopy does, or wrap the write+rename in try/after with File.rm(tmp) on failure.

**🔵 Suggestion — Orphaned `.tmp.` SRTM tiles have no sweeper** The unique suffix means a `.tmp.` file is never reused, so a failed `File.write!`/`File.rename!` or a pod killed between the two lines leaves a full-size partial tile (up to ~26 MB for SRTM1) in the tiles dir forever. Nothing prunes `srtm_tiles_dir` — the `.tmp.*` sweeper in `ScoreStore` only walks the scores/profiles/scalar trees — and `Canopy.BulkFetch`, fixed in this same PR, uses a fixed `.part` name that the next run overwrites, so this path is the odd one out (the comment above claims the file is "overwritten next time", which the unique suffix prevents). Fix: use a fixed `path <> ".part"` name like canopy does, or wrap the write+rename in try/after with `File.rm(tmp)` on failure.
skippy-bot marked this conversation as resolved
Sweep orphaned SRTM .tmp tiles in the prune worker
All checks were successful
skippy-bot/review Skippy review: clean — no open findings
Details
920ce62346
 
Skippy review on PR #27: the atomic write-then-rename in
Srtm.download_tile/3 can leave a .tmp.<uniq> orphan when the process
dies between write and rename, and nothing ever removed them.

Adds Srtm.prune_tmp_files/2 — deletes .tmp.* files older than 1 h
(age-gated so an in-flight download is never swept), routed through
FilePrune.delete_all/2 for honest delete counting — and calls it from
PropagationPruneWorker's 15-minute pass. older_than?/1 uses
File.stat(time: :posix) so mtime is unix seconds, not local time.
skippy-bot commented 2026-09-23 16:13:58 +00:00
First-time contributor
Copy link

Resolved 1 of 1 earlier findings: Srtm.prune_tmp_files/2 plus the sweep in PropagationPruneWorker (15 min cron, 1 h age gate) covers the orphaned .tmp. tiles, so the leak I flagged at srtm.ex:47 is closed. 0 still open. Nothing new in 9c390a76..920ce623.

Non-blocking: the comment at srtm.ex:45-47 still says a stray .tmp file is "overwritten next time" - the unique suffix prevents that, and it is now the sweeper that removes it. Reword when you next touch the file so nobody deletes the sweep as redundant.

Resolved 1 of 1 earlier findings: `Srtm.prune_tmp_files/2` plus the sweep in `PropagationPruneWorker` (15 min cron, 1 h age gate) covers the orphaned `.tmp.` tiles, so the leak I flagged at `srtm.ex:47` is closed. 0 still open. Nothing new in `9c390a76..920ce623`. <sub>Non-blocking: the comment at `srtm.ex:45-47` still says a stray `.tmp` file is "overwritten next time" - the unique suffix prevents that, and it is now the sweeper that removes it. Reword when you next touch the file so nobody deletes the sweep as redundant.</sub> <!-- skippy-pr-review -->
graham merged commit 619fc010af into main 2026-09-23 16:16:00 +00:00
graham deleted branch fix/audit-remainder 2026-09-23 16:16:00 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
graham/prop!27
No description provided.