mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 03:01:21 +02:00
e879cd6100a138e0cd7fc7813797f282c9318b73
3820
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 maziggy
|
e879cd6100 |
build(frontend): rebuild static/ so the restore UI actually ships (#2656)
The dev merge took dev's static/index.html, which loads the pre-restore
bundle, and left both bundles tracked. Merged as-is none of the frontend
shipped: no Restore button, no modal, no Type column.
Rebuild drops the superseded bundle and points index.html at a single one
that carries restoreFromGit and this round's new note leaf. The CSS
hashes identically to dev'"'"'s, so index.html differs by the script line
alone.
|
||
|
maziggy
|
a0d1c3674b |
fix(backup): keep both restore tallies equal to the number the preview showed (#2656)
Two ways the K-profile and spool categories broke the
restored + skipped + failed == item_count invariant the settings count
holds:
* The spools preview counted only the spools and put the usage records
in the detail, but _restore_spool_usage increments the same tally, so
any backup with usage history reported a total larger than the number
the user was shown. The preview now counts both and the detail breaks
the total down instead of adding to it.
* A K-profile entry that is not a dict was dropped silently on the
connected path. _kprofile_profile_count includes it, so the offline,
printer-missing and step-failed paths all account for it; only the one
path that talks to a printer let it leave the tally. It now counts
failed.
|
||
|
maziggy
|
43d268655f |
fix(backup): commit each database category so SQLite's writer is not held (#2656)
The database phase had the same shape the K-profile phase did: _find_
archive, _find_spool, the usage dedupe and _restore_settings are all one
SELECT per row or per key, interleaved with autoflushed INSERTs, inside a
single open write transaction. A few thousand archives plus a full usage
history plausibly passes the 15 s busy_timeout, and every concurrent
writer in the app fails with "database is locked" until it finishes.
Each category now commits before the next starts. The id maps are plain
dicts in memory and the session is expire_on_commit=False, so the
ordering tolerates it.
The cost is that a later failure no longer rolls back an earlier
category, so a tally is recorded only after its category commits and
run_restore reports the categories already on disk instead of an empty
result - the same correction the K-profile split needed.
|
||
|
maziggy
|
732b1d7034 |
fix(backup): gate every restore category on the permission owning its rows (#2656)
settings was gated on settings:update because a restore rewrites rows
PUT /api/v1/settings/ owns. The same argument applies to the other three
categories, and gating one but not the rest is the only state that is
not defensible: a role holding Backup alone could still write spools,
archives and K-profiles through a restore that it cannot write through
the endpoints that own them.
Each category now also requires that endpoint's write permission -
inventory:update, archives:update_all and kprofiles:update. archives
takes update_all rather than create because a restore writes rows owned
by other users, which is exactly what update_all means.
All missing permissions are reported in one refusal: a restore is a
multi-select, so naming them one at a time turns picking four categories
into four round trips.
|
||
|
maziggy
|
2ef43943d3 |
docs(backup): say what the secret-key hints actually refuse (#2656)
The comment called the hint list belt-and-braces over keys the collector
already refuses to write. It is not: _collect_settings filters exactly
bambu_cloud_token and auth_secret_key, so a current backup really does
carry mqtt_password, ldap_bind_password, ha_token and prometheus_token,
and the hints are the only thing that refuses them. The companion-
credential rule sits downstream of that, so reading the list as redundant
and shortening it would write a stale credential and make that rule inert
at the same time.
Comment and test docstring only - no behaviour change.
|
||
|
maziggy
|
ca6b9f8fe0 |
fix(backup): resolve a restored archive's owner by username, not by id (#2656)
created_by_id is only meaningful on the instance that wrote it. Restoring
onto a rebuilt instance - this feature's main use case - renumbers the
users table, so a live id can land on a different person and hand one
user's print history to another under archives:read_own. The id path
cannot even detect that: archivesOwnerCleared fires only for an id that
is absent, so a valid-but-wrong id produced no note at all.
The collector now records created_by_username alongside the id, and the
restore prefers it. username is unique on users, so a match is the same
person; the one case it cannot resolve - a user renamed since the backup
- falls through to ownerless with a note rather than guessing from the
id. The id stays as the fallback for backups taken before this change.
|
||
|
maziggy
|
09a7413efc | implemented pr feedback #2 | ||
|
maziggy
|
12c6749abe | Merge remote-tracking branch 'upstream/dev' into feature/billing | ||
|
maziggy
|
b1610f70ae | Security hardening (maziggy/bambuddy-security #9) | ||
|
maziggy
|
5bee8df5cd |
Broadcast AMS slot changes that keep the same material
Configuring a slot from the printer card left the card showing the old
filament until a reload or the 30s fallback poll. The command reached the
printer and the printer applied it; the update just never got broadcast.
on_printer_status_change deduplicates WebSocket pushes against a status_key
whose AMS part carried id, tray_type and state. Configure Slot writes none of
those -- it writes tray_info_idx, tray_color, tray_sub_brands and cali_idx. So
PLA to another brand or colour of PLA produced an identical key and was
dropped, while PLA to PETG came through. Reset always worked because it clears
tray_type.
Those four fields only move when someone configures a slot or swaps a spool,
so this costs no broadcasts mid-print. remain stays out of the key for the
opposite reason.
|
||
|
maziggy
|
50399144a1 |
Write printer status straight through while the tab is hidden (#2754)
Removing the requestAnimationFrame wrapper fixed the total stall but left the
100ms coalescing timer in the path, and a hidden page's timers are clamped to
once a second at best -- once a minute past five minutes hidden. The reporter
still saw a tab title at 2% beside a page at 40%.
The coalescing guards against a render cascade, which a hidden tab cannot
have, so it is skipped there and kept while visible.
The existing hidden-tab tests advanced fake timers, which simulates the timer
the browser was throttling; the new one never advances the clock.
|
||
|
maziggy
|
f35c184b52 |
Show Home Assistant sensors on the printer card (#1148, #448)
Binds binary_sensor and reading-carrying sensor entities to a printer and
renders their state on its card, worded by Home Assistant's device_class.
Optional per-sensor alert condition drives a notification on the transition
into the alert state and an opt-in interlock that holds queued prints while
alerting -- a hold with a readable waiting_reason, never a failure, and only
ever on a sensor that was read successfully.
Sensors get their own table rather than a wider entity pattern on SmartPlug:
get_smart_plug_by_printer would otherwise hand the card's power button a door
contact to switch.
The hold is passed to the model matcher directly rather than merged into
busy_printers: _check_auto_drying reads that set as "is currently printing"
and would put an idle-but-held printer down the mid-print drying path.
The notification_providers migration spells its default FALSE, not 0 --
Postgres rejects an integer default for a boolean and _safe_execute swallows
the error.
|
||
|
maziggy
|
372ab3b788 |
Route external spools to a nozzle when the printer has no AMS (#2771)
Five X2Ds with no AMS, each printing from its external spool holder,
took a job sent to a named printer and refused the same job sent to
"Any X2D": the file uploaded, the firmware answered 0700_8012 "Failed
to get AMS mapping table", and the item failed after three attempts.
A named-printer job carries a mapping the frontend resolved at queue
time, so the scheduler's matcher never runs. A model-based job has no
printer until dispatch, so the matcher does run -- and could not see an
external spool on a dual-nozzle printer. _build_loaded_filaments derived
dual-nozzle status from ams_extruder_map, which is built from AMS info
bits, so a printer with zero AMS units reported an empty map; every
external spool got extruder_id=None, and the nozzle-aware hard filter in
_match_filaments_to_slots discarded it because None equals neither 0 nor
1. The mapping came back all -1, was cleared to None, and the print
command went out as use_ams:true with no ams_mapping and no
ams_mapping2 at all.
This is the backend half of #1257, which fixed the same logic in
useFilamentMapping.ts and left this copy behind. Mirror its inference:
a populated nozzles[1].nozzle_diameter, a non-empty ams_extruder_map, or
more than one vt_tray entry. Replaying the reporter's own push-status
now yields extruder 1 for Ext-L and 0 for Ext-R, and a nozzle-1
requirement resolves to [254] -- what their working named-printer
dispatch sent. Single-nozzle printers keep extruder_id=None; nozzles
always has two entries, so its length alone must not be the signal.
Also stop dispatching a job the firmware is certain to reject. When the
matcher ran, matched nothing, and the printer has no AMS, fail the item
with the filament and nozzle it wants instead of spending an upload and
two retries on it -- that path already ended in a failed item, just an
opaque one. With an AMS attached the firmware error still stands, since
there the user can load a spool and press Resume. Fail-safe like the
nozzle-diameter guard (#1899): every branch short of a positive finding
returns None and dispatches as before.
_apply_filament_overrides is extracted from _compute_ams_mapping_for_printer
so the message names the filament the matcher looked for rather than the one
the 3MF was sliced with.
|
||
|
maziggy
|
ae802e74ca |
Say why a drying cycle ended when the firmware cuts it short (#2770)
An H2D started a twelve-hour PETG dry at 65 degC and the AMS gave up on it
twenty minutes in, with 700 of the 720 minutes still on the clock. It cooled,
humidity climbed back over the threshold, auto-drying started another
twelve-hour cycle, and that one went the same way; the reporter's AMS
temperature history shows the loop running all morning.
The log had one line for it: "AMS 0 drying complete", which is exactly what it
says for a dry that ran its full twelve hours. Nothing in a support bundle told
the two apart, and the one number that does -- the time still remaining -- was
written into that line as the previous value, where it reads like a duration
rather than a shortfall. The reporter took 700 for seconds and concluded the
cycle had lasted twelve minutes.
Bambuddy did not stop that cycle; every stop it sends is logged with the full
outgoing command and there was none. So ending it was the printer's decision,
and the account of why lives in three things already received and parsed and
never written down: the drying phase and sub-phase from the AMS info hex, the
per-unit dry_sf_reason constraint codes, and the live HMS errors.
A cycle that ends with most of its countdown left now logs all three alongside
how much of the requested duration ran. One that reaches its duration keeps the
single line it has always had. A stop Bambuddy sent is named as ours -- it is
short of its duration too, and on the telemetry alone is indistinguishable from
the firmware abandoning the cycle, so without tracking it the print-takes-priority
stop and the Stop button would both have been blamed on the printer.
Diagnostics only. Nothing about when drying starts or stops has changed, and the
restart loop is not addressed: what the firmware objects to has to be established
before Bambuddy can sensibly decide how long to wait before trying again.
|
||
|
maziggy
|
9f12eefcf9 |
Merge pull request #2763 from pascalheidmann/feature/slicer-multi-button
Feature/slicer multi button |
||
|
maziggy
|
4cbbd52aa0 |
fix(slicer): drop the legacy library:read from the slice gate (#2725)
The desktop handoff accepted library:read alongside read_all/read_own, on
the reasoning that default groups do not carry it and requiring it would
lock out Operators and Viewers. The permission grants nothing in that
position: the slicer-token endpoint gates on
require_ownership_permission(LIBRARY_READ_ALL, LIBRARY_READ_OWN), and
neither that dependency nor User.has_permission expands the legacy name,
so a group holding only library:read gets a 403 there. It cannot reach
the File Manager to try, either - GET /library/folders gates on the same
pair - and the library:read -> library:read_own migration in
core/database.py runs only over the groups named in DEFAULT_GROUPS, so a
custom role that still carries it stays stuck rather than being upgraded.
custom role that still carries it stays stuck rather than being upgraded.
Accepting it only enabled a menu item the server refuses, and the failure
is indistinguishable from "no slicer installed" once the catch hands the
unauthenticated URL over. Removed, with the comment recording the reason
so the next reader does not re-add it, and a test that pins it.
---
refactor(slicer): share one sliceable-file-type rule (#2725)
The File Manager and the 3D preview decide the same thing about the same
file and each held its own list of extensions - which is how they came to
disagree, offering a desktop handoff for an STL whose own preview showed
"Open in Slicer" greyed out. Making the two lists identical fixed the
symptom and left the drift, so SLICEABLE_FILE_TYPES now lives in
utils/slicer.ts with isSliceableFileType for a stored file_type and
isSliceableFilename for a name.
The filename form still rules out the compound extensions explicitly,
since .gcode.3mf ends with .3mf; the type form does not need to, because
classify_file_type stores that one whole.
Both test files mocked the whole slicer module, which would have replaced
the new predicates with undefined - switched to importOriginal so only
openInSlicer is stubbed. That is the better shape regardless: the tests
now exercise the rule the component runs instead of a copy declared
beside them.
Carries the rebuilt bundle. The CSS hash moves with it - the split button
introduces Tailwind classes the previous build had no reason to emit.
|
||
|
maziggy
|
da1f655c3d |
Cover nested data structures in the HA notify pass-through (#1441)
The three tests around it use flat scalars, which is also all the field's
placeholder and the wiki showed, so nothing recorded that the value is
forwarded verbatim rather than treated as a key/value list. A reporter asked
whether action buttons work; they always have, and now that is pinned.
The changelog entry said "nested options work" and left it there. It now names
actions and the two things that decide whether the buttons do anything - the
mobile_app_notification_action automation, and iOS needing a registered
category - since neither is set from Bambuddy and both are what a reader would
otherwise have to discover the way the reporter did.
|
||
|
maziggy
|
d8d637cbfe |
Survive a directory that defines no posixGroup class (#2769)
Every LDAP user on an lldap directory was rejected with "Incorrect
username or password", on an install where Test Connection passed and
where the same bind DN, filter and group membership all checked out
under ldapsearch. The directory never saw the request.
_extract_user_info searches for POSIX groups alongside the memberOf
ones, and both of those filters name the posixGroup object class. ldap3
fetches the schema at connect time (get_info=ALL) and validates class
names in a filter against it while building the request, raising
LDAPObjectClassError before anything is sent. lldap marks every account
it creates as posixAccount -- which is what makes us look for POSIX
groups at all -- but defines no group class beyond groupOfNames. The
exception escaped authenticate_ldap_user, and the login route reports
any LDAP failure as bad credentials.
A directory with no posixGroup class has no posixGroup entries, which is
exactly the answer those searches would have returned. Catch it, log it
once, and carry on with the memberOf groups collected above. Both
searches sit inside the one try: they name the same class, so once one
is rejected the other cannot succeed, and attempting it would only
produce a second identical exception to swallow.
Not a regression from
|
||
|
maziggy
|
868cfa2d1e |
Read the printer's own slot mapping when Spoolman has none (#2768)
A sliced file numbers its filaments 1..4; which AMS tray each came from is
a separate decision made when the job is sent. store_print_data learns it
from one of two sources, both of which require the print command to pass
through us: the mapping Bambuddy chose itself, or the one it intercepted
on the printer's local request topic. A job dispatched from Bambu Studio
while the printer is cloud-bound satisfies neither -- the command travels
through Bambu's broker and never reaches the topic we subscribe to.
slot_to_tray is then NULL and _resolve_global_tray_id guesses by position:
filament 1 from the first loaded tray, filament 2 from the second. The
reporter's X1C was loaded in the order 2, 4, 1, AMS-HT, so all four slots
were charged to the wrong spool. Their log carries the printer's own
answer, mapping=[1, 3, 0, 32768], sitting unread.
usage_tracker has consulted that field since it started resolving mappings
at completion, along with a colour match against the loaded trays for the
models that never publish it (A1, A1 Mini, P1S, P2S). Only the Spoolman
writer, which resolves at print start, never learned to -- and main.py
gates usage_tracker behind Spoolman being off, so enabling Spoolman is
what costs you the better resolver.
_resolve_slot_to_tray_fallback gives it both, at completion rather than at
print start: a printer keeps publishing the last job's mapping while it
sits idle, so reading it early would risk stamping the previous print's
mapping onto this one. A mapping we or the slicer actually recorded is
never second-guessed.
Applied in _report_partial_usage too. Cancelled and failed prints feed the
same slot_to_tray to the same resolver and mis-charged just as readily.
The resolved mapping and its source are now logged at print start and at
completion. "source: none" at start is the signal that completion will
have to fall back, and it was the one line that would have turned this
report into a five-minute triage.
Not addressed: editing the mapping after the fact, which the reporter also
asked for. ArchiveUpdate exposes neither filament field and there is no way
to re-run an attribution, so that is a feature rather than a fix.
|
||
|
maziggy
|
4335ee3814 |
Stop the drying badge inventing a temperature on a uniform AMS (#2759)
The follow-up to the same report: a second AMS 2 Pro, no aux power,
loaded entirely with PLA and drying at the 45C the reporter picked,
showed 45C and then switched to 55C.
Bambu never echoes back a cycle's filament or temperature, so both come
from the target cached when the command went out, and the fallback for
a missing cache reads the loaded trays. The first pass narrowed that
fallback to units whose spools agree on a filament, which fixed the
mixed-unit case in the original report but left the uniform case
answering with the spools' RFID-recommended drying_temp -- 55C here.
Agreement across slots is evidence of what is being dried, because the
dryer heats all of them. It is no evidence of the temperature, which is
picked freely in the popover, so the recommendation was never more than
a guess wearing the same confident "PLA @ 55C" as a known target.
uniform_tray_drying_hint therefore becomes uniform_tray_filament_hint
and returns the filament alone. The badge names a temperature only when
we sent it, and otherwise shows the filament and the countdown.
Both status builders also stopped filling the two fields independently.
Entering the fallback when either was missing let a cached filament pair
with a guessed temperature and render as though both were known; the
temperature now simply has no fallback to reach.
The badge required both fields before rendering anything, so dropping
the temperature would have blanked it rather than shortening it -- the
frontend now renders each on its own terms. No new translation key: the
filament type is a passthrough.
This changes what is shown when the cached target is missing, not why
it goes missing. If the reporter was on the fixed build, the falling-
edge gate is still letting a zero through on an unpowered unit, which
needs a log covering the start of the cycle.
|
||
|
maziggy
|
3a3d445db6 |
brace-expansion override ^5.0.8 -> ^5.0.9 (GHSA-rgw5-rvv9-x895, DoS).
5.0.8's maxLength cap was applied in combine(), where output is merged, but
not to the two arrays built before it runs: comma alternatives each got their
own full allowance and were concatenated with no running total, and padded
sequences never consulted maxLength at all. So a ~25 KB pattern still OOMs the
process -- fatally, past the reach of try/catch -- and a ~400 KB one blocks the
event loop for over two minutes. 5.0.9 bounds both as they are built.
Dev-only and transitive here: it reaches us as eslint -> minimatch@5 ->
brace-expansion, the only input it sees is our own lint globs, and it is not in
the shipped bundle. The ci.yml audit gate runs --omit=dev, so this never would
have failed CI; it surfaced through Dependabot.
The overrides floor is bumped alongside the lockfile so a clean install can't
resolve back to the vulnerable 5.0.8.
|
||
|
maziggy
|
8e004c3b83 | Improve local slicer selection with code review suggestions, adding fail toast and handle (legacy) read permissions | ||
|
maziggy
|
2953b2bf81 |
fix(backup): tell a restore apart from a backup in Backup History (#2656)
A restore writes a `github_backup_logs` row too — same table, same status
values, and it already carried `trigger: 'restore'`, which the API already
returned. The history table rendered date / status / commit only, so the row
read as a successful backup dated now while "Last backup" said something
else: `last_backup_at` is only stamped by an actual backup, and the two
disagreeing is alarming with nothing on screen to explain it.
Adds the Type column the trigger was always there to fill. Unknown values
fall back to the raw string rather than rendering blank, matching the
`backup.pathCheck.*` lookup a few hundred lines up — a trigger kind added
later shows up as itself instead of vanishing.
Backend unchanged: it has recorded this correctly since the restore path was
written.
Three tests, all three failing without the column. 13 locales in parity at
5776 leaves — pt-BR takes "Backup manual" rather than the parenthesised form
because "Backup (manual)" is identical to en, which the parity check counts
as untranslated.
Bundle rebuilt: `index-DhOfNgMz.js` → `index-CadgB7UN.js`. It also picks up
the `archivesOwnerUnknown` leaves from the previous commit, which changed
i18n without rebuilding.
|
||
|
maziggy
|
0827ebc5b7 |
fix(backup): let the nozzle_id default apply, and prefer the live one (#2656)
`set_kprofiles_batch` defaults the field with
`p.get("nozzle_id", f"HS00-{diameter}")`, and a `dict.get` default only
fires when the key is *absent*. The restore always set the key —
`"nozzle_id": p.get("nozzle_id")` — so a backup that carries no nozzle_id
published `nozzle_id: null` to the printer instead. Printers that omit the
field (KProfilesView's own #1748 comments) are exactly why the default is
there, and it was unreachable from this path.
Set only when known, on the same precedence the `setting_id` line beside it
already uses: the live profile first, then the backup, then absent. Live
first is the point rather than a bonus — `nozzle_id` encodes the fitted
nozzle's type as well as its diameter (`HS00-` hardened vs `SS00-`
stainless), so a nozzle swapped since the backup makes the stored value
stale, and the write lands on the nozzle fitted now.
Read with `getattr`, matching the defensive read of `extruder_id` in
`_match_kprofile`: not every live profile carries every field, and
`test_a_live_index_that_reports_no_extruder_still_matches` is the standing
control for that.
Four tests, two of which fail without the fix. The `_live` double also
gained `nozzle_id` — it is a non-default field on the real `KProfile`
dataclass, so omitting it let the double license code the real object
would have accepted.
|
||
|
maziggy
|
03b2ce5c16 |
fix(backup): say when a restored archive lands without an owner (#2656)
`created_by_id` is not attribution, it is the column the access check runs
on: `_ensure_archive_visible` fails closed on NULL, so an ownerless archive
is a 404 for every caller without `archives:read_all` and never appears in
the ownership-scoped list queries.
On the overwrite path an absent key correctly leaves the local owner alone
— that rule is deliberate and unchanged. On the insert path there is no
local row to fall back on, so the archive lands ownerless, and nothing said
so. The restore reported N archives restored while the user who asked for
them saw none. Two ways in, both silent: a commit taken before the
collector recorded the column (every pre-#2656 backup), and an archive that
genuinely had no owner on the source instance.
Adds `archivesOwnerUnknown`, emitted on insert only, and suppressed when
the stale-id branch has already spoken for that row so one cause does not
produce two notes. Wording mirrors `archivesOwnerCleared` because the
consequence and the remedy are the same; the cause is not, so it is a
separate code rather than a reuse.
Five tests, plus the existing `test_a_backup_without_the_key_still_restores`
renamed and tightened — it asserted the silence this fixes. 13 locales back
in parity at 5772 leaves. No modal change: notes render through
`translateCoded`, which resolves by code.
|
||
|
maziggy
|
a83645c859 |
fix(backup): dedupe spools and usage history on a comparison that can match (#2656)
Both `created_at` columns the restore dedupes on are
`server_default=func.now()`. SQLite fills those from `CURRENT_TIMESTAMP`,
which has second precision and stores `'2026-08-02 11:28:41'`, while
SQLAlchemy binds a Python datetime as `'2026-08-02 11:28:41.000000'`.
SQLite compares the two as strings, so `Model.created_at == created_at`
never matched a row the application itself created — not even when handed
that row's own value straight back out of the ORM.
Every dedupe keyed on it therefore missed, on the ordinary case rather
than an edge one:
* `_find_spool`'s composite fallback duplicated every tag-less spool on
each restore, and `overwrite_existing=True` never reached the original;
* the usage-history dedupe re-inserted the user's entire consumption
history on each restore.
Rows the restore itself had inserted did match, because those carry an
explicit bind in the same microsecond format — which is why the existing
repeat-restore tests passed throughout.
Fixed by filtering the candidates in SQL and comparing `created_at` in
Python, which sidesteps the bind format and behaves identically on
PostgreSQL, where the column keeps microseconds and the SQL comparison
happened to work. `_parse_dt` now also normalises an offset-bearing value
to naive UTC, matching what the naive columns actually hold; the collector
never writes one, so that guards hand-edited and foreign backups.
Seven tests, six of which fail without the fix. They seed the "existing"
row the way the application does — no explicit `created_at` — which is
what the existing coverage was missing.
|
||
|
maziggy
|
accafafa64 |
fix(backup): report the rows a failed K-profile step already committed (#2656)
`_apply` commits the database categories before the K-profile phase, and the
comment there is right about why: `get_kprofiles` is 3 x 5 s per printer per
nozzle and SQLite's `busy_timeout` is 15 s, so holding the writer across the
MQTT phase would fail every concurrent writer in the app.
But `run_restore`'s handler returns `{"success": False, ..., "results": {}}`
for anything raised after that point, and the per-call guards inside
`_restore_kprofiles` do not cover the whole phase. Two consequences, and the
second is worse:
* The user is told the restore failed and handed an empty `results` while the
archive, spool and settings rows are durable on disk. The honest-reporting
theme this whole feature is built on inverted on exactly the path where it
matters most.
* `_reconfigure_mqtt_relay` sits inside the same `try`, downstream of the
raise. A restore that rewrote the mqtt_* rows left the relay pointed at the
pre-restore broker until something else reconfigured it.
`_apply` now contains the K-profile phase: fold the error into that category's
tally as `failed` plus a `kprofilesStepFailed` note, and let the results it has
already committed be returned and reported. Every profile the payload carried
and the phase did not account for is counted failed — silence would have been
the same lie in a smaller font. `_reconfigure_mqtt_relay` is reached again
because `_apply` returns normally. The rollback in the handler discards only
the phase's own read transaction, so a database error cannot leave the session
in a state that turns the caller's commit into the very report this prevents.
`kprofilesSendFailed` was the obvious note to reuse and is the wrong one: it
names a nozzle, a printer and a serial that a phase-level failure does not
have, and "failed to send" is untrue of a step that never got as far as
sending. One new leaf x 13 locales instead.
Belt-and-braces on the trigger that found this:
`sum(len(c.get("profiles") or []) ...)` raises TypeError on a hand-edited or
truncated backup whose `profiles` is not a list, and it runs before the guards.
Counting defensively makes that a skipped category rather than an exception
thrown over committed rows.
Control kept explicit: a failure *before* the commit still rolls back, still
reports nothing restored, and still does not touch the relay.
Tests: +5 (280 -> 285 across the three restore files, 328 -> 337 across
`-k github`). Fail-pre-fix 4 — 3 for the containment, 1 for the defensive
count, checked separately. i18n parity 13 locales at 5771 leaves.
Bundle rebuilt for the new leaf: index-CHCEEMgx.js -> index-DhOfNgMz.js. CSS
hash unchanged.
|
||
|
maziggy
|
1f07f39481 |
fix(backup): stop Gitea's tree pager failing open on a missing total_count (#2656)
`if not isinstance(total, int) or seen >= total or not entries: return blobs, ""`
— the first arm short-circuited the page loop into a **success** holding page 1
only. Gitea clamps `per_page` to `MAX_RESPONSE_ITEMS` (default 50), so that is
50 entries of an arbitrarily large tree returned as a complete listing.
The restore then reports genuinely-present categories as "Not present in this
backup commit". That silent skip is the exact failure this override exists to
prevent, and the same class as E7 and G2 — G2 fixed the arithmetic here and
left the shape. GitHub and GitLab both hard-fail in the equivalent spot; only
Gitea guessed, and it guessed in the one direction that loses data quietly.
Whether Gitea always sends `total_count` on this route is beside the point: the
code was defending against a response shape it did not trust, and then trusting
it.
Now a missing or non-int `total_count` means "page until a short or empty
page". A page shorter than the first one is the last one, floored at Gitea's
default clamp so a genuinely small tree still costs exactly one request — the
reason G2 rejected paging-until-short in the `total_count`-present case, which
is unchanged and still stops on the count. The existing `page <= 50` ceiling
gives the correct hard failure for a tree that really is over cap, so this
cannot truncate.
Residual, and deliberately not widened into a `return None` on the first
ambiguous response — that would break single-page trees, the common case: an
instance whose `MAX_RESPONSE_ITEMS` is set *below* 50 *and* which omits
`total_count` would still stop at page 1. Both halves have to be true.
Tests: +6 (paged to the end with no count, on both Gitea and Forgejo; a short
page ends it; a non-int count is treated as no count; the page ceiling still
fails). Fail-pre-fix 5, control that passes either way 1 (a small tree is one
request). These don't match `-k github`, so 274 -> 280 across the three restore
files but `-k github` is unmoved.
|
||
|
maziggy
|
214df517ac |
fix(backup): match a K-profile on its own extruder, not just its filament (#2656)
`_match_kprofile` scoped candidates by `filament_id` alone, and
`_current_kprofile_index` reads the live index per nozzle *diameter* — so on a
dual-nozzle printer both extruders' profiles come back in one list.
On an H2D with the same filament calibrated on both extruders, the `setting_id`
arm then matched whichever profile the printer happened to list first. A
backed-up extruder-0 entry took extruder 1's slot and went into the batch as
`{extruder_id: 0, cali_idx: <extruder-1 slot>}`, writing one extruder's
calibration over the other's and counting it restored. With an entry per
extruder — the ordinary case, since the same preset on both nozzles is what a
dual-nozzle printer is for — the two swapped slots and clobbered each other.
The `name` arm and the single-candidate fallback were equally unscoped, so this
was never only about the ambiguous case.
The data was already in hand and already being read: the backup entry carries
`extruder_id` (`:1546` copies it straight into the outgoing dict) and
`KProfile` has carried `extruder_id: int` on the live side all along. Scope
`candidates` by it the same way `filament_id` already scopes them, and the
single-candidate fallback narrows with them — ambiguity is judged within one
extruder now.
Conditional on both sides saying which extruder they mean. A pre-#2656 backup
has no `extruder_id`, and a live index that reports none must not turn every
entry into an add — that would be a far worse regression than the bug. Two
controls cover each direction of that.
`claimed` (G3) is untouched: it is per nozzle-loop and this only narrows the
candidate set feeding it.
Tests: +4 across the three restore files (270 -> 274). Fail-pre-fix 2 (each
extruder keeps its own slot; the other extruder's profile is not a stand-in),
controls that pass either way 2.
|
||
|
maziggy
|
a2faa6accc |
fix(backup): drop the settings-pin workaround, upstream fixed the cause (#2656)
This modal carried two workarounds for #2716: `onSuccess` deliberately did not invalidate `['settings']`, and a query-cache subscription pinned the entry to the pre-restore copy for as long as the result panel was up. Both existed because SettingsPage's debounced auto-save diffed its `localSettings` form state against the live cache, so any refetch of a restored settings row -- this modal's, a window refocus, a reconnect, or any of the ~30 other observers of the key -- read as an edit and PATCHed the pre-restore values back over the restore about 500 ms later. `43cb216a` on dev fixed that. The page now keeps a server baseline and reconciles a moved snapshot field by field: an untouched field adopts the server's value instead of overwriting it. The restore no longer needs an exception, and maziggy explicitly invited dropping it. A commit on top rather than a rebase-drop of `21bb5afc`: later commits touch this file, and the workaround was right when it was written. This says so. The reload on close stays -- it was never one of the two workarounds. Its stated reason was, though, and it was the #2716 bug, so it is restated for what it actually buys: invalidating `['settings']` only resyncs what reads that query, and the interface language, currency and auth toggles are read on boot. Tests: "never invalidates the settings query" inverts; the pin test and its control go with the pin. The reload pair stays. 28 -> 26 tests in this file. |
||
|
maziggy
|
4599b6f4b4 |
fix(backup): read the printer's verdict before counting a K-profile restored (#2656)
`18938a10` on `dev` changed `set_kprofiles_batch` from returning a `bool` to
returning the sequence_id it published the command under, and moved the
verdict to a separate `await client.await_cali_ack(seq)` returning
`(ok, detail)`. Every caller in `api/routes/kprofiles.py` was updated with it.
`_restore_kprofiles` was not — it still did `sent = client.set_kprofiles_batch(...)`
and branched on `if sent:`.
A sequence_id string is truthy, so that compiled, passed, and silently made
the restore the one path left in the codebase that reports a refused
K-profile write as saved — exactly the defect `18938a10` closed everywhere
else.
Keep the sequence_id, await the ack per batch, and route an explicit refusal
into `tally.failed` with a new `kprofilesRefused` note carrying the printer's
own `reason`. Reusing `kprofilesSendFailed` would have been wrong: the
command was sent, and the printer answered.
Silence still counts restored. That is `await_cali_ack`'s own contract and
the maintainer's rule — no answer is not evidence of refusal, and firmware
predating the ack never answers. An exception reading the ack degrades the
same way rather than inventing a failure out of a write that most likely
landed.
`kprofilesAckUnreliable` is reworded to match: a refusal is now believed, so
the caveat narrows to what is genuinely left uncertain. The ack is only worth
reading at all because `18938a10` also changed the payload's `tray_id` from
`-1` to `0` — single-nozzle firmware answered `result: "fail"` to `-1` on
writes that demonstrably applied. This restore builds no `tray_id` of its
own, so it inherits that fix for free.
Tests: 4 regression (the ack is awaited for the returned sequence_id; a
refused batch counts failed and surfaces the printer's reason; one refused
nozzle does not condemn the other; the reworded caveat) + 3 controls (a
silent printer still counts restored; an unreadable ack does not fail the
batch; `None` keeps the existing send-failed path and awaits nothing). All
four confirmed failing against the pre-fix service.
|
||
|
maziggy
|
c9ce6dac3c |
fix(backup): disclose the K-profile exception before the restore, not after (#2656)
With overwrite off, the confirmation said "Missing entries are added; existing
entries stay as they are." For archives, spools and settings that is true --
_apply threads the flag into all three. _restore_kprofiles takes no overwrite
parameter at all, and deliberately: writing a slot is always an overwrite on the
printer, so it resolves the live cali_idx and publishes extrusion_cali_set
either way, replacing whatever calibration that slot currently holds.
The behaviour is right and the backend does say so, but it says so as a
kprofilesAlwaysOverwrite note -- which only reaches the user in the result panel,
after an MQTT send that cannot be taken back. The one screen that explains
overwrite-off stated the opposite. So the fix is on the frontend, where the
mismatch is.
One new leaf, kprofilesOverwriteCaveat, in all 13 locales, rendered in two
places: appended to the overwrite-off confirmation when kprofiles is among the
selected categories, and beside the K-profiles row itself as soon as it is
ticked, which is the same screen as the toggle whose promise it qualifies.
Neither appears with overwrite on, where nothing is promising otherwise.
Tests: 2 that fail pre-fix (the caveat beside the row, and inside the
confirmation the user clicks through) and 2 controls (a spools-only restore keeps
the plain message; overwrite-on keeps the strong one and adds nothing). Frontend
suite 2601 -> 2605 tests across 195 files.
|
||
|
maziggy
|
29311cab2b |
fix(backup): refuse prometheus_enabled when the backup has no token either (#2656)
The companion-credential rule has five conditions, and the second one -- "the
backup itself carried a usable credential" -- was applied to all five pairs. It
should not be. It is what stops the rule over-refusing an anonymous MQTT broker
or an anonymous LDAP bind, both of which are working configs: there, an empty
credential in the backup means the restore is not producing anything weaker
than what was backed up.
For prometheus_enabled it does not transfer. An empty prometheus_token removes
/api/v1/metrics' only gate, so the exposure is a property of the toggle, not of
a downgrade relative to the backup -- and prometheus_token is optional, so a
backup taken on an instance that enabled Prometheus without ever setting one
carries the toggle and no usable token. That payload skipped the refusal
entirely: not a candidate, so the local-state pass never ran, and the blocklist
quietly dropped the token key. On a token-less target the result was
prometheus_enabled=true, no token row anywhere, and a full unauthenticated
metrics dump -- the same hole the rule was written to close, reached from the
likelier of the two directions.
So condition 2 is now per-pair: an exposure class (prometheus) that skips it and
is judged on local state alone, and an availability class (mqtt, ldap, ha,
virtual_printer) that keeps it. Nothing else changes -- the local-state pass
already stands down when the instance has its own credential, when HA_TOKEN is
in the environment, and when the toggle is already on locally, so "the exposure
pre-dates this restore" still holds and refusals still get no tally increment.
One wording consequence: an exposure toggle can now be refused on a payload with
no credential-like key in it at all, where the shared caveat would have read "0
credential-like key(s) will be skipped". That case gets its own preview detail
code, settingsCompanionOnlyWillSkip, in all 13 locales.
Tests: 6 that fail pre-fix -- the token key absent and blank at the unit level,
the new preview wording, and the integration test through the real endpoint for
both payloads (200 with a full metrics body before this, 404 after). Plus 3
controls, because over-refusal is still the real risk: the exposure route must
still stand down for a local token and for an already-on toggle, and the
availability class must still let a credential-less mqtt/ldap/virtual_printer
toggle through. The anonymous-broker and anonymous-bind controls are unchanged
and still pass.
|
||
|
maziggy
|
10c9e53e84 |
docs(backup): give the real reason cloud profiles are not restorable (#2656)
Both docstrings said the collector never writes `cloud_profiles/*.json`. That
was true when they were written and stopped being true at `455a9e4b` (#2717,
"collect cloud profiles from every connected account"), which is this
branch's rebase base — so the PR was shipping a stated reason its own base
had invalidated.
The real reason is the one the PR body now gives: restoring a preset means
writing to a Bambu or Orca Cloud account, which is a different operation from
every other category here. Those land in the local database, or on a printer
the instance already owns.
Checked that nothing in the restore path is confused by the new files — the
category globs don't reach `cloud_profiles/`, and it stays out of
`RestoreCategory`.
|
||
|
maziggy
|
06038ecb15 |
fix(backup): require settings:update to restore the settings category (#2656)
The restore endpoint was gated on `github:restore` alone, and the settings
category rewrites arbitrary non-auth `Settings` rows. Backup and Settings are
separate permission groups, so a role holding only Backup could change
settings it cannot change through `PUT /api/v1/settings/`, the endpoint that
owns them.
The inconsistency is ours rather than an inference: this module already makes
exactly this argument — it is why the four protected auth keys are refused
outright — and `library.py` sets the precedent of elevating a route to
`settings:update` for the same reason.
Gated per-category rather than by demoting `github:restore` wholesale, so it
stays narrow and doesn't presume the answer for `spools:*`, `archives:*` and
`kprofiles`. That broader permission-model question goes to the maintainer in
the PR reply.
`current_user is None` only means auth is disabled — `github:restore` is in
`_APIKEY_DENIED_PERMISSIONS`, so an API key never reaches the route body.
No frontend change: `request()` puts the 403's `detail` on the Error, and the
modal already renders `restoreMutation.error.message` in its red block, so
the user sees the missing permission named.
Tests: 1 regression (a Backup-only role gets 403 and `run_restore` is never
awaited) + 3 controls (the same role still restores the other three
categories; a role holding both permissions still restores settings; auth
disabled is unaffected). The regression confirmed failing against the pre-fix
route. `_create_config` gained an optional token so it works under auth.
|
||
|
maziggy
|
5772523003 |
fix(backup): stop two backed-up K-profiles claiming one live slot (#2656)
`_match_kprofile` ends in a single-candidate fallback, and the per-nozzle
loop called it once per entry with no record of which live profiles were
already taken. Two backup entries sharing a `filament_id` and matching on
neither `setting_id` nor `name` both resolved to the same live profile, both
got the same `cali_idx`, and both went into the batch — so the second
overwrote the first on the printer while the tally counted two restored.
Reachable in the ordinary way: the user deletes one of a pair after the
backup, and the delete-then-add re-key this code already reasons about is
exactly what strips the `setting_id` match.
Fix: thread a `claimed` set of slot ids through the loop; a live profile can
only stand in for one entry. A displaced entry falls through to
`cali_idx: -1` — add-as-new is the safe outcome — and folds into the
existing `kprofilesUnmatched` note rather than earning a new code.
The single-candidate fallback is still judged against every candidate rather
than the unclaimed ones. Two live profiles for one filament are ambiguous
whether or not another entry has taken one, and narrowing to "available"
would turn a guess the code deliberately refuses into a match.
Tests: 2 regression (the displaced entry is added rather than aliased, and
keeps its own setting_id) + 2 controls (two genuine matches keep their own
slots; a claimed slot does not make an ambiguous pair matchable). Both
regressions confirmed failing against the pre-fix service.
|
||
|
maziggy
|
2d56ac9215 |
fix(backup): page Gitea's tree off what came back, not what we asked for (#2656)
The pager computed `seen = (page - 1) * 1000 + len(entries)`, taking the
requested `per_page` as fact. Gitea clamps `per_page` to
`MAX_RESPONSE_ITEMS`, which defaults to 50. So on a default install page 1
returns 50 entries and sets `seen` to 50, then page 2 sets it to 1050 —
which clears any `total_count` under 1050. The loop returns `success: true`
holding the first 100 entries of a much larger tree.
The restore then reads every missing path as "category not present in this
commit" and skips it silently, which is precisely the failure this override
was written to prevent. Same class as the GitLab pager fix, in the one
direction that got left behind.
Fix: accumulate `seen += len(entries)`. A genuinely over-cap tree still
hard-fails rather than truncating; the cap is a page count, not a file
count, because the page size is the server's choice.
Test: a 120-entry tree served 50 at a time reaches its last entry, in three
requests. Confirmed failing against the pre-fix backend — it stopped after
two pages and reported 100 entries as the whole tree.
|
||
|
maziggy
|
5015b02760 |
fix(backup): don't let an old backup commit blank an archive's owner (#2656)
`created_by_id` and `deleted_at` both went into the archive `fields` dict
unconditionally, via `entry.get(...)`. A backup commit taken before the
collector wrote those keys carries neither, so `.get` yielded None for both
and the overwrite branch — a blanket `setattr` over every key — wrote NULL
over a live owner.
That is exactly the failure carrying `created_by_id` was added to fix, only
now inflicted on rows that were fine: `_ensure_archive_visible` fails closed
on a NULL owner, so the archive 404s for the person who owns it. It emitted
no note either, because `archivesOwnerCleared` only fires for an id that
isn't in `valid_users`, not for an absent key — and the row still counted as
restored. `deleted_at` had the mirror problem: an old commit silently
un-deleted, since `archivesUndeleted` reads the same absent value.
Absent is not the same as explicitly null. Both keys now only enter `fields`
when the entry actually carries them, so an old commit leaves the column
alone on overwrite and a current one can still say "this archive has no
owner" or "this archive is live". Same shape as the tag-column rule: don't
clear what the backup doesn't know about.
Behaviour change to an existing test, called out deliberately:
`test_overwrite_undeletes_a_locally_deleted_archive_and_says_so` now has to
put `deleted_at: None` in the entry to mean it.
Tests: 2 regression (owner and deleted_at both left alone by a key-less
entry) + 2 controls (an explicit null is still honoured, with its note).
Both regressions confirmed failing against the pre-fix service.
|
||
|
maziggy
|
be17b62480 |
fix(backup): halve the provider round-trips, and stop losing commit metadata (#2656)
The three remaining review items, all in the read path.
E1 — four provider calls where two would do. preview() called list_commits
twice: once inside _resolve_ref to turn HEAD into a SHA, once more at limit=20
purely to find the entry describing that same SHA. And list_tree's recursive
tree GET was thrown away, so fetch_files immediately fetched the identical tree
again to map path -> blob SHA. _resolve_ref now returns the entry it already
has, and list_tree returns its blob_shas map for fetch_files to take as an
optional argument. GitLab reads files by path and ignores it.
E2 — `commit: null` for a ref outside the 20 most recent. Two causes, and the
second is the one that actually bit: REF_PATTERN accepts a 7-character ref while
providers return the full 40, so the exact `==` in the scan never matched an
abbreviated SHA *even when the commit was in the window*. Fixed by prefix
comparison, plus a get_commit(ref) on the GitHub and GitLab backends for the
genuinely-outside-the-window case. Gitea and Forgejo inherit GitHub's. Still
best-effort: it is a subject line and a date, so a failed lookup renders the
preview without them rather than failing it.
E7 — the two tree readers disagreed, and each was wrong in the other's
direction. GitHub's recursive trees endpoint is not paginated and signals
overflow with truncated=true, which _blob_shas_at hard-fails on. Gitea and
Forgejo *do* page that endpoint, and inherited that single GET unchanged — so a
large backup repo returned only the first page and every category beyond it
looked absent from the commit. GiteaBackend now has its own paging
_blob_shas_at. GitLab had the mirror-image bug the review did not name: at its
50-page cap it exited through the while condition and returned success: True
with a silently partial path list. Both now fail loudly, which is what the
GitHub version was always doing.
Both halves of E7 are the same failure the module already refuses to allow: a
restore that skips categories and calls it "not present in this backup commit".
24 new or changed tests, all failing against this commit's parent.
|
||
|
maziggy
|
812a70f326 |
fix(backup): stop overwrite writing a spool's other tag key (#2656)
Four of the review's smaller items.
E6, the substantive one. tag_uid and tray_uuid are both in the overwrite setattr
loop, so a spool matched on one key got the backup's *other* key written onto it.
Neither column has a unique constraint (models/spool.py, and no unique index in
the migrations), so nothing errors — a duplicate tag simply appears, after which
_find_spool's .scalars().first() is non-deterministic and an AMS tag lookup
resolves to an arbitrary one of the two spools. The same loop could also clear a
tag the user had scanned since the backup was taken, when the backup entry held
None.
_find_spool now reports which key matched, and _guard_tag_overwrite drops a tag
column from the write when the incoming value is empty and the local row has one
(the backup predates the scan, so the local tag is the newer fact) or when
another local spool already holds it. Announced in the tally the way the archive
un-delete case already announces itself, rather than done silently — the
spoolTagKept locale key landed with the rest of the i18n block last commit.
E5. The Restore button is hidden without github:restore. All three endpoints are
gated on it server-side, so the modal 403s on its first preview; offering the
button is offering an action that cannot work. Button only — the card stays
visible, since configuring backups is a separate permission — and hasPermission
returns true with auth off, so a single-user instance is unaffected.
E3. models/github_backup.py: the trigger comment said manual/scheduled; this PR
added a third value.
E4. ha_token_from_env: recommending no change, with the reasoning recorded as a
test rather than left in a review thread. It is built only in the settings GET
response, is absent from AppSettingsUpdate, and is therefore never a Settings
row — it cannot reach a backup, so an allowlist entry would be dead code. Worse,
a name-shaped exception to a belt-and-braces denylist is a live hole: an
attacker-authored settings/app_settings.json could get a *token*-named row
written by choosing that name.
4 unit tests and 1 frontend test that fail against this commit's parent, plus 4
controls: a free tag is still written, an unchanged tag is not reported as kept,
an insert is unaffected, and the button still shows with auth disabled.
|
||
|
maziggy
|
7d2cf440b1 |
i18n(backup): make the restore notes and preview details translatable (#2656)
A German user got a translated modal with "Not present in this backup commit" in
the middle of it. Every tally note and preview caveat was a server-built English
sentence rendered verbatim.
Follows the backup.pathCheck contract already in use one card down in the same
component, deliberately rather than inventing a second convention: the server
sends a `code` plus typed `params` and carries the English along as the
fallback, and the client renders
`t(`...${code}`, { ...params, defaultValue: message })`. The defaultValue arm is
what keeps a newer backend's unfamiliar code readable instead of printing the
raw key — covered by its own test.
Shapes:
- notes: list[str] -> list[GitHubRestoreNote] {code, params, message}. Breaking,
but the field is unreleased in this same PR.
- GitHubRestorePreviewCategory gains detail_code / detail_params; `detail` stays
as the English fallback.
- _CategoryTally.note(code, message, **params), deduped on (code, params) rather
than on the rendered text, so two offline printers both keep their names. The
20-note cap is unchanged.
28 new leaves across 13 locales: 20 notes.* and 8 details.*. `noData` collapses
the four per-category "No X data in this backup" strings into one, since the
category heading already renders beside it. Counts use single-form {{count}} in
the existing "N record(s)" style rather than i18next plural suffixes — nothing in
this block uses _one/_other and the parity script has extra rules for them.
Parity holds at 5737 leaves in all 13 locales.
Deliberately out of scope, and worth saying so rather than leaving it to look
like an oversight: result.message, the commit-picker subject lines and the HTTP
error strings stay English. Those also originate in the provider backends, so
code-ifying them widens the diff well past the restore service.
spoolTagKept is added here with the rest of the locale churn but is not emitted
until the next commit, so the 13-locale change lands once.
|
||
|
maziggy
|
2e7ff24a1d |
i18n(backup): rename the Git restore button to "Restore from Git" (#2656)
Two buttons labelled "Restore" were visible in the same viewport — this card's
and the Local Backup card's — with different destinations. The modal title
disambiguated them; the buttons did not.
backup.restoreFromGit.button only. The Local Backup card's t('backup.restore') is
unchanged, and each locale's new value follows the wording that locale already
uses in the modal title rather than being a literal translation of the English.
One correction to the review's aside: this does not simplify
GitHubRestoreModal.test.tsx. The /Restore$/ + confirmButtons[length - 1] idiom
there is not caused by the card button — the modal is rendered standalone in
those tests — but by the modal's own footer action and its confirm dialog both
reading t('backup.restore'). Left alone as out of scope.
|
||
|
maziggy
|
946d6457c2 |
fix(backup): carry the owner across, or restored archives are invisible (#2656)
Neither _collect_archives nor _restore_archives touched created_by_id, so every
restored archive row landed NULL. That column is not attribution, it is what the
access check runs on: _ensure_archive_visible (api/routes/archives.py) fails
closed on NULL — a 404 for any caller without archives:read_all — and the list
paths filter created_by_id == user.id. On a multi-user instance the tally
therefore reported archives restored while the person who owns them could
neither list nor open them.
Same shape as the deleted_at fix, and the same remedy: the collector records the
key next to deleted_at, the restore mirrors the printer_id/project_id pattern
exactly — one hoisted select(User.id), a membership test per row, an unknown id
coerced to None rather than failing the row, and one de-duplicated note. It is in
the overwrite setattr loop too, so overwrite keeps meaning "make local match the
backup". Additive on the backup side, so older backups still restore; they just
cannot know the owner.
Clearing the id is not silent-safe, so the note says what it costs: those
archives are visible only to users with archives:read_all until an admin
reassigns them.
Caveat recorded in a comment and raised in the PR, not decided here: this is the
one place the module reuses a raw backup id, against its own rule. Validating it
means a *stale* id clears rather than pointing somewhere wrong, but a live id
belonging to a different person on a different instance would still collide.
Collecting username and resolving on that would close it.
6 unit tests and 1 integration test that all fail against the parent commit,
plus 2 controls that pass either way — a backup with no created_by_id key still
restores, and a second operator still gets a 404.
|
||
|
maziggy
|
de8a86c40f |
fix(backup): never restore a toggle whose credential can't come with it (#2656)
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.
|
||
|
maziggy
|
049a638679 |
y fix(backup): don't count a stale selection, and say when archive links are dropped (#2656)
Two smaller restore-path fixes from the same review.
The modal's footer counted `selected` raw while the checkboxes rendered
`selected && isAvailable`. Switching commits keeps `selected` on purpose (it is
only pruned once the new preview lands), so for as long as the new commit's
preview was in flight — with the category list replaced by its spinner — the
footer still read "2 selected" over an enabled Restore button, and clicking it
restored the newly-picked commit with the previous commit's categories, none of
which the user had seen an item count for. The count and the POST body now come
from one `selectedCategories` memo gated on availability, exactly as the
checkboxes are, so both go empty until the preview lands.
Restoring Spool inventory without Print archives leaves archive_id_map empty,
so every usage -> archive link is nulled even where the archive exists locally.
It can't be resolved here (the archives payload isn't fetched for a category
that wasn't selected) and a later archives-only restore won't repair it either,
since the usage dedupe key doesn't include archive_id and those rows read as
already-present. So it gets a note naming the remedy while the user can still
redo the run with both categories ticked.
|
||
|
maziggy
|
e0cfa90be7 |
fix(backup): reconnect the MQTT relay after restoring mqtt_* settings (#2656)
The relay reads its broker config once, when configure() is called — which is
why the settings PUT handler reconfigures it after writing those rows
(api/routes/settings.py:246). The restore wrote the rows and stopped there, so
the relay stayed on the pre-restore broker until the next backend restart while
the UI showed the restored values: the one way a settings restore could look
applied without being applied.
_restore_settings now reports the keys it actually wrote, and run_restore
reconfigures the relay from the committed rows when any of them is an mqtt_ one.
Three details worth keeping:
* it runs after the commit, because configure() drops the connection and
rebuilds it — not something to do on values a later failure could roll back;
* it is keyed on written, not merely present: a key skipped for overwrite=off
or by the credential blocklist must not trigger a reconnect;
* mqtt_password is never restorable, so configure() gets the row already in the
database and an unchanged broker keeps working.
A broker that refuses the new config is noted on the settings tally ("restart
Bambuddy") rather than failing the restore, matching the PUT handler's
best-effort handling of the same call.
|
||
|
maziggy
|
92de15ab8e |
fix(backup): release the SQLite writer before the K-profile MQTT phase (#2656)
_apply ran archives and spools first, which autoflushes their INSERTs and so
opens SQLite's single write transaction, then called _restore_kprofiles —
which awaits get_kprofiles per printer per nozzle at timeout=5.0 with
max_retries=3, i.e. up to ~15 s each against a printer that ignores the
request. The commit only came afterwards, in run_restore. busy_timeout is 15 s
(core/database.py:21), so a restore covering a couple of unresponsive printers
held the writer past it and unrelated writes elsewhere in the app failed with
"database is locked".
Commit the database categories before the MQTT phase starts. The K-profile
work is not in that transaction anyway — it leaves over MQTT — so the only
thing lost is rolling those categories back when a K-profile send fails, and
that rollback was never the right behaviour: extrusion_cali_set has already
reached the printer by then, so undoing the database half would just make the
two disagree.
|
||
|
maziggy
|
5fd9cbd744 |
fix(backup): never restore the auth policy settings from a backup (#2656)
_collect_settings exports every Settings row minus two credential keys, so
auth_enabled / advanced_auth_enabled / local_login_enabled / setup_completed
all travel in a backup, and none of them are credential-shaped enough for
_SECRET_KEY_HINTS to catch. Writing them back was the one part of a settings
restore that changed who can reach the instance rather than how it behaves:
* auth_enabled=false — from any backup taken before auth was turned on —
disabled authentication. core.auth caches only the enabled=True result, on
a 30 s TTL, precisely so staleness fails closed; set_auth_enabled pairs its
write with invalidate_auth_enabled_cache(). The restore did neither, so it
left the stored value the open one.
* local_login_enabled=false walked straight past the #1589 refusals in
update_settings (no enabled OIDC provider / no OIDC link on the caller),
which exist to stop exactly that lockout.
* /github-backup/restore is gated on GITHUB_RESTORE alone, so honouring these
keys made that permission a way to rewrite auth config without
SETTINGS_UPDATE.
Flipping an existing row needed overwrite_existing, so the odds were lower
than the severity. Both refusals now share _is_skipped_setting_key so the
preview's item count still matches what a restore writes, and the skipped
keys get their own note pointing at the auth UI rather than being folded in
with the credential ones.
|
||
|
maziggy
|
d06d134138 |
i18n(backup): add the Ukrainian strings for the restore modal (#2656)
The Ukrainian locale landed on dev in #2695, after this branch was cut, so uk.ts was missing the 23 restoreFromGit keys the rest of this branch adds to the other twelve. check-i18n-parity.mjs auto-discovers the locales directory, so it would have failed on the merge. All 13 locales are now at parity (5699 leaves each). Terminology follows what uk.ts already uses for the local-backup restore path — "Відновити" for restore, "резервну копію" for backup. |
||
|
maziggy
|
baa82f781a |
fix(backup): resolve K-profile cali_idx live instead of reusing the backup's (#2656)
Restoring K-profiles addressed extrusion_cali_set at the cali_idx recorded in
the backup. If that slot no longer existed on the printer the write was
silently dropped and the restore still reported the profile restored.
Not an edge case: Bambuddy's own K-profile editor is what re-keys the slot.
On a single-nozzle printer an edit is delete-then-add, so any edit between
backup and restore reproduces it.
Found testing on an X1E. Backup held cali_idx 8151; an edit through the UI
re-keyed the profile to 4606; the restore published cali_idx 8151, the printer
ignored it, and the tally read "1 restored" while the k-value stayed put.
Resending the identical payload with cali_idx 4606 applied, isolating the
stale index as the cause.
Fix mirrors the natural-key matching spools and archives already use, which
the module docstring already promised but scoped to spool.id and
print_archives.id. Before writing, read the live profiles for the nozzle and
match on filament_id + setting_id, falling back to filament_id + name, then to
the sole candidate for that filament. Send that profile's current cali_idx;
where nothing matches send -1 so the printer adds a new profile instead of
addressing a dead slot, and say so in the tally. A failed read degrades to
adding rather than aborting.
Also corrects the tally note. The printer does acknowledge extrusion_cali_set
-- it answers with a result/reason pair -- so "published without
acknowledgement" was false. It reports "fail" on writes that land, though, so
the note now says the acknowledgement is unreliable rather than absent.
Consuming result is left to a follow-up.
Re-verified on the same X1E: perturbed to k=0.061, restored from the commit
carrying the stale slot, payload went out with cali_idx 4606 and the printer
read back 0.027.
|