diff --git a/AGENTS.md b/AGENTS.md index 9ecb90f..97681bd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -130,7 +130,10 @@ Responsibilities, one line each: `VLLM_HISTOGRAMS`), measured Δ rates, prefill speed as `mean(prompt tokens)/mean(prefill time)`, session list from gufo `/slots`. - `sources/nut.py` — upsd client, `UpsReading`, `VAR_MAP`, `assess()` (explicit checks, not - a mystery score), `COMMANDS` catalog with risk tiers, `available_commands()` filtering, + a mystery score), `COMMANDS` catalog with risk tiers, `COMMAND_ALIASES` + `resolve_command()` + (drivers answer two names for the same action — `beeper.enable`/`beeper.on`, + `beeper.disable`/`beeper.off` — so one entry is offered per action and any of its names hits + that entry's verdict, never a gate the rendered name missed), `available_commands()` filtering, kelvin-vs-celsius normalisation, and `charger_status` normalised to the `CHARGER_STATUS` keys (the same lowercase token `states()` and `on_battery` already compare), and `states()` publishes one chip per fact — a driver exporting both `battery.charger.status` and the @@ -463,20 +466,43 @@ memory plot; the `cpu.usage_pct@coreN` series is still stored and still queryabl ## Known gaps / next work -1. **UPS instant commands are read-only in practice.** Reads work against real upsd (see - above), but every button is gated off with "NUT_USER / NUT_PASSWORD must name a user with - `actions` in upsd.users" because no such user exists yet. Creating one safely needs a - *read* of `/etc/ups/upsd.users` first — `sudo tee` can write the file but overwriting it - blind would delete the users upsmon already authenticates as, which can take the box down - at the wrong moment. `deploy/nut-sudoers` is the installable ruleset that makes that - read (and only a scoped write + reload) possible — it carries the - `ops ALL=(root) NOPASSWD: NUT_READ, NUT_WRITE` grant and is installed with - `sudo install -m 0440 -o root -g root deploy/nut-sudoers /etc/sudoers.d/dashboards-nut`, - but it is **repo-side only so far**: the live UPS cards still answer "NUT_USER / - NUT_PASSWORD must name a user with `actions` in upsd.users" (verified through - `/api/ups/snapshot`). Once it is installed: read `upsd.users`, add `[dashboards]` with - `instcmds` limited to the battery tests, put the credentials in `.env.dev`, and the - buttons come alive. Never grant `actions = -`, and never write the file blind. +1. **UPS commands work, and the account can now cut power — the app's gates are the control.** + Three defects were cleared on 2026-10-17, each verified against real hardware. (a) `NutClient.login()` sent `USER`/`PASS`, which the deployed + upsd 2.8.5 does not implement — each answered `ERR UNKNOWN-COMMAND`, so `_read_line()` + raised and `POST /api/ups/command` could only 502 before `instcmd()`, credentials or not. + It now sends `USERNAME`/`PASSWORD`, the pair the server's own `HELP` advertises + (`HELP VER PROTVER GET LIST SET INSTCMD LOGIN LOGOUT USERNAME PASSWORD STARTTLS`), pinned + by `test_login_uses_the_verbs_upsd_2_8_5_answers_to` — the fake upsd has no `USER`/`PASS` + branch, so a regression answers `ERR UNKNOWN-COMMAND` exactly like the real thing. + (b) 12 of the 23 commands the two real UPSes advertise had **no `COMMANDS` entry**, and + `available_commands()` drops anything the catalog misses — the page silently offered less + than the hardware does. Every advertised command is catalogued now, each with a risk tier: + `test_every_command_the_hardware_advertises_is_catalogued` fails if a capture gains a + command the catalog does not know, and + `test_every_command_that_can_cut_power_is_in_the_dangerous_tier` keeps `load.off`, + `driver.killpower` and the `shutdown.*` sequence in the tier that types `DANGER` and is + refused on battery. `shutdown.stop` is the exception the name hides — it *cancels* a + shutdown, so it is `safe`, and the test says so in words. + (c) `[dashboards]` in `/etc/ups/upsd.users` is `instcmds = all` at the operator's request, + so this app may send every advertised command on both UPSes, power-cutting ones included. + `.env.dev` (mode `0600`, gitignored) carries the credentials. + What remains to know: + - **Proven live, and reversibly.** `beeper.enable` returned `OK` and `ups.beeper.status` + read `enabled` on the APC afterwards; `driver.killpower` without the word `DANGER` still + answers 422 and never reaches the wire. A battery test has deliberately not been run — + it puts real load on a battery things are plugged into, which stays the operator's call. + - **`enabled: true` cannot mean "this account may send it".** `available_commands()` + filters on `LIST CMD`, which upsd serves **unfiltered**, and NUT has no call for a + user's privileges — so the wire cannot tell the page what the account is granted. With + `instcmds = all` the two agree by configuration rather than by check, which is precisely + why the risk tiers and the typed `DANGER` are load-bearing and not decoration. + - **Pre-existing upsd log noise:** every upsd start logs `Unknown upsmon type =` (20 + occurrences over 12 starts, oldest before this work). Harmless — `upsmon` logs into both + UPSes right after it — but unexplained. + Editing `upsd.users` at all: never grant `actions = -`, and never write it blind — the + running `upsmon` authenticates as its own account from that same file for both UPSes, so + dropping it means no orderly power-down. Copy the untouched sections from the live bytes + and `cmp` them before writing; `tee` truncates in place, so owner and mode survive. 2. **GPU coverage is AMD only.** `host_agent.gpus()` reads amdgpu DRM sysfs (`gpu_busy_percent`, `mem_info_vram_*`, hwmon temps, clocks). That covers the current fleet (R9700 + Strix Halo iGPU; gateway has none). An NVIDIA host would report no GPU at diff --git a/README.md b/README.md index 764f039..1e342ca 100644 --- a/README.md +++ b/README.md @@ -249,9 +249,23 @@ not `actions = -`: # /etc/ups/upsd.users [dashboards] password = "..." - instcmds = test.battery.start.quick test.battery.start.deep test.battery.stop + instcmds = test.battery.start.quick + instcmds = test.battery.start.deep + instcmds = test.battery.stop ``` +One command per `instcmds` line — that is what `man 5 upsd.users` specifies ("to specify +multiple commands, use multiple instances of the instcmds field"), and a comma-separated list +on one line is not that. `instcmds = all` is the alternative, and the fleet runs that: every +command the drivers advertise, including the ones that cut output power. Nothing about the +gates changes — the risk tier, the typed `DANGER`, `NUT_ALLOW_ACTIONS` and the on-battery +refusal are what prevent a click from becoming an outage, because upsd gives the dashboard no +way to ask what an account is allowed to send. + +upsd authenticates with `USERNAME` + `PASSWORD` (see `HELP` on your own server for the verbs +it accepts — `USER`/`PASS` are not among them in 2.8.5); a user needs `instcmds`, never +`actions = -`. + `deploy/nut-sudoers` is the ruleset that lets the `ops` account read (and scoped-write) those files: `sudo install -m 0440 -o root -g root deploy/nut-sudoers /etc/sudoers.d/dashboards-nut`. Read `upsd.users` before you write it — `upsmon` authenticates from the same file, so