Fix DRFMutation/DRFDeletion IOSettings silent misconfiguration trap

Closes #1360.

`DRFMutation.IOSettings` declared `model: django.db.models.Model = None`,
`graphene_model: DjangoObjectType = None` and `serializer = None` - the
annotations typed instances, not classes, and there was no runtime guard
on the `None` defaults. A subclass that forgot to override any of these
crashed deep inside `mutate()` with `AttributeError` / `TypeError` that
the broad `except Exception` masks as a generic "internal error".
`DRFDeletion.IOSettings` had the inverse bug: `model` was never declared,
even though `DRFDeletion.mutate` dereferences `cls.IOSettings.model.objects`.

- Retype both `IOSettings` classes with
  `ClassVar[Optional[type[...]]] = None`, narrow `pk_fields` to `list[str]`
  (only used as a dict key).
- Add `_require_io_setting(cls, name)` helper that raises
  `NotImplementedError("<Subclass>.IOSettings.<name> must be set by the
  subclass.")` on missing / None attributes. `DRFMutation.mutate` checks
  `model` / `serializer` / `graphene_model`; `DRFDeletion.mutate` checks
  `model` and also raises a descriptive `ValueError` when the
  `required=False` `id` argument is absent.
- Graduate `config.graphql.base` out of the mypy baseline: remove the
  `[mypy-config.graphql.base]` section from `mypy.ini` and prune the ten
  corresponding lines from `docs/typing/mypy_baseline.txt`.
- Regression tests in
  `opencontractserver/tests/test_security_hardening.py::TestIOSettingsRequiredFieldsGuard`.

Author: claude

CHANGELOG.md

