Przeglądaj źródła

fix(auth): API keys with Manage Library can curate library files (#1832)

    require_ownership_permission gates API keys on `all_perm` only — the
    comment at auth.py:1659 says OWN and ALL "both map to the same scope
    flag" for queue / archives / etc., so checking `all_perm` is the
    correct gate. Library deliberately broke that: LIBRARY_UPDATE_OWN /
    LIBRARY_DELETE_OWN mapped to can_manage_library, but the ALL variants
    were in _APIKEY_DENIED_PERMISSIONS. Result — every API-key request to
    DELETE /library/files/{id}, PUT /library/files/{id} (rename), or
    POST /library/files/move hit "administrative operations" 403, even
    for keys with can_manage_library=True. Only slice worked, because it
    doesn't go through require_ownership_permission.

    The "ALL stays admin-only because it crosses the user boundary"
    intent was internally inconsistent. API keys have no per-row
    ownership identity (user=None), so the route's
    `file.created_by_id != user.id` ownership check would AttributeError
    on a key acting under OWN anyway — the only working path is
    can_modify_all=True, which `all_perm` denial blocked outright.

    Fix folds LIBRARY_UPDATE_ALL and LIBRARY_DELETE_ALL into
    _APIKEY_SCOPE_BY_PERMISSION under can_manage_library, matching the
    can_queue precedent (QUEUE_UPDATE_OWN and QUEUE_UPDATE_ALL both
    map to can_queue for the same per-key-identity reason). Both removed
    from _APIKEY_DENIED_PERMISSIONS. LIBRARY_PURGE stays denied — it
    bypasses the soft-delete window and is genuinely destructive.
maziggy 2 miesięcy temu
rodzic
commit
4c79563630

Plik diff jest za duży
+ 1 - 0
CHANGELOG.md


+ 41 - 1
backend/app/api/routes/printers.py

@@ -63,6 +63,11 @@ from backend.app.utils.http import build_content_disposition
 logger = logging.getLogger(__name__)
 logger = logging.getLogger(__name__)
 router = APIRouter(prefix="/printers", tags=["printers"])
 router = APIRouter(prefix="/printers", tags=["printers"])
 
 
+# Seconds the /hms/execute-action route waits for a printer status push
+# confirming the command landed before reporting 502 to the UI. Module-level
+# so tests can monkeypatch a near-zero value instead of mocking asyncio.sleep.
+HMS_ACTION_ACK_WAIT_SECONDS = 2.5
+
 
 
 async def _caller_can_view_printer_secrets(user: User | None, db: AsyncSession) -> bool:
 async def _caller_can_view_printer_secrets(user: User | None, db: AsyncSession) -> bool:
     """Whether the caller is trusted enough to see ``access_code`` on a printer
     """Whether the caller is trusted enough to see ``access_code`` on a printer
@@ -458,7 +463,13 @@ async def get_printer_status(
     # Convert HMS errors to response format
     # Convert HMS errors to response format
     hms_errors = [
     hms_errors = [
         HMSErrorResponse(
         HMSErrorResponse(
-            code=e.code, attr=e.attr, module=e.module, severity=e.severity, actions=e.actions, job_id=e.job_id
+            code=e.code,
+            attr=e.attr,
+            module=e.module,
+            severity=e.severity,
+            actions=e.actions,
+            job_id=e.job_id,
+            full_code=e.full_code,
         )
         )
         for e in (state.hms_errors or [])
         for e in (state.hms_errors or [])
     ]
     ]
@@ -3809,8 +3820,37 @@ async def execute_hms_action(
     if not client:
     if not client:
         raise HTTPException(400, "Printer not connected")
         raise HTTPException(400, "Printer not connected")
 
 
+    # Snapshot pre-state so we can verify the printer actually acted on the
+    # command. publish() success is NOT the same as printer-ack: Bambu's
+    # firmware silently rejects malformed HMS commands at QoS 1 (the broker
+    # ACKs the publish, but the printer drops it). Verified end-to-end against
+    # a live H2D — see #1830 §(3). We sample (gcode_state, hms_errors length)
+    # because every accepted HMS action mutates at least one of them.
+    #
+    # PrinterState.state carries the MQTT `gcode_state` value verbatim (see
+    # bambu_mqtt.py line 2144); the raw `print_error` int isn't preserved on
+    # state, only the derived HMSError entries are.
+    pre_gcode = client.state.state
+    pre_hms_count = len(client.state.hms_errors)
+
     success = client.execute_hms_action(body.print_error, body.action, body.job_id)
     success = client.execute_hms_action(body.print_error, body.action, body.job_id)
     if not success:
     if not success:
         raise HTTPException(400, "Failed to execute HMS action")
         raise HTTPException(400, "Failed to execute HMS action")
 
 
+    # Give the printer time to push a state update. The dispatch helper already
+    # publishes a pushall after every command, so a fresh status should arrive
+    # within ~1s; the default 2.5s covers slower firmware variants without
+    # making the UI feel hung. Plain sleep is fine — paho's MQTT callback
+    # runs in its own thread and updates state regardless of whether this
+    # coroutine is awaiting.
+    await asyncio.sleep(HMS_ACTION_ACK_WAIT_SECONDS)
+
+    acked = client.state.state != pre_gcode or len(client.state.hms_errors) != pre_hms_count
+    if not acked:
+        # Publish succeeded but the printer's state didn't move. Almost always
+        # firmware-side silent rejection (err mismatch, command/state mismatch).
+        # 502 makes it visible at the UI instead of the 200-but-broken loop
+        # #1830 reported.
+        raise HTTPException(502, "Printer did not acknowledge HMS action within 2.5s")
+
     return {"success": True, "message": "HMS action executed"}
     return {"success": True, "message": "HMS action executed"}

+ 16 - 5
backend/app/core/auth.py

@@ -112,13 +112,21 @@ _APIKEY_SCOPE_BY_PERMISSION: dict[Permission, str] = {
     Permission.PRINTERS_AMS_RFID: "can_control_printer",
     Permission.PRINTERS_AMS_RFID: "can_control_printer",
     Permission.PRINTERS_CLEAR_PLATE: "can_control_printer",
     Permission.PRINTERS_CLEAR_PLATE: "can_control_printer",
     Permission.SMART_PLUGS_CONTROL: "can_control_printer",
     Permission.SMART_PLUGS_CONTROL: "can_control_printer",
-    # can_manage_library — file-manager scope (upload/rename/delete OWN library
+    # can_manage_library — file-manager scope (upload/rename/delete library
     # entries + MakerWorld import which downloads files into the library).
     # entries + MakerWorld import which downloads files into the library).
-    # Bulk/ALL-ownership library ops (UPDATE_ALL / DELETE_ALL / PURGE) stay
-    # admin-only because they cross the user boundary.
+    # OWN and ALL ownership variants map to the same scope so the
+    # `require_ownership_permission` checker (which gates on `all_perm`)
+    # passes the API key through. This matches `can_queue` and the
+    # archives/inventory scopes — API keys have no per-row ownership identity
+    # (line 1663), so splitting OWN/ALL across allowlist/denylist made the
+    # whole library curation surface unreachable for API keys (#1832).
+    # LIBRARY_PURGE stays admin-only as a genuinely destructive op that
+    # bypasses the soft-delete window.
     Permission.LIBRARY_UPLOAD: "can_manage_library",
     Permission.LIBRARY_UPLOAD: "can_manage_library",
     Permission.LIBRARY_UPDATE_OWN: "can_manage_library",
     Permission.LIBRARY_UPDATE_OWN: "can_manage_library",
+    Permission.LIBRARY_UPDATE_ALL: "can_manage_library",
     Permission.LIBRARY_DELETE_OWN: "can_manage_library",
     Permission.LIBRARY_DELETE_OWN: "can_manage_library",
+    Permission.LIBRARY_DELETE_ALL: "can_manage_library",
     Permission.MAKERWORLD_IMPORT: "can_manage_library",
     Permission.MAKERWORLD_IMPORT: "can_manage_library",
     # can_manage_inventory — inventory write scope. Covers the documented
     # can_manage_inventory — inventory write scope. Covers the documented
     # spool/catalog/forecast write surface AND the SpoolBuddy kiosk endpoints
     # spool/catalog/forecast write surface AND the SpoolBuddy kiosk endpoints
@@ -183,8 +191,11 @@ _APIKEY_DENIED_PERMISSIONS: frozenset[Permission] = frozenset(
         Permission.ARCHIVES_DELETE_OWN,
         Permission.ARCHIVES_DELETE_OWN,
         Permission.ARCHIVES_DELETE_ALL,
         Permission.ARCHIVES_DELETE_ALL,
         Permission.ARCHIVES_PURGE,
         Permission.ARCHIVES_PURGE,
-        Permission.LIBRARY_UPDATE_ALL,
-        Permission.LIBRARY_DELETE_ALL,
+        # LIBRARY_UPDATE_ALL / LIBRARY_DELETE_ALL moved to the allowlist
+        # under `can_manage_library` (#1832) — split between allow/deny made
+        # the whole library curation surface unreachable for API keys via
+        # `require_ownership_permission`. Purge stays denied as a genuinely
+        # destructive op.
         Permission.LIBRARY_PURGE,
         Permission.LIBRARY_PURGE,
         Permission.PROJECTS_CREATE,
         Permission.PROJECTS_CREATE,
         Permission.PROJECTS_UPDATE,
         Permission.PROJECTS_UPDATE,

+ 12 - 3
backend/app/schemas/printer.py

@@ -155,6 +155,13 @@ class HMSErrorResponse(BaseModel):
     severity: int  # 1=fatal, 2=serious, 3=common, 4=info
     severity: int  # 1=fatal, 2=serious, 3=common, 4=info
     actions: list[str] = []  # List of user-facing action keys (e.g. "CHECK_FILAMENT")
     actions: list[str] = []  # List of user-facing action keys (e.g. "CHECK_FILAMENT")
     job_id: str | None = None  # Optional job ID for actions that require it (e.g. "CHECK_ASSISTANT")
     job_id: str | None = None  # Optional job ID for actions that require it (e.g. "CHECK_ASSISTANT")
+    # Canonical hex identifier the firmware uses to match HMS-related commands.
+    # 16 chars for `hms[]`-array faults (full 64-bit attr+code), 8 chars for
+    # `print_error` faults. The frontend echoes this back as
+    # HmsActionBody.print_error so we send the firmware-recognised key, not the
+    # truncated short_code that historically caused silent command rejection
+    # (#1830, H2D wrong-plate verification).
+    full_code: str = ""
 
 
 
 
 class AMSTray(BaseModel):
 class AMSTray(BaseModel):
@@ -219,9 +226,11 @@ class AmsLabelBody(BaseModel):
 
 
 
 
 class HmsActionBody(BaseModel):
 class HmsActionBody(BaseModel):
-    # 8-char hex short code without separator (e.g. "05000070") — frontend strips
-    # the underscore from the displayed `MMMM_EEEE` before sending.
-    print_error: str = Field(..., min_length=8, max_length=8, pattern=r"^[0-9A-Fa-f]{8}$")
+    # Canonical hex identifier (HMSErrorResponse.full_code): 8 chars for
+    # `print_error`-sourced faults, 16 chars for `hms[]`-array faults whose
+    # full 64-bit code is the firmware's matching key. Length-bounded to
+    # those two valid shapes to keep stray input from reaching the dispatcher.
+    print_error: str = Field(..., min_length=8, max_length=16, pattern=r"^[0-9A-Fa-f]{8}([0-9A-Fa-f]{8})?$")
     # One of the HMSAction enum values. Length-capped to keep stray input from
     # One of the HMSAction enum values. Length-capped to keep stray input from
     # reaching the dispatcher's `match` statement.
     # reaching the dispatcher's `match` statement.
     action: str = Field(..., min_length=1, max_length=64)
     action: str = Field(..., min_length=1, max_length=64)

+ 55 - 14
backend/app/services/bambu_mqtt.py

@@ -180,6 +180,13 @@ class HMSError:
     # The `subtask_id` snapshotted from PrinterState when this error surfaced; Bambu's
     # The `subtask_id` snapshotted from PrinterState when this error surfaced; Bambu's
     # HMS-aware commands echo it back as `job_id`. None for idle errors with no job.
     # HMS-aware commands echo it back as `job_id`. None for idle errors with no job.
     job_id: str | None = None
     job_id: str | None = None
+    # Canonical hex identifier for the firmware's `err` matching: 16 chars for the
+    # 64-bit `hms[]` array path (`f"{attr:08X}{code:08X}"`), 8 chars for the
+    # 32-bit `print_error` path. The frontend echoes this back to
+    # execute_hms_action; the truncated 8-char short code that `_parse_status`
+    # used to send caused the firmware to silently reject HMS commands on H2C
+    # (#1830) and on `hms[]`-sourced faults generally.
+    full_code: str = ""
 
 
 
 
 # HMS short codes the firmware emits during normal user-cancel sequences.
 # HMS short codes the firmware emits during normal user-cancel sequences.
@@ -2740,7 +2747,15 @@ class BambuMQTTClient:
                         short_code = f"{(attr >> 16) & 0xFFFF:04X}_{code & 0xFFFF:04X}"
                         short_code = f"{(attr >> 16) & 0xFFFF:04X}_{code & 0xFFFF:04X}"
                         if short_code in _HMS_USER_ACTION_CODES:
                         if short_code in _HMS_USER_ACTION_CODES:
                             continue
                             continue
-                        actions = get_actions_for_error_code(self.serial_number[:3], short_code.replace("_", ""))
+                        # Catalog has both 8-char keys (base class) and 16-char keys
+                        # (specific variants). The full 16-char identifier preserves
+                        # the 32 bits of `attr_low` + `code_high` that the short_code
+                        # discards — that's the firmware's matching key, so try it
+                        # first and fall back to the short form.
+                        full_code = f"{attr:08X}{code:08X}"
+                        actions = get_actions_for_error_code(self.serial_number[:3], full_code)
+                        if not actions:
+                            actions = get_actions_for_error_code(self.serial_number[:3], short_code.replace("_", ""))
                         self.state.hms_errors.append(
                         self.state.hms_errors.append(
                             HMSError(
                             HMSError(
                                 code=f"0x{code:x}" if code else "0x0",
                                 code=f"0x{code:x}" if code else "0x0",
@@ -2749,6 +2764,7 @@ class BambuMQTTClient:
                                 severity=severity if severity > 0 else 2,
                                 severity=severity if severity > 0 else 2,
                                 actions=actions,
                                 actions=actions,
                                 job_id=self.state.subtask_id,
                                 job_id=self.state.subtask_id,
+                                full_code=full_code,
                             )
                             )
                         )
                         )
 
 
@@ -2820,6 +2836,9 @@ class BambuMQTTClient:
                                     severity=3,  # Warning level for print_error
                                     severity=3,  # Warning level for print_error
                                     actions=actions,
                                     actions=actions,
                                     job_id=job_id,
                                     job_id=job_id,
+                                    # print_error is already 32-bit — `f"{print_error:08X}"`
+                                    # is the firmware's matching key with no truncation.
+                                    full_code=f"{print_error:08X}",
                                 )
                                 )
                             )
                             )
 
 
@@ -5417,13 +5436,17 @@ class BambuMQTTClient:
         """Dispatch the user's choice from the HMS-error modal as a printer command.
         """Dispatch the user's choice from the HMS-error modal as a printer command.
 
 
         Args:
         Args:
-            print_error: 8-char hex short code with no separator (e.g. "05000070").
-                The frontend strips the underscore from the displayed `MMMM_EEEE`
-                before sending.
+            print_error: Canonical hex identifier for the fault — 8 chars for the
+                32-bit `print_error` path, 16 chars for the 64-bit `hms[]` path
+                (HMSError.full_code). Carried through unchanged from the route.
+                Only the `idle_ignore` branch puts it on the wire; resume / stop
+                use BambuStudio's plain shape (verified against a live H2D, the
+                `err`-bearing shape is silently rejected by the firmware).
             action: One of HMSAction's string values.
             action: One of HMSAction's string values.
             job_id: The `subtask_id` snapshotted onto the HMSError at parse-time.
             job_id: The `subtask_id` snapshotted onto the HMSError at parse-time.
