feat(core): develop-only lockdown fields (disable, max_session_seconds, DISABLED)#7
Draft
jamesarich wants to merge 16 commits into
Draft
feat(core): develop-only lockdown fields (disable, max_session_seconds, DISABLED)#7jamesarich wants to merge 16 commits into
jamesarich wants to merge 16 commits into
Conversation
…id hard cutover Remote admin (correctness — previously rejected by firmware 2.5+): - Route remote ADMIN_APP packets over PKC when both nodes have published keys (pki_encrypted + target public_key, channel 0), falling back to a channel named "admin"; priority defaults to RELIABLE. One choke point (prepareOutboundAdminPacket) shared by the RPC, ACK'd-send, and fire-and-forget paths. - Cache session passkeys per node (each node issues its own in every admin response) instead of one global slot that cross-contaminated concurrent admin sessions and stamped the local passkey onto remote targets. The SessionKeyExpired single-shot retry now re-seeds against the target node, whose passkey is the one the replay needs. Inbound coverage: - Surface MQTT client-proxy, XModem, and FileInfo frames as typed MeshEvents instead of dropping them with a ProtocolWarning (outbound already works via sendRaw). - Buffer mesh packets that arrive mid-handshake (drop-oldest at 64, observable via PacketsDropped) and flush them through the normal packet pipeline at Ready — live traffic interleaved with the config drain was silently lost. - Fast-fail QueueStatus res != 0 as SendFailure.QueueRejected (and fail any pending RPC sharing the wire id) instead of waiting out the full ACK timer for a packet the firmware already rejected. Ergonomics: - sendText(replyId) for threaded replies. - Document that DeviceStorage.loadNodes() is host-facing (the engine reseeds the node DB from the handshake and never calls it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
…on Android Android keeps the ATT MTU at the BLE minimum (23) unless the central requests otherwise, capping write-without-response payloads at 20 bytes — nearly every real ToRadio frame (setConfig, sendText) exceeds that, so the transport was effectively broken on Android (conformance had only ever run on JVM/macOS, where CoreBluetooth auto-negotiates). The BleTransport(address) factory now installs a best-effort post-connect hook that requests MTU 517 and CONNECTION_PRIORITY_HIGH for the 30-second handshake window before downgrading to Balanced (mirrors the Android reference client). The hook is internal to the transport and runs once per link establishment, including auto-reconnect cycles. Also bumps Kable 0.42.0 -> 0.43.0 to align with Meshtastic-Android. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
…e audit Audited the engine against firmware main (PhoneAPI.cpp, AdminModule.cpp, MeshService.cpp @ 4e7181c79) and protobufs v2.7.25. Two real defects: - Stage 1 accumulators now replace by key (channel index / config section) instead of appending: a want_config_id retry restarts the firmware's config drain from scratch (PhoneAPI handleStartConfig resets its read index), which duplicated channels and config sections in the committed ConfigBundle and channels state. - FromRadio.lockdown_status (protobufs 2.7.25+) fell through every routing arm and was silently dropped, violating the no-silent-loss rule. It now surfaces as a structured ProtocolWarning until typed lockdown support lands alongside AdminMessage.lockdown_auth. Audit confirmations baked into tests/comments: firmware forces from=0 on all phone packets (MeshService.cpp:188), so local admin authorizes via the from==0 branch and never needs a passkey; every phone send gets a per-packet QueueStatus(res, mesh_packet_id) from sendToMesh, so the QueueRejected fast-fail is live; the 69420/69421 special nonces are the firmware-sanctioned config-only/nodes-only drains. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Kotlin library best-practices pass:
- Remove RadioClient's AutoCloseable/close() — the runBlocking bridge
was an ANR trap on Android main and a deadlock risk on iOS main, and
a radio session has no safe non-suspending teardown. Lifecycle is
suspend-only: try { ... } finally { client.disconnect() }. Docs
(threading-model.md, RadioClient KDoc) updated to match.
- Bridge the two byte-string vocabularies in the public surface:
okio.ByteString (Wire proto types, per ADR-001) <->
kotlinx.io.bytestring.ByteString (Frame/SessionPasskey) via
toKotlinxByteString()/toOkioByteString(), so consumers never
round-trip raw arrays.
- Codify the API conventions in docs/api-reference.md: proto exposure,
byte vocabularies, no blocking bridges, the data-class trade
(deliberate, ABI-dump-gated; copy()/destructuring are conveniences
not contract), and the Kotlin/Swift-first interop stance.
- Track Poko adoption for event/result types in the roadmap: 0.23.1
fails against Kotlin 2.3.21 (NoClassDefFoundError:
ExtensionPointDescriptor), so @poko migration waits on upstream.
ABI dumps regenerated (close()/AutoCloseable removal).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
…s the API Wire-generated proto types force okio.ByteString into every proto-typed call for both this SDK and its consumers (Meshtastic-Android consumes the same protobufs artifact), so the kotlinx-io ByteString on Frame / SessionPasskey was a gratuitous second vocabulary. Standardize: - Frame.bytes and SessionPasskey.bytes are okio.ByteString. - send(portnum, payload: kotlinx.io.Buffer) is replaced by send(portnum, payload: okio.ByteString) — the type proto payloads already arrive in, so payloads pass through without array copies. - Internal frame assembly (WireCodec.FrameDecoder, TcpTransport, SerialFrameAssembler) moves to okio.Buffer. - kotlinx-io is dropped from every published module (and the version catalog); okio is promoted to api in :core since it now appears in core's own public signatures. - The toKotlinxByteString()/toOkioByteString() adapters added earlier this cycle are deleted — nothing left to bridge. Consumers (and the app cutover) now use exactly one byte type end-to-end: proto payloads, frames, and storage. ABI dumps regenerated; docs/api-reference.md API-conventions section updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
…ode, leaner accumulators Four-angle cleanup review (reuse / simplification / efficiency / altitude) over the branch diff, fixes applied: - testing: add FromRadio.toFrame() — the device-side wire framing that 8 test files (and FakeRadioTransport) each re-implemented now lives in one helper; the local copies delegate to it. - engine: decode AdminMessage once per inbound ADMIN_APP packet — the passkey latch and external-config-change detection each ran their own decode of the same payload on a hot path. - engine: pendingConfigs/pendingModuleConfigs become immutable-list vars reassigned via mergeConfigs/mergeModuleConfigs, dropping the clear()+addAll() churn per config frame during the handshake. - engine: drain handshakePacketBuffer in place instead of copying the backlog to a list at Ready. - transport-ble: source START1/START2 from WireFraming (TCP and serial already did). Reviewed and deliberately NOT changed: prepareOutboundAdminPacket's decode+re-encode passkey stamp (binary-patching Wire payloads is fragile; remote admin is low-frequency and the single engine choke point is worth keeping); postConnectHook as an internal var (constructor plumbing isn't worth it for one platform hook); the queueStatus arm's bookkeeping (pre-transmit rejection is semantically distinct from routing NAKs); moving ScriptedTransport to :testing (900-line virtual-time fixture — tracked as follow-up, too churny for this PR). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
… factory, one decoder family
API-shape review for consumer convenience:
- RadioClient.withConnection { } — connect, run the block, always
disconnect (success, exception, AND cancellation via NonCancellable
teardown). The structured-concurrency replacement for the blocking
use{} idiom this API deliberately dropped.
- Flow<NodeChange>.asNodeMap() + RadioClient.nodeMap() — folds the node
delta stream into a live Map<NodeId, NodeInfo>. This is the exact
scan accumulator every consumer (including the Android migration
guide) was hand-writing; now it ships with stateIn-ready semantics.
- RadioClient { } builder-lambda factory as Kotlin-idiomatic sugar over
Builder (which stays, for Swift and step-wise construction).
- Removed the duplicate typed-decoder family (decodeAsText/Position/
User/NodeInfo/Telemetry/Routing/Admin) that shadowed the asText()/
asPosition()/… accessors in PayloadAccessors.kt — one accessor family
remains, plus the generic decodeAs(adapter) escape hatch for portnums
without a typed accessor (Paxcount, StoreAndForward, …).
ABI dumps refreshed; new RadioClientSugarTest covers all three sugars.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
…e admin, ERRNO queue results
Findings from a four-angle adversarial review of this branch
(correctness / concurrency / API surface / tests), all fixed:
HIGH — nodes flow late-subscriber contract (new in this branch):
Every subscription now receives a fresh NodeChange.Snapshot first,
seeded via onSubscription from the engine's current node map; the
engine SharedFlow drops its replay slot. Previously a late subscriber
got whatever DELTA was emitted last instead of the Snapshot, leaving
nodeMap()-style folds near-empty until the next reconnect.
HIGH — off-actor mutation of sessionPasskeys (regression in this
branch): the fire-and-forget admin path (enterDfuMode, setTimeOnly)
ran prepareOutboundAdminPacket — which prunes the actor-owned per-node
passkey map — on the caller's coroutine. sendAdmin now posts an
EngineMessage.AdminFireAndForget through the inbox (ADR-002).
Firmware conformance — QueueStatus.res is the ERRNO namespace, not
Routing.Error: 35 (ERRNO_SHOULD_RELEASE) is success and counts as
Sent; ERRNO rejections (>= 32) fail pending RPCs as NodeUnreachable
via the new CommandDispatcher.tryFail; 1..31 map through the normal
routing taxonomy. Previously 32 read as BAD_REQUEST, 33 as
NOT_AUTHORIZED, and the success code 35 produced a false failure.
Remote-admin correctness:
- Passkeys latch only from response-shaped admin messages; a remote
node administering US no longer poisons our passkey cache with the
key WE issued it.
- Managed-mode client gate applies to local targets only (firmware
only rejects local admin on managed devices; remote targets
authorize via their own admin keys).
- Session passkeys are no longer persisted (local ones are never
required — firmware rewrites phone packets to from=0; remote ones
expire in ~4 min). Storage methods remain, documented host-facing.
Pre-existing engine defects (surfaced by the same review):
- dispatchSend guards caller-supplied wire-id collisions at the real
key — the second send no longer strands the first handle forever.
- Stage-1 settle replay re-buffers frames behind a duplicate
completion instead of silently dropping them.
- The seeding window routes non-packet variants (node_info, client
notifications, MQTT proxy, ...) instead of silently dropping them.
- Stage-2 commit snapshots actor-owned collections before its async
storage flush (CME risk mis-reported as StorageDegraded).
API shape (pre-publication, nothing on Central):
- MeshEvent.FileInfo -> FileInfoReceived (no longer shadows
org.meshtastic.proto.FileInfo verbatim).
- sendText is (text, to, channel, replyId) — aligned with
sendReaction; all callers were named-arg or text-only.
- withConnection gains teardownTimeout (default 10s): the
NonCancellable disconnect is bounded so a wedged transport cannot
pin a cancelled caller forever.
- detekt ForbiddenImport now bans kotlinx.io.* (one byte vocabulary).
Tests: 16 new tests pin the riskiest logic — PKC asymmetric-key
fallbacks, caller-built PKC passthrough, caller priority + broadcast
guards, two-remote passkey isolation, passkey TTL pruning, identity-
rebind passkey clearing, queue rejection of in-flight RPCs, ERRNO vs
routing-error mapping, res=35 success, unmapped res, buffer-overflow
PacketsDropped + drop-oldest order, seeding-window buffering, settle-
replay preservation, post-Ready dedupe of flushed packets, seeded
late-subscriber snapshots, withConnection cancellation teardown, and
direct decodeAs escape-hatch coverage. ABI dumps regenerated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
…de can't leak across sessions The Android factory's delayed CONNECTION_PRIORITY downgrade was launched on the transport-lifetime scope; disconnect() didn't cancel it, and the transport supports reuse-after-disconnect (the engine's auto-reconnect path), so a 30-second timer from session N could fire mid-handshake of session N+1 and downgrade the connection priority during exactly the boost window it exists to protect. postConnectHook now receives a per-connect-cycle scope (child of the transport scope), cancelled in cancelJobs()/disconnect(). New BleTransportHookTest drives a fake Kable Peripheral through connect/disconnect/reconnect and proves: the hook runs once per connect and failures are non-fatal; hook-scheduled work is cancelled at disconnect; and prior-session work never fires into the next session (each test fails against the pre-fix code). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
The review found the documentation tree contradicting the shipped
surface on every axis this branch changed. Fixed across 16 files:
- CONTRIBUTING/AGENTS/GEMINI/.specify templates: the byte-payload house
rule inverted to okio.ByteString (kotlinx-io is deliberately not a
dependency; now also detekt-banned).
- SPEC.md v2.2 -> v2.3: AutoCloseable/close() removed, okio vocabulary,
send overload, sendText signature, new MeshEvent/SendFailure variants,
nodes-flow onSubscription seeding, storage passkey methods marked
host-facing, decision-log reversal entries appended with rationale.
- ADR-003 + ADR-000/001/006: superseded-by notes appended (history
preserved, not rewritten).
- api-reference.md: Frame signature, new sendText row, RadioClient {}
factory, withConnection, nodeMap()/asNodeMap(), the three new
MeshEvent rows, AutoReconnectConfig since-tag corrected to 0.1.0.
- error-taxonomy.md: QueueRejected + the QueueStatus/ERRNO namespace
section (32/33/34 rejections, 35 = success, 1..31 Routing.Error).
- transport-isolation.md: close() -> disconnect() throughout.
- README/bom README: transport-tcp is Ktor (not kotlinx-io); proto row
is Wire-generated (not kotlinx.serialization); 233-byte payload limit.
- security.md: storage at-rest guidance references SQLDelight.
- CHANGELOG: restructured per versioning.md with a consolidated
'### Breaking' section; adds the testing toFrame() entry and all of
this branch's review fixes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
…rgets FakePeripheral overrides Peripheral.identifier, which is kotlin.uuid.Uuid on Kable's iOS targets (experimental API) — the override compiled on JVM/Android (String typealias) but failed compileTestKotlinIosX64 / IosSimulatorArm64 in CI's full-check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
androidDeviceTest source set running the cs1-cs6 conformance envelope against a real radio: scan by Meshtastic service UUID (bonded-first, RSSI-sorted, 3-candidate retry), connect through the production BleTransport(address) factory (MTU-517 + connection-priority hook), two-stage handshake, read-only admin RPC round-trips, a 177-byte send to the radio's own node num as MTU proof (no LoRa TX), SQLDelight persistence, and same-transport reconnect. Assume-skips when no radio is advertising so CI never requires hardware. Run with: ./gradlew :transport-ble:connectedAndroidDeviceTest Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Five ship-blocking bugs caught by MeshtasticBleConformanceTest on a
Pixel 6a against live radios (fw 2.7.24), none reachable by JVM/fake
tests:
- transport-ble: TORADIO is CHR_PROPS_WRITE only on real firmware
(NRF52Bluetooth.cpp:213) — write-without-response is refused by
Android outright; use WriteType.WithResponse. Docs updated to match.
- engine: seed the wire packet-id counter randomly per instance.
Starting at 1 every session collided with the firmware's ~10-minute
packet-history dedup across reconnects, silently swallowing every
RPC of the new session (Timeout with no response on the air).
nextMessageId() now also skips 0 ("unset" on the wire).
- engine: publish configBundle/channels synchronously at the Stage-2
commit, before connect() resumes — visibility was previously gated
on the async storage flush and lost the race on real disk latency.
- storage-sqldelight (Android): enable WAL via
enableWriteAheadLogging(); execSQL("PRAGMA journal_mode=WAL")
returns a result row and throws on re-activation.
- transport-ble: frames() is re-collectable after disconnect()
(per-cycle frame channel + collector guard reset), honouring the
documented reuse-after-disconnect contract exercised by cs6.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Completes AdminMessage coverage with the hardened-build (MESHTASTIC_LOCKDOWN) lockdown surface, and migrates the protobuf dependency model + all docs from the vendored-submodule scheme to the published org.meshtastic:protobufs artifact. Lockdown: - AdminApi.lockdown(LockdownAuth): local-only (rejects forNode targeting; the firmware consumes the passphrase inline on the phone link), fire-and-forget; the device replies with a fresh lockdown_status. - MeshEvent.LockdownStatusChanged: replaces the prior ProtocolWarning stub; handled identically mid- and post-handshake. - Tests (outbound + inbound), updated CapturingAdminApi fake, regenerated ABI. - Builds on the pinned 2.7.25 proto; the newer LockdownAuth.disable / max_session_seconds and LockdownStatus.State.DISABLED arrive in a follow-up once a stable proto release ships them. Protobufs: - Stays pinned at 2.7.25. Audit found zero field changes across the 46 SDK-consumed messages vs develop-SNAPSHOT; the only delta is the device-only NodeDatabase restructure, not consumed here. Docs: - protocol.md: correct the AdminMessage field numbers and add a lockdown section. - ADR-015 records the submodule + :proto-module -> published-artifact migration; superseded-by notes added to ADR-000/001/003/006/008/013. - Sweep guidance/architecture docs (AGENTS, GEMINI, CONTRIBUTING, README, versioning, ci-cd, module-graph, enforcement, transport-*, migration guides, wasm roadmap) to the artifact model; drop the dead git-submodules Renovate manager; fix Dokka V1 -> V2 (dokkaGenerate) command references. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
…_seconds, DISABLED) Stacked on the 2.7.25 lockdown support. Bumps org.meshtastic:protobufs to develop-SNAPSHOT to pick up LockdownAuth.max_session_seconds / .disable and LockdownStatus.State.DISABLED, and restores the tests + KDoc that exercise them. DRAFT — do not merge until a stable proto release ships these fields; a moving -SNAPSHOT must not back a tagged release (see CHANGELOG / versioning.md). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
7 tasks
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #6 (
feat/android-cutover-prereqs). Do not merge until #6 lands and a stableorg.meshtastic:protobufsrelease ships the develop-only lockdown fields.#6 added storage-lockdown admin support (
AdminApi.lockdown,MeshEvent.LockdownStatusChanged) against the pinned 2.7.25 proto. Three lockdown capabilities exist only in newer proto and are split out here:LockdownAuth.disableLockdownAuth.max_session_secondsLockdownStatus.State.DISABLEDThis PR bumps
org.meshtastic:protobufs→develop-SNAPSHOTand restores the tests + KDoc that exercise those three symbols.Why draft
A moving
-SNAPSHOTmust not back a tagged/merged release (non-reproducible builds). Merge only once a stable proto release publishes these fields; at that point repin to that version and rebase ontomain.Verification
:core:jvmTest checkKotlinAbi detekt spotlessCheckgreen againstdevelop-SNAPSHOT(lockdown tests 22/0/0). Public ABI is unchanged from #6 — the new proto fields don't alter the SDK surface.🤖 Generated with Claude Code