Parity with the Android client: drop the "Allow self-signed certificate" escape hatch. The app now performs system TLS trust only — a self-signed appliance must have its CA installed on the device (MDM/profile). A code-only public-key pinning policy remains (no UI). - ServerTrustEvaluator: remove ServerTrustPolicy.allowSelfSignedForHost and its challenge handler; keep .system (default) and .pinPublicKeySHA256. - SettingsView: remove the self-signed toggle/state/prefill; makeConfig uses the default .system policy. - MCMSConnection / APIConfiguration: update usage-sketch + doc comments. - README / GUI-NOTES / MCMS_API.md: document system-trust-only; self-signed boxes need their CA installed on the device. - Add CHANGELOG.md for this session, flagged for the Android port. Note: configs previously persisted with the self-signed policy fail to decode and reset to defaults (one-time re-entry of the server URL). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
5.7 KiB
Changelog
2026-05-31 — Code-review pass + TLS parity
A multi-agent code review of the iOS app, the confirmed fixes applied, and the
self-signed TLS option removed for parity with the Android client. This entry is
written for cross-pollination: the two apps share REST behaviour and field paths
(see MCMS_API.md), so items below are grouped by how likely they apply to Android.
🔁 Check on Android — shared REST logic / behaviour
-
Pagination cursor must be the real
_id, not a synthesized id.getAllfollowed thenextcursor using each model'sid, which fell back to a random value when_idwas absent → the cursor became meaningless and could re-serve or skip pages (silently wrong fleet list). Fixed to drive the cursor off the actual_idand stop paging when it's absent. → Android: if the paginator derivesnextfrom a modelidthat has any non-_idfallback, it has the same bug. Use the document_id(or stop). -
Firmware image with empty compatibility metadata was shown as "compatible for every ONU."
isCompatiblereturned true when the image declared noCompatible Manufacturer/Compatible Model. For a destructive flash that invites pushing the wrong image. Now requires a positive model match; un-typed images appear only under the "Show all" escape hatch. → Android: mirror the same rule in the firmware picker's compatibility filter. -
Per-ONU alarm filter may return the WHOLE fleet's alarms. The ONU-detail Alarms tab filters the fleet-wide alarm-history endpoint with
query = {"_id": onuId}, but that key is unverified — if it's wrong the server returns 200 with every ONU's alarms (no error). Left as-is (couldn't verify the key off a live box) with a loud in-code warning. → Android: same endpoint + key risk. Verify the real filter key againstopenapi.json/ a live response, or use the dedicated per-id alarm endpoint. -
Optical RX thresholds were inconsistent between screens. Dashboard flagged abnormal at
< -28 || > -10dBm; the detail row tinted green at>= -28but its footer legend said "green ≥ -30". Unified to one source of truth (OpticalThreshold: green floor -28, red floor -30, high -10) and corrected the legend. → Android: if it shows optical levels with color thresholds, align dashboard and detail to one constant and check the legend text matches the actual rule. -
Dashboard best-effort sections went stale silently on a transient failure. During the 30s auto-refresh, if the alarm-histories fetch failed, the alarm-by-severity counts kept the previous cycle's values while the dashboard still showed "loaded" (and controller count masked a failed fetch as
0). Now keeps last-good explicitly and doesn't report a failed count as zero. → Android: if the dashboard auto-refreshes with best-effort sections, decide explicitly between keep-last-good and show-empty rather than whatever the control flow produces. -
JSON number → Int coercion could crash on a malformed/huge value. Swift's
Int(double)traps on NaN/±∞/out-of-range; a JSON literal like1e400decodes to∞and crashed an accessor. Guarded to return nil. → Android: Kotlin'sDouble.toInt()won't trap the same way, but confirm out-of-range / NaN counters (FW Bank Ptr, DHCP lease seconds, util %, temps) don't yield garbage Ints in the equivalent accessors.
🔒 Security parity (the point of this round)
-
Removed the "Allow self-signed certificate" option entirely — the
ServerTrustPolicy.allowSelfSignedForHostcase, the Settings toggle, and its wiring. The app now uses system TLS trust only (a code-only public-key pinning option remains, unused by the UI). Consequence: a self-signed appliance (e.g. the lab boxmcms.lab.svorka.net) is unreachable until its CA is installed on the device (MDM/configuration profile) or the box gets a CA-trusted cert. This matches the Android client. Old persisted configs that stored the self-signed policy will fail to decode and reset to defaults (one-time; just re-enter the server URL). -
Public-key pin was hashed over the raw key, not the SubjectPublicKeyInfo — so a standard
openssl … pkey -pubin -outform der | dgst -sha256pin never matched (pinning failed closed). Now prepends the algorithm-specific ASN.1 SPKI header and hashes the real SPKI. → Android: if it pins, OkHttpCertificatePinneralready uses SPKIsha256/…— no change needed; just don't reintroduce a raw-key hash.
❓ Analogous patterns worth a look (not certain to apply)
-
Overlapping load / stale-commit races. iOS view models let a pull-to-refresh overlap a tab-switch / auto-refresh load and commit stale or wrong-tab data; fixed with a per-load generation guard + cancellation check before committing. → Android: if
load()/ refresh use coroutines without single-flight or job-cancel, the same stale-commit / wrong-tab race can occur. -
List-row identity fallback. iOS list rows fell back to a random id when
_idwas missing, breaking diffing/scroll/animation. → Android: if DiffUtil keys fall back similarly, use a stable/content-derived key. -
Networking client lifecycle. iOS leaked a
URLSessionper connection/probe (created with a delegate, never invalidated). → Android: ensure OkHttp clients are shared/closed rather than created and dropped per connection test.
📱 iOS-only (listed for completeness; not relevant to Android)
- Swift closure type-inference build fix (annotate list-VM merge locals).
MongoDocument.idSwiftUI identity specifics; SwiftUIForEachkeying.Formatbyte/bit-rate rounding rollover at the unit boundary.- Dead-code, stale-docstring, and helper-dedup cleanups.