Add next-session note: verify the un-compiled TLS-removal merge on the Mac

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

APIConfiguration.swift

@@ -37,8 +37,9 @@ struct APIConfiguration: Equatable, Codable {
     var sessionCookieName: String = "sessionid"
     var csrfCookieName: String = "csrftoken"
 
-    /// How to treat the server's TLS certificate. These boxes typically present
+    /// How to treat the server's TLS certificate. Defaults to system trust; the
-    /// a self-signed / internal-CA cert. See `ServerTrustEvaluator`.
+    /// app has no self-signed bypass, so a self-signed box needs its CA installed
+    /// on the device (MDM/profile). See `ServerTrustEvaluator`.
     var trustPolicy: ServerTrustPolicy = .system
 
     /// Request timeout (seconds).

CHANGELOG.md

@@ -0,0 +1,100 @@
+# 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.**
+  `getAll` followed the `next` cursor using each model's `id`, which fell back to a
+  random value when `_id` was absent → the cursor became meaningless and could
+  re-serve or skip pages (silently wrong fleet list). Fixed to drive the cursor off
+  the actual `_id` and stop paging when it's absent.
+  → *Android:* if the paginator derives `next` from a model `id` that has any
+  non-`_id` fallback, it has the same bug. Use the document `_id` (or stop).
+
+- **Firmware image with empty compatibility metadata was shown as "compatible for
+  every ONU."** `isCompatible` returned true when the image declared no
+  `Compatible 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 against
+  `openapi.json` / a live response, or use the dedicated per-id alarm endpoint.
+
+- **Optical RX thresholds were inconsistent between screens.** Dashboard flagged
+  abnormal at `< -28 || > -10` dBm; the detail row tinted green at `>= -28` but 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 like `1e400` decodes
+  to `∞` and crashed an accessor. Guarded to return nil.
+  → *Android:* Kotlin's `Double.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.allowSelfSignedForHost` case, 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 box `mcms.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 -sha256` pin never
+  matched (pinning failed closed). Now prepends the algorithm-specific ASN.1 SPKI
+  header and hashes the real SPKI.
+  → *Android:* if it pins, OkHttp `CertificatePinner` already uses SPKI
+  `sha256/…` — 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 `_id`
+  was missing, breaking diffing/scroll/animation.
+  → *Android:* if DiffUtil keys fall back similarly, use a stable/content-derived
+  key.
+
+- **Networking client lifecycle.** iOS leaked a `URLSession` per 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.id` SwiftUI identity specifics; SwiftUI `ForEach` keying.
+- `Format` byte/bit-rate rounding rollover at the unit boundary.
+- Dead-code, stale-docstring, and helper-dedup cleanups.

GUI-NOTES.md

@@ -9,7 +9,7 @@ a working skeleton you run against a live MCMS box, then refine.
 App/         MCMSMobileApp (@main), RootView routing, AppEnvironment
 UI/          Shared components (badges, count tiles, load/error states, LoadPhase)
 Features/
-  Settings/  Connection config + test (direct or proxy, self-signed toggle)
+  Settings/  Connection config + test (direct or proxy; system TLS trust only)
   Login/     Email/password, Keychain prefill
   Dashboard/ Device + alarm counts (VM + View)
   ONUList/   Searchable ONU list (VM + View)
@@ -39,11 +39,12 @@ reinstate a `ContentUnavailableView` shim.
 
 ## App Transport Security (the gotcha)
 
-TLS *trust* for self-signed certs is handled in code by `ServerTrustEvaluator`,
+TLS uses **system trust only** — there is no self-signed bypass. (Historical note:
-but **ATS is separate** and can still block the connection:
+an earlier build had a `ServerTrustEvaluator` self-signed toggle; it was removed for
+parity with Android.) ATS is separate and can still block the connection:
 
-- **HTTPS to a self-signed host:** the trust delegate covers it; no Info.plist
+- **HTTPS to a self-signed host:** the connection FAILS unless the host's CA is
-  change needed for cert validation itself. (Keep TLS 1.2+.)
+  installed on the device (MDM/configuration profile). Keep TLS 1.2+.
 - **Plain HTTP** (no TLS) or weak TLS: ATS blocks it by default. Add a *scoped*
   exception for your host in Info.plist rather than a blanket allow:
 

HANDOFF.md

@@ -3,6 +3,25 @@
 Context for the next session/developer. Pairs with [`README.md`](README.md) and
 [`MCMS_API.md`](MCMS_API.md).
 
+## Last session (2026-05-31)
+
+Landed on `main`: a multi-agent code-review pass (correctness / concurrency /
+security / cleanup — see [`CHANGELOG.md`](CHANGELOG.md)), then removal of the
+self-signed TLS option (system trust only, parity with the Android client).
+
+> ⚠️ **Verify on the next Mac build.** The code-review fixes were confirmed
+> building on the Mac, but the **self-signed-TLS removal was merged to `main`
+> un-compiled** — the Linux dev box that made the change has no Swift toolchain.
+> First thing next session: build `main` and confirm it compiles. It's a small
+> change (removed `ServerTrustPolicy.allowSelfSignedForHost` + the Settings
+> toggle; the trust `switch` stays exhaustive), so it should be clean.
+
+TLS consequence: **system trust only** now — a self-signed appliance (the lab box
+`mcms.lab.svorka.net`) is unreachable until its CA is installed on the device
+(MDM/configuration profile), or it gets a CA-trusted cert. `CHANGELOG.md` is
+written to port these changes to the Android side. Merged branches
+`code-review-fixes` and `remove-self-signed-tls` can be deleted.
+
 ## Status
 
 Feature-complete and (per off-device typecheck + the user's Xcode builds)

MCMSConnection.swift

@@ -37,18 +37,14 @@ final class MCMSConnection {
 /*
  USAGE SKETCH (delete once real views exist)
 
- // Direct access to the appliance with a self-signed cert on a controlled net:
+ // The appliance must present a CA-trusted cert (real cert, or an internal CA
- let url = URL(string: "https://10.2.10.29/api")!
+ // installed on the device via MDM/profile) — system trust only, no self-signed bypass:
+ let url = URL(string: "https://mcms.example.com/api")!
  let config = APIConfiguration(
      baseURL: url,
      apiVersion: "v1",                       // confirm /api/v1 vs /v1 from openapi.json
-     databaseId: nil,                        // or a specific DB id
+     databaseId: nil                         // or a specific DB id
-     trustPolicy: .allowSelfSignedForHost("10.2.10.29")
+ )                                            // trustPolicy defaults to .system
- )
-
- // Behind a reverse proxy with a real cert — change ONLY the URL + trust:
- // let config = APIConfiguration(baseURL: URL(string: "https://mcms.example.com")!,
- //                               trustPolicy: .system)
 
  let connection = MCMSConnection(config: config)
 

MCMS_API.md

@@ -652,10 +652,10 @@ is between −28 and −30 dBm.
   HTTPCookieStorage.shared.cookies(for: baseURL)?
     .first { $0.name == "__Host-csrftoken" || $0.name == "csrftoken" }?.value
   ```
-- **Self-signed certs**: Implement
+- **Self-signed certs**: NOT accepted in-app — both clients use system trust only,
-  `URLSessionDelegate.urlSession(_:didReceive:completionHandler:)`
+  with no "accept self-signed" bypass. A self-signed appliance must have its CA
-  and accept `serverTrust` only when the user has explicitly toggled
+  installed on the device's OS trust store (MDM/configuration profile). Do not
-  "accept self-signed" for the configured host. Never default-on.
+  add a trust-bypass toggle.
 - **JSON**: `JSONSerialization` is simpler than `Codable` for the
   envelope pattern (`{status, data, details}`) because the `data`
   payload is heterogeneous. `Codable` works if you model each
@@ -700,11 +700,14 @@ MCMS APIs are typically reachable only from the operator's internal
 network. Test on cellular: many internal hosts won't be reachable,
 and your error UI needs to communicate that clearly.
 
-### 8.7 Self-signed cert prompt
+### 8.7 Self-signed certs → install the CA
 
-Make this a first-class onboarding step. The lab MCMS at
+There is no in-app "accept self-signed" prompt (removed for iOS/Android parity).
-`https://mcms.lab.svorka.net/` uses a self-signed cert, and any
+The lab MCMS at `https://mcms.lab.svorka.net/` uses a self-signed cert, so it is
-auto-rejection will silently break login.
+unreachable until that CA is installed on the device's OS trust store
+(MDM/configuration profile) — or the box is given a CA-trusted cert. Surface the
+resulting trust failure clearly in the error UI so it doesn't look like a silent
+login break.
 
 ---
 

README.md

@@ -33,11 +33,12 @@ Until then, this folder is the source of truth — edit here, copy into Xcode.
 - **App name / icon:** set the target's **Display Name → "PON Go"**, and drag
   `Assets.xcassets/AppIcon.appiconset` into the project's asset catalog (replacing
   the existing AppIcon).
-- **App Transport Security:** self-signed TLS is handled in code by
+- **App Transport Security:** the app uses **system TLS trust only** — there is no
-  `ServerTrustEvaluator` (no Info.plist change for HTTPS cert validation). For
+  self-signed bypass (parity with the Android client). A self-signed appliance must
-  plain **HTTP** hosts, add a *scoped* `NSAppTransportSecurity` →
+  have its CA installed on the device (MDM/configuration profile); otherwise the
-  `NSExceptionDomains` entry for that host in Info.plist — never
+  connection fails with `APIError.serverTrust`. For plain **HTTP** hosts, add a
-  `NSAllowsArbitraryLoads`.
+  *scoped* `NSAppTransportSecurity` → `NSExceptionDomains` entry for that host in
+  Info.plist — never `NSAllowsArbitraryLoads`.
 
 ## Verify off-device (no iOS SDK needed)
 

ServerTrustEvaluator.swift

@@ -4,22 +4,18 @@ import CryptoKit
 
 /// How to evaluate the server's TLS certificate.
 ///
-/// MCMS appliances usually present a self-signed or internal-CA certificate.
+/// The appliance must present a CA-trusted certificate — a real cert, or an
-/// Pick the *narrowest* policy that works for your deployment. Never ship
+/// internal CA installed on the device via MDM/configuration profile. There is
-/// `.allowSelfSignedForHost` against a host you don't control.
+/// deliberately NO "accept self-signed" escape hatch: the app never bypasses
+/// trust evaluation (parity with the Android client).
 enum ServerTrustPolicy: Equatable, Codable {
-    /// Default OS trust evaluation (use this if the box has a real cert or a
+    /// Default OS trust evaluation. The default policy and the only one the UI
-    /// CA your devices already trust via MDM/profile — strongly preferred).
+    /// produces.
     case system
 
-    /// Accept the server's certificate ONLY for this exact host, after a normal
-    /// trust evaluation that ignores the "untrusted root" failure. Scoped escape
-    /// hatch for self-signed boxes on a controlled mgmt network/VPN.
-    case allowSelfSignedForHost(String)
-
     /// Certificate/public-key pinning: trust only if the leaf's SHA-256
-    /// public-key hash (base64) matches one of these. The safest option for a
+    /// SubjectPublicKeyInfo hash (base64) matches one of these. Code-only (no UI);
-    /// fixed appliance — capture the hash once and pin it.
+    /// the strictest option for a fixed appliance — capture the hash once and pin it.
     case pinPublicKeySHA256([String], host: String)
 }
 
@@ -49,19 +45,6 @@ final class ServerTrustEvaluator: NSObject, URLSessionDelegate {
         case .system:
             completionHandler(.performDefaultHandling, nil)
 
-        case .allowSelfSignedForHost(let allowedHost):
-            // Scoped escape hatch for self-signed appliances on a controlled
-            // network. Because a self-signed root fails normal evaluation, this
-            // accepts ANY certificate the allowed host presents — including
-            // expired or name-mismatched ones. The only gate is the hostname, so
-            // use this only on hosts you control; prefer `.pinPublicKeySHA256`
-            // once you can capture the appliance's key hash.
-            guard host == allowedHost else {
-                completionHandler(.cancelAuthenticationChallenge, nil)
-                return
-            }
-            completionHandler(.useCredential, URLCredential(trust: trust))
-
         case .pinPublicKeySHA256(let pins, let pinnedHost):
             guard host == pinnedHost,
                   let leaf = Self.leafCertificate(from: trust),

SettingsView.swift

@@ -10,7 +10,6 @@ struct SettingsView: View {
     @State private var urlString = "https://"
     @State private var apiVersion = "v1"
     @State private var databaseId = ""
-    @State private var allowSelfSigned = false
 
     @State private var testPhase: LoadPhase = .idle
     @State private var testResult: String?
@@ -38,16 +37,6 @@ struct SettingsView: View {
                     .autocorrectionDisabled()
             }
 
-            Section {
-                Toggle("Allow self-signed certificate", isOn: $allowSelfSigned)
-            } header: {
-                Text("Security")
-            } footer: {
-                Text(allowSelfSigned
-                     ? "Trusts the certificate for this host only. Use only on a controlled management network/VPN. For production, install the CA via MDM and leave this off, or add public-key pinning in code."
-                     : "Uses standard system trust evaluation (recommended).")
-            }
-
             Section {
                 Button {
                     Task { await runTest() }
@@ -104,17 +93,15 @@ struct SettingsView: View {
         urlString = config.baseURL.absoluteString
         apiVersion = config.apiVersion
         databaseId = config.databaseId ?? ""
-        if case .allowSelfSignedForHost = config.trustPolicy { allowSelfSigned = true }
     }
 
     private func makeConfig() -> APIConfiguration? {
-        guard let url = Self.normalizedBaseURL(urlString), let host = url.host else { return nil }
+        // TLS uses system trust only (no self-signed bypass); trustPolicy defaults to .system.
-        let policy: ServerTrustPolicy = allowSelfSigned ? .allowSelfSignedForHost(host) : .system
+        guard let url = Self.normalizedBaseURL(urlString), url.host != nil else { return nil }
         return APIConfiguration(
             baseURL: url,
             apiVersion: apiVersion.trimmingCharacters(in: .whitespaces),
-            databaseId: databaseId.isEmpty ? nil : databaseId,
+            databaseId: databaseId.isEmpty ? nil : databaseId
-            trustPolicy: policy
         )
     }