feat(mqtt): route MQTT channel permissions through channel_database#3108
Conversation
The slot index in `packet.channel` is the *sender's* device slot — when
two upstream nodes use different channels in their own slot 0 (extremely
common on a public broker), the same per-source `channels` row gets its
name flipped on every packet, and channel_0..7 grants protect "whichever
name won the slot race." That's the wrong identity for MQTT.
Instead, resolve every MQTT-ingested packet to a channel_database row:
either the one server-side decryption already identified, or one looked
up by `envelope.channelId`. Unknown names auto-register a passive row
(isEnabled=false, no PSK) so admins can grant access without forcing
operators to pre-declare every observed channel. The persisted `channel`
column gets the existing `CHANNEL_DB_OFFSET + id` encoding nodeEnhancer
already enforces, so no schema migration is required — slot fallback is
preserved when no name resolves.
UI: the per-user Permissions dialog hides the channel_0..7 grid when an
MQTT source is selected and points admins at the existing Virtual
Channel Permissions section below.
Out of scope:
- Historical MQTT rows with raw 0..7 channel values keep the old
channel_${slot} enforcement until they age out.
- The per-source `channels` table is still populated by
recordChannelFromEnvelope (channel picker), unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @Yeraze's task —— View job Code Review Complete ✅This is a high-quality PR that addresses a real stability issue in MQTT channel permissions with an elegant, well-tested solution. 🎯 SummaryThe PR fixes unstable MQTT channel permissions by routing them through the existing ✅ StrengthsArchitecture & Design
Implementation Quality
Test Coverage ⭐
🔍 Minor Suggestions for Future EnhancementPerformance Considerations
Security & Validation
🧪 Test Quality AssessmentExcellent coverage including:
Test architecture highlight: The 🎯 Final Recommendation: APPROVE ✅This PR successfully:
The implementation is production-ready and represents a significant improvement to MQTT source reliability. The minor suggestions above are enhancements for future consideration, not blockers. Manual Testing: The provided checklist covers the key scenarios. I recommend testing the permission inheritance behavior when switching between sources to ensure the UI state transitions correctly. |
…ons (#3110) * fix(mqtt): stamp node.channel on ingest so the map filter honors VC perms #3108 routed MQTT message ingest through `channel_database` and stamped `message.channel = CHANNEL_DB_OFFSET + dbId`. The matching change to the node-upsert paths was missed: `NODEINFO_APP` and `POSITION_APP` built `Partial<DbNode>` rows without setting `channel`, so the column stayed NULL. `filterNodesByChannelPermission` in `nodeEnhancer.ts` then evaluated `node.channel ?? 0` for every MQTT-sourced node, falling back to slot 0 and requiring `permissions[channel_0]?.viewOnMap`. But the #3108 UI hides the channel_0..7 toggles for MQTT scopes and directs admins to Virtual Channel Permissions — leaving no way to grant map access. Result: anonymous and non-admin users saw "No nodes visible" on the map for every MQTT source, no matter what they granted. Fix: pass `channel: effectiveChannel` into the upsertNode calls in the NODEINFO_APP and POSITION_APP branches, matching the message-side encoding. Existing rows that already have channel=NULL recover as each node re-broadcasts NODEINFO (typically every few hours under default firmware config); no migration backfill needed for short- term correctness. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sources): channel-gate traceroutes and neighbor-info on the map Companion to the node.channel stamping fix in the same PR. With nodes correctly filtered out for users who lack viewOnMap on the channel, two endpoints still leaked enough position data to render lines: 1. GET /api/sources/:id/traceroutes returned every row including the embedded `routePositions` JSON snapshot. The frontend's `useTraceroutePaths` hook prefers that snapshot over the live nodes list, so it drew line segments between coordinates of nodes the user had no permission to see. Apply `maskTraceroutesByChannel` to the response. 2. GET /api/sources/:id/neighbor-info enriched each record with positions pulled from `getNodesByNums` without checking channel access. Build the visible-node set via `filterNodesByChannelPermission` and drop neighbor records whose endpoints aren't in it. User-visible symptom this fixes: anonymous viewers seeing "lines floating in space" on the map for MQTT sources whose nodes had been correctly filtered out of the nodes endpoint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(release): bump version to 4.6.3 Five user-visible files bumped per the CLAUDE.md version recipe: package.json, package-lock.json (regenerated), helm/meshmonitor/Chart.yaml, desktop/src-tauri/tauri.conf.json, desktop/package.json. CLAUDE.md banner line bumped to match. CHANGELOG entry covers the five PRs since 4.6.2-1: - #3105 unified tapback metadata fix - #3106 docs: drop worktree restriction - #3107 meshcore contact advType persistence - #3108 MQTT channel permissions via channel_database - #3109 hint banner Catppuccin restyle - #3110 node.channel ingest + traceroute/neighbor channel gate Companion blog post (docs/blog/2026-05-20-v4.6.3-permissions.md) walks operators through the new Virtual Channel Permissions flow, the map-visibility behavior change, and the floating-lines fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(release): regenerate desktop/package-lock.json for 4.6.3 The desktop sub-project carries its own lockfile and the bump to 4.6.3 left it pinned to 4.6.1. The Windows Desktop CI job runs `npm install` without `--legacy-peer-deps` and fails on the package.json / package-lock.json version mismatch. Regenerate to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
The Permissions dialog showed an 8-slot channel grid (
channel_0..channel_7) for every source type, including MQTT. For MQTT that grid keyed permissions on the sender's device slot (packet.channel), which is unstable: two upstream nodes that use different channels in their own slot 0 fight over the same(sourceId, slot)row, and a grant onchannel_0silently protects whichever channel name won the slot race most recently.This PR re-keys MQTT channel permissions on the existing
channel_database(Virtual Channels) — a global, name-based registry that already has its own per-user permission table (channel_database_permissions). No schema migration is needed:nodeEnhancer.tsalready enforces virtual-channel permissions whenever a row'schannelvalue is>= CHANNEL_DB_OFFSET (100), withchannelDatabaseId = channel - 100.Changes
src/server/mqttIngestion.ts— on every MQTT ingest, resolve achannelDatabaseId(priority: server-decrypt result, thenenvelope.channelIdvia name lookup with find-or-create), then stamp the persistedchannelcolumn asCHANNEL_DB_OFFSET + id. Falls back to the raw slot when nothing resolves. PasseseffectiveChannelto traceroute / store-forward writes too. Adds a per-process name→id cache and a_resetMqttIngestCachesForTestexport.src/db/repositories/channelDatabase.ts— newfindOrCreatePassiveByNameAsync(name)that creates a disabled (no-PSK) row for the name if none exists, so the permission target is always materialized without affecting decryption.src/components/UsersTab.tsx— when the per-user permission scope is anmqtt_brokerormqtt_bridge, the slot grid hideschannel_0..7rows and shows a hint pointing to the existing Virtual Channel Permissions section below.mqttBrokerManager.test.tsso it waits for the ingestion.thenchain (my addedawaitintroduced a microtask race).Out of scope
channel_${slot}enforcement until they age out. The cutover is forward-only.channelstable is still populated byrecordChannelFromEnvelopefor the channel picker — unchanged.Test plan
npx vitest run— 5179/5179 passnpx tsc -p tsconfig.server.json --noEmit— cleannpx tsc -p tsconfig.json --noEmit— clean (UsersTab)LongFastshow up underchannel_databasepermissions, and that an admin can grant/revoke a non-admin user access via the Virtual Channel Permissions section in the user editor.channel_0..7rows are gone and the hint banner is shown.🤖 Generated with Claude Code