From 7cadc437fd99102ac57975e5377762da27bad2a4 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Fri, 31 Jul 2026 13:01:38 +0200 Subject: [PATCH 1/2] fix(catalog): stop badging five drivers Control when they refuse every command myuplink, nibe_local, pixii_pv, solis_string and tibber declared control: true. All five have a driver_command that refuses unconditionally, and four say so in a comment on the line above the return false -- "Read-only: no actuation", "Tibber Pulse is a read-only meter; no control surface". The Control badge is the first thing an owner reads when deciding whether a driver can run their heat pump or their battery, and for these five it promised something the code refuses. They are now control: false, which is what their sources have said all along. No Lua changed and no artifact bytes moved: control is not among the fields the channel copies into the signed artifact, so no version had to move. Verified by rebuilding the channel from main and comparing all 80 digests. tests/test_control_claims.py compares the claim against the code, which nothing did before. A driver_command that calls nothing cannot reach a write however it is invoked. Reverting any of the five fails it by name. It also asserts at least 20 drivers still read as controlling, because a guard that stops recognising anything passes exactly as loudly as one that works -- which its own unit test caught while it was being written, when the call regex counted `function driver_command(` and so found every driver busy. Signed-off-by: HuggeK <48095810+HuggeK@users.noreply.github.com> Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com> --- CHANGELOG.md | 2 + devices.yaml | 10 +-- index.yaml | 10 +-- manifests/myuplink.yaml | 2 +- manifests/nibe_local.yaml | 2 +- manifests/pixii_pv.yaml | 2 +- manifests/solis_string.yaml | 2 +- manifests/tibber.yaml | 2 +- tests/test_control_claims.py | 123 +++++++++++++++++++++++++++++++++++ 9 files changed, 140 insertions(+), 15 deletions(-) create mode 100644 tests/test_control_claims.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 628f22e..c42e4c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ Driver versions follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html - **The release's version rule now runs on every pull request, where it can still be acted on.** The signed channel refuses to republish changed artifact bytes under a version it has already published — the rule that makes a version name a set of bytes an operator can pin. Asking it needed the published channel, so it only ever ran from the release job on `main`, *after* the merge: #53 merged with five green checks and took the release down, and the repair (#55) had to be a second pull request against a `main` that was already broken. Nothing about the question needs the signing key, though — the published manifest is a release asset and its signature verifies with the public key — so `ftw_repository.py check-versions` builds the artifacts in memory, signs nothing, and answers exactly what the release would. The new `signed channel accepts this tree` job runs it on every pull request, including from forks, which need no secret to read a public asset. It reports **every** colliding driver rather than the first, each with the `make bump-driver` command that fixes it. `_version_collisions` is the single statement of the rule that both the release and this check read, and `test_check_versions_answers_what_the_release_would_answer` holds them to the same verdict on the same tree — a rule stated twice is a rule that drifts, and a check that drifts from the release is worse than no check. `test_check_versions_catches_a_manifest_only_edit` is #53 in the shape it arrived in: a manifest field corrected, no Lua touched, and the bytes moved anyway ### Fixed +- **Five drivers were badged **Control** on the catalog page while being incapable of it.** `myuplink`, `nibe_local`, `pixii_pv`, `solis_string` and `tibber` all declared `control: true`, and all five have a `driver_command` that refuses unconditionally — four of them saying so in a comment on the line above the `return false`: *"Read-only: no actuation"*, *"Tibber Pulse is a read-only meter; no control surface"*. The page's **Control** badge is the first thing an owner reads when deciding whether a driver can run their heat pump or their battery, and for these five it promised something the code refuses. All five are now `control: false`, which is what their sources have said all along. No Lua changed and no artifact bytes moved — `control` is not among the fields the channel copies into the signed artifact — so no driver version had to move either +- `tests/test_control_claims.py` compares the claim against the code, which nothing did before. A `driver_command` that calls nothing cannot reach a write however it is invoked, so a manifest declaring control over one is making a claim its own entrypoint refuses. Reverting any of the five to `control: true` fails it by name, with the two ways out stated. It also asserts that at least 20 drivers still read as controlling, because a guard that quietly stops recognising anything passes exactly as loudly as one that works — a bug the guard's own unit test caught while it was being written, when the regex counted `function driver_command(` as a call and so found every driver busy - **`nibe_local` was filed as a meter.** Its manifest declared `ders: [meter]`; it reads a NIBE S-series heat pump. The driver emits nothing but metrics — `hp_power_w`, `hp_used_power_w`, the temperatures and the lifetime energy counters, all through `host.emit_metric` and never through `host.emit("meter", …)` — which its header states is deliberate, so that a pump's electrical draw cannot double-count against the site's real grid meter. The catalog was advertising a DER the driver does not provide, and the published page (#51) badges and filters on exactly that field: the pump sat under **Meter** and was absent from **Heat pump**, where an owner looking for it would start. Now `ders: [heatpump]`, matching `heishamon` — the other metric-only heat pump, which has been filed that way since it landed. The Lua source and its `sha256` are untouched, so the driver a host runs is byte-for-byte the one it ran before - **`nibe_local`** 1.1.0 → 1.1.1 — **the version has to move anyway, because the signed artifact carries this metadata.** `ders` is not catalog-only decoration: `_load_channel` copies it into the artifact's `capabilities` and the channel publishes source and metadata as one signed file, so correcting the field changed the published bytes. The channel then refused the release — `error: nibe_local: changed artifact needs a higher version than 1.1.0` — and it was right to, since that immutability is what lets an operator name the bytes a site is running. A patch rather than a minor: no register, field or emitted metric changed, only the DER the catalog states. The `DRIVER` table is deliberately left alone, keeping the driver byte-identical to its FTW baseline the way #29 established - **`myuplink` was filed under no device type at all.** Its manifest declared `ders: []`, so the one MyUplink cloud heat-pump driver was absent from **Heat pump** and from every other filter on the catalog page — unreachable by device type, which is how an owner looks for their pump. It is now `ders: [heatpump]`, matching the other two heat pumps. This revisits a deliberate choice: when #27 widened the DER vocabulary it set `myuplink` to none, reading `ders` as the emit stream a driver produces rather than the device it covers. #53 then settled the question the other way for `nibe_local`, citing `heishamon` — and the three drivers are the same case. All three report a pump through `host.emit_metric` and never call `host.emit`, `myuplink` and `heishamon` emit the same `hp_*` metric names, and two of the three already declared `heatpump`. `myuplink` was the one left behind, not the one holding the line diff --git a/devices.yaml b/devices.yaml index 9946885..a2dfcfa 100644 --- a/devices.yaml +++ b/devices.yaml @@ -542,7 +542,7 @@ manufacturers: driver: "solis_string" version: "1.1.1" ders: [pv] - control: true + control: false firmware_versions: "" notes: "Solis non-hybrid PV inverters via Modbus TCP." - name: "Growatt" @@ -965,7 +965,7 @@ manufacturers: driver: "myuplink" version: "1.1.1" ders: [heatpump] - control: true + control: false firmware_versions: "" notes: "Read-only heat-pump telemetry via MyUplink Cloud REST API v2: compressor power + hot-water/indoor/outdoor temperatures. Observe-only — no control. OAuth: authorization-code + refresh-token (connect in Settings → Devices)." - name: "NES" @@ -991,7 +991,7 @@ manufacturers: driver: "nibe_local" version: "1.1.1" ders: [heatpump] - control: true + control: false firmware_versions: "" notes: "Read-only NIBE S-series heat-pump telemetry over the on-prem Local REST API (HTTPS + Basic auth, self-signed cert pinned via tls_pin_sha256). Emits compressor/used power, lifetime energy meters, and the full ~980-point register map. Observe-only — no control." - name: "OpenEVSE" @@ -1030,7 +1030,7 @@ manufacturers: driver: "pixii_pv" version: "0.3.0" ders: [pv, meter] - control: true + control: false firmware_versions: "" notes: "Read-only Pixii telemetry: PV (external CTs) + grid meter (meter_w). Battery / SoC comes from the Modbus driver, not this one." - name: "PowerShaper" @@ -1794,7 +1794,7 @@ manufacturers: driver: "tibber" version: "1.1.0" ders: [meter] - control: true + control: false firmware_versions: "" notes: "Tibber Pulse grid meter via GraphQL-transport-ws liveMeasurement stream." - name: "VARTA" diff --git a/index.yaml b/index.yaml index 45aefd1..72ee92c 100644 --- a/index.yaml +++ b/index.yaml @@ -375,7 +375,7 @@ drivers: connectivity: cloud setup: [vendor_portal] ders: [heatpump] - control: true + control: false size_bytes: 15278 sha256: "d754fd4af96c2047f9d9e458a776ad8f10510569836618f4631dd70e2510f404" - name: "nibe_local" @@ -385,7 +385,7 @@ drivers: connectivity: local setup: [device_screen] ders: [heatpump] - control: true + control: false size_bytes: 18227 sha256: "b89177004a2eff5a5b60e2f275f584f079acf02876c049b231ba3170bb35ac50" - name: "opendtu" @@ -468,7 +468,7 @@ drivers: protocol: mqtt connectivity: local ders: [pv, meter] - control: true + control: false size_bytes: 6692 sha256: "d0901b6478e05b5cdf46fc2a9675700ba6008be2feb370be0ff7e87d2f1de883" - name: "saj" @@ -631,7 +631,7 @@ drivers: protocol: modbus connectivity: local ders: [pv] - control: true + control: false size_bytes: 5784 sha256: "3cfcf53653680dede7caf6173cee251efa3cf2c9ec46fdc16a89b9b526dceb7e" - name: "sonnen" @@ -670,7 +670,7 @@ drivers: connectivity: cloud setup: [vendor_portal] ders: [meter] - control: true + control: false size_bytes: 12410 sha256: "845ca59a775a3a4a4c27962454f14ba654fba0d1b905f24647c80db5c26a62ae" - name: "varta" diff --git a/manifests/myuplink.yaml b/manifests/myuplink.yaml index 03a17c5..c07bedd 100644 --- a/manifests/myuplink.yaml +++ b/manifests/myuplink.yaml @@ -6,7 +6,7 @@ protocol: http connectivity: cloud setup: [vendor_portal] ders: [heatpump] -control: true +control: false tested_devices: - manufacturer: "MyUplink (NIBE, Bosch, Atlantic, Daikin, ...)" model_family: "MyUplink Heat Pump (telemetry)" diff --git a/manifests/nibe_local.yaml b/manifests/nibe_local.yaml index 960b9ad..80c1c79 100644 --- a/manifests/nibe_local.yaml +++ b/manifests/nibe_local.yaml @@ -6,7 +6,7 @@ protocol: http connectivity: local setup: [device_screen] ders: [heatpump] -control: true +control: false tested_devices: - manufacturer: "NIBE" model_family: "NIBE REST API S-series" diff --git a/manifests/pixii_pv.yaml b/manifests/pixii_pv.yaml index 26967cf..0e699f4 100644 --- a/manifests/pixii_pv.yaml +++ b/manifests/pixii_pv.yaml @@ -5,7 +5,7 @@ author: "Sourceful Labs AB" protocol: mqtt connectivity: local ders: [pv, meter] -control: true +control: false tested_devices: - manufacturer: "Pixii" model_family: "Pixii PowerShaper (PV + meter)" diff --git a/manifests/solis_string.yaml b/manifests/solis_string.yaml index 8b0ba0c..133da9a 100644 --- a/manifests/solis_string.yaml +++ b/manifests/solis_string.yaml @@ -5,7 +5,7 @@ author: "Sourceful Labs AB" protocol: modbus connectivity: local ders: [pv] -control: true +control: false tested_devices: - manufacturer: "Ginlong Solis" model_family: "Solis string inverter" diff --git a/manifests/tibber.yaml b/manifests/tibber.yaml index 7ed6da5..ecd2136 100644 --- a/manifests/tibber.yaml +++ b/manifests/tibber.yaml @@ -6,7 +6,7 @@ protocol: http connectivity: cloud setup: [vendor_portal] ders: [meter] -control: true +control: false tested_devices: - manufacturer: "Tibber" model_family: "Tibber Pulse" diff --git a/tests/test_control_claims.py b/tests/test_control_claims.py new file mode 100644 index 0000000..c15e7a0 --- /dev/null +++ b/tests/test_control_claims.py @@ -0,0 +1,123 @@ +"""A driver badged "Control" must be able to control something. + +`control` is the manifest field the catalog page turns into a **Control** +badge, and it is the first thing an owner reads when deciding whether a driver +can run their battery or their heat pump. Five drivers carried `control: true` +while their `driver_command` was an unconditional refusal -- three of them +saying so in a comment on the line above the `return false`. + +Nothing caught it, because nothing compared the claim against the code. The +signed channel does not settle it either: it infers control from the mere +presence of a `driver_command` entrypoint, so a stub that always refuses reads +to it as a control path. + +This is the narrow, checkable half of that question: a command entrypoint that +cannot call anything cannot actuate anything, whatever the manifest says. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] + +# Keywords that open a Lua block, for balancing against `end`. +OPENERS = re.compile(r"\b(function|if|for|while|do)\b") +CLOSERS = re.compile(r"\bend\b") +ELSEIF = re.compile(r"\belseif\b") +CALL = re.compile(r"\b([A-Za-z_][A-Za-z_0-9.]*)\s*\(") +NOT_A_CALL = {"function", "if", "while", "for", "return", "and", "or", "not"} + + +def control_flag(manifest: Path) -> str: + for line in manifest.read_text(encoding="utf-8").splitlines(): + if line.startswith("control:"): + return line.split(":", 1)[1].strip() + return "" + + +def function_source(text: str, name: str) -> str | None: + """The source of a top-level `function name(...) ... end`, or None.""" + start = re.search(rf"^function\s+{re.escape(name)}\s*\(", text, re.M) + if not start: + return None + depth = 0 + collected: list[str] = [] + for line in text[start.start():].split("\n"): + collected.append(line) + code = re.sub(r"--.*$", "", line) + depth += len(OPENERS.findall(code)) + depth -= len(CLOSERS.findall(code)) + depth -= len(ELSEIF.findall(code)) + if len(collected) > 1 and depth <= 0: + break + return "\n".join(collected) + + +def is_inert(body: str) -> bool: + """Whether a function body can do nothing at all. + + Inert means it calls nothing: no host call, no helper, no dispatch. Such a + body can only fall through to its `return`, so it cannot reach a write no + matter what it is handed. + """ + # Drop the declaration line: `function driver_command(...)` is the + # signature, not a call, and counting it made every body look busy. + lines = re.sub(r"--.*$", "", body, flags=re.M).split("\n")[1:] + code = "\n".join(line for line in lines if line.strip()) + calls = [c for c in CALL.findall(code) if c not in NOT_A_CALL] + return not calls + + +def test_a_control_driver_has_a_command_that_can_do_something() -> None: + inert = [] + for manifest in sorted((ROOT / "manifests").glob("*.yaml")): + if control_flag(manifest) != "true": + continue + source = ROOT / "drivers" / "lua" / f"{manifest.stem}.lua" + body = function_source(source.read_text(encoding="utf-8"), "driver_command") + if body is not None and is_inert(body): + inert.append(manifest.stem) + + assert not inert, ( + "these manifests claim control: true, but their driver_command calls " + f"nothing and so can never actuate: {inert}. Either the driver has a " + "control path its command entrypoint does not reach, or the manifest " + "should say control: false.") + + +def test_the_check_can_still_see_a_real_control_path() -> None: + """A guard that flags nothing would pass just as quietly if it broke.""" + controlling = [] + for manifest in sorted((ROOT / "manifests").glob("*.yaml")): + if control_flag(manifest) != "true": + continue + source = ROOT / "drivers" / "lua" / f"{manifest.stem}.lua" + body = function_source(source.read_text(encoding="utf-8"), "driver_command") + if body is not None and not is_inert(body): + controlling.append(manifest.stem) + + assert len(controlling) >= 20, ( + "almost no driver reads as controlling, so this check has stopped " + f"measuring anything: {controlling}") + + +def test_a_refusal_stub_is_recognised_as_inert() -> None: + """The shape the five carried, so the guard is pinned to it directly.""" + assert is_inert( + "function driver_command(_action, _power_w, _cmd)\n" + " -- Read-only: no actuation.\n" + " return false\n" + "end") + # Accepting the lifecycle actions without acting on them is still inert. + assert is_inert( + "function driver_command(action)\n" + ' if action == "init" or action == "deinit" then return true end\n' + " return false\n" + "end") + # Reaching any call at all is not. + assert not is_inert( + "function driver_command(action, value)\n" + " return host.modbus_write(40001, value)\n" + "end") From d79aa5ba29006b2bb5639b4bc45525bf9f6598a8 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Fri, 31 Jul 2026 13:44:15 +0200 Subject: [PATCH 2/2] WIP: make the signed artifact agree that myuplink is read-only Not ready to merge. Two blockers found while building it, both external to this diff -- recorded here so the work is not lost. What this does: myuplink declares read_only = true and auth_post_path = "/oauth/token" in its DRIVER table. The channel then publishes it read_only with control_enabled false, and the generated guard permits host.http_post only for a URL whose path ends in the declared path, refusing it everywhere else. So the flag enforces "this POST is authentication" rather than asserting it. A path rather than a URL because base_url is config-overridable. Blocker 1 -- FTW rejects it. go/internal/driverrepo/sourceful.go, validateSourcefulReadOnlyContract, allows a read-only package only http.get, modbus.read, mqtt.subscribe, serial.read and returns "read-only package requests write-capable permission" for anything else. Publishing this without an FTW change first makes every host refuse myuplink at install. Blocker 2 -- editing the Lua forfeits the FTW-baseline exemption. drivers/tests/conftest.py exempts a driver byte-identical to baselines/ftw and says so explicitly: it "stops being exempt the moment someone edits it". Adding two metadata fields costs the exemption and lights up 7 catalog-convention checks this driver was never written to meet -- among them "should use 'http://' scheme" and "reference config.port" for an HTTPS cloud API, and "should call host.emit()" when emitting only metrics is the driver's stated design. The catalog half is already shippable and separate: see the branch fix/myuplink-read-only-catalog (#64), which is green. Signed-off-by: HuggeK <48095810+HuggeK@users.noreply.github.com> Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com> --- SUPPORT_STATUS.md | 4 +- devices.yaml | 2 +- drivers/lua/myuplink.lua | 11 ++++- index.yaml | 6 +-- manifests/myuplink.yaml | 6 +-- support-status.json | 2 +- tests/test_ftw_repository.py | 85 ++++++++++++++++++++++++++++++++++++ tools/ftw_repository.py | 60 +++++++++++++++++++++++-- 8 files changed, 161 insertions(+), 15 deletions(-) diff --git a/SUPPORT_STATUS.md b/SUPPORT_STATUS.md index e7c4672..1e6b057 100644 --- a/SUPPORT_STATUS.md +++ b/SUPPORT_STATUS.md @@ -84,8 +84,8 @@ Catalog source is not proof that a target can install or run a driver. | kstar | 1.1.1 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no | | mennekes | 1.0.3 | ftw-core | not_assessed | — | — | not_recorded | — | not_assessed | no | | mennekes | 1.0.3 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no | -| myuplink | 1.1.1 | ftw-core | not_assessed | — | — | not_recorded | — | not_assessed | no | -| myuplink | 1.1.1 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no | +| myuplink | 1.2.0 | ftw-core | not_assessed | — | — | not_recorded | — | not_assessed | no | +| myuplink | 1.2.0 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no | | nibe_local | 1.1.1 | ftw-core | not_assessed | — | — | not_recorded | — | not_assessed | no | | nibe_local | 1.1.1 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no | | opendtu | 1.0.2 | ftw-core | not_assessed | — | — | not_recorded | — | not_assessed | no | diff --git a/devices.yaml b/devices.yaml index a2dfcfa..1147dde 100644 --- a/devices.yaml +++ b/devices.yaml @@ -963,7 +963,7 @@ manufacturers: protocols: - protocol: http driver: "myuplink" - version: "1.1.1" + version: "1.2.0" ders: [heatpump] control: false firmware_versions: "" diff --git a/drivers/lua/myuplink.lua b/drivers/lua/myuplink.lua index 8d44083..456dbd9 100644 --- a/drivers/lua/myuplink.lua +++ b/drivers/lua/myuplink.lua @@ -48,9 +48,18 @@ DRIVER = { id = "myuplink", name = "MyUplink Heat Pump (telemetry)", manufacturer = "MyUplink (NIBE, Bosch, Atlantic, Daikin, ...)", - version = "1.0.0", + version = "1.2.0", protocols = { "http" }, capabilities = { "apicreds" }, + -- Says what the header, the description and driver_command have always + -- said. Without it the channel infers control from the mere presence of a + -- driver_command entrypoint and publishes this driver write-capable. + read_only = true, + -- ...but it can read nothing until it has signed in, and it signs in with a + -- POST. The generated read-only guard allows POST only to a URL ending here + -- and refuses it everywhere else, so this narrows the exemption rather than + -- asserting it. A path, not a URL, because base_url is config-overridable. + auth_post_path = "/oauth/token", description = "Read-only heat-pump telemetry via MyUplink Cloud REST API v2: compressor power + hot-water/indoor/outdoor temperatures. Observe-only — no control. OAuth: authorization-code + refresh-token (connect in Settings → Devices).", homepage = "https://dev.myuplink.com", http_hosts = { "api.myuplink.com" }, diff --git a/index.yaml b/index.yaml index 72ee92c..d95f086 100644 --- a/index.yaml +++ b/index.yaml @@ -369,15 +369,15 @@ drivers: size_bytes: 3766 sha256: "5f985b8917aea7b08fba13c02506b45e40232a763393dc6af7c6eeb6ab5853af" - name: "myuplink" - version: "1.1.1" + version: "1.2.0" tier: core protocol: http connectivity: cloud setup: [vendor_portal] ders: [heatpump] control: false - size_bytes: 15278 - sha256: "d754fd4af96c2047f9d9e458a776ad8f10510569836618f4631dd70e2510f404" + size_bytes: 15874 + sha256: "9a0917a31d29907155574b2eb6df89d1fba2431457e2608e64ea42b8fa24057b" - name: "nibe_local" version: "1.1.1" tier: core diff --git a/manifests/myuplink.yaml b/manifests/myuplink.yaml index c07bedd..59d2081 100644 --- a/manifests/myuplink.yaml +++ b/manifests/myuplink.yaml @@ -1,5 +1,5 @@ name: "myuplink" -version: "1.1.1" +version: "1.2.0" tier: core author: "Sourceful Labs AB" protocol: http @@ -21,9 +21,9 @@ upstream_docs: kind: changelog url_stability: stable min_host_version: "2.0.0" -size_bytes: 15278 +size_bytes: 15874 dkb_id: "myuplink" -sha256: "d754fd4af96c2047f9d9e458a776ad8f10510569836618f4631dd70e2510f404" +sha256: "9a0917a31d29907155574b2eb6df89d1fba2431457e2608e64ea42b8fa24057b" signature: "" bytecode_sha256: "" diff --git a/support-status.json b/support-status.json index 06b9aa3..495d46c 100644 --- a/support-status.json +++ b/support-status.json @@ -1122,7 +1122,7 @@ }, { "catalog_source": true, - "catalog_version": "1.1.1", + "catalog_version": "1.2.0", "driver_id": "myuplink", "package_id": null, "targets": { diff --git a/tests/test_ftw_repository.py b/tests/test_ftw_repository.py index 94a978f..0ca12d7 100644 --- a/tests/test_ftw_repository.py +++ b/tests/test_ftw_repository.py @@ -23,6 +23,7 @@ from ftw_repository import ( # noqa: E402 RepositoryError, + _load_channel, build_publication, canonical_json, check_publication, @@ -266,6 +267,16 @@ def test_manifest_requires_exact_canonical_envelope_bytes( ), "read-only driver has a write-capable permission", ), + # http.post is the one write permission a read-only driver may hold, + # and only when its metadata declares the sign-in it was granted for. + # Undeclared, it is refused like any other write path. + ( + lambda driver: driver.update( + read_only=True, control_enabled=False, + permissions=["http.get", "http.post"], + ), + "read-only driver has a write-capable permission", + ), ( lambda driver: driver["metadata"].update(read_only=False), "Lua metadata read_only must match the manifest", @@ -683,3 +694,77 @@ def test_a_driver_that_declares_read_only_keeps_its_write_guards( assert sdm630["permissions"] == ["modbus.read"] assert sdm630["read_only"] is True assert sdm630["control_enabled"] is False + + +def test_a_read_only_driver_may_still_sign_in( + tmp_path: Path, keypair: tuple[str, str] +) -> None: + """Reading after authenticating is still reading. + + myuplink cannot actuate anything -- driver_command refuses every command it + is handed -- but it reads nothing until it has exchanged a refresh token, + and it exchanges it with a POST. Denying that POST would have cost a + read-only driver every reading it takes, so read-only would have been + unusable for the drivers that most obviously deserve it. + """ + manifest, output = build(tmp_path, keypair) + myuplink = next(d for d in manifest["drivers"] if d["id"] == "myuplink") + artifact = (output / Path(myuplink["url"]).name).read_text() + + assert myuplink["read_only"] is True + assert myuplink["control_enabled"] is False + assert myuplink["metadata"]["auth_post_path"] == "/oauth/token" + assert "http.post" in myuplink["permissions"] + + # The exemption is scoped, not a hole: POST reaches the real host function + # only for a URL ending in the declared path. + assert 'local __sourceful_ftw_auth_path = "/oauth/token"' in artifact + assert "path:sub(-#__sourceful_ftw_auth_path) == __sourceful_ftw_auth_path" in artifact + assert "POST is allowed only for authentication" in artifact + # Everything else a read-only driver must not do is still refused. + for denied in ("modbus_write", "modbus_write_multi", "mqtt_publish", "serial_write"): + assert f"host.{denied} = __sourceful_ftw_write_denied" in artifact + + +def test_signing_in_is_declared_or_it_does_not_happen( + tmp_path: Path, keypair: tuple[str, str] +) -> None: + """A read-only driver that declares nothing keeps the blanket denial.""" + manifest, output = build(tmp_path, keypair) + exempt = [] + for driver in manifest["drivers"]: + if not driver["read_only"]: + continue + artifact = (output / Path(driver["url"]).name).read_text() + if "host.http_post = __sourceful_ftw_write_denied" not in artifact: + exempt.append(driver["id"]) + else: + assert "auth_post_path" not in driver["metadata"], driver["id"] + assert "http.post" not in driver["permissions"], driver["id"] + assert exempt == ["myuplink"], f"unexpected drivers allowed to POST: {exempt}" + + +def test_auth_post_path_must_be_a_path_and_must_mean_something( + tmp_path: Path +) -> None: + """Two ways to declare it wrongly, both refused while building.""" + repo, config_path = single_driver_repo(tmp_path, "myuplink") + source = repo / "drivers" / "lua" / "myuplink.lua" + original = source.read_text(encoding="utf-8") + + # A whole URL rather than a path: the guard matches on the path, so a URL + # here would silently never match and the driver could not sign in. + source.write_text( + original.replace('auth_post_path = "/oauth/token"', + 'auth_post_path = "https://api.myuplink.com/oauth/token"'), + encoding="utf-8") + with pytest.raises(RepositoryError, match="must be a path beginning with"): + _load_channel(config_path, repo) + + # Declared on a driver that is not read-only, where it exempts nothing. + source.write_text( + original.replace(" read_only = true,\n", ""), encoding="utf-8") + with pytest.raises(RepositoryError, match="only means anything with read_only"): + _load_channel(config_path, repo) + + source.write_text(original, encoding="utf-8") diff --git a/tools/ftw_repository.py b/tools/ftw_repository.py index 7c1d745..7452852 100644 --- a/tools/ftw_repository.py +++ b/tools/ftw_repository.py @@ -268,7 +268,8 @@ def _lua_string_list(values: list[str]) -> str: return "{ " + ", ".join(_lua_string(value) for value in values) + " }" -def _ftw_artifact(raw: bytes, metadata: dict[str, Any], read_only: bool) -> bytes: +def _ftw_artifact(raw: bytes, metadata: dict[str, Any], read_only: bool, + auth_post_path: str = "") -> bytes: """Add FTW metadata and the host-call polyfills older hosts lack. A driver the catalog marks control: true keeps its control functions. The @@ -280,6 +281,15 @@ def _ftw_artifact(raw: bytes, metadata: dict[str, Any], read_only: bool) -> byte A driver that declares read_only in its own DRIVER table still gets the write guards: those are meters and telemetry gateways saying what they are, not a policy imposed on them. + + `auth_post_path` is for a driver that can only read once it has signed in. + Over HTTP a POST is not evidence of actuation -- it is also how a driver + exchanges a refresh token -- so denying it outright would cost such a + driver every reading it takes. The exemption is scoped rather than trusted: + POST is permitted only to a URL ending in the declared path, and denied + everywhere else, so the flag enforces "this POST is authentication" instead + of merely asserting it. Matching the path rather than a whole URL survives + a site pointing the driver at its own base URL. """ protocols = metadata.get("protocols", []) capabilities = metadata.get("capabilities", []) @@ -313,11 +323,28 @@ def _ftw_artifact(raw: bytes, metadata: dict[str, Any], read_only: bool) -> byte # marks control: true keeps the control path it was ported with. write_guards = "" if read_only: + if auth_post_path: + # Sign in, then read. Anything else this driver tries to POST is + # refused exactly as if it had no exemption at all. + http_post_guard = ( + "local __sourceful_ftw_http_post = host.http_post\n" + f"local __sourceful_ftw_auth_path = {_lua_string(auth_post_path)}\n" + "host.http_post = function(url, ...)\n" + " local path = type(url) == \"string\" and url:match(\"^[^?]*\") or \"\"\n" + " if path:sub(-#__sourceful_ftw_auth_path) == __sourceful_ftw_auth_path then\n" + " return __sourceful_ftw_http_post(url, ...)\n" + " end\n" + " error(\"this driver declares itself read-only: " + "POST is allowed only for authentication\")\n" + "end\n" + ) + else: + http_post_guard = "host.http_post = __sourceful_ftw_write_denied\n" write_guards = ( "local function __sourceful_ftw_write_denied()\n" " error(\"this driver declares itself read-only\")\n" "end\n" - "host.http_post = __sourceful_ftw_write_denied\n" + + http_post_guard + "host.modbus_write = __sourceful_ftw_write_denied\n" "host.modbus_write_multi = __sourceful_ftw_write_denied\n" "host.modbus_write_multiple = __sourceful_ftw_write_denied\n" @@ -421,6 +448,17 @@ def _load_channel(config_path: Path, repo_root: Path) -> list[dict[str, Any]]: r"^\s*read_only\s*=\s*true", body, re.MULTILINE ) is not None + # A read-only driver that has to sign in before it can read anything. + # It names the path its token exchange goes to, and the generated guard + # holds it to exactly that -- see _ftw_artifact. + auth_post_path = _string_field(body, "auth_post_path") if body else "" + if auth_post_path and not auth_post_path.startswith("/"): + raise RepositoryError( + f"{driver_id}: auth_post_path must be a path beginning with '/'") + if auth_post_path and not declares_read_only: + raise RepositoryError( + f"{driver_id}: auth_post_path only means anything with read_only") + has_driver_command = "driver_command" in set(ENTRYPOINT_RE.findall(source)) required_entrypoints = {"driver_init", "driver_poll"} @@ -517,13 +555,20 @@ def _load_channel(config_path: Path, repo_root: Path) -> list[dict[str, Any]]: if value: metadata[output_name] = value - artifact = _ftw_artifact(raw, metadata, read_only=not controls) + if auth_post_path and not controls: + metadata["auth_post_path"] = auth_post_path + + artifact = _ftw_artifact(raw, metadata, read_only=not controls, + auth_post_path=auth_post_path if not controls else "") if len(artifact) > MAX_DRIVER_BYTES: raise RepositoryError(f"{driver_id}: generated FTW artifact is too large") permissions = list(PROTOCOL_PERMISSIONS[protocol]) if controls: permissions += PROTOCOL_WRITE_PERMISSIONS[protocol] + elif auth_post_path: + # Read-only, but it cannot read a thing until it has signed in. + permissions += PROTOCOL_WRITE_PERMISSIONS[protocol] entries.append( { @@ -622,7 +667,14 @@ def _validate_manifest(manifest: dict[str, Any]) -> None: for values in PROTOCOL_WRITE_PERMISSIONS.values() for permission in values } - if read_only and write_permissions.intersection(permissions): + # A read-only driver may hold exactly one write-capable permission, and + # only the one its declared sign-in needs: the artifact's guard confines + # that POST to auth_post_path, so the permission cannot reach further + # than the token exchange it was granted for. + allowed_write = set() + if metadata.get("auth_post_path"): + allowed_write = {"http.post"} + if read_only and write_permissions.intersection(permissions) - allowed_write: raise RepositoryError(f"{driver_id}: read-only driver has a write-capable permission") if driver.get("channel") not in {"beta", "stable"}: raise RepositoryError(f"{driver_id}: invalid channel")