refactor(branding): follow the module layout (moves only)

CHANGELOG.md

@@ -41,6 +41,7 @@ All notable changes to this project are documented in this file. The format is b
 - `keycloak` follows the module layout: `keycloak.core.{constants,settings,state}`, `keycloak.identity.{provider,oidc,renewal,directory,persistence,tokens,jwks}` and `keycloak.web.endpoints`. `keycloak.{constants,settings,provider,jwks,tokens}` still import, with a `DeprecationWarning`, until 0.0.46.
 - `oidc` follows the module layout: `oidc.core.{constants,settings,presets,state,runtime}`, `oidc.identity.{provider,client,discovery,renewal,directory,persistence,tokens,jwks}` and `oidc.web.endpoints`. `oidc.{settings,provider,jwks}` still import, with a `DeprecationWarning`, until 0.0.46; other old `oidc.*` paths are gone.
 - `api_tokens` follows the module layout: `api_tokens.core.{constants,settings,services,tenant_cleanup,user_cleanup}`, `api_tokens.credentials.{format,scopes,owner,service,resolver,audit}` (`tokens.py` is now `credentials/format.py`), `api_tokens.web.{deps,endpoints}`, and the page components and types move to `ui/`. `api_tokens.{constants,service,resolver,tokens}` still import, with a `DeprecationWarning`, until 0.0.46.
+- `branding` follows the module layout: `branding.core.{constants,settings,services,service,presets,tenant_definitions,tenant_branding}`, `branding.assets.{images,reaper}`, `branding.web.{deps,shared_props,endpoints}`, and the form components move from `components/` to `ui/components/`. `branding.{constants,settings,services,tenant_branding,images}` and `branding.endpoints.assets` still import, with a `DeprecationWarning`, until 0.0.46.
 
 ### Fixed
 

docs/module-layout.md

@@ -216,7 +216,7 @@ when a later module PR moves files.
 | audit_log | 13 | 18 | flat |
 | auth | 8 | 20 | flat |
 | background_tasks | 49 | 83 | flat |
-| branding | 13 | 20 | flat |
+| branding | 2 (+6 shims) | 0 | follows this layout |
 | dashboard | 2 | 0 | follows this layout |
 | feature_flags | 3 (+1 shim) | 0 | follows this layout |
 | file_storage | 39 | 65 | flat |

docs/modules/branding.md

@@ -167,7 +167,7 @@ Note the exact property: the bytes must look like **some** allowed image format,
 
 ### Asset lifecycle
 
-Replacing or clearing an image deletes the file it stopped referencing, so repeated logo tweaks don't leave orphans in `file_storage` — system and tenant images alike (`branding.reaper`):
+Replacing or clearing an image deletes the file it stopped referencing, so repeated logo tweaks don't leave orphans in `file_storage` — system and tenant images alike (`branding.assets.reaper`):
 
 - **After commit.** The delete is queued with `register_on_commit` and runs, in a session of its own, once the settings write is durable. Deleting in the request removed the bytes before that: a late rollback left the setting pointing at a file that was gone.
 - **Only when unreferenced.** A file another image field of the same owner still holds — the logo and the dark logo sharing one upload — is kept; the check runs again at reap time.
@@ -265,7 +265,7 @@ page translates, with the English text as fallback. Server-side error details
 (validator messages) are still shown as sent — they come from pydantic and are
 not keyed.
 
-Reads resolve per request (`branding.tenant_branding.resolve`): the request's
+Reads resolve per request (`branding.core.tenant_branding.resolve`): the request's
 tenant overrides on top of the system theme, falling back field by field. The
 provider leaves the merged object on `request.state.branding`, which the root
 template's pre-hydration `<head>` prefers. A hand-edited invalid override

host/tests/test_module_layout.py

