fix: harden unrendered template controls

- Prevent rendered→unrendered downgrade: updating a rendered template
  control with incomplete values now forces a full render attempt,
  returning a clear error about missing parameters instead of silently
  stripping rendered fields
- Deepen unrendered structural validation: validate_template_structure
  now walks definition_template to check $param bindings, reject
  undefined parameter references, detect unused parameters, and reject
  hardcoded agent-scoped evaluator names
- Fix PATCH enabled=false on unrendered templates: detect unrendered
  state before attempting ControlDefinition.model_validate, treating
  disable as a no-op instead of raising CORRUPTED_DATA
- Add 4 behavioral tests: rendered rejects incomplete update, PATCH
  disable no-op, unrendered rejects undefined $param / unused param /
  agent-scoped evaluator

Author: lan17

server/src/agent_control_server/endpoints/controls.py

@@ -186,7 +186,24 @@ async def _materialize_control_input(
                 enabled=enabled,
             )
 
-        # Incomplete values — store as unrendered template.
+        # Incomplete values — only allowed for new controls or already-unrendered
+        # templates.  Updating a rendered control with incomplete values is
+        # rejected to prevent silently stripping rendered fields.
+        current_is_rendered = (
+            current_payload is not None
+            and isinstance(current_payload, dict)
+            and current_payload.get("condition") is not None
+        )
+        if current_is_rendered:
+            # Force a full render attempt so the caller gets a clear error
+            # about which required parameters are missing.
+            enabled = _enabled_from_stored_payload(current_payload)
+            return await _render_and_validate_template_input(
+                control_input,
+                db=db,
+                enabled=enabled,
+            )
+
         validate_template_structure(control_input.template)
         return UnrenderedTemplateControl(
             template=control_input.template,
@@ -1117,57 +1134,64 @@ async def patch_control(
                 ],
             )
 
-        if request.enabled and _is_unrendered_template(control.data):
-            raise APIValidationError(
-                error_code=ErrorCode.VALIDATION_ERROR,
-                detail=(
-                    f"Cannot enable control '{control.name}': "
-                    "unrendered template controls must be rendered first"
-                ),
-                resource="Control",
-                hint=(
-                    f"Provide complete parameter values via PUT /api/v1/controls/{control_id}/data "
-                    "to render the template before enabling."
-                ),
-                errors=[
-                    ValidationErrorItem(
-                        resource="Control",
-                        field="enabled",
-                        code="unrendered_template_cannot_enable",
-                        message="Provide parameter values to render the template before enabling.",
-                    )
-                ],
-            )
-
-        try:
-            ctrl_def = ControlDefinition.model_validate(control.data)
-            if ctrl_def.enabled != request.enabled:
-                new_data = dict(control.data)
-                new_data["enabled"] = request.enabled
-                control.data = new_data
-                updated = True
-            current_enabled = request.enabled if updated else ctrl_def.enabled
-        except ValidationError:
-            _logger.error(
-                "Control '%s' (%s) has corrupted data in patch request",
-                control.name,
-                control_id,
-                exc_info=True,
-            )
-            raise APIValidationError(
-                error_code=ErrorCode.CORRUPTED_DATA,
-                detail=f"Control '{control.name}' has corrupted data",
-                resource="Control",
-                hint="Update the control data using PUT /{control_id}/data.",
-                errors=[
-                    ValidationErrorItem(
-                        resource="Control",
-                        field="data",
-                        code="corrupted_data",
-                        message=_CORRUPTED_CONTROL_DATA_MESSAGE,
-                    )
-                ],
-            )
+        if _is_unrendered_template(control.data):
+            if request.enabled:
+                raise APIValidationError(
+                    error_code=ErrorCode.VALIDATION_ERROR,
+                    detail=(
+                        f"Cannot enable control '{control.name}': "
+                        "unrendered template controls must be rendered first"
+                    ),
+                    resource="Control",
+                    hint=(
+                        "Provide complete parameter values via "
+                        f"PUT /api/v1/controls/{control_id}/data "
+                        "to render the template before enabling."
+                    ),
+                    errors=[
+                        ValidationErrorItem(
+                            resource="Control",
+                            field="enabled",
+                            code="unrendered_template_cannot_enable",
+                            message=(
+                            "Provide parameter values to render the "
+                            "template before enabling."
+                        ),
+                        )
+                    ],
+                )
+            # enabled=False on an unrendered template is a no-op (already false).
+            current_enabled = False
+        else:
+            try:
+                ctrl_def = ControlDefinition.model_validate(control.data)
+                if ctrl_def.enabled != request.enabled:
+                    new_data = dict(control.data)
+                    new_data["enabled"] = request.enabled
+                    control.data = new_data
+                    updated = True
+                current_enabled = request.enabled if updated else ctrl_def.enabled
+            except ValidationError:
+                _logger.error(
+                    "Control '%s' (%s) has corrupted data in patch request",
+                    control.name,
+                    control_id,
+                    exc_info=True,
+                )
+                raise APIValidationError(
+                    error_code=ErrorCode.CORRUPTED_DATA,
+                    detail=f"Control '{control.name}' has corrupted data",
+                    resource="Control",
+                    hint="Update the control data using PUT /{control_id}/data.",
+                    errors=[
+                        ValidationErrorItem(
+                            resource="Control",
+                            field="data",
+                            code="corrupted_data",
+                            message=_CORRUPTED_CONTROL_DATA_MESSAGE,
+                        )
+                    ],
+                )
     elif control.data:
         # Get current enabled status for response
         try:

