Prevent Range Test from running on public/default channel#5986
Conversation
Blocks enabling Range Test when the primary channel is unencrypted or using the well-known default key (PSK < 2 bytes). Users who intentionally want Range Test on a public channel can still do so via the CLI. Addresses reviewer feedback: - Hoist repeated condition into canConfigure val (no more duplication) - Keep save enabled so a CLI-enabled Range Test can still be turned off through the app via additionalDirtyCheck forcing the footer visible
Blocks enabling Range Test when the primary channel uses an empty PSK (cleartext) or the well-known 1-byte default shortstring code. Both are effectively public channels that would flood the shared mesh with test traffic. Users who intentionally want Range Test on a public channel can still enable it via the Meshtastic CLI. Addresses reviewer feedback: - Deduplicate repeated condition; hoist into canConfigure val - Keep screen-level enabled=connected so a CLI-enabled Range Test can still be cleared through the app (additionalDirtyCheck surfaces the save footer when the device has enabled=true on a public channel)
jamesarich
left a comment
There was a problem hiding this comment.
The PSK < 2 heuristic and the save-time enforcement are correct, and the dedup into canConfigure reads well. One UX change before merge — see the inline suggestion.
The corrective flow (device with Range Test enabled via CLI on a public channel) routes the user through a control that doesn't behave like one: the Enabled switch is grayed on, and the only way to disable is tapping Save, which silently forces enabled = false. That hidden side effect is the issue.
| title = stringResource(Res.string.range_test_enabled), | ||
| checked = formState.value.enabled, | ||
| enabled = state.connected, | ||
| enabled = canConfigure, |
There was a problem hiding this comment.
Let the Enabled switch turn off on a public channel (just not on), so the corrective action is the obvious one — flip the switch — instead of a Save button that silently disables:
| enabled = canConfigure, | |
| enabled = canConfigure || formState.value.enabled, |
Behavior with this:
- public + off → grayed off, can't enable ✅
- public + on (CLI-set) → switch is live; flipping it off makes the form dirty → footer Save persists
false✅ - after toggling off it re-grays (can't flip back on) ✅
The footer then appears naturally via isDirty, so additionalDirtyCheck (line 56) can be dropped. I'd keep the onSave force-false as cheap defense-in-depth — it also covers the brief load race where channelList is empty and isPublicPrimaryChannel is transiently true.
When a device has Range Test enabled via CLI on a public channel, the Enabled switch is now live (canConfigure || formState.value.enabled) so the user can flip it off directly. The save footer appears naturally via isDirty; additionalDirtyCheck is no longer needed and is removed. The onSave force-false is kept as cheap defense-in-depth to cover the brief load race where channelList is empty and isPublicPrimaryChannel is transiently true.
|
Applied, thank you for the reccomendation. Changed the Enabled switch to |
Summary
Changes from previous PR #3816
Test plan