From 5d6d928b3f18f6618e782b79b291f196d37618b8 Mon Sep 17 00:00:00 2001 From: maziggy Date: Tue, 2 Jun 2026 14:33:20 +0200 Subject: [PATCH] fix(virtual-printer): #1429 net.info[*].ip cache leak + mode wire-value rename MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1429 (reported by @TrickShotMLG02, confirmed by @Mape6 on a flat single-LAN that rules out subnet / mDNS-reflector theories): with the physical printer off the slicer's "Send" landed in Bambuddy's archive; once the printer powered on every subsequent "Send" went straight to the printer's SD card and bypassed Bambuddy. Bundle analysis: mape6-before showed clean FTP receive + archive lines, mape6-after had zero FTP attempts to Bambuddy once the printer was online. Cause: mqtt_bridge.py::_resolve_client encoded _target_ip_uint32_le / _vp_ip_uint32_le ONLY on client-identity change and early-returned on every refresh tick when the same client object was still bound. If target_client.ip_address was empty at first bind (DB row stale, or client constructed before SSDP refresh filled it in), the encoding stayed None, the net.info[*].ip rewrite block was skipped, the cache filled with the real printer IP, sticky-key preservation kept the poisoned net value alive across every subsequent incremental push, and the slicer followed the leaked IP. Only Bambuddy-restart-with-printer-off cleared it — the workaround both reporters independently arrived at. Same shape on multi-NIC printers (X1C, H2D Pro): the rewrite only matched entries whose ip equalled _target_ip_uint32_le, so a secondary interface IP Bambuddy never saw would leak through unchanged. Bridge fix: - _resolve_client calls a new _refresh_ip_encoding() on every refresh tick, even when client identity is unchanged; self-heals once ip_address becomes valid. - _refresh_ip_encoding() sweeps the existing _latest_print_state when encoding becomes valid for the first time. Without the sweep, sticky-key preservation keeps the pre-arm poisoned cache alive forever — incremental pushes that don't include net carry the bad value forward. - _rewrite_net_info_ips() rewrites EVERY non-zero net.info[].ip entry that doesn't already equal the VP IP, not just entries matching _target_ip_uint32_le. Multi-NIC printers stop leaking secondary interfaces. Zero-IP placeholders are left alone so "active interface" detection still works. - INFO logging on encoding arm/update and on cache sweep so future bundles directly answer "did the rewrite fire?". Mode wire-value rename (#1429 follow-up, separate confusion source): - Both reporters' support bundles showed mode: immediate while the UI said "Archive"; @TrickShotMLG02 quoted: "I have no idea why it says immediate in the support-info.json file. In the webui the printer is set to archive". UI button "Archive" had always saved immediate, and "Queue" had always saved print_queue. Canonical wire values are now archive / review / queue / proxy, matching the button labels 1:1. - New normalize_vp_mode() + VP_MODE_* constants in models/virtual_printer.py; manager.py normalises on construction so a legacy row read pre-migration still dispatches correctly. - core/database.py::run_migrations rewrites existing virtual_printers and settings rows; idempotent (re-runs are no-ops); identical SQL under SQLite and Postgres. - API routes accept both legacy and canonical on input, normalise before storage. GET /settings/virtual-printer normalises on read so the frontend's mode-button highlight works for stale legacy values. - Three frontend VP components (VirtualPrinterSettings, VirtualPrinterCard, VirtualPrinterAddDialog) switched click handlers and type aliases to canonical; each got its own normalizeMode() helper so a stale-cached settings payload still highlights the right button. Two pre-existing `printer.mode === 'queue' ? 'review'` legacy mappings in VirtualPrinterCard were the source of a test failure caught mid-implementation where the new canonical 'queue' was being mis-aliased back to 'review' and hiding the auto-dispatch + force-color-match toggles. mode handler is NOT the dispatch bug: manager.py::_archive_file (the handler for archive mode) doesn't dispatch to the physical printer. The "files end up on the printer's SD card" symptom was the IP-leak from the bridge cache. The mode rename is purely clarity / support- bundle accuracy. --- CHANGELOG.md | 1 + backend/app/api/routes/settings.py | 31 +- backend/app/api/routes/virtual_printers.py | 17 +- backend/app/core/database.py | 31 +- backend/app/models/virtual_printer.py | 36 +- backend/app/schemas/settings.py | 4 +- .../app/services/virtual_printer/manager.py | 34 +- .../services/virtual_printer/mqtt_bridge.py | 121 ++- .../integration/test_virtual_printer_api.py | 59 +- .../unit/services/test_virtual_printer.py | 84 +- .../tests/unit/services/test_vp_diagnostic.py | 2 +- .../unit/test_vp_mode_rename_migration.py | 151 ++++ backend/tests/unit/test_vp_mqtt_bridge.py | 89 ++ .../components/VirtualPrinterCard.test.tsx | 24 +- .../VirtualPrinterDiagnosticModal.test.tsx | 2 +- .../VirtualPrinterSettings.test.tsx | 47 +- .../InventoryPageArchivedConsumed.test.tsx | 2 +- .../pages/InventoryPageCopyButton.test.tsx | 2 +- .../pages/InventoryPageDeepLink.test.tsx | 2 +- .../pages/InventoryPageLowStock.test.tsx | 2 +- .../InventoryPageSpoolmanLocation.test.tsx | 2 +- frontend/src/api/client.ts | 8 +- .../components/VirtualPrinterAddDialog.tsx | 10 +- .../src/components/VirtualPrinterCard.tsx | 35 +- .../src/components/VirtualPrinterSettings.tsx | 34 +- .../{index-DqHz1llA.js => index-BuuCTvNJ.js} | 848 +++++++++--------- static/index.html | 2 +- 27 files changed, 1065 insertions(+), 615 deletions(-) create mode 100644 backend/tests/unit/test_vp_mode_rename_migration.py rename static/assets/{index-DqHz1llA.js => index-BuuCTvNJ.js} (89%) diff --git a/CHANGELOG.md b/CHANGELOG.md index b060c9d08..55615a53c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ All notable changes to Bambuddy will be documented in this file. ### Security - **Path-traversal hardening across the upload / import / file-write surface (routes + services); fifth CI backstop ships alongside** — A private path-traversal report against `POST /api/v1/projects/import/file` traced two attacker-controlled strings being joined to `library_dir` with no resolve + containment check: (a) `linked_folders[*].name` from the request's `project.json` ("Vector A" — an absolute path in this field collapsed `library_dir / "/anywhere"` to `Path("/anywhere")` because pathlib discards the left side when the right is absolute, letting the next `write_bytes` land anywhere the backend could write), and (b) per-entry `zf.namelist()` paths from the ZIP itself ("Vector B" — ZIP filenames carry `..` segments by spec and the join `library_dir / folder_name / relative_path` had no per-component check). Concrete escalation: drop a `.pth` file into the venv's `site-packages` directory for code execution on next service restart; overwrite the JWT signing-secret file to forge an admin token; overwrite `~/.ssh/authorized_keys` or `~/.bashrc` on native installs. **Fix is structural, not just patch the diff** (per [[feedback_dont_dismiss_preexisting]]). New `backend/app/utils/safe_path.py::safe_join_under(parent, *parts)` helper joins under a trusted parent, resolves both sides, asserts `is_relative_to(parent.resolve())`, and rejects up-front empty / null-byte / absolute path components. Wired into `import_project_file` at both vectors. **Adjacent fix from the routes audit**: `GET /api/v1/archives/{id}/photos/{filename}` had NO validation on `filename` and FileResponse-served arbitrary paths — the existing DELETE endpoint at least had a membership check against `archive.photos` (which is UUID-generated on upload), but GET shared neither the check nor any traversal guard. Both GET and DELETE now route through `safe_join_under` for defence-in-depth on top of the membership check. **Second adjacent fix from the services audit**: `ArchiveService.attach_timelapse(archive_id, data, filename)` in `backend/app/services/archive.py:1456` wrote `archive_dir / filename` where `filename` ultimately comes from either a printer's FTP listing (compromised-printer threat model — the printer is part of the trust surface) or the `?filename=...` query param on `POST /api/v1/archives/{id}/timelapse/select`. A malicious printer that returns a directory listing entry with `..` segments could write the timelapse bytes outside the archive directory; the `f.get("name") == filename` gate in the route did not prevent it because the gate is satisfied by whatever the printer claims is on disk. `attach_timelapse` now routes through `safe_join_under(..., http=False)` and returns `False` (logging the rejection) when the join would escape — matching the existing not-found contract of the function rather than raising 400 from inside a background task. **Audit sweep methodology**: AST-walked every Python file under `backend/app/api/routes/` AND `backend/app/services/` for `Path / Name` shapes (the exact shape that produced the original report). 25 additional route-layer sites and 8 additional service-layer sites confirmed safe case-by-case (UUID-generated filenames written by Bambuddy itself, `_safe_filename(...)` / `Path(arg).name` basename-stripped inputs, `os.walk`-discovered names, denylist + format-validated backup names, hardcoded constants iterated through a tuple, DB-stored paths whose write origin already goes through a resolved-and-containment-checked helper). Each safe site got a `# SEC-PATH-OK: ` marker so future audits can trust the inline guard at a glance. Six pre-existing safe-with-marker sites (`library.py` external upload, `archives.py` timelapse output, `projects.py` attachment download/delete, `settings.py` backup extractall) carry the same marker shape. **Fifth CI backstop** `test_route_path_arithmetic_is_safe_joined_or_marked` (`backend/tests/unit/test_no_unsafe_path_joins.py`) AST-walks every Python file in `backend/app/api/routes/` AND `backend/app/services/` and fails the build on any ` / ` join that doesn't either route through `safe_join_under` or carry the marker on the join line. Joins matching the higher-structure shapes (Attribute access, Subscript, f-string, `str(...)` call) are categorically different and out of scope — those are caught by the broader audit sweep, not the regression backstop. The services layer is in scope because it receives values from the routes verbatim AND from external sources Bambuddy has no control over (the printer FTP-listing case above). **Tests**: 17 unit tests for `safe_join_under` covering every escape vector (absolute path, Windows abs path, `..` segments, embedded `..`, null byte, empty string, no parts, non-str, plus legitimate nested-path round-trip); 4 integration tests against `POST /api/v1/projects/import/file` exercising the full FastAPI stack with the verbatim shape from the report (absolute path in `folder_name` → 400 + filesystem assertion that the target file doesn't exist; `..` in `folder_name` → 400; `..` in `relative_path` → 400; legitimate nested ZIP still imports cleanly to guard against the fix being over-strict); 3 unit tests against `ArchiveService.attach_timelapse` exercising the compromised-printer threat model (filename with `..` segments → returns False + no file at the escape target; absolute filename → returns False + no file at `/tmp`; legitimate `timelapse_YYYY-MM-DD_HH-MM-SS.mp4` → returns True + file lands inside archive_dir, guarding against the fix being over-strict). **SECURITY.md** gains a fifth rule + a fifth row in the CI-test mapping table; the rule explicitly names the printer FTP-listing case as in-scope to set the expectation for future services-layer audits. Full 5500+ test backend suite green; ruff clean. ### Fixed +- **Virtual-printer "Send file" no longer redirects from Bambuddy to the physical printer's SD card once the printer powers on, and the mode button labels finally match the wire values stored in the DB (#1429, reported by @TrickShotMLG02, confirmed by @Mape6)** — Two reporters on completely different network topologies (3-subnet routed via OPNsense vs. flat single-LAN) saw the same symptom: with the physical printer off and Bambuddy freshly restarted, the slicer's "Send" landed in Bambuddy's archive; once the printer powered on, every subsequent "Send" went straight to the printer's SD card and bypassed Bambuddy entirely. @Mape6's packet capture on the flat-LAN case ruled out subnet / mDNS-reflector / firewall theories — the slicer just had a non-Bambuddy IP for the FTP destination once the printer was online. Bundle analysis: `mape6-before` (printer off) showed clean FTP receive + archive lines; `mape6-after` (printer on) had zero FTP connection attempts to Bambuddy, full stop. The mode-label discrepancy in every support bundle was a separate red herring that needed clearing up in the same drop. **Root cause** — `backend/app/services/virtual_printer/mqtt_bridge.py::_on_printer_raw` caches the real printer's `push_status` and rewrites `net.info[*].ip` from real-printer LE-uint32 to VP-bind-IP LE-uint32 so the slicer's FTP destination resolves to the VP. The rewrite has been in tree since 2026-05-03 and the unit test that ships with it passes. But the encoding (`_target_ip_uint32_le`, `_vp_ip_uint32_le`) was only computed inside `_resolve_client` on **client-identity change**, and `_resolve_client` early-returned (`if current is self._target_client: return`) on every refresh tick when the same client object was still bound. So if the printer's MQTT client object existed but `ip_address` was empty/stale at first bind (e.g. the printer's DB row hadn't picked up its discovered IP yet, or the client was constructed before the SSDP refresh), the encoded LE-uint32 stayed `None`, the rewrite block was skipped, the cache filled with the real printer IP, the sticky-keys preservation in the same function kept that poisoned `net` value alive across every subsequent incremental push, and the slicer followed the leaked IP to the real printer. The only way to clear it was to restart Bambuddy with the printer off — which is exactly the workaround both reporters independently arrived at. **Same shape on multi-NIC printers**: the rewrite only matched entries whose `ip` equalled `_target_ip_uint32_le`, so an X1C / H2D Pro reporting two active interfaces (WiFi + Ethernet) would have one entry rewritten and the other leaking the printer's other IP — a separate FTP fallback path that bypasses the VP even when the primary rewrite worked. **Fixes (mqtt_bridge.py)**: (1) `_resolve_client` now calls a new `_refresh_ip_encoding()` helper on every refresh tick, even when the client identity is unchanged — re-reads `current.ip_address`, re-encodes if either side changed, self-heals once `ip_address` becomes valid. (2) When the encoding becomes valid for the first time *after* the cache has already been populated, `_refresh_ip_encoding()` sweeps the cached `_latest_print_state` via the new `_rewrite_net_info_ips()` helper so the slicer's next pull sees the rewritten value — without this, sticky-key preservation keeps the poisoned cache alive across every incremental update. (3) `_rewrite_net_info_ips()` rewrites **every** non-zero `net.info[].ip` entry that doesn't already equal the VP bind IP, not only entries matching `_target_ip_uint32_le` — defensive against multi-NIC printers, against `_target_ip_uint32_le` being stale, and against unknown secondary interfaces leaking. Zero-IP entries (placeholders for unpopulated interfaces) are deliberately left alone so the slicer's "active interface" detection still recognises them as absent. (4) The rewrite path now logs at INFO when encoding arms or updates and at INFO when the cache sweep rewrites entries, so future support bundles directly answer "did the rewrite fire?" without re-reasoning about timing. **Mode wire-value rename (#1429 follow-up, separate confusion source)** — The UI button labeled "Archive" had always saved the wire value `immediate`, and "Queue" had always saved `print_queue`. Both reporters' support bundles showed `mode: immediate` while the UI said "Archive", and @TrickShotMLG02 specifically asked "I have no idea why it says immediate in the support-info.json file. In the webui the printer is set to archive". The mismatch was load-bearing for the debug session and had to be cleared up. Canonical wire values are now `archive` / `review` / `queue` / `proxy` matching the button labels 1:1. **Backend rename**: new `backend/app/models/virtual_printer.py::VP_MODE_*` constants + `normalize_vp_mode()` helper accepts legacy `immediate` / `print_queue` and translates to canonical. `VirtualPrinter.mode` default flipped to `archive`. `backend/app/services/virtual_printer/manager.py::VirtualPrinterInstance.__init__` normalises on construction so a legacy DB row read before the migration window has finished still dispatches to the correct handler; `on_file_received`, `on_print_command`, and `sync_from_db`'s change-detection all consume canonical values via `normalize_vp_mode()`. `backend/app/api/routes/virtual_printers.py::create_virtual_printer` and `update_virtual_printer` accept both forms on input and normalise to canonical before storage; `backend/app/api/routes/settings.py::get_virtual_printer_settings` normalises on read so frontend mode-button highlighting works for legacy stored values; `update_virtual_printer_settings` accepts and normalises on write. `backend/app/schemas/settings.py::AppSettings.virtual_printer_mode` default flipped to `archive` with updated description. **One-shot DB migration**: `backend/app/core/database.py::run_migrations` rewrites every `virtual_printers.mode` and `settings.virtual_printer_mode` row from `immediate` → `archive` and `print_queue` → `queue`. Idempotent — re-running on canonical values is a no-op, important because the full migration set runs every boot. Identical statement under SQLite and Postgres (plain `UPDATE ... WHERE` on a string column, no dialect-specific syntax) per the [[feedback_sqlite_and_postgres_upfront]] HARD RULE; tested explicitly on SQLite in the new regression suite, behaviour-identical on Postgres by construction. The historical single-VP migration (legacy `settings` rows → `virtual_printers` table on first multi-VP boot) gets the same `immediate` → `archive` / `print_queue` → `queue` translation; the historical `queue` → `review` alias is preserved because it predates the rename and reflected the user's intent at the time (the old wire `queue` meant "pending review", not "add to print queue"). **Frontend rename**: `VirtualPrinterSettings.tsx`, `VirtualPrinterCard.tsx`, and `VirtualPrinterAddDialog.tsx` all switched their button click handlers and `LocalMode`/`Mode` type aliases from `'immediate' | 'review' | 'print_queue' | 'proxy'` to `'archive' | 'review' | 'queue' | 'proxy'`. Each file gained its own `normalizeMode()` helper that translates legacy values arriving via stale-cached settings payloads to canonical, so the right mode button lights up even when the backend migration hasn't completed for that user's session yet. The two `printer.mode === 'queue' ? 'review' : printer.mode` legacy mappings in `VirtualPrinterCard.tsx::useEffect` and the error-recovery path have been replaced with `normalizeMode()` — they were the source of the test failure I caught mid-implementation where `mode: 'queue'` (the new canonical for the Queue button) was being incorrectly aliased back to `'review'` and hiding the auto-dispatch + force-color-match toggles. `frontend/src/api/client.ts::VirtualPrinterMode` is now the union of both canonical and legacy values (`'archive' | 'review' | 'queue' | 'proxy' | 'immediate' | 'print_queue'`) so older API clients (forks, mobile shortcuts, scripted setups) typecheck; the `updateSettings` body type narrows to canonical-only to steer new code. **Mode handler is NOT the dispatch bug**: `manager.py::_archive_file` is the handler for `archive` mode and it does archive-only (no dispatch to the physical printer). The user-visible "files end up on the printer's SD card" symptom was the IP-leak from the bridge cache, not a mode-dispatch bug. The mode rename is purely a clarity / support-bundle-accuracy fix. **Tests** — `backend/tests/unit/test_vp_mqtt_bridge.py`: 2 new in the bridge-rewrite class — `test_net_info_ip_rewritten_for_unknown_secondary_interface` covers the multi-NIC X1C / H2D Pro case where the printer reports an interface IP Bambuddy never saw; both entries get rewritten, the placeholder zero entry stays untouched. `test_late_arriving_printer_ip_rewrites_existing_cache` is the primary #1429 regression — bridge binds to a client with `ip_address=""`, first push lands and poisons the cache with the real-printer IP (the pre-fix state), the printer's `ip_address` then becomes known, the next `_resolve_client` tick arms the encoding AND sweeps the cached `net.info[].ip` so the slicer's next pull sees the VP IP. Without the sweep, sticky-key preservation would keep the poisoned value alive forever. `backend/tests/unit/test_vp_mode_rename_migration.py`: new file, 3 tests — legacy `immediate` → `archive` and `print_queue` → `queue` rewrites under SQLite, canonical values pass through untouched; legacy `virtual_printer_mode` setting also gets rewritten; running the migration twice is idempotent (every boot re-runs the full migration set). `backend/tests/integration/test_virtual_printer_api.py`: 3 reworked tests cover input-side normalisation — `test_update_mode_to_queue` asserts canonical, `test_update_mode_legacy_print_queue_normalises_to_queue` and `test_update_mode_legacy_immediate_normalises_to_archive` assert legacy → canonical translation on storage. The pre-existing `test_update_mode_legacy_queue_maps_to_review` (predating the rename, asserted the old `queue` → `review` alias) is removed; the new `test_update_mode_to_archive` covers canonical archive setting. **All other VP tests were updated to canonical** — `test_virtual_printer.py` (43 occurrences), `test_vp_diagnostic.py` (1), `test_virtual_printer_api.py` mocks (5) renamed; the `sync_from_db_restarts_on_mode_change` test had to be repaired by hand because the sed pass made both sides `archive` (defeating the change detection); now uses `archive` → `review` to actually exercise the change branch. Frontend: `VirtualPrinterCard.test.tsx`, `VirtualPrinterSettings.test.tsx`, `VirtualPrinterDiagnosticModal.test.tsx` updated to canonical fixtures and assertions; the legacy `queue maps to review` test in `VirtualPrinterSettings.test.tsx` replaced with two tests — legacy `immediate` lights up the Archive button, legacy `print_queue` lights up the Queue button, both via the new client-side `normalizeMode()` helper. The five `InventoryPage*.test.tsx` files that hardcoded `virtual_printer_mode: 'immediate'` in their settings mocks bulk-renamed to `'archive'`. **CI gates green**: backend pytest 5546 passed in 73.88s + 7.10s under `-n 30` / `-n 12` parallel; ruff clean; frontend `npm run build` clean (TypeScript + Vite); ESLint zero output; vitest 2045 passed in 26.12s; i18n parity script clean at 5007 leaves × 9 locales. **Deferred (fix D in the diagnosis writeup)**: bind_ip == 0.0.0.0 path. The rewrite is still explicitly skipped when bind_ip is the unspecified address, which is correct for the routing (you can't tell a slicer to FTP to 0.0.0.0) but leaves users without a dedicated bind IP exposed to the same IP-leak pattern. Both reporters had `has_bind_ip=true` in their bundles so this isn't load-bearing for #1429 itself; will be addressed as a separate audit-shaped change that needs to enumerate the host's outbound IPs and pick the one that can reach the printer, with its own test surface. **Out of scope for this PR**: port 40024 in @Mape6's packet capture (Bambu Network Plugin's LAN-Send pre-flight port) — a probe that arrives at the VP IP, finds no listener, and the slicer falls back. Adding a 40024 listener is conceptually a different surface (handshake parsing, not MQTT cache state) and the cache-leak fix alone removes the underlying redirection so the 40024 probe lands on a VP that's actually the right destination. Will reassess if either reporter still sees mis-routing after this fix. - **Multi-plate `.gcode.3mf` archives + reprints no longer under-report filament, time, and cost — project stats and parser both fixed (#1593, reported by @needo37)** — Reporter printed 3 plates of a multi-plate file: Archive Print Log correctly recorded 3 completed runs at distinct durations and filament weights; Project page showed `Print Jobs: 1 / 1 parts printed`, plate-1's `1h53m / 58g / $1.09`; Archive card said `3 prints` but rendered plate-1's `57.6g / 1h45m / 1 object`. Two distinct causes stacked. **Root cause 1 — 3MF parser only read the first plate**: `ThreeMFParser._parse_slice_info` (`backend/app/services/archive.py:191`) called `root.find(".//plate")` and pulled `prediction` / `weight` from that one element — so for any multi-plate file the archive's file-level `print_time_seconds` / `filament_used_grams` reflected plate 1 alone. The per-plate `/plates` endpoint already looped `findall(".//plate")` and was correct, which is why the plate carousel showed the right numbers while the archive card was wrong. **Root cause 2 — project rollup aggregated `PrintArchive`, not the per-run log**: `compute_project_stats` and the `list_projects` quick-stats block (`backend/app/api/routes/projects.py`) summed `PrintArchive.print_time_seconds / filament_used_grams / cost / energy_*` `WHERE project_id = X`. A reprint reuses the source archive row and only adds a new `PrintLogEntry`, so 3 sequential runs of one file collapsed to 1 archive — and that archive's numbers were already plate-1-only because of root cause 1. The Archive Print Log path was correct because it already drove off `print_log_entries` (`archives.py:420` — *"Reads from print_log_entries so reprints contribute each run"*); project stats just hadn't been pointed at the same source. **Parser fix**: `_parse_slice_info` now loops `findall(".//plate")` and sums `prediction` → `print_time_seconds` and `weight` → `filament_used_grams` across all plates. Per-plate concepts (`plate_number`, `_plate_index`, `printable_objects`) are only set when there's exactly one plate — for multi-plate exports the archive represents all plates and a single plate index is meaningless at the file level. `bed_type` keeps the first plate's value as a best-effort archive default. Malformed `prediction` / `weight` values on individual plates skip cleanly rather than poison the sum. **Stats fix**: `compute_project_stats` and the `list_projects` quick-stats block both switch to an inner join `print_log_entries → print_archives` `WHERE archives.project_id = X`. `total_archives` becomes `COUNT(PrintLogEntry.id)` (actual runs, not files); `failed_prints` becomes the count of runs in `failed/aborted/cancelled/stopped`; `completed_items` becomes `SUM(PrintArchive.quantity)` filtered to runs with `status='completed'` (each run contributes its archive's quantity); `total_print_time_hours / total_filament_grams / estimated_cost / total_energy_*` come from `PrintLogEntry` columns. Orphan log rows (`archive_id IS NULL` after archive deletion via `ON DELETE SET NULL`) are excluded by the inner join — they can't be attributed to any project. **Backfill behaviour** (intentional, matches the reporter's "forward-only" note): users with AMS spool tracking — the reporter's case — have per-run `PrintLogEntry.filament_used_grams` from the tracked spool delta, not the plate-1 estimate, so project stats become correct *immediately* after the rollup fix with no reslice required. Users without tracking fall back to the archive estimate; their stats undercount until they reprint with the fixed parser. The Archive **card** still reads `PrintArchive.filament_used_grams` directly, so old archives keep their plate-1-only numbers until a reslice/rescan repopulates `file_metadata`. **Same-shape fix carried forward**: `system.py::system_info` (the System Info page's lifetime totals) summed `PrintArchive.print_time_seconds` / `filament_used_grams` with the identical bug — reprints collapsed to one archive, multi-plate files reported plate-1-only. The route now sums from `PrintLogEntry.duration_seconds` / `filament_used_grams` like the project rollup, so every run contributes its measured per-run actual. **Same-shape fix in the time-accuracy metric** (`archives.py::get_archive_stats`): the metric computed `estimate / actual` per run where `estimate = PrintArchive.print_time_seconds`. Post-parser-fix multi-plate archives have file-level estimate but per-run actual = one plate's duration → ratio ≈ N×100% for an N-plate file (300% for the reporter's 3-plate case), which would drag the printer-level average to noise. The calc now clamps each row to the [50%, 200%] plausibility band before contributing to the average; single-plate accuracy is fully included (the case the metric is designed for), multi-plate plate-by-plate runs and one-off outliers (manual intervention, purge waste blowing the estimate) are excluded. **Tests**: 4 new in `test_archive_service.py::TestMultiPlateSliceInfoSum` — three-plate file sums prediction + weight (the reporter's exact numerics: 7140+6000+6300 → 19440s, 19.2+20.0+18.8 → 58.0g); single-plate path preserves `plate_number` + objects + bed_type; multi-plate ignores per-plate object lists; malformed per-plate values are skipped without poisoning the sum. 4 new in `test_projects_api.py::TestProjectStatsPerRun` — 3 reprints show as 3 jobs with summed totals (matches the reporter's exact 3-run scenario); orphan log entries don't bleed into any project; mixed-outcome archive splits cleanly between `completed_prints` (quantity-weighted) and `failed_prints` (run-counted); list-view quick stats agree with per-project stats. 1 new in `test_archive_run_aggregation.py` — the accuracy band filter excludes multi-plate plate-by-plate runs (estimate 18000s / actual 6000s = 300%) so a single-plate file's near-100% reading stays the printer's average. Two pre-existing assertions updated to reflect the corrected semantics: `archive_count` and `total_archives` now count runs, so files attached but never printed (status `"archived"`) contribute 0 — that's the right answer, not a regression. Full backend suite + ruff clean. - **Webhook printer-status / stop / cancel routes 500'd on every connected printer because the route treated the PrinterState dataclass as a dict (#1584, reported via in-app bug report)** — Reporter saw `GET /api/v1/webhook/printer/{id}/status` return `500 Internal Server Error` with a valid API key carrying the `read_status` scope, while `GET /api/v1/system/info` returned 200 with the same key — so auth and routing were fine, the handler itself was crashing. Cause: `printer_manager.get_status(printer_id)` returns a `PrinterState` dataclass (`backend/app/services/bambu_mqtt.py`), not a dict. The route at `webhook.py:266-270` called `status.get("connected", False)`, `status.get("state")`, `status.get("current_print")`, `status.get("progress")`, `status.get("remaining_time")` — every one raised `AttributeError`, which Starlette surfaced as a generic 500. Reporter's id-1 (printer exists) returned 500; non-existent ids returned 404 — exactly because the early `Printer not found` branch fired before reaching the crash. Same shape in two adjacent routes: `webhook_stop_print` (`POST /printer/{id}/stop`) and `webhook_cancel_print` (`POST /printer/{id}/cancel`) checked `status.get("connected")` / `status.get("state")` for their precondition gates. 8 crash sites total across the three routes. **Fix**: every `status.get("X", default)` replaced with attribute access (`status.X if status else default`); Pydantic response schema unchanged. `PrinterState`'s dataclass defaults cleanly cover the `status is None` branch (printer registered but never connected — the route now returns 200 with `connected=false, state=null, …` rather than crashing). **Tests** (`backend/tests/integration/test_webhook_printer_status.py`): 7 new — status route returns 200 with the dataclass attributes mapped into the response (regression for the exact #1584 shape); status route returns 200 with sensible defaults when `get_status()` returns None; status route returns 404 for a non-existent printer (control case proving the auth path is unaffected); stop route returns 503 when disconnected (pre-fix would have 500'd here); stop route returns 409 when state is not `RUNNING`; cancel route returns 503 when disconnected; cancel route returns 409 when state is not `RUNNING`/`PAUSE`. Runtime-verified end-to-end against a live PG-backed instance before and after: same key + same printer id, 500 before the patch and 200 with the correct payload after. Full backend suite + ruff clean. - **Path-traversal CI backstop now recognises markers on the closing-paren line (project-wide convention)** — `test_no_unsafe_path_joins.py::test_route_path_arithmetic_is_safe_joined_or_marked` AST-walks every Path-arithmetic site in `api/routes/` + `services/` and demands either `safe_join_under(...)` or a `# SEC-PATH-OK: ` marker. The marker-detection helper only scanned the BinOp's own line range (`lineno..end_lineno`), but the project's convention puts the marker on the line of the wrapping closing paren — one past `end_lineno`. The backstop flagged 30 already-marked, already-safe sites as findings, masking the fact that the post-GHSA marker work is complete. The helper now peeks one line past `end_lineno` IF that line begins with a continuation token (`)`, `]`, `}`, `,`), capturing exactly this convention without giving a free pass to a marker on a wholly unrelated next statement. 5 new tests in `TestMarkerDetection` pin the contract: marker on the BinOp line recognised; marker on the closing-paren line recognised; an unrelated marker on a later statement does NOT silence; a marker on a non-continuation line right after the BinOp does NOT silence; no marker anywhere is still flagged. Integration test now passes against the existing tree — 30 findings → 0 — with no changes to any guard / sanitisation in routes or services. diff --git a/backend/app/api/routes/settings.py b/backend/app/api/routes/settings.py index 80505c2ce..04aa325bf 100644 --- a/backend/app/api/routes/settings.py +++ b/backend/app/api/routes/settings.py @@ -1124,10 +1124,14 @@ async def get_virtual_printer_settings( tailscale_disabled_raw = await get_setting(db, "virtual_printer_tailscale_disabled") archive_name_source = await get_setting(db, "virtual_printer_archive_name_source") + from backend.app.models.virtual_printer import VP_MODE_ARCHIVE, normalize_vp_mode + return { "enabled": enabled == "true" if enabled else False, "access_code_set": bool(access_code), - "mode": mode or "immediate", + # Normalize on read so older settings rows (with `immediate` / + # `print_queue`) come out as `archive` / `queue` for the frontend. + "mode": normalize_vp_mode(mode) or VP_MODE_ARCHIVE, "model": model or DEFAULT_VIRTUAL_PRINTER_MODEL, "target_printer_id": int(target_printer_id) if target_printer_id else None, "remote_interface_ip": remote_interface_ip or "", @@ -1168,7 +1172,9 @@ async def update_virtual_printer_settings( # Get current values current_enabled = await get_setting(db, "virtual_printer_enabled") == "true" current_access_code = await get_setting(db, "virtual_printer_access_code") or "" - current_mode = await get_setting(db, "virtual_printer_mode") or "immediate" + # Default to `archive` (the canonical name) but tolerate legacy `immediate` + # in the stored value — normalized later before validation. + current_mode = await get_setting(db, "virtual_printer_mode") or "archive" current_model = await get_setting(db, "virtual_printer_model") or DEFAULT_VIRTUAL_PRINTER_MODEL current_target_id_str = await get_setting(db, "virtual_printer_target_printer_id") current_target_id = int(current_target_id_str) if current_target_id_str else None @@ -1186,15 +1192,21 @@ async def update_virtual_printer_settings( new_remote_iface = remote_interface_ip if remote_interface_ip is not None else current_remote_iface new_ts_disabled = tailscale_disabled if tailscale_disabled is not None else current_ts_disabled - # Validate mode - # "review" is the new name for "queue" (pending review before archiving) - # "print_queue" archives and adds to print queue (unassigned) - # "proxy" is transparent TCP proxy to a real printer - if new_mode not in ("immediate", "queue", "review", "print_queue", "proxy"): + # Validate mode. Canonical wire values are `archive` / `review` / `queue` + # / `proxy`; legacy `immediate` and `print_queue` are accepted as aliases + # and translated before storage so support bundles stop showing the old + # confusing pair (#1429 mode-label discrepancy). + from backend.app.models.virtual_printer import VP_MODE_VALUES, normalize_vp_mode + + canonical_mode = normalize_vp_mode(new_mode) + if canonical_mode not in VP_MODE_VALUES: return JSONResponse( status_code=400, - content={"detail": "Mode must be 'immediate', 'review', 'print_queue', or 'proxy'"}, + content={ + "detail": f"Mode must be one of: {', '.join(VP_MODE_VALUES)}", + }, ) + new_mode = canonical_mode # Validate archive_name_source if archive_name_source is not None and archive_name_source not in ("metadata", "filename"): @@ -1202,9 +1214,6 @@ async def update_virtual_printer_settings( status_code=400, content={"detail": "archive_name_source must be 'metadata' or 'filename'"}, ) - # Normalize legacy "queue" to "review" for storage - if new_mode == "queue": - new_mode = "review" # Validate model if model is not None and model not in VIRTUAL_PRINTER_MODELS: diff --git a/backend/app/api/routes/virtual_printers.py b/backend/app/api/routes/virtual_printers.py index ec69f9aa0..5354df597 100644 --- a/backend/app/api/routes/virtual_printers.py +++ b/backend/app/api/routes/virtual_printers.py @@ -33,7 +33,7 @@ class TailscaleStatusResponse(BaseModel): class VirtualPrinterCreate(BaseModel): name: str = "Bambuddy" enabled: bool = False - mode: str = "immediate" + mode: str = "archive" model: str | None = None access_code: str | None = None target_printer_id: int | None = None @@ -133,12 +133,14 @@ async def create_virtual_printer( _: User | None = RequirePermissionIfAuthEnabled(Permission.SETTINGS_UPDATE), ): """Create a new virtual printer.""" - from backend.app.models.virtual_printer import VirtualPrinter + from backend.app.models.virtual_printer import VP_MODE_VALUES, VirtualPrinter, normalize_vp_mode from backend.app.services.virtual_printer import VIRTUAL_PRINTER_MODELS, virtual_printer_manager from backend.app.services.virtual_printer.manager import DEFAULT_VIRTUAL_PRINTER_MODEL - # Validate mode - if body.mode not in ("immediate", "review", "print_queue", "proxy"): + # Accept both canonical and legacy wire values so older clients (forks / + # mobile shortcuts / scripted setups) still work; normalize before write. + body.mode = normalize_vp_mode(body.mode) or body.mode + if body.mode not in VP_MODE_VALUES: return JSONResponse(status_code=400, content={"detail": "Invalid mode"}) # Validate model @@ -357,9 +359,12 @@ async def update_virtual_printer( if body.name is not None: vp.name = body.name if body.mode is not None: - if body.mode not in ("immediate", "review", "print_queue", "proxy"): + from backend.app.models.virtual_printer import VP_MODE_VALUES, normalize_vp_mode + + canonical_mode = normalize_vp_mode(body.mode) or body.mode + if canonical_mode not in VP_MODE_VALUES: return JSONResponse(status_code=400, content={"detail": "Invalid mode"}) - vp.mode = body.mode + vp.mode = canonical_mode if body.model is not None: if body.model not in VIRTUAL_PRINTER_MODELS: return JSONResponse( diff --git a/backend/app/core/database.py b/backend/app/core/database.py index dbdfffd10..443fd8da0 100644 --- a/backend/app/core/database.py +++ b/backend/app/core/database.py @@ -1659,9 +1659,18 @@ async def run_migrations(conn): result = await conn.execute(text("SELECT value FROM settings WHERE key = 'virtual_printer_mode'")) row = result.fetchone() - old_mode = row[0] if row else "immediate" + old_mode = row[0] if row else "archive" + # Translate to canonical wire values (#1429 mode-label + # discrepancy): legacy `immediate` → `archive`, legacy + # `print_queue` → `queue`. The historical `queue` alias + # for `review` predates the canonical rename and is + # preserved (existing user intent was "pending review"). if old_mode == "queue": old_mode = "review" + elif old_mode == "immediate": + old_mode = "archive" + elif old_mode == "print_queue": + old_mode = "queue" result = await conn.execute(text("SELECT value FROM settings WHERE key = 'virtual_printer_model'")) row = result.fetchone() @@ -1691,7 +1700,7 @@ async def run_migrations(conn): { "name": "Bambuddy", "enabled": old_enabled, - "mode": old_mode or "immediate", + "mode": old_mode or "archive", "model": old_model, "access_code": old_access_code, "target_id": old_target_id, @@ -1806,6 +1815,24 @@ async def run_migrations(conn): {"old": old_val, "new": new_val}, ) + # Migration: Rename VP mode wire values to match the user-facing labels + # (#1429 follow-up). The UI button "Archive" had always saved `immediate` + # and "Queue" had always saved `print_queue` — a mismatch that showed up + # confusingly in every support bundle. The button labels stay; the wire + # value is what changes. Idempotent: re-running the UPDATE on canonical + # values is a no-op. SQLite and Postgres both accept this statement + # unchanged (string literal comparison, no driver-specific syntax). + vp_mode_renames = [("immediate", "archive"), ("print_queue", "queue")] + for old_val, new_val in vp_mode_renames: + await conn.execute( + text("UPDATE virtual_printers SET mode = :new WHERE mode = :old"), + {"old": old_val, "new": new_val}, + ) + await conn.execute( + text("UPDATE settings SET value = :new WHERE key = 'virtual_printer_mode' AND value = :old"), + {"old": old_val, "new": new_val}, + ) + # Migration: Add per-user Bambu Cloud credential columns await _safe_execute(conn, "ALTER TABLE users ADD COLUMN cloud_token VARCHAR(500)") await _safe_execute(conn, "ALTER TABLE users ADD COLUMN cloud_email VARCHAR(255)") diff --git a/backend/app/models/virtual_printer.py b/backend/app/models/virtual_printer.py index f583cb2b0..4d7b1bc4b 100644 --- a/backend/app/models/virtual_printer.py +++ b/backend/app/models/virtual_printer.py @@ -5,6 +5,34 @@ from sqlalchemy.orm import Mapped, mapped_column from backend.app.core.database import Base +# Canonical VP mode values. The legacy values `immediate` (→ archive) and +# `print_queue` (→ queue) shipped before the UI labels were aligned with the +# wire format. `normalize_vp_mode()` translates input from either form and +# the DB migration in `core/database.py` rewrites existing rows once at boot. +VP_MODE_ARCHIVE = "archive" +VP_MODE_REVIEW = "review" +VP_MODE_QUEUE = "queue" +VP_MODE_PROXY = "proxy" +VP_MODE_VALUES = (VP_MODE_ARCHIVE, VP_MODE_REVIEW, VP_MODE_QUEUE, VP_MODE_PROXY) + +# Legacy → canonical map. Kept narrow on purpose so unrelated typos surface +# instead of getting silently re-pointed at a default. +_VP_MODE_ALIASES = { + "immediate": VP_MODE_ARCHIVE, + "print_queue": VP_MODE_QUEUE, +} + + +def normalize_vp_mode(value: str | None) -> str | None: + """Map legacy wire values (`immediate`, `print_queue`) to canonical names. + + Returns `None` unchanged so callers can decide whether to apply a default. + Returns unknown values unchanged so validators still see them and reject. + """ + if value is None: + return None + return _VP_MODE_ALIASES.get(value, value) + class VirtualPrinter(Base): """Virtual printer configuration for multi-instance support.""" @@ -14,13 +42,11 @@ class VirtualPrinter(Base): id: Mapped[int] = mapped_column(primary_key=True) name: Mapped[str] = mapped_column(String(100), default="Bambuddy") enabled: Mapped[bool] = mapped_column(Boolean, default=False) - mode: Mapped[str] = mapped_column(String(20), default="immediate") # immediate|review|print_queue|proxy - auto_dispatch: Mapped[bool] = mapped_column( - Boolean, server_default="true" - ) # print_queue mode: auto-start or manual + mode: Mapped[str] = mapped_column(String(20), default=VP_MODE_ARCHIVE) # archive|review|queue|proxy + auto_dispatch: Mapped[bool] = mapped_column(Boolean, server_default="true") # queue mode: auto-start or manual queue_force_color_match: Mapped[bool] = mapped_column( Boolean, server_default="false" - ) # print_queue mode: pin per-slot type+color from the 3MF onto the queue + ) # queue mode: pin per-slot type+color from the 3MF onto the queue # item so the scheduler refuses to dispatch onto a printer with the wrong # filament loaded (#1188). model: Mapped[str | None] = mapped_column(String(50), nullable=True) # SSDP model code (server mode) diff --git a/backend/app/schemas/settings.py b/backend/app/schemas/settings.py index d48f008ad..e7274ad75 100644 --- a/backend/app/schemas/settings.py +++ b/backend/app/schemas/settings.py @@ -113,8 +113,8 @@ class AppSettings(BaseModel): virtual_printer_enabled: bool = Field(default=False, description="Enable virtual printer for slicer uploads") virtual_printer_access_code: str = Field(default="", description="Access code for virtual printer authentication") virtual_printer_mode: str = Field( - default="immediate", - description="Mode: 'immediate' (archive now), 'review' (pending review), or 'print_queue' (add to print queue)", + default="archive", + description="Mode: 'archive' (archive now), 'review' (pending review), 'queue' (add to print queue), or 'proxy' (relay to real printer)", ) virtual_printer_archive_name_source: str = Field( default="metadata", diff --git a/backend/app/services/virtual_printer/manager.py b/backend/app/services/virtual_printer/manager.py index 192e11659..835492354 100644 --- a/backend/app/services/virtual_printer/manager.py +++ b/backend/app/services/virtual_printer/manager.py @@ -12,6 +12,11 @@ from pathlib import Path from typing import TYPE_CHECKING from backend.app.core.config import settings as app_settings +from backend.app.models.virtual_printer import ( + VP_MODE_ARCHIVE, + VP_MODE_QUEUE, + normalize_vp_mode, +) from backend.app.services.virtual_printer.bind_server import BindServer from backend.app.services.virtual_printer.certificate import CertificateService from backend.app.services.virtual_printer.ftp_server import VirtualPrinterFTPServer @@ -137,7 +142,11 @@ class VirtualPrinterInstance: ): self.id = vp_id self.name = name - self.mode = mode + # Normalize on construction so the rest of the code only compares + # canonical values, even when a legacy DB row hasn't been migrated + # yet (e.g. fresh-from-disk during the boot window before the + # one-shot migration in `core/database.py` has executed). + self.mode = normalize_vp_mode(mode) or VP_MODE_ARCHIVE self.model = model self.access_code = access_code self.serial_suffix = serial_suffix @@ -234,9 +243,14 @@ class VirtualPrinterInstance: self._pending_files[file_path.name] = file_path - if self.mode == "immediate": + # Accept both canonical (`archive`/`queue`) and legacy + # (`immediate`/`print_queue`) wire values so a stale row that hasn't + # been migrated yet still dispatches correctly. Migration in + # `core/database.py` rewrites existing rows once at boot. + mode = normalize_vp_mode(self.mode) + if mode == VP_MODE_ARCHIVE: await self._archive_file(file_path, source_ip) - elif self.mode == "print_queue": + elif mode == VP_MODE_QUEUE: await self._add_to_print_queue(file_path, source_ip) else: await self._queue_file(file_path, source_ip) @@ -267,13 +281,13 @@ class VirtualPrinterInstance: `flow_cali`, `vibration_cali`, `layer_inspect`, `use_ams`) so the VP-queue path can inherit them when adding the item to the queue, rather than falling back to the global default settings (#1403). - Only queue mode consumes the capture; immediate / review / proxy + Only queue mode consumes the capture; archive / review / proxy modes ignore the print command, so we skip the stash there to keep the dict from accumulating one entry per print over the VP's uptime. """ logger.info("[VP %s] Print command for: %s", self.name, filename) - if self.mode != "print_queue": + if normalize_vp_mode(self.mode) != VP_MODE_QUEUE: return # Drop the oldest stash if the cache is growing — happens when the # slicer sends project_file for a filename whose FTP upload was @@ -1085,8 +1099,12 @@ class VirtualPrinterManager: ): proxy_target_changed = True + # Normalize the DB value before comparing — a legacy `immediate` + # row read before the migration window finishes would otherwise + # trip the "changed" branch and bounce every VP at boot. + db_mode = normalize_vp_mode(vp.mode) changed = ( - instance.mode != vp.mode + instance.mode != db_mode or instance.model != (vp.model or DEFAULT_VIRTUAL_PRINTER_MODEL) or instance.access_code != (vp.access_code or "") or instance.bind_ip != (vp.bind_ip or "") @@ -1220,7 +1238,7 @@ class VirtualPrinterManager: return { "enabled": False, "running": False, - "mode": "immediate", + "mode": VP_MODE_ARCHIVE, "name": "Bambuddy", "serial": "", "model": DEFAULT_VIRTUAL_PRINTER_MODEL, @@ -1232,7 +1250,7 @@ class VirtualPrinterManager: self, enabled: bool, access_code: str = "", - mode: str = "immediate", + mode: str = VP_MODE_ARCHIVE, model: str = "", target_printer_ip: str = "", target_printer_serial: str = "", diff --git a/backend/app/services/virtual_printer/mqtt_bridge.py b/backend/app/services/virtual_printer/mqtt_bridge.py index 5a589110a..a95a0b245 100644 --- a/backend/app/services/virtual_printer/mqtt_bridge.py +++ b/backend/app/services/virtual_printer/mqtt_bridge.py @@ -282,6 +282,15 @@ class MQTTBridge: return if current is self._target_client: + # Same client object — but `ip_address` can fill in *after* the + # initial bind (e.g. DB row had a stale/empty value until the + # client's first SSDP-driven IP refresh). The original code only + # encoded `_target_ip_uint32_le` on client-identity change, so + # that late-arriving IP was never picked up, the `net.info[*].ip` + # rewrite stayed disabled, and the cache filled with the real + # printer IP — #1429. Refresh the encoding every tick so it + # self-heals once `ip_address` becomes valid. + self._refresh_ip_encoding() return # Client identity changed — unregister from the old, register on the new. @@ -297,20 +306,7 @@ class MQTTBridge: self._target_client = current self._target_serial = getattr(current, "serial_number", None) - - # Cache printer IP and VP bind IP encoded as little-endian uint32, so we - # can rewrite `net.info[*].ip` in cached push_status. BambuStudio reads - # that field for the FTP destination IP — without rewriting, the slicer - # bypasses the VP and FTPs straight to the real printer. - target_ip = getattr(current, "ip_address", None) - vp_ip = getattr(self._mqtt_server, "bind_address", None) - if target_ip and vp_ip and vp_ip not in ("0.0.0.0", "", None): # nosec B104 - try: - self._target_ip_uint32_le = _ip_to_uint32_le(target_ip) - self._vp_ip_uint32_le = _ip_to_uint32_le(vp_ip) - except ValueError: - self._target_ip_uint32_le = None - self._vp_ip_uint32_le = None + self._refresh_ip_encoding() logger.info( "[%s] MQTT bridge bound to printer %s (serial=%s)", @@ -348,6 +344,94 @@ class MQTTBridge: self._target_client = None self._target_serial = None + def _refresh_ip_encoding(self) -> None: + """(Re-)encode `_target_ip_uint32_le` / `_vp_ip_uint32_le` from current values. + + Called on every refresh tick, not just on client-identity change, so + a late-arriving printer IP (or a bind-address change) is picked up + without restarting the VP. When the encoding becomes valid for the + first time *after* the cache already received a push with the real + printer IP, also sweep the existing cache so the slicer's next pull + sees the rewritten value (#1429). Without this sweep the sticky-key + preservation keeps the poisoned `net.info[].ip` alive forever. + """ + client = self._target_client + if client is None: + return + + target_ip = getattr(client, "ip_address", None) + vp_ip = getattr(self._mqtt_server, "bind_address", None) + if not target_ip or not vp_ip or vp_ip in ("0.0.0.0", "", None): # nosec B104 + return + + try: + new_target_le = _ip_to_uint32_le(target_ip) + new_vp_le = _ip_to_uint32_le(vp_ip) + except ValueError: + return + + if new_target_le == self._target_ip_uint32_le and new_vp_le == self._vp_ip_uint32_le: + return # No change — nothing to do. + + # Encoding either became valid for the first time or shifted (DHCP + # renewal, bind_ip reconfigured, etc.). Update + sweep the cache. + was_armed = self._target_ip_uint32_le is not None and self._vp_ip_uint32_le is not None + self._target_ip_uint32_le = new_target_le + self._vp_ip_uint32_le = new_vp_le + logger.info( + "[%s] MQTT bridge IP encoding %s: target=%s vp=%s", + self.vp_name, + "updated" if was_armed else "armed", + target_ip, + vp_ip, + ) + + cached = self._latest_print_state + if isinstance(cached, dict): + n = self._rewrite_net_info_ips(cached) + if n: + logger.info( + "[%s] MQTT bridge swept %d net.info[].ip entries in cached push", + self.vp_name, + n, + ) + + def _rewrite_net_info_ips(self, print_state: dict) -> int: + """Rewrite every non-zero `net.info[].ip` in `print_state` to the VP bind IP. + + Returns the number of entries rewritten. Mutates `print_state` in place. + + Strategy: rewrite ALL entries with a non-zero `ip`, not only those + matching `_target_ip_uint32_le`. Real printers (X1C, H2D Pro) can + report multiple active interfaces (WiFi + Ethernet) with different + IPs — only one matches the IP Bambuddy tracks, but the slicer may + read any of them. Leaving non-matching entries pointing at real + printer interfaces leaks an FTP fallback path that bypasses the VP + (the #1429 / #1302 symptom). Entries with `ip == 0` are placeholders + for unpopulated interfaces — leave them alone so the slicer's + "active interface" detection still recognises them as absent. + """ + if self._vp_ip_uint32_le is None: + return 0 + net = print_state.get("net") + if not isinstance(net, dict): + return 0 + info = net.get("info") + if not isinstance(info, list): + return 0 + rewritten = 0 + for entry in info: + if not isinstance(entry, dict): + continue + ip_value = entry.get("ip") + if not isinstance(ip_value, int) or ip_value == 0: + continue + if ip_value == self._vp_ip_uint32_le: + continue + entry["ip"] = self._vp_ip_uint32_le + rewritten += 1 + return rewritten + def _on_printer_raw(self, topic: str, payload: bytes) -> None: """Paho-thread callback — cache the latest push_status for synthetic replay. @@ -396,14 +480,7 @@ class MQTTBridge: # (i.e. configure the VP with the same access code as its target). # Rewrite real printer IP → VP bind IP in `net.info[*].ip` so the # slicer's FTP destination resolves to the VP, not the real printer. - if self._target_ip_uint32_le is not None and self._vp_ip_uint32_le is not None: - net = print_data.get("net") - if isinstance(net, dict): - info = net.get("info") - if isinstance(info, list): - for entry in info: - if isinstance(entry, dict) and entry.get("ip") == self._target_ip_uint32_le: - entry["ip"] = self._vp_ip_uint32_le + self._rewrite_net_info_ips(print_data) # Defensive deep copy on store so the cache is fully decoupled from # the freshly-parsed tree and from any reader's reference. new_state = copy.deepcopy(print_data) diff --git a/backend/tests/integration/test_virtual_printer_api.py b/backend/tests/integration/test_virtual_printer_api.py index 3307709f3..b5752292c 100644 --- a/backend/tests/integration/test_virtual_printer_api.py +++ b/backend/tests/integration/test_virtual_printer_api.py @@ -61,33 +61,46 @@ class TestVirtualPrinterSettingsAPI: @pytest.mark.asyncio @pytest.mark.integration - async def test_update_mode_to_print_queue(self, async_client: AsyncClient): - """Verify mode can be set to print_queue.""" - response = await async_client.put("/api/v1/settings/virtual-printer?mode=print_queue") - - assert response.status_code == 200 - result = response.json() - assert result["mode"] == "print_queue" - - @pytest.mark.asyncio - @pytest.mark.integration - async def test_update_mode_legacy_queue_maps_to_review(self, async_client: AsyncClient): - """Verify legacy 'queue' mode is normalized to 'review'.""" + async def test_update_mode_to_queue(self, async_client: AsyncClient): + """Verify mode can be set to the canonical 'queue' value.""" response = await async_client.put("/api/v1/settings/virtual-printer?mode=queue") assert response.status_code == 200 result = response.json() - assert result["mode"] == "review" # Legacy queue maps to review + assert result["mode"] == "queue" @pytest.mark.asyncio @pytest.mark.integration - async def test_update_mode_to_immediate(self, async_client: AsyncClient): - """Verify mode can be set to immediate.""" + async def test_update_mode_legacy_print_queue_normalises_to_queue(self, async_client: AsyncClient): + """Legacy `print_queue` is accepted on input and translated to `queue` on + storage so the UI button label and the support-bundle field agree + (#1429 mode-label discrepancy).""" + response = await async_client.put("/api/v1/settings/virtual-printer?mode=print_queue") + + assert response.status_code == 200 + result = response.json() + assert result["mode"] == "queue" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_update_mode_legacy_immediate_normalises_to_archive(self, async_client: AsyncClient): + """Legacy `immediate` is accepted on input and translated to `archive` + on storage (#1429 mode-label discrepancy).""" response = await async_client.put("/api/v1/settings/virtual-printer?mode=immediate") assert response.status_code == 200 result = response.json() - assert result["mode"] == "immediate" + assert result["mode"] == "archive" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_update_mode_to_archive(self, async_client: AsyncClient): + """Verify mode can be set to the canonical 'archive' value.""" + response = await async_client.put("/api/v1/settings/virtual-printer?mode=archive") + + assert response.status_code == 200 + result = response.json() + assert result["mode"] == "archive" @pytest.mark.asyncio @pytest.mark.integration @@ -136,7 +149,7 @@ class TestVirtualPrinterSettingsAPI: return_value={ "enabled": True, "running": True, - "mode": "immediate", + "mode": "archive", "name": "Bambuddy", "serial": "00M09A391800001", "pending_files": 0, @@ -157,7 +170,7 @@ class TestVirtualPrinterSettingsAPI: return_value={ "enabled": False, "running": False, - "mode": "immediate", + "mode": "archive", "name": "Bambuddy", "serial": "00M09A391800001", "pending_files": 0, @@ -283,7 +296,7 @@ class TestVirtualPrinterAutoDispatchAPI: "/api/v1/virtual-printers", json={ "name": "TestDefaultDispatch", - "mode": "print_queue", + "mode": "queue", "access_code": "12345678", }, ) @@ -300,7 +313,7 @@ class TestVirtualPrinterAutoDispatchAPI: "/api/v1/virtual-printers", json={ "name": "TestManualDispatch", - "mode": "print_queue", + "mode": "queue", "access_code": "12345678", "auto_dispatch": False, }, @@ -319,7 +332,7 @@ class TestVirtualPrinterAutoDispatchAPI: "/api/v1/virtual-printers", json={ "name": "TestToggleDispatch", - "mode": "print_queue", + "mode": "queue", "access_code": "12345678", }, ) @@ -359,7 +372,7 @@ class TestVirtualPrinterTailscaleToggleAPI: "/api/v1/virtual-printers", json={ "name": "TestTailscaleToggle", - "mode": "immediate", + "mode": "archive", "access_code": "12345678", }, ) @@ -426,7 +439,7 @@ class TestVirtualPrinterDiagnosticAPI: """A freshly created (disabled) VP fails the 'enabled' check.""" create_resp = await async_client.post( "/api/v1/virtual-printers", - json={"name": "TestDiagVP", "mode": "immediate", "access_code": "12345678"}, + json={"name": "TestDiagVP", "mode": "archive", "access_code": "12345678"}, ) assert create_resp.status_code == 200 vp_id = create_resp.json()["id"] diff --git a/backend/tests/unit/services/test_virtual_printer.py b/backend/tests/unit/services/test_virtual_printer.py index 0c30e292f..242a12162 100644 --- a/backend/tests/unit/services/test_virtual_printer.py +++ b/backend/tests/unit/services/test_virtual_printer.py @@ -47,7 +47,7 @@ class TestVirtualPrinterInstance: return VirtualPrinterInstance( vp_id=1, name="TestPrinter", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix="391800001", @@ -62,7 +62,7 @@ class TestVirtualPrinterInstance: """Verify constructor stores parameters correctly.""" assert instance.id == 1 assert instance.name == "TestPrinter" - assert instance.mode == "immediate" + assert instance.mode == "archive" assert instance.model == "C11" assert instance.access_code == "12345678" assert instance.serial_suffix == "391800001" @@ -79,7 +79,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=2, name="X1C", - mode="immediate", + mode="archive", model="BL-P001", access_code="12345678", serial_suffix="391800002", @@ -182,7 +182,7 @@ class TestVirtualPrinterInstance: Send-flow slicers don't watch the post-upload state, so this is a no-op behavior change for them. """ - instance.mode = "immediate" + instance.mode = "archive" instance._mqtt = MagicMock() instance._mqtt.set_gcode_state = MagicMock() file_path = Path("/tmp/test.3mf") # nosec B108 @@ -196,7 +196,7 @@ class TestVirtualPrinterInstance: async def test_on_file_received_non_3mf_does_not_touch_state(self, instance): """Non-3MF uploads (e.g., a job's auxiliary files) must not transition the visible state — the slicer is only tracking the .3mf upload.""" - instance.mode = "immediate" + instance.mode = "archive" instance._mqtt = MagicMock() instance._mqtt.set_gcode_state = MagicMock() file_path = Path("/tmp/test.gcode") # nosec B108 @@ -236,7 +236,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=30, name="ImmediateBroadcast", - mode="immediate", + mode="archive", model="C12", access_code="12345678", serial_suffix="391800030", @@ -289,7 +289,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=10, name="DefaultDispatch", - mode="print_queue", + mode="queue", model="C11", access_code="12345678", serial_suffix="391800010", @@ -320,7 +320,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=11, name="AutoDispatchOn", - mode="print_queue", + mode="queue", model="C11", access_code="12345678", serial_suffix="391800011", @@ -374,7 +374,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=31, name="QueueBroadcast", - mode="print_queue", + mode="queue", model="C12", access_code="12345678", serial_suffix="391800031", @@ -440,7 +440,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=12, name="AutoDispatchOff", - mode="print_queue", + mode="queue", model="C11", access_code="12345678", serial_suffix="391800012", @@ -499,7 +499,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=22, name="DefaultsTest", - mode="print_queue", + mode="queue", model="C12", access_code="12345678", serial_suffix="391800022", @@ -572,7 +572,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=23, name="FreshInstallDefaults", - mode="print_queue", + mode="queue", model="C12", access_code="12345678", serial_suffix="391800023", @@ -636,7 +636,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=24, name="SlicerInherits", - mode="print_queue", + mode="queue", model="C12", access_code="12345678", serial_suffix="391800024", @@ -723,7 +723,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=25, name="SlicerIntegers", - mode="print_queue", + mode="queue", model="C12", access_code="12345678", serial_suffix="391800025", @@ -787,7 +787,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=21, name="Reqs", - mode="print_queue", + mode="queue", model="C12", access_code="12345678", serial_suffix="391800021", @@ -857,7 +857,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=22, name="ForceColor", - mode="print_queue", + mode="queue", model="C12", access_code="12345678", serial_suffix="391800022", @@ -927,7 +927,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=23, name="Unparseable", - mode="print_queue", + mode="queue", model="C12", access_code="12345678", serial_suffix="391800023", @@ -996,7 +996,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=20, name="NameSource", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix="391800020", @@ -1058,7 +1058,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=40, name="ArchiveFailCleanup", - mode="immediate", + mode="archive", model="C12", access_code="12345678", serial_suffix="391800040", @@ -1140,7 +1140,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=42, name="DispatchFailCleanup", - mode="print_queue", + mode="queue", model="C12", access_code="12345678", serial_suffix="391800042", @@ -1201,7 +1201,7 @@ class TestVirtualPrinterInstance: inst = VirtualPrinterInstance( vp_id=43, name="PositionMaxPlusOne", - mode="print_queue", + mode="queue", model="C12", access_code="12345678", serial_suffix="391800043", @@ -1265,7 +1265,7 @@ class TestVirtualPrinterManager: status = manager.get_status() assert status["enabled"] is False assert status["running"] is False - assert status["mode"] == "immediate" + assert status["mode"] == "archive" def test_manager_is_enabled_with_instance(self, manager, tmp_path): """Verify is_enabled is True when instances exist.""" @@ -1274,7 +1274,7 @@ class TestVirtualPrinterManager: inst = VirtualPrinterInstance( vp_id=1, name="Test", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix="391800001", @@ -1291,7 +1291,7 @@ class TestVirtualPrinterManager: inst = VirtualPrinterInstance( vp_id=1, name="Test", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix="391800001", @@ -1335,7 +1335,7 @@ class TestVirtualPrinterManager: inst = VirtualPrinterInstance( vp_id=1, name="Bambuddy", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix="391800001", @@ -1349,7 +1349,7 @@ class TestVirtualPrinterManager: status = manager.get_status() assert status["enabled"] is True assert status["running"] is True - assert status["mode"] == "immediate" + assert status["mode"] == "archive" assert status["name"] == "Bambuddy" assert status["serial"] == "01S00A391800001" assert status["model"] == "C11" @@ -1364,7 +1364,7 @@ class TestVirtualPrinterManager: inst = VirtualPrinterInstance( vp_id=i, name=f"VP{i}", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix=f"39180000{i}", @@ -1386,7 +1386,7 @@ class TestVirtualPrinterManager: inst = VirtualPrinterInstance( vp_id=i, name=f"VP{i}", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix=f"39180000{i}", @@ -1408,7 +1408,7 @@ class TestVirtualPrinterManager: "id": 1, "name": "TestVP", "enabled": True, - "mode": "immediate", + "mode": "archive", "model": "C11", "access_code": "12345678", "serial_suffix": "391800001", @@ -1447,7 +1447,7 @@ class TestVirtualPrinterManager: inst = VirtualPrinterInstance( vp_id=1, name="TestVP", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix="391800001", @@ -1456,8 +1456,8 @@ class TestVirtualPrinterManager: inst.stop_server = AsyncMock() manager._instances[1] = inst - # DB says mode changed to "archive" - db_vp = self._make_db_vp(mode="archive") + # DB says mode changed to "review" + db_vp = self._make_db_vp(mode="review") self._setup_sync_mocks(manager, [db_vp], tmp_path) with patch.object(manager, "remove_instance", new_callable=AsyncMock) as mock_remove: @@ -1479,7 +1479,7 @@ class TestVirtualPrinterManager: inst = VirtualPrinterInstance( vp_id=1, name="TestVP", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix="391800001", @@ -1509,7 +1509,7 @@ class TestVirtualPrinterManager: inst = VirtualPrinterInstance( vp_id=1, name="TestVP", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix="391800001", @@ -1534,7 +1534,7 @@ class TestVirtualPrinterManager: inst = VirtualPrinterInstance( vp_id=1, name="TestVP", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix="391800001", @@ -1565,7 +1565,7 @@ class TestVirtualPrinterManager: inst = VirtualPrinterInstance( vp_id=1, name="TestVP", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix="391800001", @@ -1600,7 +1600,7 @@ class TestVirtualPrinterManager: inst = VirtualPrinterInstance( vp_id=1, name="TestVP", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix="391800001", @@ -2312,7 +2312,7 @@ class TestVirtualPrinterManagerDirectories: VirtualPrinterInstance( vp_id=42, name="Test", - mode="immediate", + mode="archive", model="C11", access_code="12345678", serial_suffix="391800042", @@ -2402,7 +2402,7 @@ class TestVirtualPrinterInstanceIPOverride: return VirtualPrinterInstance( vp_id=20, name="IPTest", - mode="immediate", + mode="archive", model="BL-P001", access_code="12345678", serial_suffix="391800020", @@ -2439,7 +2439,7 @@ class TestVirtualPrinterInstanceIPOverride: inst = VirtualPrinterInstance( vp_id=21, name="NoRemote", - mode="immediate", + mode="archive", model="BL-P001", access_code="12345678", serial_suffix="391800021", @@ -2465,7 +2465,7 @@ class TestVirtualPrinterInstanceIPOverride: inst = VirtualPrinterInstance( vp_id=22, name="NoIPs", - mode="immediate", + mode="archive", model="BL-P001", access_code="12345678", serial_suffix="391800022", @@ -2598,7 +2598,7 @@ class TestBindServer: inst = VirtualPrinterInstance( vp_id=99, name="Bambuddy", - mode="immediate", + mode="archive", model="BL-P001", access_code="12345678", serial_suffix="391800099", diff --git a/backend/tests/unit/services/test_vp_diagnostic.py b/backend/tests/unit/services/test_vp_diagnostic.py index a410fde9b..7d9e59a73 100644 --- a/backend/tests/unit/services/test_vp_diagnostic.py +++ b/backend/tests/unit/services/test_vp_diagnostic.py @@ -19,7 +19,7 @@ def _vp(**overrides): base = { "id": 1, "name": "Test VP", - "mode": "immediate", + "mode": "archive", "enabled": True, "bind_ip": "192.168.1.50", "access_code": "12345678", diff --git a/backend/tests/unit/test_vp_mode_rename_migration.py b/backend/tests/unit/test_vp_mode_rename_migration.py new file mode 100644 index 000000000..4d5f606b5 --- /dev/null +++ b/backend/tests/unit/test_vp_mode_rename_migration.py @@ -0,0 +1,151 @@ +"""Regression test for the VP mode wire-value rename migration (#1429 follow-up). + +The UI buttons "Archive" and "Queue" had always saved the wire values +`immediate` and `print_queue` — confusing in every support bundle. The +rename migration in ``run_migrations`` rewrites existing rows to the +canonical names. This test verifies it on both fresh and legacy schemas +and confirms it's idempotent so reruns are safe (boot-on-boot). +""" + +from __future__ import annotations + +import pytest +from sqlalchemy import text +from sqlalchemy.ext.asyncio import create_async_engine + +from backend.app.core.database import run_migrations + + +@pytest.fixture(autouse=True) +def force_sqlite_dialect(monkeypatch): + """Force the SQLite branch regardless of test env settings.""" + from backend.app.core import db_dialect + + monkeypatch.setattr(db_dialect, "is_sqlite", lambda: True) + monkeypatch.setattr(db_dialect, "is_postgres", lambda: False) + from backend.app.core import database as database_module + + monkeypatch.setattr(database_module, "is_sqlite", lambda: True) + + +def _register_all_models(): + """run_migrations touches multiple tables; the full schema must exist.""" + from backend.app.models import ( # noqa: F401 + ams_history, + ams_label, + api_key, + archive, + color_catalog, + external_link, + filament, + group, + kprofile_note, + maintenance, + notification, + notification_template, + print_log, + print_queue, + printer, + project, + project_bom, + settings, + slot_preset, + smart_plug, + smart_plug_energy_snapshot, + spool, + spool_assignment, + spool_catalog, + spool_k_profile, + spool_usage_history, + spoolbuddy_device, + user, + user_email_pref, + virtual_printer, + ) + + +@pytest.fixture +async def engine(): + from backend.app.core.database import Base + + _register_all_models() + + eng = create_async_engine("sqlite+aiosqlite:///:memory:", echo=False) + async with eng.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + yield eng + await eng.dispose() + + +@pytest.mark.asyncio +async def test_legacy_mode_rows_get_canonical_names(engine): + """Existing rows with `immediate` / `print_queue` get rewritten to + `archive` / `queue` while canonical values and unrelated modes pass + through untouched.""" + async with engine.begin() as conn: + await conn.execute( + text( + "INSERT INTO virtual_printers (id, name, enabled, mode, serial_suffix, position) VALUES " + "(1, 'A', 0, 'immediate', '391800001', 1)," + "(2, 'B', 0, 'print_queue', '391800002', 2)," + "(3, 'C', 0, 'review', '391800003', 3)," + "(4, 'D', 0, 'proxy', '391800004', 4)," + "(5, 'E', 0, 'archive', '391800005', 5)," + "(6, 'F', 0, 'queue', '391800006', 6)" + ) + ) + + async with engine.begin() as conn: + await run_migrations(conn) + + async with engine.connect() as conn: + result = await conn.execute(text("SELECT id, mode FROM virtual_printers ORDER BY id")) + rows = dict(result.fetchall()) + + assert rows[1] == "archive" # immediate → archive + assert rows[2] == "queue" # print_queue → queue + assert rows[3] == "review" # untouched + assert rows[4] == "proxy" # untouched + assert rows[5] == "archive" # already canonical + assert rows[6] == "queue" # already canonical + + +@pytest.mark.asyncio +async def test_legacy_settings_row_gets_canonical_name(engine): + """The legacy single-VP `virtual_printer_mode` setting also gets renamed + so the GET response (which feeds the support bundle and the settings + page) reads the canonical name.""" + async with engine.begin() as conn: + await conn.execute(text("INSERT INTO settings (key, value) VALUES ('virtual_printer_mode', 'immediate')")) + + async with engine.begin() as conn: + await run_migrations(conn) + + async with engine.connect() as conn: + result = await conn.execute(text("SELECT value FROM settings WHERE key = 'virtual_printer_mode'")) + value = result.scalar() + + assert value == "archive" + + +@pytest.mark.asyncio +async def test_migration_is_idempotent(engine): + """Running the migration twice must be a no-op on canonical values — + every boot re-runs the migration set.""" + async with engine.begin() as conn: + await conn.execute( + text( + "INSERT INTO virtual_printers (id, name, enabled, mode, serial_suffix, position) " + "VALUES (1, 'A', 0, 'immediate', '391800001', 1)" + ) + ) + + async with engine.begin() as conn: + await run_migrations(conn) + # Second run on already-canonical values. + async with engine.begin() as conn: + await run_migrations(conn) + + async with engine.connect() as conn: + result = await conn.execute(text("SELECT mode FROM virtual_printers WHERE id = 1")) + assert result.scalar() == "archive" diff --git a/backend/tests/unit/test_vp_mqtt_bridge.py b/backend/tests/unit/test_vp_mqtt_bridge.py index 0e98e8689..28d81a50a 100644 --- a/backend/tests/unit/test_vp_mqtt_bridge.py +++ b/backend/tests/unit/test_vp_mqtt_bridge.py @@ -220,6 +220,95 @@ class TestPushStatusCache: await bridge.stop() + @pytest.mark.asyncio + async def test_net_info_ip_rewritten_for_unknown_secondary_interface(self): + """Regression for #1429: real printers (X1C / H2D Pro) report multiple + active interfaces (WiFi + Ethernet) — only ONE matches the IP Bambuddy + tracks. The rewrite must catch every non-zero entry, not just the one + whose IP equals `_target_ip_uint32_le`, or the slicer's FTP fallback + path leaks straight to the real printer.""" + server = _make_server(bind_address=VP_IP) + bridge = _make_bridge(server) + await bridge.start() + + h2d_le = _ip_to_uint32_le(H2D_IP) + # A second IP Bambuddy never saw (e.g. printer's ethernet interface + # while Bambuddy talks over wifi). + other_le = _ip_to_uint32_le("192.168.99.42") + vp_le = _ip_to_uint32_le(VP_IP) + payload = json.dumps( + { + "print": { + "command": "push_status", + "net": { + "info": [ + {"ip": h2d_le, "mask": 0xFFFFFF}, + {"ip": other_le, "mask": 0xFFFFFF}, + {"ip": 0, "mask": 0}, + ] + }, + } + } + ).encode() + bridge._on_printer_raw(f"device/{H2D_SERIAL}/report", payload) + await asyncio.sleep(0.01) + + cached = bridge.get_latest_print_state() + assert cached["net"]["info"][0]["ip"] == vp_le + assert cached["net"]["info"][1]["ip"] == vp_le # secondary interface also rewritten + assert cached["net"]["info"][2]["ip"] == 0 # placeholder untouched + + await bridge.stop() + + @pytest.mark.asyncio + async def test_late_arriving_printer_ip_rewrites_existing_cache(self): + """Regression for #1429: if the printer's `ip_address` is empty at + first bind (DB row stale, or the client object exists before the + first SSDP refresh fills it in), the rewrite stays disabled and the + first cached push poisons the cache with the real-printer IP. + Once `ip_address` becomes valid, the next refresh tick must (a) arm + the encoding and (b) sweep the cached `net.info[].ip` so the slicer + sees the rewritten value on its next pull. Without the sweep the + sticky-key preservation keeps the poisoned value alive across + every subsequent incremental push.""" + server = _make_server(bind_address=VP_IP) + # Bind to a client whose ip_address is empty at start — simulates the + # late-arrival path. + target = _make_paho_client(ip="") + bridge = _make_bridge(server, target) + await bridge.start() + assert bridge._target_ip_uint32_le is None # not yet armed + + h2d_le = _ip_to_uint32_le(H2D_IP) + vp_le = _ip_to_uint32_le(VP_IP) + payload = json.dumps( + { + "print": { + "command": "push_status", + "net": {"info": [{"ip": h2d_le, "mask": 0xFFFFFF}]}, + } + } + ).encode() + bridge._on_printer_raw(f"device/{H2D_SERIAL}/report", payload) + await asyncio.sleep(0.01) + + # First push landed before encoding was armed → cache holds real IP. + cached = bridge.get_latest_print_state() + assert cached["net"]["info"][0]["ip"] == h2d_le + + # Printer's IP becomes known. Next refresh tick must self-heal. + target.ip_address = H2D_IP + bridge._resolve_client() + + cached = bridge.get_latest_print_state() + assert cached["net"]["info"][0]["ip"] == vp_le, ( + "cache must be swept once encoding becomes valid; sticky-key " + "preservation would otherwise keep the poisoned IP forever" + ) + assert bridge._target_ip_uint32_le == h2d_le + + await bridge.stop() + @pytest.mark.asyncio async def test_request_topic_message_is_ignored(self): server = _make_server() diff --git a/frontend/src/__tests__/components/VirtualPrinterCard.test.tsx b/frontend/src/__tests__/components/VirtualPrinterCard.test.tsx index 0ddc122d5..14a1c1f98 100644 --- a/frontend/src/__tests__/components/VirtualPrinterCard.test.tsx +++ b/frontend/src/__tests__/components/VirtualPrinterCard.test.tsx @@ -46,7 +46,7 @@ const createMockPrinter = (overrides: Partial = {}): Virtu id: 1, name: 'Test VP', enabled: false, - mode: 'immediate', + mode: 'archive', model: 'BL-P001', model_name: 'X1C', access_code_set: false, @@ -68,7 +68,7 @@ describe('VirtualPrinterCard - auto-dispatch toggle', () => { }); it('renders auto-dispatch toggle when mode is print_queue', async () => { - const printer = createMockPrinter({ mode: 'print_queue' }); + const printer = createMockPrinter({ mode: 'queue' }); render(); await waitFor(() => { @@ -77,7 +77,7 @@ describe('VirtualPrinterCard - auto-dispatch toggle', () => { }); it('does not render auto-dispatch toggle when mode is immediate', async () => { - const printer = createMockPrinter({ mode: 'immediate' }); + const printer = createMockPrinter({ mode: 'archive' }); render(); // Wait for the card to render fully (check for something that should be there) @@ -100,7 +100,7 @@ describe('VirtualPrinterCard - auto-dispatch toggle', () => { }); it('auto-dispatch toggle defaults to on', async () => { - const printer = createMockPrinter({ mode: 'print_queue', auto_dispatch: true }); + const printer = createMockPrinter({ mode: 'queue', auto_dispatch: true }); render(); await waitFor(() => { @@ -118,9 +118,9 @@ describe('VirtualPrinterCard - auto-dispatch toggle', () => { it('clicking auto-dispatch toggle calls update API', async () => { const user = userEvent.setup(); - const printer = createMockPrinter({ mode: 'print_queue', auto_dispatch: true }); + const printer = createMockPrinter({ mode: 'queue', auto_dispatch: true }); vi.mocked(multiVirtualPrinterApi.update).mockResolvedValue( - createMockPrinter({ mode: 'print_queue', auto_dispatch: false }) + createMockPrinter({ mode: 'queue', auto_dispatch: false }) ); render(); @@ -156,7 +156,7 @@ describe('VirtualPrinterCard - force color match toggle (#1188)', () => { }); it('renders force-color-match toggle when mode is print_queue', async () => { - const printer = createMockPrinter({ mode: 'print_queue' }); + const printer = createMockPrinter({ mode: 'queue' }); render(); await waitFor(() => { @@ -165,7 +165,7 @@ describe('VirtualPrinterCard - force color match toggle (#1188)', () => { }); it('does not render force-color-match toggle when mode is immediate', async () => { - const printer = createMockPrinter({ mode: 'immediate' }); + const printer = createMockPrinter({ mode: 'archive' }); render(); await waitFor(() => { @@ -185,7 +185,7 @@ describe('VirtualPrinterCard - force color match toggle (#1188)', () => { }); it('force-color-match toggle defaults off (not green) — preserves pre-fix behaviour', async () => { - const printer = createMockPrinter({ mode: 'print_queue', queue_force_color_match: false }); + const printer = createMockPrinter({ mode: 'queue', queue_force_color_match: false }); render(); await waitFor(() => { @@ -201,7 +201,7 @@ describe('VirtualPrinterCard - force color match toggle (#1188)', () => { }); it('force-color-match toggle renders enabled (green) when queue_force_color_match is true', async () => { - const printer = createMockPrinter({ mode: 'print_queue', queue_force_color_match: true }); + const printer = createMockPrinter({ mode: 'queue', queue_force_color_match: true }); render(); await waitFor(() => { @@ -216,9 +216,9 @@ describe('VirtualPrinterCard - force color match toggle (#1188)', () => { it('clicking force-color-match toggle posts queue_force_color_match in update body', async () => { const user = userEvent.setup(); - const printer = createMockPrinter({ mode: 'print_queue', queue_force_color_match: false }); + const printer = createMockPrinter({ mode: 'queue', queue_force_color_match: false }); vi.mocked(multiVirtualPrinterApi.update).mockResolvedValue( - createMockPrinter({ mode: 'print_queue', queue_force_color_match: true }) + createMockPrinter({ mode: 'queue', queue_force_color_match: true }) ); render(); diff --git a/frontend/src/__tests__/components/VirtualPrinterDiagnosticModal.test.tsx b/frontend/src/__tests__/components/VirtualPrinterDiagnosticModal.test.tsx index 65a0e7bb7..c1ebfd206 100644 --- a/frontend/src/__tests__/components/VirtualPrinterDiagnosticModal.test.tsx +++ b/frontend/src/__tests__/components/VirtualPrinterDiagnosticModal.test.tsx @@ -14,7 +14,7 @@ import type { VPDiagnosticResult } from '../../api/client'; const problemResult: VPDiagnosticResult = { vp_id: 3, vp_name: 'Garage VP', - mode: 'immediate', + mode: 'archive', overall: 'problems', checks: [ { id: 'enabled', status: 'pass', params: {} }, diff --git a/frontend/src/__tests__/components/VirtualPrinterSettings.test.tsx b/frontend/src/__tests__/components/VirtualPrinterSettings.test.tsx index 6c78a998f..7d69b21ac 100644 --- a/frontend/src/__tests__/components/VirtualPrinterSettings.test.tsx +++ b/frontend/src/__tests__/components/VirtualPrinterSettings.test.tsx @@ -42,14 +42,14 @@ import { virtualPrinterApi } from '../../api/client'; const createMockSettings = (overrides = {}) => ({ enabled: false, access_code_set: false, - mode: 'immediate' as const, + mode: 'archive' as const, model: 'BL-P001', target_printer_id: null as number | null, remote_interface_ip: null as string | null, status: { enabled: false, running: false, - mode: 'immediate', + mode: 'archive', name: 'Bambuddy', serial: '00M00A391800001', model: 'BL-P001', @@ -155,7 +155,7 @@ describe('VirtualPrinterSettings', () => { status: { enabled: true, running: true, - mode: 'immediate', + mode: 'archive', name: 'Bambuddy', serial: '00M00A391800001', model: 'BL-P001', @@ -304,16 +304,33 @@ describe('VirtualPrinterSettings', () => { }); }); - it('highlights current mode (legacy queue maps to review)', async () => { + it('highlights current mode (legacy immediate maps to archive button)', async () => { + // Pre-#1429 the wire value was `immediate` but the UI button was + // labeled "Archive". Backend migration rewrites stored rows, but a + // stale-cached settings payload may still carry the legacy value — + // the component normalises it client-side so the right button lights up. vi.mocked(virtualPrinterApi.getSettings).mockResolvedValue( - createMockSettings({ mode: 'queue' }) + createMockSettings({ mode: 'immediate' }) ); render(); await waitFor(() => { - const reviewButton = screen.getByText('Review').closest('button'); - expect(reviewButton?.className).toContain('border-bambu-green'); + const archiveButton = screen.getByText('Archive').closest('button'); + expect(archiveButton?.className).toContain('border-bambu-green'); + }); + }); + + it('highlights current mode (legacy print_queue maps to queue button)', async () => { + vi.mocked(virtualPrinterApi.getSettings).mockResolvedValue( + createMockSettings({ mode: 'print_queue' }) + ); + + render(); + + await waitFor(() => { + const queueButton = screen.getByText('Queue').closest('button'); + expect(queueButton?.className).toContain('border-bambu-green'); }); }); @@ -339,10 +356,10 @@ describe('VirtualPrinterSettings', () => { }); }); - it('changes mode to print_queue on click', async () => { + it('changes mode to queue on click', async () => { const user = userEvent.setup(); vi.mocked(virtualPrinterApi.updateSettings).mockResolvedValue( - createMockSettings({ mode: 'print_queue' }) + createMockSettings({ mode: 'queue' }) ); render(); @@ -357,7 +374,7 @@ describe('VirtualPrinterSettings', () => { } await waitFor(() => { - expect(virtualPrinterApi.updateSettings).toHaveBeenCalledWith({ mode: 'print_queue' }); + expect(virtualPrinterApi.updateSettings).toHaveBeenCalledWith({ mode: 'queue' }); }); }); }); @@ -576,7 +593,7 @@ describe('VirtualPrinterSettings', () => { describe('network interface override', () => { it('shows interface dropdown when enabled in immediate mode', async () => { vi.mocked(virtualPrinterApi.getSettings).mockResolvedValue( - createMockSettings({ enabled: true, mode: 'immediate' }) + createMockSettings({ enabled: true, mode: 'archive' }) ); render(); @@ -600,7 +617,7 @@ describe('VirtualPrinterSettings', () => { it('shows interface dropdown when enabled in print_queue mode', async () => { vi.mocked(virtualPrinterApi.getSettings).mockResolvedValue( - createMockSettings({ enabled: true, mode: 'print_queue' }) + createMockSettings({ enabled: true, mode: 'queue' }) ); render(); @@ -624,7 +641,7 @@ describe('VirtualPrinterSettings', () => { it('hides interface dropdown when disabled', async () => { vi.mocked(virtualPrinterApi.getSettings).mockResolvedValue( - createMockSettings({ enabled: false, mode: 'immediate' }) + createMockSettings({ enabled: false, mode: 'archive' }) ); render(); @@ -638,7 +655,7 @@ describe('VirtualPrinterSettings', () => { it('shows configured status when interface is set', async () => { vi.mocked(virtualPrinterApi.getSettings).mockResolvedValue( - createMockSettings({ enabled: true, mode: 'immediate', remote_interface_ip: '10.0.0.50' }) + createMockSettings({ enabled: true, mode: 'archive', remote_interface_ip: '10.0.0.50' }) ); render(); @@ -650,7 +667,7 @@ describe('VirtualPrinterSettings', () => { it('shows optional hint when no interface is set', async () => { vi.mocked(virtualPrinterApi.getSettings).mockResolvedValue( - createMockSettings({ enabled: true, mode: 'immediate', remote_interface_ip: '' }) + createMockSettings({ enabled: true, mode: 'archive', remote_interface_ip: '' }) ); render(); diff --git a/frontend/src/__tests__/pages/InventoryPageArchivedConsumed.test.tsx b/frontend/src/__tests__/pages/InventoryPageArchivedConsumed.test.tsx index f834cd46f..d5c57c6bb 100644 --- a/frontend/src/__tests__/pages/InventoryPageArchivedConsumed.test.tsx +++ b/frontend/src/__tests__/pages/InventoryPageArchivedConsumed.test.tsx @@ -53,7 +53,7 @@ const baseSettings = { default_printer_id: null, virtual_printer_enabled: false, virtual_printer_access_code: '', - virtual_printer_mode: 'immediate', + virtual_printer_mode: 'archive', dark_style: 'classic', dark_background: 'neutral', dark_accent: 'green', diff --git a/frontend/src/__tests__/pages/InventoryPageCopyButton.test.tsx b/frontend/src/__tests__/pages/InventoryPageCopyButton.test.tsx index 6b25c999f..9f42a64f0 100644 --- a/frontend/src/__tests__/pages/InventoryPageCopyButton.test.tsx +++ b/frontend/src/__tests__/pages/InventoryPageCopyButton.test.tsx @@ -90,7 +90,7 @@ const MOCK_SETTINGS = { default_printer_id: null, virtual_printer_enabled: false, virtual_printer_access_code: '', - virtual_printer_mode: 'immediate', + virtual_printer_mode: 'archive', dark_style: 'classic', dark_background: 'neutral', dark_accent: 'green', diff --git a/frontend/src/__tests__/pages/InventoryPageDeepLink.test.tsx b/frontend/src/__tests__/pages/InventoryPageDeepLink.test.tsx index 8aab103d4..a41ecb52a 100644 --- a/frontend/src/__tests__/pages/InventoryPageDeepLink.test.tsx +++ b/frontend/src/__tests__/pages/InventoryPageDeepLink.test.tsx @@ -78,7 +78,7 @@ const MOCK_SETTINGS = { default_printer_id: null, virtual_printer_enabled: false, virtual_printer_access_code: '', - virtual_printer_mode: 'immediate', + virtual_printer_mode: 'archive', dark_style: 'classic', dark_background: 'neutral', dark_accent: 'green', diff --git a/frontend/src/__tests__/pages/InventoryPageLowStock.test.tsx b/frontend/src/__tests__/pages/InventoryPageLowStock.test.tsx index 5e56df6a1..49cbad372 100644 --- a/frontend/src/__tests__/pages/InventoryPageLowStock.test.tsx +++ b/frontend/src/__tests__/pages/InventoryPageLowStock.test.tsx @@ -46,7 +46,7 @@ const mockSettings = { default_printer_id: null, virtual_printer_enabled: false, virtual_printer_access_code: '', - virtual_printer_mode: 'immediate', + virtual_printer_mode: 'archive', dark_style: 'classic', dark_background: 'neutral', dark_accent: 'green', diff --git a/frontend/src/__tests__/pages/InventoryPageSpoolmanLocation.test.tsx b/frontend/src/__tests__/pages/InventoryPageSpoolmanLocation.test.tsx index 7adeffa40..815f681cc 100644 --- a/frontend/src/__tests__/pages/InventoryPageSpoolmanLocation.test.tsx +++ b/frontend/src/__tests__/pages/InventoryPageSpoolmanLocation.test.tsx @@ -45,7 +45,7 @@ const mockSettings = { default_printer_id: null, virtual_printer_enabled: false, virtual_printer_access_code: '', - virtual_printer_mode: 'immediate', + virtual_printer_mode: 'archive', dark_style: 'classic', dark_background: 'neutral', dark_accent: 'green', diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 7de0ca704..083d2eae3 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -6299,7 +6299,11 @@ export const discoveryApi = { }; // Virtual Printer types -export type VirtualPrinterMode = 'immediate' | 'queue' | 'review' | 'print_queue' | 'proxy'; // 'queue' is legacy, normalized to 'review' +// Canonical wire values: `archive`, `review`, `queue`, `proxy`. The legacy +// `immediate` (→ archive) and `print_queue` (→ queue) names are still +// accepted by the backend so older API clients keep working, but new code +// should send the canonical names. +export type VirtualPrinterMode = 'archive' | 'review' | 'queue' | 'proxy' | 'immediate' | 'print_queue'; export interface VirtualPrinterProxyStatus { running: boolean; @@ -6375,7 +6379,7 @@ export const virtualPrinterApi = { updateSettings: (data: { enabled?: boolean; access_code?: string; - mode?: 'immediate' | 'review' | 'print_queue' | 'proxy'; + mode?: 'archive' | 'review' | 'queue' | 'proxy'; model?: string; target_printer_id?: number; remote_interface_ip?: string; diff --git a/frontend/src/components/VirtualPrinterAddDialog.tsx b/frontend/src/components/VirtualPrinterAddDialog.tsx index d795c5c97..2de029541 100644 --- a/frontend/src/components/VirtualPrinterAddDialog.tsx +++ b/frontend/src/components/VirtualPrinterAddDialog.tsx @@ -7,12 +7,12 @@ import { Card, CardContent } from './Card'; import { Button } from './Button'; import { useToast } from '../contexts/ToastContext'; -type Mode = 'immediate' | 'review' | 'print_queue' | 'proxy'; +type Mode = 'archive' | 'review' | 'queue' | 'proxy'; const MODE_LABELS: Record = { - immediate: 'archive', + archive: 'archive', review: 'review', - print_queue: 'queue', + queue: 'queue', proxy: 'proxy', }; @@ -26,7 +26,7 @@ export function VirtualPrinterAddDialog({ onClose }: VirtualPrinterAddDialogProp const { showToast } = useToast(); const [name, setName] = useState(''); - const [mode, setMode] = useState('immediate'); + const [mode, setMode] = useState('archive'); const [targetPrinterId, setTargetPrinterId] = useState(null); const { data: printers } = useQuery({ @@ -80,7 +80,7 @@ export function VirtualPrinterAddDialog({ onClose }: VirtualPrinterAddDialogProp
- {(['immediate', 'review', 'print_queue', 'proxy'] as const).map((m) => ( + {(['archive', 'review', 'queue', 'proxy'] as const).map((m) => (
- {/* Auto-dispatch toggle - only for print_queue mode */} - {localMode === 'print_queue' && ( + {/* Auto-dispatch toggle - only for queue mode */} + {localMode === 'queue' && (
@@ -376,8 +385,8 @@ export function VirtualPrinterCard({ printer, models }: VirtualPrinterCardProps)
)} - {/* Force-color-match toggle - only for print_queue mode (#1188) */} - {localMode === 'print_queue' && ( + {/* Force-color-match toggle - only for queue mode (#1188) */} + {localMode === 'queue' && (
diff --git a/frontend/src/components/VirtualPrinterSettings.tsx b/frontend/src/components/VirtualPrinterSettings.tsx index e2c7af6c3..8b439428a 100644 --- a/frontend/src/components/VirtualPrinterSettings.tsx +++ b/frontend/src/components/VirtualPrinterSettings.tsx @@ -7,7 +7,18 @@ import { Card, CardContent, CardHeader } from './Card'; import { Button } from './Button'; import { useToast } from '../contexts/ToastContext'; -type LocalMode = 'immediate' | 'review' | 'print_queue' | 'proxy'; +type LocalMode = 'archive' | 'review' | 'queue' | 'proxy'; + +// Legacy wire values shipped before the UI labels were aligned with the wire +// format. The backend normalizes these on read but a freshly-loaded settings +// payload can still carry an old value if it pre-dates the migration. Map +// to canonical so the button highlight matches the saved mode (#1429). +function normalizeMode(value: string | undefined): LocalMode { + if (value === 'immediate') return 'archive'; + if (value === 'print_queue') return 'queue'; + if (value === 'queue' || value === 'archive' || value === 'review' || value === 'proxy') return value; + return 'archive'; +} export function VirtualPrinterSettings() { const { t } = useTranslation(); @@ -16,7 +27,7 @@ export function VirtualPrinterSettings() { const [localEnabled, setLocalEnabled] = useState(false); const [localAccessCode, setLocalAccessCode] = useState(''); - const [localMode, setLocalMode] = useState('immediate'); + const [localMode, setLocalMode] = useState('archive'); const [localModel, setLocalModel] = useState('BL-P001'); const [localTargetPrinterId, setLocalTargetPrinterId] = useState(null); const [localRemoteInterfaceIp, setLocalRemoteInterfaceIp] = useState(''); @@ -53,12 +64,7 @@ export function VirtualPrinterSettings() { useEffect(() => { if (settings) { setLocalEnabled(settings.enabled); - // Map legacy 'queue' mode to 'review' - let mode: LocalMode = settings.mode === 'queue' ? 'review' : settings.mode as LocalMode; - if (mode !== 'immediate' && mode !== 'review' && mode !== 'print_queue' && mode !== 'proxy') { - mode = 'immediate'; // fallback - } - setLocalMode(mode); + setLocalMode(normalizeMode(settings.mode)); setLocalModel(settings.model); setLocalTargetPrinterId(settings.target_printer_id); setLocalRemoteInterfaceIp(settings.remote_interface_ip || ''); @@ -79,9 +85,7 @@ export function VirtualPrinterSettings() { // Revert local state on error if (settings) { setLocalEnabled(settings.enabled); - // Map legacy 'queue' mode to 'review' - const mode = settings.mode === 'queue' ? 'review' : settings.mode; - setLocalMode(['immediate', 'review', 'print_queue', 'proxy'].includes(mode) ? mode as LocalMode : 'immediate'); + setLocalMode(normalizeMode(settings.mode)); setLocalModel(settings.model); setLocalTargetPrinterId(settings.target_printer_id); } @@ -409,10 +413,10 @@ export function VirtualPrinterSettings() {
{t('virtualPrinter.mode.title')}