-                Bambu's HMS-aware commands echo it back as `job_id`. May be None
-                for idle errors that never had a job.
+                Preserved for symmetry with the catalog but no longer sent —
+                BambuStudio's actual resume/stop commands are plain and the
+                firmware doesn't echo `job_id` back on the response either.
 
 
         Returns False when the MQTT client is offline or when `action` is unknown
         Returns False when the MQTT client is offline or when `action` is unknown
         so the route surfaces it as a 4xx rather than a silent no-op.
         so the route surfaces it as a 4xx rather than a silent no-op.
@@ -5442,34 +5465,52 @@ class BambuMQTTClient:
             )
             )
 
 
         def hms_resume():
         def hms_resume():
+            # BambuStudio's actual shape — plain resume, no err / no job_id.
+            # The `err`-bearing shape (`err`, `param: "reserve"`, `job_id`) is
+            # silently rejected by Bambu firmware on print_error- and hms[]-sourced
+            # faults alike; verified by injecting candidate shapes against a live
+            # H2D paused on a wrong-plate HMS. See #1830 §(2).
             publish(
             publish(
                 {
                 {
                     "print": {
                     "print": {
                         "command": "resume",
                         "command": "resume",
-                        "err": print_error,
-                        "param": "reserve",
-                        "job_id": job_id,
+                        "param": "",
                         "sequence_id": "0",
                         "sequence_id": "0",
                     }
                     }
                 }
                 }
             )
             )
 
 
         def hms_stop():
         def hms_stop():
