Skip to content

feat(settings): add packet authenticity policy - #6178

Merged
jamesarich merged 3 commits into
meshtastic:mainfrom
RCGV1:codex/packet-auth-policy
Jul 22, 2026
Merged

feat(settings): add packet authenticity policy#6178
jamesarich merged 3 commits into
meshtastic:mainfrom
RCGV1:codex/packet-auth-policy

Conversation

@RCGV1

@RCGV1 RCGV1 commented Jul 9, 2026

Copy link
Copy Markdown
Member

Summary

Adds the shared Android/Desktop Security setting for the device-enforced packet authenticity policy defined in design#121:

  • Compatible, Balanced, and Strict choices backed directly by SecurityConfig.packet_signature_policy.
  • Compatible is the protobuf zero/default selection; Balanced remains the recommended opt-in policy.
  • Tri-state DeviceMetadata.has_xeddsa capability handling: supported firmware can configure the policy, explicit lack of support is explained, and missing metadata remains an unknown or disconnected state.
  • Confirmation before Strict, including the compatibility impact for older firmware, ham or licensed nodes, oversized packets, and authenticated PKI direct messages.
  • Localized shared Compose UI with no app-local packet filtering.

Closes #6176.

Visual validation

Rendered screenshot-test captures below cover each required state in both light and dark themes. These committed reference assets are intentional regression tests; the links are to the immutable PR-head commit, not local files. This isolated preference has no separate compact or desktop layout branch, so the light/dark component captures are the applicable coverage.

State Light Dark
Default (Compatible) Compatible default - light Compatible default - dark
Balanced Balanced - light Balanced - dark
Strict Strict - light Strict - dark
Strict confirmation Strict confirmation - light Strict confirmation - dark
Unavailable capability Unavailable capability - light Unavailable capability - dark

Validation

  • Rebased on current main, including protobuf snapshot 2.7.26.119-g164b6e2-SNAPSHOT after protobufs#983 merged.
  • :feature:settings:jvmTest :feature:settings:spotlessCheck :feature:settings:detekt passed.
  • :feature:settings:allTests :desktopApp:compileKotlin passed.
  • :core:ui:spotlessCheck :core:ui:detekt passed.
  • :screenshot-tests:updateDebugScreenshotTest :screenshot-tests:validateDebugScreenshotTest passed.
  • The default test confirms Compatible = 0; focused settings tests cover mapping, selection, Strict confirmation, disconnect or capability-loss races, unknown capability, and unsupported firmware.

protobufs#983 and firmware#10967 are merged. No radio hardware verification has been performed; keep this PR draft until a capable device completes a read/write/read-back round trip.

Summary by CodeRabbit

  • New Features
    • Added packet authenticity controls to Security Settings.
    • Choose between Compatible, Balanced, and Strict protection modes.
    • Strict mode requires confirmation before enabling and requires cryptographic authentication for mesh packets.
    • Settings clearly indicate when authenticity controls are unavailable or unsupported by the connected device.
    • Added localized descriptions and guidance for each protection level.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a shared (Android/Desktop) “Packet authenticity” security setting that writes directly to SecurityConfig.packet_signature_policy, including Strict-mode confirmation UX, capability gating via device metadata, and test/screenshot coverage.

Changes:

  • Introduces PacketAuthenticitySetting Compose UI with Strict confirmation dialog and “unsupported” state messaging.
  • Wires the setting into SecurityConfigScreenCommon and adds previews + screenshot-test entries.
  • Adds common UI tests validating default mapping, selection behavior, and Strict confirmation dismissal on connection/support loss.

Reviewed changes

Copilot reviewed 7 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
screenshot-tests/src/screenshotTest/kotlin/org/meshtastic/screenshots/feature/SettingsScreenshotTests.kt Adds screenshot-test composables for the new Packet authenticity setting states.
feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/component/PacketAuthenticitySettingTest.kt Adds UI tests covering policy selection, Strict confirmation, and unsupported behavior.
feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/SecurityConfigScreen.kt Integrates the Packet authenticity selector into the Security config screen.
feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/PacketAuthenticitySetting.kt Implements the dropdown selector + Strict confirmation dialog logic.
feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/PacketAuthenticityPreviews.kt Adds previews for supported/unsupported states and the Strict confirmation dialog.
core/resources/src/commonMain/composeResources/values/strings.xml Adds localized strings for the Packet authenticity setting UI.
.skills/compose-ui/strings-index.txt Updates the string index with the new packet_authenticity* keys.

