From ceffcfaef607cb0c89ff218765a50dbf1aafe191 Mon Sep 17 00:00:00 2001 From: maziggy Date: Fri, 8 May 2026 09:19:09 +0200 Subject: [PATCH] fix(vp): overlay storage indicators on cached push so slicer pre-flight passes for P1S/A1 targets (issue #1228) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slicer "Send to printer" worked on 0.2.3.2 with a queue-mode VP and started failing on 0.2.4b3 with BambuStudio's generic "storage needs to be inserted before send to printer" error. Multiple users reported it across P1S, P2S, Docker bridge, macvlan, and host networking. @rtadams89's debug-level support archive showed the smoking gun: slicer establishes MQTT TLS, gets pushall + get_version, then never opens an FTP connection — pre-flight rejects before any data transfer. The 0.2.3.2 synthetic stub baked in three SD/storage indicators that BambuStudio's "Send" pre-flight reads: home_flag with bit 8 (HAS_SDCARD_NORMAL, 0x100), sdcard=True, and a storage:{free,total} block. The 0.2.4b3 cached-as-base slicer-mirror (7dea33d0) passes the live target's push_status through with only an IP rewrite — if the real firmware doesn't report those fields (P1S/A1 with no SD card, older field shapes, confirmed on P1S firmware 01.10.00.00), the slicer sees "no storage" and aborts. H2D and X1C reproductions worked because those firmwares do report the indicators. In _send_status_report's cached-as-base branch, after copying the cache and applying the existing protocol/upload-state overrides: - home_flag |= 0x100 (preserves any other bits the real printer set) - sdcard = True (force-set even when real says False) - storage = setdefault(...) (only fills in if missing — real values pass through unchanged when the printer reports them) For VP usage the slicer uploads via FTPS to Bambuddy's filesystem at /app/data/virtual_printer/uploads//; the printer's actual SD card is irrelevant on that path, so forcing "storage available" is correct for the queue / immediate / review modes the cached-as-base path covers. --- CHANGELOG.md | 1 + .../services/virtual_printer/mqtt_server.py | 14 +++++ backend/tests/unit/test_vp_mqtt_bridge.py | 59 +++++++++++++++++++ 3 files changed, 74 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9bb96eac..834352a68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to Bambuddy will be documented in this file. - **Slicer Bundle (.bbscfg) import — pick presets from a stored bundle instead of resolving cloud/local/standard PresetRefs every slice** — Closes the long tail of preset-resolution corner cases (cloud presets behind login, "from User" sentinel handling, the `# `-prefix clone trick, dangling `inherits` on renamed parents, etc.) by letting users upload a BambuStudio "Printer Preset Bundle" (`.bbscfg`) once per printer and pick from it for every subsequent slice. **Service layer (`backend/app/services/slicer_api.py`):** `BundleSummary` / `BundleNotFoundError` types, `import_bundle` / `list_bundles` / `get_bundle` / `delete_bundle` methods, `slice_with_bundle` which posts `/slice` with bundle id + per-category preset names instead of the JSON triplet. **Routes (`/api/v1/slicer/bundles`, all gated on `Permission.LIBRARY_UPLOAD`):** `POST` / `GET` / `GET :id` / `DELETE :id`. All routes proxy via `_resolve_slicer_api_url` so they follow the user's `preferred_slicer` setting (bambu_studio vs orcaslicer). Status-code mapping treats sidecar 4xx as 400, `BundleNotFoundError` as 404, sidecar unreachable as 503, and sidecar 5xx as 502. **Preview-slice (`backend/app/services/slice_preview.py::get_preview_filaments`)** picks up optional `bundle_id` + `printer_name` + `process_name` + `filament_names` params and routes through `slice_with_bundle` when set; the cache key picks up a bundle-context fingerprint so different bundle picks on the same file occupy distinct entries — gram numbers in the preview now match what the real print will produce instead of being derived from the file's embedded process settings (which can drift from the triplet the actual slice would use). The `library.py` and `archives.py` `/filament-requirements` routes forward the new params. **Dispatch (`SliceRequest.bundle: SliceBundleSpec`):** when set, `_run_slicer_with_fallback` skips `resolve_preset_ref` and calls `slice_with_bundle`; the validator skips the preset-required check so bundle-only requests validate. 3MF + bundle CLI 5xx still falls back to the embedded-settings slice path (`used_embedded_settings=True` surfaces in the response), and sidecar 404 (unknown bundle / preset name) maps to 400. **Frontend SliceModal Bundle tier:** new "Slicer bundle" picker at the top of the modal, rendered only when at least one bundle is imported (`GET /slicer/bundles` non-empty). Selecting a bundle replaces cloud / local / standard preset dropdowns with bundle-scoped pickers (process + per-slot filament names from the bundle) — printer is implicit (each `.bbscfg` has exactly one). "None" leaves the modal on the original preset-triplet path. Submit routes through `SliceRequest.bundle` so the backend skips PresetRef resolution and asks the sidecar to materialise the JSON triplet from the stored bundle by name. **Frontend types:** `SliceBundleSpec` + `bundle?: SliceBundleSpec` on `SliceRequest`; `getLibraryFileFilamentRequirements` / `getArchiveFilamentRequirements` accept an optional 4th-arg bundle context object. The orca-slicer-api fork's bundle endpoints (shipped on `bambuddy/bundle-import`) are the server side of this — see the slicer-api sidecar docker-compose for the matching versions. ### Fixed +- **Slicer "Send to printer" silently rejected the cached push_status with "storage needs to be inserted" on P1S/A1-class targets** ([#1228](https://github.com/maziggy/bambuddy/issues/1228), reported by @rtadams89, also hit by @smandon) — Slicer "Send" worked on 0.2.3.2 with a queue-mode VP and started failing on 0.2.4b3, regardless of subnet topology, with BambuStudio showing the generic "storage needs to be inserted before send to printer" error. Reproducible across Docker bridge, macvlan, and LAN-attached host networking. Network reachability ruled out (slicer reaches MQTT/FTPS, FTP passive ports 50000-50100 reachable end-to-end, pfSense rules clean). The smoking gun was in @rtadams89's debug-level support archive: slicer establishes MQTT TLS to the VP, gets `pushall` + `get_version` responses, then **never opens an FTP connection** — the slicer reads the cached push, fails its pre-flight, and aborts before attempting any data transfer. **Cause:** the 0.2.3.2 synthetic stub baked three SD/storage indicators that BambuStudio's "Send" pre-flight reads — `home_flag` with bit 8 (`HAS_SDCARD_NORMAL`, `0x100`), `sdcard: True`, and a `storage: {free, total}` block. The 0.2.4b3 cached-as-base slicer-mirror (commit `7dea33d0`) passes the live target's push_status through with only an IP rewrite; if the real firmware doesn't report those fields (P1S/A1 with no SD card inserted, older field shapes, P1S firmware `01.10.00.00` confirmed in @rtadams89's logs), the slicer sees "no storage" and refuses to send. H2D and X1C in maziggy's local cross-subnet repro worked because those firmwares do report the indicators; P1S/A1-class doesn't always. **Fix:** in `mqtt_server.py:_send_status_report` cached-as-base path, after copying the cache, OR `0x100` onto `home_flag` (preserves any other bits the printer set), force `sdcard=True`, and `setdefault` a `storage: {free: 1_000_000_000, total: 32_000_000_000}` block (only fills in if the real printer didn't report one — real values pass through unchanged when present). For VP usage the slicer uploads via FTPS to Bambuddy's filesystem under `/app/data/virtual_printer/uploads//`; the printer's actual SD card is irrelevant on that path, so forcing "storage available" is correct for the queue/immediate/review modes the cached-as-base path covers. Restores 0.2.3.2's working behaviour for these specific fields without losing the live AMS / k-profile / camera mirror that cached-as-base provides. **Tests:** new `test_storage_indicators_overlaid_for_send_preflight` (verifies SD bit OR'd onto a partial `home_flag`, `sdcard=True` forced even when real says False, `storage` injected when cache lacks it, free/total are non-zero) and `test_storage_indicators_preserve_real_storage_when_present` (real `home_flag=0x100` stays `0x100`, real `storage={free, total}` passes through unchanged so the overlay never overrides what the printer actually reported) in `test_vp_mqtt_bridge.py::TestStatusReportCachedAsBase`. Existing 25 tests in that suite still pass. - **MFA at-rest encryption is now default-on via auto-bootstrap** ([#1219](https://github.com/maziggy/bambuddy/issues/1219)) — Default Docker installs ran with `MFA_ENCRYPTION_KEY` unset, which silently fell back to plaintext storage for OIDC `client_secret` and TOTP secret rows. The single startup `logger.warning` was the only signal, and `.env.example` / `docker-compose.yml` / Settings UI never mentioned the variable, so any operator who wired up SSO or asked users to enroll in 2FA had to read the warning in the logs to know their secrets were unprotected at rest. **Auto-bootstrap:** `backend/app/core/encryption.py` now resolves the encryption key with the same precedence pattern as `_get_jwt_secret` — `MFA_ENCRYPTION_KEY` env var → `DATA_DIR/.mfa_encryption_key` file → auto-generated Fernet key written with mode `0o600`. The new helper `backend/app/core/paths.py:resolve_data_dir()` is shared with `auth.py` (DRY) and reads the env fresh on every call so test fixtures can override `DATA_DIR` per-test. Invalid env-var values (anything that doesn't decode to exactly 32 bytes via URL-safe base64) are rejected with a `logger.error` and the loader falls through to the file/auto-generate branches instead of crashing the encrypt/decrypt path with `ValueError`. **Re-encryption migration:** `_migrate_encrypt_legacy_secrets()` runs once on every startup after `run_migrations(conn)` finishes — it opens its own `async_session()` (separate from the schema-DDL connection, to avoid SQLite WAL lock contention) and converts any `oidc_providers.client_secret` / `user_totp.secret` row whose value doesn't already start with `fernet:` to the encrypted form via the existing property setters. The migration is idempotent (prefix check) and is a no-op when no key is loaded, so it can run safely on installs that never opt in. **Status endpoint + UI:** new `GET /api/v1/auth/encryption-status` (admin-only, gated on `Permission.SETTINGS_READ`) returns `key_configured`, `key_source ∈ {env, file, generated, none}`, plus per-table `legacy_plaintext_rows` and `encrypted_rows` counts and a derived `decryption_broken` flag (true iff encrypted rows exist but no key is loadable — the Phase-2 "operator deleted the key after rows were encrypted" recovery scenario). The new `frontend/src/components/SecurityStatusCard.tsx` lives in a new "Security" sub-tab under Settings → Authentication and renders four severity levels: green when everything is encrypted and a key is loaded, yellow when legacy plaintext rows still need re-encryption, orange when the key was auto-generated (with a backup hint pointing at `DATA_DIR/.mfa_encryption_key`), and red when `decryption_broken` is true. **Backup integration:** `routes/settings.py:create_backup_zip` now includes `.mfa_encryption_key` as a ZIP top-level entry (alongside `bambuddy.db`) so a self-contained backup can be restored to a fresh host without losing access to encrypted secrets. The matching `routes/settings.py:restore_backup` extracts the file back into `DATA_DIR` with `chmod(0o600)` and validates the basename exactly (`/`, `..`, `\\` rejected) so a manipulated ZIP cannot path-traverse outside `DATA_DIR`. If the file is absent from the ZIP (legacy backup) the restore proceeds without error — the next boot will auto-bootstrap a fresh key, and any plaintext rows that come back from the backup remain readable via the existing legacy-plaintext fallback in `mfa_decrypt`. **Test isolation:** new autouse `mfa_encryption_isolation` fixture in `conftest.py` per-test points `DATA_DIR` at a `tmp_path`, clears `MFA_ENCRYPTION_KEY` from env, and resets the `_fernet_instance` / `_warn_shown` / `_key_source` module globals — so the auto-bootstrap can never write a real key file into the repo and pytest-xdist workers don't share encryption state. **i18n:** new `settings.encryption.*` namespace and `settings.tabs.security` label across all 8 locales (en + de fully translated; fr/it/ja/pt-BR/zh-CN/zh-TW seeded with English copy pending native translation, matching the project's existing flow for newly-added keys). **Docs:** `.env.example` documents the new variable + the backup self-containment behaviour; `docker-compose.yml` carries an auto-commented entry; `.gitignore` adds `.mfa_encryption_key` alongside the existing `.jwt_secret` project-root guard. **Tests:** 9 new unit tests in `TestEncryption` (env/file/generated key sources, invalid-env fall-through, OSError → `none`, mode `0o600` check), 6 new in `TestEncryptLegacyMigration` (plaintext → encrypted for OIDC + TOTP, idempotent re-run, mixed state, no-op without key, log assertion), 8 new in `TestEncryptionStatusEndpoint` (each `key_source`, count assertions, `decryption_broken` recovery scenario, `Permission.SETTINGS_READ` gate), 2 new in `TestEncryptionRoundtrip` (raw column reads return ciphertext, property reads return plaintext for both OIDC and TOTP), 6 new in `TestBackupKeyFiles` (ZIP includes / skips key files, restore chmod `0o600`, missing-file tolerance, path-traversal rejection), and 6 new frontend tests in `SecurityStatusCard.test.tsx` (each severity level + the disabled state). - **Camera preview popup opened to a blank page; deep-route refresh and direct URL load broken** ([#1221](https://github.com/maziggy/bambuddy/issues/1221), reported by @enjoylifenow / @Haeckan / @elit3ge / @jc21) — Clicking "open camera in new window" from the printer card opened a popup that rendered as an empty white page across P1S / P2S / X1 series, every install method (Docker / git clone), every browser (Chrome / Firefox / Brave / Safari), starting with the daily build of 2026-05-05. **Cause:** PR #1195 (`d6a31393`, "fix(frontend): emit relative asset paths so SPA loads under any subpath") set `base: ''` in `vite.config.ts` to support path-prefixed reverse proxies (HA Ingress, nginx subpath, Cloudflare Tunnel path routing). With that, the built `index.html` references its bundle and stylesheet via relative URLs (`./assets/index-XXX.js`, `./sw-register.js`). When the popup opened at `/camera/`, the browser resolved `./assets/index-XXX.js` against the current document URL — which doesn't end in a slash, so the URL parser treated `` as a file and `/camera/` as the directory, giving `/camera/assets/index-XXX.js`. The backend's SPA catch-all returned `index.html` (text/html) for that request, and modern browsers refuse to execute HTML as a JS module under `X-Content-Type-Options: nosniff`, so the popup loaded the document but never the bundle. Same break hit any deep route on initial load — direct URL paste / refresh on `/camera/:printerId`, `/projects/:id`, `/groups/:id/edit`, `/files/trash`, `/external/:id`, and the SpoolBuddy kiosk's `/spoolbuddy/ams` if loaded directly — manifesting as a quiet "blank page on refresh" that users worked around by navigating from the home page. The console error gives it away: `Loading module … was blocked because of a disallowed MIME type ("text/html")`. **Fix:** revert PR #1195's `vite.config.ts` and `sw-register.js` changes — `base: ''` is removed (Vite default `'/'` restored), and `navigator.serviceWorker.register('sw.js')` reverts to `register('/sw.js')`. The built `index.html` now emits absolute asset URLs (`/assets/...`, `/manifest.json`, `/sw-register.js`) which resolve against host root regardless of document URL, so deep routes load their assets correctly on initial navigation. PR #1195's class of bug — path-prefixed reverse proxy users serving Bambuddy at a subpath — was already explicitly closed as wontfix in that thread because supporting it requires subpath-aware bootstrapping (API_BASE, React Router basename, PWA manifest scope, service-worker scope, push-subscription scope) for every user forever. The supported workaround for that audience is documented: NPM (Nginx Proxy Manager) addon + Cloudflare Tunnel at a real domain with HTTPS, then HA Webpage panel embedding via `TRUSTED_FRAME_ORIGINS` — that path doesn't depend on `base: ''` at all. The trade-off here is intentional: revert reaches every user impacted by deep-route initial-load bugs (much larger population than path-prefixed proxy users), in exchange for an already-wontfixed subpath proxy regression that has a working alternative. ([#1237](https://github.com/maziggy/bambuddy/issues/1237), reported by @basziee) — In the Configure AMS Slot modal, profile names like `SUNLU PETG GLOW IN THE DARK GEN2 @Bambu Lab H2C 0.4 nozzle` were visually truncated mid-name, hiding the `@ ` suffix. With several near-identical entries differing only in nozzle size, users had to open browser dev tools to tell them apart. **Fix:** the preset row now expands inline on hover — `truncate` stays as the default (so the list keeps its compact one-line shape) but `group-hover:whitespace-normal group-hover:break-all` flips it to a wrapped multi-line view the moment the cursor enters the row, so the nozzle suffix is readable instantly without waiting on the browser's title-tooltip delay. The parent button gets `group` to drive the hover. The native `title={preset.name}` is also added as a belt-and-braces fallback for assistive tech and touch devices where `:hover` doesn't fire. Same pattern in both the desktop and mobile layouts of `ConfigureAmsSlotModal.tsx`. No new dependencies. **Test:** new `ConfigureAmsSlotModal.test.tsx` regression assertion that the rendered preset span carries `title=` plus the `truncate` and `group-hover:whitespace-normal` classes, and the parent button has `group` — so a future refactor that drops any of those fails CI. - **Filament usage double-counted when AMS auto-falls-back to a same-material spool** ([#957](https://github.com/maziggy/bambuddy/issues/957)) — When one spool ran out mid-print and the AMS transparently switched to a sibling slot loaded with the same material, the usage tracker credited the originally-mapped spool with the full 3MF estimate AND added the fallback spool's remain%-delta on top — so a 78 g print could show as 78 g + 60 g = 138 g consumed across the two spools, leaving the empty spool's recorded weight beyond its label weight (the symptom the original report flagged on a 1209 g spool reading "1188.30 g used" while the new spool only got a 30 g credit). Two interacting bugs: (1) the tray-change recorder in `bambu_mqtt.py` gated on `state in ("RUNNING", "PAUSE")` literal strings, and P2S firmware briefly transitions out of RUNNING during the AMS swap, so the switch was never appended to `tray_change_log`; (2) the usage-tracker splitting branch in `usage_tracker.py` was gated on `not slot_to_tray`, so even when the tray-change log was populated the splitting code only ran for prints where the slicer's mapping had not been captured — i.e. never on the actual fallback case. **Fix:** the `bambu_mqtt.py` gate now keys on the print-lifecycle flags (`_was_running and not _completion_triggered`) so any tray change between print start and completion is captured regardless of the momentary `gcode_state` string. The `usage_tracker.py` gate is split so `tray_change_log` evidence with > 1 entries always takes over from `slot_to_tray`, treating the per-segment per-layer gcode usage as the source of truth when the printer actually fed from multiple trays. Path 2 (AMS remain%-delta fallback) then naturally skips both trays because they're already in `handled_trays` after splitting, eliminating the double-credit. **Tests:** new `test_tray_change_recorded_during_intermediate_state` and `test_tray_change_not_recorded_after_completion` in `test_bambu_mqtt.py` exercising the new gate; new `test_tray_switch_overrides_print_cmd_mapping` in `test_usage_tracker.py` pinning that with `ams_mapping=[0]` set and `tray_change_log=[(0,0),(1,30)]` the splitter produces two segments summing to the 3MF estimate (no double-count) and adds both `(0,0)` and `(0,1)` to `handled_trays`. diff --git a/backend/app/services/virtual_printer/mqtt_server.py b/backend/app/services/virtual_printer/mqtt_server.py index dde9ae1f0..4704b5106 100644 --- a/backend/app/services/virtual_printer/mqtt_server.py +++ b/backend/app/services/virtual_printer/mqtt_server.py @@ -659,6 +659,20 @@ class SimpleMQTTServer: else: # Don't override real subtask_name with empty if no upload pending. print_block.setdefault("subtask_name", "") + # Storage-availability indicators the slicer's "Send" pre-flight reads + # (#1228). P1S/A1-class firmware doesn't always include these in + # push_status (no SD card inserted, older field shapes), and BambuStudio + # rejects the send pre-flight with the generic "storage needs to be + # inserted before send to printer" error before even attempting FTP. + # For VP usage the slicer uploads via FTPS to Bambuddy's filesystem — + # the printer's actual SD/storage state is irrelevant on that path. + # Force "available" indicators so the pre-flight passes regardless of + # what the real printer reports. Restores the 0.2.3.2 synthetic-stub + # behaviour for these fields without losing the live AMS / k-profile / + # camera mirror cached-as-base provides. + print_block["home_flag"] = print_block.get("home_flag", 0) | 0x100 # bit 8 = HAS_SDCARD_NORMAL + print_block["sdcard"] = True + print_block.setdefault("storage", {"free": 1_000_000_000, "total": 32_000_000_000}) status = {"print": print_block} await self._publish_to_report(writer, status, serial or self.serial) return diff --git a/backend/tests/unit/test_vp_mqtt_bridge.py b/backend/tests/unit/test_vp_mqtt_bridge.py index 77f509a89..c0b541903 100644 --- a/backend/tests/unit/test_vp_mqtt_bridge.py +++ b/backend/tests/unit/test_vp_mqtt_bridge.py @@ -393,6 +393,65 @@ class TestStatusReportCachedAsBase: assert payload["print"]["nozzle_type"] == "hardened_steel" assert "storage" in payload["print"] + @pytest.mark.asyncio + async def test_storage_indicators_overlaid_for_send_preflight(self): + """#1228: P1S/A1-class firmware doesn't always include the SD/storage + fields BambuStudio's "Send" pre-flight reads. Without these the + slicer rejects with 'storage needs to be inserted' before even + attempting FTP. The cached-as-base path now overlays them so the + pre-flight passes regardless of what the real printer reports. + """ + server = _make_server() + bridge = MagicMock() + # Real P1S push without SD card inserted: home_flag has other bits set + # but the SD bit (0x100) is clear; sdcard is False; no storage field. + bridge.get_latest_print_state.return_value = { + "command": "push_status", + "msg": 0, + "home_flag": 0x42, + "sdcard": False, + } + server.set_bridge(bridge) + published = self._capture_published(server) + + await server._send_status_report(MagicMock()) + _serial, payload = published[0] + # SD bit ORed onto whatever was there — other bits preserved. + assert payload["print"]["home_flag"] & 0x100 == 0x100 + assert payload["print"]["home_flag"] & 0x42 == 0x42 + # Force-set so a False from the printer doesn't trip the pre-flight. + assert payload["print"]["sdcard"] is True + # storage was missing — the overlay must inject a non-empty default. + assert "storage" in payload["print"] + assert payload["print"]["storage"]["free"] > 0 + assert payload["print"]["storage"]["total"] > 0 + + @pytest.mark.asyncio + async def test_storage_indicators_preserve_real_storage_when_present(self): + """When the real printer DOES report a storage block, pass it through + unchanged (the overlay only fills in the missing field, not overrides). + """ + server = _make_server() + bridge = MagicMock() + real_storage = {"free": 12345, "total": 67890} + bridge.get_latest_print_state.return_value = { + "command": "push_status", + "msg": 0, + "home_flag": 0x100, # SD bit already set on the real printer + "sdcard": True, + "storage": real_storage, + } + server.set_bridge(bridge) + published = self._capture_published(server) + + await server._send_status_report(MagicMock()) + _serial, payload = published[0] + # SD bit OR is idempotent — already-set bit stays set. + assert payload["print"]["home_flag"] == 0x100 + assert payload["print"]["sdcard"] is True + # Real values pass through, NOT the synthetic defaults. + assert payload["print"]["storage"] == real_storage + @pytest.mark.asyncio async def test_overrides_protocol_fields_even_when_cache_present(self): """Cached value's gcode_state must NOT win over our local upload-state-machine value."""