+            # Same as hms_resume — BambuStudio's actual shape is plain. The
+            # `err`-bearing variant is silently rejected; verified on the H2D.
             publish(
             publish(
                 {
                 {
                     "print": {
                     "print": {
                         "command": "stop",
                         "command": "stop",
-                        "err": print_error,
-                        "param": "reserve",
-                        "job_id": job_id,
+                        "param": "",
                         "sequence_id": "0",
                         "sequence_id": "0",
                     }
                     }
                 }
                 }
             )
             )
 
 
         def hms_ignore(persistent: bool = False):
         def hms_ignore(persistent: bool = False):
-            # `idle_ignore` is BambuStudio's "dismiss this warning" command.
-            # type=0 dismisses once, type=1 hides the same warning permanently.
+            # `idle_ignore` is BambuStudio's "dismiss this warning" command for
+            # non-pause warnings. type=0 dismisses once, type=1 hides the same
+            # warning permanently.
+            #
+            # For HMS-paused state, `idle_ignore` is silently rejected by the
+            # firmware regardless of `err` (verified on a live H2D — see
+            # #1830 §(2)). The user-facing intent of "Ignore and resume" on a
+            # paused print is to continue, so we dispatch a plain resume
+            # instead. The `persistent` flag is informational in that branch —
+            # firmware can't honour "don't remind" through a resume — but the
+            # button still does what the user expects.
+            #
+            # NB: PrinterState's `state` field carries the MQTT `gcode_state`
+            # value verbatim — line 2144 stores `data["gcode_state"]` onto it.
+            if self.state.state == "PAUSE":
+                hms_resume()
+                return
             publish(
             publish(
                 {
                 {
                     "print": {
                     "print": {

+ 1 - 0
backend/app/services/printer_manager.py

@@ -1144,6 +1144,7 @@ def printer_state_to_dict(
                 "severity": e.severity,
                 "severity": e.severity,
                 "actions": e.actions,
                 "actions": e.actions,
                 "job_id": e.job_id,
                 "job_id": e.job_id,
+                "full_code": e.full_code,
             }
             }
             for e in (state.hms_errors or [])
             for e in (state.hms_errors or [])
         ],
         ],

+ 9 - 2
backend/tests/integration/test_auth_apikey_rbac.py

@@ -300,9 +300,15 @@ class TestCheckApiKeyPermissionsMatrix:
         ("PRINTERS_CONTROL", "can_control_printer", "start/stop print"),
         ("PRINTERS_CONTROL", "can_control_printer", "start/stop print"),
         ("PRINTERS_FILES", "can_control_printer", "send file to printer"),
         ("PRINTERS_FILES", "can_control_printer", "send file to printer"),
         ("SMART_PLUGS_CONTROL", "can_control_printer", "smart plug on/off"),
         ("SMART_PLUGS_CONTROL", "can_control_printer", "smart plug on/off"),
-        # can_manage_library
+        # can_manage_library — OWN and ALL ownership variants both fold into
+        # the same scope (#1832): API keys have no per-row ownership identity,
+        # so splitting OWN/ALL across allowlist/denylist made the curation
+        # surface unreachable. PURGE stays admin-only.
         ("LIBRARY_UPLOAD", "can_manage_library", "upload library file"),
         ("LIBRARY_UPLOAD", "can_manage_library", "upload library file"),
+        ("LIBRARY_UPDATE_OWN", "can_manage_library", "rename own library file"),
+        ("LIBRARY_UPDATE_ALL", "can_manage_library", "rename any library file"),
         ("LIBRARY_DELETE_OWN", "can_manage_library", "delete own library file"),
         ("LIBRARY_DELETE_OWN", "can_manage_library", "delete own library file"),
+        ("LIBRARY_DELETE_ALL", "can_manage_library", "delete any library file"),
         ("MAKERWORLD_IMPORT", "can_manage_library", "import from MakerWorld"),
         ("MAKERWORLD_IMPORT", "can_manage_library", "import from MakerWorld"),
         # can_manage_inventory
         # can_manage_inventory
         ("INVENTORY_CREATE", "can_manage_inventory", "create spool record"),
         ("INVENTORY_CREATE", "can_manage_inventory", "create spool record"),
@@ -321,7 +327,8 @@ class TestCheckApiKeyPermissionsMatrix:
         "FIRMWARE_UPDATE",
         "FIRMWARE_UPDATE",
         # Unmapped administrative (allowlist fail-closed catches these too)
         # Unmapped administrative (allowlist fail-closed catches these too)
         "PRINTERS_CREATE",
         "PRINTERS_CREATE",
-        "LIBRARY_DELETE_ALL",
+        # LIBRARY_DELETE_ALL / LIBRARY_UPDATE_ALL moved to can_manage_library
+        # under #1832 — covered by the _SCOPE_CASES matrix above.
         "LIBRARY_PURGE",
         "LIBRARY_PURGE",
         "DISCOVERY_SCAN",
         "DISCOVERY_SCAN",
     ]
     ]

+ 134 - 0
backend/tests/integration/test_library_api.py

@@ -1349,6 +1349,140 @@ class TestLibraryPermissions:
         # Viewers don't have delete_own or delete_all permissions
         # Viewers don't have delete_own or delete_all permissions
         assert response.status_code == 403
         assert response.status_code == 403
 
 
+    # ---------- #1832: API-key curation under can_manage_library ----------
+    #
+    # require_ownership_permission gates API keys on `all_perm`, but the
+    # library deliberately split UPDATE_OWN/DELETE_OWN (allowed under
+    # can_manage_library) from UPDATE_ALL/DELETE_ALL (previously denied).
+    # That made the entire curation surface (DELETE, PUT rename, POST move)
+    # unreachable for API keys, including for files the key's owner uploaded.
+    # The fix folds UPDATE_ALL/DELETE_ALL into can_manage_library so the
+    # checker passes; LIBRARY_PURGE stays admin-only.
+
+    @pytest.fixture
+    async def manage_library_key(self, db_session, auth_setup):
+        """Mint an API key owned by the admin user with can_manage_library."""
+        from backend.app.core.auth import generate_api_key
+        from backend.app.models.api_key import APIKey
+
+        admin = auth_setup["admin_user"]
+        full_key, key_hash, key_prefix = generate_api_key()
+        row = APIKey(
+            name="lib-curation",
+            key_hash=key_hash,
+            key_prefix=key_prefix,
+            user_id=admin.id,
+            can_manage_library=True,
+        )
+        db_session.add(row)
+        await db_session.commit()
+        return full_key
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_apikey_with_manage_library_can_delete_file(
+        self, async_client: AsyncClient, db_session, auth_setup, test_file, manage_library_key
+    ):
+        """Pre-#1832 this 403'd with "administrative operations" because
+        LIBRARY_DELETE_ALL wasn't in _APIKEY_SCOPE_BY_PERMISSION."""
+        from pathlib import Path
+
+        from backend.app.core.config import settings as app_settings
+
+        # Materialise the file on disk so the delete handler doesn't 500 on
+        # the path it tries to unlink.
+        file_path = Path(app_settings.base_dir) / test_file.file_path
+        file_path.parent.mkdir(parents=True, exist_ok=True)
+        file_path.write_text("test content")
+
+        response = await async_client.delete(
+            f"/api/v1/library/files/{test_file.id}",
+            headers={"X-API-Key": manage_library_key},
+        )
+        assert response.status_code == 200, response.text
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_apikey_with_manage_library_can_rename_file(
+        self, async_client: AsyncClient, db_session, auth_setup, test_file, manage_library_key
+    ):
+        """PUT /library/files/{id} is gated on LIBRARY_UPDATE_ALL/OWN. Same
+        #1832 path as delete."""
+        response = await async_client.put(
+            f"/api/v1/library/files/{test_file.id}",
+            headers={"X-API-Key": manage_library_key},
+            json={"filename": "renamed.txt"},
+        )
+        assert response.status_code == 200, response.text
+        assert response.json()["filename"] == "renamed.txt"
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_apikey_with_manage_library_can_move_file(
+        self, async_client: AsyncClient, db_session, auth_setup, test_file, manage_library_key
+    ):
+        """POST /library/files/move (bulk) is gated on LIBRARY_UPDATE_ALL/OWN
+        — same checker, same #1832 path."""
+        # Create a target folder the move can land in.
+        from backend.app.models.library import LibraryFolder
+
+        folder = LibraryFolder(name="target")
+        db_session.add(folder)
+        await db_session.commit()
+        await db_session.refresh(folder)
+
+        response = await async_client.post(
+            "/api/v1/library/files/move",
+            headers={"X-API-Key": manage_library_key},
+            json={"file_ids": [test_file.id], "folder_id": folder.id},
+        )
+        assert response.status_code == 200, response.text
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_apikey_without_manage_library_still_blocked(
+        self, async_client: AsyncClient, db_session, auth_setup, test_file
+    ):
+        """Regression guard: a key WITHOUT can_manage_library must still get
+        403 — the fix widens the allowed-permission set, it doesn't bypass
+        the per-key scope check."""
+        from backend.app.core.auth import generate_api_key
+        from backend.app.models.api_key import APIKey
+
+        admin = auth_setup["admin_user"]
+        full_key, key_hash, key_prefix = generate_api_key()
+        row = APIKey(
+            name="read-only",
+            key_hash=key_hash,
+            key_prefix=key_prefix,
+            user_id=admin.id,
+            can_read_status=True,
+            can_manage_library=False,
+        )
+        db_session.add(row)
+        await db_session.commit()
+
+        response = await async_client.delete(
+            f"/api/v1/library/files/{test_file.id}",
+            headers={"X-API-Key": full_key},
+        )
+        assert response.status_code == 403
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_apikey_with_manage_library_still_cannot_purge(
+        self, async_client: AsyncClient, db_session, auth_setup, manage_library_key
+    ):
+        """LIBRARY_PURGE deliberately stays in _APIKEY_DENIED_PERMISSIONS as
+        a genuinely destructive op that bypasses the soft-delete window.
+        can_manage_library does NOT grant it."""
+        response = await async_client.post(
+            "/api/v1/library/purge",
+            headers={"X-API-Key": manage_library_key},
+            json={"days_in_trash": 30},
+        )
+        assert response.status_code == 403
+
 
 
 class TestPrintFileUploadValidation:
 class TestPrintFileUploadValidation:
     """#1401: pre-flight rejection of unprintable uploads at the library +
     """#1401: pre-flight rejection of unprintable uploads at the library +

+ 90 - 6
backend/tests/integration/test_printers_api.py

@@ -1770,13 +1770,32 @@ class TestExecuteHMSActionAPI:
     @pytest.mark.asyncio
     @pytest.mark.asyncio
     @pytest.mark.integration
     @pytest.mark.integration
     async def test_execute_hms_action_success(self, async_client: AsyncClient, printer_factory):
     async def test_execute_hms_action_success(self, async_client: AsyncClient, printer_factory):
-        """200 happy path — dispatcher returns True, body forwarded verbatim."""
+        """200 happy path — dispatcher returns True AND printer state moves
+        within the ack-wait window. The state delta is the firmware's only
+        proof that the command landed (publish success is necessary but not
+        sufficient; see #1830 §(3))."""
         printer = await printer_factory(name="Test Printer")
         printer = await printer_factory(name="Test Printer")
 
 
         mock_client = MagicMock()
         mock_client = MagicMock()
-        mock_client.execute_hms_action.return_value = True
+        # Pre-action state — paused with a fault.
+        mock_client.state.state = "PAUSE"
+        mock_client.state.print_error = 0x05008051
+        mock_client.state.hms_errors = [object()]
 
 
-        with patch("backend.app.api.routes.printers.printer_manager") as mock_pm:
+        def _act(*_a, **_kw):
+            # Simulate the printer accepting the command and clearing the fault
+            # by the time the ack-wait expires.
+            mock_client.state.state = "FAILED"
+            mock_client.state.print_error = 0
+            mock_client.state.hms_errors = []
+            return True
+
+        mock_client.execute_hms_action.side_effect = _act
+
+        with (
+            patch("backend.app.api.routes.printers.printer_manager") as mock_pm,
+            patch("backend.app.api.routes.printers.HMS_ACTION_ACK_WAIT_SECONDS", 0.01),
+        ):
             mock_pm.get_client.return_value = mock_client
             mock_pm.get_client.return_value = mock_client
 
 
             body = {"print_error": "07008029", "action": "FILAMENT_EXTRUDED", "job_id": "task-7"}
             body = {"print_error": "07008029", "action": "FILAMENT_EXTRUDED", "job_id": "task-7"}
@@ -1808,17 +1827,82 @@ class TestExecuteHMSActionAPI:
             assert response.status_code == 400
             assert response.status_code == 400
             assert "failed" in response.json()["detail"].lower()
             assert "failed" in response.json()["detail"].lower()
 
 
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_execute_hms_action_no_printer_ack_returns_502(self, async_client: AsyncClient, printer_factory):
+        """502 when publish succeeded but printer state didn't move within the
+        ack-wait window. This is the silent-rejection failure mode #1830
+        identifies: the broker ACKs the publish at QoS 1 but the firmware
+        drops the command (err mismatch, wrong shape, state mismatch).
+        Surfacing this as 502 instead of 200 stops the UI from claiming
+        success while the modal sticks."""
+        printer = await printer_factory(name="Test Printer")
+
+        mock_client = MagicMock()
+        mock_client.state.state = "PAUSE"
+        mock_client.state.print_error = 0x05008051
+        mock_client.state.hms_errors = [object()]
+        mock_client.execute_hms_action.return_value = True  # publish "succeeded"
+        # Crucially: state does NOT change → ack-wait detects no movement.
+
+        with (
+            patch("backend.app.api.routes.printers.printer_manager") as mock_pm,
+            patch("backend.app.api.routes.printers.HMS_ACTION_ACK_WAIT_SECONDS", 0.01),
+        ):
+            mock_pm.get_client.return_value = mock_client
+
+            response = await async_client.post(
+                f"/api/v1/printers/{printer.id}/hms/execute-action", json=self._VALID_BODY
+            )
+
+            assert response.status_code == 502
+            assert "acknowledge" in response.json()["detail"].lower()
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_execute_hms_action_accepts_16_char_full_code(self, async_client: AsyncClient, printer_factory):
+        """200 for a 16-char full_code (hms[]-array-sourced fault). The
+        schema's relaxed pattern allows both 8-char (print_error) and
+        16-char (hms[]) shapes."""
+        printer = await printer_factory(name="Test Printer")
+
+        mock_client = MagicMock()
+        mock_client.state.state = "RUNNING"
+        mock_client.state.print_error = 0
+        mock_client.state.hms_errors = [object()]
+
+        def _act(*_a, **_kw):
+            mock_client.state.hms_errors = []
+            return True
+
+        mock_client.execute_hms_action.side_effect = _act
+
+        with (
+            patch("backend.app.api.routes.printers.printer_manager") as mock_pm,
+            patch("backend.app.api.routes.printers.HMS_ACTION_ACK_WAIT_SECONDS", 0.01),
+        ):
+            mock_pm.get_client.return_value = mock_client
+
+            body = {"print_error": "0C00030000020010", "action": "IGNORE_RESUME"}
+            response = await async_client.post(f"/api/v1/printers/{printer.id}/hms/execute-action", json=body)
+
+            assert response.status_code == 200
+            mock_client.execute_hms_action.assert_called_once_with("0C00030000020010", "IGNORE_RESUME", None)
+
     @pytest.mark.asyncio
     @pytest.mark.asyncio
     @pytest.mark.integration
     @pytest.mark.integration
     async def test_execute_hms_action_rejects_malformed_print_error(self, async_client: AsyncClient, printer_factory):
     async def test_execute_hms_action_rejects_malformed_print_error(self, async_client: AsyncClient, printer_factory):
-        """422 when print_error fails the ^[0-9A-Fa-f]{8}$ pattern — stray
+        """422 when print_error fails the relaxed pattern (8 OR 16 hex chars).
+        Lengths in between (9-15) and outside (7, 17+) are invalid; stray
         input can't reach the dispatcher's match statement."""
         input can't reach the dispatcher's match statement."""
         printer = await printer_factory(name="Test Printer")
         printer = await printer_factory(name="Test Printer")
 
 
         bad_bodies = [
         bad_bodies = [
             {"print_error": "0300_8070", "action": "OK_BUTTON"},  # underscore
             {"print_error": "0300_8070", "action": "OK_BUTTON"},  # underscore
-            {"print_error": "0300807", "action": "OK_BUTTON"},  # 7 chars
-            {"print_error": "030080700", "action": "OK_BUTTON"},  # 9 chars
+            {"print_error": "0300807", "action": "OK_BUTTON"},  # 7 chars (too short)
+            {"print_error": "030080700", "action": "OK_BUTTON"},  # 9 chars (between)
+            {"print_error": "030080700300807", "action": "OK_BUTTON"},  # 15 chars (between)
+            {"print_error": "0300807003008070A", "action": "OK_BUTTON"},  # 17 chars (too long)
             {"print_error": "0300GGGG", "action": "OK_BUTTON"},  # non-hex
             {"print_error": "0300GGGG", "action": "OK_BUTTON"},  # non-hex
         ]
         ]
         for body in bad_bodies:
         for body in bad_bodies:

+ 77 - 0
backend/tests/unit/services/test_bambu_mqtt.py

@@ -4928,6 +4928,83 @@ class TestHMSUserActionFiltering:
         assert mqtt_client.state.hms_errors[0].code == "0x8061"
         assert mqtt_client.state.hms_errors[0].code == "0x8061"
 
 
 
 
+class TestHMSFullCode:
+    """full_code is the firmware-matching key for HMS-related commands.
+    Truncating it to the 8-char short code is what caused #1830's silent
+    rejection on H2C, and the H2D wrong-plate path needs the print_error
+    32-bit form. Both branches must populate full_code consistently."""
+
+    @pytest.fixture
+    def mqtt_client(self):
+        from backend.app.services.bambu_mqtt import BambuMQTTClient
+
+        return BambuMQTTClient(
+            ip_address="192.168.1.100",
+            serial_number="TEST_FULLCODE",
+            access_code="12345678",
+        )
+
+    def test_hms_array_path_populates_16_char_full_code(self, mqtt_client):
+        """hms[] entries carry a 64-bit identifier (attr + code, 32 bits each).
+        The full 16-char hex is what BambuStudio uses to match err on
+        idle_ignore — the truncated short_code drops 32 bits and the firmware
+        silently rejects (#1830). Verifies the parser preserves the full
+        identifier on HMSError.full_code."""
+        # 0x07FF0200 / 0x8011 → displayed as 07FF_0200_0000_8011 in the wiki
+        mqtt_client._update_state({"hms": [{"attr": 0x07FF0200, "code": 0x8011}]})
+        assert len(mqtt_client.state.hms_errors) == 1
+        assert mqtt_client.state.hms_errors[0].full_code == "07FF02000000" + "8011"
+
+    def test_print_error_path_populates_8_char_full_code(self, mqtt_client):
+        """print_error is already 32 bits — no truncation. full_code is the
+        8-char hex form, which is exactly what the firmware matches against."""
+        mqtt_client._update_state({"print_error": 0x05008051})
+        assert len(mqtt_client.state.hms_errors) == 1
+        assert mqtt_client.state.hms_errors[0].full_code == "05008051"
+
+    def test_hms_array_catalog_lookup_tries_16_char_first(self, mqtt_client, monkeypatch):
+        """When the catalog has both an 8-char and a 16-char entry for the
+        same fault family, the 16-char (specific variant) wins. The 8-char
+        is the fallback for codes that aren't in the long-form catalog."""
+        from backend.app.services import bambu_mqtt as mod
+
+        calls = []
+
+        def fake_lookup(device, code):
+            calls.append((device, code))
+            if len(code) == 16:
+                return ["RESUME_PRINTING"]
+            return []
+
+        monkeypatch.setattr(mod, "get_actions_for_error_code", fake_lookup)
+        # SN prefix "TES" — irrelevant for the test, we mocked the lookup.
+        mqtt_client._update_state({"hms": [{"attr": 0x07FF0200, "code": 0x8011}]})
+        # 16-char lookup attempted first, then 8-char only if 16-char missed.
+        assert calls[0][1] == "07FF020000008011"
+        assert mqtt_client.state.hms_errors[0].actions == ["RESUME_PRINTING"]
+
+    def test_hms_array_catalog_falls_back_to_8_char(self, mqtt_client, monkeypatch):
+        """If the catalog has no 16-char entry, fall back to the 8-char short
+        code — that's where base-class HMS codes live."""
+        from backend.app.services import bambu_mqtt as mod
+
+        calls = []
+
+        def fake_lookup(device, code):
+            calls.append((device, code))
+            if len(code) == 16:
+                return []  # no specific variant
+            return ["CHECK_ASSISTANT"]  # base class hit
+
+        monkeypatch.setattr(mod, "get_actions_for_error_code", fake_lookup)
+        mqtt_client._update_state({"hms": [{"attr": 0x07FF0200, "code": 0x8011}]})
+        # Two lookups: 16-char miss, then 8-char hit.
+        assert len(calls) == 2
+        assert calls[0][1] == "07FF020000008011"
+        assert calls[1][1] == "07FF8011"
+        assert mqtt_client.state.hms_errors[0].actions == ["CHECK_ASSISTANT"]
+
+
 class TestForceReconnectRouting:
 class TestForceReconnectRouting:
     """#1136 — force_reconnect_stale_session routes between hard-reset (full
     """#1136 — force_reconnect_stale_session routes between hard-reset (full
     paho-client teardown, wipes the QoS 1 queue) and socket-close (the legacy
     paho-client teardown, wipes the QoS 1 queue) and socket-close (the legacy

+ 63 - 12
backend/tests/unit/services/test_hms_actions.py

@@ -88,7 +88,11 @@ class TestExecuteHmsActionDispatch:
         # tail — just confirm no command went out by inspecting the helper.
         # tail — just confirm no command went out by inspecting the helper.
         assert self._published_commands(client) == []
         assert self._published_commands(client) == []
 
 
-    def test_resume_carries_err_param_and_job_id(self, client):
+    def test_resume_is_plain_no_err_no_job_id(self, client):
+        # Verified against a live H2D — the `err`-bearing shape is silently
+        # rejected by Bambu firmware. BambuStudio sends a plain resume; we
+        # match that. job_id is accepted on the call for symmetry with the
+        # catalog but deliberately dropped from the wire. See #1830 §(2).
         ok = client.execute_hms_action("03008070", HMSAction.RESUME_PRINTING, job_id="task-42")
         ok = client.execute_hms_action("03008070", HMSAction.RESUME_PRINTING, job_id="task-42")
         assert ok is True
         assert ok is True
         cmds = self._published_commands(client)
         cmds = self._published_commands(client)
@@ -96,27 +100,56 @@ class TestExecuteHmsActionDispatch:
             {
             {
                 "print": {
                 "print": {
                     "command": "resume",
                     "command": "resume",
-                    "err": "03008070",
-                    "param": "reserve",
-                    "job_id": "task-42",
+                    "param": "",
                     "sequence_id": "0",
                     "sequence_id": "0",
                 }
                 }
             }
             }
         ]
         ]
