fix: enforce immutable mutability checks at runtime in PATCH operations

Move immutable field validation from parse-time to runtime in patch(),
where the resource instance is available. This aligns with RFC 7644
§3.5.2 which allows adding a value to an immutable attribute only if
it had no previous value, and tolerates no-op operations (remove on
unset fields, replace with identical value).

Author: azmeuk

doc/changelog.rst

@@ -1,6 +1,14 @@
 Changelog
 =========
 
+[0.6.8] - Unreleased
+--------------------
+
+Fixed
+^^^^^
+- PATCH operations on :attr:`~scim2_models.Mutability.read_only` fields now correctly reject ``remove`` in addition to ``add`` and ``replace``.
+- PATCH operations on :attr:`~scim2_models.Mutability.immutable` fields are now validated at runtime per :rfc:`RFC 7644 §3.5.2 <7644#section-3.5.2>`: ``add`` is only allowed when the field has no previous value, ``replace`` is only allowed with the same value, and ``remove`` is only allowed on unset fields.
+
 [0.6.7] - 2026-04-02
 --------------------
 

scim2_models/messages/patch_op.py

@@ -52,28 +52,23 @@ class Op(str, Enum):
     def _validate_mutability(
         self, resource_class: type[Resource[Any]], field_name: str
     ) -> None:
-        """Validate mutability constraints."""
-        # RFC 7644 Section 3.5.2: "Servers should be tolerant of schema extensions"
+        """Validate mutability constraints at parse-time.
+
+        Only :attr:`~scim2_models.Mutability.read_only` is validated here.
+        :attr:`~scim2_models.Mutability.immutable` validation requires access
+        to the resource instance and is enforced at runtime in
+        :meth:`PatchOp._check_immutable`.
+        """
         if field_name not in resource_class.model_fields:
             return
 
         mutability = resource_class.get_field_annotation(field_name, Mutability)
 
-        # RFC 7643 Section 7: "Attributes with mutability 'readOnly' SHALL NOT be modified"
-        if mutability == Mutability.read_only and self.op in (
-            PatchOperation.Op.add,
-            PatchOperation.Op.replace_,
-        ):
+        if mutability == Mutability.read_only:
             raise MutabilityException(
                 attribute=field_name, mutability="readOnly", operation=self.op.value
             ).as_pydantic_error()
 
-        # RFC 7643 Section 7: "Attributes with mutability 'immutable' SHALL NOT be updated"
-        if mutability == Mutability.immutable and self.op == PatchOperation.Op.replace_:
-            raise MutabilityException(
-                attribute=field_name, mutability="immutable", operation=self.op.value
-            ).as_pydantic_error()
-
     def _validate_required_attribute(
         self, resource_class: type[Resource[Any]], field_name: str
     ) -> None:
