/api/stats embeds config.mqtt_brokers as stored, so every broker
password went out on an endpoint every page polls, into browser memory,
devtools captures and anything holding a token. The default config
export, which redacts the login passwords, the JWT secret, the modem
token and the identity keys, carried them as well.
Send each broker password as the existing "*** REDACTED ***" sentinel
in both. The settings UIs post the brokers back as they received them,
so update_mqtt_config turns the sentinel into the stored password of
the broker with that name, and asks for the password again when no
single stored broker has the name; config_import keeps the stored
password in place of a redacted one, as it already does for the
security secrets and identity keys.
A bare number 0-255 named the first hash byte before multi-byte modes and
still does. acl_clients compared its identity_hash filter against the first
key byte only; it now matches the same prefix as remove.
acl_clients lists identity_hash at the path_hash_mode width (2 or 3 bytes),
but _acl_targets compared the whole value against the key's first byte, so a
remove from the Sessions page selected no ACL in 2- and 3-byte modes and the
grant stayed. Treat the hash as a public-key prefix; a bare number is read at
the configured width.
mesh_cli is imported on every boot, so it reads openhop_core.__version__
with getattr rather than importing it, and /api/stats does the same instead
of failing the whole response. Registering the repeater version with the
companion CLI cannot stop a companion loading if the core's signature
changes.
Names are unique across the repeater, rooms and companions at boot, so
create_identity and update_identity now refuse a name another identity
uses (including "repeater"), and creating a room with another identity's
key is refused as updating one already was.
A refused room update restores the live entry in place rather than
emptying and refilling it, since the identity manager holds that dict and a
concurrent config save could catch it with no name or key. A bad settings
value is refused too, undoing a rename made earlier in the same request.
An ACL request's identity_hash may be a JSON number, and 0 means hash byte
0 rather than every ACL.
Stored entries now record when they last logged in, so the web UI can
show whether a provisioned grant is still in use after a restart.
Firmware keeps this in RAM only; last_activity still starts at 0 on
load, so "not active since restart" keeps its meaning.
- acl_entries gains a nullable last_login column, added beside the
table's CREATE so a failed earlier migration cannot lock stored
admins out of a load that reads it.
- A login by a stored entry writes its time, best-effort, and not when
the stored second already matches it.
- A guest promoted by setperm keeps its login time; a row taken over
from another identity's label does not inherit that identity's.
- Key changes, renames and label adoption carry the time; a retried
load keeps the later of the live and stored times and writes a login
made while the store was unreadable.
/api/acl_clients adds client_name and client_type for each entry: the
name the node advertises, from an advert the repeater heard, or else a
companion's contact for it (a companion's own admin is often never heard
directly). Rebuilds the bundled UI (openHop_RepeaterUI feat/acl-management),
which shows the name in place of the one-byte address.
Codex confirmation round: the sweep of rows already under a name was
best-effort, so when it failed the rename or creation went ahead and the
leftovers became the room's grants. The sweep is now required: a rename
fails before the config is saved, and create_identity refuses before
adding the room.
Codex confirmation round: after a delete whose ACL cleanup failed, a room
created on that key and renamed to the deleted room's name took over its
leftover rows. Names are unique, so no live room holds the label a
rename takes or a new room gets; a rename (before taking the label) and
create_identity now drop every row already stored under it.
Codex round 5 (after the usage limit reset):
- A rekeyed or deleted room's old key kept answering logins, messages and
requests until a restart: its login, text and protocol handlers stayed
registered. The daemon now unregisters an identity from every packet
helper (_unregister_identity_everywhere), and update_identity and
delete_identity call it for the retired identity. Each helper removes
only its own handler, never another identity's on the same hash byte.
- Rows left at a key by a failed cleanup could grant their admins to a
later room on that key. A room loads only rows under its own label and
logs any others; moves copy, relabel and delete only the room's own
label's rows. A same-key rename relabels before the config is saved
and puts the label back if the save fails, so a room's own rows follow
its name. delete_identity reports an ACL cleanup that failed.
- Two overlapping hot reloads could start a superseded replacement
RoomServer whose sync loop then ran untracked. A swap starts its room
only if it is still the registered one.
A hot reload from PUT /api/update_identity re-runs TextHelper.register_identity,
which replaced self.room_servers[hash] and started a new sync loop without
stopping the old one, so the room pushed posts twice (the old loop with its
stale ACL). Take the RoomServer under the same hash, and any registered under
another hash by the same or previous name (a key change), out of room_servers,
and stop them before starting the replacement, on the running loop or via
run_coroutine_threadsafe from a request thread. update_identity passes the
original name through _register_identity_everywhere as previous_name.
Sixth review round (opencode: design still clean):
- A room whose passwords were cleared, then set again in a settings-only
update, was told no reload was needed, yet had no login handler or ACL
until a restart. Turning a password on counts as a reload.
- A restore refused because the new entry already holds the hash is logged.
- The no-login-helper move path says why it keeps its own store guard.
Fifth review round (opencode: "the ACL design is clean"; residual nits):
- The old identity entry is put back whenever the new registration did not
take its place, including when registration raises.
- A room whose update leaves it with no password is not hot-reloaded: it
is not registered for logins, so its ACL would stay under the old name.
The update waits for a restart and says so.
- delete_identity releases the room's name and hash through
unregister_identity, so the room can be re-created live rather than
conflicting with a room that no longer exists until a restart.
- LoginHelper.move_room_acl checks for a store before any path, the
superseded-ACL loop again skips the ACL being registered, and the
test daemon accepts the previous_name argument of the RoomServer
teardown fix on feat/quirky-lamarr-dfeae2, which should land with this
branch: the hot reload that now applies is what reaches that bug.
Fourth review round (opencode; codex hit its usage limit):
- update_identity's hot reload was always refused: the identity manager
rejects a name or hash already registered, which the running room's own
entry always is. The ACL name index then lagged the config, so a renamed
room was unmanageable by name, and a later rekey left a second live,
writable ACL. The old entry is now unregistered first and restored if
the new registration is refused (IdentityManager.unregister_identity),
and a move falls back to the store key when the name misses.
- A key change whose save and cleanup both failed left a copy at the new
key that blocked every retry. A copy under the room's own label is
replaced on retry; another identity's entries still refuse the move.
- A registration now detaches the ACL it replaces before loading, warns
when it displaces another identity sharing its hash byte, and the no-store
guard for a key change lives in LoginHelper.move_room_acl.
- /acl_stats is documented as it has always answered; store_error and
store_errors say they clear once a later read succeeds; the example
config says room servers keep their own security settings.
Third review round (codex, opencode):
- A failed save whose rollback also failed left the ACL under a key the
config did not name. A key change now copies the entries, saves the
config, then drops the old ones, all under the ACL's lock: any failure
leaves the key the saved config names holding its entries.
- Moves and deletes found the live ACL by scanning for its store key, and a
rekey left two ACL objects for one identity, so a second move or a delete
could act on the stale one. LoginHelper finds ACLs by name, and a new ACL
detaches and drops the one it replaces.
- A room deleted and re-added with the same key reused the detached ACL and
never stored again. A detached ACL is never reused.
- A failed read at startup was never retried; the next login or change
retries it, and acl_clients reports it as store_errors.
- A key change with no store anywhere was treated as moved. It is refused.
- The owner lookup fell back to the 1-byte hash for a room with no ACL,
attributing another identity's entries to it. acl_stats used the hash too.
- A refused update_identity left its merged settings in the live config;
every refusal now restores the entry. A key that yields no public key is
refused rather than skipping the shared-key check.
- Removing from every ACL stopped at the first failed write; it now reports
which failed and which succeeded. Room eviction carries on past one.
- Settings on a reused ACL are applied under its lock.
Second review round (codex, opencode):
- Moving a room's rows and pointing its live ACL at them happened in two
steps, so a grant could land under the old key in between.
ACL.move_store now does both under the ACL's lock.
- update_identity accepted a key another identity already uses, and the
move merged the two ACLs. It now refuses such a key before changing
anything, and the store refuses to move onto a key that has entries.
- update_identity saved the new key before moving the ACL, so a failed move
left a saved key with an empty ACL. The move now runs first, and a failed
save moves it back. It also runs when the daemon has no login helper.
- A failed read at startup looked like an empty ACL. The ACL records the
error and /api/acl_info reports it as store_error.
- A deleted room's handlers stay live until a restart, and a login there
wrote its entries back. Deleting now detaches the live ACL from storage.
- /api/acl_info looked ACLs up by the 1-byte hash, so identities sharing one
could show each other's counts. It uses the name lookup acl_clients does.
- The acl_clients filter is an object in the spec, as returned.
Review (codex, opencode) found:
- A room renamed and rekeyed in one update lost its ACL, and a room later
created under the old name could adopt it. Room servers are no longer
adopted by label; update_identity moves their rows to the new key and
name in one transaction and points the live ACL at them. Only the
repeater, which is unique, still adopts by label after a key change.
- A removal racing a grant could leave the grant stored but not listed.
Each ACL change and its write now happen under one lock.
- A failed write was reported as OK. The store now raises; setperm answers
"Err - failed to save" and leaves the table as it was, and the API
returns an error. A login still succeeds, as its session is valid.
- Re-registering an identity (a rename) built a new ACL, zeroing every
session's activity and replay watermark, so the room sync loop stopped
pushing to logged-in clients. The live ACL is now reused.
- The repeater's max_clients default rises to firmware's 32: admins are
never evicted and now persist, so 5 or 10 filled permanently. Stored
entries past max_clients all load, with a warning.
- atoi takes ASCII digits only; non-repeater identity types get their own
label; the sync-loop skip logs at debug; `get acl` has help.
API for the web UI:
- POST /api/acl_set_permissions sets an entry by identity name.
- /api/acl_clients lists entries by identity name rather than the 1-byte
hash two identities can share, with permissions_value, persisted and
identity_pubkey. /api/acl_info adds acl_entries and stored_entries, and
authenticated_clients counts entries that have been active.
- /api/acl_remove_client reads client_pubkey, as its schema always said
(the UI sent it, and the endpoint read only public_key), and takes
identity_name.
Firmware keeps its ACL in /s_contacts, so a key provisioned over serial with
`setperm <pubkey> 3` logs in with a blank password forever after. Ours was
RAM-only and setperm was a stub, so that workflow could not work.
- New acl_entries table, keyed by the local identity's full 32-byte public
key rather than its 1-byte hash, which collides between identities. An
identity_label ("repeater", "room_server:<name>") lets an identity adopt
its rows after a key change, as firmware's ACL survives a new private key.
Shared secrets are derived on load, never stored.
- The repeater stores every entry with permissions, a room server admins
only (firmware saveFilter). A write happens only when an entry's stored
permissions change, so a returning admin costs no I/O.
- setperm follows ClientACL::applyPermissions and firmware's replies: atoi
permissions stored as the whole byte, a guest role deletes by key prefix,
other roles need the full key. It refuses an empty prefix and a key that
is not a valid ed25519 point, both of which firmware accepts.
- `get acl` works from the web CLI (the serial stand-in, passed local=True)
and still errors remotely. It was unreachable: "get " matched it first.
- A full table evicts the least active non-admin, as firmware putClient,
instead of refusing; it still refuses when every entry is an admin.
- Room servers keep a stored admin through inactivity eviction and do not
push to entries that have not logged in since load, as firmware skips
clients with no activity.
- Deleting a room server drops its stored entries.
The event stream always served the first companion bridge. Every REST
endpoint that acts on one companion takes companion_name or
companion_hash, but events ignored its query string: a client asking for
another companion was served the first one's events, with nothing in
them to say whose they were, and a name that did not exist opened a
stream all the same.
Resolve the stream's companion the way the REST endpoints do, subscribe
that bridge, and route each event only to the clients of its companion.
Every event, the opening "connected" frame included, carries the
companion_hash it came from, so a client can confirm what it hears.
With no parameter the stream still follows the first companion. An
unknown companion now answers 404, and a repeater with no companion
loaded 503, as the REST endpoints do; the latter used to open a stream
that could stay silent.
_broadcast_sse drops a client whose queue is full, but the client's own
generator kept waiting on that queue and sending keepalives. The
connection looked healthy while no event could reach it again, and the
client had no reason to reconnect.
When the queue runs dry, end the stream if the client is no longer
registered. Everything already queued is still delivered first; the
client then sees the stream close and reconnects, which re-subscribes it.
CherryPy mounts StatsApp at / but sensor endpoints are on APIEndpoints.
Add @cherrypy.expose delegation methods so /sensors_types, /sensors_config,
/sensors_config_update, and /sensors_read resolve correctly instead of
falling through to the SPA catch-all.
The default method was raising 404 for all /api/* requests, blocking
sensor endpoints and other API routes from reaching the mounted
APIEndpoints object. CherryPy handles /api/* routing automatically.
- Add _settings_schema to all 9 sensor classes (bme280, ens210, ina219, shtc3,
lafvin_ups_3s, waveshare_ups_d, waveshare_ups_e, hardware_stats, openhop_modem)
- Implement GET /sensors/types, GET /sensors/config, POST /sensors/config,
POST /sensors/read endpoints in api_endpoints.py
- Update openapi.yaml with Sensors tag and endpoint schemas
- Add underscore-to-slash path mapping in contract checker
- Add sensors to contract allowlist
Built from openHop_RepeaterUI feat/per-radio-airtime at fea8f8b, so the shipped
bundle carries the fabric fan-out controls the backend validation in 8f16a2c
expects a caller to drive.
fabric.repeat_on_ingress and fabric.origin_tx are only checked when
build_radio_stack assembles the radios. Everything that writes config -- the
UI, a backup restore, a hand edit -- ran long before that, so a combination
with no defined meaning was accepted, written to disk, and refused only at the
next restart, by which point the caller that could have fixed it was gone.
validate_fabric_config runs those same checks against a whole config mapping.
config_import calls it before the section-merge loop touches self.config, and
validate_config reports the result as a fabric error, so the pre-restart check
names the thing that will block the restart; it covered no fabric or radios key
at all before.
Checking before the loop rather than inside it is the point. The loop merges
section by section, so a rejection partway through would leave half an import
applied to a running node. A malformed radios section is refused the same way,
ahead of everything, since the loop's own check for it ran only once the
earlier sections were already in. That also keeps the message about the real
problem: a non-list radios previously reached the fan-out check as "one radio"
and came back as a radio-count error.
The merge the preflight models has to match the loop exactly, or it rejects
configs the daemon would accept. A dict fabric merges by update; anything else
replaces the section wholesale, including fabric: null. That last case is how
an operator clears fan-out left invalid by an older build, so validating the
old section there would reject the one import that repairs the node.
tx_mode is checked only where the daemon applies it. _apply_fabric_tx_mode --
the thing that raises on an unknown mode -- runs for a non-empty radios list or
use_fabric and nowhere else, so a legacy single-radio node carrying a stale or
misspelled tx_mode boots fine today. Refusing its unrelated imports would be
this check inventing a failure rather than reporting one.
Two independent adversarial reviews of this branch. Every defect below was
reproduced before being fixed, and each fix is pinned by a test that fails
against the code it replaces.
A channel's ledger now lives as long as the channel does, instead of being
rebuilt from copies of its neighbours' history on every config reload. The
copying was wrong in both directions at once:
- A channel whose radios all moved away lost its ledger, so the next radio to
tune there started from zero. 3000 ms already on the air, then a fresh
3600 ms allowance inside the same minute: 6600 ms/min against a 3600 ms/min
limit, reachable with two applies in the web UI.
- A split handed the same history to both sides and the next merge added the
copies together, so retuning a radio back and forth doubled the ledger each
cycle. Six applies turned 500 ms of real airtime into 4000 ms and stopped the
node forwarding for a minute; total_airtime_ms is a lifetime counter, so the
inflation also reached the MeshCore wire field, the RRD/SQLite history,
companion stats and /stats permanently.
Nothing is carried between channels now. A channel keeps what was transmitted
on it, and a radio arriving from a busy band brings none of that band's spend
with it, because duty cycle is a property of spectrum. Spectrum the node has
left keeps its ledger until its window empties, and its lifetime total joins the
node's running total rather than vanishing. The one exception is the ``None``
key used when radio profiles cannot be read: it stands for "some channel, we
cannot say which", so it inherits the busiest window rather than starting clean.
Three more:
- A config reload lands on another thread, and the gate computed the airtime
before the send while the debit recomputed it after. A retune in that window
charged the modulation the radio has now for a transmission that went out on
the modulation it had a moment ago -- 213.5 ms recorded as 26.7 ms. The debit
now reuses the figure the gate computed for that radio.
- Only the default radio can be retuned without a restart, but a rebuild adopted
every radio's configured modulation, so an unrelated duty-cycle save started
metering a pending change the hardware had not been given. refresh() now
adopts a new modulation only for radios that really got one; limits are policy
rather than hardware and are always adopted.
- POST /set_radio_config with radio_id: "" fell past the radios[] lookup without
matching or erroring, wrote the top-level block alone, and left the default
radio's own entry behind -- hardware on one bandwidth, its meter on another. A
blank id is now read as the request not naming one.
Holding every radio the fabric could pick to its own budget was safe against
overrun but was not the behaviour these nodes had: against an older openhop-core
an exhausted local radio refused sends the fabric was about to put on an idle
link radio, black-holing that band for the rest of the window. The gate asks the
fabric which radio it is about to use -- under the TX lock, an instant before
the send -- and holds that one to its budget. The candidate set remains the
answer only when the fabric will not say.
Suite: 2163 pass, 3 skipped against openhop_core dev; 2160 pass, 6 skipped
against c95a684, which predates the selector change.
yellowcooln