+        assert "err" not in cmds[0]["print"]
+        assert "job_id" not in cmds[0]["print"]
 
 
     def test_proceed_falls_through_to_resume(self, client):
     def test_proceed_falls_through_to_resume(self, client):
         client.execute_hms_action("03008070", HMSAction.PROCEED, job_id="task-1")
         client.execute_hms_action("03008070", HMSAction.PROCEED, job_id="task-1")
         cmds = self._published_commands(client)
         cmds = self._published_commands(client)
         assert cmds[0]["print"]["command"] == "resume"
         assert cmds[0]["print"]["command"] == "resume"
-        assert cmds[0]["print"]["err"] == "03008070"
+        # Same plain shape as RESUME_PRINTING — no err.
+        assert "err" not in cmds[0]["print"]
 
 
-    def test_stop_carries_err_and_job_id(self, client):
+    def test_stop_is_plain_no_err_no_job_id(self, client):
+        # Same firmware silent-rejection class as resume — the `err` variant
+        # was confirmed broken on H2D-1 (PAUSE → PAUSE), the plain shape
+        # transitions to FAILED within ~2s.
         client.execute_hms_action("03008070", HMSAction.STOP_PRINTING, job_id="task-1")
         client.execute_hms_action("03008070", HMSAction.STOP_PRINTING, job_id="task-1")
         cmds = self._published_commands(client)
         cmds = self._published_commands(client)