server/src/agent_control_server/services/control_templates.py

@@ -525,12 +525,57 @@ def _reject_agent_scoped_evaluators(
         )
 
 
+def _collect_param_references(
+    value: JsonValue,
+    *,
+    path_parts: list[str | int],
+    template: TemplateDefinition,
+    referenced: set[str],
+) -> None:
+    """Walk definition_template collecting $param references and validating bindings."""
+    if isinstance(value, dict):
+        if "$param" in value:
+            if len(value) != 1 or not isinstance(value["$param"], str):
+                raise _render_error(
+                    detail="Invalid $param binding object in template definition",
+                    field=_format_rendered_path(path_parts),
+                    code="invalid_param_binding",
+                    message="A $param binding must be exactly {'$param': '<parameter_name>'}.",
+                )
+            parameter_name = value["$param"]
+            if parameter_name not in template.parameters:
+                raise _render_error(
+                    detail=f"Template references undefined parameter '{parameter_name}'",
+                    field=_format_rendered_path(path_parts),
+                    code="undefined_parameter_reference",
+                    message=f"Template references undefined parameter '{parameter_name}'.",
+                )
+            referenced.add(parameter_name)
+            return
+
+        for key, nested in value.items():
+            _collect_param_references(
+                nested,
+                path_parts=[*path_parts, key],
+                template=template,
+                referenced=referenced,
+            )
+    elif isinstance(value, list):
+        for idx, nested in enumerate(value):
+            _collect_param_references(
+                nested,
+                path_parts=[*path_parts, idx],
+                template=template,
+                referenced=referenced,
+            )
+
+
 def validate_template_structure(template: TemplateDefinition) -> None:
     """Validate a template definition's structure without rendering.
 
-    Checks forbidden top-level keys, legacy format, and that ``definition_template``
-    is a dict.  Used for unrendered template creation where parameter values are
-    not yet available.
+    Performs all structural checks that don't require parameter values:
+    forbidden top-level keys, legacy format, $param reference validity,
+    unused parameter detection, and agent-scoped evaluator rejection.
     """
     definition_template = template.definition_template
     if not isinstance(definition_template, dict):
@@ -566,6 +611,74 @@ def validate_template_structure(template: TemplateDefinition) -> None:
             ),
         )
 
