mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 11:12:35 +02:00
maziggy
de8a86c40f
A settings restore refuses to write anything credential-shaped, but wrote the
switches that depend on those credentials like any other key. Restoring the two
halves apart is not a partial restore, it is a downgrade.
The sharp case is Prometheus. /api/v1/metrics is on PUBLIC_API_ROUTES and its
only gate is `if token:`, so an empty or absent token means no authentication at
all. prometheus_token matches the `token` hint and is refused; prometheus_enabled
is an ordinary key and was written. On an instance that never enabled Prometheus
there is no local token row, so overwrite-*off* alone was enough to publish the
whole metrics body to anyone who could reach the port. The new integration test
shows exactly that: 200 with a full unauthenticated body before, 404 after.
Four more pairs are the same shape and break an integration rather than open one:
ldap_enabled/ldap_bind_password, mqtt_enabled/mqtt_password, ha_enabled/ha_token
(with an HA_TOKEN env arm, since get_homeassistant_settings prefers the
environment over the row), and virtual_printer_enabled/virtual_printer_access_code
— the last largely vestigial post-migration, included for consistency.
A toggle is refused only when all five hold: the payload value is truthy, the
backup carried a non-empty companion credential, that credential is denylisted,
this instance has no usable value for it, and the toggle is not already on
locally. The second condition is what keeps the rule honest — an anonymous MQTT
broker and an anonymous LDAP bind are legitimate configs that pass empty
credentials straight through, and without it both would be false positives. With
it, the rule fires only when the restore would produce a config weaker than both
the backup and the local instance. A present-but-blank prometheus_token row
counts as unusable, since that is precisely the `if token:` hole.
The rule needs the payload *and* local database state, which the old static
_count_items could not see, so preview and restore now share one classifier:
_plan_settings() runs a single SELECT over both halves of every candidate pair
before anything enters the session, and returns the three refusal buckets.
preview() takes the session the route already has. _is_skipped_setting_key is
gone rather than having its docstring corrected as asked: a name is no longer
enough to decide, so the union predicate had no caller left.
Also implements the review's third ruling — the tally counts what the preview
counted, and refusals live in the notes. Two `skipped += 1` increments are
dropped (blocked, protected) and the companion refusal adds none; the value-is-
None and overwrite-off skips stay, because they depend on the run's flags, which
the preview cannot see. restored + skipped + failed now equals the item count the
user was shown — off by three before.
Behaviour change called out for review: test_credential_keys_are_never_restored
and test_auth_settings_are_never_restored asserted skipped == 2 and 4; both are
now 0, which is the point of the ruling.
16 new unit tests plus 2 integration tests. Nine of them are controls, because
over-refusal is the real risk of this change — the anonymous-broker and
anonymous-bind guards are load-bearing, not decoration.