@@ -34,6 +34,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **DRFMutation/DRFDeletion `IOSettings` misconfiguration surfaces as a generic internal error** (Issue #1360, `config/graphql/base.py`): `DRFMutation.IOSettings` declared `model: django.db.models.Model = None`, `graphene_model: DjangoObjectType = None` and `serializer = None` — the type annotations were wrong (instance types, not class types, holding `None`), and there was no runtime guard. A subclass that forgot to override one of those would fail deep inside `mutate()` with an `AttributeError` (`'NoneType' object has no attribute 'objects'`) or `TypeError` (`'NoneType' object is not callable`), which the broad `except Exception` then masks as a generic "internal error" message back to the API caller. `DRFDeletion.IOSettings` had the inverse problem — it didn't declare `model` at all, even though `DRFDeletion.mutate` dereferences `cls.IOSettings.model.objects`. Discovered while working on #1332 (typing): mypy flagged four errors on `base.py:128/204/238/252` plus `base.py:93 "type[IOSettings]" has no attribute "model"`.
+  - Retyped both `IOSettings` declarations with `ClassVar[Optional[type[...]]] = None` so the annotation matches reality (holding a class, not an instance, and nullable by default). `pk_fields` narrowed from `list[str | int]` to `list[str]` — the only call sites use it as a dict key.
+  - Added `_require_io_setting(mutation_cls, name)` helper that fails fast with `NotImplementedError("<Subclass>.IOSettings.<name> must be set by the subclass.")` when a required attribute is `None` or missing. Called for `model`/`serializer`/`graphene_model` in `DRFMutation.mutate` and for `model` in `DRFDeletion.mutate`. The error is still caught by the broad `except Exception` and surfaced as "internal error" to the API, but the full descriptive message lands in the server log via `logger.error(traceback.format_exc())` — developer gets an obvious misconfiguration signal instead of an opaque `AttributeError`.
+  - Tightened `DRFDeletion.mutate` to raise `ValueError` when `kwargs[lookup_field]` is missing (the `id` GraphQL argument is `required=False`), instead of passing `None` into `from_global_id` and getting an opaque `AttributeError`.
+  - Graduated `config.graphql.base` out of the mypy baseline: removed the `[mypy-config.graphql.base]` section from `mypy.ini` and pruned the ten corresponding lines from `docs/typing/mypy_baseline.txt`. `mypy --config-file mypy.ini` is now clean on this module.
+  - Regression tests in `opencontractserver/tests/test_security_hardening.py::TestIOSettingsRequiredFieldsGuard`: the helper raises `NotImplementedError` for each of `model`/`serializer`/`graphene_model` when missing or `None`, returns the configured value when present, and the base classes expose the `None` defaults the guard relies on.
+
 - **Frontend coverage badge stuck on "unknown"** (`README.md:12`, `.github/workflows/codecov-notify.yml`, `.github/workflows/frontend.yml`, `.github/workflows/frontend-e2e.yml`, `frontend/package.json`, `frontend/yarn.lock`): PR #1322 pointed the README badge at a new merged `frontend` flag fed by a cross-workflow `lcov-result-merger@5` step inside `codecov-notify.yml`. Every `frontend-merged-coverage` upload since has landed at Codecov with `state: error` / `totals: null`, so the badge rendered "unknown" even though `frontend-unit` (31%), `frontend-component` (61%), and `frontend-e2e` (24%) were all processing correctly. Two defects in the merged lcov confirmed by local repro of the CI merge step: (1) `lcov-result-merger@5` emits a stripped lcov containing only `SF:`, `DA:`, `BRDA:`, `end_of_record` — it drops `TN:`, `FN`, `FNDA`, `FNF`, `FNH`, `LF`, `LH`, `BRF`, `BRH`, so line-summary fields required by Codecov's parser are absent; (2) Vitest v8 emits `src/...` (relative to `frontend/`) while `vite-plugin-istanbul` + `nyc report` emit `/home/runner/work/OpenContracts/OpenContracts/frontend/src/...` (absolute), and the merger keys on the literal path string so the same file appears as two records with conflicting hit counts. `codecov-notify.yml` also ran without `actions/checkout`, which Codecov's action docs explicitly recommend against. Fix: stop merging client-side and let Codecov aggregate server-side, since that is what flags are for. Each per-suite upload now declares two flags — `frontend-unit,frontend`, `frontend-component,frontend`, `frontend-e2e,frontend` — so the `frontend` flag total is the union of the three uploads computed by Codecov. `codecov-notify.yml` is reduced to its original gate-and-notify role (no artifact downloads, no `lcov-result-merger`, no merged upload). Deleted the `frontend-{unit,ct,e2e}-lcov` artifact publishes in `frontend.yml` / `frontend-e2e.yml`, removed the `lcov-result-merger@^5.0.1` devDep and the `coverage:merge` script from `frontend/package.json`, and pruned the orphaned entries from `frontend/yarn.lock`. README badge URL unchanged (`flag=frontend`).
 
 ### Changed

config/graphql/base.py

@@ -2,6 +2,7 @@
 import logging
 import traceback
 from abc import ABC
+from typing import Any, ClassVar, Optional
 
 import django.db.models
 import graphene
@@ -21,6 +22,30 @@
 logger = logging.getLogger(__name__)
 
 
+def _require_io_setting(mutation_cls: type, name: str) -> Any:
+    """Return ``cls.IOSettings.<name>``; raise if it is missing or ``None``.
+
+    ``IOSettings`` is config-by-subclassing — concrete ``DRFMutation`` /
+    ``DRFDeletion`` subclasses are expected to override ``model``,
+    ``serializer`` and ``graphene_model``. Without this guard, a subclass
+    that forgets one of those surfaces as a late ``AttributeError`` /
+    ``TypeError`` inside ``mutate()``, which gets swallowed by the broad
+    ``except Exception`` and reported to the user as a generic
+    "internal error". Failing fast with a clear message here makes the
+    misconfiguration obvious at mutation-invocation time.
+    """
+    io_settings = getattr(mutation_cls, "IOSettings", None)
+    value = (
+        getattr(io_settings, name, None) if io_settings is not None else None
+    )
+    if value is None:
+        raise NotImplementedError(
+            f"{mutation_cls.__name__}.IOSettings.{name} must be set by the "
+            f"subclass."
+        )
+    return value
+
+
 class OpenContractsNode(Node):
     class Meta:
         name = "Node"
@@ -72,7 +97,10 @@ def resolve_total_count(root, info, **kwargs):
 
 class DRFDeletion(graphene.Mutation):
     class IOSettings(ABC):
-        lookup_field = "id"
+        lookup_field: ClassVar[str] = "id"
+        # Concrete subclasses must override ``model`` with their Django model;
+        # see ``_require_io_setting`` for the runtime guard.
+        model: ClassVar[Optional[type[django.db.models.Model]]] = None
 
     class Arguments:
         id = graphene.String(required=False)
@@ -87,10 +115,17 @@ def mutate(cls, root, info, *args, **kwargs):
 
         ok = False
 
-        id = from_global_id(kwargs.get(cls.IOSettings.lookup_field, None))[1]
+        model = _require_io_setting(cls, "model")
+        lookup_field = cls.IOSettings.lookup_field
+        lookup_value = kwargs.get(lookup_field)
+        if lookup_value is None:
+            raise ValueError(
+                f"'{lookup_field}' is required to identify the object to delete."
+            )
+        id = from_global_id(lookup_value)[1]
         # Filter through visible_to_user() to prevent IDOR -- returns same
         # DoesNotExist error whether object is missing or user lacks access.
-        obj = cls.IOSettings.model.objects.visible_to_user(info.context.user).get(pk=id)
+        obj = model.objects.visible_to_user(info.context.user).get(pk=id)
 
         # if there's a user lock, only the lock holder (or superuser) can proceed
         if hasattr(obj, "user_lock") and obj.user_lock is not None:
@@ -123,11 +158,13 @@ def mutate(cls, root, info, *args, **kwargs):
 
 class DRFMutation(graphene.Mutation):
     class IOSettings(ABC):
-        pk_fields: list[str | int] = []
-        lookup_field = "id"
-        model: django.db.models.Model = None
-        graphene_model: DjangoObjectType = None
-        serializer = None
+        pk_fields: ClassVar[list[str]] = []
+        lookup_field: ClassVar[str] = "id"
+        # Concrete subclasses must override ``model``, ``graphene_model`` and
+        # ``serializer``; see ``_require_io_setting`` for the runtime guard.
+        model: ClassVar[Optional[type[django.db.models.Model]]] = None
+        graphene_model: ClassVar[Optional[type[DjangoObjectType]]] = None
+        serializer: ClassVar[Optional[type[serializers.Serializer]]] = None
 
     class Arguments:
         pass
@@ -175,20 +212,22 @@ def mutate(cls, root, info, *args, **kwargs):
                 raise ValueError("No user in this request...")
 
             logger.info(f"DRFMutation - kwargs: {kwargs}")
-            serializer = cls.IOSettings.serializer
+            serializer = _require_io_setting(cls, "serializer")
+            model = _require_io_setting(cls, "model")
+            graphene_model = _require_io_setting(cls, "graphene_model")
 
             if hasattr(cls.IOSettings, "pk_fields"):
                 for pk_field in cls.IOSettings.pk_fields:
                     if pk_field in kwargs:
-                        if isinstance(kwargs[pk_field], list):
-                            pk_value = []
-                            for global_id in kwargs[pk_field]:
-                                # global_id is already the ID string, not a key
-                                pk_value.append(from_global_id(global_id)[1])
+                        raw_value = kwargs[pk_field]
+                        if isinstance(raw_value, list):
+                            kwargs[pk_field] = [
+                                from_global_id(global_id)[1]
+                                for global_id in raw_value
+                            ]
                         else:
-                            logger.info(f"pk field is: {kwargs.get(pk_field, None)}")
-                            pk_value = from_global_id(kwargs.get(pk_field, None))[1]
-                        kwargs[pk_field] = pk_value
+                            logger.info(f"pk field is: {raw_value}")
+                            kwargs[pk_field] = from_global_id(raw_value)[1]
 
             # Check if lookup_field exists in IOSettings and if it's in kwargs
             # This allows create mutations to work without requiring lookup_field
@@ -201,10 +240,8 @@ def mutate(cls, root, info, *args, **kwargs):
                 logger.info("Lookup_field specified - update")
                 # Filter through visible_to_user() to prevent IDOR --
                 # returns same DoesNotExist whether missing or no access.
-                obj = cls.IOSettings.model.objects.visible_to_user(
-                    info.context.user
-                ).get(
-                    pk=from_global_id(kwargs.get(cls.IOSettings.lookup_field, None))[1]
+                obj = model.objects.visible_to_user(info.context.user).get(
+                    pk=from_global_id(kwargs[cls.IOSettings.lookup_field])[1]
                 )
 
                 logger.info(f"Retrieved obj: {obj}")
@@ -240,9 +277,7 @@ def mutate(cls, root, info, *args, **kwargs):
                 obj_serializer.save()
                 ok = True
                 message = "Success"
-                obj_id = to_global_id(
-                    cls.IOSettings.graphene_model.__class__.__name__, obj.id
-                )
+                obj_id = to_global_id(graphene_model.__class__.__name__, obj.id)
                 logger.info("Succeeded updating obj")
 
             else:
@@ -262,9 +297,7 @@ def mutate(cls, root, info, *args, **kwargs):
 
                 ok = True
                 message = "Success"
-                obj_id = to_global_id(
-                    cls.IOSettings.graphene_model.__class__.__name__, obj.id
-                )
+                obj_id = to_global_id(graphene_model.__class__.__name__, obj.id)
 
         except serializers.ValidationError as ve:
             logger.warning(f"Validation error in mutation: {ve.detail}")

docs/typing/mypy_baseline.txt

@@ -31,15 +31,6 @@ config/graphql/annotation_queries.py:641: error: Argument 1 to "from_global_id"
 config/graphql/annotation_queries.py:659: error: Argument 1 to "from_global_id" has incompatible type "Any | None"; expected "str"  [arg-type]
 config/graphql/annotation_queries.py:719: error: Argument 1 to "from_global_id" has incompatible type "Any | None"; expected "str"  [arg-type]
 config/graphql/badge_mutations.py:154: error: Argument 2 to "set_permissions_for_obj_to_user" has incompatible type "Badge"; expected "type[Model]"  [arg-type]
-config/graphql/base.py:128: error: Incompatible types in assignment (expression has type "None", variable has type "Model")  [assignment]
-config/graphql/base.py:190: error: Argument 1 to "from_global_id" has incompatible type "Any | None"; expected "str"  [arg-type]
-config/graphql/base.py:190: error: Incompatible types in assignment (expression has type "str", variable has type "list[str]")  [assignment]
-config/graphql/base.py:204: error: "Model" has no attribute "objects"  [attr-defined]
-config/graphql/base.py:207: error: Argument 1 to "from_global_id" has incompatible type "Any | None"; expected "str"  [arg-type]
-config/graphql/base.py:238: error: "None" not callable  [misc]
-config/graphql/base.py:252: error: "None" not callable  [misc]
-config/graphql/base.py:90: error: Argument 1 to "from_global_id" has incompatible type "Any | None"; expected "str"  [arg-type]
-config/graphql/base.py:93: error: "type[IOSettings]" has no attribute "model"  [attr-defined]
 config/graphql/base_types.py:28: error: Need type annotation for "id_to_children" (hint: "id_to_children: dict[<type>, <type>] = ...")  [var-annotated]
 config/graphql/conversation_mutations.py:148: error: Argument 2 to "set_permissions_for_obj_to_user" has incompatible type "Conversation"; expected "type[Model]"  [arg-type]
 config/graphql/conversation_mutations.py:242: error: Argument 2 to "set_permissions_for_obj_to_user" has incompatible type "ChatMessage"; expected "type[Model]"  [arg-type]

mypy.ini

@@ -46,7 +46,7 @@ ignore_errors = True
 [mypy-config.admin_auth.views]
 ignore_errors = True
 
-# --- config.graphql (35 files) ---
+# --- config.graphql (34 files) ---
 
 [mypy-config.graphql.action_queries]
 ignore_errors = True
@@ -63,9 +63,6 @@ ignore_errors = True
 [mypy-config.graphql.badge_mutations]
 ignore_errors = True
 
-[mypy-config.graphql.base]
-ignore_errors = True
-
 [mypy-config.graphql.base_types]
 ignore_errors = True
 

opencontractserver/tests/test_security_hardening.py

@@ -1205,3 +1205,63 @@ def test_validation_error_list_format(self):
         message = DRFMutation.format_validation_error(exc)
         self.assertIn("Error one.", message)
         self.assertIn("Error two.", message)
+
+
+# ---------------------------------------------------------------------------
+# DRFMutation / DRFDeletion IOSettings misconfiguration guard (issue #1360)
+# ---------------------------------------------------------------------------
+
+
+class TestIOSettingsRequiredFieldsGuard(TestCase):
+    """Subclasses that forget to configure IOSettings should fail with a
+    clear ``NotImplementedError`` rather than a late ``AttributeError`` /
+    ``TypeError`` that pretends to be a generic "internal error"."""
+
+    def test_require_io_setting_raises_when_io_settings_missing(self):
+        from config.graphql.base import _require_io_setting
+
+        class MisconfiguredMutation:
+            pass
+
+        with self.assertRaises(NotImplementedError) as ctx:
+            _require_io_setting(MisconfiguredMutation, "model")
+        self.assertIn("MisconfiguredMutation", str(ctx.exception))
+        self.assertIn("model", str(ctx.exception))
+
+    def test_require_io_setting_raises_when_attribute_none(self):
+        """Mirrors the inherited ``IOSettings`` on ``DRFMutation`` where
+        ``model``/``serializer``/``graphene_model`` default to ``None``."""
+        from config.graphql.base import _require_io_setting
+
+        class MisconfiguredMutation:
+            class IOSettings:
+                model = None
+                serializer = None
+                graphene_model = None
+
+        for field in ("model", "serializer", "graphene_model"):
+            with self.assertRaises(NotImplementedError) as ctx:
+                _require_io_setting(MisconfiguredMutation, field)
+            self.assertIn("MisconfiguredMutation", str(ctx.exception))
+            self.assertIn(field, str(ctx.exception))
+
+    def test_require_io_setting_returns_configured_value(self):
+        from config.graphql.base import _require_io_setting
+
+        class ConfiguredMutation:
+            class IOSettings:
+                model = Corpus
+
+        self.assertIs(_require_io_setting(ConfiguredMutation, "model"), Corpus)
+
+    def test_base_iosettings_defaults_are_none_on_mutation(self):
+        """Regression guard: the base ``DRFMutation.IOSettings`` exposes
+        ``model`` / ``graphene_model`` / ``serializer`` as ``None`` so the
+        runtime guard can detect a missing override. ``DRFDeletion`` now
+        declares ``model`` the same way (previously not declared at all)."""
+        from config.graphql.base import DRFDeletion, DRFMutation
+
+        self.assertIsNone(DRFMutation.IOSettings.model)
+        self.assertIsNone(DRFMutation.IOSettings.serializer)
+        self.assertIsNone(DRFMutation.IOSettings.graphene_model)
+        self.assertIsNone(DRFDeletion.IOSettings.model)