@@ -284,14 +279,65 @@ def _apply_operation(
         """Apply a single patch operation to a resource.
 
         :return: :data:`True` if the resource was modified, else :data:`False`.
+        :raises MutabilityException: If the operation would modify an
+            immutable attribute.
         """
+        if operation.path is not None:
+            self._check_immutable(resource, operation)
+
         if operation.op in (PatchOperation.Op.add, PatchOperation.Op.replace_):
             return self._apply_add_replace(resource, operation)
         if operation.op == PatchOperation.Op.remove:
             return self._apply_remove(resource, operation)
 
         raise InvalidValueException(detail=f"unsupported operation: {operation.op}")
 
+    def _check_immutable(
+        self, resource: Resource[Any], operation: PatchOperation[ResourceT]
+    ) -> None:
+        """Validate immutable constraints at runtime.
+
+        :rfc:`RFC 7644 §3.5.2 <7644#section-3.5.2>`:
+
+            *"A client MUST NOT modify an attribute that has mutability
+            "readOnly" or "immutable".  However, a client MAY "add" a value
+            to an "immutable" attribute if the attribute had no previous
+            value."*
+
+        An operation is considered a no-op (and thus allowed) when it would
+        not effectively change the resource state: ``remove`` on an unset
+        field, or ``replace`` with the current value.
+        """
+        resource_class = type(resource)
+        assert operation.path is not None
+        field_name = operation.path.parts[0] if operation.path.parts else None
+        if field_name is None or field_name not in resource_class.model_fields:
+            return
+
+        mutability = resource_class.get_field_annotation(field_name, Mutability)
+        if mutability != Mutability.immutable:
+            return
+
+        current_value = getattr(resource, field_name, None)
+
+        if operation.op == PatchOperation.Op.add and current_value is None:
+            return
+
+        if operation.op == PatchOperation.Op.remove and current_value is None:
+            return
+
+        if (
+            operation.op == PatchOperation.Op.replace_
+            and operation.value == current_value
+        ):
+            return
+
+        raise MutabilityException(
+            attribute=field_name,
+            mutability="immutable",
+            operation=operation.op.value,
+        )
+
     def _apply_add_replace(
         self, resource: Resource[Any], operation: PatchOperation[ResourceT]
     ) -> bool:

tests/test_patch_op_replace.py

@@ -1,9 +1,9 @@
 from typing import Annotated
 
 import pytest
-from pydantic import ValidationError
 
 from scim2_models import URN
+from scim2_models import MutabilityException
 from scim2_models import PatchOp
 from scim2_models import PatchOperation
 from scim2_models import User
@@ -205,21 +205,23 @@ def test_replace_operation_with_non_dict_value_no_path():
 
 
 def test_immutable_field():
-    """Test that replace operations on immutable fields raise validation errors."""
+    """Test that replace operations on immutable fields raise mutability errors."""
 
     class Dummy(Resource):
         __schema__ = URN("urn:test:TestResource")
 
         immutable: Annotated[str, Mutability.immutable]
 
-    with pytest.raises(ValidationError, match="mutability"):
-        PatchOp[Dummy](
-            operations=[
-                PatchOperation[Dummy](
-                    op=PatchOperation.Op.replace_, path="immutable", value="new_value"
-                )
-            ]
-        )
+    resource = Dummy.model_construct(immutable="original")
+    patch = PatchOp[Dummy](
+        operations=[
+            PatchOperation[Dummy](
+                op=PatchOperation.Op.replace_, path="immutable", value="new_value"
+            )
+        ]
+    )
+    with pytest.raises(MutabilityException):
+        patch.patch(resource)
 
 
 def test_primary_auto_exclusion_on_add():

tests/test_patch_op_validation.py

@@ -1,3 +1,4 @@
+from typing import Annotated
 from typing import TypeVar
 
 import pytest
@@ -6,13 +7,19 @@
 from scim2_models import Group
 from scim2_models import InvalidPathException
 from scim2_models import InvalidValueException
+from scim2_models import Mutability
+from scim2_models import MutabilityException
 from scim2_models import PatchOp
 from scim2_models import PatchOperation
 from scim2_models import User
 from scim2_models.base import Context
 from scim2_models.resources.resource import Resource
 
 
+class ImmutableFieldResource(Resource):
+    locked: Annotated[str | None, Mutability.immutable] = None
+
+
 def test_patch_op_add_invalid_extension_path():
     user = User(user_name="john")
     patch_op = PatchOp[User](
@@ -257,9 +264,8 @@ def test_validate_mutability_readonly_error():
         )
 
 
-def test_validate_mutability_immutable_error():
-    """Test mutability validation error for immutable attributes."""
-    # Test replace operation on immutable field within groups complex attribute
+def test_validate_mutability_readonly_replace_via_complex_path():
+    """Replacing a readOnly complex attribute path is rejected."""
     with pytest.raises(ValidationError, match="mutability"):
         PatchOp[User].model_validate(
             {
@@ -275,9 +281,83 @@ def test_validate_mutability_immutable_error():
         )
 
 
+def test_patch_remove_on_immutable_field_with_value_is_rejected():
+    """Removing an existing immutable attribute via PATCH is rejected."""
+    resource = ImmutableFieldResource.model_construct(locked="existing")
+    patch_op = PatchOp[ImmutableFieldResource].model_validate(
+        {"operations": [{"op": "remove", "path": "locked"}]},
+        context={"scim": Context.RESOURCE_PATCH_REQUEST},
+    )
+    with pytest.raises(MutabilityException):
+        patch_op.patch(resource)
+
+
+def test_patch_remove_on_immutable_field_without_value_is_allowed():
+    """Removing an unset immutable attribute is a no-op and is allowed."""
+    resource = ImmutableFieldResource.model_construct()
+    patch_op = PatchOp[ImmutableFieldResource].model_validate(
+        {"operations": [{"op": "remove", "path": "locked"}]},
+        context={"scim": Context.RESOURCE_PATCH_REQUEST},
+    )
+    patch_op.patch(resource)
+    assert resource.locked is None
+
+
+def test_patch_add_on_immutable_field_with_existing_value_is_rejected():
+    """Adding to an immutable attribute that already has a value is rejected."""
+    resource = ImmutableFieldResource.model_construct(locked="existing")
+    patch_op = PatchOp[ImmutableFieldResource].model_validate(
+        {"operations": [{"op": "add", "path": "locked", "value": "new"}]},
+        context={"scim": Context.RESOURCE_PATCH_REQUEST},
+    )
+    with pytest.raises(MutabilityException):
+        patch_op.patch(resource)
+
+
+def test_patch_add_on_immutable_field_without_value_is_allowed():
+    """Adding to an immutable attribute with no previous value is allowed per RFC 7644."""
+    resource = ImmutableFieldResource.model_construct()
+    patch_op = PatchOp[ImmutableFieldResource].model_validate(
+        {"operations": [{"op": "add", "path": "locked", "value": "initial"}]},
+        context={"scim": Context.RESOURCE_PATCH_REQUEST},
+    )
+    patch_op.patch(resource)
+    assert resource.locked == "initial"
+
+
+def test_patch_replace_on_immutable_field_with_different_value_is_rejected():
+    """Replacing an immutable attribute with a different value is rejected."""
+    resource = ImmutableFieldResource.model_construct(locked="existing")
+    patch_op = PatchOp[ImmutableFieldResource].model_validate(
+        {"operations": [{"op": "replace", "path": "locked", "value": "other"}]},
+        context={"scim": Context.RESOURCE_PATCH_REQUEST},
+    )
+    with pytest.raises(MutabilityException):
+        patch_op.patch(resource)
+
+
+def test_patch_replace_on_immutable_field_with_same_value_is_allowed():
+    """Replacing an immutable attribute with its current value is a no-op and is allowed."""
+    resource = ImmutableFieldResource.model_construct(locked="existing")
+    patch_op = PatchOp[ImmutableFieldResource].model_validate(
+        {"operations": [{"op": "replace", "path": "locked", "value": "existing"}]},
+        context={"scim": Context.RESOURCE_PATCH_REQUEST},
+    )
+    patch_op.patch(resource)
+    assert resource.locked == "existing"
+
+
+def test_patch_remove_on_readonly_field_is_rejected():
+    """Removing a readOnly attribute via PATCH is rejected per RFC 7643 §7."""
+    with pytest.raises(ValidationError, match="mutability"):
+        PatchOp[User].model_validate(
+            {"operations": [{"op": "remove", "path": "id"}]},
+            context={"scim": Context.RESOURCE_PATCH_REQUEST},
+        )
+
+
 def test_patch_validation_allows_unknown_fields():
-    """Test that patch validation allows unknown fields in operations."""
-    # This should not raise an error even though 'unknownField' doesn't exist on User
+    """Patch operations on unknown fields pass without mutability checks."""
     patch_op = PatchOp[User].model_validate(
         {
             "operations": [
@@ -290,9 +370,8 @@ def test_patch_validation_allows_unknown_fields():
     assert patch_op.operations[0].path == "unknownField"
 
 
-def test_non_replace_operations_on_immutable_fields_allowed():
-    """Test that non-replace operations on immutable fields are allowed."""
-    # Test with non-immutable fields since groups.value is immutable
+def test_patch_operations_on_readwrite_fields_allowed():
+    """All patch operations are allowed on readWrite fields."""
     patch_op = PatchOp[User].model_validate(
         {
             "operations": [