fix(MQTT): don't let a disabled channel gate downlink, PKI, or uplink

[h3lix1]

Rewritten. The earlier version of this PR argued that getByName()'s case-insensitive match and
primary-channel fallback let the wrong channel answer. That was wrong: the existing gate re-checks
with a case-sensitive strcmp against getGlobalId(ch.index), so near-misses and the primary
fallback both fail closed. Sorry for the noise. The real defect is a different one in the same lines.

Four places in MQTT.cpp test downlink_enabled or uplink_enabled on their own, without the
role and has_settings guards that Channels::anyMqttEnabled() uses a few files over: the
downlink gate in onReceiveProto(), the anyChannelHasDownlink scan that unlocks the PKI topic, the
subscribe loop in sendSubscriptions(), and the uplink scan in onSend().

A disabled channel keeps its settings. ensureLicensedOperation() takes a copy of the channel, sets
role to DISABLED and clears only the PSK, then writes it back, so a licensed operator's admin
channel ends up disabled with its name and downlink_enabled intact. fixupChannel() only zeroes
settings when has_settings is false, so nothing cleans it up. Plain admin traffic that sets a role
to DISABLED leaves the same residue.

The node then subscribes to that channel's topic, which also names it to the broker in the SUBSCRIBE,
accepts downlink on it, and stamps the packet with the disabled slot's index. That index has no key,
since Channels::getKey() returns -1 for DISABLED, so a rebroadcast attempt dies with
NO_CHANNEL and the phone is shown traffic on a channel the user turned off. The same gap lets a
disabled slot on its own satisfy the PKI unlock.

The uplink side is the worse half

The same guard is missing on the "does any channel want uplink" scan in onSend(), and there it is
not one gate among several. For a PKI packet it is the only one: the per-channel uplink_enabled
check below it is short-circuited by isPKIEncrypted, and perhapsEncode()'s PKI branch never
consults the channel at all, so nothing else can stop the publish.

So a node whose owner turned uplink off on every live channel still publishes every PKI DM it
originates or relays, because that one stale disabled slot satisfies the scan. The payload stays
encrypted, but the envelope carries sender, recipient, packet id and timestamp, and it may be going
to the public default broker. It also silently defeats the exact setting a privacy-conscious operator
would reach for.

I deliberately left the per-channel gate alone. A disabled chIndex does reach it, because the PKI
branch of perhapsEncode() does not validate the index, but isPKIEncrypted then selects the
literal "PKI" as the channel id, so getGlobalId(chIndex) is never evaluated and no disabled
channel's name is published. A guard there would be dead code.

Bounding that loop by getNumChannels() instead of a hardcoded 7 is cleanup rather than a fix.
channels_count is forced to MAX_NUM_CHANNELS on every device, so the out-of-range iterations only
happen under the native test harness, where each one logs an ERROR. It matches what the rest of the
file already does.

Why resolution stays first-match-wins

Duplicate global ids are legal. Channels::getName() maps every blank name to the modem-preset name
with no index guard, so any two blank-named active slots resolve to the same id, and fixupChannel()
rewrites a channel literally named Default to blank, which is a legacy-compat path that exists
because those configs are out there. Rejecting a duplicate as ambiguous would drop all non-PKI
downlink on stock configurations. The third test pins that down so a later rework does not
reintroduce it.

Comparing the raw settings.name instead is also wrong: the channel's on-the-wire identity is
getGlobalId(), which is what sendSubscriptions() subscribes with and what onSend() publishes
under, so matching on the raw name would make every blank-named channel unmatchable, including the
stock default primary.

Behaviour change worth naming

Folding downlink_enabled into the scan slightly widens the gate in one case: where a lower-index
slot shares a name but has downlink disabled and a higher-index slot has it enabled, develop drops
and this accepts. That is the correct direction, since sendSubscriptions() subscribed to that exact
topic on the higher-index slot's behalf, but it is a real difference rather than pure equivalence.

The root cause is arguably upstream of all of this: ensureLicensedOperation() zeroes only the PSK
when it forces a channel to DISABLED, leaving a half-scrubbed slot behind, and it latches, since on
the next pass its own guard no longer fires. Scrubbing the settings block there would remove the
source rather than the symptom. Left alone here, happy to follow up.

Tests

test_receiveIgnoresDisabledChannelDownlink, test_receiveIgnoresPkiWhenOnlyDisabledChannelHasDownlink
and test_pkiNotUplinkedWhenOnlyDisabledChannelHasUplink all fail on develop with
Expected TRUE Was FALSE. test_receiveAcceptsDownlinkWhenTwoChannelsShareAGlobalId passes on
develop and is the regression guard described above.

Testing: builds clean on heltec-v4, 61/61 in test_mqtt on native-macos. Not exercised against a
live broker.

🤖 Generated with Claude Code

🤝 Attestations

• I have tested that my proposed changes behave as described.
• I have tested that my proposed changes do not cause any obvious regressions on the following devices:
  • Heltec (Lora32) V3
  • LilyGo T-Deck
  • LilyGo T-Beam
  • RAK WisBlock 4631
  • Seeed Studio T-1000E tracker card
  • Other: compile-only on Heltec V4

[github-actions]

⚡ Try this PR in the Web Flasher

Note

Building this pull request… the flash button, badges and supported-board
list will appear here automatically once CI finishes.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7dd85b42-dcc6-4d40-9e20-114280858a72

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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