+    # Walk the template to validate $param references and collect referenced params.
+    referenced: set[str] = set()
+    _collect_param_references(
+        definition_template,
+        path_parts=[],
+        template=template,
+        referenced=referenced,
+    )
+
+    # Reject unused parameters.
+    unused = sorted(set(template.parameters) - referenced)
+    if unused:
+        raise APIValidationError(
+            error_code=ErrorCode.TEMPLATE_RENDER_ERROR,
+            detail="Template defines parameters that are never referenced",
+            resource="Control",
+            hint="Remove unused parameters or reference them in definition_template.",
+            errors=[
+                ValidationErrorItem(
+                    resource="Control",
+                    field=f"template.parameters.{name}",
+                    code="unused_template_parameter",
+                    message=f"Template parameter '{name}' is never referenced.",
+                    parameter=name,
+                    parameter_label=template.parameters[name].label,
+                )
+                for name in unused
+            ],
+        )
+
+    # Reject agent-scoped evaluator names baked into the template (not via $param).
+    _reject_hardcoded_agent_scoped_evaluators(definition_template)
+
+
+def _reject_hardcoded_agent_scoped_evaluators(
+    definition_template: dict[str, JsonValue],
+) -> None:
+    """Reject agent-scoped evaluator names that are hardcoded in the template."""
+    # Walk condition tree looking for evaluator.name fields containing ':'
+    condition = definition_template.get("condition")
+    if not isinstance(condition, dict):
+        return
+
+    stack: list[dict[str, JsonValue]] = [condition]
+    while stack:
+        node = stack.pop()
+        evaluator = node.get("evaluator")
+        if isinstance(evaluator, dict):
+            name = evaluator.get("name")
+            if isinstance(name, str) and ":" in name:
+                raise _render_error(
+                    detail="Agent-scoped evaluators are not supported in control templates",
+                    field="condition.evaluator.name",
+                    code="agent_scoped_evaluator_not_supported",
+                    message="Agent-scoped evaluators are not supported in control templates.",
+                )
+
+        for key in ("and", "or"):
+            children = node.get(key)
+            if isinstance(children, list):
+                for child in children:
+                    if isinstance(child, dict):
+                        stack.append(child)
+
+        not_child = node.get("not")
+        if isinstance(not_child, dict):
+            stack.append(not_child)
+
 
 def render_template_control_input(
     template_input: TemplateControlInput,

server/tests/test_control_templates.py

@@ -1658,7 +1658,97 @@ def test_unrendered_template_can_be_deleted(client: TestClient) -> None:
     assert response.json()["success"] is True
 
 
-def test_rendered_template_then_update_to_unrendered_stays_rendered(
+def test_patch_disable_on_unrendered_template_is_noop(client: TestClient) -> None:
+    # Given: an unrendered template control (already enabled=false)
+    control_id, _ = _create_unrendered_control(client)
+
+    # When: patching enabled=false (redundant, but should not crash)
+    response = client.patch(
+        f"/api/v1/controls/{control_id}",
+        json={"enabled": False},
+    )
+
+    # Then: the request succeeds without error (no CORRUPTED_DATA)
+    assert response.status_code == 200, response.text
+    assert response.json()["enabled"] is False
+
+
+def test_create_unrendered_template_rejects_undefined_param_reference(
+    client: TestClient,
+) -> None:
+    # Given: a template whose definition_template references an undefined parameter
+    payload = _unrendered_template_payload()
+    payload["template"]["definition_template"]["condition"]["evaluator"]["config"]["extra"] = {  # type: ignore[index]
+        "$param": "nonexistent",
+    }
+
+    # When: creating the unrendered control
+    response = client.put(
+        "/api/v1/controls",
+        json={"name": f"bad-unrendered-{uuid.uuid4()}", "data": payload},
+    )
+
+    # Then: the server rejects with a structural validation error
+    assert response.status_code == 422
+    body = response.json()
+    assert body["error_code"] == "TEMPLATE_RENDER_ERROR"
+    assert any(
+        err.get("code") == "undefined_parameter_reference"
+        for err in body.get("errors", [])
+    )
+
+
+def test_create_unrendered_template_rejects_unused_parameter(
+    client: TestClient,
+) -> None:
+    # Given: a template with an extra parameter never referenced in definition_template
+    payload = _unrendered_template_payload()
+    payload["template"]["parameters"]["unused_param"] = {  # type: ignore[index]
+        "type": "string",
+        "label": "Unused",
+        "default": "val",
+    }
+
+    # When: creating the unrendered control
+    response = client.put(
+        "/api/v1/controls",
+        json={"name": f"unused-param-{uuid.uuid4()}", "data": payload},
+    )
+
+    # Then: the server rejects with unused parameter error
+    assert response.status_code == 422
+    body = response.json()
+    assert body["error_code"] == "TEMPLATE_RENDER_ERROR"
+    assert any(
+        err.get("code") == "unused_template_parameter"
+        for err in body.get("errors", [])
+    )
+
+
+def test_create_unrendered_template_rejects_agent_scoped_evaluator(
+    client: TestClient,
+) -> None:
+    # Given: a template with a hardcoded agent-scoped evaluator name
+    payload = _unrendered_template_payload()
+    payload["template"]["definition_template"]["condition"]["evaluator"]["name"] = "agent-x:custom"  # type: ignore[index]
+
+    # When: creating the unrendered control
+    response = client.put(
+        "/api/v1/controls",
+        json={"name": f"agent-scoped-{uuid.uuid4()}", "data": payload},
+    )
+
+    # Then: the server rejects
+    assert response.status_code == 422
+    body = response.json()
+    assert body["error_code"] == "TEMPLATE_RENDER_ERROR"
+    assert any(
+        err.get("code") == "agent_scoped_evaluator_not_supported"
+        for err in body.get("errors", [])
+    )
+
+
+def test_rendered_template_rejects_update_with_incomplete_values(
     client: TestClient,
 ) -> None:
     # Given: a rendered template control
@@ -1670,9 +1760,13 @@ def test_rendered_template_then_update_to_unrendered_stays_rendered(
         json={"data": _unrendered_template_payload()},
     )
 
-    # Then: the control becomes unrendered (stored without rendered fields)
-    assert put_response.status_code == 200, put_response.text
+    # Then: the server rejects (missing required parameter for a rendered control)
+    assert put_response.status_code == 422
+    body = put_response.json()
+    assert body["error_code"] == "TEMPLATE_PARAMETER_INVALID"
+
+    # And: the control remains rendered and unchanged
     get_response = client.get(f"/api/v1/controls/{control_id}/data")
     data = get_response.json()["data"]
-    assert data["enabled"] is False
-    assert "condition" not in data
+    assert "condition" in data
+    assert data["condition"]["evaluator"]["config"]["pattern"] == "hello"