fix(settings): confine settings scopes to the caller's own on multi-tenant hosts (#368)

CHANGELOG.md

@@ -101,6 +101,17 @@ All notable changes to this project are documented in this file. The format is b
   (#363).
 
 ### Security
+- **Settings scopes on multi-tenant hosts** (#368). The settings routes checked
+  `settings.view`/`.edit`/`.delete` only, so any holder could write the
+  host-wide system scope and any tenant's or user's scope by naming it in the
+  URL. With `multi_tenant` on, a caller now reaches only its own tenant scope
+  (the request's tenant) and its own user scope; the system scope, other
+  scopes, `resolve` for another tenant/user, id-based CRUD, the module-settings
+  API and the `/admin/settings` screens need a platform settings admin — the
+  new `settings.system` permission (held by `admin` via the wildcard). On a
+  host **without** a tenant resolver, a user whose record carries a
+  `tenant_id` is that tenant's admin and never a platform one, even with the
+  `admin` role. Single-tenant hosts are unchanged.
 - The tenant header (`tenant_header`) is no longer honoured for an
   authenticated user without a tenant of their own: such a user could name any
   tenant. On the legacy path it applies to anonymous requests only; with the

docs/framework/multi-tenancy.md

@@ -289,6 +289,17 @@ async def test_same_tenant(tenant_client):
         ...
 ```
 
+## Host-wide settings
+
+The `settings` module's scopes follow the same rule (#368): with `multi_tenant`
+on, `settings.*` lets a caller manage its **own** tenant scope (the request's
+tenant) and its own user scope. The system scope — where every module's
+DB-backed configuration lives — other tenants' and users' scopes, and the
+`/admin/settings` screens need `settings.system`, which no tenant role should
+be mapped to. On a host without a tenant resolver, a user with a `tenant_id` on
+their record never counts as a platform admin, whatever their roles: there the
+global `admin` role is the only admin role a tenant's operator can have.
+
 ## Unique keys
 
 On a tenant-scoped table every business key is per tenant: put `tenant_id` in

modules/settings/settings/constants.py

@@ -90,11 +90,19 @@
 PERM_CREATE: Final = "settings.create"
 PERM_EDIT: Final = "settings.edit"
 PERM_DELETE: Final = "settings.delete"
-# Edit the *active* tenant's overrides of ``tenant_overridable`` keys — and
-# nothing else. Mapped onto ``tenant:owner`` / ``tenant:admin``; the four
-# above stay platform-operator permissions (system scope, any tenant's scope).
+# Cross-scope platform administration on multi-tenant hosts (GH #368).
+# Never granted to tenant roles; see ``settings.scope_guard``.
+PERM_SYSTEM: Final = "settings.system"
+# Self-service edits to the active tenant's overridable keys (GH #382).
 PERM_TENANT_EDIT: Final = "settings.tenant.edit"
-ALL_PERMISSIONS: Final = (PERM_VIEW, PERM_CREATE, PERM_EDIT, PERM_DELETE, PERM_TENANT_EDIT)
+ALL_PERMISSIONS: Final = (
+    PERM_VIEW,
+    PERM_CREATE,
+    PERM_EDIT,
+    PERM_DELETE,
+    PERM_SYSTEM,
+    PERM_TENANT_EDIT,
+)
 
 # ── Cache invalidation ───────────────────────────────────────────────
 # Published (after commit) for every SYSTEM / TENANT write, keyed per

modules/settings/settings/endpoints/api.py

@@ -38,6 +38,13 @@
     SettingUpsert,
 )
 from settings.deps import get_setting_service
+from settings.scope_guard import (
+    require_listable,
+    require_own_tenant,
+    require_own_user,
+    require_platform,
+    require_resolvable,
+)
 from settings.service import SettingService
 from settings.tenant_scope import (
     is_known_tenant,
@@ -57,6 +64,13 @@
 _EDIT = [Depends(RequiresPermission(PERM_EDIT))]
 _DELETE = [Depends(RequiresPermission(PERM_DELETE))]
 
+# On a multi-tenant host the permissions above say what, not whose: each route
+# also names the scopes it may touch (GH #368, ``settings.scope_guard``). The
+# permission check runs first, so a caller without it still gets its 401/403.
+_PLATFORM = [Depends(require_platform)]
+_OWN_TENANT = [Depends(require_own_tenant)]
+_OWN_USER = [Depends(require_own_user)]
+
 
 def _not_found() -> HTTPException:
     return HTTPException(status_code=STATUS_NOT_FOUND, detail=ERR_SETTING_NOT_FOUND)
@@ -65,7 +79,7 @@ def _not_found() -> HTTPException:
 # ── List / filter ───────────────────────────────────────────────────
 
 
-@router.get("/", response_model=list[SettingOut], dependencies=_VIEW)
+@router.get("/", response_model=list[SettingOut], dependencies=[*_VIEW, Depends(require_listable)])
 async def list_settings(
     scope: SettingScope | None = Query(default=None, alias=QP_SCOPE),
     scope_id: str = Query(default=SYSTEM_SCOPE_ID, alias=QP_SCOPE_ID),
@@ -79,7 +93,9 @@ async def list_settings(
 # ── Resolution (USER > TENANT > SYSTEM) ─────────────────────────────
 
 
-@router.get(API_RESOLVE_PATH, response_model=SettingOut, dependencies=_VIEW)
+@router.get(
+    API_RESOLVE_PATH, response_model=SettingOut, dependencies=[*_VIEW, Depends(require_resolvable)]
+)
 async def resolve_setting(
     key: str,
     user_id: str | None = Query(default=None, alias=QP_USER_ID),
@@ -95,7 +111,7 @@ async def resolve_setting(
 # ── Scoped (system / tenant / user) ─────────────────────────────────
 
 
-@router.get(API_SYSTEM_PATH, response_model=SettingOut, dependencies=_VIEW)
+@router.get(API_SYSTEM_PATH, response_model=SettingOut, dependencies=[*_VIEW, *_PLATFORM])
 async def get_system_setting(
     key: str, service: SettingService = Depends(get_setting_service)
 ) -> SettingOut:
@@ -105,7 +121,7 @@ async def get_system_setting(
     return result
 
 
-@router.put(API_SYSTEM_PATH, response_model=SettingOut, dependencies=_EDIT)
+@router.put(API_SYSTEM_PATH, response_model=SettingOut, dependencies=[*_EDIT, *_PLATFORM])
 async def upsert_system_setting(
     key: str,
     data: SettingUpsert,
@@ -114,21 +130,20 @@ async def upsert_system_setting(
     return await service.upsert_scoped(SettingScope.SYSTEM, SYSTEM_SCOPE_ID, key, data)
 
 
-@router.delete(API_SYSTEM_PATH, status_code=STATUS_NO_CONTENT, dependencies=_DELETE)
+@router.delete(API_SYSTEM_PATH, status_code=STATUS_NO_CONTENT, dependencies=[*_DELETE, *_PLATFORM])
 async def delete_system_setting(
     key: str, service: SettingService = Depends(get_setting_service)
 ) -> None:
     if not await service.delete_scoped(SettingScope.SYSTEM, SYSTEM_SCOPE_ID, key):
         raise _not_found()
 
 
-# Platform-operator routes: the tenant comes from the URL, so it must name a
-# real tenant (#382). DELETE stays unvalidated so a row left behind by a
-# deleted tenant can still be cleared — but while the tenant exists, a key set
-# by upload is cleared through its upload route, which reaps the file.
+# Explicit tenant-id routes require either the caller's own tenant or platform
+# authority (#368). Reads/writes validate that the tenant still exists (#382);
+# DELETE stays unvalidated so orphaned rows can still be cleared.
 
 
-@router.get(API_TENANT_PATH, response_model=SettingOut, dependencies=_VIEW)
+@router.get(API_TENANT_PATH, response_model=SettingOut, dependencies=[*_VIEW, *_OWN_TENANT])
 async def get_tenant_setting(
     scope_id: str,
     key: str,
@@ -142,7 +157,7 @@ async def get_tenant_setting(
     return result
 
 
-@router.put(API_TENANT_PATH, response_model=SettingOut, dependencies=_EDIT)
+@router.put(API_TENANT_PATH, response_model=SettingOut, dependencies=[*_EDIT, *_OWN_TENANT])
 async def upsert_tenant_setting(
     scope_id: str,
     key: str,
@@ -155,7 +170,9 @@ async def upsert_tenant_setting(
     return await service.upsert_scoped(SettingScope.TENANT, scope_id, key, data)
 
 
-@router.delete(API_TENANT_PATH, status_code=STATUS_NO_CONTENT, dependencies=_DELETE)
+@router.delete(
+    API_TENANT_PATH, status_code=STATUS_NO_CONTENT, dependencies=[*_DELETE, *_OWN_TENANT]
+)
 async def delete_tenant_setting(
     scope_id: str,
     key: str,
@@ -165,7 +182,7 @@ async def delete_tenant_setting(
         raise _not_found()
 
 
-@router.get(API_USER_PATH, response_model=SettingOut, dependencies=_VIEW)
+@router.get(API_USER_PATH, response_model=SettingOut, dependencies=[*_VIEW, *_OWN_USER])
 async def get_user_setting(
     scope_id: str,
     key: str,
@@ -177,7 +194,7 @@ async def get_user_setting(
     return result
 
 
-@router.put(API_USER_PATH, response_model=SettingOut, dependencies=_EDIT)
+@router.put(API_USER_PATH, response_model=SettingOut, dependencies=[*_EDIT, *_OWN_USER])
 async def upsert_user_setting(
     scope_id: str,
     key: str,
@@ -187,7 +204,7 @@ async def upsert_user_setting(
     return await service.upsert_scoped(SettingScope.USER, scope_id, key, data)
 
 
-@router.delete(API_USER_PATH, status_code=STATUS_NO_CONTENT, dependencies=_DELETE)
+@router.delete(API_USER_PATH, status_code=STATUS_NO_CONTENT, dependencies=[*_DELETE, *_OWN_USER])
 async def delete_user_setting(
     scope_id: str,
     key: str,
@@ -200,7 +217,9 @@ async def delete_user_setting(
 # ── Id-based CRUD (admin tooling) ───────────────────────────────────
 
 
-@router.post("/", response_model=SettingOut, status_code=STATUS_CREATED, dependencies=_CREATE)
+@router.post(
+    "/", response_model=SettingOut, status_code=STATUS_CREATED, dependencies=[*_CREATE, *_PLATFORM]
+)
 async def create_setting(
     data: SettingCreate,
     request: Request,
@@ -217,7 +236,7 @@ async def create_setting(
         raise HTTPException(status_code=STATUS_CONFLICT, detail=ERR_SETTING_EXISTS) from exc
 
 
-@router.get(API_BY_ID_PATH, response_model=SettingOut, dependencies=_VIEW)
+@router.get(API_BY_ID_PATH, response_model=SettingOut, dependencies=[*_VIEW, *_PLATFORM])
 async def get_setting(
     setting_id: int, service: SettingService = Depends(get_setting_service)
 ) -> SettingOut:
@@ -227,7 +246,7 @@ async def get_setting(
     return result
 
 
-@router.put(API_BY_ID_PATH, response_model=SettingOut, dependencies=_EDIT)
+@router.put(API_BY_ID_PATH, response_model=SettingOut, dependencies=[*_EDIT, *_PLATFORM])
 async def update_setting(
     setting_id: int,
     data: SettingUpdate,
@@ -243,7 +262,7 @@ async def update_setting(
     return result
 
 
-@router.delete(API_BY_ID_PATH, status_code=STATUS_NO_CONTENT, dependencies=_DELETE)
+@router.delete(API_BY_ID_PATH, status_code=STATUS_NO_CONTENT, dependencies=[*_DELETE, *_PLATFORM])
 async def delete_setting(
     setting_id: int, service: SettingService = Depends(get_setting_service)
 ) -> None:

modules/settings/settings/endpoints/module_api.py

@@ -24,17 +24,21 @@
 from settings.deps import get_setting_service
 from settings.hydrate import hydrate_settings
 from settings.reload import apply_changes_and_reload
+from settings.scope_guard import require_platform
 from settings.service import SettingService
 from settings.store import SettingsStore
 
 router = APIRouter(prefix="/modules", tags=["Settings Modules"])
 
 # Per-module settings UI exposes raw secret values (mailer password, JWT
 # signing keys, etc.) — every endpoint here is gated on the same permissions
-# the scoped API uses so a non-admin can't read or mutate module config.
-_VIEW = [Depends(RequiresPermission(PERM_VIEW))]
-_EDIT = [Depends(RequiresPermission(PERM_EDIT))]
-_DELETE = [Depends(RequiresPermission(PERM_DELETE))]
+# the scoped API uses so a non-admin can't read or mutate module config. Module
+# settings live in the system scope, so on a multi-tenant host they are also
+# platform-only (GH #368).
+_PLATFORM = Depends(require_platform)
+_VIEW = [Depends(RequiresPermission(PERM_VIEW)), _PLATFORM]
+_EDIT = [Depends(RequiresPermission(PERM_EDIT)), _PLATFORM]
+_DELETE = [Depends(RequiresPermission(PERM_DELETE)), _PLATFORM]
 
 
 def _masked_fields(app: FastAPI, package: str) -> frozenset[str]:

modules/settings/settings/endpoints/views.py

@@ -55,6 +55,7 @@
 from settings.contracts.schemas import SettingUpdate
 from settings.deps import get_setting_service
 from settings.endpoints._create_form import create_from_form
+from settings.scope_guard import require_platform
 from settings.service import SettingService
 from settings.tenant_scope import tenant_update_error
 
@@ -74,8 +75,10 @@
 # their env var names, and now which of the two is in force. The matching JSON
 # API (``/api/settings/...``) has always required ``settings.view``, so leaving
 # these unguarded let any signed-in account read the same data by asking for
-# the page instead. Mutating routes add their own stricter guard on top.
-router = APIRouter(dependencies=[Depends(RequiresPermission(PERM_VIEW))])
+# the page instead. Mutating routes add their own stricter guard on top. The
+# screens span every scope, so on a multi-tenant host they are platform-only
+# (GH #368).
+router = APIRouter(dependencies=[Depends(RequiresPermission(PERM_VIEW)), Depends(require_platform)])
 
 
 @router.get(VIEW_STORE_PATH, response_model=None)