diff --git a/.claude/skills/deploy/SKILL.md b/.claude/skills/deploy/SKILL.md index 8fcd752..e0cb7c5 100644 --- a/.claude/skills/deploy/SKILL.md +++ b/.claude/skills/deploy/SKILL.md @@ -105,6 +105,13 @@ ssh pbarrett@172.18.10.240 'bash -lc "sudo apt install -y pipx mosquitto-clients ssh pbarrett@172.18.10.240 'bash -lc "pipx install mpremote && pipx ensurepath"' ``` +The broker refuses anonymous clients (#7). A node's password lives only on the bench, at +`~/.config/layout-feedback/.json`, and the script copies it to the board on every +deploy. It **refuses to deploy** if that file is missing, so treat that as a provisioning gap +and not as a deploy bug. `mosquitto_sub` on the bench picks up the read-only `monitor` +identity from `~/.config/mosquitto_sub`. Creating either file is covered in +`docs/broker-auth.md` and needs the user's approval. + `python3-mpremote` is **not** in Mint 22.3's archive, and Ubuntu 24.04 bases are PEP 668 managed, so `pip install` into the system Python is refused — hence `pipx`. If a script fails because `mpremote` is missing, report that as the reason rather than as a deploy failure. diff --git a/CLAUDE.md b/CLAUDE.md index 9568372..b44b4b0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -22,6 +22,7 @@ Reasoning belongs in `docs/`. | `docs/pin-allocation.md` | Which sensor is on which expander pin, and why allocation is not installation | | `docs/block-detector-wiring.md` | The LM-iD output stage: its two modes, why Input A is unpowered, the 3.3 V hazard if it is not | | `docs/point-position-feedback.md` | Board 3 (`0x22`): why two inputs per point, the `0x22` allocation, and why command and feedback being different devices is the hazard. **Firmware built; no `S2` wiring landed** | +| `docs/broker-auth.md` | Broker identities, the ACL, where the node's password lives, and why a denied publish is silent | | `docs/startup-and-status-led.md` | The power-on race with the modem, the retry-then-reset policy, and **what the LED flash codes mean** | | `docs/DEVELOPMENT_NOTES.md` | Module conventions, the MCP23017 pin map, hardware wiring | | `docs/PN7150_STATE_MACHINE_PLAN.md` | The NCI state machine the RFID reader implements | @@ -125,6 +126,7 @@ src/lib/ -> device /lib sensor_wiring.py Allocation vs installed, and the occupancy polarity. No `machine`. point_wiring.py Point allocation, and the feedback input pair -> position. No `machine`. node_startup.py Retry-then-reset startup policy, and the LED code table. No `machine`. + broker_credentials.py Reads the board's /mqtt_credentials.json. No `machine`. status_led.py The onboard LED as a countable flash code. No `machine`. mcp23017_io.py I2C expander, in and out. Poll-only. input_pin_monitor.py Native Pico GPIO, IRQ + timer debounce. Currently unused. @@ -162,6 +164,13 @@ directory on the path for the same reason. ### Traps +- **The broker refuses anonymous clients, and its ACL drops a denied publish silently** + (#7). The node's password is not in git: it lives on the bench in + `~/.config/layout-feedback/.json`, and `deploy.sh` copies it to the board every + time. A new topic this node publishes needs `/etc/mosquitto/acl` changed as well, or it + gets acknowledged and goes nowhere. Nothing appears in the broker log either. See + `docs/broker-auth.md`. + - **A node that cannot start flashes a code and reboots — it no longer dies quietly.** Solid LED is running, `N` flashes is a failure at stage `N`, dark is no power or a hard crash. Codes and the retry policy are in `docs/startup-and-status-led.md`. Until 2026-08-27 diff --git a/docs/broker-auth.md b/docs/broker-auth.md new file mode 100644 index 0000000..003efc6 --- /dev/null +++ b/docs/broker-auth.md @@ -0,0 +1,142 @@ +# Broker authentication and ACL + +Issue #7. The bench broker used to take anonymous clients on the LAN, so any device on +that network could publish a fabricated `clear` on an occupied block, and the +orchestrator would believe it. Once contract control topics carry commands, the same gap +means throttle commands. Now every client has an identity, and each identity is limited +to the topics it owns. + +**The ACL matters more than the password.** A password alone only proves that a client is +*some* layout client. The ACL is what stops a compromised sensor node from publishing +`point/*/command` or `loco/*/command`. + +## Identities + +| User | Who | May | +|---|---|---| +| `orchestrator` | The backend, over `mqtt://localhost:1883` | read and write `layout/#` | +| `io-node` | This repo's IO node | **write** `sensor/+/reading` and `point/+/reading`; **read** `point/+/query` | +| `monitor` | People running `mosquitto_sub`, on the bench or from the LAN | read `layout/#` and `$SYS/#` | + +Anonymous clients are refused outright (`allow_anonymous false`). + +The `io-node` rules are pinned to this layout's id and scoped to topic *kinds*, not to +individual sensor ids. That choice is deliberate. Installing a sensor must never need a +broker change, because of the failure described next. + +## A denied publish is silent + +Under MQTT 3.1.1, mosquitto drops a publish that the ACL denies, and it still acknowledges +that publish to the client. `publish()` returns True and the node believes it is +reporting, but nobody hears it. That is exactly the failure rule 3 in `CLAUDE.md` exists +to prevent, and the node cannot detect it. + +**The broker log doesn't record it either.** Mosquitto 2.0.18 logs `Denied PUBLISH` +only at debug level, which the bench does not run. This was checked on 2026-09-23: three +denied publishes left nothing in the log. The one way to see a denial is to subscribe +and check whether the message arrives: + +``` +ssh pbarrett@172.18.10.240 'bash -lc "mosquitto_sub -h localhost -t \"layout/#\" -v -W 40"' +``` + +When a node is up with a solid LED but one of its topics never shows up there, suspect +the ACL before the node. A contract change that adds a topic this node publishes needs +this ACL updated in the same change. + +## Where the secrets live + +**Nothing secret is in git.** Everything below is on the bench box, `172.18.10.240`: + +| File | Holds | Mode | +|---|---|---| +| `/etc/mosquitto/passwd` | Hashed passwords for all three users | `0600 mosquitto` | +| `/etc/mosquitto/acl` | The ACL, as in the table above | `0600 mosquitto` | +| `/etc/mosquitto/conf.d/lan.conf` | The listener; turns authentication on | `0644 root` | +| `~/.config/layout-feedback/io-node.json` | The node's plaintext username and password | `0600 pbarrett` | +| `~/.config/mosquitto_sub` | `-u monitor -P …`, the default options for `mosquitto_sub` | `0600 pbarrett` | +| `/opt/layout-orchestrator/.env` | `MQTT_USERNAME` / `MQTT_PASSWORD` | `0600 pbarrett` | + +`mosquitto_sub` reads `~/.config/mosquitto_sub` automatically. That is why every +`mosquitto_sub -h localhost …` command in this repo and in the deploy skill keeps working +unchanged. `mosquitto_pub` has no default identity. Hand-publishing, for example +clearing a retained reading, now takes explicit credentials. + +The broker config: + +``` +# /etc/mosquitto/conf.d/lan.conf +listener 1883 +allow_anonymous false +password_file /etc/mosquitto/passwd +acl_file /etc/mosquitto/acl +``` + +The notes from #7 about the listener still hold. Leave the bind address off, and +remember that the orchestrator reaches the broker over loopback through this same +listener. + +## How the Pico gets its password + +1. `scripts/deploy.sh` checks that `~/.config/layout-feedback/.json` exists on the + bench **before touching the board**, and refuses to deploy if it is missing. +2. It copies that file directly from the bench's own copy to the board as + `/mqtt_credentials.json`, so the password never reaches the dev machine or git. + Every deploy copies it again, so rotating the password is: change it on the bench, + then redeploy. +3. At startup `broker_credentials.load_credentials()` reads the file and validates it, + and `MQTTATClient` sends the credentials in `AT+MQTTUSERCFG`. + +Details that matter: + +- **Not a `.py` file.** The deploy's root sweep deletes every `.py` at the root except + `main.py` and `config.py`. A `.json` survives the sweep, and nothing can import it or + shadow it. +- **No anonymous fallback.** A missing or malformed file raises `CredentialsError`, and + the node flashes **code 8**, not code 5. That sends you to the deploy rather than to + the broker. A password the broker *rejects* still flashes code 5, because the modem + reports a refused connect the same way as an unreachable one. Check the broker log + for `not authorised`. +- **Only characters AT can carry.** Values are limited to 64 printable ASCII characters, + with no `"`, `\` or `,`. The passwords are 32 random alphanumerics, so they fit. +- **Redacted on the console.** The modem echoes `AT+MQTTUSERCFG` back to the Pico, so + `MQTTATClient` strips the password from its debug log and failure dumps. +- The password sits in plain text on the Pico's flash. Anyone with the board and a USB + lead can read it. The ACL is what limits the damage: the most that stolen password + can do is publish readings. + +## Rotating a password, or adding a node + +On the bench: + +```bash +sudo mosquitto_passwd /etc/mosquitto/passwd io-node # prompts; never pass -b +# edit ~/.config/layout-feedback/io-node.json to match +sudo systemctl reload mosquitto # SIGHUP rereads passwd and acl +``` + +Then run `bash scripts/deploy.sh io-node`. A new node gets its own user, its own ACL +block and its own `.json`. **Don't share `io-node` between boards.** A shared +identity can't be revoked without taking both boards off the air. + +## Verifying + +All of these were run at cutover on 2026-09-23. + +- **Every client connects with its own user.** The broker log shows `u'orchestrator'` + and `u'io-node'` on each connect. The node reconnected by itself after a broker + restart, so its credentials survive a reconnect. +- **Anonymous and wrong-password clients are refused from the LAN**, with + `Connection refused: Not authorized`. This was run from the dev machine. +- **`monitor` can read over the LAN address.** Run + `mosquitto_sub -h 172.18.10.240 -t 'layout/+/sensor/#'` on the bench, where it picks up + `~/.config/mosquitto_sub`. +- **The ACL denies what it should.** Subscribe as `monitor` to an all-zero layoutId's + namespace, so nothing real can act on the probes. Then publish: + - `io-node` → `point/acl-probe/command` + - `io-node` → `sensor/acl-probe/reading` under that id, which tests the pin to the real + layoutId + - `monitor` → anything + + All three are dropped. An `orchestrator` publish to the same namespace is the + positive control, and it arrives. Without that control an empty result proves nothing. diff --git a/docs/startup-and-status-led.md b/docs/startup-and-status-led.md index 76125de..cf93135 100644 --- a/docs/startup-and-status-led.md +++ b/docs/startup-and-status-led.md @@ -83,9 +83,10 @@ flashing is easier to find than one that is simply dark. | **2** | Sensors | A configured expander is not on the I2C bus | Ribbon to boards 1/2, `config.py` addresses | | **3** | Modem | The ESP-AT modem is not answering AT commands | UART wiring GP8/GP9, modem power | | **4** | Network | The modem answers, but never reported an IP | Ethernet cable, DHCP, switch port | -| **5** | Broker | Network is up, the MQTT link could not be made | Mosquitto on the bench box, `MQTT_HOST` | +| **5** | Broker | Network is up, the MQTT link could not be made — including credentials the broker rejected | Mosquitto on the bench box, `MQTT_HOST`; `not authorised` in its log (`docs/broker-auth.md`) | | **6** | Runtime | The node came up, then the main loop crashed | USB console — this one is a bug, not wiring | | **7** | Unknown | Something else | USB console | +| **8** | Credentials | No usable `/mqtt_credentials.json` on the board | Redeploy with `scripts/deploy.sh`; `docs/broker-auth.md` | Codes start at 2 on purpose. A single flash is too easily confused with a board merely blinking on boot, and a miscounted code is worse than no code. diff --git a/scripts/deploy.sh b/scripts/deploy.sh index 5abc0c3..ffe49cd 100644 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -16,6 +16,12 @@ STAGING="/tmp/layout-feedback-deploy" # them is PicoDCC's debug probe. BOARD_ID="${LAYOUT_FEEDBACK_BOARD_ID:-0d9a62134acbf42d}" +# The node's broker username and password (#7). The only copy lives on the bench box, so +# it never passes through git or this machine; every deploy copies it onto the board. +# Tilde expands on the bench, not here. See docs/broker-auth.md. +CREDENTIALS_DIR='~/.config/layout-feedback' +BOARD_CREDENTIALS=":/mqtt_credentials.json" + NODE="${1:-}" DRY_RUN=0 [ "${2:-}" = "--dry-run" ] && DRY_RUN=1 @@ -53,9 +59,19 @@ if [ "$DRY_RUN" = 1 ]; then echo " $APP_DIR/ -> $STAGING/$NODE/app/" echo "(dry run) would then copy to the board and soft-reset:" echo " /lib/*.py from lib/, /main.py and /config.py from app/" + echo " $BOARD_CREDENTIALS from $BENCH:$CREDENTIALS_DIR/$NODE.json" exit 0 fi +# Before anything touches the board: without its credentials the node flashes code 8 +# forever, and a deploy that leaves it that way should not report success. +echo "== credentials ==" +if ! ssh "$BENCH" "bash -lc 'test -s $CREDENTIALS_DIR/$NODE.json'"; then + echo "no broker credentials for $NODE at $BENCH:$CREDENTIALS_DIR/$NODE.json" >&2 + echo "see docs/broker-auth.md — refusing to deploy" >&2 + exit 1 +fi + # scp rather than rsync: Git Bash on Windows ships no rsync, and the staging directory # is wiped first, so there is nothing for --delete to do that rm has not already done. ssh "$BENCH" "bash -lc 'rm -rf $STAGING/$NODE && mkdir -p $STAGING/$NODE/lib $STAGING/$NODE/app'" || exit 1 @@ -96,6 +112,12 @@ for f in app/*.py; do \$MP fs cp "\$f" ":/\$(basename \$f)" || exit 1 done +echo "-- credentials --" +# From the bench's own copy, never the staging directory: the secret is not staged. +# Not a .py, so the root sweep above leaves it alone and nothing can import it. +echo " $CREDENTIALS_DIR/$NODE.json -> $BOARD_CREDENTIALS" +\$MP fs cp $CREDENTIALS_DIR/$NODE.json "$BOARD_CREDENTIALS" >/dev/null || exit 1 + echo "-- reset --" \$MP reset REMOTE diff --git a/src/apps/io-node/config.py b/src/apps/io-node/config.py index fcd7d6c..d14d6e6 100644 --- a/src/apps/io-node/config.py +++ b/src/apps/io-node/config.py @@ -24,6 +24,9 @@ MQTT_CLIENT_ID = "layout-feedback-io-node" MQTT_HOST = "172.18.10.240" +# The broker refuses anonymous clients. Username and password are not in git: deploy.sh +# copies them onto the board from the bench box. See docs/broker-auth.md. +MQTT_CREDENTIALS_FILE = "/mqtt_credentials.json" MQTT_PORT = 1883 MQTT_UART_ID = 1 MQTT_TX_PIN = 8 diff --git a/src/apps/io-node/main.py b/src/apps/io-node/main.py index 363351a..cb35770 100644 --- a/src/apps/io-node/main.py +++ b/src/apps/io-node/main.py @@ -11,6 +11,7 @@ from machine import I2C, Pin, reset from utime import sleep_ms, ticks_ms +from broker_credentials import load_credentials from layout_mqtt import LayoutMQTT from mcp23017_io import MCP23017Expander from mqtt_at import BrokerUnreachable, MQTTATClient @@ -157,11 +158,16 @@ def bring_up(): sensors = build_sensors(i2c, available, expanders, wiring) points = build_points(i2c, available, expanders, point_wiring) + # Before the modem, so a board deployed without its credentials flashes that, not a + # broker fault. + username, password = load_credentials(config.MQTT_CREDENTIALS_FILE) + client = MQTTATClient( host=config.MQTT_HOST, port=config.MQTT_PORT, uart_id=config.MQTT_UART_ID, tx_pin=config.MQTT_TX_PIN, rx_pin=config.MQTT_RX_PIN, baud=config.MQTT_BAUD, client_id=config.MQTT_CLIENT_ID, + username=username, password=password, ) try: client.setup() diff --git a/src/lib/broker_credentials.py b/src/lib/broker_credentials.py new file mode 100644 index 0000000..8014b09 --- /dev/null +++ b/src/lib/broker_credentials.py @@ -0,0 +1,74 @@ +"""The node's broker identity, read from a file that `deploy.sh` puts on the board. + +The broker refuses anonymous clients (#7), so a node needs a username and password. The +password is not in git: its only copy lives on the bench box, in +`~/.config/layout-feedback/.json`, and every deploy copies it onto the board as +`/mqtt_credentials.json`. See `docs/broker-auth.md`. + +A missing or malformed file fails startup with its own LED code. It never falls back to +connecting anonymously — that would be refused anyway, and would flash as a broker fault, +sending whoever is looking to the bench box instead of to the deploy. + +Pure — no `machine` import — so every check here runs in the host suite. +""" + +import json + + +# ESP-AT caps `AT+MQTTUSERCFG`'s username and password at 64 bytes each. Checked here so +# an over-long value fails as a credentials fault, not as a broker that refuses us. +MAX_FIELD_LENGTH = 64 + +# `"`, `\` and `,` all have meaning inside an AT command's quoted argument. Generated +# passwords never need them, so refuse them rather than trust an escaping rule for a +# field whose only failure symptom is "connection refused". +_FORBIDDEN = '"\\,' + + +class CredentialsError(RuntimeError): + """The board has no usable broker credentials. Redeploy the node to fix it.""" + + +def parse_credentials(text): + """Return `(username, password)` from the file's JSON text, or raise.""" + try: + data = json.loads(text) + except ValueError as exc: + raise CredentialsError("credentials file is not valid JSON (%s)" % exc) + + if not isinstance(data, dict): + raise CredentialsError("credentials file must hold a JSON object") + + fields = [] + for key in ("username", "password"): + value = data.get(key) + if not isinstance(value, str) or not value: + raise CredentialsError("credentials file has no %s" % key) + if len(value) > MAX_FIELD_LENGTH: + raise CredentialsError( + "%s is %d characters; the modem accepts at most %d" + % (key, len(value), MAX_FIELD_LENGTH)) + for ch in value: + if ch in _FORBIDDEN or not (" " < ch <= "~"): + # Never echo the value itself: this goes to the console. + raise CredentialsError( + "%s contains a character the modem's AT syntax cannot carry" % key) + fields.append(value) + + return fields[0], fields[1] + + +def load_credentials(path, open_fn=open): + """Read and validate the credentials file at `path`.""" + try: + with open_fn(path) as f: + text = f.read() + except OSError: + raise CredentialsError( + "no credentials at %s — deploy the node with scripts/deploy.sh" % path) + return parse_credentials(text) + + +__all__ = [ + "CredentialsError", "MAX_FIELD_LENGTH", "load_credentials", "parse_credentials", +] diff --git a/src/lib/mqtt_at.py b/src/lib/mqtt_at.py index 276fcc9..f5ed28d 100644 --- a/src/lib/mqtt_at.py +++ b/src/lib/mqtt_at.py @@ -178,7 +178,7 @@ def _take_frame(buf): class MQTTATClient: - def __init__(self, host: str, port: int = 1883, uart=None, uart_id=1, tx_pin=8, rx_pin=9, baud=115200, keepalive: int = 1, timeout_ms=2000, debug: bool = False, client_id: str = 'sensors'): + def __init__(self, host: str, port: int = 1883, uart=None, uart_id=1, tx_pin=8, rx_pin=9, baud=115200, keepalive: int = 1, timeout_ms=2000, debug: bool = False, client_id: str = 'sensors', username: str = '', password: str = ''): """Create client bound to a single broker address. - `host` and `port` are used to connect on `setup()` and kept for diagnostics. @@ -190,6 +190,10 @@ def __init__(self, host: str, port: int = 1883, uart=None, uart_id=1, tx_pin=8, # Distinct per node: two clients sharing an MQTT client id knock each other off # the broker in a reconnect loop, which looks exactly like flaky hardware. self.client_id = client_id + # Empty means anonymous. The production broker refuses that (#7); the node's + # identity comes from `broker_credentials.py`. + self.username = username + self.password = password self.uart_id = uart_id self.tx_pin = tx_pin self.rx_pin = rx_pin @@ -290,7 +294,10 @@ def setup(self): self._dump('post-reset') raise NetworkNotReady('Network device did not report IP after reset') - config_cmd = 'AT+MQTTUSERCFG=0,1,"%s","","",0,0,""' % self.client_id + config_cmd = 'AT+MQTTUSERCFG=0,1,"%s","%s","%s",0,0,""' % ( + self._escape_at_string(self.client_id), + self._escape_at_string(self.username), + self._escape_at_string(self.password)) if not self.send_at_and_wait(config_cmd, timeout_ms=50000): self._dump('base config') raise BrokerUnreachable('set_base_config returned ERROR or timed out') @@ -327,10 +334,17 @@ def _write_line(self, line: str): # best-effort: ignore write errors so callers can continue pass + def _redact(self, text): + # The modem echoes `AT+MQTTUSERCFG` back, so the password would otherwise reach + # the console through both the debug log and a failure dump. + if self.password: + return text.replace(self._escape_at_string(self.password), '') + return text + def _log(self, msg: str): try: if self.debug: - print('[MQTTAT]', msg) + print('[MQTTAT]', self._redact(msg)) except Exception: pass @@ -547,7 +561,7 @@ def poll(self): def _record_line(self, text): try: - self._recent_lines.append(text) + self._recent_lines.append(self._redact(text)) if len(self._recent_lines) > self._max_recent_lines: self._recent_lines.pop(0) except Exception: diff --git a/src/lib/node_startup.py b/src/lib/node_startup.py index 805749c..f46191f 100644 --- a/src/lib/node_startup.py +++ b/src/lib/node_startup.py @@ -20,6 +20,7 @@ | 5 | Broker | Network is up, but the MQTT link could not be made | | 6 | Runtime | The node came up, then the main loop crashed | | 7 | Unknown | Something else — read the USB console | +| 8 | Credentials | No usable broker credentials on the board — redeploy | Codes start at 2 deliberately. A single flash is too easily confused with a board that is merely blinking on boot, and a code that gets miscounted is worse than no code. @@ -29,6 +30,7 @@ want to discover by watching a baseboard. """ +from broker_credentials import CredentialsError from mqtt_at import BrokerUnreachable, ModemNotResponding, NetworkNotReady @@ -38,6 +40,8 @@ CODE_BROKER = 5 CODE_RUNTIME = 6 CODE_UNKNOWN = 7 +# After UNKNOWN rather than renumbering it: 2-7 are already being counted on the bench. +CODE_CREDENTIALS = 8 # Three attempts, flashing for 2 s, 5 s and 10 s between them, then a hard reset — about # 17 s of trying before the board goes round again. @@ -75,6 +79,8 @@ def code_for_error(exc): return CODE_NETWORK if isinstance(exc, BrokerUnreachable): return CODE_BROKER + if isinstance(exc, CredentialsError): + return CODE_CREDENTIALS return CODE_UNKNOWN @@ -102,6 +108,6 @@ def start_supervised(attempt, on_failure, reset, backoff_ms=RETRY_BACKOFF_MS): __all__ = [ "CODE_SENSORS", "CODE_MODEM", "CODE_NETWORK", "CODE_BROKER", "CODE_RUNTIME", - "CODE_UNKNOWN", "RETRY_BACKOFF_MS", "SensorWiringError", "StartupFailed", + "CODE_UNKNOWN", "CODE_CREDENTIALS", "RETRY_BACKOFF_MS", "SensorWiringError", "StartupFailed", "code_for_error", "start_supervised", ] diff --git a/tests/test_broker_credentials.py b/tests/test_broker_credentials.py new file mode 100644 index 0000000..3a676a2 --- /dev/null +++ b/tests/test_broker_credentials.py @@ -0,0 +1,76 @@ +"""The node's broker credentials, read from the file deploy.sh puts on the board (#7). + +The failure these guard against is a node that cannot authenticate and says so as the +wrong thing. A missing file must flash as credentials, not as a broker fault, and a +value the modem cannot carry must be refused here rather than come back as "refused". +""" + +import json + +import pytest + +from broker_credentials import ( + CredentialsError, MAX_FIELD_LENGTH, load_credentials, parse_credentials, +) +from node_startup import CODE_BROKER, CODE_CREDENTIALS, code_for_error + + +def creds(**fields): + return json.dumps(fields) + + +def test_a_well_formed_file_gives_the_username_and_password(): + assert parse_credentials(creds(username="io-node", password="Abc123xyz")) == \ + ("io-node", "Abc123xyz") + + +def test_it_is_read_from_the_path_given(tmp_path): + path = tmp_path / "mqtt_credentials.json" + path.write_text(creds(username="io-node", password="Abc123xyz")) + + assert load_credentials(str(path)) == ("io-node", "Abc123xyz") + + +def test_a_missing_file_says_to_redeploy(tmp_path): + with pytest.raises(CredentialsError, match="deploy"): + load_credentials(str(tmp_path / "absent.json")) + + +@pytest.mark.parametrize("text", [ + "not json at all", + "[]", + creds(username="io-node"), + creds(password="Abc123xyz"), + creds(username="", password="Abc123xyz"), + creds(username="io-node", password=""), + creds(username="io-node", password=12345), +]) +def test_anything_incomplete_is_refused(text): + with pytest.raises(CredentialsError): + parse_credentials(text) + + +@pytest.mark.parametrize("password", ['ab"cd', "ab\\cd", "ab,cd", "ab cd", "ab\ncd", "abcé"]) +def test_characters_the_at_syntax_cannot_carry_are_refused(password): + with pytest.raises(CredentialsError): + parse_credentials(creds(username="io-node", password=password)) + + +def test_the_modems_length_limit_is_enforced_here(): + ok = "a" * MAX_FIELD_LENGTH + assert parse_credentials(creds(username="io-node", password=ok))[1] == ok + + with pytest.raises(CredentialsError, match="at most"): + parse_credentials(creds(username="io-node", password=ok + "a")) + + +def test_the_error_never_quotes_the_password(): + with pytest.raises(CredentialsError) as info: + parse_credentials(creds(username="io-node", password='Hunter2"')) + assert "Hunter2" not in str(info.value) + + +def test_a_credentials_fault_flashes_its_own_code_not_the_brokers(): + code = code_for_error(CredentialsError("no credentials")) + assert code == CODE_CREDENTIALS + assert code != CODE_BROKER diff --git a/tests/test_mqtt_setup.py b/tests/test_mqtt_setup.py index c3fd474..cf78e49 100644 --- a/tests/test_mqtt_setup.py +++ b/tests/test_mqtt_setup.py @@ -154,3 +154,52 @@ def test_the_startup_failures_are_still_runtime_errors(uart): """They subclass RuntimeError, which is what callers already catch.""" for failure in (ModemNotResponding, NetworkNotReady, BrokerUnreachable): assert issubclass(failure, RuntimeError) + + +# --- the broker identity (#7) ----------------------------------------------- + +def usercfg(uart): + return [line for line in uart.sent_lines if line.startswith("AT+MQTTUSERCFG=")] + + +def test_the_username_and_password_reach_the_modem(uart): + client = make_client(uart, client_id="layout-feedback-io-node", + username="io-node", password="s3cretPassw0rd") + uart.auto_reply = booting_modem() + + client.setup() + + assert usercfg(uart) == [ + 'AT+MQTTUSERCFG=0,1,"layout-feedback-io-node","io-node","s3cretPassw0rd",0,0,""'] + + +def test_without_credentials_the_client_connects_anonymously(uart): + """The library stays usable against a throwaway broker; the node is what insists.""" + client = make_client(uart, client_id="spike") + uart.auto_reply = booting_modem() + + client.setup() + + assert usercfg(uart) == ['AT+MQTTUSERCFG=0,1,"spike","","",0,0,""'] + + +def test_the_password_never_reaches_the_console(uart, capsys): + """The modem echoes USERCFG, and a failed connect dumps what the modem said.""" + client = make_client(uart, username="io-node", password="s3cretPassw0rd", debug=True) + + modem = booting_modem(broker="refused") + + def reply(line): + if line.startswith("AT+MQTTUSERCFG="): + uart.feed_line(line) # the echo + return modem(line) + + uart.auto_reply = reply + + with pytest.raises(BrokerUnreachable): + client.setup() + + out = capsys.readouterr().out + assert "AT+MQTTUSERCFG" in out, "the test must actually have logged the command" + assert "s3cretPassw0rd" not in out + assert all("s3cretPassw0rd" not in line for line in client._recent_lines) diff --git a/tests/test_node_startup.py b/tests/test_node_startup.py index 5cd2f0e..5a21b2b 100644 --- a/tests/test_node_startup.py +++ b/tests/test_node_startup.py @@ -10,7 +10,7 @@ from mqtt_at import BrokerUnreachable, ModemNotResponding, NetworkNotReady from node_startup import ( - CODE_BROKER, CODE_MODEM, CODE_NETWORK, CODE_RUNTIME, CODE_SENSORS, CODE_UNKNOWN, + CODE_BROKER, CODE_CREDENTIALS, CODE_MODEM, CODE_NETWORK, CODE_RUNTIME, CODE_SENSORS, CODE_UNKNOWN, RETRY_BACKOFF_MS, SensorWiringError, StartupFailed, code_for_error, start_supervised, ) @@ -58,7 +58,7 @@ def test_an_unrecognised_failure_still_gets_a_code(): def test_the_codes_are_all_distinct_and_countable(): codes = [CODE_SENSORS, CODE_MODEM, CODE_NETWORK, CODE_BROKER, CODE_RUNTIME, - CODE_UNKNOWN] + CODE_UNKNOWN, CODE_CREDENTIALS] assert len(set(codes)) == len(codes) assert min(codes) >= 2, "a single flash is too easily confused with a boot blink"