mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 19:21:33 +02:00
worktree-fix-2791-cursor-pointer
27
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 maziggy
|
b5163b94f8 |
fix(backup): report the categories a failed restore already committed (#2656)
The service reports what landed on a part-way failure -- categories commit as they finish, so results names the ones on disk -- and the modal gated the whole result panel on success, so it showed the failure message and dropped them. The cache invalidation was inside that same branch, which is the half that mattered: a run that committed the settings category and then failed left the app rendering pre-restore settings, with no reload and no re-read, which is the failure the modal's own reload-on-close exists to prevent. Gate on what was written instead. A refusal that never reached a category still carries an empty results and still keeps the form, so the mutex and backup-in-flight cases are unchanged. A partial does not read as a success: the tick becomes a warning and a line says the listed categories are the ones on disk. --- fix(backup): keep the local owner when the backup names one we cannot resolve (#2656) An owner the backup names but this instance has no user for was written as NULL, and overwrite is a blanket setattr -- so restoring over a local archive that had a perfectly good owner took it away, which is the 404-for-its-own- owner failure this column is carried across to fix. Resolving by username widened the trigger from a stale id to any user renamed since the backup. It is the same state as an absent key: the backup has not told us who owns this. So it takes the same action -- the column is not written at all. Overwrite keeps the local owner, insert lands ownerless with the note, and an explicit null still writes, so overwrite still means "match the backup". The notes move to the insert path with it. On overwrite nothing was taken away, so there is nothing to warn about, which is the rule the absent-key case already follows. |
||
|
jmoore-skild
|
f62e907e9f |
fix(backup): refuse the whole LDAP family on restore, not just its password (#2656)
A settings restore could substitute the instance'"'"'s authentication source. auth.py reads the LDAP config live from the settings table on every login, and none of ldap_server_url, ldap_user_filter, ldap_auto_provision or ldap_default_group is credential-shaped, so the secret-key hints never saw them and only the four auth-policy keys were protected. ldap_enabled was covered by the companion-credential rule instead, and that rule asks the wrong question. It judges availability - "will the integration still work?" - and an anonymous bind works, so a payload that simply OMITS ldap_bind_password skips the refusal and has its toggle written. Omitting the credential is exactly what an attacker authoring the file would do: they own the directory being pointed at, so they need no bind credential from us. Left unrefused, a backup repository anyone can write to yields admin: point ldap_server_url at your own directory, set ldap_auto_provision and ldap_default_group=Administrators, and the next login on a fresh username is provisioned into the admin group. Overwrite-off is enough on an instance that never configured LDAP - there are no rows to skip. Refused by prefix so a key added to the LDAP schema later is refused by default, and matched case-insensitively because the key comes from the backup JSON rather than from our own writer. ldap_enabled leaves _COMPANION_CREDENTIALS rather than sitting there as dead code, since _is_protected_setting_key runs first. The two tests asserting an anonymous bind was a false positive are inverted - they encoded the hole - and the refusal reuses the existing settingsAuthSkipped note, which already points at Settings > Authentication. |
||
|
jmoore-skild
|
f6fca4b927 |
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. |
||
|
jmoore-skild
|
fc2dcf76b5 |
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. |
||
|
jmoore-skild
|
8602c54c1f |
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. |
||
|
jmoore-skild
|
4ec0f3f9f2 |
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. |
||
|
jmoore-skild
|
06fa146e2f |
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.
|
||
|
jmoore-skild
|
a85fa66dcc |
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. |
||
|
jmoore-skild
|
329b506240 |
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. |
||
|
jmoore-skild
|
3bb087db54 |
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.
|
||
|
jmoore-skild
|
ae36d3ac13 |
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.
|
||
|
jmoore-skild
|
4ee9c0eecb |
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. |
||
|
jmoore-skild
|
bbd991510f |
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. |
||
|
jmoore-skild
|
733894aee6 |
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`. |
||
|
jmoore-skild
|
cb6a4e6d88 |
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. |
||
|
jmoore-skild
|
be8e8d545f |
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. |
||
|
jmoore-skild
|
158301ac8a |
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. |
||
|
jmoore-skild
|
945f50a1ab |
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. |
||
|
jmoore-skild
|
af8d14d796 |
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.
|
||
|
jmoore-skild
|
cfa82bfcfb |
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. |
||
|
jmoore-skild
|
ca93d4cf44 |
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. |
||
|
jmoore-skild
|
07244b6a43 |
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. Carries the rebuilt bundle (index-C2LOlVCR.js -> index-C16HJNOV.js). |
||
|
jmoore-skild
|
e7495dd41b |
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.
|
||
|
jmoore-skild
|
3ba89c60a1 |
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. |
||
|
jmoore-skild
|
737258202b |
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. |
||
|
jmoore-skild
|
cbe412607f |
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. |
||
|
jmoore-skild
|
6a239314dc |
feat(backup): restore selected categories from a Git backup commit (#2656)
The Git backup feature was push-only: there was no equivalent of the local backup's Restore button, so recovering meant hand-downloading JSON files from the repository. This adds the read side. Providers gain list_commits / list_tree / fetch_files on the GitProviderBackend ABC. GitHub implements them against the Git Data API and Gitea/Forgejo inherit that unchanged; GitLab overrides for its own REST shape, including tree pagination and subgroup path encoding. fetch_files is batched so the path -> blob SHA lookup happens once per restore rather than once per file, and uses the blobs API rather than contents because contents silently inlines only the first 1 MB. The new GitHubRestoreService resolves HEAD to a concrete SHA up front, so a preview and the restore that follows act on the same commit even if a scheduled backup lands in between. Categories are applied archives -> spools -> settings -> kprofiles: archives first because spool usage history references archive_id, K-profiles last because they leave the database and publish over MQTT. Restores never reuse the backup's primary keys. spool.id and print_archives.id are bare autoincrement columns, so ids from an old backup very likely belong to unrelated rows today; rows are matched on natural keys (tag_uid, then tray_uuid, then a descriptive composite for spools; content_hash or filename plus started_at for archives), inserted without an explicit id, and an old_id -> new_id map rewrites the foreign keys in spool usage history. created_at is carried across on insert so restoring the same backup twice matches instead of duplicating. Dangling printer/project links are cleared and reported rather than failing the row. Settings restore re-applies the collector's credential denylist on the read side, plus a pattern guard, because a backup taken before that denylist existed can still contain secrets. Restored archives are metadata-only: the 3MF and thumbnail bytes are not in a Git backup and print_archives.file_path is NOT NULL, so inserted rows get an empty path and the UI says so. Backup and restore take a mutex against each other; both write the same tables and talk to the same printers. Restores are logged as GitHubBackupLog rows with trigger="restore", which needs no migration and surfaces them in the existing History card. Cloud profiles are deliberately not a restore category. The collector never actually writes cloud_profiles/*.json - it reads a "setting" list key the Bambu Cloud API does not return - and the preset list it would write carries no setting payload. Filed separately. Permission github:restore already existed and is granted to Administrators, so no permission changes were needed. Tests: 125 new backend tests (provider reads across all four providers, the per-category appliers, the API endpoints) and 13 frontend tests. Full suites pass with no regressions; the 35 backend failures on Windows are byte-identical with and without this branch. |