Add security and reliability and add stubs for non essential controls

This commit is contained in:
2026-04-19 08:39:43 -07:00
parent ec8495cc54
commit 7f019cec4e
3 changed files with 258 additions and 2 deletions

103
DESIGN.md
View File

@@ -144,7 +144,7 @@ Shared PPI behavior:
| `TargetBuffer` | 2,4 | Mutex B — target data handoff between Thread 2 (traffic cop) and Thread 4 (simulator) |
| `TrafficCop` | 2 | Polls Simulator and RPi receivers each beam update; writes targets to SharedRenderState |
| `Simulator` | 4 | Runs fake targets independently; returns data to TrafficCop when polled |
| `KnobPanel` | 3 | Future hardware stub — writes to SharedRenderState under Mutex A |
| `KnobPanel` | 3 | Future hardware stub — idles without acquiring Mutex A until Arduino hardware is wired; SharedRenderState holds compile-time defaults from settings.h |
| `RPiReceiver` | 2 | Stub — one instance per Raspberry Pi; called by TrafficCop |
---
@@ -198,6 +198,88 @@ shaders/
---
## Robustness and Safety
### Input Rate Limiting (the mad-child problem)
GLFW key callbacks can fire hundreds of times per second under key-mashing or hardware
encoder noise. Each control variable carries a `last_event_time` timestamp alongside
its value. Any input arriving within `MIN_INPUT_INTERVAL_MS` of the previous accepted
input for that control is silently discarded. The `KEY_MAX_STEP` / `KEY_GONIO_MAX_STEP`
constants cap how far a single accepted input can move a value. Together these make it
impossible to slam a bearing to an extreme in under a second regardless of how fast the
input fires.
### Control Variable Clamping
Every state variable that a control modifies has companion `MIN_*` and `MAX_*` constants
in `settings.h`. The clamp is applied **at the point of write** using `std::clamp()`, in
every code path: keyboard callback, KnobPanel, and any future source. Thread 1 reads
already-clean values and applies a second `std::clamp()` before passing to the shader as
a second line of defense. No validation logic is needed at the shader boundary.
### Incoming Data Validation
`RPiReceiver::parseFrame()` and `Simulator::poll()` both return `std::optional<TargetData>`.
A return of `std::nullopt` means the frame was malformed or out of bounds. `TrafficCop`
discards nullopt silently — no exception, no abort, no log spam. Fields validated:
bearing (0360), range (0radar max), amplitude (01), altitude (060,000 ft), timestamp
(not stale, not future), target count (truncated to `MAX_SIMULTANEOUS_TARGETS`), and
frame byte length (rejected above `MAX_RPI_FRAME_BYTES`).
### Array Bounds Safety
- All fixed-size target storage uses `std::array<TargetData, MAX_TARGETS>` — never a raw C array.
- Array views passed between functions use `std::span<TargetData>` (C++20) — size always travels with the pointer.
- Ring buffers for phosphor persistence use modulo indexing (`index % CAPACITY`), never raw pointer arithmetic.
- `static_assert` guards validate that `MAX_TARGETS` and similar constants are within sane limits at compile time.
- Bounds-checked access (`.at()`) used in debug builds; validated index used in release.
### Thread Safety Rules
1. **Snapshot pattern**: Thread 1 acquires Mutex A, copies the entire `SharedRenderState` struct, releases the lock immediately, then renders from the local copy. No OpenGL calls are ever made while holding a mutex (Core Guideline CP.22).
2. **Always `std::scoped_lock`**: Mutexes are never locked/unlocked manually. `std::scoped_lock` releases on all exit paths including exceptions (SEI CERT CON51).
3. **Lock ordering**: If code ever needs both Mutex A and Mutex B simultaneously, A must be acquired first. Documented here as a project-wide invariant to prevent deadlock (SEI CERT CON53).
4. **Atomic simple flags**: Boolean toggles that are written by one thread and read by another (`prf_high`, `goniometer_mode`) use `std::atomic<bool>` — no mutex overhead for single-variable reads.
5. **KnobPanel idle guarantee**: Until hardware is connected, KnobPanel's thread loop sleeps and never calls `lock()` on Mutex A. Zero contention on that mutex from Thread 3.
### RAII Requirements
Every resource that is opened must be wrapped in an RAII holder so that it closes on any exit path:
| Resource | RAII mechanism |
|---|---|
| OpenGL VAO / VBO / texture / shader | Thin `GLHandle<T>` wrapper; destructor calls `glDelete*()` |
| GLFW window | `unique_ptr<GLFWwindow, decltype(&glfwDestroyWindow)>` |
| FreeType `FT_Library` / `FT_Face` | Small RAII structs; destructor calls `FT_Done_*` |
| Mutex locks | `std::scoped_lock` or `std::lock_guard` — never bare lock/unlock |
| Worker threads | `std::jthread` (C++20) — auto-joins on destruction; no detached threads |
| File handles (shader source) | `std::ifstream` — RAII by default |
No raw `new` or `delete` anywhere in the codebase. No `malloc`/`free`.
### SEI CERT C++ Compliance — Key Rules
| Rule | Enforcement |
|---|---|
| CON50 | Never destroy a mutex while locked — `std::scoped_lock` makes this structurally impossible |
| CON51 | Release locks on exceptions — `std::scoped_lock` handles automatically |
| CON53 | Consistent lock ordering (A before B) — documented invariant |
| ARR50 | Array indices always validated before use or bounded by `std::array::at()` |
| MEM51 | All resources managed by RAII wrappers; no raw `new`/`delete` |
| ERR50 | No `abort()` or `exit()` in normal operation; bad data is dropped, not fatal |
| INT30 | Bearing arithmetic uses `fmod()`, not unsigned wraparound subtraction |
| FLP30 | No floating-point loop counters; sweep angle is a double accumulator |
| OOP50 | No virtual method calls in constructors or destructors |
### C++ Core Guidelines Compliance — Key Rules
| Guideline | Enforcement |
|---|---|
| I.6 / I.7 | `Expects()` / `Ensures()` (GSL) at function entry/exit in debug builds |
| I.12 | `gsl::not_null<Scope*>` for the active scope pointer in ScopeManager |
| R.1 | All resource management through RAII handles (see table above) |
| CP.20 | `std::scoped_lock` for all mutex acquisition — no bare lock/unlock |
| CP.21 | `std::scoped_lock(mutexA, mutexB)` if both must be held simultaneously |
| CP.22 | No OpenGL calls while holding any mutex — snapshot pattern enforces this |
| Bounds.1 | No pointer arithmetic; `std::span` for array views |
| Bounds.2 | Array access only via `.at()` or pre-validated index |
| Bounds.4 | No `sprintf`, `strcpy`, `gets`; use `std::string` and `std::format` (C++20) |
---
## Key Design Notes
1. **ScopeManager** sits in Thread 1 and holds the active scope pointer. The GLFW
@@ -237,8 +319,25 @@ shaders/
file that needs a tunable value includes this file. No magic numbers
anywhere else in the codebase. Add candidate variables here before
coding begins; values can be refined during debugging and appearance work.
Sections defined: phosphor colors (P1/P7), sweep parameters, graticule
geometry and colors, cursor, noise floor, graticule swap animation timing,
key-hold acceleration, auto-advance timer, window/layout geometry, text
colors/sizes, **general operator control defaults** (Intensity, Focus,
Astigmatism, Gain, Rain Clutter, Wave Clutter, Graticule Intensity),
**per-scope default state** (initial bearing, range, cursor, calibrator
scale for each scope), and **PAR geometry** (non-linear scale breakpoint,
runway heading, glide path angle, scan widths).
10. **MarineAScope graticule swap** is a state machine with four states:
10. **General operator controls** (Intensity, Focus, Astigmatism, Gain, Rain
Clutter, Wave Clutter, Graticule Intensity) are placeholders for physical
encoders not yet purchased. Each has a `DEFAULT_*` constant in settings.h
and a corresponding variable in `SharedRenderState`. KnobPanel (Thread 3)
has a stub write path for each. The thread starts and idles, but never
calls `lock()` on Mutex A, so it imposes zero contention. When hardware
arrives, only KnobPanel changes — Thread 1 and the shaders are already
fully wired to consume the values.
11. **MarineAScope graticule swap** is a state machine with four states:
NORMAL → SLIDING_OUT → BARE_CRT → SLIDING_IN → NORMAL. The u and d keys
are blocked during the animation. The new range value is latched when the
key is pressed but not applied to the scope state until SLIDING_IN completes.