Some checks failed
Build and Push / Build and Push Docker Image (push) Failing after 2s
173 lines
8.3 KiB
Markdown
173 lines
8.3 KiB
Markdown
# Refactor and Hardening Implementation Handoff
|
|
|
|
Date: 2026-07-26
|
|
|
|
## Purpose
|
|
|
|
This file is the continuation point for the repository-wide architecture, security,
|
|
performance, SQL, and maintainability audit. A substantial first tranche has been
|
|
implemented, but enough work remains that it should be handled as a separate,
|
|
reviewed change set.
|
|
|
|
Do not use npm. This is a Phoenix application whose JavaScript is built with
|
|
esbuild through Mix.
|
|
|
|
## Current working tree
|
|
|
|
The working tree contains a large, uncommitted implementation. Preserve and review
|
|
those changes before continuing. In particular, `vendor/aprs` is dirty and contains
|
|
source/test changes. Those changes appeared during this implementation and must be
|
|
reviewed as part of the work; do not blindly reset the submodule.
|
|
|
|
Useful first commands:
|
|
|
|
```sh
|
|
git status --short
|
|
git diff --check
|
|
git diff --stat
|
|
git -C vendor/aprs status --short
|
|
git diff --submodule=short -- vendor/aprs
|
|
```
|
|
|
|
## Implemented
|
|
|
|
- Replaced the split packet-streaming paths with `Aprsme.SpatialPubSub` for both
|
|
LiveView and mobile clients.
|
|
- Removed per-packet broadcast tasks and made subscriptions PID-aware and
|
|
idempotent.
|
|
- Removed obsolete packet receiver, streaming pubsub, replay, DB optimizer, insert
|
|
optimizer, and sequence-counter infrastructure, together with their dead tests.
|
|
- Added a stale-packet guard to the LiveView packet batcher.
|
|
- Fixed the status page to use the correct task supervisor.
|
|
- Added callsign/transport-identifier validation at ingestion and made client-side
|
|
map marker construction safe from HTML injection.
|
|
- Added user roles, an admin role task, and admin authorization for operational
|
|
dashboards, error tracking, and bad-packet pages.
|
|
- Hardened session and remember-me cookies and enabled production HTTPS/HSTS
|
|
enforcement.
|
|
- Upgraded Bandit and `plug_crypto`.
|
|
- Made packet-retention partition deletion use an exact timestamp cutoff.
|
|
- Added a partition-aware geometry GiST index migration.
|
|
- Replaced the broken sequence-based packet count with an atomic row counter and
|
|
reconciled the existing value.
|
|
- Made partition deletion adjust the packet counter transactionally.
|
|
- Corrected database telemetry for partitioned tables and removed fabricated pool
|
|
metrics.
|
|
- Changed cumulative PromEx values from counters to last-value metrics.
|
|
- Made failed release migrations fail application startup.
|
|
- Removed duplicate release initialization and supervised deployment notification.
|
|
- Made `PartitionManager` the sole owner of packet retention.
|
|
- Removed unused registries and duplicate/empty asset build targets and loaders.
|
|
- Added Sobelow and a GitHub Actions workflow for format, compile, Credo, tests,
|
|
Sobelow, Hex audit, and Dialyzer.
|
|
- Updated affected architecture documentation.
|
|
|
|
## Validation already completed
|
|
|
|
- Full suite: `2447 passed (35 doctests, 31 properties, 2381 tests)`.
|
|
- `MIX_ENV=test mix compile --warnings-as-errors` passed.
|
|
- `MIX_ENV=dev mix esbuild default` passed.
|
|
- `git diff --check` passed before the final documentation/runtime edits.
|
|
|
|
These checks must be rerun after all remaining work:
|
|
|
|
```sh
|
|
MIX_ENV=dev mix format
|
|
git diff --check
|
|
MIX_ENV=dev mix compile --warnings-as-errors
|
|
MIX_ENV=test mix compile --warnings-as-errors
|
|
MIX_ENV=dev mix credo --strict
|
|
MIX_ENV=dev mix sobelow --config
|
|
MIX_ENV=dev mix hex.audit
|
|
MIX_ENV=dev mix dialyzer
|
|
MIX_ENV=test mix test
|
|
MIX_ENV=dev mix esbuild default
|
|
MIX_ENV=prod mix assets.deploy
|
|
```
|
|
|
|
## Remaining high-priority security work
|
|
|
|
1. Audit `RemoteIp`/forwarded-header handling. Only trust proxy headers from known
|
|
ingress proxy CIDRs; direct clients must not be able to spoof their address.
|
|
Add tests for trusted and untrusted peers.
|
|
2. Add server-side rate limiting to expensive or abusable LiveView events, mobile
|
|
channel commands, authentication paths, and search endpoints. Do not rely only
|
|
on controller plugs.
|
|
3. Finish Content Security Policy hardening. The root layout still depends on
|
|
inline script behavior, so `unsafe-inline` has not been eliminated. Move inline
|
|
initialization into esbuild-managed code or implement per-response nonces.
|
|
4. Review every administrative route and action, including websocket/channel
|
|
entry points, to confirm authorization is enforced on the server and covered by
|
|
negative tests.
|
|
5. Run Sobelow and triage the existing skip/fingerprint configuration. Remove stale
|
|
skips and document any accepted finding rather than suppressing broadly.
|
|
6. Review secrets and credentials in Kubernetes manifests. Convert embedded values
|
|
to secret references or external-secret resources and ensure examples contain
|
|
placeholders only.
|
|
7. Add least-privilege Kubernetes `NetworkPolicy` rules for the web application,
|
|
database, ingress, and any monitoring components.
|
|
|
|
## Remaining reliability and performance work
|
|
|
|
1. Bound all ingress and client-facing buffers:
|
|
- APRS connection receive/reconnect buffers.
|
|
- Mobile channel pending packet lists.
|
|
- LiveView queues and task result accumulation.
|
|
Define overflow behavior and expose drop/backpressure telemetry.
|
|
2. Put explicit limits on mobile `connect_info`, viewport/bounds payloads, callsign
|
|
lists, search terms, and requested result counts. Reject malformed or oversized
|
|
payloads before database work.
|
|
3. Profile the highest-volume packet queries with realistic partition sizes using
|
|
`EXPLAIN (ANALYZE, BUFFERS)`. Verify partition pruning and the new spatial index.
|
|
4. Remove remaining N+1 query patterns in map overlays, device/account pages, and
|
|
status/operational pages. Prefer bounded batch queries and preloads.
|
|
5. Audit mobile and map search for unbounded scans. Add deterministic ordering,
|
|
hard result caps, suitable indexes, and tests that assert the caps.
|
|
6. Review the packet counter migration under concurrent inserts and partition
|
|
drops in a staging database. Exercise rollback/failed-migration behavior.
|
|
7. Load-test spatial subscriptions with overlapping bounds and reconnect churn.
|
|
Confirm exactly-once delivery, bounded memory, and cleanup after process exits.
|
|
|
|
## Remaining maintainability work
|
|
|
|
1. Split `AprsmeWeb.MapLive.Index` into focused state, event, subscription, and
|
|
rendering modules. Preserve LiveView behavior with integration tests first.
|
|
2. Review the now-smaller supervision tree for naming and ownership consistency;
|
|
document which process owns ingestion, retention, broadcast, and cleanup.
|
|
3. Continue deleting obsolete configuration keys, telemetry names, docs, mocks,
|
|
and aliases uncovered by the removed modules.
|
|
4. Review callsign naming. Ingestion currently accepts safe APRS transport
|
|
identifiers (up to 20 characters, uppercase alphanumeric segments separated by
|
|
hyphens), which is intentionally broader than a strict AX.25 callsign. Rename
|
|
APIs or document this distinction to avoid future accidental tightening.
|
|
5. Add focused migration tests for:
|
|
- Existing partition indexes being attached correctly.
|
|
- Counter reconciliation on an existing populated database.
|
|
- Exact cutoff behavior around partition boundaries.
|
|
6. Review the deleted test count. The suite decreased because tests for removed
|
|
infrastructure were deleted; ensure important end-to-end ingestion coverage was
|
|
not lost when `packet_pipeline_integration_test.exs` was removed.
|
|
|
|
## Review notes
|
|
|
|
- The geometry-index migration creates and attaches indexes per partition in
|
|
production. Test behavior uses a simpler direct index path. Validate this on a
|
|
production-like PostgreSQL version and data volume.
|
|
- The packet-count trigger approach prioritizes exactness. Measure its write
|
|
contention at expected ingestion rates; if it is too expensive, replace it with
|
|
an explicitly approximate metric rather than a misleading exact API.
|
|
- The CSP work should retain the early theme selection behavior to avoid a flash
|
|
of the wrong theme.
|
|
- Do not reintroduce `StreamingPacketsPubSub` or per-packet spawned broadcast
|
|
tasks; extend `SpatialPubSub` if delivery behavior needs adjustment.
|
|
|
|
## Suggested sequence
|
|
|
|
1. Review and isolate the dirty `vendor/aprs` changes.
|
|
2. Run all static gates and fix existing findings.
|
|
3. Complete proxy trust, rate limiting, CSP, and authorization review.
|
|
4. Bound buffers and payloads, then load-test delivery.
|
|
5. Profile and fix SQL query/index issues.
|
|
6. Refactor the large LiveView only after behavioral and performance coverage is
|
|
stable.
|
|
7. Finish Kubernetes hardening and rerun the full release build.
|