From 3d4432e8489999354ebedc324cf87b2249f8a440 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Thu, 10 Sep 2026 19:33:54 -0700 Subject: [PATCH] Restore clearing all neighbors with an empty prefix Accept the upstream empty-prefix neighbor.remove command after whitespace normalization while preserving invalid-key checks. Cover repeater and room-server removal paths with regression tests. Fixes #8 --- docs/cli_command_availability.md | 4 +- docs/cli_commands.md | 8 +- examples/simple_repeater/MyMesh.cpp | 4 +- examples/simple_room_server/MyMesh.cpp | 3 +- src/helpers/CommonCLI.cpp | 13 +- src/helpers/CommonCLI.h | 2 + test/test_neighbor_remove_integration.py | 176 +++++++++++++++++++++++ 7 files changed, 201 insertions(+), 9 deletions(-) create mode 100644 test/test_neighbor_remove_integration.py diff --git a/docs/cli_command_availability.md b/docs/cli_command_availability.md index ee23d5484..7a488e2ca 100644 --- a/docs/cli_command_availability.md +++ b/docs/cli_command_availability.md @@ -110,7 +110,7 @@ should use command `0x42`. See [Companion radio binary protocol](companion_proto | Diagnostics | [`sensor`](#sensor-hardware-summary) | Hardware wiring summary | Yes | Yes | Yes | | Diagnostics | [`powerlog`](#powerlog) | Reset, shutdown, and boot-voltage summary | Yes | Yes | Yes | | Neighbors | [`neighbors`](cli_commands.md#list-nearby-neighbors) | Role with a neighbor table | Yes | Yes | Yes | -| Neighbors | [`neighbor.remove `](cli_commands.md#remove-a-neighbor) | Role with a mutable neighbor table | Yes | Yes | Yes | +| Neighbors | [`neighbor.remove [pubkey_prefix]`](cli_commands.md#remove-a-neighbor) | Role with a mutable neighbor table; omit prefix to clear all | Yes | Yes | Yes | | Neighbors | [`discover.neighbors`](cli_commands.md#discover-zero-hop-neighbors) | Repeater; some MQTT room servers | Yes | Yes | Yes | | Neighbors | [`discover.scopes`](cli_commands.md#discover-neighbor-scopes-mqtt-observer-neighbors-feature) | MQTT observer with compiled neighbor support | No | No | No | | Statistics | [`clear stats`](cli_commands.md#clear-stats) | All full-parser text CLI roles | Yes | Yes | Yes | @@ -276,7 +276,7 @@ should use command `0x42`. See [Companion radio binary protocol](companion_proto | Diagnostics | [`sensor`](#sensor-hardware-summary) | Hardware wiring summary | Yes | Yes | Yes | Yes | Yes | | Diagnostics | [`powerlog`](#powerlog) | Reset-reason summary | Yes | Yes | Yes | Yes | Yes | | Neighbors | [`neighbors`](cli_commands.md#list-nearby-neighbors) | Role with a neighbor table | Yes | Yes | Yes | Yes | Yes | -| Neighbors | [`neighbor.remove `](cli_commands.md#remove-a-neighbor) | Role with a mutable neighbor table | Yes | Yes | Yes | Yes | Yes | +| Neighbors | [`neighbor.remove [pubkey_prefix]`](cli_commands.md#remove-a-neighbor) | Role with a mutable neighbor table; omit prefix to clear all | Yes | Yes | Yes | Yes | Yes | | Neighbors | [`discover.neighbors`](cli_commands.md#discover-zero-hop-neighbors) | Repeater; some MQTT room servers | Yes | Yes | Yes | Yes | Yes | | Neighbors | [`discover.scopes`](cli_commands.md#discover-neighbor-scopes-mqtt-observer-neighbors-feature) | MQTT observer with compiled neighbor support | No | No | No | Feature | No | | Statistics | [`clear stats`](cli_commands.md#clear-stats) | Full parser | Yes | Yes | Yes | Yes | Yes | diff --git a/docs/cli_commands.md b/docs/cli_commands.md index 284deabc1..cc06de7bf 100644 --- a/docs/cli_commands.md +++ b/docs/cli_commands.md @@ -413,9 +413,15 @@ command `0x42` where the role supports the command; unavailable over LoRa. ### Remove a neighbor **Usage:** - `neighbor.remove ` +- `neighbor.remove` — remove all neighbors **Parameters:** -- `pubkey_prefix`: The public key of the node to remove from the neighbors list +- `pubkey_prefix`: An even-length hexadecimal prefix (up to the full 64-character + public key). All neighbors matching it are removed. Invalid keys are rejected. + +Omitting the prefix clears the table. An empty or whitespace-only prefix is +also accepted for compatibility with the mobile app's **Remove All Neighbors** +action. Newly heard zero-hop adverts can populate the table again. --- diff --git a/examples/simple_repeater/MyMesh.cpp b/examples/simple_repeater/MyMesh.cpp index 4b7188d6e..8eff6e992 100644 --- a/examples/simple_repeater/MyMesh.cpp +++ b/examples/simple_repeater/MyMesh.cpp @@ -5138,10 +5138,10 @@ void MyMesh::formatNeighborsReply(char *reply) { void MyMesh::removeNeighbor(const uint8_t *pubkey, int key_len) { #if MAX_NEIGHBOURS - if (pubkey == NULL || key_len <= 0 || key_len > PUB_KEY_SIZE) return; + if (key_len < 0 || key_len > PUB_KEY_SIZE || (key_len > 0 && pubkey == NULL)) return; for (int i = 0; i < MAX_NEIGHBOURS; i++) { NeighbourInfo *neighbour = &neighbours[i]; - if (memcmp(neighbour->id.pub_key, pubkey, key_len) == 0) { + if (key_len == 0 || memcmp(neighbour->id.pub_key, pubkey, key_len) == 0) { neighbours[i] = NeighbourInfo(); // clear neighbour entry } } diff --git a/examples/simple_room_server/MyMesh.cpp b/examples/simple_room_server/MyMesh.cpp index 36e0fafab..7d3d47b93 100644 --- a/examples/simple_room_server/MyMesh.cpp +++ b/examples/simple_room_server/MyMesh.cpp @@ -2083,9 +2083,10 @@ void MyMesh::formatNeighborsReply(char *reply) { void MyMesh::removeNeighbor(const uint8_t *pubkey, int key_len) { #if defined(WITH_MQTT_NEIGHBORS) + if (key_len < 0 || key_len > PUB_KEY_SIZE || (key_len > 0 && pubkey == NULL)) return; for (int i = 0; i < MAX_NEIGHBOURS; i++) { NeighbourInfo *neighbour = &neighbours[i]; - if (memcmp(neighbour->id.pub_key, pubkey, key_len) == 0) { + if (key_len == 0 || memcmp(neighbour->id.pub_key, pubkey, key_len) == 0) { neighbours[i] = NeighbourInfo(); // clear neighbour entry } } diff --git a/src/helpers/CommonCLI.cpp b/src/helpers/CommonCLI.cpp index 150b9790b..76a77846e 100644 --- a/src/helpers/CommonCLI.cpp +++ b/src/helpers/CommonCLI.cpp @@ -2569,12 +2569,19 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, char* command, char* re } } else if (memcmp(command, "neighbors", 9) == 0) { _callbacks->formatNeighborsReply(reply); - } else if (memcmp(command, "neighbor.remove ", 16) == 0) { - const char* hex = &command[16]; + } else if (strncmp(command, "neighbor.remove", 15) == 0 + && (command[15] == 0 || command[15] == ' ' || command[15] == '\t')) { + const char* hex = &command[15]; + while (*hex == ' ' || *hex == '\t' || *hex == '\r' || *hex == '\n') hex++; uint8_t pubkey[PUB_KEY_SIZE]; size_t hex_len = strlen(hex); int pubkey_len = (int)(hex_len / 2); - if (hex_len > 0 && hex_len <= PUB_KEY_SIZE * 2 && (hex_len & 1) == 0 + if (hex_len == 0) { + // Mobile clients clear all neighbors with an empty prefix. The + // repeater's command normalization may already have trimmed its space. + _callbacks->removeNeighbor(NULL, 0); + strcpy(reply, "OK"); + } else if (hex_len <= PUB_KEY_SIZE * 2 && (hex_len & 1) == 0 && mesh::Utils::fromHex(pubkey, pubkey_len, hex)) { _callbacks->removeNeighbor(pubkey, pubkey_len); strcpy(reply, "OK"); diff --git a/src/helpers/CommonCLI.h b/src/helpers/CommonCLI.h index 9ba125c05..0a4da7992 100644 --- a/src/helpers/CommonCLI.h +++ b/src/helpers/CommonCLI.h @@ -476,6 +476,8 @@ public: virtual void dumpLogFile() = 0; virtual bool setTxPower(int8_t power_dbm) = 0; virtual void formatNeighborsReply(char *reply) = 0; + // A zero-length prefix clears the whole table; pubkey may be null then. + // Nonempty prefixes must contain 1..PUB_KEY_SIZE bytes. virtual void removeNeighbor(const uint8_t* pubkey, int key_len) { // no op by default }; diff --git a/test/test_neighbor_remove_integration.py b/test/test_neighbor_remove_integration.py new file mode 100644 index 000000000..472d877f0 --- /dev/null +++ b/test/test_neighbor_remove_integration.py @@ -0,0 +1,176 @@ +"""Compile real neighbor CLI/removal code, including the remote whitespace path.""" +from pathlib import Path +import shutil +import subprocess +import tempfile +import unittest + +from test_replay_reset_integration import extract_braced + +ROOT = Path(__file__).resolve().parents[1] + + +class NeighborRemoveIntegrationTest(unittest.TestCase): + def test_neighbor_removal_and_mobile_clear_all(self): + compiler = shutil.which("g++") or shutil.which("clang++") + if compiler is None: + self.skipTest("a host C++17 compiler is required") + common = (ROOT / "src/helpers/CommonCLI.cpp").read_text(encoding="utf-8") + repeater = (ROOT / "examples/simple_repeater/MyMesh.cpp").read_text(encoding="utf-8") + room = (ROOT / "examples/simple_room_server/MyMesh.cpp").read_text(encoding="utf-8") + utils = (ROOT / "src/Utils.cpp").read_text(encoding="utf-8") + arm_start = common.rfind("else if", 0, common.index('"neighbor.remove')) + arm = extract_braced(common[arm_start:], "else if") + trim = extract_braced(repeater, "while (command_end > command") + prefix = extract_braced(repeater, "if (strlen(command) > 4 && command[2] == '|')") + definitions = "\n".join(extract_braced(utils, signature) for signature in ( + "static uint8_t hexVal(", "bool Utils::isHexChar(", "bool Utils::fromHex(")) + code = r''' +#include +#include +#include +#include +#include +#include +#include +#define PUB_KEY_SIZE 32 +#define MAX_NEIGHBOURS 4 +#define WITH_MQTT_NEIGHBORS 1 +namespace mesh { +struct Utils { + static bool isHexChar(char); + static bool fromHex(uint8_t*, int, const char*); +}; +''' + definitions + r''' +} +struct NeighbourInfo { + struct { uint8_t pub_key[PUB_KEY_SIZE] = {}; } id; + uint32_t heard_timestamp = 0; + int8_t snr = 0, rssi = 0; +}; +struct Callbacks { virtual void removeNeighbor(const uint8_t*, int) = 0; }; +struct CommonCLI { + Callbacks* _callbacks; + void handleCommand(char* command, char* reply) { + mesh::cli::normalizeCommandVerb(command); + if (false) {} +''' + arm + r''' + else strcpy(reply, "Unknown command"); + } +}; +''' + for namespace, source in (("repeater", repeater), ("room", room)): + code += "namespace " + namespace + r''' { +struct MyMesh : Callbacks { + NeighbourInfo neighbours[MAX_NEIGHBOURS]; + void removeNeighbor(const uint8_t*, int) override; +}; +''' + extract_braced(source, "void MyMesh::removeNeighbor(") + "\n}\n" + code += r''' +void send(CommonCLI& cli, const char* text, char* reply, bool remote) { + char storage[256] = {}; + strcpy(storage, text); + char* command = storage; + if (remote) { + char* command_end = command + strlen(command); +''' + trim + "\n" + prefix + r''' + } + cli.handleCommand(command, reply); +} +template void seed(Mesh& mesh) { + mesh.neighbours[0] = NeighbourInfo(); + mesh.neighbours[1] = NeighbourInfo(); + mesh.neighbours[2] = NeighbourInfo(); + mesh.neighbours[3] = NeighbourInfo(); + mesh.neighbours[0].id.pub_key[0] = 0xAB; + mesh.neighbours[1].id.pub_key[0] = 0xAB; + mesh.neighbours[1].id.pub_key[1] = 0xCD; + mesh.neighbours[2].id.pub_key[0] = 0x12; + for (int i = 0; i < 3; ++i) { + mesh.neighbours[i].heard_timestamp = 100 + i; + mesh.neighbours[i].snr = 5; + mesh.neighbours[i].rssi = -90; + } +} +template void check() { + Mesh mesh; + CommonCLI cli{&mesh}; + char reply[160] = {}; + for (bool remote : {true, false}) { + for (const char* clear : {"neighbor.remove ", "neighbor.remove", + "neighbor.remove ", "neighbor.remove \t\r\n", + "Neighbor.remove "}) { + seed(mesh); + send(cli, clear, reply, remote); + assert(!strcmp(reply, "OK")); + for (const auto& item : mesh.neighbours) { + assert(item.heard_timestamp == 0 && item.snr == 0 && item.rssi == 0); + for (uint8_t value : item.id.pub_key) assert(value == 0); + } + send(cli, clear, reply, remote); // Clearing an empty table is idempotent. + assert(!strcmp(reply, "OK")); + } + seed(mesh); + send(cli, "neighbor.remove aB", reply, remote); + assert(!strcmp(reply, "OK")); + assert(mesh.neighbours[0].heard_timestamp == 0); + assert(mesh.neighbours[1].heard_timestamp == 0); + assert(mesh.neighbours[2].heard_timestamp == 102); + + seed(mesh); + const std::string exact = "neighbor.remove abcd" + std::string(60, '0'); + send(cli, exact.c_str(), reply, remote); + assert(!strcmp(reply, "OK")); + assert(mesh.neighbours[0].heard_timestamp == 100); + assert(mesh.neighbours[1].heard_timestamp == 0); + assert(mesh.neighbours[2].heard_timestamp == 102); + + for (const auto& invalid : {std::string("neighbor.remove A"), + std::string("neighbor.remove AG"), std::string("neighbor.remove *"), + std::string("neighbor.remove AB CD"), std::string("neighbor.remove all"), + std::string("neighbor.remove ") + std::string(66, 'A')}) { + seed(mesh); + send(cli, invalid.c_str(), reply, remote); + assert(!strcmp(reply, "ERR: bad pubkey")); + for (int i = 0; i < 3; ++i) assert(mesh.neighbours[i].heard_timestamp == 100U+i); + } + seed(mesh); + send(cli, "neighbor.removeall", reply, remote); + assert(!strcmp(reply, "Unknown command")); + assert(mesh.neighbours[0].heard_timestamp == 100); + send(cli, "neighbor.remove FF", reply, remote); + assert(!strcmp(reply, "OK")); + assert(mesh.neighbours[0].heard_timestamp == 100); + } + seed(mesh); + send(cli, "ab|neighbor.remove \r\n", reply, true); + assert(!strcmp(reply, "ab|OK")); + assert(mesh.neighbours[0].heard_timestamp == 0); + + seed(mesh); + uint8_t key[PUB_KEY_SIZE] = {}; + mesh.removeNeighbor(nullptr, 1); + mesh.removeNeighbor(key, -1); + mesh.removeNeighbor(key, PUB_KEY_SIZE + 1); + assert(mesh.neighbours[0].heard_timestamp == 100); + mesh.removeNeighbor(nullptr, 0); + assert(mesh.neighbours[0].heard_timestamp == 0); +} +int main() { check(); check(); puts("neighbor removal checks passed"); } +''' + with tempfile.TemporaryDirectory(prefix="meshcore-neighbors-") as directory: + work = Path(directory) + source = work / "test.cpp" + source.write_text(code, encoding="utf-8") + binary = work / "neighbors.exe" + result = subprocess.run([ + compiler, "-std=c++17", "-Wall", "-Wextra", "-Werror", + "-I" + str(ROOT / "src"), str(source), "-o", str(binary), + ], capture_output=True, text=True, timeout=60) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + result = subprocess.run([str(binary)], capture_output=True, text=True, timeout=15) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + +if __name__ == "__main__": + unittest.main()