-        assert cmds[0]["print"]["command"] == "stop"
-        assert cmds[0]["print"]["job_id"] == "task-1"
+        assert cmds[0] == {
+            "print": {
+                "command": "stop",
+                "param": "",
+                "sequence_id": "0",
+            }
+        }
+        assert "err" not in cmds[0]["print"]
+        assert "job_id" not in cmds[0]["print"]
+
+    def test_ignore_resume_dispatches_resume_when_print_paused(self, client):
+        # Verified on H2D: idle_ignore is silently rejected while gcode_state
+        # is PAUSE. The user's intent on a paused HMS modal is to continue,
+        # so IGNORE_RESUME dispatches a plain resume instead. See #1830 §(2).
+        client.state.state = "PAUSE"
+        client.execute_hms_action("03008070", HMSAction.IGNORE_RESUME)
+        cmds = self._published_commands(client)
+        assert cmds[0] == {
+            "print": {
+                "command": "resume",
+                "param": "",
+                "sequence_id": "0",
+            }
+        }
 
 
-    def test_ignore_resume_uses_idle_ignore_type_zero(self, client):
+    def test_ignore_resume_uses_idle_ignore_when_not_paused(self, client):
+        # For non-pause warnings (e.g. AMS-side prompts during printing),
+        # idle_ignore IS the correct command and the firmware honours it.
+        client.state.state = "RUNNING"
         client.execute_hms_action("03008070", HMSAction.IGNORE_RESUME)
         client.execute_hms_action("03008070", HMSAction.IGNORE_RESUME)
         cmds = self._published_commands(client)
         cmds = self._published_commands(client)
         assert cmds[0] == {
         assert cmds[0] == {
@@ -128,14 +161,32 @@ class TestExecuteHmsActionDispatch:
             }
             }
         }
         }
 
 