@RCGV1
RCGV1 force-pushed the codex/packet-auth-policy branch from 995d342 to 0937f65 Compare July 10, 2026 00:06
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds packet authenticity localization, a shared Compose selector for compatible/balanced/strict policies, strict-mode confirmation and capability gating, SecurityConfigScreen integration, dropdown test tagging, behavior tests, and light/dark preview screenshot coverage.

Changes

Packet authenticity policy

Layer / File(s) Summary
Packet authenticity resources
.skills/compose-ui/strings-index.txt, core/resources/.../strings.xml
Adds labels, summaries, strict confirmation text, and unsupported-state strings for packet authenticity modes.
Policy selector and security screen integration
core/ui/.../DropDownPreference.kt, feature/settings/.../PacketAuthenticitySetting.kt, feature/settings/.../SecurityConfigScreen.kt
Adds dropdown test tags, policy selection and mapping, strict confirmation behavior, capability/connection gating, and configuration write-through.
Selector behavior validation
feature/settings/src/commonTest/.../PacketAuthenticitySettingTest.kt
Tests defaults, policy updates, strict confirmation, state loss, unsupported capability, disabled interaction, and summaries.
Preview and screenshot coverage
feature/settings/.../PacketAuthenticityPreviews.kt, screenshot-tests/.../SettingsScreenshotTests.kt
Adds previews and screenshot entry points for all policies, unsupported state, and strict confirmation.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SecurityConfigScreen
  participant PacketAuthenticitySetting
  participant MeshtasticResourceDialog
  SecurityConfigScreen->>PacketAuthenticitySetting: render policy and capability state
  PacketAuthenticitySetting->>MeshtasticResourceDialog: request strict confirmation
  MeshtasticResourceDialog-->>PacketAuthenticitySetting: confirm or dismiss
  PacketAuthenticitySetting-->>SecurityConfigScreen: update packet signature policy
Loading

Possibly related issues

  • meshtastic/design issue 121 — Defines the packet authenticity selector modes, gating, strict confirmation, and localized UI implemented here.
  • meshtastic/web issue 1260 — Describes the corresponding packet authenticity policy control and validation coverage.
  • meshtastic/Meshtastic-Apple issue 2065 — Covers the same cross-platform policy mapping, gating, confirmation, tests, and screenshots.
  • meshtastic/device-ui issue 340 — Describes the packet authenticity selector, unsupported handling, strict confirmation, and focused tests.

Suggested labels: desktop

Suggested reviewers: jamesarich, thebentern

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR implements the shared packet authenticity selector with confirmations, tests, screenshots, and a firmware-backed write path.
Out of Scope Changes check ✅ Passed The changes stay focused on packet authenticity UI, localization, previews, tests, and supporting dropdown semantics.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding a packet authenticity policy setting in settings.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@RCGV1

RCGV1 commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/component/PacketAuthenticitySettingTest.kt (2)

195-209: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Assert the gating contract directly.

These tests do not directly verify that the selector is disabled. Additionally, the “unknown capability” case combines connected = false with supported = null, so it cannot distinguish connection gating from capability gating. Add assertIsNotEnabled() on the clickable selector and separate cases for connected = false, supported = true and connected = true, supported = null, matching canConfigurePolicy = connected && supported == true in PacketAuthenticitySetting.kt.

