(superset) branch master updated: fix(mcp): reject unknown fields in nested chart-config models too (#42626) (#42732)

[ru...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 1de35d13611 fix(mcp): reject unknown fields in nested chart-config models too (#42626) (#42732)
1de35d13611 is described below

commit 1de35d136117e171c92acaf6cf898ab8ab395a42
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Wed Aug 5 16:24:55 2026 -0700

    fix(mcp): reject unknown fields in nested chart-config models too (#42626) (#42732)
    
    Co-authored-by: Claude Code <no...@anthropic.com>
---
 superset/mcp_service/chart/schemas.py              | 24 +++++++++++--------
 .../mcp_service/chart/test_chart_schemas.py        | 27 ++++++++++++++++++++--
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/superset/mcp_service/chart/schemas.py b/superset/mcp_service/chart/schemas.py
index f58da946088..12bf88e769a 100644
--- a/superset/mcp_service/chart/schemas.py
+++ b/superset/mcp_service/chart/schemas.py
@@ -771,8 +771,8 @@ class UnknownFieldCheckMixin(BaseModel):
         return _check_unknown_fields(data, cls)
 
 
-class ColumnRef(BaseModel):
-    model_config = ConfigDict(populate_by_name=True)
+class ColumnRef(UnknownFieldCheckMixin):
+    model_config = ConfigDict(extra="ignore", populate_by_name=True)
 
     name: str | None = Field(
         None,
@@ -892,21 +892,25 @@ class ColumnRef(BaseModel):
         )
 
 
-class AxisConfig(BaseModel):
+class AxisConfig(UnknownFieldCheckMixin):
+    model_config = ConfigDict(extra="ignore")
+
     title: str | None = Field(None, max_length=200)
     scale: Literal["linear", "log"] | None = "linear"
     format: str | None = Field(None, description="e.g. '$,.2f'", max_length=50)
 
 
-class LegendConfig(BaseModel):
+class LegendConfig(UnknownFieldCheckMixin):
+    model_config = ConfigDict(extra="ignore")
+
     show: bool = True
     position: Literal["top", "bottom", "left", "right"] | None = "right"
 
 
-class CurrencyFormat(BaseModel):
+class CurrencyFormat(UnknownFieldCheckMixin):
     """Currency symbol and placement applied to numeric values."""
 
-    model_config = ConfigDict(populate_by_name=True)
+    model_config = ConfigDict(extra="ignore", populate_by_name=True)
 
     symbol: str = Field(
         ...,
@@ -926,8 +930,8 @@ class CurrencyFormat(BaseModel):
 LEGEND_POSITION_LITERAL = Literal["top", "bottom", "left", "right"]
 
 
-class FilterConfig(BaseModel):
-    model_config = ConfigDict(populate_by_name=True)
+class FilterConfig(UnknownFieldCheckMixin):
+    model_config = ConfigDict(extra="ignore", populate_by_name=True)
 
     column: str = Field(
         ...,
@@ -994,7 +998,7 @@ class FilterConfig(BaseModel):
         return self
 
 
-class SortByConfig(BaseModel):
+class SortByConfig(UnknownFieldCheckMixin):
     """Sort specification with explicit direction.
 
     Accepts either this object or a bare column-name string in `sort_by`
@@ -1002,7 +1006,7 @@ class SortByConfig(BaseModel):
     sort-by-metric "top N" pattern most commonly used for tables.
     """
 
-    model_config = ConfigDict(populate_by_name=True)
+    model_config = ConfigDict(extra="ignore", populate_by_name=True)
 
     column: str = Field(
         ...,
diff --git a/tests/unit_tests/mcp_service/chart/test_chart_schemas.py b/tests/unit_tests/mcp_service/chart/test_chart_schemas.py
index 5290d44042f..7f42c1f0760 100644
--- a/tests/unit_tests/mcp_service/chart/test_chart_schemas.py
+++ b/tests/unit_tests/mcp_service/chart/test_chart_schemas.py
@@ -23,6 +23,7 @@ import pytest
 from pydantic import ValidationError
 
 from superset.mcp_service.chart.schemas import (
+    AxisConfig,
     BigNumberChartConfig,
     ColumnRef,
     FilterConfig,
@@ -705,11 +706,20 @@ class TestUnknownFieldDetection:
             )
 
     def test_known_aliases_not_flagged_as_unknown(self) -> None:
-        """Test that known aliases pass validation without errors."""
+        """Test that known aliases pass validation without errors.
+
+        Uses ``x_column`` rather than ``x_axis`` to set the X column:
+        ``x_axis`` is ambiguous between that alias on ``x`` and the
+        unrelated ``x_axis: AxisConfig`` field of the same name, and
+        pydantic resolves the collision in favor of the real field name.
+        Now that nested models reject unknown fields (#42626), routing a
+        column dict through ``x_axis`` here would raise, not silently
+        no-op into ``AxisConfig`` the way it used to.
+        """
         config = XYChartConfig.model_validate(
             {
                 "chart_type": "xy",
-                "x_axis": {"name": "category"},
+                "x_column": {"name": "category"},
                 "metrics": [{"name": "sales", "aggregate": "SUM"}],
                 "groupby": [{"name": "region"}],
                 "stack": True,
@@ -720,6 +730,19 @@ class TestUnknownFieldDetection:
         assert config.row_limit == 10000
         assert config.group_by is not None
 
+    def test_unknown_field_nested_one_level_down_is_rejected(self) -> None:
+        """
+        Regression for #42626: only the top-level chart config models
+        inherit UnknownFieldCheckMixin. Nested models like AxisConfig are
+        plain BaseModel, so a typo'd field one level down (e.g. inside
+        x_axis) is silently dropped by pydantic's default extra="ignore"
+        instead of raising the same "did you mean?" error a top-level typo
+        gets -- an MCP client (or the LLM driving it) gets no signal that
+        its setting was ignored.
+        """
+        with pytest.raises(ValidationError, match="Unknown field"):
+            AxisConfig.model_validate({"title": "State", "sort_by": "metric"})
+
 
 class TestColumnRefSavedMetric:
     """Test ColumnRef saved_metric support."""