fix(webhook): check API keys with the shared check, so errors are 503s

The /webhook routes validated API keys with get_api_key, a copy of
_validate_api_key that never got its later fixes: a database error, a
key too long to hash and an unreadable stored hash all came back as a
500, and a failed last-used write left the session dirty. get_api_key
now only reads the X-API-Key or Bearer header and calls
_validate_api_key, so the webhook routes answer like every other route:
503 when the key can't be checked, 401 "Invalid API key" when it isn't
valid, an expired key included.

CHANGELOG.md

@@ -162,6 +162,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **The frontend build no longer warns about `path` and `crypto` being externalized for the STEP previewer (#2976)** — `occt-import-js`, the Emscripten build behind STEP previews, requires both modules, but only inside its `ENVIRONMENT_IS_NODE` branches; in the browser it loads its `.wasm` from the URL the preview worker passes and draws randomness from `crypto.getRandomValues`. Vite still externalized both and printed two warnings on every build. `vite.config.ts` now drops exactly those two warnings for that one package through `build.rolldownOptions.onLog`, so an externalization anywhere else, or of any other module, still shows.
 
 ### Fixed
+- **Webhook routes answer a database problem with 503, not 500** — The `/webhook/...` routes checked API keys with their own copy of the check, which missed earlier fixes. A database error while checking the key, a key too long to hash or a damaged stored key came back as a 500. They now use the same check as every other route: 503 when the key can't be checked right now, 401 when it isn't valid. An expired key now reads "Invalid API key" there too, like everywhere else.
 - **The MQTT relay publishes what the wiki says it does** — Three topics were missing or wrong. `smart_plugs/energy` was never sent: with the relay connected, each enabled plug's power, today's energy and total energy now go out once a minute (without the relay no plug is read for this, and an MQTT plug that stopped reporting is left out). `smart_plugs/on` and `/off` only came from switching a plug by hand; auto-on, auto-off, schedules, the queue powering a printer on and failure detection switching a plug off now publish them too. `maintenance/alert` always said 0 for the current value and the threshold; it now has the hours (or days) since the item was last done, its interval and a `unit` field.
 - **PostgreSQL databases from older versions get the links a new install has** — A database created before v0.2.4.1 (or before v1.2.5.2 for the last one) was missing three links: from a print log entry to its archive and to the user who started it, and from an archive to the library file it was printed from. Deleting an archive, a user or a library file left their ID behind instead of clearing it, so such entries didn't show up under "no user", for example. The links are added on the next start, and IDs of things already deleted are cleared. A print log entry without an archive also stays that way now: every start used to attach it to the newest archive with the same name on the same printer, which could be someone else's. 51 columns, mostly settings flags, also accepted an empty value there or had a default only those databases had. They now match a new install, and an empty value gets the column's default, with two exceptions so nothing gains access: an empty API key permission becomes "off", and an OIDC provider without an email claim, which so far linked no accounts, gets auto-linking turned off instead of starting to link by email. Apart from that, SQLite installs are unchanged.
 - **Spoolman: colour, material and cost from the spool weren't applied on older PostgreSQL installs** — On a PostgreSQL database created before v0.2.4.8, the start snapshot of the AMS levels was stored as text instead of JSON and read back as text. Reporting usage to Spoolman then failed after charging the spools from the 3MF, with "Spoolman usage reporting failed" in the log. Prints without a 3MF charged nothing, and no archive got the colour, material and cost of its Spoolman spools. The column is converted on the next start. Two number columns also only had single precision on PostgreSQL databases older than them and are widened, keeping the values as entered: a spool's "Reset usage" baseline (databases created before v0.2.4.2) and a REST smart plug's energy total multiplier (before v1.2.5). SQLite installs aren't affected.

backend/app/core/auth.py

@@ -1774,9 +1774,11 @@ async def get_api_key(
     x_api_key: Annotated[str | None, Header(alias="X-API-Key")] = None,
     db: AsyncSession = Depends(get_db),
 ) -> APIKey:
-    """Get and validate API key from request headers.
+    """Get and validate API key from request headers (the webhook routes).
 
-    Checks both 'Authorization: Bearer <key>' and 'X-API-Key: <key>' headers.
+    Checks both 'Authorization: Bearer <key>' and 'X-API-Key: <key>' headers;
+    the key itself is checked by _validate_api_key. An expired key is an
+    invalid key, as on every other route.
     """
     api_key_value = None
     if x_api_key:
@@ -1790,41 +1792,15 @@ async def get_api_key(
             detail="API key required. Provide 'X-API-Key' header or 'Authorization: Bearer <key>'",
         )
 
-    # Pre-filter by key_prefix to avoid O(n) pbkdf2 hashes across all enabled keys.
-    key_lookup = api_key_value[:8] if len(api_key_value) >= 8 else api_key_value
-    result = await db.execute(
-        select(APIKey).where(
-            APIKey.enabled.is_(True),
-            APIKey.key_prefix.like(
-                key_lookup.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") + "%",
-                escape="\\",
-            ),
+    # The same check as every other API-key route: a key that can't be checked
+    # (database error) is a 503 raised in there, not an invalid key.
+    api_key = await _validate_api_key(db, api_key_value)
+    if api_key is None:
+        raise HTTPException(
+            status_code=status.HTTP_401_UNAUTHORIZED,
+            detail="Invalid API key",
         )
-    )
-    api_keys = result.scalars().all()
-
-    for api_key in api_keys:
-        # Check if key matches (verify against hash)
-        if verify_password(api_key_value, api_key.key_hash):
-            # Check expiration
-            if api_key.expires_at:
-                expires = api_key.expires_at
-                if expires.tzinfo is None:
-                    expires = expires.replace(tzinfo=timezone.utc)
-                if expires < datetime.now(timezone.utc):
-                    raise HTTPException(
-                        status_code=status.HTTP_401_UNAUTHORIZED,
-                        detail="API key has expired",
-                    )
-            # Update last_used timestamp
-            api_key.last_used = datetime.now(timezone.utc)
-            await db.commit()
-            return api_key
-
-    raise HTTPException(
-        status_code=status.HTTP_401_UNAUTHORIZED,
-        detail="Invalid API key",
-    )
+    return api_key
 
 
 async def caller_is_api_key(

backend/tests/integration/test_webhook_api_key_check.py

@@ -0,0 +1,132 @@
+"""The webhook routes check an API key the way every other route does.
+
+They used ``get_api_key``, a copy of ``_validate_api_key`` that missed its
+fixes: a database error, a key too long to hash and an unreadable stored hash
+all came back as a 500, and a failed last-used write left the session dirty.
+``get_api_key`` now reads the headers and leaves the check to
+``_validate_api_key``, so these answer as they do everywhere: 503 when the key
+can't be checked, 401 when it is not a valid key.
+"""
+
+import logging
+from contextlib import contextmanager
+from datetime import datetime, timedelta, timezone
+
+import pytest
+from httpx import AsyncClient
+from sqlalchemy import event, update
+from sqlalchemy.exc import OperationalError
+
+from backend.app.models.api_key import APIKey
+
+# A webhook route that answers 404 for a printer that doesn't exist, so a
+# request that gets past the key check is easy to tell from one that doesn't.
+ROUTE = "/api/v1/webhook/printer/99999/status"
+
+
+@pytest.fixture
+async def key(db_session) -> str:
+    from backend.app.core.auth import generate_api_key
+
+    full_key, key_hash, key_prefix = generate_api_key()
+    db_session.add(
+        APIKey(name="webhook-key", key_hash=key_hash, key_prefix=key_prefix, can_read_status=True, enabled=True)
+    )
+    await db_session.commit()
+    return full_key
+
+
+@contextmanager
+def _failing(test_engine, statement_start: str):
+    """Make the next statement starting with *statement_start* fail as SQLite does when locked."""
+    state = {"failed": False}
+
+    def hook(conn, cursor, statement, parameters, context, executemany):
+        if not state["failed"] and statement.lstrip().upper().startswith(statement_start):
+            state["failed"] = True
+            raise OperationalError(statement, parameters, Exception("database is locked"))
+
+    event.listen(test_engine.sync_engine, "before_cursor_execute", hook)
+    try:
+        yield state
+    finally:
+        event.remove(test_engine.sync_engine, "before_cursor_execute", hook)
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_a_valid_key_gets_through(async_client: AsyncClient, key):
+    response = await async_client.get(ROUTE, headers={"X-API-Key": key})
+
+    assert response.status_code == 404, response.text
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_a_failed_key_lookup_is_a_503(async_client: AsyncClient, key, test_engine):
+    with _failing(test_engine, "SELECT API_KEYS") as state:
+        response = await async_client.get(ROUTE, headers={"X-API-Key": key})
+
+    assert state["failed"]
+    assert response.status_code == 503, response.text
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_a_failed_last_used_write_is_a_503(async_client: AsyncClient, key, test_engine):
+    with _failing(test_engine, "UPDATE API_KEYS") as state:
+        response = await async_client.get(ROUTE, headers={"X-API-Key": key})
+
+    assert state["failed"]
+    assert response.status_code == 503, response.text
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_an_oversized_key_is_a_401(async_client: AsyncClient, key):
+    """passlib refuses secrets over 4096 characters; that is a wrong key, not a server error."""
+    response = await async_client.get(ROUTE, headers={"X-API-Key": key[:8] + "a" * 5000})
+
+    assert response.status_code == 401, response.text
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_a_key_row_with_an_unreadable_hash_is_a_401_and_reported(
+    async_client: AsyncClient, key, db_session, caplog
+):
+    await db_session.execute(update(APIKey).values(key_hash="not-a-hash").where(APIKey.name == "webhook-key"))
+    await db_session.commit()
+
+    with caplog.at_level(logging.WARNING):
+        response = await async_client.get(ROUTE, headers={"X-API-Key": key})
+
+    assert response.status_code == 401, response.text
+    assert "unreadable hash" in caplog.text
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_an_expired_key_is_a_401(async_client: AsyncClient, key, db_session):
+    await db_session.execute(
+        update(APIKey)
+        .values(expires_at=datetime.now(timezone.utc) - timedelta(days=1))
+        .where(APIKey.name == "webhook-key")
+    )
+    await db_session.commit()
+
+    response = await async_client.get(ROUTE, headers={"X-API-Key": key})
+
+    assert response.status_code == 401, response.text
+    assert response.json()["detail"] == "Invalid API key"
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_a_wrong_key_and_no_key_are_401s(async_client: AsyncClient, key):
+    wrong = await async_client.get(ROUTE, headers={"X-API-Key": key[:-4] + "XXXX"})
+    missing = await async_client.get(ROUTE)
+
+    assert wrong.status_code == 401
+    assert wrong.json()["detail"] == "Invalid API key"
+    assert missing.status_code == 401