-    def test_dont_remind_uses_idle_ignore_type_one(self, client):
-        # DONT_REMIND_NEXT_TIME and IGNORE_NO_REMINDER_NEXT_TIME are the
-        # persistent variants — Bambu hides the warning for future prints.
+    def test_dont_remind_dispatches_resume_when_paused(self, client):
+        # The persistent variant still degrades to resume on a paused print —
+        # the "don't remind" flag can't ride along on a resume, but the user
+        # clicked an action whose top-level intent is to continue, so we
+        # honour that. The behavioural contract is documented in hms_ignore.
+        client.state.state = "PAUSE"
+        client.execute_hms_action("03008070", HMSAction.DONT_REMIND_NEXT_TIME)
+        cmds = self._published_commands(client)
+        assert cmds[0]["print"]["command"] == "resume"
+
+    def test_dont_remind_uses_idle_ignore_type_one_when_not_paused(self, client):
+        client.state.state = "RUNNING"
         client.execute_hms_action("03008070", HMSAction.DONT_REMIND_NEXT_TIME)
         client.execute_hms_action("03008070", HMSAction.DONT_REMIND_NEXT_TIME)
         cmds = self._published_commands(client)
         cmds = self._published_commands(client)
         assert cmds[0]["print"]["command"] == "idle_ignore"
         assert cmds[0]["print"]["command"] == "idle_ignore"
         assert cmds[0]["print"]["type"] == 1
         assert cmds[0]["print"]["type"] == 1
 
 
