Signed-off-by: Stan Grams <sjg@haxx.space>
5.1 KiB
Improvement Areas
A comprehensive audit of the trx-rs codebase covering code quality, architecture, security, testing, and performance. Each item includes the affected location and a suggested fix.
Last updated: 2026-03-26
Resolved
The following items have been fixed across PRs #58, #59, and #60:
Quick Wins (all complete)
- ✅ Session cleanup timer — 5-minute periodic
cleanup_expired()task - ✅
DecodeHistory<T>type alias — replaces 9 repeatedArc<Mutex<VecDeque<...>>>patterns - ✅
mode_to_string()allocation — returnsCow<'static, str>(zero-alloc for known modes) - ✅ FTx dedup —
HashSet<u16>for O(1) lookups - ✅ Unbounded channels —
VChanAudioCmdchannels bounded at 256 - ✅ JSON serialization —
#[serde(flatten)]wrapper replaces string-level splice - ✅
AtomicUsizecounter —estimated_total_count()avoids 9 mutex acquisitions - ✅ Cookie security warning — startup warning when
cookie_secureis false - ✅ Spectrum encoding — pre-allocated output string replaces
format!overhead - ✅
pub(crate)state data —ReadyStateData/TransmittingStateDatafields restricted with constructors + getters - ✅ Lock ordering docs — module-level documentation in
<vchan.rs>establishingrigs → sessions → audio_cmd
Critical (P0)
- ✅ Plugin loading validation — rejects world-writable files on Unix;
TRX_PLUGINS_DISABLEDenv var - ✅ Audio pipeline mutex panics — all
.expect()on history mutexes and.unwrap()on audio ring buffers replaced with.unwrap_or_else(|e| e.into_inner())poison recovery - ✅ vchan lock panics — ~25
.unwrap()on RwLock/Mutex replaced with poison recovery
High (P1)
- ✅ RigCat trait split — 13 SDR-specific methods extracted into
RigSdrextension trait;RigCatretains core CAT ops +as_sdr()/as_sdr_ref(); SoapySdrRig implements both; FT-817/FT-450D/DummyRig unchanged - ✅ Decoder history contention —
AtomicUsizetotal counter maintained by record/prune/clear
Medium (P2)
- ✅ Silent state machine failures — debug-level tracing for rejected transitions
- ✅ User input in logs — raw JSON truncated to 128 chars
- ✅ Rate limiting — per-IP
LoginRateLimiter(10 attempts/60s) on/auth/login - ✅ Lock-holding serialization — clone data out under lock, serialize after release
- ✅ Overly-public API — state data fields
pub(crate)with controlled accessors - ✅ Cookie security flag — startup warning for non-TLS deployments
- ✅ Lock ordering — documented in
<vchan.rs>module header
Remaining Issues
Critical (P0)
Plugin signing and cross-platform validation
Location: src/trx-app/src/<plugins.rs>
Current protections: file permission checks (Unix), TRX_PLUGINS_DISABLED env var,
loaded plugins logged at startup.
Still missing:
- No SHA-256 checksum verification — an attacker who passes the permission check can still load a tampered library
- No per-plugin permission scoping (all plugins get full context access)
- Windows has no file permission validation
Suggestions:
- SHA-256 checksum manifest (
plugins.toml) verified beforeLibrary::new - Config option to allowlist specific plugin filenames
- On Windows, verify file owner via
GetSecurityInfoor equivalent
High Priority (P1)
Synchronous locks in async contexts
Location: src/trx-client/trx-frontend/trx-frontend-http/src/<background_decode.rs>,
src/trx-client/trx-frontend/trx-frontend-http/src/<vchan.rs>
std::sync::RwLock is used inside async tasks. Current code is safe (no locks held
across await points), but not idiomatic. Migrating to tokio::sync::RwLock would
prevent future regressions.
Large functions in audio pipeline
Locations:
src/trx-server/src/<audio.rs>—run_capture()(~200 lines),run_playback()(~217 lines)
These contain nested loops, device re-enumeration logic, and stream error handling that should be extracted into focused helper functions.
Medium Priority (P2)
Configuration duplication
Location: src/trx-server/src/<config.rs> (1512 lines),
src/trx-client/src/<config.rs> (1181 lines)
14 config structs each, many mirrored between server and client. Extract shared
definitions (GeneralConfig, RigConfig, defaults) into trx-app.
Low Priority (P3)
Missing tests for critical paths
Serial backends (FT-817, FT-450D), plugin loading/discovery, and the audio pipeline (Opus encode/decode) have no or minimal test coverage.
Core crates (trx-core, trx-server, trx-client, trx-app) have limited
[dev-dependencies] and use only inline #[test] functions. Adding test
utilities (mock serial ports, test fixtures) would improve coverage.
Plugin system lacks versioning and lifecycle
Location: src/trx-app/src/<plugins.rs>
No plugin API version, capability manifest, or unload/reload semantics. Old plugins break silently on API changes.
Fix: Add a version field to the registration struct and reject incompatible plugins at load time.