Also applies to: 213-226

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/component/PacketAuthenticitySettingTest.kt`
around lines 195 - 209, The PacketAuthenticitySetting tests do not directly
assert selector enablement and conflate connection and capability gating. In the
tests around unsupported and unknown capability, target the clickable selector
and assertIsNotEnabled(), then add separate cases for connected = false with
supported = true and connected = true with supported = null, matching
canConfigurePolicy = connected && supported == true in
PacketAuthenticitySetting.

186-188: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid relying on popup node ordering.

Selecting onAllNodesWithText(strictLabel)[1] assumes the menu item is always the second matching node. Use a stable test tag or semantic property for the strict option instead, so changes to the dropdown’s semantics do not break this test.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/component/PacketAuthenticitySettingTest.kt`
around lines 186 - 188, The test currently selects the strict packet
authenticity option by assuming it is the second matching node. In the test
around strictLabel selection, identify or add a stable test tag or semantic
property for the strict menu item and use it to target that option directly
instead of indexing onAllNodesWithText(strictLabel)[1].
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/component/PacketAuthenticitySettingTest.kt`:
- Around line 195-209: The PacketAuthenticitySetting tests do not directly
assert selector enablement and conflate connection and capability gating. In the
tests around unsupported and unknown capability, target the clickable selector
and assertIsNotEnabled(), then add separate cases for connected = false with
supported = true and connected = true with supported = null, matching
canConfigurePolicy = connected && supported == true in
PacketAuthenticitySetting.
- Around line 186-188: The test currently selects the strict packet authenticity
option by assuming it is the second matching node. In the test around
strictLabel selection, identify or add a stable test tag or semantic property
for the strict menu item and use it to target that option directly instead of
indexing onAllNodesWithText(strictLabel)[1].

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 22f4f19d-9dbc-4cf3-9912-a3d52e89a273

📥 Commits

Reviewing files that changed from the base of the PR and between b2184f9 and 0937f65.

⛔ Files ignored due to path filters (10)
  • screenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/SettingsScreenshotTestsKt/ScreenshotPacketAuthenticityBalanced_Dark_d19fbf1f_0.png is excluded by !**/*.png, !**/*.png
  • screenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/SettingsScreenshotTestsKt/ScreenshotPacketAuthenticityBalanced_Light_b29dc7a7_0.png is excluded by !**/*.png, !**/*.png
  • screenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/SettingsScreenshotTestsKt/ScreenshotPacketAuthenticityCompatible_Dark_d19fbf1f_0.png is excluded by !**/*.png, !**/*.png
  • screenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/SettingsScreenshotTestsKt/ScreenshotPacketAuthenticityCompatible_Light_b29dc7a7_0.png is excluded by !**/*.png, !**/*.png
  • screenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/SettingsScreenshotTestsKt/ScreenshotPacketAuthenticityStrictConfirmation_Dark_d19fbf1f_0.png is excluded by !**/*.png, !**/*.png
  • screenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/SettingsScreenshotTestsKt/ScreenshotPacketAuthenticityStrictConfirmation_Light_b29dc7a7_0.png is excluded by !**/*.png, !**/*.png
  • screenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/SettingsScreenshotTestsKt/ScreenshotPacketAuthenticityStrict_Dark_d19fbf1f_0.png is excluded by !**/*.png, !**/*.png
  • screenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/SettingsScreenshotTestsKt/ScreenshotPacketAuthenticityStrict_Light_b29dc7a7_0.png is excluded by !**/*.png, !**/*.png
  • screenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/SettingsScreenshotTestsKt/ScreenshotPacketAuthenticityUnsupported_Dark_d19fbf1f_0.png is excluded by !**/*.png, !**/*.png
  • screenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/SettingsScreenshotTestsKt/ScreenshotPacketAuthenticityUnsupported_Light_b29dc7a7_0.png is excluded by !**/*.png, !**/*.png
📒 Files selected for processing (7)
  • .skills/compose-ui/strings-index.txt
  • core/resources/src/commonMain/composeResources/values/strings.xml
  • feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/PacketAuthenticityPreviews.kt
  • feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/PacketAuthenticitySetting.kt
  • feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/SecurityConfigScreen.kt
  • feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/component/PacketAuthenticitySettingTest.kt
  • screenshot-tests/src/screenshotTest/kotlin/org/meshtastic/screenshots/feature/SettingsScreenshotTests.kt

@RCGV1
RCGV1 force-pushed the codex/packet-auth-policy branch from 0937f65 to fa45449 Compare July 10, 2026 00:21
@RCGV1

RCGV1 commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

Addressed the latest CodeRabbit test review in fa4544976:

  • the selector now exposes an explicit disabled semantic/test tag;
  • unsupported, unknown-capability, and disconnected-with-known-support are separate cases and directly assert disabled state;
  • unknown metadata remains distinct from explicit lack of firmware support.

I did not expand the shared DropDownPreference API solely to tag one popup row. Its current API exposes no stable per-item modifier, and the remaining exact-label/index assertion is confined to a focused test with a fixed two-node state; it does not affect production behavior.

spotlessKotlinApply and git diff --check pass. Compilation remains dependent on protobufs#983 publishing the new generated symbols.

@RCGV1

RCGV1 commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

⚠️ JUnit XML file not found

The CLI was unable to find any JUnit XML files to upload.
For more help, visit our troubleshooting guide.

@RCGV1

RCGV1 commented Jul 12, 2026

Copy link
Copy Markdown
Member Author

UI snapshot coverage is included in this draft.

Balanced policy:
Balanced policy

Strict-mode confirmation:
Strict confirmation

@jamesarich

Copy link
Copy Markdown
Collaborator

One thing to reconsider on the merits before this ships: the UI presents Balanced as the Recommended default, but per design#121 pre-2.8 firmware strips the Data field-10 signature on relayed packets during decode/re-encode, so Balanced silently drops legitimate relayed traffic from older nodes. Recommending the setting that causes the drops seems risky until 2.8 is widely deployed — worth reconsidering either the "Recommended" copy or the default until then.

Noting for context that CI is red only because it pins the unpublished protobufs SNAPSHOT (depends on protobufs#983) — that's an external block, fine for a draft. The code and test coverage themselves look solid.

@jamesarich
jamesarich force-pushed the codex/packet-auth-policy branch from 00e6921 to d5bdfb5 Compare July 19, 2026 17:58
@jamesarich

Copy link
Copy Markdown
Collaborator

Holding this until it's re-aligned with the merged schema — the default policy decision changed upstream after this PR was written.

I rebased this branch onto main and fixed the dependency pin to 2.7.26.115-gb6ad0e5-SNAPSHOT (the snapshot built from the protobufs#983 merge), so CI can now actually build instead of failing at dependency resolution.

The blocker is that the default was reversed upstream during the protobufs#983 review. thebentern requested the change, citing user reports of pre-2.8 firmware eating signed packets on decode/re-encode (a signed NodeInfo getting blackholed by a legacy node in the rebroadcast chain). The merged config.proto now has:

  • PACKET_SIGNATURE_POLICY_COMPATIBLE = 0 ← the deliberate default
  • PACKET_SIGNATURE_POLICY_BALANCED = 1
  • PACKET_SIGNATURE_POLICY_STRICT = 2

This PR still treats Balanced as the default and markets it as Recommended (packet_authenticity_balanced_summary), which now contradicts both the merged schema and the maintainer decision. Because the enum both renumbered and changed its zero value, please:

  1. Make Compatible the surfaced default, matching the proto's zero value.
  2. Drop the Recommended framing from Balanced (or move it to Compatible if a recommendation is wanted).
  3. Re-verify the enum mapping and the policy tests against the merged .115 proto — any test that assumed Balanced was the default/zero value needs updating.
  4. Complete the radio hardware round-trip this draft is gated on.

Once those are done and CI is green I'm happy to take another look.

@RCGV1
RCGV1 force-pushed the codex/packet-auth-policy branch from d5bdfb5 to 9a4936d Compare July 21, 2026 20:01
@RCGV1
RCGV1 marked this pull request as ready for review July 22, 2026 01:18

@jamesarich jamesarich left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. The UI is clean and correctly scoped: pure Security setting bound to SecurityConfig.packet_signature_policy with proper tri-state has_xeddsa handling (canConfigurePolicy = connected && supported == true), Strict-requires-confirmation flow, and no app-side packet filtering. Good test + screenshot coverage across all states, and CI is green.

One flag, not blocking merge here since we're queuing intentionally: your own validation note asks to keep this draft until a device completes a read/write/read-back round trip, and no radio-hardware verification has been done. Worth a follow-up on-device sanity check post-merge given this writes a firmware config field.

@jamesarich
jamesarich added this pull request to the merge queue Jul 22, 2026
@jamesarich
jamesarich removed this pull request from the merge queue due to a manual request Jul 22, 2026
@jamesarich
jamesarich merged commit c9506d2 into meshtastic:main Jul 22, 2026
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-generated enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[2.8.0] Configure packet authenticity policy

3 participants