fix(wifi): use OWE token for Enhanced Open security mode (#1073)#1142
Conversation
The codebase used the string 'Enhanced-Open' for the Enhanced Open (OWE) security mode, but firmware only accepts the TR-181 token 'OWE' and rejects 'Enhanced-Open' as an invalid enumeration (verified on device). This produced two latent app-side bugs, independent of the firmware issue where OWE is accepted with success:true yet not committed: - 6 GHz open mode sent 'Enhanced-Open', which firmware rejects with "Out of range or invalid enumeration". - OWE networks were never recognized as open, so the UI spuriously required a password (WifiQuickSetupSettings.isPasswordRequired) and blocked save. Replace all three 'Enhanced-Open' occurrences with 'OWE': - WifiNetworkUIModel.isOpenSecurity - WifiQuickSetupSettings.isPasswordRequired - _securityModeFor6GHz (6 GHz override + comment) Fix the test fixture that asserted the old invalid token and add an isOpenSecurity test group covering None / empty / OWE / WPA variants.
_securityModeFor6GHz had no direct test. Add a saveQuickSetup group asserting the exact ModeEnabled value sent to firmware: - 6 GHz + OWE / None (open) → 'OWE' - 6 GHz + WPA3-Personal → 'WPA3-Personal' - 2.4/5 GHz → selected mode written verbatim Guards against regressing to the invalid 'Enhanced-Open' token.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
AustinChangLinksys
left a comment
There was a problem hiding this comment.
🤖 Automated Review — Round 1 · f2af7bb..19bc7f0 (full)
Verdict: 💬 COMMENT — Critical issue found: saveAdvanced bypasses 6 GHz security override, allowing invalid security tokens to be written to firmware on 6 GHz networks.
| Conf. | Where | Issue (one-liner) | |
|---|---|---|---|
| 🔴 | 🟢High | usp_wifi_settings_service.dart:433–436 |
[both reviewers] saveAdvanced passes curr.securityMode raw to firmware — bypasses _securityModeFor6GHz() used by saveQuickSetup |
| 🟢High | wifi_ui.dart:108 |
[both reviewers] wifiDisplayValue has no case for 'OWE' — raw "OWE" shown in security mode display on WiFi status card |
|
| 🟢High | wifi_network_ui_model.dart:105 vs wifi_settings_settings.dart:55 |
[both reviewers] isOpenSecurity getter has zero callers in lib/; wifi_settings_settings.dart:55 re-implements the same open-mode check inline — dead code + DRY violation |
|
| 💡 | 🟢High | usp_wifi_settings_service_test.dart |
New 6 GHz tests cover saveQuickSetup path only — no test exercises saveAdvanced + 6 GHz band combination |
Confidence: 🟢High = code-verified · 🟡Med = located + reasoned, not fully confirmed · ⚪Low = speculative, please double-check.
Items marked [both reviewers] were independently flagged by two agents → higher confidence.
🔴 Critical Details
C-1 — [both reviewers] saveAdvanced bypasses 6 GHz security mode override (Confidence: 🟢High)
Location: lib/page/wifi_settings/services/usp_wifi_settings_service.dart:433–436 (HEAD 19bc7f0)
Evidence chain — saveQuickSetup (correct path) vs saveAdvanced (broken path):
// saveQuickSetup — lines 311–315 — CORRECT: calls _securityModeFor6GHz
final band = bandByApPath[p] ?? '';
final securityMode = _securityModeFor6GHz(
band: band,
selectedMode: pending.securityMode,
);// saveAdvanced — lines 433–436 — BUG: uses curr.securityMode verbatim, no override
securityModeEnabled:
securityChanged && curr.securityMode.isNotEmpty
? curr.securityMode // ← raw value, no _securityModeFor6GHz call
: null,// _securityModeFor6GHz — lines 635–641 — the guard that saveAdvanced skips
String _securityModeFor6GHz({required String band, required String selectedMode}) {
if (!band.contains('6')) return selectedMode;
const openModes = {'None', 'OWE', ''};
return openModes.contains(selectedMode) ? 'OWE' : 'WPA3-Personal';
}Why this is a bug: saveQuickSetup correctly calls _securityModeFor6GHz before writing Security.ModeEnabled to firmware, ensuring that on 6 GHz bands, only 'OWE' (open) or 'WPA3-Personal' are sent. saveAdvanced writes curr.securityMode verbatim — if a user in Advanced mode selects e.g. 'WPA2-Personal' on a 6 GHz network, the service sends 'WPA2-Personal' to firmware. Firmware either rejects the write (USP error) or silently applies an invalid/weaker security config.
Trigger condition: User opens WiFi Advanced settings, selects any non-WPA3 security mode on a 6 GHz band network, saves. saveAdvanced is called → curr.securityMode is written raw.
Fix:
// saveAdvanced — line 433: look up band the same way saveQuickSetup does
final band = bandByApPath[curr.accessPointInstancePath] ?? '';
securityModeEnabled:
securityChanged && curr.securityMode.isNotEmpty
? _securityModeFor6GHz(band: band, selectedMode: curr.securityMode)
: null,Note: bandByApPath is already built at the top of saveAdvanced — it just needs to be referenced here.
⚠️ Warning Details
W-1 — [both reviewers] wifiDisplayValue has no 'OWE' case — raw technical token shown in UI (Confidence: 🟢High)
lib/page/_shared/components/wifi_ui.dart:100–110:
String wifiDisplayValue(BuildContext context, String value) {
switch (value) {
case 'Auto': return loc(context).auto;
case 'None': return loc(context).none;
case 'Mixed': return loc(context).mixed;
default: return value; // ← 'OWE' falls here → UI shows "OWE"
}
}lib/page/wifi_settings/cards/usp_wifi_status_card.dart:177 calls wifiDisplayValue(context, ap.securityMode) — so after this PR, security-mode displays on the WiFi status card show raw "OWE" instead of a human-readable label like "Enhanced Open".
Fix: Add a case and a localization entry:
case 'OWE': return loc(context).enhancedOpen; // or 'Enhanced Open' as interimAdd "enhancedOpen": "Enhanced Open" to app_en.arb (and other arb files).
W-2 — [both reviewers] isOpenSecurity getter is dead code; open-mode check duplicated (Confidence: 🟢High)
lib/page/wifi_settings/models/wifi_network_ui_model.dart:105–107:
/// 'OWE' is the TR-181 token for Enhanced Open (firmware only accepts 'OWE').
bool get isOpenSecurity =>
securityMode == 'None' || securityMode.isEmpty || securityMode == 'OWE';Zero callers in lib/ (grep-verified). Meanwhile lib/page/wifi_settings/models/wifi_settings_settings.dart:55–56 re-implements the same logic inline:
// wifi_settings_settings.dart:55
final isOpen = securityMode == 'None' || securityMode == 'OWE';And usp_wifi_settings_service.dart:640 has a third independent copy:
const openModes = {'None', 'OWE', ''};Three sites defining the same set of "open modes" — any future token addition must be updated in all three places.
Fix: Make isOpenSecurity static and have wifi_settings_settings.dart:55 delegate to it:
// wifi_network_ui_model.dart
static bool isOpenSecurityMode(String mode) =>
mode == 'None' || mode.isEmpty || mode == 'OWE';
bool get isOpenSecurity => isOpenSecurityMode(securityMode);
// wifi_settings_settings.dart:55
final isOpen = WifiNetworkUIModel.isOpenSecurityMode(securityMode);✅ What looks good
- Token replacement complete (High): All three production files correctly updated:
wifi_network_ui_model.dart:107,wifi_settings_settings.dart:56,usp_wifi_settings_service.dart:640-641—'Enhanced-Open'→'OWE'throughout. - Docstring updated (High):
_securityModeFor6GHzdoc comment at line 630 updated to reflect'OWE'— no stale'Enhanced-Open'references remain in HEAD. - Test file updated (High):
usp_wifi_settings_state_test.dart:537–538correctly uses'OWE'in HEAD — no stale'Enhanced-Open'in test fixtures. - New 6 GHz override tests (High):
usp_wifi_settings_service_test.dartadds comprehensivesaveQuickSetup6 GHz test group (5 cases: OWE→OWE, None→OWE, WPA3→WPA3, 2.4GHz pass-through, 5GHz pass-through). Tests are well-structured and clearly guard against regression to invalid'Enhanced-Open'token. - Non-6 GHz transparency correct (High):
_securityModeFor6GHzreturnsselectedModeunchanged for non-6GHz bands — 2.4/5GHz behavior unaffected. - No hardcoded credentials (High): Changes are limited to security mode string constants — no auth tokens or secrets introduced.
isOpenSecuritylogic correct (High): The three-way check'None' || isEmpty || 'OWE'correctly covers all open-mode variants.
Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Automated — please sanity-check before merge.
PeterJhongLinksys
left a comment
There was a problem hiding this comment.
Automated PR Review
Verdict: 🔴 Comment — unresolved critical issue(s); not approving
Findings
- 🔴 Critical — none new (the sole critical is the prior
saveAdvancedbypass, still open — see "Already raised") - 🟡 Warning — Mislabeled/ineffective regression test at
test/page/wifi_settings/services/usp_wifi_settings_service_test.dart:1452: the case titled "6 GHz + WPA2-Personal selected → forced to WPA3-Personal" actually passesselectedMode: 'WPA3-Personal', so it only verifies WPA3→WPA3 and would still pass even if the non-open→WPA3 coercion branch were removed. Use'WPA2-Personal'as input to genuinely guard the override. - 🟢 Nit — none
Already raised (skipped to avoid duplication)
saveAdvancedwritescurr.securityModeverbatim, bypassing_securityModeFor6GHz(usp_wifi_settings_service.dart:433-436) — AustinChangLinksys, still open (PR head is the same commit reviewed; code unchanged). This is the unresolved critical driving the Comment verdict. Note: reachability is bounded — the Advanced-mode dialog populates from each network's own band-specificsupportedSecurityModes(wifi_network_card.dart:224), so an invalid 6 GHz mode is only selectable if firmware advertises it (e.g.Noneon 6 GHz); but the two save paths remaining inconsistent is a real gap worth closing in this PR.wifiDisplayValuehas no'OWE'case → raw "OWE" shown on the status card (wifi_ui.dart:100-110,usp_wifi_status_card.dart:177) — AustinChangLinksys, still open.isOpenSecuritygetter has zerolib/callers; open-mode check duplicated across three sites — AustinChangLinksys, still open (pre-existing; DRY nit).- No
saveAdvanced+ 6 GHz test coverage — AustinChangLinksys, still open.
Verified
- Token replacement
'Enhanced-Open'→'OWE'is complete and correct across all three production sites (wifi_network_ui_model.dart:107,wifi_settings_settings.dart:56,usp_wifi_settings_service.dart:639-640); doc comment updated too. saveQuickSetupcorrectly routes the selected mode through_securityModeFor6GHzbefore writingSecurity.ModeEnabled(usp_wifi_settings_service.dart:311-315,327).- USP errors are mapped to
ServiceErrorviamapUspErrorToServiceError()in bothsaveQuickSetupandsaveAdvancedcatch blocks — constitution Article XIII respected. - SET payload uses the codegen
WiFiAccessPointUpdatemodel (securityModeEnabled); no hand-editing of.g.dart. - New
isOpenSecuritytest group (None / empty / OWE / WPA2 / WPA3-Transition) asserts correct open/non-open classification; other security modes (WPA2/WPA3) are unaffected by the change. - The other four new 6 GHz tests (OWE→OWE, None→OWE on 6 GHz; OWE and None written verbatim on 2.4/5 GHz) are correct and meaningful.
saveAdvanced wrote curr.securityMode verbatim while saveQuickSetup routed it through _securityModeFor6GHz. The two save paths could therefore send different security modes to firmware, and on 6 GHz saveAdvanced could send a non-WPA3 (invalid) mode. - saveAdvanced: route securityModeEnabled through _securityModeFor6GHz (band from curr.band), matching saveQuickSetup. Still gated on securityChanged so enable-only toggles never re-write the mode. - wifiDisplayValue: add an 'OWE' case returning 'Enhanced Open' so the status card and selector show the Wi-Fi standard label instead of the raw token (like WPA2/WPA3-Personal, it is a technical term, not l10n'd). - tests: the 6 GHz "WPA2-Personal to WPA3-Personal" case passed 'WPA3-Personal' as input, an identity that would pass even without the override; pass 'WPA2-Personal' to exercise the non-open to WPA3 coercion. Add a saveAdvanced 6 GHz override group, and use a distinct baseline mode so the security diff always fires.
|
Pushed Fixed
Not changed
All wifi_settings tests pass (179); |
AustinChangLinksys
left a comment
There was a problem hiding this comment.
🤖 Automated Review — Round 2 · 19bc7f0..2aa7fe0 (incremental)
Verdict: ✅ APPROVE — All Round 1 issues resolved; one new low-confidence defensive-gap warning, no blocking issues.
| Conf. | Where | Issue (one-liner) | |
|---|---|---|---|
| ⚪Low | usp_wifi_settings_service.dart:428 |
isNotEmpty guard short-circuits _securityModeFor6GHz when securityMode == ''; on 6 GHz, empty string maps to 'OWE' inside the function but never reaches it |
|
| 💡 | 🟢High | usp_wifi_settings_service_test.dart |
saveAdvanced 6 GHz tests missing WPA3-Personal -> WPA3-Personal passthrough case |
| 💡 | 🟡Med | lib/page/_shared/components/wifi_ui.dart:108 |
'OWE' -> 'Enhanced Open' mapping has no unit test |
Confidence: 🟢High = code-verified · 🟡Med = located + reasoned, not fully confirmed · ⚪Low = speculative, please double-check.
Items marked [both reviewers] were independently flagged by two agents -> higher confidence.
⚠️ Warning Details
W-NEW-1 — isNotEmpty guard silently suppresses OWE coercion when securityMode is empty on 6 GHz (Confidence: ⚪Low)
lib/page/wifi_settings/services/usp_wifi_settings_service.dart:428–433 (head 2aa7fe0):
final securityMode = securityChanged && curr.securityMode.isNotEmpty
? _securityModeFor6GHz(
band: curr.band,
selectedMode: curr.securityMode,
)
: null;The outer curr.securityMode.isNotEmpty guard was inherited from the original code (which prevented sending '' verbatim to firmware — a valid concern). However, _securityModeFor6GHz (line 647) already handles empty string:
const openModes = {'None', 'OWE', ''};
return openModes.contains(selectedMode) ? 'OWE' : 'WPA3-Personal';This means an AP whose firmware-stored securityMode is '' on 6 GHz would yield securityModeEnabled: null (nothing sent) instead of 'OWE'. By contrast, saveQuickSetup passes curr.securityMode unconditionally to _securityModeFor6GHz when securityChanged.
Practical impact is low — firmware normally returns a non-empty mode string. This is a defensive gap rather than an active regression, and no existing data flow produces this scenario. The three new tests all use non-empty inputs and pass correctly.
Fix (if desired):
// Let _securityModeFor6GHz handle '' internally:
final securityMode = securityChanged
? _securityModeFor6GHz(band: curr.band, selectedMode: curr.securityMode)
: null;_securityModeFor6GHz already safely maps '' on non-6 GHz bands (returns '' unchanged), so removing the isNotEmpty guard is safe.
(low confidence — please verify before acting)
✅ What looks good
- C-1 resolved (High) —
saveAdvanced6 GHz override wired correctly:usp_wifi_settings_service.dart:428–433now routescurr.securityModethrough_securityModeFor6GHz(band: curr.band, ...), andsecurityModeEnabledreceives the coerced value. The raw bypass is gone. - W-1 resolved (High) —
wifiDisplayValueOWE label added:wifi_ui.dart:108–111addscase 'OWE': return 'Enhanced Open'— the firmware token is correctly mapped to the Wi-Fi standard label. Comment explains the rationale (technical term, not localized, consistent with WPA2/WPA3-Personal). - Suggestion resolved (High) —
saveAdvanced6 GHz tests added: NewcapturedModeEnabledhelper + 4 test cases covering OWE->OWE, None->OWE, WPA2->WPA3-Personal coercion, and 5 GHz verbatim passthrough. All four cases are semantically correct and genuinely guard the fix. - W-2 status — pre-existing, author accepted:
isOpenSecuritydead code / triple open-mode check duplication acknowledged as pre-existing; left for a future consolidation. Reasonable. - Mislabeled
saveQuickSetuptest fixed (High):usp_wifi_settings_service_test.dart:1517now passes'WPA2-Personal'as input instead of the identity'WPA3-Personal'— test now genuinely exercises the non-open -> WPA3 coercion branch. _securityModeFor6GHzis correct in head (High):openModes = {'None', 'OWE', ''}withreturn openModes.contains(selectedMode) ? 'OWE' : 'WPA3-Personal'— handles all open-mode variants correctly.- Docstring updated (High):
usp_wifi_settings_service.dart:636–640— doc comment correctly describes OWE as the TR-181 token and documents the three routing rules. - No hardcoded secrets, no injection vectors (High): Changes limited to string constants and routing logic.
Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Automated — please sanity-check before merge.
Summary
Fixes the app-side half of #1073.
Issue #1073 — In WiFi Settings, selecting the Enhanced Open (OWE) security mode and saving reports no error, but the mode silently reverts to its previous value. The user believes the change was saved when it was not.
Investigation (log analysis + on-device probing against Pinnacle 2.0, FW
1.2.2.26062820) showed the failure spans two layers: an app-side token bug and a firmware commit bug. This PR fixes the app side.Root cause — two layers
App side (fixed in this PR)
The codebase used the string
'Enhanced-Open'for the Enhanced Open mode, but firmware only accepts the TR-181 tokenOWEand rejects'Enhanced-Open'with"Out of range or invalid enumeration"(confirmed on device). This produced two latent bugs:_securityModeFor6GHz) sent'Enhanced-Open', which firmware rejects outright.isOpenSecurity/isPasswordRequiredcompared against'Enhanced-Open', so a real OWE network was treated as non-open — the UI spuriously required a password and disabled Save.Firmware side (NOT in this PR — still open)
Even with the correct
OWEtoken, firmware accepts the SET withsuccess: true(echoingOWE) but does not commit it — the post-save readback reverts to the previous mode. On-device probing confirmed:SET {ModeEnabled: OWE}→success: true, readback reverts to the old mode.EncryptionModeis read-only, so the client cannot force it; no client-side field combination makes OWE stick.This is a firmware bug and cannot be fixed from the app.
What this PR changes
Replaces all three
'Enhanced-Open'occurrences with'OWE':WifiNetworkUIModel.isOpenSecurityWifiQuickSetupSettings.isPasswordRequired_securityModeFor6GHz(6 GHz override + doc comment)Tests:
isOpenSecuritygroup (None / empty / OWE / WPA variants).saveQuickSetup6 GHz override group asserting the exactModeEnabledvalue sent to firmware, guarding against a regression to'Enhanced-Open'.Impact
After this PR the app does everything correctly on its side:
OWEtoken on all bands.However, #1073 will remain reproducible until the firmware commit bug is fixed — the app now sends a valid
OWE, but firmware still silently drops it. The remaining work is on the firmware team.Verification
flutter test test/page/wifi_settings/— all pass (175 tests).'Enhanced-Open'.flutter analyze— no new issues.