mirror of
https://github.com/meshtastic/firmware.git
synced 2026-08-01 19:08:59 -04:00
* feat(portduino): add `meshtasticd --check` config validator Users hand-writing files in /etc/meshtasticd/config.d/ get no feedback when a key is misplaced, misspelled or duplicated: meshtasticd silently ignores what it does not read, so a broken config looks identical to a working one. Add a --check mode that loads the configuration exactly as startup does, then reports what it found and exits: - Duplicate keys, via the yaml-cpp Parser/EventHandler stream. The Node API cannot see them because the map is already collapsed by the time it exists, and yaml-cpp keeps the FIRST occurrence, so a later override is discarded. - Unknown or misnested keys, against a schema mirroring what loadConfig() reads, with a hint naming the section a stray key actually belongs to. - rfswitch_table validation: unrecognised pins, mode rows whose length does not match the pin list, values that are not HIGH/LOW, and unknown modes. - Cross-file overlap: every .yaml in the config directory merges into one portduino_config, so the file loaded LAST wins, the opposite of the within-file rule. Those files are read in filesystem order, not alphabetical. - A warning when more than one file defines a Lora section: spidev, spiSpeed, gpiochip, DIO2_AS_RF_SWITCH, DIO3_TCXO_VOLTAGE and USB_PID/VID/Serialnum are assigned unconditionally with a default every time one is seen, so any of them not repeated in the last file loaded is silently reset. - The resolved gpiochip/line for each pin, since a line that exists on the wrong chip is claimed successfully and then silently does nothing. Exits non-zero when errors were found so it can also gate CI over bin/config.d/**, keeping one implementation rather than a second schema. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(portduino): flag pins that resolve to -1 in --check A pin key whose value will not convert to a number falls back to RADIOLIB_NC (-1) while still being marked enabled, and initGPIOPin() then trips an assertion inside LinuxGPIOPin rather than failing cleanly. YAML indentation makes this easy to hit by accident: a stray line under "CS: 8" folds into the value as a multi-line scalar, so the file parses, the daemon crashes with a stack trace from a library file, and --check reported "Configuration looks good" while printing "pin -1" two lines above. Report it as an error naming the likely cause instead. Also correct a comment claiming unparseable config.d files are skipped silently. They are not: loadConfig() prints "*** Exception ..." with the line and column. It is the discarded return value, not the diagnostic, that makes the file's absence from the merged config easy to miss. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(portduino): cover `meshtasticd --check` with fixtures and a fuzz suite Adds the tests the config validator was missing, and the checks and fixes that writing them turned up. The theme throughout is configuration that the YAML parser accepts but that does not mean what it looks like it means. Tests ----- bin/test-config-check.sh - 57 assertions driving a built meshtasticd against test/fixtures/portduino-config (50 fixtures plus two config.d trees). A shell test rather than a Unity suite because both behaviours under test are properties of the process: --check is judged by its exit status and printed report, and the "a normal run rejects a bad config" path ends in exit() inside portduinoSetup(), neither of which is reachable from a suite that links one translation unit. Every fixture carries a comment header naming its planted fault and the expected finding, so it can be read on its own. Coverage: * a clean config for each of the ten radio module families (RF95, sx1262, sx1268, LLCC68, sx1280, lr1110, lr1120, lr1121, sim, auto), asserted both findings-free and resolving to that module, so a silent fallback to sim cannot pass * LR11xx rfswitch tables: unrecognised pins, rows longer and shorter than the pin count, levels that are not exactly HIGH, a missing pins list, more than five pins, a scalar table, unknown MODE_ keys, a MODE_ row stranded one level out, and a legal partial table * the PA gain table in both accepted shapes, entries outside the uint16 range it is stored in, and more than the 22 points that are kept * values of the wrong type, split by consequence: the two settings read with no fallback stop meshtasticd starting, everything else is silently replaced by its default * out-of-range and unit mistakes: TCXO voltage written in millivolts, ports outside their usable range, an over-long StatusMessage * MAC sources: both keys set at once, a malformed address, an interface that does not exist * structural faults: duplicate keys, non-mapping and unknown sections, a key left at the top level, a sequence at the document root, an empty file, unreadable pins, unparseable YAML * cross-file behaviour over a config.d directory, including the switch tables that do not override each other * five configs run WITHOUT --check, each of which must still be refused, so check mode cannot quietly make the normal path permissive test/test_fuzz_config - adversarial fuzzing of the checker itself, the "the tool meant to diagnose your config crashes on it" failure mode. Scope is deliberately narrow: yaml-cpp does the parsing and is fuzzed upstream, so what is exercised here is our code above the parse, above all the duplicate-key detector, which is the one hand-rolled piece and walks the raw parser event stream with its own stack. Groups: the checked-in fixtures as a seed corpus, 3000 byte mutations of them (flips, truncation, insertion, splicing, deletion), and structural torture (nesting to 4096 in flow and block style, duplicate keys at depth, anchors, aliases and merge keys, 64KB keys, 256KB scalars, multi-document files). A fourth group of random bytes is present but disabled behind FUZZ_CONFIG_RANDOM_BYTES: it was half the runtime for the least return, since uniform noise is rejected on the first token. The contract is crash-freedom and termination under AddressSanitizer, not any particular finding. CI runs the shell test in the existing native simulator job; the fuzz suite is picked up by the existing ^test_fuzz_ area rule. native-suite-count 40 -> 41. The fixtures are exempt from trunk in .trunk/trunk.yaml, since prettier rejects the duplicate keys and bad indentation that are the point of them. Checker fixes found while writing the tests ------------------------------------------- --check reported a clean exit 0 on configs meshtasticd then refuses to boot, the worst failure a diagnostic tool can have. Four hard exits inside loadConfig() killed the report before it printed: an unparseable file, an unknown Lora.Module, MACAddress and MACAddressSource both set, and HUB75 on a build without it. All are now reported as findings, and all are still refused on a normal run. New validation: Lora.Module against the accepted spellings, which are matched exactly and inconsistently cased, with a suggestion when only case differs; a per-key value type table covering ~85 keys, tested by asking yaml-cpp to perform the same conversion loadConfig() will so it cannot drift; the PA gain table; DIO3_TCXO_VOLTAGE, which is in volts and multiplied by 1000, so the millivolt value everything else uses silently asks for 1800V; APIPort and Webserver.Port ranges; MaxNodes; StatusMessage truncation; MAC address and source; and an unreadable ConfigDirectory. Also fixes a crash: a ConfigDirectory that cannot be read threw an uncaught filesystem_error from directory_iterator and aborted meshtasticd with SIGABRT, taking --check down with it. It now fails cleanly. Two smaller ones: cppcheck's uselessCallsSubstr on the ancestor walk, which was failing every check job; and the duplicate-key detector's stack pop, which was unguarded and relied on yaml-cpp emitting balanced events. Switch tables are the one place "the file loaded last wins" is false. The loader only ever writes HIGH and never writes LOW back, so a HIGH from an earlier file survives a later file that clears it and the radio drives the OR of every table loaded. Confirmed with --output-yaml. Reported as an error for now; the loader itself is left alone, as that changes RF behaviour. * fix(portduino): report CH341 pins as adapter indexes, not gpiochip lines --check printed "Resolved GPIO lines (what meshtasticd will try to claim)" for every config, listing a gpiochip and line for each Lora pin and advising they be confirmed against gpiodetect and gpioinfo. For spidev: ch341 every part of that is false. portduinoSetup() skips initGPIOPin() for every Lora pin when spidev is ch341 and hands the raw numbers to Ch341Hal, so nothing is claimed from a gpiochip -- and on Windows and macOS, where a USB adapter is the only way to attach a radio, there is no gpiochip, gpiodetect or gpioinfo to check against in the first place. The checker had no ch341 coverage at all: not one fixture used it, so the whole USB-SPI path went unexercised. The summary now splits on the transport. A ch341 device gets its pins listed as adapter indexes with the gpiod advice dropped, and a gpiochip or line mapping written alongside it is reported: those are read, stored, and never used. Also: "RF switch table: not set" read as a gap on an SX126x, where there is nothing to set. setRfSwitchTable() is only ever called for an LR11xx, so absence is now "not needed for this module" everywhere else, and "not resolved yet" for auto, which has no module to judge against. Fixtures: usb-ch341.yaml (clean, the meshstick shape) and ch341-gpiochip.yaml. CI fix ------ test-native was RED on "config.d overrides are reported", which wanted 2 warnings and got 1. The fixture's two config.d files name different modules, so which one wins -- and whether the LR11xx-without-a-switch-table warning fires -- depends on the order the filesystem returns them in. That is the very thing the fixture exists to demonstrate, so the count is no longer asserted; the report's own order caveat is asserted instead. Review fixes ------------ The unreadable-ConfigDirectory diagnostic was the one new print in PortduinoGlue.cpp not gated behind !configCheck, so it landed ahead of the report header and broke the clean output the rest of the change is careful to keep. Docs: rfswitch-valid.yaml carries seven modes, not eight, and empty-file.yaml is comments-only rather than zero bytes. * style(portduino): trim --check comment blocks and reconcile suite count Condense the multi-paragraph comment blocks in the --check validator to the one-to-two-line convention, and bump test/native-suite-count to 42 for the test_fuzz_config suite added here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
268 lines
9.0 KiB
C++
268 lines
9.0 KiB
C++
// Adversarial fuzzing of `meshtasticd --check`, above all DuplicateKeyFinder, whose stack balance
|
|
// rests on yaml-cpp emitting matched start/end events. The contract is crash-freedom and
|
|
// termination only (what the report *says* is asserted by bin/test-config-check.sh); inputs come
|
|
// from a seeded LCG, so a failure reproduces from the printed seed.
|
|
|
|
// configuration.h pulls in Arduino.h, whose Common.h declares setup()/loop() inside an extern "C"
|
|
// block. Must come BEFORE TestUtil.h, same as the other suites.
|
|
#include "configuration.h"
|
|
|
|
#include "TestUtil.h"
|
|
#include <unity.h>
|
|
|
|
#include "ConfigCheck.h"
|
|
#include "support/DeterministicRng.h"
|
|
|
|
#include <cstdio>
|
|
#include <cstdlib>
|
|
#include <fcntl.h>
|
|
#include <filesystem>
|
|
#include <fstream>
|
|
#include <sstream>
|
|
#include <string>
|
|
#include <unistd.h>
|
|
#include <vector>
|
|
|
|
static constexpr uint64_t BASE_SEED = 0xC0FFEE01ULL;
|
|
|
|
// Where the checked-in fixtures live, relative to the repo root that `pio test` runs from.
|
|
static const char *kFixtureDir = "test/fixtures/portduino-config";
|
|
|
|
// Thousands of full reports would bury the CI log, so stdout goes to /dev/null for the suite.
|
|
// Restored by duplicated fd: CI has no controlling terminal, so reopening /dev/tty would lose it.
|
|
class StdoutSilencer
|
|
{
|
|
public:
|
|
StdoutSilencer()
|
|
{
|
|
fflush(stdout);
|
|
savedFd = dup(fileno(stdout));
|
|
const int devNull = open("/dev/null", O_WRONLY);
|
|
if (devNull >= 0) {
|
|
dup2(devNull, fileno(stdout));
|
|
close(devNull);
|
|
}
|
|
}
|
|
~StdoutSilencer()
|
|
{
|
|
fflush(stdout);
|
|
if (savedFd >= 0) {
|
|
dup2(savedFd, fileno(stdout));
|
|
close(savedFd);
|
|
}
|
|
}
|
|
|
|
private:
|
|
int savedFd = -1;
|
|
};
|
|
|
|
static std::string tempPath(int n)
|
|
{
|
|
return "/tmp/meshtastic-fuzz-config-" + std::to_string(n) + ".yaml";
|
|
}
|
|
|
|
/// Write `bytes` to a scratch file and hand it to the checker. Returns nothing: the assertion is
|
|
/// that we get here at all, without a crash, a hang, or an escaped exception.
|
|
static void runOn(const std::string &bytes, int slot)
|
|
{
|
|
const std::string path = tempPath(slot);
|
|
{
|
|
std::ofstream out(path, std::ios::binary);
|
|
out.write(bytes.data(), (std::streamsize)bytes.size());
|
|
}
|
|
try {
|
|
runConfigCheck({path});
|
|
} catch (const std::exception &) {
|
|
// An escaped exception is a defect in its own right: portduinoSetup() calls this on the way
|
|
// to exit() with nothing above it to catch.
|
|
TEST_FAIL_MESSAGE("runConfigCheck() let an exception escape");
|
|
}
|
|
std::error_code ec;
|
|
std::filesystem::remove(path, ec);
|
|
}
|
|
|
|
static std::vector<std::string> loadSeedCorpus()
|
|
{
|
|
std::vector<std::string> corpus;
|
|
std::error_code ec;
|
|
for (const auto &entry : std::filesystem::recursive_directory_iterator(kFixtureDir, ec)) {
|
|
if (!entry.is_regular_file() || entry.path().extension() != ".yaml")
|
|
continue;
|
|
std::ifstream in(entry.path(), std::ios::binary);
|
|
std::ostringstream buf;
|
|
buf << in.rdbuf();
|
|
corpus.push_back(buf.str());
|
|
}
|
|
return corpus;
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// C1 - the seed corpus itself
|
|
// ---------------------------------------------------------------------------
|
|
|
|
void test_seed_corpus_survives(void)
|
|
{
|
|
StdoutSilencer quiet;
|
|
const auto corpus = loadSeedCorpus();
|
|
// A corpus that failed to load would make every later group vacuous, so prove it is there.
|
|
TEST_ASSERT_TRUE_MESSAGE(corpus.size() >= 20, "fixture corpus not found - is the test running from the repo root?");
|
|
int slot = 0;
|
|
for (const auto &seed : corpus)
|
|
runOn(seed, slot++);
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// C2 - byte mutation of the corpus
|
|
// ---------------------------------------------------------------------------
|
|
|
|
void test_mutated_corpus(void)
|
|
{
|
|
StdoutSilencer quiet;
|
|
const auto corpus = loadSeedCorpus();
|
|
TEST_ASSERT_TRUE(corpus.size() >= 20);
|
|
rngSeed(BASE_SEED + 1);
|
|
|
|
for (int i = 0; i < 3000; i++) {
|
|
std::string s = corpus[rngRange((uint32_t)corpus.size())];
|
|
if (s.empty())
|
|
continue;
|
|
|
|
switch (rngRange(5)) {
|
|
case 0: // flip a byte
|
|
s[rngRange((uint32_t)s.size())] = (char)rngByte();
|
|
break;
|
|
case 1: // truncate
|
|
s.resize(rngRange((uint32_t)s.size()));
|
|
break;
|
|
case 2: // insert a run of one byte
|
|
s.insert(rngRange((uint32_t)s.size()), std::string(1 + rngRange(64), (char)rngByte()));
|
|
break;
|
|
case 3: // splice two fixtures together
|
|
s += corpus[rngRange((uint32_t)corpus.size())];
|
|
break;
|
|
default: // delete a span
|
|
if (s.size() > 1) {
|
|
const uint32_t at = rngRange((uint32_t)s.size() - 1);
|
|
s.erase(at, 1 + rngRange((uint32_t)(s.size() - at)));
|
|
}
|
|
break;
|
|
}
|
|
runOn(s, i % 8);
|
|
}
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// C3 - structural torture
|
|
// ---------------------------------------------------------------------------
|
|
// Shapes chosen to stress the event-stream walker rather than the parser.
|
|
|
|
void test_structural_torture(void)
|
|
{
|
|
StdoutSilencer quiet;
|
|
rngSeed(BASE_SEED + 2);
|
|
int slot = 0;
|
|
|
|
// Deep nesting, both flow and block style.
|
|
for (int depth : {1, 2, 8, 64, 512, 4096}) {
|
|
runOn("Lora:\n rfswitch_table: " + std::string(depth, '[') + std::string(depth, ']'), slot++);
|
|
std::string block = "Lora:\n";
|
|
for (int i = 0; i < depth; i++)
|
|
block += std::string(2 + i * 2, ' ') + "k:\n";
|
|
runOn(block, slot++);
|
|
}
|
|
|
|
// Duplicate keys at every depth, which is what DuplicateKeyFinder's stack is for.
|
|
for (int repeat : {2, 16, 256}) {
|
|
std::string dup = "Lora:\n";
|
|
for (int i = 0; i < repeat; i++)
|
|
dup += " CS: " + std::to_string(i) + "\n";
|
|
runOn(dup, slot++);
|
|
std::string nested = "Lora:\n rfswitch_table:\n";
|
|
for (int i = 0; i < repeat; i++)
|
|
nested += " MODE_RX: [HIGH]\n";
|
|
runOn(nested, slot++);
|
|
}
|
|
|
|
// Anchors and aliases: one node reachable from many paths, including a merge key.
|
|
runOn("Lora: &a\n Module: sx1262\nDisplay: *a\n", slot++);
|
|
runOn("a: &x [1, 2]\nLora:\n Enable_Pins: *x\n rfswitch_table: *x\n", slot++);
|
|
runOn("base: &b {CS: 1}\nLora:\n <<: *b\n IRQ: 2\n", slot++);
|
|
|
|
// Collections where a scalar is expected and vice versa, across every known section.
|
|
for (const char *section : {"Lora", "General", "Display", "Logging", "Webserver", "Meta", "GPIO"}) {
|
|
runOn(std::string(section) + ": [1, 2, 3]\n", slot++);
|
|
runOn(std::string(section) + ": ~\n", slot++);
|
|
runOn(std::string(section) + ":\n ? [complex, key]\n : value\n", slot++);
|
|
}
|
|
|
|
// Very long keys and scalars.
|
|
runOn("Lora:\n " + std::string(64 * 1024, 'k') + ": 1\n", slot++);
|
|
runOn("Lora:\n Module: " + std::string(256 * 1024, 'v') + "\n", slot++);
|
|
|
|
// Multiple documents in one file, which HandleNextDocument() loops over.
|
|
runOn("Lora:\n CS: 1\n---\nLora:\n CS: 2\n---\n[]\n", slot++);
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// C4 - random bytes (DISABLED)
|
|
// ---------------------------------------------------------------------------
|
|
// Off for CI cost: half the inputs and half the runtime for the least return, since uniform noise
|
|
// is almost always rejected on the first token. Flip to 1 to restore it.
|
|
#define FUZZ_CONFIG_RANDOM_BYTES 0
|
|
|
|
#if FUZZ_CONFIG_RANDOM_BYTES
|
|
void test_random_bytes(void)
|
|
{
|
|
StdoutSilencer quiet;
|
|
rngSeed(BASE_SEED + 3);
|
|
|
|
for (int i = 0; i < 1500; i++) {
|
|
std::string s;
|
|
s.resize(rngRange(2048));
|
|
for (auto &c : s)
|
|
c = (char)rngByte();
|
|
runOn(s, i % 8);
|
|
}
|
|
|
|
// Bytes drawn only from YAML's structural alphabet get much deeper into the parser than
|
|
// uniform random noise, which is usually rejected on the first token.
|
|
static const char kYamlChars[] = " \t\n-:[]{}#&*!|>'\"%@`,?abcLora0123";
|
|
for (int i = 0; i < 1500; i++) {
|
|
std::string s;
|
|
s.resize(rngRange(2048));
|
|
for (auto &c : s)
|
|
c = kYamlChars[rngRange(sizeof(kYamlChars) - 1)];
|
|
runOn(s, i % 8);
|
|
}
|
|
}
|
|
#endif // FUZZ_CONFIG_RANDOM_BYTES
|
|
|
|
void setUp(void) {}
|
|
void tearDown(void) {}
|
|
|
|
// The portduino framework supplies main() and calls setup()/loop(), so the suite's entry point is
|
|
// setup() rather than main() -- defining main() here collides with the framework's.
|
|
void setup()
|
|
{
|
|
initializeTestEnvironment();
|
|
UNITY_BEGIN();
|
|
|
|
printf("\n=== Group C1: seed corpus ===\n");
|
|
RUN_TEST(test_seed_corpus_survives);
|
|
|
|
printf("\n=== Group C2: mutated corpus ===\n");
|
|
RUN_TEST(test_mutated_corpus);
|
|
|
|
printf("\n=== Group C3: structural torture ===\n");
|
|
RUN_TEST(test_structural_torture);
|
|
|
|
#if FUZZ_CONFIG_RANDOM_BYTES
|
|
printf("\n=== Group C4: random bytes ===\n");
|
|
RUN_TEST(test_random_bytes);
|
|
#endif
|
|
|
|
exit(UNITY_END());
|
|
}
|
|
|
|
void loop() {}
|