@@ -34,6 +34,7 @@
 # Add named domain areas here only when they contain several related features.
 DOMAIN_AREAS: dict[str, frozenset[str]] = {
     "api_tokens": frozenset({"credentials"}),
+    "branding": frozenset({"assets"}),
     "keycloak": frozenset({"identity"}),
     "mail": frozenset({"delivery"}),
     "oidc": frozenset({"identity"}),
@@ -66,12 +67,6 @@
         workers_state.py
         """.split()
     ),
-    "branding": frozenset(
-        """
-        components constants.py deps.py endpoints images.py presets.py reaper.py service.py
-        services.py settings.py shared_props.py tenant_branding.py tenant_definitions.py
-        """.split()
-    ),
     "file_storage": frozenset(
         """
         aggregates.py audit_labels.py backend_guard.py backends body_limits.py checksum.py

modules/branding/branding/assets/__init__.py

@@ -0,0 +1 @@
+"""Branding images: upload validation and reaping files nothing references."""

modules/branding/branding/assets/images.py

@@ -0,0 +1,121 @@
+"""Upload validation for branding images.
+
+The declared ``Content-Type`` on a multipart part is chosen by the caller, so
+it is a claim rather than a fact. An admin who uploads ``payload.html`` renamed
+``logo.png`` would otherwise have it stored and later served back under an
+``image/*`` type. Every accepted format therefore has to also *look* like
+itself in its first bytes.
+
+Mirrors the allow-list and signature check in the IIASA.GeoWiki Branding module
+(``GeoWikiBrandingConsts``), including its deliberate omission of SVG.
+"""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING, Final
+
+from fastapi import HTTPException, UploadFile
+
+if TYPE_CHECKING:
+    from file_storage.contracts.schemas import StoredFileOut
+    from file_storage.service import FileStorageService
+
+# Raster + icon only. SVG is excluded on purpose: it is an XML document that can
+# carry <script>, so serving one back from our own origin would be stored XSS.
+# The asset route defends in depth (attachment + nosniff), but the narrower
+# allow-list is what actually keeps executable markup out of the store.
+ALLOWED_IMAGE_TYPES: Final = frozenset(
+    {
+        "image/png",
+        "image/jpeg",
+        "image/webp",
+        "image/gif",
+        "image/x-icon",
+        "image/vnd.microsoft.icon",
+    }
+)
+
+MAX_IMAGE_BYTES: Final = 2 * 1024 * 1024  # 2 MB
+
+ALLOWED_IMAGE_HINT: Final = "Allowed: PNG, JPEG, WEBP, GIF, ICO."
+
+# Enough for the longest signature we test (WEBP needs 12 bytes).
+_PROBE_BYTES: Final = 16
+
+# (offset, literal) pairs; a format matches when every pair matches.
+_SIGNATURES: Final[tuple[tuple[tuple[int, bytes], ...], ...]] = (
+    ((0, b"\x89PNG\r\n\x1a\n"),),  # PNG
+    ((0, b"\xff\xd8\xff"),),  # JPEG
+    ((0, b"GIF87a"),),  # GIF
+    ((0, b"GIF89a"),),
+    ((0, b"RIFF"), (8, b"WEBP")),  # WEBP
+    ((0, b"\x00\x00\x01\x00"),),  # ICO (Windows icon resource)
+)
+
+
+def normalize_content_type(raw: str | None) -> str:
+    """Strip any ``; charset=…`` parameter and lowercase the media type."""
+    if not raw:
+        return ""
+    return raw.split(";", 1)[0].strip().lower()
+
+
+def has_recognized_image_signature(head: bytes) -> bool:
+    """True when ``head`` starts with the magic number of an allowed format."""
+    return any(
+        all(head[at : at + len(literal)] == literal for at, literal in signature)
+        for signature in _SIGNATURES
+    )
+
+
+async def validate_image(file: UploadFile) -> None:
+    """Reject anything that isn't a small, genuine raster/icon image.
+
+    Leaves ``file`` rewound to the start so the caller can hand it straight to
+    ``file_storage``.
+    """
+    content_type = normalize_content_type(file.content_type)
+    if content_type not in ALLOWED_IMAGE_TYPES:
+        raise HTTPException(
+            status_code=415,
+            detail=f"Unsupported image type {content_type or 'unknown'!r}. {ALLOWED_IMAGE_HINT}",
+        )
+
+    # ``size`` is populated by Starlette's multipart parser. When it isn't (a
+    # non-multipart caller), file_storage still enforces its own ceiling, so
+    # this stays an early, friendlier guard rather than the only one.
+    if file.size is not None and file.size > MAX_IMAGE_BYTES:
+        raise HTTPException(
+            status_code=413,
+            detail=f"Image exceeds {MAX_IMAGE_BYTES} bytes.",
+        )
+
+    head = await file.read(_PROBE_BYTES)
+    await file.seek(0)
+    if not has_recognized_image_signature(head):
+        raise HTTPException(
+            status_code=415,
+            detail=(
+                f"File does not look like an image — its content does not match "
+                f"{content_type!r}. {ALLOWED_IMAGE_HINT}"
+            ),
+        )
+
+
+async def store_image(
+    storage: FileStorageService, file: UploadFile, *, platform: bool = False
+) -> StoredFileOut:
+    """Hand a validated image to ``file_storage``, answering 413 when the quota is full.
+
+    ``file_storage`` enforces ``quota_bytes`` on every upload; its own endpoint
+    maps the refusal to 413, but branding calls the service directly, so an
+    unmapped ``QuotaExceededError`` would surface as a 500.
+    """
+    from file_storage.upload_support import QuotaExceededError
+
+    try:
+        return await storage.upload(file, platform=platform)
+    except QuotaExceededError:
+        raise HTTPException(
+            status_code=413, detail="The storage quota is full; the image was not saved."
+        ) from None

modules/branding/branding/reaper.py → modules/branding/branding/assets/reaper.py

@@ -26,7 +26,7 @@
 from simple_module_db import tenant_context
 from simple_module_db.callbacks import register_on_commit
 
-from branding.constants import PACKAGE, TENANT_IMAGE_FIELDS
+from branding.core.constants import PACKAGE, TENANT_IMAGE_FIELDS
 
 if TYPE_CHECKING:
     from fastapi import FastAPI