From 9b5cc1b4f1e1e4949a0f60dc2005510586906a2b Mon Sep 17 00:00:00 2001 From: maziggy Date: Fri, 19 Jun 2026 11:45:39 +0200 Subject: [PATCH] Post work PR #1516 --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55017734f..aa8387ad7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ All notable changes to Bambuddy will be documented in this file. - **Admin-configurable session lifetime (#1706, reported by @AD3DStuff)** — The 24-hour session cap that ships with Bambuddy was an intentional security hardening (audit finding M-2 reduced it from 7 days), but the "Remember Me" checkbox only controlled storage location (localStorage vs sessionStorage), not session duration. iPhone PWA users and homelab admins on trusted networks were getting kicked out every 24 hours with no way to extend it. **New setting:** `session_max_hours` under Settings → Users with three presets (24h / 7 days / 30 days) plus a custom field, hard-capped at 30 days (720h). Default remains 24h so existing deployments and the M-2 audit baseline are untouched until an admin opts in. The Settings card surfaces a yellow warning whenever the value exceeds 24h: "Longer sessions reduce automatic logout protection. Recommended only for trusted single-user deployments." **Backend wiring:** new `resolve_session_max_minutes(db)` helper in `backend/app/core/auth.py` reads the setting, clamps to [1h, 720h], and falls back to 24h on missing / blank / unparseable values. The helper is called at all four token-issuance sites — plain `/auth/login`, 2FA TOTP/email completion, 2FA backup-code completion, and OIDC callback — so a long-session policy works uniformly regardless of how the user authenticates. DB errors in the resolver are deliberately NOT caught: login is already inside a transaction and a broken DB must abort the login rather than silently extend or shrink the session lifetime. Defense-in-depth `SESSION_MAX_HOURS_HARD_CEILING = 720` clamps any tampered DB row above the Pydantic ceiling. Already-issued tokens keep their original expiry — the new setting only affects future logins, so an admin lowering the value can't retroactively revoke active sessions and an admin raising it can't retroactively extend them. **What this does NOT change:** the "Remember Me" checkbox still controls only storage location (cleared on browser close vs persisted across restarts). The relabel from misleading-UX-perspective is left for a separate follow-up — that's a UX choice independent of the session-policy mechanism. API tokens (`MAX_TOKEN_LIFETIME_DAYS`), camera stream tokens (60min), WebSocket tokens (60min), and slicer download tokens (5min) keep their own TTLs and are unaffected. **Tests:** 15 new cases in `backend/tests/integration/test_session_policy.py` split across three classes. `TestResolveSessionMaxMinutes` pins the clamping resolver — missing row, empty string, unparseable value, zero/negative, 1h minimum, 7-day passthrough, 30-day passthrough, above-ceiling clamp. `TestLoginRespectsSessionPolicy` decodes the JWT `exp` claim end-to-end and asserts the token returned by `/auth/login` honours the configured ceiling for the default-24h, configured-7d, and above-ceiling-clamp cases. `TestSettingsAPIExposesSessionMaxHours` round-trips the field through `/settings/` (default = 24, valid update persists as int's string form, zero rejected with 422, above-ceiling rejected with 422). Existing 202-case auth + MFA suite still green. **i18n:** 8 new keys in `settings.sessionPolicy.*` namespace; full translations in all 10 non-en locales (de / es / fr / it / ja / ko / pt-BR / tr / zh-CN / zh-TW), no English fallback. Parity check 5149 leaves per locale. ESLint clean; `npm run build` clean; ruff clean. +- **Per-VP "G-code injection" toggle for Studio Send / FTP uploads (#1516, contributed by @phieb)** — Queue-mode Virtual Printers gain a per-VP opt-in toggle that applies the Settings → G-code Snippets per-model start/end snippets to every job that lands via the VP — Bambu Studio's "Send", OrcaSlicer's "Print Plate", the VP's own FTP upload path. Before this change the snippets were only applied to items queued through the PrintModal's "Inject auto-print G-code" checkbox; VP-incoming jobs silently bypassed injection regardless of how the snippets were configured. **Default off so upgraders don't silently start injecting**: existing `gcode_snippets` installs keep their previous behaviour until the per-VP toggle is explicitly enabled. When on, the scheduler still no-ops unless `gcode_snippets` are configured for the target printer model, so the effective semantics are "inject when enabled AND snippets exist." **DB column:** new `virtual_printers.gcode_injection BOOLEAN DEFAULT FALSE` with a branched `is_sqlite()` migration (SQLite `DEFAULT 0` / Postgres `DEFAULT FALSE`) matching the `queue_force_color_match` / `tailscale_disabled` precedent. **Multi-plate stamping:** the flag is set on every plate's `PrintQueueItem` inside the per-plate loop introduced by #1697 / #1188, so a multi-plate "Send all" upload now gets snippets injected on each plate consistently — the original PR only stamped the first plate; the merge resolution wove the flag into the loop. **Live-toggle correctness:** the `_sync_from_db_locked` change detector now compares `instance.gcode_injection != vp.gcode_injection`, so toggling the value in the UI triggers a VP restart instead of letting the in-memory instance keep the stale flag and silently propagate it onto every subsequent upload — same shape as the #1552 family. Backed by a dedicated `test_sync_from_db_restarts_on_gcode_injection_toggle`. **UI:** new toggle on `VirtualPrinterCard.tsx` (queue mode only — the toggle is hidden in archive/review/proxy modes since the feature is queue-specific), with the standard `updateMutation` save-on-click + toast on success, plus the `pendingAction='gcodeInjection'` opacity dim during the round-trip. **PrintModal hardening:** when "Inject auto-print G-code" is ticked on a reprint at quantity > 1, the modal now routes ALL copies through the queue (not just copies 2..N) so the scheduler injects every dispatch — see the separate reprint-quantity entry below for the full motivation. A new `useEffect` clears the stale `gcodeInjection` state if the user ticks the box at quantity 2, then drops back to quantity 1 — the checkbox hides at that point and the state must follow, otherwise the immediate-reprint path would silently bypass injection. **Diagnostics:** the resolved start/end snippets (with `{placeholder}` substitution already applied) are logged at DEBUG so any "snippet didn't run" report can be traced from a log bundle. **i18n:** new `virtualPrinter.gcodeInjection.title` + `description` keys translated in all 11 locales (de / en / es / fr / it / ja / ko / pt-BR / tr / zh-CN / zh-TW); parity check 5188 leaves per locale, no English fallback. **Tests:** 2 new unit cases in `test_virtual_printer.py` (queue items opt in / out based on the VP flag), 2 new integration cases in `test_virtual_printer_api.py` (create defaults to false, PUT round-trips the value), 1 new sync-restart case, plus updates to `_make_db_vp` so the change-detector test fixture carries an explicit `False` rather than relying on `MagicMock` truthiness. 2 new PrintModal vitest cases pin the reprint dispatch matrix (injection ON queues all copies and dispatches none immediately; injection OFF keeps the immediate first copy and queues the rest). Full backend pytest 6167/6167; full frontend vitest 2154/2154; ruff clean; `npm run build` clean. + ### Fixed - **Docker installer fails on the default `/opt/bambuddy` path with "Permission denied" (#1774, reported by @jmoore-skild)** — `install/docker-install.sh::create_install_dir` (line 252) ran `mkdir -p "$INSTALL_PATH"` without sudo while `DEFAULT_INSTALL_PATH="/opt/bambuddy"` (line 32) — root-owned on every Linux distro. `set -e` at line 20 then aborted the whole run before docker compose could ever pull the image. Anyone following the documented `curl … | bash` flow as a normal user hit this immediately. The native installer at `install/install.sh:361` already handles the same situation correctly with `sudo mkdir -p` + `sudo chown`; the Docker variant just never got the same treatment. **Why the fix isn't a default-path change:** the contributor's first instinct was to drop the default to `~/bambuddy` since the Docker installer only writes `docker-compose.yml` + `.env` on the host (real app data lives in named volumes), but `install/update.sh:4` and `install/update_macos.sh:4` both default `INSTALL_DIR` to `/opt/bambuddy`, and `install/README.md:274` documents `INSTALL_DIR=/opt/bambuddy sudo ./update.sh` for the update flow — changing the install default without coordinating the update path would silently break self-service updates for anyone following the docs verbatim. The actual gap is the missing privilege escalation in `create_install_dir`, not the default path. **Fix:** `create_install_dir` now tries `mkdir -p "$INSTALL_PATH" 2>/dev/null` first — the cheap no-sudo path covers `--path ~/bambuddy`, `--path /srv/bambuddy`, and any other writable target — and only falls back to `sudo mkdir -p "$INSTALL_PATH"` + `sudo chown -R "$USER:$USER" "$INSTALL_PATH"` when the unprivileged attempt fails. The chown is load-bearing: without it, the script would later try to write `docker-compose.yml` and `.env` into a root-owned dir as the unprivileged invoking user, kicking off a cascade of EACCES failures further down. Idempotent on re-run (the second `mkdir -p` succeeds against the now-owned dir, no second sudo prompt). `set -e` survives the redirected stderr because the `if !` construct is the documented escape from bash's exit-on-error semantics for an expected-failure check. **Smoke-tested all three branches:** writable target → no sudo prompt fires; idempotent re-run → no second sudo prompt; the failing-mkdir-then-fallback path → `set -e` survives intact. **What this does NOT change:** the default install path stays `/opt/bambuddy` for parity with `install.sh` / `update.sh` / the documented update flow; the Windows mirror at `install/docker-install.ps1` already uses `$env:USERPROFILE\bambuddy` (per-user convention on Windows) and is untouched. No docs change required — `install/README.md` and the wiki Docker page (`bambuddy-wiki/docs/getting-started/docker.md`) both still accurately describe the behaviour. - **MakerWorld import/resolve/status fail under API-key auth even when the owner has a Bambu Cloud login (#1777, reported by @Mx772)** — The reporter (working on a browser extension that drives Bambuddy via `X-API-Key`) noticed that `POST /api/v1/makerworld/import` and `POST /api/v1/makerworld/resolve` returned `{"detail":"Downloading files from MakerWorld requires a Bambu Cloud login"}` even when the key's owning user had a valid stored Bambu Cloud session, and the same imports succeeded from the web UI. Root cause is exactly the shape the reporter traced: `require_permission_if_auth_enabled` in `backend/app/core/auth.py:1414` deliberately returns `current_user=None` for API-keyed callers — the comment at line 1408 makes this explicit and points at `cloud.py` for the resolver. The MakerWorld routes never got that resolver wired in, so `_build_service(db, None)` → `get_stored_token(db, None)` → no token → the "requires Bambu Cloud login" branch fires regardless of what the owning account has set up. Same shape #1182 fixed for cloud slicer presets, and the canonical fix for non-`/cloud/*` routes is already in the codebase as `resolve_api_key_cloud_owner` (cloud.py:128-160) — used by `slicer_presets.py:491` and `library.py:3871`. The MakerWorld routes were missing the wire-up. **Fix:** Three routes get the extra `api_key_cloud_owner: User | None = Depends(resolve_api_key_cloud_owner)` parameter — `get_status`, `resolve_url`, `import_instance` — and each resolves `cloud_token_user = current_user or api_key_cloud_owner` before calling `get_stored_token` / `_build_service`. `import_instance` additionally uses `cloud_token_user.id` for the `owner_id` argument to `save_3mf_bytes_to_library` (which translates to `LibraryFile.created_by_id`), so library rows imported via API key are now attributed to the key's owner instead of staying NULL. `/recent-imports` is unchanged — it only uses `current_user` as a permission gate (`_ = current_user`) and never touches the cloud token. The fix preserves fail-closed semantics for keys *without* the `can_access_cloud` flag: `resolve_api_key_cloud_owner` already fences on `api_key.user_id is not None and api_key.can_access_cloud` (cloud.py:158), so a key with only the per-route scope (`can_read_status` / `can_manage_library`) still surfaces the "requires Bambu Cloud login" error path — no new auth gap. **Two scope fields the API key needs:** the per-route scope (`MAKERWORLD_VIEW` → `can_read_status`, `MAKERWORLD_IMPORT` → `can_manage_library` per `_APIKEY_SCOPE_BY_PERMISSION` in `core/auth.py`) AND the orthogonal `can_access_cloud` flag (separate column on the `api_keys` table). The fix doesn't change that surface — it just stops dropping valid `can_access_cloud=True` keys on the floor. **Tests:** 6 new cases in `backend/tests/integration/test_makerworld_apikey_auth.py` pinning the full surface — API key with `can_access_cloud=True` + owner-has-token → `/status` reports `has_cloud_token=True`, `/resolve` builds the service with the owner User (asserted on the `_build_service` mock's call args), `/import` succeeds end-to-end and the resulting `LibraryFile.created_by_id` matches the API-key owner; API key with `can_access_cloud=False` → status still reports `has_cloud_token=False` (no widening) and import-row's `created_by_id` stays NULL; JWT-authenticated parity check confirms the existing user-session flow is unchanged by the added `Depends`. 6/6 new tests green; full backend suite (6157 tests) still green; ruff clean. No frontend change, no DB migration, no new permission, no new dependency. The reporter's browser extension and any other API-keyed Home Assistant / automation integration unblocks immediately on next deploy. @@ -221,7 +223,8 @@ All notable changes to Bambuddy will be documented in this file. - **Label printing produced two identical PDFs per click (#1628)** — `LabelTemplatePickerModal.tsx::openBlobInNewTab` called `window.open(url, '_blank', 'noopener,noreferrer')` and treated a `null` return as "popup blocked → fall back to `` click." Per the WindowFeatures spec, `noopener` deliberately forces `window.open` to return `null` even on success, so the `if (!win)` fallback fired on EVERY click. Path 1 (window.open) opened the blob tab — on Linux Chromium without an inline PDF viewer the OS saved a random-named copy (the `zo70GhSL.pdf` / `f7w0OcDi.pdf` files in the reporter's screenshot). Path 2 (fallback) downloaded a second copy named `bambuddy-labels.pdf`. Two identical PDFs per click. Fix: drop `noopener,noreferrer`. The blob is same-origin (created via `URL.createObjectURL` from our own fetch response), the destination is a passive PDF preview tab with no script context to abuse `window.opener`, and `noreferrer` is a no-op for blob URLs. After removal, `window.open` returns a real window reference on success → `if (!win)` only fires on genuine popup-block, single PDF per click. Existing 17 vitest cases in `LabelTemplatePickerModal.test.tsx` still pass; the change is comment + one parameter. - **Scheduled local backup time is now interpreted as local time, not UTC (#1602 follow-up)** — Pre-fix: the time-of-day picker in Settings → Scheduled Local Backups stored the value as `HH:MM` and `_calculate_next_run` in `backend/app/services/local_backup.py` interpreted it as UTC (`datetime.now(timezone.utc).replace(hour=..., minute=...)`), so a UTC+3 user who wanted a 21:00 local backup had to enter 18:00. The UI hinted at this with a literal "UTC" label, but it was still surprising. Post-fix: the picker is interpreted in the container's local timezone, resolved from the `TZ` env var via `zoneinfo.ZoneInfo` (same source the Support page's `environment.timezone` already shows). UTC fallback when `TZ` is unset or unrecognised — preserves the legacy behaviour rather than crashing. The UI now shows the resolved zone name next to the time field (`Local time (Europe/Berlin)` / `Yerel saat (Europe/Istanbul)` / etc.) via a new i18n key `backup.localTimeHint` with real translations across all 10 non-English locales, replacing the old `backup.utc` literal. New `timezone` field on `/api/local-backup/status` exposes the resolved zone to the UI. **One-time behaviour change for existing users**: anyone who entered a UTC time as a workaround (per #1602's UTC+3 reporter — "I have to write 18:00 to get 21:00 local") will see the first scheduled cycle after upgrade run at their local TZ offset earlier than expected. Re-enter the time as local once and it's correct from then on. No migration is shipped; the population is small and migrating around a DST boundary would be ambiguous. **Tests** (`backend/tests/unit/test_local_backup.py`): existing 5 cases pinned with `monkeypatch.setenv("TZ", "UTC")` so they don't depend on the test runner's TZ; 5 new — Europe/Berlin local→UTC, Europe/Istanbul (the #1602 reporter's zone) local→UTC, no-TZ-env UTC fallback, unrecognised-TZ UTC fallback, DST spring-forward gap (Europe/Berlin 2026-03-29 02:30 wall-clock doesn't exist) asserting no crash. All 30 tests pass. Frontend i18n parity green at 5007 keys across 10 non-English locales. -- **Reprint with quantity > 1 + auto-print G-code injection now injects *every* copy, including the first** — since auto-print injection landed (#422), ticking **Inject auto-print G-code** in the Reprint dialog only applied to copies 2…N: the first copy was dispatched immediately via the direct reprint path, which bypasses the scheduler that performs injection, so it printed *without* the start/end snippets. For auto-eject / plate-clear setups (Farmloop, SwapMod, AutoClear, Printflow 3D) this left the first copy stuck on the plate, blocking the injected copies queued behind it. When injection is enabled and quantity > 1, the Reprint flow now queues *all* copies so each one is dispatched — and injected — by the scheduler. Behaviour with injection off is unchanged (first copy still prints immediately, the rest queue), as are the single-copy reprint, Add-to-Queue, Edit-queue-item, and stagger paths. +- **Auto-print end snippets silently dropped on P1S, and modified 3MFs rejected with HMS 0500-4003 (#1516, contributed by @phieb)** — Two compounding firmware quirks made the original #422 G-code injection unusable on the P1S — the most common reporter platform for auto-eject / plate-clear automation. **(1) End snippet dropped after `; EXECUTABLE_BLOCK_END`**: the snippet was appended to the end of `Metadata/plate_N.gcode`, but Bambu firmware (verified on a P1S) does not execute G-code that sits **after** the `; EXECUTABLE_BLOCK_END` marker. An auto-eject sweep injected to clear the plate ran on every other Bambu model but silently no-op'd on P1S — print finished, plate stayed loaded, the next queued copy stalled behind it. The injection now anchors the end snippet **before** `; EXECUTABLE_BLOCK_END` so it sits inside the executed block, after the printer's own machine-end sequence (cooldown / M104 S0 / etc) but before the firmware stops parsing. Files without the marker — older slicer versions, non-Bambu sources — keep the existing append-to-EOF behaviour with a warning log; the test suite pins both paths. **(2) Stale `.gcode.md5` sidecar rejected by P1S firmware**: every plate carries a `Metadata/plate_N.gcode.md5` sidecar that the P1S validates against the gcode body on load. Rewriting the gcode without refreshing the hash made the P1S reject the file with `HMS 0500-4003 "unable to parse"` and abort the print — exactly when the injection had succeeded. `inject_gcode_into_3mf` now recomputes the sidecar from the exact bytes about to be written and re-packs it into the 3MF in the same pass, matching Bambu's on-disk format exactly (uppercase hex, 32 chars, no trailing newline). The MD5 is a firmware integrity check, not a security primitive, so the call is flagged `hashlib.md5(..., usedforsecurity=False)` to keep ruff S324 / Bandit B324 clean. 3MFs **without** an `.md5` sidecar member (older files, manual hand-builds) do not gain one — inventing a member could surprise older firmware that doesn't expect it; if the source had no sidecar the firmware wasn't validating it anyway. Non-target zip members keep their original compression intact (the P1S preview parser chokes on re-DEFLATEd PNGs that the source had stored uncompressed). **Live-tested on a P1S** with `{max_layer_z}` placeholder substitution: injection happens, the file loads cleanly, the end snippet executes, the recomputed hash validates against the modified body. **Tests** (`test_gcode_injection.py`): 4 new cases in `TestMd5SidecarRecompute` (sidecar matches the exact gcode bytes after injection, uppercase-hex-no-newline format parity with Bambu's on-disk shape, no `.md5` member is invented when source lacks one, non-target zip member compression is preserved) + 1 new case `test_end_lands_before_executable_block_end` pinning the in-block placement when the marker is present + 1 case renamed from `test_end_still_appended_at_eof` to `test_end_falls_back_to_eof_without_block_marker` to reflect that EOF append is now the **fallback** path, not the primary one. 198 VP + gcode_injection tests in the slice green; full backend 6167/6167; ruff clean. +- **Reprint with quantity > 1 + auto-print G-code injection now injects *every* copy, including the first (#1516, contributed by @phieb)** — since auto-print injection landed (#422), ticking **Inject auto-print G-code** in the Reprint dialog only applied to copies 2…N: the first copy was dispatched immediately via the direct reprint path, which bypasses the scheduler that performs injection, so it printed *without* the start/end snippets. For auto-eject / plate-clear setups (Farmloop, SwapMod, AutoClear, Printflow 3D) this left the first copy stuck on the plate, blocking the injected copies queued behind it. When injection is enabled and quantity > 1, the Reprint flow now queues *all* copies so each one is dispatched — and injected — by the scheduler. Behaviour with injection off is unchanged (first copy still prints immediately, the rest queue), as are the single-copy reprint, Add-to-Queue, Edit-queue-item, and stagger paths. ## [0.2.4.5] - 2026-06-03