fix: render Swagger UI at /docs with a docs-scoped CSP

The global CSP set script-src 'self', so FastAPI's /docs page rendered
  blank: the inline boot <script> and the cdn.jsdelivr.net swagger-ui
  bundle/CSS were both blocked. /redoc and /docs/oauth2-redirect had the
  same problem.

  Branch the security_headers_middleware to emit a docs-scoped CSP for
  those three paths that allows cdn.jsdelivr.net (scripts + styles), the
  FastAPI/Redoc favicon hosts (images), and 'unsafe-inline' for the
  inline boot script. Every other route keeps the stricter SPA policy
  unchanged.
This commit is contained in:
maziggy
2026-04-25 11:29:16 +02:00
parent 12c01f029d
commit 30cf384b5a
3 changed files with 18 additions and 1 deletions
+1
View File
@@ -22,6 +22,7 @@ All notable changes to Bambuddy will be documented in this file.
- **Settings page: permission-gated instead of admin-only** — the Settings sidebar entry has always been visible to any user holding `settings:read`, but the route guard required admin role, so a non-admin with `settings:read` would see the entry, click it, and get silently redirected back to the dashboard. The route guard now matches the sidebar: any user with `settings:read` can open the page, and the individual tabs / cards continue to enforce their own per-feature permissions (`users:read`, `groups:update`, `oidc:*`, etc. — many of them admin-only, some not). Group editor routes moved to permission-based guards too (`groups:create` for `/groups/new`, `groups:update` for `/groups/:id/edit`), so permission delegation works end-to-end. Admins retain full access since admins implicitly hold every permission.
### Fixed
- **Swagger UI link in Settings → API Keys rendered a blank page** — the global CSP applied by `security_headers_middleware` set `script-src 'self'` and `style-src 'self' 'unsafe-inline' https://fonts.googleapis.com`, which blocked both the inline `<script>` that boots Swagger and the `cdn.jsdelivr.net` URL that ships `swagger-ui-bundle.js` / `swagger-ui.css`. FastAPI's `/docs` page therefore loaded a 1 KB shell with no JS executed, leaving an empty white page. The middleware now emits a docs-scoped CSP for `/docs`, `/redoc`, and `/docs/oauth2-redirect` that allows `https://cdn.jsdelivr.net` for scripts + styles, the FastAPI/Redoc favicon hosts for images, and `'unsafe-inline'` for the Swagger boot script — every other route keeps the unchanged stricter SPA policy.
- **Camera stream second viewer fails / kicks the first off** ([#1089](https://github.com/maziggy/bambuddy/issues/1089)) — Most Bambu Lab printers only allow one concurrent camera connection (RTSP socket on X1/H2/P2, port-6000 chamber-image socket on A1/P1), but `GET /printers/{id}/camera/stream` opened a fresh upstream per viewer keyed on a per-request `stream_id`. Two browser tabs / two dashboard cards → the second viewer either failed silently or kicked the first one off. New `services/camera_fanout.py::MjpegBroadcaster` owns a single upstream per printer and fans pre-formatted MJPEG chunks out to N subscriber queues; new viewers tap the existing connection. When the last subscriber leaves, the upstream stays alive for a 5 s grace window so a tab refresh or "open in new tab" doesn't pay an ffmpeg/RTSP reconnect, then tears down cleanly. Per-subscriber queues are bounded (depth 4) so a slow viewer drops frames for itself rather than blocking the broadcaster — live video, old frames have no value. Stop endpoint and app-shutdown both call into the broadcaster's force-shutdown path so subscribers wake up via an upstream-gone sentinel instead of hanging on `queue.get()`. External-camera path is unchanged (user-supplied MJPEG/RTSP servers handle multi-viewer themselves). The upstream uses a deterministic `{printer_id}-fanout` stream id so every existing prefix-match in `cleanup_orphaned_streams`, `camera_status`, the snapshot fall-through in `main.py`, and the `stop` endpoint continues to find it without changes. Two follow-up correctness fixes from the audit pass: (1) `_stream_start_times[printer_id]` is now set with `setdefault()` so `/camera/status` reports the SHARED upstream's age — previously each new viewer overwrote it, making `stream_uptime` jump backward whenever a second viewer attached; (2) the route now retries `subscribe()` once on `RuntimeError` to close a tiny race where the grace teardown can flip the broadcaster to `stopped` between the registry lookup and the subscribe call (the retry forces the registry to mint a fresh broadcaster). Detach log line shows the post-unsubscribe count returned atomically by `unsubscribe()` — no more two viewers leaving simultaneously both reporting `subscribers=0`. Permission gates unchanged: `/camera/stream` still requires the existing token (minted by `POST /camera/stream-token` with `CAMERA_VIEW`); `/camera/stop` still requires `CAMERA_VIEW`; the broadcaster is internal infra with no FastAPI surface. 13 unit tests for the broadcaster (single subscriber, multi-subscriber-shares-one-pump, slow-subscriber-doesn't-block-fast, grace-window teardown, grace-cancelled-on-rejoin, force-shutdown sentinel, `iter_subscriber` exits on upstream-gone and on client-disconnect, registry replaces stopped broadcasters, `subscribe()` raises on stopped broadcaster, `unsubscribe()` returns post-removal count atomically across concurrent leavers, double-unsubscribe is idempotent, and the route's force-shutdown-then-fresh-subscribe retry path) plus 2 new integration tests on the stop endpoint covering the deterministic fan-out stream id and the `shutdown_broadcaster` wiring. Thanks to @swheettaos for the diagnosis and broadcaster sketch.
- **Uploads to writable external folders silently landed in internal storage** ([#1112](https://github.com/maziggy/bambuddy/issues/1112)) — `LibraryFolder` has an `external_readonly` flag, so the model already distinguishes writable from read-only external mounts, but `POST /library/files` rejected only the read-only branch and then unconditionally wrote to `get_library_files_dir()` with a UUID-scoped filename. The resulting `LibraryFile` row linked back to the external folder via `folder_id`, so the file showed up in the Bambuddy UI and could be printed, but the bytes physically lived in `archive/library/files/` and never touched the mount — invisible from any other machine accessing the same NAS/SMB share. New `_resolve_upload_destination()` helper detects writable external targets and writes through to `<external_path>/<filename>` (keeping the original filename so the file is recognisable on the mount), with guards for missing/inaccessible path (400), non-writable mount (400), pre-existing filename on the mount (409 — no silent overwrite; the user is expected to rename and retry, matching how scan treats external files as externally-owned bytes), and a `resolve + relative_to` path-traversal guard on the joined destination. DB row now matches what scan produces: `is_external=True`, `file_path=<absolute external path>`, so the existing download / delete / dedupe paths work unchanged (`to_absolute_path` already fast-paths `is_absolute()` inputs, and external-file deletion already bypasses trash and only drops the DB row + internal thumbnail). `POST /library/files/extract-zip` is now rejected against *any* external folder (not just read-only) with a clear "extract the ZIP on the external mount and run Scan" message — the nested-subfolder creation path would need to `mkdir` on the mount and create matching `is_external=True` `LibraryFolder` rows, which is a separate design round, and the Scan flow already handles that shape. 7 new integration tests cover: bytes land on the mount; DB row has `is_external=True` + absolute `file_path`; filename collision → 409 with prior bytes preserved; vanished external path → 400; path-traversal filename never escapes the external dir; extract-zip into writable external rejected with the Scan hint; root uploads unchanged.
- **Queue item stuck at "printing" when print failed before reaching RUNNING** ([#1111](https://github.com/maziggy/bambuddy/issues/1111)) — Dispatching a file sliced for the wrong nozzle size (or any other pre-print error: AMS fault, wrong plate, nozzle not installed, etc.) left the queue item stuck at `status="printing"` forever, blocking every subsequent pending item for that printer (`check_queue` seeds `busy_printers` from any row in `'printing'` state and skips further dispatches for those printer IDs). Completion detection in `BambuMQTTClient._process_message` required the print to have reached `RUNNING` — either via `_previous_gcode_state == "RUNNING"` or the `_was_running` fallback — but a nozzle-mismatch failure transitions the printer `IDLE → PREPARE → FAILED` without ever entering `RUNNING`, so neither branch matched and `on_print_complete` never fired. The diagnostic log line at `bambu_mqtt.py:2690` ("State is FAILED but completion NOT triggered: prev=PREPARE, was_running=False") confirmed the path. Completion now also fires on `FAILED` from a pre-print state (`PREPARE` or `SLICING`) — restricted to those two so a stale `FAILED` on first connection (prev=None) still can't accidentally advance an unrelated queue item. Additionally, when a queue item transitions to `failed` the handler in `main.py` now populates `error_message` from the printer's current HMS error list, rendered via the existing `backend/app/services/hms_errors.py` lookup table (e.g. `[0500_4038] The nozzle diameter in sliced file is not consistent with the current nozzle setting. This file can't be printed.`) — previously `error_message` was left `NULL`, so users saw "failed" with no hint at the cause. 5 new unit tests in `TestPrePrintFailureCompletion` cover PREPARE→FAILED and SLICING→FAILED firing, IDLE→FAILED and initial-FAILED *not* firing (boot-time safety), and HMS errors being passed through in the callback payload; 6 new tests in `test_hms_error_summary.py` cover the error-message formatter (known-code lookup, unknown-code fallback, multi-error join, malformed-entry tolerance, all-malformed → None, empty → None). Thanks to @MartinNYHC for the report.
+16
View File
@@ -4417,6 +4417,22 @@ async def security_headers_middleware(request, call_next):
"frame-src 'self' http: https:; "
"frame-ancestors 'self';"
)
elif request.url.path in ("/docs", "/redoc", "/docs/oauth2-redirect"):
# FastAPI's built-in Swagger UI / ReDoc pages load assets from
# cdn.jsdelivr.net and bootstrap with an inline <script>, so the
# default CSP would render a blank page.
response.headers["Content-Security-Policy"] = (
"default-src 'self'; "
"script-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; "
"style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net https://fonts.googleapis.com; "
"img-src 'self' data: blob: https://fastapi.tiangolo.com https://cdn.redoc.ly; "
"connect-src 'self'; "
"font-src 'self' data: https://fonts.gstatic.com; "
"worker-src 'self' blob:; "
"object-src 'none'; "
"base-uri 'self'; "
"frame-ancestors 'none';"
)
else:
response.headers["Content-Security-Policy"] = (
"default-src 'self'; "
@@ -10,7 +10,7 @@
* refreshed view; covered indirectly via the create-then-refresh flow).
*/
import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { screen, waitFor, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { http, HttpResponse } from 'msw';