22104 FIX REST API: reject rule values in an outdated format when creating or editing rules

When creating or editing a rule via the REST API, Checkmk could store a value
that did not match the `value_raw` reported in the API response.

During validation the `value_raw` field is migrated to the current format first
and only then checked. Depending on the ruleset's migration logic, validation
could therefore succeed while the original, unmigrated value was persisted to
disk. Later on, the consuming plugin could stumble over the unmigrated value.
For example, a "Periodic service discovery" rule created this way could send the
"Check_MK Discovery" service into a crash.

The REST API now rejects such requests with an HTTP 400 error instead of silently
migrating the value. Persisting a migrated value would store data the client is
unaware of: unlike the GUI - which renders the migrated value before you save it -
an API client never gets to see the migration, and the `value_raw` reported back
would no longer match the input.

If you receive an "Outdated value format" error, migrate the affected `value_raw`
to the current format - for example by opening and saving the rule once in the
GUI, then reading it back via the REST API - and submit the migrated value.

Change-Id: I71bdaf99237037771c836d59ba34751c6751eaf5
Jira: CMK-35332

Author: crazyscientist

.werks/22104.md

@@ -0,0 +1,32 @@
+[//]: # (werk v3)
+# REST API: reject rule values in an outdated format when creating or editing rules
+
+key        | value
+---------- | ---
+date       | 2026-08-12T00:00:00.000000+00:00
+version    | 3.0.0b1
+class      | fix
+edition    | community
+component  | rest-api
+level      | 1
+compatible | no
+
+When creating or editing a rule via the REST API, Checkmk could store a value
+that did not match the `value_raw` reported in the API response.
+
+During validation the `value_raw` field is migrated to the current format first
+and only then checked. Depending on the ruleset's migration logic, validation
+could therefore succeed while the original, unmigrated value was persisted to
+disk. Later on, the consuming plugin could stumble over the unmigrated value.
+For example, a "Periodic service discovery" rule created this way could send the
+"Check_MK Discovery" service into a crash.
+
+The REST API now rejects such requests with an HTTP 400 error instead of silently
+migrating the value. Persisting a migrated value would store data the client is
+unaware of: unlike the GUI - which renders the migrated value before you save it -
+an API client never gets to see the migration, and the `value_raw` reported back
+would no longer match the input.
+
+If you receive an "Outdated value format" error, migrate the affected `value_raw`
+to the current format - for example by opening and saving the rule once in the
+GUI, then reading it back via the REST API - and submit the migrated value.

cmk/gui/openapi/api_endpoints/rule/_utils.py

@@ -3,6 +3,7 @@
 # This file is part of Checkmk (https://checkmk.com). It is subject to the terms and
 # conditions defined in the file COPYING, which is part of this source code package.
 
+import copy
 import dataclasses
 from collections.abc import Mapping, Sequence
 from typing import Literal
@@ -272,7 +273,20 @@ def label_groups_to_api(label_groups: LabelGroups) -> list[LabelGroupConditionMo
 
 
 def validate_value(ruleset: Ruleset, value: RuleValue) -> None:
-    """Validate a rule value via the form spec, falling back to the legacy valuespec."""
+    """Validate a rule value via the form spec, falling back to the legacy valuespec.
+
+    The value is persisted verbatim, so it must already be in the current format. A value
+    that only becomes valid after migration is rejected instead of being migrated silently:
+    unlike the GUI - which renders the migrated value before the user saves it - the API
+    client never gets to see such a change, so persisting it would store something the
+    client is unaware of (and the ``value_raw`` reported back would no longer match the
+    input).
+
+    The FormSpec path validates the raw value without migration (``migrate_values=False``),
+    so an outdated value is already rejected there. The legacy valuespec validates the
+    migrated value, hence we additionally reject values whose migrated form differs from
+    the input.
+    """
     # FormSpec validation
     try:
         if problems := get_visitor(
@@ -301,6 +315,20 @@ def validate_value(ruleset: Ruleset, value: RuleValue) -> None:
 
         raise ProblemException(status=400, title=title, detail=strip_tags(exc.message))
 
+    # Reject values that are not already in the current format. The legacy valuespec
+    # validates the migrated value, so a value in an outdated format can pass validation
+    # above. We deep-copy before migrating because some migrations mutate their input.
+    if valuespec.transform_value(copy.deepcopy(value)) != value:
+        raise ProblemException(
+            status=400,
+            title="Outdated value format",
+            detail=(
+                "The provided 'value_raw' is in an outdated format. Please migrate it to the "
+                "current format - for example by opening and saving the rule in the GUI - and "
+                "send the migrated value."
+            ),
+        )
+
 
 def get_rule_by_id(
     tree: FolderTree, rule_uuid: str, all_rulesets: AllRulesets | None = None

tests/openapi/test_openapi_rules.py

@@ -278,7 +278,9 @@ def test_create_rule_with_string_value(clients: ClientRegistry) -> None:
     assert resp.json["extensions"]["value_raw"] == "'d,u,r,f,s'"
 
 
-def test_create_rule_stores_migrated_value(clients: ClientRegistry) -> None:
+def test_create_rule_rejects_outdated_form_spec_value(clients: ClientRegistry) -> None:
+    # A form spec value in an outdated format is validated without migration
+    # (migrate_values=False) and therefore rejected instead of migrated silently (Werk 22104).
     ruleset = "diskstat_inventory"
     resp = clients.Rule.create(
         ruleset=ruleset,
@@ -291,6 +293,70 @@ def test_create_rule_stores_migrated_value(clients: ClientRegistry) -> None:
     resp.assert_status_code(400)
 
 
+# A "Periodic service discovery" value using the ancient integer rediscovery mode (0). The
+# ruleset would migrate this to a full 'custom' settings dict. The REST API must not persist
+# such a migrated value silently (unlike the GUI, the client never sees the migration), so
+# it rejects the outdated format instead (Werk 22104).
+LEGACY_VALUE_RAW = """{
+    'check_interval': 120.0,
+    'severity_unmonitored': 1,
+    'severity_vanished': 0,
+    'severity_changed_service_labels': 0,
+    'severity_new_host_label': 1,
+    'inventory_rediscovery': {
+        'group_time': 900,
+        'excluded_time': [],
+        'activation': True,
+        'mode': 0,
+    },
+}"""
+
+
+def test_create_rule_rejects_outdated_legacy_value(clients: ClientRegistry) -> None:
+    # A legacy value that only passes validation after migration must be rejected on create,
+    # not persisted in migrated form (Werk 22104).
+    resp = clients.Rule.create(
+        ruleset="periodic_discovery",
+        folder="/",
+        properties={"description": "legacy mode", "disabled": False},
+        value_raw=LEGACY_VALUE_RAW,
+        conditions={},
+        expect_ok=False,
+    )
+    resp.assert_status_code(400)
+
+    ruleset = clients.Ruleset.get(ruleset_id="periodic_discovery")
+    assert ruleset.json["extensions"]["number_of_rules"] == 0
+
+
+def test_edit_rule_rejects_outdated_legacy_value(clients: ClientRegistry) -> None:
+    # Like test_create_rule_rejects_outdated_legacy_value, but for the edit (PUT) code path:
+    # editing a rule to a legacy value that only passes validation after migration must be
+    # rejected, and must leave the stored value untouched (Werk 22104).
+    created = clients.Rule.create(
+        ruleset="periodic_discovery",
+        folder="/",
+        properties={"description": "current value", "disabled": False},
+        value_raw=DEFAULT_VALUE_RAW,
+        conditions={},
+    )
+
+    resp = clients.Rule.edit(
+        rule_id=created.json["id"],
+        value_raw=LEGACY_VALUE_RAW,
+        properties={"description": "legacy mode", "disabled": False},
+        conditions={},
+        expect_ok=False,
+    )
+    resp.assert_status_code(400)
+
+    # The stored value must be unchanged: still the current-format value, not migrated.
+    rules_mk = paths.omd_root / "etc/check_mk/conf.d/wato/rules.mk"
+    environ = load_mk_file(rules_mk, default={}, lock=False)
+    value = environ["periodic_discovery"][0]["value"]  # type: ignore[index]
+    assert "inventory_rediscovery" not in value
+
+
 def test_openapi_list_rules(
     clients: ClientRegistry,
     new_rule: tuple[Response, dict[str, typing.Any]],