diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c87b5f88..027f0f332 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ All notable changes to Bambuddy will be documented in this file. - **Slicer Bundle (.bbscfg) import — pick presets from a stored bundle instead of resolving cloud/local/standard PresetRefs every slice** — Closes the long tail of preset-resolution corner cases (cloud presets behind login, "from User" sentinel handling, the `# `-prefix clone trick, dangling `inherits` on renamed parents, etc.) by letting users upload a BambuStudio "Printer Preset Bundle" (`.bbscfg`) once per printer and pick from it for every subsequent slice. **Service layer (`backend/app/services/slicer_api.py`):** `BundleSummary` / `BundleNotFoundError` types, `import_bundle` / `list_bundles` / `get_bundle` / `delete_bundle` methods, `slice_with_bundle` which posts `/slice` with bundle id + per-category preset names instead of the JSON triplet. **Routes (`/api/v1/slicer/bundles`, all gated on `Permission.LIBRARY_UPLOAD`):** `POST` / `GET` / `GET :id` / `DELETE :id`. All routes proxy via `_resolve_slicer_api_url` so they follow the user's `preferred_slicer` setting (bambu_studio vs orcaslicer). Status-code mapping treats sidecar 4xx as 400, `BundleNotFoundError` as 404, sidecar unreachable as 503, and sidecar 5xx as 502. **Preview-slice (`backend/app/services/slice_preview.py::get_preview_filaments`)** picks up optional `bundle_id` + `printer_name` + `process_name` + `filament_names` params and routes through `slice_with_bundle` when set; the cache key picks up a bundle-context fingerprint so different bundle picks on the same file occupy distinct entries — gram numbers in the preview now match what the real print will produce instead of being derived from the file's embedded process settings (which can drift from the triplet the actual slice would use). The `library.py` and `archives.py` `/filament-requirements` routes forward the new params. **Dispatch (`SliceRequest.bundle: SliceBundleSpec`):** when set, `_run_slicer_with_fallback` skips `resolve_preset_ref` and calls `slice_with_bundle`; the validator skips the preset-required check so bundle-only requests validate. 3MF + bundle CLI 5xx still falls back to the embedded-settings slice path (`used_embedded_settings=True` surfaces in the response), and sidecar 404 (unknown bundle / preset name) maps to 400. **Frontend SliceModal Bundle tier:** new "Slicer bundle" picker at the top of the modal, rendered only when at least one bundle is imported (`GET /slicer/bundles` non-empty). Selecting a bundle replaces cloud / local / standard preset dropdowns with bundle-scoped pickers (process + per-slot filament names from the bundle) — printer is implicit (each `.bbscfg` has exactly one). "None" leaves the modal on the original preset-triplet path. Submit routes through `SliceRequest.bundle` so the backend skips PresetRef resolution and asks the sidecar to materialise the JSON triplet from the stored bundle by name. **Frontend types:** `SliceBundleSpec` + `bundle?: SliceBundleSpec` on `SliceRequest`; `getLibraryFileFilamentRequirements` / `getArchiveFilamentRequirements` accept an optional 4th-arg bundle context object. The orca-slicer-api fork's bundle endpoints (shipped on `bambuddy/bundle-import`) are the server side of this — see the slicer-api sidecar docker-compose for the matching versions. ### Fixed -- **SpoolBuddy with Spoolman enabled: NFC tag scan looked up local DB first, ignored Spoolman setting; "Assign to AMS" did nothing on freshly-linked spools; AMS slot picker hid the assigned spool's info and unassign action; LinkSpoolModal showed "Unknown color" for every Spoolman spool; tag-write didn't enforce uniqueness so the wrong spool resolved on scan; kiosk display held stale assigned-state forever** — Several intertwined bugs surfaced during `feature/spoolman-inventory-ui` testing; fixing them as one batch because they all live on the SpoolBuddy + Spoolman path. **(1) `/spoolbuddy/nfc/tag-scanned` always tried local DB first** and only consulted Spoolman as a fallback on local-DB miss, so a stale local copy of a tag silently won over the authoritative Spoolman row, and deleting the local copy was the only way to surface the Spoolman match. Now the route gates on `_get_spoolman_client_or_none(db)` (which already encodes the `spoolman_enabled` setting + SSRF guard) and routes to whichever inventory backend Bambuddy is configured for — Spoolman exclusive when enabled, local exclusive otherwise. **(2) Dashboard "Assign to AMS" button was a no-op** when the freshly-matched spool wasn't yet in the cached `getSpoolmanInventorySpools` query result (newly created or unarchived in Spoolman after the dashboard loaded). The card rendered via its own `displayedSpool ?? sbState.matchedSpool` fallback, but the modal's stricter `displayedSpool && !justLinkedSpool && displayedTagId` guard silently failed to mount. New `effectiveModalSpool` synthesises an `InventorySpool`-shaped object from the WebSocket-delivered `MatchedSpool` (a 9-field subset; `slicer_filament*` are absent but the modal only uses `id` to route the assign API call and the mismatch check yields `'none'` for profile in either case). **(3) AMS-page slot picker hid the assigned spool entirely** — when a slot had a `SpoolmanSlotAssignment` (assigned via the dashboard's Assign-to-AMS flow) but no tag-linked spool, the picker explicitly returned `null` for the assign/unassign branch and only the "Configure" button remained visible. Now the picker resolves the assignment from `spoolmanSlotAssignmentsAll + spoolmanInventorySpoolsCache`, renders a "Assigned spool: brand · material - color" info card, and exposes an Unassign button wired to a new `unassignSpoolmanSlotMutation` (calls `DELETE /spoolman/inventory/slot-assignments/`, mirroring the local-mode flow). **(4) `LinkSpoolModal` showed "Unknown color" for every Spoolman spool** because Spoolman doesn't standardise `color_name` — most installs only populate `color_hex` and the filament's `name` (which often carries the colour like "PLA Basic Red"). `_map_spoolman_spool` now falls back to the filament's subtype (filament name minus material prefix — typically "Basic Red") when `color_name` is empty, so spools are visually distinguishable in the picker without changes to the frontend. **(5) Writing a tag for spool B didn't clear the same tag binding from spool A**, so a single physical NFC UID could map to two Spoolman spools at once and `find_spool_by_tag` returned whichever came first in the cached list (typically the older one) — exactly the symptom maziggy hit during testing where re-writing a tag still surfaced the previously-assigned spool. `nfc_write_result` now searches Spoolman for any other spool currently bound to the target UID and clears its `extra.tag` (best-effort: cleanup failure logs a warning but doesn't block the write itself, since the device already wrote the chip). **(6) The kiosk display held stale `spoolmanSlotAssignments` cache** because the SpoolBuddy display is a long-running browser window with no focus/remount triggers, so a `staleTime` alone never caused a refetch. State changed elsewhere (Bambuddy main UI, direct Spoolman edit) was invisible to the kiosk and `isSpoolAssigned` reported assigned-forever — the Assign button stayed disabled, the Unassign button stayed enabled, after the spools were already unassigned. Adds `refetchInterval: 3_000` (cheap query, bounded latency below operator-noticeable) so the kiosk picks up external changes within seconds. **Tests:** new `TestMapSpoolmanSpool::test_color_name_uses_explicit_field_when_present` / `_falls_back_to_subtype_when_field_missing` / `_none_when_both_fields_empty` (3 unit tests pinning the colour-name fallback chain), new `TestNfcEndpoints::test_tag_scanned_spoolman_mode_skips_local_lookup` (verifies `get_spool_by_tag` is never called when Spoolman is enabled, even when the lookup would have returned a spool), and new `test_write_result_clears_duplicate_tag_binding` (asserts `merge_spool_extra` is called twice — once to clear the old holder's `extra.tag`, once to bind the new owner — in that order with the right spool ids). Existing 76 helper tests + 7 NFC-endpoint tests still pass. +- **SpoolBuddy with Spoolman enabled: NFC tag scan looked up local DB first, ignored Spoolman setting; "Assign to AMS" did nothing on freshly-linked spools; AMS slot picker hid the assigned spool's info and unassign action; LinkSpoolModal showed "Unknown color" for every Spoolman spool; tag-write didn't enforce uniqueness so the wrong spool resolved on scan; kiosk display held stale assigned-state forever** — Several intertwined bugs surfaced during `feature/spoolman-inventory-ui` testing; fixing them as one batch because they all live on the SpoolBuddy + Spoolman path. **(1) `/spoolbuddy/nfc/tag-scanned` always tried local DB first** and only consulted Spoolman as a fallback on local-DB miss, so a stale local copy of a tag silently won over the authoritative Spoolman row, and deleting the local copy was the only way to surface the Spoolman match. Now the route gates on `_get_spoolman_client_or_none(db)` (which already encodes the `spoolman_enabled` setting + SSRF guard) and routes to whichever inventory backend Bambuddy is configured for — Spoolman exclusive when enabled, local exclusive otherwise. **(2) Dashboard "Assign to AMS" button was a no-op** when the freshly-matched spool wasn't yet in the cached `getSpoolmanInventorySpools` query result (newly created or unarchived in Spoolman after the dashboard loaded). The card rendered via its own `displayedSpool ?? sbState.matchedSpool` fallback, but the modal's stricter `displayedSpool && !justLinkedSpool && displayedTagId` guard silently failed to mount. New `effectiveModalSpool` synthesises an `InventorySpool`-shaped object from the WebSocket-delivered `MatchedSpool` (a 9-field subset; `slicer_filament*` are absent but the modal only uses `id` to route the assign API call and the mismatch check yields `'none'` for profile in either case). **(3) AMS-page slot picker hid the assigned spool entirely** — when a slot had a `SpoolmanSlotAssignment` (assigned via the dashboard's Assign-to-AMS flow) but no tag-linked spool, the picker explicitly returned `null` for the assign/unassign branch and only the "Configure" button remained visible. Now the picker resolves the assignment from `spoolmanSlotAssignmentsAll + spoolmanInventorySpoolsCache`, renders a "Assigned spool: brand · material - color" info card, and exposes an Unassign button wired to a new `unassignSpoolmanSlotMutation` (calls `DELETE /spoolman/inventory/slot-assignments/`, mirroring the local-mode flow). **(4) `LinkSpoolModal` showed "Unknown color" for every Spoolman spool** because Spoolman doesn't standardise `color_name` — most installs only populate `color_hex` and the filament's `name` (which often carries the colour like "PLA Basic Red"). `_map_spoolman_spool` now falls back to the filament's subtype (filament name minus material prefix — typically "Basic Red") when `color_name` is empty, so spools are visually distinguishable in the picker without changes to the frontend. **(5) Writing a tag for spool B didn't clear the same tag binding from spool A**, so a single physical NFC UID could map to two Spoolman spools at once and `find_spool_by_tag` returned whichever came first in the cached list (typically the older one) — exactly the symptom maziggy hit during testing where re-writing a tag still surfaced the previously-assigned spool. `nfc_write_result` now searches Spoolman for any other spool currently bound to the target UID and clears its `extra.tag` (best-effort: cleanup failure logs a warning but doesn't block the write itself, since the device already wrote the chip). **(6) The kiosk display held stale `spoolmanSlotAssignments` cache** because the SpoolBuddy display is a long-running browser window with no focus/remount triggers, so a `staleTime` alone never caused a refetch. State changed elsewhere (Bambuddy main UI, direct Spoolman edit) was invisible to the kiosk and `isSpoolAssigned` reported assigned-forever — the Assign button stayed disabled, the Unassign button stayed enabled, after the spools were already unassigned. Adds `refetchInterval: 3_000` (cheap query, bounded latency below operator-noticeable) so the kiosk picks up external changes within seconds. **(7) Kiosk QuickMenu System buttons (Restart Daemon / Restart Browser / Reboot / Shutdown) all 403'd silently** — the `/spoolbuddy/devices//system/command` route was gated on `Permission.SETTINGS_UPDATE` (T-Gap 2 from a prior security audit), but every other kiosk-scoped device route (`calibration/tare`, `display`, `cancel-write`, `system/command-result`) uses `INVENTORY_UPDATE`. The kiosk's operator session has `INVENTORY_UPDATE` but not `SETTINGS_UPDATE`, so every System button silently failed via the modal's catch-block (no toast). Aligned the permission with the rest of the kiosk-scoped routes so operators can recover the kiosk from the kiosk itself. Risk is bounded — only the 4 named commands are accepted (no RCE), reboot/shutdown require physical-access recovery, the same operator already controls printers + weighs spools on the same device. The `/update` route keeps `SETTINGS_UPDATE` because that one can replace the daemon binary, which is a different threat surface. Test contract `test_system_command_requires_settings_update` is renamed to `test_system_command_accepts_inventory_update` and asserts the inventory-only key now reaches the device-state check (409 offline) instead of 403, so a future re-tightening of the gate surfaces immediately. **Tests:** new `TestMapSpoolmanSpool::test_color_name_uses_explicit_field_when_present` / `_falls_back_to_subtype_when_field_missing` / `_none_when_both_fields_empty` (3 unit tests pinning the colour-name fallback chain), new `TestNfcEndpoints::test_tag_scanned_spoolman_mode_skips_local_lookup` (verifies `get_spool_by_tag` is never called when Spoolman is enabled, even when the lookup would have returned a spool), and new `test_write_result_clears_duplicate_tag_binding` (asserts `merge_spool_extra` is called twice — once to clear the old holder's `extra.tag`, once to bind the new owner — in that order with the right spool ids). Existing 76 helper tests + 7 NFC-endpoint tests still pass. - **Spool assignment to a reset AMS slot left the slot unconfigured both in Bambuddy and on the printer** — Reproduced during `feature/spoolman-inventory-ui` testing (extends the #1228 family). After clicking "Reset slot" on an AMS slot that had filament physically loaded, picking an inventory spool from the printer card and clicking Assign showed a success toast — but the slot kept reporting as unconfigured, no `ams_filament_setting` MQTT command ever fired, and the spool's brand/color never appeared on either the Bambuddy printer card or BambuStudio. **Cause:** `assign_spool` in `backend/app/api/routes/inventory.py` decided the slot was empty using `slot_is_empty = not (fingerprint_type and fingerprint_type.strip())` where `fingerprint_type` came from `tray.tray_type`. The "Reset slot" command clears `tray_type` / `tray_color` / `tray_info_idx` to empty strings on the printer side but leaves the filament physically loaded. The empty `tray_type` then misled the heuristic into the pending-config (SpoolBuddy weigh-then-assign) branch, which intentionally skips the MQTT publish because Bambu firmware drops `ams_filament_setting` on truly unloaded slots. The deferred replay in `on_ams_change` only fires on an empty→loaded transition — but the slot was already loaded, so no transition ever came and the assignment sat in pending state forever. **Fix:** capture `tray.state` alongside the fingerprint fields when looking up the AMS tray (Bambu firmware reports `state == 11` for loaded, `9` for empty, `10` for spool present but filament not in feeder; documented at `bambu_mqtt.py:1631-1633`). When `state` is reported, `slot_is_empty = (state != 11)`. When `state` is not reported (older firmware), fall back to the existing `tray_type` heuristic so legacy installs continue to behave the same. Same logic applied to the external-slot path (`ams_id == 255` / `vt_tray`). **Tests:** 5 new in `TestAssignSpoolEmptyDetection` — post-reset (`state=11, tray_type=""` → MQTT must fire, `pending_config=False`), genuinely empty (`state=9` → MQTT skipped, `pending_config=True`), legacy fallback both directions (no `state` field → tray_type heuristic), and the external-slot post-reset variant. - **Slicer "Send to printer" silently rejected the cached push_status with "storage needs to be inserted" on P1S/A1-class targets** ([#1228](https://github.com/maziggy/bambuddy/issues/1228), reported by @rtadams89, also hit by @smandon) — Slicer "Send" worked on 0.2.3.2 with a queue-mode VP and started failing on 0.2.4b3, regardless of subnet topology, with BambuStudio showing the generic "storage needs to be inserted before send to printer" error. Reproducible across Docker bridge, macvlan, and LAN-attached host networking. Network reachability ruled out (slicer reaches MQTT/FTPS, FTP passive ports 50000-50100 reachable end-to-end, pfSense rules clean). The smoking gun was in @rtadams89's debug-level support archive: slicer establishes MQTT TLS to the VP, gets `pushall` + `get_version` responses, then **never opens an FTP connection** — the slicer reads the cached push, fails its pre-flight, and aborts before attempting any data transfer. **Cause:** the 0.2.3.2 synthetic stub baked three SD/storage indicators that BambuStudio's "Send" pre-flight reads — `home_flag` with bit 8 (`HAS_SDCARD_NORMAL`, `0x100`), `sdcard: True`, and a `storage: {free, total}` block. The 0.2.4b3 cached-as-base slicer-mirror (commit `7dea33d0`) passes the live target's push_status through with only an IP rewrite; if the real firmware doesn't report those fields (P1S/A1 with no SD card inserted, older field shapes, P1S firmware `01.10.00.00` confirmed in @rtadams89's logs), the slicer sees "no storage" and refuses to send. H2D and X1C in maziggy's local cross-subnet repro worked because those firmwares do report the indicators; P1S/A1-class doesn't always. **Fix:** in `mqtt_server.py:_send_status_report` cached-as-base path, after copying the cache, OR `0x100` onto `home_flag` (preserves any other bits the printer set), force `sdcard=True`, and `setdefault` a `storage: {free: 1_000_000_000, total: 32_000_000_000}` block (only fills in if the real printer didn't report one — real values pass through unchanged when present). For VP usage the slicer uploads via FTPS to Bambuddy's filesystem under `/app/data/virtual_printer/uploads//`; the printer's actual SD card is irrelevant on that path, so forcing "storage available" is correct for the queue/immediate/review modes the cached-as-base path covers. Restores 0.2.3.2's working behaviour for these specific fields without losing the live AMS / k-profile / camera mirror that cached-as-base provides. **Tests:** new `test_storage_indicators_overlaid_for_send_preflight` (verifies SD bit OR'd onto a partial `home_flag`, `sdcard=True` forced even when real says False, `storage` injected when cache lacks it, free/total are non-zero) and `test_storage_indicators_preserve_real_storage_when_present` (real `home_flag=0x100` stays `0x100`, real `storage={free, total}` passes through unchanged so the overlay never overrides what the printer actually reported) in `test_vp_mqtt_bridge.py::TestStatusReportCachedAsBase`. Existing 25 tests in that suite still pass. - **MFA at-rest encryption is now default-on via auto-bootstrap** ([#1219](https://github.com/maziggy/bambuddy/issues/1219)) — Default Docker installs ran with `MFA_ENCRYPTION_KEY` unset, which silently fell back to plaintext storage for OIDC `client_secret` and TOTP secret rows. The single startup `logger.warning` was the only signal, and `.env.example` / `docker-compose.yml` / Settings UI never mentioned the variable, so any operator who wired up SSO or asked users to enroll in 2FA had to read the warning in the logs to know their secrets were unprotected at rest. **Auto-bootstrap:** `backend/app/core/encryption.py` now resolves the encryption key with the same precedence pattern as `_get_jwt_secret` — `MFA_ENCRYPTION_KEY` env var → `DATA_DIR/.mfa_encryption_key` file → auto-generated Fernet key written with mode `0o600`. The new helper `backend/app/core/paths.py:resolve_data_dir()` is shared with `auth.py` (DRY) and reads the env fresh on every call so test fixtures can override `DATA_DIR` per-test. Invalid env-var values (anything that doesn't decode to exactly 32 bytes via URL-safe base64) are rejected with a `logger.error` and the loader falls through to the file/auto-generate branches instead of crashing the encrypt/decrypt path with `ValueError`. **Re-encryption migration:** `_migrate_encrypt_legacy_secrets()` runs once on every startup after `run_migrations(conn)` finishes — it opens its own `async_session()` (separate from the schema-DDL connection, to avoid SQLite WAL lock contention) and converts any `oidc_providers.client_secret` / `user_totp.secret` row whose value doesn't already start with `fernet:` to the encrypted form via the existing property setters. The migration is idempotent (prefix check) and is a no-op when no key is loaded, so it can run safely on installs that never opt in. **Status endpoint + UI:** new `GET /api/v1/auth/encryption-status` (admin-only, gated on `Permission.SETTINGS_READ`) returns `key_configured`, `key_source ∈ {env, file, generated, none}`, plus per-table `legacy_plaintext_rows` and `encrypted_rows` counts and a derived `decryption_broken` flag (true iff encrypted rows exist but no key is loadable — the Phase-2 "operator deleted the key after rows were encrypted" recovery scenario). The new `frontend/src/components/SecurityStatusCard.tsx` lives in a new "Security" sub-tab under Settings → Authentication and renders four severity levels: green when everything is encrypted and a key is loaded, yellow when legacy plaintext rows still need re-encryption, orange when the key was auto-generated (with a backup hint pointing at `DATA_DIR/.mfa_encryption_key`), and red when `decryption_broken` is true. **Backup integration:** `routes/settings.py:create_backup_zip` now includes `.mfa_encryption_key` as a ZIP top-level entry (alongside `bambuddy.db`) so a self-contained backup can be restored to a fresh host without losing access to encrypted secrets. The matching `routes/settings.py:restore_backup` extracts the file back into `DATA_DIR` with `chmod(0o600)` and validates the basename exactly (`/`, `..`, `\\` rejected) so a manipulated ZIP cannot path-traverse outside `DATA_DIR`. If the file is absent from the ZIP (legacy backup) the restore proceeds without error — the next boot will auto-bootstrap a fresh key, and any plaintext rows that come back from the backup remain readable via the existing legacy-plaintext fallback in `mfa_decrypt`. **Test isolation:** new autouse `mfa_encryption_isolation` fixture in `conftest.py` per-test points `DATA_DIR` at a `tmp_path`, clears `MFA_ENCRYPTION_KEY` from env, and resets the `_fernet_instance` / `_warn_shown` / `_key_source` module globals — so the auto-bootstrap can never write a real key file into the repo and pytest-xdist workers don't share encryption state. **i18n:** new `settings.encryption.*` namespace and `settings.tabs.security` label across all 8 locales (en + de fully translated; fr/it/ja/pt-BR/zh-CN/zh-TW seeded with English copy pending native translation, matching the project's existing flow for newly-added keys). **Docs:** `.env.example` documents the new variable + the backup self-containment behaviour; `docker-compose.yml` carries an auto-commented entry; `.gitignore` adds `.mfa_encryption_key` alongside the existing `.jwt_secret` project-root guard. **Tests:** 9 new unit tests in `TestEncryption` (env/file/generated key sources, invalid-env fall-through, OSError → `none`, mode `0o600` check), 6 new in `TestEncryptLegacyMigration` (plaintext → encrypted for OIDC + TOTP, idempotent re-run, mixed state, no-op without key, log assertion), 8 new in `TestEncryptionStatusEndpoint` (each `key_source`, count assertions, `decryption_broken` recovery scenario, `Permission.SETTINGS_READ` gate), 2 new in `TestEncryptionRoundtrip` (raw column reads return ciphertext, property reads return plaintext for both OIDC and TOTP), 6 new in `TestBackupKeyFiles` (ZIP includes / skips key files, restore chmod `0o600`, missing-file tolerance, path-traversal rejection), and 6 new frontend tests in `SecurityStatusCard.test.tsx` (each severity level + the disabled state). diff --git a/backend/app/api/routes/spoolbuddy.py b/backend/app/api/routes/spoolbuddy.py index bdb7b766c..dc69a7746 100644 --- a/backend/app/api/routes/spoolbuddy.py +++ b/backend/app/api/routes/spoolbuddy.py @@ -1123,7 +1123,14 @@ async def queue_system_command( device_id: str, req: SystemCommandRequest, db: AsyncSession = Depends(get_db), - _: User | None = RequirePermissionIfAuthEnabled(Permission.SETTINGS_UPDATE), + # Aligns with the rest of the kiosk-scoped device routes (calibration, + # display, cancel-write, command-result — all INVENTORY_UPDATE). The + # previous SETTINGS_UPDATE gate locked operators out of the QuickMenu's + # Restart-Daemon / Restart-Browser / Reboot / Shutdown buttons even + # though they had access to every other operation on the same device. + # Reboot and shutdown remain recoverable via physical access — the + # operator already has the kiosk in front of them. + _: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_UPDATE), ): """Queue a system command (reboot, shutdown, restart_daemon, restart_browser) for the SpoolBuddy device.""" if req.command not in VALID_SYSTEM_COMMANDS: diff --git a/backend/tests/integration/test_settings_api_key_scrubbing.py b/backend/tests/integration/test_settings_api_key_scrubbing.py index 0c8543d7f..53576c1f1 100644 --- a/backend/tests/integration/test_settings_api_key_scrubbing.py +++ b/backend/tests/integration/test_settings_api_key_scrubbing.py @@ -107,7 +107,15 @@ class TestSettingsScrubForApiKey: class TestRceEndpointPermissions: - """T-Gap 2: System command endpoints require SETTINGS_UPDATE permission.""" + """T-Gap 2 (revised): system_command was originally gated on SETTINGS_UPDATE + but that locked out kiosk operators (who hold INVENTORY_UPDATE-only keys) + from the QuickMenu's Restart-Daemon / Restart-Browser / Reboot / Shutdown + buttons — the only way to recover the kiosk from the kiosk itself. Risk + is bounded: only the 4 named commands are accepted (no RCE), reboot and + shutdown require physical-access recovery, and the same operator already + controls printers + weighs spools on the same device. The /update route + (full firmware upgrade) keeps SETTINGS_UPDATE because it can replace the + daemon binary, which is a different threat surface.""" @pytest.fixture async def auth_enabled(self, db_session): @@ -151,7 +159,7 @@ class TestRceEndpointPermissions: @pytest.mark.asyncio @pytest.mark.integration - async def test_system_command_requires_settings_update( + async def test_system_command_accepts_inventory_update( self, async_client: AsyncClient, db_session, @@ -159,12 +167,20 @@ class TestRceEndpointPermissions: inventory_only_api_key, spoolbuddy_device, ): + """T-Gap 2 (revised): system_command was lowered from SETTINGS_UPDATE + to INVENTORY_UPDATE so kiosk operators can use the QuickMenu buttons. + An inventory-only key must NOT 403 — it should reach the route's + device-state check (and 409 for offline device, since the test + fixture doesn't set last_seen). + """ resp = await async_client.post( f"/api/v1/spoolbuddy/devices/{spoolbuddy_device.device_id}/system/command", json={"command": "reboot"}, headers={"X-API-Key": inventory_only_api_key}, ) - assert resp.status_code == 403 + # Permission accepted — fails on device-state, not on auth. + assert resp.status_code == 409 + assert "offline" in resp.json()["detail"].lower() @pytest.mark.asyncio @pytest.mark.integration