fix(waypoint): reject non-owner modification of a stored locked waypoint

[jamesarich]

Why

A locked waypoint (locked_to != 0) is meant to be editable only by the node it is
locked to. Inbound waypoint handling only validated the incoming packet's locked_to
against its sender (locked_to == 0 || locked_to == from) — it never consulted the
already-stored waypoint's owner. So a non-owner could hijack someone else's locked
waypoint by re-sending an update for the same waypoint id with:

• locked_to = 0 — silently unlocks and overwrites the stored waypoint, or
• locked_to = <their own num> — takes over ownership.

Both slip past the inbound-only check. This is a pre-existing protocol-facing gap (called
out during CodeRabbit review of #6344 and deferred there as out-of-scope, since #6344 was a
straight refactor of the existing check). This PR closes it.

What

MeshDataHandlerImpl.handleWaypoint now, before persisting an update, reads the
currently-stored waypoint for that id and rejects the update when the stored copy is locked
to a different node than the sender — including inbound locked_to = 0 unlock attempts.

• 🛠️ Load the stored waypoint via PacketRepository.getWaypoints() collapsed through
  activeWaypointPackets(now) (newest-per-id, expired dropped) — the same normalisation the
  map UI and geofence engine already use, so the three can't drift.
• 🛠️ Reuse Waypoint.isModifiableBy (the shared locked_to helper added in fix(map): allow editing/deleting your own locked waypoint #6344) for the
  stored-owner check as well, so the inbound and persisted checks share one source of truth.
• 🛠️ The stored-owner read and the persist run inside one launchSessionWork lease
  (persistDataPacket extracted from rememberDataPacket), so the check sees a committed
  snapshot rather than nesting a second lease across two coroutines.
• 🧹 rememberDataPacket's body split into persistDataPacket (behaviour-preserving; the
  duplicate-skip return semantics are unchanged).

Preserved (not dropped): new-waypoint creation, owner unlock/edit, updates to unlocked
waypoints, and the inbound isModifiableBy rule (a node can't lock a waypoint to a third party).

Behaviour matrix

stored	incoming from	incoming locked_to	result
none	any	0 or self	persisted (creation)
none	X	Y≠X	dropped (inbound rule)
unlocked	any	0 or self	persisted
locked→owner	owner	0 (unlock) / owner (edit)	persisted
locked→owner	attacker	0 (unlock)	dropped ← fix
locked→owner	attacker	attacker (takeover)	dropped ← fix

Testing performed

• New regression tests in core/data MeshDataHandlerTest covering every row above (7 cases).
• Full local baseline green: spotlessApply spotlessCheck detekt assembleDebug test allTests kmpSmokeCompile.
• Not yet exercised on a real mesh / replay — see testing note below.

Testing note

Unit-tested and baseline-green (above), but not yet validated on a real mesh / replay
(burningmesh-replay sandbox) to confirm no legitimate cross-client waypoint updates are
dropped — worth a maintainer's on-device check before merge. Notes:

• Enforcement is client-side (locked_to is advisory on the wire); this protects the
  local DB/view, matching how the map UI already gates edits.
• Known, accepted residuals (documented, not fixed here):
  • Node renumber (fw 2.8 identity migration): after a renumber the owner's from no
    longer matches the old locked_to, so their edits would be dropped — but the map UI
    already blocks the owner from editing in that state (locked_to == myNodeNum), so no new
    inconsistency is introduced. Waypoints also expire.
  • Simultaneous create/attack race: getWaypoints() reflects committed rows, so a
    replay landing before the owner's create commits isn't a hard transactional lock — but it
    self-heals on the owner's next re-broadcast (their locked copy becomes newest again).

Follow-up to #6344 / thread
#6344 (comment)

Rebased onto main after #6344 merged, so the inbound check and this PR's stored-owner
check both go through the same Waypoint.isModifiableBy helper.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

Waypoint and packet handling

Layer / File(s)	Summary
Persisted waypoint owner enforcement core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerImpl.kt, core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerTest.kt	Waypoint updates normalize active packets and consult persisted ownership before persistence. Tests cover owner, non-owner, unlocked, creation, and invalid-lock scenarios.
Session-scoped packet persistence core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerImpl.kt	rememberDataPacket delegates to persistDataPacket, which handles deduplication, filtering, insertion, and conditional notifications inside session work.

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

Possibly related PRs

• meshtastic/Meshtastic-Android#6344: Updates waypoint lock and ownership checks across shared logic and UI.

Suggested reviewers: jeremiah-k

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: blocking non-owner edits to a stored locked waypoint.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[jamesarich]

@coderabbitai review

[coderabbitai]

✅ 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.