+    def test_idle_ignore_accepts_16_char_full_code(self, client):
+        # hms[]-array faults carry a 16-char full identifier. The firmware
+        # matches against the full 64-bit code; the truncated 8-char form
+        # (used pre-#1830) was silently rejected on H2C.
+        client.state.state = "RUNNING"
+        client.execute_hms_action("0C00030000020010", HMSAction.IGNORE_RESUME)
+        cmds = self._published_commands(client)
+        assert cmds[0]["print"]["err"] == "0C00030000020010"
+
     def test_filament_extruded_sends_ams_done(self, client):
     def test_filament_extruded_sends_ams_done(self, client):
         client.execute_hms_action("07008029", HMSAction.FILAMENT_EXTRUDED)
         client.execute_hms_action("07008029", HMSAction.FILAMENT_EXTRUDED)
         cmds = self._published_commands(client)
         cmds = self._published_commands(client)

+ 5 - 0
frontend/src/api/client.ts

@@ -335,6 +335,11 @@ export interface HMSError {
   severity: number;  // 1=fatal, 2=serious, 3=common, 4=info
   severity: number;  // 1=fatal, 2=serious, 3=common, 4=info
   actions?: string[];  // List of user-facing action keys (e.g. "CHECK_FILAMENT")
   actions?: string[];  // List of user-facing action keys (e.g. "CHECK_FILAMENT")
   job_id?: string;  // Optional job ID for actions that require it (e.g. "CHECK_ASSISTANT")
   job_id?: string;  // Optional job ID for actions that require it (e.g. "CHECK_ASSISTANT")
+  // Canonical hex identifier the firmware matches against — 8 chars for
+  // print_error-sourced faults, 16 chars for hms[]-array-sourced faults. Send
+  // this back as HmsActionBody.print_error so we don't truncate the 64-bit
+  // identifier into the silent-rejection short code (#1830).
+  full_code?: string;
 }
 }
 
 
 export interface HMSActionBody {
 export interface HMSActionBody {

+ 6 - 1
frontend/src/components/HMSErrorModal.tsx

@@ -1023,9 +1023,14 @@ export function HMSErrorModal({ printerName, errors, onClose, printerId, hasPerm
                               <button
                               <button
                                 key={action}
                                 key={action}
                                 onClick={() => {
                                 onClick={() => {
+                                  // full_code is the firmware-matching key (16
+                                  // chars for hms[]-array faults, 8 chars for
+                                  // print_error). Fall back to the 8-char
+                                  // shortCode for older backends that haven't
+                                  // populated it. See #1830.
                                   activateActionMutation.mutate({
                                   activateActionMutation.mutate({
                                     action,
                                     action,
-                                    print_error: shortCode.replace("_", ""),
+                                    print_error: error.full_code || shortCode.replace("_", ""),
                                     job_id: error.job_id ?? null,
                                     job_id: error.job_id ?? null,
                                   });
                                   });
                                 }}
                                 }}

Niektóre pliki nie zostały wyświetlone z powodu dużej ilości zmienionych plików