diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index c3d275070..6be39d86d 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -747,6 +747,7 @@ jobs: python3 -B test/test_repeater_radio_timing_integration.py python3 -B test/test_temp_radio_reply_delivery_contract.py python3 -B test/test_reply_tx_integration.py + python3 -B test/test_admin_cli_reply_scope.py python3 -B test/test_ota_speed.py -v python3 -B test/test_ota_announce_cadence.py -v python3 -B test/test_ota_apply_delivery.py -v diff --git a/docs/cli_commands.md b/docs/cli_commands.md index 3335d5d17..02e225a04 100644 --- a/docs/cli_commands.md +++ b/docs/cli_commands.md @@ -1223,13 +1223,16 @@ set tx.reply auto ``` Persistent choices: `auto`, `radio`, `radio2`, `both` (default), and `off`. -Explicit choices override `radio2.cross` for local replies, including CLI, -ACKs, telemetry, room subscription posts, and served LoRa OTA responses. -Append `force` to `both` or `radio2` to permit reply TX on an active RX-only +Explicit choices override `radio2.cross` only for CLI replies to authenticated +admins, including cached results and delayed admin GPIO command completion. +Login replies, ACKs, app telemetry/status responses, returned paths, room posts, +LoRa OTA, signed fleet acknowledgements, and non-admin CLI replies follow normal +crossing rules. Region/filter-manager roles do not get the admin override. +Append `force` to `both` or `radio2` to permit admin CLI reply TX on an active RX-only second profile. It never enables a disabled profile or ordinary forwarding. -Omitting `force` removes that override. `off` suppresses remote replies too. +Omitting `force` removes that override. `off` suppresses admin CLI replies only. -See [reply routing and OTA examples](radio_profiles.md#repeater-room-server-and-sensor-replies) +See [admin CLI reply routing](radio_profiles.md#repeater-room-server-and-sensor-replies) for temporary profiles, receive-only exceptions, and queued reply handling. ## Set Companion display rotation diff --git a/docs/radio_profiles.md b/docs/radio_profiles.md index 32e30920a..0cf9d32fd 100644 --- a/docs/radio_profiles.md +++ b/docs/radio_profiles.md @@ -134,7 +134,7 @@ the suggested frequency only when it also suits the network and local band plan. | Mode | Receive on profile 2 | Transmit on profile 2 | | --- | --- | --- | -| `rx` | Yes | Only infrastructure replies explicitly configured with `tx.reply ... force` | +| `rx` | Yes | Only admin CLI replies explicitly configured with `tx.reply ... force` | | `rxtx` (also `rx&tx`) | Yes | Yes | | `off` | No | No | @@ -159,8 +159,8 @@ get tempradio2 ``` This enables the second profile for 120 minutes (2 hours). Locally generated -LoRa OTA requests use the temporary second profile. Infrastructure OTA responses -follow `tx.reply` below (both TX-capable profiles by default). Ordinary local traffic stays +LoRa OTA requests use the temporary second profile. OTA responses retain the +request's profile and follow `radio2.cross`. Ordinary local traffic stays on the primary profile by default. Use an OTA-capable build and the usual OTA setup on every participating node; a temporary profile does not add OTA support to firmware built without it. @@ -189,13 +189,16 @@ time, so setting the clock backwards cannot extend a temporary session. ## Repeater, room-server, and sensor replies -These roles default to **`tx.reply both`**: locally generated replies are queued -on `radio` and on an active `radio2` that permits TX. This includes replies to -remote CLI/login/telemetry requests, ACKs, returned paths, delayed GPIO command -completion, room subscription posts, and locally served LoRa OTA catalog, -manifest, data, proof, and leaves responses. Each copy has its own retry state. -Forwarded packets, periodic adverts, sensor pushes, and OTA requests continue -to use the normal crossing rules. +These roles default to **`tx.reply both`** for CLI replies to authenticated +admins only. Those replies are queued on `radio` and on an active `radio2` +that permits TX. This includes cached CLI results and delayed admin GPIO +command completion. Each copy has its own retry state. + +Login replies, ACKs, returned paths, status/telemetry responses, room posts, +LoRa OTA traffic, signed fleet acknowledgements, and non-admin CLI replies +follow the normal `radio2.cross` rules. A telemetry command entered through +the admin CLI still produces an admin CLI reply; an app telemetry request does +not. Region/filter-manager roles do not receive the admin override. ```text get tx.reply @@ -205,13 +208,13 @@ set tx.reply radio set tx.reply auto ``` -| Choice | Replies transmit on | +| Choice | Admin CLI replies transmit on | | --- | --- | | `both` | Primary plus an active `rxtx` second profile. Default. | | `radio` | Primary only. | | `radio2` | Second only; no primary fallback. | | `auto` | Normal `radio2.cross` behavior, including permanent/temporary isolation. | -| `off` | Neither profile. This also suppresses remote CLI replies and ACKs. | +| `off` | Neither profile. Suppresses admin CLI replies only. | Append **`force`** to `both` or `radio2` to allow these replies on an active second profile configured as `rx`. Without `force`, `rx` remains receive-only. @@ -222,17 +225,16 @@ there. Existing crossover packet filters still apply to the extra copy. The explicit choices override `radio2.cross`, including `off` and mixed permanent/temporary profiles. `radio` means the active primary profile, including `tempradio`; `radio2` also includes an active `tempradio2`. -For example, an OTA source can retain its normal channel while allowing OTA -and management responses on a temporary receive profile: +For example, a repeater can retain its normal channel while allowing admin CLI +responses on a temporary receive profile: ```text set tempradio2 910.5,500,8,5,rx,120 set tx.reply both force ``` -The source must have OTA support and an active temporary-radio session as -usual. A node fetching an update still needs `rxtx` for its OTA requests. -Forcing replies does not change OTA's receive gates, scan timing, or preambles. +This override does not allow OTA responses on a receive-only profile. Use +`rxtx` for a profile that needs to exchange OTA requests and data. This setting survives reboot; older saved profiles adopt `both` without force. The getter reports when the chosen second profile cannot transmit. Already @@ -242,8 +244,6 @@ both admitted reply copies to drain before changing the primary settings; at least one must transmit successfully. Those tracked replies omit alternate paths and transport retries so no late success copy can outlive the handoff. Queue capacity and the shared transceiver's airtime budget still limit delivery. -OTA data/proof admission counts both copies against its queue credit and -reserves receive capacity before allowing the response. Companions use the per-contact/channel choices below; `tx.reply` is an infrastructure setting. @@ -922,13 +922,13 @@ These are the global rules used by Companion targets set to `auto`; an explicit contact/channel choice above overrides crossing for its own outbound packets. Recognized media uses the separate `tx.media auto` selection described above. Ordinary crossing respects `rx`. The infrastructure-only `tx.reply ... force` -override above permits locally generated replies on that profile. +override above permits authenticated admin CLI replies on that profile. With crossing allowed, a packet received there may still be forwarded on the primary profile. Existing routing, forwarding and packet-filter settings apply. An RX-only `tempradio2` also blocks locally generated OTA requests under `auto` or `off`; they do not fall back to the normal primary channel. Use `rxtx` -for a fetching node that needs to exchange requests and data. Infrastructure -OTA responses follow `tx.reply` independently of this crossing table. +for a fetching node that needs to exchange requests and data. OTA responses +also follow this crossing table; `tx.reply` does not apply to them. ## MCU power saving while scanning diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index a41dc76ee..e248fe6d5 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -700,7 +700,6 @@ void MyMesh::sendDefaultPasswordWarning(ClientInfo* client, mesh::Packet* warning = createDatagram(PAYLOAD_TYPE_TXT_MSG, client->id, client->shared_secret, data, sizeof(data)); if (warning == NULL) return; - warning->radio_reply = true; if (mesh::Packet::isValidPathLen(client->out_path_len)) { sendClientReply(client, warning, SERVER_RESPONSE_DELAY + 2000UL, path_hash_size); } else { @@ -3200,7 +3199,7 @@ bool MyMesh::sendRemoteCliReply(ClientInfo* client, const uint8_t* secret, mesh::Packet* packet = createDatagram(PAYLOAD_TYPE_TXT_MSG, client->id, secret, reply_data, 5 + text_len); - if (packet) packet->radio_reply = true; // also covers delayed GPIO completion + if (packet) packet->radio_reply = client->isAdmin(); // includes delayed admin GPIO completion // A state-changing reply tracks its first copy on each selected profile. // Suppress alternate paths and retries, which could outlive the handoff. // Prepare before admission so Dispatcher can register its second copy; @@ -3246,7 +3245,7 @@ void MyMesh::onUserGpioTimerCompleted(uint8_t pin, uint8_t state, } } } - if (client == NULL) return; + if (client == NULL || !client->isAdmin()) return; char reply[64]; snprintf(reply, sizeof(reply), "> GPIO %u timer complete: %s", pin, diff --git a/examples/simple_room_server/MyMesh.cpp b/examples/simple_room_server/MyMesh.cpp index c95486d63..e28855aae 100644 --- a/examples/simple_room_server/MyMesh.cpp +++ b/examples/simple_room_server/MyMesh.cpp @@ -1133,7 +1133,6 @@ bool MyMesh::pushRoomTextToClient(ClientInfo *client, uint32_t timestamp, auto reply = createDatagram(PAYLOAD_TYPE_TXT_MSG, client->id, client->shared_secret, reply_data, len); bool sent = false; if (reply) { - reply->radio_reply = true; // asynchronous response to the room subscription if (!mesh::Packet::isValidPathLen(client->out_path_len)) { unsigned long delay_millis = 0; sent = sendFloodScoped(default_scope, reply, delay_millis, @@ -1864,7 +1863,6 @@ void MyMesh::sendDefaultPasswordWarning(ClientInfo* client, mesh::Packet* warning = createDatagram(PAYLOAD_TYPE_TXT_MSG, client->id, client->shared_secret, data, sizeof(data)); if (warning == NULL) return; - warning->radio_reply = true; sendClientReply(client, warning, SERVER_RESPONSE_DELAY + 2000UL, path_hash_size); } @@ -2076,6 +2074,7 @@ void MyMesh::onPeerDataRecv(mesh::Packet *packet, uint8_t type, int sender_idx, uint32_t sender_timestamp; memcpy(&sender_timestamp, data, 4); // timestamp (by sender's RTC clock - which could be wrong) uint8_t flags = (data[4] >> 2); // message attempt number, and other flags + const bool admin_cli_reply = flags != TXT_TYPE_PLAIN && client->isAdmin(); if (!(flags == TXT_TYPE_PLAIN || flags == TXT_TYPE_CLI_DATA || flags == TXT_TYPE_CLI_COMMAND)) { MESH_DEBUG_PRINTLN("onPeerDataRecv: unsupported command flags received: flags=%02x", (uint32_t)flags); @@ -2241,11 +2240,11 @@ void MyMesh::onPeerDataRecv(mesh::Packet *packet, uint8_t type, int sender_idx, auto reply = createDatagram(PAYLOAD_TYPE_TXT_MSG, client->id, secret, temp, 5 + reply_text_len); + if (reply) reply->radio_reply = admin_cli_reply; if (_cli.morseReply().generation() != morse_mutation_before) { // One first copy per radio profile, without alternate paths/retries // that could remain queued after the listen handoff. _cli.morseReply().prepareReply(reply); - if (reply) reply->radio_reply = true; const bool queued = reply && (!mesh::Packet::isValidPathLen(client->out_path_len) ? sendFloodReply(reply, delay_millis + SERVER_RESPONSE_DELAY, packet->getPathHashSize()) : sendDirect(reply, client->out_path, client->out_path_len, delay_millis + SERVER_RESPONSE_DELAY)); @@ -3849,7 +3848,7 @@ void MyMesh::onUserGpioTimerCompleted(uint8_t pin, uint8_t state, } } } - if (client == NULL) return; + if (client == NULL || !client->isAdmin()) return; uint8_t data[72]; uint32_t timestamp = getRTCClock()->getCurrentTimeUnique(); @@ -3865,7 +3864,7 @@ void MyMesh::onUserGpioTimerCompleted(uint8_t pin, uint8_t state, client->shared_secret, data, 5 + (size_t)text_len); if (!packet) return; - packet->radio_reply = true; // delayed GPIO command completion + packet->radio_reply = true; // delayed admin GPIO command completion if (!mesh::Packet::isValidPathLen(client->out_path_len)) { sendFlood(packet, SERVER_RESPONSE_DELAY, path_hash_size); } else { diff --git a/examples/simple_sensor/SensorMesh.cpp b/examples/simple_sensor/SensorMesh.cpp index 08826f509..a8f7b3eab 100644 --- a/examples/simple_sensor/SensorMesh.cpp +++ b/examples/simple_sensor/SensorMesh.cpp @@ -681,7 +681,7 @@ void SensorMesh::onUserGpioTimerCompleted(uint8_t pin, uint8_t state, } } } - if (client == NULL) return; + if (client == NULL || !client->isAdmin()) return; uint8_t data[72]; uint32_t timestamp = getRTCClock()->getCurrentTimeUnique(); @@ -697,7 +697,7 @@ void SensorMesh::onUserGpioTimerCompleted(uint8_t pin, uint8_t state, client->shared_secret, data, 5 + (size_t)text_len); if (!packet) return; - packet->radio_reply = true; // delayed GPIO command completion + packet->radio_reply = true; // delayed admin GPIO command completion if (!mesh::Packet::isValidPathLen(client->out_path_len)) { sendFlood(packet, CLI_REPLY_DELAY_MILLIS, path_hash_size); } else { @@ -1079,10 +1079,10 @@ void SensorMesh::onPeerDataRecv(mesh::Packet* packet, uint8_t type, int sender_i temp[4] = (TXT_TYPE_CLI_DATA << 2); auto reply = createDatagram(PAYLOAD_TYPE_TXT_MSG, from->id, secret, temp, 5 + text_len); + if (reply) reply->radio_reply = true; // authenticated admin CLI result const bool arms_morse = _cli.morseReply().generation() != morse_mutation_before; if (arms_morse) { _cli.morseReply().prepareReply(reply); - if (reply) reply->radio_reply = true; } bool queued = false; if (reply) { diff --git a/src/Dispatcher.cpp b/src/Dispatcher.cpp index aa937f256..31ccd216d 100644 --- a/src/Dispatcher.cpp +++ b/src/Dispatcher.cpp @@ -1372,7 +1372,9 @@ Packet* Dispatcher::obtainNewPacket() { pkt->radio_bound = false; pkt->radio_local = !receive_context_active; pkt->tx_radio = RADIO_TX_AUTO; - pkt->radio_reply = receive_context_active; + // RX context preserves the response's origin, but only authenticated + // admin CLI handlers may opt into the independent tx.reply override. + pkt->radio_reply = false; pkt->radio_reply_force = false; pkt->forwarded = false; #if defined(WITH_UDP_BRIDGE) diff --git a/src/Mesh.cpp b/src/Mesh.cpp index 235fc61da..ccc4db1c5 100644 --- a/src/Mesh.cpp +++ b/src/Mesh.cpp @@ -245,16 +245,16 @@ bool Mesh::otaSendAdapter(void* ctx, const uint8_t* msg, uint16_t len, bool /*fl || m->_mgr->getFreeCount() <= OTA_EGRESS_MIN_FREE)) return false; Packet* p = m->createOtaPacket(msg, len); if (!p) return false; - p->radio_reply = len && ota::ota_is_response_message(msg[0]); + p->radio_reply = false; // OTA follows normal crossing, not admin CLI policy. if (route.profile < 2) { // DATA, PROOF and MANIFEST are emitted after the RX call stack ends. - // Restore origin only; normal reply policy still decides one/both TX. + // Restore origin only; normal crossing still decides one/both TX. p->radio_profile = p->radio_origin = route.profile; p->radio_generation = p->radio_origin_generation = route.generation; p->radio_local = false; // A direct negotiation fallback stays on its original physical // session instead of silently moving onto a replacement profile. - if (!p->radio_reply) { + if (!(len && ota::ota_is_response_message(msg[0]))) { p->radio_bound = true; p->tx_radio = route.profile == 0 ? RADIO_TX_PRIMARY : RADIO_TX_SECONDARY; } @@ -838,7 +838,7 @@ uint32_t Mesh::getOtaPacketAirtime() const { const auto* profiles = _radio->profiles(); if (!profiles || !profiles->enabled()) return _radio->getProfileAirtime(0, MAX_TRANS_UNIT); // Use the participating profiles, never the scanner's instantaneous visit. - // Replies may occupy both profiles even with crossing off or radio2 RX-only. + // OTA uses normal crossing; admin CLI reply overrides do not apply here. uint8_t mask = (profiles->primary_temporary ? 1 : 0) | (profiles->secondary_temporary ? 2 : 0); if (!mask) mask = 1; // Each temporary origin can cross independently. In particular, an RX-only @@ -846,9 +846,6 @@ uint32_t Mesh::getOtaPacketAirtime() const { for (uint8_t origin = 0; origin < 2; ++origin) { if (mask & (1U << origin)) mask |= profiles->transmitMask(origin); } - if (profiles->reply_tx != RADIO_TX_AUTO) { - mask |= explicitRadioTxMask(profiles->reply_tx, profiles->canTransmit(1, profiles->reply_force)); - } uint32_t airtime = 0; for (uint8_t profile = 0; profile < 2; ++profile) { if (mask & (1U << profile)) airtime += _radio->getProfileAirtime(profile, MAX_TRANS_UNIT); diff --git a/src/Packet.h b/src/Packet.h index 4f4d4d5ff..9bdf06f1b 100644 --- a/src/Packet.h +++ b/src/Packet.h @@ -68,7 +68,7 @@ public: bool radio_bound = false; // retries retain one profile and never fan out again bool radio_local = true; uint8_t tx_radio = RADIO_TX_AUTO; // local policy, never sent over the air - bool radio_reply = false; // locally generated response, never a forwarded packet + bool radio_reply = false; // authenticated admin CLI reply; never serialized bool radio_reply_force = false; // captured reply-only RX-mode override #if defined(WITH_UDP_BRIDGE) // Volatile origin marker, copied with radio retries but never serialized. diff --git a/src/RadioProfiles.h b/src/RadioProfiles.h index 739dfd228..d5fe3b90e 100644 --- a/src/RadioProfiles.h +++ b/src/RadioProfiles.h @@ -73,8 +73,8 @@ class RadioProfiles { bool secondary_temporary = false; RadioCrossMode cross = RadioCrossMode::Off; // Infrastructure opts into BOTH at startup; Companions keep their own - // contact/channel policy. Force applies only to locally generated replies. - uint8_t reply_tx = RADIO_TX_AUTO; + // contact/channel policy. Force applies only to authenticated admin CLI replies. + uint8_t reply_tx = RADIO_TX_AUTO; // authenticated admin CLI replies only bool reply_force = false; uint32_t generation[2] = {1, 1}; uint32_t switches = 0; diff --git a/src/RadioTxPolicy.h b/src/RadioTxPolicy.h index 4c59d5c85..0fb572059 100644 --- a/src/RadioTxPolicy.h +++ b/src/RadioTxPolicy.h @@ -7,7 +7,7 @@ namespace mesh { // Local outbound choice. AUTO follows the radio2.cross policy; explicit masks // choose profiles independently of cross. Mode permissions are enforced -// separately, including the infrastructure reply-only RX override. +// separately, including the infrastructure admin CLI reply-only RX override. enum RadioTxPolicy : uint8_t { RADIO_TX_AUTO = 0, RADIO_TX_PRIMARY = 1, diff --git a/src/helpers/FleetChannel.cpp b/src/helpers/FleetChannel.cpp index 2cdc98224..5ce5aade3 100644 --- a/src/helpers/FleetChannel.cpp +++ b/src/helpers/FleetChannel.cpp @@ -301,7 +301,6 @@ bool FleetChannel::acknowledge(Mesh& mesh, const char* name, uint32_t sequence, const size_t length = count >= int(sizeof(data) - 5) ? sizeof(data) - 6 : size_t(count); Packet* packet = mesh.createGroupDatagram(PAYLOAD_TYPE_GRP_TXT, channel_, data, 5 + length); if (!packet) return false; - packet->radio_reply = true; packet->flood_retry_policy = FLOOD_RETRY_POLICY_DENY; // Acknowledgements have one tracked physical attempt per profile. A later // untracked retry must never trigger a completed radio transition twice. diff --git a/test/README.md b/test/README.md index 1f1ff4db0..9a255a824 100644 --- a/test/README.md +++ b/test/README.md @@ -64,7 +64,8 @@ python3 test/test_esp32_tinyusb_nonblocking.py # Native CDC stalled-host/64-byte python3 test/test_esp32_tinyusb_role_hygiene.py # Repeater/room nonblocking console and paced large replies python3 test/test_esp32_tinyusb_cooperative_output.py # Real role pumps: large logs/listings, EOF, backpressure python3 test/test_temp_radio_reply_delivery_contract.py # TempRadio ACK path/barrier integration -python3 test/test_reply_tx_integration.py # Delayed OTA replies, backpressure, and all infrastructure roles +python3 test/test_reply_tx_integration.py # OTA origin, normal crossing, and backpressure +python3 test/test_admin_cli_reply_scope.py # Full-admin CLI replies; ordinary traffic stays under radio2.cross python3 test/test_repeater_radio_timing_integration.py # Real scheduler, watchdog, hourly/flood adverts and restoration python3 test/test_tls_download_clock_gates.py # Fresh-NTP/TLS download integration contract ``` diff --git a/test/fixtures/radio_profiles/mesh_tests.h b/test/fixtures/radio_profiles/mesh_tests.h index 3b5a21d9f..6a44dd361 100644 --- a/test/fixtures/radio_profiles/mesh_tests.h +++ b/test/fixtures/radio_profiles/mesh_tests.h @@ -127,13 +127,15 @@ class DualProfileTestMesh : public RetryCodingRateMesh { ++cross_filter_calls; return cross_filter_allows; } - mesh::Packet* replyOn(uint8_t profile, uint32_t generation) { + mesh::Packet* replyOn(uint8_t profile, uint32_t generation, bool admin_cli = false) { ReceiveProfileScope context(*this, profile, generation); auto* packet = obtainNewPacket(); if (!packet) return nullptr; - packet->header = (PAYLOAD_TYPE_RESPONSE << PH_TYPE_SHIFT) | ROUTE_TYPE_DIRECT; + packet->header = ((admin_cli ? PAYLOAD_TYPE_TXT_MSG : PAYLOAD_TYPE_RESPONSE) + << PH_TYPE_SHIFT) | ROUTE_TYPE_DIRECT; packet->setPathHashSizeAndCount(1, 0); packet->payload_len = 1; packet->payload[0] = 0x5a; + if (admin_cli) packet->radio_reply = true; return packet; } }; @@ -214,13 +216,13 @@ TEST_F(DualProfileTest, OtaAirtimeFollowsParticipatingProfilesInsteadOfScannerVi radio.estimated_airtime = 12345; EXPECT_EQ(node.getOtaPacketAirtime(), 70u); radio.config.reply_tx = mesh::RADIO_TX_BOTH; - EXPECT_EQ(node.getOtaPacketAirtime(), 970u); + EXPECT_EQ(node.getOtaPacketAirtime(), 70u); radio.config.secondary_temporary = false; radio.config.primary_temporary = true; radio.config.secondary.mode = mesh::RadioProfileMode::Rx; EXPECT_EQ(node.getOtaPacketAirtime(), 900u); radio.config.reply_force = true; - EXPECT_EQ(node.getOtaPacketAirtime(), 970u); + EXPECT_EQ(node.getOtaPacketAirtime(), 900u); radio.config.secondary.mode = mesh::RadioProfileMode::Off; EXPECT_EQ(node.getOtaPacketAirtime(), 900u); } @@ -811,7 +813,45 @@ TEST_F(DualProfileTest, ExplicitSecondaryRetriesStayOnSecondaryWithCrossOff) { EXPECT_EQ(40, manager.getFreeCount()); } -TEST_F(DualProfileTest, InfrastructureReplyChoiceMatrix) { +TEST_F(DualProfileTest, OrdinaryRepliesFollowCrossingRegardlessOfAdminCliPolicy) { + for (auto cross : {mesh::RadioCrossMode::Auto, mesh::RadioCrossMode::On, mesh::RadioCrossMode::Off}) { + radio.config.cross = cross; + radio.config.secondary_temporary = true; + for (auto mode : {mesh::RadioProfileMode::Off, mesh::RadioProfileMode::Rx, mesh::RadioProfileMode::RxTx}) { + radio.config.secondary.mode = mode; + for (uint8_t policy : {mesh::RADIO_TX_BOTH, mesh::RADIO_TX_SECONDARY, mesh::RADIO_TX_OFF}) { + radio.config.reply_tx = policy; + radio.config.reply_force = true; + for (uint8_t origin : {0, 1}) { + for (uint8_t type : {PAYLOAD_TYPE_RESPONSE, PAYLOAD_TYPE_ACK, PAYLOAD_TYPE_PATH, + PAYLOAD_TYPE_GRP_TXT, PAYLOAD_TYPE_OTA}) { + auto* packet = node.replyOn(origin, radio.config.generation[origin]); + ASSERT_NE(nullptr, packet); + ASSERT_FALSE(packet->radio_reply); + EXPECT_FALSE(packet->radio_local); + packet->header = (type << PH_TYPE_SHIFT) | ROUTE_TYPE_DIRECT; + const uint8_t expected = radio.config.transmitMask(origin); + EXPECT_EQ(expected, node.getTransmitProfileMask(packet)); + ASSERT_EQ(expected != 0, node.sendPacket(packet, 0)); + uint8_t actual = 0; + while (manager.getOutboundTotal()) { + auto* queued = manager.removeOutboundByIdx(0); + actual |= 1U << queued->radio_profile; + EXPECT_FALSE(queued->radio_reply); + EXPECT_FALSE(queued->radio_reply_force); + EXPECT_EQ(mesh::RADIO_TX_AUTO, queued->tx_radio); + node.releasePacket(queued); + } + EXPECT_EQ(expected, actual); + EXPECT_EQ(40, manager.getFreeCount()); + } + } + } + } + } +} + +TEST_F(DualProfileTest, AdminCliReplyChoiceMatrix) { for (auto cross : {mesh::RadioCrossMode::Auto, mesh::RadioCrossMode::On, mesh::RadioCrossMode::Off}) { radio.config.cross = cross; for (auto secondary : {mesh::RadioProfileMode::Off, mesh::RadioProfileMode::Rx, mesh::RadioProfileMode::RxTx}) { @@ -823,7 +863,7 @@ TEST_F(DualProfileTest, InfrastructureReplyChoiceMatrix) { if (force && policy!=mesh::RADIO_TX_SECONDARY && policy!=mesh::RADIO_TX_BOTH) continue; radio.config.reply_tx=policy; radio.config.reply_force=force; for (uint8_t origin : {0,1}) { - auto* packet=node.replyOn(origin,radio.config.generation[origin]); + auto* packet=node.replyOn(origin,radio.config.generation[origin],true); ASSERT_NE(nullptr,packet); ASSERT_TRUE(packet->radio_reply); const bool can_second=secondary==mesh::RadioProfileMode::RxTx || (force && secondary==mesh::RadioProfileMode::Rx); @@ -878,7 +918,7 @@ TEST_F(DualProfileTest, ForcedReplySurvivesDriverRetryOnRxOnlySecondary) { radio.config.cross=mesh::RadioCrossMode::Off; radio.config.secondary.mode=mesh::RadioProfileMode::Rx; radio.config.reply_tx=mesh::RADIO_TX_SECONDARY; radio.config.reply_force=true; - auto* packet=node.replyOn(0,radio.config.generation[0]); ASSERT_NE(nullptr,packet); + auto* packet=node.replyOn(0,radio.config.generation[0],true); ASSERT_NE(nullptr,packet); ASSERT_TRUE(node.sendPacket(packet,0)); radio.fail_next_send=true; for (int i=0;i<12;++i) { radio.complete=true; tick(1000); } @@ -892,7 +932,7 @@ TEST_F(DualProfileTest, BothForcedReplyCopiesKeepSeparateFloodRetries) { radio.config.secondary.mode=mesh::RadioProfileMode::Rx; radio.config.secondary_temporary=true; radio.config.reply_tx=mesh::RADIO_TX_BOTH; radio.config.reply_force=true; - auto* packet=node.replyOn(1,radio.config.generation[1]); ASSERT_NE(nullptr,packet); + auto* packet=node.replyOn(1,radio.config.generation[1],true); ASSERT_NE(nullptr,packet); packet->header=PAYLOAD_TYPE_GRP_TXT << PH_TYPE_SHIFT; ASSERT_TRUE(node.sendFlood(packet)); for (int i=0;i<50;++i) { radio.complete=true; tick(1000); } @@ -906,18 +946,18 @@ TEST_F(DualProfileTest, ForcedReplyRespectsCrossFiltersAndProfileExpiry) { radio.config.reply_tx=mesh::RADIO_TX_BOTH; radio.config.reply_force=true; radio.config.secondary.mode=mesh::RadioProfileMode::Rx; node.cross_filter_allows=false; - auto* reply=node.replyOn(0,radio.config.generation[0]); ASSERT_NE(nullptr,reply); + auto* reply=node.replyOn(0,radio.config.generation[0],true); ASSERT_NE(nullptr,reply); ASSERT_TRUE(node.sendPacket(reply,0)); ASSERT_EQ(1,manager.getOutboundTotal()); EXPECT_EQ(0,manager.getOutboundByIdx(0)->radio_profile); node.releasePacket(manager.removeOutboundByIdx(0)); node.cross_filter_allows=true; - reply=node.replyOn(1,radio.config.generation[1]); ASSERT_NE(nullptr,reply); + reply=node.replyOn(1,radio.config.generation[1],true); ASSERT_NE(nullptr,reply); ASSERT_TRUE(node.sendPacket(reply,0)); ASSERT_EQ(2,manager.getOutboundTotal()); radio.config.setSecondary({},false); for (int i=0;i<6;++i) { radio.complete=true; tick(); } EXPECT_TRUE(radio.transmissions.empty()); // both originated in an expired session EXPECT_EQ(40,manager.getFreeCount()); - auto* stale=node.replyOn(1,radio.config.generation[1]-1); ASSERT_NE(nullptr,stale); + auto* stale=node.replyOn(1,radio.config.generation[1]-1,true); ASSERT_NE(nullptr,stale); EXPECT_FALSE(node.sendPacket(stale,0)); EXPECT_EQ(40,manager.getFreeCount()); } @@ -943,7 +983,7 @@ TEST_F(DualProfileTest, ReplyAirtimeIncludesForcedRxProfileBeforeAdmission) { radio.distinguish_airtime=true; radio.config.secondary.mode=mesh::RadioProfileMode::Rx; radio.config.reply_tx=mesh::RADIO_TX_BOTH; - auto* reply=node.replyOn(0,radio.config.generation[0]); ASSERT_NE(nullptr,reply); + auto* reply=node.replyOn(0,radio.config.generation[0],true); ASSERT_NE(nullptr,reply); EXPECT_EQ(900U,node.getTransmitAirtime(reply)); radio.config.reply_force=true; EXPECT_EQ(970U,node.getTransmitAirtime(reply)); @@ -1048,11 +1088,10 @@ TEST_F(DualProfileTest, AsyncOtaResponseAdmissionMaskMatchesTheActualQueuedCopie mesh::ota::OTA_HAVE,mesh::ota::OTA_QUERY}) { auto* p=node.obtainNewPacket(); ASSERT_NE(nullptr,p); *p=makeFloodPacket(PAYLOAD_TYPE_OTA); p->payload[0]=type; - p->radio_reply=mesh::ota::ota_is_response_message(type); + EXPECT_FALSE(p->radio_reply); ASSERT_TRUE(p->radio_local); // no RX call stack const uint8_t mask=node.getTransmitProfileMask(p); - EXPECT_EQ(p->radio_reply ? ((force || mode==mesh::RadioProfileMode::RxTx) ? 3 : 1) - : (mode==mesh::RadioProfileMode::RxTx ? 2 : 0),mask); + EXPECT_EQ(mode==mesh::RadioProfileMode::RxTx ? 2 : 0,mask); ASSERT_EQ(mask!=0,node.sendPacket(p,0)); uint8_t actual=0; while (manager.getOutboundTotal()) { diff --git a/test/fixtures/room_topic_delivery/test.cpp b/test/fixtures/room_topic_delivery/test.cpp index c09c5c87b..66739175c 100644 --- a/test/fixtures/room_topic_delivery/test.cpp +++ b/test/fixtures/room_topic_delivery/test.cpp @@ -141,7 +141,7 @@ struct MyMesh { mesh::Packet* obtainNewPacket() { return alternate_available ? &alternate_packet : nullptr; } void releasePacket(mesh::Packet* out) { assert(out == &alternate_packet); ++releases; } bool queue(mesh::Packet* out) { - assert(out->radio_reply); + assert(!out->radio_reply); // room posts do not get the admin CLI override if (!queue_available) return false; Sent entry; memcpy(&entry.timestamp, out->payload, 4); diff --git a/test/test_admin_cli_reply_scope.py b/test/test_admin_cli_reply_scope.py new file mode 100644 index 000000000..a87fedf0e --- /dev/null +++ b/test/test_admin_cli_reply_scope.py @@ -0,0 +1,316 @@ +#!/usr/bin/env python3 +"""Execute role reply helpers to check full-admin CLI and GPIO policy scope.""" + +from pathlib import Path +import os +import re +import subprocess +import sys +import tempfile +import unittest + +from test_replay_reset_integration import extract_braced + +ROOT = Path(__file__).resolve().parents[1] + +HARNESS = r''' +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +@CONSTANTS@ +namespace mesh { +struct Identity { uint8_t pub_key[PUB_KEY_SIZE]{}; }; +struct Utils { + static void sha256(uint8_t* out, size_t size, const uint8_t*, size_t, + const uint8_t* = nullptr, size_t = 0) { memset(out, 0x11, size); } +}; +@PACKET_METHODS@ +} +using namespace mesh; +@CLIENT_INFO@; +@GPIO_STATE@ +struct TransportKey {}; +struct Clock { + uint32_t getCurrentTimeUnique() { return 1000; } + uint32_t getCurrentTime() { return 1000; } +}; +struct RNG { void random(uint8_t* out, size_t size) { memset(out, 0x42, size); } }; +struct Radio { + mesh::RadioProfiles config; + const mesh::RadioProfiles* profiles() const { return &config; } +}; +struct ACL { + ClientInfo clients[3]{}; + int count=2; + int getNumClients() const { return count; } + ClientInfo* getClientByIdx(int i) { assert(i>=0 && i data; }; + std::vector sent; + MyMesh() { + radio.config.secondary.mode=mesh::RadioProfileMode::RxTx; + radio.config.cross=mesh::RadioCrossMode::Off; + radio.config.reply_tx=mesh::RADIO_TX_BOTH; + for (int i=0;iradio_reply,getTransmitProfileMask(p), + std::vector(p->payload,p->payload+p->payload_len)}); + return true; + } + bool sendClientReplyWithFallbackScope(ClientInfo*, mesh::Packet* p, + unsigned long, uint8_t, const TransportKey*, bool copies) { + redundant=copies; return queue(p); + } + bool sendClientReply(ClientInfo*, mesh::Packet* p, unsigned long, uint8_t) { return queue(p); } + bool sendFlood(mesh::Packet* p, unsigned long, uint8_t) { return queue(p); } + bool sendFloodScoped(uint8_t,mesh::Packet* p,unsigned long,uint8_t) { return queue(p); } + bool sendDirect(mesh::Packet* p,const uint8_t*,uint8_t,unsigned long) { return queue(p); } + void replaceActiveMessageRetries(mesh::Packet*,const uint8_t*,uint32_t) {} + bool sendRemoteCliReply(ClientInfo*,const uint8_t*,uint8_t,uint32_t, + const char*,const TransportKey*,mesh::Packet** = nullptr); + void onUserGpioTimerCompleted(uint8_t,uint8_t,uint32_t); + bool pushRoomTextToClient(ClientInfo*,uint32_t,const mesh::Identity&, + const char*,uint32_t = 0); +#if SENSOR_ROLE + int matching_peer_indexes[MAX_SEARCH_RESULTS]{}; + void onPeerDataRecv(mesh::Packet*,uint8_t,int,const uint8_t*,uint8_t*,size_t); + uint8_t handleRequest(ClientInfo*,uint32_t,uint8_t,const uint8_t*,size_t,size_t) { + reply_data[0]=0x5a; return 1; + } + bool handleIncomingMsg(ClientInfo&,uint32_t,uint8_t*,uint8_t,size_t) { return true; } + void handleCommand(uint32_t,char*,char* reply,int,uint8_t) { strcpy(reply,"OK"); } + bool sendFloodReply(mesh::Packet* p,unsigned long,uint8_t) { return queue(p); } + void sendAckTo(ClientInfo& client,uint32_t,uint8_t) { + const uint8_t data=0; + queue(createDatagram(PAYLOAD_TYPE_ACK,client.id,client.shared_secret,&data,1)); + } + mesh::Packet* createPathReturn(const mesh::Identity& id,const uint8_t* key, + const uint8_t*,uint8_t,uint8_t type,const uint8_t* data,size_t length) { + return createDatagram(type,id,key,data,length); + } +#endif +}; +@METHODS@ +int main() { +#if SENSOR_ROLE + for (uint8_t role=0;role<=PERM_ACL_ROLE_MASK;++role) { + for (uint8_t flags : {TXT_TYPE_PLAIN,TXT_TYPE_CLI_DATA,TXT_TYPE_CLI_COMMAND}) { + for (bool request : {false,true}) { + MyMesh node; + auto* client=&node.acl.clients[0]; client->permissions=role; + uint8_t data[16]{}; + const uint32_t timestamp=900; + memcpy(data,×tamp,4); data[4]=flags<<2; + strcpy(reinterpret_cast(data+5),"get tx"); + mesh::Packet incoming; + incoming.header=ROUTE_TYPE_DIRECT; + node.onPeerDataRecv(&incoming,request ? PAYLOAD_TYPE_REQ : PAYLOAD_TYPE_TXT_MSG, + 0,client->shared_secret,data,11); + const bool admin=role==PERM_ACL_ADMIN; + const bool admin_cli=admin && !request && flags!=TXT_TYPE_PLAIN; + assert(node.sent.size()==(request || admin ? 1U : 0U)); + if (!node.sent.empty()) { + assert(node.sent[0].reply==admin_cli); + assert(node.sent[0].mask==(admin_cli ? 3 : 2)); + } + } + } + } +#endif +#if REPEATER_ROLE + for (unsigned permissions=0;permissions<256;++permissions) { + MyMesh node; + auto* client=&node.acl.clients[0]; client->permissions=permissions; + assert(node.sendRemoteCliReply(client,client->shared_secret,1,1000,"OK",nullptr)); + assert(node.sent.size()==1); + const bool admin=(permissions & PERM_ACL_ROLE_MASK)==PERM_ACL_ADMIN; + assert(node.sent[0].reply==admin && node.sent[0].mask==(admin ? 3 : 2)); + // The same helper emits cached, deferred, and immediate CLI results. + node.radio.config.secondary.mode=mesh::RadioProfileMode::Rx; + node.radio.config.reply_force=true; + assert(node.sendRemoteCliReply(client,client->shared_secret,1,1000,"cached OK",nullptr)); + assert(node.sent.back().mask==(admin ? 3 : 0)); + } + MyMesh failed; + auto* client=&failed.acl.clients[0]; + mesh::Packet* queued=&failed.packet; + failed.allocation_ok=false; + assert(!failed.sendRemoteCliReply(client,client->shared_secret,1,0,"OK",nullptr,&queued)); + assert(queued==nullptr && failed.sent.empty()); +#endif + for (uint8_t role=0;role<=PERM_ACL_ROLE_MASK;++role) { + for (bool move : {false,true}) { + MyMesh node; + const uint8_t original_key=node.acl.clients[0].id.pub_key[0]; + node._gpio_reply_tracker.beginCommand(0,2,node.acl.clients[0].id.pub_key); + node._gpio_reply_tracker.timerScheduled(7,123); + if (move) { + node.acl.clients[1]=node.acl.clients[0]; + memset(node.acl.clients[0].id.pub_key,0xAA,PUB_KEY_SIZE); + } + node.acl.clients[move ? 1 : 0].permissions=role; + node.onUserGpioTimerCompleted(7,UserGpio::STATE_OFF,123); + const bool admin=role==PERM_ACL_ADMIN; + assert(node.sent.size()==(admin ? 1U : 0U)); + if (admin) { + assert(node.sent[0].reply && node.sent[0].mask==3); + assert(node.acl.clients[move ? 1 : 0].id.pub_key[0]==original_key); + } + node.onUserGpioTimerCompleted(7,UserGpio::STATE_OFF,123); + assert(node.sent.size()==(admin ? 1U : 0U)); // route is consumed once + } + } + MyMesh removed; + removed._gpio_reply_tracker.beginCommand(0,1,removed.acl.clients[0].id.pub_key); + removed._gpio_reply_tracker.timerScheduled(3,456); + memset(removed.acl.clients[0].id.pub_key,0x55,PUB_KEY_SIZE); + removed.onUserGpioTimerCompleted(3,UserGpio::STATE_ON,456); + assert(removed.sent.empty()); // a replacement ACL slot cannot inherit the reply +#if ROOM_ROLE + for (bool flood : {false,true}) { + for (uint8_t role : {PERM_ACL_READ_WRITE,PERM_ACL_ADMIN}) { + MyMesh node; node.receiving=false; + node.radio.config.secondary.mode=mesh::RadioProfileMode::Rx; + node.radio.config.reply_force=true; + auto* client=&node.acl.clients[0]; client->permissions=role; + if (flood) client->out_path_len=OUT_PATH_UNKNOWN; + assert(node.pushRoomTextToClient(client,100,client->id,"room post")); + assert(node.sent.size()==1 && !node.sent[0].reply && node.sent[0].mask==1); + } + } +#endif + puts("full-admin CLI/GPIO reply scope and revoked identity checks passed"); +} +''' + + +class AdminCliReplyScopeTests(unittest.TestCase): + def test_role_helpers_gate_reply_override_and_reauthorize_gpio(self): + acl = (ROOT / "src/helpers/ClientACL.h").read_text() + packet = (ROOT / "src/Packet.cpp").read_text() + dispatcher = (ROOT / "src/Dispatcher.cpp").read_text() + constants = "\n".join(line for line in acl.splitlines() + if line.startswith(("#define PERM_ACL_", "#define OUT_PATH_"))) + for role, path, class_name in ( + ("repeater", "examples/simple_repeater", "MyMesh"), + ("room", "examples/simple_room_server", "MyMesh"), + ("sensor", "examples/simple_sensor", "SensorMesh")): + with self.subTest(role=role): + source = (ROOT / path / (class_name + ".cpp")).read_text() + header = (ROOT / path / (class_name + ".h")).read_text() + protocol = (ROOT / "src/Mesh.h").read_text() + labels = constants + names = ["SERVER_RESPONSE_DELAY"] + if role == "sensor": + names.append("TXT_ACK_DELAY") + if role != "room": + names.append("CLI_REPLY_DELAY_MILLIS") + else: + names += ["PUSH_ACK_TIMEOUT_FLOOD", + "PUSH_TIMEOUT_BASE", "PUSH_ACK_TIMEOUT_FACTOR"] + for name in names: + match = re.search(r"^\s*#define\s+" + name + r"\s+.*$", + source + "\n" + header + "\n" + protocol, re.MULTILINE) + self.assertIsNotNone(match, name) + labels += "\n" + match.group() + methods = extract_braced(source, "void " + class_name + "::onUserGpioTimerCompleted(") + methods = methods.replace(class_name + "::", "MyMesh::") + if role == "repeater": + methods += "\n" + extract_braced(source, "bool MyMesh::sendRemoteCliReply(") + elif role == "room": + methods += "\n" + extract_braced(source, "bool MyMesh::pushRoomTextToClient(") + else: + labels += "\n#define MAX_SEARCH_RESULTS 24" + methods += "\n" + extract_braced(source, "void SensorMesh::onPeerDataRecv(").replace( + "SensorMesh::", "MyMesh::") + mask = extract_braced(dispatcher, "uint8_t Dispatcher::getTransmitProfileMask(") + methods += "\n" + mask.replace("Dispatcher::", "MyMesh::") + generated = HARNESS.replace("@CONSTANTS@", labels) + generated = generated.replace("@CLIENT_INFO@", extract_braced(acl, "struct ClientInfo {")) + generated = generated.replace("@PACKET_METHODS@", "\n".join( + extract_braced(packet, signature) for signature in + ("Packet::Packet()", "bool Packet::isValidPathLen("))) + gpio = (ROOT / "src/helpers/UserGpio.cpp").read_text() + generated = generated.replace("@GPIO_STATE@", extract_braced(gpio, "const char* UserGpio::stateName(")) + generated = generated.replace("@METHODS@", methods) + with tempfile.TemporaryDirectory(prefix="admin-cli-reply-scope-") as directory: + work = Path(directory) + cpp = work / "test.cpp"; cpp.write_text(generated, encoding="ascii") + binary = work / "reply-scope" + sanitizers = (["-fsanitize=address,undefined", "-fno-sanitize-recover=all", + "-fno-omit-frame-pointer", "-fno-pie", "-no-pie"] + if sys.platform.startswith("linux") else []) + command = [os.environ.get("CXX", "g++"), "-std=c++17", "-O1", + "-Wall", "-Wextra", "-Werror", "-Wno-unused-parameter", *sanitizers, + "-DREPEATER_ROLE=" + str(int(role == "repeater")), + "-DROOM_ROLE=" + str(int(role == "room")), + "-DSENSOR_ROLE=" + str(int(role == "sensor")), + "-I" + str(ROOT / "src"), "-I" + str(ROOT / "test/mocks"), + str(cpp), "-o", str(binary)] + compiled = subprocess.run(command, capture_output=True, text=True, timeout=60) + self.assertEqual(compiled.returncode, 0, compiled.stdout + compiled.stderr) + checked = subprocess.run([str(binary)], capture_output=True, text=True, timeout=15) + self.assertEqual(checked.returncode, 0, checked.stdout + checked.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_fleet_channel_runtime.py b/test/test_fleet_channel_runtime.py index 4c943724b..0cf65a564 100644 --- a/test/test_fleet_channel_runtime.py +++ b/test/test_fleet_channel_runtime.py @@ -1154,7 +1154,7 @@ int main(int argc,char** argv){ assert(mesh.ack_codes[0]==scope.calcTransportCode(mesh.manager.outbound[0])); assert(mesh.ack_codes[0]!=packet.transport_codes[0]); assert(mesh.ack_delay==1000&&mesh.ack_path_size==2&&last_jitter_max==10000); - assert(mesh.manager.outbound[0]->radio_reply&&!mesh.manager.outbound[0]->radio_bound); + assert(!mesh.manager.outbound[0]->radio_reply&&!mesh.manager.outbound[0]->radio_bound); assert(mesh.manager.outbound[0]->flood_retry_policy==FLOOD_RETRY_POLICY_DENY); fleet.receive(&packet,mesh);assert(config(fleet,"get fleet.stats").find("busy=1")!=std::string::npos); fleet.complete(mesh.manager.removeOutboundByIdx(0),profiles);assert(fleet.waiting());assert(profiles.commits==0); diff --git a/test/test_repeater_guest_access.py b/test/test_repeater_guest_access.py index 8e87e05ac..bc3680263 100644 --- a/test/test_repeater_guest_access.py +++ b/test/test_repeater_guest_access.py @@ -493,11 +493,11 @@ static void defaultPasswordWarnings() { const auto& login = mesh.sent[0]; const auto& warning = mesh.sent[1]; assert(login.packet.type == PAYLOAD_TYPE_RESPONSE && login.packet.payload_len == 13); assert(login.packet.payload[4] == RESP_SERVER_LOGIN_OK && login.packet.payload[12] == FIRMWARE_VER_LEVEL); - assert(login.delay == SERVER_RESPONSE_DELAY); + assert(login.delay == SERVER_RESPONSE_DELAY && !login.packet.radio_reply); assert(warning.packet.type == PAYLOAD_TYPE_TXT_MSG && warning.packet.payload_len == 25); assert(warning.packet.payload[4] == (TXT_TYPE_CLI_DATA << 2)); assert(!memcmp(warning.packet.payload + 5, "CHANGE YOUR PASSWORD", 20)); - assert(warning.packet.radio_reply && warning.delay == SERVER_RESPONSE_DELAY + 2000UL); + assert(!warning.packet.radio_reply && warning.delay == SERVER_RESPONSE_DELAY + 2000UL); assert(!warning.packet.direct && warning.hash_size == 2); assert(mesh.path_returns == (flood ? 1U : 0U)); assert(mesh.client_replies == 0); // Unknown routes use the scoped flood reply boundary. diff --git a/test/test_reply_tx_integration.py b/test/test_reply_tx_integration.py index 576367b9e..f043776c8 100644 --- a/test/test_reply_tx_integration.py +++ b/test/test_reply_tx_integration.py @@ -1,4 +1,4 @@ -"""Exercise delayed OTA response classification and infrastructure reply wiring.""" +"""Keep OTA responses separate from the admin CLI reply policy.""" from pathlib import Path import os import re @@ -95,16 +95,16 @@ int main() { assert(node.packet.radio_generation==route.generation); assert(node.ota_manager.replyRoute().profile==0xFF); - // Infrastructure reply-both still overrides AUTO origin selection, including - // a receive-only secondary when the configured reply policy forces it. + // Admin CLI reply-both/force must not change an OTA response's origin or + // enable its otherwise forbidden transmit profile. node.ota_manager.clearPendingEgress(); node.masks.clear(); node.radio.config.reply_tx=mesh::RADIO_TX_BOTH; node.radio.config.reply_force=true; - node.radio.config.secondary.mode=mesh::RadioProfileMode::Rx; assert(node.ota_manager.on_message(wire,length,route)); node.service(); - assert(node.masks.size()==1 && node.masks[0]==3); - node.radio.config.reply_tx=mesh::RADIO_TX_AUTO; + assert(node.masks.size()==1 && node.masks[0]==2); + assert(!node.packet.radio_reply); + node.radio.config.reply_tx=mesh::RADIO_TX_OFF; node.radio.config.reply_force=false; node.radio.config.secondary.mode=mesh::RadioProfileMode::RxTx; @@ -195,7 +195,7 @@ int main() { node.manager.free=40; node.service(); assert(node.masks.size()==1 && node.masks[0]==2); - assert(node.packet.radio_reply && !node.packet.radio_local); + assert(!node.packet.radio_reply && !node.packet.radio_local); assert(node.ota_manager.pendingManifestJobs()==0); // Deferred affinity cannot bleed into a later local send. const uint8_t data=mesh::ota::OTA_DATA; @@ -238,7 +238,7 @@ int main() { result = subprocess.run([str(binary)], capture_output=True, text=True) self.assertEqual(result.returncode, 0, result.stderr) - def test_async_ota_adapter_marks_only_response_types_and_retains_backpressure(self): + def test_async_ota_adapter_excludes_admin_policy_and_retains_backpressure(self): source = (ROOT / 'src/Mesh.cpp').read_text() adapter = method(source, 'bool Mesh::otaSendAdapter(') paced = method(source, 'static bool isPacedOtaResponse(') @@ -293,9 +293,8 @@ int main() { mesh::Mesh node; for (unsigned type=0;type<256;++type) { const uint8_t msg=type; - bool reply=type==0x03 || type==0x05 || type==0x07 || type==0x09 || type==0x0b; assert(node.otaSendAdapter(&node,&msg,1,true)); - assert(node.was_reply==reply); + assert(!node.was_reply); } const uint8_t data=mesh::ota::OTA_DATA, proof=mesh::ota::OTA_PROOF; const uint8_t manifest=mesh::ota::OTA_MANIFEST, leaves=mesh::ota::OTA_LEAVES; @@ -331,7 +330,7 @@ int main() { assert(!node.otaSendAdapter(&node,&data,1,true)); // Use the actual Packet and deferred-route definitions. A routed reply - // retains its origin without binding normal reply fan-out. A negotiation + // retains its origin and normal crossing policy. A negotiation // fallback is not a reply and must stay bound to its original radio. node.admit=true; node.mask=1; mesh::ota::Context context; @@ -342,7 +341,7 @@ int main() { for (uint8_t type : {mesh::ota::OTA_QUERY,mesh::ota::OTA_DATA}) { assert(node.otaSendAdapter(&node,&type,1,false)); const bool reply=type==mesh::ota::OTA_DATA; - assert(node.packet.radio_reply==reply && !node.packet.radio_local); + assert(!node.packet.radio_reply && !node.packet.radio_local); assert(node.packet.radio_profile==profile && node.packet.radio_origin==profile); assert(node.packet.radio_generation==1234+profile); assert(node.packet.radio_origin_generation==1234+profile); @@ -394,18 +393,6 @@ int main() { self.assertIn('Assertion', checked.stderr) self.assertIn(failure, checked.stderr) - def test_all_infrastructure_roles_enable_replies_and_mark_delayed_completions(self): - common = (ROOT / 'src/helpers/CommonCLI.cpp').read_text() - self.assertIn('_radio_profiles.begin(fs, _callbacks->getProfileRadio(), _rtc, true);', common) - for path in ['examples/simple_repeater/MyMesh.cpp', - 'examples/simple_room_server/MyMesh.cpp', - 'examples/simple_sensor/SensorMesh.cpp']: - with self.subTest(path=path): - text = (ROOT / path).read_text() - self.assertIn('_cli.loadPrefs(', text) - self.assertIn('packet->radio_reply = true;', text) - room = (ROOT / 'examples/simple_room_server/MyMesh.cpp').read_text() - self.assertIn('reply->radio_reply = true; // asynchronous response to the room subscription', room) if __name__ == '__main__': diff --git a/test/test_room_login_integration.py b/test/test_room_login_integration.py index afbdd011d..732060a7d 100644 --- a/test/test_room_login_integration.py +++ b/test/test_room_login_integration.py @@ -190,7 +190,7 @@ struct MyMesh { uint8_t last_request_type = 0; int handleRequest(ClientInfo*, uint32_t, uint8_t* payload, size_t, size_t) { ++requests; last_request_type = payload[0]; return 0; } void handleCommand(uint32_t, char*, char* reply, int, uint8_t) { strcpy(reply, "OK"); } - mesh::Packet* createAck(uint32_t ack) { ++post_acks; response.payload_len = 4; memcpy(response.payload, &ack, 4); return &response; } + mesh::Packet* createAck(uint32_t ack) { ++post_acks; response = mesh::Packet(); response.payload_len = 4; memcpy(response.payload, &ack, 4); return &response; } mesh::Packet* createMultiAck(uint32_t ack, uint8_t) { return createAck(ack); } struct Prefs { char password[16] = "admin"; @@ -201,7 +201,7 @@ struct MyMesh { FakeClock clock; FakeRng rng; unsigned long millis_now = 20000, dirty_contacts_expiry = 0, next_push = 0; - uint8_t contacts_save_failures = 0, reply_data[32] = {}; + uint8_t contacts_save_failures = 0, reply_data[MAX_PACKET_PAYLOAD] = {}; unsigned creations = 0, direct_sends = 0, flood_sends = 0, path_returns = 0; bool create_ok = true, queue_ok = true; unsigned fail_creation_on = 0; @@ -212,7 +212,7 @@ struct MyMesh { uint8_t response_secret[PUB_KEY_SIZE] = {}, response_key[PUB_KEY_SIZE] = {}; uint8_t returned_path[MAX_PATH_SIZE] = {}, direct_path[MAX_PATH_SIZE] = {}; uint8_t returned_path_len = 0, direct_path_len = 0, reply_hash_size = 0; - uint8_t captured_reply[32] = {}; + uint8_t captured_reply[MAX_PACKET_PAYLOAD] = {}; size_t captured_reply_length = 0; mesh::LogicalMessageCache recent_room_posts; unsigned posts = 0, post_acks = 0; @@ -245,7 +245,7 @@ struct MyMesh { bool sendDirect(mesh::Packet* packet, const uint8_t* path, uint8_t path_len, unsigned long delay) { assert(packet == &response && (delay == SERVER_RESPONSE_DELAY || delay == TXT_ACK_DELAY - || (packet->radio_reply && delay == SERVER_RESPONSE_DELAY + 2000UL))); + || delay == SERVER_RESPONSE_DELAY + 2000UL)); assert(mesh::Packet::isValidPathLen(path_len)); ++direct_sends; direct_path_len = path_len; @@ -256,7 +256,7 @@ struct MyMesh { } bool sendFloodReply(mesh::Packet* packet, unsigned long delay, uint8_t hash_size) { assert(packet == &response && (delay == SERVER_RESPONSE_DELAY || delay == TXT_ACK_DELAY - || (packet->radio_reply && delay == SERVER_RESPONSE_DELAY + 2000UL))); + || delay == SERVER_RESPONSE_DELAY + 2000UL)); ++flood_sends; reply_hash_size = hash_size; if (queue_ok) sent.push_back({created_type, std::vector(captured_reply, @@ -850,12 +850,12 @@ static void defaultPasswordWarnings() { const auto& response = mesh.sent[0]; const auto& warning = mesh.sent[1]; assert(response.type == PAYLOAD_TYPE_RESPONSE && response.data.size() == 13); assert(response.data[4] == RESP_SERVER_LOGIN_OK && response.data[12] == FIRMWARE_VER_LEVEL); - assert(response.delay == SERVER_RESPONSE_DELAY); + assert(response.delay == SERVER_RESPONSE_DELAY && !response.radio_reply); assert(warning.type == PAYLOAD_TYPE_TXT_MSG && warning.data.size() == 25); assert(warning.data[4] == (TXT_TYPE_CLI_DATA << 2)); assert(!memcmp(warning.data.data() + 5, "CHANGE YOUR PASSWORD", 20)); uint32_t timestamp; memcpy(×tamp, warning.data.data(), 4); - assert(timestamp != requested && warning.radio_reply); + assert(timestamp != requested && !warning.radio_reply); assert(warning.delay == SERVER_RESPONSE_DELAY + 2000UL && !warning.direct && warning.hash_size == 2); assert(mesh.path_returns == (flood ? 1U : 0U)); } @@ -881,6 +881,40 @@ static void defaultPasswordWarnings() { assert(replay.sent.size() == 2); puts("default password warning follows successful admin login without changing the binary response passed"); } +static void adminCliReplyScope() { + for (uint8_t role = 0; role <= PERM_ACL_ROLE_MASK; ++role) { + for (const uint8_t flags : {TXT_TYPE_CLI_DATA, TXT_TYPE_CLI_COMMAND}) { + for (const char* command : {"get outpath", "get room.catchup", "get tx"}) { + MyMesh mesh; seedKnown(mesh, role); + const auto receive = [&]() { + const uint32_t timestamp = 900; + std::vector data(5 + strlen(command) + 1); + memcpy(data.data(), ×tamp, 4); data[4] = flags << 2; + memcpy(data.data() + 5, command, strlen(command)); + auto packet = incoming(); + mesh.onPeerDataRecv(&packet, PAYLOAD_TYPE_TXT_MSG, 0, + mesh.acl.client.shared_secret, data.data(), data.size() - 1); + }; + receive(); + assert(mesh.sent.size() == 1); + assert(mesh.sent.back().radio_reply == (role == PERM_ACL_ADMIN)); + receive(); // Exact CLI retries use the same full-admin classification. + assert(mesh.sent.size() == 2); + assert(mesh.sent.back().radio_reply == (role == PERM_ACL_ADMIN)); + if (role == PERM_ACL_ADMIN) { + mesh.acl.client.permissions = PERM_ACL_READ_ONLY; + receive(); // A cached admin result cannot retain reply-policy privilege. + assert(mesh.sent.size() == 3 && !mesh.sent.back().radio_reply); + } + } + } + } + MyMesh mesh; seedKnown(mesh, PERM_ACL_ADMIN); + mesh.post(&mesh.acl.client, "ordinary room post", 900); + assert(mesh.posts == 1 && !mesh.sent.empty()); + for (const auto& sent : mesh.sent) assert(!sent.radio_reply); + puts("only full-admin CLI results and cached replies use reply policy; room-post ACKs do not passed"); +} int main(int argc, char** argv) { assert(argc == 2); if (!strcmp(argv[1], "authorization")) authorizationCases(); @@ -894,6 +928,7 @@ int main(int argc, char** argv) { else if (!strcmp(argv[1], "observed")) observedLoginPath(); else if (!strcmp(argv[1], "reconnect")) reconnectCursor(); else if (!strcmp(argv[1], "default-password")) defaultPasswordWarnings(); + else if (!strcmp(argv[1], "admin-cli-scope")) adminCliReplyScope(); else assert(false); } ''' @@ -998,6 +1033,9 @@ class RoomLoginIntegrationTests(unittest.TestCase): def test_default_password_admin_login_warns_after_the_binary_reply(self): self.run_case("default-password", "default password warning follows successful admin login") + def test_reply_policy_applies_only_to_full_admin_cli_results_and_cached_retries(self): + self.run_case("admin-cli-scope", "only full-admin CLI results and cached replies use reply policy") + def test_custom_factory_password_and_bounded_long_factory_password(self): for index, factory in enumerate(("factory-admin", "12345678901234567890")): with self.subTest(factory=factory):