Gate firmware auto-check behind control+CSRF; skip faces with a null bounding box
Build and push server image / build-and-push (push) Successful in 39s
Build and push server image / build-and-push (push) Successful in 39s
/api/frames/{id}/firmware/check could silently stage new firmware as a
side effect (the auto-apply path, when firmware_auto_update is on and
a newer release exists) but was gated by require_frame_view instead of
require_frame_control like its sibling firmware routes, and being a
GET, was exempt from the app's CSRF check (which only applies to
non-GET/HEAD/OPTIONS). A linked viewer without control -- or a
cross-site page riding a control-holding victim's session via a plain
GET -- could trigger an unreviewed firmware install. Now POST +
require_frame_control, matching /firmware/apply-latest; the frontend's
two callers (passive poll on page load, "Check now" button) both
already handle a 409 from a non-controller gracefully via the existing
apiError()/control-banner pattern, so this doesn't change UX for a
frame's actual controller.
Separately: Immich has been observed to return a face detection entry
with a null bounding-box field (a still-pending or otherwise
incomplete detection). Both places that do arithmetic on those fields
-- image_pipeline._face_aware_crop_box (crop_faces display mode) and
face_labels.compute_face_labels (manage-menu name labels) -- crashed
with an unhandled TypeError on such an entry, taking down that frame's
whole photo instead of the intended graceful fallback. Both now skip
any face missing a bounding-box field via a shared _has_bounding_box()
check; a face list with zero valid entries already degrades cleanly to
the plain center crop (the existing inf/-inf sentinel math already
handled "no faces" correctly, it just couldn't tell "none passed
Immich" apart from "one broken entry" before).
This commit is contained in:
@@ -15,7 +15,7 @@ import io
|
||||
|
||||
from PIL import Image, ImageOps
|
||||
|
||||
from .image_pipeline import _placement_transform, logical_render_size, logical_to_native
|
||||
from .image_pipeline import _has_bounding_box, _placement_transform, logical_render_size, logical_to_native
|
||||
|
||||
# Small caps, not arbitrary: each label is its own malloc'd overlay
|
||||
# buffer on the device (see firmware/main/manage_qr_overlay.c), and the
|
||||
@@ -55,6 +55,8 @@ def compute_face_labels(preview_bytes: bytes, faces: list[dict], display_mode: s
|
||||
|
||||
labels = []
|
||||
for face in named[:MAX_LABELED_FACES]:
|
||||
if not _has_bounding_box(face):
|
||||
continue
|
||||
face_w = face.get("imageWidth") or fitted.width
|
||||
face_h = face.get("imageHeight") or fitted.height
|
||||
img_scale_x = fitted.width / face_w
|
||||
|
||||
@@ -118,6 +118,16 @@ def _plain_center_crop_box(
|
||||
return left, top, crop_w, crop_h
|
||||
|
||||
|
||||
def _has_bounding_box(face: dict) -> bool:
|
||||
"""Immich has occasionally been observed to return a face entry with
|
||||
a still-pending or otherwise incomplete bounding box (a null field)
|
||||
-- treat it as undetected rather than crash on arithmetic with None."""
|
||||
return all(
|
||||
face.get(k) is not None
|
||||
for k in ("boundingBoxX1", "boundingBoxX2", "boundingBoxY1", "boundingBoxY2")
|
||||
)
|
||||
|
||||
|
||||
def _face_aware_crop_box(
|
||||
img_width: int, img_height: int, target_width: int, target_height: int, faces: list[dict]
|
||||
) -> tuple[int, int, int, int]:
|
||||
@@ -138,6 +148,8 @@ def _face_aware_crop_box(
|
||||
min_x = min_y = float("inf")
|
||||
max_x = max_y = float("-inf")
|
||||
for face in faces:
|
||||
if not _has_bounding_box(face):
|
||||
continue
|
||||
face_w = face.get("imageWidth") or img_width
|
||||
face_h = face.get("imageHeight") or img_height
|
||||
scale_x = img_width / face_w
|
||||
|
||||
@@ -457,15 +457,22 @@ def _apply_gitea_update(db: Session, frame: Frame) -> str:
|
||||
return version
|
||||
|
||||
|
||||
@router.get("/api/frames/{frame_id}/firmware/check")
|
||||
@router.post("/api/frames/{frame_id}/firmware/check")
|
||||
def api_firmware_check(
|
||||
force: bool = False, frame: Frame = Depends(require_frame_view), db: Session = Depends(get_db)
|
||||
force: bool = False, frame: Frame = Depends(require_frame_control), db: Session = Depends(get_db)
|
||||
):
|
||||
"""Throttled check of the configured Gitea repo's latest release
|
||||
(gitea_releases.UPDATE_CHECK_INTERVAL_S). If firmware_auto_update is
|
||||
on and a newer version is found, applies it immediately; otherwise
|
||||
just reports it so the UI can offer the "Update frame" button.
|
||||
force=true (the "Check now" button) bypasses the throttle."""
|
||||
force=true (the "Check now" button) bypasses the throttle.
|
||||
|
||||
require_frame_control (not view), and POST (not GET): this can
|
||||
silently stage new firmware as a side effect (the auto-apply path
|
||||
below) exactly like /firmware/apply-latest, so it needs the same
|
||||
guard that route has -- a linked viewer without control shouldn't be
|
||||
able to trigger that, and as a GET it would've been exempt from the
|
||||
CSRF check require_user_api only applies to non-GET/HEAD/OPTIONS."""
|
||||
if not frame.firmware_update_repo_url:
|
||||
return {"enabled": False}
|
||||
|
||||
|
||||
@@ -259,7 +259,7 @@ async function loadFirmwareCheck(force) {
|
||||
const btn = document.getElementById('firmware-update-btn');
|
||||
const boardEl = document.getElementById('firmware-board');
|
||||
try {
|
||||
const resp = await fetch(`${window.FRAME_API}/firmware/check` + (force ? '?force=true' : ''));
|
||||
const resp = await fetch(`${window.FRAME_API}/firmware/check` + (force ? '?force=true' : ''), { method: 'POST' });
|
||||
if (!resp.ok) {
|
||||
if (force) {
|
||||
showStatus(false, await apiError(resp));
|
||||
|
||||
Reference in New Issue
Block a user